Return warnings about invalid categories

Author: carols10cents

src/category.rs

@@ -63,16 +63,23 @@ impl Category {
 
     pub fn update_crate(conn: &GenericConnection,
                         krate: &Crate,
-                        categories: &[String]) -> CargoResult<()> {
+                        categories: &[String]) -> CargoResult<Vec<String>> {
         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.
+        // it out and don't add it. Return it to be able to warn about it.
+        let mut invalid_categories = vec![];
         let new_categories: Vec<Category> = categories.iter().flat_map(|c| {
-            Category::find_by_slug(conn, &c).ok()
+            match Category::find_by_slug(conn, &c) {
+                Ok(cat) => Some(cat),
+                Err(_) => {
+                    invalid_categories.push(c.to_string());
+                    None
+                },
+            }
         }).collect();
 
         let new_categories_ids: HashSet<_> = new_categories.iter().map(|cat| {
@@ -106,7 +113,7 @@ impl Category {
                               &[]));
         }
 
-        Ok(())
+        Ok(invalid_categories)
     }
 }
 

src/krate.rs

@@ -739,8 +739,16 @@ 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));
+    // Update all categories for this crate, collecting any invalid categories
+    // in order to be able to warn about them
+    let ignored_invalid_categories = try!(
+        Category::update_crate(try!(req.tx()), &krate, &categories)
+    );
+    let warnings: Vec<String> =
+        ignored_invalid_categories.iter().map(|category| {
+            format!("'{}' is not a recognized category name \
+                     and has been ignored.", category)
+        }).collect();
 
     // Upload the crate to S3
     let mut handle = req.app().handle();
@@ -797,8 +805,8 @@ pub fn new(req: &mut Request) -> CargoResult<Response> {
     bomb.path = None;
 
     #[derive(RustcEncodable)]
-    struct R { krate: EncodableCrate }
-    Ok(req.json(&R { krate: krate.minimal_encodable() }))
+    struct R { krate: EncodableCrate, warnings: Vec<String> }
+    Ok(req.json(&R { krate: krate.minimal_encodable(), warnings: warnings }))
 }
 
 fn parse_new_headers(req: &mut Request) -> CargoResult<(upload::NewCrate, User)> {

src/tests/all.rs

@@ -283,6 +283,13 @@ fn new_req_with_keywords(app: Arc<App>, krate: Crate, version: &str,
     return req;
 }
 
+fn new_req_with_categories(app: Arc<App>, krate: Crate, version: &str,
+                           cats: Vec<String>) -> MockRequest {
+    let mut req = ::req(app, Method::Put, "/api/v1/crates/new");
+    req.with_body(&new_req_body(krate, version, Vec::new(), Vec::new(), cats));
+    return req;
+}
+
 fn new_req_body_foo_version_2() -> Vec<u8> {
     new_req_body(::krate("foo"), "2.0.0", Vec::new(), Vec::new(), Vec::new())
 }

src/tests/category.rs

@@ -94,9 +94,11 @@ fn update_crate() {
     assert_eq!(cnt(&mut req, "category-2"), 0);
 
     // Attempting to add one valid category and one invalid category
-    Category::update_crate(tx(&req), &krate, &["cat1".to_string(),
-                                               "catnope".to_string()]).unwrap();
-
+    let invalid_crates = Category::update_crate(
+        tx(&req), &krate, &["cat1".to_string(),
+                            "catnope".to_string()]
+    ).unwrap();
+    assert_eq!(invalid_crates, vec!["catnope".to_string()]);
     assert_eq!(cnt(&mut req, "cat1"), 1);
     assert_eq!(cnt(&mut req, "category-2"), 0);
 

src/tests/http-data/krate_good_categories

@@ -0,0 +1,21 @@
+===REQUEST 331
+PUT http://alexcrichton-test.s3.amazonaws.com/crates/foo/foo-1.0.0.crate HTTP/1.1
+Accept: */*
+Proxy-Connection: Keep-Alive
+Authorization: AWS AKIAJF3GEK7N44BACDZA:GDxGb6r3SIqo9wXuzHrgMNWekwk=
+Content-Length: 0
+Host: alexcrichton-test.s3.amazonaws.com
+Content-Type: application/x-tar
+Date: Sun, 28 Jun 2015 14:07:17 -0700
+
+
+===RESPONSE 258
+HTTP/1.1 200
+x-amz-request-id: CB0E925D8E3AB3E8
+x-amz-id-2: SiaMwszM1p2TzXlLauvZ6kRKcUCg7HoyBW29vts42w9ArrLwkJWl8vuvPuGFkpM6XGH+YXN852g=
+date: Sun, 28 Jun 2015 21:07:51 GMT
+etag: "d41d8cd98f00b204e9800998ecf8427e"
+content-length: 0
+server: AmazonS3
+
+

src/tests/http-data/krate_ignored_categories

@@ -0,0 +1,21 @@
+===REQUEST 331
+PUT http://alexcrichton-test.s3.amazonaws.com/crates/foo/foo-1.0.0.crate HTTP/1.1
+Accept: */*
+Proxy-Connection: Keep-Alive
+Authorization: AWS AKIAJF3GEK7N44BACDZA:GDxGb6r3SIqo9wXuzHrgMNWekwk=
+Content-Length: 0
+Host: alexcrichton-test.s3.amazonaws.com
+Content-Type: application/x-tar
+Date: Sun, 28 Jun 2015 14:07:17 -0700
+
+
+===RESPONSE 258
+HTTP/1.1 200
+x-amz-request-id: CB0E925D8E3AB3E8
+x-amz-id-2: SiaMwszM1p2TzXlLauvZ6kRKcUCg7HoyBW29vts42w9ArrLwkJWl8vuvPuGFkpM6XGH+YXN852g=
+date: Sun, 28 Jun 2015 21:07:51 GMT
+etag: "d41d8cd98f00b204e9800998ecf8427e"
+content-length: 0
+server: AmazonS3
+
+

src/tests/krate.rs

@@ -26,7 +26,7 @@ struct CrateMeta { total: i32 }
 #[derive(RustcDecodable)]
 struct GitCrate { name: String, vers: String, deps: Vec<String>, cksum: String }
 #[derive(RustcDecodable)]
-struct GoodCrate { krate: EncodableCrate }
+struct GoodCrate { krate: EncodableCrate, warnings: Vec<String> }
 #[derive(RustcDecodable)]
 struct CrateResponse { krate: EncodableCrate, versions: Vec<EncodableVersion>, keywords: Vec<EncodableKeyword> }
 #[derive(RustcDecodable)]
@@ -884,6 +884,36 @@ fn bad_keywords() {
     }
 }
 
+#[test]
+fn good_categories() {
+    let (_b, app, middle) = ::app();
+    let krate = ::krate("foo");
+    let cats = vec!["cat1".into()];
+    let mut req = ::new_req_with_categories(app, krate, "1.0.0", cats);
+    ::mock_category(&mut req, "cat1", "cat1");
+    ::mock_user(&mut req, ::user("foo"));
+    let mut response = ok_resp!(middle.call(&mut req));
+    let json: GoodCrate = ::json(&mut response);
+    assert_eq!(json.krate.name, "foo");
+    assert_eq!(json.krate.max_version, "1.0.0");
+    assert_eq!(json.warnings.len(), 0);
+}
+
+#[test]
+fn ignored_categories() {
+    let (_b, app, middle) = ::app();
+    let krate = ::krate("foo");
+    let cats = vec!["bar".into()];
+    let mut req = ::new_req_with_categories(app, krate, "1.0.0", cats);
+    ::mock_user(&mut req, ::user("foo"));
+    let mut response = ok_resp!(middle.call(&mut req));
+    let json: GoodCrate = ::json(&mut response);
+    assert_eq!(json.krate.name, "foo");
+    assert_eq!(json.krate.max_version, "1.0.0");
+    assert_eq!(json.warnings, vec!["\'bar\' is not a recognized category name \
+                                    and has been ignored.".to_string()]);
+}
+
 #[test]
 fn reverse_dependencies() {
     let (_b, app, middle) = ::app();