backend-app-api: fix ServiceRegistry default factory loader race

Co-authored-by: Fredrik Adelöw <freben@gmail.com>
Co-authored-by: blam <ben@blam.sh>
Co-authored-by: Johan Haals <johan.haals@gmail.com>
Signed-off-by: Patrik Oldsberg <poldsberg@gmail.com>
Signed-off-by: Johan Haals <johan.haals@gmail.com>

packages/backend-app-api/src/wiring/ServiceRegistry.test.ts

@@ -17,6 +17,7 @@
 import {
   createServiceRef,
   createServiceFactory,
+  ServiceRef,
 } from '@backstage/backend-plugin-api';
 import { ServiceRegistry } from './ServiceRegistry';
 
@@ -171,6 +172,28 @@ describe('ServiceRegistry', () => {
     expect(await factoryA('catalog')).not.toBe(await factoryB('catalog'));
   });
 
+  it('should only call each default factory loader once', async () => {
+    const factoryLoader = jest.fn(async (service: ServiceRef<void>) =>
+      createServiceFactory({
+        service,
+        deps: {},
+        factory: async () => async () => {},
+      }),
+    );
+    const ref = createServiceRef<void>({
+      id: '1',
+      defaultFactory: factoryLoader,
+    });
+
+    const registry = new ServiceRegistry([]);
+    const factory = registry.get(ref)!;
+    await Promise.all([
+      expect(factory('catalog')).resolves.toBeUndefined(),
+      expect(factory('catalog')).resolves.toBeUndefined(),
+    ]);
+    expect(factoryLoader).toHaveBeenCalledTimes(1);
+  });
+
   it('should not call factory functions more than once', async () => {
     const innerFactory = jest.fn(async (pluginId: string) => {
       return { x: 1, pluginId };

packages/backend-app-api/src/wiring/ServiceRegistry.ts

@@ -21,7 +21,7 @@ import {
 
 export class ServiceRegistry {
   readonly #providedFactories: Map<string, ServiceFactory>;
-  readonly #loadedDefaultFactories: Map<Function, ServiceFactory>;
+  readonly #loadedDefaultFactories: Map<Function, Promise<ServiceFactory>>;
   readonly #implementations: Map<
     ServiceFactory,
     {
@@ -47,10 +47,11 @@ export class ServiceRegistry {
       if (!factory) {
         let loadedFactory = this.#loadedDefaultFactories.get(defaultFactory!);
         if (!loadedFactory) {
-          loadedFactory = (await defaultFactory!(ref)) as ServiceFactory;
+          loadedFactory = defaultFactory!(ref) as Promise<ServiceFactory>;
           this.#loadedDefaultFactories.set(defaultFactory!, loadedFactory);
         }
-        factory = loadedFactory;
+        // NOTE: This await is safe as long as #providedFactories is not mutated.
+        factory = await loadedFactory;
       }
 
       let implementation = this.#implementations.get(factory);