diff --git a/.changeset/27745-odata-metadata-import.md b/.changeset/27745-odata-metadata-import.md new file mode 100644 index 00000000000..168a2c72619 --- /dev/null +++ b/.changeset/27745-odata-metadata-import.md @@ -0,0 +1,22 @@ +--- +'@objectstack/spec': patch +'@objectstack/driver-odata': patch +'@objectstack/service-datasource': patch +'@objectstack/rest': patch +'@objectstack/runtime': patch +'@objectstack/mcp': patch +'@object-ui/app-shell': patch +'@object-ui/i18n': patch +--- + +feat(datasource): import objects from an OData datasource's `$metadata` — EDM types map to field types, the key to the id, and navigations to lookups; Studio's datasource page imports several entity sets at once, and MCP gains `list_remote_tables` / `import_object` + +Clause-②: yes (widening) +Metadata-protocol: none + +- **Type matrix (`@objectstack/spec/data`).** `SqlDialect` gains `'odata'`, whose aliases map the EDM primitives (`Edm.String`, `Edm.Int16/32/64`, `Edm.Decimal`, `Edm.Double`, `Edm.Boolean`, `Edm.Date`, `Edm.DateTimeOffset`, `Edm.TimeOfDay`, `Edm.Guid`, `Edm.Binary`, `Edm.Stream`, `Collection(…)`) onto the matrix's canonical types. `Edm.Duration`, the geography / geometry families and a service's own enum or complex types stay unknown, so a draft flags them for review. `Edm.Int64` and `Edm.Decimal` take the same verdict as SQL `bigint` / `numeric` (exact for `number`). +- **Gate 2 gets looser, for SQL federation too.** `lookup` and `master_detail` are now exact under the `text`, `integer` and `bigint` rows, as they already were under `uuid`: a reference stores the target's id, which on a federated object is the remote key a foreign-key column carries. Before, `isCompatible` refused a lookup over `CustomerID int` or `varchar` as a `type_mismatch` error, so the boot validation (`onMismatch: 'fail'`) refused any federated object that declared one. Nothing that passed before fails now. +- **Draft (`@objectstack/service-datasource`).** For an OData datasource (the driver reports dialect `odata-4.0`, read as the `odata` vocabulary) `generateObjectDraft` / `importObject` / `refreshCatalog` and the validation map EDM types through that arm. A column whose name is not a snake_case field name drafts as its snake_case spelling with `external.columnMap` carrying the remote name (`CategoryID` → `category_id`); `rename` overrides now land in `columnMap` too, so a renamed field stays bound to its column. A remote table name drafts the same way (`Products` → `products`; a leading capital used to give `_products`). A single-valued navigation becomes a `lookup` on the column its `ReferentialConstraint` names when the constraint references the target's single key and the target is imported on the same datasource (or is the object itself); every other navigation is left out of the draft and named in `review` with its reason — import the target first. +- **Driver (`@objectstack/driver-odata`).** `introspectSchema()` lists each navigation's `referentialConstraints`. +- **Studio.** The datasource page's tables tab selects several entity sets and imports them in one go, saving lookup targets before the tables that reference them; the write opt-in is offered only on a datasource that allows writes, and an imported object stays read-only otherwise (single-table dialog included). +- **MCP.** `list_remote_tables` and `import_object` reach the same `IExternalDatasourceService` calls, registered only for a token whose scopes name `metadata:write`, and refused by the runtime bridge without the capability the REST twins require (`FEDERATION_READ_CAPABILITY` / `FEDERATION_WRITE_CAPABILITY`, now exported from `@objectstack/rest`). diff --git a/content/docs/data-modeling/odata-datasources.mdx b/content/docs/data-modeling/odata-datasources.mdx index da5122eb2d4..9fe261ffcdd 100644 --- a/content/docs/data-modeling/odata-datasources.mdx +++ b/content/docs/data-modeling/odata-datasources.mdx @@ -138,9 +138,24 @@ What each piece does on this driver: - **`id` is the entity key.** A single-property key is read as text (`'4711'`). A composite key is the body of its key predicate (`'OrderID=1,ProductID=7'`), which is also what `GET /data//` takes. -- **Declare each field by hand.** Nothing generates fields from `$metadata` today. - Navigation properties are never followed, so a related entity set is an object of its - own. +- **Or import the objects from `$metadata`.** Once the datasource is saved, Studio's + datasource page lists its entity sets on the **Tables** tab. **Import** saves one, + and **Import selected** saves several, referenced entity sets first. An MCP client + whose token carries `metadata:write` reaches the same import through + `list_remote_tables` and `import_object`. The draft maps each EDM type to a field + type, makes the key the `id`, and writes `columnMap` for every property whose name is + not a snake_case field name (`CategoryID` becomes `category_id`). EDM types with no + field type, such as `Edm.Duration` or a service's own enum, are listed for review. An + imported object is read-only unless the datasource allows writes and you tick + **Allow writes to this object**. +- **Navigation properties are never followed**, so a related entity set is an object of + its own. The import turns a single-valued navigation into a `lookup` on its foreign-key + property when its `ReferentialConstraint` is one property pair onto the target's key + and the target is already imported on the same datasource. Every other navigation is + left out and named in the review, so import the target first. The lookup reads the + related record by key, like any other lookup. These rules are what + `packages/services/service-datasource/src/__tests__/external-object-draft-odata-metadata.test.ts` + asserts, over a recorded Northwind `$metadata`. - **The service document is read lazily.** `connect()` asks the service nothing, and `$metadata` is read once, at the first request. A wrong `serviceRoot` or an unknown entity set shows up on the first read of the object, not at boot. @@ -266,7 +281,7 @@ as a sharing scope. The full mapping is in - **OData v2.** Only the v4 dialect ships. - **`$expand` and navigation properties.** Bind each related entity set as an object of - its own. + its own; an imported foreign key becomes a `lookup` onto it. - **Actions and functions** of the service. `execute` is refused. - **Transactions and `$batch`.** Each write is one request. - **More than 1000 matching rows** in one read (the cap above). diff --git a/packages/drivers/driver-odata/src/odata-driver.test.ts b/packages/drivers/driver-odata/src/odata-driver.test.ts index 05182636b89..dcc7846266d 100644 --- a/packages/drivers/driver-odata/src/odata-driver.test.ts +++ b/packages/drivers/driver-odata/src/odata-driver.test.ts @@ -225,7 +225,7 @@ describe('keys', () => { }); describe('introspection', () => { - it('lists entity sets as tables, with EDM types, the key and single-valued navigations as lookup candidates', async () => { + it('lists entity sets as tables, with EDM types, the key, and single-valued navigations with their referential constraints as lookup candidates', async () => { const { driver } = driverFor([]); const schema = await driver.introspectSchema(); expect(schema.dialect).toBe('odata-4.0'); @@ -234,8 +234,17 @@ describe('introspection', () => { expect(people.columns.find((c) => c.name === 'UserName')).toEqual({ name: 'UserName', type: 'Edm.String', nullable: false, primaryKey: true }); expect(people.columns.find((c) => c.name === 'Emails')?.type).toBe('Collection(Edm.String)'); expect(people.navigationProperties).toEqual([ - { name: 'BestFriend', targetEntityType: 'Microsoft.OData.SampleService.Models.TripPin.Person', targetEntitySet: 'People', nullable: true }, - { name: 'Photo', targetEntityType: 'Microsoft.OData.SampleService.Models.TripPin.Photo', targetEntitySet: 'Photos', nullable: true }, + { name: 'BestFriend', targetEntityType: 'Microsoft.OData.SampleService.Models.TripPin.Person', targetEntitySet: 'People', nullable: true, referentialConstraints: [] }, + { name: 'Photo', targetEntityType: 'Microsoft.OData.SampleService.Models.TripPin.Photo', targetEntitySet: 'Photos', nullable: true, referentialConstraints: [] }, + ]); + expect(schema.tables.Flights!.navigationProperties).toEqual([ + { + name: 'Airline', + targetEntityType: 'Microsoft.OData.SampleService.Models.TripPin.Airline', + targetEntitySet: 'Airlines', + nullable: false, + referentialConstraints: [{ property: 'AirlineCode', referencedProperty: 'AirlineCode' }], + }, ]); expect(schema.tables.Flights!.columns.filter((c) => c.primaryKey).map((c) => c.name)).toEqual(['PlanItemId']); }); diff --git a/packages/drivers/driver-odata/src/odata-driver.ts b/packages/drivers/driver-odata/src/odata-driver.ts index 1b8b6af09d8..a3524141aaa 100644 --- a/packages/drivers/driver-odata/src/odata-driver.ts +++ b/packages/drivers/driver-odata/src/odata-driver.ts @@ -77,8 +77,20 @@ export interface ODataDriverRuntime { export interface ODataIntrospectedTable extends IntrospectedTable { /** The qualified entity type. */ entityType: string; - /** Single-valued navigation properties: candidates for a lookup field, never followed by reads. */ - navigationProperties: Array<{ name: string; targetEntitySet?: string; targetEntityType: string; nullable: boolean }>; + /** + * Single-valued navigation properties: candidates for a lookup field, never + * followed by reads. A lookup needs a column holding the target's key, which + * only a `ReferentialConstraint` names (the dependent property on this type → + * the principal property on the target); a navigation without one has no + * column to back a lookup. + */ + navigationProperties: Array<{ + name: string; + targetEntitySet?: string; + targetEntityType: string; + nullable: boolean; + referentialConstraints: Array<{ property: string; referencedProperty: string }>; + }>; } export interface ODataIntrospectedSchema extends IntrospectedSchema { @@ -245,6 +257,7 @@ export class ODataDriver implements IDataDriver { targetEntityType: nav.type, ...(set.navigationBindings[nav.name] ? { targetEntitySet: set.navigationBindings[nav.name] } : {}), nullable: nav.nullable, + referentialConstraints: nav.referentialConstraints.map((rc) => ({ ...rc })), })), }; } diff --git a/packages/mcp/src/index.ts b/packages/mcp/src/index.ts index 1ed204a538b..ccc9e651185 100644 --- a/packages/mcp/src/index.ts +++ b/packages/mcp/src/index.ts @@ -25,7 +25,12 @@ export type { McpAuthoringToolRegistry, McpAuthoringInjection, } from './types.js'; -export { registerObjectTools, registerActionTools, grantsAuthoringTools } from './mcp-http-tools.js'; +export { + registerObjectTools, + registerActionTools, + registerFederationTools, + grantsAuthoringTools, +} from './mcp-http-tools.js'; export type { McpDataBridge, McpObjectSummary, @@ -34,6 +39,7 @@ export type { McpActionSummary, McpActionParamSummary, RegisterActionToolsOptions, + McpFederationBridge, } from './mcp-http-tools.js'; // The portable `SKILL.md` distributable (ADR-0036 Amendment C) — NOT the // `skill` metadata type; that one is projected onto MCP prompts just below. diff --git a/packages/mcp/src/mcp-federation-tools.test.ts b/packages/mcp/src/mcp-federation-tools.test.ts new file mode 100644 index 00000000000..c45d1e0639f --- /dev/null +++ b/packages/mcp/src/mcp-federation-tools.test.ts @@ -0,0 +1,179 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#27745] The federation pair, `list_remote_tables` and `import_object`: an + * agent lists a federated datasource's entity sets and imports one through the + * same `IExternalDatasourceService` calls Studio's datasource page makes. + * + * Pinned here: the pair registers only for a token whose scopes name + * `metadata:write` (the authoring family's gate, since an import lands object + * metadata) and only on a bridge that carries both members; each call reaches + * the bridge with exactly the caller's arguments; a refusal the bridge throws + * comes back as the coded tool error. + */ + +import { describe, it, expect, beforeEach } from 'vitest'; +import { MCP_OAUTH_SCOPE_DATA_READ, MCP_OAUTH_SCOPE_METADATA_WRITE } from '@objectstack/spec/ai'; +import { MCPServerRuntime } from './mcp-server-runtime.js'; +import type { McpDataBridge, McpFederationBridge } from './mcp-http-tools.js'; +import { renderSkillMarkdown } from './skill-md.js'; + +const FEDERATION_TOOLS = ['import_object', 'list_remote_tables']; + +function makeBridge(withFederation = true): McpDataBridge & Partial & { calls: unknown[] } { + const calls: unknown[] = []; + const data: McpDataBridge = { + async listObjects() { return []; }, + async describeObject() { return null; }, + async query() { return { records: [] }; }, + async get() { return null; }, + async create() { return {}; }, + async update() { return {}; }, + async remove() { return {}; }, + }; + if (!withFederation) return { ...data, calls }; + return { + ...data, + calls, + async listRemoteTables(datasource, opts) { + calls.push(['listRemoteTables', datasource, opts]); + if (datasource === 'locked') { + throw Object.assign(new Error('Introspecting an external datasource requires the `manage_platform_settings` capability.'), { + code: 'PERMISSION_DENIED', + status: 403, + }); + } + return [{ name: 'Products', columnCount: 6 }, { name: 'Categories', columnCount: 4 }]; + }, + async importObject(datasource, remoteName, opts) { + calls.push(['importObject', datasource, remoteName, opts]); + return { + name: 'products', + definition: { name: 'products', datasource, external: { remoteName } }, + review: [], + }; + }, + }; +} + +async function rpc(runtime: MCPServerRuntime, bridge: unknown, grantedScopes: string[] | undefined, body: unknown) { + const req = new Request('http://localhost/api/v1/mcp', { + method: 'POST', + headers: { 'content-type': 'application/json', accept: 'application/json, text/event-stream' }, + body: JSON.stringify(body), + }); + const res = await runtime.handleHttpRequest(req, { + bridge: bridge as McpDataBridge, + parsedBody: body, + ...(grantedScopes ? { toolOptions: { grantedScopes } } : {}), + }); + return (await res.json()) as any; +} + +async function toolNames(runtime: MCPServerRuntime, bridge: unknown, grantedScopes?: string[]): Promise { + const json = await rpc(runtime, bridge, grantedScopes, { jsonrpc: '2.0', id: 1, method: 'tools/list' }); + return (json.result?.tools ?? []).map((t: { name: string }) => t.name); +} + +async function callTool(runtime: MCPServerRuntime, bridge: unknown, name: string, args: Record) { + const json = await rpc(runtime, bridge, [MCP_OAUTH_SCOPE_METADATA_WRITE], { + jsonrpc: '2.0', + id: 2, + method: 'tools/call', + params: { name, arguments: args }, + }); + return json.result as { content: Array<{ text: string }>; isError?: boolean }; +} + +describe('the federation tools register behind metadata:write, on a bridge that carries them', () => { + let runtime: MCPServerRuntime; + beforeEach(() => { + runtime = new MCPServerRuntime({ name: 't', version: '1.0.0' }); + }); + + it('registers both for a token naming metadata:write', async () => { + const names = await toolNames(runtime, makeBridge(), [MCP_OAUTH_SCOPE_METADATA_WRITE]); + expect(names.filter((n) => FEDERATION_TOOLS.includes(n)).sort()).toEqual(FEDERATION_TOOLS); + }); + + it('registers neither for a key or a session (no scope list), nor for a token without metadata:write', async () => { + for (const scopes of [undefined, [MCP_OAUTH_SCOPE_DATA_READ]]) { + const names = await toolNames(runtime, makeBridge(), scopes); + expect(names.filter((n) => FEDERATION_TOOLS.includes(n)), String(scopes)).toEqual([]); + } + }); + + it('registers neither on a bridge without the federation members', async () => { + const names = await toolNames(runtime, makeBridge(false), [MCP_OAUTH_SCOPE_METADATA_WRITE]); + expect(names.filter((n) => FEDERATION_TOOLS.includes(n))).toEqual([]); + }); + + it('annotates the listing read-only and the import as a non-destructive write', async () => { + const json = await rpc(runtime, makeBridge(), [MCP_OAUTH_SCOPE_METADATA_WRITE], { jsonrpc: '2.0', id: 1, method: 'tools/list' }); + const byName = new Map((json.result.tools as Array<{ name: string; annotations?: Record }>).map((t) => [t.name, t])); + expect(byName.get('list_remote_tables')?.annotations).toMatchObject({ readOnlyHint: true, destructiveHint: false }); + expect(byName.get('import_object')?.annotations).toMatchObject({ readOnlyHint: false, destructiveHint: false }); + }); +}); + +describe('the federation tools reach the bridge with the caller\'s arguments', () => { + let runtime: MCPServerRuntime; + beforeEach(() => { + runtime = new MCPServerRuntime({ name: 't', version: '1.0.0' }); + }); + + it('list_remote_tables answers the bridge\'s tables with their count', async () => { + const bridge = makeBridge(); + const result = await callTool(runtime, bridge, 'list_remote_tables', { datasource: 'erp' }); + expect(result.isError).toBeFalsy(); + expect(JSON.parse(result.content[0]!.text)).toEqual({ + tables: [{ name: 'Products', columnCount: 6 }, { name: 'Categories', columnCount: 4 }], + totalCount: 2, + }); + expect(bridge.calls).toEqual([['listRemoteTables', 'erp', {}]]); + }); + + it('import_object forwards the name override and the write opt-in, and nothing it was not sent', async () => { + const bridge = makeBridge(); + await callTool(runtime, bridge, 'import_object', { datasource: 'erp', remoteName: 'Products' }); + await callTool(runtime, bridge, 'import_object', { + datasource: 'erp', + remoteName: 'Products', + name: 'erp_products', + writable: true, + }); + expect(bridge.calls).toEqual([ + ['importObject', 'erp', 'Products', {}], + ['importObject', 'erp', 'Products', { name: 'erp_products', writable: true }], + ]); + }); + + it('refuses an unknown argument by name instead of dropping it', async () => { + const bridge = makeBridge(); + const result = await callTool(runtime, bridge, 'import_object', { datasource: 'erp', entitySet: 'Products' }); + expect(result.isError).toBe(true); + expect(result.content[0]!.text).toContain('remoteName'); + expect(bridge.calls).toEqual([]); + }); + + it('answers a refusal the bridge throws as the coded tool error', async () => { + const result = await callTool(runtime, makeBridge(), 'list_remote_tables', { datasource: 'locked' }); + expect(result.isError).toBe(true); + expect(JSON.parse(result.content[0]!.text).error).toMatchObject({ code: 'PERMISSION_DENIED', status: 403 }); + }); +}); + +describe('the generated SKILL.md teaches the pair exactly where it registers', () => { + it('names both for a metadata:write grant on a host that carries them, and neither otherwise', () => { + const granted = renderSkillMarkdown({ grantedScopes: [MCP_OAUTH_SCOPE_METADATA_WRITE], federationTools: true }); + for (const name of FEDERATION_TOOLS) expect(granted).toContain(`**${name}(`); + for (const options of [ + { federationTools: true }, + { grantedScopes: [MCP_OAUTH_SCOPE_DATA_READ], federationTools: true }, + { grantedScopes: [MCP_OAUTH_SCOPE_METADATA_WRITE] }, + ]) { + const skill = renderSkillMarkdown(options); + for (const name of FEDERATION_TOOLS) expect(skill).not.toContain(name); + } + }); +}); diff --git a/packages/mcp/src/mcp-http-tools.ts b/packages/mcp/src/mcp-http-tools.ts index 0015f6b5475..526a7e2fac9 100644 --- a/packages/mcp/src/mcp-http-tools.ts +++ b/packages/mcp/src/mcp-http-tools.ts @@ -55,6 +55,8 @@ import { import { AI_ACTION_CONFIRMATION_MEMBER, type AIActionConfirmation, + type ImportObjectResult, + type RemoteTable, } from '@objectstack/spec/contracts'; import { validateExpression, @@ -276,6 +278,30 @@ export interface McpActionBridge { ): Promise; } +/** + * [#27745] The federation seam: list a federated datasource's remote tables + * (an OData datasource's entity sets) and import one as a live federated + * object — the same `IExternalDatasourceService` calls Studio's datasource + * page makes (`GET …/external/tables`, `POST …/tables/:remote/import`). + * + * The host binds both to the caller and refuses as the REST routes refuse: + * listing needs the federation READ capability, importing the WRITE one + * (`FEDERATION_READ_CAPABILITY` / `FEDERATION_WRITE_CAPABILITY`, + * `@objectstack/rest`), thrown with the ADR-0112 `code` / `status`. + * + * OPTIONAL as a pair: a bridge without both registers neither tool. The + * runtime's bridge carries both and answers `503 SERVICE_UNAVAILABLE` per call + * on a host with no `external-datasource` service, as the REST twin does. + */ +export interface McpFederationBridge { + listRemoteTables(datasource: string, opts: { schema?: string }): Promise; + importObject( + datasource: string, + remoteName: string, + opts: { name?: string; writable?: boolean }, + ): Promise; +} + export interface RegisterActionToolsOptions { /** Expose actions on `sys_*` system objects too. Default false (fail-closed). */ allowSystemObjects?: boolean; @@ -633,13 +659,119 @@ const RECORD_ID_ALIASES = { id: 'recordId', record_id: 'recordId' } as const; */ export function wireBridgeTools( server: McpServer, - bridge: McpDataBridge & Partial, + bridge: McpDataBridge & Partial & Partial, options: RegisterObjectToolsOptions & RegisterActionToolsOptions = {}, ): string[] { const registered = registerObjectTools(server, bridge, options); if (typeof bridge.listActions === 'function' && typeof bridge.runAction === 'function') { registered.push(...registerActionTools(server, bridge as McpActionBridge, options)); } + if (typeof bridge.listRemoteTables === 'function' && typeof bridge.importObject === 'function') { + registered.push(...registerFederationTools(server, bridge as McpFederationBridge, options)); + } + return registered; +} + +/** + * [#27745] Register the federation pair, `list_remote_tables` and + * `import_object`, over {@link McpFederationBridge}. + * + * Gated as the AUTHORING family is ({@link grantsAuthoringTools}): an import + * lands object metadata, so both register only for a token whose scopes name + * `metadata:write`, and a key or a session gets neither. The listing rides + * with the import because it is the import's discovery step, and it reads a + * live remote schema, which the REST twin gates as an administrator read. + * Unlike the authoring family, the import does not land a draft: it saves a + * live, read-only (unless `writable`) federated object, exactly as Studio's + * "Import as Object" does — the description says so. + */ +export function registerFederationTools( + server: McpServer, + bridge: McpFederationBridge, + options: { grantedScopes?: readonly string[] } = {}, +): string[] { + const registered: string[] = []; + const note = (name: string): string => { + registered.push(name); + return name; + }; + if (!grantsAuthoringTools(options.grantedScopes)) return registered; + + server.registerTool( + note('list_remote_tables'), + { + description: + 'List the remote tables of a federated datasource — for an OData datasource, the entity sets its ' + + '$metadata declares — with their column counts. Use import_object to turn one into an object. ' + + 'Reads the live remote schema; requires the federation read capability.', + inputSchema: strictToolInput( + { surface: 'this list_remote_tables call', aliases: { name: 'datasource', datasourceName: 'datasource' } }, + { + datasource: z.string().describe('The federated datasource name (schemaMode external or validate-only).'), + schema: z.string().optional().describe('Restrict the listing to one remote schema.'), + }, + ), + annotations: { readOnlyHint: true, destructiveHint: false, openWorldHint: true }, + }, + async (args: { datasource: string; schema?: string }) => { + try { + const tables = await bridge.listRemoteTables(args.datasource, args.schema ? { schema: args.schema } : {}); + return textResult({ tables, totalCount: tables.length }); + } catch (err) { + return errorResultWithCode(err); + } + }, + ); + + server.registerTool( + note('import_object'), + { + description: + 'Import one remote table of a federated datasource (an OData entity set) as a live federated object, ' + + 'the same import Studio\'s datasource page performs. The object is saved at once (not as a draft), is ' + + 'read-only unless writable is true (which also needs the datasource to allow writes), and maps the ' + + 'remote types to field types. A navigation becomes a lookup only when its target is already imported, ' + + 'so import targets first; the result\'s review lists lossy columns and navigations left out. ' + + 'Requires the federation write capability.', + inputSchema: strictToolInput( + { + surface: 'this import_object call', + aliases: { + datasourceName: 'datasource', + table: 'remoteName', + remoteTable: 'remoteName', + entitySet: 'remoteName', + objectName: 'name', + }, + }, + { + datasource: z.string().describe('The federated datasource name.'), + remoteName: z.string().describe('The remote table / entity set, as list_remote_tables names it.'), + name: z + .string() + .optional() + .describe('Override the derived object name (snake_case, package namespace prefix included).'), + writable: z + .boolean() + .optional() + .describe('Opt the object into writes. Default false: federated objects are read-only.'), + }, + ), + annotations: { readOnlyHint: false, destructiveHint: false, idempotentHint: false, openWorldHint: true }, + }, + async (args: { datasource: string; remoteName: string; name?: string; writable?: boolean }) => { + try { + const result = await bridge.importObject(args.datasource, args.remoteName, { + ...(args.name !== undefined ? { name: args.name } : {}), + ...(args.writable !== undefined ? { writable: args.writable } : {}), + }); + return textResult({ object: result }); + } catch (err) { + return errorResultWithCode(err); + } + }, + ); + return registered; } diff --git a/packages/mcp/src/skill-md.ts b/packages/mcp/src/skill-md.ts index 664bb847833..3adf131444b 100644 --- a/packages/mcp/src/skill-md.ts +++ b/packages/mcp/src/skill-md.ts @@ -91,6 +91,15 @@ export interface RenderSkillOptions { * host's business — this renderer names what it is handed and nothing else. */ authoringTools?: ReadonlyArray>; + /** + * [#27745] Whether the host's bridge carries the federation pair + * (`list_remote_tables` / `import_object`, `registerFederationTools`). Its + * entries render only when this is true AND {@link grantedScopes} admit the + * family the pair registers under (`grantsAuthoringTools`), so the skill + * names them exactly where `tools/list` lists them. Default false: the + * renderer cannot see the bridge, so the host says so. + */ + federationTools?: boolean; } const URL_PLACEHOLDER = ''; // e.g. https://.objectos.app/api/v1/mcp @@ -140,6 +149,22 @@ ${bullets} `; } +/** + * [#27745] The federation pair's entries in the tool list, rendered for the + * grant `registerFederationTools` registers them for ({@link grantsAuthoringTools}) + * on a host whose bridge carries them ({@link RenderSkillOptions.federationTools}), + * so a connection without `metadata:write` is never taught them. + */ +const FEDERATION_TOOL_LINES = `- **list_remote_tables({ datasource, schema? })** — the remote tables a + federated datasource exposes (an OData datasource's entity sets), read live + from the remote schema. +- **import_object({ datasource, remoteName, name?, writable? })** — import one + remote table as a federated object. It is saved at once, not as a draft, and + is read-only unless \`writable: true\`. A navigation becomes a lookup only + when its target is already imported, so import targets first; the result's + \`review\` names lossy columns and left-out navigations. +`; + /** * Render the full `SKILL.md` (YAML frontmatter + body). Pass the env's MCP URL * to produce a ready-to-install skill; the body is otherwise generic. @@ -274,7 +299,7 @@ create/update payload. screen, which you fill with another \`resume_run\`. Only the user whose call started the run can resume it, under the same gates as \`run_action\` (including \`confirm: true\` for an action flagged \`requiresConfirmation\`). -${authoringTools ? renderAuthoringSection(authoringTools, describeObjectRegistered) : ''} +${options.federationTools === true && grantsAuthoringTools(options.grantedScopes) ? FEDERATION_TOOL_LINES : ''}${authoringTools ? renderAuthoringSection(authoringTools, describeObjectRegistered) : ''} ## Conventions & gotchas - **Permissions/RLS apply to every call.** Fewer rows than expected, or a diff --git a/packages/rest/src/index.ts b/packages/rest/src/index.ts index 5c16d190276..66543947946 100644 --- a/packages/rest/src/index.ts +++ b/packages/rest/src/index.ts @@ -225,3 +225,8 @@ export type { // `rest-server.ts`, beside the doors that read it (its docblock is the // authority); this entry already loads that module. export { metaItemPackageBinding } from './rest-server.js'; + +// [#27745] The two capabilities the federation routes gate on, exported so the +// MCP federation tools (`list_remote_tables` / `import_object`) refuse a caller +// exactly where `GET …/external/tables` and `POST …/tables/:remote/import` do. +export { FEDERATION_READ_CAPABILITY, FEDERATION_WRITE_CAPABILITY } from './external-datasource-routes.js'; diff --git a/packages/runtime/src/domains/mcp-federation-bridge.test.ts b/packages/runtime/src/domains/mcp-federation-bridge.test.ts new file mode 100644 index 00000000000..a733e957aee --- /dev/null +++ b/packages/runtime/src/domains/mcp-federation-bridge.test.ts @@ -0,0 +1,80 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#27745] The MCP federation bridge (`list_remote_tables` / `import_object`) + * refuses a caller exactly where the REST twins do: the listing needs + * `FEDERATION_READ_CAPABILITY`, the import `FEDERATION_WRITE_CAPABILITY`, both + * asked before the `external-datasource` service is looked up, and a host with + * no federation service answers `503 SERVICE_UNAVAILABLE`. + */ + +import { describe, it, expect } from 'vitest'; +import { FEDERATION_READ_CAPABILITY, FEDERATION_WRITE_CAPABILITY } from '@objectstack/rest'; +import { buildMcpBridge } from './mcp.js'; + +function bridgeFor(systemPermissions: string[], service: unknown) { + const lookups: string[] = []; + const deps = { + resolveService: async (_ctx: unknown, name: string) => { + lookups.push(name); + return name === 'external-datasource' ? service : undefined; + }, + }; + const context = { environmentId: 'env_1', executionContext: { userId: 'u_1', systemPermissions } }; + return { bridge: buildMcpBridge(deps as never, context as never), lookups }; +} + +function fakeService() { + const calls: unknown[] = []; + return { + calls, + listRemoteTables: async (...args: unknown[]) => { + calls.push(['listRemoteTables', ...args]); + return [{ name: 'Products', columnCount: 6 }]; + }, + importObject: async (...args: unknown[]) => { + calls.push(['importObject', ...args]); + return { name: 'products', definition: {}, review: [] }; + }, + }; +} + +describe('the MCP federation bridge gates on the federation capabilities', () => { + it('lists through the external-datasource service for a holder of the read capability', async () => { + const service = fakeService(); + const { bridge } = bridgeFor([FEDERATION_READ_CAPABILITY], service); + await expect(bridge.listRemoteTables('erp', { schema: 'x' })).resolves.toEqual([{ name: 'Products', columnCount: 6 }]); + expect(service.calls).toEqual([['listRemoteTables', 'erp', { schema: 'x' }]]); + }); + + it('imports for a holder of the write capability, with the options as sent', async () => { + const service = fakeService(); + const { bridge } = bridgeFor([FEDERATION_WRITE_CAPABILITY], service); + await bridge.importObject('erp', 'Products', { writable: true }); + expect(service.calls).toEqual([['importObject', 'erp', 'Products', { writable: true }]]); + }); + + it('refuses 403 without the capability, before the service is looked up', async () => { + const service = fakeService(); + const read = bridgeFor([FEDERATION_WRITE_CAPABILITY], service); + await expect(read.bridge.listRemoteTables('erp', {})).rejects.toMatchObject({ + code: 'PERMISSION_DENIED', + status: 403, + message: expect.stringContaining(FEDERATION_READ_CAPABILITY), + }); + const write = bridgeFor([FEDERATION_READ_CAPABILITY], service); + await expect(write.bridge.importObject('erp', 'Products', {})).rejects.toMatchObject({ + code: 'PERMISSION_DENIED', + status: 403, + message: expect.stringContaining(FEDERATION_WRITE_CAPABILITY), + }); + expect(read.lookups).not.toContain('external-datasource'); + expect(write.lookups).not.toContain('external-datasource'); + expect(service.calls).toEqual([]); + }); + + it('answers 503 on a host with no federation service', async () => { + const { bridge } = bridgeFor([FEDERATION_READ_CAPABILITY], undefined); + await expect(bridge.listRemoteTables('erp', {})).rejects.toMatchObject({ code: 'SERVICE_UNAVAILABLE', status: 503 }); + }); +}); diff --git a/packages/runtime/src/domains/mcp.ts b/packages/runtime/src/domains/mcp.ts index 8c4d15657b6..53adedc5ce4 100644 --- a/packages/runtime/src/domains/mcp.ts +++ b/packages/runtime/src/domains/mcp.ts @@ -12,7 +12,8 @@ import { isMcpServerEnabled } from '@objectstack/types'; import { MCP_OAUTH_SCOPES } from '@objectstack/spec/ai'; import type { MetadataProtocol } from '@objectstack/spec/api'; import type { ISecurityService } from '@objectstack/spec/contracts'; -import type { AIActionConfirmation } from '@objectstack/spec/contracts'; +import type { AIActionConfirmation, IExternalDatasourceService } from '@objectstack/spec/contracts'; +import { FEDERATION_READ_CAPABILITY, FEDERATION_WRITE_CAPABILITY } from '@objectstack/rest'; import { buildApiError } from '../error-envelope.js'; import * as actionExec from '../action-execution.js'; import { isSystemObjectName } from '../action-execution.js'; @@ -879,9 +880,57 @@ export function buildMcpBridge(deps: DomainHandlerDeps, context: HttpProtocolCon ) => resumeActionRun(deps, context, runId, input ?? {}, { driver, envId, ec, getMeta, readObjects: readServedObjects, readActions: readServedActions, callData, }), + // ── Federation surface (McpFederationBridge, #27745) ─────── + // The `external-datasource` service Studio's datasource page reaches + // through `…/external/tables` and `…/tables/:remote/import`, refused + // on the same two capabilities those routes gate on. + listRemoteTables: async (datasource: string, opts: { schema?: string }) => + (await federationService(deps, context, ec, 'read')).listRemoteTables(datasource, opts), + importObject: async (datasource: string, remoteName: string, opts: { name?: string; writable?: boolean }) => + (await federationService(deps, context, ec, 'write')).importObject(datasource, remoteName, opts), }; } +/** + * [#27745] The `external-datasource` service for one federation tool call, or + * the refusal the REST twin answers: `403 PERMISSION_DENIED` without the + * capability the call's kind needs (asked first, as the route asks it, so a + * caller without it cannot learn which services the host wires), `503 + * SERVICE_UNAVAILABLE` on a host with no federation service. + */ +async function federationService( + deps: DomainHandlerDeps, + context: HttpProtocolContext, + ec: any, + kind: 'read' | 'write', +): Promise { + const required = kind === 'write' ? FEDERATION_WRITE_CAPABILITY : FEDERATION_READ_CAPABILITY; + const held: unknown[] = Array.isArray(ec?.systemPermissions) ? ec.systemPermissions : []; + if (!held.includes(required)) { + throw Object.assign( + new Error( + kind === 'write' + ? `Creating metadata from an external datasource requires the \`${required}\` capability.` + : `Introspecting an external datasource requires the \`${required}\` capability.`, + ), + { code: 'PERMISSION_DENIED', status: 403 }, + ); + } + let service: IExternalDatasourceService | undefined; + try { + service = await deps.resolveService(context, 'external-datasource', context.environmentId); + } catch { + service = undefined; + } + if (!service) { + throw Object.assign(new Error('The external-datasource service is not available.'), { + code: 'SERVICE_UNAVAILABLE', + status: 503, + }); + } + return service; +} + /** * [#15705] Build the error a resume refusal throws, through the SAME * `deps.error` builder the REST resume door answers with. So the code diff --git a/packages/services/service-datasource/package.json b/packages/services/service-datasource/package.json index f1834bcdbfe..3e2d224c1fc 100644 --- a/packages/services/service-datasource/package.json +++ b/packages/services/service-datasource/package.json @@ -63,6 +63,7 @@ }, "devDependencies": { "@objectstack/driver-mongodb": "workspace:*", + "@objectstack/driver-odata": "workspace:*", "@objectstack/driver-sqlite-wasm": "workspace:*", "@objectstack/objectql": "workspace:*", "@objectstack/platform-objects": "workspace:*", diff --git a/packages/services/service-datasource/src/__tests__/__fixtures__/northwind-metadata.xml b/packages/services/service-datasource/src/__tests__/__fixtures__/northwind-metadata.xml new file mode 100644 index 00000000000..01feea3089d --- /dev/null +++ b/packages/services/service-datasource/src/__tests__/__fixtures__/northwind-metadata.xml @@ -0,0 +1,104 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/packages/services/service-datasource/src/__tests__/external-object-draft-odata-metadata.test.ts b/packages/services/service-datasource/src/__tests__/external-object-draft-odata-metadata.test.ts new file mode 100644 index 00000000000..a1e87a9718d --- /dev/null +++ b/packages/services/service-datasource/src/__tests__/external-object-draft-odata-metadata.test.ts @@ -0,0 +1,265 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * `generateObjectDraft` / `importObject` for an OData datasource, driven off a + * REAL `ODataDriver.introspectSchema()` over a recorded `$metadata` document + * (`__fixtures__/northwind-metadata.xml`), not a hand-written schema. + * + * What an OData draft has to get right that a SQL one never met: + * + * - **EDM types.** The driver reports each column under its EDM name + * (`Edm.Int32`) and its dialect as `odata-4.0`; the type matrix reads them + * through its `odata` arm. Without it every column drafted as `text` and + * Gate 2 refused the saved object. + * - **Field names.** OData properties are PascalCase and `ObjectSchema` + * admits snake_case field keys only, so each field is the property's + * snake_case name and `external.columnMap` carries the remote spelling. + * - **Lookups.** A single-valued navigation becomes a `lookup` on the column + * its `ReferentialConstraint` names, when the target is imported; every + * other navigation is left out of the draft and named in its review. + * + * `@objectstack/driver-odata` resolves to the driver's `src/` through this + * package's vitest alias, so the draft is judged against the introspection in + * the checkout, not a build of it. + */ + +import { readFileSync } from 'node:fs'; +import { describe, it, expect } from 'vitest'; +import { ODataDriver, parseODataDriverConfig, type FetchLike } from '@objectstack/driver-odata'; +import { ObjectSchema } from '@objectstack/spec/data'; +import { + ExternalDatasourceService, + type DatasourceLike, + type ObjectLike, +} from '../external-datasource-service.js'; + +const SERVICE_ROOT = 'https://odata.example.test/V4/Northwind/Northwind.svc/'; +const NORTHWIND_METADATA = readFileSync(new URL('./__fixtures__/northwind-metadata.xml', import.meta.url), 'utf8'); + +/** A service that answers `$metadata` and records every other request. */ +function northwind(answers: Record = {}): { fetch: FetchLike; requests: string[] } { + const requests: string[] = []; + const fetch: FetchLike = async (input) => { + const url = decodeURIComponent(input).slice(SERVICE_ROOT.length); + requests.push(url); + if (url === '$metadata') { + return new Response(NORTHWIND_METADATA, { status: 200, headers: { 'Content-Type': 'application/xml' } }); + } + if (url in answers) { + return new Response(JSON.stringify(answers[url]), { status: 200, headers: { 'Content-Type': 'application/json' } }); + } + throw new Error(`northwind fixture: no answer recorded for GET ${url}`); + }; + return { fetch, requests }; +} + +function driverFor(answers?: Record) { + const service = northwind(answers); + const driver = new ODataDriver(parseODataDriverConfig({ name: 'erp', serviceRoot: SERVICE_ROOT }), { fetch: service.fetch }); + return { driver, requests: service.requests }; +} + +/** The service over the real driver, with `objects` as the registry and every save recorded. */ +function serviceOver(objects: ObjectLike[] = []) { + const { driver } = driverFor(); + const saved: Array<{ name: string; definition: Record }> = []; + const registry = [...objects]; + const service = new ExternalDatasourceService({ + introspect: () => driver.introspectSchema(), + getDatasource: async (name): Promise => ({ name, schemaMode: 'external' }), + getObject: async (name) => registry.find((o) => o.name === name), + listObjects: async () => registry, + persistObject: async (name, definition) => { + saved.push({ name, definition }); + registry.push(definition as unknown as ObjectLike); + }, + }); + return { service, saved, registry }; +} + +const imported = (name: string, remoteName: string, datasource = 'erp'): ObjectLike => ({ + name, + datasource, + external: { remoteName }, +}); + +describe('an OData entity set drafts with EDM types mapped and snake_case fields bound by columnMap', () => { + it('maps every EDM primitive the fixture uses through the matrix\'s odata arm', async () => { + const { service } = serviceOver(); + const draft = await service.generateObjectDraft('erp', 'Products'); + + expect(draft.name).toBe('products'); + expect(draft.definition.fields).toEqual({ + product_id: { type: 'number' }, + product_name: { type: 'text' }, + supplier_id: { type: 'number' }, + category_id: { type: 'number' }, + unit_price: { type: 'number' }, + discontinued: { type: 'boolean' }, + }); + expect(draft.definition.external).toEqual({ + remoteName: 'Products', + columnMap: { + ProductID: 'product_id', + ProductName: 'product_name', + SupplierID: 'supplier_id', + CategoryID: 'category_id', + UnitPrice: 'unit_price', + Discontinued: 'discontinued', + }, + }); + + const categories = await service.generateObjectDraft('erp', 'Categories'); + expect(categories.definition.fields).toMatchObject({ + category_id: { type: 'number' }, + description: { type: 'text' }, + picture: { type: 'file' }, + }); + const employees = await service.generateObjectDraft('erp', 'Employees'); + expect(employees.definition.fields).toMatchObject({ birth_date: { type: 'datetime' } }); + const lines = await service.generateObjectDraft('erp', 'Order_Details'); + expect(lines.definition.fields).toMatchObject({ quantity: { type: 'number' }, discount: { type: 'number' } }); + }); + + it('keeps the remote key, in its field spelling, in the source comment', async () => { + const { service } = serviceOver(); + const lines = await service.generateObjectDraft('erp', 'Order_Details'); + expect(lines.source).toContain('// Remote primary key: order_id, product_id'); + }); + + it('parses as an ObjectSchema, so the save door can take it', async () => { + const { service } = serviceOver([imported('categories', 'Categories')]); + const draft = await service.generateObjectDraft('erp', 'Products'); + const parsed = ObjectSchema.parse(draft.definition); + expect(parsed.external?.columnMap?.CategoryID).toBe('category_id'); + expect(parsed.fields?.category_id).toMatchObject({ type: 'lookup', reference: 'categories' }); + }); +}); + +describe('navigations become lookups only where a column holds the key of an imported target', () => { + it('leaves a navigation out, named, while its target is not imported', async () => { + const { service } = serviceOver(); + const draft = await service.generateObjectDraft('erp', 'Products'); + + expect(draft.definition.fields).toMatchObject({ category_id: { type: 'number' }, supplier_id: { type: 'number' } }); + expect(draft.review).toEqual([ + { + column: 'Category', + remoteType: 'navigation → Categories', + note: "navigation left out of the draft: 'Categories' is not imported on datasource 'erp'; import it before this table to get the lookup", + }, + { + column: 'Supplier', + remoteType: 'navigation → Suppliers', + note: "navigation left out of the draft: 'Suppliers' is not imported on datasource 'erp'; import it before this table to get the lookup", + }, + ]); + // Named in the source too, where the author reviews it. + expect(draft.source).toContain( + " // REVIEW: Category (navigation → Categories): navigation left out of the draft: 'Categories' is not imported", + ); + }); + + it('turns the constrained column into a lookup on the imported target', async () => { + const { service } = serviceOver([imported('categories', 'Categories')]); + const draft = await service.generateObjectDraft('erp', 'Products'); + + expect(draft.definition.fields).toMatchObject({ + category_id: { type: 'lookup', reference: 'categories' }, + supplier_id: { type: 'number' }, + }); + expect(draft.review.map((r) => r.column)).toEqual(['Supplier']); + expect(draft.source).toContain(" category_id: { type: 'lookup', reference: 'categories' },"); + expect(draft.source).toContain("columnMap: { 'ProductID': 'product_id',"); + }); + + it('reads "imported" per datasource: the same entity set on another datasource does not count', async () => { + const { service } = serviceOver([imported('crm_categories', 'Categories', 'crm')]); + const draft = await service.generateObjectDraft('erp', 'Products'); + expect(draft.definition.fields).toMatchObject({ category_id: { type: 'number' } }); + }); + + it('references the object itself for a navigation onto its own entity set, under an overriding name too', async () => { + const { service, saved } = serviceOver(); + const draft = await service.generateObjectDraft('erp', 'Employees'); + expect(draft.definition.fields).toMatchObject({ reports_to: { type: 'lookup', reference: 'employees' } }); + // The collection-valued `Employees1` is never a candidate, so it is not named either. + expect(draft.review).toEqual([]); + + await service.importObject('erp', 'Employees', { name: 'staff' }); + expect(saved[0]!.definition.fields).toMatchObject({ reports_to: { type: 'lookup', reference: 'staff' } }); + }); + + it('takes a lookup onto a single-key target from a composite-key entity set', async () => { + const { service } = serviceOver([imported('products', 'Products')]); + const draft = await service.generateObjectDraft('erp', 'Order_Details'); + expect(draft.definition.fields).toMatchObject({ + order_id: { type: 'number' }, + product_id: { type: 'lookup', reference: 'products' }, + }); + }); + + it('leaves a navigation out when its foreign-key column is not drafted', async () => { + const { service } = serviceOver([imported('categories', 'Categories')]); + const draft = await service.generateObjectDraft('erp', 'Products', { excludeColumns: ['CategoryID'] }); + expect(draft.review).toContainEqual({ + column: 'Category', + remoteType: 'navigation → Categories', + note: "navigation left out of the draft: its foreign-key property 'CategoryID' is not a column of this draft", + }); + }); + + it('carries the lookup on an import taken after the target\'s', async () => { + const { service, saved } = serviceOver(); + await service.importObject('erp', 'Categories'); + await service.importObject('erp', 'Products'); + expect(saved.map((s) => s.name)).toEqual(['categories', 'products']); + expect(saved[1]!.definition.fields).toMatchObject({ category_id: { type: 'lookup', reference: 'categories' } }); + }); +}); + +describe('an imported OData object passes Gate 2 against the same $metadata', () => { + it('validates every drafted field, the lookups included, with no diff', async () => { + const { service } = serviceOver(); + await service.importObject('erp', 'Categories'); + await service.importObject('erp', 'Suppliers'); + await service.importObject('erp', 'Products'); + await service.importObject('erp', 'Employees'); + + for (const name of ['categories', 'suppliers', 'products', 'employees']) { + const result = await service.validateObject(name); + expect(result, name).toEqual({ ok: true, datasource: 'erp', object: name, diffs: [] }); + } + }); + + it('NEGATIVE CONTROL — a field retyped against its EDM column is still refused', async () => { + const { service, registry } = serviceOver(); + await service.importObject('erp', 'Products'); + const products = registry.find((o) => o.name === 'products')!; + products.fields = { ...products.fields, discontinued: { type: 'datetime' } }; + const result = await service.validateObject('products'); + expect(result.ok).toBe(false); + expect(result.diffs).toEqual([ + expect.objectContaining({ kind: 'type_mismatch', column: 'Discontinued', expected: 'datetime', actual: 'Edm.Boolean' }), + ]); + }); +}); + +describe('the lookup the draft writes is one the driver can follow', () => { + it('reads the product\'s category_id as the CategoryID value, and finds the category by that id', async () => { + const { driver, requests } = driverFor({ + 'Products?$select=ProductID,CategoryID': { value: [{ ProductID: 1, CategoryID: 1 }] }, + 'Categories?$filter=CategoryID eq 1': { value: [{ CategoryID: 1, CategoryName: 'Beverages' }] }, + }); + const { service, registry } = serviceOver([imported('categories', 'Categories')]); + await service.importObject('erp', 'Products'); + await service.importObject('erp', 'Categories'); + for (const o of registry) driver.registerExternalObject(o); + + const [product] = await driver.find('products', { fields: ['id', 'category_id'] }); + expect(product).toMatchObject({ id: '1', category_id: 1 }); + const [category] = await driver.find('categories', { where: { id: product!.category_id } }); + expect(category).toMatchObject({ id: '1', category_name: 'Beverages' }); + expect(requests).toContain('Categories?$filter=CategoryID eq 1'); + }); +}); diff --git a/packages/services/service-datasource/src/external-datasource-service.ts b/packages/services/service-datasource/src/external-datasource-service.ts index 922e07c2df3..ec2389cd6db 100644 --- a/packages/services/service-datasource/src/external-datasource-service.ts +++ b/packages/services/service-datasource/src/external-datasource-service.ts @@ -323,15 +323,97 @@ function unattributedSchemaFilterError(args: { return err; } -/** Normalise a remote table name into a snake_case object name. */ +/** + * Normalise a remote table name into a snake_case object name: the name + * itself when it already is one, else its snake_case spelling + * ({@link toFieldName}'s rule) — an OData entity set is PascalCase + * (`Products`, `OrderDetails` → `products`, `order_details`). [#27745] The + * leading capital used to be prefixed before the name was lower-cased, so + * `Products` drafted as `_products`. + */ function toObjectName(remoteName: string): string { const { name } = parseQualified(remoteName); - return name - .replace(/[^a-zA-Z0-9_]/g, '_') - .replace(/^[^a-z_]/, (c) => `_${c.toLowerCase()}`) + return toFieldName(name); +} + +/** The field-name grammar `ObjectSchema` admits (a snake_case machine name). */ +const FIELD_NAME = /^[a-z_][a-z0-9_]*$/; + +/** + * A remote column's field name: the column's own name when it is already a + * field name (the SQL path, unchanged), else its snake_case spelling — + * `CustomerID` → `customer_id`, `FirstName` → `first_name`. An OData + * property is PascalCase by convention, and `ObjectSchema` refuses a field + * key that is not snake_case, so a draft that kept the remote spelling could + * not be saved. The remote name survives in `external.columnMap`. + */ +function toFieldName(column: string): string { + if (FIELD_NAME.test(column)) return column; + const snake = column + .replace(/([a-z0-9])([A-Z])/g, '$1_$2') + .replace(/([A-Z]+)([A-Z][a-z])/g, '$1_$2') + .replace(/[^a-zA-Z0-9]+/g, '_') + .replace(/^_+|_+$/g, '') .toLowerCase(); + if (!snake) return '_'; + return /^[0-9]/.test(snake) ? `_${snake}` : snake; +} + +/** + * The type vocabulary the matrix reads a driver's columns in, from the + * introspection's `dialect`. A driver reports its own dialect name; the OData + * driver adds its protocol version (`odata-4.0`), and every OData version + * names its primitives in the one EDM vocabulary the matrix's `odata` arm + * holds. + */ +function typeVocabulary(dialect: string | undefined): SqlDialect | undefined { + if (dialect === undefined) return undefined; + if (dialect === 'odata' || dialect.startsWith('odata-')) return 'odata'; + return dialect as SqlDialect; +} + +/** + * A single-valued navigation the introspection offers as a lookup candidate. + * The OData driver's `introspectSchema` lists them on each table beside its + * columns (`ODataIntrospectedTable.navigationProperties`); a SQL table has + * none. Read structurally, because `IntrospectedTable` declares no such key. + */ +interface NavigationCandidate { + name: string; + targetEntitySet?: string; + referentialConstraints: Array<{ property: string; referencedProperty: string }>; } +function navigationCandidates(table: IntrospectedTable): NavigationCandidate[] { + const raw = (table as unknown as { navigationProperties?: unknown }).navigationProperties; + if (!Array.isArray(raw)) return []; + const out: NavigationCandidate[] = []; + for (const nav of raw) { + if (!nav || typeof nav !== 'object' || typeof (nav as { name?: unknown }).name !== 'string') continue; + const { name, targetEntitySet, referentialConstraints } = nav as { + name: string; + targetEntitySet?: unknown; + referentialConstraints?: unknown; + }; + out.push({ + name, + ...(typeof targetEntitySet === 'string' ? { targetEntitySet } : {}), + referentialConstraints: Array.isArray(referentialConstraints) + ? referentialConstraints.filter( + (rc): rc is { property: string; referencedProperty: string } => + !!rc && + typeof (rc as { property?: unknown }).property === 'string' && + typeof (rc as { referencedProperty?: unknown }).referencedProperty === 'string', + ) + : [], + }); + } + return out; +} + +/** A field of a generated draft: a scalar, or a lookup over a foreign-key column. */ +type DraftField = { type: FieldType; reference?: string }; + /** * The org-wide default every generated federated object declares. * @@ -767,6 +849,8 @@ export class ExternalDatasourceService implements IExternalDatasourceService { datasource: string, remoteName: string, opts: GenerateDraftOpts, + /** The name the object will be saved under, when the caller overrides the derived one. */ + nameOverride?: string, ): Promise { const schema = await this.config.introspect(datasource); const table = this.findTable(schema, remoteName); @@ -775,7 +859,7 @@ export class ExternalDatasourceService implements IExternalDatasourceService { `Remote table '${remoteName}' not found on datasource '${datasource}'.`, ); } - const dialect = schema.dialect as SqlDialect | undefined; + const dialect = typeVocabulary(schema.dialect); // Derive the remote schema from the matched table's qualified name (the // caller may pass an unqualified `remoteName`). const matched = parseQualified(table.name); @@ -786,18 +870,34 @@ export class ExternalDatasourceService implements IExternalDatasourceService { const exclude = opts.excludeColumns ? new Set(opts.excludeColumns) : new Set(); const pkOverride = opts.primaryKey ? new Set(opts.primaryKey) : undefined; - const fields: Record = {}; + const fields: Record = {}; // The remote key is collected here rather than onto the field, because // there is no authorable field key to put it on — see // REMOTE_PRIMARY_KEY_COMMENT for the ruling and the measurements. const primaryKeyFields: string[] = []; const review: ObjectDraft['review'] = []; + // remote column → local field, for every column whose field is named + // differently: the binding the driver reads and Gate 2 inverts. + const columnMap: Record = {}; for (const col of table.columns) { if (include && !include.has(col.name)) continue; if (exclude.has(col.name)) continue; - const fieldName = opts.rename?.[col.name] ?? col.name; + let fieldName = opts.rename?.[col.name] ?? toFieldName(col.name); + if (Object.prototype.hasOwnProperty.call(fields, fieldName)) { + // Two remote columns that spell one field name (`OrderId`, `order_id`): + // the later one is numbered, never merged into the earlier. + let n = 2; + while (Object.prototype.hasOwnProperty.call(fields, `${fieldName}_${n}`)) n += 1; + review.push({ + column: col.name, + remoteType: col.type, + note: `its field name '${fieldName}' is taken by another column — named '${fieldName}_${n}', rename it`, + }); + fieldName = `${fieldName}_${n}`; + } + if (fieldName !== col.name) columnMap[col.name] = fieldName; const suggested = suggestFieldTypeForSqlType(col.type, dialect); const fieldType: FieldType = suggested ?? 'text'; if (!suggested) { @@ -826,6 +926,19 @@ export class ExternalDatasourceService implements IExternalDatasourceService { const namespace = normaliseNamespace(await this.config.getNamespace?.(datasource)); const shortName = toObjectName(resolvedRemoteName); const name = applyNamespacePrefix(shortName, namespace); + + await this.draftLookups({ + datasource, + schema, + table, + selfRemoteName: resolvedRemoteName, + selfName: nameOverride ?? name, + dialect, + fields, + columnMap, + review, + }); + const definition: Record = { name, // Derived from the SHORT name on purpose: the prefix is an addressing @@ -837,6 +950,7 @@ export class ExternalDatasourceService implements IExternalDatasourceService { external: { ...(remoteSchema ? { remoteSchema } : {}), remoteName: resolvedRemoteName, + ...(Object.keys(columnMap).length > 0 ? { columnMap } : {}), }, fields, // ADR-0090 D1 / `security-owd-unset` — see GENERATED_SHARING_MODEL. @@ -852,6 +966,111 @@ export class ExternalDatasourceService implements IExternalDatasourceService { }; } + /** + * [#27745] The lookups of a draft: one per single-valued navigation the + * introspection offers ({@link navigationCandidates}) that a column can back + * and whose target is imported. Each turns the column holding the target's + * key into a `lookup` field referencing the target's object. Every other + * navigation is left out of the draft and named in its review, with the + * reason: + * + * - it is bound to no entity set, or carries no `ReferentialConstraint` + * (TripPin's `BestFriend`): no column holds the target's key, and the + * driver never follows a navigation to read one; + * - its constraint spans more than one property, its dependent property is + * not a column of this draft, or the property it references is not the + * target's single key (a lookup holds the target's `id`, and the driver's + * `id` is the key only for a single-property key); + * - the dependent column's type cannot back a lookup by the type matrix, + * the verdict Gate 2 would reach on the saved object; + * - the target is not imported. Imported means an object on THIS datasource + * bound to the target entity set, or the object itself for a navigation + * onto its own entity set. Import the target first: a draft taken after + * it carries the lookup, while re-saving an object already imported with + * the column as a scalar is a retype the save door's destructive-change + * gate refuses. + * + * A left-out entry's `remoteType` is `navigation → `, + * which Studio's batch import reads to save targets first. + */ + private async draftLookups(args: { + datasource: string; + schema: IntrospectedSchema; + table: IntrospectedTable; + selfRemoteName: string; + selfName: string; + dialect: SqlDialect | undefined; + fields: Record; + columnMap: Record; + review: ObjectDraft['review']; + }): Promise { + const { datasource, schema, table, selfRemoteName, selfName, dialect, fields, columnMap, review } = args; + const candidates = navigationCandidates(table); + if (candidates.length === 0) return; + + const columns = new Map(table.columns.map((c) => [c.name, c])); + // Remote entity set → the object bound to it on this datasource, read once + // and only when a candidate gets that far. + let imported: Map | undefined; + const importedObject = async (entitySet: string): Promise => { + if (!imported) { + imported = new Map(); + for (const o of await this.config.listObjects()) { + if ((o.datasource ?? DEFAULT_DATASOURCE_NAME) !== datasource) continue; + const bound = o.external?.remoteName ?? o.name; + if (!imported.has(bound)) imported.set(bound, o.name); + } + } + return imported.get(entitySet); + }; + + for (const nav of candidates) { + const target = nav.targetEntitySet; + const leftOut = (why: string) => + review.push({ + column: nav.name, + remoteType: `navigation → ${target ?? '(unbound)'}`, + note: `navigation left out of the draft: ${why}`, + }); + if (!target) { + leftOut(`$metadata binds it to no entity set`); + continue; + } + if (nav.referentialConstraints.length !== 1) { + leftOut( + nav.referentialConstraints.length === 0 + ? `no ReferentialConstraint names a property holding the key of '${target}'` + : `its ReferentialConstraint spans ${nav.referentialConstraints.length} properties (a composite reference)`, + ); + continue; + } + const { property, referencedProperty } = nav.referentialConstraints[0]!; + const column = columns.get(property); + const field = columnMap[property] ?? property; + if (!column || !Object.prototype.hasOwnProperty.call(fields, field)) { + leftOut(`its foreign-key property '${property}' is not a column of this draft`); + continue; + } + const targetTable = this.findTable(schema, target); + const isTargetKey = targetTable ? primaryKeyReader(targetTable) : undefined; + const targetKeys = targetTable && isTargetKey ? targetTable.columns.filter(isTargetKey).map((c) => c.name) : []; + if (targetKeys.length !== 1 || targetKeys[0] !== referencedProperty) { + leftOut(`it references '${referencedProperty}', which is not the single key of '${target}'`); + continue; + } + if (isCompatible(column.type, 'lookup', dialect) !== true) { + leftOut(`its foreign-key property '${property}' is ${column.type}, which cannot back a lookup`); + continue; + } + const reference = target === selfRemoteName ? selfName : await importedObject(target); + if (!reference) { + leftOut(`'${target}' is not imported on datasource '${datasource}'; import it before this table to get the lookup`); + continue; + } + fields[field] = { type: 'lookup', reference }; + } + } + async importObject( datasource: string, remoteName: string, @@ -888,7 +1107,7 @@ export class ExternalDatasourceService implements IExternalDatasourceService { } // Reuse the draft pipeline (type mapping, review notes, external binding). - const draft = await this.draftObject(datasource, remoteName, opts); + const draft = await this.draftObject(datasource, remoteName, opts, opts.name); // Apply the runtime-persona overrides on top of the draft definition. const name = opts.name ?? draft.name; @@ -941,7 +1160,7 @@ export class ExternalDatasourceService implements IExternalDatasourceService { sqlType: c.type, nullable: c.nullable, primaryKey: isPk(c), - suggestedFieldType: suggestFieldTypeForSqlType(c.type, schema.dialect as SqlDialect), + suggestedFieldType: suggestFieldTypeForSqlType(c.type, typeVocabulary(schema.dialect)), })), }; }), @@ -994,7 +1213,7 @@ export class ExternalDatasourceService implements IExternalDatasourceService { } const schema = await readSchema(datasource); - const dialect = schema.dialect as SqlDialect | undefined; + const dialect = typeVocabulary(schema.dialect); const remoteName = obj.external?.remoteName ?? obj.name; const table = this.findTable(schema, remoteName); @@ -1227,6 +1446,11 @@ export class ExternalDatasourceService implements IExternalDatasourceService { } } +/** A single-quoted TypeScript string literal for `value`. */ +function quoteTs(value: string): string { + return `'${value.replace(/\\/g, '\\\\').replace(/'/g, "\\'")}'`; +} + /** * Render a reviewable `*.object.ts` source string for an object draft. * @@ -1263,19 +1487,38 @@ export class ExternalDatasourceService implements IExternalDatasourceService { */ function renderObjectSource( definition: Record, - fields: Record, + fields: Record, review: ObjectDraft['review'], namespace?: string, primaryKeyFields: readonly string[] = [], ): string { - const reviewByColumn = new Map(review.map((r) => [r.column, r.note])); - const external = definition.external as { remoteSchema?: string; remoteName?: string }; + const external = definition.external as { + remoteSchema?: string; + remoteName?: string; + columnMap?: Record; + }; + // A review entry names its REMOTE column; the field it lands on is that + // column's mapped name. An entry naming no field (a navigation left out of + // the draft) is rendered after the fields instead. + const fieldOf = (column: string): string => external.columnMap?.[column] ?? column; + const reviewByField = new Map(); + const unplaced: ObjectDraft['review'] = []; + for (const r of review) { + const field = fieldOf(r.column); + if (Object.prototype.hasOwnProperty.call(fields, field) && !reviewByField.has(field)) { + reviewByField.set(field, r.note); + } else { + unplaced.push(r); + } + } const fieldLines = Object.entries(fields).map(([fieldName, f]) => { - const note = reviewByColumn.get(fieldName); + const note = reviewByField.get(fieldName); const comment = note ? ` // REVIEW: ${note}` : ''; - return ` ${fieldName}: { type: '${f.type}' },${comment}`; + const reference = f.reference ? `, reference: ${quoteTs(f.reference)}` : ''; + return ` ${fieldName}: { type: '${f.type}'${reference} },${comment}`; }); + const unplacedLines = unplaced.map((r) => ` // REVIEW: ${r.column} (${r.remoteType}): ${r.note}`); // The whole of ruling D's second half: information for the reader, zero // contract face. Rendered only when a key was actually reported — with no @@ -1295,9 +1538,14 @@ function renderObjectSource( ] : []; + const columnMapEntries = Object.entries(external.columnMap ?? {}); + const columnMapPart = + columnMapEntries.length > 0 + ? `, columnMap: { ${columnMapEntries.map(([remote, field]) => `${quoteTs(remote)}: ${quoteTs(field)}`).join(', ')} }` + : ''; const externalLine = external.remoteSchema - ? ` external: { remoteSchema: '${external.remoteSchema}', remoteName: '${external.remoteName}' },` - : ` external: { remoteName: '${external.remoteName}' },`; + ? ` external: { remoteSchema: '${external.remoteSchema}', remoteName: '${external.remoteName}'${columnMapPart} },` + : ` external: { remoteName: '${external.remoteName}'${columnMapPart} },`; // No namespace resolved → the name is bare. That is legal in a stack whose // manifest declares no `namespace` (`defineStack` skips the check), so the @@ -1331,6 +1579,7 @@ function renderObjectSource( ...primaryKeyComment, ` fields: {`, ...fieldLines, + ...unplacedLines, ` },`, ` // Org-wide default (OWD): who can see records they do NOT own. ADR-0090 D1`, ` // requires this to be an authored decision rather than an accident — the`, diff --git a/packages/services/service-datasource/vitest.config.ts b/packages/services/service-datasource/vitest.config.ts index 4a17cccf261..dcd3e3d086d 100644 --- a/packages/services/service-datasource/vitest.config.ts +++ b/packages/services/service-datasource/vitest.config.ts @@ -60,7 +60,12 @@ export default defineConfig({ // walled bridge sitting in its source. `platform-objects` gets the explicit // subpath entry its non-uniform export map needs (the shape // `service-settings` uses). + // `@objectstack/driver-odata` is a TEST-layer import of the OData draft + // pin (`external-object-draft-odata-metadata.test.ts`, objectstack#27745), + // which drives the real driver's introspection; aliased so the draft is + // judged against the introspection in the driver's source. alias: [ + { find: /^@objectstack\/driver-odata$/, replacement: path.resolve(__dirname, '../../drivers/driver-odata/src/index.ts') }, { find: /^@objectstack\/core$/, replacement: path.resolve(__dirname, '../../core/src/index.ts') }, { find: /^@objectstack\/metadata-core$/, replacement: path.resolve(__dirname, '../../metadata-core/src/index.ts') }, { find: /^@objectstack\/objectql$/, replacement: path.resolve(__dirname, '../../objectql/src/index.ts') }, diff --git a/packages/spec/src/data/type-compat.test.ts b/packages/spec/src/data/type-compat.test.ts index 6d4c1efff01..8a0d9502f53 100644 --- a/packages/spec/src/data/type-compat.test.ts +++ b/packages/spec/src/data/type-compat.test.ts @@ -41,6 +41,31 @@ describe('canonicalizeSqlType (ADR-0015 §4.6)', () => { expect(canonicalizeSqlType('objectId', 'mongo')).toBe('text'); }); + it('applies the odata dialect: EDM primitives, case-insensitively, and collections as arrays', () => { + expect(canonicalizeSqlType('Edm.String', 'odata')).toBe('text'); + expect(canonicalizeSqlType('Edm.Int32', 'odata')).toBe('integer'); + expect(canonicalizeSqlType('Edm.Int64', 'odata')).toBe('bigint'); + expect(canonicalizeSqlType('Edm.Decimal', 'odata')).toBe('decimal'); + expect(canonicalizeSqlType('Edm.Double', 'odata')).toBe('float'); + expect(canonicalizeSqlType('Edm.Boolean', 'odata')).toBe('boolean'); + expect(canonicalizeSqlType('Edm.Date', 'odata')).toBe('date'); + expect(canonicalizeSqlType('Edm.TimeOfDay', 'odata')).toBe('time'); + expect(canonicalizeSqlType('Edm.DateTimeOffset', 'odata')).toBe('datetime'); + expect(canonicalizeSqlType('Edm.Guid', 'odata')).toBe('uuid'); + expect(canonicalizeSqlType('Edm.Binary', 'odata')).toBe('binary'); + expect(canonicalizeSqlType('Collection(Edm.String)', 'odata')).toBe('array'); + // The vocabulary is the dialect's own: no other dialect reads an EDM name. + expect(canonicalizeSqlType('Edm.Int32')).toBe('unknown'); + expect(canonicalizeSqlType('Edm.Int32', 'postgres')).toBe('unknown'); + }); + + it('leaves the EDM types with no field type unknown, so a draft flags them', () => { + expect(canonicalizeSqlType('Edm.Duration', 'odata')).toBe('unknown'); + expect(canonicalizeSqlType('Edm.GeographyPoint', 'odata')).toBe('unknown'); + // An enum or complex type is the service's own qualified name. + expect(canonicalizeSqlType('Trippin.PersonGender', 'odata')).toBe('unknown'); + }); + it('falls back to unknown for unrecognised types', () => { expect(canonicalizeSqlType('geography')).toBe('unknown'); expect(canonicalizeSqlType('')).toBe('unknown'); @@ -137,6 +162,25 @@ describe('isCompatible', () => { expect(isCompatible('enum', 'text')).toBe(true); }); + // A reference field stores the target's id, which on a federated object is + // the remote key: a foreign-key column of the key's type backs it exactly, + // and Gate 2 must not refuse a lookup over `CustomerID int` or an OData + // `ReferentialConstraint` over `Edm.Int32`. + it('accepts a foreign-key column of a key type for lookup and master_detail', () => { + for (const ft of ['lookup', 'master_detail'] as const) { + expect(isCompatible('varchar(36)', ft), ft).toBe(true); + expect(isCompatible('integer', ft), ft).toBe(true); + expect(isCompatible('bigint', ft), ft).toBe(true); + expect(isCompatible('uuid', ft), ft).toBe(true); + expect(isCompatible('Edm.Int32', ft, 'odata'), ft).toBe(true); + expect(isCompatible('Edm.String', ft, 'odata'), ft).toBe(true); + // Not a key type: no reference rides on these. + expect(isCompatible('boolean', ft), ft).toBe(false); + expect(isCompatible('jsonb', ft, 'postgres'), ft).toBe(false); + expect(isCompatible('numeric(10,2)', ft), ft).toBe(false); + } + }); + it('returns "lossy" for usable-but-imperfect mappings', () => { expect(isCompatible('jsonb', 'text', 'postgres')).toBe('lossy'); expect(isCompatible('date', 'datetime')).toBe('lossy'); @@ -176,7 +220,7 @@ describe('canonicalizeSqlType — Object.prototype fall-through', () => { // the union declares but `DIALECT_ALIASES` does not populate, which fall // through to the base map and must answer just as safely. const DIALECTS: readonly SqlDialect[] = [ - 'postgres', 'mysql', 'sqlite', 'snowflake', 'bigquery', 'mongo', + 'postgres', 'mysql', 'sqlite', 'snowflake', 'bigquery', 'mongo', 'odata', ]; // The declared return type is a private union, so the pin names it here. diff --git a/packages/spec/src/data/type-compat.ts b/packages/spec/src/data/type-compat.ts index d4a11103cf9..ea75c2c9d9d 100644 --- a/packages/spec/src/data/type-compat.ts +++ b/packages/spec/src/data/type-compat.ts @@ -26,14 +26,19 @@ import type { FieldType } from './field.zod'; -/** SQL dialects whose type vocabularies the matrix understands. */ +/** + * Remote type vocabularies the matrix understands: SQL dialects, plus the two + * non-SQL ones a federation driver reports (`mongo`'s BSON types, `odata`'s + * EDM primitives). + */ export type SqlDialect = | 'postgres' | 'mysql' | 'sqlite' | 'snowflake' | 'bigquery' - | 'mongo'; + | 'mongo' + | 'odata'; /** * Result of a compatibility check: @@ -105,6 +110,20 @@ const DIALECT_ALIASES: Partial{tr('engine.externalDatasource.validation', locale)} - + diff --git a/ui/packages/app-shell/src/views/metadata-admin/external/ImportObjectDialog.tsx b/ui/packages/app-shell/src/views/metadata-admin/external/ImportObjectDialog.tsx index b60dc75c790..cd58616d12b 100644 --- a/ui/packages/app-shell/src/views/metadata-admin/external/ImportObjectDialog.tsx +++ b/ui/packages/app-shell/src/views/metadata-admin/external/ImportObjectDialog.tsx @@ -11,7 +11,9 @@ * type-compat matrix flagged as lossy/unknown, and the generated * source. * 3. "Import as Object" PUTs the draft's definition to the metadata - * store (`/meta/object/:name`) and reports success. + * store (`/meta/object/:name`) and reports success. The object is + * read-only unless the author ticks "Allow writes", offered only when + * the datasource itself allows writes (ADR-0015 Gate 3 needs both). * * The dialog never mutates the remote schema — it only reads the draft and * writes a local Object definition. @@ -21,6 +23,7 @@ import * as React from 'react'; import { AlertTriangle, Database, FileCode2, Loader2, CheckCircle2 } from 'lucide-react'; import { Button, + Checkbox, Dialog, DialogContent, DialogHeader, @@ -31,6 +34,7 @@ import { import { generateObjectDraft, importObjectDraft, + withWriteOptIn, ExternalServiceUnavailableError, type ObjectDraft, type RemoteTable, @@ -45,6 +49,11 @@ export interface ImportObjectDialogProps { onOpenChange: (open: boolean) => void; /** Notified after a successful import (so the parent can refresh). */ onImported?: (objectName: string) => void; + /** + * Whether the datasource permits writes (`external.allowWrites`). Only then + * is the per-object write opt-in offered; the object is read-only otherwise. + */ + allowWrites?: boolean; } type Phase = 'loading' | 'ready' | 'importing' | 'done' | 'error'; @@ -63,6 +72,7 @@ export function ImportObjectDialog({ open, onOpenChange, onImported, + allowWrites, }: ImportObjectDialogProps) { // The designer locale the dialog's own words read in (objectui#10862), the // one the console's language resolves to — as its host tab reads it. @@ -70,6 +80,7 @@ export function ImportObjectDialog({ const [phase, setPhase] = React.useState('loading'); const [draft, setDraft] = React.useState(null); const [error, setError] = React.useState(null); + const [writable, setWritable] = React.useState(false); // Load the draft whenever the dialog opens for a (new) table. React.useEffect(() => { @@ -78,6 +89,7 @@ export function ImportObjectDialog({ setPhase('loading'); setDraft(null); setError(null); + setWritable(false); (async () => { try { const d = await generateObjectDraft(datasource, table.name, { @@ -108,14 +120,14 @@ export function ImportObjectDialog({ setPhase('importing'); setError(null); try { - await importObjectDraft(draft); + await importObjectDraft(withWriteOptIn(draft, allowWrites === true && writable)); setPhase('done'); onImported?.(draft.name); } catch (err) { setError(err instanceof Error ? err.message : String(err)); setPhase('ready'); } - }, [draft, onImported]); + }, [draft, onImported, allowWrites, writable]); const remoteLabel = table ? [table.schema, table.name].filter(Boolean).join('.') : ''; const errorText = error === UNAVAILABLE ? tr('engine.externalDatasource.unavailable', locale) : error; @@ -199,6 +211,17 @@ export function ImportObjectDialog({ )} + {allowWrites && ( + + )} +
{tr('engine.externalDatasource.import.source', locale)} diff --git a/ui/packages/app-shell/src/views/metadata-admin/external/SchemaBrowser.batchImport-27745.test.tsx b/ui/packages/app-shell/src/views/metadata-admin/external/SchemaBrowser.batchImport-27745.test.tsx new file mode 100644 index 00000000000..f2b7594fcec --- /dev/null +++ b/ui/packages/app-shell/src/views/metadata-admin/external/SchemaBrowser.batchImport-27745.test.tsx @@ -0,0 +1,226 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * objectstack#27745 — the Studio flow over an OData datasource's entity sets: + * the tables tab lists them, "Import selected" imports several at once with + * lookup targets saved first, and an imported object stays read-only unless + * the author opts in (offered only when the datasource allows writes). + * + * The REST client is the REAL module (`./api`); `fetch` is answered here by a + * mocked federation service that behaves as the server does on the points the + * flow depends on: a draft names a navigation whose target is not imported as + * a review entry with `remoteType` `navigation → `, and the same + * draft taken after the target's save carries the lookup instead. + */ + +import * as React from 'react'; +import { describe, it, expect, vi, afterEach } from 'vitest'; +import { render, cleanup, act, fireEvent, screen } from '@testing-library/react'; +import { I18nProvider } from '@object-ui/i18n'; +import { SchemaBrowser } from './SchemaBrowser'; +import { importRemoteTables } from './api'; + +const ENTITY_SETS = [ + { name: 'Categories', columnCount: 4 }, + { name: 'Products', columnCount: 6 }, + { name: 'Order_Details', columnCount: 5 }, +]; + +/** Which entity set references which, as the Northwind `$metadata` declares it. */ +const NAVIGATIONS: Record> = { + Products: [{ nav: 'Category', target: 'Categories', column: 'category_id' }], + Order_Details: [{ nav: 'Product', target: 'Products', column: 'product_id' }], + Categories: [], +}; + +const objectName = (entitySet: string) => entitySet.toLowerCase(); + +/** The object definition a PUT /meta/object/:name carried. */ +interface SavedBody { + fields: Record; + external: { remoteName?: string; writable?: boolean }; +} + +interface Service { + saved: Array<{ name: string; body: SavedBody }>; + drafts: string[]; +} + +function jsonResponse(body: unknown, status = 200): Response { + return { + ok: status < 400, + status, + statusText: 'STATUS', + headers: new Headers(), + json: async () => body, + } as unknown as Response; +} + +function mockService(opts: { refuse?: string } = {}): Service { + const service: Service = { saved: [], drafts: [] }; + const imported = () => new Set(service.saved.map((s) => s.name)); + vi.stubGlobal( + 'fetch', + vi.fn(async (input: RequestInfo | URL, init?: RequestInit) => { + const url = typeof input === 'string' ? input : String(input); + const method = init?.method ?? 'GET'; + if (method === 'GET' && url.endsWith('/api/v1/datasources/erp/external/tables')) { + return jsonResponse({ success: true, data: { tables: ENTITY_SETS } }); + } + const draft = /\/api\/v1\/datasources\/erp\/external\/tables\/([^/]+)\/draft$/.exec(url); + if (method === 'POST' && draft) { + const set = decodeURIComponent(draft[1]!); + service.drafts.push(set); + const fields: Record = { id: { type: 'number' } }; + const review: Array<{ column: string; remoteType: string; note: string }> = []; + for (const n of NAVIGATIONS[set] ?? []) { + if (imported().has(objectName(n.target))) { + fields[n.column] = { type: 'lookup', reference: objectName(n.target) }; + } else { + fields[n.column] = { type: 'number' }; + review.push({ column: n.nav, remoteType: `navigation → ${n.target}`, note: 'navigation left out of the draft' }); + } + } + return jsonResponse({ + success: true, + data: { + draft: { + name: objectName(set), + datasource: 'erp', + definition: { name: objectName(set), datasource: 'erp', external: { remoteName: set }, fields }, + source: '', + review, + }, + }, + }); + } + const put = /\/api\/v1\/meta\/object\/([^/]+)$/.exec(url); + if (method === 'PUT' && put) { + const name = decodeURIComponent(put[1]!); + if (name === opts.refuse) { + return jsonResponse({ success: false, error: { code: 'FORBIDDEN', message: 'manage_metadata is required' } }, 403); + } + service.saved.push({ name, body: JSON.parse(String(init?.body)) }); + return jsonResponse({ success: true }); + } + throw new Error(`mock federation service: no route for ${method} ${url}`); + }), + ); + return service; +} + +afterEach(() => { + cleanup(); + vi.unstubAllGlobals(); + vi.restoreAllMocks(); +}); + +async function flush() { + await act(async () => { + for (let i = 0; i < 20; i++) await new Promise((r) => setTimeout(r, 0)); + }); +} + +function mount(allowWrites?: boolean) { + render( + + + , + ); +} + +const checkbox = (label: string) => screen.getByRole('checkbox', { name: label }); + +describe('importRemoteTables — targets are saved before the tables that reference them', () => { + it('saves Categories, then Products, then Order_Details, whatever order they were picked in', async () => { + const service = mockService(); + const results = await importRemoteTables('erp', [ENTITY_SETS[2]!, ENTITY_SETS[1]!, ENTITY_SETS[0]!]); + + expect(service.saved.map((s) => s.name)).toEqual(['categories', 'products', 'order_details']); + expect(service.saved[1]!.body.fields.category_id).toEqual({ type: 'lookup', reference: 'categories' }); + expect(service.saved[2]!.body.fields.product_id).toEqual({ type: 'lookup', reference: 'products' }); + expect(results.map((r) => [r.table.name, r.name, r.error])).toEqual([ + ['Categories', 'categories', undefined], + ['Products', 'products', undefined], + ['Order_Details', 'order_details', undefined], + ]); + }); + + it('does not wait on a target outside the selection: the navigation stays left out and named', async () => { + const service = mockService(); + const [result] = await importRemoteTables('erp', [ENTITY_SETS[1]!]); + expect(service.saved.map((s) => s.name)).toEqual(['products']); + expect(result!.review).toEqual([ + { column: 'Category', remoteType: 'navigation → Categories', note: 'navigation left out of the draft' }, + ]); + }); + + it('reports a refused save by its own words and goes on with the rest', async () => { + const service = mockService({ refuse: 'categories' }); + const results = await importRemoteTables('erp', [ENTITY_SETS[0]!, ENTITY_SETS[1]!]); + expect(results.find((r) => r.table.name === 'Categories')?.error).toBe('manage_metadata is required (FORBIDDEN)'); + // The target never landed, so its referrer is saved without the lookup. + expect(service.saved.map((s) => s.name)).toEqual(['products']); + expect(service.saved[0]!.body.fields.category_id).toEqual({ type: 'number' }); + }); + + it('saves objects read-only unless writes are opted into', async () => { + const service = mockService(); + await importRemoteTables('erp', [ENTITY_SETS[0]!]); + await importRemoteTables('erp', [ENTITY_SETS[1]!], { writable: true }); + expect(service.saved[0]!.body.external).toEqual({ remoteName: 'Categories' }); + expect(service.saved[1]!.body.external).toEqual({ remoteName: 'Products', writable: true }); + }); +}); + +describe('SchemaBrowser — the datasource page lists the entity sets and imports the selected ones', () => { + it('lists every entity set with a checkbox and imports the picked ones in dependency order', async () => { + const service = mockService(); + mount(false); + await flush(); + + for (const set of ENTITY_SETS) expect(screen.getByText(set.name)).toBeTruthy(); + // No write opt-in on a datasource that does not allow writes. + expect(screen.queryByText('Allow writes to this object (read-only otherwise)')).toBeNull(); + + fireEvent.click(checkbox('Select Products')); + fireEvent.click(checkbox('Select Categories')); + fireEvent.click(screen.getByRole('button', { name: 'Import selected (2)' })); + await flush(); + + expect(service.saved.map((s) => s.name)).toEqual(['categories', 'products']); + expect(service.saved.every((s) => s.body.external.writable === undefined)).toBe(true); + const summary = screen.getByTestId('schema-browser-batch-results'); + expect(summary.textContent).toContain('Imported 2 of 2 tables as objects.'); + expect(summary.textContent).toContain('Products→products'); + }); + + it('select-all picks every listed entity set', async () => { + const service = mockService(); + mount(false); + await flush(); + fireEvent.click(checkbox('Select all tables')); + fireEvent.click(screen.getByRole('button', { name: 'Import selected (3)' })); + await flush(); + expect(service.saved.map((s) => s.name)).toEqual(['categories', 'products', 'order_details']); + }); + + it('offers the write opt-in when the datasource allows writes, off until ticked', async () => { + const service = mockService(); + mount(true); + await flush(); + + fireEvent.click(checkbox('Select Categories')); + fireEvent.click(screen.getByRole('checkbox', { name: 'Allow writes to this object (read-only otherwise)' })); + fireEvent.click(screen.getByRole('button', { name: 'Import selected (1)' })); + await flush(); + expect(service.saved[0]!.body.external).toEqual({ remoteName: 'Categories', writable: true }); + + fireEvent.click(checkbox('Select Products')); + // The opt-in stays as the author left it; untick it for a read-only import. + fireEvent.click(screen.getByRole('checkbox', { name: 'Allow writes to this object (read-only otherwise)' })); + fireEvent.click(screen.getByRole('button', { name: 'Import selected (1)' })); + await flush(); + expect(service.saved[1]!.body.external).toEqual({ remoteName: 'Products' }); + }); +}); diff --git a/ui/packages/app-shell/src/views/metadata-admin/external/SchemaBrowser.tsx b/ui/packages/app-shell/src/views/metadata-admin/external/SchemaBrowser.tsx index 5b9d55e4702..82a8b7b4297 100644 --- a/ui/packages/app-shell/src/views/metadata-admin/external/SchemaBrowser.tsx +++ b/ui/packages/app-shell/src/views/metadata-admin/external/SchemaBrowser.tsx @@ -5,8 +5,10 @@ * Studio panel (ADR-0015 §6.4). * * Lists the remote tables a federated datasource exposes (filtered server-side - * by `external.allowedSchemas`), with an optional client-side text filter and - * a per-table "Import" action that opens {@link ImportObjectDialog}. + * by `external.allowedSchemas`) — an OData datasource's entity sets — with an + * optional client-side text filter, a per-table "Import" action that opens + * {@link ImportObjectDialog}, and "Import selected" for several at once + * ({@link importRemoteTables}, which saves lookup targets first). * * Data is fetched on demand (mount + explicit "Refresh"), never on a timer — * introspecting a warehouse is expensive, so the user stays in control. @@ -15,23 +17,35 @@ import * as React from 'react'; import { useDisplayLocale } from '@object-ui/i18n'; import { RefreshCw, Loader2, Search, Table2, Download, Columns3 } from 'lucide-react'; -import { Button } from '@object-ui/components'; +import { Button, Checkbox } from '@object-ui/components'; import { listRemoteTables, + importRemoteTables, ExternalServiceUnavailableError, type RemoteTable, + type RemoteTableImport, } from './api.js'; import { ImportObjectDialog } from './ImportObjectDialog.js'; -import { t as tr, useMetadataLocale } from '../i18n.js'; +import { t as tr, tFormat, useMetadataLocale } from '../i18n.js'; import { withNodes } from '../previews/row-nodes.js'; export interface SchemaBrowserProps { datasource: string; + /** Whether the datasource permits writes (`external.allowWrites`): only then is the write opt-in offered. */ + allowWrites?: boolean; } type LoadState = 'idle' | 'loading' | 'loaded' | 'error' | 'unavailable'; -export function SchemaBrowser({ datasource }: SchemaBrowserProps) { +/** A remote table's key in the list: its qualified name. */ +const tableKey = (t: RemoteTable) => [t.schema, t.name].filter(Boolean).join('.'); + +type BatchState = + | { phase: 'idle' } + | { phase: 'running'; done: number; count: number } + | { phase: 'done'; results: RemoteTableImport[] }; + +export function SchemaBrowser({ datasource, allowWrites }: SchemaBrowserProps) { // Dates and numbers on this surface read the display locale; a bare // `toLocale*()` call used the MACHINE's locale (objectui#9909). const displayLocale = useDisplayLocale(); @@ -45,6 +59,9 @@ export function SchemaBrowser({ datasource }: SchemaBrowserProps) { const [query, setQuery] = React.useState(''); const [importing, setImporting] = React.useState(null); const [dialogOpen, setDialogOpen] = React.useState(false); + const [selected, setSelected] = React.useState>(new Set()); + const [batchWritable, setBatchWritable] = React.useState(false); + const [batch, setBatch] = React.useState({ phase: 'idle' }); const load = React.useCallback(async () => { setState('loading'); @@ -52,6 +69,7 @@ export function SchemaBrowser({ datasource }: SchemaBrowserProps) { try { const rows = await listRemoteTables(datasource); setTables(rows); + setSelected(new Set()); setState('loaded'); } catch (err) { if (err instanceof ExternalServiceUnavailableError) { @@ -81,6 +99,37 @@ export function SchemaBrowser({ datasource }: SchemaBrowserProps) { setDialogOpen(true); }; + const toggle = (key: string, on: boolean) => + setSelected((prev) => { + const next = new Set(prev); + if (on) next.add(key); + else next.delete(key); + return next; + }); + const allFilteredSelected = filtered.length > 0 && filtered.every((t) => selected.has(tableKey(t))); + const toggleAll = (on: boolean) => + setSelected((prev) => { + const next = new Set(prev); + for (const t of filtered) { + if (on) next.add(tableKey(t)); + else next.delete(tableKey(t)); + } + return next; + }); + + const importSelected = async () => { + const chosen = tables.filter((t) => selected.has(tableKey(t))); + if (chosen.length === 0) return; + setBatch({ phase: 'running', done: 0, count: chosen.length }); + const results = await importRemoteTables(datasource, chosen, { + writable: allowWrites === true && batchWritable, + onProgress: (done) => setBatch({ phase: 'running', done, count: chosen.length }), + }); + setSelected(new Set()); + setBatch({ phase: 'done', results }); + }; + const running = batch.phase === 'running'; + if (state === 'unavailable') { return ; } @@ -107,6 +156,34 @@ export function SchemaBrowser({ datasource }: SchemaBrowserProps) {
+ {(selected.size > 0 || running) && ( +
+ {allowWrites && ( + + )} + +
+ )} + + {batch.phase === 'done' && } + {error && (
{error} @@ -132,6 +209,14 @@ export function SchemaBrowser({ datasource }: SchemaBrowserProps) { + @@ -146,9 +231,17 @@ export function SchemaBrowser({ datasource }: SchemaBrowserProps) { {filtered.map((t) => { - const key = [t.schema, t.name].filter(Boolean).join('.'); + const key = tableKey(t); return ( +
+ toggleAll(v === true)} + aria-label={tr('engine.externalDatasource.browser.selectAll', locale)} + /> + {tr('engine.externalDatasource.browser.colTable', locale)}
+ toggle(key, v === true)} + aria-label={tFormat('engine.externalDatasource.browser.selectRow', locale, { table: key })} + /> + @@ -186,11 +279,63 @@ export function SchemaBrowser({ datasource }: SchemaBrowserProps) { table={importing} open={dialogOpen} onOpenChange={setDialogOpen} + allowWrites={allowWrites} /> ); } +/** + * What "Import selected" did, table by table, in the order the tables were + * saved: the object each landed as and how many review notes its draft + * carries (the notes themselves are in the single-table dialog), or the + * refusal, as the server worded it. + */ +function BatchResults({ results, locale }: { results: RemoteTableImport[]; locale: string }) { + const imported = results.filter((r) => r.name && !r.error).length; + return ( +
+
+ {tFormat( + imported === 1 + ? 'engine.externalDatasource.browser.batchDoneOne' + : 'engine.externalDatasource.browser.batchDoneOther', + locale, + { count: imported, total: results.length }, + )} +
+
    + {results.map((r) => ( +
  • + {tableKey(r.table)} + {r.error ? ( + — {r.error} + ) : ( + <> + → + {r.name} + {r.review.length > 0 && ( + + ( + {tFormat( + r.review.length === 1 + ? 'engine.externalDatasource.import.reviewOne' + : 'engine.externalDatasource.import.reviewOther', + locale, + { count: r.review.length }, + )} + ) + + )} + + )} +
    • + ))} +
+
+ ); +} + function UnavailableHint({ locale }: { locale: string }) { return (
diff --git a/ui/packages/app-shell/src/views/metadata-admin/external/api.ts b/ui/packages/app-shell/src/views/metadata-admin/external/api.ts index a9443dd7c94..a9d87f34e82 100644 --- a/ui/packages/app-shell/src/views/metadata-admin/external/api.ts +++ b/ui/packages/app-shell/src/views/metadata-admin/external/api.ts @@ -259,3 +259,81 @@ export async function importObjectDraft(draft: ObjectDraft): Promise { (typeof flat === 'string' && flat ? flat : `${res.status} ${res.statusText}`), ); } + +/** + * The draft with its runtime-persona write opt-in applied: `external.writable` + * set when the author asked for writes, the draft as the server wrote it + * otherwise. Federated objects are read-only unless opted in (ADR-0015 Gate 3, + * which also needs the datasource's own `external.allowWrites`). + */ +export function withWriteOptIn(draft: ObjectDraft, writable: boolean): ObjectDraft { + if (!writable) return draft; + const external = (draft.definition.external ?? {}) as Record; + return { ...draft, definition: { ...draft.definition, external: { ...external, writable: true } } }; +} + +/** One table's outcome in {@link importRemoteTables}. */ +export interface RemoteTableImport { + table: RemoteTable; + /** The object name the draft was saved under, when the import landed. */ + name?: string; + /** The draft's review notes (lossy columns, navigations left out). */ + review: ObjectDraft['review']; + /** The refusal, when the draft or the save failed. */ + error?: string; +} + +/** + * Import several remote tables in one go, targets before the tables that + * reference them (objectstack#27745). + * + * A draft turns a navigation into a lookup only when its target is already + * imported, and an imported object cannot have a field retyped into a lookup + * later without the save door's destructive-change acknowledgement. So the + * order matters: each round drafts every table still pending and saves the + * ones whose draft waits on no other pending table, then drafts the rest again + * against the objects that round saved. A draft waits on a table when its + * review names it as a navigation target the draft left out — the review entry + * whose `remoteType` the service writes as `navigation → `. When a + * round saves nothing (a cycle), the remaining tables are saved as drafted, + * their navigations onto each other left out and named in their reviews. + */ +export async function importRemoteTables( + datasource: string, + tables: readonly RemoteTable[], + opts: { writable?: boolean; onProgress?: (done: number) => void } = {}, +): Promise { + const results: RemoteTableImport[] = []; + let pending = [...tables]; + let forced = false; + while (pending.length > 0) { + const waiting: RemoteTable[] = []; + for (const table of pending) { + let draft: ObjectDraft; + try { + draft = await generateObjectDraft(datasource, table.name, { remoteSchema: table.schema }); + } catch (err) { + results.push({ table, review: [], error: err instanceof Error ? err.message : String(err) }); + opts.onProgress?.(results.length); + continue; + } + const waitsOn = !forced && draft.review.some((r) => + pending.some((other) => other !== table && r.remoteType === `navigation → ${other.name}`), + ); + if (waitsOn) { + waiting.push(table); + continue; + } + try { + await importObjectDraft(withWriteOptIn(draft, opts.writable === true)); + results.push({ table, name: draft.name, review: draft.review }); + } catch (err) { + results.push({ table, review: draft.review, error: err instanceof Error ? err.message : String(err) }); + } + opts.onProgress?.(results.length); + } + forced = waiting.length === pending.length; + pending = waiting; + } + return results; +} diff --git a/ui/packages/i18n/src/namespaces/studio/en.ts b/ui/packages/i18n/src/namespaces/studio/en.ts index 378c6f77a26..b04587d7a57 100644 --- a/ui/packages/i18n/src/namespaces/studio/en.ts +++ b/ui/packages/i18n/src/namespaces/studio/en.ts @@ -1993,6 +1993,12 @@ const STUDIO_EN: Record = { 'engine.externalDatasource.browser.colColumns': 'Columns', 'engine.externalDatasource.browser.colRows': 'Rows (est.)', 'engine.externalDatasource.browser.import': 'Import', + 'engine.externalDatasource.browser.selectAll': 'Select all tables', + 'engine.externalDatasource.browser.selectRow': 'Select {table}', + 'engine.externalDatasource.browser.importSelected': 'Import selected ({count})', + 'engine.externalDatasource.browser.importingSelected': 'Importing {done} of {count}…', + 'engine.externalDatasource.browser.batchDoneOne': 'Imported {count} of {total} tables as an object.', + 'engine.externalDatasource.browser.batchDoneOther': 'Imported {count} of {total} tables as objects.', 'engine.externalDatasource.browser.unavailable': 'Federation is not enabled on this server. The {service} service must be registered for table browsing, drafting, and validation to work.', 'engine.externalDatasource.check.unavailable': @@ -2031,6 +2037,7 @@ const STUDIO_EN: Record = { 'engine.externalDatasource.import.reviewOther': '{count} columns need review', 'engine.externalDatasource.import.source': 'Generated source', 'engine.externalDatasource.import.importing': 'Importing…', + 'engine.externalDatasource.import.writable': 'Allow writes to this object (read-only otherwise)', 'engine.studio.data.tab.advanced': 'Advanced', 'engine.studio.data.nameLabel': 'Object name', 'engine.studio.data.idLabel': 'Identifier', diff --git a/ui/packages/i18n/src/namespaces/studio/zh.ts b/ui/packages/i18n/src/namespaces/studio/zh.ts index f02d0e0f4ee..5990a9735c8 100644 --- a/ui/packages/i18n/src/namespaces/studio/zh.ts +++ b/ui/packages/i18n/src/namespaces/studio/zh.ts @@ -1810,6 +1810,12 @@ const STUDIO_ZH: Record = { 'engine.externalDatasource.browser.colColumns': '列', 'engine.externalDatasource.browser.colRows': '行数（估算）', 'engine.externalDatasource.browser.import': '导入', + 'engine.externalDatasource.browser.selectAll': '选择全部表', + 'engine.externalDatasource.browser.selectRow': '选择 {table}', + 'engine.externalDatasource.browser.importSelected': '导入所选（{count}）', + 'engine.externalDatasource.browser.importingSelected': '正在导入 {done} / {count}…', + 'engine.externalDatasource.browser.batchDoneOne': '已将 {total} 个表中的 {count} 个导入为对象。', + 'engine.externalDatasource.browser.batchDoneOther': '已将 {total} 个表中的 {count} 个导入为对象。', 'engine.externalDatasource.browser.unavailable': '此服务器未启用数据联邦。必须注册 {service} 服务，才能浏览表、生成草稿并进行校验。', 'engine.externalDatasource.check.unavailable': '此服务器未启用数据联邦，因此无法进行校验。', @@ -1844,6 +1850,7 @@ const STUDIO_ZH: Record = { 'engine.externalDatasource.import.reviewOther': '{count} 个列需要复查', 'engine.externalDatasource.import.source': '生成的源码', 'engine.externalDatasource.import.importing': '正在导入…', + 'engine.externalDatasource.import.writable': '允许写入此对象（否则只读）', 'engine.studio.data.tab.advanced': '高级', 'engine.studio.data.nameLabel': '对象名称', 'engine.studio.data.idLabel': '标识',