Skip to content

Commit d9e0f51

Browse files
committed
fix(objectql): reject short/non-array bulkCreate driver results (#3151)
engine.insert(array) built afterInsert contexts by index over rowHookContexts, so a driver bulkCreate that returned fewer records than input rows padded the tail with `undefined` — afterInsert hooks ran with `ctx.result === undefined` and the engine returned undefined entries, corrupting seed externalId→id maps and import undo logs downstream. Guard the driver result before the afterInsert loop: a batch whose result is not an array of exactly rows.length records now throws ERR_BULK_RESULT_MISMATCH, so the caller sees a real failure. Every in-repo driver already returns one-per-row in order; this defends against third-party drivers. The single-record path is untouched. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01APnSDJneEDRh3Z51KGCLga
1 parent 3234216 commit d9e0f51

2 files changed

Lines changed: 113 additions & 0 deletions

File tree

Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,94 @@
1+
// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.
2+
//
3+
// framework#3151 (engine layer): engine.insert(array) must not fabricate
4+
// afterInsert contexts (or caller results) when the driver's bulkCreate
5+
// returns the wrong number of records. A short / non-array return is refused
6+
// with ERR_BULK_RESULT_MISMATCH rather than padded with undefined.
7+
8+
import { describe, it, expect } from 'vitest';
9+
import { ObjectQL } from './engine.js';
10+
11+
function makeDriver(opts: { bulkCreate?: (object: string, rows: any[]) => Promise<any> } = {}) {
12+
const stores = new Map<string, Map<string, any>>();
13+
const storeFor = (o: string) => {
14+
let s = stores.get(o);
15+
if (!s) { s = new Map(); stores.set(o, s); }
16+
return s;
17+
};
18+
let n = 0;
19+
const driver: any = {
20+
name: 'memory', version: '0.0.0', supports: {},
21+
async connect() {}, async disconnect() {}, async checkHealth() { return true; }, async execute() { return null; },
22+
async find(object: string) { return Array.from(storeFor(object).values()); },
23+
findStream() { throw new Error('ni'); },
24+
async findOne() { return null; },
25+
async create(object: string, data: Record<string, unknown>) {
26+
n += 1;
27+
const id = (data.id as string) ?? `r_${n}`;
28+
const row = { ...data, id };
29+
storeFor(object).set(id, row);
30+
return row;
31+
},
32+
async update(object: string, id: string, data: Record<string, unknown>) {
33+
const s = storeFor(object);
34+
const row = { ...s.get(id), ...data, id };
35+
s.set(id, row);
36+
return row;
37+
},
38+
async delete(object: string, id: string) { return storeFor(object).delete(id); },
39+
async count() { return 0; },
40+
async bulkCreate(object: string, rows: Record<string, unknown>[]) {
41+
if (opts.bulkCreate) return opts.bulkCreate(object, rows);
42+
const out: any[] = [];
43+
for (const r of rows) out.push(await this.create(object, r));
44+
return out;
45+
},
46+
async bulkUpdate() { return []; }, async bulkDelete() {},
47+
async beginTransaction() { return { __trx: true, commit: async () => {}, rollback: async () => {} }; },
48+
async commit() {}, async rollback() {},
49+
};
50+
return { driver, storeFor };
51+
}
52+
53+
async function makeEngine(driverOpts?: { bulkCreate?: (object: string, rows: any[]) => Promise<any> }) {
54+
const engine = new ObjectQL();
55+
const d = makeDriver(driverOpts);
56+
engine.registerDriver(d.driver, true);
57+
await engine.init();
58+
engine.registry.registerObject({ name: 'task', fields: { title: { type: 'text' } } } as any);
59+
return { engine, storeFor: d.storeFor };
60+
}
61+
62+
describe('engine.insert(array) — driver result contract (framework#3151)', () => {
63+
it('rejects a short bulkCreate return and runs no afterInsert for the phantom rows', async () => {
64+
// Driver drops one row from its return set.
65+
const { engine } = await makeEngine({
66+
bulkCreate: async (_o, rows) => rows.slice(1).map((r, i) => ({ id: `id-${i}`, ...r })),
67+
});
68+
const afterCalls: any[] = [];
69+
engine.registerHook('afterInsert', async (ctx: any) => { afterCalls.push(ctx.result); }, { object: 'task' });
70+
71+
await expect(engine.insert('task', [{ title: 'a' }, { title: 'b' }]))
72+
.rejects.toMatchObject({ code: 'ERR_BULK_RESULT_MISMATCH' });
73+
expect(afterCalls).toHaveLength(0); // never fed undefined
74+
});
75+
76+
it('rejects a non-array bulkCreate return', async () => {
77+
const { engine } = await makeEngine({ bulkCreate: async () => undefined });
78+
await expect(engine.insert('task', [{ title: 'a' }, { title: 'b' }]))
79+
.rejects.toMatchObject({ code: 'ERR_BULK_RESULT_MISMATCH' });
80+
});
81+
82+
it('accepts a correct one-per-row bulkCreate return (regression)', async () => {
83+
const { engine } = await makeEngine();
84+
const res = await engine.insert('task', [{ title: 'a' }, { title: 'b' }]);
85+
expect(res).toHaveLength(2);
86+
expect((res as any[]).every((r) => r?.id)).toBe(true);
87+
});
88+
89+
it('leaves the single-record insert path unaffected by the batch guard', async () => {
90+
const { engine } = await makeEngine();
91+
const res = await engine.insert('task', { title: 'solo' });
92+
expect((res as any).id).toBeTruthy();
93+
});
94+
});

packages/objectql/src/engine.ts

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2274,6 +2274,25 @@ export class ObjectQL implements IDataEngine {
22742274
result = await driver.create(object, rows[0], driverOptions);
22752275
}
22762276

2277+
// Driver-result contract guard (framework#3151): a batch write must
2278+
// return one record per input row. A short / non-array return would
2279+
// otherwise be padded with `undefined` below and fed to afterInsert
2280+
// hooks (`ctx.result === undefined`) and back to the caller as phantom
2281+
// records — corrupting seed externalId→id maps and import undo logs.
2282+
// Refuse it: throw so the caller sees a real failure rather than
2283+
// silent data loss. (Every driver in this repo already returns
2284+
// one-per-row in order; this defends against third-party drivers.)
2285+
if (isBatch && (!Array.isArray(result) || result.length !== rows.length)) {
2286+
throw Object.assign(
2287+
new Error(
2288+
`bulkCreate for '${object}' returned ${
2289+
Array.isArray(result) ? `${result.length} record(s)` : String(typeof result)
2290+
} for ${rows.length} input row(s) — refusing to fabricate afterInsert contexts`,
2291+
),
2292+
{ code: 'ERR_BULK_RESULT_MISMATCH' },
2293+
);
2294+
}
2295+
22772296
// Coerce `boolean` fields (SQLite/libsql return 0/1) to real booleans on
22782297
// the after-hook view so flow trigger conditions (`record.is_escalated
22792298
// != true`) and `{record.<bool>}` interpolation see JS booleans, not

0 commit comments

Comments
 (0)