Merge pull request #13035 from vojtechszocs/add-plugin-script-cache-b…

…uster

OCPBUGS-3495: Add cacheBuster query string when requesting plugin entry scripts

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<HTMLScriptElement>(`[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');
     }
   });
 });

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

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'));
   });
 });

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