Skip to content

Commit d09dffd

Browse files
committed
fix(spec): a tree's reference: '' keeps one reading; pin the write check, expand and nested filter at the door
materializeTreeSelfReferences writes only an undefined or null reference. An empty string is refused by the parse as a foreign target, so the registry leaves it as written instead of repairing it unparsed. Claude-Session: https://claude.ai/code/session_01ADzJtzYTLUfgrRZHxkagkX Co-authored-by: Claude <noreply@anthropic.com>
1 parent b390b3c commit d09dffd

4 files changed

Lines changed: 92 additions & 17 deletions

File tree

‎packages/objectql/src/engine-cascade-delete-tree.test.ts‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@
4848
*/
4949

5050
import { describe, it, expect } from 'vitest';
51-
import { FieldSchema, ObjectSchema } from '@objectstack/spec/data';
51+
import { FieldSchema, ObjectSchema, referenceTargetOf } from '@objectstack/spec/data';
5252
import { ObjectQL } from './engine.js';
5353

5454
type Row = Record<string, unknown>;
@@ -494,6 +494,17 @@ describe('[#23065] a tree field that names no reference is a reference to its ow
494494
expect(driver.rolledBack).toHaveLength(0);
495495
});
496496

497+
it('an empty-string reference is not written over: it reaches the registry as written, and names nothing', async () => {
498+
// The parse refuses `reference: ''` on a tree (`refuseForeignTreeReference`),
499+
// so the registry, which never parses, must not repair it into a valid
500+
// self-reference: the two doors give the shape one reading. The carrier
501+
// reader then reads it as naming nothing, as it did before.
502+
const { engine } = await makeEngine([treeObject('zr_empty', { type: 'tree', reference: '' })]);
503+
const parent = (engine.registry.getObject('zr_empty') as any)?.fields?.parent;
504+
expect(parent?.reference).toBe('');
505+
expect(referenceTargetOf(parent)).toBeUndefined();
506+
});
507+
497508
it('CONTROL: a tree that wrote its own name registers exactly as written', async () => {
498509
const { engine } = await makeEngine([treeObject('zr_written', { type: 'tree', reference: 'zr_written' })]);
499510
expect((engine.registry.getObject('zr_written') as any)?.fields?.parent?.reference).toBe('zr_written');

‎packages/spec/src/data/object.zod.ts‎

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1670,11 +1670,20 @@ function refuseForeignTreeReference(ownName: unknown, fields: unknown, ctx: z.Re
16701670
* save stores the authored body verbatim and a raw registration is never
16711671
* parsed, and both reach the registry without the parse's output.
16721672
*
1673-
* Absence is what `referenceCarrierOf` reads as absence — `undefined`, `null`
1674-
* or `''` — so the carrier reader and this materialization agree on which
1675-
* fields name nothing. A present value is returned untouched: a foreign one is
1676-
* `refuseForeignTreeReference`'s to refuse, and a non-string one is the carrier
1677-
* reader's to refuse loudly. The key lands at its `FieldSchema` shape position,
1673+
* Absence is the two spellings a contract admits for "names no target":
1674+
* `undefined` (what `FieldSchema`'s `.optional()` admits) and `null` (what the
1675+
* blueprint's `StrictField` admits). Every other value is returned untouched,
1676+
* so the two doors cannot disagree about it:
1677+
*
1678+
* - `''` is NOT absence here. The parse refuses it on a `tree` as a foreign
1679+
* target (`refuseForeignTreeReference` passes only an absent key or the own
1680+
* name), so the registry leaves it as written rather than repairing,
1681+
* unparsed, a value the contract refuses; the carrier reader then reads it
1682+
* as naming nothing, exactly as before this helper existed;
1683+
* - a foreign name is `refuseForeignTreeReference`'s to refuse, and a
1684+
* non-string value is the carrier reader's to refuse loudly.
1685+
*
1686+
* The key lands at its `FieldSchema` shape position,
16781687
* so a parsed referenceless `tree` is byte-identical to one that wrote its own
16791688
* name, and a re-parse is a no-op.
16801689
*
@@ -1691,7 +1700,7 @@ export function materializeTreeSelfReferences<F>(ownName: unknown, fields: F): F
16911700
for (const [fieldName, def] of Object.entries(fields as Record<string, unknown>)) {
16921701
if (def === null || typeof def !== 'object') continue;
16931702
const { type, reference } = def as { type?: unknown; reference?: unknown };
1694-
if (type !== 'tree' || !(reference === undefined || reference === null || reference === '')) continue;
1703+
if (type !== 'tree' || !(reference === undefined || reference === null)) continue;
16951704
stamped ??= { ...(fields as Record<string, unknown>) };
16961705
stamped[fieldName] = withReferenceAtShapePosition(def as Record<string, unknown>, ownName);
16971706
}

‎packages/spec/src/data/tree-reference-self-only.test.ts‎

Lines changed: 23 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -221,31 +221,44 @@ describe('a referenceless `tree` is written as a self-reference by the doors tha
221221
expect(parsed.fields.f_text).not.toHaveProperty('reference');
222222
});
223223

224-
it('the helper: absence is the carrier reader\'s absence, a present value is untouched, nothing to write allocates nothing', () => {
224+
it('the helper: absence is `undefined` or `null`, every other value is untouched, nothing to write allocates nothing', () => {
225225
const untouched = { name: { type: 'text' }, parent: { type: 'tree', reference: 'node' } };
226226
expect(materializeTreeSelfReferences('node', untouched)).toBe(untouched);
227227

228-
const absent = {
228+
const fields = {
229229
a: { type: 'tree' },
230230
b: { type: 'tree', reference: null },
231231
c: { type: 'tree', reference: '' },
232232
d: { type: 'tree', reference: { object: 'node' } },
233233
e: { type: 'lookup' },
234234
};
235-
const out = materializeTreeSelfReferences('node', absent) as Record<string, { reference?: unknown }>;
236-
expect(out).not.toBe(absent);
237-
expect([out.a.reference, out.b.reference, out.c.reference]).toEqual(['node', 'node', 'node']);
235+
const out = materializeTreeSelfReferences('node', fields) as Record<string, { reference?: unknown }>;
236+
expect(out).not.toBe(fields);
237+
expect([out.a.reference, out.b.reference]).toEqual(['node', 'node']);
238+
// `''` is not absence here: the parse refuses it on a tree (the pin below),
239+
// so the helper leaves it as written instead of repairing it unparsed.
240+
expect(out.c).toBe(fields.c);
238241
// A non-string carrier is the carrier reader's to refuse, loudly — never
239242
// overwritten here, where it would read as a valid self-reference.
240-
expect(out.d.reference).toEqual({ object: 'node' });
241-
expect(out.e).toBe(absent.e);
243+
expect(out.d).toBe(fields.d);
244+
expect(out.e).toBe(fields.e);
242245
// The input is not mutated.
243-
expect(absent.a).toEqual({ type: 'tree' });
246+
expect(fields.a).toEqual({ type: 'tree' });
247+
// Only an empty string to look at: nothing is written, nothing allocated.
248+
const onlyEmpty = { parent: { type: 'tree', reference: '' } };
249+
expect(materializeTreeSelfReferences('node', onlyEmpty)).toBe(onlyEmpty);
244250

245251
// No name, or no field map, is nothing to write against.
246-
expect(materializeTreeSelfReferences(undefined, absent)).toBe(absent);
247-
expect(materializeTreeSelfReferences('', absent)).toBe(absent);
252+
expect(materializeTreeSelfReferences(undefined, fields)).toBe(fields);
253+
expect(materializeTreeSelfReferences('', fields)).toBe(fields);
248254
const list = [{ name: 'parent', type: 'tree' }];
249255
expect(materializeTreeSelfReferences('node', list)).toBe(list);
250256
});
257+
258+
it('`reference: \'\'` on a tree has one reading: the parse refuses it, and nothing writes over it', () => {
259+
const parsed = ObjectSchema.safeParse(zoo(''));
260+
expect(parsed.success).toBe(false);
261+
if (parsed.success) return;
262+
expect(parsed.error.issues.map((i) => i.path)).toEqual([['fields', 'f_tree', 'reference']]);
263+
});
251264
});

‎packages/verify/src/delete-behavior-referenceless-tree.test.ts‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,17 @@
2727
// through both of its doors, instead of excluding it as "names no target";
2828
// - CONTROLS: a tree that writes its own name, a `lookup` and a
2929
// `master_detail` answer exactly as they always did.
30+
//
31+
// And the other readers of the same target, at the record doors (measured on
32+
// `origin/main` `275ad4229a` before, on this stack after):
33+
// - the write-time reference check: a parent id that names no record is
34+
// refused `400` with `reference_not_found` (it was `201`, accepted) — the
35+
// one refusal this change adds at a data door; CONTROL: an existing parent
36+
// is accepted;
37+
// - `expand=parent` resolves the parent record (it was `404`, "declares no
38+
// target object");
39+
// - a nested relation condition `{ parent: { title } }` filters through the
40+
// parent (it was `400`, "declares no related object").
3041

3142
import { describe, it, expect, beforeAll, afterAll } from 'vitest';
3243

@@ -176,6 +187,37 @@ describe('[#23065] a referenceless tree field — delete behaviour at the public
176187
}
177188
});
178189

190+
it('the write check: a parent id that names no record is refused 400 reference_not_found; an existing parent is accepted', async () => {
191+
const refused = await stack.apiAs(token, 'POST', '/data/rtl_default', { title: 'orphan', parent: 'no_such_parent_id' });
192+
const body = JSON.parse(await refused.text()) as { code?: string; fields?: Array<{ field?: string; code?: string }> };
193+
expect(refused.status).toBe(400);
194+
expect(body.code).toBe('VALIDATION_FAILED');
195+
expect(body.fields).toEqual(
196+
expect.arrayContaining([expect.objectContaining({ field: 'parent', code: 'reference_not_found' })]),
197+
);
198+
199+
// CONTROL: a parent that exists is accepted, and stored as written.
200+
const p = await create('rtl_default', { title: 'p' });
201+
const c = await create('rtl_default', { title: 'c', parent: p });
202+
expect((await read('rtl_default', c)).record?.parent).toBe(p);
203+
});
204+
205+
it('expand=parent resolves the parent record, and a nested relation condition filters through it', async () => {
206+
const p = await create('rtl_default', { title: 'expand-parent' });
207+
const c = await create('rtl_default', { title: 'expand-child', parent: p });
208+
209+
const expanded = await stack.apiAs(token, 'GET', `/data/rtl_default/${c}?expand=parent`);
210+
const record = (JSON.parse(await expanded.text()) as { record?: { parent?: { id?: unknown; title?: unknown } } }).record;
211+
expect(expanded.status).toBe(200);
212+
expect(record?.parent).toMatchObject({ id: p, title: 'expand-parent' });
213+
214+
const filter = encodeURIComponent(JSON.stringify({ parent: { title: 'expand-parent' } }));
215+
const listed = await stack.apiAs(token, 'GET', `/data/rtl_default?filter=${filter}`);
216+
const rows = (JSON.parse(await listed.text()) as { records?: Array<{ id?: unknown }> }).records;
217+
expect(listed.status).toBe(200);
218+
expect(rows?.map((r) => r.id)).toEqual([c]);
219+
});
220+
179221
it('CONTROLS: a tree that writes its own name, a lookup and a master_detail answer as before', async () => {
180222
const p = await create('rtl_written', { title: 'p' });
181223
const c = await create('rtl_written', { title: 'c', parent: p });

0 commit comments

Comments
 (0)