diff --git a/.changeset/rest-list-search-groupby-aggregations-rejected.md b/.changeset/rest-list-search-groupby-aggregations-rejected.md new file mode 100644 index 0000000000..c91bfc7cc4 --- /dev/null +++ b/.changeset/rest-list-search-groupby-aggregations-rejected.md @@ -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: }] 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()` 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. diff --git a/content/docs/api/data-api.mdx b/content/docs/api/data-api.mdx index f49f0c6c3d..e720c5ba2a 100644 --- a/content/docs/api/data-api.mdx +++ b/content/docs/api/data-api.mdx @@ -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. @@ -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 { diff --git a/content/docs/api/error-catalog.mdx b/content/docs/api/error-catalog.mdx index 4f612c8f09..0bcd2567b1 100644 --- a/content/docs/api/error-catalog.mdx +++ b/content/docs/api/error-catalog.mdx @@ -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 ` 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 @@ -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` diff --git a/content/docs/data-modeling/queries.mdx b/content/docs/data-modeling/queries.mdx index 582f18b5c7..c19f79ab03 100644 --- a/content/docs/data-modeling/queries.mdx +++ b/content/docs/data-modeling/queries.mdx @@ -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. + +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()` +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. + + ### Aggregation Example ```typescript @@ -449,6 +462,17 @@ Only `query` and `fields` are implemented. The engine expands `search` into a dr `operator`. + +`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. + + ### Pinyin recall (Chinese deployments) When pinyin search is enabled (`OS_SEARCH_PINYIN_ENABLED` — auto-on when the stack's diff --git a/content/docs/protocol/objectql/query-syntax.mdx b/content/docs/protocol/objectql/query-syntax.mdx index 447babd145..e76fad45c6 100644 --- a/content/docs/protocol/objectql/query-syntax.mdx +++ b/content/docs/protocol/objectql/query-syntax.mdx @@ -616,6 +616,18 @@ FROM opportunity GROUP BY stage ``` + +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()` 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. + + ### Aggregation Functions ```typescript @@ -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 — diff --git a/packages/metadata-protocol/src/protocol.ts b/packages/metadata-protocol/src/protocol.ts index 74a36aa611..e60513c234 100644 --- a/packages/metadata-protocol/src/protocol.ts +++ b/packages/metadata-protocol/src/protocol.ts @@ -18,7 +18,14 @@ import type { } from '@objectstack/spec/api'; import type { MetadataCacheRequest, MetadataCacheResponse, ServiceInfo, ApiRoutes, WellKnownCapabilities } from '@objectstack/spec/api'; import { readServiceSelfInfo } from '@objectstack/spec/api'; -import { parseFilterAST, isFilterAST, VALID_AST_OPERATORS, REFERENCE_VALUE_TYPES, RPC_QUERY_ALIAS_SLOTS, foldQueryAliasSlots, type QueryAliasConflict, type QueryAliasSlot, type DroppedFieldsEvent, type QueryAST } from '@objectstack/spec/data'; +import { + parseFilterAST, isFilterAST, VALID_AST_OPERATORS, REFERENCE_VALUE_TYPES, + AggregationFunction, DateGranularity, resolveSearchFieldResolution, + SEARCHABLE_TEXTUAL_TYPES, SEARCHABLE_ENUM_TYPES, SEARCH_AUTO_EXCLUDED_FIELDS, + RPC_QUERY_ALIAS_SLOTS, foldQueryAliasSlots, + type QueryAliasConflict, type QueryAliasSlot, + type DroppedFieldsEvent, type QueryAST, +} from '@objectstack/spec/data'; import { PLURAL_TO_SINGULAR, SINGULAR_TO_PLURAL } from '@objectstack/spec/shared'; import { applyConversionsToStoredItem } from '@objectstack/spec'; import { type FormView, isAggregatedViewContainer } from '@objectstack/spec/ui'; @@ -795,6 +802,17 @@ const QUERY_AST_KEYS: Readonly> = { distinct: true, expand: true, }; +/** + * [#4254] The two aggregation vocabularies, read off the SPEC's own enums so a + * function or granularity added there is admitted here without a second edit — + * the same both-directions pinning `QUERY_AST_KEYS` gets from `keyof QueryAST`. + * They exist because the in-memory aggregation path answers an unknown member + * with a silent placeholder (`null` result / raw-value buckets) rather than an + * error, so the ingress must be the layer that refuses one. + */ +const AGGREGATION_FUNCTIONS: ReadonlySet = new Set(AggregationFunction.options); +const DATE_GRANULARITIES: ReadonlySet = new Set(DateGranularity.options); + /** * [#4134] Every query-parameter name `findData` consumes itself, consulted * AFTER the alias normalization in `findData` has run — so the wire spellings @@ -971,6 +989,39 @@ function invalidSortError( return err; } +/** + * [#4254] An aggregation-axis value (`groupBy` / `aggregations`) whose SHAPE + * the spec's `QueryAST` cannot read — a non-array, an entry that names no + * field, a function or granularity outside the spec enums, a missing alias. + * + * Carries `INVALID_QUERY` — the standard-catalog code (`errors.zod.ts`, + * "Malformed query syntax") that had sat in the catalog with no emitter since + * it was written, exactly as `INVALID_SORT` had before #4226. Shape mistakes + * get their own code because they are not about any FIELD: `INVALID_FIELD` on + * these axes is reserved for a well-formed entry naming a column the object + * does not have. + * + * The message spells out what the dropped/misread value used to do, because + * that is the part the caller cannot infer: every one of these shapes was + * silently ignored (rows returned ungrouped) or silently mis-answered + * (`null` aggregates, one raw-value bucket per row) with an ordinary 200. + */ +function invalidQueryError( + param: string, + detail: string, + opts?: { hint?: string, extra?: Record }, +): Error { + const err: any = new Error( + `Query parameter '${param}' ${detail}.` + + (opts?.hint ?? ''), + ); + err.status = 400; + err.code = 'INVALID_QUERY'; + err.param = param; + Object.assign(err, opts?.extra ?? {}); + return err; +} + /** * [#4226] Every wire spelling of `sort`/`orderBy`, folded to the one shape the * QueryAST declares (`SortNodeSchema[]`) — or a rejection. @@ -3295,7 +3346,7 @@ export class ObjectStackProtocolImplementation implements * object-existence gate above already warns once when the registry itself * is missing, so this stays quiet rather than warning twice per process. */ - private resolveQueryFields(object: string): { known: ReadonlySet, declared: readonly string[], fields: any } | null { + private resolveQueryFields(object: string): { known: ReadonlySet, declared: readonly string[], fields: any, schema: any } | null { const schema: any = this.engine?.registry?.getObject?.(object); const declared = schema?.fields; if (!declared || typeof declared !== 'object') return null; @@ -3310,7 +3361,7 @@ export class ObjectStackProtocolImplementation implements known.add('id'); known.add('created_at'); known.add('updated_at'); - return { known, declared: fieldNames, fields: declared }; + return { known, declared: fieldNames, fields: declared, schema }; } /** @@ -3564,6 +3615,391 @@ export class ObjectStackProtocolImplementation implements throw err; } + /** + * [#4254] SEARCH-FIELDS axis. A `searchFields` override naming something + * `search` cannot scan is refused (`400 INVALID_FIELD`). + * + * This axis is the `select` failure with the sign flipped OUTWARD. The + * engine's `resolveSearchFields` drops unknown names and, when that leaves + * the override empty, falls back to the FULL allowed set — the exact + * two-step #4226 closed on projections, except that where a widened + * projection returns extra columns, a widened search returns extra ROWS: + * `?search=alpha&searchFields=` matched rows the caller's narrowing + * excluded, in a response with nothing to distinguish it from a satisfied + * one. `searchFields` exists only to narrow (ADR-0061: the override is + * "intersected with the allowed set — it can narrow the scan, never widen + * it"), so failing open to a wider scan is the one direction it must never + * take. Its only in-framework caller today is `GET /data/:object/export` — + * the same route whose `search` support just shipped precisely so an + * export would stop downloading "the unsearched superset … in a file that + * looks authoritative". + * + * Two rejections, one code, different messages, because the fixes differ + * (the same split the expand axis draws): a name that is no field at all + * is a typo, while a REAL field outside the searchable set needs the + * OBJECT changed — added to a declared `searchableFields`, or declared + * searchable at all when the auto-default excludes its type. The allowed + * set itself comes from {@link resolveSearchFieldResolution} in + * `@objectstack/spec/data` — the same function the engine's search + * expansion consumes — so this gate cannot admit a field the engine would + * then decline to scan, nor refuse one it would. + * + * Names are judged EXACTLY (no dotted-head tolerance): the engine + * intersects the override with the allowed set by exact string, so + * `owner_id.name` — plausible from the select/sort axes — would be + * silently dropped there, and this gate letting it through would + * reintroduce the fallback it exists to close. + */ + private assertSearchFieldsAreSearchable(object: string, requested: unknown, param: string): void { + // Shape first, BEFORE the field-map tiering below — same order as the + // projection gate (#4196): a registry-less host, which skips the name + // checks, still must not carry an unreadable override to an engine + // that would ignore it and scan the default set. + let names: readonly string[]; + if (typeof requested === 'string') { + names = requested.split(',').map((s) => s.trim()).filter(Boolean); + } else if (Array.isArray(requested)) { + const badShape = requested.findIndex((f) => typeof f !== 'string'); + if (badShape !== -1) { + const err: any = new Error( + `'${param}' entry #${badShape + 1} on object '${object}' is not a field name. ` + + `'${param}' narrows which columns 'search' scans, as a comma-separated string ` + + 'or an array of field names.', + ); + err.code = 'INVALID_FIELD'; + err.status = 400; + err.object = object; + err.param = param; + throw err; + } + names = requested; + } else { + const err: any = new Error( + `'${param}' on object '${object}' must be a comma-separated string or an array of ` + + `field names, received ${requested === null ? 'null' : typeof requested}. It narrows ` + + "which columns 'search' scans; a value the server cannot read would have been " + + 'ignored, leaving the search over the DEFAULT columns instead.', + ); + err.code = 'INVALID_FIELD'; + err.status = 400; + err.object = object; + err.param = param; + throw err; + } + // An empty override is ABSENT — the engine falls through to the + // allowed set for it, which for a caller who named nothing is the + // answer they asked for, not a widened one. + if (names.length === 0) return; + const gate = this.resolveQueryFields(object); + if (!gate) return; + const { allowed, source } = resolveSearchFieldResolution({ + fields: gate.fields, + searchableFields: gate.schema?.searchableFields, + // [ADR-0079] Same precedence the engine's search expansion applies: + // `nameField` is canonical, `displayNameField` the honored alias. + displayField: gate.schema?.nameField ?? gate.schema?.displayNameField, + }); + const allowedSet = new Set(allowed); + // A name in the object's own `searchableFields` that names no field is + // a STALE DECLARATION — a bug on the OBJECT, not on the request. It + // matters because clients echo the declaration verbatim (objectui's + // list search sends `$searchFields: schema.searchableFields`), so + // calling it "unknown" would send the caller hunting a typo they + // never made. Same split the expand axis draws for a lookup whose + // `reference` was never authored. + const declaredSet = new Set(Array.isArray(gate.schema?.searchableFields) ? gate.schema.searchableFields : []); + const unknown = names.filter((n) => !allowedSet.has(n) && !gate.known.has(n) && !declaredSet.has(n)); + const staleDeclared = names.filter((n) => !allowedSet.has(n) && !gate.known.has(n) && declaredSet.has(n)); + const unsearchable = names.filter((n) => !allowedSet.has(n) && gate.known.has(n)); + const [offenders, reason] = + unknown.length > 0 ? [unknown, 'unknown' as const] + : staleDeclared.length > 0 ? [staleDeclared, 'stale-declared' as const] + : [unsearchable, 'unsearchable' as const]; + if (offenders.length === 0) return; + const first = offenders[0]; + let detail: string; + if (reason === 'stale-declared') { + detail = `Field '${first}' on object '${object}' is declared in 'searchableFields' but ` + + 'does not exist' + + (offenders.length > 1 ? ` (also: ${offenders.slice(1).join(', ')})` : '') + + '. The declaration is stale — searching it can never match, and the engine ' + + "silently skipped it. Fix the object's 'searchableFields' to name real fields."; + } else if (reason === 'unknown') { + // A dotted path is a special unknown: plausible vocabulary from the + // select/sort axes, but search scans this object's own columns. + const dottedHint = first.includes('.') && gate.known.has(first.split('.')[0]) + ? " 'search' scans this object's own columns; a related record's column cannot be a search target." + : suggestFieldName(first, gate.declared); + detail = `Unknown field '${first}' on object '${object}'` + + (offenders.length > 1 ? ` (also: ${offenders.slice(1).join(', ')})` : '') + + `. '${param}' narrows which columns 'search' scans, so a name the object does not ` + + 'declare cannot narrow anything — and the engine used to drop it and scan the ' + + 'default columns instead, answering a NARROWER search with a WIDER one.' + + dottedHint; + } else if (source === 'declared') { + detail = `Field '${first}' on object '${object}' is not searchable` + + (offenders.length > 1 ? ` (also: ${offenders.slice(1).join(', ')})` : '') + + `. The object declares 'searchableFields' (${allowed.join(', ')}), which is the set ` + + "'search' scans — a field outside it cannot be a search target until it is added there."; + } else { + const meta = gate.fields[first]; + const why = !meta || SEARCH_AUTO_EXCLUDED_FIELDS.has(first) + ? 'a system/audit column, which the default never includes' + : meta.hidden + ? 'hidden' + : `type '${meta.type}'`; + detail = `Field '${first}' on object '${object}' is not searchable` + + (offenders.length > 1 ? ` (also: ${offenders.slice(1).join(', ')})` : '') + + `. With no 'searchableFields' declared, 'search' scans the text-like columns ` + + `(${[...SEARCHABLE_TEXTUAL_TYPES, ...SEARCHABLE_ENUM_TYPES].join(' / ')}), and '${first}' is ` + + why + + ". Declare 'searchableFields' on the object to choose the searchable set explicitly."; + } + const err: any = new Error(detail); + err.code = 'INVALID_FIELD'; + err.status = 400; + err.field = first; + err.fields = offenders; + err.object = object; + err.param = param; + throw err; + } + + /** + * [#4254] GROUP-BY axis. A grouping target the object does not have is + * refused (`400 INVALID_FIELD`); a grouping target the spec cannot read is + * refused as a shape (`400 INVALID_QUERY`). + * + * The failure this closes is the quietest of the family: the in-memory + * aggregation path projects an unknown column as `null` for every row, so + * ALL rows land in one bucket — `groupBy=[]` answered + * `[{ : null, n: }]`, a structurally perfect result + * identical to "this column really holds a single value". A chart draws + * one bar; nothing anywhere says the grouping never ran. And the answer + * depended on which backend a deployment happens to sit on: a driver with + * native aggregation hands `GROUP BY ` to its database instead + * (whose refusal `SqlDriver` may or may not surface), while the in-memory + * fallback invents the one-bucket result — the "two routes, opposite + * answers" split #4226 closed, relocated one axis over. Refusing at the + * shared ingress is what makes the two paths agree. + * + * Names are judged EXACTLY, not by dotted head: the aggregation contract + * groups by THIS object's columns (`row[field]` verbatim on the in-memory + * path, a bare column reference in pushed-down SQL), so a dotted path can + * only ever produce the null bucket. + */ + private assertGroupByFieldsExist(object: string, groupBy: unknown): void { + if (groupBy === undefined || groupBy === null) return; + if (!Array.isArray(groupBy)) { + throw invalidQueryError( + 'groupBy', + `must be an array of grouping targets (a field name, or { field, dateGranularity } ` + + `for date bucketing), received ${typeof groupBy}`, + { + hint: ' A value the server cannot read used to be ignored — the rows came back ' + + 'UNGROUPED, looking exactly like a query that never asked for grouping. ' + + `Send e.g. { "groupBy": ["status"] } in the 'POST /data/:object/query' body.`, + extra: { object }, + }, + ); + } + if (groupBy.length === 0) return; + const fieldsToCheck: string[] = []; + for (let i = 0; i < groupBy.length; i++) { + const entry = groupBy[i]; + if (typeof entry === 'string') { + fieldsToCheck.push(entry); + continue; + } + if (entry === null || typeof entry !== 'object' || Array.isArray(entry)) { + throw invalidQueryError( + 'groupBy', + `entry #${i + 1} on object '${object}' is not a grouping target — expected a ` + + `field name or { field, dateGranularity }, received ` + + `${entry === null ? 'null' : Array.isArray(entry) ? 'an array' : typeof entry}`, + { extra: { object } }, + ); + } + if (typeof entry.field !== 'string' || entry.field.length === 0) { + throw invalidQueryError( + 'groupBy', + `entry #${i + 1} on object '${object}' names no field — the structured form is ` + + `{ field, dateGranularity?, alias? }`, + { extra: { object } }, + ); + } + if (entry.dateGranularity !== undefined && !DATE_GRANULARITIES.has(entry.dateGranularity)) { + throw invalidQueryError( + 'groupBy', + `entry #${i + 1} on object '${object}' buckets by '${String(entry.dateGranularity)}', ` + + `which is not a date granularity (${[...DATE_GRANULARITIES].join(' / ')})`, + { + hint: ' An unknown granularity used to fall through to one bucket per raw ' + + 'value — date bucketing that silently never bucketed.', + extra: { object }, + }, + ); + } + fieldsToCheck.push(entry.field); + } + const gate = this.resolveQueryFields(object); + if (!gate) return; + const unknown = fieldsToCheck.filter((f) => !gate.known.has(f)); + if (unknown.length === 0) return; + const first = unknown[0]; + const dottedHint = first.includes('.') && gate.known.has(first.split('.')[0]) + ? " Grouping runs over this object's own columns; a related record's column cannot be a " + + 'grouping target.' + : suggestFieldName(first, gate.declared); + const err: any = new Error( + `Unknown field '${first}' on object '${object}'` + + (unknown.length > 1 ? ` (also: ${unknown.slice(1).join(', ')})` : '') + + `. 'groupBy' buckets rows by a column's values, so an unknown column puts every row ` + + 'in ONE bucket keyed null — a result indistinguishable from a column that really ' + + 'holds a single value.' + + dottedHint, + ); + err.code = 'INVALID_FIELD'; + err.status = 400; + err.field = first; + err.fields = unknown; + err.object = object; + err.param = 'groupBy'; + throw err; + } + + /** + * [#4254] AGGREGATIONS axis. An aggregation over a field the object does + * not have is refused (`400 INVALID_FIELD`); an entry the spec cannot read + * is refused as a shape (`400 INVALID_QUERY`). + * + * The stakes are the highest of the three #4254 axes because the wrong + * answer is a NUMBER: the in-memory path collects `undefined` for every + * row and `sum` folds those to 0, so `sum()` answered `0` — the same + * `0` a genuinely empty quarter produces, in a report whose whole job is + * to be believed. (`avg`/`min`/`max` answer `null`, `count()` counts + * nothing — every function has a plausible-looking value for a column that + * is not there.) + * + * The shape checks pin the rest of the spec's `AggregationNode` contract, + * because each violation also had a silent placeholder instead of an + * error: a function outside the spec enum computed `null`, a missing + * `alias` keyed the result column `"undefined"`, and a field-less + * aggregation is only meaningful as `count(*)` — for every other function + * it answered `null`/`0` while looking like a served query. `count` with + * no field (or the explicit `'*'` sentinel) is the one legitimate + * field-less form and passes. + */ + private assertAggregationFieldsExist(object: string, aggregations: unknown): void { + if (aggregations === undefined || aggregations === null) return; + if (!Array.isArray(aggregations)) { + throw invalidQueryError( + 'aggregations', + `must be an array of { function, field?, alias } entries, received ` + + `${typeof aggregations}`, + { + hint: ' A value the server cannot read used to be ignored — the rows came back ' + + 'raw and unaggregated. Send e.g. { "aggregations": [{ "function": "sum", ' + + `"field": "amount", "alias": "total" }] } in the 'POST /data/:object/query' body.`, + extra: { object }, + }, + ); + } + if (aggregations.length === 0) return; + const fieldsToCheck: string[] = []; + for (let i = 0; i < aggregations.length; i++) { + const entry = aggregations[i]; + if (entry === null || typeof entry !== 'object' || Array.isArray(entry)) { + throw invalidQueryError( + 'aggregations', + `entry #${i + 1} on object '${object}' is not an aggregation — expected ` + + `{ function, field?, alias }, received ` + + `${entry === null ? 'null' : Array.isArray(entry) ? 'an array' : typeof entry}`, + { extra: { object } }, + ); + } + if (typeof entry.function !== 'string' || !AGGREGATION_FUNCTIONS.has(entry.function)) { + throw invalidQueryError( + 'aggregations', + `entry #${i + 1} on object '${object}' uses ` + + (typeof entry.function === 'string' + ? `'${entry.function}', which is not an aggregation function` + : 'no aggregation function') + + ` (${[...AGGREGATION_FUNCTIONS].join(' / ')})`, + { + hint: ' An unknown function used to compute null for every group while the ' + + 'response looked served.', + extra: { object }, + }, + ); + } + if (typeof entry.alias !== 'string' || entry.alias.length === 0) { + throw invalidQueryError( + 'aggregations', + `entry #${i + 1} on object '${object}' has no 'alias' — the alias names the ` + + `result column (without one it came back keyed "undefined")`, + { extra: { object } }, + ); + } + if (entry.field === undefined || entry.field === null) { + if (entry.function !== 'count') { + throw invalidQueryError( + 'aggregations', + `entry '${entry.alias}' on object '${object}' applies '${entry.function}' to no ` + + `field. Only 'count' may omit the field (count(*), every row); '${entry.function}' ` + + 'needs a column to compute over — without one it answered null while looking served', + { extra: { object } }, + ); + } + continue; + } + if (typeof entry.field !== 'string') { + throw invalidQueryError( + 'aggregations', + `entry '${entry.alias}' on object '${object}' has a non-string 'field' ` + + `(received ${typeof entry.field})`, + { extra: { object } }, + ); + } + if (entry.field === '*') { + if (entry.function !== 'count') { + throw invalidQueryError( + 'aggregations', + `entry '${entry.alias}' on object '${object}' applies '${entry.function}' to '*'. ` + + `'*' is the count-all sentinel and only 'count' reads it`, + { extra: { object } }, + ); + } + continue; + } + fieldsToCheck.push(entry.field); + } + const gate = this.resolveQueryFields(object); + if (!gate) return; + const unknown = fieldsToCheck.filter((f) => !gate.known.has(f)); + if (unknown.length === 0) return; + const first = unknown[0]; + const dottedHint = first.includes('.') && gate.known.has(first.split('.')[0]) + ? " Aggregation runs over this object's own columns; a related record's column cannot " + + 'be aggregated.' + : suggestFieldName(first, gate.declared); + const err: any = new Error( + `Unknown field '${first}' on object '${object}'` + + (unknown.length > 1 ? ` (also: ${unknown.slice(1).join(', ')})` : '') + + `. An aggregation computes over a column's values, so an unknown column could only ` + + 'aggregate blanks — sum answered 0 and avg/min/max answered null, each ' + + 'indistinguishable from the same result over real data.' + + dottedHint, + ); + err.code = 'INVALID_FIELD'; + err.status = 400; + err.field = first; + err.fields = unknown; + err.object = object; + err.param = 'aggregations'; + throw err; + } + async findData(request: { object: string, query?: any, context?: any }) { // [#3770] Existence first: an unregistered object is a 404 before any // query parameter is even parsed, so an unknown name can never be @@ -3806,6 +4242,25 @@ export class ObjectStackProtocolImplementation implements this.assertExpandTargetsExist(request.object, Object.keys(options.expand)); } + // [#4254] The `searchFields` override is validated on the value the + // ENGINE will read — the standalone parameter when present, otherwise + // the object-form `search: { query, fields }` a `POST` body may carry + // (same precedence as the engine's own `searchFields ?? search.fields`, + // and the same shapes: the engine accepts the comma-string and array + // forms from either slot). Checked whether or not a `search` term rode + // along: the caller named fields either way, and a stale override is + // the same typo before the search that will eventually use it is added. + if (options.searchFields != null) { + this.assertSearchFieldsAreSearchable( + request.object, options.searchFields, wireSpelling.searchFields ?? 'searchFields', + ); + } else if (options.search !== null && typeof options.search === 'object' + && (options.search as any)?.fields != null) { + this.assertSearchFieldsAreSearchable( + request.object, (options.search as any).fields, wireSpelling.search ?? 'search', + ); + } + // Boolean fields for (const key of ['distinct', 'count']) { if (options[key] === 'true') options[key] = true; @@ -3890,6 +4345,15 @@ export class ObjectStackProtocolImplementation implements : { $and: [explicitWhere, implicitFilters] }; } + // [#4254] The aggregation axes name fields too, and were the last + // read-path axes that answered a wrong name with a plausible result + // (one null-keyed bucket; sum = 0). Validated before the routing + // below so an unreadable SHAPE cannot slip past the `Array.isArray` + // routing guard and ride to `engine.find` as ignored AST junk — + // rows returned ungrouped, looking exactly like a served query. + this.assertGroupByFieldsExist(request.object, options.groupBy); + this.assertAggregationFieldsExist(request.object, options.aggregations); + // Route to engine.aggregate() when the query has GROUP BY / aggregations. // engine.find() does not do in-memory aggregation fallback, so without // this branch a spec-shape aggregate request would silently return diff --git a/packages/objectql/src/query-expression-conformance.test.ts b/packages/objectql/src/query-expression-conformance.test.ts index 7097712800..f6ffe516a7 100644 --- a/packages/objectql/src/query-expression-conformance.test.ts +++ b/packages/objectql/src/query-expression-conformance.test.ts @@ -41,6 +41,13 @@ * is not what the author declared (`applySystemFields` injects the audit / * tenant / owner columns at registration), and because expansion is engine * work: only the real one can show `$expand` resolving a `tree` field. + * + * #4254 extends the same machine to the three axes #4226 explicitly left out — + * `searchFields`, `groupBy` and `aggregations` — in the second describe block + * below. Same ingress, same tiering, same control-group discipline; the new + * failure modes are worse only in WHAT they corrupt (`searchFields` changes + * the row SET, `groupBy` collapses N groups into one, `sum()` answers a + * 0 no report can tell from a real one). */ import { describe, it, expect, beforeEach } from 'vitest'; @@ -65,6 +72,12 @@ const taskObject = { status: { name: 'status', label: 'Status', type: 'text' as const }, project_id: { name: 'project_id', label: 'Project', type: 'lookup' as const, reference: 'showcase_project' }, parent_id: { name: 'parent_id', label: 'Parent', type: 'tree' as const, reference: 'showcase_task' }, + // [#4254] A searchable long-text column and a NON-searchable numeric + // one: `notes` is what `searchFields=notes` legitimately narrows to, + // `estimate` is what `sum()` legitimately totals — and what the search + // auto-default excludes by TYPE, which is its own rejection. + notes: { name: 'notes', label: 'Notes', type: 'textarea' as const }, + estimate: { name: 'estimate', label: 'Estimate', type: 'number' as const }, }, }; @@ -89,11 +102,25 @@ function makeMemoryDriver() { if (!v.every((arm) => matchesWhere(row, arm))) return false; continue; } + // [#4254] `$or` + `$contains` are the shape the engine expands + // `search` into (an `$or` of case-insensitive `$contains`, ADR-0061). + // Without them the driver would MATCH EVERY ROW for any search, and + // the "searchFields really narrows the row set" controls below would + // hold vacuously against a search that never filtered anything. + if (k === '$or' && Array.isArray(v)) { + if (!v.some((arm) => matchesWhere(row, arm))) return false; + continue; + } if (k.startsWith('$')) continue; if (v && typeof v === 'object' && '$in' in (v as any)) { if (!(v as any).$in.map(String).includes(String(row[k]))) return false; continue; } + if (v && typeof v === 'object' && '$contains' in (v as any)) { + const haystack = String(row[k] ?? '').toLowerCase(); + if (!haystack.includes(String((v as any).$contains).toLowerCase())) return false; + continue; + } const expected = (v && typeof v === 'object' && '$eq' in (v as any)) ? (v as any).$eq : v; const a = row[k] === undefined ? null : row[k]; const b = expected === undefined ? null : expected; @@ -650,3 +677,487 @@ describe('#4226 — sort / select / expand on the list path (real ObjectQL engin expect(Object.keys(r.records[0]).sort()).toEqual(['id', 'project_id', 'title']); }); }); + +/** + * [#4254] An object that DECLARES `searchableFields` — the other branch of the + * allowed-set resolution. `notes` exists and is text, but the declaration + * excludes it, which earns its own rejection message (the fix is on the + * OBJECT's declaration, not the request's spelling). + */ +const memoObject = { + name: 'showcase_memo', + label: 'Memo', + searchableFields: ['title'], + fields: { + id: { name: 'id', label: 'ID', type: 'text' as const, primaryKey: true }, + title: { name: 'title', label: 'Title', type: 'text' as const }, + notes: { name: 'notes', label: 'Notes', type: 'textarea' as const }, + }, +}; + +describe('#4254 — searchFields / groupBy / aggregations on the list path (real ObjectQL engine)', () => { + let engine: ObjectQL; + let protocol: ObjectStackProtocolImplementation; + + /** Same transcript order as #4226: five rows inserted `C A E B D`. */ + const INSERTION_ORDER = ['C', 'A', 'E', 'B', 'D']; + /** C (the 'done' row) totals 10; the four 'open' rows total 12. */ + const ESTIMATES: Record = { C: 10, A: 1, E: 5, B: 2, D: 4 }; + + const titles = (r: any): string[] => r.records.map((x: any) => x.title); + const ids = (r: any): string[] => r.records.map((x: any) => x.id); + + beforeEach(async () => { + engine = new ObjectQL(); + const { driver, stores } = makeMemoryDriver(); + // The issue's transcript runs on the engine's IN-MEMORY aggregation + // fallback — the path `engine.aggregate` takes for drivers with no + // native `aggregate` (driver-rest, driver-memory, partial SQL + // drivers). The fake's one-line `aggregate` stub would both preempt + // that path and ignore `groupBy`, making every grouping control below + // vacuously green against a grouping that never ran. + delete (driver as any).aggregate; + engine.registerDriver(driver, true); + await engine.init(); + engine.registry.registerObject(projectObject as any, 'test-package'); + engine.registry.registerObject(taskObject as any, 'test-package'); + engine.registry.registerObject(memoObject as any, 'test-package'); + protocol = new ObjectStackProtocolImplementation(engine); + + const tasks = new Map>(); + INSERTION_ORDER.forEach((letter, i) => { + tasks.set(`t_${letter}`, { + id: `t_${letter}`, + title: letter, + status: i === 0 ? 'done' : 'open', + project_id: 'p1', + parent_id: letter === 'A' ? null : 't_A', + // Only B carries the term in `notes`; only A carries it in + // `title`. `?search=a` finding exactly these two rows — and + // `searchFields` narrowing to exactly one — is what proves the + // search actually scans the columns it says it does. + ...(letter === 'B' ? { notes: 'alpha in notes' } : {}), + estimate: ESTIMATES[letter], + owner_id: 'usr_1', + created_at: '2026-07-30T00:00:00.000Z', + }); + }); + stores.set('showcase_task', tasks); + stores.set('showcase_memo', new Map([ + ['m1', { id: 'm1', title: 'gamma report', notes: 'delta hidden in notes' }], + ['m2', { id: 'm2', title: 'delta summary', notes: 'plain' }], + ])); + }); + + // ───────────────────────────────────────────────────────────── + // SEARCH-FIELDS — control group + // ───────────────────────────────────────────────────────────── + + it('search scans the default columns, and searchFields really narrows the row set', async () => { + // 'a' hits A in `title` and B in `notes` — two rows, two different + // matched columns. The narrowing controls only prove something + // because the un-narrowed baseline finds BOTH. + expect(titles(await protocol.findData({ object: 'showcase_task', query: { search: 'a' } }))) + .toEqual(['A', 'B']); + expect(titles(await protocol.findData({ + object: 'showcase_task', query: { search: 'a', searchFields: 'title' }, + }))).toEqual(['A']); + expect(titles(await protocol.findData({ + object: 'showcase_task', query: { search: 'a', searchFields: ['notes'] }, + }))).toEqual(['B']); + // The object form of `search` carries the same override — in both the + // array and comma-string shapes the engine accepts. + expect(titles(await protocol.findData({ + object: 'showcase_task', query: { search: { query: 'a', fields: ['title'] } }, + }))).toEqual(['A']); + expect(titles(await protocol.findData({ + object: 'showcase_task', query: { search: { query: 'a', fields: 'notes' } }, + }))).toEqual(['B']); + }); + + it('a declared searchableFields is the allowed set — search does not scan outside it', async () => { + // `delta` sits in m2's title and m1's NOTES; the declaration limits the + // scan to `title`, so m1 must not match. This is the declared-branch + // control the declared-branch rejection below leans on. + expect(ids(await protocol.findData({ object: 'showcase_memo', query: { search: 'delta' } }))) + .toEqual(['m2']); + expect(ids(await protocol.findData({ + object: 'showcase_memo', query: { search: 'delta', searchFields: 'title' }, + }))).toEqual(['m2']); + }); + + // ───────────────────────────────────────────────────────────── + // SEARCH-FIELDS — rejected + // ───────────────────────────────────────────────────────────── + + it('an unknown searchFields no longer WIDENS the search back to the default set', async () => { + // The issue's transcript: `?search=a&searchFields=no_such_field` used + // to return BOTH matching rows — the engine dropped the unknown name, + // the emptied override fell back to every searchable column, and a + // parameter whose only purpose is to narrow answered with the WIDER + // set. Same two-step #4226 closed on `select`, except this one changes + // which ROWS come back. + await expect(protocol.findData({ + object: 'showcase_task', query: { search: 'a', searchFields: 'no_such_field' }, + })).rejects.toMatchObject({ + status: 400, + code: 'INVALID_FIELD', + field: 'no_such_field', + object: 'showcase_task', + param: 'searchFields', + }); + }); + + it('a partially-unknown searchFields is refused too — half a narrowing is not the one asked for', async () => { + await expect(protocol.findData({ + object: 'showcase_task', query: { search: 'a', searchFields: 'title,no_such_field' }, + })).rejects.toMatchObject({ status: 400, code: 'INVALID_FIELD', field: 'no_such_field' }); + }); + + it.each([ + ['comma string', { search: 'a', searchFields: 'no_such_field' }, 'searchFields'], + ['array', { search: 'a', searchFields: ['no_such_field'] }, 'searchFields'], + ['OData spelling', { search: 'a', $searchFields: 'no_such_field' }, '$searchFields'], + ['the object form of search', { search: { query: 'a', fields: ['no_such_field'] } }, 'search'], + ['the object form with a comma string', { search: { query: 'a', fields: 'no_such_field' } }, 'search'], + ])('every override spelling gets the same answer, quoting the parameter the caller wrote — %s', async (_label, query, param) => { + await expect(protocol.findData({ object: 'showcase_task', query })) + .rejects.toMatchObject({ status: 400, code: 'INVALID_FIELD', field: 'no_such_field', param }); + }); + + it('a REAL field outside the searchable set is refused with its own reason', async () => { + // A different mistake from a typo: `estimate` exists, it just cannot + // be a `$contains` target. The message names the auto-default rule and + // the field's type, because the fix (declare `searchableFields`) is on + // the object. + await expect(protocol.findData({ + object: 'showcase_task', query: { search: 'a', searchFields: 'estimate' }, + })).rejects.toMatchObject({ status: 400, code: 'INVALID_FIELD', field: 'estimate' }); + await expect(protocol.findData({ + object: 'showcase_task', query: { search: 'a', searchFields: 'estimate' }, + })).rejects.toThrow(/is not searchable.*type 'number'/s); + // System columns get the system-column reason, not "unknown". + await expect(protocol.findData({ + object: 'showcase_task', query: { search: 'a', searchFields: 'id' }, + })).rejects.toThrow(/system\/audit column/); + }); + + it('outside a DECLARED searchableFields, the message points at the declaration', async () => { + // `notes` exists on the memo and is text-like — under the auto-default + // it would be searchable. The declaration is what excludes it, so the + // rejection must say so rather than call it unknown or untextual. + await expect(protocol.findData({ + object: 'showcase_memo', query: { search: 'delta', searchFields: 'notes' }, + })).rejects.toMatchObject({ status: 400, code: 'INVALID_FIELD', field: 'notes' }); + await expect(protocol.findData({ + object: 'showcase_memo', query: { search: 'delta', searchFields: 'notes' }, + })).rejects.toThrow(/declares 'searchableFields'/); + }); + + it('a STALE searchableFields declaration gets a third message — the bug is on the OBJECT', async () => { + // Clients echo the declaration verbatim (objectui's list search sends + // `$searchFields: schema.searchableFields`), so a declared entry whose + // field was renamed away must not be reported as the caller's typo — + // same split #4226 drew for a lookup whose `reference` was never + // authored. It is still refused: with every requested name stale, the + // engine's fallback would have scanned the default set — the widening + // this axis exists to close. + engine.registry.registerObject({ + name: 'showcase_stale', + label: 'Stale', + searchableFields: ['title', 'ghost'], + fields: { + id: { name: 'id', label: 'ID', type: 'text', primaryKey: true }, + title: { name: 'title', label: 'Title', type: 'text' }, + }, + } as any, 'test-package'); + // The engine's own resolution tolerates the stale entry — a search + // WITHOUT an override works over the existing subset. + await expect(protocol.findData({ object: 'showcase_stale', query: { search: 'x' } })) + .resolves.toBeDefined(); + // The objectui echo: the full declaration, stale entry included. + await expect(protocol.findData({ + object: 'showcase_stale', query: { search: 'x', searchFields: ['title', 'ghost'] }, + })).rejects.toMatchObject({ status: 400, code: 'INVALID_FIELD', field: 'ghost' }); + await expect(protocol.findData({ + object: 'showcase_stale', query: { search: 'x', searchFields: 'ghost' }, + })).rejects.toThrow(/declared in 'searchableFields' but does not exist/); + }); + + it('the override is validated even without a search term riding along', async () => { + // The caller named fields either way; a stale override is the same + // typo before the `search` that will eventually use it is added. (The + // export route only sends `searchFields` alongside `search`, so + // nothing in the framework depends on the inert combination.) + await expect(protocol.findData({ object: 'showcase_task', query: { searchFields: 'no_such_field' } })) + .rejects.toMatchObject({ status: 400, code: 'INVALID_FIELD', field: 'no_such_field' }); + await expect(protocol.findData({ object: 'showcase_task', query: { searchFields: 'title' } })) + .resolves.toMatchObject({ total: 5 }); + }); + + it('an override the server cannot read is refused rather than ignored', async () => { + // A number/object override was silently discarded by the engine — + // which left the search over the DEFAULT columns: the same widening, + // one shape earlier. + for (const searchFields of [42, { fields: 'title' }, true]) { + await expect(protocol.findData({ + object: 'showcase_task', query: { search: 'a', searchFields }, + })).rejects.toMatchObject({ status: 400, code: 'INVALID_FIELD', param: 'searchFields' }); + } + await expect(protocol.findData({ + object: 'showcase_task', query: { search: 'a', searchFields: ['title', 42] }, + })).rejects.toThrow(/entry #2.*is not a field name/s); + }); + + it('a dotted path is refused with the scans-own-columns hint, not a bare "unknown"', async () => { + // Plausible vocabulary from the select/sort axes — but the engine + // intersects the override by EXACT name, so a dotted path could only + // be dropped (and the search widened) if it were let through. + await expect(protocol.findData({ + object: 'showcase_task', query: { search: 'a', searchFields: 'parent_id.title' }, + })).rejects.toThrow(/scans this object's own columns/); + }); + + it('an empty override is ABSENT, not malformed', async () => { + for (const searchFields of ['', []]) { + expect(titles(await protocol.findData({ + object: 'showcase_task', query: { search: 'a', searchFields }, + }))).toEqual(['A', 'B']); + } + }); + + // ───────────────────────────────────────────────────────────── + // GROUP-BY — control group, then rejected + // ───────────────────────────────────────────────────────────── + + it('a real groupBy really groups (in-memory fallback path)', async () => { + const r: any = await protocol.findData({ + object: 'showcase_task', + query: { groupBy: ['status'], aggregations: [{ function: 'count', alias: 'n' }] }, + }); + expect(r.records).toHaveLength(2); + expect(r.records).toEqual(expect.arrayContaining([ + { status: 'done', n: 1 }, + { status: 'open', n: 4 }, + ])); + }); + + it('a date-bucketed groupBy really buckets', async () => { + const r: any = await protocol.findData({ + object: 'showcase_task', + query: { + groupBy: [{ field: 'created_at', dateGranularity: 'month' }], + aggregations: [{ function: 'count', alias: 'n' }], + }, + }); + expect(r.records).toEqual([{ created_at: '2026-07', n: 5 }]); + }); + + it('grouping by an unknown field is a 400, not one null-keyed bucket', async () => { + // The pre-fix answer was `[{ no_such_field: null, n: 5 }]` — the true + // row count under a grouping that never ran, structurally identical to + // "this column really holds a single value". A chart draws one bar. + await expect(protocol.findData({ + object: 'showcase_task', + query: { groupBy: ['no_such_field'], aggregations: [{ function: 'count', alias: 'n' }] }, + })).rejects.toMatchObject({ + status: 400, + code: 'INVALID_FIELD', + field: 'no_such_field', + object: 'showcase_task', + param: 'groupBy', + }); + }); + + it.each([ + ['second of two', { groupBy: ['status', 'no_such_field'] }], + ['structured form', { groupBy: [{ field: 'no_such_field', dateGranularity: 'month' }] }], + ['no aggregations riding along', { groupBy: ['no_such_field'] }], + ])('every groupBy spelling of an unknown field gets the same answer — %s', async (_label, query) => { + await expect(protocol.findData({ object: 'showcase_task', query })) + .rejects.toMatchObject({ status: 400, code: 'INVALID_FIELD', field: 'no_such_field' }); + }); + + it('a groupBy the spec cannot read is refused as a SHAPE, with its own code', async () => { + // Every one of these used to be ignored by the `Array.isArray` routing + // guard and ride to `engine.find` as inert AST junk: rows came back + // UNGROUPED with a 200, indistinguishable from a query that never + // asked for grouping. + await expect(protocol.findData({ object: 'showcase_task', query: { groupBy: 'status' } })) + .rejects.toMatchObject({ status: 400, code: 'INVALID_QUERY', param: 'groupBy' }); + await expect(protocol.findData({ object: 'showcase_task', query: { groupBy: [42] } })) + .rejects.toMatchObject({ status: 400, code: 'INVALID_QUERY' }); + await expect(protocol.findData({ + object: 'showcase_task', query: { groupBy: [{ dateGranularity: 'month' }] }, + })).rejects.toThrow(/names no field/); + await expect(protocol.findData({ + object: 'showcase_task', + query: { groupBy: [{ field: 'created_at', dateGranularity: 'fortnight' }] }, + })).rejects.toThrow(/not a date granularity/); + }); + + it('grouping by a related column is refused with the runs-own-columns hint', async () => { + await expect(protocol.findData({ + object: 'showcase_task', query: { groupBy: ['parent_id.title'] }, + })).rejects.toThrow(/this object's own columns/); + }); + + // ───────────────────────────────────────────────────────────── + // AGGREGATIONS — control group, then rejected + // ───────────────────────────────────────────────────────────── + + it('a real sum really sums (in-memory fallback path)', async () => { + const r: any = await protocol.findData({ + object: 'showcase_task', + query: { + groupBy: ['status'], + aggregations: [{ function: 'sum', field: 'estimate', alias: 'total' }], + }, + }); + expect(r.records).toEqual(expect.arrayContaining([ + { status: 'done', total: 10 }, + { status: 'open', total: 12 }, + ])); + }); + + it('count(*) — the one legitimate field-less form — passes, in both spellings', async () => { + const bare: any = await protocol.findData({ + object: 'showcase_task', query: { aggregations: [{ function: 'count', alias: 'n' }] }, + }); + expect(bare.records).toEqual([{ n: 5 }]); + const star: any = await protocol.findData({ + object: 'showcase_task', query: { aggregations: [{ function: 'count', field: '*', alias: 'n' }] }, + }); + expect(star.records).toEqual([{ n: 5 }]); + }); + + it('summing an unknown field is a 400, not a 0 no report can question', async () => { + // The pre-fix answer was `[{status:'open', s:0}, {status:'done', s:0}]` + // — sum folded a column of undefined to 0, the exact number a + // genuinely empty quarter produces. avg/min/max answered null the + // same way. + await expect(protocol.findData({ + object: 'showcase_task', + query: { + groupBy: ['status'], + aggregations: [{ function: 'sum', field: 'no_such_field', alias: 's' }], + }, + })).rejects.toMatchObject({ + status: 400, + code: 'INVALID_FIELD', + field: 'no_such_field', + object: 'showcase_task', + param: 'aggregations', + }); + // count(field) counts the non-null values of a REAL column — an + // unknown one is the same typo as anywhere else. + await expect(protocol.findData({ + object: 'showcase_task', + query: { aggregations: [{ function: 'count', field: 'no_such_field', alias: 'n' }] }, + })).rejects.toMatchObject({ status: 400, code: 'INVALID_FIELD', field: 'no_such_field' }); + }); + + it('an aggregation the spec cannot read is refused as a SHAPE, with its own code', async () => { + // Each of these had a silent placeholder instead of an error: null + // results for an unknown function or a field-less sum, a result column + // literally keyed "undefined" for a missing alias. + await expect(protocol.findData({ + object: 'showcase_task', query: { aggregations: 'sum(estimate)' }, + })).rejects.toMatchObject({ status: 400, code: 'INVALID_QUERY', param: 'aggregations' }); + await expect(protocol.findData({ + object: 'showcase_task', query: { aggregations: [42] }, + })).rejects.toMatchObject({ status: 400, code: 'INVALID_QUERY' }); + await expect(protocol.findData({ + object: 'showcase_task', + query: { aggregations: [{ function: 'median', field: 'estimate', alias: 'm' }] }, + })).rejects.toThrow(/not an aggregation function/); + await expect(protocol.findData({ + object: 'showcase_task', query: { aggregations: [{ function: 'sum', field: 'estimate' }] }, + })).rejects.toThrow(/has no 'alias'/); + await expect(protocol.findData({ + object: 'showcase_task', query: { aggregations: [{ function: 'sum', alias: 's' }] }, + })).rejects.toThrow(/Only 'count' may omit the field/); + await expect(protocol.findData({ + object: 'showcase_task', query: { aggregations: [{ function: 'sum', field: '*', alias: 's' }] }, + })).rejects.toThrow(/count-all sentinel/); + }); + + // ───────────────────────────────────────────────────────────── + // Composition and tiering + // ───────────────────────────────────────────────────────────── + + it('search + searchFields compose with the #4226 axes on one request', async () => { + const r: any = await protocol.findData({ + object: 'showcase_task', + query: { + filter: JSON.stringify({ status: 'open' }), + search: 'a', + searchFields: 'title,notes', + sort: '-title', + select: 'id,title', + top: 2, + }, + }); + expect(titles(r)).toEqual(['B', 'A']); + expect(Object.keys(r.records[0]).sort()).toEqual(['id', 'title']); + }); + + it('where + groupBy + aggregations compose on one request', async () => { + const r: any = await protocol.findData({ + object: 'showcase_task', + query: { + filter: JSON.stringify({ status: 'open' }), + groupBy: ['status'], + aggregations: [ + { function: 'sum', field: 'estimate', alias: 'total' }, + { function: 'count', alias: 'n' }, + ], + }, + }); + expect(r.records).toEqual([{ status: 'open', total: 12, n: 4 }]); + expect(r.total).toBe(1); + expect(r.hasMore).toBe(false); + }); + + it('an unknown OBJECT is still a 404 — no new axis gate may turn it into a 400', async () => { + for (const query of [ + { search: 'a', searchFields: 'no_such_field' }, + { groupBy: ['no_such_field'] }, + { aggregations: [{ function: 'sum', field: 'no_such_field', alias: 's' }] }, + ]) { + await expect(protocol.findData({ object: 'no_such_object', query })) + .rejects.toMatchObject({ status: 404, code: 'OBJECT_NOT_FOUND' }); + } + }); + + it('a legacy ARRAY field map disables the NAME gates — but shape is still refused', async () => { + // Name checks need a field map to consult; the shape of the request + // needs nothing. A registry-less/legacy host must not reject real + // field names it cannot verify — and must still not carry a groupBy + // string or a numeric searchFields to an engine that would ignore it. + const arrayEngine: any = { + find: async () => [], + count: async () => 0, + aggregate: async () => [], + registry: { + getObject: (name: string) => ({ + name, + fields: [{ name: 'title' }], + searchableFields: ['title'], + }), + }, + }; + const lenient = new ObjectStackProtocolImplementation(arrayEngine); + for (const query of [ + { search: 'a', searchFields: 'no_such_field' }, + { groupBy: ['no_such_field'] }, + { aggregations: [{ function: 'sum', field: 'no_such_field', alias: 's' }] }, + ]) { + await expect(lenient.findData({ object: 'legacy_object', query })).resolves.toBeDefined(); + } + await expect(lenient.findData({ object: 'legacy_object', query: { groupBy: 'status' } })) + .rejects.toMatchObject({ status: 400, code: 'INVALID_QUERY' }); + await expect(lenient.findData({ object: 'legacy_object', query: { search: 'a', searchFields: 42 } })) + .rejects.toMatchObject({ status: 400, code: 'INVALID_FIELD' }); + }); +}); diff --git a/packages/objectql/src/search-filter.ts b/packages/objectql/src/search-filter.ts index 1b2f8ae749..a69064bb0b 100644 --- a/packages/objectql/src/search-filter.ts +++ b/packages/objectql/src/search-filter.ts @@ -9,10 +9,11 @@ * object's *server-resolved* searchable fields — every driver already executes * `$or` + `$contains`, so no driver changes are needed. * - * Field resolution (server-side, never client-trusted): - * 1. an explicit, validated `requestedFields` subset (`$searchFields` override) - * 2. the object's declared `searchableFields` - * 3. an auto-default: the name/title field + short-text fields. + * Field resolution (server-side, never client-trusted) lives in + * `@objectstack/spec/data` (`search-fields.ts`) since #4254, because the REST + * ingress gate must consult the SAME rule when it refuses a `$searchFields` + * override the engine would not scan — see that module for the precedence + * (declared `searchableFields` → auto-default) and the override intersection. * * Matching: case-insensitive; multiple whitespace-separated terms are AND-ed * (every term must hit some field); fields are OR-ed. `select`/`status` columns @@ -28,40 +29,26 @@ * invisible to `$searchFields` overrides and to clients). */ +import { + resolveSearchFields, + SEARCHABLE_ENUM_TYPES, + type SearchFieldMeta, + type SearchFieldResolutionOptions, +} from '@objectstack/spec/data'; import { SEARCH_COMPANION_FIELD, isCompanionMatchableTerm } from './search-companion.js'; -export interface SearchFieldMeta { - type?: string; - hidden?: boolean; - options?: Array<{ label?: string; value: unknown } | string> | unknown; -} - -export interface ExpandSearchOptions { - /** The referenced object's field map (name → metadata). */ - fields: Record; - /** Object-declared `searchableFields` (the canonical default set). */ - searchableFields?: string[]; - /** Validated `$searchFields` override — intersected with the allowed set. - * Accepts an array or a comma-separated string (the form a URL query param - * arrives as). */ - requestedFields?: string | string[]; - /** Preferred display field, placed first in the auto-default. */ - displayField?: string; -} +export { + resolveSearchFields, + resolveSearchFieldResolution, + SEARCHABLE_TEXTUAL_TYPES, + SEARCHABLE_ENUM_TYPES, + SEARCH_AUTO_EXCLUDED_FIELDS, + SEARCH_AUTO_EXCLUDED_TYPES, +} from '@objectstack/spec/data'; +export type { SearchFieldMeta } from '@objectstack/spec/data'; -/** Short-text field types that make sense as `$contains` search targets. */ -const TEXTUAL_TYPES = new Set(['text', 'email', 'phone', 'url', 'autonumber', 'textarea', 'markdown']); -/** Enumerated types searched by mapping the query to option values via labels. */ -const ENUM_TYPES = new Set(['select', 'status']); -/** System / audit / heavy fields never auto-included. */ -const EXCLUDED_FIELDS = new Set([ - 'id', '_id', 'created', 'modified', 'created_at', 'updated_at', - 'created_by', 'updated_by', 'owner_id', 'organization_id', 'space', 'company_id', -]); -const EXCLUDED_TYPES = new Set([ - 'json', 'object', 'grid', 'image', 'file', 'avatar', 'vector', 'location', - 'geometry', 'secret', 'password', 'encrypted', 'boolean', 'lookup', 'master_detail', -]); +/** Historical name for {@link SearchFieldResolutionOptions} — same shape. */ +export type ExpandSearchOptions = SearchFieldResolutionOptions; export interface NormalizedSearch { query: string; @@ -81,41 +68,6 @@ export function normalizeSearch(raw: unknown): NormalizedSearch { return { query: '' }; } -function autoDefaultFields(fields: Record, displayField?: string): string[] { - const names = Object.keys(fields).filter((f) => { - if (EXCLUDED_FIELDS.has(f)) return false; - const meta = fields[f]; - if (!meta || meta.hidden) return false; - const t = meta.type; - if (!t) return false; - if (EXCLUDED_TYPES.has(t)) return false; - return TEXTUAL_TYPES.has(t) || ENUM_TYPES.has(t); - }); - // Lead with the display/name field when present. - const lead = displayField && fields[displayField] ? displayField - : fields.name ? 'name' - : fields.title ? 'title' - : undefined; - if (!lead) return names; - return [lead, ...names.filter((f) => f !== lead)]; -} - -/** Resolve the effective searchable field set (server-side, validated). */ -export function resolveSearchFields(opts: ExpandSearchOptions): string[] { - const all = opts.fields || {}; - const declared = opts.searchableFields?.filter((f) => all[f]); - const allowed = declared && declared.length > 0 ? declared : autoDefaultFields(all, opts.displayField); - const requested = typeof opts.requestedFields === 'string' - ? opts.requestedFields.split(',').map((f) => f.trim()).filter(Boolean) - : opts.requestedFields; - if (requested && requested.length > 0) { - const allowSet = new Set(allowed); - const validated = requested.filter((f) => allowSet.has(f)); - if (validated.length > 0) return validated; - } - return allowed; -} - function optionValuesMatching(meta: SearchFieldMeta, term: string): unknown[] { if (!Array.isArray(meta.options)) return []; const lc = term.toLowerCase(); @@ -133,7 +85,7 @@ function optionValuesMatching(meta: SearchFieldMeta, term: string): unknown[] { } function fieldClausesForTerm(field: string, term: string, meta: SearchFieldMeta): any[] { - if (ENUM_TYPES.has(meta?.type ?? '')) { + if (SEARCHABLE_ENUM_TYPES.has(meta?.type ?? '')) { const values = optionValuesMatching(meta, term); if (values.length > 0) return [{ [field]: { $in: values } }]; return [{ [field]: { $contains: term } }]; diff --git a/packages/qa/dogfood/test/search-conformance.ledger.ts b/packages/qa/dogfood/test/search-conformance.ledger.ts index c1684c1837..39b0f26191 100644 --- a/packages/qa/dogfood/test/search-conformance.ledger.ts +++ b/packages/qa/dogfood/test/search-conformance.ledger.ts @@ -31,15 +31,15 @@ export const SEARCH_SURFACE: ConformanceRow[] = [ summary: '`object.searchableFields` — canonical allowed-search-field set; auto-default (name/title + short text, secret/PII/json excluded) when unset', surface: 'spec/data/object.zod.ts:searchableFields', state: 'enforced', - enforcement: 'objectql/src/search-filter.ts resolveSearchFields / autoDefaultFields', + enforcement: 'spec/data/search-fields.ts resolveSearchFieldResolution (consumed by objectql/src/search-filter.ts)', proof: 'showcase-search.dogfood.test.ts', }, { id: 'search-fields-override', - summary: '`$searchFields` per-query narrowing — validated against the allowed set, can never widen it', + summary: '`$searchFields` per-query narrowing — validated against the allowed set, can never widen it; a name outside the set is 400 INVALID_FIELD at the REST ingress (#4254), not silently dropped', surface: 'spec/api/query.zod.ts:$searchFields', state: 'enforced', - enforcement: 'objectql/src/search-filter.ts resolveSearchFields (intersection)', + enforcement: 'spec/data/search-fields.ts resolveSearchFields (intersection) + metadata-protocol/src/protocol.ts assertSearchFieldsAreSearchable (ingress gate)', proof: 'showcase-search.dogfood.test.ts', }, { diff --git a/packages/rest/src/rest-server.ts b/packages/rest/src/rest-server.ts index 33ff4f3d9d..d4279be74c 100644 --- a/packages/rest/src/rest-server.ts +++ b/packages/rest/src/rest-server.ts @@ -612,14 +612,21 @@ function isExpectedDataStatus(status: number): boolean { /** * Malformed-query rejections from the list normalizer (`findData`). They are * 400s the CALLER caused by naming a parameter the API does not have - * (`UNSUPPORTED_QUERY_PARAM`, #2926 ⑩) or a field the object does not have - * (`INVALID_FIELD`, #4134) — a client mistake the response already explains, + * (`UNSUPPORTED_QUERY_PARAM`, #2926 ⑩), a field the object does not have + * (`INVALID_FIELD`, #4134 / #4226 / #4254), or a filter/sort/aggregation + * value the spec cannot read (`INVALID_FILTER` #4181, `INVALID_SORT` #4226, + * `INVALID_QUERY` #4254) — a client mistake the response already explains, * not a server fault worth an "[REST] Unhandled error" line per request. + * The filter and sort codes joined this list late: both shipped without it, + * so every rejection they produced was ALSO logged as an unhandled error. */ function isExpectedQueryRejection(body: Record | undefined): boolean { return body?.code === 'UNSUPPORTED_QUERY_PARAM' || body?.code === 'INVALID_FIELD' - || body?.code === 'INVALID_REQUEST'; + || body?.code === 'INVALID_REQUEST' + || body?.code === 'INVALID_FILTER' + || body?.code === 'INVALID_SORT' + || body?.code === 'INVALID_QUERY'; } /** diff --git a/packages/spec/api-surface.json b/packages/spec/api-surface.json index 7510cba68d..e28d767c74 100644 --- a/packages/spec/api-surface.json +++ b/packages/spec/api-surface.json @@ -505,6 +505,10 @@ "RowCrudActionOverrideInput (type)", "RowCrudActionOverrideSchema (const)", "RowCrudPredicates (interface)", + "SEARCHABLE_ENUM_TYPES (const)", + "SEARCHABLE_TEXTUAL_TYPES (const)", + "SEARCH_AUTO_EXCLUDED_FIELDS (const)", + "SEARCH_AUTO_EXCLUDED_TYPES (const)", "SINGLE_OPTION_TYPES (const)", "SQLDialect (type)", "SQLDialectSchema (const)", @@ -525,6 +529,8 @@ "ScriptBodySchema (const)", "ScriptValidation (type)", "ScriptValidationSchema (const)", + "SearchFieldMeta (interface)", + "SearchFieldResolutionOptions (interface)", "Seed (type)", "SeedIdentity (type)", "SeedIdentitySchema (const)", @@ -634,6 +640,8 @@ "resolveDisplayField (function)", "resolveEffectiveApiMethods (function)", "resolveRecordDisplayName (function)", + "resolveSearchFieldResolution (function)", + "resolveSearchFields (function)", "sequenceWidth (function)", "stripLegacyApiMethods (function)", "suggestFieldType (function)", diff --git a/packages/spec/src/data/index.ts b/packages/spec/src/data/index.ts index 8009b4af8b..3a86910f82 100644 --- a/packages/spec/src/data/index.ts +++ b/packages/spec/src/data/index.ts @@ -72,6 +72,11 @@ export * from './aggregation-policy'; // enrichment, search field resolution, and lint. export * from './display-name'; +// `$search` field resolution (ADR-0061) — which columns a search scans. +// Shared by the objectql engine (search expansion) and the metadata-protocol +// ingress gate (#4254) so the two cannot drift. +export * from './search-fields'; + // fieldGroups layout derivation (ADR-0085 §5) — the single source of the // grouping semantics every renderer (form, detail, drawer, designer) applies. export * from './field-group-layout'; diff --git a/packages/spec/src/data/search-fields.ts b/packages/spec/src/data/search-fields.ts new file mode 100644 index 0000000000..fcd63f10de --- /dev/null +++ b/packages/spec/src/data/search-fields.ts @@ -0,0 +1,125 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * `$search` field resolution (ADR-0061, Tier 1) — which columns a search scans. + * + * ONE resolution shared by the two layers that must agree on it: + * + * - the ENGINE (`@objectstack/objectql` `expandSearchToFilter`), which expands + * a `$search` term into a `$or` of `$contains` clauses over exactly this set; + * - the INGRESS gate (`@objectstack/metadata-protocol` `findData`), which + * refuses a `$searchFields` override naming a field this set does not admit + * (#4254), instead of letting the engine drop it silently. + * + * The two used to be one (the rule lived inside the engine), and #4254 keeps + * them one by moving the rule HERE rather than copying it: a gate that + * re-implemented "which fields are searchable" would drift from what the + * engine actually scans, and then either reject a search that would have + * worked or admit one the engine ignores — the same drift #4226 avoided on the + * expand axis by having gate and engine read `REFERENCE_VALUE_TYPES`. + * + * Resolution precedence (server-side, never client-trusted): + * 1. the object's declared `searchableFields` (filtered to fields that exist) + * 2. an auto-default: the display/name field + short-text and enum fields. + * + * An explicit `$searchFields` override is then INTERSECTED with that allowed + * set — it can narrow the scan, never widen it (`resolveSearchFields`). + */ + +/** The slice of field metadata search resolution reads. */ +export interface SearchFieldMeta { + type?: string; + hidden?: boolean; + options?: Array<{ label?: string; value: unknown } | string> | unknown; +} + +/** Short-text field types that make sense as `$contains` search targets. */ +export const SEARCHABLE_TEXTUAL_TYPES: ReadonlySet = new Set([ + 'text', 'email', 'phone', 'url', 'autonumber', 'textarea', 'markdown', +]); +/** Enumerated types searched by mapping the query to option values via labels. */ +export const SEARCHABLE_ENUM_TYPES: ReadonlySet = new Set(['select', 'status']); +/** System / audit / heavy fields never auto-included. */ +export const SEARCH_AUTO_EXCLUDED_FIELDS: ReadonlySet = new Set([ + 'id', '_id', 'created', 'modified', 'created_at', 'updated_at', + 'created_by', 'updated_by', 'owner_id', 'organization_id', 'space', 'company_id', +]); +export const SEARCH_AUTO_EXCLUDED_TYPES: ReadonlySet = new Set([ + 'json', 'object', 'grid', 'image', 'file', 'avatar', 'vector', 'location', + 'geometry', 'secret', 'password', 'encrypted', 'boolean', 'lookup', 'master_detail', +]); + +export interface SearchFieldResolutionOptions { + /** The object's field map (name → metadata). */ + fields: Record; + /** Object-declared `searchableFields` (the canonical default set). */ + searchableFields?: string[]; + /** Validated `$searchFields` override — intersected with the allowed set. + * Accepts an array or a comma-separated string (the form a URL query param + * arrives as). */ + requestedFields?: string | string[]; + /** Preferred display field, placed first in the auto-default. */ + displayField?: string; +} + +function autoDefaultFields(fields: Record, displayField?: string): string[] { + const names = Object.keys(fields).filter((f) => { + if (SEARCH_AUTO_EXCLUDED_FIELDS.has(f)) return false; + const meta = fields[f]; + if (!meta || meta.hidden) return false; + const t = meta.type; + if (!t) return false; + if (SEARCH_AUTO_EXCLUDED_TYPES.has(t)) return false; + return SEARCHABLE_TEXTUAL_TYPES.has(t) || SEARCHABLE_ENUM_TYPES.has(t); + }); + // Lead with the display/name field when present. + const lead = displayField && fields[displayField] ? displayField + : fields.name ? 'name' + : fields.title ? 'title' + : undefined; + if (!lead) return names; + return [lead, ...names.filter((f) => f !== lead)]; +} + +/** + * The ALLOWED search-field set for an object, plus where it came from — + * `declared` when the object's `searchableFields` (filtered to fields that + * exist) is non-empty, `auto` otherwise. The source matters to the #4254 + * ingress gate because the two rejections it explains differ: a field missing + * from a declared list is fixed by adding it there, while a field the + * auto-default skips is excluded by its TYPE and needs `searchableFields` + * declared to become a target. + */ +export function resolveSearchFieldResolution( + opts: Omit, +): { allowed: string[]; source: 'declared' | 'auto' } { + const all = opts.fields || {}; + const declared = opts.searchableFields?.filter((f) => all[f]); + if (declared && declared.length > 0) return { allowed: declared, source: 'declared' }; + return { allowed: autoDefaultFields(all, opts.displayField), source: 'auto' }; +} + +/** + * Resolve the effective searchable field set (server-side, validated): the + * allowed set, narrowed by a validated `requestedFields` override when one is + * present. + * + * TOLERANT by design — unknown requested names are dropped, and a request + * naming none of the allowed fields falls back to the full allowed set. This + * guards INTERNAL callers (hooks, flows, registry-less hosts) that never pass + * an ingress; the REST read path refuses those same inputs loudly BEFORE the + * engine sees them (`400 INVALID_FIELD`, #4254), exactly like the projection + * axis, whose engine-side `SELECT *` tolerance sits behind the #4226 gate. + */ +export function resolveSearchFields(opts: SearchFieldResolutionOptions): string[] { + const { allowed } = resolveSearchFieldResolution(opts); + const requested = typeof opts.requestedFields === 'string' + ? opts.requestedFields.split(',').map((f) => f.trim()).filter(Boolean) + : opts.requestedFields; + if (requested && requested.length > 0) { + const allowSet = new Set(allowed); + const validated = requested.filter((f) => allowSet.has(f)); + if (validated.length > 0) return validated; + } + return allowed; +}