diff --git a/migrations/2019-05-29-192622_index_crate_downloads_by_downloads/down.sql b/migrations/2019-05-29-192622_index_crate_downloads_by_downloads/down.sql new file mode 100644 index 00000000000..ba704a2ad05 --- /dev/null +++ b/migrations/2019-05-29-192622_index_crate_downloads_by_downloads/down.sql @@ -0,0 +1,8 @@ +DROP MATERIALIZED VIEW recent_crate_downloads; +CREATE MATERIALIZED VIEW recent_crate_downloads (crate_id, downloads) AS + SELECT crate_id, SUM(version_downloads.downloads) FROM version_downloads + INNER JOIN versions + ON version_downloads.version_id = versions.id + WHERE version_downloads.date > date(CURRENT_TIMESTAMP - INTERVAL '90 days') + GROUP BY crate_id; +CREATE UNIQUE INDEX recent_crate_downloads_crate_id ON recent_crate_downloads (crate_id); diff --git a/migrations/2019-05-29-192622_index_crate_downloads_by_downloads/up.sql b/migrations/2019-05-29-192622_index_crate_downloads_by_downloads/up.sql new file mode 100644 index 00000000000..ab26d57aa93 --- /dev/null +++ b/migrations/2019-05-29-192622_index_crate_downloads_by_downloads/up.sql @@ -0,0 +1,10 @@ +DROP MATERIALIZED VIEW recent_crate_downloads; +CREATE MATERIALIZED VIEW recent_crate_downloads (crate_id, downloads) AS + SELECT crate_id, COALESCE(SUM(version_downloads.downloads), 0) FROM versions + LEFT JOIN version_downloads + ON version_downloads.version_id = versions.id + WHERE version_downloads.date > date(CURRENT_TIMESTAMP - INTERVAL '90 days') + OR version_downloads.date IS NULL + GROUP BY crate_id; +CREATE UNIQUE INDEX recent_crate_downloads_crate_id ON recent_crate_downloads (crate_id); +CREATE INDEX ON recent_crate_downloads (downloads); diff --git a/src/controllers/krate/search.rs b/src/controllers/krate/search.rs index e61ba527646..b258b2cdfc9 100644 --- a/src/controllers/krate/search.rs +++ b/src/controllers/krate/search.rs @@ -51,7 +51,7 @@ pub fn search(req: &mut dyn Request) -> CargoResult { recent_crate_downloads::downloads.nullable(), ); let mut query = crates::table - .left_join(recent_crate_downloads::table) + .inner_join(recent_crate_downloads::table) .select(selection) .into_boxed(); @@ -157,7 +157,7 @@ pub fn search(req: &mut dyn Request) -> CargoResult { if sort == "downloads" { query = query.then_order_by(crates::downloads.desc()) } else if sort == "recent-downloads" { - query = query.then_order_by(recent_crate_downloads::downloads.desc().nulls_last()) + query = query.then_order_by(recent_crate_downloads::downloads.desc()) } else if sort == "recent-updates" { query = query.order(crates::updated_at.desc()); } else { @@ -177,7 +177,7 @@ pub fn search(req: &mut dyn Request) -> CargoResult { // FIXME: Use `query.selection()` if that feature ends up in // Diesel 2.0 selection, - coalesce(crates::table.count().single_value(), 0), + coalesce(recent_crate_downloads::table.count().single_value(), 0), )) .limit(limit) .offset(offset) diff --git a/src/router.rs b/src/router.rs index efea8b8f680..f5536264745 100644 --- a/src/router.rs +++ b/src/router.rs @@ -13,7 +13,7 @@ pub fn build_router(app: &App) -> R404 { let mut api_router = RouteBuilder::new(); // Route used by both `cargo search` and the frontend - api_router.get("/crates", C(krate::search::search)); + api_router.get("/crates", C(needs_recent_downloads(krate::search::search))); // Routes used by `cargo` api_router.put("/crates/new", C(krate::publish::publish)); @@ -35,7 +35,10 @@ pub fn build_router(app: &App) -> R404 { api_router.get("/versions/:version_id", C(version::deprecated::show_by_id)); // Routes used by the frontend - api_router.get("/crates/:crate_id", C(krate::metadata::show)); + api_router.get( + "/crates/:crate_id", + C(needs_recent_downloads(krate::metadata::show)), + ); api_router.get("/crates/:crate_id/:version", C(version::metadata::show)); api_router.get( "/crates/:crate_id/:version/readme", @@ -89,7 +92,10 @@ pub fn build_router(app: &App) -> R404 { "/me/crate_owner_invitations/:crate_id", C(crate_owner_invitation::handle_invite), ); - api_router.get("/summary", C(krate::metadata::summary)); + api_router.get( + "/summary", + C(needs_recent_downloads(krate::metadata::summary)), + ); api_router.put("/confirm/:email_token", C(user::me::confirm_user_email)); api_router.put( "/users/:user_id/resend", @@ -125,12 +131,14 @@ pub fn build_router(app: &App) -> R404 { R404(router) } -struct C(pub fn(&mut dyn Request) -> CargoResult); +struct C CargoResult + Send + Sync + 'static>(F); -impl Handler for C { +impl Handler for C +where + F: Fn(&mut dyn Request) -> CargoResult + Send + Sync + 'static, +{ fn call(&self, req: &mut dyn Request) -> Result> { - let C(f) = *self; - match f(req) { + match self.0(req) { Ok(resp) => Ok(resp), Err(e) => match e.response() { Some(response) => Ok(response), @@ -154,6 +162,26 @@ impl Handler for R { } } +fn needs_recent_downloads( + f: fn(&mut dyn Request) -> CargoResult, +) -> impl Fn(&mut dyn Request) -> CargoResult { + use crate::db::RequestTransaction; + use crate::middleware::app::RequestApp; + use diesel::prelude::*; + + move |req| { + let app = req.app(); + if app.config.env == Env::Test { + let conn = req.db_conn()?; + let _ = conn.transaction(|| { + no_arg_sql_function!(refresh_recent_crate_downloads, ()); + diesel::select(refresh_recent_crate_downloads).execute(&*conn) + }); + } + f(req) + } +} + // Can't derive Debug because of RouteBuilder. #[allow(missing_debug_implementations)] pub struct R404(pub RouteBuilder); diff --git a/src/tests/builders.rs b/src/tests/builders.rs index 31a51878933..c09c419e399 100644 --- a/src/tests/builders.rs +++ b/src/tests/builders.rs @@ -230,7 +230,7 @@ impl<'a> CrateBuilder<'a> { } fn build(mut self, connection: &PgConnection) -> CargoResult { - use diesel::{insert_into, select, update}; + use diesel::{insert_into, update}; let mut krate = self .krate @@ -264,9 +264,6 @@ impl<'a> CrateBuilder<'a> { version_downloads::downloads.eq(downloads), )) .execute(connection)?; - - no_arg_sql_function!(refresh_recent_crate_downloads, ()); - select(refresh_recent_crate_downloads).execute(connection)?; } if !self.keywords.is_empty() { diff --git a/src/tests/krate.rs b/src/tests/krate.rs index 554e2410a3c..d0db48b0fcb 100644 --- a/src/tests/krate.rs +++ b/src/tests/krate.rs @@ -248,7 +248,7 @@ fn index_sorting() { let krate2 = CrateBuilder::new("bar_sort", user.id) .description("foo_sort baz_sort foo_sort baz_sort const") .downloads(3333) - .recent_downloads(0) + .recent_downloads(1) .expect_build(conn); let krate3 = CrateBuilder::new("baz_sort", user.id) @@ -331,7 +331,7 @@ fn exact_match_on_queries_with_sort() { let krate2 = CrateBuilder::new("bar_sort", user.id) .description("foo_sort baz_sort foo_sort baz_sort const") .downloads(3333) - .recent_downloads(0) + .recent_downloads(1) .expect_build(conn); let krate3 = CrateBuilder::new("baz_sort", user.id)