Skip to content

Commit 4ce08e9

Browse files
committed
feat(rest): parse the dataset-query selection at the door
`POST {basePath}/analytics/dataset/query` checked only that `selection.measures` was a non-empty array, so every other member reached `dataset-executor` unrefused while the sibling analytics routes lift the identical failure to a 400 at their entry. Measured first, because the card left it open: the dataset route's `selection` is a `DatasetSelection`, NOT the `AnalyticsQuery` the siblings parse. Seven of its eleven members declare exactly the AnalyticsQuery member of the same name; four are dataset-only. The door therefore parses a projection of the seven against `AnalyticsQuerySchema.pick(...)` — a pull-back onto published text — and projects the four away rather than refusing them. Claude-Session: https://claude.ai/code/session_01DapQyvYrFb1MxSYe7BL2nt Co-authored-by: Claude <noreply@anthropic.com>
1 parent edaf3b2 commit 4ce08e9

3 files changed

Lines changed: 584 additions & 0 deletions

File tree

Lines changed: 365 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,365 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* [#17058] `POST /analytics/dataset/query` parses its `selection` AT THE DOOR.
5+
*
6+
* The defect: the route checked only that `selection.measures` was a non-empty
7+
* array, so every other member reached `dataset-executor` unrefused — while the
8+
* sibling routes (`/analytics/query`, `/analytics/sql`) lift the identical
9+
* failure to a 400 at their entry. One family, two postures.
10+
*
11+
* ## The measurement the card left open, taken here and PINNED
12+
*
13+
* The card asked whether the dataset route's `selection` is genuinely the same
14+
* shape as the siblings' before reusing their schema. It is **not** — §1 below
15+
* drives that against the real schema — so the door parses a PROJECTION of the
16+
* members whose declarations coincide, and the four dataset-only members are
17+
* projected away rather than refused. §5 is the other half of that answer and
18+
* the one that matters most: a fully-loaded VALID selection still passes.
19+
* A door that refuses too much is a worse defect than the one being fixed.
20+
*/
21+
22+
import { describe, it, expect, vi } from 'vitest';
23+
import { RestServer } from './rest-server';
24+
import {
25+
SELECTION_MEMBERS_SHARED_WITH_ANALYTICS_QUERY,
26+
datasetSelectionRefusal,
27+
} from './analytics-selection-door';
28+
29+
// ── harness (the shape `analytics-routes.test.ts` uses) ──────────────────────
30+
31+
function mockServer() {
32+
return {
33+
get: vi.fn(), post: vi.fn(), put: vi.fn(), delete: vi.fn(), patch: vi.fn(),
34+
use: vi.fn(), listen: vi.fn().mockResolvedValue(undefined), close: vi.fn().mockResolvedValue(undefined),
35+
};
36+
}
37+
function mockProtocol() {
38+
return {
39+
getDiscovery: vi.fn().mockResolvedValue({ version: 'v0', routes: { data: '', metadata: '' } }),
40+
getMetaTypes: vi.fn().mockResolvedValue([]),
41+
getMetaItems: vi.fn().mockResolvedValue([]),
42+
};
43+
}
44+
function mockRes() {
45+
const res: any = { statusCode: 200, body: undefined };
46+
res.status = vi.fn((c: number) => { res.statusCode = c; return res; });
47+
res.json = vi.fn((b: any) => { res.body = b; return res; });
48+
res.end = vi.fn(() => res);
49+
return res;
50+
}
51+
52+
const inlineDataset = {
53+
name: 'sales',
54+
label: 'Sales',
55+
object: 'opportunity',
56+
dimensions: [
57+
{ name: 'region', field: 'region', type: 'string' },
58+
{ name: 'close_date', field: 'close_date', type: 'date' },
59+
],
60+
measures: [{ name: 'revenue', aggregate: 'sum', field: 'amount' }],
61+
};
62+
63+
function buildRoute() {
64+
const queryDataset = vi.fn().mockResolvedValue({ rows: [], fields: [] });
65+
const server = mockServer();
66+
const rest = new RestServer(
67+
server as any, mockProtocol() as any, { api: { requireAuth: false } } as any,
68+
undefined, undefined, undefined, undefined, undefined, undefined, undefined,
69+
undefined, undefined, undefined, undefined,
70+
async () => ({ queryDataset }),
71+
);
72+
(rest as any).resolveExecCtx = async () => ({ userId: 'test-user' });
73+
rest.registerRoutes();
74+
const route = rest.getRoutes().find((r) => r.method === 'POST' && r.path.endsWith('/analytics/dataset/query'))!;
75+
expect(route, 'POST …/analytics/dataset/query must be registered').toBeTruthy();
76+
return { route, queryDataset };
77+
}
78+
79+
/** POST a body through the REAL route and return `{ res, queryDataset }`. */
80+
async function post(body: unknown) {
81+
const { route, queryDataset } = buildRoute();
82+
const res = mockRes();
83+
await route.handler({ method: 'POST', params: {}, headers: {}, body } as any, res);
84+
return { res, queryDataset };
85+
}
86+
87+
// ─────────────────────────────────────────────────────────────────────────────
88+
// §1 — the shape question: `DatasetSelection` is NOT the siblings' shape
89+
// ─────────────────────────────────────────────────────────────────────────────
90+
91+
describe('#17058 §1 — the dataset route\'s `selection` is not the sibling routes\' body shape', () => {
92+
/**
93+
* A legal, ordinary dashboard-widget selection. Every member here is
94+
* declared on `DatasetSelection` (`spec/contracts/analytics-service.ts`).
95+
*/
96+
const legalSelection = {
97+
dimensions: ['region'],
98+
measures: ['revenue'],
99+
runtimeFilter: { region: 'NA' },
100+
timeDimensions: [{ dimension: 'close_date', granularity: 'month', dateRange: 'last_30_days' }],
101+
dateGranularity: 'month',
102+
order: { revenue: 'desc' },
103+
limit: 10,
104+
offset: 0,
105+
compareTo: { kind: 'previousPeriod' },
106+
totals: { groupings: [['region'], []] },
107+
timezone: 'Asia/Shanghai',
108+
} as const;
109+
110+
it('the siblings\' own schema REFUSES it — on `cube` and on all four dataset-only members', async () => {
111+
const { AnalyticsQueryRequestSchema } = await import('@objectstack/spec/api');
112+
const parsed = (AnalyticsQueryRequestSchema as any).safeParse(legalSelection);
113+
114+
expect(parsed.success, 'a legal DatasetSelection must NOT parse as an AnalyticsQuery').toBe(false);
115+
const paths: string[] = parsed.error.issues.map((i: any) => i.path.join('.'));
116+
const unrecognized: string[] = parsed.error.issues
117+
.filter((i: any) => i.code === 'unrecognized_keys')
118+
.flatMap((i: any) => i.keys ?? []);
119+
120+
// `cube` is required there and absent here — a dataset selection names
121+
// no cube; the dataset is addressed by `body.dataset`/`datasetName`.
122+
expect(paths).toContain('cube');
123+
// …and the schema is `.strict()`, so the dataset-only members are keys
124+
// it has never heard of. This is why reusing it would 400 every real
125+
// dashboard widget.
126+
expect(unrecognized.sort()).toEqual(
127+
['compareTo', 'dateGranularity', 'runtimeFilter', 'totals'].sort(),
128+
);
129+
});
130+
131+
it('the shared projection accepts the same selection — that is the half this door parses', async () => {
132+
expect(await datasetSelectionRefusal(legalSelection)).toBeUndefined();
133+
});
134+
135+
/**
136+
* The projection list is a claim about two declarations agreeing. Pin both
137+
* directions so a later edit cannot quietly move a member into or out of it.
138+
*/
139+
it('every projected member is an `AnalyticsQuery` member; no dataset-only member is', async () => {
140+
const { AnalyticsQuerySchema } = await import('@objectstack/spec/data');
141+
const analyticsMembers = Object.keys((AnalyticsQuerySchema as any).shape);
142+
for (const member of SELECTION_MEMBERS_SHARED_WITH_ANALYTICS_QUERY) {
143+
expect(analyticsMembers, `${member} must be declared on AnalyticsQuery`).toContain(member);
144+
}
145+
for (const datasetOnly of ['runtimeFilter', 'dateGranularity', 'compareTo', 'totals']) {
146+
expect(analyticsMembers).not.toContain(datasetOnly);
147+
expect(SELECTION_MEMBERS_SHARED_WITH_ANALYTICS_QUERY as readonly string[])
148+
.not.toContain(datasetOnly);
149+
}
150+
});
151+
});
152+
153+
// ─────────────────────────────────────────────────────────────────────────────
154+
// §2 — the card's measured specimen, driven through the real route
155+
// ─────────────────────────────────────────────────────────────────────────────
156+
157+
describe('#17058 §2 — a malformed `dateRange` is refused at the door, not by the face behind it', () => {
158+
it('answers 400 ANALYTICS_DATE_RANGE_UNRECOGNIZED and never reaches the service', async () => {
159+
const { res, queryDataset } = await post({
160+
dataset: inlineDataset,
161+
selection: {
162+
dimensions: ['region'],
163+
measures: ['revenue'],
164+
timeDimensions: [{ dimension: 'close_date', dateRange: 'not a range at all' }],
165+
},
166+
});
167+
168+
expect(res.statusCode).toBe(400);
169+
// The SAME code the sibling door answers for the identical condition —
170+
// one condition, one code (ADR-0112 D3 / the #5240 convention).
171+
expect(res.body.code).toBe('ANALYTICS_DATE_RANGE_UNRECOGNIZED');
172+
// The message locates the offending member on the REQUEST body…
173+
expect(res.body.message).toContain('selection.timeDimensions.0.dateRange');
174+
// …and carries the schema's own prescription, quoted not restated.
175+
expect(res.body.message).toContain('not a range at all');
176+
expect(res.body.message).toContain('last_7_days');
177+
// The whole point of a door: the executor never sees it.
178+
expect(queryDataset).not.toHaveBeenCalled();
179+
});
180+
181+
it('the envelope\'s code is a registered vocabulary member, not a dialect', async () => {
182+
const { ApiErrorSchema } = await import('@objectstack/spec/api');
183+
const { res } = await post({
184+
dataset: inlineDataset,
185+
selection: {
186+
measures: ['revenue'],
187+
timeDimensions: [{ dimension: 'close_date', dateRange: 'Last 7 days' }],
188+
},
189+
});
190+
const parsed = (ApiErrorSchema as any).safeParse({
191+
code: res.body.code,
192+
message: res.body.message,
193+
httpStatus: res.statusCode,
194+
});
195+
expect(parsed.success, JSON.stringify(parsed.success ? null : parsed.error.issues)).toBe(true);
196+
});
197+
});
198+
199+
// ─────────────────────────────────────────────────────────────────────────────
200+
// §3 — the rest of the shape: `timeDimensions` is not special
201+
// ─────────────────────────────────────────────────────────────────────────────
202+
203+
describe('#17058 §3 — the generic refusal is 400 VALIDATION_FAILED + details.fields[]', () => {
204+
const cases: Array<{ name: string; selection: Record<string, unknown>; field: string }> = [
205+
{
206+
name: 'a typo\'d nested key (`granuarity`) — top-level strictness does not recurse',
207+
selection: {
208+
measures: ['revenue'],
209+
timeDimensions: [{ dimension: 'close_date', granuarity: 'month' }],
210+
},
211+
field: 'selection.timeDimensions.0',
212+
},
213+
{
214+
name: 'a measure name that is not a string',
215+
selection: { measures: [42] },
216+
field: 'selection.measures.0',
217+
},
218+
{
219+
name: 'a dimension list that is not a list',
220+
selection: { measures: ['revenue'], dimensions: 'region' },
221+
field: 'selection.dimensions',
222+
},
223+
{
224+
name: 'an order direction outside the declared pair',
225+
selection: { measures: ['revenue'], order: { revenue: 'ASC' } },
226+
field: 'selection.order.revenue',
227+
},
228+
{
229+
name: 'a `limit` sent as a string',
230+
selection: { measures: ['revenue'], limit: '10' },
231+
field: 'selection.limit',
232+
},
233+
];
234+
235+
for (const c of cases) {
236+
it(`refuses ${c.name}`, async () => {
237+
const { res, queryDataset } = await post({ dataset: inlineDataset, selection: c.selection });
238+
expect(res.statusCode).toBe(400);
239+
expect(res.body.code).toBe('VALIDATION_FAILED');
240+
const fields: Array<{ field: string; code: string }> = res.body.details.fields;
241+
expect(fields.map((f) => f.field)).toContain(c.field);
242+
expect(queryDataset).not.toHaveBeenCalled();
243+
});
244+
}
245+
246+
it('the date-range lift is all-or-nothing: a body wrong in MORE places stays generic', async () => {
247+
const { res } = await post({
248+
dataset: inlineDataset,
249+
selection: {
250+
measures: ['revenue'],
251+
timeDimensions: [{ dimension: 'close_date', dateRange: 'nope', granuarity: 'month' }],
252+
},
253+
});
254+
expect(res.statusCode).toBe(400);
255+
expect(res.body.code).toBe('VALIDATION_FAILED');
256+
const fields: Array<{ field: string }> = res.body.details.fields;
257+
expect(fields.map((f) => f.field)).toContain('selection.timeDimensions.0.dateRange');
258+
expect(fields.map((f) => f.field)).toContain('selection.timeDimensions.0');
259+
});
260+
261+
it('the `measures` door ahead of the parse keeps its own sentence', async () => {
262+
const { res } = await post({ dataset: inlineDataset, selection: { dimensions: ['region'] } });
263+
expect(res.statusCode).toBe(400);
264+
expect(res.body.code).toBe('VALIDATION_FAILED');
265+
expect(res.body.message).toContain('body.selection.measures');
266+
});
267+
});
268+
269+
// ─────────────────────────────────────────────────────────────────────────────
270+
// §4 — the four dataset-only members keep passing (they are projected away)
271+
// ─────────────────────────────────────────────────────────────────────────────
272+
273+
describe('#17058 §4 — the dataset-only members are NOT judged by the sibling schema', () => {
274+
const datasetOnly: Array<[string, unknown]> = [
275+
['runtimeFilter', { region: { $ne: 'EU' } }],
276+
['dateGranularity', 'quarter'],
277+
['compareTo', { kind: 'previousYear' }],
278+
['totals', { groupings: [[]] }],
279+
];
280+
281+
for (const [member, value] of datasetOnly) {
282+
it(`\`${member}\` reaches the service untouched`, async () => {
283+
const selection = { dimensions: ['region'], measures: ['revenue'], [member]: value };
284+
const { res, queryDataset } = await post({ dataset: inlineDataset, selection });
285+
expect(res.statusCode).toBe(200);
286+
expect(queryDataset).toHaveBeenCalledTimes(1);
287+
expect(queryDataset.mock.calls[0][1]).toBe(selection);
288+
});
289+
}
290+
});
291+
292+
// ─────────────────────────────────────────────────────────────────────────────
293+
// §5 — ⭐ the negative side: a valid selection still passes, unmodified
294+
// ─────────────────────────────────────────────────────────────────────────────
295+
296+
describe('#17058 §5 — a valid selection still passes, and passes through unchanged', () => {
297+
it('a fully-loaded selection — all eleven members — answers 200', async () => {
298+
const selection = {
299+
dimensions: ['region'],
300+
measures: ['revenue'],
301+
runtimeFilter: { region: 'NA' },
302+
timeDimensions: [{ dimension: 'close_date', granularity: 'month', dateRange: 'last_30_days' }],
303+
dateGranularity: 'month',
304+
order: { revenue: 'desc' },
305+
limit: 10,
306+
offset: 0,
307+
compareTo: { kind: 'previousPeriod', dimension: 'close_date' },
308+
totals: { groupings: [['region'], []] },
309+
timezone: 'Asia/Shanghai',
310+
};
311+
const { res, queryDataset } = await post({ dataset: inlineDataset, selection });
312+
expect(res.statusCode).toBe(200);
313+
expect(queryDataset).toHaveBeenCalledTimes(1);
314+
// Validation-only: the CALLER's object is what the service receives,
315+
// by identity — never a parse output that could carry a schema default.
316+
expect(queryDataset.mock.calls[0][1]).toBe(selection);
317+
});
318+
319+
it('the explicit `[start, end]` window arm is untouched by the closing', async () => {
320+
const { res, queryDataset } = await post({
321+
dataset: inlineDataset,
322+
selection: {
323+
measures: ['revenue'],
324+
timeDimensions: [{ dimension: 'close_date', dateRange: ['2026-01-01', '{today}'] }],
325+
},
326+
});
327+
expect(res.statusCode).toBe(200);
328+
expect(queryDataset).toHaveBeenCalledTimes(1);
329+
});
330+
331+
/**
332+
* The leniency sweep, as a test. Every `selection` literal the in-repo
333+
* suites POST at this route (and the one `packages/qa/dogfood` sends) is
334+
* replayed through the new door: if any of them had been relying on the
335+
* leniency, the fix would have broken it, and triage asked for that answer
336+
* explicitly rather than as an impression.
337+
*/
338+
it('every in-repo selection specimen still passes the door', async () => {
339+
const specimens: Array<Record<string, unknown>> = [
340+
// packages/rest/src/analytics-dataset-dimension-gate.test.ts
341+
{ measures: ['account_count'], dimensions: ['bogus_dim'] },
342+
{ measures: ['account_count'], dimensions: ['industry'], timeDimensions: [{ dimension: 'bogus_dim', granularity: 'month' }] },
343+
// packages/rest/src/analytics-dataset-where-gate.test.ts
344+
{ measures: ['account_count'], runtimeFilter: { bogus_col: 'x' } },
345+
{ measures: ['account_count'], runtimeFilter: { industry: { $sortOf: 'tech' } } },
346+
// packages/rest/src/analytics-dataset-refusal-envelope.test.ts
347+
{ dimensions: ['stage'], measures: ['revenue'], order: { profit: 'desc' } },
348+
{ dimensions: ['stage'], measures: ['revenue'], totals: { groupings: [['region']] } },
349+
// packages/rest/src/analytics-dataset-unlisted-refusal-envelope.test.ts
350+
{ dimensions: ['stage'], measures: ['revenue'], timeDimensions: [{ dimension: 'close_date', dateRange: ['2026-01-01', 'the-first-of-never'] }], compareTo: { kind: 'previousPeriod' } },
351+
{ dimensions: ['stage'], measures: ['revenue'], timeDimensions: [{ dimension: 'close_date', granularity: 'month' }], compareTo: { kind: 'previousPeriod' } },
352+
{ dimensions: ['stage'], measures: ['revenue'], timeDimensions: [{ dimension: 'account_opened', granularity: 'month' }] },
353+
// packages/client/src/client.test.ts
354+
{ measures: ['amount_sum'] },
355+
// packages/qa/dogfood/test/temporal-storage-e2e.dogfood.test.ts
356+
{ measures: ['cnt'], timeDimensions: [{ dimension: 'issued', granularity: 'month' }] },
357+
];
358+
for (const selection of specimens) {
359+
expect(
360+
await datasetSelectionRefusal(selection),
361+
`specimen must still pass: ${JSON.stringify(selection)}`,
362+
).toBeUndefined();
363+
}
364+
});
365+
});

0 commit comments

Comments
 (0)