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
37 changes: 37 additions & 0 deletions .changeset/sort-dotted-path-rejected.md
Original file line number Diff line number Diff line change
@@ -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.
3 changes: 2 additions & 1 deletion content/docs/api/data-api.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -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`. |
Expand Down Expand Up @@ -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` |
Expand Down
8 changes: 5 additions & 3 deletions content/docs/data-modeling/queries.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -219,9 +219,11 @@ Sort results with `orderBy` (array of sort nodes):
<Callout type="info">
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.
</Callout>

---
Expand Down
20 changes: 10 additions & 10 deletions content/docs/protocol/objectql/query-syntax.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -460,16 +460,16 @@ const query: QueryAST = {
### Sorting on Related Fields

<Callout type="warn">
`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.
</Callout>

---
Expand Down
65 changes: 51 additions & 14 deletions packages/metadata-protocol/src/protocol.ts
Original file line number Diff line number Diff line change
Expand Up @@ -894,7 +894,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
Expand Down Expand Up @@ -3254,26 +3256,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 },
},
);
}

Expand Down
71 changes: 71 additions & 0 deletions packages/objectql/src/query-expression-conformance.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
// ─────────────────────────────────────────────────────────────
Expand Down
Loading