From 610d65e143b6bd86bc0fe6cb1d250b6636bde3e4 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Tue, 7 Feb 2023 13:11:24 +0100 Subject: [PATCH] backend-plugin-api: make BackendFeature opaque Signed-off-by: Patrik Oldsberg --- .changeset/nervous-phones-shout.md | 6 ++ .changeset/yellow-roses-tell.md | 5 + .../src/wiring/BackendInitializer.ts | 81 +++++++------- packages/backend-app-api/src/wiring/types.ts | 6 +- packages/backend-plugin-api/api-report.md | 24 +---- .../src/wiring/factories.test.ts | 59 ++++++++-- .../src/wiring/factories.ts | 101 ++++++++++++++++-- .../backend-plugin-api/src/wiring/index.ts | 1 - .../backend-plugin-api/src/wiring/types.ts | 55 ++++++---- .../src/next/wiring/TestBackend.ts | 21 ++-- 10 files changed, 250 insertions(+), 109 deletions(-) create mode 100644 .changeset/nervous-phones-shout.md create mode 100644 .changeset/yellow-roses-tell.md diff --git a/.changeset/nervous-phones-shout.md b/.changeset/nervous-phones-shout.md new file mode 100644 index 0000000000..a4e46b3be9 --- /dev/null +++ b/.changeset/nervous-phones-shout.md @@ -0,0 +1,6 @@ +--- +'@backstage/backend-test-utils': patch +'@backstage/backend-app-api': patch +--- + +Updates to match new `BackendFeature` type. diff --git a/.changeset/yellow-roses-tell.md b/.changeset/yellow-roses-tell.md new file mode 100644 index 0000000000..ad645df294 --- /dev/null +++ b/.changeset/yellow-roses-tell.md @@ -0,0 +1,5 @@ +--- +'@backstage/backend-plugin-api': minor +--- + +Switched `BackendFeature` to be an opaque type. diff --git a/packages/backend-app-api/src/wiring/BackendInitializer.ts b/packages/backend-app-api/src/wiring/BackendInitializer.ts index 4bdee8b085..6c551f082f 100644 --- a/packages/backend-app-api/src/wiring/BackendInitializer.ts +++ b/packages/backend-app-api/src/wiring/BackendInitializer.ts @@ -26,10 +26,13 @@ import { EnumerableServiceHolder, ServiceOrExtensionPoint, } from './types'; +// Direct internal import to avoid duplication +// eslint-disable-next-line @backstage/no-forbidden-package-imports +import { InternalBackendFeature } from '@backstage/backend-plugin-api/src/wiring/types'; export class BackendInitializer { #startPromise?: Promise; - #features = new Map(); + #features = new Array(); #registerInits = new Array(); #extensionPoints = new Map, unknown>(); #serviceHolder: EnumerableServiceHolder; @@ -74,11 +77,22 @@ export class BackendInitializer { return Object.fromEntries(result); } - add(feature: BackendFeature, options?: TOptions) { + add(feature: BackendFeature) { if (this.#startPromise) { throw new Error('feature can not be added after the backend has started'); } - this.#features.set(feature, options); + if (feature.$$type !== '@backstage/BackendFeature') { + throw new Error( + `Failed to add feature, invalid type '${feature.$$type}'`, + ); + } + const internalFeature = feature as InternalBackendFeature; + if (internalFeature.version !== 'v1') { + throw new Error( + `Failed to add feature, invalid version '${internalFeature.version}'`, + ); + } + this.#features.push(internalFeature); } async start(): Promise { @@ -117,50 +131,39 @@ export class BackendInitializer { } // Initialize all features - for (const [feature] of this.#features) { - const provides = new Set>(); + for (const feature of this.#features) { + for (const r of feature.getRegistrations()) { + const provides = new Set>(); - let registerInit: BackendRegisterInit | undefined = undefined; + if (r.type === 'plugin') { + for (const [extRef, extImpl] of r.extensionPoints) { + if (this.#extensionPoints.has(extRef)) { + throw new Error( + `ExtensionPoint with ID '${extRef.id}' is already registered`, + ); + } + this.#extensionPoints.set(extRef, extImpl); + provides.add(extRef); + } + } - feature.register({ - registerExtensionPoint: (extensionPointRef, impl) => { - if (registerInit) { - throw new Error('registerExtensionPoint called after registerInit'); - } - if (this.#extensionPoints.has(extensionPointRef)) { - throw new Error(`API ${extensionPointRef.id} already registered`); - } - this.#extensionPoints.set(extensionPointRef, impl); - provides.add(extensionPointRef); - }, - registerInit: registerOptions => { - if (registerInit) { - throw new Error('registerInit must only be called once'); - } - registerInit = { - id: feature.id, - provides, - consumes: new Set(Object.values(registerOptions.deps)), - deps: registerOptions.deps, - init: registerOptions.init as BackendRegisterInit['init'], - }; - }, - }); - - if (!registerInit) { - throw new Error( - `registerInit was not called by register in ${feature.id}`, - ); + this.#registerInits.push({ + id: r.type === 'plugin' ? r.pluginId : `${r.pluginId}.${r.moduleId}`, + provides, + consumes: new Set(Object.values(r.init.deps)), + init: r.init, + }); } - - this.#registerInits.push(registerInit); } const orderedRegisterResults = this.#resolveInitOrder(this.#registerInits); for (const registerInit of orderedRegisterResults) { - const deps = await this.#getInitDeps(registerInit.deps, registerInit.id); - await registerInit.init(deps); + const deps = await this.#getInitDeps( + registerInit.init.deps, + registerInit.id, + ); + await registerInit.init.func(deps); } } diff --git a/packages/backend-app-api/src/wiring/types.ts b/packages/backend-app-api/src/wiring/types.ts index d3c4fc48c7..2a91bf46b9 100644 --- a/packages/backend-app-api/src/wiring/types.ts +++ b/packages/backend-app-api/src/wiring/types.ts @@ -34,8 +34,10 @@ export interface BackendRegisterInit { id: string; consumes: Set; provides: Set; - deps: { [name: string]: ServiceOrExtensionPoint }; - init: (deps: { [name: string]: unknown }) => Promise; + init: { + deps: { [name: string]: ServiceOrExtensionPoint }; + func: (deps: { [name: string]: unknown }) => Promise; + }; } /** diff --git a/packages/backend-plugin-api/api-report.md b/packages/backend-plugin-api/api-report.md index 4b8e0298ec..6dbafd0242 100644 --- a/packages/backend-plugin-api/api-report.md +++ b/packages/backend-plugin-api/api-report.md @@ -17,9 +17,7 @@ import { Readable } from 'stream'; // @public (undocumented) export interface BackendFeature { // (undocumented) - id: string; - // (undocumented) - register(reg: BackendRegistrationPoints): void; + $$type: '@backstage/BackendFeature'; } // @public @@ -72,26 +70,6 @@ export interface BackendPluginRegistrationPoints { }): void; } -// @public -export interface BackendRegistrationPoints { - // (undocumented) - registerExtensionPoint( - ref: ExtensionPoint, - impl: TExtensionPoint, - ): void; - // (undocumented) - registerInit< - Deps extends { - [name in string]: unknown; - }, - >(options: { - deps: { - [name in keyof Deps]: ServiceRef | ExtensionPoint; - }; - init(deps: Deps): Promise; - }): void; -} - // @public export interface CacheClient { delete(key: string): Promise; diff --git a/packages/backend-plugin-api/src/wiring/factories.test.ts b/packages/backend-plugin-api/src/wiring/factories.test.ts index 8e1b30ea53..ae199d84e4 100644 --- a/packages/backend-plugin-api/src/wiring/factories.test.ts +++ b/packages/backend-plugin-api/src/wiring/factories.test.ts @@ -19,6 +19,7 @@ import { createBackendPlugin, createExtensionPoint, } from './factories'; +import { InternalBackendFeature } from './types'; describe('createExtensionPoint', () => { it('should create an ExtensionPoint', () => { @@ -34,11 +35,30 @@ describe('createBackendPlugin', () => { it('should create a BackendPlugin', () => { const plugin = createBackendPlugin((_options: { a: string }) => ({ pluginId: 'x', - register() {}, + register(r) { + r.registerInit({ deps: {}, async init() {} }); + }, })); expect(plugin).toBeDefined(); expect(plugin({ a: 'a' })).toBeDefined(); - expect(plugin({ a: 'a' }).id).toBe('x'); + expect(plugin({ a: 'a' })).toEqual({ + $$type: '@backstage/BackendFeature', + version: 'v1', + getRegistrations: expect.any(Function), + }); + expect( + (plugin({ a: 'a' }) as InternalBackendFeature).getRegistrations(), + ).toEqual([ + { + type: 'plugin', + pluginId: 'x', + extensionPoints: [], + init: { + deps: expect.any(Object), + func: expect.any(Function), + }, + }, + ]); // @ts-expect-error expect(plugin()).toBeDefined(); // @ts-expect-error @@ -79,7 +99,11 @@ describe('createBackendPlugin', () => { })); expect(plugin).toBeDefined(); expect(plugin({ a: 'a' })).toBeDefined(); - expect(plugin({ a: 'a' }).id).toBe('x'); + expect(plugin({ a: 'a' })).toEqual({ + $$type: '@backstage/BackendFeature', + version: 'v1', + getRegistrations: expect.any(Function), + }); // @ts-expect-error expect(plugin()).toBeDefined(); // @ts-expect-error @@ -107,11 +131,30 @@ describe('createBackendModule', () => { const mod = createBackendModule((_options: { a: string }) => ({ pluginId: 'x', moduleId: 'y', - register() {}, + register(r) { + r.registerInit({ deps: {}, async init() {} }); + }, })); expect(mod).toBeDefined(); expect(mod({ a: 'a' })).toBeDefined(); - expect(mod({ a: 'a' }).id).toBe('x.y'); + expect(mod({ a: 'a' })).toEqual({ + $$type: '@backstage/BackendFeature', + version: 'v1', + getRegistrations: expect.any(Function), + }); + expect( + (mod({ a: 'a' }) as InternalBackendFeature).getRegistrations(), + ).toEqual([ + { + type: 'module', + pluginId: 'x', + moduleId: 'y', + init: { + deps: expect.any(Object), + func: expect.any(Function), + }, + }, + ]); // @ts-expect-error expect(mod()).toBeDefined(); // @ts-expect-error @@ -155,7 +198,11 @@ describe('createBackendModule', () => { })); expect(mod).toBeDefined(); expect(mod({ a: 'a' })).toBeDefined(); - expect(mod({ a: 'a' }).id).toBe('x.y'); + expect(mod({ a: 'a' })).toEqual({ + $$type: '@backstage/BackendFeature', + version: 'v1', + getRegistrations: expect.any(Function), + }); // @ts-expect-error expect(mod()).toBeDefined(); // @ts-expect-error diff --git a/packages/backend-plugin-api/src/wiring/factories.ts b/packages/backend-plugin-api/src/wiring/factories.ts index be55474904..928c933af0 100644 --- a/packages/backend-plugin-api/src/wiring/factories.ts +++ b/packages/backend-plugin-api/src/wiring/factories.ts @@ -19,6 +19,9 @@ import { BackendPluginRegistrationPoints, BackendFeature, ExtensionPoint, + InternalBackendFeature, + InternalBackendModuleRegistration, + InternalBackendPluginRegistration, } from './types'; /** @@ -86,11 +89,59 @@ export function createBackendPlugin( config: BackendPluginConfig | ((...params: TOptions) => BackendPluginConfig), ): (...params: TOptions) => BackendFeature { const configCallback = typeof config === 'function' ? config : () => config; - return (...options: TOptions) => { + return (...options: TOptions): InternalBackendFeature => { const c = configCallback(...options); + + let registrations: InternalBackendPluginRegistration[]; + return { - ...c, - id: c.pluginId, + $$type: '@backstage/BackendFeature', + version: 'v1', + getRegistrations() { + if (registrations) { + return registrations; + } + const extensionPoints: InternalBackendPluginRegistration['extensionPoints'] = + []; + let init: InternalBackendPluginRegistration['init'] | undefined = + undefined; + + c.register({ + registerExtensionPoint(ext, impl) { + if (init) { + throw new Error( + 'registerExtensionPoint called after registerInit', + ); + } + extensionPoints.push([ext, impl]); + }, + registerInit(regInit) { + if (init) { + throw new Error('registerInit must only be called once'); + } + init = { + deps: regInit.deps, + func: regInit.init, + }; + }, + }); + + if (!init) { + throw new Error( + `registerInit was not called by register in ${c.pluginId}`, + ); + } + + registrations = [ + { + type: 'plugin', + pluginId: c.pluginId, + extensionPoints, + init, + }, + ]; + return registrations; + }, }; }; } @@ -127,11 +178,49 @@ export function createBackendModule( config: BackendModuleConfig | ((...params: TOptions) => BackendModuleConfig), ): (...params: TOptions) => BackendFeature { const configCallback = typeof config === 'function' ? config : () => config; - return (...options: TOptions) => { + return (...options: TOptions): InternalBackendFeature => { const c = configCallback(...options); + + let registrations: InternalBackendModuleRegistration[]; + return { - id: `${c.pluginId}.${c.moduleId}`, - register: c.register, + $$type: '@backstage/BackendFeature', + version: 'v1', + getRegistrations() { + if (registrations) { + return registrations; + } + let init: InternalBackendModuleRegistration['init'] | undefined = + undefined; + + c.register({ + registerInit(regInit) { + if (init) { + throw new Error('registerInit must only be called once'); + } + init = { + deps: regInit.deps, + func: regInit.init, + }; + }, + }); + + if (!init) { + throw new Error( + `registerInit was not called by register in ${c.moduleId} module for ${c.pluginId}`, + ); + } + + registrations = [ + { + type: 'module', + pluginId: c.pluginId, + moduleId: c.moduleId, + init, + }, + ]; + return registrations; + }, }; }; } diff --git a/packages/backend-plugin-api/src/wiring/index.ts b/packages/backend-plugin-api/src/wiring/index.ts index e0c25eeb4e..242301ea4d 100644 --- a/packages/backend-plugin-api/src/wiring/index.ts +++ b/packages/backend-plugin-api/src/wiring/index.ts @@ -32,7 +32,6 @@ export { export type { BackendModuleRegistrationPoints, BackendPluginRegistrationPoints, - BackendRegistrationPoints, BackendFeature, ExtensionPoint, } from './types'; diff --git a/packages/backend-plugin-api/src/wiring/types.ts b/packages/backend-plugin-api/src/wiring/types.ts index ab8ce07284..c81ee3ceb7 100644 --- a/packages/backend-plugin-api/src/wiring/types.ts +++ b/packages/backend-plugin-api/src/wiring/types.ts @@ -35,26 +35,6 @@ export type ExtensionPoint = { $$type: '@backstage/ExtensionPoint'; }; -/** - * The callbacks passed to the `register` method of a backend feature; this is - * essentially a superset of {@link BackendPluginRegistrationPoints} and - * {@link BackendModuleRegistrationPoints}. - * - * @public - */ -export interface BackendRegistrationPoints { - registerExtensionPoint( - ref: ExtensionPoint, - impl: TExtensionPoint, - ): void; - registerInit(options: { - deps: { - [name in keyof Deps]: ServiceRef | ExtensionPoint; - }; - init(deps: Deps): Promise; - }): void; -} - /** * The callbacks passed to the `register` method of a backend plugin. * @@ -89,7 +69,36 @@ export interface BackendModuleRegistrationPoints { /** @public */ export interface BackendFeature { - // TODO(Rugvip): Try to get rid of the ID at this level, allowing for a feature to register multiple features as a bundle - id: string; - register(reg: BackendRegistrationPoints): void; + // NOTE: This type is opaque in order to simplify future API evolution. + $$type: '@backstage/BackendFeature'; +} + +/** @internal */ +export interface InternalBackendFeature extends BackendFeature { + version: 'v1'; + getRegistrations(): Array< + InternalBackendPluginRegistration | InternalBackendModuleRegistration + >; +} + +/** @internal */ +export interface InternalBackendPluginRegistration { + pluginId: string; + type: 'plugin'; + extensionPoints: Array, unknown]>; + init: { + deps: Record>; + func(deps: Record): Promise; + }; +} + +/** @internal */ +export interface InternalBackendModuleRegistration { + pluginId: string; + moduleId: string; + type: 'module'; + init: { + deps: Record | ExtensionPoint>; + func(deps: Record): Promise; + }; } diff --git a/packages/backend-test-utils/src/next/wiring/TestBackend.ts b/packages/backend-test-utils/src/next/wiring/TestBackend.ts index 27a8c5bceb..bbda9854d4 100644 --- a/packages/backend-test-utils/src/next/wiring/TestBackend.ts +++ b/packages/backend-test-utils/src/next/wiring/TestBackend.ts @@ -30,6 +30,7 @@ import { BackendFeature, ExtensionPoint, coreServices, + createBackendPlugin, } from '@backstage/backend-plugin-api'; import { mockServices } from '../services'; import { ConfigReader } from '@backstage/config'; @@ -193,16 +194,18 @@ export async function startTestBackend< backendInstancesToCleanUp.push(backend); - backend.add({ - id: `---test-extension-point-registrar`, - register(reg) { - for (const [ref, impl] of extensionPoints) { - reg.registerExtensionPoint(ref, impl); - } + backend.add( + createBackendPlugin({ + pluginId: `---test-extension-point-registrar`, + register(reg) { + for (const [ref, impl] of extensionPoints) { + reg.registerExtensionPoint(ref, impl); + } - reg.registerInit({ deps: {}, async init() {} }); - }, - }); + reg.registerInit({ deps: {}, async init() {} }); + }, + })(), + ); for (const feature of features) { backend.add(feature);