diff --git a/internal/handlers/usage_wall.go b/internal/handlers/usage_wall.go index 36a8d44b..25d8f16d 100644 --- a/internal/handlers/usage_wall.go +++ b/internal/handlers/usage_wall.go @@ -36,6 +36,7 @@ package handlers import ( "database/sql" "encoding/json" + "fmt" "log/slog" "time" @@ -58,6 +59,29 @@ const usageWallKind = "near_quota_wall" // return near_wall=false even if it exists. const usageWallFreshness = 24 * time.Hour +// usageWallCacheMaxAge is the Cache-Control: max-age value emitted on +// every GET /api/v1/usage/wall response. BUG-API-420 (QA 2026-05-29): +// the dashboard fetches this endpoint on every navigation (Overview +// page on mount + every 5-min poll); without a cache hint the browser +// re-fetches on every back/forward / SPA route change, generating +// redundant DB scans on the busiest team-scoped table (audit_log). +// +// 30s is the consistency tradeoff (memory rule +// feedback_caching_and_consistency_tradeoffs): +// - The worker writes at most one near_quota_wall row per team per +// 24h (quotaWallDedupeWindow = usageWallFreshness above), so a +// 30s staleness window can miss a brand-new wall event by ≤30s. +// - The wall banner is a soft upgrade-nudge, not a hard quota +// enforcement — the hard 402 still fires synchronously on the +// next provision regardless of banner state. +// - 30s < the 5-min dashboard poll interval, so the cache flushes +// well within one polling round and walls are visible within +// 5min + 30s in the worst case. +// +// `Vary: Authorization` is added alongside so per-team caching at a +// CDN never serves team A's banner state to team B. +const usageWallCacheMaxAge = 30 + // UsageWallHandler serves GET /api/v1/usage/wall. type UsageWallHandler struct { db *sql.DB @@ -81,6 +105,23 @@ type wallMetadata struct { PercentUsed int `json:"percent_used"` } +// setUsageWallCacheHeaders stamps the per-team cache hint on every 200 +// response from GetWall. Lives on the handler struct so a future +// per-team max-age override (e.g. shorter for hobby_plus walls) has a +// single edit site. BUG-API-420. +func (h *UsageWallHandler) setUsageWallCacheHeaders(c *fiber.Ctx) { + // Sprintf instead of a literal so the constant above (with its + // consistency-tradeoff comment) is the single source of truth and + // can't drift from the wire value. golangci-lint's `unused` check + // blocks anyone deleting the constant without also touching this + // fmt format string. + c.Set("Cache-Control", fmt.Sprintf("private, max-age=%d", usageWallCacheMaxAge)) + // Vary: Authorization is the per-team boundary — without it a + // shared cache (CDN, browser back-cache after re-auth) could + // serve team A's banner state to team B. + c.Set("Vary", "Authorization") +} + // GetWall handles GET /api/v1/usage/wall. func (h *UsageWallHandler) GetWall(c *fiber.Ctx) error { teamID, err := uuid.Parse(middleware.GetTeamID(c)) @@ -92,6 +133,7 @@ func (h *UsageWallHandler) GetWall(c *fiber.Ctx) error { // Fail-open: if team lookup errors, fall through to the audit // query rather than refusing to serve. if team, terr := models.GetTeamByID(c.Context(), h.db, teamID); terr == nil && team != nil && team.PlanTier == "team" { + h.setUsageWallCacheHeaders(c) return c.JSON(fiber.Map{"ok": true, "near_wall": false}) } @@ -113,6 +155,7 @@ func (h *UsageWallHandler) GetWall(c *fiber.Ctx) error { ) if scanErr := row.Scan(&metadataRaw, &createdAt); scanErr != nil { if scanErr == sql.ErrNoRows { + h.setUsageWallCacheHeaders(c) return c.JSON(fiber.Map{"ok": true, "near_wall": false}) } slog.Error("usage.wall.query_failed", @@ -143,5 +186,6 @@ func (h *UsageWallHandler) GetWall(c *fiber.Ctx) error { resp["percent_used"] = meta.PercentUsed } } + h.setUsageWallCacheHeaders(c) return c.JSON(resp) } diff --git a/internal/handlers/usage_wall_test.go b/internal/handlers/usage_wall_test.go index 432fe9e5..4380a62e 100644 --- a/internal/handlers/usage_wall_test.go +++ b/internal/handlers/usage_wall_test.go @@ -133,6 +133,79 @@ func TestUsageWall_ReturnsFalseWhenNoRecentRow(t *testing.T) { require.NoError(t, mock.ExpectationsWereMet()) } +// TestUsageWall_CacheHeadersOnEvery200Path is the registry-iterating +// regression for BUG-API-420. /api/v1/usage/wall has three distinct 200 +// code paths in GetWall (team-tier short-circuit, no-recent-row, +// row-found-with-metadata) — every one MUST stamp the same Cache-Control +// and Vary headers, otherwise a dashboard polling the endpoint on every +// nav re-hits the DB on the busy team-scoped audit_log table. The cases +// table mirrors the three code paths in usage_wall.go; adding a fourth +// path without updating this test (which would mean the path skips the +// cache header) is the bug class rule 18 protects against. +func TestUsageWall_CacheHeadersOnEvery200Path(t *testing.T) { + cases := []struct { + name string + prime func(mock sqlmock.Sqlmock, teamID uuid.UUID) + }{ + { + name: "team_tier_short_circuit", + prime: func(mock sqlmock.Sqlmock, teamID uuid.UUID) { + expectTeamLookup(mock, teamID, "team") + }, + }, + { + name: "no_recent_row", + prime: func(mock sqlmock.Sqlmock, teamID uuid.UUID) { + expectTeamLookup(mock, teamID, "hobby") + mock.ExpectQuery(`SELECT metadata, created_at\s+FROM audit_log`). + WithArgs(teamID, "near_quota_wall", sqlmock.AnyArg()). + WillReturnError(sql.ErrNoRows) + }, + }, + { + name: "row_found_with_metadata", + prime: func(mock sqlmock.Sqlmock, teamID uuid.UUID) { + expectTeamLookup(mock, teamID, "hobby") + metadata := `{"tier":"hobby","axis":"storage","service":"postgres","current":1,"limit":2,"percent_used":50}` + mock.ExpectQuery(`SELECT metadata, created_at\s+FROM audit_log`). + WithArgs(teamID, "near_quota_wall", sqlmock.AnyArg()). + WillReturnRows(sqlmock.NewRows([]string{"metadata", "created_at"}). + AddRow(metadata, time.Now().Add(-1*time.Hour))) + }, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + db, mock, err := sqlmock.New(sqlmock.QueryMatcherOption(sqlmock.QueryMatcherRegexp)) + require.NoError(t, err) + defer db.Close() + + teamID := uuid.New() + tc.prime(mock, teamID) + + app := newUsageWallApp(t, db, teamID) + req := httptest.NewRequest(http.MethodGet, "/api/v1/usage/wall", nil) + resp, err := app.Test(req, 5000) + require.NoError(t, err) + defer resp.Body.Close() + + assert.Equal(t, http.StatusOK, resp.StatusCode) + // BUG-API-420: Cache-Control: private, max-age=30 keeps + // per-team caching local to the browser (never a shared + // CDN) and clamps staleness to 30s — well under the + // dashboard's 5-min poll interval. + assert.Equal(t, "private, max-age=30", resp.Header.Get("Cache-Control"), + "BUG-API-420: %s path must emit Cache-Control: private, max-age=30", tc.name) + // Vary: Authorization prevents team A's banner state from + // being served to team B (per-team cache key). + assert.Equal(t, "Authorization", resp.Header.Get("Vary"), + "BUG-API-420: %s path must emit Vary: Authorization so per-team cache keys never cross teams", tc.name) + require.NoError(t, mock.ExpectationsWereMet()) + }) + } +} + // TestUsageWall_TeamTierShortCircuits verifies the team-tier early // return: a team-tier caller MUST get near_wall=false without an // audit_log query (sqlmock strict mode catches the unexpected query).