mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-08 12:32:43 +00:00
fix(vaccinations): answer 409 to a concurrent duplicate vaccine name
Two adds of the same custom vaccine name both passed the name lookup, and the loser failed on the unique index with a 500. A unique violation on the add or the rename now answers the same 409 the lookup gives.
This commit is contained in:
1 parent
7c6510eba9
commit
f71becef29
3 files changed
+111
-61
No files matched your search
@@ -18,6 +18,7 @@
|
||||
import { NextRequest } from "next/server";
|
||||
|
||||
import { prisma } from "@/lib/db";
|
||||
import { isP2002 } from "@/lib/prisma-errors";
|
||||
import { apiHandler, requireRecordAuth } from "@/lib/api-handler";
|
||||
import { annotate } from "@/lib/logging/context";
|
||||
import { auditLog } from "@/lib/auth/audit";
|
||||
@@ -69,42 +70,51 @@ export const PATCH = apiHandler(
|
||||
}
|
||||
const entry = parsed.data;
|
||||
|
||||
const outcome = await prisma.$transaction(async (tx) => {
|
||||
if (
|
||||
entry.name !== undefined &&
|
||||
entry.name !== existing.name &&
|
||||
(await findLiveNameClash(tx, user.id, entry.name, id))
|
||||
) {
|
||||
return "name-taken" as const;
|
||||
}
|
||||
// A removed definition may still hold the new name on the unique
|
||||
// index. It is gone from the person's view, so it gives the name up
|
||||
// rather than blocking the rename.
|
||||
if (entry.name !== undefined && entry.name !== existing.name) {
|
||||
await tx.customVaccine.deleteMany({
|
||||
where: {
|
||||
userId: user.id,
|
||||
name: entry.name,
|
||||
deletedAt: { not: null },
|
||||
},
|
||||
// The lookup below and the unique index answer the same question; two
|
||||
// requests racing with the same name both pass the lookup, and the
|
||||
// loser meets the index. That is the same answer, so it gets the same
|
||||
// 409 rather than a 500.
|
||||
const outcome = await prisma
|
||||
.$transaction(async (tx) => {
|
||||
if (
|
||||
entry.name !== undefined &&
|
||||
entry.name !== existing.name &&
|
||||
(await findLiveNameClash(tx, user.id, entry.name, id))
|
||||
) {
|
||||
return "name-taken" as const;
|
||||
}
|
||||
// A removed definition may still hold the new name on the unique
|
||||
// index. It is gone from the person's view, so it gives the name up
|
||||
// rather than blocking the rename.
|
||||
if (entry.name !== undefined && entry.name !== existing.name) {
|
||||
await tx.customVaccine.deleteMany({
|
||||
where: {
|
||||
userId: user.id,
|
||||
name: entry.name,
|
||||
deletedAt: { not: null },
|
||||
},
|
||||
});
|
||||
}
|
||||
// Field-by-field — never spread the parsed object whole.
|
||||
const data: Prisma.CustomVaccineUpdateInput = {};
|
||||
if (entry.name !== undefined) data.name = entry.name;
|
||||
if (entry.components !== undefined) data.components = entry.components;
|
||||
if (entry.typicalSeriesDoses !== undefined) {
|
||||
data.typicalSeriesDoses = entry.typicalSeriesDoses;
|
||||
}
|
||||
if (entry.boosterIntervalMonths !== undefined) {
|
||||
data.boosterIntervalMonths = entry.boosterIntervalMonths;
|
||||
}
|
||||
return tx.customVaccine.update({
|
||||
where: { id },
|
||||
data,
|
||||
select: CUSTOM_VACCINE_SELECT,
|
||||
});
|
||||
}
|
||||
// Field-by-field — never spread the parsed object whole.
|
||||
const data: Prisma.CustomVaccineUpdateInput = {};
|
||||
if (entry.name !== undefined) data.name = entry.name;
|
||||
if (entry.components !== undefined) data.components = entry.components;
|
||||
if (entry.typicalSeriesDoses !== undefined) {
|
||||
data.typicalSeriesDoses = entry.typicalSeriesDoses;
|
||||
}
|
||||
if (entry.boosterIntervalMonths !== undefined) {
|
||||
data.boosterIntervalMonths = entry.boosterIntervalMonths;
|
||||
}
|
||||
return tx.customVaccine.update({
|
||||
where: { id },
|
||||
data,
|
||||
select: CUSTOM_VACCINE_SELECT,
|
||||
})
|
||||
.catch((err: unknown) => {
|
||||
if (isP2002(err)) return "name-taken" as const;
|
||||
throw err;
|
||||
});
|
||||
});
|
||||
|
||||
if (outcome === "name-taken") {
|
||||
return apiError("A vaccine with this name already exists", 409, {
|
||||
|
||||
@@ -20,6 +20,7 @@
|
||||
import { NextRequest } from "next/server";
|
||||
|
||||
import { prisma } from "@/lib/db";
|
||||
import { isP2002 } from "@/lib/prisma-errors";
|
||||
import { apiHandler, requireRecordAuth } from "@/lib/api-handler";
|
||||
import { annotate } from "@/lib/logging/context";
|
||||
import { auditLog } from "@/lib/auth/audit";
|
||||
@@ -80,36 +81,45 @@ export const POST = apiHandler(async (request: NextRequest) => {
|
||||
}
|
||||
const entry = parsed.data;
|
||||
|
||||
const outcome = await prisma.$transaction(async (tx) => {
|
||||
if (await findLiveNameClash(tx, user.id, entry.name)) {
|
||||
return "name-taken" as const;
|
||||
}
|
||||
// Names are unique per record, removed definitions included, because the
|
||||
// index is. A name the person removed and now adds again brings that row
|
||||
// back with the new values rather than failing on the index: deleting
|
||||
// unlinked every dose, so nothing re-attaches on the way back.
|
||||
const removed = await tx.customVaccine.findFirst({
|
||||
where: { userId: user.id, name: entry.name, deletedAt: { not: null } },
|
||||
select: { id: true },
|
||||
});
|
||||
const data = {
|
||||
name: entry.name,
|
||||
components: entry.components,
|
||||
typicalSeriesDoses: entry.typicalSeriesDoses ?? null,
|
||||
boosterIntervalMonths: entry.boosterIntervalMonths ?? null,
|
||||
};
|
||||
if (removed) {
|
||||
return tx.customVaccine.update({
|
||||
where: { id: removed.id },
|
||||
data: { ...data, deletedAt: null },
|
||||
// The lookup below and the unique index answer the same question; two
|
||||
// requests racing with the same name both pass the lookup, and the
|
||||
// loser meets the index. That is the same answer, so it gets the same
|
||||
// 409 rather than a 500.
|
||||
const outcome = await prisma
|
||||
.$transaction(async (tx) => {
|
||||
if (await findLiveNameClash(tx, user.id, entry.name)) {
|
||||
return "name-taken" as const;
|
||||
}
|
||||
// Names are unique per record, removed definitions included, because the
|
||||
// index is. A name the person removed and now adds again brings that row
|
||||
// back with the new values rather than failing on the index: deleting
|
||||
// unlinked every dose, so nothing re-attaches on the way back.
|
||||
const removed = await tx.customVaccine.findFirst({
|
||||
where: { userId: user.id, name: entry.name, deletedAt: { not: null } },
|
||||
select: { id: true },
|
||||
});
|
||||
const data = {
|
||||
name: entry.name,
|
||||
components: entry.components,
|
||||
typicalSeriesDoses: entry.typicalSeriesDoses ?? null,
|
||||
boosterIntervalMonths: entry.boosterIntervalMonths ?? null,
|
||||
};
|
||||
if (removed) {
|
||||
return tx.customVaccine.update({
|
||||
where: { id: removed.id },
|
||||
data: { ...data, deletedAt: null },
|
||||
select: CUSTOM_VACCINE_SELECT,
|
||||
});
|
||||
}
|
||||
return tx.customVaccine.create({
|
||||
data: { userId: user.id, ...data },
|
||||
select: CUSTOM_VACCINE_SELECT,
|
||||
});
|
||||
}
|
||||
return tx.customVaccine.create({
|
||||
data: { userId: user.id, ...data },
|
||||
select: CUSTOM_VACCINE_SELECT,
|
||||
})
|
||||
.catch((err: unknown) => {
|
||||
if (isP2002(err)) return "name-taken" as const;
|
||||
throw err;
|
||||
});
|
||||
});
|
||||
|
||||
if (outcome === "name-taken") {
|
||||
return apiError("A vaccine with this name already exists", 409, {
|
||||
|
||||
@@ -176,6 +176,36 @@ describe("an owner's own vaccine definitions", () => {
|
||||
expect(bad.status).toBe(422);
|
||||
});
|
||||
|
||||
it("answers 409, not 500, to the loser of two concurrent adds of one name", async () => {
|
||||
const owner = await person("owner");
|
||||
signIn(owner);
|
||||
const prisma = getPrismaClient();
|
||||
// Hold every insert long enough that both requests have passed the name
|
||||
// lookup before either commits, so the loser meets the unique index.
|
||||
await prisma.$executeRawUnsafe(`
|
||||
CREATE OR REPLACE FUNCTION slow_custom_vaccine_insert() RETURNS trigger AS $$
|
||||
BEGIN PERFORM pg_sleep(0.3); RETURN NEW; END; $$ LANGUAGE plpgsql`);
|
||||
await prisma.$executeRawUnsafe(`
|
||||
CREATE TRIGGER slow_custom_vaccine_insert BEFORE INSERT ON "custom_vaccines"
|
||||
FOR EACH ROW EXECUTE FUNCTION slow_custom_vaccine_insert()`);
|
||||
try {
|
||||
const statuses = (
|
||||
await Promise.all([addCustom(TRAVEL), addCustom(TRAVEL)])
|
||||
).map((r) => r.status);
|
||||
expect(statuses.sort()).toEqual([201, 409]);
|
||||
expect(
|
||||
await prisma.customVaccine.count({ where: { userId: owner.id } }),
|
||||
).toBe(1);
|
||||
} finally {
|
||||
await prisma.$executeRawUnsafe(
|
||||
'DROP TRIGGER IF EXISTS slow_custom_vaccine_insert ON "custom_vaccines"',
|
||||
);
|
||||
await prisma.$executeRawUnsafe(
|
||||
"DROP FUNCTION IF EXISTS slow_custom_vaccine_insert()",
|
||||
);
|
||||
}
|
||||
});
|
||||
|
||||
it("counts a dose into every listed antigen and clears a booster keyed to one", async () => {
|
||||
const owner = await person("owner");
|
||||
signIn(owner);
|
||||
|
||||
Reference in new issue
Block a user