Skip to content

Commit 64eb6ff

Browse files
committed
fix(service-datasource): Test reaches the primary datasource's live driver; Sync refuses it with the true reason
The Datasources page read the primary datasource as connected (true: its driver is live under its natural name, ADR-0062), while Test answered ok:false and Sync 400, both saying it had no registered driver. - registeredDriverName(): the default-driver resolver disconnect() applied, extracted so the Test probe reuses it instead of restating it. - testConnection(): with a liveDriver wired, the primary datasource and a driver without introspectSchema (memory, mongodb) answer checkHealth(); the latter says introspection: 'unsupported'. Every other datasource is tested as before. - listRemoteTables / generateObjectDraft / importObject / refreshCatalog refuse the primary datasource with the same 400 and code as before, and a true reason: it is the platform's managed storage, not a federation source (ADR-0015 section 1). Claude-Session: https://claude.ai/code/session_01UmqzD97e14iws6hmVhgZ5y Co-authored-by: Claude <noreply@anthropic.com>
1 parent 08d201a commit 64eb6ff

3 files changed

Lines changed: 155 additions & 4 deletions

File tree

‎packages/services/service-datasource/src/datasource-connection-service.ts‎

Lines changed: 34 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -219,6 +219,38 @@ export function availabilityOf(status: ConnectStatus): DatasourceAvailability {
219219
}
220220
}
221221

222+
/**
223+
* The host-reserved name of the platform's primary datasource (ADR-0062,
224+
* #3826): the one {@link DatasourceConnectionService.connect} connects
225+
* `asDefault`, and the one runtime-admin create and app bundles may not use.
226+
*/
227+
export const DEFAULT_DATASOURCE_NAME = 'default';
228+
229+
/**
230+
* The name the engine's driver registry holds `datasource`'s driver under —
231+
* this package's ONE spelling of ADR-0062's default-driver rule.
232+
*
233+
* Every datasource but the default is registered under its own name
234+
* ({@link DatasourceConnectionService.connect} stamps it). The default keeps
235+
* its driver's NATURAL name and registers with `isDefault: true`, so
236+
* `getDriverByName('default')` can never find it: it is reached through the
237+
* engine's default-driver name instead. `undefined` when the default is asked
238+
* for and the engine has no default driver (or cannot say).
239+
*
240+
* `asDefault` defaults to "is this the reserved name". `disconnect` passes its
241+
* own flag through unchanged; the saved-datasource Test probe
242+
* (`ExternalDatasourceServicePlugin`, #24772) asks by name. ⛔ Never a second
243+
* copy of this rule at a call site: a copy is how the Test probe came to answer
244+
* "no registered driver" for a default that was live.
245+
*/
246+
export function registeredDriverName(
247+
engine: Pick<ConnectionEngineLike, 'getDefaultDriverName'> | undefined,
248+
datasource: string,
249+
asDefault: boolean = datasource === DEFAULT_DATASOURCE_NAME,
250+
): string | undefined {
251+
return asDefault ? engine?.getDefaultDriverName?.() : datasource;
252+
}
253+
222254
/**
223255
* ADR-0062 D2 — is this declared datasource "meaningfully addressed", such that
224256
* auto-connecting it is safe and intended?
@@ -714,7 +746,7 @@ export class DatasourceConnectionService {
714746
*/
715747
async disconnect(name: string, opts: { asDefault?: boolean } = {}): Promise<void> {
716748
const engine = this.cfg.engine();
717-
const driverName = opts.asDefault ? engine?.getDefaultDriverName?.() : name;
749+
const driverName = registeredDriverName(engine, name, opts.asDefault === true);
718750
// [commit 77b91bdb4] Cast-free: `getDriverByName` now answers the contract's
719751
// `IDataDriver | undefined` instead of a locally re-declared `unknown`.
720752
// The `typeof … === 'function'` guard below stays — a host-built driver in
@@ -883,7 +915,7 @@ export class DatasourceConnectionService {
883915
.filter(([, st]) => st.status === 'connected')
884916
.map(([n]) => n);
885917
for (const n of names) {
886-
await this.disconnect(n, { asDefault: n === 'default' });
918+
await this.disconnect(n, { asDefault: n === DEFAULT_DATASOURCE_NAME });
887919
}
888920
}
889921

‎packages/services/service-datasource/src/external-datasource-service.ts‎

Lines changed: 102 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,10 @@ import type {
2323
IntrospectedSchema,
2424
IntrospectedTable,
2525
IntrospectedColumn,
26+
IDataDriver,
2627
} from '@objectstack/spec/contracts';
2728
import type { SchemaDiffEntry } from '@objectstack/spec/shared';
29+
import { DEFAULT_DATASOURCE_NAME } from './datasource-connection-service.js';
2830
import {
2931
suggestFieldTypeForSqlType,
3032
isCompatible,
@@ -77,6 +79,18 @@ export interface Logger {
7779
export interface ExternalDatasourceServiceConfig {
7880
/** Introspect a datasource's live schema via its driver. */
7981
introspect: (datasource: string) => Promise<IntrospectedSchema>;
82+
/**
83+
* [#24772] The live driver registered for `datasource`, resolved by
84+
* ADR-0062's default-driver rule (`registeredDriverName` in
85+
* `datasource-connection-service.ts`), or `undefined` when none is
86+
* registered. {@link ExternalDatasourceService.testConnection} reads it to
87+
* probe a datasource that `introspect` cannot answer for: the platform's
88+
* primary datasource, and a driver with no `introspectSchema`.
89+
*
90+
* Optional. A host that wires only `introspect` keeps Test as the
91+
* introspection round trip it always was.
92+
*/
93+
liveDriver?: (datasource: string) => Pick<IDataDriver, 'checkHealth' | 'introspectSchema'> | undefined;
8094
/** Resolve a datasource definition by name. */
8195
getDatasource: (name: string) => Promise<DatasourceLike | undefined>;
8296
/** Resolve one object definition by name. */
@@ -421,6 +435,43 @@ function importNameRefusedError(message: string): Error {
421435
return err;
422436
}
423437

438+
/**
439+
* [#24772] The refusal every federation door answers for the platform's
440+
* primary datasource: listing its remote tables, drafting or importing one,
441+
* refreshing its catalog.
442+
*
443+
* ADR-0015 §1 puts federation on datasources whose `schemaMode` is not
444+
* `managed`. The primary datasource is the platform's own managed storage:
445+
* every table in it was created from an object the platform already serves.
446+
* These doors used to answer the engine's "has no registered driver to
447+
* introspect" for it, which is false (the driver is live; ADR-0062 registers
448+
* it under its natural name), while the Datasources page reads it as
449+
* connected. The answer is still a refusal, with the same status and code
450+
* these doors served before; only the reason is now the true one.
451+
*
452+
* ⛔ Not "make introspection reach the default". Listing and importing the
453+
* primary database's tables would let an import federate a platform table, and
454+
* the engine's write gate exempts the default datasource, so nothing would
455+
* stand behind it.
456+
*
457+
* `code` is the one the calling door serves (`EXTERNAL_IMPORT_ERROR` for the
458+
* import, `EXTERNAL_DATASOURCE_ERROR` for the rest), so the declaration and
459+
* the served envelope agree, as for `importNameRefusedError` above.
460+
*/
461+
function primaryDatasourceNotFederatedError(
462+
datasource: string,
463+
code: 'EXTERNAL_DATASOURCE_ERROR' | 'EXTERNAL_IMPORT_ERROR',
464+
): Error {
465+
const err = new Error(
466+
`Datasource '${datasource}' is the platform's managed primary datasource, not a federation source: ` +
467+
`its tables are the platform's own objects, so there are no remote tables to list, draft or import ` +
468+
`from it. Sync objects from an external datasource instead.`,
469+
) as Error & { code?: string; status?: number };
470+
err.code = code;
471+
err.status = 400;
472+
return err;
473+
}
474+
424475
/** snake_case → Title Case label. */
425476
function toLabel(name: string): string {
426477
return name
@@ -454,12 +505,16 @@ export class ExternalDatasourceService implements IExternalDatasourceService {
454505
* whose schema the driver did not name is never let through a filter it
455506
* cannot be judged by. With no filter, every introspected table is listed.
456507
* See {@link unattributedSchemaFilterError} for why the in-tree SQL driver
457-
* lands on the refusal.
508+
* lands on the refusal. The platform's primary datasource is refused
509+
* outright: see {@link primaryDatasourceNotFederatedError}.
458510
*/
459511
async listRemoteTables(
460512
datasource: string,
461513
opts?: { schema?: string },
462514
): Promise<RemoteTable[]> {
515+
if (datasource === DEFAULT_DATASOURCE_NAME) {
516+
throw primaryDatasourceNotFederatedError(datasource, 'EXTERNAL_DATASOURCE_ERROR');
517+
}
463518
const [schema, ds] = await Promise.all([
464519
this.config.introspect(datasource),
465520
this.config.getDatasource(datasource),
@@ -504,12 +559,46 @@ export class ExternalDatasourceService implements IExternalDatasourceService {
504559
* structured result rather than throwing so the route can render ok/error
505560
* uniformly. This backs the `datasource` `test_connection` action
506561
* (`POST /datasources/:name/test`).
562+
*
563+
* [#24772] Two datasources the introspection round trip cannot test, which
564+
* answered `ok: false` while the Datasources page read them as connected.
565+
* With a `liveDriver` wired, both take the driver's own connectivity probe,
566+
* `checkHealth()`, instead:
567+
*
568+
* - the platform's PRIMARY datasource. Its driver is live under its natural
569+
* name (ADR-0062), and it is not a federation source, so its tables are not
570+
* introspected: that costs a round trip per table of the platform database
571+
* and answers nothing the connection question needs. No `tableCount`.
572+
* - a driver with no `introspectSchema` (memory, mongodb). The answer says
573+
* so in `introspection: 'unsupported'`, rather than calling a connected
574+
* datasource failed.
575+
*
576+
* Every other datasource is tested exactly as before. So is one with no live
577+
* driver: `introspect` names the absence itself.
507578
*/
508579
async testConnection(
509580
datasource: string,
510-
): Promise<{ ok: boolean; latencyMs?: number; tableCount?: number; error?: string }> {
581+
): Promise<{
582+
ok: boolean;
583+
latencyMs?: number;
584+
tableCount?: number;
585+
introspection?: 'unsupported';
586+
error?: string;
587+
}> {
511588
const started = Date.now();
512589
try {
590+
const driver = this.config.liveDriver?.(datasource);
591+
const canIntrospect = typeof driver?.introspectSchema === 'function';
592+
if (driver && (datasource === DEFAULT_DATASOURCE_NAME || !canIntrospect)) {
593+
if (!(await driver.checkHealth())) {
594+
return { ok: false, error: `Datasource '${datasource}' is registered, but its driver failed its health check.` };
595+
}
596+
return {
597+
ok: true,
598+
latencyMs: Date.now() - started,
599+
...(canIntrospect ? {} : { introspection: 'unsupported' as const }),
600+
};
601+
}
513602
const schema = await this.config.introspect(datasource);
514603
return {
515604
ok: true,
@@ -526,6 +615,9 @@ export class ExternalDatasourceService implements IExternalDatasourceService {
526615
remoteName: string,
527616
opts: GenerateDraftOpts = {},
528617
): Promise<ObjectDraft> {
618+
if (datasource === DEFAULT_DATASOURCE_NAME) {
619+
throw primaryDatasourceNotFederatedError(datasource, 'EXTERNAL_DATASOURCE_ERROR');
620+
}
529621
const schema = await this.config.introspect(datasource);
530622
const table = this.findTable(schema, remoteName);
531623
if (!table) {
@@ -615,6 +707,11 @@ export class ExternalDatasourceService implements IExternalDatasourceService {
615707
remoteName: string,
616708
opts: ImportObjectOpts = {},
617709
): Promise<ImportObjectResult> {
710+
// [#24772] First: no store configuration makes the primary datasource a
711+
// federation source, so its refusal is the true answer whatever else holds.
712+
if (datasource === DEFAULT_DATASOURCE_NAME) {
713+
throw primaryDatasourceNotFederatedError(datasource, 'EXTERNAL_IMPORT_ERROR');
714+
}
618715
if (!this.config.persistObject) {
619716
throw new Error(
620717
`importObject requires a writable metadata store, but none is wired ` +
@@ -667,6 +764,9 @@ export class ExternalDatasourceService implements IExternalDatasourceService {
667764
}
668765

669766
async refreshCatalog(datasource: string): Promise<ExternalCatalog> {
767+
if (datasource === DEFAULT_DATASOURCE_NAME) {
768+
throw primaryDatasourceNotFederatedError(datasource, 'EXTERNAL_DATASOURCE_ERROR');
769+
}
670770
const schema = await this.config.introspect(datasource);
671771
// Parse through the Zod schema so the persisted record is canonical
672772
// (defaults applied, shape validated) and matches the `external_catalog`

‎packages/services/service-datasource/src/plugin.ts‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import {
1010
type ObjectLike,
1111
type Logger,
1212
} from './external-datasource-service.js';
13+
import { registeredDriverName } from './datasource-connection-service.js';
1314

1415
// The structural `DataEngineLike` re-declaration that used to live here is
1516
// DELETED (#11493, part of the fix by the maintainer ruling): the `'data'`
@@ -139,8 +140,26 @@ export class ExternalDatasourceServicePlugin implements Plugin {
139140
: undefined;
140141
};
141142

143+
/**
144+
* [#24772] The live driver behind a datasource name, for the saved
145+
* datasource's Test probe: the engine's registry entry under the name
146+
* {@link registeredDriverName} resolves. That is the resolver
147+
* `DatasourceConnectionService.disconnect` applies, reused rather than
148+
* restated, because the primary datasource's driver is registered under
149+
* its natural name (ADR-0062) and a lookup by `'default'` finds nothing.
150+
*
151+
* Resolved where it is used, never at `init()`, for the reason
152+
* `metadataSaveDoor` is (AGENTS.md, "Startup registry reads").
153+
*/
154+
const liveDriver: ExternalDatasourceServiceConfig['liveDriver'] = (datasource) => {
155+
const data = safeGetService<IDataEngine>(ctx, 'data');
156+
const driverName = registeredDriverName(data, datasource);
157+
return driverName ? data?.getDriverByName?.(driverName) : undefined;
158+
};
159+
142160
const config: ExternalDatasourceServiceConfig = {
143161
introspect,
162+
liveDriver,
144163
getDatasource: async (n) => (await metadata()?.get('datasource', n)) as DatasourceLike | undefined,
145164
// [#21842] Both object reads come from the one registry, so the sweep
146165
// judges the very definitions it listed. See `objectRegistry`.

0 commit comments

Comments
 (0)