From e55f3fe1e65599184feee42439c1b31daa1eadac Mon Sep 17 00:00:00 2001 From: Adam Moussa Date: Mon, 13 Jul 2026 21:21:19 -0400 Subject: [PATCH] docs: ADR 0004 (optimistic-concurrency convention) + README contract and deploy notes --- README.md | 4 ++ .../0004-optimistic-concurrency-convention.md | 67 +++++++++++++++++++ 2 files changed, 71 insertions(+) create mode 100644 docs/adr/0004-optimistic-concurrency-convention.md diff --git a/README.md b/README.md index 16b5f48..9a90cac 100644 --- a/README.md +++ b/README.md @@ -171,6 +171,10 @@ Two-layer auth architecture with defense-in-depth: **Internal API key:** Python Lambdas call the .NET API via a Lambda Function URL with AWS_IAM auth (bypasses API Gateway JWT check). The `InternalApiKeyMiddleware` validates the `X-Internal-Api-Key` header and assigns the `admins` role to the synthetic identity. Lambdas cache the API key from Secrets Manager with a 5-minute TTL. +**Optimistic concurrency (ADR 0004):** proposal responses carry an opaque `rowVersion` token; mutations of the proposal aggregate (update, approve, return-to-review, send, revise, bulk line-item update) require `proposalVersion` in the body. Missing token → 422 `ProposalVersionRequired`, malformed → 422 `InvalidRowVersion`, stale → **409 `{ message, currentState }`** with the reloaded proposal embedded (unguarded races → 409 `{ status, message, code }`). Line-item create/delete are token-less but bump the aggregate version. Audit rows commit atomically with their mutation (stage-then-single-SaveChanges). + +**Deploy note for schema changes:** EF migrations auto-apply at API startup under a `pg_advisory_lock`. Before deploying a migration: take a manual RDS snapshot; additive-only migrations are backward-compatible with the previous Lambda version. Test the down-script against a snapshot-restored copy before any production rollback. + ## Data Flow 1. Dispatcher submits proposal request (web or mobile) diff --git a/docs/adr/0004-optimistic-concurrency-convention.md b/docs/adr/0004-optimistic-concurrency-convention.md new file mode 100644 index 0000000..2182220 --- /dev/null +++ b/docs/adr/0004-optimistic-concurrency-convention.md @@ -0,0 +1,67 @@ +# 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.