diff --git a/public/app/features/plugins/extensions/usePluginComponents.test.tsx b/public/app/features/plugins/extensions/usePluginComponents.test.tsx index 255a8a6161c..d5740d50b5a 100644 --- a/public/app/features/plugins/extensions/usePluginComponents.test.tsx +++ b/public/app/features/plugins/extensions/usePluginComponents.test.tsx @@ -11,6 +11,7 @@ import { import { config } from '@grafana/runtime'; import { ExtensionRegistriesProvider } from './ExtensionRegistriesContext'; +import * as errors from './errors'; import { log } from './logs/log'; import { resetLogMock } from './logs/testUtils'; import { AddedComponentsRegistry } from './registry/AddedComponentsRegistry'; @@ -28,7 +29,7 @@ jest.mock('./utils', () => ({ ...jest.requireActual('./utils'), // Manually set the dev mode to false - // (to make sure that by default we are testing a production scneario) + // (to make sure that by default we are testing a production scenario) isGrafanaDevMode: jest.fn().mockReturnValue(false), })); @@ -486,7 +487,7 @@ describe('usePluginComponents()', () => { // Trying to render an extension point that is not defined in the plugin meta // (No restrictions due to isGrafanaDevMode() = false) - let { result } = renderHook(() => usePluginComponents({ extensionPointId }), { wrapper }); + const { result } = renderHook(() => usePluginComponents({ extensionPointId }), { wrapper }); expect(result.current.components.length).toBe(1); expect(log.error).not.toHaveBeenCalled(); }); @@ -529,7 +530,7 @@ describe('usePluginComponents()', () => { // Trying to render an extension point that is not defined in the plugin meta // (No restrictions due to being a core plugin) - let { result } = renderHook(() => usePluginComponents({ extensionPointId }), { wrapper }); + const { result } = renderHook(() => usePluginComponents({ extensionPointId }), { wrapper }); expect(result.current.components.length).toBe(1); expect(log.error).not.toHaveBeenCalled(); }); @@ -552,7 +553,7 @@ describe('usePluginComponents()', () => { // Trying to render an extension point that is not defined in the plugin meta // (No restrictions due to isGrafanaDevMode() = false) - let { result } = renderHook(() => usePluginComponents({ extensionPointId: 'invalid-extension-point-id' }), { + const { result } = renderHook(() => usePluginComponents({ extensionPointId: 'invalid-extension-point-id' }), { wrapper, }); expect(result.current.components.length).toBe(0); @@ -581,7 +582,7 @@ describe('usePluginComponents()', () => { ], }); - let { result } = renderHook( + const { result } = renderHook( () => usePluginComponents({ extensionPointId: PluginExtensionPoints.DashboardPanelMenu }), { wrapper, @@ -615,7 +616,7 @@ describe('usePluginComponents()', () => { ], }); - let { result } = renderHook(() => usePluginComponents({ extensionPointId }), { + const { result } = renderHook(() => usePluginComponents({ extensionPointId }), { wrapper, }); expect(result.current.components.length).toBe(0); @@ -655,9 +656,10 @@ describe('usePluginComponents()', () => { }); // Trying to render an extension point that is not defined in the plugin meta - let { result } = renderHook(() => usePluginComponents({ extensionPointId }), { wrapper }); + const { result } = renderHook(() => usePluginComponents({ extensionPointId }), { wrapper }); expect(result.current.components.length).toBe(0); expect(log.error).toHaveBeenCalled(); + expect(log.error).toHaveBeenCalledWith(errors.EXTENSION_POINT_META_INFO_MISSING); }); it('should not log an error if the extension point meta-info is correct if in dev-mode and used by a plugin', () => { @@ -699,7 +701,7 @@ describe('usePluginComponents()', () => { }); // Trying to render an extension point that is not defined in the plugin meta - let { result } = renderHook(() => usePluginComponents({ extensionPointId }), { wrapper }); + const { result } = renderHook(() => usePluginComponents({ extensionPointId }), { wrapper }); expect(result.current.components.length).toBe(0); expect(log.error).toHaveBeenCalled(); }); diff --git a/public/app/features/plugins/extensions/usePluginComponents.tsx b/public/app/features/plugins/extensions/usePluginComponents.tsx index d07a2e5ac07..8126838f87a 100644 --- a/public/app/features/plugins/extensions/usePluginComponents.tsx +++ b/public/app/features/plugins/extensions/usePluginComponents.tsx @@ -10,12 +10,10 @@ import { import { UsePluginComponentsOptions, UsePluginComponentsResult } from '@grafana/runtime'; import { useAddedComponentsRegistry } from './ExtensionRegistriesContext'; -import * as errors from './errors'; -import { log } from './logs/log'; import { AddedComponentRegistryItem } from './registry/AddedComponentsRegistry'; import { useLoadAppPlugins } from './useLoadAppPlugins'; -import { generateExtensionId, getExtensionPointPluginDependencies, isGrafanaDevMode } from './utils'; -import { isExtensionPointIdValid, isExtensionPointMetaInfoMissing } from './validators'; +import { generateExtensionId, getExtensionPointPluginDependencies } from './utils'; +import { validateExtensionPoint } from './validateExtensionPoint'; // Returns an array of component extensions for the given extension point export function usePluginComponents({ @@ -28,47 +26,17 @@ export function usePluginComponents({ const { isLoading: isLoadingAppPlugins } = useLoadAppPlugins(getExtensionPointPluginDependencies(extensionPointId)); return useMemo(() => { - const isInsidePlugin = Boolean(pluginContext); - const isCoreGrafanaPlugin = pluginContext?.meta.module.startsWith('core:') ?? false; + const { result } = validateExtensionPoint({ extensionPointId, pluginContext, isLoadingAppPlugins }); + + if (result) { + return { + isLoading: result.isLoading, + components: [], + }; + } + const components: Array> = []; const extensionsByPlugin: Record = {}; - const pluginId = pluginContext?.meta.id ?? ''; - const pointLog = log.child({ - pluginId, - extensionPointId, - }); - - // Don't show extensions if the extension-point id is invalid in DEV mode - if ( - isGrafanaDevMode() && - !isExtensionPointIdValid({ extensionPointId, pluginId, isInsidePlugin, isCoreGrafanaPlugin, log: pointLog }) - ) { - return { - isLoading: false, - components: [], - }; - } - - // Don't show extensions if the extension-point misses meta info (plugin.json) in DEV mode - if ( - isGrafanaDevMode() && - !isCoreGrafanaPlugin && - pluginContext && - isExtensionPointMetaInfoMissing(extensionPointId, pluginContext) - ) { - pointLog.error(errors.EXTENSION_POINT_META_INFO_MISSING); - return { - isLoading: false, - components: [], - }; - } - - if (isLoadingAppPlugins) { - return { - isLoading: true, - components: [], - }; - } for (const registryItem of registryState?.[extensionPointId] ?? []) { const { pluginId } = registryItem; diff --git a/public/app/features/plugins/extensions/usePluginFunctions.test.tsx b/public/app/features/plugins/extensions/usePluginFunctions.test.tsx new file mode 100644 index 00000000000..19e3a20fc83 --- /dev/null +++ b/public/app/features/plugins/extensions/usePluginFunctions.test.tsx @@ -0,0 +1,502 @@ +import { act, renderHook } from '@testing-library/react'; + +import { + PluginContextProvider, + PluginExtensionPoints, + PluginLoadingStrategy, + PluginMeta, + PluginType, +} from '@grafana/data'; +import { config } from '@grafana/runtime'; + +import { ExtensionRegistriesProvider } from './ExtensionRegistriesContext'; +import * as errors from './errors'; +import { log } from './logs/log'; +import { resetLogMock } from './logs/testUtils'; +import { AddedComponentsRegistry } from './registry/AddedComponentsRegistry'; +import { AddedFunctionsRegistry } from './registry/AddedFunctionsRegistry'; +import { AddedLinksRegistry } from './registry/AddedLinksRegistry'; +import { ExposedComponentsRegistry } from './registry/ExposedComponentsRegistry'; +import { PluginExtensionRegistries } from './registry/types'; +import { useLoadAppPlugins } from './useLoadAppPlugins'; +import { usePluginFunctions } from './usePluginFunctions'; +import { isGrafanaDevMode } from './utils'; + +jest.mock('./useLoadAppPlugins'); +jest.mock('app/features/plugins/pluginSettings', () => ({ + getPluginSettings: jest.fn().mockResolvedValue({ + id: 'my-app-plugin', + enabled: true, + jsonData: {}, + type: 'panel', + name: 'My App Plugin', + module: 'app/plugins/my-app-plugin/module', + }), +})); + +jest.mock('./utils', () => ({ + ...jest.requireActual('./utils'), + + // Manually set the dev mode to false + // (to make sure that by default we are testing a production scenario) + isGrafanaDevMode: jest.fn().mockReturnValue(false), +})); + +jest.mock('./logs/log', () => { + const { createLogMock } = jest.requireActual('./logs/testUtils'); + const original = jest.requireActual('./logs/log'); + + return { + ...original, + log: createLogMock(), + }; +}); + +describe('usePluginFunctions()', () => { + let registries: PluginExtensionRegistries; + let wrapper: ({ children }: { children: React.ReactNode }) => JSX.Element; + let pluginMeta: PluginMeta; + const pluginId = 'myorg-extensions-app'; + const extensionPointId = `${pluginId}/extension-point/v1`; + + beforeEach(() => { + jest.mocked(useLoadAppPlugins).mockReturnValue({ isLoading: false }); + jest.mocked(isGrafanaDevMode).mockReturnValue(false); + registries = { + addedComponentsRegistry: new AddedComponentsRegistry(), + exposedComponentsRegistry: new ExposedComponentsRegistry(), + addedLinksRegistry: new AddedLinksRegistry(), + addedFunctionsRegistry: new AddedFunctionsRegistry(), + }; + resetLogMock(log); + + pluginMeta = { + id: pluginId, + name: 'Extensions App', + type: PluginType.app, + module: '', + baseUrl: '', + info: { + author: { + name: 'MyOrg', + }, + description: 'App for testing extensions', + links: [], + logos: { + large: '', + small: '', + }, + screenshots: [], + updated: '2023-10-26T18:25:01Z', + version: '1.0.0', + }, + extensions: { + addedLinks: [], + addedComponents: [], + exposedComponents: [], + extensionPoints: [], + addedFunctions: [], + }, + dependencies: { + grafanaVersion: '8.0.0', + plugins: [], + extensions: { + exposedComponents: [], + }, + }, + }; + + config.apps[pluginId] = { + id: pluginId, + path: '', + version: '', + preload: false, + angular: { + detected: false, + hideDeprecation: false, + }, + loadingStrategy: PluginLoadingStrategy.fetch, + dependencies: { + grafanaVersion: '8.0.0', + plugins: [], + extensions: { + exposedComponents: [], + }, + }, + extensions: { + addedLinks: [], + addedComponents: [], + addedFunctions: [], + exposedComponents: [], + extensionPoints: [], + }, + }; + + wrapper = ({ children }: { children: React.ReactNode }) => ( + + {children} + + ); + }); + + it('should return an empty array if there are no function extensions registered for the extension point', () => { + const { result } = renderHook( + () => + usePluginFunctions({ + extensionPointId: 'foo/bar', + }), + { wrapper } + ); + + expect(result.current.functions).toEqual([]); + }); + + it('should only return the function extensions for the given extension point ids', async () => { + registries.addedFunctionsRegistry.register({ + pluginId, + configs: [ + { + targets: extensionPointId, + title: '1', + description: '1', + fn: () => 'function1', + }, + { + targets: extensionPointId, + title: '2', + description: '2', + fn: () => 'function2', + }, + { + targets: 'plugins/another-extension/v1', + title: '3', + description: '3', + fn: () => 'function3', + }, + ], + }); + + const { result } = renderHook(() => usePluginFunctions({ extensionPointId }), { wrapper }); + + expect(result.current.functions.length).toBe(2); + expect(result.current.functions[0].title).toBe('1'); + expect(result.current.functions[1].title).toBe('2'); + }); + + it('should dynamically update the extensions registered for a certain extension point', () => { + let { result, rerender } = renderHook(() => usePluginFunctions({ extensionPointId }), { wrapper }); + + // No extensions yet + expect(result.current.functions.length).toBe(0); + + // Add extensions to the registry + act(() => { + registries.addedFunctionsRegistry.register({ + pluginId, + configs: [ + { + targets: extensionPointId, + title: '1', + description: '1', + fn: () => 'function1', + }, + { + targets: extensionPointId, + title: '2', + description: '2', + fn: () => 'function2', + }, + ], + }); + }); + + // Check if the hook returns the new extensions + rerender(); + + expect(result.current.functions.length).toBe(2); + expect(result.current.functions[0].title).toBe('1'); + expect(result.current.functions[1].title).toBe('2'); + }); + + it('should honour the limitPerPlugin arg if its set', () => { + const plugins = ['my-awesome1-app', 'my-awesome2-app', 'my-awesome3-app']; + let { result, rerender } = renderHook(() => usePluginFunctions({ extensionPointId, limitPerPlugin: 2 }), { + wrapper, + }); + + // No extensions yet + expect(result.current.functions.length).toBe(0); + + // Add extensions to the registry + act(() => { + for (let pluginId of plugins) { + registries.addedFunctionsRegistry.register({ + pluginId, + configs: [ + { + targets: [extensionPointId], + title: '1', + description: '1', + fn: () => 'function1', + }, + { + targets: [extensionPointId], + title: '2', + description: '2', + fn: () => 'function2', + }, + { + targets: [extensionPointId], + title: '3', + description: '3', + fn: () => 'function3', + }, + ], + }); + } + }); + + // Check if the hook returns the new extensions + rerender(); + + // Should only return 2 functions per plugin due to limitPerPlugin: 2 + expect(result.current.functions.length).toBe(6); + }); + + it('should return isLoading: true when app plugins are loading', () => { + jest.mocked(useLoadAppPlugins).mockReturnValue({ isLoading: true }); + + const { result } = renderHook(() => usePluginFunctions({ extensionPointId }), { wrapper }); + + expect(result.current.isLoading).toBe(true); + expect(result.current.functions).toEqual([]); + }); + + it('should return isLoading: false when app plugins are not loading', () => { + jest.mocked(useLoadAppPlugins).mockReturnValue({ isLoading: false }); + + const { result } = renderHook(() => usePluginFunctions({ extensionPointId }), { wrapper }); + + expect(result.current.isLoading).toBe(false); + expect(result.current.functions).toEqual([]); + }); + + it('should not validate the extension point meta-info in production mode', () => { + // Empty list of extension points in the plugin meta (from plugin.json) + wrapper = ({ children }: { children: React.ReactNode }) => ( + + {children} + + ); + + registries.addedFunctionsRegistry.register({ + pluginId, + configs: [ + { + targets: extensionPointId, + title: '1', + description: '1', + fn: () => 'function1', + }, + ], + }); + + // Trying to render an extension point that is not defined in the plugin meta + // (No restrictions due to isGrafanaDevMode() = false) + const { result } = renderHook(() => usePluginFunctions({ extensionPointId }), { wrapper }); + expect(result.current.functions.length).toBe(1); + expect(log.error).not.toHaveBeenCalled(); + }); + + // It can happen that core Grafana plugins (e.g. traces) reuse core components which implement extension points. + it('should not validate the extension point meta-info for core plugins', () => { + jest.mocked(isGrafanaDevMode).mockReturnValue(true); + + const functionConfig = { + targets: extensionPointId, + title: '1', + description: '1', + fn: () => 'function1', + }; + + // The `AddedFunctionsRegistry` is validating if the function is registered in the plugin metadata (config.apps). + config.apps[pluginId].extensions.addedFunctions = [functionConfig]; + + wrapper = ({ children }: { children: React.ReactNode }) => ( + + {children} + + ); + + registries.addedFunctionsRegistry.register({ + pluginId, + configs: [functionConfig], + }); + + // Trying to render an extension point that is not defined in the plugin meta + // (No restrictions due to being a core plugin) + const { result } = renderHook(() => usePluginFunctions({ extensionPointId }), { wrapper }); + expect(result.current.functions.length).toBe(1); + expect(log.error).not.toHaveBeenCalled(); + }); + + it('should not validate the extension point id in production mode', () => { + // Empty list of extension points in the plugin meta (from plugin.json) + wrapper = ({ children }: { children: React.ReactNode }) => ( + + {children} + + ); + + // Trying to render an extension point that is not defined in the plugin meta + // (No restrictions due to isGrafanaDevMode() = false) + const { result } = renderHook(() => usePluginFunctions({ extensionPointId: 'invalid-extension-point-id' }), { + wrapper, + }); + expect(result.current.functions.length).toBe(0); + expect(log.error).not.toHaveBeenCalled(); + }); + + it('should not validate the extension point meta-info if used in Grafana core (no plugin context)', () => { + // Imitate running in dev mode + jest.mocked(isGrafanaDevMode).mockReturnValue(true); + + // No plugin context -> used in Grafana core + wrapper = ({ children }: { children: React.ReactNode }) => ( + {children} + ); + + // Adding an extension to the extension point + registries.addedFunctionsRegistry.register({ + pluginId: 'grafana', // Only core Grafana can register extensions without a plugin context + configs: [ + { + targets: PluginExtensionPoints.DashboardPanelMenu, + title: '1', + description: '1', + fn: () => 'function1', + }, + ], + }); + + const { result } = renderHook( + () => usePluginFunctions({ extensionPointId: PluginExtensionPoints.DashboardPanelMenu }), + { + wrapper, + } + ); + expect(result.current.functions.length).toBe(1); + expect(log.error).not.toHaveBeenCalled(); + }); + + it('should not allow to create an extension point in core Grafana that is not exposed to plugins', () => { + // Imitate running in dev mode + jest.mocked(isGrafanaDevMode).mockReturnValue(true); + + // No plugin context -> used in Grafana core + wrapper = ({ children }: { children: React.ReactNode }) => ( + {children} + ); + + const extensionPointId = 'grafana/not-exposed-extension-point/v1'; + + // Adding an extension to the extension point + registries.addedFunctionsRegistry.register({ + pluginId: 'grafana', // Only core Grafana can register extensions without a plugin context + configs: [ + { + targets: extensionPointId, + title: '1', + description: '1', + fn: () => 'function1', + }, + ], + }); + + const { result } = renderHook(() => usePluginFunctions({ extensionPointId }), { wrapper }); + expect(result.current.functions.length).toBe(0); + expect(log.error).toHaveBeenCalled(); + }); + + it('should not validate the extension point id if used in Grafana core (no plugin context)', () => { + // Imitate running in dev mode + jest.mocked(isGrafanaDevMode).mockReturnValue(true); + + // No plugin context -> used in Grafana core + wrapper = ({ children }: { children: React.ReactNode }) => ( + {children} + ); + + const { result } = renderHook(() => usePluginFunctions({ extensionPointId: 'invalid-extension-point-id' }), { + wrapper, + }); + expect(result.current.functions.length).toBe(0); + expect(log.warning).not.toHaveBeenCalled(); + }); + + it('should validate if the extension point meta-info is correct if in dev-mode and used by a plugin', () => { + // Imitate running in dev mode + jest.mocked(isGrafanaDevMode).mockReturnValue(true); + + // Empty list of extension points in the plugin meta (from plugin.json) + wrapper = ({ children }: { children: React.ReactNode }) => ( + + {children} + + ); + + // Adding an extension to the extension point - it should not be returned later + registries.addedFunctionsRegistry.register({ + pluginId, + configs: [ + { + targets: extensionPointId, + title: '1', + description: '1', + fn: () => 'function1', + }, + ], + }); + + // Trying to render an extension point that is not defined in the plugin meta + const { result } = renderHook(() => usePluginFunctions({ extensionPointId }), { wrapper }); + expect(result.current.functions.length).toBe(0); + expect(log.error).toHaveBeenCalled(); + expect(log.error).toHaveBeenCalledWith(errors.EXTENSION_POINT_META_INFO_MISSING); + }); +}); diff --git a/public/app/features/plugins/extensions/usePluginFunctions.tsx b/public/app/features/plugins/extensions/usePluginFunctions.tsx index 8f8c989e316..df4f95678a7 100644 --- a/public/app/features/plugins/extensions/usePluginFunctions.tsx +++ b/public/app/features/plugins/extensions/usePluginFunctions.tsx @@ -5,11 +5,9 @@ import { usePluginContext, PluginExtensionFunction, PluginExtensionTypes } from import { UsePluginFunctionsOptions, UsePluginFunctionsResult } from '@grafana/runtime'; import { useAddedFunctionsRegistry } from './ExtensionRegistriesContext'; -import * as errors from './errors'; -import { log } from './logs/log'; import { useLoadAppPlugins } from './useLoadAppPlugins'; -import { generateExtensionId, getExtensionPointPluginDependencies, isGrafanaDevMode } from './utils'; -import { isExtensionPointIdValid, isExtensionPointMetaInfoMissing } from './validators'; +import { generateExtensionId, getExtensionPointPluginDependencies } from './utils'; +import { validateExtensionPoint } from './validateExtensionPoint'; // Returns an array of component extensions for the given extension point export function usePluginFunctions({ @@ -23,45 +21,17 @@ export function usePluginFunctions({ const { isLoading: isLoadingAppPlugins } = useLoadAppPlugins(deps); return useMemo(() => { - const isInsidePlugin = Boolean(pluginContext); - const isCoreGrafanaPlugin = pluginContext?.meta.module.startsWith('core:') ?? false; + const { result } = validateExtensionPoint({ extensionPointId, pluginContext, isLoadingAppPlugins }); + + if (result) { + return { + isLoading: result.isLoading, + functions: [], + }; + } + const results: Array> = []; const extensionsByPlugin: Record = {}; - const pluginId = pluginContext?.meta.id ?? ''; - const pointLog = log.child({ - pluginId, - extensionPointId, - }); - - if ( - isGrafanaDevMode() && - !isExtensionPointIdValid({ extensionPointId, pluginId, isInsidePlugin, isCoreGrafanaPlugin, log: pointLog }) - ) { - return { - isLoading: false, - functions: [], - }; - } - - if ( - isGrafanaDevMode() && - !isCoreGrafanaPlugin && - pluginContext && - isExtensionPointMetaInfoMissing(extensionPointId, pluginContext) - ) { - pointLog.error(errors.EXTENSION_POINT_META_INFO_MISSING); - return { - isLoading: false, - functions: [], - }; - } - - if (isLoadingAppPlugins) { - return { - isLoading: true, - functions: [], - }; - } for (const registryItem of registryState?.[extensionPointId] ?? []) { const { pluginId } = registryItem; diff --git a/public/app/features/plugins/extensions/usePluginLinks.test.tsx b/public/app/features/plugins/extensions/usePluginLinks.test.tsx index 697c9dfd91f..4f018236a59 100644 --- a/public/app/features/plugins/extensions/usePluginLinks.test.tsx +++ b/public/app/features/plugins/extensions/usePluginLinks.test.tsx @@ -10,6 +10,7 @@ import { import { config } from '@grafana/runtime'; import { ExtensionRegistriesProvider } from './ExtensionRegistriesContext'; +import * as errors from './errors'; import { log } from './logs/log'; import { resetLogMock } from './logs/testUtils'; import { AddedComponentsRegistry } from './registry/AddedComponentsRegistry'; @@ -37,7 +38,7 @@ jest.mock('./utils', () => ({ ...jest.requireActual('./utils'), // Manually set the dev mode to false - // (to make sure that by default we are testing a production scneario) + // (to make sure that by default we are testing a production scenario) isGrafanaDevMode: jest.fn().mockReturnValue(false), })); @@ -247,7 +248,7 @@ describe('usePluginLinks()', () => { // Trying to render an extension point that is not defined in the plugin meta // (No restrictions due to isGrafanaDevMode() = false) - let { result } = renderHook(() => usePluginLinks({ extensionPointId }), { wrapper }); + const { result } = renderHook(() => usePluginLinks({ extensionPointId }), { wrapper }); expect(result.current.links.length).toBe(1); expect(log.warning).not.toHaveBeenCalled(); }); @@ -290,7 +291,7 @@ describe('usePluginLinks()', () => { // Trying to render an extension point that is not defined in the plugin meta // (No restrictions due to being a core plugin) - let { result } = renderHook(() => usePluginLinks({ extensionPointId }), { wrapper }); + const { result } = renderHook(() => usePluginLinks({ extensionPointId }), { wrapper }); expect(result.current.links.length).toBe(1); expect(log.warning).not.toHaveBeenCalled(); }); @@ -313,7 +314,9 @@ describe('usePluginLinks()', () => { // Trying to render an extension point that is not defined in the plugin meta // (No restrictions due to isGrafanaDevMode() = false) - let { result } = renderHook(() => usePluginLinks({ extensionPointId: 'invalid-extension-point-id' }), { wrapper }); + const { result } = renderHook(() => usePluginLinks({ extensionPointId: 'invalid-extension-point-id' }), { + wrapper, + }); expect(result.current.links.length).toBe(0); expect(log.warning).not.toHaveBeenCalled(); }); @@ -340,9 +343,12 @@ describe('usePluginLinks()', () => { ], }); - let { result } = renderHook(() => usePluginLinks({ extensionPointId: PluginExtensionPoints.DashboardPanelMenu }), { - wrapper, - }); + const { result } = renderHook( + () => usePluginLinks({ extensionPointId: PluginExtensionPoints.DashboardPanelMenu }), + { + wrapper, + } + ); expect(result.current.links.length).toBe(1); expect(log.warning).not.toHaveBeenCalled(); }); @@ -371,7 +377,7 @@ describe('usePluginLinks()', () => { ], }); - let { result } = renderHook(() => usePluginLinks({ extensionPointId }), { wrapper }); + const { result } = renderHook(() => usePluginLinks({ extensionPointId }), { wrapper }); expect(result.current.links.length).toBe(0); expect(log.error).toHaveBeenCalled(); }); @@ -385,7 +391,9 @@ describe('usePluginLinks()', () => { {children} ); - let { result } = renderHook(() => usePluginLinks({ extensionPointId: 'invalid-extension-point-id' }), { wrapper }); + const { result } = renderHook(() => usePluginLinks({ extensionPointId: 'invalid-extension-point-id' }), { + wrapper, + }); expect(result.current.links.length).toBe(0); expect(log.warning).not.toHaveBeenCalled(); }); @@ -423,9 +431,10 @@ describe('usePluginLinks()', () => { }); // Trying to render an extension point that is not defined in the plugin meta - let { result } = renderHook(() => usePluginLinks({ extensionPointId }), { wrapper }); + const { result } = renderHook(() => usePluginLinks({ extensionPointId }), { wrapper }); expect(result.current.links.length).toBe(0); expect(log.error).toHaveBeenCalled(); + expect(log.error).toHaveBeenCalledWith(errors.EXTENSION_POINT_META_INFO_MISSING); }); it('should not log a warning if the extension point meta-info is correct if in dev-mode and used by a plugin', () => { @@ -461,7 +470,7 @@ describe('usePluginLinks()', () => { }); // Trying to render an extension point that is not defined in the plugin meta - let { result } = renderHook(() => usePluginLinks({ extensionPointId }), { wrapper }); + const { result } = renderHook(() => usePluginLinks({ extensionPointId }), { wrapper }); expect(result.current.links.length).toBe(0); expect(log.error).toHaveBeenCalled(); }); diff --git a/public/app/features/plugins/extensions/usePluginLinks.tsx b/public/app/features/plugins/extensions/usePluginLinks.tsx index 5d2b193a77c..be25ec5105a 100644 --- a/public/app/features/plugins/extensions/usePluginLinks.tsx +++ b/public/app/features/plugins/extensions/usePluginLinks.tsx @@ -6,8 +6,6 @@ import { PluginExtensionLink, PluginExtensionTypes, usePluginContext } from '@gr import { UsePluginLinksOptions, UsePluginLinksResult } from '@grafana/runtime'; import { useAddedLinksRegistry } from './ExtensionRegistriesContext'; -import * as errors from './errors'; -import { log } from './logs/log'; import { useLoadAppPlugins } from './useLoadAppPlugins'; import { generateExtensionId, @@ -16,9 +14,8 @@ import { getLinkExtensionOverrides, getLinkExtensionPathWithTracking, getReadOnlyProxy, - isGrafanaDevMode, } from './utils'; -import { isExtensionPointIdValid, isExtensionPointMetaInfoMissing } from './validators'; +import { validateExtensionPoint } from './validateExtensionPoint'; // Returns an array of component extensions for the given extension point export function usePluginLinks({ @@ -32,47 +29,15 @@ export function usePluginLinks({ const { isLoading: isLoadingAppPlugins } = useLoadAppPlugins(getExtensionPointPluginDependencies(extensionPointId)); return useMemo(() => { - const isInsidePlugin = Boolean(pluginContext); - const pluginId = pluginContext?.meta.id ?? ''; - const isCoreGrafanaPlugin = pluginContext?.meta.module.startsWith('core:') ?? false; - const pointLog = log.child({ - pluginId, + const { result, pointLog } = validateExtensionPoint({ extensionPointId, + pluginContext, + isLoadingAppPlugins, }); - if ( - isGrafanaDevMode() && - !isExtensionPointIdValid({ extensionPointId, pluginId, isInsidePlugin, isCoreGrafanaPlugin, log: pointLog }) - ) { + if (result) { return { - isLoading: false, - links: [], - }; - } - - if ( - isGrafanaDevMode() && - !isCoreGrafanaPlugin && - pluginContext && - isExtensionPointMetaInfoMissing(extensionPointId, pluginContext) - ) { - pointLog.error(errors.EXTENSION_POINT_META_INFO_MISSING); - return { - isLoading: false, - links: [], - }; - } - - if (isLoadingAppPlugins) { - return { - isLoading: true, - links: [], - }; - } - - if (!registryState || !registryState[extensionPointId]) { - return { - isLoading: false, + isLoading: result.isLoading, links: [], }; } @@ -81,7 +46,7 @@ export function usePluginLinks({ const extensions: PluginExtensionLink[] = []; const extensionsByPlugin: Record = {}; - for (const addedLink of registryState[extensionPointId] ?? []) { + for (const addedLink of registryState?.[extensionPointId] ?? []) { const { pluginId } = addedLink; const linkLog = pointLog.child({ path: addedLink.path ?? '', diff --git a/public/app/features/plugins/extensions/validateExtensionPoint.test.ts b/public/app/features/plugins/extensions/validateExtensionPoint.test.ts new file mode 100644 index 00000000000..611db3b8aa5 --- /dev/null +++ b/public/app/features/plugins/extensions/validateExtensionPoint.test.ts @@ -0,0 +1,174 @@ +import { PluginContextType } from '@grafana/data'; + +import * as errors from './errors'; +import { ExtensionsLog } from './logs/log'; +import { isGrafanaDevMode } from './utils'; +import { validateExtensionPoint } from './validateExtensionPoint'; +import * as validators from './validators'; + +jest.mock('./utils', () => ({ + ...jest.requireActual('./utils'), + + // Manually set the dev mode to false + // (to make sure that by default we are testing a production scenario) + isGrafanaDevMode: jest.fn().mockReturnValue(false), +})); + +const setup = ({ + pointValid = true, + metaMissing = false, + corePlugin = false, +}: { pointValid?: boolean; metaMissing?: boolean; corePlugin?: boolean } = {}) => { + const spyIsExtensionPointIdValid = jest.spyOn(validators, 'isExtensionPointIdValid').mockReturnValue(pointValid); + const spyisExtensionPointMetaInfoMissing = jest + .spyOn(validators, 'isExtensionPointMetaInfoMissing') + .mockReturnValue(metaMissing); + const pluginId = 'myorg-extensions-app'; + const extensionPointId = `${pluginId}/extension-point/v1`; + const pluginContext = { meta: { id: pluginId, module: corePlugin ? 'core:' : '' } } as PluginContextType; + + return { + spyIsExtensionPointIdValid, + spyisExtensionPointMetaInfoMissing, + pluginId, + extensionPointId, + pluginContext, + }; +}; + +describe('getExtensionValidationResults', () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + + describe('when calling in production mode', () => { + beforeEach(() => { + jest.mocked(isGrafanaDevMode).mockReturnValue(false); + }); + + it('should return isLoading:true while loading app plugins', () => { + const { extensionPointId, pluginContext } = setup(); + + const actual = validateExtensionPoint({ + extensionPointId, + isLoadingAppPlugins: true, + pluginContext, + }); + + expect(actual.result).toEqual({ isLoading: true }); + expect(actual.pointLog).toBeDefined(); + }); + + it('should return null when all validations pass', () => { + const { extensionPointId, pluginContext } = setup(); + + const actual = validateExtensionPoint({ + extensionPointId, + isLoadingAppPlugins: false, + pluginContext, + }); + + expect(actual.result).toBe(null); + expect(actual.pointLog).toBeDefined(); + }); + }); + + describe('when calling in dev mode', () => { + let errorSpy: jest.SpyInstance; + beforeEach(() => { + jest.mocked(isGrafanaDevMode).mockReturnValue(true); + errorSpy = jest.spyOn(console, 'error').mockImplementation(); + }); + + it('should return isLoading:false when extension point is invalid', () => { + const { + extensionPointId, + pluginContext, + pluginId, + spyIsExtensionPointIdValid, + spyisExtensionPointMetaInfoMissing, + } = setup({ pointValid: false }); + + const actual = validateExtensionPoint({ + extensionPointId, + isLoadingAppPlugins: true, + pluginContext, + }); + + expect(actual.result).toEqual({ isLoading: false }); + expect(actual.pointLog).toBeDefined(); + expect(spyisExtensionPointMetaInfoMissing).not.toHaveBeenCalled(); + expect(spyIsExtensionPointIdValid).toHaveBeenCalledTimes(1); + expect(spyIsExtensionPointIdValid).toHaveBeenCalledWith({ + extensionPointId, + pluginId, + isInsidePlugin: true, + isCoreGrafanaPlugin: false, + log: expect.any(ExtensionsLog), + }); + }); + + it('should return isLoading:false when extension point meta is missing', () => { + const { extensionPointId, pluginContext, pluginId, spyisExtensionPointMetaInfoMissing } = setup({ + metaMissing: true, + }); + + const actual = validateExtensionPoint({ + extensionPointId, + isLoadingAppPlugins: true, + pluginContext, + }); + + expect(actual.result).toEqual({ isLoading: false }); + expect(actual.pointLog).toBeDefined(); + expect(spyisExtensionPointMetaInfoMissing).toHaveBeenCalled(); + expect(spyisExtensionPointMetaInfoMissing).toHaveBeenCalledWith(extensionPointId, pluginContext); + expect(errorSpy).toHaveBeenCalled(); + expect(errorSpy).toHaveBeenCalledWith(errors.EXTENSION_POINT_META_INFO_MISSING, { extensionPointId, pluginId }); + }); + + it('should ignore core plugins when extension point meta is missing', () => { + const { extensionPointId, pluginContext, spyisExtensionPointMetaInfoMissing } = setup({ + metaMissing: true, + corePlugin: true, + }); + + const actual = validateExtensionPoint({ + extensionPointId, + isLoadingAppPlugins: false, + pluginContext, + }); + + expect(actual.result).toEqual(null); + expect(actual.pointLog).toBeDefined(); + expect(spyisExtensionPointMetaInfoMissing).not.toHaveBeenCalled(); + expect(errorSpy).not.toHaveBeenCalled(); + }); + + it('should return isLoading:true while loading app plugins', () => { + const { extensionPointId, pluginContext } = setup(); + + const actual = validateExtensionPoint({ + extensionPointId, + isLoadingAppPlugins: true, + pluginContext, + }); + + expect(actual.result).toEqual({ isLoading: true }); + expect(actual.pointLog).toBeDefined(); + }); + + it('should return null when all validations pass', () => { + const { extensionPointId, pluginContext } = setup(); + + const actual = validateExtensionPoint({ + extensionPointId, + isLoadingAppPlugins: false, + pluginContext, + }); + + expect(actual.result).toBe(null); + expect(actual.pointLog).toBeDefined(); + }); + }); +}); diff --git a/public/app/features/plugins/extensions/validateExtensionPoint.ts b/public/app/features/plugins/extensions/validateExtensionPoint.ts new file mode 100644 index 00000000000..aef4ab1fb49 --- /dev/null +++ b/public/app/features/plugins/extensions/validateExtensionPoint.ts @@ -0,0 +1,57 @@ +import { PluginContextType } from '@grafana/data'; + +import * as errors from './errors'; +import { ExtensionsLog, log } from './logs/log'; +import { isGrafanaDevMode } from './utils'; +import { isExtensionPointIdValid, isExtensionPointMetaInfoMissing } from './validators'; + +interface ValidateExtensionPointOptions { + extensionPointId: string; + isLoadingAppPlugins: boolean; + pluginContext: PluginContextType | null; +} + +interface ValidateExtensionPoint { + isLoading: boolean; +} + +type ValidateExtensionPointResult = { + result: ValidateExtensionPoint | null; + pointLog: ExtensionsLog; +}; + +export function validateExtensionPoint({ + extensionPointId, + isLoadingAppPlugins, + pluginContext, +}: ValidateExtensionPointOptions): ValidateExtensionPointResult { + const isInsidePlugin = Boolean(pluginContext); + const isCoreGrafanaPlugin = pluginContext?.meta.module.startsWith('core:') ?? false; + const pluginId = pluginContext?.meta.id ?? ''; + const pointLog = log.child({ pluginId, extensionPointId }); + + // Don't show extensions if the extension-point id is invalid in DEV mode + if ( + isGrafanaDevMode() && + !isExtensionPointIdValid({ extensionPointId, pluginId, isInsidePlugin, isCoreGrafanaPlugin, log: pointLog }) + ) { + return { result: { isLoading: false }, pointLog }; + } + + // Don't show extensions if the extension-point misses meta info (plugin.json) in DEV mode + if ( + isGrafanaDevMode() && + !isCoreGrafanaPlugin && + pluginContext && + isExtensionPointMetaInfoMissing(extensionPointId, pluginContext) + ) { + pointLog.error(errors.EXTENSION_POINT_META_INFO_MISSING); + return { result: { isLoading: false }, pointLog }; + } + + if (isLoadingAppPlugins) { + return { result: { isLoading: true }, pointLog }; + } + + return { result: null, pointLog }; +}