diff --git a/pkg/plugins/manager/pipeline/bootstrap/steps.go b/pkg/plugins/manager/pipeline/bootstrap/steps.go index d8067c9a9cc..8c9a64b7c6b 100644 --- a/pkg/plugins/manager/pipeline/bootstrap/steps.go +++ b/pkg/plugins/manager/pipeline/bootstrap/steps.go @@ -4,7 +4,6 @@ import ( "context" "path" "slices" - "strings" "github.com/grafana/grafana/pkg/infra/slugify" "github.com/grafana/grafana/pkg/plugins" @@ -144,14 +143,11 @@ func configureAppChildPlugin(parent *plugins.Plugin, child *plugins.Plugin) { return } child.IncludedInAppID = parent.ID - child.BaseURL = parent.BaseURL - // TODO move this logic within assetpath package - appSubPath := strings.ReplaceAll(strings.Replace(child.FS.Base(), parent.FS.Base(), "", 1), "\\", "/") - if parent.IsCorePlugin() { - child.Module = path.Join("core:plugin", parent.ID, appSubPath) - } else { - child.Module = path.Join("public/plugins", parent.ID, appSubPath, "module.js") + // If the child plugin does not have a version, it will inherit the version from the parent. + // This is to ensure that the frontend can appropriately cache the plugin assets. + if child.Info.Version == "" { + child.Info.Version = parent.Info.Version } } diff --git a/pkg/plugins/manager/pipeline/bootstrap/steps_test.go b/pkg/plugins/manager/pipeline/bootstrap/steps_test.go index 1fa03bac60e..31eedd6ddc2 100644 --- a/pkg/plugins/manager/pipeline/bootstrap/steps_test.go +++ b/pkg/plugins/manager/pipeline/bootstrap/steps_test.go @@ -96,7 +96,7 @@ func TestTemplateDecorateFunc(t *testing.T) { } func Test_configureAppChildPlugin(t *testing.T) { - t.Run("When setting paths based on core plugin on Windows", func(t *testing.T) { + t.Run("Child plugin will inherit parent version information when version is empty", func(t *testing.T) { child := &plugins.Plugin{ FS: fakes.NewFakePluginFiles("c:\\grafana\\public\\app\\plugins\\app\\testdata-app\\datasources\\datasource"), } @@ -104,6 +104,7 @@ func Test_configureAppChildPlugin(t *testing.T) { JSONData: plugins.JSONData{ Type: plugins.TypeApp, ID: "testdata-app", + Info: plugins.Info{Version: "1.0.0"}, }, Class: plugins.ClassCore, FS: fakes.NewFakePluginFiles("c:\\grafana\\public\\app\\plugins\\app\\testdata-app"), @@ -112,19 +113,22 @@ func Test_configureAppChildPlugin(t *testing.T) { configureAppChildPlugin(parent, child) - require.Equal(t, "core:plugin/testdata-app/datasources/datasource", child.Module) require.Equal(t, "testdata-app", child.IncludedInAppID) - require.Equal(t, "public/app/plugins/app/testdata-app", child.BaseURL) + require.Equal(t, parent.Info.Version, child.Info.Version) }) - t.Run("When setting paths based on external plugin", func(t *testing.T) { + t.Run("Child plugin will not inherit parent version information when version is non-empty", func(t *testing.T) { child := &plugins.Plugin{ FS: fakes.NewFakePluginFiles("/plugins/parent-app/child-panel"), + JSONData: plugins.JSONData{ + Info: plugins.Info{Version: "2.0.2"}, + }, } parent := &plugins.Plugin{ JSONData: plugins.JSONData{ Type: plugins.TypeApp, ID: "testdata-app", + Info: plugins.Info{Version: "2.0.0"}, }, Class: plugins.ClassExternal, FS: fakes.NewFakePluginFiles("/plugins/parent-app"), @@ -133,9 +137,8 @@ func Test_configureAppChildPlugin(t *testing.T) { configureAppChildPlugin(parent, child) - require.Equal(t, "public/plugins/testdata-app/child-panel/module.js", child.Module) require.Equal(t, "testdata-app", child.IncludedInAppID) - require.Equal(t, "plugins/parent-app", child.BaseURL) + require.NotEqual(t, parent.Info.Version, child.Info.Version) }) } diff --git a/pkg/services/pluginsintegration/loader/loader_test.go b/pkg/services/pluginsintegration/loader/loader_test.go index fb1f19d41d4..f44e4a8508f 100644 --- a/pkg/services/pluginsintegration/loader/loader_test.go +++ b/pkg/services/pluginsintegration/loader/loader_test.go @@ -1414,8 +1414,8 @@ func TestLoader_Load_NestedPlugins(t *testing.T) { Plugins: []plugins.Dependency{}, }, }, - Module: "public/plugins/myorgid-simple-app/child/module.js", - BaseURL: "public/plugins/myorgid-simple-app", + Module: "public/plugins/myorgid-simple-panel/module.js", + BaseURL: "public/plugins/myorgid-simple-panel", FS: mustNewStaticFSForTests(t, filepath.Join(testDataDir(t), "app-with-child/dist/child")), IncludedInAppID: parent.ID, Signature: plugins.SignatureStatusValid, diff --git a/public/app/features/plugins/loader/cache.test.ts b/public/app/features/plugins/loader/cache.test.ts index b0253b75663..531cadbe583 100644 --- a/public/app/features/plugins/loader/cache.test.ts +++ b/public/app/features/plugins/loader/cache.test.ts @@ -1,54 +1,64 @@ -import * as pluginSettings from '../pluginSettings'; +import { registerPluginInCache, invalidatePluginInCache, resolveWithCache, getPluginFromCache } from './cache'; -import { invalidatePluginInCache, resolveWithCache, registerPluginInCache } from './cache'; +jest.mock('./constants', () => ({ + CACHE_INITIALISED_AT: 123456, +})); -describe('Plugin Cache', () => { - const now = 12345; +describe('Cache Functions', () => { + describe('registerPluginInCache', () => { + it('should register a plugin in the cache', () => { + const plugin = { pluginId: 'plugin1', version: '1.0.0', isAngular: false }; + registerPluginInCache(plugin); + expect(getPluginFromCache('plugin1')).toEqual(plugin); + }); - it('should append plugin version as cache flag if plugin is registered in buster', () => { - const slug = 'bubble-chart-1'; - const version = 'v1.0.0'; - const path = `/public/plugins/${slug}/module.js`; - const address = `http://localhost:3000/public/${path}.js`; - - registerPluginInCache({ path, version }); - - const url = `${address}?_cache=${encodeURI(version)}`; - expect(resolveWithCache(address, now)).toBe(url); + it('should not register a plugin if it already exists in the cache', () => { + const pluginId = 'plugin2'; + const plugin = { pluginId, version: '2.0.0' }; + registerPluginInCache(plugin); + const plugin2 = { pluginId, version: '2.5.0' }; + registerPluginInCache(plugin2); + expect(getPluginFromCache(pluginId)?.version).toBe('2.0.0'); + }); }); - it('should append Date.now as cache flag if plugin is not registered in buster', () => { - const slug = 'bubble-chart-2'; - const address = `http://localhost:3000/public/plugins/${slug}/module.js`; + describe('invalidatePluginInCache', () => { + it('should invalidate a plugin in the cache', () => { + const pluginId = 'plugin3'; + const plugin = { pluginId, version: '3.0.0' }; + registerPluginInCache(plugin); + invalidatePluginInCache(pluginId); + expect(getPluginFromCache(pluginId)).toBeUndefined(); + }); - const url = `${address}?_cache=${encodeURI(String(now))}`; - expect(resolveWithCache(address, now)).toBe(url); + it('should not throw an error if the plugin does not exist in the cache', () => { + expect(() => invalidatePluginInCache('nonExistentPlugin')).not.toThrow(); + }); }); - it('should append Date.now as cache flag if plugin is invalidated in buster', () => { - const slug = 'bubble-chart-3'; - const version = 'v1.0.0'; - const path = `/public/plugins/${slug}/module.js`; - const address = `http://localhost:3000/public/${path}.js`; + describe('resolveWithCache', () => { + it('should resolve URL with timestamp cache bust parameter if plugin is not available in the cache', () => { + const url = 'http://localhost:3000/public/plugins/plugin4/module.js'; + expect(resolveWithCache(url)).toContain('_cache=123456'); + }); - registerPluginInCache({ path, version }); - invalidatePluginInCache(slug); - - const url = `${address}?_cache=${encodeURI(String(now))}`; - expect(resolveWithCache(address, now)).toBe(url); + it('should resolve URL with plugin version as cache bust parameter if available', () => { + const plugin = { pluginId: 'plugin5', version: '5.0.0' }; + registerPluginInCache(plugin); + const url = 'http://localhost:3000/public/plugins/plugin5/module.js'; + expect(resolveWithCache(url)).toContain('_cache=5.0.0'); + }); }); - it('should also clear plugin settings cache', () => { - const slug = 'bubble-chart-3'; - const version = 'v1.0.0'; - const path = `/public/plugins/${slug}/module.js`; + describe('getPluginFromCache', () => { + it('should return plugin from cache if exists', () => { + const plugin = { pluginId: 'plugin6', version: '6.0.0' }; + registerPluginInCache(plugin); + expect(getPluginFromCache('plugin6')).toEqual(plugin); + }); - const clearPluginSettingsCacheSpy = jest.spyOn(pluginSettings, 'clearPluginSettingsCache'); - - registerPluginInCache({ path, version }); - invalidatePluginInCache(slug); - - expect(clearPluginSettingsCacheSpy).toBeCalledTimes(1); - expect(clearPluginSettingsCacheSpy).toBeCalledWith('bubble-chart-3'); + it('should return undefined if plugin does not exist in cache', () => { + expect(getPluginFromCache('nonExistentPlugin')).toBeUndefined(); + }); }); }); diff --git a/public/app/features/plugins/loader/cache.ts b/public/app/features/plugins/loader/cache.ts index 7bc30c405f9..4b3fa40d031 100644 --- a/public/app/features/plugins/loader/cache.ts +++ b/public/app/features/plugins/loader/cache.ts @@ -1,35 +1,36 @@ import { clearPluginSettingsCache } from '../pluginSettings'; +import { CACHE_INITIALISED_AT } from './constants'; + const cache: Record = {}; -const initializedAt: number = Date.now(); type CacheablePlugin = { - path: string; + pluginId: string; version: string; isAngular?: boolean; }; -export function registerPluginInCache({ path, version, isAngular }: CacheablePlugin): void { - const key = extractPath(path); +export function registerPluginInCache({ pluginId, version, isAngular }: CacheablePlugin): void { + const key = pluginId; if (key && !cache[key]) { cache[key] = { version: encodeURI(version), isAngular, - path, + pluginId, }; } } export function invalidatePluginInCache(pluginId: string): void { - const path = `plugins/${pluginId}/module`; + const path = pluginId; if (cache[path]) { delete cache[path]; } clearPluginSettingsCache(pluginId); } -export function resolveWithCache(url: string, defaultBust = initializedAt): string { - const path = extractPath(url); +export function resolveWithCache(url: string, defaultBust = CACHE_INITIALISED_AT): string { + const path = getCacheKey(url); if (!path) { return `${url}?_cache=${defaultBust}`; } @@ -39,21 +40,17 @@ export function resolveWithCache(url: string, defaultBust = initializedAt): stri } export function getPluginFromCache(path: string): CacheablePlugin | undefined { - const key = extractPath(path); + const key = getCacheKey(path); if (!key) { return; } return cache[key]; } -function extractPath(address: string): string | undefined { - const match = /\/?.+\/(plugins\/.+\/module)\.js/i.exec(address); - if (!match) { +function getCacheKey(address: string): string | undefined { + const key = Object.keys(cache).find((key) => address.includes(key)); + if (!key) { return; } - const [_, path] = match; - if (!path) { - return; - } - return path; + return key; } diff --git a/public/app/features/plugins/loader/constants.ts b/public/app/features/plugins/loader/constants.ts index 0bcc4f9737e..dc90df3f4ba 100644 --- a/public/app/features/plugins/loader/constants.ts +++ b/public/app/features/plugins/loader/constants.ts @@ -1,3 +1,4 @@ export const SHARED_DEPENDENCY_PREFIX = 'package'; export const LOAD_PLUGIN_CSS_REGEX = /^plugins.+\.css$/i; export const JS_CONTENT_TYPE_REGEX = /^(text|application)\/(x-)?javascript(;|$)/; +export const CACHE_INITIALISED_AT = Date.now(); diff --git a/public/app/features/plugins/loader/systemjsHooks.test.ts b/public/app/features/plugins/loader/systemjsHooks.test.ts index b3898a4be23..517c00855d3 100644 --- a/public/app/features/plugins/loader/systemjsHooks.test.ts +++ b/public/app/features/plugins/loader/systemjsHooks.test.ts @@ -49,20 +49,20 @@ describe('SystemJS Loader Hooks', () => { const id = '/public/plugins/my-datasource/styles.css!'; const result = decorateSystemJSResolve.bind(systemJSPrototype)(originalResolve, id); - expect(result).toBe('http://localhost/public/plugins/my-datasource/styles.css'); + expect(result).toBe('http://localhost/public/plugins/my-datasource/styles.css?_cache=1234'); }); it('adds default js extension to resolved url', () => { // test against missing extension const id = '/public/plugins/my-plugin/traffic_light'; const result = decorateSystemJSResolve.bind(systemJSPrototype)(originalResolve, id); - expect(result).toBe('http://localhost/public/plugins/my-plugin/traffic_light.js'); + expect(result).toBe('http://localhost/public/plugins/my-plugin/traffic_light.js?_cache=1234'); // test against missing extension with periods in filename const id2 = '/public/plugins/my-plugin/lib/flot/jquery.flot.gauge'; const result2 = decorateSystemJSResolve.bind(systemJSPrototype)(originalResolve, id2); - expect(result2).toBe('http://localhost/public/plugins/my-plugin/lib/flot/jquery.flot.gauge.js'); + expect(result2).toBe('http://localhost/public/plugins/my-plugin/lib/flot/jquery.flot.gauge.js?_cache=1234'); // test against bare specifiers const id3 = 'package:lodash'; @@ -74,12 +74,12 @@ describe('SystemJS Loader Hooks', () => { const id4 = '/public/plugins/my-plugin/traffic_light.js'; const result4 = decorateSystemJSResolve.bind(systemJSPrototype)(originalResolve, id4); - expect(result4).toBe('http://localhost/public/plugins/my-plugin/traffic_light.js'); + expect(result4).toBe('http://localhost/public/plugins/my-plugin/traffic_light.js?_cache=1234'); const id5 = '/public/plugins/my-plugin/traffic_light.css'; const result5 = decorateSystemJSResolve.bind(systemJSPrototype)(originalResolve, id5); - expect(result5).toBe('http://localhost/public/plugins/my-plugin/traffic_light.css'); + expect(result5).toBe('http://localhost/public/plugins/my-plugin/traffic_light.css?_cache=1234'); const id6 = '/public/plugins/my-plugin/traffic_light.json'; const result6 = decorateSystemJSResolve.bind(systemJSPrototype)(originalResolve, id6); @@ -95,7 +95,7 @@ describe('SystemJS Loader Hooks', () => { const id = 'plugins/my-plugin/dark.css'; const result = decorateSystemJSResolve.bind(systemJSPrototype)(originalResolve, id); - expect(result).toBe('/public/plugins/my-plugin/dark.css'); + expect(result).toBe('http://localhost/public/plugins/my-plugin/dark.css?_cache=1234'); }); it('adds cache query param to resolved module.js url', () => { const id = '/public/plugins/my-plugin/module.js'; diff --git a/public/app/features/plugins/loader/systemjsHooks.ts b/public/app/features/plugins/loader/systemjsHooks.ts index 9a994c19ce6..de0308a2604 100644 --- a/public/app/features/plugins/loader/systemjsHooks.ts +++ b/public/app/features/plugins/loader/systemjsHooks.ts @@ -37,27 +37,31 @@ export function decorateSystemJSResolve( id: string, parentUrl?: string ) { - const isFileSystemModule = id.endsWith('module.js') && !isHostedOnCDN(id); - try { const url = originalResolve.apply(this, [id, parentUrl]); const cleanedUrl = getBackWardsCompatibleUrl(url); + const isFileSystemModule = + (cleanedUrl.endsWith('.js') || cleanedUrl.endsWith('.css')) && !isHostedOnCDN(cleanedUrl); // Add a cache query param for filesystem module.js requests // CDN hosted plugins contain the version in the path so skip return isFileSystemModule ? resolveWithCache(cleanedUrl) : cleanedUrl; } catch (err) { - // Provide fallback for old plugins that use `loadPluginCss` to load theme styles - // Only affect plugins on the filesystem. + // Provide fallback for plugins that use `loadPluginCss` to load theme styles + // Regex only targets plugins on the filesystem. if (LOAD_PLUGIN_CSS_REGEX.test(id)) { - return `${config.appSubUrl ?? ''}/public/${id}`; + const prefixId = `${config.appSubUrl ?? ''}/public/${id}`; + const url = originalResolve.apply(this, [prefixId, parentUrl]); + return resolveWithCache(url); } - console.log(`SystemJS: failed to resolve '${id}'`); + console.warn(`SystemJS: failed to resolve '${id}'`); return id; } } export function decorateSystemJsOnload(err: unknown, id: string) { - if (id.endsWith('.css') && !err) { + // IF the url is relative resolve to current origin, absolute urls passed in will ignore base. + const url = new URL(id, window.location.origin); + if (url.pathname.endsWith('.css') && !err) { const module = SystemJS.get(id); const styles = module?.default; if (styles) { diff --git a/public/app/features/plugins/plugin_loader.ts b/public/app/features/plugins/plugin_loader.ts index 62499ed0e7b..588e938484b 100644 --- a/public/app/features/plugins/plugin_loader.ts +++ b/public/app/features/plugins/plugin_loader.ts @@ -74,7 +74,7 @@ export async function importPluginModule({ isAngular?: boolean; }): Promise { if (version) { - registerPluginInCache({ path, version, isAngular }); + registerPluginInCache({ pluginId, version, isAngular }); } const builtIn = builtInPlugins[path];