diff --git a/.changeset/30518-protocol-find-data-to-facet.md b/.changeset/30518-protocol-find-data-to-facet.md new file mode 100644 index 00000000000..23bbf54267f --- /dev/null +++ b/.changeset/30518-protocol-find-data-to-facet.md @@ -0,0 +1,13 @@ +--- +'@objectstack/metadata-protocol': patch +'@objectstack/spec': patch +--- + +refactor(metadata-protocol): `findData` hands its body to its facet + +Clause-②: no +Metadata-protocol: none + +- `ObjectStackProtocolImplementation#findData` keeps its signature and docblock on the class and delegates in one line to `ProtocolDataCrudFacet`, which now holds the body, moved unchanged. The built `index.d.ts` and `index.d.cts` are byte-equal before and after, and no behaviour changes. Nothing to do on upgrade. +- `getDiscovery` stays on the class: its body reads `typeof this.searchAll === 'function'`, which a forwarded host method would always answer true. The move tool now refuses a method read as a value. +- `@objectstack/spec`'s published liveness ledger (`liveness/query.json`) cites `findData` at its new file. diff --git a/docs/qa/platform-checklist/areas/api-backend.json b/docs/qa/platform-checklist/areas/api-backend.json index 3f4a0692380..11096298060 100644 --- a/docs/qa/platform-checklist/areas/api-backend.json +++ b/docs/qa/platform-checklist/areas/api-backend.json @@ -525,7 +525,7 @@ "source": [ "packages/spec/src/data/filter.zod.ts#AST_OPERATOR_MAP (AST_OPERATOR_MAP / FieldOperatorsSchema — the operator variant source)", "packages/spec/src/data/query.zod.ts#QueryAST (QueryAST keys)", - "packages/metadata-protocol/src/protocol.ts#UNSUPPORTED_QUERY_PARAM (supported $-params + UNSUPPORTED_QUERY_PARAM #2926 ⑩, INVALID_FIELD #4134)", + "packages/metadata-protocol/src/protocol-data-crud.ts#UNSUPPORTED_QUERY_PARAM (supported $-params + UNSUPPORTED_QUERY_PARAM #2926 ⑩, INVALID_FIELD #4134)", "packages/spec/src/data/filter.zod.ts#checkFilterConditionComparands (the request-schema refusal of a scalar list comparand on the POST door, #20116 — the $-object spelling, located at query.where..$nin)", "packages/spec/src/data/data-engine.zod.ts#lowerFoldedTransportValues (the AST/FilterArray spelling on the POST door, refused at query.where when it cannot be lowered)", "packages/spec/src/data/data-engine.zod.ts#QueryTransportParamsSchema (`filter` is a declared transport spelling of `where` — why negative[0] no longer probes with it)", diff --git a/docs/qa/platform-checklist/areas/search.json b/docs/qa/platform-checklist/areas/search.json index 98926edc26c..362190d11c8 100644 --- a/docs/qa/platform-checklist/areas/search.json +++ b/docs/qa/platform-checklist/areas/search.json @@ -282,7 +282,7 @@ "examples/app-showcase/src/security/permission-sets.ts#invoice_own_rows (invoice_own_rows: owner == current_user.email, positions ['contributor'])", "examples/app-showcase/src/data/seed/index.ts (INV-1001..1012 owner spread; 'ada sees INV-1001/1002's lines but never linus's INV-1003')", "packages/objectql/src/engine.ts#expandSearchOnAst (expandSearchOnAst ANDs into ast.where; count() rides the read middleware — the #2737 total-leak fix)", - "packages/metadata-protocol/src/protocol.ts#findData (the paged arm counts a searched list: engine.count is handed the query's search and searchFields, so total is the counted number of matches and hasMore agrees with it; the `countable` exception and its page-local estimate were deleted by #22790, PR #22857) + packages/rest/src/list-search-total-door.test.ts ('[#22790] a paged search reports the counted total', including 'a member counts only the matches it can read')" + "packages/metadata-protocol/src/protocol-data-crud.ts#findData (the paged arm counts a searched list: engine.count is handed the query's search and searchFields, so total is the counted number of matches and hasMore agrees with it; the `countable` exception and its page-local estimate were deleted by #22790, PR #22857) + packages/rest/src/list-search-total-door.test.ts ('[#22790] a paged search reports the counted total', including 'a member counts only the matches it can read')" ], "history": [ { "revision": 1, "date": "2026-08-07", "change": "new item: RLS × $search both-personas contract; total-leakage clause grounded in the protocol's page-local total for searched lists and the #2737 count middleware fix, persona provisioning gap recorded honestly", "ref": "claude/platform-test-checklist-ocwugl" }, diff --git a/packages/metadata-protocol/scripts/host-split.mjs b/packages/metadata-protocol/scripts/host-split.mjs index 0fc757f264f..86d2be77b51 100644 --- a/packages/metadata-protocol/scripts/host-split.mjs +++ b/packages/metadata-protocol/scripts/host-split.mjs @@ -186,6 +186,10 @@ export function rewriteSites(sf, method, table, className = CLASS_NAME) { const member = table.instance.get(name); if (!member) { refuse(node, `\`this.${name}\` names no declared instance member`); return; } if (table.statics.has(name)) { refuse(node, `\`${name}\` is both an instance and a static member: the host cannot carry both`); return; } + // [#30518] A method's host entry is a forwarding closure, always a function, so a + // method read other than as a callee (`typeof this.m`, `const f = this.m`) would + // read the closure, not the instance's member. Same rule as a private static below. + if (member.kind === 'method' && !isCallee(p)) { refuse(node, `instance method \`${name}\` read as a value: the host carries a method as a forwarding closure, so only a call is in the closed set`); return; } sites.push({ start: node.getStart(sf), end: node.getEnd(), text: HOST_PARAM, kind: 'member', member: name }); useInstance(name, isAssignmentTarget(p) ? 'write' : isCallee(p) ? 'call' : 'read'); ts.forEachChild(p, (c) => { if (c !== node) visit(c); }); diff --git a/packages/metadata-protocol/scripts/host-split.test.mjs b/packages/metadata-protocol/scripts/host-split.test.mjs index b3ed7766ce0..e5441bbc986 100644 --- a/packages/metadata-protocol/scripts/host-split.test.mjs +++ b/packages/metadata-protocol/scripts/host-split.test.mjs @@ -54,6 +54,8 @@ line two \${k}\`; namedHost() { const host = 1; return host; } staticField() { return ${C}.LIMIT; } staticAsValue() { const f = ${C}.helper; return f(1); } + probesMethod() { return { search: typeof this.read === 'function' }; } + methodAsValue() { const f = this.read; return f('k'); } bareConstructor() { return this.constructor; } undeclared() { return this.nope; } *generator() { yield this.count; } @@ -142,6 +144,9 @@ describe('host-split: the move', () => { ['namedHost', 'already named `host`'], ['staticField', 'private static `LIMIT` (field) read'], ['staticAsValue', 'private static `helper` (method) read as a value'], + // [#30518] `getDiscovery`'s shape: a host entry forwards a method through a closure, always a function. + ['probesMethod', 'instance method `read` read as a value'], + ['methodAsValue', 'instance method `read` read as a value'], ['bareConstructor', '`this.constructor` not followed by `.`'], ['undeclared', '`this.nope` names no declared instance member'], ['generator', 'a generator method'], diff --git a/packages/metadata-protocol/src/protocol-data-crud.ts b/packages/metadata-protocol/src/protocol-data-crud.ts index 173a79263bf..b1c07fefa49 100644 --- a/packages/metadata-protocol/src/protocol-data-crud.ts +++ b/packages/metadata-protocol/src/protocol-data-crud.ts @@ -2,8 +2,8 @@ /** * [#29985 codemod] The "Data CRUD" facet of - * `ObjectStackProtocolImplementation`: the bodies of `getData`, `cloneData`, - * `probeRecord` moved out of `protocol.ts` whole by + * `ObjectStackProtocolImplementation`: the bodies of `findData`, `getData`, + * `cloneData`, `probeRecord` moved out of `protocol.ts` whole by * `scripts/codemod-split-protocol.mjs`. * * A MOVE, not a redesign. `ProtocolDataCrudFacet` is a stateless facet over the host @@ -29,14 +29,799 @@ import { omitInternalFieldsFromWriteResponse } from './write-response-internal-f import type { DroppedFieldsEvent, EngineQueryOptions, ValidationAdvisoryEvent } from '@objectstack/spec/data'; import { redactStoredMetadataRow, + redactStoredMetadataRows, + serveStoredMetadataHashColumnRows, serveStoredMetadataHashColumns, + storedMetadataBodyGroupingRefusal, + storedMetadataBodyPredicateRefusal, storedMetadataBodyProjection, + storedMetadataHashEvaluateRefusal, } from './metadata-redaction.js'; import { CLONE_STRIP_FIELDS } from './protocol-write-constants.js'; import { projectionFieldsValue } from './protocol-list-query-params.js'; +import { assertQueryReadRows, planQueryReadWindow, resolveQueryReadLimits } from '@objectstack/types'; +import { + QUERY_TRANSPORT_ALIAS_SLOTS, + QUERY_TRANSPORT_DOLLAR_ALIASES, + QUERY_TRANSPORT_DOLLAR_PARAMS, + foldQueryAliasSlots, + isFilterAST, + parseFilterAST, +} from '@objectstack/spec/data'; +import type { EngineAggregateOptions, EngineCountOptions } from '@objectstack/spec/data'; +import { malformedFilterArrayError } from './protocol-data-request-shapes.js'; +import { + RESERVED_LIST_QUERY_PARAMS, + assertQueryParamArity, + collectStoredMetadataFilterFields, + conflictingQueryParamsError, + countFlagValue, + narrowStoredMetadataSearch, + normalizeSortNodes, + pageWindowValue, + unusableFilterError, +} from './protocol-list-query-params.js'; import type { ProtocolHost } from './protocol-host.js'; export class ProtocolDataCrudFacet { + /** The body of `ObjectStackProtocolImplementation#findData`; the method's docblock there is its contract. */ + static async findData(host: Pick, request: { object: string, query?: any, context?: any }): Promise<{ + hasMore: boolean; + total?: number | undefined; + object: string; + records: any[]; + }> { + // [#3770] Existence first: an unregistered object is a 404 before any + // query parameter is even parsed, so an unknown name can never be + // probed for query-shape validity (nor reach the driver as a table). + host.assertObjectRegistered(request.object); + const options: any = { ...request.query }; + // The execution context is SERVER-DERIVED and never caller input. + // + // `request.query` is the raw request bag on every ingress that reaches + // here — the REST `POST /data/:object/query` route hands `req.body` + // straight in as `query`. `context` is in the known-params set below, so + // it was not swept into the implicit-filter bucket either: a caller's + // `context` survived this spread and, because the assignment below is + // conditional, became the operation's execution context whenever no + // server context resolved (before #3963 an anonymous request on a + // `requireAuth: false` deployment; the invariant holds regardless). + // + // What rides on it is total: plugin-security's middleware opens with + // `if (opCtx.context?.isSystem) return next()` — the entire RLS / FLS / + // CRUD chain skipped — and `__expandRead` marks a read as an expansion + // sub-read (#2850; it waived the object-level CRUD gate for "public" + // objects until #7626 removed that). Neither is ever schema-stripped on + // this path: `ExecutionContextSchema.parse` runs only in + // `engine.createContext`, which the read path does not use. + // + // Route-level `enforceAuth` is what kept that from being reachable, so + // this was a fail-OPEN default one layer down. Drop any inbound + // `context` unconditionally: the protocol must not depend on a gate + // above it staying switched on. + delete options.context; + + // [#7321] Arity BEFORE any read, fold or coercion. `IHttpRequest.query` + // is `Record< string, string | string[] >`; the array arm is real (the + // `node:http` adapter hands `?x=1&x=2` through as `['1','2']`, measured + // over a socket on #6878), and every coercion below was written for the + // string arm. `Number(['1','2'])` is `NaN`, so `?$top=1&$top=2` used to + // reach the driver as `limit: NaN` — a wrong answer served as a 200. + // + // Single-vs-multi is a PER-PARAMETER judgement, never a sweep: `$select` + // / `$expand` / `$searchFields` / `$orderby` accept the array arm on + // purpose and are untouched here. See + // {@link ARRAY_VALUED_QUERY_SLOTS} for the full disposition. + assertQueryParamArity(options); + + // Forward the dispatcher's ExecutionContext so RBAC/RLS middleware + // can apply per-request enforcement. The protocol layer is purely + // a normalizer — it must never strip security context. + if (request.context !== undefined) { + options.context = request.context; + } + + // ==================================================================== + // Normalize legacy params → QueryAST standard (where/fields/orderBy/offset/expand) + // ==================================================================== + + // OData-style `$`-prefixed params → bare aliases that the rest of + // this function knows how to normalize. Without this step, params + // like `?$top=2&$orderby=...` survive into the catch-all + // implicit-filter pass below and get merged into `where` as + // bogus field-equality predicates (e.g. `where.$top = "2"`), + // which silently returns zero rows for every list endpoint. + // + // [#4226] `wireSpelling` remembers which alias each rewritten slot + // arrived under, so a rejection quotes the parameter the caller + // actually wrote. Telling someone who sent `?$orderby=…` that + // "'orderBy' is invalid" names a parameter absent from their request. + // + // [#7321] The table itself now lives at module scope + // ({@link QUERY_TRANSPORT_DOLLAR_ALIASES}) so the arity survey above and this fold + // cannot drift apart on which `$` spellings exist. + const wireSpelling: Record = {}; + for (const [dollar, bare] of QUERY_TRANSPORT_DOLLAR_ALIASES) { + if (options[dollar] != null && options[bare] == null) { + options[bare] = options[dollar]; + wireSpelling[bare] = dollar; + } + delete options[dollar]; + } + + // [#3795] One slot, one value. Every alias spelling of the six + // QueryAST slots resolves HERE, by the spec's own table, before the + // per-slot wire coercion below ever runs — so that coercion reads + // canonical keys only. An alias alone folds into its canonical key; + // redundant identical spellings collapse; different values for one + // slot are refused (the #4181 rule, generalized from the filter slot + // to all of them — four used to resolve BACKWARDS here, each in its + // own way, disagreeing with the spec's documented precedence and with + // the runtime dispatcher's copy of the same fold; `top`→`limit` + // joined the table last, via #4346). + // + // `arrivedAs` remembers which spelling carried each slot's value; + // composed with `wireSpelling` it names the parameter the caller + // actually wrote in every rejection below (#4226). + const spellingFor = (name: string): string => wireSpelling[name] ?? name; + const arrivedAs = foldQueryAliasSlots(options, QUERY_TRANSPORT_ALIAS_SLOTS, (conflict) => { + throw conflictingQueryParamsError(conflict, spellingFor); + }); + const slotParam = (canonical: string): string => spellingFor(arrivedAs[canonical] ?? canonical); + + // Numeric fields — coerce querystring strings. `top` already folded + // into `limit` above: the pair joined the #3795 slot table with #4346, + // replacing an open-coded rewrite here that resolved it BACKWARDS + // (`options.limit = Number(options.top)` — the alias overwrote the + // canonical key, so `{top: 1, limit: 3}` answered 1 over HTTP and 3 + // through a direct engine call). + // + // [#25229] The coercion is CHECKED, not bare (see + // {@link pageWindowValue}). This is the one statement every list door + // reaches with the window still unvalidated — `GET /data/:object`'s + // querystring, `POST /data/:object/query`'s body (whose schema checks + // the type, not the sign) and the runtime dispatcher — so the refusal + // lives here, once. + if (options.limit != null) options.limit = pageWindowValue(options.limit, slotParam('limit')); + if (options.offset != null) options.offset = pageWindowValue(options.offset, slotParam('offset')); + + // Projection: comma-separated string → array. A single-value + // querystring param arrives as a bare string — `?fields=name` or the + // folded `?select=` / `$select` spellings — which drivers' `.map()` + // calls over `query.fields` would otherwise throw on. + // + // [#26644] …and a value naming no field is refused rather than becoming + // `[]`, which every driver serves as no projection at all (see + // {@link projectionFieldsValue}). + const projectionKey = slotParam('fields'); + if (typeof options.fields === 'string' || Array.isArray(options.fields)) { + options.fields = projectionFieldsValue(options.fields, projectionKey); + } else if (options.fields !== undefined) { + delete options.fields; + } + // [#4226] Unknown projection columns are refused rather than dropped — + // see `assertProjectionFieldsExist` for why this axis' silent failure + // returned MORE than was asked for. + host.assertProjectionFieldsExist(request.object, options.fields, projectionKey); + + // Sort: every wire shape → SortNode[]. + // + // [#4226] `normalizeSortNodes` folds the two shapes that used to fall + // through this block untouched — `string[]` and `{field: direction}` — + // and refuses the ones it cannot read. Before it, "not a string and not + // an array" simply skipped the branch, leaving a value on `orderBy` + // that `SqlDriver`'s `Array.isArray` guard then declined to turn into + // an ORDER BY clause: no sort, no error, no way to tell. + const sortValue = options.orderBy; + const sortKey = slotParam('orderBy'); + if (sortValue === undefined || sortValue === null) { + // Nothing to sort by — and an explicit `orderBy: null` must not ride + // to the engine as a value every driver quietly declines to read. + delete options.orderBy; + } else { + const orderBy = normalizeSortNodes(sortValue, sortKey); + // [#4226] Validated on the NORMALIZED nodes, so one gate covers + // every spelling — `?sort=no_such`, `['-no_such']`, `{no_such: + // 'desc'}` and `[{field:'no_such'}]` are one mistake with one + // answer. Assigned only after it passes, so a rejected sort cannot + // leave a half-applied one behind. + host.assertSortFieldsExist(request.object, orderBy, sortKey); + if (orderBy.length > 0) options.orderBy = orderBy; + else delete options.orderBy; + } + + // Filter: the folded slot value → a usable `FilterCondition` on + // `where`, or a rejection. The four spellings of this slot + // (`where`/`filter`/`filters`/`$filter`) already resolved through the + // #3795 fold above — #4181's one-slot-one-value rule, which this block + // pioneered before the fold generalized it. + const filterKey = slotParam('where'); + + if (options.where !== undefined) { + let parsedFilter = options.where; + // A blank `?filter=` is ABSENT, not malformed — the same `length > 0` + // guard the export route applies before parsing. Deleting `where` + // here (rather than leaving `''` on it) is what lets every consumer + // below test presence with a plain falsy check. + if (typeof parsedFilter === 'string' && parsedFilter.trim() === '') { + delete options.where; + } else { + // [#4181] JSON string → object. Parse failure is a REJECTION, not + // a fallback. The `catch { /* keep as-is */ }` this replaces left + // the raw string on `where`, a shape no driver consumes — so the + // filter was dropped whole and `?filter={status:done` (one missing + // quote) answered 200 with the UNFILTERED page. Worst member of + // the #3948 family: #4134 zeroed, #4164 dropped one predicate, + // this returned everything. + // + // The sibling `GET /data/:object/export` route has rejected this + // exact input since it was written (`400 INVALID_REQUEST`, + // "filter must be JSON"); the list path was the outlier. The + // guard lives HERE, in the shared normalizer, so `GET + // /data/:object`, `POST /data/:object/query` and the runtime + // dispatcher all inherit one answer instead of three. + if (typeof parsedFilter === 'string') { + try { + parsedFilter = JSON.parse(parsedFilter); + } catch { + throw unusableFilterError(filterKey, 'must be valid JSON'); + } + } + // Filter AST array → FilterCondition object + if (isFilterAST(parsedFilter)) { + parsedFilter = parseFilterAST(parsedFilter); + } else if (Array.isArray(parsedFilter) && parsedFilter.length > 0) { + // [#4121] `isFilterAST` was being read as a *conversion* gate, + // so an array it refused was assigned to `where` unconverted — + // an opaque value the driver then had to make sense of. Every + // driver now fails on it, so this is not a narrowing; it moves + // the failure to where the malformed filter actually arrived, + // with the request's own vocabulary in the message instead of a + // driver-internal one. + // + // It also closes what the driver-side fix could not: a lone + // `['and']` / `['or']` sets the join mode, matches no element, + // and emits NO predicate — the last shape that still returned + // every row silently after #3948. An empty `[]` is left alone: + // it means "no filter", and every path already treats it that + // way — so it falls through to the shape check below, which + // passes it (an array IS an object). + throw malformedFilterArrayError(parsedFilter); + } + // [#4181] Parsed-but-unusable is the same failure one step later: + // `?filter=5` / `?filter="open"` / `?filter=null` all yield a + // non-object `where` that no driver reads. #4121 above catches the + // array shapes; this catches the scalar ones, and together they are + // what lets the #4164 merge below trust `where` to be an object. + if (parsedFilter === null || typeof parsedFilter !== 'object') { + throw unusableFilterError( + filterKey, + `must be a filter object or condition array, received ${parsedFilter === null ? 'null' : typeof parsedFilter}`, + ); + } + options.where = parsedFilter; + } + } + + // Expand: the folded slot value → `Record`. A comma + // list (string) and a name array both lower to `{name: {object: name}}`; + // the advanced `{rel: QueryAST}` map a caller may send directly on + // `POST /data/:object/query` passes through as-is. Lowering the ARRAY + // shape here (not just the string) also closes a pre-#3795 gap: a raw + // name array used to survive this block whole, so the #4226 gate read + // its INDICES as relation names and refused real requests with + // "Unknown field '0'". + const expandValue = options.expand; + const expandNames: string[] = []; + if (typeof expandValue === 'string') { + expandNames.push(...expandValue.split(',').map((s: string) => s.trim()).filter(Boolean)); + } else if (Array.isArray(expandValue)) { + expandNames.push(...expandValue); + } + if (typeof options.expand !== 'object' || options.expand === null || Array.isArray(options.expand)) { + delete options.expand; + } + if (expandNames.length > 0 && !options.expand) { + options.expand = {} as Record; + for (const rel of expandNames) { + options.expand[rel] = { object: rel }; + } + } + // [#4226] Both routes into `expand` are gated: the comma-list spellings + // collected above, and the advanced `{rel: QueryAST}` map a caller may + // send directly on `POST /data/:object/query`. Validating the map's KEYS + // rather than `expandNames` is what covers the second one. + if (options.expand && typeof options.expand === 'object') { + host.assertExpandTargetsExist(request.object, Object.keys(options.expand)); + // [#22661] …then every level's TARGET is asked its declared exposure, + // and an entry the decision does not serve is withheld (the field + // answers as an unexpanded lookup) — see {@link servedExpand}. Ahead + // of the `internal: true` walk below, so nothing about a withheld + // target is judged, named or read. + options.expand = host.servedExpand(request.object, options.expand, 0); + if (Object.keys(options.expand).length === 0) delete 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) { + host.assertSearchFieldsAreSearchable( + request.object, options.searchFields, wireSpelling.searchFields ?? 'searchFields', + ); + } else if (options.search !== null && typeof options.search === 'object' + && (options.search as any)?.fields != null) { + host.assertSearchFieldsAreSearchable( + request.object, (options.search as any).fields, wireSpelling.search ?? 'search', + ); + } + // [#21207] …and on a stored-metadata table a search never scans the body + // or content-hash columns: refused when named, narrowed away otherwise. + // Runs after the searchability gate above, so a name that is not + // searchable at all keeps its own answer. [#21544] The ONE narrowing, + // a module function the in-process reader-context seam calls too. + const narrowedSearchFields = narrowStoredMetadataSearch( + request.object, options, host.engine?.registry?.getObject?.(request.object), wireSpelling, + ); + if (narrowedSearchFields) options.searchFields = narrowedSearchFields; + + // Boolean fields. [#26644] `count` is CHECKED, not merely lowered: a + // value that is neither true nor false is refused rather than left to + // read as "count anyway" below (see {@link countFlagValue}). + if (options.distinct === 'true') options.distinct = true; + else if (options.distinct === 'false') options.distinct = false; + if (options.count != null) options.count = countFlagValue(options.count, slotParam('count')); + + // [#2926 ⑩] Every supported OData-style `$` alias has been consumed and + // deleted above ($top/$skip/$orderby/$select/$count/$search/ + // $searchFields/$filter/$expand). A `$`-prefixed key can never be a + // field name, so anything left is an unsupported query parameter — + // fail loudly instead of letting it fall into the implicit-filter + // bucket below, where it silently matched zero rows (or, before the + // $filter alias existed, was dropped entirely and returned the + // UNFILTERED page — a footgun for scripts resolving ids by name). + const unsupportedDollarParams = Object.keys(options).filter((k) => k.startsWith('$')); + if (unsupportedDollarParams.length > 0) { + // [#16066] The supported set is QUOTED from the declaration, never + // re-typed here: a spelling added to the spec table used to leave + // this sentence naming a set the door no longer had, so the caller + // was told to use a parameter list that was already wrong. + const err: any = new Error( + `Unsupported query parameter(s): ${unsupportedDollarParams.join(', ')}. ` + + `Supported $-prefixed parameters: ${QUERY_TRANSPORT_DOLLAR_PARAMS.join(', ')}.`, + ); + err.status = 400; + err.code = 'UNSUPPORTED_QUERY_PARAM'; + throw err; + } + + // [#4134] A leftover key is about to be lowered into a field-equality + // predicate (or, when an explicit `where` won, dropped on the floor). + // Both readings are only sound if the key names a REAL field: a filter + // on a field that does not exist can never match, so `?pageSize=5` + // returned 200 + `total: 0` — indistinguishable from "no data". The + // write path already rejects the same input loudly (`INVALID_FIELD`); + // this is the read half of that one piece of knowledge, and the mirror + // of #3948's rule for driver-memory: an unapplied filter must not look + // like a satisfied one. + // + // Validated BEFORE the `!options.where` branch below, so a bad param is + // a 400 whether or not the caller also sent an explicit filter — the + // failure must not depend on which other params rode along. + const leftoverParams = Object.keys(options).filter((k) => !RESERVED_LIST_QUERY_PARAMS.has(k)); + if (leftoverParams.length > 0) { + host.assertQueryParamsAreFields(request.object, leftoverParams); + } + + // [#7534] The same question, on the EXPLICIT filter the caller wrote — + // the sibling door #4134's fix never reached. `options.where` is a + // lowered `FilterCondition` by this point whichever of the three doors + // carried it (`where` object, `$filter` string, filter AST), so one call + // covers all three. Placed here, and not earlier, on purpose: see + // `assertFilterFieldsExist` for why it runs after the param gate above + // and before the #4164 merge below. + host.assertFilterFieldsExist(request.object, options.where, filterKey); + + // Flat field filters: REST-style query params like ?id=abc&status=open + // are implicit field-level equality predicates. Every leftover key is a + // verified field name by this point — the #4134 gate above runs FIRST, + // which is exactly what makes the merge below safe: an unknown name + // already 400'd, so nothing merged here can be a predicate that matches + // nothing. + // + // [#4164] Implicit predicates now COMPOSE with an explicit `where` + // instead of being silently dropped. `?filter={...}&status=open` means + // what it says — both apply, `{ $and: [explicit, implicit] }` — the + // same composition the engine itself uses to fold the `$search` + // predicate or an expand's declared filter into an existing `where`, + // and a combinator the #3774 conformance suite pins across every + // FilterCondition backend. Contradictory sides need no special case: + // both predicates apply and the intersection is an HONEST zero (the + // filters ran), unlike the pre-#4134 zero (a filter that never ran). + // Consuming the keys here also stops them riding to `engine.find` as + // stray top-level AST junk, and count() below reads the same + // `options.where`, so pagination totals see the merged predicate too. + // + // #4164 shipped with a `typeof explicitWhere === 'object'` guard here, + // because the parse tolerance above could leave a raw unparseable string + // on `where` and folding real predicates into that garbage would neither + // apply them nor surface the bug. #4181 fixed that at the source — a + // filter now either parses to an object or 400s — so `where` is an + // object or absent by construction and the guard is gone with it. + const explicitWhere = options.where; + if (leftoverParams.length > 0) { + const implicitFilters: Record = {}; + for (const key of leftoverParams) { + implicitFilters[key] = options[key]; + delete options[key]; + } + // An absent or empty explicit filter (`?filter={}`, `?filter=`) is + // vacuous — the implicit predicates stand alone rather than being + // wrapped in a one-armed `$and`. + options.where = !explicitWhere || Object.keys(explicitWhere).length === 0 + ? implicitFilters + : { $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. + host.assertGroupByFieldsExist(request.object, options.groupBy); + host.assertAggregationFieldsExist(request.object, options.aggregations); + // [#25819] An aggregate axis sent as an EMPTY list, with no non-empty + // axis beside it, asks for an aggregate of nothing. The routing below + // reads it as no aggregate and handed the key to `engine.find`, which + // refuses an option it does not execute with an uncoded error whose + // words carry the quoted object name — read at the REST door as the + // unknown-object heuristic: `404 OBJECT_NOT_FOUND`, "Object '…' is not + // registered", for a registered object. No shipped caller sends `[]` + // meaning the plain rows, so it is refused for what it is. An empty + // list beside a non-empty one is served as before (`groupBy` with + // `aggregations: []` lists the groups). + { + const emptyAxes = (['groupBy', 'aggregations'] as const) + .filter((k) => Array.isArray(options[k]) && options[k].length === 0); + const servedAxis = (['groupBy', 'aggregations'] as const) + .some((k) => Array.isArray(options[k]) && options[k].length > 0); + if (emptyAxes.length > 0 && !servedAxis) { + const named = emptyAxes.map((k) => `'query.${k}'`).join(' and '); + const err: any = new Error( + `${named} ${emptyAxes.length > 1 ? 'are empty lists' : 'is an empty list'} on object ` + + `'${request.object}': an aggregate query names at least one aggregation, or a non-empty ` + + `'groupBy' to list its groups. The query was NOT run. Send e.g. { "aggregations": ` + + `[{ "function": "count", "alias": "n" }] }, or omit ${emptyAxes.map((k) => `'${k}'`).join(' and ')} ` + + 'for the plain rows.', + ); + err.status = 400; + err.code = 'INVALID_QUERY'; + err.param = emptyAxes[emptyAxes.length - 1]; + err.object = request.object; + throw err; + } + } + // [#21086] A stored metadata body is served only as its type's read + // projection (see `redactStoredMetadataRows` below), and a GROUP KEY + // cannot be projected — so grouping by the body column of a + // stored-metadata table is refused, after the existence gates so an + // unknown name keeps its own answer. + const bodyGroupingRefusal = storedMetadataBodyGroupingRefusal(request.object, options.groupBy); + if (bodyGroupingRefusal) throw bodyGroupingRefusal; + // [#21120] …and the FILTER / SORT half of the same family (maintainer + // ruling A): a predicate or an order key on the stored body column + // evaluates the body — a filter oracle that rebuilds a withheld + // credential by probing, or an order over the same bytes — so it is + // refused here, in the same shape as the grouping refusal, before the + // engine is asked. [#21544] The columns a filter READS are collected by + // the family's ONE collector, which the reader-context seam calls too: + // every key's head AND every cross-field `{ $field }` comparand, at any + // depth, in `where` and each `aggregations[i].filter` — so a filter that + // reads the body or a hash without naming it as a key (a comparand), or + // names it below the ingress collector's depth backstop, is refused like + // a direct one. `[]` outside the family. + const filterFields = collectStoredMetadataFilterFields(request.object, options); + const sortFields = Array.isArray(options.orderBy) + ? (options.orderBy as ReadonlyArray<{ field?: unknown }>).map((e) => e?.field) + : []; + const bodyPredicateRefusal = storedMetadataBodyPredicateRefusal(request.object, { filterFields, sortFields }); + if (bodyPredicateRefusal) throw bodyPredicateRefusal; + // [#21207] The same three shapes on the stored CONTENT-HASH columns + // (maintainer ruling A on the second execution fork): a group key would + // serve the stored hash, a filter on it is an online verifier, a sort + // orders by it. Refused in the body column's envelope, before the engine. + const hashEvaluateRefusal = storedMetadataHashEvaluateRefusal(request.object, { + groupBy: options.groupBy, + filterFields, + sortFields, + }); + if (hashEvaluateRefusal) throw hashEvaluateRefusal; + + // [#22646] A field declared `internal: true` is withheld from every + // generic exit (#21197) — the engine omits it from every row. The + // EVALUATE positions do not follow that by omission: a filter, sort, + // group-by, aggregate operand, per-aggregation filter or a one-level + // expand's own filter/sort on such a column reveals the stored value by + // evaluating it (a filter is a confirmation oracle, a sort leaks order). + // So they are refused here, at the generic data door, after the + // existence gates and before the engine — the same shape, envelope and + // caller-independence as the stored-metadata body/hash family above. + // Internal fields in a `$search` set are handled on their own axes: the + // auto-default never scans one (engine), an explicit `$searchFields` + // naming one is refused (the gate above, #4254 posture). + host.assertNoInternalFieldEvaluated(request.object, options); + + // 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 + // ungrouped raw rows on drivers (e.g. SqlDriver) that don't natively + // honor groupBy/aggregations in find(). + const hasGroupBy = Array.isArray(options.groupBy) && options.groupBy.length > 0; + const hasAggregations = Array.isArray(options.aggregations) && options.aggregations.length > 0; + // The deployment's read limits (`OS_DATA_DEFAULT_PAGE_SIZE`, + // `OS_DATA_MAX_PAGE_SIZE`, `OS_DATA_QUERY_ROW_BUDGET`; the rule is + // `@objectstack/types`' `query-read-limits.ts`). Planned here, after + // every shape gate and before the engine, so a refusal leaves the + // driver unasked. All three unset returns the caller's own `limit`. + const readLimits = resolveQueryReadLimits(); + if (hasGroupBy || hasAggregations) { + // The grouped branch slices in memory and never reads `offset`, so + // the window it is judged on is the groups it returns. + const groupRead = { + surface: 'list' as const, + object: request.object, + limit: typeof options.limit === 'number' ? options.limit : undefined, + limitParam: slotParam('limit'), + }; + const groupPlan = planQueryReadWindow(groupRead, readLimits, console); + if (!groupPlan.probe) options.limit = groupPlan.limit; + const aggregateQuery: EngineAggregateOptions = { + where: options.where, + groupBy: options.groupBy, + aggregations: options.aggregations, + // Enforced engine-side since #4286 (step 3) — dropping it here + // was finding 1: the one wire path to aggregate() lost the + // clause before any executor could ever see it. + having: options.having, + // ADR-0061 `search` is declared on the query beside `groupBy` / + // `aggregations` with no carve-out, and the flat branch below + // hands it to `engine.find`. Leaving it out of THIS bag answered + // a grouped query under a search with the UNSEARCHED groups — + // no error, no warning. The engine expands it through the same + // expander `find` uses, so the header numbers are the grouping + // of exactly the rows the flat query returns. `searchFields` was + // validated above on both branches (#4254 gate). + search: options.search, + searchFields: options.searchFields, + context: options.context, + }; + const records = await host.engine.aggregate(request.object, aggregateQuery); + assertQueryReadRows(groupRead, groupPlan, records.length, console); + // Apply limit client-side (EngineAggregateOptions doesn't carry limit). + // `records` is the full grouped set, so its length IS the real total + // and `hasMore` follows from whether the slice dropped any groups. + const limited = typeof options.limit === 'number' && options.limit > 0 + ? records.slice(0, options.limit) + : records; + return { + object: request.object, + records: limited, + total: records.length, + hasMore: limited.length < records.length, + }; + } + + // [#4371 option 2] Strip the PROTOCOL-layer keys off the bag before it + // becomes the engine option bag — the engine now rejects option keys + // it does not execute, and these belong to this layer, not to it: + // - `object`: the POST-body convenience copy of the route object + // (reserved above so it is not read as a field filter). It used to + // ride the spread into the engine AST and OVERRIDE the resolved + // object, splitting `ast.object` from the table actually queried — + // a mismatch is refused, never resolved by picking a winner. + // - `count`: a response-shape flag this method consumed above (and, + // since #10757, one it actually HONOURS — see `countOptOut` below). + // - QueryAST tombstones (`cursor`/`joins`/`windowFunctions`/ + // `distinct`): reserved at the wire gate so they are not read as + // field filters; on the wire they stay ignored-with-tombstone-docs + // (schema parse is where they 400), and they must not leak to the + // engine as junk. + // - `having`: consumed by the aggregate branch above; the find path + // cannot serve it. + if (options.object != null && options.object !== request.object) { + const err: any = new Error( + `Conflicting object: the route addresses '${request.object}' but the query body ` + + `says '${options.object}'. The body 'object' key is a convenience copy of the ` + + 'route object and must match it.', + ); + err.status = 400; + err.code = 'QUERY_OBJECT_MISMATCH'; + throw err; + } + // [#10757] `$count=false` — read the flag BEFORE the strip below deletes + // it, because the strip is what kept it from ever being honoured: the + // parameter has been declared (`ODataQuerySchema.$count`, + // `packages/spec/src/api/odata.zod.ts`), aliased (`$count` → `count`, + // {@link QUERY_TRANSPORT_DOLLAR_ALIASES}), reserved from the implicit-field-filter + // bucket ({@link RESERVED_LIST_QUERY_PARAMS}), arity-checked and boolean- + // coerced — and then deleted unread, so every list request paid for the + // COUNT query below whether or not the caller wanted a `total`. + // + // Only an EXPLICIT `false` opts out. An absent `$count` keeps today's + // behaviour (count runs, `total` is reported) rather than taking OData's + // "absent means omit" reading: every existing caller sends nothing and + // reads `total`, so the OData default would silently break all of them. + // The parameter is therefore an opt-OUT here, and that asymmetry is + // deliberate — see the changeset for the wording that ships to consumers. + const countOptOut = options.count === false; + for (const k of ['object', 'count', 'joins', 'windowFunctions', 'cursor', 'distinct', 'having']) { + delete options[k]; + } + // [#21086] The generic data door is a read exit for the stored metadata + // body too: a `sys_metadata` / `sys_metadata_history` row's `metadata` + // column is served as its type's read projection — the one `/meta` + // serves — so a stored credential is withheld here exactly as there. + // The row's `type` selects the redactor, so it is read even when the + // projection named only the body, and taken back off before serving. + const bodyProjection = storedMetadataBodyProjection(request.object, options.fields); + if (bodyProjection.addedType) options.fields = bodyProjection.fields; + // A list read with no `limit` takes the default page size, and its + // answer says whether more rows exist (`hasMore` / `total` below). A + // PROBE limit (a row budget with no page size) is the server's, not the + // caller's: the answer is refused when it overruns, and otherwise it is + // the whole set, so the probe is taken back off before the page + // metadata below reads `limit`. + const listRead = { + surface: 'list' as const, + object: request.object, + limit: typeof options.limit === 'number' ? options.limit : undefined, + offset: typeof options.offset === 'number' ? options.offset : undefined, + limitParam: slotParam('limit'), + }; + const listPlan = planQueryReadWindow(listRead, readLimits, console); + if (listPlan.limit !== undefined) options.limit = listPlan.limit; + // [#29764] The bag `engine.find` receives is typed as the engine declares + // it. `options` is the caller's normalized request bag, and by this line + // every key the normalizer does not hand on has been folded, deleted or + // moved into `where`. What can still stand is the find set below, plus + // `groupBy` / `aggregations` holding null at most (a non-empty one took + // the aggregate branch above, an empty one was refused). The engine reads + // a null option as absent, so leaving those two out changes nothing. Keys + // are copied only when present, so the engine sees the same bag as before. + const findQuery: EngineQueryOptions = { + ...(options.context !== undefined ? { context: options.context } : {}), + ...(options.where !== undefined ? { where: options.where } : {}), + ...(options.fields !== undefined ? { fields: options.fields } : {}), + ...(options.orderBy !== undefined ? { orderBy: options.orderBy } : {}), + ...(options.limit !== undefined ? { limit: options.limit } : {}), + ...(options.offset !== undefined ? { offset: options.offset } : {}), + ...(options.search !== undefined ? { search: options.search } : {}), + ...(options.searchFields !== undefined ? { searchFields: options.searchFields } : {}), + ...(options.expand !== undefined ? { expand: options.expand } : {}), + }; + // [#21207] …and its stored CONTENT HASH (`checksum`, `previous_checksum`) + // is served in keyed form — never the stored value, which beside the + // projected body confirms a guess at the withheld material offline — + // under the provider's key, or the process-scoped ephemeral one. + const found = await host.engine.find(request.object, findQuery); + if (listPlan.probe) { + assertQueryReadRows(listRead, listPlan, found.length, console); + delete options.limit; + } + const records = await serveStoredMetadataHashColumnRows( + request.object, + redactStoredMetadataRows( + request.object, + found, + { dropType: bodyProjection.addedType }, + ), + host.storedHashDigest(), + ); + // Pagination metadata. When a `limit` is present the response is a single + // page, so `records.length` is the page size — NOT the match total. Run a + // count over the same `where` so the client can render total pages and know + // whether more pages remain (true server-side pagination). Without a limit + // AND without an offset the full result set is returned, so its length + // already IS the total. + // + // [#28547] An offset with no limit is a paged request too. The read + // returns every match AFTER the offset, so `records.length` is the + // match count MINUS the offset: `?$skip=3&$count=true` over 14 rows + // answered `total: 11` while `?$skip=3&$top=100&$count=true` answered + // 14, and an offset past the end answered `total: 0`. Adding the offset + // back is not the fix (it says 999 for `offset: 999` over 14 rows), so + // such a request takes the one count path below, `$count=false` arm + // included: `total` is the filtered count whatever the offset. + // + // [#22790] The count takes the query's `search` / `searchFields` too: the + // engine expands them through the one ADR-0061 expander `find` uses, so + // `total` counts exactly the rows this query pages. A search used to skip + // the count and report a page-local estimate (`offset + page + 1`) that + // the Console rendered as the record count. + // + // [#22588] `limit: 0` IS a limit. It asks for no rows (objectstack#6485, + // the drivers' pagination conformance), so `records.length` is 0 and was + // reported as the total: `?$top=0` answered `total: 0` while `?$top=1` + // answered the true count, and the Console's footer probe — which sends + // exactly `$top: 0` to read the total and nothing else — showed `0` on + // every list page. Gated `> 0`, a zero-row page fell into the "no limit, + // the full set is the total" arm, which is true of every limit except + // this one. So `>= 0`: a zero-row request is a paged request like any + // other, and the three arms below keep their meaning for it — the count + // runs (`total` is the filtered count, whatever the page size), and + // `$count=false` still omits `total` instead of reporting the page's 0. + const pageLimit = typeof options.limit === 'number' && options.limit >= 0 ? options.limit : undefined; + const pageOffset = typeof options.offset === 'number' && options.offset > 0 ? options.offset : 0; + let total: number | undefined = records.length; + let hasMore = false; + if (pageLimit !== undefined || pageOffset > 0) { + // `distinct` used to suppress the count here too — #4286 finding 2: + // the flag's ONLY observable effect platform-wide, on a capability + // that never deduplicated a row. Removed with `query.distinct` + // (tombstoned in spec 17); `total`/`hasMore` are truthful again. + if (countOptOut) { + // [#10757] The caller said it does not need `total`, so the + // COUNT query is not issued at all — that is the whole point of + // the parameter, and on a remote database it is a full round + // trip saved per list request. + // + // `total` is OMITTED rather than estimated. `FindDataResponse` + // declares it optional ("Total number of records matching the + // filter (IF REQUESTED)", + // `packages/spec/src/api/protocol.zod.ts`), so absent is the + // declared shape for "not requested" — and it is the only + // honest one: a real number was available and the caller + // declined it. Handing back a plausible-looking guess is how a + // page-local estimate ends up rendered as a record count. + // + // `hasMore` is still answered, from the page alone: a FULL page + // means there may be more. It never over-reports the data — it + // can only say "maybe more" on an exactly-full last page. + // With an offset and no limit, every row after the offset came + // back, so there is no further page. + hasMore = pageLimit !== undefined && records.length === pageLimit; + total = undefined; + } else { + // [#10757] `counted` is a separate, always-assigned local so + // `hasMore` below compares against a `number`: `total` became + // optional when `$count=false` gained the right to omit it, and + // TypeScript cannot narrow it back across the try/catch. + let counted: number; + try { + const countQuery: EngineCountOptions = { + where: options.where, + search: options.search, + searchFields: options.searchFields, + context: options.context, + }; + counted = await host.engine.count(request.object, countQuery); + } catch { + // engine.count() has its own find().length fallback; if it still + // throws, degrade to a page-local total rather than failing the list. + counted = pageOffset + records.length; + } + total = counted; + hasMore = pageOffset + records.length < counted; + } + } + return { + object: request.object, + records, + // [#10757] Omitted, not `undefined`-valued: a JSON body carrying + // `"total": null` (or a key some serializers keep) reads as "the + // total is nothing", which is a different claim from "no total was + // requested". The key is simply absent. + ...(total === undefined ? {} : { total }), + hasMore, + }; + } + /** The body of `ObjectStackProtocolImplementation#getData`; the method's docblock there is its contract. */ static async getData(host: Pick, request: { object: string, id: string, expand?: string | string[], select?: string | string[], context?: any }): Promise<{ object: string; diff --git a/packages/metadata-protocol/src/protocol-host.ts b/packages/metadata-protocol/src/protocol-host.ts index 7f50fad31b7..6734a29ae7f 100644 --- a/packages/metadata-protocol/src/protocol-host.ts +++ b/packages/metadata-protocol/src/protocol-host.ts @@ -61,12 +61,19 @@ export interface ProtocolHost { readonly applyRemoteMetadataMutation: ObjectStackProtocolImplementation['applyRemoteMetadataMutation']; readonly applySeedBodies: ObjectStackProtocolImplementation['applySeedBodies']; readonly artifactLockLayerAt: ObjectStackProtocolImplementation['artifactLockLayerAt']; + readonly assertAggregationFieldsExist: ObjectStackProtocolImplementation['assertAggregationFieldsExist']; readonly assertExpandTargetsExist: ObjectStackProtocolImplementation['assertExpandTargetsExist']; + readonly assertFilterFieldsExist: ObjectStackProtocolImplementation['assertFilterFieldsExist']; + readonly assertGroupByFieldsExist: ObjectStackProtocolImplementation['assertGroupByFieldsExist']; readonly assertLockAllowsDelete: ObjectStackProtocolImplementation['assertLockAllowsDelete']; readonly assertLockAllowsWrite: ObjectStackProtocolImplementation['assertLockAllowsWrite']; + readonly assertNoInternalFieldEvaluated: ObjectStackProtocolImplementation['assertNoInternalFieldEvaluated']; readonly assertObjectRegistered: ObjectStackProtocolImplementation['assertObjectRegistered']; readonly assertProjectionFieldsExist: ObjectStackProtocolImplementation['assertProjectionFieldsExist']; + readonly assertQueryParamsAreFields: ObjectStackProtocolImplementation['assertQueryParamsAreFields']; readonly assertRuntimeAuthoringRules: ObjectStackProtocolImplementation['assertRuntimeAuthoringRules']; + readonly assertSearchFieldsAreSearchable: ObjectStackProtocolImplementation['assertSearchFieldsAreSearchable']; + readonly assertSortFieldsExist: ObjectStackProtocolImplementation['assertSortFieldsExist']; readonly assertVersionMatch: ObjectStackProtocolImplementation['assertVersionMatch']; readonly authoredLockFamily: ObjectStackProtocolImplementation['authoredLockFamily']; readonly authoringChannel: ObjectStackProtocolImplementation['authoringChannel']; diff --git a/packages/metadata-protocol/src/protocol.query-transport-dialect.test.ts b/packages/metadata-protocol/src/protocol.query-transport-dialect.test.ts index 9c2e7f72b37..e41df2f061f 100644 --- a/packages/metadata-protocol/src/protocol.query-transport-dialect.test.ts +++ b/packages/metadata-protocol/src/protocol.query-transport-dialect.test.ts @@ -212,7 +212,9 @@ function resolveCanonical(name: string): string { // --------------------------------------------------------------------------- describe('[#16066] §3 the single source is the spec export', () => { - const source = readFileSync(resolve(HERE, 'protocol.ts'), 'utf8'); + // [#30518] `findData`'s body moved to its facet (`protocol-data-crud.ts`), + // so the call site is read there; the class stays read for a local table. + const source = ['protocol.ts', 'protocol-data-crud.ts'].map((f) => readFileSync(resolve(HERE, f), 'utf8')).join('\n'); it('no local `WIRE_QUERY_ALIAS_SLOTS` / `WIRE_DOLLAR_ALIASES` declaration survives', () => { expect(source).not.toMatch(/(?:const|let|var)\s+WIRE_QUERY_ALIAS_SLOTS\b/); diff --git a/packages/metadata-protocol/src/protocol.ts b/packages/metadata-protocol/src/protocol.ts index 4f111ccebb4..823b198fb39 100644 --- a/packages/metadata-protocol/src/protocol.ts +++ b/packages/metadata-protocol/src/protocol.ts @@ -8,7 +8,7 @@ import type { } from '@objectstack/spec/api'; import { IDataEngine, engineCanRollBack, recordNotFoundError } from '@objectstack/core'; import { resolveThrownHttpError } from '@objectstack/types'; -import { assertQueryReadRows, planQueryReadWindow, resolveQueryReadLimits } from '@objectstack/types'; +import { resolveQueryReadLimits } from '@objectstack/types'; import { type TenancyPosture } from '@objectstack/spec/security'; // [commit 376c70f98] The derived `version` this file's `getDiscovery()` serves as the // `DiscoverySchema` "System Identity" field — never a literal again. @@ -76,17 +76,8 @@ import type { SeedAppliedLandedParsed } from '@objectstack/spec/api'; import { readServiceSelfInfo, readChannelRoute, readAuthFamilies, isSubscribableChannel, CHANNEL_SURFACE_SLOTS, resolveDiscoveryEnvironment } from '@objectstack/spec/api'; // [#24395] The save's authoring surface. import { - parseFilterAST, - isFilterAST, REFERENCE_VALUE_TYPES, - foldQueryAliasSlots, - QUERY_TRANSPORT_ALIAS_SLOTS, - QUERY_TRANSPORT_DOLLAR_ALIASES, - QUERY_TRANSPORT_DOLLAR_PARAMS, type DroppedFieldsEvent, - type EngineAggregateOptions, - type EngineCountOptions, - type EngineQueryOptions, type EngineQueryOptionsParsed, externalBindingDatasourceRefusal, } from '@objectstack/spec/data'; @@ -167,13 +158,7 @@ import { // and `getData` serve `sys_metadata` / `sys_metadata_history` rows, whose // `metadata` column is the same stored body, through the same redactor. import { - redactStoredMetadataRows, - storedMetadataBodyGroupingRefusal, - storedMetadataBodyPredicateRefusal, - storedMetadataBodyProjection, ephemeralStoredHashDigest, - serveStoredMetadataHashColumnRows, - storedMetadataHashEvaluateRefusal, type StoredHashDigest, } from './metadata-redaction.js'; import type { @@ -459,7 +444,6 @@ import { readPublishBackfillRequest } from './protocol-publish-backfill-request. import type { PublishBackfillEntry, PublishBackfillReceipt } from './protocol-publish-backfill-request.js'; export { PUBLISH_BACKFILL_PAGE_SIZE, publishBackfillRefusal } from './protocol-publish-backfill-request.js'; export type { PublishBackfillEntry, PublishBackfillReceipt } from './protocol-publish-backfill-request.js'; -import { malformedFilterArrayError } from './protocol-data-request-shapes.js'; export { byIdUpdateRecord } from './protocol-data-request-shapes.js'; import { projectStorableViewBody, @@ -532,18 +516,6 @@ export { toRowApiError, } from './protocol-client-failures.js'; export type { BatchDataLoopOutcome, BatchDataRowResult } from './protocol-client-failures.js'; -import { - RESERVED_LIST_QUERY_PARAMS, - assertQueryParamArity, - collectStoredMetadataFilterFields, - conflictingQueryParamsError, - countFlagValue, - narrowStoredMetadataSearch, - normalizeSortNodes, - pageWindowValue, - projectionFieldsValue, - unusableFilterError, -} from './protocol-list-query-params.js'; export { FILTER_LOGICAL_KEYS, ODATA_SPELLING, @@ -1194,12 +1166,19 @@ export class ObjectStackProtocolImplementation implements applyRemoteMetadataMutation: (...a) => protocol.applyRemoteMetadataMutation(...a), applySeedBodies: (...a) => protocol.applySeedBodies(...a), artifactLockLayerAt: (...a) => protocol.artifactLockLayerAt(...a), + assertAggregationFieldsExist: (...a) => protocol.assertAggregationFieldsExist(...a), assertExpandTargetsExist: (...a) => protocol.assertExpandTargetsExist(...a), + assertFilterFieldsExist: (...a) => protocol.assertFilterFieldsExist(...a), + assertGroupByFieldsExist: (...a) => protocol.assertGroupByFieldsExist(...a), assertLockAllowsDelete: (...a) => protocol.assertLockAllowsDelete(...a), assertLockAllowsWrite: (...a) => protocol.assertLockAllowsWrite(...a), + assertNoInternalFieldEvaluated: (...a) => protocol.assertNoInternalFieldEvaluated(...a), assertObjectRegistered: (...a) => protocol.assertObjectRegistered(...a), assertProjectionFieldsExist: (...a) => protocol.assertProjectionFieldsExist(...a), + assertQueryParamsAreFields: (...a) => protocol.assertQueryParamsAreFields(...a), assertRuntimeAuthoringRules: (...a) => protocol.assertRuntimeAuthoringRules(...a), + assertSearchFieldsAreSearchable: (...a) => protocol.assertSearchFieldsAreSearchable(...a), + assertSortFieldsExist: (...a) => protocol.assertSortFieldsExist(...a), assertVersionMatch: (...a) => protocol.assertVersionMatch(...a), authoredLockFamily: (...a) => protocol.authoredLockFamily(...a), get authoringChannel() { return protocol.authoringChannel; }, @@ -5486,755 +5465,8 @@ export class ObjectStackProtocolImplementation implements object: string; records: any[]; }> { - // [#3770] Existence first: an unregistered object is a 404 before any - // query parameter is even parsed, so an unknown name can never be - // probed for query-shape validity (nor reach the driver as a table). - this.assertObjectRegistered(request.object); - const options: any = { ...request.query }; - // The execution context is SERVER-DERIVED and never caller input. - // - // `request.query` is the raw request bag on every ingress that reaches - // here — the REST `POST /data/:object/query` route hands `req.body` - // straight in as `query`. `context` is in the known-params set below, so - // it was not swept into the implicit-filter bucket either: a caller's - // `context` survived this spread and, because the assignment below is - // conditional, became the operation's execution context whenever no - // server context resolved (before #3963 an anonymous request on a - // `requireAuth: false` deployment; the invariant holds regardless). - // - // What rides on it is total: plugin-security's middleware opens with - // `if (opCtx.context?.isSystem) return next()` — the entire RLS / FLS / - // CRUD chain skipped — and `__expandRead` marks a read as an expansion - // sub-read (#2850; it waived the object-level CRUD gate for "public" - // objects until #7626 removed that). Neither is ever schema-stripped on - // this path: `ExecutionContextSchema.parse` runs only in - // `engine.createContext`, which the read path does not use. - // - // Route-level `enforceAuth` is what kept that from being reachable, so - // this was a fail-OPEN default one layer down. Drop any inbound - // `context` unconditionally: the protocol must not depend on a gate - // above it staying switched on. - delete options.context; - - // [#7321] Arity BEFORE any read, fold or coercion. `IHttpRequest.query` - // is `Record< string, string | string[] >`; the array arm is real (the - // `node:http` adapter hands `?x=1&x=2` through as `['1','2']`, measured - // over a socket on #6878), and every coercion below was written for the - // string arm. `Number(['1','2'])` is `NaN`, so `?$top=1&$top=2` used to - // reach the driver as `limit: NaN` — a wrong answer served as a 200. - // - // Single-vs-multi is a PER-PARAMETER judgement, never a sweep: `$select` - // / `$expand` / `$searchFields` / `$orderby` accept the array arm on - // purpose and are untouched here. See - // {@link ARRAY_VALUED_QUERY_SLOTS} for the full disposition. - assertQueryParamArity(options); - - // Forward the dispatcher's ExecutionContext so RBAC/RLS middleware - // can apply per-request enforcement. The protocol layer is purely - // a normalizer — it must never strip security context. - if (request.context !== undefined) { - options.context = request.context; - } - - // ==================================================================== - // Normalize legacy params → QueryAST standard (where/fields/orderBy/offset/expand) - // ==================================================================== - - // OData-style `$`-prefixed params → bare aliases that the rest of - // this function knows how to normalize. Without this step, params - // like `?$top=2&$orderby=...` survive into the catch-all - // implicit-filter pass below and get merged into `where` as - // bogus field-equality predicates (e.g. `where.$top = "2"`), - // which silently returns zero rows for every list endpoint. - // - // [#4226] `wireSpelling` remembers which alias each rewritten slot - // arrived under, so a rejection quotes the parameter the caller - // actually wrote. Telling someone who sent `?$orderby=…` that - // "'orderBy' is invalid" names a parameter absent from their request. - // - // [#7321] The table itself now lives at module scope - // ({@link QUERY_TRANSPORT_DOLLAR_ALIASES}) so the arity survey above and this fold - // cannot drift apart on which `$` spellings exist. - const wireSpelling: Record = {}; - for (const [dollar, bare] of QUERY_TRANSPORT_DOLLAR_ALIASES) { - if (options[dollar] != null && options[bare] == null) { - options[bare] = options[dollar]; - wireSpelling[bare] = dollar; - } - delete options[dollar]; - } - - // [#3795] One slot, one value. Every alias spelling of the six - // QueryAST slots resolves HERE, by the spec's own table, before the - // per-slot wire coercion below ever runs — so that coercion reads - // canonical keys only. An alias alone folds into its canonical key; - // redundant identical spellings collapse; different values for one - // slot are refused (the #4181 rule, generalized from the filter slot - // to all of them — four used to resolve BACKWARDS here, each in its - // own way, disagreeing with the spec's documented precedence and with - // the runtime dispatcher's copy of the same fold; `top`→`limit` - // joined the table last, via #4346). - // - // `arrivedAs` remembers which spelling carried each slot's value; - // composed with `wireSpelling` it names the parameter the caller - // actually wrote in every rejection below (#4226). - const spellingFor = (name: string): string => wireSpelling[name] ?? name; - const arrivedAs = foldQueryAliasSlots(options, QUERY_TRANSPORT_ALIAS_SLOTS, (conflict) => { - throw conflictingQueryParamsError(conflict, spellingFor); - }); - const slotParam = (canonical: string): string => spellingFor(arrivedAs[canonical] ?? canonical); - - // Numeric fields — coerce querystring strings. `top` already folded - // into `limit` above: the pair joined the #3795 slot table with #4346, - // replacing an open-coded rewrite here that resolved it BACKWARDS - // (`options.limit = Number(options.top)` — the alias overwrote the - // canonical key, so `{top: 1, limit: 3}` answered 1 over HTTP and 3 - // through a direct engine call). - // - // [#25229] The coercion is CHECKED, not bare (see - // {@link pageWindowValue}). This is the one statement every list door - // reaches with the window still unvalidated — `GET /data/:object`'s - // querystring, `POST /data/:object/query`'s body (whose schema checks - // the type, not the sign) and the runtime dispatcher — so the refusal - // lives here, once. - if (options.limit != null) options.limit = pageWindowValue(options.limit, slotParam('limit')); - if (options.offset != null) options.offset = pageWindowValue(options.offset, slotParam('offset')); - - // Projection: comma-separated string → array. A single-value - // querystring param arrives as a bare string — `?fields=name` or the - // folded `?select=` / `$select` spellings — which drivers' `.map()` - // calls over `query.fields` would otherwise throw on. - // - // [#26644] …and a value naming no field is refused rather than becoming - // `[]`, which every driver serves as no projection at all (see - // {@link projectionFieldsValue}). - const projectionKey = slotParam('fields'); - if (typeof options.fields === 'string' || Array.isArray(options.fields)) { - options.fields = projectionFieldsValue(options.fields, projectionKey); - } else if (options.fields !== undefined) { - delete options.fields; - } - // [#4226] Unknown projection columns are refused rather than dropped — - // see `assertProjectionFieldsExist` for why this axis' silent failure - // returned MORE than was asked for. - this.assertProjectionFieldsExist(request.object, options.fields, projectionKey); - - // Sort: every wire shape → SortNode[]. - // - // [#4226] `normalizeSortNodes` folds the two shapes that used to fall - // through this block untouched — `string[]` and `{field: direction}` — - // and refuses the ones it cannot read. Before it, "not a string and not - // an array" simply skipped the branch, leaving a value on `orderBy` - // that `SqlDriver`'s `Array.isArray` guard then declined to turn into - // an ORDER BY clause: no sort, no error, no way to tell. - const sortValue = options.orderBy; - const sortKey = slotParam('orderBy'); - if (sortValue === undefined || sortValue === null) { - // Nothing to sort by — and an explicit `orderBy: null` must not ride - // to the engine as a value every driver quietly declines to read. - delete options.orderBy; - } else { - const orderBy = normalizeSortNodes(sortValue, sortKey); - // [#4226] Validated on the NORMALIZED nodes, so one gate covers - // every spelling — `?sort=no_such`, `['-no_such']`, `{no_such: - // 'desc'}` and `[{field:'no_such'}]` are one mistake with one - // answer. Assigned only after it passes, so a rejected sort cannot - // leave a half-applied one behind. - this.assertSortFieldsExist(request.object, orderBy, sortKey); - if (orderBy.length > 0) options.orderBy = orderBy; - else delete options.orderBy; - } - - // Filter: the folded slot value → a usable `FilterCondition` on - // `where`, or a rejection. The four spellings of this slot - // (`where`/`filter`/`filters`/`$filter`) already resolved through the - // #3795 fold above — #4181's one-slot-one-value rule, which this block - // pioneered before the fold generalized it. - const filterKey = slotParam('where'); - - if (options.where !== undefined) { - let parsedFilter = options.where; - // A blank `?filter=` is ABSENT, not malformed — the same `length > 0` - // guard the export route applies before parsing. Deleting `where` - // here (rather than leaving `''` on it) is what lets every consumer - // below test presence with a plain falsy check. - if (typeof parsedFilter === 'string' && parsedFilter.trim() === '') { - delete options.where; - } else { - // [#4181] JSON string → object. Parse failure is a REJECTION, not - // a fallback. The `catch { /* keep as-is */ }` this replaces left - // the raw string on `where`, a shape no driver consumes — so the - // filter was dropped whole and `?filter={status:done` (one missing - // quote) answered 200 with the UNFILTERED page. Worst member of - // the #3948 family: #4134 zeroed, #4164 dropped one predicate, - // this returned everything. - // - // The sibling `GET /data/:object/export` route has rejected this - // exact input since it was written (`400 INVALID_REQUEST`, - // "filter must be JSON"); the list path was the outlier. The - // guard lives HERE, in the shared normalizer, so `GET - // /data/:object`, `POST /data/:object/query` and the runtime - // dispatcher all inherit one answer instead of three. - if (typeof parsedFilter === 'string') { - try { - parsedFilter = JSON.parse(parsedFilter); - } catch { - throw unusableFilterError(filterKey, 'must be valid JSON'); - } - } - // Filter AST array → FilterCondition object - if (isFilterAST(parsedFilter)) { - parsedFilter = parseFilterAST(parsedFilter); - } else if (Array.isArray(parsedFilter) && parsedFilter.length > 0) { - // [#4121] `isFilterAST` was being read as a *conversion* gate, - // so an array it refused was assigned to `where` unconverted — - // an opaque value the driver then had to make sense of. Every - // driver now fails on it, so this is not a narrowing; it moves - // the failure to where the malformed filter actually arrived, - // with the request's own vocabulary in the message instead of a - // driver-internal one. - // - // It also closes what the driver-side fix could not: a lone - // `['and']` / `['or']` sets the join mode, matches no element, - // and emits NO predicate — the last shape that still returned - // every row silently after #3948. An empty `[]` is left alone: - // it means "no filter", and every path already treats it that - // way — so it falls through to the shape check below, which - // passes it (an array IS an object). - throw malformedFilterArrayError(parsedFilter); - } - // [#4181] Parsed-but-unusable is the same failure one step later: - // `?filter=5` / `?filter="open"` / `?filter=null` all yield a - // non-object `where` that no driver reads. #4121 above catches the - // array shapes; this catches the scalar ones, and together they are - // what lets the #4164 merge below trust `where` to be an object. - if (parsedFilter === null || typeof parsedFilter !== 'object') { - throw unusableFilterError( - filterKey, - `must be a filter object or condition array, received ${parsedFilter === null ? 'null' : typeof parsedFilter}`, - ); - } - options.where = parsedFilter; - } - } - - // Expand: the folded slot value → `Record`. A comma - // list (string) and a name array both lower to `{name: {object: name}}`; - // the advanced `{rel: QueryAST}` map a caller may send directly on - // `POST /data/:object/query` passes through as-is. Lowering the ARRAY - // shape here (not just the string) also closes a pre-#3795 gap: a raw - // name array used to survive this block whole, so the #4226 gate read - // its INDICES as relation names and refused real requests with - // "Unknown field '0'". - const expandValue = options.expand; - const expandNames: string[] = []; - if (typeof expandValue === 'string') { - expandNames.push(...expandValue.split(',').map((s: string) => s.trim()).filter(Boolean)); - } else if (Array.isArray(expandValue)) { - expandNames.push(...expandValue); - } - if (typeof options.expand !== 'object' || options.expand === null || Array.isArray(options.expand)) { - delete options.expand; - } - if (expandNames.length > 0 && !options.expand) { - options.expand = {} as Record; - for (const rel of expandNames) { - options.expand[rel] = { object: rel }; - } - } - // [#4226] Both routes into `expand` are gated: the comma-list spellings - // collected above, and the advanced `{rel: QueryAST}` map a caller may - // send directly on `POST /data/:object/query`. Validating the map's KEYS - // rather than `expandNames` is what covers the second one. - if (options.expand && typeof options.expand === 'object') { - this.assertExpandTargetsExist(request.object, Object.keys(options.expand)); - // [#22661] …then every level's TARGET is asked its declared exposure, - // and an entry the decision does not serve is withheld (the field - // answers as an unexpanded lookup) — see {@link servedExpand}. Ahead - // of the `internal: true` walk below, so nothing about a withheld - // target is judged, named or read. - options.expand = this.servedExpand(request.object, options.expand, 0); - if (Object.keys(options.expand).length === 0) delete 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', - ); - } - // [#21207] …and on a stored-metadata table a search never scans the body - // or content-hash columns: refused when named, narrowed away otherwise. - // Runs after the searchability gate above, so a name that is not - // searchable at all keeps its own answer. [#21544] The ONE narrowing, - // a module function the in-process reader-context seam calls too. - const narrowedSearchFields = narrowStoredMetadataSearch( - request.object, options, this.engine?.registry?.getObject?.(request.object), wireSpelling, - ); - if (narrowedSearchFields) options.searchFields = narrowedSearchFields; - - // Boolean fields. [#26644] `count` is CHECKED, not merely lowered: a - // value that is neither true nor false is refused rather than left to - // read as "count anyway" below (see {@link countFlagValue}). - if (options.distinct === 'true') options.distinct = true; - else if (options.distinct === 'false') options.distinct = false; - if (options.count != null) options.count = countFlagValue(options.count, slotParam('count')); - - // [#2926 ⑩] Every supported OData-style `$` alias has been consumed and - // deleted above ($top/$skip/$orderby/$select/$count/$search/ - // $searchFields/$filter/$expand). A `$`-prefixed key can never be a - // field name, so anything left is an unsupported query parameter — - // fail loudly instead of letting it fall into the implicit-filter - // bucket below, where it silently matched zero rows (or, before the - // $filter alias existed, was dropped entirely and returned the - // UNFILTERED page — a footgun for scripts resolving ids by name). - const unsupportedDollarParams = Object.keys(options).filter((k) => k.startsWith('$')); - if (unsupportedDollarParams.length > 0) { - // [#16066] The supported set is QUOTED from the declaration, never - // re-typed here: a spelling added to the spec table used to leave - // this sentence naming a set the door no longer had, so the caller - // was told to use a parameter list that was already wrong. - const err: any = new Error( - `Unsupported query parameter(s): ${unsupportedDollarParams.join(', ')}. ` + - `Supported $-prefixed parameters: ${QUERY_TRANSPORT_DOLLAR_PARAMS.join(', ')}.`, - ); - err.status = 400; - err.code = 'UNSUPPORTED_QUERY_PARAM'; - throw err; - } - - // [#4134] A leftover key is about to be lowered into a field-equality - // predicate (or, when an explicit `where` won, dropped on the floor). - // Both readings are only sound if the key names a REAL field: a filter - // on a field that does not exist can never match, so `?pageSize=5` - // returned 200 + `total: 0` — indistinguishable from "no data". The - // write path already rejects the same input loudly (`INVALID_FIELD`); - // this is the read half of that one piece of knowledge, and the mirror - // of #3948's rule for driver-memory: an unapplied filter must not look - // like a satisfied one. - // - // Validated BEFORE the `!options.where` branch below, so a bad param is - // a 400 whether or not the caller also sent an explicit filter — the - // failure must not depend on which other params rode along. - const leftoverParams = Object.keys(options).filter((k) => !RESERVED_LIST_QUERY_PARAMS.has(k)); - if (leftoverParams.length > 0) { - this.assertQueryParamsAreFields(request.object, leftoverParams); - } - - // [#7534] The same question, on the EXPLICIT filter the caller wrote — - // the sibling door #4134's fix never reached. `options.where` is a - // lowered `FilterCondition` by this point whichever of the three doors - // carried it (`where` object, `$filter` string, filter AST), so one call - // covers all three. Placed here, and not earlier, on purpose: see - // `assertFilterFieldsExist` for why it runs after the param gate above - // and before the #4164 merge below. - this.assertFilterFieldsExist(request.object, options.where, filterKey); - - // Flat field filters: REST-style query params like ?id=abc&status=open - // are implicit field-level equality predicates. Every leftover key is a - // verified field name by this point — the #4134 gate above runs FIRST, - // which is exactly what makes the merge below safe: an unknown name - // already 400'd, so nothing merged here can be a predicate that matches - // nothing. - // - // [#4164] Implicit predicates now COMPOSE with an explicit `where` - // instead of being silently dropped. `?filter={...}&status=open` means - // what it says — both apply, `{ $and: [explicit, implicit] }` — the - // same composition the engine itself uses to fold the `$search` - // predicate or an expand's declared filter into an existing `where`, - // and a combinator the #3774 conformance suite pins across every - // FilterCondition backend. Contradictory sides need no special case: - // both predicates apply and the intersection is an HONEST zero (the - // filters ran), unlike the pre-#4134 zero (a filter that never ran). - // Consuming the keys here also stops them riding to `engine.find` as - // stray top-level AST junk, and count() below reads the same - // `options.where`, so pagination totals see the merged predicate too. - // - // #4164 shipped with a `typeof explicitWhere === 'object'` guard here, - // because the parse tolerance above could leave a raw unparseable string - // on `where` and folding real predicates into that garbage would neither - // apply them nor surface the bug. #4181 fixed that at the source — a - // filter now either parses to an object or 400s — so `where` is an - // object or absent by construction and the guard is gone with it. - const explicitWhere = options.where; - if (leftoverParams.length > 0) { - const implicitFilters: Record = {}; - for (const key of leftoverParams) { - implicitFilters[key] = options[key]; - delete options[key]; - } - // An absent or empty explicit filter (`?filter={}`, `?filter=`) is - // vacuous — the implicit predicates stand alone rather than being - // wrapped in a one-armed `$and`. - options.where = !explicitWhere || Object.keys(explicitWhere).length === 0 - ? implicitFilters - : { $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); - // [#25819] An aggregate axis sent as an EMPTY list, with no non-empty - // axis beside it, asks for an aggregate of nothing. The routing below - // reads it as no aggregate and handed the key to `engine.find`, which - // refuses an option it does not execute with an uncoded error whose - // words carry the quoted object name — read at the REST door as the - // unknown-object heuristic: `404 OBJECT_NOT_FOUND`, "Object '…' is not - // registered", for a registered object. No shipped caller sends `[]` - // meaning the plain rows, so it is refused for what it is. An empty - // list beside a non-empty one is served as before (`groupBy` with - // `aggregations: []` lists the groups). - { - const emptyAxes = (['groupBy', 'aggregations'] as const) - .filter((k) => Array.isArray(options[k]) && options[k].length === 0); - const servedAxis = (['groupBy', 'aggregations'] as const) - .some((k) => Array.isArray(options[k]) && options[k].length > 0); - if (emptyAxes.length > 0 && !servedAxis) { - const named = emptyAxes.map((k) => `'query.${k}'`).join(' and '); - const err: any = new Error( - `${named} ${emptyAxes.length > 1 ? 'are empty lists' : 'is an empty list'} on object ` - + `'${request.object}': an aggregate query names at least one aggregation, or a non-empty ` - + `'groupBy' to list its groups. The query was NOT run. Send e.g. { "aggregations": ` - + `[{ "function": "count", "alias": "n" }] }, or omit ${emptyAxes.map((k) => `'${k}'`).join(' and ')} ` - + 'for the plain rows.', - ); - err.status = 400; - err.code = 'INVALID_QUERY'; - err.param = emptyAxes[emptyAxes.length - 1]; - err.object = request.object; - throw err; - } - } - // [#21086] A stored metadata body is served only as its type's read - // projection (see `redactStoredMetadataRows` below), and a GROUP KEY - // cannot be projected — so grouping by the body column of a - // stored-metadata table is refused, after the existence gates so an - // unknown name keeps its own answer. - const bodyGroupingRefusal = storedMetadataBodyGroupingRefusal(request.object, options.groupBy); - if (bodyGroupingRefusal) throw bodyGroupingRefusal; - // [#21120] …and the FILTER / SORT half of the same family (maintainer - // ruling A): a predicate or an order key on the stored body column - // evaluates the body — a filter oracle that rebuilds a withheld - // credential by probing, or an order over the same bytes — so it is - // refused here, in the same shape as the grouping refusal, before the - // engine is asked. [#21544] The columns a filter READS are collected by - // the family's ONE collector, which the reader-context seam calls too: - // every key's head AND every cross-field `{ $field }` comparand, at any - // depth, in `where` and each `aggregations[i].filter` — so a filter that - // reads the body or a hash without naming it as a key (a comparand), or - // names it below the ingress collector's depth backstop, is refused like - // a direct one. `[]` outside the family. - const filterFields = collectStoredMetadataFilterFields(request.object, options); - const sortFields = Array.isArray(options.orderBy) - ? (options.orderBy as ReadonlyArray<{ field?: unknown }>).map((e) => e?.field) - : []; - const bodyPredicateRefusal = storedMetadataBodyPredicateRefusal(request.object, { filterFields, sortFields }); - if (bodyPredicateRefusal) throw bodyPredicateRefusal; - // [#21207] The same three shapes on the stored CONTENT-HASH columns - // (maintainer ruling A on the second execution fork): a group key would - // serve the stored hash, a filter on it is an online verifier, a sort - // orders by it. Refused in the body column's envelope, before the engine. - const hashEvaluateRefusal = storedMetadataHashEvaluateRefusal(request.object, { - groupBy: options.groupBy, - filterFields, - sortFields, - }); - if (hashEvaluateRefusal) throw hashEvaluateRefusal; - - // [#22646] A field declared `internal: true` is withheld from every - // generic exit (#21197) — the engine omits it from every row. The - // EVALUATE positions do not follow that by omission: a filter, sort, - // group-by, aggregate operand, per-aggregation filter or a one-level - // expand's own filter/sort on such a column reveals the stored value by - // evaluating it (a filter is a confirmation oracle, a sort leaks order). - // So they are refused here, at the generic data door, after the - // existence gates and before the engine — the same shape, envelope and - // caller-independence as the stored-metadata body/hash family above. - // Internal fields in a `$search` set are handled on their own axes: the - // auto-default never scans one (engine), an explicit `$searchFields` - // naming one is refused (the gate above, #4254 posture). - this.assertNoInternalFieldEvaluated(request.object, options); - - // 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 - // ungrouped raw rows on drivers (e.g. SqlDriver) that don't natively - // honor groupBy/aggregations in find(). - const hasGroupBy = Array.isArray(options.groupBy) && options.groupBy.length > 0; - const hasAggregations = Array.isArray(options.aggregations) && options.aggregations.length > 0; - // The deployment's read limits (`OS_DATA_DEFAULT_PAGE_SIZE`, - // `OS_DATA_MAX_PAGE_SIZE`, `OS_DATA_QUERY_ROW_BUDGET`; the rule is - // `@objectstack/types`' `query-read-limits.ts`). Planned here, after - // every shape gate and before the engine, so a refusal leaves the - // driver unasked. All three unset returns the caller's own `limit`. - const readLimits = resolveQueryReadLimits(); - if (hasGroupBy || hasAggregations) { - // The grouped branch slices in memory and never reads `offset`, so - // the window it is judged on is the groups it returns. - const groupRead = { - surface: 'list' as const, - object: request.object, - limit: typeof options.limit === 'number' ? options.limit : undefined, - limitParam: slotParam('limit'), - }; - const groupPlan = planQueryReadWindow(groupRead, readLimits, console); - if (!groupPlan.probe) options.limit = groupPlan.limit; - const aggregateQuery: EngineAggregateOptions = { - where: options.where, - groupBy: options.groupBy, - aggregations: options.aggregations, - // Enforced engine-side since #4286 (step 3) — dropping it here - // was finding 1: the one wire path to aggregate() lost the - // clause before any executor could ever see it. - having: options.having, - // ADR-0061 `search` is declared on the query beside `groupBy` / - // `aggregations` with no carve-out, and the flat branch below - // hands it to `engine.find`. Leaving it out of THIS bag answered - // a grouped query under a search with the UNSEARCHED groups — - // no error, no warning. The engine expands it through the same - // expander `find` uses, so the header numbers are the grouping - // of exactly the rows the flat query returns. `searchFields` was - // validated above on both branches (#4254 gate). - search: options.search, - searchFields: options.searchFields, - context: options.context, - }; - const records = await this.engine.aggregate(request.object, aggregateQuery); - assertQueryReadRows(groupRead, groupPlan, records.length, console); - // Apply limit client-side (EngineAggregateOptions doesn't carry limit). - // `records` is the full grouped set, so its length IS the real total - // and `hasMore` follows from whether the slice dropped any groups. - const limited = typeof options.limit === 'number' && options.limit > 0 - ? records.slice(0, options.limit) - : records; - return { - object: request.object, - records: limited, - total: records.length, - hasMore: limited.length < records.length, - }; - } - - // [#4371 option 2] Strip the PROTOCOL-layer keys off the bag before it - // becomes the engine option bag — the engine now rejects option keys - // it does not execute, and these belong to this layer, not to it: - // - `object`: the POST-body convenience copy of the route object - // (reserved above so it is not read as a field filter). It used to - // ride the spread into the engine AST and OVERRIDE the resolved - // object, splitting `ast.object` from the table actually queried — - // a mismatch is refused, never resolved by picking a winner. - // - `count`: a response-shape flag this method consumed above (and, - // since #10757, one it actually HONOURS — see `countOptOut` below). - // - QueryAST tombstones (`cursor`/`joins`/`windowFunctions`/ - // `distinct`): reserved at the wire gate so they are not read as - // field filters; on the wire they stay ignored-with-tombstone-docs - // (schema parse is where they 400), and they must not leak to the - // engine as junk. - // - `having`: consumed by the aggregate branch above; the find path - // cannot serve it. - if (options.object != null && options.object !== request.object) { - const err: any = new Error( - `Conflicting object: the route addresses '${request.object}' but the query body ` + - `says '${options.object}'. The body 'object' key is a convenience copy of the ` + - 'route object and must match it.', - ); - err.status = 400; - err.code = 'QUERY_OBJECT_MISMATCH'; - throw err; - } - // [#10757] `$count=false` — read the flag BEFORE the strip below deletes - // it, because the strip is what kept it from ever being honoured: the - // parameter has been declared (`ODataQuerySchema.$count`, - // `packages/spec/src/api/odata.zod.ts`), aliased (`$count` → `count`, - // {@link QUERY_TRANSPORT_DOLLAR_ALIASES}), reserved from the implicit-field-filter - // bucket ({@link RESERVED_LIST_QUERY_PARAMS}), arity-checked and boolean- - // coerced — and then deleted unread, so every list request paid for the - // COUNT query below whether or not the caller wanted a `total`. - // - // Only an EXPLICIT `false` opts out. An absent `$count` keeps today's - // behaviour (count runs, `total` is reported) rather than taking OData's - // "absent means omit" reading: every existing caller sends nothing and - // reads `total`, so the OData default would silently break all of them. - // The parameter is therefore an opt-OUT here, and that asymmetry is - // deliberate — see the changeset for the wording that ships to consumers. - const countOptOut = options.count === false; - for (const k of ['object', 'count', 'joins', 'windowFunctions', 'cursor', 'distinct', 'having']) { - delete options[k]; - } - // [#21086] The generic data door is a read exit for the stored metadata - // body too: a `sys_metadata` / `sys_metadata_history` row's `metadata` - // column is served as its type's read projection — the one `/meta` - // serves — so a stored credential is withheld here exactly as there. - // The row's `type` selects the redactor, so it is read even when the - // projection named only the body, and taken back off before serving. - const bodyProjection = storedMetadataBodyProjection(request.object, options.fields); - if (bodyProjection.addedType) options.fields = bodyProjection.fields; - // A list read with no `limit` takes the default page size, and its - // answer says whether more rows exist (`hasMore` / `total` below). A - // PROBE limit (a row budget with no page size) is the server's, not the - // caller's: the answer is refused when it overruns, and otherwise it is - // the whole set, so the probe is taken back off before the page - // metadata below reads `limit`. - const listRead = { - surface: 'list' as const, - object: request.object, - limit: typeof options.limit === 'number' ? options.limit : undefined, - offset: typeof options.offset === 'number' ? options.offset : undefined, - limitParam: slotParam('limit'), - }; - const listPlan = planQueryReadWindow(listRead, readLimits, console); - if (listPlan.limit !== undefined) options.limit = listPlan.limit; - // [#29764] The bag `engine.find` receives is typed as the engine declares - // it. `options` is the caller's normalized request bag, and by this line - // every key the normalizer does not hand on has been folded, deleted or - // moved into `where`. What can still stand is the find set below, plus - // `groupBy` / `aggregations` holding null at most (a non-empty one took - // the aggregate branch above, an empty one was refused). The engine reads - // a null option as absent, so leaving those two out changes nothing. Keys - // are copied only when present, so the engine sees the same bag as before. - const findQuery: EngineQueryOptions = { - ...(options.context !== undefined ? { context: options.context } : {}), - ...(options.where !== undefined ? { where: options.where } : {}), - ...(options.fields !== undefined ? { fields: options.fields } : {}), - ...(options.orderBy !== undefined ? { orderBy: options.orderBy } : {}), - ...(options.limit !== undefined ? { limit: options.limit } : {}), - ...(options.offset !== undefined ? { offset: options.offset } : {}), - ...(options.search !== undefined ? { search: options.search } : {}), - ...(options.searchFields !== undefined ? { searchFields: options.searchFields } : {}), - ...(options.expand !== undefined ? { expand: options.expand } : {}), - }; - // [#21207] …and its stored CONTENT HASH (`checksum`, `previous_checksum`) - // is served in keyed form — never the stored value, which beside the - // projected body confirms a guess at the withheld material offline — - // under the provider's key, or the process-scoped ephemeral one. - const found = await this.engine.find(request.object, findQuery); - if (listPlan.probe) { - assertQueryReadRows(listRead, listPlan, found.length, console); - delete options.limit; - } - const records = await serveStoredMetadataHashColumnRows( - request.object, - redactStoredMetadataRows( - request.object, - found, - { dropType: bodyProjection.addedType }, - ), - this.storedHashDigest(), - ); - // Pagination metadata. When a `limit` is present the response is a single - // page, so `records.length` is the page size — NOT the match total. Run a - // count over the same `where` so the client can render total pages and know - // whether more pages remain (true server-side pagination). Without a limit - // AND without an offset the full result set is returned, so its length - // already IS the total. - // - // [#28547] An offset with no limit is a paged request too. The read - // returns every match AFTER the offset, so `records.length` is the - // match count MINUS the offset: `?$skip=3&$count=true` over 14 rows - // answered `total: 11` while `?$skip=3&$top=100&$count=true` answered - // 14, and an offset past the end answered `total: 0`. Adding the offset - // back is not the fix (it says 999 for `offset: 999` over 14 rows), so - // such a request takes the one count path below, `$count=false` arm - // included: `total` is the filtered count whatever the offset. - // - // [#22790] The count takes the query's `search` / `searchFields` too: the - // engine expands them through the one ADR-0061 expander `find` uses, so - // `total` counts exactly the rows this query pages. A search used to skip - // the count and report a page-local estimate (`offset + page + 1`) that - // the Console rendered as the record count. - // - // [#22588] `limit: 0` IS a limit. It asks for no rows (objectstack#6485, - // the drivers' pagination conformance), so `records.length` is 0 and was - // reported as the total: `?$top=0` answered `total: 0` while `?$top=1` - // answered the true count, and the Console's footer probe — which sends - // exactly `$top: 0` to read the total and nothing else — showed `0` on - // every list page. Gated `> 0`, a zero-row page fell into the "no limit, - // the full set is the total" arm, which is true of every limit except - // this one. So `>= 0`: a zero-row request is a paged request like any - // other, and the three arms below keep their meaning for it — the count - // runs (`total` is the filtered count, whatever the page size), and - // `$count=false` still omits `total` instead of reporting the page's 0. - const pageLimit = typeof options.limit === 'number' && options.limit >= 0 ? options.limit : undefined; - const pageOffset = typeof options.offset === 'number' && options.offset > 0 ? options.offset : 0; - let total: number | undefined = records.length; - let hasMore = false; - if (pageLimit !== undefined || pageOffset > 0) { - // `distinct` used to suppress the count here too — #4286 finding 2: - // the flag's ONLY observable effect platform-wide, on a capability - // that never deduplicated a row. Removed with `query.distinct` - // (tombstoned in spec 17); `total`/`hasMore` are truthful again. - if (countOptOut) { - // [#10757] The caller said it does not need `total`, so the - // COUNT query is not issued at all — that is the whole point of - // the parameter, and on a remote database it is a full round - // trip saved per list request. - // - // `total` is OMITTED rather than estimated. `FindDataResponse` - // declares it optional ("Total number of records matching the - // filter (IF REQUESTED)", - // `packages/spec/src/api/protocol.zod.ts`), so absent is the - // declared shape for "not requested" — and it is the only - // honest one: a real number was available and the caller - // declined it. Handing back a plausible-looking guess is how a - // page-local estimate ends up rendered as a record count. - // - // `hasMore` is still answered, from the page alone: a FULL page - // means there may be more. It never over-reports the data — it - // can only say "maybe more" on an exactly-full last page. - // With an offset and no limit, every row after the offset came - // back, so there is no further page. - hasMore = pageLimit !== undefined && records.length === pageLimit; - total = undefined; - } else { - // [#10757] `counted` is a separate, always-assigned local so - // `hasMore` below compares against a `number`: `total` became - // optional when `$count=false` gained the right to omit it, and - // TypeScript cannot narrow it back across the try/catch. - let counted: number; - try { - const countQuery: EngineCountOptions = { - where: options.where, - search: options.search, - searchFields: options.searchFields, - context: options.context, - }; - counted = await this.engine.count(request.object, countQuery); - } catch { - // engine.count() has its own find().length fallback; if it still - // throws, degrade to a page-local total rather than failing the list. - counted = pageOffset + records.length; - } - total = counted; - hasMore = pageOffset + records.length < counted; - } - } - return { - object: request.object, - records, - // [#10757] Omitted, not `undefined`-valued: a JSON body carrying - // `"total": null` (or a key some serializers keep) reads as "the - // total is nothing", which is a different claim from "no total was - // requested". The key is simply absent. - ...(total === undefined ? {} : { total }), - hasMore, - }; + // [#29985 codemod] The body moved to `protocol-data-crud.ts`; it reaches this class through `protocol-host.ts`. + return ProtocolDataCrudFacet.findData(protocolHost(this), request); } async getData(request: { object: string, id: string, expand?: string | string[], select?: string | string[], context?: any }): Promise<{ diff --git a/packages/spec/liveness/query.json b/packages/spec/liveness/query.json index 0fa648d8519..339b71fab0c 100644 --- a/packages/spec/liveness/query.json +++ b/packages/spec/liveness/query.json @@ -109,7 +109,7 @@ "having": { "status": "live", "verifiedAt": "2026-08-28", - "evidence": "packages/objectql/src/having-filter.ts#applyHaving (the evaluator entry point) and packages/objectql/src/having-filter.ts#matchesHaving (the per-row predicate over the AGGREGATED row's namespace — aliases plus groupBy projections; an unknown operator throws rather than being ignored); packages/objectql/src/engine.ts#aggregate (applies it after BOTH the native-driver path and the in-memory fallback); packages/metadata-protocol/src/protocol.ts#findData (the aggregate branch forwards it, and strips it off the find-path bag because that path cannot serve it); packages/spec/src/data/data-engine.zod.ts#EngineAggregateOptionsSchema (the declaration leg — this one is the option's DECLARATION, not a consumer, and is cited as such); packages/plugins/plugin-security/src/predicate-guard.ts#collectQueryFields (`collectConditionFields(ast.having, out)` — the FLS guard walks it beside `where`)", + "evidence": "packages/objectql/src/having-filter.ts#applyHaving (the evaluator entry point) and packages/objectql/src/having-filter.ts#matchesHaving (the per-row predicate over the AGGREGATED row's namespace — aliases plus groupBy projections; an unknown operator throws rather than being ignored); packages/objectql/src/engine.ts#aggregate (applies it after BOTH the native-driver path and the in-memory fallback); packages/metadata-protocol/src/protocol-data-crud.ts#findData (the aggregate branch forwards it, and strips it off the find-path bag because that path cannot serve it); packages/spec/src/data/data-engine.zod.ts#EngineAggregateOptionsSchema (the declaration leg — this one is the option's DECLARATION, not a consumer, and is cited as such); packages/plugins/plugin-security/src/predicate-guard.ts#collectQueryFields (`collectConditionFields(ast.having, out)` — the FLS guard walks it beside `where`)", "note": "ENFORCED 2026-07-31 (#4286 step 3, resolved to enforce): engine-owned, applied AFTER aggregation over the aggregated row's namespace (aggregation aliases + groupBy projections); an unknown operator rejects loudly instead of being ignored. Was #4286 finding 1 — aggregate() rebuilt the driver AST without it, so even a HAVING-implementing driver could never receive it. Native SQL pushdown can come later behind a driver capability flag (the dateGranularity two-tier pattern) without changing semantics. The FLS predicate guard walks it. 2026-08-28: RE-ANCHORED (commit 8cb96ec41) — all four citations were PATH-ONLY. This is the entry where that mattered most: `having`'s whole history is a key that was declared, forwarded and then dropped one layer before it could act, and four unbounded file pointers are exactly the evidence shape that could not have caught it. Re-closed by hand against 93ea19bca." }, "windowFunctions": { diff --git a/scripts/facade-delegation-registry.json b/scripts/facade-delegation-registry.json index 951232f4588..50f114e6742 100644 --- a/scripts/facade-delegation-registry.json +++ b/scripts/facade-delegation-registry.json @@ -30,14 +30,8 @@ { "file": "packages/metadata-protocol/src/protocol.ts", "member": "getDiscovery", - "card": "#30518", - "reason": "return type spelled on the signature (#30054 step 1, public .d.ts byte-equal); movable now by `codemod-split-protocol.mjs --members` (#30518), whose second PR moves it: the eight doors were split over two PRs to keep each under the 3,000-line budget" - }, - { - "file": "packages/metadata-protocol/src/protocol.ts", - "member": "findData", - "card": "#30518", - "reason": "return type spelled on the signature (#30054 step 1, public .d.ts byte-equal); movable now by `codemod-split-protocol.mjs --members` (#30518), whose second PR moves it: the eight doors were split over two PRs to keep each under the 3,000-line budget, and findData's path reader (protocol.query-transport-dialect.test.ts) gains the facet in that PR" + "card": "#30147", + "reason": "codemod limit: closed-set, reads the instance method `searchAll` as a value (`typeof this.searchAll === 'function'` sets `capabilities.search`), and a host entry forwards a method through a closure that is always a function, so the moved probe would always answer true (#30518 PR2, measured by rest discovery-search-capability-agreement.test.ts)" }, { "file": "packages/metadata-protocol/src/protocol.ts",