Skip to content

fix(spec,data): the five RPC query aliases resolve by one fold — spec table, not per-reader prose (#3795) #4323

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
os-zhuang merged 2 commits into from
Jul 31, 2026
Merged

fix(spec,data): the five RPC query aliases resolve by one fold — spec table, not per-reader prose (#3795) #4323

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ca0637e
fix(spec,data): the five RPC query aliases resolve by one fold — spec…
os-zhuang Jul 31, 2026
aa1c59c
Merge remote-tracking branch 'origin/main' into claude/protocol-rpc-a…
os-zhuang Jul 31, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 55 additions & 0 deletions .changeset/rpc-alias-precedence-one-fold.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
---
"@objectstack/spec": minor
"@objectstack/metadata-protocol": patch
"@objectstack/runtime": patch
---

fix(spec,data): the five RPC query aliases resolve by ONE fold — spec table, not per-reader prose (#3795)

`RpcQueryOptionsSchema` accepts five legacy aliases next to their canonical
QueryAST keys and stated the precedence in prose only ("the normalizer uses
the new key"). With no fold in the schema, every reader re-implemented it —
the #3713 condition — and the two readers disagreed:

| pair | spec prose | runtime dispatcher | metadata-protocol |
|---|---|---|---|
| `where` > `filter` | canonical | canonical | **alias consulted first** |
| `fields` > `select` | canonical | canonical | **alias clobbered canonical** |
| `offset` > `skip` | canonical | canonical | **alias clobbered canonical** |
| `expand` > `populate` | canonical | — | **alias consulted first** |
| `orderBy` > `sort` | canonical | canonical | canonical |

Four of five inverted in `protocol.ts`, so `?select=a&fields=b` answered
`[a]` on one path and `[b]` on the other — reachable from a plain HTTP
request.

**The mapping now lives once, in the spec** (`RPC_QUERY_ALIAS_SLOTS` +
`foldQueryAliasSlots`, both exported), under the rule #4181 already
established for the filter pair:

- an **alias alone** folds into its canonical key — `filter`→`where`,
`select`→`fields`, `sort`→`orderBy`, `skip`→`offset`, `populate`→`expand` —
and the alias key is **dropped from the parsed output**;
- **both spellings, same value**: redundant, tolerated, alias dropped;
- **both spellings, different values**: irreconcilable — picking a winner IS
the silent drop — so the parse fails (schema) / the request is `400
INVALID_REQUEST` (wire), naming the spellings and the canonical key;
- an explicit **`null` spelling is a withdrawal**, never a conflict: a null
alias is dropped silently, a null canonical keeps its slot-specific answer.

`RpcQueryOptionsSchema` and the four `filter`-mixin option schemas
(update/delete/count/aggregate requests) apply the fold as a parse transform,
so parsed output speaks canonical keys only — a TS consumer reading
`parsed.query.populate` now **fails to compile** instead of silently reading
`undefined` (the #3742 / #3764 shape, one layer down; hence the minor). The
protocol normalizer folds raw wire input by the same table (extended with the
wire-only `filters` / `$filter` / `$expand` spellings), and the runtime
dispatcher's second copy of the fold is deleted outright.

**Authoring/callers unchanged for the supported cases**: every alias alone
keeps working on every path, and identical duplicates still pass. What
changes is mixed vocabularies with **different** values — previously answered
differently per route, now refused loudly on all of them — and a direct
`expand: [names]` array on `POST /data/:object/query`, which used to be read
by its indices ("Unknown field '0'") and now lowers to the expand record like
`populate` always did.
8 changes: 4 additions & 4 deletions content/docs/references/data/data-engine.mdx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ Reference: any
| :--- | :--- | :--- | :--- |
| **method** | `'findOne'` | ✅ | |
| **object** | `string` | ✅ | |
| **query** | `{ context?: object; where?: Record<string, any> \| any; fields?: string[]; orderBy?: { field: string; order: Enum<'asc' \| 'desc'> }[]; … }` | optional | |
| **query** | `{ context?: object; where?: Record<string, any> \| any; fields?: string[]; orderBy?: { field: string; order?: Enum<'asc' \| 'desc'> }[]; … }` | optional | |


---
Expand All @@ -199,7 +199,7 @@ Reference: any
| :--- | :--- | :--- | :--- |
| **method** | `'find'` | ✅ | |
| **object** | `string` | ✅ | |
| **query** | `{ context?: object; where?: Record<string, any> \| any; fields?: string[]; orderBy?: { field: string; order: Enum<'asc' \| 'desc'> }[]; … }` | optional | |
| **query** | `{ context?: object; where?: Record<string, any> \| any; fields?: string[]; orderBy?: { field: string; order?: Enum<'asc' \| 'desc'> }[]; … }` | optional | |


---
Expand Down Expand Up @@ -268,7 +268,7 @@ This schema accepts one of the following structures:
| :--- | :--- | :--- | :--- |
| **method** | `'find'` | ✅ | |
| **object** | `string` | ✅ | |
| **query** | `{ context?: object; where?: Record<string, any> \| any; fields?: string[]; orderBy?: { field: string; order: Enum<'asc' \| 'desc'> }[]; … }` | optional | |
| **query** | `{ context?: object; where?: Record<string, any> \| any; fields?: string[]; orderBy?: { field: string; order?: Enum<'asc' \| 'desc'> }[]; … }` | optional | |

---

Expand All @@ -280,7 +280,7 @@ This schema accepts one of the following structures:
| :--- | :--- | :--- | :--- |
| **method** | `'findOne'` | ✅ | |
| **object** | `string` | ✅ | |
| **query** | `{ context?: object; where?: Record<string, any> \| any; fields?: string[]; orderBy?: { field: string; order: Enum<'asc' \| 'desc'> }[]; … }` | optional | |
| **query** | `{ context?: object; where?: Record<string, any> \| any; fields?: string[]; orderBy?: { field: string; order?: Enum<'asc' \| 'desc'> }[]; … }` | optional | |

---

Expand Down
176 changes: 94 additions & 82 deletions packages/metadata-protocol/src/protocol.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import type {
} from '@objectstack/spec/api';
import type { MetadataCacheRequest, MetadataCacheResponse, ServiceInfo, ApiRoutes, WellKnownCapabilities } from '@objectstack/spec/api';
import { readServiceSelfInfo } from '@objectstack/spec/api';
import { parseFilterAST, isFilterAST, VALID_AST_OPERATORS, REFERENCE_VALUE_TYPES, type DroppedFieldsEvent, type QueryAST } from '@objectstack/spec/data';
import { parseFilterAST, isFilterAST, VALID_AST_OPERATORS, REFERENCE_VALUE_TYPES, RPC_QUERY_ALIAS_SLOTS, foldQueryAliasSlots, type QueryAliasConflict, type QueryAliasSlot, type DroppedFieldsEvent, type QueryAST } from '@objectstack/spec/data';
import { PLURAL_TO_SINGULAR, SINGULAR_TO_PLURAL } from '@objectstack/spec/shared';
import { applyConversionsToStoredItem } from '@objectstack/spec';
import { type FormView, isAggregatedViewContainer } from '@objectstack/spec/ui';
Expand Down Expand Up @@ -866,6 +866,49 @@ const ODATA_SPELLING: Readonly<Record<string, string>> = {
filter: '$filter', select: '$select', expand: '$expand',
};

/**
* [#3795] The spec's alias table ({@link RPC_QUERY_ALIAS_SLOTS}) extended with
* the wire-only spellings no schema declares: `filters` (documented plural
* alias of the `filter` transport param) and the OData `$filter` / `$expand`.
* Every spelling of one QueryAST slot resolves through ONE fold — the four
* slots that used to resolve backwards (canonical consulted last), each in its
* own open-coded way, are the reason the table lives in the spec and not here.
*/
const WIRE_QUERY_ALIAS_SLOTS: readonly QueryAliasSlot[] = (() => {
const extra: Record<string, readonly string[]> = {
where: ['filters', '$filter'],
expand: ['$expand'],
};
return RPC_QUERY_ALIAS_SLOTS.map((slot) => ({
canonical: slot.canonical,
aliases: [...slot.aliases, ...(extra[slot.canonical] ?? [])],
}));
})();

/**
* [#4181 → #3795] Spellings of ONE slot carrying DIFFERENT values. Two values
* for one slot cannot be reconciled — merging them would invent an intent the
* caller never expressed, and picking one is the silent drop itself — so an
* ambiguous request is refused. Redundant identical spellings pass. #4181
* established this on the filter slot; the fold now applies it to all five.
*
* `spellingFor` maps each folded name back to the wire spelling the caller
* actually wrote (`$orderby`, not `orderBy`) — the #4226 discipline.
*/
function conflictingQueryParamsError(
conflict: QueryAliasConflict,
spellingFor: (name: string) => string,
): Error {
const names = conflict.spellings.map((s) => `'${spellingFor(s)}'`).join(', ');
const err: any = new Error(
`Conflicting query parameters: ${names} are spellings of the same parameter `
+ `(canonical '${conflict.canonical}') and were given different values. Send exactly one.`,
);
err.status = 400;
err.code = 'INVALID_REQUEST';
return err;
}

/**
* [#4181] A filter the normalizer cannot turn into a usable `FilterCondition`
* by any route other than the array shapes {@link malformedFilterArrayError}
Expand Down Expand Up @@ -3589,35 +3632,39 @@ export class ObjectStackProtocolImplementation implements
delete options[dollar];
}

// Numeric fields — normalize top → limit, skip → offset
// [#3795] One slot, one value. Every alias spelling of the five
// 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 five — four of which 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).
//
// `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, WIRE_QUERY_ALIAS_SLOTS, (conflict) => {
throw conflictingQueryParamsError(conflict, spellingFor);
});
const slotParam = (canonical: string): string => spellingFor(arrivedAs[canonical] ?? canonical);

// Numeric fields — normalize top → limit ($top is the OData layer,
// outside the #3795 slot table), then coerce querystring strings.
if (options.top != null) {
options.limit = Number(options.top);
delete options.top;
}
if (options.skip != null) {
options.offset = Number(options.skip);
}
// Deleted unconditionally, unlike `top` (a declared QueryAST key the
// engine aliases itself): `skip` is wire-only, so a null/undefined one
// left behind would reach the #4134 field gate below and be reported as
// an unknown FIELD — a confusing rejection for a real parameter.
delete options.skip;
if (options.limit != null) options.limit = Number(options.limit);
if (options.offset != null) options.offset = Number(options.offset);

// Select → fields: comma-separated string → array
const projectionKey = options.select !== undefined ? (wireSpelling.select ?? 'select') : 'fields';
if (typeof options.select === 'string') {
options.fields = options.select.split(',').map((s: string) => s.trim()).filter(Boolean);
} else if (Array.isArray(options.select)) {
options.fields = options.select;
}
if (options.select !== undefined) delete options.select;

// fields: comma-separated string → array. Clients may pass `?fields=name`
// directly (not only via the `?select=` alias above) — a single-value
// querystring param arrives as a bare string, which drivers' `.map()`
// 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.
const projectionKey = slotParam('fields');
if (typeof options.fields === 'string') {
options.fields = options.fields.split(',').map((s: string) => s.trim()).filter(Boolean);
} else if (options.fields !== undefined && !Array.isArray(options.fields)) {
Expand All @@ -3628,18 +3675,16 @@ export class ObjectStackProtocolImplementation implements
// returned MORE than was asked for.
this.assertProjectionFieldsExist(request.object, options.fields, projectionKey);

// Sort/orderBy → orderBy: every wire spelling → SortNode[].
// 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 usesOrderBy = options.orderBy !== undefined && options.orderBy !== null;
const sortValue = usesOrderBy ? options.orderBy : options.sort;
const sortKey = usesOrderBy ? (wireSpelling.orderBy ?? 'orderBy') : 'sort';
delete options.sort;
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.
Expand All @@ -3656,41 +3701,15 @@ export class ObjectStackProtocolImplementation implements
else delete options.orderBy;
}

// Filter/filters/$filter → where: normalize all filter aliases.
//
// [#4181] These four names are FOUR SPELLINGS OF ONE SLOT (`filters` is
// documented as a deprecated alias of `filter`), so `??` picking the
// first non-null silently discarded the others: a body carrying both
// `where` and a different `filter` ran the `filter` and dropped the
// `where` with no signal. Two different values for one slot cannot be
// reconciled — merging them would invent an intent the caller never
// expressed, and picking one is the silent drop itself — so an
// ambiguous request is refused. Redundant identical spellings are
// harmless and pass.
const filterAliases = (['filter', 'filters', '$filter', 'where'] as const)
.filter((k) => options[k] !== undefined)
.map((k) => ({ key: k, value: options[k] }));
if (filterAliases.length > 1) {
const distinct = new Set(filterAliases.map((a) => JSON.stringify(a.value)));
if (distinct.size > 1) {
const err: any = new Error(
`Conflicting filter parameters: ${filterAliases.map((a) => `'${a.key}'`).join(', ')} `
+ 'are aliases for the same filter and were given different values. Send exactly one.',
);
err.status = 400;
err.code = 'INVALID_REQUEST';
throw err;
}
}

const filterValue = options.filter ?? options.filters ?? options.$filter ?? options.where;
const filterKey = filterAliases[0]?.key ?? 'filter';
delete options.filter;
delete options.filters;
delete options.$filter;
// 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 (filterValue !== undefined) {
let parsedFilter = filterValue;
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
Expand Down Expand Up @@ -3755,31 +3774,24 @@ export class ObjectStackProtocolImplementation implements
}
}

// Populate/expand/$expand → expand (Record<string, QueryAST>)
const populateValue = options.populate;
const expandValue = options.$expand ?? options.expand;
// Expand: the folded slot value → `Record<string, QueryAST>`. 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 populateValue === 'string') {
expandNames.push(...populateValue.split(',').map((s: string) => s.trim()).filter(Boolean));
} else if (Array.isArray(populateValue)) {
expandNames.push(...populateValue);
}
if (!expandNames.length && expandValue) {
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 expandValue === 'string') {
expandNames.push(...expandValue.split(',').map((s: string) => s.trim()).filter(Boolean));
} else if (Array.isArray(expandValue)) {
expandNames.push(...expandValue);
}
delete options.populate;
delete options.$expand;
// Clean up non-object expand (e.g. string) BEFORE the Record conversion
// below, so that populate-derived names can create the expand Record even
// when a legacy string expand was also present.
if (typeof options.expand !== 'object' || options.expand === null) {
if (typeof options.expand !== 'object' || options.expand === null || Array.isArray(options.expand)) {
delete options.expand;
}
// Only set expand if not already an object (advanced usage)
if (expandNames.length > 0 && !options.expand) {
options.expand = {} as Record<string, any>;
for (const rel of expandNames) {
Expand Down
Loading
Loading