Skip to content

Commit ca0637e

Browse files
os-zhuangclaude
andcommitted
fix(spec,data): the five RPC query aliases resolve by one fold — spec table, not per-reader prose (#3795)
RpcQueryOptionsSchema documented its alias precedence in prose only, so both normalizers re-implemented it: protocol.ts had four of five pairs BACKWARDS (canonical consulted last) and disagreed with the runtime dispatcher's copy on three, so ?select=a&fields=b answered [a] or [b] depending on the path. The mapping now lives once in the spec (RPC_QUERY_ALIAS_SLOTS + foldQueryAliasSlots), applied as a parse transform on the RPC schemas and as the wire fold in findData, under #4181's rule generalized to all five slots: alias alone folds and is dropped from parsed output, identical duplicates collapse, different values are refused, an explicit null spelling is a withdrawal. The dispatcher's second copy of the fold is deleted. Also lowers a direct expand:[names] array to the expand record instead of reading its indices as relation names. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent 2e836de commit ca0637e

9 files changed

Lines changed: 604 additions & 165 deletions

File tree

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
---
2+
"@objectstack/spec": minor
3+
"@objectstack/metadata-protocol": patch
4+
"@objectstack/runtime": patch
5+
---
6+
7+
fix(spec,data): the five RPC query aliases resolve by ONE fold — spec table, not per-reader prose (#3795)
8+
9+
`RpcQueryOptionsSchema` accepts five legacy aliases next to their canonical
10+
QueryAST keys and stated the precedence in prose only ("the normalizer uses
11+
the new key"). With no fold in the schema, every reader re-implemented it —
12+
the #3713 condition — and the two readers disagreed:
13+
14+
| pair | spec prose | runtime dispatcher | metadata-protocol |
15+
|---|---|---|---|
16+
| `where` > `filter` | canonical | canonical | **alias consulted first** |
17+
| `fields` > `select` | canonical | canonical | **alias clobbered canonical** |
18+
| `offset` > `skip` | canonical | canonical | **alias clobbered canonical** |
19+
| `expand` > `populate` | canonical || **alias consulted first** |
20+
| `orderBy` > `sort` | canonical | canonical | canonical |
21+
22+
Four of five inverted in `protocol.ts`, so `?select=a&fields=b` answered
23+
`[a]` on one path and `[b]` on the other — reachable from a plain HTTP
24+
request.
25+
26+
**The mapping now lives once, in the spec** (`RPC_QUERY_ALIAS_SLOTS` +
27+
`foldQueryAliasSlots`, both exported), under the rule #4181 already
28+
established for the filter pair:
29+
30+
- an **alias alone** folds into its canonical key — `filter``where`,
31+
`select``fields`, `sort``orderBy`, `skip``offset`, `populate``expand`
32+
and the alias key is **dropped from the parsed output**;
33+
- **both spellings, same value**: redundant, tolerated, alias dropped;
34+
- **both spellings, different values**: irreconcilable — picking a winner IS
35+
the silent drop — so the parse fails (schema) / the request is `400
36+
INVALID_REQUEST` (wire), naming the spellings and the canonical key;
37+
- an explicit **`null` spelling is a withdrawal**, never a conflict: a null
38+
alias is dropped silently, a null canonical keeps its slot-specific answer.
39+
40+
`RpcQueryOptionsSchema` and the four `filter`-mixin option schemas
41+
(update/delete/count/aggregate requests) apply the fold as a parse transform,
42+
so parsed output speaks canonical keys only — a TS consumer reading
43+
`parsed.query.populate` now **fails to compile** instead of silently reading
44+
`undefined` (the #3742 / #3764 shape, one layer down; hence the minor). The
45+
protocol normalizer folds raw wire input by the same table (extended with the
46+
wire-only `filters` / `$filter` / `$expand` spellings), and the runtime
47+
dispatcher's second copy of the fold is deleted outright.
48+
49+
**Authoring/callers unchanged for the supported cases**: every alias alone
50+
keeps working on every path, and identical duplicates still pass. What
51+
changes is mixed vocabularies with **different** values — previously answered
52+
differently per route, now refused loudly on all of them — and a direct
53+
`expand: [names]` array on `POST /data/:object/query`, which used to be read
54+
by its indices ("Unknown field '0'") and now lowers to the expand record like
55+
`populate` always did.

content/docs/references/data/data-engine.mdx

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -186,7 +186,7 @@ Reference: any
186186
| :--- | :--- | :--- | :--- |
187187
| **method** | `'findOne'` || |
188188
| **object** | `string` || |
189-
| **query** | `{ context?: object; where?: Record<string, any> \| any; fields?: string[]; orderBy?: { field: string; order: Enum<'asc' \| 'desc'> }[]; … }` | optional | |
189+
| **query** | `{ context?: object; where?: Record<string, any> \| any; fields?: string[]; orderBy?: { field: string; order?: Enum<'asc' \| 'desc'> }[]; … }` | optional | |
190190

191191

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

204204

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

273273
---
274274

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

285285
---
286286

packages/metadata-protocol/src/protocol.ts

Lines changed: 94 additions & 82 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ import type {
1818
} from '@objectstack/spec/api';
1919
import type { MetadataCacheRequest, MetadataCacheResponse, ServiceInfo, ApiRoutes, WellKnownCapabilities } from '@objectstack/spec/api';
2020
import { readServiceSelfInfo } from '@objectstack/spec/api';
21-
import { parseFilterAST, isFilterAST, VALID_AST_OPERATORS, REFERENCE_VALUE_TYPES, type DroppedFieldsEvent, type QueryAST } from '@objectstack/spec/data';
21+
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';
2222
import { PLURAL_TO_SINGULAR, SINGULAR_TO_PLURAL } from '@objectstack/spec/shared';
2323
import { type FormView, isAggregatedViewContainer } from '@objectstack/spec/ui';
2424
import { METADATA_FORM_REGISTRY, CORE_SERVICE_PROVIDER, serviceUnavailableMessage } from '@objectstack/spec/system';
@@ -865,6 +865,49 @@ const ODATA_SPELLING: Readonly<Record<string, string>> = {
865865
filter: '$filter', select: '$select', expand: '$expand',
866866
};
867867

868+
/**
869+
* [#3795] The spec's alias table ({@link RPC_QUERY_ALIAS_SLOTS}) extended with
870+
* the wire-only spellings no schema declares: `filters` (documented plural
871+
* alias of the `filter` transport param) and the OData `$filter` / `$expand`.
872+
* Every spelling of one QueryAST slot resolves through ONE fold — the four
873+
* slots that used to resolve backwards (canonical consulted last), each in its
874+
* own open-coded way, are the reason the table lives in the spec and not here.
875+
*/
876+
const WIRE_QUERY_ALIAS_SLOTS: readonly QueryAliasSlot[] = (() => {
877+
const extra: Record<string, readonly string[]> = {
878+
where: ['filters', '$filter'],
879+
expand: ['$expand'],
880+
};
881+
return RPC_QUERY_ALIAS_SLOTS.map((slot) => ({
882+
canonical: slot.canonical,
883+
aliases: [...slot.aliases, ...(extra[slot.canonical] ?? [])],
884+
}));
885+
})();
886+
887+
/**
888+
* [#4181 → #3795] Spellings of ONE slot carrying DIFFERENT values. Two values
889+
* for one slot cannot be reconciled — merging them would invent an intent the
890+
* caller never expressed, and picking one is the silent drop itself — so an
891+
* ambiguous request is refused. Redundant identical spellings pass. #4181
892+
* established this on the filter slot; the fold now applies it to all five.
893+
*
894+
* `spellingFor` maps each folded name back to the wire spelling the caller
895+
* actually wrote (`$orderby`, not `orderBy`) — the #4226 discipline.
896+
*/
897+
function conflictingQueryParamsError(
898+
conflict: QueryAliasConflict,
899+
spellingFor: (name: string) => string,
900+
): Error {
901+
const names = conflict.spellings.map((s) => `'${spellingFor(s)}'`).join(', ');
902+
const err: any = new Error(
903+
`Conflicting query parameters: ${names} are spellings of the same parameter `
904+
+ `(canonical '${conflict.canonical}') and were given different values. Send exactly one.`,
905+
);
906+
err.status = 400;
907+
err.code = 'INVALID_REQUEST';
908+
return err;
909+
}
910+
868911
/**
869912
* [#4181] A filter the normalizer cannot turn into a usable `FilterCondition`
870913
* by any route other than the array shapes {@link malformedFilterArrayError}
@@ -3490,35 +3533,39 @@ export class ObjectStackProtocolImplementation implements
34903533
delete options[dollar];
34913534
}
34923535

3493-
// Numeric fields — normalize top → limit, skip → offset
3536+
// [#3795] One slot, one value. Every alias spelling of the five
3537+
// QueryAST slots resolves HERE, by the spec's own table, before the
3538+
// per-slot wire coercion below ever runs — so that coercion reads
3539+
// canonical keys only. An alias alone folds into its canonical key;
3540+
// redundant identical spellings collapse; different values for one
3541+
// slot are refused (the #4181 rule, generalized from the filter slot
3542+
// to all five — four of which used to resolve BACKWARDS here, each in
3543+
// its own way, disagreeing with the spec's documented precedence and
3544+
// with the runtime dispatcher's copy of the same fold).
3545+
//
3546+
// `arrivedAs` remembers which spelling carried each slot's value;
3547+
// composed with `wireSpelling` it names the parameter the caller
3548+
// actually wrote in every rejection below (#4226).
3549+
const spellingFor = (name: string): string => wireSpelling[name] ?? name;
3550+
const arrivedAs = foldQueryAliasSlots(options, WIRE_QUERY_ALIAS_SLOTS, (conflict) => {
3551+
throw conflictingQueryParamsError(conflict, spellingFor);
3552+
});
3553+
const slotParam = (canonical: string): string => spellingFor(arrivedAs[canonical] ?? canonical);
3554+
3555+
// Numeric fields — normalize top → limit ($top is the OData layer,
3556+
// outside the #3795 slot table), then coerce querystring strings.
34943557
if (options.top != null) {
34953558
options.limit = Number(options.top);
34963559
delete options.top;
34973560
}
3498-
if (options.skip != null) {
3499-
options.offset = Number(options.skip);
3500-
}
3501-
// Deleted unconditionally, unlike `top` (a declared QueryAST key the
3502-
// engine aliases itself): `skip` is wire-only, so a null/undefined one
3503-
// left behind would reach the #4134 field gate below and be reported as
3504-
// an unknown FIELD — a confusing rejection for a real parameter.
3505-
delete options.skip;
35063561
if (options.limit != null) options.limit = Number(options.limit);
35073562
if (options.offset != null) options.offset = Number(options.offset);
35083563

3509-
// Select → fields: comma-separated string → array
3510-
const projectionKey = options.select !== undefined ? (wireSpelling.select ?? 'select') : 'fields';
3511-
if (typeof options.select === 'string') {
3512-
options.fields = options.select.split(',').map((s: string) => s.trim()).filter(Boolean);
3513-
} else if (Array.isArray(options.select)) {
3514-
options.fields = options.select;
3515-
}
3516-
if (options.select !== undefined) delete options.select;
3517-
3518-
// fields: comma-separated string → array. Clients may pass `?fields=name`
3519-
// directly (not only via the `?select=` alias above) — a single-value
3520-
// querystring param arrives as a bare string, which drivers' `.map()`
3564+
// Projection: comma-separated string → array. A single-value
3565+
// querystring param arrives as a bare string — `?fields=name` or the
3566+
// folded `?select=` / `$select` spellings — which drivers' `.map()`
35213567
// calls over `query.fields` would otherwise throw on.
3568+
const projectionKey = slotParam('fields');
35223569
if (typeof options.fields === 'string') {
35233570
options.fields = options.fields.split(',').map((s: string) => s.trim()).filter(Boolean);
35243571
} else if (options.fields !== undefined && !Array.isArray(options.fields)) {
@@ -3529,18 +3576,16 @@ export class ObjectStackProtocolImplementation implements
35293576
// returned MORE than was asked for.
35303577
this.assertProjectionFieldsExist(request.object, options.fields, projectionKey);
35313578

3532-
// Sort/orderBy → orderBy: every wire spelling → SortNode[].
3579+
// Sort: every wire shape → SortNode[].
35333580
//
35343581
// [#4226] `normalizeSortNodes` folds the two shapes that used to fall
35353582
// through this block untouched — `string[]` and `{field: direction}` —
35363583
// and refuses the ones it cannot read. Before it, "not a string and not
35373584
// an array" simply skipped the branch, leaving a value on `orderBy`
35383585
// that `SqlDriver`'s `Array.isArray` guard then declined to turn into
35393586
// an ORDER BY clause: no sort, no error, no way to tell.
3540-
const usesOrderBy = options.orderBy !== undefined && options.orderBy !== null;
3541-
const sortValue = usesOrderBy ? options.orderBy : options.sort;
3542-
const sortKey = usesOrderBy ? (wireSpelling.orderBy ?? 'orderBy') : 'sort';
3543-
delete options.sort;
3587+
const sortValue = options.orderBy;
3588+
const sortKey = slotParam('orderBy');
35443589
if (sortValue === undefined || sortValue === null) {
35453590
// Nothing to sort by — and an explicit `orderBy: null` must not ride
35463591
// to the engine as a value every driver quietly declines to read.
@@ -3557,41 +3602,15 @@ export class ObjectStackProtocolImplementation implements
35573602
else delete options.orderBy;
35583603
}
35593604

3560-
// Filter/filters/$filter → where: normalize all filter aliases.
3561-
//
3562-
// [#4181] These four names are FOUR SPELLINGS OF ONE SLOT (`filters` is
3563-
// documented as a deprecated alias of `filter`), so `??` picking the
3564-
// first non-null silently discarded the others: a body carrying both
3565-
// `where` and a different `filter` ran the `filter` and dropped the
3566-
// `where` with no signal. Two different values for one slot cannot be
3567-
// reconciled — merging them would invent an intent the caller never
3568-
// expressed, and picking one is the silent drop itself — so an
3569-
// ambiguous request is refused. Redundant identical spellings are
3570-
// harmless and pass.
3571-
const filterAliases = (['filter', 'filters', '$filter', 'where'] as const)
3572-
.filter((k) => options[k] !== undefined)
3573-
.map((k) => ({ key: k, value: options[k] }));
3574-
if (filterAliases.length > 1) {
3575-
const distinct = new Set(filterAliases.map((a) => JSON.stringify(a.value)));
3576-
if (distinct.size > 1) {
3577-
const err: any = new Error(
3578-
`Conflicting filter parameters: ${filterAliases.map((a) => `'${a.key}'`).join(', ')} `
3579-
+ 'are aliases for the same filter and were given different values. Send exactly one.',
3580-
);
3581-
err.status = 400;
3582-
err.code = 'INVALID_REQUEST';
3583-
throw err;
3584-
}
3585-
}
3586-
3587-
const filterValue = options.filter ?? options.filters ?? options.$filter ?? options.where;
3588-
const filterKey = filterAliases[0]?.key ?? 'filter';
3589-
delete options.filter;
3590-
delete options.filters;
3591-
delete options.$filter;
3605+
// Filter: the folded slot value → a usable `FilterCondition` on
3606+
// `where`, or a rejection. The four spellings of this slot
3607+
// (`where`/`filter`/`filters`/`$filter`) already resolved through the
3608+
// #3795 fold above — #4181's one-slot-one-value rule, which this block
3609+
// pioneered before the fold generalized it.
3610+
const filterKey = slotParam('where');
35923611

3593-
if (filterValue !== undefined) {
3594-
let parsedFilter = filterValue;
3612+
if (options.where !== undefined) {
3613+
let parsedFilter = options.where;
35953614
// A blank `?filter=` is ABSENT, not malformed — the same `length > 0`
35963615
// guard the export route applies before parsing. Deleting `where`
35973616
// here (rather than leaving `''` on it) is what lets every consumer
@@ -3656,31 +3675,24 @@ export class ObjectStackProtocolImplementation implements
36563675
}
36573676
}
36583677

3659-
// Populate/expand/$expand → expand (Record<string, QueryAST>)
3660-
const populateValue = options.populate;
3661-
const expandValue = options.$expand ?? options.expand;
3678+
// Expand: the folded slot value → `Record<string, QueryAST>`. A comma
3679+
// list (string) and a name array both lower to `{name: {object: name}}`;
3680+
// the advanced `{rel: QueryAST}` map a caller may send directly on
3681+
// `POST /data/:object/query` passes through as-is. Lowering the ARRAY
3682+
// shape here (not just the string) also closes a pre-#3795 gap: a raw
3683+
// name array used to survive this block whole, so the #4226 gate read
3684+
// its INDICES as relation names and refused real requests with
3685+
// "Unknown field '0'".
3686+
const expandValue = options.expand;
36623687
const expandNames: string[] = [];
3663-
if (typeof populateValue === 'string') {
3664-
expandNames.push(...populateValue.split(',').map((s: string) => s.trim()).filter(Boolean));
3665-
} else if (Array.isArray(populateValue)) {
3666-
expandNames.push(...populateValue);
3667-
}
3668-
if (!expandNames.length && expandValue) {
3669-
if (typeof expandValue === 'string') {
3670-
expandNames.push(...expandValue.split(',').map((s: string) => s.trim()).filter(Boolean));
3671-
} else if (Array.isArray(expandValue)) {
3672-
expandNames.push(...expandValue);
3673-
}
3688+
if (typeof expandValue === 'string') {
3689+
expandNames.push(...expandValue.split(',').map((s: string) => s.trim()).filter(Boolean));
3690+
} else if (Array.isArray(expandValue)) {
3691+
expandNames.push(...expandValue);
36743692
}
3675-
delete options.populate;
3676-
delete options.$expand;
3677-
// Clean up non-object expand (e.g. string) BEFORE the Record conversion
3678-
// below, so that populate-derived names can create the expand Record even
3679-
// when a legacy string expand was also present.
3680-
if (typeof options.expand !== 'object' || options.expand === null) {
3693+
if (typeof options.expand !== 'object' || options.expand === null || Array.isArray(options.expand)) {
36813694
delete options.expand;
36823695
}
3683-
// Only set expand if not already an object (advanced usage)
36843696
if (expandNames.length > 0 && !options.expand) {
36853697
options.expand = {} as Record<string, any>;
36863698
for (const rel of expandNames) {

0 commit comments

Comments
 (0)