fix(service-datasource)!: external validate answers an unknown datasource 404 RESOURCE_NOT_FOUND

.changeset/27696-external-validate-unknown-name-404.md

@@ -0,0 +1,22 @@
+---
+'@objectstack/service-datasource': patch
+---
+
+fix(service-datasource): `POST /api/v1/datasources/:name/external/validate` answers a name no saved datasource has `404 RESOURCE_NOT_FOUND`, as `GET /datasources/:name` and the other federation doors do
+
+Clause-②: no (narrowing)
+Metadata-protocol: none
+
+<!-- adr-0087: not-required (no-migration-prescription) No key of any schema, accepted value, stored row, stored shape or export moves. What narrows is one HTTP answer: a validate request for a datasource name that does not exist now gets the 404 the sibling by-name doors already give it, instead of an empty 200 report. There is no metadata for `objectstack migrate meta` to rewrite and nothing to tombstone; the fix for a caller is to stop addressing a datasource that does not exist. -->
+
+`ExternalDatasourceService.validateDatasource(name)` selected the federated objects bound to `name` and validated them. For a name with no saved datasource it selected nothing and returned an empty, vacuously `ok` report, so the validate door answered `200 { ok: true, results: [] }` and a caller could not tell "nothing on this datasource is federated" from "this datasource does not exist".
+
+It now checks that the datasource exists first, the check `testConnection` makes (#27510): a name that resolves no saved datasource throws the package's one `datasourceNotFoundError` (`404`, `RESOURCE_NOT_FOUND`, `Datasource "NAME" does not exist.`) before anything is listed or introspected, and the REST route relays it as such. Only existence is asked, not the full federation guard: a saved datasource with nothing federated, a managed one included, and the platform's primary datasource (not looked up, as it may have no stored record) keep their `200` empty report.
+
+**Side effect.** The runtime's background drift check (`external.validation.checkIntervalMs`) also calls `validateDatasource`. For a datasource that was deleted after its timer was armed, a tick now lands in the check's existing catch (one `warn`, "drift check could not be performed", no events) where it used to return an empty report silently.
+
+**BREAKING** for a caller of `POST /api/v1/datasources/:name/external/validate`, or of `validateDatasource`, that addresses a datasource name that does not exist.
+
+- **Before.** `200` with `{ ok: true, results: [] }`; `validateDatasource` resolved to `{ ok: true, results: [] }`.
+- **Now.** `404 RESOURCE_NOT_FOUND`; `validateDatasource` rejects with that error.
+- **Fix.** Address a saved datasource, or treat the `404` as "no such datasource".

packages/rest/src/external-datasource-routes.ts

@@ -599,6 +599,11 @@ export function registerExternalDatasourceRoutes(
     // the post-filter used to keep are exactly the rows this returns (see
     // `external-datasource-validate-scope.test.ts`, which pins the two answers
     // against each other and the introspection call record beside them).
+    //
+    // [#27696] A name no saved datasource has is the service's 404
+    // `datasourceNotFoundError`, thrown before any validation and relayed as
+    // such by `refused`; a datasource with nothing federated keeps its empty
+    // `200` report.
     {
       method: 'POST',
       path: `${ext}/validate`,

packages/rest/src/external-datasource-unknown-name.test.ts

@@ -118,3 +118,46 @@ describe('the federation twins on a datasource that exists and is not a federati
     expect(answer.body.error.message).toContain("is the platform's managed primary datasource");
   });
 });
+
+/**
+ * [#27696] `POST …/external/validate`, the one by-name federation door the
+ * pins above left out. It answered an unknown name `200` with an empty,
+ * vacuously `ok` report, so a caller could not tell "nothing on this
+ * datasource is federated" from "this datasource does not exist". The ruling:
+ * no definition ⇒ the same `404`, before any validation; a datasource that
+ * exists and has nothing federated keeps its `200` empty report.
+ */
+describe('POST …/external/validate on a name that resolves no saved datasource', () => {
+  it('answers 404 RESOURCE_NOT_FOUND in the words GET /datasources/:name uses', async () => {
+    const call = mount();
+
+    const answer = await call('POST', '/no_such_ds/external/validate');
+
+    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');
+  });
+
+  it.each([
+    ['a saved datasource', 'warehouse'],
+    ['the primary datasource', 'default'],
+  ])('keeps the 200 empty report for %s with nothing federated', async (_what, name) => {
+    const call = mount();
+
+    const answer = await call('POST', `/${name}/external/validate`);
+
+    expect({ status: answer.status, data: answer.body.data }).toEqual({
+      status: 200,
+      data: { ok: true, results: [] },
+    });
+  });
+});

packages/rest/src/external-datasource-validate-scope.test.ts

@@ -213,16 +213,18 @@ describe('[#10537] POST /external/validate scopes its WORK to :name', () => {
     expect(body.data).toEqual(reference);
   });
 
-  it('an unknown :name answers the same empty report as before — and dials nothing', async () => {
+  // [#27696] Until this ruling an unknown name answered an empty report with
+  // a vacuously true verdict; it is now the 404 every by-name door answers.
+  // `external-datasource-unknown-name.test.ts` pins the 200 a saved datasource
+  // with nothing federated keeps.
+  it('an unknown :name answers 404 RESOURCE_NOT_FOUND — and dials nothing', async () => {
     const { app, introspected, validateAllSpy } = mount();
     const { status, body } = await validate(app, 'no_such_ds');
 
-    expect(status).toBe(200);
-    expect(body.success).toBe(true);
-    // Unchanged: an unknown name is not a 404 on this route, it is an empty
-    // report with a vacuously true verdict.
-    expect(body.data).toEqual(await referenceAnswer('no_such_ds'));
-    expect(body.data).toEqual({ ok: true, results: [] });
+    expect(status).toBe(404);
+    expect(body.success).toBe(false);
+    expect(body.error.code).toBe('RESOURCE_NOT_FOUND');
+    expect(body.error.message).toBe('Datasource "no_such_ds" does not exist.');
     expect(introspected).toEqual([]);
     expect(validateAllSpy).not.toHaveBeenCalled();
   });

packages/services/service-datasource/src/__tests__/external-datasource-service.test.ts

@@ -11,6 +11,7 @@ import {
   type DatasourceLike,
   type ObjectLike,
 } from '../external-datasource-service.js';
+import { datasourceNotFoundError } from '../datasource-admin-service.js';
 
 /** Build a fake introspected schema for the `warehouse` datasource. */
 function warehouseSchema(): IntrospectedSchema {
@@ -662,14 +663,52 @@ describe('validateDatasource', () => {
     });
   });
 
-  it('answers an empty report for a datasource nothing is bound to — and introspects nothing', async () => {
-    const { svc, introspected } = makeMulti();
+  it('answers an empty report for a saved datasource nothing is bound to — and introspects nothing', async () => {
+    const introspected: string[] = [];
+    const svc = new ExternalDatasourceService({
+      introspect: async (datasource: string) => {
+        introspected.push(datasource);
+        return REMOTE;
+      },
+      getDatasource: async (name: string) => (name === 'wh_empty' ? { name, schemaMode: 'external' } : undefined),
+      getObject: async () => undefined,
+      listObjects: async () => OBJECTS,
+    });
 
-    const report = await svc.validateDatasource('no_such_ds');
+    const report = await svc.validateDatasource('wh_empty');
 
     expect(report).toEqual({ ok: true, results: [] });
     expect(introspected).toEqual([]);
   });
+
+  // [#27696] A name with no saved datasource is not a datasource with nothing
+  // federated: it is the 404 every by-name door answers, thrown before
+  // anything is listed or introspected.
+  it('throws the 404 datasourceNotFoundError for a name that resolves no saved datasource', async () => {
+    let listed = 0;
+    const introspected: string[] = [];
+    const svc = new ExternalDatasourceService({
+      introspect: async (datasource: string) => {
+        introspected.push(datasource);
+        return REMOTE;
+      },
+      getDatasource: async () => undefined,
+      getObject: async () => undefined,
+      listObjects: async () => {
+        listed++;
+        return OBJECTS;
+      },
+    });
+
+    const thrown = await svc.validateDatasource('no_such_ds').catch((err: unknown) => err);
+
+    expect(thrown).toMatchObject({
+      status: 404,
+      code: 'RESOURCE_NOT_FOUND',
+      message: datasourceNotFoundError('no_such_ds').message,
+    });
+    expect({ listed, introspected }).toEqual({ listed: 0, introspected: [] });
+  });
 });
 
 /**

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

@@ -1206,11 +1206,19 @@ export class ExternalDatasourceService implements IExternalDatasourceService {
    * the sweep would have kept" and "the objects this selects" are the same set
    * (pinned in `__tests__/external-datasource-service.test.ts`).
    *
-   * A name nothing is bound to selects nothing and answers an empty, vacuously
-   * `ok` report — the sweep-then-filter answer for an unknown name, kept rather
-   * than upgraded to a throw: whether an unknown datasource is an error is a
-   * separate question from this one, and this method must not decide it in
-   * passing.
+   * A saved datasource nothing is bound to selects nothing and answers an
+   * empty, vacuously `ok` report.
+   *
+   * [#27696] A name that resolves no saved datasource is not that datasource:
+   * it THROWS the 404 {@link datasourceNotFoundError} before anything is
+   * listed or validated, the check {@link testConnection} makes (#27510), so
+   * `POST …/external/validate` answers it as `GET /datasources/:name` and its
+   * federation siblings do. It answered the empty `ok` report above, and a
+   * caller could not tell "nothing here is federated" from "no such
+   * datasource". Only existence is asked, not the federation guard
+   * ({@link assertFederationSource}): a managed datasource, and the primary
+   * one (not looked up, as it may have no stored record), keep their empty
+   * report.
    *
    * NOT on `IExternalDatasourceService` (spec): the route composition this
    * serves was authorized as a service-side helper, while adding a
@@ -1220,6 +1228,9 @@ export class ExternalDatasourceService implements IExternalDatasourceService {
    * scoped spelling, rather than silently falling back to the fan-out.
    */
   async validateDatasource(datasource: string): Promise<SchemaValidationReport> {
+    if (datasource !== DEFAULT_DATASOURCE_NAME && !(await this.config.getDatasource(datasource))) {
+      throw datasourceNotFoundError(datasource);
+    }
     const objects = await this.config.listObjects();
     return this.validateEach(
       objects.filter((o) => this.isFederated(o) && (o.datasource ?? 'default') === datasource),