From e1095cb68cdc68fd60cb0023b76d6d198d632074 Mon Sep 17 00:00:00 2001 From: eth3lbert Date: Sat, 28 Dec 2024 17:05:16 +0800 Subject: [PATCH 1/2] controllers/krate/versions: Add multiple `nums[]` parameters support for `GET /api/v1/crates/:name/versions` --- src/controllers/krate/versions.rs | 48 +++++++++----- ..._io__openapi__tests__openapi_snapshot.snap | 13 ++++ src/tests/routes/crates/versions/list.rs | 65 +++++++++++++++++++ 3 files changed, 111 insertions(+), 15 deletions(-) diff --git a/src/controllers/krate/versions.rs b/src/controllers/krate/versions.rs index 51733c22fb4..8f763efdac4 100644 --- a/src/controllers/krate/versions.rs +++ b/src/controllers/krate/versions.rs @@ -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; @@ -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; @@ -41,6 +43,11 @@ pub struct ListQueryParams { /// /// Defaults to `semver`. sort: Option, + + /// If set, only versions with the specified semver strings are returned. + #[serde(rename = "nums[]", default)] + #[param(inline)] + nums: Vec, } impl ListQueryParams { @@ -123,11 +130,20 @@ async fn list_by_date( ) -> AppResult { use seek::*; - let mut query = versions::table - .filter(versions::crate_id.eq(crate_id)) - .left_outer_join(users::table) - .select(<(Version, Option)>::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)>::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!( @@ -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 }; @@ -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 @@ -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? @@ -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)>::as_select()) .load_stream::<(Version, Option)>(conn) diff --git a/src/snapshots/crates_io__openapi__tests__openapi_snapshot.snap b/src/snapshots/crates_io__openapi__tests__openapi_snapshot.snap index 5545da1a503..59b58d4e2eb 100644 --- a/src/snapshots/crates_io__openapi__tests__openapi_snapshot.snap +++ b/src/snapshots/crates_io__openapi__tests__openapi_snapshot.snap @@ -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", diff --git a/src/tests/routes/crates/versions/list.rs b/src/tests/routes/crates/versions/list.rs index 041e36f1238..6657145015c 100644 --- a/src/tests/routes/crates/versions/list.rs +++ b/src/tests/routes/crates/versions/list.rs @@ -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; From 285e483728f71a086241b6a63c25df484993ee52 Mon Sep 17 00:00:00 2001 From: eth3lbert Date: Sat, 4 Jan 2025 12:50:57 +0800 Subject: [PATCH 2/2] mirage: Add multiple `nums[]` parameters support for `GET /api/v1/crates/:name/versions` --- mirage/route-handlers/crates.js | 7 ++++++- tests/mirage/crates/versions/list-test.js | 15 +++++++++++++++ 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/mirage/route-handlers/crates.js b/mirage/route-handlers/crates.js index d432e01f02c..6a57695a71e 100644 --- a/mirage/route-handlers/crates.js +++ b/mirage/route-handlers/crates.js @@ -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); diff --git a/tests/mirage/crates/versions/list-test.js b/tests/mirage/crates/versions/list-test.js index bae2ce112bf..42fe9fbd90b 100644 --- a/tests/mirage/crates/versions/list-test.js +++ b/tests/mirage/crates/versions/list-test.js @@ -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' });