Merge #903

903: Port `categories::sync` over to use Diesel r=carols10cents

I've changed this to do the whole thing in one insert rather than one
per category.

Author: bors-voyager[bot]

src/bin/server.rs

@@ -144,7 +144,8 @@ fn main() {
 
     // On every server restart, ensure the categories available in the database match
     // the information in *src/categories.toml*.
-    cargo_registry::categories::sync().unwrap();
+    let categories_toml = include_str!("../categories.toml");
+    cargo_registry::categories::sync(&categories_toml).unwrap();
 
     let port = if heroku {
         8888

src/categories.rs

@@ -1,9 +1,13 @@
 // Sync available crate categories from `src/categories.toml`.
 // Runs when the server is started.
 
+use diesel;
+use diesel::prelude::*;
+use diesel::pg::PgConnection;
 use toml;
 
 use db;
+use schema::categories;
 use util::errors::{CargoResult, ChainError, internal};
 
 #[derive(Debug)]
@@ -87,45 +91,56 @@ fn categories_from_toml(
     Ok(result)
 }
 
-pub fn sync() -> CargoResult<()> {
-    let conn = db::connect_now_old();
-    let tx = conn.transaction().unwrap();
+#[derive(Insertable, Debug)]
+#[table_name = "categories"]
+struct NewCategory {
+    slug: String,
+    category: String,
+    description: String,
+}
 
-    let categories = include_str!("./categories.toml");
-    let toml: toml::value::Table =
-        toml::from_str(categories).expect("Could not parse categories.toml");
-
-    let categories =
-        categories_from_toml(&toml, None).expect("Could not convert categories from TOML");
-
-    for category in &categories {
-        tx.execute(
-            "\
-             INSERT INTO categories (slug, category, description) \
-             VALUES (LOWER($1), $2, $3) \
-             ON CONFLICT (slug) DO UPDATE \
-             SET category = EXCLUDED.category, \
-             description = EXCLUDED.description;",
-            &[&category.slug, &category.name, &category.description],
-        )?;
-    }
+pub fn sync(toml_str: &str) -> CargoResult<()> {
+    let conn = db::connect_now().unwrap();
+    sync_with_connection(toml_str, &conn)
+}
+
+pub fn sync_with_connection(toml_str: &str, conn: &PgConnection) -> CargoResult<()> {
+    use diesel::pg::upsert::*;
+    use diesel::expression::dsl::all;
 
-    let in_clause = categories
-        .iter()
-        .map(|category| format!("LOWER('{}')", category.slug))
-        .collect::<Vec<_>>()
-        .join(",");
-
-    tx.execute(
-        &format!(
-            "\
-             DELETE FROM categories \
-             WHERE slug NOT IN ({});",
-            in_clause
-        ),
-        &[],
-    )?;
-    tx.set_commit();
-    tx.finish().unwrap();
-    Ok(())
+    let toml: toml::value::Table =
+        toml::from_str(toml_str).expect("Could not parse categories toml");
+
+    let categories = categories_from_toml(&toml, None)
+        .expect("Could not convert categories from TOML")
+        .into_iter()
+        .map(|c| {
+            NewCategory {
+                slug: c.slug.to_lowercase(),
+                category: c.name,
+                description: c.description,
+            }
+        })
+        .collect::<Vec<_>>();
+
+    let to_insert = categories.on_conflict(
+        categories::slug,
+        do_update().set((
+            categories::category.eq(excluded(categories::category)),
+            categories::description.eq(
+                excluded(categories::description),
+            ),
+        )),
+    );
+
+    conn.transaction(|| {
+        let slugs = diesel::insert(&to_insert)
+            .into(categories::table)
+            .returning(categories::slug)
+            .get_results::<String>(&*conn)?;
+
+        let to_delete = categories::table.filter(categories::slug.ne(all(slugs)));
+        diesel::delete(to_delete).execute(&*conn)?;
+        Ok(())
+    })
 }

src/tests/all.rs

@@ -87,6 +87,7 @@ struct Bad {
 }
 
 mod badge;
+mod categories;
 mod category;
 mod git;
 mod keyword;

src/tests/categories.rs

@@ -0,0 +1,90 @@
+use cargo_registry::schema::categories;
+use diesel::*;
+use diesel::pg::PgConnection;
+use dotenv::dotenv;
+
+use std::env;
+
+const ALGORITHMS: &'static str = r#"
+[algorithms]
+name = "Algorithms"
+description = """
+Rust implementations of core algorithms such as hashing, sorting, \
+searching, and more.\
+""""#;
+
+const ALGORITHMS_AND_SUCH: &'static str = r#"
+[algorithms]
+name = "Algorithms"
+description = """
+Rust implementations of core algorithms such as hashing, sorting, \
+searching, and more.\
+"""
+
+[algorithms.categories.such]
+name = "Such"
+description = """
+Other stuff
+""""#;
+
+const ALGORITHMS_AND_ANOTHER: &'static str = r#"
+[algorithms]
+name = "Algorithms"
+description = """
+Rust implementations of core algorithms such as hashing, sorting, \
+searching, and more.\
+"""
+
+[another]
+name = "Another"
+description = "Another category ho hum"
+"#;
+
+fn pg_connection() -> PgConnection {
+    let _ = dotenv();
+    let database_url =
+        env::var("TEST_DATABASE_URL").expect("TEST_DATABASE_URL must be set to run tests");
+    let conn = PgConnection::establish(&database_url).unwrap();
+    conn.begin_test_transaction().unwrap();
+    conn
+}
+
+fn select_slugs(conn: &PgConnection) -> Vec<String> {
+    categories::table
+        .select(categories::slug)
+        .order(categories::slug)
+        .load::<String>(conn)
+        .unwrap()
+}
+
+#[test]
+fn sync_adds_new_categories() {
+    let conn = pg_connection();
+
+    ::cargo_registry::categories::sync_with_connection(ALGORITHMS_AND_SUCH, &conn).unwrap();
+
+    let categories = select_slugs(&conn);
+    assert_eq!(categories, vec!["algorithms", "algorithms::such"]);
+}
+
+#[test]
+fn sync_removes_missing_categories() {
+    let conn = pg_connection();
+
+    ::cargo_registry::categories::sync_with_connection(ALGORITHMS_AND_SUCH, &conn).unwrap();
+    ::cargo_registry::categories::sync_with_connection(ALGORITHMS, &conn).unwrap();
+
+    let categories = select_slugs(&conn);
+    assert_eq!(categories, vec!["algorithms"]);
+}
+
+#[test]
+fn sync_adds_and_removes() {
+    let conn = pg_connection();
+
+    ::cargo_registry::categories::sync_with_connection(ALGORITHMS_AND_SUCH, &conn).unwrap();
+    ::cargo_registry::categories::sync_with_connection(ALGORITHMS_AND_ANOTHER, &conn).unwrap();
+
+    let categories = select_slugs(&conn);
+    assert_eq!(categories, vec!["algorithms", "another"]);
+}