From 88e7fb015e1311e4af96aebfe33a154c27ed774a Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 29 Oct 2026 18:22:31 +0000 Subject: [PATCH] fix(service-datasource, rest): Sync doors answer an unknown datasource name 404 RESOURCE_NOT_FOUND The federation guard folded a name with no datasource definition into the "not a federation source" 400. It now throws the package's one datasourceNotFoundError (the two private copies are merged into one helper in datasource-admin-service.ts, not exported from the package index), and the rest federation twins relay that 404 envelope as the admin routes do. A definition that is not a federation source keeps its 400. Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_01L6bhgXTXpQTz45k8v4WACt --- ...-datasource-sync-doors-unknown-name-404.md | 13 ++ .../rest/src/external-datasource-routes.ts | 13 ++ .../external-datasource-unknown-name.test.ts | 120 +++++++++++++ .../external-metadata-read-at-use.test.ts | 14 +- .../federation-doors-unknown-name.test.ts | 166 ++++++++++++++++++ .../managed-datasource-sync-refusal.test.ts | 12 +- .../service-datasource/src/admin-routes.ts | 13 +- .../src/datasource-admin-service.ts | 14 +- .../src/external-datasource-service.ts | 50 +++--- 9 files changed, 369 insertions(+), 46 deletions(-) create mode 100644 .changeset/27618-datasource-sync-doors-unknown-name-404.md create mode 100644 packages/rest/src/external-datasource-unknown-name.test.ts create mode 100644 packages/services/service-datasource/src/__tests__/federation-doors-unknown-name.test.ts diff --git a/.changeset/27618-datasource-sync-doors-unknown-name-404.md b/.changeset/27618-datasource-sync-doors-unknown-name-404.md new file mode 100644 index 00000000000..f261966782a --- /dev/null +++ b/.changeset/27618-datasource-sync-doors-unknown-name-404.md @@ -0,0 +1,13 @@ +--- +'@objectstack/service-datasource': patch +'@objectstack/rest': patch +--- + +fix(service-datasource, rest): the datasource Sync doors and their federation twins answer a name that no saved datasource has `404 RESOURCE_NOT_FOUND`, as `GET /api/v1/datasources/:name` does, instead of a `400` federation refusal + +Clause-②: no +Metadata-protocol: none + +- **Before.** `GET /api/v1/datasources/:name/remote-tables`, `POST /api/v1/datasources/:name/object-draft`, and the federation twins under `/api/v1/datasources/:name/external/` (`GET tables`, `POST tables/:remote/draft`, `POST tables/:remote/import`, `POST refresh-catalog`) answered a name that resolves no saved datasource `400 EXTERNAL_DATASOURCE_ERROR` (`EXTERNAL_IMPORT_ERROR` for the import) with "has no datasource definition, so nothing declares it a federation source …". That reads as a malformed request, or as advice to re-declare the datasource `external`, when the datasource does not exist. +- **Now.** They answer `404` with `error.code` `RESOURCE_NOT_FOUND` and the message `GET /api/v1/datasources/:name` gives for the same name (`Datasource "NAME" does not exist.`). In process, `ExternalDatasourceService.listRemoteTables`, `generateObjectDraft`, `importObject` and `refreshCatalog` throw that `404` error for such a name. Every by-name datasource door now spells this answer through one helper. +- **Unchanged.** A datasource that exists and is not a federation source (a managed one, or the platform's primary datasource) is still refused `400` with the door's code and its reason. diff --git a/packages/rest/src/external-datasource-routes.ts b/packages/rest/src/external-datasource-routes.ts index 964627be658..1629a50c6b7 100644 --- a/packages/rest/src/external-datasource-routes.ts +++ b/packages/rest/src/external-datasource-routes.ts @@ -454,6 +454,15 @@ export function registerExternalDatasourceRoutes( * included: a sandboxed refusal declaring one, and the metadata door's * `409 DESTRUCTIVE_CHANGE` the import dogfood pins at `400`. That status is * a separate decision this does not take. + * + * [#27618] One exception, the relay `service-datasource`'s admin routes make + * for the same throw: the `404` / `RESOURCE_NOT_FOUND` envelope says the + * NAME resolves no saved datasource (the service's federation guard throws + * it for a name with no definition). It is answered as such, as + * `GET /datasources/:name` answers that name, not as this family's `400`, + * which told the caller to re-declare a datasource that does not exist. Both + * fields are required, so an unrelated error carrying a stray `status` + * cannot re-route itself. */ const answerThrown = ( res: any, @@ -466,6 +475,10 @@ export function registerExternalDatasourceRoutes( const nested = nestedRefusalEnvelope(fault); return sendError(res, nested.status, nested.code, nested.message, nested.extra); } + const envelope = err as { code?: unknown; status?: unknown } | null | undefined; + if (envelope?.status === 404 && envelope?.code === 'RESOURCE_NOT_FOUND') { + return sendError(res, 404, 'RESOURCE_NOT_FOUND', (err as Error).message); + } return sendError(res, 400, code, refusalSentence(err)); }; diff --git a/packages/rest/src/external-datasource-unknown-name.test.ts b/packages/rest/src/external-datasource-unknown-name.test.ts new file mode 100644 index 00000000000..b2e2f5b56f7 --- /dev/null +++ b/packages/rest/src/external-datasource-unknown-name.test.ts @@ -0,0 +1,120 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * The federation twins on a name that resolves no saved datasource. + * + * `GET /datasources/:name/external/tables`, `POST …/external/tables/:remote/draft`, + * `POST …/external/tables/:remote/import` and `POST …/external/refresh-catalog` + * all run the service's federation guard first. It folded a name with no + * datasource definition into "not a federation source", and this family's + * catch answered every non-fault throw `400`, so an unknown name got the + * federation lecture as a malformed request, where `GET /datasources/:name` + * answers it `404 RESOURCE_NOT_FOUND`, `Datasource "NAME" does not exist.` + * (its words, asserted equal to the admin route's answer in + * `service-datasource`'s `federation-doors-unknown-name.test.ts`). + * + * The ruling these pins hold: no definition ⇒ that `404`, relayed by + * {@link registerExternalDatasourceRoutes}'s catch; a definition that is not a + * federation source ⇒ the door's `400` and code, unchanged. + * + * The service under the routes is the REAL `ExternalDatasourceService` (this + * package's `vitest.config.ts` aliases the specifier to its `src/`), over an + * introspector that must never be reached: every answer here is the guard's. + * Driven through the real `HonoHttpServer`. + */ + +import { describe, it, expect } from 'vitest'; +import { HonoHttpServer } from '@objectstack/plugin-hono-server'; +import { ExternalDatasourceService } from '@objectstack/service-datasource'; +import { registerExternalDatasourceRoutes } from './external-datasource-routes.js'; + +/** `warehouse` exists and is managed: it declares no `schemaMode`. */ +const DEFINITIONS: Record = { + warehouse: { name: 'warehouse' }, +}; + +function mount() { + const service = new ExternalDatasourceService({ + introspect: async () => { + throw new Error('introspect must not be reached'); + }, + getDatasource: async (name: string) => DEFINITIONS[name], + getObject: async () => undefined, + listObjects: async () => [], + persistObject: async () => { + throw new Error('persistObject must not be reached'); + }, + }); + const server = new HonoHttpServer(0); + registerExternalDatasourceRoutes( + server, + { + getService: (name: string) => { + if (name === 'external-datasource') return service; + throw new Error(`no service: ${name}`); + }, + } as any, + '/api/v1', + { resolveExecutionContext: async () => ({ userId: 'u_admin', systemPermissions: ['manage_platform_settings', 'manage_metadata'] }) }, + ); + const app = server.getRawApp(); + return async (method: 'GET' | 'POST', path: string) => { + const res = await app.fetch( + new Request(`http://local/api/v1/datasources${path}`, { + method, + headers: { 'content-type': 'application/json' }, + ...(method === 'POST' ? { body: '{}' } : {}), + }), + ); + return { status: res.status, body: (await res.json()) as any }; + }; +} + +/** Each twin, its path for `name`, and the code its refusals carry. */ +const TWINS: Array<[door: string, method: 'GET' | 'POST', path: (name: string) => string, code: string]> = [ + ['GET …/external/tables', 'GET', (n) => `/${n}/external/tables`, 'EXTERNAL_DATASOURCE_ERROR'], + ['POST …/external/tables/:remote/draft', 'POST', (n) => `/${n}/external/tables/orders/draft`, 'EXTERNAL_DATASOURCE_ERROR'], + ['POST …/external/tables/:remote/import', 'POST', (n) => `/${n}/external/tables/orders/import`, 'EXTERNAL_IMPORT_ERROR'], + ['POST …/external/refresh-catalog', 'POST', (n) => `/${n}/external/refresh-catalog`, 'EXTERNAL_DATASOURCE_ERROR'], +]; + +describe('the federation twins on a name that resolves no saved datasource', () => { + it.each(TWINS)('%s answers 404 RESOURCE_NOT_FOUND in the words GET /datasources/:name uses', async (_d, method, path) => { + const call = mount(); + + const answer = await call(method, path('no_such_ds')); + + expect({ + status: answer.status, + success: answer.body.success, + code: answer.body.error?.code, + message: answer.body.error?.message, + }).toEqual({ + status: 404, + success: false, + code: 'RESOURCE_NOT_FOUND', + message: 'Datasource "no_such_ds" does not exist.', + }); + expect(answer.body).not.toHaveProperty('data'); + }); +}); + +describe('the federation twins on a datasource that exists and is not a federation source', () => { + it.each(TWINS)('%s still refuses a managed datasource 400 with its own code', async (_d, method, path, code) => { + const call = mount(); + + const answer = await call(method, path('warehouse')); + + expect({ status: answer.status, code: answer.body.error?.code }).toEqual({ status: 400, code }); + expect(answer.body.error.message).toContain("Datasource 'warehouse' is managed"); + }); + + it.each(TWINS)('%s still refuses the primary datasource 400 with its own code', async (_d, method, path, code) => { + const call = mount(); + + const answer = await call(method, path('default')); + + expect({ status: answer.status, code: answer.body.error?.code }).toEqual({ status: 400, code }); + expect(answer.body.error.message).toContain("is the platform's managed primary datasource"); + }); +}); 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 a5f8e6c046d..bf3f2a2e595 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 @@ -288,7 +288,9 @@ describe('with no metadata service at all, every reader keeps its fallback', () // same reading validate already made of an unreadable definition ("not // federated", the empty report below), so an import can no longer bind an // object the sweep would never check. The refusal comes before any - // introspection. + // introspection. [#27618] A name with no definition is refused as every + // by-name door refuses it: `404 RESOURCE_NOT_FOUND`, not the `400` a + // definition that is not a federation source gets. it('validate and the sweep answer an empty report, and the Sync doors refuse an undeclared datasource', async () => { // [#21842] No engine registry either: that is where validate's objects // come from, so with neither service the report has nothing to list. @@ -296,11 +298,11 @@ describe('with no metadata service at all, every reader keeps its fallback', () const service = await federation(h); expect(await service.validateAll()).toEqual({ ok: true, results: [] }); - const refusal = { status: 400, message: expect.stringContaining("Datasource 'warehouse' has no datasource definition") }; - await expect(service.generateObjectDraft('warehouse', 'customers')).rejects.toMatchObject({ code: 'EXTERNAL_DATASOURCE_ERROR', ...refusal }); - await expect(service.refreshCatalog('warehouse')).rejects.toMatchObject({ code: 'EXTERNAL_DATASOURCE_ERROR', ...refusal }); - await expect(service.listRemoteTables('warehouse')).rejects.toMatchObject({ code: 'EXTERNAL_DATASOURCE_ERROR', ...refusal }); - await expect(service.importObject('warehouse', 'customers')).rejects.toMatchObject({ code: 'EXTERNAL_IMPORT_ERROR', ...refusal }); + const refusal = { status: 404, code: 'RESOURCE_NOT_FOUND', message: 'Datasource "warehouse" does not exist.' }; + await expect(service.generateObjectDraft('warehouse', 'customers')).rejects.toMatchObject(refusal); + await expect(service.refreshCatalog('warehouse')).rejects.toMatchObject(refusal); + await expect(service.listRemoteTables('warehouse')).rejects.toMatchObject(refusal); + await expect(service.importObject('warehouse', 'customers')).rejects.toMatchObject(refusal); const data = h.services.get('data') as { introspectDatasource: Mock }; expect(data.introspectDatasource).not.toHaveBeenCalled(); }); diff --git a/packages/services/service-datasource/src/__tests__/federation-doors-unknown-name.test.ts b/packages/services/service-datasource/src/__tests__/federation-doors-unknown-name.test.ts new file mode 100644 index 00000000000..9914538b10e --- /dev/null +++ b/packages/services/service-datasource/src/__tests__/federation-doors-unknown-name.test.ts @@ -0,0 +1,166 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * The Sync doors on a name that resolves no saved datasource. + * + * `GET /datasources/:name/remote-tables` and `POST /datasources/:name/object-draft` + * answered an unknown name `400 EXTERNAL_DATASOURCE_ERROR` with the federation + * lecture ("has no datasource definition, so nothing declares it a federation + * source: ADR-0015 reserves federation …"), while `GET /datasources/:name` + * answered the same name `404 RESOURCE_NOT_FOUND`. That told an admin tool, + * script or AI caller its request was malformed, or that it should re-declare + * the datasource `external`, when the datasource does not exist. + * + * The triage ruling these pins hold: a name with no definition is the one + * shared `404` every by-name door answers; a definition that is not a + * federation source keeps its `400` (a managed datasource, and the platform's + * primary one). + * + * The composition is the real one, end to end, as in + * `saved-datasource-test-unknown-name.test.ts`: a real `ObjectQL` engine, the + * real connection service, the real admin service, the real + * `ExternalDatasourceServicePlugin` wiring, and the real admin routes over a + * Hono server. Both services read the same stored records, as the two plugins + * do (`metadata.get('datasource', name)`). + */ + +import { afterEach, describe, expect, it } from 'vitest'; +import { ObjectQL } from '@objectstack/objectql'; +import { HonoHttpServer } from '@objectstack/plugin-hono-server'; +import { DatasourceConnectionService, DEFAULT_DATASOURCE_NAME } from '../datasource-connection-service.js'; +import { createDefaultDatasourceDriverFactory } from '../default-datasource-driver-factory.js'; +import { ExternalDatasourceServicePlugin } from '../plugin.js'; +import { ExternalDatasourceService } from '../external-datasource-service.js'; +import { DatasourceAdminService, type StoredDatasource } from '../datasource-admin-service.js'; +import { registerDatasourceAdminRoutes } from '../admin-routes.js'; +import { ENTITLED_CREDENTIAL, createSessionAuthService, createGrantsEngine } from './entitled-caller.fixture.js'; + +const teardowns: Array<() => Promise> = []; +afterEach(async () => { + for (const t of teardowns.splice(0)) await t(); +}); + +/** + * The saved datasources. `warehouse` is managed (it declares no `schemaMode`, + * and `managed` is the default), so it exists and is not a federation source. + * The primary datasource has no record, as `DefaultDatasourcePlugin.init()` + * leaves it. + */ +const RECORDS: StoredDatasource[] = [{ name: 'warehouse', driver: 'memory', origin: 'runtime', config: {} }]; + +async function boot() { + const ql = new ObjectQL(); + const connection = new DatasourceConnectionService({ + factory: () => createDefaultDatasourceDriverFactory({}), + engine: () => ql as never, + }); + teardowns.push(() => connection.disconnectAll()); + await connection.connect( + { name: DEFAULT_DATASOURCE_NAME, driver: 'memory', config: {}, origin: 'code', bootCritical: true }, + { asDefault: true, context: { origin: 'code', trigger: 'declared-auto' } }, + ); + await connection.connect(RECORDS[0], { context: { origin: 'runtime', trigger: 'runtime-admin' } }); + + const records = new Map(RECORDS.map((r) => [r.name, r])); + const admin = new DatasourceAdminService({ + listDatasourceRecords: async () => [...records.values()], + getDatasourceRecord: async (n: string) => records.get(n), + connectionStates: () => connection.listConnectionStates(), + } as never); + + const services = new Map([ + ['data', ql], + ['metadata', { get: async (type: string, n: string) => (type === 'datasource' ? records.get(n) : undefined) }], + ['datasource-connection', connection], + ]); + await new ExternalDatasourceServicePlugin().init({ + getService: (n: string) => { + if (!services.has(n)) throw new Error(`no service: ${n}`); + return services.get(n); + }, + registerService: (n: string, s: unknown) => void services.set(n, s), + } as never); + const external = services.get('external-datasource') as ExternalDatasourceService; + + const server = new HonoHttpServer(0); + const authService = createSessionAuthService(); + const grantsEngine = createGrantsEngine(); + registerDatasourceAdminRoutes( + server, + { + getService: (n: string) => { + if (n === 'auth') return authService; + if (n === 'objectql' || n === 'data') return grantsEngine; + if (n === 'external-datasource') return external; + if (n === 'datasource-admin') return admin; + throw new Error(`no service: ${n}`); + }, + } as never, + '/api/v1', + ); + const app = server.getRawApp(); + const call = async (method: 'GET' | 'POST', path: string, body?: unknown) => { + const res = await app.fetch( + new Request(`http://local/api/v1/datasources${path}`, { + method, + headers: { authorization: ENTITLED_CREDENTIAL, 'content-type': 'application/json' }, + ...(method === 'POST' ? { body: JSON.stringify(body ?? {}) } : {}), + }), + ); + return { status: res.status, body: (await res.json()) as any }; + }; + + return { call }; +} + +type Call = Awaited>['call']; + +/** The two Sync doors, each addressing `name`. */ +const SYNC_DOORS: Array<[door: string, ask: (call: Call, name: string) => ReturnType]> = [ + ['GET /datasources/:name/remote-tables', (call, name) => call('GET', `/${name}/remote-tables`)], + ['POST /datasources/:name/object-draft', (call, name) => call('POST', `/${name}/object-draft`, { table: 'orders' })], +]; + +describe('the Sync doors on a name that resolves no saved datasource', () => { + it.each(SYNC_DOORS)('%s answers what GET /datasources/:name answers: 404 RESOURCE_NOT_FOUND', async (_door, ask) => { + const { call } = await boot(); + + const read = await call('GET', '/no_such_ds'); + const answer = await ask(call, 'no_such_ds'); + + expect({ status: read.status, code: read.body.error?.code }).toEqual({ status: 404, code: 'RESOURCE_NOT_FOUND' }); + expect({ + status: answer.status, + success: answer.body.success, + code: answer.body.error?.code, + message: answer.body.error?.message, + }).toEqual({ status: 404, success: false, code: 'RESOURCE_NOT_FOUND', message: read.body.error.message }); + expect(answer.body).not.toHaveProperty('data'); + }); +}); + +describe('the Sync doors on a datasource that exists and is not a federation source', () => { + it.each(SYNC_DOORS)('%s still refuses a managed datasource 400 EXTERNAL_DATASOURCE_ERROR', async (_door, ask) => { + const { call } = await boot(); + + const answer = await ask(call, 'warehouse'); + + expect({ status: answer.status, code: answer.body.error?.code }).toEqual({ + status: 400, + code: 'EXTERNAL_DATASOURCE_ERROR', + }); + expect(answer.body.error.message).toContain("Datasource 'warehouse' is managed"); + }); + + it.each(SYNC_DOORS)('%s still refuses the primary datasource 400 EXTERNAL_DATASOURCE_ERROR', async (_door, ask) => { + const { call } = await boot(); + + const answer = await ask(call, DEFAULT_DATASOURCE_NAME); + + expect({ status: answer.status, code: answer.body.error?.code }).toEqual({ + status: 400, + code: 'EXTERNAL_DATASOURCE_ERROR', + }); + expect(answer.body.error.message).toContain("is the platform's managed primary datasource"); + }); +}); diff --git a/packages/services/service-datasource/src/__tests__/managed-datasource-sync-refusal.test.ts b/packages/services/service-datasource/src/__tests__/managed-datasource-sync-refusal.test.ts index 5585d986ad5..1edee520542 100644 --- a/packages/services/service-datasource/src/__tests__/managed-datasource-sync-refusal.test.ts +++ b/packages/services/service-datasource/src/__tests__/managed-datasource-sync-refusal.test.ts @@ -245,13 +245,17 @@ describe('every Sync door refuses a managed datasource', () => { ); }); - it('a name with no datasource definition is refused too: nothing declares it a federation source', async () => { + // [#27618] Refused as every by-name door refuses a name that resolves no + // saved datasource, not with this family's 400: it is not a datasource that + // fails to be a federation source, it does not exist + // (`federation-doors-unknown-name.test.ts` holds the doors equal to GET). + it('a name with no datasource definition is refused 404 RESOURCE_NOT_FOUND: it does not exist', async () => { const { call } = await boot(); const tables = await call('GET', '/ghost/remote-tables'); - expect(tables.status).toBe(400); - expect(tables.body.error.code).toBe('EXTERNAL_DATASOURCE_ERROR'); - expect(tables.body.error.message).toContain("Datasource 'ghost' has no datasource definition"); + expect(tables.status).toBe(404); + expect(tables.body.error.code).toBe('RESOURCE_NOT_FOUND'); + expect(tables.body.error.message).toBe('Datasource "ghost" does not exist.'); }); it('no refused datasource is introspected, the primary one included', async () => { diff --git a/packages/services/service-datasource/src/admin-routes.ts b/packages/services/service-datasource/src/admin-routes.ts index 664cf9d25bf..fe3f01a3029 100644 --- a/packages/services/service-datasource/src/admin-routes.ts +++ b/packages/services/service-datasource/src/admin-routes.ts @@ -45,6 +45,9 @@ import type { IAuthService, IDataEngine, IHttpServer } from '@objectstack/spec/c // The declared envelope is written in ONE place for the whole platform (#3973). import { inProcessSessionReadInput, sendOk, sendError } from '@objectstack/types'; import { DRIVER_CATALOG } from './driver-catalog.js'; +// [#27618] The package's one "no such datasource" refusal: GET answers an +// unknown name in its words, as every other by-name door does. +import { datasourceNotFoundError } from './datasource-admin-service.js'; /** * The services these routes dispatch to. @@ -86,7 +89,8 @@ const SERVICE_ERROR_CODE: Record = { * `DATASOURCE_ADMIN_ERROR` or `EXTERNAL_DATASOURCE_ERROR`. A name that resolves * no stored datasource is not a refusal: every `datasource-admin` door that * takes `:name` answers it `404` `RESOURCE_NOT_FOUND`, the read and the writes - * alike, and so does `POST /datasources/:name/test` (see `badRequest`). + * alike, and so do `POST /datasources/:name/test` and the two Sync doors, + * `GET …/remote-tables` and `POST …/object-draft` (see `badRequest`). * * Served by `datasource-admin`: * @@ -641,8 +645,9 @@ export function registerDatasourceAdminRoutes( * unknown datasource reads the same at every by-name door, and a caller can * tell "no such datasource" from "your request was refused". The throwers * today are the admin service's `updateDatasource`, `removeDatasource` and - * `migrateCredential`, and the external service's `testConnection` (each - * through its `datasourceNotFoundError`). + * `migrateCredential`, and the external service's `testConnection` and the + * federation guard behind `remote-tables` and `object-draft`, all through + * the one `datasourceNotFoundError`. */ const badRequest = (res: any, service: ServiceName, err: unknown) => { const envelope = err as { code?: unknown; status?: unknown } | null | undefined; @@ -747,7 +752,7 @@ export function registerDatasourceAdminRoutes( if (!svc) return; try { const datasource = await svc.getDatasource(req.params.name); - if (!datasource) return sendError(res, 404, 'RESOURCE_NOT_FOUND', `Datasource "${req.params.name}" does not exist.`); + if (!datasource) return sendError(res, 404, 'RESOURCE_NOT_FOUND', datasourceNotFoundError(req.params.name).message); sendOk(res, { datasource }); } catch (err) { badRequest(res, 'datasource-admin', err); diff --git a/packages/services/service-datasource/src/datasource-admin-service.ts b/packages/services/service-datasource/src/datasource-admin-service.ts index d15b6c9f309..e6cea3eff55 100644 --- a/packages/services/service-datasource/src/datasource-admin-service.ts +++ b/packages/services/service-datasource/src/datasource-admin-service.ts @@ -333,17 +333,23 @@ function withRemaining(remaining: string[]): { remaining?: string[] } { * internal datasources and tables, and the message is served to an API caller. */ /** - * The refusal every by-name lifecycle door answers when `name` resolves no - * stored datasource — `updateDatasource`, `removeDatasource` and - * `migrateCredential`. + * The refusal every by-name door answers when `name` resolves no stored + * datasource: here `updateDatasource`, `removeDatasource` and + * `migrateCredential`; in `ExternalDatasourceService`, `testConnection` and + * the federation guard every Sync door runs first; and `GET /datasources/:name` + * answers in its words. * * It carries the ADR-0112 envelope (`404` / `RESOURCE_NOT_FOUND`) so the admin * routes answer it exactly as `GET /datasources/:name` answers the same name. * A plain `Error` here was rendered `400 DATASOURCE_ADMIN_ERROR`, which tells an * SDK or AI caller its request was malformed rather than that the datasource * does not exist — and the read sibling already said the latter. + * + * [#27618] ONE helper for the package, so every by-name door names a missing + * datasource in one spelling. Exported from this module for its siblings only: + * ⛔ not from the package index, which would widen the public surface. */ -function datasourceNotFoundError(name: string): Error { +export function datasourceNotFoundError(name: string): Error { // GET's own words, so every by-name door names the missing datasource alike. const err = new Error(`Datasource "${name}" does not exist.`) as Error & { code?: string; status?: number }; err.code = 'RESOURCE_NOT_FOUND'; diff --git a/packages/services/service-datasource/src/external-datasource-service.ts b/packages/services/service-datasource/src/external-datasource-service.ts index 4750f4f6e7f..922e07c2df3 100644 --- a/packages/services/service-datasource/src/external-datasource-service.ts +++ b/packages/services/service-datasource/src/external-datasource-service.ts @@ -27,6 +27,9 @@ import type { } from '@objectstack/spec/contracts'; import type { SchemaDiffEntry } from '@objectstack/spec/shared'; import { DEFAULT_DATASOURCE_NAME, type DatasourceConnectionState } from './datasource-connection-service.js'; +// [#27618] The package's one "no such datasource" refusal, shared with the +// admin service so every by-name door answers an unknown name alike. +import { datasourceNotFoundError } from './datasource-admin-service.js'; import { suggestFieldTypeForSqlType, isCompatible, @@ -447,21 +450,6 @@ function importNameRefusedError(message: string): Error { return err; } -/** - * [#27510] What the saved-datasource Test answers for a name that resolves no - * stored datasource: the `404` / `RESOURCE_NOT_FOUND` envelope (ADR-0112), in - * the words `GET /datasources/:name` uses, so the admin route relays it as the - * read sibling answers the same name. It mirrors the admin service's helper of - * the same name; `saved-datasource-test-unknown-name.test.ts` holds the two - * doors' answers equal. - */ -function datasourceNotFoundError(name: string): Error { - const err = new Error(`Datasource "${name}" does not exist.`) as Error & { code?: string; status?: number }; - err.code = 'RESOURCE_NOT_FOUND'; - err.status = 404; - return err; -} - /** * [#24772] The refusal every federation door answers for the platform's * primary datasource: listing its remote tables, drafting or importing one, @@ -533,8 +521,10 @@ function isFederationSource(ds: DatasourceLike | undefined): ds is DatasourceLik /** * [#24837] The refusal every federation door answers for a datasource that is - * not a federation source by {@link isFederationSource}: a managed one (declared - * or by default), or a name with no datasource definition. + * not a federation source by {@link isFederationSource}: a managed one, declared + * or by default. [#27618] A name with no datasource definition is not refused + * here: it does not exist, and is answered as every by-name door answers it + * ({@link datasourceNotFoundError}). * * Until this guard, only the primary datasource was refused * ({@link primaryDatasourceNotFederatedError}). Every other managed datasource, @@ -547,14 +537,12 @@ function isFederationSource(ds: DatasourceLike | undefined): ds is DatasourceLik * Same status and codes as the primary datasource's refusal, by the same * reasoning: the code is the one the calling door serves. */ -function notAFederationSourceError(datasource: string, ds: DatasourceLike | undefined, door: FederationDoor): Error { +function notAFederationSourceError(datasource: string, ds: DatasourceLike, door: FederationDoor): Error { const code = door === 'import' ? 'EXTERNAL_IMPORT_ERROR' : 'EXTERNAL_DATASOURCE_ERROR'; - const lead = !ds - ? `Datasource '${datasource}' has no datasource definition, so nothing declares it a federation source` - : ds.schemaMode - ? `Datasource '${datasource}' is managed (schemaMode '${ds.schemaMode}'), not a federation source` - : `Datasource '${datasource}' is managed (it declares no schemaMode, and 'managed' is the default), ` + - `not a federation source`; + const lead = ds.schemaMode + ? `Datasource '${datasource}' is managed (schemaMode '${ds.schemaMode}'), not a federation source` + : `Datasource '${datasource}' is managed (it declares no schemaMode, and 'managed' is the default), ` + + `not a federation source`; const rule = `ADR-0015 reserves federation for a datasource whose schemaMode is not 'managed', ` + `because the platform owns a managed datasource's schema and its tables are the platform's own objects.`; @@ -600,16 +588,21 @@ export class ExternalDatasourceService implements IExternalDatasourceService { * tables, drafting one, importing one, refreshing the catalog. Each door * names its {@link FederationDoor} kind, which decides the refusal's code. * - * Two refusals, in this order, and nothing else: + * Three refusals, in this order, and nothing else: * * - the platform's primary datasource, by name, with its own reason * ({@link primaryDatasourceNotFederatedError}, #24772). No definition is * read for it: no store configuration makes it a federation source. + * - [#27618] a name with no datasource definition: it does not exist, so it + * is the `404` every by-name door answers ({@link datasourceNotFoundError}, + * the words `GET /datasources/:name` uses), which both route families + * relay as such. It was folded into the next refusal, a `400` that told + * the caller to re-declare a datasource that is not there. * - any other datasource that is not a federation source by - * {@link isFederationSource}: managed, declared or by default, or with no - * definition ({@link notAFederationSourceError}). + * {@link isFederationSource}: managed, declared or by default + * ({@link notAFederationSourceError}). * - * Both come before any introspection, so a refused datasource is never + * All come before any introspection, so a refused datasource is never * read: its tables are the platform's, and listing them is the very thing * refused. * @@ -619,6 +612,7 @@ export class ExternalDatasourceService implements IExternalDatasourceService { private async assertFederationSource(datasource: string, door: FederationDoor): Promise { if (datasource === DEFAULT_DATASOURCE_NAME) throw primaryDatasourceNotFederatedError(datasource, door); const ds = await this.config.getDatasource(datasource); + if (!ds) throw datasourceNotFoundError(datasource); if (!isFederationSource(ds)) throw notAFederationSourceError(datasource, ds, door); return ds; }