backend-app-api: enable service overrides over feature loaders

Signed-off-by: Patrik Oldsberg <poldsberg@gmail.com>
2024-12-07 20:12:23 +01:00
parent c0f0e263b0
commit ebf083d902
5 changed files with 185 additions and 2 deletions
@@ -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',
@@ -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<string, unknown>();
      const missingRefs = new Set<ServiceOrExtensionPoint>();
@@ -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);
+          }
        }
      }

@@ -191,6 +191,16 @@ export class ServiceRegistry {
    }
  }

+  hasBeenAdded(ref: ServiceRef<any>) {
+    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) {