Skip to content

Commit 9b6fe7c

Browse files
os-zhuangclaude
andauthored
fix(spec,runtime)!: AnalyticsQueryRequest is the bare AnalyticsQuery; validate /analytics bodies at the entry (#3878) (#4010)
The spec's AnalyticsQueryRequestSchema described a {cube, query, format} ENVELOPE — the dialect of the retired degraded shim (#3891) that the real engine never understood: an envelope body inferred a column-less cube and died as a driver SQL syntax error (SELECT FROM ...) instead of a shape error, and a non-contract `filters` key was silently ignored. That mismatch sent #3867's first investigation to a wrong conclusion. Spec: - AnalyticsQueryRequestSchema = bare AnalyticsQuery + required top-level `cube`, `.strict()`. `query` and `format` are retiredKey-tombstoned (tsc `never` + parse-time prescription); `format` was never implemented (declared != enforced). - Registered as two step-17 semantic migrations (an HTTP-wire change with no stored metadata to rewrite); spec-changes.json, upgrade guide, authorable-surface, JSON schemas, OpenAPI, reference docs regenerated. Runtime: - POST /analytics/query and /analytics/sql validate the body at the entry and raise the duck-typed VALIDATION_FAILED shape both dispatcher error exits already map to 400 + fields[] (#3918) — the envelope answers with the tombstone's migration text, `filters` gets a bespoke hint at `where`. The domain throws through, matching its existing error contract; the wire-level 400 is pinned in dispatcher-validation-error.real.test.ts. - A valid body is forwarded byte-identical (validation only): parsing would inject the schema's timezone 'UTC' default and override org-timezone resolution (#1982/#2018). - Service-absent still answers 404 before body inspection (#3891). Tests: 5 new entry-validation cases + wire-level 400/200 pins; 9 legacy fixtures updated to contract-valid bodies. Refs #3878, #3891, #3867, #3918. Part 1 of the #3878 follow-through; route-mount conditionality is the next PR. Claude-Session: https://claude.ai/code/session_017WZBR9XyhXoqKCU6qQVyjx Co-authored-by: Claude <noreply@anthropic.com>
1 parent 48fcf70 commit 9b6fe7c

14 files changed

Lines changed: 431 additions & 69 deletions
Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
---
2+
"@objectstack/spec": major
3+
"@objectstack/runtime": minor
4+
---
5+
6+
fix(spec,runtime)!: `AnalyticsQueryRequest` is the bare `AnalyticsQuery`; the dispatcher validates `/analytics` bodies at the entry (#3878)
7+
8+
**Spec.** `AnalyticsQueryRequestSchema` used to describe a
9+
`{ cube, query: {...}, format }` ENVELOPE — the dialect of the retired degraded
10+
analytics shim (#3891), which the real engine never understood: an envelope
11+
body inferred a column-less cube and died as an SQL syntax error
12+
(`SELECT FROM …`) instead of a shape error. The schema now describes what the
13+
engine and every real caller actually use — the **bare `AnalyticsQuery`**:
14+
15+
```
16+
FROM { "cube": "orders", "query": { "measures": ["count"] }, "format": "json" }
17+
TO { "cube": "orders", "measures": ["count"], "dimensions": [...], "where": {...} }
18+
```
19+
20+
`cube` + `measures` are required at the top level; `dimensions` / `where` /
21+
`timeDimensions` / `order` / `limit` / `offset` / `timezone` sit beside them.
22+
The schema is `.strict()`; `query` and `format` are tombstoned (`retiredKey`)
23+
so both `tsc` and the parse answer with this exact migration. `format` was
24+
never implemented (every response is the JSON envelope) — for CSV/XLSX use the
25+
export surface. The removal is registered as two step-17 semantic migrations
26+
(`analytics-query-request-envelope-retired`,
27+
`analytics-query-request-format-retired`) — it is an HTTP-wire change with no
28+
stored metadata to rewrite.
29+
30+
**Runtime.** `POST /api/v1/analytics/query` and `/analytics/sql` now validate
31+
the body against that schema AT THE ENTRY and answer
32+
**400 `VALIDATION_FAILED`** with per-field details — including the envelope
33+
prescription above, and a bespoke hint that `filters` is not a contract field
34+
(the filter field is `where`, the same canonical FilterCondition `find()`
35+
takes). Previously a malformed body reached the engine and failed as a 500 SQL
36+
syntax error, or had its off-contract filter key silently ignored. A valid
37+
body is forwarded to the analytics service byte-identical (validation only —
38+
parsing would inject the schema's `timezone: 'UTC'` default and override
39+
org-timezone resolution). An uninstalled analytics capability still answers
40+
404 before any body inspection (#3891).

content/docs/references/api/analytics.mdx

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -58,9 +58,17 @@ const result = AnalyticsEndpoint.parse(data);
5858

5959
| Property | Type | Required | Description |
6060
| :--- | :--- | :--- | :--- |
61-
| **query** | `{ cube?: string; measures: string[]; dimensions?: string[]; where?: any; … }` || The analytic query definition |
6261
| **cube** | `string` || Target cube name |
63-
| **format** | `Enum<'json' \| 'csv' \| 'xlsx'>` || Response format |
62+
| **measures** | `string[]` || List of metrics to calculate |
63+
| **dimensions** | `string[]` | optional | List of dimensions to group by |
64+
| **where** | `any` | optional | Filtering criteria (canonical Query DSL FilterCondition) |
65+
| **timeDimensions** | `{ dimension: string; granularity?: Enum<'second' \| 'minute' \| 'hour' \| 'day' \| 'week' \| 'month' \| 'quarter' \| 'year'>; dateRange?: string \| string[] }[]` | optional | |
66+
| **order** | `Record<string, Enum<'asc' \| 'desc'>>` | optional | |
67+
| **limit** | `number` | optional | |
68+
| **offset** | `number` | optional | |
69+
| **timezone** | `string` || |
70+
| **query** | `any` | optional | [REMOVED] `query` was removed from AnalyticsQueryRequest in @objectstack/spec 17.0.0 (#3878). The `{ cube, query: {...}` } envelope was the dialect of the retired degraded analytics shim (#3891) — the real engine never understood it. Move the query.* fields to the body top level: `{ cube, measures, dimensions?, where?, timeDimensions?, order?, limit?, offset?, timezone? }`. |
71+
| **format** | `any` | optional | [REMOVED] `format` was removed from AnalyticsQueryRequest in @objectstack/spec 17.0.0 (#3878). It was never implemented — every response is the JSON envelope. Delete the key; for CSV/XLSX use the export surface instead. |
6472

6573

6674
---

docs/protocol-upgrade-guide.md

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,8 @@ Beyond those spec-surface removals, it graduates the seven flow-node config key
126126

127127
And it removes the RLS-policy key `priority` (#3896 security audit): promised "conflict resolution" that cannot exist, because applicable policies OR-combine (most permissive wins) — there is never a conflict to order, and nothing ever read the key (call graph closed across the collection site, the projection round-trip and the compiler). A pure lossless delete: outcomes are identical with or without it; the schema tombstones the key with the same prescription.
128128

129+
On the wire contract it also retires the `/analytics/query` request ENVELOPE (#3878): `AnalyticsQueryRequestSchema` used to describe `{ cube, query: {...}, format }` — the dialect of the retired degraded analytics shim (#3891) that the real engine never understood (an envelope body inferred a column-less cube and died as an SQL syntax error). The canonical request body is now the BARE AnalyticsQuery — `cube` + `measures` at the top level — which is what every real caller already sends; the schema tombstones `query`/`format`, and the dispatcher entry validates bodies and answers 400 with the prescription. No stored metadata carries this shape (it was HTTP-only), so the change is two semantic TODOs for API callers rather than a stack conversion.
130+
129131
### Mechanical (applied for you)
130132

131133
| Conversion | Surface | Change | Load window |
@@ -140,6 +142,15 @@ And it removes the RLS-policy key `priority` (#3896 security audit): promised "c
140142
| `flow-node-script-config-aliases` | `flow.node.script.config` | script flow-node config keys 'functionName' → 'function', 'input' → 'inputs' (#3796) | live — protocol 17 loader accepts the old shape |
141143
| `permission-rls-priority-removed` | `permission.rowLevelSecurity.priority` | RLS-policy key 'priority' removed (#3896 audit — policies OR-combine, so the promised conflict-resolution semantics cannot exist; dropping it changes no outcome) | retired — `migrate meta` only |
142144

145+
### Semantic (delegated to you, with acceptance criteria)
146+
147+
- **`analytics-query-request-envelope-retired`**`api.analyticsQueryRequest.query` → bare AnalyticsQuery body (top-level cube/measures/dimensions/where/...)
148+
- Why not automatic: The { cube, query: {...} } envelope was an HTTP-wire dialect of the retired degraded analytics shim (#3891), never stored in stack metadata — there is no source for the chain to rewrite. Callers of POST /analytics/query and /analytics/sql must move the query.* fields to the body top level themselves.
149+
- Done when: Every /analytics/query and /analytics/sql call sends the bare AnalyticsQuery shape and succeeds; no request answers 400 VALIDATION_FAILED with the envelope prescription.
150+
- **`analytics-query-request-format-retired`**`api.analyticsQueryRequest.format` → (removed — responses are always the JSON envelope; use the export surface for CSV/XLSX)
151+
- Why not automatic: The `format` key was declared but never implemented (declared ≠ enforced): every response is the JSON envelope regardless of the requested value, so there is no behaviour to preserve and nothing stored to rewrite.
152+
- Done when: No /analytics/query or /analytics/sql call sends `format`; exports go through the export surface.
153+
143154
---
144155

145156
*Machine-readable equivalents: `spec-changes.json` (shipped in `@objectstack/spec` and attached to each GitHub Release) and the structured output of `objectstack migrate meta --json`.*

packages/runtime/src/dispatcher-plugin.error-envelope.test.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,9 @@ async function postAnalyticsQuery(err: unknown) {
8484
expect(handler, 'POST /api/v1/analytics/query must be mounted').toBeTypeOf('function');
8585

8686
const res = makeRes();
87-
await handler({ body: { cube: 'x', query: {} }, query: {} }, res);
87+
// [#3878] Body must pass entry validation so the SERVICE's thrown error —
88+
// the thing under test — is what reaches the exit, not an entry 400.
89+
await handler({ body: { cube: 'x', measures: ['count'] }, query: {} }, res);
8890
return res;
8991
}
9092

packages/runtime/src/dispatcher-validation-error.real.test.ts

Lines changed: 59 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@
2020
* are the tests that fail if the contract moves.
2121
*/
2222

23-
import { describe, it, expect } from 'vitest';
23+
import { describe, it, expect, vi } from 'vitest';
2424
import { ValidationError } from '@objectstack/objectql';
2525

2626
import { HttpDispatcher } from './http-dispatcher.js';
@@ -100,7 +100,9 @@ async function analyticsQuery(thrown: unknown) {
100100
header() { return res; },
101101
json(b: any) { res.body = b; return res; },
102102
};
103-
await handlers['POST /api/v1/analytics/query']({ body: { cube: 'x', query: {} }, query: {} }, res);
103+
// [#3878] Body must pass entry validation so the SERVICE's thrown error —
104+
// the thing under test — is what reaches the exit, not an entry 400.
105+
await handlers['POST /api/v1/analytics/query']({ body: { cube: 'x', measures: ['count'] }, query: {} }, res);
104106
return res;
105107
}
106108

@@ -138,3 +140,58 @@ describe('#3918 — both exits serve the real ValidationError as 400 + fields[]'
138140
expect(res.body.error.details.fields).toEqual([]);
139141
});
140142
});
143+
144+
// ---------------------------------------------------------------------------
145+
146+
/** [#3878] Post `body` through the real route handler; the service never throws. */
147+
async function postAnalyticsBody(body: unknown) {
148+
const handlers: Record<string, (req: any, res: any) => any> = {};
149+
const rec = (verb: string) => (path: string, h: any) => { handlers[`${verb} ${path}`] = h; };
150+
const server = {
151+
get: rec('GET'), post: rec('POST'), put: rec('PUT'),
152+
delete: rec('DELETE'), patch: rec('PATCH'),
153+
};
154+
const query = vi.fn(async () => ({ rows: [] }));
155+
const analytics = { query, getMeta: async () => ({ cubes: [] }), generateSql: async () => ({ sql: null }) };
156+
const kernel = {
157+
getService: (n: string) => (n === 'analytics' ? analytics : undefined),
158+
getServiceAsync: async (n: string) => (n === 'analytics' ? analytics : undefined),
159+
};
160+
const plugin = createDispatcherPlugin({ prefix: '/api/v1', securityHeaders: false });
161+
await plugin.start?.({
162+
getKernel: () => kernel,
163+
getService: (n: string) => (n === 'http.server' ? server : undefined),
164+
environmentId: undefined,
165+
logger: { info() {}, warn() {}, error() {}, debug() {} },
166+
hook: () => {}, on: () => {},
167+
} as any);
168+
169+
const res: any = {
170+
statusCode: undefined, body: undefined,
171+
status(c: number) { res.statusCode = c; return res; },
172+
header() { return res; },
173+
json(b: any) { res.body = b; return res; },
174+
};
175+
await handlers['POST /api/v1/analytics/query']({ body, query: {} }, res);
176+
return { res, query };
177+
}
178+
179+
describe('#3878 — entry validation reaches the wire as a 400, service untouched', () => {
180+
it('the retired envelope answers 400 with the tombstone prescription', async () => {
181+
const { res, query } = await postAnalyticsBody({ cube: 'x', query: { measures: ['count'] } });
182+
183+
expect(res.statusCode).toBe(400);
184+
expect(res.body.error.code).toBe('VALIDATION_FAILED');
185+
expect(res.body.error.message).toContain('top level');
186+
expect(query).not.toHaveBeenCalled();
187+
// A caller mistake, not a server fault: the reporter side-channel stays clear.
188+
expect(res.__obsRecordedError).toBeUndefined();
189+
});
190+
191+
it('a valid bare body passes through and answers 200', async () => {
192+
const { res, query } = await postAnalyticsBody({ cube: 'x', measures: ['count'] });
193+
194+
expect(res.statusCode).toBe(200);
195+
expect(query).toHaveBeenCalledOnce();
196+
});
197+
});

packages/runtime/src/dispatcher-validation-error.test.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -239,7 +239,9 @@ async function postAnalyticsQuery(err: unknown) {
239239
expect(handler, 'POST /api/v1/analytics/query must be mounted').toBeTypeOf('function');
240240

241241
const res = makeRes();
242-
await handler({ body: { cube: 'x', query: {} }, query: {} }, res);
242+
// [#3878] Body must pass entry validation so the SERVICE's thrown error —
243+
// the thing under test — is what reaches the exit, not an entry 400.
244+
await handler({ body: { cube: 'x', measures: ['count'] }, query: {} }, res);
243245
return res;
244246
}
245247

packages/runtime/src/domain-handler-registry.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -135,7 +135,7 @@ describe('HttpDispatcher domain registry (D11 step ③)', () => {
135135
analyticsQuery: vi.fn().mockResolvedValue({ rows: [{ n: 1 }] }),
136136
};
137137
const result = await makeDispatcher({ analytics }).dispatch(
138-
'POST', '/analytics/query', { metric: 'count' }, {}, {} as any,
138+
'POST', '/analytics/query', { cube: 'orders', measures: ['count'] }, {}, {} as any,
139139
);
140140
expect(result.handled).toBe(true);
141141
// The bridge consulted the service (whichever entry point it uses).

packages/runtime/src/domains/analytics.ts

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,9 +11,65 @@
1111
*/
1212

1313
import { CoreServiceName } from '@objectstack/spec/system';
14+
import { AnalyticsQueryRequestSchema } from '@objectstack/spec/api';
1415
import type { HttpProtocolContext, HttpDispatcherResult } from '../http-dispatcher.js';
1516
import type { DomainHandlerDeps, DomainRoute } from '../domain-handler-registry.js';
1617

18+
/**
19+
* [#3878] The duck-typed validation-failure shape both dispatcher error exits
20+
* already map to 400 `VALIDATION_FAILED` + `details.fields[]` (see
21+
* `validation-failure.ts`, #3918) — thrown here so entry validation needs no
22+
* new error channel and no dependency on objectql's `ValidationError` class.
23+
*/
24+
function validationFailure(message: string, fields: unknown[]): Error {
25+
const err = new Error(message) as Error & { code: string; fields: unknown[] };
26+
err.name = 'ValidationError';
27+
err.code = 'VALIDATION_FAILED';
28+
err.fields = fields;
29+
return err;
30+
}
31+
32+
/**
33+
* [#3878] Reject a malformed `AnalyticsQuery` body AT THE ENTRY with a 400
34+
* naming what is wrong, instead of letting it reach the engine — where a
35+
* shapeless body used to infer a column-less cube and die as an SQL syntax
36+
* error deep in the driver (`SELECT FROM …`), or worse, have its off-contract
37+
* filter key silently ignored.
38+
*
39+
* The retired `{ cube, query: {...} }` envelope (#3891 shim dialect) needs no
40+
* special case here: the schema tombstones `query`/`format` (`retiredKey`),
41+
* so the Zod issue itself carries the migration prescription. Only `filters` —
42+
* never a declared key, so `.strict()` would answer a generic
43+
* "unrecognized key" — gets a bespoke hint at the contract field `where`.
44+
*
45+
* Validation only — the ORIGINAL body is forwarded to the service untouched.
46+
* (Parsing would inject the schema's `timezone: 'UTC'` default and silently
47+
* override the engine's org-timezone resolution, #1982/#2018.)
48+
*/
49+
function assertAnalyticsQueryBody(body: unknown): void {
50+
if (body && typeof body === 'object' && !Array.isArray(body)) {
51+
const b = body as Record<string, unknown>;
52+
if ('filters' in b && !('where' in b)) {
53+
throw validationFailure(
54+
'`filters` is not an AnalyticsQuery field — use `where` (canonical Query DSL FilterCondition, the same shape find() takes).',
55+
[{ field: 'filters', code: 'unrecognized_keys', message: 'use `where` instead of `filters`' }],
56+
);
57+
}
58+
}
59+
const parsed = AnalyticsQueryRequestSchema.safeParse(body);
60+
if (!parsed.success) {
61+
const fields = parsed.error.issues.map((issue) => ({
62+
field: issue.path.length > 0 ? issue.path.join('.') : '(body)',
63+
code: issue.code,
64+
message: issue.message,
65+
}));
66+
throw validationFailure(
67+
`Invalid AnalyticsQuery body: ${fields.map((f) => `${f.field}: ${f.message}`).join('; ')}`,
68+
fields,
69+
);
70+
}
71+
}
72+
1773
export function createAnalyticsDomain(deps: DomainHandlerDeps): DomainRoute {
1874
return {
1975
prefix: '/analytics',
@@ -39,6 +95,10 @@ export async function handleAnalyticsRequest(
3995

4096
// POST /analytics/query
4197
if (subPath === 'query' && m === 'POST') {
98+
// [#3878] Entry validation AFTER the service check on purpose: an
99+
// uninstalled analytics capability answers 404 (the honest "install
100+
// service-analytics" signal, #3891) regardless of body shape.
101+
assertAnalyticsQueryBody(body);
42102
// [#2852] Pass the request's execution context so the analytics
43103
// service scopes each object by its per-object read filter (tenant +
44104
// RLS). Without it, `getReadScope(object, undefined)` returned no
@@ -60,6 +120,8 @@ export async function handleAnalyticsRequest(
60120

61121
// POST /analytics/sql (Dry-run or debug)
62122
if (subPath === 'sql' && m === 'POST') {
123+
// [#3878] Same body contract as /query — validated the same way.
124+
assertAnalyticsQueryBody(body);
63125
// [#2852] Scope the generated SQL to the caller too, so a preview
64126
// reflects the same per-object read filter the real query applies.
65127
const result = await analyticsService.generateSql(body, context?.executionContext);

0 commit comments

Comments
 (0)