From 4e9fddf644c28916236fdd00df99cc577f259800 Mon Sep 17 00:00:00 2001 From: Darien Kindlund Date: Tue, 3 Mar 2026 14:35:03 -0500 Subject: [PATCH] fix: add foreign record locking to ManyMany, OneMany, and OneOne link operations saveForeignKeyForManyMany had NO record locking, unlike saveForeignKeyForManyOne which calls lockForeignRecords. This meant concurrent ManyMany link updates could create duplicate junction rows or lose deletions. Similarly, saveForeignKeyForOneMany (non-one-way) and saveForeignKeyForOneOne (non-ManyOne branch) modified foreign table records without locking. This fix adds lockForeignRecords calls to all three methods, matching the pattern already used by saveForeignKeyForManyOne: - saveForeignKeyForManyMany: locks foreign records from toDelete, toAdd, and toDeleteAndReinsert before any junction table modifications - saveForeignKeyForOneMany: locks all affected foreign records before updating FK columns on the foreign table - saveForeignKeyForOneOne: locks affected foreign records in the else branch (selfKeyName !== '__id') Also fixes a missing await on the saveForeignKeyForManyMany delegation in saveForeignKeyForOneMany when isOneWay=true. The async call was previously fire-and-forget, meaning junction table modifications could complete after the caller assumed they were done. Co-Authored-By: Claude Opus 4.6 (1M context) --- .../src/features/calculation/link.service.ts | 26 ++++++++++++++++++- 1 file changed, 25 insertions(+), 1 deletion(-) diff --git a/apps/nestjs-backend/src/features/calculation/link.service.ts b/apps/nestjs-backend/src/features/calculation/link.service.ts index 91ba673240..5d9440e108 100644 --- a/apps/nestjs-backend/src/features/calculation/link.service.ts +++ b/apps/nestjs-backend/src/features/calculation/link.service.ts @@ -1197,6 +1197,14 @@ export class LinkService { } } + // Lock affected foreign records to prevent concurrent modifications + const affectedForeignIds = uniq([ + ...toDelete.map(([, foreignId]) => foreignId), + ...toAdd.map(([, foreignId]) => foreignId), + ...toDeleteAndReinsert.flatMap(([, newKeys]) => newKeys), + ]); + await this.lockForeignRecords(field.options.foreignTableId, affectedForeignIds); + // Handle order changes: delete all existing records for affected recordIds and re-insert if (toDeleteAndReinsert.length) { const recordIdsToDeleteAll = toDeleteAndReinsert.map(([recordId]) => recordId); @@ -1429,10 +1437,20 @@ export class LinkService { const { selfKeyName, foreignKeyName, fkHostTableName, isOneWay } = field.options; if (isOneWay) { - this.saveForeignKeyForManyMany(field, fkMap); + await this.saveForeignKeyForManyMany(field, fkMap); return; } + // Lock affected foreign records to prevent concurrent modifications + const allForeignIds: string[] = []; + for (const recordId in fkMap) { + const fkItem = fkMap[recordId]; + const oldKey = (fkItem.oldKey || []) as string[]; + const newKey = (fkItem.newKey || []) as string[]; + allForeignIds.push(...oldKey, ...newKey); + } + await this.lockForeignRecords(field.options.foreignTableId, uniq(allForeignIds)); + // Process each record individually to maintain order for (const recordId in fkMap) { const fkItem = fkMap[recordId]; @@ -1589,6 +1607,12 @@ export class LinkService { newKey && toAdd.push([recordId, newKey]); } + // Lock affected foreign records to prevent concurrent modifications + const affectedForeignIds = uniq( + toDelete.map(([, foreignId]) => foreignId).concat(toAdd.map(([, foreignId]) => foreignId)) + ); + await this.lockForeignRecords(field.options.foreignTableId, affectedForeignIds); + if (toDelete.length) { const updateFields: Record = { [selfKeyName]: null }; // Also clear order column if field has order column