diff --git a/.changeset/analytics-query-bare-shape-entry-validation.md b/.changeset/analytics-query-bare-shape-entry-validation.md new file mode 100644 index 0000000000..71339c011d --- /dev/null +++ b/.changeset/analytics-query-bare-shape-entry-validation.md @@ -0,0 +1,40 @@ +--- +"@objectstack/spec": major +"@objectstack/runtime": minor +--- + +fix(spec,runtime)!: `AnalyticsQueryRequest` is the bare `AnalyticsQuery`; the dispatcher validates `/analytics` bodies at the entry (#3878) + +**Spec.** `AnalyticsQueryRequestSchema` used to describe a +`{ cube, query: {...}, format }` ENVELOPE — the dialect of the retired degraded +analytics shim (#3891), which the real engine never understood: an envelope +body inferred a column-less cube and died as an SQL syntax error +(`SELECT FROM …`) instead of a shape error. The schema now describes what the +engine and every real caller actually use — the **bare `AnalyticsQuery`**: + +``` +FROM { "cube": "orders", "query": { "measures": ["count"] }, "format": "json" } +TO { "cube": "orders", "measures": ["count"], "dimensions": [...], "where": {...} } +``` + +`cube` + `measures` are required at the top level; `dimensions` / `where` / +`timeDimensions` / `order` / `limit` / `offset` / `timezone` sit beside them. +The schema is `.strict()`; `query` and `format` are tombstoned (`retiredKey`) +so both `tsc` and the parse answer with this exact migration. `format` was +never implemented (every response is the JSON envelope) — for CSV/XLSX use the +export surface. The removal is registered as two step-17 semantic migrations +(`analytics-query-request-envelope-retired`, +`analytics-query-request-format-retired`) — it is an HTTP-wire change with no +stored metadata to rewrite. + +**Runtime.** `POST /api/v1/analytics/query` and `/analytics/sql` now validate +the body against that schema AT THE ENTRY and answer +**400 `VALIDATION_FAILED`** with per-field details — including the envelope +prescription above, and a bespoke hint that `filters` is not a contract field +(the filter field is `where`, the same canonical FilterCondition `find()` +takes). Previously a malformed body reached the engine and failed as a 500 SQL +syntax error, or had its off-contract filter key silently ignored. A valid +body is forwarded to the analytics service byte-identical (validation only — +parsing would inject the schema's `timezone: 'UTC'` default and override +org-timezone resolution). An uninstalled analytics capability still answers +404 before any body inspection (#3891). diff --git a/content/docs/references/api/analytics.mdx b/content/docs/references/api/analytics.mdx index ef4c20db93..8fb317f218 100644 --- a/content/docs/references/api/analytics.mdx +++ b/content/docs/references/api/analytics.mdx @@ -58,9 +58,17 @@ const result = AnalyticsEndpoint.parse(data); | Property | Type | Required | Description | | :--- | :--- | :--- | :--- | -| **query** | `{ cube?: string; measures: string[]; dimensions?: string[]; where?: any; … }` | ✅ | The analytic query definition | | **cube** | `string` | ✅ | Target cube name | -| **format** | `Enum<'json' \| 'csv' \| 'xlsx'>` | ✅ | Response format | +| **measures** | `string[]` | ✅ | List of metrics to calculate | +| **dimensions** | `string[]` | optional | List of dimensions to group by | +| **where** | `any` | optional | Filtering criteria (canonical Query DSL FilterCondition) | +| **timeDimensions** | `{ dimension: string; granularity?: Enum<'second' \| 'minute' \| 'hour' \| 'day' \| 'week' \| 'month' \| 'quarter' \| 'year'>; dateRange?: string \| string[] }[]` | optional | | +| **order** | `Record>` | optional | | +| **limit** | `number` | optional | | +| **offset** | `number` | optional | | +| **timezone** | `string` | ✅ | | +| **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? }`. | +| **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. | --- diff --git a/docs/protocol-upgrade-guide.md b/docs/protocol-upgrade-guide.md index de1fd7fa51..3552522ac9 100644 --- a/docs/protocol-upgrade-guide.md +++ b/docs/protocol-upgrade-guide.md @@ -126,6 +126,8 @@ Beyond those spec-surface removals, it graduates the seven flow-node config key 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. +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. + ### Mechanical (applied for you) | Conversion | Surface | Change | Load window | @@ -140,6 +142,15 @@ And it removes the RLS-policy key `priority` (#3896 security audit): promised "c | `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 | | `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 | +### Semantic (delegated to you, with acceptance criteria) + +- **`analytics-query-request-envelope-retired`** — `api.analyticsQueryRequest.query` → bare AnalyticsQuery body (top-level cube/measures/dimensions/where/...) + - 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. + - 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. +- **`analytics-query-request-format-retired`** — `api.analyticsQueryRequest.format` → (removed — responses are always the JSON envelope; use the export surface for CSV/XLSX) + - 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. + - Done when: No /analytics/query or /analytics/sql call sends `format`; exports go through the export surface. + --- *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`.* diff --git a/packages/runtime/src/dispatcher-plugin.error-envelope.test.ts b/packages/runtime/src/dispatcher-plugin.error-envelope.test.ts index bba27aad76..4790bc3b8a 100644 --- a/packages/runtime/src/dispatcher-plugin.error-envelope.test.ts +++ b/packages/runtime/src/dispatcher-plugin.error-envelope.test.ts @@ -84,7 +84,9 @@ async function postAnalyticsQuery(err: unknown) { expect(handler, 'POST /api/v1/analytics/query must be mounted').toBeTypeOf('function'); const res = makeRes(); - await handler({ body: { cube: 'x', query: {} }, query: {} }, res); + // [#3878] Body must pass entry validation so the SERVICE's thrown error — + // the thing under test — is what reaches the exit, not an entry 400. + await handler({ body: { cube: 'x', measures: ['count'] }, query: {} }, res); return res; } diff --git a/packages/runtime/src/dispatcher-validation-error.real.test.ts b/packages/runtime/src/dispatcher-validation-error.real.test.ts index f124df55c7..39be9f6930 100644 --- a/packages/runtime/src/dispatcher-validation-error.real.test.ts +++ b/packages/runtime/src/dispatcher-validation-error.real.test.ts @@ -20,7 +20,7 @@ * are the tests that fail if the contract moves. */ -import { describe, it, expect } from 'vitest'; +import { describe, it, expect, vi } from 'vitest'; import { ValidationError } from '@objectstack/objectql'; import { HttpDispatcher } from './http-dispatcher.js'; @@ -100,7 +100,9 @@ async function analyticsQuery(thrown: unknown) { header() { return res; }, json(b: any) { res.body = b; return res; }, }; - await handlers['POST /api/v1/analytics/query']({ body: { cube: 'x', query: {} }, query: {} }, res); + // [#3878] Body must pass entry validation so the SERVICE's thrown error — + // the thing under test — is what reaches the exit, not an entry 400. + await handlers['POST /api/v1/analytics/query']({ body: { cube: 'x', measures: ['count'] }, query: {} }, res); return res; } @@ -138,3 +140,58 @@ describe('#3918 — both exits serve the real ValidationError as 400 + fields[]' expect(res.body.error.details.fields).toEqual([]); }); }); + +// --------------------------------------------------------------------------- + +/** [#3878] Post `body` through the real route handler; the service never throws. */ +async function postAnalyticsBody(body: unknown) { + const handlers: Record any> = {}; + const rec = (verb: string) => (path: string, h: any) => { handlers[`${verb} ${path}`] = h; }; + const server = { + get: rec('GET'), post: rec('POST'), put: rec('PUT'), + delete: rec('DELETE'), patch: rec('PATCH'), + }; + const query = vi.fn(async () => ({ rows: [] })); + const analytics = { query, getMeta: async () => ({ cubes: [] }), generateSql: async () => ({ sql: null }) }; + const kernel = { + getService: (n: string) => (n === 'analytics' ? analytics : undefined), + getServiceAsync: async (n: string) => (n === 'analytics' ? analytics : undefined), + }; + const plugin = createDispatcherPlugin({ prefix: '/api/v1', securityHeaders: false }); + await plugin.start?.({ + getKernel: () => kernel, + getService: (n: string) => (n === 'http.server' ? server : undefined), + environmentId: undefined, + logger: { info() {}, warn() {}, error() {}, debug() {} }, + hook: () => {}, on: () => {}, + } as any); + + const res: any = { + statusCode: undefined, body: undefined, + status(c: number) { res.statusCode = c; return res; }, + header() { return res; }, + json(b: any) { res.body = b; return res; }, + }; + await handlers['POST /api/v1/analytics/query']({ body, query: {} }, res); + return { res, query }; +} + +describe('#3878 — entry validation reaches the wire as a 400, service untouched', () => { + it('the retired envelope answers 400 with the tombstone prescription', async () => { + const { res, query } = await postAnalyticsBody({ cube: 'x', query: { measures: ['count'] } }); + + expect(res.statusCode).toBe(400); + expect(res.body.error.code).toBe('VALIDATION_FAILED'); + expect(res.body.error.message).toContain('top level'); + expect(query).not.toHaveBeenCalled(); + // A caller mistake, not a server fault: the reporter side-channel stays clear. + expect(res.__obsRecordedError).toBeUndefined(); + }); + + it('a valid bare body passes through and answers 200', async () => { + const { res, query } = await postAnalyticsBody({ cube: 'x', measures: ['count'] }); + + expect(res.statusCode).toBe(200); + expect(query).toHaveBeenCalledOnce(); + }); +}); diff --git a/packages/runtime/src/dispatcher-validation-error.test.ts b/packages/runtime/src/dispatcher-validation-error.test.ts index 4df7e616fa..56a4d0bca3 100644 --- a/packages/runtime/src/dispatcher-validation-error.test.ts +++ b/packages/runtime/src/dispatcher-validation-error.test.ts @@ -239,7 +239,9 @@ async function postAnalyticsQuery(err: unknown) { expect(handler, 'POST /api/v1/analytics/query must be mounted').toBeTypeOf('function'); const res = makeRes(); - await handler({ body: { cube: 'x', query: {} }, query: {} }, res); + // [#3878] Body must pass entry validation so the SERVICE's thrown error — + // the thing under test — is what reaches the exit, not an entry 400. + await handler({ body: { cube: 'x', measures: ['count'] }, query: {} }, res); return res; } diff --git a/packages/runtime/src/domain-handler-registry.test.ts b/packages/runtime/src/domain-handler-registry.test.ts index 87c0c85401..628aceb6ce 100644 --- a/packages/runtime/src/domain-handler-registry.test.ts +++ b/packages/runtime/src/domain-handler-registry.test.ts @@ -135,7 +135,7 @@ describe('HttpDispatcher domain registry (D11 step ③)', () => { analyticsQuery: vi.fn().mockResolvedValue({ rows: [{ n: 1 }] }), }; const result = await makeDispatcher({ analytics }).dispatch( - 'POST', '/analytics/query', { metric: 'count' }, {}, {} as any, + 'POST', '/analytics/query', { cube: 'orders', measures: ['count'] }, {}, {} as any, ); expect(result.handled).toBe(true); // The bridge consulted the service (whichever entry point it uses). diff --git a/packages/runtime/src/domains/analytics.ts b/packages/runtime/src/domains/analytics.ts index 31e5a81fb8..58e877d04f 100644 --- a/packages/runtime/src/domains/analytics.ts +++ b/packages/runtime/src/domains/analytics.ts @@ -11,9 +11,65 @@ */ import { CoreServiceName } from '@objectstack/spec/system'; +import { AnalyticsQueryRequestSchema } from '@objectstack/spec/api'; import type { HttpProtocolContext, HttpDispatcherResult } from '../http-dispatcher.js'; import type { DomainHandlerDeps, DomainRoute } from '../domain-handler-registry.js'; +/** + * [#3878] The duck-typed validation-failure shape both dispatcher error exits + * already map to 400 `VALIDATION_FAILED` + `details.fields[]` (see + * `validation-failure.ts`, #3918) — thrown here so entry validation needs no + * new error channel and no dependency on objectql's `ValidationError` class. + */ +function validationFailure(message: string, fields: unknown[]): Error { + const err = new Error(message) as Error & { code: string; fields: unknown[] }; + err.name = 'ValidationError'; + err.code = 'VALIDATION_FAILED'; + err.fields = fields; + return err; +} + +/** + * [#3878] Reject a malformed `AnalyticsQuery` body AT THE ENTRY with a 400 + * naming what is wrong, instead of letting it reach the engine — where a + * shapeless body used to infer a column-less cube and die as an SQL syntax + * error deep in the driver (`SELECT FROM …`), or worse, have its off-contract + * filter key silently ignored. + * + * The retired `{ cube, query: {...} }` envelope (#3891 shim dialect) needs no + * special case here: the schema tombstones `query`/`format` (`retiredKey`), + * so the Zod issue itself carries the migration prescription. Only `filters` — + * never a declared key, so `.strict()` would answer a generic + * "unrecognized key" — gets a bespoke hint at the contract field `where`. + * + * Validation only — the ORIGINAL body is forwarded to the service untouched. + * (Parsing would inject the schema's `timezone: 'UTC'` default and silently + * override the engine's org-timezone resolution, #1982/#2018.) + */ +function assertAnalyticsQueryBody(body: unknown): void { + if (body && typeof body === 'object' && !Array.isArray(body)) { + const b = body as Record; + if ('filters' in b && !('where' in b)) { + throw validationFailure( + '`filters` is not an AnalyticsQuery field — use `where` (canonical Query DSL FilterCondition, the same shape find() takes).', + [{ field: 'filters', code: 'unrecognized_keys', message: 'use `where` instead of `filters`' }], + ); + } + } + const parsed = AnalyticsQueryRequestSchema.safeParse(body); + if (!parsed.success) { + const fields = parsed.error.issues.map((issue) => ({ + field: issue.path.length > 0 ? issue.path.join('.') : '(body)', + code: issue.code, + message: issue.message, + })); + throw validationFailure( + `Invalid AnalyticsQuery body: ${fields.map((f) => `${f.field}: ${f.message}`).join('; ')}`, + fields, + ); + } +} + export function createAnalyticsDomain(deps: DomainHandlerDeps): DomainRoute { return { prefix: '/analytics', @@ -39,6 +95,10 @@ export async function handleAnalyticsRequest( // POST /analytics/query if (subPath === 'query' && m === 'POST') { + // [#3878] Entry validation AFTER the service check on purpose: an + // uninstalled analytics capability answers 404 (the honest "install + // service-analytics" signal, #3891) regardless of body shape. + assertAnalyticsQueryBody(body); // [#2852] Pass the request's execution context so the analytics // service scopes each object by its per-object read filter (tenant + // RLS). Without it, `getReadScope(object, undefined)` returned no @@ -60,6 +120,8 @@ export async function handleAnalyticsRequest( // POST /analytics/sql (Dry-run or debug) if (subPath === 'sql' && m === 'POST') { + // [#3878] Same body contract as /query — validated the same way. + assertAnalyticsQueryBody(body); // [#2852] Scope the generated SQL to the caller too, so a preview // reflects the same per-object read filter the real query applies. const result = await analyticsService.generateSql(body, context?.executionContext); diff --git a/packages/runtime/src/http-dispatcher.test.ts b/packages/runtime/src/http-dispatcher.test.ts index 7e2e02f192..ffc2b77ccb 100644 --- a/packages/runtime/src/http-dispatcher.test.ts +++ b/packages/runtime/src/http-dispatcher.test.ts @@ -518,7 +518,7 @@ describe('HttpDispatcher', () => { return null; }); - const result = await dispatcher.handleAnalytics('query', 'POST', { sql: 'SELECT 1' }, { request: {} }); + const result = await dispatcher.handleAnalytics('query', 'POST', { cube: 't1', measures: ['count'] }, { request: {} }); expect(result.handled).toBe(true); expect(result.response?.status).toBe(200); expect(mockAnalytics.query).toHaveBeenCalled(); @@ -537,11 +537,16 @@ describe('HttpDispatcher', () => { ); const ec = { userId: 'u1', positions: [], permissions: [], tenantId: 'org-1' }; - await dispatcher.handleAnalytics('query', 'POST', { cube: 'leads' }, { request: {}, executionContext: ec } as any); - expect(mockAnalytics.query).toHaveBeenCalledWith({ cube: 'leads' }, ec); + // [#3878] The ORIGINAL body must be forwarded (no parse-output + // substitution): a parsed body would carry the schema's + // `timezone: 'UTC'` default and override org-timezone resolution. + const body = { cube: 'leads', measures: ['count'] }; + await dispatcher.handleAnalytics('query', 'POST', body, { request: {}, executionContext: ec } as any); + expect(mockAnalytics.query).toHaveBeenCalledWith(body, ec); + expect(mockAnalytics.query.mock.calls[0][0]).toBe(body); - await dispatcher.handleAnalytics('sql', 'POST', { cube: 'leads' }, { request: {}, executionContext: ec } as any); - expect(mockAnalytics.generateSql).toHaveBeenCalledWith({ cube: 'leads' }, ec); + await dispatcher.handleAnalytics('sql', 'POST', body, { request: {}, executionContext: ec } as any); + expect(mockAnalytics.generateSql).toHaveBeenCalledWith(body, ec); }); it('should handle POST /analytics/sql with async service', async () => { @@ -550,7 +555,7 @@ describe('HttpDispatcher', () => { }; (kernel as any).getService = vi.fn().mockResolvedValue(mockAnalytics); - const result = await dispatcher.handleAnalytics('sql', 'POST', { object: 'test' }, { request: {} }); + const result = await dispatcher.handleAnalytics('sql', 'POST', { cube: 'test', measures: ['count'] }, { request: {} }); expect(result.handled).toBe(true); expect(result.response?.status).toBe(200); expect(mockAnalytics.generateSql).toHaveBeenCalled(); @@ -602,6 +607,79 @@ describe('HttpDispatcher', () => { const result = await dispatcher.handleAnalytics('unknown', 'POST', {}, { request: {} }); expect(result.handled).toBe(false); }); + + // [#3878] Entry validation: a malformed body raises the duck-typed + // VALIDATION_FAILED shape BEFORE the service runs — previously it + // reached the engine, inferred a column-less cube, and died as an + // SQL syntax error (or had its off-contract filter silently + // dropped). The domain throws through (same contract as service + // errors, see 'should propagate analytics query error'); the HTTP + // bridge maps the shape to a 400 envelope — pinned end-to-end in + // `dispatcher-validation-error.real.test.ts`. + describe('AnalyticsQuery body validation (#3878)', () => { + const service = () => { + const mockAnalytics = { + query: vi.fn().mockResolvedValue({ rows: [] }), + generateSql: vi.fn().mockResolvedValue({ sql: 'SELECT 1', params: [] }), + }; + (kernel as any).getService = vi.fn().mockResolvedValue(mockAnalytics); + return mockAnalytics; + }; + + it('rejects the retired {cube, query:{...}} envelope with the tombstone prescription', async () => { + const mockAnalytics = service(); + await expect(dispatcher.dispatch( + 'POST', '/analytics/query', { cube: 'x', query: { measures: ['count'] } }, {}, { request: {} }, + )).rejects.toMatchObject({ + name: 'ValidationError', + code: 'VALIDATION_FAILED', + message: expect.stringContaining('top level'), + }); + expect(mockAnalytics.query).not.toHaveBeenCalled(); + }); + + it('rejects a `filters` key, pointing at the contract field `where`', async () => { + const mockAnalytics = service(); + await expect(dispatcher.dispatch( + 'POST', '/analytics/query', + { cube: 'x', measures: ['count'], filters: [{ member: 'status', operator: 'equals', values: ['active'] }] }, + {}, { request: {} }, + )).rejects.toMatchObject({ + code: 'VALIDATION_FAILED', + message: expect.stringContaining('`where`'), + }); + expect(mockAnalytics.query).not.toHaveBeenCalled(); + }); + + it('rejects a body with no measures, naming the missing field', async () => { + const mockAnalytics = service(); + await expect(dispatcher.dispatch( + 'POST', '/analytics/query', { cube: 'x' }, {}, { request: {} }, + )).rejects.toMatchObject({ + code: 'VALIDATION_FAILED', + fields: expect.arrayContaining([expect.objectContaining({ field: 'measures' })]), + }); + expect(mockAnalytics.query).not.toHaveBeenCalled(); + }); + + it('validates /analytics/sql with the same contract', async () => { + const mockAnalytics = service(); + await expect(dispatcher.dispatch( + 'POST', '/analytics/sql', { cube: 'x', query: {} }, {}, { request: {} }, + )).rejects.toMatchObject({ code: 'VALIDATION_FAILED' }); + expect(mockAnalytics.generateSql).not.toHaveBeenCalled(); + }); + + it('a valid bare body still reaches the service untouched', async () => { + const mockAnalytics = service(); + const body = { cube: 'x', measures: ['count'], where: { status: 'active' } }; + const result: any = await dispatcher.dispatch( + 'POST', '/analytics/query', body, {}, { request: {} }, + ); + expect(result.response?.status).toBe(200); + expect(mockAnalytics.query.mock.calls[0][0]).toBe(body); + }); + }); }); // ADR-0030: the /api/v1/notifications surface, resolved from the @@ -942,7 +1020,7 @@ describe('HttpDispatcher', () => { }; (kernel as any).services = new Map([['analytics', syncAnalytics]]); - const result = await dispatcher.handleAnalytics('query', 'POST', {}, { request: {} }); + const result = await dispatcher.handleAnalytics('query', 'POST', { cube: 't', measures: ['count'] }, { request: {} }); expect(result.handled).toBe(true); expect(syncAnalytics.query).toHaveBeenCalled(); }); @@ -973,7 +1051,7 @@ describe('HttpDispatcher', () => { throw new Error("Service 'analytics' is async - use await"); }); - const result = await dispatcher.handleAnalytics('query', 'POST', {}, { request: {} }); + const result = await dispatcher.handleAnalytics('query', 'POST', { cube: 't', measures: ['count'] }, { request: {} }); expect(result.handled).toBe(true); expect(asyncAnalytics.query).toHaveBeenCalled(); expect((kernel as any).getServiceAsync).toHaveBeenCalledWith('analytics'); @@ -1043,7 +1121,7 @@ describe('HttpDispatcher', () => { }; (kernel as any).services = new Map([['analytics', syncAnalytics]]); - const result = await dispatcher.handleAnalytics('query', 'POST', {}, { request: {} }); + const result = await dispatcher.handleAnalytics('query', 'POST', { cube: 't', measures: ['count'] }, { request: {} }); expect(result.handled).toBe(true); expect(syncAnalytics.query).toHaveBeenCalled(); }); @@ -1055,7 +1133,7 @@ describe('HttpDispatcher', () => { }; (kernel as any).services = new Map([['analytics', syncAnalytics]]); - const result = await dispatcher.handleAnalytics('query', 'POST', {}, { request: {} }); + const result = await dispatcher.handleAnalytics('query', 'POST', { cube: 't', measures: ['count'] }, { request: {} }); expect(result.handled).toBe(true); expect(syncAnalytics.query).toHaveBeenCalled(); }); @@ -1159,7 +1237,7 @@ describe('HttpDispatcher', () => { (kernel as any).getService = vi.fn().mockResolvedValue(badAnalytics); await expect( - dispatcher.handleAnalytics('query', 'POST', {}, { request: {} }) + dispatcher.handleAnalytics('query', 'POST', { cube: 't', measures: ['count'] }, { request: {} }) ).rejects.toThrow('Query timeout'); }); diff --git a/packages/spec/authorable-surface.json b/packages/spec/authorable-surface.json index 542ee3bfec..9a886fa7f7 100644 --- a/packages/spec/authorable-surface.json +++ b/packages/spec/authorable-surface.json @@ -448,8 +448,16 @@ "api/AnalyticsMetadataResponse:meta", "api/AnalyticsMetadataResponse:success", "api/AnalyticsQueryRequest:cube", - "api/AnalyticsQueryRequest:format", - "api/AnalyticsQueryRequest:query", + "api/AnalyticsQueryRequest:dimensions", + "api/AnalyticsQueryRequest:format [RETIRED]", + "api/AnalyticsQueryRequest:limit", + "api/AnalyticsQueryRequest:measures", + "api/AnalyticsQueryRequest:offset", + "api/AnalyticsQueryRequest:order", + "api/AnalyticsQueryRequest:query [RETIRED]", + "api/AnalyticsQueryRequest:timeDimensions", + "api/AnalyticsQueryRequest:timezone", + "api/AnalyticsQueryRequest:where", "api/AnalyticsResultResponse:data", "api/AnalyticsResultResponse:error", "api/AnalyticsResultResponse:meta", diff --git a/packages/spec/spec-changes.json b/packages/spec/spec-changes.json index 212c8e650d..c7f38c4d08 100644 --- a/packages/spec/spec-changes.json +++ b/packages/spec/spec-changes.json @@ -193,6 +193,20 @@ "migrationId": "dashboard-widget-strict-unknown-keys", "toMajor": 16, "rationale": "The `.strict()` flip turns a previously silently-stripped unknown key into a parse error. There is no mapping target for an arbitrary unknown key — auto-deleting it would be exactly the silent data loss ADR-0078 bans — so each occurrence needs the author to decide: bind a `dataset` and select `dimensions`/`values`, move a renderer setting under `options`, or delete the dead key." + }, + { + "surface": "api.analyticsQueryRequest.query", + "replacement": "bare AnalyticsQuery body (top-level cube/measures/dimensions/where/...)", + "migrationId": "analytics-query-request-envelope-retired", + "toMajor": 17, + "rationale": "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." + }, + { + "surface": "api.analyticsQueryRequest.format", + "replacement": "(removed — responses are always the JSON envelope; use the export surface for CSV/XLSX)", + "migrationId": "analytics-query-request-format-retired", + "toMajor": 17, + "rationale": "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." } ], "removed": [] @@ -445,7 +459,22 @@ "toMajor": 17 } ], - "migrated": [], + "migrated": [ + { + "surface": "api.analyticsQueryRequest.query", + "replacement": "bare AnalyticsQuery body (top-level cube/measures/dimensions/where/...)", + "migrationId": "analytics-query-request-envelope-retired", + "toMajor": 17, + "rationale": "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." + }, + { + "surface": "api.analyticsQueryRequest.format", + "replacement": "(removed — responses are always the JSON envelope; use the export surface for CSV/XLSX)", + "migrationId": "analytics-query-request-format-retired", + "toMajor": 17, + "rationale": "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." + } + ], "removed": [] } ] diff --git a/packages/spec/src/api/analytics.test.ts b/packages/spec/src/api/analytics.test.ts index 45526ba88e..e7660206fe 100644 --- a/packages/spec/src/api/analytics.test.ts +++ b/packages/spec/src/api/analytics.test.ts @@ -24,70 +24,72 @@ describe('AnalyticsEndpoint', () => { }); }); -describe('AnalyticsQueryRequestSchema', () => { - it('should accept valid query request with defaults', () => { +describe('AnalyticsQueryRequestSchema — the BARE AnalyticsQuery shape (#3878)', () => { + it('should accept a minimal bare query', () => { const req = AnalyticsQueryRequestSchema.parse({ - query: { - measures: ['total_revenue'], - }, cube: 'orders', + measures: ['total_revenue'], }); expect(req.cube).toBe('orders'); - expect(req.format).toBe('json'); - expect(req.query.measures).toEqual(['total_revenue']); - }); - - it('should accept query with explicit format', () => { - const req = AnalyticsQueryRequestSchema.parse({ - query: { - measures: ['count'], - dimensions: ['category'], - }, - cube: 'products', - format: 'csv', - }); - expect(req.format).toBe('csv'); + expect(req.measures).toEqual(['total_revenue']); }); - it('should accept query with where (canonical) and time dimensions', () => { + it('should accept the full bare shape: where (canonical), timeDimensions, order, limit', () => { const req = AnalyticsQueryRequestSchema.parse({ - query: { - measures: ['total_revenue'], - dimensions: ['product_category'], - where: { status: 'active' }, - timeDimensions: [ - { dimension: 'created_at', granularity: 'month', dateRange: 'Last 7 days' }, - ], - order: { total_revenue: 'desc' }, - limit: 100, - }, cube: 'sales', - format: 'xlsx', + measures: ['total_revenue'], + dimensions: ['product_category'], + where: { status: 'active', stage: { $nin: ['lost'] } }, + timeDimensions: [ + { dimension: 'created_at', granularity: 'month', dateRange: 'Last 7 days' }, + ], + order: { total_revenue: 'desc' }, + limit: 100, + offset: 10, + timezone: 'Asia/Shanghai', }); - expect(req.query.where).toEqual({ status: 'active' }); - expect(req.query.timeDimensions).toHaveLength(1); + expect(req.where).toEqual({ status: 'active', stage: { $nin: ['lost'] } }); + expect(req.timeDimensions).toHaveLength(1); + expect(req.timezone).toBe('Asia/Shanghai'); }); it('should reject missing cube', () => { + expect(() => + AnalyticsQueryRequestSchema.parse({ measures: ['x'] }) + ).toThrow(); + }); + + it('should reject missing measures', () => { + expect(() => + AnalyticsQueryRequestSchema.parse({ cube: 'test' }) + ).toThrow(); + }); + + it('should reject the retired {cube, query: {...}} envelope (#3891 shim dialect)', () => { expect(() => AnalyticsQueryRequestSchema.parse({ - query: { measures: ['x'] }, + cube: 'orders', + query: { measures: ['total_revenue'] }, }) ).toThrow(); }); - it('should reject missing query', () => { + it('should reject the non-contract `filters` key (the field is `where`)', () => { expect(() => - AnalyticsQueryRequestSchema.parse({ cube: 'test' }) + AnalyticsQueryRequestSchema.parse({ + cube: 'orders', + measures: ['count'], + filters: [{ member: 'status', operator: 'equals', values: ['active'] }], + }) ).toThrow(); }); - it('should reject invalid format', () => { + it('should reject the retired unimplemented `format` key', () => { expect(() => AnalyticsQueryRequestSchema.parse({ - query: { measures: ['x'] }, - cube: 'test', - format: 'xml', + cube: 'orders', + measures: ['count'], + format: 'csv', }) ).toThrow(); }); diff --git a/packages/spec/src/api/analytics.zod.ts b/packages/spec/src/api/analytics.zod.ts index 71d00bff9c..f4e64a97e9 100644 --- a/packages/spec/src/api/analytics.zod.ts +++ b/packages/spec/src/api/analytics.zod.ts @@ -3,6 +3,7 @@ import { z } from 'zod'; import { AnalyticsQuerySchema, CubeSchema } from '../data/analytics.zod'; import { BaseResponseSchema } from './contract.zod'; +import { retiredKey } from '../shared/retired-key'; /** * Analytics API Protocol @@ -27,13 +28,39 @@ export const AnalyticsEndpoint = z.enum([ // ========================================== /** - * Query Request Body + * Query Request Body — the BARE `AnalyticsQuery` shape (#3878). + * + * The body IS the `AnalyticsQuery`: `cube` + `measures` at the top level with + * the optional `dimensions` / `where` / `timeDimensions` / `order` / `limit` / + * `offset` / `timezone` fields beside them. This is what + * `AnalyticsService.query` (the domain's one implementation, + * `@objectstack/service-analytics`) consumes and what every real caller + * (objectui dashboards, `client.analytics.query`) sends. + * + * History: this schema used to describe a `{ cube, query: {...}, format }` + * ENVELOPE — the dialect of the retired degraded shim (#3891), which the real + * engine never understood (an envelope body inferred a column-less cube and + * died as an SQL syntax error instead of a shape error). The envelope is + * rejected now — `.strict()` — and the dispatcher's `/analytics` entry answers + * 400 with a migration hint. The unimplemented `format` field went with it + * (declared ≠ enforced: every response is the JSON envelope). */ -export const AnalyticsQueryRequestSchema = lazySchema(() => z.object({ - query: AnalyticsQuerySchema.describe('The analytic query definition'), - cube: z.string().describe('Target cube name'), - format: z.enum(['json', 'csv', 'xlsx']).default('json').describe('Response format'), -})); +export const AnalyticsQueryRequestSchema = lazySchema(() => + AnalyticsQuerySchema.extend({ + cube: z.string().describe('Target cube name'), + query: retiredKey( + '`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? }.', + ), + format: retiredKey( + '`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.', + ), + }).strict() +); /** * Query Response (JSON) diff --git a/packages/spec/src/migrations/registry.ts b/packages/spec/src/migrations/registry.ts index c5b841fd97..fdfcb24293 100644 --- a/packages/spec/src/migrations/registry.ts +++ b/packages/spec/src/migrations/registry.ts @@ -385,7 +385,17 @@ const step17: MigrationStep = { '(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.', + 'with or without it; the schema tombstones the key with the same prescription.\n\n' + + '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.', conversionIds: [ 'action-execute-to-target', 'field-conditionalRequired-to-requiredWhen', @@ -397,7 +407,33 @@ const step17: MigrationStep = { 'flow-node-script-config-aliases', 'permission-rls-priority-removed', ], - semantic: [], + semantic: [ + { + id: 'analytics-query-request-envelope-retired', + surface: 'api.analyticsQueryRequest.query', + replacement: 'bare AnalyticsQuery body (top-level cube/measures/dimensions/where/...)', + reason: + '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.', + acceptanceCriteria: + 'Every /analytics/query and /analytics/sql call sends the bare AnalyticsQuery shape and ' + + 'succeeds; no request answers 400 VALIDATION_FAILED with the envelope prescription.', + }, + { + id: 'analytics-query-request-format-retired', + surface: 'api.analyticsQueryRequest.format', + replacement: '(removed — responses are always the JSON envelope; use the export surface for CSV/XLSX)', + reason: + '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.', + acceptanceCriteria: + 'No /analytics/query or /analytics/sql call sends `format`; exports go through the ' + + 'export surface.', + }, + ], }; /** All migration steps, keyed by the major they migrate into. */