Skip to content

Commit

Permalink
Don't try to delete rustdoc files for binary releases
Browse files Browse the repository at this point in the history
This avoids confusing errors like the following:
```
Error: failed to delete the crate

Caused by:
    <?xml version="1.0" encoding="UTF-8"?>
<Error><Code>MalformedXML</Code><Message>The XML you provided was not well-formed or did not validate against our published schema</Message><RequestId>9F280BB6EBFEC7F3</RequestId><HostId>sgrbwS6rXn83txaNR4V3+YyVm/+VhIrD1Nhaiv9og8H5hNnGq2yntXaGt45DQ8rcmopLRGoPqkg=</HostId></Error>
```

Tested locally like so:
```
$ cargo run build crate regex 1.4.1
$ cargo run build crate bat 0.12.1
$ psql postgresql://cratesfyi:password@localhost:15432
cratesfyi=# select path from files where path like '%bat%';
             path
------------------------------
 sources/bat/0.12.1.zip
 sources/bat/0.12.1.zip.index
(2 rows)

cratesfyi=# select path from files where path like '%regex%';
             path
-------------------------------
 rustdoc/regex/1.4.1.zip
 rustdoc/regex/1.4.1.zip.index
 sources/regex/1.4.1.zip
 sources/regex/1.4.1.zip.index
(4 rows)

cratesfyi=# \q
$ cargo run database delete version regex 1.4.1
    Finished dev [unoptimized + debuginfo] target(s) in 0.15s
     Running `target/debug/cratesfyi database delete version regex 1.4.1`
$ psql postgresql://cratesfyi:password@localhost:15432
cratesfyi=# select path from files where path like '%regex%';
 path
------
(0 rows)

cratesfyi=# select path from files where path like '%bat%';
             path
------------------------------
 sources/bat/0.12.1.zip
 sources/bat/0.12.1.zip.index
(2 rows)

cratesfyi=# \q
$ cargo run database delete version bat 0.12.1
    Finished dev [unoptimized + debuginfo] target(s) in 0.14s
     Running `target/debug/cratesfyi database delete version bat 0.12.1`
$ psql postgresql://cratesfyi:password@localhost:15432
cratesfyi=# select path from files where path like '%bat%';
 path
------
(0 rows)

cratesfyi=# \q
```
  • Loading branch information
jyn514 committed Feb 13, 2022
1 parent de30fde commit 46ea6c4
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 19 deletions.
2 changes: 1 addition & 1 deletion src/bin/cratesfyi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -447,7 +447,7 @@ 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
47 changes: 29 additions & 18 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,11 @@ 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
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 +48,22 @@ 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 +98,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 +108,10 @@ 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",
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 +121,20 @@ 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 +152,14 @@ 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

0 comments on commit 46ea6c4

Please sign in to comment.