Merge pull request #859 from sgrif/sg-port-delete-crate

sgrif · web-flow · commit ccadd334152d · 2017-07-17T10:28:10.000-04:00
Port `bin/delete-crate` over to Diesel
diff --git a/migrations/20170708133715_ensure_crate_id_foreign_keys_cascade/down.sql b/migrations/20170708133715_ensure_crate_id_foreign_keys_cascade/down.sql
@@ -0,0 +1,21 @@
+ALTER TABLE "crate_downloads"
+    DROP CONSTRAINT "fk_crate_downloads_crate_id",
+    ADD CONSTRAINT "fk_crate_downloads_crate_id" FOREIGN KEY (crate_id) REFERENCES crates(id);
+ALTER TABLE "crate_owners"
+    DROP CONSTRAINT "fk_crate_owners_crate_id",
+    ADD CONSTRAINT "fk_crate_owners_crate_id" FOREIGN KEY (crate_id) REFERENCES crates(id);
+ALTER TABLE "crates_categories"
+    DROP CONSTRAINT "fk_crates_categories_crate_id",
+    ADD CONSTRAINT "fk_crates_categories_crate_id" FOREIGN KEY (crate_id) REFERENCES crates(id);
+ALTER TABLE "crates_keywords"
+    DROP CONSTRAINT "fk_crates_keywords_crate_id",
+    ADD CONSTRAINT "fk_crates_keywords_crate_id" FOREIGN KEY (crate_id) REFERENCES crates(id);
+ALTER TABLE "dependencies"
+    DROP CONSTRAINT "fk_dependencies_crate_id",
+    ADD CONSTRAINT "fk_dependencies_crate_id" FOREIGN KEY (crate_id) REFERENCES crates(id);
+ALTER TABLE "follows"
+    DROP CONSTRAINT "fk_follows_crate_id",
+    ADD CONSTRAINT "fk_follows_crate_id" FOREIGN KEY (crate_id) REFERENCES crates(id);
+ALTER TABLE "versions"
+    DROP CONSTRAINT "fk_versions_crate_id",
+    ADD CONSTRAINT "fk_versions_crate_id" FOREIGN KEY (crate_id) REFERENCES crates(id);
diff --git a/migrations/20170708133715_ensure_crate_id_foreign_keys_cascade/up.sql b/migrations/20170708133715_ensure_crate_id_foreign_keys_cascade/up.sql
@@ -0,0 +1,21 @@
+ALTER TABLE "crate_downloads"
+    DROP CONSTRAINT "fk_crate_downloads_crate_id",
+    ADD CONSTRAINT "fk_crate_downloads_crate_id" FOREIGN KEY (crate_id) REFERENCES crates(id) ON DELETE CASCADE;
+ALTER TABLE "crate_owners"
+    DROP CONSTRAINT "fk_crate_owners_crate_id",
+    ADD CONSTRAINT "fk_crate_owners_crate_id" FOREIGN KEY (crate_id) REFERENCES crates(id) ON DELETE CASCADE;
+ALTER TABLE "crates_categories"
+    DROP CONSTRAINT "fk_crates_categories_crate_id",
+    ADD CONSTRAINT "fk_crates_categories_crate_id" FOREIGN KEY (crate_id) REFERENCES crates(id) ON DELETE CASCADE;
+ALTER TABLE "crates_keywords"
+    DROP CONSTRAINT "fk_crates_keywords_crate_id",
+    ADD CONSTRAINT "fk_crates_keywords_crate_id" FOREIGN KEY (crate_id) REFERENCES crates(id) ON DELETE CASCADE;
+ALTER TABLE "dependencies"
+    DROP CONSTRAINT "fk_dependencies_crate_id",
+    ADD CONSTRAINT "fk_dependencies_crate_id" FOREIGN KEY (crate_id) REFERENCES crates(id) ON DELETE CASCADE;
+ALTER TABLE "follows"
+    DROP CONSTRAINT "fk_follows_crate_id",
+    ADD CONSTRAINT "fk_follows_crate_id" FOREIGN KEY (crate_id) REFERENCES crates(id) ON DELETE CASCADE;
+ALTER TABLE "versions"
+    DROP CONSTRAINT "fk_versions_crate_id",
+    ADD CONSTRAINT "fk_versions_crate_id" FOREIGN KEY (crate_id) REFERENCES crates(id) ON DELETE CASCADE;
diff --git a/migrations/20170708135123_ensure_version_id_foreign_keys_cascade/down.sql b/migrations/20170708135123_ensure_version_id_foreign_keys_cascade/down.sql
@@ -0,0 +1,9 @@
+ALTER TABLE "dependencies"
+    DROP CONSTRAINT "fk_dependencies_version_id",
+    ADD CONSTRAINT "fk_dependencies_version_id" FOREIGN KEY (version_id) REFERENCES versions(id);
+ALTER TABLE "version_authors"
+    DROP CONSTRAINT "fk_version_authors_version_id",
+    ADD CONSTRAINT "fk_version_authors_version_id" FOREIGN KEY (version_id) REFERENCES versions(id);
+ALTER TABLE "version_downloads"
+    DROP CONSTRAINT "fk_version_downloads_version_id",
+    ADD CONSTRAINT "fk_version_downloads_version_id" FOREIGN KEY (version_id) REFERENCES versions(id);
diff --git a/migrations/20170708135123_ensure_version_id_foreign_keys_cascade/up.sql b/migrations/20170708135123_ensure_version_id_foreign_keys_cascade/up.sql
@@ -0,0 +1,9 @@
+ALTER TABLE "dependencies"
+    DROP CONSTRAINT "fk_dependencies_version_id",
+    ADD CONSTRAINT "fk_dependencies_version_id" FOREIGN KEY (version_id) REFERENCES versions(id) ON DELETE CASCADE;
+ALTER TABLE "version_authors"
+    DROP CONSTRAINT "fk_version_authors_version_id",
+    ADD CONSTRAINT "fk_version_authors_version_id" FOREIGN KEY (version_id) REFERENCES versions(id) ON DELETE CASCADE;
+ALTER TABLE "version_downloads"
+    DROP CONSTRAINT "fk_version_downloads_version_id",
+    ADD CONSTRAINT "fk_version_downloads_version_id" FOREIGN KEY (version_id) REFERENCES versions(id) ON DELETE CASCADE;
diff --git a/src/bin/delete-crate.rs b/src/bin/delete-crate.rs
@@ -8,27 +8,28 @@
 #![deny(warnings)]
 
 extern crate cargo_registry;
-extern crate postgres;
+extern crate diesel;
 extern crate time;
 
+use diesel::prelude::*;
+use diesel::pg::PgConnection;
 use std::env;
 use std::io;
 use std::io::prelude::*;
 
 use cargo_registry::Crate;
+use cargo_registry::schema::crates;
 
 #[allow(dead_code)]
 fn main() {
-    let conn = cargo_registry::db::connect_now_old();
-    {
-        let tx = conn.transaction().unwrap();
-        delete(&tx);
-        tx.set_commit();
-        tx.finish().unwrap();
-    }
+    let conn = cargo_registry::db::connect_now().unwrap();
+    conn.transaction::<_, diesel::result::Error, _>(|| {
+        delete(&conn);
+        Ok(())
+    }).unwrap()
 }
 
-fn delete(tx: &postgres::transaction::Transaction) {
+fn delete(conn: &PgConnection) {
     let name = match env::args().nth(1) {
         None => {
             println!("needs a crate-name argument");
@@ -37,7 +38,7 @@ fn delete(tx: &postgres::transaction::Transaction) {
         Some(s) => s,
     };
 
-    let krate = Crate::find_by_name(tx, &name).unwrap();
+    let krate = Crate::by_name(&name).first::<Crate>(conn).unwrap();
     print!(
         "Are you sure you want to delete {} ({}) [y/N]: ",
         name,
@@ -50,77 +51,9 @@ fn delete(tx: &postgres::transaction::Transaction) {
         return;
     }
 
-    let versions = krate.versions(tx).unwrap();
-
-    for v in versions.iter() {
-        println!("deleting version {} ({})", v.num, v.id);
-        let n = tx.execute(
-            "DELETE FROM version_downloads WHERE version_id = $1",
-            &[&v.id],
-        ).unwrap();
-        println!("  {} download records deleted", n);
-        let n = tx.execute(
-            "DELETE FROM version_authors WHERE version_id = $1",
-            &[&v.id],
-        ).unwrap();
-        println!("  {} author records deleted", n);
-        let n = tx.execute("DELETE FROM dependencies WHERE version_id = $1", &[&v.id])
-            .unwrap();
-        println!("  {} dependencies deleted", n);
-        tx.execute("DELETE FROM versions WHERE id = $1", &[&v.id])
-            .unwrap();
-    }
-
-    println!("deleting follows");
-    let n = tx.execute("DELETE FROM follows WHERE crate_id = $1", &[&krate.id])
-        .unwrap();
-    println!("  {} deleted", n);
-
-    println!("deleting crate download records");
-    let n = tx.execute(
-        "DELETE FROM crate_downloads WHERE crate_id = $1",
-        &[&krate.id],
-    ).unwrap();
-    println!("  {} deleted", n);
-
-    println!("deleting crate owners");
-    let n = tx.execute("DELETE FROM crate_owners WHERE crate_id = $1", &[&krate.id])
-        .unwrap();
-    println!("  {} deleted", n);
-
-    println!("disabling reserved crate name trigger");
-    let _ = tx.execute(
-        "ALTER TABLE crates DISABLE TRIGGER trigger_ensure_crate_name_not_reserved;",
-        &[],
-    ).unwrap();
-
-    println!("deleting crate keyword connections");
-    let n = tx.execute(
-        "DELETE FROM crates_keywords WHERE crate_id = $1",
-        &[&krate.id],
-    ).unwrap();
-    println!("  {} deleted", n);
-
-    println!("deleting crate category connections");
-    let n = tx.execute(
-        "DELETE FROM crates_categories WHERE crate_id = $1",
-        &[&krate.id],
-    ).unwrap();
-    println!("  {} deleted", n);
-
-    println!("enabling reserved crate name trigger");
-    let _ = tx.execute(
-        "ALTER TABLE crates ENABLE TRIGGER trigger_ensure_crate_name_not_reserved;",
-        &[],
-    ).unwrap();
-
-    println!("deleting crate badges");
-    let n = tx.execute("DELETE FROM badges WHERE crate_id = $1", &[&krate.id])
-        .unwrap();
-    println!("  {} deleted", n);
-
     println!("deleting the crate");
-    let n = tx.execute("DELETE FROM crates WHERE id = $1", &[&krate.id])
+    let n = diesel::delete(crates::table.find(krate.id))
+        .execute(conn)
         .unwrap();
     println!("  {} deleted", n);
 
diff --git a/src/tests/all.rs b/src/tests/all.rs
@@ -3,6 +3,8 @@
 #[macro_use]
 extern crate serde_derive;
 extern crate diesel;
+#[macro_use]
+extern crate diesel_codegen;
 extern crate bufstream;
 extern crate cargo_registry;
 extern crate conduit;
@@ -89,6 +91,7 @@ mod git;
 mod keyword;
 mod krate;
 mod record;
+mod schema_details;
 mod team;
 mod token;
 mod user;
diff --git a/src/tests/load_foreign_key_constraints.sql b/src/tests/load_foreign_key_constraints.sql
@@ -0,0 +1,9 @@
+-- This can't quite use Diesel's query builder yet, because diesel doesn't
+-- support custom ON clauses yet. We need the attnum comparison in the join
+-- to make sure that we get NULL if no constraint is present
+SELECT relname, conname, pg_get_constraintdef(pg_constraint.oid, true)
+    FROM pg_attribute
+    INNER JOIN pg_class ON pg_class.oid = attrelid
+    LEFT JOIN pg_constraint ON pg_class.oid = conrelid AND ARRAY[attnum] = conkey
+    WHERE attname = $1
+      AND contype = 'f'
diff --git a/src/tests/schema_details.rs b/src/tests/schema_details.rs
@@ -0,0 +1,66 @@
+use diesel::prelude::*;
+
+#[test]
+fn all_columns_called_crate_id_have_a_cascading_foreign_key() {
+    for (table_name, constraint) in get_fk_constraint_definitions("crate_id") {
+        let constraint = match constraint {
+            Some(c) => c,
+            None => {
+                panic!(
+                    "Column called crate_id on {} has no foreign key",
+                    table_name
+                )
+            }
+        };
+        if !constraint.definition.contains("ON DELETE CASCADE") {
+            panic!(
+                "Foreign key {} on table {} should have `ON DELETE CASCADE` \
+            but it doesn't.",
+                constraint.name,
+                table_name
+            );
+        }
+    }
+}
+
+#[test]
+fn all_columns_called_version_id_have_a_cascading_foreign_key() {
+    for (table_name, constraint) in get_fk_constraint_definitions("version_id") {
+        let constraint = match constraint {
+            Some(c) => c,
+            None => {
+                panic!(
+                    "Column called version_id on {} has no foreign key",
+                    table_name
+                )
+            }
+        };
+        if !constraint.definition.contains("ON DELETE CASCADE") {
+            panic!(
+                "Foreign key {} on table {} should have `ON DELETE CASCADE` \
+            but it doesn't.",
+                constraint.name,
+                table_name
+            );
+        }
+    }
+}
+
+#[derive(Queryable)]
+struct FkConstraint {
+    name: String,
+    definition: String,
+}
+
+fn get_fk_constraint_definitions(column_name: &str) -> Vec<(String, Option<FkConstraint>)> {
+    use diesel::expression::dsl::sql;
+    use diesel::types::Text;
+
+    let (_r, app, _) = ::app();
+    let conn = app.diesel_database.get().unwrap();
+
+    sql(include_str!("load_foreign_key_constraints.sql"))
+        .bind::<Text, _>(column_name)
+        .load(&*conn)
+        .unwrap()
+}
