From bd3ebbc368f484c1fffa11d04b624f882cd5541e Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Wed, 22 Nov 2023 16:24:45 +0100 Subject: [PATCH] frontend-plugin-api: add extension duplication validation for createPlugin Co-authored-by: Camila Belo Signed-off-by: Patrik Oldsberg --- .../src/wiring/createPlugin.test.ts | 151 +++++++++++++----- .../src/wiring/createPlugin.ts | 23 ++- 2 files changed, 127 insertions(+), 47 deletions(-) diff --git a/packages/frontend-plugin-api/src/wiring/createPlugin.test.ts b/packages/frontend-plugin-api/src/wiring/createPlugin.test.ts index f69b54aa42..0653c42d7c 100644 --- a/packages/frontend-plugin-api/src/wiring/createPlugin.test.ts +++ b/packages/frontend-plugin-api/src/wiring/createPlugin.test.ts @@ -28,48 +28,34 @@ import { createExtensionInput } from './createExtensionInput'; const nameExtensionDataRef = createExtensionDataRef('name'); -const TechRadarPage = createExtension({ - id: 'plugin.techradar.page', - attachTo: { id: 'test.output', input: 'names' }, +const Extension1 = createExtension({ + name: '1', + attachTo: { id: 'test/output', input: 'names' }, output: { name: nameExtensionDataRef, }, factory() { - return { name: 'TechRadar' }; + return { name: 'extension-1' }; }, }); -const CatalogPage = createExtension({ - id: 'plugin.catalog.page', - attachTo: { id: 'test.output', input: 'names' }, +const Extension2 = createExtension({ + name: '2', + attachTo: { id: 'test/output', input: 'names' }, output: { name: nameExtensionDataRef, }, configSchema: createSchemaFromZod(z => - z.object({ name: z.string().default('Catalog') }), + z.object({ name: z.string().default('extension-2') }), ), factory({ config }) { return { name: config.name }; }, }); -const TechDocsAddon = createExtension({ - id: 'plugin.techdocs.addon.example', - attachTo: { id: 'plugin.techdocs.page', input: 'addons' }, - output: { - name: nameExtensionDataRef, - }, - configSchema: createSchemaFromZod(z => - z.object({ name: z.string().default('TechDocsAddon') }), - ), - factory({ config }) { - return { name: config.name }; - }, -}); - -const TechDocsPage = createExtension({ - id: 'plugin.techdocs.page', - attachTo: { id: 'test.output', input: 'names' }, +const Extension3 = createExtension({ + name: '3', + attachTo: { id: 'test/output', input: 'names' }, inputs: { addons: createExtensionInput({ name: nameExtensionDataRef, @@ -79,12 +65,40 @@ const TechDocsPage = createExtension({ name: nameExtensionDataRef, }, factory({ inputs }) { - return { name: `TechDocs-${inputs.addons.map(n => n.name).join('-')}` }; + return { name: `extension-3:${inputs.addons.map(n => n.name).join('-')}` }; + }, +}); + +const Child = createExtension({ + name: 'child', + attachTo: { id: 'test/3', input: 'addons' }, + output: { + name: nameExtensionDataRef, + }, + configSchema: createSchemaFromZod(z => + z.object({ name: z.string().default('child') }), + ), + factory({ config }) { + return { name: config.name }; + }, +}); + +const Child2 = createExtension({ + name: 'child2', + attachTo: { id: 'test/3', input: 'addons' }, + output: { + name: nameExtensionDataRef, + }, + configSchema: createSchemaFromZod(z => + z.object({ name: z.string().default('child2') }), + ), + factory({ config }) { + return { name: config.name }; }, }); const outputExtension = createExtension({ - id: 'test.output', + name: 'output', attachTo: { id: 'core', input: 'root' }, inputs: { names: createExtensionInput({ @@ -118,40 +132,34 @@ function createTestAppRoot({ describe('createPlugin', () => { it('should create an empty plugin', () => { - const plugin = createPlugin({ id: 'empty' }); + const plugin = createPlugin({ id: 'test' }); expect(plugin).toBeDefined(); }); it('should create a plugin with extension instances', async () => { const plugin = createPlugin({ - id: 'empty', - extensions: [TechRadarPage, CatalogPage, outputExtension], + id: 'test', + extensions: [Extension1, Extension2, outputExtension], }); expect(plugin).toBeDefined(); await renderWithEffects( createTestAppRoot({ features: [plugin], - config: { app: { extensions: [{ 'core.router': false }] } }, + config: { app: { extensions: [{ 'core/router': false }] } }, }), ); await expect( - screen.findByText('Names: TechRadar, Catalog'), + screen.findByText('Names: extension-1, extension-2'), ).resolves.toBeInTheDocument(); }); it('should create a plugin with nested extension instances', async () => { const plugin = createPlugin({ - id: 'empty', - extensions: [ - TechRadarPage, - CatalogPage, - TechDocsPage, - TechDocsAddon, - outputExtension, - ], + id: 'test', + extensions: [Extension1, Extension2, Extension3, Child, outputExtension], }); expect(plugin).toBeDefined(); @@ -161,10 +169,10 @@ describe('createPlugin', () => { config: { app: { extensions: [ - { 'core.router': false }, + { 'core/router': false }, { - 'plugin.catalog.page': { - config: { name: 'CatalogRenamed' }, + 'test/2': { + config: { name: 'extension-2-renamed' }, }, }, ], @@ -175,8 +183,63 @@ describe('createPlugin', () => { await expect( screen.findByText( - 'Names: TechRadar, CatalogRenamed, TechDocs-TechDocsAddon', + 'Names: extension-1, extension-2-renamed, extension-3:child', ), ).resolves.toBeInTheDocument(); }); + + it('should create a plugin with nested extension instances and multiple children', async () => { + const plugin = createPlugin({ + id: 'test', + extensions: [ + Extension1, + Extension2, + Extension3, + Child, + Child2, + outputExtension, + ], + }); + expect(plugin).toBeDefined(); + + await renderWithEffects( + createTestAppRoot({ + features: [plugin], + config: { + app: { + extensions: [{ 'core/router': false }], + }, + }, + }), + ); + + await expect( + screen.findByText( + 'Names: extension-1, extension-2, extension-3:child-child2', + ), + ).resolves.toBeInTheDocument(); + }); + + it('should throw on duplicate extensions', async () => { + expect(() => + createPlugin({ + id: 'test', + extensions: [Extension1, Extension1], + }), + ).toThrow("Plugin 'test' provided duplicate extensions: test/1"); + + expect(() => + createPlugin({ + id: 'test', + extensions: [ + Extension1, + Extension2, + Extension2, + Extension3, + Extension3, + Extension3, + ], + }), + ).toThrow("Plugin 'test' provided duplicate extensions: test/2, test/3"); + }); }); diff --git a/packages/frontend-plugin-api/src/wiring/createPlugin.ts b/packages/frontend-plugin-api/src/wiring/createPlugin.ts index d7ee418990..ecc4d08f9a 100644 --- a/packages/frontend-plugin-api/src/wiring/createPlugin.ts +++ b/packages/frontend-plugin-api/src/wiring/createPlugin.ts @@ -65,6 +65,25 @@ export function createPlugin< >( options: PluginOptions, ): BackstagePlugin { + const extensions = (options.extensions ?? []).map(def => + resolveExtensionDefinition(def, { namespace: options.id }), + ); + + const extensionIds = extensions.map(e => e.id); + if (extensionIds.length !== new Set(extensionIds).size) { + const duplicates = Array.from( + new Set( + extensionIds.filter((id, index) => extensionIds.indexOf(id) !== index), + ), + ); + // TODO(Rugvip): This could provide some more information about the kind + name of the extensions + throw new Error( + `Plugin '${options.id}' provided duplicate extensions: ${duplicates.join( + ', ', + )}`, + ); + } + return { $$type: '@backstage/BackstagePlugin', version: 'v1', @@ -72,9 +91,7 @@ export function createPlugin< routes: options.routes ?? ({} as Routes), externalRoutes: options.externalRoutes ?? ({} as ExternalRoutes), featureFlags: options.featureFlags ?? [], - extensions: (options.extensions ?? []).map(def => - resolveExtensionDefinition(def, { namespace: options.id }), - ), + extensions, } as InternalBackstagePlugin; }