Auto merge of #4649 - rust-lang:exclude-top, r=Turbo87

Add an environment variable to let us exclude crate IDs from the recent downloads list

I haven't figured out a great way to add a test for this because I need to create a crate and get a crate ID and then change the app's config, and the test app doesn't easily have this... so I thought I'd throw up this change for discussion and see if anyone had better ideas.

Author: bors

src/config.rs

@@ -25,6 +25,7 @@ pub struct Server {
     pub blocked_traffic: Vec<(String, Vec<String>)>,
     pub max_allowed_page_offset: u32,
     pub page_offset_ua_blocklist: Vec<String>,
+    pub excluded_crate_names: Vec<String>,
     pub domain_name: String,
     pub allowed_origins: Vec<String>,
     pub downloads_persist_interval_ms: usize,
@@ -84,6 +85,11 @@ impl Default for Server {
             Some(s) => s.split(',').map(String::from).collect(),
         };
         let base = Base::from_environment();
+        let excluded_crate_names = match env_optional::<String>("EXCLUDED_CRATE_NAMES") {
+            None => vec![],
+            Some(s) if s.is_empty() => vec![],
+            Some(s) => s.split(',').map(String::from).collect(),
+        };
         Server {
             db: DatabasePools::full_from_environment(&base),
             base,
@@ -97,6 +103,7 @@ impl Default for Server {
             blocked_traffic: blocked_traffic(),
             max_allowed_page_offset: env_optional("WEB_MAX_ALLOWED_PAGE_OFFSET").unwrap_or(200),
             page_offset_ua_blocklist,
+            excluded_crate_names,
             domain_name: domain_name(),
             allowed_origins,
             downloads_persist_interval_ms: dotenv::var("DOWNLOADS_PERSIST_INTERVAL_MS")

src/controllers/krate/metadata.rs

@@ -24,6 +24,9 @@ use crate::models::krate::ALL_COLUMNS;
 /// Handles the `GET /summary` route.
 pub fn summary(req: &mut dyn RequestExt) -> EndpointResult {
     use crate::schema::crates::dsl::*;
+    use diesel::dsl::all;
+
+    let config = &req.app().config;
 
     let conn = req.db_read_only()?;
     let num_crates: i64 = crates.count().get_result(&*conn)?;
@@ -70,15 +73,26 @@ pub fn summary(req: &mut dyn RequestExt) -> EndpointResult {
         .select(selection)
         .limit(10)
         .load(&*conn)?;
-    let most_downloaded = crates
-        .left_join(recent_crate_downloads::table)
+
+    let mut most_downloaded_query = crates.left_join(recent_crate_downloads::table).into_boxed();
+    if !config.excluded_crate_names.is_empty() {
+        most_downloaded_query =
+            most_downloaded_query.filter(name.ne(all(&config.excluded_crate_names)));
+    }
+    let most_downloaded = most_downloaded_query
         .then_order_by(downloads.desc())
         .select(selection)
         .limit(10)
         .load(&*conn)?;
 
-    let most_recently_downloaded = crates
+    let mut most_recently_downloaded_query = crates
         .inner_join(recent_crate_downloads::table)
+        .into_boxed();
+    if !config.excluded_crate_names.is_empty() {
+        most_recently_downloaded_query =
+            most_recently_downloaded_query.filter(name.ne(all(&config.excluded_crate_names)));
+    }
+    let most_recently_downloaded = most_recently_downloaded_query
         .then_order_by(recent_crate_downloads::downloads.desc())
         .select(selection)
         .limit(10)

src/tests/krate/summary.rs

@@ -107,3 +107,46 @@ fn summary_new_crates() {
 
     assert_eq!(json.new_crates.len(), 5);
 }
+
+#[test]
+fn excluded_crate_id() {
+    let (app, anon, user) = TestApp::init()
+        .with_config(|config| {
+            config.excluded_crate_names = vec![
+                "most_recent_downloads".into(),
+                // make sure no error occurs with a crate name that doesn't exist and that the name
+                // matches are exact, not substrings
+                "downloads".into(),
+            ];
+        })
+        .with_user();
+    let user = user.as_model();
+    app.db(|conn| {
+        CrateBuilder::new("some_downloads", user.id)
+            .version(VersionBuilder::new("0.1.0"))
+            .description("description")
+            .keyword("popular")
+            .category("cat1")
+            .downloads(20)
+            .recent_downloads(10)
+            .expect_build(conn);
+
+        CrateBuilder::new("most_recent_downloads", user.id)
+            .version(VersionBuilder::new("0.2.0"))
+            .keyword("popular")
+            .category("cat1")
+            .downloads(5000)
+            .recent_downloads(50)
+            .expect_build(conn);
+    });
+
+    let json: SummaryResponse = anon.get("/api/v1/summary").good();
+
+    assert_eq!(json.most_downloaded.len(), 1);
+    assert_eq!(json.most_downloaded[0].name, "some_downloads");
+    assert_eq!(json.most_downloaded[0].downloads, 20);
+
+    assert_eq!(json.most_recently_downloaded.len(), 1);
+    assert_eq!(json.most_recently_downloaded[0].name, "some_downloads");
+    assert_eq!(json.most_recently_downloaded[0].recent_downloads, Some(10));
+}

src/tests/util/test_app.rs

@@ -300,6 +300,7 @@ fn simple_config() -> config::Server {
         blocked_traffic: Default::default(),
         max_allowed_page_offset: 200,
         page_offset_ua_blocklist: vec![],
+        excluded_crate_names: vec![],
         domain_name: "crates.io".into(),
         allowed_origins: Vec::new(),
         downloads_persist_interval_ms: 1000,