diff --git a/.changeset/21067-json-refusal-under-bound.md b/.changeset/21067-json-refusal-under-bound.md new file mode 100644 index 00000000000..7da1122f96b --- /dev/null +++ b/.changeset/21067-json-refusal-under-bound.md @@ -0,0 +1,16 @@ +--- +"@objectstack/core": patch +"@objectstack/driver-sql": patch +"@objectstack/driver-memory": patch +"@objectstack/objectql": patch +--- + +fix(core): the refusal a filter gets for a scalar comparison or text operator on a multi-value or JSON field reads true on every backend that prints it, and reaches a REST caller whole + +Clause-②: no + +The `INVALID_FILTER` / 400 refusal `driver-sql`'s `where`, the engine's per-aggregation `filter` and `driver-memory` all print (`jsonColumnOperatorRefusalText`) explained itself with `driver-sql`'s storage ("a field this driver stores as a JSON TEXT column") and the two wrong answers SQL used to give. That is untrue on the engine and on `driver-memory`. The message was also 748 characters, and the REST envelope cuts a 4xx message at 499 plus an ellipsis, so callers on SQLite and PostgreSQL read `…Refused rather than compiled because the answ…` and never reached the sentence saying the field and the operator were withheld. + +The message now reads, on every backend, in 486 characters: `A constraint in this filter WAS NOT APPLIED: it aims a scalar comparison or text operator at a multi-value or JSON field, which it cannot test for one member.`, then the same `$contains` / `$or` of `$contains` remedy, then `For no value, use "$null" or "$empty".` (a `null` comparand such as `{ f: null }`, `$eq: null` or `$ne: null` is refused too, and `$contains` could not express it), then `The field and the operator are withheld from the message; the full diagnostic is in the server log.` The diagnostic (the server-log text, and what a filter's own author is shown) gives the same reason with the operator named, names the field, and spells the remedy with the field's name. It drops the storage and the SQL history too, and is now whole on the wire for field names up to 26 characters (it was 643 characters or more and always cut). + +Code, status, the refused operator set and the `$contains` remedy are unchanged. A client that matched on the old words `JSON TEXT column` or `Refused rather than compiled` should match on `code: "INVALID_FILTER"` instead. diff --git a/packages/core/src/utils/json-column-operator-refusal.test.ts b/packages/core/src/utils/json-column-operator-refusal.test.ts index 5cd099dc812..b885a9642fd 100644 --- a/packages/core/src/utils/json-column-operator-refusal.test.ts +++ b/packages/core/src/utils/json-column-operator-refusal.test.ts @@ -4,26 +4,35 @@ * [#21007] The JSON-column refusal's operator set and words, moved here from * `driver-sql` so the engine's per-aggregation `filter` refuses with them too. * - * Two pins, both against what `driver-sql` answered BEFORE the move: + * Two pins: * * - **The set** — the 22 spellings `driver-sql`'s module-private * `JSON_COLUMN_INCOMPATIBLE_OPERATORS` held, member for member, and * [#21009] the five text operators that joined them since. - * - **The words** — the SHA-256 of each text, captured from `driver-sql`'s - * built `jsonColumnOperatorError` at the commit before the move (`8f784959c`) - * through a real `SqlDriver` over SQLite: the withheld message (one text for - * every operator), and the diagnostic for an operator, for `$between`, and - * for the bare equality spelling, on a column named `members`. A hash rather - * than a second literal copy, so this file is not a third place the sentence - * lives; the length beside each hash says how far a failure moved it. + * - **The words** — the SHA-256 of each text: the withheld message (one text + * for every operator), and the diagnostic for an operator, for `$between`, + * and for the bare equality spelling, on a column named `members`. A hash + * rather than a second literal copy, so this file is not a third place the + * sentence lives; the length beside each hash says how far a failure moved + * it. The hashes were captured from `driver-sql`'s built + * `jsonColumnOperatorError` at the commit before the move (`8f784959c`), and + * [#21067] re-captured from this builder when the words were rewritten. + * + * [#21067] And what the words must SAY, whatever they are: the message reaches + * the REST caller WHOLE — run through the envelope's own bound + * (`truncateClientMessage`, `@objectstack/types`; the 500 itself is + * module-private there, so the function that applies it is what is imported) + * it comes back unchanged — and it carries the remedy and the "withheld" + * sentence. It names no face's storage or wrong answer, since three faces + * print it. * * A deliberate change of wording updates the hashes in the PR that makes it — - * and then reaches `where` and the per-aggregation `filter` alike, which is the - * point of the move. + * and then reaches every face alike, which is the point of the move. */ import { describe, it, expect } from 'vitest'; import { createHash } from 'node:crypto'; +import { truncateClientMessage } from '@objectstack/types'; import { JSON_COLUMN_INCOMPATIBLE_OPERATORS, jsonColumnOperatorRefusalText } from './json-column-operator-refusal.js'; const sha256 = (text: string): string => createHash('sha256').update(text, 'utf8').digest('hex'); @@ -51,13 +60,13 @@ describe('[#21007] JSON_COLUMN_INCOMPATIBLE_OPERATORS', () => { }); }); -describe('[#21007] jsonColumnOperatorRefusalText — byte for byte what driver-sql printed before the move', () => { - const MESSAGE = { sha: 'c6103dd665625ab3a779822fecbe650bd7f67fc6ea38d17d5fd57cf6ae9193ff', length: 748 }; +describe('[#21067] jsonColumnOperatorRefusalText — the words, by hash', () => { + const MESSAGE = { sha: 'f0a54a98fb30ae8e201c7be5e9f0d1715649fd34303e176abea131981dfd36f7', length: 486 }; it.each([ - ['an operator', '$in', false, { sha: '358d5aae39170ab3da198beb39368a3f3476dd057dbd5da5f4295cf244eab67e', length: 648 }], - ['$between', '$between', false, { sha: '505c094ac5212ac121cdeb4803787ba036f46177450d542fbccf60a9386924fe', length: 658 }], - ['the bare equality spelling', '=', true, { sha: '92bb1728ca012c4cde6deb05be3cb0320e113c8c5ac250ecad5b091f3bb87094', length: 660 }], + ['an operator', '$in', false, { sha: 'da62717ac99e4e7470c977fd13ec7e36c32bcecb5c7d45b95d5ba09ded5d81cd', length: 406 }], + ['$between', '$between', false, { sha: '4ff8d528fb53e31a73b8a190424fa7e2446bd0bd64ac984bf1f70e4ebe1e83d0', length: 416 }], + ['the bare equality spelling', '=', true, { sha: '35e2fd3a5e287ac76ef3fb09922d537935283b930143035b69a07267a762cb5c', length: 418 }], ] as const)('%s', (_name, op, bare, diagnostic) => { const text = jsonColumnOperatorRefusalText('members', op, bare); expect({ sha: sha256(text.message), length: text.message.length }).toEqual(MESSAGE); @@ -83,3 +92,62 @@ describe('[#21007] jsonColumnOperatorRefusalText — byte for byte what driver-s expect(diagnostic).toContain('{ "secret_col": { "$contains": "a" } }'); }); }); + +describe('[#21067] jsonColumnOperatorRefusalText — whole on the wire, and true on every face', () => { + /** Every spelling that gets this refusal: each member of the set, and the bare one. */ + const SPELLINGS: ReadonlyArray = [ + ...[...JSON_COLUMN_INCOMPATIBLE_OPERATORS].map((op) => [op, false] as const), + ['=', true], + ]; + + it('the message passes the REST envelope\'s bound unchanged, for every refused spelling', () => { + for (const [op, bare] of SPELLINGS) { + const { message } = jsonColumnOperatorRefusalText('members', op, bare); + // The bound's own function: a message at or past it comes back cut to 499 + // characters plus an ellipsis, so equality here means every word arrives. + expect(truncateClientMessage(message), op).toBe(message); + } + }); + + it('inside the bound: the remedy for one member, for any-of and for no value, then where the field and the operator went', () => { + const { message } = jsonColumnOperatorRefusalText('members', '$in', false); + expect(message.startsWith('A constraint in this filter WAS NOT APPLIED: ')).toBe(true); + expect(message).toContain( + 'Use "$contains" for membership ({ "FIELD": { "$contains": "a" } }), or an $or of "$contains" for any-of ' + + '({ "$or": [{ "FIELD": { "$contains": "a" } }, { "FIELD": { "$contains": "b" } }] }).', + ); + // A `null` comparand (`{ f: null }`, `$eq: null`, `$ne: null`) is refused too, and asks + // about presence, which `$contains` cannot spell: the presence operators are named. + expect(message).toContain('For no value, use "$null" or "$empty".'); + expect(message.endsWith( + 'The field and the operator are withheld from the message; the full diagnostic is in the server log.', + )).toBe(true); + }); + + it('names the rule, not a face: the field declaration, never a storage form or one backend\'s wrong answer', () => { + for (const [op, bare] of SPELLINGS) { + const { message, diagnostic } = jsonColumnOperatorRefusalText('members', op, bare); + for (const text of [message, diagnostic]) { + expect(text, op).toContain('a scalar comparison or text operator'); + expect(text, op).toContain('at a multi-value or JSON field, which it cannot test for one member.'); + expect(text, op).toContain('For no value, use "$null" or "$empty".'); + expect(text, op).not.toMatch(/this driver|JSON TEXT|serializ|matched nothing|asked to exclude/); + } + // The diagnostic names the operator in the reason too — the bare spelling's as `=`. + expect(diagnostic, op).toContain(`it aims "${op}", a scalar comparison or text operator, at`); + } + }); + + it('the diagnostic is the message with the names put back: same reason, same remedy with the field in it', () => { + const { message, diagnostic } = jsonColumnOperatorRefusalText('members', '$nin', false); + const reason = message.slice(message.indexOf(': ') + 2, message.indexOf(' Use "$contains"')); + expect(diagnostic.startsWith('Operator "$nin" on field "members" WAS NOT APPLIED: ')).toBe(true); + expect(diagnostic).toContain(reason.replace('a scalar comparison or text operator at', '"$nin", a scalar comparison or text operator, at')); + expect(diagnostic.endsWith( + '{ "$or": [{ "members": { "$contains": "a" } }, { "members": { "$contains": "b" } }] }). For no value, use "$null" or "$empty".', + )).toBe(true); + // An author-marked refusal puts this text on the wire (driver-sql's #8220 + // arm): for a field name of an ordinary length it is whole there too. + expect(truncateClientMessage(diagnostic)).toBe(diagnostic); + }); +}); diff --git a/packages/core/src/utils/json-column-operator-refusal.ts b/packages/core/src/utils/json-column-operator-refusal.ts index 7e2301c55ef..934b76bc87d 100644 --- a/packages/core/src/utils/json-column-operator-refusal.ts +++ b/packages/core/src/utils/json-column-operator-refusal.ts @@ -26,6 +26,30 @@ * carries the #8220 provenance seam, the engine's the ADR-0112 envelope); what * they share is what the caller reads. * + * ## The words: true on every face, whole on the wire + * + * [#21067] Three faces print the sentence below — `driver-sql`'s `where`, the + * engine's per-aggregation `filter`, and `driver-memory`'s filter gate (#21066) + * — and each reached its wrong answer by a different route: SQL compared the + * serialization, the engine compared an array in JS, and mingo compared each + * member. So the reason the sentence gives is the one that holds on all three: + * a scalar comparison or text operator met a multi-value or JSON field; + * membership is `$contains`, and no value is `$null` / `$empty`. It names no + * storage form and no backend's wrong answer; those stay in these docblocks, + * out of what a caller reads. + * + * It is also sized for the door it is read through. The REST envelope cuts a + * 4xx message of 500 characters or more to 499 plus an ellipsis, keeping the + * head (`truncateClientMessage`, `@objectstack/types`). The withheld message is + * one constant text, so it is held WHOLE under that bound, the remedy and the + * "withheld" sentence included, by a pin that runs it through that function + * (`json-column-operator-refusal.test.ts`). The diagnostic names the field four + * times, so its length grows with the name. It is whole on the wire for a field + * name of up to 26 characters when an author-marked refusal discloses it. Its + * order (what was refused, why, then the remedy) leaves the presence clause + * and then the any-of example last, so a longer name pushes those out first; + * the any-of example survives up to 36 characters. + * * ## The other half of the JSON column's contract * * `$contains` is the membership spelling on such a column (`FILTER_OPERATORS`' @@ -108,12 +132,53 @@ export interface JsonColumnOperatorRefusalText { * What the caller is told. It names neither the field nor the operator: on a * read scope the predicate is an administrator's, so both are withheld * (#7929 / #8197), and the sentence says where they went. + * + * [#21067] One constant text, shorter than the REST envelope's 4xx bound, so + * every word of it reaches the caller. */ readonly message: string; - /** The full diagnostic — the field and the operator named — for the server log. */ + /** + * The full diagnostic — the field and the operator named — for the server log, + * and the wire text when a face discloses it to the filter's own author + * (`driver-sql`'s `'author'` provenance arm, #8220). + */ readonly diagnostic: string; } +/** + * [#21067] Why the operator was refused, in words true on every face that + * prints them — see the module docblock. Shared by both texts, so the message + * and the diagnostic cannot come to give two reasons; the diagnostic passes + * `op` and so names the operator, the bare spelling's as `=`. + */ +function refusalReason(op?: string): string { + const operator = op === undefined + ? 'a scalar comparison or text operator' + : `"${op}", a scalar comparison or text operator,`; + return `it aims ${operator} at a multi-value or JSON field, which it cannot test for one member.`; +} + +/** + * [#21067] The other half of the prescription. The refused set also catches a + * `null` comparand — `{ f: null }`, `$eq: null`, `$ne: null` — whose caller + * asked whether the field has a value, not which member it holds, so + * `$contains` cannot express it. The presence spellings can, and they answer on + * a multi-value or JSON field on every face (they are outside the set): + * `$null` is the literal `= null`, and `$empty` also counts an empty list. + * One constant clause, never a branch on the comparand, so the withheld message + * stays one text. + */ +const PRESENCE_REMEDY = 'For no value, use "$null" or "$empty".'; + +/** The prescription, spelled with `name` in the field position. */ +function containsRemedy(name: string): string { + return ( + `Use "$contains" for membership ({ "${name}": { "$contains": "a" } }), or an $or of ` + + `"$contains" for any-of ({ "$or": [{ "${name}": { "$contains": "a" } }, ` + + `{ "${name}": { "$contains": "b" } }] }). ${PRESENCE_REMEDY}` + ); +} + /** * [#7398] The words of the refusal: a scalar-comparison operator met a field * stored as JSON TEXT, so the comparison can never mean what the caller wrote. @@ -147,39 +212,33 @@ export interface JsonColumnOperatorRefusalText { * byte, so `where` and the per-aggregation `filter` print one sentence. The * caller builds the error: this returns only the text. * - * [#21009] The text family that joined the set reads these same words, - * unchanged. Its prescription holds as written — membership is `$contains` — - * while the "scalar comparison" wording and the two directions the closing - * sentence names are the equality family's. + * [#21009] The text family that joined the set reads these same words. Its + * prescription holds as written — membership is `$contains`. + * + * [#21067] Reworded once, for two reasons. The words named `driver-sql`'s + * mechanism ("a field this driver stores as a JSON TEXT column", and the two + * wrong answers SQL gave), which is untrue where the engine's per-aggregation + * `filter` and `driver-memory` print them; the reason is now + * {@link refusalReason}'s. And the message ran to 748 characters, so the REST + * envelope cut it at 499, partway through the sentence explaining the refusal, + * and no caller read the sentence saying the field and the operator were + * withheld; it is now 486, with the presence spellings a `null` comparand + * needs (see `PRESENCE_REMEDY`). The mechanism above stays here, where the + * next author reads it. */ export function jsonColumnOperatorRefusalText( field: string, op: string, bare: boolean, ): JsonColumnOperatorRefusalText { - const spelling = bare + const subject = bare ? `The bare equality spelling { "${field}": value }` - : `Operator "${op}"`; - const on = bare ? '' : ` on field "${field}"`; + : `Operator "${op}" on field "${field}"`; return { message: - `A constraint in this filter WAS NOT APPLIED: it aims a scalar comparison operator at a ` + - `field this driver stores as a JSON TEXT column (e.g. ["a","b"]), and such an operator ` + - `compares that whole serialized text against a single value — it can never equal one ` + - `member. Use "$contains" for membership ({ "FIELD": { "$contains": "a" } }), or an $or of ` + - `"$contains" for any-of ({ "$or": [{ "FIELD": { "$contains": "a" } }, ` + - `{ "FIELD": { "$contains": "b" } }] }). Refused rather than compiled because the answer ` + - `was silently wrong in BOTH directions: $in/$eq matched nothing, while $nin/$ne returned ` + - `the very rows they were asked to exclude. The field and the operator this filter ` + - `used are withheld from the message; the full diagnostic is in the server log.`, - diagnostic: - `${spelling}${on} WAS NOT APPLIED: "${field}" is a multi-value (or otherwise JSON-valued) ` + - `field, stored by this driver as a JSON TEXT column (e.g. ["a","b"]), and "${op}" compares ` + - `that whole serialized text against a single value — it can never equal one member. ` + - `Use "$contains" for membership ({ "${field}": { "$contains": "a" } }), or an $or of ` + - `"$contains" for any-of ({ "$or": [{ "${field}": { "$contains": "a" } }, ` + - `{ "${field}": { "$contains": "b" } }] }). Refused rather than compiled because the answer ` + - `was silently wrong in BOTH directions: $in/$eq matched nothing, while $nin/$ne returned ` + - `the very rows they were asked to exclude.`, + `A constraint in this filter WAS NOT APPLIED: ${refusalReason()} ${containsRemedy('FIELD')} ` + + `The field and the operator are withheld from the message; the full diagnostic is in the ` + + `server log.`, + diagnostic: `${subject} WAS NOT APPLIED: ${refusalReason(op)} ${containsRemedy(field)}`, }; } diff --git a/packages/drivers/driver-sql/src/sql-driver-json-column-operator-refusal.test.ts b/packages/drivers/driver-sql/src/sql-driver-json-column-operator-refusal.test.ts index 3558985da3b..3e83f6d78dd 100644 --- a/packages/drivers/driver-sql/src/sql-driver-json-column-operator-refusal.test.ts +++ b/packages/drivers/driver-sql/src/sql-driver-json-column-operator-refusal.test.ts @@ -120,10 +120,12 @@ function expectJsonColumnRefusal(err: WireBearingError, op: string, field: strin expect(err.message).toContain(op); expect(err.message).toContain(field); // The three things the message owes a caller: that the filter did NOT run, - // why (the column is JSON text), and what to write instead. + // why, and what to write instead. [#21067] The why is the field's + // declaration, not this driver's storage form: the same words are printed by + // faces that store no JSON text. expect(err.message).toContain('WAS NOT APPLIED'); - expect(err.message).toContain('JSON TEXT column'); - expect(err.message).toContain('$contains'); + expect(err.message).toContain(`it aims "${op}", a scalar comparison or text operator, at a multi-value or JSON field`); + expect(err.message).toContain(`{ "${field}": { "$contains": "a" } }`); } /** diff --git a/packages/drivers/driver-sql/src/sql-driver-json-column-refusal-shared-text.test.ts b/packages/drivers/driver-sql/src/sql-driver-json-column-refusal-shared-text.test.ts index 202ffb553c2..f468cd608d5 100644 --- a/packages/drivers/driver-sql/src/sql-driver-json-column-refusal-shared-text.test.ts +++ b/packages/drivers/driver-sql/src/sql-driver-json-column-refusal-shared-text.test.ts @@ -7,10 +7,11 @@ * move: what it throws IS the shared text, and what it refuses IS the shared * set, on a real `SqlDriver` over SQLite. * - * That the shared text is byte for byte what this driver printed before the move - * is pinned beside the text itself (`@objectstack/core`'s - * `json-column-operator-refusal.test.ts`, hashes captured from this driver at the - * commit before). Together: this driver's refusal did not change by one byte. + * The words themselves are pinned beside the text (`@objectstack/core`'s + * `json-column-operator-refusal.test.ts`, by hash): captured from this driver at + * the commit before the move, and re-captured when [#21067] rewrote them for + * every face and under the REST bound. That they reach a caller whole is + * `sql-driver-json-column-refusal-wire-bound.test.ts`. * * The per-operator content of the refusal (code, status, the prescription, every * lowering face) stays pinned in `sql-driver-json-column-operator-refusal.test.ts`. diff --git a/packages/drivers/driver-sql/src/sql-driver-json-column-refusal-wire-bound.test.ts b/packages/drivers/driver-sql/src/sql-driver-json-column-refusal-wire-bound.test.ts new file mode 100644 index 00000000000..446a73fc78e --- /dev/null +++ b/packages/drivers/driver-sql/src/sql-driver-json-column-refusal-wire-bound.test.ts @@ -0,0 +1,157 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#21067] The JSON-column refusal reaches the caller WHOLE: run through the + * envelope the REST `/data` door builds from a thrown refusal (`mapDataError`, + * `@objectstack/types`), its `error` is every word the refusal wrote, on every + * dialect cell. + * + * ## What the door cut + * + * The door cuts a 4xx message of 500 characters or more to 499 plus an + * ellipsis, keeping the head. The withheld message this driver throws on a + * multi-value or JSON field was 748 characters. Measured with that text back in + * place, on SQLite and a live PostgreSQL 16.14 alike, the envelope ended + * `Refused rather than compiled because the answ…`: no caller read the end of + * the reason, or the sentence saying the field and the operator were withheld + * and where the diagnostic went. The author-disclosed text (the diagnostic, + * which the `'author'` provenance arm puts on the wire) was 643 characters for + * a field named `owners`. It was cut for every field name, and from a + * 10-character name on, the cut took the any-of example as well. + * + * ## What this file pins, per dialect cell + * + * For every `$` spelling in `JSON_COLUMN_INCOMPATIBLE_OPERATORS` and the bare + * equality spelling, on a multi-value lookup, a `tags` field and a `json` + * field: + * + * - an unmarked filter: `400` / `INVALID_FILTER`, and the envelope's `error` IS + * the shared withheld message — so it was not cut — carrying the `$contains` + * remedy, the presence spellings a `null` comparand needs, and the sentence + * saying where the field and the operator went; + * - the same filter marked `'author'`: the envelope's `error` IS the shared + * diagnostic, whole, the remedy spelled with the field's own name. + * + * Compared with `jsonColumnOperatorRefusalText`'s output and the door's own + * function, never a copy of the sentence or of the 500: a later rewording that + * stays inside the bound leaves this file green, and one that does not turns + * it red. The words themselves are pinned once, in `@objectstack/core`. + * + * The SQLite cell always runs. The PostgreSQL and MySQL cells run where + * `OS_TEST_POSTGRES_URL` / `OS_TEST_MYSQL_URL` are set; the + * `Temporal Conformance (live PG + MySQL)` job sets both and runs this + * package's whole suite. Otherwise they are a named skip. + */ + +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import type { Knex } from 'knex'; +import { JSON_COLUMN_INCOMPATIBLE_OPERATORS, jsonColumnOperatorRefusalText } from '@objectstack/core'; +import { mapDataError } from '@objectstack/types'; +import { markFilterSubtreeProvenance } from '@objectstack/spec/data'; +import type { DriverOptions, FilterCondition } from '@objectstack/spec/data'; +import { SqlDriver } from './sql-driver.js'; +import { + DIALECT_CELLS, + declareDialectCell, + LIVE_CELL_TIMEOUT_MS, + type DialectCell, +} from './live-dialect-matrix.testkit.js'; + +/** Issue-prefixed: each live cell owns its table, dropped before and after. */ +const OBJECT = 'os21067_wire'; + +/** Diagnostics-only; it never changes which rows a read touches. */ +const BYPASS: DriverOptions = { bypassTenantAudit: true }; + +const FIELDS: Record> = { + label: { type: 'text' }, + owners: { type: 'lookup', reference: 'os21067_owner', multiple: true }, + tags_: { type: 'tags' }, + meta: { type: 'json' }, +}; + +/** The spellings a caller writes as an operator key: the `$` members of the set. */ +const REFUSED_OPERATORS = [...JSON_COLUMN_INCOMPATIBLE_OPERATORS].filter((op) => op.startsWith('$')); + +/** A comparand each operator's own contract accepts, so the refusal is always this gate's. */ +function comparandFor(op: string): unknown { + if (op === '$in' || op === '$nin') return ['u1', 'u9']; + if (op === '$between') return ['a', 'b']; + if (op === '$like' || op === '$ilike') return '%u1%'; + return 'u1'; +} + +interface WireBearingError extends Error { + code?: string; + status?: number; +} + +async function refusalOf(run: () => Promise): Promise { + try { + await run(); + } catch (e) { + return e as WireBearingError; + } + throw new Error('expected the driver to refuse this filter, but it resolved'); +} + +for (const cell of DIALECT_CELLS) { + declareDialectCell(cell, '[#21067] the JSON-column refusal on the wire', declareWireCell); +} + +function declareWireCell(cell: DialectCell): void { + describe(`[#21067] SqlDriver — the JSON-column refusal is whole in the /data door's envelope (${cell.label})`, () => { + let driver: SqlDriver; + let knexInstance: Knex; + + beforeAll(async () => { + driver = new SqlDriver(cell.config()); + knexInstance = driver.getKnex(); + await knexInstance.schema.dropTableIfExists(OBJECT); + await driver.initObjects([{ name: OBJECT, fields: FIELDS } as never]); + await driver.create(OBJECT, { id: '1', label: 'u1', owners: ['u1', 'u2'], tags_: ['red'], meta: ['a'] }, BYPASS); + }, LIVE_CELL_TIMEOUT_MS); + + afterAll(async () => { + await knexInstance?.schema.dropTableIfExists(OBJECT).catch(() => {}); + await driver?.disconnect?.(); + }); + + /** What the `/data` door answers for the refusal `where` draws. */ + const wireOf = async (where: Record) => { + const err = await refusalOf(() => driver.find(OBJECT, { where: where as FilterCondition }, BYPASS)); + const { status, body } = mapDataError(err, OBJECT); + return { status, code: body.code, error: String(body.error) }; + }; + + for (const field of ['owners', 'tags_', 'meta']) { + const spellings: ReadonlyArray = [ + ...REFUSED_OPERATORS.map((op) => [op, op, false, { [op]: comparandFor(op) }] as const), + ['bare equality', '=', true, 'u1'], + ]; + for (const [name, op, bare, condition] of spellings) { + it(`${field} ${name}: the withheld message, whole, then the diagnostic, whole, for the author`, async () => { + const shared = jsonColumnOperatorRefusalText(field, op, bare); + + const withheld = await wireOf({ [field]: condition }); + expect({ status: withheld.status, code: withheld.code }).toEqual({ status: 400, code: 'INVALID_FILTER' }); + // Equal to what the refusal wrote ⇒ the door's bound cut nothing. + expect(withheld.error).toBe(shared.message); + expect(withheld.error).toContain( + '({ "$or": [{ "FIELD": { "$contains": "a" } }, { "FIELD": { "$contains": "b" } }] })', + ); + expect(withheld.error).toContain('For no value, use "$null" or "$empty".'); + expect(withheld.error).toContain('withheld from the message; the full diagnostic is in the server log.'); + expect(withheld.error).not.toContain(`"${field}"`); + + const authored = await wireOf(markFilterSubtreeProvenance({ [field]: condition }, 'author')); + expect({ status: authored.status, code: authored.code }).toEqual({ status: 400, code: 'INVALID_FILTER' }); + expect(authored.error).toBe(shared.diagnostic); + expect(authored.error).toContain( + `({ "$or": [{ "${field}": { "$contains": "a" } }, { "${field}": { "$contains": "b" } }] })`, + ); + }); + } + } + }); +} diff --git a/packages/drivers/driver-sql/src/sql-driver-target-field-provenance.test.ts b/packages/drivers/driver-sql/src/sql-driver-target-field-provenance.test.ts index 35cde2ce4ae..95510c6e213 100644 --- a/packages/drivers/driver-sql/src/sql-driver-target-field-provenance.test.ts +++ b/packages/drivers/driver-sql/src/sql-driver-target-field-provenance.test.ts @@ -120,13 +120,13 @@ describe('[#8197] target-field refusals × filter-subtree provenance', () => { name: 'JSON column + $in (#7398)', where: () => ({ [POLICY_JSON_COL]: { $in: ['usr_1111'] } }), target: POLICY_JSON_COL, - klass: 'JSON TEXT column', + klass: 'at a multi-value or JSON field', }, { name: 'JSON column, bare equality spelling (#7398)', where: () => ({ [POLICY_JSON_COL]: 'usr_1111' }), target: POLICY_JSON_COL, - klass: 'JSON TEXT column', + klass: 'at a multi-value or JSON field', }, { name: 'empty field constraint (#5240)', diff --git a/packages/drivers/driver-sql/src/sql-driver.ts b/packages/drivers/driver-sql/src/sql-driver.ts index a7a0da5ef63..cfb10f21aa6 100644 --- a/packages/drivers/driver-sql/src/sql-driver.ts +++ b/packages/drivers/driver-sql/src/sql-driver.ts @@ -3329,9 +3329,11 @@ function unrenderableTextComparandError( * administrator's. The prescription survives redaction with PLACEHOLDER names — * the SHAPE is the repair, and the shape names nothing. * - * [#21007] The TEXT is `@objectstack/core`'s, byte for byte what this builder - * spelled before the move; the CONSTRUCTOR stays here, because the #8220 - * provenance seam it goes through is this driver's. + * [#21007] The TEXT is `@objectstack/core`'s; the CONSTRUCTOR stays here, + * because the #8220 provenance seam it goes through is this driver's. + * [#21067] That text no longer names this driver's storage form, since the + * engine and `driver-memory` print it too; the measured SQL consequences above + * live in the builder's docblock. */ function jsonColumnOperatorError(field: string, op: string, bare: boolean, subtree?: unknown): Error { const { message, diagnostic } = jsonColumnOperatorRefusalText(field, op, bare); diff --git a/packages/rest/src/aggregation-filter-json-column-refusal.test.ts b/packages/rest/src/aggregation-filter-json-column-refusal.test.ts index a36c29832b7..dc2488badd3 100644 --- a/packages/rest/src/aggregation-filter-json-column-refusal.test.ts +++ b/packages/rest/src/aggregation-filter-json-column-refusal.test.ts @@ -30,6 +30,12 @@ * and `500` `DATABASE_ERROR` on PostgreSQL 16.14; the per-aggregation `filter` * counted `m = 0` for each. Both faces now answer the `400` above. * + * [#21067] And the body carries the WHOLE refusal. The withheld message was + * 748 characters, so this door cut it to 499 plus an ellipsis, on SQLite and + * PostgreSQL alike: the wire ended mid-reason, before the sentence saying the + * field and the operator were withheld. It is now one text under the bound, and + * each row asserts the body equals it, not merely that the two faces agree. + * * ## The dialect axis of THIS file * * The SQLite cell always runs. The PostgreSQL and MySQL cells run where @@ -41,6 +47,7 @@ */ import { describe, it, expect, beforeAll, afterAll, vi } from 'vitest'; +import { jsonColumnOperatorRefusalText } from '@objectstack/core'; import { ObjectQL } from '@objectstack/objectql'; import { SqlDriver } from '@objectstack/driver-sql'; import { ObjectStackProtocolImplementation } from '@objectstack/metadata-protocol'; @@ -74,6 +81,13 @@ const VALUES: Record = { meta: ['a', 'b'], }; +/** + * [#21067] The withheld message every refusal below answers with: one constant + * text (it names neither the field nor the operator), so any call reads it. + * Read from the builder rather than copied, so a rewording moves it here too. + */ +const WITHHELD_MESSAGE = jsonColumnOperatorRefusalText('owners', '$in', false).message; + /** Every member of the family `where` refuses on a JSON column, as a filter on `field`. */ function family(field: string): Array]> { const [a, b] = VALUES[field]; @@ -128,6 +142,10 @@ const CONTROLS: ReadonlyArray]> = [ ['title implicit equality', { title: 'x' }], ['owners $contains — the prescribed spelling', { owners: { $contains: 'u1' } }], ['owners an $or of $contains — the any-of spelling', { $or: [{ owners: { $contains: 'u1' } }, { owners: { $contains: 'u3' } }] }], + // [#21067] The presence spellings the refusal prescribes for a `null` comparand answer too. + ['owners $null — the no-value spelling', { owners: { $null: true } }], + ['owners $empty — the no-value spelling that counts an empty list', { owners: { $empty: true } }], + ['owners $null: false — the has-a-value spelling', { owners: { $null: false } }], ]; interface Cell { @@ -229,10 +247,20 @@ for (const cell of CELLS) { const agg = await post(perAggregation(filter)); expect(agg.status, JSON.stringify(agg.json)).toBe(400); expect(agg.json.code).toBe('INVALID_FILTER'); - // The same words — byte for byte, including the envelope's own cut. + // The same words — byte for byte. expect(agg.json.error).toBe(twin.json.error); + // [#21067] …and every one of them: the envelope cuts a 4xx message at + // its bound, so equal to what the refusal wrote means nothing was cut, + // the any-of remedy and the "withheld" sentence included. + expect(twin.json.error).toBe(WITHHELD_MESSAGE); expect(agg.json.error).toContain('{ "FIELD": { "$contains": "a" } }'); - expect(agg.json.error).toContain('{ "$or": [{ "FIELD": { "$contains": "a" } }'); + expect(agg.json.error).toContain('{ "$or": [{ "FIELD": { "$contains": "a" } }, { "FIELD": { "$contains": "b" } }] }'); + // The no-value spelling a null comparand needs, in two pieces: this file + // is in `check:live-db-isolation`'s scan, which reads a MySQL USE + // statement in the verb followed by a quoted operand. + expect(agg.json.error).toContain('For no value,'); + expect(agg.json.error).toContain('"$null" or "$empty".'); + expect(agg.json.error).toContain('withheld from the message; the full diagnostic is in the server log.'); expect(agg.json.error).not.toContain(`"${field}"`); // The field and the operator are in the server log, not the response. const logged = warn.mock.calls.map((call: unknown[]) => String(call[0])).join('\n'); @@ -263,6 +291,7 @@ for (const cell of CELLS) { expect(agg.status, JSON.stringify(agg.json)).toBe(400); expect(agg.json.code).toBe('INVALID_FILTER'); expect(agg.json.error).toBe(twin.json.error); + expect(twin.json.error).toBe(WITHHELD_MESSAGE); expect(agg.json.error).toContain('{ "FIELD": { "$contains": "a" } }'); expect(agg.json.error).not.toContain(`"${field}"`); const logged = warn.mock.calls.map((call: unknown[]) => String(call[0])).join('\n');