Skip to content

Commit be25f97

Browse files
fix(rest): strip read decorations before validating a dataset query (#4879)
Every widget on every dataset-bound dashboard failed with "Dataset query failed: 400 Bad Request — Invalid dataset definition." POST /analytics/dataset/query resolves a saved `datasetName` through `getMetaItems`, and the metadata READ path stamps its spec-validation verdict `_diagnostics` onto every document it serves. Since #4001 closed the metadata schemas, `DatasetSchema.parse()` rejects unrecognized keys rather than dropping them — so the route handed a served document back to the schema that produced it and got `unrecognized_keys: ["_diagnostics"]`. The 400 blamed the author for a key the server had just added. This is exactly what `stripReadDecorations` exists to prevent, and the failure `spec/kernel/metadata-read-decorations.ts` already documents from the cold-boot flow bind (cloud#971). Strip before validating. Applied on BOTH branches, not only the `datasetName` read: the Studio dataset preview posts its draft inline, and that draft is the document the designer GET-loaded, decorations included. A hand-authored draft never carries these keys, so the strip is a no-op there. The ADR-0010 provenance envelope (`_packageId`, `_provenance`, …) is not a read decoration and still survives. Regression coverage for the saved-dataset path was the gap that let this ship — every existing case passed the dataset inline, so nothing exercised the read. Tests now cover resolve-by-name, the inline decorated draft, the 404, and a genuinely malformed saved dataset (still 400). Also bumps the vendored console pin to objectui@f5bc4c78be76 via scripts/bump-objectui.sh, with the changeset it generates. Verified in a real browser against the showcase app: before, all 8 widgets of showcase_ops_dashboard rendered the 400; after, all 8 render live data, with zero failed responses and zero console errors. Claude-Session: https://claude.ai/code/session_01QuQBvLsJWuYFAYdkqDbM4N Co-authored-by: Claude <noreply@anthropic.com>
1 parent a114caa commit be25f97

5 files changed

Lines changed: 135 additions & 3 deletions

File tree

.changeset/console-f5bc4c78be76.md

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
---
2+
"@objectstack/console": minor
3+
---
4+
5+
Console (objectui) refreshed to `f5bc4c78be76`. Frontend changes in this range:
6+
7+
Derived from the changesets objectui declared over the range — 11 releasing of 11 changesets added across 31 non-merge commits; omitted: 20 commits carrying no changeset (they ship no package code).
8+
9+
- **minor** — Field widgets are finally told when their field fails validation, and the props slot that carries it takes the name the published contract gives it (objectui#3222). (objectui `56409c28c`)
10+
- **minor** — Retire `validation` from the action-param contract — it was declared on both halves, read by neither, and rejected outright by the server (objectui#3201). (objectui `f833d3ae4`)
11+
- **patch** — Five metadata designers stop rendering keys `@objectstack/spec` rejects, and start rendering the keys it declares (objectui#3275, objectui#3281). (objectui `8ff3ad7b8`)
12+
- **patch** — The Page block inspector's conditional-visibility control now authors `visibleWhen`, and says "Visible when" while doing it (objectui#3229). (objectui `8e02ad7f2`)
13+
- **patch** — The record discussion panel no longer shows the PREVIOUS record's comments and activity (objectui#3268). (objectui `a8aa57663`)
14+
- **patch** — The form renderer's built-in `select` branch stops saying "No options available" in English to non-English sessions (objectui#3263). (objectui `a7651e640`)
15+
- **patch** — The record discussion panel now says "loading" while it is loading, instead of "No comments yet" (objectui#3209). (objectui `12bf6691e`)
16+
- **patch** — The legacy `page-header` alias stops advertising `description` as an authorable key (objectui#3226). (objectui `d2363e710`)
17+
- **patch** — The option widgets' "this list cannot be filled" message now has one source, and it is translated (objectui#3231). (objectui `825bbe33c`)
18+
- **patch**`ToolPreview` stops advertising retired `ToolSchema` flags (objectui#3236). (objectui `30ac2e1ee`)
19+
- **patch**`TextAreaField`'s mobile fullscreen flag converges on its one real producer (objectui#3232). (objectui `a321fa461`)
20+
21+
objectui range: `785b8a5d432c...f5bc4c78be76`
Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
---
2+
"@objectstack/rest": patch
3+
---
4+
5+
fix(rest): dataset queries stop rejecting their own read-time annotation
6+
7+
Every widget on every dataset-bound dashboard failed with
8+
9+
```
10+
Dataset query failed: 400 Bad Request — Invalid dataset definition.
11+
```
12+
13+
The dataset itself was fine. `POST /analytics/dataset/query` resolves a saved
14+
`datasetName` through `getMetaItems`, and the metadata READ path stamps the
15+
spec-validation verdict `_diagnostics` onto every document it serves. Since
16+
#4001 closed the metadata schemas, `DatasetSchema.parse()` rejects unrecognized
17+
keys instead of dropping them — so the route handed a served document back to
18+
the very schema that produced it and got `unrecognized_keys: ["_diagnostics"]`
19+
for its trouble. The 400 blamed the author for a key the server had just added.
20+
21+
This is the failure mode `stripReadDecorations` exists to prevent, and the one
22+
`spec/kernel/metadata-read-decorations.ts` already documents from the cold-boot
23+
flow bind (cloud#971): *a served body is not a valid input to the schema that
24+
produced it.* The route now strips read decorations before validating.
25+
26+
Stripped on **both** branches, not only the `datasetName` read: the Studio
27+
dataset preview posts its draft inline, and that draft is the document the
28+
designer GET-loaded — decorations and all. A hand-authored draft never carries
29+
these keys, so the strip is a no-op there. The ADR-0010 provenance envelope
30+
(`_packageId`, `_provenance`, `_lock`, …) is deliberately *not* a read
31+
decoration and still survives the round-trip.
32+
33+
Regression coverage for the saved-dataset path was the gap that let this ship —
34+
every existing case passed the dataset inline, so nothing exercised the read.
35+
The route's tests now cover resolve-by-name, the inline decorated draft, the
36+
404, and a genuinely malformed saved dataset (still a 400).

.objectui-sha

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
785b8a5d432cf009389a1a9180fdac2a8297543f
1+
f5bc4c78be7629ea0c585b5be42bc9f23682532c

packages/rest/src/analytics-routes.test.ts

Lines changed: 60 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,10 +30,10 @@ const inlineDataset = {
3030
const selection = { dimensions: ['region'], measures: ['revenue'] };
3131

3232
/** Build a RestServer with an optional analytics provider (positional arg #15). */
33-
function buildServer(analyticsProvider?: any) {
33+
function buildServer(analyticsProvider?: any, protocol: any = mockProtocol()) {
3434
const server = mockServer();
3535
const rest = new RestServer(
36-
server as any, mockProtocol() as any, { api: { requireAuth: false } } as any,
36+
server as any, protocol as any, { api: { requireAuth: false } } as any,
3737
undefined, undefined, undefined, undefined, undefined, undefined, undefined,
3838
undefined, undefined, undefined, undefined,
3939
analyticsProvider,
@@ -90,6 +90,64 @@ describe('POST /analytics/dataset/query', () => {
9090
expect(res.body.code).toBe('VALIDATION_FAILED');
9191
});
9292

93+
// ── saved datasets (`datasetName`) ─────────────────────────────────────────
94+
// Every case above passes the dataset INLINE, which is why the read path
95+
// below shipped broken: `getMetaItems` stamps the read-time `_diagnostics`
96+
// verdict onto every served item, and `DatasetSchema` is closed (#4001), so
97+
// the strict re-parse rejected our own decoration and answered 400 "Invalid
98+
// dataset definition." for every saved dataset — i.e. every dashboard widget.
99+
it('resolves a saved dataset by name and strips read decorations before parsing', async () => {
100+
const served = {
101+
...inlineDataset,
102+
_packageId: 'com.example.showcase',
103+
_diagnostics: { valid: true },
104+
};
105+
const protocol = { ...mockProtocol(), getMetaItems: vi.fn().mockResolvedValue({ items: [served] }) };
106+
const queryDataset = vi.fn().mockResolvedValue({ rows: [{ region: 'NA', revenue: 100 }], fields: [] });
107+
const { route } = buildServer(async () => ({ queryDataset }), protocol);
108+
const res = mockRes();
109+
await route!.handler({ method: 'POST', params: {}, headers: {}, body: { datasetName: 'sales', selection } } as any, res);
110+
111+
expect(res.statusCode).toBe(200);
112+
expect(res.body.rows).toEqual([{ region: 'NA', revenue: 100 }]);
113+
const passed = queryDataset.mock.calls[0][0];
114+
expect(passed).not.toHaveProperty('_diagnostics');
115+
// The ADR-0010 provenance envelope is NOT a read decoration — it survives.
116+
expect(passed._packageId).toBe('com.example.showcase');
117+
});
118+
119+
// The Studio dataset preview posts the draft INLINE — and that draft is the
120+
// document the designer GET-loaded, decorations included.
121+
it('strips read decorations from an INLINE dataset too', async () => {
122+
const queryDataset = vi.fn().mockResolvedValue({ rows: [], fields: [] });
123+
const { route } = buildServer(async () => ({ queryDataset }));
124+
const res = mockRes();
125+
const served = { ...inlineDataset, _diagnostics: { valid: true }, _provenance: 'package' };
126+
await route!.handler({ method: 'POST', params: {}, headers: {}, body: { dataset: served, selection } } as any, res);
127+
128+
expect(res.statusCode).toBe(200);
129+
expect(queryDataset.mock.calls[0][0]).not.toHaveProperty('_diagnostics');
130+
});
131+
132+
it('returns 404 for an unknown datasetName', async () => {
133+
const protocol = { ...mockProtocol(), getMetaItems: vi.fn().mockResolvedValue({ items: [] }) };
134+
const { route } = buildServer(async () => ({ queryDataset: vi.fn() }), protocol);
135+
const res = mockRes();
136+
await route!.handler({ method: 'POST', params: {}, headers: {}, body: { datasetName: 'nope', selection } } as any, res);
137+
expect(res.statusCode).toBe(404);
138+
expect(res.body.code).toBe('NOT_FOUND');
139+
});
140+
141+
it('still rejects a saved dataset that is genuinely malformed', async () => {
142+
const served = { ...inlineDataset, measures: [{ name: 'x', aggregate: 'not_a_real_agg' }], _diagnostics: { valid: false } };
143+
const protocol = { ...mockProtocol(), getMetaItems: vi.fn().mockResolvedValue({ items: [served] }) };
144+
const { route } = buildServer(async () => ({ queryDataset: vi.fn() }), protocol);
145+
const res = mockRes();
146+
await route!.handler({ method: 'POST', params: {}, headers: {}, body: { datasetName: 'sales', selection } } as any, res);
147+
expect(res.statusCode).toBe(400);
148+
expect(res.body.code).toBe('VALIDATION_FAILED');
149+
});
150+
93151
it('maps a dataset D-C compile error to 400 (undeclared relationship)', async () => {
94152
const queryDataset = vi.fn().mockRejectedValue(new Error("dimension \"region\" references relationship \"account\" via \"account.region\", but \"account\" is not declared in the dataset's `include`."));
95153
const { route } = buildServer(async () => ({ queryDataset }));

packages/rest/src/rest-server.ts

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import { DataProtocol, MetadataProtocol } from '@objectstack/spec/api';
1212
import type { FieldErrorCode } from '@objectstack/spec/api';
1313
import { PUBLIC_FORM_SERVER_MANAGED_FIELDS } from '@objectstack/spec/security';
1414
import { PLURAL_TO_SINGULAR } from '@objectstack/spec/shared';
15+
import { stripReadDecorations } from '@objectstack/spec/kernel';
1516
import type { DroppedFieldsEvent } from '@objectstack/spec/data';
1617
import { preferredLocaleFromHeader } from '@objectstack/spec/system';
1718
import type { ISecurityService } from '@objectstack/spec/contracts';
@@ -5993,6 +5994,22 @@ export class RestServer {
59935994
return res.status(400).json({ code: 'VALIDATION_FAILED', message: 'Provide body.dataset (inline) or body.datasetName.' });
59945995
}
59955996

5997+
// A SERVED document is not a valid input to the schema that
5998+
// produced it: the read path stamps `_diagnostics` on every
5999+
// item `getMetaItems` returns, and since #4001
6000+
// `DatasetSchema` is CLOSED — so the parse below rejected
6001+
// our OWN annotation with `unrecognized_keys`, answering
6002+
// 400 "Invalid dataset definition." for every saved dataset,
6003+
// i.e. every widget on every dataset-bound dashboard. Same
6004+
// shape as the cold-boot flow bind (cloud#971).
6005+
//
6006+
// Stripped on BOTH branches, not just the `datasetName`
6007+
// read: the Studio dataset preview posts its draft INLINE,
6008+
// and that draft is the document the designer GET-loaded —
6009+
// decorations and all. A genuinely hand-authored draft
6010+
// never carries these keys, so the strip is a no-op there.
6011+
dataset = stripReadDecorations(dataset);
6012+
59966013
// Validate against the spec schema so a malformed draft
59976014
// yields a clean 400 instead of a runtime throw.
59986015
try {

0 commit comments

Comments
 (0)