[ARVADOS] updated: 52192560846cf801b8e9e29cb06060a16a1d154d

[git at public.curoverse.com]

Summary of changes:
 services/keep/src/keep/handler_test.go | 18 +++++--------
 services/keep/src/keep/handlers.go     | 46 ++++++++++++++++------------------
 2 files changed, 27 insertions(+), 37 deletions(-)

       via  52192560846cf801b8e9e29cb06060a16a1d154d (commit)
      from  fe5556e0f3105f2d8b752d796fea21363a1b0eb6 (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.


commit 52192560846cf801b8e9e29cb06060a16a1d154d
Author: Tim Pierce <twp at curoverse.com>
Date:   Fri Aug 1 21:19:21 2014 -0400

    2769: more code review comments
    
    * Abandon HTTP 405, return HTTP 200 whenever any blocks were found at
      all
    
    * Return a JSON message only with HTTP 200 (404 implies
      copies_deleted=0, copies_failed=0)
    
    * More clarity in unit tests
    
    Refs #2769

diff --git a/services/keep/src/keep/handler_test.go b/services/keep/src/keep/handler_test.go
index 030e333..df8a0ba 100644
--- a/services/keep/src/keep/handler_test.go
+++ b/services/keep/src/keep/handler_test.go
@@ -500,20 +500,13 @@ func TestDeleteHandler(t *testing.T) {
 		Deleted int `json:"copies_deleted"`
 		Failed  int `json:"copies_failed"`
 	}
-	var dc, expected_dc deletecounter
+	var response_dc, expected_dc deletecounter
 
 	response = IssueRequest(rest, superuser_nonexistent_block_req)
 	ExpectStatusCode(t,
 		"data manager request, nonexistent block",
 		http.StatusNotFound,
 		response)
-	// Expect response {"copies_deleted":0,"copies_failed":0}
-	expected_dc = deletecounter{0, 0}
-	json.NewDecoder(response.Body).Decode(&dc)
-	if dc != expected_dc {
-		t.Errorf("superuser_nonexistent_block_req\nexpected: %+v\nreceived: %+v",
-			expected_dc, dc)
-	}
 
 	// Authenticated admin request for existing block.
 	response = IssueRequest(rest, superuser_existing_block_req)
@@ -523,14 +516,15 @@ func TestDeleteHandler(t *testing.T) {
 		response)
 	// Expect response {"copies_deleted":1,"copies_failed":0}
 	expected_dc = deletecounter{1, 0}
-	json.NewDecoder(response.Body).Decode(&dc)
-	if dc != expected_dc {
+	json.NewDecoder(response.Body).Decode(&response_dc)
+	if response_dc != expected_dc {
 		t.Errorf("superuser_existing_block_req\nexpected: %+v\nreceived: %+v",
-			expected_dc, dc)
+			expected_dc, response_dc)
 	}
 	// Confirm the block has been deleted
 	_, err := vols[0].Get(TEST_HASH)
-	if !os.IsNotExist(err) {
+	var block_deleted = os.IsNotExist(err)
+	if !block_deleted {
 		t.Error("superuser_existing_block_req: block not deleted")
 	}
 }
diff --git a/services/keep/src/keep/handlers.go b/services/keep/src/keep/handlers.go
index d5a2a35..dc0f517 100644
--- a/services/keep/src/keep/handlers.go
+++ b/services/keep/src/keep/handlers.go
@@ -328,27 +328,21 @@ func GetVolumeStatus(volume string) *VolumeStatus {
 //
 // Upon receiving a valid request from an authorized user,
 // DeleteHandler deletes all copies of the specified block on local
-// volumes.
+// writable volumes.
 //
 // Response format:
 //
-// The response body consists of the JSON message
+// If the requested blocks was not found on any volume, the response
+// code is HTTP 404 Not Found.
+//
+// Otherwise, the response code is 200 OK, with a response body
+// consisting of the JSON message
 //
 //    {"copies_deleted":d,"copies_failed":f}
 //
 // where d and f are integers representing the number of blocks that
 // were successfully and unsuccessfully deleted.
 //
-//   * If any blocks were successfully deleted (copies_deleted > 0), the
-//     HTTP response code is 200 OK.
-//
-//   * If no blocks were found at all (copies_deleted == copies_failed
-//     == 0), the response code is 404 Not Found.
-//
-//   * If blocks were found but none could be deleted (copies_deleted
-//     == 0 and copies_failed > 0), the response code is 405 Method Not
-//     Allowed.
-//
 func DeleteHandler(resp http.ResponseWriter, req *http.Request) {
 	hash := mux.Vars(req)["hash"]
 	log.Printf("%s %s", req.Method, hash)
@@ -378,21 +372,23 @@ func DeleteHandler(resp http.ResponseWriter, req *http.Request) {
 		}
 	}
 
-	if j, err := json.Marshal(result); err == nil {
-		if result.Deleted == 0 && result.Failed == 0 {
-			// If no blocks were found, HTTP 404
-			resp.WriteHeader(http.StatusNotFound)
-		} else if result.Deleted == 0 && result.Failed > 0 {
-			// If all delete attempts failed, HTTP 405
-			resp.WriteHeader(http.StatusMethodNotAllowed)
+	var st int
+
+	if result.Deleted == 0 && result.Failed == 0 {
+		st = http.StatusNotFound
+	} else {
+		st = http.StatusOK
+	}
+
+	resp.WriteHeader(st)
+
+	if st == http.StatusOK {
+		if body, err := json.Marshal(result); err == nil {
+			resp.Write(body)
 		} else {
-			resp.WriteHeader(http.StatusOK)
+			log.Printf("json.Marshal: %s (result = %v)\n", err, result)
+			http.Error(resp, err.Error(), 500)
 		}
-		resp.Write(j)
-	} else {
-		log.Printf("json.Marshal: %s\n", err)
-		log.Printf("result = %v\n", result)
-		http.Error(resp, err.Error(), 500)
 	}
 }
 

-----------------------------------------------------------------------


hooks/post-receive
--