Add an option to delete a version of a crate

- Test for authors in delete_version
- Enforce that table_name and column_name are hard-coded

Author: jyn514

src/bin/cratesfyi.rs

@@ -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
@@ -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(())
@@ -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>>,

src/db/delete_crate.rs

@@ -1,6 +1,6 @@
 use crate::storage::s3::{s3_client, S3_BUCKET_NAME};
 use failure::{Error, Fail};
-use postgres::Connection;
+use postgres::{transaction::Transaction, Connection};
 use rusoto_s3::{DeleteObjectsRequest, ListObjectsV2Request, ObjectIdentifier, S3Client, S3};
 
 /// List of directories in docs.rs's underlying storage (either the database or S3) containing a
@@ -14,22 +14,89 @@ 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))?;
+        }
+    }
+
+    Ok(())
+}
+
+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())
     }
+}
 
+// metaprogramming!
+// NOTE: it is _crucial_ that table_name and column_name be hard-coded, i.e. NOT user input
+fn delete_metadata(
+    transaction: &Transaction,
+    crate_id: i32,
+    version: &str,
+    table_name: &'static str,
+    column_name: &'static str,
+) -> Result<(), Error> {
+    transaction.execute(
+        &format!("DELETE FROM {} WHERE {} IN (SELECT id FROM releases WHERE crate_id = $1 AND version = $2)", table_name, column_name),
+        &[&crate_id, &version],
+    )?;
     Ok(())
 }
 
-fn delete_from_database(conn: &Connection, name: &str, crate_id: i32) -> Result<(), Error> {
+fn delete_version_from_database(conn: &Connection, name: &str, version: &str) -> Result<(), Error> {
+    let crate_id = get_id(conn, name)?;
+    let transaction = conn.transaction()?;
+    let metadata = [
+        ("author_rels", "rid"),
+        ("owner_rels", "cid"),
+        ("keyword_rels", "rid"),
+        ("builds", "rid"),
+    ];
+    for &(table, column) in &metadata {
+        delete_metadata(&transaction, crate_id, version, table, column)?;
+    }
+    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(
@@ -68,13 +135,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 {
@@ -124,23 +184,25 @@ fn delete_prefix_from_s3(s3: &S3Client, name: &str) -> Result<(), Error> {
 #[cfg(test)]
 mod tests {
     use super::*;
+    use crate::test::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
@@ -168,7 +230,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")?);
@@ -179,4 +241,60 @@ 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()]
+            );
+
+            Ok(())
+        })
+    }
 }

src/db/mod.rs

@@ -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_crate::{delete_crate, delete_version};
 pub use self::file::add_path_into_database;
 pub use self::migrate::migrate;
 pub use self::pool::{Pool, PoolError};