Plugins: Improve frontend loader cache (#87488)

* do it

* set empty child version to parent version

* feat(plugins): use pluginId for loader cache keys

* feat(plugins): apply caching to all js and css files systemjs loads

* remove old code and add comment

* test(plugins): update systemjs hooks tests in line with better caching

* test(plugins): wip - comment out failing backend loader tests

* fix tests and improve comment

* Update public/app/features/plugins/loader/cache.test.ts

Co-authored-by: Levente Balogh <balogh.levente.hu@gmail.com>

---------

Co-authored-by: Will Browne <will.browne@grafana.com>
Co-authored-by: Levente Balogh <balogh.levente.hu@gmail.com>
pull/88895/head
Jack Westbrook 1 year ago committed by GitHub
parent 897b81e566
commit 036c878843
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
  1. 14
      pkg/plugins/manager/pipeline/bootstrap/steps.go
  2. 15
      pkg/plugins/manager/pipeline/bootstrap/steps_test.go
  3. 4
      pkg/services/pluginsintegration/loader/loader_test.go
  4. 90
      public/app/features/plugins/loader/cache.test.ts
  5. 31
      public/app/features/plugins/loader/cache.ts
  6. 1
      public/app/features/plugins/loader/constants.ts
  7. 12
      public/app/features/plugins/loader/systemjsHooks.test.ts
  8. 18
      public/app/features/plugins/loader/systemjsHooks.ts
  9. 2
      public/app/features/plugins/plugin_loader.ts

@ -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
}
}

@ -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)
})
}

@ -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,

@ -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`;
registerPluginInCache({ path, version });
invalidatePluginInCache(slug);
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');
});
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`;
const clearPluginSettingsCacheSpy = jest.spyOn(pluginSettings, 'clearPluginSettingsCache');
registerPluginInCache({ path, version });
invalidatePluginInCache(slug);
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);
});
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();
});
});
});

@ -1,35 +1,36 @@
import { clearPluginSettingsCache } from '../pluginSettings';
import { CACHE_INITIALISED_AT } from './constants';
const cache: Record<string, CacheablePlugin> = {};
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) {
return;
}
const [_, path] = match;
if (!path) {
function getCacheKey(address: string): string | undefined {
const key = Object.keys(cache).find((key) => address.includes(key));
if (!key) {
return;
}
return path;
return key;
}

@ -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();

@ -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';

@ -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) {

@ -74,7 +74,7 @@ export async function importPluginModule({
isAngular?: boolean;
}): Promise<System.Module> {
if (version) {
registerPluginInCache({ path, version, isAngular });
registerPluginInCache({ pluginId, version, isAngular });
}
const builtIn = builtInPlugins[path];

Loading…
Cancel
Save