From 5f09da3f4bc9a7e337c49674f0fa8639b462d2e7 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 7 Feb 2026 21:42:57 +0000 Subject: [PATCH 1/8] Add online rating service design plan Cloudflare Workers + D1 backend with Firebase Anonymous Auth for cross-device identity. Covers API design, Flutter integration, offline sync strategy, and phased implementation plan. https://claude.ai/code/session_01SusUN4s9TQ4AqeoyoGAGYa --- docs/planning/rating-service/design.md | 497 +++++++++++++++++++++++++ 1 file changed, 497 insertions(+) create mode 100644 docs/planning/rating-service/design.md diff --git a/docs/planning/rating-service/design.md b/docs/planning/rating-service/design.md new file mode 100644 index 00000000..c0394bfd --- /dev/null +++ b/docs/planning/rating-service/design.md @@ -0,0 +1,497 @@ +# Online Rating Service — Design Plan + +## Decision Summary + +| Decision | Choice | Rationale | +|----------|--------|-----------| +| Backend | Cloudflare Workers + D1 | Existing Worker infra, SQL aggregation, unified stack | +| Identity | Firebase Anonymous Auth (client-side only) | Cross-device upgrade path, no Firebase backend needed | +| Rating model | Simple star rating (1-5) | Matches existing UI, low complexity | +| API response | Average + count + distribution + user's own rating | Full data for rich UI | +| Endpoints | Single-drink + batch | Stars on list screen without N+1 requests | +| Anti-abuse | One rating per user per drink + frequency limiting | Proportionate for festival app | +| Offline | Optimistic local + background sync | Festival venues have patchy signal | + +--- + +## Architecture Overview + +``` +┌─────────────────────────────────────────────────────────┐ +│ Flutter App │ +│ │ +│ ┌──────────────┐ ┌──────────────┐ ┌──────────────┐ │ +│ │ Firebase Auth │ │ RatingsService│ │ BeerProvider │ │ +│ │ (Anonymous) │ │ (local cache) │ │ │ │ +│ └──────┬───────┘ └──────┬───────┘ └──────┬───────┘ │ +│ │ │ │ │ +│ │ ┌────────────┴──────────────┐ │ │ +│ │ │ OnlineRatingService │ │ │ +│ └───►│ - sends Firebase UID │◄──┘ │ +│ │ - optimistic local update │ │ +│ │ - background sync queue │ │ +│ └────────────┬──────────────┘ │ +└───────────────────────────┼─────────────────────────────┘ + │ HTTPS + ▼ +┌─────────────────────────────────────────────────────────┐ +│ Cloudflare Worker (ratings-api) │ +│ │ +│ ┌─────────────┐ ┌──────────────┐ ┌──────────────┐ │ +│ │ Auth │ │ Rate Limiter │ │ Router │ │ +│ │ Middleware │ │ (per-UID) │ │ │ │ +│ └──────┬──────┘ └──────┬───────┘ └──────┬───────┘ │ +│ │ │ │ │ +│ └────────────────┴──────────────────┘ │ +│ │ │ +│ ┌─────▼─────┐ │ +│ │ D1 SQLite │ │ +│ │ Database │ │ +│ └───────────┘ │ +└─────────────────────────────────────────────────────────┘ +``` + +--- + +## 1. Cloudflare Worker — Ratings API + +### 1.1 New Worker or Extend Existing? + +**Recommendation: New Worker (`cbf-ratings-api`)** + +Reasons: +- Existing worker is a stateless CORS proxy — different concern +- Ratings worker needs D1 database binding +- Separate deployment lifecycle (ratings API can change without touching the data proxy) +- Separate rate limiting and auth middleware +- Can share the same CORS utility code + +### 1.2 D1 Database Schema + +```sql +-- Individual ratings (one per user per drink per festival) +CREATE TABLE ratings ( + id INTEGER PRIMARY KEY AUTOINCREMENT, + festival_id TEXT NOT NULL, + drink_id TEXT NOT NULL, + user_id TEXT NOT NULL, -- Firebase Anonymous UID + rating INTEGER NOT NULL CHECK (rating BETWEEN 1 AND 5), + created_at TEXT NOT NULL DEFAULT (datetime('now')), + updated_at TEXT NOT NULL DEFAULT (datetime('now')), + UNIQUE (festival_id, drink_id, user_id) +); + +-- Indexes for common queries +CREATE INDEX idx_ratings_drink ON ratings (festival_id, drink_id); +CREATE INDEX idx_ratings_user ON ratings (user_id, festival_id); + +-- Pre-computed aggregates (updated on write via trigger or application code) +CREATE TABLE rating_aggregates ( + festival_id TEXT NOT NULL, + drink_id TEXT NOT NULL, + count INTEGER NOT NULL DEFAULT 0, + sum INTEGER NOT NULL DEFAULT 0, + dist_1 INTEGER NOT NULL DEFAULT 0, + dist_2 INTEGER NOT NULL DEFAULT 0, + dist_3 INTEGER NOT NULL DEFAULT 0, + dist_4 INTEGER NOT NULL DEFAULT 0, + dist_5 INTEGER NOT NULL DEFAULT 0, + updated_at TEXT NOT NULL DEFAULT (datetime('now')), + PRIMARY KEY (festival_id, drink_id) +); +``` + +**Why pre-computed aggregates?** +- The batch endpoint would otherwise require `GROUP BY` across all drinks per request +- Pre-computed table makes batch reads a simple `SELECT *` — fast and cheap +- Updated atomically with each rating insert/update via application logic + +### 1.3 API Endpoints + +Base URL: `https://ratings.cambeerfestival.app` (or `cbf-ratings-api..workers.dev`) + +#### `PUT /api/v1/{festivalId}/drinks/{drinkId}/rating` + +Submit or update a rating. + +**Request:** +```json +{ + "rating": 4 +} +``` + +**Headers:** +``` +Authorization: Bearer +Content-Type: application/json +``` + +**Response (200):** +```json +{ + "userRating": 4, + "average": 4.2, + "count": 37, + "distribution": { "1": 2, "2": 3, "3": 5, "4": 12, "5": 15 } +} +``` + +**Why PUT?** Idempotent — same user rating the same drink twice just updates. Safe to retry on network failure. + +#### `DELETE /api/v1/{festivalId}/drinks/{drinkId}/rating` + +Remove the user's rating. + +**Headers:** +``` +Authorization: Bearer +``` + +**Response (200):** +```json +{ + "userRating": null, + "average": 4.1, + "count": 36, + "distribution": { "1": 2, "2": 3, "3": 5, "4": 11, "5": 15 } +} +``` + +#### `GET /api/v1/{festivalId}/drinks/{drinkId}/rating` + +Get rating stats for a single drink (includes user's own rating if authenticated). + +**Headers:** +``` +Authorization: Bearer (optional) +``` + +**Response (200):** +```json +{ + "userRating": 4, + "average": 4.2, + "count": 37, + "distribution": { "1": 2, "2": 3, "3": 5, "4": 12, "5": 15 } +} +``` + +#### `GET /api/v1/{festivalId}/ratings` + +Batch endpoint — all rating aggregates for a festival. + +**Headers:** +``` +Authorization: Bearer (optional — needed for userRating) +``` + +**Response (200):** +```json +{ + "festivalId": "cbf2025", + "ratings": { + "drink-id-1": { + "userRating": 4, + "average": 4.2, + "count": 37, + "distribution": { "1": 2, "2": 3, "3": 5, "4": 12, "5": 15 } + }, + "drink-id-2": { + "userRating": null, + "average": 3.8, + "count": 12, + "distribution": { "1": 0, "2": 1, "3": 3, "4": 5, "5": 3 } + } + } +} +``` + +**Caching:** Response can be cached for 30-60s with `Cache-Control`. User-specific data (userRating) means this needs `Vary: Authorization` or should be handled client-side by merging batch aggregates with locally-known user ratings. + +### 1.4 Auth Middleware + +The Worker verifies Firebase ID tokens without the Firebase Admin SDK: + +``` +1. Extract token from Authorization: Bearer +2. Decode JWT header to get key ID (kid) +3. Fetch Google's public keys from: + https://www.googleapis.com/robot/v1/metadata/x509/securetoken@system.gserviceaccount.com + (cache for 1 hour) +4. Verify JWT signature using the matching public key +5. Validate claims: + - iss == "https://securetoken.google.com/" + - aud == "" + - exp > now + - sub is non-empty (this is the user_id) +6. Extract user_id = token.sub +``` + +This keeps the Worker self-contained — no Firebase Admin SDK, no Node.js runtime dependency. + +### 1.5 Rate Limiting + +Implemented in-Worker using a simple sliding window per user ID: + +``` +Rule: Max 30 write requests per user per minute +Storage: Cloudflare Worker in-memory (reset per isolate) or D1 table + +On exceeded: + 429 Too Many Requests + { "error": "Rate limit exceeded", "retryAfter": 45 } +``` + +For a festival app, this is sufficient. If abuse becomes a problem, Cloudflare's built-in rate limiting (paid) can be added later. + +### 1.6 Write Path (Rating Submission) + +``` +1. Validate auth token → extract user_id +2. Check rate limit → reject if exceeded +3. Validate request body (rating 1-5) +4. UPSERT into ratings table +5. Update rating_aggregates table: + - If new rating: increment count, add to sum, increment dist_N + - If update: adjust sum and dist_N columns (subtract old, add new) + - If delete: decrement count, subtract from sum, decrement dist_N +6. Return updated aggregate + user's rating +``` + +All done in a single D1 transaction for consistency. + +--- + +## 2. Flutter App Changes + +### 2.1 New Dependencies + +```yaml +# pubspec.yaml +dependencies: + firebase_auth: ^5.3.0 # For anonymous auth (already have firebase_core) +``` + +No other new dependencies needed — HTTP calls use existing Dart `http` package. + +### 2.2 New Service: `OnlineRatingService` + +**Location:** `lib/services/online_rating_service.dart` + +```dart +class OnlineRatingService { + final String baseUrl; + final FirebaseAuth _auth; + + /// Submit or update a rating, returns updated aggregates + Future ratedrink(String festivalId, String drinkId, int rating); + + /// Remove a rating + Future removeRating(String festivalId, String drinkId); + + /// Get rating for a single drink + Future getDrinkRating(String festivalId, String drinkId); + + /// Get all ratings for a festival (batch) + Future> getFestivalRatings(String festivalId); +} +``` + +### 2.3 New Model: `DrinkRatingResult` + +**Location:** `lib/models/drink_rating_result.dart` + +```dart +class DrinkRatingResult { + final int? userRating; // Current user's rating (null if not rated) + final double average; // Community average + final int count; // Total number of ratings + final Map distribution; // { 1: n, 2: n, 3: n, 4: n, 5: n } +} +``` + +### 2.4 Offline Sync Queue + +**Location:** `lib/services/rating_sync_service.dart` + +```dart +class RatingSyncService { + final OnlineRatingService _onlineService; + final RatingsService _localService; // Existing SharedPreferences service + final SharedPreferences _prefs; + + /// Queue a rating for sync (called when offline or as fire-and-forget) + Future queueRating(String festivalId, String drinkId, int rating); + + /// Process pending sync queue (called on connectivity restore) + Future syncPendingRatings(); + + /// Get pending ratings that haven't synced yet + List getPendingRatings(); +} +``` + +**Sync strategy:** +1. User taps a star → local rating saved immediately (existing `RatingsService`) +2. `RatingSyncService.queueRating()` called → adds to pending queue +3. If online: sends to API immediately, clears from queue on success +4. If offline: stays in queue +5. On connectivity change: `syncPendingRatings()` processes the queue +6. On app launch: check and sync any pending ratings + +**Conflict resolution:** Last-write-wins (server timestamp). Simple and appropriate — a user's latest rating is always their intent. + +### 2.5 Firebase Anonymous Auth Integration + +**Location:** `lib/services/auth_service.dart` + +```dart +class AuthService { + final FirebaseAuth _auth = FirebaseAuth.instance; + + /// Sign in anonymously (called on app startup) + Future ensureAuthenticated() async { + if (_auth.currentUser != null) { + return _auth.currentUser!; + } + final credential = await _auth.signInAnonymously(); + return credential.user!; + } + + /// Get current ID token for API calls + Future getIdToken() async { + final user = _auth.currentUser; + if (user == null) throw StateError('Not authenticated'); + return await user.getIdToken() ?? ''; + } +} +``` + +### 2.6 BeerProvider Changes + +Add community rating data alongside existing personal rating: + +```dart +// New state +Map _communityRatings = {}; + +// Enhanced setRating method +Future setRating(Drink drink, int? rating) async { + // 1. Save locally immediately (optimistic) + if (rating == null) { + await _drinkRepository!.removeRating(currentFestival.id, drink.id); + drink.rating = null; + } else { + await _drinkRepository!.setRating(currentFestival.id, drink.id, rating); + drink.rating = rating; + } + notifyListeners(); // UI updates immediately + + // 2. Sync to server in background + _ratingSyncService.queueRating(currentFestival.id, drink.id, rating); +} + +// New method to load community ratings +Future loadCommunityRatings() async { + _communityRatings = await _onlineRatingService + .getFestivalRatings(currentFestival.id); + notifyListeners(); +} +``` + +### 2.7 UI Changes + +**Drink list screen** — Show community average as small stars/number next to each drink. + +**Drink detail screen** — Show: +- User's personal rating (existing star widget, interactive) +- Community average + count (e.g., "4.2 avg from 37 ratings") +- Distribution bar chart (optional, nice-to-have) + +--- + +## 3. Wrangler Configuration + +```toml +# cloudflare-worker/ratings/wrangler.toml +name = "cbf-ratings-api" +main = "src/index.js" +compatibility_date = "2024-01-01" + +[[d1_databases]] +binding = "RATINGS_DB" +database_name = "cbf-ratings" +database_id = "" + +[vars] +FIREBASE_PROJECT_ID = "your-firebase-project-id" +ENVIRONMENT = "production" +``` + +--- + +## 4. Implementation Phases + +### Phase 1: Backend (Cloudflare Worker + D1) +1. Create D1 database and run schema migrations +2. Build ratings Worker with PUT/DELETE/GET single-drink endpoints +3. Add Firebase JWT verification middleware +4. Add rate limiting +5. Deploy and test with curl/Postman +6. Add batch GET endpoint + +### Phase 2: Flutter — Auth + Online Rating +1. Add `firebase_auth` dependency +2. Create `AuthService` with anonymous sign-in +3. Create `OnlineRatingService` (HTTP client) +4. Create `DrinkRatingResult` model +5. Wire into `BeerProvider` — send ratings to API after local save +6. Display community ratings on detail screen + +### Phase 3: Flutter — Offline Sync + Batch +1. Create `RatingSyncService` with pending queue +2. Add connectivity listener for auto-sync +3. Integrate batch endpoint — load community ratings on festival load +4. Show community averages on drink list cards + +### Phase 4: Future — Cross-Device +1. Add Google/Apple sign-in option in settings +2. Firebase Auth `linkWithCredential` to upgrade anonymous → authenticated +3. Server merges ratings from old anonymous UID to new authenticated UID +4. Ratings follow the user across devices + +--- + +## 5. Data Sizing Estimates + +For a typical festival: +- ~500 drinks +- ~5,000 active users +- ~10,000 ratings per festival + +**D1 storage:** ~2 MB per festival (trivial) +**D1 reads:** Batch endpoint = 1 query per page load. At 5K users × 5 loads/day = 25K reads/day (well within free tier of 5M reads/day) +**D1 writes:** 10K ratings over 5 days = ~2K/day (well within free tier of 100K writes/day) + +--- + +## 6. Security Considerations + +| Threat | Mitigation | +|--------|------------| +| Token forgery | JWT signature verification against Google's public keys | +| Rating spam | One rating per user per drink (DB constraint) + rate limiting | +| Data scraping | Aggregates only, no individual user data exposed | +| Token replay | Short-lived Firebase tokens (1 hour), verified server-side | +| SQL injection | D1 parameterized queries (no string concatenation) | +| CORS abuse | Same origin whitelist as existing worker | + +--- + +## 7. Open Questions + +1. **Custom domain?** `ratings.cambeerfestival.app` vs just `cbf-ratings-api.workers.dev` +2. **Minimum ratings to show average?** Hide average until N ratings (e.g., 3) to prevent one person skewing display? +3. **Festival archival?** Keep ratings readable after festival ends? Or archive/delete? +4. **Distribution bar chart?** Worth the UI effort in v1 or defer? From 30232f1b34cfcbffe3f54639d19e4fddfa3cd79f Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 7 Feb 2026 22:03:20 +0000 Subject: [PATCH 2/8] Add staged releases, testing strategy, and data proxy tests to plan - Three environments: dev (PRs), staging (main), production (tags) - Each with separate D1 database and Worker deployment - Phase 0: add tests to existing data proxy Worker (CORS, parsing) - Vitest + Miniflare testing pattern for both Workers - Flutter unit/widget/E2E test coverage for rating features - CI pipeline with test gates before each deployment stage https://claude.ai/code/session_01SusUN4s9TQ4AqeoyoGAGYa --- docs/planning/rating-service/design.md | 368 ++++++++++++++++++++++++- 1 file changed, 357 insertions(+), 11 deletions(-) diff --git a/docs/planning/rating-service/design.md b/docs/planning/rating-service/design.md index c0394bfd..d3aba336 100644 --- a/docs/planning/rating-service/design.md +++ b/docs/planning/rating-service/design.md @@ -431,15 +431,359 @@ ENVIRONMENT = "production" --- -## 4. Implementation Phases +## 4. Staged Releases -### Phase 1: Backend (Cloudflare Worker + D1) -1. Create D1 database and run schema migrations -2. Build ratings Worker with PUT/DELETE/GET single-drink endpoints -3. Add Firebase JWT verification middleware -4. Add rate limiting -5. Deploy and test with curl/Postman -6. Add batch GET endpoint +### Environments + +The ratings Worker follows the same staging model as the existing app: + +| Environment | Worker Name | D1 Database | URL | Deployed When | +|-------------|-------------|-------------|-----|---------------| +| **Dev/Preview** | `cbf-ratings-api-dev` | `cbf-ratings-dev` | `ratings-dev.cambeerfestival.app` | PR branches, local dev | +| **Staging** | `cbf-ratings-api-staging` | `cbf-ratings-staging` | `ratings-staging.cambeerfestival.app` | Merge to `main` | +| **Production** | `cbf-ratings-api` | `cbf-ratings` | `ratings.cambeerfestival.app` | Version tags (`v*`) | + +**Each environment gets its own D1 database** — no risk of test data polluting production. + +### Wrangler Multi-Environment Config + +```toml +# cloudflare-worker/ratings/wrangler.toml +name = "cbf-ratings-api" +main = "src/index.js" +compatibility_date = "2024-01-01" + +[vars] +FIREBASE_PROJECT_ID = "your-firebase-project-id" + +# Production (default) +[[d1_databases]] +binding = "RATINGS_DB" +database_name = "cbf-ratings" +database_id = "" + +[env.staging] +name = "cbf-ratings-api-staging" +[env.staging.vars] +FIREBASE_PROJECT_ID = "your-firebase-project-id" +[[env.staging.d1_databases]] +binding = "RATINGS_DB" +database_name = "cbf-ratings-staging" +database_id = "" + +[env.dev] +name = "cbf-ratings-api-dev" +[env.dev.vars] +FIREBASE_PROJECT_ID = "your-firebase-project-id" +[[env.dev.d1_databases]] +binding = "RATINGS_DB" +database_name = "cbf-ratings-dev" +database_id = "" +``` + +### Flutter Environment Routing + +The app already detects environment via `EnvironmentService`. The ratings API base URL follows the same pattern: + +```dart +String get ratingsApiBaseUrl { + if (EnvironmentService.isProduction()) { + return 'https://ratings.cambeerfestival.app'; + } else if (EnvironmentService.isStaging()) { + return 'https://ratings-staging.cambeerfestival.app'; + } else { + return 'https://ratings-dev.cambeerfestival.app'; + } +} +``` + +### Database Migrations + +D1 schema changes need care across environments: + +- Keep a `cloudflare-worker/ratings/migrations/` folder with numbered SQL files +- Migrations run **dev → staging → production** (never skip) +- Migrations must be **backwards-compatible** (add columns, don't remove/rename) so the old Worker code still works during rollout +- CI validates migration syntax before deployment + +``` +migrations/ +├── 0001_create_ratings.sql +├── 0002_create_aggregates.sql +└── 0003_add_index.sql +``` + +### Deployment Flow + +``` +PR opened/updated: + 1. Run Worker unit tests (Vitest + Miniflare) + 2. Deploy to cbf-ratings-api-dev + 3. Run integration tests against dev Worker + 4. PR preview app (staging-cambeerfestival.pages.dev) hits dev ratings API + +Merge to main: + 1. Run Worker unit tests + 2. Run D1 migrations on staging DB + 3. Deploy to cbf-ratings-api-staging + 4. Run integration tests against staging + 5. App deploys to staging.cambeerfestival.app (hits staging ratings API) + +Version tag (v*): + 1. Run D1 migrations on production DB + 2. Deploy to cbf-ratings-api (production) + 3. App deploys to cambeerfestival.app (hits production ratings API) +``` + +--- + +## 5. Testing Strategy + +### 5.0 Phase 0 — Tests for Existing Data Proxy Worker + +The existing `cloudflare-worker/worker.js` has no tests. Adding tests here first: +- Establishes the Worker testing pattern (Vitest + Miniflare) before building the ratings Worker +- Catches regressions in CORS logic (security-relevant) +- Enables safe extraction of shared code (CORS utils) for the ratings Worker + +**Test areas for existing Worker:** + +| Area | Priority | What to test | +|------|----------|-------------| +| CORS origin matching | **High** | Allowed origins accepted, wildcard `.pages.dev` patterns, unknown origins rejected | +| CORS preflight | **High** | Correct headers returned, max-age differs for staging vs production | +| Beverage type parsing | Medium | HTML directory listing correctly parsed to JSON array | +| Festivals endpoint | Medium | Returns valid JSON, correct cache headers | +| Upstream proxy | Low | Error handling (502), charset enforcement, header passthrough | +| Health check | Low | Returns `{ status: "ok" }` | + +**Setup:** + +``` +cloudflare-worker/ +├── worker.js # Existing (unchanged) +├── wrangler.toml # Existing (unchanged) +├── package.json # NEW — add vitest, miniflare +├── vitest.config.js # NEW — Miniflare environment +└── test/ + ├── cors.test.js # Origin matching, preflight + ├── festivals.test.js # festivals.json endpoint + ├── beverage-types.test.js # Directory listing parser + └── proxy.test.js # Upstream proxy behaviour +``` + +**Example test (CORS origin matching):** + +```js +import { describe, it, expect } from 'vitest'; +import worker from '../worker.js'; + +describe('CORS', () => { + it('allows production origin', async () => { + const request = new Request('https://worker.example.com/health', { + headers: { 'Origin': 'https://cambeerfestival.app' }, + }); + const response = await worker.fetch(request); + expect(response.headers.get('Access-Control-Allow-Origin')) + .toBe('https://cambeerfestival.app'); + }); + + it('allows Cloudflare Pages preview URLs', async () => { + const request = new Request('https://worker.example.com/health', { + headers: { 'Origin': 'https://abc123.cambeerfestival.pages.dev' }, + }); + const response = await worker.fetch(request); + expect(response.headers.get('Access-Control-Allow-Origin')) + .toBe('https://abc123.cambeerfestival.pages.dev'); + }); + + it('rejects unknown origins', async () => { + const request = new Request('https://worker.example.com/health', { + headers: { 'Origin': 'https://evil.example.com' }, + }); + const response = await worker.fetch(request); + expect(response.headers.get('Access-Control-Allow-Origin')).toBeNull(); + }); + + it('returns short max-age for staging preflight', async () => { + const request = new Request('https://worker.example.com/', { + method: 'OPTIONS', + headers: { 'Origin': 'https://staging.cambeerfestival.app' }, + }); + const response = await worker.fetch(request); + expect(response.headers.get('Access-Control-Max-Age')).toBe('10'); + }); +}); +``` + +**CI integration:** Add a `test-worker` job to the existing `deploy-worker.yml` workflow, running before the deploy step. + +### 5.1 Ratings Worker — Unit Tests + +**Framework:** Vitest + Miniflare (standard for Cloudflare Workers) + +Miniflare provides local D1 (in-memory SQLite), so tests run without network calls. + +``` +cloudflare-worker/ratings/ +├── src/ +│ ├── index.js # Worker entry, router +│ ├── auth.js # JWT verification +│ ├── ratings.js # Rating CRUD + aggregation +│ ├── rate-limit.js # Frequency limiting +│ └── cors.js # Shared CORS utilities +├── test/ +│ ├── auth.test.js # JWT verification, token edge cases, expired tokens +│ ├── ratings.test.js # CRUD, aggregate math, upsert behaviour +│ ├── rate-limit.test.js # Frequency limiting, window reset +│ ├── batch.test.js # Batch endpoint, empty festival, large result sets +│ ├── cors.test.js # Origin matching (shared with data proxy) +│ └── integration.test.js # Full request→response cycle with D1 +├── migrations/ +│ └── 0001_initial.sql +├── vitest.config.js +├── package.json +└── wrangler.toml +``` + +**Key test scenarios:** + +| Test | What it verifies | +|------|-----------------| +| Submit first rating | Creates rating + aggregate row, returns correct stats | +| Update existing rating | Aggregate adjusts (old subtracted, new added) | +| Delete rating | Aggregate decrements, userRating returns null | +| Concurrent ratings on same drink | Aggregates stay consistent | +| Rating out of range (0, 6, -1) | Returns 400 | +| Missing/invalid auth token | Returns 401 | +| Expired auth token | Returns 401 | +| Rate limit exceeded | Returns 429 with retryAfter | +| Batch with no ratings | Returns empty map | +| Batch with 500 drinks | Returns within reasonable time | + +### 5.2 Ratings Worker — Integration Tests + +Run against a live Worker (dev environment) with real HTTP requests: + +```bash +# Deploy to dev +wrangler deploy --env dev + +# Run integration tests against live endpoint +RATINGS_API_URL=https://ratings-dev.cambeerfestival.app npm run test:integration +``` + +Tests use a real Firebase Anonymous Auth token to verify the full auth flow end-to-end. + +### 5.3 Flutter — Unit Tests + +Mock `OnlineRatingService` (follows existing pattern used for `BeerApiService`): + +| Test file | What it tests | +|-----------|--------------| +| `test/services/online_rating_service_test.dart` | HTTP calls, JSON parsing, error handling | +| `test/services/rating_sync_service_test.dart` | Queue/dequeue, retry logic, conflict resolution | +| `test/services/auth_service_test.dart` | Anonymous sign-in, token refresh | +| `test/models/drink_rating_result_test.dart` | JSON deserialization, edge cases | +| `test/providers/beer_provider_rating_test.dart` | Optimistic update, sync trigger, community ratings | + +### 5.4 Flutter — Widget Tests + +| Test | What it tests | +|------|--------------| +| Community rating display | Average + count render correctly on drink detail | +| Star rating interaction | Tap star → local update → sync queued | +| Offline indicator | Rating saved locally when offline, no error shown | +| List screen averages | Community stars appear on drink cards | + +### 5.5 E2E Tests + +Extend existing Playwright suite: + +```typescript +// test-e2e/ratings.spec.ts +test('drink detail shows community rating', async ({ page }) => { + await page.goto('/drink/some-drink-id'); + // Verify community rating section renders + await expect(page.getByLabel(/community rating/i)).toBeVisible(); +}); + +test('star rating is interactive', async ({ page }) => { + await page.goto('/drink/some-drink-id'); + // Verify star rating widget has correct ARIA labels + await expect(page.getByLabel(/rate this drink/i)).toBeVisible(); +}); +``` + +E2E tests run against the dev ratings API in CI. + +### 5.6 CI Pipeline for Ratings Worker + +**New workflow: `.github/workflows/deploy-ratings-worker.yml`** + +```yaml +name: Ratings Worker +on: + push: + branches: [main] + paths: ['cloudflare-worker/ratings/**'] + pull_request: + paths: ['cloudflare-worker/ratings/**'] + +jobs: + test: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - uses: actions/setup-node@v4 + - run: npm ci + working-directory: cloudflare-worker/ratings + - run: npm test + working-directory: cloudflare-worker/ratings + + deploy-dev: + if: github.event_name == 'pull_request' + needs: test + runs-on: ubuntu-latest + steps: + - run: wrangler deploy --env dev + - run: npm run test:integration # Against live dev endpoint + + deploy-staging: + if: github.ref == 'refs/heads/main' + needs: test + runs-on: ubuntu-latest + steps: + - run: wrangler d1 migrations apply cbf-ratings-staging --env staging + - run: wrangler deploy --env staging + - run: npm run test:integration # Against live staging endpoint + + # Production deployment triggered by release-web.yml or separate release workflow +``` + +--- + +## 6. Implementation Phases + +### Phase 0: Test Existing Data Proxy Worker +1. Add `package.json` with Vitest + Miniflare to `cloudflare-worker/` +2. Write CORS, preflight, and origin matching tests +3. Write beverage type parsing tests +4. Write festivals endpoint tests +5. Add `test-worker` job to `deploy-worker.yml` CI workflow +6. Extract shared CORS utilities for reuse by ratings Worker + +### Phase 1: Ratings Worker Backend +1. Create D1 databases (dev, staging, production) +2. Write and run schema migrations +3. Build ratings Worker with PUT/DELETE/GET single-drink endpoints +4. Add Firebase JWT verification middleware +5. Add rate limiting +6. Write unit tests (Vitest + Miniflare) +7. Set up CI workflow (`deploy-ratings-worker.yml`) +8. Deploy to dev, test with curl/integration tests +9. Add batch GET endpoint +10. Deploy to staging ### Phase 2: Flutter — Auth + Online Rating 1. Add `firebase_auth` dependency @@ -448,12 +792,14 @@ ENVIRONMENT = "production" 4. Create `DrinkRatingResult` model 5. Wire into `BeerProvider` — send ratings to API after local save 6. Display community ratings on detail screen +7. Write unit + widget tests ### Phase 3: Flutter — Offline Sync + Batch 1. Create `RatingSyncService` with pending queue 2. Add connectivity listener for auto-sync 3. Integrate batch endpoint — load community ratings on festival load 4. Show community averages on drink list cards +5. Extend Playwright E2E tests ### Phase 4: Future — Cross-Device 1. Add Google/Apple sign-in option in settings @@ -463,7 +809,7 @@ ENVIRONMENT = "production" --- -## 5. Data Sizing Estimates +## 8. Data Sizing Estimates For a typical festival: - ~500 drinks @@ -476,7 +822,7 @@ For a typical festival: --- -## 6. Security Considerations +## 9. Security Considerations | Threat | Mitigation | |--------|------------| @@ -489,7 +835,7 @@ For a typical festival: --- -## 7. Open Questions +## 10. Open Questions 1. **Custom domain?** `ratings.cambeerfestival.app` vs just `cbf-ratings-api.workers.dev` 2. **Minimum ratings to show average?** Hide average until N ratings (e.g., 3) to prevent one person skewing display? From ee5d5f50877ad7213e999ae37588892ee3e363ba Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 7 Feb 2026 22:32:58 +0000 Subject: [PATCH 3/8] Resolve open questions: custom domain, min ratings, archival - Custom domain: yes, ratings.cambeerfestival.app for all environments - Min ratings: server-configurable threshold, 0 for testing, tunable for prod - Archival: read-only after festival end date, block new writes - Distribution bar chart: defer to v2 https://claude.ai/code/session_01SusUN4s9TQ4AqeoyoGAGYa --- docs/planning/rating-service/design.md | 22 +++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/docs/planning/rating-service/design.md b/docs/planning/rating-service/design.md index d3aba336..2184fa60 100644 --- a/docs/planning/rating-service/design.md +++ b/docs/planning/rating-service/design.md @@ -835,9 +835,21 @@ For a typical festival: --- -## 10. Open Questions +## 10. Resolved Questions -1. **Custom domain?** `ratings.cambeerfestival.app` vs just `cbf-ratings-api.workers.dev` -2. **Minimum ratings to show average?** Hide average until N ratings (e.g., 3) to prevent one person skewing display? -3. **Festival archival?** Keep ratings readable after festival ends? Or archive/delete? -4. **Distribution bar chart?** Worth the UI effort in v1 or defer? +1. **Custom domain?** Yes — `ratings.cambeerfestival.app` (+ `ratings-staging`, `ratings-dev`). Simplifies CORS since `*.cambeerfestival.app` patterns are already handled. + +2. **Minimum ratings to show average?** Server-configurable threshold. The API returns the raw data (average, count, distribution) regardless. The **client** decides whether to display the average based on a configurable minimum (default: 0 for testing, raise to 3-5 for production). This keeps testing easy while allowing the threshold to be tuned without a code change. + + ```dart + // Config: minimum ratings before showing community average + static const int minRatingsToShowAverage = 0; // 0 for dev/testing, 3+ for production + ``` + +3. **Festival archival?** Keep readable, block writes after festival end date. The API checks the festival's end date and returns `403 Festival ended` for PUT/DELETE requests after that date. GET requests (single + batch) remain available indefinitely so users can look back at what they enjoyed. + +4. **Distribution bar chart?** Defer to v2. Not needed for launch — average + count is enough. Revisit once there's real usage data. + +## 11. Open Questions + +1. **Shared CORS module?** Extract to a shared package (`cloudflare-worker/shared/cors.js`) or duplicate between workers? Shared is cleaner but adds a build step. From 87ae39f840a151bf81b1d427cde89422ba9f1477 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 7 Feb 2026 22:35:00 +0000 Subject: [PATCH 4/8] Resolve shared CORS question: duplicate now, extract later Type 2 decision - start simple with copy in each Worker, refactor to shared module if maintenance burden emerges. https://claude.ai/code/session_01SusUN4s9TQ4AqeoyoGAGYa --- docs/planning/rating-service/design.md | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/docs/planning/rating-service/design.md b/docs/planning/rating-service/design.md index 2184fa60..b902e4e4 100644 --- a/docs/planning/rating-service/design.md +++ b/docs/planning/rating-service/design.md @@ -850,6 +850,4 @@ For a typical festival: 4. **Distribution bar chart?** Defer to v2. Not needed for launch — average + count is enough. Revisit once there's real usage data. -## 11. Open Questions - -1. **Shared CORS module?** Extract to a shared package (`cloudflare-worker/shared/cors.js`) or duplicate between workers? Shared is cleaner but adds a build step. +5. **Shared CORS module?** Start with duplication between the two Workers. Extract to a shared module later if they diverge or become a maintenance burden. Not a one-way door — easy to refactor. From 348d01cd0169d7f7df5b5b1a83aad2bf701bab7f Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 8 Feb 2026 09:56:04 +0000 Subject: [PATCH 5/8] Add risk assessment and critical review of rating service plan Identifies 14 risks across high/medium/low severity: - JWT verification complexity (spike candidate) - Aggregate table consistency (needs recompute safety net) - Firebase Auth web persistence limitations - Missing EnvironmentService.isStaging() method - Festival end-date source undefined for ratings Worker - Batch endpoint caching problem - Rate limiting won't work in-memory - Existing local ratings migration gap - Bundle size impact of firebase_auth on web Includes 5 pre-Phase-1 actions to reduce risk. https://claude.ai/code/session_01SusUN4s9TQ4AqeoyoGAGYa --- docs/planning/rating-service/design.md | 127 +++++++++++++++++++++++++ 1 file changed, 127 insertions(+) diff --git a/docs/planning/rating-service/design.md b/docs/planning/rating-service/design.md index b902e4e4..ec56db62 100644 --- a/docs/planning/rating-service/design.md +++ b/docs/planning/rating-service/design.md @@ -851,3 +851,130 @@ For a typical festival: 4. **Distribution bar chart?** Defer to v2. Not needed for launch — average + count is enough. Revisit once there's real usage data. 5. **Shared CORS module?** Start with duplication between the two Workers. Extract to a shared module later if they diverge or become a maintenance burden. Not a one-way door — easy to refactor. + +--- + +## 11. Risk Assessment & Review + +### Bugs in the Plan + +**1. `EnvironmentService.isStaging()` doesn't exist** + +The plan references `EnvironmentService.isStaging()` in the Flutter environment routing (section 4), but the actual class only has `isProduction()` and `getEnvironmentName()`. Need to either add `isStaging()` or use `getEnvironmentName() == 'staging'`. + +**2. Mobile always routes to production** + +`EnvironmentService` treats all mobile platforms as production (`return true` in `isProduction()`). This means there's no way to test the ratings API staging environment on Android without a code change. If mobile staging is needed, this needs addressing. + +**3. Festival end date — where does the Worker get it?** + +The plan says the API blocks writes after festival end date (resolved question 3), but the ratings Worker has no access to festival metadata. `festivals.json` lives in the data proxy Worker. Options: +- Store festival dates in D1 (extra migration + sync) +- Worker env var per festival (manual, doesn't scale) +- Ratings Worker fetches from data proxy on startup (adds a dependency) +- Pass festival end date from the client (insecure — client can lie) + +This needs a design decision before Phase 1. + +### High Risk + +**4. JWT verification in Cloudflare Workers — hardest piece of the backend** + +The plan describes JWT verification as 6 clean steps, but the implementation is non-trivial: +- Workers use the Web Crypto API (not Node.js `crypto`) +- Google's public keys are X.509 PEM certificates — need to parse ASN.1 to extract the RSA public key for `crypto.subtle.verify()` +- Key rotation: Google rotates keys; the Worker must handle `kid` lookup and cache invalidation +- This is the most complex piece of the Worker and the most security-critical + +**Mitigation:** Use the `jose` npm library (works in Workers, handles all of this). Don't hand-roll JWT verification. Write thorough tests with real and forged tokens. This is a **spike candidate** — build it first in isolation and verify it works before building the rest. + +**5. Aggregate table consistency** + +The write path does: UPSERT rating → UPDATE aggregate. If the second statement fails, the aggregate is wrong forever (silent data corruption). The plan says "single D1 transaction" but D1's transaction model has quirks: +- `D1.batch()` executes statements sequentially in one transaction — this should work +- But: if aggregates ever drift, there's no self-healing mechanism + +**Mitigation:** Add a `/admin/recompute-aggregates` endpoint (or scheduled cron) that rebuilds aggregates from the ratings table. Run it periodically or on-demand as a safety net. Cheap insurance. + +**6. Firebase Anonymous Auth persistence on web is fragile** + +Firebase Auth on web stores identity in IndexedDB. This means: +- **Private/incognito browsing:** New anonymous user every session. They lose their "one vote per drink" identity. +- **Clearing browser data:** Identity lost, gets a new UID, can rate again. +- **Different browsers on same device:** Different identity per browser. + +This weakens the "anonymous but tracked" guarantee on web. Not a dealbreaker for a festival app (determined fraudsters aren't the threat model), but worth knowing. + +**Mitigation:** Accept the limitation. Document it. If duplicate voting becomes visible in the data, the min-ratings threshold hides the impact. For determined abuse, the rate limiter still applies per-session. + +### Medium Risk + +**7. Existing local ratings migration** + +Users already have local ratings (via `RatingsService` / SharedPreferences). When online ratings launches, these local ratings won't automatically sync to the server. Users who rated drinks before the feature goes live will see their personal ratings but won't contribute to community aggregates. + +**Mitigation:** On first authenticated session, scan local ratings and bulk-upload them to the API. Add this as a step in Phase 2 or Phase 3. Design the API to accept batch PUT for this purpose, or just iterate through them client-side. + +**8. Batch endpoint + user ratings = caching problem** + +The batch endpoint returns `userRating` per drink, which is user-specific. This means: +- Can't use CDN/edge caching (every user gets different data) +- `Vary: Authorization` effectively disables caching + +At ~5K users this is fine (D1 handles it easily), but it's an architectural smell. + +**Mitigation:** Split the batch endpoint into two concerns: +- `GET /api/v1/{festivalId}/ratings` — returns aggregates only (cacheable, 60s TTL) +- `GET /api/v1/{festivalId}/ratings/mine` — returns just the user's ratings (small, fast, auth required) + +The client merges them locally. This keeps the common path (aggregates) cacheable. Decide during Phase 1 — not urgent now. + +**9. Rate limiting via in-memory storage won't work** + +The plan says rate limiting can use "in-memory (reset per isolate)". Workers are stateless — each request can hit a different V8 isolate. In-memory counters reset constantly and provide almost no protection. + +**Mitigation:** Use D1 for rate limiting (simple `rate_limits` table with user_id + window timestamp), or use Cloudflare's built-in rate limiting rules (free tier allows 1 rule). D1 adds one extra read+write per request but is reliable. For a festival app's write volume (~2K/day), the overhead is negligible. + +**10. D1 write concurrency under burst load** + +D1 is built on SQLite (single-writer). At a festival, bursts could happen (e.g., a popular new beer tapped, 200 people rate it in 5 minutes). D1 should handle this fine — 200 writes over 5 minutes is ~0.7/second — but worth knowing the ceiling. + +**Mitigation:** None needed at current scale. Monitor D1 latency in production. If it becomes an issue (unlikely), writes could be buffered through a Durable Object. + +**11. Bundle size — adding `firebase_auth`** + +The app is Flutter web-first. Adding `firebase_auth` adds the Firebase Auth JS SDK to the web bundle. This can add 50-100KB gzipped to the initial load. For a festival app on potentially slow mobile connections, this matters. + +**Mitigation:** Measure before and after. Consider lazy-loading the auth initialization (don't block app startup on auth). Auth is only needed when the user first rates — not on initial page load. + +### Low Risk (but worth noting) + +**12. Drink ID stability** + +Ratings are keyed by `drink_id` from the upstream API. If the festival data provider changes drink IDs between data refreshes (e.g., re-publishing the beer list), ratings would be orphaned. Looking at the current data, IDs appear to be `json['id'].toString()` from the API — likely stable, but we don't control this. + +**Mitigation:** Accept the risk. If it happens, the aggregates recompute endpoint (from #5) can help clean up. Could also log a warning if we detect drink IDs changing. + +**13. No monitoring/observability mentioned** + +The plan has no mention of how we'll know if the ratings API is healthy, error rates, latency, or D1 approaching limits. + +**Mitigation:** Add basic observability: +- Health check endpoint (like existing Worker) +- Cloudflare Analytics (built-in, free) for request rates and error rates +- Log errors to a simple D1 `error_log` table or use `console.error` (visible in Cloudflare dashboard) +- Alert on elevated 500 rates (Cloudflare Notifications, free) + +**14. CORS for `ratings.cambeerfestival.app` — not actually in the existing whitelist** + +The existing data proxy Worker allows `cambeerfestival.app` and `staging.cambeerfestival.app`. But `ratings.cambeerfestival.app` is a *different* origin serving the API, not calling it. The ratings Worker needs its own CORS config allowing the *app* origins to call it. This should work fine with the duplicated CORS code — just calling it out so it's not forgotten. + +### Summary: What to do before starting Phase 1 + +| # | Action | Effort | +|---|--------|--------| +| 1 | **Spike JWT verification** — build a minimal Worker that verifies a Firebase token using `jose`. Proves the hardest piece works. | 2-3 hours | +| 2 | **Decide festival end-date source** — how does the ratings Worker know when a festival ends? | 15 min decision | +| 3 | **Decide batch endpoint caching strategy** — split into aggregates + user-ratings, or accept no caching? | 15 min decision | +| 4 | **Decide rate limiting storage** — D1 table or Cloudflare rate limiting rules? | 15 min decision | +| 5 | **Plan local ratings migration** — add to Phase 2 or 3 scope | 15 min | From 057e80ad9c9d3b1133d6684fbd599653b17eac77 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 8 Feb 2026 09:59:02 +0000 Subject: [PATCH 6/8] =?UTF-8?q?Remove=20local=20ratings=20migration=20risk?= =?UTF-8?q?=20=E2=80=94=20launches=20before=20next=20festival?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Existing local ratings are for past festivals. Online ratings will launch with a new festival so everyone starts fresh. No migration needed. https://claude.ai/code/session_01SusUN4s9TQ4AqeoyoGAGYa --- docs/planning/rating-service/design.md | 21 +++++++-------------- 1 file changed, 7 insertions(+), 14 deletions(-) diff --git a/docs/planning/rating-service/design.md b/docs/planning/rating-service/design.md index ec56db62..1fd021ad 100644 --- a/docs/planning/rating-service/design.md +++ b/docs/planning/rating-service/design.md @@ -909,13 +909,7 @@ This weakens the "anonymous but tracked" guarantee on web. Not a dealbreaker for ### Medium Risk -**7. Existing local ratings migration** - -Users already have local ratings (via `RatingsService` / SharedPreferences). When online ratings launches, these local ratings won't automatically sync to the server. Users who rated drinks before the feature goes live will see their personal ratings but won't contribute to community aggregates. - -**Mitigation:** On first authenticated session, scan local ratings and bulk-upload them to the API. Add this as a step in Phase 2 or Phase 3. Design the API to accept batch PUT for this purpose, or just iterate through them client-side. - -**8. Batch endpoint + user ratings = caching problem** +**7. Batch endpoint + user ratings = caching problem** The batch endpoint returns `userRating` per drink, which is user-specific. This means: - Can't use CDN/edge caching (every user gets different data) @@ -929,19 +923,19 @@ At ~5K users this is fine (D1 handles it easily), but it's an architectural smel The client merges them locally. This keeps the common path (aggregates) cacheable. Decide during Phase 1 — not urgent now. -**9. Rate limiting via in-memory storage won't work** +**8. Rate limiting via in-memory storage won't work** The plan says rate limiting can use "in-memory (reset per isolate)". Workers are stateless — each request can hit a different V8 isolate. In-memory counters reset constantly and provide almost no protection. **Mitigation:** Use D1 for rate limiting (simple `rate_limits` table with user_id + window timestamp), or use Cloudflare's built-in rate limiting rules (free tier allows 1 rule). D1 adds one extra read+write per request but is reliable. For a festival app's write volume (~2K/day), the overhead is negligible. -**10. D1 write concurrency under burst load** +**9. D1 write concurrency under burst load** D1 is built on SQLite (single-writer). At a festival, bursts could happen (e.g., a popular new beer tapped, 200 people rate it in 5 minutes). D1 should handle this fine — 200 writes over 5 minutes is ~0.7/second — but worth knowing the ceiling. **Mitigation:** None needed at current scale. Monitor D1 latency in production. If it becomes an issue (unlikely), writes could be buffered through a Durable Object. -**11. Bundle size — adding `firebase_auth`** +**10. Bundle size — adding `firebase_auth`** The app is Flutter web-first. Adding `firebase_auth` adds the Firebase Auth JS SDK to the web bundle. This can add 50-100KB gzipped to the initial load. For a festival app on potentially slow mobile connections, this matters. @@ -949,13 +943,13 @@ The app is Flutter web-first. Adding `firebase_auth` adds the Firebase Auth JS S ### Low Risk (but worth noting) -**12. Drink ID stability** +**11. Drink ID stability** Ratings are keyed by `drink_id` from the upstream API. If the festival data provider changes drink IDs between data refreshes (e.g., re-publishing the beer list), ratings would be orphaned. Looking at the current data, IDs appear to be `json['id'].toString()` from the API — likely stable, but we don't control this. **Mitigation:** Accept the risk. If it happens, the aggregates recompute endpoint (from #5) can help clean up. Could also log a warning if we detect drink IDs changing. -**13. No monitoring/observability mentioned** +**12. No monitoring/observability mentioned** The plan has no mention of how we'll know if the ratings API is healthy, error rates, latency, or D1 approaching limits. @@ -965,7 +959,7 @@ The plan has no mention of how we'll know if the ratings API is healthy, error r - Log errors to a simple D1 `error_log` table or use `console.error` (visible in Cloudflare dashboard) - Alert on elevated 500 rates (Cloudflare Notifications, free) -**14. CORS for `ratings.cambeerfestival.app` — not actually in the existing whitelist** +**13. CORS for `ratings.cambeerfestival.app` — not actually in the existing whitelist** The existing data proxy Worker allows `cambeerfestival.app` and `staging.cambeerfestival.app`. But `ratings.cambeerfestival.app` is a *different* origin serving the API, not calling it. The ratings Worker needs its own CORS config allowing the *app* origins to call it. This should work fine with the duplicated CORS code — just calling it out so it's not forgotten. @@ -977,4 +971,3 @@ The existing data proxy Worker allows `cambeerfestival.app` and `staging.cambeer | 2 | **Decide festival end-date source** — how does the ratings Worker know when a festival ends? | 15 min decision | | 3 | **Decide batch endpoint caching strategy** — split into aggregates + user-ratings, or accept no caching? | 15 min decision | | 4 | **Decide rate limiting storage** — D1 table or Cloudflare rate limiting rules? | 15 min decision | -| 5 | **Plan local ratings migration** — add to Phase 2 or 3 scope | 15 min | From 7bff469e63d5c0015cd29ea82d2f9928036300a9 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 8 Feb 2026 14:14:11 +0000 Subject: [PATCH 7/8] Resolve rate limiting and batch caching decisions Rate limiting: launch with DB constraint only (UNIQUE per user/drink), upgrade to D1 rate-limit table if abuse observed. Batch endpoint: aggregates only (no per-user data), fully cacheable with 60s TTL. Client uses local ratings. Add /mine in Phase 4. Two pre-Phase-1 items remain: JWT spike and festival end-date source. https://claude.ai/code/session_01SusUN4s9TQ4AqeoyoGAGYa --- docs/planning/rating-service/design.md | 58 ++++++++------------------ 1 file changed, 17 insertions(+), 41 deletions(-) diff --git a/docs/planning/rating-service/design.md b/docs/planning/rating-service/design.md index 1fd021ad..ec614030 100644 --- a/docs/planning/rating-service/design.md +++ b/docs/planning/rating-service/design.md @@ -179,12 +179,7 @@ Authorization: Bearer (optional) #### `GET /api/v1/{festivalId}/ratings` -Batch endpoint — all rating aggregates for a festival. - -**Headers:** -``` -Authorization: Bearer (optional — needed for userRating) -``` +Batch endpoint — community aggregates for all drinks at a festival. No auth required. No per-user data. **Response (200):** ```json @@ -192,13 +187,11 @@ Authorization: Bearer (optional — needed for userRating) "festivalId": "cbf2025", "ratings": { "drink-id-1": { - "userRating": 4, "average": 4.2, "count": 37, "distribution": { "1": 2, "2": 3, "3": 5, "4": 12, "5": 15 } }, "drink-id-2": { - "userRating": null, "average": 3.8, "count": 12, "distribution": { "1": 0, "2": 1, "3": 3, "4": 5, "5": 3 } @@ -207,7 +200,9 @@ Authorization: Bearer (optional — needed for userRating) } ``` -**Caching:** Response can be cached for 30-60s with `Cache-Control`. User-specific data (userRating) means this needs `Vary: Authorization` or should be handled client-side by merging batch aggregates with locally-known user ratings. +**Caching:** Fully cacheable — `Cache-Control: public, max-age=60`. Same response for all users. The client already knows the user's own ratings from local storage (SharedPreferences). + +**Upgrade path (Phase 4 — cross-device):** Add `GET /api/v1/{festivalId}/ratings/mine` to return the user's ratings from the server, for syncing across devices. ### 1.4 Auth Middleware @@ -232,18 +227,11 @@ This keeps the Worker self-contained — no Firebase Admin SDK, no Node.js runti ### 1.5 Rate Limiting -Implemented in-Worker using a simple sliding window per user ID: - -``` -Rule: Max 30 write requests per user per minute -Storage: Cloudflare Worker in-memory (reset per isolate) or D1 table +**Launch approach:** Rely on the DB `UNIQUE` constraint (one rating per user per drink). No additional rate limiting for v1. -On exceeded: - 429 Too Many Requests - { "error": "Rate limit exceeded", "retryAfter": 45 } -``` +The constraint prevents duplicate ratings. At festival scale (~2K writes/day), even a bot hammering updates on the same rating can't corrupt data or exhaust D1's free tier (100K writes/day). -For a festival app, this is sufficient. If abuse becomes a problem, Cloudflare's built-in rate limiting (paid) can be added later. +**Upgrade path:** If abuse is observed in Cloudflare Analytics, add a D1 `rate_limits` table with per-user sliding window. Backwards-compatible — just an extra middleware check. ### 1.6 Write Path (Rating Submission) @@ -909,25 +897,13 @@ This weakens the "anonymous but tracked" guarantee on web. Not a dealbreaker for ### Medium Risk -**7. Batch endpoint + user ratings = caching problem** - -The batch endpoint returns `userRating` per drink, which is user-specific. This means: -- Can't use CDN/edge caching (every user gets different data) -- `Vary: Authorization` effectively disables caching - -At ~5K users this is fine (D1 handles it easily), but it's an architectural smell. - -**Mitigation:** Split the batch endpoint into two concerns: -- `GET /api/v1/{festivalId}/ratings` — returns aggregates only (cacheable, 60s TTL) -- `GET /api/v1/{festivalId}/ratings/mine` — returns just the user's ratings (small, fast, auth required) - -The client merges them locally. This keeps the common path (aggregates) cacheable. Decide during Phase 1 — not urgent now. +**7. ~~Batch endpoint + user ratings = caching problem~~ RESOLVED** -**8. Rate limiting via in-memory storage won't work** +Batch endpoint now returns aggregates only (no per-user data). Fully cacheable with 60s TTL. Client uses local ratings for "my rating". Add `/mine` endpoint in Phase 4 for cross-device. -The plan says rate limiting can use "in-memory (reset per isolate)". Workers are stateless — each request can hit a different V8 isolate. In-memory counters reset constantly and provide almost no protection. +**8. ~~Rate limiting via in-memory storage won't work~~ RESOLVED** -**Mitigation:** Use D1 for rate limiting (simple `rate_limits` table with user_id + window timestamp), or use Cloudflare's built-in rate limiting rules (free tier allows 1 rule). D1 adds one extra read+write per request but is reliable. For a festival app's write volume (~2K/day), the overhead is negligible. +Launch with DB constraint only (one rating per user per drink). Upgrade to D1 rate-limit table if abuse is observed. See section 1.5. **9. D1 write concurrency under burst load** @@ -965,9 +941,9 @@ The existing data proxy Worker allows `cambeerfestival.app` and `staging.cambeer ### Summary: What to do before starting Phase 1 -| # | Action | Effort | -|---|--------|--------| -| 1 | **Spike JWT verification** — build a minimal Worker that verifies a Firebase token using `jose`. Proves the hardest piece works. | 2-3 hours | -| 2 | **Decide festival end-date source** — how does the ratings Worker know when a festival ends? | 15 min decision | -| 3 | **Decide batch endpoint caching strategy** — split into aggregates + user-ratings, or accept no caching? | 15 min decision | -| 4 | **Decide rate limiting storage** — D1 table or Cloudflare rate limiting rules? | 15 min decision | +| # | Action | Effort | Status | +|---|--------|--------|--------| +| 1 | **Spike JWT verification** — build a minimal Worker that verifies a Firebase token using `jose`. Proves the hardest piece works. | 2-3 hours | TODO | +| 2 | **Decide festival end-date source** — how does the ratings Worker know when a festival ends? | 15 min decision | TODO | +| ~~3~~ | ~~**Decide batch endpoint caching strategy**~~ | — | RESOLVED: aggregates only, cacheable. Add `/mine` in Phase 4. | +| ~~4~~ | ~~**Decide rate limiting storage**~~ | — | RESOLVED: DB constraint only for launch. D1 table if needed later. | From 085d46978e75922c74f50b9aae87097a671dd6b7 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 8 Feb 2026 14:37:38 +0000 Subject: [PATCH 8/8] Resolve remaining decisions: festivals.json embed, drink IDs, cleanup - Festival metadata: embed festivals.json at build time (same as data proxy) - Drink ID validation: trust client, don't validate for launch - Update decision summary table with all resolved decisions - Update write path to validate festivalId against embedded data - Clean up anti-abuse description (DB constraint only, no rate limiting) - Only remaining pre-Phase-1 item: JWT verification spike https://claude.ai/code/session_01SusUN4s9TQ4AqeoyoGAGYa --- docs/planning/rating-service/design.md | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/docs/planning/rating-service/design.md b/docs/planning/rating-service/design.md index ec614030..509c9e0f 100644 --- a/docs/planning/rating-service/design.md +++ b/docs/planning/rating-service/design.md @@ -9,7 +9,10 @@ | Rating model | Simple star rating (1-5) | Matches existing UI, low complexity | | API response | Average + count + distribution + user's own rating | Full data for rich UI | | Endpoints | Single-drink + batch | Stars on list screen without N+1 requests | -| Anti-abuse | One rating per user per drink + frequency limiting | Proportionate for festival app | +| Anti-abuse | DB UNIQUE constraint (one rating per user per drink) | Sufficient for festival scale; upgrade to rate-limit table if needed | +| Batch endpoint | Aggregates only (no per-user data) | Fully cacheable; client uses local ratings for "my rating" | +| Festival metadata | Embed `festivals.json` at build time (same as data proxy) | Single source of truth for IDs, end dates, validation | +| Drink ID validation | Trust client (don't validate) | No incentive to fabricate; orphan rows are harmless | | Offline | Optimistic local + background sync | Festival venues have patchy signal | --- @@ -237,7 +240,7 @@ The constraint prevents duplicate ratings. At festival scale (~2K writes/day), e ``` 1. Validate auth token → extract user_id -2. Check rate limit → reject if exceeded +2. Validate festivalId against embedded festivals.json → reject if unknown or ended 3. Validate request body (rating 1-5) 4. UPSERT into ratings table 5. Update rating_aggregates table: @@ -854,15 +857,9 @@ The plan references `EnvironmentService.isStaging()` in the Flutter environment `EnvironmentService` treats all mobile platforms as production (`return true` in `isProduction()`). This means there's no way to test the ratings API staging environment on Android without a code change. If mobile staging is needed, this needs addressing. -**3. Festival end date — where does the Worker get it?** +**3. ~~Festival end date — where does the Worker get it?~~ RESOLVED** -The plan says the API blocks writes after festival end date (resolved question 3), but the ratings Worker has no access to festival metadata. `festivals.json` lives in the data proxy Worker. Options: -- Store festival dates in D1 (extra migration + sync) -- Worker env var per festival (manual, doesn't scale) -- Ratings Worker fetches from data proxy on startup (adds a dependency) -- Pass festival end date from the client (insecure — client can lie) - -This needs a design decision before Phase 1. +Embed `data/festivals.json` at build time — same pattern as the data proxy Worker. CI copies the file during build (`cp data/festivals.json cloudflare-worker/ratings/festivals.json`). The ratings Worker imports it and has access to festival IDs, end dates, and can validate requests. Ratings Worker redeploys when `data/festivals.json` changes (add path trigger to CI). Drink IDs are trusted from the client — no validation needed. ### High Risk @@ -943,7 +940,9 @@ The existing data proxy Worker allows `cambeerfestival.app` and `staging.cambeer | # | Action | Effort | Status | |---|--------|--------|--------| -| 1 | **Spike JWT verification** — build a minimal Worker that verifies a Firebase token using `jose`. Proves the hardest piece works. | 2-3 hours | TODO | -| 2 | **Decide festival end-date source** — how does the ratings Worker know when a festival ends? | 15 min decision | TODO | +| # | Action | Effort | Status | +|---|--------|--------|--------| +| 1 | **Spike JWT verification** — build a minimal Worker that verifies a Firebase token using `jose`. Proves the hardest piece works. | 2-3 hours | **TODO — only remaining pre-Phase-1 item** | +| ~~2~~ | ~~**Decide festival end-date source**~~ | — | RESOLVED: embed `festivals.json` at build time, same as data proxy. | | ~~3~~ | ~~**Decide batch endpoint caching strategy**~~ | — | RESOLVED: aggregates only, cacheable. Add `/mine` in Phase 4. | | ~~4~~ | ~~**Decide rate limiting storage**~~ | — | RESOLVED: DB constraint only for launch. D1 table if needed later. |