Added HTTP endpoints for Correctness counts

golden/go/skiacorrectness/main.go

 package main
 
 import (
+        "encoding/json"
         "flag"
+        "fmt"
         "net/http"
-)
+        "time"
 
-import (
         "github.com/golang/glog"
+        "github.com/gorilla/mux"
+
+        // "skia.googlesource.com/buildbot.git/golden/go/analysis"

[jcgregorio, 2014/10/17 18:07:52]

leftover?

[stephana, 2014/10/17 20:01:45]

Yes. Thank you. Removed.

+        "skia.googlesource.com/buildbot.git/go/database"
+        "skia.googlesource.com/buildbot.git/golden/go/analysis"
+        "skia.googlesource.com/buildbot.git/golden/go/db"
+        "skia.googlesource.com/buildbot.git/golden/go/expstorage"
+        "skia.googlesource.com/buildbot.git/golden/go/filediffstore"
+        "skia.googlesource.com/buildbot.git/perf/go/filetilestore"
 )
 
 // flags
 var (
-        port      = flag.String("port", ":9000", "HTTP service address (e.g., ':9000')")
-        staticDir = flag.String("static", "./app", "Directory with static content to serve")
+        port         = flag.String("port", ":9000", "HTTP service address (e.g., ':9000')")
+        local        = flag.Bool("local", false, "Running locally if true. As opposed to in production.")
+        staticDir    = flag.String("static_dir", "./app", "Directory with static content to serve")
+        tileStoreDir = flag.String("tile_store_dir", "/tmp/tileStore", "What directory to look for tiles in.")
+        imageDiffDir = flag.String("image_diff_dir", "/tmp/imagediffdir", "What directory to store diff images in.")
+        gsBucketName = flag.String("gs_bucket", "chromium-skia-gm", "Name of the google storage bucket that holds uploaded images.")
+        mysqlConnStr = flag.String("mysql_conn", "", "MySQL connection string for backend database. If 'local' is false the password in this string will be substituted via the metadata server.")
+        sqlitePath   = flag.String("sqlite_path", "./golden.db", "Filepath of the embedded SQLite database. Requires 'local' to be set to true and 'mysql_conn' to be empty to take effect.")
+)
 
-// TODO (stephana): Just ideas to be sorted out later
-// tempDir   = flag.String("temp", "./.cache",
-// "Directory to store temporary file and application cache")
-)
+// Response envelope. All responses follow this format. Some fields might
+// be empty depending on context.
+type ResponseEnvelope struct {
+        Data   *interface{} `json:"data"`
+        Err    *string      `json:"err"`
+        Status int          `json:"status"`
+}
+
+type RestResources struct {
+        analizer *analysis.Analyzer
+}
+
+func NewRestResources(a *analysis.Analyzer) *RestResources {
+        return &RestResources{
+                analizer: a,
+        }
+}
+
+// Get the aggregated counts
+func (rr *RestResources) GetTileCountsHandler(w http.ResponseWriter, r *http.Request) {

[jcgregorio, 2014/10/17 18:07:52]

sendResponse looks useful, but I'm not sure about the role RestResources is
playing.

func tileCountsHandler(w http.ResponseWriter, r *http.Request) {
  result, err := analyzer.GetTileCounts()
  if err != nil {
    sendErrorResponse(w, err.Error(), http.StatusInternalServerError)
    return
  }
  sendResponse(w, result, http.StatusOK)
}

[stephana, 2014/10/17 20:01:45]

I added RestResources as a container for the variables that are used by the
various handlers. Right now it's only 'analyzer'. 
The alternative was to make it a global variable and set it in main.
I am not a big fan of that pattern, if you have any suggestions please let me
know.

[jcgregorio, 2014/10/17 20:12:16]

As for global, that's not too much of a concern since it will only be global
within the scope of the 'main' package, but I don't care either way. 

As for naming, I've seen similar pieces wrapped up as a Server or State struct
that contains all the state. 

+        result, err := rr.analizer.GetTileCounts()
+        if err != nil {
+                sendErrorResponse(w, err.Error(), http.StatusInternalServerError)
+                return
+        }
+
+        sendResponse(w, result, http.StatusOK)
+}
+
+// Process a diff request send via HTTP.
+func (rr *RestResources) GetTestCountsHandler(w http.ResponseWriter, r *http.Request) {
+        testName := mux.Vars(r)["testname"]
+        result, err := rr.analizer.GetTestCounts(testName)
+        if err != nil {
+                sendErrorResponse(w, err.Error(), http.StatusInternalServerError)
+                return
+        }
+
+        sendResponse(w, result, http.StatusOK)
+}
+
+// Send an error response with the given error message.
+func sendErrorResponse(w http.ResponseWriter, errorMsg string, status int) {
+        resp := ResponseEnvelope{nil, &errorMsg, status}
+        sendJson(w, &resp)
+}
+
+// Send a non-error response with the given data.
+func sendResponse(w http.ResponseWriter, data interface{}, status int) {
+        resp := ResponseEnvelope{&data, nil, status}
+        sendJson(w, &resp)
+}
+
+// Parse JSON input and validate it.
+// TODO (stephana): Validation is still missing. It's not sufficient
+// to just parse the JSON. Could be done with Json schemas.
+func parseJson(r *http.Request, v interface{}) error {

[jcgregorio, 2014/10/17 18:07:52]

Doesn't appear to be used?

[stephana, 2014/10/17 20:01:45]

Agreed that's a remainder from another version. Removed. 

+        // TODO: validate the JSON against a schema. Might not be necessary !
+        decoder := json.NewDecoder(r.Body)
+        return decoder.Decode(v)
+}
+
+// Sends the response envelope rendered as JSON.
+func sendJson(w http.ResponseWriter, resp *ResponseEnvelope) {
+        jsonBytes, err := json.Marshal(resp)
+        if err != nil {
+                http.Error(w, err.Error(), http.StatusInternalServerError)
+                return
+        }
+
+        w.Header().Set("Content-Type", "application/json")
+        w.Header().Set("Content-Length", fmt.Sprintf("%d", len(jsonBytes)))

[jcgregorio, 2014/10/17 18:07:52]

Content-Length isn't necessary.

[stephana, 2014/10/17 20:01:45]

Removed.

+        w.Write(jsonBytes)
+}
 
 func main() {
         // parse the arguments
         flag.Parse()
 
+        // Get the expecations storage, the filediff storage and the tilestore.
+        diffStore := filediffstore.NewFileDiffStore(nil, *imageDiffDir, *gsBucketName)
+        vdb := database.NewVersionedDB(db.GetDatabaseConfig(*mysqlConnStr, *sqlitePath, *local))
+        expStore := expstorage.NewSQLExpectationStore(vdb)
+        tileStore := filetilestore.NewFileTileStore(*tileStoreDir, "golden", -1)
+
+        // Create the analyer and plug it into the rest resources.
+        analyzer := analysis.NewAnalyzer(expStore, tileStore, diffStore, 5*time.Minute)
+        resources := NewRestResources(analyzer)
+
+        router := mux.NewRouter()
+
+        // Wire up the resources. We use the 'rest' prefix to avoid any name
+        // clashes witht the static files being served.
+        router.HandleFunc("/rest/tilecounts", resources.GetTileCountsHandler)

[jcgregorio, 2014/10/17 18:07:52]

Consider adding autogzip for JSON responses:

 http://godoc.org/github.com/fiorix/go-web/autogzip

[stephana, 2014/10/17 20:01:45]

I actually had that and removed it. I think we should not add gzip to the app
itself but defer it to the proxy in front of it.

Added a TODO to add it later. I like the bigger data sizes during development to
keep me honest about response sizes. 

+        router.HandleFunc("/rest/tilecounts/{testname}", resources.GetTestCountsHandler)
+
         // // Static file handling
-        http.Handle("/", http.FileServer(http.Dir(*staticDir)))
+        router.Handle("/", http.FileServer(http.Dir(*staticDir)))
 
-        // Wire up the resources
+        // Send all requests to the router
+        http.Handle("/", router)
 
         // Start the server
         glog.Infoln("Serving on http://127.0.0.1" + *port)
         glog.Fatal(http.ListenAndServe(*port, nil))
 }