mirror of
https://github.com/Sea-Haven-Industries/proposal-system.git
synced 2026-09-30 07:43:14 +00:00
67 lines
3.9 KiB
Markdown
67 lines
3.9 KiB
Markdown
# ADR 0004 — Optimistic concurrency: SHOC wire contract on a Postgres version column
|
||
|
||
- **Status:** Accepted (2026-07-13)
|
||
- **Decision owner:** Adam Moussa
|
||
- **Scope:** `api/` proposal aggregate, `shared/api-contracts`, all clients (web, mobile, suggestions Lambda)
|
||
|
||
## Context
|
||
|
||
SHOC-alignment Phase 6 ports shoc-backend's optimistic-concurrency convention
|
||
(PRs #10/#13–#18) to the proposal aggregate. SHOC's mechanics are built on SQL
|
||
Server `rowversion` (`byte[8]`, auto-rotated, base64 on the wire) with a
|
||
double-guard: pre-check the client token against the loaded row, stamp it as
|
||
EF's original value so the UPDATE's WHERE clause re-enforces it, and on a lost
|
||
race reload and embed the winner's state in a 409. PostgreSQL has no
|
||
`rowversion`; the candidates were the `xmin` system column or an explicit
|
||
version column.
|
||
|
||
## Decision
|
||
|
||
1. **Explicit `long Version` column** on `Proposals` and `LineItems`,
|
||
`IsConcurrencyToken`, additive migration with `DEFAULT 1`, incremented by
|
||
the mutating services. Not `xmin`: the xUnit suite runs on InMemory/SQLite
|
||
where xmin doesn't exist (SHOC needed an InMemory shim for the same
|
||
reason), the handbook expects a real, reversible migration, and xmin leaks
|
||
storage internals onto the wire.
|
||
2. **SHOC's wire contract verbatim.** Tokens are opaque base64 strings
|
||
(`RowVersionCodec`: 8-byte big-endian long — same shape as SHOC's
|
||
`"AQAAAAAAAAA="` tokens). Responses carry `rowVersion`; guarded requests
|
||
carry `proposalVersion`. Missing token → 422 `ProposalVersionRequired`;
|
||
malformed → 422 `InvalidRowVersion`; conflict → **409
|
||
`{ message, currentState }`** with the reloaded `ProposalResponse`
|
||
embedded; unguarded races → 409 `{ status, message, code }` fallback.
|
||
Both envelopes are deliberately **not** ProblemDetails (SHOC parity) and
|
||
serialize with the MVC pipeline's conventions (camelCase, string enums).
|
||
3. **One aggregate, one token.** Deviation from the drafted plan, forced by
|
||
the code: bulk line-item update is delete-all-and-recreate, so per-item
|
||
tokens are meaningless. The proposal token guards proposal fields, state
|
||
transitions, and the bulk replace; every line-item mutation
|
||
(create/bulk/delete) bumps the proposal version so nothing goes stale
|
||
silently. `LineItem.Version` exists (additive, on the wire) for future
|
||
per-item mutations only.
|
||
4. **Guard scope per SHOC precedent.** Updates and state transitions demand
|
||
the token; creates and deletes don't, but still bump the aggregate version
|
||
— a lost race there surfaces as the fallback 409 instead of a silent
|
||
overwrite (this includes `VendorProposalsController`'s vendor-cost
|
||
recalc). Internal writers (suggestions Lambda) fetch-and-echo the token
|
||
with one conflict retry.
|
||
5. **Caller contract:** the 409 `currentState` reload has no ownership
|
||
filter, so guard-reaching endpoints must stay admin-gated — enforced by
|
||
`GuardedEndpointAuthorizationTests`. Extend the guard with an ownership
|
||
predicate before wiring it to any dispatcher-reachable write.
|
||
6. **Audit atomicity (same phase):** `IAuditService.Stage` adds to the shared
|
||
context; every mutation stages before its single `SaveChangesAsync`, so
|
||
the domain change and its audit row commit or fail together. Self-saving
|
||
`LogAsync` remains for standalone events only.
|
||
|
||
## Consequences
|
||
|
||
- Breaking API change for mutating clients; web, mobile, and the suggestions
|
||
Lambda ship the token pass-through in the same change set.
|
||
- Deploys: migration auto-applies at API startup under `pg_advisory_lock`;
|
||
the column is additive with a default, so the previous Lambda version keeps
|
||
working against the migrated schema. Manual RDS snapshot before deploy;
|
||
down-script is two `DropColumn`s, to be tested against a snapshot-restored
|
||
copy before any production rollback.
|
||
- ADR 0002's boundary holds: the convention and wire contract converge with
|
||
SHOC; the platform (Postgres, integer column, explicit increments) does not.
|