Merge branch '38394-smarter-interval' into 'master'

don't re-run smart interval callback if there is already one in progress

Closes #38394

See merge request gitlab-org/gitlab-ce!15032

app/assets/javascripts/smart_interval.js

@@ -3,9 +3,10 @@
  * and controllable by a public API.
  */
 
-class SmartInterval {
+export default class SmartInterval {
   /**
-   * @param { function } opts.callback Function to be called on each iteration (required)
+   * @param { function } opts.callback Function that returns a promise, called on each iteration
+   *                     unless still in progress (required)
    * @param { milliseconds } opts.startingInterval `currentInterval` is set to this initially
    * @param { milliseconds } opts.maxInterval `currentInterval` will be incremented to this
    * @param { milliseconds } opts.hiddenInterval `currentInterval` is set to this
@@ -42,13 +43,16 @@ class SmartInterval {
     const cfg = this.cfg;
     const state = this.state;
 
-    if (cfg.immediateExecution) {
+    if (cfg.immediateExecution && !this.isLoading) {
       cfg.immediateExecution = false;
-      cfg.callback();
+      this.triggerCallback();
     }
 
     state.intervalId = window.setInterval(() => {
-      cfg.callback();
+      if (this.isLoading) {
+        return;
+      }
+      this.triggerCallback();
 
       if (this.getCurrentInterval() === cfg.maxInterval) {
         return;
@@ -76,7 +80,7 @@ class SmartInterval {
 
   // start a timer, using the existing interval
   resume() {
-    this.stopTimer(); // stop exsiting timer, in case timer was not previously stopped
+    this.stopTimer(); // stop existing timer, in case timer was not previously stopped
     this.start();
   }
 
@@ -104,6 +108,18 @@ class SmartInterval {
     this.initPageUnloadHandling();
   }
 
+  triggerCallback() {
+    this.isLoading = true;
+    this.cfg.callback()
+      .then(() => {
+        this.isLoading = false;
+      })
+      .catch((err) => {
+        this.isLoading = false;
+        throw err;
+      });
+  }
+
   initVisibilityChangeHandling() {
     // cancel interval when tab no longer shown (prevents cached pages from polling)
     document.addEventListener('visibilitychange', this.handleVisibilityChange.bind(this));
@@ -154,4 +170,3 @@ class SmartInterval {
   }
 }
 
-window.gl.SmartInterval = SmartInterval;

app/assets/javascripts/vue_merge_request_widget/mr_widget_options.js

+import SmartInterval from '~/smart_interval';
 import Flash from '../flash';
 import {
   WidgetHeader,
@@ -81,7 +82,7 @@ export default {
       return new MRWidgetService(endpoints);
     },
     checkStatus(cb) {
-      this.service.checkStatus()
+      return this.service.checkStatus()
         .then(res => res.json())
         .then((res) => {
           this.handleNotification(res);
@@ -97,7 +98,7 @@ export default {
         });
     },
     initPolling() {
-      this.pollingInterval = new gl.SmartInterval({
+      this.pollingInterval = new SmartInterval({
         callback: this.checkStatus,
         startingInterval: 10000,
         maxInterval: 30000,
@@ -106,7 +107,7 @@ export default {
       });
     },
     initDeploymentsPolling() {
-      this.deploymentsInterval = new gl.SmartInterval({
+      this.deploymentsInterval = new SmartInterval({
         callback: this.fetchDeployments,
         startingInterval: 30000,
         maxInterval: 120000,
@@ -121,7 +122,7 @@ export default {
       }
     },
     fetchDeployments() {
-      this.service.fetchDeployments()
+      return this.service.fetchDeployments()
         .then(res => res.json())
         .then((res) => {
           if (res.length) {

changelogs/unreleased/38394-smarter-interval.yml

+---
+title: Update Merge Request polling so there is only one request at a time
+merge_request: 15032
+author:
+type: fixed

spec/javascripts/smart_interval_spec.js

-import '~/smart_interval';
+import SmartInterval from '~/smart_interval';
 
-(() => {
+describe('SmartInterval', function () {
   const DEFAULT_MAX_INTERVAL = 100;
   const DEFAULT_STARTING_INTERVAL = 5;
   const DEFAULT_SHORT_TIMEOUT = 75;
@@ -9,7 +9,7 @@ import '~/smart_interval';
 
   function createDefaultSmartInterval(config) {
     const defaultParams = {
-      callback: () => {},
+      callback: () => Promise.resolve(),
       startingInterval: DEFAULT_STARTING_INTERVAL,
       maxInterval: DEFAULT_MAX_INTERVAL,
       incrementByFactorOf: DEFAULT_INCREMENT_FACTOR,
@@ -22,10 +22,9 @@ import '~/smart_interval';
       _.extend(defaultParams, config);
     }
 
-    return new gl.SmartInterval(defaultParams);
+    return new SmartInterval(defaultParams);
   }
 
-  describe('SmartInterval', function () {
   describe('Increment Interval', function () {
     beforeEach(function () {
       this.smartInterval = createDefaultSmartInterval();
@@ -58,6 +57,21 @@ import '~/smart_interval';
         done();
       }, DEFAULT_LONG_TIMEOUT);
     });
+
+    it('does not increment while waiting for callback', function () {
+      jasmine.clock().install();
+
+      const smartInterval = createDefaultSmartInterval({
+        callback: () => new Promise($.noop),
+      });
+
+      jasmine.clock().tick(DEFAULT_SHORT_TIMEOUT);
+
+      const oneInterval = smartInterval.cfg.startingInterval * DEFAULT_INCREMENT_FACTOR;
+      expect(smartInterval.getCurrentInterval()).toEqual(oneInterval);
+
+      jasmine.clock().uninstall();
+    });
   });
 
   describe('Public methods', function () {
@@ -175,5 +189,4 @@ import '~/smart_interval';
       expect(interval.cfg.immediateExecution).toBeFalsy();
     });
   });
-  });
-})(window.gl || (window.gl = {}));
+});

spec/javascripts/vue_mr_widget/mr_widget_options_spec.js

@@ -121,24 +121,28 @@ describe('mrWidgetOptions', () => {
 
     describe('initPolling', () => {
       it('should call SmartInterval', () => {
-        spyOn(gl, 'SmartInterval').and.returnValue({
-          resume() {},
-          stopTimer() {},
-        });
+        spyOn(vm, 'checkStatus').and.returnValue(Promise.resolve());
+        jasmine.clock().install();
         vm.initPolling();
 
+        expect(vm.checkStatus).not.toHaveBeenCalled();
+
+        jasmine.clock().tick(10000);
+
         expect(vm.pollingInterval).toBeDefined();
-        expect(gl.SmartInterval).toHaveBeenCalled();
+        expect(vm.checkStatus).toHaveBeenCalled();
+
+        jasmine.clock().uninstall();
       });
     });
 
     describe('initDeploymentsPolling', () => {
       it('should call SmartInterval', () => {
-        spyOn(gl, 'SmartInterval');
+        spyOn(vm, 'fetchDeployments').and.returnValue(Promise.resolve());
         vm.initDeploymentsPolling();
 
         expect(vm.deploymentsInterval).toBeDefined();
-        expect(gl.SmartInterval).toHaveBeenCalled();
+        expect(vm.fetchDeployments).toHaveBeenCalled();
       });
     });