Switch several category routes over to diesel

Author: jtgeibel

src/category.rs

@@ -193,17 +193,18 @@ impl Category {
         Ok(invalid_categories)
     }
 
-    pub fn count_toplevel(conn: &GenericConnection) -> CargoResult<i64> {
-        let sql = format!(
+    pub fn count_toplevel(conn: &PgConnection) -> CargoResult<i64> {
+        use diesel::expression::*;
+        use diesel::types::*;
+
+        let query = sql::<BigInt>(
             "\
             SELECT COUNT(*) \
-            FROM {} \
-            WHERE category NOT LIKE '%::%'",
-            Model::table_name(None::<Self>)
+            FROM categories \
+            WHERE category NOT LIKE '%::%'"
         );
-        let stmt = conn.prepare(&sql)?;
-        let rows = stmt.query(&[])?;
-        Ok(rows.iter().next().unwrap().get("count"))
+        let count = query.get_result(&*conn)?;
+        Ok(count)
     }
 
     pub fn toplevel(
@@ -269,27 +270,31 @@ impl Category {
         Ok(categories)
     }
 
-    pub fn subcategories(&self, conn: &GenericConnection) -> CargoResult<Vec<Category>> {
-        let stmt = conn.prepare(
+    pub fn subcategories(&self, conn: &PgConnection) -> CargoResult<Vec<Category>> {
+        use diesel::expression::*;
+        use diesel::types::*;
+
+        let query = sql::<(Integer, Text, Text, Text, Integer, Timestamp)>(
             "\
-            SELECT c.id, c.category, c.slug, c.description, c.created_at, \
+            SELECT c.id, c.category, c.slug, c.description, \
             COALESCE (( \
                 SELECT sum(c2.crates_cnt)::int \
                 FROM categories as c2 \
                 WHERE c2.slug = c.slug \
                 OR c2.slug LIKE c.slug || '::%' \
-            ), 0) as crates_cnt \
+            ), 0) as crates_cnt, \
+            c.created_at \
             FROM categories as c \
             WHERE c.category ILIKE $1 || '::%' \
             AND c.category NOT ILIKE $1 || '::%::%'",
-        )?;
+        ).bind::<Text, _>(&self.category);
 
-        let rows = stmt.query(&[&self.category])?;
-        Ok(rows.iter().map(|r| Model::from_row(&r)).collect())
+        let rows = query.get_results(conn)?;
+        Ok(rows)
     }
 }
 
-#[derive(Insertable, Default)]
+#[derive(Insertable, AsChangeset, Default)]
 #[table_name = "categories"]
 pub struct NewCategory<'a> {
     pub category: &'a str,
@@ -312,6 +317,18 @@ impl<'a> NewCategory<'a> {
 
         categories.filter(slug.eq(self.slug)).first(conn)
     }
+
+    /// Inserts the user into the database, or updates an existing one.
+    pub fn create_or_update(&self, conn: &PgConnection) -> CargoResult<Category> {
+        use diesel::insert;
+        use diesel::pg::upsert::*;
+
+        insert(&self.on_conflict(categories::slug, do_update().set(self)))
+            .into(categories::table)
+            .get_result(conn)
+            .map_err(Into::into)
+    }
+
 }
 
 impl Model for Category {
@@ -332,16 +349,16 @@ impl Model for Category {
 
 /// Handles the `GET /categories` route.
 pub fn index(req: &mut Request) -> CargoResult<Response> {
-    let conn = req.tx()?;
+    let conn = req.db_conn()?;
     let (offset, limit) = req.pagination(10, 100)?;
     let query = req.query();
     let sort = query.get("sort").map_or("alpha", String::as_str);
 
-    let categories = Category::toplevel_old(conn, sort, limit, offset)?;
+    let categories = Category::toplevel(&conn, sort, limit, offset)?;
     let categories = categories.into_iter().map(Category::encodable).collect();
 
     // Query for the total count of categories
-    let total = Category::count_toplevel(conn)?;
+    let total = Category::count_toplevel(&conn)?;
 
     #[derive(RustcEncodable)]
     struct R {
@@ -361,13 +378,15 @@ pub fn index(req: &mut Request) -> CargoResult<Response> {
 
 /// Handles the `GET /categories/:category_id` route.
 pub fn show(req: &mut Request) -> CargoResult<Response> {
-    let slug = &req.params()["category_id"];
-    let conn = req.tx()?;
-    let cat = Category::find_by_slug(conn, slug)?;
-    let subcats = cat.subcategories(conn)?
-        .into_iter()
-        .map(|s| s.encodable())
-        .collect();
+    use self::categories::dsl::{categories, slug};
+
+    let id = &req.params()["category_id"];
+    let conn = req.db_conn()?;
+    let cat = categories.filter(slug.eq(id)).first::<Category>(&*conn)?;
+    let subcats = cat.subcategories(&*conn)?.into_iter().map(|s| {
+        s.encodable()
+    }).collect();
+
     let cat = cat.encodable();
     let cat_with_subcats = EncodableCategoryWithSubcategories {
         id: cat.id,

src/tests/all.rs

@@ -32,7 +32,7 @@ use cargo_registry::krate::NewCrate;
 use cargo_registry::upload as u;
 use cargo_registry::user::NewUser;
 use cargo_registry::version::NewVersion;
-use cargo_registry::{User, Crate, Version, Dependency, Category, Model, Replica};
+use cargo_registry::{User, Crate, Version, Dependency, Replica};
 use conduit::{Request, Method};
 use conduit_test::MockRequest;
 use diesel::pg::PgConnection;
@@ -436,16 +436,17 @@ fn new_category<'a>(category: &'a str, slug: &'a str) -> NewCategory<'a> {
     }
 }
 
-fn mock_category(req: &mut Request, name: &str, slug: &str) -> Category {
-    let conn = req.tx().unwrap();
-    let stmt = conn.prepare(
-        " \
-        INSERT INTO categories (category, slug) \
-        VALUES ($1, $2) \
-        RETURNING *",
-    ).unwrap();
-    let rows = stmt.query(&[&name, &slug]).unwrap();
-    Model::from_row(&rows.iter().next().unwrap())
+fn new_crate(name: &str) -> NewCrate {
+    NewCrate {
+        name: name,
+        description: Some("desc"),
+        homepage: None,
+        documentation: None,
+        readme: None,
+        repository: None,
+        license: Some("MIT"),
+        max_upload_size: None,
+    }
 }
 
 fn logout(req: &mut Request) {

src/tests/category.rs

@@ -1,7 +1,6 @@
 use conduit::{Handler, Method};
 use conduit_test::MockRequest;
 
-use cargo_registry::db::RequestTransaction;
 use cargo_registry::category::{Category, EncodableCategory, EncodableCategoryWithSubcategories};
 
 #[derive(RustcDecodable)]
@@ -25,7 +24,7 @@ struct CategoryWithSubcategories {
 #[test]
 fn index() {
     let (_b, app, middle) = ::app();
-    let mut req = ::req(app, Method::Get, "/api/v1/categories");
+    let mut req = ::req(app.clone(), Method::Get, "/api/v1/categories");
 
     // List 0 categories if none exist
     let mut response = ok_resp!(middle.call(&mut req));
@@ -34,9 +33,11 @@ fn index() {
     assert_eq!(json.meta.total, 0);
 
     // Create a category and a subcategory
-    ::mock_category(&mut req, "foo", "foo");
-    ::mock_category(&mut req, "foo::bar", "foo::bar");
-
+    {
+        let conn = t!(app.diesel_database.get());
+        ::new_category("foo", "foo").create_or_update(&conn).unwrap();
+        ::new_category("foo::bar", "foo::bar").create_or_update(&conn).unwrap();
+    }
     let mut response = ok_resp!(middle.call(&mut req));
     let json: CategoryList = ::json(&mut response);
 
@@ -51,13 +52,18 @@ fn show() {
     let (_b, app, middle) = ::app();
 
     // Return not found if a category doesn't exist
-    let mut req = ::req(app, Method::Get, "/api/v1/categories/foo-bar");
+    let mut req = ::req(app.clone(), Method::Get, "/api/v1/categories/foo-bar");
     let response = t_resp!(middle.call(&mut req));
     assert_eq!(response.status.0, 404);
 
     // Create a category and a subcategory
-    ::mock_category(&mut req, "Foo Bar", "foo-bar");
-    ::mock_category(&mut req, "Foo Bar::Baz", "foo-bar::baz");
+    {
+        let conn = t!(app.diesel_database.get());
+
+        // Create a category and a subcategory
+        t!(::new_category("Foo Bar", "foo-bar").create_or_update(&conn));
+        t!(::new_category("Foo Bar::Baz", "foo-bar::baz").create_or_update(&conn));
+    }
 
     // The category and its subcategories should be in the json
     let mut response = ok_resp!(middle.call(&mut req));
@@ -71,57 +77,82 @@ fn show() {
 #[test]
 fn update_crate() {
     let (_b, app, middle) = ::app();
-    let mut req = ::req(app, Method::Get, "/api/v1/categories/foo");
+    let mut req = ::req(app.clone(), Method::Get, "/api/v1/categories/foo");
     let cnt = |req: &mut MockRequest, cat: &str| {
         req.with_path(&format!("/api/v1/categories/{}", cat));
         let mut response = ok_resp!(middle.call(req));
         ::json::<GoodCategory>(&mut response).category.crates_cnt as usize
     };
-    ::mock_user(&mut req, ::user("foo"));
-    let (krate, _) = ::mock_crate(&mut req, ::krate("foocat"));
-    ::mock_category(&mut req, "cat1", "cat1");
-    ::mock_category(&mut req, "Category 2", "category-2");
+
+    let krate = {
+        let conn = t!(app.diesel_database.get());
+        let user = t!(::new_user("foo").create_or_update(&conn));
+        t!(::new_category("cat1", "cat1").create_or_update(&conn));
+        t!(::new_category("Category 2", "category-2").create_or_update(&conn));
+        t!(::new_crate("foo_crate").create_or_update(&conn, None, user.id))
+    };
 
     // Updating with no categories has no effect
-    Category::update_crate_old(req.tx().unwrap(), &krate, &[]).unwrap();
+    {
+        let conn = t!(app.diesel_database.get());
+        Category::update_crate(&conn, &krate, &[]).unwrap();
+    }
     assert_eq!(cnt(&mut req, "cat1"), 0);
     assert_eq!(cnt(&mut req, "category-2"), 0);
 
     // Happy path adding one category
-    Category::update_crate_old(req.tx().unwrap(), &krate, &["cat1".to_string()]).unwrap();
+    {
+        let conn = t!(app.diesel_database.get());
+        Category::update_crate(&conn, &krate, &["cat1"]).unwrap();
+    }
     assert_eq!(cnt(&mut req, "cat1"), 1);
     assert_eq!(cnt(&mut req, "category-2"), 0);
 
     // Replacing one category with another
-    Category::update_crate_old(req.tx().unwrap(), &krate, &["category-2".to_string()]).unwrap();
+    {
+        let conn = t!(app.diesel_database.get());
+        Category::update_crate(&conn, &krate, &["category-2"]).unwrap();
+    }
     assert_eq!(cnt(&mut req, "cat1"), 0);
     assert_eq!(cnt(&mut req, "category-2"), 1);
 
     // Removing one category
-    Category::update_crate_old(req.tx().unwrap(), &krate, &[]).unwrap();
+    {
+        let conn = t!(app.diesel_database.get());
+        Category::update_crate(&conn, &krate, &[]).unwrap();
+    }
     assert_eq!(cnt(&mut req, "cat1"), 0);
     assert_eq!(cnt(&mut req, "category-2"), 0);
 
     // Adding 2 categories
-    Category::update_crate_old(
-        req.tx().unwrap(),
-        &krate,
-        &["cat1".to_string(), "category-2".to_string()],
-    ).unwrap();
+    {
+        let conn = t!(app.diesel_database.get());
+        Category::update_crate(
+            &conn,
+            &krate,
+            &["cat1", "category-2"],
+        ).unwrap();
+    }
     assert_eq!(cnt(&mut req, "cat1"), 1);
     assert_eq!(cnt(&mut req, "category-2"), 1);
 
     // Removing all categories
-    Category::update_crate_old(req.tx().unwrap(), &krate, &[]).unwrap();
+    {
+        let conn = t!(app.diesel_database.get());
+        Category::update_crate(&conn, &krate, &[]).unwrap();
+    }
     assert_eq!(cnt(&mut req, "cat1"), 0);
     assert_eq!(cnt(&mut req, "category-2"), 0);
 
     // Attempting to add one valid category and one invalid category
-    let invalid_categories = Category::update_crate_old(
-        req.tx().unwrap(),
-        &krate,
-        &["cat1".to_string(), "catnope".to_string()],
-    ).unwrap();
+    let invalid_categories = {
+        let conn = t!(app.diesel_database.get());
+        Category::update_crate(
+            &conn,
+            &krate,
+            &["cat1", "catnope"],
+        ).unwrap()
+    };
     assert_eq!(invalid_categories, vec!["catnope".to_string()]);
     assert_eq!(cnt(&mut req, "cat1"), 1);
     assert_eq!(cnt(&mut req, "category-2"), 0);
@@ -135,17 +166,23 @@ fn update_crate() {
     assert_eq!(json.meta.total, 2);
 
     // Attempting to add a category by display text; must use slug
-    Category::update_crate_old(req.tx().unwrap(), &krate, &["Category 2".to_string()]).unwrap();
+    {
+        let conn = t!(app.diesel_database.get());
+        Category::update_crate(&conn, &krate, &["Category 2"]).unwrap();
+    }
     assert_eq!(cnt(&mut req, "cat1"), 0);
     assert_eq!(cnt(&mut req, "category-2"), 0);
 
     // Add a category and its subcategory
-    ::mock_category(&mut req, "cat1::bar", "cat1::bar");
-    Category::update_crate_old(
-        req.tx().unwrap(),
-        &krate,
-        &["cat1".to_string(), "cat1::bar".to_string()],
-    ).unwrap();
+    {
+        let conn = t!(app.diesel_database.get());
+        t!(::new_category("cat1::bar", "cat1::bar").create_or_update(&conn));
+        Category::update_crate(
+            &conn,
+            &krate,
+            &["cat1", "cat1::bar"],
+        ).unwrap();
+    }
     assert_eq!(cnt(&mut req, "cat1"), 1);
     assert_eq!(cnt(&mut req, "cat1::bar"), 1);
     assert_eq!(cnt(&mut req, "category-2"), 0);