Convert krate::show_minimal into a mode of krate::show

Author: nightkr

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;
@@ -111,83 +112,145 @@ 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<User>)> = 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::<Vec<_>>();
-    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::<Vec<_>>();
+    let versions_publishers_and_audit_actions = if include.full {
+        let mut versions_and_publishers: Vec<(Version, Option<User>)> = 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::<Vec<_>>();
+        Some(
+            versions_and_publishers
+                .into_iter()
+                .zip(VersionOwnerAction::for_versions(&conn, &versions)?.into_iter())
+                .map(|((v, pb), aas)| (v, pb, aas))
+                .collect::<Vec<_>>(),
+        )
+    } 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.full {
+        Some(
+            CrateKeyword::belonging_to(&krate)
+                .inner_join(keywords::table)
+                .select(keywords::all_columns)
+                .load(&*conn)?,
+        )
+    } else {
+        None
+    };
+    let cats = if include.full {
+        Some(
+            CrateCategory::belonging_to(&krate)
+                .inner_join(categories::table)
+                .select(categories::all_columns)
+                .load(&*conn)?,
+        )
+    } else {
+        None
+    };
+    let recent_downloads = if include.full {
+        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.full {
+        Some(
+            badges::table
+                .filter(badges::crate_id.eq(krate.id))
+                .load(&*conn)?,
+        )
+    } else {
+        None
+    };
+    let top_versions = if include.full {
+        Some(krate.top_versions(&conn)?)
+    } else {
+        None
+    };
 
-    Ok(req.json(&json!({
-        "crate": EncodableCrate::from(
-            krate.clone(),
-            Some(&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::<Vec<_>>(),
-        "keywords": kws.into_iter().map(Keyword::into).collect::<Vec<EncodableKeyword>>(),
-        "categories": cats.into_iter().map(Category::into).collect::<Vec<EncodableCategory>>(),
+            .collect::<Vec<_>>()
+    });
+    let encodable_keywords = kws.map(|kws| {
+        kws.into_iter()
+            .map(Keyword::into)
+            .collect::<Vec<EncodableKeyword>>()
+    });
+    let encodable_cats = cats.map(|cats| {
+        cats.into_iter()
+            .map(Category::into)
+            .collect::<Vec<EncodableCategory>>()
+    });
+    Ok(req.json(&json!({
+        "crate": encodable_crate,
+        "versions": encodable_versions,
+        "keywords": encodable_keywords,
+        "categories": encodable_cats,
     })))
 }
 
-/// Handles the `GET /crates/:crate_id/crate` route.
-///
-/// A minimal version of [`show`] that only covers the crate itself, without versions or catalog information
-/// (such as keywords and categories).
-pub fn show_minimal(req: &mut dyn RequestExt) -> EndpointResult {
-    let name = &req.params()["crate_id"];
-    let conn = req.db_read_only()?;
-    let krate: Crate = Crate::by_name(name).first(&*conn)?;
+#[derive(Debug)]
+struct ShowIncludeMode {
+    full: bool,
+}
 
-    Ok(req.json(&EncodableCrate::from_minimal(
-        krate, None, None, false, None,
-    )))
+impl Default for ShowIncludeMode {
+    fn default() -> Self {
+        // Send everything for legacy clients that expect the full response
+        Self { full: true }
+    }
+}
+
+impl FromStr for ShowIncludeMode {
+    type Err = Box<dyn AppError>;
+
+    fn from_str(s: &str) -> Result<Self, Self::Err> {
+        match s {
+            "" => Ok(ShowIncludeMode { full: false }),
+            "full" => Ok(ShowIncludeMode { full: true }),
+            _ => Err(bad_request(
+                "invalid value for ?mode= (expected '' or 'full')",
+            )),
+        }
+    }
 }
 
 /// Handles the `GET /crates/:crate_id/:version/readme` route.

src/router.rs

@@ -36,7 +36,6 @@ pub fn build_router(app: &App) -> RouteBuilder {
 
     // Routes used by the frontend
     api_router.get("/crates/:crate_id", C(krate::metadata::show));
-    api_router.get("/crates/:crate_id/crate", C(krate::metadata::show_minimal));
     api_router.get("/crates/:crate_id/:version", C(version::metadata::show));
     api_router.get(
         "/crates/:crate_id/:version/readme",

src/tests/all.rs

@@ -69,8 +69,8 @@ struct CrateMeta {
 pub struct CrateResponse {
     #[serde(rename = "crate")]
     krate: EncodableCrate,
-    versions: Vec<EncodableVersion>,
-    keywords: Vec<EncodableKeyword>,
+    versions: Option<Vec<EncodableVersion>>,
+    keywords: Option<Vec<EncodableKeyword>>,
 }
 #[derive(Deserialize)]
 pub struct VersionResponse {

src/tests/krate/show.rs

@@ -43,27 +43,29 @@ 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
     );
 }
@@ -102,14 +104,16 @@ fn show_minimal() {
     });
 
     let json = anon.show_crate_minimal("foo_show_minimal");
-    assert_eq!(json.name, krate.name);
-    assert_eq!(json.id, krate.name);
-    assert_eq!(json.description, krate.description);
-    assert_eq!(json.homepage, krate.homepage);
-    assert_eq!(json.documentation, krate.documentation);
-    assert_eq!(json.keywords, None);
-    assert_eq!(json.recent_downloads, None);
-    assert_eq!(json.versions, None);
+    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]

src/tests/util.rs

@@ -23,10 +23,7 @@ use crate::{
     builders::PublishBuilder, CategoryListResponse, CategoryResponse, CrateList, CrateResponse,
     GoodCrate, OkBool, OwnersResponse, VersionResponse,
 };
-use cargo_registry::{
-    models::{ApiToken, CreatedApiToken, User},
-    views::EncodableCrate,
-};
+use cargo_registry::models::{ApiToken, CreatedApiToken, User};
 
 use conduit::{BoxError, Handler, Method};
 use conduit_cookie::SessionMiddleware;
@@ -106,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<T>(&self, path: &str, query: &str) -> Response<T> {
+        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<T>(&self, path: &str, query: &str) -> Response<T> {
@@ -166,9 +171,9 @@ pub trait RequestHelper {
     }
 
     /// Request the JSON used for a crate's minimal page
-    fn show_crate_minimal(&self, krate_name: &str) -> EncodableCrate {
-        let url = format!("/api/v1/crates/{krate_name}/crate");
-        self.get(&url).good()
+    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

src/tests/version.rs

@@ -150,13 +150,17 @@ 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");
     assert_eq!(version1.crate_size, Some(35));
 
     let version2 = crate_json
         .versions
+        .as_ref()
+        .unwrap()
         .iter()
         .find(|v| v.num == "2.0.0")
         .expect("Could not find v2.0.0");