golem15/summercms
3
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

docs(quick-261001-ddh): Replace lagoon hardcoded ICU pl-PL database locale with per-query COLLATE option

Browse Source
This commit is contained in:
Jakub Zych
2026-10-01 09:55:02 +02:00
parent 026ce74234
commit 9115a43a71
3 changed files with 427 additions and 1 deletions
Download Patch File Download Diff File Expand all files Collapse all files

301
.planning/quick/261001-ddh-replace-lagoon-hardcoded-icu-pl-pl-datab/261001-ddh-PLAN.md Normal file
View File

@@ -0,0 +1,301 @@
---
phase: quick-261001-ddh
plan: 01
type: execute
wave: 1
depends_on: []
files_modified:
# summercms.go (framework) - Task 1, one commit
- modules/lagoon/order.go
- modules/lagoon/connection.go
- modules/lagoon/order_test.go
- modules/lagoon/connection_test.go
- modules/lagoon/migrations_test.go
- modules/lagoon/example_test.go
- modules/lagoon/postgres_test.go
- modules/lagoon/export_docs_test.go
- modules/lagoon/attach/lifecycle_test.go
- modules/cabana/auth_test.go
- modules/beachcomber/postgres_test.go
- modules/lighthouse/postgres_test.go
- modules/bouncer/phase07_coverage_test.go
- modules/conga/postgres_test.go
- docs/examples/blog/postgres_test.go
- modules/lagoon/README.md
- README.md
- docs/database/queries-and-pagination.md
- docs/database/models.md
- docs/setup/installation.md
- docs/plugins/testing.md
- scripts/check-phase8.sh
# fonoteka.go (sibling application repo) - Task 2, one commit there
- ../fonoteka.go/plugins/golem15/fonoteka/controllers/genre_controller.go
- ../fonoteka.go/parity/genre_integration_test.go
- ../fonoteka.go/parity/migrate_test.go
- ../fonoteka.go/README.md
# summercms.go planning docs - Task 3, separate commit
- .planning/notes/lagoon-per-query-collation.md
- .planning/phases/03-first-vertical-slice-genres-end-to-end/03-CONTEXT.md
autonomous: true
requirements: [QUICK-261001-ddh]
estimate:
tokens: 185000
raw_tokens: 185000
tasks: 3
confidence: low
must_haves:
truths:
- "lagoon.Open and lagoon.Use connect to a PostgreSQL database whatever its default locale (a libc 'C' database included); no locale query or locale error exists in lagoon any more"
- "lagoon.OrderBy(db, col, dir, allowed, lagoon.Collate(\"pl-x-icu\")) emits ORDER BY <col> COLLATE \"pl-x-icu\" ASC|DESC; called without options it emits exactly the SQL it emitted before (existing callers compile and behave unchanged)"
- "A collation name that is empty, longer than 63 bytes, or contains anything outside ASCII letters, digits, underscore, hyphen, dot and @ is rejected by lagoon.OrderBy with an error before any SQL is built, so the collation cannot carry SQL"
- "On a libc 'C' database, ordering with lagoon.Collate(\"pl-x-icu\") puts a name starting with Ł between L and M, while the plain ORDER BY puts it last"
- "No framework README, docs page or framework test harness requires or provisions an ICU pl-PL database; summer docs:build --check and TestDocsTree pass"
- "The application's genre list orders genre names with COLLATE \"pl-x-icu\" and returns the recorded Polish order both on the ICU pl-PL parity database and on a libc 'C' database"
- "A decision note superseding Phase 3 D-06 exists and D-06 points to it"
artifacts:
- path: "modules/lagoon/order.go"
provides: "OrderBy with variadic OrderOption; Collate option; collation validation and identifier quoting"
contains: "func Collate("
- path: "modules/lagoon/connection.go"
provides: "Open/Use without any locale check"
- path: "modules/lagoon/order_test.go"
provides: "Unit tests for collation clause and rejection, plus a libc-database integration test of Polish ordering"
contains: "pl-x-icu"
- path: "docs/database/queries-and-pagination.md"
provides: "Sorting with a collation section backed by ExampleCollate"
contains: "src=modules/lagoon/example_test.go#ExampleCollate"
- path: "../fonoteka.go/plugins/golem15/fonoteka/controllers/genre_controller.go"
provides: "Genre list ordered with lagoon.Collate"
contains: "lagoon.Collate("
- path: ".planning/notes/lagoon-per-query-collation.md"
provides: "Decision note superseding D-06"
contains: "Supersedes"
key_links:
- from: "lagoon.OrderBy"
to: "orderClause"
via: "validated collation passed through and quoted as a double-quoted identifier"
pattern: "COLLATE \\\""
- from: "genre_controller.go ListGenres query"
to: "lagoon.OrderBy"
via: "lagoon.Collate(\"pl-x-icu\") option at the PHP PolishOrder::apply call site"
pattern: "lagoon\\.Collate\\("
- from: "docs/database/queries-and-pagination.md"
to: "modules/lagoon/example_test.go ExampleCollate"
via: "src= snippet kept in sync by summer docs:sync and checked by docs:build --check"
pattern: "#ExampleCollate"
---
<!-- planner-discipline-allow: CheckLocale -->
<!-- planner-discipline-allow: ICU_LOCALE -->
<!-- planner-discipline-allow: icu-locale -->
<!-- planner-discipline-allow: fonoteka -->
<!-- planner-discipline-allow: TestGenreQueryFailsOnWrongLocale -->
<!-- planner-discipline-allow: TestMigrateRejectsNonPolishLocale -->
<!-- planner-discipline-allow: co-authored-by -->
<objective>
Replace lagoon's hardcoded database-level ICU pl-PL locale requirement (Phase 3 D-06, commits d0d8450 and 56be82f) with a per-query collation option on `lagoon.OrderBy`: `lagoon.Collate("pl-x-icu")` makes the helper emit `ORDER BY <col> COLLATE "pl-x-icu" ASC`. Remove the connect-time locale check from `lagoon.Open`/`lagoon.Use`, remove the exported `CheckLocale` (pre-v1 framework API; no consumer calls it directly — the application's tests only assert its error through Open/Use), switch the framework's own test databases to plain PostgreSQL, update every README and docs page in the same commit, then move the application's genre list onto the new option and record the decision note superseding D-06.
Purpose: the framework must know nothing about the consuming application. Today every SummerCMS application, test container and docs example must provision a Polish ICU database because one application's PHP original ordered four name columns with `COLLATE utf8mb4_polish_ci`. A per-query collation keeps that ordering exactly where the PHP code applied it (the `PolishOrder::apply` call site) and needs no database setup: `pl-x-icu` exists in any ICU-enabled PostgreSQL, including `postgres:16-alpine`.
Output: lagoon `Collate`/`OrderOption` API with tests and docs (summercms.go commit 1), the application genre call site and tests (fonoteka.go commit), the decision note (summercms.go planning-docs commit).
Locked direction (decided with the user, option 1): per-query COLLATE option; no replacement config key; no database-level locale requirement in the framework; no collated index now (follow-up only); the genre fixture-order parity test stays; the application's many pl-PL test containers stay as they are.
</objective>
<execution_context>
@~/.claude/gsd-core/workflows/execute-plan.md
@~/.claude/gsd-core/templates/summary.md
</execution_context>
<context>
@.planning/STATE.md
@CLAUDE.md
@modules/lagoon/order.go
@modules/lagoon/connection.go
@modules/lagoon/README.md
Two git repositories, both on branch master:
- Framework: /media/nvme/dev/golem15/summercms.io/summercms/summercms.go (this repo; planning docs live here)
- Application: /media/nvme/dev/golem15/summercms.io/summercms/fonoteka.go (go.work workspace; its go.mod and the plugin go.mod `replace git.golem15.com/golem15/summercms => ../summercms.go`, so it builds against the framework working tree)
The framework working tree has unrelated untracked files (`summer` binary, `go.work.sum`, `.gsd/`, `.planning/debug/...`, `.planning/research/.cache/`) and modified `.planning/milestone.lock` / `.planning/state.json`. Stage files by explicit path only; never `git add -A` or `git add .`.
Commit rules (both repos): no co-author or attribution trailers of any kind; one logical change per commit; planning docs and code in separate commits.
Current lagoon shapes the executor needs (read the files for full detail):
- `func OrderBy(db *gorm.DB, column, dir string, allowed []string) (*gorm.DB, error)` calls `orderClause(column, dir, allowed)` which returns `column + " ASC"` / `" DESC"` after an exact allow-list match; result goes to `db.Order(clause)` (GORM treats a string as a raw column, so quotes pass through untouched).
- connection.go: constants `requiredLocaleProvider`/`requiredICULocale`, exported `CheckLocale(ctx, *sql.DB) error`, unexported `checkLocale(provider, icu string) error`; `Open` and `Use` both call `CheckLocale` after Ping.
- Test harness modules/lagoon/postgres_test.go: `startLagoonPostgres` passes `POSTGRES_INITDB_ARGS` with an ICU pl-PL locale; `lagoonDB(t)` returns the admin pool (skips under -short); `dedicatedDB(t, name)` creates `<name>` with ICU pl-PL and returns `(*sql.DB, dsn)`; `dsnWithDB(dsn, name)` swaps the database name.
- modules/lagoon/example_test.go `ExampleOrderBy` (around line 188) uses a DryRun GORM handle and the `Post` model (`acme_blog_posts`); docs/database/queries-and-pagination.md embeds it via `src=modules/lagoon/example_test.go#ExampleOrderBy`. `summer docs:sync` rewrites `src=` blocks from source; `summer docs:build --check` fails on drift, unknown identifiers (every backticked `lagoon.X` must exist), broken links and anchors, and consuming-application names in docs/ and module READMEs.
Not to touch: the `pl-PL` strings in modules/phrasebook, modules/cabana/form_schema_test.go and examples/hello/hello_test.go are i18n translation locales, unrelated to the database. examples/hello has no database locale setup.
</context>
<tasks>
<task type="tracer" tdd="true">
<name>Task 1: Framework: lagoon.Collate per-query collation, drop the database locale check, plain test databases, README and docs (summercms.go, one commit)</name>
<files>modules/lagoon/order.go, modules/lagoon/connection.go, modules/lagoon/order_test.go, modules/lagoon/connection_test.go, modules/lagoon/migrations_test.go, modules/lagoon/example_test.go, modules/lagoon/postgres_test.go, modules/lagoon/export_docs_test.go, modules/lagoon/attach/lifecycle_test.go, modules/cabana/auth_test.go, modules/beachcomber/postgres_test.go, modules/lighthouse/postgres_test.go, modules/bouncer/phase07_coverage_test.go, modules/conga/postgres_test.go, docs/examples/blog/postgres_test.go, modules/lagoon/README.md, README.md, docs/database/queries-and-pagination.md, docs/database/models.md, docs/setup/installation.md, docs/plugins/testing.md, scripts/check-phase8.sh</files>
<precondition>Docker daemon is running (`docker ps` succeeds): the lagoon integration test and the other module database tests start postgres:16-alpine through testcontainers-go.</precondition>
<read_first>
- modules/lagoon/order.go (all), modules/lagoon/connection.go (all)
- modules/lagoon/order_test.go (all), modules/lagoon/connection_test.go lines 17-130, modules/lagoon/migrations_test.go lines 40-56
- modules/lagoon/postgres_test.go lines 40-130, modules/lagoon/export_docs_test.go (all)
- modules/lagoon/example_test.go lines 180-210 (ExampleOrderBy and the imports at the top)
- modules/lagoon/README.md lines 14-24, 110-145, 195-202
- README.md lines 13-18 and 33-70; docs/setup/installation.md lines 12-16, 74-82, 114-122
- docs/database/queries-and-pagination.md lines 13-45; docs/database/models.md line 11; docs/plugins/testing.md lines 32-40
- internal/docsite/snippet.go around lines 520-660 only if a snippet check fails and the message is unclear
</read_first>
<behavior>
RED first: add the new tests below, run `go test -short ./modules/lagoon/ -run 'TestOrderClause|TestCollate|ExampleCollate'` and confirm it fails to compile (no `Collate` yet), then implement.
- Unit (order_test.go, neutral table names such as `acme_blog_posts.title` and `items.title`; drop the existing application table name from this file): `orderClause` with no collation returns exactly `items.title ASC` / `items.title DESC` and contains no COLLATE (keep the existing assertion); with collation `pl-x-icu` it returns exactly `items.title COLLATE "pl-x-icu" ASC`, and `DESC` likewise.
- Unit: accepted collation names: `pl-x-icu`, `und-x-icu`, `en-US-x-icu`, `C`, `POSIX`, `ucs_basic`, `default`, `sr_RS.utf8@latin`, and a 63-byte name.
- Unit: rejected with an error whose text is `lagoon: order collation "<name>" is not a valid collation name` (use %q): empty string, a 64-byte name, `pl x icu` (space), `pl-x-icu"` (quote), `pl-x-icu" ASC, (SELECT 1) --`, `pl;DROP TABLE x`, a backslash name, a NUL byte, a non-ASCII name such as `pł-x-icu`, and a name starting with `-` or `.`. Every rejection comes back from `OrderBy` as `(nil, err)` without touching the GORM handle.
- Unit: multiple `Collate` options: the last one wins; no options keeps the old behaviour; column and direction allow-list errors are unchanged and are checked before the collation.
- Example (example_test.go): new `ExampleCollate` on the DryRun handle orders `Post` by `title` asc with `lagoon.Collate("pl-x-icu")` and prints `SELECT * FROM "acme_blog_posts" WHERE "acme_blog_posts"."deleted_at" IS NULL ORDER BY title COLLATE "pl-x-icu" ASC`, then prints the rejection error for the injection-shaped name `pl-x-icu" ASC, (SELECT 1) --`. `ExampleOrderBy` keeps its current output unchanged.
- Integration (order_test.go, skipped under -short via `lagoonDB`): create a dedicated database `lagoon_collate_libc` with `TEMPLATE template0 ENCODING 'UTF8' LOCALE_PROVIDER libc LOCALE 'C'` (drop it in t.Cleanup with `WITH (FORCE)`), open it with `lagoon.Open` (must succeed: proves no locale gate) and also `lagoon.Use` on the returned pool, create a table `words(name text)`, insert `Zebra`, `Łoś`, `Lis`, `Mysz`; `OrderBy` on `words.name` asc without options scans `[Lis Mysz Zebra Łoś]`; with `lagoon.Collate("pl-x-icu")` scans `[Lis Łoś Mysz Zebra]`; with `lagoon.Collate("no-such-collation-x")` the query fails with a Postgres error mentioning the collation (proves the quoted name reaches Postgres as an identifier). Do not put Polish album or catalogue words in framework tests; neutral words only.
</behavior>
<action>
Implements the locked decision (option 1) that supersedes Phase 3 D-06. One commit in summercms.go containing code, tests, README and docs together, as CLAUDE.md requires for an exported-API change under modules/.
1. modules/lagoon/order.go: add an exported opaque option type `OrderOption` (a func over an unexported options struct holding the collation name and a flag that a collation was given) and `func Collate(name string) OrderOption` with a doc comment saying it sorts the column with the named PostgreSQL collation, for example the ICU collation `pl-x-icu`, which needs no database setup in an ICU-enabled server. Change `OrderBy` to `func OrderBy(db *gorm.DB, column, dir string, allowed []string, opts ...OrderOption) (*gorm.DB, error)` — variadic so every existing caller compiles unchanged. Apply options in order (last Collate wins). Pass the collation into `orderClause` (add a parameter). In `orderClause`: keep the column allow-list check first and the direction check second, then, only when a collation was given, validate it with a small byte-loop helper (no regexp needed): length 1-63 bytes (PostgreSQL NAMEDATALEN-1), first byte an ASCII letter, digit or underscore, remaining bytes ASCII letters, digits, `_`, `-`, `.`, `@`. On failure return `lagoon: order collation %q is not a valid collation name`. On success emit `<column> COLLATE "<name>" ASC|DESC`, wrapping the name in double quotes as a PostgreSQL identifier (the character set already excludes `"`, so no escaping is reachable; still build the quoted form in one helper so the quoting rule lives in one place). Rewrite the OrderBy doc comment: identifiers and directions are never taken from untrusted input, and no COLLATE is emitted unless `Collate` is passed; remove the sentence about the database default locale.
2. modules/lagoon/connection.go: delete the `requiredLocaleProvider`/`requiredICULocale` constants, the exported `CheckLocale` and the unexported `checkLocale`, and the two call sites in `Open` and `Use`. Update the Open and Use doc comments to drop the ICU pl-PL requirement (Open pings the DSN through pgx stdlib and returns the pool plus a GORM handle on it; Use pings an existing pool). Keep all other behaviour (ping, shared-pool identity check, after-commit callback registration) byte-for-byte.
3. Tests in modules/lagoon: write the tests from `<behavior>` first (RED), then implement 1 and 2 (GREEN). Delete `TestCheckLocaleRejectsNilSQL` (connection_test.go), `TestWrongICULocaleFailsOpen` (connection_test.go; its libc-database proof moves into the new integration test) and `TestCheckLocaleMessage` (migrations_test.go); remove imports that become unused. Add `ExampleCollate` to example_test.go right after `ExampleOrderBy`.
4. Plain test databases across the framework (the check is gone, so nothing needs ICU pl-PL any more): in modules/lagoon/postgres_test.go, modules/lagoon/attach/lifecycle_test.go, modules/cabana/auth_test.go, modules/beachcomber/postgres_test.go, modules/lighthouse/postgres_test.go, modules/bouncer/phase07_coverage_test.go and modules/conga/postgres_test.go remove the `POSTGRES_INITDB_ARGS` env entry that sets the ICU locale (remove the whole `testcontainers.WithEnv(...)` option when it held nothing else; keep the testcontainers import only if still used). Change every per-test `CREATE DATABASE <name> TEMPLATE template0 ENCODING 'UTF8' LOCALE_PROVIDER icu ICU_LOCALE 'pl-PL'` to `CREATE DATABASE <name> TEMPLATE template0 ENCODING 'UTF8'`, and reword the comments that say "dedicated ICU pl-PL database" to "dedicated database". modules/lagoon/export_docs_test.go: reword the `DocsDB` comment to "a fresh database". docs/examples/blog/postgres_test.go: same CREATE DATABASE change, rename `icuDatabase` to `testDatabase` (update its caller around line 140 and the comments at lines ~103 and ~136 so they no longer mention an ICU locale or that lagoon requires one).
5. scripts/check-phase8.sh (around line 252): keep the container's initdb args as they are (that gate boots the application, whose recommended database stays ICU pl-PL), but rewrite the stale comment: the ICU pl-PL locale matches the application's recommended database; lagoon itself no longer checks the database locale.
6. modules/lagoon/README.md: replace the "Database check at connect time" Features bullet by extending the "Safe ordering" bullet: `lagoon.OrderBy` appends an ORDER BY only for an allow-listed column and an `asc` or `desc` direction, and `lagoon.Collate` adds a validated `COLLATE` clause for language-specific text order (for example the ICU collation `pl-x-icu`); lagoon puts no requirement on the database's default locale. API reference table: delete the `CheckLocale` row; `lagoon.Open` row becomes "Opens and pings a DSN and returns the pool plus a GORM handle on it."; `lagoon.Use` row becomes "Returns a GORM handle on an existing pool after a ping."; `lagoon.OrderBy` row becomes "Allow-listed ORDER BY, with an optional `lagoon.Collate`."; add rows `lagoon.Collate` ("Order option that sorts the column with a named PostgreSQL collation; the name is validated and quoted.") and `lagoon.OrderOption` ("Option type accepted by `lagoon.OrderBy`."). Testing section: drop "initialised with the ICU `pl-PL` locale". The README must not name the consuming application.
7. Root README.md: Requirements line becomes "PostgreSQL 16 for any application that uses the data layer ([lagoon](modules/lagoon/README.md))." (drop the locale sentence and the Known issues pointer). Quick start step 1 becomes "Create a PostgreSQL 16 database:" with `CREATE DATABASE hello;`. Delete the lagoon bullet from "Known issues"; keep the other two bullets unchanged.
8. docs/setup/installation.md: Requirements line drops the locale sentence; "Create the database" section says "The commands that touch data open PostgreSQL. Create a database for the example:" with `CREATE DATABASE hello;`; delete the lagoon bullet from the Known issues warning block.
9. docs/database/queries-and-pagination.md: replace the paragraph that says OrderBy never adds a COLLATE clause with a new `## Sorting with a collation` section (placed before `## Pagination`): text sorts by the database's default collation unless you pass `lagoon.Collate`; a list that must follow one language's alphabet (Polish puts Ł between L and M, for example) passes `lagoon.Collate("pl-x-icu")`; ICU collations named `<language>-x-icu` exist in any PostgreSQL built with ICU support, which includes the official Docker images, so the database needs no special locale; the name is validated (letters, digits, `_`, `-`, `.`, `@`, at most 63 bytes) and quoted, and anything else is an error; an index only helps that ORDER BY when it is built with the same collation. Embed the example with a `go src=modules/lagoon/example_test.go#ExampleCollate` fenced block and fill its body by running `go run ./cmd/summer docs:sync` from the repo root. Update the frontmatter description only if it stops being accurate.
10. docs/database/models.md line 11: the data layer supports PostgreSQL only and puts no requirement on the database's default locale; a list that must sort text in one language's order passes a collation to `lagoon.OrderBy`, as described in [Queries and pagination](queries-and-pagination.md#sorting-with-a-collation). Remove the `CheckLocale` mention and the instruction to create the database with a locale.
11. docs/plugins/testing.md lines 34-38: the framework's own tests start a `postgres:16-alpine` container through testcontainers-go and create a fresh database per test; delete the bullet about creating the database with an ICU locale; keep the skip-under-short and migrate-per-test bullets.
12. Run `gofmt -l modules docs/examples` (must print nothing), `go vet ./...`, the targeted lagoon tests, `go test ./cmd/summer -run TestDocsTree`, `go run ./cmd/summer docs:build --check`, then the full `go test ./...` (Docker; allow up to ~15 minutes). If any framework database test turns out to depend on Polish default collation for ordering, stop and report it in the summary rather than restoring the ICU database.
13. Commit only the files in `<files>` by explicit path, message: `feat(lagoon): per-query collation for OrderBy, drop the database locale check` with a short body naming the new API (`lagoon.Collate`, `lagoon.OrderOption`), the removed `CheckLocale`, the plain test databases and the README/docs updates. No co-author or attribution trailer.
</action>
<verify>
<automated>cd /media/nvme/dev/golem15/summercms.io/summercms/summercms.go && test -z "$(gofmt -l modules docs/examples)" && go vet ./... && go test -count=1 ./modules/lagoon/ -run 'TestOrderClause|TestCollate|ExampleOrderBy|ExampleCollate|TestSharedSQLPool|TestOpenFromAppReadsDSN' && go test -count=1 ./cmd/summer -run TestDocsTree && go run ./cmd/summer docs:build --check && test -z "$(grep -rnE 'CheckLocale|ICU_LOCALE|icu-locale|ICU .?pl-PL' modules docs examples README.md)" && test -z "$(grep -n fonoteka modules/lagoon/order_test.go modules/lagoon/example_test.go)"</automated>
</verify>
<done>
`lagoon.Collate` and `lagoon.OrderOption` exist; OrderBy emits `COLLATE "pl-x-icu"` only when asked and rejects unsafe names; Open/Use accept a libc 'C' database; the libc integration test shows Ł between L and M with the collation and last without it. No framework README, docs page or test harness provisions an ICU pl-PL database (scripts/check-phase8.sh keeps its args with a corrected comment). gofmt clean, `go vet ./...` and full `go test ./...` green, TestDocsTree and `docs:build --check` pass. One summercms.go commit with code, tests, READMEs and docs, no co-author trailer.
</done>
</task>
<task type="auto" tdd="true">
<name>Task 2: Application: genre list sorts with lagoon.Collate("pl-x-icu"); replace the locale-check tests (fonoteka.go, one commit)</name>
<files>/media/nvme/dev/golem15/summercms.io/summercms/fonoteka.go/plugins/golem15/fonoteka/controllers/genre_controller.go, /media/nvme/dev/golem15/summercms.io/summercms/fonoteka.go/parity/genre_integration_test.go, /media/nvme/dev/golem15/summercms.io/summercms/fonoteka.go/parity/migrate_test.go, /media/nvme/dev/golem15/summercms.io/summercms/fonoteka.go/README.md</files>
<precondition>Task 1 is committed in summercms.go: `go doc ./modules/lagoon Collate` run in the framework repo prints the Collate doc; Docker daemon is running.</precondition>
<read_first>
- fonoteka.go/plugins/golem15/fonoteka/controllers/genre_controller.go lines 30-45 and 100-145
- fonoteka.go/parity/genre_integration_test.go lines 1-120 (imports, recordedPolishGenreNames, harness use, app.Handler call, signToken) and 136-220 (source grep, locale test)
- fonoteka.go/parity/migrate_test.go lines 205-240 (locale test to delete) and 550-557 (gormOnSharedPool)
- fonoteka.go/parity/genre_integration_test.go helper signatures at lines 253-360 (assertPolishNameOrder, namesOf, genreIDsBySlug, insertCollection, setContext) and wherever insertUser, activateAppPlugins, testConfig, dsnWithDB, parityDSN and testJWTSecret are defined (grep for them)
- fonoteka.go/plugins/golem15/fonoteka/updates/00_base.go lines 28-36 (golem15_fonoteka_genres columns: id, name, slug, description, timestamps with defaults)
- fonoteka.go/README.md lines 9-22
</read_first>
<behavior>
- The existing `TestGenreCountsScopedToActiveCollection` (ICU pl-PL parity database) still passes unchanged apart from its source grep, which now also requires `lagoon.Collate(` in genre_controller.go next to `lagoon.OrderBy`.
- New `TestGenrePolishOrderOnLibcDatabase` replaces `TestGenreQueryFailsOnWrongLocale`: on a dedicated libc 'C' database the app migrates and serves the genre list; with one extra genre whose name starts with Ł the response order is the recorded 15 names with that genre inserted right after `Latin` and before `Non-Music`; a raw `SELECT name FROM golem15_fonoteka_genres ORDER BY name` on the same database puts that genre last (proves the test discriminates and the order comes from the per-query collation, not the database).
- The genre fixture-order parity tests (genre_smoke_test.go and the recorded `recordedPolishGenreNames` order) stay and pass.
</behavior>
<action>
Implements the application side of the locked option-1 decision that supersedes Phase 3 D-06. Work in /media/nvme/dev/golem15/summercms.io/summercms/fonoteka.go; commit there (separate repo).
1. genre_controller.go: add a package-level constant `polishCollation = "pl-x-icu"` with a comment: PostgreSQL ICU Polish collation standing in for the `COLLATE utf8mb4_polish_ci` that PHP `PolishOrder::apply` adds; applied per query, so the database's default locale does not matter (ICU Polish and MariaDB utf8mb4_polish_ci can still differ on edge cases, which is why the recorded fixture order is tested). Change the call at line ~134 to `lagoon.OrderBy(q, "golem15_fonoteka_genres.name", "asc", polishOrderColumns, lagoon.Collate(polishCollation))`. Response shapes and everything else in the handler stay unchanged (API parity rule).
2. parity/genre_integration_test.go: extend the source grep (line ~144) to also require `lagoon.Collate(` with a message saying the PolishOrder call site must pass the Polish collation. Delete `TestGenreQueryFailsOnWrongLocale` and add `TestGenrePolishOrderOnLibcDatabase` per `<behavior>`: from `parityDB(t)` create database `genre_libc` with `TEMPLATE template0 ENCODING 'UTF8' LOCALE_PROVIDER libc LOCALE 'C'` (tolerate "already exists"; drop it `WITH (FORCE)` in t.Cleanup after closing the pool), open it via `dsnWithDB(parityDSN, ...)` and `sql.Open("pgx", ...)`, `gormOnSharedPool` (lagoon.Use must succeed), `activateAppPlugins` + `lagoon.Migrate`, insert genre name `Łódź Beat` slug `lodz-beat`, assert the raw plain ORDER BY puts it last, then create a user, an owned collection and its context with the existing helpers, build the handler with `app.Handler(t.Context(), testConfig(t), app.PluginIDs, <libc pool>)`, sign a token the way the existing test does, call the genre list and assert the expected order (build the expected slice from `recordedPolishGenreNames` by inserting `Łódź Beat` after `Latin`). Remove imports that become unused.
3. parity/migrate_test.go: delete `TestMigrateRejectsNonPolishLocale` (its subject no longer exists; the new genre test covers Open/Use plus Migrate on a libc database). Remove imports that become unused.
4. README.md "Database setup": keep the recommended `CREATE DATABASE fonoteka ... ICU_LOCALE 'pl-PL'` statement but change "must use" to a recommendation, explaining that queries without an explicit collation (for example admin relation option lists ordered by name) follow the database default, so a Polish ICU default keeps them closest to the PHP app. Replace the sentence claiming boot refuses libc or non-pl-PL databases with: the genre list (the PHP `PolishOrder` call site) sorts with an explicit `COLLATE "pl-x-icu"`, so its order does not depend on the database default, and the framework does not check the database locale. Keep the test-container initdb line (the app's test containers stay on ICU pl-PL). Leave config/database.yaml comments as they are.
5. Do not churn the other application test containers that pass ICU pl-PL initdb args or create ICU databases; they are harmless and listed as follow-up cleanup in Task 3's note.
6. Run `gofmt -l` on the touched packages (must print nothing), `go vet ./...`, `go test -count=1 ./parity -run 'Genre'`, then the full `go test ./...` in fonoteka.go (Docker; allow up to ~15 minutes).
7. Commit the four files by explicit path in fonoteka.go with message `fix(genres): sort genre names with a per-query Polish collation` and a short body (lagoon.Collate("pl-x-icu") at the PolishOrder call site; locale-check tests replaced by a libc-database ordering test; README database section). No co-author or attribution trailer.
</action>
<verify>
<automated>cd /media/nvme/dev/golem15/summercms.io/summercms/fonoteka.go && test -z "$(gofmt -l plugins/golem15/fonoteka/controllers parity)" && go vet ./... && go test -count=1 ./parity -run 'Genre' && grep -q 'lagoon.Collate(' plugins/golem15/fonoteka/controllers/genre_controller.go && test -z "$(grep -n 'TestGenreQueryFailsOnWrongLocale\|TestMigrateRejectsNonPolishLocale' parity/*.go)"</automated>
</verify>
<done>
The genre handler passes `lagoon.Collate("pl-x-icu")`; the ICU pl-PL parity genre tests and fixture-order tests pass; the new libc-database test proves the Polish order comes from the query; both old locale-check tests are gone; README no longer claims boot refuses a non-Polish database. `go vet ./...` and full `go test ./...` green in fonoteka.go. One fonoteka.go commit, no co-author trailer.
</done>
</task>
<task type="auto">
<name>Task 3: Decision note superseding Phase 3 D-06 (summercms.go planning-docs commit)</name>
<files>.planning/notes/lagoon-per-query-collation.md, .planning/phases/03-first-vertical-slice-genres-end-to-end/03-CONTEXT.md</files>
<read_first>
- .planning/notes/oauth2-wristband-direct-port.md lines 1-20 (note format: title, Date, Status, Supersedes, Decision, Rationale)
- .planning/phases/03-first-vertical-slice-genres-end-to-end/03-CONTEXT.md line 26 (D-06)
</read_first>
<action>
Records the option-1 decision that supersedes D-06, in a planning-docs commit separate from the code commits.
1. Create .planning/notes/lagoon-per-query-collation.md in the existing note format: `# Decision: Per-query collation in lagoon.OrderBy`, Date 2026-10-01, Status Accepted, Supersedes Phase 3 D-06 (database-level ICU pl-PL locale, implemented by d0d8450 and 56be82f). Sections: Decision (lagoon.OrderBy takes `lagoon.Collate(name)`, emitting a validated, double-quoted `COLLATE`; lagoon no longer checks the database locale; `CheckLocale` removed as pre-v1 API with no direct consumers; no replacement config key); Rationale (the framework must know nothing about the application; the DB-level requirement forced every app, test container and docs example to provision Polish ICU; `pl-x-icu` ships with any ICU-enabled PostgreSQL; the collation now sits exactly where PHP applied `COLLATE utf8mb4_polish_ci`); Consequences (framework tests and docs use plain databases; the application passes `lagoon.Collate("pl-x-icu")` at the genre call site and still recommends an ICU pl-PL default for queries without an explicit collation; name the framework and application commit hashes from Tasks 1 and 2); Follow-ups (a collated expression index on the PolishOrder columns once those tables grow, only useful when built with the same collation; the other three PolishOrder columns adopt `lagoon.Collate` when their list endpoints are ported; drop ICU pl-PL from the application's test containers in a cleanup pass; the application's admin relation option query that orders by name without a collation; ICU Polish versus MariaDB utf8mb4_polish_ci can still differ on edge cases, which is why the recorded fixture-order parity test stays; any deviation recorded in the Task 1 or Task 2 summaries).
2. 03-CONTEXT.md: append to the end of the D-06 bullet one sentence: `**Superseded 2026-10-01** by .planning/notes/lagoon-per-query-collation.md: the Polish order is now a per-query COLLATE "pl-x-icu" via lagoon.Collate, and lagoon no longer checks the database locale.` Change nothing else in that file.
3. Commit both files by explicit path in summercms.go with message `docs(quick-261001-ddh): record per-query collation decision superseding D-06`. No co-author or attribution trailer.
</action>
<verify>
<automated>cd /media/nvme/dev/golem15/summercms.io/summercms/summercms.go && grep -q 'Supersedes' .planning/notes/lagoon-per-query-collation.md && grep -q 'pl-x-icu' .planning/notes/lagoon-per-query-collation.md && grep -q 'lagoon-per-query-collation' .planning/phases/03-first-vertical-slice-genres-end-to-end/03-CONTEXT.md && MSG="$(git log -1 --format=%B)" && test -n "$MSG" && test -z "$(printf '%s\n' "$MSG" | grep -i 'co-authored-by')"</automated>
</verify>
<done>The note exists with Supersedes, Decision, Rationale, Consequences and Follow-ups; D-06 points to it; one planning-docs commit in summercms.go without a co-author trailer.</done>
</task>
</tasks>
<threat_model>
## Trust Boundaries
| Boundary | Description |
|----------|-------------|
| caller code -> lagoon.OrderBy -> SQL text | Column, direction and now collation become raw SQL in ORDER BY; GORM does not bind or quote them |
| HTTP query string -> list handlers | Clients choose sort column and direction; lagoon allow-lists them (unchanged) |
## STRIDE Threat Register
| Threat ID | Category | Component | Severity | Disposition | Mitigation Plan |
|-----------|----------|-----------|----------|-------------|-----------------|
| T-ddh-01 | Tampering (SQL injection) | lagoon.Collate / orderClause | high | mitigate | Collation validated byte-by-byte (1-63 bytes, ASCII letters, digits, `_ - . @`, first byte letter/digit/underscore), so `"`, `;`, spaces, comments and non-ASCII are rejected before any SQL is built; valid names are emitted as a double-quoted identifier from one helper; unit tests feed injection-shaped names, ExampleCollate shows the rejection |
| T-ddh-02 | Tampering (SQL injection) | lagoon.OrderBy column/direction | high | mitigate | Existing exact allow-list and asc/desc checks kept and run before the collation check; tests unchanged |
| T-ddh-03 | Information disclosure | Postgres "collation does not exist" error | low | accept | Collation names are application constants, never client input; an unknown name fails the query like any other server error |
| T-ddh-04 | Tampering (data integrity of ordering) | removal of the connect-time locale check | low | accept | Ordering that must follow a language is now explicit per query; queries without a collation follow the database default by design; the application keeps recommending ICU pl-PL for those |
| T-ddh-SC | Tampering | npm/pip/cargo/go installs | low | accept | No new dependencies in either repo; go.mod and go.sum stay unchanged |
</threat_model>
<verification>
- summercms.go: `gofmt -l modules docs/examples` empty; `go vet ./...` and `go test ./...` green (Docker); `go test ./cmd/summer -run TestDocsTree` and `go run ./cmd/summer docs:build --check` pass; `grep -rnE 'CheckLocale|ICU_LOCALE|icu-locale|ICU .?pl-PL' modules docs examples README.md` prints nothing.
- fonoteka.go: `go vet ./...` and `go test ./...` green; genre_controller.go passes `lagoon.Collate(`; the libc-database genre test passes.
- Three commits: framework code+docs, application call site+tests, planning note. None carries a co-author trailer; staged by explicit path only.
</verification>
<success_criteria>
- No database-level locale requirement anywhere in the framework (code, tests, READMEs, docs).
- `lagoon.OrderBy` stays source-compatible and gains a safe, validated per-query collation.
- The application's genre list returns the recorded Polish order regardless of the database's default locale.
- D-06 is superseded by a recorded decision note with follow-ups (collated index, other PolishOrder columns, test-container cleanup).
</success_criteria>
<output>
Create `.planning/quick/261001-ddh-replace-lagoon-hardcoded-icu-pl-pl-datab/261001-ddh-SUMMARY.md` when done, listing the three commit hashes (two repos) and any deviation.
</output>
</content>
</invoke>

124
.planning/quick/261001-ddh-replace-lagoon-hardcoded-icu-pl-pl-datab/261001-ddh-SUMMARY.md Normal file
View File

@@ -0,0 +1,124 @@
---
phase: quick-261001-ddh
plan: 01
status: complete
subsystem: lagoon (data layer), application genre list
tags: [lagoon, collation, postgres, icu, docs, parity]
requires: []
provides:
- lagoon.Collate / lagoon.OrderOption per-query collation for lagoon.OrderBy
- lagoon.Open / lagoon.Use without any database locale check
affects:
- every framework test harness (plain PostgreSQL databases)
- application genre list (fonoteka.go)
tech-stack:
added: []
patterns:
- variadic functional options on an existing exported func (source-compatible API growth)
- byte-loop identifier validation plus one quoting helper for raw SQL fragments
key-files:
created:
- .planning/notes/lagoon-per-query-collation.md
modified:
- modules/lagoon/order.go
- modules/lagoon/connection.go
- modules/lagoon/order_test.go
- modules/lagoon/example_test.go
- modules/lagoon/README.md
- docs/database/queries-and-pagination.md
- ../fonoteka.go/plugins/golem15/fonoteka/controllers/genre_controller.go
- ../fonoteka.go/parity/genre_integration_test.go
decisions:
- Per-query COLLATE via lagoon.Collate supersedes Phase 3 D-06 (database-level ICU pl-PL); CheckLocale removed as pre-v1 API, no replacement config key
metrics:
duration: ~8 min
completed: 2026-10-01
actuals:
tokens: 15200
tasks: 3
commits: 2
plan_head_before: 565ce982d98e02077e4dfc99f219a46ad42a5851
plan_head_after: 026ce7423404edc5771f816fa61eea5eb7d229c7
---
# Quick 261001-ddh: Per-query collation replaces lagoon's ICU pl-PL database requirement
`lagoon.OrderBy(db, col, dir, allowed, lagoon.Collate("pl-x-icu"))` now emits a validated, double-quoted `COLLATE` clause. lagoon no longer checks the database locale, and the framework's tests and docs use plain PostgreSQL. The application's genre list sorts with `pl-x-icu`, so it returns the recorded Polish order on any database. A libc `C` database test proves it.
## Commits
| Task | Repo | Commit | Message |
|------|------|--------|---------|
| 1 | summercms.go | `037dc53` | feat(lagoon): per-query collation for OrderBy, drop the database locale check |
| 2 | fonoteka.go | `2da9482` | fix(genres): sort genre names with a per-query Polish collation |
| 3 | summercms.go | `026ce74` | docs(quick-261001-ddh): record per-query collation decision superseding D-06 |
`commits: 2` counts summercms.go only (measured with `git rev-list --count 565ce98..HEAD`). The fonoteka.go commit `2da9482` sits on top of `1c88199` in the sibling repo. `actuals.tokens` is chars/4 over both repos' diffs (53422 + 7458 bytes).
## What changed
**Task 1, framework (`037dc53`):**
- `modules/lagoon/order.go`:
- Adds `OrderOption` and `Collate(name)`. `OrderBy` now has the signature `OrderBy(db, column, dir, allowed, opts ...OrderOption)`, so existing callers compile unchanged.
- Checks run in order: column allow-list, then direction, then collation. The collation check accepts 1-63 bytes, starting with an ASCII letter, digit or `_` and continuing with letters, digits, `_`, `-`, `.` or `@`.
- The name is emitted through a single `quoteCollation` helper. When the last `Collate` option wins, the clause is `<col> COLLATE "<name>" ASC|DESC`.
- `modules/lagoon/connection.go`: removes the `CheckLocale`/`checkLocale` functions, their constants and both call sites. The Open/Use doc comments are rewritten.
- New tests:
- The clause tests now use neutral table names.
- Tables of accepted and rejected collation names. The rejected list includes injection-shaped names, NUL, non-ASCII, a 64-byte name, and names with a leading `-` or `.`. Each rejection comes back from `OrderBy` as `(nil, err)` and leaves the GORM handle untouched.
- The last `Collate` option wins, and the check order is pinned.
- `ExampleCollate`.
- Integration test `TestCollatePolishOrderOnLibcDatabase`: Open and Use succeed on a libc `C` database. The plain order is `[Lis Mysz Zebra Łoś]` and the `pl-x-icu` order is `[Lis Łoś Mysz Zebra]`. An unknown collation reaches Postgres as an identifier (SQLSTATE 42704).
- Deleted tests: `TestCheckLocaleRejectsNilSQL`, `TestWrongICULocaleFailsOpen`, `TestCheckLocaleMessage`.
- Removed the ICU initdb args and ICU `CREATE DATABASE` from the lagoon, lagoon/attach, cabana, beachcomber, lighthouse, bouncer and conga harnesses and from docs/examples/blog (`icuDatabase` was renamed to `testDatabase`). The testcontainers imports that became unused were dropped in the bouncer and cabana tests.
- Updated the lagoon README, root README, installation, models, queries-and-pagination (new "Sorting with a collation" section with an `ExampleCollate` snippet filled by `docs:sync`) and plugin testing docs. The comment in `scripts/check-phase8.sh` is corrected; its initdb args are kept.
**Task 2, application (`2da9482`):**
- `polishCollation = "pl-x-icu"` is passed at the PolishOrder call site. The response shape is unchanged.
- The source check now also requires `lagoon.Collate(`.
- `TestGenrePolishOrderOnLibcDatabase` replaces `TestGenreQueryFailsOnWrongLocale`. Before the controller change it failed (RED), with `Łódź Beat` sorted last. After the change it passes: `Łódź Beat` comes after `Latin`, while the plain `ORDER BY` on the same database still puts it last.
- `TestMigrateRejectsNonPolishLocale` is deleted.
- README: ICU pl-PL is now a recommendation. The section explains the explicit collation on the genre list and that the framework no longer checks the locale.
**Task 3, decision note (`026ce74`):** `.planning/notes/lagoon-per-query-collation.md` supersedes D-06, and D-06 in `03-CONTEXT.md` points to it.
## Verification
- summercms.go:
- `go vet ./...` is clean.
- Full `go test -count=1 ./...` with Docker passed (35 packages ok, exit 0).
- The targeted lagoon tests pass, including the libc integration test.
- `go test ./cmd/summer -run TestDocsTree` passes, and `go run ./cmd/summer docs:build --check` reports "no problems found".
- `grep -rnE 'CheckLocale|ICU_LOCALE|icu-locale|ICU .?pl-PL' modules docs examples README.md` and the `fonoteka` grep on order_test.go and example_test.go both print nothing.
- fonoteka.go:
- `go vet ./...` is clean in the root module and in both plugin modules.
- Full `go test -count=1 ./...` passes in the root module and in `plugins/golem15/user` and `plugins/golem15/fonoteka`.
- `go test ./parity -run Genre` passes.
- `gofmt -l` on the touched packages is empty.
- None of the three commits has a co-author trailer, and all files were staged by explicit path.
## Deviations from Plan
**1. [Scope boundary] gofmt drift in files this task did not touch.** `gofmt -l modules docs/examples` (part of the plan's automated verify) prints 6 files that were already in that state at `565ce98`: `modules/compass/config_test.go`, `modules/compass/persist_test.go`, `modules/party/registry_test.go`, `modules/tide/flow_test.go`, `modules/tide/headers_test.go`, `modules/wristband/server.go`. The drift is method-alignment whitespace from the newer gofmt. None of the files this task changed are listed. I did not fix them because they are out of scope; they need a separate `gofmt -w` commit.
**2. [Verification scope] fonoteka.go workspace.** In fonoteka.go, `go vet ./...` and `go test ./...` from the repo root cover only the root module (5 packages) under go.work. I also ran both inside `plugins/golem15/user` and `plugins/golem15/fonoteka`, and all passed.
**3. [Rule 3 - Blocking] Unused imports.** After the `WithEnv` option was removed, the `testcontainers` import was unused in `modules/bouncer/phase07_coverage_test.go` and `modules/cabana/auth_test.go`, so I removed it. The plan anticipated this.
Commits were made directly on master, as the plan says (`branching_strategy: none`).
## Known Stubs
None.
## Threat Flags
None. The only new raw-SQL surface is the collation name. It is covered by T-ddh-01: byte validation, one quoting helper, and unit tests with injection-shaped names.
## Self-Check: PASSED
- FOUND: modules/lagoon/order.go (contains `func Collate(`)
- FOUND: docs/database/queries-and-pagination.md (contains `src=modules/lagoon/example_test.go#ExampleCollate`)
- FOUND: ../fonoteka.go/plugins/golem15/fonoteka/controllers/genre_controller.go (contains `lagoon.Collate(`)
- FOUND: .planning/notes/lagoon-per-query-collation.md (contains `Supersedes`)
- FOUND: commits 037dc53, 026ce74 (summercms.go), 2da9482 (fonoteka.go)