Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions .changeset/21067-json-refusal-under-bound.md
Original file line number Diff line number Diff line change
@@ -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.
98 changes: 83 additions & 15 deletions packages/core/src/utils/json-column-operator-refusal.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down Expand Up @@ -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);
Expand All @@ -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<readonly [op: string, bare: boolean]> = [
...[...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);
});
});
111 changes: 85 additions & 26 deletions packages/core/src/utils/json-column-operator-refusal.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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`'
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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)}`,
};
}
Original file line number Diff line number Diff line change
Expand Up @@ -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" } }`);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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`.
Expand Down
Loading
Loading