From 6c6f392c74bec7ad04d9d1ec3b95a170d1a08ed4 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Sun, 15 Oct 2023 13:03:23 +0200 Subject: [PATCH 01/31] frontend-app-api: extract extension config reading into separate module Signed-off-by: Patrik Oldsberg --- .../frontend-app-api/src/wiring/createApp.tsx | 4 +- .../graph/readAppExtensionsConfig.test.ts | 268 ++++++++++++++++++ .../wiring/graph/readAppExtensionsConfig.ts | 199 +++++++++++++ .../src/wiring/parameters.test.ts | 254 +---------------- .../frontend-app-api/src/wiring/parameters.ts | 184 +----------- 5 files changed, 471 insertions(+), 438 deletions(-) create mode 100644 packages/frontend-app-api/src/wiring/graph/readAppExtensionsConfig.test.ts create mode 100644 packages/frontend-app-api/src/wiring/graph/readAppExtensionsConfig.ts diff --git a/packages/frontend-app-api/src/wiring/createApp.tsx b/packages/frontend-app-api/src/wiring/createApp.tsx index e770a6b4c1..dc132b6654 100644 --- a/packages/frontend-app-api/src/wiring/createApp.tsx +++ b/packages/frontend-app-api/src/wiring/createApp.tsx @@ -35,7 +35,6 @@ import { import { ExtensionInstanceParameters, mergeExtensionParameters, - readAppExtensionParameters, } from './parameters'; import { AnyApiFactory, @@ -96,6 +95,7 @@ import { AppRouteBinder } from '../routing'; import { RoutingProvider } from '../routing/RoutingProvider'; import { resolveRouteBindings } from '../routing/resolveRouteBindings'; import { collectRouteIds } from '../routing/collectRouteIds'; +import { readAppExtensionsConfig } from './graph/readAppExtensionsConfig'; /** @public */ export interface ExtensionTreeNode { @@ -200,7 +200,7 @@ export function createInstances(options: { const extensionParams = mergeExtensionParameters({ features: options.features, builtinExtensions, - parameters: readAppExtensionParameters(options.config), + parameters: readAppExtensionsConfig(options.config), }); // TODO: validate the config of all extension instances diff --git a/packages/frontend-app-api/src/wiring/graph/readAppExtensionsConfig.test.ts b/packages/frontend-app-api/src/wiring/graph/readAppExtensionsConfig.test.ts new file mode 100644 index 0000000000..91f6992f29 --- /dev/null +++ b/packages/frontend-app-api/src/wiring/graph/readAppExtensionsConfig.test.ts @@ -0,0 +1,268 @@ +/* + * Copyright 2023 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { ConfigReader } from '@backstage/config'; +import { JsonValue } from '@backstage/types'; +import { + expandShorthandExtensionParameters, + readAppExtensionsConfig, +} from './readAppExtensionsConfig'; + +describe('readAppExtensionsConfig', () => { + it('should disable extension with shorthand notation', () => { + expect( + readAppExtensionsConfig( + new ConfigReader({ app: { extensions: [{ 'core.router': false }] } }), + ), + ).toEqual([ + { + id: 'core.router', + disabled: true, + }, + ]); + expect( + readAppExtensionsConfig( + new ConfigReader({ + app: { extensions: [{ 'core.router': { disabled: true } }] }, + }), + ), + ).toEqual([ + { + at: undefined, + config: undefined, + disabled: true, + id: 'core.router', + }, + ]); + }); + + it('should enable extension with shorthand notation', () => { + expect( + readAppExtensionsConfig( + new ConfigReader({ app: { extensions: ['core.router'] } }), + ), + ).toEqual([ + { + id: 'core.router', + disabled: false, + }, + ]); + expect( + readAppExtensionsConfig( + new ConfigReader({ app: { extensions: [{ 'core.router': true }] } }), + ), + ).toEqual([ + { + id: 'core.router', + disabled: false, + }, + ]); + expect( + readAppExtensionsConfig( + new ConfigReader({ + app: { extensions: [{ 'core.router': { disabled: false } }] }, + }), + ), + ).toEqual([ + { + id: 'core.router', + disabled: false, + }, + ]); + }); + + it('should not allow string keys', () => { + expect(() => + readAppExtensionsConfig( + new ConfigReader({ + app: { + extensions: [{ 'core.router': 'some-string' }], + }, + }), + ), + ).toThrow( + 'Invalid extension configuration at app.extensions[0][core.router], value must be a boolean or object', + ); + }); + + it('should not allow invalid keys', () => { + expect(() => + readAppExtensionsConfig( + new ConfigReader({ + app: { + extensions: [ + { + 'core.router/routes': { + extension: 'example-package#MyPage', + config: { foo: 'bar' }, + }, + }, + ], + }, + }), + ), + ).toThrowErrorMatchingInlineSnapshot( + `"Invalid extension configuration at app.extensions[0], extension ID must not contain slashes; got 'core.router/routes', did you mean 'core.router'?"`, + ); + }); +}); + +describe('expandShorthandExtensionParameters', () => { + const run = (value: JsonValue) => { + return expandShorthandExtensionParameters(value, 1); + }; + + it('rejects unknown keys', () => { + expect(() => run(null)).toThrowErrorMatchingInlineSnapshot( + `"Invalid extension configuration at app.extensions[1], must be a string or an object"`, + ); + expect(() => run(1)).toThrowErrorMatchingInlineSnapshot( + `"Invalid extension configuration at app.extensions[1], must be a string or an object"`, + ); + expect(() => run([])).toThrowErrorMatchingInlineSnapshot( + `"Invalid extension configuration at app.extensions[1], must be a string or an object"`, + ); + }); + + it('rejects the wrong number of keys', () => { + expect(() => run({})).toThrowErrorMatchingInlineSnapshot( + `"Invalid extension configuration at app.extensions[1], must have exactly one key, got none"`, + ); + expect(() => run({ a: {}, b: {} })).toThrowErrorMatchingInlineSnapshot( + `"Invalid extension configuration at app.extensions[1], must have exactly one key, got 'a', 'b'"`, + ); + }); + + it('rejects unknown values', () => { + expect(() => run({ a: 1 })).toThrowErrorMatchingInlineSnapshot( + `"Invalid extension configuration at app.extensions[1][a], value must be a boolean or object"`, + ); + expect(() => run({ a: [] })).toThrowErrorMatchingInlineSnapshot( + `"Invalid extension configuration at app.extensions[1][a], value must be a boolean or object"`, + ); + }); + + it('supports string key', () => { + expect(run('core.router')).toEqual({ + id: 'core.router', + disabled: false, + }); + expect(() => run('')).toThrowErrorMatchingInlineSnapshot( + `"Invalid extension configuration at app.extensions[1], extension ID must not be empty or contain whitespace"`, + ); + expect(() => run(' a')).toThrowErrorMatchingInlineSnapshot( + `"Invalid extension configuration at app.extensions[1], extension ID must not be empty or contain whitespace"`, + ); + expect(() => run('core.router/routes')).toThrowErrorMatchingInlineSnapshot( + `"Invalid extension configuration at app.extensions[1], extension ID must not contain slashes; got 'core.router/routes', did you mean 'core.router'?"`, + ); + }); + + it('supports null value', () => { + // this is the result of typing: + // - core.router: + // The missing value is interpreted as null by the yaml parser so we deal with that + expect(run({ 'core.router': null })).toEqual({ + id: 'core.router', + disabled: false, + }); + }); + + it('supports boolean value', () => { + expect(run({ 'core.router': true })).toEqual({ + id: 'core.router', + disabled: false, + }); + expect(run({ 'core.router': false })).toEqual({ + id: 'core.router', + disabled: true, + }); + }); + + it('should not support string values', () => { + expect(() => + run({ 'core.router': 'example-package#MyRouter' }), + ).toThrowErrorMatchingInlineSnapshot( + `"Invalid extension configuration at app.extensions[1][core.router], value must be a boolean or object"`, + ); + }); + + it('supports object id only in the key', () => { + expect(() => + run({ 'core.router': { id: 'some.id' } }), + ).toThrowErrorMatchingInlineSnapshot( + `"Invalid extension configuration at app.extensions[1][core.router].id, unknown parameter; expected one of 'attachTo', 'disabled', 'config'"`, + ); + }); + + it('supports object attachTo', () => { + expect( + run({ + 'core.router': { attachTo: { id: 'other.root', input: 'inputs' } }, + }), + ).toEqual({ + id: 'core.router', + attachTo: { id: 'other.root', input: 'inputs' }, + }); + expect(() => + run({ + 'core.router': { + id: 'other-id', + }, + }), + ).toThrowErrorMatchingInlineSnapshot( + `"Invalid extension configuration at app.extensions[1][core.router].id, unknown parameter; expected one of 'attachTo', 'disabled', 'config'"`, + ); + }); + + it('supports object disabled', () => { + expect(run({ 'core.router': { disabled: true } })).toEqual({ + id: 'core.router', + disabled: true, + }); + expect(run({ 'core.router': { disabled: false } })).toEqual({ + id: 'core.router', + disabled: false, + }); + expect(() => + run({ 'core.router': { disabled: 0 } }), + ).toThrowErrorMatchingInlineSnapshot( + `"Invalid extension configuration at app.extensions[1][core.router].disabled, must be a boolean"`, + ); + }); + + it('supports object config', () => { + expect( + run({ 'core.router': { config: { disableRedirects: true } } }), + ).toEqual({ + id: 'core.router', + config: { disableRedirects: true }, + }); + expect(() => + run({ 'core.router': { config: 0 } }), + ).toThrowErrorMatchingInlineSnapshot( + `"Invalid extension configuration at app.extensions[1][core.router].config, must be an object"`, + ); + }); + + it('rejects unknown object keys', () => { + expect(() => + run({ 'core.router': { foo: { settings: true } } }), + ).toThrowErrorMatchingInlineSnapshot( + `"Invalid extension configuration at app.extensions[1][core.router].foo, unknown parameter; expected one of 'attachTo', 'disabled', 'config'"`, + ); + }); +}); diff --git a/packages/frontend-app-api/src/wiring/graph/readAppExtensionsConfig.ts b/packages/frontend-app-api/src/wiring/graph/readAppExtensionsConfig.ts new file mode 100644 index 0000000000..2b5b51ce92 --- /dev/null +++ b/packages/frontend-app-api/src/wiring/graph/readAppExtensionsConfig.ts @@ -0,0 +1,199 @@ +/* + * Copyright 2023 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { Config } from '@backstage/config'; +import { JsonValue } from '@backstage/types'; + +export interface ExtensionParameters { + id: string; + attachTo?: { id: string; input: string }; + disabled?: boolean; + config?: unknown; +} + +const knownExtensionParameters = ['attachTo', 'disabled', 'config']; + +// Since we'll never merge arrays in config the config reader context +// isn't too much of a help. Fall back to manual config reading logic +// as the Config interface makes it quite hard for us otherwise. +/** @internal */ +export function readAppExtensionsConfig( + rootConfig: Config, +): ExtensionParameters[] { + const arr = rootConfig.getOptional('app.extensions'); + if (!Array.isArray(arr)) { + if (arr === undefined) { + return []; + } + // This will throw, and show which part of config had the wrong type + rootConfig.getConfigArray('app.extensions'); + return []; + } + + return arr.map((arrayEntry, arrayIndex) => + expandShorthandExtensionParameters(arrayEntry, arrayIndex), + ); +} + +/** @internal */ +export function expandShorthandExtensionParameters( + arrayEntry: JsonValue, + arrayIndex: number, +): ExtensionParameters { + function errorMsg(msg: string, key?: string, prop?: string) { + return `Invalid extension configuration at app.extensions[${arrayIndex}]${ + key ? `[${key}]` : '' + }${prop ? `.${prop}` : ''}, ${msg}`; + } + + // NOTE(freben): This check is intentionally not complete and doesn't check + // whether letters and digits are used, etc. It's not up to the config reading + // logic to decide what constitutes a valid extension ID; that should be + // decided by the logic that loads and instantiates the extensions. This check + // is just here to catch real mistakes or truly conceptually wrong input. + function assertValidId(id: string) { + if (!id || id !== id.trim()) { + throw new Error( + errorMsg('extension ID must not be empty or contain whitespace'), + ); + } + + if (id.includes('/')) { + let message = `extension ID must not contain slashes; got '${id}'`; + const good = id.split('/')[0]; + if (good) { + message += `, did you mean '${good}'?`; + } + throw new Error(errorMsg(message)); + } + } + + // Example YAML: + // - entity.card.about + if (typeof arrayEntry === 'string') { + assertValidId(arrayEntry); + return { + id: arrayEntry, + disabled: false, + }; + } + + // All remaining cases are single-key objects + if ( + typeof arrayEntry !== 'object' || + arrayEntry === null || + Array.isArray(arrayEntry) + ) { + throw new Error(errorMsg('must be a string or an object')); + } + const keys = Object.keys(arrayEntry); + if (keys.length !== 1) { + const joinedKeys = keys.length ? `'${keys.join("', '")}'` : 'none'; + throw new Error(errorMsg(`must have exactly one key, got ${joinedKeys}`)); + } + + const id = String(keys[0]); + const value = arrayEntry[id]; + assertValidId(id); + + // This example covers a potentially common mistake in the syntax + // Example YAML: + // - entity.card.about: + if (value === null) { + return { + id, + disabled: false, + }; + } + + // Example YAML: + // - catalog.page.cicd: false + if (typeof value === 'boolean') { + return { + id, + disabled: !value, + }; + } + + // The remaining case is the generic object. Example YAML: + // - tech-radar.page: + // at: core.router/routes + // disabled: false + // config: + // path: /tech-radar + // width: 1500 + // height: 800 + if (typeof value !== 'object' || Array.isArray(value)) { + // We don't mention null here - we don't want people to explicitly enter + // - entity.card.about: null + throw new Error(errorMsg('value must be a boolean or object', id)); + } + + const attachTo = value.attachTo as { id: string; input: string } | undefined; + const disabled = value.disabled; + const config = value.config; + + if (attachTo !== undefined) { + if ( + attachTo === null || + typeof attachTo !== 'object' || + Array.isArray(attachTo) + ) { + throw new Error(errorMsg('must be an object', id, 'attachTo')); + } + if (typeof attachTo.id !== 'string' || attachTo.id === '') { + throw new Error( + errorMsg('must be a non-empty string', id, 'attachTo.id'), + ); + } + if (typeof attachTo.input !== 'string' || attachTo.input === '') { + throw new Error( + errorMsg('must be a non-empty string', id, 'attachTo.input'), + ); + } + } + if (disabled !== undefined && typeof disabled !== 'boolean') { + throw new Error(errorMsg('must be a boolean', id, 'disabled')); + } + if ( + config !== undefined && + (typeof config !== 'object' || config === null || Array.isArray(config)) + ) { + throw new Error(errorMsg('must be an object', id, 'config')); + } + + const unknownKeys = Object.keys(value).filter( + k => !knownExtensionParameters.includes(k), + ); + if (unknownKeys.length > 0) { + throw new Error( + errorMsg( + `unknown parameter; expected one of '${knownExtensionParameters.join( + "', '", + )}'`, + id, + unknownKeys.join(', '), + ), + ); + } + + return { + id, + attachTo, + disabled, + config, + }; +} diff --git a/packages/frontend-app-api/src/wiring/parameters.test.ts b/packages/frontend-app-api/src/wiring/parameters.test.ts index 4ee420f667..89d9c0c3f3 100644 --- a/packages/frontend-app-api/src/wiring/parameters.test.ts +++ b/packages/frontend-app-api/src/wiring/parameters.test.ts @@ -14,18 +14,12 @@ * limitations under the License. */ -import { ConfigReader } from '@backstage/config'; import { createExtensionOverrides, createPlugin, Extension, } from '@backstage/frontend-plugin-api'; -import { JsonValue } from '@backstage/types'; -import { - expandShorthandExtensionParameters, - mergeExtensionParameters, - readAppExtensionParameters, -} from './parameters'; +import { mergeExtensionParameters } from './parameters'; function makeExt( id: string, @@ -208,249 +202,3 @@ describe('mergeExtensionParameters', () => { expect(result.map(r => r.extension.id)).toEqual(['b', 'c', 'a']); }); }); - -describe('readAppExtensionParameters', () => { - it('should disable extension with shorthand notation', () => { - expect( - readAppExtensionParameters( - new ConfigReader({ app: { extensions: [{ 'core.router': false }] } }), - ), - ).toEqual([ - { - id: 'core.router', - disabled: true, - }, - ]); - expect( - readAppExtensionParameters( - new ConfigReader({ - app: { extensions: [{ 'core.router': { disabled: true } }] }, - }), - ), - ).toEqual([ - { - at: undefined, - config: undefined, - disabled: true, - id: 'core.router', - }, - ]); - }); - - it('should enable extension with shorthand notation', () => { - expect( - readAppExtensionParameters( - new ConfigReader({ app: { extensions: ['core.router'] } }), - ), - ).toEqual([ - { - id: 'core.router', - disabled: false, - }, - ]); - expect( - readAppExtensionParameters( - new ConfigReader({ app: { extensions: [{ 'core.router': true }] } }), - ), - ).toEqual([ - { - id: 'core.router', - disabled: false, - }, - ]); - expect( - readAppExtensionParameters( - new ConfigReader({ - app: { extensions: [{ 'core.router': { disabled: false } }] }, - }), - ), - ).toEqual([ - { - id: 'core.router', - disabled: false, - }, - ]); - }); - - it('should not allow string keys', () => { - expect(() => - readAppExtensionParameters( - new ConfigReader({ - app: { - extensions: [{ 'core.router': 'some-string' }], - }, - }), - ), - ).toThrow( - 'Invalid extension configuration at app.extensions[0][core.router], value must be a boolean or object', - ); - }); - - it('should not allow invalid keys', () => { - expect(() => - readAppExtensionParameters( - new ConfigReader({ - app: { - extensions: [ - { - 'core.router/routes': { - extension: 'example-package#MyPage', - config: { foo: 'bar' }, - }, - }, - ], - }, - }), - ), - ).toThrowErrorMatchingInlineSnapshot( - `"Invalid extension configuration at app.extensions[0], extension ID must not contain slashes; got 'core.router/routes', did you mean 'core.router'?"`, - ); - }); -}); - -describe('expandShorthandExtensionParameters', () => { - const run = (value: JsonValue) => { - return expandShorthandExtensionParameters(value, 1); - }; - - it('rejects unknown keys', () => { - expect(() => run(null)).toThrowErrorMatchingInlineSnapshot( - `"Invalid extension configuration at app.extensions[1], must be a string or an object"`, - ); - expect(() => run(1)).toThrowErrorMatchingInlineSnapshot( - `"Invalid extension configuration at app.extensions[1], must be a string or an object"`, - ); - expect(() => run([])).toThrowErrorMatchingInlineSnapshot( - `"Invalid extension configuration at app.extensions[1], must be a string or an object"`, - ); - }); - - it('rejects the wrong number of keys', () => { - expect(() => run({})).toThrowErrorMatchingInlineSnapshot( - `"Invalid extension configuration at app.extensions[1], must have exactly one key, got none"`, - ); - expect(() => run({ a: {}, b: {} })).toThrowErrorMatchingInlineSnapshot( - `"Invalid extension configuration at app.extensions[1], must have exactly one key, got 'a', 'b'"`, - ); - }); - - it('rejects unknown values', () => { - expect(() => run({ a: 1 })).toThrowErrorMatchingInlineSnapshot( - `"Invalid extension configuration at app.extensions[1][a], value must be a boolean or object"`, - ); - expect(() => run({ a: [] })).toThrowErrorMatchingInlineSnapshot( - `"Invalid extension configuration at app.extensions[1][a], value must be a boolean or object"`, - ); - }); - - it('supports string key', () => { - expect(run('core.router')).toEqual({ - id: 'core.router', - disabled: false, - }); - expect(() => run('')).toThrowErrorMatchingInlineSnapshot( - `"Invalid extension configuration at app.extensions[1], extension ID must not be empty or contain whitespace"`, - ); - expect(() => run(' a')).toThrowErrorMatchingInlineSnapshot( - `"Invalid extension configuration at app.extensions[1], extension ID must not be empty or contain whitespace"`, - ); - expect(() => run('core.router/routes')).toThrowErrorMatchingInlineSnapshot( - `"Invalid extension configuration at app.extensions[1], extension ID must not contain slashes; got 'core.router/routes', did you mean 'core.router'?"`, - ); - }); - - it('supports null value', () => { - // this is the result of typing: - // - core.router: - // The missing value is interpreted as null by the yaml parser so we deal with that - expect(run({ 'core.router': null })).toEqual({ - id: 'core.router', - disabled: false, - }); - }); - - it('supports boolean value', () => { - expect(run({ 'core.router': true })).toEqual({ - id: 'core.router', - disabled: false, - }); - expect(run({ 'core.router': false })).toEqual({ - id: 'core.router', - disabled: true, - }); - }); - - it('should not support string values', () => { - expect(() => - run({ 'core.router': 'example-package#MyRouter' }), - ).toThrowErrorMatchingInlineSnapshot( - `"Invalid extension configuration at app.extensions[1][core.router], value must be a boolean or object"`, - ); - }); - - it('supports object id only in the key', () => { - expect(() => - run({ 'core.router': { id: 'some.id' } }), - ).toThrowErrorMatchingInlineSnapshot( - `"Invalid extension configuration at app.extensions[1][core.router].id, unknown parameter; expected one of 'attachTo', 'disabled', 'config'"`, - ); - }); - - it('supports object attachTo', () => { - expect( - run({ - 'core.router': { attachTo: { id: 'other.root', input: 'inputs' } }, - }), - ).toEqual({ - id: 'core.router', - attachTo: { id: 'other.root', input: 'inputs' }, - }); - expect(() => - run({ - 'core.router': { - id: 'other-id', - }, - }), - ).toThrowErrorMatchingInlineSnapshot( - `"Invalid extension configuration at app.extensions[1][core.router].id, unknown parameter; expected one of 'attachTo', 'disabled', 'config'"`, - ); - }); - - it('supports object disabled', () => { - expect(run({ 'core.router': { disabled: true } })).toEqual({ - id: 'core.router', - disabled: true, - }); - expect(run({ 'core.router': { disabled: false } })).toEqual({ - id: 'core.router', - disabled: false, - }); - expect(() => - run({ 'core.router': { disabled: 0 } }), - ).toThrowErrorMatchingInlineSnapshot( - `"Invalid extension configuration at app.extensions[1][core.router].disabled, must be a boolean"`, - ); - }); - - it('supports object config', () => { - expect( - run({ 'core.router': { config: { disableRedirects: true } } }), - ).toEqual({ - id: 'core.router', - config: { disableRedirects: true }, - }); - expect(() => - run({ 'core.router': { config: 0 } }), - ).toThrowErrorMatchingInlineSnapshot( - `"Invalid extension configuration at app.extensions[1][core.router].config, must be an object"`, - ); - }); - - it('rejects unknown object keys', () => { - expect(() => - run({ 'core.router': { foo: { settings: true } } }), - ).toThrowErrorMatchingInlineSnapshot( - `"Invalid extension configuration at app.extensions[1][core.router].foo, unknown parameter; expected one of 'attachTo', 'disabled', 'config'"`, - ); - }); -}); diff --git a/packages/frontend-app-api/src/wiring/parameters.ts b/packages/frontend-app-api/src/wiring/parameters.ts index 911f12f416..329c9ea42a 100644 --- a/packages/frontend-app-api/src/wiring/parameters.ts +++ b/packages/frontend-app-api/src/wiring/parameters.ts @@ -14,7 +14,6 @@ * limitations under the License. */ -import { Config } from '@backstage/config'; import { BackstagePlugin, Extension, @@ -22,188 +21,7 @@ import { } from '@backstage/frontend-plugin-api'; // eslint-disable-next-line @backstage/no-relative-monorepo-imports import { toInternalExtensionOverrides } from '../../../frontend-plugin-api/src/wiring/createExtensionOverrides'; -import { JsonValue } from '@backstage/types'; - -export interface ExtensionParameters { - id: string; - attachTo?: { id: string; input: string }; - disabled?: boolean; - config?: unknown; -} - -const knownExtensionParameters = ['attachTo', 'disabled', 'config']; - -// Since we'll never merge arrays in config the config reader context -// isn't too much of a help. Fall back to manual config reading logic -// as the Config interface makes it quite hard for us otherwise. -/** @internal */ -export function readAppExtensionParameters( - rootConfig: Config, -): ExtensionParameters[] { - const arr = rootConfig.getOptional('app.extensions'); - if (!Array.isArray(arr)) { - if (arr === undefined) { - return []; - } - // This will throw, and show which part of config had the wrong type - rootConfig.getConfigArray('app.extensions'); - return []; - } - - return arr.map((arrayEntry, arrayIndex) => - expandShorthandExtensionParameters(arrayEntry, arrayIndex), - ); -} - -/** @internal */ -export function expandShorthandExtensionParameters( - arrayEntry: JsonValue, - arrayIndex: number, -): ExtensionParameters { - function errorMsg(msg: string, key?: string, prop?: string) { - return `Invalid extension configuration at app.extensions[${arrayIndex}]${ - key ? `[${key}]` : '' - }${prop ? `.${prop}` : ''}, ${msg}`; - } - - // NOTE(freben): This check is intentionally not complete and doesn't check - // whether letters and digits are used, etc. It's not up to the config reading - // logic to decide what constitutes a valid extension ID; that should be - // decided by the logic that loads and instantiates the extensions. This check - // is just here to catch real mistakes or truly conceptually wrong input. - function assertValidId(id: string) { - if (!id || id !== id.trim()) { - throw new Error( - errorMsg('extension ID must not be empty or contain whitespace'), - ); - } - - if (id.includes('/')) { - let message = `extension ID must not contain slashes; got '${id}'`; - const good = id.split('/')[0]; - if (good) { - message += `, did you mean '${good}'?`; - } - throw new Error(errorMsg(message)); - } - } - - // Example YAML: - // - entity.card.about - if (typeof arrayEntry === 'string') { - assertValidId(arrayEntry); - return { - id: arrayEntry, - disabled: false, - }; - } - - // All remaining cases are single-key objects - if ( - typeof arrayEntry !== 'object' || - arrayEntry === null || - Array.isArray(arrayEntry) - ) { - throw new Error(errorMsg('must be a string or an object')); - } - const keys = Object.keys(arrayEntry); - if (keys.length !== 1) { - const joinedKeys = keys.length ? `'${keys.join("', '")}'` : 'none'; - throw new Error(errorMsg(`must have exactly one key, got ${joinedKeys}`)); - } - - const id = String(keys[0]); - const value = arrayEntry[id]; - assertValidId(id); - - // This example covers a potentially common mistake in the syntax - // Example YAML: - // - entity.card.about: - if (value === null) { - return { - id, - disabled: false, - }; - } - - // Example YAML: - // - catalog.page.cicd: false - if (typeof value === 'boolean') { - return { - id, - disabled: !value, - }; - } - - // The remaining case is the generic object. Example YAML: - // - tech-radar.page: - // at: core.router/routes - // disabled: false - // config: - // path: /tech-radar - // width: 1500 - // height: 800 - if (typeof value !== 'object' || Array.isArray(value)) { - // We don't mention null here - we don't want people to explicitly enter - // - entity.card.about: null - throw new Error(errorMsg('value must be a boolean or object', id)); - } - - const attachTo = value.attachTo as { id: string; input: string } | undefined; - const disabled = value.disabled; - const config = value.config; - - if (attachTo !== undefined) { - if ( - attachTo === null || - typeof attachTo !== 'object' || - Array.isArray(attachTo) - ) { - throw new Error(errorMsg('must be an object', id, 'attachTo')); - } - if (typeof attachTo.id !== 'string' || attachTo.id === '') { - throw new Error( - errorMsg('must be a non-empty string', id, 'attachTo.id'), - ); - } - if (typeof attachTo.input !== 'string' || attachTo.input === '') { - throw new Error( - errorMsg('must be a non-empty string', id, 'attachTo.input'), - ); - } - } - if (disabled !== undefined && typeof disabled !== 'boolean') { - throw new Error(errorMsg('must be a boolean', id, 'disabled')); - } - if ( - config !== undefined && - (typeof config !== 'object' || config === null || Array.isArray(config)) - ) { - throw new Error(errorMsg('must be an object', id, 'config')); - } - - const unknownKeys = Object.keys(value).filter( - k => !knownExtensionParameters.includes(k), - ); - if (unknownKeys.length > 0) { - throw new Error( - errorMsg( - `unknown parameter; expected one of '${knownExtensionParameters.join( - "', '", - )}'`, - id, - unknownKeys.join(', '), - ), - ); - } - - return { - id, - attachTo, - disabled, - config, - }; -} +import { ExtensionParameters } from './graph/readAppExtensionsConfig'; export interface ExtensionInstanceParameters { extension: Extension; From c617af1d6d0cd34d225957b547bfef2ede802e7e Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Sun, 15 Oct 2023 13:04:16 +0200 Subject: [PATCH 02/31] frontend-app-api: add initial app graph types Signed-off-by: Patrik Oldsberg --- .../src/wiring/graph/index.ts | 22 +++++++ .../src/wiring/graph/types.ts | 65 +++++++++++++++++++ 2 files changed, 87 insertions(+) create mode 100644 packages/frontend-app-api/src/wiring/graph/index.ts create mode 100644 packages/frontend-app-api/src/wiring/graph/types.ts diff --git a/packages/frontend-app-api/src/wiring/graph/index.ts b/packages/frontend-app-api/src/wiring/graph/index.ts new file mode 100644 index 0000000000..608ecb7675 --- /dev/null +++ b/packages/frontend-app-api/src/wiring/graph/index.ts @@ -0,0 +1,22 @@ +/* + * Copyright 2023 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +export type { + AppNode, + AppNodeEdges, + AppNodeInstance, + AppNodeSpec, +} from './types'; diff --git a/packages/frontend-app-api/src/wiring/graph/types.ts b/packages/frontend-app-api/src/wiring/graph/types.ts new file mode 100644 index 0000000000..3c5cd5df1b --- /dev/null +++ b/packages/frontend-app-api/src/wiring/graph/types.ts @@ -0,0 +1,65 @@ +/* + * Copyright 2023 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { + BackstagePlugin, + Extension, + ExtensionDataRef, +} from '@backstage/frontend-plugin-api'; + +/** + * The specification for this node in the app graph. + * @public + */ +export interface AppNodeSpec { + id: string; + attachTo: { id: string; input: string }; + extension: Extension; + disabled: boolean; + config?: unknown; + source?: BackstagePlugin; +} + +/** + * The connections from this node to other nodes. + * @public + */ +export interface AppNodeEdges { + attachedTo: { node: AppNode; input: string }; + attachments: Map; +} + +/** + * The instance of this node in the app graph. + * @public + */ +export interface AppNodeInstance { + getDataRefs(): ExtensionDataRef[]; + getData(ref: ExtensionDataRef): T | unknown; +} + +/** + * + * @public + */ +export interface AppNode { + /** The specification for how this node should be instantiated */ + spec: AppNodeSpec; + /** The edges from this node to other nodes in the app graph */ + edges: AppNodeEdges; + /** The instance of this node, if it was instantiated */ + instance?: AppNodeInstance; +} From b3977b9864b55c284fa0cec0b9e622d6381923d0 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Sun, 15 Oct 2023 13:11:11 +0200 Subject: [PATCH 03/31] frontend-app-api: refactor mergeExtensionParameters -> resolveAppNodeSpecs Signed-off-by: Patrik Oldsberg --- .../frontend-app-api/src/wiring/createApp.tsx | 21 +++++++------------ .../resolveAppNodeSpecs.test.ts} | 18 ++++++++-------- .../resolveAppNodeSpecs.ts} | 20 +++++++----------- 3 files changed, 24 insertions(+), 35 deletions(-) rename packages/frontend-app-api/src/wiring/{parameters.test.ts => graph/resolveAppNodeSpecs.test.ts} (93%) rename packages/frontend-app-api/src/wiring/{parameters.ts => graph/resolveAppNodeSpecs.ts} (93%) diff --git a/packages/frontend-app-api/src/wiring/createApp.tsx b/packages/frontend-app-api/src/wiring/createApp.tsx index dc132b6654..b2c82dc8cf 100644 --- a/packages/frontend-app-api/src/wiring/createApp.tsx +++ b/packages/frontend-app-api/src/wiring/createApp.tsx @@ -32,10 +32,7 @@ import { createExtensionInstance, ExtensionInstance, } from './createExtensionInstance'; -import { - ExtensionInstanceParameters, - mergeExtensionParameters, -} from './parameters'; +import { resolveAppNodeSpecs } from './graph/resolveAppNodeSpecs'; import { AnyApiFactory, ApiHolder, @@ -96,6 +93,7 @@ import { RoutingProvider } from '../routing/RoutingProvider'; import { resolveRouteBindings } from '../routing/resolveRouteBindings'; import { collectRouteIds } from '../routing/collectRouteIds'; import { readAppExtensionsConfig } from './graph/readAppExtensionsConfig'; +import { AppNodeSpec } from './graph'; /** @public */ export interface ExtensionTreeNode { @@ -197,7 +195,7 @@ export function createInstances(options: { // pull in default extension instance from discovered packages // apply config to adjust default extension instances and add more - const extensionParams = mergeExtensionParameters({ + const appNodeSpecs = resolveAppNodeSpecs({ features: options.features, builtinExtensions, parameters: readAppExtensionsConfig(options.config), @@ -207,11 +205,8 @@ export function createInstances(options: { // We do it at this point to ensure that merging (if any) of config has already happened // Create attachment map so that we can look attachments up during instance creation - const attachmentMap = new Map< - string, - Map - >(); - for (const instanceParams of extensionParams) { + const attachmentMap = new Map>(); + for (const instanceParams of appNodeSpecs) { const extensionId = instanceParams.attachTo.id; const pointId = instanceParams.attachTo.input; let pointMap = attachmentMap.get(extensionId); @@ -231,9 +226,7 @@ export function createInstances(options: { const instances = new Map(); - function createInstance( - instanceParams: ExtensionInstanceParameters, - ): ExtensionInstance { + function createInstance(instanceParams: AppNodeSpec): ExtensionInstance { const extensionId = instanceParams.extension.id; const existingInstance = instances.get(extensionId); if (existingInstance) { @@ -261,7 +254,7 @@ export function createInstances(options: { } const coreInstance = createInstance( - extensionParams.find(p => p.extension.id === 'core')!, + appNodeSpecs.find(p => p.extension.id === 'core')!, ); return { coreInstance, instances }; diff --git a/packages/frontend-app-api/src/wiring/parameters.test.ts b/packages/frontend-app-api/src/wiring/graph/resolveAppNodeSpecs.test.ts similarity index 93% rename from packages/frontend-app-api/src/wiring/parameters.test.ts rename to packages/frontend-app-api/src/wiring/graph/resolveAppNodeSpecs.test.ts index 89d9c0c3f3..1d3c0a26ed 100644 --- a/packages/frontend-app-api/src/wiring/parameters.test.ts +++ b/packages/frontend-app-api/src/wiring/graph/resolveAppNodeSpecs.test.ts @@ -19,7 +19,7 @@ import { createPlugin, Extension, } from '@backstage/frontend-plugin-api'; -import { mergeExtensionParameters } from './parameters'; +import { resolveAppNodeSpecs } from './resolveAppNodeSpecs'; function makeExt( id: string, @@ -33,10 +33,10 @@ function makeExt( } as Extension; } -describe('mergeExtensionParameters', () => { +describe('resolveAppNodeSpecs', () => { it('should filter out disabled extension instances', () => { expect( - mergeExtensionParameters({ + resolveAppNodeSpecs({ features: [], builtinExtensions: [makeExt('a', 'disabled')], parameters: [], @@ -48,7 +48,7 @@ describe('mergeExtensionParameters', () => { const a = makeExt('a'); const b = makeExt('b'); expect( - mergeExtensionParameters({ + resolveAppNodeSpecs({ features: [], builtinExtensions: [a, b], parameters: [], @@ -64,7 +64,7 @@ describe('mergeExtensionParameters', () => { const b = makeExt('b'); const pluginA = createPlugin({ id: 'test', extensions: [a] }); expect( - mergeExtensionParameters({ + resolveAppNodeSpecs({ features: [pluginA], builtinExtensions: [b], parameters: [ @@ -89,7 +89,7 @@ describe('mergeExtensionParameters', () => { const b = makeExt('b'); const plugin = createPlugin({ id: 'test', extensions: [a, b] }); expect( - mergeExtensionParameters({ + resolveAppNodeSpecs({ features: [plugin], builtinExtensions: [], parameters: [ @@ -127,7 +127,7 @@ describe('mergeExtensionParameters', () => { const a = makeExt('a', 'disabled'); const b = makeExt('b', 'disabled'); expect( - mergeExtensionParameters({ + resolveAppNodeSpecs({ features: [createPlugin({ id: 'empty', extensions: [] })], builtinExtensions: [a, b], parameters: [ @@ -155,7 +155,7 @@ describe('mergeExtensionParameters', () => { const bOverride = makeExt('b', 'disabled', 'other'); const cOverride = makeExt('c'); - const result = mergeExtensionParameters({ + const result = resolveAppNodeSpecs({ features: [ plugin, createExtensionOverrides({ @@ -188,7 +188,7 @@ describe('mergeExtensionParameters', () => { const bOverride = makeExt('b', 'disabled'); const cOverride = makeExt('a', 'disabled'); - const result = mergeExtensionParameters({ + const result = resolveAppNodeSpecs({ features: [ createPlugin({ id: 'test', extensions: [a, b, c] }), createExtensionOverrides({ diff --git a/packages/frontend-app-api/src/wiring/parameters.ts b/packages/frontend-app-api/src/wiring/graph/resolveAppNodeSpecs.ts similarity index 93% rename from packages/frontend-app-api/src/wiring/parameters.ts rename to packages/frontend-app-api/src/wiring/graph/resolveAppNodeSpecs.ts index 329c9ea42a..8cad54f4be 100644 --- a/packages/frontend-app-api/src/wiring/parameters.ts +++ b/packages/frontend-app-api/src/wiring/graph/resolveAppNodeSpecs.ts @@ -20,22 +20,16 @@ import { ExtensionOverrides, } from '@backstage/frontend-plugin-api'; // eslint-disable-next-line @backstage/no-relative-monorepo-imports -import { toInternalExtensionOverrides } from '../../../frontend-plugin-api/src/wiring/createExtensionOverrides'; -import { ExtensionParameters } from './graph/readAppExtensionsConfig'; - -export interface ExtensionInstanceParameters { - extension: Extension; - source?: BackstagePlugin; - attachTo: { id: string; input: string }; - config?: unknown; -} +import { toInternalExtensionOverrides } from '../../../../frontend-plugin-api/src/wiring/createExtensionOverrides'; +import { ExtensionParameters } from './readAppExtensionsConfig'; +import { AppNodeSpec } from './types'; /** @internal */ -export function mergeExtensionParameters(options: { +export function resolveAppNodeSpecs(options: { features: (BackstagePlugin | ExtensionOverrides)[]; builtinExtensions: Extension[]; parameters: Array; -}): ExtensionInstanceParameters[] { +}): AppNodeSpec[] { const { builtinExtensions, parameters } = options; const plugins = options.features.filter( @@ -207,8 +201,10 @@ export function mergeExtensionParameters(options: { return configuredExtensions .filter(override => !override.params.disabled) .map(param => ({ - extension: param.extension, + id: param.extension.id, attachTo: param.params.attachTo, + extension: param.extension, + disabled: param.params.disabled, source: param.params.source, config: param.params.config, })); From d2bee21c914ddbaa7a2ef613411ec3b8fc18d32d Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Mon, 16 Oct 2023 00:30:46 +0200 Subject: [PATCH 04/31] frontend-app-api: add initial app graph builder Signed-off-by: Patrik Oldsberg --- .../src/wiring/graph/buildAppGraph.test.ts | 74 +++++++++++++ .../src/wiring/graph/buildAppGraph.ts | 104 ++++++++++++++++++ .../src/wiring/graph/types.ts | 26 +++-- 3 files changed, 193 insertions(+), 11 deletions(-) create mode 100644 packages/frontend-app-api/src/wiring/graph/buildAppGraph.test.ts create mode 100644 packages/frontend-app-api/src/wiring/graph/buildAppGraph.ts diff --git a/packages/frontend-app-api/src/wiring/graph/buildAppGraph.test.ts b/packages/frontend-app-api/src/wiring/graph/buildAppGraph.test.ts new file mode 100644 index 0000000000..1d5d8f415f --- /dev/null +++ b/packages/frontend-app-api/src/wiring/graph/buildAppGraph.test.ts @@ -0,0 +1,74 @@ +/* + * Copyright 2023 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { createExtension } from '@backstage/frontend-plugin-api'; +import { buildAppGraph } from './buildAppGraph'; + +const extBaseConfig = { + id: 'test', + attachTo: { id: 'root', input: 'default' }, + output: {}, + factory() {}, +}; + +const extension = createExtension(extBaseConfig); + +const baseSpec = { extension, disabled: false }; + +describe('buildAppGraph', () => { + it('creates an empty graph', () => { + const graph = buildAppGraph([]); + expect([...graph.rootNodes.keys()]).toEqual([]); + expect([...graph.orphanNodes.keys()]).toEqual([]); + }); + + it('should create a graph', () => { + const graph = buildAppGraph([ + { ...baseSpec, id: 'a' }, + { ...baseSpec, id: 'b' }, + { ...baseSpec, id: 'c' }, + { ...baseSpec, attachTo: { id: 'b', input: 'x' }, id: 'bx1' }, + { ...baseSpec, attachTo: { id: 'b', input: 'x' }, id: 'bx2' }, + { ...baseSpec, attachTo: { id: 'b', input: 'y' }, id: 'by1' }, + { ...baseSpec, attachTo: { id: 'd', input: 'x' }, id: 'dx1' }, + ]); + expect([...graph.rootNodes.keys()]).toEqual(['a', 'b', 'c']); + expect([...graph.orphanNodes.keys()]).toEqual(['dx1']); + }); + + it('should create a graph out of order', () => { + const graph = buildAppGraph([ + { ...baseSpec, attachTo: { id: 'b', input: 'x' }, id: 'bx2' }, + { ...baseSpec, id: 'a' }, + { ...baseSpec, attachTo: { id: 'b', input: 'y' }, id: 'by1' }, + { ...baseSpec, id: 'b' }, + { ...baseSpec, attachTo: { id: 'b', input: 'x' }, id: 'bx1' }, + { ...baseSpec, id: 'c' }, + { ...baseSpec, attachTo: { id: 'd', input: 'x' }, id: 'dx1' }, + ]); + expect([...graph.rootNodes.keys()]).toEqual(['a', 'b', 'c']); + expect([...graph.orphanNodes.keys()]).toEqual(['dx1']); + }); + + it('throws an error when duplicated extensions are detected', () => { + expect(() => + buildAppGraph([ + { ...baseSpec, id: 'a' }, + { ...baseSpec, id: 'a' }, + ]), + ).toThrow("Unexpected duplicate extension id 'a'"); + }); +}); diff --git a/packages/frontend-app-api/src/wiring/graph/buildAppGraph.ts b/packages/frontend-app-api/src/wiring/graph/buildAppGraph.ts new file mode 100644 index 0000000000..151faa0e1d --- /dev/null +++ b/packages/frontend-app-api/src/wiring/graph/buildAppGraph.ts @@ -0,0 +1,104 @@ +/* + * Copyright 2023 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { AppNode, AppNodeEdges, AppNodeSpec } from './types'; + +type Mutable = { + -readonly [P in keyof T]: T[P]; +}; + +/** + * Build the app graph by iterating through all node specs and constructing the app + * tree with all attachments in the same order as they appear in the input specs array. + * @internal + */ +export function buildAppGraph(specs: AppNodeSpec[]) { + const nodes = new Map(); + const rootNodes = new Map(); + // While iterating through the inputs specs we keep track of all nodes that were created + // before their parent, and attach them later when the parent is created. + // As we find the parents and attach the children, we remove them from this map. This means + // that after iterating through all input specs, this will be a map for each root node. + const orphansByParent = new Map< + string /* parentId */, + { orphan: AppNode; input: string }[] + >(); + + for (const spec of specs) { + // The main check with a more helpful error message happens in resolveAppNodeSpecs + if (nodes.has(spec.id)) { + throw new Error(`Unexpected duplicate extension id '${spec.id}'`); + } + + const node: AppNode = { + spec, + edges: { + attachments: new Map(), + }, + }; + nodes.set(spec.id, node); + + if (spec.attachTo) { + const parent = nodes.get(spec.attachTo.id); + if (parent) { + (node.edges as Mutable).attachedTo = { + node: parent, + input: spec.attachTo.input, + }; + const parentInputEdges = parent.edges.attachments.get( + spec.attachTo.input, + ); + if (parentInputEdges) { + parentInputEdges.push(node); + } else { + parent.edges.attachments.set(spec.attachTo.input, [node]); + } + } else { + const orphanNodesForParent = orphansByParent.get(spec.attachTo.id); + const orphan = { orphan: node, input: spec.attachTo.input }; + if (orphanNodesForParent) { + orphanNodesForParent.push(orphan); + } else { + orphansByParent.set(spec.attachTo.id, [orphan]); + } + } + } else { + rootNodes.set(spec.id, node); + } + + const orphanedChildren = orphansByParent.get(spec.id); + if (orphanedChildren) { + orphansByParent.delete(spec.id); + for (const { orphan, input } of orphanedChildren) { + (orphan.edges as Mutable).attachedTo = { node, input }; + const attachments = node.edges.attachments.get(input); + if (attachments) { + attachments.push(orphan); + } else { + node.edges.attachments.set(input, [orphan]); + } + } + } + } + + const orphanNodes = new Map( + Array.from(orphansByParent).flatMap(([, orphans]) => + orphans.map(({ orphan }) => [orphan.spec.id, orphan]), + ), + ); + + return { rootNodes, orphanNodes }; +} diff --git a/packages/frontend-app-api/src/wiring/graph/types.ts b/packages/frontend-app-api/src/wiring/graph/types.ts index 3c5cd5df1b..f6de7018f5 100644 --- a/packages/frontend-app-api/src/wiring/graph/types.ts +++ b/packages/frontend-app-api/src/wiring/graph/types.ts @@ -25,12 +25,12 @@ import { * @public */ export interface AppNodeSpec { - id: string; - attachTo: { id: string; input: string }; - extension: Extension; - disabled: boolean; - config?: unknown; - source?: BackstagePlugin; + readonly id: string; + readonly attachTo?: { id: string; input: string }; + readonly extension: Extension; + readonly disabled: boolean; + readonly config?: unknown; + readonly source?: BackstagePlugin; } /** @@ -38,8 +38,8 @@ export interface AppNodeSpec { * @public */ export interface AppNodeEdges { - attachedTo: { node: AppNode; input: string }; - attachments: Map; + readonly attachedTo?: { node: AppNode; input: string }; + readonly attachments: Map; } /** @@ -57,9 +57,13 @@ export interface AppNodeInstance { */ export interface AppNode { /** The specification for how this node should be instantiated */ - spec: AppNodeSpec; + readonly spec: AppNodeSpec; /** The edges from this node to other nodes in the app graph */ - edges: AppNodeEdges; + readonly edges: AppNodeEdges; /** The instance of this node, if it was instantiated */ - instance?: AppNodeInstance; + readonly instance?: AppNodeInstance; +} + +export interface AppGraph { + rootNodes: Map; } From 650309205b899fab980d7c20e89657191cc6328c Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Tue, 17 Oct 2023 17:44:29 +0200 Subject: [PATCH 05/31] frontend-app-api: move createExtensionInstance to graph Signed-off-by: Patrik Oldsberg --- .../src/routing/extractRouteInfoFromInstanceTree.ts | 2 +- packages/frontend-app-api/src/wiring/createApp.tsx | 2 +- .../src/wiring/{ => graph}/createExtensionInstance.test.ts | 0 .../src/wiring/{ => graph}/createExtensionInstance.ts | 0 4 files changed, 2 insertions(+), 2 deletions(-) rename packages/frontend-app-api/src/wiring/{ => graph}/createExtensionInstance.test.ts (100%) rename packages/frontend-app-api/src/wiring/{ => graph}/createExtensionInstance.ts (100%) diff --git a/packages/frontend-app-api/src/routing/extractRouteInfoFromInstanceTree.ts b/packages/frontend-app-api/src/routing/extractRouteInfoFromInstanceTree.ts index 0bf9c2d1dd..fadcb719b9 100644 --- a/packages/frontend-app-api/src/routing/extractRouteInfoFromInstanceTree.ts +++ b/packages/frontend-app-api/src/routing/extractRouteInfoFromInstanceTree.ts @@ -15,7 +15,7 @@ */ import { RouteRef, coreExtensionData } from '@backstage/frontend-plugin-api'; -import { ExtensionInstance } from '../wiring/createExtensionInstance'; +import { ExtensionInstance } from '../wiring/graph/createExtensionInstance'; // eslint-disable-next-line @backstage/no-relative-monorepo-imports import { toLegacyPlugin } from '../wiring/createApp'; import { BackstageRouteObject } from './types'; diff --git a/packages/frontend-app-api/src/wiring/createApp.tsx b/packages/frontend-app-api/src/wiring/createApp.tsx index b2c82dc8cf..485882c8eb 100644 --- a/packages/frontend-app-api/src/wiring/createApp.tsx +++ b/packages/frontend-app-api/src/wiring/createApp.tsx @@ -31,7 +31,7 @@ import { CoreNav } from '../extensions/CoreNav'; import { createExtensionInstance, ExtensionInstance, -} from './createExtensionInstance'; +} from './graph/createExtensionInstance'; import { resolveAppNodeSpecs } from './graph/resolveAppNodeSpecs'; import { AnyApiFactory, diff --git a/packages/frontend-app-api/src/wiring/createExtensionInstance.test.ts b/packages/frontend-app-api/src/wiring/graph/createExtensionInstance.test.ts similarity index 100% rename from packages/frontend-app-api/src/wiring/createExtensionInstance.test.ts rename to packages/frontend-app-api/src/wiring/graph/createExtensionInstance.test.ts diff --git a/packages/frontend-app-api/src/wiring/createExtensionInstance.ts b/packages/frontend-app-api/src/wiring/graph/createExtensionInstance.ts similarity index 100% rename from packages/frontend-app-api/src/wiring/createExtensionInstance.ts rename to packages/frontend-app-api/src/wiring/graph/createExtensionInstance.ts From 7df6a42e4ad95bacaed4403ee7cab2aac273afb8 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Tue, 17 Oct 2023 17:44:49 +0200 Subject: [PATCH 06/31] frontend-app-api: extract Mutable to common internal type in graph Signed-off-by: Patrik Oldsberg --- packages/frontend-app-api/src/wiring/graph/buildAppGraph.ts | 6 +----- packages/frontend-app-api/src/wiring/graph/types.ts | 5 +++++ 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/packages/frontend-app-api/src/wiring/graph/buildAppGraph.ts b/packages/frontend-app-api/src/wiring/graph/buildAppGraph.ts index 151faa0e1d..9cfb430bcb 100644 --- a/packages/frontend-app-api/src/wiring/graph/buildAppGraph.ts +++ b/packages/frontend-app-api/src/wiring/graph/buildAppGraph.ts @@ -14,11 +14,7 @@ * limitations under the License. */ -import { AppNode, AppNodeEdges, AppNodeSpec } from './types'; - -type Mutable = { - -readonly [P in keyof T]: T[P]; -}; +import { AppNode, AppNodeEdges, AppNodeSpec, Mutable } from './types'; /** * Build the app graph by iterating through all node specs and constructing the app diff --git a/packages/frontend-app-api/src/wiring/graph/types.ts b/packages/frontend-app-api/src/wiring/graph/types.ts index f6de7018f5..88fc363d6f 100644 --- a/packages/frontend-app-api/src/wiring/graph/types.ts +++ b/packages/frontend-app-api/src/wiring/graph/types.ts @@ -20,6 +20,11 @@ import { ExtensionDataRef, } from '@backstage/frontend-plugin-api'; +/** @internal */ +export type Mutable = { + -readonly [P in keyof T]: T[P]; +}; + /** * The specification for this node in the app graph. * @public From 544968da085cca68110f42af2675ce414ab9d762 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Tue, 17 Oct 2023 18:01:45 +0200 Subject: [PATCH 07/31] frontend-app-api: make app node serializable Signed-off-by: Patrik Oldsberg --- .../src/wiring/graph/buildAppGraph.test.ts | 57 ++++++++++++++++ .../src/wiring/graph/buildAppGraph.ts | 68 ++++++++++++++++--- .../src/wiring/graph/types.ts | 3 +- 3 files changed, 119 insertions(+), 9 deletions(-) diff --git a/packages/frontend-app-api/src/wiring/graph/buildAppGraph.test.ts b/packages/frontend-app-api/src/wiring/graph/buildAppGraph.test.ts index 1d5d8f415f..21a5009a95 100644 --- a/packages/frontend-app-api/src/wiring/graph/buildAppGraph.test.ts +++ b/packages/frontend-app-api/src/wiring/graph/buildAppGraph.test.ts @@ -47,6 +47,49 @@ describe('buildAppGraph', () => { ]); expect([...graph.rootNodes.keys()]).toEqual(['a', 'b', 'c']); expect([...graph.orphanNodes.keys()]).toEqual(['dx1']); + + expect(JSON.parse(JSON.stringify([...graph.rootNodes.values()]))) + .toMatchInlineSnapshot(` + [ + { + "id": "a", + }, + { + "attachments": { + "x": [ + { + "id": "bx1", + }, + { + "id": "bx2", + }, + ], + "y": [ + { + "id": "by1", + }, + ], + }, + "id": "b", + }, + { + "id": "c", + }, + ] + `); + expect(String(graph.rootNodes.get('a'))).toMatchInlineSnapshot(`""`); + expect(String(graph.rootNodes.get('b'))).toMatchInlineSnapshot(` + " + x [ + + + ] + y [ + + ] + " + `); + expect(String(graph.rootNodes.get('c'))).toMatchInlineSnapshot(`""`); }); it('should create a graph out of order', () => { @@ -61,6 +104,20 @@ describe('buildAppGraph', () => { ]); expect([...graph.rootNodes.keys()]).toEqual(['a', 'b', 'c']); expect([...graph.orphanNodes.keys()]).toEqual(['dx1']); + + expect(String(graph.rootNodes.get('a'))).toMatchInlineSnapshot(`""`); + expect(String(graph.rootNodes.get('b'))).toMatchInlineSnapshot(` + " + x [ + + + ] + y [ + + ] + " + `); + expect(String(graph.rootNodes.get('c'))).toMatchInlineSnapshot(`""`); }); it('throws an error when duplicated extensions are detected', () => { diff --git a/packages/frontend-app-api/src/wiring/graph/buildAppGraph.ts b/packages/frontend-app-api/src/wiring/graph/buildAppGraph.ts index 9cfb430bcb..79977f6993 100644 --- a/packages/frontend-app-api/src/wiring/graph/buildAppGraph.ts +++ b/packages/frontend-app-api/src/wiring/graph/buildAppGraph.ts @@ -14,14 +14,71 @@ * limitations under the License. */ -import { AppNode, AppNodeEdges, AppNodeSpec, Mutable } from './types'; +import { + AppGraph, + AppNode, + AppNodeEdges, + AppNodeInstance, + AppNodeSpec, + Mutable, +} from './types'; + +function indent(str: string) { + return str.replace(/^/gm, ' '); +} + +/** @internal */ +class SerializableAppNode implements AppNode { + public readonly spec: AppNodeSpec; + public readonly edges: AppNodeEdges = { attachments: new Map() }; + public readonly instance?: AppNodeInstance; + + constructor(spec: AppNodeSpec) { + this.spec = spec; + } + + toJSON() { + const dataRefs = this.instance && [...this.instance.getDataRefs()]; + return { + id: this.spec.id, + output: + dataRefs && dataRefs.length > 0 + ? dataRefs.map(ref => ref.id) + : undefined, + attachments: + this.edges.attachments.size > 0 + ? Object.fromEntries(this.edges.attachments) + : undefined, + }; + } + + toString() { + const dataRefs = this.instance && [...this.instance.getDataRefs()]; + const out = + dataRefs && dataRefs.length > 0 + ? ` out=[${[...dataRefs.keys()].join(', ')}]` + : ''; + + if (this.edges.attachments.size === 0) { + return `<${this.spec.id}${out} />`; + } + + return [ + `<${this.spec.id}${out}>`, + ...[...this.edges.attachments.entries()].map(([k, v]) => + indent([`${k} [`, ...v.map(e => indent(e.toString())), `]`].join('\n')), + ), + ``, + ].join('\n'); + } +} /** * Build the app graph by iterating through all node specs and constructing the app * tree with all attachments in the same order as they appear in the input specs array. * @internal */ -export function buildAppGraph(specs: AppNodeSpec[]) { +export function buildAppGraph(specs: AppNodeSpec[]): AppGraph { const nodes = new Map(); const rootNodes = new Map(); // While iterating through the inputs specs we keep track of all nodes that were created @@ -39,12 +96,7 @@ export function buildAppGraph(specs: AppNodeSpec[]) { throw new Error(`Unexpected duplicate extension id '${spec.id}'`); } - const node: AppNode = { - spec, - edges: { - attachments: new Map(), - }, - }; + const node = new SerializableAppNode(spec); nodes.set(spec.id, node); if (spec.attachTo) { diff --git a/packages/frontend-app-api/src/wiring/graph/types.ts b/packages/frontend-app-api/src/wiring/graph/types.ts index 88fc363d6f..29c0028d66 100644 --- a/packages/frontend-app-api/src/wiring/graph/types.ts +++ b/packages/frontend-app-api/src/wiring/graph/types.ts @@ -52,7 +52,7 @@ export interface AppNodeEdges { * @public */ export interface AppNodeInstance { - getDataRefs(): ExtensionDataRef[]; + getDataRefs(): Iterable>; getData(ref: ExtensionDataRef): T | unknown; } @@ -71,4 +71,5 @@ export interface AppNode { export interface AppGraph { rootNodes: Map; + orphanNodes: Map; } From bb4d40b481d92f9e85157a20bc18064a7fb69adc Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Wed, 18 Oct 2023 01:04:50 +0200 Subject: [PATCH 08/31] frontend-app-api: refactor createExtensionInstance -> createAppNodeInstance Signed-off-by: Patrik Oldsberg --- ...onInstance.ts => createAppNodeInstance.ts} | 136 ++++++------------ 1 file changed, 41 insertions(+), 95 deletions(-) rename packages/frontend-app-api/src/wiring/graph/{createExtensionInstance.ts => createAppNodeInstance.ts} (58%) diff --git a/packages/frontend-app-api/src/wiring/graph/createExtensionInstance.ts b/packages/frontend-app-api/src/wiring/graph/createAppNodeInstance.ts similarity index 58% rename from packages/frontend-app-api/src/wiring/graph/createExtensionInstance.ts rename to packages/frontend-app-api/src/wiring/graph/createAppNodeInstance.ts index 767329da7f..310852cd1c 100644 --- a/packages/frontend-app-api/src/wiring/graph/createExtensionInstance.ts +++ b/packages/frontend-app-api/src/wiring/graph/createAppNodeInstance.ts @@ -18,10 +18,12 @@ import { AnyExtensionDataMap, AnyExtensionInputMap, BackstagePlugin, - Extension, ExtensionDataRef, } from '@backstage/frontend-plugin-api'; import mapValues from 'lodash/mapValues'; +import { AppNode, AppNodeInstance, AppNodeSpec } from './types'; + +type AppNodeWithInstance = AppNode & { instance: AppNodeInstance }; /** @internal */ export interface ExtensionInstance { @@ -42,14 +44,14 @@ export interface ExtensionInstance { function resolveInputData( dataMap: AnyExtensionDataMap, - attachment: ExtensionInstance, + attachment: AppNodeWithInstance, inputName: string, ) { return mapValues(dataMap, ref => { - const value = attachment.getData(ref); + const value = attachment.instance.getData(ref); if (value === undefined && !ref.config.optional) { throw new Error( - `input '${inputName}' did not receive required extension data '${ref.id}' from extension '${attachment.id}'`, + `input '${inputName}' did not receive required extension data '${ref.id}' from extension '${attachment.spec.id}'`, ); } return value; @@ -58,7 +60,7 @@ function resolveInputData( function resolveInputs( inputMap: AnyExtensionInputMap, - attachments: Map, + attachments: Map, ) { const undeclaredAttachments = Array.from(attachments.entries()).filter( ([inputName]) => inputMap[inputName] === undefined, @@ -72,7 +74,7 @@ function resolveInputs( .map( ([k, exts]) => `'${k}' from extension${exts.length > 1 ? 's' : ''} '${exts - .map(e => e.id) + .map(e => e.spec.id) .join("', '")}'`, ) .join(' and ')}`, @@ -80,113 +82,54 @@ function resolveInputs( } return mapValues(inputMap, (input, inputName) => { - const attachedInstances = attachments.get(inputName) ?? []; + const attachedNodes = + attachments + .get(inputName) + ?.filter((node): node is AppNodeWithInstance => + Boolean(node.instance), + ) ?? []; + if (input.config.singleton) { - if (attachedInstances.length > 1) { + if (attachedNodes.length > 1) { + const attachedNodeIds = attachedNodes.map(e => e.spec.id); throw Error( `expected ${ input.config.optional ? 'at most' : 'exactly' - } one '${inputName}' input but received multiple: '${attachedInstances - .map(e => e.id) - .join("', '")}'`, + } one '${inputName}' input but received multiple: '${attachedNodeIds.join( + "', '", + )}'`, ); - } else if (attachedInstances.length === 0) { + } else if (attachedNodes.length === 0) { if (input.config.optional) { return undefined; } throw Error(`input '${inputName}' is required but was not received`); } - return resolveInputData( - input.extensionData, - attachedInstances[0], - inputName, - ); + return resolveInputData(input.extensionData, attachedNodes[0], inputName); } - return attachedInstances.map(attachment => + return attachedNodes.map(attachment => resolveInputData(input.extensionData, attachment, inputName), ); }); } -function indent(str: string) { - return str.replace(/^/gm, ' '); -} - -class ExtensionInstanceImpl implements ExtensionInstance { - readonly $$type = '@backstage/ExtensionInstance'; - - readonly id: string; - readonly #extensionData: Map; - readonly attachments: Map; - readonly source?: BackstagePlugin; - - constructor( - id: string, - extensionData: Map, - attachments: Map, - source: BackstagePlugin | undefined, - ) { - this.id = id; - this.#extensionData = extensionData; - this.attachments = attachments; - this.source = source; - } - - getData(ref: ExtensionDataRef): T | undefined { - return this.#extensionData.get(ref.id) as T | undefined; - } - - toJSON() { - return { - id: this.id, - output: - this.#extensionData.size > 0 - ? [...this.#extensionData.keys()] - : undefined, - attachments: - this.attachments.size > 0 - ? Object.fromEntries(this.attachments) - : undefined, - }; - } - - toString() { - const out = - this.#extensionData.size > 0 - ? ` out=[${[...this.#extensionData.keys()].join(', ')}]` - : ''; - - if (this.attachments.size === 0) { - return `<${this.id}${out} />`; - } - - return [ - `<${this.id}${out}>`, - ...[...this.attachments.entries()].map(([k, v]) => - indent([`${k} [`, ...v.map(e => indent(e.toString())), `]`].join('\n')), - ), - ``, - ].join('\n'); - } -} - /** @internal */ -export function createExtensionInstance(options: { - extension: Extension; - config: unknown; - source?: BackstagePlugin; - attachments: Map; -}): ExtensionInstance { - const { extension, config, source, attachments } = options; +export function createAppNodeInstance(options: { + spec: AppNodeSpec; + attachments: Map; +}): AppNodeInstance { + const { spec, attachments } = options; + const { id, extension, config, source } = spec; const extensionData = new Map(); + const extensionDataRefs = new Set>(); let parsedConfig: unknown; try { parsedConfig = extension.configSchema?.parse(config ?? {}); } catch (e) { throw new Error( - `Invalid configuration for extension '${extension.id}'; caused by ${e}`, + `Invalid configuration for extension '${id}'; caused by ${e}`, ); } @@ -206,22 +149,25 @@ export function createExtensionInstance(options: { ); } extensionData.set(ref.id, output); + extensionDataRefs.add(ref); } }, inputs: resolveInputs(extension.inputs, attachments), }); } catch (e) { throw new Error( - `Failed to instantiate extension '${extension.id}'${ + `Failed to instantiate extension '${id}'${ e.name === 'Error' ? `, ${e.message}` : `; caused by ${e}` }`, ); } - return new ExtensionInstanceImpl( - options.extension.id, - extensionData, - attachments, - source, - ); + return { + getDataRefs() { + return extensionDataRefs.values(); + }, + getData(ref: ExtensionDataRef): T | undefined { + return extensionData.get(ref.id) as T | undefined; + }, + }; } From 2ff857bdb4d83eba66493bec3c892a611a7b7edc Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Wed, 18 Oct 2023 01:22:54 +0200 Subject: [PATCH 09/31] frontend-app-api: add instantiateAppNodeTree Signed-off-by: Patrik Oldsberg --- .../wiring/graph/instantiateAppNodeTree.ts | 44 +++++++++++++++++++ 1 file changed, 44 insertions(+) create mode 100644 packages/frontend-app-api/src/wiring/graph/instantiateAppNodeTree.ts diff --git a/packages/frontend-app-api/src/wiring/graph/instantiateAppNodeTree.ts b/packages/frontend-app-api/src/wiring/graph/instantiateAppNodeTree.ts new file mode 100644 index 0000000000..b01ad020b9 --- /dev/null +++ b/packages/frontend-app-api/src/wiring/graph/instantiateAppNodeTree.ts @@ -0,0 +1,44 @@ +/* + * Copyright 2023 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { createAppNodeInstance } from './createAppNodeInstance'; +import { AppNode, Mutable } from './types'; + +export function instantiateAppNodeTree(rootNode: AppNode): void { + function createInstance(node: AppNode): void { + if (node.instance) { + return; + } + if (node.spec.disabled) { + return; + } + + for (const children of node.edges.attachments.values()) { + for (const child of children) { + createInstance(child); + } + } + + (node as Mutable).instance = createAppNodeInstance({ + spec: node.spec, + attachments: node.edges.attachments, + }); + + return; + } + + createInstance(rootNode); +} From 6e6458f9c2b47b060e3739f89fda42aa7ae010f6 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Wed, 18 Oct 2023 10:55:55 +0200 Subject: [PATCH 10/31] frontend-app-api: tweak app graph to have an explicit root node Signed-off-by: Patrik Oldsberg --- .../frontend-app-api/src/extensions/Core.tsx | 2 +- .../src/wiring/graph/buildAppGraph.test.ts | 142 ++++++++++-------- .../src/wiring/graph/buildAppGraph.ts | 29 ++-- .../src/wiring/graph/types.ts | 6 +- 4 files changed, 104 insertions(+), 75 deletions(-) diff --git a/packages/frontend-app-api/src/extensions/Core.tsx b/packages/frontend-app-api/src/extensions/Core.tsx index 6efe3984dc..f48aa2ea6b 100644 --- a/packages/frontend-app-api/src/extensions/Core.tsx +++ b/packages/frontend-app-api/src/extensions/Core.tsx @@ -22,7 +22,7 @@ import { export const Core = createExtension({ id: 'core', - attachTo: { id: 'root', input: 'default' }, + attachTo: { id: 'root', input: 'default' }, // ignored inputs: { apis: createExtensionInput({ api: coreExtensionData.apiFactory, diff --git a/packages/frontend-app-api/src/wiring/graph/buildAppGraph.test.ts b/packages/frontend-app-api/src/wiring/graph/buildAppGraph.test.ts index 21a5009a95..dc489565f4 100644 --- a/packages/frontend-app-api/src/wiring/graph/buildAppGraph.test.ts +++ b/packages/frontend-app-api/src/wiring/graph/buildAppGraph.test.ts @@ -19,66 +19,70 @@ import { buildAppGraph } from './buildAppGraph'; const extBaseConfig = { id: 'test', - attachTo: { id: 'root', input: 'default' }, + attachTo: { id: 'nonexistent', input: 'nonexistent' }, output: {}, factory() {}, }; const extension = createExtension(extBaseConfig); -const baseSpec = { extension, disabled: false }; +const baseSpec = { + extension, + attachTo: { id: 'nonexistent', input: 'nonexistent' }, + disabled: false, +}; describe('buildAppGraph', () => { - it('creates an empty graph', () => { - const graph = buildAppGraph([]); - expect([...graph.rootNodes.keys()]).toEqual([]); - expect([...graph.orphanNodes.keys()]).toEqual([]); + it('should fail to create an empty graph', () => { + expect(() => buildAppGraph([])).toThrow( + "No root node with id 'core' found in app graph", + ); + }); + + it('should create a graph with only one node', () => { + const graph = buildAppGraph([{ ...baseSpec, id: 'core' }]); + expect(graph.root).toEqual({ + spec: { ...baseSpec, id: 'core' }, + edges: { attachments: new Map() }, + }); + expect(Array.from(graph.orphans)).toEqual([]); }); it('should create a graph', () => { - const graph = buildAppGraph([ - { ...baseSpec, id: 'a' }, - { ...baseSpec, id: 'b' }, - { ...baseSpec, id: 'c' }, - { ...baseSpec, attachTo: { id: 'b', input: 'x' }, id: 'bx1' }, - { ...baseSpec, attachTo: { id: 'b', input: 'x' }, id: 'bx2' }, - { ...baseSpec, attachTo: { id: 'b', input: 'y' }, id: 'by1' }, - { ...baseSpec, attachTo: { id: 'd', input: 'x' }, id: 'dx1' }, - ]); - expect([...graph.rootNodes.keys()]).toEqual(['a', 'b', 'c']); - expect([...graph.orphanNodes.keys()]).toEqual(['dx1']); - - expect(JSON.parse(JSON.stringify([...graph.rootNodes.values()]))) - .toMatchInlineSnapshot(` + const graph = buildAppGraph( [ - { - "id": "a", + { ...baseSpec, id: 'a' }, + { ...baseSpec, id: 'b' }, + { ...baseSpec, id: 'c' }, + { ...baseSpec, attachTo: { id: 'b', input: 'x' }, id: 'bx1' }, + { ...baseSpec, attachTo: { id: 'b', input: 'x' }, id: 'bx2' }, + { ...baseSpec, attachTo: { id: 'b', input: 'y' }, id: 'by1' }, + { ...baseSpec, attachTo: { id: 'd', input: 'x' }, id: 'dx1' }, + ], + 'b', + ); + + expect(JSON.parse(JSON.stringify(graph.root))).toMatchInlineSnapshot(` + { + "attachments": { + "x": [ + { + "id": "bx1", + }, + { + "id": "bx2", + }, + ], + "y": [ + { + "id": "by1", + }, + ], }, - { - "attachments": { - "x": [ - { - "id": "bx1", - }, - { - "id": "bx2", - }, - ], - "y": [ - { - "id": "by1", - }, - ], - }, - "id": "b", - }, - { - "id": "c", - }, - ] + "id": "b", + } `); - expect(String(graph.rootNodes.get('a'))).toMatchInlineSnapshot(`""`); - expect(String(graph.rootNodes.get('b'))).toMatchInlineSnapshot(` + expect(String(graph.root)).toMatchInlineSnapshot(` " x [ @@ -89,24 +93,32 @@ describe('buildAppGraph', () => { ] " `); - expect(String(graph.rootNodes.get('c'))).toMatchInlineSnapshot(`""`); + + const orphans = Array.from(graph.orphans).map(String); + expect(orphans).toMatchInlineSnapshot(` + [ + "", + "", + "", + ] + `); }); it('should create a graph out of order', () => { - const graph = buildAppGraph([ - { ...baseSpec, attachTo: { id: 'b', input: 'x' }, id: 'bx2' }, - { ...baseSpec, id: 'a' }, - { ...baseSpec, attachTo: { id: 'b', input: 'y' }, id: 'by1' }, - { ...baseSpec, id: 'b' }, - { ...baseSpec, attachTo: { id: 'b', input: 'x' }, id: 'bx1' }, - { ...baseSpec, id: 'c' }, - { ...baseSpec, attachTo: { id: 'd', input: 'x' }, id: 'dx1' }, - ]); - expect([...graph.rootNodes.keys()]).toEqual(['a', 'b', 'c']); - expect([...graph.orphanNodes.keys()]).toEqual(['dx1']); + const graph = buildAppGraph( + [ + { ...baseSpec, attachTo: { id: 'b', input: 'x' }, id: 'bx2' }, + { ...baseSpec, id: 'a' }, + { ...baseSpec, attachTo: { id: 'b', input: 'y' }, id: 'by1' }, + { ...baseSpec, id: 'b' }, + { ...baseSpec, attachTo: { id: 'b', input: 'x' }, id: 'bx1' }, + { ...baseSpec, id: 'c' }, + { ...baseSpec, attachTo: { id: 'd', input: 'x' }, id: 'dx1' }, + ], + 'b', + ); - expect(String(graph.rootNodes.get('a'))).toMatchInlineSnapshot(`""`); - expect(String(graph.rootNodes.get('b'))).toMatchInlineSnapshot(` + expect(String(graph.root)).toMatchInlineSnapshot(` " x [ @@ -117,7 +129,15 @@ describe('buildAppGraph', () => { ] " `); - expect(String(graph.rootNodes.get('c'))).toMatchInlineSnapshot(`""`); + + const orphans = Array.from(graph.orphans).map(String); + expect(orphans).toMatchInlineSnapshot(` + [ + "", + "", + "", + ] + `); }); it('throws an error when duplicated extensions are detected', () => { diff --git a/packages/frontend-app-api/src/wiring/graph/buildAppGraph.ts b/packages/frontend-app-api/src/wiring/graph/buildAppGraph.ts index 79977f6993..72d4d90d48 100644 --- a/packages/frontend-app-api/src/wiring/graph/buildAppGraph.ts +++ b/packages/frontend-app-api/src/wiring/graph/buildAppGraph.ts @@ -78,9 +78,15 @@ class SerializableAppNode implements AppNode { * tree with all attachments in the same order as they appear in the input specs array. * @internal */ -export function buildAppGraph(specs: AppNodeSpec[]): AppGraph { +export function buildAppGraph( + specs: AppNodeSpec[], + rootNodeId = 'core', +): AppGraph { const nodes = new Map(); - const rootNodes = new Map(); + + // A node with the provided rootNodeId must be found in the graph, and it must not be attached to anything + let rootNode: AppNode | undefined = undefined; + // While iterating through the inputs specs we keep track of all nodes that were created // before their parent, and attach them later when the parent is created. // As we find the parents and attach the children, we remove them from this map. This means @@ -99,7 +105,10 @@ export function buildAppGraph(specs: AppNodeSpec[]): AppGraph { const node = new SerializableAppNode(spec); nodes.set(spec.id, node); - if (spec.attachTo) { + // TODO: For now we simply ignore the attachTo spec of the root node, but it'd be cleaner if we could avoid defining it + if (spec.id === rootNodeId) { + rootNode = node; + } else { const parent = nodes.get(spec.attachTo.id); if (parent) { (node.edges as Mutable).attachedTo = { @@ -123,8 +132,6 @@ export function buildAppGraph(specs: AppNodeSpec[]): AppGraph { orphansByParent.set(spec.attachTo.id, [orphan]); } } - } else { - rootNodes.set(spec.id, node); } const orphanedChildren = orphansByParent.get(spec.id); @@ -142,11 +149,13 @@ export function buildAppGraph(specs: AppNodeSpec[]): AppGraph { } } - const orphanNodes = new Map( - Array.from(orphansByParent).flatMap(([, orphans]) => - orphans.map(({ orphan }) => [orphan.spec.id, orphan]), - ), + const orphanNodes = Array.from(orphansByParent).flatMap(([, orphans]) => + orphans.map(({ orphan }) => orphan), ); - return { rootNodes, orphanNodes }; + if (!rootNode) { + throw new Error(`No root node with id '${rootNodeId}' found in app graph`); + } + + return { root: rootNode, orphans: orphanNodes }; } diff --git a/packages/frontend-app-api/src/wiring/graph/types.ts b/packages/frontend-app-api/src/wiring/graph/types.ts index 29c0028d66..6cf4ade360 100644 --- a/packages/frontend-app-api/src/wiring/graph/types.ts +++ b/packages/frontend-app-api/src/wiring/graph/types.ts @@ -31,7 +31,7 @@ export type Mutable = { */ export interface AppNodeSpec { readonly id: string; - readonly attachTo?: { id: string; input: string }; + readonly attachTo: { id: string; input: string }; readonly extension: Extension; readonly disabled: boolean; readonly config?: unknown; @@ -70,6 +70,6 @@ export interface AppNode { } export interface AppGraph { - rootNodes: Map; - orphanNodes: Map; + root: AppNode; + orphans: Iterable; } From ff33a006a1356f59122534e31b13e113809200ca Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Wed, 18 Oct 2023 11:01:30 +0200 Subject: [PATCH 11/31] frontend-app-api: add node map to app graph Signed-off-by: Patrik Oldsberg --- .../src/wiring/graph/buildAppGraph.test.ts | 21 +++++++++++++++++++ .../src/wiring/graph/buildAppGraph.ts | 2 +- .../src/wiring/graph/types.ts | 1 + 3 files changed, 23 insertions(+), 1 deletion(-) diff --git a/packages/frontend-app-api/src/wiring/graph/buildAppGraph.test.ts b/packages/frontend-app-api/src/wiring/graph/buildAppGraph.test.ts index dc489565f4..8cf8279e96 100644 --- a/packages/frontend-app-api/src/wiring/graph/buildAppGraph.test.ts +++ b/packages/frontend-app-api/src/wiring/graph/buildAppGraph.test.ts @@ -46,6 +46,7 @@ describe('buildAppGraph', () => { edges: { attachments: new Map() }, }); expect(Array.from(graph.orphans)).toEqual([]); + expect(Array.from(graph.nodes.keys())).toEqual(['core']); }); it('should create a graph', () => { @@ -62,6 +63,16 @@ describe('buildAppGraph', () => { 'b', ); + expect(Array.from(graph.nodes.keys())).toEqual([ + 'a', + 'b', + 'c', + 'bx1', + 'bx2', + 'by1', + 'dx1', + ]); + expect(JSON.parse(JSON.stringify(graph.root))).toMatchInlineSnapshot(` { "attachments": { @@ -118,6 +129,16 @@ describe('buildAppGraph', () => { 'b', ); + expect(Array.from(graph.nodes.keys())).toEqual([ + 'bx2', + 'a', + 'by1', + 'b', + 'bx1', + 'c', + 'dx1', + ]); + expect(String(graph.root)).toMatchInlineSnapshot(` " x [ diff --git a/packages/frontend-app-api/src/wiring/graph/buildAppGraph.ts b/packages/frontend-app-api/src/wiring/graph/buildAppGraph.ts index 72d4d90d48..f87d191908 100644 --- a/packages/frontend-app-api/src/wiring/graph/buildAppGraph.ts +++ b/packages/frontend-app-api/src/wiring/graph/buildAppGraph.ts @@ -157,5 +157,5 @@ export function buildAppGraph( throw new Error(`No root node with id '${rootNodeId}' found in app graph`); } - return { root: rootNode, orphans: orphanNodes }; + return { root: rootNode, nodes, orphans: orphanNodes }; } diff --git a/packages/frontend-app-api/src/wiring/graph/types.ts b/packages/frontend-app-api/src/wiring/graph/types.ts index 6cf4ade360..21ea55c780 100644 --- a/packages/frontend-app-api/src/wiring/graph/types.ts +++ b/packages/frontend-app-api/src/wiring/graph/types.ts @@ -71,5 +71,6 @@ export interface AppNode { export interface AppGraph { root: AppNode; + nodes: Map; orphans: Iterable; } From 3230953cc971bfa6da8e19c0b6055c156c495352 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Wed, 18 Oct 2023 11:02:24 +0200 Subject: [PATCH 12/31] frontend-app-api: switch to ReadonlyMap in app graph types Signed-off-by: Patrik Oldsberg --- packages/frontend-app-api/src/wiring/graph/types.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/frontend-app-api/src/wiring/graph/types.ts b/packages/frontend-app-api/src/wiring/graph/types.ts index 21ea55c780..d7f80faa1b 100644 --- a/packages/frontend-app-api/src/wiring/graph/types.ts +++ b/packages/frontend-app-api/src/wiring/graph/types.ts @@ -44,7 +44,7 @@ export interface AppNodeSpec { */ export interface AppNodeEdges { readonly attachedTo?: { node: AppNode; input: string }; - readonly attachments: Map; + readonly attachments: ReadonlyMap; } /** @@ -71,6 +71,6 @@ export interface AppNode { export interface AppGraph { root: AppNode; - nodes: Map; + nodes: ReadonlyMap; orphans: Iterable; } From 1a9acc4ff19b6d53ba6e7ec67d0b50776a4a3928 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Wed, 18 Oct 2023 11:10:26 +0200 Subject: [PATCH 13/31] frontend-app-api: refactor buildAppGraph to avoid mutability casts Signed-off-by: Patrik Oldsberg --- .../src/wiring/graph/buildAppGraph.ts | 71 ++++++++----------- 1 file changed, 31 insertions(+), 40 deletions(-) diff --git a/packages/frontend-app-api/src/wiring/graph/buildAppGraph.ts b/packages/frontend-app-api/src/wiring/graph/buildAppGraph.ts index f87d191908..06347b1ece 100644 --- a/packages/frontend-app-api/src/wiring/graph/buildAppGraph.ts +++ b/packages/frontend-app-api/src/wiring/graph/buildAppGraph.ts @@ -14,14 +14,7 @@ * limitations under the License. */ -import { - AppGraph, - AppNode, - AppNodeEdges, - AppNodeInstance, - AppNodeSpec, - Mutable, -} from './types'; +import { AppGraph, AppNode, AppNodeInstance, AppNodeSpec } from './types'; function indent(str: string) { return str.replace(/^/gm, ' '); @@ -30,13 +23,29 @@ function indent(str: string) { /** @internal */ class SerializableAppNode implements AppNode { public readonly spec: AppNodeSpec; - public readonly edges: AppNodeEdges = { attachments: new Map() }; + public readonly edges = { + attachedTo: undefined as { node: AppNode; input: string } | undefined, + attachments: new Map(), + }; public readonly instance?: AppNodeInstance; constructor(spec: AppNodeSpec) { this.spec = spec; } + setParent(parent: SerializableAppNode) { + const input = this.spec.attachTo.input; + + this.edges.attachedTo = { node: parent, input }; + + const parentInputEdges = parent.edges.attachments.get(input); + if (parentInputEdges) { + parentInputEdges.push(this); + } else { + parent.edges.attachments.set(input, [this]); + } + } + toJSON() { const dataRefs = this.instance && [...this.instance.getDataRefs()]; return { @@ -52,7 +61,7 @@ class SerializableAppNode implements AppNode { }; } - toString() { + toString(): string { const dataRefs = this.instance && [...this.instance.getDataRefs()]; const out = dataRefs && dataRefs.length > 0 @@ -82,7 +91,7 @@ export function buildAppGraph( specs: AppNodeSpec[], rootNodeId = 'core', ): AppGraph { - const nodes = new Map(); + const nodes = new Map(); // A node with the provided rootNodeId must be found in the graph, and it must not be attached to anything let rootNode: AppNode | undefined = undefined; @@ -93,7 +102,7 @@ export function buildAppGraph( // that after iterating through all input specs, this will be a map for each root node. const orphansByParent = new Map< string /* parentId */, - { orphan: AppNode; input: string }[] + SerializableAppNode[] >(); for (const spec of specs) { @@ -111,25 +120,13 @@ export function buildAppGraph( } else { const parent = nodes.get(spec.attachTo.id); if (parent) { - (node.edges as Mutable).attachedTo = { - node: parent, - input: spec.attachTo.input, - }; - const parentInputEdges = parent.edges.attachments.get( - spec.attachTo.input, - ); - if (parentInputEdges) { - parentInputEdges.push(node); - } else { - parent.edges.attachments.set(spec.attachTo.input, [node]); - } + node.setParent(parent); } else { const orphanNodesForParent = orphansByParent.get(spec.attachTo.id); - const orphan = { orphan: node, input: spec.attachTo.input }; if (orphanNodesForParent) { - orphanNodesForParent.push(orphan); + orphanNodesForParent.push(node); } else { - orphansByParent.set(spec.attachTo.id, [orphan]); + orphansByParent.set(spec.attachTo.id, [node]); } } } @@ -137,25 +134,19 @@ export function buildAppGraph( const orphanedChildren = orphansByParent.get(spec.id); if (orphanedChildren) { orphansByParent.delete(spec.id); - for (const { orphan, input } of orphanedChildren) { - (orphan.edges as Mutable).attachedTo = { node, input }; - const attachments = node.edges.attachments.get(input); - if (attachments) { - attachments.push(orphan); - } else { - node.edges.attachments.set(input, [orphan]); - } + for (const orphan of orphanedChildren) { + orphan.setParent(node); } } } - const orphanNodes = Array.from(orphansByParent).flatMap(([, orphans]) => - orphans.map(({ orphan }) => orphan), - ); - if (!rootNode) { throw new Error(`No root node with id '${rootNodeId}' found in app graph`); } - return { root: rootNode, nodes, orphans: orphanNodes }; + return { + root: rootNode, + nodes, + orphans: Array.from(orphansByParent.values()).flat(), + }; } From be2afe8d48597463ae17a86d8d07f75ca76526d4 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Wed, 18 Oct 2023 11:12:01 +0200 Subject: [PATCH 14/31] frontend-app-api: update createAppNodeInstance for ReadonlyMap + remove ExtensionInstance type Signed-off-by: Patrik Oldsberg --- .../src/wiring/graph/createAppNodeInstance.ts | 22 ++----------------- 1 file changed, 2 insertions(+), 20 deletions(-) diff --git a/packages/frontend-app-api/src/wiring/graph/createAppNodeInstance.ts b/packages/frontend-app-api/src/wiring/graph/createAppNodeInstance.ts index 310852cd1c..22cb3d4df7 100644 --- a/packages/frontend-app-api/src/wiring/graph/createAppNodeInstance.ts +++ b/packages/frontend-app-api/src/wiring/graph/createAppNodeInstance.ts @@ -17,7 +17,6 @@ import { AnyExtensionDataMap, AnyExtensionInputMap, - BackstagePlugin, ExtensionDataRef, } from '@backstage/frontend-plugin-api'; import mapValues from 'lodash/mapValues'; @@ -25,23 +24,6 @@ import { AppNode, AppNodeInstance, AppNodeSpec } from './types'; type AppNodeWithInstance = AppNode & { instance: AppNodeInstance }; -/** @internal */ -export interface ExtensionInstance { - readonly $$type: '@backstage/ExtensionInstance'; - - readonly id: string; - /** - * Get concrete value for the given extension data reference. Returns undefined if no value is available. - */ - getData(ref: ExtensionDataRef): T | undefined; - /** - * Maps input names to the actual instances given to them. - */ - readonly attachments: Map; - - readonly source?: BackstagePlugin; -} - function resolveInputData( dataMap: AnyExtensionDataMap, attachment: AppNodeWithInstance, @@ -60,7 +42,7 @@ function resolveInputData( function resolveInputs( inputMap: AnyExtensionInputMap, - attachments: Map, + attachments: ReadonlyMap, ) { const undeclaredAttachments = Array.from(attachments.entries()).filter( ([inputName]) => inputMap[inputName] === undefined, @@ -117,7 +99,7 @@ function resolveInputs( /** @internal */ export function createAppNodeInstance(options: { spec: AppNodeSpec; - attachments: Map; + attachments: ReadonlyMap; }): AppNodeInstance { const { spec, attachments } = options; const { id, extension, config, source } = spec; From 2b1ceb2fe152410fa52a27f8dc5eacf62fbd84be Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Wed, 18 Oct 2023 11:59:44 +0200 Subject: [PATCH 15/31] frontend-app-api: move app node instance creation into instantiateAppNodeTree Signed-off-by: Patrik Oldsberg --- .../src/wiring/graph/createAppNodeInstance.ts | 155 ---------------- .../wiring/graph/instantiateAppNodeTree.ts | 170 ++++++++++++++++-- .../src/wiring/graph/types.ts | 5 - 3 files changed, 159 insertions(+), 171 deletions(-) delete mode 100644 packages/frontend-app-api/src/wiring/graph/createAppNodeInstance.ts diff --git a/packages/frontend-app-api/src/wiring/graph/createAppNodeInstance.ts b/packages/frontend-app-api/src/wiring/graph/createAppNodeInstance.ts deleted file mode 100644 index 22cb3d4df7..0000000000 --- a/packages/frontend-app-api/src/wiring/graph/createAppNodeInstance.ts +++ /dev/null @@ -1,155 +0,0 @@ -/* - * Copyright 2023 The Backstage Authors - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -import { - AnyExtensionDataMap, - AnyExtensionInputMap, - ExtensionDataRef, -} from '@backstage/frontend-plugin-api'; -import mapValues from 'lodash/mapValues'; -import { AppNode, AppNodeInstance, AppNodeSpec } from './types'; - -type AppNodeWithInstance = AppNode & { instance: AppNodeInstance }; - -function resolveInputData( - dataMap: AnyExtensionDataMap, - attachment: AppNodeWithInstance, - inputName: string, -) { - return mapValues(dataMap, ref => { - const value = attachment.instance.getData(ref); - if (value === undefined && !ref.config.optional) { - throw new Error( - `input '${inputName}' did not receive required extension data '${ref.id}' from extension '${attachment.spec.id}'`, - ); - } - return value; - }); -} - -function resolveInputs( - inputMap: AnyExtensionInputMap, - attachments: ReadonlyMap, -) { - const undeclaredAttachments = Array.from(attachments.entries()).filter( - ([inputName]) => inputMap[inputName] === undefined, - ); - // TODO: Make this a warning rather than an error - if (undeclaredAttachments.length > 0) { - throw new Error( - `received undeclared input${ - undeclaredAttachments.length > 1 ? 's' : '' - } ${undeclaredAttachments - .map( - ([k, exts]) => - `'${k}' from extension${exts.length > 1 ? 's' : ''} '${exts - .map(e => e.spec.id) - .join("', '")}'`, - ) - .join(' and ')}`, - ); - } - - return mapValues(inputMap, (input, inputName) => { - const attachedNodes = - attachments - .get(inputName) - ?.filter((node): node is AppNodeWithInstance => - Boolean(node.instance), - ) ?? []; - - if (input.config.singleton) { - if (attachedNodes.length > 1) { - const attachedNodeIds = attachedNodes.map(e => e.spec.id); - throw Error( - `expected ${ - input.config.optional ? 'at most' : 'exactly' - } one '${inputName}' input but received multiple: '${attachedNodeIds.join( - "', '", - )}'`, - ); - } else if (attachedNodes.length === 0) { - if (input.config.optional) { - return undefined; - } - throw Error(`input '${inputName}' is required but was not received`); - } - return resolveInputData(input.extensionData, attachedNodes[0], inputName); - } - - return attachedNodes.map(attachment => - resolveInputData(input.extensionData, attachment, inputName), - ); - }); -} - -/** @internal */ -export function createAppNodeInstance(options: { - spec: AppNodeSpec; - attachments: ReadonlyMap; -}): AppNodeInstance { - const { spec, attachments } = options; - const { id, extension, config, source } = spec; - const extensionData = new Map(); - const extensionDataRefs = new Set>(); - - let parsedConfig: unknown; - try { - parsedConfig = extension.configSchema?.parse(config ?? {}); - } catch (e) { - throw new Error( - `Invalid configuration for extension '${id}'; caused by ${e}`, - ); - } - - try { - extension.factory({ - source, - config: parsedConfig, - bind: namedOutputs => { - for (const [name, output] of Object.entries(namedOutputs)) { - const ref = extension.output[name]; - if (!ref) { - throw new Error(`unknown output provided via '${name}'`); - } - if (extensionData.has(ref.id)) { - throw new Error( - `duplicate extension data '${ref.id}' received via output '${name}'`, - ); - } - extensionData.set(ref.id, output); - extensionDataRefs.add(ref); - } - }, - inputs: resolveInputs(extension.inputs, attachments), - }); - } catch (e) { - throw new Error( - `Failed to instantiate extension '${id}'${ - e.name === 'Error' ? `, ${e.message}` : `; caused by ${e}` - }`, - ); - } - - return { - getDataRefs() { - return extensionDataRefs.values(); - }, - getData(ref: ExtensionDataRef): T | undefined { - return extensionData.get(ref.id) as T | undefined; - }, - }; -} diff --git a/packages/frontend-app-api/src/wiring/graph/instantiateAppNodeTree.ts b/packages/frontend-app-api/src/wiring/graph/instantiateAppNodeTree.ts index b01ad020b9..ba0c85692e 100644 --- a/packages/frontend-app-api/src/wiring/graph/instantiateAppNodeTree.ts +++ b/packages/frontend-app-api/src/wiring/graph/instantiateAppNodeTree.ts @@ -14,30 +14,178 @@ * limitations under the License. */ -import { createAppNodeInstance } from './createAppNodeInstance'; -import { AppNode, Mutable } from './types'; +import { + AnyExtensionDataMap, + AnyExtensionInputMap, + ExtensionDataRef, +} from '@backstage/frontend-plugin-api'; +import mapValues from 'lodash/mapValues'; +import { AppNode, AppNodeInstance, AppNodeSpec } from './types'; +type Mutable = { + -readonly [P in keyof T]: T[P]; +}; + +function resolveInputData( + dataMap: AnyExtensionDataMap, + attachment: { id: string; instance: AppNodeInstance }, + inputName: string, +) { + return mapValues(dataMap, ref => { + const value = attachment.instance.getData(ref); + if (value === undefined && !ref.config.optional) { + throw new Error( + `input '${inputName}' did not receive required extension data '${ref.id}' from extension '${attachment.id}'`, + ); + } + return value; + }); +} + +function resolveInputs( + inputMap: AnyExtensionInputMap, + attachments: ReadonlyMap, +) { + const undeclaredAttachments = Array.from(attachments.entries()).filter( + ([inputName]) => inputMap[inputName] === undefined, + ); + // TODO: Make this a warning rather than an error + if (undeclaredAttachments.length > 0) { + throw new Error( + `received undeclared input${ + undeclaredAttachments.length > 1 ? 's' : '' + } ${undeclaredAttachments + .map( + ([k, exts]) => + `'${k}' from extension${exts.length > 1 ? 's' : ''} '${exts + .map(e => e.id) + .join("', '")}'`, + ) + .join(' and ')}`, + ); + } + + return mapValues(inputMap, (input, inputName) => { + const attachedNodes = attachments.get(inputName) ?? []; + + if (input.config.singleton) { + if (attachedNodes.length > 1) { + const attachedNodeIds = attachedNodes.map(e => e.id); + throw Error( + `expected ${ + input.config.optional ? 'at most' : 'exactly' + } one '${inputName}' input but received multiple: '${attachedNodeIds.join( + "', '", + )}'`, + ); + } else if (attachedNodes.length === 0) { + if (input.config.optional) { + return undefined; + } + throw Error(`input '${inputName}' is required but was not received`); + } + return resolveInputData(input.extensionData, attachedNodes[0], inputName); + } + + return attachedNodes.map(attachment => + resolveInputData(input.extensionData, attachment, inputName), + ); + }); +} + +/** @internal */ +export function createAppNodeInstance(options: { + spec: AppNodeSpec; + attachments: ReadonlyMap; +}): AppNodeInstance { + const { spec, attachments } = options; + const { id, extension, config, source } = spec; + const extensionData = new Map(); + const extensionDataRefs = new Set>(); + + let parsedConfig: unknown; + try { + parsedConfig = extension.configSchema?.parse(config ?? {}); + } catch (e) { + throw new Error( + `Invalid configuration for extension '${id}'; caused by ${e}`, + ); + } + + try { + extension.factory({ + source, + config: parsedConfig, + bind: namedOutputs => { + for (const [name, output] of Object.entries(namedOutputs)) { + const ref = extension.output[name]; + if (!ref) { + throw new Error(`unknown output provided via '${name}'`); + } + if (extensionData.has(ref.id)) { + throw new Error( + `duplicate extension data '${ref.id}' received via output '${name}'`, + ); + } + extensionData.set(ref.id, output); + extensionDataRefs.add(ref); + } + }, + inputs: resolveInputs(extension.inputs, attachments), + }); + } catch (e) { + throw new Error( + `Failed to instantiate extension '${id}'${ + e.name === 'Error' ? `, ${e.message}` : `; caused by ${e}` + }`, + ); + } + + return { + getDataRefs() { + return extensionDataRefs.values(); + }, + getData(ref: ExtensionDataRef): T | undefined { + return extensionData.get(ref.id) as T | undefined; + }, + }; +} + +/** + * Starting at the provided node, instantiate all reachable nodes in the graph that have not been disabled. + * @internal + */ export function instantiateAppNodeTree(rootNode: AppNode): void { - function createInstance(node: AppNode): void { + function createInstance(node: AppNode): AppNodeInstance | undefined { if (node.instance) { - return; + return node.instance; } if (node.spec.disabled) { - return; + return undefined; } - for (const children of node.edges.attachments.values()) { - for (const child of children) { - createInstance(child); - } + const instantiatedAttachments = new Map< + string, + { id: string; instance: AppNodeInstance }[] + >(); + + for (const [input, children] of node.edges.attachments) { + const instantiatedChildren = children.flatMap(child => { + const childInstance = createInstance(child); + if (!childInstance) { + return []; + } + return [{ id: child.spec.id, instance: childInstance }]; + }); + instantiatedAttachments.set(input, instantiatedChildren); } (node as Mutable).instance = createAppNodeInstance({ spec: node.spec, - attachments: node.edges.attachments, + attachments: instantiatedAttachments, }); - return; + return node.instance; } createInstance(rootNode); diff --git a/packages/frontend-app-api/src/wiring/graph/types.ts b/packages/frontend-app-api/src/wiring/graph/types.ts index d7f80faa1b..b828c9762e 100644 --- a/packages/frontend-app-api/src/wiring/graph/types.ts +++ b/packages/frontend-app-api/src/wiring/graph/types.ts @@ -20,11 +20,6 @@ import { ExtensionDataRef, } from '@backstage/frontend-plugin-api'; -/** @internal */ -export type Mutable = { - -readonly [P in keyof T]: T[P]; -}; - /** * The specification for this node in the app graph. * @public From aded5d1ce4a47ed9284e2559f41c6dff27270343 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Wed, 18 Oct 2023 12:29:32 +0200 Subject: [PATCH 16/31] frontend-app-api: migrate createExtensionInstance tests Signed-off-by: Patrik Oldsberg --- .../graph/createExtensionInstance.test.ts | 456 ------------------ .../graph/instantiateAppNodeTree.test.ts | 441 +++++++++++++++++ 2 files changed, 441 insertions(+), 456 deletions(-) delete mode 100644 packages/frontend-app-api/src/wiring/graph/createExtensionInstance.test.ts create mode 100644 packages/frontend-app-api/src/wiring/graph/instantiateAppNodeTree.test.ts diff --git a/packages/frontend-app-api/src/wiring/graph/createExtensionInstance.test.ts b/packages/frontend-app-api/src/wiring/graph/createExtensionInstance.test.ts deleted file mode 100644 index 6e69e99cb7..0000000000 --- a/packages/frontend-app-api/src/wiring/graph/createExtensionInstance.test.ts +++ /dev/null @@ -1,456 +0,0 @@ -/* - * Copyright 2023 The Backstage Authors - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -import { - createExtension, - createExtensionDataRef, - createExtensionInput, - createSchemaFromZod, -} from '@backstage/frontend-plugin-api'; -import { createExtensionInstance } from './createExtensionInstance'; - -const testDataRef = createExtensionDataRef('test'); -const otherDataRef = createExtensionDataRef('other'); -const inputMirrorDataRef = createExtensionDataRef('mirror'); - -const simpleExtension = createExtension({ - id: 'core.test', - attachTo: { id: 'ignored', input: 'ignored' }, - output: { - test: testDataRef, - other: otherDataRef.optional(), - }, - configSchema: createSchemaFromZod(z => - z.object({ - output: z.string().default('test'), - other: z.number().optional(), - }), - ), - factory({ bind, config }) { - bind({ test: config.output, other: config.other }); - }, -}); - -describe('createExtensionInstance', () => { - it('should create a simple extension instance', () => { - const attachments = new Map(); - const instance = createExtensionInstance({ - attachments, - config: undefined, - extension: simpleExtension, - }); - - expect(instance.id).toBe('core.test'); - expect(instance.attachments).toBe(attachments); - expect(instance.getData(testDataRef)).toEqual('test'); - }); - - it('should create an extension with different kind of inputs', () => { - const attachments = new Map([ - [ - 'optionalSingletonPresent', - [ - createExtensionInstance({ - attachments: new Map(), - config: { output: 'optionalSingletonPresent' }, - extension: simpleExtension, - }), - ], - ], - [ - 'singleton', - [ - createExtensionInstance({ - attachments: new Map(), - config: { output: 'singleton', other: 2 }, - extension: simpleExtension, - }), - ], - ], - [ - 'many', - [ - createExtensionInstance({ - attachments: new Map(), - config: { output: 'many1' }, - extension: simpleExtension, - }), - createExtensionInstance({ - attachments: new Map(), - config: { output: 'many2', other: 3 }, - extension: simpleExtension, - }), - ], - ], - ]); - const instance = createExtensionInstance({ - attachments, - config: undefined, - extension: createExtension({ - id: 'core.test', - attachTo: { id: 'ignored', input: 'ignored' }, - inputs: { - optionalSingletonPresent: createExtensionInput( - { - test: testDataRef, - other: otherDataRef.optional(), - }, - { singleton: true, optional: true }, - ), - optionalSingletonMissing: createExtensionInput( - { - test: testDataRef, - other: otherDataRef.optional(), - }, - { singleton: true, optional: true }, - ), - singleton: createExtensionInput( - { - test: testDataRef, - other: otherDataRef.optional(), - }, - { singleton: true }, - ), - many: createExtensionInput({ - test: testDataRef, - other: otherDataRef.optional(), - }), - }, - output: { - inputMirror: inputMirrorDataRef, - }, - factory({ bind, inputs }) { - bind({ inputMirror: inputs }); - }, - }), - }); - - expect(instance.id).toBe('core.test'); - expect(instance.attachments).toBe(attachments); - expect(instance.getData(inputMirrorDataRef)).toEqual({ - optionalSingletonPresent: { test: 'optionalSingletonPresent' }, - singleton: { test: 'singleton', other: 2 }, - many: [{ test: 'many1' }, { test: 'many2', other: 3 }], - }); - }); - - it('should refuse to create an extension with invalid config', () => { - expect(() => - createExtensionInstance({ - attachments: new Map(), - config: { other: 'not-a-number' }, - extension: simpleExtension, - }), - ).toThrow( - "Invalid configuration for extension 'core.test'; caused by Error: Expected number, received string at 'other'", - ); - }); - - it('should forward extension factory errors', () => { - expect(() => - createExtensionInstance({ - attachments: new Map(), - config: { other: 'not-a-number' }, - extension: createExtension({ - id: 'core.test', - attachTo: { id: 'ignored', input: 'ignored' }, - output: {}, - factory() { - const error = new Error('NOPE'); - error.name = 'NopeError'; - throw error; - }, - }), - }), - ).toThrow( - "Failed to instantiate extension 'core.test'; caused by NopeError: NOPE", - ); - }); - - it('should refuse to create an instance with duplicate output', () => { - const attachments = new Map(); - expect(() => - createExtensionInstance({ - attachments, - config: undefined, - extension: createExtension({ - id: 'core.test', - attachTo: { id: 'ignored', input: 'ignored' }, - output: { - test1: testDataRef, - test2: testDataRef, - }, - factory({ bind }) { - bind({ test1: 'test', test2: 'test2' }); - }, - }), - }), - ).toThrow( - "Failed to instantiate extension 'core.test', duplicate extension data 'test' received via output 'test2'", - ); - }); - - it('should refuse to create an instance with disconnected output data', () => { - const attachments = new Map(); - expect(() => - createExtensionInstance({ - attachments, - config: undefined, - extension: createExtension({ - id: 'core.test', - attachTo: { id: 'ignored', input: 'ignored' }, - output: { - test: testDataRef, - }, - factory({ bind }) { - bind({ nonexistent: 'test' } as any); - }, - }), - }), - ).toThrow( - "Failed to instantiate extension 'core.test', unknown output provided via 'nonexistent'", - ); - }); - - it('should refuse to create an instance with missing required input', () => { - expect(() => - createExtensionInstance({ - attachments: new Map(), - config: undefined, - extension: createExtension({ - id: 'core.test', - attachTo: { id: 'ignored', input: 'ignored' }, - inputs: { - singleton: createExtensionInput( - { - test: testDataRef, - }, - { singleton: true }, - ), - }, - output: {}, - factory() {}, - }), - }), - ).toThrow( - "Failed to instantiate extension 'core.test', input 'singleton' is required but was not received", - ); - }); - - it('should refuse to create an instance with undeclared inputs', () => { - expect(() => - createExtensionInstance({ - attachments: new Map([ - [ - 'declared', - [ - createExtensionInstance({ - attachments: new Map(), - config: { output: 'many1' }, - extension: simpleExtension, - }), - ], - ], - [ - 'undeclared', - [ - createExtensionInstance({ - attachments: new Map(), - config: { output: 'many1' }, - extension: simpleExtension, - }), - ], - ], - ]), - config: undefined, - extension: createExtension({ - id: 'core.test', - attachTo: { id: 'ignored', input: 'ignored' }, - inputs: { - declared: createExtensionInput({ - test: testDataRef, - }), - }, - output: {}, - factory() {}, - }), - }), - ).toThrow( - "Failed to instantiate extension 'core.test', received undeclared input 'undeclared' from extension 'core.test'", - ); - }); - - it('should refuse to create an instance with multiple undeclared inputs', () => { - expect(() => - createExtensionInstance({ - attachments: new Map([ - [ - 'undeclared1', - [ - createExtensionInstance({ - attachments: new Map(), - config: { output: 'many1' }, - extension: simpleExtension, - }), - ], - ], - [ - 'undeclared2', - [ - createExtensionInstance({ - attachments: new Map(), - config: { output: 'many1' }, - extension: simpleExtension, - }), - createExtensionInstance({ - attachments: new Map(), - config: { output: 'many1' }, - extension: simpleExtension, - }), - ], - ], - ]), - config: undefined, - extension: createExtension({ - id: 'core.test', - attachTo: { id: 'ignored', input: 'ignored' }, - output: {}, - factory() {}, - }), - }), - ).toThrow( - "Failed to instantiate extension 'core.test', received undeclared inputs 'undeclared1' from extension 'core.test' and 'undeclared2' from extensions 'core.test', 'core.test'", - ); - }); - - it('should refuse to create an instance with multiple inputs for required singleton', () => { - expect(() => - createExtensionInstance({ - attachments: new Map([ - [ - 'singleton', - [ - createExtensionInstance({ - attachments: new Map(), - config: { output: 'many1' }, - extension: simpleExtension, - }), - createExtensionInstance({ - attachments: new Map(), - config: { output: 'many2' }, - extension: simpleExtension, - }), - ], - ], - ]), - config: undefined, - extension: createExtension({ - id: 'core.test', - attachTo: { id: 'ignored', input: 'ignored' }, - inputs: { - singleton: createExtensionInput( - { - test: testDataRef, - }, - { singleton: true }, - ), - }, - output: {}, - factory() {}, - }), - }), - ).toThrow( - "Failed to instantiate extension 'core.test', expected exactly one 'singleton' input but received multiple: 'core.test', 'core.test'", - ); - }); - - it('should refuse to create an instance with multiple inputs for optional singleton', () => { - expect(() => - createExtensionInstance({ - attachments: new Map([ - [ - 'singleton', - [ - createExtensionInstance({ - attachments: new Map(), - config: { output: 'many1' }, - extension: simpleExtension, - }), - createExtensionInstance({ - attachments: new Map(), - config: { output: 'many2' }, - extension: simpleExtension, - }), - ], - ], - ]), - config: undefined, - extension: createExtension({ - id: 'core.test', - attachTo: { id: 'ignored', input: 'ignored' }, - inputs: { - singleton: createExtensionInput( - { - test: testDataRef, - }, - { singleton: true, optional: true }, - ), - }, - output: {}, - factory() {}, - }), - }), - ).toThrow( - "Failed to instantiate extension 'core.test', expected at most one 'singleton' input but received multiple: 'core.test', 'core.test'", - ); - }); - - it('should refuse to create an instance with multiple inputs that did not provide required data', () => { - expect(() => - createExtensionInstance({ - attachments: new Map([ - [ - 'singleton', - [ - createExtensionInstance({ - attachments: new Map(), - config: undefined, - extension: simpleExtension, - }), - ], - ], - ]), - config: undefined, - extension: createExtension({ - id: 'core.test', - attachTo: { id: 'ignored', input: 'ignored' }, - inputs: { - singleton: createExtensionInput( - { - other: otherDataRef, - }, - { singleton: true }, - ), - }, - output: {}, - factory() {}, - }), - }), - ).toThrow( - "Failed to instantiate extension 'core.test', input 'singleton' did not receive required extension data 'other' from extension 'core.test'", - ); - }); -}); diff --git a/packages/frontend-app-api/src/wiring/graph/instantiateAppNodeTree.test.ts b/packages/frontend-app-api/src/wiring/graph/instantiateAppNodeTree.test.ts new file mode 100644 index 0000000000..ddd842c6c0 --- /dev/null +++ b/packages/frontend-app-api/src/wiring/graph/instantiateAppNodeTree.test.ts @@ -0,0 +1,441 @@ +/* + * Copyright 2023 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { + Extension, + createExtension, + createExtensionDataRef, + createExtensionInput, + createSchemaFromZod, +} from '@backstage/frontend-plugin-api'; +import { createAppNodeInstance } from './instantiateAppNodeTree'; +import { AppNodeInstance, AppNodeSpec } from './types'; + +const testDataRef = createExtensionDataRef('test'); +const otherDataRef = createExtensionDataRef('other'); +const inputMirrorDataRef = createExtensionDataRef('mirror'); + +const simpleExtension = createExtension({ + id: 'core.test', + attachTo: { id: 'ignored', input: 'ignored' }, + output: { + test: testDataRef, + other: otherDataRef.optional(), + }, + configSchema: createSchemaFromZod(z => + z.object({ + output: z.string().default('test'), + other: z.number().optional(), + }), + ), + factory({ bind, config }) { + bind({ test: config.output, other: config.other }); + }, +}); + +function makeSpec( + extension: Extension, + config?: TConfig, +): AppNodeSpec { + return { + id: extension.id, + attachTo: extension.attachTo, + disabled: extension.disabled, + extension, + config, + source: undefined, + }; +} + +function makeInstanceWithId( + extension: Extension, + config?: TConfig, +): { id: string; instance: AppNodeInstance } { + return { + id: extension.id, + instance: createAppNodeInstance({ + spec: makeSpec(extension, config), + attachments: new Map(), + }), + }; +} + +describe('createAppNodeInstance', () => { + it('should create a simple extension instance', () => { + const attachments = new Map(); + const instance = createAppNodeInstance({ + spec: makeSpec(simpleExtension), + attachments, + }); + + expect(Array.from(instance.getDataRefs())).toEqual([ + testDataRef, + otherDataRef.optional(), + ]); + expect(instance.getData(testDataRef)).toEqual('test'); + }); + + it('should create an extension with different kind of inputs', () => { + const attachments = new Map([ + [ + 'optionalSingletonPresent', + [ + makeInstanceWithId(simpleExtension, { + output: 'optionalSingletonPresent', + }), + ], + ], + [ + 'singleton', + [ + makeInstanceWithId(simpleExtension, { + output: 'singleton', + other: 2, + }), + ], + ], + [ + 'many', + [ + makeInstanceWithId(simpleExtension, { output: 'many1' }), + makeInstanceWithId(simpleExtension, { output: 'many2', other: 3 }), + ], + ], + ]); + const instance = createAppNodeInstance({ + attachments, + spec: makeSpec( + createExtension({ + id: 'core.test', + attachTo: { id: 'ignored', input: 'ignored' }, + inputs: { + optionalSingletonPresent: createExtensionInput( + { + test: testDataRef, + other: otherDataRef.optional(), + }, + { singleton: true, optional: true }, + ), + optionalSingletonMissing: createExtensionInput( + { + test: testDataRef, + other: otherDataRef.optional(), + }, + { singleton: true, optional: true }, + ), + singleton: createExtensionInput( + { + test: testDataRef, + other: otherDataRef.optional(), + }, + { singleton: true }, + ), + many: createExtensionInput({ + test: testDataRef, + other: otherDataRef.optional(), + }), + }, + output: { + inputMirror: inputMirrorDataRef, + }, + factory({ bind, inputs }) { + bind({ inputMirror: inputs }); + }, + }), + ), + }); + + expect(Array.from(instance.getDataRefs())).toEqual([inputMirrorDataRef]); + expect(instance.getData(inputMirrorDataRef)).toEqual({ + optionalSingletonPresent: { test: 'optionalSingletonPresent' }, + singleton: { test: 'singleton', other: 2 }, + many: [{ test: 'many1' }, { test: 'many2', other: 3 }], + }); + }); + + it('should refuse to create an extension with invalid config', () => { + expect(() => + createAppNodeInstance({ + spec: { + ...makeSpec(simpleExtension), + config: { other: 'not-a-number' }, + }, + attachments: new Map(), + }), + ).toThrow( + "Invalid configuration for extension 'core.test'; caused by Error: Expected number, received string at 'other'", + ); + }); + + it('should forward extension factory errors', () => { + expect(() => + createAppNodeInstance({ + spec: makeSpec( + createExtension({ + id: 'core.test', + attachTo: { id: 'ignored', input: 'ignored' }, + output: {}, + factory() { + const error = new Error('NOPE'); + error.name = 'NopeError'; + throw error; + }, + }), + ), + attachments: new Map(), + }), + ).toThrow( + "Failed to instantiate extension 'core.test'; caused by NopeError: NOPE", + ); + }); + + it('should refuse to create an instance with duplicate output', () => { + expect(() => + createAppNodeInstance({ + spec: makeSpec( + createExtension({ + id: 'core.test', + attachTo: { id: 'ignored', input: 'ignored' }, + output: { + test1: testDataRef, + test2: testDataRef, + }, + factory({ bind }) { + bind({ test1: 'test', test2: 'test2' }); + }, + }), + ), + attachments: new Map(), + }), + ).toThrow( + "Failed to instantiate extension 'core.test', duplicate extension data 'test' received via output 'test2'", + ); + }); + + it('should refuse to create an instance with disconnected output data', () => { + expect(() => + createAppNodeInstance({ + spec: makeSpec( + createExtension({ + id: 'core.test', + attachTo: { id: 'ignored', input: 'ignored' }, + output: { + test: testDataRef, + }, + factory({ bind }) { + bind({ nonexistent: 'test' } as any); + }, + }), + ), + attachments: new Map(), + }), + ).toThrow( + "Failed to instantiate extension 'core.test', unknown output provided via 'nonexistent'", + ); + }); + + it('should refuse to create an instance with missing required input', () => { + expect(() => + createAppNodeInstance({ + spec: makeSpec( + createExtension({ + id: 'core.test', + attachTo: { id: 'ignored', input: 'ignored' }, + inputs: { + singleton: createExtensionInput( + { + test: testDataRef, + }, + { singleton: true }, + ), + }, + output: {}, + factory() {}, + }), + ), + attachments: new Map(), + }), + ).toThrow( + "Failed to instantiate extension 'core.test', input 'singleton' is required but was not received", + ); + }); + + it('should refuse to create an instance with undeclared inputs', () => { + expect(() => + createAppNodeInstance({ + attachments: new Map([ + [ + 'declared', + [ + makeInstanceWithId(simpleExtension, { + output: 'many1', + }), + ], + ], + [ + 'undeclared', + [ + makeInstanceWithId(simpleExtension, { + output: 'many1', + }), + ], + ], + ]), + spec: makeSpec( + createExtension({ + id: 'core.test', + attachTo: { id: 'ignored', input: 'ignored' }, + inputs: { + declared: createExtensionInput({ + test: testDataRef, + }), + }, + output: {}, + factory() {}, + }), + ), + }), + ).toThrow( + "Failed to instantiate extension 'core.test', received undeclared input 'undeclared' from extension 'core.test'", + ); + }); + + it('should refuse to create an instance with multiple undeclared inputs', () => { + expect(() => + createAppNodeInstance({ + attachments: new Map([ + [ + 'undeclared1', + [makeInstanceWithId(simpleExtension, { output: 'many1' })], + ], + [ + 'undeclared2', + [ + makeInstanceWithId(simpleExtension, { output: 'many1' }), + makeInstanceWithId(simpleExtension, { output: 'many1' }), + ], + ], + ]), + spec: makeSpec( + createExtension({ + id: 'core.test', + attachTo: { id: 'ignored', input: 'ignored' }, + output: {}, + factory() {}, + }), + ), + }), + ).toThrow( + "Failed to instantiate extension 'core.test', received undeclared inputs 'undeclared1' from extension 'core.test' and 'undeclared2' from extensions 'core.test', 'core.test'", + ); + }); + + it('should refuse to create an instance with multiple inputs for required singleton', () => { + expect(() => + createAppNodeInstance({ + attachments: new Map([ + [ + 'singleton', + [ + makeInstanceWithId(simpleExtension, { output: 'many1' }), + makeInstanceWithId(simpleExtension, { output: 'many2' }), + ], + ], + ]), + spec: makeSpec( + createExtension({ + id: 'core.test', + attachTo: { id: 'ignored', input: 'ignored' }, + inputs: { + singleton: createExtensionInput( + { + test: testDataRef, + }, + { singleton: true }, + ), + }, + output: {}, + factory() {}, + }), + ), + }), + ).toThrow( + "Failed to instantiate extension 'core.test', expected exactly one 'singleton' input but received multiple: 'core.test', 'core.test'", + ); + }); + + it('should refuse to create an instance with multiple inputs for optional singleton', () => { + expect(() => + createAppNodeInstance({ + attachments: new Map([ + [ + 'singleton', + [ + makeInstanceWithId(simpleExtension, { output: 'many1' }), + makeInstanceWithId(simpleExtension, { output: 'many2' }), + ], + ], + ]), + spec: makeSpec( + createExtension({ + id: 'core.test', + attachTo: { id: 'ignored', input: 'ignored' }, + inputs: { + singleton: createExtensionInput( + { + test: testDataRef, + }, + { singleton: true, optional: true }, + ), + }, + output: {}, + factory() {}, + }), + ), + }), + ).toThrow( + "Failed to instantiate extension 'core.test', expected at most one 'singleton' input but received multiple: 'core.test', 'core.test'", + ); + }); + + it('should refuse to create an instance with multiple inputs that did not provide required data', () => { + expect(() => + createAppNodeInstance({ + attachments: new Map([ + ['singleton', [makeInstanceWithId(simpleExtension, undefined)]], + ]), + spec: makeSpec( + createExtension({ + id: 'core.test', + attachTo: { id: 'ignored', input: 'ignored' }, + inputs: { + singleton: createExtensionInput( + { + other: otherDataRef, + }, + { singleton: true }, + ), + }, + output: {}, + factory() {}, + }), + ), + }), + ).toThrow( + "Failed to instantiate extension 'core.test', input 'singleton' did not receive required extension data 'other' from extension 'core.test'", + ); + }); +}); From d844ec8a0be4c6d67c06110f18e12b11f503c594 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Wed, 18 Oct 2023 12:52:17 +0200 Subject: [PATCH 17/31] frontend-app-api: add tests for instantiateAppNodeTree Signed-off-by: Patrik Oldsberg --- .../graph/instantiateAppNodeTree.test.ts | 123 +++++++++++++++++- 1 file changed, 122 insertions(+), 1 deletion(-) diff --git a/packages/frontend-app-api/src/wiring/graph/instantiateAppNodeTree.test.ts b/packages/frontend-app-api/src/wiring/graph/instantiateAppNodeTree.test.ts index ddd842c6c0..43aad4f9ae 100644 --- a/packages/frontend-app-api/src/wiring/graph/instantiateAppNodeTree.test.ts +++ b/packages/frontend-app-api/src/wiring/graph/instantiateAppNodeTree.test.ts @@ -21,8 +21,12 @@ import { createExtensionInput, createSchemaFromZod, } from '@backstage/frontend-plugin-api'; -import { createAppNodeInstance } from './instantiateAppNodeTree'; +import { + createAppNodeInstance, + instantiateAppNodeTree, +} from './instantiateAppNodeTree'; import { AppNodeInstance, AppNodeSpec } from './types'; +import { buildAppGraph } from './buildAppGraph'; const testDataRef = createExtensionDataRef('test'); const otherDataRef = createExtensionDataRef('other'); @@ -73,6 +77,123 @@ function makeInstanceWithId( }; } +describe('instantiateAppNodeTree', () => { + it('should instantiate a single node', () => { + const graph = buildAppGraph( + [{ ...makeSpec(simpleExtension), id: 'root-node' }], + 'root-node', + ); + expect(graph.root.instance).not.toBeDefined(); + instantiateAppNodeTree(graph.root); + expect(graph.root.instance).toBeDefined(); + expect(graph.root.instance?.getData(testDataRef)).toBe('test'); + + // Multiple calls should have no effect + instantiateAppNodeTree(graph.root); + expect(graph.root.instance).toBeDefined(); + }); + + it('should not instantiate disabled nodes', () => { + const graph = buildAppGraph( + [{ ...makeSpec(simpleExtension), id: 'root-node', disabled: true }], + 'root-node', + ); + expect(graph.root.instance).not.toBeDefined(); + instantiateAppNodeTree(graph.root); + expect(graph.root.instance).not.toBeDefined(); + }); + + it('should instantiate a node with attachments', () => { + const graph = buildAppGraph( + [ + { + ...makeSpec( + createExtension({ + id: 'root-node', + attachTo: { id: 'ignored', input: 'ignored' }, + inputs: { + test: createExtensionInput({ test: testDataRef }), + }, + output: { + inputMirror: inputMirrorDataRef, + }, + factory({ bind, inputs }) { + bind({ inputMirror: inputs }); + }, + }), + ), + }, + { + ...makeSpec(simpleExtension), + id: 'child-node', + attachTo: { id: 'root-node', input: 'test' }, + }, + ], + 'root-node', + ); + + const childNode = graph.nodes.get('child-node'); + expect(childNode).toBeDefined(); + + expect(graph.root.instance).not.toBeDefined(); + expect(childNode?.instance).not.toBeDefined(); + instantiateAppNodeTree(graph.root); + expect(graph.root.instance).toBeDefined(); + expect(childNode?.instance).toBeDefined(); + expect(graph.root.instance?.getData(inputMirrorDataRef)).toEqual({ + test: [{ test: 'test' }], + }); + + // Multiple calls should have no effect + instantiateAppNodeTree(graph.root); + expect(graph.root.instance).toBeDefined(); + expect(childNode?.instance).toBeDefined(); + }); + + it('should not instantiate disabled attachments', () => { + const graph = buildAppGraph( + [ + { + ...makeSpec( + createExtension({ + id: 'root-node', + attachTo: { id: 'ignored', input: 'ignored' }, + inputs: { + test: createExtensionInput({ test: testDataRef }), + }, + output: { + inputMirror: inputMirrorDataRef, + }, + factory({ bind, inputs }) { + bind({ inputMirror: inputs }); + }, + }), + ), + }, + { + ...makeSpec(simpleExtension), + id: 'child-node', + attachTo: { id: 'root-node', input: 'test' }, + disabled: true, + }, + ], + 'root-node', + ); + + const childNode = graph.nodes.get('child-node'); + expect(childNode).toBeDefined(); + + expect(graph.root.instance).not.toBeDefined(); + expect(childNode?.instance).not.toBeDefined(); + instantiateAppNodeTree(graph.root); + expect(graph.root.instance).toBeDefined(); + expect(childNode?.instance).not.toBeDefined(); + expect(graph.root.instance?.getData(inputMirrorDataRef)).toEqual({ + test: [], + }); + }); +}); + describe('createAppNodeInstance', () => { it('should create a simple extension instance', () => { const attachments = new Map(); From 65b6a918944c7120b7ae69c218002b0e6699f6c5 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Wed, 18 Oct 2023 13:02:12 +0200 Subject: [PATCH 18/31] frontend-app-api: rename buildAppGraph -> resolveAppGraph Signed-off-by: Patrik Oldsberg --- .../src/wiring/graph/instantiateAppNodeTree.test.ts | 10 +++++----- ...buildAppGraph.test.ts => resolveAppGraph.test.ts} | 12 ++++++------ .../graph/{buildAppGraph.ts => resolveAppGraph.ts} | 2 +- 3 files changed, 12 insertions(+), 12 deletions(-) rename packages/frontend-app-api/src/wiring/graph/{buildAppGraph.test.ts => resolveAppGraph.test.ts} (93%) rename packages/frontend-app-api/src/wiring/graph/{buildAppGraph.ts => resolveAppGraph.ts} (99%) diff --git a/packages/frontend-app-api/src/wiring/graph/instantiateAppNodeTree.test.ts b/packages/frontend-app-api/src/wiring/graph/instantiateAppNodeTree.test.ts index 43aad4f9ae..7bffc69c7c 100644 --- a/packages/frontend-app-api/src/wiring/graph/instantiateAppNodeTree.test.ts +++ b/packages/frontend-app-api/src/wiring/graph/instantiateAppNodeTree.test.ts @@ -26,7 +26,7 @@ import { instantiateAppNodeTree, } from './instantiateAppNodeTree'; import { AppNodeInstance, AppNodeSpec } from './types'; -import { buildAppGraph } from './buildAppGraph'; +import { resolveAppGraph } from './resolveAppGraph'; const testDataRef = createExtensionDataRef('test'); const otherDataRef = createExtensionDataRef('other'); @@ -79,7 +79,7 @@ function makeInstanceWithId( describe('instantiateAppNodeTree', () => { it('should instantiate a single node', () => { - const graph = buildAppGraph( + const graph = resolveAppGraph( [{ ...makeSpec(simpleExtension), id: 'root-node' }], 'root-node', ); @@ -94,7 +94,7 @@ describe('instantiateAppNodeTree', () => { }); it('should not instantiate disabled nodes', () => { - const graph = buildAppGraph( + const graph = resolveAppGraph( [{ ...makeSpec(simpleExtension), id: 'root-node', disabled: true }], 'root-node', ); @@ -104,7 +104,7 @@ describe('instantiateAppNodeTree', () => { }); it('should instantiate a node with attachments', () => { - const graph = buildAppGraph( + const graph = resolveAppGraph( [ { ...makeSpec( @@ -151,7 +151,7 @@ describe('instantiateAppNodeTree', () => { }); it('should not instantiate disabled attachments', () => { - const graph = buildAppGraph( + const graph = resolveAppGraph( [ { ...makeSpec( diff --git a/packages/frontend-app-api/src/wiring/graph/buildAppGraph.test.ts b/packages/frontend-app-api/src/wiring/graph/resolveAppGraph.test.ts similarity index 93% rename from packages/frontend-app-api/src/wiring/graph/buildAppGraph.test.ts rename to packages/frontend-app-api/src/wiring/graph/resolveAppGraph.test.ts index 8cf8279e96..2d1acbbc04 100644 --- a/packages/frontend-app-api/src/wiring/graph/buildAppGraph.test.ts +++ b/packages/frontend-app-api/src/wiring/graph/resolveAppGraph.test.ts @@ -15,7 +15,7 @@ */ import { createExtension } from '@backstage/frontend-plugin-api'; -import { buildAppGraph } from './buildAppGraph'; +import { resolveAppGraph } from './resolveAppGraph'; const extBaseConfig = { id: 'test', @@ -34,13 +34,13 @@ const baseSpec = { describe('buildAppGraph', () => { it('should fail to create an empty graph', () => { - expect(() => buildAppGraph([])).toThrow( + expect(() => resolveAppGraph([])).toThrow( "No root node with id 'core' found in app graph", ); }); it('should create a graph with only one node', () => { - const graph = buildAppGraph([{ ...baseSpec, id: 'core' }]); + const graph = resolveAppGraph([{ ...baseSpec, id: 'core' }]); expect(graph.root).toEqual({ spec: { ...baseSpec, id: 'core' }, edges: { attachments: new Map() }, @@ -50,7 +50,7 @@ describe('buildAppGraph', () => { }); it('should create a graph', () => { - const graph = buildAppGraph( + const graph = resolveAppGraph( [ { ...baseSpec, id: 'a' }, { ...baseSpec, id: 'b' }, @@ -116,7 +116,7 @@ describe('buildAppGraph', () => { }); it('should create a graph out of order', () => { - const graph = buildAppGraph( + const graph = resolveAppGraph( [ { ...baseSpec, attachTo: { id: 'b', input: 'x' }, id: 'bx2' }, { ...baseSpec, id: 'a' }, @@ -163,7 +163,7 @@ describe('buildAppGraph', () => { it('throws an error when duplicated extensions are detected', () => { expect(() => - buildAppGraph([ + resolveAppGraph([ { ...baseSpec, id: 'a' }, { ...baseSpec, id: 'a' }, ]), diff --git a/packages/frontend-app-api/src/wiring/graph/buildAppGraph.ts b/packages/frontend-app-api/src/wiring/graph/resolveAppGraph.ts similarity index 99% rename from packages/frontend-app-api/src/wiring/graph/buildAppGraph.ts rename to packages/frontend-app-api/src/wiring/graph/resolveAppGraph.ts index 06347b1ece..18f481fa30 100644 --- a/packages/frontend-app-api/src/wiring/graph/buildAppGraph.ts +++ b/packages/frontend-app-api/src/wiring/graph/resolveAppGraph.ts @@ -87,7 +87,7 @@ class SerializableAppNode implements AppNode { * tree with all attachments in the same order as they appear in the input specs array. * @internal */ -export function buildAppGraph( +export function resolveAppGraph( specs: AppNodeSpec[], rootNodeId = 'core', ): AppGraph { From 90b9ef567d88a2127568a39a6d4303f06dadb142 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Wed, 18 Oct 2023 13:05:48 +0200 Subject: [PATCH 19/31] frontend-app-api: add createAppGraph Signed-off-by: Patrik Oldsberg --- .../src/wiring/graph/createAppGraph.ts | 47 +++++++++++++++++++ .../src/wiring/graph/index.ts | 1 + 2 files changed, 48 insertions(+) create mode 100644 packages/frontend-app-api/src/wiring/graph/createAppGraph.ts diff --git a/packages/frontend-app-api/src/wiring/graph/createAppGraph.ts b/packages/frontend-app-api/src/wiring/graph/createAppGraph.ts new file mode 100644 index 0000000000..4c08b7c5b4 --- /dev/null +++ b/packages/frontend-app-api/src/wiring/graph/createAppGraph.ts @@ -0,0 +1,47 @@ +/* + * Copyright 2023 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { + BackstagePlugin, + Extension, + ExtensionOverrides, +} from '@backstage/frontend-plugin-api'; +import { readAppExtensionsConfig } from './readAppExtensionsConfig'; +import { resolveAppGraph } from './resolveAppGraph'; +import { resolveAppNodeSpecs } from './resolveAppNodeSpecs'; +import { AppGraph } from './types'; +import { Config } from '@backstage/config'; +import { instantiateAppNodeTree } from './instantiateAppNodeTree'; + +/** @internal */ +export interface CreateAppGraphOptions { + features: (BackstagePlugin | ExtensionOverrides)[]; + builtinExtensions: Extension[]; + config: Config; +} + +/** @internal */ +export function createAppGraph(options: CreateAppGraphOptions): AppGraph { + const appGraph = resolveAppGraph( + resolveAppNodeSpecs({ + features: options.features, + builtinExtensions: options.builtinExtensions, + parameters: readAppExtensionsConfig(options.config), + }), + ); + instantiateAppNodeTree(appGraph.root); + return appGraph; +} diff --git a/packages/frontend-app-api/src/wiring/graph/index.ts b/packages/frontend-app-api/src/wiring/graph/index.ts index 608ecb7675..4a3b54dffb 100644 --- a/packages/frontend-app-api/src/wiring/graph/index.ts +++ b/packages/frontend-app-api/src/wiring/graph/index.ts @@ -20,3 +20,4 @@ export type { AppNodeInstance, AppNodeSpec, } from './types'; +export { createAppGraph } from './createAppGraph'; From eb7c037e6fb93a782a367fb3d48e1ac45007328e Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Wed, 18 Oct 2023 13:07:15 +0200 Subject: [PATCH 20/31] frontend-app-api: fix AppNodeInstance.getData return type Signed-off-by: Patrik Oldsberg --- packages/frontend-app-api/src/wiring/graph/types.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/frontend-app-api/src/wiring/graph/types.ts b/packages/frontend-app-api/src/wiring/graph/types.ts index b828c9762e..6e205db9dd 100644 --- a/packages/frontend-app-api/src/wiring/graph/types.ts +++ b/packages/frontend-app-api/src/wiring/graph/types.ts @@ -48,7 +48,7 @@ export interface AppNodeEdges { */ export interface AppNodeInstance { getDataRefs(): Iterable>; - getData(ref: ExtensionDataRef): T | unknown; + getData(ref: ExtensionDataRef): T | undefined; } /** From fd6dd7c079ad4f82377e3cf3a27c56a35c50c285 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Wed, 18 Oct 2023 13:13:08 +0200 Subject: [PATCH 21/31] frontend-app-api: migrate extractRouteInfoFromInstanceTree to app graph Signed-off-by: Patrik Oldsberg --- .../extractRouteInfoFromInstanceTree.test.ts | 11 ++++++++--- .../extractRouteInfoFromInstanceTree.ts | 18 +++++++++--------- 2 files changed, 17 insertions(+), 12 deletions(-) diff --git a/packages/frontend-app-api/src/routing/extractRouteInfoFromInstanceTree.test.ts b/packages/frontend-app-api/src/routing/extractRouteInfoFromInstanceTree.test.ts index 0de33b50f2..d990d998b9 100644 --- a/packages/frontend-app-api/src/routing/extractRouteInfoFromInstanceTree.test.ts +++ b/packages/frontend-app-api/src/routing/extractRouteInfoFromInstanceTree.test.ts @@ -27,8 +27,12 @@ import { createPlugin, createRouteRef, } from '@backstage/frontend-plugin-api'; -import { createInstances } from '../wiring/createApp'; import { MockConfigApi } from '@backstage/test-utils'; +import { createAppGraph } from '../wiring/graph'; +import { Core } from '../extensions/Core'; +import { CoreRoutes } from '../extensions/CoreRoutes'; +import { CoreNav } from '../extensions/CoreNav'; +import { CoreLayout } from '../extensions/CoreLayout'; const ref1 = createRouteRef(); const ref2 = createRouteRef(); @@ -73,12 +77,13 @@ function routeInfoFromExtensions(extensions: Extension[]) { id: 'test', extensions, }); - const { coreInstance } = createInstances({ + const graph = createAppGraph({ config: new MockConfigApi({}), + builtinExtensions: [Core, CoreRoutes, CoreNav, CoreLayout], features: [plugin], }); - return extractRouteInfoFromInstanceTree(coreInstance); + return extractRouteInfoFromInstanceTree(graph.root); } function sortedEntries(map: Map): [RouteRef, T][] { diff --git a/packages/frontend-app-api/src/routing/extractRouteInfoFromInstanceTree.ts b/packages/frontend-app-api/src/routing/extractRouteInfoFromInstanceTree.ts index fadcb719b9..16e0e7d8ec 100644 --- a/packages/frontend-app-api/src/routing/extractRouteInfoFromInstanceTree.ts +++ b/packages/frontend-app-api/src/routing/extractRouteInfoFromInstanceTree.ts @@ -15,10 +15,10 @@ */ import { RouteRef, coreExtensionData } from '@backstage/frontend-plugin-api'; -import { ExtensionInstance } from '../wiring/graph/createExtensionInstance'; // eslint-disable-next-line @backstage/no-relative-monorepo-imports import { toLegacyPlugin } from '../wiring/createApp'; import { BackstageRouteObject } from './types'; +import { AppNode } from '../wiring/graph'; // We always add a child that matches all subroutes but without any route refs. This makes // sure that we're always able to match each route no matter how deep the navigation goes. @@ -41,7 +41,7 @@ export function joinPaths(...paths: string[]): string { return normalized; } -export function extractRouteInfoFromInstanceTree(core: ExtensionInstance): { +export function extractRouteInfoFromInstanceTree(core: AppNode): { routePaths: Map; routeParents: Map; routeObjects: BackstageRouteObject[]; @@ -56,17 +56,17 @@ export function extractRouteInfoFromInstanceTree(core: ExtensionInstance): { const routeObjects = new Array(); function visit( - current: ExtensionInstance, + current: AppNode, collectedPath?: string, foundRefForCollectedPath: boolean = false, parentRef?: RouteRef, candidateParentRef?: RouteRef, parentObj?: BackstageRouteObject, ) { - const routePath = current - .getData(coreExtensionData.routePath) + const routePath = current.instance + ?.getData(coreExtensionData.routePath) ?.replace(/^\//, ''); - const routeRef = current.getData(coreExtensionData.routeRef); + const routeRef = current.instance?.getData(coreExtensionData.routeRef); const parentChildren = parentObj?.children ?? routeObjects; let currentObj = parentObj; @@ -124,12 +124,12 @@ export function extractRouteInfoFromInstanceTree(core: ExtensionInstance): { routeParents.set(routeRef, newParentRef); currentObj?.routeRefs.add(routeRef); - if (current.source) { - currentObj?.plugins.add(toLegacyPlugin(current.source)); + if (current.spec.source) { + currentObj?.plugins.add(toLegacyPlugin(current.spec.source)); } } - for (const children of current.attachments.values()) { + for (const children of current.edges.attachments.values()) { for (const child of children) { visit( child, From ea262277488df2e3b5b27dea9bfe42948176b381 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Wed, 18 Oct 2023 13:17:09 +0200 Subject: [PATCH 22/31] frontend-app-api: rename extractRouteInfoFromInstanceTree -> extractRouteInfoFromAppNode Signed-off-by: Patrik Oldsberg --- packages/frontend-app-api/src/routing/RouteResolver.test.ts | 2 +- ...stanceTree.test.ts => extractRouteInfoFromAppNode.test.ts} | 4 ++-- ...InfoFromInstanceTree.ts => extractRouteInfoFromAppNode.ts} | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) rename packages/frontend-app-api/src/routing/{extractRouteInfoFromInstanceTree.test.ts => extractRouteInfoFromAppNode.test.ts} (98%) rename packages/frontend-app-api/src/routing/{extractRouteInfoFromInstanceTree.ts => extractRouteInfoFromAppNode.ts} (98%) diff --git a/packages/frontend-app-api/src/routing/RouteResolver.test.ts b/packages/frontend-app-api/src/routing/RouteResolver.test.ts index e37ee14929..81e9d10be4 100644 --- a/packages/frontend-app-api/src/routing/RouteResolver.test.ts +++ b/packages/frontend-app-api/src/routing/RouteResolver.test.ts @@ -24,7 +24,7 @@ import { } from '@backstage/frontend-plugin-api'; import { BackstagePlugin } from '@backstage/core-plugin-api'; import { RouteResolver } from './RouteResolver'; -import { MATCH_ALL_ROUTE } from './extractRouteInfoFromInstanceTree'; +import { MATCH_ALL_ROUTE } from './extractRouteInfoFromAppNode'; const rest = { element: null, diff --git a/packages/frontend-app-api/src/routing/extractRouteInfoFromInstanceTree.test.ts b/packages/frontend-app-api/src/routing/extractRouteInfoFromAppNode.test.ts similarity index 98% rename from packages/frontend-app-api/src/routing/extractRouteInfoFromInstanceTree.test.ts rename to packages/frontend-app-api/src/routing/extractRouteInfoFromAppNode.test.ts index d990d998b9..2eab3c8e7a 100644 --- a/packages/frontend-app-api/src/routing/extractRouteInfoFromInstanceTree.test.ts +++ b/packages/frontend-app-api/src/routing/extractRouteInfoFromAppNode.test.ts @@ -16,7 +16,7 @@ import React from 'react'; import { BackstagePlugin } from '@backstage/core-plugin-api'; -import { extractRouteInfoFromInstanceTree } from './extractRouteInfoFromInstanceTree'; +import { extractRouteInfoFromAppNode } from './extractRouteInfoFromAppNode'; import { AnyRouteRefParams, Extension, @@ -83,7 +83,7 @@ function routeInfoFromExtensions(extensions: Extension[]) { features: [plugin], }); - return extractRouteInfoFromInstanceTree(graph.root); + return extractRouteInfoFromAppNode(graph.root); } function sortedEntries(map: Map): [RouteRef, T][] { diff --git a/packages/frontend-app-api/src/routing/extractRouteInfoFromInstanceTree.ts b/packages/frontend-app-api/src/routing/extractRouteInfoFromAppNode.ts similarity index 98% rename from packages/frontend-app-api/src/routing/extractRouteInfoFromInstanceTree.ts rename to packages/frontend-app-api/src/routing/extractRouteInfoFromAppNode.ts index 16e0e7d8ec..1ecb28113b 100644 --- a/packages/frontend-app-api/src/routing/extractRouteInfoFromInstanceTree.ts +++ b/packages/frontend-app-api/src/routing/extractRouteInfoFromAppNode.ts @@ -41,7 +41,7 @@ export function joinPaths(...paths: string[]): string { return normalized; } -export function extractRouteInfoFromInstanceTree(core: AppNode): { +export function extractRouteInfoFromAppNode(node: AppNode): { routePaths: Map; routeParents: Map; routeObjects: BackstageRouteObject[]; @@ -143,7 +143,7 @@ export function extractRouteInfoFromInstanceTree(core: AppNode): { } } - visit(core); + visit(node); return { routePaths, routeParents, routeObjects }; } From fcf1b80beda4edbcffc1b906b50d69e47f9eb219 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Wed, 18 Oct 2023 13:26:56 +0200 Subject: [PATCH 23/31] frontend-app-api: migrate createApp to use createAppGraph Signed-off-by: Patrik Oldsberg --- .../frontend-app-api/src/wiring/createApp.tsx | 150 +++++------------- 1 file changed, 44 insertions(+), 106 deletions(-) diff --git a/packages/frontend-app-api/src/wiring/createApp.tsx b/packages/frontend-app-api/src/wiring/createApp.tsx index 485882c8eb..1dbfb3912b 100644 --- a/packages/frontend-app-api/src/wiring/createApp.tsx +++ b/packages/frontend-app-api/src/wiring/createApp.tsx @@ -28,11 +28,6 @@ import { Core } from '../extensions/Core'; import { CoreRoutes } from '../extensions/CoreRoutes'; import { CoreLayout } from '../extensions/CoreLayout'; import { CoreNav } from '../extensions/CoreNav'; -import { - createExtensionInstance, - ExtensionInstance, -} from './graph/createExtensionInstance'; -import { resolveAppNodeSpecs } from './graph/resolveAppNodeSpecs'; import { AnyApiFactory, ApiHolder, @@ -82,7 +77,7 @@ import { import { BrowserRouter, Route } from 'react-router-dom'; import { SidebarItem } from '@backstage/core-components'; import { DarkTheme, LightTheme } from '../extensions/themes'; -import { extractRouteInfoFromInstanceTree } from '../routing/extractRouteInfoFromInstanceTree'; +import { extractRouteInfoFromAppNode } from '../routing/extractRouteInfoFromAppNode'; import { getOrCreateGlobalSingleton } from '@backstage/version-bridge'; import { appLanguageApiRef, @@ -92,8 +87,16 @@ import { AppRouteBinder } from '../routing'; import { RoutingProvider } from '../routing/RoutingProvider'; import { resolveRouteBindings } from '../routing/resolveRouteBindings'; import { collectRouteIds } from '../routing/collectRouteIds'; -import { readAppExtensionsConfig } from './graph/readAppExtensionsConfig'; -import { AppNodeSpec } from './graph'; +import { AppNode, createAppGraph } from './graph'; + +const builtinExtensions = [ + Core, + CoreRoutes, + CoreNav, + CoreLayout, + LightTheme, + DarkTheme, +]; /** @public */ export interface ExtensionTreeNode { @@ -114,20 +117,38 @@ export function createExtensionTree(options: { config: Config; }): ExtensionTree { const features = getAvailableFeatures(options.config); - const { instances } = createInstances({ + const graph = createAppGraph({ features, + builtinExtensions, config: options.config, }); + function convertNode(node?: AppNode): ExtensionTreeNode | undefined { + return ( + node && { + id: node.spec.id, + getData(ref: ExtensionDataRef): T | undefined { + return node.instance?.getData(ref); + }, + } + ); + } + return { getExtension(id: string): ExtensionTreeNode | undefined { - return instances.get(id); + return convertNode(graph.nodes.get(id)); }, getExtensionAttachments( id: string, inputName: string, ): ExtensionTreeNode[] { - return instances.get(id)?.attachments.get(inputName) ?? []; + return ( + graph.nodes + .get(id) + ?.edges.attachments.get(inputName) + ?.map(convertNode) + .filter((node): node is ExtensionTreeNode => Boolean(node)) ?? [] + ); }, getRootRoutes(): JSX.Element[] { return this.getExtensionAttachments('core.routes', 'routes').map(node => { @@ -177,89 +198,6 @@ export function createExtensionTree(options: { }; } -/** - * @internal - */ -export function createInstances(options: { - features: (BackstagePlugin | ExtensionOverrides)[]; - config: Config; -}) { - const builtinExtensions = [ - Core, - CoreRoutes, - CoreNav, - CoreLayout, - LightTheme, - DarkTheme, - ]; - - // pull in default extension instance from discovered packages - // apply config to adjust default extension instances and add more - const appNodeSpecs = resolveAppNodeSpecs({ - features: options.features, - builtinExtensions, - parameters: readAppExtensionsConfig(options.config), - }); - - // TODO: validate the config of all extension instances - // We do it at this point to ensure that merging (if any) of config has already happened - - // Create attachment map so that we can look attachments up during instance creation - const attachmentMap = new Map>(); - for (const instanceParams of appNodeSpecs) { - const extensionId = instanceParams.attachTo.id; - const pointId = instanceParams.attachTo.input; - let pointMap = attachmentMap.get(extensionId); - if (!pointMap) { - pointMap = new Map(); - attachmentMap.set(extensionId, pointMap); - } - - let instances = pointMap.get(pointId); - if (!instances) { - instances = []; - pointMap.set(pointId, instances); - } - - instances.push(instanceParams); - } - - const instances = new Map(); - - function createInstance(instanceParams: AppNodeSpec): ExtensionInstance { - const extensionId = instanceParams.extension.id; - const existingInstance = instances.get(extensionId); - if (existingInstance) { - return existingInstance; - } - - const attachments = new Map( - Array.from(attachmentMap.get(extensionId)?.entries() ?? []).map( - ([inputName, attachmentConfigs]) => { - return [inputName, attachmentConfigs.map(createInstance)]; - }, - ), - ); - - const newInstance = createExtensionInstance({ - extension: instanceParams.extension, - source: instanceParams.source, - config: instanceParams.config, - attachments, - }); - - instances.set(extensionId, newInstance); - - return newInstance; - } - - const coreInstance = createInstance( - appNodeSpecs.find(p => p.extension.id === 'core')!, - ); - - return { coreInstance, instances }; -} - function deduplicateFeatures( allFeatures: (BackstagePlugin | ExtensionOverrides)[], ): (BackstagePlugin | ExtensionOverrides)[] { @@ -309,8 +247,9 @@ export function createApp(options: { ...(options.features ?? []), ]); - const { coreInstance } = createInstances({ + const appGraph = createAppGraph({ features: allFeatures, + builtinExtensions, config, }); @@ -323,11 +262,11 @@ export function createApp(options: { const routeIds = collectRouteIds(allFeatures); const App = () => ( - + {/* TODO: set base path using the logic from AppRouter */} - {coreInstance.getData(coreExtensionData.reactElement)} + {appGraph.root.instance!.getData( + coreExtensionData.reactElement, + )} @@ -417,22 +358,19 @@ function createLegacyAppContext(plugins: BackstagePlugin[]): AppContext { }; } -function createApiHolder( - coreExtension: ExtensionInstance, - configApi: ConfigApi, -): ApiHolder { +function createApiHolder(core: AppNode, configApi: ConfigApi): ApiHolder { const factoryRegistry = new ApiFactoryRegistry(); const pluginApis = - coreExtension.attachments + core.edges.attachments .get('apis') - ?.map(e => e.getData(coreExtensionData.apiFactory)) + ?.map(e => e.instance?.getData(coreExtensionData.apiFactory)) .filter((x): x is AnyApiFactory => !!x) ?? []; const themeExtensions = - coreExtension.attachments + core.edges.attachments .get('themes') - ?.map(e => e.getData(coreExtensionData.theme)) + ?.map(e => e.instance?.getData(coreExtensionData.theme)) .filter((x): x is AppTheme => !!x) ?? []; for (const factory of [...defaultApis, ...pluginApis]) { From 6473b37182161d76e0a3b006dae7ef42a87ce8f4 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Wed, 18 Oct 2023 13:28:54 +0200 Subject: [PATCH 24/31] frontend-app-api: move graph/ to top level Signed-off-by: Patrik Oldsberg --- .../frontend-app-api/src/{wiring => }/graph/createAppGraph.ts | 0 packages/frontend-app-api/src/{wiring => }/graph/index.ts | 0 .../src/{wiring => }/graph/instantiateAppNodeTree.test.ts | 0 .../src/{wiring => }/graph/instantiateAppNodeTree.ts | 0 .../src/{wiring => }/graph/readAppExtensionsConfig.test.ts | 0 .../src/{wiring => }/graph/readAppExtensionsConfig.ts | 0 .../src/{wiring => }/graph/resolveAppGraph.test.ts | 0 .../frontend-app-api/src/{wiring => }/graph/resolveAppGraph.ts | 0 .../src/{wiring => }/graph/resolveAppNodeSpecs.test.ts | 0 .../src/{wiring => }/graph/resolveAppNodeSpecs.ts | 2 +- packages/frontend-app-api/src/{wiring => }/graph/types.ts | 0 .../src/routing/extractRouteInfoFromAppNode.test.ts | 2 +- .../frontend-app-api/src/routing/extractRouteInfoFromAppNode.ts | 2 +- packages/frontend-app-api/src/wiring/createApp.tsx | 2 +- 14 files changed, 4 insertions(+), 4 deletions(-) rename packages/frontend-app-api/src/{wiring => }/graph/createAppGraph.ts (100%) rename packages/frontend-app-api/src/{wiring => }/graph/index.ts (100%) rename packages/frontend-app-api/src/{wiring => }/graph/instantiateAppNodeTree.test.ts (100%) rename packages/frontend-app-api/src/{wiring => }/graph/instantiateAppNodeTree.ts (100%) rename packages/frontend-app-api/src/{wiring => }/graph/readAppExtensionsConfig.test.ts (100%) rename packages/frontend-app-api/src/{wiring => }/graph/readAppExtensionsConfig.ts (100%) rename packages/frontend-app-api/src/{wiring => }/graph/resolveAppGraph.test.ts (100%) rename packages/frontend-app-api/src/{wiring => }/graph/resolveAppGraph.ts (100%) rename packages/frontend-app-api/src/{wiring => }/graph/resolveAppNodeSpecs.test.ts (100%) rename packages/frontend-app-api/src/{wiring => }/graph/resolveAppNodeSpecs.ts (98%) rename packages/frontend-app-api/src/{wiring => }/graph/types.ts (100%) diff --git a/packages/frontend-app-api/src/wiring/graph/createAppGraph.ts b/packages/frontend-app-api/src/graph/createAppGraph.ts similarity index 100% rename from packages/frontend-app-api/src/wiring/graph/createAppGraph.ts rename to packages/frontend-app-api/src/graph/createAppGraph.ts diff --git a/packages/frontend-app-api/src/wiring/graph/index.ts b/packages/frontend-app-api/src/graph/index.ts similarity index 100% rename from packages/frontend-app-api/src/wiring/graph/index.ts rename to packages/frontend-app-api/src/graph/index.ts diff --git a/packages/frontend-app-api/src/wiring/graph/instantiateAppNodeTree.test.ts b/packages/frontend-app-api/src/graph/instantiateAppNodeTree.test.ts similarity index 100% rename from packages/frontend-app-api/src/wiring/graph/instantiateAppNodeTree.test.ts rename to packages/frontend-app-api/src/graph/instantiateAppNodeTree.test.ts diff --git a/packages/frontend-app-api/src/wiring/graph/instantiateAppNodeTree.ts b/packages/frontend-app-api/src/graph/instantiateAppNodeTree.ts similarity index 100% rename from packages/frontend-app-api/src/wiring/graph/instantiateAppNodeTree.ts rename to packages/frontend-app-api/src/graph/instantiateAppNodeTree.ts diff --git a/packages/frontend-app-api/src/wiring/graph/readAppExtensionsConfig.test.ts b/packages/frontend-app-api/src/graph/readAppExtensionsConfig.test.ts similarity index 100% rename from packages/frontend-app-api/src/wiring/graph/readAppExtensionsConfig.test.ts rename to packages/frontend-app-api/src/graph/readAppExtensionsConfig.test.ts diff --git a/packages/frontend-app-api/src/wiring/graph/readAppExtensionsConfig.ts b/packages/frontend-app-api/src/graph/readAppExtensionsConfig.ts similarity index 100% rename from packages/frontend-app-api/src/wiring/graph/readAppExtensionsConfig.ts rename to packages/frontend-app-api/src/graph/readAppExtensionsConfig.ts diff --git a/packages/frontend-app-api/src/wiring/graph/resolveAppGraph.test.ts b/packages/frontend-app-api/src/graph/resolveAppGraph.test.ts similarity index 100% rename from packages/frontend-app-api/src/wiring/graph/resolveAppGraph.test.ts rename to packages/frontend-app-api/src/graph/resolveAppGraph.test.ts diff --git a/packages/frontend-app-api/src/wiring/graph/resolveAppGraph.ts b/packages/frontend-app-api/src/graph/resolveAppGraph.ts similarity index 100% rename from packages/frontend-app-api/src/wiring/graph/resolveAppGraph.ts rename to packages/frontend-app-api/src/graph/resolveAppGraph.ts diff --git a/packages/frontend-app-api/src/wiring/graph/resolveAppNodeSpecs.test.ts b/packages/frontend-app-api/src/graph/resolveAppNodeSpecs.test.ts similarity index 100% rename from packages/frontend-app-api/src/wiring/graph/resolveAppNodeSpecs.test.ts rename to packages/frontend-app-api/src/graph/resolveAppNodeSpecs.test.ts diff --git a/packages/frontend-app-api/src/wiring/graph/resolveAppNodeSpecs.ts b/packages/frontend-app-api/src/graph/resolveAppNodeSpecs.ts similarity index 98% rename from packages/frontend-app-api/src/wiring/graph/resolveAppNodeSpecs.ts rename to packages/frontend-app-api/src/graph/resolveAppNodeSpecs.ts index 8cad54f4be..51480117d7 100644 --- a/packages/frontend-app-api/src/wiring/graph/resolveAppNodeSpecs.ts +++ b/packages/frontend-app-api/src/graph/resolveAppNodeSpecs.ts @@ -20,7 +20,7 @@ import { ExtensionOverrides, } from '@backstage/frontend-plugin-api'; // eslint-disable-next-line @backstage/no-relative-monorepo-imports -import { toInternalExtensionOverrides } from '../../../../frontend-plugin-api/src/wiring/createExtensionOverrides'; +import { toInternalExtensionOverrides } from '../../../frontend-plugin-api/src/wiring/createExtensionOverrides'; import { ExtensionParameters } from './readAppExtensionsConfig'; import { AppNodeSpec } from './types'; diff --git a/packages/frontend-app-api/src/wiring/graph/types.ts b/packages/frontend-app-api/src/graph/types.ts similarity index 100% rename from packages/frontend-app-api/src/wiring/graph/types.ts rename to packages/frontend-app-api/src/graph/types.ts diff --git a/packages/frontend-app-api/src/routing/extractRouteInfoFromAppNode.test.ts b/packages/frontend-app-api/src/routing/extractRouteInfoFromAppNode.test.ts index 2eab3c8e7a..e0adf3062a 100644 --- a/packages/frontend-app-api/src/routing/extractRouteInfoFromAppNode.test.ts +++ b/packages/frontend-app-api/src/routing/extractRouteInfoFromAppNode.test.ts @@ -28,7 +28,7 @@ import { createRouteRef, } from '@backstage/frontend-plugin-api'; import { MockConfigApi } from '@backstage/test-utils'; -import { createAppGraph } from '../wiring/graph'; +import { createAppGraph } from '../graph'; import { Core } from '../extensions/Core'; import { CoreRoutes } from '../extensions/CoreRoutes'; import { CoreNav } from '../extensions/CoreNav'; diff --git a/packages/frontend-app-api/src/routing/extractRouteInfoFromAppNode.ts b/packages/frontend-app-api/src/routing/extractRouteInfoFromAppNode.ts index 1ecb28113b..71991ee44e 100644 --- a/packages/frontend-app-api/src/routing/extractRouteInfoFromAppNode.ts +++ b/packages/frontend-app-api/src/routing/extractRouteInfoFromAppNode.ts @@ -18,7 +18,7 @@ import { RouteRef, coreExtensionData } from '@backstage/frontend-plugin-api'; // eslint-disable-next-line @backstage/no-relative-monorepo-imports import { toLegacyPlugin } from '../wiring/createApp'; import { BackstageRouteObject } from './types'; -import { AppNode } from '../wiring/graph'; +import { AppNode } from '../graph'; // We always add a child that matches all subroutes but without any route refs. This makes // sure that we're always able to match each route no matter how deep the navigation goes. diff --git a/packages/frontend-app-api/src/wiring/createApp.tsx b/packages/frontend-app-api/src/wiring/createApp.tsx index 1dbfb3912b..4f7817314a 100644 --- a/packages/frontend-app-api/src/wiring/createApp.tsx +++ b/packages/frontend-app-api/src/wiring/createApp.tsx @@ -87,7 +87,7 @@ import { AppRouteBinder } from '../routing'; import { RoutingProvider } from '../routing/RoutingProvider'; import { resolveRouteBindings } from '../routing/resolveRouteBindings'; import { collectRouteIds } from '../routing/collectRouteIds'; -import { AppNode, createAppGraph } from './graph'; +import { AppNode, createAppGraph } from '../graph'; const builtinExtensions = [ Core, From 6d2258621f5e3ed4ef98e18d5478ca0e3ad9600d Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Wed, 18 Oct 2023 13:36:39 +0200 Subject: [PATCH 25/31] frontend-app-api: update resolveAppNodeSpecs tests Signed-off-by: Patrik Oldsberg --- .../src/graph/resolveAppNodeSpecs.test.ts | 43 ++++++++++++++++--- 1 file changed, 38 insertions(+), 5 deletions(-) diff --git a/packages/frontend-app-api/src/graph/resolveAppNodeSpecs.test.ts b/packages/frontend-app-api/src/graph/resolveAppNodeSpecs.test.ts index 1d3c0a26ed..a3cda23113 100644 --- a/packages/frontend-app-api/src/graph/resolveAppNodeSpecs.test.ts +++ b/packages/frontend-app-api/src/graph/resolveAppNodeSpecs.test.ts @@ -54,8 +54,18 @@ describe('resolveAppNodeSpecs', () => { parameters: [], }), ).toEqual([ - { extension: a, attachTo: { id: 'root', input: 'default' } }, - { extension: b, attachTo: { id: 'root', input: 'default' } }, + { + id: 'a', + extension: a, + attachTo: { id: 'root', input: 'default' }, + disabled: false, + }, + { + id: 'b', + extension: b, + attachTo: { id: 'root', input: 'default' }, + disabled: false, + }, ]); }); @@ -76,11 +86,18 @@ describe('resolveAppNodeSpecs', () => { }), ).toEqual([ { + id: 'a', extension: a, attachTo: { id: 'root', input: 'default' }, source: pluginA, + disabled: false, + }, + { + id: 'b', + extension: b, + attachTo: { id: 'derp', input: 'default' }, + disabled: false, }, - { extension: b, attachTo: { id: 'derp', input: 'default' } }, ]); }); @@ -109,16 +126,20 @@ describe('resolveAppNodeSpecs', () => { }), ).toEqual([ { + id: 'a', extension: a, attachTo: { id: 'root', input: 'default' }, source: plugin, config: { foo: { bar: 1 } }, + disabled: false, }, { + id: 'b', extension: b, attachTo: { id: 'root', input: 'default' }, source: plugin, config: { foo: { qux: 3 } }, + disabled: false, }, ]); }); @@ -142,8 +163,18 @@ describe('resolveAppNodeSpecs', () => { ], }), ).toEqual([ - { extension: b, attachTo: { id: 'root', input: 'default' } }, - { extension: a, attachTo: { id: 'root', input: 'default' } }, + { + id: 'b', + extension: b, + attachTo: { id: 'root', input: 'default' }, + disabled: false, + }, + { + id: 'a', + extension: a, + attachTo: { id: 'root', input: 'default' }, + disabled: false, + }, ]); }); @@ -173,10 +204,12 @@ describe('resolveAppNodeSpecs', () => { expect(result[0].source).toBe(plugin); expect(result[1]).toEqual({ + id: 'c', extension: cOverride, attachTo: { id: 'root', input: 'default' }, config: undefined, source: undefined, + disabled: false, }); }); From 5aad4f7720080956f4e646f8d029e14410f513bd Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Wed, 18 Oct 2023 13:37:07 +0200 Subject: [PATCH 26/31] frontend-app-api: no longer reject core extension configuration Signed-off-by: Patrik Oldsberg --- packages/frontend-app-api/src/graph/resolveAppNodeSpecs.ts | 7 ------- 1 file changed, 7 deletions(-) diff --git a/packages/frontend-app-api/src/graph/resolveAppNodeSpecs.ts b/packages/frontend-app-api/src/graph/resolveAppNodeSpecs.ts index 51480117d7..d6cfcaede2 100644 --- a/packages/frontend-app-api/src/graph/resolveAppNodeSpecs.ts +++ b/packages/frontend-app-api/src/graph/resolveAppNodeSpecs.ts @@ -164,13 +164,6 @@ export function resolveAppNodeSpecs(options: { for (const overrideParam of parameters) { const extensionId = overrideParam.id; - // Prevent core parametrization - if (extensionId === 'core') { - throw new Error( - "A 'core' extension configuration was detected, but the core extension is not configurable", - ); - } - const existingIndex = configuredExtensions.findIndex( e => e.extension.id === extensionId, ); From 89dc1250d86138f73b4b7e927e55c56b326291bc Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Wed, 18 Oct 2023 19:18:10 +0200 Subject: [PATCH 27/31] frontend-app-api: make it possible to configure which extensions are forbidden Signed-off-by: Patrik Oldsberg --- .../src/graph/createAppGraph.ts | 2 + .../src/graph/resolveAppNodeSpecs.test.ts | 41 +++++++++++++++++++ .../src/graph/resolveAppNodeSpecs.ts | 28 ++++++++----- 3 files changed, 61 insertions(+), 10 deletions(-) diff --git a/packages/frontend-app-api/src/graph/createAppGraph.ts b/packages/frontend-app-api/src/graph/createAppGraph.ts index 4c08b7c5b4..cb72fa3cd1 100644 --- a/packages/frontend-app-api/src/graph/createAppGraph.ts +++ b/packages/frontend-app-api/src/graph/createAppGraph.ts @@ -40,7 +40,9 @@ export function createAppGraph(options: CreateAppGraphOptions): AppGraph { features: options.features, builtinExtensions: options.builtinExtensions, parameters: readAppExtensionsConfig(options.config), + forbidden: new Set(['core']), }), + 'core', ); instantiateAppNodeTree(appGraph.root); return appGraph; diff --git a/packages/frontend-app-api/src/graph/resolveAppNodeSpecs.test.ts b/packages/frontend-app-api/src/graph/resolveAppNodeSpecs.test.ts index a3cda23113..59930968c6 100644 --- a/packages/frontend-app-api/src/graph/resolveAppNodeSpecs.test.ts +++ b/packages/frontend-app-api/src/graph/resolveAppNodeSpecs.test.ts @@ -234,4 +234,45 @@ describe('resolveAppNodeSpecs', () => { expect(result.map(r => r.extension.id)).toEqual(['b', 'c', 'a']); }); + + it('throws an error when a forbidden extension is overridden by a plugin', () => { + expect(() => + resolveAppNodeSpecs({ + features: [ + createPlugin({ id: 'test', extensions: [makeExt('forbidden')] }), + ], + builtinExtensions: [], + parameters: [], + forbidden: new Set(['forbidden']), + }), + ).toThrow( + "It is forbidden to override the following extension(s): 'forbidden', which is done by the following plugin(s): 'test'", + ); + }); + + it('throws an error when a forbidden extension is overridden by overrides', () => { + expect(() => + resolveAppNodeSpecs({ + features: [ + createExtensionOverrides({ extensions: [makeExt('forbidden')] }), + ], + builtinExtensions: [], + parameters: [], + forbidden: new Set(['forbidden']), + }), + ).toThrow( + "It is forbidden to override the following extension(s): 'forbidden', which is done by one or more extension overrides", + ); + }); + + it('throws an error when a forbidden extension is parametrized', () => { + expect(() => + resolveAppNodeSpecs({ + features: [], + builtinExtensions: [], + parameters: [{ id: 'forbidden', disabled: false }], + forbidden: new Set(['forbidden']), + }), + ).toThrow("Configuration of the 'forbidden' extension is forbidden"); + }); }); diff --git a/packages/frontend-app-api/src/graph/resolveAppNodeSpecs.ts b/packages/frontend-app-api/src/graph/resolveAppNodeSpecs.ts index d6cfcaede2..ca3c4fd1ea 100644 --- a/packages/frontend-app-api/src/graph/resolveAppNodeSpecs.ts +++ b/packages/frontend-app-api/src/graph/resolveAppNodeSpecs.ts @@ -29,8 +29,9 @@ export function resolveAppNodeSpecs(options: { features: (BackstagePlugin | ExtensionOverrides)[]; builtinExtensions: Extension[]; parameters: Array; + forbidden?: Set; }): AppNodeSpec[] { - const { builtinExtensions, parameters } = options; + const { builtinExtensions, parameters, forbidden = new Set() } = options; const plugins = options.features.filter( (f): f is BackstagePlugin => f.$$type === '@backstage/BackstagePlugin', @@ -48,20 +49,21 @@ export function resolveAppNodeSpecs(options: { ); // Prevent core override - if (pluginExtensions.some(({ id }) => id === 'core')) { - const pluginIds = pluginExtensions - .filter(({ id }) => id === 'core') - .map(({ source }) => source.id); + if (pluginExtensions.some(({ id }) => forbidden.has(id))) { + const pluginsStr = pluginExtensions + .filter(({ id }) => forbidden.has(id)) + .map(({ source }) => `'${source.id}'`) + .join(', '); + const forbiddenStr = [...forbidden].map(id => `'${id}'`).join(', '); throw new Error( - `The following plugin(s) are overriding the 'core' extension which is forbidden: ${pluginIds.join( - ',', - )}`, + `It is forbidden to override the following extension(s): ${forbiddenStr}, which is done by the following plugin(s): ${pluginsStr}`, ); } - if (overrideExtensions.some(({ id }) => id === 'root')) { + if (overrideExtensions.some(({ id }) => forbidden.has(id))) { + const forbiddenStr = [...forbidden].map(id => `'${id}'`).join(', '); throw new Error( - `An extension override is overriding the 'root' extension which is forbidden`, + `It is forbidden to override the following extension(s): ${forbiddenStr}, which is done by one or more extension overrides`, ); } const overrideExtensionIds = overrideExtensions.map(({ id }) => id); @@ -164,6 +166,12 @@ export function resolveAppNodeSpecs(options: { for (const overrideParam of parameters) { const extensionId = overrideParam.id; + if (forbidden.has(extensionId)) { + throw new Error( + `Configuration of the '${extensionId}' extension is forbidden`, + ); + } + const existingIndex = configuredExtensions.findIndex( e => e.extension.id === extensionId, ); From ee39751fdffa80b0a6f0535ea520863887958fcf Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Wed, 18 Oct 2023 19:20:12 +0200 Subject: [PATCH 28/31] frontend-app-api: make rootNodeId required in resolveAppGraph Signed-off-by: Patrik Oldsberg --- .../src/graph/createAppGraph.ts | 2 +- .../src/graph/instantiateAppNodeTree.test.ts | 118 ++++++++---------- .../src/graph/resolveAppGraph.test.ts | 48 ++++--- .../src/graph/resolveAppGraph.ts | 2 +- 4 files changed, 78 insertions(+), 92 deletions(-) diff --git a/packages/frontend-app-api/src/graph/createAppGraph.ts b/packages/frontend-app-api/src/graph/createAppGraph.ts index cb72fa3cd1..ace8595f53 100644 --- a/packages/frontend-app-api/src/graph/createAppGraph.ts +++ b/packages/frontend-app-api/src/graph/createAppGraph.ts @@ -36,13 +36,13 @@ export interface CreateAppGraphOptions { /** @internal */ export function createAppGraph(options: CreateAppGraphOptions): AppGraph { const appGraph = resolveAppGraph( + 'core', resolveAppNodeSpecs({ features: options.features, builtinExtensions: options.builtinExtensions, parameters: readAppExtensionsConfig(options.config), forbidden: new Set(['core']), }), - 'core', ); instantiateAppNodeTree(appGraph.root); return appGraph; diff --git a/packages/frontend-app-api/src/graph/instantiateAppNodeTree.test.ts b/packages/frontend-app-api/src/graph/instantiateAppNodeTree.test.ts index 7bffc69c7c..21f364dda2 100644 --- a/packages/frontend-app-api/src/graph/instantiateAppNodeTree.test.ts +++ b/packages/frontend-app-api/src/graph/instantiateAppNodeTree.test.ts @@ -79,10 +79,9 @@ function makeInstanceWithId( describe('instantiateAppNodeTree', () => { it('should instantiate a single node', () => { - const graph = resolveAppGraph( - [{ ...makeSpec(simpleExtension), id: 'root-node' }], - 'root-node', - ); + const graph = resolveAppGraph('root-node', [ + { ...makeSpec(simpleExtension), id: 'root-node' }, + ]); expect(graph.root.instance).not.toBeDefined(); instantiateAppNodeTree(graph.root); expect(graph.root.instance).toBeDefined(); @@ -94,43 +93,39 @@ describe('instantiateAppNodeTree', () => { }); it('should not instantiate disabled nodes', () => { - const graph = resolveAppGraph( - [{ ...makeSpec(simpleExtension), id: 'root-node', disabled: true }], - 'root-node', - ); + const graph = resolveAppGraph('root-node', [ + { ...makeSpec(simpleExtension), id: 'root-node', disabled: true }, + ]); expect(graph.root.instance).not.toBeDefined(); instantiateAppNodeTree(graph.root); expect(graph.root.instance).not.toBeDefined(); }); it('should instantiate a node with attachments', () => { - const graph = resolveAppGraph( - [ - { - ...makeSpec( - createExtension({ - id: 'root-node', - attachTo: { id: 'ignored', input: 'ignored' }, - inputs: { - test: createExtensionInput({ test: testDataRef }), - }, - output: { - inputMirror: inputMirrorDataRef, - }, - factory({ bind, inputs }) { - bind({ inputMirror: inputs }); - }, - }), - ), - }, - { - ...makeSpec(simpleExtension), - id: 'child-node', - attachTo: { id: 'root-node', input: 'test' }, - }, - ], - 'root-node', - ); + const graph = resolveAppGraph('root-node', [ + { + ...makeSpec( + createExtension({ + id: 'root-node', + attachTo: { id: 'ignored', input: 'ignored' }, + inputs: { + test: createExtensionInput({ test: testDataRef }), + }, + output: { + inputMirror: inputMirrorDataRef, + }, + factory({ bind, inputs }) { + bind({ inputMirror: inputs }); + }, + }), + ), + }, + { + ...makeSpec(simpleExtension), + id: 'child-node', + attachTo: { id: 'root-node', input: 'test' }, + }, + ]); const childNode = graph.nodes.get('child-node'); expect(childNode).toBeDefined(); @@ -151,34 +146,31 @@ describe('instantiateAppNodeTree', () => { }); it('should not instantiate disabled attachments', () => { - const graph = resolveAppGraph( - [ - { - ...makeSpec( - createExtension({ - id: 'root-node', - attachTo: { id: 'ignored', input: 'ignored' }, - inputs: { - test: createExtensionInput({ test: testDataRef }), - }, - output: { - inputMirror: inputMirrorDataRef, - }, - factory({ bind, inputs }) { - bind({ inputMirror: inputs }); - }, - }), - ), - }, - { - ...makeSpec(simpleExtension), - id: 'child-node', - attachTo: { id: 'root-node', input: 'test' }, - disabled: true, - }, - ], - 'root-node', - ); + const graph = resolveAppGraph('root-node', [ + { + ...makeSpec( + createExtension({ + id: 'root-node', + attachTo: { id: 'ignored', input: 'ignored' }, + inputs: { + test: createExtensionInput({ test: testDataRef }), + }, + output: { + inputMirror: inputMirrorDataRef, + }, + factory({ bind, inputs }) { + bind({ inputMirror: inputs }); + }, + }), + ), + }, + { + ...makeSpec(simpleExtension), + id: 'child-node', + attachTo: { id: 'root-node', input: 'test' }, + disabled: true, + }, + ]); const childNode = graph.nodes.get('child-node'); expect(childNode).toBeDefined(); diff --git a/packages/frontend-app-api/src/graph/resolveAppGraph.test.ts b/packages/frontend-app-api/src/graph/resolveAppGraph.test.ts index 2d1acbbc04..2668c51863 100644 --- a/packages/frontend-app-api/src/graph/resolveAppGraph.test.ts +++ b/packages/frontend-app-api/src/graph/resolveAppGraph.test.ts @@ -34,13 +34,13 @@ const baseSpec = { describe('buildAppGraph', () => { it('should fail to create an empty graph', () => { - expect(() => resolveAppGraph([])).toThrow( + expect(() => resolveAppGraph('core', [])).toThrow( "No root node with id 'core' found in app graph", ); }); it('should create a graph with only one node', () => { - const graph = resolveAppGraph([{ ...baseSpec, id: 'core' }]); + const graph = resolveAppGraph('core', [{ ...baseSpec, id: 'core' }]); expect(graph.root).toEqual({ spec: { ...baseSpec, id: 'core' }, edges: { attachments: new Map() }, @@ -50,18 +50,15 @@ describe('buildAppGraph', () => { }); it('should create a graph', () => { - const graph = resolveAppGraph( - [ - { ...baseSpec, id: 'a' }, - { ...baseSpec, id: 'b' }, - { ...baseSpec, id: 'c' }, - { ...baseSpec, attachTo: { id: 'b', input: 'x' }, id: 'bx1' }, - { ...baseSpec, attachTo: { id: 'b', input: 'x' }, id: 'bx2' }, - { ...baseSpec, attachTo: { id: 'b', input: 'y' }, id: 'by1' }, - { ...baseSpec, attachTo: { id: 'd', input: 'x' }, id: 'dx1' }, - ], - 'b', - ); + const graph = resolveAppGraph('b', [ + { ...baseSpec, id: 'a' }, + { ...baseSpec, id: 'b' }, + { ...baseSpec, id: 'c' }, + { ...baseSpec, attachTo: { id: 'b', input: 'x' }, id: 'bx1' }, + { ...baseSpec, attachTo: { id: 'b', input: 'x' }, id: 'bx2' }, + { ...baseSpec, attachTo: { id: 'b', input: 'y' }, id: 'by1' }, + { ...baseSpec, attachTo: { id: 'd', input: 'x' }, id: 'dx1' }, + ]); expect(Array.from(graph.nodes.keys())).toEqual([ 'a', @@ -116,18 +113,15 @@ describe('buildAppGraph', () => { }); it('should create a graph out of order', () => { - const graph = resolveAppGraph( - [ - { ...baseSpec, attachTo: { id: 'b', input: 'x' }, id: 'bx2' }, - { ...baseSpec, id: 'a' }, - { ...baseSpec, attachTo: { id: 'b', input: 'y' }, id: 'by1' }, - { ...baseSpec, id: 'b' }, - { ...baseSpec, attachTo: { id: 'b', input: 'x' }, id: 'bx1' }, - { ...baseSpec, id: 'c' }, - { ...baseSpec, attachTo: { id: 'd', input: 'x' }, id: 'dx1' }, - ], - 'b', - ); + const graph = resolveAppGraph('b', [ + { ...baseSpec, attachTo: { id: 'b', input: 'x' }, id: 'bx2' }, + { ...baseSpec, id: 'a' }, + { ...baseSpec, attachTo: { id: 'b', input: 'y' }, id: 'by1' }, + { ...baseSpec, id: 'b' }, + { ...baseSpec, attachTo: { id: 'b', input: 'x' }, id: 'bx1' }, + { ...baseSpec, id: 'c' }, + { ...baseSpec, attachTo: { id: 'd', input: 'x' }, id: 'dx1' }, + ]); expect(Array.from(graph.nodes.keys())).toEqual([ 'bx2', @@ -163,7 +157,7 @@ describe('buildAppGraph', () => { it('throws an error when duplicated extensions are detected', () => { expect(() => - resolveAppGraph([ + resolveAppGraph('core', [ { ...baseSpec, id: 'a' }, { ...baseSpec, id: 'a' }, ]), diff --git a/packages/frontend-app-api/src/graph/resolveAppGraph.ts b/packages/frontend-app-api/src/graph/resolveAppGraph.ts index 18f481fa30..354b4c53ee 100644 --- a/packages/frontend-app-api/src/graph/resolveAppGraph.ts +++ b/packages/frontend-app-api/src/graph/resolveAppGraph.ts @@ -88,8 +88,8 @@ class SerializableAppNode implements AppNode { * @internal */ export function resolveAppGraph( + rootNodeId: string, specs: AppNodeSpec[], - rootNodeId = 'core', ): AppGraph { const nodes = new Map(); From d2e2b01ae1fa93b21eb66d0ba1cf4b655f6228a8 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Wed, 18 Oct 2023 19:25:52 +0200 Subject: [PATCH 29/31] frontend-app-api: move some createApp tests to createAppGraph Signed-off-by: Patrik Oldsberg --- .../src/graph/createAppGraph.test.ts | 123 +++++++++++ .../src/wiring/createApp.test.tsx | 194 +----------------- 2 files changed, 124 insertions(+), 193 deletions(-) create mode 100644 packages/frontend-app-api/src/graph/createAppGraph.test.ts diff --git a/packages/frontend-app-api/src/graph/createAppGraph.test.ts b/packages/frontend-app-api/src/graph/createAppGraph.test.ts new file mode 100644 index 0000000000..2c9f062199 --- /dev/null +++ b/packages/frontend-app-api/src/graph/createAppGraph.test.ts @@ -0,0 +1,123 @@ +/* + * Copyright 2023 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { + createExtension, + createExtensionOverrides, + createPlugin, +} from '@backstage/frontend-plugin-api'; +import { MockConfigApi } from '@backstage/test-utils'; +import { createAppGraph } from './createAppGraph'; + +const extBase = { + id: 'test', + attachTo: { id: 'core', input: 'root' }, + output: {}, + factory() {}, +}; + +describe('createAppGraph', () => { + it('throws an error when a core extension is parametrized', () => { + const config = new MockConfigApi({ + app: { + extensions: [ + { + core: {}, + }, + ], + }, + }); + const features = [ + createPlugin({ + id: 'plugin', + extensions: [], + }), + ]; + expect(() => + createAppGraph({ features, config, builtinExtensions: [] }), + ).toThrow("Configuration of the 'core' extension is forbidden"); + }); + + it('throws an error when a core extension is overridden', () => { + const config = new MockConfigApi({}); + const features = [ + createPlugin({ + id: 'plugin', + extensions: [ + createExtension({ + id: 'core', + attachTo: { id: 'core.routes', input: 'route' }, + inputs: {}, + output: {}, + factory() {}, + }), + ], + }), + ]; + expect(() => + createAppGraph({ features, config, builtinExtensions: [] }), + ).toThrow( + "It is forbidden to override the following extension(s): 'core', which is done by the following plugin(s): 'plugin'", + ); + }); + + it('throws an error when duplicated extensions are detected', () => { + const config = new MockConfigApi({}); + + const ExtensionA = createExtension({ ...extBase, id: 'A' }); + + const ExtensionB = createExtension({ ...extBase, id: 'B' }); + + const PluginA = createPlugin({ + id: 'A', + extensions: [ExtensionA, ExtensionA], + }); + + const PluginB = createPlugin({ + id: 'B', + extensions: [ExtensionA, ExtensionB, ExtensionB], + }); + + const features = [PluginA, PluginB]; + + expect(() => + createAppGraph({ features, config, builtinExtensions: [] }), + ).toThrow( + "The following extensions are duplicated: The extension 'A' was provided 2 time(s) by the plugin 'A' and 1 time(s) by the plugin 'B', The extension 'B' was provided 2 time(s) by the plugin 'B'", + ); + }); + + it('throws an error when duplicated extension overrides are detected', () => { + expect(() => + createAppGraph({ + features: [ + createExtensionOverrides({ + extensions: [ + createExtension({ ...extBase, id: 'a' }), + createExtension({ ...extBase, id: 'a' }), + createExtension({ ...extBase, id: 'b' }), + ], + }), + createExtensionOverrides({ + extensions: [createExtension({ ...extBase, id: 'b' })], + }), + ], + config: new MockConfigApi({}), + builtinExtensions: [], + }), + ).toThrow('The following extensions had duplicate overrides: a, b'); + }); +}); diff --git a/packages/frontend-app-api/src/wiring/createApp.test.tsx b/packages/frontend-app-api/src/wiring/createApp.test.tsx index de574bc6a9..e9fb48f957 100644 --- a/packages/frontend-app-api/src/wiring/createApp.test.tsx +++ b/packages/frontend-app-api/src/wiring/createApp.test.tsx @@ -15,123 +15,15 @@ */ import { - createExtension, - createExtensionOverrides, createPageExtension, createPlugin, - createRouteRef, createThemeExtension, } from '@backstage/frontend-plugin-api'; -import { createApp, createInstances } from './createApp'; import { screen, waitFor } from '@testing-library/react'; +import { createApp } from './createApp'; import { MockConfigApi, renderWithEffects } from '@backstage/test-utils'; import React from 'react'; -const extBaseConfig = { - id: 'test', - attachTo: { id: 'root', input: 'default' }, - output: {}, - factory() {}, -}; - -describe('createInstances', () => { - it('throws an error when a core extension is parametrized', () => { - const config = new MockConfigApi({ - app: { - extensions: [ - { - core: {}, - }, - ], - }, - }); - const features = [ - createPlugin({ - id: 'plugin', - extensions: [], - }), - ]; - expect(() => createInstances({ config, features })).toThrow( - "A 'core' extension configuration was detected, but the core extension is not configurable", - ); - }); - - it('throws an error when a core extension is overridden', () => { - const config = new MockConfigApi({}); - const features = [ - createPlugin({ - id: 'plugin', - extensions: [ - createExtension({ - id: 'core', - attachTo: { id: 'core.routes', input: 'route' }, - inputs: {}, - output: {}, - factory() {}, - }), - ], - }), - ]; - expect(() => createInstances({ config, features })).toThrow( - "The following plugin(s) are overriding the 'core' extension which is forbidden: plugin", - ); - }); - - it('throws an error when duplicated extensions are detected', () => { - const config = new MockConfigApi({}); - - const ExtensionA = createPageExtension({ - id: 'A', - defaultPath: '/', - routeRef: createRouteRef(), - loader: async () =>
Extension A
, - }); - - const ExtensionB = createPageExtension({ - id: 'B', - defaultPath: '/', - routeRef: createRouteRef(), - loader: async () =>
Extension B
, - }); - - const PluginA = createPlugin({ - id: 'A', - extensions: [ExtensionA, ExtensionA], - }); - - const PluginB = createPlugin({ - id: 'B', - extensions: [ExtensionA, ExtensionB, ExtensionB], - }); - - const features = [PluginA, PluginB]; - - expect(() => createInstances({ config, features })).toThrow( - "The following extensions are duplicated: The extension 'A' was provided 2 time(s) by the plugin 'A' and 1 time(s) by the plugin 'B', The extension 'B' was provided 2 time(s) by the plugin 'B'", - ); - }); - - it('throws an error when duplicated extension overrides are detected', () => { - expect(() => - createInstances({ - config: new MockConfigApi({}), - features: [ - createExtensionOverrides({ - extensions: [ - createExtension({ ...extBaseConfig, id: 'a' }), - createExtension({ ...extBaseConfig, id: 'a' }), - createExtension({ ...extBaseConfig, id: 'b' }), - ], - }), - createExtensionOverrides({ - extensions: [createExtension({ ...extBaseConfig, id: 'b' })], - }), - ], - }), - ).toThrow('The following extensions had duplicate overrides: a, b'); - }); -}); - describe('createApp', () => { it('should allow themes to be installed', async () => { const app = createApp({ @@ -161,90 +53,6 @@ describe('createApp', () => { await expect(screen.findByText('Derp')).resolves.toBeInTheDocument(); }); - it('should log an app', () => { - const { coreInstance } = createInstances({ - config: new MockConfigApi({}), - features: [], - }); - - expect(String(coreInstance)).toMatchInlineSnapshot(` - " - root [ - - content [ - - ] - nav [ - - ] - - ] - themes [ - - - ] - " - `); - }); - - it('should serialize an app as JSON', () => { - const { coreInstance } = createInstances({ - config: new MockConfigApi({}), - features: [], - }); - - expect(JSON.parse(JSON.stringify(coreInstance))).toMatchInlineSnapshot(` - { - "attachments": { - "root": [ - { - "attachments": { - "content": [ - { - "id": "core.routes", - "output": [ - "core.reactElement", - ], - }, - ], - "nav": [ - { - "id": "core.nav", - "output": [ - "core.reactElement", - ], - }, - ], - }, - "id": "core.layout", - "output": [ - "core.reactElement", - ], - }, - ], - "themes": [ - { - "id": "themes.light", - "output": [ - "core.theme", - ], - }, - { - "id": "themes.dark", - "output": [ - "core.theme", - ], - }, - ], - }, - "id": "core", - "output": [ - "core.reactElement", - ], - } - `); - }); - it('should deduplicate features keeping the last received one', async () => { const duplicatedFeatureId = 'test'; const app = createApp({ From 062f7a85cbf8e6612f4b1255ee8321e25720fa1e Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Wed, 18 Oct 2023 21:46:37 +0200 Subject: [PATCH 30/31] frontend-app-api: add graph types docs Signed-off-by: Patrik Oldsberg --- packages/frontend-app-api/src/graph/types.ts | 38 +++++++++++++++++--- 1 file changed, 34 insertions(+), 4 deletions(-) diff --git a/packages/frontend-app-api/src/graph/types.ts b/packages/frontend-app-api/src/graph/types.ts index 6e205db9dd..9267f13158 100644 --- a/packages/frontend-app-api/src/graph/types.ts +++ b/packages/frontend-app-api/src/graph/types.ts @@ -20,9 +20,18 @@ import { ExtensionDataRef, } from '@backstage/frontend-plugin-api'; +/* +NOTE: These types are marked as @internal for now, but the intention is for this to be a public API in the future. +*/ + /** * The specification for this node in the app graph. - * @public + * + * @internal + * @remarks + * + * The specifications for a collection of app nodes is all the information needed + * to build the graph and instantiate the nodes. */ export interface AppNodeSpec { readonly id: string; @@ -35,7 +44,12 @@ export interface AppNodeSpec { /** * The connections from this node to other nodes. - * @public + * + * @internal + * @remarks + * + * The app node edges are resolved based on the app node specs, regardless of whether + * adjacent nodes are disabled or not. If no parent attachment is present or */ export interface AppNodeEdges { readonly attachedTo?: { node: AppNode; input: string }; @@ -44,16 +58,24 @@ export interface AppNodeEdges { /** * The instance of this node in the app graph. - * @public + * + * @internal + * @remarks + * + * The app node instance is created when the `factory` function of an extension is called. + * Instances will only be present for nodes in the app that are connected to the root + * node and not disabled */ export interface AppNodeInstance { + /** Returns a sequence of all extension data refs that were output by this instance */ getDataRefs(): Iterable>; + /** Get the output data for a single extension data ref */ getData(ref: ExtensionDataRef): T | undefined; } /** * - * @public + * @internal */ export interface AppNode { /** The specification for how this node should be instantiated */ @@ -64,8 +86,16 @@ export interface AppNode { readonly instance?: AppNodeInstance; } +/** + * The app graph containing all nodes of the app. + * + * @internal + */ export interface AppGraph { + /** The root node of the app */ root: AppNode; + /** A map of all nodes in the app by ID, including orphaned or disabled nodes */ nodes: ReadonlyMap; + /** A sequence of all nodes with a parent that is not reachable from the app root node */ orphans: Iterable; } From e28d379e32f9506d9cf67d65add82bba25534ce7 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Thu, 19 Oct 2023 00:17:37 +0200 Subject: [PATCH 31/31] changesets: add changeset for frontend-app-api app graph refactor Signed-off-by: Patrik Oldsberg --- .changeset/good-plums-confess.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/good-plums-confess.md diff --git a/.changeset/good-plums-confess.md b/.changeset/good-plums-confess.md new file mode 100644 index 0000000000..941adec8b3 --- /dev/null +++ b/.changeset/good-plums-confess.md @@ -0,0 +1,5 @@ +--- +'@backstage/frontend-app-api': patch +--- + +Refactor internal extension instance system into an app graph.