fetch CrateDetails via MatchedRelease to save one SQL query per request

Author: syphar

.sqlx/query-908827d37c731be4a6611eacd2e53aa03ae0b38047e0e58c034a3d34f8e29631.json

@@ -9,7 +9,7 @@ use crate::{
         encode_url_path,
         error::{AxumNope, AxumResult},
         extractors::{DbConnection, Path},
-        ReqVersion,
+        MatchedRelease, ReqVersion,
     },
     AsyncStorage,
 };
@@ -98,11 +98,27 @@ pub struct Release {
 
 impl CrateDetails {
     #[tracing::instrument(skip(conn))]
-    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
@@ -163,9 +179,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(),
@@ -205,7 +218,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,
@@ -399,21 +412,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),
@@ -670,23 +679,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,
@@ -851,10 +881,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!(
@@ -972,10 +999,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,
@@ -1002,10 +1026,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,
@@ -1033,10 +1054,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,
@@ -1072,10 +1090,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,
@@ -1132,10 +1147,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,
@@ -1158,10 +1170,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();
@@ -1185,10 +1194,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,
@@ -1207,10 +1213,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,
@@ -1654,10 +1657,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/crate_details.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 {
@@ -274,6 +277,7 @@ async fn match_version(
                     corrected_name,
                     req_version: input_version.clone(),
                     release: release.clone(),
+                    all_releases: releases,
                 });
             }
 
@@ -302,6 +306,7 @@ async fn match_version(
             corrected_name,
             req_version: input_version.clone(),
             release: release.clone(),
+            all_releases: releases,
         });
     }
 
@@ -311,11 +316,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/mod.rs

@@ -180,12 +180,9 @@ pub(crate) async fn rustdoc_redirector_handler(
             rendering_time.step("serve JS for crate");
 
             return async {
-                let version = matched_release.into_version();
+                let krate = CrateDetails::from_matched_release(&mut conn, &matched_release).await?;
 
-                let krate =
-                    CrateDetails::new(&mut conn, &crate_name, &version, params.version.clone())
-                        .await?
-                        .ok_or(AxumNope::ResourceNotFound)?;
+                let version = matched_release.into_version();
 
                 rendering_time.step("fetch from storage");
 
@@ -409,7 +406,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(
@@ -432,22 +429,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");
@@ -733,14 +722,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.