Skip to content

G
gitlab-ce
Commit a3e1cb81 authored by Filipa Lacerda's avatar Filipa Lacerda
Browse files
Options

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
parents c8eb789b 862d781d
Hide whitespace changes
Inline Side-by-side
Showing with 171 additions and 133 deletions
app/assets/javascripts/smart_interval.js
View file @ a3e1cb81
......@@ -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
View file @ a3e1cb81
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 0 → 100644
View file @ a3e1cb81
---
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
View file @ a3e1cb81
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,158 +22,171 @@ 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();
});
describe('Increment Interval', function () {
beforeEach(function () {
this.smartInterval = createDefaultSmartInterval();
});
it('should increment the interval delay', function (done) {
const interval = this.smartInterval;
setTimeout(() => {
const intervalConfig = this.smartInterval.cfg;
const iterationCount = 4;
const maxIntervalAfterIterations = intervalConfig.startingInterval *
(intervalConfig.incrementByFactorOf ** (iterationCount - 1)); // 40
const currentInterval = interval.getCurrentInterval();
// Provide some flexibility for performance of testing environment
expect(currentInterval).toBeGreaterThan(intervalConfig.startingInterval);
expect(currentInterval <= maxIntervalAfterIterations).toBeTruthy();
done();
}, DEFAULT_SHORT_TIMEOUT); // 4 iterations, increment by 2x = (5 + 10 + 20 + 40)
});
it('should increment the interval delay', function (done) {
const interval = this.smartInterval;
setTimeout(() => {
const intervalConfig = this.smartInterval.cfg;
const iterationCount = 4;
const maxIntervalAfterIterations = intervalConfig.startingInterval *
(intervalConfig.incrementByFactorOf ** (iterationCount - 1)); // 40
const currentInterval = interval.getCurrentInterval();
// Provide some flexibility for performance of testing environment
expect(currentInterval).toBeGreaterThan(intervalConfig.startingInterval);
expect(currentInterval <= maxIntervalAfterIterations).toBeTruthy();
done();
}, DEFAULT_SHORT_TIMEOUT); // 4 iterations, increment by 2x = (5 + 10 + 20 + 40)
});
it('should not increment past maxInterval', function (done) {
const interval = this.smartInterval;
it('should not increment past maxInterval', function (done) {
const interval = this.smartInterval;
setTimeout(() => {
const currentInterval = interval.getCurrentInterval();
expect(currentInterval).toBe(interval.cfg.maxInterval);
setTimeout(() => {
const currentInterval = interval.getCurrentInterval();
expect(currentInterval).toBe(interval.cfg.maxInterval);
done();
}, DEFAULT_LONG_TIMEOUT);
});
done();
}, DEFAULT_LONG_TIMEOUT);
});
describe('Public methods', function () {
beforeEach(function () {
this.smartInterval = createDefaultSmartInterval();
it('does not increment while waiting for callback', function () {
jasmine.clock().install();
const smartInterval = createDefaultSmartInterval({
callback: () => new Promise($.noop),
});
it('should cancel an interval', function (done) {
const interval = this.smartInterval;
jasmine.clock().tick(DEFAULT_SHORT_TIMEOUT);
const oneInterval = smartInterval.cfg.startingInterval * DEFAULT_INCREMENT_FACTOR;
expect(smartInterval.getCurrentInterval()).toEqual(oneInterval);
setTimeout(() => {
interval.cancel();
jasmine.clock().uninstall();
});
});
const intervalId = interval.state.intervalId;
const currentInterval = interval.getCurrentInterval();
const intervalLowerLimit = interval.cfg.startingInterval;
describe('Public methods', function () {
beforeEach(function () {
this.smartInterval = createDefaultSmartInterval();
});
expect(intervalId).toBeUndefined();
expect(currentInterval).toBe(intervalLowerLimit);
it('should cancel an interval', function (done) {
const interval = this.smartInterval;
done();
}, DEFAULT_SHORT_TIMEOUT);
});
setTimeout(() => {
interval.cancel();
it('should resume an interval', function (done) {
const interval = this.smartInterval;
const intervalId = interval.state.intervalId;
const currentInterval = interval.getCurrentInterval();
const intervalLowerLimit = interval.cfg.startingInterval;
setTimeout(() => {
interval.cancel();
expect(intervalId).toBeUndefined();
expect(currentInterval).toBe(intervalLowerLimit);
interval.resume();
done();
}, DEFAULT_SHORT_TIMEOUT);
});
const intervalId = interval.state.intervalId;
it('should resume an interval', function (done) {
const interval = this.smartInterval;
expect(intervalId).toBeTruthy();
setTimeout(() => {
interval.cancel();
done();
}, DEFAULT_SHORT_TIMEOUT);
});
interval.resume();
const intervalId = interval.state.intervalId;
expect(intervalId).toBeTruthy();
done();
}, DEFAULT_SHORT_TIMEOUT);
});
});
describe('DOM Events', function () {
beforeEach(function () {
// This ensures DOM and DOM events are initialized for these specs.
setFixtures('<div></div>');
describe('DOM Events', function () {
beforeEach(function () {
// This ensures DOM and DOM events are initialized for these specs.
setFixtures('<div></div>');
this.smartInterval = createDefaultSmartInterval();
});
this.smartInterval = createDefaultSmartInterval();
});
it('should pause when page is not visible', function (done) {
const interval = this.smartInterval;
it('should pause when page is not visible', function (done) {
const interval = this.smartInterval;
setTimeout(() => {
expect(interval.state.intervalId).toBeTruthy();
setTimeout(() => {
expect(interval.state.intervalId).toBeTruthy();
// simulates triggering of visibilitychange event
interval.handleVisibilityChange({ target: { visibilityState: 'hidden' } });
// simulates triggering of visibilitychange event
interval.handleVisibilityChange({ target: { visibilityState: 'hidden' } });
expect(interval.state.intervalId).toBeUndefined();
done();
}, DEFAULT_SHORT_TIMEOUT);
});
expect(interval.state.intervalId).toBeUndefined();
done();
}, DEFAULT_SHORT_TIMEOUT);
});
it('should change to the hidden interval when page is not visible', function (done) {
const HIDDEN_INTERVAL = 1500;
const interval = createDefaultSmartInterval({ hiddenInterval: HIDDEN_INTERVAL });
it('should change to the hidden interval when page is not visible', function (done) {
const HIDDEN_INTERVAL = 1500;
const interval = createDefaultSmartInterval({ hiddenInterval: HIDDEN_INTERVAL });
setTimeout(() => {
expect(interval.state.intervalId).toBeTruthy();
expect(interval.getCurrentInterval() >= DEFAULT_STARTING_INTERVAL &&
interval.getCurrentInterval() <= DEFAULT_MAX_INTERVAL).toBeTruthy();
setTimeout(() => {
expect(interval.state.intervalId).toBeTruthy();
expect(interval.getCurrentInterval() >= DEFAULT_STARTING_INTERVAL &&
interval.getCurrentInterval() <= DEFAULT_MAX_INTERVAL).toBeTruthy();
// simulates triggering of visibilitychange event
interval.handleVisibilityChange({ target: { visibilityState: 'hidden' } });
// simulates triggering of visibilitychange event
interval.handleVisibilityChange({ target: { visibilityState: 'hidden' } });
expect(interval.state.intervalId).toBeTruthy();
expect(interval.getCurrentInterval()).toBe(HIDDEN_INTERVAL);
done();
}, DEFAULT_SHORT_TIMEOUT);
});
expect(interval.state.intervalId).toBeTruthy();
expect(interval.getCurrentInterval()).toBe(HIDDEN_INTERVAL);
done();
}, DEFAULT_SHORT_TIMEOUT);
});
it('should resume when page is becomes visible at the previous interval', function (done) {
const interval = this.smartInterval;
it('should resume when page is becomes visible at the previous interval', function (done) {
const interval = this.smartInterval;
setTimeout(() => {
expect(interval.state.intervalId).toBeTruthy();
setTimeout(() => {
expect(interval.state.intervalId).toBeTruthy();
// simulates triggering of visibilitychange event
interval.handleVisibilityChange({ target: { visibilityState: 'hidden' } });
// simulates triggering of visibilitychange event
interval.handleVisibilityChange({ target: { visibilityState: 'hidden' } });
expect(interval.state.intervalId).toBeUndefined();
expect(interval.state.intervalId).toBeUndefined();
// simulates triggering of visibilitychange event
interval.handleVisibilityChange({ target: { visibilityState: 'visible' } });
// simulates triggering of visibilitychange event
interval.handleVisibilityChange({ target: { visibilityState: 'visible' } });
expect(interval.state.intervalId).toBeTruthy();
expect(interval.state.intervalId).toBeTruthy();
done();
}, DEFAULT_SHORT_TIMEOUT);
});
done();
}, DEFAULT_SHORT_TIMEOUT);
});
it('should cancel on page unload', function (done) {
const interval = this.smartInterval;
it('should cancel on page unload', function (done) {
const interval = this.smartInterval;
setTimeout(() => {
$(document).triggerHandler('beforeunload');
expect(interval.state.intervalId).toBeUndefined();
expect(interval.getCurrentInterval()).toBe(interval.cfg.startingInterval);
done();
}, DEFAULT_SHORT_TIMEOUT);
});
setTimeout(() => {
$(document).triggerHandler('beforeunload');
expect(interval.state.intervalId).toBeUndefined();
expect(interval.getCurrentInterval()).toBe(interval.cfg.startingInterval);
done();
}, DEFAULT_SHORT_TIMEOUT);
});
it('should execute callback before first interval', function () {
const interval = createDefaultSmartInterval({ immediateExecution: true });
expect(interval.cfg.immediateExecution).toBeFalsy();
});
it('should execute callback before first interval', function () {
const interval = createDefaultSmartInterval({ immediateExecution: true });
expect(interval.cfg.immediateExecution).toBeFalsy();
});
});
})(window.gl || (window.gl = {}));
});
spec/javascripts/vue_mr_widget/mr_widget_options_spec.js
View file @ a3e1cb81
......@@ -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();
});
});
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment
GitLab Nexedi Edition | About GitLab | About Nexedi | 沪ICP备2021021310号-2 | 沪ICP备2021021310号-7