diff --git a/src/controllers/krate/metadata.rs b/src/controllers/krate/metadata.rs index 3e3c64c308e..0aac33c30ee 100644 --- a/src/controllers/krate/metadata.rs +++ b/src/controllers/krate/metadata.rs @@ -5,6 +5,7 @@ //! `Cargo.toml` file. use std::cmp::Reverse; +use std::str::FromStr; use crate::controllers::frontend_prelude::*; use crate::controllers::helpers::pagination::PaginationOptions; @@ -45,7 +46,7 @@ pub fn summary(req: &mut dyn RequestExt) -> EndpointResult { .map(|((top_versions, krate), recent_downloads)| { Ok(EncodableCrate::from_minimal( krate, - &top_versions, + Some(&top_versions), None, false, recent_downloads, @@ -111,71 +112,183 @@ pub fn summary(req: &mut dyn RequestExt) -> EndpointResult { /// Handles the `GET /crates/:crate_id` route. pub fn show(req: &mut dyn RequestExt) -> EndpointResult { let name = &req.params()["crate_id"]; + let include = req + .query() + .get("include") + .map(|mode| ShowIncludeMode::from_str(mode)) + .transpose()? + .unwrap_or_default(); + let conn = req.db_read_only()?; let krate: Crate = Crate::by_name(name).first(&*conn)?; - let mut versions_and_publishers: Vec<(Version, Option)> = krate - .all_versions() - .left_outer_join(users::table) - .select((versions::all_columns, users::all_columns.nullable())) - .load(&*conn)?; - - versions_and_publishers - .sort_by_cached_key(|(version, _)| Reverse(semver::Version::parse(&version.num).ok())); - - let versions = versions_and_publishers - .iter() - .map(|(v, _)| v) - .cloned() - .collect::>(); - let versions_publishers_and_audit_actions = versions_and_publishers - .into_iter() - .zip(VersionOwnerAction::for_versions(&conn, &versions)?.into_iter()) - .map(|((v, pb), aas)| (v, pb, aas)) - .collect::>(); + let versions_publishers_and_audit_actions = if include.versions { + let mut versions_and_publishers: Vec<(Version, Option)> = krate + .all_versions() + .left_outer_join(users::table) + .select((versions::all_columns, users::all_columns.nullable())) + .load(&*conn)?; + versions_and_publishers + .sort_by_cached_key(|(version, _)| Reverse(semver::Version::parse(&version.num).ok())); + + let versions = versions_and_publishers + .iter() + .map(|(v, _)| v) + .cloned() + .collect::>(); + Some( + versions_and_publishers + .into_iter() + .zip(VersionOwnerAction::for_versions(&conn, &versions)?.into_iter()) + .map(|((v, pb), aas)| (v, pb, aas)) + .collect::>(), + ) + } else { + None + }; let ids = versions_publishers_and_audit_actions - .iter() - .map(|v| v.0.id) - .collect(); - - let kws = CrateKeyword::belonging_to(&krate) - .inner_join(keywords::table) - .select(keywords::all_columns) - .load(&*conn)?; - let cats = CrateCategory::belonging_to(&krate) - .inner_join(categories::table) - .select(categories::all_columns) - .load(&*conn)?; - let recent_downloads = RecentCrateDownloads::belonging_to(&krate) - .select(recent_crate_downloads::downloads) - .get_result(&*conn) - .optional()?; + .as_ref() + .map(|vps| vps.iter().map(|v| v.0.id).collect()); + + let kws = if include.keywords { + Some( + CrateKeyword::belonging_to(&krate) + .inner_join(keywords::table) + .select(keywords::all_columns) + .load(&*conn)?, + ) + } else { + None + }; + let cats = if include.categories { + Some( + CrateCategory::belonging_to(&krate) + .inner_join(categories::table) + .select(categories::all_columns) + .load(&*conn)?, + ) + } else { + None + }; + let recent_downloads = if include.downloads { + RecentCrateDownloads::belonging_to(&krate) + .select(recent_crate_downloads::downloads) + .get_result(&*conn) + .optional()? + } else { + None + }; - let badges = badges::table - .filter(badges::crate_id.eq(krate.id)) - .load(&*conn)?; - let top_versions = krate.top_versions(&conn)?; + let badges = if include.badges { + Some( + badges::table + .filter(badges::crate_id.eq(krate.id)) + .load(&*conn)?, + ) + } else { + None + }; + let top_versions = if include.versions { + Some(krate.top_versions(&conn)?) + } else { + None + }; - Ok(req.json(&json!({ - "crate": EncodableCrate::from( - krate.clone(), - &top_versions, - Some(ids), - Some(&kws), - Some(&cats), - Some(badges), - false, - recent_downloads, - ), - "versions": versions_publishers_and_audit_actions - .into_iter() + let encodable_crate = EncodableCrate::from( + krate.clone(), + top_versions.as_ref(), + ids, + kws.as_deref(), + cats.as_deref(), + badges, + false, + recent_downloads, + ); + let encodable_versions = versions_publishers_and_audit_actions.map(|vpa| { + vpa.into_iter() .map(|(v, pb, aas)| EncodableVersion::from(v, &krate.name, pb, aas)) - .collect::>(), - "keywords": kws.into_iter().map(Keyword::into).collect::>(), - "categories": cats.into_iter().map(Category::into).collect::>(), + .collect::>() + }); + let encodable_keywords = kws.map(|kws| { + kws.into_iter() + .map(Keyword::into) + .collect::>() + }); + let encodable_cats = cats.map(|cats| { + cats.into_iter() + .map(Category::into) + .collect::>() + }); + Ok(req.json(&json!({ + "crate": encodable_crate, + "versions": encodable_versions, + "keywords": encodable_keywords, + "categories": encodable_cats, }))) } +#[derive(Debug)] +struct ShowIncludeMode { + versions: bool, + keywords: bool, + categories: bool, + badges: bool, + downloads: bool, +} + +impl Default for ShowIncludeMode { + fn default() -> Self { + // Send everything for legacy clients that expect the full response + Self { + versions: true, + keywords: true, + categories: true, + badges: true, + downloads: true, + } + } +} + +impl ShowIncludeMode { + const INVALID_COMPONENT: &'static str = + "invalid component for ?include= (expected 'versions', 'keywords', 'categories', 'badges', 'downloads', or 'full')"; +} + +impl FromStr for ShowIncludeMode { + type Err = Box; + + fn from_str(s: &str) -> Result { + let mut mode = Self { + versions: false, + keywords: false, + categories: false, + badges: false, + downloads: false, + }; + for component in s.split(',') { + match component { + "" => {} + "full" => { + mode = Self { + versions: true, + keywords: true, + categories: true, + badges: true, + downloads: true, + } + } + "versions" => mode.versions = true, + "keywords" => mode.keywords = true, + "categories" => mode.categories = true, + "badges" => mode.badges = true, + "downloads" => mode.downloads = true, + _ => return Err(bad_request(Self::INVALID_COMPONENT)), + } + } + Ok(mode) + } +} + /// Handles the `GET /crates/:crate_id/:version/readme` route. pub fn readme(req: &mut dyn RequestExt) -> EndpointResult { let crate_name = &req.params()["crate_id"]; diff --git a/src/controllers/krate/publish.rs b/src/controllers/krate/publish.rs index 361d808bd17..165b9b86e89 100644 --- a/src/controllers/krate/publish.rs +++ b/src/controllers/krate/publish.rs @@ -252,7 +252,7 @@ pub fn publish(req: &mut dyn RequestExt) -> EndpointResult { }; Ok(req.json(&GoodCrate { - krate: EncodableCrate::from_minimal(krate, &top_versions, None, false, None), + krate: EncodableCrate::from_minimal(krate, Some(&top_versions), None, false, None), warnings, })) }) diff --git a/src/controllers/krate/search.rs b/src/controllers/krate/search.rs index 3951776e1a0..116e8147511 100644 --- a/src/controllers/krate/search.rs +++ b/src/controllers/krate/search.rs @@ -330,7 +330,7 @@ pub fn search(req: &mut dyn RequestExt) -> EndpointResult { |((((max_version, krate), perfect_match), recent_downloads), badges)| { EncodableCrate::from_minimal( krate, - &max_version, + Some(&max_version), Some(badges), perfect_match, Some(recent_downloads), diff --git a/src/tests/all.rs b/src/tests/all.rs index 493a1b63a75..07badc60c81 100644 --- a/src/tests/all.rs +++ b/src/tests/all.rs @@ -69,8 +69,8 @@ struct CrateMeta { pub struct CrateResponse { #[serde(rename = "crate")] krate: EncodableCrate, - versions: Vec, - keywords: Vec, + versions: Option>, + keywords: Option>, } #[derive(Deserialize)] pub struct VersionResponse { diff --git a/src/tests/krate/show.rs b/src/tests/krate/show.rs index 6177fc62185..49d89a90b10 100644 --- a/src/tests/krate/show.rs +++ b/src/tests/krate/show.rs @@ -43,31 +43,79 @@ fn show() { assert_eq!(json.krate.documentation, krate.documentation); assert_eq!(json.krate.keywords, Some(vec!["kw1".into()])); assert_eq!(json.krate.recent_downloads, Some(10)); - let versions = json.krate.versions.as_ref().unwrap(); + let crate_versions = json.krate.versions.as_ref().unwrap(); + assert_eq!(crate_versions.len(), 3); + let versions = json.versions.as_ref().unwrap(); assert_eq!(versions.len(), 3); - assert_eq!(json.versions.len(), 3); - assert_eq!(json.versions[0].id, versions[0]); - assert_eq!(json.versions[0].krate, json.krate.id); - assert_eq!(json.versions[0].num, "1.0.0"); - assert_none!(&json.versions[0].published_by); + assert_eq!(versions[0].id, crate_versions[0]); + assert_eq!(versions[0].krate, json.krate.id); + assert_eq!(versions[0].num, "1.0.0"); + assert_none!(&versions[0].published_by); let suffix = "/api/v1/crates/foo_show/1.0.0/download"; assert!( - json.versions[0].dl_path.ends_with(suffix), + versions[0].dl_path.ends_with(suffix), "bad suffix {}", - json.versions[0].dl_path + versions[0].dl_path ); - assert_eq!(1, json.keywords.len()); - assert_eq!("kw1", json.keywords[0].id); + let keywords = json.keywords.as_ref().unwrap(); + assert_eq!(1, keywords.len()); + assert_eq!("kw1", keywords[0].id); - assert_eq!(json.versions[1].num, "0.5.1"); - assert_eq!(json.versions[2].num, "0.5.0"); + assert_eq!(versions[1].num, "0.5.1"); + assert_eq!(versions[2].num, "0.5.0"); assert_eq!( - json.versions[1].published_by.as_ref().unwrap().login, + versions[1].published_by.as_ref().unwrap().login, user.gh_login ); } +#[test] +fn show_minimal() { + let (app, anon, user) = TestApp::init().with_user(); + let user = user.as_model(); + + let krate = app.db(|conn| { + use cargo_registry::schema::versions; + use diesel::{update, ExpressionMethods}; + + let krate = CrateBuilder::new("foo_show_minimal", user.id) + .description("description") + .documentation("https://example.com") + .homepage("http://example.com") + .version(VersionBuilder::new("1.0.0")) + .version(VersionBuilder::new("0.5.0")) + .version(VersionBuilder::new("0.5.1")) + .keyword("kw1") + .downloads(20) + .recent_downloads(10) + .expect_build(conn); + + // Make version 1.0.0 mimic a version published before we started recording who published + // versions + let none: Option = None; + update(versions::table) + .filter(versions::num.eq("1.0.0")) + .set(versions::published_by.eq(none)) + .execute(conn) + .unwrap(); + + krate + }); + + let json = anon.show_crate_minimal("foo_show_minimal"); + assert_eq!(json.krate.name, krate.name); + assert_eq!(json.krate.id, krate.name); + assert_eq!(json.krate.description, krate.description); + assert_eq!(json.krate.homepage, krate.homepage); + assert_eq!(json.krate.documentation, krate.documentation); + assert_eq!(json.krate.keywords, None); + assert_eq!(json.krate.recent_downloads, None); + assert_eq!(json.krate.versions, None); + assert!(json.versions.is_none()); + assert!(json.keywords.is_none()); +} + #[test] fn block_bad_documentation_url() { let (app, anon, user) = TestApp::init().with_user(); diff --git a/src/tests/util.rs b/src/tests/util.rs index e2f4d69f554..e96cc1ea362 100644 --- a/src/tests/util.rs +++ b/src/tests/util.rs @@ -103,6 +103,14 @@ pub trait RequestHelper { self.run(self.get_request(path)) } + /// Issue a GET request with a query string + #[track_caller] + fn get_query(&self, path: &str, query: &str) -> Response { + let mut req = self.get_request(path); + req.with_query(query); + self.run(req) + } + /// Issue a GET request that includes query parameters #[track_caller] fn get_with_query(&self, path: &str, query: &str) -> Response { @@ -162,6 +170,12 @@ pub trait RequestHelper { self.get(&url).good() } + /// Request the JSON used for a crate's minimal page + fn show_crate_minimal(&self, krate_name: &str) -> CrateResponse { + let url = format!("/api/v1/crates/{krate_name}"); + self.get_query(&url, "include=").good() + } + /// Request the JSON used to list a crate's owners fn show_crate_owners(&self, krate_name: &str) -> OwnersResponse { let url = format!("/api/v1/crates/{krate_name}/owners"); diff --git a/src/tests/version.rs b/src/tests/version.rs index 610c62182d1..82c4bbe567f 100644 --- a/src/tests/version.rs +++ b/src/tests/version.rs @@ -150,6 +150,8 @@ fn version_size() { let version1 = crate_json .versions + .as_ref() + .unwrap() .iter() .find(|v| v.num == "1.0.0") .expect("Could not find v1.0.0"); @@ -157,6 +159,8 @@ fn version_size() { let version2 = crate_json .versions + .as_ref() + .unwrap() .iter() .find(|v| v.num == "2.0.0") .expect("Could not find v2.0.0"); diff --git a/src/views.rs b/src/views.rs index af6ce34733a..787fafb968a 100644 --- a/src/views.rs +++ b/src/views.rs @@ -238,7 +238,7 @@ impl EncodableCrate { #[allow(clippy::too_many_arguments)] pub fn from( krate: Crate, - top_versions: &TopVersions, + top_versions: Option<&TopVersions>, versions: Option>, keywords: Option<&[Keyword]>, categories: Option<&[Category]>, @@ -267,18 +267,18 @@ impl EncodableCrate { let documentation = Self::remove_blocked_documentation_urls(documentation); let max_version = top_versions - .highest - .as_ref() + .and_then(|v| v.highest.as_ref()) .map(|v| v.to_string()) .unwrap_or_else(|| "0.0.0".to_string()); let newest_version = top_versions - .newest - .as_ref() + .and_then(|v| v.newest.as_ref()) .map(|v| v.to_string()) .unwrap_or_else(|| "0.0.0".to_string()); - let max_stable_version = top_versions.highest_stable.as_ref().map(|v| v.to_string()); + let max_stable_version = top_versions + .and_then(|v| v.highest_stable.as_ref()) + .map(|v| v.to_string()); EncodableCrate { id: name.clone(), @@ -312,7 +312,7 @@ impl EncodableCrate { pub fn from_minimal( krate: Crate, - top_versions: &TopVersions, + top_versions: Option<&TopVersions>, badges: Option>, exact_match: bool, recent_downloads: Option,