Track pending requests in AjaxCache

2 files changed, 89 insertions, 38 deletions

diff --git a/app/assets/javascripts/lib/utils/ajax_cache.js b/app/assets/javascripts/lib/utils/ajax_cache.js
index d99eefb5089..cf030d613df 100644
--- a/app/assets/javascripts/lib/utils/ajax_cache.js
+++ b/app/assets/javascripts/lib/utils/ajax_cache.js
@@ -1,32 +1,54 @@
-const AjaxCache = {
-  internalStorage: { },
+class AjaxCache {
+  constructor() {
+    this.internalStorage = { };
+    this.pendingRequests = { };
+  }
+
   get(endpoint) {
     return this.internalStorage[endpoint];
-  },
+  }
+
   hasData(endpoint) {
     return Object.prototype.hasOwnProperty.call(this.internalStorage, endpoint);
-  },
-  purge(endpoint) {
+  }
+
+  remove(endpoint) {
     delete this.internalStorage[endpoint];
-  },
+  }
+
   retrieve(endpoint) {
-    if (AjaxCache.hasData(endpoint)) {
-      return Promise.resolve(AjaxCache.get(endpoint));
+    if (this.hasData(endpoint)) {
+      return Promise.resolve(this.get(endpoint));
     }
 
-    return new Promise((resolve, reject) => {
-      $.ajax(endpoint) // eslint-disable-line promise/catch-or-return
-      .then(data => resolve(data),
-        (jqXHR, textStatus, errorThrown) => {
-          const error = new Error(`${endpoint}: ${errorThrown}`);
-          error.textStatus = textStatus;
-          reject(error);
-        },
-      );
-    })
-    .then((data) => { this.internalStorage[endpoint] = data; })
-    .then(() => AjaxCache.get(endpoint));
-  },
-};
-
-export default AjaxCache;
+    let pendingRequest = this.pendingRequests[endpoint];
+
+    if (!pendingRequest) {
+      pendingRequest = new Promise((resolve, reject) => {
+        // jQuery 2 is not Promises/A+ compatible (missing catch)
+        $.ajax(endpoint) // eslint-disable-line promise/catch-or-return
+        .then(data => resolve(data),
+          (jqXHR, textStatus, errorThrown) => {
+            const error = new Error(`${endpoint}: ${errorThrown}`);
+            error.textStatus = textStatus;
+            reject(error);
+          },
+        );
+      })
+      .then((data) => {
+        this.internalStorage[endpoint] = data;
+        delete this.pendingRequests[endpoint];
+      })
+      .catch((error) => {
+        delete this.pendingRequests[endpoint];
+        throw error;
+      });
+
+      this.pendingRequests[endpoint] = pendingRequest;
+    }
+
+    return pendingRequest.then(() => this.get(endpoint));
+  }
+}
+
+export default new AjaxCache();
diff --git a/spec/javascripts/lib/utils/ajax_cache_spec.js b/spec/javascripts/lib/utils/ajax_cache_spec.js
index 7b466a11b92..e1747a82329 100644
--- a/spec/javascripts/lib/utils/ajax_cache_spec.js
+++ b/spec/javascripts/lib/utils/ajax_cache_spec.js
@@ -5,19 +5,13 @@ describe('AjaxCache', () => {
   const dummyResponse = {
     important: 'dummy data',
   };
-  let ajaxSpy = (url) => {
-    expect(url).toBe(dummyEndpoint);
-    const deferred = $.Deferred();
-    deferred.resolve(dummyResponse);
-    return deferred.promise();
-  };
 
   beforeEach(() => {
     AjaxCache.internalStorage = { };
-    spyOn(jQuery, 'ajax').and.callFake(url => ajaxSpy(url));
+    AjaxCache.pendingRequests = { };
   });
 
-  describe('#get', () => {
+  describe('get', () => {
     it('returns undefined if cache is empty', () => {
       const data = AjaxCache.get(dummyEndpoint);
 
@@ -41,7 +35,7 @@ describe('AjaxCache', () => {
     });
   });
 
-  describe('#hasData', () => {
+  describe('hasData', () => {
     it('returns false if cache is empty', () => {
       expect(AjaxCache.hasData(dummyEndpoint)).toBe(false);
     });
@@ -59,9 +53,9 @@ describe('AjaxCache', () => {
     });
   });
 
-  describe('#purge', () => {
+  describe('remove', () => {
     it('does nothing if cache is empty', () => {
-      AjaxCache.purge(dummyEndpoint);
+      AjaxCache.remove(dummyEndpoint);
 
       expect(AjaxCache.internalStorage).toEqual({ });
     });
@@ -69,7 +63,7 @@ describe('AjaxCache', () => {
     it('does nothing if cache contains no matching data', () => {
       AjaxCache.internalStorage['not matching'] = dummyResponse;
 
-      AjaxCache.purge(dummyEndpoint);
+      AjaxCache.remove(dummyEndpoint);
 
       expect(AjaxCache.internalStorage['not matching']).toBe(dummyResponse);
     });
@@ -77,14 +71,27 @@ describe('AjaxCache', () => {
     it('removes matching data', () => {
       AjaxCache.internalStorage[dummyEndpoint] = dummyResponse;
 
-      AjaxCache.purge(dummyEndpoint);
+      AjaxCache.remove(dummyEndpoint);
 
       expect(AjaxCache.internalStorage).toEqual({ });
     });
   });
 
-  describe('#retrieve', () => {
+  describe('retrieve', () => {
+    let ajaxSpy;
+
+    beforeEach(() => {
+      spyOn(jQuery, 'ajax').and.callFake(url => ajaxSpy(url));
+    });
+
     it('stores and returns data from Ajax call if cache is empty', (done) => {
+      ajaxSpy = (url) => {
+        expect(url).toBe(dummyEndpoint);
+        const deferred = $.Deferred();
+        deferred.resolve(dummyResponse);
+        return deferred.promise();
+      };
+
       AjaxCache.retrieve(dummyEndpoint)
       .then((data) => {
         expect(data).toBe(dummyResponse);
@@ -94,6 +101,28 @@ describe('AjaxCache', () => {
       .catch(fail);
     });
 
+    it('makes no Ajax call if request is pending', () => {
+      const responseDeferred = $.Deferred();
+
+      ajaxSpy = (url) => {
+        expect(url).toBe(dummyEndpoint);
+        // neither reject nor resolve to keep request pending
+        return responseDeferred.promise();
+      };
+
+      const unexpectedResponse = data => fail(`Did not expect response: ${data}`);
+
+      AjaxCache.retrieve(dummyEndpoint)
+      .then(unexpectedResponse)
+      .catch(fail);
+
+      AjaxCache.retrieve(dummyEndpoint)
+      .then(unexpectedResponse)
+      .catch(fail);
+
+      expect($.ajax.calls.count()).toBe(1);
+    });
+
     it('returns undefined if Ajax call fails and cache is empty', (done) => {
       const dummyStatusText = 'exploded';
       const dummyErrorMessage = 'server exploded';