fix: close MaintenanceRecord race condition + restrict targetStatus
createMaintenanceRecord()'s AssetItem status guard pinned status:{not:"retired"}
instead of status:"in_stock", so two concurrent opens (or an open racing a
concurrent createAssignment()) could both pass the guard, producing two open
MaintenanceRecords or an orphaned active Assignment. Guard now pins "in_stock"
exactly, mirroring createAssignment()'s own atomic guard.
closeMaintenanceRecord()'s targetStatus accepted any of the 4 AssetItemStatus
values, including "assigned" — which sets AssetItem.status without creating a
corresponding Assignment row, desyncing Assignment (source of truth for who
holds what) from AssetItem.status with no way back. Restricted to
in_stock|retired, the only two outcomes a closed repair can actually resolve
to.
This commit is contained in:
@@ -108,9 +108,11 @@ export function validateCreateMaintenanceRecordInput(input: {
|
||||
return { ok: true, value: result.data };
|
||||
}
|
||||
|
||||
// Closed set of the 4 AssetItemStatus enum values — reused verbatim (not
|
||||
// re-derived) as the z.enum() literal list backing targetStatus below.
|
||||
const assetItemStatusValues = ["in_stock", "assigned", "repair", "retired"] as const;
|
||||
// Closed set of the 2 AssetItemStatus values a maintenance-record close is
|
||||
// allowed to land on — deliberately NOT the full 4-value AssetItemStatus
|
||||
// enum (see closeMaintenanceRecordSchema's targetStatus comment below for
|
||||
// why "assigned"/"repair" are excluded).
|
||||
const closeableTargetStatusValues = ["in_stock", "retired"] as const;
|
||||
|
||||
const closeMaintenanceRecordSchema = z.object({
|
||||
endDate: dateString("endDate is required and must be a valid date string"),
|
||||
@@ -125,13 +127,26 @@ const closeMaintenanceRecordSchema = z.object({
|
||||
// Required — deliberately NOT defaulted to "in_stock", per docs/
|
||||
// asset-license-management.md's "MaintenanceRecord" section: "không tự
|
||||
// động mặc định về in_stock, vì sửa không phải lúc nào cũng thành công."
|
||||
// Any of the 4 AssetItemStatus values is accepted at the Zod layer (the
|
||||
// doc's examples are in_stock/retired, but nothing in the spec restricts
|
||||
// the set further) — see feature-implementer report's "Assumed" section.
|
||||
targetStatus: z.enum(assetItemStatusValues, {
|
||||
// Restricted to "in_stock" | "retired" only (NOT the full 4-value
|
||||
// AssetItemStatus enum) — those are the only two outcomes a repair can
|
||||
// actually resolve to (fixed → back to stock, unfixable → write off).
|
||||
// "assigned" is deliberately rejected: setting AssetItem.status to
|
||||
// "assigned" here would NOT create a corresponding Assignment row, so
|
||||
// Assignment (the source of truth for "who currently holds what") and
|
||||
// AssetItem.status would go out of sync with no way back — createAssignment()
|
||||
// only accepts a "in_stock" AssetItem, and there is no standalone
|
||||
// AssetItem-status-edit endpoint to undo it by hand. "repair" is
|
||||
// rejected too: it would leave the record's own AssetItem "in repair"
|
||||
// despite this exact call being the one that closes out its maintenance
|
||||
// record — a closed record whose item is still marked "repair" is a
|
||||
// contradiction, and it would also make the item wrongly ineligible for
|
||||
// a new maintenance record's own "no other open record" precondition to
|
||||
// ever apply cleanly (a new one could still be opened as a *different*
|
||||
// record row while the old one sits closed but the item is still
|
||||
// "repair").
|
||||
targetStatus: z.enum(closeableTargetStatusValues, {
|
||||
errorMap: () => ({
|
||||
message:
|
||||
"targetStatus is required and must be one of 'in_stock', 'assigned', 'repair', 'retired'",
|
||||
message: "targetStatus is required and must be one of 'in_stock', 'retired'",
|
||||
}),
|
||||
}),
|
||||
});
|
||||
@@ -234,8 +249,12 @@ export type CreateMaintenanceRecordResult =
|
||||
*
|
||||
* Race-condition note: mirrors src/lib/assignments.ts's createAssignment()
|
||||
* pattern — the AssetItem.status flip uses a WHERE-guarded `updateMany`
|
||||
* (`where: { id: assetItemId, status: { not: "retired" } }`) rather than a
|
||||
* plain read-then-write. That `updateMany`'s `count` is the actual
|
||||
* (`where: { id: assetItemId, status: "in_stock" }`) rather than a
|
||||
* plain read-then-write. Pinning the exact required prior value (not
|
||||
* excluding "retired") matters: an excluding guard would still match an
|
||||
* item another concurrent transaction just flipped to "assigned" or
|
||||
* "repair", letting two concurrent opens (or an open racing an Assignment
|
||||
* create) both win. That `updateMany`'s `count` is the actual
|
||||
* race-closing gate: two concurrent opens racing on the same AssetItem
|
||||
* serialize on Postgres's row lock for that UPDATE, the first to commit
|
||||
* wins, and the second's WHERE clause no longer matches (status is no
|
||||
@@ -281,14 +300,20 @@ export async function createMaintenanceRecord(
|
||||
);
|
||||
}
|
||||
|
||||
// Atomic race-closing gate — see this function's doc comment.
|
||||
// Atomic race-closing gate — see this function's doc comment. Pins the
|
||||
// exact prior value ("in_stock") rather than excluding a value
|
||||
// ("not retired"), mirroring createAssignment()'s own status guard
|
||||
// (src/lib/assignments.ts) — the excluding form would still match an
|
||||
// AssetItem another concurrent transaction just flipped to "assigned"
|
||||
// or "repair" (neither equals "retired"), letting two opens (or an
|
||||
// open racing an Assignment create) both win.
|
||||
const statusUpdate = await tx.assetItem.updateMany({
|
||||
where: { id: input.assetItemId, status: { not: "retired" } },
|
||||
where: { id: input.assetItemId, status: "in_stock" },
|
||||
data: { status: "repair" },
|
||||
});
|
||||
if (statusUpdate.count === 0) {
|
||||
throw new MaintenanceRecordBusinessError(
|
||||
"assetItemId refers to an AssetItem that is retired (lost a race to a concurrent retire)"
|
||||
"assetItemId refers to an AssetItem that is not in_stock (lost a race to a concurrent status change)"
|
||||
);
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user