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
14 changes: 14 additions & 0 deletions .changeset/27351-relation-condition-route-first.md
Original file line number Diff line number Diff line change
@@ -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.
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
12 changes: 8 additions & 4 deletions packages/objectql/src/no-operator-object-door.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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})`;
}

/**
Expand Down
68 changes: 64 additions & 4 deletions packages/objectql/src/no-operator-object-prescription-cap.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand All @@ -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);
}
Expand Down Expand Up @@ -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);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -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();
});

Expand Down
154 changes: 154 additions & 0 deletions packages/objectql/src/relation-condition-prescription-cap.test.ts
Original file line number Diff line number Diff line change
@@ -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<string, readonly [string, string]> {
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<string, unknown>, 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);
});
});
}
});
Loading
Loading