Add the ability to add a category to a crate while publishing

Author: carols10cents

src/category.rs

@@ -1,11 +1,12 @@
+use std::collections::HashSet;
 use time::Timespec;
 
 use conduit::{Request, Response};
 use conduit_router::RequestParams;
 use pg::GenericConnection;
 use pg::rows::Row;
 
-use Model;
+use {Model, Crate};
 use db::RequestTransaction;
 use util::{RequestUtils, CargoResult, ChainError};
 use util::errors::NotFound;
@@ -35,6 +36,14 @@ impl Category {
         Ok(rows.iter().next().map(|r| Model::from_row(&r)))
     }
 
+    pub fn find_all_by_category(conn: &GenericConnection, names: &[String])
+                                  -> CargoResult<Vec<Category>> {
+        let stmt = try!(conn.prepare("SELECT * FROM categories \
+                                      WHERE category = ANY($1)"));
+        let rows = try!(stmt.query(&[&names]));
+        Ok(rows.iter().map(|r| Model::from_row(&r)).collect())
+    }
+
     pub fn encodable(self) -> EncodableCategory {
         let Category { id: _, crates_cnt, category, created_at } = self;
         EncodableCategory {
@@ -44,6 +53,52 @@ impl Category {
             category: category,
         }
     }
+
+    pub fn update_crate(conn: &GenericConnection,
+                        krate: &Crate,
+                        categories: &[String]) -> CargoResult<()> {
+        let old_categories = try!(krate.categories(conn));
+        let old_categories_ids: HashSet<_> = old_categories.iter().map(|cat| {
+            cat.id
+        }).collect();
+        // If a new category specified is not in the database, filter
+        // it out and don't add it.
+        let new_categories = try!(
+            Category::find_all_by_category(conn, categories)
+        );
+        let new_categories_ids: HashSet<_> = new_categories.iter().map(|cat| {
+            cat.id
+        }).collect();
+
+        let to_rm: Vec<_> = old_categories_ids
+                                .difference(&new_categories_ids)
+                                .cloned()
+                                .collect();
+        let to_add: Vec<_> = new_categories_ids
+                                .difference(&old_categories_ids)
+                                .cloned()
+                                .collect();
+
+        if to_rm.len() > 0 {
+            try!(conn.execute("DELETE FROM crates_categories \
+                                WHERE category_id = ANY($1) \
+                                  AND crate_id = $2",
+                              &[&to_rm, &krate.id]));
+        }
+
+        if !to_add.is_empty() {
+            let insert: Vec<_> = to_add.into_iter().map(|id| {
+                format!("({}, {})", krate.id, id)
+            }).collect();
+            let insert = insert.join(", ");
+            try!(conn.execute(&format!("INSERT INTO crates_categories \
+                                        (crate_id, category_id) VALUES {}",
+                                       insert),
+                              &[]));
+        }
+
+        Ok(())
+    }
 }
 
 impl Model for Category {

src/krate.rs

@@ -20,13 +20,14 @@ use semver;
 use time::{Timespec, Duration};
 use url::Url;
 
-use {Model, User, Keyword, Version};
+use {Model, User, Keyword, Version, Category};
 use app::{App, RequestApp};
 use db::RequestTransaction;
 use dependency::{Dependency, EncodableDependency};
 use download::{VersionDownload, EncodableVersionDownload};
 use git;
 use keyword::EncodableKeyword;
+use category::EncodableCategory;
 use upload;
 use user::RequestUser;
 use owner::{EncodableOwner, Owner, Rights, OwnerKind, Team, rights};
@@ -59,6 +60,7 @@ pub struct EncodableCrate {
     pub updated_at: String,
     pub versions: Option<Vec<i32>>,
     pub keywords: Option<Vec<String>>,
+    pub categories: Option<Vec<String>>,
     pub created_at: String,
     pub downloads: i32,
     pub max_version: String,
@@ -229,7 +231,8 @@ impl Crate {
 
     pub fn encodable(self,
                      versions: Option<Vec<i32>>,
-                     keywords: Option<&[Keyword]>)
+                     keywords: Option<&[Keyword]>,
+                     categories: Option<&[Category]>)
                      -> EncodableCrate {
         let Crate {
             name, created_at, updated_at, downloads, max_version, description,
@@ -241,6 +244,7 @@ impl Crate {
             None => Some(format!("/api/v1/crates/{}/versions", name)),
         };
         let keyword_ids = keywords.map(|kws| kws.iter().map(|kw| kw.keyword.clone()).collect());
+        let category_ids = categories.map(|cats| cats.iter().map(|cat| cat.category.clone()).collect());
         EncodableCrate {
             id: name.clone(),
             name: name.clone(),
@@ -249,6 +253,7 @@ impl Crate {
             downloads: downloads,
             versions: versions,
             keywords: keyword_ids,
+            categories: category_ids,
             max_version: max_version.to_string(),
             documentation: documentation,
             homepage: homepage,
@@ -386,6 +391,16 @@ impl Crate {
         Ok(rows.iter().map(|r| Model::from_row(&r)).collect())
     }
 
+    pub fn categories(&self, conn: &GenericConnection) -> CargoResult<Vec<Category>> {
+        let stmt = try!(conn.prepare("SELECT categories.* FROM categories \
+                                      LEFT JOIN crates_categories \
+                                      ON categories.id = \
+                                         crates_categories.category_id \
+                                      WHERE crates_categories.crate_id = $1"));
+        let rows = try!(stmt.query(&[&self.id]));
+        Ok(rows.iter().map(|r| Model::from_row(&r)).collect())
+    }
+
     /// Returns (dependency, dependent crate name)
     pub fn reverse_dependencies(&self,
                                 conn: &GenericConnection,
@@ -567,7 +582,7 @@ pub fn index(req: &mut Request) -> CargoResult<Response> {
     let mut crates = Vec::new();
     for row in try!(stmt.query(&args)).iter() {
         let krate: Crate = Model::from_row(&row);
-        crates.push(krate.encodable(None, None));
+        crates.push(krate.encodable(None, None, None));
     }
 
     // Query for the total count of crates
@@ -602,7 +617,7 @@ pub fn summary(req: &mut Request) -> CargoResult<Response> {
         let rows = try!(stmt.query(&[]));
         Ok(rows.iter().map(|r| {
             let krate: Crate = Model::from_row(&r);
-            krate.encodable(None, None)
+            krate.encodable(None, None, None)
         }).collect::<Vec<EncodableCrate>>())
     };
     let new_crates = try!(tx.prepare("SELECT * FROM crates \
@@ -639,19 +654,22 @@ pub fn show(req: &mut Request) -> CargoResult<Response> {
     let versions = try!(krate.versions(conn));
     let ids = versions.iter().map(|v| v.id).collect();
     let kws = try!(krate.keywords(conn));
+    let cats = try!(krate.categories(conn));
 
     #[derive(RustcEncodable)]
     struct R {
         krate: EncodableCrate,
         versions: Vec<EncodableVersion>,
         keywords: Vec<EncodableKeyword>,
+        categories: Vec<EncodableCategory>,
     }
     Ok(req.json(&R {
-        krate: krate.clone().encodable(Some(ids), Some(&kws)),
+        krate: krate.clone().encodable(Some(ids), Some(&kws), Some(&cats)),
         versions: versions.into_iter().map(|v| {
             v.encodable(&krate.name)
         }).collect(),
         keywords: kws.into_iter().map(|k| k.encodable()).collect(),
+        categories: cats.into_iter().map(|k| k.encodable()).collect(),
     }))
 }
 
@@ -669,6 +687,10 @@ pub fn new(req: &mut Request) -> CargoResult<Response> {
                                      .unwrap_or(&[]);
     let keywords = keywords.iter().map(|k| k[..].to_string()).collect::<Vec<_>>();
 
+    let categories = new_crate.categories.as_ref().map(|s| &s[..])
+                                     .unwrap_or(&[]);
+    let categories = categories.iter().map(|k| k[..].to_string()).collect::<Vec<_>>();
+
     // Persist the new crate, if it doesn't already exist
     let mut krate = try!(Crate::find_or_insert(try!(req.tx()), name, user.id,
                                                &new_crate.description,
@@ -713,6 +735,9 @@ pub fn new(req: &mut Request) -> CargoResult<Response> {
     // Update all keywords for this crate
     try!(Keyword::update_crate(try!(req.tx()), &krate, &keywords));
 
+    // Update all categories for this crate
+    try!(Category::update_crate(try!(req.tx()), &krate, &categories));
+
     // Upload the crate to S3
     let mut handle = req.app().handle();
     let path = krate.s3_path(&vers.to_string());
@@ -769,7 +794,7 @@ pub fn new(req: &mut Request) -> CargoResult<Response> {
 
     #[derive(RustcEncodable)]
     struct R { krate: EncodableCrate }
-    Ok(req.json(&R { krate: krate.encodable(None, None) }))
+    Ok(req.json(&R { krate: krate.encodable(None, None, None) }))
 }
 
 fn parse_new_headers(req: &mut Request) -> CargoResult<(upload::NewCrate, User)> {

src/tests/all.rs

@@ -272,20 +272,21 @@ fn new_req(app: Arc<App>, krate: &str, version: &str) -> MockRequest {
 fn new_req_full(app: Arc<App>, krate: Crate, version: &str,
                 deps: Vec<u::CrateDependency>) -> MockRequest {
     let mut req = ::req(app, Method::Put, "/api/v1/crates/new");
-    req.with_body(&new_req_body(krate, version, deps, Vec::new()));
+    req.with_body(&new_req_body(krate, version, deps, Vec::new(), Vec::new()));
     return req;
 }
 
 fn new_req_with_keywords(app: Arc<App>, krate: Crate, version: &str,
                          kws: Vec<String>) -> MockRequest {
     let mut req = ::req(app, Method::Put, "/api/v1/crates/new");
-    req.with_body(&new_req_body(krate, version, Vec::new(), kws));
+    req.with_body(&new_req_body(krate, version, Vec::new(), kws, Vec::new()));
     return req;
 }
 
 fn new_req_body(krate: Crate, version: &str, deps: Vec<u::CrateDependency>,
-                kws: Vec<String>) -> Vec<u8> {
+                kws: Vec<String>, cats: Vec<String>) -> Vec<u8> {
     let kws = kws.into_iter().map(u::Keyword).collect();
+    let cats = cats.into_iter().map(u::Category).collect();
     new_crate_to_body(&u::NewCrate {
         name: u::CrateName(krate.name),
         vers: u::CrateVersion(semver::Version::parse(version).unwrap()),
@@ -297,6 +298,7 @@ fn new_req_body(krate: Crate, version: &str, deps: Vec<u::CrateDependency>,
         documentation: krate.documentation,
         readme: krate.readme,
         keywords: Some(u::KeywordList(kws)),
+        categories: Some(u::CategoryList(cats)),
         license: Some("MIT".to_string()),
         license_file: None,
         repository: krate.repository,

src/tests/category.rs

@@ -1,6 +1,9 @@
-use conduit::{Handler, Method};
+use postgres::GenericConnection;
+use conduit::{Handler, Request, Method};
+use conduit_test::MockRequest;
 
-use cargo_registry::category::EncodableCategory;
+use cargo_registry::db::RequestTransaction;
+use cargo_registry::category::{Category, EncodableCategory};
 
 #[derive(RustcDecodable)]
 struct CategoryList { categories: Vec<EncodableCategory>, meta: CategoryMeta }
@@ -38,3 +41,66 @@ fn show() {
     let json: GoodCategory = ::json(&mut response);
     assert_eq!(json.category.category, "foo");
 }
+
+fn tx(req: &Request) -> &GenericConnection { req.tx().unwrap() }
+
+#[test]
+fn update_crate() {
+    let (_b, app, middle) = ::app();
+    let mut req = ::req(app, 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("foo"));
+    ::mock_category(&mut req, "cat1");
+    ::mock_category(&mut req, "cat2");
+
+    // Updating with no categories has no effect
+    Category::update_crate(tx(&req), &krate, &[]).unwrap();
+    assert_eq!(cnt(&mut req, "cat1"), 0);
+    assert_eq!(cnt(&mut req, "cat2"), 0);
+
+    // Happy path adding one category
+    Category::update_crate(tx(&req), &krate, &["cat1".to_string()]).unwrap();
+    assert_eq!(cnt(&mut req, "cat1"), 1);
+    assert_eq!(cnt(&mut req, "cat2"), 0);
+
+    // Replacing one category with another
+    Category::update_crate(tx(&req), &krate, &["cat2".to_string()]).unwrap();
+    assert_eq!(cnt(&mut req, "cat1"), 0);
+    assert_eq!(cnt(&mut req, "cat2"), 1);
+
+    // Removing one category
+    Category::update_crate(tx(&req), &krate, &[]).unwrap();
+    assert_eq!(cnt(&mut req, "cat1"), 0);
+    assert_eq!(cnt(&mut req, "cat2"), 0);
+
+    // Adding 2 categories
+    Category::update_crate(tx(&req), &krate, &["cat1".to_string(),
+                                               "cat2".to_string()]).unwrap();
+    assert_eq!(cnt(&mut req, "cat1"), 1);
+    assert_eq!(cnt(&mut req, "cat2"), 1);
+
+    // Removing all categories
+    Category::update_crate(tx(&req), &krate, &[]).unwrap();
+    assert_eq!(cnt(&mut req, "cat1"), 0);
+    assert_eq!(cnt(&mut req, "cat2"), 0);
+
+    // Attempting to add one valid category and one invalid category
+    Category::update_crate(tx(&req), &krate, &["cat1".to_string(),
+                                               "catnope".to_string()]).unwrap();
+
+    assert_eq!(cnt(&mut req, "cat1"), 1);
+    assert_eq!(cnt(&mut req, "cat2"), 0);
+
+    // Does not add the invalid category to the category list
+    // (unlike the behavior of keywords)
+    req.with_path("/api/v1/categories");
+    let mut response = ok_resp!(middle.call(&mut req));
+    let json: CategoryList = ::json(&mut response);
+    assert_eq!(json.categories.len(), 2);
+    assert_eq!(json.meta.total, 2);
+}

src/tests/krate.rs

@@ -48,6 +48,7 @@ fn new_crate(name: &str) -> u::NewCrate {
         documentation: None,
         readme: None,
         keywords: None,
+        categories: None,
         license: Some("MIT".to_string()),
         license_file: None,
         repository: None,
@@ -190,7 +191,7 @@ fn exact_match_on_queries_with_sort() {
     krate4.description = Some("other const".to_string());
     krate4.downloads = 999999;
     let (k4, _) = ::mock_crate(&mut req, krate4.clone());
-    
+
     {
         let req2: &mut Request = &mut req;
         let tx = req2.tx().unwrap();
@@ -461,7 +462,9 @@ fn new_crate_owner() {
     assert_eq!(::json::<CrateList>(&mut response).crates.len(), 1);
 
     // And upload a new crate as the first user
-    let body = ::new_req_body(::krate("foo"), "2.0.0", Vec::new(), Vec::new());
+    let body = ::new_req_body(
+        ::krate("foo"), "2.0.0", Vec::new(), Vec::new(), Vec::new()
+    );
     req.mut_extensions().insert(u2);
     let mut response = ok_resp!(middle.call(req.with_path("/api/v1/crates/new")
                                                .with_method(Method::Put)