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

fix(data): `searchFields` / `groupBy` / `aggregations` naming a field that does not exist are rejected, not silently degraded (#4254)

#4226 closed `sort` / `select` / `expand`; with the filter axis (#4134 / #4164 /
#4181 / #4121) that made four field-naming read axes that either apply or fail.
The same machine kept leaking on the remaining three, and each failure corrupted
something the closed axes never touched:

```
search=alpha&searchFields=no_such -> 200 MORE rows than the narrowing allowed
groupBy=[no_such] -> 200 [{no_such: null, n: <true count>}] N groups collapsed into 1
sum(no_such) -> 200 0 — indistinguishable from a real zero
```

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

- **`searchFields` → `400 INVALID_FIELD`.** The `select` failure with the sign
flipped outward: the engine dropped unknown names and, when that emptied the
override, fell back to the FULL searchable set — so a parameter that exists
only to narrow a search widened it, and it changed which ROWS came back, not
just which columns. Its only in-framework caller is `GET /data/:object/export`
— the route whose `search` support just shipped so exports would stop
downloading "the unsearched superset … in a file that looks authoritative";
a typo'd `searchFields` did exactly that, one parameter over. Three causes,
three messages, because the fixes differ (the split #4226 drew on expand): a
name that is no field is a request typo; a REAL field outside the searchable
set needs the object changed (its message names the declared
`searchableFields` or the auto-default's type rule, whichever applies); and
a `searchableFields` entry that names no field is a STALE DECLARATION — a
bug on the object, called out as such because clients (objectui's list
search) echo the declaration verbatim. The allowed set is resolved by the
same `@objectstack/spec/data` function the engine's search expansion
consumes (`resolveSearchFieldResolution`, moved from objectql), so the gate
cannot drift from what search actually scans.
- **`groupBy` → `400 INVALID_FIELD`.** The in-memory aggregation path projects
an unknown column as `null` for every row, so all rows landed in ONE bucket
whose count is the true row count — structurally perfect, identical to "this
column really holds a single value". A chart draws one bar; nothing says the
grouping never ran. Native SQL aggregation errors on the same input, so which
backend a deployment sits on decided the answer — the "two routes, opposite
answers" split, one axis over.
- **`aggregations` → `400 INVALID_FIELD`.** `sum(<typo>)` folded a column of
`undefined` to `0` — the exact number an empty quarter produces, in reports
whose whole job is to be believed (`avg`/`min`/`max` answered `null` the same
way). `count` with no `field` (or the `'*'` sentinel) is the one legitimate
field-less form and passes.
- **Unreadable SHAPES on the aggregation axes → `400 INVALID_QUERY`** — the
standard-catalog code that had no emitter since it was written, like
`INVALID_SORT` before #4226. A string `groupBy`, an entry naming no field, a
function or `dateGranularity` outside the spec enums, a missing `alias`: each
slipped past the `Array.isArray` routing guard (rows returned UNGROUPED) or
computed a silent placeholder (`null` results, a column keyed `"undefined"`,
one bucket per raw value under an unknown granularity).

Tiering is unchanged from #4226: registry + field map present → authoritative;
no registry / no field map / legacy array field map → the NAME gates skip (shape
gates still apply — they need no schema). The engine's own tolerance is
untouched: internal callers reaching `engine.find()` / `engine.aggregate()`
directly are unaffected. `@objectstack/rest` also stops logging
`INVALID_FILTER` / `INVALID_SORT` / `INVALID_QUERY` rejections as
"[REST] Unhandled error" — they are client mistakes the response already
explains, as `INVALID_FIELD` always was.

Requests that name real fields are unaffected.
43 changes: 42 additions & 1 deletion content/docs/api/data-api.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,8 @@ Query records with filtering, sorting, selection, and pagination.
| `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`. |
| `search` | query | Full-text search query |
| `search` | query | Full-text search term, scanned case-insensitively across the object's searchable fields (its declared `searchableFields`, or a text-like auto-default when none are declared). |
| `searchFields` | query | Comma-separated subset of the searchable fields for `search` to scan — narrows the scan, never widens it. A name outside the searchable set is `400 INVALID_FIELD`. |

> **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 @@ -116,6 +117,46 @@ 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.

#### Neither is a search narrowing, a grouping, or an aggregation

The last three field-naming axes follow the same rule. Each of these used to
answer `200` with something that looked exactly like a served query — and each
corrupts something the earlier axes do not:

| Request | Result |
|:---|:---|
| `?search=alpha&searchFields=title` | scans only `title` |
| `?search=alpha&searchFields=no_such_field` | `400 INVALID_FIELD` |
| `?search=alpha&searchFields=amount` | `400 INVALID_FIELD` — real field, but not searchable |
| `groupBy: ["status"]` | one bucket per status value |
| `groupBy: ["no_such_field"]` | `400 INVALID_FIELD` |
| `aggregations: [{function:"sum", field:"amount", alias:"total"}]` | the real total |
| `aggregations: [{function:"sum", field:"no_such_field", alias:"total"}]` | `400 INVALID_FIELD` |
| `aggregations: [{function:"count", alias:"n"}]` | `count(*)` — the one legitimate field-less form |

- **`searchFields`** — the only parameter whose failure changed which **rows**
came back. An unknown name used to be dropped, and an override left empty
fell back to scanning *every* searchable column: a parameter that exists
only to **narrow** a search failed by **widening** it. Three causes get
three messages, because the fixes differ: a name that is no field (a typo in
the request), a real field outside the searchable set (declare it in
`searchableFields`), and a `searchableFields` entry that names no field (a
stale declaration — the bug is on the object, and clients that echo the
declaration verbatim are told so).
- **`groupBy`** — an unknown column projected `null` for every row, so all
rows fell into **one bucket** whose count is the true row count:
structurally perfect, indistinguishable from a column that really holds a
single value. A chart draws one bar and nothing says the grouping never ran.
- **`aggregations`** — `sum` over an unknown column folded blanks to **`0`**,
the exact number a genuinely empty quarter produces (`avg`/`min`/`max`
answered `null` the same way), in reports whose whole job is to be believed.

A `groupBy` / `aggregations` value the spec cannot read at all — a bare string
instead of an array, an entry that names no field, a function or date
granularity outside the spec's enums, a missing `alias` — is
`400 INVALID_QUERY`: those shapes were silently ignored, returning **ungrouped
raw rows** with nothing to say the aggregation never happened.

**Response**:
```json
{
Expand Down
21 changes: 18 additions & 3 deletions content/docs/api/error-catalog.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,10 @@ ObjectStack uses a structured error system with **9 error categories** and **53
**Cause:** A field name in the request does not exist on the target object. On a
list read this also covers an unreserved query parameter — `GET /data/:object`
reads those as field filters, so one naming no field could only match zero
records and is rejected rather than answered with an empty page.
records and is rejected rather than answered with an empty page — plus every
other read axis that names a field: `select`, `expand` (a real field that holds
no reference gets its own message), `searchFields` (a real field outside the
searchable set gets its own message), `groupBy`, and `aggregations[].field`.
**Fix:** Check the object schema for valid field names. Use `os meta get object <name>` to inspect the object's fields. If the name was meant as a
*parameter* rather than a field, use the real one — page size is `top` / `$top`
/ `limit`, not `pageSize` / `perPage`; the response's `error` names the
Expand Down Expand Up @@ -117,8 +120,20 @@ substitute. See the [Data API](/docs/api/data-api).
**Retry:** `no_retry`

### `INVALID_QUERY`
**Cause:** The query structure is malformed.
**Fix:** Check the query syntax against the [Query Cheat Sheet](/docs/data-modeling/queries).
**Cause:** A `groupBy` / `aggregations` value the spec cannot read: a bare
string where an array belongs, an entry that names no field, an aggregation
function or `dateGranularity` outside the spec's enums, or a missing `alias`.
(Field names that simply don't exist on the object are `INVALID_FIELD`
instead.)
**Fix:** Check the aggregation shapes against the
[Query Cheat Sheet](/docs/data-modeling/queries) — e.g.
`{ "groupBy": ["status"], "aggregations": [{ "function": "sum", "field":
"amount", "alias": "total" }] }`. `count` is the only function that may omit
`field`.
**Why it is an error and not an empty result:** every one of these shapes used
to be silently ignored or mis-read — rows came back **ungrouped**, or an
unknown function computed `null` — in a response indistinguishable from a
served aggregation.
**Retry:** `no_retry`

### `INVALID_FILTER`
Expand Down
24 changes: 24 additions & 0 deletions content/docs/data-modeling/queries.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -364,6 +364,19 @@ three (only `count`/`sum`/`avg`/`min`/`max` are mapped), and the in-memory drive
silently returns `null` for them. Avoid these three on SQL- or memory-backed objects.
</Callout>

<Callout type="info">
Over the REST/protocol ingress, `groupBy` and `aggregations` are validated
before they reach any driver. A field the object does not have is
`400 INVALID_FIELD` — the in-memory fallback used to collapse an unknown
`groupBy` column into one `null`-keyed bucket, and to answer `sum(<typo>)`
with `0`. A value the spec cannot read (a non-array, an entry naming no
field, a function or `dateGranularity` outside the enums above, a missing
`alias`) is `400 INVALID_QUERY` — those shapes used to be ignored, returning
ungrouped raw rows. `count` with no `field` (or `field: "*"`) is the one
legitimate field-less form and passes. Internal callers reaching
`engine.aggregate()` directly are unaffected.
</Callout>

### Aggregation Example

```typescript
Expand Down Expand Up @@ -449,6 +462,17 @@ Only `query` and `fields` are implemented. The engine expands `search` into a dr
`operator`.
</Callout>

<Callout type="info">
`fields` (and its query-parameter spelling, `?searchFields=` / `?$searchFields=`) can only
**narrow** the scan within the server-resolved searchable set — the object's declared
`searchableFields`, or a text-like auto-default when none are declared. Over the
REST/protocol ingress a name outside that set is `400 INVALID_FIELD`, with distinct
messages for a field that does not exist and a real field that is not searchable. The
engine used to drop unknown names and fall back to the full searchable set — a parameter
that exists to narrow a search, silently widening it. Internal callers reaching
`engine.find()` directly are unaffected.
</Callout>

### Pinyin recall (Chinese deployments)

When pinyin search is enabled (`OS_SEARCH_PINYIN_ENABLED` — auto-on when the stack's
Expand Down
18 changes: 17 additions & 1 deletion content/docs/protocol/objectql/query-syntax.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -616,6 +616,18 @@ FROM opportunity
GROUP BY stage
```

<Callout type="info">
Over the REST/protocol ingress, both axes are validated before any driver
runs: a `groupBy` entry or `aggregations[].field` naming a field the object
does not have is `400 INVALID_FIELD` (the in-memory fallback used to collapse
every row into one `null`-keyed bucket, and to answer `sum(<typo>)` with `0`),
and a shape the spec cannot read — a non-array, an entry naming no field, an
unknown function or `dateGranularity`, a missing `alias` — is
`400 INVALID_QUERY`. `count` with no `field` (or `field: '*'`) is the one
legitimate field-less form. Internal callers reaching `engine.aggregate()`
directly are unaffected.
</Callout>

### Aggregation Functions

```typescript
Expand Down Expand Up @@ -753,7 +765,11 @@ const query: QueryAST = {
Field resolution is server-side and never client-trusted: `search.fields` is
**intersected** with the object's declared `searchableFields` (or, absent those, an
auto-default of the name field plus short-text/enum fields), so naming a field outside
that set does not widen the search. Multiple whitespace-separated terms are AND-ed and
that set can never widen the search — and over the REST/protocol ingress it is
`400 INVALID_FIELD` outright (#4254), because the engine-side intersection alone used to
drop the unknown name and fall back to scanning the full searchable set. Internal
callers reaching `engine.find()` directly keep the tolerant intersection.
Multiple whitespace-separated terms are AND-ed and
fields are OR-ed. Case sensitivity is the **driver's**, not the expansion's: the
expansion emits a plain `$contains`, which `SqlDriver` compiles to a parameterised
`LIKE '%…%'` with no case folding — so the dialect's own `LIKE`/collation rules decide —
Expand Down
Loading
Loading