feat(ajax): allow caching concurrent requests

.changeset/brave-tools-tie.md

@@ -0,0 +1,5 @@
+---
+'@lion/ajax': patch
+---
+
+allow caching concurrent requests

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;
+    const interceptedRequestOrResponse = await this.__interceptRequest(request);
-    for (const intercept of this._requestInterceptors) {
+    if (interceptedRequestOrResponse instanceof Response) {
-      // In this instance we actually do want to await for each sequence
+      return interceptedRequestOrResponse;
-      // eslint-disable-next-line no-await-in-loop
-      const interceptedRequestOrResponse = await intercept(interceptedRequest);
-      if (interceptedRequestOrResponse instanceof Request) {
-        interceptedRequest = interceptedRequestOrResponse;
-      } else {
-        return interceptedRequestOrResponse;
-      }
     }
 
-    const response = /** @type {CacheResponse} */ (await fetch(interceptedRequest));
+    const response = /** @type {CacheResponse} */ (await fetch(interceptedRequestOrResponse));
-    response.request = interceptedRequest;
+    response.request = interceptedRequestOrResponse;
 
-    let interceptedResponse = response;
+    const interceptedResponse = await this.__interceptResponse(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);
-    }
 
     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<Request | Response>}
+   */
+  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<Response>}
+   */
+  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;
+  }
 }

packages/ajax/src/interceptors-cache.js

@@ -20,6 +20,36 @@ class Cache {
      * @protected
      */
     this._cacheObject = {};
+    /** @type {{ [url: string]: { promise: Promise<void>, 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<void> | 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
+      // store the response data in the cache and mark request as resolved
-      const cacheId = cacheOptions.requestIdentificationFn(
-        cacheResponse.request,
-        searchParamSerializer,
-      );
-
       const responseBody = await cacheResponse.clone().text();
-      // store the response data in the cache
+      currentCache.set(cacheId, responseBody);
-      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.resolvePendingRequest(cacheId);
     return cacheResponse;
   };
 };

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);
+    });
   });
 });