Use methods instead of add/remove for owners

Author: alexcrichton

src/krate.rs

@@ -703,7 +703,15 @@ pub fn owners(req: &mut Request) -> CargoResult<Response> {
     Ok(req.json(&R{ users: owners }))
 }
 
-pub fn modify_owners(req: &mut Request) -> CargoResult<Response> {
+pub fn add_owners(req: &mut Request) -> CargoResult<Response> {
+    modify_owners(req, true)
+}
+
+pub fn remove_owners(req: &mut Request) -> CargoResult<Response> {
+    modify_owners(req, false)
+}
+
+fn modify_owners(req: &mut Request, add: bool) -> CargoResult<Response> {
     let body = try!(req.body().read_to_string());
     let (user, krate) = try!(user_and_crate(req));
     let tx = try!(req.tx());
@@ -712,31 +720,21 @@ pub fn modify_owners(req: &mut Request) -> CargoResult<Response> {
         return Err(human("must already be an owner to modify owners"))
     }
 
-    #[deriving(Decodable)]
-    struct Request { add: Option<Vec<String>>, remove: Option<Vec<String>> }
-    let request: Request = try!(json::decode(body.as_slice()).map_err(|_| {
+    #[deriving(Decodable)] struct Request { users: Vec<String> }
+    let request: Request = try!(json::decode(body.as_slice()).map_err(|e| {
         human("invalid json request")
     }));
 
-    match request.add {
-        Some(to_add) => {
-            for to_add in to_add.iter() {
-                try!(krate.owner_add(tx, user.id, to_add.as_slice()));
-            }
-        }
-        None => {}
-    }
-    match request.remove {
-        Some(to_remove) => {
-            for to_remove in to_remove.iter() {
-                if to_remove.as_slice() == user.gh_login.as_slice() {
-                    req.rollback();
-                    return Err(human("cannot remove yourself as an owner"))
-                }
-                try!(krate.owner_remove(tx, user.id, to_remove.as_slice()));
+    for login in request.users.iter() {
+        if add {
+            try!(krate.owner_add(tx, user.id, login.as_slice()));
+        } else {
+            if login.as_slice() == user.gh_login.as_slice() {
+                req.rollback();
+                return Err(human("cannot remove yourself as an owner"))
             }
+            try!(krate.owner_remove(tx, user.id, login.as_slice()));
         }
-        None => {}
     }
 
     #[deriving(Encodable)]

src/lib.rs

@@ -88,7 +88,8 @@ pub fn middleware(app: Arc<App>) -> MiddlewareBuilder {
     router.put("/crates/:crate_id/unfollow", C(krate::unfollow));
     router.get("/crates/:crate_id/following", C(krate::following));
     router.get("/crates/:crate_id/owners", C(krate::owners));
-    router.put("/crates/:crate_id/owners", C(krate::modify_owners));
+    router.put("/crates/:crate_id/owners", C(krate::add_owners));
+    router.delete("/crates/:crate_id/owners", C(krate::remove_owners));
     router.get("/versions", C(version::index));
     router.get("/versions/:version_id", C(version::show));
     router.get("/git/index/*path", C(git::serve_index));

src/tests/krate.rs

@@ -287,7 +287,7 @@ fn new_crate_owner() {
     let mut response = t_resp!(middle.call(&mut req));
     ::json::<GoodCrate>(&mut response);
 
-    let body = r#"{"add":["foobar"]}"#;
+    let body = r#"{"users":["foobar"]}"#;
     let mut response = ok_resp!(middle.call(req.with_path("/crates/foo/owners")
                                                .with_method(conduit::Put)
                                                .with_body(body)));
@@ -541,15 +541,11 @@ fn owners() {
     let r: R = ::json(&mut response);
     assert_eq!(r.users.len(), 1);
 
-    let mut response = ok_resp!(middle.call(req.with_method(conduit::Put)
-                                               .with_body("{}")));
-    assert!(::json::<O>(&mut response).ok);
-
     let mut response = ok_resp!(middle.call(req.with_method(conduit::Get)));
     let r: R = ::json(&mut response);
     assert_eq!(r.users.len(), 1);
 
-    let body = r#"{"add":["foobar"]}"#;
+    let body = r#"{"users":["foobar"]}"#;
     let mut response = ok_resp!(middle.call(req.with_method(conduit::Put)
                                                .with_body(body)));
     assert!(::json::<O>(&mut response).ok);
@@ -558,17 +554,17 @@ fn owners() {
     let r: R = ::json(&mut response);
     assert_eq!(r.users.len(), 2);
 
-    let body = r#"{"remove":["foobar"]}"#;
-    let mut response = ok_resp!(middle.call(req.with_method(conduit::Put)
+    let body = r#"{"users":["foobar"]}"#;
+    let mut response = ok_resp!(middle.call(req.with_method(conduit::Delete)
                                                .with_body(body)));
     assert!(::json::<O>(&mut response).ok);
 
     let mut response = ok_resp!(middle.call(req.with_method(conduit::Get)));
     let r: R = ::json(&mut response);
     assert_eq!(r.users.len(), 1);
 
-    let body = r#"{"remove":["foo"]}"#;
-    let mut response = ok_resp!(middle.call(req.with_method(conduit::Put)
+    let body = r#"{"users":["foo"]}"#;
+    let mut response = ok_resp!(middle.call(req.with_method(conduit::Delete)
                                                .with_body(body)));
     ::json::<::Bad>(&mut response);
 }