From 0474a4bb1609a2eff805184db1bc959597eb1004 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Fri, 11 Aug 2023 13:19:37 +0200 Subject: [PATCH] backend-app-api: use dependency tree to implement module initialization Signed-off-by: Patrik Oldsberg --- .../src/wiring/BackendInitializer.test.ts | 48 +-------- .../src/wiring/BackendInitializer.ts | 99 ++++++------------- 2 files changed, 32 insertions(+), 115 deletions(-) diff --git a/packages/backend-app-api/src/wiring/BackendInitializer.test.ts b/packages/backend-app-api/src/wiring/BackendInitializer.test.ts index 555ce63f15..b05194f594 100644 --- a/packages/backend-app-api/src/wiring/BackendInitializer.test.ts +++ b/packages/backend-app-api/src/wiring/BackendInitializer.test.ts @@ -21,7 +21,7 @@ import { createBackendPlugin, createBackendModule, } from '@backstage/backend-plugin-api'; -import { BackendInitializer, resolveInitChunks } from './BackendInitializer'; +import { BackendInitializer } from './BackendInitializer'; import { ServiceRegistry } from './ServiceRegistry'; import { rootLifecycleServiceFactory } from '../services/implementations'; @@ -176,49 +176,3 @@ describe('BackendInitializer', () => { ); }); }); - -describe('resolveInitChunks', () => { - it('should resolve flat chunks', () => { - const resolved = resolveInitChunks( - new Map( - Object.entries({ - a: { consumes: new Set(), provides: new Set(['a']) } as any, - b: { consumes: new Set(), provides: new Set(['b']) } as any, - c: { consumes: new Set(), provides: new Set(['c']) } as any, - d: { consumes: new Set(), provides: new Set(['d']) } as any, - }), - ), - ).map(chunk => Array.from(chunk.keys())); - expect(resolved).toEqual([['a', 'b', 'c', 'd']]); - }); - - it('should resolve nested chunks', () => { - const resolved = resolveInitChunks( - new Map( - Object.entries({ - a: { consumes: new Set(), provides: new Set(['a']) } as any, - b: { consumes: new Set(['a']), provides: new Set(['b', 'c']) } as any, - c: { consumes: new Set(['b']), provides: new Set() } as any, - d: { consumes: new Set(['c']), provides: new Set() } as any, - }), - ), - ).map(chunk => Array.from(chunk.keys())); - expect(resolved).toEqual([['a'], ['b'], ['c', 'd']]); - }); - - it('should detect circular dependencies', () => { - expect(() => - resolveInitChunks( - new Map( - Object.entries({ - a: { consumes: new Set(), provides: new Set(['a']) } as any, - b: { consumes: new Set(['a', 'b']), provides: new Set() } as any, - c: { consumes: new Set(['a', 'c']), provides: new Set() } as any, - }), - ), - ), - ).toThrow( - "Failed to resolve module initialization order, the following modules have circular dependencies, 'b', 'c'", - ); - }); -}); diff --git a/packages/backend-app-api/src/wiring/BackendInitializer.ts b/packages/backend-app-api/src/wiring/BackendInitializer.ts index 19b933d96a..e5aebb52fa 100644 --- a/packages/backend-app-api/src/wiring/BackendInitializer.ts +++ b/packages/backend-app-api/src/wiring/BackendInitializer.ts @@ -26,8 +26,9 @@ 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'; -import { ForwardedError } from '@backstage/errors'; +import { ForwardedError, InputError } from '@backstage/errors'; import { featureDiscoveryServiceRef } from '@backstage/backend-plugin-api/alpha'; +import { DependencyTree } from '../lib/DependencyTree'; export interface BackendRegisterInit { consumes: Set; @@ -38,58 +39,6 @@ export interface BackendRegisterInit { }; } -/** @internal */ -export function resolveInitChunks( - registerInits: Map, -): Map[] { - let registerInitsToOrder = Array.from(registerInits); - const orderedRegisterInits = new Array>(); - - // Keep track of all deps that have been provided so far - const provided = new Set(); - - // Loop until we have ordered all registerInits - while (registerInitsToOrder.length > 0) { - const chunk = new Map(); - - // Find all registerInits that have all their deps provided - for (const [id, registerInit] of registerInitsToOrder) { - if ( - Array.from(registerInit.consumes).every(consumed => - provided.has(consumed), - ) - ) { - chunk.set(id, registerInit); - } - } - - // Add to the result set - orderedRegisterInits.push(chunk); - - // Register all the deps in the current chunk as provided - chunk.forEach(r => r.provides.forEach(p => provided.add(p))); - - // Remove the current chunk from the registerInits to order - const newRegisterInitsToOrder = registerInitsToOrder.filter( - ([id]) => !chunk.has(id), - ); - // Check that we have not hit a circular dependency - if ( - registerInitsToOrder.length !== 0 && - registerInitsToOrder.length === newRegisterInitsToOrder.length - ) { - throw new Error( - `Failed to resolve module initialization order, the following modules have circular dependencies, '${registerInitsToOrder - .map(r => r[0]) - .join("', '")}'`, - ); - } - registerInitsToOrder = newRegisterInitsToOrder; - } - - return orderedRegisterInits; -} - export class BackendInitializer { #startPromise?: Promise; #features = new Array(); @@ -262,22 +211,36 @@ export class BackendInitializer { await Promise.all( allPluginIds.map(async pluginId => { // Modules are initialized before plugins, so that they can provide extension to the plugin - const modules = moduleInits.get(pluginId) ?? new Map(); - for (const chunk of resolveInitChunks(modules)) { - await Promise.all( - Array.from(chunk).map(async ([moduleId, moduleInit]) => { - const moduleDeps = await this.#getInitDeps( - moduleInit.init.deps, - pluginId, - ); - await moduleInit.init.func(moduleDeps).catch(error => { - throw new ForwardedError( - `Module '${moduleId}' for plugin '${pluginId}' startup failed`, - error, - ); - }); - }), + const modules = moduleInits.get(pluginId); + if (modules) { + const tree = DependencyTree.fromIterable( + Array.from(modules).map(([id, { provides, consumes }]) => ({ + id, + provides: Array.from(provides).map(p => p.id), + consumes: Array.from(consumes).map(c => c.id), + })), ); + const circular = tree.detectCircularDependency(); + if (circular) { + throw new InputError( + `Circular dependency detected for modules of plugin '${pluginId}', '${circular.join( + "' -> '", + )}'`, + ); + } + await tree.parallelTopologicalTraversal(async moduleId => { + const moduleInit = modules.get(moduleId)!; + const moduleDeps = await this.#getInitDeps( + moduleInit.init.deps, + pluginId, + ); + await moduleInit.init.func(moduleDeps).catch(error => { + throw new ForwardedError( + `Module '${moduleId}' for plugin '${pluginId}' startup failed`, + error, + ); + }); + }); } // Once all modules have been initialized, we can initialize the plugin itself