Skip to content

Check parish ID out-of-bounds condition#97

Open
hepplerj wants to merge 10 commits into
mainfrom
refactor/parish-out-of-range-check
Open

Check parish ID out-of-bounds condition#97
hepplerj wants to merge 10 commits into
mainfrom
refactor/parish-out-of-range-check

Conversation

@hepplerj

@hepplerj hepplerj commented Feb 17, 2026

Copy link
Copy Markdown
Member

Closes #44

Replaces the original hardcoded parish ID range check with validation against the database, per the design in docs/superpowers/specs/2026-07-23-parish-id-validation-design.md.

  • parseAPIParameters (in bom-bills.go) rejects only malformed (non-integer) parish IDs.
  • A new Server.invalidParishIDs method (in bom-parishes.go) checks requested IDs against bom.parishes with a single WHERE id = ANY($1) lookup.
  • BillsHandler returns 400 invalid parish ID(s): <ids> for unknown IDs and 500 if the lookup query fails.
  • clientCacheMiddleware now marks error responses Cache-Control: no-store (previously every response, including errors, was client-cacheable for a week — a stale 400 could otherwise mask a newly added parish).

Chosen over a corrected hardcoded bound (drifts as the parish table grows — the original 158 was already stale at 151) and a TTL-cached ID set (staleness window rejects newly added parishes). The per-request lookup is a single primary-key query, and the endpoint sits behind the server's 1-hour response cache.

  • Also folds in a one-line go vet fix: the Types: debug log in DeathCausesHandler (bom-causes.go) listed 14 %T verbs for 15 arguments, which blocked go test's implicit vet step repo-wide. go vet ./... is clean on this branch.

Pre-merge check: ✅ Done — TestBomBillsInvalidParish passed against the live database (2026-07-23), verifying the exact 400 body contract end-to-end.

@hepplerj hepplerj self-assigned this Feb 17, 2026
hepplerj and others added 9 commits July 23, 2026 15:04
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Parish ID existence will be validated against the database instead.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Unknown parish IDs now return 400 with the offending IDs listed,
instead of silently returning an empty array. Closes #44.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…on test

Error responses previously carried Cache-Control: max-age=604800, so a
400 for a not-yet-added parish could be cached by clients for a week.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The log.Printf listed 14 %T verbs but passed 15 arguments (row.Name had
no verb), which failed go vet and blocked go test's implicit vet step.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
TestMain set lowercase apiary_LOGGING, so access logging stayed on
during integration test runs and polluted test output.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Should we add checking for out-of-range values when looking up parish IDs?

1 participant