feat(fp): WP-22 — durable persistence (SQLite/EF Core)
Applications, documents (+ audit log) and the brief move off static in-memory Dictionaries onto a real SQLite file via EF Core, so demo data survives a process restart or `docker compose restart api` for the first time. The three stores (ApplicationStore/DocumentStore/BriefStore) keep their exact public signatures and static-class shape — no DI, no async ripple into Program.cs's minimal-API handlers — each method just opens a short-lived AppDbContext via Db.Create() under the same lock it already had. Opaque nested shapes (a wizard's draft snapshot, a brief's sections/placeholders/status) are stored as JSON text columns rather than redesigned into relational tables, matching the existing "don't interpret it" posture. Found two things the WP's own text got wrong, corrected in docs/backlog/WP-22-durable-persistence.md's Deviations section: SeedData never seeded these three stores (only the read-only BRP/DUO-mimicking GETs, which stay in-memory) so there's no seed step; and no new docker-compose volume is needed since the existing bind mount already covers the SQLite file — verified against this environment's real podman-backed compose stack, not just by reading the file. Also: pinned SQLitePCLRaw.bundle_e_sqlite3 to 3.0.3 (EF Core Sqlite's own transitive default bundles a pre-3.50.2 SQLite with a known high-severity memory-corruption advisory); found and fixed a real xUnit test race where concurrent test-class hosts stomped a shared static connection-string field, fixed by disabling cross-class test parallelization rather than adding DI the stores don't otherwise need. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
@@ -1,6 +1,6 @@
|
||||
# WP-22 — Durable persistence (optional tier)
|
||||
|
||||
Status: todo
|
||||
Status: done (pending commit)
|
||||
Phase: 5 — productie-volwassenheid
|
||||
|
||||
## Why
|
||||
@@ -98,22 +98,30 @@ speculatively.
|
||||
|
||||
## Acceptance criteria
|
||||
|
||||
- [ ] All three stores are EF Core/SQLite-backed; no `static Dictionary` remains in
|
||||
- [x] All three stores are EF Core/SQLite-backed; no `static Dictionary` remains in
|
||||
`Data/*.cs` for application/document/brief state.
|
||||
- [ ] Every existing backend test passes unchanged (signatures didn't change).
|
||||
- [ ] Restarting the backend process preserves previously created applications,
|
||||
- [x] Every existing backend test passes unchanged (signatures didn't change).
|
||||
84/84 green, stable across repeated runs (see Deviations for a real race this
|
||||
surfaced).
|
||||
- [x] Restarting the backend process preserves previously created applications,
|
||||
documents, and brief drafts (manually verified).
|
||||
- [ ] The audit log survives a restart and is queryable (even if no new endpoint
|
||||
exposes it yet — persistence is the bar, not a new audit UI).
|
||||
- [ ] `docker compose up` with the new volume also survives a container restart.
|
||||
- [x] The audit log survives a restart and is queryable (even if no new endpoint
|
||||
exposes it yet — persistence is the bar, not a new audit UI). `AuditEntries`
|
||||
is a real table now; not separately re-verified across restart beyond the
|
||||
applications/brief checks (same store mechanism, same `Db.Create()` seam).
|
||||
- [x] `docker compose up` with a container restart preserves data — **no new
|
||||
volume** turned out to be needed (see Deviations).
|
||||
|
||||
## Verification
|
||||
|
||||
`cd backend && dotnet test`. Manual: `dotnet run --project src/BigRegister.Api`,
|
||||
create an application via the FE or a curl request, kill and restart the process,
|
||||
confirm `GET /api/v1/applications` still returns it. Repeat with `docker compose up`
|
||||
|
||||
- `docker compose restart api`.
|
||||
`cd backend && dotnet test` — 84/84 green. Manual: `dotnet run --project
|
||||
src/BigRegister.Api`, created an application via curl, killed and restarted the
|
||||
process, confirmed `GET /api/v1/applications` still returned it (repeated for the
|
||||
brief). Repeated the same check against the **real** `docker compose up` stack
|
||||
(this environment has an actual podman-backed compose, not a mock) — created an
|
||||
application via `curl localhost:5000`, ran `docker compose restart api`, confirmed
|
||||
it survived, and confirmed on the host that `backend/src/BigRegister.Api/bigregister.db`
|
||||
is the file being written (gitignored, not tracked).
|
||||
|
||||
## Out of scope
|
||||
|
||||
@@ -131,3 +139,47 @@ synchronously; converting to `async`/`await` may ripple further than "just the
|
||||
Data/ layer" if minimal-API handlers aren't already `async`. Check this before
|
||||
starting and budget for handler signature changes (still not a _behavior_ change,
|
||||
but a wider diff than the Files section implies if handlers need `async` added).
|
||||
|
||||
**Resolved**: didn't ripple at all. EF Core's SQLite provider fully supports
|
||||
synchronous APIs (`.Find()`, `.ToList()`, `.SaveChanges()`, `.ExecuteDelete()`); every
|
||||
store method stayed synchronous, so `Program.cs`'s minimal-API handlers needed zero
|
||||
changes. The stores stayed **static classes** with no DI — each method opens its
|
||||
own short-lived `AppDbContext` via a small `Db.Create()` factory (`Data/Db.cs`) under
|
||||
the same `lock (_gate)` each store already had, which now doubles as a single-writer
|
||||
guard for the SQLite file (SQLite tolerates only one writer at a time anyway).
|
||||
|
||||
## Deviations from the plan
|
||||
|
||||
- **No SeedData → DB seed step.** The WP's own "Decisions"/"Files" sections assumed
|
||||
`SeedData` populates the three stores and needs a "seed if empty" migration. It
|
||||
doesn't — `SeedData` only backs the read-only BRP/DUO-mimicking GET endpoints
|
||||
(registration, person, diplomas, notes), which stay in-memory and are untouched by
|
||||
this WP. Applications/Documents/Briefs never had seed data; they started empty
|
||||
before this WP and still do. One less step than planned.
|
||||
- **No new docker-compose volume.** The existing `./backend:/src` bind mount already
|
||||
covers `bigregister.db` (it's written under `src/BigRegister.Api/`, itself inside
|
||||
the bind-mounted tree — confirmed empirically, not just by reading the compose
|
||||
file), so a container restart already persists it for free. Added a comment
|
||||
instead of a redundant `volumes:` entry.
|
||||
- **Opaque nested shapes (wizard draft, brief sections/placeholders/status) became
|
||||
JSON text columns**, not new relational tables — matches the WP's own "relocate
|
||||
the shape, don't redesign it" instruction and the existing "the backend treats
|
||||
brief content as opaque" posture.
|
||||
- **Found and fixed a real test race, not a hypothetical one.** The stores read a
|
||||
single static `Db.ConnectionString` (matching their pre-WP-22 static-Dictionary
|
||||
shape — no DI). xUnit's default parallel-across-classes execution ran multiple
|
||||
`WebApplicationFactory` hosts concurrently in the one test process, each
|
||||
overwriting that same static field with its own temp-file path — caught as a
|
||||
`SQLite Error 1: 'table "Applications" already exists'` from two `Migrate()` calls
|
||||
interleaving on whichever file won the race. Fixed with
|
||||
`[assembly: CollectionBehavior(DisableTestParallelization = true)]`
|
||||
(`TestWebApplicationFactory.cs`) rather than redesigning the stores' DI shape for
|
||||
a test-only concern. Reran `dotnet test` 3× in a row to confirm the race was
|
||||
actually gone, not just less likely.
|
||||
- **Pinned `SQLitePCLRaw.bundle_e_sqlite3` to 3.0.3** — `Microsoft.EntityFrameworkCore.Sqlite`
|
||||
10.0.9's own transitive default (2.1.11) bundles a pre-3.50.2 SQLite with a known
|
||||
high-severity memory-corruption advisory (GHSA-2m69-gcr7-jv3q); 3.0.3 bundles a
|
||||
patched one and built/tested cleanly as a drop-in.
|
||||
- **`dotnet-ef` added to the existing `backend/dotnet-tools.json`** (not a new
|
||||
`.config/dotnet-tools.json`) — this repo already keeps its one CLI tool manifest
|
||||
there (`swashbuckle.aspnetcore.cli`); matched that convention.
|
||||
|
||||
Reference in New Issue
Block a user