Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't try to delete rustdoc files for binary releases #1676

Merged
merged 1 commit into from
Feb 16, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion src/bin/cratesfyi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -447,7 +447,9 @@ impl DatabaseSubcommand {

Self::Delete {
command: DeleteSubcommand::Version { name, version },
} => db::delete_version(&ctx, &name, &version).context("failed to delete the crate")?,
} => {
db::delete_version(&ctx, &name, &version).context("failed to delete the version")?
}
Self::Delete {
command: DeleteSubcommand::Crate { name },
} => db::delete_crate(&ctx, &name).context("failed to delete the crate")?,
Expand Down
68 changes: 49 additions & 19 deletions src/db/delete.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,8 @@ use std::fs;

/// List of directories in docs.rs's underlying storage (either the database or S3) containing a
/// subdirectory named after the crate. Those subdirectories will be deleted.
static STORAGE_PATHS_TO_DELETE: &[&str] = &["rustdoc", "sources"];
static LIBRARY_STORAGE_PATHS_TO_DELETE: &[&str] = &["rustdoc", "sources"];
static BINARY_STORAGE_PATHS_TO_DELETE: &[&str] = &["sources"];

#[derive(Debug, thiserror::Error)]
enum CrateDeletionError {
Expand All @@ -18,9 +19,15 @@ enum CrateDeletionError {
pub fn delete_crate(ctx: &dyn Context, name: &str) -> Result<()> {
let conn = &mut ctx.pool()?.get()?;
let crate_id = get_id(conn, name)?;
delete_crate_from_database(conn, name, crate_id)?;
let is_library = delete_crate_from_database(conn, name, crate_id)?;
// #899
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: this comment feels unnecessary here.

let paths = if is_library {
LIBRARY_STORAGE_PATHS_TO_DELETE
} else {
BINARY_STORAGE_PATHS_TO_DELETE
};

for prefix in STORAGE_PATHS_TO_DELETE {
for prefix in paths {
// delete the whole rustdoc/source folder for this crate.
// it will include existing archives.
let remote_folder = format!("{}/{}/", prefix, name);
Expand All @@ -45,19 +52,26 @@ pub fn delete_version(ctx: &dyn Context, name: &str, version: &str) -> Result<()
let conn = &mut ctx.pool()?.get()?;
let storage = ctx.storage()?;

delete_version_from_database(conn, name, version)?;
let is_library = delete_version_from_database(conn, name, version)?;
let paths = if is_library {
LIBRARY_STORAGE_PATHS_TO_DELETE
} else {
BINARY_STORAGE_PATHS_TO_DELETE
};

for prefix in STORAGE_PATHS_TO_DELETE {
for prefix in paths {
storage.delete_prefix(&format!("{}/{}/{}/", prefix, name, version))?;
}

let local_archive_cache = &ctx.config()?.local_archive_cache_path;
for archive_filename in &[
rustdoc_archive_path(name, version),
source_archive_path(name, version),
] {
let mut paths = vec![source_archive_path(name, version)];
if is_library {
paths.push(rustdoc_archive_path(name, version));
}

for archive_filename in paths {
// delete remove archive and remote index
storage.delete_prefix(archive_filename)?;
storage.delete_prefix(&archive_filename)?;

// delete eventually existing local indexes
let local_index_file = local_archive_cache.join(&format!("{}.index", archive_filename));
Expand Down Expand Up @@ -92,7 +106,8 @@ const METADATA: &[(&str, &str)] = &[
("doc_coverage", "release_id"),
];

fn delete_version_from_database(conn: &mut Client, name: &str, version: &str) -> Result<()> {
/// Returns whether this release was a library
fn delete_version_from_database(conn: &mut Client, name: &str, version: &str) -> Result<bool> {
let crate_id = get_id(conn, name)?;
let mut transaction = conn.transaction()?;
for &(table, column) in METADATA {
Expand All @@ -101,10 +116,12 @@ fn delete_version_from_database(conn: &mut Client, name: &str, version: &str) ->
&[&crate_id, &version],
)?;
}
transaction.execute(
"DELETE FROM releases WHERE crate_id = $1 AND version = $2",
&[&crate_id, &version],
)?;
let is_library: bool = transaction
.query_one(
"DELETE FROM releases WHERE crate_id = $1 AND version = $2 RETURNING is_library",
&[&crate_id, &version],
)?
.get("is_library");
transaction.execute(
"UPDATE crates SET latest_version_id = (
SELECT id FROM releases WHERE release_time = (
Expand All @@ -114,17 +131,24 @@ fn delete_version_from_database(conn: &mut Client, name: &str, version: &str) ->
&[&crate_id],
)?;

for prefix in STORAGE_PATHS_TO_DELETE {
let paths = if is_library {
LIBRARY_STORAGE_PATHS_TO_DELETE
} else {
BINARY_STORAGE_PATHS_TO_DELETE
};
for prefix in paths {
transaction.execute(
"DELETE FROM files WHERE path LIKE $1;",
&[&format!("{}/{}/{}/%", prefix, name, version)],
)?;
}

transaction.commit().map_err(Into::into)
transaction.commit()?;
Ok(is_library)
}

fn delete_crate_from_database(conn: &mut Client, name: &str, crate_id: i32) -> Result<()> {
/// Returns whether any release in this crate was a library
fn delete_crate_from_database(conn: &mut Client, name: &str, crate_id: i32) -> Result<bool> {
let mut transaction = conn.transaction()?;

transaction.execute(
Expand All @@ -142,13 +166,19 @@ fn delete_crate_from_database(conn: &mut Client, name: &str, crate_id: i32) -> R
)?;
}
transaction.execute("DELETE FROM owner_rels WHERE cid = $1;", &[&crate_id])?;
let has_library = transaction
.query_one(
"SELECT BOOL_OR(releases.is_library) AS has_library FROM releases",
&[],
)?
.get("has_library");
transaction.execute("DELETE FROM releases WHERE crate_id = $1;", &[&crate_id])?;
transaction.execute("DELETE FROM crates WHERE id = $1;", &[&crate_id])?;

// Transactions automatically rollback when not committing, so if any of the previous queries
// fail the whole transaction will be aborted.
transaction.commit()?;
Ok(())
Ok(has_library)
}

#[cfg(test)]
Expand Down