Skip to content

Commit 6e6f6ba

Browse files
jcscottiiijcscottiii
authored
Add HTTP layer logic to handle bookmark operations (#1358)
This change adds the HTTP layer logic for the [putUserSavedSearchBookmark](https://github.com/GoogleChrome/webstatus.dev/blob/80e7bccea843ad8d02dd9a662c89143d650c06a8/openapi/backend/openapi.yaml#L656) and [removeUserSavedSearchBookmark](https://github.com/GoogleChrome/webstatus.dev/blob/80e7bccea843ad8d02dd9a662c89143d650c06a8/openapi/backend/openapi.yaml#L696) operations. Other changes: I started to get duplicate code lint errors for these two operations. So I: - Increased the threshold by a little bit. - Refactored some of the duplicate user checking into a common CheckAuthenticatedUser method - Reduced some duplicate test boilerplate code - Introduced a common basicHTTPTestCase struct that we can probably use for most cases.
1 parent 8021816 commit 6e6f6ba

12 files changed

+512
-65
lines changed

‎.golangci.yaml

+3
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,9 @@ linters:
5454
settings:
5555
gomoddirectives:
5656
replace-local: true
57+
dupl:
58+
# The default is 150
59+
threshold: 175
5760
exclusions:
5861
generated: lax
5962
presets:

‎backend/cmd/server/main.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -147,7 +147,7 @@ func main() {
147147
//nolint: exhaustruct // No need to use every option of 3rd party struct.
148148
cors.Options{
149149
AllowedOrigins: []string{allowedOrigin, "http://*"},
150-
AllowedMethods: []string{"GET", "OPTIONS", "PATCH", "DELETE"},
150+
AllowedMethods: []string{"GET", "OPTIONS", "PATCH", "DELETE", "PUT"},
151151
AllowedHeaders: []string{"Authorization"},
152152
AllowCredentials: true, // Remove after UbP
153153
MaxAge: 300, // Maximum value not ignored by any of major browsers

‎backend/pkg/httpserver/middlewares.go

+38
Original file line numberDiff line numberDiff line change
@@ -16,13 +16,51 @@ package httpserver
1616

1717
import (
1818
"context"
19+
"log/slog"
1920
"net/http"
2021

22+
"github.com/GoogleChrome/webstatus.dev/lib/auth"
2123
"github.com/GoogleChrome/webstatus.dev/lib/gen/openapi/backend"
2224
"github.com/GoogleChrome/webstatus.dev/lib/httpmiddlewares"
2325
"github.com/oapi-codegen/runtime/strictmiddleware/nethttp"
2426
)
2527

28+
// UserCheckResult represents either a successful user retrieval or an error response.
29+
type UserCheckResult[T any] struct {
30+
User *auth.User
31+
Response T
32+
}
33+
34+
// CheckAuthenticatedUser retrieves the authenticated user from the context.
35+
// If the user is not found, it returns an error response.
36+
func CheckAuthenticatedUser[T any](
37+
ctx context.Context,
38+
operation string,
39+
newErrorResponse func(code int, message string) T) UserCheckResult[T] {
40+
// At this point, the user should be authenticated and in the context.
41+
// If for some reason the user is not in the context, it is a library or
42+
// internal issue and not an user issue. Return 500 error in that case.
43+
user, found := httpmiddlewares.AuthenticatedUserFromContext(ctx)
44+
if !found {
45+
slog.ErrorContext(ctx, "user not found in context. middleware failure", "operation", operation)
46+
47+
return UserCheckResult[T]{
48+
User: nil,
49+
Response: newErrorResponse(
50+
http.StatusInternalServerError,
51+
"internal server error",
52+
),
53+
}
54+
}
55+
56+
var zeroT T
57+
58+
return UserCheckResult[T]{
59+
User: user,
60+
Response: zeroT,
61+
}
62+
}
63+
2664
// applyPreRequestValidationMiddlewares applies a list of middleware functions to a given http.Handler.
2765
// The middlewares are applied in reverse order to ensure they execute in the order they are defined.
2866
func applyPreRequestValidationMiddlewares(mux *http.ServeMux,

‎backend/pkg/httpserver/middlewares_test.go

+6
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,12 @@ import (
2525
"github.com/GoogleChrome/webstatus.dev/lib/httpmiddlewares"
2626
)
2727

28+
func createTestID1User() *auth.User {
29+
return &auth.User{
30+
ID: "testID1",
31+
}
32+
}
33+
2834
// TODO: Move recoveryMiddleware into the lib directory to actually be used by the real server.
2935
func recoveryMiddleware(next http.Handler) http.Handler {
3036
return http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {

‎backend/pkg/httpserver/put_user_saved_search_bookmark.go

+66
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
// Copyright 2025 Google LLC
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
package httpserver
16+
17+
import (
18+
"context"
19+
"errors"
20+
"log/slog"
21+
"net/http"
22+
23+
"github.com/GoogleChrome/webstatus.dev/lib/gcpspanner/spanneradapters/backendtypes"
24+
"github.com/GoogleChrome/webstatus.dev/lib/gen/openapi/backend"
25+
)
26+
27+
// PutUserSavedSearchBookmark implements backend.StrictServerInterface.
28+
// nolint: ireturn // Name generated from openapi
29+
func (s *Server) PutUserSavedSearchBookmark(
30+
ctx context.Context, request backend.PutUserSavedSearchBookmarkRequestObject) (
31+
backend.PutUserSavedSearchBookmarkResponseObject, error) {
32+
userCheckResult := CheckAuthenticatedUser(ctx, "PutUserSavedSearchBookmark",
33+
func(code int, message string) backend.PutUserSavedSearchBookmark500JSONResponse {
34+
return backend.PutUserSavedSearchBookmark500JSONResponse{
35+
Code: code,
36+
Message: message,
37+
}
38+
})
39+
if userCheckResult.User == nil {
40+
return userCheckResult.Response, nil
41+
}
42+
43+
err := s.wptMetricsStorer.PutUserSavedSearchBookmark(ctx, userCheckResult.User.ID, request.SearchId)
44+
if err != nil {
45+
if errors.Is(err, backendtypes.ErrEntityDoesNotExist) {
46+
return backend.PutUserSavedSearchBookmark404JSONResponse{
47+
Code: http.StatusNotFound,
48+
Message: "saved search to bookmark not found",
49+
}, nil
50+
} else if errors.Is(err, backendtypes.ErrUserMaxBookmarks) {
51+
return backend.PutUserSavedSearchBookmark403JSONResponse{
52+
Code: http.StatusForbidden,
53+
Message: "user has reached the maximum number of allowed bookmarks",
54+
}, nil
55+
}
56+
slog.ErrorContext(ctx, "unable to add bookmark", "error", err)
57+
58+
return backend.PutUserSavedSearchBookmark500JSONResponse{
59+
Code: http.StatusInternalServerError,
60+
Message: "unable to add bookmark",
61+
}, nil
62+
63+
}
64+
65+
return backend.PutUserSavedSearchBookmark200Response{}, nil
66+
}

‎backend/pkg/httpserver/put_user_saved_search_bookmark_test.go

+116
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,116 @@
1+
// Copyright 2025 Google LLC
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
// nolint: dupl // WONTFIX
16+
package httpserver
17+
18+
import (
19+
"net/http"
20+
"net/http/httptest"
21+
"testing"
22+
23+
"github.com/GoogleChrome/webstatus.dev/lib/gcpspanner/spanneradapters/backendtypes"
24+
)
25+
26+
func TestPutUserSavedSearchBookmark(t *testing.T) {
27+
authMiddlewareOption := withAuthMiddleware(mockAuthMiddleware(createTestID1User()))
28+
testCases := []basicHTTPTestCase[MockPutUserSavedSearchBookmarkConfig]{
29+
{
30+
name: "success",
31+
cfg: &MockPutUserSavedSearchBookmarkConfig{
32+
expectedSavedSearchID: "saved-search-id",
33+
expectedUserID: "testID1",
34+
err: nil,
35+
},
36+
request: httptest.NewRequest(
37+
http.MethodPut,
38+
"/v1/users/me/saved-searches/saved-search-id/bookmark_status",
39+
nil,
40+
),
41+
expectedResponse: createEmptyBodyResponse(http.StatusOK),
42+
},
43+
{
44+
name: "general error",
45+
cfg: &MockPutUserSavedSearchBookmarkConfig{
46+
expectedSavedSearchID: "saved-search-id",
47+
expectedUserID: "testID1",
48+
err: errTest,
49+
},
50+
request: httptest.NewRequest(
51+
http.MethodPut,
52+
"/v1/users/me/saved-searches/saved-search-id/bookmark_status",
53+
nil,
54+
),
55+
expectedResponse: testJSONResponse(500,
56+
`{
57+
"code":500,
58+
"message":"unable to add bookmark"
59+
}`,
60+
),
61+
},
62+
{
63+
name: "not found",
64+
cfg: &MockPutUserSavedSearchBookmarkConfig{
65+
expectedSavedSearchID: "saved-search-id",
66+
expectedUserID: "testID1",
67+
err: backendtypes.ErrEntityDoesNotExist,
68+
},
69+
request: httptest.NewRequest(
70+
http.MethodPut,
71+
"/v1/users/me/saved-searches/saved-search-id/bookmark_status",
72+
nil,
73+
),
74+
expectedResponse: testJSONResponse(404,
75+
`{
76+
"code":404,
77+
"message":"saved search to bookmark not found"
78+
}`,
79+
),
80+
},
81+
{
82+
name: "too many bookmarks",
83+
cfg: &MockPutUserSavedSearchBookmarkConfig{
84+
expectedSavedSearchID: "saved-search-id",
85+
expectedUserID: "testID1",
86+
err: backendtypes.ErrUserMaxBookmarks,
87+
},
88+
request: httptest.NewRequest(
89+
http.MethodPut,
90+
"/v1/users/me/saved-searches/saved-search-id/bookmark_status",
91+
nil,
92+
),
93+
expectedResponse: testJSONResponse(403,
94+
`{
95+
"code":403,
96+
"message":"user has reached the maximum number of allowed bookmarks"
97+
}`,
98+
),
99+
},
100+
}
101+
for _, tc := range testCases {
102+
t.Run(tc.name, func(t *testing.T) {
103+
//nolint:exhaustruct // WONTFIX
104+
mockStorer := &MockWPTMetricsStorer{
105+
putUserSavedSearchBookmarkCfg: tc.cfg,
106+
t: t,
107+
}
108+
myServer := Server{wptMetricsStorer: mockStorer, metadataStorer: nil,
109+
operationResponseCaches: nil}
110+
assertTestServerRequest(t, &myServer, tc.request, tc.expectedResponse,
111+
[]testServerOption{authMiddlewareOption}...)
112+
assertMocksExpectations(t, 1, mockStorer.callCountPutUserSavedSearchBookmark,
113+
"PutUserSavedSearchBookmark", nil)
114+
})
115+
}
116+
}

‎backend/pkg/httpserver/remove_saved_search.go

+10-13
Original file line numberDiff line numberDiff line change
@@ -22,28 +22,25 @@ import (
2222

2323
"github.com/GoogleChrome/webstatus.dev/lib/gcpspanner/spanneradapters/backendtypes"
2424
"github.com/GoogleChrome/webstatus.dev/lib/gen/openapi/backend"
25-
"github.com/GoogleChrome/webstatus.dev/lib/httpmiddlewares"
2625
)
2726

2827
// RemoveSavedSearch implements backend.StrictServerInterface.
2928
// nolint: ireturn // Name generated from openapi
3029
func (s *Server) RemoveSavedSearch(
3130
ctx context.Context, request backend.RemoveSavedSearchRequestObject) (
3231
backend.RemoveSavedSearchResponseObject, error) {
33-
// At this point, the user should be authenticated and in the context.
34-
// If for some reason the user is not in the context, it is a library or
35-
// internal issue and not an user issue. Return 500 error in that case.
36-
user, found := httpmiddlewares.AuthenticatedUserFromContext(ctx)
37-
if !found {
38-
slog.ErrorContext(ctx, "user not found in context. middleware malfunction")
39-
40-
return backend.RemoveSavedSearch500JSONResponse{
41-
Code: http.StatusInternalServerError,
42-
Message: "internal server error",
43-
}, nil
32+
userCheckResult := CheckAuthenticatedUser(ctx, "RemoveSavedSearch",
33+
func(code int, message string) backend.RemoveSavedSearch500JSONResponse {
34+
return backend.RemoveSavedSearch500JSONResponse{
35+
Code: code,
36+
Message: message,
37+
}
38+
})
39+
if userCheckResult.User == nil {
40+
return userCheckResult.Response, nil
4441
}
4542

46-
err := s.wptMetricsStorer.DeleteUserSavedSearch(ctx, user.ID, request.SearchId)
43+
err := s.wptMetricsStorer.DeleteUserSavedSearch(ctx, userCheckResult.User.ID, request.SearchId)
4744
if err != nil {
4845
if errors.Is(err, backendtypes.ErrEntityDoesNotExist) {
4946
return backend.RemoveSavedSearch404JSONResponse{

0 commit comments

Comments
 (0)