Skip to content

Commit 2030d88

Browse files
committed
fix(objectql): the referential delete walk honours a tree field's deleteBehavior
One family predicate (statesDeleteSemantics: master_detail, lookup, tree) shared by cascadeRelationBehavior, the collection and dependents walks that read it, and planCascadeAtomicity. Claude-Session: https://claude.ai/code/session_01ADzJtzYTLUfgrRZHxkagkX Co-authored-by: Claude <noreply@anthropic.com>
1 parent 9f4b652 commit 2030d88

2 files changed

Lines changed: 442 additions & 11 deletions

File tree

Lines changed: 381 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,381 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* [#22957] A `tree` field's `deleteBehavior` is what a delete of the record it
5+
* references does: `set_null` (the default the spec materializes) clears the
6+
* child's parent, `cascade` deletes the subtree, `restrict` refuses the delete.
7+
*
8+
* ## The defect
9+
*
10+
* `FieldSchema` materializes `deleteBehavior: 'set_null'` on a `tree` beside
11+
* `lookup`, because the type "is in the relational family, where the key
12+
* states delete semantics". The referential delete walk admitted
13+
* `master_detail` and `lookup` only, so it never read a `tree` relation:
14+
* deleting a parent answered success and left every child holding the
15+
* deleted id, and an authored `cascade` or `restrict` was inert the same way.
16+
* Measured at the public doors on `examples/app-showcase`
17+
* (`showcase_field_zoo.f_tree`): `os verify --delete-behaviors` reported it
18+
* contradicted through REST and the system door.
19+
*
20+
* ## What is pinned here
21+
*
22+
* - `set_null`, materialized by the spec's own parse and authored, clears the
23+
* child's parent and keeps the child;
24+
* - `cascade` deletes the whole subtree, past `MAX_CASCADE_DEPTH`, and
25+
* nothing outside it;
26+
* - `restrict` refuses with the ADR-0112 envelope naming the tree's own object
27+
* as the dependent, and keeps the row; a leaf still deletes;
28+
* - a cycle in the data terminates: a corrupt row pointing at its own
29+
* descendant, and a row pointing at itself;
30+
* - a subtree cascade is one unit of work: a refusal from a node deep in it
31+
* rolls back the nodes already deleted (`planCascadeAtomicity` reads the
32+
* same family);
33+
* - `multiple: true` is refused on a `tree` by the parse, and a raw
34+
* registration around it is read single-valued, as the walk's multi-value
35+
* arbiter (`isMultiValueField`) reads it for every type;
36+
* - CONTROLS: `lookup` (default `set_null`, `cascade`, `restrict`) and
37+
* `master_detail` (default `cascade`, `restrict`) answer exactly as before.
38+
*
39+
* The driver has REAL snapshot rollback (the `engine-cascade-delete-atomic`
40+
* shape): a stub whose rollback is a no-op cannot tell "refused and rolled
41+
* back" from "refused halfway", so the atomicity pin would pass vacuously.
42+
*/
43+
44+
import { describe, it, expect } from 'vitest';
45+
import { FieldSchema } from '@objectstack/spec/data';
46+
import { ObjectQL } from './engine.js';
47+
48+
type Row = Record<string, unknown>;
49+
50+
function makeDriver() {
51+
const stores = new Map<string, Map<string, Row>>();
52+
const storeFor = (o: string): Map<string, Row> => {
53+
let s = stores.get(o);
54+
if (!s) { s = new Map(); stores.set(o, s); }
55+
return s;
56+
};
57+
const committed: unknown[] = [];
58+
const rolledBack: unknown[] = [];
59+
const snapshots = new Map<unknown, Map<string, Map<string, Row>>>();
60+
let nextId = 0;
61+
const matches = (row: Row, where: unknown): boolean => {
62+
if (!where || typeof where !== 'object') return true;
63+
for (const [k, v] of Object.entries(where as Row)) {
64+
if (k.startsWith('$')) continue;
65+
const exp = v && typeof v === 'object' && '$eq' in (v as Row) ? (v as Row).$eq : v;
66+
if ((row[k] ?? null) !== (exp ?? null)) return false;
67+
}
68+
return true;
69+
};
70+
const driver: any = {
71+
name: 'primary', version: '0.0.0', supports: {},
72+
committed, rolledBack,
73+
rowsOf: (o: string): Row[] => Array.from(storeFor(o).values()),
74+
rowOf: (o: string, id: unknown): Row | undefined => storeFor(o).get(String(id)),
75+
async connect() {}, async disconnect() {}, async checkHealth() { return true; }, async execute() { return null; },
76+
async syncSchema() {},
77+
async find(o: string, ast: { where?: unknown; limit?: unknown } | undefined) {
78+
const rows = Array.from(storeFor(o).values()).filter((r) => matches(r, ast?.where));
79+
return typeof ast?.limit === 'number' ? rows.slice(0, ast.limit) : rows;
80+
},
81+
async findOne(o: string, ast: { where?: unknown } | undefined) {
82+
for (const r of storeFor(o).values()) if (matches(r, ast?.where)) return r;
83+
return null;
84+
},
85+
async create(o: string, data: Row) {
86+
nextId += 1;
87+
const id = (data.id as string | undefined) ?? `r_${nextId}`;
88+
const row = { ...data, id };
89+
storeFor(o).set(id, row);
90+
return row;
91+
},
92+
async update(o: string, id: string, data: Row) {
93+
const s = storeFor(o);
94+
const cur = s.get(String(id));
95+
if (!cur) throw new Error(`not found ${o}/${id}`);
96+
const up = { ...cur, ...data, id };
97+
s.set(String(id), up);
98+
return up;
99+
},
100+
async upsert(o: string, data: Row) {
101+
const id = data.id as string | undefined;
102+
return id && storeFor(o).has(id) ? this.update(o, id, data) : this.create(o, data);
103+
},
104+
async delete(o: string, id: string) { return storeFor(o).delete(String(id)); },
105+
async count(o: string, ast: { where?: unknown } | undefined) { return (await this.find(o, ast)).length; },
106+
async bulkCreate(o: string, rows: Row[]) { return Promise.all(rows.map((r) => this.create(o, r))); },
107+
async bulkUpdate() { return []; },
108+
async bulkDelete() {},
109+
async beginTransaction() {
110+
const handle = { __trx: snapshots.size + committed.length + rolledBack.length + 1 };
111+
const snap = new Map<string, Map<string, Row>>();
112+
for (const [o, s] of stores) snap.set(o, new Map(Array.from(s, ([k, v]) => [k, { ...v }])));
113+
snapshots.set(handle, snap);
114+
return handle;
115+
},
116+
async commit(handle: unknown) { snapshots.delete(handle); committed.push(handle); },
117+
async rollback(handle: unknown) {
118+
const snap = snapshots.get(handle);
119+
if (snap) { stores.clear(); for (const [o, s] of snap) stores.set(o, s); }
120+
snapshots.delete(handle);
121+
rolledBack.push(handle);
122+
},
123+
};
124+
return driver;
125+
}
126+
127+
async function makeEngine(objects: unknown[]) {
128+
const quiet = () => {};
129+
const engine = new ObjectQL({ logger: { debug: quiet, info: quiet, warn: quiet, error: quiet } } as any);
130+
const driver = makeDriver();
131+
engine.registerDriver(driver, true);
132+
await engine.init();
133+
for (const o of objects) engine.registry.registerObject(o as any, '__test__');
134+
return { engine, driver };
135+
}
136+
137+
const id = { name: 'id', type: 'text' as const, primaryKey: true };
138+
const name = { name: 'name', type: 'text' as const };
139+
140+
/** An object whose hierarchy is the `tree` field `parent`, declared as `parentDef`. */
141+
const treeObject = (objectName: string, parentDef: Record<string, unknown>) => ({
142+
name: objectName,
143+
label: objectName,
144+
fields: { id, name, parent: { name: 'parent', ...parentDef } },
145+
});
146+
147+
/** The tree field as the spec's parse produces it, which is what a built artifact carries. */
148+
const parsedTree = (reference: string, extra: Record<string, unknown> = {}) =>
149+
FieldSchema.parse({ type: 'tree', reference, ...extra }) as Record<string, unknown>;
150+
151+
const namesOf = (rows: Row[]) => rows.map((r) => r.name).sort();
152+
153+
async function refusalOf(promise: Promise<unknown>): Promise<any> {
154+
try { await promise; } catch (err) { return err; }
155+
throw new Error('expected the delete to be refused');
156+
}
157+
158+
describe('[#22957] a tree field\'s deleteBehavior takes effect on a parent delete', () => {
159+
describe('set_null — the materialized default and the authored value', () => {
160+
it('the spec\'s parse materializes set_null on a tree that authors nothing', () => {
161+
// The premise the engine pins below read: the value a built artifact carries.
162+
expect(parsedTree('zt_node').deleteBehavior).toBe('set_null');
163+
});
164+
165+
it.each([
166+
['materialized by the parse', parsedTree('zt_node')],
167+
['authored', { type: 'tree', reference: 'zt_node', deleteBehavior: 'set_null' }],
168+
['absent (a raw registration around the parse)', { type: 'tree', reference: 'zt_node' }],
169+
])('%s: deleting the parent clears the child\'s parent and keeps the child', async (_label, def) => {
170+
const { engine, driver } = await makeEngine([treeObject('zt_node', def)]);
171+
const p = await engine.insert('zt_node', { name: 'P' });
172+
const c = await engine.insert('zt_node', { name: 'C', parent: p.id });
173+
const bystander = await engine.insert('zt_node', { name: 'B', parent: c.id });
174+
175+
await engine.delete('zt_node', { where: { id: p.id } } as any);
176+
177+
expect(driver.rowOf('zt_node', p.id)).toBeUndefined();
178+
expect(driver.rowOf('zt_node', c.id)).toMatchObject({ name: 'C', parent: null });
179+
// The other direction: a row that never referenced the deleted record is untouched.
180+
expect(driver.rowOf('zt_node', bystander.id)).toMatchObject({ name: 'B', parent: c.id });
181+
});
182+
});
183+
184+
describe('cascade — a subtree delete', () => {
185+
const node = treeObject('zt_branch', { type: 'tree', reference: 'zt_branch', deleteBehavior: 'cascade' });
186+
187+
it('deletes the node and every descendant, and nothing outside the subtree', async () => {
188+
const { engine, driver } = await makeEngine([node]);
189+
const root = await engine.insert('zt_branch', { name: 'root' });
190+
const a = await engine.insert('zt_branch', { name: 'a', parent: root.id });
191+
const b = await engine.insert('zt_branch', { name: 'b', parent: a.id });
192+
await engine.insert('zt_branch', { name: 'c', parent: b.id });
193+
await engine.insert('zt_branch', { name: 'b2', parent: a.id });
194+
await engine.insert('zt_branch', { name: 'a2', parent: root.id });
195+
const other = await engine.insert('zt_branch', { name: 'other' });
196+
await engine.insert('zt_branch', { name: 'other_kid', parent: other.id });
197+
198+
await engine.delete('zt_branch', { where: { id: a.id } } as any);
199+
200+
expect(namesOf(driver.rowsOf('zt_branch'))).toEqual(['a2', 'other', 'other_kid', 'root']);
201+
// One unit of work: the hierarchy makes the object reference itself.
202+
expect(driver.committed).toHaveLength(1);
203+
expect(driver.rolledBack).toHaveLength(0);
204+
});
205+
206+
it('reaches past MAX_CASCADE_DEPTH (10): a 15-level chain is deleted whole', async () => {
207+
// The cap bounds `planCascadeAtomicity`'s schema walk. The row cascade
208+
// is bounded by its set, so a deep subtree is never cut off part-way,
209+
// which would strand nodes whose parent is gone.
210+
const { engine, driver } = await makeEngine([node]);
211+
const top = await engine.insert('zt_branch', { name: 'L0' });
212+
let prev = top;
213+
for (let i = 1; i < 15; i++) prev = await engine.insert('zt_branch', { name: `L${i}`, parent: prev.id });
214+
const outside = await engine.insert('zt_branch', { name: 'outside' });
215+
216+
await engine.delete('zt_branch', { where: { id: top.id } } as any);
217+
218+
expect(namesOf(driver.rowsOf('zt_branch'))).toEqual(['outside']);
219+
expect(driver.rowOf('zt_branch', outside.id)).toBeDefined();
220+
});
221+
222+
it('terminates on a corrupt cycle: a row pointing at its own descendant', async () => {
223+
const { engine, driver } = await makeEngine([node]);
224+
const r = await engine.insert('zt_branch', { name: 'R' });
225+
const k = await engine.insert('zt_branch', { name: 'K', parent: r.id });
226+
const g = await engine.insert('zt_branch', { name: 'G', parent: k.id });
227+
const outside = await engine.insert('zt_branch', { name: 'outside' });
228+
// Written at the driver, the way corrupt data arrives: R's parent is its grandchild.
229+
await driver.update('zt_branch', r.id, { parent: g.id });
230+
231+
await engine.delete('zt_branch', { where: { id: k.id } } as any);
232+
233+
// K's subtree is G, whose subtree is R, whose subtree is K: each once.
234+
expect(namesOf(driver.rowsOf('zt_branch'))).toEqual(['outside']);
235+
expect(driver.rowOf('zt_branch', outside.id)).toBeDefined();
236+
});
237+
238+
it('terminates on a row that is its own parent', async () => {
239+
const { engine, driver } = await makeEngine([node]);
240+
const s = await engine.insert('zt_branch', { name: 'S' });
241+
const kid = await engine.insert('zt_branch', { name: 'kid', parent: s.id });
242+
await driver.update('zt_branch', s.id, { parent: s.id });
243+
244+
await engine.delete('zt_branch', { where: { id: s.id } } as any);
245+
246+
expect(driver.rowOf('zt_branch', s.id)).toBeUndefined();
247+
expect(driver.rowOf('zt_branch', kid.id)).toBeUndefined();
248+
});
249+
250+
it('is one unit of work: a refusal deep in the subtree rolls back the nodes already deleted', async () => {
251+
const { engine, driver } = await makeEngine([node]);
252+
const root = await engine.insert('zt_branch', { name: 'root' });
253+
await engine.insert('zt_branch', { name: 'k1', parent: root.id });
254+
const k2 = await engine.insert('zt_branch', { name: 'k2', parent: root.id });
255+
const vetoed = await engine.insert('zt_branch', { name: 'vetoed', parent: k2.id });
256+
engine.registerHook(
257+
'beforeDelete',
258+
async (ctx: any) => {
259+
if (String(ctx.input?.id) === String(vetoed.id)) {
260+
throw Object.assign(new Error('this node is held'), { status: 409 });
261+
}
262+
},
263+
{ object: 'zt_branch' },
264+
);
265+
266+
const err = await refusalOf(engine.delete('zt_branch', { where: { id: root.id } } as any));
267+
268+
expect(err).toMatchObject({ status: 409 });
269+
// k1 was cascaded away before the hook on `vetoed` refused: it is back.
270+
expect(namesOf(driver.rowsOf('zt_branch'))).toEqual(['k1', 'k2', 'root', 'vetoed']);
271+
expect(driver.rolledBack).toHaveLength(1);
272+
expect(driver.committed).toHaveLength(0);
273+
});
274+
});
275+
276+
describe('restrict', () => {
277+
const node = treeObject('zt_held', { type: 'tree', reference: 'zt_held', deleteBehavior: 'restrict' });
278+
279+
it('refuses a parent delete while a child references it, and keeps both rows', async () => {
280+
const { engine, driver } = await makeEngine([node]);
281+
const p = await engine.insert('zt_held', { name: 'P' });
282+
const c = await engine.insert('zt_held', { name: 'C', parent: p.id });
283+
284+
const err = await refusalOf(engine.delete('zt_held', { where: { id: p.id } } as any));
285+
286+
expect(err).toMatchObject({ code: 'DELETE_RESTRICTED', status: 409, dependentObject: 'zt_held' });
287+
expect(driver.rowOf('zt_held', p.id)).toMatchObject({ name: 'P' });
288+
expect(driver.rowOf('zt_held', c.id)).toMatchObject({ name: 'C', parent: p.id });
289+
});
290+
291+
it('lets a leaf go: nothing references it', async () => {
292+
const { engine, driver } = await makeEngine([node]);
293+
const p = await engine.insert('zt_held', { name: 'P' });
294+
const c = await engine.insert('zt_held', { name: 'C', parent: p.id });
295+
296+
await engine.delete('zt_held', { where: { id: c.id } } as any);
297+
298+
expect(driver.rowOf('zt_held', c.id)).toBeUndefined();
299+
expect(driver.rowOf('zt_held', p.id)).toMatchObject({ name: 'P' });
300+
});
301+
});
302+
303+
describe('multiple: true', () => {
304+
it('is refused on a tree by the spec\'s parse, so no multi-valued tree reaches the walk through it', () => {
305+
const result = FieldSchema.safeParse({ type: 'tree', reference: 'zt_multi', multiple: true });
306+
expect(result.success).toBe(false);
307+
expect(result.error?.issues.some((i) => i.path.join('.') === 'multiple')).toBe(true);
308+
});
309+
310+
it('a raw registration around the parse is read single-valued, as the walk reads every type', async () => {
311+
// `isMultiValueField` is the walk's one multi-value arbiter, and a
312+
// `tree` is not a multi-capable type: the probe asks scalar equality
313+
// and `set_null` writes `null`, never a member-removed array.
314+
const { engine, driver } = await makeEngine([
315+
treeObject('zt_multi', { type: 'tree', reference: 'zt_multi', multiple: true }),
316+
]);
317+
const p = await engine.insert('zt_multi', { name: 'P' });
318+
const c = await engine.insert('zt_multi', { name: 'C', parent: p.id });
319+
320+
await engine.delete('zt_multi', { where: { id: p.id } } as any);
321+
322+
expect(driver.rowOf('zt_multi', p.id)).toBeUndefined();
323+
expect(driver.rowOf('zt_multi', c.id)).toMatchObject({ name: 'C', parent: null });
324+
});
325+
});
326+
327+
describe('CONTROLS — lookup and master_detail are unchanged', () => {
328+
const account = { name: 'zc_account', label: 'Account', fields: { id, name } };
329+
const ref = (fieldName: string, def: Record<string, unknown>) => ({ [fieldName]: { name: fieldName, ...def } });
330+
331+
it('lookup: the default clears, cascade deletes; master_detail: the default cascades', async () => {
332+
const { engine, driver } = await makeEngine([
333+
account,
334+
{ name: 'zc_cleared', label: 'Cleared', fields: { id, name, ...ref('account', FieldSchema.parse({ type: 'lookup', reference: 'zc_account' }) as Row) } },
335+
{ name: 'zc_cascaded', label: 'Cascaded', fields: { id, name, ...ref('account', { type: 'lookup', reference: 'zc_account', deleteBehavior: 'cascade' }) } },
336+
{ name: 'zc_detail', label: 'Detail', fields: { id, name, ...ref('account', { type: 'master_detail', reference: 'zc_account' }) } },
337+
]);
338+
const a = await engine.insert('zc_account', { name: 'A' });
339+
const cleared = await engine.insert('zc_cleared', { name: 'n', account: a.id });
340+
await engine.insert('zc_cascaded', { name: 'c', account: a.id });
341+
await engine.insert('zc_detail', { name: 'd', account: a.id });
342+
343+
await engine.delete('zc_account', { where: { id: a.id } } as any);
344+
345+
expect(driver.rowsOf('zc_account')).toHaveLength(0);
346+
expect(driver.rowOf('zc_cleared', cleared.id)).toMatchObject({ name: 'n', account: null });
347+
expect(driver.rowsOf('zc_cascaded')).toHaveLength(0);
348+
expect(driver.rowsOf('zc_detail')).toHaveLength(0);
349+
});
350+
351+
it.each([
352+
['lookup', 'zc_lookup_held'],
353+
['master_detail', 'zc_detail_held'],
354+
])('%s + restrict still refuses with the envelope and keeps the row', async (type, holder) => {
355+
const { engine, driver } = await makeEngine([
356+
account,
357+
{ name: holder, label: holder, fields: { id, name, ...ref('account', { type, reference: 'zc_account', deleteBehavior: 'restrict' }) } },
358+
]);
359+
const a = await engine.insert('zc_account', { name: 'A' });
360+
await engine.insert(holder, { name: 'h', account: a.id });
361+
362+
const err = await refusalOf(engine.delete('zc_account', { where: { id: a.id } } as any));
363+
364+
expect(err).toMatchObject({ code: 'DELETE_RESTRICTED', status: 409, dependentObject: holder });
365+
expect(driver.rowOf('zc_account', a.id)).toMatchObject({ name: 'A' });
366+
});
367+
368+
it('an object nothing references opens no transaction for its delete', async () => {
369+
// The `'none'` verdict of `planCascadeAtomicity`, unchanged: the plain
370+
// delete is the CONTROL the atomicity pin above is read against.
371+
const { engine, driver } = await makeEngine([account]);
372+
const a = await engine.insert('zc_account', { name: 'A' });
373+
374+
await engine.delete('zc_account', { where: { id: a.id } } as any);
375+
376+
expect(driver.rowsOf('zc_account')).toHaveLength(0);
377+
expect(driver.committed).toHaveLength(0);
378+
expect(driver.rolledBack).toHaveLength(0);
379+
});
380+
});
381+
});

0 commit comments

Comments
 (0)