Skip to content

Commit

Permalink
Abstract GAE-compatible logging interface (#518)
Browse files Browse the repository at this point in the history
  • Loading branch information
mdittmer authored Sep 5, 2018
1 parent b0fe5f8 commit 082fb61
Show file tree
Hide file tree
Showing 22 changed files with 231 additions and 146 deletions.
5 changes: 2 additions & 3 deletions api/diff.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import (
"net/url"

"github.com/web-platform-tests/wpt.fyi/shared"
"google.golang.org/appengine"
)

// apiDiffHandler takes 2 test-run results JSON blobs and produces JSON in the same format, with only the differences
Expand All @@ -28,7 +27,7 @@ func apiDiffHandler(w http.ResponseWriter, r *http.Request) {
}

func handleAPIDiffGet(w http.ResponseWriter, r *http.Request) {
ctx := appengine.NewContext(r)
ctx := shared.NewAppEngineContext(r)

var err error
params, err := url.ParseQuery(r.URL.RawQuery)
Expand Down Expand Up @@ -83,7 +82,7 @@ func handleAPIDiffGet(w http.ResponseWriter, r *http.Request) {
// handleAPIDiffPost handles POST requests to /api/diff, which allows the caller to produce the diff of an arbitrary
// run result JSON blob against a historical production run.
func handleAPIDiffPost(w http.ResponseWriter, r *http.Request) {
ctx := appengine.NewContext(r)
ctx := shared.NewAppEngineContext(r)

var err error
params, err := url.ParseQuery(r.URL.RawQuery)
Expand Down
3 changes: 1 addition & 2 deletions api/interop.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,13 @@ import (

"github.com/web-platform-tests/results-analysis/metrics"
"github.com/web-platform-tests/wpt.fyi/shared"
"google.golang.org/appengine"
"google.golang.org/appengine/datastore"
)

// interopHandler handles the view of test results broken down by the
// number of browsers for which the test passes.
func apiInteropHandler(w http.ResponseWriter, r *http.Request) {
ctx := appengine.NewContext(r)
ctx := shared.NewAppEngineContext(r)
passRateType := metrics.GetDatastoreKindName(metrics.PassRateMetadata{})
query := datastore.NewQuery(passRateType).Order("-StartTime").Limit(1)

Expand Down
3 changes: 1 addition & 2 deletions api/manifest.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ import (

"github.com/deckarep/golang-set"
"github.com/web-platform-tests/wpt.fyi/shared"
"google.golang.org/appengine"
"google.golang.org/appengine/datastore"
"google.golang.org/appengine/memcache"
)
Expand All @@ -29,7 +28,7 @@ func apiManifestHandler(w http.ResponseWriter, r *http.Request) {
return
}
paths := shared.ParsePathsParam(r)
ctx := appengine.NewContext(r)
ctx := shared.NewAppEngineContext(r)
sha, manifestBytes, err := getManifestForSHA(ctx, sha)
if err != nil {
http.Error(w, err.Error(), http.StatusNotFound)
Expand Down
4 changes: 2 additions & 2 deletions api/query/autocomplete.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import (

mapset "github.com/deckarep/golang-set"
"github.com/web-platform-tests/wpt.fyi/shared"
"google.golang.org/appengine"
)

var (
Expand Down Expand Up @@ -62,10 +61,11 @@ type autocompleteHandler struct {
}

func apiAutocompleteHandler(w http.ResponseWriter, r *http.Request) {
ctx := appengine.NewContext(r)
ctx := shared.NewAppEngineContext(r)
sh := autocompleteHandler{queryHandler{
sharedImpl: defaultShared{ctx},
dataSource: cachedStore{
ctx: ctx,
cache: gzipReadWritable{memcacheReadWritable{ctx}},
store: httpReadable{ctx},
},
Expand Down
6 changes: 3 additions & 3 deletions api/query/autocomplete_medium_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ import (
"github.com/web-platform-tests/wpt.fyi/api/query/test"
"github.com/web-platform-tests/wpt.fyi/shared"
"github.com/web-platform-tests/wpt.fyi/shared/sharedtest"
"google.golang.org/appengine"
"google.golang.org/appengine/datastore"
)

Expand Down Expand Up @@ -50,7 +49,7 @@ func TestAutocompleteHandler(t *testing.T) {
{
req, err := i.NewRequest("GET", "/", nil)
assert.Nil(t, err)
ctx := appengine.NewContext(req)
ctx := shared.NewAppEngineContext(req)

for idx, testRun := range testRuns {
key, err := datastore.Put(ctx, datastore.NewIncompleteKey(ctx, "TestRun", nil), &testRun)
Expand Down Expand Up @@ -83,12 +82,13 @@ func TestAutocompleteHandler(t *testing.T) {
url.QueryEscape("2"))
r, err := i.NewRequest("GET", url, nil)
assert.Nil(t, err)
ctx := appengine.NewContext(r)
ctx := shared.NewAppEngineContext(r)
w := httptest.NewRecorder()

sh := autocompleteHandler{queryHandler{
sharedImpl: defaultShared{ctx},
dataSource: cachedStore{
ctx: ctx,
cache: memcacheReadWritable{ctx},
store: store,
},
Expand Down
19 changes: 10 additions & 9 deletions api/query/query.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import (
"fmt"
"io"
"io/ioutil"
"log"
"net/http"
"strconv"
"sync"
Expand Down Expand Up @@ -145,36 +144,38 @@ func (sharedImpl defaultShared) LoadTestRun(id int64) (*shared.TestRun, error) {
}

type cachedStore struct {
ctx context.Context
cache readWritable
store readable
}

func (cs cachedStore) Get(cacheID, storeID string) ([]byte, error) {
logger := cs.ctx.Value(shared.DefaultLoggerCtxKey()).(shared.Logger)
cr, err := cs.cache.NewReadCloser(cacheID)
if err == nil {
defer func() {
if err := cr.Close(); err != nil {
log.Printf("WARNING: Error closing cache reader for key %s: %v", cacheID, err)
logger.Warningf("Error closing cache reader for key %s: %v", cacheID, err)
}
}()
cached, err := ioutil.ReadAll(cr)
if err == nil {
log.Printf("INFO: Serving summary from cache: %s", cacheID)
logger.Infof("Serving summary from cache: %s", cacheID)
return cached, nil
}
}

log.Printf("WARNING: Error fetching cache key %s: %v", cacheID, err)
logger.Warningf("Error fetching cache key %s: %v", cacheID, err)
err = nil

log.Printf("INFO: Loading summary from store: %s", storeID)
logger.Infof("Loading summary from store: %s", storeID)
sr, err := cs.store.NewReadCloser(storeID)
if err != nil {
return nil, err
}
defer func() {
if err := sr.Close(); err != nil {
log.Printf("WARNING: Error closing store reader for key %s: %v", storeID, err)
logger.Warningf("Error closing store reader for key %s: %v", storeID, err)
}
}()

Expand All @@ -187,16 +188,16 @@ func (cs cachedStore) Get(cacheID, storeID string) ([]byte, error) {
go func() {
w, err := cs.cache.NewWriteCloser(cacheID)
if err != nil {
log.Printf("WARNING: Error cache writer for key %s: %v", cacheID, err)
logger.Warningf("Error cache writer for key %s: %v", cacheID, err)
return
}
defer func() {
if err := w.Close(); err != nil {
log.Printf("WARNING: Error cache writer for key %s: %v", cacheID, err)
logger.Warningf("Error cache writer for key %s: %v", cacheID, err)
}
}()
if _, err := w.Write(data); err != nil {
log.Printf("WARNING: Failed to write to cache key %s: %v", cacheID, err)
logger.Warningf("Failed to write to cache key %s: %v", cacheID, err)
return
}
}()
Expand Down
13 changes: 11 additions & 2 deletions api/query/query_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ func TestLoadSummary_cacheMiss(t *testing.T) {
store := NewMockreadable(mockCtrl)
sh := searchHandler{queryHandler{
dataSource: cachedStore{
ctx: shared.NewTestContext(),
cache: cache,
store: store,
},
Expand Down Expand Up @@ -77,7 +78,10 @@ func TestLoadSummary_cacheHit(t *testing.T) {

cache := NewMockreadWritable(mockCtrl)
sh := searchHandler{queryHandler{
dataSource: cachedStore{cache: cache},
dataSource: cachedStore{
ctx: shared.NewTestContext(),
cache: cache,
},
}}
smry := []byte("{}")
r := test.NewMockReadCloser(t, smry)
Expand Down Expand Up @@ -108,6 +112,7 @@ func TestLoadSummary_missing(t *testing.T) {
store := NewMockreadable(mockCtrl)
sh := searchHandler{queryHandler{
dataSource: cachedStore{
ctx: shared.NewTestContext(),
cache: cache,
store: store,
},
Expand Down Expand Up @@ -150,7 +155,10 @@ func TestLoadSummaries_success(t *testing.T) {

cache := NewMockreadWritable(mockCtrl)
sh := searchHandler{queryHandler{
dataSource: cachedStore{cache: cache},
dataSource: cachedStore{
ctx: shared.NewTestContext(),
cache: cache,
},
}}
summaryBytes := [][]byte{
[]byte(`{"/a/b/c":[1,2]}`),
Expand Down Expand Up @@ -203,6 +211,7 @@ func TestLoadSummaries_fail(t *testing.T) {
store := NewMockreadable(mockCtrl)
sh := searchHandler{queryHandler{
dataSource: cachedStore{
ctx: shared.NewTestContext(),
cache: cache,
store: store,
},
Expand Down
4 changes: 2 additions & 2 deletions api/query/search.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import (
"strings"

"github.com/web-platform-tests/wpt.fyi/shared"
"google.golang.org/appengine"
)

// LegacySearchRunResult is the results data from legacy test summarys. These
Expand Down Expand Up @@ -61,10 +60,11 @@ type searchHandler struct {

func apiSearchHandler(w http.ResponseWriter, r *http.Request) {
// Parse query params.
ctx := appengine.NewContext(r)
ctx := shared.NewAppEngineContext(r)
sh := searchHandler{queryHandler{
sharedImpl: defaultShared{ctx},
dataSource: cachedStore{
ctx: ctx,
cache: gzipReadWritable{memcacheReadWritable{ctx}},
store: httpReadable{ctx},
},
Expand Down
6 changes: 3 additions & 3 deletions api/query/search_medium_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ import (
"github.com/web-platform-tests/wpt.fyi/api/query/test"
"github.com/web-platform-tests/wpt.fyi/shared"
"github.com/web-platform-tests/wpt.fyi/shared/sharedtest"
"google.golang.org/appengine"
"google.golang.org/appengine/datastore"
)

Expand Down Expand Up @@ -50,7 +49,7 @@ func TestSearchHandler(t *testing.T) {
{
req, err := i.NewRequest("GET", "/", nil)
assert.Nil(t, err)
ctx := appengine.NewContext(req)
ctx := shared.NewAppEngineContext(req)

for idx, testRun := range testRuns {
key, err := datastore.Put(ctx, datastore.NewIncompleteKey(ctx, "TestRun", nil), &testRun)
Expand Down Expand Up @@ -83,12 +82,13 @@ func TestSearchHandler(t *testing.T) {
url.QueryEscape(q))
r, err := i.NewRequest("GET", url, nil)
assert.Nil(t, err)
ctx := appengine.NewContext(r)
ctx := shared.NewAppEngineContext(r)
w := httptest.NewRecorder()

sh := searchHandler{queryHandler{
sharedImpl: defaultShared{ctx},
dataSource: cachedStore{
ctx: ctx,
cache: memcacheReadWritable{ctx},
store: store,
},
Expand Down
7 changes: 3 additions & 4 deletions api/results_receive_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,8 @@ package api
import (
"net/http"

"google.golang.org/appengine"

"github.com/web-platform-tests/wpt.fyi/api/receiver"
"github.com/web-platform-tests/wpt.fyi/shared"
)

func apiResultsUploadHandler(w http.ResponseWriter, r *http.Request) {
Expand All @@ -18,7 +17,7 @@ func apiResultsUploadHandler(w http.ResponseWriter, r *http.Request) {
return
}

ctx := appengine.NewContext(r)
ctx := shared.NewAppEngineContext(r)
a := receiver.NewAppEngineAPI(ctx)
receiver.HandleResultsUpload(a, w, r)
}
Expand All @@ -29,7 +28,7 @@ func apiResultsCreateHandler(w http.ResponseWriter, r *http.Request) {
return
}

ctx := appengine.NewContext(r)
ctx := shared.NewAppEngineContext(r)
a := receiver.NewAppEngineAPI(ctx)
receiver.HandleResultsCreate(a, w, r)
}
3 changes: 1 addition & 2 deletions api/results_redirect_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import (
"strings"

"github.com/web-platform-tests/wpt.fyi/shared"
"google.golang.org/appengine"
)

// apiResultsRedirectHandler is responsible for redirecting to the Google Cloud Storage API
Expand All @@ -31,7 +30,7 @@ func apiResultsRedirectHandler(w http.ResponseWriter, r *http.Request) {
return
}

ctx := appengine.NewContext(r)
ctx := shared.NewAppEngineContext(r)
one := 1
testRuns, err := shared.LoadTestRuns(ctx, filters.Products, filters.Labels, []string{filters.SHA}, nil, nil, &one)
if err != nil {
Expand Down
3 changes: 1 addition & 2 deletions api/shas.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import (

mapset "github.com/deckarep/golang-set"
"github.com/web-platform-tests/wpt.fyi/shared"
"google.golang.org/appengine"
)

// apiSHAsHandler is responsible for emitting just the revision SHAs for test runs.
Expand All @@ -21,7 +20,7 @@ func apiSHAsHandler(w http.ResponseWriter, r *http.Request) {
return
}

ctx := appengine.NewContext(r)
ctx := shared.NewAppEngineContext(r)

var shas []string
products := filters.GetProductsOrDefault()
Expand Down
3 changes: 1 addition & 2 deletions api/shas_medium_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import (
"github.com/stretchr/testify/assert"
"github.com/web-platform-tests/wpt.fyi/shared"
"github.com/web-platform-tests/wpt.fyi/shared/sharedtest"
"google.golang.org/appengine"
"google.golang.org/appengine/datastore"
)

Expand All @@ -22,7 +21,7 @@ func TestApiSHAsHandler(t *testing.T) {
defer i.Close()
r, err := i.NewRequest("GET", "/api/shas", nil)
assert.Nil(t, err)
ctx := appengine.NewContext(r)
ctx := shared.NewAppEngineContext(r)

// No results - empty JSON array, 404
var shas []string
Expand Down
3 changes: 1 addition & 2 deletions api/test_run.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import (
"github.com/gorilla/mux"

"github.com/web-platform-tests/wpt.fyi/shared"
"google.golang.org/appengine"
"google.golang.org/appengine/datastore"
)

Expand All @@ -26,7 +25,7 @@ func apiTestRunHandler(w http.ResponseWriter, r *http.Request) {

vars := mux.Vars(r)
idParam := vars["id"]
ctx := appengine.NewContext(r)
ctx := shared.NewAppEngineContext(r)
var testRun shared.TestRun
if idParam != "" {
id, err := strconv.ParseInt(idParam, 10, 0)
Expand Down
3 changes: 1 addition & 2 deletions api/test_run_medium_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import (
"github.com/stretchr/testify/assert"
"github.com/web-platform-tests/wpt.fyi/shared"
"github.com/web-platform-tests/wpt.fyi/shared/sharedtest"
"google.golang.org/appengine"
"google.golang.org/appengine/datastore"
)

Expand All @@ -25,7 +24,7 @@ func TestGetTestRunByID(t *testing.T) {
r = mux.SetURLVars(r, map[string]string{"id": "123"})
assert.Nil(t, err)

ctx := appengine.NewContext(r)
ctx := shared.NewAppEngineContext(r)
resp := httptest.NewRecorder()
apiTestRunHandler(resp, r)
assert.Equal(t, http.StatusNotFound, resp.Code)
Expand Down
Loading

0 comments on commit 082fb61

Please sign in to comment.