diff --git a/.changeset/brave-tools-tie.md b/.changeset/brave-tools-tie.md new file mode 100644 index 000000000..4c0008163 --- /dev/null +++ b/.changeset/brave-tools-tie.md @@ -0,0 +1,5 @@ +--- +'@lion/ajax': patch +--- + +allow caching concurrent requests diff --git a/.changeset/lucky-games-poke.md b/.changeset/lucky-games-poke.md new file mode 100644 index 000000000..0a3e3cbf7 --- /dev/null +++ b/.changeset/lucky-games-poke.md @@ -0,0 +1,5 @@ +--- +'@lion/ajax': patch +--- + +return cached status and headers diff --git a/docs/docs/systems/form/formatting-and-parsing.md b/docs/docs/systems/form/formatting-and-parsing.md index 99aab66fb..d5d1520b6 100644 --- a/docs/docs/systems/form/formatting-and-parsing.md +++ b/docs/docs/systems/form/formatting-and-parsing.md @@ -157,9 +157,9 @@ In the example below, we do not allow you to write digits. ```js preview-story export const preprocessors = () => { - const preprocess = (value) => { + const preprocess = value => { return value.replace(/[0-9]/g, ''); - } + }; return html` = 400 && interceptedResponse.status < 600) { throw new AjaxClientFetchError(request, interceptedResponse); @@ -190,4 +178,39 @@ export class AjaxClient { throw new Error(`Failed to parse response from ${response.url} as JSON.`); } } + + /** + * @param {Request} request + * @returns {Promise} + */ + async __interceptRequest(request) { + // run request interceptors, returning directly and skipping the network + // if a interceptor returns a Response + let interceptedRequest = request; + for (const intercept of this._requestInterceptors) { + // In this instance we actually do want to await for each sequence + // eslint-disable-next-line no-await-in-loop + const interceptedRequestOrResponse = await intercept(interceptedRequest); + if (interceptedRequestOrResponse instanceof Request) { + interceptedRequest = interceptedRequestOrResponse; + } else { + return this.__interceptResponse(interceptedRequestOrResponse); + } + } + return interceptedRequest; + } + + /** + * @param {Response} response + * @returns {Promise} + */ + async __interceptResponse(response) { + let interceptedResponse = response; + for (const intercept of this._responseInterceptors) { + // In this instance we actually do want to await for each sequence + // eslint-disable-next-line no-await-in-loop + interceptedResponse = await intercept(interceptedResponse); + } + return interceptedResponse; + } } diff --git a/packages/ajax/src/interceptors-cache.js b/packages/ajax/src/interceptors-cache.js index 89fbad1ff..5ad542a9a 100644 --- a/packages/ajax/src/interceptors-cache.js +++ b/packages/ajax/src/interceptors-cache.js @@ -16,22 +16,55 @@ class Cache { this.cacheConfig = {}; /** - * @type {{[url: string]: {expires: number, data: object} }} - * @protected + * @type {{[url: string]: {expires: number, response: CacheResponse} }} + * @private */ this._cacheObject = {}; + /** + * @type {{ [url: string]: { promise: Promise, resolve: (v?: any) => void } }} + * @private + */ + this._pendingRequests = {}; + } + + /** @param {string} url */ + setPendingRequest(url) { + /** @type {(v: any) => void} */ + let resolve = () => {}; + const promise = new Promise(_resolve => { + resolve = _resolve; + }); + this._pendingRequests[url] = { promise, resolve }; + } + + /** + * @param {string} url + * @returns {Promise | undefined} + */ + getPendingRequest(url) { + if (this._pendingRequests[url]) { + return this._pendingRequests[url].promise; + } + } + + /** @param {string} url */ + resolvePendingRequest(url) { + if (this._pendingRequests[url]) { + this._pendingRequests[url].resolve(); + delete this._pendingRequests[url]; + } } /** * Store an item in the cache * @param {string} url key by which the cache is stored - * @param {object} data the cached object + * @param {Response} response the cached response */ - set(url, data) { + set(url, response) { this._validateCache(); this._cacheObject[url] = { expires: new Date().getTime(), - data, + response, }; } @@ -39,6 +72,7 @@ class Cache { * Retrieve an item from the cache * @param {string} url key by which the cache is stored * @param {number} timeToLive maximum time to allow cache to live + * @returns {CacheResponse | false} */ get(url, timeToLive) { this._validateCache(); @@ -52,7 +86,7 @@ class Cache { if (timeToLive !== null && cacheAge > timeToLive) { return false; } - return cacheResult.data; + return cacheResult.response; } /** @@ -65,6 +99,7 @@ class Cache { Object.keys(this._cacheObject).forEach(key => { if (key.indexOf(url) > -1) { delete this._cacheObject[key]; + this.resolvePendingRequest(key); } }); } @@ -82,6 +117,7 @@ class Cache { if (notMatch) return; const isDataDeleted = delete this._cacheObject[key]; + this.resolvePendingRequest(key); if (!isDataDeleted) { throw new Error(`Failed to delete cache for a request '${key}'`); @@ -228,7 +264,7 @@ export const cacheRequestInterceptorFactory = (getCacheIdentifier, globalCacheOp const validatedInitialCacheOptions = validateOptions(globalCacheOptions); return /** @param {CacheRequest} cacheRequest */ async cacheRequest => { - const { method, status, statusText, headers } = cacheRequest; + const { method } = cacheRequest; const cacheOptions = composeCacheOptions( validatedInitialCacheOptions, @@ -245,7 +281,6 @@ export const cacheRequestInterceptorFactory = (getCacheIdentifier, globalCacheOp // cacheIdentifier is used to bind the cache to the current session const currentCache = getCache(getCacheIdentifier()); - const cacheResponse = currentCache.get(cacheId, cacheOptions.timeToLive); // don't use cache if the request method is not part of the configs methods if (cacheOptions.methods.indexOf(method.toLowerCase()) === -1) { @@ -267,24 +302,29 @@ export const cacheRequestInterceptorFactory = (getCacheIdentifier, globalCacheOp return cacheRequest; } + const pendingRequest = currentCache.getPendingRequest(cacheId); + if (pendingRequest) { + // there is another concurrent request, wait for it to finish + await pendingRequest; + } + + const cacheResponse = currentCache.get(cacheId, cacheOptions.timeToLive); if (cacheResponse) { // eslint-disable-next-line no-param-reassign if (!cacheRequest.cacheOptions) { cacheRequest.cacheOptions = { useCache: false }; } - const init = /** @type {LionRequestInit} */ ({ - status, - statusText, - headers, - }); - - const response = /** @type {CacheResponse} */ (new Response(cacheResponse, init)); + const response = /** @type {CacheResponse} */ cacheResponse.clone(); response.request = cacheRequest; response.fromCache = true; return response; } + // we do want to use caching for this requesting, but it's not already cached + // mark this as a pending request, so that concurrent requests can reuse it from the cache + currentCache.setPendingRequest(cacheId); + return cacheRequest; }; }; @@ -306,38 +346,35 @@ export const cacheResponseInterceptorFactory = (getCacheIdentifier, globalCacheO throw new Error(`getCacheIdentifier returns falsy`); } + if (!cacheResponse.request) { + throw new Error('Missing request in response.'); + } + const cacheOptions = composeCacheOptions( validatedInitialCacheOptions, cacheResponse.request?.cacheOptions, ); - + // string that identifies cache entry + const cacheId = cacheOptions.requestIdentificationFn( + cacheResponse.request, + searchParamSerializer, + ); + const currentCache = getCache(getCacheIdentifier()); const isAlreadyFromCache = !!cacheResponse.fromCache; // caching all responses with not default `timeToLive` const isCacheActive = cacheOptions.timeToLive > 0; - if (isAlreadyFromCache || !isCacheActive) { - return cacheResponse; - } - // if the request is one of the options.methods; store response in cache if ( - cacheResponse.request && + !isAlreadyFromCache && + isCacheActive && cacheOptions.methods.indexOf(cacheResponse.request.method.toLowerCase()) > -1 ) { - // string that identifies cache entry - const cacheId = cacheOptions.requestIdentificationFn( - cacheResponse.request, - searchParamSerializer, - ); - - const responseBody = await cacheResponse.clone().text(); - // store the response data in the cache - getCache(getCacheIdentifier()).set(cacheId, responseBody); - } else { - // don't store in cache if the request method is not part of the configs methods - return cacheResponse; + // store the response data in the cache and mark request as resolved + currentCache.set(cacheId, cacheResponse.clone()); } + currentCache.resolvePendingRequest(cacheId); return cacheResponse; }; }; diff --git a/packages/ajax/test/interceptors-cache.test.js b/packages/ajax/test/interceptors-cache.test.js index a62a1fdfe..b0bf7866e 100644 --- a/packages/ajax/test/interceptors-cache.test.js +++ b/packages/ajax/test/interceptors-cache.test.js @@ -1,4 +1,4 @@ -import { expect } from '@open-wc/testing'; +import { aTimeout, expect } from '@open-wc/testing'; import { spy, stub, useFakeTimers } from 'sinon'; import '../src/typedef.js'; @@ -452,5 +452,70 @@ describe('ajax cache', () => { ajaxAlwaysRequestSpy.restore(); removeCacheInterceptors(ajax, indexes); }); + + it('caches concurrent requests', async () => { + newCacheId(); + + let i = 0; + fetchStub.returns( + new Promise(resolve => { + i += 1; + setTimeout(() => { + resolve(new Response(`mock response ${i}`)); + }, 5); + }), + ); + + const indexes = addCacheInterceptors(ajax, { + useCache: true, + timeToLive: 100, + }); + const ajaxRequestSpy = spy(ajax, 'request'); + + const request1 = ajax.request('/test'); + const request2 = ajax.request('/test'); + await aTimeout(1); + const request3 = ajax.request('/test'); + await aTimeout(3); + const request4 = ajax.request('/test'); + const responses = await Promise.all([request1, request2, request3, request4]); + expect(fetchStub.callCount).to.equal(1); + const responseTexts = await Promise.all(responses.map(r => r.text())); + expect(responseTexts).to.eql([ + 'mock response 1', + 'mock response 1', + 'mock response 1', + 'mock response 1', + ]); + + ajaxRequestSpy.restore(); + removeCacheInterceptors(ajax, indexes); + }); + + 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' } }), + ), + ); + + const indexes = addCacheInterceptors(ajax, { + useCache: true, + timeToLive: 100, + }); + const ajaxRequestSpy = spy(ajax, 'request'); + + const response1 = await ajax.request('/test'); + const response2 = await ajax.request('/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'); + + ajaxRequestSpy.restore(); + removeCacheInterceptors(ajax, indexes); + }); }); });