UI Extensions: Rename placement to extensionPointId (#65841)

* UI Extensions: Rename `placement` to `extensionPointId`

* Fix tests.

* Fix conflicts and review notes

* Remove `placement` from some other places
This commit is contained in:
Levente Balogh
2023-04-03 17:59:54 +02:00
committed by GitHub
parent 44ccd73d46
commit 1380fa54d6
16 changed files with 126 additions and 124 deletions

View File

@@ -1 +1 @@
export const MAX_EXTENSIONS_PER_PLACEMENT_PER_PLUGIN = 2;
export const MAX_EXTENSIONS_PER_POINT = 2;

View File

@@ -14,7 +14,7 @@ describe('createRegistry()', () => {
title: 'Link 1',
description: 'Link 1 description',
path: `/a/${pluginId}/declare-incident`,
placement: placement1,
extensionPointId: placement1,
configure: jest.fn().mockReturnValue({}),
};
link2 = {
@@ -22,7 +22,7 @@ describe('createRegistry()', () => {
title: 'Link 2',
description: 'Link 2 description',
path: `/a/${pluginId}/declare-incident`,
placement: placement2,
extensionPointId: placement2,
configure: jest.fn().mockImplementation((context) => ({ title: context?.title })),
};

View File

@@ -1,14 +1,14 @@
import type { PluginPreloadResult } from '../pluginPreloader';
import { MAX_EXTENSIONS_PER_PLACEMENT_PER_PLUGIN } from './constants';
import { PlacementsPerPlugin } from './placementsPerPlugin';
import { MAX_EXTENSIONS_PER_POINT } from './constants';
import { ExtensionsPerPlugin } from './extensionsPerPlugin';
import type { PluginExtensionRegistryItem, PluginExtensionRegistry } from './types';
import { deepFreeze, logWarning } from './utils';
import { isPluginExtensionConfigValid } from './validators';
export function createPluginExtensionRegistry(pluginPreloadResults: PluginPreloadResult[]): PluginExtensionRegistry {
const registry: PluginExtensionRegistry = {};
const placementsPerPlugin = new PlacementsPerPlugin();
const extensionsPerPlugin = new ExtensionsPerPlugin();
for (const { pluginId, extensionConfigs, error } of pluginPreloadResults) {
if (error) {
@@ -17,11 +17,11 @@ export function createPluginExtensionRegistry(pluginPreloadResults: PluginPreloa
}
for (const extensionConfig of extensionConfigs) {
const { placement } = extensionConfig;
const { extensionPointId } = extensionConfig;
if (!placementsPerPlugin.allowedToAdd(extensionConfig)) {
if (!extensionsPerPlugin.allowedToAdd(extensionConfig)) {
logWarning(
`"${pluginId}" plugin has reached the limit of ${MAX_EXTENSIONS_PER_PLACEMENT_PER_PLUGIN} for "${placement}", skip registering extension "${extensionConfig.title}".`
`"${pluginId}" plugin has reached the limit of ${MAX_EXTENSIONS_PER_POINT} for "${extensionPointId}", skip registering extension "${extensionConfig.title}".`
);
continue;
}
@@ -37,10 +37,10 @@ export function createPluginExtensionRegistry(pluginPreloadResults: PluginPreloa
pluginId,
};
if (!Array.isArray(registry[placement])) {
registry[placement] = [registryItem];
if (!Array.isArray(registry[extensionPointId])) {
registry[extensionPointId] = [registryItem];
} else {
registry[placement].push(registryItem);
registry[extensionPointId].push(registryItem);
}
}
}

View File

@@ -0,0 +1,33 @@
import { PluginExtensionLinkConfig } from '@grafana/data';
import { MAX_EXTENSIONS_PER_POINT } from './constants';
export class ExtensionsPerPlugin {
private extensionsByExtensionPoint: Record<string, string[]> = {};
allowedToAdd({ extensionPointId, title }: PluginExtensionLinkConfig): boolean {
if (this.countByExtensionPoint(extensionPointId) >= MAX_EXTENSIONS_PER_POINT) {
return false;
}
this.addExtensionToExtensionPoint(extensionPointId, title);
return true;
}
addExtensionToExtensionPoint(extensionPointId: string, extensionTitle: string) {
if (!this.extensionsByExtensionPoint[extensionPointId]) {
this.extensionsByExtensionPoint[extensionPointId] = [];
}
this.extensionsByExtensionPoint[extensionPointId].push(extensionTitle);
}
countByExtensionPoint(extensionPointId: string) {
return this.extensionsByExtensionPoint[extensionPointId]?.length ?? 0;
}
getExtensionTitlesByExtensionPoint(extensionPointId: string) {
return this.extensionsByExtensionPoint[extensionPointId];
}
}

View File

@@ -5,8 +5,8 @@ import { getPluginExtensions } from './getPluginExtensions';
import { assertPluginExtensionLink } from './validators';
describe('getPluginExtensions()', () => {
const placement1 = 'grafana/dashboard/panel/menu';
const placement2 = 'plugins/myorg-basic-app/start';
const extensionPoint1 = 'grafana/dashboard/panel/menu';
const extensionPoint2 = 'plugins/myorg-basic-app/start';
const pluginId = 'grafana-basic-app';
let link1: PluginExtensionLinkConfig, link2: PluginExtensionLinkConfig;
@@ -16,7 +16,7 @@ describe('getPluginExtensions()', () => {
title: 'Link 1',
description: 'Link 1 description',
path: `/a/${pluginId}/declare-incident`,
placement: placement1,
extensionPointId: extensionPoint1,
configure: jest.fn().mockReturnValue({}),
};
link2 = {
@@ -24,7 +24,7 @@ describe('getPluginExtensions()', () => {
title: 'Link 2',
description: 'Link 2 description',
path: `/a/${pluginId}/declare-incident`,
placement: placement2,
extensionPointId: extensionPoint2,
configure: jest.fn().mockImplementation((context) => ({ title: context?.title })),
};
@@ -33,7 +33,7 @@ describe('getPluginExtensions()', () => {
test('should return the extensions for the given placement', () => {
const registry = createPluginExtensionRegistry([{ pluginId, extensionConfigs: [link1, link2] }]);
const { extensions } = getPluginExtensions({ registry, placement: placement1 });
const { extensions } = getPluginExtensions({ registry, extensionPointId: extensionPoint1 });
expect(extensions).toHaveLength(1);
expect(extensions[0]).toEqual(
@@ -49,7 +49,7 @@ describe('getPluginExtensions()', () => {
test('should return with an empty list if there are no extensions registered for a placement yet', () => {
const registry = createPluginExtensionRegistry([{ pluginId, extensionConfigs: [link1, link2] }]);
const { extensions } = getPluginExtensions({ registry, placement: 'placement-with-no-extensions' });
const { extensions } = getPluginExtensions({ registry, extensionPointId: 'placement-with-no-extensions' });
expect(extensions).toEqual([]);
});
@@ -58,7 +58,7 @@ describe('getPluginExtensions()', () => {
const context = { title: 'New title from the context!' };
const registry = createPluginExtensionRegistry([{ pluginId, extensionConfigs: [link2] }]);
getPluginExtensions({ registry, context, placement: placement2 });
getPluginExtensions({ registry, context, extensionPointId: extensionPoint2 });
expect(link2.configure).toHaveBeenCalledTimes(1);
expect(link2.configure).toHaveBeenCalledWith(context);
@@ -72,7 +72,7 @@ describe('getPluginExtensions()', () => {
}));
const registry = createPluginExtensionRegistry([{ pluginId, extensionConfigs: [link2] }]);
const { extensions } = getPluginExtensions({ registry, placement: placement2 });
const { extensions } = getPluginExtensions({ registry, extensionPointId: extensionPoint2 });
const [extension] = extensions;
assertPluginExtensionLink(extension);
@@ -91,7 +91,7 @@ describe('getPluginExtensions()', () => {
}));
const registry = createPluginExtensionRegistry([{ pluginId, extensionConfigs: [link2] }]);
const { extensions } = getPluginExtensions({ registry, placement: placement2 });
const { extensions } = getPluginExtensions({ registry, extensionPointId: extensionPoint2 });
expect(link2.configure).toHaveBeenCalledTimes(1);
expect(extensions).toHaveLength(0);
@@ -99,7 +99,7 @@ describe('getPluginExtensions()', () => {
test('should pass a frozen copy of the context to the configure() function', () => {
const context = { title: 'New title from the context!' };
const registry = createPluginExtensionRegistry([{ pluginId, extensionConfigs: [link2] }]);
const { extensions } = getPluginExtensions({ registry, context, placement: placement2 });
const { extensions } = getPluginExtensions({ registry, context, extensionPointId: extensionPoint2 });
const [extension] = extensions;
const frozenContext = (link2.configure as jest.Mock).mock.calls[0][0];
@@ -121,7 +121,7 @@ describe('getPluginExtensions()', () => {
const registry = createPluginExtensionRegistry([{ pluginId, extensionConfigs: [link2] }]);
expect(() => {
getPluginExtensions({ registry, placement: placement2 });
getPluginExtensions({ registry, extensionPointId: extensionPoint2 });
}).not.toThrow();
expect(link2.configure).toHaveBeenCalledTimes(1);
@@ -138,8 +138,8 @@ describe('getPluginExtensions()', () => {
}));
const registry = createPluginExtensionRegistry([{ pluginId, extensionConfigs: [link1, link2] }]);
const { extensions: extensionsAtPlacement1 } = getPluginExtensions({ registry, placement: placement1 });
const { extensions: extensionsAtPlacement2 } = getPluginExtensions({ registry, placement: placement2 });
const { extensions: extensionsAtPlacement1 } = getPluginExtensions({ registry, extensionPointId: extensionPoint1 });
const { extensions: extensionsAtPlacement2 } = getPluginExtensions({ registry, extensionPointId: extensionPoint2 });
expect(extensionsAtPlacement1).toHaveLength(0);
expect(extensionsAtPlacement2).toHaveLength(0);
@@ -158,7 +158,7 @@ describe('getPluginExtensions()', () => {
link2.configure = jest.fn().mockImplementation(() => overrides);
const registry = createPluginExtensionRegistry([{ pluginId, extensionConfigs: [link2] }]);
const { extensions } = getPluginExtensions({ registry, placement: placement2 });
const { extensions } = getPluginExtensions({ registry, extensionPointId: extensionPoint2 });
expect(extensions).toHaveLength(0);
expect(link2.configure).toHaveBeenCalledTimes(1);
@@ -169,7 +169,7 @@ describe('getPluginExtensions()', () => {
link2.configure = jest.fn().mockImplementation(() => Promise.resolve({}));
const registry = createPluginExtensionRegistry([{ pluginId, extensionConfigs: [link2] }]);
const { extensions } = getPluginExtensions({ registry, placement: placement2 });
const { extensions } = getPluginExtensions({ registry, extensionPointId: extensionPoint2 });
expect(extensions).toHaveLength(0);
expect(link2.configure).toHaveBeenCalledTimes(1);
@@ -180,7 +180,7 @@ describe('getPluginExtensions()', () => {
link2.configure = jest.fn().mockImplementation(() => undefined);
const registry = createPluginExtensionRegistry([{ pluginId, extensionConfigs: [link2] }]);
const { extensions } = getPluginExtensions({ registry, placement: placement2 });
const { extensions } = getPluginExtensions({ registry, extensionPointId: extensionPoint2 });
expect(extensions).toHaveLength(0);
expect(global.console.warn).toHaveBeenCalledTimes(0); // As this is intentional, no warning should be logged
@@ -194,7 +194,7 @@ describe('getPluginExtensions()', () => {
const context = {};
const registry = createPluginExtensionRegistry([{ pluginId, extensionConfigs: [link2] }]);
const { extensions } = getPluginExtensions({ registry, placement: placement2 });
const { extensions } = getPluginExtensions({ registry, extensionPointId: extensionPoint2 });
const [extension] = extensions;
assertPluginExtensionLink(extension);
@@ -217,7 +217,7 @@ describe('getPluginExtensions()', () => {
link2.onClick = jest.fn().mockRejectedValue(new Error('testing'));
const registry = createPluginExtensionRegistry([{ pluginId, extensionConfigs: [link2] }]);
const { extensions } = getPluginExtensions({ registry, placement: placement2 });
const { extensions } = getPluginExtensions({ registry, extensionPointId: extensionPoint2 });
const [extension] = extensions;
assertPluginExtensionLink(extension);
@@ -236,7 +236,7 @@ describe('getPluginExtensions()', () => {
});
const registry = createPluginExtensionRegistry([{ pluginId, extensionConfigs: [link2] }]);
const { extensions } = getPluginExtensions({ registry, placement: placement2 });
const { extensions } = getPluginExtensions({ registry, extensionPointId: extensionPoint2 });
const [extension] = extensions;
assertPluginExtensionLink(extension);

View File

@@ -11,18 +11,18 @@ import { assertIsNotPromise, assertLinkPathIsValid, assertStringProps, isPromise
type GetExtensions = ({
context,
placement,
extensionPointId,
registry,
}: {
context?: object | Record<string | symbol, unknown>;
placement: string;
extensionPointId: string;
registry: PluginExtensionRegistry;
}) => { extensions: PluginExtension[] };
// Returns with a list of plugin extensions for the given placement
export const getPluginExtensions: GetExtensions = ({ context, placement, registry }) => {
// Returns with a list of plugin extensions for the given extension point
export const getPluginExtensions: GetExtensions = ({ context, extensionPointId, registry }) => {
const frozenContext = context ? deepFreeze(context) : {};
const registryItems = registry[placement] ?? [];
const registryItems = registry[extensionPointId] ?? [];
// We don't return the extensions separated by type, because in that case it would be much harder to define a sort-order for them.
const extensions: PluginExtension[] = [];

View File

@@ -1,33 +0,0 @@
import { PluginExtensionLinkConfig } from '@grafana/data';
import { MAX_EXTENSIONS_PER_PLACEMENT_PER_PLUGIN } from './constants';
export class PlacementsPerPlugin {
private extensionsByPlacement: Record<string, string[]> = {};
allowedToAdd({ placement, title }: PluginExtensionLinkConfig): boolean {
if (this.countByPlacement(placement) >= MAX_EXTENSIONS_PER_PLACEMENT_PER_PLUGIN) {
return false;
}
this.addExtensionToPlacement(placement, title);
return true;
}
addExtensionToPlacement(placement: string, extensionTitle: string) {
if (!this.extensionsByPlacement[placement]) {
this.extensionsByPlacement[placement] = [];
}
this.extensionsByPlacement[placement].push(extensionTitle);
}
countByPlacement(placement: string) {
return this.extensionsByPlacement[placement]?.length ?? 0;
}
getExtensionTitlesByPlacement(placement: string) {
return this.extensionsByPlacement[placement];
}
}

View File

@@ -104,7 +104,7 @@ export function deepFreeze(value?: object | Record<string | symbol, unknown> | u
}
export function generateExtensionId(pluginId: string, extensionConfig: PluginExtensionConfig): string {
const str = `${pluginId}${extensionConfig.placement}${extensionConfig.title}`;
const str = `${pluginId}${extensionConfig.extensionPointId}${extensionConfig.title}`;
return Array.from(str)
.reduce((s, c) => (Math.imul(31, s) + c.charCodeAt(0)) | 0, 0)

View File

@@ -3,7 +3,7 @@ import { PluginExtension, PluginExtensionLinkConfig, PluginExtensionTypes } from
import {
assertConfigureIsValid,
assertLinkPathIsValid,
assertPlacementIsValid,
assertExtensionPointIdIsValid,
assertPluginExtensionLink,
assertStringProps,
isPluginExtensionConfigValid,
@@ -43,7 +43,7 @@ describe('Plugin Extension Validators', () => {
path: `/a/${pluginId}/overview`,
title: 'My Plugin',
description: 'My Plugin Description',
placement: '...',
extensionPointId: '...',
};
assertLinkPathIsValid(pluginId, extension.path);
@@ -56,7 +56,7 @@ describe('Plugin Extension Validators', () => {
path: `/a/myorg-b-app/overview`,
title: 'My Plugin',
description: 'My Plugin Description',
placement: '...',
extensionPointId: '...',
};
assertLinkPathIsValid('another-plugin-app', extension.path);
@@ -69,7 +69,7 @@ describe('Plugin Extension Validators', () => {
path: `/some-bad-path`,
title: 'My Plugin',
description: 'My Plugin Description',
placement: '...',
extensionPointId: '...',
};
assertLinkPathIsValid('myorg-b-app', extension.path);
@@ -77,35 +77,35 @@ describe('Plugin Extension Validators', () => {
});
});
describe('assertPlacementIsValid()', () => {
it('should throw an error if the placement does not have the right prefix', () => {
describe('assertExtensionPointIdIsValid()', () => {
it('should throw an error if the extensionPointId does not have the right prefix', () => {
expect(() => {
assertPlacementIsValid({
assertExtensionPointIdIsValid({
type: PluginExtensionTypes.link,
title: 'Title',
description: 'Description',
path: '...',
placement: 'some-bad-placement',
extensionPointId: 'wrong-extension-point-id',
});
}).toThrowError();
});
it('should NOT throw an error if the placement is correct', () => {
it('should NOT throw an error if the extensionPointId is correct', () => {
expect(() => {
assertPlacementIsValid({
assertExtensionPointIdIsValid({
type: PluginExtensionTypes.link,
title: 'Title',
description: 'Description',
path: '...',
placement: 'grafana/some-page/some-placement',
extensionPointId: 'grafana/some-page/extension-point-a',
});
assertPlacementIsValid({
assertExtensionPointIdIsValid({
type: PluginExtensionTypes.link,
title: 'Title',
description: 'Description',
path: '...',
placement: 'plugins/my-super-plugin/some-page/some-placement',
extensionPointId: 'plugins/my-super-plugin/some-page/extension-point-a',
});
}).not.toThrowError();
});
@@ -117,7 +117,7 @@ describe('Plugin Extension Validators', () => {
assertConfigureIsValid({
title: 'Title',
description: 'Description',
placement: 'grafana/some-page/some-placement',
extensionPointId: 'grafana/some-page/extension-point-a',
} as PluginExtensionLinkConfig);
}).not.toThrowError();
});
@@ -127,7 +127,7 @@ describe('Plugin Extension Validators', () => {
assertConfigureIsValid({
title: 'Title',
description: 'Description',
placement: 'grafana/some-page/some-placement',
extensionPointId: 'grafana/some-page/extension-point-a',
configure: () => {},
} as PluginExtensionLinkConfig);
}).not.toThrowError();
@@ -140,7 +140,7 @@ describe('Plugin Extension Validators', () => {
{
title: 'Title',
description: 'Description',
placement: 'grafana/some-page/some-placement',
extensionPointId: 'grafana/some-page/extension-point-a',
handler: () => {},
configure: '() => {}',
} as PluginExtensionLinkConfig
@@ -155,9 +155,9 @@ describe('Plugin Extension Validators', () => {
assertStringProps(
{
description: 'Description',
placement: 'grafana/some-page/some-placement',
extensionPointId: 'grafana/some-page/extension-point-a',
},
['title', 'description', 'placement']
['title', 'description', 'extensionPointId']
);
}).toThrowError();
});
@@ -168,9 +168,9 @@ describe('Plugin Extension Validators', () => {
{
title: '',
description: 'Description',
placement: 'grafana/some-page/some-placement',
extensionPointId: 'grafana/some-page/extension-point-a',
},
['title', 'description', 'placement']
['title', 'description', 'extensionPointId']
);
}).toThrowError();
});
@@ -181,9 +181,9 @@ describe('Plugin Extension Validators', () => {
{
title: 'Title',
description: 'Description',
placement: 'grafana/some-page/some-placement',
extensionPointId: 'grafana/some-page/extension-point-a',
},
['title', 'description', 'placement']
['title', 'description', 'extensionPointId']
);
}).not.toThrowError();
});
@@ -194,10 +194,10 @@ describe('Plugin Extension Validators', () => {
{
title: 'Title',
description: 'Description',
placement: 'grafana/some-page/some-placement',
extensionPointId: 'grafana/some-page/extension-point-a',
dontCare: '',
},
['title', 'description', 'placement']
['title', 'description', 'extensionPointId']
);
}).not.toThrowError();
});
@@ -212,8 +212,8 @@ describe('Plugin Extension Validators', () => {
type: PluginExtensionTypes.link,
title: 'Title',
description: 'Description',
placement: 'grafana/some-page/some-placement',
onClick: jest.fn(),
extensionPointId: 'grafana/some-page/extension-point-a',
} as PluginExtensionLinkConfig)
).toBe(true);
@@ -222,7 +222,7 @@ describe('Plugin Extension Validators', () => {
type: PluginExtensionTypes.link,
title: 'Title',
description: 'Description',
placement: 'grafana/some-page/some-placement',
extensionPointId: 'grafana/some-page/extension-point-a',
path: `/a/${pluginId}/page`,
} as PluginExtensionLinkConfig)
).toBe(true);
@@ -239,7 +239,7 @@ describe('Plugin Extension Validators', () => {
type: PluginExtensionTypes.link,
title: 'Title',
description: 'Description',
placement: 'grafana/some-page/some-placement',
extensionPointId: 'grafana/some-page/extension-point-a',
path: '/administration/users',
} as PluginExtensionLinkConfig)
).toBe(false);
@@ -250,7 +250,7 @@ describe('Plugin Extension Validators', () => {
type: PluginExtensionTypes.link,
title: 'Title',
description: 'Description',
placement: 'grafana/some-page/some-placement',
extensionPointId: 'grafana/some-page/extension-point-a',
} as PluginExtensionLinkConfig)
).toBe(false);
@@ -260,7 +260,7 @@ describe('Plugin Extension Validators', () => {
type: PluginExtensionTypes.link,
title: '',
description: 'Description',
placement: 'grafana/some-page/some-placement',
extensionPointId: 'grafana/some-page/extension-point-a',
path: `/a/${pluginId}/page`,
} as PluginExtensionLinkConfig)
).toBe(false);

View File

@@ -29,10 +29,10 @@ export function assertLinkPathIsValid(pluginId: string, path: string) {
}
}
export function assertPlacementIsValid(extension: PluginExtensionLinkConfig) {
if (!isPlacementValid(extension)) {
export function assertExtensionPointIdIsValid(extension: PluginExtensionLinkConfig) {
if (!isExtensionPointIdValid(extension)) {
throw new Error(
`Invalid extension "${extension.title}". The placement should start with either "grafana/" or "plugins/" (currently: "${extension.placement}"). Skipping the extension.`
`Invalid extension "${extension.title}". The extensionPointId should start with either "grafana/" or "plugins/" (currently: "${extension.extensionPointId}"). Skipping the extension.`
);
}
}
@@ -65,8 +65,10 @@ export function isLinkPathValid(pluginId: string, path: string) {
return Boolean(typeof path === 'string' && path.length > 0 && path.startsWith(`/a/${pluginId}/`));
}
export function isPlacementValid(extension: PluginExtensionLinkConfig) {
return Boolean(extension.placement?.startsWith('grafana/') || extension.placement?.startsWith('plugins/'));
export function isExtensionPointIdValid(extension: PluginExtensionLinkConfig) {
return Boolean(
extension.extensionPointId?.startsWith('grafana/') || extension.extensionPointId?.startsWith('plugins/')
);
}
export function isConfigureFnValid(extension: PluginExtensionLinkConfig) {
@@ -79,8 +81,8 @@ export function isStringPropValid(prop: unknown) {
export function isPluginExtensionConfigValid(pluginId: string, extension: PluginExtensionLinkConfig): boolean {
try {
assertStringProps(extension, ['title', 'description', 'placement']);
assertPlacementIsValid(extension);
assertStringProps(extension, ['title', 'description', 'extensionPointId']);
assertExtensionPointIdIsValid(extension);
assertConfigureIsValid(extension);
if (isPluginExtensionLinkConfig(extension)) {