diff --git a/.changeset/filter-logic-conformance-single-source.md b/.changeset/filter-logic-conformance-single-source.md new file mode 100644 index 0000000000..030fffa011 --- /dev/null +++ b/.changeset/filter-logic-conformance-single-source.md @@ -0,0 +1,52 @@ +--- +"@objectstack/spec": minor +--- + +feat(spec): one canonical conformance table for the filter logical combinators + +`FilterCondition` is evaluated by four independent implementations, and nothing +held them to a shared standard: + +| Backend | Where | +|---|---| +| SQL compiler | `driver-sql` `applyFilterCondition` | +| In-memory matcher | `driver-memory` `memory-matcher` | +| Record-at-a-time evaluator | `formula` `matchesFilterCondition` (RLS write-side `check`) | +| Read-scope SQL lowering | `service-analytics` `read-scope-sql` | + +In #3774 the SQL compiler OR-ed the contents *within* a `$or` branch instead of +AND-ing them, so every `$or` filter matched more rows than it should. The other +three were correct — but that was luck, not enforcement, and the divergence was +invisible until someone ran a real query. The fix for #3774 left three +near-identical shape tables copied across packages and the fourth backend +unlocked entirely, which is the same drift setup one step later. + +`@objectstack/spec/data` now exports the table itself: + +- `FILTER_LOGIC_ROWS` — a 2x2 truth table over two columns (so a wrongly-OR-ed + pair always shows up as extra ids rather than by luck of the data), plus the + record-scope columns real read scopes are written against. +- `FILTER_LOGIC_CASES` — 17 cases, each a `FilterCondition` and the ids it must + match: keys within a branch, multiple operators on one field, `$and`/`$or`/ + `$not` nesting in both key orders, and the scope shapes that occur in shipped + metadata. + +Each backend now has a thin test that feeds the rows through its own evaluator +and asserts the shared expectations. **Adding a case to the table adds it to all +four at once** — that is the point. + +Two things this bought immediately: + +- `read-scope-sql` — the compiler that lowers RLS read scopes for the analytics + path — is now verified by **executing** its SQL against a real engine and + comparing rows. It was previously only checked by asserting the emitted SQL + string, whose ceiling is the author's own reading of SQL. It passes unchanged. +- The table is a public export, so a third-party driver author can check a new + backend against the same standard. + +**Deliberate scope:** logical combinators only. The predicates are boring on +purpose — string equality, `$in`, `$ne`, `$gte`/`$lt`. Nothing here exercises +null handling, dates, numeric coercion, `LIKE` escaping or case sensitivity, +because those legitimately differ between a SQL engine and a JS matcher; folding +them in would make the table unpassable rather than more useful. A case belongs +in it only if **every** backend must agree. diff --git a/packages/formula/src/matches-filter-or-semantics.test.ts b/packages/formula/src/matches-filter-or-semantics.test.ts index 185cf7a014..e2ee8da142 100644 --- a/packages/formula/src/matches-filter-or-semantics.test.ts +++ b/packages/formula/src/matches-filter-or-semantics.test.ts @@ -1,78 +1,31 @@ // Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. /** - * `$or` semantics conformance for the record-at-a-time filter evaluator. + * Filter logical-combinator conformance for the record-at-a-time evaluator. * - * Companion to `driver-sql`'s `sql-driver-or-filter.test.ts` and - * `driver-memory`'s `memory-matcher-or-semantics.test.ts`: same shapes, same - * 2x2 fixture, same expected ids. driver-sql used to OR the field keys within a - * `$or` branch, widening the match set; this evaluator was already correct. + * The cases come from `@objectstack/spec/data` so this backend, `driver-sql`, + * `driver-memory` and `read-scope-sql` are all held to one standard — see + * `filter-logic-conformance.ts` for why that standard exists (#3774). Adding a + * case there adds it to all four at once; that is the point. * - * Agreement here is load-bearing rather than cosmetic: this evaluator decides - * the RLS `check` clause on writes while the SQL compiler decides the `where` - * on reads. If they disagree on `$or`, a record can be writable but unreadable - * (or worse, readable when the scope says otherwise). + * Agreement matters here beyond tidiness: this evaluator decides the RLS + * `check` clause on writes while the SQL compilers decide the `where` on reads. + * If they disagree, a record can be writable but unreadable — or readable when + * the scope says otherwise. */ import { describe, expect, it } from 'vitest'; +import { FILTER_LOGIC_CASES, FILTER_LOGIC_ROWS } from '@objectstack/spec/data'; import { matchesFilterCondition as m } from './matches-filter'; -const ROWS = [ - { id: '1', a: 'x', b: 'y', c: 'z' }, - { id: '2', a: 'x', b: 'zz', c: 'z' }, - { id: '3', a: 'qq', b: 'y', c: 'z' }, - { id: '4', a: 'qq', b: 'zz', c: 'z' }, -]; - -const ids = (filter: any): string[] => ROWS.filter((r) => m(r, filter)).map((r) => r.id); - -describe('matchesFilterCondition — $or semantics', () => { - it('ANDs the keys of a single multi-key branch', () => { - expect(ids({ $or: [{ a: 'x', b: 'y' }] })).toEqual(['1']); - }); - - it('ANDs the keys of each branch independently', () => { - expect(ids({ $or: [{ a: 'x', b: 'y' }, { a: 'qq', b: 'zz' }] })).toEqual(['1', '4']); - }); - - it('ANDs operator-object keys within a branch', () => { - expect(ids({ $or: [{ a: { $eq: 'x' }, b: { $ne: 'zz' } }] })).toEqual(['1']); - }); - - it('ANDs keys inside a $or nested in a $or branch', () => { - expect(ids({ $or: [{ $or: [{ a: 'x', b: 'zz' }] }] })).toEqual(['2']); - }); - - it('ANDs multiple operators on ONE field within a branch', () => { - expect(ids({ $or: [{ a: { $ne: 'qq', $eq: 'x' }, b: 'y' }] })).toEqual(['1']); - }); - - it('OR-s a $and branch against a sibling multi-key branch', () => { - expect(ids({ $or: [{ $and: [{ a: 'x' }, { b: 'y' }] }, { a: 'qq', b: 'zz' }] })).toEqual(['1', '4']); - }); - - it('ANDs a $and with a sibling key in the same branch, either order', () => { - expect(ids({ $or: [{ c: 'nope' }, { $and: [{ a: 'qq' }], b: 'y' }] })).toEqual(['3']); - expect(ids({ $or: [{ c: 'nope' }, { b: 'y', $and: [{ a: 'qq' }] }] })).toEqual(['3']); - }); - - it('keeps single-key $or branches as a plain OR', () => { - expect(ids({ $or: [{ a: 'x' }, { b: 'y' }] })).toEqual(['1', '2', '3']); - }); - - it('ANDs a $or with a sibling top-level field key', () => { - expect(ids({ $or: [{ a: 'x' }, { b: 'y' }], b: 'zz' })).toEqual(['2']); - }); - - it('does not widen an "own AND active, OR shared" read scope', () => { - const docs = [ - { id: 'own-active', owner: 'u1', status: 'active', shared_with: null }, - { id: 'own-archived', owner: 'u1', status: 'archived', shared_with: null }, - { id: 'other-active', owner: 'u2', status: 'active', shared_with: null }, - { id: 'shared', owner: 'u2', status: 'active', shared_with: 'u1' }, - ]; - const scope = { $or: [{ owner: 'u1', status: 'active' }, { shared_with: 'u1' }] }; - expect(docs.filter((d) => m(d, scope)).map((d) => d.id)).toEqual(['own-active', 'shared']); - }); +describe('matchesFilterCondition — filter logic conformance', () => { + for (const c of FILTER_LOGIC_CASES) { + it(c.name, () => { + const got = FILTER_LOGIC_ROWS.filter((r) => m(r as unknown as Record, c.filter)).map( + (r) => r.id, + ); + expect(got, c.note).toEqual(c.expected); + }); + } }); diff --git a/packages/plugins/driver-memory/src/memory-matcher-or-semantics.test.ts b/packages/plugins/driver-memory/src/memory-matcher-or-semantics.test.ts index 789e1137cb..29be0e348b 100644 --- a/packages/plugins/driver-memory/src/memory-matcher-or-semantics.test.ts +++ b/packages/plugins/driver-memory/src/memory-matcher-or-semantics.test.ts @@ -1,75 +1,24 @@ // Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. /** - * `$or` semantics conformance for the in-memory matcher. + * Filter logical-combinator conformance for the in-memory matcher. * - * Companion to `driver-sql`'s `sql-driver-or-filter.test.ts`: the same filter - * shapes, the same 2x2 fixture, the same expected ids. driver-sql used to - * compile a `$or` branch's own field keys with OR instead of AND, so - * `{$or:[{a,b}]}` matched strictly more rows than the Filter Protocol allows. - * This matcher was already correct — these cases exist so the two backends - * cannot silently drift apart again, since a read scope evaluated by one and - * pushed down by the other must agree. + * The cases come from `@objectstack/spec/data` so this backend, `driver-sql`, + * `formula`'s `matchesFilterCondition` and `read-scope-sql` are all held to one + * standard — see `filter-logic-conformance.ts` for why that standard exists + * (#3774). Adding a case there adds it to all four at once; that is the point. */ import { describe, it, expect } from 'vitest'; -import { match } from './memory-matcher.js'; - -const ROWS = [ - { id: '1', a: 'x', b: 'y', c: 'z' }, - { id: '2', a: 'x', b: 'zz', c: 'z' }, - { id: '3', a: 'qq', b: 'y', c: 'z' }, - { id: '4', a: 'qq', b: 'zz', c: 'z' }, -]; - -const ids = (filter: any): string[] => ROWS.filter((r) => match(r, filter)).map((r) => r.id); - -describe('memory-matcher $or semantics', () => { - it('ANDs the keys of a single multi-key branch', () => { - expect(ids({ $or: [{ a: 'x', b: 'y' }] })).toEqual(['1']); - }); - - it('ANDs the keys of each branch independently', () => { - expect(ids({ $or: [{ a: 'x', b: 'y' }, { a: 'qq', b: 'zz' }] })).toEqual(['1', '4']); - }); - - it('ANDs operator-object keys within a branch', () => { - expect(ids({ $or: [{ a: { $eq: 'x' }, b: { $ne: 'zz' } }] })).toEqual(['1']); - }); +import { FILTER_LOGIC_CASES, FILTER_LOGIC_ROWS } from '@objectstack/spec/data'; - it('ANDs keys inside a $or nested in a $or branch', () => { - expect(ids({ $or: [{ $or: [{ a: 'x', b: 'zz' }] }] })).toEqual(['2']); - }); - - it('ANDs multiple operators on ONE field within a branch', () => { - expect(ids({ $or: [{ a: { $ne: 'qq', $eq: 'x' }, b: 'y' }] })).toEqual(['1']); - }); - - it('OR-s a $and branch against a sibling multi-key branch', () => { - expect(ids({ $or: [{ $and: [{ a: 'x' }, { b: 'y' }] }, { a: 'qq', b: 'zz' }] })).toEqual(['1', '4']); - }); - - it('ANDs a $and with a sibling key in the same branch, either order', () => { - expect(ids({ $or: [{ c: 'nope' }, { $and: [{ a: 'qq' }], b: 'y' }] })).toEqual(['3']); - expect(ids({ $or: [{ c: 'nope' }, { b: 'y', $and: [{ a: 'qq' }] }] })).toEqual(['3']); - }); - - it('keeps single-key $or branches as a plain OR', () => { - expect(ids({ $or: [{ a: 'x' }, { b: 'y' }] })).toEqual(['1', '2', '3']); - }); - - it('ANDs a $or with a sibling top-level field key', () => { - expect(ids({ $or: [{ a: 'x' }, { b: 'y' }], b: 'zz' })).toEqual(['2']); - }); +import { match } from './memory-matcher.js'; - it('does not widen an "own AND active, OR shared" read scope', () => { - const docs = [ - { id: 'own-active', owner: 'u1', status: 'active', shared_with: null }, - { id: 'own-archived', owner: 'u1', status: 'archived', shared_with: null }, - { id: 'other-active', owner: 'u2', status: 'active', shared_with: null }, - { id: 'shared', owner: 'u2', status: 'active', shared_with: 'u1' }, - ]; - const scope = { $or: [{ owner: 'u1', status: 'active' }, { shared_with: 'u1' }] }; - expect(docs.filter((d) => match(d, scope)).map((d) => d.id)).toEqual(['own-active', 'shared']); - }); +describe('memory-matcher — filter logic conformance', () => { + for (const c of FILTER_LOGIC_CASES) { + it(c.name, () => { + const got = FILTER_LOGIC_ROWS.filter((r) => match(r, c.filter)).map((r) => r.id); + expect(got, c.note).toEqual(c.expected); + }); + } }); diff --git a/packages/plugins/driver-sql/src/sql-driver-advanced.test.ts b/packages/plugins/driver-sql/src/sql-driver-advanced.test.ts index 45a67b4a57..1fa42c9379 100644 --- a/packages/plugins/driver-sql/src/sql-driver-advanced.test.ts +++ b/packages/plugins/driver-sql/src/sql-driver-advanced.test.ts @@ -323,7 +323,14 @@ describe('SqlDriver Advanced Operations (SQLite)', () => { }, }); - expect(results.length).toBeGreaterThan(0); + // Was `toBeGreaterThan(0)`, which any non-empty result satisfies. This + // particular shape was never miscompiled by #3774 — each `$or` branch + // holds a single `$and` key, so there were no sibling keys to wrongly OR + // — but the assertion was too weak to have noticed either way, which is + // the only reason it is worth tightening. Branch one is completed AND + // over 100 (Laptop 1200, Monitor 350); branch two is Alice AND pending + // (Keyboard). + expect(results.map((r: any) => r.id).sort()).toEqual(['1', '3', '4']); }); it('should handle contains filter', async () => { diff --git a/packages/plugins/driver-sql/src/sql-driver-or-filter.test.ts b/packages/plugins/driver-sql/src/sql-driver-or-filter.test.ts index 226bba6343..8897c259d1 100644 --- a/packages/plugins/driver-sql/src/sql-driver-or-filter.test.ts +++ b/packages/plugins/driver-sql/src/sql-driver-or-filter.test.ts @@ -1,32 +1,30 @@ // Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. /** - * `$or` filter semantics on a real SQL engine (in-memory better-sqlite3). + * Filter logical-combinator conformance for the SQL compiler, on a real engine + * (in-memory better-sqlite3). * - * The invariant this file defends is the Filter Protocol's (Mongo's) core rule: + * The shared cases come from `@objectstack/spec/data` so this backend, + * `driver-memory`, `formula`'s `matchesFilterCondition` and `read-scope-sql` + * are all held to one standard — see `filter-logic-conformance.ts` for why that + * standard exists (#3774). Adding a case there adds it to all four at once. * - * **Everything inside ONE filter object is AND-ed, at every nesting depth** — - * both its field keys and the operators of a single field. A `$or` array - * OR-s its BRANCHES together; it does not change how the contents *within* a - * branch combine. + * This file is the one that was actually failing: `applyFilterCondition` passed + * `logicalOp='or'` down into each `$or` branch's recursive call, so the branch's + * own contents were joined with `orWhere` too — `{$or:[{a,b}]}` compiled to + * `a = ? OR b = ?`, and `{$or:[{d:{$gte:X,$lt:Y}}]}` to `d >= ? OR d < ?`, which + * matches every row. Every such miscompile widens the result set. * - * `applyFilterCondition` used to pass `logicalOp='or'` down into each `$or` - * branch's recursive call, so the branch's own contents were joined with - * `orWhere` too: `{$or:[{a,b}]}` compiled to `a = ? OR b = ?`, and - * `{$or:[{d:{$gte:X,$lt:Y}}]}` to `d >= ? OR d < ?` (which matches every row). - * - * Every such miscompile *widens* the result set, never narrows it, which is - * why it matters beyond tidiness: these shapes are how scoping filters and - * scheduled-flow windows are written. The last two describe blocks pin the two - * that occur in real metadata — a parent-scope branch (`{parent_object, - * parent_id:{$in}}`) and the abutting `$gte`/`$lt` window the automation skill - * docs and the CLI flow linter both tell authors to write. + * The SQL-specific cases below the conformance sweep cover ground the shared + * table deliberately leaves out: a real DATE-typed column, and columns whose + * values are not the shared fixture's plain strings. */ import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import { FILTER_LOGIC_CASES, FILTER_LOGIC_ROWS } from '@objectstack/spec/data'; import { SqlDriver } from '../src/index.js'; -describe('SqlDriver $or filter semantics (SQLite)', () => { +describe('SqlDriver filter logic conformance (SQLite)', () => { let driver: SqlDriver; let knexInstance: any; @@ -43,176 +41,45 @@ describe('SqlDriver $or filter semantics (SQLite)', () => { t.string('a'); t.string('b'); t.string('c'); + t.string('owner'); + t.string('status'); + t.string('parent_object'); + t.string('parent_id'); }); - - // The 2x2 truth table over (a, b) — every combination appears exactly once, - // so a wrongly-OR-ed pair of predicates is always visible as extra rows. - await knexInstance('t').insert([ - { id: '1', a: 'x', b: 'y', c: 'z' }, - { id: '2', a: 'x', b: 'zz', c: 'z' }, - { id: '3', a: 'qq', b: 'y', c: 'z' }, - { id: '4', a: 'qq', b: 'zz', c: 'z' }, - ]); + await knexInstance('t').insert([...FILTER_LOGIC_ROWS]); }); afterEach(async () => { await knexInstance.destroy(); }); - const ids = async (where: any): Promise => - (await driver.find('t', { where })).map((r: any) => r.id).sort(); - - describe('keys within a $or branch are AND-ed', () => { - it('ANDs the keys of a single multi-key branch', async () => { - // Was: `a = 'x' OR b = 'y'` → ['1','2','3']. - expect(await ids({ $or: [{ a: 'x', b: 'y' }] })).toEqual(['1']); - }); - - it('ANDs the keys of each branch independently', async () => { - expect(await ids({ $or: [{ a: 'x', b: 'y' }, { a: 'qq', b: 'zz' }] })).toEqual(['1', '4']); - }); - - it('ANDs operator-object keys within a branch', async () => { - // Exercises the `{ field: { $op: v } }` path rather than the bare-value one. - expect(await ids({ $or: [{ a: { $eq: 'x' }, b: { $ne: 'zz' } }] })).toEqual(['1']); - }); - - it('ANDs keys inside a $or nested in a $or branch', async () => { - expect(await ids({ $or: [{ $or: [{ a: 'x', b: 'zz' }] }] })).toEqual(['2']); - }); - - it('ANDs multiple operators on ONE field within a branch', async () => { - // A single-key branch is still miscompilable: the operator map is looped - // with the same `logicalOp`, so `{a:{$ne,$ne}}` became `OR` of the two. - expect(await ids({ $or: [{ a: { $ne: 'qq', $eq: 'x' }, b: 'y' }] })).toEqual(['1']); - }); - - it('ANDs a $not with its sibling keys inside a branch', async () => { - // (a <> 'x' AND b = 'zz') → row 4 only. - expect(await ids({ $or: [{ c: 'nope' }, { $not: { a: 'x' }, b: 'zz' }] })).toEqual(['4']); - }); - }); - - describe('$and nested inside $or', () => { - it('OR-s a $and branch against a sibling multi-key branch', async () => { - expect( - await ids({ $or: [{ $and: [{ a: 'x' }, { b: 'y' }] }, { a: 'qq', b: 'zz' }] }), - ).toEqual(['1', '4']); - }); - - it('OR-s a multi-key branch against a sibling $and branch', async () => { - expect( - await ids({ $or: [{ a: 'x', b: 'y' }, { $and: [{ a: 'qq' }, { b: 'zz' }] }] }), - ).toEqual(['1', '4']); - }); - - it('ANDs a $and with a key that FOLLOWS it in the same branch', async () => { - // Key order must not matter: `$and` first, plain key second. - expect(await ids({ $or: [{ c: 'nope' }, { $and: [{ a: 'qq' }], b: 'y' }] })).toEqual(['3']); - }); - - it('ANDs a $and with a key that PRECEDES it in the same branch', async () => { - // The mirror of the above — this ordering was already correct; it stays a - // regression guard so a future refactor cannot flip only one of the two. - expect(await ids({ $or: [{ c: 'nope' }, { b: 'y', $and: [{ a: 'qq' }] }] })).toEqual(['3']); - }); - }); - - describe('unaffected shapes stay unaffected (controls)', () => { - it('keeps single-key $or branches as a plain OR', async () => { - expect(await ids({ $or: [{ a: 'x' }, { b: 'y' }] })).toEqual(['1', '2', '3']); - }); - - it('keeps top-level multi-key AND', async () => { - expect(await ids({ a: 'x', b: 'y' })).toEqual(['1']); - }); - - it('keeps a $or nested under a top-level $and', async () => { - expect(await ids({ $and: [{ $or: [{ a: 'x' }, { b: 'y' }] }, { b: 'y' }] })).toEqual(['1', '3']); - }); - - it('keeps a $or sibling of a top-level field key AND-ed', async () => { - expect(await ids({ $or: [{ a: 'x' }, { b: 'y' }], b: 'zz' })).toEqual(['2']); - }); - }); - - /** - * Read scopes are ordinary FilterConditions, and the shapes that express - * "rows I own, or rows shared with me" and "rows under a parent I can see" - * both put a multi-key object inside a `$or` branch. A widening miscompile - * there returns rows the scope was written to exclude, so these shapes get - * their own explicit guards rather than relying on the abstract cases above. - */ - describe('read-scope shapes', () => { - beforeEach(async () => { - await knexInstance.schema.createTable('doc', (t: any) => { - t.string('id').primary(); - t.string('owner'); - t.string('status'); - t.string('shared_with'); + describe('shared conformance cases', () => { + for (const c of FILTER_LOGIC_CASES) { + it(c.name, async () => { + const rows = await driver.find('t', { where: c.filter }); + const got = rows + .map((r: any) => String(r.id)) + .sort((x: string, y: string) => x.localeCompare(y)); + expect(got, c.note).toEqual(c.expected); }); - await knexInstance('doc').insert([ - { id: 'own-active', owner: 'u1', status: 'active', shared_with: null }, - { id: 'own-archived', owner: 'u1', status: 'archived', shared_with: null }, - { id: 'other-active', owner: 'u2', status: 'active', shared_with: null }, - { id: 'shared', owner: 'u2', status: 'active', shared_with: 'u1' }, - ]); - }); - - it('does not widen an "own AND active, OR shared" scope', async () => { - const scope = { $or: [{ owner: 'u1', status: 'active' }, { shared_with: 'u1' }] }; - const rows = await driver.find('doc', { where: scope }); - // Was: `owner='u1' OR status='active' OR shared_with='u1'` — which also - // returned `own-archived` (excluded by the scope's AND) and - // `other-active` (another user's row). - expect(rows.map((r: any) => r.id).sort()).toEqual(['own-active', 'shared']); - }); - - it('does not widen a scope whose branch uses $in + status', async () => { - const scope = { $or: [{ owner: { $in: ['u1'] }, status: 'active' }, { shared_with: 'u1' }] }; - const rows = await driver.find('doc', { where: scope }); - expect(rows.map((r: any) => r.id).sort()).toEqual(['own-active', 'shared']); - }); - - it('keeps a multi-parent-type scope pinned to its own parent ids', async () => { - // "Rows under parent c1 of type case, or under parent t1 of type todo" — - // one branch per parent type, each pairing a type with its own id list. - // The pairing is the whole point of the branch and must survive compile. - await knexInstance.schema.createTable('att', (t: any) => { - t.string('id').primary(); - t.string('parent_object'); - t.string('parent_id'); - }); - await knexInstance('att').insert([ - { id: 'ok-case', parent_object: 'case', parent_id: 'c1' }, - { id: 'ok-todo', parent_object: 'todo', parent_id: 't1' }, - { id: 'other-case', parent_object: 'case', parent_id: 'c2' }, - { id: 'cross-type', parent_object: 'todo', parent_id: 'c1' }, - ]); - - const scope = { - $or: [ - { parent_object: 'case', parent_id: { $in: ['c1'] } }, - { parent_object: 'todo', parent_id: { $in: ['t1'] } }, - ], - }; - const rows = await driver.find('att', { where: scope }); - expect(rows.map((r: any) => r.id).sort()).toEqual(['ok-case', 'ok-todo']); - }); + } }); /** * The abutting-window pattern the automation skill docs recommend and the CLI * flow linter blesses (`lint-flow-patterns`): each tier is one field carrying - * two operators. "Windows tile the timeline so each record matches exactly - * one tier" only holds if those operators AND — under the old compile every - * tier degenerated to `d >= lo OR d < hi`, i.e. matched every row. + * two operators. "Windows tile the timeline so each record matches exactly one + * tier" only holds if those operators AND — under the old compile every tier + * degenerated to `d >= lo OR d < hi`, i.e. matched every row. + * + * The shared table pins this shape on plain strings; this pins it on a real + * date column, where value coercion also runs. */ describe('multi-operator date windows inside $or', () => { beforeEach(async () => { await knexInstance.schema.createTable('task', (t: any) => { t.string('id').primary(); - t.string('end_date'); + t.date('end_date'); }); await knexInstance('task').insert([ { id: 'd07', end_date: '2026-08-07' }, diff --git a/packages/services/service-analytics/package.json b/packages/services/service-analytics/package.json index d309746333..62fbb1f20e 100644 --- a/packages/services/service-analytics/package.json +++ b/packages/services/service-analytics/package.json @@ -23,6 +23,8 @@ }, "devDependencies": { "@types/node": "^26.1.1", + "@types/sql.js": "^1.4.11", + "sql.js": "^1.14.1", "typescript": "^6.0.3", "vitest": "^4.1.10" }, diff --git a/packages/services/service-analytics/src/__tests__/read-scope-sql-conformance.test.ts b/packages/services/service-analytics/src/__tests__/read-scope-sql-conformance.test.ts new file mode 100644 index 0000000000..958771a6ff --- /dev/null +++ b/packages/services/service-analytics/src/__tests__/read-scope-sql-conformance.test.ts @@ -0,0 +1,117 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * Filter logical-combinator conformance for the read-scope SQL lowering, + * executed against a real SQLite engine (`sql.js`, pure WASM). + * + * The cases come from `@objectstack/spec/data` so this backend, `driver-sql`, + * `driver-memory` and `formula`'s `matchesFilterCondition` are all held to one + * standard — see `filter-logic-conformance.ts` for why that standard exists + * (#3774). + * + * ## Why execute, when `read-scope-sql.test.ts` already asserts the SQL string + * + * The two answer different questions, and the difference is what each one's + * correctness *depends on*: + * + * - A string assertion checks "does the compiler emit the text I wrote down?" + * Its ceiling is the author's own reading of SQL. Write the expected string + * with a missing pair of parentheses and the test locks the bug in, green. + * - This file checks "does the compiler mean the same thing as the other three + * backends?" Its ceiling is the database. The expected values are row ids + * shared with `driver-sql`, `driver-memory` and `matchesFilterCondition`, so + * a divergence shows up as rows, not as a diff against a hand-written string. + * + * That matters most where this compiler is subtle: `compileNode` joins a node's + * own clauses with `' AND '` and returns them **unparenthesized**, so a + * multi-key node nested inside a `$or` emits `a = ? AND b = ? OR c = ?`. It is + * correct — SQL binds `AND` tighter than `OR` — but correct by relying on + * precedence rather than by construction. (Both suites do catch a deleted + * paren; the string one only because its expectations happen to be written + * correctly today.) + * + * The other half of the value is cheap coverage: a case added to the shared + * table lands on all four backends at once, with no SQL to hand-write here. + * + * ## Why `sql.js` and not `better-sqlite3` + * + * An earlier revision imported `better-sqlite3` here and killed the vitest + * worker outright on CI — a process-level abort with no JS error to catch, so + * the 17 cases silently did not run. The same symptom reproduces locally under + * Node 20 (CI's version); `better-sqlite3@13` declares `engines: >=22`, and a + * native binding is only loadable by the exact Node ABI it was built for. + * + * `driver-sql` can afford the native dependency because it *falls back* to WASM + * SQLite when the binding fails to load (see the step-down warning in + * `sql-driver.ts`). A test has no such fallback, so it uses the pure-WASM engine + * directly — no ABI to match, no build step, identical behaviour on every Node + * version. `sql.js` is the same engine that fallback lands on. + */ + +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import { FILTER_LOGIC_CASES, FILTER_LOGIC_ROWS } from '@objectstack/spec/data'; + +import { compileScopedFilterToSql } from '../read-scope-sql.js'; + +const ALIAS = 't'; + +/** Point sql.js at the `.wasm` shipped inside its own package (Node-safe). */ +async function locateWasm(): Promise<((file: string) => string) | undefined> { + try { + const { createRequire } = await import('node:module'); + const require = createRequire(import.meta.url); + const pkgJsonPath = require.resolve('sql.js/package.json'); + const { dirname, join } = await import('node:path'); + const dir = dirname(pkgJsonPath); + return (file: string) => join(dir, 'dist', file); + } catch { + return undefined; + } +} + +describe('compileScopedFilterToSql — filter logic conformance', () => { + let db: any; + + beforeAll(async () => { + const mod: any = await import('sql.js'); + const initSqlJs = mod.default ?? mod; + const locateFile = await locateWasm(); + const SQL = await initSqlJs(locateFile ? { locateFile } : undefined); + + db = new SQL.Database(); + db.run(` + CREATE TABLE "t" ( + "id" TEXT PRIMARY KEY, + "a" TEXT, "b" TEXT, "c" TEXT, + "owner" TEXT, "status" TEXT, + "parent_object" TEXT, "parent_id" TEXT + ); + `); + const insert = db.prepare( + `INSERT INTO "t" ("id","a","b","c","owner","status","parent_object","parent_id") + VALUES (?,?,?,?,?,?,?,?)`, + ); + for (const r of FILTER_LOGIC_ROWS) { + insert.run([r.id, r.a, r.b, r.c, r.owner, r.status, r.parent_object, r.parent_id]); + } + insert.free(); + }); + + afterAll(() => { + db?.close(); + }); + + for (const c of FILTER_LOGIC_CASES) { + it(c.name, () => { + const { sql, params } = compileScopedFilterToSql(c.filter, ALIAS); + // The compiler returns a boolean expression, exactly as the analytics + // query builder splices it — including the unparenthesized top level. + const stmt = db.prepare(`SELECT "id" FROM "t" AS "${ALIAS}" WHERE ${sql} ORDER BY "id"`); + stmt.bind(params as any[]); + const got: string[] = []; + while (stmt.step()) got.push(String(stmt.get()[0])); + stmt.free(); + expect(got, c.note).toEqual(c.expected); + }); + } +}); diff --git a/packages/spec/api-surface.json b/packages/spec/api-surface.json index c4dc3ad89c..3e608427d1 100644 --- a/packages/spec/api-surface.json +++ b/packages/spec/api-surface.json @@ -330,6 +330,8 @@ "ExternalTableSchema (const)", "FIELD_GROUP_SYSTEM_FIELDS (const)", "FILE_REFERENCE_TYPES (const)", + "FILTER_LOGIC_CASES (const)", + "FILTER_LOGIC_ROWS (const)", "FILTER_OPERATORS (const)", "FeedFilterMode (type)", "FeedItemType (type)", @@ -353,6 +355,8 @@ "Filter (type)", "FilterCondition (type)", "FilterConditionSchema (const)", + "FilterLogicCase (interface)", + "FilterLogicRow (interface)", "FilterOperatorKey (type)", "FormatValidation (type)", "FormatValidationSchema (const)", diff --git a/packages/spec/src/data/filter-logic-conformance.ts b/packages/spec/src/data/filter-logic-conformance.ts new file mode 100644 index 0000000000..8c11c447d0 --- /dev/null +++ b/packages/spec/src/data/filter-logic-conformance.ts @@ -0,0 +1,198 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * Canonical conformance cases for the Filter Protocol's **logical combinators** + * — the single source of truth every filter backend is checked against. + * + * ## Why this exists + * + * `FilterCondition` is evaluated by four independent implementations, and they + * had drifted: + * + * | Backend | Where | + * |---|---| + * | SQL compiler | `driver-sql` `applyFilterCondition` | + * | In-memory matcher | `driver-memory` `memory-matcher` | + * | Record-at-a-time evaluator | `formula` `matchesFilterCondition` (RLS write-side `check`) | + * | Read-scope SQL lowering | `service-analytics` `read-scope-sql` | + * + * In #3774 the SQL compiler OR-ed the contents *within* a `$or` branch instead + * of AND-ing them, so every `$or` filter matched more rows than it should — + * including read-visibility filters. The other three were correct, but nothing + * held them to a shared standard, so the divergence was invisible until someone + * ran a real query. These cases are that standard: each backend has a thin test + * that feeds {@link FILTER_LOGIC_ROWS} through its own evaluator and asserts the + * ids in {@link FilterLogicCase.expected}. + * + * Third-party driver authors can use the same table to check a new backend. + * + * ## Deliberate scope + * + * **Logical combinator semantics only** — `$and` / `$or` / `$not`, and the rule + * they hang off: + * + * > Everything inside ONE filter object is AND-ed, at every nesting depth — + * > both its field keys and the operators of a single field. A `$or` array + * > OR-s its BRANCHES; it does not change how the contents within a branch + * > combine. + * + * The predicates are deliberately boring: string equality, `$in`, `$ne`, `$gte` + * / `$lt` on lexicographic strings. Nothing here exercises null handling, dates, + * numeric coercion, `LIKE` escaping, or case sensitivity — those legitimately + * differ between a SQL engine and a JS matcher, and folding them in would make + * the table unpassable rather than more useful. Keep it that way: a case belongs + * here only if **every** backend must agree on it. + */ + +import type { FilterCondition } from './filter.zod'; + +/** A row in the conformance fixture. All columns are plain strings. */ +export interface FilterLogicRow { + id: string; + /** 2x2 truth table over (a, b) — see {@link FILTER_LOGIC_ROWS}. */ + a: string; + b: string; + /** Constant across every row; a predicate on it never changes a result. */ + c: string; + /** Record-scope columns, for the shapes read scopes are actually written in. */ + owner: string; + status: string; + parent_object: string; + parent_id: string; +} + +/** + * The fixture. Rows 1-4 are the 2x2 truth table over `(a, b)` — every + * combination appears exactly once, so a wrongly-OR-ed pair of predicates always + * shows up as extra ids rather than by luck of the data. Rows 5-8 carry the + * record-scope columns used by the read-scope cases. + */ +export const FILTER_LOGIC_ROWS: readonly FilterLogicRow[] = [ + { id: '1', a: 'x', b: 'y', c: 'z', owner: 'u1', status: 'active', parent_object: 'case', parent_id: 'c1' }, + { id: '2', a: 'x', b: 'zz', c: 'z', owner: 'u1', status: 'archived', parent_object: 'case', parent_id: 'c2' }, + { id: '3', a: 'qq', b: 'y', c: 'z', owner: 'u2', status: 'active', parent_object: 'todo', parent_id: 't1' }, + { id: '4', a: 'qq', b: 'zz', c: 'z', owner: 'u2', status: 'archived', parent_object: 'todo', parent_id: 'c1' }, +] as const; + +/** One conformance case: a filter and the ids it must match, in id order. */ +export interface FilterLogicCase { + /** Stable identifier, usable as a test name. */ + name: string; + filter: FilterCondition; + /** Ids of matching rows, ascending. */ + expected: string[]; + /** Why the case is here — surfaced in failure output. */ + note?: string; +} + +/** + * The cases. Ordered from the core rule outward: keys within a branch, then + * operators within a key, then combinator nesting, then the shapes that occur + * in real read scopes. + */ +export const FILTER_LOGIC_CASES: readonly FilterLogicCase[] = [ + // ── Keys within one object AND, at every depth ──────────────────────────── + { + name: 'multi-key $or branch ANDs its own keys', + filter: { $or: [{ a: 'x', b: 'y' }] }, + expected: ['1'], + note: '#3774: compiled to `a = x OR b = y`, matching 1,2,3.', + }, + { + name: 'each $or branch ANDs independently', + filter: { $or: [{ a: 'x', b: 'y' }, { a: 'qq', b: 'zz' }] }, + expected: ['1', '4'], + }, + { + name: 'top-level multi-key ANDs', + filter: { a: 'x', b: 'y' }, + expected: ['1'], + }, + { + name: 'a $or AND-s with a sibling top-level key', + filter: { $or: [{ a: 'x' }, { b: 'y' }], b: 'zz' }, + expected: ['2'], + }, + + // ── Operators within one field AND ──────────────────────────────────────── + { + name: 'multiple operators on one field AND within a branch', + filter: { $or: [{ a: { $ne: 'qq', $eq: 'x' }, b: 'y' }] }, + expected: ['1'], + note: '#3774: a single-key branch is miscompilable too — the operator map is looped with the same flag.', + }, + { + name: 'an abutting $gte/$lt window ANDs its bounds', + filter: { $or: [{ b: { $gte: 'y', $lt: 'z' } }] }, + expected: ['1', '3'], + note: 'The multi-tier scheduled-flow window pattern. OR-ing the bounds matches every row.', + }, + + // ── Combinator nesting ──────────────────────────────────────────────────── + { + name: '$and branch OR-s against a sibling multi-key branch', + filter: { $or: [{ $and: [{ a: 'x' }, { b: 'y' }] }, { a: 'qq', b: 'zz' }] }, + expected: ['1', '4'], + }, + { + name: 'multi-key branch OR-s against a sibling $and branch', + filter: { $or: [{ a: 'x', b: 'y' }, { $and: [{ a: 'qq' }, { b: 'zz' }] }] }, + expected: ['1', '4'], + }, + { + name: '$and ANDs with a key that FOLLOWS it in the same branch', + filter: { $or: [{ c: 'nope' }, { $and: [{ a: 'qq' }], b: 'y' }] }, + expected: ['3'], + note: 'Key order must not matter; this ordering and the next must agree.', + }, + { + name: '$and ANDs with a key that PRECEDES it in the same branch', + filter: { $or: [{ c: 'nope' }, { b: 'y', $and: [{ a: 'qq' }] }] }, + expected: ['3'], + }, + { + name: 'keys AND inside a $or nested in a $or branch', + filter: { $or: [{ $or: [{ a: 'x', b: 'zz' }] }] }, + expected: ['2'], + }, + { + name: 'a $or nested under a top-level $and', + filter: { $and: [{ $or: [{ a: 'x' }, { b: 'y' }] }, { b: 'y' }] }, + expected: ['1', '3'], + }, + { + name: '$not ANDs with its sibling keys inside a branch', + filter: { $or: [{ c: 'nope' }, { $not: { a: 'x' }, b: 'zz' }] }, + expected: ['4'], + }, + { + name: 'single-key $or branches stay a plain OR', + filter: { $or: [{ a: 'x' }, { b: 'y' }] }, + expected: ['1', '2', '3'], + note: 'The control: the shape that was always correct must stay correct.', + }, + + // ── Shapes read scopes are actually written in ──────────────────────────── + { + name: 'read scope: own AND active, OR another owner\'s row', + filter: { $or: [{ owner: 'u1', status: 'active' }, { owner: 'u2', status: 'active' }] }, + expected: ['1', '3'], + note: 'Widening here returns rows the scope excludes — an unauthorized read, not a wrong count.', + }, + { + name: 'read scope: owner $in AND status', + filter: { $or: [{ owner: { $in: ['u1'] }, status: 'active' }, { owner: 'u2', status: 'archived' }] }, + expected: ['1', '4'], + }, + { + name: 'read scope: parent type paired with its own id list', + filter: { + $or: [ + { parent_object: 'case', parent_id: { $in: ['c1'] } }, + { parent_object: 'todo', parent_id: { $in: ['t1'] } }, + ], + }, + expected: ['1', '3'], + note: 'Row 4 is parent_object=todo with parent_id=c1 — it matches neither pairing, and is the row a widened compile leaks.', + }, +] as const; diff --git a/packages/spec/src/data/index.ts b/packages/spec/src/data/index.ts index a3e49cdebd..44103c05dc 100644 --- a/packages/spec/src/data/index.ts +++ b/packages/spec/src/data/index.ts @@ -2,6 +2,10 @@ export * from './query.zod'; export * from './filter.zod'; +// Canonical conformance cases for the filter logical combinators — the shared +// standard the four independent FilterCondition backends are each checked +// against, so they cannot drift apart again (#3774). +export * from './filter-logic-conformance'; export * from './date-macros.zod'; // Session-scoped filter placeholders ({current_user_id} / {current_org_id}) — // the sibling vocabulary to date macros. Presentation scope only; RLS is the diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index cdfc1d0467..aa09de532f 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -1833,6 +1833,12 @@ importers: '@types/node': specifier: ^26.1.1 version: 26.1.1 + '@types/sql.js': + specifier: ^1.4.11 + version: 1.4.11 + sql.js: + specifier: ^1.14.1 + version: 1.14.1 typescript: specifier: ^6.0.3 version: 6.0.3