diff --git a/.changeset/27351-relation-condition-route-first.md b/.changeset/27351-relation-condition-route-first.md new file mode 100644 index 00000000000..e357be7aa17 --- /dev/null +++ b/.changeset/27351-relation-condition-route-first.md @@ -0,0 +1,14 @@ +--- +'@objectstack/objectql': patch +--- + +fix(objectql): the nested-relation condition refusal states what to write inside the 500-character client cap + +Clause-②: no +Metadata-protocol: none + +`POST /api/v1/data/OBJECT/query` (and `find` / `count` / `aggregate` / `update` / `delete`) refuses a nested-relation condition in `where` with `400 INVALID_FILTER` when it reaches a second relation, uses a dotted key, names no key, names a key the related object does not declare, or sits beneath a relation whose related object is not registered. A client reads at most 500 characters of that message, and for a second-level relation, a dotted key and an empty condition the refusal printed its route after "NOT applied", so with an ordinary field name the route was cut off. Each refusal now reads: where the condition sits, the short reason, what to write instead, then "NOT applied" and any reasoning. The head names the column after the path by its type (`beneath the lookup field`), and the two-step route reads `then match its ids: …`; on a multi-valued relation the ids route reads `an $or of { "FIELD": { "$contains": ID } }, one per id` (the cap refusal uses the same spelling). + +Both refusal heads — this one and the no-operator-object refusal's — now name the condition's or object's first key and count the rest (`keys "region", and 2 more`) instead of listing three, so with long object, field and key names every route still lands inside the cut. + +The cap, the truncation, the code and the status are unchanged, and so is the set of filters refused. diff --git a/packages/objectql/src/engine-nested-relation-lowering.test.ts b/packages/objectql/src/engine-nested-relation-lowering.test.ts index de4e8d97740..ea6bb834d14 100644 --- a/packages/objectql/src/engine-nested-relation-lowering.test.ts +++ b/packages/objectql/src/engine-nested-relation-lowering.test.ts @@ -302,7 +302,7 @@ describe('[#20802] the nested-relation form is lowered at the engine\'s where se const err = await refusalOf(engine.find(OBJECT, { where })); expect(envelopeOf(err), JSON.stringify(where)).toEqual(INVALID_FILTER); expect(err!.message, JSON.stringify(where)).toContain('puts a nested-relation condition ('); - expect(err!.message, JSON.stringify(where)).toContain(`at ${path}, beneath the declared`); + expect(err!.message, JSON.stringify(where)).toMatch(new RegExp(`at ${path.replace(/[$[\]().]/g, '\\$&')}, beneath the (lookup|tree) field, and `)); expect(err!.message, JSON.stringify(where)).toContain(words); expect(err!.message, JSON.stringify(where)).toContain('The filter was NOT applied.'); expect(rec.reads, JSON.stringify(where)).toHaveLength(0); diff --git a/packages/objectql/src/no-operator-object-door.ts b/packages/objectql/src/no-operator-object-door.ts index 58f30588710..c06bf275a4e 100644 --- a/packages/objectql/src/no-operator-object-door.ts +++ b/packages/objectql/src/no-operator-object-door.ts @@ -251,12 +251,16 @@ export interface NoOperatorObjectRefusal { readonly aggregated: boolean; } -/** How the object is named in the words: its keys, never its values. */ +/** + * How the object is named in the words: its keys, never its values — the + * first, and a count of the rest. [#27351] Every position and kind prints this + * head before its route, and the path already places the object, so listing + * more keys would cost the route its room inside the client cap. + */ function describeObject(keys: readonly string[]): string { if (keys.length === 0) return 'an empty object {}'; - const shown = keys.slice(0, 3).map((key) => JSON.stringify(key)).join(', '); - const more = keys.length > 3 ? `, and ${keys.length - 3} more` : ''; - return `an object with no operator key (keys ${shown}${more})`; + const more = keys.length > 1 ? `, and ${keys.length - 1} more` : ''; + return `an object with no operator key (keys ${JSON.stringify(keys[0])}${more})`; } /** diff --git a/packages/objectql/src/no-operator-object-prescription-cap.test.ts b/packages/objectql/src/no-operator-object-prescription-cap.test.ts index 4cda612d16e..aced51a3464 100644 --- a/packages/objectql/src/no-operator-object-prescription-cap.test.ts +++ b/packages/objectql/src/no-operator-object-prescription-cap.test.ts @@ -57,8 +57,12 @@ function clientReads(fn: () => unknown): { status: number; code: unknown; text: return { status, code: body.code, text: String(body.error), full: (thrown as Error).message }; } -/** The route is whole in what the client reads, ends MARGIN inside the cut, and precedes "NOT applied". */ -function expectRouteInside(fn: () => unknown, [open, close]: readonly [string, string]): void { +/** The route is whole in what the client reads, ends by `limit` (MARGIN inside the cut), and precedes "NOT applied". */ +function expectRouteInside( + fn: () => unknown, + [open, close]: readonly [string, string], + limit = CLIENT_CUT - MARGIN, +): void { const read = clientReads(fn); expect(read.status).toBe(400); expect(read.code).toBe('INVALID_FILTER'); @@ -68,8 +72,8 @@ function expectRouteInside(fn: () => unknown, [open, close]: readonly [string, s expect(closeAt, read.full).toBeGreaterThan(start); const end = closeAt + close.length; expect(read.text, 'the client text holds the whole route').toContain(read.full.slice(start, end)); - expect(end, `the route ends at ${end}, within ${MARGIN} of the ${CLIENT_CUT}-character cut`) - .toBeLessThanOrEqual(CLIENT_CUT - MARGIN); + expect(end, `the route ends at ${end}; the limit is ${limit} (the cut is ${CLIENT_CUT})`) + .toBeLessThanOrEqual(limit); const notApplied = read.full.indexOf(' was NOT applied'); expect(notApplied, read.full).toBeGreaterThan(end - 1); } @@ -106,3 +110,59 @@ describe('[#27337] the no-operator-object refusal: every route lands inside the expectRouteInside(having('contract_value_amount', 'number'), SCALAR_ROUTE); }); }); + +/** + * [#27351] The stress fixture: a 27-character object and target, 32-character + * fields, a four-key object and a path through `aggregations[12].filter` and a + * three-deep `$and` / `$or` / `$and`. The shared head (context, field, keys, + * path) is long enough here that every route must still end inside the cut — + * the head names the object's first key and counts the rest. Against the head + * as it stood on `origin/main` `cf218eed1` (three keys shown), six rows end + * past the cut (501–532). + */ +describe('[#27351] the no-operator-object refusal: the stress fixture keeps every route inside the cut', () => { + const STRESS_OBJECT = 'customer_support_case_entry'; + const STRESS_TARGET = 'enterprise_customer_account'; + const STRESS_VALUE = { region_code: 'x', service_tier: 'x', account_status: 'x', billing_country: 'x' }; + const F = { + lookup: 'primary_customer_account_manager', + multi: 'secondary_account_manager_owners', + json: 'integration_metadata_payload_raw', + address: 'primary_shipping_destination_adr', + number: 'contract_value_amount_in_dollars', + } as const; + const STRESS_SCHEMA = { + fields: { + [F.lookup]: { name: F.lookup, type: 'lookup', reference: STRESS_TARGET }, + [F.multi]: { name: F.multi, type: 'lookup', reference: STRESS_TARGET, multiple: true }, + [F.json]: { name: F.json, type: 'json' }, + [F.address]: { name: F.address, type: 'address' }, + [F.number]: { name: F.number, type: 'number' }, + amount: { name: 'amount', type: 'number' }, + }, + }; + const deep = (field: string) => ({ + $and: [{ amount: 1 }, { $or: [{ $and: [{ amount: 2 }, { [field]: STRESS_VALUE }] }] }], + }); + const at = { + where: (field: string) => () => narrowNumberComparands(STRESS_OBJECT, 'find', STRESS_SCHEMA, deep(field)), + filter: (field: string) => () => + narrowNumberComparands(STRESS_OBJECT, 'aggregate', STRESS_SCHEMA, deep(field), 'aggregations[12].filter'), + having: (field: string, type: string) => () => + narrowHavingNumberComparands(STRESS_OBJECT, { [field]: STRESS_VALUE }, new Map(), new Map([[field, type]])), + }; + + it('every kind, at every position it is refused', () => { + expectRouteInside(at.filter(F.lookup), SINGLE_LOOKUP_ROUTE, CLIENT_CUT); + expectRouteInside(at.having(F.lookup, 'lookup'), SINGLE_LOOKUP_ROUTE, CLIENT_CUT); + expectRouteInside(at.filter(F.multi), MULTI_LOOKUP_ROUTE, CLIENT_CUT); + for (const kind of ['json', 'address'] as const) { + expectRouteInside(at.where(F[kind]), JSON_ROUTE, CLIENT_CUT); + expectRouteInside(at.filter(F[kind]), JSON_ROUTE, CLIENT_CUT); + expectRouteInside(at.having(F[kind], kind), JSON_ROUTE, CLIENT_CUT); + } + expectRouteInside(at.where(F.number), SCALAR_ROUTE, CLIENT_CUT); + expectRouteInside(at.filter(F.number), SCALAR_ROUTE, CLIENT_CUT); + expectRouteInside(at.having(F.number, 'number'), SCALAR_ROUTE, CLIENT_CUT); + }); +}); diff --git a/packages/objectql/src/protocol-explicit-filter-field-gate.test.ts b/packages/objectql/src/protocol-explicit-filter-field-gate.test.ts index 295003a1229..8d4ff3788ea 100644 --- a/packages/objectql/src/protocol-explicit-filter-field-gate.test.ts +++ b/packages/objectql/src/protocol-explicit-filter-field-gate.test.ts @@ -401,7 +401,7 @@ describe('#7534 — unknown field on the EXPLICIT filter axes (real ObjectQL eng // descends: the answer is not its INVALID_FIELD about `region`. const err: any = await find({ where: { owner_id: { region: 'NA' } } }).then(() => null, (e: any) => e); expect({ code: err?.code, status: err?.status }).toEqual({ code: 'INVALID_FILTER', status: 400 }); - expect(err.message).toContain("puts a nested-relation condition (keys \"region\") at where.owner_id, beneath the declared lookup field 'owner_id'"); + expect(err.message).toContain("puts a nested-relation condition (keys \"region\") at where.owner_id, beneath the lookup field,"); expect(err.field).toBeUndefined(); }); diff --git a/packages/objectql/src/relation-condition-prescription-cap.test.ts b/packages/objectql/src/relation-condition-prescription-cap.test.ts new file mode 100644 index 00000000000..e59e0ec4dc9 --- /dev/null +++ b/packages/objectql/src/relation-condition-prescription-cap.test.ts @@ -0,0 +1,154 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#27351] The nested-relation condition refusal at `where` states what to + * write instead INSIDE the 500 characters a client reads, for every reason it + * refuses a condition. + * + * The cap stays: `mapDataError` (`@objectstack/types`) cuts a 4xx message at + * `CLIENT_MESSAGE_MAX` with `…`. So `relation-filter-lowering.ts` orders each + * reason's words head, the short reason, the route, then "NOT applied" and any + * reasoning, and this file reads every reason the way a client does — through + * `mapDataError` — and requires the whole route in what survives the cut, + * ahead of "NOT applied". Run against the words as they stood on `origin/main` + * `cf218eed1`, the second-level and dotted-key routes are cut on the card's + * fixture, and every reason's route is cut on the stress fixture. + * + * Two fixtures. The card's — a 21-character object, a 21-character field, a + * 16-character target, three keys, a path through a nested `$and` / `$or` — + * must end each route {@link MARGIN} characters inside the cut. The stress + * one — a 27-character object and target, a 32-character field, four 11- to + * 15-character keys, a three-deep path, single- and multi-valued — must keep + * each route whole inside the cut. + */ + +import { describe, it, expect } from 'vitest'; +import { mapDataError } from '@objectstack/types'; +import { narrowNumberComparands } from './number-comparand-declared-type-door.js'; + +/** The characters a client reads before the `…`: `CLIENT_MESSAGE_MAX - 1`. */ +const CLIENT_CUT = 499; +/** How far inside the cut every route on the card's fixture must end. */ +const MARGIN = 40; + +interface Fixture { + readonly object: string; + readonly target: string; + readonly field: string; + readonly keys: readonly string[]; + readonly multiple: boolean; + /** Wraps the condition in the fixture's path. */ + readonly wrap: (field: string, condition: unknown) => unknown; + /** The latest a route may end. */ + readonly limit: number; +} + +const CARD: Fixture = { + object: 'customer_support_case', + target: 'customer_account', + field: 'primary_account_owner', + keys: ['region', 'tier', 'status'], + multiple: false, + wrap: (field, condition) => ({ $and: [{ amount: 1 }, { $or: [{ [field]: condition }] }] }), + limit: CLIENT_CUT - MARGIN, +}; + +const STRESS: Fixture = { + object: 'customer_support_case_entry', + target: 'enterprise_customer_account', + field: 'primary_customer_account_manager', + keys: ['region_code', 'service_tier', 'account_status', 'billing_country'], + multiple: false, + wrap: (field, condition) => ({ + $and: [{ amount: 1 }, { $or: [{ $and: [{ amount: 2 }, { [field]: condition }] }] }], + }), + limit: CLIENT_CUT, +}; + +const STRESS_MULTI: Fixture = { ...STRESS, multiple: true }; + +/** The opening and the closing words of each reason's route. */ +function routes(f: Fixture): Record { + const ids = f.multiple ? '"$contains": ID } }, one per id.' : '"$in": [ID, …] } }.'; + return { + secondLevel: [`Read '${f.target}' with { "parent_account": { … } }`, ids], + dottedKey: [`Read '${f.target}' with that condition`, ids], + empty: ['Name one (', '"$null": false } }.'], + undeclaredKey: ['Name a field it declares', '"FIELD": VALUE } }.'], + unregistered: ['Match ', ids], + }; +} + +/** The refusal a client reads: status, code, the bounded `error` text, and the full message. */ +function clientReads(fn: () => unknown): { status: number; code: unknown; text: string; full: string } { + let thrown: unknown; + try { fn(); } catch (e) { thrown = e; } + expect(thrown, 'the door refused').toBeInstanceOf(Error); + const { status, body } = mapDataError(thrown); + return { status, code: body.code, text: String(body.error), full: (thrown as Error).message }; +} + +/** The route is whole in what the client reads, ends by the fixture's limit, and precedes "NOT applied". */ +function expectRouteInside(f: Fixture, fn: () => unknown, [open, close]: readonly [string, string]): void { + const read = clientReads(fn); + expect(read.status).toBe(400); + expect(read.code).toBe('INVALID_FILTER'); + const start = read.full.indexOf(open); + expect(start, read.full).toBeGreaterThan(0); + const closeAt = read.full.indexOf(close, start); + expect(closeAt, read.full).toBeGreaterThan(start); + const end = closeAt + close.length; + expect(read.text, 'the client text holds the whole route').toContain(read.full.slice(start, end)); + expect(end, `the route ends at ${end}; the limit is ${f.limit}`).toBeLessThanOrEqual(f.limit); + const notApplied = read.full.indexOf(' was NOT applied'); + expect(notApplied, read.full).toBeGreaterThan(end - 1); +} + +/** Refuse `condition` beneath the fixture's relation field, at `where`. */ +function refusal(f: Fixture, condition: Record, registered = true) { + const related = { + fields: { + ...Object.fromEntries(f.keys.map((key) => [key, { name: key, type: 'text' }])), + parent_account: { name: 'parent_account', type: 'lookup', reference: f.target }, + }, + }; + const schema = { + fields: { + [f.field]: { name: f.field, type: 'lookup', reference: f.target, ...(f.multiple ? { multiple: true } : {}) }, + amount: { name: 'amount', type: 'number' }, + }, + }; + const schemaOf = (name: string) => (registered && name === f.target ? related : undefined); + return () => narrowNumberComparands(f.object, 'find', schema, f.wrap(f.field, condition), 'where', { schemaOf }); +} + +const conditionOf = (f: Fixture) => Object.fromEntries(f.keys.map((key) => [key, 'x'])); + +describe('[#27351] the nested-relation condition refusal: every route lands inside the client cap', () => { + for (const [label, f] of [['the card', CARD], ['stress, single-valued', STRESS], ['stress, multi-valued', STRESS_MULTI]] as const) { + describe(label, () => { + const route = routes(f); + const condition = conditionOf(f); + + it('a second-level relation', () => { + expectRouteInside(f, refusal(f, { ...condition, parent_account: { [f.keys[0]]: 'x' } }), route.secondLevel); + }); + + it('a dotted key', () => { + expectRouteInside(f, refusal(f, { ...condition, 'parent_account.name': 'x' }), route.dottedKey); + }); + + it('an empty condition', () => { + expectRouteInside(f, refusal(f, {}), route.empty); + }); + + it('a key the related object does not declare', () => { + expectRouteInside(f, refusal(f, { ...condition, regio: 'x' }), route.undeclaredKey); + }); + + it('a related object not registered here', () => { + expectRouteInside(f, refusal(f, condition, false), route.unregistered); + }); + }); + } +}); diff --git a/packages/objectql/src/relation-filter-lowering.ts b/packages/objectql/src/relation-filter-lowering.ts index 868138675b4..2f1d2a495ff 100644 --- a/packages/objectql/src/relation-filter-lowering.ts +++ b/packages/objectql/src/relation-filter-lowering.ts @@ -218,26 +218,38 @@ export function lowerRelationSite(site: RelationFilterSite, ids: readonly unknow }; } -/** How the words name the condition: its keys, never its values. */ +/** + * How the words name the condition: its keys, never its values — the first, + * and a count of the rest. [#27351] Every reason that is about one key names + * that key in its own words, and the path already places the condition, so + * listing more keys would cost the route its room inside the client cap. + */ function describeKeys(keys: readonly string[]): string { if (keys.length === 0) return 'no keys'; - const shown = keys.slice(0, 3).map((key) => JSON.stringify(key)).join(', '); - const more = keys.length > 3 ? `, and ${keys.length - 3} more` : ''; - return `keys ${shown}${more}`; + const more = keys.length > 1 ? `, and ${keys.length - 1} more` : ''; + return `keys ${JSON.stringify(keys[0])}${more}`; } /** The ids route every driver serves: `$in` on a single-valued relation, `$contains` per id on a multi-valued one. */ function idsRoute(field: string, multiple: boolean): string { return multiple - ? `{ "${field}": { "$contains": ID } } for one id, an $or of those for several` + ? `an $or of { "${field}": { "$contains": ID } }, one per id` : `{ "${field}": { "$in": [ID, …] } }`; } /** * The words of a structural refusal ({@link admitRelationCondition}): the - * position, the verdict and the route first — the REST door bounds a 4xx - * message at 500 characters by truncation — and nothing after the route that a - * caller needs. + * position, the short reason and the route first — the REST door bounds a 4xx + * message at 500 characters by truncation — then "NOT applied" and any + * reasoning, which is the part a cut may take. + * + * [#27351] The route comes right after the short reason, ahead of "NOT + * applied", for every reason — the rule the temporal, text-operator and + * no-operator-object doors follow. The head names the field once by name and + * once at the end of its path, so the column after the path is named by its + * type alone, and the route names the field only in its spelling. + * `relation-condition-prescription-cap.test.ts` pins each reason's route + * inside the cap. */ export function relationConditionRefusalMessage(refusal: RelationConditionRefusal, context: string): string { const { field, type, target, multiple, path, key } = refusal; @@ -245,9 +257,9 @@ export function relationConditionRefusalMessage(refusal: RelationConditionRefusa const named = target === undefined ? 'the related object' : `'${target}'`; const head = `${context}: filter on '${field}' puts a nested-relation condition (${describeKeys(refusal.keys)}) at ` - + `${path}, beneath the declared ${type} field '${field}'`; + + `${path}, beneath the ${type} field`; const verdict = 'The filter was NOT applied.'; - const twoStep = `then match '${field}' against its ids: ${idsRoute(field, multiple)}.`; + const twoStep = `then match its ids: ${idsRoute(field, multiple)}.`; switch (refusal.reason) { case 'unregistered-target': return ( @@ -255,30 +267,31 @@ export function relationConditionRefusalMessage(refusal: RelationConditionRefusa + (target === undefined ? ', and the field declares no related object to read. ' : `, and no object '${target}' is registered here to read. `) - + `${verdict} Match '${field}' against ids you hold: ${idsRoute(field, multiple)}.` + + `Match ids you hold: ${idsRoute(field, multiple)}. ${verdict}` ); case 'empty': return ( - `${head}, and it names no field of ${related}. ${verdict} Name one ` - + `({ "${field}": { "FIELD": VALUE } }), or test that '${field}' has a value: ` - + `{ "${field}": { "$null": false } }.` + `${head}, and it names no field of ${related}. Name one ` + + `({ "${field}": { "FIELD": VALUE } }), or test for a value: { "${field}": { "$null": false } }. ` + + verdict ); case 'undeclared-key': return ( - `${head}, and '${key}' is not a field of ${related}. ${verdict} Name a field it declares: ` - + `{ "${field}": { "FIELD": VALUE } }. A key it does not declare would read as a real empty answer.` + `${head}, and '${key}' is not a field of ${related}. Name a field it declares: ` + + `{ "${field}": { "FIELD": VALUE } }. ${verdict} A key it does not declare would read as a real empty ` + + 'answer.' ); case 'dotted-key': return ( - `${head}, and '${key}' is a dotted path: the condition reaches one level only. ${verdict} ` - + `Read ${named} with that condition yourself, ${twoStep}` + `${head}, and '${key}' is a dotted path: the condition reaches one level only. ` + + `Read ${named} with that condition, ${twoStep} ${verdict}` ); case 'second-level': default: return ( - `${head}, and '${key}' is itself a ${refusal.keyType ?? 'relation'} field of ${related} holding a ` - + `condition of its own: one level only. ${verdict} Read ${named} with { "${key}": { … } } yourself, ` - + twoStep + `${head}, and '${key}' holds a condition of its own: one level only. Read ${named} with ` + + `{ "${key}": { … } }, ${twoStep} ${verdict} '${key}' is itself a ` + + `${refusal.keyType ?? 'relation'} field of ${related}.` ); } }