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
69 changes: 69 additions & 0 deletions .changeset/rest-list-sort-select-expand-rejected.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
---
"@objectstack/metadata-protocol": patch
"@objectstack/objectql": patch
---

fix(data): `sort` / `select` / `expand` naming a field that does not exist are rejected, not silently dropped (#4226)

The list path has four axes on which a caller names a field. `filter` was
closed over #4134 / #4164 / #4181 / #4121 — a filter the server cannot apply is
now a 400, never a 200 over the wrong rows. The other three still leaked, all
answering `200`:

```
sort=no_such_field -> 200 CAEBD byte-identical to "no sort at all"
select=no_such_field -> 200 <every field> asked for one column, got all of them
expand=no_such_rel -> 200 <no such key> no relation, no complaint
```

Each is now refused at the shared normalizer, so `GET /data/:object`,
`GET /data/:object/:id`, `POST /data/:object/query`, the export route and the
runtime dispatcher give one answer instead of five.

- **`sort` → `400 INVALID_SORT`.** The row set is unchanged, so this is not
#4181's "returned everything" — it is worse in one specific way: `sort` +
`top` is how a caller asks for "the latest N", and a dropped sort makes that
an arbitrary N that nothing in the response reveals. This is the list half of
the bug #4181 fixed on the export route's `orderby`. `INVALID_SORT` had sat
in the standard catalog since it was written with no emitter.
- **`select` → `400 INVALID_FIELD`.** `engine.find()` drops unknown columns
(deliberate `SELECT *` tolerance) and then falls back to `*` when that empties
the projection, and the two compose into `?select=<typo>` asking for ONE
column and receiving EVERY column — a parameter whose purpose is to return
less, failing by returning more, against both FLS and data minimisation. The
partially-unknown case (`?select=title,no_such`) is refused on the same terms:
half a projection is not the one that was asked for, and the tolerant reading
would have to explain why `?status=<typo>` is a 400 and `?select=<typo>` is
not, on one endpoint, about one field map.
- **`expand` → `400 INVALID_FIELD`.** The lightest of the three — same rows,
same columns, the relation simply is not there — but the response cannot be
told apart from "every foreign key is null", and the client renders raw ids
where names belong. A name that is no field at all and a name that is a field
holding no reference (`?expand=title`) get different messages, since the fixes
differ.

**Sorts that were silently never applied now are.** Two wire spellings reached
the normalizer and fell through it untouched, and every driver then declined
them (`SqlDriver` guards its ORDER BY with `Array.isArray(orderBy)`): the
client SDK's own declared `orderBy: string[]`, and the `{field: direction}` map
that `GET /data/:object/export`, `GET /data/import/jobs` and objectui's calendar
all emit. Both are now folded to `SortNode[]` — so the import-job history, which
has asked for `created_at desc` since it was written and served insertion order,
sorts. A sort shape that still cannot be read (a number, an entry naming no
field, a direction that is neither `asc` nor `desc`) is `400 INVALID_SORT`
rather than a silent no-op.

**`$expand` of a `tree` field works.** `REFERENCE_VALUE_TYPES` lists `tree`
among the types whose value "points at another record … the related record
object in expanded form", and objectui requests it, but
`engine.expandRelatedRecords` tested membership with a hand-copied `!==` chain
that omitted it — so a hierarchy field came back as a raw parent id. The loop
now reads the shared spec set, which is also what the new expand gate validates
against, so the gate cannot admit a field the engine then skips.

**What changes for callers:** requests naming a non-existent field in `sort`,
`select` or `expand` now fail loudly instead of receiving an unsorted, widened
or unexpanded response. Every axis naming real fields is unaffected. The
engine's own tolerance is untouched — it guards internal callers (hooks, flows,
expand sub-reads, registry-less hosts) that never pass through this ingress,
the same tiering the object-existence and unknown-field gates already use.
47 changes: 42 additions & 5 deletions content/docs/api/data-api.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,12 @@ Query records with filtering, sorting, selection, and pagination.
| Parameter | Location | Description |
|:----------|:---------|:------------|
| `object` | path | Object name |
| `select` | query | Comma-separated field names |
| `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`) |
| `sort` | query | Sort expression (e.g. `name asc` or `-created_at`). Must name a real field — otherwise `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 |
| `expand` | query | Comma-separated list of relations to eager-load. Must name a reference field (`lookup` / `master_detail` / `user` / `tree`) — otherwise `400 INVALID_FIELD`. |
| `search` | query | Full-text search query |

> **Note:** OData-style `$`-prefixed parameters (`$filter`, `$select`, `$orderby`, `$top`, `$skip`, `$expand`, `$count`, `$search`) are also accepted directly on this same endpoint as aliases — they're normalized internally to the parameter names above. There is no separate standalone OData endpoint.
Expand Down Expand Up @@ -78,6 +78,43 @@ successful query:

The same rule applies to `orderby` on `GET /data/:object/export`.

#### Nor is a sort, a projection, or an expansion

`filter` is not the only parameter that names a field. `sort`, `select` and
`expand` do too, and each one used to be dropped in silence when the name was
wrong — three more responses that looked exactly like successful ones:

| Request | Result |
|:---|:---|
| `?sort=-created_at` | sorts |
| `?sort=no_such_field` | `400 INVALID_SORT` |
| `?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=owner_id` | expands the reference |
| `?expand=no_such_rel` | `400 INVALID_FIELD` — no such field |
| `?expand=title` | `400 INVALID_FIELD` — real field, but it holds no reference |

Why each one matters, since none of them changes which rows match:

- **`sort`** — `sort` + `top` is how you ask for "the latest N". A sort that is
dropped turns that into an **arbitrary** N, and nothing in the response says so.
- **`select`** — an unknown column used to be dropped, and a projection left with
no known column fell back to *every* column: a parameter that exists to return
**less** failed by returning **more**.
- **`expand`** — an unexpanded relation is indistinguishable from one whose
foreign keys are all null, so clients render raw ids where names belong.

Sorts accept any of these spellings, all equivalent:
`?sort=-created_at`, `?$orderby=-created_at`, and — on
`POST /data/:object/query` — `{"orderBy": [{"field": "created_at", "order":
"desc"}]}`, `{"orderBy": ["-created_at"]}` or `{"orderBy": {"created_at":
"desc"}}`. A shape that is none of these (a number, an entry naming no field,
a direction that is neither `asc` nor `desc`) is `400 INVALID_SORT`.

`GET /data/:object/:id` applies the same `select` and `expand` rules, so the
list and single-record routes cannot disagree about one field map.

**Response**:
```json
{
Expand All @@ -96,8 +133,8 @@ Get a single record by ID. Only `select` and `expand` query parameters are allow
|:----------|:---------|:------------|
| `object` | path | Object name |
| `id` | path | Record ID |
| `select` | query | Comma-separated field names to include |
| `expand` | query | Comma-separated list of relations to eager-load |
| `select` | query | Comma-separated field names to include. Unknown name → `400 INVALID_FIELD`. |
| `expand` | query | Comma-separated list of relations to eager-load. Not a reference field → `400 INVALID_FIELD`. |

**Response**: `{ object: "account", id: "1", record: { ... } }`

Expand Down
17 changes: 16 additions & 1 deletion content/docs/data-modeling/queries.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -216,6 +216,14 @@ Sort results with `orderBy` (array of sort nodes):
| `field` | `string` | Field name to sort by |
| `order` | `'asc' \| 'desc'` | Sort direction |

<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.
</Callout>

---

## Pagination
Expand Down Expand Up @@ -288,13 +296,20 @@ and you get every column; if it doesn't, the error is rethrown. Either way there
`owner.name` key.

Use `expand` to pull in a relationship's fields instead.

Both of these are `engine.find()` behaviours, reached by internal callers. Over
the REST/protocol ingress a projection column that names no field at all is
`400 INVALID_FIELD` — including the case where *no* requested column is known,
which used to fall back to `SELECT *` and answer a one-column request with every
column.
</Callout>

---

## Expand (Related Records)

Load related records through `lookup`, `master_detail`, and `user` fields with
Load related records through the reference field types — `lookup`,
`master_detail`, `user` and `tree` (`REFERENCE_VALUE_TYPES`) — with
`expand`. Each key is a relationship field name; the value is a nested query that
can select fields, filter, and expand further (max depth 3 — a fixed constant, not
configurable).
Expand Down
14 changes: 13 additions & 1 deletion content/docs/protocol/objectql/query-syntax.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -415,6 +415,14 @@ const opportunities = await engine.find('opportunity', {

Sorting uses the **`orderBy`** array of `SortNode` objects.

<Callout type="info">
Over the REST/protocol ingress, `orderBy` also accepts `'-created_at'`,
`['-created_at']` and `{created_at: 'desc'}` — all normalized to the
`SortNode[]` above. A sort naming a field the object does not have is
`400 INVALID_SORT` there, rather than being dropped. Internal callers reaching
`engine.find()` directly are unaffected.
</Callout>

### Single Field Sort

```typescript
Expand Down Expand Up @@ -449,13 +457,17 @@ const query: QueryAST = {
`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.
</Callout>

---

## 4. Relationships (Expand)

The `expand` property enables **recursive loading of related records** through `lookup`, `master_detail`, and `user` fields. Each key is a relationship field name; the value is a nested `QueryAST`.
The `expand` property enables **recursive loading of related records** through the reference field types — `lookup`, `master_detail`, `user` and `tree` (`REFERENCE_VALUE_TYPES`). Each key is a relationship field name; the value is a nested `QueryAST`. Over the REST/protocol ingress, a key that is not one of those is `400 INVALID_FIELD` rather than a silently absent relation.

### Basic Expand

Expand Down
2 changes: 1 addition & 1 deletion content/docs/references/api/odata.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,7 @@ const result = ODataConfig.parse(data);
| **$orderby** | `string \| string[]` | optional | Sort order |
| **$top** | `integer` | optional | Max results to return |
| **$skip** | `integer` | optional | Results to skip |
| **$expand** | `string \| string[]` | optional | Navigation properties to expand (lookup/master_detail fields) |
| **$expand** | `string \| string[]` | optional | Navigation properties to expand (reference fields: lookup/master_detail/user/tree) |
| **$count** | `boolean` | optional | Include total count |
| **$search** | `string` | optional | Search expression |
| **$format** | `Enum<'json' \| 'xml' \| 'atom'>` | optional | Response format |
Expand Down
Loading
Loading