Rewrite API to use wrappers for hstspreload calls, and implement api.…

…TestStatusSubmitPendingUpdate().

This also necessitated rewriting a lot of other mock code.

api/api.go

@@ -10,9 +10,21 @@ import (
 	"github.com/chromium/hstspreload.appspot.com/database"
 )
 
-// API holds the server API.
+// API holds the server API. Use api.New() to construct.
 type API struct {
-	Database database.Database
+	database        database.Database
+	hstspreload     hstspreloadWrapper
+	chromiumpreload chromiumpreloadWrapper
+}
+
+// New creates a new API struct with the given database and the proper
+// unexported fields.
+func New(db database.Database) API {
+	return API{
+		database:        db,
+		hstspreload:     actualHstspreload{},
+		chromiumpreload: actualChromiumpreload{},
+	}
 }
 
 // writeJSONOrBust should only be called if nothing has been written yet.
@@ -32,7 +44,7 @@ func writeJSONOrBust(w http.ResponseWriter, v interface{}) {
 // CheckConnection tests if we can connect the database.
 func (api API) CheckConnection() error {
 	// Make sure we can connect to the datastore by forcing a fetch.
-	_, err := api.Database.StateForDomain("garron.net")
+	_, err := api.database.StateForDomain("garron.net")
 	if err != nil {
 		if strings.Contains(err.Error(), "missing project/dataset id") {
 			fmt.Fprintf(os.Stderr, "Try running: make serve\n")

api/api_test.go

@@ -7,48 +7,166 @@ import (
 	"net/http/httptest"
 	"testing"
 
+	"github.com/chromium/hstspreload"
 	"github.com/chromium/hstspreload.appspot.com/database"
+	"github.com/chromium/hstspreload/chromiumpreload"
 )
 
+var emptyIssues = hstspreload.Issues{}
+
+var issuesWithWarnings = hstspreload.Issues{
+	Warnings: []hstspreload.Issue{{"code", "warning", "message"}},
+}
+
+var issuesWithErrors = hstspreload.Issues{
+	Errors: []hstspreload.Issue{
+		{"code1", "warning1", "message1"},
+		{"code2", "warning2", "message2"},
+	},
+	Warnings: []hstspreload.Issue{{"code", "warning", "message"}},
+}
+
+func mockAPI() (api API, ms *database.MockState, h *mockHstspreload, c *mockChromiumpreload) {
+	db, ms := database.NewMock()
+	h = &mockHstspreload{}
+	c = &mockChromiumpreload{}
+	api = API{
+		database:        db,
+		hstspreload:     h,
+		chromiumpreload: c,
+	}
+	return api, ms, h, c
+}
+
 func TestCheckConnection(t *testing.T) {
-	m, ms := database.NewMock()
-	a := API{m}
-	if err := a.CheckConnection(); err != nil {
+	api, ms, _, _ := mockAPI()
+
+	if err := api.CheckConnection(); err != nil {
 		t.Errorf("%s", err)
 	}
 
 	ms.FailCalls = true
-	if err := a.CheckConnection(); err == nil {
+	if err := api.CheckConnection(); err == nil {
 		t.Error("connection should fail")
 	}
 }
 
-func TestStatus(t *testing.T) {
-	m, _ := database.NewMock()
-	a := API{m}
-
-	w := httptest.NewRecorder()
+// Any non-zero values are considered wanted.
+type wantBody struct {
+	text   string
+	state  *database.DomainState
+	issues *hstspreload.Issues
+}
 
-	r, err := http.NewRequest("GET", "?domain=garron.net", nil)
-	if err != nil {
-		t.Fatal(err)
-	}
+type apiTestCase struct {
+	description string
+	handlerFunc http.HandlerFunc
+	method      string
+	url         string
+	wantCode    int
+	wantBody    wantBody
+}
 
-	b := &bytes.Buffer{}
-	w.Body = b
+func TestStatusSubmitPendingUpdate(t *testing.T) {
+	api, _, h, c := mockAPI()
 
-	a.Status(w, r)
+	h.preloadableResponses = make(map[string]hstspreload.Issues)
+	h.preloadableResponses["garron.net"] = emptyIssues
+	h.preloadableResponses["badssl.com"] = issuesWithWarnings
+	h.preloadableResponses["example.com"] = issuesWithErrors
 
-	s := database.DomainState{}
-	if err := json.Unmarshal(w.Body.Bytes(), &s); err != nil {
-		t.Fatal(err)
+	c.list.Entries = []chromiumpreload.PreloadEntry{
+		{"garron.net", chromiumpreload.ForceHTTPS, true},
+		{"chromium.org", chromiumpreload.ForceHTTPS, false},
+		{"godoc.og", "", true},
 	}
 
-	if s.Name != "garron.net" {
-		t.Errorf("Wrong name: %s", s.Name)
+	wantStatus := func(description string, domain string, status database.PreloadStatus) apiTestCase {
+		return apiTestCase{description, api.Status, "GET", "?domain=" + domain,
+			200, wantBody{state: &database.DomainState{
+				Name:   domain,
+				Status: status,
+			}},
+		}
 	}
-	if s.Status != database.StatusUnknown {
-		t.Errorf("Wrong status: %s", s.Status)
+
+	apiTestSequence := []apiTestCase{
+		// wrong HTTP method
+		{"status wrong method", api.Status, "POST", "?domain=garron.net",
+			405, wantBody{text: "Wrong method. Requires GET.\n"}},
+		{"pending wrong method", api.Pending, "POST", "",
+			405, wantBody{text: "Wrong method. Requires GET.\n"}},
+		{"submit wrong method", api.Submit, "GET", "?domain=garron.net",
+			405, wantBody{text: "Wrong method. Requires POST.\n"}},
+
+		// initial
+		wantStatus("garron.net initial", "garron.net", database.StatusUnknown),
+		wantStatus("example.com initial", "example.com", database.StatusUnknown),
+		{"pending 1", api.Pending, "GET", "",
+			200, wantBody{text: "[\n]\n"}},
+
+		// submit
+		{"bad submit", api.Submit, "POST", "?domain=example.com",
+			200, wantBody{issues: &issuesWithErrors}},
+		{"good submit", api.Submit, "POST", "?domain=garron.net",
+			200, wantBody{issues: &emptyIssues}},
+		{"pending 2", api.Pending, "GET", "",
+			200, wantBody{text: "[\n    { \"name\": \"garron.net\", \"include_subdomains\": true, \"mode\": \"force-https\" }\n]\n"}},
+
+		// update
+		wantStatus("garron.net pending", "garron.net", database.StatusPending),
+		{"update", api.Update, "GET", "",
+			200, wantBody{text: "The preload list has 3 entries.\n- # of preloaded HSTS entries: 2\n- # to be added in this update: 2\n- # to be removed this update: 0\nSuccess. 2 domain states updated.\n"}},
+		{"pending 3", api.Pending, "GET", "",
+			200, wantBody{text: "[\n]\n"}},
+
+		// after update
+		wantStatus("example.com after update", "example.com", database.StatusUnknown),
+		wantStatus("garron.net after update", "garron.net", database.StatusPreloaded),
+		wantStatus("chromium.org after update", "chromium.org", database.StatusPreloaded),
+		wantStatus("godoc.org after update", "godoc.org", database.StatusUnknown),
 	}
 
+	for _, tt := range apiTestSequence {
+		w := httptest.NewRecorder()
+		w.Body = &bytes.Buffer{}
+
+		r, err := http.NewRequest(tt.method, tt.url, nil)
+		if err != nil {
+			t.Fatalf("[%s] %s", tt.description, err)
+		}
+
+		tt.handlerFunc(w, r)
+
+		if w.Code != tt.wantCode {
+			t.Errorf("[%s] Status code does not match wanted: %d", tt.description, w.Code)
+		}
+
+		if tt.wantBody.text != "" {
+			text := w.Body.String()
+			if text != tt.wantBody.text {
+				t.Errorf("[%s] Body text does not match wanted: %#v", tt.description, text)
+			}
+		}
+
+		if tt.wantBody.state != nil {
+			var s database.DomainState
+			if err := json.Unmarshal(w.Body.Bytes(), &s); err != nil {
+				t.Fatalf("[%s] %s", tt.description, err)
+			}
+			if !s.MatchesWanted(*tt.wantBody.state) {
+				t.Errorf("[%s] Domain state does not match wanted: %#v", tt.description, s)
+			}
+		}
+
+		if tt.wantBody.issues != nil {
+			var iss hstspreload.Issues
+			if err := json.Unmarshal(w.Body.Bytes(), &iss); err != nil {
+				t.Fatalf("[%s] %s", tt.description, err)
+			}
+			if !iss.Match(*tt.wantBody.issues) {
+				t.Errorf("[%s] Issues do not match wanted: %#v", tt.description, iss)
+			}
+		}
+	}
 }

api/domain_handlers.go

@@ -42,7 +42,7 @@ func (api API) Preloadable(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	_, issues := hstspreload.PreloadableDomain(domain)
+	_, issues := api.hstspreload.PreloadableDomain(domain)
 	writeJSONOrBust(w, issues)
 }
 
@@ -55,7 +55,7 @@ func (api API) Removable(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	_, issues := hstspreload.RemovableDomain(domain)
+	_, issues := api.hstspreload.RemovableDomain(domain)
 	writeJSONOrBust(w, issues)
 }
 
@@ -68,7 +68,7 @@ func (api API) Status(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	state, err := api.Database.StateForDomain(domain)
+	state, err := api.database.StateForDomain(domain)
 	if err != nil {
 		msg := fmt.Sprintf("Internal error: could not retrieve status. (%s)\n", err)
 		http.Error(w, msg, http.StatusInternalServerError)
@@ -92,13 +92,13 @@ func (api API) Submit(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	_, issues := hstspreload.PreloadableDomain(domain)
+	_, issues := api.hstspreload.PreloadableDomain(domain)
 	if len(issues.Errors) > 0 {
 		writeJSONOrBust(w, issues)
 		return
 	}
 
-	state, stateErr := api.Database.StateForDomain(domain)
+	state, stateErr := api.database.StateForDomain(domain)
 	if stateErr != nil {
 		msg := fmt.Sprintf("Internal error: could not get current domain status. (%s)\n", stateErr)
 		http.Error(w, msg, http.StatusInternalServerError)
@@ -110,7 +110,7 @@ func (api API) Submit(w http.ResponseWriter, r *http.Request) {
 	case database.StatusRejected:
 		fallthrough
 	case database.StatusRemoved:
-		putErr := api.Database.PutState(database.DomainState{
+		putErr := api.database.PutState(database.DomainState{
 			Name:           domain,
 			Status:         database.StatusPending,
 			SubmissionDate: time.Now(),

api/pending.go

@@ -16,7 +16,7 @@ func (api API) Pending(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	names, err := api.Database.DomainsWithStatus(database.StatusPending)
+	names, err := api.database.DomainsWithStatus(database.StatusPending)
 	if err != nil {
 		msg := fmt.Sprintf("Internal error: not convert domain to ASCII. (%s)\n", err)
 		http.Error(w, msg, http.StatusInternalServerError)

api/update.go

@@ -31,7 +31,7 @@ func (api API) Update(w http.ResponseWriter, r *http.Request) {
 	// In order to allow visiting the URL directly in the browser, we allow any method.
 
 	// Get preload list.
-	preloadList, listErr := chromiumpreload.GetLatest()
+	preloadList, listErr := api.chromiumpreload.GetLatest()
 	if listErr != nil {
 		msg := fmt.Sprintf(
 			"Internal error: could not retrieve latest preload list. (%s)\n",
@@ -48,7 +48,7 @@ func (api API) Update(w http.ResponseWriter, r *http.Request) {
 	}
 
 	// Get domains currently recorded as preloaded.
-	databasePreload, dbErr := api.Database.DomainsWithStatus(database.StatusPreloaded)
+	databasePreload, dbErr := api.database.DomainsWithStatus(database.StatusPreloaded)
 	if dbErr != nil {
 		msg := fmt.Sprintf(
 			"Internal error: could not retrieve domain names previously marked as preloaded. (%s)\n",
@@ -102,7 +102,7 @@ func (api API) Update(w http.ResponseWriter, r *http.Request) {
 	}
 
 	// Update the database
-	putErr := api.Database.PutStates(updates, logf)
+	putErr := api.database.PutStates(updates, logf)
 	if putErr != nil {
 		msg := fmt.Sprintf(
 			"Internal error: datastore update failed. (%s)\n",

api/wrappers.go

@@ -0,0 +1,52 @@
+package api
+
+import (
+	"github.com/chromium/hstspreload"
+	"github.com/chromium/hstspreload/chromiumpreload"
+)
+
+type hstspreloadWrapper interface {
+	PreloadableDomain(string) (*string, hstspreload.Issues)
+	RemovableDomain(string) (*string, hstspreload.Issues)
+}
+
+type chromiumpreloadWrapper interface {
+	GetLatest() (chromiumpreload.PreloadList, error)
+}
+
+/******** actual ********/
+
+type actualHstspreload struct{}
+type actualChromiumpreload struct{}
+
+func (h actualHstspreload) PreloadableDomain(domain string) (*string, hstspreload.Issues) {
+	return h.PreloadableDomain(domain)
+}
+func (h actualHstspreload) RemovableDomain(domain string) (*string, hstspreload.Issues) {
+	return h.RemovableDomain(domain)
+}
+func (c actualChromiumpreload) GetLatest() (chromiumpreload.PreloadList, error) {
+	return c.GetLatest()
+}
+
+/******** mock ********/
+
+type mockHstspreload struct {
+	// The mock will return verdicts based on these maps.
+	// Remember that you must `make` a map before adding values:  https://blog.golang.org/go-maps-in-action#TOC_2.
+	preloadableResponses map[string]hstspreload.Issues
+	removableResponses   map[string]hstspreload.Issues
+}
+type mockChromiumpreload struct {
+	list chromiumpreload.PreloadList
+}
+
+func (h mockHstspreload) PreloadableDomain(domain string) (*string, hstspreload.Issues) {
+	return nil, h.preloadableResponses[domain]
+}
+func (h mockHstspreload) RemovableDomain(domain string) (*string, hstspreload.Issues) {
+	return nil, h.removableResponses[domain]
+}
+func (c mockChromiumpreload) GetLatest() (chromiumpreload.PreloadList, error) {
+	return c.list, nil
+}