Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add multiple nums[] parameters support for GET /api/v1/crates/:name/versions #10314

Merged
merged 2 commits into from
Jan 8, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion mirage/route-handlers/crates.js
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,12 @@ export function register(server) {
let crate = schema.crates.findBy({ name });
if (!crate) return notFound();

let versions = crate.versions.sort((a, b) => compareIsoDates(b.created_at, a.created_at));
let versions = crate.versions;
let { nums } = request.queryParams;
if (nums) {
versions = versions.filter(version => nums.includes(version.num));
}
versions = versions.sort((a, b) => compareIsoDates(b.created_at, a.created_at));
let total = versions.length;
let include = request.queryParams?.include ?? '';
let release_tracks = include.split(',').includes('release_tracks') && releaseTracks(crate.versions);
Expand Down
48 changes: 33 additions & 15 deletions src/controllers/krate/versions.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
//! Endpoint for versions of a crate

use axum::extract::{FromRequestParts, Query};
use axum::extract::FromRequestParts;
use axum_extra::extract::Query;
use axum_extra::json;
use axum_extra::response::ErasedJson;
use diesel::dsl::not;
Expand All @@ -19,6 +20,7 @@ use crate::controllers::krate::CratePath;
use crate::models::{User, Version, VersionOwnerAction};
use crate::schema::{users, versions};
use crate::util::errors::{bad_request, AppResult, BoxedAppError};
use crate::util::string_excl_null::StringExclNull;
use crate::util::RequestUtils;
use crate::views::EncodableVersion;

Expand All @@ -41,6 +43,11 @@ pub struct ListQueryParams {
///
/// Defaults to `semver`.
sort: Option<String>,

/// If set, only versions with the specified semver strings are returned.
#[serde(rename = "nums[]", default)]
#[param(inline)]
nums: Vec<StringExclNull>,
}

impl ListQueryParams {
Expand Down Expand Up @@ -123,11 +130,20 @@ async fn list_by_date(
) -> AppResult<PaginatedVersionsAndPublishers> {
use seek::*;

let mut query = versions::table
.filter(versions::crate_id.eq(crate_id))
.left_outer_join(users::table)
.select(<(Version, Option<User>)>::as_select())
.into_boxed();
let make_base_query = || {
let mut query = versions::table
.filter(versions::crate_id.eq(crate_id))
.left_outer_join(users::table)
.select(<(Version, Option<User>)>::as_select())
.into_boxed();

if !params.nums.is_empty() {
query = query.filter(versions::num.eq_any(params.nums.iter().map(|s| s.as_str())));
}
query
};

let mut query = make_base_query();

if let Some(options) = options {
assert!(
Expand Down Expand Up @@ -192,11 +208,7 @@ async fn list_by_date(
// Since the total count is retrieved through an additional query, to maintain consistency
// with other pagination methods, we only make a count query while data is not empty.
let total = if !data.is_empty() {
versions::table
.filter(versions::crate_id.eq(crate_id))
.count()
.get_result(conn)
.await?
make_base_query().count().get_result(conn).await?
} else {
0
};
Expand Down Expand Up @@ -229,6 +241,14 @@ async fn list_by_semver(
use seek::*;

let include = params.include()?;
let mut query = versions::table
.filter(versions::crate_id.eq(crate_id))
.into_boxed();

if !params.nums.is_empty() {
query = query.filter(versions::num.eq_any(params.nums.iter().map(|s| s.as_str())));
}

let (data, total, release_tracks) = if let Some(options) = options {
// Since versions will only increase in the future and both sorting and pagination need to
// happen on the app server, implementing it with fetching only the data needed for sorting
Expand All @@ -239,8 +259,7 @@ async fn list_by_semver(
// while id values are significantly smaller.

let mut sorted_versions = IndexMap::new();
versions::table
.filter(versions::crate_id.eq(crate_id))
query
.select((versions::id, versions::num, versions::yanked))
.load_stream::<(i32, String, bool)>(conn)
.await?
Expand Down Expand Up @@ -313,8 +332,7 @@ async fn list_by_semver(
}
} else {
let mut data = IndexMap::new();
versions::table
.filter(versions::crate_id.eq(crate_id))
query
.left_outer_join(users::table)
.select(<(Version, Option<User>)>::as_select())
.load_stream::<(Version, Option<User>)>(conn)
Expand Down
13 changes: 13 additions & 0 deletions src/snapshots/crates_io__openapi__tests__openapi_snapshot.snap
Original file line number Diff line number Diff line change
Expand Up @@ -855,6 +855,19 @@ expression: response.json()
"type": "string"
}
},
{
"description": "If set, only versions with the specified semver strings are returned.",
"in": "query",
"name": "nums[]",
"required": false,
"schema": {
"items": {
"description": "A string that does not contain null bytes (`\\0`).",
"type": "string"
},
"type": "array"
}
},
{
"description": "The page number to request.\n\nThis parameter is mutually exclusive with `seek` and not supported for\nall requests.",
"in": "query",
Expand Down
65 changes: 65 additions & 0 deletions src/tests/routes/crates/versions/list.rs
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,71 @@ async fn test_sorting() -> anyhow::Result<()> {
Ok(())
}

#[tokio::test(flavor = "multi_thread")]
async fn multiple_ids() -> anyhow::Result<()> {
let (app, anon, user) = TestApp::init().with_user().await;
let mut conn = app.db_conn().await;
let user = user.as_model();
let mut builder = CrateBuilder::new("foo_versions", user.id);

let versions = [
"2.0.0",
"2.0.0-alpha",
"1.0.0-alpha.beta",
"1.0.0-beta.11",
"1.0.0-beta",
"1.0.0",
"0.5.1",
"0.5.0",
];
for version in versions {
builder = builder.version(version);
}
builder.expect_build(&mut conn).await;

// Sort by semver without pagination
let url = "/api/v1/crates/foo_versions/versions";
let query = [
"nums[]=0.5.1",
"nums[]=1.0.0-alpha.beta",
"nums[]=1.0.0-beta",
"nums[]=2.0.0",
"nums[]=unknown",
]
.join("&");
let json: VersionList = anon.get_with_query(url, &query).await.good();
let expects = ["2.0.0", "1.0.0-beta", "1.0.0-alpha.beta", "0.5.1"];
assert_eq!(nums(&json.versions), expects);
assert!(json.meta.next_page.is_none());
assert_eq!(json.meta.total as usize, expects.len());
assert_eq!(json.meta.release_tracks, None);

let (resp, calls) = page_with_seek(&anon, &format!("{url}?{query}")).await;
for (json, expect) in resp.iter().zip(expects) {
assert_eq!(json.versions[0].num, expect);
assert_eq!(json.meta.total as usize, expects.len());
}
assert_eq!(calls as usize, expects.len() + 1);

// Sort by date without pagination
let query = format!("{query}&sort=date");
let json: VersionList = anon.get_with_query(url, &query).await.good();
let expects = ["0.5.1", "1.0.0-beta", "1.0.0-alpha.beta", "2.0.0"];
assert_eq!(nums(&json.versions), expects);
assert!(json.meta.next_page.is_none());
assert_eq!(json.meta.total as usize, expects.len());
assert_eq!(json.meta.release_tracks, None);

let (resp, calls) = page_with_seek(&anon, &format!("{url}?{query}")).await;
for (json, expect) in resp.iter().zip(expects) {
assert_eq!(json.versions[0].num, expect);
assert_eq!(json.meta.total as usize, expects.len());
}
assert_eq!(calls as usize, expects.len() + 1);

Ok(())
}

#[tokio::test(flavor = "multi_thread")]
async fn test_seek_based_pagination_semver_sorting() -> anyhow::Result<()> {
let (app, anon, user) = TestApp::init().with_user().await;
Expand Down
15 changes: 15 additions & 0 deletions tests/mirage/crates/versions/list-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,21 @@ module('Mirage | GET /api/v1/crates/:name/versions', function (hooks) {
});
});

test('supports multiple `ids[]` parameters', async function (assert) {
let user = this.server.create('user');
let crate = this.server.create('crate', { name: 'rand' });
this.server.create('version', { crate, num: '1.0.0' });
this.server.create('version', { crate, num: '1.1.0', publishedBy: user });
this.server.create('version', { crate, num: '1.2.0', rust_version: '1.69' });
let response = await fetch('/api/v1/crates/rand/versions?nums[]=1.0.0&nums[]=1.2.0');
assert.strictEqual(response.status, 200);
let json = await response.json();
assert.deepEqual(
json.versions.map(v => v.num),
['1.0.0', '1.2.0'],
);
});

test('include `release_tracks` meta', async function (assert) {
let user = this.server.create('user');
let crate = this.server.create('crate', { name: 'rand' });
Expand Down
Loading