Merge pull request #10314 from eth3lbert/versions-query-ids

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

Author: Turbo87

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);

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<String>,
+
+    /// If set, only versions with the specified semver strings are returned.
+    #[serde(rename = "nums[]", default)]
+    #[param(inline)]
+    nums: Vec<StringExclNull>,
 }
 
 impl ListQueryParams {
@@ -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!(
@@ -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<User>)>::as_select())
             .load_stream::<(Version, Option<User>)>(conn)

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",

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;

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' });