Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 29 additions & 0 deletions .changeset/driver-memory-bulkupdate-id-type-agreement.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
---
"@objectstack/driver-memory": patch
---

fix(driver-memory): `bulkUpdate`'s touched-row set now agrees with its own id resolution, so a mixed id-type batch is no longer false-refused (#13911)

`IDataDriver.bulkUpdate` declares `id: string | number`, and this driver
resolves an id to a row with a loose comparison — the way `update` and
`delete` always have — so naming a stored `1` as `'1'` finds the same row.
The all-or-nothing rework shipped one release earlier then built its
untouched-row set from the *caller's* ids using strict `Set` membership, so
for a mixed-type id the two lookups disagreed: the row was resolved and
updated, yet also stayed in the untouched set carrying its **pre-image**. It
faced the uniqueness check twice — once with the value it was vacating, once
with the value it was taking — and a batch that merely HANDS a unique value
from one row to another was refused with a false `UNIQUE_VIOLATION` / 409.

`bulkUpdate` now resolves every id to its table index first and derives the
touched set from the *resolved rows' own ids*, so both lookups read the same
stored value and cannot drift apart — the property the sibling `updateMany`
gets for free by drawing its target ids from table rows. The loose resolution
is deliberately preserved: tightening it would silently change which ids
resolve at all, a far wider behaviour change than this defect.

A genuine collision is still refused, and the stored id keeps its own type —
naming a row with a differently-typed id does not restamp it. `bulkDelete`
needed no change: it has exactly one id comparison and dedups on the resolved
table index rather than on caller input, so a mixed-type or repeated id
collapses to a single index by construction.
Original file line numberDiff line numberDiff line change
Expand Up@@ -328,3 +328,109 @@ describe('[#13435] non-regression — updateMany and bulkCreate still behave as
expect(await driver.count('doc')).toBe(3);
});
});

/**
* [#13911] The batch's two id lookups must AGREE.
*
* `IDataDriver.bulkUpdate` declares `id: string | number`, so a caller may
* legitimately name a row with an id whose JS type differs from the stored
* row's — and `update`/`bulkUpdate` resolve ids with a LOOSE `==` precisely so
* that `'1'` still finds stored `1`. The first cut of #13435 then built its
* untouched-row set (`settled`) from the CALLER's ids with a STRICT `Set.has`,
* so for a mixed-type id the two disagreed: `findIndex` resolved the row (it
* got updated) while `settled` still carried that row's PRE-image. The row sat
* in the projected check set twice — once stale, once pending — and a batch
* that merely MOVES a unique value between rows was refused with a false
* `UNIQUE_VIOLATION`.
*
* The sibling `updateMany` never had this gap: its `targetIds` come from table
* ROWS and its `findIndex` is strict `===`, so both sides agree by
* construction. The fix restores that property here the other way round —
* keeping the loose resolution (narrowing it would silently change which ids
* resolve at all) and drawing the touched set from the RESOLVED rows' own ids.
*
* ⛔ The discriminating fact is that a legitimate batch SUCCEEDS. A test that
* only asserted "a collision still refuses" would pass against the defect.
*/
describe('[#13911] caller id TYPE never changes the outcome of a batch', () => {
let driver: InMemoryDriver;

/** Numeric stored ids — the caller may still name them as strings. */
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' });
});

it('POSITIVE CONTROL: the same hand-off with CONSISTENT id types succeeds', async () => {
// Row 1 vacates D-0001; row 2 takes it. Nothing about this batch is
// unusual — it is here so the mixed-type case below cannot pass vacuously.
const out = await driver.bulkUpdate('doc', [
{ id: 1, data: { doc_no: 'D-0900' } },
{ id: 2, data: { doc_no: 'D-0001' } },
]);

expect(out).toHaveLength(2);
const rows = await snapshot(driver, 'doc');
expect(rows.map((r: any) => [r.id, r.doc_no])).toEqual([
[1, 'D-0900'],
[2, 'D-0001'],
]);
});

it('a STRING id naming a NUMERIC row still hands a unique value over cleanly', async () => {
// Identical to the control except the first id is a string. It resolves
// (loose `==`), so row 1 really does vacate D-0001 — and row 2 taking it
// must therefore NOT collide. Against the defect this threw a false
// UNIQUE_VIOLATION, because row 1's stale pre-image stayed in `settled`.
const out = await driver.bulkUpdate('doc', [
{ id: '1', data: { doc_no: 'D-0900' } },
{ id: 2, data: { doc_no: 'D-0001' } },
]);

expect(out).toHaveLength(2);
const rows = await snapshot(driver, 'doc');
expect(rows.map((r: any) => [r.id, r.doc_no])).toEqual([
[1, 'D-0900'],
[2, 'D-0001'],
]);
});

it('the stored id KEEPS its own type — a string id in the batch does not restamp it', async () => {
await driver.bulkUpdate('doc', [{ id: '1', data: { title: 'Renamed' } }]);

const row: any = (await driver.find('doc', { where: { id: 1 } }))[0];
expect(row.id).toBe(1);
expect(row.title).toBe('Renamed');
});

it('a REAL collision is still refused when the id types are mixed', async () => {
// The fix must not turn the check off: row 2 keeps D-0002, so row 1 taking
// it is a genuine violation however the caller spelled row 1's id.
const before = await snapshot(driver, 'doc');

const err = await refusalOf(() => driver.bulkUpdate('doc', [{ id: '1', data: { doc_no: 'D-0002' } }]));

expect(err.code).toBe('UNIQUE_VIOLATION');
expect(err.status).toBe(409);
expect(await snapshot(driver, 'doc')).toEqual(before);
});

it('bulkDelete: a mixed-type id removes exactly its own row', async () => {
await driver.bulkDelete('doc', ['1']);

const rows = await snapshot(driver, 'doc');
expect(rows.map((r: any) => r.id)).toEqual([2]);
});

it('bulkDelete: the SAME row named twice in two id types is removed once, and only it', async () => {
// `bulkDelete` dedups on the RESOLVED table index, not on the caller's id.
// Keying the set on caller input instead would make '1' and 1 two entries
// and splice index 0 twice — taking row 2 with it.
await driver.bulkDelete('doc', ['1', 1]);

const rows = await snapshot(driver, 'doc');
expect(rows.map((r: any) => [r.id, r.doc_no])).toEqual([[2, 'D-0002']]);
});
});
25 changes: 22 additions & 3 deletions packages/drivers/driver-memory/src/memory-driver.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -907,14 +907,33 @@ export class InMemoryDriver implements IDataDriver {
this.logger.debug('BulkUpdate operation', { object, count: updates.length });

const table = this.getTable(object);
const touchedIds = new Set(updates.map((u) => u.id));

// [#13911] Resolve every id to its table row FIRST, then draw the touched
// set from the RESOLVED rows' OWN ids — never from caller input. Ids are
// resolved with a loose `==` (matching `update`, one method up), but a
// `Set` membership test is always strict, so a caller naming a stored `1`
// as `'1'` — which `IDataDriver.bulkUpdate` explicitly allows, `id` being
// `string | number` — used to satisfy the resolving lookup while failing
// the `settled` one. That row was then updated AND left in `settled`
// carrying its PRE-image, so it faced the uniqueness check twice and a
// batch merely HANDING a unique value from one row to another was refused
// with a false `UNIQUE_VIOLATION`. Both lookups now read the same stored
// value, so they cannot disagree — the property `updateMany` gets for free
// by drawing its `targetIds` from table rows.
const resolvedIndexes = updates.map((u) => table.findIndex((r) => r.id == u.id));
const touchedIds = new Set(
resolvedIndexes.filter((index) => index !== -1).map((index) => table[index].id),
);
const settled = table.filter((r) => !touchedIds.has(r.id));

const perUpdate: Array<{ index: number; row: Record<string, any> } | null> = [];
const pending: Record<string, any>[] = [];

for (const u of updates) {
const index = table.findIndex((r) => r.id == u.id);
// Indexed rather than `for…of`, to read each id's ALREADY-resolved index:
// resolving a second time here is what let the two lookups drift apart.
for (let position = 0; position < updates.length; position++) {
const u = updates[position];
const index = resolvedIndexes[position];
if (index === -1) {
if (this.config.strictMode) {
this.logger.warn('Record not found for bulk update', { object, id: u.id });
Expand Down
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 29 additions & 0 deletions .changeset/driver-memory-bulkupdate-id-type-agreement.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
---
"@objectstack/driver-memory": patch
---

fix(driver-memory): `bulkUpdate`'s touched-row set now agrees with its own id resolution, so a mixed id-type batch is no longer false-refused (#13911)

`IDataDriver.bulkUpdate` declares `id: string | number`, and this driver
resolves an id to a row with a loose comparison — the way `update` and
`delete` always have — so naming a stored `1` as `'1'` finds the same row.
The all-or-nothing rework shipped one release earlier then built its
untouched-row set from the *caller's* ids using strict `Set` membership, so
for a mixed-type id the two lookups disagreed: the row was resolved and
updated, yet also stayed in the untouched set carrying its **pre-image**. It
faced the uniqueness check twice — once with the value it was vacating, once
with the value it was taking — and a batch that merely HANDS a unique value
from one row to another was refused with a false `UNIQUE_VIOLATION` / 409.

`bulkUpdate` now resolves every id to its table index first and derives the
touched set from the *resolved rows' own ids*, so both lookups read the same
stored value and cannot drift apart — the property the sibling `updateMany`
gets for free by drawing its target ids from table rows. The loose resolution
is deliberately preserved: tightening it would silently change which ids
resolve at all, a far wider behaviour change than this defect.

A genuine collision is still refused, and the stored id keeps its own type —
naming a row with a differently-typed id does not restamp it. `bulkDelete`
needed no change: it has exactly one id comparison and dedups on the resolved
table index rather than on caller input, so a mixed-type or repeated id
collapses to a single index by construction.
Original file line numberDiff line numberDiff line change
Expand Up@@ -328,3 +328,109 @@ describe('[#13435] non-regression — updateMany and bulkCreate still behave as
expect(await driver.count('doc')).toBe(3);
});
});

/**
* [#13911] The batch's two id lookups must AGREE.
*
* `IDataDriver.bulkUpdate` declares `id: string | number`, so a caller may
* legitimately name a row with an id whose JS type differs from the stored
* row's — and `update`/`bulkUpdate` resolve ids with a LOOSE `==` precisely so
* that `'1'` still finds stored `1`. The first cut of #13435 then built its
* untouched-row set (`settled`) from the CALLER's ids with a STRICT `Set.has`,
* so for a mixed-type id the two disagreed: `findIndex` resolved the row (it
* got updated) while `settled` still carried that row's PRE-image. The row sat
* in the projected check set twice — once stale, once pending — and a batch
* that merely MOVES a unique value between rows was refused with a false
* `UNIQUE_VIOLATION`.
*
* The sibling `updateMany` never had this gap: its `targetIds` come from table
* ROWS and its `findIndex` is strict `===`, so both sides agree by
* construction. The fix restores that property here the other way round —
* keeping the loose resolution (narrowing it would silently change which ids
* resolve at all) and drawing the touched set from the RESOLVED rows' own ids.
*
* ⛔ The discriminating fact is that a legitimate batch SUCCEEDS. A test that
* only asserted "a collision still refuses" would pass against the defect.
*/
describe('[#13911] caller id TYPE never changes the outcome of a batch', () => {
let driver: InMemoryDriver;

/** Numeric stored ids — the caller may still name them as strings. */
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' });
});

it('POSITIVE CONTROL: the same hand-off with CONSISTENT id types succeeds', async () => {
// Row 1 vacates D-0001; row 2 takes it. Nothing about this batch is
// unusual — it is here so the mixed-type case below cannot pass vacuously.
const out = await driver.bulkUpdate('doc', [
{ id: 1, data: { doc_no: 'D-0900' } },
{ id: 2, data: { doc_no: 'D-0001' } },
]);

expect(out).toHaveLength(2);
const rows = await snapshot(driver, 'doc');
expect(rows.map((r: any) => [r.id, r.doc_no])).toEqual([
[1, 'D-0900'],
[2, 'D-0001'],
]);
});

it('a STRING id naming a NUMERIC row still hands a unique value over cleanly', async () => {
// Identical to the control except the first id is a string. It resolves
// (loose `==`), so row 1 really does vacate D-0001 — and row 2 taking it
// must therefore NOT collide. Against the defect this threw a false
// UNIQUE_VIOLATION, because row 1's stale pre-image stayed in `settled`.
const out = await driver.bulkUpdate('doc', [
{ id: '1', data: { doc_no: 'D-0900' } },
{ id: 2, data: { doc_no: 'D-0001' } },
]);

expect(out).toHaveLength(2);
const rows = await snapshot(driver, 'doc');
expect(rows.map((r: any) => [r.id, r.doc_no])).toEqual([
[1, 'D-0900'],
[2, 'D-0001'],
]);
});

it('the stored id KEEPS its own type — a string id in the batch does not restamp it', async () => {
await driver.bulkUpdate('doc', [{ id: '1', data: { title: 'Renamed' } }]);

const row: any = (await driver.find('doc', { where: { id: 1 } }))[0];
expect(row.id).toBe(1);
expect(row.title).toBe('Renamed');
});

it('a REAL collision is still refused when the id types are mixed', async () => {
// The fix must not turn the check off: row 2 keeps D-0002, so row 1 taking
// it is a genuine violation however the caller spelled row 1's id.
const before = await snapshot(driver, 'doc');

const err = await refusalOf(() => driver.bulkUpdate('doc', [{ id: '1', data: { doc_no: 'D-0002' } }]));

expect(err.code).toBe('UNIQUE_VIOLATION');
expect(err.status).toBe(409);
expect(await snapshot(driver, 'doc')).toEqual(before);
});

it('bulkDelete: a mixed-type id removes exactly its own row', async () => {
await driver.bulkDelete('doc', ['1']);

const rows = await snapshot(driver, 'doc');
expect(rows.map((r: any) => r.id)).toEqual([2]);
});

it('bulkDelete: the SAME row named twice in two id types is removed once, and only it', async () => {
// `bulkDelete` dedups on the RESOLVED table index, not on the caller's id.
// Keying the set on caller input instead would make '1' and 1 two entries
// and splice index 0 twice — taking row 2 with it.
await driver.bulkDelete('doc', ['1', 1]);

const rows = await snapshot(driver, 'doc');
expect(rows.map((r: any) => [r.id, r.doc_no])).toEqual([[2, 'D-0002']]);
});
});
25 changes: 22 additions & 3 deletions packages/drivers/driver-memory/src/memory-driver.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -907,14 +907,33 @@ export class InMemoryDriver implements IDataDriver {
this.logger.debug('BulkUpdate operation', { object, count: updates.length });

const table = this.getTable(object);
const touchedIds = new Set(updates.map((u) => u.id));

// [#13911] Resolve every id to its table row FIRST, then draw the touched
// set from the RESOLVED rows' OWN ids — never from caller input. Ids are
// resolved with a loose `==` (matching `update`, one method up), but a
// `Set` membership test is always strict, so a caller naming a stored `1`
// as `'1'` — which `IDataDriver.bulkUpdate` explicitly allows, `id` being
// `string | number` — used to satisfy the resolving lookup while failing
// the `settled` one. That row was then updated AND left in `settled`
// carrying its PRE-image, so it faced the uniqueness check twice and a
// batch merely HANDING a unique value from one row to another was refused
// with a false `UNIQUE_VIOLATION`. Both lookups now read the same stored
// value, so they cannot disagree — the property `updateMany` gets for free
// by drawing its `targetIds` from table rows.
const resolvedIndexes = updates.map((u) => table.findIndex((r) => r.id == u.id));
const touchedIds = new Set(
resolvedIndexes.filter((index) => index !== -1).map((index) => table[index].id),
);
const settled = table.filter((r) => !touchedIds.has(r.id));

const perUpdate: Array<{ index: number; row: Record<string, any> } | null> = [];
const pending: Record<string, any>[] = [];

for (const u of updates) {
const index = table.findIndex((r) => r.id == u.id);
// Indexed rather than `for…of`, to read each id's ALREADY-resolved index:
// resolving a second time here is what let the two lookups drift apart.
for (let position = 0; position < updates.length; position++) {
const u = updates[position];
const index = resolvedIndexes[position];
if (index === -1) {
if (this.config.strictMode) {
this.logger.warn('Record not found for bulk update', { object, id: u.id });
Expand Down
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 29 additions & 0 deletions .changeset/driver-memory-bulkupdate-id-type-agreement.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
---
"@objectstack/driver-memory": patch
---

fix(driver-memory): `bulkUpdate`'s touched-row set now agrees with its own id resolution, so a mixed id-type batch is no longer false-refused (#13911)

`IDataDriver.bulkUpdate` declares `id: string | number`, and this driver
resolves an id to a row with a loose comparison — the way `update` and
`delete` always have — so naming a stored `1` as `'1'` finds the same row.
The all-or-nothing rework shipped one release earlier then built its
untouched-row set from the *caller's* ids using strict `Set` membership, so
for a mixed-type id the two lookups disagreed: the row was resolved and
updated, yet also stayed in the untouched set carrying its **pre-image**. It
faced the uniqueness check twice — once with the value it was vacating, once
with the value it was taking — and a batch that merely HANDS a unique value
from one row to another was refused with a false `UNIQUE_VIOLATION` / 409.

`bulkUpdate` now resolves every id to its table index first and derives the
touched set from the *resolved rows' own ids*, so both lookups read the same
stored value and cannot drift apart — the property the sibling `updateMany`
gets for free by drawing its target ids from table rows. The loose resolution
is deliberately preserved: tightening it would silently change which ids
resolve at all, a far wider behaviour change than this defect.

A genuine collision is still refused, and the stored id keeps its own type —
naming a row with a differently-typed id does not restamp it. `bulkDelete`
needed no change: it has exactly one id comparison and dedups on the resolved
table index rather than on caller input, so a mixed-type or repeated id
collapses to a single index by construction.
Original file line numberDiff line numberDiff line change
Expand Up@@ -328,3 +328,109 @@ describe('[#13435] non-regression — updateMany and bulkCreate still behave as
expect(await driver.count('doc')).toBe(3);
});
});

/**
* [#13911] The batch's two id lookups must AGREE.
*
* `IDataDriver.bulkUpdate` declares `id: string | number`, so a caller may
* legitimately name a row with an id whose JS type differs from the stored
* row's — and `update`/`bulkUpdate` resolve ids with a LOOSE `==` precisely so
* that `'1'` still finds stored `1`. The first cut of #13435 then built its
* untouched-row set (`settled`) from the CALLER's ids with a STRICT `Set.has`,
* so for a mixed-type id the two disagreed: `findIndex` resolved the row (it
* got updated) while `settled` still carried that row's PRE-image. The row sat
* in the projected check set twice — once stale, once pending — and a batch
* that merely MOVES a unique value between rows was refused with a false
* `UNIQUE_VIOLATION`.
*
* The sibling `updateMany` never had this gap: its `targetIds` come from table
* ROWS and its `findIndex` is strict `===`, so both sides agree by
* construction. The fix restores that property here the other way round —
* keeping the loose resolution (narrowing it would silently change which ids
* resolve at all) and drawing the touched set from the RESOLVED rows' own ids.
*
* ⛔ The discriminating fact is that a legitimate batch SUCCEEDS. A test that
* only asserted "a collision still refuses" would pass against the defect.
*/
describe('[#13911] caller id TYPE never changes the outcome of a batch', () => {
let driver: InMemoryDriver;

/** Numeric stored ids — the caller may still name them as strings. */
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' });
});

it('POSITIVE CONTROL: the same hand-off with CONSISTENT id types succeeds', async () => {
// Row 1 vacates D-0001; row 2 takes it. Nothing about this batch is
// unusual — it is here so the mixed-type case below cannot pass vacuously.
const out = await driver.bulkUpdate('doc', [
{ id: 1, data: { doc_no: 'D-0900' } },
{ id: 2, data: { doc_no: 'D-0001' } },
]);

expect(out).toHaveLength(2);
const rows = await snapshot(driver, 'doc');
expect(rows.map((r: any) => [r.id, r.doc_no])).toEqual([
[1, 'D-0900'],
[2, 'D-0001'],
]);
});

it('a STRING id naming a NUMERIC row still hands a unique value over cleanly', async () => {
// Identical to the control except the first id is a string. It resolves
// (loose `==`), so row 1 really does vacate D-0001 — and row 2 taking it
// must therefore NOT collide. Against the defect this threw a false
// UNIQUE_VIOLATION, because row 1's stale pre-image stayed in `settled`.
const out = await driver.bulkUpdate('doc', [
{ id: '1', data: { doc_no: 'D-0900' } },
{ id: 2, data: { doc_no: 'D-0001' } },
]);

expect(out).toHaveLength(2);
const rows = await snapshot(driver, 'doc');
expect(rows.map((r: any) => [r.id, r.doc_no])).toEqual([
[1, 'D-0900'],
[2, 'D-0001'],
]);
});

it('the stored id KEEPS its own type — a string id in the batch does not restamp it', async () => {
await driver.bulkUpdate('doc', [{ id: '1', data: { title: 'Renamed' } }]);

const row: any = (await driver.find('doc', { where: { id: 1 } }))[0];
expect(row.id).toBe(1);
expect(row.title).toBe('Renamed');
});

it('a REAL collision is still refused when the id types are mixed', async () => {
// The fix must not turn the check off: row 2 keeps D-0002, so row 1 taking
// it is a genuine violation however the caller spelled row 1's id.
const before = await snapshot(driver, 'doc');

const err = await refusalOf(() => driver.bulkUpdate('doc', [{ id: '1', data: { doc_no: 'D-0002' } }]));

expect(err.code).toBe('UNIQUE_VIOLATION');
expect(err.status).toBe(409);
expect(await snapshot(driver, 'doc')).toEqual(before);
});

it('bulkDelete: a mixed-type id removes exactly its own row', async () => {
await driver.bulkDelete('doc', ['1']);

const rows = await snapshot(driver, 'doc');
expect(rows.map((r: any) => r.id)).toEqual([2]);
});

it('bulkDelete: the SAME row named twice in two id types is removed once, and only it', async () => {
// `bulkDelete` dedups on the RESOLVED table index, not on the caller's id.
// Keying the set on caller input instead would make '1' and 1 two entries
// and splice index 0 twice — taking row 2 with it.
await driver.bulkDelete('doc', ['1', 1]);

const rows = await snapshot(driver, 'doc');
expect(rows.map((r: any) => [r.id, r.doc_no])).toEqual([[2, 'D-0002']]);
});
});
25 changes: 22 additions & 3 deletions packages/drivers/driver-memory/src/memory-driver.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -907,14 +907,33 @@ export class InMemoryDriver implements IDataDriver {
this.logger.debug('BulkUpdate operation', { object, count: updates.length });

const table = this.getTable(object);
const touchedIds = new Set(updates.map((u) => u.id));

// [#13911] Resolve every id to its table row FIRST, then draw the touched
// set from the RESOLVED rows' OWN ids — never from caller input. Ids are
// resolved with a loose `==` (matching `update`, one method up), but a
// `Set` membership test is always strict, so a caller naming a stored `1`
// as `'1'` — which `IDataDriver.bulkUpdate` explicitly allows, `id` being
// `string | number` — used to satisfy the resolving lookup while failing
// the `settled` one. That row was then updated AND left in `settled`
// carrying its PRE-image, so it faced the uniqueness check twice and a
// batch merely HANDING a unique value from one row to another was refused
// with a false `UNIQUE_VIOLATION`. Both lookups now read the same stored
// value, so they cannot disagree — the property `updateMany` gets for free
// by drawing its `targetIds` from table rows.
const resolvedIndexes = updates.map((u) => table.findIndex((r) => r.id == u.id));
const touchedIds = new Set(
resolvedIndexes.filter((index) => index !== -1).map((index) => table[index].id),
);
const settled = table.filter((r) => !touchedIds.has(r.id));

const perUpdate: Array<{ index: number; row: Record<string, any> } | null> = [];
const pending: Record<string, any>[] = [];

for (const u of updates) {
const index = table.findIndex((r) => r.id == u.id);
// Indexed rather than `for…of`, to read each id's ALREADY-resolved index:
// resolving a second time here is what let the two lookups drift apart.
for (let position = 0; position < updates.length; position++) {
const u = updates[position];
const index = resolvedIndexes[position];
if (index === -1) {
if (this.config.strictMode) {
this.logger.warn('Record not found for bulk update', { object, id: u.id });
Expand Down
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 29 additions & 0 deletions .changeset/driver-memory-bulkupdate-id-type-agreement.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
---
"@objectstack/driver-memory": patch
---

fix(driver-memory): `bulkUpdate`'s touched-row set now agrees with its own id resolution, so a mixed id-type batch is no longer false-refused (#13911)

`IDataDriver.bulkUpdate` declares `id: string | number`, and this driver
resolves an id to a row with a loose comparison — the way `update` and
`delete` always have — so naming a stored `1` as `'1'` finds the same row.
The all-or-nothing rework shipped one release earlier then built its
untouched-row set from the *caller's* ids using strict `Set` membership, so
for a mixed-type id the two lookups disagreed: the row was resolved and
updated, yet also stayed in the untouched set carrying its **pre-image**. It
faced the uniqueness check twice — once with the value it was vacating, once
with the value it was taking — and a batch that merely HANDS a unique value
from one row to another was refused with a false `UNIQUE_VIOLATION` / 409.

`bulkUpdate` now resolves every id to its table index first and derives the
touched set from the *resolved rows' own ids*, so both lookups read the same
stored value and cannot drift apart — the property the sibling `updateMany`
gets for free by drawing its target ids from table rows. The loose resolution
is deliberately preserved: tightening it would silently change which ids
resolve at all, a far wider behaviour change than this defect.

A genuine collision is still refused, and the stored id keeps its own type —
naming a row with a differently-typed id does not restamp it. `bulkDelete`
needed no change: it has exactly one id comparison and dedups on the resolved
table index rather than on caller input, so a mixed-type or repeated id
collapses to a single index by construction.
Original file line numberDiff line numberDiff line change
Expand Up@@ -328,3 +328,109 @@ describe('[#13435] non-regression — updateMany and bulkCreate still behave as
expect(await driver.count('doc')).toBe(3);
});
});

/**
* [#13911] The batch's two id lookups must AGREE.
*
* `IDataDriver.bulkUpdate` declares `id: string | number`, so a caller may
* legitimately name a row with an id whose JS type differs from the stored
* row's — and `update`/`bulkUpdate` resolve ids with a LOOSE `==` precisely so
* that `'1'` still finds stored `1`. The first cut of #13435 then built its
* untouched-row set (`settled`) from the CALLER's ids with a STRICT `Set.has`,
* so for a mixed-type id the two disagreed: `findIndex` resolved the row (it
* got updated) while `settled` still carried that row's PRE-image. The row sat
* in the projected check set twice — once stale, once pending — and a batch
* that merely MOVES a unique value between rows was refused with a false
* `UNIQUE_VIOLATION`.
*
* The sibling `updateMany` never had this gap: its `targetIds` come from table
* ROWS and its `findIndex` is strict `===`, so both sides agree by
* construction. The fix restores that property here the other way round —
* keeping the loose resolution (narrowing it would silently change which ids
* resolve at all) and drawing the touched set from the RESOLVED rows' own ids.
*
* ⛔ The discriminating fact is that a legitimate batch SUCCEEDS. A test that
* only asserted "a collision still refuses" would pass against the defect.
*/
describe('[#13911] caller id TYPE never changes the outcome of a batch', () => {
let driver: InMemoryDriver;

/** Numeric stored ids — the caller may still name them as strings. */
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' });
});

it('POSITIVE CONTROL: the same hand-off with CONSISTENT id types succeeds', async () => {
// Row 1 vacates D-0001; row 2 takes it. Nothing about this batch is
// unusual — it is here so the mixed-type case below cannot pass vacuously.
const out = await driver.bulkUpdate('doc', [
{ id: 1, data: { doc_no: 'D-0900' } },
{ id: 2, data: { doc_no: 'D-0001' } },
]);

expect(out).toHaveLength(2);
const rows = await snapshot(driver, 'doc');
expect(rows.map((r: any) => [r.id, r.doc_no])).toEqual([
[1, 'D-0900'],
[2, 'D-0001'],
]);
});

it('a STRING id naming a NUMERIC row still hands a unique value over cleanly', async () => {
// Identical to the control except the first id is a string. It resolves
// (loose `==`), so row 1 really does vacate D-0001 — and row 2 taking it
// must therefore NOT collide. Against the defect this threw a false
// UNIQUE_VIOLATION, because row 1's stale pre-image stayed in `settled`.
const out = await driver.bulkUpdate('doc', [
{ id: '1', data: { doc_no: 'D-0900' } },
{ id: 2, data: { doc_no: 'D-0001' } },
]);

expect(out).toHaveLength(2);
const rows = await snapshot(driver, 'doc');
expect(rows.map((r: any) => [r.id, r.doc_no])).toEqual([
[1, 'D-0900'],
[2, 'D-0001'],
]);
});

it('the stored id KEEPS its own type — a string id in the batch does not restamp it', async () => {
await driver.bulkUpdate('doc', [{ id: '1', data: { title: 'Renamed' } }]);

const row: any = (await driver.find('doc', { where: { id: 1 } }))[0];
expect(row.id).toBe(1);
expect(row.title).toBe('Renamed');
});

it('a REAL collision is still refused when the id types are mixed', async () => {
// The fix must not turn the check off: row 2 keeps D-0002, so row 1 taking
// it is a genuine violation however the caller spelled row 1's id.
const before = await snapshot(driver, 'doc');

const err = await refusalOf(() => driver.bulkUpdate('doc', [{ id: '1', data: { doc_no: 'D-0002' } }]));

expect(err.code).toBe('UNIQUE_VIOLATION');
expect(err.status).toBe(409);
expect(await snapshot(driver, 'doc')).toEqual(before);
});

it('bulkDelete: a mixed-type id removes exactly its own row', async () => {
await driver.bulkDelete('doc', ['1']);

const rows = await snapshot(driver, 'doc');
expect(rows.map((r: any) => r.id)).toEqual([2]);
});

it('bulkDelete: the SAME row named twice in two id types is removed once, and only it', async () => {
// `bulkDelete` dedups on the RESOLVED table index, not on the caller's id.
// Keying the set on caller input instead would make '1' and 1 two entries
// and splice index 0 twice — taking row 2 with it.
await driver.bulkDelete('doc', ['1', 1]);

const rows = await snapshot(driver, 'doc');
expect(rows.map((r: any) => [r.id, r.doc_no])).toEqual([[2, 'D-0002']]);
});
});
25 changes: 22 additions & 3 deletions packages/drivers/driver-memory/src/memory-driver.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -907,14 +907,33 @@ export class InMemoryDriver implements IDataDriver {
this.logger.debug('BulkUpdate operation', { object, count: updates.length });

const table = this.getTable(object);
const touchedIds = new Set(updates.map((u) => u.id));

// [#13911] Resolve every id to its table row FIRST, then draw the touched
// set from the RESOLVED rows' OWN ids — never from caller input. Ids are
// resolved with a loose `==` (matching `update`, one method up), but a
// `Set` membership test is always strict, so a caller naming a stored `1`
// as `'1'` — which `IDataDriver.bulkUpdate` explicitly allows, `id` being
// `string | number` — used to satisfy the resolving lookup while failing
// the `settled` one. That row was then updated AND left in `settled`
// carrying its PRE-image, so it faced the uniqueness check twice and a
// batch merely HANDING a unique value from one row to another was refused
// with a false `UNIQUE_VIOLATION`. Both lookups now read the same stored
// value, so they cannot disagree — the property `updateMany` gets for free
// by drawing its `targetIds` from table rows.
const resolvedIndexes = updates.map((u) => table.findIndex((r) => r.id == u.id));
const touchedIds = new Set(
resolvedIndexes.filter((index) => index !== -1).map((index) => table[index].id),
);
const settled = table.filter((r) => !touchedIds.has(r.id));

const perUpdate: Array<{ index: number; row: Record<string, any> } | null> = [];
const pending: Record<string, any>[] = [];

for (const u of updates) {
const index = table.findIndex((r) => r.id == u.id);
// Indexed rather than `for…of`, to read each id's ALREADY-resolved index:
// resolving a second time here is what let the two lookups drift apart.
for (let position = 0; position < updates.length; position++) {
const u = updates[position];
const index = resolvedIndexes[position];
if (index === -1) {
if (this.config.strictMode) {
this.logger.warn('Record not found for bulk update', { object, id: u.id });
Expand Down
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 29 additions & 0 deletions .changeset/driver-memory-bulkupdate-id-type-agreement.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
---
"@objectstack/driver-memory": patch
---

fix(driver-memory): `bulkUpdate`'s touched-row set now agrees with its own id resolution, so a mixed id-type batch is no longer false-refused (#13911)

`IDataDriver.bulkUpdate` declares `id: string | number`, and this driver
resolves an id to a row with a loose comparison — the way `update` and
`delete` always have — so naming a stored `1` as `'1'` finds the same row.
The all-or-nothing rework shipped one release earlier then built its
untouched-row set from the *caller's* ids using strict `Set` membership, so
for a mixed-type id the two lookups disagreed: the row was resolved and
updated, yet also stayed in the untouched set carrying its **pre-image**. It
faced the uniqueness check twice — once with the value it was vacating, once
with the value it was taking — and a batch that merely HANDS a unique value
from one row to another was refused with a false `UNIQUE_VIOLATION` / 409.

`bulkUpdate` now resolves every id to its table index first and derives the
touched set from the *resolved rows' own ids*, so both lookups read the same
stored value and cannot drift apart — the property the sibling `updateMany`
gets for free by drawing its target ids from table rows. The loose resolution
is deliberately preserved: tightening it would silently change which ids
resolve at all, a far wider behaviour change than this defect.

A genuine collision is still refused, and the stored id keeps its own type —
naming a row with a differently-typed id does not restamp it. `bulkDelete`
needed no change: it has exactly one id comparison and dedups on the resolved
table index rather than on caller input, so a mixed-type or repeated id
collapses to a single index by construction.
Original file line numberDiff line numberDiff line change
Expand Up@@ -328,3 +328,109 @@ describe('[#13435] non-regression — updateMany and bulkCreate still behave as
expect(await driver.count('doc')).toBe(3);
});
});

/**
* [#13911] The batch's two id lookups must AGREE.
*
* `IDataDriver.bulkUpdate` declares `id: string | number`, so a caller may
* legitimately name a row with an id whose JS type differs from the stored
* row's — and `update`/`bulkUpdate` resolve ids with a LOOSE `==` precisely so
* that `'1'` still finds stored `1`. The first cut of #13435 then built its
* untouched-row set (`settled`) from the CALLER's ids with a STRICT `Set.has`,
* so for a mixed-type id the two disagreed: `findIndex` resolved the row (it
* got updated) while `settled` still carried that row's PRE-image. The row sat
* in the projected check set twice — once stale, once pending — and a batch
* that merely MOVES a unique value between rows was refused with a false
* `UNIQUE_VIOLATION`.
*
* The sibling `updateMany` never had this gap: its `targetIds` come from table
* ROWS and its `findIndex` is strict `===`, so both sides agree by
* construction. The fix restores that property here the other way round —
* keeping the loose resolution (narrowing it would silently change which ids
* resolve at all) and drawing the touched set from the RESOLVED rows' own ids.
*
* ⛔ The discriminating fact is that a legitimate batch SUCCEEDS. A test that
* only asserted "a collision still refuses" would pass against the defect.
*/
describe('[#13911] caller id TYPE never changes the outcome of a batch', () => {
let driver: InMemoryDriver;

/** Numeric stored ids — the caller may still name them as strings. */
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' });
});

it('POSITIVE CONTROL: the same hand-off with CONSISTENT id types succeeds', async () => {
// Row 1 vacates D-0001; row 2 takes it. Nothing about this batch is
// unusual — it is here so the mixed-type case below cannot pass vacuously.
const out = await driver.bulkUpdate('doc', [
{ id: 1, data: { doc_no: 'D-0900' } },
{ id: 2, data: { doc_no: 'D-0001' } },
]);

expect(out).toHaveLength(2);
const rows = await snapshot(driver, 'doc');
expect(rows.map((r: any) => [r.id, r.doc_no])).toEqual([
[1, 'D-0900'],
[2, 'D-0001'],
]);
});

it('a STRING id naming a NUMERIC row still hands a unique value over cleanly', async () => {
// Identical to the control except the first id is a string. It resolves
// (loose `==`), so row 1 really does vacate D-0001 — and row 2 taking it
// must therefore NOT collide. Against the defect this threw a false
// UNIQUE_VIOLATION, because row 1's stale pre-image stayed in `settled`.
const out = await driver.bulkUpdate('doc', [
{ id: '1', data: { doc_no: 'D-0900' } },
{ id: 2, data: { doc_no: 'D-0001' } },
]);

expect(out).toHaveLength(2);
const rows = await snapshot(driver, 'doc');
expect(rows.map((r: any) => [r.id, r.doc_no])).toEqual([
[1, 'D-0900'],
[2, 'D-0001'],
]);
});

it('the stored id KEEPS its own type — a string id in the batch does not restamp it', async () => {
await driver.bulkUpdate('doc', [{ id: '1', data: { title: 'Renamed' } }]);

const row: any = (await driver.find('doc', { where: { id: 1 } }))[0];
expect(row.id).toBe(1);
expect(row.title).toBe('Renamed');
});

it('a REAL collision is still refused when the id types are mixed', async () => {
// The fix must not turn the check off: row 2 keeps D-0002, so row 1 taking
// it is a genuine violation however the caller spelled row 1's id.
const before = await snapshot(driver, 'doc');

const err = await refusalOf(() => driver.bulkUpdate('doc', [{ id: '1', data: { doc_no: 'D-0002' } }]));

expect(err.code).toBe('UNIQUE_VIOLATION');
expect(err.status).toBe(409);
expect(await snapshot(driver, 'doc')).toEqual(before);
});

it('bulkDelete: a mixed-type id removes exactly its own row', async () => {
await driver.bulkDelete('doc', ['1']);

const rows = await snapshot(driver, 'doc');
expect(rows.map((r: any) => r.id)).toEqual([2]);
});

it('bulkDelete: the SAME row named twice in two id types is removed once, and only it', async () => {
// `bulkDelete` dedups on the RESOLVED table index, not on the caller's id.
// Keying the set on caller input instead would make '1' and 1 two entries
// and splice index 0 twice — taking row 2 with it.
await driver.bulkDelete('doc', ['1', 1]);

const rows = await snapshot(driver, 'doc');
expect(rows.map((r: any) => [r.id, r.doc_no])).toEqual([[2, 'D-0002']]);
});
});
25 changes: 22 additions & 3 deletions packages/drivers/driver-memory/src/memory-driver.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -907,14 +907,33 @@ export class InMemoryDriver implements IDataDriver {
this.logger.debug('BulkUpdate operation', { object, count: updates.length });

const table = this.getTable(object);
const touchedIds = new Set(updates.map((u) => u.id));

// [#13911] Resolve every id to its table row FIRST, then draw the touched
// set from the RESOLVED rows' OWN ids — never from caller input. Ids are
// resolved with a loose `==` (matching `update`, one method up), but a
// `Set` membership test is always strict, so a caller naming a stored `1`
// as `'1'` — which `IDataDriver.bulkUpdate` explicitly allows, `id` being
// `string | number` — used to satisfy the resolving lookup while failing
// the `settled` one. That row was then updated AND left in `settled`
// carrying its PRE-image, so it faced the uniqueness check twice and a
// batch merely HANDING a unique value from one row to another was refused
// with a false `UNIQUE_VIOLATION`. Both lookups now read the same stored
// value, so they cannot disagree — the property `updateMany` gets for free
// by drawing its `targetIds` from table rows.
const resolvedIndexes = updates.map((u) => table.findIndex((r) => r.id == u.id));
const touchedIds = new Set(
resolvedIndexes.filter((index) => index !== -1).map((index) => table[index].id),
);
const settled = table.filter((r) => !touchedIds.has(r.id));

const perUpdate: Array<{ index: number; row: Record<string, any> } | null> = [];
const pending: Record<string, any>[] = [];

for (const u of updates) {
const index = table.findIndex((r) => r.id == u.id);
// Indexed rather than `for…of`, to read each id's ALREADY-resolved index:
// resolving a second time here is what let the two lookups drift apart.
for (let position = 0; position < updates.length; position++) {
const u = updates[position];
const index = resolvedIndexes[position];
if (index === -1) {
if (this.config.strictMode) {
this.logger.warn('Record not found for bulk update', { object, id: u.id });
Expand Down
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 29 additions & 0 deletions .changeset/driver-memory-bulkupdate-id-type-agreement.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
---
"@objectstack/driver-memory": patch
---

fix(driver-memory): `bulkUpdate`'s touched-row set now agrees with its own id resolution, so a mixed id-type batch is no longer false-refused (#13911)

`IDataDriver.bulkUpdate` declares `id: string | number`, and this driver
resolves an id to a row with a loose comparison — the way `update` and
`delete` always have — so naming a stored `1` as `'1'` finds the same row.
The all-or-nothing rework shipped one release earlier then built its
untouched-row set from the *caller's* ids using strict `Set` membership, so
for a mixed-type id the two lookups disagreed: the row was resolved and
updated, yet also stayed in the untouched set carrying its **pre-image**. It
faced the uniqueness check twice — once with the value it was vacating, once
with the value it was taking — and a batch that merely HANDS a unique value
from one row to another was refused with a false `UNIQUE_VIOLATION` / 409.

`bulkUpdate` now resolves every id to its table index first and derives the
touched set from the *resolved rows' own ids*, so both lookups read the same
stored value and cannot drift apart — the property the sibling `updateMany`
gets for free by drawing its target ids from table rows. The loose resolution
is deliberately preserved: tightening it would silently change which ids
resolve at all, a far wider behaviour change than this defect.

A genuine collision is still refused, and the stored id keeps its own type —
naming a row with a differently-typed id does not restamp it. `bulkDelete`
needed no change: it has exactly one id comparison and dedups on the resolved
table index rather than on caller input, so a mixed-type or repeated id
collapses to a single index by construction.
Original file line numberDiff line numberDiff line change
Expand Up@@ -328,3 +328,109 @@ describe('[#13435] non-regression — updateMany and bulkCreate still behave as
expect(await driver.count('doc')).toBe(3);
});
});

/**
* [#13911] The batch's two id lookups must AGREE.
*
* `IDataDriver.bulkUpdate` declares `id: string | number`, so a caller may
* legitimately name a row with an id whose JS type differs from the stored
* row's — and `update`/`bulkUpdate` resolve ids with a LOOSE `==` precisely so
* that `'1'` still finds stored `1`. The first cut of #13435 then built its
* untouched-row set (`settled`) from the CALLER's ids with a STRICT `Set.has`,
* so for a mixed-type id the two disagreed: `findIndex` resolved the row (it
* got updated) while `settled` still carried that row's PRE-image. The row sat
* in the projected check set twice — once stale, once pending — and a batch
* that merely MOVES a unique value between rows was refused with a false
* `UNIQUE_VIOLATION`.
*
* The sibling `updateMany` never had this gap: its `targetIds` come from table
* ROWS and its `findIndex` is strict `===`, so both sides agree by
* construction. The fix restores that property here the other way round —
* keeping the loose resolution (narrowing it would silently change which ids
* resolve at all) and drawing the touched set from the RESOLVED rows' own ids.
*
* ⛔ The discriminating fact is that a legitimate batch SUCCEEDS. A test that
* only asserted "a collision still refuses" would pass against the defect.
*/
describe('[#13911] caller id TYPE never changes the outcome of a batch', () => {
let driver: InMemoryDriver;

/** Numeric stored ids — the caller may still name them as strings. */
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' });
});

it('POSITIVE CONTROL: the same hand-off with CONSISTENT id types succeeds', async () => {
// Row 1 vacates D-0001; row 2 takes it. Nothing about this batch is
// unusual — it is here so the mixed-type case below cannot pass vacuously.
const out = await driver.bulkUpdate('doc', [
{ id: 1, data: { doc_no: 'D-0900' } },
{ id: 2, data: { doc_no: 'D-0001' } },
]);

expect(out).toHaveLength(2);
const rows = await snapshot(driver, 'doc');
expect(rows.map((r: any) => [r.id, r.doc_no])).toEqual([
[1, 'D-0900'],
[2, 'D-0001'],
]);
});

it('a STRING id naming a NUMERIC row still hands a unique value over cleanly', async () => {
// Identical to the control except the first id is a string. It resolves
// (loose `==`), so row 1 really does vacate D-0001 — and row 2 taking it
// must therefore NOT collide. Against the defect this threw a false
// UNIQUE_VIOLATION, because row 1's stale pre-image stayed in `settled`.
const out = await driver.bulkUpdate('doc', [
{ id: '1', data: { doc_no: 'D-0900' } },
{ id: 2, data: { doc_no: 'D-0001' } },
]);

expect(out).toHaveLength(2);
const rows = await snapshot(driver, 'doc');
expect(rows.map((r: any) => [r.id, r.doc_no])).toEqual([
[1, 'D-0900'],
[2, 'D-0001'],
]);
});

it('the stored id KEEPS its own type — a string id in the batch does not restamp it', async () => {
await driver.bulkUpdate('doc', [{ id: '1', data: { title: 'Renamed' } }]);

const row: any = (await driver.find('doc', { where: { id: 1 } }))[0];
expect(row.id).toBe(1);
expect(row.title).toBe('Renamed');
});

it('a REAL collision is still refused when the id types are mixed', async () => {
// The fix must not turn the check off: row 2 keeps D-0002, so row 1 taking
// it is a genuine violation however the caller spelled row 1's id.
const before = await snapshot(driver, 'doc');

const err = await refusalOf(() => driver.bulkUpdate('doc', [{ id: '1', data: { doc_no: 'D-0002' } }]));

expect(err.code).toBe('UNIQUE_VIOLATION');
expect(err.status).toBe(409);
expect(await snapshot(driver, 'doc')).toEqual(before);
});

it('bulkDelete: a mixed-type id removes exactly its own row', async () => {
await driver.bulkDelete('doc', ['1']);

const rows = await snapshot(driver, 'doc');
expect(rows.map((r: any) => r.id)).toEqual([2]);
});

it('bulkDelete: the SAME row named twice in two id types is removed once, and only it', async () => {
// `bulkDelete` dedups on the RESOLVED table index, not on the caller's id.
// Keying the set on caller input instead would make '1' and 1 two entries
// and splice index 0 twice — taking row 2 with it.
await driver.bulkDelete('doc', ['1', 1]);

const rows = await snapshot(driver, 'doc');
expect(rows.map((r: any) => [r.id, r.doc_no])).toEqual([[2, 'D-0002']]);
});
});
25 changes: 22 additions & 3 deletions packages/drivers/driver-memory/src/memory-driver.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -907,14 +907,33 @@ export class InMemoryDriver implements IDataDriver {
this.logger.debug('BulkUpdate operation', { object, count: updates.length });

const table = this.getTable(object);
const touchedIds = new Set(updates.map((u) => u.id));

// [#13911] Resolve every id to its table row FIRST, then draw the touched
// set from the RESOLVED rows' OWN ids — never from caller input. Ids are
// resolved with a loose `==` (matching `update`, one method up), but a
// `Set` membership test is always strict, so a caller naming a stored `1`
// as `'1'` — which `IDataDriver.bulkUpdate` explicitly allows, `id` being
// `string | number` — used to satisfy the resolving lookup while failing
// the `settled` one. That row was then updated AND left in `settled`
// carrying its PRE-image, so it faced the uniqueness check twice and a
// batch merely HANDING a unique value from one row to another was refused
// with a false `UNIQUE_VIOLATION`. Both lookups now read the same stored
// value, so they cannot disagree — the property `updateMany` gets for free
// by drawing its `targetIds` from table rows.
const resolvedIndexes = updates.map((u) => table.findIndex((r) => r.id == u.id));
const touchedIds = new Set(
resolvedIndexes.filter((index) => index !== -1).map((index) => table[index].id),
);
const settled = table.filter((r) => !touchedIds.has(r.id));

const perUpdate: Array<{ index: number; row: Record<string, any> } | null> = [];
const pending: Record<string, any>[] = [];

for (const u of updates) {
const index = table.findIndex((r) => r.id == u.id);
// Indexed rather than `for…of`, to read each id's ALREADY-resolved index:
// resolving a second time here is what let the two lookups drift apart.
for (let position = 0; position < updates.length; position++) {
const u = updates[position];
const index = resolvedIndexes[position];
if (index === -1) {
if (this.config.strictMode) {
this.logger.warn('Record not found for bulk update', { object, id: u.id });
Expand Down
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 29 additions & 0 deletions .changeset/driver-memory-bulkupdate-id-type-agreement.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
---
"@objectstack/driver-memory": patch
---

fix(driver-memory): `bulkUpdate`'s touched-row set now agrees with its own id resolution, so a mixed id-type batch is no longer false-refused (#13911)

`IDataDriver.bulkUpdate` declares `id: string | number`, and this driver
resolves an id to a row with a loose comparison — the way `update` and
`delete` always have — so naming a stored `1` as `'1'` finds the same row.
The all-or-nothing rework shipped one release earlier then built its
untouched-row set from the *caller's* ids using strict `Set` membership, so
for a mixed-type id the two lookups disagreed: the row was resolved and
updated, yet also stayed in the untouched set carrying its **pre-image**. It
faced the uniqueness check twice — once with the value it was vacating, once
with the value it was taking — and a batch that merely HANDS a unique value
from one row to another was refused with a false `UNIQUE_VIOLATION` / 409.

`bulkUpdate` now resolves every id to its table index first and derives the
touched set from the *resolved rows' own ids*, so both lookups read the same
stored value and cannot drift apart — the property the sibling `updateMany`
gets for free by drawing its target ids from table rows. The loose resolution
is deliberately preserved: tightening it would silently change which ids
resolve at all, a far wider behaviour change than this defect.

A genuine collision is still refused, and the stored id keeps its own type —
naming a row with a differently-typed id does not restamp it. `bulkDelete`
needed no change: it has exactly one id comparison and dedups on the resolved
table index rather than on caller input, so a mixed-type or repeated id
collapses to a single index by construction.
Original file line numberDiff line numberDiff line change
Expand Up@@ -328,3 +328,109 @@ describe('[#13435] non-regression — updateMany and bulkCreate still behave as
expect(await driver.count('doc')).toBe(3);
});
});

/**
* [#13911] The batch's two id lookups must AGREE.
*
* `IDataDriver.bulkUpdate` declares `id: string | number`, so a caller may
* legitimately name a row with an id whose JS type differs from the stored
* row's — and `update`/`bulkUpdate` resolve ids with a LOOSE `==` precisely so
* that `'1'` still finds stored `1`. The first cut of #13435 then built its
* untouched-row set (`settled`) from the CALLER's ids with a STRICT `Set.has`,
* so for a mixed-type id the two disagreed: `findIndex` resolved the row (it
* got updated) while `settled` still carried that row's PRE-image. The row sat
* in the projected check set twice — once stale, once pending — and a batch
* that merely MOVES a unique value between rows was refused with a false
* `UNIQUE_VIOLATION`.
*
* The sibling `updateMany` never had this gap: its `targetIds` come from table
* ROWS and its `findIndex` is strict `===`, so both sides agree by
* construction. The fix restores that property here the other way round —
* keeping the loose resolution (narrowing it would silently change which ids
* resolve at all) and drawing the touched set from the RESOLVED rows' own ids.
*
* ⛔ The discriminating fact is that a legitimate batch SUCCEEDS. A test that
* only asserted "a collision still refuses" would pass against the defect.
*/
describe('[#13911] caller id TYPE never changes the outcome of a batch', () => {
let driver: InMemoryDriver;

/** Numeric stored ids — the caller may still name them as strings. */
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' });
});

it('POSITIVE CONTROL: the same hand-off with CONSISTENT id types succeeds', async () => {
// Row 1 vacates D-0001; row 2 takes it. Nothing about this batch is
// unusual — it is here so the mixed-type case below cannot pass vacuously.
const out = await driver.bulkUpdate('doc', [
{ id: 1, data: { doc_no: 'D-0900' } },
{ id: 2, data: { doc_no: 'D-0001' } },
]);

expect(out).toHaveLength(2);
const rows = await snapshot(driver, 'doc');
expect(rows.map((r: any) => [r.id, r.doc_no])).toEqual([
[1, 'D-0900'],
[2, 'D-0001'],
]);
});

it('a STRING id naming a NUMERIC row still hands a unique value over cleanly', async () => {
// Identical to the control except the first id is a string. It resolves
// (loose `==`), so row 1 really does vacate D-0001 — and row 2 taking it
// must therefore NOT collide. Against the defect this threw a false
// UNIQUE_VIOLATION, because row 1's stale pre-image stayed in `settled`.
const out = await driver.bulkUpdate('doc', [
{ id: '1', data: { doc_no: 'D-0900' } },
{ id: 2, data: { doc_no: 'D-0001' } },
]);

expect(out).toHaveLength(2);
const rows = await snapshot(driver, 'doc');
expect(rows.map((r: any) => [r.id, r.doc_no])).toEqual([
[1, 'D-0900'],
[2, 'D-0001'],
]);
});

it('the stored id KEEPS its own type — a string id in the batch does not restamp it', async () => {
await driver.bulkUpdate('doc', [{ id: '1', data: { title: 'Renamed' } }]);

const row: any = (await driver.find('doc', { where: { id: 1 } }))[0];
expect(row.id).toBe(1);
expect(row.title).toBe('Renamed');
});

it('a REAL collision is still refused when the id types are mixed', async () => {
// The fix must not turn the check off: row 2 keeps D-0002, so row 1 taking
// it is a genuine violation however the caller spelled row 1's id.
const before = await snapshot(driver, 'doc');

const err = await refusalOf(() => driver.bulkUpdate('doc', [{ id: '1', data: { doc_no: 'D-0002' } }]));

expect(err.code).toBe('UNIQUE_VIOLATION');
expect(err.status).toBe(409);
expect(await snapshot(driver, 'doc')).toEqual(before);
});

it('bulkDelete: a mixed-type id removes exactly its own row', async () => {
await driver.bulkDelete('doc', ['1']);

const rows = await snapshot(driver, 'doc');
expect(rows.map((r: any) => r.id)).toEqual([2]);
});

it('bulkDelete: the SAME row named twice in two id types is removed once, and only it', async () => {
// `bulkDelete` dedups on the RESOLVED table index, not on the caller's id.
// Keying the set on caller input instead would make '1' and 1 two entries
// and splice index 0 twice — taking row 2 with it.
await driver.bulkDelete('doc', ['1', 1]);

const rows = await snapshot(driver, 'doc');
expect(rows.map((r: any) => [r.id, r.doc_no])).toEqual([[2, 'D-0002']]);
});
});
25 changes: 22 additions & 3 deletions packages/drivers/driver-memory/src/memory-driver.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -907,14 +907,33 @@ export class InMemoryDriver implements IDataDriver {
this.logger.debug('BulkUpdate operation', { object, count: updates.length });

const table = this.getTable(object);
const touchedIds = new Set(updates.map((u) => u.id));

// [#13911] Resolve every id to its table row FIRST, then draw the touched
// set from the RESOLVED rows' OWN ids — never from caller input. Ids are
// resolved with a loose `==` (matching `update`, one method up), but a
// `Set` membership test is always strict, so a caller naming a stored `1`
// as `'1'` — which `IDataDriver.bulkUpdate` explicitly allows, `id` being
// `string | number` — used to satisfy the resolving lookup while failing
// the `settled` one. That row was then updated AND left in `settled`
// carrying its PRE-image, so it faced the uniqueness check twice and a
// batch merely HANDING a unique value from one row to another was refused
// with a false `UNIQUE_VIOLATION`. Both lookups now read the same stored
// value, so they cannot disagree — the property `updateMany` gets for free
// by drawing its `targetIds` from table rows.
const resolvedIndexes = updates.map((u) => table.findIndex((r) => r.id == u.id));
const touchedIds = new Set(
resolvedIndexes.filter((index) => index !== -1).map((index) => table[index].id),
);
const settled = table.filter((r) => !touchedIds.has(r.id));

const perUpdate: Array<{ index: number; row: Record<string, any> } | null> = [];
const pending: Record<string, any>[] = [];

for (const u of updates) {
const index = table.findIndex((r) => r.id == u.id);
// Indexed rather than `for…of`, to read each id's ALREADY-resolved index:
// resolving a second time here is what let the two lookups drift apart.
for (let position = 0; position < updates.length; position++) {
const u = updates[position];
const index = resolvedIndexes[position];
if (index === -1) {
if (this.config.strictMode) {
this.logger.warn('Record not found for bulk update', { object, id: u.id });
Expand Down
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 29 additions & 0 deletions .changeset/driver-memory-bulkupdate-id-type-agreement.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
---
"@objectstack/driver-memory": patch
---

fix(driver-memory): `bulkUpdate`'s touched-row set now agrees with its own id resolution, so a mixed id-type batch is no longer false-refused (#13911)

`IDataDriver.bulkUpdate` declares `id: string | number`, and this driver
resolves an id to a row with a loose comparison — the way `update` and
`delete` always have — so naming a stored `1` as `'1'` finds the same row.
The all-or-nothing rework shipped one release earlier then built its
untouched-row set from the *caller's* ids using strict `Set` membership, so
for a mixed-type id the two lookups disagreed: the row was resolved and
updated, yet also stayed in the untouched set carrying its **pre-image**. It
faced the uniqueness check twice — once with the value it was vacating, once
with the value it was taking — and a batch that merely HANDS a unique value
from one row to another was refused with a false `UNIQUE_VIOLATION` / 409.

`bulkUpdate` now resolves every id to its table index first and derives the
touched set from the *resolved rows' own ids*, so both lookups read the same
stored value and cannot drift apart — the property the sibling `updateMany`
gets for free by drawing its target ids from table rows. The loose resolution
is deliberately preserved: tightening it would silently change which ids
resolve at all, a far wider behaviour change than this defect.

A genuine collision is still refused, and the stored id keeps its own type —
naming a row with a differently-typed id does not restamp it. `bulkDelete`
needed no change: it has exactly one id comparison and dedups on the resolved
table index rather than on caller input, so a mixed-type or repeated id
collapses to a single index by construction.
Original file line numberDiff line numberDiff line change
Expand Up@@ -328,3 +328,109 @@ describe('[#13435] non-regression — updateMany and bulkCreate still behave as
expect(await driver.count('doc')).toBe(3);
});
});

/**
* [#13911] The batch's two id lookups must AGREE.
*
* `IDataDriver.bulkUpdate` declares `id: string | number`, so a caller may
* legitimately name a row with an id whose JS type differs from the stored
* row's — and `update`/`bulkUpdate` resolve ids with a LOOSE `==` precisely so
* that `'1'` still finds stored `1`. The first cut of #13435 then built its
* untouched-row set (`settled`) from the CALLER's ids with a STRICT `Set.has`,
* so for a mixed-type id the two disagreed: `findIndex` resolved the row (it
* got updated) while `settled` still carried that row's PRE-image. The row sat
* in the projected check set twice — once stale, once pending — and a batch
* that merely MOVES a unique value between rows was refused with a false
* `UNIQUE_VIOLATION`.
*
* The sibling `updateMany` never had this gap: its `targetIds` come from table
* ROWS and its `findIndex` is strict `===`, so both sides agree by
* construction. The fix restores that property here the other way round —
* keeping the loose resolution (narrowing it would silently change which ids
* resolve at all) and drawing the touched set from the RESOLVED rows' own ids.
*
* ⛔ The discriminating fact is that a legitimate batch SUCCEEDS. A test that
* only asserted "a collision still refuses" would pass against the defect.
*/
describe('[#13911] caller id TYPE never changes the outcome of a batch', () => {
let driver: InMemoryDriver;

/** Numeric stored ids — the caller may still name them as strings. */
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' });
});

it('POSITIVE CONTROL: the same hand-off with CONSISTENT id types succeeds', async () => {
// Row 1 vacates D-0001; row 2 takes it. Nothing about this batch is
// unusual — it is here so the mixed-type case below cannot pass vacuously.
const out = await driver.bulkUpdate('doc', [
{ id: 1, data: { doc_no: 'D-0900' } },
{ id: 2, data: { doc_no: 'D-0001' } },
]);

expect(out).toHaveLength(2);
const rows = await snapshot(driver, 'doc');
expect(rows.map((r: any) => [r.id, r.doc_no])).toEqual([
[1, 'D-0900'],
[2, 'D-0001'],
]);
});

it('a STRING id naming a NUMERIC row still hands a unique value over cleanly', async () => {
// Identical to the control except the first id is a string. It resolves
// (loose `==`), so row 1 really does vacate D-0001 — and row 2 taking it
// must therefore NOT collide. Against the defect this threw a false
// UNIQUE_VIOLATION, because row 1's stale pre-image stayed in `settled`.
const out = await driver.bulkUpdate('doc', [
{ id: '1', data: { doc_no: 'D-0900' } },
{ id: 2, data: { doc_no: 'D-0001' } },
]);

expect(out).toHaveLength(2);
const rows = await snapshot(driver, 'doc');
expect(rows.map((r: any) => [r.id, r.doc_no])).toEqual([
[1, 'D-0900'],
[2, 'D-0001'],
]);
});

it('the stored id KEEPS its own type — a string id in the batch does not restamp it', async () => {
await driver.bulkUpdate('doc', [{ id: '1', data: { title: 'Renamed' } }]);

const row: any = (await driver.find('doc', { where: { id: 1 } }))[0];
expect(row.id).toBe(1);
expect(row.title).toBe('Renamed');
});

it('a REAL collision is still refused when the id types are mixed', async () => {
// The fix must not turn the check off: row 2 keeps D-0002, so row 1 taking
// it is a genuine violation however the caller spelled row 1's id.
const before = await snapshot(driver, 'doc');

const err = await refusalOf(() => driver.bulkUpdate('doc', [{ id: '1', data: { doc_no: 'D-0002' } }]));

expect(err.code).toBe('UNIQUE_VIOLATION');
expect(err.status).toBe(409);
expect(await snapshot(driver, 'doc')).toEqual(before);
});

it('bulkDelete: a mixed-type id removes exactly its own row', async () => {
await driver.bulkDelete('doc', ['1']);

const rows = await snapshot(driver, 'doc');
expect(rows.map((r: any) => r.id)).toEqual([2]);
});

it('bulkDelete: the SAME row named twice in two id types is removed once, and only it', async () => {
// `bulkDelete` dedups on the RESOLVED table index, not on the caller's id.
// Keying the set on caller input instead would make '1' and 1 two entries
// and splice index 0 twice — taking row 2 with it.
await driver.bulkDelete('doc', ['1', 1]);

const rows = await snapshot(driver, 'doc');
expect(rows.map((r: any) => [r.id, r.doc_no])).toEqual([[2, 'D-0002']]);
});
});
25 changes: 22 additions & 3 deletions packages/drivers/driver-memory/src/memory-driver.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -907,14 +907,33 @@ export class InMemoryDriver implements IDataDriver {
this.logger.debug('BulkUpdate operation', { object, count: updates.length });

const table = this.getTable(object);
const touchedIds = new Set(updates.map((u) => u.id));

// [#13911] Resolve every id to its table row FIRST, then draw the touched
// set from the RESOLVED rows' OWN ids — never from caller input. Ids are
// resolved with a loose `==` (matching `update`, one method up), but a
// `Set` membership test is always strict, so a caller naming a stored `1`
// as `'1'` — which `IDataDriver.bulkUpdate` explicitly allows, `id` being
// `string | number` — used to satisfy the resolving lookup while failing
// the `settled` one. That row was then updated AND left in `settled`
// carrying its PRE-image, so it faced the uniqueness check twice and a
// batch merely HANDING a unique value from one row to another was refused
// with a false `UNIQUE_VIOLATION`. Both lookups now read the same stored
// value, so they cannot disagree — the property `updateMany` gets for free
// by drawing its `targetIds` from table rows.
const resolvedIndexes = updates.map((u) => table.findIndex((r) => r.id == u.id));
const touchedIds = new Set(
resolvedIndexes.filter((index) => index !== -1).map((index) => table[index].id),
);
const settled = table.filter((r) => !touchedIds.has(r.id));

const perUpdate: Array<{ index: number; row: Record<string, any> } | null> = [];
const pending: Record<string, any>[] = [];

for (const u of updates) {
const index = table.findIndex((r) => r.id == u.id);
// Indexed rather than `for…of`, to read each id's ALREADY-resolved index:
// resolving a second time here is what let the two lookups drift apart.
for (let position = 0; position < updates.length; position++) {
const u = updates[position];
const index = resolvedIndexes[position];
if (index === -1) {
if (this.config.strictMode) {
this.logger.warn('Record not found for bulk update', { object, id: u.id });
Expand Down
Loading