Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 21 additions & 0 deletions .changeset/import-object-name-override-namespace-prefix.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
---
"@objectstack/service-datasource": patch
---

`importObject` now refuses an explicit `opts.name` that violates the ADR-0028
namespace-prefix rule (#11061). The override used to be taken verbatim
(`opts.name ?? draft.name`) and persisted through `metadata.register('object',
…)` — the one runtime write path no namespace gate looks at — so
`POST /api/v1/datasources/:name/external/tables/:remote/import` with
`{"name": "customers"}` minted an unprefixed federated object that
`defineStack()` and the publish pre-flight would both have refused.

The refusal answers `400 EXTERNAL_IMPORT_ERROR` (the family's registered
ADR-0112 code, in the #8016 thrown-refusal shape) carrying
`validateObjectNamespacePrefix`'s own actionable message — the same text the
publish gate serves for the identical violation, e.g. `Object 'customers' is
missing the package namespace prefix. Rename it to 'wh_customers' (namespace =
'wh').` A compliant override (`wh_customers`), a `sys_*` platform-reserved
name, and any override on a datasource whose package resolves no namespace are
accepted exactly as before; the derived-name path (no `name` in the body) is
unchanged.
Original file line numberDiff line numberDiff line change
Expand Up@@ -2,6 +2,10 @@

import { describe, it, expect } from 'vitest';
import type { IntrospectedSchema } from '@objectstack/spec/contracts';
// The prefix rule's ONE authored message source — refusal pins below compute
// their expected text from it rather than copying strings, so the assertion
// can never drift from the rule (#11061).
import { validateObjectNamespacePrefix } from '@objectstack/spec/kernel';
import {
ExternalDatasourceService,
type DatasourceLike,
Expand DownExpand Up@@ -219,6 +223,106 @@ describe('importObject', () => {
});
});

/**
* ADR-0028 namespace prefix on the `opts.name` OVERRIDE path (#11061).
*
* The derived path resolves the datasource's package namespace and prefixes
* the generated name; `opts.name` used to be taken verbatim, so a caller could
* persist an unprefixed federated object through `metadata.register` — the one
* runtime write path with no namespace gate. The override now answers to the
* SAME rule (loud refusal — the publish pre-flight's treatment of the
* identical violation — never a silent rewrite).
*
* Refusal pins assert the ADR-0112 envelope declaration (`status` + `code`,
* the #8016 thrown-refusal shape) AND that nothing was persisted or
* introspected; the message is asserted byte-equal to
* `validateObjectNamespacePrefix`'s own return, computed — not copied — so the
* rule keeps exactly one authored message everywhere it fires.
*/
describe('importObject — namespace prefix on the name override (#11061)', () => {
function makeNamespacedImporter(namespace?: string) {
const persisted: Array<{ name: string; def: Record<string, unknown> }> = [];
let introspectCalls = 0;
const svc = new ExternalDatasourceService({
introspect: async () => {
introspectCalls += 1;
return warehouseSchema();
},
getDatasource: async () => ({ name: 'warehouse', schemaMode: 'external' }),
getObject: async () => undefined,
listObjects: async () => [],
persistObject: async (name, def) => { persisted.push({ name, def }); },
getNamespace: async () => namespace,
});
return { svc, persisted, introspectCalls: () => introspectCalls };
}

it('refuses an unprefixed override with the rule\'s own message — 400 EXTERNAL_IMPORT_ERROR, nothing persisted, nothing introspected', async () => {
const { svc, persisted, introspectCalls } = makeNamespacedImporter('wh');
const err = await svc.importObject('warehouse', 'fact_orders', { name: 'orders' }).catch((e) => e);

expect(err).toBeInstanceOf(Error);
expect((err as { status?: number }).status).toBe(400);
expect((err as { code?: string }).code).toBe('EXTERNAL_IMPORT_ERROR');
// Byte-equal to the validator's own authored prescription — one rule, one
// message, at this gate and the publish gate alike.
expect((err as Error).message).toBe(validateObjectNamespacePrefix('orders', 'wh'));
expect((err as Error).message).toContain("Rename it to 'wh_orders'");
// The service was never persuaded: no persist, and no remote round trip —
// the verdict needs only the injected namespace read.
expect(persisted).toHaveLength(0);
expect(introspectCalls()).toBe(0);
});

it('refuses the legacy FQN form (ns__short) with the validator\'s legacy-form message', async () => {
const { svc, persisted } = makeNamespacedImporter('wh');
const err = await svc.importObject('warehouse', 'fact_orders', { name: 'wh__orders' }).catch((e) => e);

expect(err).toBeInstanceOf(Error);
expect((err as { status?: number }).status).toBe(400);
expect((err as { code?: string }).code).toBe('EXTERNAL_IMPORT_ERROR');
expect((err as Error).message).toBe(validateObjectNamespacePrefix('wh__orders', 'wh'));
expect(persisted).toHaveLength(0);
});

it('accepts a compliant override verbatim — no double prefixing, persisted as asked', async () => {
const { svc, persisted } = makeNamespacedImporter('wh');
const result = await svc.importObject('warehouse', 'fact_orders', { name: 'wh_orders' });
expect(result.name).toBe('wh_orders');
expect(persisted).toHaveLength(1);
expect(persisted[0].name).toBe('wh_orders');
});

it('accepts a sys_* override — the platform-reserved carve-out every gate on this rule shares', async () => {
const { svc, persisted } = makeNamespacedImporter('wh');
const result = await svc.importObject('warehouse', 'fact_orders', { name: 'sys_orders' });
expect(result.name).toBe('sys_orders');
expect(persisted).toHaveLength(1);
});

it('skips the rule when no namespace resolves — an unprefixed override stays accepted (mirrors defineStack)', async () => {
const { svc, persisted } = makeNamespacedImporter(undefined);
const result = await svc.importObject('warehouse', 'fact_orders', { name: 'orders' });
expect(result.name).toBe('orders');
expect(persisted).toHaveLength(1);
});

it('treats a blank namespace as absent — same normalisation as the derived path', async () => {
const { svc, persisted } = makeNamespacedImporter(' ');
const result = await svc.importObject('warehouse', 'fact_orders', { name: 'orders' });
expect(result.name).toBe('orders');
expect(persisted).toHaveLength(1);
});

it('still prefixes the DERIVED name under a namespace when no override is passed', async () => {
const { svc, persisted } = makeNamespacedImporter('wh');
const result = await svc.importObject('warehouse', 'fact_orders');
expect(result.name).toBe('wh_fact_orders');
expect(persisted).toHaveLength(1);
expect(persisted[0].name).toBe('wh_fact_orders');
});
});

describe('validateObject', () => {
const baseObject: ObjectLike = {
name: 'wh_order',
Expand Down
Original file line numberDiff line numberDiff line change
Expand Up@@ -298,6 +298,37 @@ function applyNamespacePrefix(objectName: string, namespace: string | undefined)
: `${namespace}_${objectName}`;
}

/**
* The refusal `importObject` answers when an explicit `opts.name` violates the
* ADR-0028 namespace-prefix rule.
*
* REFUSED, never silently rewritten: the derived path above may prefix its own
* invention (`applyNamespacePrefix` adjusts a name the service itself derived),
* but an override is the caller's explicit input, and every other gate on this
* rule — `defineStack()` at compile time, the publish pre-flight at runtime
* (`NAMESPACE_PREFIX`) — refuses a violating name rather than editing it.
* Persisting a rewritten name behind a 201 would leave the caller holding a
* name that does not exist, which is worse than the 400 (and exactly the
* tolerant-consumer accommodation Prime Directive #12 forbids).
*
* `message` is `validateObjectNamespacePrefix`'s own authored text, verbatim —
* the same prescription the publish gate serves for the identical violation,
* so one rule keeps one message everywhere it fires.
*
* The throw carries its own `status`/`code` — the #8016 declaration shape
* (`resolveThrownHttpError` reads them): this is a *refusal*, not a fault.
* `EXTERNAL_IMPORT_ERROR` is the ledger's registered code for a refused
* federated import, and 400 + that code is also exactly what the REST import
* route answers for any `importObject` throw, so the declaration and the
* served envelope agree by construction.
*/
function importNameRefusedError(message: string): Error {
const err = new Error(message) as Error & { code?: string; status?: number };
err.code = 'EXTERNAL_IMPORT_ERROR';
err.status = 400;
return err;
}

/** snake_case → Title Case label. */
function toLabel(name: string): string {
return name
Expand DownExpand Up@@ -471,6 +502,24 @@ export class ExternalDatasourceService implements IExternalDatasourceService {
);
}

// ADR-0028 invariant on the OVERRIDE path. The derived path below resolves
// the datasource's package namespace and prefixes the name it generates;
// `opts.name` used to be taken verbatim, so a caller could persist an
// unprefixed federated object through the one runtime write path no gate
// looks at (`metadata.register` applies no namespace check — the checks
// live at `defineStack()` and the publish pre-flight, and an imported
// object passes through neither). Same normalisation, same validator, so
// the override answers to exactly the rule the derived name already obeys;
// when no namespace resolves the rule is skipped, mirroring `defineStack`.
// Checked BEFORE the draft on purpose: the verdict needs only the injected
// namespace read, and a doomed request should not cost a live remote
// introspection round trip.
if (opts.name !== undefined) {
const namespace = normaliseNamespace(await this.config.getNamespace?.(datasource));
const violation = validateObjectNamespacePrefix(opts.name, namespace);
if (violation) throw importNameRefusedError(violation);
}

// Reuse the draft pipeline (type mapping, review notes, external binding).
const draft = await this.generateObjectDraft(datasource, remoteName, opts);

Expand Down
Loading