diff --git a/.changeset/heavy-tomatoes-laugh.md b/.changeset/heavy-tomatoes-laugh.md new file mode 100644 index 0000000000..9a4740e209 --- /dev/null +++ b/.changeset/heavy-tomatoes-laugh.md @@ -0,0 +1,5 @@ +--- +'@backstage/backend-app-api': minor +--- + +Service factories added by feature loaders now have lower priority and will be ignored if a factory for the same service is added directly by `backend.add(serviceFactory)`. diff --git a/docs/backend-system/architecture/07-feature-loaders.md b/docs/backend-system/architecture/07-feature-loaders.md index 43954ec443..3edabfabbd 100644 --- a/docs/backend-system/architecture/07-feature-loaders.md +++ b/docs/backend-system/architecture/07-feature-loaders.md @@ -73,6 +73,29 @@ export default createBackendFeatureLoader({ }); ``` +### Overriding service factories + +Service factories registered by feature loaders have lower priority by ones added directly via `backend.add`. This allows you to use a feature loader for a larger number of service implementations, but still override individual services. + +The ordering in which different feature loaders or service factories are added does not matter. There is also no priority between feature loaders, if two different feature loaders add a factory for the same service, the backend will fail to start. + +```ts +const backend = createBackend(); + +backend.add( + createBackendFeatureLoader({ + async *loader() { + yield import('./commonDiscoveryService'); // discovery service + yield import('./commonRootLoggerService'); // root logger service + }, + }), +); + +backend.add(import('./myDiscoveryService')); // discovery service +``` + +The result of the above example is that the backend starts up with `./myDiscoveryService` as the discovery service implementation, while `./commonDiscoveryService` is ignored. The `./commonRootLoggerService` will still be used. + ### Dynamic logic A feature loader can also be asynchronous, and for example fetch data from an external source to determine which features to load: diff --git a/packages/backend-app-api/src/wiring/BackendInitializer.test.ts b/packages/backend-app-api/src/wiring/BackendInitializer.test.ts index f6d65e883e..5e98a71d46 100644 --- a/packages/backend-app-api/src/wiring/BackendInitializer.test.ts +++ b/packages/backend-app-api/src/wiring/BackendInitializer.test.ts @@ -25,6 +25,7 @@ import { createBackendModule, createExtensionPoint, createBackendFeatureLoader, + ServiceRef, } from '@backstage/backend-plugin-api'; import { BackendInitializer } from './BackendInitializer'; import { instanceMetadataServiceRef } from '@backstage/backend-plugin-api/alpha'; @@ -50,6 +51,18 @@ const baseFactories = [ loggerServiceFactory, ]; +function mkNoopFactory(ref: ServiceRef<{}, 'plugin'>) { + const fn = jest.fn().mockReturnValue({}); + return Object.assign( + fn, + createServiceFactory({ + service: ref, + deps: {}, + factory: fn, + }), + ); +} + const testPlugin = createBackendPlugin({ pluginId: 'test', register(reg) { @@ -167,6 +180,109 @@ describe('BackendInitializer', () => { expect(moduleInit).toHaveBeenCalled(); }); + it('should ignore services provided by feature loaders that have already been explicitly added', async () => { + const ref = createServiceRef<{}>({ id: '1' }); + const factory1 = mkNoopFactory(ref); + const factory2 = mkNoopFactory(ref); + const factory3 = mkNoopFactory(ref); + + const init = new BackendInitializer([...baseFactories, factory1]); + init.add(factory2); + init.add( + createBackendFeatureLoader({ + deps: {}, + *loader() { + yield factory3; + }, + }), + ); + init.add( + createBackendPlugin({ + pluginId: 'tester', + register(reg) { + reg.registerInit({ + deps: { ref }, + async init() {}, + }); + }, + }), + ); + + await init.start(); + + expect(factory1).not.toHaveBeenCalled(); + expect(factory2).toHaveBeenCalled(); + expect(factory3).not.toHaveBeenCalled(); + }); + + // Note: this is an important escape hatch in case to loaders conflict and you need to select the winning service factory + it('should allow duplicate service from feature loaders if overridden', async () => { + const ref = createServiceRef<{}>({ id: '1' }); + const factory1 = mkNoopFactory(ref); + const factory2 = mkNoopFactory(ref); + const factory3 = mkNoopFactory(ref); + const factory4 = mkNoopFactory(ref); + + const init = new BackendInitializer([...baseFactories, factory1]); + init.add(factory2); + init.add( + createBackendFeatureLoader({ + deps: {}, + *loader() { + yield factory3; + yield factory4; + }, + }), + ); + init.add( + createBackendPlugin({ + pluginId: 'tester', + register(reg) { + reg.registerInit({ + deps: { ref }, + async init() {}, + }); + }, + }), + ); + + await init.start(); + + expect(factory1).not.toHaveBeenCalled(); + expect(factory2).toHaveBeenCalled(); + expect(factory3).not.toHaveBeenCalled(); + expect(factory4).not.toHaveBeenCalled(); + }); + + it('should reject duplicate service factories from feature loader without an explicit override', async () => { + const ref = createServiceRef<{}>({ id: '1' }); + const factory1 = mkNoopFactory(ref); + const factory2 = mkNoopFactory(ref); + const factory3 = mkNoopFactory(ref); + + const init = new BackendInitializer([...baseFactories, factory1]); + init.add( + createBackendFeatureLoader({ + deps: {}, + *loader() { + yield factory2; + }, + }), + ); + init.add( + createBackendFeatureLoader({ + deps: {}, + *loader() { + yield factory3; + }, + }), + ); + + await expect(init.start()).rejects.toThrow( + 'Duplicate service implementations provided for 1 by both feature loader created at', + ); + }); + it('should refuse to override already initialized services through loaded features', async () => { const ref1 = createServiceRef<{ x: number }>({ id: '1', diff --git a/packages/backend-app-api/src/wiring/BackendInitializer.ts b/packages/backend-app-api/src/wiring/BackendInitializer.ts index 3e7b46fc9e..89e07ac405 100644 --- a/packages/backend-app-api/src/wiring/BackendInitializer.ts +++ b/packages/backend-app-api/src/wiring/BackendInitializer.ts @@ -519,6 +519,11 @@ export class BackendInitializer { } async #applyBackendFeatureLoaders(loaders: InternalBackendFeatureLoader[]) { + const servicesAddedByLoaders = new Map< + string, + InternalBackendFeatureLoader + >(); + for (const loader of loaders) { const deps = new Map(); const missingRefs = new Set(); @@ -564,8 +569,32 @@ export class BackendInitializer { if (isBackendFeatureLoader(feature)) { newLoaders.push(feature); } else { - didAddServiceFactory ||= isServiceFactory(feature); - this.#addFeature(feature); + // This block makes sure that feature loaders do not provide duplicate + // implementations for the same service, but at the same time allows + // service factories provided by feature loaders to be overridden by + // ones that are explicitly installed with backend.add(serviceFactory). + // + // If a factory has already been explicitly installed, the service + // factory provided by the loader will simply be ignored. + if (isServiceFactory(feature)) { + const conflictingLoader = servicesAddedByLoaders.get( + feature.service.id, + ); + if (conflictingLoader) { + throw new Error( + `Duplicate service implementations provided for ${feature.service.id} by both feature loader ${loader.description} and feature loader ${conflictingLoader.description}`, + ); + } + + // Check that this service wasn't already explicitly added by backend.add(serviceFactory) + if (!this.#serviceRegistry.hasBeenAdded(feature.service)) { + didAddServiceFactory = true; + servicesAddedByLoaders.set(feature.service.id, loader); + this.#addFeature(feature); + } + } else { + this.#addFeature(feature); + } } } diff --git a/packages/backend-app-api/src/wiring/ServiceRegistry.ts b/packages/backend-app-api/src/wiring/ServiceRegistry.ts index 0abd5605ae..162c175615 100644 --- a/packages/backend-app-api/src/wiring/ServiceRegistry.ts +++ b/packages/backend-app-api/src/wiring/ServiceRegistry.ts @@ -191,6 +191,16 @@ export class ServiceRegistry { } } + hasBeenAdded(ref: ServiceRef) { + if (ref.id === coreServices.pluginMetadata.id) { + return true; + } + if (ref.multiton) { + return false; + } + return this.#addedFactoryIds.has(ref.id); + } + add(factory: ServiceFactory) { const factoryId = factory.service.id; if (factoryId === coreServices.pluginMetadata.id) {