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/packages/ajax/src/AjaxClient.js b/packages/ajax/src/AjaxClient.js index f932ee50c..996f02107 100644 --- a/packages/ajax/src/AjaxClient.js +++ b/packages/ajax/src/AjaxClient.js @@ -118,27 +118,15 @@ export class AjaxClient { // 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 interceptedRequestOrResponse; - } + const interceptedRequestOrResponse = await this.__interceptRequest(request); + if (interceptedRequestOrResponse instanceof Response) { + return interceptedRequestOrResponse; } - const response = /** @type {CacheResponse} */ (await fetch(interceptedRequest)); - response.request = interceptedRequest; + const response = /** @type {CacheResponse} */ (await fetch(interceptedRequestOrResponse)); + response.request = interceptedRequestOrResponse; - 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); - } + const interceptedResponse = await this.__interceptResponse(response); if (interceptedResponse.status >= 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..6c5f569ef 100644 --- a/packages/ajax/src/interceptors-cache.js +++ b/packages/ajax/src/interceptors-cache.js @@ -20,6 +20,36 @@ class Cache { * @protected */ this._cacheObject = {}; + /** @type {{ [url: string]: { promise: Promise, resolve: (v?: any) => void } }} */ + 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]; + } } /** @@ -65,6 +95,7 @@ class Cache { Object.keys(this._cacheObject).forEach(key => { if (key.indexOf(url) > -1) { delete this._cacheObject[key]; + this.resolvePendingRequest(key); } }); } @@ -82,6 +113,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}'`); @@ -245,7 +277,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,6 +298,13 @@ 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) { @@ -285,6 +323,10 @@ export const cacheRequestInterceptorFactory = (getCacheIdentifier, globalCacheOp 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 +348,36 @@ 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, - ); - + // store the response data in the cache and mark request as resolved 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; + currentCache.set(cacheId, responseBody); } + 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..469df3df9 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,44 @@ 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); + }); }); });