diff --git a/.changeset/21066-memory-json-column-family-refusal.md b/.changeset/21066-memory-json-column-family-refusal.md new file mode 100644 index 00000000000..60560c2c62b --- /dev/null +++ b/.changeset/21066-memory-json-column-family-refusal.md @@ -0,0 +1,21 @@ +--- +"@objectstack/driver-memory": minor +--- + +fix(driver-memory)!: on a declared JSON-stored field, the query path and the analytics face refuse `$eq` / `$ne` / an ordering / `$between` / `$in` / `$nin` / implicit equality with `INVALID_FILTER` / 400, in the words the SQL family refuses them in, instead of answering each per element + +Clause-②: yes (narrowing) + + + +**BREAKING** (`@objectstack/driver-memory`): this narrows what the driver's filter doors accept, for every caller that reaches them: `find`, `findOne`, `count`, `updateMany`, `deleteMany` and `aggregate` with a `where`, through the engine or called directly, and `MemoryAnalyticsService`'s `query` and `generateSql`. It ships as `minor` under the launch-window convention for accept-set narrowings. + +**What is refused.** On a field the object declares JSON-stored (a structured-JSON type such as `json` or `address`, an inherently multi-value option type such as `tags`, `multiselect` or `checkboxes`, or a `select`, `radio`, `lookup`, `user`, `file` or `image` field declared `multiple: true`), a `where` that compares the field with `$eq`, `$ne`, `$gt`, `$gte`, `$lt`, `$lte`, `$between`, `$in`, `$nin` or implicit equality (`{ "owners": "u1" }`) is refused with `INVALID_FILTER` / 400, whatever the comparand (`null` and an empty list included), at any depth under `$and` / `$or` / `$not`, before any row is read. On the analytics face a `where` key is a cube member, judged by the field it resolves to. That is the set `driver-sql` refuses on such a column, for the same reason. + +**What an author sees now.** The body `driver-sql` answers for the same filter: the filter WAS NOT APPLIED, the comparison can never equal one member of a stored list, and the spelling to use, `{ "FIELD": { "$contains": "a" } }` for membership, or an `$or` of `$contains` for any-of. The field and the operator are withheld from the message, and the full diagnostic, naming both and the position in the filter, is written to the driver's (or the analytics service's) logger at `warn`. + +**Why a refusal.** This driver answered each of those operators per element, through mingo's array semantics. Measured through `engine.find` over six rows of a `multiple: true` lookup, two of them holding `u1`: `{ owners: { $eq: 'u1' } }` and `{ owners: { $in: ['u1', 'u9'] } }` returned those two rows, `$nin` the other four, and `{ owners: { $gt: 'u1' } }` four rows by comparing each member as text, where every SQL dialect answers the same filters 400. An application whose tests run on this driver passed on a filter its production backend refuses. + +**Who is affected.** A test suite, demo or dev setup on this driver that filters a JSON-stored field with one of those operators and read the per-element rows as the answer. Write `$contains` for "holds this member", an `$or` of `$contains` for "holds any of these", and `$not` around either for the exclusion. + +**Unchanged.** `$contains` and `$notContains` (membership on such a field), `$exists`, `$null` and `$empty`; every operator on a field that is not declared JSON-stored; and an object this driver holds no declaration for (one never passed through `syncSchema`), where nothing is judged and every operator answers as before. `InMemoryDriver` gains one method, `filterFieldDeclarations`, tagged `@internal`: it exists so the analytics face judges its `where` by the same declarations, and it is not a consumer contract. diff --git a/packages/drivers/driver-memory/src/filter-refusal.ts b/packages/drivers/driver-memory/src/filter-refusal.ts index c1be71dc879..ba107b4bc13 100644 --- a/packages/drivers/driver-memory/src/filter-refusal.ts +++ b/packages/drivers/driver-memory/src/filter-refusal.ts @@ -46,6 +46,11 @@ import { BUCKET_GRANULARITIES } from '@objectstack/core'; // refusal can tell a value the contract never declared apart from one it // declares and this backend cannot label. import { RETIRED_SUB_DAY_INTERVALS, TimeUpdateInterval } from '@objectstack/spec/data'; +// [#21066] The scalar-comparison family a JSON-stored field refuses, and the +// words of that refusal — the ONE set and sentence `driver-sql`'s `where` and +// `@objectstack/objectql`'s per-aggregation `filter` already read (#21007), so +// this driver's faces refuse the same operators in the same words. +import { JSON_COLUMN_INCOMPATIBLE_OPERATORS, jsonColumnOperatorRefusalText } from '@objectstack/core'; import { StandardErrorCode } from '@objectstack/spec/api'; /** @@ -821,6 +826,91 @@ export function arrayComparandError(field: string, value: unknown, path: string, ); } +/** + * [#21066] What the shape gate is told about the DECLARED fields a filter names. + * + * Every other rule on the walk reads the filter alone. This one cannot: whether + * `{ owners: { $in: ['u1'] } }` is a well-formed question depends on how + * `owners` is STORED, which is declared metadata the calling face holds and the + * filter does not carry. So the face hands the walk the two things it needs, and + * the walk keeps deciding everything in one pass, before any face evaluates. + * + * Omitted (the default) ⇒ no field is judged JSON-stored, which is the answer + * for a face holding no declarations: the walk does not guess a storage shape + * from a name or a value. + */ +export interface FilterFieldDeclarations { + /** + * Is `field`, as the filter spells it, DECLARED JSON-stored on the object + * being read — a structured-JSON type or a multi-valued field? A field the + * face holds no declaration for answers `false`. + */ + readonly isJsonStoredField: (field: string) => boolean; + /** + * Handed the withheld half of a refusal — the field and the operator named, + * with the position in the filter — just before the refusal is thrown, so the + * face writes it to its server log. The caller is told only that it is there. + */ + readonly reportWithheld: (diagnostic: string) => void; +} + +/** + * [#21066] The server-log line for a withheld filter diagnostic, one wording + * for every face of this package. + */ +export function withheldFilterLogLine(diagnostic: string): string { + return ( + `[driver-memory] INVALID_FILTER — refusal detail withheld from the response. ` + + `Full diagnostic: ${diagnostic}` + ); +} + +/** + * [#21066] A scalar comparison — the equality and ordering family, `$between`, + * `$in` / `$nin`, or implicit equality — aimed at a field DECLARED JSON-stored. + * + * ## What this driver answered instead + * + * Each operator, PER ELEMENT: mingo applies a scalar comparison to every member + * of an array value, so on six rows whose `owners` (a `multiple: true` lookup) + * hold `['u1','u2']`, `['u2']`, `['u3','u1']`, `[]`, `['u10']` and `null`, + * `{ owners: { $eq: 'u1' } }` and `$in ['u1','u9']` returned the two rows + * holding `u1`, `$nin` the other four, and `$gt 'u1'` four rows by comparing + * each member as text. `driver-sql` refuses every one of those filters with + * `INVALID_FILTER` / 400 on every dialect (#7398), and so does the engine's + * per-aggregation `filter` (#21007). One filter, rows on this driver and a + * refusal on the SQL family, breaks the conformance invariant every backend is + * held to — the same rows as `find()`, or a refusal — and it is the worse + * direction for a test double: an application's tests pass here on a filter + * its production backend refuses. + * + * No declared contract gives the family a per-element reading: the spec's + * `$contains` docblock names `$contains` as the membership spelling on such a + * column and the one operator the equality family's refusal left working, and + * that is what the refusal prescribes. + * + * ## The words, and what they withhold + * + * The set and the sentence are `@objectstack/core`'s, read rather than copied — + * the text `driver-sql` refuses with, byte for byte. The message names neither + * the field nor the operator (on a read scope the predicate is an + * administrator's), and says the full diagnostic is in the server log; the + * diagnostic, prefixed with the position in the filter, goes to + * `reportWithheld` just before the refusal is thrown, so that sentence is true + * here too. The constructor is this package's, as each face keeps its own. + */ +export function jsonStoredFieldOperatorError( + field: string, + op: string, + bare: boolean, + path: string, + reportWithheld: (diagnostic: string) => void, +): Error { + const { message, diagnostic } = jsonColumnOperatorRefusalText(field, op, bare); + reportWithheld(`At ${path}: ${diagnostic}`); + return unsupportedFilterError(message); +} + /** [#5324] `$and`/`$or` take a list of nodes; anything else is refused. */ export function filterNodeListExpectedError(key: string, value: unknown, path: string): Error { return unsupportedFilterError( @@ -890,11 +980,23 @@ export function filterNodeExpectedError(value: unknown, path: string): Error { * deliberately: on a face that cannot compile `$or` at all, reporting that its * operand should have been an array would send the author to fix the wrong * thing, then refuse the corrected filter anyway. + * + * ## What `declarations` adds (#21066) + * + * The one rule on this walk that reads the DECLARATION rather than the filter: + * a scalar comparison aimed at a field declared JSON-stored is refused (see + * {@link jsonStoredFieldOperatorError}). It is a MEANING the filter would get + * wrong rather than a shape, and it lives here anyway, because here is where + * every face of this package already stops a filter before evaluating it — on + * the whole tree at once, `$not` and every `$or` branch included, so the + * refusal cannot depend on which row reaches which arm. Omitted ⇒ nothing is + * judged JSON-stored (see {@link FilterFieldDeclarations}). */ export function assertFilterConditionShape( node: unknown, path: string, capabilities: FilterFaceCapabilities = DRIVER_FILTER_CAPABILITIES, + declarations?: FilterFieldDeclarations, ): void { if (!isFilterNode(node)) return; for (const [key, value] of Object.entries(node)) { @@ -905,18 +1007,18 @@ export function assertFilterConditionShape( value.forEach((child, index) => { const childPath = `${here}[${index}]`; if (!isFilterNode(child)) throw filterNodeExpectedError(child, childPath); - assertFilterConditionShape(child, childPath, capabilities); + assertFilterConditionShape(child, childPath, capabilities, declarations); }); continue; } if (key === '$not') { if (!capabilities.combinators.has(key)) throw uncompilableCombinatorError(key, here, capabilities); if (!isFilterNode(value)) throw filterNodeExpectedError(value, here); - assertFilterConditionShape(value, here, capabilities); + assertFilterConditionShape(value, here, capabilities, declarations); continue; } if (key.startsWith('$')) throw unknownLogicalOperatorError(key, here); - assertFieldConstraintShape(key, value, here, capabilities); + assertFieldConstraintShape(key, value, here, capabilities, declarations); } } @@ -936,13 +1038,25 @@ function assertFieldConstraintShape( spec: unknown, path: string, capabilities: FilterFaceCapabilities, + declarations: FilterFieldDeclarations | undefined, ): void { // [#16810] The IMPLICIT-equality position, checked before the plain-object // test below because an array is not a filter node and would otherwise leave // this walk unjudged — which is how it reached the matcher's `==` arm and the // live path's deep equality with nobody reconciling the two. if (Array.isArray(spec)) throw arrayComparandError(field, spec, path); - if (!isFilterNode(spec)) return; + if (!isFilterNode(spec)) { + // [#21066] The bare `{ field: value }` spelling IS equality, and on a + // declared JSON-stored field it is refused as `$eq` is — reported as `=`, + // bare, as `driver-sql` reports it — whatever the comparand, `null` + // included, because `driver-sql`'s `where` refuses that one too. After the + // array refusal above, the order `driver-sql` takes (its comparand gate, + // then its column-type gate). + if (declarations?.isJsonStoredField(field)) { + throw jsonStoredFieldOperatorError(field, '=', true, path, declarations.reportWithheld); + } + return; + } // [#5240] The zero-operator constraint keeps its own predicate rather than an // inlined `keys.length === 0`, so the reasoning for what does and does not // count as one (a `Date` enumerates to nothing but is a comparand) stays @@ -1028,6 +1142,18 @@ function assertFieldConstraintShape( throw nulLikePatternError(field, op, spec[op] as string, `${path}.${op}`); } } + // [#21066] The column-type question, after every comparand-shape rule above + // — the order `driver-sql` takes, so a malformed `$between` or an array + // under `$eq` is still told what is wrong with its COMPARAND first. The set + // is the shared one, so `$contains` / `$notContains` (membership), the null + // predicates and `$empty` keep answering on such a field. + if ( + declarations && + JSON_COLUMN_INCOMPATIBLE_OPERATORS.has(op) && + declarations.isJsonStoredField(field) + ) { + throw jsonStoredFieldOperatorError(field, op, false, `${path}.${op}`, declarations.reportWithheld); + } } // [#5702] The `$options`-without-`$regex` companion check that stood here is // GONE. It was needed while `$options` was an allowlisted MODIFIER — a key the diff --git a/packages/drivers/driver-memory/src/memory-20444-empty-operator.test.ts b/packages/drivers/driver-memory/src/memory-20444-empty-operator.test.ts index 875db96ff07..f9534f5462d 100644 --- a/packages/drivers/driver-memory/src/memory-20444-empty-operator.test.ts +++ b/packages/drivers/driver-memory/src/memory-20444-empty-operator.test.ts @@ -28,6 +28,7 @@ import { beforeAll, describe, expect, it } from 'vitest'; import type { Cube, FilterCondition } from '@objectstack/spec/data'; import { isEmptyFilterValue } from '@objectstack/spec/data'; +import { jsonColumnOperatorRefusalText } from '@objectstack/core'; import { InMemoryDriver } from './memory-driver.js'; import { assertFilterConditionShape } from './filter-refusal.js'; import { MemoryAnalyticsService } from './memory-analytics.js'; @@ -65,7 +66,12 @@ const CASES: Array<{ where: FilterCondition; expected: string[] }> = [ { where: { $or: [{ score: 5 }, { tags: { $empty: true } }] }, expected: ['r1', 'r2', 'r3', 'r5'] }, { where: { $and: [{ title: { $empty: false } }, { owners: { $empty: false } }] }, expected: ['r1', 'r4'] }, { where: { title: { $empty: false, $ne: 'x' } }, expected: ['r4'] }, - { where: { tags: { $empty: true, $ne: null } }, expected: ['r2'] }, + // [#21066] This cell was `{ tags: { $empty: true, $ne: null } }` → `r2`. A + // `$ne` on a declared multi-value field is now refused, as the SQL family + // refuses it (pinned below), so the composition — `$empty` beside a "has a + // value" sibling on ONE multi-value field — is held through `$null: false`, + // the sibling that still answers there. `driver-sql`/SQLite answers `r2` too. + { where: { tags: { $empty: true, $null: false } }, expected: ['r2'] }, ]; function refusal(run: () => unknown): Promise<{ code?: string; status?: number } | 'answered'> { @@ -150,6 +156,20 @@ describe('[#20444] InMemoryDriver — $empty on the live path, the by-value read expect(byValue(rows, { score: { $empty: true } })).toEqual(['blank']); }); + it('[#21066] $ne beside $empty on a declared multi-value field is refused, in the SQL family\'s words', async () => { + let thrown: { code?: string; status?: number; message?: string } | undefined; + try { + await driver.find(TABLE, { where: { tags: { $empty: true, $ne: null } } }); + } catch (err) { + thrown = err as typeof thrown; + } + expect({ code: thrown?.code, status: thrown?.status, message: thrown?.message }).toEqual({ + code: 'INVALID_FILTER', + status: 400, + message: jsonColumnOperatorRefusalText('tags', '$ne', false).message, + }); + }); + it('a non-boolean flag is refused on the live path and by the shared shape gate itself', async () => { expect(await refusal(() => driver.find(TABLE, { where: { title: { $empty: 'yes' as never } } }))) .toEqual({ code: 'INVALID_FILTER', status: 400 }); diff --git a/packages/drivers/driver-memory/src/memory-21066-json-column-family-refusal.test.ts b/packages/drivers/driver-memory/src/memory-21066-json-column-family-refusal.test.ts new file mode 100644 index 00000000000..f76580cbbce --- /dev/null +++ b/packages/drivers/driver-memory/src/memory-21066-json-column-family-refusal.test.ts @@ -0,0 +1,325 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#21066] On a field DECLARED JSON-stored — a `multiple: true` field, an + * inherently multi-value option type (`tags`) or a structured-JSON type + * (`json`) — this driver REFUSES the scalar-comparison family `driver-sql`'s + * `where` refuses (#7398) and the engine's per-aggregation `filter` refuses + * (#21007): `INVALID_FILTER` / 400, in the same words, read from the one set + * and sentence `@objectstack/core` holds. `$contains` / `$notContains` + * (membership), the null predicates and `$empty` keep answering. + * + * ## The defect, measured through `engine.find` before the change + * + * `origin/main` `670680e93`, a real `ObjectQL` over this driver, the six rows + * below (#21004's fixture). The SQL family refuses every one of these 400 + * `INVALID_FILTER`; this driver answered each PER ELEMENT, through mingo's array + * semantics: + * + * | `where` | before | now | + * |:--|:--|:--| + * | `owners` `$eq 'u1'` | `d1`, `d3` | 400 | + * | `owners` `$in ['u1','u9']` | `d1`, `d3` | 400 | + * | `owners` `$nin ['u1','u9']` | `d2`, `d4`, `d5`, `d6` | 400 | + * | `owners` `$gt 'u1'` | `d1`, `d2`, `d3`, `d5` | 400 | + * | `tags` `$gt 'red'` | `d3` | 400 | + * | `owners` `$contains 'u1'` (the prescription, the control) | `d1`, `d3` | `d1`, `d3` | + * + * The analytics (cube) face answered the same rows, and its SQL echo rendered + * `owners = 'u1'` — a statement that, run over the JSON text the SQL family + * stores, returns none. The engine's run was a one-off: this package may not + * import the engine (`check:driver-memory-census`), so what is pinned here is + * the `where` the engine hands the driver, recorded in that run — the card's + * operators as written, and `$ne` / `$nin` inside the spec's null-safe lowering + * (`{ $and: [{ $or: [{ f: { $null: true } }, { f: { $nin: [...] } }] }] }`). + * + * ## Why the words are compared, not just the code + * + * The refusal is ONE refusal across backends, so a caller swapping this driver + * for SQL must see the same body. The message is asserted equal to + * `jsonColumnOperatorRefusalText`'s — imported, so the sentence has no copy + * here — on top of `code` and `status`. + */ + +import { describe, it, expect, beforeAll, vi } from 'vitest'; +import type { Cube } from '@objectstack/spec/data'; +import { JSON_COLUMN_INCOMPATIBLE_OPERATORS, jsonColumnOperatorRefusalText } from '@objectstack/core'; +import { InMemoryDriver } from './memory-driver.js'; +import { MemoryAnalyticsService } from './memory-analytics.js'; +import { + assertFilterConditionShape, + DRIVER_FILTER_CAPABILITIES, + SUPPORTED_FIELD_OPERATORS, +} from './filter-refusal.js'; + +const OBJECT = 'mem21066_doc'; + +const FIELDS = { + title: { type: 'text' }, + owners: { type: 'lookup', reference: 'mem21066_user', multiple: true }, + tags: { type: 'tags' }, + meta: { type: 'json' }, +} as const; + +/** #21004's six rows, plus a `json` column. */ +const ROWS = [ + { id: 'd1', title: 'u1 memo', owners: ['u1', 'u2'], tags: ['red', 'blue'], meta: ['a'] }, + { id: 'd2', title: 'none', owners: ['u2'], tags: ['blue'], meta: { k: 'a' } }, + { id: 'd3', title: 'about u10', owners: ['u3', 'u1'], tags: ['redwood'], meta: 'a' }, + { id: 'd4', title: 'x', owners: [], tags: [], meta: [] }, + { id: 'd5', title: 'u1', owners: ['u10'], tags: ['red'], meta: null }, + { id: 'd6', title: null, owners: null, tags: null, meta: null }, +] as const; + +const byId = (a: string, b: string) => a.localeCompare(b); + +interface Refusal { + code?: string; + status?: number; + message?: string; +} + +async function refusalOf(run: () => unknown): Promise { + try { + await run(); + return 'answered'; + } catch (err) { + const e = err as Refusal; + return { code: e.code, status: e.status, message: e.message }; + } +} + +/** What the SQL family answers for the same filter — the shared text, never a copy. */ +function sharedRefusal(field: string, op: string, bare: boolean): Refusal { + return { code: 'INVALID_FILTER', status: 400, message: jsonColumnOperatorRefusalText(field, op, bare).message }; +} + +async function seed({ declare = true } = {}): Promise<{ driver: InMemoryDriver; warn: ReturnType }> { + const driver = new InMemoryDriver({ persistence: false }); + await driver.connect(); + if (declare) await driver.syncSchema(OBJECT, { name: OBJECT, fields: FIELDS } as never); + for (const row of ROWS) await driver.create(OBJECT, structuredClone({ ...row }) as never); + const warn = vi.spyOn((driver as unknown as { logger: { warn: (...a: unknown[]) => void } }).logger, 'warn') + .mockImplementation(() => undefined); + return { driver, warn }; +} + +const findIds = async (driver: InMemoryDriver, where: unknown): Promise => + ((await driver.find(OBJECT, { where } as never)) as Array>).map((r) => String(r.id)).sort(byId); + +/** A comparand an author could write for `op` on `field` — a list for the list operators, a pair for `$between`. */ +function comparandFor(op: string, field: string): unknown { + const [a, b] = field === 'owners' ? ['u1', 'u9'] : field === 'tags' ? ['red', 'green'] : ['a', 'b']; + if (op === '$in' || op === '$nin') return [a, b]; + if (op === '$between') return [a, b]; + return a; +} + +/** + * The family as THIS driver's FilterCondition vocabulary spells it: every + * member of the shared set the shape gate admits as an operator. The bare infix + * spellings (`=`, `in`, …) are not FilterCondition operators here and are + * refused by the vocabulary gate before this one; they are the AST door's. + */ +const FAMILY = [...JSON_COLUMN_INCOMPATIBLE_OPERATORS].filter((op) => SUPPORTED_FIELD_OPERATORS.has(op)); + +/** The card's table: [name, where, field, operator the refusal names, bare?]. */ +const CARD: ReadonlyArray, string, string, boolean]> = [ + ["owners $eq 'u1'", { owners: { $eq: 'u1' } }, 'owners', '$eq', false], + ["owners $in ['u1','u9']", { owners: { $in: ['u1', 'u9'] } }, 'owners', '$in', false], + ["owners $nin ['u1','u9'] (as the engine lowers it)", { $and: [{ $or: [{ owners: { $null: true } }, { owners: { $nin: ['u1', 'u9'] } }] }] }, 'owners', '$nin', false], + ["owners $gt 'u1'", { owners: { $gt: 'u1' } }, 'owners', '$gt', false], + ["tags $gt 'red'", { tags: { $gt: 'red' } }, 'tags', '$gt', false], +]; + +/** Shapes of the family beyond one operator — every one refused, as `driver-sql`'s `where` refuses it. */ +function shapes(field: string): ReadonlyArray, string, boolean]> { + const a = comparandFor('$eq', field); + return [ + ['implicit equality', { [field]: a }, '=', true], + ['implicit equality with null', { [field]: null }, '=', true], + ['$eq null', { [field]: { $eq: null } }, '$eq', false], + ['$ne null', { [field]: { $ne: null } }, '$ne', false], + ['$ne, as the engine lowers it', { $and: [{ $or: [{ [field]: { $null: true } }, { [field]: { $ne: a } }] }] }, '$ne', false], + ['$in []', { [field]: { $in: [] } }, '$in', false], + ['$nin []', { [field]: { $nin: [] } }, '$nin', false], + ['$in under $not', { $not: { [field]: { $in: [a] } } }, '$in', false], + ['$nin in an $or branch after one that holds', { $or: [{ title: 'x' }, { [field]: { $nin: [a] } }] }, '$nin', false], + ['$eq beside $contains on the same field', { [field]: { $contains: a, $eq: a } }, '$eq', false], + ]; +} + +describe('[#21066] InMemoryDriver refuses the scalar-comparison family on a declared JSON-stored field', () => { + let driver: InMemoryDriver; + let warn: ReturnType; + beforeAll(async () => { + ({ driver, warn } = await seed()); + }); + + it('the fixture holds all six rows, and the family it iterates is the shared set, at least the nine $-spellings', async () => { + expect(await findIds(driver, {})).toEqual(['d1', 'd2', 'd3', 'd4', 'd5', 'd6']); + // A floor, not an equality: a member the shared set gains (a later card + // widening it) is pinned here by the loops below without editing this file. + expect(FAMILY).toEqual(expect.arrayContaining(['$eq', '$ne', '$gt', '$gte', '$lt', '$lte', '$between', '$in', '$nin'])); + }); + + for (const [name, where, field, op, bare] of CARD) { + it(`the card: ${name} → 400 INVALID_FILTER in the SQL family's words`, async () => { + expect(await refusalOf(() => driver.find(OBJECT, { where } as never))).toEqual(sharedRefusal(field, op, bare)); + }); + } + + for (const field of ['owners', 'tags', 'meta']) { + for (const op of FAMILY) { + it(`${field} ${op} → refused`, async () => { + const where = { [field]: { [op]: comparandFor(op, field) } }; + expect(await refusalOf(() => driver.find(OBJECT, { where } as never))).toEqual(sharedRefusal(field, op, false)); + }); + } + for (const [name, where, op, bare] of shapes(field)) { + it(`${field}: ${name} → refused`, async () => { + expect(await refusalOf(() => driver.find(OBJECT, { where } as never))).toEqual(sharedRefusal(field, op, bare)); + }); + } + } + + it('every filter door of the query path refuses — count, findOne, updateMany and deleteMany — and a write leaves the table untouched', async () => { + const where = { owners: { $nin: ['u1'] } }; + const expected = sharedRefusal('owners', '$nin', false); + expect(await refusalOf(() => driver.count(OBJECT, { where } as never))).toEqual(expected); + expect(await refusalOf(() => driver.findOne(OBJECT, { where } as never))).toEqual(expected); + expect(await refusalOf(() => driver.updateMany(OBJECT, { where } as never, { title: 'clobbered' }))).toEqual(expected); + expect(await refusalOf(() => driver.deleteMany(OBJECT, { where } as never))).toEqual(expected); + const rows = (await driver.find(OBJECT, {} as never)) as Array>; + expect(rows.map((r) => r.id).sort()).toEqual(['d1', 'd2', 'd3', 'd4', 'd5', 'd6']); + expect(rows.some((r) => r.title === 'clobbered')).toBe(false); + }); + + it('the message withholds the field and the operator; the diagnostic, with its position, goes to the server log', async () => { + warn.mockClear(); + const got = await refusalOf(() => driver.find(OBJECT, { where: { $or: [{ title: 'x' }, { owners: { $gte: 'u1' } }] } } as never)); + expect(got).toEqual(sharedRefusal('owners', '$gte', false)); + const message = (got as Refusal).message!; + expect(message).not.toContain('owners'); + expect(message).not.toContain('$gte'); + expect(warn).toHaveBeenCalledTimes(1); + const line = String(warn.mock.calls[0]![0]); + expect(line).toContain('At filter.$or[1].owners.$gte: '); + expect(line).toContain(jsonColumnOperatorRefusalText('owners', '$gte', false).diagnostic); + }); + + it('a comparand the gate already refuses is still told about its COMPARAND first — driver-sql\'s order', async () => { + // An array under a single-value operator, and a one-element `$between`: + // both `INVALID_FILTER` / 400 already, in their own words. + const array = await refusalOf(() => driver.find(OBJECT, { where: { owners: { $eq: ['u1'] } } } as never)); + expect(array).toMatchObject({ code: 'INVALID_FILTER', status: 400 }); + expect((array as Refusal).message).toContain('requires a single comparable value'); + const between = await refusalOf(() => driver.find(OBJECT, { where: { owners: { $between: ['u1'] } } } as never)); + expect(between).toMatchObject({ code: 'INVALID_FILTER', status: 400 }); + expect((between as Refusal).message).toContain('requires a [min, max] value array'); + }); + + // The other half of the contract: what still answers on the same fields. + const ANSWERED: ReadonlyArray, readonly string[]]> = [ + ["owners $contains 'u1' — the prescription", { owners: { $contains: 'u1' } }, ['d1', 'd3']], + ['an $or of $contains — the any-of prescription', { $or: [{ owners: { $contains: 'u1' } }, { owners: { $contains: 'u10' } }] }, ['d1', 'd3', 'd5']], + ["owners $notContains 'u1', as the engine lowers it", { $and: [{ $or: [{ owners: { $null: true } }, { owners: { $notContains: 'u1' } }] }] }, ['d2', 'd4', 'd5', 'd6']], + ["tags $contains 'red'", { tags: { $contains: 'red' } }, ['d1', 'd5']], + ['owners $null: true', { owners: { $null: true } }, ['d6']], + ['owners $exists: false', { owners: { $exists: false } }, ['d6']], + ['owners $empty: true', { owners: { $empty: true } }, ['d4', 'd6']], + ['meta $null: false', { meta: { $null: false } }, ['d1', 'd2', 'd3', 'd4']], + ['title $in — a scalar column, untouched', { title: { $in: ['u1', 'none'] } }, ['d2', 'd5']], + ["title implicit equality — a scalar column, untouched", { title: 'x' }, ['d4']], + ["title $gt — a scalar column, untouched", { title: { $gt: 'u1' } }, ['d1', 'd4']], + ]; + for (const [name, where, expected] of ANSWERED) { + it(`still answered: ${name}`, async () => { + expect(await findIds(driver, where)).toEqual([...expected]); + }); + } +}); + +describe('[#21066] the population is the DECLARATION', () => { + it('an object never passed through syncSchema is not judged — the per-element answer it always had, as driver-sql answers a table it was never told about', async () => { + const { driver } = await seed({ declare: false }); + expect(await findIds(driver, { owners: { $eq: 'u1' } })).toEqual(['d1', 'd3']); + expect(await findIds(driver, { tags: { $gt: 'red' } })).toEqual(['d3']); + }); + + it('a declared SCALAR field is not judged by the row it holds: a text column holding an array still answers', async () => { + const driver = new InMemoryDriver({ persistence: false }); + await driver.connect(); + await driver.syncSchema('mem21066_scalar', { fields: { label: { type: 'text' } } } as never); + await driver.create('mem21066_scalar', { id: 's1', label: ['a', 'b'] } as never); + const rows = (await driver.find('mem21066_scalar', { where: { label: { $eq: 'a' } } } as never)) as Array>; + expect(rows.map((r) => r.id)).toEqual(['s1']); + }); + + it('the shared shape gate refuses only when it is handed the declarations, and reports before it throws', () => { + const reported: string[] = []; + const declarations = { + isJsonStoredField: (field: string) => field === 'owners', + reportWithheld: (diagnostic: string) => { reported.push(diagnostic); }, + }; + const where = { $not: { owners: { $in: ['u1'] } } }; + expect(() => assertFilterConditionShape(where, 'filter')).not.toThrow(); + expect(() => assertFilterConditionShape({ title: { $in: ['u1'] } }, 'filter', DRIVER_FILTER_CAPABILITIES, declarations)).not.toThrow(); + let thrown: Refusal | undefined; + try { + assertFilterConditionShape(where, 'filter', DRIVER_FILTER_CAPABILITIES, declarations); + } catch (err) { + thrown = err as Refusal; + } + expect({ code: thrown?.code, status: thrown?.status, message: thrown?.message }).toEqual(sharedRefusal('owners', '$in', false)); + expect(reported).toEqual([`At filter.$not.owners.$in: ${jsonColumnOperatorRefusalText('owners', '$in', false).diagnostic}`]); + }); +}); + +describe('[#21066] the analytics (cube) face refuses the same filters, and its SQL echo with it', () => { + let service: MemoryAnalyticsService; + let warn: ReturnType; + + beforeAll(async () => { + const { driver } = await seed(); + const dimensions: Record = { id: { label: 'Id', type: 'string', sql: 'id' } }; + for (const field of Object.keys(FIELDS)) dimensions[field] = { label: field, type: 'string', sql: field }; + const cube = { + name: 'docs', + title: 'Docs', + sql: OBJECT, + measures: { count: { label: 'Count', type: 'count', sql: 'id' } }, + dimensions, + } as unknown as Cube; + service = new MemoryAnalyticsService({ driver, cubes: [cube] }); + warn = vi.spyOn((service as unknown as { logger: { warn: (...a: unknown[]) => void } }).logger, 'warn') + .mockImplementation(() => undefined); + }); + + const query = (where: unknown) => ({ cube: 'docs', measures: ['docs.count'], dimensions: ['docs.id'], where }) as never; + + for (const [name, where, field, op, bare] of [ + ...CARD.filter(([name]) => !name.includes('as the engine lowers it')), + ["owners $nin ['u1','u9'] — the face lowers it itself", { owners: { $nin: ['u1', 'u9'] } }, 'owners', '$nin', false] as const, + ["owners implicit equality 'u1'", { owners: 'u1' }, 'owners', '=', true] as const, + ["the member spelled cube.member — docs.owners $in", { 'docs.owners': { $in: ['u1'] } }, 'docs.owners', '$in', false] as const, + ]) { + it(`${name} → refused by query() and by generateSql()`, async () => { + const expected = sharedRefusal(field, op, bare); + expect(await refusalOf(() => service.query(query(where)))).toEqual(expected); + expect(await refusalOf(() => service.generateSql(query(where)))).toEqual(expected); + }); + } + + it('the withheld half goes to the face\'s own log', async () => { + warn.mockClear(); + await refusalOf(() => service.query(query({ tags: { $gt: 'red' } }))); + expect(warn).toHaveBeenCalledTimes(1); + expect(String(warn.mock.calls[0]![0])).toContain(`At where.tags.$gt: ${jsonColumnOperatorRefusalText('tags', '$gt', false).diagnostic}`); + }); + + it("$contains still answers on the face — find()'s rows", async () => { + const result = await service.query(query({ owners: { $contains: 'u1' } })); + expect(result.rows.map((r: Record) => String(r['docs.id'])).sort(byId)).toEqual(['d1', 'd3']); + }); +}); diff --git a/packages/drivers/driver-memory/src/memory-analytics.ts b/packages/drivers/driver-memory/src/memory-analytics.ts index 9de9198bd6f..bc1d14c7203 100644 --- a/packages/drivers/driver-memory/src/memory-analytics.ts +++ b/packages/drivers/driver-memory/src/memory-analytics.ts @@ -59,6 +59,8 @@ import { unsupportedFilterError, unsupportedTimeGranularityError, type FilterFaceCapabilities, + // [#21066] The JSON-stored refusal's log line, written by this face's logger. + withheldFilterLogLine, } from './filter-refusal.js'; /** @@ -1143,7 +1145,7 @@ export class MemoryAnalyticsService implements IAnalyticsService { // into the cube-style `{member, operator, values}` list this pipeline // consumes, and anything this face cannot lower is refused there rather than // dropped (#5345). - const normalizedFilters = this.normalizeFilters(query); + const normalizedFilters = this.normalizeFilters(query, cube); if (normalizedFilters.length > 0) { const matchStage = this.mongoConjunction(cube, normalizedFilters); if (Object.keys(matchStage).length > 0) { @@ -1587,7 +1589,7 @@ export class MemoryAnalyticsService implements IAnalyticsService { // construction for the same reason. There is deliberately no // `values.length > 0` guard any more: an empty list IS a predicate, and // skipping the clause described the whole table (see the `in` row). - const whereClauses = this.sqlConjunction(cube, this.normalizeFilters(query)); + const whereClauses = this.sqlConjunction(cube, this.normalizeFilters(query, cube)); let sql = `SELECT ${selectClauses.join(', ')} FROM ${tableName}`; if (whereClauses.length > 0) { @@ -1812,7 +1814,7 @@ export class MemoryAnalyticsService implements IAnalyticsService { * lowering is refused `INVALID_FILTER` / 400. It used to skip all three steps * and the flatten, and so answered every row. */ - private normalizeFilters(query: unknown): NormalizedCubeEntry[] { + private normalizeFilters(query: unknown, cube: Cube): NormalizedCubeEntry[] { if (!query || typeof query !== 'object') return []; const out: NormalizedCubeEntry[] = []; @@ -1822,7 +1824,18 @@ export class MemoryAnalyticsService implements IAnalyticsService { assertListComparandShapes(where); const admitted = normalizeFilterComparandTypes(where); const lowered = lowerFilterCondition(admitted); - assertFilterConditionShape(lowered, 'where', ANALYTICS_FILTER_CAPABILITIES); + // [#21066] A `where` key here is a cube MEMBER; the declaration it is + // judged by is the field it resolves to on the cube's table — the same + // (table, field path) pair `$contains` asks the driver about + // (`filterContainsTest`), so this face refuses the equality and ordering + // family on exactly the fields where `$contains` asks membership, as + // `find()` does. The withheld half goes to this face's own log. + const table = this.extractTableName(cube.sql); + const declared = this.driver.filterFieldDeclarations(table); + assertFilterConditionShape(lowered, 'where', ANALYTICS_FILTER_CAPABILITIES, { + isJsonStoredField: (member) => declared.isJsonStoredField(this.resolveFieldPath(cube, member)), + reportWithheld: (diagnostic) => this.logger.warn(withheldFilterLogLine(diagnostic)), + }); this.flattenFilterCondition(lowered as Record, out, 'where'); } diff --git a/packages/drivers/driver-memory/src/memory-driver.ts b/packages/drivers/driver-memory/src/memory-driver.ts index d376dadbd86..c13b5b07c80 100644 --- a/packages/drivers/driver-memory/src/memory-driver.ts +++ b/packages/drivers/driver-memory/src/memory-driver.ts @@ -27,6 +27,11 @@ import { } from './memory-tenancy-guard.js'; import { assertFilterConditionShape, + // [#21066] What the shape gate is told about the declared fields, and the + // one log line a withheld refusal writes. + DRIVER_FILTER_CAPABILITIES, + type FilterFieldDeclarations, + withheldFilterLogLine, filterArrayReachedDriverError, filterNodeExpectedError, filterNodeListExpectedError, @@ -1441,7 +1446,10 @@ export class InMemoryDriver implements IDataDriver { // again. It must run before `normalizeFilterCondition` // and not inside it: the translator recurses per key and would therefore // refuse or not refuse depending on where in the tree it gave up. - assertFilterConditionShape(filters, 'filter'); + // [#21066] With the object's declarations, so a scalar comparison on a + // declared JSON-stored field is refused there too, before mingo answers + // it per element. + assertFilterConditionShape(filters, 'filter', DRIVER_FILTER_CAPABILITIES, this.filterFieldDeclarations(object)); // Translate non-standard operators ($contains, $notContains, etc.) to Mingo-compatible format return this.normalizeFilterCondition(filters, object); } @@ -2386,6 +2394,41 @@ export class InMemoryDriver implements IDataDriver { return STRUCTURED_JSON_TYPES.has(shape.type) || isMultiValueField(shape); } + /** + * [#21066] What the shape gate is told about `object`'s declared fields + * (`FilterFieldDeclarations` in `filter-refusal.ts`): which of them are + * JSON-stored, and where the withheld half of a refusal is written. + * + * The population is {@link isJsonStoredField}'s — the one `$contains` forks + * on — so the fields on which `$contains` asks membership are exactly the + * fields on which the equality and ordering family is refused: one declared + * set, two halves of one contract (the spec's `$contains` docblock names + * `$contains` as the operator left working where the family is refused). It + * matches `driver-sql`'s JSON-column registry less that registry's + * driver-internal aliases, as {@link isJsonStoredField} records. + * + * **A field with no recorded declaration is not judged**: an object never + * passed through {@link syncSchema} keeps answering every operator as it + * always has, as `SqlDriver.isJsonColumn` answers `false` for a table it was + * never told about. The refusal fires only where the storage shape is KNOWN. + * + * The diagnostic goes to this driver's logger at `warn` — the level + * `driver-sql` writes its withheld filter diagnostics at — so the refusal's + * "the full diagnostic is in the server log" is true here too. + * + * @internal Not private only because the analytics face + * (`memory-analytics.ts`) is another class of this package and must judge + * its `where` by the same declarations, for the reason + * {@link filterContainsTest} is reachable from it. Not a consumer contract: + * `FilterFieldDeclarations` is not exported from the package root. + */ + filterFieldDeclarations(object: string | undefined): FilterFieldDeclarations { + return { + isJsonStoredField: (field) => this.isJsonStoredField(object, field), + reportWithheld: (diagnostic) => this.logger.warn(withheldFilterLogLine(diagnostic)), + }; + } + /** * [#20874] The one un-negated test `$contains` asks of `field` on `object` — * behind every spelling of the operator on this package: the `$`-spelling and diff --git a/packages/drivers/driver-memory/src/memory-matcher-scalar-comparand-array-value.test.ts b/packages/drivers/driver-memory/src/memory-matcher-scalar-comparand-array-value.test.ts index 5a25d97c490..8ce1fad477e 100644 --- a/packages/drivers/driver-memory/src/memory-matcher-scalar-comparand-array-value.test.ts +++ b/packages/drivers/driver-memory/src/memory-matcher-scalar-comparand-array-value.test.ts @@ -49,6 +49,13 @@ * same composition mingo performs, which is why the two faces agree here by * construction rather than by coincidence. * + * ⚠️ [#21066] The column here is DECLARED `text` — a scalar column holding + * arrays — and that is the only population this per-element reading still + * covers. On a column DECLARED JSON-stored (a `multiple: true` field, `tags`, + * `json`, …) the equality and ordering family is REFUSED `INVALID_FILTER` / + * 400 by the shape gate, as the SQL family refuses it, and the membership + * question is `$contains`'s: `memory-21066-json-column-family-refusal.test.ts`. + * * ⚠️ One level only, measured rather than reasoned: mingo does not descend into * a NESTED array, so neither does this face — `[['a']]` does not match `'a'` on * either face, and that row is in the fixture to hold it.