diff --git a/.changeset/21889-code-datasource-namespace.md b/.changeset/21889-code-datasource-namespace.md new file mode 100644 index 00000000000..eb112ab1c0f --- /dev/null +++ b/.changeset/21889-code-datasource-namespace.md @@ -0,0 +1,14 @@ +--- +'@objectstack/runtime': patch +'@objectstack/service-datasource': patch +--- + +An import over a code-defined datasource is held to the namespace of the package that declares it (ADR-0028), and the draft door answers the prefixed name (#21889). + +Clause-②: no + +- **Before.** `POST /api/v1/datasources/:name/external/tables/:remote/import` with an explicit `name` that carried no namespace prefix answered `201` and saved an unprefixed federated object, and `POST …/external/tables/:remote/draft` answered the bare remote table name with a `TODO(namespace)` note. Measured on the showcase's `showcase_external` under `objectstack dev` and `objectstack start`. +- **`@objectstack/runtime`.** `AppPlugin` registers each code-defined datasource through `applyProtection` with the id and version of the package body that declares it, so the item carries `_packageId`, `_packageVersion` and `_provenance: 'package'`. On an ADR-0130 `packages[]` artifact each datasource takes its own body's id, never the artifact's top-level manifest id. A top-level datasource that no body declares keeps its registration under the artifact's own id, and a warning names it. +- **`@objectstack/service-datasource`.** The federation service reads the datasource's package record from the engine registry (`registry.getPackage` on the `objectql` service), the store the runtime publish gate reads for the same check. It used to ask the `metadata` service, which holds no package records in any composition, so no datasource resolved a namespace. The package id still comes only from the stamped `_packageId`. +- **What a caller sees now.** On a datasource whose package declares `manifest.namespace`, an unprefixed import `name` answers `400 EXTERNAL_IMPORT_ERROR` with ADR-0028's message, which names the prefixed name to use. An import with no `name` override saves the prefixed name the draft derives (for example `showcase_customers` instead of `customers`). `GET /api/v1/meta/datasource` lists the three provenance keys on a code-defined datasource; all three are declared on `DatasourceSchema`. The datasource admin list (`GET /api/v1/datasources`) is unchanged, and the admin door still refuses to edit or remove a code-defined datasource. +- **Unchanged.** A datasource that carries no `_packageId` (the host `default` is one) and a package that declares no namespace resolve no namespace, so their imports and drafts answer as before. diff --git a/packages/qa/dogfood/test/external-import-code-datasource-namespace.dogfood.test.ts b/packages/qa/dogfood/test/external-import-code-datasource-namespace.dogfood.test.ts new file mode 100644 index 00000000000..e56ff72bbfb --- /dev/null +++ b/packages/qa/dogfood/test/external-import-code-datasource-namespace.dogfood.test.ts @@ -0,0 +1,177 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// [#21889, ADR-0028] An import over a CODE-DEFINED datasource is held to the +// namespace of the package that declares the datasource, over the real +// showcase composition. +// +// ## What was broken +// +// The showcase declares `showcase_external` in its own package, whose +// `manifest.namespace` is `showcase`. `POST +// /api/v1/datasources/showcase_external/external/tables/orders/import` with an +// explicit `name` that carried no prefix answered `201` and persisted an +// unprefixed federated object, and the draft door answered the bare remote +// table name with its `TODO(namespace)` note. Two links were missing: +// `AppPlugin` registered the code-defined datasource with no `_packageId`, and +// the federation service then looked the package record up on the `metadata` +// service, which holds none in any composition. The datasource is now stamped +// with the package body that declares it, and the namespace is read from the +// engine registry, the store the publish gate reads. +// +// ## What each case pins +// +// The ruling's two pins: an unprefixed import name is refused with ADR-0028's +// message and saves nothing, and the draft door answers the prefixed name with +// no `TODO(namespace)` note. Then the controls that show the stamp moved +// nothing else: a prefixed import is saved and serves the remote rows, the +// host `default` datasource carries no package, the admin service keeps +// refusing edits to a code-defined datasource and lists it as before, and +// validate and the existing federated objects answer as before. +// +// The verify harness does not mount the federation service, so this file +// mounts `ExternalDatasourceServicePlugin` itself, as `objectstack dev` and +// `serve` do. The working directory is a temporary one because the showcase's +// external datasource and its fixture both name a cwd-relative SQLite file. + +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import showcaseStack, { onEnable } from '@objectstack/example-showcase'; +import { ExternalDatasourceServicePlugin } from '@objectstack/service-datasource'; +import { bootStack, type VerifyStack } from '@objectstack/verify'; +import { mkdtempSync, rmSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; + +/** The showcase's code-defined external datasource (`showcase-external.datasource.ts`). */ +const DATASOURCE = 'showcase_external'; +/** The package that declares it, and that package's ADR-0028 namespace. */ +const PACKAGE_ID = 'com.example.showcase'; +const NAMESPACE = 'showcase'; +/** An explicit import name with no namespace prefix: the defect's shape. */ +const UNPREFIXED = 'dogfood_ext_order_21889'; +const PREFIXED = `${NAMESPACE}_${UNPREFIXED}`; + +const ext = `/datasources/${DATASOURCE}/external`; + +interface Envelope { + success?: boolean; + error?: { code?: string; message?: string }; + item?: Record; + data?: unknown; + records?: Array>; +} + +describe('an import over a code-defined datasource is held to its package\'s namespace (showcase)', () => { + let stack: VerifyStack; + let token: string; + let prevCwd: string; + let dir: string; + + const call = async (method: string, path: string, body?: unknown) => { + const res = await stack.apiAs(token, method, path, body); + const json = (await res.json().catch(() => ({}))) as Envelope; + return { status: res.status, json }; + }; + + beforeAll(async () => { + prevCwd = process.cwd(); + dir = mkdtempSync(join(tmpdir(), 'dogfood-21889-')); + process.chdir(dir); + // Provision the remote tables, exactly as `os dev` does at boot (the + // harness imports only the stack's default export, so `onEnable` never + // runs on its own). + await onEnable({ logger: { info() {}, warn() {} } } as never); + stack = await bootStack(showcaseStack, { + databaseFile: join(dir, 'showcase.db'), + extraPlugins: [new ExternalDatasourceServicePlugin()], + }); + token = await stack.signIn(); + }, 180_000); + + afterAll(async () => { + await stack?.stop(); + if (prevCwd) process.chdir(prevCwd); + if (dir) rmSync(dir, { recursive: true, force: true }); + }); + + it('premise: the code-defined datasource carries the provenance of the package that declares it', async () => { + const version = (showcaseStack as unknown as { manifest?: { version?: string } }).manifest?.version; + expect(version).toMatch(/^\d+\.\d+\.\d+/); + + const read = await call('GET', `/meta/datasource/${DATASOURCE}`); + expect(read.status, JSON.stringify(read.json)).toBe(200); + expect(read.json.item).toMatchObject({ + origin: 'code', + _packageId: PACKAGE_ID, + _packageVersion: version, + _provenance: 'package', + }); + }); + + it('control: validate and the code-defined federated objects answer as before, ahead of any import', async () => { + const validated = await call('POST', `${ext}/validate`); + expect(validated.status, JSON.stringify(validated.json)).toBe(200); + const report = validated.json.data as { ok?: boolean; results?: Array<{ object: string }> }; + expect(report.ok).toBe(true); + expect(report.results?.map((r) => r.object).sort()).toEqual(['showcase_ext_customer', 'showcase_ext_order']); + + const rows = await call('GET', '/data/showcase_ext_order'); + expect(rows.status, JSON.stringify(rows.json)).toBe(200); + expect(rows.json.records).toHaveLength(4); + }); + + it('an explicit import name without the prefix is refused with ADR-0028\'s message, and nothing is saved', async () => { + const refused = await call('POST', `${ext}/tables/orders/import`, { name: UNPREFIXED }); + + expect(refused.status, JSON.stringify(refused.json)).toBe(400); + expect(refused.json.error?.code).toBe('EXTERNAL_IMPORT_ERROR'); + expect(refused.json.error?.message).toContain('missing the package namespace prefix'); + expect(refused.json.error?.message).toContain(`'${PREFIXED}'`); + + const stored = await call('GET', `/meta/object/${UNPREFIXED}`); + expect(stored.status, JSON.stringify(stored.json)).toBe(404); + }); + + it('the draft door answers the prefixed name, with no TODO(namespace) note', async () => { + const drafted = await call('POST', `${ext}/tables/orders/draft`, {}); + + expect(drafted.status, JSON.stringify(drafted.json)).toBe(200); + const draft = (drafted.json.data as { draft?: { name?: string; source?: string } } | undefined)?.draft; + expect(draft?.name).toBe(`${NAMESPACE}_orders`); + expect(draft?.source).not.toContain('TODO(namespace)'); + }); + + it('control: a prefixed import name is saved and serves the remote rows', async () => { + const imported = await call('POST', `${ext}/tables/orders/import`, { name: PREFIXED }); + expect(imported.status, JSON.stringify(imported.json)).toBe(201); + + const rows = await call('GET', `/data/${PREFIXED}`); + expect(rows.status, JSON.stringify(rows.json)).toBe(200); + expect(rows.json.records).toHaveLength(4); + }); + + it('control: the host `default` datasource carries no package, so no namespace is demanded on it', async () => { + const read = await call('GET', '/meta/datasource/default'); + expect(read.status, JSON.stringify(read.json)).toBe(200); + expect(read.json.item).not.toHaveProperty('_packageId'); + expect(read.json.item).not.toHaveProperty('_provenance'); + }); + + it('control: the admin service still refuses to edit or remove a code-defined datasource, and lists no envelope key', async () => { + // The harness mounts no `/api/v1/datasources` admin routes (the CLI's + // `serve` does), so this reads the service those routes relay: a refusal + // here is the door's `400 DATASOURCE_ADMIN_ERROR`. The refusals key on + // `origin`, which the stamp leaves as it was. + const admin = stack.kernel.getService<{ + updateDatasource(name: string, patch: Record): Promise; + removeDatasource(name: string): Promise; + listDatasources(): Promise>>; + }>('datasource-admin'); + + await expect(admin.updateDatasource(DATASOURCE, { label: 'Renamed 21889' })).rejects.toBeInstanceOf(Error); + await expect(admin.removeDatasource(DATASOURCE)).rejects.toBeInstanceOf(Error); + + const entry = (await admin.listDatasources()).find((d) => d.name === DATASOURCE); + expect(entry).toMatchObject({ origin: 'code', label: 'External Analytics (SQLite)' }); + expect(entry).not.toHaveProperty('_packageId'); + }); +}); diff --git a/packages/qa/dogfood/test/external-import-destructive-remedy.dogfood.test.ts b/packages/qa/dogfood/test/external-import-destructive-remedy.dogfood.test.ts index eb76927d9ec..6d27d0e8c26 100644 --- a/packages/qa/dogfood/test/external-import-destructive-remedy.dogfood.test.ts +++ b/packages/qa/dogfood/test/external-import-destructive-remedy.dogfood.test.ts @@ -30,6 +30,10 @@ // mounts `ExternalDatasourceServicePlugin` itself, as `objectstack dev` and // `serve` do. The working directory is a temporary one because the showcase's // external datasource and its fixture both name a cwd-relative SQLite file. +// +// [#21889] Both imported names carry the showcase's ADR-0028 prefix: +// `showcase_external` is declared by the showcase package (namespace +// `showcase`), so an import over it is held to that namespace. import { describe, it, expect, beforeAll, afterAll } from 'vitest'; import showcaseStack, { onEnable } from '@objectstack/example-showcase'; @@ -43,9 +47,9 @@ import { join } from 'node:path'; const DATASOURCE = 'showcase_external'; const REMOTE = 'customers'; /** The object the first import creates and the re-import would shrink. */ -const NAME = 'dogfood_ext_cust_21841'; +const NAME = 'showcase_dogfood_ext_cust_21841'; /** The remedy's "new `name`". */ -const NEW_NAME = 'dogfood_ext_cust_21841_v2'; +const NEW_NAME = 'showcase_dogfood_ext_cust_21841_v2'; /** The column the re-import leaves out, so the stored object would lose its field. */ const DROPPED = 'region'; /** The re-import's options: the same table, one column fewer. */ diff --git a/packages/qa/dogfood/test/external-import-saves-like-meta.dogfood.test.ts b/packages/qa/dogfood/test/external-import-saves-like-meta.dogfood.test.ts index 00ad71e8fd7..1280f3ff66c 100644 --- a/packages/qa/dogfood/test/external-import-saves-like-meta.dogfood.test.ts +++ b/packages/qa/dogfood/test/external-import-saves-like-meta.dogfood.test.ts @@ -31,6 +31,13 @@ // `serve` do. The working directory is a temporary one because the showcase's // external datasource and its fixture both name a cwd-relative SQLite file: // this file's remote database is its own, not one a parallel file writes. +// +// [#21889] Both imported names carry the showcase's ADR-0028 prefix: +// `showcase_external` is declared by the showcase package (namespace +// `showcase`), so an import over it is held to that namespace. That retires the +// shape this file's control first pinned, an object named exactly as its +// remote table (`orders`), which the namespace now refuses; the control imports +// with no `name` override instead, under the prefixed name the draft derives. import { describe, it, expect, beforeAll, afterAll } from 'vitest'; import showcaseStack, { onEnable } from '@objectstack/example-showcase'; @@ -43,9 +50,9 @@ import { join } from 'node:path'; /** The showcase's external datasource and its two remote tables (`external-fixture.ts`). */ const DATASOURCE = 'showcase_external'; /** Imported under a name that differs from its remote table — the defect's shape. */ -const RENAMED = 'dogfood_ext_cust_21788'; -/** Imported under the remote table's own name — the shape that hid the defect. */ -const SAME_NAME = 'orders'; +const RENAMED = 'showcase_dogfood_ext_cust_21788'; +/** Imported with no `name` override: the draft's name, the remote table's prefixed with the namespace. */ +const DERIVED = 'showcase_orders'; const importPath = (remote: string) => `/datasources/${DATASOURCE}/external/tables/${remote}/import`; @@ -101,11 +108,12 @@ describe('Import as Object persists like the metadata door (showcase, cold boot) ); }); - it('control: an object imported under its remote table\'s own name serves the remote rows', async () => { - const imported = await call('POST', importPath('orders'), { name: SAME_NAME }); + it('control: an object imported with no name override is saved under the prefixed name and serves the remote rows', async () => { + const imported = await call('POST', importPath('orders'), {}); expect(imported.status, JSON.stringify(imported.json)).toBe(201); + expect((imported.json as { data?: { object?: { name?: string } } }).data?.object?.name).toBe(DERIVED); - const read = await call('GET', `/data/${SAME_NAME}`); + const read = await call('GET', `/data/${DERIVED}`); expect(read.status, JSON.stringify(read.json)).toBe(200); expect(recordsOf(read.json)).toHaveLength(4); }); @@ -120,8 +128,8 @@ describe('Import as Object persists like the metadata door (showcase, cold boot) expect.soft(renamed.status, JSON.stringify(renamed.json)).toBe(200); expect.soft(recordsOf(renamed.json)).toHaveLength(3); - const sameName = await call('GET', `/data/${SAME_NAME}`); - expect.soft(sameName.status, JSON.stringify(sameName.json)).toBe(200); - expect.soft(recordsOf(sameName.json)).toHaveLength(4); + const derived = await call('GET', `/data/${DERIVED}`); + expect.soft(derived.status, JSON.stringify(derived.json)).toBe(200); + expect.soft(recordsOf(derived.json)).toHaveLength(4); }, 180_000); }); diff --git a/packages/qa/dogfood/test/external-validate-sees-runtime-save.dogfood.test.ts b/packages/qa/dogfood/test/external-validate-sees-runtime-save.dogfood.test.ts index 113b176dc2b..3d555e508c4 100644 --- a/packages/qa/dogfood/test/external-validate-sees-runtime-save.dogfood.test.ts +++ b/packages/qa/dogfood/test/external-validate-sees-runtime-save.dogfood.test.ts @@ -49,7 +49,8 @@ const SAVED = 'dogfood_ext_cust_21842'; /** A column the saved object declares and the remote `customers` table does not have. */ const MISSING = 'loyalty_tier'; /** Imported at runtime from the remote `orders` table. */ -const IMPORTED = 'dogfood_ext_ord_21842'; +// [#21889] Prefixed: an import over `showcase_external` is held to the showcase package's namespace. +const IMPORTED = 'showcase_dogfood_ext_ord_21842'; const validatePath = `/datasources/${DATASOURCE}/external/validate`; diff --git a/packages/runtime/src/app-plugin.datasource-provenance.test.ts b/packages/runtime/src/app-plugin.datasource-provenance.test.ts new file mode 100644 index 00000000000..18c5710aae7 --- /dev/null +++ b/packages/runtime/src/app-plugin.datasource-provenance.test.ts @@ -0,0 +1,191 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #21889 — `AppPlugin` registers each code-defined datasource with the package + * that declares it. + * + * ## The defect this pins closed + * + * `AppPlugin.start()` registered a code-defined datasource as + * `{ ...ds, origin: 'code' }`, with no `_packageId`. The federation service + * resolves the ADR-0028 namespace an imported object must carry from the + * datasource's `_packageId`, so for every code-defined datasource it resolved + * none: `POST /datasources/:name/external/tables/:remote/import` accepted an + * unprefixed `name`, and the draft door emitted the bare remote table name. + * The datasource is now stamped through `applyProtection`, the stamping helper + * the other load paths use. + * + * ## What each case pins + * + * - a single-package bundle (the `defineStack()` shape every example boots): + * the manifest's id and version, in both datasource spellings; + * - an ADR-0130 `packages[]` artifact, in today's ADDITIVE shape and in the + * option-B shape: each datasource carries the id of the package BODY that + * declares it, never the artifact's top-level manifest id (#14599's + * misattribution class), and each is registered once; + * - a top-level datasource no body declares keeps its registration, under the + * artifact's own manifest id, and the disagreement is logged. + * + * No in-repo example declares a datasource in a multi-package artifact + * (`examples/app-multi-package` declares none), so the two-body cases are + * pinned here, at the plugin. The booted-stack half (the showcase's + * `showcase_external` at the import door) is the dogfood pin's. + */ + +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import type { PluginContext } from '@objectstack/core'; + +import { AppPlugin } from './app-plugin.js'; + +type Register = (type: string, name: string, data: unknown) => void; + +const datasource = (name: string) => ({ + name, + label: name, + driver: 'sqlite', + schemaMode: 'external', + config: { filename: ':memory:' }, + external: { allowWrites: false }, +}); + +const coreBody = () => ({ + id: 'com.test.dsprov.core', + name: 'Datasource Provenance Core', + version: '1.0.0', + type: 'app', + namespace: 'dpcore', + datasources: [datasource('dpcore_wh')], +}); + +const ordersBody = () => ({ + id: 'com.test.dsprov.orders', + name: 'Datasource Provenance Orders', + version: '3.2.0', + type: 'module', + namespace: 'dporders', + dependencies: { 'com.test.dsprov.core': '^1.0.0' }, + datasources: [datasource('dporders_wh')], +}); + +/** The artifact's top-level manifest: the core package's, as `selectManifest` may pick it. */ +const topManifest = { id: 'com.test.dsprov.core', name: 'Datasource Provenance Core', version: '1.0.0', type: 'app' }; + +/** What the platform emits today: every datasource flattened to the top level AND under its body. */ +const additiveArtifact = () => { + const core = coreBody(); + const orders = ordersBody(); + return { + manifest: topManifest, + datasources: [...orders.datasources.map((d) => ({ ...d })), ...core.datasources.map((d) => ({ ...d }))], + packages: [{ manifest: orders }, { manifest: core }], + }; +}; + +/** Option B: `packages[]` carries every datasource once, the top level none. */ +const optionBArtifact = () => ({ + manifest: topManifest, + packages: [{ manifest: ordersBody() }, { manifest: coreBody() }], +}); + +describe('#21889 — AppPlugin stamps each code-defined datasource with the package that declares it', () => { + let register: ReturnType>; + let warn: ReturnType; + let ctx: PluginContext; + + beforeEach(() => { + register = vi.fn(); + warn = vi.fn(); + ctx = { + logger: { info: vi.fn(), error: vi.fn(), warn, debug: vi.fn() }, + registerService: vi.fn(), + getService: vi.fn((name: string) => { + if (name === 'objectql') return { setDatasourceMapping: () => undefined }; + if (name === 'metadata') return { registerInMemory: register }; + return undefined; + }), + getServices: vi.fn(() => []), + hook: vi.fn(), + trigger: vi.fn(), + } as unknown as PluginContext; + }); + + /** Every `registerInMemory('datasource', …)` call, by name. */ + const registered = () => { + const calls = register.mock.calls.filter(([type]) => type === 'datasource'); + return { + names: calls.map(([, name]) => name), + item: (name: string) => calls.find(([, n]) => n === name)?.[2] as Record | undefined, + }; + }; + + it('a single-package bundle: the manifest\'s id and version, with provenance `package`', async () => { + const declared = datasource('solo_wh'); + await new AppPlugin({ + manifest: { id: 'com.test.dsprov.solo', name: 'Solo', version: '2.1.0', namespace: 'solo' }, + datasources: [declared], + }).start!(ctx); + + expect(registered().names).toEqual(['solo_wh']); + expect(registered().item('solo_wh')).toEqual({ + ...declared, + origin: 'code', + _packageId: 'com.test.dsprov.solo', + _packageVersion: '2.1.0', + _provenance: 'package', + }); + // The author's definition is not written to: the stamp lands on the copy. + expect(declared).not.toHaveProperty('_packageId'); + }); + + it('a single-package bundle in the record spelling: the key is the name, and the same stamp lands', async () => { + await new AppPlugin({ + manifest: { id: 'com.test.dsprov.solo', name: 'Solo', version: '2.1.0' }, + datasources: { solo_wh: { driver: 'sqlite', schemaMode: 'external', config: {} } }, + }).start!(ctx); + + expect(registered().item('solo_wh')).toMatchObject({ + name: 'solo_wh', + origin: 'code', + _packageId: 'com.test.dsprov.solo', + _packageVersion: '2.1.0', + }); + }); + + it('today\'s additive `packages[]` artifact: each datasource carries ITS body\'s id, not the top-level manifest\'s', async () => { + await new AppPlugin(additiveArtifact()).start!(ctx); + + // Read once each: the flattened copy and the body's copy are one datasource. + expect(registered().names.sort()).toEqual(['dpcore_wh', 'dporders_wh']); + expect(registered().item('dporders_wh')).toMatchObject({ + origin: 'code', + _packageId: 'com.test.dsprov.orders', + _packageVersion: '3.2.0', + _provenance: 'package', + }); + expect(registered().item('dpcore_wh')).toMatchObject({ + _packageId: 'com.test.dsprov.core', + _packageVersion: '1.0.0', + }); + expect(warn).not.toHaveBeenCalledWith(expect.stringContaining('none of its package bodies declare')); + }); + + it('the option-B artifact (the top level carries no datasource): the same attribution', async () => { + await new AppPlugin(optionBArtifact()).start!(ctx); + + expect(registered().names.sort()).toEqual(['dpcore_wh', 'dporders_wh']); + expect(registered().item('dporders_wh')?._packageId).toBe('com.test.dsprov.orders'); + expect(registered().item('dpcore_wh')?._packageId).toBe('com.test.dsprov.core'); + }); + + it('a top-level datasource no body declares keeps its registration, under the artifact\'s own id, and is logged', async () => { + const artifact = additiveArtifact(); + artifact.datasources.push({ ...datasource('stray_wh') }); + + await new AppPlugin(artifact).start!(ctx); + + expect(registered().names.sort()).toEqual(['dpcore_wh', 'dporders_wh', 'stray_wh']); + expect(registered().item('stray_wh')).toMatchObject({ _packageId: 'com.test.dsprov.core' }); + expect(registered().item('dporders_wh')?._packageId).toBe('com.test.dsprov.orders'); + expect(warn).toHaveBeenCalledWith(expect.stringMatching(/1 top-level datasource\(s\) that none of its package bodies declare: 'stray_wh'/)); + }); +}); diff --git a/packages/runtime/src/app-plugin.ts b/packages/runtime/src/app-plugin.ts index 4caef82e7c2..769970175c3 100644 --- a/packages/runtime/src/app-plugin.ts +++ b/packages/runtime/src/app-plugin.ts @@ -3,7 +3,9 @@ import { Plugin, PluginContext, + artifactPackageId, createPluginPermissionEnforcer, + resolveArtifactPackageOrder, wireAuthoredTranslationSync, type PluginPermissionEnforcer, } from '@objectstack/core'; @@ -24,6 +26,7 @@ import type { IMetadataService, IObjectQLEngine, II18nService } from '@objectsta import { normalizeFlowFunctionEntry, type NormalizedFlowFunction } from '@objectstack/spec/automation'; import { readServiceSelfInfo } from '@objectstack/spec/api'; import { SEED_WRITE_EXECUTION_CONTEXT } from '@objectstack/spec/kernel'; +import { applyProtection } from '@objectstack/spec/shared'; import { QuickJSScriptRunner } from './sandbox/quickjs-runner.js'; import { hookBodyRunnerFactory, actionBodyRunnerFactory } from './sandbox/body-runner.js'; import { bindAppArtifactHandlers, scheduleAppArtifactJobs } from './app-artifact-handlers.js'; @@ -662,6 +665,84 @@ export class AppPlugin implements Plugin { return out; } + /** + * Every code-defined datasource this artifact declares, each paired with + * the coordinates of the package that declares it — what `start()` hands + * `applyProtection` when it registers the datasource. + * + * [#21889] The federation service reads those coordinates back: its + * `getNamespace` (`@objectstack/service-datasource`) takes the datasource's + * `_packageId` to the engine registry's package record, whose + * `manifest.namespace` is the ADR-0028 prefix an object imported over the + * datasource must carry. Registered without coordinates, a code-defined + * datasource resolved no namespace: the import accepted an unprefixed + * name and the draft emitted one. + * + * The owner is read the way the metadata plugin's artifact door reads it + * (ADR-0130 D4, `packages/metadata/src/plugin.ts`), and for its reasons: + * + * - `packages` ABSENT: the artifact is its one package's body, so every + * datasource takes the manifest's id, the key `registerApp` installs the + * package under. The list is `this.collections.datasources`, the one + * this registration has always read (D7). + * - `packages` PRESENT: each body's datasources take THAT body's id + * (`artifactPackageId`, the one spelling `registerApp` keys on), in the + * order `resolveArtifactPackageOrder` gives. ⛔ Never the artifact's + * top-level manifest id on every entry: on a composed artifact that id + * is one member's, and stamping it everywhere hands one package another + * package's datasources (#14599). ⛔ Never a name-to-package index over + * the flattened list either: that is a second resolution path for + * ownership, and a dangling one fails silently. + * - A datasource no body declares (an artifact composed from a stack that + * already carried `packages` and one that did not) falls back to the + * artifact's own manifest id, as the door's residual sweep does, and is + * logged, because the artifact's two halves disagree. + */ + private codeDefinedDatasourceOwners(ctx: PluginContext): Array<{ + datasource: any; + owner: { packageId?: string; packageVersion?: string }; + }> { + const listOf = (dsDefs: unknown): any[] => + Array.isArray(dsDefs) + ? dsDefs + : dsDefs && typeof dsDefs === 'object' + ? Object.entries(dsDefs).map(([name, def]) => ({ name, ...(def as any) })) + : []; + const coordsOf = (body: any) => ({ + packageId: artifactPackageId(body), + packageVersion: typeof body?.version === 'string' ? body.version : undefined, + }); + const own = coordsOf(this.bundle.manifest || this.bundle); + const declared = listOf(this.collections.datasources); + + if ((this.bundle as { packages?: unknown }).packages === undefined) { + return declared.map((datasource) => ({ datasource, owner: own })); + } + + const owned: Array<{ datasource: any; owner: { packageId?: string; packageVersion?: string } }> = []; + const claimed = new Set(); + for (const body of resolveArtifactPackageOrder(this.bundle)) { + const owner = coordsOf(body); + for (const datasource of listOf((body as { datasources?: unknown } | null)?.datasources)) { + claimed.add(datasource?.name); + owned.push({ datasource, owner }); + } + } + const residual = declared.filter((datasource) => !claimed.has(datasource?.name)); + if (residual.length > 0) { + ctx.logger.warn( + `[AppPlugin] artifact '${own.packageId ?? ''}' carries ${residual.length} top-level ` + + 'datasource(s) that none of its package bodies declare: ' + + `${residual.map((d) => `'${String(d?.name)}'`).join(', ')}. They were registered under ` + + "the artifact's own manifest id because no package in the artifact claims them, so the " + + 'ADR-0028 namespace an import over them must carry is that package\'s. Rebuild the ' + + 'artifact with one `composeStacks` run so every datasource sits in the body that owns it.', + ); + owned.push(...residual.map((datasource) => ({ datasource, owner: own }))); + } + return owned; + } + start = async (ctx: PluginContext) => { if (this.empty) { ctx.logger.debug('[AppPlugin] empty env — no app payload, skipping start', { @@ -747,9 +828,17 @@ export class AppPlugin implements Plugin { | { registerInMemory?: (t: string, n: string, d: unknown) => void } | undefined; if (typeof metadata?.registerInMemory === 'function') { - for (const ds of dsList) { + // [#21889] Each one stamped with the package that declares + // it (ADR-0010 `_packageId` / `_packageVersion`), through the + // one stamping helper both other load paths use. See + // `codeDefinedDatasourceOwners` for who that package is. + for (const { datasource: ds, owner } of this.codeDefinedDatasourceOwners(ctx)) { if (!ds?.name) continue; - metadata.registerInMemory('datasource', ds.name, { ...ds, origin: 'code' }); + metadata.registerInMemory( + 'datasource', + ds.name, + applyProtection({ ...ds, origin: 'code' }, owner), + ); } ctx.logger.info('Registered code-defined datasources in metadata registry', { appId, diff --git a/packages/services/service-datasource/src/__tests__/external-metadata-read-at-use.test.ts b/packages/services/service-datasource/src/__tests__/external-metadata-read-at-use.test.ts index 2f7ce50f93b..765b4997b8d 100644 --- a/packages/services/service-datasource/src/__tests__/external-metadata-read-at-use.test.ts +++ b/packages/services/service-datasource/src/__tests__/external-metadata-read-at-use.test.ts @@ -24,9 +24,6 @@ * the one every reader asks: * - validate compares each federated object, and a drifted column is reported; * - the boot gate's sweep lists every federated object; - * - the draft's name carries the namespace of the datasource's package; - * - the import's explicit name is held to that namespace, and refused when - * it breaks it; * - the refreshed catalog is persisted; * - the remote-table list honours the datasource's `allowedSchemas`. * The service is asked again at each use, never remembered from the first @@ -34,6 +31,11 @@ * gets the same answers. With no `'metadata'` service at all, every reader * keeps its documented fallback. * + * [#21889] The namespace half (the draft's prefix and the import's name + * check) moved to `external-namespace-reads-engine-registry.test.ts`: the + * package record it needs lives in the engine registry, not in the metadata + * service, so a `package` map seeded here pinned a store no composition fills. + * * The plugin is imported through a RELATIVE specifier, so these cases measure * `src/` and need no build. The booted-stack half (the showcase on a * composition with no metadata plugin) is the dogfood pin's. @@ -111,7 +113,6 @@ function metadataFake( const store = new Map>([ ['datasource', new Map([['warehouse', datasource]])], ['object', new Map([[CUSTOMER.name, CUSTOMER], [ORDER.name, ORDER]])], - ['package', new Map([['com.acme.warehouse', { manifest: { id: 'com.acme.warehouse', namespace: 'wh' } }]])], ]); const typeMap = (type: string) => { let map = store.get(type); @@ -212,32 +213,6 @@ describe('the federation service reads a metadata service registered after init ]); }); - it('the draft\'s name carries the namespace of the datasource\'s package', async () => { - const { service } = await startOrdering( - metadataFake({ name: 'warehouse', schemaMode: 'external', _packageId: 'com.acme.warehouse' }), - ); - - const draft = await service.generateObjectDraft('warehouse', 'customers'); - - expect(draft.name).toBe('wh_customers'); - }); - - it('the import\'s explicit name is held to that namespace, and refused when it breaks it', async () => { - const { h, service } = await startOrdering( - metadataFake({ name: 'warehouse', schemaMode: 'external', _packageId: 'com.acme.warehouse' }), - ); - const saveMetaItem = vi.fn(async () => ({ success: true })); - h.services.set('protocol', { saveMetaItem }); - - const outcome = await service - .importObject('warehouse', 'customers', { name: 'ext_customers' }) - .catch((e: unknown) => e); - - expect(outcome).toBeInstanceOf(Error); - expect(outcome).toMatchObject({ code: 'EXTERNAL_IMPORT_ERROR', status: 400 }); - expect(saveMetaItem).not.toHaveBeenCalled(); - }); - it('the refreshed catalog is persisted', async () => { const { service, metadata } = await startOrdering(); @@ -285,9 +260,9 @@ describe('the federation service reads a metadata service registered after init }); describe('control: a metadata service registered before init (the dev ordering) gives the same answers', () => { - it('validate, the sweep, the draft and the catalog write answer as under the start ordering', async () => { + it('validate, the sweep and the catalog write answer as under the start ordering', async () => { const h = harness(); - const metadata = metadataFake({ name: 'warehouse', schemaMode: 'external', _packageId: 'com.acme.warehouse' }); + const metadata = metadataFake(); h.services.set('metadata', metadata.service); const service = await federation(h); @@ -296,7 +271,6 @@ describe('control: a metadata service registered before init (the dev ordering) expect(report.results.find((r) => r.object === 'wh_customer')?.diffs).toEqual([ expect.objectContaining({ kind: 'missing_column', column: 'email' }), ]); - expect((await service.generateObjectDraft('warehouse', 'customers')).name).toBe('wh_customers'); await service.refreshCatalog('warehouse'); expect(metadata.register).toHaveBeenCalledWith('external_catalog', 'warehouse_catalog', expect.anything()); }); diff --git a/packages/services/service-datasource/src/__tests__/external-namespace-reads-engine-registry.test.ts b/packages/services/service-datasource/src/__tests__/external-namespace-reads-engine-registry.test.ts new file mode 100644 index 00000000000..bc0a0bb390c --- /dev/null +++ b/packages/services/service-datasource/src/__tests__/external-namespace-reads-engine-registry.test.ts @@ -0,0 +1,246 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#21889] The federation service reads a datasource's ADR-0028 namespace from + * the engine registry's package record, the store the publish gate reads. + * + * ## The defect this pins closed + * + * `getNamespace` took the datasource's stamped `_packageId` and then asked the + * `'metadata'` service for a `package` item under that id. No composition + * writes package records into the metadata service: they live in the engine's + * registry (`installPackage`, reached through `registerApp`), which is where + * the publish gate reads the namespace for the identical check + * (`publishPackageDrafts`, `@objectstack/metadata-protocol`). So the lookup + * missed on every composition measured, and the import door and the draft + * door never applied the prefix. This file's earlier sibling seeded a + * `package` map into its metadata fake, a store no real composition fills, + * and stayed green through the defect. + * + * ## What each case pins + * + * - the draft's name carries the namespace of the package the registry holds + * under the datasource's `_packageId`, and its source carries no + * `TODO(namespace)` note; + * - an import whose explicit name breaks that namespace is refused with + * ADR-0028's own message, before any save; a prefixed one is saved; + * - the engine is asked when it is used, so an `'objectql'` service registered + * after `init()` answers, and one registered before answers the same; + * - ⛔ one store, one id: a `package` item in the metadata service is never + * read, a datasource with no `_packageId` (or the `'sys_metadata'` sentinel) + * resolves nothing however many packages the registry holds, and a package + * that declares no namespace resolves nothing. + * + * The plugin is imported through a RELATIVE specifier, so these cases measure + * `src/` and need no build. The booted-stack half (the showcase's code-defined + * `showcase_external`) is the dogfood pin's. + */ + +import { describe, it, expect, vi } from 'vitest'; +import type { PluginContext } from '@objectstack/core'; +import type { IntrospectedSchema, IExternalDatasourceService } from '@objectstack/spec/contracts'; +import { validateObjectNamespacePrefix } from '@objectstack/spec/kernel'; +import { ExternalDatasourceServicePlugin } from '../plugin.js'; + +const remoteSchema = (): IntrospectedSchema => + ({ + dialect: 'sqlite', + tables: { + customers: { + name: 'customers', + columns: [ + { name: 'id', type: 'text', nullable: false, primaryKey: true }, + { name: 'name', type: 'text', nullable: true, primaryKey: false }, + ], + }, + }, + }) as unknown as IntrospectedSchema; + +const PACKAGE_ID = 'com.acme.warehouse'; + +/** A datasource as `AppPlugin` registers a code-defined one: stamped with its package. */ +const stamped = (extra: Record = {}) => ({ + name: 'warehouse', + schemaMode: 'external', + origin: 'code', + _packageId: PACKAGE_ID, + _packageVersion: '1.0.0', + _provenance: 'package', + ...extra, +}); + +/** + * A metadata service holding one datasource and, deliberately, a `package` + * item under the same id with a DIFFERENT namespace — so a reader that still + * asked it would answer `meta_customers` and be told apart. + */ +function metadataService(datasource: Record) { + const store = new Map>([ + ['datasource', new Map([['warehouse', datasource]])], + ['package', new Map([[PACKAGE_ID, { manifest: { id: PACKAGE_ID, namespace: 'meta' } }]])], + ]); + return { get: async (type: string, name: string) => store.get(type)?.get(name) }; +} + +/** The engine on the `'objectql'` slot: its registry's package records, keyed as `installPackage` keys them. */ +function engine(packages: Record }>) { + const getPackage = vi.fn((id: string) => packages[id]); + return { service: { registry: { getPackage } }, getPackage }; +} + +const WAREHOUSE = { [PACKAGE_ID]: { manifest: { id: PACKAGE_ID, namespace: 'wh' } } }; + +function harness() { + const services = new Map(); + services.set('data', { introspectDatasource: vi.fn(async () => remoteSchema()) }); + const ctx = { + getService: (name: string) => { + if (!services.has(name)) throw new Error(`Service '${name}' not found`); + return services.get(name); + }, + registerService: (name: string, service: unknown) => { + services.set(name, service); + }, + trigger: async () => undefined, + logger: { info() {}, warn() {}, error() {}, debug() {} }, + } as unknown as PluginContext; + return { ctx, services }; +} + +async function federation(h: ReturnType): Promise { + await new ExternalDatasourceServicePlugin().init(h.ctx); + return h.services.get('external-datasource') as IExternalDatasourceService; +} + +/** The `objectstack start` ordering: the plugin inits, then the services it reads arrive. */ +async function startOrdering(opts: { + datasource?: Record; + packages?: Record }>; +} = {}) { + const h = harness(); + const service = await federation(h); + const ql = engine(opts.packages ?? WAREHOUSE); + h.services.set('metadata', metadataService(opts.datasource ?? stamped())); + h.services.set('objectql', ql.service); + return { h, service, getPackage: ql.getPackage }; +} + +describe('the namespace comes from the engine registry\'s package record', () => { + it('the draft\'s name carries it, and the draft carries no TODO(namespace) note', async () => { + const { service, getPackage } = await startOrdering(); + + const draft = await service.generateObjectDraft('warehouse', 'customers'); + + expect(draft.name).toBe('wh_customers'); + expect(draft.source).not.toContain('TODO(namespace)'); + expect(getPackage).toHaveBeenCalledWith(PACKAGE_ID); + }); + + it('an explicit import name without the prefix is refused with ADR-0028\'s message, and nothing is saved', async () => { + const { h, service } = await startOrdering(); + const saveMetaItem = vi.fn(async () => ({ success: true })); + h.services.set('protocol', { saveMetaItem }); + + const outcome = await service + .importObject('warehouse', 'customers', { name: 'ext_customers' }) + .catch((e: unknown) => e); + + expect(outcome).toMatchObject({ code: 'EXTERNAL_IMPORT_ERROR', status: 400 }); + // ADR-0028's message is the shared validator's own (the one `defineStack()` + // and the publish gate raise), and it names the prefixed name to use. + expect((outcome as Error).message).toBe(validateObjectNamespacePrefix('ext_customers', 'wh')); + expect((outcome as Error).message).toContain("'wh_ext_customers'"); + expect(saveMetaItem).not.toHaveBeenCalled(); + }); + + it('control: a prefixed import name is saved under that name', async () => { + const { h, service } = await startOrdering(); + const saveMetaItem = vi.fn(async () => ({ success: true })); + h.services.set('protocol', { saveMetaItem }); + + const result = await service.importObject('warehouse', 'customers', { name: 'wh_ext_customers' }); + + expect(result.name).toBe('wh_ext_customers'); + expect(saveMetaItem).toHaveBeenCalledWith(expect.objectContaining({ type: 'object', name: 'wh_ext_customers' })); + }); + + it('control: an import with no name override saves the prefixed name the draft derives', async () => { + const { h, service } = await startOrdering(); + const saveMetaItem = vi.fn(async () => ({ success: true })); + h.services.set('protocol', { saveMetaItem }); + + const result = await service.importObject('warehouse', 'customers'); + + expect(result.name).toBe('wh_customers'); + expect(saveMetaItem).toHaveBeenCalledWith(expect.objectContaining({ name: 'wh_customers' })); + }); + + it('an engine registered before init (the dev ordering) gives the same answer', async () => { + const h = harness(); + h.services.set('metadata', metadataService(stamped())); + h.services.set('objectql', engine(WAREHOUSE).service); + const service = await federation(h); + + expect((await service.generateObjectDraft('warehouse', 'customers')).name).toBe('wh_customers'); + }); + + it('the engine is asked at each use: one registered after the first draft is the one read', async () => { + const { h, service } = await startOrdering(); + expect((await service.generateObjectDraft('warehouse', 'customers')).name).toBe('wh_customers'); + + h.services.set('objectql', engine({ [PACKAGE_ID]: { manifest: { id: PACKAGE_ID, namespace: 'whx' } } }).service); + + expect((await service.generateObjectDraft('warehouse', 'customers')).name).toBe('whx_customers'); + }); +}); + +describe('⛔ one store, one id: nothing else resolves a namespace', () => { + it('a `package` item in the metadata service is never read: with no registry record the name is bare', async () => { + const { service } = await startOrdering({ packages: {} }); + + const draft = await service.generateObjectDraft('warehouse', 'customers'); + + // The metadata fake holds `namespace: 'meta'` under the same id. + expect(draft.name).toBe('customers'); + expect(draft.source).toContain('TODO(namespace)'); + }); + + it('a datasource with no `_packageId` resolves nothing, however many packages the registry holds', async () => { + const { service, getPackage } = await startOrdering({ + datasource: { name: 'warehouse', schemaMode: 'external', origin: 'runtime' }, + }); + + expect((await service.generateObjectDraft('warehouse', 'customers')).name).toBe('customers'); + expect(getPackage).not.toHaveBeenCalled(); + }); + + it('the `sys_metadata` rehydration sentinel is not a package', async () => { + const { service, getPackage } = await startOrdering({ + datasource: stamped({ _packageId: 'sys_metadata' }), + packages: { sys_metadata: { manifest: { id: 'sys_metadata', namespace: 'sys' } } }, + }); + + expect((await service.generateObjectDraft('warehouse', 'customers')).name).toBe('customers'); + expect(getPackage).not.toHaveBeenCalled(); + }); + + it('a package that declares no namespace resolves nothing, and an unprefixed import name is accepted', async () => { + const { h, service } = await startOrdering({ packages: { [PACKAGE_ID]: { manifest: { id: PACKAGE_ID } } } }); + const saveMetaItem = vi.fn(async () => ({ success: true })); + h.services.set('protocol', { saveMetaItem }); + + expect((await service.generateObjectDraft('warehouse', 'customers')).name).toBe('customers'); + expect((await service.importObject('warehouse', 'customers', { name: 'ext_customers' })).name).toBe('ext_customers'); + }); + + it('with no `objectql` service at all, the draft keeps its documented fallback: a bare name and the TODO', async () => { + const h = harness(); + const service = await federation(h); + h.services.set('metadata', metadataService(stamped())); + + const draft = await service.generateObjectDraft('warehouse', 'customers'); + + expect(draft.name).toBe('customers'); + expect(draft.source).toContain('TODO(namespace)'); + }); +}); diff --git a/packages/services/service-datasource/src/plugin.ts b/packages/services/service-datasource/src/plugin.ts index 0ba69670c69..e1a8fb94762 100644 --- a/packages/services/service-datasource/src/plugin.ts +++ b/packages/services/service-datasource/src/plugin.ts @@ -24,7 +24,7 @@ import { // the first time instead of silently dead. /** - * The `'metadata'` service members this plugin reads: datasource and package + * The `'metadata'` service members this plugin reads: datasource * definitions, and the catalog write. Objects are not read here — see * `objectRegistry` in {@link ExternalDatasourceServicePlugin.init}. */ @@ -75,7 +75,7 @@ export class ExternalDatasourceServicePlugin implements Plugin { const engine = safeGetService(ctx, 'data'); /** - * [#21876] The `'metadata'` service: every datasource and package read + * [#21876] The `'metadata'` service: every datasource read * below, and the catalog write, go through it. Objects are read from the * engine's registry instead ([#21842], `objectRegistry` below). * @@ -218,13 +218,22 @@ export class ExternalDatasourceServicePlugin implements Plugin { * Both links are read, not assumed: * - `_packageId` is stamped onto every registered metadata item that has * package coords (`applyProtection`, `@objectstack/spec/shared`), by - * both load paths — the artifact loader and `registry.registerItem`. + * every load path — the artifact loader, `registry.registerItem`, and + * `AppPlugin`'s registration of a code-defined datasource, which + * stamps the package body that declares it ([#21889]). * `'sys_metadata'` is the rehydration sentinel, not a real package, so * it is excluded exactly as the registry's own `isCodeArtifactBody` * excludes it. * - the package record is what `installPackage` stored under - * `manifest.id`, i.e. the same `{ manifest }` shape the runtime publish - * gate reads for this identical check. + * `manifest.id` in the engine registry (`registry.getPackage` on the + * `'objectql'` service), the store the runtime publish gate reads for + * this identical check (`publishPackageDrafts`, + * `@objectstack/metadata-protocol`). [#21889] It used to be asked of + * the `'metadata'` service, which no composition writes package + * records into, so no datasource ever resolved a namespace. ⛔ One + * store, one id: the id comes only from `_packageId`, never guessed, + * and no second store is consulted when the registry has no record. + * The engine is resolved where it is used, like `metadata()` above. * * Every step is allowed to come up empty (a DB-only datasource, a * GitOps deployment with no package registry, a legacy package that @@ -233,15 +242,16 @@ export class ExternalDatasourceServicePlugin implements Plugin { */ getNamespace: async (datasource: string) => { try { - const service = metadata(); - const ds = (await service?.get('datasource', datasource)) as + const ds = (await metadata()?.get('datasource', datasource)) as | { _packageId?: unknown } | undefined; const pkgId = typeof ds?._packageId === 'string' ? ds._packageId : undefined; if (!pkgId || pkgId === 'sys_metadata') return undefined; - const pkg = (await service?.get('package', pkgId)) as - | { manifest?: { namespace?: unknown } } - | undefined; + const registry = safeGetService<{ registry?: { getPackage?: (id: string) => unknown } }>( + ctx, + 'objectql', + )?.registry; + const pkg = registry?.getPackage?.(pkgId) as { manifest?: { namespace?: unknown } } | undefined; const ns = pkg?.manifest?.namespace; return typeof ns === 'string' ? ns : undefined; } catch {