From 6ed925fee8fbaee4e3b07ec0aa1eb4efdb49e7e5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Thu, 14 May 2026 18:14:30 +0200 Subject: [PATCH] feat(api): correct batch-workout race reconciliation under contention MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Concurrent-write integration coverage that validates the W10 fix-C reconciliation pattern carries over correctly to the /api/workouts/batch endpoint: - Two batches with overlapping (userId, source, externalId) tuples posted in parallel resolve to exactly one DB row per tuple (the composite unique index enforces it; createMany.skipDuplicates absorbs duplicate-key conflicts). - Each response body's per-entry envelope sums to its own aggregate counts — the invariant the iOS sync cursor relies on to checkpoint correctly. - Aggregate inserted counts across both responses equal the row count actually present, cross-validating that createMany.count is the truth source rather than the older 'stored vs not-stored' diff (which was a no-op under standard Postgres semantics because every attempted row IS in the DB after the call, written by whichever batch won the race). - A two-batch shared-key fixture asserts the shared tuple lands exactly once across both responses. Also adds workouts and workout_routes to the integration suite's truncate list so beforeEach() actually clears them between tests rather than relying on CASCADE from users. --- tests/integration/setup.ts | 2 + tests/integration/workout-batch-race.test.ts | 220 +++++++++++++++++++ 2 files changed, 222 insertions(+) create mode 100644 tests/integration/workout-batch-race.test.ts diff --git a/tests/integration/setup.ts b/tests/integration/setup.ts index e61704681..70e77f333 100644 --- a/tests/integration/setup.ts +++ b/tests/integration/setup.ts @@ -67,6 +67,8 @@ export async function truncateAllTables(client: PrismaClient): Promise { "user_achievements", "users", "withings_connections", + "workout_routes", + "workouts", ]; const quoted = tables.map((t) => `"${t}"`).join(", "); diff --git a/tests/integration/workout-batch-race.test.ts b/tests/integration/workout-batch-race.test.ts new file mode 100644 index 000000000..5a6cd6857 --- /dev/null +++ b/tests/integration/workout-batch-race.test.ts @@ -0,0 +1,220 @@ +/** + * Concurrent-write race coverage for `POST /api/workouts/batch`. + * + * Mirrors the v1.4.25 W10 fix-C reconciliation pattern that landed for + * the measurements batch endpoint + * (`tests/integration/measurements-batch.test.ts` — the + * `keeps per-entry status in sync with aggregate counts under a + * concurrent-write race` case). When two batches with overlapping + * `(userId, source, externalId)` tuples land in parallel, only one + * row per tuple may exist in the DB after the dust settles, and each + * response body's per-entry envelope MUST sum to its own aggregate + * counts so the iOS sync cursor can advance unambiguously. + * + * Pulled into its own file so the contention assertion remains + * isolated from the broader workout-batch surface — the W10 fix was a + * one-line invariant that took two hours of audit work to expose, and + * regressions on it would be near-invisible in a mixed-purpose file. + */ +import { NextRequest } from "next/server"; +import { beforeEach, describe, expect, it, vi } from "vitest"; + +import { cookieJar, headerJar } from "./mock-next-headers"; +import { getPrismaClient, truncateAllTables } from "./setup"; + +const TEST_USER_ID = "user-workout-race-test"; + +vi.mock("next/headers", async () => { + const { cookieJar, headerJar } = await import("./mock-next-headers"); + return { + headers: vi.fn(async () => ({ + get: (name: string) => headerJar.get(name.toLowerCase()) ?? null, + })), + cookies: vi.fn(async () => ({ + get: (name: string) => { + const value = cookieJar.get(name); + return value ? { name, value } : undefined; + }, + set: (name: string, value: string) => { + cookieJar.set(name, value); + }, + delete: (name: string) => { + cookieJar.delete(name); + }, + })), + }; +}); + +vi.mock("@/lib/db-compat", () => ({ + ensureDbCompatibility: vi.fn().mockResolvedValue(undefined), +})); + +beforeEach(async () => { + await truncateAllTables(getPrismaClient()); + cookieJar.clear(); + headerJar.clear(); + await getPrismaClient().user.create({ + data: { + id: TEST_USER_ID, + username: "workout-race", + email: "workout-race@example.test", + }, + }); + const session = await getPrismaClient().session.create({ + data: { + userId: TEST_USER_ID, + expiresAt: new Date(Date.now() + 60 * 60 * 1000), + }, + }); + cookieJar.set("healthlog_session", session.id); +}); + +interface WorkoutFixture { + sportType: string; + startedAt: string; + endedAt: string; + source: string; + externalId: string; +} + +function makeRequest(body: { workouts: WorkoutFixture[] }): NextRequest { + return new NextRequest("http://localhost/api/workouts/batch", { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify(body), + }); +} + +function workout(externalId: string, minuteOffset: number): WorkoutFixture { + return { + sportType: "running", + startedAt: new Date(2026, 4, 14, 6, minuteOffset).toISOString(), + endedAt: new Date(2026, 4, 14, 7, minuteOffset).toISOString(), + source: "APPLE_HEALTH", + externalId, + }; +} + +describe("POST /api/workouts/batch — concurrent-write race", () => { + // v1.4.25 W10 reconcile (senior-dev H-1 parity for workouts). + // + // Two batches with overlapping `(userId, source, externalId)` tuples + // posted in parallel MUST resolve to exactly one DB row per tuple + // (the composite unique index enforces it; createMany.skipDuplicates + // absorbs the duplicate-key conflicts). What can go wrong is the + // per-entry envelope: the original pattern compared "rows we + // attempted" vs "rows now in the DB" — but every attempted row is + // present after the call (either we wrote it or the other batch did), + // so the diff is always zero and the envelope drifts out of sync + // with the aggregate counts under contention. The fix trusts + // `createMany.count` for the aggregate `insertedCount` and downgrades + // exactly that many "inserted" statuses to "duplicate", so the per- + // entry envelope sums match the aggregate counts — the invariant the + // iOS sync cursor relies on to checkpoint correctly. + it("keeps per-entry status in sync with aggregate counts under contention", async () => { + const { POST } = await import("@/app/api/workouts/batch/route"); + + const sharedWorkouts = (): WorkoutFixture[] => + Array.from({ length: 6 }, (_, i) => workout(`hk-uuid-race-${i}`, i)); + + const [first, second] = await Promise.all([ + POST(makeRequest({ workouts: sharedWorkouts() })), + POST(makeRequest({ workouts: sharedWorkouts() })), + ]); + expect(first.status).toBe(200); + expect(second.status).toBe(200); + + const firstJson = (await first.json()) as { + data: { + processed: number; + inserted: number; + duplicates: number; + entries: Array<{ status: string }>; + }; + }; + const secondJson = (await second.json()) as typeof firstJson; + + // Invariant 1 — per-entry statuses sum to the aggregate counts. + // Before the W10 fix-C the comparable measurements test failed + // under contention because the no-op reconciliation left the + // envelope out of sync with the counts. + for (const json of [firstJson, secondJson]) { + const insertedEntries = json.data.entries.filter( + (e) => e.status === "inserted", + ).length; + const duplicateEntries = json.data.entries.filter( + (e) => e.status === "duplicate", + ).length; + const skippedEntries = json.data.entries.filter( + (e) => e.status === "skipped", + ).length; + expect(insertedEntries).toBe(json.data.inserted); + expect(duplicateEntries).toBe(json.data.duplicates); + expect(insertedEntries + duplicateEntries + skippedEntries).toBe( + json.data.processed, + ); + } + + // Invariant 2 — counts non-negative. A naive "downgrade and also + // decrement" implementation could otherwise produce a negative + // `inserted` count under specific commit orderings. + expect(firstJson.data.inserted).toBeGreaterThanOrEqual(0); + expect(secondJson.data.inserted).toBeGreaterThanOrEqual(0); + + // Invariant 3 — DB ends with at most 6 rows (composite unique + // index enforces single-copy) and the two responses' inserted + // counts sum to the row count actually present. Cross-validates + // that `createMany.count` is the truth source for the aggregate. + const stored = await getPrismaClient().workout.findMany({ + where: { userId: TEST_USER_ID }, + }); + expect(stored.length).toBeLessThanOrEqual(6); + expect(firstJson.data.inserted + secondJson.data.inserted).toBe( + stored.length, + ); + }); + + it("two batches with one overlapping externalId resolve to a single DB row each", async () => { + const { POST } = await import("@/app/api/workouts/batch/route"); + + // Batch A — entries 0..3 unique-to-A plus one shared key. + const batchA: WorkoutFixture[] = [ + workout("hk-uuid-only-a-0", 1), + workout("hk-uuid-only-a-1", 2), + workout("hk-uuid-shared", 10), + ]; + // Batch B — entries 0..3 unique-to-B plus the SAME shared key. + const batchB: WorkoutFixture[] = [ + workout("hk-uuid-only-b-0", 21), + workout("hk-uuid-only-b-1", 22), + workout("hk-uuid-shared", 10), + ]; + + const [resA, resB] = await Promise.all([ + POST(makeRequest({ workouts: batchA })), + POST(makeRequest({ workouts: batchB })), + ]); + expect(resA.status).toBe(200); + expect(resB.status).toBe(200); + + const aJson = (await resA.json()) as { + data: { inserted: number; duplicates: number }; + }; + const bJson = (await resB.json()) as typeof aJson; + + // Five unique tuples total across both batches; one shared. The + // composite unique index means the shared key lands exactly once, + // either as A's "inserted" or B's "inserted" — never both. The + // sum of the two batches' inserted counts is exactly 5. + expect(aJson.data.inserted + bJson.data.inserted).toBe(5); + + const stored = await getPrismaClient().workout.findMany({ + where: { userId: TEST_USER_ID }, + }); + expect(stored).toHaveLength(5); + const sharedRows = stored.filter( + (r) => r.externalId === "hk-uuid-shared", + ); + expect(sharedRows).toHaveLength(1); + }); +});