125 lines
8.2 KiB
Markdown
125 lines
8.2 KiB
Markdown
---
|
|
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)
|