Skip to content

Commit 7b64f96

Browse files
committed
docs(spec): HookContext.input 契约表改成引擎真正构造的形状 (#5273)
The `HookContext.input` table named three keys no producer sets: `ast` on bulk update AND bulk delete, and `doc` on insert. `input` is `z.record(z.string(), z.unknown())` — an open shape — so Zod validated none of it and the prose was the only contract an author could read. - Bulk writes carry no `ast`. The row-scoping predicate lives on the engine-internal `OperationContext.ast` (#2982) so middleware-composed filters bind the driver call where no handler can widen them. Deleted the "the row-scoping predicate is carried in `input.ast`" sentence. - `input.id` on a bulk before-event is present but `undefined` (the engine builds `{ id, … }` with shorthand), not absent — documented as such, since `'id' in input` answers true. - Documented the post-#5038 per-row after-event shape: `after*` on a bulk write dispatches once per matched row on a single-record-shaped context, so `input.id` IS bound there. - insert builds `{ data }`, not `{ doc }`. Kept: before-events still fire once per batch, and there is no `*Many` event. No engine change. The `ast` special-case in `hook-wrappers.ts` deliberately stays — it is live on the READ path, where `input.ast` is real and a handler may rewrite it. Pinned in `packages/objectql/src/hook-input-shape-contract.test.ts`: spec cannot execute a dispatch (objectql depends on spec, so a spec-side test would invert the dependency), so the facts are asserted next to the engine that produces them. `beforeFind` is the positive control (#4865) — it really does carry `ast`, so "no ast on writes" is a measurement rather than a vacuous pass. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018fxLGQdatPbBUvCgiVxg6D
1 parent 23dba62 commit 7b64f96

2 files changed

Lines changed: 355 additions & 6 deletions

File tree

Lines changed: 320 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,320 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* [#5273] The `HookContext.input` shape table in `packages/spec` is TRUE of
5+
* this engine.
6+
*
7+
* `packages/spec/src/data/hook.zod.ts` documents, per operation, the exact
8+
* `input` a handler receives. That table is the whole contract: `input` itself
9+
* is `z.record(z.string(), z.unknown())` — an open shape by design, so Zod
10+
* validates NOTHING about which keys are present, and the prose is the only
11+
* thing an author (human or AI) can read to learn what to reach for. Which is
12+
* why it drifted silently: three keys it named had no producer left.
13+
*
14+
* - `update (bulk)` / `delete (bulk)` were documented as carrying
15+
* `{ ast: QueryAST, ... }`, with the table repeating below it that "the
16+
* row-scoping predicate is carried in `input.ast`". The engine has never
17+
* put an AST on a WRITE context: the bulk predicate lives on the internal
18+
* `OperationContext.ast` (#2982) precisely so middleware-composed row
19+
* filters bind the driver call where no handler can widen them. So the one
20+
* field the docs pointed at resolved `undefined`.
21+
* - `insert` was documented as `{ doc: Record, ... }`; the engine builds
22+
* `{ data: row, ... }`. (`trigger-record-change` still carries a defensive
23+
* `input.doc` alias read for that reason — filed separately, not fixed
24+
* here.)
25+
*
26+
* The table was also silent about #5038: since ADR-0058's bulk-write addendum
27+
* the `after*` events on a bulk write fire PER MATCHED ROW on a
28+
* single-record-shaped context, so `input.id` — documented as absent on bulk
29+
* writes — is in fact bound on every after-event a bulk write dispatches.
30+
*
31+
* ## Why this file lives in objectql
32+
*
33+
* The defect is in spec's prose, but prose is unassertable and `packages/spec`
34+
* cannot execute a hook dispatch: objectql depends on spec, so a spec-side
35+
* test importing the engine would invert the dependency. The FACTS the prose
36+
* claims are pinned here instead, next to the engine that produces them and
37+
* next to #5038's own `bulk-write-per-row-hooks.test.ts`.
38+
*
39+
* ## Reading the assertions
40+
*
41+
* Hooks are registered with `engine.registerHook` — the RAW context, so what
42+
* is asserted is what the engine constructs, not a view of it. The declarative
43+
* (metadata `Hook`) path additionally wraps `ctx.input` in the flat-input
44+
* proxy of `hook-wrappers.ts`; the last describe pins that an author on THAT
45+
* path sees the same answer, since the false `input.ast` sentence was aimed at
46+
* exactly those authors.
47+
*
48+
* `beforeFind` is the POSITIVE CONTROL (#4865): it really does carry
49+
* `input.ast`, so "no `ast` on the write paths" is a measurement, not an
50+
* assertion that would pass against an engine that had stopped setting `ast`
51+
* anywhere at all.
52+
*/
53+
54+
import { describe, it, expect } from 'vitest';
55+
import { ObjectQL } from './engine.js';
56+
import { bindHooksToEngine } from './hook-binder.js';
57+
import type { Hook, HookContext } from '@objectstack/spec/data';
58+
59+
const TASK_FIELDS = {
60+
id: { name: 'id', label: 'ID', type: 'text' as const, primaryKey: true },
61+
title: { name: 'title', label: 'Title', type: 'text' as const },
62+
status: { name: 'status', label: 'Status', type: 'text' as const },
63+
};
64+
const taskObject = { name: 'task', label: 'Task', fields: TASK_FIELDS };
65+
66+
const silentLogger = { debug: () => {}, info: () => {}, warn: () => {}, error: () => {} };
67+
68+
/* ────────────────────────────────────────────────────────────────────────────
69+
* 1. The row-scoping predicate is NOT on `input` (the deleted claim)
70+
* ──────────────────────────────────────────────────────────────────────────── */
71+
72+
describe('[#5273] a bulk write carries no `ast` on `input`', () => {
73+
it('POSITIVE CONTROL — a read DOES carry `input.ast`', async () => {
74+
// Without this, every "no ast" assertion below would also pass against an
75+
// engine that had stopped building read contexts correctly.
76+
const seen: Array<Record<string, unknown>> = [];
77+
const { engine } = await boot();
78+
engine.registerHook('beforeFind', async (ctx: any) => { seen.push(ctx.input); }, { object: 'task' });
79+
80+
await engine.find('task', {} as any);
81+
82+
expect(seen).toHaveLength(1);
83+
expect('ast' in seen[0]!).toBe(true);
84+
expect(seen[0]!.ast).toBeDefined();
85+
});
86+
87+
it('`beforeUpdate` on a bulk write has no `ast` key', async () => {
88+
const seen: Array<Record<string, unknown>> = [];
89+
const { engine } = await boot();
90+
engine.registerHook('beforeUpdate', async (ctx: any) => { seen.push(ctx.input); }, { object: 'task' });
91+
92+
await seedTasks(engine, [{ title: 'a', status: 'todo' }, { title: 'b', status: 'todo' }]);
93+
await engine.update('task', { status: 'done' }, { multi: true, where: { status: 'todo' } } as any);
94+
95+
expect(seen).toHaveLength(1); // before* fires ONCE for the whole batch
96+
expect('ast' in seen[0]!).toBe(false);
97+
expect(seen[0]!.ast).toBeUndefined();
98+
});
99+
100+
it('`beforeDelete` on a bulk write has no `ast` key', async () => {
101+
const seen: Array<Record<string, unknown>> = [];
102+
const { engine } = await boot();
103+
engine.registerHook('beforeDelete', async (ctx: any) => { seen.push(ctx.input); }, { object: 'task' });
104+
105+
await seedTasks(engine, [{ title: 'a', status: 'todo' }, { title: 'b', status: 'todo' }]);
106+
await engine.delete('task', { multi: true, where: { status: 'todo' } } as any);
107+
108+
expect(seen).toHaveLength(1);
109+
expect('ast' in seen[0]!).toBe(false);
110+
expect(seen[0]!.ast).toBeUndefined();
111+
});
112+
});
113+
114+
/* ────────────────────────────────────────────────────────────────────────────
115+
* 2. `input.id` — present-but-undefined on the batch, bound per row after
116+
* ──────────────────────────────────────────────────────────────────────────── */
117+
118+
describe('[#5273] `input.id` on a bulk write', () => {
119+
it('`beforeUpdate` leaves `id` undefined (the key exists; nothing binds it)', async () => {
120+
const seen: Array<Record<string, unknown>> = [];
121+
const { engine } = await boot();
122+
engine.registerHook('beforeUpdate', async (ctx: any) => { seen.push(ctx.input); }, { object: 'task' });
123+
124+
await seedTasks(engine, [{ title: 'a', status: 'todo' }, { title: 'b', status: 'todo' }]);
125+
await engine.update('task', { status: 'done' }, { multi: true, where: { status: 'todo' } } as any);
126+
127+
// The engine builds `{ id, data, options }` with the shorthand `id`, so the
128+
// KEY is there while the value is not. Documented as `{ id: undefined, … }`
129+
// rather than "no id" because `'id' in input` answers true.
130+
expect('id' in seen[0]!).toBe(true);
131+
expect(seen[0]!.id).toBeUndefined();
132+
expect(seen[0]!.data).toEqual({ status: 'done' });
133+
expect(seen[0]!.options).toBeDefined();
134+
});
135+
136+
it('`afterUpdate` fires per matched row, each naming its own `id`', async () => {
137+
const seen: Array<Record<string, unknown>> = [];
138+
const { engine } = await boot();
139+
engine.registerHook('afterUpdate', async (ctx: any) => { seen.push(ctx.input); }, { object: 'task' });
140+
141+
const rows = await seedTasks(engine, [{ title: 'a', status: 'todo' }, { title: 'b', status: 'todo' }]);
142+
await engine.update('task', { status: 'done' }, { multi: true, where: { status: 'todo' } } as any);
143+
144+
expect(seen).toHaveLength(2);
145+
expect(seen.map((i) => i.id).sort()).toEqual(rows.map((r) => r.id).sort());
146+
// Single-record shape: the payload rides along, exactly as on a single-id
147+
// write, so a handler needs no bulk-aware branch.
148+
for (const input of seen) expect(input.data).toEqual({ status: 'done' });
149+
});
150+
151+
it('`afterDelete` fires per matched row, each naming its own `id`', async () => {
152+
const seen: Array<Record<string, unknown>> = [];
153+
const { engine } = await boot();
154+
engine.registerHook('afterDelete', async (ctx: any) => { seen.push(ctx.input); }, { object: 'task' });
155+
156+
const rows = await seedTasks(engine, [{ title: 'a', status: 'todo' }, { title: 'b', status: 'todo' }]);
157+
await engine.delete('task', { multi: true, where: { status: 'todo' } } as any);
158+
159+
expect(seen).toHaveLength(2);
160+
expect(seen.map((i) => i.id).sort()).toEqual(rows.map((r) => r.id).sort());
161+
// A delete has no post-state, so no payload rides along.
162+
for (const input of seen) expect('data' in input).toBe(false);
163+
});
164+
});
165+
166+
/* ────────────────────────────────────────────────────────────────────────────
167+
* 3. The rest of the table, so the whole thing is measured and not just the
168+
* two rows #5273 named
169+
* ──────────────────────────────────────────────────────────────────────────── */
170+
171+
describe('[#5273] the single-record rows of the table', () => {
172+
it('insert carries `data` — never `doc`', async () => {
173+
const seen: Array<Record<string, unknown>> = [];
174+
const { engine } = await boot();
175+
engine.registerHook('beforeInsert', async (ctx: any) => { seen.push(ctx.input); }, { object: 'task' });
176+
177+
await engine.insert('task', { title: 'a', status: 'todo' } as any);
178+
179+
expect(seen).toHaveLength(1);
180+
expect(seen[0]!.data).toMatchObject({ title: 'a', status: 'todo' });
181+
expect('doc' in seen[0]!).toBe(false);
182+
});
183+
184+
it('a batch insert builds ONE context per row (#2922)', async () => {
185+
const seen: Array<Record<string, unknown>> = [];
186+
const { engine } = await boot();
187+
engine.registerHook('beforeInsert', async (ctx: any) => { seen.push(ctx.input); }, { object: 'task' });
188+
189+
await engine.insert('task', [{ title: 'a' }, { title: 'b' }] as any);
190+
191+
expect(seen).toHaveLength(2);
192+
expect(seen.map((i) => (i.data as any).title)).toEqual(['a', 'b']);
193+
});
194+
195+
it('a single-id update binds `id` and `data`', async () => {
196+
const seen: Array<Record<string, unknown>> = [];
197+
const { engine } = await boot();
198+
engine.registerHook('beforeUpdate', async (ctx: any) => { seen.push(ctx.input); }, { object: 'task' });
199+
200+
const [row] = await seedTasks(engine, [{ title: 'a', status: 'todo' }]);
201+
await engine.update('task', { status: 'done' }, { where: { id: row.id } } as any);
202+
203+
expect(seen[0]!.id).toBe(row.id);
204+
expect(seen[0]!.data).toEqual({ status: 'done' });
205+
expect('ast' in seen[0]!).toBe(false);
206+
});
207+
208+
it('a single-id delete binds `id` and carries no `data`', async () => {
209+
const seen: Array<Record<string, unknown>> = [];
210+
const { engine } = await boot();
211+
engine.registerHook('beforeDelete', async (ctx: any) => { seen.push(ctx.input); }, { object: 'task' });
212+
213+
const [row] = await seedTasks(engine, [{ title: 'a', status: 'todo' }]);
214+
await engine.delete('task', { where: { id: row.id } } as any);
215+
216+
expect(seen[0]!.id).toBe(row.id);
217+
expect('data' in seen[0]!).toBe(false);
218+
expect('ast' in seen[0]!).toBe(false);
219+
});
220+
});
221+
222+
/* ────────────────────────────────────────────────────────────────────────────
223+
* 4. The declarative path sees the same answer
224+
* ──────────────────────────────────────────────────────────────────────────── */
225+
226+
describe('[#5273] a metadata-declared hook reads the same shape', () => {
227+
it('`ctx.input.ast` is undefined on a bulk update through the flat-input proxy', async () => {
228+
// The deleted sentence told THIS author to read `input.ast`. The proxy
229+
// passes `ast` through to the wrapper rather than folding it into `data`,
230+
// so the read is faithful — there is simply nothing behind it on a write.
231+
const seen: unknown[] = [];
232+
const { engine } = await boot();
233+
bindHooksToEngine(
234+
engine,
235+
[{
236+
name: 'reads_ast', object: 'task', events: ['beforeUpdate'], priority: 100,
237+
handler: (ctx: HookContext) => { seen.push((ctx.input as any).ast); },
238+
} as unknown as Hook],
239+
{ packageId: 'app:test', logger: silentLogger },
240+
);
241+
242+
await seedTasks(engine, [{ title: 'a', status: 'todo' }]);
243+
await engine.update('task', { status: 'done' }, { multi: true, where: { status: 'todo' } } as any);
244+
245+
expect(seen).toEqual([undefined]);
246+
});
247+
});
248+
249+
/* ────────────────────────────────────────────────────────────────────────────
250+
* Harness — a memory driver just wide enough for the dispatch paths above.
251+
* ──────────────────────────────────────────────────────────────────────────── */
252+
253+
async function seedTasks(engine: ObjectQL, rows: Record<string, unknown>[]): Promise<any[]> {
254+
const written = await engine.insert('task', rows as any);
255+
return Array.isArray(written) ? written : [written];
256+
}
257+
258+
function makeMemoryDriver(): any {
259+
const stores = new Map<string, Map<string, Record<string, unknown>>>();
260+
const storeFor = (o: string) => {
261+
let s = stores.get(o);
262+
if (!s) { s = new Map(); stores.set(o, s); }
263+
return s;
264+
};
265+
let nextId = 0;
266+
const matches = (row: Record<string, unknown>, where: any): boolean => {
267+
if (!where || typeof where !== 'object') return true;
268+
for (const [k, v] of Object.entries(where)) {
269+
if (k.startsWith('$')) continue;
270+
const expected = v && typeof v === 'object' && '$eq' in (v as any) ? (v as any).$eq : v;
271+
if ((row[k] ?? null) !== (expected ?? null)) return false;
272+
}
273+
return true;
274+
};
275+
const d: any = {
276+
name: 'memory', version: '0.0.0', supports: {},
277+
async connect() {}, async disconnect() {}, async checkHealth() { return true; },
278+
async execute() { return null; }, async syncSchema() {},
279+
async find(o: string, ast: any) {
280+
return Array.from(storeFor(o).values()).filter((r) => matches(r, ast?.where));
281+
},
282+
async findOne(o: string, ast: any) { for (const r of storeFor(o).values()) if (matches(r, ast?.where)) return r; return null; },
283+
async create(o: string, data: Record<string, unknown>) {
284+
nextId += 1;
285+
const id = (data.id as string) ?? `r_${nextId}`;
286+
const row = { ...data, id }; storeFor(o).set(id, row); return row;
287+
},
288+
async update(o: string, id: string, data: Record<string, unknown>) {
289+
const s = storeFor(o); const cur = s.get(id); if (!cur) return null;
290+
const u = { ...cur, ...data, id }; s.set(id, u); return u;
291+
},
292+
async upsert(o: string, data: any) { const id = data.id; return id && storeFor(o).has(id) ? this.update(o, id, data) : this.create(o, data); },
293+
async delete(o: string, id: string) { return storeFor(o).delete(id); },
294+
async count(o: string, ast: any) { return Array.from(storeFor(o).values()).filter((r) => matches(r, ast?.where)).length; },
295+
async bulkCreate(o: string, rows: any[]) { return Promise.all(rows.map((r) => this.create(o, r))); },
296+
async bulkUpdate() { return []; }, async bulkDelete() {},
297+
async updateMany(o: string, ast: any, data: Record<string, unknown>) {
298+
const rows = Array.from(storeFor(o).values()).filter((r) => matches(r, ast?.where));
299+
for (const r of rows) storeFor(o).set(r.id as string, { ...r, ...data, id: r.id });
300+
return rows.length;
301+
},
302+
async deleteMany(o: string, ast: any) {
303+
const rows = Array.from(storeFor(o).values()).filter((r) => matches(r, ast?.where));
304+
for (const r of rows) storeFor(o).delete(r.id as string);
305+
return rows.length;
306+
},
307+
async beginTransaction() { return { commit: async () => {}, rollback: async () => {} }; },
308+
async commit() {}, async rollback() {},
309+
};
310+
return d;
311+
}
312+
313+
async function boot(): Promise<{ engine: ObjectQL; driver: any }> {
314+
const engine = new ObjectQL();
315+
const driver = makeMemoryDriver();
316+
engine.registerDriver(driver, true);
317+
await engine.init();
318+
engine.registry.registerObject(taskObject as any);
319+
return { engine, driver };
320+
}

0 commit comments

Comments
 (0)