From 0aa579f8c5fc1960f636cb88aba5667fc4c0f32d Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 31 Aug 2026 14:55:47 +0000 Subject: [PATCH 1/2] fix(driver-memory): make bulkUpdate and bulkDelete all-or-nothing bulkUpdate and bulkDelete were still Promise.all(map(...)) over update/delete, and both of those write into the table synchronously. A mid-batch refusal left every row processed before it already mutated, so the caller got a rejection describing a batch that had partly landed -- the same defect #13340 fixed on bulkCreate, on the third and fourth batch doors it did not reach. bulkUpdate now builds and checks every pending row's post-image before writing any of them -- new construction (each id keeps its own patch, so the projected row set per pending row generalizes updateMany's single-shared-patch posture rather than copying it) -- and bulkDelete resolves every id to a table index first, refusing the whole batch under strictMode before touching the table if one is missing. Both follow update/delete's own existing missing-id contract rather than a third posture. bulkDelete still returns void. Fixes #13435 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01F3jdziLbAPGeceVNmSox5L --- ...ver-memory-bulk-update-delete-atomicity.md | 27 ++ ...emory-bulk-update-delete-atomicity.test.ts | 327 ++++++++++++++++++ .../driver-memory/src/memory-driver.ts | 136 +++++++- 3 files changed, 481 insertions(+), 9 deletions(-) create mode 100644 .changeset/driver-memory-bulk-update-delete-atomicity.md create mode 100644 packages/drivers/driver-memory/src/memory-bulk-update-delete-atomicity.test.ts diff --git a/.changeset/driver-memory-bulk-update-delete-atomicity.md b/.changeset/driver-memory-bulk-update-delete-atomicity.md new file mode 100644 index 0000000000..a76c4df6bd --- /dev/null +++ b/.changeset/driver-memory-bulk-update-delete-atomicity.md @@ -0,0 +1,27 @@ +--- +"@objectstack/driver-memory": patch +--- + +fix(driver-memory): make `bulkUpdate` and `bulkDelete` all-or-nothing, so a refused row leaves the table exactly as it found it (#13435) + +`bulkUpdate` and `bulkDelete` were still `Promise.all(map(...))` over +`update`/`delete`, and both of those write into the table synchronously. So a +mid-batch refusal — a `UNIQUE_VIOLATION`/409 on `bulkUpdate`, a missing-id +throw under `strictMode` on `bulkDelete` — left every row processed *before* +it already mutated, and the caller got a rejection describing a batch that +had partly landed. #13340 fixed the identical shape on `bulkCreate`; this is +the third and fourth batch door. + +`bulkUpdate` now builds and checks every pending row's post-image — each id +keeps its own patch, so this is new construction (a projected row set per +pending row, generalized from `updateMany`'s single-shared-patch posture) +rather than a copy of either sibling door — before writing any of them. +`bulkDelete` resolves every id to a table index first, refusing the whole +batch under `strictMode` before touching the table if one is missing, and +only then splices. Both follow `update`/`delete`'s own existing contract for +a missing id (skip under non-strict, refuse under strict) rather than a third +posture. `bulkDelete` still returns `void` — no current caller reads a +per-row outcome, so widening the return type stays out of scope. + +All four batch doors of `InMemoryDriver` (`bulkCreate`, `updateMany`, +`bulkUpdate`, `bulkDelete`) now agree that a batch is atomic. diff --git a/packages/drivers/driver-memory/src/memory-bulk-update-delete-atomicity.test.ts b/packages/drivers/driver-memory/src/memory-bulk-update-delete-atomicity.test.ts new file mode 100644 index 0000000000..2ff18c5f6a --- /dev/null +++ b/packages/drivers/driver-memory/src/memory-bulk-update-delete-atomicity.test.ts @@ -0,0 +1,327 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#13435] `bulkUpdate` and `bulkDelete` are ALL-OR-NOTHING: a refused row + * leaves the table exactly as it found it. + * + * ## The defect this pins, and why "it still throws" would not have caught it + * + * Both doors used to be `Promise.all(map(...))` over `update`/`delete`, and + * both of THOSE write into the table synchronously. So when one row of a + * batch was refused — `UNIQUE_VIOLATION`/409 on `bulkUpdate`, a missing-id + * throw under `strictMode` on `bulkDelete` — every row processed BEFORE it + * stayed mutated, and the caller got a rejection describing a batch that had + * partly landed. #13340 measured the identical shape on `bulkCreate`; these + * are the third and fourth batch doors it did not reach. + * + * ⛔ Asserting "the refusal still happens" proves NOTHING here — the refusal + * was already correct (#13197/#13239 pin `assertUnique`, `delete`'s own + * strict-mode throw predates this file). **The discriminating fact is that + * the TABLE DOES NOT MOVE**, so every test below reads the store back after + * the refusal — full rows, not just a count — rather than stopping at the + * envelope. + * + * ## The construction, and why it is NOT `updateMany`'s shape copied over + * + * `updateMany` stamps ONE shared `data` onto every matched row and has no + * per-row pre-image to exclude. `bulkCreate` has no pre-image at all (every + * row is new). `bulkUpdate` is neither: each id in the batch carries its OWN + * patch, so the fix needed new construction — per pending row, its own + * `exceptId` AND a projected row set holding the OTHER rows' post-images + * while dropping their pre-images — not a transcription of either sibling. + * + * ## What this does NOT claim + * + * Atomicity here is the driver refusing before it writes — not a + * transaction, and no rollback: nothing is written until the whole batch has + * been checked (`bulkUpdate`) or resolved to indices (`bulkDelete`). + */ + +import { describe, it, expect, beforeEach } from 'vitest'; +import { InMemoryDriver } from './memory-driver.js'; + +interface WireBearingError extends Error { + code?: string; + status?: number; +} + +/** Run `fn`, requiring it to reject; hand back the rejection for inspection. */ +async function refusalOf(fn: () => Promise): Promise { + try { + await fn(); + } catch (e) { + return e as WireBearingError; + } + throw new Error('expected the driver to refuse this write, but it resolved'); +} + +const DOC_SCHEMA = { + name: 'doc', + fields: { + id: { type: 'text' }, + doc_no: { type: 'text', unique: 'global' }, + title: { type: 'text' }, + }, +} as any; + +/** The whole table, sorted by `id` — used to prove BYTE-IDENTITY, not just a count. */ +async function snapshot(driver: InMemoryDriver, object: string) { + const rows = await driver.find(object, {}); + return rows.slice().sort((a: any, b: any) => String(a.id).localeCompare(String(b.id))); +} + +describe('[#13435] bulkUpdate refuses BEFORE writing — no surviving prefix', () => { + let driver: InMemoryDriver; + + beforeEach(async () => { + driver = new InMemoryDriver(); + await driver.syncSchema('doc', DOC_SCHEMA); + await driver.create('doc', { id: '1', doc_no: 'D-0001', title: 'One' }); + await driver.create('doc', { id: '2', doc_no: 'D-0002', title: 'Two' }); + await driver.create('doc', { id: '3', doc_no: 'D-0003', title: 'Three' }); + }); + + it('a batch colliding WITHIN itself leaves the table byte-identical', async () => { + const before = await snapshot(driver, 'doc'); + + const err = await refusalOf(() => + driver.bulkUpdate('doc', [ + { id: '1', data: { doc_no: 'D-0100' } }, + { id: '2', data: { doc_no: 'D-0100' } }, // collides with the row above, not with the table + ]), + ); + expect(err.code).toBe('UNIQUE_VIOLATION'); + expect(err.status).toBe(409); + + // Named explicitly: it is row '1' — accepted BEFORE the refusal under the + // old `Promise.all` shape — whose survival used to be the defect. + const after = await snapshot(driver, 'doc'); + expect(after).toEqual(before); + expect((await driver.find('doc', { where: { id: '1' } }))[0].doc_no).toBe('D-0001'); + }); + + it('a batch colliding with a STORED, UNTOUCHED row leaves the table byte-identical', async () => { + const before = await snapshot(driver, 'doc'); + // Row '1' would be accepted (no collision on its own) before row '2' + // collides with row '3' — untouched by this batch, so it sits in + // `settled` — under the old `Promise.all` shape row '1' would have + // landed as a survivor before the second `update()` call threw. + const err = await refusalOf(() => + driver.bulkUpdate('doc', [ + { id: '1', data: { doc_no: 'D-0100' } }, + { id: '2', data: { doc_no: 'D-0003' } }, // row '3's stored value — untouched by this batch + ]), + ); + expect(err.code).toBe('UNIQUE_VIOLATION'); + expect(err.status).toBe(409); + + const after = await snapshot(driver, 'doc'); + expect(after).toEqual(before); + }); + + it('the FIRST row colliding refuses the batch too — the check is not order-dependent', async () => { + const before = await snapshot(driver, 'doc'); + // Row '1' collides with STORED untouched row '3' on the very first + // iteration; row '2's patch is unrelated and would still land under a + // check that bailed out of the loop but had already pushed nothing. + const err = await refusalOf(() => + driver.bulkUpdate('doc', [ + { id: '1', data: { doc_no: 'D-0003' } }, // row '3's stored value — collides immediately + { id: '2', data: { doc_no: 'D-0300' } }, + ]), + ); + expect(err.code).toBe('UNIQUE_VIOLATION'); + const after = await snapshot(driver, 'doc'); + expect(after).toEqual(before); + // The row AFTER the refusal must not land either — named directly, not + // just inferred from the full-snapshot equality above. + expect((await driver.find('doc', { where: { id: '2' } }))[0].doc_no).toBe('D-0002'); + }); + + it('a clean batch still lands in FULL and returns every updated row', async () => { + // The non-vacuity control: a fix that refused everything would pass every + // assertion above. + const out = await driver.bulkUpdate('doc', [ + { id: '1', data: { doc_no: 'D-0100' } }, + { id: '2', data: { doc_no: 'D-0200' } }, + ]); + expect(out.map((r: any) => r.doc_no)).toEqual(['D-0100', 'D-0200']); + expect((await driver.find('doc', { where: { id: '1' } }))[0].doc_no).toBe('D-0100'); + expect((await driver.find('doc', { where: { id: '2' } }))[0].doc_no).toBe('D-0200'); + }); + + it('a row that keeps its OWN unique value (untouched field) does not collide with itself', async () => { + // exceptId must still exclude a row from its own pre-image when the patch + // does not touch the unique field. + const out = await driver.bulkUpdate('doc', [{ id: '1', data: { title: 'Renamed' } }]); + expect(out[0].doc_no).toBe('D-0001'); + expect(out[0].title).toBe('Renamed'); + }); + + it('an empty batch is a no-op that resolves to an empty array', async () => { + const before = await snapshot(driver, 'doc'); + expect(await driver.bulkUpdate('doc', [])).toEqual([]); + expect(await snapshot(driver, 'doc')).toEqual(before); + }); + + describe('non-strict missing id (default `strictMode`)', () => { + it('a missing id resolves to null at its position; the rest of the batch still lands', async () => { + const out = await driver.bulkUpdate('doc', [ + { id: 'ghost', data: { doc_no: 'D-9999' } }, + { id: '1', data: { doc_no: 'D-0100' } }, + ]); + expect(out[0]).toBeNull(); + expect(out[1]?.doc_no).toBe('D-0100'); + expect((await driver.find('doc', { where: { id: '1' } }))[0].doc_no).toBe('D-0100'); + }); + }); + + describe('strictMode: true', () => { + it('a missing id refuses the WHOLE batch — table byte-identical, valid rows included', async () => { + const strict = new InMemoryDriver({ strictMode: true }); + await strict.syncSchema('doc', DOC_SCHEMA); + await strict.create('doc', { id: '1', doc_no: 'D-0001', title: 'One' }); + await strict.create('doc', { id: '2', doc_no: 'D-0002', title: 'Two' }); + const before = await snapshot(strict, 'doc'); + + await expect( + strict.bulkUpdate('doc', [ + { id: '1', data: { doc_no: 'D-0100' } }, // would have been valid alone + { id: 'ghost', data: { doc_no: 'D-9999' } }, + ]), + ).rejects.toThrow(); + + expect(await snapshot(strict, 'doc')).toEqual(before); + }); + }); +}); + +describe('[#13435] bulkDelete refuses BEFORE writing — no surviving prefix', () => { + let driver: InMemoryDriver; + + beforeEach(async () => { + driver = new InMemoryDriver({ strictMode: true }); + await driver.syncSchema('doc', DOC_SCHEMA); + await driver.create('doc', { id: '1', doc_no: 'D-0001' }); + await driver.create('doc', { id: '2', doc_no: 'D-0002' }); + await driver.create('doc', { id: '3', doc_no: 'D-0003' }); + }); + + it('strictMode: a missing id refuses the WHOLE batch — table byte-identical, valid ids included', async () => { + const before = await snapshot(driver, 'doc'); + + await expect(driver.bulkDelete('doc', ['1', 'ghost', '2'])).rejects.toThrow(); + + // Id '1' would have been removed BEFORE the refusal under the old + // `Promise.all` shape. Named explicitly, not just via count. + const after = await snapshot(driver, 'doc'); + expect(after).toEqual(before); + expect(await driver.count('doc')).toBe(3); + }); + + it('strictMode: a clean batch still removes every named row', async () => { + await driver.bulkDelete('doc', ['1', '3']); + expect(await driver.count('doc')).toBe(1); + expect((await driver.find('doc', { where: {} }))[0].id).toBe('2'); + }); + + describe('non-strict (default `strictMode`)', () => { + it('a missing id is SKIPPED; the rest of the batch still lands', async () => { + const loose = new InMemoryDriver(); + await loose.syncSchema('doc', DOC_SCHEMA); + await loose.create('doc', { id: '1', doc_no: 'D-0001' }); + await loose.create('doc', { id: '2', doc_no: 'D-0002' }); + + await loose.bulkDelete('doc', ['1', 'ghost']); + expect(await loose.count('doc')).toBe(1); + expect((await loose.find('doc', { where: {} }))[0].id).toBe('2'); + }); + }); + + it('an empty batch is a no-op', async () => { + const before = await snapshot(driver, 'doc'); + await driver.bulkDelete('doc', []); + expect(await snapshot(driver, 'doc')).toEqual(before); + }); + + it('duplicate ids in one batch delete the row once, not twice', async () => { + await driver.bulkDelete('doc', ['1', '1']); + expect(await driver.count('doc')).toBe(2); + }); +}); + +describe('[#13435] all FOUR batch doors of this driver now agree that a batch is atomic', () => { + it('bulkCreate, updateMany, bulkUpdate and bulkDelete all refuse without moving the table', async () => { + const driver = new InMemoryDriver({ strictMode: true }); + await driver.syncSchema('doc', DOC_SCHEMA); + await driver.create('doc', { id: '1', doc_no: 'D-0001' }); + await driver.create('doc', { id: '2', doc_no: 'D-0002' }); + const before = await snapshot(driver, 'doc'); + + const createErr = await refusalOf(() => + driver.bulkCreate('doc', [ + { id: 'a', doc_no: 'D-0100' }, + { id: 'b', doc_no: 'D-0100' }, + ]), + ); + const updateManyErr = await refusalOf(() => driver.updateMany('doc', { where: {} }, { doc_no: 'D-0009' })); + const bulkUpdateErr = await refusalOf(() => + driver.bulkUpdate('doc', [ + { id: '1', data: { doc_no: 'D-0100' } }, + { id: '2', data: { doc_no: 'D-0100' } }, // collides with the row above, not the table + ]), + ); + const bulkDeleteErr = await refusalOf(() => driver.bulkDelete('doc', ['1', 'ghost'])); + + expect(createErr.code).toBe('UNIQUE_VIOLATION'); + expect(updateManyErr.code).toBe('UNIQUE_VIOLATION'); + expect(bulkUpdateErr.code).toBe('UNIQUE_VIOLATION'); + expect(bulkDeleteErr).toBeInstanceOf(Error); + + // None of the four doors moved the store. + expect(await snapshot(driver, 'doc')).toEqual(before); + }); +}); + +describe('[#13435] non-regression — updateMany and bulkCreate still behave as #13197/#13340 left them', () => { + let driver: InMemoryDriver; + + beforeEach(async () => { + driver = new InMemoryDriver(); + await driver.syncSchema('doc', DOC_SCHEMA); + await driver.create('doc', { id: '1', doc_no: 'D-0001' }); + await driver.create('doc', { id: '2', doc_no: 'D-0002' }); + }); + + it('updateMany still refuses a colliding shared patch and leaves the table untouched', async () => { + const before = await snapshot(driver, 'doc'); + const err = await refusalOf(() => driver.updateMany('doc', { where: {} }, { doc_no: 'D-SAME' })); + expect(err.code).toBe('UNIQUE_VIOLATION'); + expect(await snapshot(driver, 'doc')).toEqual(before); + }); + + it('updateMany still applies a clean shared patch to every matched row', async () => { + const count = await driver.updateMany('doc', { where: {} }, { title: 'Bulk' }); + expect(count).toBe(2); + expect((await driver.find('doc', { where: { id: '1' } }))[0].title).toBe('Bulk'); + expect((await driver.find('doc', { where: { id: '2' } }))[0].title).toBe('Bulk'); + }); + + it('bulkCreate still refuses a self-colliding batch and leaves the table untouched', async () => { + const before = await snapshot(driver, 'doc'); + const err = await refusalOf(() => + driver.bulkCreate('doc', [ + { id: 'a', doc_no: 'D-0100' }, + { id: 'b', doc_no: 'D-0100' }, + ]), + ); + expect(err.code).toBe('UNIQUE_VIOLATION'); + expect(await snapshot(driver, 'doc')).toEqual(before); + }); + + it('bulkCreate still lands a clean batch in full', async () => { + const out = await driver.bulkCreate('doc', [{ id: 'a', doc_no: 'D-0100' }]); + expect(out).toHaveLength(1); + expect(await driver.count('doc')).toBe(3); + }); +}); diff --git a/packages/drivers/driver-memory/src/memory-driver.ts b/packages/drivers/driver-memory/src/memory-driver.ts index 421e00e694..49a3aa0b08 100644 --- a/packages/drivers/driver-memory/src/memory-driver.ts +++ b/packages/drivers/driver-memory/src/memory-driver.ts @@ -334,10 +334,17 @@ interface MemoryTransaction { * — which made a caller's retry of the same array unsafe for reasons that had * nothing to do with constraints. * - * ⚠️ That is a statement about those TWO doors, not about batches in general. - * {@link bulkUpdate} and {@link bulkDelete} are still `Promise.all(map(...))` - * and still leave the rows applied before a refusal standing — #13435. Do not - * read the paragraph above as covering them. + * Since #13435 {@link bulkUpdate} and {@link bulkDelete} carry the same + * discipline, so all FOUR batch doors of this driver now agree that a batch + * is atomic. `bulkUpdate` builds and checks every pending row's post-image — + * each id keeps its OWN patch, so the check is against the untouched rows plus + * every other pending row's post-image, exceptId'd against its own id — before + * writing any of them; `bulkDelete` resolves every id to a table index first + * (refusing the whole batch under `strictMode` before touching the table if + * one is missing) and only then splices. Both used to be `Promise.all(map(...))` + * over `update`/`delete`, which write synchronously, so a refusal left every + * row processed before it mutated — the same defect #13340 measured on + * `bulkCreate`, on the two doors that stayed the pre-#13340 shape. * * Everything else is still unenforced. {@link syncSchema} allocates an array, * indexes temporal fields and records those unique constraints — and nothing @@ -865,17 +872,128 @@ export class InMemoryDriver implements IDataDriver { } // Compatibility aliases + /** + * [#13435] All-or-nothing, generalized from {@link updateMany}'s posture + * (#13197) to a PER-ID patch. Used to be + * `Promise.all(updates.map(u => this.update(object, u.id, u.data, options)))`, + * and `update` writes into the table synchronously and calls + * {@link assertUnique}, so a mid-batch refusal left every row processed + * BEFORE it mutated — the same defect #13340 measured on `bulkCreate`. + * + * `updateMany` has no per-row pre-image to exclude (one shared `data` stamped + * onto every matched row); `bulkCreate` has no pre-image at all (new rows). + * `bulkUpdate` is neither: each id carries its OWN patch, so what transfers + * is the DISCIPLINE — build and check every pending row's post-image before + * mutating any of them — not the shape. The projected row set for each + * pending row's check is the untouched rows (`settled`, never touched by + * this batch) plus every ALREADY-VALIDATED pending row's post-image; a + * not-yet-processed row of the same batch needs no look-ahead entry, because + * whichever of two colliding rows is checked SECOND will always find the + * first already sitting in `pending` — the same incremental discipline + * `bulkCreate`/`updateMany` use, generalized to per-row patches rather than + * one shared patch or no pre-image at all. + * + * A missing id follows `update`'s OWN existing contract — never a third + * posture: refuse the WHOLE batch (before any row is touched) when + * `strictMode` is on, resolve that position to `null` (skipped) when it is + * off, exactly as a single `update()` call on a missing id already does. + */ async bulkUpdate(object: string, updates: { id: string | number, data: Record }[], options?: DriverOptions) { this.logger.debug('BulkUpdate operation', { object, count: updates.length }); - const results = await Promise.all(updates.map(u => this.update(object, u.id, u.data, options))); - this.logger.debug('BulkUpdate completed', { object, count: results.length }); - return results; + + const table = this.getTable(object); + const touchedIds = new Set(updates.map((u) => u.id)); + const settled = table.filter((r) => !touchedIds.has(r.id)); + + const perUpdate: Array<{ index: number; row: Record } | null> = []; + const pending: Record[] = []; + + for (const u of updates) { + const index = table.findIndex((r) => r.id == u.id); + if (index === -1) { + if (this.config.strictMode) { + this.logger.warn('Record not found for bulk update', { object, id: u.id }); + throw new Error(`Record with ID ${u.id} not found in ${object}`); + } + perUpdate.push(null); + continue; + } + + const updatedRecord = this.toStoredRecord(object, { + ...table[index], + ...u.data, + id: table[index].id, // Preserve original ID + created_at: table[index].created_at, // Preserve created_at + updated_at: new Date().toISOString(), + }); + + // [#13435] Checked against the OTHER rows of the eventual write — settled + // rows plus every pending row already validated in this loop — and + // `exceptId` still excludes this row from colliding with itself, exactly + // as `update`'s own single-row check does. + this.assertUnique(object, updatedRecord, table[index].id, [...settled, ...pending]); + + pending.push(updatedRecord); + perUpdate.push({ index, row: updatedRecord }); + } + + // Every pending row is CHECKED at this point — nothing above has written + // to `table` yet, so a refusal anywhere in the loop left it untouched. + // Only now do the validated rows land, all at once. + for (const entry of perUpdate) { + if (entry) table[entry.index] = entry.row; + } + + if (pending.length > 0) this.markDirty(); + this.logger.debug('BulkUpdate completed', { object, count: pending.length }); + return perUpdate.map((entry) => (entry ? { ...entry.row } : null)); } + /** + * [#13435] All-or-nothing under `strictMode`. Used to be + * `Promise.all(ids.map(id => this.delete(object, id, options)))`, and + * `delete` splices out of the table synchronously and throws on a missing id + * when `strictMode` is on, so a refusal partway through left every id + * processed BEFORE it already removed — the same defect #13340 measured on + * `bulkCreate`, on the delete door. + * + * A missing id follows `delete`'s OWN existing contract — never a third + * posture: refuse the WHOLE batch (before any row is removed) when + * `strictMode` is on — `history-cleanup.ts` and `lifecycle-service.ts` both + * call this with id lists that can contain already-gone rows, so refusing on + * a missing id would break those live callers. `bulkDelete` still returns + * `void`: no current caller reads a per-row outcome, and widening the return + * type is out of scope for #13435. + */ async bulkDelete(object: string, ids: (string | number)[], options?: DriverOptions) { this.logger.debug('BulkDelete operation', { object, count: ids.length }); - await Promise.all(ids.map(id => this.delete(object, id, options))); - this.logger.debug('BulkDelete completed', { object, count: ids.length }); + + const table = this.getTable(object); + + // Resolve every id to a table index BEFORE removing any of them, so a + // strict-mode refusal on a later id cannot leave an earlier one already + // spliced out. A `Set` absorbs a duplicate id naming the same index twice. + const indices = new Set(); + for (const id of ids) { + const index = table.findIndex((r) => r.id == id); + if (index === -1) { + if (this.config.strictMode) { + throw new Error(`Record with ID ${id} not found in ${object}`); + } + this.logger.warn('Record not found for bulk deletion', { object, id }); + continue; + } + indices.add(index); + } + + // Highest index first: splicing low-to-high would shift every later index + // out from under itself. + for (const index of [...indices].sort((a, b) => b - a)) { + table.splice(index, 1); + } + + if (indices.size > 0) this.markDirty(); + this.logger.debug('BulkDelete completed', { object, count: indices.size }); } // =================================== From fdd74c66eef0e17d549611f495587f360a79259e Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 31 Aug 2026 15:04:17 +0000 Subject: [PATCH 2/2] fix(driver-memory): bulkUpdate omits a skipped row instead of null IDataDriver.bulkUpdate is declared Promise[]> -- no null member. Padding the result with null for a non-strict missing id (as update() itself returns) failed tsc against that contract once the intermediate array was explicitly typed. Follow SqlDriver.bulkUpdate's own existing convention instead: omit the row entirely (if (updated) results.push(updated)) rather than inventing a second "missing" representation. Part of #13435 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01F3jdziLbAPGeceVNmSox5L --- .../memory-bulk-update-delete-atomicity.test.ts | 9 ++++++--- .../drivers/driver-memory/src/memory-driver.ts | 14 +++++++++++--- 2 files changed, 17 insertions(+), 6 deletions(-) diff --git a/packages/drivers/driver-memory/src/memory-bulk-update-delete-atomicity.test.ts b/packages/drivers/driver-memory/src/memory-bulk-update-delete-atomicity.test.ts index 2ff18c5f6a..d93c83bab2 100644 --- a/packages/drivers/driver-memory/src/memory-bulk-update-delete-atomicity.test.ts +++ b/packages/drivers/driver-memory/src/memory-bulk-update-delete-atomicity.test.ts @@ -165,13 +165,16 @@ describe('[#13435] bulkUpdate refuses BEFORE writing — no surviving prefix', ( }); describe('non-strict missing id (default `strictMode`)', () => { - it('a missing id resolves to null at its position; the rest of the batch still lands', async () => { + it('a missing id is SKIPPED (no placeholder); the rest of the batch still lands', async () => { + // `IDataDriver.bulkUpdate` is declared `Promise[]>` + // — no `null` member — so a skipped id is OMITTED, not padded, mirroring + // `SqlDriver.bulkUpdate`'s own `if (updated) results.push(updated)`. const out = await driver.bulkUpdate('doc', [ { id: 'ghost', data: { doc_no: 'D-9999' } }, { id: '1', data: { doc_no: 'D-0100' } }, ]); - expect(out[0]).toBeNull(); - expect(out[1]?.doc_no).toBe('D-0100'); + expect(out).toHaveLength(1); + expect(out[0].doc_no).toBe('D-0100'); expect((await driver.find('doc', { where: { id: '1' } }))[0].doc_no).toBe('D-0100'); }); }); diff --git a/packages/drivers/driver-memory/src/memory-driver.ts b/packages/drivers/driver-memory/src/memory-driver.ts index 49a3aa0b08..20f35d075e 100644 --- a/packages/drivers/driver-memory/src/memory-driver.ts +++ b/packages/drivers/driver-memory/src/memory-driver.ts @@ -895,8 +895,13 @@ export class InMemoryDriver implements IDataDriver { * * A missing id follows `update`'s OWN existing contract — never a third * posture: refuse the WHOLE batch (before any row is touched) when - * `strictMode` is on, resolve that position to `null` (skipped) when it is - * off, exactly as a single `update()` call on a missing id already does. + * `strictMode` is on, skip it when it is off. The returned array holds + * only the rows actually updated, in `updates` order, with no placeholder + * for a skipped id — `IDataDriver.bulkUpdate` is declared + * `Promise[]>`, no `null` member, and `SqlDriver`'s + * own `bulkUpdate` (`packages/drivers/driver-sql`) already resolves a + * missing id the same way (`if (updated) results.push(updated)`); this + * follows that established convention rather than inventing a second one. */ async bulkUpdate(object: string, updates: { id: string | number, data: Record }[], options?: DriverOptions) { this.logger.debug('BulkUpdate operation', { object, count: updates.length }); @@ -946,7 +951,10 @@ export class InMemoryDriver implements IDataDriver { if (pending.length > 0) this.markDirty(); this.logger.debug('BulkUpdate completed', { object, count: pending.length }); - return perUpdate.map((entry) => (entry ? { ...entry.row } : null)); + // `pending` already holds only the rows that were actually resolved and + // written — a skipped (non-strict missing) id never entered it — so this + // is `updates` order with no placeholder for the ones that were skipped. + return pending.map((row) => ({ ...row })); } /**