diff --git a/.changeset/analytics-notcontains-inert-predicate.md b/.changeset/analytics-notcontains-inert-predicate.md new file mode 100644 index 0000000000..d8a18939f8 --- /dev/null +++ b/.changeset/analytics-notcontains-inert-predicate.md @@ -0,0 +1,74 @@ +--- +"@objectstack/driver-memory": minor +--- + +fix(driver-memory): the analytics (cube) face compiles `$notContains` to a predicate that actually excludes rows, instead of a bare mingo `{$not: 'x'}` that constrains nothing (#5374) + +**This is an observable behaviour change on a shipped surface: widgets whose +`where` carries `$notContains`, `$contains`, or an empty `$in` will show +different — correct — numbers.** Every one of them moves in the same direction, +from a wider row set to the rows actually asked for, because each of these +defects made a predicate mean less than it says. + +## What was happening + +`MemoryAnalyticsService` mapped each cube operator to the NAME of a mingo +operator, and the call site filled that name in as +`matchStage[field] = {[name]: comparand}`. That shape can express "compare this +field to this value" and nothing else, so the two operators that need to WRAP +their comparand were pushed through it anyway: + +| `where` | compiled `$match` | analytics | `find()` | +|---|---|---|---| +| `{name: {$notContains: 'et'}}` | `{name: {$not: 'et'}}` | **3** | 2 | +| `{name: {$notContains: 'a'}}` | `{name: {$not: 'a'}}` | **3** | 0 | +| `{name: {$contains: 'a.p'}}` | `{name: {$regex: 'a.p'}}` | **1** | 0 | +| `{name: {$contains: 'ALPHA'}}` | `{name: {$regex: 'ALPHA'}}` | **0** | 1 | +| `{code: {$in: []}}` | *(no predicate emitted)* | **3** | 0 | + +- **`notContains` → `'$not'`.** mingo's `$not` takes a regex or an operator + expression; handed a bare scalar it constrains nothing. The predicate was + emitted, appeared in the pipeline, and passed the whole table. A predicate + that is emitted and inert is indistinguishable from a working one at the + author's end — the same amplifying direction as #3948, reached a third way. +- **`contains` → `'$regex'`** was the right operator with the comparand handed + in raw, so it was neither escaped (a `.` matched any character) nor + case-folded, while the live query path escapes and matches `/…/i`. One + `where`, two meanings, depending on which face read it (#5240). +- **an empty `$in`** hit the call site's `values.length > 0` guard and emitted + no predicate at all, so the query widened to the whole table where `find()` + returned nothing. +- **an operand that is not a comparand** — a `$contains` pattern, a `$exists` + flag — went through the field's storage-form conversion anyway, so on a + declared `datetime` column the PATTERN itself was rewritten into canonical + form and then matched rows `find()` does not match (#4047). + +## What changed + +The operator table now holds a **predicate builder** per operator rather than an +operator name, so `notContains` can say `{$not: {$regex: …}}` and the class of +"this operator needs a structure and the table can only hold a name" is gone +rather than this one instance of it. `$in` / `$nin` / `$lte` / `$exists`, which +the call site had grown an `if` chain for, are ordinary rows in that table now. + +The substring rule itself is **borrowed from the driver** (new narrow +`InMemoryDriver.filterSubstringPattern`, alongside `filterComparandStorageForm`) +instead of re-derived, so `contains` on the analytics face escapes and case-folds +exactly as `find()` does and the two cannot drift apart again. + +The `opMap[operator] || '$eq'` fallback — under which a misspelled or unmapped +operator silently became an EQUALITY comparison — is gone. It was already +unreachable after #5345 gated the vocabulary upstream, but only until someone +widened that vocabulary, which #5345 deliberately made a one-line edit. The +predicate table is keyed by the operator union derived from that same table, so +the widening edit now **fails to compile** until the predicate exists. + +Two dead entries were deleted with it: `'notSet': '$exists'` (unreachable, and +inverted if it ever had been reached) and `'inDateRange': '$gte'` (unreachable, +and a one-ended `>=` answer to a two-ended range — its own comment conceded +"Will need special handling" and nothing implemented it). + +## Not changed + +The `generateSql()` exit is untouched. Its operator-layer defects are #5433, +filed and deliberately not bundled. diff --git a/packages/plugins/driver-memory/src/memory-analytics.ts b/packages/plugins/driver-memory/src/memory-analytics.ts index c56f5f184e..2281d2cdaa 100644 --- a/packages/plugins/driver-memory/src/memory-analytics.ts +++ b/packages/plugins/driver-memory/src/memory-analytics.ts @@ -26,15 +26,22 @@ import { * here the only way to widen what this face accepts, and makes forgetting to * add one a loud refusal rather than a wrong number. * - * A row here means the face ATTEMPTS the operator, not that the predicate it - * builds is correct — `$notContains` lowers to a bare mingo `{$not: 'x'}` that - * constrains nothing (#5374). That one is out of #5345's scope (which ruled on - * operators with NO mapping) and is filed rather than fixed here; do not read - * this list as eleven operators known to work. The comparand half of that - * caveat is closed: #5373 removed the `string[]` round-trip that lost booleans - * and `null` (see {@link NormalizedCubeFilter}). + * A row here used to mean only that the face ATTEMPTS the operator, not that the + * predicate it builds is correct. Both halves of that caveat are now closed: + * #5373 removed the `string[]` comparand round-trip that lost booleans and + * `null` (see {@link NormalizedCubeFilter}), and #5374 replaced the + * operator-name→operator-name mapping — under which `notContains` compiled to a + * bare mingo `{$not: 'x'}` that constrains nothing — with + * {@link CUBE_OPERATOR_TO_MONGO_PREDICATE}, which builds the whole predicate. + * + * The literal `as const` is load-bearing, not style: it makes + * {@link CubeOperator} the exact union of this table's values, and that union is + * the key type of the predicate table. Adding a row here without teaching the + * compiler how to build its predicate is therefore a TYPE ERROR rather than a + * wrong number — which is the whole point of #5345 keeping the gate's vocabulary + * and the compiler's table as one statement. */ -const MONGO_TO_CUBE_OPERATOR: Readonly> = Object.freeze({ +const MONGO_TO_CUBE_OPERATOR = Object.freeze({ $eq: 'equals', $ne: 'notEquals', $gt: 'gt', @@ -46,7 +53,14 @@ const MONGO_TO_CUBE_OPERATOR: Readonly> = Object.freeze({ $contains: 'contains', $notContains: 'notContains', $exists: 'set', -}); +} as const); + +/** + * [#5374] The cube-style operator names this face lowers into — exactly the + * values of {@link MONGO_TO_CUBE_OPERATOR}, derived rather than restated so the + * two cannot drift. + */ +type CubeOperator = (typeof MONGO_TO_CUBE_OPERATOR)[keyof typeof MONGO_TO_CUBE_OPERATOR]; /** * [#5345] What the analytics (cube) face compiles, for the shared filter walk. @@ -99,7 +113,7 @@ export const ANALYTICS_FILTER_CAPABILITIES: FilterFaceCapabilities = Object.free */ interface NormalizedCubeFilter { member: string; - operator: string; + operator: CubeOperator; /** * The comparands, as authored. Temporal values are put into the field's * storage form at the exits ({@link MemoryAnalyticsService.comparandsFor}), @@ -108,6 +122,117 @@ interface NormalizedCubeFilter { values: unknown[]; } +/** + * [#5374] What one lowered entry gives its predicate builder. + * + * Two comparand lists, not one, because the driver's own translation makes the + * same split and for the same reason (#4047, `normalizeFieldOperators`): a + * VALUE COMPARISON must be put into the field's storage form or mingo's + * cross-type comparison drops every row, while an operand that is not a + * comparand — a `$exists` flag, a `$regex` pattern — must NOT be, because + * "storage form" is meaningless for it and applying it corrupts the operand. + * + * That was not hypothetical here. This face ran every operand through the + * comparand conversion, so on a declared `datetime` column + * `{made_at: {$contains: '2026-01-01T00:00:00Z'}}` had its PATTERN rewritten to + * canonical `'2026-01-01T00:00:00.000Z'` and then matched the row, where + * `find()` — which never rewrites a pattern — matched nothing. + */ +interface MongoPredicateInput { + /** Comparands in the field's storage form (#4047). For value comparisons. */ + readonly comparands: readonly unknown[]; + /** The operands as authored. For operands that are not comparands. */ + readonly raw: readonly unknown[]; + /** + * A comparand as a case-insensitive literal-substring pattern, built by the + * DRIVER's own rule (`filterSubstringPattern`) rather than re-derived here. + */ + readonly substring: (value: unknown) => RegExp; +} + +type MongoPredicateBuilder = (input: MongoPredicateInput) => Record; + +/** + * [#5374] How each cube operator becomes a mingo field predicate — the whole + * `{$op: …}` object, not the name of an operator. + * + * # Why the shape changed + * + * This was `convertOperatorToMongo(operator): string`, a name→name map, and the + * call site filled the name in as `matchStage[field] = {[name]: comparand}`. + * That shape can express "compare this field to this value" and NOTHING else, + * so the two entries that need to WRAP their comparand were forced through it + * anyway: + * + * - `notContains` → `'$not'` became `{name: {$not: 'et'}}`. mingo's `$not` + * takes a regex or an operator expression; given a bare scalar it + * constrains nothing, so the predicate was emitted, looked present in the + * pipeline, and passed the whole table (#5374: 3 rows where `find()` + * returns 2). A predicate that is emitted and inert is indistinguishable + * from a correct one at the author's end, and widens in the #3948 + * direction. + * - `contains` → `'$regex'` became `{name: {$regex: 'a.p'}}` — the right + * operator, but the comparand went in raw, so it was neither escaped nor + * case-folded and meant something other than what `find()` means by it. + * + * A builder can say `{$not: {$regex: …}}`, so the class of "this operator needs + * a structure and the table can only hold a name" is gone rather than this one + * instance of it. `$in`/`$nin`/`$lte`/`$exists`, which the call site had grown + * an `if` chain for, are ordinary rows here for the same reason. + * + * # Why it is a `Record` + * + * Because the missing-entry case had a `|| '$eq'` fallback, and a misspelled or + * unmapped operator silently became an EQUALITY comparison — the exact + * silent-wrong-answer shape #5345, #5373 and this issue have each been closing. + * After #5345 that fallback was unreachable (`mongoOperatorToCubeOperator` + * refuses anything not in {@link MONGO_TO_CUBE_OPERATOR}, and both exits consume + * only `normalizeFilters` output), but only until someone widened the vocabulary + * — which #5345 deliberately made a ONE-LINE edit to that table. Keying this + * table by {@link CubeOperator} makes that edit fail to compile until the + * predicate exists, so the fallback is not merely unreachable, it is + * unnecessary: the totality is proven, not defended. + * + * Two entries were deleted rather than kept. `'notSet': '$exists'` and + * `'inDateRange': '$gte'` were both unreachable (nothing lowers to either name) + * and both wrong if they ever had been: the first inverts — the call site would + * have compiled `notSet` to `{$exists: true}` — and the second answers a + * two-ended range with a one-ended `>=`, which its own comment conceded ("Will + * need special handling") and which nothing implemented. Dead code that is + * ALSO wrong is a trap primed for whoever widens the vocabulary next; the type + * error they now get instead says so at the only moment it helps. + */ +const CUBE_OPERATOR_TO_MONGO_PREDICATE: Readonly> = Object.freeze({ + equals: ({ comparands }) => ({ $eq: comparands[0] }), + notEquals: ({ comparands }) => ({ $ne: comparands[0] }), + gt: ({ comparands }) => ({ $gt: comparands[0] }), + gte: ({ comparands }) => ({ $gte: comparands[0] }), + lt: ({ comparands }) => ({ $lt: comparands[0] }), + // A bare-day `lte` bound means "through that whole day" (#4042; the SQL twin + // is #3777): compile half-open so timestamp values on the final day stay in. + // Order-equivalent to `$lte` for plain `YYYY-MM-DD` values. + lte: ({ comparands }) => { + const nextDay = nextUtcCalendarDay(comparands[0]); + return nextDay != null ? { $lt: nextDay } : { $lte: comparands[0] }; + }, + // The list operators take the WHOLE list. An empty one is a real predicate — + // `$in: []` selects nothing, `$nin: []` selects everything — and saying so + // here is what retires the call site's `values.length > 0` guard, under which + // `{code: {$in: []}}` emitted no predicate at all and answered with the whole + // table while `find()` answered with none of it. + in: ({ comparands }) => ({ $in: [...comparands] }), + notIn: ({ comparands }) => ({ $nin: [...comparands] }), + // A pattern, not a comparand: `raw`, and the driver's own substring rule. + contains: ({ raw, substring }) => ({ $regex: substring(raw[0]) }), + // The fix this issue is about. `{$not: }` constrains nothing; the + // negation has to wrap a pattern, which is exactly what the live query path + // builds for `$notContains` (`memory-driver.ts` `normalizeFieldOperators`). + notContains: ({ raw, substring }) => ({ $not: { $regex: substring(raw[0]) } }), + // A presence flag, not a comparand. The `raw.length === 0` arm keeps the old + // call site's reading of a valueless `set` ("does it exist" → true). + set: ({ raw }) => ({ $exists: raw.length > 0 ? Boolean(raw[0]) : true }), +}); + /** * Configuration for MemoryAnalyticsService */ @@ -180,34 +305,22 @@ export class MemoryAnalyticsService implements IAnalyticsService { if (normalizedFilters.length > 0) { const matchStage: Record = {}; for (const filter of normalizedFilters) { - const mongoOp = this.convertOperatorToMongo(filter.operator); const fieldPath = this.resolveFieldPath(cube, filter.member); - - if (filter.values && filter.values.length > 0) { - // [#5373] The comparands as authored, in the storage form of the field - // they are compared against. There is no type recovery step any more, - // because there is no longer a stringification to recover FROM: a - // boolean reaches mingo as a boolean and `null` as `null`, so a - // predicate over `is_active` or `closed_at` selects the same rows - // `find()` selects instead of none / all of them. - const coerced = this.comparandsFor(cube, filter.member, filter.values); - if (mongoOp === '$in') { - matchStage[fieldPath] = { $in: coerced }; - } else if (mongoOp === '$nin') { - matchStage[fieldPath] = { $nin: coerced }; - } else if (mongoOp === '$lte') { - // A bare-day `lte` bound means "through that whole day" (#4042; - // the SQL twin is #3777): compile half-open so timestamp values on - // the final day stay in. Order-equivalent to `$lte` for plain - // `YYYY-MM-DD` values. - const nextDay = nextUtcCalendarDay(coerced[0]); - matchStage[fieldPath] = nextDay != null ? { $lt: nextDay } : { $lte: coerced[0] }; - } else { - matchStage[fieldPath] = { [mongoOp]: coerced[0] }; - } - } else if (mongoOp === '$exists') { - matchStage[fieldPath] = { $exists: filter.operator === 'set' }; - } + // [#5374] The operator decides the WHOLE predicate, not just its name — + // so `notContains` can say `{$not: {$regex: …}}` instead of being forced + // into `{$not: }`, which mingo reads as no constraint at all. + // + // [#5373] `comparands` are the values as authored, in the storage form + // of the field they are compared against. There is no type recovery step + // any more, because there is no longer a stringification to recover + // FROM: a boolean reaches mingo as a boolean and `null` as `null`, so a + // predicate over `is_active` or `closed_at` selects the same rows + // `find()` selects instead of none / all of them. + matchStage[fieldPath] = this.mongoPredicateBuilder(filter.operator)({ + comparands: this.comparandsFor(cube, filter.member, filter.values), + raw: filter.values, + substring: (value) => this.driver.filterSubstringPattern(value), + }); } if (Object.keys(matchStage).length > 0) { pipeline.push({ $match: matchStage }); @@ -624,8 +737,8 @@ export class MemoryAnalyticsService implements IAnalyticsService { * function with a synthesised `{'a.b': spec}` node the gate never saw, and * that is a real path to an unmapped operator. It used to `continue`. */ - private mongoOperatorToCubeOperator(op: string, field: string, path: string): string { - const cubeOp = MONGO_TO_CUBE_OPERATOR[op]; + private mongoOperatorToCubeOperator(op: string, field: string, path: string): CubeOperator { + const cubeOp = (MONGO_TO_CUBE_OPERATOR as Record)[op]; if (!cubeOp) throw uncompilableFieldOperatorError(op, field, path, ANALYTICS_FILTER_CAPABILITIES); return cubeOp; } @@ -776,23 +889,27 @@ export class MemoryAnalyticsService implements IAnalyticsService { } } - private convertOperatorToMongo(operator: string): string { - const opMap: Record = { - 'equals': '$eq', - 'notEquals': '$ne', - 'contains': '$regex', - 'notContains': '$not', - 'gt': '$gt', - 'gte': '$gte', - 'lt': '$lt', - 'lte': '$lte', - 'in': '$in', - 'notIn': '$nin', - 'set': '$exists', - 'notSet': '$exists', - 'inDateRange': '$gte', // Will need special handling - }; - return opMap[operator] || '$eq'; + /** + * [#5374] The mingo predicate builder for one lowered operator. + * + * Total by construction: {@link CUBE_OPERATOR_TO_MONGO_PREDICATE} is keyed by + * {@link CubeOperator}, and `filter.operator` IS a `CubeOperator`, so the + * lookup cannot miss without a type error somewhere first. The throw is the + * totality floor that keeps the old `|| '$eq'` from coming back — the two + * tables drifting must fail loudly, never compile a filter into an equality + * comparison nobody wrote. It is not a user-input path: everything the author + * can get wrong was already refused by {@link ANALYTICS_FILTER_CAPABILITIES}. + */ + private mongoPredicateBuilder(operator: CubeOperator): MongoPredicateBuilder { + const build = (CUBE_OPERATOR_TO_MONGO_PREDICATE as Record)[operator]; + if (!build) { + throw new Error( + `[driver-memory] analytics face: no mingo predicate for cube operator '${operator}'. ` + + `MONGO_TO_CUBE_OPERATOR and CUBE_OPERATOR_TO_MONGO_PREDICATE have drifted — ` + + `add the missing builder rather than letting the operator compile to something else.`, + ); + } + return build; } private operatorToSql(operator: string): string { diff --git a/packages/plugins/driver-memory/src/memory-driver-filter-logic-conformance.test.ts b/packages/plugins/driver-memory/src/memory-driver-filter-logic-conformance.test.ts index 38fb819d7e..c5739987d9 100644 --- a/packages/plugins/driver-memory/src/memory-driver-filter-logic-conformance.test.ts +++ b/packages/plugins/driver-memory/src/memory-driver-filter-logic-conformance.test.ts @@ -72,6 +72,23 @@ * table exists at all: the thing being defended is "this package's filter faces * agree", and a divergence introduced on either axis has to fail in the place * someone looks when they change a lowering. + * + * # The OPERATOR axis (#5374) + * + * A third axis, and the third time this face was measured on one it had never + * been measured on. Shape was covered, comparand TYPE was covered, and what an + * operator MEANS once it has been mapped was not — so `$notContains`, which the + * face declares and the gate admits, compiled to a bare mingo `{$not: 'et'}` + * that constrains nothing and answered with the whole table, for as long as it + * had existed. Same amplifying direction as the other two, arrived at from a + * third place: #5345 was an operator with NO mapping, #5373 was the comparand + * ENCODING, this is an operator whose mapping pointed at the wrong target. + * + * The last section holds the same invariant over that axis, and closes it for + * the whole vocabulary rather than for the one operator: EVERY operator the face + * declares is driven through both paths and must agree, so an operator added to + * `ANALYTICS_FILTER_CAPABILITIES` without a working lowering fails here instead + * of shipping a quietly wrong number. */ import { describe, it, expect, beforeAll } from 'vitest'; @@ -79,7 +96,7 @@ import { FILTER_LOGIC_CASES, FILTER_LOGIC_ROWS } from '@objectstack/spec/data'; import type { Cube, FilterCondition } from '@objectstack/spec/data'; import { InMemoryDriver } from './memory-driver.js'; -import { MemoryAnalyticsService } from './memory-analytics.js'; +import { MemoryAnalyticsService, ANALYTICS_FILTER_CAPABILITIES } from './memory-analytics.js'; import { match } from './memory-matcher.js'; const TABLE = 'conformance'; @@ -472,3 +489,205 @@ describe('[#5373] the generateSql exit emits literals that mean the same thing', .toBe("made_at = '2026-01-01T00:00:00.000Z'"); }); }); + +/** + * [#5374] The same invariant again, over what an operator MEANS. + * + * The fixture is the one the issue measured on — three rows whose `name` is + * `alpha` / `beta` / `gamma`, of which only `beta` contains `et` — so the first + * case below IS the issue's acceptance criterion, on its own numbers. + * + * Every case is asserted against `find()` first and the analytics face second, + * for the reason the #5373 block gives: the live path is the reference, so an + * expectation that is simply wrong is reported as a wrong expectation instead of + * being blamed on the face under test. + */ +const OPERATOR_CASES: Array<[name: string, where: FilterCondition, expected: string[]]> = [ + // ── The issue, on its own measurement ────────────────────────────────────── + // `notContains` mapped to `'$not'` and the call site filled it in as + // `{name: {$not: 'et'}}`. mingo's `$not` takes a regex or an operator + // expression; a bare scalar constrains nothing, so this answered 3 where + // `find()` answers 2 — a predicate emitted, visible in the pipeline, and inert. + ['$notContains excludes the rows that contain the comparand', { name: { $notContains: 'et' } } as FilterCondition, ['1', '3']], + // The tell that separates "inert" from "merely wrong": a comparand every row + // contains must exclude every row. Under the bare `$not` this was the whole + // table, which is also what a correct `$notContains` returns for case 3 below + // — so without this case the defect hides behind an accidentally-right answer. + ['a $notContains that matches every row selects none of them', { name: { $notContains: 'a' } } as FilterCondition, []], + ['a $notContains that matches no row selects all of them', { name: { $notContains: 'zzz' } } as FilterCondition, ['1', '2', '3']], + // `contains` and `notContains` have to partition the table between them. They + // could not while one was a real `$regex` and the other was inert. + ['$contains selects exactly what $notContains excludes', { name: { $contains: 'et' } } as FilterCondition, ['2']], + + // ── The comparand is a LITERAL, not a pattern ────────────────────────────── + // `contains` was mapped correctly to `$regex` but handed the comparand raw, so + // regex metacharacters were live: `a.p` matched `alpha` through `.`, where the + // live path escapes and matched nothing. Fixing `notContains` without fixing + // this would have made the two non-complementary in a new way — `alpha` would + // be in BOTH answers. + ['$contains treats a metacharacter as a literal', { name: { $contains: 'a.p' } } as FilterCondition, []], + ['$notContains treats a metacharacter as a literal', { name: { $notContains: 'a.p' } } as FilterCondition, ['1', '2', '3']], + + // ── Case folding, borrowed rather than re-derived ────────────────────────── + // The live path matches `/…/i`; this face built a case-SENSITIVE regex, so one + // `where` meant two different things depending on which face read it (#5240). + ['$contains is case-insensitive, as the live path is', { name: { $contains: 'ALPHA' } } as FilterCondition, ['1']], + ['$notContains is case-insensitive, as the live path is', { name: { $notContains: 'ALPHA' } } as FilterCondition, ['2', '3']], + ['$contains matches a mixed-case comparand', { name: { $contains: 'Bet' } } as FilterCondition, ['2']], + + // ── A pattern is not a comparand (#4047) ─────────────────────────────────── + // Every operand used to go through the storage-form conversion, so on a + // declared `datetime` column the PATTERN itself was rewritten into canonical + // form and then matched — `find()`, which never rewrites a pattern, matched + // nothing. The two-list `MongoPredicateInput` split is what stops this. + ['$contains does not rewrite its pattern into a datetime storage form', { made_at: { $contains: '2026-01-01T00:00:00Z' } } as FilterCondition, []], + ['$notContains does not rewrite its pattern either', { made_at: { $notContains: '2026-01-01T00:00:00Z' } } as FilterCondition, ['1', '2', '3']], + + // A column holding nulls: the negation must not resurrect the rows it cannot + // test, which is the other way a `$not` goes wrong. + ['$notContains over a column holding nulls', { closed_at: { $notContains: '2026' } } as FilterCondition, ['1', '3']], + ['$contains over a column holding nulls', { closed_at: { $contains: '2026' } } as FilterCondition, ['2']], + + // ── An empty list is a predicate ─────────────────────────────────────────── + // The call site guarded the whole lowering with `values.length > 0`, so an + // empty `$in` emitted NO predicate and the query widened to the whole table + // while `find()` returned nothing — the #3948 direction again, reached through + // the operator table's inability to say "this operator takes the whole list". + ['an empty $in selects nothing', { code: { $in: [] } } as FilterCondition, []], + ['an empty $nin selects everything', { code: { $nin: [] } } as FilterCondition, ['1', '2', '3']], + ['an empty implicit-equality list selects nothing', { code: [] } as unknown as FilterCondition, []], + ['a non-empty $in still selects its members', { code: { $in: ['100'] } } as FilterCondition, ['1', '3']], +]; + +/** + * One probe per operator the face DECLARES — the "declared = enforced" half. + * + * The case table above is chosen by a human who knew where the bug was, which is + * exactly why it cannot be the whole test: #5374 existed because nobody thought + * to look at `$notContains`. This map is keyed by the face's own declared + * vocabulary and the test below fails if a key is missing, so widening + * `ANALYTICS_FILTER_CAPABILITIES` without proving the new operator agrees with + * `find()` is no longer possible to do quietly. + * + * Every probe must EXCLUDE at least one row. A probe that selects the whole + * table agrees with `find()` for free and would certify an inert predicate — + * which is the precise shape of the bug this section exists for. + */ +const DECLARED_OPERATOR_PROBES: Record = { + $eq: { code: { $eq: '100' } } as FilterCondition, + $ne: { code: { $ne: '100' } } as FilterCondition, + $gt: { qty: { $gt: 100 } } as FilterCondition, + $gte: { qty: { $gte: 200 } } as FilterCondition, + $lt: { qty: { $lt: 200 } } as FilterCondition, + $lte: { qty: { $lte: 100 } } as FilterCondition, + $in: { code: { $in: ['100'] } } as FilterCondition, + $nin: { code: { $nin: ['100'] } } as FilterCondition, + $contains: { name: { $contains: 'et' } } as FilterCondition, + $notContains: { name: { $notContains: 'et' } } as FilterCondition, + $exists: { closed_at: { $exists: false } } as FilterCondition, +}; + +describe('[#5374] operator semantics — the analytics face against the live query path', () => { + let driver: InMemoryDriver; + let service: MemoryAnalyticsService; + + beforeAll(async () => { + driver = new InMemoryDriver({ persistence: false }); + await driver.connect(); + await driver.syncSchema(COMPARAND_TABLE, { fields: { ...COMPARAND_FIELDS } } as never); + for (const row of COMPARAND_ROWS) await driver.create(COMPARAND_TABLE, { ...row }); + service = new MemoryAnalyticsService({ driver, cubes: [COMPARAND_CUBE] }); + }); + + const sorted = (ids: string[]): string[] => [...ids].sort((x, y) => x.localeCompare(y)); + + const findIds = async (where: FilterCondition): Promise => { + const rows = await driver.find(COMPARAND_TABLE, { object: COMPARAND_TABLE, fields: ['id'], where }); + return sorted((rows as Array>).map((r) => String(r.id))); + }; + + const analyticsIds = async (where: FilterCondition): Promise => { + const result = await service.query({ + cube: COMPARAND_TABLE, + measures: [`${COMPARAND_TABLE}.count`], + dimensions: [`${COMPARAND_TABLE}.id`], + where, + }); + return sorted((result.rows as Array>).map((r) => String(r[`${COMPARAND_TABLE}.id`]))); + }; + + it('the fixture really is the three rows the issue measured', async () => { + expect(await findIds({})).toEqual(['1', '2', '3']); + expect(await analyticsIds({})).toEqual(['1', '2', '3']); + // Only `beta` contains `et` — the property the issue's numbers rest on. + expect(COMPARAND_ROWS.filter((r) => String(r.name).includes('et')).map((r) => r.id)).toEqual(['2']); + }); + + for (const [name, where, expected] of OPERATOR_CASES) { + it(name, async () => { + expect(await findIds(where), `${name}: the LIVE path disagrees with the expectation`).toEqual(sorted(expected)); + expect( + await analyticsIds(where), + `${name}: the analytics face answered a different row set than find() — the #5240 divergence`, + ).toEqual(sorted(expected)); + }); + } + + /** + * `contains` and `notContains` partition the table, stated as a predicate over + * every comparand rather than case by case. An inert `$notContains` fails this + * for every comparand at once; a `notContains` fixed WITHOUT fixing + * `contains`'s escaping and case folding fails it on the last two. + */ + it('$contains and $notContains partition the table for every comparand', async () => { + for (const needle of ['et', 'a', 'zzz', 'ALPHA', 'a.p', 'Bet']) { + const inside = await analyticsIds({ name: { $contains: needle } } as FilterCondition); + const outside = await analyticsIds({ name: { $notContains: needle } } as FilterCondition); + expect(sorted([...inside, ...outside]), `'${needle}': the two halves are not a partition`).toEqual(['1', '2', '3']); + expect(inside.filter((id) => outside.includes(id)), `'${needle}': a row is in both halves`).toEqual([]); + } + }); + + /** + * The vocabulary, end to end. This is the assertion that would have caught + * #5374 on the day `notContains` was mapped to `$not`. + */ + it('every operator this face DECLARES compiles to a predicate that agrees with find()', async () => { + const declared = [...ANALYTICS_FILTER_CAPABILITIES.fieldOperators].sort(); + expect( + Object.keys(DECLARED_OPERATOR_PROBES).sort(), + 'the face declares an operator with no probe here (or a probe survives an operator that was removed) — ' + + 'widening the vocabulary means proving the new operator agrees with find()', + ).toEqual(declared); + + for (const op of declared) { + const where = DECLARED_OPERATOR_PROBES[op]; + const live = await findIds(where); + expect( + live.length, + `${op}: the probe selects the whole table, so "the faces agree" would prove nothing — pick a discriminating one`, + ).toBeLessThan(COMPARAND_ROWS.length); + expect(await analyticsIds(where), `${op}: the analytics face does not agree with find()`).toEqual(live); + } + }); + + /** + * The pipeline itself, once. The row-set assertions above are what matters, + * but they cannot distinguish "the predicate is right" from "the predicate is + * absent and the rows happen to line up", and the emitted `$match` is the + * artifact the issue actually diagnosed. + */ + it('the emitted $match wraps the negation around a pattern instead of a bare scalar', async () => { + const { sql } = await service.query({ + cube: COMPARAND_TABLE, + measures: [`${COMPARAND_TABLE}.count`], + where: { name: { $notContains: 'et' } } as FilterCondition, + }); + const matchStage = /\/\* Stage 1: \$match \*\/ (.*)/.exec(sql ?? '')?.[1] ?? ''; + // `JSON.stringify` renders a RegExp as `{}`, so assert on the STRUCTURE the + // pipeline carries rather than on that rendering. + expect(matchStage).not.toBe('{"name":{"$not":"et"}}'); + expect(matchStage).toContain('"$not"'); + expect(matchStage).toContain('"$regex"'); + }); +}); diff --git a/packages/plugins/driver-memory/src/memory-driver.ts b/packages/plugins/driver-memory/src/memory-driver.ts index 2bdfbd5048..c3e04a7ee4 100644 --- a/packages/plugins/driver-memory/src/memory-driver.ts +++ b/packages/plugins/driver-memory/src/memory-driver.ts @@ -1233,6 +1233,33 @@ export class InMemoryDriver implements IDataDriver { return this.toStorageForm(object, field, value); } + /** + * [#5374] The pattern a `$contains` / `$notContains` comparand becomes — the + * substring rule itself, for the analytics (cube) face. + * + * Same reasoning as {@link filterComparandStorageForm} one method up, on the + * other half of what a `contains` predicate needs. This driver's rule is + * `escapeRegex` + the `i` flag ({@link normalizeFieldOperators}): the comparand + * is a LITERAL substring, matched case-insensitively. The analytics face has to + * build a `$regex` too, and every byte of that rule it re-derives is a way for + * the two faces to answer one `where` differently — which is what happened + * before this method existed. That face emitted a bare `{$regex: value}`: + * - unescaped, so `{name: {$contains: 'a.p'}}` matched `alpha` through the + * regex `.`, where `find()` matched nothing; and + * - case-SENSITIVE, so `{name: {$contains: 'ALPHA'}}` matched nothing where + * `find()` matched the row. + * + * Returning the built `RegExp` rather than a source string is deliberate: a + * string leaves the flags for the caller to re-choose, which is the half that + * drifted. + * + * Deliberately narrow — one comparand in, one pattern out, no filter semantics + * — so it exposes the convention without exposing the filter pipeline. + */ + filterSubstringPattern(value: unknown): RegExp { + return new RegExp(this.escapeRegex(value as string), 'i'); + } + /** * Put every declared temporal field of a record into its storage form — the * write half of the convention the filter path reads against. Returns the