Skip to content

Commit b34c614

Browse files
DanielRyanSmithDanielRyanSmith
committed
changes based on reviews
1 parent eaedf3c commit b34c614

File tree

4 files changed

+47
-48
lines changed

4 files changed

+47
-48
lines changed

‎api/query/query.go

+7-6
Original file line numberDiff line numberDiff line change
@@ -63,20 +63,21 @@ func (qh queryHandler) processInput(w http.ResponseWriter, r *http.Request) (*sh
6363
return &filters, testRuns, summaries, nil
6464
}
6565

66-
func (qh queryHandler) validateSummaryVersions(v url.Values) (bool, error) {
66+
func (qh queryHandler) validateSummaryVersions(v url.Values, logger shared.Logger) (bool, error) {
6767
filters, err := shared.ParseQueryFilterParams(v)
6868
if err != nil {
6969
return false, err
7070
}
71-
testRuns, filters, err := qh.getRunsAndFilters(filters)
71+
testRuns, _, err := qh.getRunsAndFilters(filters)
7272
if err != nil {
7373
return false, err
7474
}
7575

7676
for _, testRun := range testRuns {
7777
summaryURL := shared.GetResultsURL(testRun, "")
7878
// All new summary URLs end with "-summary_v2.json.gz".
79-
if !strings.HasSuffix(summaryURL, "-summary_v2.json.gz") {
79+
if !strings.HasSuffix(summaryURL, "-summary_v2.json.gz") && !strings.HasSuffix(summaryURL, "-summary.json.gz") {
80+
logger.Infof("summary URL has invalid suffix: %s", summaryURL)
8081
return false, nil
8182
}
8283
}
@@ -159,15 +160,15 @@ func (qh queryHandler) loadSummaries(testRuns shared.TestRuns) ([]summary, error
159160
}
160161

161162
func (qh queryHandler) loadSummary(testRun shared.TestRun) ([]byte, error) {
162-
mkey := getRedisKey(testRun)
163+
mkey := getSummaryFileRedisKey(testRun)
163164
url := shared.GetResultsURL(testRun, "")
164165
var data []byte
165166
err := qh.dataSource.Get(mkey, url, &data)
166167
return data, err
167168
}
168169

169-
func getRedisKey(testRun shared.TestRun) string {
170-
return "RESULTS_SUMMARY-v2-" + strconv.FormatInt(testRun.ID, 10)
170+
func getSummaryFileRedisKey(testRun shared.TestRun) string {
171+
return "RESULTS_SUMMARY_v2-" + strconv.FormatInt(testRun.ID, 10)
171172
}
172173

173174
func isRequestCacheable(r *http.Request) bool {

‎api/query/query_test.go

+7-7
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ import (
2020
)
2121

2222
func TestGetRedisKey(t *testing.T) {
23-
assert.Equal(t, "RESULTS_SUMMARY-v2-1", getRedisKey(shared.TestRun{
23+
assert.Equal(t, "RESULTS_SUMMARY_v2-1", getSummaryFileRedisKey(shared.TestRun{
2424
ID: 1,
2525
}))
2626
}
@@ -44,8 +44,8 @@ func TestLoadOldSummaries_success(t *testing.T) {
4444
},
4545
}
4646
keys := []string{
47-
getRedisKey(testRuns[0]),
48-
getRedisKey(testRuns[1]),
47+
getSummaryFileRedisKey(testRuns[0]),
48+
getSummaryFileRedisKey(testRuns[1]),
4949
}
5050

5151
cachedStore := sharedtest.NewMockCachedStore(mockCtrl)
@@ -100,8 +100,8 @@ func TestLoadNewSummaries_success(t *testing.T) {
100100
},
101101
}
102102
keys := []string{
103-
getRedisKey(testRuns[0]),
104-
getRedisKey(testRuns[1]),
103+
getSummaryFileRedisKey(testRuns[0]),
104+
getSummaryFileRedisKey(testRuns[1]),
105105
}
106106

107107
cachedStore := sharedtest.NewMockCachedStore(mockCtrl)
@@ -156,8 +156,8 @@ func TestLoadSummaries_fail(t *testing.T) {
156156
},
157157
}
158158
keys := []string{
159-
getRedisKey(testRuns[0]),
160-
getRedisKey(testRuns[1]),
159+
getSummaryFileRedisKey(testRuns[0]),
160+
getSummaryFileRedisKey(testRuns[1]),
161161
}
162162

163163
cachedStore := sharedtest.NewMockCachedStore(mockCtrl)

‎api/query/search.go

+32-35
Original file line numberDiff line numberDiff line change
@@ -89,34 +89,42 @@ func (sh structuredSearchHandler) ServeHTTP(w http.ResponseWriter, r *http.Reque
8989
ctx := sh.api.Context()
9090
logger := shared.GetLogger(ctx)
9191

92-
// Check if the query is a simple (empty/just True, or test name only) query
9392
var simpleQ TestNamePattern
93+
94+
r2 := r.Clone(r.Context())
95+
r2url := *r.URL
96+
r2.URL = &r2url
97+
r2.Method = "GET"
98+
q := r.URL.Query()
99+
q.Add("q", simpleQ.Pattern)
100+
// Assemble list of run IDs for later use.
101+
runIDStrs := make([]string, 0, len(rq.RunIDs))
102+
for _, id := range rq.RunIDs {
103+
runID := strconv.FormatInt(id, 10)
104+
q.Add("run_id", runID)
105+
runIDStrs = append(runIDStrs, strconv.FormatInt(id, 10))
106+
}
107+
runIDsStr := strings.Join(runIDStrs, ",")
108+
r2.URL.RawQuery = q.Encode()
109+
110+
// Check if the query is a simple (empty/just True, or test name only) query
94111
var isSimpleQ bool
95112
{
96113
if _, isTrueQ := rq.AbstractQuery.(True); isTrueQ {
97114
isSimpleQ = true
98115
} else if exists, isExists := rq.AbstractQuery.(AbstractExists); isExists && len(exists.Args) == 1 {
99116
simpleQ, isSimpleQ = exists.Args[0].(TestNamePattern)
100117
}
101-
q := r.URL.Query()
102118
for _, param := range []string{"interop", "subtests", "diff"} {
103119
val, _ := shared.ParseBooleanParam(q, param)
104120
isSimpleQ = isSimpleQ && (val == nil || !*val)
105121
}
106122

107-
q.Add("q", url.QueryEscape(simpleQ.Pattern))
108123
// Check old summary files. If any can't be found,
109124
// use the searchcache to aggregate the runs.
110-
for _, id := range rq.RunIDs {
111-
q.Add("run_id", strconv.FormatInt(id, 10))
112-
}
113-
114-
r2 := r.Clone(r.Context())
115-
r2.Method = "GET"
116-
r2.URL.RawQuery = q.Encode()
117-
summariesValid, err := sh.validateSummaryVersions(r2.URL.Query())
125+
summariesValid, err := sh.validateSummaryVersions(r2.URL.Query(), logger)
118126
if err != nil {
119-
logger.Debugf("Error checking version files: %v", err)
127+
logger.Debugf("Error checking summary file names: %v", err)
120128
}
121129
isSimpleQ = isSimpleQ && summariesValid
122130
}
@@ -136,35 +144,24 @@ func (sh structuredSearchHandler) ServeHTTP(w http.ResponseWriter, r *http.Reque
136144
}
137145
return
138146
}
139-
sh.handleSimpleQuery(w, r, simpleQ, rq)
140-
}
141-
142-
func (sh structuredSearchHandler) handleSimpleQuery(w http.ResponseWriter,
143-
r *http.Request, simpleQ TestNamePattern, rq RunQuery) {
144-
// Assemble list of run IDs for later use.
145-
runIDStrs := make([]string, 0, len(rq.RunIDs))
146-
for _, id := range rq.RunIDs {
147-
runIDStrs = append(runIDStrs, strconv.FormatInt(id, 10))
148-
}
149-
runIDsStr := strings.Join(runIDStrs, ",")
150147

148+
q = r.URL.Query()
149+
q.Set("q", simpleQ.Pattern)
150+
q.Set("run_ids", runIDsStr)
151+
r2.URL.RawQuery = q.Encode()
151152
// Structured query is equivalent to unstructured query.
152-
// Create an unstructured query request and delegate to unstructured query
153-
// handler.
154-
r2 := r.Clone(r.Context())
155-
r2url := *r.URL
156-
r2.URL = &r2url
157-
r2.Method = "GET"
158-
r2.URL.RawQuery = fmt.Sprintf("run_ids=%s&q=%s",
159-
url.QueryEscape(runIDsStr), url.QueryEscape(simpleQ.Pattern))
160-
153+
//delegate to unstructured query handler.
161154
unstructuredSearchHandler{queryHandler: sh.queryHandler}.ServeHTTP(w, r2)
162155
}
163156

164-
func (sh structuredSearchHandler) useSearchcache(w http.ResponseWriter, r *http.Request, data []byte, logger shared.Logger) (*http.Response, error) {
157+
func (sh structuredSearchHandler) useSearchcache(w http.ResponseWriter, r *http.Request,
158+
data []byte, logger shared.Logger) (*http.Response, error) {
165159
hostname := sh.api.GetServiceHostname("searchcache")
166-
// TODO: This will not work when hostname is localhost (http scheme needed).
167-
fwdURL, _ := url.Parse(fmt.Sprintf("https://%s/api/search/cache", hostname))
160+
// TODO(Issue #2941): This will not work when hostname is localhost (http scheme needed).
161+
fwdURL, err := url.Parse(fmt.Sprintf("https://%s/api/search/cache", hostname))
162+
if err != nil {
163+
logger.Debugf("Error parsing hostname.")
164+
}
168165
fwdURL.RawQuery = r.URL.RawQuery
169166

170167
logger.Infof("Forwarding structured search request to %s: %s", hostname, string(data))

‎webapp/components/test-file-results.js

+1
Original file line numberDiff line numberDiff line change
@@ -268,6 +268,7 @@ class TestFileResults extends WPTFlags(LoadingState(PathInfo(
268268
});
269269
}
270270

271+
// Slice summary file URL to infer the URL path to get single test data.
271272
resultsURL(testRun, path) {
272273
path = this.encodeTestPath(path);
273274
// This is relying on the assumption that result

0 commit comments

Comments
 (0)