Skip to content

Commit 4642f4c

Browse files
fix(driver-memory): make bulkUpdate and bulkDelete all-or-nothing (#13875)
* 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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01F3jdziLbAPGeceVNmSox5L * fix(driver-memory): bulkUpdate omits a skipped row instead of null IDataDriver.bulkUpdate is declared Promise<Record<string, unknown>[]> -- 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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01F3jdziLbAPGeceVNmSox5L --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 0c143ec commit 4642f4c

3 files changed

Lines changed: 492 additions & 9 deletions

File tree

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
---
2+
"@objectstack/driver-memory": patch
3+
---
4+
5+
fix(driver-memory): make `bulkUpdate` and `bulkDelete` all-or-nothing, so a refused row leaves the table exactly as it found it (#13435)
6+
7+
`bulkUpdate` and `bulkDelete` were still `Promise.all(map(...))` over
8+
`update`/`delete`, and both of those write into the table synchronously. So a
9+
mid-batch refusal — a `UNIQUE_VIOLATION`/409 on `bulkUpdate`, a missing-id
10+
throw under `strictMode` on `bulkDelete` — left every row processed *before*
11+
it already mutated, and the caller got a rejection describing a batch that
12+
had partly landed. #13340 fixed the identical shape on `bulkCreate`; this is
13+
the third and fourth batch door.
14+
15+
`bulkUpdate` now builds and checks every pending row's post-image — each id
16+
keeps its own patch, so this is new construction (a projected row set per
17+
pending row, generalized from `updateMany`'s single-shared-patch posture)
18+
rather than a copy of either sibling door — before writing any of them.
19+
`bulkDelete` resolves every id to a table index first, refusing the whole
20+
batch under `strictMode` before touching the table if one is missing, and
21+
only then splices. Both follow `update`/`delete`'s own existing contract for
22+
a missing id (skip under non-strict, refuse under strict) rather than a third
23+
posture. `bulkDelete` still returns `void` — no current caller reads a
24+
per-row outcome, so widening the return type stays out of scope.
25+
26+
All four batch doors of `InMemoryDriver` (`bulkCreate`, `updateMany`,
27+
`bulkUpdate`, `bulkDelete`) now agree that a batch is atomic.
Lines changed: 330 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,330 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* [#13435] `bulkUpdate` and `bulkDelete` are ALL-OR-NOTHING: a refused row
5+
* leaves the table exactly as it found it.
6+
*
7+
* ## The defect this pins, and why "it still throws" would not have caught it
8+
*
9+
* Both doors used to be `Promise.all(map(...))` over `update`/`delete`, and
10+
* both of THOSE write into the table synchronously. So when one row of a
11+
* batch was refused — `UNIQUE_VIOLATION`/409 on `bulkUpdate`, a missing-id
12+
* throw under `strictMode` on `bulkDelete` — every row processed BEFORE it
13+
* stayed mutated, and the caller got a rejection describing a batch that had
14+
* partly landed. #13340 measured the identical shape on `bulkCreate`; these
15+
* are the third and fourth batch doors it did not reach.
16+
*
17+
* ⛔ Asserting "the refusal still happens" proves NOTHING here — the refusal
18+
* was already correct (#13197/#13239 pin `assertUnique`, `delete`'s own
19+
* strict-mode throw predates this file). **The discriminating fact is that
20+
* the TABLE DOES NOT MOVE**, so every test below reads the store back after
21+
* the refusal — full rows, not just a count — rather than stopping at the
22+
* envelope.
23+
*
24+
* ## The construction, and why it is NOT `updateMany`'s shape copied over
25+
*
26+
* `updateMany` stamps ONE shared `data` onto every matched row and has no
27+
* per-row pre-image to exclude. `bulkCreate` has no pre-image at all (every
28+
* row is new). `bulkUpdate` is neither: each id in the batch carries its OWN
29+
* patch, so the fix needed new construction — per pending row, its own
30+
* `exceptId` AND a projected row set holding the OTHER rows' post-images
31+
* while dropping their pre-images — not a transcription of either sibling.
32+
*
33+
* ## What this does NOT claim
34+
*
35+
* Atomicity here is the driver refusing before it writes — not a
36+
* transaction, and no rollback: nothing is written until the whole batch has
37+
* been checked (`bulkUpdate`) or resolved to indices (`bulkDelete`).
38+
*/
39+
40+
import { describe, it, expect, beforeEach } from 'vitest';
41+
import { InMemoryDriver } from './memory-driver.js';
42+
43+
interface WireBearingError extends Error {
44+
code?: string;
45+
status?: number;
46+
}
47+
48+
/** Run `fn`, requiring it to reject; hand back the rejection for inspection. */
49+
async function refusalOf(fn: () => Promise<unknown>): Promise<WireBearingError> {
50+
try {
51+
await fn();
52+
} catch (e) {
53+
return e as WireBearingError;
54+
}
55+
throw new Error('expected the driver to refuse this write, but it resolved');
56+
}
57+
58+
const DOC_SCHEMA = {
59+
name: 'doc',
60+
fields: {
61+
id: { type: 'text' },
62+
doc_no: { type: 'text', unique: 'global' },
63+
title: { type: 'text' },
64+
},
65+
} as any;
66+
67+
/** The whole table, sorted by `id` — used to prove BYTE-IDENTITY, not just a count. */
68+
async function snapshot(driver: InMemoryDriver, object: string) {
69+
const rows = await driver.find(object, {});
70+
return rows.slice().sort((a: any, b: any) => String(a.id).localeCompare(String(b.id)));
71+
}
72+
73+
describe('[#13435] bulkUpdate refuses BEFORE writing — no surviving prefix', () => {
74+
let driver: InMemoryDriver;
75+
76+
beforeEach(async () => {
77+
driver = new InMemoryDriver();
78+
await driver.syncSchema('doc', DOC_SCHEMA);
79+
await driver.create('doc', { id: '1', doc_no: 'D-0001', title: 'One' });
80+
await driver.create('doc', { id: '2', doc_no: 'D-0002', title: 'Two' });
81+
await driver.create('doc', { id: '3', doc_no: 'D-0003', title: 'Three' });
82+
});
83+
84+
it('a batch colliding WITHIN itself leaves the table byte-identical', async () => {
85+
const before = await snapshot(driver, 'doc');
86+
87+
const err = await refusalOf(() =>
88+
driver.bulkUpdate('doc', [
89+
{ id: '1', data: { doc_no: 'D-0100' } },
90+
{ id: '2', data: { doc_no: 'D-0100' } }, // collides with the row above, not with the table
91+
]),
92+
);
93+
expect(err.code).toBe('UNIQUE_VIOLATION');
94+
expect(err.status).toBe(409);
95+
96+
// Named explicitly: it is row '1' — accepted BEFORE the refusal under the
97+
// old `Promise.all` shape — whose survival used to be the defect.
98+
const after = await snapshot(driver, 'doc');
99+
expect(after).toEqual(before);
100+
expect((await driver.find('doc', { where: { id: '1' } }))[0].doc_no).toBe('D-0001');
101+
});
102+
103+
it('a batch colliding with a STORED, UNTOUCHED row leaves the table byte-identical', async () => {
104+
const before = await snapshot(driver, 'doc');
105+
// Row '1' would be accepted (no collision on its own) before row '2'
106+
// collides with row '3' — untouched by this batch, so it sits in
107+
// `settled` — under the old `Promise.all` shape row '1' would have
108+
// landed as a survivor before the second `update()` call threw.
109+
const err = await refusalOf(() =>
110+
driver.bulkUpdate('doc', [
111+
{ id: '1', data: { doc_no: 'D-0100' } },
112+
{ id: '2', data: { doc_no: 'D-0003' } }, // row '3's stored value — untouched by this batch
113+
]),
114+
);
115+
expect(err.code).toBe('UNIQUE_VIOLATION');
116+
expect(err.status).toBe(409);
117+
118+
const after = await snapshot(driver, 'doc');
119+
expect(after).toEqual(before);
120+
});
121+
122+
it('the FIRST row colliding refuses the batch too — the check is not order-dependent', async () => {
123+
const before = await snapshot(driver, 'doc');
124+
// Row '1' collides with STORED untouched row '3' on the very first
125+
// iteration; row '2's patch is unrelated and would still land under a
126+
// check that bailed out of the loop but had already pushed nothing.
127+
const err = await refusalOf(() =>
128+
driver.bulkUpdate('doc', [
129+
{ id: '1', data: { doc_no: 'D-0003' } }, // row '3's stored value — collides immediately
130+
{ id: '2', data: { doc_no: 'D-0300' } },
131+
]),
132+
);
133+
expect(err.code).toBe('UNIQUE_VIOLATION');
134+
const after = await snapshot(driver, 'doc');
135+
expect(after).toEqual(before);
136+
// The row AFTER the refusal must not land either — named directly, not
137+
// just inferred from the full-snapshot equality above.
138+
expect((await driver.find('doc', { where: { id: '2' } }))[0].doc_no).toBe('D-0002');
139+
});
140+
141+
it('a clean batch still lands in FULL and returns every updated row', async () => {
142+
// The non-vacuity control: a fix that refused everything would pass every
143+
// assertion above.
144+
const out = await driver.bulkUpdate('doc', [
145+
{ id: '1', data: { doc_no: 'D-0100' } },
146+
{ id: '2', data: { doc_no: 'D-0200' } },
147+
]);
148+
expect(out.map((r: any) => r.doc_no)).toEqual(['D-0100', 'D-0200']);
149+
expect((await driver.find('doc', { where: { id: '1' } }))[0].doc_no).toBe('D-0100');
150+
expect((await driver.find('doc', { where: { id: '2' } }))[0].doc_no).toBe('D-0200');
151+
});
152+
153+
it('a row that keeps its OWN unique value (untouched field) does not collide with itself', async () => {
154+
// exceptId must still exclude a row from its own pre-image when the patch
155+
// does not touch the unique field.
156+
const out = await driver.bulkUpdate('doc', [{ id: '1', data: { title: 'Renamed' } }]);
157+
expect(out[0].doc_no).toBe('D-0001');
158+
expect(out[0].title).toBe('Renamed');
159+
});
160+
161+
it('an empty batch is a no-op that resolves to an empty array', async () => {
162+
const before = await snapshot(driver, 'doc');
163+
expect(await driver.bulkUpdate('doc', [])).toEqual([]);
164+
expect(await snapshot(driver, 'doc')).toEqual(before);
165+
});
166+
167+
describe('non-strict missing id (default `strictMode`)', () => {
168+
it('a missing id is SKIPPED (no placeholder); the rest of the batch still lands', async () => {
169+
// `IDataDriver.bulkUpdate` is declared `Promise<Record<string, unknown>[]>`
170+
// — no `null` member — so a skipped id is OMITTED, not padded, mirroring
171+
// `SqlDriver.bulkUpdate`'s own `if (updated) results.push(updated)`.
172+
const out = await driver.bulkUpdate('doc', [
173+
{ id: 'ghost', data: { doc_no: 'D-9999' } },
174+
{ id: '1', data: { doc_no: 'D-0100' } },
175+
]);
176+
expect(out).toHaveLength(1);
177+
expect(out[0].doc_no).toBe('D-0100');
178+
expect((await driver.find('doc', { where: { id: '1' } }))[0].doc_no).toBe('D-0100');
179+
});
180+
});
181+
182+
describe('strictMode: true', () => {
183+
it('a missing id refuses the WHOLE batch — table byte-identical, valid rows included', async () => {
184+
const strict = new InMemoryDriver({ strictMode: true });
185+
await strict.syncSchema('doc', DOC_SCHEMA);
186+
await strict.create('doc', { id: '1', doc_no: 'D-0001', title: 'One' });
187+
await strict.create('doc', { id: '2', doc_no: 'D-0002', title: 'Two' });
188+
const before = await snapshot(strict, 'doc');
189+
190+
await expect(
191+
strict.bulkUpdate('doc', [
192+
{ id: '1', data: { doc_no: 'D-0100' } }, // would have been valid alone
193+
{ id: 'ghost', data: { doc_no: 'D-9999' } },
194+
]),
195+
).rejects.toThrow();
196+
197+
expect(await snapshot(strict, 'doc')).toEqual(before);
198+
});
199+
});
200+
});
201+
202+
describe('[#13435] bulkDelete refuses BEFORE writing — no surviving prefix', () => {
203+
let driver: InMemoryDriver;
204+
205+
beforeEach(async () => {
206+
driver = new InMemoryDriver({ strictMode: true });
207+
await driver.syncSchema('doc', DOC_SCHEMA);
208+
await driver.create('doc', { id: '1', doc_no: 'D-0001' });
209+
await driver.create('doc', { id: '2', doc_no: 'D-0002' });
210+
await driver.create('doc', { id: '3', doc_no: 'D-0003' });
211+
});
212+
213+
it('strictMode: a missing id refuses the WHOLE batch — table byte-identical, valid ids included', async () => {
214+
const before = await snapshot(driver, 'doc');
215+
216+
await expect(driver.bulkDelete('doc', ['1', 'ghost', '2'])).rejects.toThrow();
217+
218+
// Id '1' would have been removed BEFORE the refusal under the old
219+
// `Promise.all` shape. Named explicitly, not just via count.
220+
const after = await snapshot(driver, 'doc');
221+
expect(after).toEqual(before);
222+
expect(await driver.count('doc')).toBe(3);
223+
});
224+
225+
it('strictMode: a clean batch still removes every named row', async () => {
226+
await driver.bulkDelete('doc', ['1', '3']);
227+
expect(await driver.count('doc')).toBe(1);
228+
expect((await driver.find('doc', { where: {} }))[0].id).toBe('2');
229+
});
230+
231+
describe('non-strict (default `strictMode`)', () => {
232+
it('a missing id is SKIPPED; the rest of the batch still lands', async () => {
233+
const loose = new InMemoryDriver();
234+
await loose.syncSchema('doc', DOC_SCHEMA);
235+
await loose.create('doc', { id: '1', doc_no: 'D-0001' });
236+
await loose.create('doc', { id: '2', doc_no: 'D-0002' });
237+
238+
await loose.bulkDelete('doc', ['1', 'ghost']);
239+
expect(await loose.count('doc')).toBe(1);
240+
expect((await loose.find('doc', { where: {} }))[0].id).toBe('2');
241+
});
242+
});
243+
244+
it('an empty batch is a no-op', async () => {
245+
const before = await snapshot(driver, 'doc');
246+
await driver.bulkDelete('doc', []);
247+
expect(await snapshot(driver, 'doc')).toEqual(before);
248+
});
249+
250+
it('duplicate ids in one batch delete the row once, not twice', async () => {
251+
await driver.bulkDelete('doc', ['1', '1']);
252+
expect(await driver.count('doc')).toBe(2);
253+
});
254+
});
255+
256+
describe('[#13435] all FOUR batch doors of this driver now agree that a batch is atomic', () => {
257+
it('bulkCreate, updateMany, bulkUpdate and bulkDelete all refuse without moving the table', async () => {
258+
const driver = new InMemoryDriver({ strictMode: true });
259+
await driver.syncSchema('doc', DOC_SCHEMA);
260+
await driver.create('doc', { id: '1', doc_no: 'D-0001' });
261+
await driver.create('doc', { id: '2', doc_no: 'D-0002' });
262+
const before = await snapshot(driver, 'doc');
263+
264+
const createErr = await refusalOf(() =>
265+
driver.bulkCreate('doc', [
266+
{ id: 'a', doc_no: 'D-0100' },
267+
{ id: 'b', doc_no: 'D-0100' },
268+
]),
269+
);
270+
const updateManyErr = await refusalOf(() => driver.updateMany('doc', { where: {} }, { doc_no: 'D-0009' }));
271+
const bulkUpdateErr = await refusalOf(() =>
272+
driver.bulkUpdate('doc', [
273+
{ id: '1', data: { doc_no: 'D-0100' } },
274+
{ id: '2', data: { doc_no: 'D-0100' } }, // collides with the row above, not the table
275+
]),
276+
);
277+
const bulkDeleteErr = await refusalOf(() => driver.bulkDelete('doc', ['1', 'ghost']));
278+
279+
expect(createErr.code).toBe('UNIQUE_VIOLATION');
280+
expect(updateManyErr.code).toBe('UNIQUE_VIOLATION');
281+
expect(bulkUpdateErr.code).toBe('UNIQUE_VIOLATION');
282+
expect(bulkDeleteErr).toBeInstanceOf(Error);
283+
284+
// None of the four doors moved the store.
285+
expect(await snapshot(driver, 'doc')).toEqual(before);
286+
});
287+
});
288+
289+
describe('[#13435] non-regression — updateMany and bulkCreate still behave as #13197/#13340 left them', () => {
290+
let driver: InMemoryDriver;
291+
292+
beforeEach(async () => {
293+
driver = new InMemoryDriver();
294+
await driver.syncSchema('doc', DOC_SCHEMA);
295+
await driver.create('doc', { id: '1', doc_no: 'D-0001' });
296+
await driver.create('doc', { id: '2', doc_no: 'D-0002' });
297+
});
298+
299+
it('updateMany still refuses a colliding shared patch and leaves the table untouched', async () => {
300+
const before = await snapshot(driver, 'doc');
301+
const err = await refusalOf(() => driver.updateMany('doc', { where: {} }, { doc_no: 'D-SAME' }));
302+
expect(err.code).toBe('UNIQUE_VIOLATION');
303+
expect(await snapshot(driver, 'doc')).toEqual(before);
304+
});
305+
306+
it('updateMany still applies a clean shared patch to every matched row', async () => {
307+
const count = await driver.updateMany('doc', { where: {} }, { title: 'Bulk' });
308+
expect(count).toBe(2);
309+
expect((await driver.find('doc', { where: { id: '1' } }))[0].title).toBe('Bulk');
310+
expect((await driver.find('doc', { where: { id: '2' } }))[0].title).toBe('Bulk');
311+
});
312+
313+
it('bulkCreate still refuses a self-colliding batch and leaves the table untouched', async () => {
314+
const before = await snapshot(driver, 'doc');
315+
const err = await refusalOf(() =>
316+
driver.bulkCreate('doc', [
317+
{ id: 'a', doc_no: 'D-0100' },
318+
{ id: 'b', doc_no: 'D-0100' },
319+
]),
320+
);
321+
expect(err.code).toBe('UNIQUE_VIOLATION');
322+
expect(await snapshot(driver, 'doc')).toEqual(before);
323+
});
324+
325+
it('bulkCreate still lands a clean batch in full', async () => {
326+
const out = await driver.bulkCreate('doc', [{ id: 'a', doc_no: 'D-0100' }]);
327+
expect(out).toHaveLength(1);
328+
expect(await driver.count('doc')).toBe(3);
329+
});
330+
});

0 commit comments

Comments
 (0)