fetch CrateDetails via MatchedRelease to save one SQL query per request

src/web/crate_details.rs

@@ -9,7 +9,7 @@ use crate::{
         encode_url_path,
         error::{AxumNope, AxumResult},
         extractors::{DbConnection, Path},
-        ReqVersion,
+        MatchedRelease, ReqVersion,
     },
     AsyncStorage,
 };
@@ -97,11 +97,27 @@ pub struct Release {
 }
 
 impl CrateDetails {
-    pub async fn new(
+    pub(crate) async fn from_matched_release(
+        conn: &mut sqlx::PgConnection,
+        release: &MatchedRelease,
+    ) -> Result<Self> {
+        Ok(Self::new(
+            conn,
+            &release.name,
+            release.version(),
+            Some(release.req_version.clone()),
+            release.all_releases.clone(),
+        )
+        .await?
+        .unwrap())
+    }
+
+    async fn new(
         conn: &mut sqlx::PgConnection,
         name: &str,
         version: &Version,
         req_version: Option<ReqVersion>,
+        prefetched_releases: Vec<Release>,
     ) -> Result<Option<CrateDetails>, anyhow::Error> {
         let krate = match sqlx::query!(
             r#"SELECT
@@ -162,9 +178,6 @@ impl CrateDetails {
             None => return Ok(None),
         };
 
-        // get releases, sorted by semver
-        let releases = releases_for_crate(conn, krate.crate_id).await?;
-
         let repository_metadata = krate.repo_host.map(|_| RepositoryMetadata {
             issues: krate.repo_issues.unwrap(),
             stars: krate.repo_stars.unwrap(),
@@ -204,7 +217,7 @@ impl CrateDetails {
             keywords: krate.keywords,
             have_examples: krate.have_examples,
             target_name: krate.target_name,
-            releases,
+            releases: prefetched_releases,
             repository_metadata,
             metadata,
             is_library: krate.is_library,
@@ -398,21 +411,17 @@ pub(crate) async fn crate_details_handler(
         )
     })?;
 
-    let version = match_version(&mut conn, &params.name, &req_version)
+    let matched_release = match_version(&mut conn, &params.name, &req_version)
         .await?
         .assume_exact_name()?
         .into_canonical_req_version_or_else(|version| {
             AxumNope::Redirect(
                 format!("/crate/{}/{}", &params.name, version),
                 CachePolicy::ForeverInCdn,
             )
-        })?
-        .into_version();
+        })?;
 
-    let mut details =
-        CrateDetails::new(&mut conn, &params.name, &version, Some(req_version.clone()))
-            .await?
-            .ok_or(AxumNope::VersionNotFound)?;
+    let mut details = CrateDetails::from_matched_release(&mut conn, &matched_release).await?;
 
     match details.fetch_readme(&storage).await {
         Ok(readme) => details.readme = readme.or(details.readme),
@@ -669,23 +678,44 @@ mod tests {
         assert_cache_control, assert_redirect, assert_redirect_cached, async_wrapper, wrapper,
         TestDatabase, TestEnvironment,
     };
-    use anyhow::{Context, Error};
+    use anyhow::Error;
     use kuchikiki::traits::TendrilSink;
     use semver::Version;
     use std::collections::HashMap;
 
+    async fn crate_details(
+        conn: &mut sqlx::PgConnection,
+        name: &str,
+        version: &str,
+        req_version: Option<ReqVersion>,
+    ) -> CrateDetails {
+        let crate_id: i32 = sqlx::query_scalar!("SELECT id FROM crates WHERE name = $1", name)
+            .fetch_one(&mut *conn)
+            .await
+            .unwrap();
+
+        let releases = releases_for_crate(&mut *conn, crate_id).await.unwrap();
+
+        CrateDetails::new(
+            &mut *conn,
+            name,
+            &Version::parse(version).unwrap(),
+            req_version,
+            releases,
+        )
+        .await
+        .unwrap()
+        .unwrap()
+    }
+
     async fn assert_last_successful_build_equals(
         db: &TestDatabase,
         package: &str,
         version: &str,
         expected_last_successful_build: Option<&str>,
     ) -> Result<(), Error> {
         let mut conn = db.async_conn().await;
-        let details =
-            CrateDetails::new(&mut conn, package, &Version::parse(version).unwrap(), None)
-                .await
-                .with_context(|| anyhow::anyhow!("could not fetch crate details"))?
-                .unwrap();
+        let details = crate_details(&mut conn, package, version, None).await;
 
         assert_eq!(
             details.last_successful_build,
@@ -850,10 +880,7 @@ mod tests {
 
             let details = env.runtime().block_on(async move {
                 let mut conn = db.async_conn().await;
-                CrateDetails::new(&mut conn, "foo", &"0.2.0".parse().unwrap(), None)
-                    .await
-                    .unwrap()
-                    .unwrap()
+                crate_details(&mut conn, "foo", "0.2.0", None).await
             });
 
             assert_eq!(
@@ -971,10 +998,7 @@ mod tests {
             for version in &["0.0.1", "0.0.2", "0.0.3"] {
                 let details = env.runtime().block_on(async move {
                     let mut conn = db.async_conn().await;
-                    CrateDetails::new(&mut conn, "foo", &version.parse().unwrap(), None)
-                        .await
-                        .unwrap()
-                        .unwrap()
+                    crate_details(&mut conn, "foo", version, None).await
                 });
                 assert_eq!(
                     details.latest_release().unwrap().version,
@@ -1001,10 +1025,7 @@ mod tests {
             for version in &["0.0.1", "0.0.2", "0.0.3-pre.1"] {
                 let details = env.runtime().block_on(async move {
                     let mut conn = db.async_conn().await;
-                    CrateDetails::new(&mut conn, "foo", &version.parse().unwrap(), None)
-                        .await
-                        .unwrap()
-                        .unwrap()
+                    crate_details(&mut conn, "foo", version, None).await
                 });
                 assert_eq!(
                     details.latest_release().unwrap().version,
@@ -1032,10 +1053,7 @@ mod tests {
             for version in &["0.0.1", "0.0.2", "0.0.3"] {
                 let details = env.runtime().block_on(async move {
                     let mut conn = db.async_conn().await;
-                    CrateDetails::new(&mut conn, "foo", &(version.parse().unwrap()), None)
-                        .await
-                        .unwrap()
-                        .unwrap()
+                    crate_details(&mut conn, "foo", version, None).await
                 });
                 assert_eq!(
                     details.latest_release().unwrap().version,
@@ -1071,10 +1089,7 @@ mod tests {
             for version in &["0.0.1", "0.0.2", "0.0.3"] {
                 let details = env.runtime().block_on(async move {
                     let mut conn = db.async_conn().await;
-                    CrateDetails::new(&mut conn, "foo", &version.parse().unwrap(), None)
-                        .await
-                        .unwrap()
-                        .unwrap()
+                    crate_details(&mut conn, "foo", version, None).await
                 });
                 assert_eq!(
                     details.latest_release().unwrap().version,
@@ -1131,10 +1146,7 @@ mod tests {
 
             let details = env.runtime().block_on(async move {
                 let mut conn = db.async_conn().await;
-                CrateDetails::new(&mut conn, "foo", &"0.0.1".parse().unwrap(), None)
-                    .await
-                    .unwrap()
-                    .unwrap()
+                crate_details(&mut conn, "foo", "0.0.1", None).await
             });
             assert_eq!(
                 details.owners,
@@ -1157,10 +1169,7 @@ mod tests {
 
             let details = env.runtime().block_on(async move {
                 let mut conn = db.async_conn().await;
-                CrateDetails::new(&mut conn, "foo", &"0.0.1".parse().unwrap(), None)
-                    .await
-                    .unwrap()
-                    .unwrap()
+                crate_details(&mut conn, "foo", "0.0.1", None).await
             });
             let mut owners = details.owners;
             owners.sort();
@@ -1184,10 +1193,7 @@ mod tests {
 
             let details = env.runtime().block_on(async move {
                 let mut conn = db.async_conn().await;
-                CrateDetails::new(&mut conn, "foo", &"0.0.1".parse().unwrap(), None)
-                    .await
-                    .unwrap()
-                    .unwrap()
+                crate_details(&mut conn, "foo", "0.0.1", None).await
             });
             assert_eq!(
                 details.owners,
@@ -1206,10 +1212,7 @@ mod tests {
 
             let details = env.runtime().block_on(async move {
                 let mut conn = db.async_conn().await;
-                CrateDetails::new(&mut conn, "foo", &"0.0.1".parse().unwrap(), None)
-                    .await
-                    .unwrap()
-                    .unwrap()
+                crate_details(&mut conn, "foo", "0.0.1", None).await
             });
             assert_eq!(
                 details.owners,
@@ -1653,10 +1656,7 @@ mod tests {
 
             let details = env.runtime().block_on(async move {
                 let mut conn = env.async_db().await.async_conn().await;
-                CrateDetails::new(&mut conn, "dummy", &"0.5.0".parse().unwrap(), None)
-                    .await
-                    .unwrap()
-                    .unwrap()
+                crate_details(&mut conn, "dummy", "0.5.0", None).await
             });
             assert!(matches!(
                 env.runtime()

src/web/mod.rs

@@ -111,7 +111,7 @@ impl FromStr for ReqVersion {
 }
 
 #[derive(Debug)]
-struct MatchedRelease {
+pub(crate) struct MatchedRelease {
     /// crate name
     pub name: String,
 
@@ -125,6 +125,9 @@ struct MatchedRelease {
 
     /// the matched release
     pub release: crate_details::Release,
+
+    /// all releases since we have them anyways and so we can pass them to CrateDetails
+    all_releases: Vec<crate_details::Release>,
 }
 
 impl MatchedRelease {
@@ -277,6 +280,7 @@ async fn match_version(
                     corrected_name,
                     req_version: input_version.clone(),
                     release: release.clone(),
+                    all_releases: releases,
                 });
             }
 
@@ -305,6 +309,7 @@ async fn match_version(
             corrected_name,
             req_version: input_version.clone(),
             release: release.clone(),
+            all_releases: releases,
         });
     }
 
@@ -314,11 +319,13 @@ async fn match_version(
     if req_semver == VersionReq::STAR {
         return releases
             .first()
+            .cloned()
             .map(|release| MatchedRelease {
                 name: name.to_owned(),
                 corrected_name: corrected_name.clone(),
                 req_version: input_version.clone(),
-                release: release.clone(),
+                release,
+                all_releases: releases,
             })
             .ok_or(AxumNope::VersionNotFound);
     }

src/web/rustdoc.rs

@@ -176,11 +176,10 @@ pub(crate) async fn rustdoc_redirector_handler(
         if target.ends_with(".js") {
             // this URL is actually from a crate-internal path, serve it there instead
             rendering_time.step("serve JS for crate");
-            let version = matched_release.into_version();
 
-            let krate = CrateDetails::new(&mut conn, &crate_name, &version, params.version.clone())
-                .await?
-                .ok_or(AxumNope::ResourceNotFound)?;
+            let krate = CrateDetails::from_matched_release(&mut conn, &matched_release).await?;
+
+            let version = matched_release.into_version();
 
             rendering_time.step("fetch from storage");
 
@@ -414,7 +413,7 @@ pub(crate) async fn rustdoc_html_server_handler(
     // * If both the name and the version are an exact match, return the version of the crate.
     // * If there is an exact match, but the requested crate name was corrected (dashes vs. underscores), redirect to the corrected name.
     // * If there is a semver (but not exact) match, redirect to the exact version.
-    let version = match_version(&mut conn, &params.name, &params.version)
+    let matched_release = match_version(&mut conn, &params.name, &params.version)
         .await?
         .into_exactly_named_or_else(|corrected_name, req_version| {
             AxumNope::Redirect(
@@ -437,22 +436,14 @@ pub(crate) async fn rustdoc_html_server_handler(
                 )),
                 CachePolicy::ForeverInCdn,
             )
-        })?
-        .into_version();
+        })?;
 
     trace!("crate details");
     rendering_time.step("crate details");
 
     // Get the crate's details from the database
-    // NOTE: we know this crate must exist because we just checked it above (or else `match_version` is buggy)
-    let krate = CrateDetails::new(
-        &mut conn,
-        &params.name,
-        &version,
-        Some(params.version.clone()),
-    )
-    .await?
-    .ok_or(AxumNope::ResourceNotFound)?;
+    let krate = CrateDetails::from_matched_release(&mut conn, &matched_release).await?;
+    let version = matched_release.into_version();
 
     if !krate.rustdoc_status {
         rendering_time.step("redirect to crate");
@@ -736,14 +727,12 @@ pub(crate) async fn target_redirect_handler(
     mut conn: DbConnection,
     Extension(storage): Extension<Arc<AsyncStorage>>,
 ) -> AxumResult<impl IntoResponse> {
-    let version = match_version(&mut conn, &name, &req_version)
+    let matched_release = match_version(&mut conn, &name, &req_version)
         .await?
-        .into_canonical_req_version_or_else(|_| AxumNope::VersionNotFound)?
-        .into_version();
+        .into_canonical_req_version_or_else(|_| AxumNope::VersionNotFound)?;
 
-    let crate_details = CrateDetails::new(&mut conn, &name, &version, Some(req_version.clone()))
-        .await?
-        .ok_or(AxumNope::VersionNotFound)?;
+    let crate_details = CrateDetails::from_matched_release(&mut conn, &matched_release).await?;
+    let version = matched_release.into_version();
 
     // We're trying to find the storage location
     // for the requested path in the target-redirect.