Skip to content

Commit f726076

Browse files
DanielRyanSmithDanielRyanSmith
DanielRyanSmith
and
DanielRyanSmith
authored
Combine desktop and mobile for "Missing one" chart (#1461)
* combine desktop and mobile for missing-one chart * code cleanup * changes suggested by @jcscottiii --------- Co-authored-by: DanielRyanSmith <[email protected]>
1 parent 2597ade commit f726076

19 files changed

+787
-323
lines changed

‎backend/pkg/httpserver/list_aggregated_feature_support.go

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -57,10 +57,17 @@ func (s *Server) ListAggregatedFeatureSupport(
5757
var err error
5858
matchingMobileBrowser, err := getDesktopsMobileProduct(request.Browser)
5959
if err != nil {
60-
return backend.ListAggregatedFeatureSupport400JSONResponse{
61-
Code: 400,
60+
if errors.Is(err, ErrNoMatchingMobileBrowser) {
61+
return backend.ListAggregatedFeatureSupport400JSONResponse{
62+
Code: 400,
63+
Message: err.Error(),
64+
}, nil
65+
}
66+
67+
return backend.ListAggregatedFeatureSupport500JSONResponse{
68+
Code: 500,
6269
Message: err.Error(),
63-
}, err
70+
}, nil
6471
}
6572
targetMobileBrowser = (*string)(&matchingMobileBrowser)
6673
}

‎backend/pkg/httpserver/list_missing_one_implementation_counts.go

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -34,14 +34,28 @@ func (s *Server) ListMissingOneImplementationCounts(
3434
if found {
3535
return cachedResponse, nil
3636
}
37-
otherBrowsers := make([]string, len(request.Params.Browser))
38-
for i := 0; i < len(request.Params.Browser); i++ {
39-
otherBrowsers[i] = string(request.Params.Browser[i])
37+
38+
browserParams, err := PrepareMissingOneBrowserParams(
39+
request.Browser, request.Params.Browser, request.Params.IncludeBaselineMobileBrowsers != nil)
40+
if err != nil {
41+
if errors.Is(err, ErrNoMatchingMobileBrowser) {
42+
return backend.ListMissingOneImplementationCounts400JSONResponse{
43+
Code: 400,
44+
Message: err.Error(),
45+
}, nil
46+
}
47+
48+
return backend.ListMissingOneImplementationCounts500JSONResponse{
49+
Code: 500,
50+
Message: err.Error(),
51+
}, nil
4052
}
53+
4154
page, err := s.wptMetricsStorer.ListMissingOneImplCounts(
4255
ctx,
43-
string(request.Browser),
44-
otherBrowsers,
56+
browserParams.targetBrowser,
57+
browserParams.targetMobileBrowser,
58+
browserParams.otherBrowsers,
4559
request.Params.StartAt.Time,
4660
request.Params.EndAt.Time,
4761
getPageSizeOrDefault(request.Params.PageSize),

‎backend/pkg/httpserver/list_missing_one_implementation_counts_test.go

Lines changed: 61 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -38,14 +38,15 @@ func TestListMissingOneImplementationCounts(t *testing.T) {
3838
{
3939
name: "Success Case - no optional params - use defaults",
4040
mockConfig: &MockListMissingOneImplCountsConfig{
41-
expectedTargetBrowser: "chrome",
42-
expectedOtherBrowsers: []string{"edge", "firefox", "safari"},
43-
expectedStartAt: time.Date(2000, time.January, 1, 0, 0, 0, 0, time.UTC),
44-
expectedEndAt: time.Date(2000, time.January, 10, 0, 0, 0, 0, time.UTC),
45-
expectedPageSize: 100,
46-
expectedPageToken: nil,
47-
pageToken: nil,
48-
err: nil,
41+
expectedTargetBrowser: "chrome",
42+
expectedTargetMobileBrowser: nil,
43+
expectedOtherBrowsers: []string{"edge", "firefox", "safari"},
44+
expectedStartAt: time.Date(2000, time.January, 1, 0, 0, 0, 0, time.UTC),
45+
expectedEndAt: time.Date(2000, time.January, 10, 0, 0, 0, 0, time.UTC),
46+
expectedPageSize: 100,
47+
expectedPageToken: nil,
48+
pageToken: nil,
49+
err: nil,
4950
page: &backend.BrowserReleaseFeatureMetricsPage{
5051
Metadata: &backend.PageMetadata{
5152
NextPageToken: nil,
@@ -143,13 +144,14 @@ func TestListMissingOneImplementationCounts(t *testing.T) {
143144
{
144145
name: "Success Case - include optional params",
145146
mockConfig: &MockListMissingOneImplCountsConfig{
146-
expectedTargetBrowser: "chrome",
147-
expectedOtherBrowsers: []string{"edge", "firefox", "safari"},
148-
expectedStartAt: time.Date(2000, time.January, 1, 0, 0, 0, 0, time.UTC),
149-
expectedEndAt: time.Date(2000, time.January, 10, 0, 0, 0, 0, time.UTC),
150-
expectedPageSize: 50,
151-
expectedPageToken: inputPageToken,
152-
err: nil,
147+
expectedTargetBrowser: "chrome",
148+
expectedTargetMobileBrowser: valuePtr("chrome_android"),
149+
expectedOtherBrowsers: []string{"firefox", "firefox_android", "safari", "safari_ios"},
150+
expectedStartAt: time.Date(2000, time.January, 1, 0, 0, 0, 0, time.UTC),
151+
expectedEndAt: time.Date(2000, time.January, 10, 0, 0, 0, 0, time.UTC),
152+
expectedPageSize: 50,
153+
expectedPageToken: inputPageToken,
154+
err: nil,
153155
page: &backend.BrowserReleaseFeatureMetricsPage{
154156
Metadata: &backend.PageMetadata{
155157
NextPageToken: nextPageToken,
@@ -171,7 +173,7 @@ func TestListMissingOneImplementationCounts(t *testing.T) {
171173
{
172174
Key: `ListMissingOneImplementationCounts-{"browser":"chrome","Params":{"startAt":"2000-01-01",` +
173175
`"endAt":"2000-01-10","page_token":"input-token",` +
174-
`"page_size":50,"browser":["edge","firefox","safari"]}}`,
176+
`"page_size":50,"include_baseline_mobile_browsers":true,"browser":["firefox","safari"]}}`,
175177
Value: nil,
176178
Err: cachetypes.ErrCachedDataNotFound,
177179
},
@@ -180,7 +182,7 @@ func TestListMissingOneImplementationCounts(t *testing.T) {
180182
{
181183
Key: `ListMissingOneImplementationCounts-{"browser":"chrome","Params":{"startAt":"2000-01-01",` +
182184
`"endAt":"2000-01-10","page_token":"input-token",` +
183-
`"page_size":50,"browser":["edge","firefox","safari"]}}`,
185+
`"page_size":50,"include_baseline_mobile_browsers":true,"browser":["firefox","safari"]}}`,
184186
Value: []byte(
185187
`{"data":[{"count":10,"timestamp":"2000-01-10T00:00:00Z"},` +
186188
`{"count":9,"timestamp":"2000-01-09T00:00:00Z"}],` +
@@ -208,7 +210,7 @@ func TestListMissingOneImplementationCounts(t *testing.T) {
208210
}`),
209211
request: httptest.NewRequest(http.MethodGet,
210212
"/v1/stats/features/browsers/chrome/missing_one_implementation_counts?"+
211-
"browser=edge&browser=firefox&browser=safari&"+
213+
"browser=firefox&browser=safari&include_baseline_mobile_browsers=true&"+
212214
"startAt=2000-01-01&endAt=2000-01-10&page_size=50&page_token="+*inputPageToken, nil),
213215
},
214216
{
@@ -253,15 +255,16 @@ func TestListMissingOneImplementationCounts(t *testing.T) {
253255
{
254256
name: "500 case",
255257
mockConfig: &MockListMissingOneImplCountsConfig{
256-
expectedTargetBrowser: "chrome",
257-
expectedOtherBrowsers: []string{"edge", "firefox", "safari"},
258-
expectedStartAt: time.Date(2000, time.January, 1, 0, 0, 0, 0, time.UTC),
259-
expectedEndAt: time.Date(2000, time.January, 10, 0, 0, 0, 0, time.UTC),
260-
expectedPageSize: 100,
261-
expectedPageToken: nil,
262-
page: nil,
263-
pageToken: nil,
264-
err: errTest,
258+
expectedTargetBrowser: "chrome",
259+
expectedTargetMobileBrowser: nil,
260+
expectedOtherBrowsers: []string{"edge", "firefox", "safari"},
261+
expectedStartAt: time.Date(2000, time.January, 1, 0, 0, 0, 0, time.UTC),
262+
expectedEndAt: time.Date(2000, time.January, 10, 0, 0, 0, 0, time.UTC),
263+
expectedPageSize: 100,
264+
expectedPageToken: nil,
265+
page: nil,
266+
pageToken: nil,
267+
err: errTest,
265268
},
266269
expectedGetCalls: []*ExpectedGetCall{
267270
{
@@ -283,15 +286,16 @@ func TestListMissingOneImplementationCounts(t *testing.T) {
283286
{
284287
name: "400 case - invalid page token",
285288
mockConfig: &MockListMissingOneImplCountsConfig{
286-
expectedTargetBrowser: "chrome",
287-
expectedOtherBrowsers: []string{"edge", "firefox", "safari"},
288-
expectedStartAt: time.Date(2000, time.January, 1, 0, 0, 0, 0, time.UTC),
289-
expectedEndAt: time.Date(2000, time.January, 10, 0, 0, 0, 0, time.UTC),
290-
expectedPageSize: 100,
291-
expectedPageToken: badPageToken,
292-
page: nil,
293-
pageToken: nil,
294-
err: backendtypes.ErrInvalidPageToken,
289+
expectedTargetBrowser: "chrome",
290+
expectedTargetMobileBrowser: nil,
291+
expectedOtherBrowsers: []string{"edge", "firefox", "safari"},
292+
expectedStartAt: time.Date(2000, time.January, 1, 0, 0, 0, 0, time.UTC),
293+
expectedEndAt: time.Date(2000, time.January, 10, 0, 0, 0, 0, time.UTC),
294+
expectedPageSize: 100,
295+
expectedPageToken: badPageToken,
296+
page: nil,
297+
pageToken: nil,
298+
err: backendtypes.ErrInvalidPageToken,
295299
},
296300
expectedGetCalls: []*ExpectedGetCall{
297301
{
@@ -309,6 +313,27 @@ func TestListMissingOneImplementationCounts(t *testing.T) {
309313
"browser=edge&browser=firefox&browser=safari&"+
310314
"startAt=2000-01-01&endAt=2000-01-10&page_token"+*badPageToken, nil),
311315
},
316+
{
317+
name: "400 case - no matching mobile browser",
318+
mockConfig: nil,
319+
expectedGetCalls: []*ExpectedGetCall{
320+
{
321+
Key: `ListMissingOneImplementationCounts-{"browser":"edge","Params":{"startAt":"2000-01-01",` +
322+
`"endAt":"2000-01-10","include_baseline_mobile_browsers":true,` +
323+
`"browser":["chrome","firefox","safari"]}}`,
324+
Value: nil,
325+
Err: cachetypes.ErrCachedDataNotFound,
326+
},
327+
},
328+
expectedCacheCalls: nil,
329+
expectedCallCount: 0,
330+
expectedResponse: testJSONResponse(400,
331+
`{"code":400,"message":"browser does not have a matching mobile browser"}`),
332+
request: httptest.NewRequest(http.MethodGet,
333+
"/v1/stats/features/browsers/edge/missing_one_implementation_counts?"+
334+
"browser=chrome&browser=firefox&browser=safari&"+
335+
"startAt=2000-01-01&endAt=2000-01-10&include_baseline_mobile_browsers=true", nil),
336+
},
312337
}
313338

314339
for _, tc := range testCases {

‎backend/pkg/httpserver/list_missing_one_implementation_features.go

Lines changed: 83 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -23,20 +23,98 @@ import (
2323
"github.com/GoogleChrome/webstatus.dev/lib/gen/openapi/backend"
2424
)
2525

26+
type MissingOneBrowserParams struct {
27+
targetBrowser string
28+
targetMobileBrowser *string
29+
otherBrowsers []string
30+
}
31+
32+
// GetDesktopsMobileProduct returns the mobile version of the given desktop browser.
33+
func GetDesktopsMobileProduct(browser backend.BrowserPathParam) (backend.BrowserPathParam, error) {
34+
switch browser {
35+
case backend.Chrome:
36+
return backend.ChromeAndroid, nil
37+
case backend.Firefox:
38+
return backend.FirefoxAndroid, nil
39+
case backend.Safari:
40+
return backend.SafariIos, nil
41+
case backend.Edge, backend.ChromeAndroid, backend.FirefoxAndroid, backend.SafariIos:
42+
return backend.BrowserPathParam(""), ErrNoMatchingMobileBrowser
43+
}
44+
45+
return backend.BrowserPathParam(""), ErrNoMatchingMobileBrowser
46+
}
47+
48+
// PrepareMissingOneBrowserParams takes the raw request arguments for "missing in one browser" requests
49+
// and formats them.
50+
func PrepareMissingOneBrowserParams(
51+
targetBrowserParam backend.BrowserPathParam,
52+
otherBrowsersParam []backend.BrowserPathParam,
53+
includeMobileBrowsers bool,
54+
) (*MissingOneBrowserParams, error) {
55+
var otherBrowsers []string
56+
var targetMobileBrowser *string
57+
if includeMobileBrowsers {
58+
var err error
59+
matchingMobileBrowser, err := getDesktopsMobileProduct(targetBrowserParam)
60+
if err != nil {
61+
return nil, err
62+
}
63+
targetMobileBrowser = (*string)(&matchingMobileBrowser)
64+
65+
// Other browsers will include their mobile equivalents, so we'll need twice the size.
66+
otherBrowsers = make([]string, len(otherBrowsersParam)*2)
67+
var matchingMobileOtherBrowser backend.BrowserPathParam
68+
for i := range otherBrowsersParam {
69+
otherBrowsers[i*2] = string(otherBrowsersParam[i])
70+
matchingMobileOtherBrowser, err = getDesktopsMobileProduct(otherBrowsersParam[i])
71+
if err != nil {
72+
return nil, err
73+
}
74+
otherBrowsers[i*2+1] = string(matchingMobileOtherBrowser)
75+
}
76+
} else {
77+
otherBrowsers = make([]string, len(otherBrowsersParam))
78+
for i := range otherBrowsersParam {
79+
otherBrowsers[i] = string(otherBrowsersParam[i])
80+
}
81+
}
82+
83+
return &MissingOneBrowserParams{
84+
targetBrowser: string(targetBrowserParam),
85+
targetMobileBrowser: targetMobileBrowser,
86+
otherBrowsers: otherBrowsers,
87+
}, nil
88+
}
89+
2690
// ListMissingOneImplementationFeatures implements backend.StrictServerInterface.
2791
// nolint: ireturn // Signature generated from openapi
2892
func (s *Server) ListMissingOneImplementationFeatures(
2993
ctx context.Context,
3094
request backend.ListMissingOneImplementationFeaturesRequestObject) (
3195
backend.ListMissingOneImplementationFeaturesResponseObject, error) {
32-
otherBrowsers := make([]string, len(request.Params.Browser))
33-
for i := 0; i < len(request.Params.Browser); i++ {
34-
otherBrowsers[i] = string(request.Params.Browser[i])
96+
97+
browserParams, err := PrepareMissingOneBrowserParams(
98+
request.Browser, request.Params.Browser, request.Params.IncludeBaselineMobileBrowsers != nil)
99+
if err != nil {
100+
if errors.Is(err, ErrNoMatchingMobileBrowser) {
101+
return backend.ListMissingOneImplementationFeatures400JSONResponse{
102+
Code: 400,
103+
Message: err.Error(),
104+
}, nil
105+
}
106+
107+
return backend.ListMissingOneImplementationFeatures500JSONResponse{
108+
Code: 500,
109+
Message: err.Error(),
110+
}, nil
35111
}
112+
36113
page, err := s.wptMetricsStorer.ListMissingOneImplementationFeatures(
37114
ctx,
38-
string(request.Browser),
39-
otherBrowsers,
115+
browserParams.targetBrowser,
116+
browserParams.targetMobileBrowser,
117+
browserParams.otherBrowsers,
40118
request.Date.Time,
41119
getPageSizeOrDefault(request.Params.PageSize),
42120
request.Params.PageToken,

0 commit comments

Comments
 (0)