dashboard: make response format consistent, implement commit GET mode

R=golang-dev, dsymonds, rsc
CC=golang-dev
https://golang.org/cl/5437113

diff --git a/misc/dashboard/app/build/build.go b/misc/dashboard/app/build/build.go
index 802d343ddcb9296471907f6fb5a4a7046b8b4fe4..459b4ee652d0b5a2882949a1874468e1cfa4bfac 100644 (file)
--- a/misc/dashboard/app/build/build.go
+++ b/misc/dashboard/app/build/build.go
@@ -193,29 +193,42 @@ func (t *Tag) Valid() os.Error {
        return nil
 }
 
-// commitHandler records a new commit. It reads a JSON-encoded Commit value
-// from the request body and creates a new Commit entity.
-// commitHandler also updates the "tip" Tag for each new commit at tip.
+// commitHandler retrieves commit data or records a new commit.
+//
+// For GET requests it returns a Commit value for the specified
+// packagePath and hash.
+//
+// For POST requests it reads a JSON-encoded Commit value from the request
+// body and creates a new Commit entity. It also updates the "tip" Tag for
+// each new commit at tip.
 //
 // This handler is used by a gobuilder process in -commit mode.
-func commitHandler(w http.ResponseWriter, r *http.Request) {
+func commitHandler(r *http.Request) (interface{}, os.Error) {
+       c := appengine.NewContext(r)
        com := new(Commit)
+
+       // TODO(adg): support unauthenticated GET requests to this handler
+       if r.Method == "GET" {
+               com.PackagePath = r.FormValue("packagePath")
+               com.Hash = r.FormValue("hash")
+               if err := datastore.Get(c, com.Key(c), com); err != nil {
+                       return nil, err
+               }
+               return com, nil
+       }
+
+       // POST request
        defer r.Body.Close()
        if err := json.NewDecoder(r.Body).Decode(com); err != nil {
-               logErr(w, r, err)
-               return
+               return nil, err
        }
        if err := com.Valid(); err != nil {
-               logErr(w, r, err)
-               return
+               return nil, err
        }
        tx := func(c appengine.Context) os.Error {
                return addCommit(c, com)
        }
-       c := appengine.NewContext(r)
-       if err := datastore.RunInTransaction(c, tx, nil); err != nil {
-               logErr(w, r, err)
-       }
+       return nil, datastore.RunInTransaction(c, tx, nil)
 }
 
 // addCommit adds the Commit entity to the datastore and updates the tip Tag.
@@ -266,25 +279,21 @@ func addCommit(c appengine.Context, com *Commit) os.Error {
 // request body and updates the Tag entity for the Kind of tag provided.
 //
 // This handler is used by a gobuilder process in -commit mode.
-func tagHandler(w http.ResponseWriter, r *http.Request) {
+func tagHandler(r *http.Request) (interface{}, os.Error) {
        t := new(Tag)
        defer r.Body.Close()
        if err := json.NewDecoder(r.Body).Decode(t); err != nil {
-               logErr(w, r, err)
-               return
+               return nil, err
        }
        if err := t.Valid(); err != nil {
-               logErr(w, r, err)
-               return
+               return nil, err
        }
        c := appengine.NewContext(r)
-       if _, err := datastore.Put(c, t.Key(c), t); err != nil {
-               logErr(w, r, err)
-               return
-       }
+       _, err := datastore.Put(c, t.Key(c), t)
+       return nil, err
 }
 
-// todoHandler returns the string of the hash of the next Commit to be built.
+// todoHandler returns the hash of the next Commit to be built.
 // It expects a "builder" query parameter.
 //
 // By default it scans the first 20 Go Commits in Num-descending order and
@@ -294,29 +303,28 @@ func tagHandler(w http.ResponseWriter, r *http.Request) {
 // and scans the first 20 Commits in Num-descending order for the specified
 // packagePath and returns the first that doesn't have a Result for this builder
 // and goHash combination.
-func todoHandler(w http.ResponseWriter, r *http.Request) {
+func todoHandler(r *http.Request) (interface{}, os.Error) {
        builder := r.FormValue("builder")
        goHash := r.FormValue("goHash")
 
        c := appengine.NewContext(r)
        p, err := GetPackage(c, r.FormValue("packagePath"))
        if err != nil {
-               logErr(w, r, err)
-               return
+               return nil, err
        }
 
-       q := datastore.NewQuery("Commit").
+       t := datastore.NewQuery("Commit").
                Ancestor(p.Key(c)).
                Limit(commitsPerPage).
-               Order("-Num")
-       var nextHash string
-       for t := q.Run(c); nextHash == ""; {
+               Order("-Num").
+               Run(c)
+       for {
                com := new(Commit)
-               if _, err := t.Next(com); err == datastore.Done {
-                       break
-               } else if err != nil {
-                       logErr(w, r, err)
-                       return
+               if _, err := t.Next(com); err != nil {
+                       if err == datastore.Done {
+                               err = nil
+                       }
+                       return nil, err
                }
                var hasResult bool
                if goHash != "" {
@@ -325,15 +333,15 @@ func todoHandler(w http.ResponseWriter, r *http.Request) {
                        hasResult = com.HasResult(builder)
                }
                if !hasResult {
-                       nextHash = com.Hash
+                       return com.Hash, nil
                }
        }
-       fmt.Fprint(w, nextHash)
+       panic("unreachable")
 }
 
-// packagesHandler returns a JSON-encoded list of the non-Go Packages
-// monitored by the dashboard.
-func packagesHandler(w http.ResponseWriter, r *http.Request) {
+// packagesHandler returns a list of the non-Go Packages monitored
+// by the dashboard.
+func packagesHandler(r *http.Request) (interface{}, os.Error) {
        c := appengine.NewContext(r)
        var pkgs []*Package
        for t := datastore.NewQuery("Package").Run(c); ; {
@@ -341,16 +349,13 @@ func packagesHandler(w http.ResponseWriter, r *http.Request) {
                if _, err := t.Next(pkg); err == datastore.Done {
                        break
                } else if err != nil {
-                       logErr(w, r, err)
-                       return
+                       return nil, err
                }
                if pkg.Path != "" {
                        pkgs = append(pkgs, pkg)
                }
        }
-       if err := json.NewEncoder(w).Encode(pkgs); err != nil {
-               logErr(w, r, err)
-       }
+       return pkgs, nil
 }
 
 // resultHandler records a build result.
@@ -358,24 +363,21 @@ func packagesHandler(w http.ResponseWriter, r *http.Request) {
 // creates a new Result entity, and updates the relevant Commit entity.
 // If the Log field is not empty, resultHandler creates a new Log entity
 // and updates the LogHash field before putting the Commit entity.
-func resultHandler(w http.ResponseWriter, r *http.Request) {
+func resultHandler(r *http.Request) (interface{}, os.Error) {
+       c := appengine.NewContext(r)
        res := new(Result)
        defer r.Body.Close()
        if err := json.NewDecoder(r.Body).Decode(res); err != nil {
-               logErr(w, r, err)
-               return
+               return nil, err
        }
        if err := res.Valid(); err != nil {
-               logErr(w, r, err)
-               return
+               return nil, err
        }
-       c := appengine.NewContext(r)
        // store the Log text if supplied
        if len(res.Log) > 0 {
                hash, err := PutLog(c, res.Log)
                if err != nil {
-                       logErr(w, r, err)
-                       return
+                       return nil, err
                }
                res.LogHash = hash
        }
@@ -392,9 +394,7 @@ func resultHandler(w http.ResponseWriter, r *http.Request) {
                com := &Commit{PackagePath: res.PackagePath, Hash: res.Hash}
                return com.AddResult(c, res)
        }
-       if err := datastore.RunInTransaction(c, tx, nil); err != nil {
-               logErr(w, r, err)
-       }
+       return nil, datastore.RunInTransaction(c, tx, nil)
 }
 
 func logHandler(w http.ResponseWriter, r *http.Request) {
@@ -416,9 +416,16 @@ func logHandler(w http.ResponseWriter, r *http.Request) {
        }
 }
 
+type dashHandler func(*http.Request) (interface{}, os.Error)
+
+type dashResponse struct {
+       Response interface{}
+       Error    os.Error
+}
+
 // AuthHandler wraps a http.HandlerFunc with a handler that validates the
 // supplied key and builder query parameters.
-func AuthHandler(h http.HandlerFunc) http.HandlerFunc {
+func AuthHandler(h dashHandler) http.HandlerFunc {
        return func(w http.ResponseWriter, r *http.Request) {
                // Put the URL Query values into r.Form to avoid parsing the
                // request body when calling r.FormValue.
@@ -435,7 +442,17 @@ func AuthHandler(h http.HandlerFunc) http.HandlerFunc {
                        }
                }
 
-               h(w, r) // Call the original HandlerFunc.
+               // Call the original HandlerFunc and return the response.
+               c := appengine.NewContext(r)
+               resp, err := h(r)
+               if err != nil {
+                       c.Errorf("%v", err)
+               }
+               w.Header().Set("Content-Type", "application/json")
+               err = json.NewEncoder(w).Encode(dashResponse{resp, err})
+               if err != nil {
+                       c.Criticalf("%v", err)
+               }
        }
 }
 

diff --git a/misc/dashboard/app/build/test.go b/misc/dashboard/app/build/test.go
index 10dd0c44fb5b0e5f78050c097ce555e51ae2106b..dacfa52226f9ce4abf66d940226846af89bd762b 100644 (file)
--- a/misc/dashboard/app/build/test.go
+++ b/misc/dashboard/app/build/test.go
@@ -16,6 +16,7 @@ import (
        "io"
        "json"
        "os"
+       "strings"
        "url"
 )
 
@@ -77,7 +78,7 @@ var testRequests = []struct {
        // logs
        {"/result", nil, &Result{Builder: "linux-386", Hash: "0003", OK: false, Log: []byte("test")}, nil},
        {"/log/a94a8fe5ccb19ba61c4c0873d391e987982fbbd3", nil, nil, "test"},
-       {"/todo", url.Values{"builder": {"linux-386"}}, nil, ""},
+       {"/todo", url.Values{"builder": {"linux-386"}}, nil, nil},
 
        // non-Go repos
        {"/commit", nil, &Commit{PackagePath: testPkg, Hash: "1001", ParentHash: "1000"}, nil},
@@ -89,7 +90,7 @@ var testRequests = []struct {
        {"/result", nil, &Result{PackagePath: testPkg, Builder: "linux-386", Hash: "1002", GoHash: "0001", OK: true}, nil},
        {"/todo", url.Values{"builder": {"linux-386"}, "packagePath": {testPkg}, "goHash": {"0001"}}, nil, "1001"},
        {"/result", nil, &Result{PackagePath: testPkg, Builder: "linux-386", Hash: "1001", GoHash: "0001", OK: true}, nil},
-       {"/todo", url.Values{"builder": {"linux-386"}, "packagePath": {testPkg}, "goHash": {"0001"}}, nil, ""},
+       {"/todo", url.Values{"builder": {"linux-386"}, "packagePath": {testPkg}, "goHash": {"0001"}}, nil, nil},
        {"/todo", url.Values{"builder": {"linux-386"}, "packagePath": {testPkg}, "goHash": {"0002"}}, nil, "1003"},
 }
 
@@ -111,13 +112,11 @@ func testHandler(w http.ResponseWriter, r *http.Request) {
                }
        }
 
-       failed := false
        for i, t := range testRequests {
                errorf := func(format string, args ...interface{}) {
                        fmt.Fprintf(w, "%d %s: ", i, t.path)
                        fmt.Fprintf(w, format, args...)
                        fmt.Fprintln(w)
-                       failed = true
                }
                var body io.ReadWriter
                if t.req != nil {
@@ -133,6 +132,9 @@ func testHandler(w http.ResponseWriter, r *http.Request) {
                        logErr(w, r, err)
                        return
                }
+               if t.req != nil {
+                       req.Method = "POST"
+               }
                req.Header = r.Header
                rec := httptest.NewRecorder()
                http.DefaultServeMux.ServeHTTP(rec, req)
@@ -140,17 +142,34 @@ func testHandler(w http.ResponseWriter, r *http.Request) {
                        errorf(rec.Body.String())
                        return
                }
+               resp := new(dashResponse)
+               if strings.HasPrefix(t.path, "/log/") {
+                       resp.Response = rec.Body.String()
+               } else {
+                       err := json.NewDecoder(rec.Body).Decode(resp)
+                       if err != nil {
+                               errorf("decoding response: %v", err)
+                               return
+                       }
+               }
                if e, ok := t.res.(string); ok {
-                       g := rec.Body.String()
+                       g, ok := resp.Response.(string)
+                       if !ok {
+                               errorf("Response not string: %T", resp.Response)
+                               return
+                       }
                        if g != e {
-                               errorf("body mismatch: got %q want %q", g, e)
+                               errorf("response mismatch: got %q want %q", g, e)
                                return
                        }
                }
+               if t.res == nil && resp.Response != nil {
+                       errorf("response mismatch: got %q expected <nil>",
+                               resp.Response)
+                       return
+               }
        }
-       if !failed {
-               fmt.Fprint(w, "PASS")
-       }
+       fmt.Fprint(w, "PASS")
 }
 
 func nukeEntities(c appengine.Context, kinds []string) os.Error {