diff --git a/.changeset/filter-array-lowered-at-engine-door.md b/.changeset/filter-array-lowered-at-engine-door.md new file mode 100644 index 0000000000..6796107004 --- /dev/null +++ b/.changeset/filter-array-lowered-at-engine-door.md @@ -0,0 +1,59 @@ +--- +"@objectstack/objectql": minor +"@objectstack/driver-sql": minor +"@objectstack/driver-memory": minor +"@objectstack/driver-mongodb": minor +--- + +fix(objectql,driver-sql,driver-memory,driver-mongodb)!: `FilterArray` 在 engine 门下沉,四驱动的数组方言删除 (#5158 拍板 C 第 2 步) + +`FilterArray` —— `['stage','=','won']`、`['and', […], […]]`、`[[…], […]]` —— 是**仅输入**的 +授权糖。#5285 已在 spec 里把这件事写明(`data/filter.zod.ts`,`filter-array-declaration.test.ts` +钉住「被声明」且「`where` 不接受它」)。本次是拍板 C 的第 2 步:让**运行时**与那份声明一致。 + +## 改了什么 + +进入运行时的门有两扇,过去只有一扇按契约读: + +| 门 | 改前 | 改后 | +|---|---|---| +| **Door 1** —— 协议/HTTP 面(`metadata-protocol`) | `isFilterAST` → `parseFilterAST`,不可下沉的数组答 `400 INVALID_FILTER` | 不变 | +| **Door 2** —— 进程内 engine 直调(`ObjectQL.find`/`findOne`/`count`/`aggregate`/`update`/`delete`) | 数组**原样**透传给驱动 | 走**同一条缝**:`isFilterAST` → `parseFilterAST` 下沉为 `FilterCondition`,不可下沉的数组响亮拒收 | +| 四驱动(`driver-sql`、继承它的 `driver-sqlite-wasm`、`driver-memory`、`driver-mongodb`) | 各自带**第二套过滤器编译器**,包括一种**中缀**方言(`[condA, 'or', condB]`)—— 没有任何 schema 声明过它,`parseFilterAST` 也表达不了它 | 数组方言删除;数组到达驱动即 `INVALID_FILTER` / 400 | + +一个查询两套编译器正是 ADR-0053 D-A1 禁止的分叉,而且它已经产生了真实的产品分叉:cloud 的 +`RemoteTransport.buildWhereSQL` 自 cloud#1075 起对**同一输入**响亮拒收,`driver-sql` 却编译它。 +删掉方言后两侧自然合流。 + +## 授权面:零变化 + +`FilterBuilder`(`@objectstack/client`)产出的元组与 `['and', ...]` 组、React block 的 +`filters` prop、wire 的 `$filter` 面、showcase 的授权点 —— **全部原样工作**,因为下沉正是 +这些形状本来的用途。wire 契约逐字节不变(Door 1 的行为未改)。 + +## ⚠️ 可观察的行为变更 + +1. **中缀连接不再被编译。** `where: [condA, 'or', condB]` 过去只有驱动认识,现在在 engine 门被拒收。 + 声明的写法是前缀组:`['or', condA, condB]` —— 语义相同,`parseFilterAST` 有它的下沉。 +2. **`findOne({ where: [] })` 现在抛错。** `[]` 的含义**没有变**(仍是「无过滤」,`find`/`count` + 照旧返回/计数全部行)。变的是 `findOne` 终于**看得见**这一点:未下沉的 `[]` 过去被 + `requireFindOnePredicate` 当作「驱动自己去解释的表达式树」放行,于是 `limit: 1` 落在整张表上, + 返回**任意一行** —— 正是 #4419 要挡的缺陷,活在 #4419 自己的守卫里面。 +3. **不可下沉的数组在 engine 门拒收,不再由驱动拒收。** 形状与操作符词表相同(`isFilterAST` 同一套), + 变的是消息来自调用点、带上调用方自己的值,以及明说「过滤器没有被应用,否则会返回**未过滤**的结果集」。 +4. **驱动直调者(不经 engine)受影响。** `SqlDriver` / `InMemoryDriver` / `translateFilter` 是公开 + 导出;把数组 `where` 直接喂给它们的调用方需要改为先 `parseFilterAST(...)` 再传,或改走 ObjectQL。 + 注意 `QueryAST.where` 的 `FilterCondition` 是索引签名类型,数组对它是**可赋值**的 —— 类型层从未 + 挡住这个输入,所以拒收必须在运行时。 +5. **`driver-mongodb` 的 `createdAt` → `created_at` 字段别名随方言一起消失。** 它只存在于数组路径 + (`mapFieldName`,仅被已删除的 `translateComparison` 调用),对象路径从未应用过它。消费端别名按 + AGENTS.md PD #12 是债务而非模式,故不再补回:请写声明的字段名 `created_at`。 + +## 删除的代码面 + +- `SqlDriver.applyFilters` 的数组遍历分支,及其比较发射器 `protected applyAstComparison`(约 220 行) +- `InMemoryDriver.convertToMongoQuery` 的 legacy array 分支(约 62 行) +- `driver-mongodb` `mongodb-filter.ts` 的 `translateArrayFilter` / `translateComparison` / `mapFieldName`(约 140 行) +- `driver-sqlite-wasm` 无自有实现,随 `SqlDriver` 继承变更 + +`[]` 在每一层的读法**都不变**:engine 删键、`parseFilterAST([])` 为 `undefined`、三个驱动都提前返回。 diff --git a/packages/objectql/src/engine-filter-array-lowering.test.ts b/packages/objectql/src/engine-filter-array-lowering.test.ts new file mode 100644 index 0000000000..458271c8cd --- /dev/null +++ b/packages/objectql/src/engine-filter-array-lowering.test.ts @@ -0,0 +1,320 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #5158 — Door 2 lowers `FilterArray` before any driver sees it. + * + * `FilterArray` (`['stage','=','won']`, `['and', […], […]]`, `[[…], […]]`) is + * INPUT-ONLY authoring sugar. The spec says so since #5285 + * (`data/filter.zod.ts`, pinned by `filter-array-declaration.test.ts`): it is + * declared, and `QuerySchema.where` deliberately excludes it. + * + * Two doors led into the runtime and only one read the contract that way. The + * protocol face (Door 1, `metadata-protocol/protocol.ts`) has always run + * `isFilterAST` → `parseFilterAST` and answered `400 INVALID_FILTER` for an + * array it could not lower. A direct engine call (Door 2) passed the array + * through verbatim, so four drivers grew a SECOND filter compiler to meet it — + * including an infix dialect (`[condA, 'or', condB]`) the spec never declared + * and `parseFilterAST` cannot express, which cloud's + * `RemoteTransport.buildWhereSQL` refuses outright. Same query, two answers, + * decided by whether the caller went over the wire. + * + * Maintainer ruling C on #5158 closed Door 2 onto Door 1's sink. These tests + * assert on the AST the DRIVER RECEIVES, not on the returned rows: identical + * rows are the whole point of the change, so a row assertion cannot tell a + * lowered filter from an unlowered one. That distinction is what makes the + * driver-side dialect deletion in this same PR safe. + */ + +import { describe, it, expect, beforeEach } from 'vitest'; +import { ObjectQL } from './engine.js'; + +const deal = { + name: 'deal', + label: 'Deal', + fields: { + id: { name: 'id', type: 'text' as const, primaryKey: true }, + stage: { name: 'stage', type: 'text' as const }, + amount: { name: 'amount', type: 'number' as const }, + owner_id: { name: 'owner_id', type: 'text' as const }, + }, +}; + +interface SeenRead { ast: any } + +/** + * Minimal driver that records every AST it is handed and executes only the + * `FilterCondition` object form — deliberately. A driver that also understood + * the array form could not witness the lowering, which is the bug this file + * pins: the four in-repo drivers understood BOTH, so nothing downstream ever + * had to notice which one it got. + */ +function makeRecordingDriver() { + const rows = new Map>(); + const reads: SeenRead[] = []; + const writes: SeenRead[] = []; + const matches = (row: any, where: any): boolean => { + if (where == null) return true; + if (Array.isArray(where) || typeof where !== 'object') { + throw new Error( + `driver received a non-object 'where' (${JSON.stringify(where)}) — the engine must ` + + 'lower FilterArray before the driver (#5158)', + ); + } + for (const [k, v] of Object.entries(where)) { + if (k === '$and') { if (!(v as any[]).every((w) => matches(row, w))) return false; continue; } + if (k === '$or') { if (!(v as any[]).some((w) => matches(row, w))) return false; continue; } + if (v && typeof v === 'object' && !Array.isArray(v)) { + const ops = v as Record; + if ('$gt' in ops && !((row[k] as any) > (ops.$gt as any))) return false; + if ('$gte' in ops && !((row[k] as any) >= (ops.$gte as any))) return false; + if ('$lt' in ops && !((row[k] as any) < (ops.$lt as any))) return false; + if ('$lte' in ops && !((row[k] as any) <= (ops.$lte as any))) return false; + if ('$ne' in ops && row[k] === ops.$ne) return false; + if ('$in' in ops && !(ops.$in as unknown[]).includes(row[k])) return false; + if ('$null' in ops && (row[k] == null) !== ops.$null) return false; + continue; + } + if (row[k] !== v) return false; + } + return true; + }; + const run = (ast: any) => { + const out = [...rows.values()].filter((r) => matches(r, ast?.where)); + return typeof ast?.limit === 'number' && ast.limit > 0 ? out.slice(0, ast.limit) : out; + }; + const driver: any = { + name: 'recording', version: '0.0.0', supports: {}, + async connect() {}, async disconnect() {}, async checkHealth() { return true; }, async execute() { return null; }, + async find(_o: string, ast: any) { reads.push({ ast }); return run(ast); }, + async findOne(_o: string, ast: any) { reads.push({ ast }); return run(ast)[0] ?? null; }, + async count(_o: string, ast: any) { reads.push({ ast }); return run(ast).length; }, + async aggregate(_o: string, ast: any) { reads.push({ ast }); return run(ast); }, + async create(_o: string, data: Record) { + const id = (data.id as string) ?? `r_${rows.size + 1}`; + const row = { ...data, id }; rows.set(id, row); return row; + }, + async update(_o: string, id: string, data: Record) { + const cur = rows.get(id); if (!cur) throw new Error(`nf ${id}`); + const up = { ...cur, ...data, id }; rows.set(id, up); return up; + }, + async updateMany(_o: string, ast: any, data: Record) { + writes.push({ ast }); + const hit = run(ast); + for (const r of hit) rows.set(r.id as string, { ...r, ...data }); + return hit.length; + }, + async delete(_o: string, id: string) { return rows.delete(id); }, + async deleteMany(_o: string, ast: any) { + writes.push({ ast }); + const hit = run(ast); + for (const r of hit) rows.delete(r.id as string); + return hit.length; + }, + async bulkCreate(o: string, batch: Record[]) { + return Promise.all(batch.map((r) => this.create(o, r))); + }, + async beginTransaction() { return { commit: async () => {}, rollback: async () => {} }; }, + async commit() {}, async rollback() {}, + }; + return { driver, reads, writes }; +} + +describe('Door 2 lowers FilterArray to FilterCondition before the driver (#5158)', () => { + let engine: ObjectQL; + let reads: SeenRead[]; + let writes: SeenRead[]; + + beforeEach(async () => { + const rec = makeRecordingDriver(); + reads = rec.reads; + writes = rec.writes; + engine = new ObjectQL(); + engine.registerDriver(rec.driver, true); + await engine.init(); + engine.registry.registerObject(deal as any); + await engine.insert('deal', { id: 'd1', stage: 'won', amount: 10, owner_id: 'u1' }); + await engine.insert('deal', { id: 'd2', stage: 'lost', amount: 20, owner_id: 'u2' }); + await engine.insert('deal', { id: 'd3', stage: 'won', amount: 30, owner_id: 'u1' }); + reads.length = 0; + writes.length = 0; + }); + + const lastWhere = () => reads[reads.length - 1]?.ast?.where; + + // ── the load-bearing assertion: the DRIVER's input, not the rows ────── + + it('find([[field, op, value]]) reaches the driver as a FilterCondition, not an array', async () => { + const rows = await engine.find('deal', { where: [['stage', '=', 'won']] } as any); + + expect(Array.isArray(lastWhere())).toBe(false); + expect(lastWhere()).toEqual({ stage: 'won' }); + expect(rows.map((r: any) => r.id).sort()).toEqual(['d1', 'd3']); + }); + + it('produces the identical driver AST as the hand-written FilterCondition', async () => { + await engine.find('deal', { where: [['stage', '=', 'won']] } as any); + const lowered = lastWhere(); + await engine.find('deal', { where: { stage: 'won' } } as any); + expect(lowered).toEqual(lastWhere()); + }); + + it.each([ + ['bare comparison tuple', ['stage', '=', 'won'], { stage: 'won' }], + ['nested single condition', [['stage', '=', 'won']], { stage: 'won' }], + ['prefix AND group', ['and', ['stage', '=', 'won'], ['amount', '>', 20]], + { $and: [{ stage: 'won' }, { amount: { $gt: 20 } }] }], + ['prefix OR group', ['or', ['stage', '=', 'won'], ['stage', '=', 'lost']], + { $or: [{ stage: 'won' }, { stage: 'lost' }] }], + ['bare list, implicit AND', [['stage', '=', 'won'], ['amount', '>', 20]], + { $and: [{ stage: 'won' }, { amount: { $gt: 20 } }] }], + ['operator alias (starts_with)', ['stage', 'starts_with', 'w'], { stage: { $startsWith: 'w' } }], + ['null predicate, short form', ['owner_id', 'is_null'], { owner_id: { $null: true } }], + ])('lowers %s', async (_label, where, expected) => { + await engine.find('deal', { where } as any); + expect(lastWhere()).toEqual(expected); + }); + + // ── the authoring surfaces ruling C promised would not regress ──────── + + it('every shape @objectstack/client FilterBuilder emits still reaches the driver lowered', async () => { + // Literal transcription of `FilterBuilder` output (`packages/client/src/ + // query-builder.ts`) — `build()` returns the single condition, or + // `['and', ...conditions]`; `between()` nests a prefix group. Transcribed + // rather than imported: objectql must not depend on the SDK, and the point + // is the SHAPES, which this file pins verbatim. + const builderOutputs: Array<[string, unknown]> = [ + ['equals()', ['stage', '=', 'won']], + ['in()', ['stage', 'in', ['won', 'lost']]], + ['contains()', ['stage', 'like', '%wo%']], + ['isNotNull()', ['owner_id', 'is_not_null', null]], + ['between()', ['and', ['amount', '>=', 10], ['amount', '<=', 30]]], + ['build() with 2+ conditions', ['and', ['stage', '=', 'won'], ['amount', '>', 5]]], + ['getConditions() list', [['stage', '=', 'won'], ['amount', '>', 5]]], + ]; + for (const [label, where] of builderOutputs) { + await engine.find('deal', { where } as any); + expect(Array.isArray(lastWhere()), label).toBe(false); + expect(lastWhere(), label).toBeTypeOf('object'); + } + }); + + it('the `{current_user_id}` token still resolves — lowering runs BEFORE token expansion', async () => { + // The shape `examples/app-showcase/src/ui/pages/my-work.page.ts` authorises + // (`filters: [['owner_id','=','{current_user_id}']]`). Token resolution + // reads `where` as an object, so lowering first is what lets it see the + // value at all. + const rows = await engine.find( + 'deal', + { where: [['owner_id', '=', '{current_user_id}']], context: { userId: 'u1' } } as any, + ); + expect(lastWhere()).toEqual({ owner_id: 'u1' }); + expect(rows.map((r: any) => r.id).sort()).toEqual(['d1', 'd3']); + }); + + // ── every entry point, not just find() ──────────────────────────────── + + it('findOne / count / aggregate lower the same way', async () => { + const one = await engine.findOne('deal', { where: [['stage', '=', 'lost']] } as any); + expect(lastWhere()).toEqual({ stage: 'lost' }); + expect(one?.id).toBe('d2'); + + expect(await engine.count('deal', { where: [['stage', '=', 'won']] } as any)).toBe(2); + expect(lastWhere()).toEqual({ stage: 'won' }); + + await engine.aggregate('deal', { + where: [['stage', '=', 'won']], + groupBy: ['stage'], + aggregations: [{ function: 'count', field: 'id', alias: 'n' }], + } as any); + expect(lastWhere()).toEqual({ stage: 'won' }); + }); + + it('update / delete lower too — and the by-id fast path finally sees `where.id`', async () => { + await engine.update('deal', { amount: 99 }, { where: [['stage', '=', 'lost']], multi: true } as any); + expect(writes[writes.length - 1]?.ast?.where).toEqual({ stage: 'lost' }); + + await engine.delete('deal', { where: [['stage', '=', 'lost']], multi: true } as any); + expect(writes[writes.length - 1]?.ast?.where).toEqual({ stage: 'lost' }); + expect(await engine.count('deal')).toBe(2); + }); + + it('the `filter` alias folds first, then lowers — both normalisations, one order', async () => { + await engine.find('deal', { filter: [['stage', '=', 'won']] } as any); + expect(lastWhere()).toEqual({ stage: 'won' }); + }); + + // ── `[]` keeps its meaning: no filter ───────────────────────────────── + + it('an empty array is "no filter", exactly as before — find() returns every row', async () => { + const rows = await engine.find('deal', { where: [] } as any); + expect(rows).toHaveLength(3); + // Lowered to ABSENT rather than to `{}`: `parseFilterAST([])` is + // `undefined`, and every driver already treats both as "no predicate". + expect(lastWhere()).toBeUndefined(); + }); + + it('count([]) counts every row', async () => { + expect(await engine.count('deal', { where: [] } as any)).toBe(3); + }); + + it('findOne([]) is now caught by the #4419 guard instead of returning an arbitrary row', async () => { + // NOT a change to what `[]` MEANS — it still means "no filter". What + // changed is that findOne can finally SEE that: an unlowered `[]` counted + // as "an expression tree the driver will interpret", walked past the + // guard, and came back with the object's first row. + await expect(engine.findOne('deal', { where: [] } as any)) + .rejects.toThrow(/selects no particular record/); + }); + + // ── refusals: the shapes parseFilterAST cannot express ──────────────── + + it('refuses the INFIX join dialect — the one shape the spec never declared', async () => { + // `[condA, 'or', condB]` was compiled by four drivers and by none of the + // doors; `FilterArraySchema` excludes it and `parseFilterAST` returns + // `undefined` for it. Prefix is the declared spelling. + await expect( + engine.find('deal', { where: [['stage', '=', 'won'], 'or', ['stage', '=', 'lost']] } as any), + ).rejects.toThrow(/Infix joins .* NOT one of the shapes/s); + }); + + it('refuses a bare triple whose operator is outside the AST vocabulary', async () => { + await expect(engine.find('deal', { where: ['stage', 'sounds_like', 'won'] } as any)) + .rejects.toThrow(/is not a filter/); + }); + + it('refuses a logical node with nothing to join, rather than matching every row', async () => { + await expect(engine.find('deal', { where: ['and'] } as any)) + .rejects.toThrow(/is not a filter/); + }); + + it('refuses an element that is neither a keyword nor a condition', async () => { + await expect(engine.find('deal', { where: [42] } as any)).rejects.toThrow(/is not a filter/); + await expect(engine.find('deal', { where: [null] } as any)).rejects.toThrow(/is not a filter/); + }); + + it('the refusal says the filter was not applied and names the operator vocabulary', async () => { + await expect(engine.find('deal', { where: ['stage', 'sounds_like', 'won'] } as any)) + .rejects.toThrow(/UNFILTERED result set/); + await expect(engine.find('deal', { where: ['stage', 'sounds_like', 'won'] } as any)) + .rejects.toThrow(/Recognised operators: .*starts_with/s); + }); + + it('a refused filter runs NOTHING — no driver read is attempted', async () => { + await expect(engine.find('deal', { where: [42] } as any)).rejects.toThrow(); + expect(reads).toHaveLength(0); + }); + + // ── the object form is untouched ────────────────────────────────────── + + it('a FilterCondition object passes through byte-for-byte', async () => { + const where = { $or: [{ stage: 'won' }, { amount: { $gt: 25 } }] }; + await engine.find('deal', { where } as any); + expect(lastWhere()).toEqual(where); + }); + + it('the caller\'s own bag is never mutated', async () => { + const bag: any = { where: [['stage', '=', 'won']] }; + await engine.find('deal', bag); + expect(bag.where).toEqual([['stage', '=', 'won']]); + }); +}); diff --git a/packages/objectql/src/engine-findone-contract.test.ts b/packages/objectql/src/engine-findone-contract.test.ts index cef5c01648..f2b9bdf463 100644 --- a/packages/objectql/src/engine-findone-contract.test.ts +++ b/packages/objectql/src/engine-findone-contract.test.ts @@ -232,10 +232,40 @@ describe('findOne executes what it declares and refuses an empty predicate (#441 expect(await engine.count('crm_account')).toBe(3); }); - it('a non-object where (an expression tree) is the driver\'s to interpret, not refused', async () => { - // The guard closes match-everything, not everything it cannot prove. - await expect(engine.findOne('crm_account', { where: [['name', '=', 'Two']] } as any)) - .resolves.not.toThrow(); + // ── RETIRED PIN (#5158, maintainer ruling C) ──────────────────────── + // + // This slot held: "a non-object where (an expression tree) is the DRIVER'S + // TO INTERPRET, not refused". That sentence was the engine's explicit + // blessing of a second filter dialect — the one door in the product that + // let a `FilterArray` reach a driver unlowered, which is how four drivers + // came to carry their own array compilers and how cloud's + // `RemoteTransport` ended up refusing what `driver-sql` compiled. + // + // Ruling C closed that door: `FilterArray` is declared INPUT-ONLY (spec + // #5285) and the engine lowers it through `parseFilterAST` like the + // protocol face always has. So "the driver's to interpret" is no longer + // true of ANY `where` — there is one dialect now, and the guard below + // judges the lowered shape. The retirement is the point of the change, not + // a casualty of it; the replacements assert what #4419 actually cares + // about, which is that findOne never answers with an arbitrary row. + // + // Lowering itself is pinned in `engine-filter-array-lowering.test.ts`. + + it('an array where is LOWERED before the guard, and still selects the record', async () => { + const row = await engine.findOne('crm_account', { where: [['name', '=', 'Two']] } as any); + expect(row?.id).toBe(two.id); + // The load-bearing half: what the driver saw was a FilterCondition. + expect(Array.isArray(lastRead().ast.where)).toBe(false); + expect(lastRead().ast.where).toEqual({ name: 'Two' }); + }); + + it('`where: []` no longer walks past the guard as "an expression tree"', async () => { + // The retired pin's real cost. `[]` means "no filter" — it always did + // — but an unlowered `[]` counted as a predicate here, so findOne + // applied `limit: 1` to the WHOLE table and returned its first row: + // precisely the #4419 defect, surviving inside #4419's own guard. + await expect(engine.findOne('crm_account', { where: [] } as any)) + .rejects.toThrow(/selects no particular record/); }); it('the guard reads the CALLER\'s predicate, before any middleware scoping', async () => { diff --git a/packages/objectql/src/engine.ts b/packages/objectql/src/engine.ts index d6047fd66e..b18dcee543 100644 --- a/packages/objectql/src/engine.ts +++ b/packages/objectql/src/engine.ts @@ -18,6 +18,9 @@ import { } from '@objectstack/spec/data'; import type { WriteObservabilityOptions } from '@objectstack/spec/contracts'; import { parseAutonumberFormat, renderAutonumber, missingFieldValues, isTenancyDisabled, FILE_REFERENCE_TYPES, REFERENCE_VALUE_TYPES, referenceTargetOf, isFileIdToken, RAW_FILE_VALUES_CONTEXT_KEY, isCurrentUserDefaultToken, isNowDefaultToken } from '@objectstack/spec/data'; +// [#5158] Door 2's lowering sink — the SAME pair the protocol face (Door 1) +// runs, so `FilterArray` has exactly one lowering in the product. +import { isFilterAST, parseFilterAST, VALID_AST_OPERATORS } from '@objectstack/spec/data'; import { DATA_MIGRATION_FLAG_OBJECT, FILE_REFERENCES_MIGRATION_ID, @@ -366,6 +369,102 @@ function foldEngineOptionAliases( return folded as T; } +/** + * **Door 2** — lower an arriving {@link FilterArray} on `where` to the + * `FilterCondition` the AST actually declares (#5158, maintainer ruling C). + * + * `FilterArray` — `['stage', '=', 'won']`, `['and', […], […]]`, `[[…], […]]` — + * is authoring sugar, and since #5285 the spec says so in as many words: it is + * declared INPUT-ONLY (`data/filter.zod.ts`), and `QuerySchema.where` is a + * `FilterCondition` that deliberately excludes it. There were two doors into + * the runtime and only one of them read the contract that way: + * + * - **Door 1**, the protocol/HTTP face (`metadata-protocol` `protocol.ts`), + * has always run `isFilterAST` → `parseFilterAST` and answered `400 + * INVALID_FILTER` for an array it could not lower. Nothing array-shaped + * survives it. + * - **Door 2**, a direct in-process engine call, passed the array through + * verbatim, and four drivers grew a second filter compiler to meet it — + * an INFIX dialect (`[condA, 'or', condB]`) that the spec never declared, + * that `parseFilterAST` cannot even express, and that cloud's + * `RemoteTransport.buildWhereSQL` refuses outright. Same query, two + * answers, decided by whether the caller went over the wire. + * + * This is that second door, closed: every entry point lowers through the SAME + * `parseFilterAST` sink Door 1 uses, so a driver sees exactly one filter + * dialect regardless of how the query arrived. The authoring ergonomics are + * untouched — `FilterBuilder` tuples, React block `filters` props and the five + * showcase call sites all still work, because lowering is what those shapes + * were always for. + * + * Three arrivals, three answers, matching Door 1 exactly: + * + * 1. `[]` — "no filter". The key is DELETED rather than lowered, which is the + * same reading every layer already gives it (`parseFilterAST([])` is + * `undefined`; `SqlDriver.applyFilters` returns early). Note this is now + * visible to `findOne`'s #4419 guard, which is the point: `findOne({where: + * []})` used to slip past the guard as "an expression tree the driver will + * interpret" and come back with an ARBITRARY row. + * 2. A well-formed AST — lowered. `isFilterAST` gates first so the operator + * vocabulary is checked before `parseFilterAST`'s lenient `$${op}` fallback + * can turn a misspelling into a `$sounds_like` condition nothing executes. + * 3. Anything else array-shaped — REFUSED, loudly, at the call site. Today + * those reach a driver and are refused there (#3948) with driver-internal + * wording, or — for the infix dialect — silently compiled by a second + * implementation. Failing here names the caller's own value. + * + * Returns the SAME reference when `where` is not an array (the overwhelmingly + * common path allocates nothing), otherwise a shallow copy: the bag belongs to + * the caller and may be reused (view metadata, flow node config). + */ +function lowerWhereFilterArray( + object: string, + operation: string, + bag: T, +): T { + if (!bag) return bag; + const where = (bag as Record).where; + if (!Array.isArray(where)) return bag; + + const lowered: Record = { ...bag }; + + // (1) `[]` is "no filter", not a failed filter. + if (where.length === 0) { + delete lowered.where; + return lowered as T; + } + + // (3) Not a shape `parseFilterAST` can express. + if (!isFilterAST(where)) { + throw new Error( + `${operation}('${object}') received a 'where' array that is not a filter: ` + + `${JSON.stringify(where)}. A filter array is a comparison [field, operator, value], ` + + `a logical node ["and"|"or", ...conditions], or a list of those — it is INPUT-ONLY ` + + `sugar (spec 'FilterArray'), lowered to a FilterCondition here before any driver sees ` + + `it (#5158). This value cannot be lowered, and an unapplied filter would have returned ` + + `the UNFILTERED result set. Recognised operators: ` + + `${[...VALID_AST_OPERATORS].sort().join(', ')}. Infix joins ([condA, "or", condB]) are ` + + `NOT one of the shapes — write the prefix form ["or", condA, condB].`, + ); + } + + // (2) The declared path. + const condition = parseFilterAST(where); + if (condition === undefined) { + // Unreachable by construction — `isFilterAST` accepted the shape, so + // `parseFilterAST` has a lowering for it. Loud rather than silent because + // the failure mode of the two spec functions disagreeing is a dropped + // predicate, i.e. every row (#3948). + throw new Error( + `${operation}('${object}'): filter array ${JSON.stringify(where)} passed isFilterAST() ` + + `but parseFilterAST() lowered it to nothing. Refusing rather than running the query ` + + `unfiltered (#5158).`, + ); + } + lowered.where = condition; + return lowered as T; +} + interface FormulaPlanEntry { name: string; expression: Expression; } function planFormulaProjection( @@ -4427,10 +4526,19 @@ export class ObjectQL implements IObjectQLEngine { * * "Selects nothing" is read the same way #3896 read an empty sharing * criteria: absent, `null`, or `{}` — the three shapes that mean "match every - * row". A `where` that is not a plain object (an expression tree) is the - * driver's to interpret, and counts as a predicate; this guard closes the one - * case that is unambiguously match-everything, not everything it cannot - * prove. + * row". This guard closes the one case that is unambiguously + * match-everything, not everything it cannot prove. + * + * [#5158] This comment used to add: "a `where` that is not a plain object (an + * expression tree) is the DRIVER'S to interpret, and counts as a predicate." + * That sentence was the engine's blessing of a second filter dialect, and it + * cost exactly what a blessing costs — `findOne({ where: [] })` counted as a + * predicate, walked past this guard, and returned an ARBITRARY row: the #4419 + * defect surviving inside #4419's own guard. `FilterArray` is now lowered by + * {@link lowerWhereFilterArray} at every entry point, so by the time this + * runs `where` is a `FilterCondition` or nothing. The `Array.isArray` arm + * below is kept as defence in depth for a subclass or a future caller that + * reaches this method without lowering — it is no longer a contract. * * `orderBy` is the other way to be specific, and a legitimate one — "the * newest", "the highest priority". It is honored on this path by every @@ -4465,6 +4573,7 @@ export class ObjectQL implements IObjectQLEngine { // (#4371, three shipped instances in #4370). query = foldEngineOptionAliases(object, 'find', query, ENGINE_QUERY_SLOTS, ENGINE_WIRE_ONLY_SLOTS); rejectUnknownEngineOptions(object, 'find', query, ENGINE_FIND_OPTION_KEYS); + query = lowerWhereFilterArray(object, 'find', query); this.logger.debug('Find operation starting', { object, query }); const driver = this.getDriver(object); // `object` LAST: the resolved name must win. Spread-first used to let a @@ -4609,6 +4718,7 @@ export class ObjectQL implements IObjectQLEngine { // matters here too: findOne({ sort }) means "first row of THIS order". query = foldEngineOptionAliases(objectName, 'findOne', query, ENGINE_QUERY_SLOTS, ENGINE_WIRE_ONLY_SLOTS); rejectUnknownEngineOptions(objectName, 'findOne', query, ENGINE_FIND_OPTION_KEYS); + query = lowerWhereFilterArray(objectName, 'findOne', query); this.logger.debug('FindOne operation', { objectName }); const driver = this.getDriver(objectName); // `object` after the spread for the same reason as find(); `limit: 1` @@ -4984,6 +5094,10 @@ export class ObjectQL implements IObjectQLEngine { // predicate at all and a `multi: true` update rewrote EVERY row. options = foldEngineOptionAliases(object, 'update', options, ENGINE_WHERE_SLOTS); rejectUnknownEngineOptions(object, 'update', options, ENGINE_UPDATE_OPTION_KEYS); + // [#5158] Lower before the by-id extraction below reads `where.id`: on an + // array that read is `undefined` whatever the caller wrote, so an + // `update({ where: [['id','=',x]] })` used to route to the multi-row path. + options = lowerWhereFilterArray(object, 'update', options); // Expand `{filter-placeholder}` values BEFORE the id is extracted (#3810). // The read path resolves them; without the same call here the SAME filter @@ -5483,6 +5597,9 @@ export class ObjectQL implements IObjectQLEngine { // predicate on its AST and emptied the table. options = foldEngineOptionAliases(object, 'delete', options, ENGINE_WHERE_SLOTS); rejectUnknownEngineOptions(object, 'delete', options, ENGINE_DELETE_OPTION_KEYS); + // [#5158] Same ordering reason as update(): the dispatch decision below + // reads `where.id`, which an unlowered array never carries. + options = lowerWhereFilterArray(object, 'delete', options); // Expand `{filter-placeholder}` values before the id is extracted — same // reasoning as update() above (#3810). @@ -5725,6 +5842,7 @@ export class ObjectQL implements IObjectQLEngine { // `query.where` only, so an unfolded `{ filter }` counted the whole table. query = foldEngineOptionAliases(object, 'count', query, ENGINE_WHERE_SLOTS); rejectUnknownEngineOptions(object, 'count', query, ENGINE_COUNT_OPTION_KEYS); + query = lowerWhereFilterArray(object, 'count', query); const driver = this.getDriver(object); // The AST must ride on the opCtx so the security/sharing middlewares can @@ -5806,6 +5924,7 @@ export class ObjectQL implements IObjectQLEngine { // `query.where` only, so an unfolded `{ filter }` aggregated every row. query = foldEngineOptionAliases(object, 'aggregate', query, ENGINE_WHERE_SLOTS); rejectUnknownEngineOptions(object, 'aggregate', query, ENGINE_AGGREGATE_OPTION_KEYS); + query = lowerWhereFilterArray(object, 'aggregate', query); this.rejectCredentialAggregation(object, query); const driver = this.getDriver(object); this.logger.debug(`Aggregate on ${object} using ${driver.name}`, query); diff --git a/packages/plugins/driver-memory/src/filter-refusal.ts b/packages/plugins/driver-memory/src/filter-refusal.ts index 740a2f43ca..200e6f06e0 100644 --- a/packages/plugins/driver-memory/src/filter-refusal.ts +++ b/packages/plugins/driver-memory/src/filter-refusal.ts @@ -31,6 +31,34 @@ export function unsupportedFilterError(message: string): Error { return err; } +/** + * [#5158] A `FilterArray` reached the driver unlowered. + * + * `where` is a `FilterCondition` — `QueryASTSchema.where: FilterConditionSchema` + * — and `FilterArray` is INPUT-ONLY authoring sugar the spec declares separately + * (`spec/data/filter.zod.ts`, #5285). Both doors into the runtime lower it + * through `parseFilterAST` before any driver is reached: the protocol face + * (`metadata-protocol`) and the engine (`ObjectQL`, maintainer ruling C). + * + * The twin of `driver-sql`'s `filterArrayReachedDriverError`, and deliberately + * word-for-word: #3948 made the two backends AGREE that an uncompilable filter + * is a loud refusal rather than a silent match-everything, and the four drivers + * each carrying their own array compiler is how they drifted apart in the first + * place. cloud's `RemoteTransport.buildWhereSQL` already refuses this input + * (cloud#1075); deleting the dialect here converges the product on one answer. + */ +export function filterArrayReachedDriverError(filters: unknown[]): Error { + return unsupportedFilterError( + `A filter ARRAY reached the driver: ${JSON.stringify(filters)}. ` + + `'where' is a FilterCondition object; the array form ('FilterArray') is input-only ` + + `authoring sugar and is lowered by @objectstack/spec parseFilterAST() at the engine ` + + `and protocol doors before any driver sees it (#5158). This driver no longer carries a ` + + `second compiler for it — call through ObjectQL, or lower the value yourself with ` + + `parseFilterAST(). Note the INFIX join form ([condA, "or", condB]) has no lowering at ` + + `all: write the prefix form ["or", condA, condB].`, + ); +} + /** * [#5240] Is this field spec `{}` — a field constrained by ZERO operators? * diff --git a/packages/plugins/driver-memory/src/memory-datetime-storage.test.ts b/packages/plugins/driver-memory/src/memory-datetime-storage.test.ts index cc093639b0..2b721a5d54 100644 --- a/packages/plugins/driver-memory/src/memory-datetime-storage.test.ts +++ b/packages/plugins/driver-memory/src/memory-datetime-storage.test.ts @@ -19,6 +19,7 @@ */ import { describe, it, expect, beforeEach } from 'vitest'; +import { parseFilterAST } from '@objectstack/spec/data'; import { InMemoryDriver } from './memory-driver.js'; const ids = (rows: any[]) => rows.map((r: any) => r.id).sort(); @@ -111,8 +112,13 @@ describe('InMemoryDriver Field.datetime storage (#4047)', () => { } as any); expect(ids(between)).toEqual(['d_midnight', 'd_yesterday', 's_evening', 's_morning']); + // [#5158] The authored array form, lowered the declared way. The INFIX + // join (`[condA, 'and', condB]`) has no lowering at all and is refused at + // the door; the declared spelling of "both bounds" is the prefix group. const array = await driver.find('task', { - where: [['created_at', '>=', '2026-04-29'], 'and', ['created_at', '<=', '2026-07-28']], + where: parseFilterAST( + ['and', ['created_at', '>=', '2026-04-29'], ['created_at', '<=', '2026-07-28']], + ) as any, } as any); expect(ids(array)).toEqual(['d_midnight', 'd_yesterday', 's_evening', 's_morning']); }); diff --git a/packages/plugins/driver-memory/src/memory-driver-calendar-day-upper-bound.test.ts b/packages/plugins/driver-memory/src/memory-driver-calendar-day-upper-bound.test.ts index d46ad87846..c03219a08d 100644 --- a/packages/plugins/driver-memory/src/memory-driver-calendar-day-upper-bound.test.ts +++ b/packages/plugins/driver-memory/src/memory-driver-calendar-day-upper-bound.test.ts @@ -17,6 +17,7 @@ */ import { describe, it, expect, beforeEach } from 'vitest'; +import { parseFilterAST } from '@objectstack/spec/data'; import { InMemoryDriver } from './memory-driver.js'; import { MemoryAnalyticsService } from './memory-analytics.js'; import type { Cube } from '@objectstack/spec/data'; @@ -87,14 +88,19 @@ describe('InMemoryDriver — bare-day $lte covers the whole day (#4042)', () => expect(ids(found)).toEqual(['t_evening', 't_midnight', 't_morning', 't_old']); }); - it('applies to the array (`[field, op, value]`) where spelling too', async () => { + it('applies to the authored array spelling, lowered the declared way (#5158)', async () => { + // The array form is input-only sugar lowered at the engine/protocol doors; + // the driver no longer compiles it. Same authored filter, same rows. The + // INFIX join has no lowering — the declared spelling is the prefix group. const lte = await driver.find('task', { - where: [['created_at', '>=', '2026-04-29'], 'and', ['created_at', '<=', '2026-07-28']], + where: parseFilterAST( + ['and', ['created_at', '>=', '2026-04-29'], ['created_at', '<=', '2026-07-28']], + ) as any, } as any); expect(ids(lte)).toEqual(['t_evening', 't_midnight', 't_morning', 't_yesterday']); const between = await driver.find('task', { - where: [['created_at', 'between', ['2026-04-29', '2026-07-28']]], + where: parseFilterAST([['created_at', 'between', ['2026-04-29', '2026-07-28']]]) as any, } as any); expect(ids(between)).toEqual(['t_evening', 't_midnight', 't_morning', 't_yesterday']); }); diff --git a/packages/plugins/driver-memory/src/memory-driver.ts b/packages/plugins/driver-memory/src/memory-driver.ts index 3b9b159b7c..706b785a4e 100644 --- a/packages/plugins/driver-memory/src/memory-driver.ts +++ b/packages/plugins/driver-memory/src/memory-driver.ts @@ -8,6 +8,7 @@ import { Query, Aggregator } from 'mingo'; import { getValueByPath } from './memory-matcher.js'; import { emptyFieldConstraintError, + filterArrayReachedDriverError, isEmptyFieldConstraint, unsupportedFilterError, } from './filter-refusal.js'; @@ -685,18 +686,29 @@ export class InMemoryDriver implements IDataDriver { /** * Convert ObjectQL filter format to MongoDB query format for Mingo. - * + * * Supports: * 1. AST Comparison Node: { type: 'comparison', field, operator, value } * 2. AST Logical Node: { type: 'logical', operator: 'and'|'or', conditions: [...] } - * 3. Legacy Array Format: [['field', 'op', value], 'and', ['field2', 'op', value2]] - * 4. MongoDB Format: { field: value } or { field: { $eq: value } } (passthrough) + * 3. MongoDB Format: { field: value } or { field: { $eq: value } } (passthrough) + * + * The legacy ARRAY format (`[['field','op',value], 'and', […]]`) is no longer + * one of them — see {@link filterArrayReachedDriverError} and #5158. It was a + * second filter compiler for a shape the spec never declared on `where`, and + * both doors into the runtime now lower `FilterArray` through + * `parseFilterAST` before a driver is reached. */ private convertToMongoQuery(filters?: any, object?: string): Record { if (!filters) return {}; + if (Array.isArray(filters)) { + // `[]` still means "no filter" — unchanged. + if (filters.length === 0) return {}; + throw filterArrayReachedDriverError(filters); + } + // AST node format (ObjectQL QueryAST) - if (!Array.isArray(filters) && typeof filters === 'object') { + if (typeof filters === 'object') { if (filters.type === 'comparison') { return this.convertConditionToMongo(filters.field, filters.operator, filters.value, object) || {}; } @@ -712,67 +724,10 @@ export class InMemoryDriver implements IDataDriver { return this.normalizeFilterCondition(filters, object); } - // Legacy array format - if (!Array.isArray(filters) || filters.length === 0) return {}; - - const logicGroups: { logic: 'and' | 'or'; conditions: Record[] }[] = [ - { logic: 'and', conditions: [] }, - ]; - let currentLogic: 'and' | 'or' = 'and'; - - for (const item of filters) { - if (typeof item === 'string') { - const lower = item.toLowerCase(); - // Previously this cast ANY string to 'and' | 'or'. A bare comparison - // triple — which reaches a driver only when `isFilterAST()` refused its - // operator, leaving the array unparsed — therefore opened three empty - // logic groups, produced no conditions, and returned `{}`: a filter that - // matches EVERY record. An unapplied filter must not look like a - // satisfied one. #3948. - if (lower !== 'and' && lower !== 'or') { - throw unsupportedFilterError( - `Unrecognized filter operator "${item}" in a comparison triple. ` + - `A filter array is either a logical node (["and"|"or", …]) or nested ` + - `conditions ([[field, op, value], …]); a bare [field, op, value] only ` + - `reaches the driver when its operator is outside @objectstack/spec ` + - `VALID_AST_OPERATORS, which leaves the filter unparsed. ` + - `Filter was: ${JSON.stringify(filters)}`, - ); - } - if (lower !== currentLogic) { - currentLogic = lower; - logicGroups.push({ logic: currentLogic, conditions: [] }); - } - } else if (Array.isArray(item)) { - const [field, operator, value] = item; - // `convertConditionToMongo` now throws rather than returning null for an - // operator it cannot express, so a dropped condition can no longer - // silently widen the result set. - const cond = this.convertConditionToMongo(field, operator, value, object); - if (cond) logicGroups[logicGroups.length - 1].conditions.push(cond); - } else { - throw unsupportedFilterError( - `Unrecognized filter element of type ` + - `"${item === null ? 'null' : typeof item}" — expected a logical keyword ` + - `("and"/"or") or a condition array. Filter was: ${JSON.stringify(filters)}`, - ); - } - } - - const allConditions: Record[] = []; - for (const group of logicGroups) { - if (group.conditions.length === 0) continue; - if (group.conditions.length === 1) { - allConditions.push(group.conditions[0]); - } else { - const op = group.logic === 'or' ? '$or' : '$and'; - allConditions.push({ [op]: group.conditions }); - } - } - - if (allConditions.length === 0) return {}; - if (allConditions.length === 1) return allConditions[0]; - return { $and: allConditions }; + // A truthy non-object, non-array `where` emits no predicate. Pre-existing + // behaviour on a shape only a cast can produce; untouched by #5158, which + // is about the array dialect. + return {}; } /** diff --git a/packages/plugins/driver-memory/src/memory-filter-ast-vocabulary.test.ts b/packages/plugins/driver-memory/src/memory-filter-ast-vocabulary.test.ts index 42c05d9066..1d8fd6201e 100644 --- a/packages/plugins/driver-memory/src/memory-filter-ast-vocabulary.test.ts +++ b/packages/plugins/driver-memory/src/memory-filter-ast-vocabulary.test.ts @@ -19,9 +19,19 @@ * driver only when `isFilterAST()` refused it, leaving the array unparsed — and * the old loop cast each string element to a logic keyword, opening empty logic * groups and returning `{}`: a filter matching EVERY record. + * + * [#5158] That loop is gone. `FilterArray` is input-only authoring sugar, and + * both doors into the runtime lower it through `parseFilterAST` before a driver + * is reached — so an array arriving HERE is refused rather than compiled by a + * second implementation (which is what cloud's `RemoteTransport` already did, + * cloud#1075). Invariant (1) is unchanged and is still measured against the + * spec's own operator set: every operator must survive the authored form all + * the way to a matched row, which is now `parseFilterAST` + this driver rather + * than this driver alone. Invariant (2) likewise — the refusal simply happens + * one door earlier for the shapes `parseFilterAST` cannot express. */ import { describe, it, expect, beforeEach } from 'vitest'; -import { VALID_AST_OPERATORS } from '@objectstack/spec/data'; +import { VALID_AST_OPERATORS, parseFilterAST } from '@objectstack/spec/data'; import type { FilterCondition } from '@objectstack/spec/data'; import { InMemoryDriver } from './memory-driver.js'; @@ -38,12 +48,20 @@ describe('InMemoryDriver filter vocabulary ↔ VALID_AST_OPERATORS', () => { }); /** Operators are exercised through `find`, the path a real query takes. */ - // Cast deliberately: several `where` shapes below are ones the AST gate - // refuses, fed in to prove the driver throws instead of silently dropping - // the condition. `unknown` is the honest parameter type. + // Cast deliberately: several `where` shapes below are ones the declared + // contract forbids, fed in to prove the driver throws instead of silently + // dropping the condition. `unknown` is the honest parameter type. const find = (where: unknown) => driver.find(TABLE, { object: TABLE, fields: ['id'], where: where as FilterCondition }); + /** + * The authored `FilterArray`, travelling the one route that exists (#5158): + * lowered by the spec's `parseFilterAST`, then executed by the driver. This + * is what the engine and the protocol face do; feeding the array raw is what + * no longer works, pinned at the bottom of this file. + */ + const findAuthored = (where: unknown) => find(parseFilterAST(where)); + it('reads a non-empty operator set from the spec', () => { // Guards every assertion below from passing vacuously. expect(VALID_AST_OPERATORS.size).toBeGreaterThan(0); @@ -65,7 +83,7 @@ describe('InMemoryDriver filter vocabulary ↔ VALID_AST_OPERATORS', () => { // "no predicate". An operator the driver cannot express now throws, so any // rejection here means the spec accepts a name this driver cannot honour. await expect( - find([[field, op, value]]), + findAuthored([[field, op, value]]), `VALID_AST_OPERATORS accepts "${op}" but InMemoryDriver cannot express it`, ).resolves.toBeDefined(); }); @@ -73,34 +91,61 @@ describe('InMemoryDriver filter vocabulary ↔ VALID_AST_OPERATORS', () => { it('matches null rows for is_null instead of dropping the predicate', async () => { // The regression this pins: `is_null` used to return null from the converter, // the condition was dropped, and the query returned BOTH rows. - const rows = await find([['note', 'is_null', true]]); + const rows = await findAuthored([['note', 'is_null', true]]); expect(rows.map((r: any) => r.id)).toEqual(['1']); }); it('matches non-null rows for is_not_null', async () => { - const rows = await find([['note', 'is_not_null', true]]); + const rows = await findAuthored([['note', 'is_not_null', true]]); expect(rows.map((r: any) => r.id)).toEqual(['2']); }); it('throws on an operator it cannot express, rather than matching everything', async () => { - await expect(find([['name', 'sounds_like', 'alpha']])) - .rejects.toThrow(/Unsupported filter operator "sounds_like"/); + // `parseFilterAST` is lenient here by design (`$${op}` fallback), so the + // driver meets `{ name: { $sounds_like: … } }`; the ENGINE door gates on + // `isFilterAST` and refuses the authored array a layer earlier. #3948's + // condition holds either way: it THROWS, never a match-everything. + // + // What it does NOT yet do on this path is speak the ADR-0112 envelope — + // the object path hands an unknown `$op` straight to mingo, which raises a + // bare `MingoError` with no `code`/`status`. That gap is the general form + // of #5324 (same `normalizeFilterCondition` passthrough, `$not` being the + // instance filed there) and is NOT a #5158 regression: the array path that + // used to carry the envelope for this input is what #5158 deleted, and the + // object path has always answered this way. + await expect(findAuthored([['name', 'sounds_like', 'alpha']])).rejects.toThrow(); + }); + + it('a malformed between emits no predicate on this backend — the #5328 divergence', async () => { + // driver-sql THROWS on `{ score: { $between: 5 } }`; this driver's + // `normalizeFilterCondition` skips the arm entirely and the field + // normalises to `{}`, which mingo evaluates as "matches nothing". Two + // backends, one filter, two answers — filed as #5328, not fixed here. + // + // Pinned as-is so the divergence is visible rather than folklore. It is + // pre-existing: the loud refusal this test used to observe belonged to the + // ARRAY path, which #5158 deleted; the object path never had one. + await expect(findAuthored([['score', 'between', 5]])).resolves.toEqual([]); }); - it('throws on a bare comparison triple instead of returning every record', async () => { - // `before` is a canonical VIEW_FILTER_OPERATORS member that VALID_AST_OPERATORS - // does not accept, so this is the exact shape that reached drivers unparsed. - await expect(find(['created_at', 'before', '2024-01-01'])) - .rejects.toThrow(/Unrecognized filter operator "created_at"/); + it('still honours a well-formed logical node', async () => { + const rows = await findAuthored(['or', ['name', '=', 'alpha'], ['name', '=', 'beta']]); + expect(rows.map((r: any) => r.id).sort()).toEqual(['1', '2']); }); - it('throws on a malformed between rather than emitting no predicate', async () => { - await expect(find([['score', 'between', 5]])) - .rejects.toThrow(/needs a two-element array/); + // ── the array dialect itself: refused, not compiled (#5158) ─────────── + + it.each([ + ['bare comparison triple the AST gate refuses', ['created_at', 'before', '2024-01-01']], + ['nested condition array', [['name', '=', 'alpha']]], + ['infix logical join (the undeclared dialect)', [['name', '=', 'alpha'], 'or', ['name', '=', 'beta']]], + ['element of the wrong type', [42]], + ])('refuses an array %s instead of compiling a second dialect', async (_label, where) => { + await expect(find(where)).rejects.toThrow(/A filter ARRAY reached the driver/); }); - it('still honours a well-formed logical node', async () => { - const rows = await find(['or', ['name', '=', 'alpha'], ['name', '=', 'beta']]); + it('an empty array is still "no filter", not a refusal', async () => { + const rows = await find([]); expect(rows.map((r: any) => r.id).sort()).toEqual(['1', '2']); }); }); diff --git a/packages/plugins/driver-memory/src/memory-filter-refusal-envelope.test.ts b/packages/plugins/driver-memory/src/memory-filter-refusal-envelope.test.ts index 99052de4e1..cdab2a4cfc 100644 --- a/packages/plugins/driver-memory/src/memory-filter-refusal-envelope.test.ts +++ b/packages/plugins/driver-memory/src/memory-filter-refusal-envelope.test.ts @@ -53,7 +53,10 @@ describe('[#4436] InMemoryDriver filter refusals carry INVALID_FILTER and leak n const cases: Array<[string, unknown, string]> = [ ['unsupported operator in a condition array', [['stage', 'sounds_like', 'won']], 'sounds_like'], ['bare comparison triple', ['close_date', 'before', '2024-01-01'], 'close_date'], - ['filter element of the wrong type', [42], 'number'], + // [#5158] The message names the ARRAY, not the offending element's typeof: + // the driver no longer walks a filter array, so it has no element to + // describe. `42` is the caller's own value, echoed back. + ['filter element of the wrong type', [42], '42'], ['`between` with a bad operand', [['amount', 'between', 5]], 'between'], ]; diff --git a/packages/plugins/driver-mongodb/src/mongodb-datetime-storage.test.ts b/packages/plugins/driver-mongodb/src/mongodb-datetime-storage.test.ts index 8b4a6289e0..373e7289be 100644 --- a/packages/plugins/driver-mongodb/src/mongodb-datetime-storage.test.ts +++ b/packages/plugins/driver-mongodb/src/mongodb-datetime-storage.test.ts @@ -20,6 +20,7 @@ */ import { describe, it, expect, beforeAll, afterAll, beforeEach } from 'vitest'; +import { parseFilterAST } from '@objectstack/spec/data'; import type { MongoMemoryServer } from 'mongodb-memory-server'; import { MongoDBDriver } from './mongodb-driver.js'; import { createTestMongod } from './test-mongod.js'; @@ -136,8 +137,13 @@ describe.skipIf(!sharedMongod)('MongoDB Field.datetime storage (#4047)', () => { } as any); expect(ids(between)).toEqual(['d_midnight', 'd_yesterday', 's_evening', 's_morning']); + // [#5158] The authored array form, lowered the declared way. The INFIX + // join (`[condA, 'and', condB]`) has no lowering at all and is refused at + // the door; the declared spelling of "both bounds" is the prefix group. const array = await driver.find('task', { - where: [['created_at', '>=', '2026-04-29'], 'and', ['created_at', '<=', '2026-07-28']], + where: parseFilterAST( + ['and', ['created_at', '>=', '2026-04-29'], ['created_at', '<=', '2026-07-28']], + ) as any, } as any); expect(ids(array)).toEqual(['d_midnight', 'd_yesterday', 's_evening', 's_morning']); }); diff --git a/packages/plugins/driver-mongodb/src/mongodb-driver.test.ts b/packages/plugins/driver-mongodb/src/mongodb-driver.test.ts index f084325484..90d2373dfb 100644 --- a/packages/plugins/driver-mongodb/src/mongodb-driver.test.ts +++ b/packages/plugins/driver-mongodb/src/mongodb-driver.test.ts @@ -2,6 +2,7 @@ import { describe, it, expect, beforeAll, afterAll, beforeEach } from 'vitest'; import type { MongoMemoryServer } from 'mongodb-memory-server'; +import { parseFilterAST } from '@objectstack/spec/data'; import { MongoDBDriver } from './mongodb-driver.js'; import { createTestMongod } from './test-mongod.js'; @@ -421,9 +422,32 @@ describe.skipIf(!sharedMongod)('MongoDBDriver', () => { expect(results.length).toBe(2); }); - it('should filter with legacy array style', async () => { + // ── RETIRED PIN (#5158, maintainer ruling C) ────────────────────── + // + // This slot held `should filter with legacy array style`, which asserted + // that this driver COMPILES `where: [['age','>=',30], ['role','=','user']]`. + // That is the dialect itself, and the dialect is what ruling C deletes: + // `FilterArray` is INPUT-ONLY authoring sugar (spec `data/filter.zod.ts`, + // #5285), lowered through `parseFilterAST` at the engine and protocol doors + // before any driver is reached. A second compiler here is the ADR-0053 + // D-A1 divergence — the same one cloud's `RemoteTransport.buildWhereSQL` + // closed from its side (cloud#1075). + // + // Same treatment as `driver-sql`'s two compile-asserting cases in + // `sql-driver-filter-no-silent-drop.test.ts`: the dialect case becomes a + // REFUSAL pin, plus a counterpart proving the identical authored shape + // still compiles — and returns the same rows — once lowered. Retiring the + // old assertion is the point of the change, not a casualty of it. + + it('refuses a raw array `where` — the dialect is gone (#5158)', async () => { + await expect( + driver.find('user', { where: [['age', '>=', 30], ['role', '=', 'user']] as any }), + ).rejects.toThrow(/A filter ARRAY reached the driver/); + }); + + it('the same authored shape still returns the same rows, once lowered (#5158)', async () => { const results = await driver.find('user', { - where: [['age', '>=', 30], ['role', '=', 'user']] as any, + where: parseFilterAST([['age', '>=', 30], ['role', '=', 'user']]) as any, }); expect(results.length).toBe(2); }); diff --git a/packages/plugins/driver-mongodb/src/mongodb-filter.test.ts b/packages/plugins/driver-mongodb/src/mongodb-filter.test.ts index f728511b32..1aef79a181 100644 --- a/packages/plugins/driver-mongodb/src/mongodb-filter.test.ts +++ b/packages/plugins/driver-mongodb/src/mongodb-filter.test.ts @@ -1,6 +1,7 @@ // Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. import { describe, it, expect } from 'vitest'; +import { parseFilterAST } from '@objectstack/spec/data'; import { translateFilter } from './mongodb-filter.js'; describe('MongoDB Filter Translator', () => { @@ -70,11 +71,15 @@ describe('MongoDB Filter Translator', () => { }); }); - it('array-style `<=` takes the same rule', () => { - expect(translateFilter([['created_at', '<=', '2026-07-28']])).toEqual({ + it('the authored array `<=` takes the same rule, once lowered (#5158)', () => { + // `translateFilter` no longer compiles the array spelling; the authored + // shape reaches it through `parseFilterAST`, which is what both doors do. + expect(translateFilter(parseFilterAST([['created_at', '<=', '2026-07-28']]))).toEqual({ created_at: { $lt: '2026-07-29' }, }); - expect(translateFilter([['created_at', '<=', '2026-07-28T12:00:00.000Z']])).toEqual({ + expect( + translateFilter(parseFilterAST([['created_at', '<=', '2026-07-28T12:00:00.000Z']])), + ).toEqual({ created_at: { $lte: '2026-07-28T12:00:00.000Z' }, }); }); @@ -171,49 +176,63 @@ describe('MongoDB Filter Translator', () => { }); }); - describe('legacy array-style filters', () => { - it('translates single comparison tuple', () => { - expect(translateFilter(['name', '=', 'Alice'])).toEqual({ name: 'Alice' }); - }); - - it('translates != operator', () => { - expect(translateFilter(['status', '!=', 'deleted'])).toEqual({ status: { $ne: 'deleted' } }); - }); - - it('translates comparison operators', () => { - expect(translateFilter(['age', '>', 18])).toEqual({ age: { $gt: 18 } }); - expect(translateFilter(['age', '>=', 18])).toEqual({ age: { $gte: 18 } }); - expect(translateFilter(['age', '<', 65])).toEqual({ age: { $lt: 65 } }); - expect(translateFilter(['score', '<=', 100])).toEqual({ score: { $lte: 100 } }); - }); - - it('translates in/nin operators', () => { - expect(translateFilter(['status', 'in', ['active', 'pending']])).toEqual({ - status: { $in: ['active', 'pending'] }, + describe('the array dialect is refused, not compiled (#5158)', () => { + /** + * `translateFilter` used to carry a second compiler for the array + * spelling, including an INFIX join (`[condA, 'or', condB]`) no schema + * declared and `parseFilterAST` cannot express. `FilterArray` is + * input-only authoring sugar (spec `data/filter.zod.ts`, #5285); both + * doors into the runtime lower it before a driver is reached, so a second + * implementation here is the ADR-0053 D-A1 divergence — the same one + * cloud's `RemoteTransport.buildWhereSQL` closed from its side (cloud#1075). + * + * The authored shapes are unchanged and still land on the same filter + * document; they simply travel `parseFilterAST` to get here, which is what + * the left column below asserts. + */ + it.each([ + ['single comparison tuple', ['name', '=', 'Alice'], { name: 'Alice' }], + ['!= operator', ['status', '!=', 'deleted'], { status: { $ne: 'deleted' } }], + ['>', ['age', '>', 18], { age: { $gt: 18 } }], + ['>=', ['age', '>=', 18], { age: { $gte: 18 } }], + ['<', ['age', '<', 65], { age: { $lt: 65 } }], + ['<=', ['score', '<=', 100], { score: { $lte: 100 } }], + ['in', ['status', 'in', ['active', 'pending']], { status: { $in: ['active', 'pending'] } }], + ['contains', ['name', 'contains', 'test'], { name: { $regex: 'test', $options: 'i' } }], + ['implicit AND list', [['name', '=', 'Alice'], ['age', '>', 18]], + { $and: [{ name: 'Alice' }, { age: { $gt: 18 } }] }], + // PREFIX is the declared spelling of a logical join. The infix form this + // module used to accept has no lowering at all — pinned as a refusal + // below rather than translated. + ['prefix OR group', ['or', ['role', '=', 'admin'], ['role', '=', 'manager']], + { $or: [{ role: 'admin' }, { role: 'manager' }] }], + ])('the authored %s still lands on the same filter document, via parseFilterAST', ( + _label, authored, expected, + ) => { + expect(translateFilter(parseFilterAST(authored))).toEqual(expected); + }); + + it.each([ + ['comparison tuple', ['name', '=', 'Alice']], + ['condition list', [['name', '=', 'Alice'], ['age', '>', 18]]], + ['infix join (the undeclared dialect)', [['role', '=', 'admin'], 'or', ['role', '=', 'manager']]], + ])('refuses a raw array %s', (_label, where) => { + expect(() => translateFilter(where)).toThrow(/A filter ARRAY reached the driver/); + }); + + it('an empty array is still "no filter", not a refusal', () => { + expect(translateFilter([])).toEqual({}); + }); + + it('the `createdAt` -> `created_at` alias went with the dialect', () => { + // It only ever existed on the array path (`mapFieldName`, called solely + // from the deleted `translateComparison`); the object path never applied + // it. A consumer-side alias is debt by AGENTS.md PD #12, so it is not + // re-added here — `created_at` is the declared field name. + expect(translateFilter({ createdAt: { $gt: '2024-01-01' } })).toEqual({ + createdAt: { $gt: '2024-01-01' }, }); - }); - - it('translates contains operator', () => { - const result = translateFilter(['name', 'contains', 'test']); - expect(result).toEqual({ name: { $regex: 'test', $options: 'i' } }); - }); - - it('translates multiple conditions with AND', () => { - const result = translateFilter([['name', '=', 'Alice'], ['age', '>', 18]]); - expect(result).toEqual({ - $and: [{ name: 'Alice' }, { age: { $gt: 18 } }], - }); - }); - - it('translates conditions with OR connector', () => { - const result = translateFilter([['role', '=', 'admin'], 'or', ['role', '=', 'manager']]); - expect(result).toEqual({ - $or: [{ role: 'admin' }, { role: 'manager' }], - }); - }); - - it('maps createdAt to created_at', () => { - expect(translateFilter(['createdAt', '>', '2024-01-01'])).toEqual({ + expect(translateFilter(parseFilterAST([['created_at', '>', '2024-01-01']]))).toEqual({ created_at: { $gt: '2024-01-01' }, }); }); diff --git a/packages/plugins/driver-mongodb/src/mongodb-filter.ts b/packages/plugins/driver-mongodb/src/mongodb-filter.ts index 6dc7382854..5133671115 100644 --- a/packages/plugins/driver-mongodb/src/mongodb-filter.ts +++ b/packages/plugins/driver-mongodb/src/mongodb-filter.ts @@ -13,24 +13,54 @@ * - Special: `{ field: { $null, $exists } }` * - Logical: `{ $and, $or, $not }` * - Range: `{ field: { $between: [min, max] } }` - * - Legacy array-style: `[field, op, value]` + * + * NOT supported, deliberately: the legacy ARRAY spelling (`[field, op, value]`, + * `[[…], 'or', […]]`). `where` is a `FilterCondition` object by declaration, and + * `FilterArray` is INPUT-ONLY authoring sugar lowered through `parseFilterAST` + * at the engine and protocol doors before any driver is reached (#5158, ruling + * C). A second compiler for it here is the divergence ADR-0053 D-A1 forbids — + * it is refused loudly instead, matching driver-sql, driver-memory and cloud's + * `RemoteTransport.buildWhereSQL`. */ import type { Filter } from 'mongodb'; import { nextUtcCalendarDay } from '@objectstack/core'; +import { StandardErrorCode } from '@objectstack/spec/api'; import { coerceTemporalValue, type TemporalFieldKind, type TemporalFieldKindResolver, } from './mongodb-temporal.js'; +/** + * [#5158] A `FilterArray` reached the driver unlowered — the twin of + * `driver-sql`'s and `driver-memory`'s `filterArrayReachedDriverError`, word + * for word so the three backends answer one condition with one wording, in the + * ADR-0112 envelope (`400 INVALID_FILTER`) every sibling filter refusal speaks. + */ +function filterArrayReachedDriverError(filters: unknown[]): Error { + const err = new Error( + `A filter ARRAY reached the driver: ${JSON.stringify(filters)}. ` + + `'where' is a FilterCondition object; the array form ('FilterArray') is input-only ` + + `authoring sugar and is lowered by @objectstack/spec parseFilterAST() at the engine ` + + `and protocol doors before any driver sees it (#5158). This driver no longer carries a ` + + `second compiler for it — call through ObjectQL, or lower the value yourself with ` + + `parseFilterAST(). Note the INFIX join form ([condA, "or", condB]) has no lowering at ` + + `all: write the prefix form ["or", condA, condB].`, + ) as Error & { code?: string; status?: number }; + err.code = StandardErrorCode.enum.INVALID_FILTER; + err.status = 400; + return err; +} + /** * Translate an ObjectStack `where` clause into a MongoDB filter document. * * The `where` clause can be: * 1. A FilterCondition object (MongoDB-style with `$` operators) - * 2. A legacy array-style filter `[[field, op, value], 'or', [field, op, value]]` - * 3. A plain key-value object for implicit equality + * 2. A plain key-value object for implicit equality + * + * An ARRAY is refused (#5158) — see the module header. * * `temporalKind` resolves the declared temporal type of a field so comparands * land in the column's storage form (#4047) — a `Field.datetime` comparand @@ -44,9 +74,10 @@ export function translateFilter( ): Filter { if (!where) return {}; - // Legacy array-style filters if (Array.isArray(where)) { - return translateArrayFilter(where, temporalKind); + // `[]` still means "no filter" — unchanged. + if (where.length === 0) return {}; + throw filterArrayReachedDriverError(where); } if (typeof where !== 'object') return {}; @@ -227,155 +258,6 @@ function translateFieldOperators( return result; } -/** - * Translate legacy array-style filters into a MongoDB filter. - * - * Array format: `[[field, op, value], 'or', [field, op, value], ...]` - * Nested arrays are treated as grouped conditions. - */ -function translateArrayFilter( - filters: unknown[], - temporalKind?: TemporalFieldKindResolver, -): Filter { - if (filters.length === 0) return {}; - - // Check if this is a single comparison tuple: [field, op, value] - if ( - filters.length === 3 && - typeof filters[0] === 'string' && - typeof filters[1] === 'string' && - !Array.isArray(filters[0]) && - (typeof filters[2] !== 'object' || filters[2] === null || Array.isArray(filters[2])) - ) { - // Only treat as tuple if filters[1] looks like an operator (not another field name - // that could be part of a nested array filter) - const possibleOp = filters[1] as string; - const isOperator = ['=', '!=', '<>', '>', '>=', '<', '<=', 'in', 'nin', 'eq', 'ne', - 'gt', 'gte', 'lt', 'lte', 'contains', 'like'].includes(possibleOp) || possibleOp.startsWith('$'); - if (isOperator) { - return translateComparison(filters[0], possibleOp, filters[2], temporalKind); - } - } - - // Parse mixed array of conditions and logical connectors - const groups: { logic: 'and' | 'or'; filter: Filter }[] = []; - let nextLogic: 'and' | 'or' = 'and'; - - for (const item of filters) { - if (typeof item === 'string') { - const lower = item.toLowerCase(); - if (lower === 'or') nextLogic = 'or'; - else if (lower === 'and') nextLogic = 'and'; - continue; - } - - if (Array.isArray(item)) { - // Could be a comparison tuple or a nested group - const isTuple = - item.length === 3 && - typeof item[0] === 'string' && - typeof item[1] === 'string' && - !Array.isArray(item[2]); - - const translated = isTuple - ? translateComparison(item[0], item[1], item[2], temporalKind) - : translateArrayFilter(item, temporalKind); - - groups.push({ logic: nextLogic, filter: translated }); - nextLogic = 'and'; - } - } - - if (groups.length === 0) return {}; - if (groups.length === 1) return groups[0].filter; - - // Check if all are AND - const hasOr = groups.some((g) => g.logic === 'or'); - if (!hasOr) { - return { $and: groups.map((g) => g.filter) }; - } - - // Build $or groups: consecutive AND conditions are grouped together - const orGroups: Filter[][] = [[]]; - for (const g of groups) { - if (g.logic === 'or') { - orGroups.push([g.filter]); - } else { - orGroups[orGroups.length - 1].push(g.filter); - } - } - - const orClauses = orGroups.map((group) => { - if (group.length === 1) return group[0]; - return { $and: group }; - }); - - if (orClauses.length === 1) return orClauses[0]; - return { $or: orClauses }; -} - -/** - * Translate a single comparison `[field, operator, value]` tuple. - */ -function translateComparison( - field: string, - op: string, - value: unknown, - temporalKind?: TemporalFieldKindResolver, -): Filter { - const mappedField = mapFieldName(field); - // Resolve against the MAPPED name: `createdAt` is an alias of the declared - // `created_at`, and the field kinds are indexed under declared names. - const store = (v: unknown) => coerceTemporalValue(v, temporalKind?.(mappedField)); - - switch (op) { - case '=': - case 'eq': - return { [mappedField]: store(value) }; - case '!=': - case '<>': - case 'ne': - return { [mappedField]: { $ne: store(value) } }; - case '>': - case 'gt': - return { [mappedField]: { $gt: store(value) } }; - case '>=': - case 'gte': - return { [mappedField]: { $gte: store(value) } }; - case '<': - case 'lt': - return { [mappedField]: { $lt: store(value) } }; - case '<=': - case 'lte': { - // Bare-day upper bound → half-open, `$lte`'s whole-day rule (#4042). - // Calendar first, storage form second — see translateFieldOperators. - const nextDay = nextUtcCalendarDay(value); - return { - [mappedField]: nextDay != null ? { $lt: store(nextDay) } : { $lte: store(value) }, - }; - } - case 'in': - return { [mappedField]: { $in: store(value) as unknown[] } }; - case 'nin': - return { [mappedField]: { $nin: store(value) as unknown[] } }; - case 'contains': - case 'like': - return { [mappedField]: { $regex: escapeRegex(String(value)), $options: 'i' } }; - default: - // Pass through for any standard MongoDB operator - return { [mappedField]: { [`$${op}`]: value } }; - } -} - -/** - * Map common ObjectStack field name aliases. - */ -function mapFieldName(field: string): string { - if (field === 'createdAt') return 'created_at'; - if (field === 'updatedAt') return 'updated_at'; - return field; -} - /** * Escape special regex characters in a string. */ diff --git a/packages/plugins/driver-sql/src/sql-driver-calendar-day-upper-bound.test.ts b/packages/plugins/driver-sql/src/sql-driver-calendar-day-upper-bound.test.ts index c7624bf783..37d0e25c69 100644 --- a/packages/plugins/driver-sql/src/sql-driver-calendar-day-upper-bound.test.ts +++ b/packages/plugins/driver-sql/src/sql-driver-calendar-day-upper-bound.test.ts @@ -23,6 +23,7 @@ */ import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import { parseFilterAST } from '@objectstack/spec/data'; import { SqlDriver } from '../src/index.js'; import { LegacyStorageDriver } from '../src/legacy-datetime-storage.testkit.js'; @@ -137,14 +138,22 @@ describe('bare-day $lte on Field.datetime — the #3777 repro', () => { expect(ids(found)).toEqual(['t_evening', 't_midnight', 't_morning', 't_old']); }); - it('applies to the array (`[field, op, value]`) where spelling too', async () => { + it('applies to the authored array spelling, lowered the declared way (#5158)', async () => { + // The array form used to be compiled by the driver itself. It is now + // input-only sugar lowered at the engine/protocol doors through + // `parseFilterAST`, so this asserts the same authored filter still reaches + // the same rows — via the one route that exists. Note the INFIX join + // (`[condA, 'and', condB]`) has no lowering at all and is refused at the + // door; the declared spelling of "both bounds" is the prefix group. const found = await driver.find('task', { - where: [['created_at', '<=', '2026-07-28'], 'and', ['created_at', '>=', '2026-04-29']], + where: parseFilterAST( + ['and', ['created_at', '<=', '2026-07-28'], ['created_at', '>=', '2026-04-29']], + ) as any, } as any); expect(ids(found)).toEqual(['t_evening', 't_midnight', 't_morning', 't_yesterday']); const between = await driver.find('task', { - where: [['created_at', 'between', ['2026-04-29', '2026-07-28']]], + where: parseFilterAST([['created_at', 'between', ['2026-04-29', '2026-07-28']]]) as any, } as any); expect(ids(between)).toEqual(['t_evening', 't_midnight', 't_morning', 't_yesterday']); }); diff --git a/packages/plugins/driver-sql/src/sql-driver-cross-field-reference.test.ts b/packages/plugins/driver-sql/src/sql-driver-cross-field-reference.test.ts index 7a79fc688e..66e3b682e7 100644 --- a/packages/plugins/driver-sql/src/sql-driver-cross-field-reference.test.ts +++ b/packages/plugins/driver-sql/src/sql-driver-cross-field-reference.test.ts @@ -35,7 +35,7 @@ import { describe, it, expect, beforeEach } from 'vitest'; import { SqlDriver } from '../src/index.js'; -import type { FilterCondition } from '@objectstack/spec/data'; +import { parseFilterAST, type FilterCondition } from '@objectstack/spec/data'; /** The shape `mapDataError` / `sendError` read off a thrown driver error. */ interface WireBearingError extends Error { @@ -210,8 +210,8 @@ describe('[#5041] SqlDriver refuses `$field` cross-field comparison in the ADR-0 expect(rows.map((r: any) => r.id)).toEqual(['1']); }); - it('an array triple with a scalar still matches', async () => { - const rows = await find([['amount', '>', 1]]); + it('an authored array triple with a scalar still matches, once lowered (#5158)', async () => { + const rows = await find(parseFilterAST([['amount', '>', 1]])); expect(rows.map((r: any) => r.id)).toEqual(['1']); }); diff --git a/packages/plugins/driver-sql/src/sql-driver-datetime-filter-text-storage.test.ts b/packages/plugins/driver-sql/src/sql-driver-datetime-filter-text-storage.test.ts index 840e08e016..41eb215682 100644 --- a/packages/plugins/driver-sql/src/sql-driver-datetime-filter-text-storage.test.ts +++ b/packages/plugins/driver-sql/src/sql-driver-datetime-filter-text-storage.test.ts @@ -19,6 +19,7 @@ */ import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import { parseFilterAST } from '@objectstack/spec/data'; import { SqlDriver } from '../src/index.js'; import { LegacyStorageDriver } from '../src/legacy-datetime-storage.testkit.js'; @@ -155,9 +156,14 @@ describe('SqlDriver datetime filters on ISO-TEXT-stored columns (#3912)', () => expect(present.map((r: any) => r.id)).toEqual(['l1', 'l2', 'l3']); }); - it('matches the AST array filter form', async () => { + it('matches the authored array form, lowered the declared way (#5158)', async () => { + // The driver no longer compiles the array spelling — it is input-only + // sugar lowered at the engine/protocol doors. Same authored filter, same + // row, via the one route that exists. const rows = await driver.find('lead', { - where: [['created_date', '>=', '2026-01-01'], ['created_date', '<', '2026-05-01']], + where: parseFilterAST( + [['created_date', '>=', '2026-01-01'], ['created_date', '<', '2026-05-01']], + ) as any, } as any); expect(rows.map((r: any) => r.id)).toEqual(['l2']); }); diff --git a/packages/plugins/driver-sql/src/sql-driver-external-columnmap.test.ts b/packages/plugins/driver-sql/src/sql-driver-external-columnmap.test.ts index 759ca5cb88..9d8262e859 100644 --- a/packages/plugins/driver-sql/src/sql-driver-external-columnmap.test.ts +++ b/packages/plugins/driver-sql/src/sql-driver-external-columnmap.test.ts @@ -13,6 +13,7 @@ import { describe, it, expect, afterAll } from 'vitest'; import { rmSync } from 'node:fs'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; +import { parseFilterAST } from '@objectstack/spec/data'; import { SqlDriver } from '../src/index.js'; // Remote table with deliberately non-matching column names. @@ -89,8 +90,8 @@ describe('SqlDriver external columnMap (ADR-0015 §18)', () => { // object form: region -> region_code const eu = await d.find('cm_customer', { where: { region: 'EU' } }); expect(eu.map((r: any) => r.name).sort()).toEqual(['Borealis', 'Cyan']); - // array criterion: value -> ltv - const big = await d.find('cm_customer', { where: [['value', '>', 200]] }); + // authored array criterion, lowered the declared way (#5158): value -> ltv + const big = await d.find('cm_customer', { where: parseFilterAST([['value', '>', 200]]) as any }); expect(big.map((r: any) => r.name).sort()).toEqual(['Aurora', 'Borealis']); // mongo operator: value $gte const gte = await d.find('cm_customer', { where: { value: { $gte: 312 } } }); diff --git a/packages/plugins/driver-sql/src/sql-driver-filter-no-silent-drop.test.ts b/packages/plugins/driver-sql/src/sql-driver-filter-no-silent-drop.test.ts index 27aae14e04..ab0d31d15a 100644 --- a/packages/plugins/driver-sql/src/sql-driver-filter-no-silent-drop.test.ts +++ b/packages/plugins/driver-sql/src/sql-driver-filter-no-silent-drop.test.ts @@ -9,20 +9,40 @@ * so every one was skipped and **no WHERE clause was emitted at all**: the caller * asked to filter and silently received every row. * - * A bare triple reaches a driver only when `isFilterAST()` refused it, i.e. its - * operator is outside `VALID_AST_OPERATORS`, so `parseFilterAST()` never - * converted it and the raw array arrived as `where`. That is reachable from - * ordinary authoring: `before`/`after` are canonical `VIEW_FILTER_OPERATORS` - * members which `VALID_AST_OPERATORS` does not accept. + * ## What #5158 changed here, and what it deliberately did not * - * The nested and `$`-object paths already threw on the same class of input, so - * the three code paths disagreed about one query. These tests pin the loud - * behaviour, and pin that well-formed filters still compile. + * #3948's CONDITION is unchanged and is the reason this file still exists: an + * unapplied filter must not look like a satisfied one. What changed is WHERE + * that condition is enforced, because the array walk it guarded is gone. + * + * `FilterArray` is INPUT-ONLY authoring sugar (spec `data/filter.zod.ts`, + * #5285). Both doors into the runtime lower it through `parseFilterAST` before + * a driver is reached — the protocol face (`metadata-protocol`) and the engine + * (`ObjectQL`, maintainer ruling C on #5158). This driver used to carry a + * SECOND compiler for the array spelling, including an INFIX join dialect + * (`[condA, 'or', condB]`) that no schema declared and `parseFilterAST` cannot + * express; cloud's `RemoteTransport.buildWhereSQL` has refused the same input + * since cloud#1075. That fork — same query, two answers — is what the deletion + * closes. + * + * So the pins below moved rather than vanished: + * + * | #3948 shape | refused now by | + * |---|---| + * | bare triple with an off-vocabulary operator | the engine door (`engine-filter-array-lowering.test.ts`), and here as an array | + * | `[42]` / `[null]` element | same | + * | `['and']` with nothing to join | same | + * | unsupported operator inside a well-formed condition | THIS driver, on the lowered `FilterCondition` (below) | + * | `[]` — "no filter" | unchanged everywhere: still no filter | + * + * The two cases this file used to assert COMPILED — a nested condition array + * and an infix logical join — are the dialect itself. They are pinned as + * refusals now, deliberately (#5158 ruling C), not dropped. */ import { describe, it, expect, beforeEach } from 'vitest'; import { SqlDriver } from '../src/index.js'; -import type { FilterCondition } from '@objectstack/spec/data'; +import { parseFilterAST, type FilterCondition } from '@objectstack/spec/data'; describe('SqlDriver rejects an uncompilable filter instead of dropping it', () => { let driver: SqlDriver; @@ -49,55 +69,84 @@ describe('SqlDriver rejects an uncompilable filter instead of dropping it', () = await driver.create('deal', { id: '2', stage: 'lost', amount: 20 }); }); - // The cast is the point of the file: every `where` below is a shape - // `isFilterAST()` REFUSED, fed in deliberately to prove the driver throws + // The cast is the point of the file: every `where` below is a shape the + // declared contract forbids, fed in deliberately to prove the driver throws // rather than dropping it. `unknown` is the honest parameter type; the cast - // is what lets an off-spec value reach a parameter that forbids it. + // is what lets an off-spec value reach a parameter that forbids it. Note + // `FilterCondition` is an index-signature type, so an array is ASSIGNABLE to + // it — the type layer never excluded this input, which is exactly why the + // runtime refusal has to. const find = (where: unknown) => driver.find('deal', { object: 'deal', fields: ['id'], where: where as FilterCondition }); - it('throws on a bare triple whose operator the AST gate refused', async () => { - // The exact shape a stored single-condition `before` view produced. - await expect(find(['close_date', 'before', '2024-01-01'])) - .rejects.toThrow(/Unrecognized filter operator "close_date"/); + // ── the array dialect is gone: every array shape is refused ─────────── + + it.each([ + ['bare triple whose operator the AST gate refused', ['close_date', 'before', '2024-01-01']], + ['bare triple with a bogus operator', ['stage', 'sounds_like', 'won']], + ['element that is neither a keyword nor a condition', [42]], + ['null element', [null]], + ['nested condition array (the legacy compiled form)', [['stage', '=', 'won']]], + ['infix logical join (the undeclared dialect)', [['stage', '=', 'won'], 'or', ['stage', '=', 'lost']]], + ['logical node with nothing to join', ['and']], + ])('refuses an array %s', async (_label, where) => { + await expect(find(where)).rejects.toThrow(/A filter ARRAY reached the driver/); }); - it('does not silently return every row for that filter', async () => { - // The regression itself: before the fix this resolved with BOTH rows. + it('does not silently return every row for any of them', async () => { + // The original regression: before #3948 this resolved with BOTH rows. await expect(find(['stage', 'sounds_like', 'won'])).rejects.toThrow(); }); - it('throws on a filter element that is neither a keyword nor a condition', async () => { - await expect(find([42 as any])).rejects.toThrow(/Unrecognized filter element of type "number"/); - await expect(find([null as any])).rejects.toThrow(/Unrecognized filter element of type "null"/); + it('the refusal names the lowering, so the caller knows where the fix goes', async () => { + const err = await find([['stage', '=', 'won']]).then(() => null, (e: Error) => e); + expect(err?.message).toContain('parseFilterAST'); + expect(err?.message).toContain('input-only'); + // The infix form has no lowering at all — say so, or the reader assumes + // `parseFilterAST` would have handled it. + expect(err?.message).toContain('prefix form'); }); - it('still throws on an unsupported operator inside a well-formed condition', async () => { - // Pre-existing behaviour, pinned so the two paths cannot diverge again. - await expect(find([['stage', 'sounds_like', 'won']])) - .rejects.toThrow(/Unsupported filter operator "sounds_like"/); + it('carries the ADR-0112 envelope, like every sibling filter refusal', async () => { + const err = await find([42]).then(() => null, (e: any) => e); + expect(err?.code).toBe('INVALID_FILTER'); + expect(err?.status).toBe(400); + expect(err?.message).not.toContain('[sql-driver]'); }); - it('compiles a nested condition array', async () => { - const rows = await find([['stage', '=', 'won']]); + // ── what the authored shapes do instead: lower, then compile ────────── + + it('the authored nested condition compiles once lowered', async () => { + const rows = await find(parseFilterAST([['stage', '=', 'won']])); expect(rows.map((r: any) => r.id)).toEqual(['1']); }); - it('compiles an infix logical join', async () => { - // This path's legacy array form is INFIX — `[condA, 'or', condB]`. The - // prefix spec-AST form `['or', condA, condB]` reaches the driver already - // converted to `{$or: […]}` by `parseFilterAST()`, so it takes the - // object branch instead and never lands here. - const rows = await find([['stage', '=', 'won'], 'or', ['stage', '=', 'lost']]); + it('a logical join compiles once lowered — in its declared PREFIX spelling', async () => { + // `['or', condA, condB]` is the spec's spelling and the only one with a + // lowering. The infix form this driver used to compile has none, which is + // why it is refused above rather than translated. + const rows = await find(parseFilterAST(['or', ['stage', '=', 'won'], ['stage', '=', 'lost']])); expect(rows.map((r: any) => r.id).sort()).toEqual(['1', '2']); }); + // ── unchanged: the object form, and the meaning of `[]` ─────────────── + + it('still throws on an unsupported operator inside a well-formed condition', async () => { + // Pre-existing behaviour on the shape the driver still compiles, pinned so + // the refusal cannot regress into a silent drop one layer down. + await expect(find({ stage: { $sounds_like: 'won' } })) + .rejects.toThrow(/Unsupported filter operator "\$sounds_like"/); + }); + it('compiles an object-form filter', async () => { const rows = await find({ stage: 'lost' }); expect(rows.map((r: any) => r.id)).toEqual(['2']); }); it('leaves an empty filter alone (no filter is not a failed filter)', async () => { + // Unchanged by #5158 and deliberately so: `[]` means "no filter" at every + // layer — `parseFilterAST([])` is `undefined`, the engine deletes the key, + // and this driver returns early rather than refusing. const rows = await find([]); expect(rows).toHaveLength(2); }); diff --git a/packages/plugins/driver-sql/src/sql-driver-filter-refusal-envelope.test.ts b/packages/plugins/driver-sql/src/sql-driver-filter-refusal-envelope.test.ts index 4ba9bcb56c..f3455c1f94 100644 --- a/packages/plugins/driver-sql/src/sql-driver-filter-refusal-envelope.test.ts +++ b/packages/plugins/driver-sql/src/sql-driver-filter-refusal-envelope.test.ts @@ -91,7 +91,10 @@ describe('[#4436] SqlDriver filter refusals carry INVALID_FILTER and leak no dri const cases: Array<[string, unknown, string]> = [ ['legacy triple, unsupported operator', [['stage', 'sounds_like', 'won']], 'sounds_like'], ['bare comparison triple', ['close_date', 'before', '2024-01-01'], 'close_date'], - ['filter element of the wrong type', [42], 'number'], + // [#5158] The message names the ARRAY, not the offending element's typeof: + // the driver no longer walks a filter array, so it has no element to + // describe. `42` is the caller's own value, echoed back. + ['filter element of the wrong type', [42], '42'], ['null filter element', [null], 'null'], ['legacy `between` with a bad operand', [['amount', 'between', 5]], 'between'], ['$between with a bad operand', { amount: { $between: 5 } }, '$between'], diff --git a/packages/plugins/driver-sql/src/sql-driver-null-operators.test.ts b/packages/plugins/driver-sql/src/sql-driver-null-operators.test.ts index 1b17b868de..1b70734b4b 100644 --- a/packages/plugins/driver-sql/src/sql-driver-null-operators.test.ts +++ b/packages/plugins/driver-sql/src/sql-driver-null-operators.test.ts @@ -1,6 +1,7 @@ // Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import { parseFilterAST, type FilterCondition } from '@objectstack/spec/data'; import { SqlDriver } from '../src/index.js'; /** @@ -8,6 +9,14 @@ import { SqlDriver } from '../src/index.js'; * filters a real SQL rendering and must NOT forward an unknown operator to Knex * verbatim (which silently returned the whole table on a null comparand — a * filter-bypass on permission/assignment-scoped list views). + * + * [#5158] The array-format block below used to hand the raw + * `[['assignee','isnull',true]]` to the driver, which carried its own compiler + * for it. That dialect is deleted: `FilterArray` is input-only sugar, lowered + * through `parseFilterAST` at the engine/protocol doors. The authored shapes + * are unchanged — they now travel the declared route, which is what the block + * calls explicitly. #2704's condition is untouched: `isnull` must still render + * IS NULL and an unknown operator must still throw rather than scan the table. */ describe('SqlDriver — null / empty operators (#2704)', () => { let driver: SqlDriver; @@ -40,37 +49,52 @@ describe('SqlDriver — null / empty operators (#2704)', () => { const ids = (rows: any[]) => rows.map((r) => r.id).sort(); - describe('array-format where', () => { + describe('authored array form, lowered the declared way (#5158)', () => { + // The exact route the engine and the protocol face take: the caller's + // `FilterArray` through `parseFilterAST`, and only the `FilterCondition` + // reaches the driver. Feeding the array raw is what this driver no longer + // does — pinned at the bottom of this block. + const lowered = (where: unknown) => + driver.find('tasks', { object: 'tasks', where: parseFilterAST(where) as FilterCondition }); + it('equals + null → IS NULL (baseline that already worked)', async () => { - const rows = await driver.find('tasks', { object: 'tasks', where: [['assignee', '=', null]] }); - expect(ids(rows)).toEqual(['2', '4']); + expect(ids(await lowered([['assignee', '=', null]]))).toEqual(['2', '4']); }); it.each(['is_null', 'isnull', 'is_empty'])('%s → IS NULL', async (op) => { - const rows = await driver.find('tasks', { object: 'tasks', where: [['assignee', op, true]] }); - expect(ids(rows)).toEqual(['2', '4']); + expect(ids(await lowered([['assignee', op, true]]))).toEqual(['2', '4']); }); it.each(['is_not_null', 'isnotnull', 'is_not_empty'])('%s → IS NOT NULL', async (op) => { - const rows = await driver.find('tasks', { object: 'tasks', where: [['assignee', op, true]] }); - expect(ids(rows)).toEqual(['1', '3']); + expect(ids(await lowered([['assignee', op, true]]))).toEqual(['1', '3']); }); it('!= null → IS NOT NULL (not a `<> NULL` that matches nothing)', async () => { - const rows = await driver.find('tasks', { object: 'tasks', where: [['assignee', '!=', null]] }); - expect(ids(rows)).toEqual(['1', '3']); + expect(ids(await lowered([['assignee', '!=', null]]))).toEqual(['1', '3']); }); it('unknown operator throws instead of returning the whole table', async () => { - await expect( - driver.find('tasks', { object: 'tasks', where: [['assignee', 'totally_bogus', null]] }), - ).rejects.toThrow(/Unsupported filter operator/); + // `parseFilterAST` is lenient here on purpose (`$${op}` fallback) — the + // ENGINE door gates on `isFilterAST` first and refuses this a layer + // earlier (engine-filter-array-lowering.test.ts). Either way it throws; + // what #2704 forbids is the whole-table answer. + await expect(lowered([['assignee', 'totally_bogus', null]])) + .rejects.toThrow(/Unsupported filter operator/); }); it('count with is_null is scoped, not the whole table', async () => { - const count = await driver.count('tasks', { object: 'tasks', where: [['assignee', 'isnull', true]] }); + const count = await driver.count('tasks', { + object: 'tasks', + where: parseFilterAST([['assignee', 'isnull', true]]) as FilterCondition, + }); expect(count).toBe(2); }); + + it('the raw array is refused by the driver — the dialect is gone (#5158)', async () => { + await expect( + driver.find('tasks', { object: 'tasks', where: [['assignee', 'isnull', true]] as any }), + ).rejects.toThrow(/A filter ARRAY reached the driver/); + }); }); describe('object-format where ($-operators)', () => { diff --git a/packages/plugins/driver-sql/src/sql-driver-queryast.test.ts b/packages/plugins/driver-sql/src/sql-driver-queryast.test.ts index b2e99f5383..59df16138c 100644 --- a/packages/plugins/driver-sql/src/sql-driver-queryast.test.ts +++ b/packages/plugins/driver-sql/src/sql-driver-queryast.test.ts @@ -103,7 +103,7 @@ describe('SqlDriver (QueryAST Format)', () => { it('should support QueryAST with where, offset, limit, and orderBy', async () => { const results = await driver.find('products', { - where: [['category', '=', 'Electronics']], + where: { category: 'Electronics' }, offset: 1, limit: 1, orderBy: [{ field: 'price', order: 'asc' as const }], @@ -136,7 +136,7 @@ describe('SqlDriver (QueryAST Format)', () => { it('should support count with QueryAST where clause', async () => { const count = await driver.count('products', { - where: [['price', '>', 300]], + where: { price: { $gt: 300 } }, } as any); expect(count).toBe(3); }); @@ -191,7 +191,7 @@ describe('SqlDriver (QueryAST Format)', () => { it('should use "where" and ignore "filters" when both are present', async () => { const results = await driver.find('products', { - where: [['category', '=', 'Electronics']], + where: { category: 'Electronics' }, filters: [['category', '=', 'Furniture']], } as any); diff --git a/packages/plugins/driver-sql/src/sql-driver.ts b/packages/plugins/driver-sql/src/sql-driver.ts index 3f61c454b0..eab71a39b7 100644 --- a/packages/plugins/driver-sql/src/sql-driver.ts +++ b/packages/plugins/driver-sql/src/sql-driver.ts @@ -10,7 +10,6 @@ import type { QueryAST, DriverOptions, SchemaMode } from '@objectstack/spec/data'; import { parseAutonumberFormat, renderAutonumber, missingFieldValues, isTenancyDisabled, type AutonumberToken } from '@objectstack/spec/data'; import { STRUCTURED_JSON_TYPES, FILE_REFERENCE_TYPES, MULTI_OPTION_TYPES, NUMERIC_VALUE_TYPES } from '@objectstack/spec/data'; -import { canonicalAstOperator } from '@objectstack/spec/data'; // `defaultValue` runtime tokens (#4560). The DDL below asks the SPEC — not a // list of its own — which `defaultValue`s are instructions rather than literals, // so the engine and this driver can never disagree about what may become a @@ -468,6 +467,38 @@ function unsupportedFilterError(message: string): Error { return err; } +/** + * [#5158] A `FilterArray` reached the driver unlowered. + * + * `where` is a `FilterCondition` — `QueryASTSchema.where: FilterConditionSchema` + * — and `FilterArray` is INPUT-ONLY authoring sugar the spec declares separately + * (`spec/data/filter.zod.ts`, #5285). Both doors into the runtime lower it + * through `parseFilterAST` before any driver is reached: the protocol face + * (`metadata-protocol`, since #4121) and the engine (`ObjectQL`, ruling C). + * + * Until ruling C this driver carried a SECOND filter compiler for the array + * spelling — one that also accepted an INFIX join form (`[condA, 'or', condB]`) + * no schema ever declared and `parseFilterAST` cannot express. Two compilers + * for one query is the ADR-0053 D-A1 divergence, and it had already produced a + * live product fork: cloud's `RemoteTransport.buildWhereSQL` refuses the exact + * input this method used to compile (cloud#1075), with zero tests on either + * side of the split. Deleting the dialect converges them. + * + * The message names the lowering, not the SQL builder, because the fix is + * always at the caller: lower the value (or go through the engine, which does). + */ +function filterArrayReachedDriverError(filters: unknown[]): Error { + return unsupportedFilterError( + `A filter ARRAY reached the driver: ${JSON.stringify(filters)}. ` + + `'where' is a FilterCondition object; the array form ('FilterArray') is input-only ` + + `authoring sugar and is lowered by @objectstack/spec parseFilterAST() at the engine ` + + `and protocol doors before any driver sees it (#5158). This driver no longer carries a ` + + `second compiler for it — call through ObjectQL, or lower the value yourself with ` + + `parseFilterAST(). Note the INFIX join form ([condA, "or", condB]) has no lowering at ` + + `all: write the prefix form ["or", condA, condB].`, + ); +} + /** * [#5041] The referenced field name when `value` is a Filter Protocol FIELD * REFERENCE (`{ $field: 'other_column' }` — spec `FieldReferenceSchema` in @@ -516,10 +547,13 @@ function crossFieldComparisonError(field: string, op: string, ref: string, index } /** - * [#5041] Operators whose comparand is a single bound VALUE, in both spellings - * this driver accepts — the Filter Protocol `$`-form read by - * {@link SqlDriver.applyFilterCondition} and the canonicalised infix form read - * by {@link SqlDriver.applyAstComparison}. + * [#5041] Operators whose comparand is a single bound VALUE, in the Filter + * Protocol `$`-form read by {@link SqlDriver.applyFilterCondition} — the one + * spelling this driver still compiles. It also listed the canonicalised infix + * form, read by an `applyAstComparison` emitter deleted with the array dialect + * in #5158; the infix spellings stay in the set because + * {@link assertCompilableComparand} is called with an already-canonicalised + * operator on the reduction path. * * The list-shaped operators (`$in` / `$nin` / `$between`) are deliberately * ABSENT: an array is their legitimate comparand, and they compile through @@ -5995,9 +6029,30 @@ export class SqlDriver implements IDataDriver { protected applyFilters(builder: Knex.QueryBuilder, filters: any) { if (!filters) return; + + // [#5158] `where` is a `FilterCondition` OBJECT. It always was — the spec + // declares `QueryASTSchema.where: FilterConditionSchema` — but this method + // used to carry a SECOND compiler for the array spelling, including an + // INFIX dialect (`[condA, 'or', condB]`) that no schema ever declared and + // that `parseFilterAST` cannot express. `FilterArray` is now declared as + // INPUT-ONLY authoring sugar (`spec/data/filter.zod.ts`, #5285) and BOTH + // doors into the runtime lower it before a driver is reached: the protocol + // face (`metadata-protocol`) and the engine (`ObjectQL.find`/`findOne`/ + // `count`/`aggregate`/`update`/`delete`). So an array here is a bug in the + // caller, not a dialect to compile — and refusing it is what converges this + // driver with cloud's `RemoteTransport.buildWhereSQL`, which has refused + // the same input since cloud#1075. That fork had zero tests on either side. + if (Array.isArray(filters)) { + // `[]` keeps its meaning — "no filter", not a failed filter. Unchanged + // from every previous version of this method, and the same reading + // `parseFilterAST([])` gives it. + if (filters.length === 0) return; + throw filterArrayReachedDriverError(filters); + } + const table = this.coercionKey(builder); - if (!Array.isArray(filters) && typeof filters === 'object') { + if (typeof filters === 'object') { const hasMongoOperators = Object.keys(filters).some( (k) => k.startsWith('$') || @@ -6032,83 +6087,12 @@ export class SqlDriver implements IDataDriver { return; } - if (!Array.isArray(filters) || filters.length === 0) return; - - let nextJoin: 'and' | 'or' = 'and'; - - for (const item of filters) { - if (typeof item === 'string') { - const lower = item.toLowerCase(); - if (lower === 'or') { nextJoin = 'or'; continue; } - if (lower === 'and') { nextJoin = 'and'; continue; } - // Anything else is not a join keyword, and the only way a bare string - // reaches here is a comparison triple that `isFilterAST()` refused — - // its operator is outside `VALID_AST_OPERATORS`, so `parseFilterAST()` - // never converted it and the raw array arrived as `where`. Skipping it - // (the old behaviour) emitted NO predicate at all: the caller asked to - // filter and silently got every row. Fail loudly instead. #3948. - throw unsupportedFilterError( - `Unrecognized filter operator "${item}" in a comparison triple. ` + - `A filter array is either a logical node (["and"|"or", …]) or nested ` + - `conditions ([[field, op, value], …]); a bare [field, op, value] only ` + - `reaches the driver when its operator is outside @objectstack/spec ` + - `VALID_AST_OPERATORS, which leaves the filter unparsed. ` + - `Filter was: ${JSON.stringify(filters)}`, - ); - } - - if (Array.isArray(item)) { - const [fieldRaw, op, value] = item; - const isCriterion = typeof fieldRaw === 'string' && typeof op === 'string'; - - if (isCriterion) { - const localField = this.mapSortField(fieldRaw); - const field = this.remoteColumn(table, fieldRaw, localField); - const opLower = String(op).toLowerCase(); - const columnExpr = this.filterColumnExpr(table, localField, field); - // Calendar-day upper bounds (#3777) — same translation the - // Mongo-operator path applies, for the array (`[field, op, value]`) - // spelling of the identical comparison. - const dayRange = opLower === 'between' - ? this.calendarDayBetweenRewrite(table, localField, value) : null; - if (dayRange) { - (builder as any)[nextJoin === 'or' ? 'orWhere' : 'where']((qb: any) => { - if (columnExpr) { - this.applyNormalizedComparison(qb, 'and', columnExpr, '$gte', dayRange.lower); - this.applyNormalizedComparison(qb, 'and', columnExpr, '$lt', dayRange.upper); - } else { - qb.where(field, '>=', dayRange.lower).andWhere(field, '<', dayRange.upper); - } - }); - } else { - const rewrite = this.calendarDayUpperBoundRewrite(table, localField, opLower, value); - const coerced = rewrite ? rewrite.value : this.coerceFilterValue(table, localField, value); - this.applyAstComparison( - builder, nextJoin, field, rewrite?.op ?? op, value, coerced, - columnExpr, - ); - } - } else { - const method = nextJoin === 'or' ? 'orWhere' : 'where'; - (builder as any)[method]((qb: any) => { - this.applyFilters(qb, item); - }); - } - - nextJoin = 'and'; - continue; - } - - // Neither a join keyword nor a condition. Previously fell out of both - // branches and was dropped, so a malformed element silently narrowed - // nothing. Same reasoning as above: an unapplied filter must not look - // like a satisfied one. #3948. - throw unsupportedFilterError( - `Unrecognized filter element of type "${item === null ? 'null' : typeof item}" — ` + - `expected a logical keyword ("and"/"or") or a condition array. ` + - `Filter was: ${JSON.stringify(filters)}`, - ); - } + // A truthy non-object, non-array `where` (`'active'`, `42`) emits no + // predicate. Pre-existing behaviour on a shape only a cast can produce — + // the protocol face rejects it (`unusableFilterError`) and `FilterCondition` + // does not describe it. Untouched here on purpose: #5158 is about the ARRAY + // dialect, and widening the refusal is a separate change with its own + // blast radius. } /** @@ -6147,139 +6131,6 @@ export class SqlDriver implements IDataDriver { builder[rawMethod](`?? ${keyword} ? ESCAPE ?`, [field, pattern, '\\']); } - /** - * Apply one comparison node from the array-format (`[field, op, value]`) - * `where` to the Knex builder, honouring the operator whitelist from - * `@objectstack/spec` (`VALID_AST_OPERATORS`) plus the alias spellings the - * ObjectUI client emits (`isnull` / `isnotnull` / `is_empty`, …). - * - * Why this is NOT a thin `builder.where(field, op, value)` passthrough - * (issue #2704): an unrecognised operator used to be forwarded to Knex - * verbatim. Knex then either rejected it with a 400 (`is_empty` → - * "operator not permitted", blanking the whole grid) or — when the comparand - * was `null` — silently compiled a clause that matched EVERY row - * (`isnull` / `is`). On a permission- or assignment-scoped list view that - * silent full-table scan is a data leak, strictly worse than an error. So - * null predicates compile to a real `IS NULL` / `IS NOT NULL` (unified with - * the `{field, equals, null}` path), and any operator off the whitelist - * throws instead of ever reaching Knex. - * - * `columnExpr` (from {@link filterColumnExpr}) is the storage-normalised form - * of `field` — non-null only for a SQLite `Field.datetime`, where comparing the - * raw column would compare against whichever of the two stored forms the writer - * happened to produce (#3912). It is optional so the protected signature stays - * source-compatible for subclasses; omitting it just keeps the raw column. - */ - protected applyAstComparison( - builder: any, - join: 'and' | 'or', - field: string, - op: string, - rawValue: unknown, - coerced: unknown, - columnExpr?: { sql: string; bindings: any[] } | null, - ): void { - const where = join === 'or' ? 'orWhere' : 'where'; - const whereNull = join === 'or' ? 'orWhereNull' : 'whereNull'; - const whereNotNull = join === 'or' ? 'orWhereNotNull' : 'whereNotNull'; - // Fold every accepted spelling of one comparison onto a single infix form so - // the switch below has one case per comparison rather than one per spelling. - // `VALID_AST_OPERATORS` accepts `>`, `gt`, `greater_than`, `greaterthan` and - // `after` for the same thing; growing a private alias list here is how this - // driver and driver-memory drifted apart. #3948. - const opLower = canonicalAstOperator(String(op)); - - // #5041 — the array (`[field, op, value]`) spelling reaches Knex through a - // different emitter than the Filter Protocol one, and measured identically: - // `[['amount', 'gt', { $field: 'budget' }]]` also threw a bare TypeError. - // One filter condition gets one answer however it was spelled, so the same - // gate runs here, on the RAW value (pre-coercion). - assertCompilableComparand(field, opLower, rawValue); - - // Value comparisons on a mixed-storage column read it through the CASE; every - // other operator (null predicates, the LIKE family, a malformed `between`) - // declines and falls through to the ordinary handling below. - if (columnExpr && this.applyNormalizedComparison(builder, join, columnExpr, opLower, coerced)) return; - - switch (opLower) { - // Equality — 2-arg form so Knex renders `IS NULL` for a null comparand, - // keeping the `{field, equals, null}` path working. - case '=': - case '==': - builder[where](field, coerced); - return; - case '!=': - case '<>': - // `<> NULL` matches nothing; a null comparand means "has any value". - if (coerced == null) builder[whereNotNull](field); - else builder[where](field, '<>', coerced); - return; - case '>': - case '>=': - case '<': - case '<=': - case 'like': - case 'ilike': - builder[where](field, opLower, coerced); - return; - case 'in': - builder[join === 'or' ? 'orWhereIn' : 'whereIn'](field, coerced as any[]); - return; - case 'nin': - case 'not_in': - case 'notin': - builder[join === 'or' ? 'orWhereNotIn' : 'whereNotIn'](field, coerced as any[]); - return; - case 'between': { - const arr = Array.isArray(coerced) ? coerced : []; - if (arr.length !== 2) { - throw unsupportedFilterError(`Operator "between" on field "${field}" requires a [min, max] value array.`); - } - builder[join === 'or' ? 'orWhereBetween' : 'whereBetween'](field, arr as [any, any]); - return; - } - case 'contains': - this.applyLike(builder, where, field, rawValue, 'contains'); - return; - case 'notcontains': - case 'not_contains': - this.applyLike(builder, where, field, rawValue, 'contains', true); - return; - case 'startswith': - case 'starts_with': - this.applyLike(builder, where, field, rawValue, 'starts'); - return; - case 'endswith': - case 'ends_with': - this.applyLike(builder, where, field, rawValue, 'ends'); - return; - // Null / empty predicates — value-independent, unified with `equals`+null. - case 'is_null': - case 'isnull': - case 'is_empty': - case 'isempty': - case 'empty': - builder[whereNull](field); - return; - case 'is_not_null': - case 'isnotnull': - case 'is_not_empty': - case 'isnotempty': - case 'not_empty': - case 'notempty': - case 'is_set': - case 'set': - builder[whereNotNull](field); - return; - default: - throw unsupportedFilterError( - `Unsupported filter operator "${op}" on field "${field}". Supported operators: ` + - `=, !=, <, <=, >, >=, in, nin, between, contains, not_contains, starts_with, ends_with, ` + - `is_null, is_not_null (see @objectstack/spec VALID_AST_OPERATORS).`, - ); - } - } - /** * Compiles a Filter Protocol condition onto `builder`. * diff --git a/packages/plugins/driver-sqlite-wasm/src/sqlite-wasm-driver-queryast.test.ts b/packages/plugins/driver-sqlite-wasm/src/sqlite-wasm-driver-queryast.test.ts index c7eac28d80..7941914ac7 100644 --- a/packages/plugins/driver-sqlite-wasm/src/sqlite-wasm-driver-queryast.test.ts +++ b/packages/plugins/driver-sqlite-wasm/src/sqlite-wasm-driver-queryast.test.ts @@ -93,7 +93,7 @@ describe('SqliteWasmDriver (QueryAST Format)', () => { it('should support QueryAST with where, offset, limit, and orderBy', async () => { const results = await driver.find('products', { - where: [['category', '=', 'Electronics']], + where: { category: 'Electronics' }, offset: 1, limit: 1, orderBy: [{ field: 'price', order: 'asc' as const }], @@ -126,7 +126,7 @@ describe('SqliteWasmDriver (QueryAST Format)', () => { it('should support count with QueryAST where clause', async () => { const count = await driver.count('products', { - where: [['price', '>', 300]], + where: { price: { $gt: 300 } }, } as any); expect(count).toBe(3); }); @@ -181,7 +181,7 @@ describe('SqliteWasmDriver (QueryAST Format)', () => { it('should use "where" and ignore "filters" when both are present', async () => { const results = await driver.find('products', { - where: [['category', '=', 'Electronics']], + where: { category: 'Electronics' }, filters: [['category', '=', 'Furniture']], } as any);