From 6c165d7ac461d86c6ce5c69c09ae170eaf1608dc Mon Sep 17 00:00:00 2001 From: Andrew Gerrand Date: Fri, 2 Dec 2011 16:05:12 +1100 Subject: [PATCH] dashboard: make response format consistent, implement commit GET mode R=golang-dev, dsymonds, rsc CC=golang-dev https://golang.org/cl/5437113 --- misc/dashboard/app/build/build.go | 131 +++++++++++++++++------------- misc/dashboard/app/build/test.go | 41 +++++++--- 2 files changed, 104 insertions(+), 68 deletions(-) diff --git a/misc/dashboard/app/build/build.go b/misc/dashboard/app/build/build.go index 802d343ddcb..459b4ee652d 100644 --- 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 10dd0c44fb5..dacfa52226f 100644 --- 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 } - if e, ok := t.res.(string); ok { - g := rec.Body.String() - if g != e { - errorf("body mismatch: got %q want %q", g, e) + 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, ok := resp.Response.(string) + if !ok { + errorf("Response not string: %T", resp.Response) + return + } + if 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 ", + resp.Response) + return + } } - if !failed { - fmt.Fprint(w, "PASS") - } + fmt.Fprint(w, "PASS") } func nukeEntities(c appengine.Context, kinds []string) os.Error {