Skip to content

Commit 28ce594

Browse files
os-zhuangclaude
andauthored
docs(spec): HookContext.input 契约表改成引擎真正构造的形状 (#5273) (#5668)
* 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 * test(objectql): 正控的 find 选项不再擦成 `any`,过 #4918 query-options 棘轮 `check:query-options-erasure`(PR #5600 / #4918,在本分支切出之后落地) 把测试面计入只增不减的棘轮。本分支新增的正控里 `engine.find('task', {} as any)` 是其中一处擦除,使测试面 267 → 268 而红。 该调用点是**合约内**形状——`find(object, query?: EngineQueryOptions)` 的空查询——不是「断言引擎拒绝未知选项」那类刻意越契约的输入,所以按 门禁处方的第 1 条直接给它正确类型(这里等于去掉断言,签名本就能推 断),而不是写 `as unknown as EngineQueryOptions`。 基线文件未动:抬高天花板是「reviewed edit, not a remedy」,这里修的是 站点本身。 验证(合并 origin/main 后): - node scripts/check-query-options-erasure-ratchet.mjs → 267,at the ceiling,no files added - @objectstack/objectql test → 1934 passed (120 files) - @objectstack/spec + @objectstack/objectql typecheck → Done Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018fxLGQdatPbBUvCgiVxg6D --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 9fdb2f6 commit 28ce594

2 files changed

Lines changed: 359 additions & 6 deletions

File tree

Lines changed: 324 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,324 @@
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+
// No `as any` on the options: `find(object, query?: EngineQueryOptions)`
81+
// already infers an empty query, and erasing it would add a site to the
82+
// #4918 query-options ratchet (`check:query-options-erasure`) for no gain —
83+
// this call is in-contract, not a deliberate off-contract probe.
84+
await engine.find('task', {});
85+
86+
expect(seen).toHaveLength(1);
87+
expect('ast' in seen[0]!).toBe(true);
88+
expect(seen[0]!.ast).toBeDefined();
89+
});
90+
91+
it('`beforeUpdate` on a bulk write has no `ast` key', async () => {
92+
const seen: Array<Record<string, unknown>> = [];
93+
const { engine } = await boot();
94+
engine.registerHook('beforeUpdate', async (ctx: any) => { seen.push(ctx.input); }, { object: 'task' });
95+
96+
await seedTasks(engine, [{ title: 'a', status: 'todo' }, { title: 'b', status: 'todo' }]);
97+
await engine.update('task', { status: 'done' }, { multi: true, where: { status: 'todo' } } as any);
98+
99+
expect(seen).toHaveLength(1); // before* fires ONCE for the whole batch
100+
expect('ast' in seen[0]!).toBe(false);
101+
expect(seen[0]!.ast).toBeUndefined();
102+
});
103+
104+
it('`beforeDelete` on a bulk write has no `ast` key', async () => {
105+
const seen: Array<Record<string, unknown>> = [];
106+
const { engine } = await boot();
107+
engine.registerHook('beforeDelete', async (ctx: any) => { seen.push(ctx.input); }, { object: 'task' });
108+
109+
await seedTasks(engine, [{ title: 'a', status: 'todo' }, { title: 'b', status: 'todo' }]);
110+
await engine.delete('task', { multi: true, where: { status: 'todo' } } as any);
111+
112+
expect(seen).toHaveLength(1);
113+
expect('ast' in seen[0]!).toBe(false);
114+
expect(seen[0]!.ast).toBeUndefined();
115+
});
116+
});
117+
118+
/* ────────────────────────────────────────────────────────────────────────────
119+
* 2. `input.id` — present-but-undefined on the batch, bound per row after
120+
* ──────────────────────────────────────────────────────────────────────────── */
121+
122+
describe('[#5273] `input.id` on a bulk write', () => {
123+
it('`beforeUpdate` leaves `id` undefined (the key exists; nothing binds it)', async () => {
124+
const seen: Array<Record<string, unknown>> = [];
125+
const { engine } = await boot();
126+
engine.registerHook('beforeUpdate', async (ctx: any) => { seen.push(ctx.input); }, { object: 'task' });
127+
128+
await seedTasks(engine, [{ title: 'a', status: 'todo' }, { title: 'b', status: 'todo' }]);
129+
await engine.update('task', { status: 'done' }, { multi: true, where: { status: 'todo' } } as any);
130+
131+
// The engine builds `{ id, data, options }` with the shorthand `id`, so the
132+
// KEY is there while the value is not. Documented as `{ id: undefined, … }`
133+
// rather than "no id" because `'id' in input` answers true.
134+
expect('id' in seen[0]!).toBe(true);
135+
expect(seen[0]!.id).toBeUndefined();
136+
expect(seen[0]!.data).toEqual({ status: 'done' });
137+
expect(seen[0]!.options).toBeDefined();
138+
});
139+
140+
it('`afterUpdate` fires per matched row, each naming its own `id`', async () => {
141+
const seen: Array<Record<string, unknown>> = [];
142+
const { engine } = await boot();
143+
engine.registerHook('afterUpdate', async (ctx: any) => { seen.push(ctx.input); }, { object: 'task' });
144+
145+
const rows = await seedTasks(engine, [{ title: 'a', status: 'todo' }, { title: 'b', status: 'todo' }]);
146+
await engine.update('task', { status: 'done' }, { multi: true, where: { status: 'todo' } } as any);
147+
148+
expect(seen).toHaveLength(2);
149+
expect(seen.map((i) => i.id).sort()).toEqual(rows.map((r) => r.id).sort());
150+
// Single-record shape: the payload rides along, exactly as on a single-id
151+
// write, so a handler needs no bulk-aware branch.
152+
for (const input of seen) expect(input.data).toEqual({ status: 'done' });
153+
});
154+
155+
it('`afterDelete` fires per matched row, each naming its own `id`', async () => {
156+
const seen: Array<Record<string, unknown>> = [];
157+
const { engine } = await boot();
158+
engine.registerHook('afterDelete', async (ctx: any) => { seen.push(ctx.input); }, { object: 'task' });
159+
160+
const rows = await seedTasks(engine, [{ title: 'a', status: 'todo' }, { title: 'b', status: 'todo' }]);
161+
await engine.delete('task', { multi: true, where: { status: 'todo' } } as any);
162+
163+
expect(seen).toHaveLength(2);
164+
expect(seen.map((i) => i.id).sort()).toEqual(rows.map((r) => r.id).sort());
165+
// A delete has no post-state, so no payload rides along.
166+
for (const input of seen) expect('data' in input).toBe(false);
167+
});
168+
});
169+
170+
/* ────────────────────────────────────────────────────────────────────────────
171+
* 3. The rest of the table, so the whole thing is measured and not just the
172+
* two rows #5273 named
173+
* ──────────────────────────────────────────────────────────────────────────── */
174+
175+
describe('[#5273] the single-record rows of the table', () => {
176+
it('insert carries `data` — never `doc`', async () => {
177+
const seen: Array<Record<string, unknown>> = [];
178+
const { engine } = await boot();
179+
engine.registerHook('beforeInsert', async (ctx: any) => { seen.push(ctx.input); }, { object: 'task' });
180+
181+
await engine.insert('task', { title: 'a', status: 'todo' } as any);
182+
183+
expect(seen).toHaveLength(1);
184+
expect(seen[0]!.data).toMatchObject({ title: 'a', status: 'todo' });
185+
expect('doc' in seen[0]!).toBe(false);
186+
});
187+
188+
it('a batch insert builds ONE context per row (#2922)', async () => {
189+
const seen: Array<Record<string, unknown>> = [];
190+
const { engine } = await boot();
191+
engine.registerHook('beforeInsert', async (ctx: any) => { seen.push(ctx.input); }, { object: 'task' });
192+
193+
await engine.insert('task', [{ title: 'a' }, { title: 'b' }] as any);
194+
195+
expect(seen).toHaveLength(2);
196+
expect(seen.map((i) => (i.data as any).title)).toEqual(['a', 'b']);
197+
});
198+
199+
it('a single-id update binds `id` and `data`', async () => {
200+
const seen: Array<Record<string, unknown>> = [];
201+
const { engine } = await boot();
202+
engine.registerHook('beforeUpdate', async (ctx: any) => { seen.push(ctx.input); }, { object: 'task' });
203+
204+
const [row] = await seedTasks(engine, [{ title: 'a', status: 'todo' }]);
205+
await engine.update('task', { status: 'done' }, { where: { id: row.id } } as any);
206+
207+
expect(seen[0]!.id).toBe(row.id);
208+
expect(seen[0]!.data).toEqual({ status: 'done' });
209+
expect('ast' in seen[0]!).toBe(false);
210+
});
211+
212+
it('a single-id delete binds `id` and carries no `data`', async () => {
213+
const seen: Array<Record<string, unknown>> = [];
214+
const { engine } = await boot();
215+
engine.registerHook('beforeDelete', async (ctx: any) => { seen.push(ctx.input); }, { object: 'task' });
216+
217+
const [row] = await seedTasks(engine, [{ title: 'a', status: 'todo' }]);
218+
await engine.delete('task', { where: { id: row.id } } as any);
219+
220+
expect(seen[0]!.id).toBe(row.id);
221+
expect('data' in seen[0]!).toBe(false);
222+
expect('ast' in seen[0]!).toBe(false);
223+
});
224+
});
225+
226+
/* ────────────────────────────────────────────────────────────────────────────
227+
* 4. The declarative path sees the same answer
228+
* ──────────────────────────────────────────────────────────────────────────── */
229+
230+
describe('[#5273] a metadata-declared hook reads the same shape', () => {
231+
it('`ctx.input.ast` is undefined on a bulk update through the flat-input proxy', async () => {
232+
// The deleted sentence told THIS author to read `input.ast`. The proxy
233+
// passes `ast` through to the wrapper rather than folding it into `data`,
234+
// so the read is faithful — there is simply nothing behind it on a write.
235+
const seen: unknown[] = [];
236+
const { engine } = await boot();
237+
bindHooksToEngine(
238+
engine,
239+
[{
240+
name: 'reads_ast', object: 'task', events: ['beforeUpdate'], priority: 100,
241+
handler: (ctx: HookContext) => { seen.push((ctx.input as any).ast); },
242+
} as unknown as Hook],
243+
{ packageId: 'app:test', logger: silentLogger },
244+
);
245+
246+
await seedTasks(engine, [{ title: 'a', status: 'todo' }]);
247+
await engine.update('task', { status: 'done' }, { multi: true, where: { status: 'todo' } } as any);
248+
249+
expect(seen).toEqual([undefined]);
250+
});
251+
});
252+
253+
/* ────────────────────────────────────────────────────────────────────────────
254+
* Harness — a memory driver just wide enough for the dispatch paths above.
255+
* ──────────────────────────────────────────────────────────────────────────── */
256+
257+
async function seedTasks(engine: ObjectQL, rows: Record<string, unknown>[]): Promise<any[]> {
258+
const written = await engine.insert('task', rows as any);
259+
return Array.isArray(written) ? written : [written];
260+
}
261+
262+
function makeMemoryDriver(): any {
263+
const stores = new Map<string, Map<string, Record<string, unknown>>>();
264+
const storeFor = (o: string) => {
265+
let s = stores.get(o);
266+
if (!s) { s = new Map(); stores.set(o, s); }
267+
return s;
268+
};
269+
let nextId = 0;
270+
const matches = (row: Record<string, unknown>, where: any): boolean => {
271+
if (!where || typeof where !== 'object') return true;
272+
for (const [k, v] of Object.entries(where)) {
273+
if (k.startsWith('$')) continue;
274+
const expected = v && typeof v === 'object' && '$eq' in (v as any) ? (v as any).$eq : v;
275+
if ((row[k] ?? null) !== (expected ?? null)) return false;
276+
}
277+
return true;
278+
};
279+
const d: any = {
280+
name: 'memory', version: '0.0.0', supports: {},
281+
async connect() {}, async disconnect() {}, async checkHealth() { return true; },
282+
async execute() { return null; }, async syncSchema() {},
283+
async find(o: string, ast: any) {
284+
return Array.from(storeFor(o).values()).filter((r) => matches(r, ast?.where));
285+
},
286+
async findOne(o: string, ast: any) { for (const r of storeFor(o).values()) if (matches(r, ast?.where)) return r; return null; },
287+
async create(o: string, data: Record<string, unknown>) {
288+
nextId += 1;
289+
const id = (data.id as string) ?? `r_${nextId}`;
290+
const row = { ...data, id }; storeFor(o).set(id, row); return row;
291+
},
292+
async update(o: string, id: string, data: Record<string, unknown>) {
293+
const s = storeFor(o); const cur = s.get(id); if (!cur) return null;
294+
const u = { ...cur, ...data, id }; s.set(id, u); return u;
295+
},
296+
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); },
297+
async delete(o: string, id: string) { return storeFor(o).delete(id); },
298+
async count(o: string, ast: any) { return Array.from(storeFor(o).values()).filter((r) => matches(r, ast?.where)).length; },
299+
async bulkCreate(o: string, rows: any[]) { return Promise.all(rows.map((r) => this.create(o, r))); },
300+
async bulkUpdate() { return []; }, async bulkDelete() {},
301+
async updateMany(o: string, ast: any, data: Record<string, unknown>) {
302+
const rows = Array.from(storeFor(o).values()).filter((r) => matches(r, ast?.where));
303+
for (const r of rows) storeFor(o).set(r.id as string, { ...r, ...data, id: r.id });
304+
return rows.length;
305+
},
306+
async deleteMany(o: string, ast: any) {
307+
const rows = Array.from(storeFor(o).values()).filter((r) => matches(r, ast?.where));
308+
for (const r of rows) storeFor(o).delete(r.id as string);
309+
return rows.length;
310+
},
311+
async beginTransaction() { return { commit: async () => {}, rollback: async () => {} }; },
312+
async commit() {}, async rollback() {},
313+
};
314+
return d;
315+
}
316+
317+
async function boot(): Promise<{ engine: ObjectQL; driver: any }> {
318+
const engine = new ObjectQL();
319+
const driver = makeMemoryDriver();
320+
engine.registerDriver(driver, true);
321+
await engine.init();
322+
engine.registry.registerObject(taskObject as any);
323+
return { engine, driver };
324+
}

0 commit comments

Comments
 (0)