From 822811a99df6d3ced063b18665a61475ddfd4322 Mon Sep 17 00:00:00 2001 From: Denis Cornehl Date: Tue, 4 Jul 2023 07:10:09 +0200 Subject: [PATCH] don't delete full crates from docs.rs when only a version was deleted from the index --- src/bin/cratesfyi.rs | 11 +++++-- src/build_queue.rs | 56 +++++++++++++++++++++++++++++------- src/db/delete.rs | 17 ++++++----- src/utils/consistency/mod.rs | 4 ++- 4 files changed, 67 insertions(+), 21 deletions(-) diff --git a/src/bin/cratesfyi.rs b/src/bin/cratesfyi.rs index 94f75441b..8bead0151 100644 --- a/src/bin/cratesfyi.rs +++ b/src/bin/cratesfyi.rs @@ -554,9 +554,14 @@ impl DatabaseSubcommand { Self::Delete { command: DeleteSubcommand::Version { name, version }, - } => { - db::delete_version(&ctx, &name, &version).context("failed to delete the version")? - } + } => db::delete_version( + &mut *ctx.pool()?.get()?, + &*ctx.storage()?, + &*ctx.config()?, + &name, + &version, + ) + .context("failed to delete the version")?, Self::Delete { command: DeleteSubcommand::Crate { name }, } => db::delete_crate( diff --git a/src/build_queue.rs b/src/build_queue.rs index 82bc95c1a..a4279f8dd 100644 --- a/src/build_queue.rs +++ b/src/build_queue.rs @@ -1,5 +1,5 @@ use crate::cdn; -use crate::db::{delete_crate, Pool}; +use crate::db::{delete_crate, delete_version, Pool}; use crate::docbuilder::PackageKind; use crate::error::Result; use crate::storage::Storage; @@ -283,16 +283,52 @@ impl BuildQueue { changes.reverse(); for change in &changes { - if let Some((ref krate, ..)) = change.deleted() { - match delete_crate(&mut conn, &self.storage, &self.config, krate) - .with_context(|| format!("failed to delete crate {krate}")) - { - Ok(_) => info!( - "crate {} was deleted from the index and the database", - krate - ), - Err(err) => report_error(&err), + if let Some((ref krate, versions)) = change.deleted() { + for version in versions { + let version = version.version.as_str(); + match delete_version(&mut conn, &self.storage, &self.config, krate, version) + .with_context(|| format!("failed to delete version {krate} {version}")) + { + Ok(_) => info!( + "version {} {} was deleted from the index and the database", + krate, version, + ), + Err(err) => report_error(&err), + } } + + // FIXME: Change::Deleted (and `.deleted()`) is documented to be only crate-deletes, + // but can also be one or multiple version-deletes. + // Until crates-index-diff can separately emit crate- or version-deletes, + // we try to figure this out here. + let versions_remaining: i32 = conn + .query_one( + " + SELECT + COUNT(*) + FROM + crates + INNER JOIN releases ON crates.id = releases.crate_id + WHERE + crates.name = $1 + ", + &[&krate], + ) + .with_context(|| format!("could not fetch version count for crate {krate}"))? + .get(0); + + if versions_remaining == 0 { + match delete_crate(&mut conn, &self.storage, &self.config, krate) + .with_context(|| format!("failed to delete crate {krate}")) + { + Ok(_) => info!( + "crate {} was deleted from the index and the database", + krate + ), + Err(err) => report_error(&err), + } + } + if let Err(err) = cdn::queue_crate_invalidation(&mut *conn, &self.config, krate) { report_error(&err); } diff --git a/src/db/delete.rs b/src/db/delete.rs index 840459dcd..017f6531d 100644 --- a/src/db/delete.rs +++ b/src/db/delete.rs @@ -1,6 +1,6 @@ use crate::error::Result; use crate::storage::{rustdoc_archive_path, source_archive_path, Storage}; -use crate::{Config, Context}; +use crate::Config; use anyhow::Context as _; use fn_error_context::context; use postgres::Client; @@ -55,10 +55,13 @@ pub fn delete_crate( } #[context("error trying to delete release {name}-{version} from database")] -pub fn delete_version(ctx: &dyn Context, name: &str, version: &str) -> Result<()> { - let conn = &mut ctx.pool()?.get()?; - let storage = ctx.storage()?; - +pub fn delete_version( + conn: &mut Client, + storage: &Storage, + config: &Config, + name: &str, + version: &str, +) -> Result<()> { let is_library = delete_version_from_database(conn, name, version)?; let paths = if is_library { LIBRARY_STORAGE_PATHS_TO_DELETE @@ -70,7 +73,7 @@ pub fn delete_version(ctx: &dyn Context, name: &str, version: &str) -> Result<() storage.delete_prefix(&format!("{prefix}/{name}/{version}/"))?; } - let local_archive_cache = &ctx.config()?.local_archive_cache_path; + let local_archive_cache = &config.local_archive_cache_path; let mut paths = vec![source_archive_path(name, version)]; if is_library { paths.push(rustdoc_archive_path(name, version)); @@ -359,7 +362,7 @@ mod tests { vec!["Peter Rabbit".to_string()] ); - delete_version(env, "a", "1.0.0")?; + delete_version(&mut db.conn(), &env.storage(), &env.config(), "a", "1.0.0")?; assert!(!release_exists(&mut db.conn(), v1)?); if archive_storage { // for archive storage the archive and index files diff --git a/src/utils/consistency/mod.rs b/src/utils/consistency/mod.rs index 7dc5fa7b7..811562bb4 100644 --- a/src/utils/consistency/mod.rs +++ b/src/utils/consistency/mod.rs @@ -111,7 +111,9 @@ where } diff::Difference::ReleaseNotInIndex(name, version) => { if !dry_run { - if let Err(err) = delete::delete_version(ctx, name, version) { + if let Err(err) = + delete::delete_version(&mut conn, &storage, &config, name, version) + { warn!("{:?}", err); } }