feat(worker): add /v1alpha Review API on D1 - #426
Merged
Conversation
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.
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.
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.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR extends the Cloudflare proxy worker with a new resource-oriented /v1 API backed by D1 to collect per-device drink ratings and “would recommend” answers and return aggregated summaries, as a first step toward an online “my festival”.
Changes:
- Adds D1 schema + worker routing for
ratings/recommendationsrecord endpoints and their aggregate summary/list endpoints. - Introduces a shared “resource family” engine (
shared.js) used by thinratings.jsandrecommendations.jshandlers. - Adds vitest + Miniflare/D1 migration setup and integration tests covering upsert/read/delete, summaries, pagination, CORS, and bucket isolation.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| cloudflare-worker/wrangler.toml | Adds D1 binding/config for ratings storage (placeholder database_id) and migrations dir. |
| cloudflare-worker/worker.js | Routes /v1 requests to ratings/recommendations handlers; returns 404 for unknown /v1 routes; expands CORS methods. |
| cloudflare-worker/vitest.config.js | Loads D1 migrations at config time and exposes them to tests via bindings + setup file. |
| cloudflare-worker/shared.js | Implements shared routing/validation, structured errors, pagination, and D1 queries for resource families. |
| cloudflare-worker/ratings.js | Defines ratings family (1–5 integer validation) and serialization/summary logic. |
| cloudflare-worker/recommendations.js | Defines recommendations family (boolean validation) and serialization/summary logic. |
| cloudflare-worker/migrations/0001_create_ratings_table.sql | Creates ratings table with bucket scoping and aggregate index. |
| cloudflare-worker/migrations/0002_create_recommendations_table.sql | Creates recommendations table with bucket scoping and aggregate index. |
| cloudflare-worker/README.md | Documents the new /v1 API and D1 provisioning steps. |
| cloudflare-worker/test/apply-migrations.js | Applies migrations to simulated D1 before tests run. |
| cloudflare-worker/test/ratings.test.js | Integration + helper tests for ratings endpoints, pagination, and bucket isolation. |
| cloudflare-worker/test/recommendations.test.js | Integration + helper tests for recommendations endpoints and bucket isolation. |
| cloudflare-worker/test/cors.test.js | Updates preflight assertions for expanded allowed methods. |
Comment on lines
+135
to
+138
| const segments = parseV1Path(url.pathname); | ||
| if (!segments || segments[0] !== "festivals" || segments.length < 3) { | ||
| return null; | ||
| } |
Comment on lines
+26
to
+31
| export function resolveBucket(origin, env) { | ||
| if (env && typeof env.RATINGS_BUCKET === "string" && env.RATINGS_BUCKET) { | ||
| return env.RATINGS_BUCKET; | ||
| } | ||
| return isProductionOrigin(origin) ? "prod" : "test"; | ||
| } |
Comment on lines
+252
to
+266
| 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, | ||
| ); | ||
| } | ||
| const name = writeResourceName(family, festivalId, drinkId, deviceId); | ||
| return jsonResponse(family.serializeResource(name, row), 200, corsHeaders); | ||
| } |
Comment on lines
+313
to
+336
| 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); | ||
| } |
Comment on lines
+150
to
+151
| 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'`. |
Comment on lines
+3
to
+5
| // 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); |
Rebases the ratings/recommendations worker from PR #426 onto current main and updates the implementation to conform to the v1alpha proto contract merged in PR #425. Changes from the original design: - URL prefix: /v1/ → /v1alpha/ - Separate `ratings/{device}` + `recommendations/{device}` collections replaced by a single `Review` singleton at `drinks/{d}/review` - Device ID moves from the URL to the `X-Device-Id` request header; it no longer appears in resource names (auth-upgrade transparent) - Separate `ratingSummaries` + `recommendationSummaries` merged into `reviewSummaries` (combined ratingCount + responseCount/recommendRate) - `starRating` + `wouldRecommend` signals independently nullable; `updateMask` in the PATCH body allows updating one without clearing the other - DB schema: two tables → single `reviews` table with nullable columns New routes: GET/PATCH/DELETE /v1alpha/festivals/{f}/drinks/{d}/review GET /v1alpha/festivals/{f}/reviews GET /v1alpha/festivals/{f}/reviewSummaries[/{d}] Implementation: - reviews.js replaces ratings.js + recommendations.js - shared.js retains utility functions (bucket, errors, pagination) - Single migration: 0001_create_reviews_table.sql - 85 vitest tests pass (pure helpers, upsert, get/delete, list, summaries, bucket isolation, missing header, routing) - CORS: allow X-Device-Id header alongside Content-Type https://claude.ai/code/session_01VVTCRjdHqcTJCVEjEVQV6C
Populated by mise during toolchain install (buf 1.70.0 via aqua backend). https://claude.ai/code/session_01VVTCRjdHqcTJCVEjEVQV6C
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
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
richardthe3rd
changed the title
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
- 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
TypeScript 6 breaks npm ci: openapi-typescript@7.13.0 requires peer typescript@"^5.x". Downgrade to ^5.9.3 to restore compatibility. Also apply prettier formatting to reviews.ts, shared.ts, and reviews.test.js which CI's fmt check was rejecting. https://claude.ai/code/session_01VVTCRjdHqcTJCVEjEVQV6C
richardthe3rd
force-pushed
the
feat/worker-ratings-api
branch
from
June 13, 2026 09:56
8c04fd0 to
1f3e4bd
Compare
Contributor
🚀 Cloudflare Pages PreviewYour preview deployment is ready! Preview URL: https://feat-worker-ratings-api.staging-cambeerfestival.pages.dev This preview will be automatically updated when you push new commits to this PR. |
Merged
richardthe3rd
pushed a commit
that referenced
this pull request
Jun 13, 2026
Adds a Redoc-rendered API docs page at /api-docs/ in the Cloudflare Pages deployment. The OpenAPI spec is generated from proto at build time (not committed), consistent with the proto-in-CI pattern from #426. - web/api-docs/index.html: Redoc page loading openapi.yaml from jsDelivr CDN - web/api-docs/.gitignore: openapi.yaml is generated, not committed - web/_headers: /api-docs/* CSP override allowing cdn.jsdelivr.net for Redoc - ci.yml build-web: buf generate + copy openapi.yaml before flutter build - mise.dev.toml proto:generate: also copies to web/api-docs/ for local dev https://claude.ai/code/session_015uTnGiC56cEELZMH2cQQU4
richardthe3rd
added a commit
that referenced
this pull request
Jun 13, 2026
) * docs(api): publish MyFestival OpenAPI spec via Redoc at /api-docs/ Adds a Redoc-rendered API docs page at /api-docs/ in the Cloudflare Pages deployment. The OpenAPI spec is generated from proto at build time (not committed), consistent with the proto-in-CI pattern from #426. - web/api-docs/index.html: Redoc page loading openapi.yaml from jsDelivr CDN - web/api-docs/.gitignore: openapi.yaml is generated, not committed - web/_headers: /api-docs/* CSP override allowing cdn.jsdelivr.net for Redoc - ci.yml build-web: buf generate + copy openapi.yaml before flutter build - mise.dev.toml proto:generate: also copies to web/api-docs/ for local dev https://claude.ai/code/session_015uTnGiC56cEELZMH2cQQU4 * docs: add API docs link to README https://claude.ai/code/session_015uTnGiC56cEELZMH2cQQU4 * fix(api-docs): exempt /api-docs/ from Flutter SPA catch-all rewrite Cloudflare Pages does not resolve implicit directory indexes before evaluating _redirects rewrite rules, so /api-docs/ was being caught by /* /index.html 200 before the Redoc page could be served. Adding explicit /api-docs and /api-docs/ rules before the catch-all routes those two paths to /api-docs/index.html directly. https://claude.ai/code/session_015uTnGiC56cEELZMH2cQQU4 * fix(api-docs): serve Redoc from self rather than CDN to avoid CSP issues Downloading redoc.standalone.js at build time (CI) and local dev (proto:generate) so it is served from 'self', which the existing Flutter CSP already allows. Removes the /api-docs/* CSP override that was added to permit cdn.jsdelivr.net. https://claude.ai/code/session_015uTnGiC56cEELZMH2cQQU4 --------- Co-authored-by: Claude <noreply@anthropic.com>
Merged
This was referenced Aug 10, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
First step towards an online "my festival". Clients submit a star rating and/or a "would recommend" answer and get back the shared aggregate. Conforms to the v1alpha proto contract merged in #425. Split out from #423.
Changes
Storage
reviewstable — one row per (bucket, festival, drink, device) with upsert semantics so re-reviewing never inflates countsstar_ratingandrecommendcolumns are independently nullable: a caller can rate without answering the recommendation question, or vice versa0001_create_reviews_table.sqlbucket(testorprod, derived from request origin);RATINGS_BUCKETworker var can pin itAPI (resource-oriented, conforms to v1alpha proto contract in
proto/)The Review is a singleton per (caller, drink). Caller identity comes from the
X-Device-Idrequest header — the device ID never appears in resource names, so the sign-in upgrade (phase 3) is transparent to clients.PATCH/v1alpha/festivals/{f}/drinks/{d}/reviewstarRatingand/orwouldRecommend)GET/v1alpha/festivals/{f}/drinks/{d}/reviewDELETE/v1alpha/festivals/{f}/drinks/{d}/reviewGET/v1alpha/festivals/{f}/reviewsGET/v1alpha/festivals/{f}/reviewSummaries/{d}GET/v1alpha/festivals/{f}/reviewSummariesPATCHbody:{ starRating?: 1–5, wouldRecommend?: bool, updateMask?: "starRating,wouldRecommend" }. UseupdateMaskto update one signal without clearing the other. Structuredgoogle.rpc.Statuserrors (AIP-193). CORS extended toGET/PATCH/DELETEwithX-Device-Idallowed header. Unknown/v1alpharoutes 404 instead of proxying upstream.Implementation
reviews.ts+shared.ts— TypeScript, response bodies typed against the generated OpenAPI types insrc/api-types.ts(proto → OpenAPI → openapi-typescript). A field rename in the proto surfaces as a compile error here.tsconfig.jsonadded;npm run typecheck(tsc --noEmit, strict mode) added totest:workerin misepackage.json:typecheckscript;typescriptand@cloudflare/workers-typesadded as devDependenciesTests
updateMask, aggregation, validation, deletion, list, pagination, bucket isolation, and missingX-Device-Idheader (simulated local D1, no real database needed)CI
protojob: runsbuf linton every proto-touching PR/push; runsbuf breakingon PRs only (FILE stability, appropriate for v1alpha — switch to WIRE_JSON_COMPATIBLE when the API graduates to v1)Deploy notes
One-time provisioning before first deploy:
The deploy
CLOUDFLARE_API_TOKENmust include D1: Edit in addition to Workers Scripts: Edit.Regenerate types after proto changes: