diff --git a/.changeset/analytics-bridge-engine-aggregate-vocabulary.md b/.changeset/analytics-bridge-engine-aggregate-vocabulary.md new file mode 100644 index 0000000000..42b1669b3b --- /dev/null +++ b/.changeset/analytics-bridge-engine-aggregate-vocabulary.md @@ -0,0 +1,33 @@ +--- +"@objectstack/service-analytics": patch +--- + +refactor(service-analytics): derive the analytics auto-bridge's engine view from the declared contracts (#11833) + +`plugin.ts` named the data engine through a consumer-local structural +`DataEngineLike` — the second of the two sites #11833 records, after the +datasource half that landed as PR #12011. It is now derived from the declared +contracts: `IDataEngine.aggregate` / `execute?` / +`resolveEffectiveDatasource?` / `getDriverForObject?` and +`IObjectQLEngine.getObject`. Optionality is preserved exactly — `aggregate` +required, everything else `Partial<>` — because these probes are the plugin's +graceful-degradation seam. + +**Why this is `patch` and not a type-only no-op.** Four of the five members +substitute with no behaviour change. The fifth does not: the deleted structural +type declared `aggregations[].function` as `string`, while the contract +declares the six-value `AggregationFunction`. The bridge therefore forwarded +whatever method string reached it. That forward is now parsed with the spec's +own enum, so a method the engine contract does not declare is refused at the +bridge — loudly, naming the aggregation and the legal vocabulary — instead of +reaching the engine, where `driver-sql` blamed a `function` key the author +never wrote and the in-memory evaluator answered `null` for every bucket under +the author's own measure name. + +No authored analytics can trigger the new refusal: the one reachable producer +of a non-aggregate method — a custom-SQL measure (`AggregationMetricType` +`number` / `string` / `boolean`) — is already refused earlier, caller-facing, +by `ObjectQLStrategy.resolveMeasureAggregation` (#12209). What is left is host +drift (a cube object registered without meeting `CubeSchema`), which is why +the new refusal is a bare `Error` in the undeclared-500 tier rather than an +ADR-0112 400 that would blame the caller for something they did not write. diff --git a/packages/services/service-analytics/src/__tests__/aggregate-bridge-function-vocabulary.test.ts b/packages/services/service-analytics/src/__tests__/aggregate-bridge-function-vocabulary.test.ts new file mode 100644 index 0000000000..48d3e4bb0a --- /dev/null +++ b/packages/services/service-analytics/src/__tests__/aggregate-bridge-function-vocabulary.test.ts @@ -0,0 +1,139 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#11833] The plugin's aggregate auto-bridge speaks the ENGINE's aggregate + * vocabulary, and refuses anything else instead of forwarding it. + * + * `plugin.ts` used to name the engine through a consumer-local structural + * `DataEngineLike` that declared `aggregations[].function` as `string`. The + * declared contract (`IDataEngine.aggregate` → + * `EngineAggregateOptions.aggregations[].function`) is the SIX-value + * `AggregationFunction`. Nothing compiled the two against each other, so the + * bridge forwarded whatever string reached it — and the engine then failed in + * the two ways #12209 documents: `driver-sql` blaming a `function` key the + * author never wrote, or the in-memory evaluator answering `null` for every + * bucket under the author's own measure name (the #4157 class). + * + * Deriving the local view from the contract makes the forward a compile error; + * these cases pin the RUNTIME half of that repair. + * + * ## Why this refusal is deliberately NOT in the ADR-0112 envelope + * + * The reachable producer of a non-aggregate method — a custom-SQL measure + * (`AggregationMetricType` `number`/`string`/`boolean`) — is refused earlier + * and caller-facing by `ObjectQLStrategy.resolveMeasureAggregation` (#12209, + * `INVALID_FIELD` / 400). Anything still arriving at the bridge is host drift + * (an unparsed cube object, our own drift), which `dataset-refusal.ts`'s module + * header assigns to the bare-`Error`, undeclared-500 tier — the same tier it + * assigns to `native-sql-strategy.ts`'s "measure … has unrecognised type". The + * absence of a `code` is therefore asserted, not overlooked: enveloping this as + * a 400 would tell the author to fix something they did not write. + */ + +import { describe, it, expect, vi } from 'vitest'; +import { AggregationFunction } from '@objectstack/spec/data'; +import type { Cube } from '@objectstack/spec/data'; +import type { AnalyticsService } from '../analytics-service.js'; +import { AnalyticsServicePlugin } from '../plugin.js'; + +type EngineAggregateCall = { + object: string; + aggregations?: Array<{ function: string; field?: string; alias: string }>; +}; + +/** + * Minimal `'data'` service: the one member the aggregate bridge requires. + * `getObject` answers the schema so the source-field gates can stand. + */ +function fakeEngine(calls: EngineAggregateCall[], fields: Record) { + return { + getObject: (name: string) => (name === 'opportunity' ? { fields } : undefined), + aggregate: async (object: string, options: EngineAggregateCall) => { + calls.push({ object, aggregations: options.aggregations }); + return [{ region: 'west', total: 1 }]; + }, + }; +} + +function fakePluginContext(services: Record) { + const registered: Record = {}; + const warn = vi.fn(); + return { + registered, + ctx: { + getService: (name: string) => services[name] ?? registered[name], + registerService: (name: string, svc: unknown) => { registered[name] = svc; }, + replaceService: (name: string, svc: unknown) => { registered[name] = svc; }, + logger: { info() {}, warn, error() {}, debug() {} }, + }, + }; +} + +const objectqlOnly = () => ({ nativeSql: false, objectqlAggregate: true, inMemory: false }); + +/** + * A cube carrying one measure of the given metric type. + * + * `type` is widened past `AggregationMetricType` on purpose: the drift case + * below needs a cube that never met `CubeSchema`'s parse, which is exactly the + * arrival path the refusal is tiered for. A parsed cube cannot carry it. + */ +const cubeWithMeasureType = (type: string): Cube => ({ + name: 'sales', + title: 'Sales', + sql: 'opportunity', + measures: { revenue: { name: 'revenue', label: 'Revenue', type, sql: 'amount' } as Cube['measures'][string] }, + dimensions: { region: { name: 'region', label: 'Region', type: 'string', sql: 'region' } }, + public: false, +}); + +async function analyticsVia(engine: unknown, cube: Cube): Promise { + const { ctx, registered } = fakePluginContext({ data: engine }); + await new AnalyticsServicePlugin({ + cubes: [cube], + queryCapabilities: objectqlOnly, + }).init(ctx as never); + return registered.analytics as AnalyticsService; +} + +const selection = { cube: 'sales', dimensions: ['region'], measures: ['revenue'] }; +const schema = { region: { type: 'text' }, amount: { type: 'number' } }; + +describe('[#11833] the aggregate auto-bridge speaks the engine contract vocabulary', () => { + it('forwards a declared aggregate function through to the engine', async () => { + // Positive control: without this, the refusal case below could pass because + // NOTHING reaches the engine, for reasons that have nothing to do with the + // vocabulary. + const calls: EngineAggregateCall[] = []; + const service = await analyticsVia(fakeEngine(calls, schema), cubeWithMeasureType('sum')); + + await service.query(selection as never); + + expect(calls).toHaveLength(1); + expect(calls[0].aggregations?.[0].function).toBe('sum'); + expect(AggregationFunction.options).toContain(calls[0].aggregations?.[0].function); + }); + + it('refuses a method outside the engine vocabulary instead of forwarding it', async () => { + // Host drift: a cube object registered without meeting `CubeSchema`, so its + // `type` never faced the enum's parse. This is the arrival path the tiering + // note above describes. + const calls: EngineAggregateCall[] = []; + const service = await analyticsVia(fakeEngine(calls, schema), cubeWithMeasureType('median')); + + const err = await service.query(selection as never).then(() => null, (e: Error) => e); + + expect(err).toBeInstanceOf(Error); + // The wording IS the contract here: it must name the offending method, the + // aggregation it belongs to, and the legal vocabulary. + expect(err?.message).toContain('"median" is not one of the engine\'s aggregate functions'); + expect(err?.message).toContain('revenue'); + for (const fn of AggregationFunction.options) expect(err?.message).toContain(fn); + // Undeclared-500 tier, deliberately: no ADR-0112 envelope on this family. + expect((err as Error & { code?: string }).code).toBeUndefined(); + // The load-bearing half — the bad method never reached the engine, so no + // driver got a chance to blame a `function` key nobody wrote and no bucket + // came back silently null. + expect(calls).toHaveLength(0); + }); +}); diff --git a/packages/services/service-analytics/src/plugin.ts b/packages/services/service-analytics/src/plugin.ts index 3ed52d8bbe..01ef2ea1cf 100644 --- a/packages/services/service-analytics/src/plugin.ts +++ b/packages/services/service-analytics/src/plugin.ts @@ -2,113 +2,99 @@ import type { Plugin, PluginContext } from '@objectstack/core'; import type { Cube, FilterCondition } from '@objectstack/spec/data'; +import { AggregationFunction } from '@objectstack/spec/data'; import type { ExecutionContext } from '@objectstack/spec/kernel'; -import type { IAnalyticsService, IDataDriver } from '@objectstack/spec/contracts'; +import type { IAnalyticsService, IDataDriver, IDataEngine, IObjectQLEngine } from '@objectstack/spec/contracts'; import { AnalyticsService } from './analytics-service.js'; import type { AnalyticsServiceConfig } from './analytics-service.js'; import type { AnalyticsDriverCapabilities } from './strategies/types.js'; import { pickDisplayField, type DimensionLabelDeps } from './dimension-labels.js'; /** - * Minimal IDataEngine surface required for the auto-bridge. - * ObjectQL exposes: - * - `aggregate(object, { where, groupBy, aggregations: [{ function, field, alias }] })` - * - `execute(sql, options)` for raw SQL pass-through (enables NativeSQLStrategy - * and lets the analytics layer emit JOINs for relation traversal). + * The slice of the DECLARED engine contracts this plugin's auto-bridges + * consume, derived from `IDataEngine` / `IObjectQLEngine` instead of being + * re-declared structurally (#4251 B3, extending #11493's ruling and the + * datasource half of #11833 that landed as PR #12011). + * + * A consumer-local structural re-declaration meets no compiler on the PRODUCER + * side, so engine-surface drift lands here silently. This seam carried exactly + * that: the hand-written `aggregate` declared `aggregations[].function` as + * `string` where the contract declares a six-value enum, and nothing compiled + * the two against each other. + * + * Three of these members could not be named from a contract until #12248 + * landed the #11833 ruling: `resolveEffectiveDatasource` and + * `getDriverForObject` (fork 1, declared OPTIONAL exactly so seams like this + * one keep degrading), and `getObject`'s structured `ServiceObject` return + * (fork 3, which used to be `unknown`). Before those, the structural type was + * the only way to name them at all. + * + * ## Optionality is load-bearing, and this preserves the profile exactly + * + * `aggregate` stays REQUIRED, as the hand-written type had it: it is the + * member a `'data'` service must expose for this bridge to exist, and the + * runtime `typeof svc.aggregate === 'function'` probe below is what decides + * whether a registered service qualifies. + * + * Everything else stays OPTIONAL. A lightweight kernel can register a `'data'` + * service with no raw-SQL escape hatch, no datasource registry and no schema + * registry; the `engine?.x` / `typeof … === 'function'` probes below are the + * other half of that graceful-degradation contract. `getObject` is REQUIRED on + * `IObjectQLEngine`, so the `Partial<>` around it is not decoration — it is + * what keeps this seam usable against an engine that is not ObjectQL. */ -interface DataEngineLike { - aggregate(object: string, options: { - where?: Record; - groupBy?: string[]; - /** - * `filter` (#10576, the contract half of #10413's ruling) is a - * per-aggregation predicate over the SOURCE rows — SQL - * `FILTER (WHERE …)` semantics — kept optional here in lockstep with - * `EngineAggregateOptions['aggregations'][number].filter` so the bridge - * below can forward a measure-scoped filter without widening this - * interface again the next time #10413's phase-2 lowering needs a place - * to put one. - */ - aggregations?: Array<{ function: string; field: string; alias: string; filter?: Record }>; - /** Reference timezone (IANA) for date bucketing — ADR-0053 Phase 2. */ - timezone?: string; - /** - * `BaseEngineOptions.context` — identity/tenant of the request. The engine - * merges it into the operation context (`mergeReadContext`), which is what - * lets its middleware chain inject RLS into `opCtx.ast.where` (#3602). - */ - context?: ExecutionContext; - }): Promise; - /** - * Raw command pass-through (SQL on driver-sql). The options bag is spelled out - * rather than left as `Record` because **`object` is load-bearing**: - * `ObjectQL.execute()` picks its driver in the order - * `options.object` → `getDriver(object)`, then `options.datasource`, then the - * default driver. A command that reads an object and omits `object` therefore - * runs against the DEFAULT datasource — which is how every dataset backed by a - * telemetry/audit-routed object read `0` from a populated table (#5033). - */ - execute?(command: unknown, options?: { - /** Bound parameters for the command. */ - args?: unknown[]; - /** The object this command reads — routes to that object's own datasource. */ - object?: string; - }): Promise; - /** Return the registered object schema (relationship → target + display-label resolution). */ - getObject?(name: string): { - fields?: Record; - }>; - /** Federation marker (ADR-0015): set on objects bound to an external datasource. */ - external?: unknown; - } | undefined; - /** - * [#5288] The datasource an object's rows actually live on, by NAME — the - * engine's own five-step resolution (explicit `datasource` → - * `datasourceMapping` → the ADR-0057 §3.6 lifecycle split → the owning - * package's `defaultDatasource` → the deployment default), not the value the - * object declares. - * - * The declared value used to be read straight off `getObject().datasource`, - * and it is only step 1 of those five: `ObjectSchema.datasource` defaults to - * `'default'`, which in the engine means "no explicit binding, keep looking". - * So every object routed by steps 2-4 — `sys_audit_log` among them, routed by - * `lifecycle.class: 'audit'` — answered `'default'` and pointed diagnostics at - * a database its rows are not in. - * - * `undefined` ⇒ nothing binds the object anywhere and it rides the - * deployment's default datasource (or this engine cannot answer). Optional - * because the analytics service runs against engines other than ObjectQL; - * absent, the probe below simply never answers, which is the same "cannot - * answer, do not block" tiering it already carries. - */ - resolveEffectiveDatasource?(objectName: string): string | undefined; - /** - * Resolve the storage driver backing an object (public ObjectQL accessor). - * Used to delegate temporal storage-form coercion to the driver, which is the - * single source of truth for how a `Field.date`/`Field.datetime` is stored on - * the active dialect. When the hooks are absent, values and column SQL pass - * through untouched — the contract's identity semantics. - */ - getDriverForObject?(objectName: string): TemporalDriverSurface | undefined; -} +type DataEngineLike = + Pick + & Partial> + & Partial>; /** * The slice of the `IDataDriver` CONTRACT the analytics layer consumes — * `temporalFilterValue` / `temporalFilterColumnSql` are first-class contract - * members since ADR-0053 D-A2, no longer a duck-typed local invention. Picked - * (rather than using `IDataDriver` whole) because `getDriverForObject` hands - * back whatever the engine registered, and this seam only needs the temporal - * surface; the runtime `typeof` guards below remain the correct way to consume - * an optional contract member. + * members since ADR-0053 D-A2, no longer a duck-typed local invention. + * + * Since #12248 declared `IDataEngine.getDriverForObject?`, this is the + * RETURN-side narrowing that member's own docblock prescribes, applied at the + * two call sites below — not a re-declaration of the member. `Pick` admits the full contract value, so the engine keeps handing back whatever + * driver it registered while this seam states the only two members it reads. + * The runtime `typeof` guards below remain the correct way to consume an + * optional contract member. */ type TemporalDriverSurface = Pick< IDataDriver, 'temporalFilterValue' | 'temporalFilterColumnSql' >; +/** + * Narrow a strategy-supplied aggregation `method` to the engine contract's + * `AggregationFunction`, refusing anything else. + * + * The two sides genuinely differ: `IDataEngine.aggregate`'s + * `aggregations[].function` is the six-value enum, while the analytics + * strategy contract that feeds this bridge declares `aggregations[].method` as + * `string`. Parsing with the spec's OWN enum keeps a single vocabulary — no + * local literal list to drift, and `AggregationFunction`'s error map already + * knows the retired `array_agg` / `string_agg` spellings. + */ +function parseEngineAggregateFunction( + method: string, + alias: string, +): NonNullable[1]['aggregations']>[number]['function'] { + const parsed = AggregationFunction.safeParse(method); + if (!parsed.success) { + throw new Error( + `[Analytics] The aggregate bridge cannot forward the aggregation ` + + `"${alias}": "${method}" is not one of the engine's aggregate functions ` + + `(${AggregationFunction.options.join(', ')}). A custom-SQL measure is ` + + `refused earlier, with a caller-facing diagnostic, by ObjectQLStrategy; ` + + `reaching this point means the analytics layer produced a method the ` + + `engine contract does not declare.`, + ); + } + return parsed.data; +} + /** * Configuration for AnalyticsServicePlugin. */ @@ -301,7 +287,31 @@ export class AnalyticsServicePlugin implements Plugin { // aggregation carries none, matching the engine's own // vacuous-filter convention. aggregations: aggregations?.map((a) => ({ - function: a.method, + // [#11833] `function` is the engine contract's SIX-value + // `AggregationFunction`, while this bridge's own input declares + // `method: string` (`StrategyContext.executeAggregate`, spec + // `contracts/analytics-service.ts:300`). Narrowing the engine side + // to the contract turned that forward into a compile error — the + // correct signal, and the one the deleted structural type hid by + // declaring `function: string` on both sides. + // + // Closed by PARSING with the spec enum itself rather than by + // widening back to `string` (what hid it) or casting past it + // (which keeps the hole and adds a lie). `AggregationFunction` is + // the same schema `AggregationNodeSchema.function` is built from, + // so there is one vocabulary, and its own error map already + // carries the `array_agg`/`string_agg` retirement prescriptions. + // + // TIERING, deliberately: the reachable producer of a non-aggregate + // method — a custom-SQL measure (`AggregationMetricType` + // `number`/`string`/`boolean`) — is already refused upstream with a + // caller-blaming 400 by `ObjectQLStrategy.resolveMeasureAggregation` + // (#12209). Anything still arriving here is host drift, which that + // refusal's docblock assigns to the undeclared-500 tier — so this + // throws rather than re-blaming the caller, and it answers loudly + // instead of letting the engine answer `null` per bucket under the + // author's own measure name (the #4157 class). + function: parseEngineAggregateFunction(a.method, a.alias), field: a.field, alias: a.alias, ...(a.filter ? { filter: a.filter } : {}), @@ -563,7 +573,7 @@ export class AnalyticsServicePlugin implements Plugin { ): unknown => { try { const svc = ctx.getService('data'); - const driver = svc?.getDriverForObject?.(objectName); + const driver: TemporalDriverSurface | undefined = svc?.getDriverForObject?.(objectName); if (driver && typeof driver.temporalFilterValue === 'function') { return driver.temporalFilterValue(objectName, fieldName, value); } @@ -586,7 +596,7 @@ export class AnalyticsServicePlugin implements Plugin { ): string => { try { const svc = ctx.getService('data'); - const driver = svc?.getDriverForObject?.(objectName); + const driver: TemporalDriverSurface | undefined = svc?.getDriverForObject?.(objectName); if (driver && typeof driver.temporalFilterColumnSql === 'function') { return driver.temporalFilterColumnSql(objectName, fieldName, columnSql); }