Skip to content

Commit ac2fbed

Browse files
KyleJuKyle Ju
KyleJu
and
Kyle Ju
authored
[Triage Metadata] Cache pending metadata in a triage (#2492)
* Cache pending metadata and PR number to Redis * Address comments Co-authored-by: Kyle Ju <[email protected]>
1 parent 7d03415 commit ac2fbed

File tree

2 files changed

+110
-11
lines changed

2 files changed

+110
-11
lines changed

‎api/metadata_handler.go‎

Lines changed: 28 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import (
1111
"io/ioutil"
1212
"net/http"
1313
"strings"
14+
"time"
1415

1516
"github.com/web-platform-tests/wpt.fyi/api/query"
1617
"github.com/web-platform-tests/wpt.fyi/shared"
@@ -75,10 +76,14 @@ func apiMetadataTriageHandler(w http.ResponseWriter, r *http.Request) {
7576
http.Error(w, "Unable to create GitHub OAuth client: "+err.Error(), http.StatusInternalServerError)
7677
return
7778
}
78-
handleMetadataTriage(ctx, gac, tm, w, r)
79+
80+
cacheSet := shared.NewRedisSet()
81+
// jsonCache writes pending metadata to Redis, with a 7-day TTL.
82+
jsonCache := shared.NewJSONObjectCache(ctx, shared.NewRedisReadWritable(ctx, 24*7*time.Hour))
83+
handleMetadataTriage(ctx, gac, tm, jsonCache, cacheSet, w, r)
7984
}
8085

81-
func handleMetadataTriage(ctx context.Context, gac shared.GitHubAccessControl, tm shared.TriageMetadata, w http.ResponseWriter, r *http.Request) {
86+
func handleMetadataTriage(ctx context.Context, gac shared.GitHubAccessControl, tm shared.TriageMetadata, jsonCache shared.ObjectCache, cacheSet shared.RedisSet, w http.ResponseWriter, r *http.Request) {
8287
if r.Method != "PATCH" {
8388
http.Error(w, "Invalid HTTP method; only accept PATCH", http.StatusBadRequest)
8489
return
@@ -121,13 +126,32 @@ func handleMetadataTriage(ctx context.Context, gac shared.GitHubAccessControl, t
121126
}
122127

123128
// TODO(kyleju): Check github client permission levels for auto merge.
124-
pr, err := tm.Triage(metadata)
129+
prURL, err := tm.Triage(metadata)
125130
if err != nil {
126131
http.Error(w, "Unable to triage metadata: "+err.Error(), http.StatusInternalServerError)
127132
return
128133
}
129134

130-
w.Write([]byte(pr))
135+
prArray := strings.Split(prURL, "/")
136+
if len(prArray) == 0 {
137+
http.Error(w, "Invalid PR URL format: "+prURL, http.StatusInternalServerError)
138+
return
139+
}
140+
141+
prNum := prArray[len(prArray)-1]
142+
// Stores the PR number and its pending metadata to Redis.
143+
err = cacheSet.Add(shared.PendingMetadataCacheKey, prNum)
144+
if err == nil {
145+
pendingMetadataKey := shared.PendingMetadataCachePrefix + prNum
146+
err = jsonCache.Put(pendingMetadataKey, metadata)
147+
}
148+
149+
if err != nil {
150+
logger := shared.GetLogger(ctx)
151+
logger.Errorf("Unable to cache %s to Redis: %s", prURL, err.Error())
152+
}
153+
154+
w.Write([]byte(prURL))
131155
}
132156

133157
func (h MetadataHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {

‎api/metadata_handler_test.go‎

Lines changed: 82 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -44,9 +44,84 @@ func TestHandleMetadataTriage_Success(t *testing.T) {
4444
mockgac.EXPECT().IsValidWPTMember().Return(true, nil)
4545

4646
mocktm := sharedtest.NewMockTriageMetadata(mockCtrl)
47-
mocktm.EXPECT().Triage(gomock.Any()).Return("", nil)
47+
mocktm.EXPECT().Triage(gomock.Any()).Return("https://github.com/web-platform-tests/wpt-metadata/pull/1", nil)
4848

49-
handleMetadataTriage(ctx, mockgac, mocktm, w, req)
49+
mockSet := sharedtest.NewMockRedisSet(mockCtrl)
50+
mockSet.EXPECT().Add(shared.PendingMetadataCacheKey, "1").Return(nil)
51+
52+
mockCache := sharedtest.NewMockObjectCache(mockCtrl)
53+
mockCache.EXPECT().Put(shared.PendingMetadataCachePrefix+"1", gomock.Any()).Return(nil)
54+
55+
handleMetadataTriage(ctx, mockgac, mocktm, mockCache, mockSet, w, req)
56+
57+
assert.Equal(t, http.StatusOK, w.Code)
58+
}
59+
60+
func TestHandleMetadataTriage_FailToCachePr(t *testing.T) {
61+
mockCtrl := gomock.NewController(t)
62+
defer mockCtrl.Finish()
63+
ctx := sharedtest.NewTestContext()
64+
w := httptest.NewRecorder()
65+
66+
body :=
67+
`{
68+
"/bar/foo.html": [
69+
{
70+
"product":"chrome",
71+
"url":"bugs.bar",
72+
"results":[{"status":6}]
73+
}
74+
]}`
75+
bodyReader := strings.NewReader(body)
76+
req := httptest.NewRequest("PATCH", "https://foo/metadata", bodyReader)
77+
req.Header.Set("Content-Type", "application/json")
78+
79+
mockgac := sharedtest.NewMockGitHubAccessControl(mockCtrl)
80+
mockgac.EXPECT().IsValidWPTMember().Return(true, nil)
81+
82+
mocktm := sharedtest.NewMockTriageMetadata(mockCtrl)
83+
mocktm.EXPECT().Triage(gomock.Any()).Return("https://github.com/web-platform-tests/wpt-metadata/pull/1", nil)
84+
85+
mockSet := sharedtest.NewMockRedisSet(mockCtrl)
86+
mockSet.EXPECT().Add(shared.PendingMetadataCacheKey, "1").Return(errors.New("Cache failed"))
87+
88+
handleMetadataTriage(ctx, mockgac, mocktm, nil, mockSet, w, req)
89+
90+
assert.Equal(t, http.StatusOK, w.Code)
91+
}
92+
93+
func TestHandleMetadataTriage_FailToCacheMetadata(t *testing.T) {
94+
mockCtrl := gomock.NewController(t)
95+
defer mockCtrl.Finish()
96+
ctx := sharedtest.NewTestContext()
97+
w := httptest.NewRecorder()
98+
99+
body :=
100+
`{
101+
"/bar/foo.html": [
102+
{
103+
"product":"chrome",
104+
"url":"bugs.bar",
105+
"results":[{"status":6}]
106+
}
107+
]}`
108+
bodyReader := strings.NewReader(body)
109+
req := httptest.NewRequest("PATCH", "https://foo/metadata", bodyReader)
110+
req.Header.Set("Content-Type", "application/json")
111+
112+
mockgac := sharedtest.NewMockGitHubAccessControl(mockCtrl)
113+
mockgac.EXPECT().IsValidWPTMember().Return(true, nil)
114+
115+
mocktm := sharedtest.NewMockTriageMetadata(mockCtrl)
116+
mocktm.EXPECT().Triage(gomock.Any()).Return("https://github.com/web-platform-tests/wpt-metadata/pull/1", nil)
117+
118+
mockSet := sharedtest.NewMockRedisSet(mockCtrl)
119+
mockSet.EXPECT().Add(shared.PendingMetadataCacheKey, "1").Return(nil)
120+
121+
mockCache := sharedtest.NewMockObjectCache(mockCtrl)
122+
mockCache.EXPECT().Put(shared.PendingMetadataCachePrefix+"1", gomock.Any()).Return(errors.New("Cache failed"))
123+
124+
handleMetadataTriage(ctx, mockgac, mocktm, mockCache, mockSet, w, req)
50125

51126
assert.Equal(t, http.StatusOK, w.Code)
52127
}
@@ -66,14 +141,14 @@ func TestHandleMetadataTriage_NonSimpleRequests(t *testing.T) {
66141
req := httptest.NewRequest("GET", "https://foo/metadata", bodyReader)
67142
req.Header.Set("Content-Type", "application/json")
68143

69-
handleMetadataTriage(nil, nil, nil, w, req)
144+
handleMetadataTriage(nil, nil, nil, nil, nil, w, req)
70145

71146
assert.Equal(t, http.StatusBadRequest, w.Code)
72147

73148
w = httptest.NewRecorder()
74149
req = httptest.NewRequest("POST", "https://foo/metadata", bodyReader)
75150

76-
handleMetadataTriage(nil, nil, nil, w, req)
151+
handleMetadataTriage(nil, nil, nil, nil, nil, w, req)
77152

78153
assert.Equal(t, http.StatusBadRequest, w.Code)
79154
}
@@ -93,7 +168,7 @@ func TestHandleMetadataTriage_WrongContentType(t *testing.T) {
93168
req := httptest.NewRequest("PATCH", "https://foo/metadata", bodyReader)
94169
req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
95170

96-
handleMetadataTriage(nil, nil, nil, w, req)
171+
handleMetadataTriage(nil, nil, nil, nil, nil, w, req)
97172

98173
assert.Equal(t, http.StatusBadRequest, w.Code)
99174
}
@@ -104,7 +179,7 @@ func TestHandleMetadataTriage_InvalidBody(t *testing.T) {
104179
req := httptest.NewRequest("PATCH", "https://foo/metadata", bodyReader)
105180
req.Header.Set("Content-Type", "application/json")
106181

107-
handleMetadataTriage(nil, nil, nil, w, req)
182+
handleMetadataTriage(nil, nil, nil, nil, nil, w, req)
108183

109184
assert.Equal(t, http.StatusBadRequest, w.Code)
110185
}
@@ -128,7 +203,7 @@ func TestHandleMetadataTriage_InvalidProduct(t *testing.T) {
128203
req := httptest.NewRequest("PATCH", "https://foo/metadata", bodyReader)
129204
req.Header.Set("Content-Type", "application/json")
130205

131-
handleMetadataTriage(ctx, nil, nil, w, req)
206+
handleMetadataTriage(ctx, nil, nil, nil, nil, w, req)
132207

133208
assert.Equal(t, http.StatusBadRequest, w.Code)
134209
}

0 commit comments

Comments
 (0)