playground: refactor handleVet and handleCompile methods

handleVet and handleCompile methods were almost indentical.
The only differences were the prefix for the cache key and
the function which executed command and made a response struct.

This changes introduces the commandHandler function that
returns an HTTP handler specified by the cache prefix and
command function (cmdFunc).

vetCheck and compileAndRun are those command functions.

Fixes golang/go#24535.

Change-Id: I4eaa5364cd4ee5f778c5b5b272b74b23e4caef7c
Reviewed-on: https://go-review.googlesource.com/112795
Reviewed-by: Andrew Bonventre <andybons@golang.org>
diff --git a/sandbox.go b/sandbox.go
index 84c4b25..ce020ec 100644
--- a/sandbox.go
+++ b/sandbox.go
@@ -50,42 +50,51 @@
 	Events []Event
 }
 
-func (s *server) handleCompile(w http.ResponseWriter, r *http.Request) {
-	var req request
-	// Until programs that depend on golang.org/x/tools/godoc/static/playground.js
-	// are updated to always send JSON, this check is in place.
-	if b := r.FormValue("body"); b != "" {
-		req.Body = b
-	} else if err := json.NewDecoder(r.Body).Decode(&req); err != nil {
-		http.Error(w, fmt.Sprintf("error decoding request: %v", err), http.StatusBadRequest)
-		return
-	}
-
-	resp := &response{}
-	key := cacheKey("prog", req.Body)
-	if err := s.cache.Get(key, resp); err != nil {
-		if err != memcache.ErrCacheMiss {
-			s.log.Errorf("s.cache.Get(%q, &response): %v", key, err)
-		}
-		var err error
-		resp, err = s.compileAndRun(&req)
-		if err != nil {
-			http.Error(w, err.Error(), http.StatusInternalServerError)
+// commandHandler returns an http.HandlerFunc.
+// This handler creates a *request, assigning the "Body" field a value
+// from the "body" form parameter or from the HTTP request body.
+// If there is no cached *response for the combination of cachePrefix and request.Body,
+// handler calls cmdFunc and in case of a nil error, stores the value of *response in the cache.
+func (s *server) commandHandler(cachePrefix string, cmdFunc func(*request) (*response, error)) http.HandlerFunc {
+	return func(w http.ResponseWriter, r *http.Request) {
+		var req request
+		// Until programs that depend on golang.org/x/tools/godoc/static/playground.js
+		// are updated to always send JSON, this check is in place.
+		if b := r.FormValue("body"); b != "" {
+			req.Body = b
+		} else if err := json.NewDecoder(r.Body).Decode(&req); err != nil {
+			s.log.Errorf("error decoding request: %v", err)
+			http.Error(w, http.StatusText(http.StatusBadRequest), http.StatusBadRequest)
 			return
 		}
-		if err := s.cache.Set(key, resp); err != nil {
-			s.log.Errorf("cache.Set(%q, resp): %v", key, err)
-		}
-	}
 
-	var buf bytes.Buffer
-	if err := json.NewEncoder(&buf).Encode(resp); err != nil {
-		http.Error(w, fmt.Sprintf("error encoding response: %v", err), http.StatusInternalServerError)
-		return
-	}
-	if _, err := io.Copy(w, &buf); err != nil {
-		s.log.Errorf("io.Copy(w, %+v): %v", buf, err)
-		return
+		resp := &response{}
+		key := cacheKey(cachePrefix, req.Body)
+		if err := s.cache.Get(key, resp); err != nil {
+			if err != memcache.ErrCacheMiss {
+				s.log.Errorf("s.cache.Get(%q, &response): %v", key, err)
+			}
+			resp, err = cmdFunc(&req)
+			if err != nil {
+				s.log.Errorf("cmdFunc error: %v", err)
+				http.Error(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError)
+				return
+			}
+			if err := s.cache.Set(key, resp); err != nil {
+				s.log.Errorf("cache.Set(%q, resp): %v", key, err)
+			}
+		}
+
+		var buf bytes.Buffer
+		if err := json.NewEncoder(&buf).Encode(resp); err != nil {
+			s.log.Errorf("error encoding response: %v", err)
+			http.Error(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError)
+			return
+		}
+		if _, err := io.Copy(w, &buf); err != nil {
+			s.log.Errorf("io.Copy(w, &buf): %v", err)
+			return
+		}
 	}
 }
 
@@ -245,7 +254,11 @@
 }
 `))
 
-func (s *server) compileAndRun(req *request) (*response, error) {
+// compileAndRun tries to build and run a user program.
+// The output of successfully ran program is returned in *response.Events.
+// If a program cannot be built or has timed out,
+// *response.Errors contains an explanation for a user.
+func compileAndRun(req *request) (*response, error) {
 	// TODO(andybons): Add semaphore to limit number of running programs at once.
 	tmpDir, err := ioutil.TempDir("", "sandbox")
 	if err != nil {
@@ -315,7 +328,7 @@
 }
 
 func (s *server) healthCheck() error {
-	resp, err := s.compileAndRun(&request{Body: healthProg})
+	resp, err := compileAndRun(&request{Body: healthProg})
 	if err != nil {
 		return err
 	}
@@ -341,7 +354,7 @@
 		stdlog.Fatal(err)
 	}
 	for _, t := range tests {
-		resp, err := s.compileAndRun(&request{Body: t.prog})
+		resp, err := compileAndRun(&request{Body: t.prog})
 		if err != nil {
 			stdlog.Fatal(err)
 		}
diff --git a/server.go b/server.go
index 7035d1d..d3a491b 100644
--- a/server.go
+++ b/server.go
@@ -50,9 +50,9 @@
 func (s *server) init() {
 	s.mux.HandleFunc("/", s.handleEdit)
 	s.mux.HandleFunc("/fmt", handleFmt)
-	s.mux.HandleFunc("/vet", s.handleVet)
+	s.mux.HandleFunc("/vet", s.commandHandler("vet", vetCheck))
+	s.mux.HandleFunc("/compile", s.commandHandler("prog", compileAndRun))
 	s.mux.HandleFunc("/share", s.handleShare)
-	s.mux.HandleFunc("/compile", s.handleCompile)
 	s.mux.HandleFunc("/playground.js", s.handlePlaygroundJS)
 	s.mux.HandleFunc("/favicon.ico", handleFavicon)
 	s.mux.HandleFunc("/_ah/health", s.handleHealthCheck)
diff --git a/server_test.go b/server_test.go
index 01891e3..a0f00eb 100644
--- a/server_test.go
+++ b/server_test.go
@@ -144,3 +144,70 @@
 		t.Errorf("got %q; want %q", got, want)
 	}
 }
+
+func TestCommandHandler(t *testing.T) {
+	s, err := newServer(func(s *server) error {
+		s.db = &inMemStore{}
+		// testLogger makes tests fail.
+		// Should we verify that s.log.Errorf was called
+		// instead of just printing or failing the test?
+		s.log = newStdLogger()
+		return nil
+	})
+	if err != nil {
+		t.Fatalf("newServer(testingOptions(t)): %v", err)
+	}
+	testHandler := s.commandHandler("test", func(r *request) (*response, error) {
+		if r.Body == "fail" {
+			return nil, fmt.Errorf("non recoverable")
+		}
+		if r.Body == "error" {
+			return &response{Errors: "errors"}, nil
+		}
+		resp := &response{Events: []Event{{r.Body, "stdout", 0}}}
+		return resp, nil
+	})
+
+	testCases := []struct {
+		desc       string
+		method     string
+		statusCode int
+		reqBody    []byte
+		respBody   []byte
+	}{
+		{"OPTIONS request", http.MethodOptions, http.StatusBadRequest, nil, nil},
+		{"GET request", http.MethodGet, http.StatusBadRequest, nil, nil},
+		{"Empty POST", http.MethodPost, http.StatusBadRequest, nil, nil},
+		{"Failed cmdFunc", http.MethodPost, http.StatusInternalServerError, []byte(`{"Body":"fail"}`), nil},
+		{"Standard flow", http.MethodPost, http.StatusOK,
+			[]byte(`{"Body":"ok"}`),
+			[]byte(`{"Errors":"","Events":[{"Message":"ok","Kind":"stdout","Delay":0}]}
+`),
+		},
+		{"Errors in response", http.MethodPost, http.StatusOK,
+			[]byte(`{"Body":"error"}`),
+			[]byte(`{"Errors":"errors","Events":null}
+`),
+		},
+	}
+
+	for _, tc := range testCases {
+		req := httptest.NewRequest(tc.method, "/compile", bytes.NewReader(tc.reqBody))
+		w := httptest.NewRecorder()
+		testHandler(w, req)
+		resp := w.Result()
+		if got, want := resp.StatusCode, tc.statusCode; got != want {
+			t.Errorf("%s: got unexpected status code %d; want %d", tc.desc, got, want)
+		}
+		if tc.respBody != nil {
+			defer resp.Body.Close()
+			b, err := ioutil.ReadAll(resp.Body)
+			if err != nil {
+				t.Errorf("%s: ioutil.ReadAll(resp.Body): %v", tc.desc, err)
+			}
+			if !bytes.Equal(b, tc.respBody) {
+				t.Errorf("%s: got unexpected body %q; want %q", tc.desc, b, tc.respBody)
+			}
+		}
+	}
+}
diff --git a/vet.go b/vet.go
index 1dfa215..314fd40 100644
--- a/vet.go
+++ b/vet.go
@@ -5,69 +5,18 @@
 package main
 
 import (
-	"bytes"
-	"encoding/json"
 	"fmt"
-	"io"
 	"io/ioutil"
-	"net/http"
 	"os"
 	"os/exec"
 	"path/filepath"
 	"strings"
-
-	"github.com/bradfitz/gomemcache/memcache"
 )
 
-// handleVet performs a vet check on source code, trying cache for results first.
-// It serves an empty response if no errors were found by the "vet" tool.
-func (s *server) handleVet(w http.ResponseWriter, r *http.Request) {
-	// TODO(ysmolsky): refactor common code in this function and handleCompile.
-	// See golang.org/issue/24535.
-	var req request
-	// Until programs that depend on golang.org/x/tools/godoc/static/playground.js
-	// are updated to always send JSON, this check is in place.
-	if b := r.FormValue("body"); b != "" {
-		req.Body = b
-	} else if err := json.NewDecoder(r.Body).Decode(&req); err != nil {
-		s.log.Errorf("error decoding request: %v", err)
-		http.Error(w, http.StatusText(http.StatusBadRequest), http.StatusBadRequest)
-		return
-	}
-
-	resp := &response{}
-	key := cacheKey("vet", req.Body)
-	if err := s.cache.Get(key, resp); err != nil {
-		if err != memcache.ErrCacheMiss {
-			s.log.Errorf("s.cache.Get(%q, &response): %v", key, err)
-		}
-		resp, err = s.vetCheck(&req)
-		if err != nil {
-			s.log.Errorf("error checking vet: %v", err)
-			http.Error(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError)
-			return
-		}
-		if err := s.cache.Set(key, resp); err != nil {
-			s.log.Errorf("cache.Set(%q, resp): %v", key, err)
-		}
-	}
-
-	var buf bytes.Buffer
-	if err := json.NewEncoder(&buf).Encode(resp); err != nil {
-		s.log.Errorf("error encoding response: %v", err)
-		http.Error(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError)
-		return
-	}
-	if _, err := io.Copy(w, &buf); err != nil {
-		s.log.Errorf("io.Copy(w, &buf): %v", err)
-		return
-	}
-}
-
 // vetCheck runs the "vet" tool on the source code in req.Body.
 // In case of no errors it returns an empty, non-nil *response.
 // Otherwise &response.Errors contains found errors.
-func (s *server) vetCheck(req *request) (*response, error) {
+func vetCheck(req *request) (*response, error) {
 	tmpDir, err := ioutil.TempDir("", "vet")
 	if err != nil {
 		return nil, fmt.Errorf("error creating temp directory: %v", err)