From b18625440e9c4910bda57d1a8e6e252c30935a96 Mon Sep 17 00:00:00 2001 From: Tobias Bieniek Date: Mon, 4 Mar 2024 11:34:55 +0100 Subject: [PATCH 1/3] views/EncodableCrate: Add `downloads` argument to `from()` fn --- src/controllers/krate/metadata.rs | 1 + src/views.rs | 5 +++-- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/controllers/krate/metadata.rs b/src/controllers/krate/metadata.rs index 8c63982ed10..4dc1874c4e2 100644 --- a/src/controllers/krate/metadata.rs +++ b/src/controllers/krate/metadata.rs @@ -115,6 +115,7 @@ pub async fn show(app: AppState, Path(name): Path, req: Parts) -> AppRes cats.as_deref(), badges, false, + krate.downloads as i64, recent_downloads, ); let encodable_versions = versions_publishers_and_audit_actions.map(|vpa| { diff --git a/src/views.rs b/src/views.rs index 8c9574efb5c..7d0f64f7844 100644 --- a/src/views.rs +++ b/src/views.rs @@ -230,13 +230,13 @@ impl EncodableCrate { categories: Option<&[Category]>, badges: Option>, exact_match: bool, + downloads: i64, recent_downloads: Option, ) -> Self { let Crate { name, created_at, updated_at, - downloads, description, homepage, documentation, @@ -272,7 +272,6 @@ impl EncodableCrate { // behind the number of "recent downloads". to hide this inconsistency // we will use the "recent downloads" as "total downloads" in case it is // higher. - let downloads = downloads as i64; let downloads = if matches!(recent_downloads, Some(x) if x > downloads) { recent_downloads.unwrap() } else { @@ -316,6 +315,7 @@ impl EncodableCrate { exact_match: bool, recent_downloads: Option, ) -> Self { + let downloads = krate.downloads as i64; Self::from( krate, top_versions, @@ -324,6 +324,7 @@ impl EncodableCrate { None, badges, exact_match, + downloads, recent_downloads, ) } From 4ed5d0f9daac9bed4a68030c0de9fa88e928af55 Mon Sep 17 00:00:00 2001 From: Tobias Bieniek Date: Mon, 4 Mar 2024 11:36:11 +0100 Subject: [PATCH 2/3] views/EncodableCrate: Add `downloads` argument to `from_minimal()` fn --- src/controllers/krate/publish.rs | 3 ++- src/controllers/krate/search.rs | 2 ++ src/controllers/summary.rs | 2 ++ src/views.rs | 2 +- 4 files changed, 7 insertions(+), 2 deletions(-) diff --git a/src/controllers/krate/publish.rs b/src/controllers/krate/publish.rs index 554433e7665..429013eccca 100644 --- a/src/controllers/krate/publish.rs +++ b/src/controllers/krate/publish.rs @@ -411,8 +411,9 @@ pub async fn publish(app: AppState, req: BytesRequest) -> AppResult AppResult> { .zip(recent_downloads) .map( |(((max_version, krate), perfect_match), recent_downloads)| { + let downloads = krate.downloads as i64; EncodableCrate::from_minimal( krate, Some(&max_version), Some(vec![]), perfect_match, + downloads, Some(recent_downloads), ) }, diff --git a/src/controllers/summary.rs b/src/controllers/summary.rs index 518b06bf982..cc15572944c 100644 --- a/src/controllers/summary.rs +++ b/src/controllers/summary.rs @@ -35,11 +35,13 @@ pub async fn summary(state: AppState) -> AppResult> { .zip(krates) .zip(recent_downloads) .map(|((top_versions, krate), recent_downloads)| { + let downloads = krate.downloads as i64; Ok(EncodableCrate::from_minimal( krate, Some(&top_versions), None, false, + downloads, recent_downloads, )) }) diff --git a/src/views.rs b/src/views.rs index 7d0f64f7844..52b3a3477c5 100644 --- a/src/views.rs +++ b/src/views.rs @@ -313,9 +313,9 @@ impl EncodableCrate { top_versions: Option<&TopVersions>, badges: Option>, exact_match: bool, + downloads: i64, recent_downloads: Option, ) -> Self { - let downloads = krate.downloads as i64; Self::from( krate, top_versions, From ef105ce117f673b79dd2bd92916ba6b792da5cc9 Mon Sep 17 00:00:00 2001 From: Tobias Bieniek Date: Mon, 4 Mar 2024 14:45:23 +0100 Subject: [PATCH 3/3] controllers/krate/search: Read downloads from `crate_downloads` table --- src/controllers/krate/search.rs | 70 ++++++++++++++++++--------------- 1 file changed, 39 insertions(+), 31 deletions(-) diff --git a/src/controllers/krate/search.rs b/src/controllers/krate/search.rs index e999a3022e1..de00e028855 100644 --- a/src/controllers/krate/search.rs +++ b/src/controllers/krate/search.rs @@ -72,6 +72,7 @@ pub async fn search(app: AppState, req: Parts) -> AppResult> { let selection = ( ALL_COLUMNS, false.into_sql::(), + crate_downloads::downloads, recent_crate_downloads::downloads.nullable(), 0_f32.into_sql::(), ); @@ -80,6 +81,7 @@ pub async fn search(app: AppState, req: Parts) -> AppResult> { let mut seek: Option = None; let mut query = filter_params .make_query(&req, conn)? + .inner_join(crate_downloads::table) .left_join(recent_crate_downloads::table) .select(selection); @@ -97,6 +99,7 @@ pub async fn search(app: AppState, req: Parts) -> AppResult> { query = query.select(( ALL_COLUMNS, Crate::with_name(q_string), + crate_downloads::downloads, recent_crate_downloads::downloads.nullable(), rank.clone(), )); @@ -106,6 +109,7 @@ pub async fn search(app: AppState, req: Parts) -> AppResult> { query = query.select(( ALL_COLUMNS, Crate::with_name(q_string), + crate_downloads::downloads, recent_crate_downloads::downloads.nullable(), 0_f32.into_sql::(), )); @@ -121,7 +125,7 @@ pub async fn search(app: AppState, req: Parts) -> AppResult> { // to ensure predictable pagination behavior. if sort == Some("downloads") { seek = Some(Seek::Downloads); - query = query.order((crates::downloads.desc(), crates::id.desc())) + query = query.order((crate_downloads::downloads.desc(), crates::id.desc())) } else if sort == Some("recent-downloads") { seek = Some(Seek::RecentDownloads); query = query.order(( @@ -170,7 +174,7 @@ pub async fn search(app: AppState, req: Parts) -> AppResult> { pagination, filter_params.make_query(&req, conn)?.count(), ); - let data: Paginated<(Crate, bool, Option, f32)> = + let data: Paginated<(Crate, bool, i64, Option, f32)> = info_span!("db.query", message = "SELECT ..., COUNT(*) FROM crates") .in_scope(|| query.load(conn))?; @@ -187,7 +191,7 @@ pub async fn search(app: AppState, req: Parts) -> AppResult> { pagination, filter_params.make_query(&req, conn)?.count(), ); - let data: Paginated<(Crate, bool, Option, f32)> = + let data: Paginated<(Crate, bool, i64, Option, f32)> = info_span!("db.query", message = "SELECT ..., COUNT(*) FROM crates") .in_scope(|| query.load(conn))?; ( @@ -199,12 +203,15 @@ pub async fn search(app: AppState, req: Parts) -> AppResult> { ) }; - let perfect_matches = data.iter().map(|&(_, b, _, _)| b).collect::>(); - let recent_downloads = data + let perfect_matches = data.iter().map(|&(_, b, _, _, _)| b).collect::>(); + let downloads = data .iter() - .map(|&(_, _, s, _)| s.unwrap_or(0)) + .map(|&(_, _, total, recent, _)| (total, recent.unwrap_or(0))) + .collect::>(); + let crates = data + .into_iter() + .map(|(c, _, _, _, _)| c) .collect::>(); - let crates = data.into_iter().map(|(c, _, _, _)| c).collect::>(); let versions: Vec = info_span!("db.query", message = "SELECT ... FROM versions") .in_scope(|| crates.versions().load(conn))?; @@ -216,20 +223,17 @@ pub async fn search(app: AppState, req: Parts) -> AppResult> { let crates = versions .zip(crates) .zip(perfect_matches) - .zip(recent_downloads) - .map( - |(((max_version, krate), perfect_match), recent_downloads)| { - let downloads = krate.downloads as i64; - EncodableCrate::from_minimal( - krate, - Some(&max_version), - Some(vec![]), - perfect_match, - downloads, - Some(recent_downloads), - ) - }, - ) + .zip(downloads) + .map(|(((max_version, krate), perfect_match), (total, recent))| { + EncodableCrate::from_minimal( + krate, + Some(&max_version), + Some(vec![]), + perfect_match, + total, + Some(recent), + ) + }) .collect::>(); Ok(Json(json!({ @@ -478,12 +482,12 @@ impl<'a> FilterParams<'a> { // `WHERE (downloads = downloads' AND id < id') OR downloads < downloads'` vec![ Box::new( - crates::downloads + crate_downloads::downloads .eq(downloads) .and(crates::id.lt(id)) .nullable(), ), - Box::new(crates::downloads.lt(downloads).nullable()), + Box::new(crate_downloads::downloads.lt(downloads).nullable()), ] } SeekPayload::Query(Query(exact_match, id)) => { @@ -553,14 +557,17 @@ mod seek { New(#[serde(with="ts_microseconds")] chrono::NaiveDateTime, i32) RecentUpdates(#[serde(with="ts_microseconds")] chrono::NaiveDateTime, i32) RecentDownloads(Option, i32) - Downloads(i32, i32) + Downloads(i64, i32) Query(bool, i32) Relevance(bool, f32, i32) } } impl Seek { - pub(crate) fn to_payload(&self, record: &(Crate, bool, Option, f32)) -> SeekPayload { + pub(crate) fn to_payload( + &self, + record: &(Crate, bool, i64, Option, f32), + ) -> SeekPayload { match *self { Seek::Name => SeekPayload::Name(Name(record.0.id)), Seek::New => SeekPayload::New(New(record.0.created_at, record.0.id)), @@ -568,14 +575,12 @@ mod seek { SeekPayload::RecentUpdates(RecentUpdates(record.0.updated_at, record.0.id)) } Seek::RecentDownloads => { - SeekPayload::RecentDownloads(RecentDownloads(record.2, record.0.id)) - } - Seek::Downloads => { - SeekPayload::Downloads(Downloads(record.0.downloads, record.0.id)) + SeekPayload::RecentDownloads(RecentDownloads(record.3, record.0.id)) } + Seek::Downloads => SeekPayload::Downloads(Downloads(record.2, record.0.id)), Seek::Query => SeekPayload::Query(Query(record.1, record.0.id)), Seek::Relevance => { - SeekPayload::Relevance(Relevance(record.1, record.3, record.0.id)) + SeekPayload::Relevance(Relevance(record.1, record.4, record.0.id)) } } } @@ -584,7 +589,10 @@ mod seek { type BoxedCondition<'a> = Box< dyn BoxableExpression< - LeftJoinQuerySource, + LeftJoinQuerySource< + InnerJoinQuerySource, + recent_crate_downloads::table, + >, diesel::pg::Pg, SqlType = diesel::sql_types::Nullable, > + 'a,