From 2b60c636c2a451758c91c24da0946a9e63103c19 Mon Sep 17 00:00:00 2001 From: Jonathan Adshead Date: Fri, 3 Mar 2023 10:27:43 -0800 Subject: [PATCH] fix(load-modules): unnecessary calls to updateModuleRegistry --- .../__snapshots__/loadModules.spec.jsx.snap | 8 +- __tests__/server/utils/loadModules.spec.jsx | 101 ++++++++++++------ package-lock.json | 43 ++++---- package.json | 1 + src/server/utils/loadModules.js | 9 ++ 5 files changed, 100 insertions(+), 62 deletions(-) diff --git a/__tests__/server/utils/__snapshots__/loadModules.spec.jsx.snap b/__tests__/server/utils/__snapshots__/loadModules.spec.jsx.snap index cfeee2af6..f1d6501e4 100644 --- a/__tests__/server/utils/__snapshots__/loadModules.spec.jsx.snap +++ b/__tests__/server/utils/__snapshots__/loadModules.spec.jsx.snap @@ -16,10 +16,10 @@ exports[`loadModules updates the client module map cache 1`] = ` "browser": { "modules": { "some-root": { - "baseUrl": "https://example.com/cdn/some-root/2.2.2/", + "baseUrl": "https://example.com/cdn/some-root/1.1.2/", "browser": { "integrity": "nggdfhr34", - "url": "https://example.com/cdn/some-root/2.2.2/some-root.browser.js", + "url": "https://example.com/cdn/some-root/1.1.2/some-root.browser.js", }, }, }, @@ -27,10 +27,10 @@ exports[`loadModules updates the client module map cache 1`] = ` "legacyBrowser": { "modules": { "some-root": { - "baseUrl": "https://example.com/cdn/some-root/2.2.2/", + "baseUrl": "https://example.com/cdn/some-root/1.1.2/", "legacyBrowser": { "integrity": "7567ee", - "url": "https://example.com/cdn/some-root/2.2.2/some-root.legacy.browser.js", + "url": "https://example.com/cdn/some-root/1.1.2/some-root.legacy.browser.js", }, }, }, diff --git a/__tests__/server/utils/loadModules.spec.jsx b/__tests__/server/utils/loadModules.spec.jsx index 2372fa1ac..21fab912e 100644 --- a/__tests__/server/utils/loadModules.spec.jsx +++ b/__tests__/server/utils/loadModules.spec.jsx @@ -44,47 +44,36 @@ jest.mock('../../../src/server/middleware/csp', () => ({ const RootModule = () => ({}); -describe('loadModules', () => { - const moduleMap = { - modules: { - 'some-root': { - node: { - url: 'https://example.com/cdn/some-root/2.2.2/some-root.node.js', - integrity: '4y45hr', - }, - browser: { - url: 'https://example.com/cdn/some-root/2.2.2/some-root.browser.js', - integrity: 'nggdfhr34', - }, - legacyBrowser: { - url: 'https://example.com/cdn/some-root/2.2.2/some-root.legacy.browser.js', - integrity: '7567ee', - }, +let modules; +let fetchedModuleMap; +function setFetchedRootModuleVersion(version) { + modules = { + 'some-root': { + node: { + url: `https://example.com/cdn/some-root/${version}/some-root.node.js`, + integrity: '4y45hr', + }, + browser: { + url: `https://example.com/cdn/some-root/${version}/some-root.browser.js`, + integrity: 'nggdfhr34', + }, + legacyBrowser: { + url: `https://example.com/cdn/some-root/${version}/some-root.legacy.browser.js`, + integrity: '7567ee', }, }, }; + fetchedModuleMap = { modules }; +} + +describe('loadModules', () => { beforeAll(() => { getModule.mockImplementation(() => RootModule); - updateModuleRegistry.mockImplementation(() => ({ - 'some-root': { - node: { - url: 'https://example.com/cdn/some-root/2.2.2/some-root.node.js', - integrity: '4y45hr', - }, - browser: { - url: 'https://example.com/cdn/some-root/2.2.2/some-root.browser.js', - integrity: 'nggdfhr34', - }, - legacyBrowser: { - url: 'https://example.com/cdn/some-root/2.2.2/some-root.legacy.browser.js', - integrity: '7567ee', - }, - }, - })); + updateModuleRegistry.mockImplementation(() => modules); global.fetch = jest.fn(() => Promise.resolve({ - json: () => Promise.resolve(moduleMap), + json: () => Promise.resolve(fetchedModuleMap), })); }); @@ -94,9 +83,10 @@ describe('loadModules', () => { }); it('updates the holocron module registry', async () => { + setFetchedRootModuleVersion('1.1.1'); await loadModules(); expect(updateModuleRegistry).toHaveBeenCalledWith({ - moduleMap: addBaseUrlToModuleMap(moduleMap), + moduleMap: addBaseUrlToModuleMap(fetchedModuleMap), batchModulesToUpdate: require('../../../src/server/utils/batchModulesToUpdate').default, getModulesToUpdate: require('../../../src/server/utils/getModulesToUpdate').default, onModuleLoad: require('../../../src/server/utils/onModuleLoad').default, @@ -104,11 +94,34 @@ describe('loadModules', () => { }); it('updates the client module map cache', async () => { + setFetchedRootModuleVersion('1.1.2'); await loadModules(); expect(getClientModuleMapCache()).toMatchSnapshot(); }); + it('returns loaded modules', async () => { + setFetchedRootModuleVersion('2.0.0'); + const loadedModules = await loadModules(); + expect(loadedModules).toEqual({ + 'some-root': { + browser: { + integrity: 'nggdfhr34', + url: 'https://example.com/cdn/some-root/2.0.0/some-root.browser.js', + }, + legacyBrowser: { + integrity: '7567ee', + url: 'https://example.com/cdn/some-root/2.0.0/some-root.legacy.browser.js', + }, + node: { + integrity: '4y45hr', + url: 'https://example.com/cdn/some-root/2.0.0/some-root.node.js', + }, + }, + }); + }); + it('doesnt update caches when there are no changes', async () => { + setFetchedRootModuleVersion('1.1.3'); updateModuleRegistry.mockImplementationOnce(() => ({})); await loadModules(); expect(getClientModuleMapCache()).toMatchSnapshot(); @@ -118,12 +131,13 @@ describe('loadModules', () => { RootModule[CONFIGURATION_KEY] = { csp: "default-src 'none';", }; - + setFetchedRootModuleVersion('1.1.4'); await loadModules(); expect(updateCSP).toHaveBeenCalledWith("default-src 'none';"); }); it('calls updateCSP even when csp is not set', async () => { + setFetchedRootModuleVersion('1.1.5'); delete RootModule[CONFIGURATION_KEY].csp; await loadModules(); expect(updateCSP).toHaveBeenCalledWith(undefined); @@ -150,8 +164,25 @@ describe('loadModules', () => { }); it('does not attempt to update CSP', async () => { + setFetchedRootModuleVersion('1.1.6'); await loadModules(); expect(updateCSP).not.toHaveBeenCalled(); }); }); + + describe('when module map does not change', () => { + beforeAll(async () => { + setFetchedRootModuleVersion('1.1.1'); + await loadModules(); + jest.clearAllMocks(); + }); + it('does not updateModuleRegistry', async () => { + await loadModules(); + expect(updateModuleRegistry).not.toHaveBeenCalled(); + }); + it('does not return any modules', async () => { + const loadedModules = await loadModules(); + expect(loadedModules).toEqual({}); + }); + }); }); diff --git a/package-lock.json b/package-lock.json index 059d6c96c..7cc159330 100644 --- a/package-lock.json +++ b/package-lock.json @@ -44,6 +44,7 @@ "joi": "^17.6.0", "lean-intl": "^4.2.2", "matcher": "^4.0.0", + "object-hash": "^3.0.0", "opossum": "^7.1.0", "opossum-prometheus": "^0.3.0", "pidusage": "^3.0.2", @@ -10202,6 +10203,14 @@ "url": "https://github.com/sponsors/sindresorhus" } }, + "node_modules/eslint-loader/node_modules/object-hash": { + "version": "2.2.0", + "resolved": "https://registry.npmjs.org/object-hash/-/object-hash-2.2.0.tgz", + "integrity": "sha512-gScRMn0bS5fH+IuwyIFgnh9zBdo4DV+6GhygmWM9HyNJSgS0hScp1f5vjtm7oIIOiT9trXrShAkLFSc2IqKNgw==", + "engines": { + "node": ">= 6" + } + }, "node_modules/eslint-loader/node_modules/p-locate": { "version": "4.1.0", "resolved": "https://registry.npmjs.org/p-locate/-/p-locate-4.1.0.tgz", @@ -16766,15 +16775,6 @@ "node": ">=10.0.0" } }, - "node_modules/lockfile-lint-api/node_modules/object-hash": { - "version": "3.0.0", - "resolved": "https://registry.npmjs.org/object-hash/-/object-hash-3.0.0.tgz", - "integrity": "sha512-RSn9F68PjH9HqtltsSnqYC1XXoWe9Bju5+213R98cNGttag9q9yAOTzdbsqvIa7aNm5WffBZFpWYr2aWrklWAw==", - "dev": true, - "engines": { - "node": ">= 6" - } - }, "node_modules/lockfile-lint/node_modules/yargs": { "version": "16.2.0", "resolved": "https://registry.npmjs.org/yargs/-/yargs-16.2.0.tgz", @@ -18084,9 +18084,9 @@ } }, "node_modules/object-hash": { - "version": "2.2.0", - "resolved": "https://registry.npmjs.org/object-hash/-/object-hash-2.2.0.tgz", - "integrity": "sha512-gScRMn0bS5fH+IuwyIFgnh9zBdo4DV+6GhygmWM9HyNJSgS0hScp1f5vjtm7oIIOiT9trXrShAkLFSc2IqKNgw==", + "version": "3.0.0", + "resolved": "https://registry.npmjs.org/object-hash/-/object-hash-3.0.0.tgz", + "integrity": "sha512-RSn9F68PjH9HqtltsSnqYC1XXoWe9Bju5+213R98cNGttag9q9yAOTzdbsqvIa7aNm5WffBZFpWYr2aWrklWAw==", "engines": { "node": ">= 6" } @@ -33034,6 +33034,11 @@ "semver": "^6.0.0" } }, + "object-hash": { + "version": "2.2.0", + "resolved": "https://registry.npmjs.org/object-hash/-/object-hash-2.2.0.tgz", + "integrity": "sha512-gScRMn0bS5fH+IuwyIFgnh9zBdo4DV+6GhygmWM9HyNJSgS0hScp1f5vjtm7oIIOiT9trXrShAkLFSc2IqKNgw==" + }, "p-locate": { "version": "4.1.0", "resolved": "https://registry.npmjs.org/p-locate/-/p-locate-4.1.0.tgz", @@ -38071,14 +38076,6 @@ "requires": { "@yarnpkg/parsers": "^3.0.0-rc.6", "object-hash": "^3.0.0" - }, - "dependencies": { - "object-hash": { - "version": "3.0.0", - "resolved": "https://registry.npmjs.org/object-hash/-/object-hash-3.0.0.tgz", - "integrity": "sha512-RSn9F68PjH9HqtltsSnqYC1XXoWe9Bju5+213R98cNGttag9q9yAOTzdbsqvIa7aNm5WffBZFpWYr2aWrklWAw==", - "dev": true - } } }, "lodash": { @@ -39127,9 +39124,9 @@ } }, "object-hash": { - "version": "2.2.0", - "resolved": "https://registry.npmjs.org/object-hash/-/object-hash-2.2.0.tgz", - "integrity": "sha512-gScRMn0bS5fH+IuwyIFgnh9zBdo4DV+6GhygmWM9HyNJSgS0hScp1f5vjtm7oIIOiT9trXrShAkLFSc2IqKNgw==" + "version": "3.0.0", + "resolved": "https://registry.npmjs.org/object-hash/-/object-hash-3.0.0.tgz", + "integrity": "sha512-RSn9F68PjH9HqtltsSnqYC1XXoWe9Bju5+213R98cNGttag9q9yAOTzdbsqvIa7aNm5WffBZFpWYr2aWrklWAw==" }, "object-inspect": { "version": "1.12.2", diff --git a/package.json b/package.json index 42392516c..572191329 100644 --- a/package.json +++ b/package.json @@ -115,6 +115,7 @@ "joi": "^17.6.0", "lean-intl": "^4.2.2", "matcher": "^4.0.0", + "object-hash": "^3.0.0", "opossum": "^7.1.0", "opossum-prometheus": "^0.3.0", "pidusage": "^3.0.2", diff --git a/src/server/utils/loadModules.js b/src/server/utils/loadModules.js index 6362008d0..6f6c71ca2 100644 --- a/src/server/utils/loadModules.js +++ b/src/server/utils/loadModules.js @@ -17,6 +17,7 @@ import { getModule } from 'holocron'; import { updateModuleRegistry } from 'holocron/server'; +import hash from 'object-hash'; import onModuleLoad, { CONFIGURATION_KEY } from './onModuleLoad'; import batchModulesToUpdate from './batchModulesToUpdate'; import getModulesToUpdate from './getModulesToUpdate'; @@ -25,9 +26,17 @@ import { setClientModuleMapCache } from './clientModuleMapCache'; import { updateCSP } from '../middleware/csp'; import addBaseUrlToModuleMap from './addBaseUrlToModuleMap'; +let cashedModuleMapHash; + const loadModules = async () => { const moduleMapResponse = await fetch(process.env.HOLOCRON_MODULE_MAP_URL); const moduleMap = addBaseUrlToModuleMap(await moduleMapResponse.json()); + + const moduleMapHash = hash(moduleMap); + if (cashedModuleMapHash && cashedModuleMapHash === moduleMapHash) { + return {}; + } + cashedModuleMapHash = moduleMapHash; const serverConfig = getServerStateConfig(); const loadedModules = await updateModuleRegistry({