From 95ae3a5bc9922cd8692ae409e780af3c485131d0 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 12 Jun 2026 19:29:45 +0000 Subject: [PATCH 1/7] feat(worker): add aggregate ratings API on D1 (test bucket) First step towards an online "my festival". Clients submit a drink rating and get back the shared aggregate (count + average + their own rating). - New /v1/ratings endpoints on the existing proxy worker: POST/DELETE upsert/remove a device's rating, GET single + batch aggregates. - D1-backed storage with upsert semantics (one row per device/drink) so re-rating never inflates counts. Anonymous device_id now; user_id column reserved for the sign-in upgrade. - Every row/query scoped by a `bucket` so test traffic stays isolated from production data; bucket derived from origin, overridable via RATINGS_BUCKET. - CORS extended to POST/DELETE. - Full vitest coverage against a simulated local D1 (no real database needed): pure-helper unit tests plus integration tests for upsert, aggregation, validation, deletion and bucket isolation. 78 worker tests pass. The wrangler.toml database_id is a placeholder; local dev and tests use a simulated D1. README documents the endpoints and the one-time `wrangler d1 create` / migrations-apply provisioning before first deploy. --- cloudflare-worker/README.md | 47 +++ .../migrations/0001_create_ratings_table.sql | 24 ++ cloudflare-worker/ratings.js | 312 +++++++++++++++ cloudflare-worker/test/apply-migrations.js | 5 + cloudflare-worker/test/cors.test.js | 2 +- cloudflare-worker/test/ratings.test.js | 355 ++++++++++++++++++ cloudflare-worker/vitest.config.js | 31 +- cloudflare-worker/worker.js | 15 +- cloudflare-worker/wrangler.toml | 18 + 9 files changed, 799 insertions(+), 10 deletions(-) create mode 100644 cloudflare-worker/migrations/0001_create_ratings_table.sql create mode 100644 cloudflare-worker/ratings.js create mode 100644 cloudflare-worker/test/apply-migrations.js create mode 100644 cloudflare-worker/test/ratings.test.js diff --git a/cloudflare-worker/README.md b/cloudflare-worker/README.md index 0e7ca18f..ec1ddc81 100644 --- a/cloudflare-worker/README.md +++ b/cloudflare-worker/README.md @@ -92,6 +92,53 @@ This endpoint: - Returns them as a sorted array - Caches the result for 1 hour +### Ratings API (v1) + +Aggregate drink ratings backed by a D1 (SQLite) database. This is the first +step towards an online "my festival". Writes are local-first on the client; the +server holds the shared aggregate. Every row and query is scoped by a `bucket` +(`test` or `prod`) so test traffic never mixes with production data. + +| Method | Path | Purpose | +| -------- | ------------------------------------- | -------------------------------------------------- | +| `POST` | `/v1/ratings` | Upsert a device's rating (1–5) | +| `DELETE` | `/v1/ratings` | Remove a device's rating | +| `GET` | `/v1/ratings/{festivalId}/{drinkId}` | Aggregate for one drink | +| `GET` | `/v1/ratings/{festivalId}` | Aggregate for every rated drink (batch, keyed map) | + +`POST`/`DELETE` take a JSON body `{ festivalId, drinkId, deviceId, rating }` +(`rating` omitted for `DELETE`). `GET` requests accept an optional +`?deviceId=` to include the caller's own `yourRating`. The bucket is derived +from the request origin (only `https://cambeerfestival.app` → `prod`; everything +else → `test`) and can be pinned with a `RATINGS_BUCKET` worker var. + +```bash +# Submit a rating, get back the aggregate +curl -X POST https://data.cambeerfestival.app/v1/ratings \ + -H 'Content-Type: application/json' \ + -d '{"festivalId":"cbf2025","drinkId":"beer-1","deviceId":"dev-1","rating":4}' +# -> {"festivalId":"cbf2025","drinkId":"beer-1","count":1,"average":4,"yourRating":4} + +# Read the aggregate for one drink +curl https://data.cambeerfestival.app/v1/ratings/cbf2025/beer-1?deviceId=dev-1 +``` + +#### D1 provisioning (one-time, before first deploy) + +The `database_id` in `wrangler.toml` is a placeholder. Local `wrangler dev` and +the vitest test pool use a simulated local D1 and ignore it, so the full test +suite runs with no real database. Before deploying: + +```bash +cd cloudflare-worker +wrangler d1 create cbf-ratings # prints the database_id +# paste the id into wrangler.toml ([[d1_databases]].database_id) +wrangler d1 migrations apply cbf-ratings # applies migrations/*.sql +``` + +The deploy `CLOUDFLARE_API_TOKEN` must include **D1: Edit** in addition to +Workers Scripts: Edit. To wipe test data: `DELETE FROM ratings WHERE bucket='test'`. + ### Health Check - `/health` - Returns `{"status": "ok"}` for monitoring diff --git a/cloudflare-worker/migrations/0001_create_ratings_table.sql b/cloudflare-worker/migrations/0001_create_ratings_table.sql new file mode 100644 index 00000000..5c6fa62d --- /dev/null +++ b/cloudflare-worker/migrations/0001_create_ratings_table.sql @@ -0,0 +1,24 @@ +-- Aggregate drink ratings (first step towards online "my festival"). +-- +-- One row per (bucket, festival, drink, device). The composite primary key +-- gives upsert semantics: a device re-rating a drink updates its existing row +-- rather than inserting a duplicate, so aggregate counts never inflate. +-- +-- `bucket` isolates data by environment ('test' vs 'prod') so we can exercise +-- the system end to end without polluting real festival data. `user_id` is +-- reserved for the sign-in upgrade (phase 3) and stays NULL while anonymous. + +CREATE TABLE IF NOT EXISTS ratings ( + bucket TEXT NOT NULL, + festival_id TEXT NOT NULL, + drink_id TEXT NOT NULL, + device_id TEXT NOT NULL, + user_id TEXT, + rating INTEGER NOT NULL CHECK (rating BETWEEN 1 AND 5), + updated_at INTEGER NOT NULL, + PRIMARY KEY (bucket, festival_id, drink_id, device_id) +); + +-- Aggregate reads always filter by (bucket, festival_id) and group by drink_id. +CREATE INDEX IF NOT EXISTS idx_ratings_aggregate + ON ratings (bucket, festival_id, drink_id); diff --git a/cloudflare-worker/ratings.js b/cloudflare-worker/ratings.js new file mode 100644 index 00000000..78c01abd --- /dev/null +++ b/cloudflare-worker/ratings.js @@ -0,0 +1,312 @@ +/** + * Aggregate drink ratings API (v1). + * + * Endpoints (all under /v1/ratings, served by the same worker as the proxy): + * POST /v1/ratings upsert a device's rating + * DELETE /v1/ratings remove a device's rating + * GET /v1/ratings/{festivalId}/{drinkId} aggregate for one drink + * GET /v1/ratings/{festivalId} aggregate for every rated drink + * + * Writes are local-first on the client; the server is the shared aggregate. + * Every row and query is scoped by a `bucket` so test traffic never mixes with + * production data — see resolveBucket(). + */ + +const MAX_ID_LENGTH = 200; + +// Only the production web origin maps to the 'prod' bucket. Everything else +// (staging, Pages previews, localhost, tunnels, native apps with no Origin) +// lands in 'test'. This mirrors EnvironmentService.isProductionHost on the +// client and keeps a single worker deploy serving both buckets safely — +// bucket is a data-hygiene boundary, not a security one. +export function isProductionOrigin(origin) { + return origin === "https://cambeerfestival.app"; +} + +/** + * Resolve the storage bucket for a request. + * + * An explicit `RATINGS_BUCKET` worker var wins (lets us pin a deploy to a + * bucket during rollout); otherwise it is derived from the request origin. + */ +export function resolveBucket(origin, env) { + if (env && typeof env.RATINGS_BUCKET === "string" && env.RATINGS_BUCKET) { + return env.RATINGS_BUCKET; + } + return isProductionOrigin(origin) ? "prod" : "test"; +} + +function isValidId(value) { + return ( + typeof value === "string" && + value.length > 0 && + value.length <= MAX_ID_LENGTH + ); +} + +/** + * Validate a write payload (POST/DELETE share the same shape, minus `rating` + * for DELETE). Returns { ok: true, value } or { ok: false, error }. + */ +export function validateWritePayload(body, { requireRating }) { + if (body === null || typeof body !== "object") { + return { ok: false, error: "Request body must be a JSON object" }; + } + + const { festivalId, drinkId, deviceId, rating } = body; + + if (!isValidId(festivalId)) { + return { ok: false, error: "festivalId is required" }; + } + if (!isValidId(drinkId)) { + return { ok: false, error: "drinkId is required" }; + } + if (!isValidId(deviceId)) { + return { ok: false, error: "deviceId is required" }; + } + + if (requireRating) { + if (!Number.isInteger(rating) || rating < 1 || rating > 5) { + return { ok: false, error: "rating must be an integer between 1 and 5" }; + } + } + + return { ok: true, value: { festivalId, drinkId, deviceId, rating } }; +} + +/** Round an average to one decimal place, or null when there are no ratings. */ +export function formatAverage(average, count) { + if (!count || average == null) return null; + return Math.round(average * 10) / 10; +} + +function jsonResponse(body, status, corsHeaders) { + return new Response(JSON.stringify(body), { + status, + headers: { + "Content-Type": "application/json; charset=utf-8", + ...corsHeaders, + }, + }); +} + +async function parseJsonBody(request) { + try { + return { ok: true, body: await request.json() }; + } catch { + return { ok: false }; + } +} + +/** Aggregate (count + average) for a single drink in a bucket. */ +async function readAggregate(db, bucket, festivalId, drinkId, deviceId) { + const agg = await db + .prepare( + "SELECT COUNT(*) AS count, AVG(rating) AS average " + + "FROM ratings WHERE bucket = ? AND festival_id = ? AND drink_id = ?", + ) + .bind(bucket, festivalId, drinkId) + .first(); + + let yourRating = null; + if (deviceId) { + const own = await db + .prepare( + "SELECT rating FROM ratings " + + "WHERE bucket = ? AND festival_id = ? AND drink_id = ? AND device_id = ?", + ) + .bind(bucket, festivalId, drinkId, deviceId) + .first(); + yourRating = own ? own.rating : null; + } + + const count = agg ? agg.count : 0; + return { + festivalId, + drinkId, + count, + average: formatAverage(agg ? agg.average : null, count), + yourRating, + }; +} + +async function handlePost(request, env, bucket, corsHeaders) { + const parsed = await parseJsonBody(request); + if (!parsed.ok) { + return jsonResponse({ error: "Invalid JSON body" }, 400, corsHeaders); + } + + const result = validateWritePayload(parsed.body, { requireRating: true }); + if (!result.ok) { + return jsonResponse({ error: result.error }, 400, corsHeaders); + } + + const { festivalId, drinkId, deviceId, rating } = result.value; + await env.RATINGS_DB.prepare( + "INSERT INTO ratings (bucket, festival_id, drink_id, device_id, rating, updated_at) " + + "VALUES (?, ?, ?, ?, ?, ?) " + + "ON CONFLICT (bucket, festival_id, drink_id, device_id) " + + "DO UPDATE SET rating = excluded.rating, updated_at = excluded.updated_at", + ) + .bind(bucket, festivalId, drinkId, deviceId, rating, Date.now()) + .run(); + + const aggregate = await readAggregate( + env.RATINGS_DB, + bucket, + festivalId, + drinkId, + deviceId, + ); + return jsonResponse(aggregate, 200, corsHeaders); +} + +async function handleDelete(request, env, bucket, corsHeaders) { + const parsed = await parseJsonBody(request); + if (!parsed.ok) { + return jsonResponse({ error: "Invalid JSON body" }, 400, corsHeaders); + } + + const result = validateWritePayload(parsed.body, { requireRating: false }); + if (!result.ok) { + return jsonResponse({ error: result.error }, 400, corsHeaders); + } + + const { festivalId, drinkId, deviceId } = result.value; + await env.RATINGS_DB.prepare( + "DELETE FROM ratings " + + "WHERE bucket = ? AND festival_id = ? AND drink_id = ? AND device_id = ?", + ) + .bind(bucket, festivalId, drinkId, deviceId) + .run(); + + const aggregate = await readAggregate( + env.RATINGS_DB, + bucket, + festivalId, + drinkId, + deviceId, + ); + return jsonResponse(aggregate, 200, corsHeaders); +} + +async function handleGetSingle( + env, + bucket, + festivalId, + drinkId, + deviceId, + corsHeaders, +) { + const aggregate = await readAggregate( + env.RATINGS_DB, + bucket, + festivalId, + drinkId, + deviceId, + ); + return jsonResponse(aggregate, 200, corsHeaders); +} + +/** Batch: every rated drink for a festival, keyed by drink id. */ +async function handleGetFestival( + env, + bucket, + festivalId, + deviceId, + corsHeaders, +) { + const { results } = await env.RATINGS_DB.prepare( + "SELECT drink_id, COUNT(*) AS count, AVG(rating) AS average " + + "FROM ratings WHERE bucket = ? AND festival_id = ? GROUP BY drink_id", + ) + .bind(bucket, festivalId) + .all(); + + const own = new Map(); + if (deviceId) { + const ownRows = await env.RATINGS_DB.prepare( + "SELECT drink_id, rating FROM ratings " + + "WHERE bucket = ? AND festival_id = ? AND device_id = ?", + ) + .bind(bucket, festivalId, deviceId) + .all(); + for (const row of ownRows.results) { + own.set(row.drink_id, row.rating); + } + } + + const aggregates = {}; + for (const row of results) { + aggregates[row.drink_id] = { + count: row.count, + average: formatAverage(row.average, row.count), + yourRating: own.has(row.drink_id) ? own.get(row.drink_id) : null, + }; + } + + return jsonResponse({ festivalId, aggregates }, 200, corsHeaders); +} + +/** + * Route and handle a /v1/ratings request. Returns a Response, or null if the + * path is not a ratings path (so the caller can fall through to the proxy). + */ +export async function handleRatings(request, url, env, corsHeaders) { + if ( + url.pathname !== "/v1/ratings" && + !url.pathname.startsWith("/v1/ratings/") + ) { + return null; + } + + if (!env || !env.RATINGS_DB) { + return jsonResponse( + { error: "Ratings storage is not configured" }, + 503, + corsHeaders, + ); + } + + const origin = request.headers.get("Origin") || ""; + const bucket = resolveBucket(origin, env); + + // Collection endpoint: POST / DELETE on /v1/ratings + if (url.pathname === "/v1/ratings") { + if (request.method === "POST") { + return handlePost(request, env, bucket, corsHeaders); + } + if (request.method === "DELETE") { + return handleDelete(request, env, bucket, corsHeaders); + } + return jsonResponse({ error: "Method not allowed" }, 405, corsHeaders); + } + + // Read endpoints: GET /v1/ratings/{festivalId}[/{drinkId}] + if (request.method !== "GET") { + return jsonResponse({ error: "Method not allowed" }, 405, corsHeaders); + } + + const segments = url.pathname + .slice("/v1/ratings/".length) + .split("/") + .filter((s) => s.length > 0) + .map((s) => decodeURIComponent(s)); + const deviceId = url.searchParams.get("deviceId") || null; + + if (segments.length === 1) { + return handleGetFestival(env, bucket, segments[0], deviceId, corsHeaders); + } + if (segments.length === 2) { + return handleGetSingle( + env, + bucket, + segments[0], + segments[1], + deviceId, + corsHeaders, + ); + } + + return jsonResponse({ error: "Not found" }, 404, corsHeaders); +} diff --git a/cloudflare-worker/test/apply-migrations.js b/cloudflare-worker/test/apply-migrations.js new file mode 100644 index 00000000..40c89ef4 --- /dev/null +++ b/cloudflare-worker/test/apply-migrations.js @@ -0,0 +1,5 @@ +import { applyD1Migrations, env } from "cloudflare:test"; + +// Apply the ratings schema to the per-test simulated D1 before any test runs. +// `TEST_MIGRATIONS` is provided by vitest.config.js via readD1Migrations(). +await applyD1Migrations(env.RATINGS_DB, env.TEST_MIGRATIONS); diff --git a/cloudflare-worker/test/cors.test.js b/cloudflare-worker/test/cors.test.js index 4352f587..f477cb26 100644 --- a/cloudflare-worker/test/cors.test.js +++ b/cloudflare-worker/test/cors.test.js @@ -160,7 +160,7 @@ describe("CORS preflight (OPTIONS)", () => { "OPTIONS", ); expect(response.headers.get("Access-Control-Allow-Methods")).toBe( - "GET, OPTIONS", + "GET, POST, DELETE, OPTIONS", ); expect(response.headers.get("Access-Control-Allow-Headers")).toBe( "Content-Type", diff --git a/cloudflare-worker/test/ratings.test.js b/cloudflare-worker/test/ratings.test.js new file mode 100644 index 00000000..3db680ba --- /dev/null +++ b/cloudflare-worker/test/ratings.test.js @@ -0,0 +1,355 @@ +import { describe, it, expect, beforeEach } from "vitest"; +import { + env, + createExecutionContext, + waitOnExecutionContext, +} from "cloudflare:test"; +import worker from "../worker.js"; +import { + isProductionOrigin, + resolveBucket, + validateWritePayload, + formatAverage, +} from "../ratings.js"; + +const TEST_ORIGIN = "http://localhost:8080"; // non-prod -> 'test' bucket +const PROD_ORIGIN = "https://cambeerfestival.app"; // -> 'prod' bucket + +async function send(method, path, { body, origin = TEST_ORIGIN } = {}) { + const init = { method, headers: { Origin: origin } }; + if (body !== undefined) { + init.headers["Content-Type"] = "application/json"; + init.body = typeof body === "string" ? body : JSON.stringify(body); + } + const request = new Request(`https://worker.example.com${path}`, init); + const ctx = createExecutionContext(); + const response = await worker.fetch(request, env, ctx); + await waitOnExecutionContext(ctx); + return response; +} + +const post = (body, opts) => send("POST", "/v1/ratings", { body, ...opts }); +const del = (body, opts) => send("DELETE", "/v1/ratings", { body, ...opts }); + +// This pool version shares D1 storage across tests in a file, so reset the +// table before each test to keep aggregates deterministic. +beforeEach(async () => { + await env.RATINGS_DB.prepare("DELETE FROM ratings").run(); +}); + +describe("ratings — pure helpers", () => { + it("only the production web origin is a production origin", () => { + expect(isProductionOrigin("https://cambeerfestival.app")).toBe(true); + expect(isProductionOrigin("https://staging.cambeerfestival.app")).toBe( + false, + ); + expect(isProductionOrigin("http://localhost:8080")).toBe(false); + expect(isProductionOrigin("")).toBe(false); + }); + + it("resolveBucket derives from origin", () => { + expect(resolveBucket(PROD_ORIGIN, {})).toBe("prod"); + expect(resolveBucket(TEST_ORIGIN, {})).toBe("test"); + expect(resolveBucket("", {})).toBe("test"); + }); + + it("resolveBucket honours an explicit RATINGS_BUCKET override", () => { + expect(resolveBucket(PROD_ORIGIN, { RATINGS_BUCKET: "test" })).toBe("test"); + expect(resolveBucket(TEST_ORIGIN, { RATINGS_BUCKET: "prod" })).toBe("prod"); + expect(resolveBucket(TEST_ORIGIN, { RATINGS_BUCKET: "" })).toBe("test"); + }); + + it("validateWritePayload accepts a well-formed rating", () => { + const result = validateWritePayload( + { + festivalId: "cbf2025", + drinkId: "beer-1", + deviceId: "dev-1", + rating: 4, + }, + { requireRating: true }, + ); + expect(result.ok).toBe(true); + expect(result.value.rating).toBe(4); + }); + + it("validateWritePayload rejects out-of-range and non-integer ratings", () => { + for (const rating of [0, 6, 3.5, "4", null, undefined]) { + const result = validateWritePayload( + { festivalId: "f", drinkId: "d", deviceId: "x", rating }, + { requireRating: true }, + ); + expect(result.ok).toBe(false); + } + }); + + it("validateWritePayload rejects missing ids", () => { + expect( + validateWritePayload( + { drinkId: "d", deviceId: "x", rating: 3 }, + { requireRating: true }, + ).ok, + ).toBe(false); + expect( + validateWritePayload( + { festivalId: "f", deviceId: "x", rating: 3 }, + { requireRating: true }, + ).ok, + ).toBe(false); + expect( + validateWritePayload( + { festivalId: "f", drinkId: "d", rating: 3 }, + { requireRating: true }, + ).ok, + ).toBe(false); + }); + + it("validateWritePayload skips rating when not required (DELETE)", () => { + const result = validateWritePayload( + { festivalId: "f", drinkId: "d", deviceId: "x" }, + { requireRating: false }, + ); + expect(result.ok).toBe(true); + }); + + it("formatAverage rounds to one decimal and is null when empty", () => { + expect(formatAverage(4.25, 4)).toBe(4.3); + expect(formatAverage(3, 1)).toBe(3); + expect(formatAverage(null, 0)).toBe(null); + expect(formatAverage(5, 0)).toBe(null); + }); +}); + +describe("ratings — POST upsert", () => { + it("records a rating and returns the aggregate", async () => { + const response = await post({ + festivalId: "cbf2025", + drinkId: "beer-1", + deviceId: "dev-1", + rating: 4, + }); + expect(response.status).toBe(200); + const data = await response.json(); + expect(data).toMatchObject({ + festivalId: "cbf2025", + drinkId: "beer-1", + count: 1, + average: 4, + yourRating: 4, + }); + }); + + it("re-rating from the same device updates rather than duplicates", async () => { + await post({ + festivalId: "cbf2025", + drinkId: "beer-1", + deviceId: "dev-1", + rating: 2, + }); + const response = await post({ + festivalId: "cbf2025", + drinkId: "beer-1", + deviceId: "dev-1", + rating: 5, + }); + const data = await response.json(); + expect(data.count).toBe(1); + expect(data.average).toBe(5); + expect(data.yourRating).toBe(5); + }); + + it("aggregates across multiple devices", async () => { + await post({ + festivalId: "cbf2025", + drinkId: "beer-1", + deviceId: "dev-1", + rating: 4, + }); + await post({ + festivalId: "cbf2025", + drinkId: "beer-1", + deviceId: "dev-2", + rating: 5, + }); + const response = await post({ + festivalId: "cbf2025", + drinkId: "beer-1", + deviceId: "dev-3", + rating: 3, + }); + const data = await response.json(); + expect(data.count).toBe(3); + expect(data.average).toBe(4); // (4+5+3)/3 + expect(data.yourRating).toBe(3); // dev-3 + }); +}); + +describe("ratings — validation", () => { + it("rejects an invalid rating with 400", async () => { + const response = await post({ + festivalId: "cbf2025", + drinkId: "beer-1", + deviceId: "dev-1", + rating: 9, + }); + expect(response.status).toBe(400); + }); + + it("rejects missing fields with 400", async () => { + const response = await post({ drinkId: "beer-1", rating: 4 }); + expect(response.status).toBe(400); + }); + + it("rejects malformed JSON with 400", async () => { + const response = await post("{not json", {}); + expect(response.status).toBe(400); + }); + + it("rejects unsupported methods on the collection with 405", async () => { + const response = await send("PUT", "/v1/ratings", { + body: { festivalId: "f", drinkId: "d", deviceId: "x", rating: 3 }, + }); + expect(response.status).toBe(405); + }); +}); + +describe("ratings — GET", () => { + it("returns an empty aggregate for an unrated drink", async () => { + const response = await send("GET", "/v1/ratings/cbf2025/never-rated"); + expect(response.status).toBe(200); + const data = await response.json(); + expect(data).toMatchObject({ + count: 0, + average: null, + yourRating: null, + }); + }); + + it("includes yourRating only when deviceId is supplied", async () => { + await post({ + festivalId: "cbf2025", + drinkId: "beer-1", + deviceId: "dev-1", + rating: 4, + }); + + const anon = await send("GET", "/v1/ratings/cbf2025/beer-1"); + expect((await anon.json()).yourRating).toBe(null); + + const known = await send( + "GET", + "/v1/ratings/cbf2025/beer-1?deviceId=dev-1", + ); + expect((await known.json()).yourRating).toBe(4); + }); + + it("returns a festival-wide map of aggregates", async () => { + await post({ + festivalId: "cbf2025", + drinkId: "beer-1", + deviceId: "dev-1", + rating: 4, + }); + await post({ + festivalId: "cbf2025", + drinkId: "beer-2", + deviceId: "dev-1", + rating: 2, + }); + await post({ + festivalId: "cbf2025", + drinkId: "beer-2", + deviceId: "dev-2", + rating: 4, + }); + + const response = await send("GET", "/v1/ratings/cbf2025?deviceId=dev-1"); + const data = await response.json(); + expect(data.festivalId).toBe("cbf2025"); + expect(data.aggregates["beer-1"]).toMatchObject({ + count: 1, + average: 4, + yourRating: 4, + }); + expect(data.aggregates["beer-2"]).toMatchObject({ + count: 2, + average: 3, + yourRating: 2, + }); + }); + + it("returns 404 for an over-long ratings path", async () => { + const response = await send("GET", "/v1/ratings/cbf2025/beer-1/extra"); + expect(response.status).toBe(404); + }); +}); + +describe("ratings — DELETE", () => { + it("removes a device's rating and returns the fresh aggregate", async () => { + await post({ + festivalId: "cbf2025", + drinkId: "beer-1", + deviceId: "dev-1", + rating: 4, + }); + await post({ + festivalId: "cbf2025", + drinkId: "beer-1", + deviceId: "dev-2", + rating: 2, + }); + + const response = await del({ + festivalId: "cbf2025", + drinkId: "beer-1", + deviceId: "dev-1", + }); + const data = await response.json(); + expect(data.count).toBe(1); // only dev-2 remains + expect(data.average).toBe(2); + expect(data.yourRating).toBe(null); // dev-1's rating is gone + }); + + it("is a no-op when there is nothing to delete", async () => { + const response = await del({ + festivalId: "cbf2025", + drinkId: "beer-1", + deviceId: "ghost", + }); + expect(response.status).toBe(200); + expect((await response.json()).count).toBe(0); + }); +}); + +describe("ratings — bucket isolation", () => { + it("keeps test and prod traffic in separate buckets", async () => { + await post( + { + festivalId: "cbf2025", + drinkId: "beer-1", + deviceId: "dev-1", + rating: 5, + }, + { origin: PROD_ORIGIN }, + ); + await post( + { + festivalId: "cbf2025", + drinkId: "beer-1", + deviceId: "dev-1", + rating: 1, + }, + { origin: TEST_ORIGIN }, + ); + + const prodView = await send("GET", "/v1/ratings/cbf2025/beer-1", { + origin: PROD_ORIGIN, + }); + const testView = await send("GET", "/v1/ratings/cbf2025/beer-1", { + origin: TEST_ORIGIN, + }); + + expect((await prodView.json()).average).toBe(5); + expect((await testView.json()).average).toBe(1); + }); +}); diff --git a/cloudflare-worker/vitest.config.js b/cloudflare-worker/vitest.config.js index adce4cc5..6374e7e6 100644 --- a/cloudflare-worker/vitest.config.js +++ b/cloudflare-worker/vitest.config.js @@ -1,11 +1,26 @@ -import { cloudflareTest } from "@cloudflare/vitest-pool-workers"; +import { + cloudflareTest, + readD1Migrations, +} from "@cloudflare/vitest-pool-workers"; import { defineConfig } from "vitest/config"; -export default defineConfig({ - plugins: [ - cloudflareTest({ - wrangler: { configPath: "./wrangler.toml" }, - }), - ], - test: {}, +export default defineConfig(async () => { + // Read the SQL migrations once at config time. They are exposed to tests as + // the TEST_MIGRATIONS binding and applied to the simulated D1 in a setup file + // (see test/apply-migrations.js), so no real database is needed. + const migrations = await readD1Migrations("./migrations"); + + return { + plugins: [ + cloudflareTest({ + wrangler: { configPath: "./wrangler.toml" }, + miniflare: { + bindings: { TEST_MIGRATIONS: migrations }, + }, + }), + ], + test: { + setupFiles: ["./test/apply-migrations.js"], + }, + }; }); diff --git a/cloudflare-worker/worker.js b/cloudflare-worker/worker.js index 34ae2d4d..72de5922 100644 --- a/cloudflare-worker/worker.js +++ b/cloudflare-worker/worker.js @@ -15,6 +15,7 @@ // Import festivals data directly - copied from data/festivals.json during build import festivalsData from "./festivals.json"; +import { handleRatings } from "./ratings.js"; const UPSTREAM_URL = "https://data.cambridgebeerfestival.com"; @@ -65,6 +66,18 @@ export default { }); } + // Aggregate ratings API (/v1/ratings...). Handled before the proxy + // fall-through so these paths are never forwarded upstream. + const ratingsResponse = await handleRatings( + request, + url, + env, + getCorsHeaders(request), + ); + if (ratingsResponse) { + return ratingsResponse; + } + // Handle dynamic available_beverage_types.json endpoint // Pattern: /{festivalId}/available_beverage_types.json const availableTypesMatch = url.pathname.match( @@ -244,7 +257,7 @@ function handleCorsPreflight(request) { status: 204, headers: { ...getCorsHeaders(request), - "Access-Control-Allow-Methods": "GET, OPTIONS", + "Access-Control-Allow-Methods": "GET, POST, DELETE, OPTIONS", "Access-Control-Allow-Headers": "Content-Type", "Access-Control-Max-Age": maxAge, }, diff --git a/cloudflare-worker/wrangler.toml b/cloudflare-worker/wrangler.toml index 9284424e..fac8e95d 100644 --- a/cloudflare-worker/wrangler.toml +++ b/cloudflare-worker/wrangler.toml @@ -7,3 +7,21 @@ compatibility_date = "2024-01-01" [vars] ENVIRONMENT = "production" + +# Aggregate ratings storage (D1). +# +# `database_id` is a placeholder until the database is provisioned in the +# Cloudflare account. Local dev (`wrangler dev`) and the vitest test pool use a +# simulated local D1 and ignore this id, so the whole test suite runs without a +# real database. Before the first `wrangler deploy`, run: +# +# wrangler d1 create cbf-ratings +# +# then paste the returned id below and apply migrations with: +# +# wrangler d1 migrations apply cbf-ratings +[[d1_databases]] +binding = "RATINGS_DB" +database_name = "cbf-ratings" +database_id = "00000000-0000-0000-0000-000000000000" +migrations_dir = "migrations" From 02c1e8158e9b3675febdcb0ffab120a82bcf9e98 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 12 Jun 2026 19:52:21 +0000 Subject: [PATCH 2/7] feat(worker): add "would recommend" API alongside ratings A yes/no "would recommend" signal, separate from the star rating, so each drink can surface a "% would recommend". - New /v1/recommendations endpoints mirroring ratings (POST/DELETE upsert, GET single + batch). Aggregate reports total responses, "yes" count and the recommend percentage, plus the caller's own answer. - Stored in a new `recommendations` table in the same D1 database, with the same per-device upsert and bucket-isolation model. - Extracted shared bucket/id-validation/JSON/REST-routing plumbing into shared.js so ratings and recommendations stay thin; ratings refactored to consume it (behaviour unchanged). - 19 new tests (pure helpers + integration for upsert, aggregation, validation, deletion, bucket isolation). 97 worker tests pass. README documents the new endpoints; migration 0002 adds the table. --- cloudflare-worker/README.md | 24 ++ .../0002_create_recommendations_table.sql | 20 ++ cloudflare-worker/ratings.js | 208 +++-------- cloudflare-worker/recommendations.js | 223 ++++++++++++ cloudflare-worker/shared.js | 145 ++++++++ cloudflare-worker/test/ratings.test.js | 8 +- .../test/recommendations.test.js | 325 ++++++++++++++++++ cloudflare-worker/worker.js | 11 + 8 files changed, 802 insertions(+), 162 deletions(-) create mode 100644 cloudflare-worker/migrations/0002_create_recommendations_table.sql create mode 100644 cloudflare-worker/recommendations.js create mode 100644 cloudflare-worker/shared.js create mode 100644 cloudflare-worker/test/recommendations.test.js diff --git a/cloudflare-worker/README.md b/cloudflare-worker/README.md index ec1ddc81..a01a3c76 100644 --- a/cloudflare-worker/README.md +++ b/cloudflare-worker/README.md @@ -123,6 +123,30 @@ curl -X POST https://data.cambeerfestival.app/v1/ratings \ curl https://data.cambeerfestival.app/v1/ratings/cbf2025/beer-1?deviceId=dev-1 ``` +### Would-recommend API (v1) + +A yes/no "would recommend" signal, separate from the star rating, surfacing a +`% would recommend` per drink. Same D1 database, shape and bucket rules as the +ratings API, in a `recommendations` table. + +| Method | Path | Purpose | +| -------- | --------------------------------------------- | -------------------------------- | +| `POST` | `/v1/recommendations` | Upsert a device's yes/no answer | +| `DELETE` | `/v1/recommendations` | Remove a device's answer | +| `GET` | `/v1/recommendations/{festivalId}/{drinkId}` | Aggregate for one drink | +| `GET` | `/v1/recommendations/{festivalId}` | Aggregate for every drink (batch) | + +`POST` takes `{ festivalId, drinkId, deviceId, recommend }` where `recommend` +is a JSON boolean (`DELETE` omits it). Responses report total responses, the +"yes" count, the percentage, and the caller's own answer: + +```bash +curl -X POST https://data.cambeerfestival.app/v1/recommendations \ + -H 'Content-Type: application/json' \ + -d '{"festivalId":"cbf2025","drinkId":"beer-1","deviceId":"dev-1","recommend":true}' +# -> {"festivalId":"cbf2025","drinkId":"beer-1","count":1,"recommendCount":1,"recommendPercent":100,"youRecommend":true} +``` + #### D1 provisioning (one-time, before first deploy) The `database_id` in `wrangler.toml` is a placeholder. Local `wrangler dev` and diff --git a/cloudflare-worker/migrations/0002_create_recommendations_table.sql b/cloudflare-worker/migrations/0002_create_recommendations_table.sql new file mode 100644 index 00000000..76dc3007 --- /dev/null +++ b/cloudflare-worker/migrations/0002_create_recommendations_table.sql @@ -0,0 +1,20 @@ +-- "Would recommend" — a yes/no signal per (bucket, festival, drink, device), +-- separate from the star rating so we can surface a "% would recommend". +-- +-- Shares the same upsert/bucket model as the ratings table. `recommend` is +-- stored as 0/1 because SQLite has no native boolean. `user_id` is reserved +-- for the sign-in upgrade and stays NULL while anonymous. + +CREATE TABLE IF NOT EXISTS recommendations ( + bucket TEXT NOT NULL, + festival_id TEXT NOT NULL, + drink_id TEXT NOT NULL, + device_id TEXT NOT NULL, + user_id TEXT, + recommend INTEGER NOT NULL CHECK (recommend IN (0, 1)), + updated_at INTEGER NOT NULL, + PRIMARY KEY (bucket, festival_id, drink_id, device_id) +); + +CREATE INDEX IF NOT EXISTS idx_recommendations_aggregate + ON recommendations (bucket, festival_id, drink_id); diff --git a/cloudflare-worker/ratings.js b/cloudflare-worker/ratings.js index 78c01abd..1c4c0fc9 100644 --- a/cloudflare-worker/ratings.js +++ b/cloudflare-worker/ratings.js @@ -8,70 +8,31 @@ * GET /v1/ratings/{festivalId} aggregate for every rated drink * * Writes are local-first on the client; the server is the shared aggregate. - * Every row and query is scoped by a `bucket` so test traffic never mixes with - * production data — see resolveBucket(). + * Shared bucket/validation/routing plumbing lives in shared.js. */ -const MAX_ID_LENGTH = 200; - -// Only the production web origin maps to the 'prod' bucket. Everything else -// (staging, Pages previews, localhost, tunnels, native apps with no Origin) -// lands in 'test'. This mirrors EnvironmentService.isProductionHost on the -// client and keeps a single worker deploy serving both buckets safely — -// bucket is a data-hygiene boundary, not a security one. -export function isProductionOrigin(origin) { - return origin === "https://cambeerfestival.app"; -} +import { + validateIds, + jsonResponse, + parseJsonBody, + routeResource, +} from "./shared.js"; /** - * Resolve the storage bucket for a request. - * - * An explicit `RATINGS_BUCKET` worker var wins (lets us pin a deploy to a - * bucket during rollout); otherwise it is derived from the request origin. - */ -export function resolveBucket(origin, env) { - if (env && typeof env.RATINGS_BUCKET === "string" && env.RATINGS_BUCKET) { - return env.RATINGS_BUCKET; - } - return isProductionOrigin(origin) ? "prod" : "test"; -} - -function isValidId(value) { - return ( - typeof value === "string" && - value.length > 0 && - value.length <= MAX_ID_LENGTH - ); -} - -/** - * Validate a write payload (POST/DELETE share the same shape, minus `rating` - * for DELETE). Returns { ok: true, value } or { ok: false, error }. + * Validate a write payload. POST requires `rating`; DELETE only needs ids. + * Returns { ok: true, value } or { ok: false, error }. */ export function validateWritePayload(body, { requireRating }) { - if (body === null || typeof body !== "object") { - return { ok: false, error: "Request body must be a JSON object" }; - } - - const { festivalId, drinkId, deviceId, rating } = body; - - if (!isValidId(festivalId)) { - return { ok: false, error: "festivalId is required" }; - } - if (!isValidId(drinkId)) { - return { ok: false, error: "drinkId is required" }; - } - if (!isValidId(deviceId)) { - return { ok: false, error: "deviceId is required" }; - } + const ids = validateIds(body); + if (!ids.ok) return ids; + const { rating } = body; if (requireRating) { if (!Number.isInteger(rating) || rating < 1 || rating > 5) { return { ok: false, error: "rating must be an integer between 1 and 5" }; } } - - return { ok: true, value: { festivalId, drinkId, deviceId, rating } }; + return { ok: true, value: { ...ids.value, rating } }; } /** Round an average to one decimal place, or null when there are no ratings. */ @@ -80,24 +41,6 @@ export function formatAverage(average, count) { return Math.round(average * 10) / 10; } -function jsonResponse(body, status, corsHeaders) { - return new Response(JSON.stringify(body), { - status, - headers: { - "Content-Type": "application/json; charset=utf-8", - ...corsHeaders, - }, - }); -} - -async function parseJsonBody(request) { - try { - return { ok: true, body: await request.json() }; - } catch { - return { ok: false }; - } -} - /** Aggregate (count + average) for a single drink in a bucket. */ async function readAggregate(db, bucket, festivalId, drinkId, deviceId) { const agg = await db @@ -130,7 +73,7 @@ async function readAggregate(db, bucket, festivalId, drinkId, deviceId) { }; } -async function handlePost(request, env, bucket, corsHeaders) { +async function handlePost(request, db, bucket, corsHeaders) { const parsed = await parseJsonBody(request); if (!parsed.ok) { return jsonResponse({ error: "Invalid JSON body" }, 400, corsHeaders); @@ -142,17 +85,18 @@ async function handlePost(request, env, bucket, corsHeaders) { } const { festivalId, drinkId, deviceId, rating } = result.value; - await env.RATINGS_DB.prepare( - "INSERT INTO ratings (bucket, festival_id, drink_id, device_id, rating, updated_at) " + - "VALUES (?, ?, ?, ?, ?, ?) " + - "ON CONFLICT (bucket, festival_id, drink_id, device_id) " + - "DO UPDATE SET rating = excluded.rating, updated_at = excluded.updated_at", - ) + await db + .prepare( + "INSERT INTO ratings (bucket, festival_id, drink_id, device_id, rating, updated_at) " + + "VALUES (?, ?, ?, ?, ?, ?) " + + "ON CONFLICT (bucket, festival_id, drink_id, device_id) " + + "DO UPDATE SET rating = excluded.rating, updated_at = excluded.updated_at", + ) .bind(bucket, festivalId, drinkId, deviceId, rating, Date.now()) .run(); const aggregate = await readAggregate( - env.RATINGS_DB, + db, bucket, festivalId, drinkId, @@ -161,7 +105,7 @@ async function handlePost(request, env, bucket, corsHeaders) { return jsonResponse(aggregate, 200, corsHeaders); } -async function handleDelete(request, env, bucket, corsHeaders) { +async function handleDelete(request, db, bucket, corsHeaders) { const parsed = await parseJsonBody(request); if (!parsed.ok) { return jsonResponse({ error: "Invalid JSON body" }, 400, corsHeaders); @@ -173,15 +117,16 @@ async function handleDelete(request, env, bucket, corsHeaders) { } const { festivalId, drinkId, deviceId } = result.value; - await env.RATINGS_DB.prepare( - "DELETE FROM ratings " + - "WHERE bucket = ? AND festival_id = ? AND drink_id = ? AND device_id = ?", - ) + await db + .prepare( + "DELETE FROM ratings " + + "WHERE bucket = ? AND festival_id = ? AND drink_id = ? AND device_id = ?", + ) .bind(bucket, festivalId, drinkId, deviceId) .run(); const aggregate = await readAggregate( - env.RATINGS_DB, + db, bucket, festivalId, drinkId, @@ -191,7 +136,7 @@ async function handleDelete(request, env, bucket, corsHeaders) { } async function handleGetSingle( - env, + db, bucket, festivalId, drinkId, @@ -199,7 +144,7 @@ async function handleGetSingle( corsHeaders, ) { const aggregate = await readAggregate( - env.RATINGS_DB, + db, bucket, festivalId, drinkId, @@ -210,25 +155,27 @@ async function handleGetSingle( /** Batch: every rated drink for a festival, keyed by drink id. */ async function handleGetFestival( - env, + db, bucket, festivalId, deviceId, corsHeaders, ) { - const { results } = await env.RATINGS_DB.prepare( - "SELECT drink_id, COUNT(*) AS count, AVG(rating) AS average " + - "FROM ratings WHERE bucket = ? AND festival_id = ? GROUP BY drink_id", - ) + const { results } = await db + .prepare( + "SELECT drink_id, COUNT(*) AS count, AVG(rating) AS average " + + "FROM ratings WHERE bucket = ? AND festival_id = ? GROUP BY drink_id", + ) .bind(bucket, festivalId) .all(); const own = new Map(); if (deviceId) { - const ownRows = await env.RATINGS_DB.prepare( - "SELECT drink_id, rating FROM ratings " + - "WHERE bucket = ? AND festival_id = ? AND device_id = ?", - ) + const ownRows = await db + .prepare( + "SELECT drink_id, rating FROM ratings " + + "WHERE bucket = ? AND festival_id = ? AND device_id = ?", + ) .bind(bucket, festivalId, deviceId) .all(); for (const row of ownRows.results) { @@ -248,65 +195,14 @@ async function handleGetFestival( return jsonResponse({ festivalId, aggregates }, 200, corsHeaders); } -/** - * Route and handle a /v1/ratings request. Returns a Response, or null if the - * path is not a ratings path (so the caller can fall through to the proxy). - */ -export async function handleRatings(request, url, env, corsHeaders) { - if ( - url.pathname !== "/v1/ratings" && - !url.pathname.startsWith("/v1/ratings/") - ) { - return null; - } - - if (!env || !env.RATINGS_DB) { - return jsonResponse( - { error: "Ratings storage is not configured" }, - 503, - corsHeaders, - ); - } - - const origin = request.headers.get("Origin") || ""; - const bucket = resolveBucket(origin, env); - - // Collection endpoint: POST / DELETE on /v1/ratings - if (url.pathname === "/v1/ratings") { - if (request.method === "POST") { - return handlePost(request, env, bucket, corsHeaders); - } - if (request.method === "DELETE") { - return handleDelete(request, env, bucket, corsHeaders); - } - return jsonResponse({ error: "Method not allowed" }, 405, corsHeaders); - } - - // Read endpoints: GET /v1/ratings/{festivalId}[/{drinkId}] - if (request.method !== "GET") { - return jsonResponse({ error: "Method not allowed" }, 405, corsHeaders); - } - - const segments = url.pathname - .slice("/v1/ratings/".length) - .split("/") - .filter((s) => s.length > 0) - .map((s) => decodeURIComponent(s)); - const deviceId = url.searchParams.get("deviceId") || null; - - if (segments.length === 1) { - return handleGetFestival(env, bucket, segments[0], deviceId, corsHeaders); - } - if (segments.length === 2) { - return handleGetSingle( - env, - bucket, - segments[0], - segments[1], - deviceId, - corsHeaders, - ); - } - - return jsonResponse({ error: "Not found" }, 404, corsHeaders); +/** Route and handle a /v1/ratings request, or null if not a ratings path. */ +export function handleRatings(request, url, env, corsHeaders) { + return routeResource(request, url, env, corsHeaders, { + basePath: "/v1/ratings", + db: "RATINGS_DB", + post: handlePost, + del: handleDelete, + getSingle: handleGetSingle, + getFestival: handleGetFestival, + }); } diff --git a/cloudflare-worker/recommendations.js b/cloudflare-worker/recommendations.js new file mode 100644 index 00000000..754489ea --- /dev/null +++ b/cloudflare-worker/recommendations.js @@ -0,0 +1,223 @@ +/** + * "Would recommend" API (v1). + * + * A yes/no signal, separate from the star rating, so we can surface a + * "% would recommend" for each drink. Endpoints mirror the ratings API and + * share the same D1 database (RATINGS_DB), in a separate `recommendations` + * table: + * POST /v1/recommendations upsert a device's yes/no + * DELETE /v1/recommendations remove a device's answer + * GET /v1/recommendations/{festivalId}/{drinkId} aggregate for one drink + * GET /v1/recommendations/{festivalId} aggregate for every drink + * + * `recommend` is stored as 0/1 (SQLite has no boolean) and exposed as a JSON + * boolean. The aggregate reports total responses, the count of "yes", and the + * percentage that would recommend. + */ + +import { + validateIds, + jsonResponse, + parseJsonBody, + routeResource, +} from "./shared.js"; + +/** + * Validate a write payload. POST requires a boolean `recommend`; DELETE only + * needs ids. Returns { ok: true, value } or { ok: false, error }. + */ +export function validateRecommendPayload(body, { requireRecommend }) { + const ids = validateIds(body); + if (!ids.ok) return ids; + + const { recommend } = body; + if (requireRecommend && typeof recommend !== "boolean") { + return { ok: false, error: "recommend must be a boolean" }; + } + return { ok: true, value: { ...ids.value, recommend } }; +} + +/** Whole-number "% would recommend", or null when there are no responses. */ +export function formatPercent(recommendCount, count) { + if (!count) return null; + return Math.round((recommendCount / count) * 100); +} + +function aggregateShape(count, recommendCount, youRecommend) { + return { + count, + recommendCount, + recommendPercent: formatPercent(recommendCount, count), + youRecommend, + }; +} + +/** Aggregate (responses + yes count + percentage) for one drink. */ +async function readRecommendation(db, bucket, festivalId, drinkId, deviceId) { + const agg = await db + .prepare( + "SELECT COUNT(*) AS count, SUM(recommend) AS yes " + + "FROM recommendations WHERE bucket = ? AND festival_id = ? AND drink_id = ?", + ) + .bind(bucket, festivalId, drinkId) + .first(); + + let youRecommend = null; + if (deviceId) { + const own = await db + .prepare( + "SELECT recommend FROM recommendations " + + "WHERE bucket = ? AND festival_id = ? AND drink_id = ? AND device_id = ?", + ) + .bind(bucket, festivalId, drinkId, deviceId) + .first(); + youRecommend = own ? Boolean(own.recommend) : null; + } + + const count = agg ? agg.count : 0; + const recommendCount = agg && agg.yes != null ? agg.yes : 0; + return { + festivalId, + drinkId, + ...aggregateShape(count, recommendCount, youRecommend), + }; +} + +async function handlePost(request, db, bucket, corsHeaders) { + const parsed = await parseJsonBody(request); + if (!parsed.ok) { + return jsonResponse({ error: "Invalid JSON body" }, 400, corsHeaders); + } + + const result = validateRecommendPayload(parsed.body, { + requireRecommend: true, + }); + if (!result.ok) { + return jsonResponse({ error: result.error }, 400, corsHeaders); + } + + const { festivalId, drinkId, deviceId, recommend } = result.value; + await db + .prepare( + "INSERT INTO recommendations (bucket, festival_id, drink_id, device_id, recommend, updated_at) " + + "VALUES (?, ?, ?, ?, ?, ?) " + + "ON CONFLICT (bucket, festival_id, drink_id, device_id) " + + "DO UPDATE SET recommend = excluded.recommend, updated_at = excluded.updated_at", + ) + .bind(bucket, festivalId, drinkId, deviceId, recommend ? 1 : 0, Date.now()) + .run(); + + const aggregate = await readRecommendation( + db, + bucket, + festivalId, + drinkId, + deviceId, + ); + return jsonResponse(aggregate, 200, corsHeaders); +} + +async function handleDelete(request, db, bucket, corsHeaders) { + const parsed = await parseJsonBody(request); + if (!parsed.ok) { + return jsonResponse({ error: "Invalid JSON body" }, 400, corsHeaders); + } + + const result = validateRecommendPayload(parsed.body, { + requireRecommend: false, + }); + if (!result.ok) { + return jsonResponse({ error: result.error }, 400, corsHeaders); + } + + const { festivalId, drinkId, deviceId } = result.value; + await db + .prepare( + "DELETE FROM recommendations " + + "WHERE bucket = ? AND festival_id = ? AND drink_id = ? AND device_id = ?", + ) + .bind(bucket, festivalId, drinkId, deviceId) + .run(); + + const aggregate = await readRecommendation( + db, + bucket, + festivalId, + drinkId, + deviceId, + ); + return jsonResponse(aggregate, 200, corsHeaders); +} + +async function handleGetSingle( + db, + bucket, + festivalId, + drinkId, + deviceId, + corsHeaders, +) { + const aggregate = await readRecommendation( + db, + bucket, + festivalId, + drinkId, + deviceId, + ); + return jsonResponse(aggregate, 200, corsHeaders); +} + +/** Batch: every drink with a response for a festival, keyed by drink id. */ +async function handleGetFestival( + db, + bucket, + festivalId, + deviceId, + corsHeaders, +) { + const { results } = await db + .prepare( + "SELECT drink_id, COUNT(*) AS count, SUM(recommend) AS yes " + + "FROM recommendations WHERE bucket = ? AND festival_id = ? GROUP BY drink_id", + ) + .bind(bucket, festivalId) + .all(); + + const own = new Map(); + if (deviceId) { + const ownRows = await db + .prepare( + "SELECT drink_id, recommend FROM recommendations " + + "WHERE bucket = ? AND festival_id = ? AND device_id = ?", + ) + .bind(bucket, festivalId, deviceId) + .all(); + for (const row of ownRows.results) { + own.set(row.drink_id, Boolean(row.recommend)); + } + } + + const aggregates = {}; + for (const row of results) { + const recommendCount = row.yes != null ? row.yes : 0; + aggregates[row.drink_id] = aggregateShape( + row.count, + recommendCount, + own.has(row.drink_id) ? own.get(row.drink_id) : null, + ); + } + + return jsonResponse({ festivalId, aggregates }, 200, corsHeaders); +} + +/** Route and handle a /v1/recommendations request, or null if not one. */ +export function handleRecommendations(request, url, env, corsHeaders) { + return routeResource(request, url, env, corsHeaders, { + basePath: "/v1/recommendations", + db: "RATINGS_DB", + post: handlePost, + del: handleDelete, + getSingle: handleGetSingle, + getFestival: handleGetFestival, + }); +} diff --git a/cloudflare-worker/shared.js b/cloudflare-worker/shared.js new file mode 100644 index 00000000..610fc010 --- /dev/null +++ b/cloudflare-worker/shared.js @@ -0,0 +1,145 @@ +/** + * Shared helpers for the /v1 "my festival" APIs (ratings, recommendations, …). + * + * These resources are structurally identical — a device upserts a signal for a + * drink and reads back a bucket-scoped aggregate — so the bucket resolution, + * id validation, JSON plumbing and REST routing live here once. + */ + +export const MAX_ID_LENGTH = 200; + +// Only the production web origin maps to the 'prod' bucket. Everything else +// (staging, Pages previews, localhost, tunnels, native apps with no Origin) +// lands in 'test'. This mirrors EnvironmentService.isProductionHost on the +// client and keeps a single worker deploy serving both buckets safely — +// bucket is a data-hygiene boundary, not a security one. +export function isProductionOrigin(origin) { + return origin === "https://cambeerfestival.app"; +} + +/** + * Resolve the storage bucket for a request. + * + * An explicit `RATINGS_BUCKET` worker var wins (lets us pin a deploy to a + * bucket during rollout); otherwise it is derived from the request origin. + */ +export function resolveBucket(origin, env) { + if (env && typeof env.RATINGS_BUCKET === "string" && env.RATINGS_BUCKET) { + return env.RATINGS_BUCKET; + } + return isProductionOrigin(origin) ? "prod" : "test"; +} + +export function isValidId(value) { + return ( + typeof value === "string" && + value.length > 0 && + value.length <= MAX_ID_LENGTH + ); +} + +/** + * Validate the identity fields every write shares. Returns + * { ok: true, value: { festivalId, drinkId, deviceId } } or { ok: false, error }. + */ +export function validateIds(body) { + if (body === null || typeof body !== "object") { + return { ok: false, error: "Request body must be a JSON object" }; + } + const { festivalId, drinkId, deviceId } = body; + if (!isValidId(festivalId)) { + return { ok: false, error: "festivalId is required" }; + } + if (!isValidId(drinkId)) { + return { ok: false, error: "drinkId is required" }; + } + if (!isValidId(deviceId)) { + return { ok: false, error: "deviceId is required" }; + } + return { ok: true, value: { festivalId, drinkId, deviceId } }; +} + +export function jsonResponse(body, status, corsHeaders) { + return new Response(JSON.stringify(body), { + status, + headers: { + "Content-Type": "application/json; charset=utf-8", + ...corsHeaders, + }, + }); +} + +export async function parseJsonBody(request) { + try { + return { ok: true, body: await request.json() }; + } catch { + return { ok: false }; + } +} + +/** + * Route a REST request for a /v1 resource. Returns a Response, or null if the + * path is not for this resource (so the caller can fall through). + * + * Routes: + * POST {basePath} -> handlers.post(request, db, bucket, cors) + * DELETE {basePath} -> handlers.del(request, db, bucket, cors) + * GET {basePath}/{festivalId} -> handlers.getFestival(db, bucket, festivalId, deviceId, cors) + * GET {basePath}/{festivalId}/{drinkId}-> handlers.getSingle(db, bucket, festivalId, drinkId, deviceId, cors) + */ +export async function routeResource(request, url, env, corsHeaders, handlers) { + const { basePath, db: dbBinding } = handlers; + const prefix = `${basePath}/`; + if (url.pathname !== basePath && !url.pathname.startsWith(prefix)) { + return null; + } + + if (!env || !env[dbBinding]) { + return jsonResponse( + { error: "Storage is not configured" }, + 503, + corsHeaders, + ); + } + + const origin = request.headers.get("Origin") || ""; + const bucket = resolveBucket(origin, env); + const db = env[dbBinding]; + + if (url.pathname === basePath) { + if (request.method === "POST") { + return handlers.post(request, db, bucket, corsHeaders); + } + if (request.method === "DELETE") { + return handlers.del(request, db, bucket, corsHeaders); + } + return jsonResponse({ error: "Method not allowed" }, 405, corsHeaders); + } + + if (request.method !== "GET") { + return jsonResponse({ error: "Method not allowed" }, 405, corsHeaders); + } + + const segments = url.pathname + .slice(prefix.length) + .split("/") + .filter((s) => s.length > 0) + .map((s) => decodeURIComponent(s)); + const deviceId = url.searchParams.get("deviceId") || null; + + if (segments.length === 1) { + return handlers.getFestival(db, bucket, segments[0], deviceId, corsHeaders); + } + if (segments.length === 2) { + return handlers.getSingle( + db, + bucket, + segments[0], + segments[1], + deviceId, + corsHeaders, + ); + } + + return jsonResponse({ error: "Not found" }, 404, corsHeaders); +} diff --git a/cloudflare-worker/test/ratings.test.js b/cloudflare-worker/test/ratings.test.js index 3db680ba..a557c6f7 100644 --- a/cloudflare-worker/test/ratings.test.js +++ b/cloudflare-worker/test/ratings.test.js @@ -5,12 +5,8 @@ import { waitOnExecutionContext, } from "cloudflare:test"; import worker from "../worker.js"; -import { - isProductionOrigin, - resolveBucket, - validateWritePayload, - formatAverage, -} from "../ratings.js"; +import { isProductionOrigin, resolveBucket } from "../shared.js"; +import { validateWritePayload, formatAverage } from "../ratings.js"; const TEST_ORIGIN = "http://localhost:8080"; // non-prod -> 'test' bucket const PROD_ORIGIN = "https://cambeerfestival.app"; // -> 'prod' bucket diff --git a/cloudflare-worker/test/recommendations.test.js b/cloudflare-worker/test/recommendations.test.js new file mode 100644 index 00000000..6f6a3be0 --- /dev/null +++ b/cloudflare-worker/test/recommendations.test.js @@ -0,0 +1,325 @@ +import { describe, it, expect, beforeEach } from "vitest"; +import { + env, + createExecutionContext, + waitOnExecutionContext, +} from "cloudflare:test"; +import worker from "../worker.js"; +import { validateRecommendPayload, formatPercent } from "../recommendations.js"; + +const TEST_ORIGIN = "http://localhost:8080"; // non-prod -> 'test' bucket +const PROD_ORIGIN = "https://cambeerfestival.app"; // -> 'prod' bucket +const BASE = "/v1/recommendations"; + +async function send(method, path, { body, origin = TEST_ORIGIN } = {}) { + const init = { method, headers: { Origin: origin } }; + if (body !== undefined) { + init.headers["Content-Type"] = "application/json"; + init.body = typeof body === "string" ? body : JSON.stringify(body); + } + const request = new Request(`https://worker.example.com${path}`, init); + const ctx = createExecutionContext(); + const response = await worker.fetch(request, env, ctx); + await waitOnExecutionContext(ctx); + return response; +} + +const post = (body, opts) => send("POST", BASE, { body, ...opts }); +const del = (body, opts) => send("DELETE", BASE, { body, ...opts }); + +// This pool version shares D1 storage across tests in a file, so reset the +// table before each test to keep aggregates deterministic. +beforeEach(async () => { + await env.RATINGS_DB.prepare("DELETE FROM recommendations").run(); +}); + +describe("recommendations — pure helpers", () => { + it("validateRecommendPayload accepts a boolean recommend", () => { + for (const recommend of [true, false]) { + const result = validateRecommendPayload( + { + festivalId: "cbf2025", + drinkId: "beer-1", + deviceId: "dev-1", + recommend, + }, + { requireRecommend: true }, + ); + expect(result.ok).toBe(true); + expect(result.value.recommend).toBe(recommend); + } + }); + + it("validateRecommendPayload rejects non-boolean recommend", () => { + for (const recommend of [1, 0, "yes", null, undefined]) { + const result = validateRecommendPayload( + { festivalId: "f", drinkId: "d", deviceId: "x", recommend }, + { requireRecommend: true }, + ); + expect(result.ok).toBe(false); + } + }); + + it("validateRecommendPayload rejects missing ids", () => { + expect( + validateRecommendPayload( + { drinkId: "d", deviceId: "x", recommend: true }, + { requireRecommend: true }, + ).ok, + ).toBe(false); + }); + + it("validateRecommendPayload skips recommend when not required (DELETE)", () => { + const result = validateRecommendPayload( + { festivalId: "f", drinkId: "d", deviceId: "x" }, + { requireRecommend: false }, + ); + expect(result.ok).toBe(true); + }); + + it("formatPercent is a rounded whole number, null when empty", () => { + expect(formatPercent(3, 4)).toBe(75); + expect(formatPercent(1, 3)).toBe(33); + expect(formatPercent(2, 3)).toBe(67); + expect(formatPercent(0, 2)).toBe(0); + expect(formatPercent(0, 0)).toBe(null); + }); +}); + +describe("recommendations — POST upsert", () => { + it("records a recommendation and returns the aggregate", async () => { + const response = await post({ + festivalId: "cbf2025", + drinkId: "beer-1", + deviceId: "dev-1", + recommend: true, + }); + expect(response.status).toBe(200); + const data = await response.json(); + expect(data).toMatchObject({ + festivalId: "cbf2025", + drinkId: "beer-1", + count: 1, + recommendCount: 1, + recommendPercent: 100, + youRecommend: true, + }); + }); + + it("changing answer from the same device updates rather than duplicates", async () => { + await post({ + festivalId: "cbf2025", + drinkId: "beer-1", + deviceId: "dev-1", + recommend: true, + }); + const response = await post({ + festivalId: "cbf2025", + drinkId: "beer-1", + deviceId: "dev-1", + recommend: false, + }); + const data = await response.json(); + expect(data.count).toBe(1); + expect(data.recommendCount).toBe(0); + expect(data.recommendPercent).toBe(0); + expect(data.youRecommend).toBe(false); + }); + + it("aggregates yes/no across multiple devices", async () => { + await post({ + festivalId: "cbf2025", + drinkId: "beer-1", + deviceId: "dev-1", + recommend: true, + }); + await post({ + festivalId: "cbf2025", + drinkId: "beer-1", + deviceId: "dev-2", + recommend: true, + }); + const response = await post({ + festivalId: "cbf2025", + drinkId: "beer-1", + deviceId: "dev-3", + recommend: false, + }); + const data = await response.json(); + expect(data.count).toBe(3); + expect(data.recommendCount).toBe(2); + expect(data.recommendPercent).toBe(67); // 2/3 + expect(data.youRecommend).toBe(false); // dev-3 + }); +}); + +describe("recommendations — validation", () => { + it("rejects a non-boolean recommend with 400", async () => { + const response = await post({ + festivalId: "cbf2025", + drinkId: "beer-1", + deviceId: "dev-1", + recommend: "yes", + }); + expect(response.status).toBe(400); + }); + + it("rejects missing fields with 400", async () => { + const response = await post({ drinkId: "beer-1", recommend: true }); + expect(response.status).toBe(400); + }); + + it("rejects malformed JSON with 400", async () => { + const response = await post("{not json", {}); + expect(response.status).toBe(400); + }); + + it("rejects unsupported methods on the collection with 405", async () => { + const response = await send("PUT", BASE, { + body: { festivalId: "f", drinkId: "d", deviceId: "x", recommend: true }, + }); + expect(response.status).toBe(405); + }); +}); + +describe("recommendations — GET", () => { + it("returns an empty aggregate for a drink with no responses", async () => { + const response = await send("GET", `${BASE}/cbf2025/never-rated`); + expect(response.status).toBe(200); + const data = await response.json(); + expect(data).toMatchObject({ + count: 0, + recommendCount: 0, + recommendPercent: null, + youRecommend: null, + }); + }); + + it("includes youRecommend only when deviceId is supplied", async () => { + await post({ + festivalId: "cbf2025", + drinkId: "beer-1", + deviceId: "dev-1", + recommend: true, + }); + + const anon = await send("GET", `${BASE}/cbf2025/beer-1`); + expect((await anon.json()).youRecommend).toBe(null); + + const known = await send("GET", `${BASE}/cbf2025/beer-1?deviceId=dev-1`); + expect((await known.json()).youRecommend).toBe(true); + }); + + it("returns a festival-wide map of aggregates", async () => { + await post({ + festivalId: "cbf2025", + drinkId: "beer-1", + deviceId: "dev-1", + recommend: true, + }); + await post({ + festivalId: "cbf2025", + drinkId: "beer-2", + deviceId: "dev-1", + recommend: false, + }); + await post({ + festivalId: "cbf2025", + drinkId: "beer-2", + deviceId: "dev-2", + recommend: true, + }); + + const response = await send("GET", `${BASE}/cbf2025?deviceId=dev-1`); + const data = await response.json(); + expect(data.festivalId).toBe("cbf2025"); + expect(data.aggregates["beer-1"]).toMatchObject({ + count: 1, + recommendCount: 1, + recommendPercent: 100, + youRecommend: true, + }); + expect(data.aggregates["beer-2"]).toMatchObject({ + count: 2, + recommendCount: 1, + recommendPercent: 50, + youRecommend: false, + }); + }); + + it("returns 404 for an over-long path", async () => { + const response = await send("GET", `${BASE}/cbf2025/beer-1/extra`); + expect(response.status).toBe(404); + }); +}); + +describe("recommendations — DELETE", () => { + it("removes a device's answer and returns the fresh aggregate", async () => { + await post({ + festivalId: "cbf2025", + drinkId: "beer-1", + deviceId: "dev-1", + recommend: false, + }); + await post({ + festivalId: "cbf2025", + drinkId: "beer-1", + deviceId: "dev-2", + recommend: true, + }); + + const response = await del({ + festivalId: "cbf2025", + drinkId: "beer-1", + deviceId: "dev-1", + }); + const data = await response.json(); + expect(data.count).toBe(1); // only dev-2 remains + expect(data.recommendCount).toBe(1); + expect(data.recommendPercent).toBe(100); + expect(data.youRecommend).toBe(null); // dev-1's answer is gone + }); + + it("is a no-op when there is nothing to delete", async () => { + const response = await del({ + festivalId: "cbf2025", + drinkId: "beer-1", + deviceId: "ghost", + }); + expect(response.status).toBe(200); + expect((await response.json()).count).toBe(0); + }); +}); + +describe("recommendations — bucket isolation", () => { + it("keeps test and prod traffic in separate buckets", async () => { + await post( + { + festivalId: "cbf2025", + drinkId: "beer-1", + deviceId: "dev-1", + recommend: true, + }, + { origin: PROD_ORIGIN }, + ); + await post( + { + festivalId: "cbf2025", + drinkId: "beer-1", + deviceId: "dev-1", + recommend: false, + }, + { origin: TEST_ORIGIN }, + ); + + const prodView = await send("GET", `${BASE}/cbf2025/beer-1`, { + origin: PROD_ORIGIN, + }); + const testView = await send("GET", `${BASE}/cbf2025/beer-1`, { + origin: TEST_ORIGIN, + }); + + expect((await prodView.json()).recommendPercent).toBe(100); + expect((await testView.json()).recommendPercent).toBe(0); + }); +}); diff --git a/cloudflare-worker/worker.js b/cloudflare-worker/worker.js index 72de5922..27d8945f 100644 --- a/cloudflare-worker/worker.js +++ b/cloudflare-worker/worker.js @@ -16,6 +16,7 @@ // Import festivals data directly - copied from data/festivals.json during build import festivalsData from "./festivals.json"; import { handleRatings } from "./ratings.js"; +import { handleRecommendations } from "./recommendations.js"; const UPSTREAM_URL = "https://data.cambridgebeerfestival.com"; @@ -78,6 +79,16 @@ export default { return ratingsResponse; } + const recommendationsResponse = await handleRecommendations( + request, + url, + env, + getCorsHeaders(request), + ); + if (recommendationsResponse) { + return recommendationsResponse; + } + // Handle dynamic available_beverage_types.json endpoint // Pattern: /{festivalId}/available_beverage_types.json const availableTypesMatch = url.pathname.match( From 501bb2de5b9e7ecc25267932a6b1d3bc7c36909f Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 12 Jun 2026 20:49:36 +0000 Subject: [PATCH 3/7] refactor(worker)!: resource-oriented AIP API for ratings/recommendations Rework the /v1 API to conform to the proto contract and Google's AIPs. BREAKING CHANGE: replaces the flat POST/DELETE /v1/ratings endpoints with resource-oriented routes. Nothing consumes them yet (no client, placeholder DB), so this is a safe pre-launch change. - Resource names: PATCH/GET/DELETE on /v1/festivals/{f}/drinks/{d}/ratings/{device} (and .../recommendations/...). - Upsert via PATCH with allow_missing semantics (AIP-134); bodyless DELETE that is NOT_FOUND when absent (AIP-135). - Read aggregates as RatingSummary / RecommendationSummary resources: GET .../{f}/ratingSummaries/{d} and a paginated list GET .../{f}/ratingSummaries (page_size/page_token/next_page_token + total_size, keyset cursor) (AIP-158). - Structured google.rpc.Status errors with ErrorInfo reason+domain (AIP-193). - RFC3339 update_time; camelCase resource fields matching the proto JSON mapping; dropped redundant your_* (client is local-first and knows its own). - Generic family engine in shared.js drives both resources; CORS now allows GET/PATCH/DELETE; unknown /v1 routes 404 instead of proxying upstream. - 87 worker tests pass. --- cloudflare-worker/README.md | 79 ++- cloudflare-worker/ratings.js | 248 ++-------- cloudflare-worker/recommendations.js | 279 +++-------- cloudflare-worker/shared.js | 439 ++++++++++++++--- cloudflare-worker/test/cors.test.js | 2 +- cloudflare-worker/test/ratings.test.js | 464 ++++++++---------- .../test/recommendations.test.js | 365 ++++---------- cloudflare-worker/worker.js | 18 +- 8 files changed, 841 insertions(+), 1053 deletions(-) diff --git a/cloudflare-worker/README.md b/cloudflare-worker/README.md index a01a3c76..b12a565e 100644 --- a/cloudflare-worker/README.md +++ b/cloudflare-worker/README.md @@ -92,59 +92,46 @@ This endpoint: - Returns them as a sorted array - Caches the result for 1 hour -### Ratings API (v1) - -Aggregate drink ratings backed by a D1 (SQLite) database. This is the first -step towards an online "my festival". Writes are local-first on the client; the -server holds the shared aggregate. Every row and query is scoped by a `bucket` -(`test` or `prod`) so test traffic never mixes with production data. - -| Method | Path | Purpose | -| -------- | ------------------------------------- | -------------------------------------------------- | -| `POST` | `/v1/ratings` | Upsert a device's rating (1–5) | -| `DELETE` | `/v1/ratings` | Remove a device's rating | -| `GET` | `/v1/ratings/{festivalId}/{drinkId}` | Aggregate for one drink | -| `GET` | `/v1/ratings/{festivalId}` | Aggregate for every rated drink (batch, keyed map) | - -`POST`/`DELETE` take a JSON body `{ festivalId, drinkId, deviceId, rating }` -(`rating` omitted for `DELETE`). `GET` requests accept an optional -`?deviceId=` to include the caller's own `yourRating`. The bucket is derived -from the request origin (only `https://cambeerfestival.app` → `prod`; everything -else → `test`) and can be pinned with a `RATINGS_BUCKET` worker var. +### "My festival" API (v1) -```bash -# Submit a rating, get back the aggregate -curl -X POST https://data.cambeerfestival.app/v1/ratings \ - -H 'Content-Type: application/json' \ - -d '{"festivalId":"cbf2025","drinkId":"beer-1","deviceId":"dev-1","rating":4}' -# -> {"festivalId":"cbf2025","drinkId":"beer-1","count":1,"average":4,"yourRating":4} - -# Read the aggregate for one drink -curl https://data.cambeerfestival.app/v1/ratings/cbf2025/beer-1?deviceId=dev-1 -``` +Aggregate drink ratings and "would recommend" answers, backed by D1 (SQLite). +The first step towards an online "my festival". The API is resource-oriented +following [Google's AIPs](https://google.aip.dev) — the contract is defined in +`proto/` and an OpenAPI spec is generated from it (see `proto/README.md`). -### Would-recommend API (v1) +Writes are local-first on the client; the server holds the shared aggregate. +Every row and query is scoped by a `bucket` (`test` or `prod`, derived from the +request origin; only `https://cambeerfestival.app` → `prod`) so test traffic +never mixes with production data. A `RATINGS_BUCKET` worker var can pin it. -A yes/no "would recommend" signal, separate from the star rating, surfacing a -`% would recommend` per drink. Same D1 database, shape and bucket rules as the -ratings API, in a `recommendations` table. +Resources (the device is the record id, so a device has one record per drink): -| Method | Path | Purpose | -| -------- | --------------------------------------------- | -------------------------------- | -| `POST` | `/v1/recommendations` | Upsert a device's yes/no answer | -| `DELETE` | `/v1/recommendations` | Remove a device's answer | -| `GET` | `/v1/recommendations/{festivalId}/{drinkId}` | Aggregate for one drink | -| `GET` | `/v1/recommendations/{festivalId}` | Aggregate for every drink (batch) | +| Method | Path | Purpose | +| -------- | ------------------------------------------------------------ | ------------------------------- | +| `PATCH` | `/v1/festivals/{f}/drinks/{d}/ratings/{device}` | Upsert a rating (`{value:1-5}`) | +| `GET` | `/v1/festivals/{f}/drinks/{d}/ratings/{device}` | Get a device's rating | +| `DELETE` | `/v1/festivals/{f}/drinks/{d}/ratings/{device}` | Remove a device's rating | +| `GET` | `/v1/festivals/{f}/ratingSummaries/{d}` | Aggregate for one drink | +| `GET` | `/v1/festivals/{f}/ratingSummaries?page_size=&page_token=` | Paginated list of aggregates | -`POST` takes `{ festivalId, drinkId, deviceId, recommend }` where `recommend` -is a JSON boolean (`DELETE` omits it). Responses report total responses, the -"yes" count, the percentage, and the caller's own answer: +The `recommendations` / `recommendationSummaries` collections mirror this with +a `{wouldRecommend: bool}` body. `PATCH` is an upsert (AIP-134 `allow_missing`); +`DELETE` takes the id in the path with no body (AIP-135) and is `NOT_FOUND` when +absent. Errors use the structured `google.rpc.Status` shape (AIP-193). ```bash -curl -X POST https://data.cambeerfestival.app/v1/recommendations \ - -H 'Content-Type: application/json' \ - -d '{"festivalId":"cbf2025","drinkId":"beer-1","deviceId":"dev-1","recommend":true}' -# -> {"festivalId":"cbf2025","drinkId":"beer-1","count":1,"recommendCount":1,"recommendPercent":100,"youRecommend":true} +# Upsert a rating, get back the Rating resource +curl -X PATCH https://data.cambeerfestival.app/v1/festivals/cbf2025/drinks/beer-1/ratings/dev-1 \ + -H 'Content-Type: application/json' -d '{"value":4}' +# -> {"name":"festivals/cbf2025/drinks/beer-1/ratings/dev-1","value":4,"updateTime":"2026-06-12T20:00:00.000Z"} + +# Aggregate for one drink +curl https://data.cambeerfestival.app/v1/festivals/cbf2025/ratingSummaries/beer-1 +# -> {"name":"festivals/cbf2025/ratingSummaries/beer-1","ratingCount":3,"averageRating":4.0} + +# % would recommend for one drink +curl https://data.cambeerfestival.app/v1/festivals/cbf2025/recommendationSummaries/beer-1 +# -> {"name":"...","responseCount":2,"recommendCount":1,"recommendRate":0.5} ``` #### D1 provisioning (one-time, before first deploy) diff --git a/cloudflare-worker/ratings.js b/cloudflare-worker/ratings.js index 1c4c0fc9..31f33063 100644 --- a/cloudflare-worker/ratings.js +++ b/cloudflare-worker/ratings.js @@ -1,208 +1,66 @@ /** - * Aggregate drink ratings API (v1). + * Ratings resource family for the /v1 API (AIP resource-oriented). * - * Endpoints (all under /v1/ratings, served by the same worker as the proxy): - * POST /v1/ratings upsert a device's rating - * DELETE /v1/ratings remove a device's rating - * GET /v1/ratings/{festivalId}/{drinkId} aggregate for one drink - * GET /v1/ratings/{festivalId} aggregate for every rated drink + * GET /v1/festivals/{f}/drinks/{d}/ratings/{device} get my rating + * PATCH /v1/festivals/{f}/drinks/{d}/ratings/{device} upsert my rating + * DELETE /v1/festivals/{f}/drinks/{d}/ratings/{device} remove my rating + * GET /v1/festivals/{f}/ratingSummaries/{d} aggregate for a drink + * GET /v1/festivals/{f}/ratingSummaries list (paginated) * - * Writes are local-first on the client; the server is the shared aggregate. - * Shared bucket/validation/routing plumbing lives in shared.js. + * Backed by the `ratings` table in D1. Writes are local-first on the client. */ -import { - validateIds, - jsonResponse, - parseJsonBody, - routeResource, -} from "./shared.js"; +import { handleResourceFamily, rfc3339 } from "./shared.js"; -/** - * Validate a write payload. POST requires `rating`; DELETE only needs ids. - * Returns { ok: true, value } or { ok: false, error }. - */ -export function validateWritePayload(body, { requireRating }) { - const ids = validateIds(body); - if (!ids.ok) return ids; - - const { rating } = body; - if (requireRating) { - if (!Number.isInteger(rating) || rating < 1 || rating > 5) { - return { ok: false, error: "rating must be an integer between 1 and 5" }; - } - } - return { ok: true, value: { ...ids.value, rating } }; -} - -/** Round an average to one decimal place, or null when there are no ratings. */ -export function formatAverage(average, count) { - if (!count || average == null) return null; - return Math.round(average * 10) / 10; -} - -/** Aggregate (count + average) for a single drink in a bucket. */ -async function readAggregate(db, bucket, festivalId, drinkId, deviceId) { - const agg = await db - .prepare( - "SELECT COUNT(*) AS count, AVG(rating) AS average " + - "FROM ratings WHERE bucket = ? AND festival_id = ? AND drink_id = ?", - ) - .bind(bucket, festivalId, drinkId) - .first(); - - let yourRating = null; - if (deviceId) { - const own = await db - .prepare( - "SELECT rating FROM ratings " + - "WHERE bucket = ? AND festival_id = ? AND drink_id = ? AND device_id = ?", - ) - .bind(bucket, festivalId, drinkId, deviceId) - .first(); - yourRating = own ? own.rating : null; - } - - const count = agg ? agg.count : 0; - return { - festivalId, - drinkId, - count, - average: formatAverage(agg ? agg.average : null, count), - yourRating, - }; +function round1(value) { + return Math.round(value * 10) / 10; } -async function handlePost(request, db, bucket, corsHeaders) { - const parsed = await parseJsonBody(request); - if (!parsed.ok) { - return jsonResponse({ error: "Invalid JSON body" }, 400, corsHeaders); - } - - const result = validateWritePayload(parsed.body, { requireRating: true }); - if (!result.ok) { - return jsonResponse({ error: result.error }, 400, corsHeaders); - } - - const { festivalId, drinkId, deviceId, rating } = result.value; - await db - .prepare( - "INSERT INTO ratings (bucket, festival_id, drink_id, device_id, rating, updated_at) " + - "VALUES (?, ?, ?, ?, ?, ?) " + - "ON CONFLICT (bucket, festival_id, drink_id, device_id) " + - "DO UPDATE SET rating = excluded.rating, updated_at = excluded.updated_at", - ) - .bind(bucket, festivalId, drinkId, deviceId, rating, Date.now()) - .run(); - - const aggregate = await readAggregate( - db, - bucket, - festivalId, - drinkId, - deviceId, - ); - return jsonResponse(aggregate, 200, corsHeaders); -} - -async function handleDelete(request, db, bucket, corsHeaders) { - const parsed = await parseJsonBody(request); - if (!parsed.ok) { - return jsonResponse({ error: "Invalid JSON body" }, 400, corsHeaders); - } - - const result = validateWritePayload(parsed.body, { requireRating: false }); - if (!result.ok) { - return jsonResponse({ error: result.error }, 400, corsHeaders); - } - - const { festivalId, drinkId, deviceId } = result.value; - await db - .prepare( - "DELETE FROM ratings " + - "WHERE bucket = ? AND festival_id = ? AND drink_id = ? AND device_id = ?", - ) - .bind(bucket, festivalId, drinkId, deviceId) - .run(); - - const aggregate = await readAggregate( - db, - bucket, - festivalId, - drinkId, - deviceId, - ); - return jsonResponse(aggregate, 200, corsHeaders); -} - -async function handleGetSingle( - db, - bucket, - festivalId, - drinkId, - deviceId, - corsHeaders, -) { - const aggregate = await readAggregate( - db, - bucket, - festivalId, - drinkId, - deviceId, - ); - return jsonResponse(aggregate, 200, corsHeaders); -} - -/** Batch: every rated drink for a festival, keyed by drink id. */ -async function handleGetFestival( - db, - bucket, - festivalId, - deviceId, - corsHeaders, -) { - const { results } = await db - .prepare( - "SELECT drink_id, COUNT(*) AS count, AVG(rating) AS average " + - "FROM ratings WHERE bucket = ? AND festival_id = ? GROUP BY drink_id", - ) - .bind(bucket, festivalId) - .all(); - - const own = new Map(); - if (deviceId) { - const ownRows = await db - .prepare( - "SELECT drink_id, rating FROM ratings " + - "WHERE bucket = ? AND festival_id = ? AND device_id = ?", - ) - .bind(bucket, festivalId, deviceId) - .all(); - for (const row of ownRows.results) { - own.set(row.drink_id, row.rating); +export const RATINGS_FAMILY = { + table: "ratings", + valueColumn: "rating", + writeCollection: "ratings", + summaryCollection: "ratingSummaries", + + /** Validate the Rating body { value: 1..5 }. */ + parseValue(body) { + if (body === null || typeof body !== "object") { + return { + ok: false, + reason: "INVALID_BODY", + message: "Body must be a JSON object", + }; } - } - - const aggregates = {}; - for (const row of results) { - aggregates[row.drink_id] = { - count: row.count, - average: formatAverage(row.average, row.count), - yourRating: own.has(row.drink_id) ? own.get(row.drink_id) : null, + const { value } = body; + if (!Number.isInteger(value) || value < 1 || value > 5) { + return { + ok: false, + reason: "RATING_VALUE_OUT_OF_RANGE", + message: "value must be an integer between 1 and 5", + }; + } + return { ok: true, columnValue: value }; + }, + + /** Serialize a Rating resource from a DB row { value, updated_at }. */ + serializeResource(name, row) { + return { name, value: row.value, updateTime: rfc3339(row.updated_at) }; + }, + + // Aggregate columns selected for summary single + list queries. + summaryColumns: "COUNT(*) AS agg_count, AVG(rating) AS agg_average", + + /** Build RatingSummary fields from an aggregate row. */ + summaryFields(row) { + const count = row.agg_count || 0; + return { + ratingCount: count, + averageRating: count ? round1(row.agg_average) : 0, }; - } - - return jsonResponse({ festivalId, aggregates }, 200, corsHeaders); -} + }, +}; -/** Route and handle a /v1/ratings request, or null if not a ratings path. */ +/** Route a ratings request, or null if the path is not a ratings path. */ export function handleRatings(request, url, env, corsHeaders) { - return routeResource(request, url, env, corsHeaders, { - basePath: "/v1/ratings", - db: "RATINGS_DB", - post: handlePost, - del: handleDelete, - getSingle: handleGetSingle, - getFestival: handleGetFestival, - }); + return handleResourceFamily(request, url, env, corsHeaders, RATINGS_FAMILY); } diff --git a/cloudflare-worker/recommendations.js b/cloudflare-worker/recommendations.js index 754489ea..4b666927 100644 --- a/cloudflare-worker/recommendations.js +++ b/cloudflare-worker/recommendations.js @@ -1,223 +1,78 @@ /** - * "Would recommend" API (v1). + * Recommendations resource family for the /v1 API (AIP resource-oriented). * - * A yes/no signal, separate from the star rating, so we can surface a - * "% would recommend" for each drink. Endpoints mirror the ratings API and - * share the same D1 database (RATINGS_DB), in a separate `recommendations` - * table: - * POST /v1/recommendations upsert a device's yes/no - * DELETE /v1/recommendations remove a device's answer - * GET /v1/recommendations/{festivalId}/{drinkId} aggregate for one drink - * GET /v1/recommendations/{festivalId} aggregate for every drink + * GET /v1/festivals/{f}/drinks/{d}/recommendations/{device} get my answer + * PATCH /v1/festivals/{f}/drinks/{d}/recommendations/{device} upsert my answer + * DELETE /v1/festivals/{f}/drinks/{d}/recommendations/{device} remove my answer + * GET /v1/festivals/{f}/recommendationSummaries/{d} aggregate for a drink + * GET /v1/festivals/{f}/recommendationSummaries list (paginated) * - * `recommend` is stored as 0/1 (SQLite has no boolean) and exposed as a JSON - * boolean. The aggregate reports total responses, the count of "yes", and the - * percentage that would recommend. + * A yes/no signal separate from the star rating, surfacing a "% would + * recommend". Backed by the `recommendations` table (`recommend` stored as 0/1). */ -import { - validateIds, - jsonResponse, - parseJsonBody, - routeResource, -} from "./shared.js"; +import { handleResourceFamily, rfc3339 } from "./shared.js"; -/** - * Validate a write payload. POST requires a boolean `recommend`; DELETE only - * needs ids. Returns { ok: true, value } or { ok: false, error }. - */ -export function validateRecommendPayload(body, { requireRecommend }) { - const ids = validateIds(body); - if (!ids.ok) return ids; - - const { recommend } = body; - if (requireRecommend && typeof recommend !== "boolean") { - return { ok: false, error: "recommend must be a boolean" }; - } - return { ok: true, value: { ...ids.value, recommend } }; -} - -/** Whole-number "% would recommend", or null when there are no responses. */ -export function formatPercent(recommendCount, count) { - if (!count) return null; - return Math.round((recommendCount / count) * 100); -} - -function aggregateShape(count, recommendCount, youRecommend) { - return { - count, - recommendCount, - recommendPercent: formatPercent(recommendCount, count), - youRecommend, - }; -} - -/** Aggregate (responses + yes count + percentage) for one drink. */ -async function readRecommendation(db, bucket, festivalId, drinkId, deviceId) { - const agg = await db - .prepare( - "SELECT COUNT(*) AS count, SUM(recommend) AS yes " + - "FROM recommendations WHERE bucket = ? AND festival_id = ? AND drink_id = ?", - ) - .bind(bucket, festivalId, drinkId) - .first(); - - let youRecommend = null; - if (deviceId) { - const own = await db - .prepare( - "SELECT recommend FROM recommendations " + - "WHERE bucket = ? AND festival_id = ? AND drink_id = ? AND device_id = ?", - ) - .bind(bucket, festivalId, drinkId, deviceId) - .first(); - youRecommend = own ? Boolean(own.recommend) : null; - } - - const count = agg ? agg.count : 0; - const recommendCount = agg && agg.yes != null ? agg.yes : 0; - return { - festivalId, - drinkId, - ...aggregateShape(count, recommendCount, youRecommend), - }; -} - -async function handlePost(request, db, bucket, corsHeaders) { - const parsed = await parseJsonBody(request); - if (!parsed.ok) { - return jsonResponse({ error: "Invalid JSON body" }, 400, corsHeaders); - } - - const result = validateRecommendPayload(parsed.body, { - requireRecommend: true, - }); - if (!result.ok) { - return jsonResponse({ error: result.error }, 400, corsHeaders); - } - - const { festivalId, drinkId, deviceId, recommend } = result.value; - await db - .prepare( - "INSERT INTO recommendations (bucket, festival_id, drink_id, device_id, recommend, updated_at) " + - "VALUES (?, ?, ?, ?, ?, ?) " + - "ON CONFLICT (bucket, festival_id, drink_id, device_id) " + - "DO UPDATE SET recommend = excluded.recommend, updated_at = excluded.updated_at", - ) - .bind(bucket, festivalId, drinkId, deviceId, recommend ? 1 : 0, Date.now()) - .run(); - - const aggregate = await readRecommendation( - db, - bucket, - festivalId, - drinkId, - deviceId, - ); - return jsonResponse(aggregate, 200, corsHeaders); +function round2(value) { + return Math.round(value * 100) / 100; } -async function handleDelete(request, db, bucket, corsHeaders) { - const parsed = await parseJsonBody(request); - if (!parsed.ok) { - return jsonResponse({ error: "Invalid JSON body" }, 400, corsHeaders); - } - - const result = validateRecommendPayload(parsed.body, { - requireRecommend: false, - }); - if (!result.ok) { - return jsonResponse({ error: result.error }, 400, corsHeaders); - } - - const { festivalId, drinkId, deviceId } = result.value; - await db - .prepare( - "DELETE FROM recommendations " + - "WHERE bucket = ? AND festival_id = ? AND drink_id = ? AND device_id = ?", - ) - .bind(bucket, festivalId, drinkId, deviceId) - .run(); - - const aggregate = await readRecommendation( - db, - bucket, - festivalId, - drinkId, - deviceId, - ); - return jsonResponse(aggregate, 200, corsHeaders); -} - -async function handleGetSingle( - db, - bucket, - festivalId, - drinkId, - deviceId, - corsHeaders, -) { - const aggregate = await readRecommendation( - db, - bucket, - festivalId, - drinkId, - deviceId, - ); - return jsonResponse(aggregate, 200, corsHeaders); -} - -/** Batch: every drink with a response for a festival, keyed by drink id. */ -async function handleGetFestival( - db, - bucket, - festivalId, - deviceId, - corsHeaders, -) { - const { results } = await db - .prepare( - "SELECT drink_id, COUNT(*) AS count, SUM(recommend) AS yes " + - "FROM recommendations WHERE bucket = ? AND festival_id = ? GROUP BY drink_id", - ) - .bind(bucket, festivalId) - .all(); - - const own = new Map(); - if (deviceId) { - const ownRows = await db - .prepare( - "SELECT drink_id, recommend FROM recommendations " + - "WHERE bucket = ? AND festival_id = ? AND device_id = ?", - ) - .bind(bucket, festivalId, deviceId) - .all(); - for (const row of ownRows.results) { - own.set(row.drink_id, Boolean(row.recommend)); +export const RECOMMENDATIONS_FAMILY = { + table: "recommendations", + valueColumn: "recommend", + writeCollection: "recommendations", + summaryCollection: "recommendationSummaries", + + /** Validate the Recommendation body { wouldRecommend: bool }. */ + parseValue(body) { + if (body === null || typeof body !== "object") { + return { + ok: false, + reason: "INVALID_BODY", + message: "Body must be a JSON object", + }; } - } - - const aggregates = {}; - for (const row of results) { - const recommendCount = row.yes != null ? row.yes : 0; - aggregates[row.drink_id] = aggregateShape( - row.count, - recommendCount, - own.has(row.drink_id) ? own.get(row.drink_id) : null, - ); - } - - return jsonResponse({ festivalId, aggregates }, 200, corsHeaders); -} - -/** Route and handle a /v1/recommendations request, or null if not one. */ + const { wouldRecommend } = body; + if (typeof wouldRecommend !== "boolean") { + return { + ok: false, + reason: "RECOMMENDATION_VALUE_INVALID", + message: "wouldRecommend must be a boolean", + }; + } + return { ok: true, columnValue: wouldRecommend ? 1 : 0 }; + }, + + /** Serialize a Recommendation resource from a DB row { value, updated_at }. */ + serializeResource(name, row) { + return { + name, + wouldRecommend: Boolean(row.value), + updateTime: rfc3339(row.updated_at), + }; + }, + + summaryColumns: "COUNT(*) AS agg_count, SUM(recommend) AS agg_yes", + + /** Build RecommendationSummary fields from an aggregate row. */ + summaryFields(row) { + const count = row.agg_count || 0; + const yes = row.agg_yes != null ? row.agg_yes : 0; + return { + responseCount: count, + recommendCount: yes, + recommendRate: count ? round2(yes / count) : 0, + }; + }, +}; + +/** Route a recommendations request, or null if not a recommendations path. */ export function handleRecommendations(request, url, env, corsHeaders) { - return routeResource(request, url, env, corsHeaders, { - basePath: "/v1/recommendations", - db: "RATINGS_DB", - post: handlePost, - del: handleDelete, - getSingle: handleGetSingle, - getFestival: handleGetFestival, - }); + return handleResourceFamily( + request, + url, + env, + corsHeaders, + RECOMMENDATIONS_FAMILY, + ); } diff --git a/cloudflare-worker/shared.js b/cloudflare-worker/shared.js index 610fc010..2dbc6652 100644 --- a/cloudflare-worker/shared.js +++ b/cloudflare-worker/shared.js @@ -1,28 +1,28 @@ /** - * Shared helpers for the /v1 "my festival" APIs (ratings, recommendations, …). + * Shared engine for the resource-oriented /v1 "my festival" APIs. * - * These resources are structurally identical — a device upserts a signal for a - * drink and reads back a bucket-scoped aggregate — so the bucket resolution, - * id validation, JSON plumbing and REST routing live here once. + * Routes follow AIP resource names. For a "family" (ratings, recommendations) + * with write-collection W and summary-collection S: + * + * GET /v1/festivals/{f}/drinks/{d}/W/{device} get this device's record + * PATCH /v1/festivals/{f}/drinks/{d}/W/{device} upsert (allow_missing) + * DELETE /v1/festivals/{f}/drinks/{d}/W/{device} remove this device's record + * GET /v1/festivals/{f}/S/{d} aggregate for one drink + * GET /v1/festivals/{f}/S list aggregates (paginated) + * + * Errors use the structured google.rpc.Status shape (AIP-193). Lists paginate + * with opaque keyset tokens (AIP-158). */ -export const MAX_ID_LENGTH = 200; +const MAX_ID_LENGTH = 200; +const DEFAULT_PAGE_SIZE = 100; +const MAX_PAGE_SIZE = 1000; +const ERROR_DOMAIN = "cambeerfestival.app"; -// Only the production web origin maps to the 'prod' bucket. Everything else -// (staging, Pages previews, localhost, tunnels, native apps with no Origin) -// lands in 'test'. This mirrors EnvironmentService.isProductionHost on the -// client and keeps a single worker deploy serving both buckets safely — -// bucket is a data-hygiene boundary, not a security one. export function isProductionOrigin(origin) { return origin === "https://cambeerfestival.app"; } -/** - * Resolve the storage bucket for a request. - * - * An explicit `RATINGS_BUCKET` worker var wins (lets us pin a deploy to a - * bucket during rollout); otherwise it is derived from the request origin. - */ export function resolveBucket(origin, env) { if (env && typeof env.RATINGS_BUCKET === "string" && env.RATINGS_BUCKET) { return env.RATINGS_BUCKET; @@ -30,7 +30,11 @@ export function resolveBucket(origin, env) { return isProductionOrigin(origin) ? "prod" : "test"; } -export function isValidId(value) { +export function rfc3339(epochMillis) { + return new Date(epochMillis).toISOString(); +} + +function isValidId(value) { return ( typeof value === "string" && value.length > 0 && @@ -38,26 +42,7 @@ export function isValidId(value) { ); } -/** - * Validate the identity fields every write shares. Returns - * { ok: true, value: { festivalId, drinkId, deviceId } } or { ok: false, error }. - */ -export function validateIds(body) { - if (body === null || typeof body !== "object") { - return { ok: false, error: "Request body must be a JSON object" }; - } - const { festivalId, drinkId, deviceId } = body; - if (!isValidId(festivalId)) { - return { ok: false, error: "festivalId is required" }; - } - if (!isValidId(drinkId)) { - return { ok: false, error: "drinkId is required" }; - } - if (!isValidId(deviceId)) { - return { ok: false, error: "deviceId is required" }; - } - return { ok: true, value: { festivalId, drinkId, deviceId } }; -} +// --- Responses (AIP-193) --------------------------------------------------- export function jsonResponse(body, status, corsHeaders) { return new Response(JSON.stringify(body), { @@ -69,77 +54,373 @@ export function jsonResponse(body, status, corsHeaders) { }); } -export async function parseJsonBody(request) { +/** Structured error body per AIP-193 (google.rpc.Status + ErrorInfo). */ +export function errorResponse( + httpCode, + status, + message, + reason, + corsHeaders, + metadata, +) { + const errorInfo = { + "@type": "type.googleapis.com/google.rpc.ErrorInfo", + reason, + domain: ERROR_DOMAIN, + }; + if (metadata) errorInfo.metadata = metadata; + return jsonResponse( + { error: { code: httpCode, message, status, details: [errorInfo] } }, + httpCode, + corsHeaders, + ); +} + +// --- Pagination (AIP-158) -------------------------------------------------- + +/** Encode a keyset cursor (last drink id) as an opaque URL-safe token. */ +export function encodePageToken(drinkId) { + return btoa(unescape(encodeURIComponent(drinkId))) + .replace(/\+/g, "-") + .replace(/\//g, "_") + .replace(/=+$/, ""); +} + +/** Decode a page token back to its cursor, or null if absent. */ +export function decodePageToken(token) { + if (!token) return null; try { - return { ok: true, body: await request.json() }; + const b64 = token.replace(/-/g, "+").replace(/_/g, "/"); + return decodeURIComponent(escape(atob(b64))); } catch { - return { ok: false }; + return undefined; // signal "invalid token" } } +/** Resolve an effective page size, or { error } for a bad value. */ +export function resolvePageSize(raw) { + if (raw == null || raw === "") return { value: DEFAULT_PAGE_SIZE }; + const n = Number(raw); + if (!Number.isInteger(n) || n < 0) return { error: true }; + if (n === 0) return { value: DEFAULT_PAGE_SIZE }; + return { value: Math.min(n, MAX_PAGE_SIZE) }; +} + +// --- Routing --------------------------------------------------------------- + +function parseV1Path(pathname) { + if (pathname !== "/v1" && !pathname.startsWith("/v1/")) return null; + return pathname + .slice("/v1/".length) + .split("/") + .filter((s) => s.length > 0) + .map((s) => decodeURIComponent(s)); +} + /** - * Route a REST request for a /v1 resource. Returns a Response, or null if the - * path is not for this resource (so the caller can fall through). + * Handle a request for one resource family. Returns a Response if the path + * belongs to this family, otherwise null so the caller can try the next. * - * Routes: - * POST {basePath} -> handlers.post(request, db, bucket, cors) - * DELETE {basePath} -> handlers.del(request, db, bucket, cors) - * GET {basePath}/{festivalId} -> handlers.getFestival(db, bucket, festivalId, deviceId, cors) - * GET {basePath}/{festivalId}/{drinkId}-> handlers.getSingle(db, bucket, festivalId, drinkId, deviceId, cors) + * `family` provides: table, valueColumn, writeCollection, summaryCollection, + * parseValue(body), serializeResource(name,row), summaryColumns, + * summaryFields(row). */ -export async function routeResource(request, url, env, corsHeaders, handlers) { - const { basePath, db: dbBinding } = handlers; - const prefix = `${basePath}/`; - if (url.pathname !== basePath && !url.pathname.startsWith(prefix)) { +export async function handleResourceFamily( + request, + url, + env, + corsHeaders, + family, +) { + const segments = parseV1Path(url.pathname); + if (!segments || segments[0] !== "festivals" || segments.length < 3) { return null; } - if (!env || !env[dbBinding]) { - return jsonResponse( - { error: "Storage is not configured" }, + // /v1/festivals/{f}/drinks/{d}/{writeCollection}/{device} + const isWrite = + segments.length === 6 && + segments[2] === "drinks" && + segments[4] === family.writeCollection; + // /v1/festivals/{f}/{summaryCollection}[/{drink}] + const isSummary = + (segments.length === 3 || segments.length === 4) && + segments[2] === family.summaryCollection; + + if (!isWrite && !isSummary) return null; + + if (!env || !env.RATINGS_DB) { + return errorResponse( 503, + "UNAVAILABLE", + "Storage is not configured", + "STORAGE_UNCONFIGURED", corsHeaders, ); } const origin = request.headers.get("Origin") || ""; const bucket = resolveBucket(origin, env); - const db = env[dbBinding]; + const db = env.RATINGS_DB; - if (url.pathname === basePath) { - if (request.method === "POST") { - return handlers.post(request, db, bucket, corsHeaders); + if (isWrite) { + const [, festivalId, , drinkId, , deviceId] = segments; + if (!isValidId(festivalId) || !isValidId(drinkId) || !isValidId(deviceId)) { + return errorResponse( + 400, + "INVALID_ARGUMENT", + "Invalid resource name", + "INVALID_RESOURCE_NAME", + corsHeaders, + ); } - if (request.method === "DELETE") { - return handlers.del(request, db, bucket, corsHeaders); + const ctx = { + db, + bucket, + family, + festivalId, + drinkId, + deviceId, + corsHeaders, + }; + switch (request.method) { + case "GET": + return getRecord(ctx); + case "PATCH": + return upsertRecord(request, ctx); + case "DELETE": + return deleteRecord(ctx); + default: + return methodNotAllowed(corsHeaders); } - return jsonResponse({ error: "Method not allowed" }, 405, corsHeaders); } - if (request.method !== "GET") { - return jsonResponse({ error: "Method not allowed" }, 405, corsHeaders); + // Summary read / list + if (request.method !== "GET") return methodNotAllowed(corsHeaders); + const festivalId = segments[1]; + if (!isValidId(festivalId)) { + return errorResponse( + 400, + "INVALID_ARGUMENT", + "Invalid resource name", + "INVALID_RESOURCE_NAME", + corsHeaders, + ); + } + if (segments.length === 4) { + return getSummary({ + db, + bucket, + family, + festivalId, + drinkId: segments[3], + corsHeaders, + }); } + return listSummaries({ db, bucket, family, festivalId, url, corsHeaders }); +} - const segments = url.pathname - .slice(prefix.length) - .split("/") - .filter((s) => s.length > 0) - .map((s) => decodeURIComponent(s)); - const deviceId = url.searchParams.get("deviceId") || null; +function methodNotAllowed(corsHeaders) { + return errorResponse( + 405, + "UNIMPLEMENTED", + "Method not allowed for this resource", + "METHOD_NOT_ALLOWED", + corsHeaders, + ); +} + +function writeResourceName(family, festivalId, drinkId, deviceId) { + return `festivals/${festivalId}/drinks/${drinkId}/${family.writeCollection}/${deviceId}`; +} + +function summaryResourceName(family, festivalId, drinkId) { + return `festivals/${festivalId}/${family.summaryCollection}/${drinkId}`; +} - if (segments.length === 1) { - return handlers.getFestival(db, bucket, segments[0], deviceId, corsHeaders); +async function readRow(ctx) { + const { db, family, bucket, festivalId, drinkId, deviceId } = ctx; + return db + .prepare( + `SELECT ${family.valueColumn} AS value, updated_at FROM ${family.table} ` + + "WHERE bucket = ? AND festival_id = ? AND drink_id = ? AND device_id = ?", + ) + .bind(bucket, festivalId, drinkId, deviceId) + .first(); +} + +async function getRecord(ctx) { + const { family, festivalId, drinkId, deviceId, corsHeaders } = ctx; + const row = await readRow(ctx); + if (!row) { + return errorResponse( + 404, + "NOT_FOUND", + "No such rating", + "NOT_FOUND", + corsHeaders, + ); } - if (segments.length === 2) { - return handlers.getSingle( - db, - bucket, - segments[0], - segments[1], - deviceId, + const name = writeResourceName(family, festivalId, drinkId, deviceId); + return jsonResponse(family.serializeResource(name, row), 200, corsHeaders); +} + +async function upsertRecord(request, ctx) { + const { db, family, bucket, festivalId, drinkId, deviceId, corsHeaders } = + ctx; + + let body; + try { + body = await request.json(); + } catch { + return errorResponse( + 400, + "INVALID_ARGUMENT", + "Invalid JSON body", + "INVALID_BODY", + corsHeaders, + ); + } + + const parsed = family.parseValue(body); + if (!parsed.ok) { + return errorResponse( + 400, + "INVALID_ARGUMENT", + parsed.message, + parsed.reason, corsHeaders, ); } - return jsonResponse({ error: "Not found" }, 404, corsHeaders); + await db + .prepare( + `INSERT INTO ${family.table} ` + + `(bucket, festival_id, drink_id, device_id, ${family.valueColumn}, updated_at) ` + + "VALUES (?, ?, ?, ?, ?, ?) " + + "ON CONFLICT (bucket, festival_id, drink_id, device_id) " + + `DO UPDATE SET ${family.valueColumn} = excluded.${family.valueColumn}, ` + + "updated_at = excluded.updated_at", + ) + .bind(bucket, festivalId, drinkId, deviceId, parsed.columnValue, Date.now()) + .run(); + + const row = await readRow(ctx); + const name = writeResourceName(family, festivalId, drinkId, deviceId); + return jsonResponse(family.serializeResource(name, row), 200, corsHeaders); +} + +async function deleteRecord(ctx) { + const { db, family, bucket, festivalId, drinkId, deviceId, corsHeaders } = + ctx; + const result = await db + .prepare( + `DELETE FROM ${family.table} ` + + "WHERE bucket = ? AND festival_id = ? AND drink_id = ? AND device_id = ?", + ) + .bind(bucket, festivalId, drinkId, deviceId) + .run(); + + // AIP-135: deleting a missing resource is NOT_FOUND. + const changes = result.meta ? result.meta.changes : 0; + if (!changes) { + return errorResponse( + 404, + "NOT_FOUND", + "No such rating", + "NOT_FOUND", + ctx.corsHeaders, + ); + } + return jsonResponse({}, 200, corsHeaders); +} + +async function getSummary(ctx) { + const { db, family, bucket, festivalId, drinkId, corsHeaders } = ctx; + const row = await db + .prepare( + `SELECT ${family.summaryColumns} FROM ${family.table} ` + + "WHERE bucket = ? AND festival_id = ? AND drink_id = ?", + ) + .bind(bucket, festivalId, drinkId) + .first(); + + const name = summaryResourceName(family, festivalId, drinkId); + return jsonResponse( + { name, ...family.summaryFields(row || {}) }, + 200, + corsHeaders, + ); +} + +async function listSummaries(ctx) { + const { db, family, bucket, festivalId, url, corsHeaders } = ctx; + + const sizeResult = resolvePageSize(url.searchParams.get("page_size")); + if (sizeResult.error) { + return errorResponse( + 400, + "INVALID_ARGUMENT", + "page_size must be >= 0", + "INVALID_PAGE_SIZE", + corsHeaders, + ); + } + const pageSize = sizeResult.value; + + const cursor = decodePageToken(url.searchParams.get("page_token")); + if (cursor === undefined) { + return errorResponse( + 400, + "INVALID_ARGUMENT", + "Invalid page_token", + "INVALID_PAGE_TOKEN", + corsHeaders, + ); + } + + const where = ["bucket = ?", "festival_id = ?"]; + const binds = [bucket, festivalId]; + if (cursor !== null) { + where.push("drink_id > ?"); + binds.push(cursor); + } + + // Fetch one extra row to detect whether another page follows. + const { results } = await db + .prepare( + `SELECT drink_id, ${family.summaryColumns} FROM ${family.table} ` + + `WHERE ${where.join(" AND ")} GROUP BY drink_id ORDER BY drink_id LIMIT ?`, + ) + .bind(...binds, pageSize + 1) + .all(); + + const page = results.slice(0, pageSize); + const items = page.map((row) => ({ + name: summaryResourceName(family, festivalId, row.drink_id), + ...family.summaryFields(row), + })); + + let nextPageToken = ""; + if (results.length > pageSize) { + nextPageToken = encodePageToken(page[page.length - 1].drink_id); + } + + const totalRow = await db + .prepare( + `SELECT COUNT(DISTINCT drink_id) AS n FROM ${family.table} ` + + "WHERE bucket = ? AND festival_id = ?", + ) + .bind(bucket, festivalId) + .first(); + + return jsonResponse( + { + [family.summaryCollection]: items, + nextPageToken, + totalSize: totalRow ? totalRow.n : 0, + }, + 200, + corsHeaders, + ); } diff --git a/cloudflare-worker/test/cors.test.js b/cloudflare-worker/test/cors.test.js index f477cb26..3c1cdbbe 100644 --- a/cloudflare-worker/test/cors.test.js +++ b/cloudflare-worker/test/cors.test.js @@ -160,7 +160,7 @@ describe("CORS preflight (OPTIONS)", () => { "OPTIONS", ); expect(response.headers.get("Access-Control-Allow-Methods")).toBe( - "GET, POST, DELETE, OPTIONS", + "GET, PATCH, DELETE, OPTIONS", ); expect(response.headers.get("Access-Control-Allow-Headers")).toBe( "Content-Type", diff --git a/cloudflare-worker/test/ratings.test.js b/cloudflare-worker/test/ratings.test.js index a557c6f7..5f57e490 100644 --- a/cloudflare-worker/test/ratings.test.js +++ b/cloudflare-worker/test/ratings.test.js @@ -5,8 +5,14 @@ import { waitOnExecutionContext, } from "cloudflare:test"; import worker from "../worker.js"; -import { isProductionOrigin, resolveBucket } from "../shared.js"; -import { validateWritePayload, formatAverage } from "../ratings.js"; +import { + isProductionOrigin, + resolveBucket, + resolvePageSize, + encodePageToken, + decodePageToken, +} from "../shared.js"; +import { RATINGS_FAMILY } from "../ratings.js"; const TEST_ORIGIN = "http://localhost:8080"; // non-prod -> 'test' bucket const PROD_ORIGIN = "https://cambeerfestival.app"; // -> 'prod' bucket @@ -24,328 +30,266 @@ async function send(method, path, { body, origin = TEST_ORIGIN } = {}) { return response; } -const post = (body, opts) => send("POST", "/v1/ratings", { body, ...opts }); -const del = (body, opts) => send("DELETE", "/v1/ratings", { body, ...opts }); +const ratingPath = (f, d, device) => + `/v1/festivals/${f}/drinks/${d}/ratings/${device}`; +const upsert = (f, d, device, value, opts) => + send("PATCH", ratingPath(f, d, device), { body: { value }, ...opts }); -// This pool version shares D1 storage across tests in a file, so reset the -// table before each test to keep aggregates deterministic. beforeEach(async () => { await env.RATINGS_DB.prepare("DELETE FROM ratings").run(); }); describe("ratings — pure helpers", () => { - it("only the production web origin is a production origin", () => { - expect(isProductionOrigin("https://cambeerfestival.app")).toBe(true); - expect(isProductionOrigin("https://staging.cambeerfestival.app")).toBe( - false, - ); - expect(isProductionOrigin("http://localhost:8080")).toBe(false); - expect(isProductionOrigin("")).toBe(false); - }); - - it("resolveBucket derives from origin", () => { + it("resolves the bucket from origin, with override", () => { + expect(isProductionOrigin(PROD_ORIGIN)).toBe(true); expect(resolveBucket(PROD_ORIGIN, {})).toBe("prod"); expect(resolveBucket(TEST_ORIGIN, {})).toBe("test"); - expect(resolveBucket("", {})).toBe("test"); + expect(resolveBucket(PROD_ORIGIN, { RATINGS_BUCKET: "test" })).toBe("test"); }); - it("resolveBucket honours an explicit RATINGS_BUCKET override", () => { - expect(resolveBucket(PROD_ORIGIN, { RATINGS_BUCKET: "test" })).toBe("test"); - expect(resolveBucket(TEST_ORIGIN, { RATINGS_BUCKET: "prod" })).toBe("prod"); - expect(resolveBucket(TEST_ORIGIN, { RATINGS_BUCKET: "" })).toBe("test"); + it("resolvePageSize applies defaults, caps, and rejects negatives", () => { + expect(resolvePageSize(null).value).toBe(100); + expect(resolvePageSize("").value).toBe(100); + expect(resolvePageSize("0").value).toBe(100); + expect(resolvePageSize("25").value).toBe(25); + expect(resolvePageSize("9999").value).toBe(1000); + expect(resolvePageSize("-1").error).toBe(true); }); - it("validateWritePayload accepts a well-formed rating", () => { - const result = validateWritePayload( - { - festivalId: "cbf2025", - drinkId: "beer-1", - deviceId: "dev-1", - rating: 4, - }, - { requireRating: true }, - ); - expect(result.ok).toBe(true); - expect(result.value.rating).toBe(4); + it("page tokens round-trip and reject garbage", () => { + expect(decodePageToken(encodePageToken("beer-1"))).toBe("beer-1"); + expect(decodePageToken("")).toBe(null); + expect(decodePageToken(null)).toBe(null); + expect(decodePageToken("!!!not-base64!!!")).toBe(undefined); }); - it("validateWritePayload rejects out-of-range and non-integer ratings", () => { - for (const rating of [0, 6, 3.5, "4", null, undefined]) { - const result = validateWritePayload( - { festivalId: "f", drinkId: "d", deviceId: "x", rating }, - { requireRating: true }, - ); - expect(result.ok).toBe(false); + it("RATINGS_FAMILY.parseValue enforces 1..5 integer", () => { + expect(RATINGS_FAMILY.parseValue({ value: 4 })).toMatchObject({ + ok: true, + columnValue: 4, + }); + for (const value of [0, 6, 3.5, "4", null]) { + expect(RATINGS_FAMILY.parseValue({ value }).ok).toBe(false); } + expect(RATINGS_FAMILY.parseValue(null).ok).toBe(false); }); - it("validateWritePayload rejects missing ids", () => { - expect( - validateWritePayload( - { drinkId: "d", deviceId: "x", rating: 3 }, - { requireRating: true }, - ).ok, - ).toBe(false); - expect( - validateWritePayload( - { festivalId: "f", deviceId: "x", rating: 3 }, - { requireRating: true }, - ).ok, - ).toBe(false); + it("RATINGS_FAMILY.summaryFields handles empty and populated rows", () => { + expect(RATINGS_FAMILY.summaryFields({})).toEqual({ + ratingCount: 0, + averageRating: 0, + }); expect( - validateWritePayload( - { festivalId: "f", drinkId: "d", rating: 3 }, - { requireRating: true }, - ).ok, - ).toBe(false); - }); - - it("validateWritePayload skips rating when not required (DELETE)", () => { - const result = validateWritePayload( - { festivalId: "f", drinkId: "d", deviceId: "x" }, - { requireRating: false }, - ); - expect(result.ok).toBe(true); - }); - - it("formatAverage rounds to one decimal and is null when empty", () => { - expect(formatAverage(4.25, 4)).toBe(4.3); - expect(formatAverage(3, 1)).toBe(3); - expect(formatAverage(null, 0)).toBe(null); - expect(formatAverage(5, 0)).toBe(null); + RATINGS_FAMILY.summaryFields({ agg_count: 2, agg_average: 4.5 }), + ).toEqual({ ratingCount: 2, averageRating: 4.5 }); }); }); -describe("ratings — POST upsert", () => { - it("records a rating and returns the aggregate", async () => { - const response = await post({ - festivalId: "cbf2025", - drinkId: "beer-1", - deviceId: "dev-1", - rating: 4, - }); +describe("ratings — upsert (PATCH)", () => { + it("creates a rating and returns the resource", async () => { + const response = await upsert("cbf2025", "beer-1", "dev-1", 4); expect(response.status).toBe(200); const data = await response.json(); - expect(data).toMatchObject({ - festivalId: "cbf2025", - drinkId: "beer-1", - count: 1, - average: 4, - yourRating: 4, - }); + expect(data.name).toBe("festivals/cbf2025/drinks/beer-1/ratings/dev-1"); + expect(data.value).toBe(4); + expect(typeof data.updateTime).toBe("string"); + expect(Number.isNaN(Date.parse(data.updateTime))).toBe(false); }); - it("re-rating from the same device updates rather than duplicates", async () => { - await post({ - festivalId: "cbf2025", - drinkId: "beer-1", - deviceId: "dev-1", - rating: 2, - }); - const response = await post({ - festivalId: "cbf2025", - drinkId: "beer-1", - deviceId: "dev-1", - rating: 5, - }); - const data = await response.json(); - expect(data.count).toBe(1); - expect(data.average).toBe(5); - expect(data.yourRating).toBe(5); + it("re-rating updates in place rather than duplicating", async () => { + await upsert("cbf2025", "beer-1", "dev-1", 2); + await upsert("cbf2025", "beer-1", "dev-1", 5); + const summary = await send( + "GET", + "/v1/festivals/cbf2025/ratingSummaries/beer-1", + ); + const data = await summary.json(); + expect(data.ratingCount).toBe(1); + expect(data.averageRating).toBe(5); }); - it("aggregates across multiple devices", async () => { - await post({ - festivalId: "cbf2025", - drinkId: "beer-1", - deviceId: "dev-1", - rating: 4, - }); - await post({ - festivalId: "cbf2025", - drinkId: "beer-1", - deviceId: "dev-2", - rating: 5, - }); - const response = await post({ - festivalId: "cbf2025", - drinkId: "beer-1", - deviceId: "dev-3", - rating: 3, - }); - const data = await response.json(); - expect(data.count).toBe(3); - expect(data.average).toBe(4); // (4+5+3)/3 - expect(data.yourRating).toBe(3); // dev-3 + it("rejects an out-of-range value with a structured error", async () => { + const response = await upsert("cbf2025", "beer-1", "dev-1", 9); + expect(response.status).toBe(400); + const { error } = await response.json(); + expect(error.code).toBe(400); + expect(error.status).toBe("INVALID_ARGUMENT"); + expect(error.details[0].reason).toBe("RATING_VALUE_OUT_OF_RANGE"); + expect(error.details[0].domain).toBe("cambeerfestival.app"); }); -}); -describe("ratings — validation", () => { - it("rejects an invalid rating with 400", async () => { - const response = await post({ - festivalId: "cbf2025", - drinkId: "beer-1", - deviceId: "dev-1", - rating: 9, - }); + it("rejects malformed JSON", async () => { + const response = await send( + "PATCH", + ratingPath("cbf2025", "beer-1", "dev-1"), + { + body: "{not json", + }, + ); expect(response.status).toBe(400); + expect((await response.json()).error.details[0].reason).toBe( + "INVALID_BODY", + ); + }); +}); + +describe("ratings — get/delete record", () => { + it("gets a device's own rating", async () => { + await upsert("cbf2025", "beer-1", "dev-1", 3); + const response = await send( + "GET", + ratingPath("cbf2025", "beer-1", "dev-1"), + ); + expect(response.status).toBe(200); + expect((await response.json()).value).toBe(3); }); - it("rejects missing fields with 400", async () => { - const response = await post({ drinkId: "beer-1", rating: 4 }); - expect(response.status).toBe(400); + it("returns 404 for a missing rating", async () => { + const response = await send( + "GET", + ratingPath("cbf2025", "beer-1", "ghost"), + ); + expect(response.status).toBe(404); + expect((await response.json()).error.status).toBe("NOT_FOUND"); }); - it("rejects malformed JSON with 400", async () => { - const response = await post("{not json", {}); - expect(response.status).toBe(400); + it("deletes a rating, then reads 404", async () => { + await upsert("cbf2025", "beer-1", "dev-1", 3); + const del = await send("DELETE", ratingPath("cbf2025", "beer-1", "dev-1")); + expect(del.status).toBe(200); + expect(await del.json()).toEqual({}); + const after = await send("GET", ratingPath("cbf2025", "beer-1", "dev-1")); + expect(after.status).toBe(404); }); - it("rejects unsupported methods on the collection with 405", async () => { - const response = await send("PUT", "/v1/ratings", { - body: { festivalId: "f", drinkId: "d", deviceId: "x", rating: 3 }, - }); - expect(response.status).toBe(405); + it("deleting a missing rating is 404 (AIP-135)", async () => { + const response = await send( + "DELETE", + ratingPath("cbf2025", "beer-1", "ghost"), + ); + expect(response.status).toBe(404); }); }); -describe("ratings — GET", () => { - it("returns an empty aggregate for an unrated drink", async () => { - const response = await send("GET", "/v1/ratings/cbf2025/never-rated"); - expect(response.status).toBe(200); +describe("ratings — summaries", () => { + it("aggregates across devices", async () => { + await upsert("cbf2025", "beer-1", "dev-1", 4); + await upsert("cbf2025", "beer-1", "dev-2", 5); + await upsert("cbf2025", "beer-1", "dev-3", 3); + const response = await send( + "GET", + "/v1/festivals/cbf2025/ratingSummaries/beer-1", + ); const data = await response.json(); - expect(data).toMatchObject({ - count: 0, - average: null, - yourRating: null, - }); + expect(data.name).toBe("festivals/cbf2025/ratingSummaries/beer-1"); + expect(data.ratingCount).toBe(3); + expect(data.averageRating).toBe(4); }); - it("includes yourRating only when deviceId is supplied", async () => { - await post({ - festivalId: "cbf2025", - drinkId: "beer-1", - deviceId: "dev-1", - rating: 4, + it("returns an empty summary for an unrated drink", async () => { + const response = await send( + "GET", + "/v1/festivals/cbf2025/ratingSummaries/never", + ); + expect(response.status).toBe(200); + expect(await response.json()).toMatchObject({ + ratingCount: 0, + averageRating: 0, }); + }); + + it("lists summaries with total size", async () => { + await upsert("cbf2025", "beer-1", "dev-1", 4); + await upsert("cbf2025", "beer-2", "dev-1", 2); + const response = await send("GET", "/v1/festivals/cbf2025/ratingSummaries"); + const data = await response.json(); + expect(data.totalSize).toBe(2); + expect(data.nextPageToken).toBe(""); + expect(data.ratingSummaries.map((s) => s.name)).toEqual([ + "festivals/cbf2025/ratingSummaries/beer-1", + "festivals/cbf2025/ratingSummaries/beer-2", + ]); + }); - const anon = await send("GET", "/v1/ratings/cbf2025/beer-1"); - expect((await anon.json()).yourRating).toBe(null); + it("paginates with opaque tokens", async () => { + await upsert("cbf2025", "beer-1", "dev-1", 4); + await upsert("cbf2025", "beer-2", "dev-1", 4); + await upsert("cbf2025", "beer-3", "dev-1", 4); - const known = await send( + const first = await send( "GET", - "/v1/ratings/cbf2025/beer-1?deviceId=dev-1", + "/v1/festivals/cbf2025/ratingSummaries?page_size=2", ); - expect((await known.json()).yourRating).toBe(4); - }); - - it("returns a festival-wide map of aggregates", async () => { - await post({ - festivalId: "cbf2025", - drinkId: "beer-1", - deviceId: "dev-1", - rating: 4, - }); - await post({ - festivalId: "cbf2025", - drinkId: "beer-2", - deviceId: "dev-1", - rating: 2, - }); - await post({ - festivalId: "cbf2025", - drinkId: "beer-2", - deviceId: "dev-2", - rating: 4, - }); + const firstData = await first.json(); + expect(firstData.ratingSummaries).toHaveLength(2); + expect(firstData.nextPageToken).not.toBe(""); - const response = await send("GET", "/v1/ratings/cbf2025?deviceId=dev-1"); - const data = await response.json(); - expect(data.festivalId).toBe("cbf2025"); - expect(data.aggregates["beer-1"]).toMatchObject({ - count: 1, - average: 4, - yourRating: 4, - }); - expect(data.aggregates["beer-2"]).toMatchObject({ - count: 2, - average: 3, - yourRating: 2, - }); + const second = await send( + "GET", + `/v1/festivals/cbf2025/ratingSummaries?page_size=2&page_token=${firstData.nextPageToken}`, + ); + const secondData = await second.json(); + expect(secondData.ratingSummaries).toHaveLength(1); + expect(secondData.ratingSummaries[0].name).toBe( + "festivals/cbf2025/ratingSummaries/beer-3", + ); + expect(secondData.nextPageToken).toBe(""); }); - it("returns 404 for an over-long ratings path", async () => { - const response = await send("GET", "/v1/ratings/cbf2025/beer-1/extra"); - expect(response.status).toBe(404); + it("rejects a negative page_size", async () => { + const response = await send( + "GET", + "/v1/festivals/cbf2025/ratingSummaries?page_size=-1", + ); + expect(response.status).toBe(400); + expect((await response.json()).error.details[0].reason).toBe( + "INVALID_PAGE_SIZE", + ); }); }); -describe("ratings — DELETE", () => { - it("removes a device's rating and returns the fresh aggregate", async () => { - await post({ - festivalId: "cbf2025", - drinkId: "beer-1", - deviceId: "dev-1", - rating: 4, - }); - await post({ - festivalId: "cbf2025", - drinkId: "beer-1", - deviceId: "dev-2", - rating: 2, - }); - - const response = await del({ - festivalId: "cbf2025", - drinkId: "beer-1", - deviceId: "dev-1", - }); - const data = await response.json(); - expect(data.count).toBe(1); // only dev-2 remains - expect(data.average).toBe(2); - expect(data.yourRating).toBe(null); // dev-1's rating is gone +describe("ratings — routing", () => { + it("returns 405 for an unsupported method on a record", async () => { + const response = await send( + "POST", + ratingPath("cbf2025", "beer-1", "dev-1"), + { + body: { value: 3 }, + }, + ); + expect(response.status).toBe(405); + expect((await response.json()).error.status).toBe("UNIMPLEMENTED"); }); - it("is a no-op when there is nothing to delete", async () => { - const response = await del({ - festivalId: "cbf2025", - drinkId: "beer-1", - deviceId: "ghost", - }); - expect(response.status).toBe(200); - expect((await response.json()).count).toBe(0); + it("returns 404 for an unknown /v1 route", async () => { + const response = await send("GET", "/v1/festivals/cbf2025/bogus/beer-1"); + expect(response.status).toBe(404); + expect((await response.json()).error.details[0].reason).toBe( + "ROUTE_NOT_FOUND", + ); }); }); describe("ratings — bucket isolation", () => { - it("keeps test and prod traffic in separate buckets", async () => { - await post( + it("keeps test and prod traffic separate", async () => { + await upsert("cbf2025", "beer-1", "dev-1", 5, { origin: PROD_ORIGIN }); + await upsert("cbf2025", "beer-1", "dev-1", 1, { origin: TEST_ORIGIN }); + + const prod = await send( + "GET", + "/v1/festivals/cbf2025/ratingSummaries/beer-1", { - festivalId: "cbf2025", - drinkId: "beer-1", - deviceId: "dev-1", - rating: 5, + origin: PROD_ORIGIN, }, - { origin: PROD_ORIGIN }, ); - await post( + const test = await send( + "GET", + "/v1/festivals/cbf2025/ratingSummaries/beer-1", { - festivalId: "cbf2025", - drinkId: "beer-1", - deviceId: "dev-1", - rating: 1, + origin: TEST_ORIGIN, }, - { origin: TEST_ORIGIN }, ); - - const prodView = await send("GET", "/v1/ratings/cbf2025/beer-1", { - origin: PROD_ORIGIN, - }); - const testView = await send("GET", "/v1/ratings/cbf2025/beer-1", { - origin: TEST_ORIGIN, - }); - - expect((await prodView.json()).average).toBe(5); - expect((await testView.json()).average).toBe(1); + expect((await prod.json()).averageRating).toBe(5); + expect((await test.json()).averageRating).toBe(1); }); }); diff --git a/cloudflare-worker/test/recommendations.test.js b/cloudflare-worker/test/recommendations.test.js index 6f6a3be0..21ac0284 100644 --- a/cloudflare-worker/test/recommendations.test.js +++ b/cloudflare-worker/test/recommendations.test.js @@ -5,11 +5,10 @@ import { waitOnExecutionContext, } from "cloudflare:test"; import worker from "../worker.js"; -import { validateRecommendPayload, formatPercent } from "../recommendations.js"; +import { RECOMMENDATIONS_FAMILY } from "../recommendations.js"; const TEST_ORIGIN = "http://localhost:8080"; // non-prod -> 'test' bucket const PROD_ORIGIN = "https://cambeerfestival.app"; // -> 'prod' bucket -const BASE = "/v1/recommendations"; async function send(method, path, { body, origin = TEST_ORIGIN } = {}) { const init = { method, headers: { Origin: origin } }; @@ -24,302 +23,154 @@ async function send(method, path, { body, origin = TEST_ORIGIN } = {}) { return response; } -const post = (body, opts) => send("POST", BASE, { body, ...opts }); -const del = (body, opts) => send("DELETE", BASE, { body, ...opts }); +const recPath = (f, d, device) => + `/v1/festivals/${f}/drinks/${d}/recommendations/${device}`; +const upsert = (f, d, device, wouldRecommend, opts) => + send("PATCH", recPath(f, d, device), { body: { wouldRecommend }, ...opts }); -// This pool version shares D1 storage across tests in a file, so reset the -// table before each test to keep aggregates deterministic. beforeEach(async () => { await env.RATINGS_DB.prepare("DELETE FROM recommendations").run(); }); describe("recommendations — pure helpers", () => { - it("validateRecommendPayload accepts a boolean recommend", () => { - for (const recommend of [true, false]) { - const result = validateRecommendPayload( - { - festivalId: "cbf2025", - drinkId: "beer-1", - deviceId: "dev-1", - recommend, - }, - { requireRecommend: true }, - ); - expect(result.ok).toBe(true); - expect(result.value.recommend).toBe(recommend); - } - }); - - it("validateRecommendPayload rejects non-boolean recommend", () => { - for (const recommend of [1, 0, "yes", null, undefined]) { - const result = validateRecommendPayload( - { festivalId: "f", drinkId: "d", deviceId: "x", recommend }, - { requireRecommend: true }, + it("parseValue requires a boolean", () => { + expect( + RECOMMENDATIONS_FAMILY.parseValue({ wouldRecommend: true }), + ).toMatchObject({ + ok: true, + columnValue: 1, + }); + expect( + RECOMMENDATIONS_FAMILY.parseValue({ wouldRecommend: false }), + ).toMatchObject({ + ok: true, + columnValue: 0, + }); + for (const wouldRecommend of [1, 0, "yes", null, undefined]) { + expect(RECOMMENDATIONS_FAMILY.parseValue({ wouldRecommend }).ok).toBe( + false, ); - expect(result.ok).toBe(false); } }); - it("validateRecommendPayload rejects missing ids", () => { + it("summaryFields computes a 0..1 rate", () => { + expect(RECOMMENDATIONS_FAMILY.summaryFields({})).toEqual({ + responseCount: 0, + recommendCount: 0, + recommendRate: 0, + }); expect( - validateRecommendPayload( - { drinkId: "d", deviceId: "x", recommend: true }, - { requireRecommend: true }, - ).ok, - ).toBe(false); - }); - - it("validateRecommendPayload skips recommend when not required (DELETE)", () => { - const result = validateRecommendPayload( - { festivalId: "f", drinkId: "d", deviceId: "x" }, - { requireRecommend: false }, - ); - expect(result.ok).toBe(true); - }); - - it("formatPercent is a rounded whole number, null when empty", () => { - expect(formatPercent(3, 4)).toBe(75); - expect(formatPercent(1, 3)).toBe(33); - expect(formatPercent(2, 3)).toBe(67); - expect(formatPercent(0, 2)).toBe(0); - expect(formatPercent(0, 0)).toBe(null); + RECOMMENDATIONS_FAMILY.summaryFields({ agg_count: 3, agg_yes: 2 }), + ).toEqual({ responseCount: 3, recommendCount: 2, recommendRate: 0.67 }); }); }); -describe("recommendations — POST upsert", () => { - it("records a recommendation and returns the aggregate", async () => { - const response = await post({ - festivalId: "cbf2025", - drinkId: "beer-1", - deviceId: "dev-1", - recommend: true, - }); +describe("recommendations — upsert (PATCH)", () => { + it("creates an answer and returns the resource", async () => { + const response = await upsert("cbf2025", "beer-1", "dev-1", true); expect(response.status).toBe(200); const data = await response.json(); - expect(data).toMatchObject({ - festivalId: "cbf2025", - drinkId: "beer-1", - count: 1, - recommendCount: 1, - recommendPercent: 100, - youRecommend: true, - }); - }); - - it("changing answer from the same device updates rather than duplicates", async () => { - await post({ - festivalId: "cbf2025", - drinkId: "beer-1", - deviceId: "dev-1", - recommend: true, - }); - const response = await post({ - festivalId: "cbf2025", - drinkId: "beer-1", - deviceId: "dev-1", - recommend: false, - }); - const data = await response.json(); - expect(data.count).toBe(1); - expect(data.recommendCount).toBe(0); - expect(data.recommendPercent).toBe(0); - expect(data.youRecommend).toBe(false); - }); - - it("aggregates yes/no across multiple devices", async () => { - await post({ - festivalId: "cbf2025", - drinkId: "beer-1", - deviceId: "dev-1", - recommend: true, - }); - await post({ - festivalId: "cbf2025", - drinkId: "beer-1", - deviceId: "dev-2", - recommend: true, - }); - const response = await post({ - festivalId: "cbf2025", - drinkId: "beer-1", - deviceId: "dev-3", - recommend: false, - }); - const data = await response.json(); - expect(data.count).toBe(3); - expect(data.recommendCount).toBe(2); - expect(data.recommendPercent).toBe(67); // 2/3 - expect(data.youRecommend).toBe(false); // dev-3 + expect(data.name).toBe( + "festivals/cbf2025/drinks/beer-1/recommendations/dev-1", + ); + expect(data.wouldRecommend).toBe(true); + expect(typeof data.updateTime).toBe("string"); }); -}); -describe("recommendations — validation", () => { - it("rejects a non-boolean recommend with 400", async () => { - const response = await post({ - festivalId: "cbf2025", - drinkId: "beer-1", - deviceId: "dev-1", - recommend: "yes", - }); - expect(response.status).toBe(400); + it("changing the answer updates in place", async () => { + await upsert("cbf2025", "beer-1", "dev-1", true); + const response = await upsert("cbf2025", "beer-1", "dev-1", false); + expect((await response.json()).wouldRecommend).toBe(false); }); - it("rejects missing fields with 400", async () => { - const response = await post({ drinkId: "beer-1", recommend: true }); + it("rejects a non-boolean with a structured error", async () => { + const response = await send( + "PATCH", + recPath("cbf2025", "beer-1", "dev-1"), + { + body: { wouldRecommend: "yes" }, + }, + ); expect(response.status).toBe(400); + expect((await response.json()).error.details[0].reason).toBe( + "RECOMMENDATION_VALUE_INVALID", + ); }); +}); - it("rejects malformed JSON with 400", async () => { - const response = await post("{not json", {}); - expect(response.status).toBe(400); - }); +describe("recommendations — get/delete record", () => { + it("gets and deletes a device's own answer", async () => { + await upsert("cbf2025", "beer-1", "dev-1", true); + const got = await send("GET", recPath("cbf2025", "beer-1", "dev-1")); + expect((await got.json()).wouldRecommend).toBe(true); - it("rejects unsupported methods on the collection with 405", async () => { - const response = await send("PUT", BASE, { - body: { festivalId: "f", drinkId: "d", deviceId: "x", recommend: true }, - }); - expect(response.status).toBe(405); + const del = await send("DELETE", recPath("cbf2025", "beer-1", "dev-1")); + expect(del.status).toBe(200); + const after = await send("GET", recPath("cbf2025", "beer-1", "dev-1")); + expect(after.status).toBe(404); }); }); -describe("recommendations — GET", () => { - it("returns an empty aggregate for a drink with no responses", async () => { - const response = await send("GET", `${BASE}/cbf2025/never-rated`); - expect(response.status).toBe(200); +describe("recommendations — summaries", () => { + it("aggregates yes/no into a rate", async () => { + await upsert("cbf2025", "beer-1", "dev-1", true); + await upsert("cbf2025", "beer-1", "dev-2", true); + await upsert("cbf2025", "beer-1", "dev-3", false); + const response = await send( + "GET", + "/v1/festivals/cbf2025/recommendationSummaries/beer-1", + ); const data = await response.json(); - expect(data).toMatchObject({ - count: 0, - recommendCount: 0, - recommendPercent: null, - youRecommend: null, - }); - }); - - it("includes youRecommend only when deviceId is supplied", async () => { - await post({ - festivalId: "cbf2025", - drinkId: "beer-1", - deviceId: "dev-1", - recommend: true, - }); - - const anon = await send("GET", `${BASE}/cbf2025/beer-1`); - expect((await anon.json()).youRecommend).toBe(null); - - const known = await send("GET", `${BASE}/cbf2025/beer-1?deviceId=dev-1`); - expect((await known.json()).youRecommend).toBe(true); + expect(data.name).toBe("festivals/cbf2025/recommendationSummaries/beer-1"); + expect(data.responseCount).toBe(3); + expect(data.recommendCount).toBe(2); + expect(data.recommendRate).toBe(0.67); }); - it("returns a festival-wide map of aggregates", async () => { - await post({ - festivalId: "cbf2025", - drinkId: "beer-1", - deviceId: "dev-1", - recommend: true, - }); - await post({ - festivalId: "cbf2025", - drinkId: "beer-2", - deviceId: "dev-1", - recommend: false, - }); - await post({ - festivalId: "cbf2025", - drinkId: "beer-2", - deviceId: "dev-2", - recommend: true, - }); - - const response = await send("GET", `${BASE}/cbf2025?deviceId=dev-1`); - const data = await response.json(); - expect(data.festivalId).toBe("cbf2025"); - expect(data.aggregates["beer-1"]).toMatchObject({ - count: 1, - recommendCount: 1, - recommendPercent: 100, - youRecommend: true, - }); - expect(data.aggregates["beer-2"]).toMatchObject({ - count: 2, - recommendCount: 1, - recommendPercent: 50, - youRecommend: false, + it("returns an empty summary for a drink with no answers", async () => { + const response = await send( + "GET", + "/v1/festivals/cbf2025/recommendationSummaries/never", + ); + expect(await response.json()).toMatchObject({ + responseCount: 0, + recommendCount: 0, + recommendRate: 0, }); }); - it("returns 404 for an over-long path", async () => { - const response = await send("GET", `${BASE}/cbf2025/beer-1/extra`); - expect(response.status).toBe(404); - }); -}); - -describe("recommendations — DELETE", () => { - it("removes a device's answer and returns the fresh aggregate", async () => { - await post({ - festivalId: "cbf2025", - drinkId: "beer-1", - deviceId: "dev-1", - recommend: false, - }); - await post({ - festivalId: "cbf2025", - drinkId: "beer-1", - deviceId: "dev-2", - recommend: true, - }); - - const response = await del({ - festivalId: "cbf2025", - drinkId: "beer-1", - deviceId: "dev-1", - }); + it("lists summaries", async () => { + await upsert("cbf2025", "beer-1", "dev-1", true); + await upsert("cbf2025", "beer-2", "dev-1", false); + const response = await send( + "GET", + "/v1/festivals/cbf2025/recommendationSummaries", + ); const data = await response.json(); - expect(data.count).toBe(1); // only dev-2 remains - expect(data.recommendCount).toBe(1); - expect(data.recommendPercent).toBe(100); - expect(data.youRecommend).toBe(null); // dev-1's answer is gone - }); - - it("is a no-op when there is nothing to delete", async () => { - const response = await del({ - festivalId: "cbf2025", - drinkId: "beer-1", - deviceId: "ghost", - }); - expect(response.status).toBe(200); - expect((await response.json()).count).toBe(0); + expect(data.totalSize).toBe(2); + expect(data.recommendationSummaries.map((s) => s.name)).toEqual([ + "festivals/cbf2025/recommendationSummaries/beer-1", + "festivals/cbf2025/recommendationSummaries/beer-2", + ]); }); }); describe("recommendations — bucket isolation", () => { - it("keeps test and prod traffic in separate buckets", async () => { - await post( - { - festivalId: "cbf2025", - drinkId: "beer-1", - deviceId: "dev-1", - recommend: true, - }, + it("keeps test and prod traffic separate", async () => { + await upsert("cbf2025", "beer-1", "dev-1", true, { origin: PROD_ORIGIN }); + await upsert("cbf2025", "beer-1", "dev-1", false, { origin: TEST_ORIGIN }); + + const prod = await send( + "GET", + "/v1/festivals/cbf2025/recommendationSummaries/beer-1", { origin: PROD_ORIGIN }, ); - await post( - { - festivalId: "cbf2025", - drinkId: "beer-1", - deviceId: "dev-1", - recommend: false, - }, + const test = await send( + "GET", + "/v1/festivals/cbf2025/recommendationSummaries/beer-1", { origin: TEST_ORIGIN }, ); - - const prodView = await send("GET", `${BASE}/cbf2025/beer-1`, { - origin: PROD_ORIGIN, - }); - const testView = await send("GET", `${BASE}/cbf2025/beer-1`, { - origin: TEST_ORIGIN, - }); - - expect((await prodView.json()).recommendPercent).toBe(100); - expect((await testView.json()).recommendPercent).toBe(0); + expect((await prod.json()).recommendRate).toBe(1); + expect((await test.json()).recommendRate).toBe(0); }); }); diff --git a/cloudflare-worker/worker.js b/cloudflare-worker/worker.js index 27d8945f..9fa46618 100644 --- a/cloudflare-worker/worker.js +++ b/cloudflare-worker/worker.js @@ -17,6 +17,7 @@ import festivalsData from "./festivals.json"; import { handleRatings } from "./ratings.js"; import { handleRecommendations } from "./recommendations.js"; +import { errorResponse } from "./shared.js"; const UPSTREAM_URL = "https://data.cambridgebeerfestival.com"; @@ -67,8 +68,8 @@ export default { }); } - // Aggregate ratings API (/v1/ratings...). Handled before the proxy - // fall-through so these paths are never forwarded upstream. + // "My festival" API (/v1/...). Handled before the proxy fall-through so + // these paths are never forwarded upstream. const ratingsResponse = await handleRatings( request, url, @@ -89,6 +90,17 @@ export default { return recommendationsResponse; } + // Any other /v1 path is an unknown API route — don't proxy it upstream. + if (url.pathname === "/v1" || url.pathname.startsWith("/v1/")) { + return errorResponse( + 404, + "NOT_FOUND", + "Unknown API route", + "ROUTE_NOT_FOUND", + getCorsHeaders(request), + ); + } + // Handle dynamic available_beverage_types.json endpoint // Pattern: /{festivalId}/available_beverage_types.json const availableTypesMatch = url.pathname.match( @@ -268,7 +280,7 @@ function handleCorsPreflight(request) { status: 204, headers: { ...getCorsHeaders(request), - "Access-Control-Allow-Methods": "GET, POST, DELETE, OPTIONS", + "Access-Control-Allow-Methods": "GET, PATCH, DELETE, OPTIONS", "Access-Control-Allow-Headers": "Content-Type", "Access-Control-Max-Age": maxAge, }, From 2aa74db2cb3c2ffab50069e2470d5a91db84d990 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 13 Jun 2026 09:02:36 +0000 Subject: [PATCH 4/7] chore: update mise.dev.lock with buf platform checksums Populated by mise during toolchain install (buf 1.70.0 via aqua backend). https://claude.ai/code/session_01VVTCRjdHqcTJCVEjEVQV6C --- mise.dev.lock | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/mise.dev.lock b/mise.dev.lock index 3b4ea72a..12d5e0b9 100644 --- a/mise.dev.lock +++ b/mise.dev.lock @@ -1,5 +1,37 @@ # @generated - this file is auto-generated by `mise lock` https://mise.en.dev/dev-tools/mise-lock.html +[[tools.buf]] +version = "1.70.0" +backend = "aqua:bufbuild/buf" + +[tools.buf."platforms.linux-arm64"] +checksum = "sha256:ff3c79a6bb44c41a8053ec0d863234cee82378ea4c29ed5292c852dbf829fec0" +url = "https://github.com/bufbuild/buf/releases/download/v1.70.0/buf-Linux-aarch64.tar.gz" + +[tools.buf."platforms.linux-arm64-musl"] +checksum = "sha256:ff3c79a6bb44c41a8053ec0d863234cee82378ea4c29ed5292c852dbf829fec0" +url = "https://github.com/bufbuild/buf/releases/download/v1.70.0/buf-Linux-aarch64.tar.gz" + +[tools.buf."platforms.linux-x64"] +checksum = "sha256:18498bdadbdce80792ce08c2c59e0c49f157aa3f5d9b0f9c6e3db795be208b9d" +url = "https://github.com/bufbuild/buf/releases/download/v1.70.0/buf-Linux-x86_64.tar.gz" + +[tools.buf."platforms.linux-x64-musl"] +checksum = "sha256:18498bdadbdce80792ce08c2c59e0c49f157aa3f5d9b0f9c6e3db795be208b9d" +url = "https://github.com/bufbuild/buf/releases/download/v1.70.0/buf-Linux-x86_64.tar.gz" + +[tools.buf."platforms.macos-arm64"] +checksum = "sha256:9cec23658df093d50f3318580e17f1e309d45b817927f22e9de30f78f3d4d45b" +url = "https://github.com/bufbuild/buf/releases/download/v1.70.0/buf-Darwin-arm64.tar.gz" + +[tools.buf."platforms.macos-x64"] +checksum = "sha256:ebeed1af15bcb145c35c5e51eadc8fe0bdba9311e08fd6abca8c23294929d20b" +url = "https://github.com/bufbuild/buf/releases/download/v1.70.0/buf-Darwin-x86_64.tar.gz" + +[tools.buf."platforms.windows-x64"] +checksum = "sha256:96422e4313efe2741946f31f57ab4000c440b48d14936992ab98e94c16322f6f" +url = "https://github.com/bufbuild/buf/releases/download/v1.70.0/buf-Windows-x86_64.zip" + [[tools."github:googleapis/api-linter"]] version = "2.3.1" backend = "github:googleapis/api-linter" From 7f4137c0db18de2f8f5cbce1c079cb4715b33672 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 13 Jun 2026 09:08:44 +0000 Subject: [PATCH 5/7] feat(worker): type reviews and shared against generated OpenAPI types MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Convert reviews.js → reviews.ts and shared.js → shared.ts. Response bodies (Review, ReviewSummary, ListReviewsResponse, etc.) are now typed against the generated src/api-types.ts (proto → OpenAPI → openapi-typescript), so a field rename or type change in the proto surfaces as a compile error in the implementation. - types: Review, ReviewSummary, ListReviewsResponse, ListReviewSummariesResponse imported from components["schemas"][...] in the generated api-types.ts - Env interface (RATINGS_DB: D1Database, RATINGS_BUCKET?) centralised in shared.ts - D1 row shapes (ReviewRow, SummaryRow, etc.) typed for all queries - tsc --noEmit passes clean (strict mode, moduleResolution: bundler) - 85 vitest tests still pass - package.json: add typecheck script; tsconfig.json added - mise.toml: test:worker now runs tsc before vitest Regenerate types after proto changes: MISE_ENV=dev ./bin/mise run proto:generate MISE_ENV=dev ./bin/mise run proto:clients:types https://claude.ai/code/session_01VVTCRjdHqcTJCVEjEVQV6C --- cloudflare-worker/package-lock.json | 16 +- cloudflare-worker/package.json | 5 +- cloudflare-worker/{reviews.js => reviews.ts} | 315 ++++++++++--------- cloudflare-worker/{shared.js => shared.ts} | 66 ++-- cloudflare-worker/tsconfig.json | 14 + mise.toml | 6 +- 6 files changed, 239 insertions(+), 183 deletions(-) rename cloudflare-worker/{reviews.js => reviews.ts} (60%) rename cloudflare-worker/{shared.js => shared.ts} (60%) create mode 100644 cloudflare-worker/tsconfig.json diff --git a/cloudflare-worker/package-lock.json b/cloudflare-worker/package-lock.json index bb0998a7..c7e25d3c 100644 --- a/cloudflare-worker/package-lock.json +++ b/cloudflare-worker/package-lock.json @@ -9,7 +9,9 @@ "version": "1.0.0", "devDependencies": { "@cloudflare/vitest-pool-workers": "^0.16.13", + "@cloudflare/workers-types": "^4.20260613.1", "openapi-typescript": "^7.13.0", + "typescript": "^6.0.3", "vitest": "^4.1.8", "wrangler": "^4.88.0" } @@ -169,6 +171,13 @@ "node": ">=16" } }, + "node_modules/@cloudflare/workers-types": { + "version": "4.20260613.1", + "resolved": "https://registry.npmjs.org/@cloudflare/workers-types/-/workers-types-4.20260613.1.tgz", + "integrity": "sha512-1mrgjE6epolwBhroeGAp5ud5H6Vyi6tl1o/NP0T4rXJ8bmEjmhHnbCzAhHTDHV0PIeip43wcuzHKJarvaGTaUA==", + "dev": true, + "license": "MIT OR Apache-2.0" + }, "node_modules/@cspotcode/source-map-support": { "version": "0.8.1", "resolved": "https://registry.npmjs.org/@cspotcode/source-map-support/-/source-map-support-0.8.1.tgz", @@ -2720,12 +2729,11 @@ } }, "node_modules/typescript": { - "version": "5.9.3", - "resolved": "https://registry.npmjs.org/typescript/-/typescript-5.9.3.tgz", - "integrity": "sha512-jl1vZzPDinLr9eUt3J/t7V6FgNEw9QjvBPdysz9KfQDD41fQrC2Y4vKQdiaUpFT4bXlb1RHhLpp8wtm6M5TgSw==", + "version": "6.0.3", + "resolved": "https://registry.npmjs.org/typescript/-/typescript-6.0.3.tgz", + "integrity": "sha512-y2TvuxSZPDyQakkFRPZHKFm+KKVqIisdg9/CZwm9ftvKXLP8NRWj38/ODjNbr43SsoXqNuAisEf1GdCxqWcdBw==", "dev": true, "license": "Apache-2.0", - "peer": true, "bin": { "tsc": "bin/tsc", "tsserver": "bin/tsserver" diff --git a/cloudflare-worker/package.json b/cloudflare-worker/package.json index e936b55f..ba406435 100644 --- a/cloudflare-worker/package.json +++ b/cloudflare-worker/package.json @@ -7,11 +7,14 @@ "deploy": "wrangler deploy", "dev": "wrangler dev", "pretest": "cp ../data/festivals.json ./festivals.json", - "test": "vitest run" + "test": "vitest run", + "typecheck": "tsc --noEmit" }, "devDependencies": { "@cloudflare/vitest-pool-workers": "^0.16.13", + "@cloudflare/workers-types": "^4.20260613.1", "openapi-typescript": "^7.13.0", + "typescript": "^6.0.3", "vitest": "^4.1.8", "wrangler": "^4.88.0" } diff --git a/cloudflare-worker/reviews.js b/cloudflare-worker/reviews.ts similarity index 60% rename from cloudflare-worker/reviews.js rename to cloudflare-worker/reviews.ts index c06a6ce1..e272ae20 100644 --- a/cloudflare-worker/reviews.js +++ b/cloudflare-worker/reviews.ts @@ -9,17 +9,19 @@ * GET /v1alpha/festivals/{f}/reviewSummaries/{d} aggregate for one drink * GET /v1alpha/festivals/{f}/reviewSummaries list aggregates (paginated) * - * The Review is a singleton per (caller, drink). Caller identity comes from - * the X-Device-Id request header in the anonymous phase; it never appears in - * resource names, so the sign-in upgrade is transparent to clients. + * Response shapes are typed against the generated OpenAPI types in + * src/api-types.ts (generated from proto via proto:clients:types). TypeScript + * enforces that every response field matches the proto contract — a field + * rename in the proto surfaces here as a compile error. * - * Both signals (starRating, wouldRecommend) are independently optional: - * a caller can rate without answering the recommendation question, or vice - * versa. Use the updateMask field in the PATCH body to update only one signal - * without clearing the other. + * Caller identity comes from the X-Device-Id request header (anonymous phase). + * It never appears in resource names, so the sign-in upgrade is transparent. */ +import type { components } from "./src/api-types"; import { + type CorsHeaders, + type Env, resolveBucket, rfc3339, jsonResponse, @@ -29,9 +31,67 @@ import { resolvePageSize, } from "./shared.js"; +// Response shapes enforced by the proto contract. +type Review = components["schemas"]["Review"]; +type ReviewSummary = components["schemas"]["ReviewSummary"]; +type ListReviewsResponse = components["schemas"]["ListReviewsResponse"]; +type ListReviewSummariesResponse = + components["schemas"]["ListReviewSummariesResponse"]; + const MAX_ID_LENGTH = 200; -function isValidId(value) { +// D1 row shapes returned by SQL queries. +interface ReviewRow { + star_rating: number | null; + recommend: number | null; + updated_at: number; +} +interface ReviewListRow extends ReviewRow { + drink_id: string; +} +interface SummaryRow { + rating_count: number; + avg_rating: number | null; + response_count: number; + recommend_count: number | null; + drink_id?: string; +} +interface TotalRow { + n: number; +} + +interface ReviewCtx { + db: D1Database; + bucket: string; + festivalId: string; + drinkId: string; + deviceId: string; + corsHeaders: CorsHeaders; +} +interface SummaryCtx { + db: D1Database; + bucket: string; + festivalId: string; + drinkId: string; + corsHeaders: CorsHeaders; +} +interface ListCtx { + db: D1Database; + bucket: string; + festivalId: string; + deviceId: string; + url: URL; + corsHeaders: CorsHeaders; +} +interface ListSummaryCtx { + db: D1Database; + bucket: string; + festivalId: string; + url: URL; + corsHeaders: CorsHeaders; +} + +function isValidId(value: string | null): value is string { return ( typeof value === "string" && value.length > 0 && @@ -39,7 +99,10 @@ function isValidId(value) { ); } -function getDeviceId(request, corsHeaders) { +function getDeviceId( + request: Request, + corsHeaders: CorsHeaders, +): { deviceId: string } | { error: Response } { const deviceId = request.headers.get("X-Device-Id"); if (!isValidId(deviceId)) { return { @@ -55,7 +118,7 @@ function getDeviceId(request, corsHeaders) { return { deviceId }; } -function parseV1alphaPath(pathname) { +function parseV1alphaPath(pathname: string): string[] | null { if (pathname !== "/v1alpha" && !pathname.startsWith("/v1alpha/")) return null; return pathname .slice("/v1alpha/".length) @@ -65,7 +128,12 @@ function parseV1alphaPath(pathname) { } /** Route a request, or return null if the path doesn't match any review route. */ -export async function handleReviews(request, url, env, corsHeaders) { +export async function handleReviews( + request: Request, + url: URL, + env: Env, + corsHeaders: CorsHeaders, +): Promise { const segments = parseV1alphaPath(url.pathname); if (!segments || segments[0] !== "festivals" || segments.length < 3) { return null; @@ -87,7 +155,7 @@ export async function handleReviews(request, url, env, corsHeaders) { if (!isReviewRecord && !isReviewList && !isSummary) return null; - if (!env || !env.RATINGS_DB) { + if (!env?.RATINGS_DB) { return errorResponse( 503, "UNAVAILABLE", @@ -97,7 +165,7 @@ export async function handleReviews(request, url, env, corsHeaders) { ); } - const origin = request.headers.get("Origin") || ""; + const origin = request.headers.get("Origin") ?? ""; const bucket = resolveBucket(origin, env); const db = env.RATINGS_DB; @@ -114,36 +182,15 @@ export async function handleReviews(request, url, env, corsHeaders) { ); } const deviceResult = getDeviceId(request, corsHeaders); - if (deviceResult.error) return deviceResult.error; + if ("error" in deviceResult) return deviceResult.error; switch (request.method) { case "GET": - return getReview({ - db, - bucket, - festivalId, - drinkId, - deviceId: deviceResult.deviceId, - corsHeaders, - }); + return getReview({ db, bucket, festivalId, drinkId, deviceId: deviceResult.deviceId, corsHeaders }); case "PATCH": - return upsertReview(request, { - db, - bucket, - festivalId, - drinkId, - deviceId: deviceResult.deviceId, - corsHeaders, - }); + return upsertReview(request, { db, bucket, festivalId, drinkId, deviceId: deviceResult.deviceId, corsHeaders }); case "DELETE": - return deleteReview({ - db, - bucket, - festivalId, - drinkId, - deviceId: deviceResult.deviceId, - corsHeaders, - }); + return deleteReview({ db, bucket, festivalId, drinkId, deviceId: deviceResult.deviceId, corsHeaders }); default: return methodNotAllowed(corsHeaders); } @@ -164,31 +211,18 @@ export async function handleReviews(request, url, env, corsHeaders) { if (isReviewList) { const deviceResult = getDeviceId(request, corsHeaders); - if (deviceResult.error) return deviceResult.error; - return listReviews({ - db, - bucket, - festivalId, - deviceId: deviceResult.deviceId, - url, - corsHeaders, - }); + if ("error" in deviceResult) return deviceResult.error; + return listReviews({ db, bucket, festivalId, deviceId: deviceResult.deviceId, url, corsHeaders }); } // isSummary if (segments.length === 4) { - return getReviewSummary({ - db, - bucket, - festivalId, - drinkId: segments[3], - corsHeaders, - }); + return getReviewSummary({ db, bucket, festivalId, drinkId: segments[3], corsHeaders }); } return listReviewSummaries({ db, bucket, festivalId, url, corsHeaders }); } -function methodNotAllowed(corsHeaders) { +function methodNotAllowed(corsHeaders: CorsHeaders): Response { return errorResponse( 405, "UNIMPLEMENTED", @@ -198,99 +232,94 @@ function methodNotAllowed(corsHeaders) { ); } -function reviewName(festivalId, drinkId) { +function reviewName(festivalId: string, drinkId: string): string { return `festivals/${festivalId}/drinks/${drinkId}/review`; } -function summaryName(festivalId, drinkId) { +function summaryName(festivalId: string, drinkId: string): string { return `festivals/${festivalId}/reviewSummaries/${drinkId}`; } -function serializeReview(name, row) { - const resource = { name, updateTime: rfc3339(row.updated_at) }; +function serializeReview(name: string, row: ReviewRow): Review { + const resource: Review = { name, updateTime: rfc3339(row.updated_at) }; if (row.star_rating != null) resource.starRating = row.star_rating; if (row.recommend != null) resource.wouldRecommend = Boolean(row.recommend); return resource; } -function round1(value) { +function round1(value: number): number { return Math.round(value * 10) / 10; } -function round2(value) { +function round2(value: number): number { return Math.round(value * 100) / 100; } -function summaryFields(row) { - const ratingCount = row.rating_count || 0; - const responseCount = row.response_count || 0; - const recommendCount = row.recommend_count || 0; +function summaryFields(row: Partial): Omit { + const ratingCount = row.rating_count ?? 0; + const responseCount = row.response_count ?? 0; + const recommendCount = row.recommend_count ?? 0; return { ratingCount, - averageRating: ratingCount ? round1(row.avg_rating) : 0, + averageRating: ratingCount && row.avg_rating != null ? round1(row.avg_rating) : 0, responseCount, recommendCount, recommendRate: responseCount ? round2(recommendCount / responseCount) : 0, }; } -async function readRow(db, bucket, festivalId, drinkId, deviceId) { +async function readRow( + db: D1Database, + bucket: string, + festivalId: string, + drinkId: string, + deviceId: string, +): Promise { return db .prepare( "SELECT star_rating, recommend, updated_at FROM reviews " + "WHERE bucket = ? AND festival_id = ? AND drink_id = ? AND device_id = ?", ) .bind(bucket, festivalId, drinkId, deviceId) - .first(); + .first(); } -async function getReview(ctx) { +async function getReview(ctx: ReviewCtx): Promise { const { db, bucket, festivalId, drinkId, deviceId, corsHeaders } = ctx; const row = await readRow(db, bucket, festivalId, drinkId, deviceId); if (!row) { return errorResponse(404, "NOT_FOUND", "No review found", "NOT_FOUND", corsHeaders); } - return jsonResponse( + return jsonResponse( serializeReview(reviewName(festivalId, drinkId), row), 200, corsHeaders, ); } -async function upsertReview(request, ctx) { +async function upsertReview(request: Request, ctx: ReviewCtx): Promise { const { db, bucket, festivalId, drinkId, deviceId, corsHeaders } = ctx; - let body; + let body: unknown; try { body = await request.json(); } catch { - return errorResponse( - 400, - "INVALID_ARGUMENT", - "Invalid JSON body", - "INVALID_BODY", - corsHeaders, - ); + return errorResponse(400, "INVALID_ARGUMENT", "Invalid JSON body", "INVALID_BODY", corsHeaders); } if (body === null || typeof body !== "object") { - return errorResponse( - 400, - "INVALID_ARGUMENT", - "Body must be a JSON object", - "INVALID_BODY", - corsHeaders, - ); + return errorResponse(400, "INVALID_ARGUMENT", "Body must be a JSON object", "INVALID_BODY", corsHeaders); } // Parse updateMask: comma-separated field names. Absent/empty = all provided fields. - const maskRaw = body.updateMask; - const mask = + const patch = body as Record; + const maskRaw = patch.updateMask; + const mask: Set | null = typeof maskRaw === "string" && maskRaw.length > 0 ? new Set(maskRaw.split(",").map((s) => s.trim())) : null; - const updateStar = mask === null ? "starRating" in body : mask.has("starRating"); - const updateRec = mask === null ? "wouldRecommend" in body : mask.has("wouldRecommend"); + const updateStar = mask === null ? "starRating" in patch : mask.has("starRating"); + const updateRec = mask === null ? "wouldRecommend" in patch : mask.has("wouldRecommend"); if (!updateStar && !updateRec) { return errorResponse( @@ -302,10 +331,10 @@ async function upsertReview(request, ctx) { ); } - let starRating; + let starRating: number | undefined; if (updateStar) { - const v = body.starRating; - if (!Number.isInteger(v) || v < 1 || v > 5) { + const v = patch.starRating; + if (!Number.isInteger(v) || (v as number) < 1 || (v as number) > 5) { return errorResponse( 400, "INVALID_ARGUMENT", @@ -314,12 +343,12 @@ async function upsertReview(request, ctx) { corsHeaders, ); } - starRating = v; + starRating = v as number; } - let recommend; + let recommend: number | undefined; if (updateRec) { - const v = body.wouldRecommend; + const v = patch.wouldRecommend; if (typeof v !== "boolean") { return errorResponse( 400, @@ -345,10 +374,7 @@ async function upsertReview(request, ctx) { updateStar ? starRating : existing.star_rating, updateRec ? recommend : existing.recommend, now, - bucket, - festivalId, - drinkId, - deviceId, + bucket, festivalId, drinkId, deviceId, ) .run(); } else { @@ -358,26 +384,23 @@ async function upsertReview(request, ctx) { "VALUES (?, ?, ?, ?, ?, ?, ?)", ) .bind( - bucket, - festivalId, - drinkId, - deviceId, - updateStar ? starRating : null, - updateRec ? recommend : null, + bucket, festivalId, drinkId, deviceId, + updateStar ? (starRating ?? null) : null, + updateRec ? (recommend ?? null) : null, now, ) .run(); } const row = await readRow(db, bucket, festivalId, drinkId, deviceId); - return jsonResponse( - serializeReview(reviewName(festivalId, drinkId), row), + return jsonResponse( + serializeReview(reviewName(festivalId, drinkId), row!), 200, corsHeaders, ); } -async function deleteReview(ctx) { +async function deleteReview(ctx: ReviewCtx): Promise { const { db, bucket, festivalId, drinkId, deviceId, corsHeaders } = ctx; const result = await db .prepare( @@ -387,41 +410,29 @@ async function deleteReview(ctx) { .bind(bucket, festivalId, drinkId, deviceId) .run(); - const changes = result.meta ? result.meta.changes : 0; + const changes = result.meta?.changes ?? 0; if (!changes) { return errorResponse(404, "NOT_FOUND", "No review found", "NOT_FOUND", corsHeaders); } return jsonResponse({}, 200, corsHeaders); } -async function listReviews(ctx) { +async function listReviews(ctx: ListCtx): Promise { const { db, bucket, festivalId, deviceId, url, corsHeaders } = ctx; const sizeResult = resolvePageSize(url.searchParams.get("page_size")); - if (sizeResult.error) { - return errorResponse( - 400, - "INVALID_ARGUMENT", - "page_size must be >= 0", - "INVALID_PAGE_SIZE", - corsHeaders, - ); + if ("error" in sizeResult) { + return errorResponse(400, "INVALID_ARGUMENT", "page_size must be >= 0", "INVALID_PAGE_SIZE", corsHeaders); } const pageSize = sizeResult.value; const cursor = decodePageToken(url.searchParams.get("page_token")); if (cursor === undefined) { - return errorResponse( - 400, - "INVALID_ARGUMENT", - "Invalid page_token", - "INVALID_PAGE_TOKEN", - corsHeaders, - ); + return errorResponse(400, "INVALID_ARGUMENT", "Invalid page_token", "INVALID_PAGE_TOKEN", corsHeaders); } const where = ["bucket = ?", "festival_id = ?", "device_id = ?"]; - const binds = [bucket, festivalId, deviceId]; + const binds: unknown[] = [bucket, festivalId, deviceId]; if (cursor !== null) { where.push("drink_id > ?"); binds.push(cursor); @@ -433,10 +444,10 @@ async function listReviews(ctx) { `WHERE ${where.join(" AND ")} ORDER BY drink_id LIMIT ?`, ) .bind(...binds, pageSize + 1) - .all(); + .all(); const page = results.slice(0, pageSize); - const reviews = page.map((row) => + const reviews: Review[] = page.map((row) => serializeReview(reviewName(festivalId, row.drink_id), row), ); @@ -445,10 +456,14 @@ async function listReviews(ctx) { nextPageToken = encodePageToken(page[page.length - 1].drink_id); } - return jsonResponse({ reviews, nextPageToken }, 200, corsHeaders); + return jsonResponse( + { reviews, nextPageToken }, + 200, + corsHeaders, + ); } -async function getReviewSummary(ctx) { +async function getReviewSummary(ctx: SummaryCtx): Promise { const { db, bucket, festivalId, drinkId, corsHeaders } = ctx; const row = await db .prepare( @@ -460,43 +475,31 @@ async function getReviewSummary(ctx) { "FROM reviews WHERE bucket = ? AND festival_id = ? AND drink_id = ?", ) .bind(bucket, festivalId, drinkId) - .first(); + .first(); - return jsonResponse( - { name: summaryName(festivalId, drinkId), ...summaryFields(row || {}) }, + return jsonResponse( + { name: summaryName(festivalId, drinkId), ...summaryFields(row ?? {}) }, 200, corsHeaders, ); } -async function listReviewSummaries(ctx) { +async function listReviewSummaries(ctx: ListSummaryCtx): Promise { const { db, bucket, festivalId, url, corsHeaders } = ctx; const sizeResult = resolvePageSize(url.searchParams.get("page_size")); - if (sizeResult.error) { - return errorResponse( - 400, - "INVALID_ARGUMENT", - "page_size must be >= 0", - "INVALID_PAGE_SIZE", - corsHeaders, - ); + if ("error" in sizeResult) { + return errorResponse(400, "INVALID_ARGUMENT", "page_size must be >= 0", "INVALID_PAGE_SIZE", corsHeaders); } const pageSize = sizeResult.value; const cursor = decodePageToken(url.searchParams.get("page_token")); if (cursor === undefined) { - return errorResponse( - 400, - "INVALID_ARGUMENT", - "Invalid page_token", - "INVALID_PAGE_TOKEN", - corsHeaders, - ); + return errorResponse(400, "INVALID_ARGUMENT", "Invalid page_token", "INVALID_PAGE_TOKEN", corsHeaders); } const where = ["bucket = ?", "festival_id = ?"]; - const binds = [bucket, festivalId]; + const binds: unknown[] = [bucket, festivalId]; if (cursor !== null) { where.push("drink_id > ?"); binds.push(cursor); @@ -513,10 +516,10 @@ async function listReviewSummaries(ctx) { `WHERE ${where.join(" AND ")} GROUP BY drink_id ORDER BY drink_id LIMIT ?`, ) .bind(...binds, pageSize + 1) - .all(); + .all(); const page = results.slice(0, pageSize); - const reviewSummaries = page.map((row) => ({ + const reviewSummaries: ReviewSummary[] = page.map((row) => ({ name: summaryName(festivalId, row.drink_id), ...summaryFields(row), })); @@ -531,13 +534,13 @@ async function listReviewSummaries(ctx) { "SELECT COUNT(DISTINCT drink_id) AS n FROM reviews WHERE bucket = ? AND festival_id = ?", ) .bind(bucket, festivalId) - .first(); + .first(); - return jsonResponse( + return jsonResponse( { reviewSummaries, nextPageToken, - totalSize: totalRow ? totalRow.n : 0, + totalSize: totalRow?.n ?? 0, }, 200, corsHeaders, diff --git a/cloudflare-worker/shared.js b/cloudflare-worker/shared.ts similarity index 60% rename from cloudflare-worker/shared.js rename to cloudflare-worker/shared.ts index 0cff5b45..beadf452 100644 --- a/cloudflare-worker/shared.js +++ b/cloudflare-worker/shared.ts @@ -9,24 +9,35 @@ const DEFAULT_PAGE_SIZE = 100; const MAX_PAGE_SIZE = 1000; const ERROR_DOMAIN = "cambeerfestival.app"; -export function isProductionOrigin(origin) { +export type CorsHeaders = Record; + +export interface Env { + RATINGS_DB: D1Database; + RATINGS_BUCKET?: string; +} + +export function isProductionOrigin(origin: string): boolean { return origin === "https://cambeerfestival.app"; } -export function resolveBucket(origin, env) { +export function resolveBucket(origin: string, env: Partial): string { if (env && typeof env.RATINGS_BUCKET === "string" && env.RATINGS_BUCKET) { return env.RATINGS_BUCKET; } return isProductionOrigin(origin) ? "prod" : "test"; } -export function rfc3339(epochMillis) { +export function rfc3339(epochMillis: number): string { return new Date(epochMillis).toISOString(); } // --- Responses (AIP-193) --------------------------------------------------- -export function jsonResponse(body, status, corsHeaders) { +export function jsonResponse( + body: T, + status: number, + corsHeaders: CorsHeaders, +): Response { return new Response(JSON.stringify(body), { status, headers: { @@ -36,32 +47,47 @@ export function jsonResponse(body, status, corsHeaders) { }); } +interface ErrorInfo { + "@type": string; + reason: string; + domain: string; + metadata?: Record; +} + +interface ErrorBody { + error: { + code: number; + message: string; + status: string; + details: ErrorInfo[]; + }; +} + /** Structured error body per AIP-193 (google.rpc.Status + ErrorInfo). */ export function errorResponse( - httpCode, - status, - message, - reason, - corsHeaders, - metadata, -) { - const errorInfo = { + httpCode: number, + status: string, + message: string, + reason: string, + corsHeaders: CorsHeaders, + metadata?: Record, +): Response { + const errorInfo: ErrorInfo = { "@type": "type.googleapis.com/google.rpc.ErrorInfo", reason, domain: ERROR_DOMAIN, }; if (metadata) errorInfo.metadata = metadata; - return jsonResponse( - { error: { code: httpCode, message, status, details: [errorInfo] } }, - httpCode, - corsHeaders, - ); + const body: ErrorBody = { + error: { code: httpCode, message, status, details: [errorInfo] }, + }; + return jsonResponse(body, httpCode, corsHeaders); } // --- Pagination (AIP-158) -------------------------------------------------- /** Encode a keyset cursor (last drink id) as an opaque URL-safe token. */ -export function encodePageToken(drinkId) { +export function encodePageToken(drinkId: string): string { return btoa(unescape(encodeURIComponent(drinkId))) .replace(/\+/g, "-") .replace(/\//g, "_") @@ -69,7 +95,7 @@ export function encodePageToken(drinkId) { } /** Decode a page token back to its cursor, or null if absent. */ -export function decodePageToken(token) { +export function decodePageToken(token: string | null): string | null | undefined { if (!token) return null; try { const b64 = token.replace(/-/g, "+").replace(/_/g, "/"); @@ -80,7 +106,7 @@ export function decodePageToken(token) { } /** Resolve an effective page size, or { error } for a bad value. */ -export function resolvePageSize(raw) { +export function resolvePageSize(raw: string | null): { value: number } | { error: true } { if (raw == null || raw === "") return { value: DEFAULT_PAGE_SIZE }; const n = Number(raw); if (!Number.isInteger(n) || n < 0) return { error: true }; diff --git a/cloudflare-worker/tsconfig.json b/cloudflare-worker/tsconfig.json new file mode 100644 index 00000000..e70b0877 --- /dev/null +++ b/cloudflare-worker/tsconfig.json @@ -0,0 +1,14 @@ +{ + "compilerOptions": { + "target": "ES2022", + "module": "ESNext", + "moduleResolution": "bundler", + "lib": ["ES2022"], + "types": ["@cloudflare/workers-types"], + "strict": true, + "noEmit": true, + "skipLibCheck": true + }, + "include": ["**/*.ts"], + "exclude": ["node_modules", ".wrangler"] +} diff --git a/mise.toml b/mise.toml index 02d3fb04..0d0783d0 100644 --- a/mise.toml +++ b/mise.toml @@ -82,14 +82,16 @@ sources = ['data/festivals.json', 'docs/code/api/festival-registry-schema.json'] run = 'npm ci && node ../scripts/validate-festivals.js' [tasks."test:worker"] -description = "Run Cloudflare Worker tests (Vitest + workerd)" +description = "Run Cloudflare Worker type check and tests (tsc + Vitest + workerd)" dir = "cloudflare-worker" sources = [ 'cloudflare-worker/package.json', 'cloudflare-worker/package-lock.json', + 'cloudflare-worker/tsconfig.json', 'cloudflare-worker/worker.js', + 'cloudflare-worker/**/*.ts', 'cloudflare-worker/test/**/*.js', 'cloudflare-worker/vitest.config.js', 'data/festivals.json', ] -run = 'npm ci && npm test' +run = 'npm ci && npm run typecheck && npm test' From 0f8a0e8670cd116416450809fa9ce522f98e3bd3 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 13 Jun 2026 09:12:02 +0000 Subject: [PATCH 6/7] ci(proto): add buf lint and breaking change check MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add a proto job to CI that runs buf lint and buf breaking on every PR that touches proto/. Breaking change detection uses FILE stability level (configured in proto/buf.yaml), appropriate for v1alpha APIs — catches source-breaking changes to generated code while allowing additive changes. buf breaking only runs on pull_request events (bufbuild/buf-action skips it on push to main where the PR is already merged). Lint runs on both. Switch breaking.use from FILE to WIRE_JSON_COMPATIBLE in proto/buf.yaml when the API graduates from v1alpha to v1 stable. https://claude.ai/code/session_01VVTCRjdHqcTJCVEjEVQV6C --- .github/workflows/ci.yml | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index ddc14b1c..3003de19 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -24,6 +24,7 @@ jobs: outputs: app: ${{ steps.filter.outputs.app }} functions: ${{ steps.filter.outputs.functions }} + proto: ${{ steps.filter.outputs.proto }} steps: - name: Checkout uses: actions/checkout@v6 @@ -46,6 +47,8 @@ jobs: - 'mise.toml' functions: - 'functions/**' + proto: + - 'proto/**' # Check formatting before expensive jobs fmt: @@ -96,6 +99,30 @@ jobs: mapfile -t SH_FILES < <(find . -name '*.sh' -not -path './.git/*' -not -path './.mise/*' -not -path '*/node_modules/*' | sort) [[ ${#SH_FILES[@]} -gt 0 ]] && shfmt -d -i 0 -ci "${SH_FILES[@]}" + # Lint proto files and check for breaking changes against main. + # buf breaking runs on PRs only (bufbuild/buf-action skips it on push to main + # where the change is already merged). FILE stability level (configured in + # proto/buf.yaml) is appropriate for v1alpha; switch to WIRE_JSON_COMPATIBLE + # when the API graduates to v1. + proto: + needs: changes + runs-on: ubuntu-latest + if: | + github.event_name == 'workflow_dispatch' || + needs.changes.outputs.proto == 'true' + steps: + - name: Checkout + uses: actions/checkout@v6 + with: + fetch-depth: 0 # buf breaking needs full history to compare against main + + - uses: bufbuild/buf-action@v1 + with: + input: proto + push: false + pr_comment: false + breaking_against: "https://github.com/${{ github.repository }}.git#branch=main,subdir=proto" + # Analyze code in parallel with tests so builds can start sooner analyze: needs: changes From bb9e68ff3d9aeb1da6e9c5824958dc0037669d2e Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 13 Jun 2026 09:36:45 +0000 Subject: [PATCH 7/7] fix(worker): three correctness fixes found in review - Validate drinkId before getReviewSummary to prevent an injection path through the resource-name segments (INVALID_RESOURCE_NAME 400) - Reject unknown updateMask fields rather than silently ignoring them (UNKNOWN_FIELD_MASK 400), matching AIP-134 contract guarantees - Eliminate post-write readRow() in upsert: compute finalStarRating / finalRecommend before writing and build the response from those values, removing one DB round trip and closing a TOCTOU race where a concurrent DELETE between write and re-read caused a non-null assertion crash Test: adds UNKNOWN_FIELD_MASK case; all 86 tests pass https://claude.ai/code/session_01VVTCRjdHqcTJCVEjEVQV6C --- cloudflare-worker/reviews.ts | 59 +++++++++++++++++--------- cloudflare-worker/test/reviews.test.js | 11 +++++ 2 files changed, 51 insertions(+), 19 deletions(-) diff --git a/cloudflare-worker/reviews.ts b/cloudflare-worker/reviews.ts index e272ae20..bdaf8bfa 100644 --- a/cloudflare-worker/reviews.ts +++ b/cloudflare-worker/reviews.ts @@ -217,7 +217,17 @@ export async function handleReviews( // isSummary if (segments.length === 4) { - return getReviewSummary({ db, bucket, festivalId, drinkId: segments[3], corsHeaders }); + const drinkId = segments[3]; + if (!isValidId(drinkId)) { + return errorResponse( + 400, + "INVALID_ARGUMENT", + "Invalid resource name", + "INVALID_RESOURCE_NAME", + corsHeaders, + ); + } + return getReviewSummary({ db, bucket, festivalId, drinkId, corsHeaders }); } return listReviewSummaries({ db, bucket, festivalId, url, corsHeaders }); } @@ -311,12 +321,24 @@ async function upsertReview(request: Request, ctx: ReviewCtx): Promise } // Parse updateMask: comma-separated field names. Absent/empty = all provided fields. + const KNOWN_FIELDS = new Set(["starRating", "wouldRecommend"]); const patch = body as Record; const maskRaw = patch.updateMask; - const mask: Set | null = - typeof maskRaw === "string" && maskRaw.length > 0 - ? new Set(maskRaw.split(",").map((s) => s.trim())) - : null; + let mask: Set | null = null; + if (typeof maskRaw === "string" && maskRaw.length > 0) { + const fields = maskRaw.split(",").map((s) => s.trim()); + const unknown = fields.filter((f) => !KNOWN_FIELDS.has(f)); + if (unknown.length > 0) { + return errorResponse( + 400, + "INVALID_ARGUMENT", + `Unknown updateMask field(s): ${unknown.join(", ")}`, + "UNKNOWN_FIELD_MASK", + corsHeaders, + ); + } + mask = new Set(fields); + } const updateStar = mask === null ? "starRating" in patch : mask.has("starRating"); const updateRec = mask === null ? "wouldRecommend" in patch : mask.has("wouldRecommend"); @@ -364,18 +386,19 @@ async function upsertReview(request: Request, ctx: ReviewCtx): Promise const existing = await readRow(db, bucket, festivalId, drinkId, deviceId); const now = Date.now(); + // Compute the final column values upfront so we can build the response + // without a second DB read — avoids a round trip and the race where a + // concurrent DELETE between write and re-read would make row! throw. + const finalStarRating = updateStar ? (starRating ?? null) : (existing?.star_rating ?? null); + const finalRecommend = updateRec ? (recommend ?? null) : (existing?.recommend ?? null); + if (existing) { await db .prepare( "UPDATE reviews SET star_rating = ?, recommend = ?, updated_at = ? " + "WHERE bucket = ? AND festival_id = ? AND drink_id = ? AND device_id = ?", ) - .bind( - updateStar ? starRating : existing.star_rating, - updateRec ? recommend : existing.recommend, - now, - bucket, festivalId, drinkId, deviceId, - ) + .bind(finalStarRating, finalRecommend, now, bucket, festivalId, drinkId, deviceId) .run(); } else { await db @@ -383,18 +406,16 @@ async function upsertReview(request: Request, ctx: ReviewCtx): Promise "INSERT INTO reviews (bucket, festival_id, drink_id, device_id, star_rating, recommend, updated_at) " + "VALUES (?, ?, ?, ?, ?, ?, ?)", ) - .bind( - bucket, festivalId, drinkId, deviceId, - updateStar ? (starRating ?? null) : null, - updateRec ? (recommend ?? null) : null, - now, - ) + .bind(bucket, festivalId, drinkId, deviceId, finalStarRating, finalRecommend, now) .run(); } - const row = await readRow(db, bucket, festivalId, drinkId, deviceId); return jsonResponse( - serializeReview(reviewName(festivalId, drinkId), row!), + serializeReview(reviewName(festivalId, drinkId), { + star_rating: finalStarRating, + recommend: finalRecommend, + updated_at: now, + }), 200, corsHeaders, ); diff --git a/cloudflare-worker/test/reviews.test.js b/cloudflare-worker/test/reviews.test.js index 8345c171..9be7cbdb 100644 --- a/cloudflare-worker/test/reviews.test.js +++ b/cloudflare-worker/test/reviews.test.js @@ -134,6 +134,17 @@ describe("reviews — PATCH (upsert)", () => { expect(data.averageRating).toBe(5); }); + it("rejects an unknown updateMask field with a structured error", async () => { + const response = await patch("cbf2025", "beer-1", { + starRating: 3, + updateMask: "starRating,bogusField", + }); + expect(response.status).toBe(400); + const { error } = await response.json(); + expect(error.status).toBe("INVALID_ARGUMENT"); + expect(error.details[0].reason).toBe("UNKNOWN_FIELD_MASK"); + }); + it("rejects an out-of-range starRating with a structured error", async () => { const response = await patch("cbf2025", "beer-1", { starRating: 9 }); expect(response.status).toBe(400);