From efcdf653a4e12762fc627118891cfce4ac4b1f54 Mon Sep 17 00:00:00 2001 From: Martin Pool Date: Mon, 25 Apr 2022 17:58:45 +0200 Subject: [PATCH] feat: content-type filter for ajax cache --- .changeset/pretty-mice-beam.md | 5 + packages/ajax/src/cacheManager.js | 10 +- .../src/interceptors/cacheInterceptors.js | 19 +- packages/ajax/test/cacheManager.test.js | 22 +- .../interceptors/cacheInterceptors.test.js | 303 +++++++++++------- packages/ajax/types/types.d.ts | 1 + 6 files changed, 235 insertions(+), 125 deletions(-) create mode 100644 .changeset/pretty-mice-beam.md diff --git a/.changeset/pretty-mice-beam.md b/.changeset/pretty-mice-beam.md new file mode 100644 index 000000000..667407ed7 --- /dev/null +++ b/.changeset/pretty-mice-beam.md @@ -0,0 +1,5 @@ +--- +'@lion/ajax': minor +--- + +Add cache filter for content types diff --git a/packages/ajax/src/cacheManager.js b/packages/ajax/src/cacheManager.js index 67ad5c3b0..3e07eeeb8 100644 --- a/packages/ajax/src/cacheManager.js +++ b/packages/ajax/src/cacheManager.js @@ -82,6 +82,7 @@ export const extendCacheOptions = ({ requestIdFunction = DEFAULT_GET_REQUEST_ID, invalidateUrls, invalidateUrlsRegex, + contentTypes, }) => ({ useCache, methods, @@ -89,6 +90,7 @@ export const extendCacheOptions = ({ requestIdFunction, invalidateUrls, invalidateUrlsRegex, + contentTypes, }); /** @@ -101,6 +103,7 @@ export const validateCacheOptions = ({ requestIdFunction, invalidateUrls, invalidateUrlsRegex, + contentTypes, } = {}) => { if (useCache !== undefined && typeof useCache !== 'boolean') { throw new Error('Property `useCache` must be a `boolean`'); @@ -112,14 +115,17 @@ export const validateCacheOptions = ({ throw new Error('Property `maxAge` must be a finite `number`'); } if (invalidateUrls !== undefined && !Array.isArray(invalidateUrls)) { - throw new Error('Property `invalidateUrls` must be an `Array` or `falsy`'); + throw new Error('Property `invalidateUrls` must be an `Array` or `undefined`'); } if (invalidateUrlsRegex !== undefined && !(invalidateUrlsRegex instanceof RegExp)) { - throw new Error('Property `invalidateUrlsRegex` must be a `RegExp` or `falsy`'); + throw new Error('Property `invalidateUrlsRegex` must be a `RegExp` or `undefined`'); } if (requestIdFunction !== undefined && typeof requestIdFunction !== 'function') { throw new Error('Property `requestIdFunction` must be a `function`'); } + if (contentTypes !== undefined && !Array.isArray(contentTypes)) { + throw new Error('Property `contentTypes` must be an `Array` or `undefined`'); + } }; /** diff --git a/packages/ajax/src/interceptors/cacheInterceptors.js b/packages/ajax/src/interceptors/cacheInterceptors.js index dd440a9ea..e33b90af9 100644 --- a/packages/ajax/src/interceptors/cacheInterceptors.js +++ b/packages/ajax/src/interceptors/cacheInterceptors.js @@ -19,6 +19,18 @@ import { const isMethodSupported = (cacheOptions, method) => cacheOptions.methods.includes(method.toLowerCase()); +/** + * Tests whether the response content type is supported by the `contentTypes` whitelist + * @param {Response} response + * @param {CacheOptions} cacheOptions + * @returns {boolean} `true` if the contentTypes property is not an array, or if the value of the Content-Type header is in the array + */ +const isResponseContentTypeSupported = (response, { contentTypes } = {}) => { + if (!Array.isArray(contentTypes)) return true; + + return contentTypes.includes(String(response.headers.get('Content-Type'))); +}; + /** * Request interceptor to return relevant cached requests * @param {function(): string} getCacheId used to invalidate cache if identifier is changed @@ -58,7 +70,7 @@ const createCacheRequestInterceptor = } const cachedResponse = ajaxCache.get(requestId, cacheOptions.maxAge); - if (cachedResponse) { + if (cachedResponse && isResponseContentTypeSupported(cachedResponse, cacheOptions)) { // Return the response from cache request.cacheOptions = request.cacheOptions ?? { useCache: false }; /** @type {CacheResponse} */ @@ -92,7 +104,10 @@ const createCacheResponseInterceptor = if (!response.fromCache && isMethodSupported(cacheOptions, response.request.method)) { const requestId = cacheOptions.requestIdFunction(response.request); - if (isCurrentSessionId(response.request.cacheSessionId)) { + if ( + isCurrentSessionId(response.request.cacheSessionId) && + isResponseContentTypeSupported(response, cacheOptions) + ) { // Cache the response ajaxCache.set(requestId, response.clone()); } diff --git a/packages/ajax/test/cacheManager.test.js b/packages/ajax/test/cacheManager.test.js index 904119849..9aed123dd 100644 --- a/packages/ajax/test/cacheManager.test.js +++ b/packages/ajax/test/cacheManager.test.js @@ -74,6 +74,7 @@ describe('cacheManager', () => { requestIdFunction, invalidateUrls: invalidateUrlsResult, invalidateUrlsRegex: invalidateUrlsRegexResult, + contentTypes, } = extendCacheOptions({ invalidateUrls, invalidateUrlsRegex }); // Assert expect(useCache).to.be.false; @@ -82,6 +83,7 @@ describe('cacheManager', () => { expect(typeof requestIdFunction).to.eql('function'); expect(invalidateUrlsResult).to.equal(invalidateUrls); expect(invalidateUrlsRegexResult).to.equal(invalidateUrlsRegex); + expect(contentTypes).to.be.undefined; }); it('the DEFAULT_GET_REQUEST_ID function throws when called with no arguments', () => { @@ -197,7 +199,7 @@ describe('cacheManager', () => { it('does not accept anything else', () => { // @ts-ignore expect(() => validateCacheOptions({ invalidateUrls: 'not-an-array' })).to.throw( - 'Property `invalidateUrls` must be an `Array` or `falsy`', + 'Property `invalidateUrls` must be an `Array` or `undefined`', ); }); }); @@ -213,7 +215,7 @@ describe('cacheManager', () => { // @ts-ignore expect(() => validateCacheOptions({ invalidateUrlsRegex: 'a string is not a regex' }), - ).to.throw('Property `invalidateUrlsRegex` must be a `RegExp` or `falsy`'); + ).to.throw('Property `invalidateUrlsRegex` must be a `RegExp` or `undefined`'); }); }); describe('the requestIdFunction property', () => { @@ -233,6 +235,22 @@ describe('cacheManager', () => { ); }); }); + describe('the contentTypes property', () => { + it('accepts an array', () => { + // @ts-ignore Typescript requires this to be an array of string, but this is not checked by validateCacheOptions + expect(() => validateCacheOptions({ contentTypes: [6, 'elements', 'in', 1, true, Array] })) + .not.to.throw; + }); + it('accepts undefined', () => { + expect(() => validateCacheOptions({ contentTypes: undefined })).not.to.throw; + }); + it('does not accept anything else', () => { + // @ts-ignore + expect(() => validateCacheOptions({ contentTypes: 'not-an-array' })).to.throw( + 'Property `contentTypes` must be an `Array` or `undefined`', + ); + }); + }); }); describe('invalidateMatchingCache', () => { diff --git a/packages/ajax/test/interceptors/cacheInterceptors.test.js b/packages/ajax/test/interceptors/cacheInterceptors.test.js index ca14abe42..3b88c4dc3 100644 --- a/packages/ajax/test/interceptors/cacheInterceptors.test.js +++ b/packages/ajax/test/interceptors/cacheInterceptors.test.js @@ -22,6 +22,8 @@ describe('cache interceptors', () => { let cacheId; /** @type {sinon.SinonStub} */ let fetchStub; + /** @type {Response} */ + let mockResponse; const getCacheIdentifier = () => String(cacheId); /** @type {sinon.SinonSpy} */ let ajaxRequestSpy; @@ -51,8 +53,9 @@ describe('cache interceptors', () => { beforeEach(() => { ajax = new Ajax(); + mockResponse = new Response('mock response'); fetchStub = sinon.stub(window, 'fetch'); - fetchStub.returns(Promise.resolve(new Response('mock response'))); + fetchStub.resolves(mockResponse); ajaxRequestSpy = sinon.spy(ajax, 'fetch'); }); @@ -291,7 +294,7 @@ describe('cache interceptors', () => { ); // @ts-ignore not an actual valid CacheResponse object - await cacheResponseInterceptor({ request: { method: 'get' } }) + await cacheResponseInterceptor({ request: { method: 'get' }, headers: new Headers() }) .then(() => expect('everything').to.be.ok) .catch(err => expect.fail( @@ -299,6 +302,185 @@ describe('cache interceptors', () => { ), ); }); + + it('caches concurrent requests', async () => { + newCacheId(); + + const clock = sinon.useFakeTimers(); + + fetchStub.onFirstCall().returns(returnResponseOnTick(900, 1)); + fetchStub.onSecondCall().returns(returnResponseOnTick(1900, 2)); + + addCacheInterceptors(ajax, { + useCache: true, + maxAge: 750, + }); + + const firstRequest = ajax.fetch('/test').then(r => r.text()); + const concurrentFirstRequest1 = ajax.fetch('/test').then(r => r.text()); + const concurrentFirstRequest2 = ajax.fetch('/test').then(r => r.text()); + + clock.tick(1000); + + // firstRequest is cached at tick 1000 in the next line! + const firstResponses = await Promise.all([ + firstRequest, + concurrentFirstRequest1, + concurrentFirstRequest2, + ]); + + expect(fetchStub.callCount).to.equal(1); + + const cachedFirstRequest = ajax.fetch('/test').then(r => r.text()); + + clock.tick(500); + + const cachedFirstResponse = await cachedFirstRequest; + + expect(fetchStub.callCount).to.equal(1); + + const secondRequest = ajax.fetch('/test').then(r => r.text()); + const secondConcurrentRequest = ajax.fetch('/test').then(r => r.text()); + + clock.tick(1000); + + const secondResponses = await Promise.all([secondRequest, secondConcurrentRequest]); + + expect(fetchStub.callCount).to.equal(2); + + expect(firstResponses).to.eql(['mock response 1', 'mock response 1', 'mock response 1']); + + expect(cachedFirstResponse).to.equal('mock response 1'); + + expect(secondResponses).to.eql(['mock response 2', 'mock response 2']); + + clock.restore(); + }); + + it('preserves status and headers when returning cached response', async () => { + newCacheId(); + fetchStub.returns( + Promise.resolve( + new Response('mock response', { status: 206, headers: { 'x-foo': 'x-bar' } }), + ), + ); + + addCacheInterceptors(ajax, { + useCache: true, + maxAge: 100, + }); + + const response1 = await ajax.fetch('/test'); + const response2 = await ajax.fetch('/test'); + expect(fetchStub.callCount).to.equal(1); + expect(response1.status).to.equal(206); + expect(response1.headers.get('x-foo')).to.equal('x-bar'); + expect(response2.status).to.equal(206); + expect(response2.headers.get('x-foo')).to.equal('x-bar'); + }); + + it('does save to the cache when `contentTypes` is specified and a supported content type is returned', async () => { + // Given + newCacheId(); + mockResponse.headers.set('content-type', 'application/xml'); + addCacheInterceptors(ajax, { + useCache: true, + contentTypes: ['application/json', 'application/xml'], + }); + + // When + await ajax.fetch('/test'); + await ajax.fetch('/test'); + + // Then + expect(fetchStub.callCount).to.equal(1); + }); + }); + + describe('Bypassing the cache', () => { + it('caches response but does not return it when expiration time is 0', async () => { + newCacheId(); + + addCacheInterceptors(ajax, { + useCache: true, + maxAge: 0, + }); + + const clock = sinon.useFakeTimers(); + + await ajax.fetch('/test'); + + expect(ajaxRequestSpy.calledOnce).to.be.true; + + expect(ajaxRequestSpy.calledWith('/test')).to.be.true; + + clock.tick(1); + + await ajax.fetch('/test'); + + clock.restore(); + + expect(fetchStub.callCount).to.equal(2); + }); + + it('does not use cache when cacheOption `useCache: false` is passed to fetch method', async () => { + // Given + newCacheId(); + addCacheInterceptors(ajax, { useCache: true }); + + // When + await ajax.fetch('/test'); + await ajax.fetch('/test'); + + // Then + expect(fetchStub.callCount).to.equal(1); + + // When + await ajax.fetch('/test', { cacheOptions: { useCache: false } }); + + // Then + expect(fetchStub.callCount).to.equal(2); + }); + + it('does not save to the cache when `contentTypes` is specified and an unsupported content type is returned', async () => { + // Given + newCacheId(); + mockResponse.headers.set('content-type', 'text/html'); + addCacheInterceptors(ajax, { + useCache: true, + contentTypes: ['application/json', 'application/xml'], + }); + + // When + await ajax.fetch('/test'); + await ajax.fetch('/test', { cacheOptions: { contentTypes: ['text/html'] } }); + + // Then + expect(fetchStub.callCount).to.equal(2); + }); + + it('does not read from the cache when `contentTypes` is specified and an unsupported content type is returned', async () => { + // Given + newCacheId(); + mockResponse.headers.set('content-type', 'application/json'); + addCacheInterceptors(ajax, { + useCache: true, + contentTypes: ['application/json', 'application/xml'], + }); + + // When + await ajax.fetch('/test'); + await ajax.fetch('/test'); + + // Then + expect(fetchStub.callCount).to.equal(1); + + // When + await ajax.fetch('/test', { cacheOptions: { contentTypes: [] } }); + + // Then + expect(fetchStub.callCount).to.equal(2); + }); }); describe('Cache invalidation', () => { @@ -443,101 +625,6 @@ describe('cache interceptors', () => { expect(fetchStub.callCount).to.equal(3); }); - it('caches response but does not return it when expiration time is 0', async () => { - newCacheId(); - - addCacheInterceptors(ajax, { - useCache: true, - maxAge: 0, - }); - - const clock = sinon.useFakeTimers(); - - await ajax.fetch('/test'); - - expect(ajaxRequestSpy.calledOnce).to.be.true; - - expect(ajaxRequestSpy.calledWith('/test')).to.be.true; - - clock.tick(1); - - await ajax.fetch('/test'); - - clock.restore(); - - expect(fetchStub.callCount).to.equal(2); - }); - - it('does not use cache when cacheOption `useCache: false` is passed to fetch method', async () => { - // Given - addCacheInterceptors(ajax, { useCache: true }); - - // When - await ajax.fetch('/test'); - await ajax.fetch('/test'); - - // Then - expect(fetchStub.callCount).to.equal(1); - - // When - await ajax.fetch('/test', { cacheOptions: { useCache: false } }); - - // Then - expect(fetchStub.callCount).to.equal(2); - }); - - it('caches concurrent requests', async () => { - newCacheId(); - - const clock = sinon.useFakeTimers(); - - fetchStub.onFirstCall().returns(returnResponseOnTick(900, 1)); - fetchStub.onSecondCall().returns(returnResponseOnTick(1900, 2)); - - addCacheInterceptors(ajax, { - useCache: true, - maxAge: 750, - }); - - const firstRequest = ajax.fetch('/test').then(r => r.text()); - const concurrentFirstRequest1 = ajax.fetch('/test').then(r => r.text()); - const concurrentFirstRequest2 = ajax.fetch('/test').then(r => r.text()); - - clock.tick(1000); - - // firstRequest is cached at tick 1000 in the next line! - const firstResponses = await Promise.all([ - firstRequest, - concurrentFirstRequest1, - concurrentFirstRequest2, - ]); - - expect(fetchStub.callCount).to.equal(1); - - const cachedFirstRequest = ajax.fetch('/test').then(r => r.text()); - - clock.tick(500); - - const cachedFirstResponse = await cachedFirstRequest; - - expect(fetchStub.callCount).to.equal(1); - - const secondRequest = ajax.fetch('/test').then(r => r.text()); - const secondConcurrentRequest = ajax.fetch('/test').then(r => r.text()); - - clock.tick(1000); - - const secondResponses = await Promise.all([secondRequest, secondConcurrentRequest]); - - expect(fetchStub.callCount).to.equal(2); - - expect(firstResponses).to.eql(['mock response 1', 'mock response 1', 'mock response 1']); - - expect(cachedFirstResponse).to.equal('mock response 1'); - - expect(secondResponses).to.eql(['mock response 2', 'mock response 2']); - }); - it('discards responses that are requested in a different cache session', async () => { newCacheId(); @@ -563,27 +650,5 @@ describe('cache interceptors', () => { expect(ajaxCache._cachedRequests).to.deep.equal({}); expect(fetchStub.callCount).to.equal(1); }); - - it('preserves status and headers when returning cached response', async () => { - newCacheId(); - fetchStub.returns( - Promise.resolve( - new Response('mock response', { status: 206, headers: { 'x-foo': 'x-bar' } }), - ), - ); - - addCacheInterceptors(ajax, { - useCache: true, - maxAge: 100, - }); - - const response1 = await ajax.fetch('/test'); - const response2 = await ajax.fetch('/test'); - expect(fetchStub.callCount).to.equal(1); - expect(response1.status).to.equal(206); - expect(response1.headers.get('x-foo')).to.equal('x-bar'); - expect(response2.status).to.equal(206); - expect(response2.headers.get('x-foo')).to.equal('x-bar'); - }); }); }); diff --git a/packages/ajax/types/types.d.ts b/packages/ajax/types/types.d.ts index fa63fea9f..3dcac0b6c 100644 --- a/packages/ajax/types/types.d.ts +++ b/packages/ajax/types/types.d.ts @@ -40,6 +40,7 @@ export interface CacheOptions { invalidateUrls?: string[]; invalidateUrlsRegex?: RegExp; requestIdFunction?: RequestIdFunction; + contentTypes?: string[]; } export interface CacheOptionsWithIdentifier extends CacheOptions {