From 43e8c672198a1283c1c071b579484149c6643046 Mon Sep 17 00:00:00 2001 From: Vojtech Szocs Date: Fri, 21 Jul 2023 17:21:32 +0200 Subject: [PATCH] Add cacheBuster query string when requesting plugin entry scripts --- .../runtime/__tests__/plugin-loader.spec.ts | 26 +++++---- .../src/runtime/plugin-loader.ts | 14 +++-- .../src/utils/__tests__/url.spec.ts | 54 ++++++++++++++++--- .../src/utils/url.ts | 5 +- 4 files changed, 75 insertions(+), 24 deletions(-) diff --git a/frontend/packages/console-dynamic-plugin-sdk/src/runtime/__tests__/plugin-loader.spec.ts b/frontend/packages/console-dynamic-plugin-sdk/src/runtime/__tests__/plugin-loader.spec.ts index 07431a07bb4..f58f26a563a 100644 --- a/frontend/packages/console-dynamic-plugin-sdk/src/runtime/__tests__/plugin-loader.spec.ts +++ b/frontend/packages/console-dynamic-plugin-sdk/src/runtime/__tests__/plugin-loader.spec.ts @@ -1,6 +1,7 @@ import { act } from '@testing-library/react'; import { Simulate } from 'react-dom/test-utils'; import { PluginStore } from '@console/plugin-sdk/src/store'; +import * as utilsModule from '@console/shared/src/utils/utils'; import { ConsolePluginManifestJSON } from '../../schema/plugin-manifest'; import { Extension } from '../../types'; import { @@ -27,6 +28,7 @@ const { const fetchPluginManifest = jest.spyOn(pluginManifestModule, 'fetchPluginManifest'); const resolvePluginDependencies = jest.spyOn(pluginDependenciesModule, 'resolvePluginDependencies'); const loadDynamicPluginMock = jest.spyOn(pluginLoaderModule, 'loadDynamicPlugin'); +const getRandomCharsMock = jest.spyOn(utilsModule, 'getRandomChars'); const originalConsole = { ...console }; const originalServerFlags = window.SERVER_FLAGS; @@ -57,6 +59,10 @@ describe('loadDynamicPlugin', () => { const getAllScriptElements = () => document.querySelectorAll(`[id^="${scriptIDPrefix}"]`); + beforeEach(() => { + getRandomCharsMock.mockImplementation(() => 'r4nd0m'); + }); + it('updates pluginMap and adds a script element to document head', () => { const manifest = getPluginManifest('Test', '1.2.3'); loadDynamicPlugin('http://example.com/test/', manifest); @@ -71,7 +77,9 @@ describe('loadDynamicPlugin', () => { expect(script instanceof HTMLScriptElement).toBe(true); expect(script.parentElement).toBe(document.head); expect(script.id).toBe(getScriptElementID(manifest)); - expect(script.src).toBe('http://example.com/test/plugin-entry.js'); + expect(script.src).toBe('http://example.com/test/plugin-entry.js?cacheBuster=r4nd0m'); + + expect(getRandomCharsMock).toHaveBeenCalledTimes(1); }); it('throws an error if a plugin with the same name is already registered', async () => { @@ -93,12 +101,12 @@ describe('loadDynamicPlugin', () => { const allScripts = getAllScriptElements(); expect(allScripts.length).toBe(1); expect(allScripts[0].id).toBe(getScriptElementID(manifest1)); - expect(allScripts[0].src).toBe('http://example.com/test1/plugin-entry.js'); + expect(allScripts[0].src).toBe('http://example.com/test1/plugin-entry.js?cacheBuster=r4nd0m'); + + expect(getRandomCharsMock).toHaveBeenCalledTimes(1); expect(consoleMock).toHaveBeenCalledTimes(1); - expect(consoleMock).toHaveBeenCalledWith( - 'Loading entry script for plugin Test@1.2.3 from http://example.com/test1/plugin-entry.js', - ); + expect(consoleMock).toHaveBeenCalledWith('Loading entry script for plugin Test@1.2.3'); } }); @@ -132,9 +140,7 @@ describe('loadDynamicPlugin', () => { new Error('Entry script for plugin Test@1.2.3 loaded without callback'), ); expect(consoleMock).toHaveBeenCalledTimes(1); - expect(consoleMock).toHaveBeenCalledWith( - 'Loading entry script for plugin Test@1.2.3 from http://example.com/test/plugin-entry.js', - ); + expect(consoleMock).toHaveBeenCalledWith('Loading entry script for plugin Test@1.2.3'); } }); @@ -154,9 +160,7 @@ describe('loadDynamicPlugin', () => { new Error('Entry script for plugin Test@1.2.3 loaded without callback'), ); expect(consoleMock).toHaveBeenCalledTimes(1); - expect(consoleMock).toHaveBeenCalledWith( - 'Loading entry script for plugin Test@1.2.3 from http://example.com/test/plugin-entry.js', - ); + expect(consoleMock).toHaveBeenCalledWith('Loading entry script for plugin Test@1.2.3'); } }); }); diff --git a/frontend/packages/console-dynamic-plugin-sdk/src/runtime/plugin-loader.ts b/frontend/packages/console-dynamic-plugin-sdk/src/runtime/plugin-loader.ts index 72439645f86..032b7829a20 100644 --- a/frontend/packages/console-dynamic-plugin-sdk/src/runtime/plugin-loader.ts +++ b/frontend/packages/console-dynamic-plugin-sdk/src/runtime/plugin-loader.ts @@ -3,6 +3,7 @@ import * as _ from 'lodash'; import * as semver from 'semver'; import { PluginStore } from '@console/plugin-sdk/src/store'; +import { getRandomChars } from '@console/shared/src/utils/utils'; import { resolveEncodedCodeRefs } from '../coderefs/coderef-resolver'; import { remoteEntryFile } from '../constants'; import { ConsolePluginManifestJSON } from '../schema/plugin-manifest'; @@ -46,9 +47,14 @@ export const loadDynamicPlugin = (baseURL: string, manifest: ConsolePluginManife entryCallbackFired: false, }); + const scriptURL = resolveURL(baseURL, remoteEntryFile, (url) => { + url.search = `?cacheBuster=${getRandomChars()}`; + return url; + }); + const script = document.createElement('script'); script.id = getScriptElementID(manifest); - script.src = resolveURL(baseURL, remoteEntryFile); + script.src = scriptURL; script.async = true; script.onload = () => { @@ -60,10 +66,12 @@ export const loadDynamicPlugin = (baseURL: string, manifest: ConsolePluginManife }; script.onerror = (event) => { - reject(new ErrorWithCause(`Error while loading entry script for plugin ${pluginID}`, event)); + reject( + new ErrorWithCause(`Error while loading plugin entry script from ${scriptURL}`, event), + ); }; - console.info(`Loading entry script for plugin ${pluginID} from ${script.src}`); + console.info(`Loading entry script for plugin ${pluginID}`); document.head.appendChild(script); }); diff --git a/frontend/packages/console-dynamic-plugin-sdk/src/utils/__tests__/url.spec.ts b/frontend/packages/console-dynamic-plugin-sdk/src/utils/__tests__/url.spec.ts index 89d73c45b60..c61b20c2ddd 100644 --- a/frontend/packages/console-dynamic-plugin-sdk/src/utils/__tests__/url.spec.ts +++ b/frontend/packages/console-dynamic-plugin-sdk/src/utils/__tests__/url.spec.ts @@ -1,18 +1,56 @@ +import * as _ from 'lodash'; import { resolveURL } from '../url'; describe('resolveURL', () => { - const getDocumentOrigin = () => 'https://example:1234'; + const getDocumentOrigin = jest.fn(() => 'https://example:1234'); it('uses the base URL as-is if it has the protocol', () => { - expect(resolveURL('http://test', 'foo', getDocumentOrigin)).toBe('http://test/foo'); - expect(resolveURL('http://test/', 'foo', getDocumentOrigin)).toBe('http://test/foo'); - expect(resolveURL('http://test/foo', 'bar', getDocumentOrigin)).toBe('http://test/bar'); - expect(resolveURL('http://test/foo/', 'bar', getDocumentOrigin)).toBe('http://test/foo/bar'); + expect(resolveURL('http://test', 'foobar', _.identity, getDocumentOrigin)).toBe( + 'http://test/foobar', + ); + expect(resolveURL('http://test/', 'foobar', _.identity, getDocumentOrigin)).toBe( + 'http://test/foobar', + ); + expect(resolveURL('http://test/foo', 'bar', _.identity, getDocumentOrigin)).toBe( + 'http://test/bar', + ); + expect(resolveURL('http://test/foo/', 'bar', _.identity, getDocumentOrigin)).toBe( + 'http://test/foo/bar', + ); + + expect(getDocumentOrigin).not.toHaveBeenCalled(); }); it("makes the base URL relative to document origin if it's missing the protocol", () => { - expect(resolveURL('/', 'foo', getDocumentOrigin)).toBe('https://example:1234/foo'); - expect(resolveURL('/foo', 'bar', getDocumentOrigin)).toBe('https://example:1234/bar'); - expect(resolveURL('/foo/', 'bar', getDocumentOrigin)).toBe('https://example:1234/foo/bar'); + expect(resolveURL('/', 'foobar', _.identity, getDocumentOrigin)).toBe( + 'https://example:1234/foobar', + ); + expect(resolveURL('/foo', 'bar', _.identity, getDocumentOrigin)).toBe( + 'https://example:1234/bar', + ); + expect(resolveURL('/foo/', 'bar', _.identity, getDocumentOrigin)).toBe( + 'https://example:1234/foo/bar', + ); + + expect(getDocumentOrigin).toHaveBeenCalledTimes(3); + }); + + it('calls the URL processing callback before returning the URL string', () => { + const processURL = jest.fn((url: URL) => { + const newURL = new URL(url); + + newURL.protocol = 'http'; + newURL.hostname = 'custom-host'; + newURL.port = '8080'; + newURL.search = '?test=true'; + + return newURL; + }); + + expect(resolveURL('/', 'foobar', processURL, getDocumentOrigin)).toBe( + 'http://custom-host:8080/foobar?test=true', + ); + + expect(processURL).toHaveBeenCalledWith(new URL('https://example:1234/foobar')); }); }); diff --git a/frontend/packages/console-dynamic-plugin-sdk/src/utils/url.ts b/frontend/packages/console-dynamic-plugin-sdk/src/utils/url.ts index ba68d5075ab..53bfb5a4edb 100644 --- a/frontend/packages/console-dynamic-plugin-sdk/src/utils/url.ts +++ b/frontend/packages/console-dynamic-plugin-sdk/src/utils/url.ts @@ -7,13 +7,14 @@ import * as _ from 'lodash'; * * @param base Base URL. * @param to Target resource URL. - * @param options Resolution options. + * @param processURL Custom URL processing callback. */ export const resolveURL = ( base: string, to: string, + processURL: (url: URL) => URL = _.identity, getDocumentOrigin: () => string = _.constant(window.location.origin), ) => { const baseAbsoluteURL = base.indexOf('://') === -1 ? getDocumentOrigin() + base : base; - return new URL(to, baseAbsoluteURL).toString(); + return processURL(new URL(to, baseAbsoluteURL)).toString(); };