mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-09 13:01:00 +00:00
chore(release): v1.28.49 — Apple Health re-import survives a natural-key collision
The Apple Health ZIP import upserted each spot reading on the (userId, type, source, externalId) index only. When two readings shared the natural key (userId, type, measuredAt, source, sleepStage) with different externalIds — a re-import of overlapping data, or two same-instant Apple samples from different source apps — the externalId upsert missed and the create collided on the second unique index (migration 0055), raising P2002 and aborting the entire import (issue #486, after the v1.28.36 dedup fix let the upload run). flushSpotBatch now wraps the per-row upsert: on P2002 it probes the occupying row by the natural key (tombstone-inclusive) and adopts it in place — re-keys to the incoming externalId, takes the new value/unit, resurrects if soft-deleted — mirroring the v1.28.39 provider natural-key rescues (closest to withings/sync-sleep). If no twin resolves it, the one reading is skipped and the run continues; the catch sits in the per-row loop outside any transaction, so a collision can never abort the batch or the import. Adopted rows count as updated, skipped ones surface honestly. Workout + stats: paths verified unable to hit this (no second unique index / deterministic externalId). Repro + real-Postgres integration tests added.
This commit is contained in:
1 parent
a523d2cc79
commit
0f4e0be1a6
7 files changed
+468
-33
No files matched your search
@@ -3,6 +3,7 @@ import { writeFileSync, mkdtempSync, readFileSync } from "node:fs";
|
||||
import { join } from "node:path";
|
||||
import { tmpdir } from "node:os";
|
||||
|
||||
import { Prisma } from "@/generated/prisma/client";
|
||||
import {
|
||||
hashSampleKey,
|
||||
parseRecordValue,
|
||||
@@ -96,6 +97,27 @@ function makeFakePrisma() {
|
||||
type Row = Record<string, unknown>;
|
||||
const measurements: Row[] = [];
|
||||
const workouts: Row[] = [];
|
||||
let nextId = 1;
|
||||
|
||||
// The real `measurements` table carries a SECOND full unique index — the
|
||||
// natural key `(userId, type, measuredAt, source, sleepStage)` (migration
|
||||
// 0055, NULLS NOT DISTINCT) — on top of the `(userId, type, source,
|
||||
// externalId)` one the upsert keys on. Model BOTH so a create that would
|
||||
// collide on the natural key raises the exact P2002 the production DB does,
|
||||
// which is what the natural-key rescue must catch. `measuredAt` compares by
|
||||
// epoch; NULL `sleepStage` collides with NULL (NULLS NOT DISTINCT).
|
||||
const sameMeasuredAt = (a: unknown, b: unknown): boolean =>
|
||||
a instanceof Date && b instanceof Date
|
||||
? a.getTime() === b.getTime()
|
||||
: a === b;
|
||||
const sameStage = (a: unknown, b: unknown): boolean =>
|
||||
(a ?? null) === (b ?? null);
|
||||
const naturalKeyMatch = (m: Row, r: Row): boolean =>
|
||||
m.userId === r.userId &&
|
||||
m.type === r.type &&
|
||||
m.source === r.source &&
|
||||
sameMeasuredAt(m.measuredAt, r.measuredAt) &&
|
||||
sameStage(m.sleepStage, r.sleepStage);
|
||||
|
||||
return {
|
||||
_measurements: measurements,
|
||||
@@ -137,6 +159,24 @@ function makeFakePrisma() {
|
||||
) ?? null
|
||||
);
|
||||
},
|
||||
// Natural-key probe used by the rescue path. Tombstone-inclusive: no
|
||||
// `deletedAt` filter is applied here or by the caller.
|
||||
findFirst: async ({ where }: { where: Record<string, unknown> }) => {
|
||||
const match = measurements.find((m) => naturalKeyMatch(m, where));
|
||||
return match ? { id: match.id } : null;
|
||||
},
|
||||
update: async ({
|
||||
where,
|
||||
data,
|
||||
}: {
|
||||
where: { id: unknown };
|
||||
data: Row;
|
||||
}) => {
|
||||
const idx = measurements.findIndex((m) => m.id === where.id);
|
||||
if (idx < 0) throw new Error(`no row with id ${String(where.id)}`);
|
||||
measurements[idx] = { ...measurements[idx], ...data };
|
||||
return measurements[idx];
|
||||
},
|
||||
upsert: async ({
|
||||
where,
|
||||
create,
|
||||
@@ -158,8 +198,17 @@ function makeFakePrisma() {
|
||||
measurements[idx] = { ...measurements[idx], ...update };
|
||||
return measurements[idx];
|
||||
}
|
||||
measurements.push(create);
|
||||
return create;
|
||||
// The externalId leg missed → this is a create. Enforce the natural-key
|
||||
// unique before inserting, exactly as Postgres would.
|
||||
if (measurements.some((m) => naturalKeyMatch(m, create))) {
|
||||
throw new Prisma.PrismaClientKnownRequestError(
|
||||
"Unique constraint failed on the fields: (`user_id`,`type`,`measured_at`,`source`,`sleep_stage`)",
|
||||
{ code: "P2002", clientVersion: "test" },
|
||||
);
|
||||
}
|
||||
const row = { id: `m${nextId++}`, ...create };
|
||||
measurements.push(row);
|
||||
return row;
|
||||
},
|
||||
},
|
||||
workout: {
|
||||
@@ -398,6 +447,134 @@ describe("streamParseExportXml — end-to-end", () => {
|
||||
});
|
||||
});
|
||||
|
||||
/**
|
||||
* Issue #486 — two Apple samples that share the `measurements` natural key
|
||||
* `(userId, type, measuredAt, source, sleepStage)` but carry DIFFERENT
|
||||
* externalIds. The externalId upsert misses on the second → its create leg
|
||||
* collides on the natural key → P2002. Before the natural-key rescue this
|
||||
* aborted the whole import after minutes of work; after it, the run completes
|
||||
* with the colliding row adopted in place.
|
||||
*/
|
||||
describe("streamParseExportXml — natural-key collision (issue #486)", () => {
|
||||
// Both records share `endDate` (→ same `measuredAt`, the natural key) but
|
||||
// differ in `startDate`/`value` (→ different `hashSampleKey` externalId).
|
||||
function collidingExportXml(): string {
|
||||
return `<?xml version="1.0" encoding="UTF-8"?>
|
||||
<!DOCTYPE HealthData [<!ELEMENT HealthData (Record)*>]>
|
||||
<HealthData locale="en_US">
|
||||
<Record type="HKQuantityTypeIdentifierBodyMass"
|
||||
unit="kg"
|
||||
startDate="2026-05-14 08:10:00 +0200"
|
||||
endDate="2026-05-14 08:14:00 +0200"
|
||||
value="78.4"
|
||||
sourceName="Source App A"/>
|
||||
<Record type="HKQuantityTypeIdentifierBodyMass"
|
||||
unit="kg"
|
||||
startDate="2026-05-14 08:12:00 +0200"
|
||||
endDate="2026-05-14 08:14:00 +0200"
|
||||
value="78.6"
|
||||
sourceName="Source App B"/>
|
||||
</HealthData>`;
|
||||
}
|
||||
|
||||
it("completes and adopts the colliding row instead of throwing P2002", async () => {
|
||||
const tmp = mkdtempSync(join(tmpdir(), "healthlog-parser-test-"));
|
||||
const xmlPath = join(tmp, "export.xml");
|
||||
writeFileSync(xmlPath, collidingExportXml());
|
||||
const prisma = makeFakePrisma();
|
||||
|
||||
// On pre-fix code this rejects with the P2002 the fake raises when the
|
||||
// second create hits the natural key; on fixed code it resolves.
|
||||
const result = await streamParseExportXml({
|
||||
xmlPath,
|
||||
userId: "user-1",
|
||||
userTimezone: "Europe/Berlin",
|
||||
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
||||
prisma: prisma as any,
|
||||
});
|
||||
|
||||
// Both samples were read; one row physically landed, the twin adopted it.
|
||||
expect(result.perType.WEIGHT?.read).toBe(2);
|
||||
expect(result.perType.WEIGHT?.inserted).toBe(1);
|
||||
expect(result.perType.WEIGHT?.updated).toBe(1);
|
||||
|
||||
const weightRows = prisma._measurements.filter((m) => m.type === "WEIGHT");
|
||||
expect(weightRows).toHaveLength(1);
|
||||
// Last write wins: value + externalId re-keyed onto the second sample.
|
||||
expect(weightRows[0]?.value).toBe(78.6);
|
||||
const secondExternalId = hashSampleKey(
|
||||
"HKQuantityTypeIdentifierBodyMass",
|
||||
"78.6",
|
||||
"2026-05-14 08:12:00 +0200",
|
||||
"2026-05-14 08:14:00 +0200",
|
||||
);
|
||||
expect(weightRows[0]?.externalId).toBe(secondExternalId);
|
||||
|
||||
// The collision was rescued, not skipped.
|
||||
expect(result.unknown["WEIGHT::natural_key_unresolved"]).toBeUndefined();
|
||||
expect(result.unknown["WEIGHT::natural_key_rescue_failed"]).toBeUndefined();
|
||||
});
|
||||
|
||||
it("re-import of overlapping data with a re-keyed externalId adopts the prior row", async () => {
|
||||
const tmp = mkdtempSync(join(tmpdir(), "healthlog-parser-test-"));
|
||||
const prisma = makeFakePrisma();
|
||||
|
||||
// First import: single reading, externalId derived from its start/end.
|
||||
const firstPath = join(tmp, "export-1.xml");
|
||||
writeFileSync(
|
||||
firstPath,
|
||||
`<?xml version="1.0" encoding="UTF-8"?>
|
||||
<!DOCTYPE HealthData [<!ELEMENT HealthData (Record)*>]>
|
||||
<HealthData locale="en_US">
|
||||
<Record type="HKQuantityTypeIdentifierBodyMass"
|
||||
unit="kg"
|
||||
startDate="2026-05-14 08:10:00 +0200"
|
||||
endDate="2026-05-14 08:14:00 +0200"
|
||||
value="78.4"/>
|
||||
</HealthData>`,
|
||||
);
|
||||
const first = await streamParseExportXml({
|
||||
xmlPath: firstPath,
|
||||
userId: "user-1",
|
||||
userTimezone: "Europe/Berlin",
|
||||
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
||||
prisma: prisma as any,
|
||||
});
|
||||
expect(first.perType.WEIGHT?.inserted).toBe(1);
|
||||
expect(prisma._measurements).toHaveLength(1);
|
||||
|
||||
// Re-export: SAME instant (endDate), but the sample now carries a
|
||||
// different start window → a re-keyed externalId. The externalId upsert
|
||||
// misses; the natural key is still held by the first row.
|
||||
const secondPath = join(tmp, "export-2.xml");
|
||||
writeFileSync(
|
||||
secondPath,
|
||||
`<?xml version="1.0" encoding="UTF-8"?>
|
||||
<!DOCTYPE HealthData [<!ELEMENT HealthData (Record)*>]>
|
||||
<HealthData locale="en_US">
|
||||
<Record type="HKQuantityTypeIdentifierBodyMass"
|
||||
unit="kg"
|
||||
startDate="2026-05-14 08:11:30 +0200"
|
||||
endDate="2026-05-14 08:14:00 +0200"
|
||||
value="79.0"/>
|
||||
</HealthData>`,
|
||||
);
|
||||
const second = await streamParseExportXml({
|
||||
xmlPath: secondPath,
|
||||
userId: "user-1",
|
||||
userTimezone: "Europe/Berlin",
|
||||
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
||||
prisma: prisma as any,
|
||||
});
|
||||
|
||||
// No duplicate row; the prior row was adopted, counted as an update.
|
||||
expect(second.perType.WEIGHT?.inserted).toBe(0);
|
||||
expect(second.perType.WEIGHT?.updated).toBe(1);
|
||||
expect(prisma._measurements).toHaveLength(1);
|
||||
expect(prisma._measurements[0]?.value).toBe(79.0);
|
||||
});
|
||||
});
|
||||
|
||||
/**
|
||||
* Streaming-memory ceiling: synthesise a ~5 MB export and assert peak
|
||||
* heap stays bounded. Smaller than the §11 §5 fixture goal (50 MB) to
|
||||
|
||||
@@ -374,39 +374,112 @@ export async function streamParseExportXml(
|
||||
const stat = bumpStat(row.type);
|
||||
const rowStart = Date.now();
|
||||
const exists = existingKey.has(`${row.type}::${row.externalId}`);
|
||||
await prisma.measurement.upsert({
|
||||
where: {
|
||||
userId_type_source_externalId: {
|
||||
try {
|
||||
await prisma.measurement.upsert({
|
||||
where: {
|
||||
userId_type_source_externalId: {
|
||||
userId,
|
||||
type: row.type,
|
||||
source: "APPLE_HEALTH",
|
||||
externalId: row.externalId,
|
||||
},
|
||||
},
|
||||
create: {
|
||||
userId,
|
||||
type: row.type,
|
||||
value: row.value,
|
||||
unit: row.unit,
|
||||
source: "APPLE_HEALTH",
|
||||
measuredAt: row.measuredAt,
|
||||
externalId: row.externalId,
|
||||
externalSourceVersion: row.externalSourceVersion,
|
||||
sleepStage: row.sleepStage ?? null,
|
||||
deviceType: row.deviceType,
|
||||
},
|
||||
},
|
||||
create: {
|
||||
userId,
|
||||
type: row.type,
|
||||
value: row.value,
|
||||
unit: row.unit,
|
||||
source: "APPLE_HEALTH",
|
||||
measuredAt: row.measuredAt,
|
||||
externalId: row.externalId,
|
||||
externalSourceVersion: row.externalSourceVersion,
|
||||
sleepStage: row.sleepStage ?? null,
|
||||
deviceType: row.deviceType,
|
||||
},
|
||||
update: {
|
||||
value: row.value,
|
||||
measuredAt: row.measuredAt,
|
||||
externalSourceVersion: row.externalSourceVersion,
|
||||
sleepStage: row.sleepStage ?? null,
|
||||
deviceType: row.deviceType,
|
||||
},
|
||||
});
|
||||
stat.durationMs += Date.now() - rowStart;
|
||||
if (exists) stat.updated += 1;
|
||||
else stat.inserted += 1;
|
||||
rowsUpserted += 1;
|
||||
update: {
|
||||
value: row.value,
|
||||
measuredAt: row.measuredAt,
|
||||
externalSourceVersion: row.externalSourceVersion,
|
||||
sleepStage: row.sleepStage ?? null,
|
||||
deviceType: row.deviceType,
|
||||
},
|
||||
});
|
||||
stat.durationMs += Date.now() - rowStart;
|
||||
if (exists) stat.updated += 1;
|
||||
else stat.inserted += 1;
|
||||
rowsUpserted += 1;
|
||||
} catch (err) {
|
||||
// Natural-key rescue. `Measurement` carries a SECOND full unique
|
||||
// index beyond the externalId one this upsert keys on — the natural
|
||||
// key `(userId, type, measuredAt, source, sleepStage)` (migration
|
||||
// 0055, NULLS NOT DISTINCT). When two Apple samples share that natural
|
||||
// key but carry DIFFERENT externalIds — two source apps writing at the
|
||||
// same instant, or hash-derived ids on a re-import of overlapping data
|
||||
// — the externalId leg above MISSES (the incoming externalId isn't in
|
||||
// the table) and the create leg then collides on the natural key →
|
||||
// P2002. A blind throw aborts the whole import mid-run (issue #486).
|
||||
// Instead, mirror the sync providers' natural-key rescue
|
||||
// (`withings/sync-sleep.ts`, `google-health/sync.ts`, `fitbit/sync.ts`):
|
||||
// probe the occupied row by its natural key — tombstone-inclusive, no
|
||||
// `deletedAt` filter, since a soft-deleted row still holds the key and
|
||||
// would otherwise wedge the night forever — and ADOPT it: re-key onto
|
||||
// the incoming externalId, take the new value/unit, resurrect if it was
|
||||
// tombstoned. A single sample must NEVER abort the run: this catch sits
|
||||
// OUTSIDE any transaction (each upsert is its own statement), so the
|
||||
// caught P2002 can't poison a surrounding tx.
|
||||
if (
|
||||
err instanceof Prisma.PrismaClientKnownRequestError &&
|
||||
err.code === "P2002"
|
||||
) {
|
||||
try {
|
||||
const twin = await prisma.measurement.findFirst({
|
||||
where: {
|
||||
userId,
|
||||
type: row.type,
|
||||
source: "APPLE_HEALTH",
|
||||
measuredAt: row.measuredAt,
|
||||
sleepStage: row.sleepStage ?? null,
|
||||
},
|
||||
select: { id: true },
|
||||
});
|
||||
if (twin) {
|
||||
await prisma.measurement.update({
|
||||
where: { id: twin.id },
|
||||
// `measuredAt` + `sleepStage` already match by construction
|
||||
// (they ARE the natural key we probed on), so only value/unit,
|
||||
// the externalId re-key, provenance, and the resurrect move.
|
||||
data: {
|
||||
value: row.value,
|
||||
unit: row.unit,
|
||||
externalId: row.externalId,
|
||||
externalSourceVersion: row.externalSourceVersion,
|
||||
deviceType: row.deviceType,
|
||||
deletedAt: null,
|
||||
},
|
||||
});
|
||||
// An adopted natural-key twin is an in-place UPDATE, not a fresh
|
||||
// row — keep the inserted/updated split honest.
|
||||
stat.updated += 1;
|
||||
rowsUpserted += 1;
|
||||
} else {
|
||||
// P2002 with no findable natural-key twin (a race, or a collision
|
||||
// we can't reconcile here). Skip this one sample and keep going —
|
||||
// surfaced under `unknown` so operators can see the skip.
|
||||
unknown[`${row.type}::natural_key_unresolved`] =
|
||||
(unknown[`${row.type}::natural_key_unresolved`] ?? 0) + 1;
|
||||
}
|
||||
} catch {
|
||||
unknown[`${row.type}::natural_key_rescue_failed`] =
|
||||
(unknown[`${row.type}::natural_key_rescue_failed`] ?? 0) + 1;
|
||||
}
|
||||
} else {
|
||||
// Any other single-row write error: skip the sample, never abort the
|
||||
// import over one bad row.
|
||||
unknown[`${row.type}::upsert_failed`] =
|
||||
(unknown[`${row.type}::upsert_failed`] ?? 0) + 1;
|
||||
}
|
||||
stat.durationMs += Date.now() - rowStart;
|
||||
}
|
||||
}
|
||||
};
|
||||
|
||||
|
||||
Reference in new issue
Block a user