Merge pull request #20032 from backstage/camilaibs/create-extension-instance-validation

[Frontend DI] Create Extension Instance Validations

.changeset/wild-jobs-greet.md

@@ -0,0 +1,5 @@
+---
+'@backstage/frontend-app-api': patch
+---
+
+Prevents root extension override and duplicated plugin extensions.

packages/frontend-app-api/package.json

@@ -24,6 +24,7 @@
   },
   "devDependencies": {
     "@backstage/cli": "workspace:^",
+    "@backstage/test-utils": "workspace:^",
     "@testing-library/jest-dom": "^5.10.1"
   },
   "configSchema": "config.d.ts",

packages/frontend-app-api/src/wiring/createApp.test.tsx

@@ -0,0 +1,106 @@
+/*
+ * 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,
+  createPageExtension,
+  createPlugin,
+} from '@backstage/frontend-plugin-api';
+import { createInstances } from './createApp';
+
+import { MockConfigApi } from '@backstage/test-utils';
+import React from 'react';
+import { createRouteRef } from '@backstage/core-plugin-api';
+
+describe('createInstances', () => {
+  it('throws an error when a root extension is parametrized', () => {
+    const config = new MockConfigApi({
+      app: {
+        extensions: [
+          {
+            root: {
+              at: '',
+            },
+          },
+        ],
+      },
+    });
+    const plugins = [
+      createPlugin({
+        id: 'plugin',
+        extensions: [],
+      }),
+    ];
+    expect(() => createInstances({ config, plugins })).toThrow(
+      "A 'root' extension configuration was detected, but the root extension is not configurable",
+    );
+  });
+
+  it('throws an error when a root extension is overridden', () => {
+    const config = new MockConfigApi({});
+    const plugins = [
+      createPlugin({
+        id: 'plugin',
+        extensions: [
+          createExtension({
+            id: 'root',
+            at: 'core.routes/route',
+            inputs: {},
+            output: {},
+            factory() {},
+          }),
+        ],
+      }),
+    ];
+    expect(() => createInstances({ config, plugins })).toThrow(
+      "The following plugin(s) are overriding the 'root' 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({ id: 'A.route' }),
+      loader: async () => <div>Extension A</div>,
+    });
+
+    const ExtensionB = createPageExtension({
+      id: 'B',
+      defaultPath: '/',
+      routeRef: createRouteRef({ id: 'B.route' }),
+      loader: async () => <div>Extension B</div>,
+    });
+
+    const PluginA = createPlugin({
+      id: 'A',
+      extensions: [ExtensionA, ExtensionA],
+    });
+
+    const PluginB = createPlugin({
+      id: 'B',
+      extensions: [ExtensionA, ExtensionB, ExtensionB],
+    });
+
+    const plugins = [PluginA, PluginB];
+
+    expect(() => createInstances({ config, plugins })).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'",
+    );
+  });
+});

packages/frontend-app-api/src/wiring/createApp.tsx

@@ -209,18 +209,18 @@ export function createInstances(options: {
   function createInstance(
     instanceParams: ExtensionInstanceParameters,
   ): ExtensionInstance {
-    const existingInstance = instances.get(instanceParams.extension.id);
+    const extensionId = instanceParams.extension.id;
+    const existingInstance = instances.get(extensionId);
     if (existingInstance) {
       return existingInstance;
     }
 
     const attachments = new Map(
-      Array.from(
-        attachmentMap.get(instanceParams.extension.id)?.entries() ?? [],
-      ).map(([inputName, attachmentConfigs]) => [
-        inputName,
-        attachmentConfigs.map(createInstance),
-      ]),
+      Array.from(attachmentMap.get(extensionId)?.entries() ?? []).map(
+        ([inputName, attachmentConfigs]) => {
+          return [inputName, attachmentConfigs.map(createInstance)];
+        },
+      ),
     );
 
     const newInstance = createExtensionInstance({
@@ -230,7 +230,7 @@ export function createInstances(options: {
       attachments,
     });
 
-    instances.set(instanceParams.extension.id, newInstance);
+    instances.set(extensionId, newInstance);
 
     return newInstance;
   }

packages/frontend-app-api/src/wiring/parameters.ts

@@ -196,18 +196,32 @@ export function mergeExtensionParameters(options: {
 }): ExtensionInstanceParameters[] {
   const { sources, builtinExtensions, parameters } = options;
 
+  const pluginExtensions = sources.flatMap(source => {
+    return source.extensions.map(extension => ({ ...extension, source }));
+  });
+
+  // Prevent root override
+  if (pluginExtensions.some(({ id }) => id === 'root')) {
+    const rootPluginIds = pluginExtensions
+      .filter(({ id }) => id === 'root')
+      .map(({ source }) => source.id);
+    throw new Error(
+      `The following plugin(s) are overriding the 'root' extension which is forbidden: ${rootPluginIds.join(
+        ',',
+      )}`,
+    );
+  }
+
   const overrides = [
-    ...sources.flatMap(plugin =>
-      plugin.extensions.map(extension => ({
-        extension,
-        params: {
-          source: plugin,
-          at: extension.at,
-          disabled: extension.disabled,
-          config: undefined as unknown,
-        },
-      })),
-    ),
+    ...pluginExtensions.map(({ source, ...extension }) => ({
+      extension,
+      params: {
+        source,
+        at: extension.at,
+        disabled: extension.disabled,
+        config: undefined as unknown,
+      },
+    })),
     ...builtinExtensions.map(extension => ({
       extension,
       params: {
@@ -219,9 +233,53 @@ export function mergeExtensionParameters(options: {
     })),
   ];
 
+  const duplicatedExtensionIds = new Set<string>();
+  const duplicatedExtensionData = overrides.reduce<
+    Record<string, Record<string, number>>
+  >((data, { extension, params }) => {
+    const extensionId = extension.id;
+    const extensionData = data?.[extensionId];
+    if (extensionData) duplicatedExtensionIds.add(extensionId);
+    const pluginId = params.source?.id ?? 'internal';
+    const pluginCount = extensionData?.[pluginId] ?? 0;
+    return {
+      ...data,
+      [extensionId]: { ...extensionData, [pluginId]: pluginCount + 1 },
+    };
+  }, {});
+
+  if (duplicatedExtensionIds.size > 0) {
+    throw new Error(
+      `The following extensions are duplicated: ${Array.from(
+        duplicatedExtensionIds,
+      )
+        .map(
+          extensionId =>
+            `The extension '${extensionId}' was provided ${Object.keys(
+              duplicatedExtensionData[extensionId],
+            )
+              .map(
+                pluginId =>
+                  `${duplicatedExtensionData[extensionId][pluginId]} time(s) by the plugin '${pluginId}'`,
+              )
+              .join(' and ')}`,
+        )
+        .join(', ')}`,
+    );
+  }
+
   for (const overrideParam of parameters) {
+    const extensionId = overrideParam.id;
+
+    // Prevent root parametrization
+    if (extensionId === 'root') {
+      throw new Error(
+        "A 'root' extension configuration was detected, but the root extension is not configurable",
+      );
+    }
+
     const existingIndex = overrides.findIndex(
-      e => e.extension.id === overrideParam.id,
+      e => e.extension.id === extensionId,
     );
     if (existingIndex !== -1) {
       const existing = overrides[existingIndex];
@@ -243,7 +301,7 @@ export function mergeExtensionParameters(options: {
         }
       }
     } else {
-      throw new Error(`Extension ${overrideParam.id} does not exist`);
+      throw new Error(`Extension ${extensionId} does not exist`);
     }
   }
 

yarn.lock

@@ -4213,6 +4213,7 @@ __metadata:
     "@backstage/core-plugin-api": "workspace:^"
     "@backstage/frontend-plugin-api": "workspace:^"
     "@backstage/plugin-graphiql": "workspace:^"
+    "@backstage/test-utils": "workspace:^"
     "@backstage/types": "workspace:^"
     "@material-ui/core": ^4.12.4
     "@testing-library/jest-dom": ^5.10.1