Skip to content

Commit fb1a1e6

Browse files
committed
fix(objectql,driver-sql,driver-memory,driver-mongodb)!: lower FilterArray at the engine door, delete the drivers' array dialect (#5158)
Maintainer ruling C, step 2. `FilterArray` is INPUT-ONLY authoring sugar and the spec has said so since #5285; this makes the runtime agree. Door 2 — a direct in-process engine call — used to pass a `where` array through verbatim, so four drivers grew a second filter compiler to meet it, including an INFIX dialect (`[condA, 'or', condB]`) no schema declared and `parseFilterAST` cannot express. Cloud's `RemoteTransport.buildWhereSQL` has refused that same input since cloud#1075: one query, two answers, decided by whether the caller went over the wire. All six engine entry points (`find`/`findOne`/`count`/`aggregate`/`update`/ `delete`) now run `isFilterAST` -> `parseFilterAST`, the same sink the protocol face has always used, and refuse an array that cannot be lowered. `[]` keeps its meaning everywhere: no filter. The drivers' array compilers are deleted and replaced by a loud `INVALID_FILTER` / 400 refusal that names the lowering — converging driver-sql, driver-sqlite-wasm, driver-memory and driver-mongodb with cloud's transport. Deliberately retired: `engine-findone-contract`'s pin that a non-object `where` "is the driver's to interpret". That sentence was the engine's blessing of the second dialect, and it cost `findOne({ where: [] })` walking past #4419's own guard to return an arbitrary row. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Pbu27iNUfQCHeuS551Rqo7
1 parent 0f17114 commit fb1a1e6

23 files changed

Lines changed: 997 additions & 576 deletions
Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
---
2+
"@objectstack/objectql": minor
3+
"@objectstack/driver-sql": minor
4+
"@objectstack/driver-memory": minor
5+
"@objectstack/driver-mongodb": minor
6+
---
7+
8+
fix(objectql,driver-sql,driver-memory,driver-mongodb)!: `FilterArray` 在 engine 门下沉,四驱动的数组方言删除 (#5158 拍板 C 第 2 步)
9+
10+
`FilterArray` —— `['stage','=','won']``['and', […], […]]``[[…], […]]` —— 是**仅输入**
11+
授权糖。#5285 已在 spec 里把这件事写明(`data/filter.zod.ts`,`filter-array-declaration.test.ts`
12+
钉住「被声明」且「`where` 不接受它」)。本次是拍板 C 的第 2 步:让**运行时**与那份声明一致。
13+
14+
## 改了什么
15+
16+
进入运行时的门有两扇,过去只有一扇按契约读:
17+
18+
|| 改前 | 改后 |
19+
|---|---|---|
20+
| **Door 1** —— 协议/HTTP 面(`metadata-protocol`) | `isFilterAST``parseFilterAST`,不可下沉的数组答 `400 INVALID_FILTER` | 不变 |
21+
| **Door 2** —— 进程内 engine 直调(`ObjectQL.find`/`findOne`/`count`/`aggregate`/`update`/`delete`) | 数组**原样**透传给驱动 |**同一条缝**:`isFilterAST``parseFilterAST` 下沉为 `FilterCondition`,不可下沉的数组响亮拒收 |
22+
| 四驱动(`driver-sql`、继承它的 `driver-sqlite-wasm``driver-memory``driver-mongodb`) | 各自带**第二套过滤器编译器**,包括一种**中缀**方言(`[condA, 'or', condB]`)—— 没有任何 schema 声明过它,`parseFilterAST` 也表达不了它 | 数组方言删除;数组到达驱动即 `INVALID_FILTER` / 400 |
23+
24+
一个查询两套编译器正是 ADR-0053 D-A1 禁止的分叉,而且它已经产生了真实的产品分叉:cloud
25+
`RemoteTransport.buildWhereSQL`cloud#1075 起对**同一输入**响亮拒收,`driver-sql` 却编译它。
26+
删掉方言后两侧自然合流。
27+
28+
## 授权面:零变化
29+
30+
`FilterBuilder`(`@objectstack/client`)产出的元组与 `['and', ...]` 组、React block 的
31+
`filters` prop、wire 的 `$filter` 面、showcase 的授权点 —— **全部原样工作**,因为下沉正是
32+
这些形状本来的用途。wire 契约逐字节不变(Door 1 的行为未改)。
33+
34+
## ⚠️ 可观察的行为变更
35+
36+
1. **中缀连接不再被编译。** `where: [condA, 'or', condB]` 过去只有驱动认识,现在在 engine 门被拒收。
37+
声明的写法是前缀组:`['or', condA, condB]` —— 语义相同,`parseFilterAST` 有它的下沉。
38+
2. **`findOne({ where: [] })` 现在抛错。** `[]` 的含义**没有变**(仍是「无过滤」,`find`/`count`
39+
照旧返回/计数全部行)。变的是 `findOne` 终于**看得见**这一点:未下沉的 `[]` 过去被
40+
`requireFindOnePredicate` 当作「驱动自己去解释的表达式树」放行,于是 `limit: 1` 落在整张表上,
41+
返回**任意一行** —— 正是 #4419 要挡的缺陷,活在 #4419 自己的守卫里面。
42+
3. **不可下沉的数组在 engine 门拒收,不再由驱动拒收。** 形状与操作符词表相同(`isFilterAST` 同一套),
43+
变的是消息来自调用点、带上调用方自己的值,以及明说「过滤器没有被应用,否则会返回**未过滤**的结果集」。
44+
4. **驱动直调者(不经 engine)受影响。** `SqlDriver` / `InMemoryDriver` / `translateFilter` 是公开
45+
导出;把数组 `where` 直接喂给它们的调用方需要改为先 `parseFilterAST(...)` 再传,或改走 ObjectQL。
46+
注意 `QueryAST.where``FilterCondition` 是索引签名类型,数组对它是**可赋值**的 —— 类型层从未
47+
挡住这个输入,所以拒收必须在运行时。
48+
5. **`driver-mongodb``createdAt``created_at` 字段别名随方言一起消失。** 它只存在于数组路径
49+
(`mapFieldName`,仅被已删除的 `translateComparison` 调用),对象路径从未应用过它。消费端别名按
50+
AGENTS.md PD #12 是债务而非模式,故不再补回:请写声明的字段名 `created_at`
51+
52+
## 删除的代码面
53+
54+
- `SqlDriver.applyFilters` 的数组遍历分支,及其比较发射器 `protected applyAstComparison`(约 220 行)
55+
- `InMemoryDriver.convertToMongoQuery` 的 legacy array 分支(约 62 行)
56+
- `driver-mongodb` `mongodb-filter.ts``translateArrayFilter` / `translateComparison` / `mapFieldName`(约 140 行)
57+
- `driver-sqlite-wasm` 无自有实现,随 `SqlDriver` 继承变更
58+
59+
`[]` 在每一层的读法**都不变**:engine 删键、`parseFilterAST([])``undefined`、三个驱动都提前返回。
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+
* #5158 — Door 2 lowers `FilterArray` before any driver sees it.
5+
*
6+
* `FilterArray` (`['stage','=','won']`, `['and', […], […]]`, `[[…], […]]`) is
7+
* INPUT-ONLY authoring sugar. The spec says so since #5285
8+
* (`data/filter.zod.ts`, pinned by `filter-array-declaration.test.ts`): it is
9+
* declared, and `QuerySchema.where` deliberately excludes it.
10+
*
11+
* Two doors led into the runtime and only one read the contract that way. The
12+
* protocol face (Door 1, `metadata-protocol/protocol.ts`) has always run
13+
* `isFilterAST` → `parseFilterAST` and answered `400 INVALID_FILTER` for an
14+
* array it could not lower. A direct engine call (Door 2) passed the array
15+
* through verbatim, so four drivers grew a SECOND filter compiler to meet it —
16+
* including an infix dialect (`[condA, 'or', condB]`) the spec never declared
17+
* and `parseFilterAST` cannot express, which cloud's
18+
* `RemoteTransport.buildWhereSQL` refuses outright. Same query, two answers,
19+
* decided by whether the caller went over the wire.
20+
*
21+
* Maintainer ruling C on #5158 closed Door 2 onto Door 1's sink. These tests
22+
* assert on the AST the DRIVER RECEIVES, not on the returned rows: identical
23+
* rows are the whole point of the change, so a row assertion cannot tell a
24+
* lowered filter from an unlowered one. That distinction is what makes the
25+
* driver-side dialect deletion in this same PR safe.
26+
*/
27+
28+
import { describe, it, expect, beforeEach } from 'vitest';
29+
import { ObjectQL } from './engine.js';
30+
31+
const deal = {
32+
name: 'deal',
33+
label: 'Deal',
34+
fields: {
35+
id: { name: 'id', type: 'text' as const, primaryKey: true },
36+
stage: { name: 'stage', type: 'text' as const },
37+
amount: { name: 'amount', type: 'number' as const },
38+
owner_id: { name: 'owner_id', type: 'text' as const },
39+
},
40+
};
41+
42+
interface SeenRead { ast: any }
43+
44+
/**
45+
* Minimal driver that records every AST it is handed and executes only the
46+
* `FilterCondition` object form — deliberately. A driver that also understood
47+
* the array form could not witness the lowering, which is the bug this file
48+
* pins: the four in-repo drivers understood BOTH, so nothing downstream ever
49+
* had to notice which one it got.
50+
*/
51+
function makeRecordingDriver() {
52+
const rows = new Map<string, Record<string, unknown>>();
53+
const reads: SeenRead[] = [];
54+
const writes: SeenRead[] = [];
55+
const matches = (row: any, where: any): boolean => {
56+
if (where == null) return true;
57+
if (Array.isArray(where) || typeof where !== 'object') {
58+
throw new Error(
59+
`driver received a non-object 'where' (${JSON.stringify(where)}) — the engine must ` +
60+
'lower FilterArray before the driver (#5158)',
61+
);
62+
}
63+
for (const [k, v] of Object.entries(where)) {
64+
if (k === '$and') { if (!(v as any[]).every((w) => matches(row, w))) return false; continue; }
65+
if (k === '$or') { if (!(v as any[]).some((w) => matches(row, w))) return false; continue; }
66+
if (v && typeof v === 'object' && !Array.isArray(v)) {
67+
const ops = v as Record<string, unknown>;
68+
if ('$gt' in ops && !((row[k] as any) > (ops.$gt as any))) return false;
69+
if ('$gte' in ops && !((row[k] as any) >= (ops.$gte as any))) return false;
70+
if ('$lt' in ops && !((row[k] as any) < (ops.$lt as any))) return false;
71+
if ('$lte' in ops && !((row[k] as any) <= (ops.$lte as any))) return false;
72+
if ('$ne' in ops && row[k] === ops.$ne) return false;
73+
if ('$in' in ops && !(ops.$in as unknown[]).includes(row[k])) return false;
74+
if ('$null' in ops && (row[k] == null) !== ops.$null) return false;
75+
continue;
76+
}
77+
if (row[k] !== v) return false;
78+
}
79+
return true;
80+
};
81+
const run = (ast: any) => {
82+
const out = [...rows.values()].filter((r) => matches(r, ast?.where));
83+
return typeof ast?.limit === 'number' && ast.limit > 0 ? out.slice(0, ast.limit) : out;
84+
};
85+
const driver: any = {
86+
name: 'recording', version: '0.0.0', supports: {},
87+
async connect() {}, async disconnect() {}, async checkHealth() { return true; }, async execute() { return null; },
88+
async find(_o: string, ast: any) { reads.push({ ast }); return run(ast); },
89+
async findOne(_o: string, ast: any) { reads.push({ ast }); return run(ast)[0] ?? null; },
90+
async count(_o: string, ast: any) { reads.push({ ast }); return run(ast).length; },
91+
async aggregate(_o: string, ast: any) { reads.push({ ast }); return run(ast); },
92+
async create(_o: string, data: Record<string, unknown>) {
93+
const id = (data.id as string) ?? `r_${rows.size + 1}`;
94+
const row = { ...data, id }; rows.set(id, row); return row;
95+
},
96+
async update(_o: string, id: string, data: Record<string, unknown>) {
97+
const cur = rows.get(id); if (!cur) throw new Error(`nf ${id}`);
98+
const up = { ...cur, ...data, id }; rows.set(id, up); return up;
99+
},
100+
async updateMany(_o: string, ast: any, data: Record<string, unknown>) {
101+
writes.push({ ast });
102+
const hit = run(ast);
103+
for (const r of hit) rows.set(r.id as string, { ...r, ...data });
104+
return hit.length;
105+
},
106+
async delete(_o: string, id: string) { return rows.delete(id); },
107+
async deleteMany(_o: string, ast: any) {
108+
writes.push({ ast });
109+
const hit = run(ast);
110+
for (const r of hit) rows.delete(r.id as string);
111+
return hit.length;
112+
},
113+
async bulkCreate(o: string, batch: Record<string, unknown>[]) {
114+
return Promise.all(batch.map((r) => this.create(o, r)));
115+
},
116+
async beginTransaction() { return { commit: async () => {}, rollback: async () => {} }; },
117+
async commit() {}, async rollback() {},
118+
};
119+
return { driver, reads, writes };
120+
}
121+
122+
describe('Door 2 lowers FilterArray to FilterCondition before the driver (#5158)', () => {
123+
let engine: ObjectQL;
124+
let reads: SeenRead[];
125+
let writes: SeenRead[];
126+
127+
beforeEach(async () => {
128+
const rec = makeRecordingDriver();
129+
reads = rec.reads;
130+
writes = rec.writes;
131+
engine = new ObjectQL();
132+
engine.registerDriver(rec.driver, true);
133+
await engine.init();
134+
engine.registry.registerObject(deal as any);
135+
await engine.insert('deal', { id: 'd1', stage: 'won', amount: 10, owner_id: 'u1' });
136+
await engine.insert('deal', { id: 'd2', stage: 'lost', amount: 20, owner_id: 'u2' });
137+
await engine.insert('deal', { id: 'd3', stage: 'won', amount: 30, owner_id: 'u1' });
138+
reads.length = 0;
139+
writes.length = 0;
140+
});
141+
142+
const lastWhere = () => reads[reads.length - 1]?.ast?.where;
143+
144+
// ── the load-bearing assertion: the DRIVER's input, not the rows ──────
145+
146+
it('find([[field, op, value]]) reaches the driver as a FilterCondition, not an array', async () => {
147+
const rows = await engine.find('deal', { where: [['stage', '=', 'won']] } as any);
148+
149+
expect(Array.isArray(lastWhere())).toBe(false);
150+
expect(lastWhere()).toEqual({ stage: 'won' });
151+
expect(rows.map((r: any) => r.id).sort()).toEqual(['d1', 'd3']);
152+
});
153+
154+
it('produces the identical driver AST as the hand-written FilterCondition', async () => {
155+
await engine.find('deal', { where: [['stage', '=', 'won']] } as any);
156+
const lowered = lastWhere();
157+
await engine.find('deal', { where: { stage: 'won' } } as any);
158+
expect(lowered).toEqual(lastWhere());
159+
});
160+
161+
it.each([
162+
['bare comparison tuple', ['stage', '=', 'won'], { stage: 'won' }],
163+
['nested single condition', [['stage', '=', 'won']], { stage: 'won' }],
164+
['prefix AND group', ['and', ['stage', '=', 'won'], ['amount', '>', 20]],
165+
{ $and: [{ stage: 'won' }, { amount: { $gt: 20 } }] }],
166+
['prefix OR group', ['or', ['stage', '=', 'won'], ['stage', '=', 'lost']],
167+
{ $or: [{ stage: 'won' }, { stage: 'lost' }] }],
168+
['bare list, implicit AND', [['stage', '=', 'won'], ['amount', '>', 20]],
169+
{ $and: [{ stage: 'won' }, { amount: { $gt: 20 } }] }],
170+
['operator alias (starts_with)', ['stage', 'starts_with', 'w'], { stage: { $startsWith: 'w' } }],
171+
['null predicate, short form', ['owner_id', 'is_null'], { owner_id: { $null: true } }],
172+
])('lowers %s', async (_label, where, expected) => {
173+
await engine.find('deal', { where } as any);
174+
expect(lastWhere()).toEqual(expected);
175+
});
176+
177+
// ── the authoring surfaces ruling C promised would not regress ────────
178+
179+
it('every shape @objectstack/client FilterBuilder emits still reaches the driver lowered', async () => {
180+
// Literal transcription of `FilterBuilder` output (`packages/client/src/
181+
// query-builder.ts`) — `build()` returns the single condition, or
182+
// `['and', ...conditions]`; `between()` nests a prefix group. Transcribed
183+
// rather than imported: objectql must not depend on the SDK, and the point
184+
// is the SHAPES, which this file pins verbatim.
185+
const builderOutputs: Array<[string, unknown]> = [
186+
['equals()', ['stage', '=', 'won']],
187+
['in()', ['stage', 'in', ['won', 'lost']]],
188+
['contains()', ['stage', 'like', '%wo%']],
189+
['isNotNull()', ['owner_id', 'is_not_null', null]],
190+
['between()', ['and', ['amount', '>=', 10], ['amount', '<=', 30]]],
191+
['build() with 2+ conditions', ['and', ['stage', '=', 'won'], ['amount', '>', 5]]],
192+
['getConditions() list', [['stage', '=', 'won'], ['amount', '>', 5]]],
193+
];
194+
for (const [label, where] of builderOutputs) {
195+
await engine.find('deal', { where } as any);
196+
expect(Array.isArray(lastWhere()), label).toBe(false);
197+
expect(lastWhere(), label).toBeTypeOf('object');
198+
}
199+
});
200+
201+
it('the `{current_user_id}` token still resolves — lowering runs BEFORE token expansion', async () => {
202+
// The shape `examples/app-showcase/src/ui/pages/my-work.page.ts` authorises
203+
// (`filters: [['owner_id','=','{current_user_id}']]`). Token resolution
204+
// reads `where` as an object, so lowering first is what lets it see the
205+
// value at all.
206+
const rows = await engine.find(
207+
'deal',
208+
{ where: [['owner_id', '=', '{current_user_id}']], context: { userId: 'u1' } } as any,
209+
);
210+
expect(lastWhere()).toEqual({ owner_id: 'u1' });
211+
expect(rows.map((r: any) => r.id).sort()).toEqual(['d1', 'd3']);
212+
});
213+
214+
// ── every entry point, not just find() ────────────────────────────────
215+
216+
it('findOne / count / aggregate lower the same way', async () => {
217+
const one = await engine.findOne('deal', { where: [['stage', '=', 'lost']] } as any);
218+
expect(lastWhere()).toEqual({ stage: 'lost' });
219+
expect(one?.id).toBe('d2');
220+
221+
expect(await engine.count('deal', { where: [['stage', '=', 'won']] } as any)).toBe(2);
222+
expect(lastWhere()).toEqual({ stage: 'won' });
223+
224+
await engine.aggregate('deal', {
225+
where: [['stage', '=', 'won']],
226+
groupBy: ['stage'],
227+
aggregations: [{ function: 'count', field: 'id', alias: 'n' }],
228+
} as any);
229+
expect(lastWhere()).toEqual({ stage: 'won' });
230+
});
231+
232+
it('update / delete lower too — and the by-id fast path finally sees `where.id`', async () => {
233+
await engine.update('deal', { amount: 99 }, { where: [['stage', '=', 'lost']], multi: true } as any);
234+
expect(writes[writes.length - 1]?.ast?.where).toEqual({ stage: 'lost' });
235+
236+
await engine.delete('deal', { where: [['stage', '=', 'lost']], multi: true } as any);
237+
expect(writes[writes.length - 1]?.ast?.where).toEqual({ stage: 'lost' });
238+
expect(await engine.count('deal')).toBe(2);
239+
});
240+
241+
it('the `filter` alias folds first, then lowers — both normalisations, one order', async () => {
242+
await engine.find('deal', { filter: [['stage', '=', 'won']] } as any);
243+
expect(lastWhere()).toEqual({ stage: 'won' });
244+
});
245+
246+
// ── `[]` keeps its meaning: no filter ─────────────────────────────────
247+
248+
it('an empty array is "no filter", exactly as before — find() returns every row', async () => {
249+
const rows = await engine.find('deal', { where: [] } as any);
250+
expect(rows).toHaveLength(3);
251+
// Lowered to ABSENT rather than to `{}`: `parseFilterAST([])` is
252+
// `undefined`, and every driver already treats both as "no predicate".
253+
expect(lastWhere()).toBeUndefined();
254+
});
255+
256+
it('count([]) counts every row', async () => {
257+
expect(await engine.count('deal', { where: [] } as any)).toBe(3);
258+
});
259+
260+
it('findOne([]) is now caught by the #4419 guard instead of returning an arbitrary row', async () => {
261+
// NOT a change to what `[]` MEANS — it still means "no filter". What
262+
// changed is that findOne can finally SEE that: an unlowered `[]` counted
263+
// as "an expression tree the driver will interpret", walked past the
264+
// guard, and came back with the object's first row.
265+
await expect(engine.findOne('deal', { where: [] } as any))
266+
.rejects.toThrow(/selects no particular record/);
267+
});
268+
269+
// ── refusals: the shapes parseFilterAST cannot express ────────────────
270+
271+
it('refuses the INFIX join dialect — the one shape the spec never declared', async () => {
272+
// `[condA, 'or', condB]` was compiled by four drivers and by none of the
273+
// doors; `FilterArraySchema` excludes it and `parseFilterAST` returns
274+
// `undefined` for it. Prefix is the declared spelling.
275+
await expect(
276+
engine.find('deal', { where: [['stage', '=', 'won'], 'or', ['stage', '=', 'lost']] } as any),
277+
).rejects.toThrow(/Infix joins .* NOT one of the shapes/s);
278+
});
279+
280+
it('refuses a bare triple whose operator is outside the AST vocabulary', async () => {
281+
await expect(engine.find('deal', { where: ['stage', 'sounds_like', 'won'] } as any))
282+
.rejects.toThrow(/is not a filter/);
283+
});
284+
285+
it('refuses a logical node with nothing to join, rather than matching every row', async () => {
286+
await expect(engine.find('deal', { where: ['and'] } as any))
287+
.rejects.toThrow(/is not a filter/);
288+
});
289+
290+
it('refuses an element that is neither a keyword nor a condition', async () => {
291+
await expect(engine.find('deal', { where: [42] } as any)).rejects.toThrow(/is not a filter/);
292+
await expect(engine.find('deal', { where: [null] } as any)).rejects.toThrow(/is not a filter/);
293+
});
294+
295+
it('the refusal says the filter was not applied and names the operator vocabulary', async () => {
296+
await expect(engine.find('deal', { where: ['stage', 'sounds_like', 'won'] } as any))
297+
.rejects.toThrow(/UNFILTERED result set/);
298+
await expect(engine.find('deal', { where: ['stage', 'sounds_like', 'won'] } as any))
299+
.rejects.toThrow(/Recognised operators: .*starts_with/s);
300+
});
301+
302+
it('a refused filter runs NOTHING — no driver read is attempted', async () => {
303+
await expect(engine.find('deal', { where: [42] } as any)).rejects.toThrow();
304+
expect(reads).toHaveLength(0);
305+
});
306+
307+
// ── the object form is untouched ──────────────────────────────────────
308+
309+
it('a FilterCondition object passes through byte-for-byte', async () => {
310+
const where = { $or: [{ stage: 'won' }, { amount: { $gt: 25 } }] };
311+
await engine.find('deal', { where } as any);
312+
expect(lastWhere()).toEqual(where);
313+
});
314+
315+
it('the caller\'s own bag is never mutated', async () => {
316+
const bag: any = { where: [['stage', '=', 'won']] };
317+
await engine.find('deal', bag);
318+
expect(bag.where).toEqual([['stage', '=', 'won']]);
319+
});
320+
});

0 commit comments

Comments
 (0)