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
56 changes: 56 additions & 0 deletions .changeset/datasource-validate-scoped-to-url.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
---
"@objectstack/rest": patch
"@objectstack/service-datasource": patch
---

fix(rest): `POST /datasources/:name/external/validate` does URL-scoped work (#10537)

The route asked the `external-datasource` service for `validateAll()` — every
federated object on every federated datasource, each validation driving a live
`introspect(datasource)` remote-schema read — and then kept only the rows whose
`datasource` matched the URL. The rows it kept were correct; the *work* was not
scoped, so one datasource's health check paid for N datasources' remote
round-trips and threw most of the measurement away. An unreachable *unrelated*
remote slowed the answer for the datasource actually asked about (and produced
rows that were then filtered off).

Measured at the branch point, through the real Hono adapter and the real
`ExternalDatasourceService` over a recording introspector: a request for one of
three federated datasources introspected `['wh_a', 'wh_b', 'wh_c']`. A request
naming a datasource that does not exist introspected all three as well, to
answer the empty report it already answered.

`ExternalDatasourceService` now carries `validateDatasource(datasource)`, the
scoped twin of the sweep composed from the same primitives (`listObjects` →
filter → `validateObject`) and the same per-object catch, and the route calls
it. Same request answers `['wh_a']`; an unknown name answers `[]`.

**No response change.** The rows the post-filter used to keep are the rows the
scoped composition returns — same objects, same diffs, same `data.ok` verdict,
same `200`, the same `400 EXTERNAL_DATASOURCE_ERROR` when the service refuses,
the same `503 SERVICE_UNAVAILABLE` when federation is not wired in, and an
unknown `:name` still answers an empty, vacuously `ok` report rather than a
`404`. The selection is keyed on `o.datasource ?? 'default'`, which is exactly
the value `validateObject` reports back as `result.datasource`, so "the rows the
sweep would have kept" and "the objects this selects" are the same set — pinned
directly, in both packages, by comparing the scoped answer against the
sweep-then-filter answer rather than against a remembered body.

Because the output was already right, the pins that matter here are about the
CALL RECORD, not the body: `external-datasource-validate-scope.test.ts` asserts
which datasources were introspected and that `validateAll()` is not called at
all, over a fixture carrying three federated datasources so the assertion can
actually fail. A body-only test passes on both sides of this change.

`validateDatasource` is **not** on `IExternalDatasourceService`: the contract
offers `validateObject(objectName)` and `validateAll()`, and adding a
per-datasource spelling to it is a spec-surface decision to take on its own
terms. The composition therefore lives in the service — the only registrant of
the `external-datasource` slot — and the REST registrar probes for it. A wired
service with no scoped spelling takes the same `503` arm every other route in
this family takes when the service cannot serve it, deliberately *not* a silent
fallback to the fan-out: a fallback would leave the old behaviour reachable on a
path no test drives.

Unchanged: `validateAll()` itself, and the boot-validation sweep in
`packages/runtime` that legitimately validates every federated object.
Original file line numberDiff line numberDiff line change
Expand Up@@ -149,7 +149,7 @@ describe('external-datasource envelope (#3843) — success bodies', () => {
status: 200,
dataKeys: ['ok', 'results'],
run: () => drive(
mount({ validateAll: async () => ({ results: [{ datasource: 'ext', ok: true }] }) }),
mount({ validateDatasource: async () => ({ results: [{ datasource: 'ext', ok: true }] }) }),
'POST',
`${EXT}/validate`,
),
Expand DownExpand Up@@ -192,7 +192,7 @@ describe('external-datasource envelope (#3843) — success bodies', () => {
it("POST /validate keeps its `ok` — a domain verdict, not a second `success`", async () => {
// All results valid → data.ok true, while `success` reports the request.
const pass = await drive(
mount({ validateAll: async () => ({ results: [{ datasource: 'ext', ok: true }] }) }),
mount({ validateDatasource: async () => ({ results: [{ datasource: 'ext', ok: true }] }) }),
'POST',
`${EXT}/validate`,
);
Expand All@@ -204,7 +204,7 @@ describe('external-datasource envelope (#3843) — success bodies', () => {
// `success` the way storage's was.
const fail = await drive(
mount({
validateAll: async () => ({
validateDatasource: async () => ({
results: [{ datasource: 'ext', ok: true }, { datasource: 'ext', ok: false }],
}),
}),
Expand DownExpand Up@@ -279,7 +279,7 @@ describe('external-datasource envelope (#3843) — error bodies', () => {
status: 400,
code: 'EXTERNAL_DATASOURCE_ERROR',
run: () => drive(
mount({ validateAll: async () => { throw new Error('metadata store offline'); } }),
mount({ validateDatasource: async () => { throw new Error('metadata store offline'); } }),
'POST',
`${EXT}/validate`,
),
Expand Down
28 changes: 21 additions & 7 deletions packages/rest/src/external-datasource-routes-auth-guard.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -61,7 +61,7 @@
* authentication floor — pinned here as an explicit `capability: null` row so
* that gating it later had to change the table. That later card is #10255,
* ruled 2026-08-20 (verbatim: 「同意你的意见。」, accepting option A): validate
* takes the READ capability, because `validateAll` drives the same live
* takes the READ capability, because validation drives the same live
* remote-schema introspection the read twins gate and reports on it. The row
* now carries `READ_CAPABILITY`, and its case below flips from "still served
* holding nothing" to "refused holding nothing" — deliberately, not by a
Expand DownExpand Up@@ -106,8 +106,13 @@ type Handler = (req: any, res: any) => any;
* /external/validate` carried one — spelled as an explicit `null` rather than
* omitted, so that a later edit gating it had to change this table — and the
* 2026-08-20 #10255 ruling is that later edit: validate is a read
* (`validateAll` drives the same live remote introspection the read twins
* (validation drives the same live remote introspection the read twins
* gate), so its row now carries `READ_CAPABILITY` like its two read siblings.
*
* [#10537] The validate row's `call` is the SCOPED composition
* (`validateDatasource`), which is what the route dispatches to since the
* fan-out fix; the fixture keeps a `validateAll` spy beside it precisely so a
* regression to the whole-farm sweep is visible here rather than silent.
*/
const READ_CAPABILITY = 'manage_platform_settings';
const WRITE_CAPABILITY = 'manage_metadata';
Expand All@@ -117,7 +122,7 @@ const FAMILY = [
{ method: 'POST', url: `${BASE}/datasources/${DS}/external/tables/customers/draft`, ok: 200, call: 'generateObjectDraft', writes: false, capability: READ_CAPABILITY },
{ method: 'POST', url: `${BASE}/datasources/${DS}/external/tables/customers/import`, ok: 201, call: 'importObject', writes: true, capability: WRITE_CAPABILITY },
{ method: 'POST', url: `${BASE}/datasources/${DS}/external/refresh-catalog`, ok: 200, call: 'refreshCatalog', writes: true, capability: WRITE_CAPABILITY },
{ method: 'POST', url: `${BASE}/datasources/${DS}/external/validate`, ok: 200, call: 'validateAll', writes: false, capability: READ_CAPABILITY },
{ method: 'POST', url: `${BASE}/datasources/${DS}/external/validate`, ok: 200, call: 'validateDatasource', writes: false, capability: READ_CAPABILITY },
] as const;

/** Every capability an entitled caller needs to clear all five routes. */
Expand DownExpand Up@@ -174,7 +179,13 @@ function federationServiceSpies() {
generateObjectDraft: vi.fn(async () => ({ name: 'customers' })),
importObject: vi.fn(async () => ({ name: 'customers' })),
refreshCatalog: vi.fn(async () => ({ tables: {} })),
// [#10537] `POST /external/validate` dispatches to the SCOPED composition
// now. The whole-farm `validateAll` stays in the set, spied and never
// expected to run: "the service never ran" then means the method the
// route actually reaches, and a regression to the sweep shows up as a
// call on a spy nobody expects rather than as silence.
validateAll: vi.fn(async () => ({ results: [{ datasource: DS, ok: true }] })),
validateDatasource: vi.fn(async (name: string) => ({ results: [{ datasource: name, ok: true }] })),
};
}

Expand DownExpand Up@@ -475,14 +486,14 @@ describe('[#9901] the family requires a capability above authentication', () =>
// 2026-08-20 #10255 ruling it asserted the exact opposite — an
// authenticated caller holding nothing was SERVED here while refused the
// other four — because #9901's ruling did not name this route. The ruling
// that changed it is recorded on #10255 (option A): `validateAll` drives
// that changed it is recorded on #10255 (option A): validation drives
// the same live remote-schema introspection the read twins gate, so
// validate is a read and answers to the read capability.
const { table, service } = await bootFederation({
withAuth: true, withEngine: true, grants: [],
});

const validate = FAMILY.find((r) => r.call === 'validateAll')!;
const validate = FAMILY.find((r) => r.call === 'validateDatasource')!;
const { statusCode, body } = await call(table, validate, { authorization: `Bearer ${SESSION}` });

// Refused with the capability NAMED — the one thing the refused caller can
Expand All@@ -492,6 +503,7 @@ describe('[#9901] the family requires a capability above authentication', () =>
expect(body?.success).toBe(false);
expect(body?.error?.code).toBe('PERMISSION_DENIED');
expect(body?.error?.message).toContain(READ_CAPABILITY);
expect(service.validateDatasource).not.toHaveBeenCalled();
expect(service.validateAll).not.toHaveBeenCalled();
});

Expand All@@ -503,12 +515,14 @@ describe('[#9901] the family requires a capability above authentication', () =>
withAuth: true, withEngine: true, grants: [READ_CAPABILITY],
});

const validate = FAMILY.find((r) => r.call === 'validateAll')!;
const validate = FAMILY.find((r) => r.call === 'validateDatasource')!;
const { statusCode, body } = await call(table, validate, { authorization: `Bearer ${SESSION}` });

expect(statusCode).toBe(validate.ok);
expect(body?.success).toBe(true);
expect(service.validateAll).toHaveBeenCalled();
expect(service.validateDatasource).toHaveBeenCalledWith(DS);
// [#10537] …and the served request did NOT fan out across every datasource.
expect(service.validateAll).not.toHaveBeenCalled();
});

it('a capability the caller does not hold is not granted by an api key either', async () => {
Expand Down
83 changes: 75 additions & 8 deletions packages/rest/src/external-datasource-routes.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -10,7 +10,11 @@ import {
ANONYMOUS_DENY_MESSAGE,
type PluginContext,
} from '@objectstack/core';
import type { IExternalDatasourceService, IHttpServer } from '@objectstack/spec/contracts';
import type {
IExternalDatasourceService,
IHttpServer,
SchemaValidationReport,
} from '@objectstack/spec/contracts';
// The declared envelope is written in ONE place for the whole platform (#3973).
import { sendOk, sendError } from '@objectstack/types';
import { mountDirectRoutes, type DirectMountedRoute } from './direct-mount.js';
Expand All@@ -27,7 +31,7 @@ import { mountDirectRoutes, type DirectMountedRoute } from './direct-mount.js';
* POST /datasources/:name/external/tables/:remote/draft → generateObjectDraft
* POST /datasources/:name/external/tables/:remote/import → importObject
* POST /datasources/:name/external/refresh-catalog → refreshCatalog
* POST /datasources/:name/external/validate → validateAll (this ds)
* POST /datasources/:name/external/validate → validateDatasource(:name)
*
* NOTE: the datasource *lifecycle* routes (`/api/v1/datasources` —
* list / test / create / update / remove, ADR-0015 Addendum) were extracted
Expand DownExpand Up@@ -68,6 +72,17 @@ import { mountDirectRoutes, type DirectMountedRoute } from './direct-mount.js';
* (`results.every(r => r.ok)`). A domain verdict that happens to share the name
* is not a second envelope flag, so it belongs inside `data` rather than being
* dropped.
*
* [#10537] `POST /validate` does URL-SCOPED WORK. It used to call
* `validateAll()` — every federated object on every federated datasource, each
* validation driving a live `introspect(datasource)` — and then keep only the
* rows matching `:name`. The rows were right; the work was not scoped, so one
* datasource's health check paid for N datasources' remote round-trips and an
* unreachable *unrelated* remote slowed the answer for the datasource actually
* asked about (measured at head: a request for one of three federated
* datasources introspected all three). It now calls the service's scoped
* composition — see {@link scopedValidation} for what that is and why it is
* probed rather than declared on `IExternalDatasourceService`.
*/
export interface ExternalDatasourceRoutesOptions {
/**
Expand DownExpand Up@@ -128,12 +143,16 @@ export interface ExternalDatasourceRoutesOptions {
* the #9686 authentication floor and filed the question instead of deciding
* it. The follow-up ruling (maintainer, 2026-08-20, verbatim:
* 「同意你的意见。」, accepting option A on #10255) converged it here: what
* `validateAll` does is drive the SAME live remote-schema introspection the
* validation does is drive the SAME live remote-schema introspection the
* two read twins gate (`introspect` per datasource, in
* `service-datasource/src/external-datasource-service.ts`), and its report —
* schema diffs naming remote columns and types, driver error strings for
* unreachable remotes — is a read of the same federation surface. One family,
* one door-type: reads here, writes on {@link FEDERATION_WRITE_CAPABILITY}.
*
* [#10537] The reasoning is unchanged by the scoping fix and was never about
* the sweep's WIDTH: it is the same introspection whether one datasource is
* read or all of them, so `validate` answers to the read capability either way.
*/
export const FEDERATION_READ_CAPABILITY = 'manage_platform_settings';

Expand DownExpand Up@@ -321,6 +340,45 @@ export function registerExternalDatasourceRoutes(
}
};

/**
* [#10537] The scoped validation the `POST /validate` route needs: validate
* the federated objects bound to ONE datasource, composed service-side from
* the same primitives the whole-farm sweep uses (`listObjects` → filter →
* `validateObject`).
*
* ## Why it is PROBED rather than read off the contract
*
* `IExternalDatasourceService` declares `validateObject(objectName)` and
* `validateAll()` and no per-datasource spelling. Adding one is a
* spec-surface change that has to be decided on its own terms, so this fix
* takes the other authorized shape: the composition lives in the service
* (`ExternalDatasourceService.validateDatasource`, the only registrant of
* this slot) and the route probes for it. Everything about the ANSWER is
* still contract-typed — {@link SchemaValidationReport} is the contract's
* own report type — so what is asserted here rather than checked is the
* method's name, and nothing about the shape it returns.
*
* ## Absence answers 503, deliberately not a fan-out fallback
*
* A wired service with no scoped spelling could be served by falling back to
* `validateAll()` and post-filtering — which is precisely the behaviour
* #10537 removed. A silent fallback would leave the fan-out reachable, on a
* path no test drives, for exactly the deployments nobody is looking at. So
* absence takes the same 503 arm every other route here takes when the
* service cannot serve it: loud, and already the declared shape of "this
* deployment's federation service cannot do this".
*/
interface ScopedValidation {
validateDatasource(datasource: string): Promise<SchemaValidationReport>;
}

const scopedValidation = (): ScopedValidation | undefined => {
const svc = externalService() as
| (IExternalDatasourceService & Partial<ScopedValidation>)
| undefined;
return typeof svc?.validateDatasource === 'function' ? (svc as ScopedValidation) : undefined;
};

const unavailable = (res: any) =>
sendError(res, 503, 'SERVICE_UNAVAILABLE', 'The external-datasource service is not available.');

Expand DownExpand Up@@ -437,20 +495,29 @@ export function registerExternalDatasourceRoutes(
},

// Validate the federated objects on this datasource. [#10255] A 'read':
// validateAll drives the same live remote-schema introspection the two
// validation drives the same live remote-schema introspection the two
// read twins gate, so it answers to the same capability (ruled 2026-08-20;
// the constant's doc carries the reasoning).
//
// [#10537] The work is scoped by the CALL, not by a filter over a
// whole-farm sweep: `validateDatasource(:name)` introspects the named
// datasource's remote and no other. The response is unchanged — the rows
// 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).
{
method: 'POST',
path: `${ext}/validate`,
metadata: { summary: 'Validate the federated objects on a datasource', tags: ['datasources'] },
handler: async (req: any, res: any) => {
if (await refuseFederationRequest(req, res, 'read')) return;
const svc = externalService();
if (!svc?.validateAll) return unavailable(res);
const scoped = scopedValidation();
if (!scoped) return unavailable(res);
try {
const report = await svc.validateAll();
const results = (report.results ?? []).filter((r) => r.datasource === req.params.name);
const report = await scoped.validateDatasource(req.params.name);
const results = report.results ?? [];
// The domain verdict stays this route's own computation over the rows
// it answers with — the same expression as before the scoping fix.
sendOk(res, { ok: results.every((r) => r.ok), results });
} catch (err) {
refused(res, err);
Expand Down
Loading
Loading