ai cooking
0
fork

Configure Feed

Select the types of activity you want to include in your feed.

Pmiller/sesionnotcached (#382)

* don't do session cookies on static assets

* different way to wrap some routes

* n logigng of ready

* app insights doens't need to know about ready

* app a basee middle ware that still passes

* gofumpt

* ordering and frumpt

* more fumpt and fake trackers better than nil

* fake request tracker

* fumpt

---------

Co-authored-by: paul miller <paul.miller>

authored by

Paul Miller
paul miller
and committed by
GitHub
e44166b7 ad72c271

+155 -74
+1
AGENTS.md
··· 31 31 - Prefer standard library first; add dependencies sparingly and record rationale in PR description if new. 32 32 - Prefer simple html to javascript frameworks 33 33 - For UI copy, prefer plain culinary language over technical terms (example: use "Try again, chef" instead of "Regenerate", and "make it vegetarian" instead of "prefer vegetarian"). 34 + - Nothing is used outside of this repository so if a method is only used in tests it can be removed even if its public 34 35 35 36 ## Testing Guidelines 36 37 - Always run tests after making code changes. Default to `go test ./...`; use a narrower `go test ./... -run TestName` only when appropriate for quick iteration. If you cannot run tests, explicitly say why.
+23 -20
cmd/careme/middleware.go
··· 49 49 50 50 lrw := &loggingResponseWriter{w, http.StatusOK} 51 51 l.Handler.ServeHTTP(lrw, r) 52 - if r.URL.Path == "/ready" { 53 - return 54 - } 55 52 56 53 slog.InfoContext(r.Context(), "request", "method", r.Method, "url", r.URL.Path, "query", r.URL.Query(), "response", lrw.statusCode, "user", user, "form", r.Form, "duration", time.Since(start)) 57 54 } ··· 88 85 lrw := &loggingResponseWriter{w, http.StatusOK} 89 86 a.Handler.ServeHTTP(lrw, r) 90 87 91 - if r.URL.Path == "/ready" { 92 - return 93 - } 94 - 95 88 a.tracker.TrackRequest(r.Context(), r.Method, r.URL.String(), time.Since(start), strconv.Itoa(lrw.statusCode)) 96 89 } 97 90 98 - func newAppInsightsTracker(next http.Handler, connectionString string) (http.Handler, error) { 91 + func newAppInsightsTracker(next http.Handler, tracker requestTracker) http.Handler { 92 + return &appInsightsTracker{ 93 + Handler: next, 94 + tracker: tracker, 95 + } 96 + } 97 + 98 + func newRequestTracker(connectionString string) (requestTracker, error) { 99 99 client, err := newAppInsightsTelemetryClient(connectionString) 100 100 if err != nil { 101 101 return nil, err 102 102 } 103 - return &appInsightsTracker{ 104 - Handler: next, 105 - tracker: &appInsightsTelemetryTracker{client: client}, 106 - }, nil 103 + return &appInsightsTelemetryTracker{client: client}, nil 107 104 } 108 105 109 - func newAppInsightsTrackerFromEnv(next http.Handler) http.Handler { 106 + func newRequestTrackerFromEnv() requestTracker { 110 107 connectionString := os.Getenv(logsetup.AppInsightsConnectionStringEnv) 111 108 if connectionString == "" { 112 - return next 109 + return nil 113 110 } 114 111 115 - handler, err := newAppInsightsTracker(next, connectionString) 112 + tracker, err := newRequestTracker(connectionString) 116 113 if err != nil { 117 114 slog.Error("failed to configure app insights request tracking", "error", err) 118 - return next 115 + return nil 119 116 } 120 117 121 - return handler 118 + return tracker 122 119 } 123 120 124 121 func newAppInsightsTelemetryClient(connectionString string) (azureappinsights.TelemetryClient, error) { ··· 231 228 } 232 229 } 233 230 234 - func WithMiddleware(h http.Handler) http.Handler { 231 + // just recover and log 232 + func BaseMiddleware(h http.Handler) http.Handler { 235 233 h = &recoverer{h} 236 - h = newAppInsightsTrackerFromEnv(h) 237 - h = &logger{h} 234 + return &logger{h} 235 + } 236 + 237 + // instrument with app insights and log with operation and session ids. 238 + func AppMiddleWare(h http.Handler, tracker requestTracker) http.Handler { 239 + h = BaseMiddleware(h) 240 + h = newAppInsightsTracker(h, tracker) // must be "inside" operatid and session handler. 238 241 h = &operationIDHandler{h} 239 242 return &sessionIDHandler{h} 240 243 }
+36 -20
cmd/careme/middleware_test.go
··· 90 90 } 91 91 } 92 92 93 - func TestAppInsightsTrackerSkipsReady(t *testing.T) { 94 - tracker := &fakeRequestTracker{} 95 - mw := &appInsightsTracker{ 96 - Handler: http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { 97 - w.WriteHeader(http.StatusOK) 98 - }), 99 - tracker: tracker, 100 - } 101 - 102 - req := httptest.NewRequest(http.MethodGet, "https://careme.cooking/ready", nil) 103 - rec := httptest.NewRecorder() 104 - mw.ServeHTTP(rec, req) 105 - 106 - if len(tracker.calls) != 0 { 107 - t.Fatalf("expected 0 tracked requests for /ready, got %d", len(tracker.calls)) 108 - } 109 - } 110 - 111 93 func TestAppInsightsTrackerTracksRecoveredPanicAs500(t *testing.T) { 112 94 tracker := &fakeRequestTracker{} 113 95 mw := &appInsightsTracker{ ··· 270 252 func TestWithMiddlewareProvidesBothIDs(t *testing.T) { 271 253 var operationID string 272 254 var sessionID string 273 - handler := WithMiddleware(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { 255 + handler := AppMiddleWare(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { 274 256 operationID, _ = logsetup.OperationIDFromContext(r.Context()) 275 257 sessionID, _ = logsetup.SessionIDFromContext(r.Context()) 276 258 w.WriteHeader(http.StatusNoContent) 277 - })) 259 + }), &fakeRequestTracker{}) 278 260 279 261 req := httptest.NewRequest(http.MethodGet, "http://careme.cooking/about", nil) 280 262 rec := httptest.NewRecorder() ··· 304 286 } 305 287 t.Fatalf("expected cookie %q", name) 306 288 return nil 289 + } 290 + 291 + func TestRouteScopedMiddlewareSkipsSessionCookieForStaticRoutes(t *testing.T) { 292 + rootMux := http.NewServeMux() 293 + appMux := http.NewServeMux() 294 + infraMux := http.NewServeMux() 295 + infraMux.HandleFunc("/static/app.js", func(w http.ResponseWriter, _ *http.Request) { 296 + w.Header().Set("Cache-Control", "public, max-age=31536000, immutable") 297 + w.WriteHeader(http.StatusNoContent) 298 + }) 299 + appMux.HandleFunc("/about", func(w http.ResponseWriter, _ *http.Request) { 300 + w.WriteHeader(http.StatusNoContent) 301 + }) 302 + rootMux.Handle("/static/", BaseMiddleware(infraMux)) 303 + rootMux.Handle("/", AppMiddleWare(appMux, &fakeRequestTracker{})) 304 + 305 + staticReq := httptest.NewRequest(http.MethodGet, "http://careme.cooking/static/app.js", nil) 306 + staticRec := httptest.NewRecorder() 307 + rootMux.ServeHTTP(staticRec, staticReq) 308 + 309 + if got := staticRec.Header().Values("Set-Cookie"); len(got) != 0 { 310 + t.Fatalf("expected no Set-Cookie on static route, got %v", got) 311 + } 312 + if staticRec.Header().Get("X-Operation-ID") != "" { 313 + t.Fatal("expected static route to NOT receive operation id from base middleware") 314 + } 315 + 316 + appReq := httptest.NewRequest(http.MethodGet, "http://careme.cooking/about", nil) 317 + appRec := httptest.NewRecorder() 318 + rootMux.ServeHTTP(appRec, appReq) 319 + 320 + if findCookie(t, appRec.Result().Cookies(), sessionCookieName).Value == "" { 321 + t.Fatal("expected session cookie on app route") 322 + } 307 323 } 308 324 309 325 func TestParseAppInsightsConnectionString(t *testing.T) {
+2
cmd/careme/ready.go
··· 2 2 3 3 import ( 4 4 "context" 5 + "fmt" 5 6 "log/slog" 6 7 "net/http" 7 8 ) ··· 17 18 } 18 19 for _, check := range r.checks { 19 20 if err := check.Ready(ctx); err != nil { 21 + slog.ErrorContext(ctx, "check failed", "error", err, "check", fmt.Sprintf("%T", check)) 20 22 return err 21 23 } 22 24 }
+21 -13
cmd/careme/web.go
··· 20 20 "careme/internal/ingredients" 21 21 "careme/internal/locations" 22 22 "careme/internal/recipes" 23 + "careme/internal/routing" 23 24 "careme/internal/seasons" 24 25 "careme/internal/sitemap" 25 26 "careme/internal/static" 26 27 "careme/internal/templates" 27 28 "careme/internal/users" 29 + 28 30 utypes "careme/internal/users/types" 29 31 ) 30 32 ··· 39 41 return fmt.Errorf("failed to create auth client: %w", err) 40 42 } 41 43 42 - mux := http.NewServeMux() 43 - authClient.Register(mux) 44 - static.Register(mux) 44 + rootMux := http.NewServeMux() 45 + appRoutes := routing.Wrap(rootMux, func(h http.Handler) http.Handler { 46 + return authClient.WithAuthHTTP(AppMiddleWare(h, newRequestTrackerFromEnv())) 47 + }) 48 + infraRoutes := routing.Wrap(rootMux, BaseMiddleware) 49 + 50 + authClient.Register(appRoutes) 51 + static.Register(infraRoutes) 45 52 46 53 userStorage := users.NewStorage(cache) 47 54 ··· 58 65 } 59 66 60 67 userHandler := users.NewHandler(userStorage, locationStorage, authClient) 61 - userHandler.Register(mux) 68 + userHandler.Register(appRoutes) 62 69 63 70 locationServer := locations.NewServer(locationStorage, centroids, userStorage) 64 - locationServer.Register(mux, authClient) 71 + locationServer.Register(appRoutes, authClient) 65 72 66 73 sitemapHandler := sitemap.New(cache) 67 - sitemapHandler.Register(mux) 74 + sitemapHandler.Register(infraRoutes) 68 75 69 76 recipeHandler := recipes.NewHandler(cfg, userStorage, generator, locationStorage, cache, authClient) 70 - recipeHandler.Register(mux) 77 + recipeHandler.Register(appRoutes) 71 78 72 - actowiz.NewServer().Register(mux) 79 + actowiz.NewServer().Register(infraRoutes) 73 80 74 81 adminMux := http.NewServeMux() 75 82 adminMux.Handle("/users", users.AdminUsersPage(userStorage)) 76 83 ingredientsHandler := ingredients.NewHandler(cache) 77 84 ingredientsHandler.Register(adminMux) 78 - mux.Handle("/admin/", admin.New(cfg, authClient).Enforce(http.StripPrefix("/admin", adminMux))) 85 + appRoutes.Handle("/admin/", admin.New(cfg, authClient).Enforce(http.StripPrefix("/admin", adminMux))) 79 86 80 - mux.HandleFunc("/about", func(w http.ResponseWriter, r *http.Request) { 87 + appRoutes.HandleFunc("/about", func(w http.ResponseWriter, r *http.Request) { 81 88 ctx := r.Context() 82 89 data := struct { 83 90 ClarityScript template.HTML ··· 94 101 } 95 102 }) 96 103 97 - mux.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) { 104 + appRoutes.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) { 98 105 ctx := r.Context() 99 106 currentUser, err := userStorage.FromRequest(ctx, r, authClient) 100 107 if err != nil { ··· 142 149 ro := &readyOnce{} 143 150 ro.Add(generator, locationServer) 144 151 145 - mux.Handle("/ready", ro) 152 + // no logging for readyiness too noisy. 153 + rootMux.Handle("/ready", &recoverer{ro}) 146 154 147 155 server := &http.Server{ 148 156 Addr: addr, 149 - Handler: authClient.WithAuthHTTP(WithMiddleware(mux)), 157 + Handler: rootMux, 150 158 } 151 159 152 160 // Channel to listen for errors coming from the server
+11 -6
cmd/careme/web_e2e_test.go
··· 16 16 "careme/internal/config" 17 17 "careme/internal/locations" 18 18 "careme/internal/recipes" 19 + "careme/internal/routing" 19 20 "careme/internal/templates" 20 21 "careme/internal/users" 21 22 ··· 174 175 175 176 mockAuth := auth.Mock(cfg) 176 177 177 - mux := http.NewServeMux() 178 + rootMux := http.NewServeMux() 179 + appRoutes := routing.Wrap(rootMux, func(h http.Handler) http.Handler { 180 + return mockAuth.WithAuthHTTP(AppMiddleWare(h, &fakeRequestTracker{})) 181 + }) 182 + infraRoutes := routing.Wrap(rootMux, BaseMiddleware) 178 183 locationServer := locations.NewServer(locationStorage, centroids, userStorage) 179 - locationServer.Register(mux, mockAuth) 180 - users.NewHandler(userStorage, locationStorage, mockAuth).Register(mux) 181 - recipes.NewHandler(cfg, userStorage, generator, locationStorage, cacheStore, mockAuth).Register(mux) 184 + locationServer.Register(appRoutes, mockAuth) 185 + users.NewHandler(userStorage, locationStorage, mockAuth).Register(appRoutes) 186 + recipes.NewHandler(cfg, userStorage, generator, locationStorage, cacheStore, mockAuth).Register(appRoutes) 182 187 183 188 ro := &readyOnce{} 184 189 ro.Add(generator, locationServer) 185 190 186 - mux.Handle("/ready", ro) 191 + infraRoutes.Handle("/ready", ro) 187 192 188 - return httptest.NewServer(WithMiddleware(mux)) 193 + return httptest.NewServer(rootMux) 189 194 } 190 195 191 196 func newTestClient(t *testing.T) *http.Client {
+3 -1
internal/actowiz/server.go
··· 3 3 import ( 4 4 "encoding/json" 5 5 "net/http" 6 + 7 + "careme/internal/routing" 6 8 ) 7 9 8 10 const scrapeIntervalDays = 7 ··· 61 63 } 62 64 } 63 65 64 - func (s *server) Register(mux *http.ServeMux) { 66 + func (s *server) Register(mux routing.Registrar) { 65 67 mux.HandleFunc("GET /actowiz/stores.json", s.handleStores) 66 68 } 67 69
+2 -1
internal/admin/middleware_test.go
··· 9 9 10 10 "careme/internal/auth" 11 11 "careme/internal/config" 12 + "careme/internal/routing" 12 13 ) 13 14 14 15 type stubAuthClient struct { ··· 36 37 return handler 37 38 } 38 39 39 - func (s stubAuthClient) Register(_ *http.ServeMux) {} 40 + func (s stubAuthClient) Register(_ routing.Registrar) {} 40 41 41 42 func TestMiddlewareWrapRejectsNoSession(t *testing.T) { 42 43 t.Parallel()
+3 -2
internal/auth/clerk.go
··· 11 11 "time" 12 12 13 13 "careme/internal/config" 14 + "careme/internal/routing" 14 15 "careme/internal/templates" 15 16 16 17 "github.com/clerk/clerk-sdk-go/v2" ··· 26 27 GetUserEmail(ctx context.Context, clerkUserID string) (string, error) 27 28 GetUserIDFromRequest(r *http.Request) (string, error) 28 29 WithAuthHTTP(handler http.Handler) http.Handler 29 - Register(mux *http.ServeMux) 30 + Register(mux routing.Registrar) 30 31 } 31 32 32 33 // Client wraps Clerk SDK functionality ··· 159 160 }) 160 161 } 161 162 162 - func (c *clerkClient) Register(mux *http.ServeMux) { 163 + func (c *clerkClient) Register(mux routing.Registrar) { 163 164 mux.HandleFunc("/logout", c.logout) 164 165 mux.HandleFunc("/sign-in", func(w http.ResponseWriter, r *http.Request) { 165 166 http.Redirect(w, r, c.cfg.Clerk.Signin(), http.StatusSeeOther)
+2 -1
internal/auth/mock.go
··· 5 5 "net/http" 6 6 7 7 "careme/internal/config" 8 + "careme/internal/routing" 8 9 ) 9 10 10 11 // Client wraps Clerk SDK functionality ··· 48 49 http.Error(w, "not implemented in mock auth client", http.StatusNotImplemented) 49 50 } 50 51 51 - func (c *mockClient) Register(mux *http.ServeMux) { 52 + func (c *mockClient) Register(mux routing.Registrar) { 52 53 mux.HandleFunc("/logout", c.logout) 53 54 }
+2 -1
internal/ingredients/server.go
··· 9 9 "careme/internal/cache" 10 10 "careme/internal/kroger" 11 11 "careme/internal/recipes" 12 + "careme/internal/routing" 12 13 ) 13 14 14 15 type server struct { ··· 19 20 return &server{cache: c} 20 21 } 21 22 22 - func (s *server) Register(mux *http.ServeMux) { 23 + func (s *server) Register(mux routing.Registrar) { 23 24 mux.HandleFunc("GET /ingredients/{hash}", s.handleIngredients) 24 25 } 25 26
+2 -1
internal/locations/locations.go
··· 25 25 "careme/internal/locations/geo" 26 26 locationtypes "careme/internal/locations/types" 27 27 "careme/internal/publix" 28 + "careme/internal/routing" 28 29 "careme/internal/seasons" 29 30 "careme/internal/templates" 30 31 utypes "careme/internal/users/types" ··· 288 289 return err 289 290 } 290 291 291 - func (l *locationServer) Register(mux *http.ServeMux, authClient auth.AuthClient) { 292 + func (l *locationServer) Register(mux routing.Registrar, authClient auth.AuthClient) { 292 293 mux.HandleFunc("GET /locations/zip-from-coordinates", func(w http.ResponseWriter, r *http.Request) { 293 294 lat, err := strconv.ParseFloat(r.URL.Query().Get("lat"), 64) 294 295 if err != nil {
+2 -1
internal/locations/mock.go
··· 7 7 "net/http" 8 8 9 9 "careme/internal/auth" 10 + "careme/internal/routing" 10 11 "careme/internal/seasons" 11 12 "careme/internal/templates" 12 13 ··· 51 52 return "", false 52 53 } 53 54 54 - func (m mock) Register(mux *http.ServeMux, _ auth.AuthClient) { 55 + func (m mock) Register(mux routing.Registrar, _ auth.AuthClient) { 55 56 mux.HandleFunc("/locations", func(w http.ResponseWriter, r *http.Request) { 56 57 data := struct { 57 58 Locations []Location
+2 -1
internal/recipes/server.go
··· 19 19 "careme/internal/cache" 20 20 "careme/internal/config" 21 21 "careme/internal/locations" 22 + "careme/internal/routing" 22 23 "careme/internal/seasons" 23 24 "careme/internal/templates" 24 25 "careme/internal/users" ··· 67 68 } 68 69 } 69 70 70 - func (s *server) Register(mux *http.ServeMux) { 71 + func (s *server) Register(mux routing.Registrar) { 71 72 mux.HandleFunc("GET /recipes", s.handleRecipes) 72 73 mux.HandleFunc("POST /recipes/{hash}/regenerate", s.handleRegenerate) 73 74 mux.HandleFunc("POST /recipes/{hash}/finalize", s.handleFinalize)
+2 -1
internal/recipes/server_test.go
··· 17 17 "careme/internal/auth" 18 18 "careme/internal/cache" 19 19 "careme/internal/locations" 20 + "careme/internal/routing" 20 21 "careme/internal/users" 21 22 utypes "careme/internal/users/types" 22 23 ) ··· 446 447 return handler 447 448 } 448 449 449 - func (n noSessionAuth) Register(mux *http.ServeMux) {} 450 + func (n noSessionAuth) Register(mux routing.Registrar) {} 450 451 451 452 func TestHandleQuestion_RequiresSignedInUser(t *testing.T) { 452 453 cacheStore := cache.NewFileCache(filepath.Join(t.TempDir(), "cache"))
+31
internal/routing/routing.go
··· 1 + package routing 2 + 3 + import "net/http" 4 + 5 + type Registrar interface { 6 + Handle(string, http.Handler) 7 + HandleFunc(string, func(http.ResponseWriter, *http.Request)) 8 + } 9 + 10 + type middlewareRegistrar struct { 11 + target Registrar 12 + wrap func(http.Handler) http.Handler 13 + } 14 + 15 + func Wrap(target Registrar, wrap func(http.Handler) http.Handler) Registrar { 16 + if wrap == nil { 17 + return target 18 + } 19 + return middlewareRegistrar{ 20 + target: target, 21 + wrap: wrap, 22 + } 23 + } 24 + 25 + func (m middlewareRegistrar) Handle(pattern string, handler http.Handler) { 26 + m.target.Handle(pattern, m.wrap(handler)) 27 + } 28 + 29 + func (m middlewareRegistrar) HandleFunc(pattern string, handler func(http.ResponseWriter, *http.Request)) { 30 + m.target.Handle(pattern, m.wrap(http.HandlerFunc(handler))) 31 + }
+2 -1
internal/sitemap/sitemap.go
··· 9 9 10 10 "careme/internal/cache" 11 11 "careme/internal/recipes" 12 + "careme/internal/routing" 12 13 ) 13 14 14 15 type Server struct { ··· 30 31 return &Server{cache: c} 31 32 } 32 33 33 - func (s *Server) Register(mux *http.ServeMux) { 34 + func (s *Server) Register(mux routing.Registrar) { 34 35 mux.HandleFunc("GET /sitemap.xml", s.handleSitemap) 35 36 mux.HandleFunc("GET /robots.txt", s.handleRobots) 36 37 }
+2 -1
internal/static/static.go
··· 7 7 "log/slog" 8 8 "net/http" 9 9 10 + "careme/internal/routing" 10 11 "careme/internal/seasons" 11 12 ) 12 13 ··· 36 37 } 37 38 38 39 // Register serves static assets and wires template asset paths. 39 - func Register(mux *http.ServeMux) { 40 + func Register(mux routing.Registrar) { 40 41 mux.HandleFunc(TailwindAssetPath, func(w http.ResponseWriter, r *http.Request) { 41 42 w.Header().Set("Content-Type", "text/css; charset=utf-8") 42 43 w.Header().Set("Cache-Control", "public, max-age=31536000, immutable")
+2 -1
internal/users/server.go
··· 11 11 12 12 "careme/internal/auth" 13 13 "careme/internal/locations" 14 + "careme/internal/routing" 14 15 "careme/internal/seasons" 15 16 "careme/internal/templates" 16 17 utypes "careme/internal/users/types" ··· 37 38 } 38 39 } 39 40 40 - func (s *server) Register(mux *http.ServeMux) { 41 + func (s *server) Register(mux routing.Registrar) { 41 42 mux.HandleFunc("/user", s.handleUser) 42 43 mux.HandleFunc("POST /user/recipes", s.handleUserRecipes) 43 44 mux.HandleFunc("POST /user/favorite", s.handleFavorite)
+2 -1
internal/users/server_favorite_test.go
··· 11 11 12 12 "careme/internal/auth" 13 13 "careme/internal/cache" 14 + "careme/internal/routing" 14 15 ) 15 16 16 17 type noSessionAuth struct{} ··· 27 28 return handler 28 29 } 29 30 30 - func (n noSessionAuth) Register(_ *http.ServeMux) {} 31 + func (n noSessionAuth) Register(_ routing.Registrar) {} 31 32 32 33 func newFavoriteTestServer(t *testing.T, clerk auth.AuthClient) *server { 33 34 t.Helper()
+2 -1
internal/users/server_test.go
··· 14 14 15 15 "careme/internal/cache" 16 16 "careme/internal/locations" 17 + "careme/internal/routing" 17 18 utypes "careme/internal/users/types" 18 19 ) 19 20 ··· 31 32 return handler 32 33 } 33 34 34 - func (t testAuthClient) Register(_ *http.ServeMux) {} 35 + func (t testAuthClient) Register(_ routing.Registrar) {} 35 36 36 37 type failingLocationGetter struct{} 37 38