diff --git a/.changeset/18977-orderby-dual-declaration-cross-reference.md b/.changeset/18977-orderby-dual-declaration-cross-reference.md new file mode 100644 index 00000000000..128263f0386 --- /dev/null +++ b/.changeset/18977-orderby-dual-declaration-cross-reference.md @@ -0,0 +1,23 @@ +--- +"@objectstack/spec": patch +--- + +`$orderby` is declared twice — `ODataQuerySchema.$orderby` and `QueryTransportParamsSchema.$orderby` now cross-reference each other, and a pin holds the two accept sets apart (#18977). + +Clause-②: no. No accept set moves and no export is added, removed or renamed: the change is two docblocks in published source (`src/api/odata.zod.ts`, `src/data/data-engine.zod.ts`) plus a new pin test. Measured — `check:generated` reports all 16 generated artifacts up to date, `check:api-surface` and `check:authorable-surface` included. + +The two declarations are **complementary refusals**: each accepts exactly what the other rejects, and neither pointed at the other, so reading one of them carefully and completely still produced the wrong answer about the other. + +| `$orderby` value | `ODataQuerySchema` | `QueryTransportParamsSchema` (`DataEngineSortSchema`) | +|:---|:---|:---| +| `'name desc'` / `'-created_at'` | accepted | REFUSED | +| `['name desc', 'email asc']` | accepted | REFUSED | +| `[{field, order}]` | REFUSED | accepted | +| `{name: 'asc'}` / `{name: 1}` | REFUSED | accepted | + +- **Which one grades a query bag**: `QueryTransportParamsSchema`, reached from `FindDataRequestSchema.query` through `QueryWithTransportSchema` — the schema `POST /data/:object/query` parses its body against. `ODataQuerySchema` grades no runtime door: measured on this tree, its only consumers are the `OData.buildUrl` helper in its own file and its own unit test. +- **The refusal on the transport side is deliberate and stays** — `#18704` settled it: lowering an OData sort *expression* means PARSING, and a second parser beside the door's is how one rule gets two implementations that disagree. Widening either side to close the gap is a decision, not a tidy-up, so this change closes the **reader's** half only. +- **The string forms are not unserved.** `normalizeSortNodes` (`@objectstack/metadata-protocol`) reads `'name desc'`, `'-created_at'` and the `string[]` form at the shared ingress behind `GET /data/:object`, the export route and in-process `findData`. A querystring spelled the OData way works; the same bag sent as a `POST /data/:object/query` body answers `400 VALIDATION_FAILED`. The difference is the door, and neither door is `ODataQuerySchema`. +- **The cost this repairs was already paid.** objectui#9554 was filed, triaged, graded and dispatched against a shipped `object-grid` producer that had been sending the canonical shape all along, because the filing seat read the OData declaration and quoted it correctly. + +`src/api/odata-orderby-dual-declaration.test.ts` is the mechanical half: 25 cases pinning each side's accept set, their disjointness (with the lit control that neither set is empty), and which of the two `FindDataRequestSchema.query` is graded by. Widening or narrowing either declaration turns it red and lands the author on the cross-reference. diff --git a/packages/spec/src/api/odata-orderby-dual-declaration.test.ts b/packages/spec/src/api/odata-orderby-dual-declaration.test.ts new file mode 100644 index 00000000000..b9ca5b27579 --- /dev/null +++ b/packages/spec/src/api/odata-orderby-dual-declaration.test.ts @@ -0,0 +1,149 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#18977] `$orderby` is declared TWICE, and the two declarations are + * COMPLEMENTARY REFUSALS — each accepts exactly what the other rejects. + * + * | declaration | accepts | refuses | + * |:---|:---|:---| + * | `ODataQuerySchema.$orderby` (`api/odata.zod.ts`) | `string`, `string[]` | the record maps, `SortNode[]` | + * | `QueryTransportParamsSchema.$orderby` = `DataEngineSortSchema` (`data/data-engine.zod.ts`) | the record maps, `SortNode[]` | `string`, `string[]` — deliberately, #18704 | + * + * Neither file pointed at the other, so reading one of them carefully and + * completely still produced the wrong answer about the other — measured, at a + * price: objectui#9554 was filed, triaged, graded and dispatched against a + * shipped `object-grid` producer that had been sending the canonical shape all + * along, because the filing seat read the OData declaration and correctly + * quoted it. The cross-references landed in both files with this card; this + * file is their MECHANICAL half. + * + * ## What this pins, and what it deliberately does not + * + * It pins the two accept sets AS THEY ARE, and their disjointness. It is not + * an argument that either set is right: + * + * - §1/§2 hold each side's accept set, so widening or narrowing either one + * turns this red and lands the author on the cross-reference that explains + * why the gap is a decision rather than a defect. Widening the TRANSPORT + * side is the one the source argues against in its own words — lowering a + * sort expression means PARSING, and a second parser beside the door's is + * how one rule gets two implementations that disagree. + * - §3 is the disjointness itself: no value parses under both. That is the + * property a reader cannot get from either file alone, and the one that + * makes "I read the declaration" insufficient. + * - §4 says which of the two grades a query bag, through the slot the REST + * door actually parses (`FindDataRequestSchema.query`). + * + * ⛔ It pins nothing about what the RUNTIME serves. The OData string forms are + * not unserved — `normalizeSortNodes` (`@objectstack/metadata-protocol`) reads + * them at the GET querystring ingress and for in-process `findData`. What this + * file measures is the two SCHEMAS, which is where the card's trap lives. + */ + +import { describe, it, expect } from 'vitest'; +import { ODataQuerySchema } from './odata.zod'; +import { FindDataRequestSchema } from './protocol.zod'; +import { DataEngineSortSchema, QueryTransportParamsSchema } from '../data/data-engine.zod'; + +/** The two shapes `ODataQuerySchema` declares and the transport schema refuses. */ +const ODATA_SPELLINGS: ReadonlyArray = [ + ["the 'field direction' expression", 'name desc'], + ["the '-field' shorthand", '-created_at'], + ['the expression array', ['name desc', 'email asc']], + ['a single-element expression array', ['name']], +]; + +/** The three shapes `DataEngineSortSchema` declares and the OData schema refuses. */ +const TRANSPORT_SPELLINGS: ReadonlyArray = [ + ['the asc/desc record map', { name: 'desc' }], + ['the 1/-1 record map', { name: 1 }], + ['the SortNode array', [{ field: 'name', order: 'desc' }]], +]; + +describe('[#18977] $orderby is declared twice — the two accept sets', () => { + describe('§1 ODataQuerySchema.$orderby — the OData URL-convention vocabulary', () => { + it.each(ODATA_SPELLINGS)('accepts %s', (_label, value) => { + const parsed = ODataQuerySchema.safeParse({ $orderby: value }); + expect(parsed.success).toBe(true); + }); + + it.each(TRANSPORT_SPELLINGS)('refuses %s, at the $orderby member', (_label, value) => { + const parsed = ODataQuerySchema.safeParse({ $orderby: value }); + expect(parsed.success).toBe(false); + if (parsed.success) return; + expect(parsed.error.issues.map((i) => i.path.join('.'))).toContain('$orderby'); + }); + }); + + describe('§2 DataEngineSortSchema — what QueryTransportParamsSchema.$orderby is', () => { + it.each(TRANSPORT_SPELLINGS)('accepts %s', (_label, value) => { + expect(DataEngineSortSchema.safeParse(value).success).toBe(true); + expect(QueryTransportParamsSchema.safeParse({ $orderby: value }).success).toBe(true); + }); + + it.each(ODATA_SPELLINGS)('refuses %s, at the $orderby member', (_label, value) => { + expect(DataEngineSortSchema.safeParse(value).success).toBe(false); + const parsed = QueryTransportParamsSchema.safeParse({ $orderby: value }); + expect(parsed.success).toBe(false); + if (parsed.success) return; + expect(parsed.error.issues.map((i) => i.path.join('.'))).toContain('$orderby'); + }); + + it('refuses the OData spelling on `sort` too — the bare transport alias of the same slot', () => { + // A reader who works around the `$orderby` refusal by re-spelling the key + // gets the same answer: the refusal is on the VALUE, not on the spelling. + for (const [, value] of ODATA_SPELLINGS) { + expect(QueryTransportParamsSchema.safeParse({ sort: value }).success).toBe(false); + } + }); + }); + + describe('§3 the two accept sets are DISJOINT — this is the trap', () => { + it('no declared $orderby value parses under both', () => { + const every = [...ODATA_SPELLINGS, ...TRANSPORT_SPELLINGS]; + const bothAccept = every.filter(([, value]) => + ODataQuerySchema.safeParse({ $orderby: value }).success + && DataEngineSortSchema.safeParse(value).success); + expect(bothAccept.map(([label]) => label)).toEqual([]); + }); + + it('every declared $orderby value parses under exactly one of them — neither set is empty', () => { + // The lit control for the emptiness above: a disjointness assertion is + // also satisfied by two schemas that accept nothing at all. + const every = [...ODATA_SPELLINGS, ...TRANSPORT_SPELLINGS]; + const accepted = every.map(([, value]) => + Number(ODataQuerySchema.safeParse({ $orderby: value }).success) + + Number(DataEngineSortSchema.safeParse(value).success)); + expect(accepted).toEqual(every.map(() => 1)); + }); + }); + + describe('§4 which one grades a query bag', () => { + // `POST /data/:object/query` parses its body through this schema and answers + // `400 VALIDATION_FAILED` on a refusal (`rest-server.ts`), so this is the + // declaration an author's stored query bag is actually judged against. + const findInput = (query: Record) => ({ object: 't', query: { ...query, object: 't' } }); + + it.each(TRANSPORT_SPELLINGS)('FindDataRequest.query accepts %s on $orderby', (_label, value) => { + expect(FindDataRequestSchema.safeParse(findInput({ $orderby: value })).success).toBe(true); + }); + + it.each(ODATA_SPELLINGS)('FindDataRequest.query refuses %s on $orderby', (_label, value) => { + const parsed = FindDataRequestSchema.safeParse(findInput({ $orderby: value })); + expect(parsed.success).toBe(false); + if (parsed.success) return; + expect(parsed.error.issues.map((i) => i.path.join('.'))).toContain('query.$orderby'); + }); + + it('ODataQuerySchema grades nothing here — it is not on any path into FindDataRequest', () => { + // The canonical AST key is the one the output carries, whichever declared + // spelling arrived: the transport spelling folds onto `orderBy`. + const parsed = FindDataRequestSchema.safeParse(findInput({ $orderby: { created_at: 'desc' } })); + expect(parsed.success).toBe(true); + if (!parsed.success) return; + expect((parsed.data as { query: { orderBy?: unknown } }).query.orderBy) + .toEqual([{ field: 'created_at', order: 'desc' }]); + expect((parsed.data as { query: Record }).query.$orderby).toBeUndefined(); + }); + }); +}); diff --git a/packages/spec/src/api/odata.zod.ts b/packages/spec/src/api/odata.zod.ts index 30399fd71ab..b25169449f6 100644 --- a/packages/spec/src/api/odata.zod.ts +++ b/packages/spec/src/api/odata.zod.ts @@ -71,6 +71,47 @@ import { z } from 'zod'; * System query options defined by OData v4 specification. * These are URL query parameters that control the query execution. * + * ## [#18977] This schema grades no runtime door — and a SECOND declaration of + * the same parameter names does + * + * The declaration that grades a query bag is `QueryTransportParamsSchema` + * (`../data/data-engine.zod.ts`), reached from `FindDataRequestSchema.query` + * through `QueryWithTransportSchema`. This one is the OData v4 URL-convention + * VOCABULARY and nothing parses through it: measured on this tree, its only + * consumers are the `OData.buildUrl` helper at the foot of this file and this + * file's own unit test — no route, no ingress, no normalizer. + * + * ⚠️ On `$orderby` the two declarations are COMPLEMENTARY REFUSALS: each + * accepts exactly what the other rejects, so reading either one carefully and + * completely still produces the wrong answer about the other. Measured with + * `safeParse` on both, one tree: + * + * | `$orderby` value | here | `QueryTransportParamsSchema.$orderby` | + * |:---|:---|:---| + * | `'name desc'` / `'-created_at'` | accepted | REFUSED | + * | `['name desc', 'email asc']` | accepted | REFUSED | + * | `[{field, order}]` | REFUSED | accepted | + * | `{name: 'asc'}` / `{name: 1}` | REFUSED | accepted | + * + * Both halves are pinned in `odata-orderby-dual-declaration.test.ts`, which is + * the mechanical half of this cross-reference: widening or narrowing either + * side turns it red and lands the author here. + * + * ⛔ The gap is NOT closed by widening one side to match the other. #18704 + * settled which spelling is canonical and why the transport schema refuses + * these two: lowering a sort EXPRESSION means PARSING, and a second parser + * beside the door's is how one rule gets two implementations that disagree + * (the paragraph above `QueryTransportParamsSchema` states it verbatim). + * Changing either accept set is a decision, not a tidy-up. + * + * ⭐ What the string forms are not is unserved. `normalizeSortNodes` + * (`@objectstack/metadata-protocol`) is the one shared ingress normalizer + * behind `GET /data/:object`, the export route and in-process `findData`, and + * it reads `'name desc'`, `'-created_at'` and the `string[]` form — so a + * querystring spelled the OData way works, while the same bag sent as a + * `POST /data/:object/query` body answers `400 VALIDATION_FAILED`. The + * difference is the DOOR, and neither door is this schema. + * * @see https://docs.oasis-open.org/odata/odata/v4.01/odata-v4.01-part2-url-conventions.html#sec_SystemQueryOptions */ import { lazySchema } from '../shared/lazy-schema'; @@ -115,6 +156,15 @@ export const ODataQuerySchema = lazySchema(() => z.object({ * @example "name" * @example "revenue desc" * @example "country asc, revenue desc" + * + * ⛔ [#18977] These two shapes are exactly the ones + * `QueryTransportParamsSchema.$orderby` (`DataEngineSortSchema`, + * `../data/data-engine.zod.ts`) DELIBERATELY refuses, and that is the + * declaration a query bag is graded against. Sent as a + * `POST /data/:object/query` body this spelling answers + * `400 VALIDATION_FAILED`; sent on the querystring it is parsed by + * `normalizeSortNodes` at the ingress and works. See the cross-reference on + * the schema above before writing either shape into stored metadata. */ $orderby: z.union([ z.string(), // "name desc" diff --git a/packages/spec/src/data/data-engine.zod.ts b/packages/spec/src/data/data-engine.zod.ts index 1d95c4acd37..683e600b5a4 100644 --- a/packages/spec/src/data/data-engine.zod.ts +++ b/packages/spec/src/data/data-engine.zod.ts @@ -830,6 +830,25 @@ const TransportCountValueSchema = lazySchema(() => z.union([z.boolean(), z.enum( * its `string[]` form. They fail the parse at the member that carries them * rather than reaching the AST as a string the engine would have to re-read. * + * ⚠️ [#18977] `$orderby` is declared a SECOND time, and the other declaration + * accepts exactly the two shapes this one refuses: `ODataQuerySchema.$orderby` + * (`../api/odata.zod.ts`) is `string | string[]` and refuses this one's record + * maps and `SortNode[]`. It cannot contradict this schema at a DOOR — measured + * on this tree, nothing parses through it: its only consumers are its own + * `OData.buildUrl` helper and its own unit test. It contradicts it in a READER, + * which is the cost already paid: objectui#9554 was filed, triaged and + * dispatched against a shipped producer that had been sending the canonical + * shape all along, because a competent seat read the OTHER declaration, quoted + * it correctly, and had no signal that this one exists. The two accept sets are + * disjoint and pinned as such in + * `../api/odata-orderby-dual-declaration.test.ts`. + * + * ⛔ Closing that gap by widening either side is a decision, not a tidy-up — + * and widening THIS one is precisely the second parser the paragraph above + * refuses. What serves the string forms is `normalizeSortNodes` at the + * `@objectstack/metadata-protocol` ingress (the GET querystring path, the + * export route and in-process `findData`), not a schema. + * * ⛔ Declaring the narrower structured form ALONE would have turned live * traffic into a `400` — measured: the body-form AST array on * `POST /data/:object/query`, pinned by `#7390 §3` in