From 6dcec265a0646a4dff8d1440306683d18998cf5d Mon Sep 17 00:00:00 2001
From: David Lawrence <david.lawrence@docker.com>
Date: Mon, 9 Mar 2015 16:23:27 -0700
Subject: [PATCH] minor refactor + tests for app.go just to improve test
 coverage. Signed-off-by: David Lawrence <david.lawrence@docker.com> (github:
 endophage)

---
 registry/handlers/app.go      | 68 +++++++++++++++++++----------------
 registry/handlers/app_test.go | 68 +++++++++++++++++++++++++++++++++++
 2 files changed, 105 insertions(+), 31 deletions(-)

diff --git a/registry/handlers/app.go b/registry/handlers/app.go
index 12837cc8..4d860cc4 100644
--- a/registry/handlers/app.go
+++ b/registry/handlers/app.go
@@ -304,37 +304,7 @@ func (app *App) authorized(w http.ResponseWriter, r *http.Request, context *Cont
 	var accessRecords []auth.Access
 
 	if repo != "" {
-		resource := auth.Resource{
-			Type: "repository",
-			Name: repo,
-		}
-
-		switch r.Method {
-		case "GET", "HEAD":
-			accessRecords = append(accessRecords,
-				auth.Access{
-					Resource: resource,
-					Action:   "pull",
-				})
-		case "POST", "PUT", "PATCH":
-			accessRecords = append(accessRecords,
-				auth.Access{
-					Resource: resource,
-					Action:   "pull",
-				},
-				auth.Access{
-					Resource: resource,
-					Action:   "push",
-				})
-		case "DELETE":
-			// DELETE access requires full admin rights, which is represented
-			// as "*". This may not be ideal.
-			accessRecords = append(accessRecords,
-				auth.Access{
-					Resource: resource,
-					Action:   "*",
-				})
-		}
+		accessRecords = appendAccessRecords(accessRecords, r.Method, repo)
 	} else {
 		// Only allow the name not to be set on the base route.
 		if app.nameRequired(r) {
@@ -411,3 +381,39 @@ func apiBase(w http.ResponseWriter, r *http.Request) {
 
 	fmt.Fprint(w, emptyJSON)
 }
+
+// appendAccessRecords checks the method and adds the appropriate Access records to the records list.
+func appendAccessRecords(records []auth.Access, method string, repo string) []auth.Access {
+	resource := auth.Resource{
+		Type: "repository",
+		Name: repo,
+	}
+
+	switch method {
+	case "GET", "HEAD":
+		records = append(records,
+			auth.Access{
+				Resource: resource,
+				Action:   "pull",
+			})
+	case "POST", "PUT", "PATCH":
+		records = append(records,
+			auth.Access{
+				Resource: resource,
+				Action:   "pull",
+			},
+			auth.Access{
+				Resource: resource,
+				Action:   "push",
+			})
+	case "DELETE":
+		// DELETE access requires full admin rights, which is represented
+		// as "*". This may not be ideal.
+		records = append(records,
+			auth.Access{
+				Resource: resource,
+				Action:   "*",
+			})
+	}
+	return records
+}
diff --git a/registry/handlers/app_test.go b/registry/handlers/app_test.go
index ba580b11..80f92490 100644
--- a/registry/handlers/app_test.go
+++ b/registry/handlers/app_test.go
@@ -5,10 +5,12 @@ import (
 	"net/http"
 	"net/http/httptest"
 	"net/url"
+	"reflect"
 	"testing"
 
 	"github.com/docker/distribution/configuration"
 	"github.com/docker/distribution/registry/api/v2"
+	"github.com/docker/distribution/registry/auth"
 	_ "github.com/docker/distribution/registry/auth/silly"
 	"github.com/docker/distribution/registry/storage"
 	"github.com/docker/distribution/registry/storage/driver/inmemory"
@@ -200,3 +202,69 @@ func TestNewApp(t *testing.T) {
 		t.Fatalf("unexpected error code: %v != %v", errs.Errors[0].Code, v2.ErrorCodeUnauthorized)
 	}
 }
+
+// Test the access record accumulator
+func TestAppendAccessRecords(t *testing.T) {
+	repo := "testRepo"
+
+	expectedResource := auth.Resource{
+		Type: "repository",
+		Name: repo,
+	}
+
+	expectedPullRecord := auth.Access{
+		Resource: expectedResource,
+		Action:   "pull",
+	}
+	expectedPushRecord := auth.Access{
+		Resource: expectedResource,
+		Action:   "push",
+	}
+	expectedAllRecord := auth.Access{
+		Resource: expectedResource,
+		Action:   "*",
+	}
+
+	records := []auth.Access{}
+	result := appendAccessRecords(records, "GET", repo)
+	expectedResult := []auth.Access{expectedPullRecord}
+	if ok := reflect.DeepEqual(result, expectedResult); !ok {
+		t.Fatalf("Actual access record differs from expected")
+	}
+
+	records = []auth.Access{}
+	result = appendAccessRecords(records, "HEAD", repo)
+	expectedResult = []auth.Access{expectedPullRecord}
+	if ok := reflect.DeepEqual(result, expectedResult); !ok {
+		t.Fatalf("Actual access record differs from expected")
+	}
+
+	records = []auth.Access{}
+	result = appendAccessRecords(records, "POST", repo)
+	expectedResult = []auth.Access{expectedPullRecord, expectedPushRecord}
+	if ok := reflect.DeepEqual(result, expectedResult); !ok {
+		t.Fatalf("Actual access record differs from expected")
+	}
+
+	records = []auth.Access{}
+	result = appendAccessRecords(records, "PUT", repo)
+	expectedResult = []auth.Access{expectedPullRecord, expectedPushRecord}
+	if ok := reflect.DeepEqual(result, expectedResult); !ok {
+		t.Fatalf("Actual access record differs from expected")
+	}
+
+	records = []auth.Access{}
+	result = appendAccessRecords(records, "PATCH", repo)
+	expectedResult = []auth.Access{expectedPullRecord, expectedPushRecord}
+	if ok := reflect.DeepEqual(result, expectedResult); !ok {
+		t.Fatalf("Actual access record differs from expected")
+	}
+
+	records = []auth.Access{}
+	result = appendAccessRecords(records, "DELETE", repo)
+	expectedResult = []auth.Access{expectedAllRecord}
+	if ok := reflect.DeepEqual(result, expectedResult); !ok {
+		t.Fatalf("Actual access record differs from expected")
+	}
+
+}