From 13ee573cd8dfbd97dcb9f45ed90513bb9725923d Mon Sep 17 00:00:00 2001 From: JohnnyT Date: Sat, 22 Aug 2026 09:23:51 -0600 Subject: [PATCH 1/6] Adds the sp-02x implementation plan Plan for ADR-0002's pre-migration deliverables, worked as the sp-4an.3 epic's first task: the key-generator behaviour, the use StatifierPersistence.Ecto macro, and the versioned migrations helper, plus the epic-entry decisions (Ecto in-package behind optional ecto_sql; real Postgres as the test harness) to be recorded as ADR-0005. Four phases, each independently gate-green; critic-reviewed. Docs only, so the quality gate does not apply to this commit. Refs: sp-02x --- .../260822-sp-02x-keys-tables-migrations.md | 463 ++++++++++++++++++ 1 file changed, 463 insertions(+) create mode 100644 docs/plans/260822-sp-02x-keys-tables-migrations.md diff --git a/docs/plans/260822-sp-02x-keys-tables-migrations.md b/docs/plans/260822-sp-02x-keys-tables-migrations.md new file mode 100644 index 0000000..7c5e096 --- /dev/null +++ b/docs/plans/260822-sp-02x-keys-tables-migrations.md @@ -0,0 +1,463 @@ +# Configurable keys, table names, and migrations (ADR-0002) Implementation Plan + +## Overview + +Implement ADR-0002's deliverables that must exist ahead of the Ecto +adapter's first migration: the key-generator behaviour (UXID default, +UUIDv7, bigserial, custom), the `use StatifierPersistence.Ecto` macro that +carries a host's compile-time configuration, and the versioned +Oban.Migration-style migrations helper that takes the same options - plus +the two epic-entry decisions the sp-4an.3 epic left open (dependency shape +and test harness), recorded as ADR-0005. Bead: sp-02x (first task of the +sp-4an.3 epic). + +## Current State Analysis + +- `mix.exs:52-59` records the open dependency question verbatim: "The Ecto + adapter epic decides its own dependency shape (ecto_sql optional vs a + separate statifier_ecto package) when that work starts." No Ecto anywhere + in the tree yet; no `config/` directory; no database in CI + (`.github/workflows/ci.yml` runs the bare gate on ubuntu-latest). +- ADR-0002 (accepted) fixes: engine identities stored verbatim and + non-configurable; surrogate PKs compile-time configurable (`:uxid` + default via the `uxid` package, `:uuid` as UUIDv7, `:bigserial`, + `{module, opts}` against a key-generator behaviour); configuration on the + host's module via `use StatifierPersistence.Ecto` (no app env); a + versioned migrations helper taking the same options; `statifier_` table + prefix with a per-table override map and a separate Postgres-schema + option; runs vocabulary with a nullable `session_id` column. +- ADR-0003/ADR-0004 fix the adapter surface this DDL must eventually serve: + charts keyed by `content_hash` (verbatim, unique), positions keyed by + engine `session_id`, runs keyed by caller-supplied `run_id` with + `{status, content_hash, identity_blob, position_blob | nil, failure | + nil}`, and an optional `lock_run/3` an Ecto adapter implements as a + transaction-scoped row lock (`SELECT ... FOR UPDATE`) - which constrains + the test-harness choice (see Implementation Approach). +- The conformance suite + (`lib/statifier_persistence/testing/storage_conformance.ex`) and the + optional `isolate/1` callback (`lib/statifier_persistence/storage/adapter.ex:211-224`) + already anticipate an `Ecto.Adapters.SQL.Sandbox` checkout per test. + +### Key Discoveries: + +- `lib/statifier_persistence/storage/adapter.ex:29-63` - the storage + contract's field set is the column list; nothing else may leak into the + behaviour (ADR-0002 decision 1, ADR-0003 decision 3). +- ADR-0002 decision 4 sketches three tables (`statifier_charts`, + `statifier_chart_versions`, `statifier_runs`), but the behaviour keys + charts by content hash only - there is no callback a logical-chart table + would serve. V01 therefore creates `charts` (hash-keyed), + `positions`, and `runs`, recording the collapse as a dated amendment to + ADR-0002 (same unexercised-contract reasoning ADR-0003/0004 use). +- `docs/plans/260822-sp-4an.2.1-run-lifecycle-executor-seam-stepper.md` - + the phase/gate discipline this plan mirrors. +- `.github/workflows/ci.yml` needs a Postgres service container; the gate + (`mix quality`) runs the full test suite, so database-backed tests must + find a server in CI and locally. + +## Desired End State + +A host writes: + + defmodule MyApp.Persistence do + use StatifierPersistence.Ecto, repo: MyApp.Repo + end + +and gets: resolved configuration readable via +`MyApp.Persistence.__statifier_persistence__/1`, three schema modules +(`MyApp.Persistence.Chart`, `.Position`, `.Run`) with UXID string primary +keys (`chart_`/`pos_`/`run_` prefixes) and the engine identity columns +verbatim, and one supported way to create the tables: + + defmodule MyApp.Repo.Migrations.AddStatifierPersistence do + use Ecto.Migration + def up, do: StatifierPersistence.Ecto.Migrations.up(for: MyApp.Persistence) + def down, do: StatifierPersistence.Ecto.Migrations.down(for: MyApp.Persistence) + end + +Every knob (`key:`, `table_prefix:`, `tables:`, `prefix:`) changes both the +schemas and the DDL through one shared config resolver, so they cannot +disagree. Verified end-to-end against real Postgres: migrate up under each +key option, insert through the generated schemas, assert the engine +identity columns and their unique indexes are identical across all key +configurations, migrate down clean. + +## What We're NOT Doing + +- **The Ecto adapter itself** (implementing + `StatifierPersistence.Storage.Adapter` over these schemas, `lock_run/3` + as a row lock, passing the conformance suite) - that is the epic's next + task, planned separately on top of this one. +- **A logical-chart / chart-versions split.** ADR-0002's sketch named + `statifier_chart_versions`; the behaviour exercises only a hash-keyed + chart store, so V01 ships `charts`/`positions`/`runs` and the ADR-0002 + amendment records why. A logical-chart table returns when a real embedder + needs one (the charter's own design rule). +- **Tenancy columns.** ADR-0002 says they ride the same `use`; no host has + specified any yet, and inventing placeholder columns is the unexercised + contract ADR-0003's Consequences warn about. The option surface leaves + room (documented), nothing more. +- **Writing `session_id` on runs from library code.** The column exists + (ADR-0002 decision 5, accepted) as nullable DDL; the run lifecycle does + not populate it yet - that is host/adapter territory later. +- **SQLite or in-memory-fake harnesses.** Rejected in ADR-0005: `lock_run/3` + needs real `SELECT ... FOR UPDATE` semantics and the conformance bar is + meaningless against a fake. +- **Editing `.quality.exs`** (excluded by campaign consent) - the gate is + taken as it stands. + +## Implementation Approach + +Four phases, each independently gate-green and committable. Phase 1 makes +the two epic-entry decisions (ADR-0005) and stands up the Postgres harness +so later phases can test against a real database; Phase 2 is the pure +key-generator layer; Phase 3 the `use` macro over a shared `Config` +resolver; Phase 4 the versioned migrations helper reading the same +`Config`, proven live against Postgres under every key option. + +Dependency shape (ADR-0005, decision made here per the epic's charge): the +Ecto layer lives **in this package** behind `{:ecto_sql, "~> 3.10", +optional: true}` - a separate `statifier_ecto` package would duplicate the +conformance/test surface and split one contract across two repos for no +consumer benefit; `use StatifierPersistence.Ecto` (ADR-0002 decision 3, +accepted) already names this package as the home. Modules that reference +Ecto are wrapped in `if Code.ensure_loaded?(Ecto)` so a host without +ecto_sql still compiles this package. `uxid` becomes a required dependency +(ADR-0002 decision 2: the default must work out of the box); `postgrex` is +`only: :test` here - a host brings its own driver. + +Test harness (ADR-0005): real Postgres - `docker compose up -d db` locally +(port/credentials overridable via `PG*` env vars), a `postgres` service +container in CI. Database-backed tests are ordinary tests in the ordinary +suite; there is no tag that skips them when the server is absent, because +an auto-skipped database suite is the gate weakening `CLAUDE.md` forbids. + +## Phase 1: ADR-0005, dependencies, and the Postgres test harness + +### Overview + +Record the two epic-entry decisions; add the dependencies; stand up a real +Postgres the suite and CI both reach; amend ADR-0002's table sketch. + +### Changes Required: + +#### 1. ADR-0005 +**File**: `docs/adr/0005-ecto-in-package-and-postgres-test-harness.md` +**Changes**: New record, two decisions with the reasoning above: (1) the +Ecto layer ships in this package behind optional ecto_sql, uxid required, +postgrex test-only, `Code.ensure_loaded?` guards; (2) the test harness is +real Postgres with the SQL sandbox, compose locally / service container in +CI, no skip tag. Consequences must state the standing operational cost +explicitly: from this record on, every `mix quality` run in this +repository requires a reachable Postgres server, by design - and what +would reopen each decision (a host that cannot take uxid transitively; a +second database the adapter must support). Update `docs/adr/README.md` +index. + +The whole harness lands here rather than incrementally with Phase 4, +deliberately: ADR-0005 is the epic-entry decision Phases 2-4 are written +against, the CI/compose wiring is the riskiest-to-debug piece so it fails +early on its own commit, and only a connectivity smoke test consumes it +until Phase 4 - an accepted, noted idle stretch. + +#### 2. ADR-0002 amendment +**File**: `docs/adr/0002-configurable-keys-and-table-names.md` +**Changes**: Dated amendment under decision 4 (house style: the +parenthesized amendment blocks ADR-0003 uses): V01's table set is +`charts`/`positions`/`runs` - the behaviour (ADR-0003 decision 3) keys +charts by content hash only and stores positions by session id, so the +`chart_versions` sketch collapses into the hash-keyed `charts` table and +`positions` joins the set under the same prefix knob; UXID row prefixes +become `chart_`/`pos_`/`run_` accordingly. A second, shorter parenthetical +under decision 3 records that tenancy columns remain unimplemented option +surface - deferred, not changed - so a reader of ADR-0002 alone learns the +promise is not yet delivered. + +#### 3. Dependencies +**File**: `mix.exs` +**Changes**: Add `{:uxid, "~> 2.0"}` (2.9.0 is current on Hex), +`{:ecto_sql, "~> 3.10", optional: true}`, `{:postgrex, "~> 0.19", only: +:test}`; replace the recorded open +question in the deps comment with a pointer to ADR-0005. + +#### 4. Harness +**Files**: `docker-compose.yml`, `config/config.exs`, `config/test.exs`, +`test/support/test_repo.ex`, `test/test_helper.exs`, `README.md` +**Changes**: Compose file with one `postgres:17` service (healthcheck, +host port from `PGPORT`, default 5432). `config/test.exs` configures +`StatifierPersistence.TestRepo` from `PG*` env vars with those defaults, +`pool: Ecto.Adapters.SQL.Sandbox`. `test_helper.exs` creates the database +if absent (`storage_up`), starts the repo, runs the package migrations +(Phase 4 extends this; in Phase 1 it only starts the repo), sets sandbox +`:manual`. README gains a short "Running the tests" note (compose command, +env vars). A smoke test asserts the repo answers `SELECT 1` so a missing +server fails loudly with a clear message, not obscurely; because the pool +is the SQL sandbox in `:manual` mode, the smoke test performs an explicit +`Ecto.Adapters.SQL.Sandbox.checkout(TestRepo)` in its setup. + +#### 5. CI +**File**: `.github/workflows/ci.yml` +**Changes**: Add a `postgres:17` service to the `gate` job (env +`POSTGRES_PASSWORD: postgres`, port 5432 mapped, `pg_isready` +healthcheck); export the matching `PG*` env at the job level. + +### Success Criteria: + +#### Automated Verification: +- [ ] Full quality gate passes locally (`mix quality`), including the new + smoke test against the compose Postgres +- [ ] `mix gate.verify` passes +- [ ] `mix deps.get` resolves uxid/ecto_sql/postgrex without conflicts + +#### Manual Verification: +- [ ] CI run on the pushed branch is green with the service container +- [ ] ADR-0005 reads as a decision record, not a plan restatement + +**Implementation Note**: Use the project's loop gate between edits while +iterating; run the full gate as the phase gate. In looped (`--loop`) +execution, Automated Verification gates advancement automatically (via +`/wurk:commit --auto`), and Manual Verification items are deferred and +surfaced once at the end. + +--- + +## Phase 2: The key-generator behaviour and its three implementations + +### Overview + +The pure layer: a behaviour fixing what a key scheme must answer, the +`:uxid`/`:uuid`/`:bigserial` implementations, and the resolver from +ADR-0002's option spellings to `{module, opts}`. No database. + +### Changes Required: + +#### 1. Behaviour and resolver +**File**: `lib/statifier_persistence/ecto/key_generator.ex` +**Changes**: `StatifierPersistence.Ecto.KeyGenerator` behaviour with three +callbacks the schemas and DDL both consume: + +```elixir +@callback ecto_type(opts :: keyword()) :: atom() +# schema field type: :string | Ecto.UUID | :id + +@callback migration_type(opts :: keyword()) :: atom() +# column type for DDL: :text | :uuid | :bigserial + +@callback autogenerate(table :: :charts | :positions | :runs, opts :: keyword()) :: + {module(), atom(), [term()]} | nil +# Ecto @primary_key autogenerate MFA; nil = database-assigned +``` + +plus `resolve(:uxid | :uuid | :bigserial | {module, opts}) :: +{module, keyword()}` mapping ADR-0002's spellings onto implementations +(`{Mod, opts}` passes through after a behaviour check). Wrapped in the +`Code.ensure_loaded?(Ecto)` guard only where Ecto types are referenced +(the behaviour itself is Ecto-free except `Ecto.UUID` in one +implementation). + +#### 2. Implementations +**Files**: `lib/statifier_persistence/ecto/key_generator/uxid.ex`, +`.../uuid_v7.ex`, `.../bigserial.ex` +**Changes**: UXID: `:string`/`:text`, autogenerate via the `uxid` package +with per-table prefixes `chart_`/`pos_`/`run_` (ADR-0002 amendment). +UUIDv7: `Ecto.UUID`/`:uuid`, RFC 9562 v7 generation implemented here +(48-bit ms timestamp, version 7, variant 2, random tail - ~15 lines; +Ecto.UUID is v4, and a dependency for one function is not worth taking), +encoded through `Ecto.UUID.load/1`. Bigserial: `:id`/`:bigserial`, +`autogenerate/2` returns nil. + +#### 3. Tests +**File**: `test/statifier_persistence/ecto/key_generator_test.exs` +**Changes**: Resolver mapping for all four spellings (including the +behaviour check refusing a non-implementing module); UXID keys carry the +right per-table prefix and are k-sortable in generation order; UUIDv7 keys +are valid UUID strings with version nibble 7 and variant bits 10, and sort +by generation time across a millisecond boundary; bigserial declares +database assignment. Sabotage each per repo convention. + +### Success Criteria: + +#### Automated Verification: +- [ ] Full quality gate passes (`mix quality`) +- [ ] `mix gate.verify` passes + +#### Manual Verification: +- [ ] Generated UXID/UUIDv7 examples eyeballed once for sanity + +**Implementation Note**: same loop/full-gate discipline as Phase 1. + +--- + +## Phase 3: `use StatifierPersistence.Ecto` - config and schemas on the host's module + +### Overview + +One shared config resolver, and the `use` macro that validates options at +compile time, exposes the resolved config, and defines the three schema +modules - the "same options" half of ADR-0002 decision 3's +schemas-and-DDL-cannot-disagree rule. + +### Changes Required: + +#### 1. Config resolver +**File**: `lib/statifier_persistence/ecto/config.ex` +**Changes**: `StatifierPersistence.Ecto.Config` struct + `new/1`: +validates `repo:` (required), `key:` (default `:uxid`, resolved via +`KeyGenerator.resolve/1`), `table_prefix:` (default `"statifier_"`), +`tables:` (per-table override map, keys `:charts | :positions | :runs`), +`prefix:` (Postgres schema, default nil). Rejects unknown options and +unknown table keys with a clear `ArgumentError` at compile time. Exposes +`table(config, :runs)` etc. This one module is what Phase 4's migrations +helper also consumes - the single definition site. + +#### 2. The macro +**File**: `lib/statifier_persistence/ecto.ex` +**Changes**: `__using__/1` builds the `Config` at compile time, defines +`__statifier_persistence__(:config | :repo)` on the host module, and +defines `Host.Chart`, `Host.Position`, `Host.Run` schema modules: +configured PK (type + autogenerate MFA from the key generator; +`autogenerate: false, read_after_writes: true` for database-assigned), +`@schema_prefix` from `prefix:`, source from `Config.table/2`, engine +identity columns verbatim (`content_hash`, `session_id`, `run_id` as +`:string`; `identity_blob`/`chart_blob`/`position_blob` as `:binary`; +`status`/`failure` as `:string`; `timestamps(type: :utc_datetime_usec)`). +Whole file inside the `Code.ensure_loaded?(Ecto)` guard. + +#### 3. Tests +**File**: `test/statifier_persistence/ecto_test.exs` (+ fixture host +modules under `test/support/`) +**Changes**: Zero-options-beyond-repo host gets UXID string PKs and +`statifier_*` sources (the acceptance criterion's "defaults work with zero +options beyond repo:"); a fully-overridden host (uuid key, custom prefix, +per-table override, Postgres schema) reflects every knob in +`__schema__(:source)`, `__schema__(:type, :id)`, `@schema_prefix`, and PK +autogeneration; a bigserial host declares db-assigned keys; invalid +options raise at compile time (asserted via `Code.compile_string/1` in the +test); the moduledoc's zero-config host example compiles as written +(asserted by compiling the snippet verbatim in a test, so it cannot drift +from the macro's real behavior). Sabotage per convention. + +### Success Criteria: + +#### Automated Verification: +- [ ] Full quality gate passes (`mix quality`) +- [ ] `mix gate.verify` passes +- [ ] The moduledoc's zero-config example compiles verbatim in a test + +#### Manual Verification: +- [ ] Schema module names and option spellings read as a host author would + expect (naming judgment, not machine-checkable) + +**Implementation Note**: same loop/full-gate discipline. Schemas are +exercised against live DDL in Phase 4; in this phase their metadata is the +test surface. + +--- + +## Phase 4: The versioned migrations helper, proven live + +### Overview + +`StatifierPersistence.Ecto.Migrations` in the Oban.Migration mold: V01 +DDL generated from the same `Config`, migrated up and down against real +Postgres under every key option, with the identity columns proven +identical across all of them. + +### Changes Required: + +#### 1. The helper +**Files**: `lib/statifier_persistence/ecto/migrations.ex`, +`lib/statifier_persistence/ecto/migrations/v01.ex` +**Changes**: `Migrations.up(opts)` / `down(opts)`: `for: HostModule` reads +the host's compiled `Config` (the no-drift path); alternatively the same +literal options `use` takes, funneled through `Config.new/1` - one +resolver, both doors. `version:` selects the target (only V01 exists; +the versioned shape is the deliverable). V01 creates, per `Config`: +`charts` (PK per key config; `content_hash` text NOT NULL UNIQUE; +`identity_blob`/`chart_blob` bytea NOT NULL; timestamps), +`positions` (`session_id` text NOT NULL UNIQUE; `content_hash` +text NOT NULL; `identity_blob`/`position_blob` bytea NOT NULL; +timestamps), `runs` (`run_id` text NOT NULL UNIQUE; `status` text +NOT NULL; `content_hash` text NOT NULL; `identity_blob` bytea NOT NULL; +`position_blob` bytea NULL; `failure` text NULL; `session_id` text NULL +per ADR-0002 decision 5; timestamps). All DDL honors `prefix:` (Postgres +schema) and the per-table overrides. `down` drops in reverse. + +#### 2. Live tests +**File**: `test/statifier_persistence/ecto/migrations_test.exs` +**Changes**: `async: false`. For each key option (`:uxid`, `:uuid`, +`:bigserial`), a fixture host with a distinct `table_prefix` +(`kx_uxid_` etc. - which also exercises the prefix knob): run +`Ecto.Migrator` up with a generated migration module calling the helper +with `for:`; insert a row through each generated schema and read it back - +PK generated per config (or db-assigned), engine identity strings verbatim +byte-for-byte; assert via `information_schema` that the +`content_hash`/`session_id`/`run_id` columns and their unique indexes are +**identical across all three key configs** (the "identity guard provably +independent of the configured key" acceptance criterion at the DDL level - +the guard itself lives in the facade, `storage.ex`, and never touches a +surrogate key by construction, ADR-0003 decision 2); duplicate +`content_hash`/`run_id` inserts violate the unique index; migrate down +leaves no `kx_*` tables. One config additionally exercises the literal- +options door and the `prefix:` Postgres-schema option. Sabotage per +convention (e.g. drop the unique index from V01 -> duplicate-insert test +red). + +#### 3. Test helper wiring +**File**: `test/test_helper.exs` +**Changes**: Migration tests manage their own DDL (up in `setup_all`, +down on exit) outside the sandbox: they switch the repo with +`Ecto.Adapters.SQL.Sandbox.mode(TestRepo, :auto)` for the duration of +`setup_all`/`on_exit` DDL and their own inserts (which is why the module +is `async: false`), restoring `:manual` afterward; the sandbox stays +`:manual` for everything else. + +### Success Criteria: + +#### Automated Verification: +- [ ] Full quality gate passes (`mix quality`) +- [ ] `mix gate.verify` passes + +#### Manual Verification: +- [ ] `psql \d` on the migrated default tables matches ADR-0002's sketch +- [ ] CI green on the pushed branch (service container exercised by the + live tests) + +**Implementation Note**: same loop/full-gate discipline. This phase closes +sp-02x's acceptance criteria: behaviour + macro + helper exist and agree +on options (one `Config`), identity guard provably independent of the key, +defaults work with zero options beyond `repo:`. + +--- + +## Testing Strategy + +### Unit Tests: +- Key generators: pure format/ordering properties per implementation; + resolver totality over ADR-0002's four spellings. +- Config/macro: compile-time validation and schema metadata under default, + fully-overridden, and db-assigned configurations. +- Migrations: live round trips per key config; cross-config identity-column + equality out of `information_schema`; unique-index enforcement; + up/down symmetry. +- Every test asserting `lib/` behavior is sabotaged (break, verify red, + revert, one-line note above the test) per repo convention. + +### Manual Testing Steps: +1. `docker compose up -d db && mix quality` from a clean checkout. +2. Inspect `\d statifier_runs` in psql after the migration tests, compare + against ADR-0002 decision 4/5. +3. Confirm the pushed branch's CI run is green. + +## References + +- Bead: `sp-02x` (parent epic `sp-4an.3`) +- ADRs: `docs/adr/0002-configurable-keys-and-table-names.md`, + `docs/adr/0003-storage-adapter-behaviour-and-the-identity-guard.md`, + `docs/adr/0004-run-lifecycle-executor-seam-and-serialization.md` +- Adapter contract: `lib/statifier_persistence/storage/adapter.ex:29-63` +- Conformance suite (consumer of Phase 1's harness in the next task): + `lib/statifier_persistence/testing/storage_conformance.ex` +- Precedent plan: + `docs/plans/260822-sp-4an.2.1-run-lifecycle-executor-seam-stepper.md` +- Open question this resolves: `mix.exs:52-59` (dependency shape) From 757e74b3afc979326d8aec5fe905f76a433f08bd Mon Sep 17 00:00:00 2001 From: JohnnyT Date: Sat, 22 Aug 2026 09:41:57 -0600 Subject: [PATCH 2/6] Adds ADR-0005 and the Postgres test harness Records the two epic-entry decisions as ADR-0005: the Ecto layer ships in this package behind optional ecto_sql (uxid required, postgrex test-only), and the test harness is real Postgres with the SQL sandbox - no skip tag when the server is absent. Amends ADR-0002: the V01 table set collapses to charts/positions/runs, and tenancy columns remain unimplemented option surface. Adds the harness itself: docker-compose.yml (postgres:17), TestRepo configured from PG* env vars, database creation and sandbox setup in test_helper.exs, a connectivity smoke test that fails loudly without a server, a Postgres service container in CI, and a "Running the tests" README note. Replaces mix.exs's recorded dependency-shape question with a pointer to ADR-0005. Refs: sp-02x --- .github/workflows/ci.yml | 24 +++++ README.md | 14 +++ config/config.exs | 9 ++ config/test.exs | 14 +++ docker-compose.yml | 12 +++ .../0002-configurable-keys-and-table-names.md | 24 ++++- ...to-in-package-and-postgres-test-harness.md | 89 +++++++++++++++++++ docs/adr/README.md | 1 + .../260822-sp-02x-keys-tables-migrations.md | 25 +++++- mix.exs | 12 +-- mix.lock | 6 ++ .../postgres_smoke_test.exs | 35 ++++++++ test/support/test_repo.ex | 15 ++++ test/test_helper.exs | 16 ++++ 14 files changed, 287 insertions(+), 9 deletions(-) create mode 100644 config/config.exs create mode 100644 config/test.exs create mode 100644 docker-compose.yml create mode 100644 docs/adr/0005-ecto-in-package-and-postgres-test-harness.md create mode 100644 test/statifier_persistence/postgres_smoke_test.exs create mode 100644 test/support/test_repo.ex diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 197d88e..2958e16 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -20,6 +20,30 @@ jobs: runs-on: ubuntu-latest timeout-minutes: 30 + # ADR-0005: database-backed tests are ordinary tests in the ordinary + # suite, against a real Postgres server - no tag skips them here either. + # Credentials match docker-compose.yml's local `db` service so the PG* + # env below is an override mechanism, not a setup step. + services: + postgres: + image: postgres:17 + env: + POSTGRES_PASSWORD: postgres + ports: + - 5432:5432 + options: >- + --health-cmd pg_isready + --health-interval 5s + --health-timeout 5s + --health-retries 10 + + env: + PGHOST: localhost + PGPORT: 5432 + PGUSER: postgres + PGPASSWORD: postgres + PGDATABASE: statifier_persistence_test + steps: - uses: actions/checkout@v4 diff --git a/README.md b/README.md index e7eb45a..c62af20 100644 --- a/README.md +++ b/README.md @@ -14,6 +14,20 @@ crash semantics. This package is that loop, packaged. Nothing is implemented yet. This repository holds the scaffold only. +## Running the tests + +The suite includes database-backed tests against a real Postgres server - +ADR-0005 rejects a skip tag for when one is absent, so `mix quality` and +`mix test` both need one reachable. Start it once with: + + docker compose up -d db + +which brings up `postgres:17` on `localhost:5432` with user/password +`postgres`. Override host, port, user, password, or database name with the +`PGHOST`, `PGPORT`, `PGUSER`, `PGPASSWORD`, and `PGDATABASE` env vars (see +`config/test.exs` for the defaults) if a server is already running +elsewhere. + ## The contract this package builds on The persisted-position story is already specified upstream, and this package diff --git a/config/config.exs b/config/config.exs new file mode 100644 index 0000000..acbcf32 --- /dev/null +++ b/config/config.exs @@ -0,0 +1,9 @@ +import Config + +# This package carries no runtime app-env configuration of its own (ADR-0002 +# decision 3: Ecto configuration is compile-time, on the host's module, never +# app env). The only thing config/ configures is the test harness's own repo, +# so only :test has an env-specific file to import. +if config_env() == :test do + import_config "test.exs" +end diff --git a/config/test.exs b/config/test.exs new file mode 100644 index 0000000..128c004 --- /dev/null +++ b/config/test.exs @@ -0,0 +1,14 @@ +import Config + +# ADR-0005: the test harness is a real Postgres server (docker compose +# locally, a service container in CI), reached through these PG* env vars so +# both environments configure the same repo without a mix.exs edit. Defaults +# match docker-compose.yml's `db` service. +config :statifier_persistence, StatifierPersistence.TestRepo, + hostname: System.get_env("PGHOST", "localhost"), + port: String.to_integer(System.get_env("PGPORT", "5432")), + username: System.get_env("PGUSER", "postgres"), + password: System.get_env("PGPASSWORD", "postgres"), + database: System.get_env("PGDATABASE", "statifier_persistence_test"), + pool: Ecto.Adapters.SQL.Sandbox, + pool_size: System.schedulers_online() * 2 diff --git a/docker-compose.yml b/docker-compose.yml new file mode 100644 index 0000000..e47f65a --- /dev/null +++ b/docker-compose.yml @@ -0,0 +1,12 @@ +services: + db: + image: postgres:17 + environment: + POSTGRES_PASSWORD: postgres + ports: + - "${PGPORT:-5432}:5432" + healthcheck: + test: ["CMD-SHELL", "pg_isready -U postgres"] + interval: 5s + timeout: 5s + retries: 10 diff --git a/docs/adr/0002-configurable-keys-and-table-names.md b/docs/adr/0002-configurable-keys-and-table-names.md index 0b6f520..5e8edb6 100644 --- a/docs/adr/0002-configurable-keys-and-table-names.md +++ b/docs/adr/0002-configurable-keys-and-table-names.md @@ -1,6 +1,8 @@ # ADR-0002: Storage keys and table names are host-configurable; engine identities are not -Status: accepted (2026-08-20) +Status: accepted (2026-08-20) - amended 2026-08-22 (sp-02x Phase 1: collapses +decision 4's table sketch to `charts`/`positions`/`runs` and records that +decision 3's tenancy columns remain unimplemented option surface) ## Context @@ -76,6 +78,13 @@ options, so DDL and schemas cannot disagree; it is the only supported way to create or upgrade the tables. Tenancy columns (host-supplied, per the charter) ride the same `use` so host configuration has one home. +*(Amended 2026-08-22, sp-02x Phase 1: tenancy columns remain unimplemented +option surface. No host has specified any yet, and inventing placeholder +columns ahead of a real one would be the unexercised contract ADR-0003's +Consequences warn against. This decision's promise stands - tenancy +columns ride the same `use` when a host needs them - but nothing ships +until then.)* + **4. Table names default to the full `statifier_` prefix:** `statifier_charts`, `statifier_chart_versions`, `statifier_runs`. Discoverability wins over brevity - someone meeting `statifier_runs` in a @@ -85,6 +94,19 @@ escape hatch for hosts with naming standards the prefix cannot satisfy; the Postgres-schema option covers hosts that isolate by schema instead of by name. +*(Amended 2026-08-22, sp-02x Phase 1: this sketch's table set collapses to +`charts`, `positions`, and `runs`. The storage-adapter behaviour (ADR-0003 +decision 3) keys a chart by content hash only and a position by the engine +session id; nothing in that contract exercises a logical-chart / +chart-versions split, so the `statifier_chart_versions` table above never +gets a callback that would use it - the same unexercised-contract +reasoning ADR-0003's Consequences apply to `delete_position`. V01 +therefore ships the hash-keyed `statifier_charts` table and joins +`statifier_positions` to the set under the same prefix knob, and the +default UXID row prefixes become `chart_`, `pos_`, and `run_` accordingly. +A logical-chart table returns, under whatever name fits then, when a real +embedder needs one - the charter's own design rule.)* + **5. The vocabulary is *runs*, not sessions.** The charter's lifecycle (`create/step/complete/fail`) operates on runs; "session" keeps the meaning statifier-ex gives it - the live GenServer runtime this package diff --git a/docs/adr/0005-ecto-in-package-and-postgres-test-harness.md b/docs/adr/0005-ecto-in-package-and-postgres-test-harness.md new file mode 100644 index 0000000..38ffea4 --- /dev/null +++ b/docs/adr/0005-ecto-in-package-and-postgres-test-harness.md @@ -0,0 +1,89 @@ +# ADR-0005: The Ecto layer ships in this package; the test harness is real Postgres + +Status: accepted (2026-08-22) + +## Context + +The Ecto adapter epic (sp-4an.3) opens on two questions its charter left +deliberately unanswered until the work started. `mix.exs` recorded the +first verbatim: does the Ecto layer live in this package behind an +optional dependency, or in a separate `statifier_ecto` package? The second +is what the epic's database-backed tests run against: a real Postgres +server, an embedded stand-in (SQLite), or an in-memory fake. + +The surrounding records constrain both answers. ADR-0002 decision 3 names +`use StatifierPersistence.Ecto` - this package's namespace - as where a +host configures keys and tables, and makes the versioned migrations helper +"the only supported way to create or upgrade the tables". ADR-0003 +decision 5 ships the conformance suite in this package's `lib/` precisely +so a downstream adapter can `use` it; ADR-0004's Consequences hand the +Ecto adapter three run callbacks and an optional `lock_run/3` to implement +as a transaction-scoped row lock (`SELECT ... FOR UPDATE`). Ecto already +has the pattern for a library that is optional to compile against: +`Code.ensure_loaded?/1` guards around the modules that reference it, and +the host brings its own database driver. + +On the harness side, the repository's own gate rules bind harder than +convenience: "never go green by weakening the check", explicitly including +`@tag :skip` on tests that cannot run. A database suite that silently +skips when no server is reachable is exactly that weakening - the gate +reports green while the adapter is untested. + +## Decision + +**1. The Ecto layer lives in this package, behind optional `ecto_sql`.** +No separate `statifier_ecto` package: it would split one contract +(behaviour + conformance suite here, the adapter that must pass them +there) across two repos, duplicate the test surface, and contradict +ADR-0002's already-accepted `use StatifierPersistence.Ecto` spelling, all +for no consumer benefit. Concretely: + +- `{:ecto_sql, "~> 3.10", optional: true}` - a host that only wants the + behaviour, the in-memory adapter, or the stepper loop compiles this + package without Ecto anywhere in its tree. +- Every module that references Ecto is wrapped in + `if Code.ensure_loaded?(Ecto)` so the package compiles clean either way. +- `{:uxid, "~> 2.0"}` is a required dependency: ADR-0002 decision 2 makes + `:uxid` the default key scheme and the default must work out of the box. +- `{:postgrex, "~> 0.19", only: :test}` - a host brings its own database + driver; this package needs one only to test itself. + +**2. The test harness is a real Postgres server with the SQL sandbox.** +Database-backed tests are ordinary tests in the ordinary suite, isolated +per test through `Ecto.Adapters.SQL.Sandbox` (the checkout the +conformance suite's optional `isolate/1` callback already anticipates). +The server comes from `docker compose up -d db` locally (postgres:17, +credentials and port overridable via `PG*` env vars) and a `postgres:17` +service container in CI. Rejected alternatives: + +- SQLite or any embedded stand-in: `lock_run/3` is specified as a + transaction-scoped row lock, and `SELECT ... FOR UPDATE` semantics are + exactly what an embedded engine fakes differently or not at all. A + migration suite proven against a database no host will run proves + little. +- An in-memory fake: the conformance suite exists to test adapters + against real backends; running it against a fake of the backend is + circular. +- A skip tag for when the server is absent: an auto-skipped database + suite is the gate weakening this repository's rules forbid. Absent + server, red suite, loudly. + +## Consequences + +- From this record on, every `mix quality` run in this repository + requires a reachable Postgres server, by design. `docker compose up -d + db` is now part of standing up a working checkout, and the README says + so. A run without the server fails loudly in the connectivity smoke + test rather than skipping quietly. +- CI carries a Postgres service container from here forward; its + credentials and the local compose defaults are the same values, so the + `PG*` env vars are an override mechanism, not a setup step. +- Hosts compiling without `ecto_sql` get no Ecto modules and no + migrations helper - the `Code.ensure_loaded?` guard is the seam, and a + missing-guard compile failure in such a host is a bug in this package. +- `uxid` becomes a transitive dependency of every host, ecto or not. +- What would reopen this record: a host that cannot take `uxid` + transitively (decision 1's required-dependency clause); a second + database the adapter must support (decision 2's Postgres-only harness); + or the optional-dependency seam failing in practice - a host without + `ecto_sql` that this package will not compile for. diff --git a/docs/adr/README.md b/docs/adr/README.md index bcf1ae7..92a51df 100644 --- a/docs/adr/README.md +++ b/docs/adr/README.md @@ -6,6 +6,7 @@ | [0002](0002-configurable-keys-and-table-names.md) | Storage keys and table names are host-configurable at compile time (UXID default, `statifier_` prefix, runs vocabulary); engine identities are not | accepted | | [0003](0003-storage-adapter-behaviour-and-the-identity-guard.md) | The storage adapter stores opaque blobs keyed by engine identities; the identity guard lives above every adapter and cannot be skipped | accepted | | [0004](0004-run-lifecycle-executor-seam-and-serialization.md) | The run record owns its position, the loop's order is the contract, effects cross a host-executor seam (failures re-enter as `error.communication`), and per-run serialization is a pluggable strategy | accepted | +| [0005](0005-ecto-in-package-and-postgres-test-harness.md) | The Ecto layer ships in this package behind optional `ecto_sql`; the test harness is a real Postgres server with the SQL sandbox, no skip tag | accepted | New ADRs: next number, same three-section format (Context, Decision, Consequences). Pick the number against a freshly fetched remote. A bare diff --git a/docs/plans/260822-sp-02x-keys-tables-migrations.md b/docs/plans/260822-sp-02x-keys-tables-migrations.md index 7c5e096..4037f3a 100644 --- a/docs/plans/260822-sp-02x-keys-tables-migrations.md +++ b/docs/plans/260822-sp-02x-keys-tables-migrations.md @@ -204,10 +204,10 @@ healthcheck); export the matching `PG*` env at the job level. ### Success Criteria: #### Automated Verification: -- [ ] Full quality gate passes locally (`mix quality`), including the new +- [x] Full quality gate passes locally (`mix quality`), including the new smoke test against the compose Postgres -- [ ] `mix gate.verify` passes -- [ ] `mix deps.get` resolves uxid/ecto_sql/postgrex without conflicts +- [x] `mix gate.verify` passes +- [x] `mix deps.get` resolves uxid/ecto_sql/postgrex without conflicts #### Manual Verification: - [ ] CI run on the pushed branch is green with the service container @@ -461,3 +461,22 @@ defaults work with zero options beyond `repo:`. - Precedent plan: `docs/plans/260822-sp-4an.2.1-run-lifecycle-executor-seam-stepper.md` - Open question this resolves: `mix.exs:52-59` (dependency shape) + +## Deferred Manual Verification + +Manual verification items are deferred during looped (--loop) execution and +surfaced here once, rather than blocking after each phase. Confirm these +before considering the plan fully landed. + +### Phase 1 + +- [ ] CI run on the pushed branch is green with the service container +- [ ] ADR-0005 reads as a decision record, not a plan restatement + +**Implementation Note**: Use the project's loop gate between edits while +iterating; run the full gate as the phase gate. In looped (`--loop`) +execution, Automated Verification gates advancement automatically (via +`/wurk:commit --auto`), and Manual Verification items are deferred and +surfaced once at the end. + +--- diff --git a/mix.exs b/mix.exs index d4f8633..0da7aed 100644 --- a/mix.exs +++ b/mix.exs @@ -37,13 +37,16 @@ defmodule StatifierPersistence.MixProject do defp deps do [ statifier_dep(), + {:uxid, "~> 2.0"}, + {:ecto_sql, "~> 3.10", optional: true}, # Dev / test {:ex_quality, "~> 0.13", only: :dev, runtime: false}, {:credo, "~> 1.7", only: [:dev, :test], runtime: false}, {:dialyxir, "~> 1.4", only: [:dev, :test], runtime: false}, {:excoveralls, "~> 0.18", only: :test}, - {:ex_doc, "~> 0.34", only: :dev, runtime: false} + {:ex_doc, "~> 0.34", only: :dev, runtime: false}, + {:postgrex, "~> 0.19", only: :test} ] end @@ -52,10 +55,9 @@ defmodule StatifierPersistence.MixProject do # publish a package that carries a git dependency, so this package cannot # ship until statifier is published. That is upstream's call, not ours. # - # Ecto is deliberately absent: the storage-adapter behaviour, run lifecycle, - # and stepper loop need none of it. The Ecto adapter epic decides its own - # dependency shape (ecto_sql optional vs a separate statifier_ecto package) - # when that work starts. + # The Ecto layer's dependency shape (ecto_sql optional here vs a separate + # statifier_ecto package, uxid required, postgrex test-only) is decided in + # ADR-0005 - see docs/adr/0005-ecto-in-package-and-postgres-test-harness.md. # # Export STATIFIER_PATH to point at a local checkout while co-developing a # change that spans both repos. It is an env var rather than a mix.exs edit diff --git a/mix.lock b/mix.lock index 344eb7b..de37431 100644 --- a/mix.lock +++ b/mix.lock @@ -1,8 +1,12 @@ %{ "bunt": {:hex, :bunt, "1.0.0", "081c2c665f086849e6d57900292b3a161727ab40431219529f13c4ddcf3e7a44", [:mix], [], "hexpm", "dc5f86aa08a5f6fa6b8096f0735c4e76d54ae5c9fa2c143e5a1fc7c1cd9bb6b5"}, "credo": {:hex, :credo, "1.7.19", "cc52129665fc7c15143d47838fda0f9cd6dac9ceced7bf4da6f85fcbfe64b12a", [:mix], [{:bunt, "~> 0.2.1 or ~> 1.0", [hex: :bunt, repo: "hexpm", optional: false]}, {:file_system, "~> 0.2 or ~> 1.0", [hex: :file_system, repo: "hexpm", optional: false]}, {:jason, "~> 1.0", [hex: :jason, repo: "hexpm", optional: false]}], "hexpm", "2d8bc95d5a7bb99dd2613621d4f08c6a3575c3fd4b62e6a2b48a100352a557b8"}, + "db_connection": {:hex, :db_connection, "2.10.2", "ae391e803a5adff104da913c2fc1c0c14a37f8b10001dcef568796e1fb7bf95c", [:mix], [{:telemetry, "~> 0.4 or ~> 1.0", [hex: :telemetry, repo: "hexpm", optional: false]}], "hexpm", "510b14482330f1af6490a2fa0efd8d4f1435d1529b165647df22ac0f2df0fa93"}, + "decimal": {:hex, :decimal, "3.1.1", "430d87b04011ce6cbd4fd205be758311a81f87d552d40904abd00f015935b1d0", [:mix], [], "hexpm", "c5f25f2ced74a0587d03e6023f595db8e924c9d3922c8c8ffd9edfc4498cf1f6"}, "dialyxir": {:hex, :dialyxir, "1.4.7", "dda948fcee52962e4b6c5b4b16b2d8fa7d50d8645bbae8b8685c3f9ecb7f5f4d", [:mix], [{:erlex, ">= 0.2.8", [hex: :erlex, repo: "hexpm", optional: false]}], "hexpm", "b34527202e6eb8cee198efec110996c25c5898f43a4094df157f8d28f27d9efe"}, "earmark_parser": {:hex, :earmark_parser, "1.4.46", "67607a0532e810c6f630a515c548d0b24949643f168cc556303bee4cf96105c7", [:mix], [], "hexpm", "9c44636e8a1c68c62f526b2dcd85d941dbbcee7ab82cf64ba06ce28bef8e89f5"}, + "ecto": {:hex, :ecto, "3.14.2", "99db28a864293a789c970651de711e3cae184291e0e7ea1166c54055ac41c1f3", [:mix], [{:decimal, "~> 3.0", [hex: :decimal, repo: "hexpm", optional: false]}, {:jason, "~> 1.0", [hex: :jason, repo: "hexpm", optional: true]}, {:telemetry, "~> 0.4 or ~> 1.0", [hex: :telemetry, repo: "hexpm", optional: false]}], "hexpm", "25d60b8c816a07d19d85b80bdf60978bd8b102209dda198d768cd7c6745339a6"}, + "ecto_sql": {:hex, :ecto_sql, "3.14.0", "06446ab8410d2f85bfbb80857ee224ab3b693700cbb38f6535d507449a627b2e", [:mix], [{:db_connection, "~> 2.9", [hex: :db_connection, repo: "hexpm", optional: false]}, {:decimal, "~> 3.0", [hex: :decimal, repo: "hexpm", optional: false]}, {:ecto, "~> 3.14.0", [hex: :ecto, repo: "hexpm", optional: false]}, {:myxql, "~> 0.8", [hex: :myxql, repo: "hexpm", optional: true]}, {:postgrex, "~> 0.19 or ~> 1.0", [hex: :postgrex, repo: "hexpm", optional: true]}, {:tds, "~> 2.1.1 or ~> 2.2", [hex: :tds, repo: "hexpm", optional: true]}, {:telemetry, "~> 0.4.0 or ~> 1.0", [hex: :telemetry, repo: "hexpm", optional: false]}], "hexpm", "f4d8d36faf294c9417b5a37ec7ac8217ee2abdef5fcf197ba690f361548d3949"}, "erlex": {:hex, :erlex, "0.2.9", "7debbbaa9f4f368b8cd648983e0f1d7963028508e9c59e9d4ed504e94ef52a55", [:mix], [], "hexpm", "8cfffc0ec7159e6d73de2ab28a588064de80f88b2798d5cbe4482cbbc200178b"}, "ex_doc": {:hex, :ex_doc, "0.40.3", "4a972ffe64bc07dc605af487e98fc19b72a4185f55ca031b94c0552d6071c1d9", [:mix], [{:earmark_parser, "~> 1.4.44", [hex: :earmark_parser, repo: "hexpm", optional: false]}, {:makeup_c, ">= 0.1.0", [hex: :makeup_c, repo: "hexpm", optional: true]}, {:makeup_elixir, "~> 0.14 or ~> 1.0", [hex: :makeup_elixir, repo: "hexpm", optional: false]}, {:makeup_erlang, "~> 0.1 or ~> 1.0", [hex: :makeup_erlang, repo: "hexpm", optional: false]}, {:makeup_html, ">= 0.1.0", [hex: :makeup_html, repo: "hexpm", optional: true]}], "hexpm", "2756e357742fecd9749b489b85d67c9ce99c465f2e75728d9e6dc8d704b973de"}, "ex_quality": {:hex, :ex_quality, "0.13.0", "dbe2dc02d40d7c6007d1808e677fbef34db1d1370f4814d10c6b8e46bae4cc43", [:mix], [{:jason, "~> 1.4", [hex: :jason, repo: "hexpm", optional: false]}], "hexpm", "8dfc1df8b2fe5dcfff6073cd60303a09a2745a47e6b91143db32e72949dab0d2"}, @@ -13,8 +17,10 @@ "makeup_elixir": {:hex, :makeup_elixir, "1.0.1", "e928a4f984e795e41e3abd27bfc09f51db16ab8ba1aebdba2b3a575437efafc2", [:mix], [{:makeup, "~> 1.0", [hex: :makeup, repo: "hexpm", optional: false]}, {:nimble_parsec, "~> 1.2.3 or ~> 1.3", [hex: :nimble_parsec, repo: "hexpm", optional: false]}], "hexpm", "7284900d412a3e5cfd97fdaed4f5ed389b8f2b4cb49efc0eb3bd10e2febf9507"}, "makeup_erlang": {:hex, :makeup_erlang, "1.1.0", "835f7e60792e08824cda445639555d7bf1bbbddb1b60b306e33cb6f6db24dc74", [:mix], [{:makeup, "~> 1.0", [hex: :makeup, repo: "hexpm", optional: false]}], "hexpm", "1cd6780fb1dd1a03979abaed0fe82712b0625118fd5257d3ebbf73f960c73c3c"}, "nimble_parsec": {:hex, :nimble_parsec, "1.4.2", "8efba0122db06df95bfaa78f791344a89352ba04baedd3849593bfce4d0dc1c6", [:mix], [], "hexpm", "4b21398942dda052b403bbe1da991ccd03a053668d147d53fb8c4e0efe09c973"}, + "postgrex": {:hex, :postgrex, "0.22.4", "d271f595dfd25230b6398354e19d17bb5e2d20130fd2d9bdca7e15f125d43552", [:mix], [{:db_connection, "~> 2.9", [hex: :db_connection, repo: "hexpm", optional: false]}, {:decimal, "~> 1.5 or ~> 2.0 or ~> 3.0", [hex: :decimal, repo: "hexpm", optional: false]}, {:jason, "~> 1.0", [hex: :jason, repo: "hexpm", optional: true]}, {:table, "~> 0.1.0", [hex: :table, repo: "hexpm", optional: true]}], "hexpm", "4aae45a2d60e35b04eea2602440be152fae332901f1fc7a60fc7cb7f0f9a9c5a"}, "predicator": {:hex, :predicator, "9.0.0", "c15685ce74e249195df9bbaedf7593c50339ac75b8700a3b13f56cfbcc837ff7", [:mix], [], "hexpm", "8e31739e3d448f9f5d91eb716498a0dfe26f3b27c8bcf96bc6b2ce93c0087e95"}, "saxy": {:hex, :saxy, "1.6.1", "742eff28f553c066d0b54e84662dbf384a1d1f38595472ed15f6e0a33038bbe1", [:mix], [], "hexpm", "8989d504424ba29460a61950f8968380651413fa05e63b6118084db057da1a6b"}, "statifier": {:git, "https://github.com/riddler/statifier-ex.git", "68b814aad5ae0875881a4af05eefbaa81846a7ea", [branch: "main"]}, "telemetry": {:hex, :telemetry, "1.4.2", "a0cb522801dffb1c49fe6e30561badffc7b6d0e180db1300df759faa22062855", [:rebar3], [], "hexpm", "928f6495066506077862c0d1646609eed891a4326bee3126ba54b60af61febb1"}, + "uxid": {:hex, :uxid, "2.9.0", "e61508fa4d0f997995fd3958d0033acceb70f23e5ec85ea3144485b8b9683c61", [:mix], [{:ecto, "~> 3.12", [hex: :ecto, repo: "hexpm", optional: true]}], "hexpm", "af4a43db3e9f25b7b182eb1870c712224e9a205f39676d98e6cfaaf3fe6029c1"}, } diff --git a/test/statifier_persistence/postgres_smoke_test.exs b/test/statifier_persistence/postgres_smoke_test.exs new file mode 100644 index 0000000..a3c2beb --- /dev/null +++ b/test/statifier_persistence/postgres_smoke_test.exs @@ -0,0 +1,35 @@ +defmodule StatifierPersistence.PostgresSmokeTest do + @moduledoc """ + Connectivity smoke test for ADR-0005's Postgres harness. + + This asserts infrastructure (a reachable database), not `lib/` behavior, + so it carries no sabotage note per the repo's convention - there is no + application code here to break and revert. + """ + + use ExUnit.Case, async: true + + alias Ecto.Adapters.SQL.Sandbox + alias StatifierPersistence.TestRepo + + setup do + case Sandbox.checkout(TestRepo) do + :ok -> + :ok + + {:error, reason} -> + flunk(""" + Could not check out a connection to the Postgres test database \ + (#{inspect(reason)}). + + Start it with `docker compose up -d db` (see README.md, "Running \ + the tests"), or point PGHOST/PGPORT/PGUSER/PGPASSWORD/PGDATABASE \ + at an existing server. + """) + end + end + + test "the repo answers SELECT 1" do + assert {:ok, %{rows: [[1]]}} = TestRepo.query("SELECT 1") + end +end diff --git a/test/support/test_repo.ex b/test/support/test_repo.ex new file mode 100644 index 0000000..8ebbc0a --- /dev/null +++ b/test/support/test_repo.ex @@ -0,0 +1,15 @@ +defmodule StatifierPersistence.TestRepo do + @moduledoc """ + The Ecto repo backing this package's own test suite. + + ADR-0005: database-backed tests run against a real Postgres server rather + than a fake or an embedded stand-in, isolated per test through + `Ecto.Adapters.SQL.Sandbox`. This module is test-only support code, not + part of the package's public API - a host configures its own repo through + `use StatifierPersistence.Ecto, repo: MyApp.Repo` (ADR-0002 decision 3). + """ + + use Ecto.Repo, + otp_app: :statifier_persistence, + adapter: Ecto.Adapters.Postgres +end diff --git a/test/test_helper.exs b/test/test_helper.exs index 869559e..a692016 100644 --- a/test/test_helper.exs +++ b/test/test_helper.exs @@ -1 +1,17 @@ +# ADR-0005: database-backed tests are ordinary tests in the ordinary suite, +# against a real Postgres server - no tag skips them when the server is +# absent. Create the test database if it does not exist yet, start the +# repo, and put the SQL sandbox in :manual mode so each test checks out its +# own connection. +{:ok, _} = Application.ensure_all_started(:postgrex) + +case Ecto.Adapters.Postgres.storage_up(StatifierPersistence.TestRepo.config()) do + :ok -> :ok + {:error, :already_up} -> :ok +end + +{:ok, _pid} = StatifierPersistence.TestRepo.start_link() + +Ecto.Adapters.SQL.Sandbox.mode(StatifierPersistence.TestRepo, :manual) + ExUnit.start() From 2bc484ed1a3e66cd2f956c3bde2180b569c21e86 Mon Sep 17 00:00:00 2001 From: JohnnyT Date: Sat, 22 Aug 2026 09:49:33 -0600 Subject: [PATCH 3/6] Adds the key-generator behaviour and schemes Adds StatifierPersistence.Ecto.KeyGenerator: the behaviour a key scheme implements (ecto_type/1, migration_type/1, autogenerate/2) plus resolve/1 mapping ADR-0002's spellings - :uxid, :uuid, :bigserial, {module, opts} - onto implementations, refusing modules that do not implement the behaviour. Ships the three implementations: UXID (string/text, per-table prefixes chart_/pos_/run_), UUIDv7 (RFC 9562 v7 generated locally, Ecto.UUID column type, guarded so the package compiles without ecto_sql), and Bigserial (database-assigned). Pure layer, no database; live DDL follows with the migrations helper. Refs: sp-02x --- .../260822-sp-02x-keys-tables-migrations.md | 12 +- .../ecto/key_generator.ex | 87 ++++++++++++++ .../ecto/key_generator/bigserial.ex | 21 ++++ .../ecto/key_generator/uuid_v7.ex | 38 ++++++ .../ecto/key_generator/uxid.ex | 32 +++++ .../ecto/key_generator_test.exs | 109 ++++++++++++++++++ 6 files changed, 297 insertions(+), 2 deletions(-) create mode 100644 lib/statifier_persistence/ecto/key_generator.ex create mode 100644 lib/statifier_persistence/ecto/key_generator/bigserial.ex create mode 100644 lib/statifier_persistence/ecto/key_generator/uuid_v7.ex create mode 100644 lib/statifier_persistence/ecto/key_generator/uxid.ex create mode 100644 test/statifier_persistence/ecto/key_generator_test.exs diff --git a/docs/plans/260822-sp-02x-keys-tables-migrations.md b/docs/plans/260822-sp-02x-keys-tables-migrations.md index 4037f3a..88b904d 100644 --- a/docs/plans/260822-sp-02x-keys-tables-migrations.md +++ b/docs/plans/260822-sp-02x-keys-tables-migrations.md @@ -278,8 +278,8 @@ database assignment. Sabotage each per repo convention. ### Success Criteria: #### Automated Verification: -- [ ] Full quality gate passes (`mix quality`) -- [ ] `mix gate.verify` passes +- [x] Full quality gate passes (`mix quality`) +- [x] `mix gate.verify` passes #### Manual Verification: - [ ] Generated UXID/UUIDv7 examples eyeballed once for sanity @@ -480,3 +480,11 @@ execution, Automated Verification gates advancement automatically (via surfaced once at the end. --- + +### Phase 2 + +- [ ] Generated UXID/UUIDv7 examples eyeballed once for sanity + +**Implementation Note**: same loop/full-gate discipline as Phase 1. + +--- diff --git a/lib/statifier_persistence/ecto/key_generator.ex b/lib/statifier_persistence/ecto/key_generator.ex new file mode 100644 index 0000000..d51af7d --- /dev/null +++ b/lib/statifier_persistence/ecto/key_generator.ex @@ -0,0 +1,87 @@ +defmodule StatifierPersistence.Ecto.KeyGenerator do + @moduledoc """ + Behaviour a surrogate-key scheme implements for the Ecto layer. + + ADR-0002 makes the surrogate primary keys of this package's tables + compile-time configurable per host: `:uxid` (the default), `:uuid` + (UUIDv7), `:bigserial`, or any `{module, opts}` implementing this + behaviour. A key scheme must answer three questions - the Ecto schema + field type, the migration column type, and how a key is generated (or + that the database assigns it) - and both the generated schemas and the + migrations helper read their answers from the same resolved generator, + so the two cannot disagree. + + Engine identities (the chart content hash, the engine session id, the + caller's run id) are stored verbatim and are not touched by any key + generator - ADR-0002 decision 1. + + The bundled implementations: + + * `StatifierPersistence.Ecto.KeyGenerator.UXID` - k-sortable strings + with per-table prefixes (`chart_`, `pos_`, `run_`) + * `StatifierPersistence.Ecto.KeyGenerator.UUIDv7` - RFC 9562 UUIDv7 + * `StatifierPersistence.Ecto.KeyGenerator.Bigserial` - + database-assigned auto-increment + """ + + @typedoc "The tables whose rows carry a generated surrogate key." + @type table :: :charts | :positions | :runs + + @typedoc "ADR-0002's key option spellings." + @type spelling :: :uxid | :uuid | :bigserial | {module(), keyword()} + + @doc """ + The Ecto schema field type for the primary key, e.g. `:string`, + `Ecto.UUID`, or `:id`. + """ + @callback ecto_type(opts :: keyword()) :: atom() + + @doc """ + The column type the migrations helper emits for the primary key, e.g. + `:text`, `:uuid`, or `:bigserial`. + """ + @callback migration_type(opts :: keyword()) :: atom() + + @doc """ + The `{module, function, args}` an Ecto schema uses to autogenerate a + primary key for a row of `table`, or `nil` when the database assigns + the key itself. + """ + @callback autogenerate(table(), opts :: keyword()) :: + {module(), atom(), [term()]} | nil + + @doc """ + Resolves one of ADR-0002's key option spellings to `{module, opts}`. + + `:uxid`, `:uuid`, and `:bigserial` map onto the bundled + implementations. A `{module, opts}` tuple passes through after a check + that `module` declares this behaviour; anything else raises + `ArgumentError`. + """ + @spec resolve(spelling()) :: {module(), keyword()} + def resolve(:uxid), do: {StatifierPersistence.Ecto.KeyGenerator.UXID, []} + def resolve(:uuid), do: {StatifierPersistence.Ecto.KeyGenerator.UUIDv7, []} + def resolve(:bigserial), do: {StatifierPersistence.Ecto.KeyGenerator.Bigserial, []} + + def resolve({module, opts}) when is_atom(module) and is_list(opts) do + if implements_behaviour?(module) do + {module, opts} + else + raise ArgumentError, + "#{inspect(module)} does not implement the " <> + "StatifierPersistence.Ecto.KeyGenerator behaviour" + end + end + + def resolve(other) do + raise ArgumentError, + "unknown key option #{inspect(other)}; expected :uxid, :uuid, " <> + ":bigserial, or {module, opts} implementing " <> + "StatifierPersistence.Ecto.KeyGenerator" + end + + defp implements_behaviour?(module) do + Code.ensure_loaded?(module) and + __MODULE__ in List.flatten(Keyword.get_values(module.module_info(:attributes), :behaviour)) + end +end diff --git a/lib/statifier_persistence/ecto/key_generator/bigserial.ex b/lib/statifier_persistence/ecto/key_generator/bigserial.ex new file mode 100644 index 0000000..e18aa96 --- /dev/null +++ b/lib/statifier_persistence/ecto/key_generator/bigserial.ex @@ -0,0 +1,21 @@ +defmodule StatifierPersistence.Ecto.KeyGenerator.Bigserial do + @moduledoc """ + Database-assigned auto-increment keys (ADR-0002 decision 2). + + Supported for hosts with that convention, not defaulted: a sequential + id leaks row counts if exposed, and that is the host's trade to make. + Schema fields are `:id` over `:bigserial` columns; `autogenerate/2` + returns `nil` because the database assigns the key. + """ + + @behaviour StatifierPersistence.Ecto.KeyGenerator + + @impl true + def ecto_type(_opts), do: :id + + @impl true + def migration_type(_opts), do: :bigserial + + @impl true + def autogenerate(_table, _opts), do: nil +end diff --git a/lib/statifier_persistence/ecto/key_generator/uuid_v7.ex b/lib/statifier_persistence/ecto/key_generator/uuid_v7.ex new file mode 100644 index 0000000..375c472 --- /dev/null +++ b/lib/statifier_persistence/ecto/key_generator/uuid_v7.ex @@ -0,0 +1,38 @@ +if Code.ensure_loaded?(Ecto) do + defmodule StatifierPersistence.Ecto.KeyGenerator.UUIDv7 do + @moduledoc """ + RFC 9562 UUIDv7 keys (ADR-0002 decision 2). + + `:uuid` keys are generated as UUIDv7, not v4: random keys fragment + b-tree indexes, and v7's 48-bit millisecond timestamp keeps insertion + locality. Generation is implemented here - `Ecto.UUID` generates v4, + and a dependency for one function is not worth taking. Schema fields + are `Ecto.UUID` over `:uuid` columns. + """ + + @behaviour StatifierPersistence.Ecto.KeyGenerator + + @impl true + def ecto_type(_opts), do: Ecto.UUID + + @impl true + def migration_type(_opts), do: :uuid + + @impl true + def autogenerate(_table, _opts), do: {__MODULE__, :generate, []} + + @doc """ + Generates an RFC 9562 UUIDv7 as a canonical hyphenated string. + + Layout: 48-bit Unix millisecond timestamp, 4-bit version (7), 12 + random bits, 2-bit variant (`10`), 62 random bits. + """ + @spec generate() :: Ecto.UUID.t() + def generate do + <> = :crypto.strong_rand_bytes(10) + raw = <> + {:ok, encoded} = Ecto.UUID.load(raw) + encoded + end + end +end diff --git a/lib/statifier_persistence/ecto/key_generator/uxid.ex b/lib/statifier_persistence/ecto/key_generator/uxid.ex new file mode 100644 index 0000000..61f796c --- /dev/null +++ b/lib/statifier_persistence/ecto/key_generator/uxid.ex @@ -0,0 +1,32 @@ +defmodule StatifierPersistence.Ecto.KeyGenerator.UXID do + @moduledoc """ + The default key generator: k-sortable UXID strings (ADR-0002 decision 2). + + Keys are generated by the `uxid` package and carry per-table prefixes in + the engine's own style - `chart_`, `pos_`, `run_` (ADR-0002 decision 4, + as amended) - so a key met in a psql console or a log line names its + table. Stored as `:string` fields over `:text` columns. + + Options given to the generator (via `{module, opts}` or the resolved + default) are passed through to `UXID.generate!/1`; the per-table + `:prefix` is supplied unless the options already carry one. + """ + + @behaviour StatifierPersistence.Ecto.KeyGenerator + + # UXID's default delimiter is "_", so a "chart" prefix yields "chart_...". + @prefixes %{charts: "chart", positions: "pos", runs: "run"} + + @impl true + def ecto_type(_opts), do: :string + + @impl true + def migration_type(_opts), do: :text + + @impl true + def autogenerate(table, opts) do + # UXID here is the uxid package's module, not this one - a flat + # defmodule creates no alias for its last segment. + {UXID, :generate!, [Keyword.put_new(opts, :prefix, Map.fetch!(@prefixes, table))]} + end +end diff --git a/test/statifier_persistence/ecto/key_generator_test.exs b/test/statifier_persistence/ecto/key_generator_test.exs new file mode 100644 index 0000000..186bdce --- /dev/null +++ b/test/statifier_persistence/ecto/key_generator_test.exs @@ -0,0 +1,109 @@ +defmodule StatifierPersistence.Ecto.KeyGeneratorTest do + use ExUnit.Case, async: true + + alias StatifierPersistence.Ecto.KeyGenerator + + defmodule NotAGenerator do + @moduledoc false + end + + describe "resolve/1" do + # sabotage: resolve(:uuid) returns the UXID module -> red (match on UUIDv7) + test "maps ADR-0002's atom spellings onto the bundled implementations" do + assert {KeyGenerator.UXID, []} = KeyGenerator.resolve(:uxid) + assert {KeyGenerator.UUIDv7, []} = KeyGenerator.resolve(:uuid) + assert {KeyGenerator.Bigserial, []} = KeyGenerator.resolve(:bigserial) + end + + # sabotage: resolve({module, opts}) returns {module, []} -> red (opts dropped) + test "passes {module, opts} through when the module implements the behaviour" do + assert {KeyGenerator.UXID, [rand_size: 5]} = + KeyGenerator.resolve({KeyGenerator.UXID, [rand_size: 5]}) + end + + # sabotage: implements_behaviour? check short-circuited to true -> red (nothing raised) + test "refuses a module that does not implement the behaviour" do + assert_raise ArgumentError, ~r/does not implement the .*KeyGenerator behaviour/, fn -> + KeyGenerator.resolve({NotAGenerator, []}) + end + end + + # sabotage: catch-all resolve/1 returns {UXID, []} instead of raising -> red (nothing raised) + test "refuses an unknown spelling" do + assert_raise ArgumentError, ~r/expected :uxid, :uuid, :bigserial/, fn -> + KeyGenerator.resolve(:ulid) + end + end + end + + describe "UXID" do + # sabotage: UXID.ecto_type/1 returns :binary_id -> red (match on :string) + test "declares :string schema fields over :text columns" do + assert :string = KeyGenerator.UXID.ecto_type([]) + assert :text = KeyGenerator.UXID.migration_type([]) + end + + # sabotage: @prefixes positions entry changed to "position" -> red (expects "pos_") + test "keys carry the per-table prefix" do + for {table, prefix} <- [charts: "chart_", positions: "pos_", runs: "run_"] do + {mod, fun, args} = KeyGenerator.UXID.autogenerate(table, []) + key = apply(mod, fun, args) + assert String.starts_with?(key, prefix) + end + end + + # sabotage: autogenerate/2 forces from: "fixed" (deterministic UXIDs) -> red (first == second) + test "keys are k-sortable in generation order" do + {mod, fun, args} = KeyGenerator.UXID.autogenerate(:runs, []) + first = apply(mod, fun, args) + # UXID timestamps have millisecond resolution; cross a boundary. + Process.sleep(2) + second = apply(mod, fun, args) + assert first < second + end + end + + describe "UUIDv7" do + # sabotage: UUIDv7.migration_type/1 returns :binary -> red (match on :uuid) + test "declares Ecto.UUID schema fields over :uuid columns" do + assert Ecto.UUID = KeyGenerator.UUIDv7.ecto_type([]) + assert :uuid = KeyGenerator.UUIDv7.migration_type([]) + end + + # sabotage: generate/0 writes version nibble 4 instead of 7 -> red (version match fails) + test "generates valid UUID strings with version 7 and variant 10" do + {mod, fun, args} = KeyGenerator.UUIDv7.autogenerate(:charts, []) + uuid = apply(mod, fun, args) + + assert {:ok, _raw} = Ecto.UUID.dump(uuid) + + assert <<_::binary-size(14), "7", _::binary-size(3), "-", variant::binary-size(1), + _::binary>> = uuid + + assert variant in ~w(8 9 a b) + end + + # sabotage: generate/0 inverts the 48-bit timestamp -> red (first > second) + test "keys sort by generation time across a millisecond boundary" do + first = KeyGenerator.UUIDv7.generate() + Process.sleep(2) + second = KeyGenerator.UUIDv7.generate() + assert first < second + end + end + + describe "Bigserial" do + # sabotage: Bigserial.ecto_type/1 returns :integer -> red (match on :id) + test "declares :id schema fields over :bigserial columns" do + assert :id = KeyGenerator.Bigserial.ecto_type([]) + assert :bigserial = KeyGenerator.Bigserial.migration_type([]) + end + + # sabotage: Bigserial.autogenerate/2 returns an MFA -> red (expects nil) + test "declares database-assigned keys for every table" do + for table <- [:charts, :positions, :runs] do + assert nil == KeyGenerator.Bigserial.autogenerate(table, []) + end + end + end +end From a9a9852ed8f73295b3dc2e17ce8a31b70bfe3444 Mon Sep 17 00:00:00 2001 From: JohnnyT Date: Sat, 22 Aug 2026 09:59:21 -0600 Subject: [PATCH 4/6] Adds use StatifierPersistence.Ecto and Config Adds StatifierPersistence.Ecto.Config, the single resolver for the host's compile-time options: repo (required), key (default :uxid via KeyGenerator.resolve/1), table_prefix (default statifier_), per-table overrides, and a separate Postgres-schema prefix, rejecting unknown options with a clear ArgumentError at compile time. Adds the use macro: it exposes the resolved config through __statifier_persistence__/1 and defines Chart, Position, and Run schema modules on the host - configured primary key (generated MFA or database-assigned), engine identity columns verbatim, sources from Config.table/2. The whole module sits behind the ensure_loaded guard so the package compiles without ecto_sql. Phase 4's migrations helper consumes the same Config so schemas and DDL cannot disagree. Refs: sp-02x --- .../260822-sp-02x-keys-tables-migrations.md | 17 +- lib/statifier_persistence/ecto.ex | 117 +++++++++ lib/statifier_persistence/ecto/config.ex | 141 ++++++++++ test/statifier_persistence/ecto_test.exs | 245 ++++++++++++++++++ test/support/ecto_hosts.ex | 31 +++ 5 files changed, 548 insertions(+), 3 deletions(-) create mode 100644 lib/statifier_persistence/ecto.ex create mode 100644 lib/statifier_persistence/ecto/config.ex create mode 100644 test/statifier_persistence/ecto_test.exs create mode 100644 test/support/ecto_hosts.ex diff --git a/docs/plans/260822-sp-02x-keys-tables-migrations.md b/docs/plans/260822-sp-02x-keys-tables-migrations.md index 88b904d..6e15b9e 100644 --- a/docs/plans/260822-sp-02x-keys-tables-migrations.md +++ b/docs/plans/260822-sp-02x-keys-tables-migrations.md @@ -340,9 +340,9 @@ from the macro's real behavior). Sabotage per convention. ### Success Criteria: #### Automated Verification: -- [ ] Full quality gate passes (`mix quality`) -- [ ] `mix gate.verify` passes -- [ ] The moduledoc's zero-config example compiles verbatim in a test +- [x] Full quality gate passes (`mix quality`) +- [x] `mix gate.verify` passes +- [x] The moduledoc's zero-config example compiles verbatim in a test #### Manual Verification: - [ ] Schema module names and option spellings read as a host author would @@ -488,3 +488,14 @@ surfaced once at the end. **Implementation Note**: same loop/full-gate discipline as Phase 1. --- + +### Phase 3 + +- [ ] Schema module names and option spellings read as a host author would + expect (naming judgment, not machine-checkable) + +**Implementation Note**: same loop/full-gate discipline. Schemas are +exercised against live DDL in Phase 4; in this phase their metadata is the +test surface. + +--- diff --git a/lib/statifier_persistence/ecto.ex b/lib/statifier_persistence/ecto.ex new file mode 100644 index 0000000..645dc4b --- /dev/null +++ b/lib/statifier_persistence/ecto.ex @@ -0,0 +1,117 @@ +if Code.ensure_loaded?(Ecto) do + defmodule StatifierPersistence.Ecto do + @moduledoc """ + Compile-time Ecto configuration on the host's own module (ADR-0002). + + A host declares its persistence module once: + + defmodule MyApp.Persistence do + use StatifierPersistence.Ecto, repo: MyApp.Repo + end + + and gets, with zero further options: the resolved configuration + readable via `MyApp.Persistence.__statifier_persistence__/1`, and + three Ecto schema modules - `MyApp.Persistence.Chart`, + `MyApp.Persistence.Position`, `MyApp.Persistence.Run` - over the + `statifier_charts` / `statifier_positions` / `statifier_runs` tables + with UXID string primary keys (`chart_` / `pos_` / `run_` prefixes). + + Every knob is compile-time, on this `use`, never in application env + (ADR-0002 decision 3), and the migrations helper consumes the same + resolved configuration so schemas and DDL cannot disagree. See + `StatifierPersistence.Ecto.Config` for the options (`:key`, + `:table_prefix`, `:tables`, `:prefix`). + + The engine identity columns (`content_hash`, `session_id`, `run_id`) + are stored verbatim as strings and are never touched by the + configured key scheme - ADR-0002 decision 1. + """ + + alias StatifierPersistence.Ecto.Config + + @schema_modules [{Chart, :charts}, {Position, :positions}, {Run, :runs}] + + # The storage contract's field set is the column list (ADR-0003 + # decision 3); the migrations helper's V01 DDL mirrors these exactly. + @fields %{ + charts: [content_hash: :string, identity_blob: :binary, chart_blob: :binary], + positions: [ + session_id: :string, + content_hash: :string, + identity_blob: :binary, + position_blob: :binary + ], + runs: [ + run_id: :string, + status: :string, + content_hash: :string, + identity_blob: :binary, + position_blob: :binary, + failure: :string, + session_id: :string + ] + } + + defmacro __using__(opts) do + quote bind_quoted: [opts: opts] do + @statifier_persistence_config StatifierPersistence.Ecto.Config.new(opts) + + @doc false + @spec __statifier_persistence__(:config | :repo) :: + StatifierPersistence.Ecto.Config.t() | module() + def __statifier_persistence__(:config), do: @statifier_persistence_config + def __statifier_persistence__(:repo), do: @statifier_persistence_config.repo + + StatifierPersistence.Ecto.__define_schemas__(__MODULE__, @statifier_persistence_config) + end + end + + @doc false + @spec __define_schemas__(module(), Config.t()) :: :ok + def __define_schemas__(host, %Config{} = config) do + for {name, table} <- @schema_modules do + Module.create( + Module.concat(host, name), + schema_ast(host, table, config), + Macro.Env.location(__ENV__) + ) + end + + :ok + end + + defp schema_ast(host, table, %Config{} = config) do + fields = + for {field, type} <- Map.fetch!(@fields, table) do + quote do: field(unquote(field), unquote(type)) + end + + quote do + @moduledoc """ + Ecto schema for the `#{unquote(Config.table(config, table))}` table, + generated by `use StatifierPersistence.Ecto` on + `#{unquote(inspect(host))}`. + """ + + use Ecto.Schema + + @schema_prefix unquote(config.prefix) + @primary_key unquote(Macro.escape(primary_key(table, config))) + schema unquote(Config.table(config, table)) do + unquote_splicing(fields) + timestamps(type: :utc_datetime_usec) + end + end + end + + defp primary_key(table, %Config{key: {key_mod, key_opts}}) do + type = key_mod.ecto_type(key_opts) + + case key_mod.autogenerate(table, key_opts) do + # Database-assigned (e.g. bigserial): read the key back on insert. + nil -> {:id, type, autogenerate: false, read_after_writes: true} + {_m, _f, _a} = mfa -> {:id, type, autogenerate: mfa} + end + end + end +end diff --git a/lib/statifier_persistence/ecto/config.ex b/lib/statifier_persistence/ecto/config.ex new file mode 100644 index 0000000..d274f54 --- /dev/null +++ b/lib/statifier_persistence/ecto/config.ex @@ -0,0 +1,141 @@ +defmodule StatifierPersistence.Ecto.Config do + @moduledoc """ + The resolved configuration behind `use StatifierPersistence.Ecto`. + + ADR-0002 decision 3 requires that the generated schemas and the + migrations helper take the same options and cannot disagree. This module + is the single definition site that makes that true: `__using__/1` builds + a `Config` at the host's compile time, and the migrations helper reads + the same struct (via `for: HostModule`) or funnels literal options + through the same `new/1`. + + Options: + + * `:repo` - required, the host's `Ecto.Repo` module + * `:key` - the surrogate-key scheme, `:uxid` (default), `:uuid`, + `:bigserial`, or `{module, opts}` implementing + `StatifierPersistence.Ecto.KeyGenerator` + * `:table_prefix` - prefix for the generated table names, default + `"statifier_"` + * `:tables` - per-table override map with keys `:charts`, + `:positions`, `:runs`; an override replaces the whole name, + prefix included + * `:prefix` - the Postgres schema (Ecto's `@schema_prefix`), default + `nil` + + Unknown options and unknown table keys raise `ArgumentError` - at the + host's compile time when reached through `use`. + """ + + alias StatifierPersistence.Ecto.KeyGenerator + + @known_options [:repo, :key, :table_prefix, :tables, :prefix] + @table_keys [:charts, :positions, :runs] + + @enforce_keys [:repo, :key, :table_prefix, :tables, :prefix] + defstruct [:repo, :key, :table_prefix, :tables, :prefix] + + @typedoc "Resolved configuration for one host module." + @type t :: %__MODULE__{ + repo: module(), + key: {module(), keyword()}, + table_prefix: String.t(), + tables: %{optional(KeyGenerator.table()) => String.t()}, + prefix: String.t() | nil + } + + @doc """ + Validates and resolves the options `use StatifierPersistence.Ecto` + accepts. Raises `ArgumentError` on anything malformed. + """ + @spec new(keyword()) :: t() + def new(opts) when is_list(opts) do + reject_unknown!(opts) + + %__MODULE__{ + repo: fetch_repo!(opts), + key: KeyGenerator.resolve(Keyword.get(opts, :key, :uxid)), + table_prefix: validate_table_prefix!(Keyword.get(opts, :table_prefix, "statifier_")), + tables: validate_tables!(Keyword.get(opts, :tables, %{})), + prefix: validate_prefix!(Keyword.get(opts, :prefix)) + } + end + + def new(other) do + raise ArgumentError, + "expected a keyword list of options, got: #{inspect(other)}" + end + + @doc """ + The table name (source) for `table` under this configuration: the + per-table override when one was given, otherwise the table prefix plus + the table's own name. + """ + @spec table(t(), KeyGenerator.table()) :: String.t() + def table(%__MODULE__{} = config, table) when table in @table_keys do + Map.get(config.tables, table, config.table_prefix <> Atom.to_string(table)) + end + + defp reject_unknown!(opts) do + case Keyword.keys(opts) -- @known_options do + [] -> + :ok + + unknown -> + raise ArgumentError, + "unknown option(s) #{inspect(unknown)} for use StatifierPersistence.Ecto; " <> + "known options are #{inspect(@known_options)}" + end + end + + defp fetch_repo!(opts) do + case Keyword.fetch(opts, :repo) do + {:ok, repo} when is_atom(repo) and not is_nil(repo) -> + repo + + {:ok, other} -> + raise ArgumentError, "the :repo option must be a repo module, got: #{inspect(other)}" + + :error -> + raise ArgumentError, "the :repo option is required for use StatifierPersistence.Ecto" + end + end + + defp validate_table_prefix!(prefix) when is_binary(prefix), do: prefix + + defp validate_table_prefix!(other) do + raise ArgumentError, "the :table_prefix option must be a string, got: #{inspect(other)}" + end + + defp validate_tables!(tables) when is_map(tables) do + Enum.each(tables, fn + {key, name} when key in @table_keys and is_binary(name) -> + :ok + + {key, name} when key in @table_keys -> + raise ArgumentError, + "the :tables override for #{inspect(key)} must be a string, " <> + "got: #{inspect(name)}" + + {key, _name} -> + raise ArgumentError, + "unknown table key #{inspect(key)} in :tables; " <> + "known keys are #{inspect(@table_keys)}" + end) + + tables + end + + defp validate_tables!(other) do + raise ArgumentError, "the :tables option must be a map, got: #{inspect(other)}" + end + + defp validate_prefix!(nil), do: nil + defp validate_prefix!(prefix) when is_binary(prefix), do: prefix + + defp validate_prefix!(other) do + raise ArgumentError, + "the :prefix option (Postgres schema) must be a string or nil, " <> + "got: #{inspect(other)}" + end +end diff --git a/test/statifier_persistence/ecto_test.exs b/test/statifier_persistence/ecto_test.exs new file mode 100644 index 0000000..cb4fa38 --- /dev/null +++ b/test/statifier_persistence/ecto_test.exs @@ -0,0 +1,245 @@ +defmodule StatifierPersistence.EctoTest do + use ExUnit.Case, async: true + + alias StatifierPersistence.Ecto.Config + alias StatifierPersistence.Ecto.KeyGenerator + alias StatifierPersistence.EctoHosts.Bigserial + alias StatifierPersistence.EctoHosts.Default + alias StatifierPersistence.EctoHosts.Overridden + alias StatifierPersistence.TestRepo + + describe "zero-config host (repo: only)" do + # sabotage: Config default table_prefix changed to "statifier" -> red (sources lose the underscore) + test "schemas read from statifier_* tables" do + assert Default.Chart.__schema__(:source) == "statifier_charts" + assert Default.Position.__schema__(:source) == "statifier_positions" + assert Default.Run.__schema__(:source) == "statifier_runs" + end + + # sabotage: primary_key/2 MFA branch returns autogenerate: false -> red (no MFA in :autogenerate) + test "primary keys are UXID strings autogenerated with per-table prefixes" do + for {schema, prefix} <- [ + {Default.Chart, "chart"}, + {Default.Position, "pos"}, + {Default.Run, "run"} + ] do + assert schema.__schema__(:type, :id) == :string + + assert {[:id], {UXID, :generate!, [[prefix: ^prefix]]}} = + List.keyfind(schema.__schema__(:autogenerate), [:id], 0) + end + end + + # sabotage: __statifier_persistence__(:repo) returns the whole config -> red + test "exposes the resolved config and repo on the host module" do + assert %Config{ + repo: TestRepo, + key: {KeyGenerator.UXID, []}, + table_prefix: "statifier_", + tables: %{}, + prefix: nil + } = Default.__statifier_persistence__(:config) + + assert Default.__statifier_persistence__(:repo) == TestRepo + assert Default.Chart.__schema__(:prefix) == nil + end + + # sabotage: @fields runs list drops :failure -> red (runs field list mismatch) + test "schemas carry the engine identity columns verbatim" do + assert Default.Chart.__schema__(:fields) == + [:id, :content_hash, :identity_blob, :chart_blob, :inserted_at, :updated_at] + + assert Default.Position.__schema__(:fields) == + [ + :id, + :session_id, + :content_hash, + :identity_blob, + :position_blob, + :inserted_at, + :updated_at + ] + + assert Default.Run.__schema__(:fields) == + [ + :id, + :run_id, + :status, + :content_hash, + :identity_blob, + :position_blob, + :failure, + :session_id, + :inserted_at, + :updated_at + ] + + # Identities are strings, blobs are binaries, verbatim per ADR-0002 + # decision 1 - never the configured key type. + assert Default.Run.__schema__(:type, :run_id) == :string + assert Default.Run.__schema__(:type, :content_hash) == :string + assert Default.Run.__schema__(:type, :session_id) == :string + assert Default.Run.__schema__(:type, :identity_blob) == :binary + assert Default.Run.__schema__(:type, :status) == :string + assert Default.Run.__schema__(:type, :failure) == :string + assert Default.Position.__schema__(:type, :position_blob) == :binary + assert Default.Chart.__schema__(:type, :chart_blob) == :binary + end + + # sabotage: timestamps type changed to :utc_datetime -> red + test "timestamps are utc_datetime_usec" do + assert Default.Chart.__schema__(:type, :inserted_at) == :utc_datetime_usec + assert Default.Chart.__schema__(:type, :updated_at) == :utc_datetime_usec + end + end + + describe "fully-overridden host (uuid key, prefixes, per-table override)" do + # sabotage: Config.table/2 ignores the :tables override map -> red (runs source falls + # back to "wf_runs") + test "every table-name knob lands in __schema__(:source)" do + assert Overridden.Chart.__schema__(:source) == "wf_charts" + assert Overridden.Position.__schema__(:source) == "wf_positions" + assert Overridden.Run.__schema__(:source) == "workflow_runs" + end + + # sabotage: schema_ast hardcodes @schema_prefix nil -> red + test "the Postgres schema lands in __schema__(:prefix)" do + for schema <- [Overridden.Chart, Overridden.Position, Overridden.Run] do + assert schema.__schema__(:prefix) == "workflows" + end + end + + # sabotage: Config key default resolved regardless of the :key option -> red (id type + # comes back :string) + test "uuid keys are Ecto.UUID fields autogenerated as UUIDv7" do + assert Overridden.Chart.__schema__(:type, :id) == Ecto.UUID + + assert {[:id], {KeyGenerator.UUIDv7, :generate, []}} = + List.keyfind(Overridden.Run.__schema__(:autogenerate), [:id], 0) + end + end + + describe "bigserial host" do + # sabotage: primary_key/2 nil branch drops read_after_writes -> red + test "declares database-assigned keys the repo reads back" do + assert Bigserial.Run.__schema__(:type, :id) == :id + assert List.keyfind(Bigserial.Run.__schema__(:autogenerate), [:id], 0) == nil + assert Bigserial.Run.__schema__(:autogenerate_id) == nil + assert Bigserial.Run.__schema__(:read_after_writes) == [:id] + end + end + + describe "compile-time validation" do + # sabotage: reject_unknown! made a no-op -> red (nothing raised) + test "an unknown option raises ArgumentError at compile time" do + assert_raise ArgumentError, ~r/unknown option\(s\) \[:nope\]/, fn -> + Code.compile_string(""" + defmodule StatifierPersistence.EctoTest.BadOption do + use StatifierPersistence.Ecto, repo: Foo, nope: 1 + end + """) + end + end + + # sabotage: fetch_repo! defaults a missing :repo to nil -> red (nothing raised) + test "a missing :repo raises ArgumentError at compile time" do + assert_raise ArgumentError, ~r/the :repo option is required/, fn -> + Code.compile_string(""" + defmodule StatifierPersistence.EctoTest.NoRepo do + use StatifierPersistence.Ecto, key: :uuid + end + """) + end + end + + # sabotage: validate_tables! unknown-key clause accepts any atom key -> red (nothing raised) + test "an unknown table key raises ArgumentError at compile time" do + assert_raise ArgumentError, ~r/unknown table key :sessions/, fn -> + Code.compile_string(""" + defmodule StatifierPersistence.EctoTest.BadTable do + use StatifierPersistence.Ecto, repo: Foo, tables: %{sessions: "s"} + end + """) + end + end + + # sabotage: KeyGenerator.resolve/1 catch-all returns the UXID default -> red (nothing raised) + test "an unknown key spelling raises ArgumentError at compile time" do + assert_raise ArgumentError, ~r/unknown key option :ulid/, fn -> + Code.compile_string(""" + defmodule StatifierPersistence.EctoTest.BadKey do + use StatifierPersistence.Ecto, repo: Foo, key: :ulid + end + """) + end + end + end + + describe "Config.new/1 validation not reachable through a compiling use" do + # sabotage: validate_table_prefix! accepts any term -> red (nothing raised) + test "rejects a non-string table_prefix" do + assert_raise ArgumentError, ~r/:table_prefix option must be a string/, fn -> + Config.new(repo: TestRepo, table_prefix: :statifier) + end + end + + # sabotage: validate_prefix! accepts any term -> red (nothing raised) + test "rejects a non-string prefix" do + assert_raise ArgumentError, ~r/:prefix option \(Postgres schema\) must be a string/, fn -> + Config.new(repo: TestRepo, prefix: :workflows) + end + end + + # sabotage: validate_tables! map clause accepts non-string names -> red (nothing raised) + test "rejects a non-string table override and a non-map tables option" do + assert_raise ArgumentError, ~r/override for :runs must be a string/, fn -> + Config.new(repo: TestRepo, tables: %{runs: :workflow_runs}) + end + + assert_raise ArgumentError, ~r/:tables option must be a map/, fn -> + Config.new(repo: TestRepo, tables: [runs: "workflow_runs"]) + end + end + + # sabotage: fetch_repo! accepts any term -> red (nothing raised) + test "rejects a non-module repo and non-keyword options" do + assert_raise ArgumentError, ~r/:repo option must be a repo module/, fn -> + Config.new(repo: "MyApp.Repo") + end + + assert_raise ArgumentError, ~r/expected a keyword list/, fn -> + Config.new(%{repo: TestRepo}) + end + end + end + + describe "the moduledoc's zero-config example" do + # sabotage: moduledoc example switched to a `repository:` option -> red (snippet no + # longer compiles) + test "compiles verbatim" do + {:docs_v1, _anno, _lang, _fmt, %{"en" => moduledoc}, _meta, _docs} = + Code.fetch_docs(StatifierPersistence.Ecto) + + assert [example] = + Regex.run( + ~r/^ defmodule MyApp\.Persistence do\n(?: .*\n)*? end$/m, + moduledoc + ), + "moduledoc no longer carries the zero-config example" + + code = + example + |> String.split("\n") + |> Enum.map_join("\n", &String.replace_prefix(&1, " ", "")) + + modules = code |> Code.compile_string() |> Enum.map(&elem(&1, 0)) + + # Bind through variables: the modules only exist once the snippet + # compiles, and literal remote calls would warn at test compile time. + assert host = Enum.find(modules, &(&1 == MyApp.Persistence)) + run_schema = Module.concat(host, Run) + assert run_schema.__schema__(:source) == "statifier_runs" + assert host.__statifier_persistence__(:repo) == MyApp.Repo + end + end +end diff --git a/test/support/ecto_hosts.ex b/test/support/ecto_hosts.ex new file mode 100644 index 0000000..94e33f3 --- /dev/null +++ b/test/support/ecto_hosts.ex @@ -0,0 +1,31 @@ +defmodule StatifierPersistence.EctoHosts do + @moduledoc """ + Fixture host modules for `use StatifierPersistence.Ecto` tests. + + Three hosts spanning the option surface: the zero-config default, a + host overriding every knob, and a host on database-assigned keys. + Test-only support code, not part of the package's public API. + """ + + defmodule Default do + @moduledoc false + use StatifierPersistence.Ecto, repo: StatifierPersistence.TestRepo + end + + defmodule Overridden do + @moduledoc false + use StatifierPersistence.Ecto, + repo: StatifierPersistence.TestRepo, + key: :uuid, + table_prefix: "wf_", + tables: %{runs: "workflow_runs"}, + prefix: "workflows" + end + + defmodule Bigserial do + @moduledoc false + use StatifierPersistence.Ecto, + repo: StatifierPersistence.TestRepo, + key: :bigserial + end +end From 47735d9e138f9cbd589d6feae475ece12c61c1ad Mon Sep 17 00:00:00 2001 From: JohnnyT Date: Sat, 22 Aug 2026 10:10:48 -0600 Subject: [PATCH 5/6] Adds the versioned migrations helper, proven live Adds StatifierPersistence.Ecto.Migrations in the Oban.Migration mold: up/down through two doors - for: HostModule reading the compiled Config, or the same literal options use takes - both funneled through the one Config resolver so schemas and DDL cannot disagree. V01 creates charts, positions, and runs per Config, with the engine identity columns and their unique indexes independent of the configured key, honoring the Postgres-schema prefix and per-table overrides; down drops in reverse. Proves it live against Postgres: per-key round trips through the generated schemas, information_schema assertions that identity columns and unique indexes are identical across uxid, uuid, and bigserial configs, duplicate-identity inserts refused, and clean down migrations. Closes sp-02x's acceptance criteria. Refs: sp-02x --- .../260822-sp-02x-keys-tables-migrations.md | 17 +- lib/statifier_persistence/ecto/migrations.ex | 107 ++++++ .../ecto/migrations/v01.ex | 85 ++++ .../ecto/migrations_test.exs | 362 ++++++++++++++++++ test/support/ecto_hosts.ex | 28 ++ test/test_helper.exs | 4 + 6 files changed, 601 insertions(+), 2 deletions(-) create mode 100644 lib/statifier_persistence/ecto/migrations.ex create mode 100644 lib/statifier_persistence/ecto/migrations/v01.ex create mode 100644 test/statifier_persistence/ecto/migrations_test.exs diff --git a/docs/plans/260822-sp-02x-keys-tables-migrations.md b/docs/plans/260822-sp-02x-keys-tables-migrations.md index 6e15b9e..a23068d 100644 --- a/docs/plans/260822-sp-02x-keys-tables-migrations.md +++ b/docs/plans/260822-sp-02x-keys-tables-migrations.md @@ -415,8 +415,8 @@ is `async: false`), restoring `:manual` afterward; the sandbox stays ### Success Criteria: #### Automated Verification: -- [ ] Full quality gate passes (`mix quality`) -- [ ] `mix gate.verify` passes +- [x] Full quality gate passes (`mix quality`) +- [x] `mix gate.verify` passes #### Manual Verification: - [ ] `psql \d` on the migrated default tables matches ADR-0002's sketch @@ -499,3 +499,16 @@ exercised against live DDL in Phase 4; in this phase their metadata is the test surface. --- + +### Phase 4 + +- [ ] `psql \d` on the migrated default tables matches ADR-0002's sketch +- [ ] CI green on the pushed branch (service container exercised by the + live tests) + +**Implementation Note**: same loop/full-gate discipline. This phase closes +sp-02x's acceptance criteria: behaviour + macro + helper exist and agree +on options (one `Config`), identity guard provably independent of the key, +defaults work with zero options beyond `repo:`. + +--- diff --git a/lib/statifier_persistence/ecto/migrations.ex b/lib/statifier_persistence/ecto/migrations.ex new file mode 100644 index 0000000..55b6c78 --- /dev/null +++ b/lib/statifier_persistence/ecto/migrations.ex @@ -0,0 +1,107 @@ +if Code.ensure_loaded?(Ecto.Migration) do + defmodule StatifierPersistence.Ecto.Migrations do + @moduledoc """ + Versioned migrations for this package's tables, in the `Oban.Migration` + mold: the host writes one ordinary migration that delegates here, and + later package versions ship higher-numbered migration modules the same + call picks up. + + The supported spelling reads the host's compiled configuration, so the + DDL cannot drift from the generated schemas (ADR-0002 decision 3): + + defmodule MyApp.Repo.Migrations.AddStatifierPersistence do + use Ecto.Migration + + def up, do: StatifierPersistence.Ecto.Migrations.up(for: MyApp.Persistence) + def down, do: StatifierPersistence.Ecto.Migrations.down(for: MyApp.Persistence) + end + + Alternatively, `up/1` and `down/1` accept the same literal options + `use StatifierPersistence.Ecto` takes (`:repo`, `:key`, `:table_prefix`, + `:tables`, `:prefix`), funneled through the same + `StatifierPersistence.Ecto.Config.new/1` - one resolver, both doors. + The two spellings cannot be mixed in one call. + + `version:` selects the target version. `up/1` migrates from V01 through + the target (default: the newest this package knows); `down/1` rolls back + from the newest through the target (default: V01, i.e. everything). + + When `prefix:` names a Postgres schema, `up/1` creates the schema if it + does not exist; `down/1` leaves the schema in place (dropping a schema + the host may share is not this package's call). + """ + + alias StatifierPersistence.Ecto.Config + + @initial_version 1 + @current_version 1 + + @migrations %{1 => StatifierPersistence.Ecto.Migrations.V01} + + @doc """ + Migrates the tables up through `version:` (default: the newest). + + Takes `for: HostModule` or the literal options `use` takes - see the + moduledoc. + """ + @spec up(keyword()) :: :ok + def up(opts) when is_list(opts) do + {config, target} = parse!(opts, @current_version) + + Enum.each(@initial_version..target, fn version -> + Map.fetch!(@migrations, version).up(config) + end) + end + + @doc """ + Rolls the tables back from the newest version through `version:` + (default: V01, i.e. everything). Takes the same options as `up/1`. + """ + @spec down(keyword()) :: :ok + def down(opts) when is_list(opts) do + {config, target} = parse!(opts, @initial_version) + + Enum.each(@current_version..target//-1, fn version -> + Map.fetch!(@migrations, version).down(config) + end) + end + + defp parse!(opts, default_version) do + {version, opts} = Keyword.pop(opts, :version, default_version) + + if not (is_integer(version) and version in @initial_version..@current_version) do + raise ArgumentError, + "unknown migration version #{inspect(version)}; " <> + "this package knows versions #{@initial_version} " <> + "through #{@current_version}" + end + + {config!(opts), version} + end + + defp config!(opts) do + case Keyword.pop(opts, :for) do + {nil, opts} -> + Config.new(opts) + + {host, []} when is_atom(host) -> + host_config!(host) + + {_host, rest} -> + raise ArgumentError, + "for: cannot be combined with literal options; " <> + "got #{inspect(Keyword.keys(rest))} alongside it" + end + end + + defp host_config!(host) do + if Code.ensure_loaded?(host) and function_exported?(host, :__statifier_persistence__, 1) do + host.__statifier_persistence__(:config) + else + raise ArgumentError, + "#{inspect(host)} does not use StatifierPersistence.Ecto, " <> + "so it carries no configuration to migrate for" + end + end + end +end diff --git a/lib/statifier_persistence/ecto/migrations/v01.ex b/lib/statifier_persistence/ecto/migrations/v01.ex new file mode 100644 index 0000000..6c5efa8 --- /dev/null +++ b/lib/statifier_persistence/ecto/migrations/v01.ex @@ -0,0 +1,85 @@ +if Code.ensure_loaded?(Ecto.Migration) do + defmodule StatifierPersistence.Ecto.Migrations.V01 do + @moduledoc """ + V01 of the package DDL: the `charts`, `positions`, and `runs` tables + per ADR-0002 (as amended) and the storage contract's field set + (ADR-0003 decision 3). + + Table names, the surrogate primary key's column type, and the Postgres + schema all come from the resolved `StatifierPersistence.Ecto.Config` - + the same struct the generated schemas are built from. The engine + identity columns (`content_hash`, `session_id`, `run_id`) are `text` + with their unique indexes regardless of the configured key scheme: + the identity guard never touches a surrogate key. + """ + + use Ecto.Migration + + alias StatifierPersistence.Ecto.Config + + @doc "Creates the V01 tables and their unique indexes per `config`." + @spec up(Config.t()) :: :ok + def up(%Config{key: {key_mod, key_opts}} = config) do + if config.prefix do + execute(~s(CREATE SCHEMA IF NOT EXISTS "#{config.prefix}")) + end + + pk_type = key_mod.migration_type(key_opts) + + charts = Config.table(config, :charts) + + create table(charts, primary_key: false, prefix: config.prefix) do + add(:id, pk_type, primary_key: true) + add(:content_hash, :text, null: false) + add(:identity_blob, :binary, null: false) + add(:chart_blob, :binary, null: false) + timestamps(type: :utc_datetime_usec) + end + + create(unique_index(charts, [:content_hash], prefix: config.prefix)) + + positions = Config.table(config, :positions) + + create table(positions, primary_key: false, prefix: config.prefix) do + add(:id, pk_type, primary_key: true) + add(:session_id, :text, null: false) + add(:content_hash, :text, null: false) + add(:identity_blob, :binary, null: false) + add(:position_blob, :binary, null: false) + timestamps(type: :utc_datetime_usec) + end + + create(unique_index(positions, [:session_id], prefix: config.prefix)) + + runs = Config.table(config, :runs) + + create table(runs, primary_key: false, prefix: config.prefix) do + add(:id, pk_type, primary_key: true) + add(:run_id, :text, null: false) + add(:status, :text, null: false) + add(:content_hash, :text, null: false) + add(:identity_blob, :binary, null: false) + add(:position_blob, :binary, null: true) + add(:failure, :text, null: true) + # Nullable by design (ADR-0002 decision 5): library code does not + # populate it yet. + add(:session_id, :text, null: true) + timestamps(type: :utc_datetime_usec) + end + + create(unique_index(runs, [:run_id], prefix: config.prefix)) + + :ok + end + + @doc "Drops the V01 tables in reverse creation order." + @spec down(Config.t()) :: :ok + def down(%Config{} = config) do + for name <- [:runs, :positions, :charts] do + drop(table(Config.table(config, name), prefix: config.prefix)) + end + + :ok + end + end +end diff --git a/test/statifier_persistence/ecto/migrations_test.exs b/test/statifier_persistence/ecto/migrations_test.exs new file mode 100644 index 0000000..8bdb166 --- /dev/null +++ b/test/statifier_persistence/ecto/migrations_test.exs @@ -0,0 +1,362 @@ +defmodule StatifierPersistence.Ecto.MigrationsTest do + # Live migration tests manage their own DDL and inserts outside the SQL + # sandbox: setup_all switches the repo to :auto mode for the module and + # restores :manual on exit, hence async: false. + use ExUnit.Case, async: false + + alias Ecto.Adapters.SQL + alias Ecto.Adapters.SQL.Sandbox + alias Ecto.Migrator + alias StatifierPersistence.Ecto.Migrations + alias StatifierPersistence.EctoHosts.{KxBigserial, KxUuid, KxUxid} + alias StatifierPersistence.TestRepo + + defmodule MigrateKxUxid do + use Ecto.Migration + + def up, do: Migrations.up(for: StatifierPersistence.EctoHosts.KxUxid) + def down, do: Migrations.down(for: StatifierPersistence.EctoHosts.KxUxid) + end + + defmodule MigrateKxUuid do + use Ecto.Migration + + def up, do: Migrations.up(for: StatifierPersistence.EctoHosts.KxUuid) + def down, do: Migrations.down(for: StatifierPersistence.EctoHosts.KxUuid) + end + + defmodule MigrateKxBigserial do + use Ecto.Migration + + def up, do: Migrations.up(for: StatifierPersistence.EctoHosts.KxBigserial) + def down, do: Migrations.down(for: StatifierPersistence.EctoHosts.KxBigserial) + end + + # The literal-options door: the same options `use` takes, plus the + # `prefix:` Postgres-schema knob. + defmodule MigrateKxLiteral do + use Ecto.Migration + + @opts [ + repo: StatifierPersistence.TestRepo, + key: :uxid, + table_prefix: "kx_lit_", + prefix: "kx_schema" + ] + + def up, do: Migrations.up(@opts) + def down, do: Migrations.down(@opts) + end + + @host_migrations [ + {20_260_822_000_001, MigrateKxUxid}, + {20_260_822_000_002, MigrateKxUuid}, + {20_260_822_000_003, MigrateKxBigserial} + ] + + @literal_version 20_260_822_000_009 + + @key_prefixes ["kx_uxid_", "kx_uuid_", "kx_big_"] + + setup_all do + Sandbox.mode(TestRepo, :auto) + on_exit(fn -> Sandbox.mode(TestRepo, :manual) end) + + for {version, module} <- @host_migrations do + :ok = migrate(:up, version, module) + end + + on_exit(fn -> + for {version, module} <- Enum.reverse(@host_migrations) do + :ok = migrate(:down, version, module) + end + end) + + :ok + end + + defp migrate(direction, version, module) do + case apply(Migrator, direction, [TestRepo, version, module, [log: false]]) do + :ok -> :ok + :already_up -> :ok + :already_down -> :ok + end + end + + describe "rows through the generated schemas" do + # sabotage: removed V01's charts chart_blob column -> inserts red (undefined column) + test "uxid host: keys carry per-table prefixes, identities verbatim" do + chart = + TestRepo.insert!(%KxUxid.Chart{ + content_hash: "sha256:kx-uxid-chart", + identity_blob: <<1, 2, 3>>, + chart_blob: <<4, 5, 6>> + }) + + assert String.starts_with?(chart.id, "chart_") + fetched = TestRepo.get!(KxUxid.Chart, chart.id) + assert fetched.content_hash == "sha256:kx-uxid-chart" + assert fetched.identity_blob == <<1, 2, 3>> + assert fetched.chart_blob == <<4, 5, 6>> + + position = + TestRepo.insert!(%KxUxid.Position{ + session_id: "sess-kx-uxid", + content_hash: "sha256:kx-uxid-chart", + identity_blob: <<1, 2, 3>>, + position_blob: <<7, 8>> + }) + + assert String.starts_with?(position.id, "pos_") + assert TestRepo.get!(KxUxid.Position, position.id).session_id == "sess-kx-uxid" + + run = + TestRepo.insert!(%KxUxid.Run{ + run_id: "run-kx-uxid", + status: "running", + content_hash: "sha256:kx-uxid-chart", + identity_blob: <<1, 2, 3>> + }) + + assert String.starts_with?(run.id, "run_") + fetched_run = TestRepo.get!(KxUxid.Run, run.id) + assert fetched_run.run_id == "run-kx-uxid" + assert fetched_run.position_blob == nil + assert fetched_run.failure == nil + assert fetched_run.session_id == nil + end + + # sabotage: hardcoded V01's pk_type to :text -> uuid insert red (type mismatch) + test "uuid host: primary keys are UUIDv7, identities verbatim" do + chart = + TestRepo.insert!(%KxUuid.Chart{ + content_hash: "sha256:kx-uuid-chart", + identity_blob: <<9>>, + chart_blob: <<10>> + }) + + # Canonical form: the character at index 14 is the version nibble. + assert String.at(chart.id, 14) == "7" + assert {:ok, _} = Ecto.UUID.dump(chart.id) + assert TestRepo.get!(KxUuid.Chart, chart.id).content_hash == "sha256:kx-uuid-chart" + + run = + TestRepo.insert!(%KxUuid.Run{ + run_id: "run-kx-uuid", + status: "completed", + content_hash: "sha256:kx-uuid-chart", + identity_blob: <<9>> + }) + + assert TestRepo.get!(KxUuid.Run, run.id).run_id == "run-kx-uuid" + end + + # sabotage: hardcoded V01's pk_type to :text -> insert red (no db-assigned key) + test "bigserial host: database assigns integer keys, identities verbatim" do + chart = + TestRepo.insert!(%KxBigserial.Chart{ + content_hash: "sha256:kx-big-chart", + identity_blob: <<11>>, + chart_blob: <<12>> + }) + + assert is_integer(chart.id) + assert TestRepo.get!(KxBigserial.Chart, chart.id).content_hash == "sha256:kx-big-chart" + + position = + TestRepo.insert!(%KxBigserial.Position{ + session_id: "sess-kx-big", + content_hash: "sha256:kx-big-chart", + identity_blob: <<11>>, + position_blob: <<13>> + }) + + assert is_integer(position.id) + assert TestRepo.get!(KxBigserial.Position, position.id).session_id == "sess-kx-big" + end + end + + describe "identity columns across key configurations" do + # sabotage: made V01's runs content_hash :string (varchar) -> red on runs drift + test "content_hash/session_id/run_id columns identical across all three" do + # [column_name, data_type, is_nullable] per ADR-0002: identities are + # text, verbatim; only runs.session_id is nullable (decision 5). + for {table, expected} <- [ + {"charts", [["content_hash", "text", "NO"]]}, + {"positions", [["content_hash", "text", "NO"], ["session_id", "text", "NO"]]}, + {"runs", + [ + ["content_hash", "text", "NO"], + ["run_id", "text", "NO"], + ["session_id", "text", "YES"] + ]} + ] do + columns = Enum.map(expected, &hd/1) + + for prefix <- @key_prefixes do + assert identity_columns(prefix <> table, columns) == expected, + "identity columns of #{prefix}#{table} drifted" + end + end + end + + # sabotage: removed V01's charts/runs unique_index calls -> red + test "unique indexes on the identity columns identical across all three" do + for {table, unique_columns} <- [ + {"charts", [["content_hash"]]}, + {"positions", [["session_id"]]}, + {"runs", [["run_id"]]} + ] do + for prefix <- @key_prefixes do + assert unique_index_columns(prefix <> table) == unique_columns + end + end + end + end + + describe "unique indexes enforced" do + # sabotage: removed V01's charts unique_index -> duplicate insert red + test "a duplicate content_hash insert violates the charts unique index" do + TestRepo.insert!(%KxUxid.Chart{ + content_hash: "sha256:kx-dup-chart", + identity_blob: <<1>>, + chart_blob: <<2>> + }) + + assert_raise Ecto.ConstraintError, ~r/content_hash/, fn -> + TestRepo.insert!(%KxUxid.Chart{ + content_hash: "sha256:kx-dup-chart", + identity_blob: <<1>>, + chart_blob: <<2>> + }) + end + end + + # sabotage: removed V01's runs unique_index -> duplicate insert red + test "a duplicate run_id insert violates the runs unique index" do + TestRepo.insert!(%KxUuid.Run{ + run_id: "run-kx-dup", + status: "running", + content_hash: "sha256:kx-dup", + identity_blob: <<1>> + }) + + assert_raise Ecto.ConstraintError, ~r/run_id/, fn -> + TestRepo.insert!(%KxUuid.Run{ + run_id: "run-kx-dup", + status: "failed", + content_hash: "sha256:kx-dup", + identity_blob: <<1>> + }) + end + end + end + + describe "the literal-options door and down" do + # sabotage: made V01.down drop only charts -> red on leftover kx_lit_ tables + test "literal options with a Postgres schema migrate up and down clean" do + on_exit(fn -> + SQL.query!(TestRepo, "DROP SCHEMA IF EXISTS kx_schema CASCADE", []) + + SQL.query!(TestRepo, "DELETE FROM schema_migrations WHERE version = $1", [ + @literal_version + ]) + end) + + :ok = migrate(:up, @literal_version, MigrateKxLiteral) + + assert tables_in_schema("kx_schema", "kx_lit_") == + ["kx_lit_charts", "kx_lit_positions", "kx_lit_runs"] + + assert unique_index_columns("kx_lit_runs", "kx_schema") == [["run_id"]] + + :ok = migrate(:down, @literal_version, MigrateKxLiteral) + + assert tables_in_schema("kx_schema", "kx_lit_") == [] + + # A plain (non-CASCADE) drop doubles as proof down left the schema empty. + SQL.query!(TestRepo, ~s(DROP SCHEMA "kx_schema"), []) + end + end + + describe "option validation" do + # sabotage: skipped parse!'s version validation -> red (KeyError, not ArgumentError) + test "an unknown version raises before any DDL" do + assert_raise ArgumentError, ~r/unknown migration version/, fn -> + Migrations.up(for: KxUxid, version: 2) + end + + assert_raise ArgumentError, ~r/unknown migration version/, fn -> + Migrations.down(for: KxUxid, version: 0) + end + end + + # sabotage: made the for: clause swallow extra options -> red (nothing raised) + test "for: combined with literal options raises" do + assert_raise ArgumentError, ~r/cannot be combined/, fn -> + Migrations.up(for: KxUxid, table_prefix: "kx_other_") + end + end + + # sabotage: skipped host_config!'s exported-function check -> red (UndefinedFunctionError) + test "for: a module that did not use StatifierPersistence.Ecto raises" do + assert_raise ArgumentError, ~r/does not use StatifierPersistence.Ecto/, fn -> + Migrations.up(for: Enum) + end + end + end + + defp identity_columns(table, columns) do + %{rows: rows} = + SQL.query!( + TestRepo, + """ + SELECT column_name, data_type, is_nullable + FROM information_schema.columns + WHERE table_schema = 'public' AND table_name = $1 + AND column_name = ANY($2) + ORDER BY column_name + """, + [table, columns] + ) + + rows + end + + defp unique_index_columns(table, schema \\ "public") do + %{rows: rows} = + SQL.query!( + TestRepo, + """ + SELECT array_agg(a.attname ORDER BY a.attname) + FROM pg_index ix + JOIN pg_class t ON t.oid = ix.indrelid + JOIN pg_namespace n ON n.oid = t.relnamespace + JOIN pg_class i ON i.oid = ix.indexrelid + JOIN pg_attribute a ON a.attrelid = t.oid AND a.attnum = ANY(ix.indkey) + WHERE n.nspname = $1 AND t.relname = $2 + AND ix.indisunique AND NOT ix.indisprimary + GROUP BY i.relname + ORDER BY i.relname + """, + [schema, table] + ) + + Enum.map(rows, fn [columns] -> Enum.sort(columns) end) + end + + defp tables_in_schema(schema, like_prefix) do + %{rows: rows} = + SQL.query!( + TestRepo, + """ + SELECT table_name FROM information_schema.tables + WHERE table_schema = $1 AND table_name LIKE $2 + ORDER BY table_name + """, + [schema, like_prefix <> "%"] + ) + + List.flatten(rows) + end +end diff --git a/test/support/ecto_hosts.ex b/test/support/ecto_hosts.ex index 94e33f3..d916a26 100644 --- a/test/support/ecto_hosts.ex +++ b/test/support/ecto_hosts.ex @@ -5,6 +5,10 @@ defmodule StatifierPersistence.EctoHosts do Three hosts spanning the option surface: the zero-config default, a host overriding every knob, and a host on database-assigned keys. Test-only support code, not part of the package's public API. + + The `Kx*` hosts back the live migration tests: one per key scheme, each + with a distinct `kx_` table prefix so their DDL coexists in one database + and is dropped wholesale after the suite. """ defmodule Default do @@ -28,4 +32,28 @@ defmodule StatifierPersistence.EctoHosts do repo: StatifierPersistence.TestRepo, key: :bigserial end + + defmodule KxUxid do + @moduledoc false + use StatifierPersistence.Ecto, + repo: StatifierPersistence.TestRepo, + key: :uxid, + table_prefix: "kx_uxid_" + end + + defmodule KxUuid do + @moduledoc false + use StatifierPersistence.Ecto, + repo: StatifierPersistence.TestRepo, + key: :uuid, + table_prefix: "kx_uuid_" + end + + defmodule KxBigserial do + @moduledoc false + use StatifierPersistence.Ecto, + repo: StatifierPersistence.TestRepo, + key: :bigserial, + table_prefix: "kx_big_" + end end diff --git a/test/test_helper.exs b/test/test_helper.exs index a692016..f8c3630 100644 --- a/test/test_helper.exs +++ b/test/test_helper.exs @@ -12,6 +12,10 @@ end {:ok, _pid} = StatifierPersistence.TestRepo.start_link() +# The sandbox stays :manual for everything except the live migration tests +# (migrations_test.exs, async: false), which manage their own DDL and +# inserts: they switch the repo to :auto for the duration of their +# setup_all/on_exit work and restore :manual afterward. Ecto.Adapters.SQL.Sandbox.mode(StatifierPersistence.TestRepo, :manual) ExUnit.start() From aacd8e5ae488e4402b8dd0dad43c81c930492e00 Mon Sep 17 00:00:00 2001 From: JohnnyT Date: Sat, 22 Aug 2026 10:12:15 -0600 Subject: [PATCH 6/6] Adds the sp-02x changelog fragment Records the new public surface: use StatifierPersistence.Ecto, the KeyGenerator behaviour with its uxid/uuid/bigserial schemes, the versioned Migrations helper, and the dependency changes (uxid required, ecto_sql optional). Refs: sp-02x --- changelog.d/sp-02x.md | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) create mode 100644 changelog.d/sp-02x.md diff --git a/changelog.d/sp-02x.md b/changelog.d/sp-02x.md new file mode 100644 index 0000000..03724af --- /dev/null +++ b/changelog.d/sp-02x.md @@ -0,0 +1,22 @@ +# sp-02x + +## Added + +- `use StatifierPersistence.Ecto`: compile-time configuration on the host's + module (`repo:`, `key:`, `table_prefix:`, `tables:`, `prefix:`) that + defines `Chart`, `Position`, and `Run` schema modules and exposes the + resolved config via `__statifier_persistence__/1`. Requires the optional + `ecto_sql` dependency. +- `StatifierPersistence.Ecto.KeyGenerator`: the behaviour a surrogate-key + scheme implements, with `:uxid` (default), `:uuid` (UUIDv7), `:bigserial`, + and `{module, opts}` resolved through `resolve/1`. +- `StatifierPersistence.Ecto.Migrations`: the versioned migrations helper + (`up/1`, `down/1`, taking `for: HostModule` or the same literal options + `use` takes) that creates the `charts`/`positions`/`runs` tables from the + same resolved config the schemas use. + +## Changed + +- `uxid` is now a required dependency (the default key scheme works out of + the box); `ecto_sql` is an optional dependency and the package compiles + without it.