From 23d5cf744d81c4b276376276e0b8a866e19bdcee Mon Sep 17 00:00:00 2001 From: Alex Thirlwall Date: Thu, 18 Apr 2024 15:17:36 +0200 Subject: [PATCH] If session cache Id changes, reset the cache and pending requests --- .changeset/quick-seas-help.md | 5 +++ packages/ajax/src/PendingRequestStore.js | 1 + packages/ajax/src/cacheManager.js | 11 +++++- .../src/interceptors/cacheInterceptors.js | 6 +++ packages/ajax/test/Ajax.test.js | 26 ++++++++++++- .../interceptors/cacheInterceptors.test.js | 38 ++++++++++++++++++- 6 files changed, 84 insertions(+), 3 deletions(-) create mode 100644 .changeset/quick-seas-help.md diff --git a/.changeset/quick-seas-help.md b/.changeset/quick-seas-help.md new file mode 100644 index 0000000000..d3b1e5f579 --- /dev/null +++ b/.changeset/quick-seas-help.md @@ -0,0 +1,5 @@ +--- +'@lion/ajax': patch +--- + +Reset cache and pending requests when cache session ID changes diff --git a/packages/ajax/src/PendingRequestStore.js b/packages/ajax/src/PendingRequestStore.js index 38f3e51fff..7aba3b63cf 100644 --- a/packages/ajax/src/PendingRequestStore.js +++ b/packages/ajax/src/PendingRequestStore.js @@ -55,6 +55,7 @@ export default class PendingRequestStore { } reset() { + this.resolveMatching(/.*/); this._pendingRequests = {}; } } diff --git a/packages/ajax/src/cacheManager.js b/packages/ajax/src/cacheManager.js index e96545a9d3..fd01131620 100644 --- a/packages/ajax/src/cacheManager.js +++ b/packages/ajax/src/cacheManager.js @@ -32,6 +32,15 @@ export const pendingRequestStore = new PendingRequestStore(); */ export const isCurrentSessionId = cacheId => cacheId === cacheSessionId; +/** + * Sets the current cache session ID. + * + * @param {string} id The id that will be tied to the current session + */ +export const setCacheSessionId = id => { + cacheSessionId = id; +}; + /** * Resets the cache session when the cacheId changes. * @@ -43,7 +52,7 @@ export const resetCacheSession = cacheId => { throw new Error('Invalid cache identifier'); } if (!isCurrentSessionId(cacheId)) { - cacheSessionId = cacheId; + setCacheSessionId(cacheId); ajaxCache.reset(); pendingRequestStore.reset(); } diff --git a/packages/ajax/src/interceptors/cacheInterceptors.js b/packages/ajax/src/interceptors/cacheInterceptors.js index 1cb33883d2..53b1f3bdc3 100644 --- a/packages/ajax/src/interceptors/cacheInterceptors.js +++ b/packages/ajax/src/interceptors/cacheInterceptors.js @@ -96,6 +96,12 @@ const createCacheRequestInterceptor = if (pendingRequest) { // there is another concurrent request, wait for it to finish await pendingRequest; + + // If session ID changes while waiting for the pending request to complete, + // then do not read the cache. + if (!isCurrentSessionId(request.cacheSessionId)) { + return request; + } } const cachedResponse = ajaxCache.get(requestId, { maxAge, maxResponseSize }); diff --git a/packages/ajax/test/Ajax.test.js b/packages/ajax/test/Ajax.test.js index 4e0d82a811..0c6049ed2d 100644 --- a/packages/ajax/test/Ajax.test.js +++ b/packages/ajax/test/Ajax.test.js @@ -503,7 +503,7 @@ describe('Ajax', () => { expect(customAjax._responseInterceptors.length).to.equal(1); }); - describe('caching interceptors', async () => { + describe('Caching interceptors: synchronous getCacheIdentifier tests', async () => { /** * @type {Ajax} */ @@ -530,6 +530,30 @@ describe('Ajax', () => { expect(secondResponse.headers.get('Content-Type')).to.equal('application/json'); }); + it('resets the cache if the cache ID changes', async () => { + /* Three calls to the same endpoint should result in a + single fetchStubCall due to caching */ + await customAjax.fetch('/foo'); + await customAjax.fetch('/foo'); + await customAjax.fetch('/foo'); + expect(fetchStub.callCount).to.equal(1); + + newCacheId(); + await customAjax.fetch('/foo'); + + /* The newCacheId call should reset the cache, thereby adding an + extra call to the fetchStub call count. */ + expect(fetchStub.callCount).to.equal(2); + }); + + it('Completes pending requests when the cache ID changes', async () => { + const requestOne = customAjax.fetch('/foo').then(() => 'completedRequestOne'); + newCacheId(); + const requestTwo = customAjax.fetch('/foo').then(() => 'completedRequestTwo'); + expect(await requestOne).to.equal('completedRequestOne'); + expect(await requestTwo).to.equal('completedRequestTwo'); + }); + it('works with fetchJson', async () => { fetchStub.returns(Promise.resolve(new Response('{"a":1,"b":2}', responseInit()))); diff --git a/packages/ajax/test/interceptors/cacheInterceptors.test.js b/packages/ajax/test/interceptors/cacheInterceptors.test.js index 22c87f992b..70cd5bd19f 100644 --- a/packages/ajax/test/interceptors/cacheInterceptors.test.js +++ b/packages/ajax/test/interceptors/cacheInterceptors.test.js @@ -4,7 +4,12 @@ import { Ajax, createCacheInterceptors } from '@lion/ajax'; import { isResponseContentTypeSupported } from '../../src/interceptors/cacheInterceptors.js'; // TODO: these are private API? should they be exposed? if not why do we test them? -import { extendCacheOptions, resetCacheSession, ajaxCache } from '../../src/cacheManager.js'; +import { + extendCacheOptions, + resetCacheSession, + ajaxCache, + setCacheSessionId, +} from '../../src/cacheManager.js'; const MOCK_RESPONSE = 'mock response'; @@ -437,6 +442,37 @@ describe('cache interceptors', () => { expect(fetchStub.callCount).to.equal(1); }); + it('Does not use cached request when session ID changes during processing a pending request', async () => { + addCacheInterceptors(ajax, { + useCache: true, + maxAge: 750, + }); + + // Reset cache + newCacheId(); + + /* Bump sessionID manually in an injected response interceptor. + This will simulate the cache session ID getting changed while + waiting for a pending request */ + + ajax._responseInterceptors.unshift(async (/** @type {Response} */ response) => { + newCacheId(); + setCacheSessionId(getCacheIdentifier()); + return response; + }); + + const requestOne = ajax.fetch('/foo').then(() => 'completedRequestOne'); + const requestTwo = ajax.fetch('/foo').then(() => 'completedRequestTwo'); + expect(await requestOne).to.equal('completedRequestOne'); + // At this point the response interceptor of requestOne has called setCacheSessionId + expect(await requestTwo).to.equal('completedRequestTwo'); + + /* Neither call should use the cache. During the first call there is no cache entry for '/foo'. + During the second call there is, but since the first call's injected interceptor has bumped + the cache session ID, it shouldn't use the cached response. */ + expect(fetchStub.callCount).to.equal(2); + }); + it('does save to the cache when `maxResponseSize` is specified and the response size is within the threshold', async () => { // Given newCacheId();