Added HTTP endpoints for Correctness counts

golden/go/skiacorrectness/main.go

Index: golden/go/skiacorrectness/main.go
diff --git a/golden/go/skiacorrectness/main.go b/golden/go/skiacorrectness/main.go
index 3b23b3161c47f6e0b40804b5695b035521126f64..bc37cc151dbe88cd9279c68250a36c054ecb3c30 100644
--- a/golden/go/skiacorrectness/main.go
+++ b/golden/go/skiacorrectness/main.go
@@ -1,32 +1,137 @@
 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")
-
-// TODO (stephana): Just ideas to be sorted out later
-// tempDir   = flag.String("temp", "./.cache",
-// "Directory to store temporary file and application cache")
+	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.")
 )
 
+// 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)