Skip to content

Commit

Permalink
controllers/krate/metadata: Add support for default_version include…
Browse files Browse the repository at this point in the history
… mode

This allows us to respond with a version data of `default_version`
included, which potentially benefits apps by eliminating the need to
wait for the crate response to obtain the default version and then make
a subsequent request to get the actual version data. This is
particularly useful when we move towards not requesting with `versions`
included.
  • Loading branch information
eth3lbert committed Dec 30, 2024
1 parent f027ea1 commit 5e2da2c
Show file tree
Hide file tree
Showing 8 changed files with 148 additions and 3 deletions.
27 changes: 25 additions & 2 deletions src/controllers/krate/metadata.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ pub struct FindQueryParams {
/// Additional data to include in the response.
///
/// Valid values: `versions`, `keywords`, `categories`, `badges`,
/// `downloads`, or `full`.
/// `downloads`, `default_version`, or `full`.
///
/// Defaults to `full` for backwards compatibility.
///
Expand Down Expand Up @@ -118,6 +118,21 @@ pub async fn find_crate(
.as_ref()
.map(|vps| vps.iter().map(|v| v.0.id).collect());

let mut default_version_meta = None;
if let Some(default_version) = default_version.as_ref().filter(|_| include.default_version) {
// Find the default_version from composed versions to minimize round trips when possible.
if let Some(versions) = versions_publishers_and_audit_actions.as_ref() {
default_version_meta = versions
.iter()
.find(|(version, _, _)| version.num.as_str() == default_version)
.cloned();
} else if let Ok(version) = krate.find_version(&mut conn, default_version).await {
let published_by = version.published_by(&mut conn).await?;
let actions = VersionOwnerAction::by_version(&mut conn, &version).await?;
default_version_meta = Some((version, published_by, actions));
}
};

let kws = if include.keywords {
Some(
CrateKeyword::belonging_to(&krate)
Expand Down Expand Up @@ -174,6 +189,8 @@ pub async fn find_crate(
.map(|(v, pb, aas)| EncodableVersion::from(v, &krate.name, pb, aas))
.collect::<Vec<_>>()
});
let encodable_default_version =
default_version_meta.map(|(v, pb, aas)| EncodableVersion::from(v, &krate.name, pb, aas));

let encodable_keywords = kws.map(|kws| {
kws.into_iter()
Expand All @@ -192,6 +209,7 @@ pub async fn find_crate(
"versions": encodable_versions,
"keywords": encodable_keywords,
"categories": encodable_cats,
"default_version": encodable_default_version,
}))
}

Expand All @@ -202,6 +220,7 @@ struct ShowIncludeMode {
categories: bool,
badges: bool,
downloads: bool,
default_version: bool,
}

impl Default for ShowIncludeMode {
Expand All @@ -213,13 +232,14 @@ impl Default for ShowIncludeMode {
categories: true,
badges: true,
downloads: true,
default_version: false,
}
}
}

impl ShowIncludeMode {
const INVALID_COMPONENT: &'static str =
"invalid component for ?include= (expected 'versions', 'keywords', 'categories', 'badges', 'downloads', or 'full')";
"invalid component for ?include= (expected 'versions', 'keywords', 'categories', 'badges', 'downloads', 'default_version', or 'full')";
}

impl FromStr for ShowIncludeMode {
Expand All @@ -232,6 +252,7 @@ impl FromStr for ShowIncludeMode {
categories: false,
badges: false,
downloads: false,
default_version: false,
};
for component in s.split(',') {
match component {
Expand All @@ -243,13 +264,15 @@ impl FromStr for ShowIncludeMode {
categories: true,
badges: true,
downloads: true,
default_version: true,
}
}
"versions" => mode.versions = true,
"keywords" => mode.keywords = true,
"categories" => mode.categories = true,
"badges" => mode.badges = true,
"downloads" => mode.downloads = true,
"default_version" => mode.default_version = true,
_ => return Err(bad_request(Self::INVALID_COMPONENT)),
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -523,7 +523,7 @@ snapshot_kind: text
}
},
{
"description": "Additional data to include in the response.\n\nValid values: `versions`, `keywords`, `categories`, `badges`,\n`downloads`, or `full`.\n\nDefaults to `full` for backwards compatibility.\n\nThis parameter expects a comma-separated list of values.",
"description": "Additional data to include in the response.\n\nValid values: `versions`, `keywords`, `categories`, `badges`,\n`downloads`, `default_version`, or `full`.\n\nDefaults to `full` for backwards compatibility.\n\nThis parameter expects a comma-separated list of values.",
"in": "query",
"name": "include",
"required": false,
Expand Down
40 changes: 40 additions & 0 deletions src/tests/routes/crates/read.rs
Original file line number Diff line number Diff line change
Expand Up @@ -178,3 +178,43 @@ async fn test_new_name() {
".crate.updated_at" => "[datetime]",
});
}

#[tokio::test(flavor = "multi_thread")]
async fn test_include_default_version() {
let (app, anon, user) = TestApp::init().with_user().await;
let mut conn = app.db_conn().await;
let user = user.as_model();

CrateBuilder::new("foo_default_version", user.id)
.description("description")
.documentation("https://example.com")
.homepage("http://example.com")
.version(VersionBuilder::new("1.0.0").yanked(true))
.version(VersionBuilder::new("0.5.0"))
.version(VersionBuilder::new("0.5.1"))
.keyword("kw1")
.downloads(20)
.recent_downloads(10)
.expect_build(&mut conn)
.await;

let response = anon
.get::<()>("/api/v1/crates/foo_default_version?include=default_version")
.await;
assert_eq!(response.status(), StatusCode::OK);
assert_json_snapshot!(response.json(), {
".crate.created_at" => "[datetime]",
".crate.updated_at" => "[datetime]",
".default_version.created_at" => "[datetime]",
".default_version.updated_at" => "[datetime]",
});

let response_with_versions = anon
.get::<()>("/api/v1/crates/foo_default_version?include=versions,default_version")
.await;
assert_eq!(response_with_versions.status(), StatusCode::OK);
assert_eq!(
response_with_versions.json().get("default_version"),
response.json().get("default_version")
);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
---
source: src/tests/routes/crates/read.rs
expression: response.json()
snapshot_kind: text
---
{
"categories": null,
"crate": {
"badges": [],
"categories": null,
"created_at": "[datetime]",
"default_version": "0.5.1",
"description": "description",
"documentation": "https://example.com",
"downloads": 20,
"exact_match": false,
"homepage": "http://example.com",
"id": "foo_default_version",
"keywords": null,
"links": {
"owner_team": "/api/v1/crates/foo_default_version/owner_team",
"owner_user": "/api/v1/crates/foo_default_version/owner_user",
"owners": "/api/v1/crates/foo_default_version/owners",
"reverse_dependencies": "/api/v1/crates/foo_default_version/reverse_dependencies",
"version_downloads": "/api/v1/crates/foo_default_version/downloads",
"versions": "/api/v1/crates/foo_default_version/versions"
},
"max_stable_version": null,
"max_version": "0.0.0",
"name": "foo_default_version",
"newest_version": "0.0.0",
"recent_downloads": null,
"repository": null,
"updated_at": "[datetime]",
"versions": null,
"yanked": false
},
"default_version": {
"audit_actions": [],
"bin_names": null,
"checksum": " ",
"crate": "foo_default_version",
"crate_size": 0,
"created_at": "[datetime]",
"description": null,
"dl_path": "/api/v1/crates/foo_default_version/0.5.1/download",
"documentation": null,
"downloads": 0,
"edition": null,
"features": {},
"has_lib": null,
"homepage": null,
"id": 3,
"lib_links": null,
"license": null,
"links": {
"authors": "/api/v1/crates/foo_default_version/0.5.1/authors",
"dependencies": "/api/v1/crates/foo_default_version/0.5.1/dependencies",
"version_downloads": "/api/v1/crates/foo_default_version/0.5.1/downloads"
},
"num": "0.5.1",
"published_by": {
"avatar": null,
"id": 1,
"login": "foo",
"name": null,
"url": "https://github.com/foo"
},
"readme_path": "/api/v1/crates/foo_default_version/0.5.1/readme",
"repository": null,
"rust_version": null,
"updated_at": "[datetime]",
"yank_message": null,
"yanked": false
},
"keywords": null,
"versions": null
}
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ snapshot_kind: text
"versions": null,
"yanked": false
},
"default_version": null,
"keywords": null,
"versions": null
}
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ snapshot_kind: text
],
"yanked": false
},
"default_version": null,
"keywords": [
{
"crates_cnt": 1,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ snapshot_kind: text
],
"yanked": true
},
"default_version": null,
"keywords": [
{
"crates_cnt": 1,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ snapshot_kind: text
"versions": null,
"yanked": false
},
"default_version": null,
"keywords": null,
"versions": null
}

0 comments on commit 5e2da2c

Please sign in to comment.