frontend-app-api: restrict the ability for plugins to override APIs
Signed-off-by: Patrik Oldsberg <poldsberg@gmail.com>
This commit is contained in:
@@ -109,6 +109,14 @@ export type AppErrorTypes = {
|
||||
node: AppNode;
|
||||
};
|
||||
};
|
||||
API_FACTORY_CONFLICT: {
|
||||
context: {
|
||||
node: AppNode;
|
||||
apiRefId: string;
|
||||
pluginId: string;
|
||||
existingPluginId: string;
|
||||
};
|
||||
};
|
||||
ROUTE_DUPLICATE: {
|
||||
context: {
|
||||
routeId: string;
|
||||
|
||||
@@ -66,6 +66,14 @@ export type AppErrorTypes = {
|
||||
API_EXTENSION_INVALID: {
|
||||
context: { node: AppNode };
|
||||
};
|
||||
API_FACTORY_CONFLICT: {
|
||||
context: {
|
||||
node: AppNode;
|
||||
apiRefId: string;
|
||||
pluginId: string;
|
||||
existingPluginId: string;
|
||||
};
|
||||
};
|
||||
// routing
|
||||
ROUTE_DUPLICATE: {
|
||||
context: { routeId: string };
|
||||
|
||||
@@ -20,7 +20,9 @@ import {
|
||||
coreExtensionData,
|
||||
createExtension,
|
||||
createFrontendPlugin,
|
||||
createFrontendModule,
|
||||
ApiBlueprint,
|
||||
createApiRef,
|
||||
createRouteRef,
|
||||
createExternalRouteRef,
|
||||
createExtensionInput,
|
||||
@@ -36,21 +38,22 @@ import { MemoryRouter } from 'react-router-dom';
|
||||
import { ApiProvider, ConfigReader } from '@backstage/core-app-api';
|
||||
import { Fragment } from 'react';
|
||||
|
||||
function makeAppPlugin(label: string = 'Test') {
|
||||
return createFrontendPlugin({
|
||||
pluginId: 'app',
|
||||
extensions: [
|
||||
createExtension({
|
||||
attachTo: { id: 'root', input: 'app' },
|
||||
output: [coreExtensionData.reactElement],
|
||||
factory: () => [coreExtensionData.reactElement(<div>{label}</div>)],
|
||||
}),
|
||||
],
|
||||
});
|
||||
}
|
||||
describe('createSpecializedApp', () => {
|
||||
it('should render the root app', () => {
|
||||
const app = createSpecializedApp({
|
||||
features: [
|
||||
createFrontendPlugin({
|
||||
pluginId: 'test',
|
||||
extensions: [
|
||||
createExtension({
|
||||
attachTo: { id: 'root', input: 'app' },
|
||||
output: [coreExtensionData.reactElement],
|
||||
factory: () => [coreExtensionData.reactElement(<div>Test</div>)],
|
||||
}),
|
||||
],
|
||||
}),
|
||||
],
|
||||
features: [makeAppPlugin()],
|
||||
});
|
||||
|
||||
render(app.tree.root.instance!.getData(coreExtensionData.reactElement));
|
||||
@@ -60,32 +63,7 @@ describe('createSpecializedApp', () => {
|
||||
|
||||
it('should deduplicate features keeping the last received one', () => {
|
||||
const app = createSpecializedApp({
|
||||
features: [
|
||||
createFrontendPlugin({
|
||||
pluginId: 'test',
|
||||
extensions: [
|
||||
createExtension({
|
||||
attachTo: { id: 'root', input: 'app' },
|
||||
output: [coreExtensionData.reactElement],
|
||||
factory: () => [
|
||||
coreExtensionData.reactElement(<div>Test 1</div>),
|
||||
],
|
||||
}),
|
||||
],
|
||||
}),
|
||||
createFrontendPlugin({
|
||||
pluginId: 'test',
|
||||
extensions: [
|
||||
createExtension({
|
||||
attachTo: { id: 'root', input: 'app' },
|
||||
output: [coreExtensionData.reactElement],
|
||||
factory: () => [
|
||||
coreExtensionData.reactElement(<div>Test 2</div>),
|
||||
],
|
||||
}),
|
||||
],
|
||||
}),
|
||||
],
|
||||
features: [makeAppPlugin('Test 1'), makeAppPlugin('Test 2')],
|
||||
});
|
||||
|
||||
render(app.tree.root.instance!.getData(coreExtensionData.reactElement));
|
||||
@@ -249,8 +227,9 @@ describe('createSpecializedApp', () => {
|
||||
|
||||
const app = createSpecializedApp({
|
||||
features: [
|
||||
createFrontendPlugin({
|
||||
pluginId: 'first',
|
||||
makeAppPlugin(),
|
||||
createFrontendModule({
|
||||
pluginId: 'app',
|
||||
extensions: [
|
||||
ApiBlueprint.make({
|
||||
params: defineParams =>
|
||||
@@ -264,9 +243,8 @@ describe('createSpecializedApp', () => {
|
||||
}),
|
||||
],
|
||||
}),
|
||||
createFrontendPlugin({
|
||||
pluginId: 'test',
|
||||
featureFlags: [{ name: 'a' }, { name: 'b' }],
|
||||
createFrontendModule({
|
||||
pluginId: 'app',
|
||||
extensions: [
|
||||
createExtension({
|
||||
attachTo: { id: 'root', input: 'app' },
|
||||
@@ -311,6 +289,107 @@ describe('createSpecializedApp', () => {
|
||||
expect(mockAnalyticsApi).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should select the API factory from the owning plugin on conflict', () => {
|
||||
const testApiRef = createApiRef<{ value: string }>({ id: 'test.api' });
|
||||
|
||||
const app = createSpecializedApp({
|
||||
features: [
|
||||
makeAppPlugin(),
|
||||
createFrontendPlugin({
|
||||
pluginId: 'other-before',
|
||||
extensions: [
|
||||
ApiBlueprint.make({
|
||||
params: defineParams =>
|
||||
defineParams({
|
||||
api: testApiRef,
|
||||
deps: {},
|
||||
factory: () => ({ value: 'other' }),
|
||||
}),
|
||||
}),
|
||||
],
|
||||
}),
|
||||
createFrontendPlugin({
|
||||
pluginId: 'test',
|
||||
extensions: [
|
||||
ApiBlueprint.make({
|
||||
params: defineParams =>
|
||||
defineParams({
|
||||
api: testApiRef,
|
||||
deps: {},
|
||||
factory: () => ({ value: 'owner' }),
|
||||
}),
|
||||
}),
|
||||
],
|
||||
}),
|
||||
createFrontendPlugin({
|
||||
pluginId: 'other-after',
|
||||
extensions: [
|
||||
ApiBlueprint.make({
|
||||
params: defineParams =>
|
||||
defineParams({
|
||||
api: testApiRef,
|
||||
deps: {},
|
||||
factory: () => ({ value: 'other' }),
|
||||
}),
|
||||
}),
|
||||
],
|
||||
}),
|
||||
],
|
||||
});
|
||||
|
||||
expect(app.errors).toEqual([
|
||||
expect.objectContaining({
|
||||
code: 'API_FACTORY_CONFLICT',
|
||||
message: expect.stringContaining("API 'test.api'"),
|
||||
}),
|
||||
expect.objectContaining({
|
||||
code: 'API_FACTORY_CONFLICT',
|
||||
message: expect.stringContaining("API 'test.api'"),
|
||||
}),
|
||||
]);
|
||||
|
||||
expect(app.apis.get(testApiRef)).toEqual({ value: 'owner' });
|
||||
});
|
||||
|
||||
it('should allow API overrides within the same plugin', () => {
|
||||
const testApiRef = createApiRef<{ value: string }>({ id: 'test.api' });
|
||||
|
||||
const app = createSpecializedApp({
|
||||
features: [
|
||||
makeAppPlugin(),
|
||||
createFrontendPlugin({
|
||||
pluginId: 'test',
|
||||
extensions: [
|
||||
ApiBlueprint.make({
|
||||
params: defineParams =>
|
||||
defineParams({
|
||||
api: testApiRef,
|
||||
deps: {},
|
||||
factory: () => ({ value: 'plugin' }),
|
||||
}),
|
||||
}),
|
||||
],
|
||||
}),
|
||||
createFrontendModule({
|
||||
pluginId: 'test',
|
||||
extensions: [
|
||||
ApiBlueprint.make({
|
||||
params: defineParams =>
|
||||
defineParams({
|
||||
api: testApiRef,
|
||||
deps: {},
|
||||
factory: () => ({ value: 'module' }),
|
||||
}),
|
||||
}),
|
||||
],
|
||||
}),
|
||||
],
|
||||
});
|
||||
|
||||
expect(app.errors).toBeUndefined();
|
||||
expect(app.apis.get(testApiRef)).toEqual({ value: 'module' });
|
||||
});
|
||||
|
||||
it('should use provided apis', async () => {
|
||||
const app = createSpecializedApp({
|
||||
advanced: {
|
||||
|
||||
@@ -388,7 +388,10 @@ function createApiFactories(options: {
|
||||
collector: ErrorCollector;
|
||||
}): AnyApiFactory[] {
|
||||
const emptyApiHolder = ApiRegistry.from([]);
|
||||
const factories = new Array<AnyApiFactory>();
|
||||
const factoriesById = new Map<
|
||||
string,
|
||||
{ pluginId: string; factory: AnyApiFactory }
|
||||
>();
|
||||
|
||||
for (const apiNode of options.tree.root.edges.attachments.get('apis') ?? []) {
|
||||
if (!instantiateAppNodeTree(apiNode, emptyApiHolder, options.collector)) {
|
||||
@@ -396,7 +399,45 @@ function createApiFactories(options: {
|
||||
}
|
||||
const apiFactory = apiNode.instance?.getData(ApiBlueprint.dataRefs.factory);
|
||||
if (apiFactory) {
|
||||
factories.push(apiFactory);
|
||||
const apiRefId = apiFactory.api.id;
|
||||
const ownerId = getApiOwnerId(apiRefId);
|
||||
const pluginId = apiNode.spec.plugin.id ?? 'app';
|
||||
const existingFactory = factoriesById.get(apiRefId);
|
||||
|
||||
// This allows modules to override factories provided by the plugin, but
|
||||
// it rejects API overrides from other plugins. In the event of a
|
||||
// conflict, the owning plugin is attempted to be inferred from the API
|
||||
// reference ID.
|
||||
if (existingFactory && existingFactory.pluginId !== pluginId) {
|
||||
const shouldReplace =
|
||||
ownerId === pluginId && existingFactory.pluginId !== ownerId;
|
||||
const acceptedPluginId = shouldReplace
|
||||
? pluginId
|
||||
: existingFactory.pluginId;
|
||||
const rejectedPluginId = shouldReplace
|
||||
? existingFactory.pluginId
|
||||
: pluginId;
|
||||
|
||||
options.collector.report({
|
||||
code: 'API_FACTORY_CONFLICT',
|
||||
message: `API '${apiRefId}' is already provided by plugin '${acceptedPluginId}', cannot also be provided by '${rejectedPluginId}'.`,
|
||||
context: {
|
||||
node: apiNode,
|
||||
apiRefId,
|
||||
pluginId: rejectedPluginId,
|
||||
existingPluginId: acceptedPluginId,
|
||||
},
|
||||
});
|
||||
if (shouldReplace) {
|
||||
factoriesById.set(apiRefId, {
|
||||
pluginId,
|
||||
factory: apiFactory,
|
||||
});
|
||||
}
|
||||
continue;
|
||||
}
|
||||
|
||||
factoriesById.set(apiRefId, { pluginId, factory: apiFactory });
|
||||
} else {
|
||||
options.collector.report({
|
||||
code: 'API_EXTENSION_INVALID',
|
||||
@@ -408,7 +449,23 @@ function createApiFactories(options: {
|
||||
}
|
||||
}
|
||||
|
||||
return factories;
|
||||
return Array.from(factoriesById.values(), entry => entry.factory);
|
||||
}
|
||||
|
||||
// TODO(Rugvip): It would be good if this was more explicit, but I think that
|
||||
// might need to wait for some future update for API factories.
|
||||
function getApiOwnerId(apiRefId: string): string {
|
||||
const [prefix, ...rest] = apiRefId.split('.');
|
||||
if (!prefix) {
|
||||
return apiRefId;
|
||||
}
|
||||
if (prefix === 'core') {
|
||||
return 'app';
|
||||
}
|
||||
if (prefix === 'plugin' && rest[0]) {
|
||||
return rest[0];
|
||||
}
|
||||
return prefix;
|
||||
}
|
||||
|
||||
function createApiHolder(options: {
|
||||
|
||||
Reference in New Issue
Block a user