diff --git a/.changeset/20822-having-contains-membership.md b/.changeset/20822-having-contains-membership.md new file mode 100644 index 00000000000..5ac58171ed9 --- /dev/null +++ b/.changeset/20822-having-contains-membership.md @@ -0,0 +1,18 @@ +--- +'@objectstack/objectql': patch +'@objectstack/driver-mongodb': patch +'@objectstack/formula': patch +'@objectstack/spec': patch +--- + +fix(objectql,driver-mongodb,formula): the `having` and per-aggregation evaluator compiles the whole-day comparison it is handed, and `$contains` asks membership on a JSON-stored field in `MongoDBDriver` and in `matchesFilterCondition` (ADR-0053 D-D1 items 5 and 9; the `FILTER_OPERATORS` `$contains` contract, #20822) + +Clause-②: no + +- **`@objectstack/objectql`: the aggregate evaluator's own whole-day copy is deleted.** The walker behind `having` and `aggregations[i].filter` no longer widens a bare `YYYY-MM-DD` `$lte`, or a `$between` maximum, on a `datetime` column to the whole day, and no longer drops the bound on `9999-12-31`. Through `engine.aggregate` nothing changes: the engine's seam lowers both positions with the shared `lowerFilterCondition` (`@objectstack/spec/data`) before the walker runs, by the object's declared `datetime` fields for the per-aggregation `filter` and by the aggregated column's type for `having`. A caller that passes no seam gets the comparison it wrote: `applyInMemoryAggregation(rows, ast, tz, fields)` called directly now counts `{ at: { $lte: '2026-02-01' } }` against that day's midnight. To keep the seam's reading on a direct call, lower each `filter` first with `lowerFilterCondition(filter, { isDatetimeColumn })`. +- **`@objectstack/driver-mongodb`: `$contains` / `$notContains` ask membership on a declared JSON-stored field.** On a field `syncSchema` recorded as `multiple: true`, a multi-option type (`tags`, `multiselect`, `checkboxes`) or a JSON type, `translateFilter` (every verb, and the aggregation `$match`) now emits an array-only `$elemMatch` over the members the comparand names, with the candidate rule the SQL dialects bind (`jsonMembershipCandidates`, `@objectstack/core`): `'1'` names the string `'1'` or the number `1`, `'true'` the string or `true`. It used to emit a `$regex`, which MongoDB applies to each element, so `{ owners: { $contains: 'u1' } }` matched a stored `['u10']` and `{ tags: { $contains: 'red' } }` a stored `['redwood']`. `$notContains` is the exact complement, and still admits a row with no value. A scalar column, and a field whose declaration the driver does not hold (an object never synced, a standalone `translateFilter` call), keep the substring `$regex`. +- **`@objectstack/formula`: `matchesFilterCondition` asks membership of a JSON-stored column.** When the caller supplies `options.fields` and it names the column, the declaration decides: membership on a JSON-stored column, substring on any other. Otherwise the stored value decides: an array asks membership, anything else substring. A stored array used to fail `$contains` and pass `$notContains` whatever it held. + - **The RLS write check, which evaluates a policy with this function, moves with it.** Under a `check` such as `record.tags.contains('x')` on a multi-valued field, a write whose post-image holds `['x']` (a row the same policy's read shows) is now admitted; it was refused `PERMISSION_DENIED` / 403. `['xy']` stays refused, and the read hides it. + - A scalar written to a declared multi-valued field is judged as written, before the write door wraps it in a list. So `tags: 'xy'`, which the check used to admit while the read hides the stored `['xy']`, is now refused 403. And `tags: 'x'` is now refused 403 too, although the read shows the stored `['x']`. Send the list, `tags: ['x']`. +- **`@objectstack/spec`: docblock only, in the shipped `src/data/filter.zod.ts`.** The three pointers to the deleted `SqlDriver.calendarDayUpperBoundRewrite` / `calendarDayBetweenRewrite` now name the shared `lowerFilterCondition` at the seams, and the `FILTER_OPERATORS` `$contains` implementation-status list gains `driver-mongodb` and `formula`. No schema, type or export changes. +- No exported name changes. diff --git a/content/docs/protocol/objectql/query-syntax.mdx b/content/docs/protocol/objectql/query-syntax.mdx index 30b64e7a6ec..a51ff829eba 100644 --- a/content/docs/protocol/objectql/query-syntax.mdx +++ b/content/docs/protocol/objectql/query-syntax.mdx @@ -571,14 +571,17 @@ means the start of that day (midnight UTC); as an upper bound (`$lte`, or the max of a `$between`) it covers the **whole** day. On a `date` column that is plain comparison, so the `$between` above includes Dec 31. On a `datetime` column it is the shared filter lowering (`lowerFilterCondition` from -`@objectstack/spec/data`, ADR-0053 D-D1 as amended): the engine's `where` seam -and the RLS compile seam rewrite a bare-day upper bound on a `datetime` column -half-open (`$lt` the next day) before any driver sees the filter, and a seam -that cannot read the declared types applies that rewrite to every column. A -filter handed directly to `SqlDriver` or a driver built on it -(`SqliteWasmDriver`, `TursoDriver`), past both seams, is compared as written: a -bare-day `$lte` on a `datetime` column compares against that day's midnight. A -full ISO timestamp keeps exact-instant semantics on every operator. +`@objectstack/spec/data`, ADR-0053 D-D1 as amended): the engine's filter seams +(`where`, and on `aggregate` the per-aggregation `filter` and `having`) and the +RLS compile seam rewrite a bare-day upper bound on a `datetime` column +half-open (`$lt` the next day) before any driver or in-memory evaluator sees the +filter, and a seam that cannot read the declared types applies that rewrite to +every column. A filter handed directly to a driver past those seams — +`SqlDriver` or a driver built on it (`SqliteWasmDriver`, `TursoDriver`), +`InMemoryDriver` or `MongoDBDriver` — or to `applyInMemoryAggregation` is +compared as written: a bare-day `$lte` on a `datetime` column compares against +that day's midnight. A full ISO timestamp keeps exact-instant semantics on every +operator. ### Null Checks diff --git a/packages/drivers/driver-mongodb/src/mongodb-contains-json-membership.test.ts b/packages/drivers/driver-mongodb/src/mongodb-contains-json-membership.test.ts new file mode 100644 index 00000000000..9dc3d2536ed --- /dev/null +++ b/packages/drivers/driver-mongodb/src/mongodb-contains-json-membership.test.ts @@ -0,0 +1,201 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * `$contains` / `$notContains` on `translateFilter`, by the field's DECLARED + * value shape — the contract `FILTER_OPERATORS`' `$contains` docblock + * (`@objectstack/spec`) states: MEMBERSHIP on a `multiple: true` field or a + * JSON-stored type, the SUBSTRING test on a scalar string column, as + * `driver-sql` answers on every dialect and `driver-memory` on every face. + * + * | declared shape | `$contains: v` | `$notContains: v` | + * |---|---|---| + * | JSON-stored | `{ f: { $elemMatch: { $in: members, $not: { $type: 'array' } } } }` | the same test under `$not` | + * | anything else, or none held | `{ f: { $regex: escaped(v) } }` | `{ f: { $not: { $regex } } }` | + * + * Before, every field took the `$regex`, and MongoDB applies a `$regex` to each + * element of an array value: `'u1'` matched a stored `['u10']`, the cell this + * file pins first, beside a scalar text column that still answers substring. + * + * Pinned twice, as `mongodb-20444-empty-operator.test.ts` pins `$empty`: the + * emitted DOCUMENTS, and the rows they select under a server-free reading of + * the MongoDB semantics those documents use. A live `mongod` suite runs the + * same cases when the opt-in server is available. + */ + +import { afterAll, beforeAll, describe, expect, it } from 'vitest'; +import type { MongoMemoryServer } from 'mongodb-memory-server'; +import type { FilterCondition } from '@objectstack/spec/data'; +import { translateFilter, type ValueShapeResolver } from './mongodb-filter.js'; +import { buildAggregationPipeline } from './mongodb-aggregation.js'; +import { MongoDBDriver } from './mongodb-driver.js'; +import { createTestMongod } from './test-mongod.js'; + +const OBJECT = 'os_contains_membership'; + +const FIELDS: Record = { + title: { type: 'text' }, + owners: { type: 'lookup', reference: OBJECT, multiple: true }, + tags: { type: 'tags' }, + meta: { type: 'json' }, +}; +const SHAPES: ValueShapeResolver = (field) => FIELDS[field]; + +const ROWS: Array> = [ + { id: 'r1', title: 'u10', owners: ['u10'], tags: ['redwood'], meta: [10, 21] }, + { id: 'r2', title: 'u1', owners: ['u1', 'u2'], tags: ['red'], meta: [1, 2] }, + { id: 'r3', title: 'x', owners: [], tags: [], meta: [true, null] }, + { id: 'r4', title: null, owners: null, tags: null, meta: null }, + { id: 'r5' }, + { id: 'r6', title: 'y', owners: [['u1']], tags: ['RED'], meta: { k: 'u1' } }, + { id: 'r7', title: 'z', owners: 'u1', tags: ['blue'], meta: [1.5] }, +]; + +const CASES: Array<{ name: string; where: FilterCondition; expected: string[] }> = [ + { name: "'u1' against a stored ['u10'] is not a member", where: { owners: { $contains: 'u1' } }, expected: ['r2'] }, + { name: 'the exact complement, the rows with no value included', where: { owners: { $notContains: 'u1' } }, expected: ['r1', 'r3', 'r4', 'r5', 'r6', 'r7'] }, + { name: 'the scalar control: a text column keeps the substring test', where: { title: { $contains: 'u1' } }, expected: ['r1', 'r2'] }, + { name: 'the scalar control, negated', where: { title: { $notContains: 'u1' } }, expected: ['r3', 'r4', 'r5', 'r6', 'r7'] }, + { name: "a tags field: 'red' is not a member of ['redwood'], and the test is case-exact", where: { tags: { $contains: 'red' } }, expected: ['r2'] }, + { name: "a json field: '1' names the number 1, and not 10 or 21", where: { meta: { $contains: '1' } }, expected: ['r2'] }, + { name: "a json field: '1.50' names the number 1.5", where: { meta: { $contains: '1.50' } }, expected: ['r7'] }, + { name: "a json field: 'true' and 'null' name the literals", where: { $and: [{ meta: { $contains: 'true' } }, { meta: { $contains: 'null' } }] }, expected: ['r3'] }, + { name: 'an object root has no member', where: { meta: { $contains: 'u1' } }, expected: [] }, + { name: 'under $or beside a scalar column', where: { $or: [{ owners: { $contains: 'u2' } }, { title: { $contains: 'x' } }] }, expected: ['r2', 'r3'] }, +]; + +// ── A server-free reading of the MongoDB vocabulary these documents use ───── + +/** `$elemMatch` with operator queries: some ELEMENT of an array value satisfies every one. */ +function elemMatches(value: unknown, query: Record): boolean { + if (!Array.isArray(value)) return false; + return value.some((element) => Object.entries(query).every(([op, arg]) => { + switch (op) { + // A scalar element equals a member of its own BSON type only: `'1'` never equals `1`. + case '$in': return (arg as unknown[]).some((member) => element === member); + case '$not': { + const inner = arg as Record; + if (Object.keys(inner).length !== 1 || inner.$type !== 'array') throw new Error(`unmodelled $not ${JSON.stringify(inner)}`); + return !Array.isArray(element); + } + default: throw new Error(`unmodelled $elemMatch operator ${op}`); + } + })); +} + +/** `$regex` on a field: a string value, or ANY string element of an array value (one level). */ +function regexMatches(value: unknown, pattern: string): boolean { + const re = new RegExp(pattern); + if (typeof value === 'string') return re.test(value); + if (Array.isArray(value)) return value.some((element) => typeof element === 'string' && re.test(element)); + return false; +} + +function matchOps(value: unknown, ops: Record): boolean { + for (const [op, arg] of Object.entries(ops)) { + switch (op) { + case '$elemMatch': if (!elemMatches(value, arg as Record)) return false; break; + case '$regex': if (!regexMatches(value, arg as string)) return false; break; + // MongoDB's field `$not` also matches a document whose field is missing. + case '$not': if (matchOps(value, arg as Record)) return false; break; + default: throw new Error(`unmodelled field operator ${op}`); + } + } + return true; +} + +/** MongoDB implicit equality: a scalar matches itself, and an array value matches by element. */ +function equals(value: unknown, comparand: unknown): boolean { + if (Array.isArray(value)) return value.some((element) => element === comparand); + return value === comparand; +} + +function matchDoc(row: Record, doc: Record): boolean { + for (const [key, value] of Object.entries(doc)) { + if (key === '$and') { if (!(value as Array>).every((d) => matchDoc(row, d))) return false; continue; } + if (key === '$or') { if (!(value as Array>).some((d) => matchDoc(row, d))) return false; continue; } + if (key.startsWith('$')) throw new Error(`unmodelled document operator ${key}`); + const cond = value as Record; + const isOps = cond !== null && typeof cond === 'object' && !Array.isArray(cond) + && Object.keys(cond).every((k) => k.startsWith('$')); + if (isOps ? !matchOps(row[key], cond) : !equals(row[key], value)) return false; + } + return true; +} + +const select = (doc: Record) => ROWS.filter((r) => matchDoc(r, doc)).map((r) => String(r.id)).sort(); + +describe('translateFilter — $contains asks membership on a declared JSON-stored field', () => { + it('emits the membership test on a JSON-stored field and the substring pattern on a scalar column', () => { + const membership = (members: unknown[]) => ({ $elemMatch: { $in: members, $not: { $type: 'array' } } }); + expect(translateFilter({ owners: { $contains: 'u1' } }, undefined, SHAPES)).toEqual({ owners: membership(['u1']) }); + expect(translateFilter({ owners: { $notContains: 'u1' } }, undefined, SHAPES)).toEqual({ owners: { $not: membership(['u1']) } }); + expect(translateFilter({ meta: { $contains: '1' } }, undefined, SHAPES)).toEqual({ meta: membership(['1', 1]) }); + expect(translateFilter({ meta: { $contains: '1.50' } }, undefined, SHAPES)).toEqual({ meta: membership(['1.50', 1.5]) }); + expect(translateFilter({ meta: { $contains: 'true' } }, undefined, SHAPES)).toEqual({ meta: membership(['true', true]) }); + expect(translateFilter({ meta: { $contains: 'null' } }, undefined, SHAPES)).toEqual({ meta: membership(['null', null]) }); + expect(translateFilter({ title: { $contains: 'u1' } }, undefined, SHAPES)).toEqual({ title: { $regex: 'u1' } }); + expect(translateFilter({ title: { $notContains: 'u1' } }, undefined, SHAPES)).toEqual({ title: { $not: { $regex: 'u1' } } }); + }); + + it('a member is matched literally: no regex metacharacter reaches the membership test', () => { + expect(translateFilter({ tags: { $contains: 'a.b+c' } }, undefined, SHAPES)) + .toEqual({ tags: { $elemMatch: { $in: ['a.b+c'], $not: { $type: 'array' } } } }); + }); + + for (const c of CASES) { + it(`${c.name}: ${JSON.stringify(c.where)} selects ${JSON.stringify(c.expected)}`, () => { + const doc = translateFilter(c.where, undefined, SHAPES) as Record; + expect(select(doc), JSON.stringify(doc)).toEqual(c.expected); + }); + } + + it('beside $startsWith on the same field, both constraints survive — $elemMatch contests no key', () => { + expect(translateFilter({ tags: { $contains: 'red', $startsWith: 'r' } }, undefined, SHAPES)) + .toEqual({ tags: { $elemMatch: { $in: ['red'], $not: { $type: 'array' } }, $regex: '^r' } }); + }); + + it('the aggregate $match translates it the way find() does', () => { + const pipeline = buildAggregationPipeline({ + where: { owners: { $contains: 'u1' } }, + aggregations: [{ function: 'count', alias: 'n' }] as never, + valueShape: SHAPES, + }); + expect(pipeline[0]).toEqual({ $match: { owners: { $elemMatch: { $in: ['u1'], $not: { $type: 'array' } } } } }); + }); + + it('with no declaration held, every field keeps the substring reading, as driver-sql does for a table it was never told about', () => { + const doc = translateFilter({ owners: { $contains: 'u1' } }) as Record; + expect(doc).toEqual({ owners: { $regex: 'u1' } }); + // The per-element substring the declaration exists to replace: 'u1' answers + // ['u10']. (Whether a `$regex` reaches into the NESTED array of r6 is left + // unmodelled here; the membership test above excludes it by construction.) + expect(select(doc)).toEqual(expect.arrayContaining(['r1', 'r2', 'r7'])); + expect(translateFilter({ nope: { $contains: 'u1' } }, undefined, SHAPES)).toEqual({ nope: { $regex: 'u1' } }); + }); +}); + +const sharedMongod: MongoMemoryServer | undefined = await createTestMongod('$contains membership'); + +describe.skipIf(!sharedMongod)('MongoDBDriver — $contains membership against a live mongod', () => { + const mongod = sharedMongod as MongoMemoryServer; + let driver: MongoDBDriver; + + beforeAll(async () => { + driver = new MongoDBDriver({ url: mongod.getUri(), database: OBJECT }); + await driver.connect(); + await driver.syncSchema(OBJECT, { name: OBJECT, fields: FIELDS }); + for (const row of ROWS) await driver.create(OBJECT, { ...row }); + }, 90_000); + + afterAll(async () => { + if (driver) await driver.disconnect(); + if (sharedMongod) await sharedMongod.stop(); + }); + + for (const c of CASES) { + it(`${c.name}: selects ${JSON.stringify(c.expected)}`, async () => { + const rows = (await driver.find(OBJECT, { where: c.where })) as Array>; + expect(rows.map((r) => String(r.id)).sort()).toEqual(c.expected); + }); + } +}); diff --git a/packages/drivers/driver-mongodb/src/mongodb-driver.ts b/packages/drivers/driver-mongodb/src/mongodb-driver.ts index fd15ec34c80..a9b1a091f25 100644 --- a/packages/drivers/driver-mongodb/src/mongodb-driver.ts +++ b/packages/drivers/driver-mongodb/src/mongodb-driver.ts @@ -202,7 +202,9 @@ export class MongoDBDriver implements IDataDriver { * {@link syncSchema} beside {@link temporalFields} and for the same reason: * the `$empty` operator is answered by the field's DECLARED row, and a field * this map does not hold is refused rather than given a row guessed from the - * data. + * data. The same shape decides whether `$contains` asks membership (a + * JSON-stored field) or substring (anything else, a field it does not hold + * included). */ private valueShapes = new Map>(); @@ -694,7 +696,8 @@ export class MongoDBDriver implements IDataDriver { // Learn which fields are temporal BEFORE any write can land, so the write // path and the filter path share one storage convention (#4047). this.temporalFields.set(object, indexTemporalFields(objectDef.fields)); - // [#20444] …and each field's declared value shape, for `$empty`. + // [#20444] …and each field's declared value shape, for `$empty` and for + // the question `$contains` asks. this.valueShapes.set(object, indexValueShapes(objectDef.fields)); await syncCollectionSchema(this.db, object, objectDef); } @@ -833,8 +836,10 @@ export class MongoDBDriver implements IDataDriver { /** * [#20444] The declared-value-shape lookup for one object, handed to - * {@link translateFilter} so `$empty` translates the field's declared row. - * `undefined` for an undeclared object — `$empty` is then refused. + * {@link translateFilter} so `$empty` translates the field's declared row, + * and so `$contains` / `$notContains` ask MEMBERSHIP on a declared + * JSON-stored field. `undefined` for an undeclared object — `$empty` is then + * refused, and `$contains` keeps the substring reading. */ private valueShapeFor(object: string): ValueShapeResolver | undefined { const shapes = this.valueShapes.get(object); diff --git a/packages/drivers/driver-mongodb/src/mongodb-filter.ts b/packages/drivers/driver-mongodb/src/mongodb-filter.ts index de35f1ee395..0db26f8d9f8 100644 --- a/packages/drivers/driver-mongodb/src/mongodb-filter.ts +++ b/packages/drivers/driver-mongodb/src/mongodb-filter.ts @@ -50,6 +50,14 @@ import { FILTER_OPERATORS } from '@objectstack/spec/data'; // [#20444] The `$empty` operator's ONE expansion — the field's declared row of // the ruled 「is empty」 table, asked of the spec per translation. import { expandEmptyOperator, type ValueShapeFieldDef } from '@objectstack/spec/data'; +// The JSON-stored population — the declared fields on which `$contains` asks +// MEMBERSHIP — from the spec's value-shape classes, the same two `driver-sql`'s +// JSON-column registry and `driver-memory`'s population are built from. +import { STRUCTURED_JSON_TYPES, isMultiValueField } from '@objectstack/spec/data'; +// The members a `$contains` comparand names on a JSON-stored field: the one +// candidate rule every SQL dialect binds (`@objectstack/core`), read here as +// JSON values instead of as JSON text. See {@link containsMembers}. +import { jsonMembershipCandidates } from '@objectstack/core'; import { coerceTemporalValue, type TemporalFieldKind, @@ -619,6 +627,69 @@ function undeclaredEmptyOperatorFieldError(field: string, path: string): Error { */ export type ValueShapeResolver = (field: string) => ValueShapeFieldDef | undefined; +/** + * Is a field with this DECLARED value shape JSON-stored — the population on + * which `$contains` / `$notContains` ask MEMBERSHIP rather than SUBSTRING? + * + * The contract is `FILTER_OPERATORS`' `$contains` docblock + * (`@objectstack/spec`): on a `multiple: true` field or a JSON-stored type, + * `$contains: v` asks whether `v` is a member of the stored array; on a scalar + * string column it stays the substring test. The question is selected by the + * DECLARATION, never by the row, so this reads the shape `MongoDBDriver.syncSchema` + * recorded: `STRUCTURED_JSON_TYPES`, or a multi-valued field + * (`isMultiValueField`, which covers `MULTI_OPTION_TYPES`) — the two halves + * `driver-sql`'s JSON-column registry and `driver-memory`'s population are built + * from. + * + * **No declaration ⇒ `false`.** A field whose declaration this translator was + * not handed — an object never synced, a field its schema does not name, a + * standalone {@link translateFilter} call with no {@link ValueShapeResolver} — + * keeps the substring reading, exactly as `SqlDriver.isJsonColumn` answers for + * a table it was never told about. + */ +function isJsonStoredShape(shape: ValueShapeFieldDef | undefined): boolean { + if (!shape) return false; + return STRUCTURED_JSON_TYPES.has(shape.type) || isMultiValueField(shape); +} + +/** + * The stored MEMBERS a `$contains` comparand names on a JSON-stored field. + * + * The comparand is a STRING by contract, so a member stored as a JSON number or + * boolean is named by its TEXT: `'1'` names the string `'1'` OR the number `1`, + * `'true'` the string OR `true`, `'null'` the string OR `null`, and `'1.50'` + * the number `1.5`. That is `@objectstack/core`'s `jsonMembershipCandidates`, + * the one rule every SQL dialect binds, which answers each candidate as JSON + * TEXT; parsing it gives the value MongoDB compares a stored element against. + * One rule, read in two encodings, so this driver and the SQL family cannot + * disagree about WHICH members a comparand names. + */ +function containsMembers(value: unknown): unknown[] { + return jsonMembershipCandidates(value).map((candidate) => JSON.parse(candidate) as unknown); +} + +/** + * The un-negated MEMBERSHIP test `$contains` lowers to on a JSON-stored field: + * some element of the stored array IS one of the members the comparand names. + * + * `$elemMatch` is array-only by construction, so a stored scalar, an object or + * `null` has no member — the answer `driver-sql` gives on every dialect (its + * constructs are array-only too). The `$not: { $type: 'array' }` clause keeps a + * NESTED array out: `$in` reaches through an element that is itself an array, + * so `[['u1']]` would otherwise answer `'u1'`, where SQLite compares the + * element's JSON text `["u1"]` and does not. `driver-memory` emits the same + * document for the same question (`InMemoryDriver.filterContainsTest`), and + * mingo is that package's MongoDB-compatible evaluator. + * + * Before this test existed, `$contains` wrote `$regex` here on every field, and + * MongoDB applies a `$regex` to each element of an array value: `'u1'` matched + * a stored `['u10']` and `'red'` a stored `['redwood']`, substrings across the + * member boundary the contract rules out. + */ +function containsMembershipTest(value: unknown): Record { + return { $elemMatch: { $in: containsMembers(value), $not: { $type: 'array' } } }; +} + /** * [#20444] Translate `{ field: { $empty: true | false } }` by the field's * DECLARED row of the ruled 「is empty」 table — ruling B on #20311 (record @@ -847,7 +918,9 @@ export function translateFilter( temporalKind?: TemporalFieldKindResolver, // [#20444] The declared value shape of each field, for `$empty`'s declared // row. Omitted, `$empty` is refused — the pure shape translation has no - // declaration to read a row from. + // declaration to read a row from. It also selects the question `$contains` / + // `$notContains` ask: membership on a declared JSON-stored field; omitted, + // every field keeps the substring reading. valueShape?: ValueShapeResolver, ): Filter { if (!where) return {}; @@ -957,7 +1030,9 @@ function translateCondition( objValue = rest; } if (hasOps) { - const translated = translateFieldOperators(objValue, temporalKind?.(key), key, `${path}.${key}`); + const translated = translateFieldOperators( + objValue, temporalKind?.(key), key, `${path}.${key}`, valueShape?.(key), + ); // [#13524] Lowered writes whose MongoDB key was already taken by a // sibling operator on the same field. Merging one would drop a // constraint silently, so each becomes its own `$and` branch — see @@ -1062,6 +1137,11 @@ interface LoweredWrite { * written by `$notContains` and by nothing else, so it is covered here by * construction rather than curatively. * + * On a declared JSON-stored field `$contains` writes `$elemMatch` instead of + * `$regex` (its membership test), a key no other operator writes, so it + * contests nothing; `$startsWith` / `$endsWith` / `$icontains` still meet on + * `$regex` there, by the rule below. + * * ## The rule, and why it is this one * * Free key → merge inline (the overwhelmingly common case). Taken key → the @@ -1147,6 +1227,9 @@ function translateFieldOperators( // refused, the way `driver-sql` and `driver-memory` do. field = '', path = 'filter', + // The field's DECLARED value shape, when the caller holds it: it selects the + // question `$contains` / `$notContains` ask ({@link isJsonStoredShape}). + shape?: ValueShapeFieldDef, ): Record { const store = (v: unknown) => coerceTemporalValue(v, kind); /** @@ -1229,16 +1312,25 @@ function translateFieldOperators( // dropping the flag changes which CASES match, never which characters are // metacharacters. The deliberate case-insensitive spelling is // `$icontains` below — one operator, one answer, per #5374. + // + // On a declared JSON-stored field the question is MEMBERSHIP instead + // (`FILTER_OPERATORS`' `$contains` docblock): {@link containsMembershipTest}. + // A `$regex` there matched per element, so `'u1'` answered a stored + // `['u10']`. case '$contains': - put('$regex', escapeRegex(String(value))); + if (isJsonStoredShape(shape)) put('$elemMatch', containsMembershipTest(value).$elemMatch); + else put('$regex', escapeRegex(String(value))); break; case '$notContains': // The negated twin needs the same treatment in this ONE place: the // pattern under `$not` is the same predicate, so a flag left here would // have excluded rows the positive form includes — the negation widening - // rather than mirroring. - put('$not', { $regex: escapeRegex(String(value)) }); + // rather than mirroring. On a declared JSON-stored field it is the + // exact complement of the membership test: `$not` over `$elemMatch` + // also admits a row whose field is null, missing or not an array, the + // rows with no member (`driver-sql`'s `col IS NULL OR NOT (…)`). + put('$not', isJsonStoredShape(shape) ? containsMembershipTest(value) : { $regex: escapeRegex(String(value)) }); break; // [#13524] These four all write `$regex`, so before the assembly below diff --git a/packages/formula/src/matches-filter-contains-membership.test.ts b/packages/formula/src/matches-filter-contains-membership.test.ts new file mode 100644 index 00000000000..b511ad95f01 --- /dev/null +++ b/packages/formula/src/matches-filter-contains-membership.test.ts @@ -0,0 +1,120 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * `$contains` / `$notContains` on `matchesFilterCondition` ask the question + * `FILTER_OPERATORS`' `$contains` docblock (`@objectstack/spec`) gives the + * column: MEMBERSHIP on a JSON-stored column, SUBSTRING on a scalar one. + * + * Which question is asked: + * + * | the caller supplied `options.fields`, and it names the column | asked | + * |---|---| + * | yes, declared JSON-stored (`STRUCTURED_JSON_TYPES`, or multi-valued) | membership | + * | yes, any other declared type | substring | + * | no (no map, or a column the map does not name) | by the stored value: an array asks membership, anything else substring | + * + * The declaration wins where it exists (contract first); where it does not, the + * stored value decides, the reading this face already gives `$empty` because it + * judges a record rather than a declaration. + * + * Before, the arm answered the substring test alone, so a stored array never + * matched `$contains` and always matched `$notContains`. The cell first pinned + * is the one the contract names: `'u1'` against a stored `['u10']` (not a + * member), beside a scalar text control (a substring). + */ + +import { describe, expect, it } from 'vitest'; +import type { FilterCondition } from '@objectstack/spec/data'; +import { matchesFilterCondition } from './matches-filter'; + +const DECLARED = { + fields: { + owners: { type: 'lookup', multiple: true }, + tags: { type: 'tags' }, + meta: { type: 'json' }, + title: { type: 'text' }, + }, +}; + +const holds = (record: Record, filter: Record, options?: typeof DECLARED) => + matchesFilterCondition(record, filter as FilterCondition, options); + +describe('$contains asks membership of a stored array — with no declaration, by the stored value', () => { + it("'u1' is not a member of a stored ['u10']; it is a member of ['u1', 'u2']", () => { + expect(holds({ owners: ['u10'] }, { owners: { $contains: 'u1' } })).toBe(false); + expect(holds({ owners: ['u1', 'u2'] }, { owners: { $contains: 'u1' } })).toBe(true); + }); + + it("the scalar control: a stored string keeps the substring test, so 'u1' is in 'u10'", () => { + expect(holds({ title: 'u10' }, { title: { $contains: 'u1' } })).toBe(true); + expect(holds({ title: 'x' }, { title: { $contains: 'u1' } })).toBe(false); + }); + + it('$notContains is the exact complement on both readings, and a value-less column satisfies it', () => { + expect(holds({ owners: ['u10'] }, { owners: { $notContains: 'u1' } })).toBe(true); + expect(holds({ owners: ['u1'] }, { owners: { $notContains: 'u1' } })).toBe(false); + expect(holds({ title: 'u10' }, { title: { $notContains: 'u1' } })).toBe(false); + expect(holds({ owners: null }, { owners: { $notContains: 'u1' } })).toBe(true); + expect(holds({}, { owners: { $notContains: 'u1' } })).toBe(true); + expect(holds({ owners: [] }, { owners: { $notContains: 'u1' } })).toBe(true); + }); + + it('the member is matched exactly and case-sensitively, never as a substring of an element', () => { + expect(holds({ tags: ['redwood'] }, { tags: { $contains: 'red' } })).toBe(false); + expect(holds({ tags: ['RED'] }, { tags: { $contains: 'red' } })).toBe(false); + expect(holds({ tags: ['red'] }, { tags: { $contains: 'red' } })).toBe(true); + }); + + it('a member stored as a number, boolean or null is named by its text — the SQL candidate set', () => { + expect(holds({ meta: [1, 2] }, { meta: { $contains: '1' } })).toBe(true); + expect(holds({ meta: [10, 21] }, { meta: { $contains: '1' } })).toBe(false); + expect(holds({ meta: [1.5] }, { meta: { $contains: '1.50' } })).toBe(true); + expect(holds({ meta: ['1.5'] }, { meta: { $contains: '1.50' } })).toBe(false); + expect(holds({ meta: [true] }, { meta: { $contains: 'true' } })).toBe(true); + expect(holds({ meta: [null] }, { meta: { $contains: 'null' } })).toBe(true); + // Not a JSON number, so not the number 16: the text '0x10' names only itself. + expect(holds({ meta: [16] }, { meta: { $contains: '0x10' } })).toBe(false); + }); + + it('a nested array or an object element is not a member', () => { + expect(holds({ owners: [['u1']] }, { owners: { $contains: 'u1' } })).toBe(false); + expect(holds({ owners: [{ id: 'u1' }] }, { owners: { $contains: 'u1' } })).toBe(false); + }); + + it('a comparand that is not a string matches nothing, and its negation everything, as before', () => { + expect(holds({ meta: [1] }, { meta: { $contains: 1 } })).toBe(false); + expect(holds({ meta: [1] }, { meta: { $notContains: 1 } })).toBe(true); + }); +}); + +describe('$contains with the declared columns supplied — the declaration decides', () => { + it('a declared multi-valued column asks membership; a scalar stored there has no member', () => { + expect(holds({ owners: ['u10'] }, { owners: { $contains: 'u1' } }, DECLARED)).toBe(false); + expect(holds({ owners: ['u1'] }, { owners: { $contains: 'u1' } }, DECLARED)).toBe(true); + expect(holds({ owners: 'u1' }, { owners: { $contains: 'u1' } }, DECLARED)).toBe(false); + expect(holds({ owners: 'u1' }, { owners: { $notContains: 'u1' } }, DECLARED)).toBe(true); + }); + + it('a declared JSON type and a multi-option type ask membership too', () => { + expect(holds({ meta: [1, 2] }, { meta: { $contains: '1' } }, DECLARED)).toBe(true); + expect(holds({ tags: ['redwood'] }, { tags: { $contains: 'red' } }, DECLARED)).toBe(false); + }); + + it('a declared scalar column asks substring, so an array stored there matches nothing', () => { + expect(holds({ title: 'u10' }, { title: { $contains: 'u1' } }, DECLARED)).toBe(true); + expect(holds({ title: ['u1'] }, { title: { $contains: 'u1' } }, DECLARED)).toBe(false); + expect(holds({ title: ['u1'] }, { title: { $notContains: 'u1' } }, DECLARED)).toBe(true); + }); + + it('a column the map does not name is judged by its stored value', () => { + expect(holds({ extra: ['u1'] }, { extra: { $contains: 'u1' } }, DECLARED)).toBe(true); + expect(holds({ extra: 'u10' }, { extra: { $contains: 'u1' } }, DECLARED)).toBe(true); + }); + + it('the declaration reaches a column under $or, $and and $not', () => { + const record = { owners: ['u10'], title: 'x' }; + expect(holds(record, { $or: [{ owners: { $contains: 'u1' } }, { title: { $contains: 'y' } }] }, DECLARED)).toBe(false); + expect(holds(record, { $and: [{ owners: { $contains: 'u10' } }, { title: { $contains: 'x' } }] }, DECLARED)).toBe(true); + expect(holds(record, { $not: { owners: { $contains: 'u1' } } }, DECLARED)).toBe(true); + }); +}); diff --git a/packages/formula/src/matches-filter.ts b/packages/formula/src/matches-filter.ts index 708cde7678c..31f963a2576 100644 --- a/packages/formula/src/matches-filter.ts +++ b/packages/formula/src/matches-filter.ts @@ -123,6 +123,10 @@ import { matchesLikePattern } from '@objectstack/spec/data'; // [#20444] `$empty`'s value-level half — the spec's one definition of what a // stored value counts as empty for a face that reads no field declaration. import { isEmptyFilterValue } from '@objectstack/spec/data'; +// The JSON-stored population — the declared columns on which `$contains` asks +// MEMBERSHIP — from the spec's value-shape classes, the same two every typed +// face's population is built from. See {@link containsAsksMembership}. +import { STRUCTURED_JSON_TYPES, isMultiValueField } from '@objectstack/spec/data'; import { StandardErrorCode } from '@objectstack/spec/api'; /** @@ -314,6 +318,11 @@ export interface MatchesFilterOptions { * Omitted, the evaluator judges values only, as it always has: it has no * schema of its own, and a caller without one (an aggregated row, a probe * record) is not asked for one. + * + * A column named here also selects the question `$contains` / `$notContains` + * ask of it: MEMBERSHIP on a JSON-stored column, SUBSTRING on any other. A + * column it does not name is judged by its stored value instead + * ({@link containsAsksMembership}). */ readonly fields?: Readonly>; } @@ -340,7 +349,7 @@ export function matchesFilterCondition( const refusal = findCrossFieldClassRefusal(filter, options.fields); if (refusal) throw crossFieldClassError(refusal); } - return evalNode(record, filter as Record); + return evalNode(record, filter as Record, options?.fields); } /** @@ -596,26 +605,29 @@ function isEmptyFieldConstraint(spec: unknown): boolean { return Object.keys(spec as Record).length === 0; } -function evalNode(record: Record, node: Record): boolean { +/** The caller's declared columns ({@link MatchesFilterOptions.fields}), when it supplied them. */ +type DeclaredColumns = Readonly> | undefined; + +function evalNode(record: Record, node: Record, fields?: DeclaredColumns): boolean { // A node is the AND of all its entries. for (const [key, val] of Object.entries(node)) { if (key === '$and') { - if (!Array.isArray(val) || !val.every((c) => evalNode(record, c as Record))) return false; + if (!Array.isArray(val) || !val.every((c) => evalNode(record, c as Record, fields))) return false; } else if (key === '$or') { - if (!Array.isArray(val) || val.length === 0 || !val.some((c) => evalNode(record, c as Record))) return false; + if (!Array.isArray(val) || val.length === 0 || !val.some((c) => evalNode(record, c as Record, fields))) return false; } else if (key === '$not') { if (val == null || typeof val !== 'object') return false; - if (evalNode(record, val as Record)) return false; + if (evalNode(record, val as Record, fields)) return false; } else if (key.startsWith('$')) { return false; // unknown top-level operator → fail closed } else { - if (!evalField(record, key, val)) return false; + if (!evalField(record, key, val, fields)) return false; } } return true; } -function evalField(record: Record, field: string, spec: unknown): boolean { +function evalField(record: Record, field: string, spec: unknown, fields?: DeclaredColumns): boolean { const actual = getPath(record, field); // `{ field: null }` → IS NULL. if (spec === null) return actual == null; @@ -638,13 +650,21 @@ function evalField(record: Record, field: string, spec: unknown // `evalNode` on a subtree, and a total function must stay total — but it is a // floor, no longer this backend's ANSWER to the shape. if (keys.length === 0 || keys.some((k) => !k.startsWith('$'))) return false; + const declared = fields && Object.prototype.hasOwnProperty.call(fields, field) ? fields[field] : undefined; for (const op of keys) { - if (!evalOp(actual, op, ops[op], record)) return false; + if (!evalOp(actual, op, ops[op], record, declared)) return false; } return true; } -function evalOp(actual: unknown, op: string, raw: unknown, record: Record): boolean { +function evalOp( + actual: unknown, + op: string, + raw: unknown, + record: Record, + // The column's declaration, when the caller supplied one for it. + declared?: CrossFieldComparisonFieldMeta, +): boolean { // [#19886 stage 2d] A `{ $field }` comparison whose column holds a list or an // object ON THIS RECORD — either side. See {@link assertComparableReference}. if (isFieldReference(raw)) assertComparableReference(actual, op, raw, record); @@ -671,7 +691,20 @@ function evalOp(actual: unknown, op: string, raw: unknown, record: Record a >= b) && lteBound(actual, v[1]); - case '$contains': return typeof actual === 'string' && typeof v === 'string' && actual.includes(v); + /** + * MEMBERSHIP on a JSON-stored column, SUBSTRING on a scalar one — the two + * questions `FILTER_OPERATORS`' `$contains` docblock (`@objectstack/spec`) + * gives this one operator. Which one is asked is decided by + * {@link containsAsksMembership}: the column's declaration when the caller + * supplied it, the stored value's shape otherwise. + * + * Before, this arm answered the substring test alone, and a stored array is + * not a string, so a `check` written as `{ tags: { $contains: 'x' } }` over + * a multi-valued field denied every write, while the read the same policy + * scopes (the typed drivers, the read-scope SQL) showed the rows holding + * `'x'` by membership. + */ + case '$contains': return containsHolds(actual, v, declared); /** * [#6520] `$contains`' case-INSENSITIVE twin, folding ASCII case and nothing * else — `asciiCaseInsensitiveContains` is the spec's shared definition, the @@ -717,7 +750,9 @@ function evalOp(actual: unknown, op: string, raw: unknown, record: Record { + if (typeof element === 'string') return element === comparand; + if (typeof element === 'number') return Number.isFinite(number) && element === number; + if (typeof element === 'boolean' || element === null) return String(element) === comparand; + return false; + }); +} diff --git a/packages/objectql/src/engine-aggregate-temporal-storage-rule.test.ts b/packages/objectql/src/engine-aggregate-temporal-storage-rule.test.ts index 7459399364f..2f4e5785cdc 100644 --- a/packages/objectql/src/engine-aggregate-temporal-storage-rule.test.ts +++ b/packages/objectql/src/engine-aggregate-temporal-storage-rule.test.ts @@ -37,12 +37,19 @@ import { TEMPORAL_ROWS, TEMPORAL_TIME_CASES, TEMPORAL_TIME_ROWS, + lowerFilterCondition, type EngineAggregateOptions, } from '@objectstack/spec/data'; import { resolveFilterTokens } from '@objectstack/core'; import { ObjectQL } from './engine.js'; import { applyInMemoryAggregation } from './in-memory-aggregation.js'; -import { declaredFieldClasses, matchesAggregationFilter } from './having-filter.js'; +import { + aggregatedRowColumnClasses, + aggregatedRowColumnTypes, + applyHaving, + declaredFieldClasses, + matchesAggregationFilter, +} from './having-filter.js'; const OBJECT = 'ledger_order'; @@ -348,3 +355,49 @@ describe('[#20176] where no class is known, a comparand is compared as written', expect(matchesAggregationFilter(row, bound, 0, declaredFieldClasses(FIELDS))).toBe(true); }); }); + +// [ADR-0053 D-D1 items 5 and 9, as amended] Neither position's walker applies +// the whole-day upper bound of its own any more. The engine's seam lowers both +// before the walker sees them (`lowerFilterCondition`, by the object's declared +// `datetime` columns for the per-aggregation `filter` and by the aggregated +// column's type for `having`): the `engine.aggregate` rows above — the card's +// rows 3 and 4, the `having` rows on `min(datetime)` and the conformance kit — +// hold that half. A caller that evaluates rows without the seam, such as +// `applyInMemoryAggregation`, the package's public direct door, gets the +// comparison it wrote: a bare day as an upper bound is that day's midnight. +describe('[ADR-0053 D-D1 item 5] the walker compares a bare-day upper bound as written; the seam lowers it to the whole day', () => { + const classes = declaredFieldClasses(FIELDS); + const isDatetimeField = (column: string): boolean => + (FIELDS as Record)[column]?.type === 'datetime'; + const counted = (filter: Record): number => + ROWS.filter((row) => matchesAggregationFilter(row, filter as any, 0, classes)).length; + + const BOUNDS: ReadonlyArray, number, number]> = [ + ['a bare day as the $lte of a datetime', { opened_at: { $lte: '2026-02-01' } }, 2, 3], + ['a bare day as the $between max of a datetime', { opened_at: { $between: ['2026-01-01', '2026-02-01'] } }, 2, 3], + ]; + for (const [name, filter, asWritten, lowered] of BOUNDS) { + it(`per-aggregation filter, ${name}: ${asWritten} as written, ${lowered} once the seam lowers it`, () => { + expect(counted(filter), 'as written').toBe(asWritten); + expect(counted(lowerFilterCondition(filter, { isDatetimeColumn: isDatetimeField })), 'lowered').toBe(lowered); + }); + } + + it('applyInMemoryAggregation, called directly with the field map, counts the bound as written', () => { + const ast = { aggregations: [{ function: 'count' as const, alias: 'm', filter: { opened_at: { $lte: '2026-02-01' } } }] }; + expect(applyInMemoryAggregation(ROWS, ast, undefined, FIELDS)).toEqual([{ m: 2 }]); + }); + + it('having on min(datetime): c1 as written; c1 and c2 once lowered by the aggregated column type', () => { + const groupBy = ['customer_id']; + const groups = applyInMemoryAggregation(ROWS, { groupBy, aggregations: HAVING_AGGREGATIONS }, undefined, FIELDS); + const having = { first_opened: { $lte: '2026-02-01' } }; + const columnClasses = aggregatedRowColumnClasses(groupBy, HAVING_AGGREGATIONS, FIELDS); + const columnTypes = aggregatedRowColumnTypes(groupBy, HAVING_AGGREGATIONS, FIELDS); + const kept = (h: Record) => + applyHaving(groups, h as any, columnClasses).map((r: any) => r.customer_id).sort(); + expect(kept(having), 'as written').toEqual(['c1']); + expect(kept(lowerFilterCondition(having, { isDatetimeColumn: (c) => columnTypes.get(c) === 'datetime' })), 'lowered') + .toEqual(['c1', 'c2']); + }); +}); diff --git a/packages/objectql/src/engine.ts b/packages/objectql/src/engine.ts index b79ef8648a6..02fe46a5248 100644 --- a/packages/objectql/src/engine.ts +++ b/packages/objectql/src/engine.ts @@ -15625,29 +15625,39 @@ export class ObjectQL implements IObjectQLEngine { * DECLARED relation. * * The multi-value spelling is `$contains`, which is what the refusal itself - * prescribes and the one membership spelling every driver here answers: - * `driver-sql` (and `driver-sqlite-wasm` / `driver-turso`, which extend it) - * lowers it to `LIKE '%v%'` over the serialization, `driver-mongodb` to - * `$regex` over the array, and `driver-memory` to a mingo `$regex`, which - * matches per ELEMENT. No public filter surface is widened: `$contains` is + * prescribes and the one membership spelling every driver here answers. On a + * column the driver holds declared multi-valued it asks MEMBERSHIP, as + * `FILTER_OPERATORS`' `$contains` docblock (`@objectstack/spec`) declares for + * a JSON-stored column: `driver-sql` (and `driver-sqlite-wasm` and the local + * `driver-turso`, which extend it) compiles a per-dialect membership + * construct, `driver-memory` a mingo `$elemMatch`, and `driver-mongodb` the + * same `$elemMatch`. No public filter surface is widened: `$contains` is * already declared, and this is the only construction site that changes. * - * `$contains` is a SUBSTRING test, so on every one of those backends it - * answers a SUPERSET: with ids `acc_1` and `acc_10`, a row holding `acc_10` - * also matches a probe for `acc_1`. That is why the caller narrows the rows - * exactly through {@link ObjectQL.storedReferenceIncludes} — over-matching - * here would make `cascade` DELETE and `set_null` clear rows that never - * referenced this record, which is worse than the 400 being fixed. The - * pushdown's only job is to keep the probe from reading the whole table. + * Membership answers EXACTLY on a well-formed slot, but a backend that does + * not hold the declaration (a table it was never told about) answers the + * SUBSTRING reading instead, which is a SUPERSET: with ids `acc_1` and + * `acc_10`, a row holding `acc_10` also matches a probe for `acc_1`. That is + * why the caller still narrows the rows exactly through + * {@link ObjectQL.storedReferenceIncludes} — over-matching here would make + * `cascade` DELETE and `set_null` clear rows that never referenced this + * record, which is worse than the 400 being fixed. The pushdown's only job is + * to keep the probe from reading the whole table. + * + * ⚠️ Membership is array-only on every backend that answers it, so a slot + * holding an off-shape bare scalar (the write door never stores one, it wraps + * a scalar; out-of-band data can) is not matched by the pushdown, and the + * scalar arm of {@link ObjectQL.storedReferenceIncludes} never sees that row. * * The `$or` limb covers the other direction — a FALSE NEGATIVE, which on an * integrity guard is the fail-OPEN that #8895 ruled out. An id needing JSON - * escaping (a quote, a backslash) appears in a SQL backend's serialized text - * in its ESCAPED form, so a probe for the raw id would miss the row that - * holds it; a document/in-memory backend compares the element itself and - * needs the RAW form. Both are asked whenever they differ, and the exact - * narrowing discards whatever the extra limb over-matched. Identical for an - * ordinary id, which is every id this engine generates. + * escaping (a quote, a backslash) appears in the serialized text in its + * ESCAPED form, so a backend answering the substring reading over that text + * would miss the row for the raw id; a membership test, and a document or + * in-memory backend, compares the element itself and needs the RAW form. Both + * are asked whenever they differ, and the exact narrowing discards whatever + * the extra limb over-matched. Identical for an ordinary id, which is every + * id this engine generates. */ private referenceProbeFilter( fieldName: string, @@ -16210,8 +16220,9 @@ export class ObjectQL implements IObjectQLEngine { if (isMissingTableError(error, childName)) continue; throw error; } - // [#9362] The multi-value pushdown above is a SUPERSET, so the exact - // answer is taken here, on the rows themselves. Everything below — + // [#9362] The multi-value pushdown above can be a SUPERSET (the + // substring reading, on a backend without the declaration), so the + // exact answer is taken here, on the rows themselves. Everything below — // the `restrict` count in the 409 envelope, the `cascade` recursion, // the `set_null` write — reads `dependents`, so narrowing anywhere // later would leave one of them acting on a row that never referenced diff --git a/packages/objectql/src/having-filter.ts b/packages/objectql/src/having-filter.ts index 1a4df8715a8..e2213e2d224 100644 --- a/packages/objectql/src/having-filter.ts +++ b/packages/objectql/src/having-filter.ts @@ -103,9 +103,12 @@ // // [#20176] …and, where the column's CLASS is known, every temporal comparand is // compared by that column's storage rule — the rule the drivers apply to the -// same comparand in a `where`, `@objectstack/core`'s `temporalStorageForm`, -// with ADR-0053 D-D's whole-day reading of a bare-day upper bound on a -// `datetime` column. The class comes from the object's declaration for the +// same comparand in a `where`, `@objectstack/core`'s `temporalStorageForm`. +// [ADR-0053 D-D1 item 5, as amended] The whole-day reading of a bare-day upper +// bound on a `datetime` column is not applied here: the engine's seam lowers +// both positions before this walker sees them (`lowerFilterCondition`), and a +// caller that reaches it without a seam gets the comparison it wrote. The class +// comes from the object's declaration for the // per-aggregation `filter` (`declaredFieldClasses`) and from the query for // `having` (`aggregatedRowColumnClasses`, #20127's rule). See // {@link checkCondition}. Before, both positions compared a temporal comparand @@ -173,11 +176,8 @@ import { utcInstantMs } from '@objectstack/spec/data'; // stored value counts as empty for a face that judges by value. import { isEmptyFilterValue } from '@objectstack/spec/data'; // [#20176] The storage rule a temporal column puts a value in — ONE function, -// shared with `driver-sql`'s and `driver-memory`'s `where` — and the whole-day -// reading of a bare-day upper bound on a `datetime` column (ADR-0053 D-D), from -// the spec, where that rule is declared. +// shared with `driver-sql`'s and `driver-memory`'s `where`. import { temporalStorageForm, type TemporalComparandKind } from '@objectstack/core'; -import { nextUtcCalendarDay, UNBOUNDED_ABOVE, isUnboundedAbove, type UnboundedAbove } from '@objectstack/spec/data'; // [#20873] The JSON-stored population — the declared fields on which `$contains` // asks MEMBERSHIP — from the spec's value-shape classes, the same two // `driver-sql`'s JSON-column registry is built from. @@ -1663,31 +1663,6 @@ function listHolds(list: readonly unknown[], value: unknown): boolean { }); } -/** - * [#20176] ADR-0053 D-D: a bare `YYYY-MM-DD` as the UPPER bound of a - * `datetime` column (`$lte`, a `$between` max) means that WHOLE day — the - * exclusive bound at the next day's midnight, in the column's storage form. - * `undefined` when the rule does not apply: another class, or a bound that is - * not a bare calendar day (a full timestamp and a `Date` keep instant - * semantics). The same decision both drivers' `where` emitters take - * (`SqlDriver.calendarDayUpperBoundRewrite`, `driver-memory`'s `$lte` arm), - * read from the spec's `nextUtcCalendarDay`. - * - * [#20600] `UNBOUNDED_ABOVE` for `9999-12-31`, the last supported day: every - * supported value is inside its whole day, so the callers compare against NO - * upper bound — `$lte` asks only for a value, a `$between` keeps its minimum. - * The drivers compile the same (`IS NOT NULL`, `$ne: null`). - */ -function wholeDayUpperBound( - bound: unknown, - kind: TemporalComparandKind | undefined, -): unknown | UnboundedAbove { - if (kind !== 'datetime') return undefined; - const next = nextUtcCalendarDay(bound); - if (isUnboundedAbove(next)) return UNBOUNDED_ABOVE; - return next === null ? undefined : temporalStorageForm(next, 'datetime'); -} - /** * [#20873] The JSON NUMBER grammar, spelled out — the pattern `driver-sql`'s * `jsonMembershipCandidates` tests a `$contains` comparand against, for its @@ -1741,15 +1716,16 @@ function storedArrayHasMember(value: unknown, comparand: unknown): boolean { * its class and the class is `date`, `datetime` or `time`. Then the row's value * AND every comparand of `$eq` / `$ne` / the four orderings / `$between` / * `$in` / `$nin` / implicit equality are put in that rule's storage form - * (`temporalStorageForm`) before they are compared, and a bare-day upper bound - * on a `datetime` column reads as the whole day ({@link wholeDayUpperBound}) — - * the reading the drivers give the same comparand in a `where`. So - * `'2026-02-01T00:00:00.000Z'` against a `date` column is the day - * `'2026-02-01'`, `'2026-02-01'` as a `datetime` `$lte` includes that day's - * rows, epoch milliseconds are an instant, and a `Date` against a `date` or - * `time` column is its UTC day or time of day. The value takes the form too - * because that is the pairing the drivers compare (`driver-sql` wraps a legacy - * SQLite column in the same canon); rows a driver returns are in it already. + * (`temporalStorageForm`) before they are compared — the reading the drivers + * give the same comparand in a `where`. So `'2026-02-01T00:00:00.000Z'` against + * a `date` column is the day `'2026-02-01'`, epoch milliseconds are an instant, + * and a `Date` against a `date` or `time` column is its UTC day or time of day. + * A bare-day `$lte` on a `datetime` column reaches this function already lowered + * to the next day's `$lt` by the engine's seam (ADR-0053 D-D1 item 5, as + * amended); this function applies no whole-day rule of its own. The value + * takes the form too because that is the pairing the drivers compare + * (`driver-sql` wraps a legacy SQLite column in the same canon); rows a driver + * returns are in it already. * Presence (`$exists`, `$null`), the text operators and a `{ $field }` * reference are not comparands of a value, and are read as before. * @@ -1847,8 +1823,7 @@ function checkCondition( // [#20148] The comparison and list arms read a `Date` bound — or a `Date` // value — as an instant ({@link instantsOf}); every other pair compares // exactly as before. [#20176] On a temporal column both sides are in its - // storage form first (`form`), and a bare-day upper bound on a `datetime` - // column is the whole day ({@link wholeDayUpperBound}). + // storage form first (`form`). const stored = form(value); switch (op) { case '$eq': if (!comparandEquals(stored, form(target))) return false; break; @@ -1856,29 +1831,18 @@ function checkCondition( case '$gt': if (!ordered(stored, form(target), (a, b) => a > b)) return false; break; case '$gte': if (!ordered(stored, form(target), (a, b) => a >= b)) return false; break; case '$lt': if (!ordered(stored, form(target), (a, b) => a < b)) return false; break; - case '$lte': { - const dayAfter = wholeDayUpperBound(target, kind); - if (isUnboundedAbove(dayAfter)) { - // [#20600] No upper bound: what `$lte` still asks is a value. - if (stored === null || stored === undefined) return false; - break; - } - if (dayAfter !== undefined - ? !ordered(stored, dayAfter, (a, b) => a < b) - : !ordered(stored, form(target), (a, b) => a <= b)) return false; - break; - } + // [ADR-0053 D-D1 items 5 and 9, as amended] Both upper bounds compare as + // written. The whole-day reading of a bare day on a `datetime` column, + // and the last supported day bounding nothing, are applied once by the + // engine's seam (`lowerFilterCondition`), which hands this arm a `$lt` the + // next day (or `$null: false`) and splits a literal `$between` into `$gte` + // and that bound. A caller evaluating rows without the seam gets the + // comparison it wrote. + case '$lte': if (!ordered(stored, form(target), (a, b) => a <= b)) return false; break; case '$between': { if (!Array.isArray(target)) break; - const dayAfter = wholeDayUpperBound(target[1], kind); - // [#20600] A max on the last supported day bounds nothing: the range - // keeps its minimum alone. if (ordered(stored, form(target[0]), (a, b) => a < b) - || (isUnboundedAbove(dayAfter) - ? false - : dayAfter !== undefined - ? ordered(stored, dayAfter, (a, b) => a >= b) - : ordered(stored, form(target[1]), (a, b) => a > b))) return false; + || ordered(stored, form(target[1]), (a, b) => a > b)) return false; break; } case '$in': if (!Array.isArray(target) || !listHolds(target.map(form), stored)) return false; break; diff --git a/packages/plugins/plugin-security/src/rls-check-contains-membership.test.ts b/packages/plugins/plugin-security/src/rls-check-contains-membership.test.ts new file mode 100644 index 00000000000..8732e695b54 --- /dev/null +++ b/packages/plugins/plugin-security/src/rls-check-contains-membership.test.ts @@ -0,0 +1,162 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * A row-level policy written with `contains` over a multi-valued field gets ONE + * answer on both sides of a write, through the real plugin, the real engine + * and the SQL drivers: the read it scopes (`using`) and the write its `check` + * judges both ask MEMBERSHIP, the question `FILTER_OPERATORS`' `$contains` + * docblock (`@objectstack/spec`) gives a JSON-stored column. + * + * `record.tags.contains('x')` lowers to `{ tags: { $contains: 'x' } }`. The + * read is compiled by `driver-sql` as a membership test over the JSON column, + * so it shows the row holding `['x']` and hides the row holding `['xy']`. The + * write check evaluates the same filter in-process against the post-image + * (`@objectstack/formula`'s `matchesFilterCondition`, handed the object's + * declared columns), which answered the substring test alone: a stored array is + * not a string, so it refused EVERY write, the one the read shows included. + * It now asks membership too: + * + * | post-image `tags` | read under `using` | `check` insert / by-id update, before | after | + * |---|---|---|---| + * | `['x']` | shown | 403 | **admitted** | + * | `['xy']` | hidden | 403 | 403 | + * | `['a', 'x']` | shown | 403 | **admitted** | + * + * The change only widens the check's admit set, and only onto rows the read + * under the same predicate shows; a member the read hides stays refused. + */ + +import { describe, it, expect, afterEach, vi } from 'vitest'; +import { ObjectQL } from '@objectstack/objectql'; +import { SqlDriver } from '@objectstack/driver-sql'; +import { SqliteWasmDriver } from '@objectstack/driver-sqlite-wasm'; +import { PermissionSetSchema } from '@objectstack/spec/security'; +import { SecurityPlugin } from './security-plugin.js'; +import { defaultPermissionSets } from './objects/default-permission-sets.js'; + +const SYS_CTX = { isSystem: true, userId: 'usr_system' }; +const MEMBER_DEFAULT = defaultPermissionSets.find((p) => p.name === 'member_default')!; +const PREDICATE = "record.tags.contains('x')"; + +type Driver = { disconnect?: () => Promise }; +const DRIVERS: Array<[name: string, make: () => Driver]> = [ + ['driver-sql (better-sqlite3)', () => + new SqlDriver({ client: 'better-sqlite3', connection: { filename: ':memory:' }, useNullAsDefault: true })], + ['driver-sqlite-wasm', () => new SqliteWasmDriver({ filename: ':memory:' })], +]; + +const booted: ObjectQL[] = []; +afterEach(async () => { + while (booted.length) { + try { await booted.pop()!.destroy(); } catch { /* noop */ } + } +}); + +let seq = 0; +async function boot(makeDriver: () => Driver, clause: 'using' | 'check') { + const OBJ = `qa_ticket_contains_${process.pid}_${++seq}`; + const engine = new ObjectQL(); + engine.registerDriver(makeDriver() as never, true); + await engine.init(); + engine.registerApp({ + id: `com.objectstack.qa.rls-contains-membership-${seq}`, + name: 'RLS contains membership', + version: '1.0.0', + type: 'plugin', + scope: 'system', + objects: [ + { + name: OBJ, + label: 'Ticket', + sharingModel: 'public_read_write', + fields: { + id: { name: 'id', type: 'text', primaryKey: true }, + title: { name: 'title', type: 'text' }, + tags: { name: 'tags', type: 'tags' }, + }, + }, + ], + } as never); + await engine.syncSchemas(); + booted.push(engine); + + const set = PermissionSetSchema.parse({ + name: 'qa_ticket_guard', + objects: { [OBJ]: { allowRead: true, allowCreate: true, allowEdit: true, allowDelete: true } }, + rowLevelSecurity: [{ name: 'ticket_guard', object: OBJ, operation: 'all', [clause]: PREDICATE }], + }); + const services: Record = { + manifest: { register: vi.fn() }, + objectql: engine, + metadata: { + get: async (_type: string, name: string) => engine.getSchema(name) ?? null, + list: async () => [MEMBER_DEFAULT, set], + }, + }; + const logger = { info: vi.fn(), warn: vi.fn(), error: vi.fn(), debug: vi.fn() }; + const ctx = { + logger, + registerService: vi.fn(), + getService: (name: string) => { + if (!(name in services)) throw new Error(`service not registered: ${name}`); + return services[name]; + }, + }; + const plugin = new SecurityPlugin({ fallbackPermissionSet: 'member_default' }); + await plugin.init(ctx as never); + await plugin.start(ctx as never); + vi.spyOn((engine as unknown as { logger: { warn: () => void } }).logger, 'warn').mockImplementation(() => undefined); + + const caller = { userId: 'usr_member', positions: ['qa_pos'], permissions: [set.name], posture: 'MEMBER' }; + const storedIds = async () => + ((await engine.find(OBJ, { context: SYS_CTX } as never)) as Array>) + .map((r) => String(r.id)) + .sort(); + return { OBJ, engine, caller, storedIds }; +} + +type Envelope = { code: string; status: number }; +const DENIED: Envelope = { code: 'PERMISSION_DENIED', status: 403 }; +const envelopeOf = (e: unknown): Envelope => { + const x = e as { code?: string; status?: number; statusCode?: number }; + return { code: String(x?.code), status: Number(x?.statusCode ?? x?.status) }; +}; +const outcome = (p: Promise): Promise<'admitted' | Envelope> => + p.then(() => 'admitted' as const, (e: unknown) => envelopeOf(e)); + +const ROWS = [ + { id: 'r1', title: 'member', tags: ['x'] }, + { id: 'r2', title: 'substring only', tags: ['xy'] }, + { id: 'r3', title: 'member among others', tags: ['a', 'x'] }, +]; + +for (const [driverName, makeDriver] of DRIVERS) { + describe(`${driverName}: a contains policy over a multi-valued field answers membership on read and on write`, () => { + it("using: the read shows the rows holding the member 'x', and hides ['xy']", async () => { + const r = await boot(makeDriver, 'using'); + await r.engine.insert(r.OBJ, ROWS, { context: SYS_CTX } as never); + const shown = (await r.engine.find(r.OBJ, { context: r.caller } as never)) as Array<{ id: string }>; + expect(shown.map((x) => x.id).sort()).toEqual(['r1', 'r3']); + }); + + it("check: an insert of the rows the read shows is admitted and stored; ['xy'] is refused 403", async () => { + const w = await boot(makeDriver, 'check'); + for (const row of ROWS) { + const expected = row.id === 'r2' ? DENIED : 'admitted'; + expect(await outcome(w.engine.insert(w.OBJ, row, { context: w.caller } as never)), row.id).toEqual(expected); + } + expect(await w.storedIds()).toEqual(['r1', 'r3']); + }); + + it("check: a by-id update into ['xy'] is refused 403 and changes nothing; back into ['x', 'b'] is admitted", async () => { + const w = await boot(makeDriver, 'check'); + await w.engine.insert(w.OBJ, [ROWS[0]], { context: SYS_CTX } as never); + expect(await outcome(w.engine.update(w.OBJ, { tags: ['xy'] }, { where: { id: 'r1' }, context: w.caller } as never))) + .toEqual(DENIED); + const after = (await w.engine.find(w.OBJ, { where: { id: 'r1' }, context: SYS_CTX } as never)) as Array<{ tags: unknown }>; + expect(after[0]?.tags).toEqual(['x']); + expect(await outcome(w.engine.update(w.OBJ, { tags: ['x', 'b'] }, { where: { id: 'r1' }, context: w.caller } as never))) + .toBe('admitted'); + }); + }); +} diff --git a/packages/services/service-analytics/src/__tests__/read-scope-shared-lowering-seam.test.ts b/packages/services/service-analytics/src/__tests__/read-scope-shared-lowering-seam.test.ts index d31a3c28028..7898d1fa311 100644 --- a/packages/services/service-analytics/src/__tests__/read-scope-shared-lowering-seam.test.ts +++ b/packages/services/service-analytics/src/__tests__/read-scope-shared-lowering-seam.test.ts @@ -32,10 +32,12 @@ * - **An RLS `using` bound** — the RLS compile seam (`judgeCompiledComparands`, * step 2) lowers every compiled policy filter before `getReadFilter` returns * it. The half is pinned in BOTH spellings the read scope can be handed: the - * policy as that seam lowers it on a typed guard, and the policy as written - * (what a guard without types, or any producer that skipped that seam, - * hands over). Both answer `find()`'s rows, so this face's answer does not - * depend on which one reached it. + * policy as that seam lowers it, and the policy as written (what a producer + * that skipped that seam hands over). A guard without types no longer hands + * the written form: since the faces' copies were deleted, that seam lowers + * it type-blind (ADR-0053 D-D1 item 7), so it arrives as the first spelling. + * Both answer `find()`'s rows, so this face's answer does not depend on + * which one reached it. * * ## Column-type scope (the amendment's item 7) * @@ -43,9 +45,10 @@ * `declaredValueShape` — so the whole-day rule rewrites a declared `datetime` * column only, the scope `SqlDriver` holds. A `date` column compiles * byte-identical to before (`<= day` orders exactly as `< next-day` on date - * text). A caller that hands no declarations reads NO column as `datetime`: - * the step-2 RLS seam's choice for a guard without types, for the same reason - * — it moves no answer. + * text). A caller that hands no declarations reads NO column as `datetime`, + * and its bound compiles as written (pinned below). The RLS compile seam reads + * a guard without types the other way since the faces' copies were deleted — + * type-blind, item 7's other half — so this is no longer that seam's choice. */ import { describe, it, expect, beforeAll, afterAll, vi } from 'vitest'; @@ -188,13 +191,14 @@ describe('[ADR-0053 D-D1 amended — #5930 step 3] the read scope lowers its fil // ── The RLS `using` half, in both spellings the read scope can be handed ───── - it('RLS using, as the RLS seam lowers it on a typed guard: the whole named day', async () => { + it('RLS using, as the RLS seam lowers it: the whole named day', async () => { // `record.signed_at <= '2026-07-28'` compiled and lowered by - // `judgeCompiledComparands` with `signed_at` in the guard's datetime set. + // `judgeCompiledComparands`: with `signed_at` in a typed guard's datetime + // set, or type-blind for a guard without types (ADR-0053 D-D1 item 7). await expectSameRows({ signed_at: { $lt: '2026-07-29' } }, ['c27', 'c28']); }); - it('RLS using, as written (a guard without types): the whole named day all the same', async () => { + it('RLS using, as written (a producer that skipped the RLS seam): the whole named day all the same', async () => { await expectSameRows({ signed_at: { $lte: '2026-07-28' } }, ['c27', 'c28']); }); diff --git a/packages/spec/src/data/filter.zod.ts b/packages/spec/src/data/filter.zod.ts index e26330ebcc6..0ec404b85bf 100644 --- a/packages/spec/src/data/filter.zod.ts +++ b/packages/spec/src/data/filter.zod.ts @@ -428,11 +428,12 @@ const ORDERING_COMPARAND_DESCRIPTION = * that in the COMPARAND position (`'14:30'` → `'14:30:00'`, the #3979 * contract pair). A `$gte: '09:00'` on a `time` column is a supported * comparison an ISO refinement would refuse. - * 3. **date-only and full-timestamp are already reconciled by the driver**, so + * 3. **date-only and full-timestamp are already reconciled downstream**, so * narrowing buys no safety there. A bare `YYYY-MM-DD` anchors to midnight - * UTC for a lower bound and is rewritten to the half-open - * `< next-day-midnight` for an upper bound (`calendarDayUpperBoundRewrite`, - * the #3777 convention). + * UTC for a lower bound, and an upper bound is rewritten to the half-open + * `< next-day-midnight` by the shared `lowerFilterCondition` at the seams + * (ADR-0053 D-D1, as amended; the #3777 convention) before each driver + * puts it in its storage form. * * ## What widening ADMITS, stated plainly * @@ -728,12 +729,13 @@ const RANGE_ENDPOINT_DESCRIPTION = * `{ at: { $between: ['08:00:00', '18:00:00'] } }` on a `Field.time` column. * A declaration contradicted by the conformance table one directory over is * not under-describing reality; it is disagreeing with it. - * - **The driver already normalises both ends per column type.** + * - **Both ends are already normalised per column type downstream.** * `SqlDriver.coerceFilterValue` recurses through arrays member-wise - * (`value.map(v => this.coerceFilterValue(table, field, v))`), and - * `calendarDayBetweenRewrite` coerces the min and rewrites a bare-calendar-day - * max into the half-open `< next-day(max)` bound — knex's `whereBetween` being - * inclusive on both ends, it inherits the same rule `$lte` has (#3777). + * (`value.map(v => this.coerceFilterValue(table, field, v))`), and the shared + * `lowerFilterCondition` (ADR-0053 D-D1, as amended) splits a `$between` at + * the seams into `$gte` its min and `$lte` its max, so a bare-calendar-day max + * becomes the half-open `< next-day(max)` bound by the same rule `$lte` has + * (#3777) — a range being inclusive on both ends. * * A closed interval is the natural spelling of a **date window**, which makes * this the slot an author — an AI author in particular — is most likely to @@ -747,9 +749,9 @@ const RANGE_ENDPOINT_DESCRIPTION = * range applies to), an ISO refinement would reject the `HH:MM[:SS[.fff]]` form * `field-value.zod.ts`'s `CLOCK_TIME_TYPES` declares and the conformance case * above exercises, and date-only vs full-timestamp is already reconciled - * downstream by `calendarDayBetweenRewrite`. Endpoint-vs-column correctness is - * a field-TYPED judgement that already has an owner; re-guessing it here would - * refuse working ranges. + * downstream, by the shared `lowerFilterCondition` at the seams. + * Endpoint-vs-column correctness is a field-TYPED judgement that already has an + * owner; re-guessing it here would refuse working ranges. * * ## What widening ADMITS, stated plainly * @@ -1022,6 +1024,24 @@ export const RangeOperatorSchema = lazySchema(() => z.object({ * analytics face's SQL echo renders SQLite's `json_each` construct for the * same question. A field with no recorded declaration keeps the substring * reading, as a table `driver-sql` was never told about does. + * - **`driver-mongodb` — ANSWERS the membership contract, on every face.** + * `translateFilter` (query, count, write and the aggregation `$match` alike) + * forks on the field's DECLARED value shape, read from the schema + * `syncSchema` recorded (`STRUCTURED_JSON_TYPES`, or `isMultiValueField`): an + * array-only `$elemMatch` over the members `@objectstack/core`'s + * `jsonMembershipCandidates` names there, the substring `$regex` on a scalar + * column. `['u10']` no longer answers `'u1'`; the stored number `1` answers + * `'1'`. Measured on the emitted documents, read server-free and cross-checked + * against mingo; a real `mongod` was not measured. A field with no recorded + * declaration keeps the substring reading. + * - **`@objectstack/formula`'s `matchesFilterCondition` — ANSWERS the + * membership contract.** This face judges one record (the RLS write check, + * the explain engine), so it reads the column's DECLARATION when its caller + * supplies one (`options.fields`) and the stored value's shape otherwise: an + * array asks membership, anything else substring — the by-value reading this + * face already gives `$empty`. Measured through `plugin-security` on SQLite: a + * `contains` policy over a multi-valued field admits a write of the row its + * read shows (`['x']`) and refuses `['xy']`, which its read hides. * * Only `$contains` / `$notContains` are ruled here. The other text operators * over a stored array are not: measured on the same fixture, `$startsWith`