From 8048be0f308e0f72ba36aad155e8ed075d791706 Mon Sep 17 00:00:00 2001 From: eth3lbert Date: Wed, 26 Feb 2025 22:00:28 +0800 Subject: [PATCH 1/6] controllers/krate/metadata: Extract `load_versions_and_publishers()` fn --- src/controllers/krate/metadata.rs | 66 ++++++++++++++++++++----------- 1 file changed, 42 insertions(+), 24 deletions(-) diff --git a/src/controllers/krate/metadata.rs b/src/controllers/krate/metadata.rs index 1fe1cb20e9f..0ecf090cd24 100644 --- a/src/controllers/krate/metadata.rs +++ b/src/controllers/krate/metadata.rs @@ -19,7 +19,9 @@ use axum::extract::{FromRequestParts, Query}; use axum_extra::json; use axum_extra::response::ErasedJson; use diesel::prelude::*; -use diesel_async::RunQueryDsl; +use diesel_async::{AsyncPgConnection, RunQueryDsl}; +use futures_util::FutureExt; +use futures_util::future::{BoxFuture, always_ready}; use std::str::FromStr; #[derive(Debug, Deserialize, FromRequestParts, utoipa::IntoParams)] @@ -97,29 +99,25 @@ pub async fn find_crate( .optional()? .ok_or_else(|| crate_not_found(&path.name))?; - let mut versions_publishers_and_audit_actions = if include.versions { - let versions_and_publishers: Vec<(Version, Option)> = Version::belonging_to(&krate) - .left_outer_join(users::table) - .select(<(Version, Option)>::as_select()) - .order_by(versions::id.desc()) - .load(&mut conn) - .await?; - - let versions = versions_and_publishers - .iter() - .map(|(v, _)| v) - .collect::>(); - let actions = VersionOwnerAction::for_versions(&mut conn, &versions).await?; - Some( - versions_and_publishers - .into_iter() - .zip(actions) - .map(|((v, pb), aas)| (v, pb, aas)) - .collect::>(), - ) - } else { - None - }; + let versions_and_publishers = + load_versions_and_publishers(&mut conn, &krate, include.versions).await?; + let mut versions_publishers_and_audit_actions = + if let Some(versions_and_publishers) = versions_and_publishers { + let versions = versions_and_publishers + .iter() + .map(|(v, _)| v) + .collect::>(); + let actions = VersionOwnerAction::for_versions(&mut conn, &versions).await?; + Some( + versions_and_publishers + .into_iter() + .zip(actions) + .map(|((v, pb), aas)| (v, pb, aas)) + .collect::>(), + ) + } else { + None + }; let ids = versions_publishers_and_audit_actions .as_ref() .map(|vps| vps.iter().map(|v| v.0.id).collect()); @@ -226,6 +224,26 @@ pub async fn find_crate( })) } +type VersionsAndPublishers = (Version, Option); + +fn load_versions_and_publishers<'a>( + conn: &mut AsyncPgConnection, + krate: &'a Crate, + includes: bool, +) -> BoxFuture<'a, AppResult>>> { + if !includes { + return always_ready(|| Ok(None)).boxed(); + } + + let fut = Version::belonging_to(&krate) + .left_outer_join(users::table) + .select(<(Version, Option)>::as_select()) + .order_by(versions::id.desc()) + .load(conn); + + async move { Ok(Some(fut.await?)) }.boxed() +} + #[derive(Debug)] struct ShowIncludeMode { versions: bool, From be018e82ab0981e82790b974a97e6871c793a529 Mon Sep 17 00:00:00 2001 From: eth3lbert Date: Wed, 26 Feb 2025 22:36:10 +0800 Subject: [PATCH 2/6] controllers/krate/metadata: Extract `load_default_versions_and_publishers()` fn --- src/controllers/krate/metadata.rs | 64 ++++++++++++++++++++++++++----- 1 file changed, 54 insertions(+), 10 deletions(-) diff --git a/src/controllers/krate/metadata.rs b/src/controllers/krate/metadata.rs index 0ecf090cd24..3c12c8812b7 100644 --- a/src/controllers/krate/metadata.rs +++ b/src/controllers/krate/metadata.rs @@ -128,14 +128,22 @@ pub async fn find_crate( .as_ref() .filter(|_| include.default_version && !include.versions) { - let version = krate.find_version(&mut conn, default_version).await?; - let version = version.ok_or_else(|| version_not_found(&krate.name, default_version))?; - - let (actions, published_by) = tokio::try_join!( - VersionOwnerAction::by_version(&mut conn, &version), - version.published_by(&mut conn), - )?; - versions_publishers_and_audit_actions = Some(vec![(version, published_by, actions)]); + let versions_and_publishers = + load_default_versions_and_publishers(&mut conn, &krate, Some(default_version), true) + .await? + .expect("default_version should exists"); + let versions = versions_and_publishers + .iter() + .map(|(v, _)| v) + .collect::>(); + let actions = VersionOwnerAction::for_versions(&mut conn, &versions).await?; + versions_publishers_and_audit_actions = Some( + versions_and_publishers + .into_iter() + .zip(actions) + .map(|((v, pb), aas)| (v, pb, aas)) + .collect::>(), + ) }; let kws = if include.keywords { @@ -235,12 +243,48 @@ fn load_versions_and_publishers<'a>( return always_ready(|| Ok(None)).boxed(); } - let fut = Version::belonging_to(&krate) + _load_versions_and_publishers(conn, krate, None) +} + +fn load_default_versions_and_publishers<'a>( + conn: &mut AsyncPgConnection, + krate: &'a Crate, + version_num: Option<&'a str>, + includes: bool, +) -> BoxFuture<'a, AppResult>>> { + if !includes || version_num.is_none() { + return always_ready(|| Ok(None)).boxed(); + } + + let fut = _load_versions_and_publishers(conn, krate, version_num); + async move { + let records = fut.await?.ok_or_else(|| { + version_not_found( + &krate.name, + version_num.expect("default_version should not be None"), + ) + })?; + Ok(Some(records)) + } + .boxed() +} + +fn _load_versions_and_publishers<'a>( + conn: &mut AsyncPgConnection, + krate: &'a Crate, + version_num: Option<&'a str>, +) -> BoxFuture<'a, AppResult>>> { + let mut query = Version::belonging_to(&krate) .left_outer_join(users::table) .select(<(Version, Option)>::as_select()) .order_by(versions::id.desc()) - .load(conn); + .into_boxed(); + + if let Some(num) = version_num { + query = query.filter(versions::num.eq(num)); + } + let fut = query.load(conn); async move { Ok(Some(fut.await?)) }.boxed() } From 472ad84daccbb5d68464b9f8ba2e61d1b4acb163 Mon Sep 17 00:00:00 2001 From: eth3lbert Date: Wed, 26 Feb 2025 22:44:39 +0800 Subject: [PATCH 3/6] controllers/krate/metadata: Extract `load_keywords()` fn --- src/controllers/krate/metadata.rs | 28 +++++++++++++++++----------- 1 file changed, 17 insertions(+), 11 deletions(-) diff --git a/src/controllers/krate/metadata.rs b/src/controllers/krate/metadata.rs index 3c12c8812b7..29654c0caed 100644 --- a/src/controllers/krate/metadata.rs +++ b/src/controllers/krate/metadata.rs @@ -146,17 +146,7 @@ pub async fn find_crate( ) }; - let kws = if include.keywords { - Some( - CrateKeyword::belonging_to(&krate) - .inner_join(keywords::table) - .select(Keyword::as_select()) - .load(&mut conn) - .await?, - ) - } else { - None - }; + let kws = load_keywords(&mut conn, &krate, include.keywords).await?; let cats = if include.categories { Some( CrateCategory::belonging_to(&krate) @@ -269,6 +259,22 @@ fn load_default_versions_and_publishers<'a>( .boxed() } +fn load_keywords<'a>( + conn: &mut AsyncPgConnection, + krate: &'a Crate, + includes: bool, +) -> BoxFuture<'a, AppResult>>> { + if !includes { + return always_ready(|| Ok(None)).boxed(); + } + + let fut = CrateKeyword::belonging_to(&krate) + .inner_join(keywords::table) + .select(Keyword::as_select()) + .load(conn); + async move { Ok(Some(fut.await?)) }.boxed() +} + fn _load_versions_and_publishers<'a>( conn: &mut AsyncPgConnection, krate: &'a Crate, From 01311d946560dc2ae16aba1c3888d3f7a6a05d2c Mon Sep 17 00:00:00 2001 From: eth3lbert Date: Wed, 26 Feb 2025 22:46:40 +0800 Subject: [PATCH 4/6] controllers/krate/metadata: Extract `load_categories()` fn --- src/controllers/krate/metadata.rs | 28 +++++++++++++++++----------- 1 file changed, 17 insertions(+), 11 deletions(-) diff --git a/src/controllers/krate/metadata.rs b/src/controllers/krate/metadata.rs index 29654c0caed..612a72f248d 100644 --- a/src/controllers/krate/metadata.rs +++ b/src/controllers/krate/metadata.rs @@ -147,17 +147,7 @@ pub async fn find_crate( }; let kws = load_keywords(&mut conn, &krate, include.keywords).await?; - let cats = if include.categories { - Some( - CrateCategory::belonging_to(&krate) - .inner_join(categories::table) - .select(Category::as_select()) - .load(&mut conn) - .await?, - ) - } else { - None - }; + let cats = load_categories(&mut conn, &krate, include.categories).await?; let recent_downloads = if include.downloads { RecentCrateDownloads::belonging_to(&krate) .select(recent_crate_downloads::downloads) @@ -275,6 +265,22 @@ fn load_keywords<'a>( async move { Ok(Some(fut.await?)) }.boxed() } +fn load_categories<'a>( + conn: &mut AsyncPgConnection, + krate: &'a Crate, + includes: bool, +) -> BoxFuture<'a, AppResult>>> { + if !includes { + return always_ready(|| Ok(None)).boxed(); + } + + let fut = CrateCategory::belonging_to(&krate) + .inner_join(categories::table) + .select(Category::as_select()) + .load(conn); + async move { Ok(Some(fut.await?)) }.boxed() +} + fn _load_versions_and_publishers<'a>( conn: &mut AsyncPgConnection, krate: &'a Crate, From 63e52f1fe5008db4d1cf241e0f1356c29338a6e7 Mon Sep 17 00:00:00 2001 From: eth3lbert Date: Wed, 26 Feb 2025 22:52:57 +0800 Subject: [PATCH 5/6] controllers/krate/metadata: Extract `load_recent_downloads()` fn --- src/controllers/krate/metadata.rs | 25 ++++++++++++++++--------- 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/src/controllers/krate/metadata.rs b/src/controllers/krate/metadata.rs index 612a72f248d..a9571caa5d6 100644 --- a/src/controllers/krate/metadata.rs +++ b/src/controllers/krate/metadata.rs @@ -148,15 +148,7 @@ pub async fn find_crate( let kws = load_keywords(&mut conn, &krate, include.keywords).await?; let cats = load_categories(&mut conn, &krate, include.categories).await?; - let recent_downloads = if include.downloads { - RecentCrateDownloads::belonging_to(&krate) - .select(recent_crate_downloads::downloads) - .get_result(&mut conn) - .await - .optional()? - } else { - None - }; + let recent_downloads = load_recent_downloads(&mut conn, &krate, include.downloads).await?; let top_versions = if let Some(versions) = versions_publishers_and_audit_actions .as_ref() @@ -281,6 +273,21 @@ fn load_categories<'a>( async move { Ok(Some(fut.await?)) }.boxed() } +fn load_recent_downloads<'a>( + conn: &mut AsyncPgConnection, + krate: &'a Crate, + includes: bool, +) -> BoxFuture<'a, AppResult>> { + if !includes { + return always_ready(|| Ok(None)).boxed(); + } + + let fut = RecentCrateDownloads::belonging_to(&krate) + .select(recent_crate_downloads::downloads) + .get_result(conn); + async move { Ok(fut.await.optional()?) }.boxed() +} + fn _load_versions_and_publishers<'a>( conn: &mut AsyncPgConnection, krate: &'a Crate, From 9611c72f19b966bba8670b22aa84ab5bc7b81484 Mon Sep 17 00:00:00 2001 From: eth3lbert Date: Wed, 26 Feb 2025 23:03:33 +0800 Subject: [PATCH 6/6] controllers/krate/metadata: Pipeline multiple queries --- src/controllers/krate/metadata.rs | 58 +++++++++++++------------------ 1 file changed, 24 insertions(+), 34 deletions(-) diff --git a/src/controllers/krate/metadata.rs b/src/controllers/krate/metadata.rs index a9571caa5d6..7e2068dd54c 100644 --- a/src/controllers/krate/metadata.rs +++ b/src/controllers/krate/metadata.rs @@ -99,57 +99,47 @@ pub async fn find_crate( .optional()? .ok_or_else(|| crate_not_found(&path.name))?; - let versions_and_publishers = - load_versions_and_publishers(&mut conn, &krate, include.versions).await?; - let mut versions_publishers_and_audit_actions = - if let Some(versions_and_publishers) = versions_and_publishers { - let versions = versions_and_publishers - .iter() - .map(|(v, _)| v) - .collect::>(); - let actions = VersionOwnerAction::for_versions(&mut conn, &versions).await?; - Some( - versions_and_publishers - .into_iter() - .zip(actions) - .map(|((v, pb), aas)| (v, pb, aas)) - .collect::>(), - ) - } else { - None - }; - let ids = versions_publishers_and_audit_actions + // Since `versions` and `default_version` share the same key (versions), we should only settle + // the `include.default_version` when `include.versions` is not included, and ignore when no + // `default_version` available. + let include_default_version = + include.default_version && !include.versions && default_version.is_some(); + let (versions_and_publishers, default_versions_and_publishers, kws, cats, recent_downloads) = tokio::try_join!( + load_versions_and_publishers(&mut conn, &krate, include.versions), + load_default_versions_and_publishers( + &mut conn, + &krate, + default_version.as_deref(), + include_default_version, + ), + load_keywords(&mut conn, &krate, include.keywords), + load_categories(&mut conn, &krate, include.categories), + load_recent_downloads(&mut conn, &krate, include.downloads), + )?; + + let ids = versions_and_publishers .as_ref() .map(|vps| vps.iter().map(|v| v.0.id).collect()); - // Since `versions` and `default_version` share the same key (versions), we should only settle - // the `default_version` when `versions` is not included. - if let Some(default_version) = default_version - .as_ref() - .filter(|_| include.default_version && !include.versions) + let versions_publishers_and_audit_actions = if let Some(versions_and_publishers) = + versions_and_publishers.or(default_versions_and_publishers) { - let versions_and_publishers = - load_default_versions_and_publishers(&mut conn, &krate, Some(default_version), true) - .await? - .expect("default_version should exists"); let versions = versions_and_publishers .iter() .map(|(v, _)| v) .collect::>(); let actions = VersionOwnerAction::for_versions(&mut conn, &versions).await?; - versions_publishers_and_audit_actions = Some( + Some( versions_and_publishers .into_iter() .zip(actions) .map(|((v, pb), aas)| (v, pb, aas)) .collect::>(), ) + } else { + None }; - let kws = load_keywords(&mut conn, &krate, include.keywords).await?; - let cats = load_categories(&mut conn, &krate, include.categories).await?; - let recent_downloads = load_recent_downloads(&mut conn, &krate, include.downloads).await?; - let top_versions = if let Some(versions) = versions_publishers_and_audit_actions .as_ref() .filter(|_| include.versions)