Second template axis (org identity: letterhead, footer, signature, margins) server-side: OrgTemplateStore with JSON version history, publish/rollback, sent-brief version pinning, admin role + capability, 5 admin endpoints, org-logo upload category. FE seam widened only (Role/Capability unions, interceptor); WP-24/26 consume it. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
186 lines
11 KiB
Markdown
186 lines
11 KiB
Markdown
# WP-22 — Durable persistence (optional tier)
|
||
|
||
Status: done (556f2f4)
|
||
Phase: 5 — productie-volwassenheid
|
||
|
||
## Why
|
||
|
||
Every backend store (`ApplicationStore`, `DocumentStore`, `BriefStore`) is a
|
||
`static Dictionary` guarded by a single `lock` object, explicitly documented as
|
||
in-memory ("no DB", per `backend/README.md` and CLAUDE.md's own framing). Data —
|
||
including the audit log — is lost on every restart. This is a deliberate POC
|
||
simplification (CLAUDE.md lists "runtime DTO validation on every endpoint" and
|
||
similar as out-of-scope, and a database was never promised), but it's the one gap
|
||
that would visibly break the moment someone tries to run this as a real demo across
|
||
multiple sessions or deploys it anywhere that restarts (e.g. most PaaS platforms
|
||
recycle instances).
|
||
|
||
This WP is marked **optional tier** — lower priority than WP-18/19/20/21 — because
|
||
unlike auth/e2e/i18n/resilience, the current in-memory design is explicitly
|
||
documented and defensible for a POC. Do this when the POC needs to survive restarts
|
||
(demoing over multiple days, deploying somewhere with instance recycling), not
|
||
speculatively.
|
||
|
||
## Read first
|
||
|
||
- `backend/README.md` (the "in-memory seeded, no DB" framing to preserve or
|
||
supersede)
|
||
- `backend/src/BigRegister.Api/Data/ApplicationStore.cs`,
|
||
`backend/src/BigRegister.Api/Data/DocumentStore.cs`,
|
||
`backend/src/BigRegister.Api/Data/BriefStore.cs` — the three stores, each
|
||
`static Dictionary` + `lock`
|
||
- `backend/src/BigRegister.Api/Data/SeedData.cs` (current in-memory seed — becomes
|
||
a first-run DB seed)
|
||
- `docs/architecture/0001-bff-lite-decision-dtos.md` (confirm this WP doesn't touch
|
||
the decision-DTO contracts — persistence is purely behind the existing store
|
||
interfaces)
|
||
|
||
## Decisions (pre-made, don't relitigate)
|
||
|
||
- **SQLite + EF Core**, not a heavier database — matches the POC's zero-external-
|
||
infrastructure posture (no docker service to add, no connection string to manage
|
||
beyond a file path) while proving real persistence.
|
||
- **Persistence lives entirely behind the existing static-class store APIs** — the
|
||
public methods on `ApplicationStore`/`DocumentStore`/`BriefStore` keep their
|
||
signatures; only the implementation swaps from `Dictionary` to `DbContext`. No
|
||
endpoint or domain-rule code changes (`Program.cs`, `Domain/*`).
|
||
- **Seed on empty DB**, not on every startup — `SeedData` runs once (checked via
|
||
"is the DB empty") so restarts don't reset demo data, which is the entire point
|
||
of this WP.
|
||
- **Document bytes stay a deliberate exception** if storage size becomes a concern:
|
||
either store them as a BLOB column (simplest, consistent with "one DB, no extra
|
||
infra") or explicitly punt file bytes to disk with only metadata in SQLite —
|
||
decide based on actual seeded file sizes, don't over-engineer a blob-storage
|
||
abstraction for a POC.
|
||
- **Audit log becomes a real table**, not just "no longer volatile" — this closes
|
||
the "audit log is in-memory" gap named in the original gap analysis alongside
|
||
persistence, since it's the same static-dict problem in `DocumentStore.cs`.
|
||
|
||
## Files
|
||
|
||
- `backend/src/BigRegister.Api/BigRegister.Api.csproj` — add
|
||
`Microsoft.EntityFrameworkCore.Sqlite` + `Microsoft.EntityFrameworkCore.Design`.
|
||
- New `backend/src/BigRegister.Api/Data/AppDbContext.cs` — `DbSet`s mirroring the
|
||
three stores' current in-memory shapes (`StoredDocument`, `AuditEntry`, whatever
|
||
`ApplicationStore`/`BriefStore` hold internally — read those files first to avoid
|
||
redesigning the shape, just relocate it).
|
||
- `backend/src/BigRegister.Api/Data/ApplicationStore.cs`,
|
||
`DocumentStore.cs`, `BriefStore.cs` — convert static dictionary methods to
|
||
`DbContext`-backed queries; keep every public method signature identical (this is
|
||
the acceptance bar — a signature change means a caller in `Program.cs` or
|
||
`Domain/*` needs to change, which should be zero).
|
||
- `backend/src/BigRegister.Api/Data/SeedData.cs` — becomes "seed if empty" run once
|
||
at startup against the real DB.
|
||
- `backend/src/BigRegister.Api/Program.cs` — register `AppDbContext` (DI), run
|
||
migrations/`EnsureCreated` + conditional seed at startup.
|
||
- New EF Core migration (generated via `dotnet ef migrations add Initial`).
|
||
- `.gitignore` — exclude the runtime `.db` file (ship the migration, not the
|
||
database).
|
||
- `backend/README.md` — update "in-memory seeded, no DB" framing to describe the
|
||
SQLite file and its lifecycle (created/seeded on first run, persists thereafter,
|
||
delete the file to reset demo data).
|
||
- `docker-compose.yml` — mount a volume for the SQLite file so `docker compose up`
|
||
restarts don't lose data either (currently the `api-bin`/`api-obj` volumes exist
|
||
for build caching only, not data).
|
||
|
||
## Steps
|
||
|
||
1. Add the EF Core packages; define `AppDbContext` matching the current in-memory
|
||
record shapes exactly (no schema redesign in this WP).
|
||
2. Convert one store at a time (`DocumentStore` first — it's the smallest and has
|
||
the audit log, which is the most valuable win), keeping
|
||
`backend/tests/BigRegister.Tests/*` green after each conversion.
|
||
3. Wire `AppDbContext` + startup migration/seed in `Program.cs`.
|
||
4. Convert `ApplicationStore`, then `BriefStore`.
|
||
5. Update `docker-compose.yml` with a persistent volume; update `backend/README.md`.
|
||
6. Full backend test suite + a manual restart test: run the backend, create an
|
||
application, restart the process, confirm the application still exists.
|
||
|
||
## Acceptance criteria
|
||
|
||
- [x] All three stores are EF Core/SQLite-backed; no `static Dictionary` remains in
|
||
`Data/*.cs` for application/document/brief state.
|
||
- [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).
|
||
- [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` — 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
|
||
|
||
A production-grade database (Postgres/SQL Server) — SQLite is the deliberate,
|
||
right-sized choice for a POC that still wants to prove real persistence. Migrating
|
||
existing in-memory demo data on upgrade (a fresh SQLite file starts from
|
||
`SeedData`, same as today's in-memory start). Blob storage for document bytes
|
||
beyond a BLOB column (only revisit if seeded files are large enough to matter).
|
||
|
||
## Risks
|
||
|
||
EF Core's async patterns don't drop in as a 1:1 replacement for synchronous
|
||
dictionary lookups — endpoint handlers in `Program.cs` currently call store methods
|
||
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.
|