diff --git a/.changeset/effective-datasource-accessor.md b/.changeset/effective-datasource-accessor.md new file mode 100644 index 0000000000..081efd0b21 --- /dev/null +++ b/.changeset/effective-datasource-accessor.md @@ -0,0 +1,49 @@ +--- +"@objectstack/objectql": patch +"@objectstack/service-analytics": patch +--- + +fix(objectql,service-analytics): report the datasource an object is actually on, not the one it declares (#5288) + +Analytics' `getObjectDatasource` probe read `getObject(name).datasource` — the +object's **declared** value, which is step 1 of the five `ObjectQL.getDriver` +resolves by. `ObjectSchema.datasource` carries `.default('default')`, and +`'default'` means "no explicit binding, keep looking" inside the engine, so +every object placed by a `datasourceMapping` rule, by the ADR-0057 §3.6 +lifecycle split, or by its package's `defaultDatasource` answered `'default'` +and was read out here as "the primary DB". + +`sys_audit_log` is the live specimen: `lifecycle.class: 'audit'` puts it on the +`telemetry` datasource with nothing declared to read. So #5033's query-time +diagnostic — whose entire job is to NAME the database a table is missing from — +named the wrong one: + +``` +before: table "account" is not on datasource "default", which is where its base object "sys_audit_log" lives +after: table "account" is not on datasource "telemetry", which is where its base object "sys_audit_log" lives +``` + +**New engine accessor — `ObjectQL.resolveEffectiveDatasource(objectName)`.** The +public, name-only face of the resolution order `getDriver` already routes by, +extracted so the order exists exactly once (the same argument that produced +`resolveMappedDatasource` in #4462: a second, shorter copy of a routing order +drifts by one step, silently). `getDriver` now consumes the same resolver and +keeps every existing behaviour — precedence, the refusal to fall through to the +default store when a declared or mapped datasource has no live driver, and both +of its diagnostics. + +It answers `undefined` when nothing binds the object anywhere and it simply +rides the deployment's default driver. That is deliberate and unchanged from +what consumers already documented: the default driver keeps its natural name +(#3826), so that name identifies a driver rather than a datasource anyone bound +the object to. `getDefaultDriverName()` is still there for callers that want it. + +Analytics' probe now asks the engine instead of the declaration; the routing +rules are **not** re-implemented on the analytics side. #5115's compile-time +cross-datasource join gate keeps its predicate exactly as written — what changed +is that its input can now answer for objects bound by a mapping rule, by the +lifecycle split, or by a package default, so a join between two bound +datasources is refused at registration instead of exploding at query time. A +join from a bound object to one that merely rides the deployment default is +still not decidable at compile time and remains the query-time diagnostic's +business. diff --git a/packages/objectql/src/engine-effective-datasource.test.ts b/packages/objectql/src/engine-effective-datasource.test.ts new file mode 100644 index 0000000000..a29d694245 --- /dev/null +++ b/packages/objectql/src/engine-effective-datasource.test.ts @@ -0,0 +1,251 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #5288 — `resolveEffectiveDatasource`: the datasource an object's rows are + * actually on, by NAME. + * + * The engine has five resolution steps (`getDriver`), and everyone who only + * needed the NAME used to read `object.datasource` — step 1. That value is the + * DECLARATION, and `ObjectSchema.datasource` carries `.default('default')`, so + * an object placed by a `datasourceMapping` rule, by the ADR-0057 §3.6 + * lifecycle split, or by its package's `defaultDatasource` still answered + * `'default'` — which in `getDriver` means "no explicit binding, keep looking", + * never "the primary DB". Analytics' `getObjectDatasource` probe was such a + * reader, and #5033's query-time diagnostic composed from it therefore named a + * database the rows are not in. + * + * These cases pin the accessor against `getDriver` itself: for every routing + * mechanism, the NAME it answers is the name of the driver `getDriver` picks. + * One resolution order, two shapes of answer. + */ + +import { describe, it, expect, beforeEach } from 'vitest'; +import { ObjectSchema } from '@objectstack/spec/data'; +import { ObjectQL } from './engine.js'; + +/** Owning package for the probe objects — no manifest is registered for it, so + * step 4 stays a no-op unless a case registers one. */ +const PKG = 'com.example.probe'; + +function stubDriver(name: string) { + return { + name, + version: '0.0.0', + supports: {}, + async connect() {}, + async disconnect() {}, + async checkHealth() { + return true; + }, + async execute() { + return null; + }, + async find() { + return []; + }, + async findOne() { + return null; + }, + async create(_o: string, d: Record) { + return d; + }, + async update() { + return {}; + }, + async upsert() { + return {}; + }, + async delete() { + return true; + }, + async count() { + return 0; + }, + async bulkCreate() { + return []; + }, + async bulkUpdate() { + return []; + }, + async bulkDelete() {}, + async beginTransaction() { + return {}; + }, + async commit() {}, + async rollback() {}, + } as any; +} + +describe('resolveEffectiveDatasource — one name per routing step (#5288)', () => { + let engine: ObjectQL; + let primary: any; + + beforeEach(async () => { + engine = new ObjectQL(); + primary = stubDriver('memory'); + engine.registerDriver(primary, true); + await engine.init(); + }); + + // ── Step 1: an explicit binding. The one step the old declared read got right ─ + + it('answers the explicit `datasource` — the step the declared read already had', () => { + engine.registerDriver(stubDriver('warehouse')); + engine.registry.registerObject({ name: 'wh_fact', datasource: 'warehouse', fields: {} }, PKG); + + expect(engine.resolveEffectiveDatasource('wh_fact')).toBe('warehouse'); + // …and it is the driver `getDriver` picks, not a parallel opinion about it. + expect(engine.getDriverForObject('wh_fact')).toBe(engine.getDriverByName('warehouse')); + }); + + // ── Step 2: a datasourceMapping rule ──────────────────────────────────────── + + it('answers the mapped datasource for an object a mapping rule places (step 2)', () => { + engine.registerDriver(stubDriver('archive')); + engine.setDatasourceMapping([{ objectPattern: 'log_*', datasource: 'archive' }]); + engine.registry.registerObject({ name: 'log_request', fields: {} }, PKG); + + // What the probe used to read — the declaration — says nothing at all here. + expect(engine.getObject('log_request')?.datasource).toBeUndefined(); + + expect(engine.resolveEffectiveDatasource('log_request')).toBe('archive'); + expect(engine.getDriverForObject('log_request')).toBe(engine.getDriverByName('archive')); + }); + + // ── Step 3: the ADR-0057 §3.6 lifecycle split — #5033's own object ────────── + + it('answers `telemetry` for a lifecycle-classed ledger (step 3) — the #5033 case', () => { + engine.registerDriver(stubDriver(ObjectQL.LIFECYCLE_DATASOURCE)); + engine.registry.registerObject({ + name: 'sys_audit_log', + lifecycle: { class: 'audit', retention: { maxAge: '90d' } }, + fields: {}, + }, PKG); + + expect(engine.resolveEffectiveDatasource('sys_audit_log')).toBe('telemetry'); + expect(engine.getDriverForObject('sys_audit_log')).toBe( + engine.getDriverByName(ObjectQL.LIFECYCLE_DATASOURCE), + ); + }); + + it('is Zod-parse independent: the declared read answers `default`, this answers `telemetry`', () => { + // The exact shape the defect wore in a real deployment. `ObjectSchema` gives + // `datasource` a `.default('default')`, so a PARSED object carries the + // string `'default'` — and the old probe reported it as if it were a + // database name, for an object the engine had routed elsewhere. + engine.registerDriver(stubDriver(ObjectQL.LIFECYCLE_DATASOURCE)); + const parsed = ObjectSchema.parse({ + name: 'sys_audit_log', + label: 'Audit log', + lifecycle: { class: 'audit', retention: { maxAge: '90d' } }, + fields: { action: { type: 'text', label: 'Action' } }, + }); + expect(parsed.datasource).toBe('default'); + engine.registry.registerObject(parsed, PKG); + + expect(engine.resolveEffectiveDatasource('sys_audit_log')).toBe('telemetry'); + }); + + it('does NOT invent lifecycle routing when no telemetry datasource is registered', () => { + // Step 3 is opt-in by the datasource's existence. Without it the ledger + // really is on the default store, and saying `'telemetry'` would be the + // same lie in the other direction. + engine.registry.registerObject({ + name: 'sys_audit_log', + lifecycle: { class: 'audit', retention: { maxAge: '90d' } }, + fields: {}, + }, PKG); + + expect(engine.resolveEffectiveDatasource('sys_audit_log')).toBeUndefined(); + expect(engine.getDriverForObject('sys_audit_log')).toBe(primary); + }); + + // ── Step 4: the owning package's defaultDatasource ────────────────────────── + + it("answers the owning package's `defaultDatasource` (step 4)", () => { + engine.registerDriver(stubDriver('billing_db')); + engine.registerApp({ + id: 'com.example.billing', + name: 'billing', + defaultDatasource: 'billing_db', + objects: [{ name: 'invoice', fields: {} }], + }); + + expect(engine.getObject('invoice')?.datasource).toBeUndefined(); + expect(engine.resolveEffectiveDatasource('invoice')).toBe('billing_db'); + expect(engine.getDriverForObject('invoice')).toBe(engine.getDriverByName('billing_db')); + }); + + it('ignores a package default whose datasource has no driver, exactly as getDriver does', () => { + engine.registerApp({ + id: 'com.example.billing', + name: 'billing', + defaultDatasource: 'never_connected', + objects: [{ name: 'invoice', fields: {} }], + }); + + // Step 4 answers only when the driver exists — the rows are on the default + // store, and that is what both the driver lookup and the name report. + expect(engine.resolveEffectiveDatasource('invoice')).toBeUndefined(); + expect(engine.getDriverForObject('invoice')).toBe(primary); + }); + + // ── Step 5 / no routing at all: unchanged, and deliberately `undefined` ───── + + it('answers `undefined` for an object nothing binds — it rides the default driver', () => { + // Unchanged from what the declared read answered for such an object, and + // deliberate: the default driver keeps its NATURAL name (#3826, here + // `memory`), so that name identifies a DRIVER, not a datasource anyone bound + // this object to. Callers that want it have `getDefaultDriverName()`. + engine.registry.registerObject({ name: 'biz_account', fields: {} }, PKG); + + expect(engine.resolveEffectiveDatasource('biz_account')).toBeUndefined(); + expect(engine.getDriverForObject('biz_account')).toBe(primary); + expect(engine.getDefaultDriverName()).toBe('memory'); + }); + + it('answers `undefined` for an object this engine has never heard of', () => { + expect(engine.resolveEffectiveDatasource('no_such_object')).toBeUndefined(); + }); + + // ── Precedence and the broken-deployment case ────────────────────────────── + + it('keeps getDriver’s precedence: an explicit binding outranks lifecycle routing', () => { + engine.registerDriver(stubDriver(ObjectQL.LIFECYCLE_DATASOURCE)); + engine.registerDriver(stubDriver('special')); + engine.registry.registerObject({ + name: 'probe_pinned', + datasource: 'special', + lifecycle: { class: 'telemetry', retention: { maxAge: '14d' } }, + fields: {}, + }, PKG); + + expect(engine.resolveEffectiveDatasource('probe_pinned')).toBe('special'); + }); + + it('keeps getDriver’s precedence: a mapping rule outranks lifecycle routing', () => { + engine.registerDriver(stubDriver(ObjectQL.LIFECYCLE_DATASOURCE)); + engine.registerDriver(stubDriver('archive')); + engine.setDatasourceMapping([{ objectPattern: 'sys_audit_*', datasource: 'archive' }]); + engine.registry.registerObject({ + name: 'sys_audit_log', + lifecycle: { class: 'audit', retention: { maxAge: '90d' } }, + fields: {}, + }, PKG); + + expect(engine.resolveEffectiveDatasource('sys_audit_log')).toBe('archive'); + expect(engine.getDriverForObject('sys_audit_log')).toBe(engine.getDriverByName('archive')); + }); + + it('names a binding whose driver is missing instead of throwing', () => { + // A naming probe exists to be READ, including while the deployment is + // broken: `getDriver` refuses to serve this object (it will not silently + // write to the default store — #4462), and the name it refuses ON is + // precisely what a diagnostic needs to print. + engine.registry.registerObject({ name: 'wh_fact', datasource: 'warehouse', fields: {} }, PKG); + + expect(engine.resolveEffectiveDatasource('wh_fact')).toBe('warehouse'); + expect(() => engine.getDriverForObject('wh_fact')).not.toThrow(); + expect(engine.getDriverForObject('wh_fact')).toBeUndefined(); + }); +}); diff --git a/packages/objectql/src/engine.ts b/packages/objectql/src/engine.ts index 48d5d1a653..13786395cd 100644 --- a/packages/objectql/src/engine.ts +++ b/packages/objectql/src/engine.ts @@ -3895,31 +3895,100 @@ export class ObjectQL implements IObjectQLEngine { * objects route to the dedicated 'telemetry' datasource when registered * 4. Package's `defaultDatasource` from manifest * 5. Global default driver + * + * The order itself lives in {@link resolveDatasourceBinding} — this method + * turns the name it decides on into a driver, and diagnoses the cases where + * that driver is missing. */ private getDriver(objectName: string): IDataDriver { - const object = this._registry.getObject(objectName); - - // 1. Object's explicit datasource field (highest priority) - if (object?.datasource && object.datasource !== 'default') { - if (this.drivers.has(object.datasource)) { - return this.drivers.get(object.datasource)!; + const binding = this.resolveDatasourceBinding(objectName); + + if (binding) { + const driver = this.drivers.get(binding.datasource); + if (driver) { + // Debug lines kept at the DECISION they describe, but emitted here so + // the resolver stays free of side effects: it also answers the public + // {@link resolveEffectiveDatasource} probe, which must be able to name + // a datasource without logging a routing event that never happened. + if (binding.via === 'mapping') { + this.logger.debug('Resolved datasource from mapping', { + object: objectName, + datasource: binding.datasource + }); + } else if (binding.via === 'package') { + this.logger.debug('Resolved datasource from package manifest', { + object: objectName, + package: binding.packageId, + datasource: binding.datasource + }); + } + return driver; } + + // Only steps 1 and 2 reach here. Steps 3-5 answer ONLY when their driver + // is registered (that registration is what opts the deployment into + // lifecycle separation / a package default / a global default at all), so + // a binding they produce always resolves. + // // The datasource layer may have recorded WHY this one has no driver — // refused by the host policy, or failed to connect under // OS_ALLOW_DRIVER_CONNECT_FAILURE (framework#3828). Saying so beats // sending the reader hunting for a typo that isn't there. - const unavailable = this.unavailableDatasources.get(object.datasource); + const unavailable = this.unavailableDatasources.get(binding.datasource); if (unavailable) { throw new DatasourceUnavailableError( - object.datasource, + binding.datasource, objectName, unavailable.kind, unavailable.publicDetail, ); } + if (binding.via === 'mapping') { + throw new Error( + `[ObjectQL] Datasource '${binding.datasource}' mapped for object '${objectName}' is not registered. ` + + `A datasourceMapping rule routes this object to it, so falling back to the default store would ` + + `write the object's data to a different database than the one it declares. Fix the datasource ` + + `configuration, or remove the mapping rule.`, + ); + } // No record: nothing ever tried to connect this name, so it is genuinely // undeclared (or misspelled). Unchanged message — there is nothing to add. - throw new Error(`[ObjectQL] Datasource '${object.datasource}' configured for object '${objectName}' is not registered.`); + throw new Error(`[ObjectQL] Datasource '${binding.datasource}' configured for object '${objectName}' is not registered.`); + } + + throw new Error(`[ObjectQL] No driver available for object '${objectName}'`); + } + + /** + * WHERE does `objectName`'s data live, and WHICH step decided — the single + * implementation of the resolution order documented on {@link getDriver}. + * + * Split out for #5288 so the order exists exactly once. It had two readers + * with two different answers: `getDriver` (all five steps) and every caller + * that only needed the NAME, which read `object.datasource` — step 1 of five. + * A second, shorter copy of a routing order is the failure + * `resolveMappedDatasource` (#4462) was extracted to prevent, and it fails the + * same way: silently, in whichever direction the copy is short. + * + * Steps 1-2 answer even when the named datasource has no registered driver: + * they are BINDINGS, and `getDriver` stops there (it throws rather than + * falling through — #4462). Steps 3-5 answer only when their driver is + * registered, because that registration is what turns each of them on. + * + * `undefined` means nothing routes the object anywhere: no binding, and no + * global default driver to fall back to. + */ + private resolveDatasourceBinding(objectName: string): { + datasource: string; + via: 'explicit' | 'mapping' | 'lifecycle' | 'package' | 'default'; + /** Owning package, on `via: 'package'` only — for the debug line. */ + packageId?: string; + } | undefined { + const object = this._registry.getObject(objectName); + + // 1. Object's explicit datasource field (highest priority) + if (object?.datasource && object.datasource !== 'default') { + return { datasource: object.datasource, via: 'explicit' }; } // 2. Check datasourceMapping rules @@ -3937,31 +4006,7 @@ export class ObjectQL implements IObjectQLEngine { // false by construction and step 5 is how routing to it works. const mappedDatasource = this.resolveDatasourceFromMapping(objectName, object); if (mappedDatasource && mappedDatasource !== 'default') { - if (this.drivers.has(mappedDatasource)) { - this.logger.debug('Resolved datasource from mapping', { - object: objectName, - datasource: mappedDatasource - }); - return this.drivers.get(mappedDatasource)!; - } - // Same three-way diagnosis as an explicit `object.datasource` binding — - // the two are the same promise made in two places, so they owe the reader - // the same answer. - const unavailable = this.unavailableDatasources.get(mappedDatasource); - if (unavailable) { - throw new DatasourceUnavailableError( - mappedDatasource, - objectName, - unavailable.kind, - unavailable.publicDetail, - ); - } - throw new Error( - `[ObjectQL] Datasource '${mappedDatasource}' mapped for object '${objectName}' is not registered. ` + - `A datasourceMapping rule routes this object to it, so falling back to the default store would ` + - `write the object's data to a different database than the one it declares. Fix the datasource ` + - `configuration, or remove the mapping rule.`, - ); + return { datasource: mappedDatasource, via: 'mapping' }; } // 3. Lifecycle-class separation (ADR-0057 §3.6): high-frequency @@ -3977,7 +4022,7 @@ export class ObjectQL implements IObjectQLEngine { ObjectQL.SYSTEM_LEDGER_LIFECYCLE_CLASSES.has(lifecycleClass) && this.drivers.has(ObjectQL.LIFECYCLE_DATASOURCE) ) { - return this.drivers.get(ObjectQL.LIFECYCLE_DATASOURCE)!; + return { datasource: ObjectQL.LIFECYCLE_DATASOURCE, via: 'lifecycle' }; } // 4. Check package's defaultDatasource @@ -3986,24 +4031,51 @@ export class ObjectQL implements IObjectQLEngine { const owner = this._registry.getObjectOwner(fqn); if (owner?.packageId) { const manifest = this.manifests.get(owner.packageId); - if (manifest?.defaultDatasource && manifest.defaultDatasource !== 'default') { - if (this.drivers.has(manifest.defaultDatasource)) { - this.logger.debug('Resolved datasource from package manifest', { - object: objectName, - package: owner.packageId, - datasource: manifest.defaultDatasource - }); - return this.drivers.get(manifest.defaultDatasource)!; - } + const packageDatasource = manifest?.defaultDatasource; + if (packageDatasource && packageDatasource !== 'default' && this.drivers.has(packageDatasource)) { + return { datasource: packageDatasource, via: 'package', packageId: owner.packageId }; } } // 5. Fallback to global default driver if (this.defaultDriver && this.drivers.has(this.defaultDriver)) { - return this.drivers.get(this.defaultDriver)!; + return { datasource: this.defaultDriver, via: 'default' }; } - throw new Error(`[ObjectQL] No driver available for object '${objectName}'`); + return undefined; + } + + /** + * Which datasource is `objectName` BOUND to? — the effective one, resolved + * through the same five steps {@link getDriver} routes by, computed as a NAME + * and without taking a driver. + * + * The public face of {@link resolveDatasourceBinding}, added for #5288, and + * the same argument as {@link resolveMappedDatasource} (#4462): a caller that + * only needs to NAME the datasource used to read `object.datasource` and stop + * there. That is step 1 of five, and `ObjectSchema.datasource` carries + * `.default('default')` — so an object routed by a `datasourceMapping` rule, + * by the ADR-0057 §3.6 lifecycle split, or by its package's + * `defaultDatasource` answered `'default'`, which in this engine means "no + * explicit binding, keep looking", never "the primary DB". A diagnostic built + * on that answer names a database the rows are not in. + * + * Returns `undefined` when nothing binds the object anywhere and it simply + * rides the deployment's default driver (step 5), which is deliberate on two + * counts. The default driver keeps its NATURAL name (#3826 — + * `drivers.has('default')` is false by construction), so that name identifies + * a driver, not a datasource anyone bound this object to; and "rides the + * default" is what the consumers of this answer already document as + * `undefined`. Callers that need the default driver's name have + * {@link getDefaultDriverName}. + * + * Never throws. A binding whose datasource has no registered driver is still + * this object's datasource — the deployment is broken, `getDriver` says so + * loudly, and a naming probe must be able to report the name that is broken. + */ + resolveEffectiveDatasource(objectName: string): string | undefined { + const binding = this.resolveDatasourceBinding(objectName); + return binding && binding.via !== 'default' ? binding.datasource : undefined; } /** diff --git a/packages/services/service-analytics/src/__tests__/effective-datasource-probe.test.ts b/packages/services/service-analytics/src/__tests__/effective-datasource-probe.test.ts new file mode 100644 index 0000000000..5146088b17 --- /dev/null +++ b/packages/services/service-analytics/src/__tests__/effective-datasource-probe.test.ts @@ -0,0 +1,258 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #5288 — the `getObjectDatasource` probe reports the EFFECTIVE datasource, not + * the declared one. + * + * `plugin.ts` used to wire the probe to `getObject(name).datasource` — the value + * the object DECLARES, which is step 1 of the five `ObjectQL.getDriver` routes + * by. `ObjectSchema.datasource` defaults to `'default'`, so every object placed + * by a `datasourceMapping` rule, by the ADR-0057 §3.6 lifecycle split, or by its + * package's `defaultDatasource` answered `'default'` — a string that means "no + * explicit binding, keep looking" inside the engine and "the primary DB" to + * everyone reading it out here. + * + * `sys_audit_log` is the live specimen: `lifecycle.class: 'audit'` puts it on + * the `telemetry` datasource without any declaration to read, so #5033's + * query-time diagnostic — whose entire job is to NAME the database a table is + * missing from — named the wrong one. + * + * These cases drive the REAL plugin wiring (the `fakePluginContext` harness from + * `execution-context-bridge.test.ts` / `raw-sql-object-routing.test.ts`) against + * an engine double that answers the two questions SEPARATELY: what the object + * declares, and where the engine actually routes it. That separation is what the + * defect lived in, and no test that made the double declare its routing could + * see it. + */ + +import { describe, it, expect, vi } from 'vitest'; +import { DatasetSchema } from '@objectstack/spec/ui'; +import type { AnalyticsService } from '../analytics-service.js'; +import { AnalyticsServicePlugin } from '../plugin.js'; + +// ── Engine double ─────────────────────────────────────────────────────────── + +interface EngineShape { + /** object → declared fields. Membership is also what `isRegisteredObject` reads. */ + schema: Record>; + /** + * What each object DECLARES. `'default'` is what a Zod-parsed object carries + * when its author declared nothing — the shape a real deployment has. + */ + declared: Record; + /** + * Where the engine actually routes each object — `resolveEffectiveDatasource`. + * Absent ⇒ the object rides the deployment default, which the accessor + * reports as `undefined`. + */ + routed: Record; + /** Tables physically present, per datasource. */ + tables: Record; +} + +const DEFAULT_DS = ''; + +/** + * Minimal ObjectQL stand-in. `execute` resolves its datasource the way + * `engine.execute` documents (by `options.object`) and then answers as a driver + * would: a relation that is not on the resolved datasource raises `no such + * table`, which is what drives #5033's triage. + * + * `withResolver: false` builds an engine that does NOT implement + * `resolveEffectiveDatasource` at all — an older or non-ObjectQL data service. + */ +function fakeEngine(shape: EngineShape, opts: { withResolver?: boolean } = {}) { + const withResolver = opts.withResolver !== false; + const datasourceOf = (object?: string) => (object ? shape.routed[object] ?? DEFAULT_DS : DEFAULT_DS); + const relationsIn = (sql: string): string[] => { + const names: string[] = []; + const from = /\bFROM\s+["`]?([a-z0-9_]+)["`]?/i.exec(sql); + if (from?.[1]) names.push(from[1]); + for (const m of sql.matchAll(/\bJOIN\s+["`]?([a-z0-9_]+)["`]?/gi)) names.push(m[1]); + return names; + }; + + const engine: Record = { + execute: async (sql: unknown, options?: { args?: unknown[]; object?: string }) => { + const ds = datasourceOf(options?.object); + for (const rel of relationsIn(String(sql))) { + if (!(shape.tables[ds] ?? []).includes(rel)) throw new Error(`SQLITE_ERROR: no such table: ${rel}`); + } + return { rows: [] }; + }, + getObject: (name: string) => { + const fields = shape.schema[name]; + if (!fields) return undefined; + // A real registered object carries its DECLARED datasource and nothing + // about the routing that placed it. + return { fields, datasource: shape.declared[name] ?? 'default' }; + }, + }; + if (withResolver) { + engine.resolveEffectiveDatasource = (name: string): string | undefined => + shape.schema[name] ? shape.routed[name] : undefined; + } + return engine; +} + +/** Minimal PluginContext: the four members `AnalyticsServicePlugin.init` uses. */ +function fakePluginContext(services: Record) { + const registered: Record = {}; + const warn = vi.fn(); + return { + warn, + 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 nativeSql = () => ({ nativeSql: true, objectqlAggregate: false, inMemory: false }); + +async function analyticsVia(engine: unknown) { + const { ctx, registered } = fakePluginContext({ data: engine }); + await new AnalyticsServicePlugin({ queryCapabilities: nativeSql }).init(ctx as never); + return registered.analytics as AnalyticsService; +} + +// ── The #5033 diagnostic, over a lifecycle-routed ledger ───────────────────── + +const TELEMETRY = 'telemetry'; + +/** `sys_audit_log` joined to `account` — the dataset #5033's message describes. */ +const auditByActor = DatasetSchema.parse({ + name: 'audit_by_actor', + label: 'Audit by actor', + object: 'sys_audit_log', + include: ['account'], + dimensions: [{ name: 'region', field: 'account.region', type: 'string' }], + measures: [{ name: 'event_count', aggregate: 'count' }], +}); +const auditSelection = { dimensions: ['region'], measures: ['event_count'] }; + +/** + * The issue's exact deployment shape: `sys_audit_log` DECLARES nothing (so it + * carries the schema default `'default'`) and is routed to `telemetry` by its + * `lifecycle.class`; `account` is an ordinary business object on the default + * store. + */ +const lifecycleRoutedShape: EngineShape = { + schema: { + sys_audit_log: { action: { type: 'text' }, account: { type: 'lookup', reference: 'account' } }, + account: { region: { type: 'text' } }, + }, + declared: { sys_audit_log: 'default', account: 'default' }, + routed: { sys_audit_log: TELEMETRY }, + tables: { [TELEMETRY]: ['sys_audit_log'], [DEFAULT_DS]: ['account'] }, +}; + +async function diagnosticFor(engine: unknown): Promise { + const service = await analyticsVia(engine); + const err = await service + .queryDataset(auditByActor as never, auditSelection as never) + .then(() => null, (e: Error) => e); + return String(err?.message ?? ''); +} + +describe('#5033 diagnostic names the datasource the object is actually on (#5288)', () => { + it('names `telemetry` for a ledger the lifecycle split routed there', async () => { + const message = await diagnosticFor(fakeEngine(lifecycleRoutedShape)); + + // The whole point of this wording is to name the real cause. + expect(message).toContain('table "account"'); + expect(message).toContain('is not on datasource "telemetry"'); + expect(message).toContain('base object "sys_audit_log"'); + // …and the joined side, which nothing binds, is reported as riding the + // default rather than being given an invented name. + expect(message).toContain('"account" is registered on the default datasource'); + }); + + it('no longer reports the object\'s DECLARED value as if it were a database', async () => { + const message = await diagnosticFor(fakeEngine(lifecycleRoutedShape)); + + // `'default'` is what `ObjectSchema.datasource` defaults to and what the + // engine reads as "no explicit binding, keep looking". Printing it as a + // database name is the defect — it pointed the reader at the main DB for a + // table that is in the telemetry one. + expect(message).not.toContain('datasource "default"'); + expect(message).not.toContain('is not on the default datasource'); + }); + + it('leaves an explicitly-bound object saying exactly what it said before', async () => { + // The step the declared read already got right must not move: an object + // that binds itself is reported by its own name, through the new probe. + const message = await diagnosticFor( + fakeEngine({ + ...lifecycleRoutedShape, + declared: { sys_audit_log: 'warehouse', account: 'default' }, + routed: { sys_audit_log: 'warehouse' }, + tables: { warehouse: ['sys_audit_log'], [DEFAULT_DS]: ['account'] }, + }), + ); + + expect(message).toContain('is not on datasource "warehouse"'); + }); + + it('stands down to the pre-#5288 wording when the engine cannot answer', async () => { + // A data service that does not implement the accessor (an older engine, a + // non-ObjectQL one). The probe answers nothing, the diagnostic says "the + // default datasource" rather than inventing a name — the same tiering every + // other probe on this config carries, and exactly the (wrong-but-honest) + // sentence this object produced before the fix. + const message = await diagnosticFor(fakeEngine(lifecycleRoutedShape, { withResolver: false })); + + expect(message).toContain('is not on the default datasource'); + expect(message).not.toContain('telemetry'); + }); +}); + +// ── #5115's compile-time gate: same rule, better-informed inputs ───────────── + +/** + * The gate's PREDICATE is untouched by #5288 ("both sides answered a + * non-`'default'` name, and the names differ"). What changed is what the host + * can answer with — so these two cases record where the compile-time verdict now + * lands, and where it still does not. Widening the predicate itself (so that + * "rides the deployment default" counts as an answer) is #5115's follow-up. + */ +describe('cross-datasource compile gate, with the effective probe (#5288)', () => { + const bind = (routed: Record) => + fakeEngine({ ...lifecycleRoutedShape, routed, tables: { [DEFAULT_DS]: ['account'] } }); + + it('refuses at compile time when BOTH sides are bound, whichever mechanism bound them', async () => { + // Base explicitly on `warehouse`, joined object routed to `telemetry` by its + // lifecycle class. Before #5288 the joined side answered `'default'` and the + // gate stood down; the join still could not execute, it just failed later. + const service = await analyticsVia( + bind({ sys_audit_log: 'warehouse', account: TELEMETRY }), + ); + + const err = (() => { + try { + service.registerDataset(auditByActor as never); + return null; + } catch (e) { + return e as Error & { code?: string; status?: number }; + } + })(); + + // ADR-0112 envelope — the author's verdict, not a runtime fault. + expect(err?.code).toBe('DATASET_INVALID'); + expect(err?.status).toBe(400); + expect(err?.message).toContain('declares a JOIN that crosses datasources'); + }); + + it('still stands down when one side merely rides the deployment default', async () => { + // The #5115 motivating scenario, and still undecidable HERE: `account` is + // bound by nothing, so the probe reports `undefined` rather than the default + // driver's natural name, and an unanswered side never rejects. It is caught + // at query time by the diagnostic above. + const service = await analyticsVia(bind({ sys_audit_log: TELEMETRY })); + + expect(() => service.registerDataset(auditByActor as never)).not.toThrow(); + }); +}); diff --git a/packages/services/service-analytics/src/__tests__/raw-sql-object-routing.test.ts b/packages/services/service-analytics/src/__tests__/raw-sql-object-routing.test.ts index 6294a7075b..267af58df0 100644 --- a/packages/services/service-analytics/src/__tests__/raw-sql-object-routing.test.ts +++ b/packages/services/service-analytics/src/__tests__/raw-sql-object-routing.test.ts @@ -107,6 +107,15 @@ function fakeEngine(opts: { const datasource = opts.routing[name]; return datasource ? { fields, datasource } : { fields }; }, + // [#5288] The engine's own answer to "where does this object's data live", + // which is what the analytics probe asks now — `getObject().datasource` is + // the DECLARED value and covers only the first of five resolution steps. + // `routing` above is already the effective placement (it is what `execute` + // resolves by), so the double answers both faces from the one map, the way + // `ObjectQL.resolveEffectiveDatasource` and `getDriver` answer from one + // resolution order. + resolveEffectiveDatasource: (name: string) => + (opts.schema[name] ? opts.routing[name] : undefined), }, }; } diff --git a/packages/services/service-analytics/src/analytics-service.ts b/packages/services/service-analytics/src/analytics-service.ts index 172adadece..baaf2bb48f 100644 --- a/packages/services/service-analytics/src/analytics-service.ts +++ b/packages/services/service-analytics/src/analytics-service.ts @@ -451,6 +451,15 @@ export interface AnalyticsServiceConfig { * datasources before any query is ever built. Absence keeps the pre-#5115 * behaviour exactly ("cannot answer, do not block") — the query-time * diagnostic above stays as the backstop. + * + * [#5288] "Bound to" above is the whole contract, and it took until #5288 for + * the built-in host to honour it: `plugin.ts` answered with the object's + * DECLARED `datasource` — step 1 of the five `ObjectQL.getDriver` routes by — + * so an object placed by a `datasourceMapping` rule, by the ADR-0057 §3.6 + * lifecycle split, or by its package's `defaultDatasource` reported + * `'default'` and sent the message above to the wrong database. It now asks + * `ObjectQL.resolveEffectiveDatasource`. A custom host owes the same answer: + * the datasource an object is BOUND to, `undefined` when nothing binds it. */ getObjectDatasource?: (objectName: string) => string | undefined; /** diff --git a/packages/services/service-analytics/src/dataset-compiler.ts b/packages/services/service-analytics/src/dataset-compiler.ts index b73a726382..173e640081 100644 --- a/packages/services/service-analytics/src/dataset-compiler.ts +++ b/packages/services/service-analytics/src/dataset-compiler.ts @@ -114,23 +114,32 @@ export type RelationshipResolver = ( */ export interface DatasetCompileOptions { /** - * [#5115] The datasource `objectName` DECLARES (`object.datasource`), or - * `undefined` when nothing authoritative can answer (no data engine, unknown - * object). + * [#5115] The datasource `objectName` is BOUND to, or `undefined` when nothing + * authoritative can answer (no data engine, unknown object) — or when nothing + * binds the object at all and it rides the deployment's default datasource. * * With it the compiler can settle at COMPILE time what #5033 could only * report at QUERY time: a dataset whose join crosses datasources declares a * statement no driver can execute, because the analytics engine lowers the * whole dataset into ONE SQL statement on the base object's datasource. * - * IMPORTANT — `'default'` is not an answer. In `ObjectQL.getDriver`'s - * resolution order an explicit `object.datasource` other than `'default'` - * wins outright (step 1); `'default'` is the schema's DEFAULT value and means - * only "no explicit binding", after which routing is decided by - * `datasourceMapping` rules, the ADR-0057 §3.6 lifecycle split, and the - * owning package's `defaultDatasource` — none of which are visible from here. - * The compiler therefore treats `'default'`/`undefined` as UNANSWERED. See - * {@link compileDataset}. + * [#5288] What the host supplies here changed shape, the rule below did not. + * It used to be the object's DECLARED `datasource` — step 1 of the five + * `ObjectQL.getDriver` routes by — so an object placed by a + * `datasourceMapping` rule, by the ADR-0057 §3.6 lifecycle split, or by its + * package's `defaultDatasource` answered `'default'` and was read here as + * unanswered. The built-in host (`plugin.ts`) now asks the engine's own + * resolver instead, so those three placements ARE visible from here and a join + * between two objects bound to two different datasources is decidable + * whichever mechanism bound them. + * + * Still deliberately UNANSWERED: `'default'`, and the object that no rule + * places anywhere. The deployment's default driver keeps its natural name + * (#3826), so "rides the default" is reported as `undefined` rather than as a + * name — which means a join from a bound object to a default-riding one stays + * undecidable here and remains the query-time diagnostic's business (#5288 + * records this boundary; widening it is #5115's follow-up, not this rule's). + * See {@link compileDataset}. */ getObjectDatasource?: (objectName: string) => string | undefined; /** @@ -226,18 +235,26 @@ export function compileDataset( // already exists; a false REJECT would blank a working dashboard on upgrade, // so this gate fires ONLY on a conflict the metadata itself proves. // - // What counts as an ANSWER (deliberately narrow): an EXPLICIT, non-`'default'` - // `object.datasource`. That is step 1 of `ObjectQL.getDriver`'s resolution - // order and it wins outright, so two objects declaring two different names are - // provably in two databases. `'default'` is the schema's default VALUE, not a - // routing decision: an object that leaves it alone is still routed by - // `datasourceMapping` rules, by the ADR-0057 §3.6 lifecycle split - // (audit/telemetry/event → the `telemetry` datasource), or by its package's - // `defaultDatasource` — rules this compiler cannot see. Treating `'default'` - // as "the primary DB" would reject a dataset whose two objects a mapping rule - // in fact lands on the SAME datasource, and would make the verdict depend on - // whether the object happened to be Zod-parsed (which materializes the - // default) — so `'default'` is read as UNANSWERED. + // What counts as an ANSWER: a non-`'default'` datasource NAME for the object. + // Two objects bound to two different names are provably in two databases. + // + // `'default'` is not one. It is what `ObjectSchema.datasource` defaults to and + // what `ObjectQL.getDriver` reads as "no explicit binding, keep looking", so + // treating it as "the primary DB" would reject a dataset whose two objects a + // mapping rule in fact lands on the SAME datasource, and would make the + // verdict depend on whether the object happened to be Zod-parsed (which + // materializes the default). It is read as UNANSWERED, as is `undefined`. + // + // [#5288] The probe used to report only step 1 of that resolution — the + // DECLARED value — which left every object placed by a `datasourceMapping` + // rule, by the ADR-0057 §3.6 lifecycle split (audit/telemetry/event → the + // `telemetry` datasource), or by its package's `defaultDatasource` answering + // `'default'` and therefore unanswerable here. The built-in host now asks + // `ObjectQL.resolveEffectiveDatasource`, so this rule — unchanged — sees those + // placements too. What it still cannot see is the object nothing binds at all: + // that one rides the deployment's default driver and is reported as + // `undefined`, so a join from a bound object to a default-riding one is not + // decidable here and stays with #5033's query-time diagnostic. const declaredDatasource = (objectName: string): string | undefined => { const declared = options?.getObjectDatasource?.(objectName); return declared && declared.toLowerCase() !== 'default' ? declared : undefined; diff --git a/packages/services/service-analytics/src/plugin.ts b/packages/services/service-analytics/src/plugin.ts index 077b98fe8b..2dd6cbcbb7 100644 --- a/packages/services/service-analytics/src/plugin.ts +++ b/packages/services/service-analytics/src/plugin.ts @@ -54,9 +54,28 @@ interface DataEngineLike { }>; /** Federation marker (ADR-0015): set on objects bound to an external datasource. */ external?: unknown; - /** The datasource this object is bound to (ADR-0062 D6 external detection). */ - datasource?: string; } | 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 @@ -563,7 +582,17 @@ export class AnalyticsServicePlugin implements Plugin { // datasource the query was routed to. Undefined ⇒ the object rides the // default datasource (or the engine cannot answer), and the diagnostic // says so rather than inventing a name. - getObjectDatasource: (objectName: string) => dataEngine()?.getObject?.(objectName)?.datasource, + // + // [#5288] Asked of the ENGINE's resolver, not of the object's declaration. + // `getObject(name).datasource` is the declared value — step 1 of the five + // `getDriver` routes by — so an object placed by a `datasourceMapping` + // rule, by the ADR-0057 §3.6 lifecycle split, or by its package's + // `defaultDatasource` answered `'default'`, and the diagnostic named a + // database the rows are not in. Recomputing those rules here instead would + // be the second implementation `resolveMappedDatasource` (#4462) exists to + // prevent: it drifts by one step, silently, and the drift only surfaces as + // an error message pointing at the wrong database. + getObjectDatasource: (objectName: string) => dataEngine()?.resolveEffectiveDatasource?.(objectName), // ADR-0062 D6 — a federated object carries an `external` block (ADR-0015). // Reported so NativeSQLStrategy declines it (its hand-compiled FROM would // hit the wrong physical table) and the driver-correct ObjectQL path runs.