test-results: Add revisions handler

go/src/infra/appengine/test-results/frontend/revision.go

Index: go/src/infra/appengine/test-results/frontend/revision.go
diff --git a/go/src/infra/appengine/test-results/frontend/revision.go b/go/src/infra/appengine/test-results/frontend/revision.go
new file mode 100644
index 0000000000000000000000000000000000000000..6397c5de7f28e9c7cdc9bd63cc08fa3302d2b59a
--- /dev/null
+++ b/go/src/infra/appengine/test-results/frontend/revision.go
@@ -0,0 +1,85 @@
+package frontend
+
+import (
+	"encoding/json"
+	"fmt"
+	"net/http"
+	"sync"
+
+	"golang.org/x/net/context"
+
+	"github.com/luci/gae/service/urlfetch"
+	"github.com/luci/luci-go/server/router"
+)
+
+// TODO(estaab): Comment copied from python implementation:
+//   Get rid of this one crrev.com supports this directly.
+//   See http://crbug.com/407198.
+
+func revisionHandler(c *router.Context) {
+	type result struct {
+		hash string
+		err  error
+	}
+	results := make([]result, 2)
+	wg := sync.WaitGroup{}
+	wg.Add(2)
+
+	go func() {
+		defer wg.Done()
+		hash, err := commitPositionToHash(c.Context, c.Request.FormValue("start"))
+		results[0] = result{hash, err}
+	}()
+	go func() {
+		defer wg.Done()
+		hash, err := commitPositionToHash(c.Context, c.Request.FormValue("end"))
+		results[1] = result{hash, err}
+	}()
+
+	wg.Wait()
+	for _, r := range results {
+		if r.err != nil {
+			http.Error(c.Writer, r.err.Error(), http.StatusInternalServerError)
+			return
+		}
+	}
+
+	http.Redirect(c.Writer, c.Request, fmt.Sprintf(
+		"https://chromium.googlesource.com/chromium/src/+log/%s^..%s?pretty=fuller",
+		results[0].hash,
+		results[1].hash,
+	), http.StatusMovedPermanently)
+}
+
+var (
+	// crRevURL is the URL used by commitPositionToHash.
+	// It is global to allow switching in tests.
+	crRevURL = "https://cr-rev.appspot.com/_ah/api/crrev/v1/redirect"
+
+	// makeCrRevClient returns a HTTP client for use by commitPositionToHash.
+	// It is global to allow switching in tests.
+	makeCrRevClient = func(c context.Context) *http.Client {
+		return &http.Client{Transport: urlfetch.Get(c)}
+	}
+)
+
+func commitPositionToHash(c context.Context, pos string) (string, error) {

[estaab, 2016/08/17 18:34:50]

How about having a separate function that takes a http.Client parameter and the
test uses it, and a wrapper that creates its own http.Client that regular code
calls? The global pattern isn't great.

[nishanths, 2016/08/17 18:44:08]

Much better!
Should I do the same for the crRevURL global also?

[estaab, 2016/08/17 18:47:42]

Is the test actually using it? I see it being set but I don't think it's used,
right? Maybe just make it const.

+	client := makeCrRevClient(c)
+	resp, err := client.Get(crRevURL + "/" + pos)
+	if err != nil {
+		return "", err
+	}
+	defer resp.Body.Close()
+
+	if resp.StatusCode != http.StatusOK {
+		return "", fmt.Errorf("HTTP status code %d from %s", resp.StatusCode, resp.Request.URL)
+	}
+
+	hash := struct {
+		Hash string `json:"git_sha"`
+	}{}
+	if err := json.NewDecoder(resp.Body).Decode(&hash); err != nil {
+		return "", err
+	}
+	return hash.Hash, nil
+}