Merge pull request #247 from endophage/coverage

minor refactor + tests for app.go just to improve test coverage.

docs/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{
+		accessRecords = appendAccessRecords(accessRecords, r.Method, repo)
-			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:   "*",
-				})
-		}
 	} 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
+}

docs/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")
+	}
+
+}