fix(economy): make catalog translation atomic

src/features/housekeeping/domains/economy/services/mutation-runtime-catalog-translate.test.ts

@@ -0,0 +1,323 @@
+import { beforeEach, describe, expect, it, vi } from "vitest";
+import type { HousekeepingCapabilityContext } from "../../../foundation/contracts";
+
+const doubles = vi.hoisted(() => ({
+	events: [] as string[],
+	globalSelect: vi.fn(),
+	globalUpdate: vi.fn(),
+	invalidateFurniDataCache: vi.fn(),
+	patchFurniEntryNames: vi.fn(),
+	rconUpdateCatalog: vi.fn(),
+	transaction: vi.fn(),
+	transactionSelect: vi.fn(),
+	transactionUpdate: vi.fn(),
+}));
+
+const transactionToken = {
+	select: doubles.transactionSelect,
+	update: doubles.transactionUpdate,
+};
+
+vi.mock("@/lib/db", async (importOriginal) => {
+	const actual = await importOriginal<typeof import("@/lib/db")>();
+	return {
+		...actual,
+		db: {
+			...actual.db,
+			select: doubles.globalSelect,
+			transaction: doubles.transaction,
+			update: doubles.globalUpdate,
+		},
+	};
+});
+
+vi.mock("@/lib/services/catalog-items-loader", () => ({
+	invalidateFurniDataCache: doubles.invalidateFurniDataCache,
+}));
+
+vi.mock("@/lib/services/furni-data", () => ({
+	patchFurniEntryNames: doubles.patchFurniEntryNames,
+}));
+
+vi.mock("@/lib/services/rcon", () => ({
+	rcon: { updateCatalog: doubles.rconUpdateCatalog },
+}));
+
+import { executeEconomyCatalogMutation } from "./mutation-runtime-catalog";
+import { EconomyCommittedExternalFailure } from "./mutations-production";
+
+const capability: HousekeepingCapabilityContext = {
+	actor: { id: 42, username: "operator", rank: 7 },
+	isSuperAdmin: false,
+	has: () => true,
+	hasAny: () => true,
+	hasAll: () => true,
+};
+
+const context = {
+	capability,
+	correlationId: "catalog-translation",
+	legacy: false,
+};
+
+const baseOne = {
+	id: 1,
+	publicName: "Old chair",
+	itemName: "chair_class",
+	type: "s",
+	spriteId: 10,
+};
+const baseTwo = {
+	id: 2,
+	publicName: "Old table",
+	itemName: "table_class",
+	type: "s",
+	spriteId: 20,
+};
+
+function selection(rows: readonly unknown[], event: string) {
+	const resolve = () => {
+		doubles.events.push(event);
+		return Promise.resolve(rows);
+	};
+	const thenable = {
+		for: resolve,
+		limit: resolve,
+	};
+	return { from: () => ({ where: () => thenable }) };
+}
+
+function update(event: string) {
+	return {
+		set: () => ({
+			where: async () => {
+				doubles.events.push(event);
+			},
+		}),
+	};
+}
+
+beforeEach(() => {
+	for (const mock of Object.values(doubles)) {
+		if (Array.isArray(mock)) mock.length = 0;
+		else mock.mockReset();
+	}
+	doubles.transaction.mockImplementation(async (run) => {
+		doubles.events.push("transaction:begin");
+		const result = await run(transactionToken);
+		doubles.events.push("transaction:commit");
+		return result;
+	});
+	doubles.transactionUpdate.mockImplementation(() =>
+		update("transaction:update"),
+	);
+	doubles.globalUpdate.mockImplementation(() => update("global:update"));
+	doubles.patchFurniEntryNames.mockImplementation(async () => {
+		doubles.events.push("external:furni");
+		return { updated: 2, inserted: 0 };
+	});
+	doubles.invalidateFurniDataCache.mockImplementation(() => {
+		doubles.events.push("external:invalidate");
+	});
+	doubles.rconUpdateCatalog.mockImplementation(async () => {
+		doubles.events.push("external:rcon");
+		return true;
+	});
+});
+
+describe("catalog item translation atomicity", () => {
+	it("reports every missing or rejected target before issuing any update", async () => {
+		doubles.globalSelect
+			.mockImplementationOnce(() => selection([baseOne], "global:base-1"))
+			.mockImplementationOnce(() => selection([], "global:catalog-1"))
+			.mockImplementationOnce(() => selection([], "global:base-999"));
+		doubles.transactionSelect.mockImplementationOnce(() =>
+			selection([baseOne], "transaction:bases"),
+		);
+
+		await expect(
+			executeEconomyCatalogMutation(
+				"catalog-item.translate",
+				{
+					items: [
+						{ id: 1, publicName: "New chair" },
+						{ id: 999, publicName: "Missing" },
+						{ id: "bad", publicName: "Rejected" },
+					],
+				},
+				context,
+			),
+		).rejects.toMatchObject({
+			name: "EconomyMutationFailure",
+			code: "VALIDATION",
+			fieldErrors: {
+				missingIds: ["999"],
+				rejectedIds: ["bad"],
+			},
+		});
+		expect(doubles.transaction).toHaveBeenCalledOnce();
+		expect(doubles.transactionUpdate).not.toHaveBeenCalled();
+		expect(doubles.globalUpdate).not.toHaveBeenCalled();
+		expect(doubles.patchFurniEntryNames).not.toHaveBeenCalled();
+		expect(doubles.rconUpdateCatalog).not.toHaveBeenCalled();
+	});
+
+	it("preloads every target, commits exact before/after changes atomically, then patches external state", async () => {
+		doubles.globalSelect
+			.mockImplementationOnce(() => selection([baseOne], "global:base-1"))
+			.mockImplementationOnce(() =>
+				selection([{ id: 101, catalogName: "Old chair" }], "global:catalog-1"),
+			)
+			.mockImplementationOnce(() => selection([baseTwo], "global:base-2"))
+			.mockImplementationOnce(() =>
+				selection([{ id: 102, catalogName: "Old table" }], "global:catalog-2"),
+			);
+		doubles.transactionSelect
+			.mockImplementationOnce(() =>
+				selection([baseTwo, baseOne], "transaction:bases"),
+			)
+			.mockImplementationOnce(() =>
+				selection(
+					[{ id: 101, catalogName: "Old chair" }],
+					"transaction:catalog-1",
+				),
+			)
+			.mockImplementationOnce(() =>
+				selection(
+					[{ id: 102, catalogName: "Old table" }],
+					"transaction:catalog-2",
+				),
+			);
+
+		const result = await executeEconomyCatalogMutation(
+			"catalog-item.translate",
+			{
+				items: [
+					{ id: 2, publicName: "New table", description: "Table text" },
+					{ id: 1, publicName: "New chair", description: "Chair text" },
+				],
+			},
+			context,
+		);
+
+		expect(result).toMatchObject({
+			before: {
+				items: [
+					{
+						id: "1",
+						publicName: "Old chair",
+						catalogItems: [{ id: "101", catalogName: "Old chair" }],
+					},
+					{
+						id: "2",
+						publicName: "Old table",
+						catalogItems: [{ id: "102", catalogName: "Old table" }],
+					},
+				],
+			},
+			after: {
+				items: [
+					{
+						id: "1",
+						publicName: "New chair",
+						catalogItems: [{ id: "101", catalogName: "New chair" }],
+					},
+					{
+						id: "2",
+						publicName: "New table",
+						catalogItems: [{ id: "102", catalogName: "New table" }],
+					},
+				],
+			},
+			output: {
+				namesUpdated: 2,
+				descriptionsUpdated: 2,
+				updatedIds: ["1", "2"],
+				missingIds: [],
+				rejectedIds: [],
+				furniDataUpdated: 2,
+				furniDataInserted: 0,
+			},
+		});
+		expect(doubles.globalSelect).not.toHaveBeenCalled();
+		expect(doubles.globalUpdate).not.toHaveBeenCalled();
+		expect(doubles.transactionUpdate).toHaveBeenCalledTimes(4);
+		expect(doubles.events).toEqual([
+			"transaction:begin",
+			"transaction:bases",
+			"transaction:catalog-1",
+			"transaction:catalog-2",
+			"transaction:update",
+			"transaction:update",
+			"transaction:update",
+			"transaction:update",
+			"transaction:commit",
+			"external:furni",
+			"external:invalidate",
+			"external:rcon",
+		]);
+	});
+
+	it("rolls the batch back and skips external work when a later database update fails", async () => {
+		doubles.transactionSelect
+			.mockImplementationOnce(() =>
+				selection([baseOne, baseTwo], "transaction:bases"),
+			)
+			.mockImplementationOnce(() => selection([], "transaction:catalog-1"))
+			.mockImplementationOnce(() => selection([], "transaction:catalog-2"));
+		let updateCount = 0;
+		doubles.transactionUpdate.mockImplementation(() => ({
+			set: () => ({
+				where: async () => {
+					updateCount += 1;
+					doubles.events.push(`transaction:update-${updateCount}`);
+					if (updateCount === 2) throw new Error("database update failed");
+				},
+			}),
+		}));
+
+		await expect(
+			executeEconomyCatalogMutation(
+				"catalog-item.translate",
+				{
+					items: [
+						{ id: 1, publicName: "New chair" },
+						{ id: 2, publicName: "New table" },
+					],
+				},
+				context,
+			),
+		).rejects.toThrow("database update failed");
+		expect(doubles.transaction).toHaveBeenCalledOnce();
+		expect(doubles.events).not.toContain("transaction:commit");
+		expect(doubles.patchFurniEntryNames).not.toHaveBeenCalled();
+		expect(doubles.rconUpdateCatalog).not.toHaveBeenCalled();
+	});
+
+	it("uses the committed-external-failure envelope only after the database transaction commits", async () => {
+		doubles.globalSelect
+			.mockImplementationOnce(() => selection([baseOne], "global:base-1"))
+			.mockImplementationOnce(() => selection([], "global:catalog-1"));
+		doubles.transactionSelect
+			.mockImplementationOnce(() => selection([baseOne], "transaction:bases"))
+			.mockImplementationOnce(() => selection([], "transaction:catalog-1"));
+		doubles.patchFurniEntryNames.mockImplementationOnce(async () => {
+			doubles.events.push("external:furni-failed");
+			throw new Error("furni store unavailable");
+		});
+
+		await expect(
+			executeEconomyCatalogMutation(
+				"catalog-item.translate",
+				{ items: [{ id: 1, publicName: "New chair" }] },
+				context,
+			),
+		).rejects.toBeInstanceOf(EconomyCommittedExternalFailure);
+		expect(doubles.transaction).toHaveBeenCalledOnce();
+		expect(doubles.events).toContain("transaction:commit");
+		expect(doubles.events.indexOf("transaction:commit")).toBeLessThan(
+			doubles.events.indexOf("external:furni-failed"),
+		);
+		expect(doubles.rconUpdateCatalog).not.toHaveBeenCalled();
+	});
+});

src/features/housekeeping/domains/economy/services/mutation-runtime-catalog.ts

@@ -799,20 +799,49 @@ async function catalogItemTranslate(
 		data.items.length > 500
 	)
 		throw validation();
-	const furniPatches: Array<{
-		classname: string;
-		itemType: string;
-		name?: string;
-		description?: string;
-		spriteId?: number;
-		createIfMissing?: boolean;
-	}> = [];
-	let namesUpdated = 0;
-	let descriptionsUpdated = 0;
-	for (const candidate of data.items) {
-		const item = record(candidate);
-		const id = positiveInteger(item.id);
-		const [base] = await db
+
+	type TranslationRequest = {
+		readonly id: number;
+		readonly nextName: string;
+		readonly nextDescription: string;
+	};
+	const requests: TranslationRequest[] = [];
+	const rejectedIds: string[] = [];
+	const requestedIds = new Set<number>();
+	for (const [index, candidate] of data.items.entries()) {
+		const rejectedLabel =
+			typeof candidate === "object" &&
+			candidate !== null &&
+			!Array.isArray(candidate) &&
+			"id" in candidate
+				? String((candidate as Record<string, unknown>).id).slice(0, 64)
+				: `index:${index}`;
+		try {
+			const item = record(candidate);
+			const id = positiveInteger(item.id);
+			const nextName = text(item.publicName, 56);
+			const nextDescription = text(item.description, 500);
+			if ((!nextName && !nextDescription) || requestedIds.has(id)) {
+				rejectedIds.push(String(id));
+				continue;
+			}
+			requestedIds.add(id);
+			requests.push({ id, nextName, nextDescription });
+		} catch {
+			rejectedIds.push(rejectedLabel || `index:${index}`);
+		}
+	}
+	requests.sort((left, right) => left.id - right.id);
+	if (requests.length === 0) {
+		throw validation({
+			missingIds: [],
+			rejectedIds: [...new Set(rejectedIds)],
+		});
+	}
+
+	const committed = await db.transaction(async (transaction) => {
+		const connection = transaction as unknown as EconomyDatabase;
+		const bases = await connection
 			.select({
 				id: ItemsBase.id,
 				publicName: ItemsBase.publicName,
@@ -821,19 +850,36 @@ async function catalogItemTranslate(
 				spriteId: ItemsBase.spriteId,
 			})
 			.from(ItemsBase)
-			.where(eq(ItemsBase.id, id))
-			.limit(1);
-		if (!base) continue;
-		const nextName = text(item.publicName, 56);
-		const nextDescription = text(item.description, 500);
-		if (nextName && nextName !== base.publicName) {
-			await db
-				.update(ItemsBase)
-				.set({ publicName: nextName })
-				.where(eq(ItemsBase.id, id));
-			const idText = String(id);
-			const related = await db
-				.select({ id: CatalogItems.id })
+			.where(
+				inArray(
+					ItemsBase.id,
+					requests.map((request) => request.id),
+				),
+			)
+			.for("update");
+		const baseById = new Map(bases.map((base) => [base.id, base]));
+		const missingIds = requests
+			.filter((request) => !baseById.has(request.id))
+			.map((request) => String(request.id));
+		for (const request of requests) {
+			const base = baseById.get(request.id);
+			if (base && !base.itemName) rejectedIds.push(String(request.id));
+		}
+		if (missingIds.length > 0 || rejectedIds.length > 0) {
+			throw validation({
+				missingIds,
+				rejectedIds: [...new Set(rejectedIds)],
+			});
+		}
+
+		const relatedByBaseId = new Map<
+			number,
+			Array<{ id: number; catalogName: string }>
+		>();
+		for (const request of requests) {
+			const idText = String(request.id);
+			const related = await connection
+				.select({ id: CatalogItems.id, catalogName: CatalogItems.catalogName })
 				.from(CatalogItems)
 				.where(
 					or(
@@ -842,57 +888,110 @@ async function catalogItemTranslate(
 						like(CatalogItems.itemIds, `%;${idText};%`),
 						like(CatalogItems.itemIds, `%;${idText}`),
 					),
-				);
-			for (const row of related)
-				await db
-					.update(CatalogItems)
-					.set({ catalogName: nextName })
-					.where(eq(CatalogItems.id, row.id));
-			namesUpdated += 1;
+				)
+				.for("update");
+			relatedByBaseId.set(request.id, related);
 		}
-		if (nextDescription || nextName) {
-			if (nextDescription) descriptionsUpdated += 1;
+
+		const beforeItems: Array<Record<string, unknown>> = [];
+		const afterItems: Array<Record<string, unknown>> = [];
+		const furniPatches: Array<{
+			classname: string;
+			itemType: string;
+			name?: string;
+			description?: string;
+			spriteId?: number;
+			createIfMissing?: boolean;
+		}> = [];
+		let namesUpdated = 0;
+		let descriptionsUpdated = 0;
+		for (const request of requests) {
+			const base = baseById.get(request.id);
+			if (!base) throw validation({ missingIds: [String(request.id)] });
+			const related = relatedByBaseId.get(request.id) ?? [];
+			const nameChanged =
+				Boolean(request.nextName) && request.nextName !== base.publicName;
+			const nextPublicName = nameChanged ? request.nextName : base.publicName;
+			beforeItems.push({
+				id: String(request.id),
+				publicName: base.publicName,
+				catalogItems: related.map((row) => ({
+					id: String(row.id),
+					catalogName: row.catalogName,
+				})),
+			});
+			afterItems.push({
+				id: String(request.id),
+				publicName: nextPublicName,
+				catalogItems: related.map((row) => ({
+					id: String(row.id),
+					catalogName: nameChanged ? request.nextName : row.catalogName,
+				})),
+			});
+			if (nameChanged) {
+				await connection
+					.update(ItemsBase)
+					.set({ publicName: request.nextName })
+					.where(eq(ItemsBase.id, request.id));
+				for (const row of related) {
+					await connection
+						.update(CatalogItems)
+						.set({ catalogName: request.nextName })
+						.where(eq(CatalogItems.id, row.id));
+				}
+				namesUpdated += 1;
+			}
+			if (request.nextDescription) descriptionsUpdated += 1;
 			furniPatches.push({
 				classname: base.itemName,
 				itemType: base.type || "s",
-				name: nextName || base.publicName || base.itemName,
-				description: nextDescription,
+				name: request.nextName || base.publicName || base.itemName,
+				description: request.nextDescription,
 				spriteId: base.spriteId,
 				createIfMissing: true,
 			});
 		}
-	}
-	const snapshot: EconomyMutationSnapshot = {
-		before: null,
-		after: { namesUpdated, descriptionsUpdated },
-		output: { namesUpdated, descriptionsUpdated },
-	};
+		const output = {
+			namesUpdated,
+			descriptionsUpdated,
+			updatedIds: requests.map((request) => String(request.id)),
+			missingIds: [] as string[],
+			rejectedIds: [] as string[],
+		};
+		return {
+			furniPatches,
+			snapshot: {
+				before: { items: beforeItems },
+				after: { items: afterItems },
+				output,
+			} satisfies EconomyMutationSnapshot,
+		};
+	});
 	try {
 		let furniDataUpdated = 0;
 		let furniDataInserted = 0;
-		if (furniPatches.length > 0) {
+		if (committed.furniPatches.length > 0) {
 			const [{ patchFurniEntryNames }, { invalidateFurniDataCache }] =
 				await Promise.all([
 					import("@/lib/services/furni-data"),
 					import("@/lib/services/catalog-items-loader"),
 				]);
-			const result = await patchFurniEntryNames(furniPatches);
+			const result = await patchFurniEntryNames(committed.furniPatches);
 			if (result.updated > 0 || result.inserted > 0) invalidateFurniDataCache();
 			furniDataUpdated = result.updated;
 			furniDataInserted = result.inserted;
 		}
 		return synchronizeCatalog({
-			...snapshot,
+			...committed.snapshot,
 			output: {
-				namesUpdated,
-				descriptionsUpdated,
+				...committed.snapshot.output,
 				furniDataUpdated,
 				furniDataInserted,
 			},
 		});
 	} catch (error) {
 		if (error instanceof EconomyCommittedExternalFailure) throw error;
-		throw new EconomyCommittedExternalFailure(snapshot);
+		throw new EconomyCommittedExternalFailure(committed.snapshot);
 	}
 }