From 8b48460457b9fb9088068d2c47981e730d144091 Mon Sep 17 00:00:00 2001 From: Tobias Bieniek Date: Wed, 23 Oct 2024 15:23:58 +0200 Subject: [PATCH 1/5] admin/delete_crate: Extract `CrateInfo` struct --- src/admin/delete_crate.rs | 37 +++++++++++++++++++++++++++++-------- 1 file changed, 29 insertions(+), 8 deletions(-) diff --git a/src/admin/delete_crate.rs b/src/admin/delete_crate.rs index f86773410bc..59c357b34bf 100644 --- a/src/admin/delete_crate.rs +++ b/src/admin/delete_crate.rs @@ -8,6 +8,7 @@ use diesel::sql_types::Text; use diesel::{ExpressionMethods, JoinOnDsl, QueryDsl}; use diesel_async::RunQueryDsl; use std::collections::HashMap; +use std::fmt::Display; #[derive(clap::Parser, Debug)] #[command( @@ -49,23 +50,20 @@ pub async fn run(opts: Opts) -> anyhow::Result<()> { .await .context("Failed to look up crate name from the database")?; - let mut existing_crates: HashMap)> = HashMap::new(); + let mut existing_crates: HashMap = HashMap::new(); for (name, id, login) in query_result { let entry = existing_crates .entry(name) - .or_insert_with(|| (id, Vec::new())); + .or_insert_with(|| CrateInfo::new(id)); - entry.1.push(login); + entry.owners.push(login); } println!("Deleting the following crates:"); println!(); for name in &crate_names { match existing_crates.get(name) { - Some((id, owners)) => { - let owners = owners.join(", "); - println!(" - {name} (id={id}, owners={owners})"); - } + Some(info) => println!(" - {name} ({info})"), None => println!(" - {name} (⚠️ crate not found)"), } } @@ -78,7 +76,9 @@ pub async fn run(opts: Opts) -> anyhow::Result<()> { } for name in &crate_names { - if let Some((id, _)) = existing_crates.get(name) { + if let Some(crate_info) = existing_crates.get(name) { + let id = crate_info.id; + info!("{name}: Deleting crate from the database…"); if let Err(error) = diesel::delete(crates::table.find(id)) .execute(&mut conn) @@ -110,3 +110,24 @@ pub async fn run(opts: Opts) -> anyhow::Result<()> { Ok(()) } + +#[derive(Debug, Clone)] +struct CrateInfo { + id: i32, + owners: Vec, +} + +impl CrateInfo { + pub fn new(id: i32) -> Self { + let owners = Vec::with_capacity(1); + Self { id, owners } + } +} + +impl Display for CrateInfo { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + let id = self.id; + let owners = self.owners.join(", "); + write!(f, "id={id}, owners={owners}") + } +} From dae8dbb6d0405311ae052f1ed1a89af134aadeff Mon Sep 17 00:00:00 2001 From: Tobias Bieniek Date: Wed, 23 Oct 2024 15:34:26 +0200 Subject: [PATCH 2/5] admin/delete_crate: Print crate name with bold font --- Cargo.lock | 11 +++++++++++ Cargo.toml | 1 + src/admin/delete_crate.rs | 3 ++- 3 files changed, 14 insertions(+), 1 deletion(-) diff --git a/Cargo.lock b/Cargo.lock index d8a78128798..a70fea61dc4 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -924,6 +924,16 @@ version = "1.0.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "d3fd119d74b830634cea2a0f58bbd0d54540518a14397557951e79340abc28c0" +[[package]] +name = "colored" +version = "2.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "cbf2150cce219b664a8a70df7a1f933836724b503f8a413af9365b4dcc4d90b8" +dependencies = [ + "lazy_static", + "windows-sys 0.48.0", +] + [[package]] name = "comrak" version = "0.29.0" @@ -1023,6 +1033,7 @@ dependencies = [ "chrono", "claims", "clap", + "colored", "cookie", "crates_io_cdn_logs", "crates_io_database", diff --git a/Cargo.toml b/Cargo.toml index 2441b04e2e4..487534745a8 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -49,6 +49,7 @@ base64 = "=0.22.1" bigdecimal = { version = "=0.4.5", features = ["serde"] } bon = "=2.3.0" cargo-manifest = "=0.15.2" +colored = "=2.1.0" crates_io_cdn_logs = { path = "crates/crates_io_cdn_logs" } crates_io_database = { path = "crates/crates_io_database" } crates_io_database_dump = { path = "crates/crates_io_database_dump" } diff --git a/src/admin/delete_crate.rs b/src/admin/delete_crate.rs index 59c357b34bf..1dcabc765f7 100644 --- a/src/admin/delete_crate.rs +++ b/src/admin/delete_crate.rs @@ -2,6 +2,7 @@ use crate::schema::{crate_owners, teams, users}; use crate::worker::jobs; use crate::{admin::dialoguer, db, schema::crates}; use anyhow::Context; +use colored::Colorize; use crates_io_worker::BackgroundJob; use diesel::dsl::sql; use diesel::sql_types::Text; @@ -63,7 +64,7 @@ pub async fn run(opts: Opts) -> anyhow::Result<()> { println!(); for name in &crate_names { match existing_crates.get(name) { - Some(info) => println!(" - {name} ({info})"), + Some(info) => println!(" - {} ({info})", name.bold()), None => println!(" - {name} (⚠️ crate not found)"), } } From a6e1a4546335328c4661f7325a8654c9b65d8acf Mon Sep 17 00:00:00 2001 From: Tobias Bieniek Date: Wed, 23 Oct 2024 15:55:10 +0200 Subject: [PATCH 3/5] admin/delete_crate: Print warning for crates with 5000+ downloads --- src/admin/delete_crate.rs | 37 +++++++++++++++++++++++++------------ 1 file changed, 25 insertions(+), 12 deletions(-) diff --git a/src/admin/delete_crate.rs b/src/admin/delete_crate.rs index 1dcabc765f7..6ab0159c639 100644 --- a/src/admin/delete_crate.rs +++ b/src/admin/delete_crate.rs @@ -1,4 +1,4 @@ -use crate::schema::{crate_owners, teams, users}; +use crate::schema::{crate_downloads, crate_owners, teams, users}; use crate::worker::jobs; use crate::{admin::dialoguer, db, schema::crates}; use anyhow::Context; @@ -36,26 +36,28 @@ pub async fn run(opts: Opts) -> anyhow::Result<()> { crate_names.sort(); let query_result = crates::table + .inner_join(crate_downloads::table) + .left_join(crate_owners::table.on(crate_owners::crate_id.eq(crates::id))) + .left_join(teams::table.on(teams::id.eq(crate_owners::owner_id))) + .left_join(users::table.on(users::id.eq(crate_owners::owner_id))) + .filter(crates::name.eq_any(&crate_names)) .select(( crates::name, crates::id, + crate_downloads::downloads, sql::( "CASE WHEN crate_owners.owner_kind = 1 THEN teams.login ELSE users.gh_login END", ), )) - .left_join(crate_owners::table.on(crate_owners::crate_id.eq(crates::id))) - .left_join(teams::table.on(teams::id.eq(crate_owners::owner_id))) - .left_join(users::table.on(users::id.eq(crate_owners::owner_id))) - .filter(crates::name.eq_any(&crate_names)) - .load::<(String, i32, String)>(&mut conn) + .load::<(String, i32, i64, String)>(&mut conn) .await .context("Failed to look up crate name from the database")?; let mut existing_crates: HashMap = HashMap::new(); - for (name, id, login) in query_result { + for (name, id, downloads, login) in query_result { let entry = existing_crates .entry(name) - .or_insert_with(|| CrateInfo::new(id)); + .or_insert_with(|| CrateInfo::new(id, downloads)); entry.owners.push(login); } @@ -115,13 +117,17 @@ pub async fn run(opts: Opts) -> anyhow::Result<()> { #[derive(Debug, Clone)] struct CrateInfo { id: i32, + downloads: i64, owners: Vec, } impl CrateInfo { - pub fn new(id: i32) -> Self { - let owners = Vec::with_capacity(1); - Self { id, owners } + pub fn new(id: i32, downloads: i64) -> Self { + Self { + id, + downloads, + owners: Vec::with_capacity(1), + } } } @@ -129,6 +135,13 @@ impl Display for CrateInfo { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { let id = self.id; let owners = self.owners.join(", "); - write!(f, "id={id}, owners={owners}") + + write!(f, "id={id}, owners={owners}")?; + if self.downloads > 5000 { + let downloads = format!("downloads={}", self.downloads).bright_red().bold(); + write!(f, ", {downloads}")?; + } + + Ok(()) } } From 1fc6226d49a7e011fadb32897d01b039daec2fb9 Mon Sep 17 00:00:00 2001 From: Tobias Bieniek Date: Wed, 23 Oct 2024 16:16:32 +0200 Subject: [PATCH 4/5] admin/delete_crate: Aggregate owner names within the query --- src/admin/delete_crate.rs | 51 ++++++++++++++++++++++----------------- 1 file changed, 29 insertions(+), 22 deletions(-) diff --git a/src/admin/delete_crate.rs b/src/admin/delete_crate.rs index 6ab0159c639..f728b9c4df8 100644 --- a/src/admin/delete_crate.rs +++ b/src/admin/delete_crate.rs @@ -1,13 +1,14 @@ -use crate::schema::{crate_downloads, crate_owners, teams, users}; +use crate::schema::crate_downloads; use crate::worker::jobs; use crate::{admin::dialoguer, db, schema::crates}; use anyhow::Context; use colored::Colorize; use crates_io_worker::BackgroundJob; use diesel::dsl::sql; -use diesel::sql_types::Text; -use diesel::{ExpressionMethods, JoinOnDsl, QueryDsl}; +use diesel::sql_types::{Array, Text}; +use diesel::{ExpressionMethods, QueryDsl}; use diesel_async::RunQueryDsl; +use futures_util::TryStreamExt; use std::collections::HashMap; use std::fmt::Display; @@ -35,32 +36,38 @@ pub async fn run(opts: Opts) -> anyhow::Result<()> { let mut crate_names = opts.crate_names; crate_names.sort(); - let query_result = crates::table + let existing_crates = crates::table .inner_join(crate_downloads::table) - .left_join(crate_owners::table.on(crate_owners::crate_id.eq(crates::id))) - .left_join(teams::table.on(teams::id.eq(crate_owners::owner_id))) - .left_join(users::table.on(users::id.eq(crate_owners::owner_id))) .filter(crates::name.eq_any(&crate_names)) .select(( crates::name, crates::id, crate_downloads::downloads, - sql::( - "CASE WHEN crate_owners.owner_kind = 1 THEN teams.login ELSE users.gh_login END", + sql::>( + r#" + ARRAY( + SELECT + CASE WHEN crate_owners.owner_kind = 1 THEN + teams.login + ELSE + users.gh_login + END + FROM crate_owners + LEFT JOIN teams ON teams.id = crate_owners.owner_id + LEFT JOIN users ON users.id = crate_owners.owner_id + WHERE crate_owners.crate_id = crates.id + ) + "#, ), )) - .load::<(String, i32, i64, String)>(&mut conn) + .load_stream::<(String, i32, i64, Vec)>(&mut conn) .await - .context("Failed to look up crate name from the database")?; - - let mut existing_crates: HashMap = HashMap::new(); - for (name, id, downloads, login) in query_result { - let entry = existing_crates - .entry(name) - .or_insert_with(|| CrateInfo::new(id, downloads)); - - entry.owners.push(login); - } + .context("Failed to look up crate name from the database")? + .try_fold(HashMap::new(), |mut map, (name, id, downloads, owners)| { + map.insert(name, CrateInfo::new(id, downloads, owners)); + futures_util::future::ready(Ok(map)) + }) + .await?; println!("Deleting the following crates:"); println!(); @@ -122,11 +129,11 @@ struct CrateInfo { } impl CrateInfo { - pub fn new(id: i32, downloads: i64) -> Self { + pub fn new(id: i32, downloads: i64, owners: Vec) -> Self { Self { id, downloads, - owners: Vec::with_capacity(1), + owners, } } } From db4682b5b8410371ca63183fe252069e7d2338fa Mon Sep 17 00:00:00 2001 From: Tobias Bieniek Date: Wed, 23 Oct 2024 16:27:38 +0200 Subject: [PATCH 5/5] admin/delete_crate: Print warning for crates with reverse dependencies --- src/admin/delete_crate.rs | 36 +++++++++++++++++++++++++++++------- 1 file changed, 29 insertions(+), 7 deletions(-) diff --git a/src/admin/delete_crate.rs b/src/admin/delete_crate.rs index f728b9c4df8..47a31ac7f18 100644 --- a/src/admin/delete_crate.rs +++ b/src/admin/delete_crate.rs @@ -5,7 +5,7 @@ use anyhow::Context; use colored::Colorize; use crates_io_worker::BackgroundJob; use diesel::dsl::sql; -use diesel::sql_types::{Array, Text}; +use diesel::sql_types::{Array, BigInt, Text}; use diesel::{ExpressionMethods, QueryDsl}; use diesel_async::RunQueryDsl; use futures_util::TryStreamExt; @@ -59,14 +59,30 @@ pub async fn run(opts: Opts) -> anyhow::Result<()> { ) "#, ), + sql::( + // This is an incorrect reverse dependencies query, since it + // includes the `dependencies` rows for all versions, not just + // the "default version" per crate. However, it's good enough + // for our purposes here. + r#" + ( + SELECT COUNT(*) + FROM dependencies + WHERE dependencies.crate_id = crates.id + ) + "#, + ), )) - .load_stream::<(String, i32, i64, Vec)>(&mut conn) + .load_stream::<(String, i32, i64, Vec, i64)>(&mut conn) .await .context("Failed to look up crate name from the database")? - .try_fold(HashMap::new(), |mut map, (name, id, downloads, owners)| { - map.insert(name, CrateInfo::new(id, downloads, owners)); - futures_util::future::ready(Ok(map)) - }) + .try_fold( + HashMap::new(), + |mut map, (name, id, downloads, owners, rev_deps)| { + map.insert(name, CrateInfo::new(id, downloads, owners, rev_deps)); + futures_util::future::ready(Ok(map)) + }, + ) .await?; println!("Deleting the following crates:"); @@ -126,14 +142,16 @@ struct CrateInfo { id: i32, downloads: i64, owners: Vec, + rev_deps: i64, } impl CrateInfo { - pub fn new(id: i32, downloads: i64, owners: Vec) -> Self { + pub fn new(id: i32, downloads: i64, owners: Vec, rev_deps: i64) -> Self { Self { id, downloads, owners, + rev_deps, } } } @@ -148,6 +166,10 @@ impl Display for CrateInfo { let downloads = format!("downloads={}", self.downloads).bright_red().bold(); write!(f, ", {downloads}")?; } + if self.rev_deps > 0 { + let rev_deps = format!("rev_deps={}", self.rev_deps).bright_red().bold(); + write!(f, ", {rev_deps}")?; + } Ok(()) }