Skip to content

Commit 24ca898

Browse files
committed
test(objectql,metadata-protocol): pin per-row dropped fields on dry run and commit
Claude-Session: https://claude.ai/code/session_01Ujdtvqs7ree7WyQmEDwEnG Co-authored-by: Claude <noreply@anthropic.com>
1 parent b8976bf commit 24ca898

3 files changed

Lines changed: 307 additions & 9 deletions

File tree

‎packages/metadata-protocol/src/protocol.dropped-fields.bulk.test.ts‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -141,6 +141,11 @@ describe('insertManyData — BATCH-LEVEL droppedFields that names no row (#3455)
141141
// so the cases are REPLACED rather than amended. The engine double below
142142
// models the exemption, which the old one had no concept of; that is why the
143143
// old case stayed green through exactly the shape it was written for.
144+
//
145+
// [#20922] Row precision now comes from the ENGINE, on each `ok` outcome's
146+
// own `droppedFields` (pinned against the real engine in objectql's
147+
// `engine-per-row-dropped-fields.test.ts`). The double below attributes no
148+
// row, so these cases pin what THIS seam adds: nothing derived from the union.
144149

145150
/**
146151
* `hookStamps` names the rows whose `beforeInsert` hook re-assigns
@@ -250,6 +255,29 @@ describe('insertManyData — BATCH-LEVEL droppedFields that names no row (#3455)
250255
});
251256
expect(res).not.toHaveProperty('droppedFields');
252257
});
258+
259+
it('[#20922] an outcome the ENGINE attributed passes through as answered, beside the unchanged union', async () => {
260+
// The per-row channel is the engine's (`InsertManyRowOutcome.droppedFields`,
261+
// recorded at its strips). This seam relays it and derives nothing: the
262+
// cases above, whose engine attributes no row, still get none.
263+
const rowDrop = { object: 'approval_case', fields: ['approval_status'], reason: 'readonly' as const };
264+
const insertMany = vi.fn(async (object: string, rows: any[], options?: any) => {
265+
options?.onFieldsDropped?.({ object, fields: ['approval_status'], reason: 'readonly' });
266+
return rows.map((r, i) => (i === 1
267+
? { ok: true, record: { id: 'rec-2', title: r.title, approval_status: 'draft' }, droppedFields: [rowDrop] }
268+
: { ok: true, record: { id: 'rec-1', title: r.title, approval_status: 'draft' } }));
269+
});
270+
const p = makeProtocol(insertMany as any);
271+
272+
const res: any = await p.insertManyData({
273+
object: 'approval_case',
274+
records: [{ title: 'A' }, { title: 'B', approval_status: 'approved' }],
275+
});
276+
277+
expect(res.outcomes[0]).not.toHaveProperty('droppedFields');
278+
expect(res.outcomes[1].droppedFields).toEqual([rowDrop]);
279+
expect(res.droppedFields).toEqual([rowDrop]);
280+
});
253281
});
254282

255283
describe('batchData — per-row droppedFields + context threading (#3455)', () => {

‎packages/objectql/src/engine-autonumber-runtime-owned.test.ts‎

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -305,15 +305,15 @@ describe('#5503 — autonumber is runtime-owned: bulk-create surfaces', () => {
305305
expect((res.droppedFields ?? []).flatMap((e: DroppedFieldsEvent) => e.fields)).toContain('account_number');
306306
});
307307

308-
it('insertManyData reports the union at BATCH level and names no row', async () => {
308+
it('insertManyData reports the union at BATCH level, and the row that lost the value on its own outcome', async () => {
309309
// The import runner prefers this partial-success surface, so it is the one
310-
// that has to stay honest — and honest here means naming no row: the
311-
// engine's event carries no row index, and the two facts that would let a
312-
// caller resolve it are both unavailable at the protocol seam. The row
313-
// records below are the second one: BOTH come back carrying
314-
// `account_number`, because the strip is followed by `applyAutonumbers`.
315-
// So "is the key still on the row?" answers the same for the row that was
316-
// stripped and the row that was not.
310+
// that has to stay honest. The batch-level union names no row: the
311+
// engine's event carries no row index. Nor can the row records resolve it:
312+
// BOTH come back carrying `account_number`, because the strip is followed
313+
// by `applyAutonumbers`, so "is the key still on the row?" answers the
314+
// same for the row that was stripped and the row that was not.
315+
// [#20922] Row precision comes from the ENGINE instead: the strip records
316+
// what it took per row, and the row's `ok` outcome carries it.
317317
const rig = await makeEngine();
318318
const res: any = await rig.protocol.insertManyData({
319319
object: 'an_account',
@@ -323,7 +323,10 @@ describe('#5503 — autonumber is runtime-owned: bulk-create surfaces', () => {
323323
],
324324
});
325325
expect(res.outcomes.map((o: any) => o.record.account_number)).toEqual(['ACC-0001', 'ACC-0002']);
326-
for (const o of res.outcomes) expect(o).not.toHaveProperty('droppedFields');
326+
expect(res.outcomes[0]).not.toHaveProperty('droppedFields');
327+
expect(res.outcomes[1].droppedFields).toEqual([
328+
{ object: 'an_account', fields: ['account_number'], reason: 'readonly' },
329+
]);
327330
expect((res.droppedFields ?? []).flatMap((e: DroppedFieldsEvent) => e.fields)).toEqual(['account_number']);
328331
});
329332
});
Lines changed: 267 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,267 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
//
3+
// #20922 — the per-row drop report. The dry run (`validate` → a row's
4+
// `droppedFields`) and the commit (`insertMany` → an `ok` outcome's
5+
// `droppedFields`) name what the strips took from EACH row, recorded at the
6+
// strips themselves; the batch-level `onFieldsDropped` union beside them is
7+
// unchanged and still names no row.
8+
//
9+
// ## What was wrong (measured on `main` 9b0de7de7)
10+
//
11+
// `engine.validate` and `engine.insert` built the strips' result across ALL
12+
// rows and emitted ONE event per reason, so neither the dry run nor the commit
13+
// could say which row lost which field. `ValidateDataResponseSchema.results[]`
14+
// and `ImportRowResultSchema` declare a per-row `droppedFields`, and nothing
15+
// set it: a formula value or a forged `readonly` column was answered
16+
// `valid: true` / `ok: true` on its row with nothing to say it would not land.
17+
//
18+
// ## What this file pins, on a recording driver
19+
//
20+
// A formula column (`doubled`, reason `computed`) and a static `readonly`
21+
// column (`note`, reason `readonly`): dry run and commit answer the same
22+
// per-row drops with the right reason; a clean row carries none (the control);
23+
// the listener's union is the one it always was. Through the protocol too
24+
// (`validateData`, `insertManyData`), which relay what the engine answers.
25+
26+
import { describe, it, expect } from 'vitest';
27+
import { ObjectStackProtocolImplementation } from '@objectstack/metadata-protocol';
28+
import { ObjectQL, type InsertManyRowOutcome } from './engine.js';
29+
import type { DroppedFieldsEvent } from '@objectstack/spec/data';
30+
31+
const silentLogger: any = (() => {
32+
const l: any = {
33+
trace() {}, debug() {}, info() {}, warn() {}, error() {}, fatal() {},
34+
child() { return l; },
35+
};
36+
return l;
37+
})();
38+
39+
/** Records what reaches the driver — the payload is the verdict. */
40+
function makeRecordingDriver() {
41+
const created: Array<Record<string, unknown>> = [];
42+
let seq = 0;
43+
const driver: any = {
44+
name: 'recording', version: '0.0.0', supports: {},
45+
async connect() {}, async disconnect() {}, async checkHealth() { return true; }, async execute() { return null; },
46+
async find() { return []; },
47+
async findOne() { return null; },
48+
async create(_o: string, data: Record<string, unknown>) {
49+
created.push({ ...data });
50+
return { id: (data.id as string | undefined) ?? `gen_${++seq}`, ...data };
51+
},
52+
async update(_o: string, id: string, data: Record<string, unknown>) { return { id, ...data }; },
53+
async updateMany() { return 0; },
54+
async delete() { return true; },
55+
async deleteMany() { return 0; },
56+
async count() { return 0; },
57+
async bulkCreate(o: string, list: Record<string, unknown>[]) {
58+
const out = [];
59+
for (const r of list) out.push(await driver.create(o, r));
60+
return out;
61+
},
62+
async bulkUpdate() { return []; }, async bulkDelete() {},
63+
async beginTransaction() { return { commit: async () => {}, rollback: async () => {} }; },
64+
async commit() {}, async rollback() {},
65+
};
66+
return { driver, created };
67+
}
68+
69+
/**
70+
* `doubled` is the formula under test; `note` is a static `readonly` column;
71+
* `code` carries a `maxLength`, so a row can be made INVALID without touching
72+
* either strip; `n` / `title` are the writable controls.
73+
*/
74+
async function makeRig() {
75+
const engine = new ObjectQL({ logger: silentLogger });
76+
const { driver, created } = makeRecordingDriver();
77+
engine.registerDriver(driver, true);
78+
await engine.init();
79+
engine.registry.registerObject({
80+
name: 'proj',
81+
fields: {
82+
n: { type: 'number' },
83+
title: { type: 'text' },
84+
code: { type: 'text', maxLength: 3 },
85+
doubled: { type: 'formula', expression: 'record.n * 2' },
86+
note: { type: 'text', readonly: true },
87+
},
88+
} as any);
89+
const protocol = new ObjectStackProtocolImplementation(engine as any);
90+
return { engine, protocol, created };
91+
}
92+
93+
function listener() {
94+
const events: DroppedFieldsEvent[] = [];
95+
return { events, onFieldsDropped: (e: DroppedFieldsEvent) => { events.push(e); } };
96+
}
97+
98+
const COMPUTED = (...fields: string[]): DroppedFieldsEvent => ({ object: 'proj', fields, reason: 'computed' });
99+
const READONLY = (...fields: string[]): DroppedFieldsEvent => ({ object: 'proj', fields, reason: 'readonly' });
100+
101+
/**
102+
* Four rows, one per cell: a formula value, a forged `readonly` value, a clean
103+
* row (the control), and both at once.
104+
*/
105+
const ROWS: Array<Record<string, unknown>> = [
106+
{ n: 1, doubled: 5 },
107+
{ n: 2, note: 'forged' },
108+
{ n: 3, title: 'clean' },
109+
{ n: 4, doubled: 9, note: 'forged' },
110+
];
111+
/** What each row of {@link ROWS} loses, in the strips' order. */
112+
const PER_ROW: Array<DroppedFieldsEvent[] | undefined> = [
113+
[COMPUTED('doubled')],
114+
[READONLY('note')],
115+
undefined,
116+
[COMPUTED('doubled'), READONLY('note')],
117+
];
118+
/** The batch-level union over {@link ROWS}: one event per reason, naming no row. */
119+
const UNION: DroppedFieldsEvent[] = [COMPUTED('doubled'), READONLY('note')];
120+
121+
function okOutcome(o: InsertManyRowOutcome): Extract<InsertManyRowOutcome, { ok: true }> {
122+
if (!o.ok) throw new Error(`expected an ok outcome, got ${JSON.stringify(o)}`);
123+
return o;
124+
}
125+
126+
describe('[#20922] the dry run answers per-row drops — engine.validate', () => {
127+
it('each row names what the write would take from IT, with the right reason; the clean row carries none', async () => {
128+
const { engine, created } = await makeRig();
129+
const res = await engine.validate('proj', ROWS);
130+
131+
expect(res.valid).toBe(true);
132+
expect(res.results).toHaveLength(ROWS.length);
133+
res.results.forEach((r, i) => {
134+
expect(r.valid).toBe(true);
135+
if (PER_ROW[i] === undefined) expect(r, `row ${i} took nothing`).not.toHaveProperty('droppedFields');
136+
else expect(r.droppedFields, `row ${i}`).toEqual(PER_ROW[i]);
137+
// A strip is not a finding.
138+
expect(r.errors).toEqual([]);
139+
});
140+
expect(created).toHaveLength(0);
141+
});
142+
143+
it('the listener still reports the batch-level union, one event per reason, naming no row', async () => {
144+
const { engine } = await makeRig();
145+
const l = listener();
146+
await engine.validate('proj', ROWS, { onFieldsDropped: l.onFieldsDropped });
147+
expect(l.events).toEqual(UNION);
148+
});
149+
150+
it('a row the verdict refuses carries no drops — the write would not complete it', async () => {
151+
const { engine } = await makeRig();
152+
const res = await engine.validate('proj', [{ n: 1, code: 'too-long', doubled: 5 }, { n: 2, doubled: 5 }]);
153+
154+
expect(res.results[0]!.valid).toBe(false);
155+
expect(res.results[0]).not.toHaveProperty('droppedFields');
156+
expect(res.results[1]!.droppedFields).toEqual([COMPUTED('doubled')]);
157+
});
158+
159+
it('isSystem: the readonly strip stands down per row as it does on the write, the computed strip does not', async () => {
160+
const { engine } = await makeRig();
161+
const res = await engine.validate('proj', ROWS, { context: { isSystem: true } as any });
162+
expect(res.results.map((r) => r.droppedFields)).toEqual([
163+
[COMPUTED('doubled')], undefined, undefined, [COMPUTED('doubled')],
164+
]);
165+
});
166+
167+
it('update mode: the same per-row report from the update-side strips', async () => {
168+
const { engine } = await makeRig();
169+
const res = await engine.validate('proj', ROWS, { mode: 'update' });
170+
expect(res.results.map((r) => r.droppedFields)).toEqual(PER_ROW);
171+
});
172+
});
173+
174+
describe('[#20922] the commit answers per-row drops — engine.insertMany', () => {
175+
it('each ok outcome names what was taken from THAT row; the clean row carries none', async () => {
176+
const { engine, created } = await makeRig();
177+
const outcomes = await engine.insertMany('proj', ROWS);
178+
179+
expect(outcomes).toHaveLength(ROWS.length);
180+
outcomes.forEach((o, i) => {
181+
const ok = okOutcome(o);
182+
if (PER_ROW[i] === undefined) expect(ok, `row ${i} took nothing`).not.toHaveProperty('droppedFields');
183+
else expect(ok.droppedFields, `row ${i}`).toEqual(PER_ROW[i]);
184+
});
185+
// The report describes the stored payload: neither key reached the driver.
186+
expect(created).toHaveLength(ROWS.length);
187+
for (const row of created) {
188+
expect(row).not.toHaveProperty('doubled');
189+
expect(row).not.toHaveProperty('note');
190+
}
191+
});
192+
193+
it('the dry run and the commit answer the same drops, row for row', async () => {
194+
const { engine } = await makeRig();
195+
const dry = await engine.validate('proj', ROWS);
196+
const commit = await engine.insertMany('proj', ROWS);
197+
expect(commit.map((o) => okOutcome(o).droppedFields)).toEqual(dry.results.map((r) => r.droppedFields));
198+
});
199+
200+
it('the listener still reports the batch-level union, one event per reason, naming no row', async () => {
201+
const { engine } = await makeRig();
202+
const l = listener();
203+
await engine.insertMany('proj', ROWS, { onFieldsDropped: l.onFieldsDropped });
204+
expect(l.events).toEqual(UNION);
205+
});
206+
207+
it('a failed outcome carries no drops — its write did not complete', async () => {
208+
const { engine } = await makeRig();
209+
const outcomes = await engine.insertMany('proj', [{ n: 1, code: 'too-long', note: 'forged' }, { n: 2, note: 'forged' }]);
210+
211+
expect(outcomes[0]!.ok).toBe(false);
212+
expect(outcomes[0]).not.toHaveProperty('droppedFields');
213+
expect(okOutcome(outcomes[1]!).droppedFields).toEqual([READONLY('note')]);
214+
});
215+
216+
it('a hook that writes the readonly key on ONE row keeps it there: only the other row is named', async () => {
217+
// The case a reconstruction from the union cannot get right: both rows
218+
// SUPPLIED `note`, so "which rows supplied it" names both. Row 1's
219+
// `beforeInsert` hook assigns it, which exempts it on that row alone.
220+
const { engine, created } = await makeRig();
221+
engine.registerHook('beforeInsert', async (ctx: any) => {
222+
if (ctx.input.data.n === 2) ctx.input.data.note = 'hook-stamped';
223+
}, { object: 'proj' });
224+
const l = listener();
225+
const outcomes = await engine.insertMany(
226+
'proj',
227+
[{ n: 1, note: 'forged' }, { n: 2, note: 'forged' }],
228+
{ onFieldsDropped: l.onFieldsDropped },
229+
);
230+
231+
expect(okOutcome(outcomes[0]!).droppedFields).toEqual([READONLY('note')]);
232+
expect(okOutcome(outcomes[1]!), 'the hook wrote it, so it was not dropped').not.toHaveProperty('droppedFields');
233+
expect(created[1]).toMatchObject({ note: 'hook-stamped' });
234+
expect(l.events).toEqual([READONLY('note')]);
235+
});
236+
237+
it('the non-partial batch insert is unchanged: it returns the records, with no per-row slot', async () => {
238+
const { engine } = await makeRig();
239+
const records = await engine.insert('proj', ROWS);
240+
expect(Array.isArray(records)).toBe(true);
241+
for (const r of records as Array<Record<string, unknown>>) expect(r).not.toHaveProperty('droppedFields');
242+
});
243+
});
244+
245+
describe('[#20922] through the protocol — validateData and insertManyData relay the engine\'s per-row report', () => {
246+
it('validateData answers each row\'s drops in results[].droppedFields', async () => {
247+
const { protocol } = await makeRig();
248+
const res: any = await protocol.validateData({ object: 'proj', data: ROWS });
249+
expect(res.results.map((r: any) => r.droppedFields)).toEqual(PER_ROW);
250+
});
251+
252+
it('insertManyData passes the per-outcome report through and keeps the batch-level union', async () => {
253+
const { protocol } = await makeRig();
254+
const res = await protocol.insertManyData({ object: 'proj', records: ROWS });
255+
expect(res.outcomes.map((o) => o.droppedFields)).toEqual(PER_ROW);
256+
expect(res.droppedFields).toEqual(UNION);
257+
});
258+
259+
it('control: nothing taken anywhere ⇒ no per-row key and no batch-level key', async () => {
260+
const { protocol } = await makeRig();
261+
const dry: any = await protocol.validateData({ object: 'proj', data: [{ n: 1 }, { n: 2, title: 'x' }] });
262+
for (const r of dry.results) expect(r).not.toHaveProperty('droppedFields');
263+
const commit = await protocol.insertManyData({ object: 'proj', records: [{ n: 1 }, { n: 2, title: 'x' }] });
264+
for (const o of commit.outcomes) expect(o).not.toHaveProperty('droppedFields');
265+
expect(commit).not.toHaveProperty('droppedFields');
266+
});
267+
});

0 commit comments

Comments
 (0)