Skip to content

Add an option to delete versions of a crate #893

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

Merged
merged 4 commits into from
Jul 16, 2020
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
40 changes: 31 additions & 9 deletions src/bin/cratesfyi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -432,11 +432,10 @@ enum DatabaseSubcommand {
/// Updates monthly release activity chart
UpdateReleaseActivity,

/// Removes a whole crate from the database
DeleteCrate {
/// Name of the crate to delete
#[structopt(name = "CRATE_NAME")]
crate_name: String,
/// Remove documentation from the database
Delete {
#[structopt(subcommand)]
command: DeleteSubcommand,
},

/// Blacklist operations
Expand Down Expand Up @@ -467,10 +466,13 @@ impl DatabaseSubcommand {
Self::UpdateReleaseActivity => cratesfyi::utils::update_release_activity(&*ctx.conn()?)
.expect("Failed to update release activity"),

Self::DeleteCrate { crate_name } => {
db::delete_crate(&*ctx.conn()?, &crate_name).expect("failed to delete the crate");
}

Self::Delete {
command: DeleteSubcommand::Version { name, version },
} => db::delete_version(&*ctx.conn()?, &name, &version)
.expect("failed to delete the crate"),
Self::Delete {
command: DeleteSubcommand::Crate { name },
} => db::delete_crate(&*ctx.conn()?, &name).expect("failed to delete the crate"),
Self::Blacklist { command } => command.handle_args(ctx)?,
}
Ok(())
Expand Down Expand Up @@ -518,6 +520,26 @@ impl BlacklistSubcommand {
}
}

#[derive(Debug, Clone, PartialEq, Eq, StructOpt)]
enum DeleteSubcommand {
/// Delete a whole crate
Crate {
/// Name of the crate to delete
#[structopt(name = "CRATE_NAME")]
name: String,
},
/// Delete a single version of a crate (which may include multiple builds)
Version {
/// Name of the crate to delete
#[structopt(name = "CRATE_NAME")]
name: String,

/// The version of the crate to delete
#[structopt(name = "VERSION")]
version: String,
},
}

struct Context {
build_queue: OnceCell<Arc<BuildQueue>>,
config: OnceCell<Arc<Config>>,
Expand Down
169 changes: 140 additions & 29 deletions src/db/delete_crate.rs → src/db/delete.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,22 +14,78 @@ enum CrateDeletionError {
}

pub fn delete_crate(conn: &Connection, name: &str) -> Result<(), Error> {
let crate_id_res = conn.query("SELECT id FROM crates WHERE name = $1", &[&name])?;
let crate_id = if crate_id_res.is_empty() {
return Err(CrateDeletionError::MissingCrate(name.into()).into());
} else {
crate_id_res.get(0).get("id")
};
let crate_id = get_id(conn, name)?;
delete_crate_from_database(conn, name, crate_id)?;
if let Some(s3) = s3_client() {
for prefix in STORAGE_PATHS_TO_DELETE {
delete_prefix_from_s3(&s3, &format!("{}/{}/", prefix, name))?;
}
}
Ok(())
}

pub fn delete_version(conn: &Connection, name: &str, version: &str) -> Result<(), Error> {
delete_version_from_database(conn, name, version)?;

delete_from_database(conn, name, crate_id)?;
if let Some(s3) = s3_client() {
delete_from_s3(&s3, name)?;
for prefix in STORAGE_PATHS_TO_DELETE {
delete_prefix_from_s3(&s3, &format!("{}/{}/{}/", prefix, name, version))?;
}
}

Ok(())
}

fn delete_from_database(conn: &Connection, name: &str, crate_id: i32) -> Result<(), Error> {
fn get_id(conn: &Connection, name: &str) -> Result<i32, Error> {
let crate_id_res = conn.query("SELECT id FROM crates WHERE name = $1", &[&name])?;
if let Some(row) = crate_id_res.into_iter().next() {
Ok(row.get("id"))
} else {
Err(CrateDeletionError::MissingCrate(name.into()).into())
}
}

fn delete_version_from_database(conn: &Connection, name: &str, version: &str) -> Result<(), Error> {
let crate_id = get_id(conn, name)?;
let transaction = conn.transaction()?;
// metaprogramming!
// WARNING: these must be hard-coded and NEVER user input.
let metadata: [(&'static str, &'static str); 4] = [
("author_rels", "rid"),
("owner_rels", "cid"),
("keyword_rels", "rid"),
("builds", "rid"),
];
for &(table, column) in &metadata {
transaction.execute(
&format!("DELETE FROM {} WHERE {} IN (SELECT id FROM releases WHERE crate_id = $1 AND version = $2)", table, column),
&[&crate_id, &version],
)?;
}
transaction.execute(
"DELETE FROM releases WHERE crate_id = $1 AND version = $2",
&[&crate_id, &version],
)?;
transaction.execute(
"UPDATE crates SET latest_version_id = (
SELECT id FROM releases WHERE release_time = (
SELECT MAX(release_time) FROM releases WHERE crate_id = $1
)
) WHERE id = $1",
&[&crate_id],
)?;

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

transaction.commit().map_err(Into::into)
}

fn delete_crate_from_database(conn: &Connection, name: &str, crate_id: i32) -> Result<(), Error> {
let transaction = conn.transaction()?;

transaction.execute(
Expand Down Expand Up @@ -68,13 +124,6 @@ fn delete_from_database(conn: &Connection, name: &str, crate_id: i32) -> Result<
Ok(())
}

fn delete_from_s3(s3: &S3Client, name: &str) -> Result<(), Error> {
for prefix in STORAGE_PATHS_TO_DELETE {
delete_prefix_from_s3(s3, &format!("{}/{}/", prefix, name))?;
}
Ok(())
}

fn delete_prefix_from_s3(s3: &S3Client, name: &str) -> Result<(), Error> {
let mut continuation_token = None;
loop {
Expand Down Expand Up @@ -124,23 +173,25 @@ fn delete_prefix_from_s3(s3: &S3Client, name: &str) -> Result<(), Error> {
#[cfg(test)]
mod tests {
use super::*;
use crate::test::{assert_success, wrapper};
use failure::Error;
use postgres::Connection;

fn crate_exists(conn: &Connection, name: &str) -> Result<bool, Error> {
Ok(!conn
.query("SELECT * FROM crates WHERE name = $1;", &[&name])?
.is_empty())
}

fn release_exists(conn: &Connection, id: i32) -> Result<bool, Error> {
Ok(!conn
.query("SELECT * FROM releases WHERE id = $1;", &[&id])?
.is_empty())
}

#[test]
fn test_delete_from_database() {
fn crate_exists(conn: &Connection, name: &str) -> Result<bool, Error> {
Ok(!conn
.query("SELECT * FROM crates WHERE name = $1;", &[&name])?
.is_empty())
}
fn release_exists(conn: &Connection, id: i32) -> Result<bool, Error> {
Ok(!conn
.query("SELECT * FROM releases WHERE id = $1;", &[&id])?
.is_empty())
}

crate::test::wrapper(|env| {
wrapper(|env| {
let db = env.db();

// Create fake packages in the database
Expand Down Expand Up @@ -168,7 +219,7 @@ mod tests {
.get(0)
.get("id");

delete_from_database(&db.conn(), "package-1", *pkg1_id)?;
delete_crate_from_database(&db.conn(), "package-1", *pkg1_id)?;

assert!(!crate_exists(&db.conn(), "package-1")?);
assert!(crate_exists(&db.conn(), "package-2")?);
Expand All @@ -179,4 +230,64 @@ mod tests {
Ok(())
});
}

#[test]
fn test_delete_version() {
wrapper(|env| {
fn authors(conn: &Connection, crate_id: i32) -> Result<Vec<String>, Error> {
Ok(conn
.query(
"SELECT name FROM authors
INNER JOIN author_rels ON authors.id = author_rels.aid
INNER JOIN releases ON author_rels.rid = releases.id
WHERE releases.crate_id = $1",
&[&crate_id],
)?
.into_iter()
.map(|row| row.get(0))
.collect())
}

let db = env.db();
let v1 = db
.fake_release()
.name("a")
.version("1.0.0")
.author("malicious actor")
.create()?;
let v2 = db
.fake_release()
.name("a")
.version("2.0.0")
.author("Peter Rabbit")
.create()?;
assert!(release_exists(&db.conn(), v1)?);
assert!(release_exists(&db.conn(), v2)?);
let crate_id = db
.conn()
.query("SELECT crate_id FROM releases WHERE id = $1", &[&v1])?
.into_iter()
.next()
.unwrap()
.get(0);
assert_eq!(
authors(&db.conn(), crate_id)?,
vec!["malicious actor".to_string(), "Peter Rabbit".to_string()]
);

delete_version(&db.conn(), "a", "1.0.0")?;
assert!(!release_exists(&db.conn(), v1)?);
assert!(release_exists(&db.conn(), v2)?);
assert_eq!(
authors(&db.conn(), crate_id)?,
vec!["Peter Rabbit".to_string()]
);

let web = env.frontend();
assert_success("/a/2.0.0/a/", web)?;
assert_eq!(web.get("/a/1.0.0/a/").send()?.status(), 404);

Ok(())
})
}
}
4 changes: 2 additions & 2 deletions src/db/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

pub(crate) use self::add_package::add_build_into_database;
pub(crate) use self::add_package::add_package_into_database;
pub use self::delete_crate::delete_crate;
pub use self::delete::{delete_crate, delete_version};
pub use self::file::add_path_into_database;
pub use self::migrate::migrate;
pub use self::pool::{Pool, PoolError};
Expand All @@ -12,7 +12,7 @@ pub(crate) use self::pool::PoolConnection;

mod add_package;
pub mod blacklist;
mod delete_crate;
mod delete;
pub(crate) mod file;
mod migrate;
mod pool;