diff --git a/CHANGELOG.md b/CHANGELOG.md index a98d936ba..b2ae8aadc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -34,6 +34,20 @@ that follow the source priority. ### Fixed +- **Blood pressure saved without its diastolic value (iOS #15, migration + 0371).** The iPhone app sends the two halves of a manual reading as two + requests under one `Idempotency-Key`, and the server answered the second + with the first one's cached response. The replay cache now stores a SHA-256 + fingerprint of method, path and body (JSON canonicalised, a multipart + boundary normalised) and replays only the same request; a different body + under the same key is saved. Readings saved before this stay incomplete; + `docs/ops/blood-pressure-missing-diastolic.md` lists them. +- **Document previews and scanned PDFs (#1124).** The image hoisted an older + `@napi-rs/canvas` binary next to the pinned loader, so every thumbnail + failed; the build now hoists the pinned version and fails on a mismatch. + PDFs render with pdfjs font, cmap and wasm data and a registered fallback + font instead of absent system fonts, and an all-blank render reports + `raster-failed`. The thumbnail job logs why a preview is missing. - **Lab unit mismatch (#1094).** A reading in another unit than its marker is refused (`labs.unit.mismatch`) on every write path; a marker's unit is locked while it has readings (`biomarkers.unit.locked`). diff --git a/Dockerfile b/Dockerfile index 6b871db84..20fe839da 100644 --- a/Dockerfile +++ b/Dockerfile @@ -54,6 +54,9 @@ RUN NODE_OPTIONS=--max-old-space-size=8192 pnpm build # name; see the note there for why guessing was wrong. RUN node -p "require('/app/node_modules/pdfjs-dist/package.json').version" \ > /app/.pdfjs-version +# The same for @napi-rs/canvas, whose native binary must match its JS loader. +RUN node -p "require('/app/node_modules/@napi-rs/canvas/package.json').version" \ + > /app/.canvas-version # ── Stage 3: Production runner ───────────────────────────── FROM node:22-alpine@sha256:16e22a550f3863206a3f701448c45f7912c6896a62de43add43bb9c86130c3e2 AS runner @@ -120,21 +123,41 @@ RUN node -e "require.resolve('pg-boss'); require.resolve('pg')" # @napi-rs/canvas" → "Cannot polyfill DOMMatrix" → rasterize ReferenceError); # (2) @napi-rs/canvas's own loader then resolves its platform binary. Hoist BOTH # the canvas package and the musl binary package to /app/node_modules/@napi-rs -# via symlink into the .pnpm store, and also drop the prebuilt .node into the -# canvas dir so the loader's local-file fallback (`require('./skia..node')`) -# is belt-and-suspenders. Per-arch buildx installs only the matching musl triple. +# via symlink into the .pnpm store, and drop the prebuilt .node into the canvas +# dir: the loader tries that local file (`require('./skia..node')`) +# before the platform package, so it decides which binary runs. +# +# Hoist the version the app pins, by name, exactly like pdfjs-dist below. The +# store holds a second, older canvas that pdf-parse's own pdfjs-dist pulls in, +# and this step used to take whichever `find | head -1` returned first. It +# returned the old binary, which then sat next to the new loader: rendering +# still worked, but `loadImage` called an `Image.decode` the old binary does +# not have, and every preview thumbnail failed with "image.decode is not a +# function". Name the version, and fail the build when the binary that lands +# in the canvas dir is not that version's. +COPY --from=builder /app/.canvas-version /tmp/.canvas-version RUN set -e; \ - CANVAS_DIR="$(find /app/node_modules/.pnpm -maxdepth 4 -type d -path '*@napi-rs+canvas@*/node_modules/@napi-rs/canvas' 2>/dev/null | head -1)"; \ - MUSL_DIR="$(find /app/node_modules/.pnpm -maxdepth 4 -type d -path '*@napi-rs+canvas-linux-*-musl@*/node_modules/@napi-rs/canvas-linux-*-musl' 2>/dev/null | head -1)"; \ - NODE_BIN="$(find /app/node_modules/.pnpm -maxdepth 5 -name 'skia.linux-*-musl.node' 2>/dev/null | head -1)"; \ - if [ -n "$CANVAS_DIR" ]; then \ - mkdir -p /app/node_modules/@napi-rs; \ - ln -sfn "$CANVAS_DIR" /app/node_modules/@napi-rs/canvas; \ - [ -n "$MUSL_DIR" ] && ln -sfn "$MUSL_DIR" "/app/node_modules/@napi-rs/$(basename "$MUSL_DIR")"; \ - [ -n "$NODE_BIN" ] && cp "$NODE_BIN" "$CANVAS_DIR/"; \ - chown -R nextjs:nodejs /app/node_modules/@napi-rs; \ - echo "canvas hoisted for pdfjs: $CANVAS_DIR (musl=$MUSL_DIR)"; \ - else echo "WARN: @napi-rs/canvas not found; PDF rasterization will degrade to local text"; fi + CANVAS_VERSION="$(cat /tmp/.canvas-version)"; \ + rm -f /tmp/.canvas-version; \ + CANVAS_DIR="/app/node_modules/.pnpm/@napi-rs+canvas@${CANVAS_VERSION}/node_modules/@napi-rs/canvas"; \ + MUSL_DIR="$(find /app/node_modules/.pnpm -maxdepth 4 -type d -path "*@napi-rs+canvas-linux-*-musl@${CANVAS_VERSION}/node_modules/@napi-rs/canvas-linux-*-musl" 2>/dev/null | head -1)"; \ + if [ ! -d "$CANVAS_DIR" ] || [ -z "$MUSL_DIR" ]; then \ + echo "ERROR: pinned @napi-rs/canvas@${CANVAS_VERSION} or its musl binary did not reach the traced image."; \ + echo "Present instead:"; \ + find /app/node_modules/.pnpm -maxdepth 1 -name '@napi-rs+canvas*' 2>/dev/null; \ + exit 1; \ + fi; \ + NODE_BIN="$(find "$MUSL_DIR" -maxdepth 1 -name 'skia.linux-*-musl.node' | head -1)"; \ + [ -n "$NODE_BIN" ] || { echo "ERROR: no skia binary in $MUSL_DIR"; exit 1; }; \ + mkdir -p /app/node_modules/@napi-rs; \ + ln -sfn "$CANVAS_DIR" /app/node_modules/@napi-rs/canvas; \ + ln -sfn "$MUSL_DIR" "/app/node_modules/@napi-rs/$(basename "$MUSL_DIR")"; \ + cp "$NODE_BIN" "$CANVAS_DIR/"; \ + chown -R nextjs:nodejs /app/node_modules/@napi-rs; \ + BIN_VERSION="$(node -p "require('$MUSL_DIR/package.json').version")"; \ + [ "$BIN_VERSION" = "$CANVAS_VERSION" ] || { echo "ERROR: canvas binary is $BIN_VERSION, loader is $CANVAS_VERSION"; exit 1; }; \ + cmp -s "$NODE_BIN" "$CANVAS_DIR/$(basename "$NODE_BIN")" || { echo "ERROR: the canvas dir holds a different binary"; exit 1; }; \ + echo "canvas hoisted for pdfjs: $CANVAS_DIR (binary $BIN_VERSION from $MUSL_DIR)" # pdfjs-dist is `serverExternalPackages` (NOT bundled — the bundled copy's render # path breaks in the standalone image), so the server does a runtime bare diff --git a/docs/CONTRIBUTING-CLIENTS.md b/docs/CONTRIBUTING-CLIENTS.md index 2f5660022..e2f627e8c 100644 --- a/docs/CONTRIBUTING-CLIENTS.md +++ b/docs/CONTRIBUTING-CLIENTS.md @@ -19,7 +19,7 @@ Every JSON route answers the same shape: `{ data, error }`, with `meta` on the e - Refresh tokens rotate per device and are one-time-use. Reusing an old one revokes that device's whole token family, on purpose. Persist the newest pair atomically and never retry a refresh with a token that already succeeded once. - Bearer token scopes are fail-closed. A token minted for one purpose does not open unrelated routes, and a route that does not declare a scope refuses narrow tokens outright. Expect `403` to be a normal, recoverable answer, not an anomaly. - Admin surfaces are cookie-session only, by construction. A native client cannot be an admin client, whatever scopes its token carries. Do not build admin features. -- Mutations accept an `Idempotency-Key` header. Replays answer with the original result plus `X-Idempotent-Replay`, so retrying on a dropped connection is safe. Use it for anything a user would notice twice. +- Mutations accept an `Idempotency-Key` header. A repeat of the same key with the same body answers with the original result plus `X-Idempotent-Replay`, so retrying on a dropped connection is safe. A different body under the same key is treated as a new request, not a replay; still, give each distinct write its own key. Use it for anything a user would notice twice. ## Data semantics worth knowing before you sync diff --git a/docs/api/openapi.yaml b/docs/api/openapi.yaml index 8b11dbff7..e2cb195e9 100644 --- a/docs/api/openapi.yaml +++ b/docs/api/openapi.yaml @@ -162,12 +162,14 @@ paths: pattern: ^[A-Za-z0-9_\-:.]{8,128}$ minLength: 8 maxLength: 128 - description: "Makes this write safe to retry. Send the same key again and the first response is replayed rather than the - work repeated, marked with `X-Idempotent-Replay: true`. A key that does not match the pattern is IGNORED - rather than refused — the write then proceeds unprotected and looks exactly like a request that carried no - key, so validate the key on your side. Scoped per user and per (method, path): the same key on a different - route is a different key. While a first request under this key is still in flight, a second is refused with - 409 rather than queued." + description: "Makes this write safe to retry. Send the same key with the same body again and the first response is + replayed rather than the work repeated, marked with `X-Idempotent-Replay: true`. The body is compared as + JSON with key order ignored; a different body under the same key is not a retry and runs as a new write, + while the first response stays stored for retries of the first request. A key that does not match the + pattern is IGNORED rather than refused — the write then proceeds unprotected and looks exactly like a + request that carried no key, so validate the key on your side. Scoped per user and per (method, path): the + same key on a different route is a different key. While a first request under this key is still in flight, a + second with the same body is refused with 409 rather than queued." requestBody: required: true content: diff --git a/docs/integrations/home-assistant.md b/docs/integrations/home-assistant.md index c521a8c74..71d679ccc 100644 --- a/docs/integrations/home-assistant.md +++ b/docs/integrations/home-assistant.md @@ -171,8 +171,9 @@ tells you which rows to retry. ## Retries and duplicates Send an `Idempotency-Key` header with a value your automation can reproduce for -the same reading. A retry with the same key returns the original response -instead of writing a second row: +the same reading. A retry with the same key and the same body returns the +original response instead of writing a second row. A different body under the +same key is a different reading and is written: ```yaml headers: diff --git a/docs/ops/blood-pressure-missing-diastolic.md b/docs/ops/blood-pressure-missing-diastolic.md new file mode 100644 index 000000000..1509a6ade --- /dev/null +++ b/docs/ops/blood-pressure-missing-diastolic.md @@ -0,0 +1,60 @@ +# Blood pressure readings missing their diastolic half + +## What happened + +The iPhone app (every App Store build up to 1.0.3 and TestFlight 1.1.0) +saves a manually entered blood pressure as two requests to +`POST /api/measurements`: one `BLOOD_PRESSURE_SYS`, one +`BLOOD_PRESSURE_DIA`, both carrying the same `Idempotency-Key`. Until the +replay cache learned to compare request bodies, the server treated the second +request as a retry of the first. It answered with the first request's cached +`201` (header `X-Idempotent-Replay: true`) and never wrote the diastolic +value. The app showed success, so nobody noticed. + +The fix makes the cache replay only a request with the same body. A different +body under the same key now runs normally. Nothing recovers the values that +were never sent to the database: they exist only on the person's phone, if at +all. + +## Finding the affected readings + +Read-only. Run it against the HealthLog database (with the bundled compose +file: `docker compose exec db psql -U healthlog healthlog`). It lists every live systolic reading that has no +live diastolic reading for the same account at the same instant. + + + +```sql +SELECT u.username, + m.id, + m.measured_at, + m.value AS systolic, + m.source +FROM measurements m +JOIN users u ON u.id = m.user_id +WHERE m.type = 'BLOOD_PRESSURE_SYS' + AND m.deleted_at IS NULL + AND NOT EXISTS ( + SELECT 1 + FROM measurements d + WHERE d.user_id = m.user_id + AND d.measured_at = m.measured_at + AND d.type = 'BLOOD_PRESSURE_DIA' + AND d.deleted_at IS NULL + ) +ORDER BY u.username, m.measured_at; +``` + + + +Readings from the phone app carry `source = 'MANUAL'`. Rows from a synced +source (Withings, Apple Health) that show up here are a different gap, not +this one: those sources write both halves themselves. + +## What to tell users + +There is nothing the server can repair: the diastolic value never reached +it. A person whose reading appears in the list can re-enter it: delete the +incomplete reading, then add the blood pressure again with both values and +the original date and time. From this release on, both halves are saved on +the first try, from any app version. diff --git a/prisma/migrations/0371_idempotency_request_fingerprint/migration.sql b/prisma/migrations/0371_idempotency_request_fingerprint/migration.sql new file mode 100644 index 000000000..dcb360852 --- /dev/null +++ b/prisma/migrations/0371_idempotency_request_fingerprint/migration.sql @@ -0,0 +1,8 @@ +-- What request a replay-cache cell answered, as a SHA-256 of the canonical +-- request body scoped by method and path. A request that reuses a key with a +-- different body is executed instead of being handed the first body's cached +-- response. The body itself is never stored. Nullable: rows written before +-- this release carry none, and a null never matches, so they stop replaying +-- for anything but their own retries already in flight (they expire within +-- 24 hours either way). +ALTER TABLE "idempotency_keys" ADD COLUMN IF NOT EXISTS "request_fingerprint" TEXT; diff --git a/prisma/schema.prisma b/prisma/schema.prisma index 6a05a5659..9250aef3f 100644 --- a/prisma/schema.prisma +++ b/prisma/schema.prisma @@ -5545,15 +5545,19 @@ model ProviderHealth { /// (userId, key, method, path) so retries within the TTL replay the /// original outcome instead of producing a duplicate side-effect. model IdempotencyKey { - id String @id @default(cuid()) - userId String @map("user_id") - key String - method String - path String - responseStatus Int @map("response_status") - responseBody String @map("response_body") // serialized JSON envelope - expiresAt DateTime @map("expires_at") - createdAt DateTime @default(now()) @map("created_at") + id String @id @default(cuid()) + userId String @map("user_id") + key String + method String + path String + responseStatus Int @map("response_status") + responseBody String @map("response_body") // serialized JSON envelope + /// SHA-256 of the canonical request body, scoped by method and path. A + /// lookup replays only when it matches; a different body under the same key + /// runs normally. Null on rows written before it existed, which never match. + requestFingerprint String? @map("request_fingerprint") + expiresAt DateTime @map("expires_at") + createdAt DateTime @default(now()) @map("created_at") user User @relation(fields: [userId], references: [id], onDelete: Cascade) diff --git a/src/__tests__/canvas-binary-hoist-guard.test.ts b/src/__tests__/canvas-binary-hoist-guard.test.ts new file mode 100644 index 000000000..d230fab26 --- /dev/null +++ b/src/__tests__/canvas-binary-hoist-guard.test.ts @@ -0,0 +1,65 @@ +/** + * Structural guard: the runtime image puts the @napi-rs/canvas native binary + * of the version the app pins next to that version's loader. + * + * ## The failure this freezes (GitHub #1124) + * + * The pnpm store holds two canvas versions: the one in package.json and an + * older one that pdf-parse's own pdfjs-dist pulls in. The runner stage picked + * the musl binary with `find | head -1`, got the old one, and copied it into + * the new loader's directory, where the loader looks first. PDF rendering + * still worked, so nothing looked wrong; `loadImage` called an + * `Image.decode` the old binary lacks, and every preview thumbnail failed + * with "image.decode is not a function" on both architectures. + * + * ## What it proves and what it does not + * + * The builder records the resolved canvas version, the runner hoists by that + * name, and the build stops when the copied binary's package is a different + * version. Whether the binary loads is proved when the image runs; this file + * reads the Dockerfile only. + */ +import { readFileSync } from "node:fs"; +import { join } from "node:path"; + +import { describe, expect, it } from "vitest"; + +const DOCKERFILE = readFileSync(join(process.cwd(), "Dockerfile"), "utf8"); + +function stage(name: string): string { + const start = DOCKERFILE.search(new RegExp(`^FROM .* AS ${name}$`, "m")); + expect(start, `the Dockerfile has a ${name} stage`).toBeGreaterThan(0); + const next = DOCKERFILE.slice(start + 1).search(/^FROM /m); + return next < 0 + ? DOCKERFILE.slice(start) + : DOCKERFILE.slice(start, start + 1 + next); +} + +describe("canvas binary hoist", () => { + it("records the resolved canvas version in the builder", () => { + expect(stage("builder")).toMatch( + /require\('\/app\/node_modules\/@napi-rs\/canvas\/package\.json'\)\.version"[\s\\]*>\s*\/app\/\.canvas-version/, + ); + }); + + it("hoists the loader and the musl binary of that version only", () => { + const runner = stage("runner"); + expect(runner).toContain( + "COPY --from=builder /app/.canvas-version /tmp/.canvas-version", + ); + expect(runner).toContain( + 'CANVAS_DIR="/app/node_modules/.pnpm/@napi-rs+canvas@${CANVAS_VERSION}/', + ); + expect(runner).toContain("canvas-linux-*-musl@${CANVAS_VERSION}/"); + // A version-blind pick is the defect: any `canvas…@*` glob decides by + // readdir order which of the store's copies wins. + expect(runner).not.toMatch(/@napi-rs\+canvas[^"'\s]*@\*/); + }); + + it("fails the build when the copied binary is another version", () => { + const runner = stage("runner"); + expect(runner).toMatch( + /\[ "\$BIN_VERSION" = "\$CANVAS_VERSION" \] \|\| \{[^}]*exit 1; \}/, + ); + }); +}); diff --git a/src/app/api/anamnesis/facts/__tests__/idempotency-conflict.test.ts b/src/app/api/anamnesis/facts/__tests__/idempotency-conflict.test.ts index adb31bbdc..dc93fd8e2 100644 --- a/src/app/api/anamnesis/facts/__tests__/idempotency-conflict.test.ts +++ b/src/app/api/anamnesis/facts/__tests__/idempotency-conflict.test.ts @@ -58,10 +58,13 @@ vi.mock("@/lib/profile/health-facts", () => ({ import { NextRequest } from "next/server"; import { POST } from "../route"; +import { requestFingerprint } from "@/lib/idempotency"; /** The pending sentinel `findCached` recognises as "another call is running". */ const PENDING_STATUS = 0; +const BODY = JSON.stringify({ kind: "SMOKING_STATUS", value: "FORMER" }); + function request(): NextRequest { return new NextRequest("http://localhost/api/anamnesis/facts", { method: "POST", @@ -69,7 +72,7 @@ function request(): NextRequest { "content-type": "application/json", "idempotency-key": "key-in-flight", }, - body: JSON.stringify({ kind: "SMOKING_STATUS", value: "FORMER" }), + body: BODY, }); } @@ -99,6 +102,12 @@ describe("idempotency 409 envelope, over the real route", () => { it("serves `error` as a string when a request is already in flight", async () => { mocks.findUnique.mockResolvedValue({ id: "idem-1", + // The same request in flight: a different body would run instead. + requestFingerprint: requestFingerprint( + "POST", + "/api/anamnesis/facts", + new TextEncoder().encode(BODY), + ), responseStatus: PENDING_STATUS, responseBody: "", expiresAt: new Date(Date.now() + 60_000), diff --git a/src/lib/__tests__/idempotency.test.ts b/src/lib/__tests__/idempotency.test.ts index 61494372e..37efb2d1c 100644 --- a/src/lib/__tests__/idempotency.test.ts +++ b/src/lib/__tests__/idempotency.test.ts @@ -49,6 +49,8 @@ import { withIdempotency, defaultUserIdResolver, isCachableStatus, + canonicalJson, + requestFingerprint, } from "../idempotency"; import { prisma } from "@/lib/db"; import { getSession } from "@/lib/auth/session"; @@ -81,6 +83,13 @@ function makeRequest( }); } +/** The fingerprint `makeRequest("POST", …)` produces. */ +const OK_BODY_FINGERPRINT = requestFingerprint( + "POST", + "/api/example", + new TextEncoder().encode(JSON.stringify({ ok: true })), +); + beforeEach(() => { vi.resetAllMocks(); vi.mocked(prisma.idempotencyKey.findUnique).mockResolvedValue(null); @@ -144,6 +153,11 @@ describe("withIdempotency", () => { data: { responseBody: string; responseStatus: number }; } ).data; + const claimedFingerprint = ( + vi.mocked(prisma.idempotencyKey.create).mock.calls[0][0] as { + data: { requestFingerprint: string }; + } + ).data.requestFingerprint; // Second call: cache hit returns persisted envelope. vi.mocked(prisma.idempotencyKey.findUnique).mockResolvedValueOnce({ @@ -152,6 +166,7 @@ describe("withIdempotency", () => { key: "abc-12345678", method: "POST", path: "/api/example", + requestFingerprint: claimedFingerprint, responseStatus: persistedBody.responseStatus, responseBody: persistedBody.responseBody, expiresAt: new Date(Date.now() + 86_400_000), @@ -296,6 +311,7 @@ describe("withIdempotency concurrency claim (A3)", () => { key: "abc-12345678", method: "POST", path: "/api/example", + requestFingerprint: OK_BODY_FINGERPRINT, responseStatus: 0, responseBody: "", expiresAt: new Date(Date.now() + 60_000), @@ -928,3 +944,227 @@ describe("the record-session fence sits above the replay cache", () => { expect(handler).toHaveBeenCalledTimes(1); }); }); + +describe("request fingerprint", () => { + const enc = (text: string) => new TextEncoder().encode(text); + + it("sorts object keys recursively and keeps array order", () => { + expect(canonicalJson({ b: 1, a: { d: [3, 1], c: null } })).toBe( + '{"a":{"c":null,"d":[3,1]},"b":1}', + ); + expect(canonicalJson([{ y: 1, x: 2 }, "s"])).toBe('[{"x":2,"y":1},"s"]'); + }); + + it("treats the same JSON object in a different key order as one request", () => { + const a = requestFingerprint("POST", "/p", enc('{"type":"X","value":1}')); + const b = requestFingerprint( + "POST", + "/p", + enc('{ "value": 1, "type": "X" }'), + ); + expect(a).toBe(b); + expect(a).toMatch(/^[0-9a-f]{64}$/); + }); + + it("tells nested values and array order apart", () => { + const base = requestFingerprint("POST", "/p", enc('{"a":{"b":[1,2]}}')); + expect(requestFingerprint("POST", "/p", enc('{"a":{"b":[2,1]}}'))).not.toBe( + base, + ); + expect(requestFingerprint("POST", "/p", enc('{"a":{"b":[1,3]}}'))).not.toBe( + base, + ); + }); + + it("tells the two halves of a blood pressure apart", () => { + const sys = requestFingerprint( + "POST", + "/api/measurements", + enc('{"type":"BLOOD_PRESSURE_SYS","value":120}'), + ); + const dia = requestFingerprint( + "POST", + "/api/measurements", + enc('{"type":"BLOOD_PRESSURE_DIA","value":80}'), + ); + expect(sys).not.toBe(dia); + }); + + it("reads a rebuilt multipart form with a fresh boundary as the same upload", () => { + const form = (boundary: string, name: string) => + enc( + `--${boundary}\r\nContent-Disposition: form-data; name="file"; filename="${name}"\r\nContent-Type: application/pdf\r\n\r\n%PDF-1.4 body\r\n--${boundary}--\r\n`, + ); + const type = (boundary: string) => + `multipart/form-data; boundary=${boundary}`; + const first = requestFingerprint( + "POST", + "/api/documents", + form("----formdata-a1", "brief.pdf"), + null, + type("----formdata-a1"), + ); + const retry = requestFingerprint( + "POST", + "/api/documents", + form("----formdata-b2", "brief.pdf"), + null, + type("----formdata-b2"), + ); + expect(retry).toBe(first); + const other = requestFingerprint( + "POST", + "/api/documents", + form("----formdata-b2", "other.pdf"), + null, + type("----formdata-b2"), + ); + expect(other).not.toBe(first); + }); + + it("hashes a non-JSON or empty body as its raw bytes", () => { + const text = requestFingerprint("POST", "/p", enc("not json")); + expect(text).toBe(requestFingerprint("POST", "/p", enc("not json"))); + expect(text).not.toBe(requestFingerprint("POST", "/p", enc("not json!"))); + const empty = requestFingerprint("DELETE", "/p", new Uint8Array()); + expect(empty).toBe(requestFingerprint("DELETE", "/p", new Uint8Array())); + expect(empty).not.toBe(text); + // Invalid UTF-8 is raw bytes, not an error. + expect(() => + requestFingerprint("POST", "/p", new Uint8Array([0xff, 0xfe])), + ).not.toThrow(); + }); + + it("is scoped by method and path", () => { + const body = enc('{"a":1}'); + const base = requestFingerprint("POST", "/p", body); + expect(requestFingerprint("PUT", "/p", body)).not.toBe(base); + expect(requestFingerprint("POST", "/q", body)).not.toBe(base); + }); +}); + +describe("withIdempotency with a different body under one key", () => { + function postWith(body: unknown): NextRequest { + return new NextRequest("http://localhost/api/example", { + method: "POST", + headers: { + "content-type": "application/json", + "idempotency-key": "abc-12345678", + }, + body: JSON.stringify(body), + }); + } + + function liveRow(fingerprint: string | null, responseStatus: number) { + return { + id: "idem-1", + userId: "u-1", + key: "abc-12345678", + method: "POST", + path: "/api/example", + requestFingerprint: fingerprint, + responseStatus, + responseBody: "", + expiresAt: new Date(Date.now() + 60_000), + createdAt: new Date(), + } as never; + } + + it.each([ + ["a completed row for another body", "f".repeat(64), 201], + ["an in-flight claim for another body", "f".repeat(64), 0], + ["a legacy row without a fingerprint", null, 201], + ])( + "runs the handler without claiming or caching over %s", + async (_label, fingerprint, status) => { + vi.mocked(prisma.idempotencyKey.findUnique).mockResolvedValueOnce( + liveRow(fingerprint, status), + ); + const handler = vi.fn(async () => + NextResponse.json({ data: "dia", error: null }, { status: 201 }), + ); + const wrapped = withIdempotency<[NextRequest]>( + handler, + async () => "u-1", + ); + const res = await wrapped(postWith({ type: "BLOOD_PRESSURE_DIA" })); + + expect(res.status).toBe(201); + expect(res.headers.get("X-Idempotent-Replay")).toBeNull(); + expect(handler).toHaveBeenCalledTimes(1); + expect(prisma.idempotencyKey.create).not.toHaveBeenCalled(); + expect(prisma.idempotencyKey.updateMany).not.toHaveBeenCalled(); + expect(prisma.idempotencyKey.deleteMany).not.toHaveBeenCalled(); + expect(prisma.idempotencyKey.delete).not.toHaveBeenCalled(); + }, + ); + + it("runs the handler when a racing claim for another body wins the insert", async () => { + vi.mocked(prisma.idempotencyKey.create).mockRejectedValueOnce( + Object.assign(new Error("unique"), { code: "P2002" }), + ); + // First lookup: nothing yet. Second, after the collision: the other body. + vi.mocked(prisma.idempotencyKey.findUnique) + .mockResolvedValueOnce(null) + .mockResolvedValueOnce(liveRow("f".repeat(64), 0)); + const handler = vi.fn(async () => + NextResponse.json({ data: "dia", error: null }, { status: 201 }), + ); + const wrapped = withIdempotency<[NextRequest]>(handler, async () => "u-1"); + const res = await wrapped(postWith({ type: "BLOOD_PRESSURE_DIA" })); + + expect(res.status).toBe(201); + expect(handler).toHaveBeenCalledTimes(1); + expect(prisma.idempotencyKey.updateMany).not.toHaveBeenCalled(); + }); + + it("stores the fingerprint, never the body, on the claim", async () => { + const handler = vi.fn(async () => + NextResponse.json({ data: "ok", error: null }, { status: 201 }), + ); + const wrapped = withIdempotency<[NextRequest]>(handler, async () => "u-1"); + await wrapped(postWith({ type: "BLOOD_PRESSURE_SYS", value: 123 })); + + const claim = ( + vi.mocked(prisma.idempotencyKey.create).mock.calls[0][0] as { + data: Record; + } + ).data; + expect(claim.requestFingerprint).toBe( + requestFingerprint( + "POST", + "/api/example", + new TextEncoder().encode( + JSON.stringify({ type: "BLOOD_PRESSURE_SYS", value: 123 }), + ), + ), + ); + expect(JSON.stringify(claim)).not.toContain("BLOOD_PRESSURE_SYS"); + }); + + it("leaves the body readable for the handler", async () => { + const handler = vi.fn(async (req: NextRequest) => + NextResponse.json({ data: await req.json(), error: null }), + ); + const wrapped = withIdempotency<[NextRequest]>(handler, async () => "u-1"); + const res = await wrapped(postWith({ type: "WEIGHT", value: 70 })); + expect((await res.json()).data).toEqual({ type: "WEIGHT", value: 70 }); + }); + + it("reads at most a bounded prefix of a large body and still hands the whole body on", async () => { + const big = "x".repeat(3 * 1024 * 1024); + const handler = vi.fn(async (req: NextRequest) => + NextResponse.json({ data: (await req.text()).length, error: null }), + ); + const wrapped = withIdempotency<[NextRequest]>(handler, async () => "u-1"); + const res = await wrapped( + new NextRequest("http://localhost/api/example", { + method: "POST", + headers: { "idempotency-key": "abc-12345678" }, + body: big, + }), + ); + expect((await res.json()).data).toBe(big.length); + expect(prisma.idempotencyKey.create).toHaveBeenCalledTimes(1); + }); +}); diff --git a/src/lib/documents/__tests__/native-canvas-support.test.ts b/src/lib/documents/__tests__/native-canvas-support.test.ts index df978c965..df282df1d 100644 --- a/src/lib/documents/__tests__/native-canvas-support.test.ts +++ b/src/lib/documents/__tests__/native-canvas-support.test.ts @@ -115,6 +115,6 @@ describe("generateThumbnail behind a closed gate", () => { Buffer.from([0xff, 0xd8, 0xff]), "image/jpeg", ); - expect(result).toEqual({ ok: false }); + expect(result).toEqual({ ok: false, reason: "native-canvas-unsupported" }); }); }); diff --git a/src/lib/documents/__tests__/prepare-vision-raster-failure.test.ts b/src/lib/documents/__tests__/prepare-vision-raster-failure.test.ts index 05af83fdc..3e01f2d75 100644 --- a/src/lib/documents/__tests__/prepare-vision-raster-failure.test.ts +++ b/src/lib/documents/__tests__/prepare-vision-raster-failure.test.ts @@ -1,5 +1,7 @@ import { describe, expect, it, vi } from "vitest"; import { Buffer } from "node:buffer"; +import { readFileSync } from "node:fs"; +import { join } from "node:path"; /** * Refs #776 — the raster-failed chain, end to end through the REAL @@ -68,4 +70,17 @@ describe("raster-failed end to end (#776)", () => { const vision = await prepareVisionInput(DOC as never, true); expect(vision).toMatchObject({ ok: true }); }); + + it("a PDF whose pages all render blank is rasterFailed, not an image of nothing (#1124)", async () => { + // Text in render mode 3 only: pdfjs opens it and draws an empty page. + // Before the blank-page guard this went to the provider as one white + // JPEG and came back as a near-empty transcript indexed as a success. + vi.mocked(decryptDocumentContent).mockReturnValue( + readFileSync( + join(process.cwd(), "tests/fixtures/pdf/invisible-text-only.pdf"), + ) as never, + ); + const vision = await prepareVisionInput(DOC as never, false); + expect(vision).toEqual({ ok: false, reason: "rasterFailed" }); + }); }); diff --git a/src/lib/documents/__tests__/rasterize-pdf-cleanup.test.ts b/src/lib/documents/__tests__/rasterize-pdf-cleanup.test.ts index 4e0d2cdba..bdafb0bfa 100644 --- a/src/lib/documents/__tests__/rasterize-pdf-cleanup.test.ts +++ b/src/lib/documents/__tests__/rasterize-pdf-cleanup.test.ts @@ -18,9 +18,16 @@ vi.mock("pdfjs-dist/legacy/build/pdf.mjs", () => ({ canvasFactory: { create: () => ({ canvas: { + width: 1, + height: 1, toBuffer: () => Buffer.from([0xff, 0xd8]), }, - context: {}, + // One black pixel: an inked page, so the blank-page guard passes. + context: { + getImageData: () => ({ + data: new Uint8ClampedArray([0, 0, 0, 255]), + }), + }, }), }, getPage: async () => ({ diff --git a/src/lib/documents/__tests__/rasterize-pdf-font-data.test.ts b/src/lib/documents/__tests__/rasterize-pdf-font-data.test.ts new file mode 100644 index 000000000..86fd7c5fe --- /dev/null +++ b/src/lib/documents/__tests__/rasterize-pdf-font-data.test.ts @@ -0,0 +1,155 @@ +import { Buffer } from "node:buffer"; +import { existsSync, readFileSync } from "node:fs"; +import { join } from "node:path"; + +import { createCanvas, loadImage } from "@napi-rs/canvas"; +import { beforeEach, describe, expect, it, vi } from "vitest"; + +/** + * GitHub #1124 — PDFs from older OCRmyPDF archives got no thumbnail and an + * image-only vision read of them came back nearly empty. + * + * The runtime image has no system fonts. pdfjs was opened without its own + * font data and told to lean on system fonts, so any text it could not draw + * from an embedded program (a non-embedded standard font, or an embedded + * program it cannot parse) came out as nothing, and the page that reached + * the provider was the background alone. A workstation hides this because + * its own fonts stand in, so the rendering assertions below are regression + * guards here and the container is where they were proven red; the option + * and registration assertions are red on any host. + * + * Fixtures (`tests/fixtures/pdf/`, synthetic text only): + * - `ocrmypdf11-two-images-type1c.pdf` — the reporter's shape: PDF 1.7, + * pikepdf producer, one 630.5 x 861.6 pt page with a 150 ppi RGB JPEG and + * a 75 ppi RGB Flate image, Type 1C Helvetica and Palatino-Roman, and an + * invisible OCR text layer. + * - `fallback-fonts.pdf` — a letter in non-embedded Helvetica and in a + * Type 1C Palatino-Roman whose program pdfjs cannot parse. + * - `invisible-text-only.pdf` — text in render mode 3 and nothing else, so + * every page renders blank. + */ + +const { annotate, registerFromPath } = vi.hoisted(() => ({ + annotate: vi.fn(), + registerFromPath: vi.fn(), +})); +vi.mock("@/lib/logging/context", () => ({ annotate })); + +const getDocumentOptions: Record[] = []; +vi.mock("pdfjs-dist/legacy/build/pdf.mjs", async (importActual) => { + const actual = + await importActual(); + return { + ...actual, + getDocument: (opts: Record) => { + getDocumentOptions.push(opts); + return actual.getDocument(opts as never); + }, + }; +}); +vi.mock("@napi-rs/canvas", async (importActual) => { + const actual = await importActual(); + return { + ...actual, + GlobalFonts: { + ...actual.GlobalFonts, + registerFromPath: (path: string, alias?: string) => { + registerFromPath(path, alias); + return actual.GlobalFonts.registerFromPath(path, alias); + }, + }, + }; +}); + +import * as raster from "../rasterize-pdf"; +import { generateThumbnail } from "../thumbnail"; + +const FIXTURES = join(process.cwd(), "tests", "fixtures", "pdf"); +const fixture = (name: string) => readFileSync(join(FIXTURES, name)); + +/** Share of dark pixels in a JPEG, measured the way the rasterizer does. */ +async function inkOf(base64: string): Promise { + const image = await loadImage(Buffer.from(base64, "base64")); + const canvas = createCanvas(image.width, image.height); + const ctx = canvas.getContext("2d"); + ctx.drawImage(image, 0, 0); + const { data } = ctx.getImageData(0, 0, image.width, image.height); + let dark = 0; + for (let i = 0; i < data.length; i += 4) { + if (data[i]! + data[i + 1]! + data[i + 2]! < 384) dark++; + } + return dark / (data.length / 4); +} + +beforeEach(() => { + annotate.mockClear(); + registerFromPath.mockClear(); + getDocumentOptions.length = 0; + raster.__resetPdfjsAssetsForTests(); +}); + +describe("rasterizePdf font and decoder data (#1124)", () => { + it("gives pdfjs its font, CMap and wasm directories and no system fonts", async () => { + const result = await raster.rasterizePdf(fixture("fallback-fonts.pdf"), 1); + expect(result.ok).toBe(true); + const opts = getDocumentOptions.at(-1)!; + expect(opts.useSystemFonts).toBe(false); + for (const [key, probe] of [ + ["standardFontDataUrl", "FoxitSerif.pfb"], + ["standardFontDataUrl", "LiberationSans-Regular.ttf"], + ["cMapUrl", "UniJIS-UTF16-H.bcmap"], + ["wasmUrl", "openjpeg.wasm"], + ] as const) { + expect(typeof opts[key], key).toBe("string"); + expect(existsSync(`${opts[key] as string}${probe}`), probe).toBe(true); + } + expect(opts.cMapPacked).toBe(true); + }); + + it("registers a fallback face for every generic family pdfjs falls back to", async () => { + await raster.rasterizePdf(fixture("fallback-fonts.pdf"), 1); + const families = new Set(registerFromPath.mock.calls.map((c) => c[1])); + expect(families).toEqual(new Set(["sans-serif", "serif", "monospace"])); + for (const [path] of registerFromPath.mock.calls) { + expect(existsSync(path as string)).toBe(true); + } + }); + + it("renders the OCRmyPDF page with its scan and its text", async () => { + const result = await raster.rasterizePdf( + fixture("ocrmypdf11-two-images-type1c.pdf"), + ); + expect(result.ok).toBe(true); + if (!result.ok) return; + expect(result.images).toHaveLength(1); + expect(await inkOf(result.images[0]!.dataBase64)).toBeGreaterThan(0.005); + }); + + it("renders a letter whose fonts pdfjs has to substitute", async () => { + const result = await raster.rasterizePdf(fixture("fallback-fonts.pdf")); + expect(result.ok).toBe(true); + if (!result.ok) return; + expect(await inkOf(result.images[0]!.dataBase64)).toBeGreaterThan(0.001); + }); + + it("reports a document whose every page renders blank as render-failed", async () => { + const result = await raster.rasterizePdf( + fixture("invisible-text-only.pdf"), + ); + expect(result).toEqual({ ok: false, reason: "render-failed" }); + expect(annotate).toHaveBeenCalledWith({ + action: { name: "documents.rasterize.failed" }, + meta: { reason: "blank_pages", pages: 1 }, + }); + }); +}); + +describe("generateThumbnail on the OCRmyPDF shape (#1124)", () => { + it("produces a preview of the first page", async () => { + const result = await generateThumbnail( + fixture("ocrmypdf11-two-images-type1c.pdf"), + "application/pdf", + ); + expect(result.ok).toBe(true); + }); +}); diff --git a/src/lib/documents/__tests__/thumbnail.test.ts b/src/lib/documents/__tests__/thumbnail.test.ts index 59a03ab64..05a2630d6 100644 --- a/src/lib/documents/__tests__/thumbnail.test.ts +++ b/src/lib/documents/__tests__/thumbnail.test.ts @@ -126,19 +126,19 @@ describe("generateThumbnail", () => { it("returns { ok: false } for an unsupported MIME type", async () => { await expect( generateThumbnail(Buffer.from("plain text"), "text/plain"), - ).resolves.toEqual({ ok: false }); + ).resolves.toEqual({ ok: false, reason: "unsupported-type" }); }); it("returns { ok: false } and never throws on malformed image bytes", async () => { await expect( generateThumbnail(Buffer.from("not a real image"), "image/png"), - ).resolves.toEqual({ ok: false }); + ).resolves.toEqual({ ok: false, reason: "error" }); }); it("returns { ok: false } on a malformed PDF", async () => { await expect( generateThumbnail(Buffer.from("not a pdf"), "application/pdf"), - ).resolves.toEqual({ ok: false }); + ).resolves.toEqual({ ok: false, reason: "raster-failed" }); }); it("refuses a decompression bomb before decoding (pixel cap)", async () => { @@ -153,6 +153,7 @@ describe("generateThumbnail", () => { bomb.writeUInt32BE(30_000, 20); // height await expect(generateThumbnail(bomb, "image/png")).resolves.toEqual({ ok: false, + reason: "pixel-cap", }); }); @@ -164,6 +165,6 @@ describe("generateThumbnail", () => { loadImageMock.mockResolvedValueOnce({ width: 30_000, height: 30_000 }); await expect( generateThumbnail(Buffer.from("desynced jpeg bytes"), "image/jpeg"), - ).resolves.toEqual({ ok: false }); + ).resolves.toEqual({ ok: false, reason: "pixel-cap" }); }); }); diff --git a/src/lib/documents/rasterize-pdf.ts b/src/lib/documents/rasterize-pdf.ts index f61e74fc3..452ddfae5 100644 --- a/src/lib/documents/rasterize-pdf.ts +++ b/src/lib/documents/rasterize-pdf.ts @@ -31,6 +31,9 @@ * preserves the "a bad document never aborts an upload/batch" contract. */ import { Buffer } from "node:buffer"; +import { existsSync } from "node:fs"; +import { createRequire } from "node:module"; +import { dirname, join } from "node:path"; import { annotate } from "@/lib/logging/context"; @@ -60,6 +63,14 @@ export const RASTER_TARGET_LONG_EDGE = 1600; /** JPEG quality (0-100). ~80 keeps text legible at a fraction of PNG's bytes. */ export const RASTER_JPEG_QUALITY = 80; +/** + * A rendered page counts as blank when fewer than this share of its pixels + * are dark. One short line of 16 pt text on an A4 page is ~0.1 %; a page + * number alone sits near this floor. A page set that is blank throughout is + * not a reading of the document, whatever the provider would make of it. + */ +export const RASTER_MIN_INK_RATIO = 0.0001; + /** One rasterized page, shaped as a vision `input_image` part. */ export interface RasterImage { mediaType: "image/jpeg"; @@ -98,9 +109,17 @@ interface PdfCanvas { height: number; toBuffer(mime: "image/jpeg", quality?: number): Buffer; } +interface PdfContext2D { + getImageData( + x: number, + y: number, + width: number, + height: number, + ): { data: Uint8ClampedArray }; +} interface PdfCanvasAndContext { canvas: PdfCanvas; - context: unknown; + context: PdfContext2D; } interface PdfCanvasFactory { create(width: number, height: number): PdfCanvasAndContext; @@ -125,12 +144,127 @@ interface PdfLoadingTask { destroy(): Promise; } interface PdfjsModule { - getDocument(opts: { - data: Uint8Array; - verbosity?: number; - isEvalSupported?: boolean; - useSystemFonts?: boolean; - }): PdfLoadingTask; + getDocument(opts: PdfDocumentOptions): PdfLoadingTask; +} +interface PdfDocumentOptions { + data: Uint8Array; + verbosity?: number; + isEvalSupported?: boolean; + useSystemFonts?: boolean; + standardFontDataUrl?: string; + cMapUrl?: string; + cMapPacked?: boolean; + wasmUrl?: string; +} +interface GlobalFontsLike { + registerFromPath(path: string, nameAlias?: string): unknown; +} + +/** + * Where pdfjs finds the data it does not carry in its JS: the standard 14 + * fonts (a PDF that names Helvetica or Times without embedding them), the + * CMaps (CID fonts with a predefined encoding), and the wasm image decoders + * (JPEG 2000, JBIG2). In a browser pdfjs fetches these from a URL; under Node + * they are read from disk, and without a directory pdfjs simply draws the text + * without glyphs. + * + * The runtime image carries no system fonts at all, so nothing else catches + * that: a letter set in a non-embedded font rendered as an empty page in the + * container while it rendered correctly on a workstation, where the operating + * system's fonts stood in. The image-only provider then read the empty page + * and returned a line of nothing. + * + * Resolved from the working directory with Node's own resolution, which is + * what finds the hoisted `node_modules/pdfjs-dist` in the standalone image + * and the pnpm link in a checkout. The specifier is assembled at runtime so + * the bundler leaves the lookup alone. The ICC profile option is not set: its + * loader needs a synchronous XMLHttpRequest that Node lacks, and pdfjs falls + * back to each colour space's alternate. + */ +export interface PdfjsAssets { + standardFontDataUrl: string; + cMapUrl: string; + wasmUrl: string; + /** The Liberation Sans faces registered as the generic fallback families. */ + fallbackFonts: string[]; +} + +let cachedAssets: PdfjsAssets | null | undefined; + +export function resolvePdfjsAssets(): PdfjsAssets | null { + if (cachedAssets !== undefined) return cachedAssets; + cachedAssets = null; + try { + const anchor = createRequire(join(process.cwd(), "package.json")); + const root = dirname( + anchor.resolve(["pdfjs-dist", "package.json"].join("/")), + ); + const fonts = join(root, "standard_fonts"); + const assets: PdfjsAssets = { + standardFontDataUrl: `${fonts}/`, + cMapUrl: `${join(root, "cmaps")}/`, + wasmUrl: `${join(root, "wasm")}/`, + fallbackFonts: [ + "LiberationSans-Regular.ttf", + "LiberationSans-Bold.ttf", + "LiberationSans-Italic.ttf", + "LiberationSans-BoldItalic.ttf", + ].map((file) => join(fonts, file)), + }; + if ( + existsSync(join(fonts, "FoxitSerif.pfb")) && + existsSync(join(root, "cmaps", "UniJIS-UTF16-H.bcmap")) && + existsSync(join(root, "wasm", "openjpeg.wasm")) && + assets.fallbackFonts.every((file) => existsSync(file)) + ) { + cachedAssets = assets; + } + } catch { + // Unresolvable: render without the data, as before, and say so below. + } + return cachedAssets; +} + +let fallbackFamiliesRegistered = false; + +/** + * Give the canvas a face for the generic families pdfjs falls back to. + * + * When pdfjs cannot use a font program (a damaged or unsupported embedded + * font, or a non-embedded face outside the standard 14) it draws the text + * with the canvas' own `fillText` under the PDF's font name and a generic + * family. With no system fonts in the image that text vanished. Registering + * Liberation Sans, which pdfjs ships, under `sans-serif`, `serif` and + * `monospace` makes it legible instead. Once per process. + */ +async function registerFallbackFamilies(assets: PdfjsAssets): Promise { + if (fallbackFamiliesRegistered) return; + fallbackFamiliesRegistered = true; + const { GlobalFonts } = (await import("@napi-rs/canvas")) as unknown as { + GlobalFonts: GlobalFontsLike; + }; + for (const family of ["sans-serif", "serif", "monospace"]) { + for (const file of assets.fallbackFonts) { + GlobalFonts.registerFromPath(file, family); + } + } +} + +/** Share of dark pixels on a rendered page (luminance below half). */ +function inkRatio(context: PdfContext2D, width: number, height: number) { + const { data } = context.getImageData(0, 0, width, height); + let dark = 0; + for (let i = 0; i < data.length; i += 4) { + if (data[i]! + data[i + 1]! + data[i + 2]! < 384) dark++; + } + const pixels = data.length / 4; + return pixels > 0 ? dark / pixels : 0; +} + +/** Test hook: the asset lookup and the font registration are per process. */ +export function __resetPdfjsAssetsForTests(): void { + cachedAssets = undefined; + fallbackFamiliesRegistered = false; } /** @@ -163,11 +297,31 @@ export async function rasterizePdf( const pdfjs = (await import("pdfjs-dist/legacy/build/pdf.mjs")) as unknown as PdfjsModule; + const assets = resolvePdfjsAssets(); + if (assets) { + await registerFallbackFamilies(assets); + } else { + annotate({ + action: { name: "documents.rasterize.assetsMissing" }, + meta: { reason: "pdfjs_assets_unresolved" }, + }); + } + task = pdfjs.getDocument({ data: new Uint8Array(buffer), verbosity: 0, // VerbosityLevel.ERRORS — no console spam on odd PDFs. isEvalSupported: false, // never eval font programs (defence in depth). - useSystemFonts: true, + // There are no system fonts to use: substitutes come from pdfjs' own + // standard font data and the fallback families registered above. + useSystemFonts: false, + ...(assets + ? { + standardFontDataUrl: assets.standardFontDataUrl, + cMapUrl: assets.cMapUrl, + cMapPacked: true, + wasmUrl: assets.wasmUrl, + } + : {}), }); doc = await task.promise; @@ -175,6 +329,7 @@ export async function rasterizePdf( const cap = Math.min(Math.max(1, maxPages), RASTER_MAX_PAGES); const pageCount = Math.min(doc.numPages, cap); const images: RasterImage[] = []; + let blankPages = 0; for (let pageNumber = 1; pageNumber <= pageCount; pageNumber++) { const page = await doc.getPage(pageNumber); @@ -196,6 +351,12 @@ export async function rasterizePdf( viewport, canvas: cc.canvas, }).promise; + if ( + inkRatio(cc.context, cc.canvas.width, cc.canvas.height) < + RASTER_MIN_INK_RATIO + ) { + blankPages++; + } const jpeg = cc.canvas.toBuffer("image/jpeg", RASTER_JPEG_QUALITY); images.push({ mediaType: "image/jpeg", @@ -217,9 +378,19 @@ export async function rasterizePdf( }); return { ok: false, reason: "render-failed" }; } + if (blankPages === images.length) { + // Every rendered page came out empty. Sending those to a vision model + // buys a transcript of nothing that would be indexed as a success; say + // the render failed instead, so the read reports `raster-failed`. + annotate({ + action: { name: "documents.rasterize.failed" }, + meta: { reason: "blank_pages", pages: images.length }, + }); + return { ok: false, reason: "render-failed" }; + } annotate({ action: { name: "documents.rasterize.ok" }, - meta: { pages: images.length, cappedFrom: doc.numPages }, + meta: { pages: images.length, cappedFrom: doc.numPages, blankPages }, }); return { ok: true, images, pageCount: doc.numPages }; } catch (err) { diff --git a/src/lib/documents/thumbnail.ts b/src/lib/documents/thumbnail.ts index 8d975954f..26ed69c1b 100644 --- a/src/lib/documents/thumbnail.ts +++ b/src/lib/documents/thumbnail.ts @@ -157,9 +157,23 @@ export interface Thumbnail { height: number; } +/** + * Why no preview was made. `unsupported-type` is the expected answer for an + * Office file or a text document; every other reason means a preview should + * have existed, so the thumbnail job logs it as a warning. + */ +export type ThumbnailSkipReason = + | "unsupported-type" + | "native-canvas-unsupported" + | "pixel-cap" + | "raster-failed" + | "empty-render" + | "error"; + /** Best-effort generation outcome — never an exception. */ export type ThumbnailResult = - { ok: true; thumbnail: Thumbnail } | { ok: false }; + | { ok: true; thumbnail: Thumbnail } + | { ok: false; reason: ThumbnailSkipReason }; // Minimal structural types for the slice of `@napi-rs/canvas` this module uses. // A type-only shape (erased at build) so we never eagerly evaluate the module @@ -229,7 +243,7 @@ export async function generateThumbnail( action: { name: "documents.thumbnail.rejected" }, meta: { reason: "native_canvas_unsupported" }, }); - return { ok: false }; + return { ok: false, reason: "native-canvas-unsupported" }; } // Lazy import: keeps @napi-rs/canvas (+ transitively pdfjs on the PDF path) // out of the eager server chunk. Resolved on first generation, then cached. @@ -244,7 +258,7 @@ export async function generateThumbnail( action: { name: "documents.thumbnail.rejected" }, meta: { source: "image", reason: "pixel_cap", pixels }, }); - return { ok: false }; + return { ok: false, reason: "pixel-cap" }; } const image = await napi.loadImage(bytes); // Post-decode backstop: the header sniff returns null on any marker @@ -260,10 +274,10 @@ export async function generateThumbnail( pixels: image.width * image.height, }, }); - return { ok: false }; + return { ok: false, reason: "pixel-cap" }; } const thumb = drawScaledJpeg(napi, image); - if (!thumb) return { ok: false }; + if (!thumb) return { ok: false, reason: "empty-render" }; annotate({ action: { name: "documents.thumbnail.ok" }, meta: { source: "image", width: thumb.width, height: thumb.height }, @@ -274,11 +288,13 @@ export async function generateThumbnail( if (mimeType === "application/pdf") { // Page 1 only — a preview never renders the whole document. const raster = await rasterizePdf(bytes, 1); - if (!raster.ok || raster.images.length === 0) return { ok: false }; + if (!raster.ok || raster.images.length === 0) { + return { ok: false, reason: "raster-failed" }; + } const pageJpeg = Buffer.from(raster.images[0]!.dataBase64, "base64"); const image = await napi.loadImage(pageJpeg); const thumb = drawScaledJpeg(napi, image); - if (!thumb) return { ok: false }; + if (!thumb) return { ok: false, reason: "empty-render" }; annotate({ action: { name: "documents.thumbnail.ok" }, meta: { source: "pdf", width: thumb.width, height: thumb.height }, @@ -287,7 +303,7 @@ export async function generateThumbnail( } // Every other MIME (Office/text/TIFF/HEIC/XML/JSON) — no preview. - return { ok: false }; + return { ok: false, reason: "unsupported-type" }; } catch (err) { annotate({ action: { name: "documents.thumbnail.failed" }, @@ -297,6 +313,6 @@ export async function generateThumbnail( message: err instanceof Error ? err.message.slice(0, 300) : String(err), }, }); - return { ok: false }; + return { ok: false, reason: "error" }; } } diff --git a/src/lib/idempotency.ts b/src/lib/idempotency.ts index 70e2ddcbc..7b374faf3 100644 --- a/src/lib/idempotency.ts +++ b/src/lib/idempotency.ts @@ -11,6 +11,19 @@ * the record the request claims is folded into `key` (see `cellKey`), so the * tuple identifies a request to one person's record rather than a request by * one person. A caller acting only as themselves is unaffected, byte for byte. + * + * A cell also remembers WHICH request it answered: a SHA-256 of the canonical + * request body (`requestFingerprint`). A key names one request, not a slot, and + * a client that reuses a key for a second, different request must not be + * handed the first request's response. Native clients up to 1.1.0 sent the two + * halves of a manual blood pressure as two POSTs under one key; the second half + * replayed the first's 201 and was never written. A lookup therefore replays, + * or reports in-flight, only for the same body. A different body — or a row + * written before fingerprints existed — runs the handler normally without + * claiming or caching anything, and the existing cell stays for its own + * retries. This is deliberately not the IETF draft's 422: refusing would turn + * every affected client's second write into a visible error with no way for + * the person to recover the value. */ import type { NextRequest } from "next/server"; import { headers } from "next/headers"; @@ -109,6 +122,155 @@ export interface IdempotencyContext { key: string; method: string; path: string; + /** See `requestFingerprint`. Never the body itself. */ + fingerprint: string; +} + +/** + * How much of a body the fingerprint reads before it stops. The wrapper runs + * before the handler's own bounded readers and upload slots, so it must not + * buffer an arbitrarily large body on their behalf: reading the clone holds + * what it has read in the original's queue until the handler consumes it. + * Every JSON write the cache fronts is far below this; the one multipart route + * (document upload) is above it, and there the prefix plus the declared length + * identifies a retry of the same upload, with the route's own content hash as + * the duplicate check behind it. + */ +const FINGERPRINT_MAX_BYTES = 1024 * 1024; + +/** + * JSON with every object's keys sorted, recursively. Arrays keep their order — + * it is part of the request. Exported for its unit tests. + */ +export function canonicalJson(value: unknown): string { + if (Array.isArray(value)) { + return `[${value.map((v) => canonicalJson(v)).join(",")}]`; + } + if (value !== null && typeof value === "object") { + const entries = Object.keys(value) + .sort() + .map( + (k) => + `${JSON.stringify(k)}:${canonicalJson((value as Record)[k])}`, + ); + return `{${entries.join(",")}}`; + } + return JSON.stringify(value); +} + +/** + * Replace every occurrence of the multipart boundary named in `contentType` + * with a fixed token. Any other body is returned as it is. + */ +function withoutMultipartBoundary( + body: Uint8Array, + contentType: string | null, +): Uint8Array { + if (!contentType || !/^multipart\//i.test(contentType)) return body; + const match = /;\s*boundary="?([^";]+)"?/i.exec(contentType); + if (!match) return body; + const boundary = Buffer.from(match[1], "utf8"); + const token = Buffer.from("boundary", "utf8"); + const source = Buffer.from(body.buffer, body.byteOffset, body.byteLength); + const parts: Buffer[] = []; + let from = 0; + for (;;) { + const at = source.indexOf(boundary, from); + if (at === -1) break; + parts.push(source.subarray(from, at), token); + from = at + boundary.length; + } + if (parts.length === 0) return body; + parts.push(source.subarray(from)); + return new Uint8Array(Buffer.concat(parts)); +} + +/** + * The fingerprint of a request: SHA-256 hex over method, path and the body in + * canonical form. A body that parses as JSON is hashed as `canonicalJson`, so + * the same object serialised with a different key order is the same request; + * anything else (multipart, plain text, an empty body) is hashed as its raw + * bytes. `truncatedAt` marks a body cut at `FINGERPRINT_MAX_BYTES`, hashed as + * that raw prefix together with the declared length. A multipart body has its + * boundary replaced by a fixed token first: a client that rebuilds the form + * for a retry picks a fresh random boundary, and the same upload must still + * replay. Exported for unit tests. + */ +export function requestFingerprint( + method: string, + path: string, + rawBody: Uint8Array, + truncatedAt: string | null = null, + contentType: string | null = null, +): string { + const body = withoutMultipartBoundary(rawBody, contentType); + const hash = createHash("sha256") + .update(method) + .update("\0") + .update(path) + .update("\0"); + if (truncatedAt !== null) { + return hash.update(`partial:${truncatedAt}\0`).update(body).digest("hex"); + } + if (body.length > 0) { + try { + const text = new TextDecoder("utf-8", { fatal: true }).decode(body); + return hash + .update("json:") + .update(canonicalJson(JSON.parse(text))) + .digest("hex"); + } catch { + // Not UTF-8 or not JSON — fall through to the raw bytes. + } + } + return hash.update("raw:").update(body).digest("hex"); +} + +/** + * Read the request's body from a clone, at most `FINGERPRINT_MAX_BYTES`, and + * fingerprint it. The original request keeps its body for the handler. + */ +async function fingerprintRequest( + request: Request | NextRequest, + path: string, +): Promise { + const stream = request.clone().body; + if (!stream) + return requestFingerprint(request.method, path, new Uint8Array()); + const reader = stream.getReader(); + const chunks: Uint8Array[] = []; + let total = 0; + let truncated = false; + for (;;) { + const { done, value } = await reader.read(); + if (done) break; + if (total + value.byteLength > FINGERPRINT_MAX_BYTES) { + chunks.push(value.subarray(0, FINGERPRINT_MAX_BYTES - total)); + total = FINGERPRINT_MAX_BYTES; + truncated = true; + // Cancelling this branch of the tee stops it from queueing the rest of + // the body, and leaves the handler's branch intact. Not awaited: a tee + // branch's cancel settles only once BOTH branches are done, which is + // after the handler has run. + void reader.cancel().catch(() => {}); + break; + } + chunks.push(value); + total += value.byteLength; + } + const body = new Uint8Array(total); + let offset = 0; + for (const chunk of chunks) { + body.set(chunk, offset); + offset += chunk.byteLength; + } + return requestFingerprint( + request.method, + path, + body, + truncated ? (request.headers.get("content-length") ?? "unknown") : null, + request.headers.get("content-type"), + ); } function getIdempotencyKey(request: Request | NextRequest): string | null { @@ -125,10 +287,16 @@ function getIdempotencyKey(request: Request | NextRequest): string | null { * - `{ kind: "pending" }` — another request holds an in-flight claim * for this key; the caller must NOT run the * handler again. + * - `{ kind: "mismatch" }` — the live row answered a different body (or + * predates fingerprints); the caller runs the + * handler without claiming or caching. * - `null` — no live row; the caller may claim + run. */ type CacheLookup = - { kind: "replay"; response: NextResponse } | { kind: "pending" } | null; + | { kind: "replay"; response: NextResponse } + | { kind: "pending" } + | { kind: "mismatch" } + | null; /** * Look up the state of a (userId, key, method, path) tuple. Distinguishes @@ -157,6 +325,13 @@ async function findCached(ctx: IdempotencyContext): Promise { return null; } + // Checked before both the pending and the replay arm: a different request + // under the same key is neither in flight nor answered. A legacy row's null + // never equals a fingerprint. + if (row.requestFingerprint !== ctx.fingerprint) { + return { kind: "mismatch" }; + } + if (row.responseStatus === PENDING_STATUS) { // A concurrent request claimed this key and is still running its // side-effect. Signal the caller to refuse rather than double-execute. @@ -208,6 +383,7 @@ async function claimKey(ctx: IdempotencyContext): Promise { key: ctx.key, method: ctx.method, path: ctx.path, + requestFingerprint: ctx.fingerprint, responseStatus: PENDING_STATUS, responseBody: "", expiresAt: new Date(Date.now() + PENDING_TTL_MS), @@ -233,6 +409,7 @@ async function releaseClaim(ctx: IdempotencyContext): Promise { key: ctx.key, method: ctx.method, path: ctx.path, + requestFingerprint: ctx.fingerprint, responseStatus: PENDING_STATUS, }, }) @@ -312,6 +489,7 @@ async function persistCached( key: ctx.key, method: ctx.method, path: ctx.path, + requestFingerprint: ctx.fingerprint, }, data: { responseStatus: response.status, @@ -502,6 +680,23 @@ async function narrowTokenFacet(): Promise { return apiToken.permissions.includes("*") ? null : `t:${apiToken.id}`; } +/** + * Run a request whose key is already taken by a different body: the handler + * runs exactly as it would without a key, and nothing is claimed or cached, so + * the existing cell keeps answering its own retries. + */ +function runUncached( + ctx: IdempotencyContext, + handler: (...args: Args) => Promise, + args: Args, +): Promise { + annotate({ + action: { name: "idempotency.key.body_mismatch" }, + meta: { method: ctx.method, path: ctx.path }, + }); + return handler(...args); +} + export function withIdempotency< Args extends [Request | NextRequest, ...unknown[]], >( @@ -589,14 +784,27 @@ export function withIdempotency< if (tokenFacet !== null) facets.push(tokenFacet); const url = new URL(request.url); + const fingerprint = await fingerprintRequest(request, url.pathname).catch( + () => null, + ); + if (fingerprint === null) { + // The body could not be read for the fingerprint (already consumed, or + // the stream failed). Without knowing which request this is, no cell + // may answer for it; the handler meets the body problem itself. + return handler(...args); + } const ctx: IdempotencyContext = { userId, key: cellKey(key, claimedRecord, facets), method: request.method, path: url.pathname, + fingerprint, }; const cached = await findCached(ctx); + if (cached?.kind === "mismatch") { + return runUncached(ctx, handler, args); + } if (cached?.kind === "replay") { return cached.response; } @@ -617,6 +825,13 @@ export function withIdempotency< // one caller may execute the side-effect for a given key. const won = await claimKey(ctx); if (!won) { + // The racing claim may be a different request under the same key — + // the two halves of one reading posted together. That one is not a + // duplicate of this one and must still run. + const raced = await findCached(ctx); + if (raced?.kind === "mismatch") { + return runUncached(ctx, handler, args); + } annotate({ action: { name: "idempotency.inflight_conflict" }, meta: { method: ctx.method, path: ctx.path }, diff --git a/src/lib/jobs/__tests__/document-thumbnail.test.ts b/src/lib/jobs/__tests__/document-thumbnail.test.ts new file mode 100644 index 000000000..4270507d1 --- /dev/null +++ b/src/lib/jobs/__tests__/document-thumbnail.test.ts @@ -0,0 +1,98 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; + +/** + * GitHub #1124 — the thumbnail job ran 355 times on one instance, left + * sixteen PDFs without a preview, and the log held not one line about it: + * the job opened no wide event, so every annotation went nowhere. Pins: each + * run is a `job.document_thumbnail` background event; a run that should have + * made a preview and did not is raised to `warn` with the reason; a type that + * never has a preview stays at info. + */ + +const { evt, findFirst, upsert, generateThumbnail, withBackgroundEvent } = + vi.hoisted(() => { + const evt = { addMeta: vi.fn(), elevateLevel: vi.fn() }; + return { + evt, + findFirst: vi.fn(), + upsert: vi.fn(), + generateThumbnail: vi.fn(), + withBackgroundEvent: vi.fn( + async (_name: string, fn: (e: typeof evt) => Promise) => + fn(evt), + ), + }; + }); + +vi.mock("@/lib/db", () => ({ + prisma: { + inboundDocument: { findFirst }, + documentThumbnail: { upsert }, + }, +})); +vi.mock("@/lib/documents/store", () => ({ + decryptDocumentContent: vi.fn(() => Buffer.from("%PDF-1.7")), + encryptThumbnail: vi.fn(() => new Uint8Array([1])), +})); +vi.mock("@/lib/documents/thumbnail", () => ({ generateThumbnail })); +vi.mock("@/lib/jobs/boss-instance", () => ({ getGlobalBoss: vi.fn() })); +vi.mock("@/lib/logging/context", () => ({ annotate: vi.fn() })); +vi.mock("@/lib/logging/background", () => ({ withBackgroundEvent })); + +import { runDocumentThumbnail } from "../document-thumbnail"; + +const DOC = { + id: "doc-1", + mimeType: "application/pdf", + contentEncrypted: new Uint8Array([1]), + contentCodec: "binary2", + thumbnail: null, +}; + +beforeEach(() => { + vi.clearAllMocks(); + findFirst.mockResolvedValue(DOC); +}); + +describe("runDocumentThumbnail wide event", () => { + it("runs inside a background event named for the job", async () => { + generateThumbnail.mockResolvedValue({ ok: false, reason: "raster-failed" }); + await runDocumentThumbnail({ userId: "user-1", documentId: "doc-1" }); + expect(withBackgroundEvent).toHaveBeenCalledWith( + "job.document_thumbnail", + expect.any(Function), + ); + expect(evt.addMeta).toHaveBeenCalledWith("document_id", "doc-1"); + }); + + it("warns with the reason when a PDF gets no preview", async () => { + generateThumbnail.mockResolvedValue({ ok: false, reason: "error" }); + await runDocumentThumbnail({ userId: "user-1", documentId: "doc-1" }); + expect(evt.elevateLevel).toHaveBeenCalledWith("warn"); + expect(evt.addMeta).toHaveBeenCalledWith( + "thumbnail_missing_reason", + "error", + ); + expect(upsert).not.toHaveBeenCalled(); + }); + + it("stays at info for a type that never has a preview", async () => { + findFirst.mockResolvedValue({ ...DOC, mimeType: "text/plain" }); + generateThumbnail.mockResolvedValue({ + ok: false, + reason: "unsupported-type", + }); + await runDocumentThumbnail({ userId: "user-1", documentId: "doc-1" }); + expect(evt.elevateLevel).not.toHaveBeenCalled(); + }); + + it("stores the preview and stays at info when one is made", async () => { + generateThumbnail.mockResolvedValue({ + ok: true, + thumbnail: { jpeg: Buffer.from([0xff, 0xd8]), width: 2, height: 3 }, + }); + await runDocumentThumbnail({ userId: "user-1", documentId: "doc-1" }); + expect(upsert).toHaveBeenCalledOnce(); + expect(evt.elevateLevel).not.toHaveBeenCalled(); + }); +}); diff --git a/src/lib/jobs/document-thumbnail.ts b/src/lib/jobs/document-thumbnail.ts index 91a8d44ed..1b0a1a91d 100644 --- a/src/lib/jobs/document-thumbnail.ts +++ b/src/lib/jobs/document-thumbnail.ts @@ -15,6 +15,12 @@ * kind icon), and `runDocumentThumbnail` returns cleanly so pg-boss does not * retry it. Only an unexpected throw (a DB hiccup) propagates for retry. * + * Each run is one background wide event (`job.document_thumbnail`). A run + * that should have produced a preview and did not is raised to `warn` with + * the reason, so a document left with its kind icon is explained in the log. + * Without the event every annotation below went nowhere: a whole class of + * PDFs went without previews for weeks and the log never said so. + * * The queue MUST be registered in the maintenance registrar * (`src/lib/jobs/reminder/register-maintenance.ts`) so pg-boss provisions it. */ @@ -25,6 +31,7 @@ import { } from "@/lib/documents/store"; import { generateThumbnail } from "@/lib/documents/thumbnail"; import { getGlobalBoss } from "@/lib/jobs/boss-instance"; +import { withBackgroundEvent } from "@/lib/logging/background"; import { annotate } from "@/lib/logging/context"; export const DOCUMENT_THUMBNAIL_QUEUE = "document-thumbnail"; @@ -49,7 +56,20 @@ export async function runDocumentThumbnail( ): Promise { const { userId, documentId } = payload; if (!userId || !documentId) return; + await withBackgroundEvent("job.document_thumbnail", async (evt) => { + evt.addMeta("document_id", documentId); + await renderThumbnail(userId, documentId, (reason) => { + evt.addMeta("thumbnail_missing_reason", reason); + evt.elevateLevel("warn"); + }); + }); +} +async function renderThumbnail( + userId: string, + documentId: string, + warn: (reason: string) => void, +): Promise { // Owner-scoped load; the blob column is selected ONLY here (the job that // renders the preview), never in the list/detail queries. Skip a document // that already has a thumbnail so a re-enqueue is a cheap no-op. @@ -84,6 +104,7 @@ export async function runDocumentThumbnail( action: { name: "documents.thumbnail.decryptFailed" }, meta: { documentId }, }); + warn("decrypt-failed"); return; } @@ -91,8 +112,11 @@ export async function runDocumentThumbnail( if (!result.ok) { annotate({ action: { name: "documents.thumbnail.run" }, - meta: { documentId, generated: false }, + meta: { documentId, generated: false, mimeType: document.mimeType }, }); + // An Office or text document has no preview by design; anything else + // was meant to get one. + if (result.reason !== "unsupported-type") warn(result.reason); return; } diff --git a/src/lib/openapi/routes/shared.ts b/src/lib/openapi/routes/shared.ts index c96613bc0..831933630 100644 --- a/src/lib/openapi/routes/shared.ts +++ b/src/lib/openapi/routes/shared.ts @@ -391,7 +391,8 @@ export const profileEmailRateLimitResponse = { * * Scope, so nobody reads more into it than is there: only POST, PUT, PATCH and * DELETE are covered (`SUPPORTED_METHODS`), the key is scoped per user and per - * `(method, path)`, and `isCachableStatus` refuses to cache 5xx or the + * `(method, path)`, a replay needs the same body (a different body under the + * same key runs as a new write and is neither replayed nor refused), and `isCachableStatus` refuses to cache 5xx or the * transient 401 / 403 / 408 / 429 — a retry after one of those runs for real. * * `src/__tests__/idempotency-contract-publication.test.ts` holds this to the @@ -410,7 +411,7 @@ export const idempotencyKeyParameter = { maxLength: 128, }, description: - "Makes this write safe to retry. Send the same key again and the first response is replayed rather than the work repeated, marked with `X-Idempotent-Replay: true`. A key that does not match the pattern is IGNORED rather than refused — the write then proceeds unprotected and looks exactly like a request that carried no key, so validate the key on your side. Scoped per user and per (method, path): the same key on a different route is a different key. While a first request under this key is still in flight, a second is refused with 409 rather than queued.", + "Makes this write safe to retry. Send the same key with the same body again and the first response is replayed rather than the work repeated, marked with `X-Idempotent-Replay: true`. The body is compared as JSON with key order ignored; a different body under the same key is not a retry and runs as a new write, while the first response stays stored for retries of the first request. A key that does not match the pattern is IGNORED rather than refused — the write then proceeds unprotected and looks exactly like a request that carried no key, so validate the key on your side. Scoped per user and per (method, path): the same key on a different route is a different key. While a first request under this key is still in flight, a second with the same body is refused with 409 rather than queued.", }; /** diff --git a/tests/fixtures/pdf/fallback-fonts.pdf b/tests/fixtures/pdf/fallback-fonts.pdf new file mode 100644 index 000000000..ee01b4fa0 Binary files /dev/null and b/tests/fixtures/pdf/fallback-fonts.pdf differ diff --git a/tests/fixtures/pdf/invisible-text-only.pdf b/tests/fixtures/pdf/invisible-text-only.pdf new file mode 100644 index 000000000..929c6870c Binary files /dev/null and b/tests/fixtures/pdf/invisible-text-only.pdf differ diff --git a/tests/fixtures/pdf/ocrmypdf11-two-images-type1c.pdf b/tests/fixtures/pdf/ocrmypdf11-two-images-type1c.pdf new file mode 100644 index 000000000..b9b9bd6a6 Binary files /dev/null and b/tests/fixtures/pdf/ocrmypdf11-two-images-type1c.pdf differ diff --git a/tests/integration/idempotency-body-fingerprint.test.ts b/tests/integration/idempotency-body-fingerprint.test.ts new file mode 100644 index 000000000..c0d04d93c --- /dev/null +++ b/tests/integration/idempotency-body-fingerprint.test.ts @@ -0,0 +1,249 @@ +/** + * One `Idempotency-Key`, two different requests. + * + * Native clients up to 1.1.0 post a manual blood pressure as two + * `POST /api/measurements` calls — the systolic half, then the diastolic half — + * under the same key. While the replay cell was keyed on + * `(user, key, method, path)` alone, the second call was answered with the + * first call's cached 201 and the diastolic value was never written. The cell + * now carries a fingerprint of the request body: only the same body replays. + * + * Everything here is real — the shipped route, the shipped wrapper, the + * session resolver and the `idempotency_keys` unique index. + */ +import { readFileSync } from "node:fs"; +import { join } from "node:path"; + +import { NextRequest } from "next/server"; +import { beforeEach, describe, expect, it, vi } from "vitest"; + +import { POST as postMeasurement } from "@/app/api/measurements/route"; + +import { cookieJar, headerJar } from "./mock-next-headers"; +import { getPrismaClient, truncateAllTables } from "./setup"; + +process.env.ENCRYPTION_KEY ??= + "0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef"; + +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), +})); + +const USER_A = "idem-fingerprint-a"; +const USER_B = "idem-fingerprint-b"; +const KEY = "bp-entry-5c1e2a7f"; +const MEASURED_AT = "2026-09-30T07:15:00.000Z"; + +const SYS = { type: "BLOOD_PRESSURE_SYS", value: 128, measuredAt: MEASURED_AT }; +const DIA = { type: "BLOOD_PRESSURE_DIA", value: 84, measuredAt: MEASURED_AT }; + +const sessions = new Map(); + +function post(body: unknown, key = KEY): Promise { + return postMeasurement( + new NextRequest("http://localhost/api/measurements", { + method: "POST", + headers: { + "content-type": "application/json", + "idempotency-key": key, + }, + body: JSON.stringify(body), + }), + ); +} + +function signInAs(userId: string) { + cookieJar.set("healthlog_session", sessions.get(userId)!); +} + +async function rowsOf(userId: string) { + const rows = await getPrismaClient().measurement.findMany({ + where: { userId, deletedAt: null }, + select: { type: true, value: true }, + }); + // By name, not by the enum's declaration order. + return rows.sort((a, b) => a.type.localeCompare(b.type)); +} + +beforeEach(async () => { + const prisma = getPrismaClient(); + await truncateAllTables(prisma); + cookieJar.clear(); + headerJar.clear(); + sessions.clear(); + for (const id of [USER_A, USER_B]) { + await prisma.user.create({ + data: { + id, + username: id, + email: `${id}@example.test`, + timezone: "UTC", + }, + }); + const session = await prisma.session.create({ + data: { userId: id, expiresAt: new Date(Date.now() + 60 * 60 * 1000) }, + }); + sessions.set(id, session.id); + } + signInAs(USER_A); +}); + +describe("Idempotency-Key reused for a different body (real Postgres)", () => { + it("writes both halves of a blood pressure posted under one key", async () => { + const first = await post(SYS); + expect(first.status).toBe(201); + + const second = await post(DIA); + expect(second.status).toBe(201); + expect(second.headers.get("X-Idempotent-Replay")).toBeNull(); + + expect(await rowsOf(USER_A)).toEqual([ + { type: "BLOOD_PRESSURE_DIA", value: 84 }, + { type: "BLOOD_PRESSURE_SYS", value: 128 }, + ]); + }); + + it("still replays a retry of the same body and writes one row", async () => { + const first = await post(SYS); + expect(first.status).toBe(201); + const firstBody = await first.json(); + + // Same object, different key order: the same request. + const retry = await post({ + measuredAt: MEASURED_AT, + value: 128, + type: "BLOOD_PRESSURE_SYS", + }); + expect(retry.status).toBe(201); + expect(retry.headers.get("X-Idempotent-Replay")).toBe("true"); + expect(await retry.json()).toEqual(firstBody); + + expect(await rowsOf(USER_A)).toEqual([ + { type: "BLOOD_PRESSURE_SYS", value: 128 }, + ]); + }); + + it("keeps the first cell answering its own retries after a different body ran", async () => { + await post(SYS); + await post(DIA); + const retry = await post(SYS); + expect(retry.headers.get("X-Idempotent-Replay")).toBe("true"); + expect(await rowsOf(USER_A)).toHaveLength(2); + const cells = await getPrismaClient().idempotencyKey.findMany({ + where: { userId: USER_A }, + }); + expect(cells).toHaveLength(1); + expect(cells[0]?.requestFingerprint).toMatch(/^[0-9a-f]{64}$/); + }); + + it("writes both halves when they are in flight at the same time", async () => { + const [a, b] = await Promise.all([post(SYS), post(DIA)]); + expect([a.status, b.status]).toEqual([201, 201]); + expect(await rowsOf(USER_A)).toEqual([ + { type: "BLOOD_PRESSURE_DIA", value: 84 }, + { type: "BLOOD_PRESSURE_SYS", value: 128 }, + ]); + }); + + it("still lets only one of two identical bodies in flight run", async () => { + const [a, b] = await Promise.all([post(SYS), post(SYS)]); + expect([a.status, b.status].sort()).toEqual([201, 409]); + expect(await rowsOf(USER_A)).toHaveLength(1); + }); + + it("does not replay a row written before fingerprints existed", async () => { + await getPrismaClient().idempotencyKey.create({ + data: { + userId: USER_A, + key: KEY, + method: "POST", + path: "/api/measurements", + responseStatus: 201, + responseBody: JSON.stringify({ data: { id: "legacy" }, error: null }), + expiresAt: new Date(Date.now() + 60 * 60 * 1000), + }, + }); + + const res = await post(DIA); + expect(res.status).toBe(201); + expect(res.headers.get("X-Idempotent-Replay")).toBeNull(); + expect(await rowsOf(USER_A)).toEqual([ + { type: "BLOOD_PRESSURE_DIA", value: 84 }, + ]); + // The legacy row is left alone, not claimed over. + const cell = await getPrismaClient().idempotencyKey.findFirstOrThrow({ + where: { userId: USER_A, key: KEY }, + }); + expect(cell.requestFingerprint).toBeNull(); + }); + + it("never hands one user's cached response to another using the same key", async () => { + const first = await post(SYS); + const firstBody = await first.json(); + + signInAs(USER_B); + const other = await post(SYS); + expect(other.status).toBe(201); + expect(other.headers.get("X-Idempotent-Replay")).toBeNull(); + expect(await other.json()).not.toEqual(firstBody); + + expect(await rowsOf(USER_A)).toHaveLength(1); + expect(await rowsOf(USER_B)).toHaveLength(1); + }); + + it("the documented operator query finds a systolic reading left without its diastolic half", async () => { + const doc = readFileSync( + join(process.cwd(), "docs/ops/blood-pressure-missing-diastolic.md"), + "utf8", + ); + const block = + /([\s\S]*?)/.exec( + doc, + ); + const sql = block?.[1] + .replace(/```sql|```/g, "") + .trim() + .replace(/;$/, ""); + expect(sql).toBeTruthy(); + + // A complete reading through the fixed path, and the damage the old + // cache left behind: a systolic half on its own. + await post(SYS); + await post(DIA); + const lone = await getPrismaClient().measurement.create({ + data: { + userId: USER_A, + type: "BLOOD_PRESSURE_SYS", + value: 141, + unit: "mmHg", + measuredAt: new Date("2026-09-29T19:40:00.000Z"), + }, + }); + + const found = await getPrismaClient().$queryRawUnsafe< + Array<{ id: string; systolic: number }> + >(sql!); + expect(found.map((r) => [r.id, r.systolic])).toEqual([[lone.id, 141]]); + }); +});