diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index d759234..e01f17b 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -105,6 +105,9 @@ User Backfilled to created_at for pre-existing rows in the add_email_verification migration, so accounts from before this phase are treated as verified) + version (int, default 1 — Phase 8 issue #49; optimistic-concurrency + counter, see §5.4. Bumped by every write to the account, including + email verification) created_at, updated_at EmailVerificationToken (Phase 8 issue #34) @@ -117,9 +120,12 @@ EmailVerificationToken (Phase 8 issue #34) PatientProfile (1:1 User) id, user_id, full_name, date_of_birth, guardian_user_id (nullable — see PRD edge case: minors) + version (int, default 1 — issue #49, §5.4) DoctorProfile (1:1 User) id, user_id, full_name, verification_status (enum: pending|verified|rejected) + version (int, default 1 — issue #49, §5.4. Bumped by the admin + verification transition too) specialties: DoctorSpecialty[] Specialty (controlled vocabulary, admin-managed) @@ -129,11 +135,24 @@ DoctorSpecialty (join table) id, doctor_profile_id, specialty_id FacilityProfile (1:1 User) - id, user_id, name, type (enum: lab|pharmacy), capabilities: FacilityCapability[] + id, user_id, name, type (enum: lab|pharmacy), version (int, default 1 — issue #49, §5.4) + capabilities: FacilityCapability[] FacilityCapability id, facility_profile_id, capability_type (enum: lab_test|medication_dispense), detail (e.g. test category) + version (int, default 1 — issue #49, §5.4) ``` +**`version` columns (Phase 8, issue #49):** added by +`backend/prisma/migrations/20261019090000_add_optimistic_concurrency/` as +`INTEGER NOT NULL DEFAULT 1` on exactly the mutable records above plus +`Appointment` (§4.2) and `PaymentIntent`. They are the storage half of the +`ETag`/`If-Match` convention in §5.4 and carry no meaning on their own — +nothing may treat a higher `version` as "more correct", and no code may +compare-then-write instead of guarding the UPDATE itself. Tables that are +append-only (`Prescription`, `LabOrder`) or immutable +(`Specialty`, `WalletLink`, `AuditEvent`, the export tables) deliberately do +**not** have one; the integration test asserts that set so a later change +can't quietly widen it. ### 4.2 Availability & Appointments ``` @@ -141,12 +160,16 @@ AvailabilitySlot id, doctor_profile_id, start_time, end_time, status (enum: open|held|booked) # Uniqueness/exclusion constraint at DB level on (doctor_profile_id, start_time, end_time) # to make double-booking a constraint violation, not an app-logic bug. + # No `version`: a slot is only ever moved by the conditional claim in §5.1, + not edited. Appointment id, patient_profile_id, doctor_profile_id, availability_slot_id (FK, unique) intake_id (FK -> IntakeForm) status (enum: scheduled|completed|cancelled|no_show) origin (enum: direct_booking|referral), referral_id (nullable FK -> Referral) + version (int, default 1 — issue #49, §5.4; no If-Match-guarded route exists + for appointments yet, but every future status write must move it) created_at, updated_at IntakeForm @@ -312,6 +335,48 @@ schemaVersion, exportId, generatedAt, sections: { : { version, items } } } 4. Facility updates its own `FulfillmentRecord` status; for labs, attaches `LabResult`. 5. Order's own `status` field is updated to `fulfilled` only via this facility-confirmed path — patient/doctor cannot self-mark it. +### 5.4 Mutable-record optimistic concurrency (Phase 8, issue #49) +Slot booking (§5.1) is protected by a conditional update, but nothing else was: a +doctor editing their profile in one tab and an admin in another, or two tabs +updating the same appointment, silently overwrote each other — last write wins, +and the loser never finds out. This is the convention that closes that whole +class of bug, and **new mutable-resource routes must follow it.** + +1. An individual mutable-resource read returns a strong `ETag` carrying the row's + integer `version`, e.g. `ETag: "4"` (also mirrored as `version` in the JSON + body). Collection reads deliberately do not get one: a client fetches the + individual resource before editing. +2. `PATCH`/`PUT` sends that exact value back in `If-Match`. Missing header → + `428 Precondition Required` (`code: 'IF_MATCH_REQUIRED'`); weak validator, a + validator list, or anything else that isn't one strong `""` → `400` + (`code: 'INVALID_IF_MATCH'`) rather than a guess at which member was meant; + a version that no longer matches → `412 Precondition Failed` + (`code: 'VERSION_CONFLICT'`). +3. The repository performs **one** conditional update — `UPDATE ... WHERE id = ? + AND version = ?` with `version = version + 1` in the same statement — and + returns the row that statement wrote. A zero-row result is a stale write. + Read-compare-write is forbidden: it reopens the exact race it closes, because + two callers can both read version 4 and both pass a check made before the + write. In Prisma that is `updateManyAndReturn` / `updateMany` with + `count === 1`, never `findUnique` → compare → `update`. +4. Everything else keeps working the way it did. Endpoints whose state + transition is *already* conditional (booking's slot claim, payment + fund/release/refund, doctor verification, token redemption) keep that status + guard and are **not** additionally `If-Match`-gated — two guards on one write + produce two different failure modes and the second one teaches clients + nothing. They do still increment `version`, and still publish the new `ETag`, + so a client can precondition whatever it does next. Every other write to a + versioned row increments it too, including system writes (email verification). +5. Applies to `User`, `PatientProfile`, `DoctorProfile`, `FacilityProfile`, + `FacilityCapability`, `Appointment` and `PaymentIntent` (columns documented in + §4.1/§4.2). Append-only `Prescription`/`LabOrder` are excluded by design — + a correction is a new row (`supersedes_*`), so there is nothing to lose. +6. Shared pieces live in `backend/src/middleware/optimistic-concurrency.ts` + (`requireIfMatch`, `expectedVersion`, `setVersionEtag`, `versionConflict`) so + every module answers with the same status, message and code. Proof: + `backend/tests/integration/optimistic-concurrency.test.mts`, including eight + simultaneous writes on one version where exactly one succeeds. + ## 6. Hard questions (architecture-level — see PRD §8 for product-level) ### 6.1 What do we think would break? @@ -320,6 +385,7 @@ schemaVersion, exportId, generatedAt, sections: { : { version, items } } } 3. Polymorphic `FulfillmentRecord.order_id` (§4.7) — polymorphic FKs are a known footgun for referential integrity in Postgres. Needs either two separate tables (`PrescriptionFulfillment`, `LabOrderFulfillment`) or a check-constraint + application-level guard. **Current schema sketch is a simplification that should be revisited before implementation**, not treated as final. 4. JWT refresh rotation without a revocation list — a stolen refresh token has no clean kill switch until this is added. 5. **Discovered 2026-10-15 while building email verification (Phase 8, issue #34):** the email/SMS provider is still undecided (Phase 6, issue #17), so verification links can't actually be delivered in a hosted environment yet — `backend/src/modules/auth/services/email-dispatcher.ts` is a no-op seam until a provider is integrated. Until then `backend/scripts/mark-email-verified.mjs` is the documented out-of-band path for an operator with DB access. Not a reason to skip the guard: the guard is what makes the flow safe once delivery exists, and shipping it locked-off would just mean shipping it untested. +6. **Resolved 2026-10-19 (issue #49):** silent last-write-wins on every mutable record that wasn't the slot claim — a doctor in one tab and an admin in another, or two tabs editing one appointment, each wrote and the second erased the first with no signal to either. §5.4 now makes the compare and the write one conditional statement behind an `If-Match` precondition. **Still open:** the convention is only *enforced* where a route opts in, so a future mutable route that forgets `requireIfMatch` silently opts out — there is no lint or schema-level way to catch that yet, and ESLint itself is still untracked (issue #28). ### 6.2 What edge cases are we missing (architecture-level)? 1. What happens to `AvailabilitySlot.status = 'held'` if a booking transaction fails/times out mid-flight — is there a TTL to release the hold? (Needed or slots leak into permanent limbo.) diff --git a/ARCHITECTURE_ESSENTIALS.md b/ARCHITECTURE_ESSENTIALS.md index bdbb1d4..ca76986 100644 --- a/ARCHITECTURE_ESSENTIALS.md +++ b/ARCHITECTURE_ESSENTIALS.md @@ -18,6 +18,7 @@ Quick-reference only. Full reasoning lives in `ARCHITECTURE.md`. Full product sc 4. **Cross-module access only through a module's service interface.** Never import another module's `repositories/*` directly. 5. **Facilities write to their own `FulfillmentRecord` only** — never directly to `Prescription.status` / `LabOrder.status`. 6. Every module = `routes/ controllers/ services/ repositories/`. Keep that shape. +7. **Mutable-record edits use optimistic concurrency (Phase 8, issue #49).** Individual-resource read returns a strong version `ETag` (`"4"`); PATCH/PUT must send it in `If-Match` — missing is `428`, weak/list/garbage is `400`, stale is `412`. Write it as ONE conditional `UPDATE ... WHERE id = ? AND version = ?` that increments `version` (Prisma: `updateManyAndReturn`); read-compare-write is forbidden. Status transitions that are *already* conditional (booking, payment status, verification) keep their own guard, don't get a second `If-Match` one, but still bump + republish the version. Append-only `Prescription`/`LabOrder` are excluded. See ARCHITECTURE.md §5.4. ## Data model map (see ARCHITECTURE.md §4 for full field lists) `User` → `PatientProfile | DoctorProfile | FacilityProfile` @@ -31,6 +32,8 @@ Quick-reference only. Full reasoning lives in `ARCHITECTURE.md`. Full product sc `PatientProfile` → `PatientExport` → `PatientExportChunk` (Phase 8, issue #40 — right-of-access export jobs + their chunked JSON bundle; each module contributes a section via its own service, registered in `src/export-sections.ts` — see ARCHITECTURE.md §3.1). `FacilityProfile` → `WalletLink` (payout address) ; `PatientProfile`/`FacilityProfile` → `PaymentIntent` (opaque `orderType`/`orderId` — see docs/adr/0004) +Mutable records above (plus `Appointment` and `PaymentIntent`) carry a `version` int for the `ETag`/`If-Match` convention — hard rule 7, ARCHITECTURE.md §5.4. Mutable profile fields a new route exposes need a conditional write, not just a `.update()`. + ## Payments (Phase 5.5 — see docs/adr/0004-custodial-payments-v1.md) - **Custodial v1**: `docforum-core` holds its own Stellar payer + releaser identities (`PLATFORM_PAYER_SECRET`/`PLATFORM_RELEASER_SECRET` env vars). Patients never hold or sign with a wallet — the only reason this is possible without violating `docforum-web`'s "no direct Stellar calls" hard rule. - `WalletLink` is scoped to the **facility** (the on-chain payee), not the patient. diff --git a/ROADMAP.md b/ROADMAP.md index dd2c646..883a67c 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -118,6 +118,7 @@ for status and issue links. - [ ] JWT refresh revocation list (ARCHITECTURE.md §6.1.4). Tracked as [issue #18](https://github.com/DocForum/docforum-core/issues/18) (Medium, 150 pts) — unblocked now, operates on Phase 1's auth module. - [ ] Held-slot TTL/release job (ARCHITECTURE.md §6.2.1). Tracked as [issue #19](https://github.com/DocForum/docforum-core/issues/19) (Medium, 150 pts) — unblocked now, operates on Phase 1's `AvailabilitySlot`. - [x] Access-control test suite: who may read intake, consultation and payment data. Integration tests (`tests/integration/access-control.test.mts`) establishing the test harness and access-control patterns for the three PHI data categories; intake/consultation test cases are scaffolded as placeholders until Phase 2/3 routes exist, payment-intent access cases exercise the live admin-gated routes. Tracked as [issue #44](https://github.com/DocForum/docforum-core/issues/44) — **Done**. +- [ ] Optimistic concurrency (`version` + `If-Match`) for mutable records. A `version` int on `User`, `PatientProfile`, `DoctorProfile`, `FacilityProfile`, `FacilityCapability`, `Appointment` and `PaymentIntent`; individual reads publish it as a strong `ETag`; `PATCH`/`PUT` require that exact `If-Match` (`428` missing, `400` weak/list/garbage, `412` stale); writes are a single conditional `UPDATE ... WHERE id = ? AND version = ?` that increments, never read-compare-write. Transitions that are already conditional (booking, payment status, verification) keep their own guard and are **not** given a second one. Append-only `Prescription`/`LabOrder` excluded. Convention in ARCHITECTURE.md §5.4. Tracked as [issue #49](https://github.com/DocForum/docforum-core/issues/49) (High, 200 pts) — **in review**, branch `feat/optimistic-concurrency-version-if-match-for`. Foundation only: the one existing editable resource (`DoctorProfile`, via `PATCH /doctors/me`) is wired end-to-end as the reference implementation; every new mutable-resource route in Phases 2–5 must adopt the convention. - [ ] Privacy access-control audit: confirm intake/consultation notes are visible only per PRD §7 (patient, treating doctor(s) in-thread, fulfilling facility for that specific order only) — write tests, not just code review. Still blocked — meaningfully auditable only once Phase 3/4/5 access paths exist. Not yet scoped into an issue. - [ ] Decide and document what's explicitly out of scope for v1 launch vs. genuinely blocking (e.g., drug-interaction checking, PRD §8.2.8 — needs an explicit go/no-go, not silence). Tracked as [issue #20](https://github.com/DocForum/docforum-core/issues/20) (Trivial, 100 pts) — a decision task, unblocked now. @@ -405,3 +406,23 @@ for status and issue links. observed state, so a racing second action gets 409 instead of silently overwriting). Rollback coupling proven with a real injected DB failure, not a mock. Phase 3/4 wiring explicitly left pending. +- 2026-10-19 — Issue #49 (optimistic concurrency) opened as a PR: a `version` + int on the seven mutable models (`User`, `PatientProfile`, `DoctorProfile`, + `FacilityProfile`, `FacilityCapability`, `Appointment`, `PaymentIntent`), + individual-resource reads publishing it as a strong `ETag`, and `PATCH`/ + `PUT` requiring that exact `If-Match` — `428` missing, `400` for a weak + validator or a list, `412` stale. The write is one conditional + `UPDATE ... WHERE id = ? AND version = ?` with the increment in the same + statement (Prisma `updateManyAndReturn`, so the response body is the row + that statement wrote), which is what makes eight simultaneous edits on one + version resolve to exactly one winner in the integration test. Transitions + that are already conditional — booking's slot claim, payment + fund/release/refund, doctor verification — deliberately keep only their own + status guard instead of gaining a second, unrelated one; they do bump and + republish the version. Append-only `Prescription`/`LabOrder` are excluded by + design, and the test asserts the exact set of tables carrying a `version` so + it can't quietly widen. `PATCH /doctors/me` is the reference implementation + of the convention (ARCHITECTURE.md §5.4) — there was no existing + patient-profile or appointment edit route to convert, so later phases' + mutable routes adopt the convention as they're built rather than being + retrofitted here. diff --git a/backend/prisma/migrations/20261019090000_add_optimistic_concurrency/migration.sql b/backend/prisma/migrations/20261019090000_add_optimistic_concurrency/migration.sql new file mode 100644 index 0000000..13f0dba --- /dev/null +++ b/backend/prisma/migrations/20261019090000_add_optimistic_concurrency/migration.sql @@ -0,0 +1,31 @@ +-- Optimistic concurrency for mutable records (Phase 8, issue #49). +-- +-- One integer per row, defaulting to 1 so existing rows are valid at their +-- current state without a backfill. It is the storage half of the ETag/If-Match +-- convention in ARCHITECTURE.md §5.4 and is meaningless on its own: nothing may +-- compare-then-write, the compare has to live in the UPDATE's WHERE clause. +-- +-- Deliberately NOT added to Prescription/LabOrder (append-only — a correction is +-- a new row, so there is no update to lose) or to immutable tables +-- (AvailabilitySlot, Specialty, WalletLink, AuditEvent, the export tables). + +-- AlterTable +ALTER TABLE "User" ADD COLUMN "version" INTEGER NOT NULL DEFAULT 1; + +-- AlterTable +ALTER TABLE "PatientProfile" ADD COLUMN "version" INTEGER NOT NULL DEFAULT 1; + +-- AlterTable +ALTER TABLE "DoctorProfile" ADD COLUMN "version" INTEGER NOT NULL DEFAULT 1; + +-- AlterTable +ALTER TABLE "FacilityProfile" ADD COLUMN "version" INTEGER NOT NULL DEFAULT 1; + +-- AlterTable +ALTER TABLE "FacilityCapability" ADD COLUMN "version" INTEGER NOT NULL DEFAULT 1; + +-- AlterTable +ALTER TABLE "Appointment" ADD COLUMN "version" INTEGER NOT NULL DEFAULT 1; + +-- AlterTable +ALTER TABLE "PaymentIntent" ADD COLUMN "version" INTEGER NOT NULL DEFAULT 1; \ No newline at end of file diff --git a/backend/prisma/schema.prisma b/backend/prisma/schema.prisma index 7cfb28b..09e4322 100644 --- a/backend/prisma/schema.prisma +++ b/backend/prisma/schema.prisma @@ -99,6 +99,7 @@ enum Urgency { model User { id String @id @default(uuid()) + version Int @default(1) email String @unique phone String? passwordHash String @@ -169,6 +170,7 @@ model LoginAttempt { model PatientProfile { id String @id @default(uuid()) + version Int @default(1) userId String @unique fullName String dateOfBirth DateTime? @@ -255,6 +257,7 @@ model PatientExportChunk { model DoctorProfile { id String @id @default(uuid()) + version Int @default(1) userId String @unique fullName String verificationStatus VerificationStatus @default(pending) @@ -290,6 +293,7 @@ model DoctorSpecialty { model FacilityProfile { id String @id @default(uuid()) + version Int @default(1) userId String @unique name String type FacilityType @@ -304,6 +308,7 @@ model FacilityProfile { model FacilityCapability { id String @id @default(uuid()) + version Int @default(1) facilityProfileId String capabilityType CapabilityType // e.g. a lab test category, or a medication class — free text on purpose, @@ -339,6 +344,7 @@ model AvailabilitySlot { model Appointment { id String @id @default(uuid()) + version Int @default(1) patientProfileId String doctorProfileId String availabilitySlotId String @unique @@ -475,6 +481,7 @@ enum OrderTypeForPayment { model PaymentIntent { id String @id @default(uuid()) + version Int @default(1) orderType OrderTypeForPayment // Opaque — Prescription/LabOrder don't exist as real records yet // (Phase 4). Not validated against anything until then. diff --git a/backend/src/middleware/optimistic-concurrency.ts b/backend/src/middleware/optimistic-concurrency.ts new file mode 100644 index 0000000..d4929a0 --- /dev/null +++ b/backend/src/middleware/optimistic-concurrency.ts @@ -0,0 +1,84 @@ +// Optimistic concurrency for mutable records (Phase 8, issue #49). +// See ARCHITECTURE.md §5.4 for the convention every new module must follow. +// +// The whole point is that the compare and the write are ONE statement +// (`UPDATE ... WHERE id = ? AND version = ?` + `version = version + 1`). +// A read-compare-write would reopen the exact race this closes: two callers +// can both read version 4, both pass the check, and the second write +// silently destroys the first. +import type { NextFunction, Request, Response } from 'express'; +import { HttpError } from '../http-error'; + +declare global { + // eslint-disable-next-line @typescript-eslint/no-namespace + namespace Express { + interface Request { + expectedVersion?: number; + } + } +} + +/** + * A weak validator (`W/"4"`) or a list (`"4", "7"`) is refused rather than + * parsed: this API only ever issues one strong validator per resource, and + * guessing which member of a list the caller meant is how silent overwrites + * come back. + */ +const VERSION_ETAG = /^"([1-9]\d*)"$/; + +/** Strong `ETag` carrying the row's integer version, e.g. `ETag: "4"`. + * Call it on the individual-resource read; collection reads deliberately + * don't get one (§5.4.5). */ +export function setVersionEtag(res: Response, version: number) { + res.set('ETag', `"${version}"`); +} + +/** Rejects a mutable-resource write that doesn't carry the version it read. + * Must run after `authenticate`/`requireRole` so an unauthorised caller gets + * 401/403 rather than being told the resource exists. */ +export function requireIfMatch(req: Request, _res: Response, next: NextFunction) { + const value = req.get('If-Match'); + if (!value) { + throw new HttpError( + 428, + 'If-Match is required. Read the resource first and retry with its ETag in the If-Match header.', + 'IF_MATCH_REQUIRED', + ); + } + + const match = VERSION_ETAG.exec(value.trim()); + const version = match ? Number(match[1]) : Number.NaN; + if (!Number.isSafeInteger(version)) { + throw new HttpError( + 400, + 'If-Match must be the strong version ETag returned by this API, e.g. If-Match: "4".', + 'INVALID_IF_MATCH', + ); + } + + req.expectedVersion = version; + next(); +} + +/** The version the caller claimed. Read by a controller after + * `requireIfMatch` has run; throws rather than defaulting to 0, because a + * missing precondition silently skipping the guard would reintroduce the + * lost-update bug this exists to prevent. */ +export function expectedVersion(req: Request): number { + const version = req.expectedVersion; + if (typeof version !== 'number') { + throw new Error('requireIfMatch must run before expectedVersion().'); + } + return version; +} + +/** The error a repository's zero-row conditional update means: someone else + * moved the record since the caller read it. Shared so every module returns + * the same status, message and code. */ +export function versionConflict() { + return new HttpError( + 412, + 'This record changed since you read it. Fetch it again and reapply your change.', + 'VERSION_CONFLICT', + ); +} \ No newline at end of file diff --git a/backend/src/modules/auth/repositories/index.ts b/backend/src/modules/auth/repositories/index.ts index a4e1031..4d920db 100644 --- a/backend/src/modules/auth/repositories/index.ts +++ b/backend/src/modules/auth/repositories/index.ts @@ -95,7 +95,7 @@ export async function consumeVerificationToken(tokenId: string, consumedAt: Date } export function markUserEmailVerified(userId: string, verifiedAt: Date) { - return prisma.user.update({ where: { id: userId }, data: { emailVerifiedAt: verifiedAt } }); + return prisma.user.update({ where: { id: userId }, data: { emailVerifiedAt: verifiedAt, version: { increment: 1 } } }); } // --- Login throttling (Phase 8, issue #35) ------------------------------- diff --git a/backend/src/modules/doctors/controllers/index.ts b/backend/src/modules/doctors/controllers/index.ts index dddef28..600897f 100644 --- a/backend/src/modules/doctors/controllers/index.ts +++ b/backend/src/modules/doctors/controllers/index.ts @@ -1,13 +1,23 @@ import type { Request, Response } from 'express'; import * as doctorsService from '../services'; -import { setVerificationStatusSchema } from '../validation'; +import { setVerificationStatusSchema, updateDoctorProfileSchema } from '../validation'; import { HttpError } from '../../../http-error'; import { getParam } from '../../../request-params'; import { auditActor } from '../../../middleware/request-id'; +import { expectedVersion, setVersionEtag } from '../../../middleware/optimistic-concurrency'; export async function getMe(req: Request, res: Response) { if (!req.user) throw new HttpError(401, 'Authentication required.'); const profile = await doctorsService.getMyDoctorProfile(req.user.id); + setVersionEtag(res, profile.version); + res.status(200).json(profile); +} + +export async function updateMe(req: Request, res: Response) { + if (!req.user) throw new HttpError(401, 'Authentication required.'); + const { fullName } = updateDoctorProfileSchema.parse(req.body); + const profile = await doctorsService.updateMyDoctorProfile(req.user.id, expectedVersion(req), fullName); + setVersionEtag(res, profile.version); res.status(200).json(profile); } diff --git a/backend/src/modules/doctors/repositories/index.ts b/backend/src/modules/doctors/repositories/index.ts index 6b99f81..866299e 100644 --- a/backend/src/modules/doctors/repositories/index.ts +++ b/backend/src/modules/doctors/repositories/index.ts @@ -11,6 +11,25 @@ export function findDoctorProfileById(id: string, db: DbClient = prisma) { return db.doctorProfile.findUnique({ where: { id } }); } +/** + * The optimistic-locking write (ARCHITECTURE.md §5.4, issue #49). + * + * One statement: the row is only updated if it still carries the version the + * caller read, and that same statement bumps it. `updateManyAndReturn` gives + * back the row *this* statement wrote, so the response the caller sees is the + * state it produced — a re-read would be a second window in which someone + * else's write could land first. Null means the guard didn't match: the + * caller is stale and must re-read. + */ +export function updateDoctorProfile(id: string, version: number, fullName: string) { + return prisma.doctorProfile + .updateManyAndReturn({ + where: { id, version }, + data: { fullName, version: { increment: 1 } }, + }) + .then((rows) => rows[0] ?? null); +} + export function findDoctorSummariesByIds(ids: string[]) { return prisma.doctorProfile.findMany({ where: { id: { in: ids } }, select: { id: true, fullName: true } }); } @@ -29,5 +48,5 @@ export async function lockVerificationStatus( } export function updateVerificationStatus(db: DbClient, id: string, status: VerificationStatus) { - return db.doctorProfile.update({ where: { id }, data: { verificationStatus: status } }); + return db.doctorProfile.update({ where: { id }, data: { verificationStatus: status, version: { increment: 1 } } }); } diff --git a/backend/src/modules/doctors/routes/index.ts b/backend/src/modules/doctors/routes/index.ts index 7583e3f..95f3ae2 100644 --- a/backend/src/modules/doctors/routes/index.ts +++ b/backend/src/modules/doctors/routes/index.ts @@ -1,10 +1,12 @@ import { Router } from 'express'; import { authenticate, requireRole } from '../../../middleware/auth'; +import { requireIfMatch } from '../../../middleware/optimistic-concurrency'; import * as doctorsController from '../controllers'; export const doctorsRouter = Router(); doctorsRouter.get('/me', authenticate, requireRole('doctor'), doctorsController.getMe); +doctorsRouter.patch('/me', authenticate, requireRole('doctor'), requireIfMatch, doctorsController.updateMe); doctorsRouter.patch( '/:doctorProfileId/verification', authenticate, diff --git a/backend/src/modules/doctors/services/index.ts b/backend/src/modules/doctors/services/index.ts index 704dcd9..b54a407 100644 --- a/backend/src/modules/doctors/services/index.ts +++ b/backend/src/modules/doctors/services/index.ts @@ -1,5 +1,6 @@ import { HttpError } from '../../../http-error'; import { prisma } from '../../../db/prisma-client'; +import { versionConflict } from '../../../middleware/optimistic-concurrency'; import * as auditService from '../../audit/services'; import * as doctorsRepo from '../repositories'; import type { VerificationStatus } from '@prisma/client'; @@ -10,6 +11,19 @@ export async function getMyDoctorProfile(userId: string) { return profile; } +/** + * Renames the caller's own profile, guarded by the version they read + * (ARCHITECTURE.md §5.4). The `version` the caller sent is only used to find + * the profile's id — the guard itself is the repository's single conditional + * UPDATE, never a comparison made here. + */ +export async function updateMyDoctorProfile(userId: string, version: number, fullName: string) { + const current = await getMyDoctorProfile(userId); + const profile = await doctorsRepo.updateDoctorProfile(current.id, version, fullName); + if (!profile) throw versionConflict(); + return profile; +} + export async function getDoctorProfileById(doctorProfileId: string) { const profile = await doctorsRepo.findDoctorProfileById(doctorProfileId); if (!profile) throw new HttpError(404, 'Doctor profile not found.'); diff --git a/backend/src/modules/doctors/validation.ts b/backend/src/modules/doctors/validation.ts index f07df6a..8c312a5 100644 --- a/backend/src/modules/doctors/validation.ts +++ b/backend/src/modules/doctors/validation.ts @@ -3,3 +3,7 @@ import { z } from 'zod'; export const setVerificationStatusSchema = z.object({ status: z.enum(['pending', 'verified', 'rejected']), }); + +export const updateDoctorProfileSchema = z.object({ + fullName: z.string().trim().min(1).max(200), +}).strict(); diff --git a/backend/src/modules/payments/controllers/index.ts b/backend/src/modules/payments/controllers/index.ts index 14e1a7f..e1d3180 100644 --- a/backend/src/modules/payments/controllers/index.ts +++ b/backend/src/modules/payments/controllers/index.ts @@ -4,6 +4,7 @@ import { linkWalletSchema, createPaymentIntentSchema } from '../validation'; import { HttpError } from '../../../http-error'; import { getParam } from '../../../request-params'; import { auditActor } from '../../../middleware/request-id'; +import { setVersionEtag } from '../../../middleware/optimistic-concurrency'; // BigInt doesn't serialize via JSON.stringify by default — every response // here that carries one converts to a string explicitly. @@ -18,6 +19,9 @@ function serializeIntent(intent: { escrowId: bigint | null; stellarTxHash: string | null; sorobanContractId: string | null; + // Phase 8, issue #49: optimistic-concurrency version, surfaced in the body + // and as the ETag so a client can precondition its next write. + version: number; createdAt: Date; updatedAt: Date; }) { @@ -47,29 +51,38 @@ export async function getOwnWallet(req: Request, res: Response) { export async function createIntent(req: Request, res: Response) { const input = createPaymentIntentSchema.parse(req.body); const intent = await paymentsService.createPaymentIntent(input); + setVersionEtag(res, intent.version); res.status(201).json(serializeIntent(intent)); } export async function getIntent(req: Request, res: Response) { const id = getParam(req, 'id'); const intent = await paymentsService.getPaymentIntent(id); + setVersionEtag(res, intent.version); res.status(200).json(serializeIntent(intent)); } +/** fund/release/refund keep their own conditional status guard (issue #43) — + * they are deliberately NOT additionally If-Match-gated (§5.4.4) — but they + * still publish the resulting version so a client can precondition whatever + * it does next. */ export async function fundIntent(req: Request, res: Response) { const id = getParam(req, 'id'); const intent = await paymentsService.fundPaymentIntent(auditActor(req), id); + setVersionEtag(res, intent.version); res.status(200).json(serializeIntent(intent)); } export async function releaseIntent(req: Request, res: Response) { const id = getParam(req, 'id'); const intent = await paymentsService.releasePaymentIntent(auditActor(req), id); + setVersionEtag(res, intent.version); res.status(200).json(serializeIntent(intent)); } export async function refundIntent(req: Request, res: Response) { const id = getParam(req, 'id'); const intent = await paymentsService.refundPaymentIntent(auditActor(req), id); + setVersionEtag(res, intent.version); res.status(200).json(serializeIntent(intent)); } diff --git a/backend/src/modules/payments/repositories/index.ts b/backend/src/modules/payments/repositories/index.ts index bfc8c32..9c9998a 100644 --- a/backend/src/modules/payments/repositories/index.ts +++ b/backend/src/modules/payments/repositories/index.ts @@ -68,7 +68,7 @@ export async function markEscrowed( ) { const result = await tx.paymentIntent.updateMany({ where: { id, status: 'created' }, - data: { status: 'escrowed', ...input }, + data: { status: 'escrowed', ...input, version: { increment: 1 } }, }); return result.count === 1 ? tx.paymentIntent.findUniqueOrThrow({ where: { id } }) : null; } @@ -79,6 +79,9 @@ export async function markStatus( from: PaymentIntentStatus, to: PaymentIntentStatus, ) { - const result = await tx.paymentIntent.updateMany({ where: { id, status: from }, data: { status: to } }); + const result = await tx.paymentIntent.updateMany({ + where: { id, status: from }, + data: { status: to, version: { increment: 1 } }, + }); return result.count === 1 ? tx.paymentIntent.findUniqueOrThrow({ where: { id } }) : null; } diff --git a/backend/tests/integration/optimistic-concurrency.test.mts b/backend/tests/integration/optimistic-concurrency.test.mts new file mode 100644 index 0000000..bf0503b --- /dev/null +++ b/backend/tests/integration/optimistic-concurrency.test.mts @@ -0,0 +1,359 @@ +// Integration tests for optimistic concurrency on mutable records +// (Phase 8, issue #49; convention documented in ARCHITECTURE.md §5.4). +// +// This is the lost-update proof: two actors read the same record, both +// decide to write, and the API must let exactly one of them through and +// answer the other with 412 instead of silently discarding their change. +// Self-contained, same embedded-Postgres pattern as booking.test.mts, on its +// own port (5443). The escrow SDK is faked — on-chain behaviour is the +// testnet suite's job. +import { execFileSync } from 'node:child_process'; +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { afterAll, beforeAll, describe, expect, it } from 'vitest'; +import request from 'supertest'; +import jwt from 'jsonwebtoken'; +import EmbeddedPostgres from 'embedded-postgres'; + +const BACKEND_ROOT = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '../..'); +const PORT = 5443; +const runId = Date.now(); +const TX_HASH = 'd'.repeat(64); + +const dataDir = fs.mkdtempSync(path.join(os.tmpdir(), 'docforum-concurrency-pg-')); +const pg = new EmbeddedPostgres({ + databaseDir: dataDir, + user: 'docforum', + password: 'docforum', + port: PORT, + persistent: false, +}); +const databaseUrl = `postgresql://docforum:docforum@localhost:${PORT}/docforum_concurrency_test`; + +type App = ReturnType; +type Prisma = typeof import('../../src/db/prisma-client.js').prisma; + +const emailedTokens = new Map(); + +describe('optimistic concurrency (version + If-Match)', () => { + let app: App; + let prisma: Prisma; + let admin: { id: string; token: string }; + let doctor: { userId: string; profileId: string; token: string }; + /** A second doctor, so the admin verification test can't move the version + * the PATCH /doctors/me assertions are built on. */ + let otherDoctor: { userId: string; profileId: string; token: string }; + let patient: { userId: string; profileId: string; token: string }; + let facilityId: string; + let slotId: string; + + const adminAuth = () => ({ Authorization: `Bearer ${admin.token}` }); + const asUser = (d: { token: string }) => ({ Authorization: `Bearer ${d.token}` }); + + async function signup(role: 'patient' | 'doctor', label: string) { + const email = `${label}-${runId}@example.com`; + const res = await request(app) + .post('/auth/signup') + .send({ role, email, password: 'longenough123', fullName: `${label} Fullname` }); + expect(res.status).toBe(200); + expect((await request(app).post('/auth/verify-email').send({ token: emailedTokens.get(email) })).status).toBe(200); + return { userId: res.body.user.id as string, token: res.body.accessToken as string, email }; + } + + async function createIntent() { + return prisma.paymentIntent.create({ + data: { + orderType: 'lab_order', + orderId: `order-${Math.random()}`, + patientProfileId: patient.profileId, + facilityId, + amountStroops: 5000n, + }, + }); + } + + beforeAll(async () => { + await pg.initialise(); + await pg.start(); + await pg.createDatabase('docforum_concurrency_test'); + execFileSync('npx', ['prisma', 'migrate', 'deploy'], { + cwd: BACKEND_ROOT, + env: { ...process.env, DATABASE_URL: databaseUrl }, + stdio: 'pipe', + }); + + process.env.DATABASE_URL = databaseUrl; + process.env.PLATFORM_PAYER_SECRET = `CCONCPPAYERSENTINEL${runId}`; + process.env.PLATFORM_RELEASER_SECRET = `CCONCPRELEASERSENTINEL${runId}`; + process.env.ESCROW_CONTRACT_ID = 'CTESTCONTRACT'; + process.env.ESCROW_TOKEN_ID = 'CTESTTOKEN'; + + const appModule = await import('../../src/app.js'); + const dbModule = await import('../../src/db/prisma-client.js'); + const authServices = await import('../../src/modules/auth/services/index.js'); + const paymentsServices = await import('../../src/modules/payments/services/index.js'); + app = appModule.createApp(); + prisma = dbModule.prisma; + + authServices.setEmailSender(async (message) => { + const match = /[?&]token=([^&\s]+)/.exec(message.text); + if (match) emailedTokens.set(message.to, decodeURIComponent(match[1])); + }); + + class FakeEscrowClient { + async createEscrow() { + return { escrowId: 501n, txHash: TX_HASH }; + } + async release() {} + async refund() {} + } + paymentsServices.setEscrowSdkForTests({ + Keypair: { fromSecret: () => ({ publicKey: () => 'GPLATFORMRELEASERPUBLICKEY' }) }, + EscrowClient: FakeEscrowClient, + } as unknown as import('../../src/modules/payments/services/index.js').EscrowSdkModule); + + const adminUser = await prisma.user.create({ + data: { email: `admin-${runId}@example.com`, passwordHash: 'x', role: 'admin', emailVerifiedAt: new Date() }, + }); + admin = { + id: adminUser.id, + token: jwt.sign({ sub: adminUser.id, role: 'admin' }, 'test-access-secret', { expiresIn: '15m' }), + }; + + const doctorUser = await signup('doctor', 'cc-doctor'); + doctor = { ...doctorUser, profileId: (await prisma.doctorProfile.findUniqueOrThrow({ where: { userId: doctorUser.userId } })).id }; + const otherDoctorUser = await signup('doctor', 'cc-doctor-other'); + otherDoctor = { + ...otherDoctorUser, + profileId: (await prisma.doctorProfile.findUniqueOrThrow({ where: { userId: otherDoctorUser.userId } })).id, + }; + const patientUser = await signup('patient', 'cc-patient'); + patient = { + ...patientUser, + profileId: (await prisma.patientProfile.findUniqueOrThrow({ where: { userId: patientUser.userId } })).id, + }; + + const tomorrow = new Date(Date.now() + 24 * 60 * 60 * 1000).toISOString().slice(0, 10); + await request(app) + .post(`/availability/doctors/${doctor.profileId}/slots/generate`) + .set(asUser(doctor)) + .send({ date: tomorrow, startHour: 9, endHour: 12, slotLengthMinutes: 30 }); + slotId = ((await request(app).get(`/availability/doctors/${doctor.profileId}/slots`)).body as { id: string }[])[0]!.id; + + const facilityUser = await prisma.user.create({ data: { email: `lab-${runId}@example.com`, passwordHash: 'x', role: 'facility' } }); + facilityId = (await prisma.facilityProfile.create({ data: { userId: facilityUser.id, name: 'Concurrency Lab', type: 'lab' } })).id; + await prisma.walletLink.create({ data: { facilityId, stellarPublicKey: `G${'B'.repeat(55)}` } }); + }, 180_000); + + afterAll(async () => { + const authServices = await import('../../src/modules/auth/services/index.js'); + const paymentsServices = await import('../../src/modules/payments/services/index.js'); + authServices.resetEmailSender(); + paymentsServices.setEscrowSdkForTests(undefined); + await prisma?.$disconnect(); + await pg.stop(); + fs.rmSync(dataDir, { recursive: true, force: true }); + }); + + describe('doctor profile — PATCH /doctors/me', () => { + it('publishes a strong version ETag on the individual resource read', async () => { + const res = await request(app).get('/doctors/me').set(asUser(doctor)); + expect(res.status).toBe(200); + expect(res.body.version).toBe(1); + expect(res.headers.etag).toBe('"1"'); + }); + + it('refuses a write with no If-Match and says how to recover (428)', async () => { + const res = await request(app).patch('/doctors/me').set(asUser(doctor)).send({ fullName: 'No Precondition' }); + expect(res.status).toBe(428); + expect(res.body.code).toBe('IF_MATCH_REQUIRED'); + expect(res.body.message).toMatch(/If-Match/); + // Nothing changed: a refused precondition is not a partial write. + expect((await prisma.doctorProfile.findUniqueOrThrow({ where: { id: doctor.profileId } })).fullName).not.toBe( + 'No Precondition', + ); + }); + + it('rejects a weak validator, a list and garbage with 400 rather than guessing', async () => { + for (const value of ['W/"2"', '"2", "3"', '2', 'abc', '"-1"', '"0"']) { + const res = await request(app).patch('/doctors/me').set(asUser(doctor)).set('If-Match', value).send({ fullName: 'Bad Tag' }); + expect(res.status, `If-Match: ${value}`).toBe(400); + expect(res.body.code).toBe('INVALID_IF_MATCH'); + } + expect((await prisma.doctorProfile.findUniqueOrThrow({ where: { id: doctor.profileId } })).version).toBe(1); + }); + + it('applies the write and increments the version in the same response', async () => { + const current = await request(app).get('/doctors/me').set(asUser(doctor)); + const res = await request(app) + .patch('/doctors/me') + .set(asUser(doctor)) + .set('If-Match', current.headers.etag) + .send({ fullName: 'Renamed Doctor' }); + expect(res.status).toBe(200); + expect(res.body.fullName).toBe('Renamed Doctor'); + expect(res.body.version).toBe(current.body.version + 1); + expect(res.headers.etag).toBe(`"${res.body.version}"`); + // The database agrees — the increment happened in the guarded UPDATE. + expect((await prisma.doctorProfile.findUniqueOrThrow({ where: { id: doctor.profileId } })).version).toBe( + res.body.version, + ); + }); + + it('rejects a stale write with 412 and leaves the winning value in place', async () => { + const stale = (await request(app).get('/doctors/me').set(asUser(doctor))).headers.etag; + const first = await request(app).patch('/doctors/me').set(asUser(doctor)).set('If-Match', stale).send({ fullName: 'First Writer' }); + expect(first.status).toBe(200); + + const loser = await request(app) + .patch('/doctors/me') + .set(asUser(doctor)) + .set('If-Match', stale) + .send({ fullName: 'Second Writer' }); + expect(loser.status).toBe(412); + expect(loser.body.code).toBe('VERSION_CONFLICT'); + + // The classic lost-update check: the first writer's value survived, and + // the version moved exactly once — not twice. + const stored = await prisma.doctorProfile.findUniqueOrThrow({ where: { id: doctor.profileId } }); + expect(stored.fullName).toBe('First Writer'); + expect(stored.version).toBe(first.body.version); + + // And re-reading then retrying the loser's intent succeeds. + const retry = await request(app).patch('/doctors/me').set(asUser(doctor)).set('If-Match', `"${stored.version}"`).send({ + fullName: 'Second Writer', + }); + expect(retry.status).toBe(200); + expect(retry.body.fullName).toBe('Second Writer'); + }); + + it('lets exactly one of 8 simultaneous writes on the same version through', async () => { + const etag = (await request(app).get('/doctors/me').set(asUser(doctor))).headers.etag; + const versionBefore = (await prisma.doctorProfile.findUniqueOrThrow({ where: { id: doctor.profileId } })).version; + + const responses = await Promise.all( + Array.from({ length: 8 }, (_, i) => + request(app).patch('/doctors/me').set(asUser(doctor)).set('If-Match', etag).send({ fullName: `Racer ${i}` }), + ), + ); + + expect(responses.filter((r) => r.status === 200)).toHaveLength(1); + expect(responses.filter((r) => r.status === 412)).toHaveLength(7); + for (const loser of responses.filter((r) => r.status === 412)) expect(loser.body.code).toBe('VERSION_CONFLICT'); + + const stored = await prisma.doctorProfile.findUniqueOrThrow({ where: { id: doctor.profileId } }); + expect(stored.version).toBe(versionBefore + 1); + expect(stored.fullName).toBe(responses.find((r) => r.status === 200)!.body.fullName); + }); + + it('authorises before checking the precondition, and validates the body', async () => { + // No token -> 401, not a hint that this route has a precondition. + expect((await request(app).patch('/doctors/me').send({ fullName: 'Anon' })).status).toBe(401); + // Wrong role -> 403, likewise. + expect((await request(app).patch('/doctors/me').set(asUser(patient)).send({ fullName: 'Patient' })).status).toBe( + 403, + ); + + const etag = (await request(app).get('/doctors/me').set(asUser(doctor))).headers.etag; + const before = (await prisma.doctorProfile.findUniqueOrThrow({ where: { id: doctor.profileId } })).version; + expect( + (await request(app).patch('/doctors/me').set(asUser(doctor)).set('If-Match', etag).send({ fullName: '' })).status, + ).toBe(400); + expect( + ( + await request(app) + .patch('/doctors/me') + .set(asUser(doctor)) + .set('If-Match', etag) + .send({ fullName: 'Sneaky', verificationStatus: 'verified' }) + ).status, + ).toBe(400); + expect((await prisma.doctorProfile.findUniqueOrThrow({ where: { id: doctor.profileId } })).version).toBe(before); + }); + }); + + describe('already-conditional transitions are not given a second guard', () => { + it('books without If-Match — the slot claim is already the guard', async () => { + const res = await request(app) + .post('/appointments') + .set({ Authorization: `Bearer ${patient.token}` }) + .send({ availabilitySlotId: slotId }); + expect(res.status).toBe(201); + expect((await prisma.appointment.findUniqueOrThrow({ where: { id: res.body.id } })).version).toBe(1); + }); + + it('flips doctor verification without If-Match and still bumps the version', async () => { + const before = await prisma.doctorProfile.findUniqueOrThrow({ where: { id: otherDoctor.profileId } }); + const res = await request(app) + .patch(`/doctors/${otherDoctor.profileId}/verification`) + .set(adminAuth()) + .send({ status: 'verified' }); + expect(res.status).toBe(200); + expect(res.body.verificationStatus).toBe('verified'); + expect(res.body.version).toBe(before.version + 1); + }); + + it('funds and releases without If-Match, advancing the version each time', async () => { + const intent = await createIntent(); + expect(intent.version).toBe(1); + + const read = await request(app).get(`/payments/intents/${intent.id}`).set(adminAuth()); + expect(read.status).toBe(200); + expect(read.headers.etag).toBe('"1"'); + expect(read.body.version).toBe(1); + + const funded = await request(app).post(`/payments/intents/${intent.id}/fund`).set(adminAuth()); + expect(funded.status).toBe(200); + expect(funded.body.version).toBe(2); + expect(funded.headers.etag).toBe('"2"'); + + const released = await request(app).post(`/payments/intents/${intent.id}/release`).set(adminAuth()); + expect(released.status).toBe(200); + expect(released.body.version).toBe(3); + + expect((await prisma.paymentIntent.findUniqueOrThrow({ where: { id: intent.id } })).version).toBe(3); + + // The status transition's own conditional write is untouched: a second + // release still loses on status (409), not on the version. + expect((await request(app).post(`/payments/intents/${intent.id}/release`).set(adminAuth())).status).toBe(409); + expect((await prisma.paymentIntent.findUniqueOrThrow({ where: { id: intent.id } })).version).toBe(3); + }); + }); + + describe('schema', () => { + it('puts a version column on the mutable models and nothing else', async () => { + const rows = await prisma.$queryRaw<{ table_name: string }[]>` + SELECT table_name FROM information_schema.columns + WHERE table_schema = 'public' AND column_name = 'version' + ORDER BY table_name + `; + // Append-only Prescription/LabOrder and every immutable record + // (Specialty, WalletLink, AuditEvent, the export tables) are absent by + // design — see ARCHITECTURE.md §5.4.4. + expect(rows.map((r) => r.table_name)).toEqual([ + 'Appointment', + 'DoctorProfile', + 'FacilityCapability', + 'FacilityProfile', + 'PatientProfile', + 'PaymentIntent', + 'User', + ]); + }); + + it('bumps the User version when the address is verified', async () => { + const email = `cc-verify-${runId}@example.com`; + const signupRes = await request(app) + .post('/auth/signup') + .send({ role: 'patient', email, password: 'longenough123', fullName: 'Verify Me' }); + expect(signupRes.status).toBe(200); + const userId = signupRes.body.user.id as string; + expect((await prisma.user.findUniqueOrThrow({ where: { id: userId } })).version).toBe(1); + + expect((await request(app).post('/auth/verify-email').send({ token: emailedTokens.get(email) })).status).toBe(200); + expect((await prisma.user.findUniqueOrThrow({ where: { id: userId } })).version).toBe(2); + }); + }); +}); \ No newline at end of file diff --git a/docs/api/README.md b/docs/api/README.md index 5e67678..11e91ab 100644 --- a/docs/api/README.md +++ b/docs/api/README.md @@ -134,8 +134,53 @@ drift apart. See `docs/adr/0003` for why production needs `SameSite=None`. | Method & path | Auth | Notes | |---|---|---| -| `GET /doctors/me` | doctor | Returns the caller's own `DoctorProfile`. | -| `PATCH /doctors/:doctorProfileId/verification` | admin | Body `{ status: 'pending'\|'verified'\|'rejected' }`. Manual, admin-gated per PRD FR-3 — there is no automated verification pipeline. | +| `GET /doctors/me` | doctor | Returns the caller's own `DoctorProfile`. Carries `ETag: ""` and `version` in the body — see below. | +| `PATCH /doctors/me` | doctor | Body `{ fullName }`. **Requires `If-Match`** with the `ETag` from the read (below). `428` without it, `412` if stale. | +| `PATCH /doctors/:doctorProfileId/verification` | admin | Body `{ status: 'pending'\|'verified'\|'rejected' }`. Manual, admin-gated per PRD FR-3 — there is no automated verification pipeline. This is an already-conditional status transition, so it takes **no** `If-Match`; it bumps `version` and returns it. | + +### Optimistic concurrency (Phase 8, issue #49) + +Mutable records (`User`, the three profiles, `FacilityCapability`, +`Appointment`, `PaymentIntent`) carry an integer `version`. Any **individual** +read of one returns it two ways, and every `PATCH`/`PUT` on one must send it +back: + +```http +GET /doctors/me → 200, ETag: "4", body.version = 4 +PATCH /doctors/me → If-Match: "4" +``` + +| Situation | Response | +|---|---| +| `If-Match` absent | `428 { message, code: 'IF_MATCH_REQUIRED' }` — nothing is written | +| `If-Match` is a weak validator (`W/"4"`), a list (`"4", "5"`), unquoted, or otherwise not a strong `""` | `400 { code: 'INVALID_IF_MATCH' }` | +| `If-Match` doesn't match the stored version | `412 { message, code: 'VERSION_CONFLICT' }` — nothing is written | +| Match | `200`, body carries the new `version`, and `ETag` is the new one | + +Why it exists: without it, two actors who read the same record both write and +the second silently destroys the first — last write wins, and the loser is +never told. With it, the compare and the write are a single conditional +`UPDATE`, so exactly one write can win. Proven against a real database in +`backend/tests/integration/optimistic-concurrency.test.mts`, including eight +simultaneous writes on one version. + +**Client requirements (docforum-web):** read the resource, keep the `ETag`, send +it on every `PATCH`/`PUT`, and on `412` re-read and reapply the change (don't +blind-retry with the old tag). On `428` the client is simply too old to have +learned this — surface it as an error, not a silent retry. + +**Not applied to** endpoints that are already conditionally guarded — booking +(`POST /appointments`), payment `fund`/`release`/`refund`, doctor verification. +Their existing status guard is the guard; adding `If-Match` on top would give +one write two unrelated failure modes. They still return the updated `ETag`. + +**Client requirements (docforum-web)** for the affected routes only: +`fetch(url, { method: 'PATCH', headers: { 'If-Match': etag } })`. + +Collection reads (`GET /appointments/mine`, `/admin/audit-events`, …) do not +carry a record-level `ETag`: fetch the individual resource before editing. +Append-only records (`Prescription`, `LabOrder`) have no version — a +correction is a new row, so there is nothing to lose. ## Availability (`/availability`) — `backend/src/modules/availability`