From d4d048b3fffb61d76f47efb065919d7ff80da635 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Mon, 5 Aug 2024 12:16:19 +0200 Subject: [PATCH] backend-*-api: refactor for more explicit internal feature types Signed-off-by: Patrik Oldsberg --- .../src/wiring/BackendInitializer.ts | 73 ++++++++++++++----- .../src/wiring/createBackendModule.test.ts | 6 +- .../src/wiring/createBackendPlugin.test.ts | 6 +- .../backend-plugin-api/src/wiring/types.ts | 10 ++- .../src/next/wiring/TestBackend.ts | 48 +++++++----- 5 files changed, 96 insertions(+), 47 deletions(-) diff --git a/packages/backend-app-api/src/wiring/BackendInitializer.ts b/packages/backend-app-api/src/wiring/BackendInitializer.ts index ddad94de14..902258b9b0 100644 --- a/packages/backend-app-api/src/wiring/BackendInitializer.ts +++ b/packages/backend-app-api/src/wiring/BackendInitializer.ts @@ -26,7 +26,13 @@ import { import { 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'; +import type { + InternalBackendFeature, + InternalBackendFeatureLoader, + InternalBackendRegistrations, +} from '@backstage/backend-plugin-api/src/wiring/types'; +// eslint-disable-next-line @backstage/no-forbidden-package-imports +import type { InternalServiceFactory } from '@backstage/backend-plugin-api/src/services/system/types'; import { ForwardedError, ConflictError } from '@backstage/errors'; import { featureDiscoveryServiceRef } from '@backstage/backend-plugin-api/alpha'; import { DependencyGraph } from '../lib/DependencyGraph'; @@ -44,7 +50,7 @@ export interface BackendRegisterInit { export class BackendInitializer { #startPromise?: Promise; - #features = new Array(); + #registrations = new Array(); #extensionPoints = new Map(); #serviceRegistry: ServiceRegistry; #registeredFeatures = new Array>(); @@ -101,21 +107,15 @@ export class BackendInitializer { } #addFeature(feature: BackendFeature) { - if (feature.$$type !== '@backstage/BackendFeature') { - throw new Error( - `Failed to add feature, invalid type '${feature.$$type}'`, - ); - } - if (isServiceFactory(feature)) { this.#serviceRegistry.add(feature); - } else if (isInternalBackendFeature(feature)) { + } else if (isBackendRegistrations(feature)) { if (feature.version !== 'v1') { throw new Error( `Failed to add feature, invalid version '${feature.version}'`, ); } - this.#features.push(feature); + this.#registrations.push(feature); } else { throw new Error( `Failed to add feature, invalid feature ${JSON.stringify(feature)}`, @@ -176,8 +176,8 @@ export class BackendInitializer { const pluginInits = new Map(); const moduleInits = new Map>(); - // Enumerate all features - for (const feature of this.#features) { + // Enumerate all registrations + for (const feature of this.#registrations) { for (const r of feature.getRegistrations()) { const provides = new Set>(); @@ -205,7 +205,7 @@ export class BackendInitializer { consumes: new Set(Object.values(r.init.deps)), init: r.init, }); - } else { + } else if (r.type === 'module') { let modules = moduleInits.get(r.pluginId); if (!modules) { modules = new Map(); @@ -221,6 +221,8 @@ export class BackendInitializer { consumes: new Set(Object.values(r.init.deps)), init: r.init, }); + } else { + throw new Error(`Invalid registration type '${(r as any).type}'`); } } } @@ -385,14 +387,45 @@ export class BackendInitializer { } } -function isServiceFactory(feature: BackendFeature): feature is ServiceFactory { - return !!(feature as ServiceFactory).service; +function toInternalBackendFeature( + feature: BackendFeature, +): InternalBackendFeature { + if (feature.$$type !== '@backstage/BackendFeature') { + throw new Error(`Invalid BackendFeature, bad type '${feature.$$type}'`); + } + const internal = feature as InternalBackendFeature; + if (internal.version !== 'v1') { + throw new Error( + `Invalid BackendFeature, bad version '${internal.version}'`, + ); + } + return internal; } -function isInternalBackendFeature( +function isServiceFactory( feature: BackendFeature, -): feature is InternalBackendFeature { - return ( - typeof (feature as InternalBackendFeature).getRegistrations === 'function' - ); +): feature is InternalServiceFactory { + const internal = toInternalBackendFeature(feature); + if (internal.featureType === 'service') { + return true; + } + // Backwards compatibility for v1 registrations that use duck typing + if ('service' in internal) { + return true; + } + return false; +} + +function isBackendRegistrations( + feature: BackendFeature, +): feature is InternalBackendRegistrations { + const internal = toInternalBackendFeature(feature); + if (internal.featureType === 'registrations') { + return true; + } + // Backwards compatibility for v1 registrations that use duck typing + if ('getRegistrations' in internal) { + return true; + } + return false; } diff --git a/packages/backend-plugin-api/src/wiring/createBackendModule.test.ts b/packages/backend-plugin-api/src/wiring/createBackendModule.test.ts index 31496020bf..e3c9f9007a 100644 --- a/packages/backend-plugin-api/src/wiring/createBackendModule.test.ts +++ b/packages/backend-plugin-api/src/wiring/createBackendModule.test.ts @@ -15,7 +15,7 @@ */ import { createBackendModule } from './createBackendModule'; -import { InternalBackendFeature } from './types'; +import { InternalBackendRegistrations } from './types'; describe('createBackendModule', () => { it('should create a BackendModule', () => { @@ -28,13 +28,13 @@ describe('createBackendModule', () => { }); // legacy form - const legacy = result() as unknown as InternalBackendFeature; + const legacy = result() as unknown as InternalBackendRegistrations; expect(legacy.$$type).toEqual('@backstage/BackendFeature'); expect(legacy.version).toEqual('v1'); expect(legacy.getRegistrations).toEqual(expect.any(Function)); // new form - const module = result as unknown as InternalBackendFeature; + const module = result as unknown as InternalBackendRegistrations; expect(module.$$type).toEqual('@backstage/BackendFeature'); expect(module.version).toEqual('v1'); expect(module.getRegistrations).toEqual(expect.any(Function)); diff --git a/packages/backend-plugin-api/src/wiring/createBackendPlugin.test.ts b/packages/backend-plugin-api/src/wiring/createBackendPlugin.test.ts index cd1ce164bf..db4d60ee19 100644 --- a/packages/backend-plugin-api/src/wiring/createBackendPlugin.test.ts +++ b/packages/backend-plugin-api/src/wiring/createBackendPlugin.test.ts @@ -15,7 +15,7 @@ */ import { createBackendPlugin } from './createBackendPlugin'; -import { InternalBackendFeature } from './types'; +import { InternalBackendRegistrations } from './types'; describe('createBackendPlugin', () => { it('should create a BackendPlugin', () => { @@ -27,13 +27,13 @@ describe('createBackendPlugin', () => { }); // legacy form - const legacy = result() as unknown as InternalBackendFeature; + const legacy = result() as unknown as InternalBackendRegistrations; expect(legacy.$$type).toEqual('@backstage/BackendFeature'); expect(legacy.version).toEqual('v1'); expect(legacy.getRegistrations).toEqual(expect.any(Function)); // new form - const plugin = result as unknown as InternalBackendFeature; + const plugin = result as unknown as InternalBackendRegistrations; expect(plugin.$$type).toEqual('@backstage/BackendFeature'); expect(plugin.version).toEqual('v1'); expect(plugin.getRegistrations).toEqual(expect.any(Function)); diff --git a/packages/backend-plugin-api/src/wiring/types.ts b/packages/backend-plugin-api/src/wiring/types.ts index df0ca93089..01ab8c2e3e 100644 --- a/packages/backend-plugin-api/src/wiring/types.ts +++ b/packages/backend-plugin-api/src/wiring/types.ts @@ -14,7 +14,7 @@ * limitations under the License. */ -import { ServiceRef } from '../services/system/types'; +import { InternalServiceFactory, ServiceRef } from '../services/system/types'; import { BackendFeature } from '../types'; /** @@ -73,7 +73,7 @@ export interface BackendModuleRegistrationPoints { } /** @internal */ -export interface InternalBackendFeature extends BackendFeature { +export interface InternalBackendRegistrations extends BackendFeature { version: 'v1'; featureType: 'registrations'; getRegistrations(): Array< @@ -114,3 +114,9 @@ export interface InternalBackendFeatureLoader extends BackendFeature { deps: Record>; loader(deps: Record): Promise; } + +/** @internal */ +export type InternalBackendFeature = + | InternalBackendRegistrations + | InternalBackendFeatureLoader + | InternalServiceFactory; diff --git a/packages/backend-test-utils/src/next/wiring/TestBackend.ts b/packages/backend-test-utils/src/next/wiring/TestBackend.ts index 3d662a8c12..cf98c25e4a 100644 --- a/packages/backend-test-utils/src/next/wiring/TestBackend.ts +++ b/packages/backend-test-utils/src/next/wiring/TestBackend.ts @@ -36,7 +36,10 @@ import { ConfigReader } from '@backstage/config'; import express from 'express'; // 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'; +import { + InternalBackendFeature, + InternalBackendRegistrations, +} from '@backstage/backend-plugin-api/src/wiring/types'; import { createHealthRouter } from '@backstage/backend-defaults/rootHttpRouter'; /** @public */ @@ -94,7 +97,7 @@ function createPluginsForOrphanModules(features: Array) { const modulePluginIds = new Set(); for (const feature of features) { - if (isInternalBackendFeature(feature)) { + if (isInternalBackendRegistrations(feature)) { const registrations = feature.getRegistrations(); for (const registration of registrations) { if (registration.type === 'plugin') { @@ -137,18 +140,7 @@ function createExtensionPointTestModules( } const registrations = features.flatMap(feature => { - if (feature.$$type !== '@backstage/BackendFeature') { - throw new Error( - `Failed to add feature, invalid type '${feature.$$type}'`, - ); - } - - if (isInternalBackendFeature(feature)) { - if (feature.version !== 'v1') { - throw new Error( - `Failed to add feature, invalid version '${feature.version}'`, - ); - } + if (isInternalBackendRegistrations(feature)) { return feature.getRegistrations(); } return []; @@ -361,10 +353,28 @@ function registerTestHooks() { registerTestHooks(); -function isInternalBackendFeature( +function toInternalBackendFeature( feature: BackendFeature, -): feature is InternalBackendFeature { - return ( - typeof (feature as InternalBackendFeature).getRegistrations === 'function' - ); +): InternalBackendFeature { + if (feature.$$type !== '@backstage/BackendFeature') { + throw new Error(`Invalid BackendFeature, bad type '${feature.$$type}'`); + } + const internal = feature as InternalBackendFeature; + if (internal.version !== 'v1') { + throw new Error( + `Invalid BackendFeature, bad version '${internal.version}'`, + ); + } + return internal; +} + +function isInternalBackendRegistrations( + feature: BackendFeature, +): feature is InternalBackendRegistrations { + const internal = toInternalBackendFeature(feature); + if (internal.featureType === 'registrations') { + return true; + } + // Backwards compatibility for v1 registrations that use duck typing + return 'getRegistrations' in internal; }