Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
23 changes: 23 additions & 0 deletions .changeset/18977-orderby-dual-declaration-cross-reference.md
Original file line number Diff line number Diff line change
@@ -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.
149 changes: 149 additions & 0 deletions packages/spec/src/api/odata-orderby-dual-declaration.test.ts
Original file line number Diff line number Diff line change
@@ -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<readonly [string, unknown]> = [
["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<readonly [string, unknown]> = [
['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<string, unknown>) => ({ 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<string, unknown> }).query.$orderby).toBeUndefined();
});
});
});
50 changes: 50 additions & 0 deletions packages/spec/src/api/odata.zod.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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"
Expand Down
19 changes: 19 additions & 0 deletions packages/spec/src/data/data-engine.zod.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading