From 6ed4759db18fd87a2249326dd35e47596105c624 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Fri, 11 Aug 2023 14:20:59 +0200 Subject: [PATCH] backend-app-api: refactor dependency tree to use values instead of IDs Signed-off-by: Patrik Oldsberg --- .../src/lib/DependencyTree.test.ts | 14 +- .../backend-app-api/src/lib/DependencyTree.ts | 122 +++++++++--------- .../src/wiring/BackendInitializer.ts | 39 +++--- 3 files changed, 87 insertions(+), 88 deletions(-) diff --git a/packages/backend-app-api/src/lib/DependencyTree.test.ts b/packages/backend-app-api/src/lib/DependencyTree.test.ts index 88593754bc..c76c23c36a 100644 --- a/packages/backend-app-api/src/lib/DependencyTree.test.ts +++ b/packages/backend-app-api/src/lib/DependencyTree.test.ts @@ -17,11 +17,13 @@ import { DependencyTree } from './DependencyTree'; describe('DependencyTree', () => { - it('should be empty', () => { + it('should be empty', async () => { const empty = DependencyTree.fromMap({}); - expect(Array.from(empty.nodes)).toEqual([]); expect(empty.findUnsatisfiedDeps()).toEqual([]); expect(empty.detectCircularDependency()).toBeUndefined(); + await expect( + empty.parallelTopologicalTraversal(async id => id), + ).resolves.toEqual([]); }); it('should detect circular dependencies', () => { @@ -89,14 +91,14 @@ describe('DependencyTree', () => { DependencyTree.fromMap({ 1: { consumes: ['a'] }, }).findUnsatisfiedDeps(), - ).toEqual([{ id: '1', unsatisfied: ['a'] }]); + ).toEqual([{ value: '1', unsatisfied: ['a'] }]); expect( DependencyTree.fromMap({ 1: { produces: ['a'], consumes: ['b'] }, 2: { produces: ['b'], consumes: ['a', 'd', 'e'] }, }).findUnsatisfiedDeps(), - ).toEqual([{ id: '2', unsatisfied: ['d', 'e'] }]); + ).toEqual([{ value: '2', unsatisfied: ['d', 'e'] }]); expect( DependencyTree.fromMap({ @@ -106,8 +108,8 @@ describe('DependencyTree', () => { 4: { produces: [], consumes: ['c', 'a'] }, }).findUnsatisfiedDeps(), ).toEqual([ - { id: '2', unsatisfied: ['d', 'e'] }, - { id: '4', unsatisfied: ['c'] }, + { value: '2', unsatisfied: ['d', 'e'] }, + { value: '4', unsatisfied: ['c'] }, ]); }); diff --git a/packages/backend-app-api/src/lib/DependencyTree.ts b/packages/backend-app-api/src/lib/DependencyTree.ts index a11ce3cbaf..addaa873a5 100644 --- a/packages/backend-app-api/src/lib/DependencyTree.ts +++ b/packages/backend-app-api/src/lib/DependencyTree.ts @@ -14,111 +14,109 @@ * limitations under the License. */ -import { ForwardedError, InputError } from '@backstage/errors'; - -interface NodeInput { - id: string; +interface NodeInput { + value: T; consumes?: Iterable; produces?: Iterable; } /** @internal */ -class Node { - static from(input: NodeInput) { - return new Node( - input.id, +class Node { + static from(input: NodeInput) { + return new Node( + input.value, input.consumes ? new Set(input.consumes) : new Set(), input.produces ? new Set(input.produces) : new Set(), ); } private constructor( - readonly id: string, + readonly value: T, readonly consumes: Set, readonly produces: Set, ) {} } /** @internal */ -export class DependencyTree { - static fromMap(nodes: Record>) { +export class DependencyTree { + static fromMap( + nodes: Record, 'value'>>, + ): DependencyTree { return this.fromIterable( - Object.entries(nodes).map(([id, node]) => ({ id, ...node })), + Object.entries(nodes).map(([key, node]) => ({ + value: String(key), + ...node, + })), ); } - static fromIterable(nodeInputs: Iterable) { - const nodes = new Map(); + static fromIterable( + nodeInputs: Iterable>, + ): DependencyTree { + const nodes = new Array>(); for (const nodeInput of nodeInputs) { - const node = Node.from(nodeInput); - if (nodes.has(node.id)) { - throw new InputError(`Duplicate node with id ${node.id}`); - } - nodes.set(node.id, node); + nodes.push(Node.from(nodeInput)); } return new DependencyTree(nodes); } + #nodes: Array>; #allProduced: Set; #allConsumed: Set; - #consumedBy: Map>; - private constructor(readonly nodes: Map) { + private constructor(nodes: Array>) { + this.#nodes = nodes; this.#allProduced = new Set(); this.#allConsumed = new Set(); - this.#consumedBy = new Map(); - for (const node of this.nodes.values()) { + for (const node of this.#nodes.values()) { for (const produced of node.produces) { this.#allProduced.add(produced); } for (const consumed of node.consumes) { this.#allConsumed.add(consumed); - if (!this.#consumedBy.get(consumed)?.add(node.id)) { - this.#consumedBy.set(consumed, new Set([node.id])); - } } } } - findUnsatisfiedDeps(): Array<{ id: string; unsatisfied: string[] }> { + findUnsatisfiedDeps(): Array<{ value: T; unsatisfied: string[] }> { const unsatisfiedDependencies = []; - for (const node of this.nodes.values()) { + for (const node of this.#nodes.values()) { const unsatisfied = Array.from(node.consumes).filter( id => !this.#allProduced.has(id), ); if (unsatisfied.length > 0) { - unsatisfiedDependencies.push({ id: node.id, unsatisfied }); + unsatisfiedDependencies.push({ value: node.value, unsatisfied }); } } return unsatisfiedDependencies; } - detectCircularDependency(): string[] | undefined { - for (const nodeId of this.nodes.keys()) { - const visited = new Set(); - const stack = new Array<[id: string, path: string[]]>([nodeId, [nodeId]]); + detectCircularDependency(): T[] | undefined { + for (const startNode of this.#nodes) { + const visited = new Set>(); + const stack = new Array<[node: Node, path: T[]]>([ + startNode, + [startNode.value], + ]); while (stack.length > 0) { - const [id, path] = stack.pop()!; - if (visited.has(id)) { + const [node, path] = stack.pop()!; + if (visited.has(node)) { continue; } - visited.add(id); - const node = this.nodes.get(id); - if (node) { - for (const produced of node.produces) { - const consumers = this.#consumedBy.get(produced); - if (consumers) { - for (const consumer of consumers) { - if (consumer === nodeId) { - return [...path, nodeId]; - } - if (!visited.has(consumer)) { - stack.push([consumer, [...path, consumer]]); - } - } + visited.add(node); + for (const produced of node.produces) { + const consumerNodes = this.#nodes.filter(other => + other.consumes.has(produced), + ); + for (const consumer of consumerNodes) { + if (consumer === startNode) { + return [...path, startNode.value]; + } + if (!visited.has(consumer)) { + stack.push([consumer, [...path, consumer.value]]); } } } @@ -127,14 +125,14 @@ export class DependencyTree { return undefined; } - async parallelTopologicalTraversal( - fn: (nodeId: string) => Promise, - ): Promise { + async parallelTopologicalTraversal( + fn: (value: T) => Promise, + ): Promise { const allProduced = this.#allProduced; const producedSoFar = new Set(); - const waiting = new Set(this.nodes.values()); - const visited = new Set(); - const results = new Array(); + const waiting = new Set(this.#nodes.values()); + const visited = new Set>(); + const results = new Array(); let inFlight = 0; async function processMoreNodes() { @@ -168,15 +166,13 @@ export class DependencyTree { await Promise.all(nodesToProcess.map(processNode)); } - async function processNode(node: Node) { - visited.add(node.id); + async function processNode(node: Node) { + visited.add(node); inFlight += 1; - try { - const result = await fn(node.id); - results.push(result); - } catch (error) { - throw new ForwardedError(`Failed at ${node.id}`, error); - } + + const result = await fn(node.value); + results.push(result); + node.produces.forEach(produced => producedSoFar.add(produced)); inFlight -= 1; await processMoreNodes(); diff --git a/packages/backend-app-api/src/wiring/BackendInitializer.ts b/packages/backend-app-api/src/wiring/BackendInitializer.ts index fe3120713e..1061ad8944 100644 --- a/packages/backend-app-api/src/wiring/BackendInitializer.ts +++ b/packages/backend-app-api/src/wiring/BackendInitializer.ts @@ -214,36 +214,37 @@ export class BackendInitializer { const modules = moduleInits.get(pluginId); if (modules) { const tree = DependencyTree.fromIterable( - Array.from(modules).map(([id, { provides, consumes }]) => ({ - id, + Array.from(modules).map(([moduleId, moduleInit]) => ({ + value: { moduleId, moduleInit }, // Relationships are reversed at this point since we're only interested in the extension points. // If a modules provides extension point A we want it to be initialized AFTER all modules // that depend on extension point A, so that they can provide their extensions. - consumes: Array.from(provides).map(p => p.id), - produces: Array.from(consumes).map(c => c.id), + consumes: Array.from(moduleInit.provides).map(p => p.id), + produces: Array.from(moduleInit.consumes).map(c => c.id), })), ); const circular = tree.detectCircularDependency(); if (circular) { throw new ConflictError( - `Circular dependency detected for modules of plugin '${pluginId}', '${circular.join( - "' -> '", - )}'`, + `Circular dependency detected for modules of plugin '${pluginId}', ${circular + .map(({ moduleId }) => `'${moduleId}'`) + .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, + await tree.parallelTopologicalTraversal( + 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, + ); + }); + }, + ); } // Once all modules have been initialized, we can initialize the plugin itself