From 6ff4d0ba7c98edb45df7936ee186af7bbd9e415b Mon Sep 17 00:00:00 2001 From: Jack Zhuang <277994282+os-zhuang@users.noreply.github.com> Date: Fri, 31 Jul 2026 12:40:28 +0800 Subject: [PATCH] fix(data): a dotted-path sort is refused, not silently unapplied (#4256) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The one sort shape #4226 deliberately left open. `?sort=account.company_name` passed the gate on its head segment (a real field) and was then unusable by every driver: SqlDriver rendered `"account"."company_name"` against a table that was never joined and the #3821 backstop retried WITHOUT the sort; Mongo and the memory driver resolved the path against the row, where a foreign key is a scalar id. 200, every row present, arbitrary order — which `top` then sliced into an arbitrary "latest N". Now `400 INVALID_SORT` at the shared normalizer, with the message split by what the head segment is: a relationship head names the relation it tried to cross and prescribes denormalising the value onto the queried object (formula / rollup field); a non-reference head states the contract (sort reaches whole columns, not values inside them). Unknown heads keep the #4226 typo answer, reported first — the same precedence the expand gate uses. A survey of framework, objectui and cloud found zero callers emitting a dotted sort; objectui's column-header sort structurally cannot produce one (lookup columns are keyed by their flat field name, relations load via $expand). Blast radius is hand-authored requests — exactly the callers the silent degradation was misleading. Co-Authored-By: Claude Fable 5 --- .changeset/sort-dotted-path-rejected.md | 37 ++++++++++ content/docs/api/data-api.mdx | 3 +- content/docs/data-modeling/queries.mdx | 8 ++- .../docs/protocol/objectql/query-syntax.mdx | 20 +++--- packages/metadata-protocol/src/protocol.ts | 65 +++++++++++++---- .../src/query-expression-conformance.test.ts | 71 +++++++++++++++++++ 6 files changed, 176 insertions(+), 28 deletions(-) create mode 100644 .changeset/sort-dotted-path-rejected.md diff --git a/.changeset/sort-dotted-path-rejected.md b/.changeset/sort-dotted-path-rejected.md new file mode 100644 index 0000000000..bc84257ac2 --- /dev/null +++ b/.changeset/sort-dotted-path-rejected.md @@ -0,0 +1,37 @@ +--- +"@objectstack/metadata-protocol": patch +--- + +fix(data): a dotted-path `sort` (`?sort=account.company_name`) is rejected with `400 INVALID_SORT`, not silently unapplied (#4256) + +The one sort shape #4226 deliberately left open is now closed. A dotted path +passed the sort gate on its head segment (`account` is a real field) and was +then unusable by every driver: `SqlDriver` handed it to Knex, which rendered +`"account"."company_name"` against a table that was never joined, and the +#3821 unknown-column backstop retried **without the sort**; Mongo and the +memory driver resolved the path against the row itself, where a foreign key is +a scalar id. Result: `200`, every row present, arbitrary order — and since +`sort` + `top` is how a caller asks for "the latest N", an arbitrary N with +nothing in the response to reveal it. + +The rejection distinguishes the two mistakes a dotted path can be: + +- a head that IS a relationship (`project_id.name`) — the message names the + relationship it tried to cross and prescribes the supported alternative: + denormalise the value onto the queried object (formula or rollup field) and + sort by that; +- a head that is not (`title.length`) — the message states the contract: sort + reaches only whole columns of the queried object, not values inside them. + +An unknown head (`no_such.title`) keeps the existing typo-shaped answer, and a +list carrying both mistakes reports the typo first — the same precedence the +expand gate uses. + +**What changes for callers:** requests whose sort crosses a relationship now +fail loudly instead of receiving an ordinary-looking 200 over unordered rows. +A survey of framework, objectui and cloud found zero callers emitting a dotted +sort (objectui's column-header sort keys lookup columns by their flat field +name and loads relations via `$expand`), so the practical blast radius is +hand-authored requests — exactly the callers the silent degradation was +misleading. Internal callers reaching `engine.find()` directly are unaffected, +the same tiering every #4226 gate uses. diff --git a/content/docs/api/data-api.mdx b/content/docs/api/data-api.mdx index 1f2a6d9a27..f49f0c6c3d 100644 --- a/content/docs/api/data-api.mdx +++ b/content/docs/api/data-api.mdx @@ -20,7 +20,7 @@ Query records with filtering, sorting, selection, and pagination. | `object` | path | Object name | | `select` | query | Comma-separated field names. Every name must exist — an unknown one is `400 INVALID_FIELD`, never dropped. | | `filter` | query | Filter expression (JSON). `filters` also accepted for backward compatibility. Malformed JSON is rejected with `400 INVALID_FILTER` — never ignored. | -| `sort` | query | Sort expression (e.g. `name asc` or `-created_at`). Must name a real field — otherwise `400 INVALID_SORT`. | +| `sort` | query | Sort expression (e.g. `name asc` or `-created_at`). Must name a real field on the object itself — an unknown name or a dotted path (`account.company_name`) is `400 INVALID_SORT`. | | `top` | query | Max records to return. No default — omitting it returns all matching records. | | `skip` | query | Offset | | `expand` | query | Comma-separated list of relations to eager-load. Must name a reference field (`lookup` / `master_detail` / `user` / `tree`) — otherwise `400 INVALID_FIELD`. | @@ -88,6 +88,7 @@ wrong — three more responses that looked exactly like successful ones: |:---|:---| | `?sort=-created_at` | sorts | | `?sort=no_such_field` | `400 INVALID_SORT` | +| `?sort=account.company_name` | `400 INVALID_SORT` — sort reaches only the object's own columns; denormalise the related value (formula/rollup field) to sort by it | | `?sort={oops` / `?sort=title:desc` | `400 INVALID_SORT` — the list route spells a direction with a space (`title desc`) or a leading `-` | | `?select=id,title` | projects those two columns | | `?select=no_such_field`, `?select=title,no_such_field` | `400 INVALID_FIELD` | diff --git a/content/docs/data-modeling/queries.mdx b/content/docs/data-modeling/queries.mdx index 219f0313eb..0fe5d1587b 100644 --- a/content/docs/data-modeling/queries.mdx +++ b/content/docs/data-modeling/queries.mdx @@ -219,9 +219,11 @@ Sort results with `orderBy` (array of sort nodes): Over the REST/protocol ingress, `orderBy` also accepts `'-created_at'`, `['-created_at']` and `{created_at: 'desc'}`, all normalized to the node array -above. A sort naming a field the object does not have — or an `order` that is -neither `asc` nor `desc` — is `400 INVALID_SORT` there rather than a dropped -sort. Internal callers reaching `engine.find()` directly are unaffected. +above. A sort naming a field the object does not have, an `order` that is +neither `asc` nor `desc`, or a dotted path into a related record +(`account.company_name` — sort reaches only the queried object's own columns) +is `400 INVALID_SORT` there rather than a dropped sort. Internal callers +reaching `engine.find()` directly are unaffected. --- diff --git a/content/docs/protocol/objectql/query-syntax.mdx b/content/docs/protocol/objectql/query-syntax.mdx index 3eb59416f2..c3c61fffcc 100644 --- a/content/docs/protocol/objectql/query-syntax.mdx +++ b/content/docs/protocol/objectql/query-syntax.mdx @@ -454,16 +454,16 @@ const query: QueryAST = { ### Sorting on Related Fields -`orderBy` only reaches columns of the queried table. A dotted path -(`account.company_name`) is handed to Knex verbatim and renders as -`"account"."company_name"`, which the database rejects as an unknown column; -`SqlDriver.find()` then retries **without the sort**, so the rows come back unordered -rather than sorted — and no error surfaces. Denormalise the value onto the queried -object (for example with a formula or rollup field) when you need to sort by it. - -This is the one sort that still degrades silently: the REST ingress judges a -dotted path on its **head segment** (`account` here, a real field), so the -`INVALID_SORT` gate passes it through to the driver backstop. +`orderBy` only reaches columns of the queried table. Over the REST/protocol +ingress a dotted path (`account.company_name`) is `400 INVALID_SORT` (#4256): +no driver can order by it — `SqlDriver` would render it as +`"account"."company_name"` against a table that was never joined, and until the +path was refused, the unknown-column backstop retried **without the sort** and +answered 200 with unordered rows. Denormalise the value onto the queried object +(for example with a formula or rollup field) when you need to sort by it. + +Internal callers reaching `engine.find()` directly are unaffected: a dotted +`orderBy` there still falls through to the driver backstop and orders nothing. --- diff --git a/packages/metadata-protocol/src/protocol.ts b/packages/metadata-protocol/src/protocol.ts index 63a108f2b2..fa678bbe71 100644 --- a/packages/metadata-protocol/src/protocol.ts +++ b/packages/metadata-protocol/src/protocol.ts @@ -884,7 +884,9 @@ function unusableFilterError(param: string, detail: string): Error { /** * [#4226] A sort the normalizer cannot turn into a usable `SortNode[]`, or one - * that names a field the object does not have. + * that names a field the object does not have — or, since #4256, a dotted path + * (`account.company_name`) that would have to cross into a related record no + * driver joins for. * * Carries `INVALID_SORT` — the standard-catalog code (`errors.zod.ts`, * "Invalid sort specification") that had sat in the catalog with no emitter @@ -3244,26 +3246,61 @@ export class ObjectStackProtocolImplementation implements * The colon form gets its own hint: `?sort=title:desc` is the spelling * `GET /data/:object/export` accepts, and a caller who moved between the * two routes deserves better than "no such field 'title:desc'". + * + * [#4256] A dotted path (`?sort=account.company_name`) is refused on the + * same terms — the last sort shape that still degraded silently after + * #4226. Its head segment being a real field is what carried it past the + * unknown-field check while no driver could then order by it: `SqlDriver` + * hands the path to Knex, which renders `"account"."company_name"` against + * a table that was never joined, and the #3821 unknown-column backstop + * retries WITHOUT the sort; Mongo and the memory driver resolve the path + * against the row itself, where a foreign key is a scalar id, so every + * value is missing and the ordering is a no-op. Unknown heads keep the + * typo-shaped rejection above (reported first, like the expand gate's + * `unknown` > `not-a-reference` precedence); a dotted path on a real head + * gets a message that says which relationship it tried to cross and + * prescribes what `query-syntax.mdx` has prescribed since #4240: + * denormalise the value onto the queried object and sort by that. */ private assertSortFieldsExist(object: string, orderBy: ReadonlyArray<{ field: string }>, param: string): void { if (orderBy.length === 0) return; const gate = this.resolveQueryFields(object); if (!gate) return; - const unknown = orderBy - .map((s) => String(s.field)) - .filter((f) => !gate.known.has(f.split('.')[0])); - if (unknown.length === 0) return; - const first = unknown[0]; - const hint = first.includes(':') - ? ` The list route spells a direction with a space or a leading '-'` - + ` ('sort=${first.split(':')[0]} desc', 'sort=-${first.split(':')[0]}');` - + " 'field:direction' is the export route's spelling." - : suggestFieldName(first, gate.declared); + const names = orderBy.map((s) => String(s.field)); + const unknown = names.filter((f) => !gate.known.has(f.split('.')[0])); + if (unknown.length > 0) { + const first = unknown[0]; + const hint = first.includes(':') + ? ` The list route spells a direction with a space or a leading '-'` + + ` ('sort=${first.split(':')[0]} desc', 'sort=-${first.split(':')[0]}');` + + " 'field:direction' is the export route's spelling." + : suggestFieldName(first, gate.declared); + throw invalidSortError( + param, + `sorts by '${first}', which is not a field on object '${object}'` + + (unknown.length > 1 ? ` (also: ${unknown.slice(1).join(', ')})` : ''), + { hint, extra: { field: first, fields: unknown, object } }, + ); + } + const dotted = names.filter((f) => f.includes('.')); + if (dotted.length === 0) return; + const first = dotted[0]; + const head = first.split('.')[0]; + const headDef: any = gate.fields[head]; + const crossesRelation = headDef != null && REFERENCE_VALUE_TYPES.has(headDef.type); throw invalidSortError( param, - `sorts by '${first}', which is not a field on object '${object}'` - + (unknown.length > 1 ? ` (also: ${unknown.slice(1).join(', ')})` : ''), - { hint, extra: { field: first, fields: unknown, object } }, + (crossesRelation + ? `sorts by '${first}', which follows the relationship '${head}' into another object — ` + + `sort reaches only columns of '${object}' itself` + : `sorts by '${first}', a dotted path — sort reaches only whole columns of '${object}', ` + + "not values inside them") + + (dotted.length > 1 ? ` (also: ${dotted.slice(1).join(', ')})` : ''), + { + hint: ` Denormalise the value onto '${object}' (a formula or rollup field that` + + ' copies it into a real column) and sort by that.', + extra: { field: first, fields: dotted, object }, + }, ); } diff --git a/packages/objectql/src/query-expression-conformance.test.ts b/packages/objectql/src/query-expression-conformance.test.ts index f77a6ba9a0..7097712800 100644 --- a/packages/objectql/src/query-expression-conformance.test.ts +++ b/packages/objectql/src/query-expression-conformance.test.ts @@ -349,6 +349,77 @@ describe('#4226 — sort / select / expand on the list path (real ObjectQL engin })).rejects.toMatchObject({ code: 'INVALID_SORT', param: 'sort' }); }); + // ───────────────────────────────────────────────────────────── + // SORT — dotted paths (#4256) + // ───────────────────────────────────────────────────────────── + + it('the foreign-key column itself still sorts — the gate is about the dot, not the relationship', async () => { + await expect(protocol.findData({ object: 'showcase_task', query: { sort: 'project_id' } })) + .resolves.toMatchObject({ records: expect.any(Array) }); + await expect(protocol.findData({ object: 'showcase_task', query: { sort: '-parent_id' } })) + .resolves.toMatchObject({ records: expect.any(Array) }); + }); + + it.each([ + ['bare string', { sort: 'project_id.name' }], + ['descending', { sort: '-project_id.name' }], + ['second of two', { sort: 'title,project_id.name' }], + ['string array', { orderBy: ['project_id.name'] }], + ['SortNode array', { orderBy: [{ field: 'project_id.name', order: 'desc' }] }], + ['direction map', { orderBy: { 'project_id.name': 'desc' } }], + ['OData spelling', { $orderby: 'project_id.name' }], + ])('a dotted path into a related object is a 400, not insertion order — %s', async (_label, query) => { + // The head segment (`project_id`) is a real field, which is exactly + // what carried this shape past the #4226 gate while no driver could + // then order by it: SQL renders `"project_id"."name"` against a table + // that was never joined and the #3821 backstop retries WITHOUT the + // sort; Mongo and the memory driver resolve the path against the row, + // where the foreign key is a scalar id. The last sort response that + // looked applied and was not. + await expect(protocol.findData({ object: 'showcase_task', query })) + .rejects.toMatchObject({ + status: 400, + code: 'INVALID_SORT', + field: 'project_id.name', + object: 'showcase_task', + }); + }); + + it('the dotted rejection names the relationship it tried to cross and prescribes the fix', async () => { + await expect(protocol.findData({ object: 'showcase_task', query: { sort: 'project_id.name' } })) + .rejects.toThrow(/follows the relationship 'project_id'[\s\S]*formula or rollup/); + }); + + it('a dotted path under a non-reference head is refused on the same axis, minus the relationship claim', async () => { + // `title.length` reaches into a VALUE, not a related record. Telling + // this caller they "followed a relationship" would be false — `title` + // holds text — so the message states the contract instead. + await expect(protocol.findData({ object: 'showcase_task', query: { sort: 'title.length' } })) + .rejects.toMatchObject({ status: 400, code: 'INVALID_SORT', field: 'title.length' }); + await expect(protocol.findData({ object: 'showcase_task', query: { sort: 'title.length' } })) + .rejects.toThrow(/whole columns/); + }); + + it('an unknown head keeps the typo answer — dotted precedence mirrors the expand gate', async () => { + // `?sort=no_such.title` was a 400 before this gate existed (judged on + // its head segment) and must keep reading as a typo, not as a + // relationship crossing; a list carrying both mistakes reports the + // typo first, like expand's `unknown` > `not-a-reference`. + await expect(protocol.findData({ object: 'showcase_task', query: { sort: 'no_such.title' } })) + .rejects.toThrow(/not a field on object/); + await expect(protocol.findData({ + object: 'showcase_task', query: { sort: 'no_such.title,project_id.name' }, + })).rejects.toMatchObject({ code: 'INVALID_SORT', field: 'no_such.title' }); + }); + + it('the "latest N by a related column" footgun is closed too', async () => { + // `?sort=-project_id.created_at&top=2` used to answer 200 with an + // arbitrary 2 of 5 — indistinguishable from the real latest-2. + await expect(protocol.findData({ + object: 'showcase_task', query: { sort: '-project_id.created_at', top: 2 }, + })).rejects.toMatchObject({ status: 400, code: 'INVALID_SORT', field: 'project_id.created_at' }); + }); + // ───────────────────────────────────────────────────────────── // SELECT — control group, then rejected // ─────────────────────────────────────────────────────────────