Skip to content

Commit 74155c7

Browse files
os-zhuangclaude
andauthored
feat(spec,objectql): IDataEngine.find/findOne accept the author state — engine fills SortNode.order's declared default (#6300) (#7269)
ADR-0122's core argument — the first key an author writes must default correctly — now holds on the engine's primary read entry. find/findOne's query parameter flips from EngineQueryOptionsParsed (z.infer) back to EngineQueryOptions (z.input), the author-state shape count already took, and ObjectQL fills the one consumed default (SortNode.order → 'asc') by running each authored sort node through SortNodeSchema before the QueryAST is built — recursively through expand — so the declared default stays single-sourced in packages/spec. Phase-1 measured delta between the two states: orderBy[].order (the one default anything consumes — every driver already coalesced its absence to 'asc'), the three inert search flags (read by no executor, deleted from the AST unread), and their recursion through expand. context is .partial() (no delta); where/fields/limit/offset/top carry no defaults or transforms. Widening for typed callers; the one behavior change is for type-bypassing callers only: a malformed sort node (retired 'direction' spelling, unknown key) is refused with SortNodeSchema's own prescription instead of silently dropped-or-honored per driver (#4721's class), matching the wire path's normalizeSortNodes. Closes #6300 Claude-Session: https://claude.ai/code/session_011SaEx5461eovGX7AS9aLDV Co-authored-by: Claude <noreply@anthropic.com>
1 parent 7fa2aae commit 74155c7

7 files changed

Lines changed: 377 additions & 19 deletions

File tree

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
---
2+
'@objectstack/spec': minor
3+
'@objectstack/objectql': minor
4+
---
5+
6+
feat(spec,objectql): `IDataEngine.find`/`findOne` accept the author state — the engine fills `SortNode.order`'s declared default (#6300)
7+
8+
ADR-0122's core argument — "the first key an author writes must default
9+
correctly" — now holds on the engine's primary read entry:
10+
11+
```ts
12+
engine.find('task', { orderBy: [{ field: 'updated_at' }] }) // compiles; sorts asc
13+
engine.find('task', { search: { query: 'renewal' } }) // compiles uncast
14+
```
15+
16+
`find`/`findOne`'s `query` parameter flips from `EngineQueryOptionsParsed`
17+
(`z.infer`) to `EngineQueryOptions` (`z.input`) — the same author-state shape
18+
`count` already took. #6083 had pinned these two methods back to the parsed
19+
state because the engine built its `QueryAST` by bare spread and filled no
20+
default, so `order: undefined` would have reached drivers. The engine now runs
21+
each authored sort node through `SortNodeSchema` (recursively through
22+
`expand`) before the AST is built, so the declared default stays
23+
single-sourced in `packages/spec`.
24+
25+
**Widening, not breaking, for typed callers**: every previously-compiling call
26+
still compiles (`z.infer` values are valid `z.input`), and no query's answer
27+
changes — the measured driver-side status quo was that all drivers already
28+
coalesced a missing `order` to `'asc'`, the schema's declared default. The
29+
three defaulted `search` flags (`fuzzy`/`operator`/`highlight`) are
30+
`[EXPERIMENTAL — not enforced]`, read by no executor, and deleted from the AST
31+
before anything downstream sees it — so `search` is deliberately not parsed,
32+
which also keeps the wire-tolerated comma-string `search.fields` shape
33+
working.
34+
35+
**One behavior change, for type-BYPASSING callers only**: a malformed sort
36+
node smuggled past the type (`as any` / unparsed wire input) — the retired
37+
`direction` spelling, or an unknown key — is now refused with
38+
`SortNodeSchema`'s own prescription instead of being silently
39+
dropped-or-honored per driver (one query, two orders — #4721's defect class;
40+
the wire path's `normalizeSortNodes` already refused it). Write
41+
`{ field, order: 'asc' | 'desc' }`, or omit `order` for the default.
Lines changed: 224 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,224 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* #6300 — `find`/`findOne` take the AUTHOR state (`z.input`), and the engine
5+
* fills the defaults the schemas declare before the AST leaves it.
6+
*
7+
* ADR-0122's core argument is "the first key an author writes must default
8+
* correctly". `engine.find(obj, { orderBy: [{ field: 'updated_at' }] })` is
9+
* the natural spelling of "newest-ish first" — and until this card it did not
10+
* compile: #6083 pinned `find`/`findOne` back to `EngineQueryOptionsParsed`
11+
* (`z.infer`) because the engine built its `QueryAST` by bare spread and
12+
* filled no default, so admitting the author state would have sent
13+
* `order: undefined` to the driver.
14+
*
15+
* The measured driver-side status quo (part of #6300's own premise): every
16+
* driver coalesces a missing `order` to `'asc'` — `sql-driver.ts`
17+
* (`s.order || 'asc'`), `memory-driver.ts`, `mongodb-driver.ts`,
18+
* `mongodb-aggregation.ts`, `remote-transport.ts`. So the filled `'asc'`
19+
* changes no query's answer; what changes is that the AST now SAYS it, which
20+
* is what these pins hold:
21+
*
22+
* 1. the author-state calls in this file COMPILE WITHOUT A CAST — that is
23+
* the contract flip itself, pinned by `tsc`;
24+
* 2. the driver receives `order: 'asc'`, not `undefined` — the engine fills
25+
* the default rather than delegating it to per-driver tolerance;
26+
* 3. direction is right: defaulted ≡ explicit `'asc'`, ≢ explicit `'desc'`;
27+
* 4. the strictness the schema declares comes with its defaulting parse: a
28+
* type-bypassing malformed sort node is refused with the schema's own
29+
* prescription instead of being silently dropped-or-honored per driver
30+
* (#4721's defect class, already refused on the wire path).
31+
*/
32+
33+
import { describe, it, expect, beforeEach } from 'vitest';
34+
import type { IDataEngine } from '@objectstack/spec/contracts';
35+
import type { EngineQueryOptions } from '@objectstack/spec/data';
36+
import { ObjectQL } from './engine.js';
37+
38+
const account = {
39+
name: 'crm_account',
40+
label: 'Account',
41+
fields: {
42+
id: { name: 'id', type: 'text' as const, primaryKey: true },
43+
name: { name: 'name', type: 'text' as const },
44+
owner: { name: 'owner', type: 'lookup' as const, reference: 'person' },
45+
},
46+
};
47+
const person = {
48+
name: 'person',
49+
label: 'Person',
50+
fields: {
51+
id: { name: 'id', type: 'text' as const, primaryKey: true },
52+
name: { name: 'name', type: 'text' as const },
53+
},
54+
};
55+
56+
interface SeenRead { object: string; ast: any }
57+
58+
/** Memory driver recording the AST of every read (same shape as the #4419 suite's). */
59+
function makeRecordingDriver() {
60+
const stores = new Map<string, Map<string, Record<string, unknown>>>();
61+
const storeFor = (o: string) => { let s = stores.get(o); if (!s) { s = new Map(); stores.set(o, s); } return s; };
62+
const reads: SeenRead[] = [];
63+
let nextId = 0;
64+
const matches = (row: any, where: any): boolean => {
65+
if (!where || typeof where !== 'object') return true;
66+
for (const [k, v] of Object.entries(where)) {
67+
if (k === '$and') return (v as any[]).every((w) => matches(row, w));
68+
if (k === '$or') return (v as any[]).some((w) => matches(row, w));
69+
if (k.startsWith('$')) continue;
70+
if (v && typeof v === 'object' && '$in' in (v as any)) {
71+
if (!(v as any).$in.map(String).includes(String(row[k]))) return false;
72+
continue;
73+
}
74+
if (v && typeof v === 'object' && '$contains' in (v as any)) {
75+
const needle = String((v as any).$contains).toLowerCase();
76+
if (!String(row[k] ?? '').toLowerCase().includes(needle)) return false;
77+
continue;
78+
}
79+
const exp = (v && typeof v === 'object' && '$eq' in (v as any)) ? (v as any).$eq : v;
80+
if ((row[k] ?? null) !== (exp ?? null)) return false;
81+
}
82+
return true;
83+
};
84+
const run = (o: string, ast: any) => {
85+
let rows = Array.from(storeFor(o).values()).filter((r) => matches(r, ast?.where));
86+
const ord = Array.isArray(ast?.orderBy) ? ast.orderBy : [];
87+
if (ord.length > 0) {
88+
rows = [...rows].sort((a: any, b: any) => {
89+
for (const { field, order } of ord) {
90+
const cmp = String(a?.[field] ?? '').localeCompare(String(b?.[field] ?? ''));
91+
if (cmp !== 0) return order === 'desc' ? -cmp : cmp;
92+
}
93+
return 0;
94+
});
95+
}
96+
return typeof ast?.limit === 'number' && ast.limit > 0 ? rows.slice(0, ast.limit) : rows;
97+
};
98+
const driver: any = {
99+
name: 'memory', version: '0.0.0', supports: {},
100+
async connect() {}, async disconnect() {}, async checkHealth() { return true; }, async execute() { return null; },
101+
async find(o: string, ast: any) { reads.push({ object: o, ast }); return run(o, ast); },
102+
async findOne(o: string, ast: any) { reads.push({ object: o, ast }); return run(o, ast)[0] ?? null; },
103+
async create(o: string, data: Record<string, unknown>) {
104+
nextId += 1; const id = (data.id as string) ?? `r_${nextId}`; const row = { ...data, id }; storeFor(o).set(id, row); return row;
105+
},
106+
async update(o: string, id: string, data: Record<string, unknown>) {
107+
const s = storeFor(o); const cur = s.get(id); if (!cur) throw new Error(`nf ${o}/${id}`);
108+
const up = { ...cur, ...data, id }; s.set(id, up); return up;
109+
},
110+
async delete(o: string, id: string) { return storeFor(o).delete(id); },
111+
async count(o: string, ast: any) { return run(o, ast).length; },
112+
async bulkCreate(o: string, rows: Record<string, unknown>[]) { return Promise.all(rows.map((r) => this.create(o, r))); },
113+
async beginTransaction() { return { commit: async () => {}, rollback: async () => {} }; }, async commit() {}, async rollback() {},
114+
};
115+
return { driver, reads };
116+
}
117+
118+
describe('find/findOne accept the author state and the engine fills the declared defaults (#6300)', () => {
119+
let engine: ObjectQL;
120+
let reads: SeenRead[];
121+
122+
beforeEach(async () => {
123+
engine = new ObjectQL();
124+
const mem = makeRecordingDriver();
125+
reads = mem.reads;
126+
engine.registerDriver(mem.driver, true);
127+
await engine.init();
128+
engine.registry.registerObject(account);
129+
engine.registry.registerObject(person);
130+
const alice = await engine.insert('person', { name: 'Alice' });
131+
const bob = await engine.insert('person', { name: 'Bob' });
132+
// Names chosen so ascending ≠ descending ≠ insertion order.
133+
await engine.insert('crm_account', { name: 'Beta', owner: bob.id });
134+
await engine.insert('crm_account', { name: 'Alpha', owner: alice.id });
135+
await engine.insert('crm_account', { name: 'Gamma', owner: alice.id });
136+
reads.length = 0;
137+
});
138+
139+
// ── (1) The contract flip, pinned by the compiler ────────────────────────
140+
// Every call in this block is UNCAST. Under #6083's `...Parsed` parameter
141+
// none of them compiled — `orderBy[].order` was required to write. The
142+
// `IDataEngine`-typed alias pins the spec contract, not just the class.
143+
144+
it('an orderBy without `order` compiles against IDataEngine and sorts ascending', async () => {
145+
const dataEngine: IDataEngine = engine;
146+
const rows = await dataEngine.find('crm_account', { orderBy: [{ field: 'name' }] });
147+
expect(rows.map((r: any) => r.name)).toEqual(['Alpha', 'Beta', 'Gamma']);
148+
});
149+
150+
it('an object-form `search` without the flag keys compiles uncast and matches', async () => {
151+
// `EngineQueryOptionsParsed['search']` required `fuzzy`/`operator`/
152+
// `highlight` (parse-time defaults); the author state makes them
153+
// optional — which is the truth, since no executor reads them (#4286).
154+
const dataEngine: IDataEngine = engine;
155+
const rows = await dataEngine.find('crm_account', { search: { query: 'Beta' } });
156+
expect(rows.map((r: any) => r.name)).toEqual(['Beta']);
157+
});
158+
159+
// ── (2) The engine fills the default — `undefined` stops reaching drivers ─
160+
161+
it("the driver receives order: 'asc', not undefined", async () => {
162+
await engine.find('crm_account', { orderBy: [{ field: 'name' }] });
163+
const { ast } = reads.at(-1)!;
164+
expect(ast.orderBy).toEqual([{ field: 'name', order: 'asc' }]);
165+
});
166+
167+
it('a nested expand query is the same authoring surface, filled on its own read', async () => {
168+
await engine.find('crm_account', {
169+
where: { name: 'Alpha' },
170+
expand: { owner: { object: 'person', orderBy: [{ field: 'name' }] } },
171+
});
172+
const personRead = reads.find((r) => r.object === 'person');
173+
expect(personRead).toBeTruthy();
174+
expect(personRead!.ast.orderBy).toEqual([{ field: 'name', order: 'asc' }]);
175+
});
176+
177+
// ── (3) Direction, predicted first ───────────────────────────────────────
178+
// Prediction (written before execution): the defaulted spelling behaves as
179+
// the schema's declared `'asc'` — identical to explicit-asc, and the exact
180+
// reverse of explicit-desc on this tie-free fixture.
181+
182+
it("defaulted ≡ explicit 'asc', ≢ explicit 'desc'", async () => {
183+
const defaulted = await engine.find('crm_account', { orderBy: [{ field: 'name' }] });
184+
const explicitAsc = await engine.find('crm_account', { orderBy: [{ field: 'name', order: 'asc' }] });
185+
const explicitDesc = await engine.find('crm_account', { orderBy: [{ field: 'name', order: 'desc' }] });
186+
expect(defaulted.map((r: any) => r.name)).toEqual(explicitAsc.map((r: any) => r.name));
187+
expect(defaulted.map((r: any) => r.name)).toEqual([...explicitDesc.map((r: any) => r.name)].reverse());
188+
expect(explicitDesc.map((r: any) => r.name)).toEqual(['Gamma', 'Beta', 'Alpha']);
189+
});
190+
191+
it('findOne: an order-less orderBy is a legal #4419 predicate and answers the FIRST-ascending row', async () => {
192+
const dataEngine: IDataEngine = engine;
193+
const row = await dataEngine.findOne('crm_account', { orderBy: [{ field: 'name' }] });
194+
expect(row?.name).toBe('Alpha');
195+
const { ast } = reads.at(-1)!;
196+
expect(ast.orderBy).toEqual([{ field: 'name', order: 'asc' }]);
197+
expect(ast.limit).toBe(1);
198+
});
199+
200+
// ── (4) The schema's strictness rides with its defaulting parse ──────────
201+
// These callers bypass the type (`as unknown as EngineQueryOptions` — the
202+
// #4918 spelling for a DELIBERATELY off-contract probe), which is the only
203+
// way these shapes can occur. Before #6300 the engine forwarded them
204+
// verbatim and each driver decided alone: memory honored `direction`,
205+
// SQL/Mongo silently dropped it and sorted ascending — one query, two
206+
// orders (#4721's class).
207+
208+
it("the retired `direction` spelling is refused with the schema's rename prescription", async () => {
209+
const offContract = { orderBy: [{ field: 'name', direction: 'desc' }] } as unknown as EngineQueryOptions;
210+
await expect(engine.find('crm_account', offContract)).rejects.toThrow(/order/);
211+
});
212+
213+
it('an unknown sort-node key is refused by name, not silently dropped', async () => {
214+
const offContract = { orderBy: [{ field: 'name', frobnicate: true }] } as unknown as EngineQueryOptions;
215+
await expect(engine.find('crm_account', offContract)).rejects.toThrow(/frobnicate/);
216+
});
217+
218+
it('an explicit `order` is never clobbered by the fill', async () => {
219+
const rows = await engine.find('crm_account', { orderBy: [{ field: 'name', order: 'desc' }] });
220+
expect(rows.map((r: any) => r.name)).toEqual(['Gamma', 'Beta', 'Alpha']);
221+
const { ast } = reads.at(-1)!;
222+
expect(ast.orderBy).toEqual([{ field: 'name', order: 'desc' }]);
223+
});
224+
});

packages/objectql/src/engine-filter-array-lowering.test.ts

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -29,28 +29,30 @@ import { describe, it, expect, beforeEach } from 'vitest';
2929
import type {
3030
EngineAggregateOptions,
3131
EngineCountOptions,
32-
EngineQueryOptionsParsed,
32+
EngineQueryOptions,
3333
} from '@objectstack/spec/data';
3434
import { ObjectQL } from './engine.js';
3535

3636
/**
3737
* [#4918] `FilterArray` on `where` is off-contract BY DECLARATION, and these
38-
* tests exist to drive it: `EngineQueryOptionsParsed.where` is a `FilterCondition` /
38+
* tests exist to drive it: `EngineQueryOptions.where` is a `FilterCondition` /
3939
* `Record< string, unknown >`, which an array is not assignable to, because
4040
* `FilterArray` is INPUT-ONLY authoring sugar the spec deliberately excludes
4141
* (#5285). So a test that hands the engine one has to say so, and
42-
* `as unknown as EngineQueryOptionsParsed` is how: it names the contract being
42+
* `as unknown as EngineQueryOptions` is how: it names the contract being
4343
* bypassed, keeps the rest of the call type-checked, and greps as an
44-
* intentional act — none of which a bare `as any` does.
44+
* intentional act — none of which a bare `as any` does. (#6300 flipped the
45+
* find/findOne parameter from `EngineQueryOptionsParsed` to the author-state
46+
* `EngineQueryOptions`; the cast target follows the contract it names.)
4547
*
4648
* Deliberately NOT used for the malformed-COMPARAND cases below
4749
* (`{ stage: { $nin: 'won' } }`). Those are ordinary objects that `tsc`
4850
* accepts, because `where` is declared loosely on purpose — which is the whole
4951
* reason the runtime gate this file pins has to exist. Erasing them would hide
5052
* that they are type-legal, which is the point.
5153
*/
52-
const asFilterArrayQuery = (where: unknown): EngineQueryOptionsParsed =>
53-
({ where }) as unknown as EngineQueryOptionsParsed;
54+
const asFilterArrayQuery = (where: unknown): EngineQueryOptions =>
55+
({ where }) as unknown as EngineQueryOptions;
5456

5557
const deal = {
5658
name: 'deal',

0 commit comments

Comments
 (0)