From 8d516604643103e87c2511ed6524522803f5a528 Mon Sep 17 00:00:00 2001 From: Pablo Alonso Garcia Date: Mon, 10 Aug 2015 17:28:03 +0200 Subject: [PATCH 01/20] Do not poll until previous requests have been received --- .../models/geocoding_model.js | 13 ++- .../geocoding_model.spec.js | 91 +++++++++++++++++++ 2 files changed, 100 insertions(+), 4 deletions(-) diff --git a/lib/assets/javascripts/cartodb/common/background_polling/models/geocoding_model.js b/lib/assets/javascripts/cartodb/common/background_polling/models/geocoding_model.js index 19773d238912..061c6c55831f 100644 --- a/lib/assets/javascripts/cartodb/common/background_polling/models/geocoding_model.js +++ b/lib/assets/javascripts/cartodb/common/background_polling/models/geocoding_model.js @@ -82,17 +82,22 @@ module.exports = cdb.core.Model.extend({ return false; } - this.pollTimer = setInterval(function() { + var fetch = function() { self.fetch({ + success: function() { + self.pollTimer = setTimeout(fetch, pollTimer); + }, error: function(e) { self.trigger("change"); } - }); - }, pollTimer); + }) + }; + + this.pollTimer = setTimeout(fetch, pollTimer); }, destroyCheck: function() { - clearInterval(this.pollTimer); + clearTimeout(this.pollTimer); delete this.pollTimer; }, diff --git a/lib/assets/test/spec/cartodb/common/background_importer/geocoding_model.spec.js b/lib/assets/test/spec/cartodb/common/background_importer/geocoding_model.spec.js index 1d610f0f54c8..0667a9310ae9 100644 --- a/lib/assets/test/spec/cartodb/common/background_importer/geocoding_model.spec.js +++ b/lib/assets/test/spec/cartodb/common/background_importer/geocoding_model.spec.js @@ -31,6 +31,97 @@ describe('common/background_polling/geocoding_model', function() { expect(this.model.bind.calls.argsFor(1)[0]).toEqual('change:state'); }); + describe('.pollCheck', function() { + + beforeEach(function() { + jasmine.clock().install(); + spyOn(this.model, 'fetch'); + }); + + afterEach(function() { + jasmine.clock().uninstall(); + }); + + it('should fetch after a number of seconds', function() { + this.model.pollCheck(); + + expect(this.model.fetch).not.toHaveBeenCalled(); + + jasmine.clock().tick(2001); + + expect(this.model.fetch).toHaveBeenCalled(); + expect(this.model.fetch.calls.count()).toEqual(1); + }); + + it('should not try to poll if already polling', function() { + this.model.pollCheck(); + this.model.pollCheck(); + + expect(this.model.fetch).not.toHaveBeenCalled(); + + jasmine.clock().tick(2001); + + expect(this.model.fetch).toHaveBeenCalled(); + expect(this.model.fetch.calls.count()).toEqual(1); + }); + + it('should trigger a change event if an error occurs', function() { + var onErrorCallback = jasmine.createSpy('error'); + + this.model.pollCheck(); + this.model.bind('change', onErrorCallback); + + jasmine.clock().tick(2001); + + this.model.fetch.calls.mostRecent().args[0].error(); + + expect(onErrorCallback).toHaveBeenCalled(); + }); + + it('should not trigger new requests if previous request haven\t succeeded', function() { + this.model.pollCheck(); + + jasmine.clock().tick(4001); + + // Fetch has only been called once because the first request hasn't been received + expect(this.model.fetch.calls.count()).toEqual(1); + + // First request has been received + this.model.fetch.calls.mostRecent().args[0].success(); + + // 2001 more millisecs are gone + jasmine.clock().tick(2001); + + // Another request has been triggered + expect(this.model.fetch.calls.count()).toEqual(2); + }); + }); + + describe('.destroyCheck', function() { + + beforeEach(function() { + jasmine.clock().install(); + spyOn(this.model, 'fetch'); + }); + + afterEach(function() { + jasmine.clock().uninstall(); + }); + + it('should destroy the check', function() { + this.model.pollCheck(); + this.model.destroyCheck(); + + expect(this.model.pollTimer).toBeUndefined(); + expect(this.model.fetch).not.toHaveBeenCalled(); + + jasmine.clock().tick(2001); + + expect(this.model.pollTimer).toBeUndefined(); + expect(this.model.fetch).not.toHaveBeenCalled(); + }); + }); + describe('polling', function() { beforeEach(function() { From c928303b3da41099c6fee59bd7405fb026b57067 Mon Sep 17 00:00:00 2001 From: Pablo Alonso Garcia Date: Mon, 10 Aug 2015 17:37:59 +0200 Subject: [PATCH 02/20] Revert to using an interval with a boolean flag to determine if fetch should be invoked on every cycle --- .../models/geocoding_model.js | 29 ++++++++++--------- 1 file changed, 16 insertions(+), 13 deletions(-) diff --git a/lib/assets/javascripts/cartodb/common/background_polling/models/geocoding_model.js b/lib/assets/javascripts/cartodb/common/background_polling/models/geocoding_model.js index 061c6c55831f..9c738cb4866e 100644 --- a/lib/assets/javascripts/cartodb/common/background_polling/models/geocoding_model.js +++ b/lib/assets/javascripts/cartodb/common/background_polling/models/geocoding_model.js @@ -82,22 +82,25 @@ module.exports = cdb.core.Model.extend({ return false; } - var fetch = function() { - self.fetch({ - success: function() { - self.pollTimer = setTimeout(fetch, pollTimer); - }, - error: function(e) { - self.trigger("change"); - } - }) - }; - - this.pollTimer = setTimeout(fetch, pollTimer); + self.polling = false; + + this.pollTimer = setInterval(function() { + if (!self.polling) { + self.polling = true; + self.fetch({ + success: function() { + self.polling = false; + }, + error: function(e) { + self.trigger("change"); + } + }) + } + }, pollTimer); }, destroyCheck: function() { - clearTimeout(this.pollTimer); + clearInterval(this.pollTimer); delete this.pollTimer; }, From 316c42c086a96a1428cdcc68232238af345933f8 Mon Sep 17 00:00:00 2001 From: Pablo Alonso Garcia Date: Mon, 10 Aug 2015 17:38:23 +0200 Subject: [PATCH 03/20] Typo --- .../cartodb/common/background_importer/geocoding_model.spec.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/assets/test/spec/cartodb/common/background_importer/geocoding_model.spec.js b/lib/assets/test/spec/cartodb/common/background_importer/geocoding_model.spec.js index 0667a9310ae9..4317fe4548d9 100644 --- a/lib/assets/test/spec/cartodb/common/background_importer/geocoding_model.spec.js +++ b/lib/assets/test/spec/cartodb/common/background_importer/geocoding_model.spec.js @@ -78,7 +78,7 @@ describe('common/background_polling/geocoding_model', function() { expect(onErrorCallback).toHaveBeenCalled(); }); - it('should not trigger new requests if previous request haven\t succeeded', function() { + it('should not trigger new requests if previous request haven\'t succeeded', function() { this.model.pollCheck(); jasmine.clock().tick(4001); From 106c085dd1ccd7085275ba85b158b0de7d66c1f6 Mon Sep 17 00:00:00 2001 From: Pablo Alonso Garcia Date: Mon, 10 Aug 2015 18:49:29 +0200 Subject: [PATCH 04/20] Extracted Poller class --- .../models/geocoding_model.js | 49 +++++---------- .../background_polling/models/poller.js | 63 +++++++++++++++++++ .../geocoding_model.spec.js | 6 +- 3 files changed, 80 insertions(+), 38 deletions(-) create mode 100644 lib/assets/javascripts/cartodb/common/background_polling/models/poller.js diff --git a/lib/assets/javascripts/cartodb/common/background_polling/models/geocoding_model.js b/lib/assets/javascripts/cartodb/common/background_polling/models/geocoding_model.js index 9c738cb4866e..6a647c1d51c8 100644 --- a/lib/assets/javascripts/cartodb/common/background_polling/models/geocoding_model.js +++ b/lib/assets/javascripts/cartodb/common/background_polling/models/geocoding_model.js @@ -1,12 +1,12 @@ var Backbone = require('backbone'); var _ = require('underscore'); -var pollTimer = 2000; +var Poller = require('./poller'); +var POLLING_INTERVAL = 2000; /** * Geocoding model * */ - module.exports = cdb.core.Model.extend({ options: { @@ -35,16 +35,25 @@ module.exports = cdb.core.Model.extend({ }, initialize: function(opts) { + var self = this; this._initBinds(); _.extend(this.options, opts); if (this.options.startPollingAutomatically) { this._checkModel(); - } + }; + this.poller = new Poller(this, { + interval: POLLING_INTERVAL, + condition: function(model) { + return model.hasFailed() || model.hasCompleted(); + }, + error: function() { + self.trigger("change"); + } + }); }, _initBinds: function() { this.bind('change:id', this._checkModel, this); - this.bind('change:state', this._checkState, this); }, _checkModel: function() { @@ -76,38 +85,12 @@ module.exports = cdb.core.Model.extend({ // checks for poll to finish pollCheck: function(i) { - var self = this; - - if (this.pollTimer) { - return false; - } - - self.polling = false; - - this.pollTimer = setInterval(function() { - if (!self.polling) { - self.polling = true; - self.fetch({ - success: function() { - self.polling = false; - }, - error: function(e) { - self.trigger("change"); - } - }) - } - }, pollTimer); + this.poller.start(); }, destroyCheck: function() { - clearInterval(this.pollTimer); - delete this.pollTimer; - }, - - _checkState: function() { - var state = this.get('state'); - if (this.hasFailed() || this.hasCompleted()) { - this.destroyCheck(); + if (this.poller) { + this.poller.stop(); } }, diff --git a/lib/assets/javascripts/cartodb/common/background_polling/models/poller.js b/lib/assets/javascripts/cartodb/common/background_polling/models/poller.js new file mode 100644 index 000000000000..73b9b25d8fe1 --- /dev/null +++ b/lib/assets/javascripts/cartodb/common/background_polling/models/poller.js @@ -0,0 +1,63 @@ +/* + * Periodically fetches a model/collection. It waits for ongoing + * fetch requests before trying to fetch again. A stop condition + * can be specified. + * + * Usage example: + * + * var poller = new Poller(model, { + * interval: 1000, + * condition: function(model) { + * return model.get('state') === 'completed'; + * } + * }); + * + * poller.start(); + * + * // ... + * + * poller.stop(); + * + */ +var Poller = function(model, options) { + this.model = model; + this.interval = options['interval']; + this.condition = options['condition']; + this.error = options['error']; +} + +Poller.prototype.start = function() { + var self = this; + + if (this.pollingInterval) { + return; + } + + var poll = function() { + if (!self.polling) { + self.polling = true; + self.model.fetch({ + success: function() { + self.polling = false; + if (self.condition && self.condition(self.model)) { + self.stop(); + } + }, + error: function(e) { + self.error && self.error(); + } + }) + } + } + + this.polling = false; + this.pollingInterval = setInterval(poll, this.interval); +} + +Poller.prototype.stop = function() { + this.polling = false; + clearInterval(this.pollingInterval); + delete(this.pollingInterval); +} + +module.exports = Poller; diff --git a/lib/assets/test/spec/cartodb/common/background_importer/geocoding_model.spec.js b/lib/assets/test/spec/cartodb/common/background_importer/geocoding_model.spec.js index 4317fe4548d9..99da6259e363 100644 --- a/lib/assets/test/spec/cartodb/common/background_importer/geocoding_model.spec.js +++ b/lib/assets/test/spec/cartodb/common/background_importer/geocoding_model.spec.js @@ -112,17 +112,15 @@ describe('common/background_polling/geocoding_model', function() { this.model.pollCheck(); this.model.destroyCheck(); - expect(this.model.pollTimer).toBeUndefined(); expect(this.model.fetch).not.toHaveBeenCalled(); jasmine.clock().tick(2001); - expect(this.model.pollTimer).toBeUndefined(); expect(this.model.fetch).not.toHaveBeenCalled(); }); }); - describe('polling', function() { + fdescribe('polling', function() { beforeEach(function() { this.model.set('id', '1'); @@ -135,7 +133,6 @@ describe('common/background_polling/geocoding_model', function() { setTimeout(function(){ self.model.set('state', 'failed'); expect(self.model.fetch.calls.count()).toBe(1); - expect(self.model.pollTimer).toBeUndefined(); done(); }, 2000); }); @@ -145,7 +142,6 @@ describe('common/background_polling/geocoding_model', function() { setTimeout(function(){ self.model.set('state', 'finished'); expect(self.model.fetch.calls.count()).toBe(1); - expect(self.model.pollTimer).toBeUndefined(); done(); }, 2000); }); From 22f071a2b21ba2be840831a4268d906e7bee787d Mon Sep 17 00:00:00 2001 From: Pablo Alonso Garcia Date: Tue, 11 Aug 2015 12:13:43 +0200 Subject: [PATCH 05/20] Extracted tests for Poller --- .../geocoding_model.spec.js | 52 ++------ .../common/background_importer/poller.spec.js | 111 ++++++++++++++++++ 2 files changed, 119 insertions(+), 44 deletions(-) create mode 100644 lib/assets/test/spec/cartodb/common/background_importer/poller.spec.js diff --git a/lib/assets/test/spec/cartodb/common/background_importer/geocoding_model.spec.js b/lib/assets/test/spec/cartodb/common/background_importer/geocoding_model.spec.js index 99da6259e363..99dc01739741 100644 --- a/lib/assets/test/spec/cartodb/common/background_importer/geocoding_model.spec.js +++ b/lib/assets/test/spec/cartodb/common/background_importer/geocoding_model.spec.js @@ -28,7 +28,6 @@ describe('common/background_polling/geocoding_model', function() { it('should have several change binds', function() { this.model._initBinds(); expect(this.model.bind.calls.argsFor(0)[0]).toEqual('change:id'); - expect(this.model.bind.calls.argsFor(1)[0]).toEqual('change:state'); }); describe('.pollCheck', function() { @@ -42,27 +41,12 @@ describe('common/background_polling/geocoding_model', function() { jasmine.clock().uninstall(); }); - it('should fetch after a number of seconds', function() { - this.model.pollCheck(); - - expect(this.model.fetch).not.toHaveBeenCalled(); - - jasmine.clock().tick(2001); - - expect(this.model.fetch).toHaveBeenCalled(); - expect(this.model.fetch.calls.count()).toEqual(1); - }); + it('should start polling', function() { + spyOn(this.model.poller, 'start'); - it('should not try to poll if already polling', function() { this.model.pollCheck(); - this.model.pollCheck(); - - expect(this.model.fetch).not.toHaveBeenCalled(); - - jasmine.clock().tick(2001); - expect(this.model.fetch).toHaveBeenCalled(); - expect(this.model.fetch.calls.count()).toEqual(1); + expect(this.model.poller.start).toHaveBeenCalled(); }); it('should trigger a change event if an error occurs', function() { @@ -77,24 +61,6 @@ describe('common/background_polling/geocoding_model', function() { expect(onErrorCallback).toHaveBeenCalled(); }); - - it('should not trigger new requests if previous request haven\'t succeeded', function() { - this.model.pollCheck(); - - jasmine.clock().tick(4001); - - // Fetch has only been called once because the first request hasn't been received - expect(this.model.fetch.calls.count()).toEqual(1); - - // First request has been received - this.model.fetch.calls.mostRecent().args[0].success(); - - // 2001 more millisecs are gone - jasmine.clock().tick(2001); - - // Another request has been triggered - expect(this.model.fetch.calls.count()).toEqual(2); - }); }); describe('.destroyCheck', function() { @@ -108,19 +74,17 @@ describe('common/background_polling/geocoding_model', function() { jasmine.clock().uninstall(); }); - it('should destroy the check', function() { + it('should stop polling', function() { + spyOn(this.model.poller, 'stop'); + this.model.pollCheck(); this.model.destroyCheck(); - expect(this.model.fetch).not.toHaveBeenCalled(); - - jasmine.clock().tick(2001); - - expect(this.model.fetch).not.toHaveBeenCalled(); + expect(this.model.poller.stop).toHaveBeenCalled(); }); }); - fdescribe('polling', function() { + describe('polling', function() { beforeEach(function() { this.model.set('id', '1'); diff --git a/lib/assets/test/spec/cartodb/common/background_importer/poller.spec.js b/lib/assets/test/spec/cartodb/common/background_importer/poller.spec.js new file mode 100644 index 000000000000..90fe608094fd --- /dev/null +++ b/lib/assets/test/spec/cartodb/common/background_importer/poller.spec.js @@ -0,0 +1,111 @@ +var Poller = require('../../../../../javascripts/cartodb/common/background_polling/models/poller'); + +describe('Poller', function() { + + var model; + + beforeEach(function() { + model = new Backbone.Model(); + jasmine.clock().install(); + spyOn(model, 'fetch'); + }); + + afterEach(function() { + jasmine.clock().uninstall(); + }); + + describe('.start', function() { + + it('should start fetching periodically after the specified number of seconds', function() { + var poller = new Poller(model, { interval: 100 }); + poller.start(); + + expect(model.fetch).not.toHaveBeenCalled(); + + jasmine.clock().tick(101); + + expect(model.fetch.calls.count()).toEqual(1); + + model.fetch.calls.mostRecent().args[0].success(); + + jasmine.clock().tick(100); + + expect(model.fetch.calls.count()).toEqual(2); + }); + + it('should not trigger new requests if previous request haven\'t succeeded', function() { + var poller = new Poller(model, { interval: 100 }); + poller.start(); + + jasmine.clock().tick(101); + + expect(model.fetch.calls.count()).toEqual(1); + + // First request has been received + model.fetch.calls.mostRecent().args[0].success(); + + // 2000 more millisecs are gone + jasmine.clock().tick(2000); + + // Only one more request has been triggered + expect(model.fetch.calls.count()).toEqual(2); + }); + + it('should not try to poll if already polling', function() { + var poller = new Poller(model, { interval: 100 }); + poller.start(); + poller.start(); + + expect(model.fetch).not.toHaveBeenCalled(); + + jasmine.clock().tick(101); + + expect(model.fetch.calls.count()).toEqual(1); + }); + + it('should stop polling when the condition is satisfied', function() { + + // This satisfies the condition + model.set({ 'something': 'wadus' }); + + var poller = new Poller(model, { + interval: 100, + condition: function(model) { + return model.get('something') === 'wadus'; + } + }); + + poller.start(); + + expect(model.fetch).not.toHaveBeenCalled(); + + jasmine.clock().tick(101); + + // Fetch has been called + expect(model.fetch.calls.count()).toEqual(1); + + // Fetch request suceeds + model.fetch.calls.mostRecent().args[0].success(); + + jasmine.clock().tick(101); + + // Fetch was only called the first time (condition was met afterwards) + expect(model.fetch.calls.count()).toEqual(1); + }); + }); + + describe('.stop', function() { + + it('should stop polling', function() { + var poller = new Poller(model, { interval: 100 }); + poller.start(); + poller.stop(); + + expect(model.fetch).not.toHaveBeenCalled(); + + jasmine.clock().tick(1000); + + expect(model.fetch).not.toHaveBeenCalled(); + }); + }); +}); \ No newline at end of file From bdf95d0ffc6e773217f40fff1e1065873193a44f Mon Sep 17 00:00:00 2001 From: Pablo Alonso Garcia Date: Tue, 11 Aug 2015 12:40:34 +0200 Subject: [PATCH 06/20] Added autoStart option to the Poller --- .../background_polling/models/poller.js | 5 +++++ .../common/background_importer/poller.spec.js | 19 +++++++++++++++++++ 2 files changed, 24 insertions(+) diff --git a/lib/assets/javascripts/cartodb/common/background_polling/models/poller.js b/lib/assets/javascripts/cartodb/common/background_polling/models/poller.js index 73b9b25d8fe1..8527e63fec1e 100644 --- a/lib/assets/javascripts/cartodb/common/background_polling/models/poller.js +++ b/lib/assets/javascripts/cartodb/common/background_polling/models/poller.js @@ -24,6 +24,11 @@ var Poller = function(model, options) { this.interval = options['interval']; this.condition = options['condition']; this.error = options['error']; + this.autoStart = options['autoStart']; + + if (this.autoStart) { + this.start(); + } } Poller.prototype.start = function() { diff --git a/lib/assets/test/spec/cartodb/common/background_importer/poller.spec.js b/lib/assets/test/spec/cartodb/common/background_importer/poller.spec.js index 90fe608094fd..97a2e11fac7c 100644 --- a/lib/assets/test/spec/cartodb/common/background_importer/poller.spec.js +++ b/lib/assets/test/spec/cartodb/common/background_importer/poller.spec.js @@ -33,6 +33,25 @@ describe('Poller', function() { expect(model.fetch.calls.count()).toEqual(2); }); + it('should auto start fetching if autoStart option is true', function() { + var poller = new Poller(model, { + interval: 100, + autoStart: true + }); + + expect(model.fetch).not.toHaveBeenCalled(); + + jasmine.clock().tick(101); + + expect(model.fetch.calls.count()).toEqual(1); + + model.fetch.calls.mostRecent().args[0].success(); + + jasmine.clock().tick(100); + + expect(model.fetch.calls.count()).toEqual(2); + }); + it('should not trigger new requests if previous request haven\'t succeeded', function() { var poller = new Poller(model, { interval: 100 }); poller.start(); From cce5ac4f279e603e50e0c4ae51038efb52381db2 Mon Sep 17 00:00:00 2001 From: Pablo Alonso Garcia Date: Tue, 11 Aug 2015 12:45:52 +0200 Subject: [PATCH 07/20] Poller needs to be instantiated before polling can actually happen --- .../common/background_polling/models/geocoding_model.js | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/lib/assets/javascripts/cartodb/common/background_polling/models/geocoding_model.js b/lib/assets/javascripts/cartodb/common/background_polling/models/geocoding_model.js index 6a647c1d51c8..769a420d4c4e 100644 --- a/lib/assets/javascripts/cartodb/common/background_polling/models/geocoding_model.js +++ b/lib/assets/javascripts/cartodb/common/background_polling/models/geocoding_model.js @@ -38,9 +38,6 @@ module.exports = cdb.core.Model.extend({ var self = this; this._initBinds(); _.extend(this.options, opts); - if (this.options.startPollingAutomatically) { - this._checkModel(); - }; this.poller = new Poller(this, { interval: POLLING_INTERVAL, condition: function(model) { @@ -50,6 +47,9 @@ module.exports = cdb.core.Model.extend({ self.trigger("change"); } }); + if (this.options.startPollingAutomatically) { + this._checkModel(); + }; }, _initBinds: function() { @@ -112,13 +112,10 @@ module.exports = cdb.core.Model.extend({ }, cancelGeocoding: function() { - this.destroyCheck(); this.save({ state: 'cancelled' }, { wait:true }); }, resetGeocoding: function() { - this.destroyCheck(); this.set('state', 'reset'); } - }); From 356141b4fbb9d8bdd93a9a034779e35a9fa8c036 Mon Sep 17 00:00:00 2001 From: Pablo Alonso Garcia Date: Tue, 11 Aug 2015 16:32:25 +0200 Subject: [PATCH 08/20] Poller accepts a function to specify a variable interval --- .../background_polling/models/poller.js | 52 ++++++++------- .../common/background_importer/poller.spec.js | 66 ++++++++++++++----- 2 files changed, 80 insertions(+), 38 deletions(-) diff --git a/lib/assets/javascripts/cartodb/common/background_polling/models/poller.js b/lib/assets/javascripts/cartodb/common/background_polling/models/poller.js index 8527e63fec1e..b2dd76b6d4d9 100644 --- a/lib/assets/javascripts/cartodb/common/background_polling/models/poller.js +++ b/lib/assets/javascripts/cartodb/common/background_polling/models/poller.js @@ -21,7 +21,12 @@ */ var Poller = function(model, options) { this.model = model; + this.numberOfTries = 0; + this.polling = false; this.interval = options['interval']; + if (typeof this.interval !== "function") { + this.interval = function() { return options['interval']; }; + } this.condition = options['condition']; this.error = options['error']; this.autoStart = options['autoStart']; @@ -32,37 +37,40 @@ var Poller = function(model, options) { } Poller.prototype.start = function() { - var self = this; - - if (this.pollingInterval) { + if (this.timeout) { return; } - var poll = function() { - if (!self.polling) { - self.polling = true; - self.model.fetch({ - success: function() { - self.polling = false; - if (self.condition && self.condition(self.model)) { - self.stop(); - } - }, - error: function(e) { - self.error && self.error(); + this._scheduleFetch(); +} + +Poller.prototype._scheduleFetch = function() { + this.timeout = setTimeout(this._fetch.bind(this), this.interval(this.numberOfTries)); +} + +Poller.prototype._fetch = function() { + var self = this; + if (!self.polling) { + self.polling = true; + self.model.fetch({ + success: function() { + self.polling = false; + if (!self.condition || (self.condition && !self.condition(self.model))) { + self.numberOfTries++; + self._scheduleFetch(); } - }) - } + }, + error: function(e) { + self.error && self.error(); + } + }) } - - this.polling = false; - this.pollingInterval = setInterval(poll, this.interval); } Poller.prototype.stop = function() { this.polling = false; - clearInterval(this.pollingInterval); - delete(this.pollingInterval); + clearTimeout(this.timeout); + delete(this.timeout); } module.exports = Poller; diff --git a/lib/assets/test/spec/cartodb/common/background_importer/poller.spec.js b/lib/assets/test/spec/cartodb/common/background_importer/poller.spec.js index 97a2e11fac7c..289b24c9043c 100644 --- a/lib/assets/test/spec/cartodb/common/background_importer/poller.spec.js +++ b/lib/assets/test/spec/cartodb/common/background_importer/poller.spec.js @@ -2,12 +2,21 @@ var Poller = require('../../../../../javascripts/cartodb/common/background_polli describe('Poller', function() { - var model; + var model, originalSync; beforeEach(function() { - model = new Backbone.Model(); + var Model = Backbone.Model.extend({ + url: '/hello' + }); + model = new Model(); + originalSync = model.sync; + model.sync = function(a,b,opts) { + opts.success(); + }; + jasmine.clock().install(); - spyOn(model, 'fetch'); + spyOn(model, 'fetch').and.callThrough(); + }); afterEach(function() { @@ -22,12 +31,10 @@ describe('Poller', function() { expect(model.fetch).not.toHaveBeenCalled(); - jasmine.clock().tick(101); + jasmine.clock().tick(100); expect(model.fetch.calls.count()).toEqual(1); - model.fetch.calls.mostRecent().args[0].success(); - jasmine.clock().tick(100); expect(model.fetch.calls.count()).toEqual(2); @@ -41,22 +48,22 @@ describe('Poller', function() { expect(model.fetch).not.toHaveBeenCalled(); - jasmine.clock().tick(101); + jasmine.clock().tick(100); expect(model.fetch.calls.count()).toEqual(1); - model.fetch.calls.mostRecent().args[0].success(); - jasmine.clock().tick(100); expect(model.fetch.calls.count()).toEqual(2); }); it('should not trigger new requests if previous request haven\'t succeeded', function() { + model.sync = originalSync; + var poller = new Poller(model, { interval: 100 }); poller.start(); - jasmine.clock().tick(101); + jasmine.clock().tick(100); expect(model.fetch.calls.count()).toEqual(1); @@ -77,7 +84,7 @@ describe('Poller', function() { expect(model.fetch).not.toHaveBeenCalled(); - jasmine.clock().tick(101); + jasmine.clock().tick(100); expect(model.fetch.calls.count()).toEqual(1); }); @@ -98,19 +105,46 @@ describe('Poller', function() { expect(model.fetch).not.toHaveBeenCalled(); - jasmine.clock().tick(101); + jasmine.clock().tick(100); // Fetch has been called expect(model.fetch.calls.count()).toEqual(1); - // Fetch request suceeds - model.fetch.calls.mostRecent().args[0].success(); - - jasmine.clock().tick(101); + jasmine.clock().tick(100); // Fetch was only called the first time (condition was met afterwards) expect(model.fetch.calls.count()).toEqual(1); }); + + it('should use the given function to determine the interval', function() { + + model.sync = function(a,b,opts) { + opts.success(); + }; + + var poller = new Poller(model, { + interval: function(n) { + if (n === 0) { + return 100; + } else { + return 1; + } + + return 100; + }, + autoStart: true + }); + + expect(model.fetch).not.toHaveBeenCalled(); + + jasmine.clock().tick(100); + + expect(model.fetch.calls.count()).toEqual(1); + + jasmine.clock().tick(1); + + expect(model.fetch.calls.count()).toEqual(2); + }) }); describe('.stop', function() { From 4be9e0ac01bffc3a0721bed219f321d9b1879814 Mon Sep 17 00:00:00 2001 From: Pablo Alonso Garcia Date: Tue, 11 Aug 2015 17:42:51 +0200 Subject: [PATCH 09/20] ImportModel uses Poller to poll the model --- .../models/geocoding_model.js | 7 +-- .../background_polling/models/import_model.js | 50 +++++++------------ .../background_importer/imports_model.spec.js | 12 ----- 3 files changed, 20 insertions(+), 49 deletions(-) diff --git a/lib/assets/javascripts/cartodb/common/background_polling/models/geocoding_model.js b/lib/assets/javascripts/cartodb/common/background_polling/models/geocoding_model.js index 769a420d4c4e..dffca3a0d83d 100644 --- a/lib/assets/javascripts/cartodb/common/background_polling/models/geocoding_model.js +++ b/lib/assets/javascripts/cartodb/common/background_polling/models/geocoding_model.js @@ -83,15 +83,12 @@ module.exports = cdb.core.Model.extend({ } }, - // checks for poll to finish - pollCheck: function(i) { + pollCheck: function() { this.poller.start(); }, destroyCheck: function() { - if (this.poller) { - this.poller.stop(); - } + this.poller.stop(); }, getError: function() { diff --git a/lib/assets/javascripts/cartodb/common/background_polling/models/import_model.js b/lib/assets/javascripts/cartodb/common/background_polling/models/import_model.js index 2d6604228ace..4a060f0c05c8 100644 --- a/lib/assets/javascripts/cartodb/common/background_polling/models/import_model.js +++ b/lib/assets/javascripts/cartodb/common/background_polling/models/import_model.js @@ -1,15 +1,15 @@ var cdb = require('cartodb.js'); var cdbAdmin = require('cdb.admin'); -var pollTimer = 2000; // Interval time between poll checkings -var timerMultiply = 2.5; // Multiply interval for this number -var maxTries = 30; // Max tries until interval change +var Poller = require('./poller'); +var POLLING_INTERVAL = 2000; // Interval time between poll checkings +var POLLING_INTERVAL_MULTIPLIER = 2.5; // Multiply interval by this number +var POLLING_REQUESTS_BEFORE_INTERVAL_CHANGE = 30; // Max tries until interval change /** * New import model that controls * the state of an import * */ - module.exports = cdb.core.Model.extend({ url: function(method) { @@ -25,11 +25,21 @@ module.exports = cdb.core.Model.extend({ initialize: function() { this._initBinds(); + this.poller = new Poller(this, { + interval: function(numberOfRequests) { + if (numberOfRequests >= POLLING_REQUESTS_BEFORE_INTERVAL_CHANGE) { + return POLLING_INTERVAL * POLLING_INTERVAL_MULTIPLIER; + } + return POLLING_INTERVAL; + }, + condition: function(model) { + var state = model.get('state'); + return (state === "complete" || state === "failure"); + } + }); }, _initBinds: function() { - this.bind('change:state', this._checkState, this); - this.bind('change:success', this._checkState, this); this.bind('change:item_queue_id', this._checkQueueId, this); }, @@ -39,13 +49,6 @@ module.exports = cdb.core.Model.extend({ this[ data.interval > 0 ? '_createSyncImport' : '_createRegularImport'](d); }, - _checkState: function() { - var state = this.get('state'); - if (state === "complete" || state === "failure") { - this.destroyCheck(); - } - }, - _checkQueueId: function() { if (this.get('item_queue_id')) { this.pollCheck(); @@ -138,28 +141,11 @@ module.exports = cdb.core.Model.extend({ }, pollCheck: function() { - if (this.pollTimer) return; - var self = this; - var tries = 0; - this.pollTimer = setInterval(request, pollTimer); - - function request() { - self.destroyCheck(); - self.fetch(); - ++tries; - // Multiply polling timer by a number when a max - // of tries have been reached - var multiply = tries > maxTries ? timerMultiply : 1 ; - self.pollTimer = setInterval(request, pollTimer * multiply); - } - - // Start doing a fetch - self.fetch(); + this.poller.start(); }, destroyCheck: function() { - clearInterval(this.pollTimer); - delete this.pollTimer; + this.poller.stop(); } }); diff --git a/lib/assets/test/spec/cartodb/common/background_importer/imports_model.spec.js b/lib/assets/test/spec/cartodb/common/background_importer/imports_model.spec.js index f33a3bb4c6c0..d8e7d13dbe96 100644 --- a/lib/assets/test/spec/cartodb/common/background_importer/imports_model.spec.js +++ b/lib/assets/test/spec/cartodb/common/background_importer/imports_model.spec.js @@ -218,22 +218,10 @@ describe('common/background_polling/imports_model', function() { expect(this.imp._createSyncImport).not.toHaveBeenCalled(); }); - it('should stop polling when state is failure or complete', function() { - spyOn(this.imp, 'destroyCheck'); - this.imp.set('state', 'failure'); - expect(this.imp.destroyCheck).toHaveBeenCalled(); - expect(this.imp.destroyCheck.calls.count()).toEqual(1); - this.imp.set('state', 'complete'); - expect(this.imp.destroyCheck).toHaveBeenCalled(); - expect(this.imp.destroyCheck.calls.count()).toEqual(2); - }); - it('should start polling when item_queue_id is set', function() { spyOn(this.imp, 'pollCheck'); this.imp.set('item_queue_id', 'vamos-neno'); expect(this.imp.pollCheck).toHaveBeenCalled(); }); - }); - }); From 5074c0e38ec57daa92301bd102ba0ca241e732d4 Mon Sep 17 00:00:00 2001 From: Pablo Alonso Garcia Date: Tue, 11 Aug 2015 17:43:23 +0200 Subject: [PATCH 10/20] Renamed instance variable --- .../cartodb/common/background_polling/models/poller.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/assets/javascripts/cartodb/common/background_polling/models/poller.js b/lib/assets/javascripts/cartodb/common/background_polling/models/poller.js index b2dd76b6d4d9..f021b91a68da 100644 --- a/lib/assets/javascripts/cartodb/common/background_polling/models/poller.js +++ b/lib/assets/javascripts/cartodb/common/background_polling/models/poller.js @@ -21,7 +21,7 @@ */ var Poller = function(model, options) { this.model = model; - this.numberOfTries = 0; + this.numberOfRequests = 0; this.polling = false; this.interval = options['interval']; if (typeof this.interval !== "function") { @@ -45,7 +45,7 @@ Poller.prototype.start = function() { } Poller.prototype._scheduleFetch = function() { - this.timeout = setTimeout(this._fetch.bind(this), this.interval(this.numberOfTries)); + this.timeout = setTimeout(this._fetch.bind(this), this.interval(this.numberOfRequests)); } Poller.prototype._fetch = function() { @@ -56,7 +56,7 @@ Poller.prototype._fetch = function() { success: function() { self.polling = false; if (!self.condition || (self.condition && !self.condition(self.model))) { - self.numberOfTries++; + self.numberOfRequests++; self._scheduleFetch(); } }, From 8f106967f7a6c8d05035cc3d8ca6889ad5b89eb1 Mon Sep 17 00:00:00 2001 From: Pablo Alonso Garcia Date: Tue, 11 Aug 2015 17:48:14 +0200 Subject: [PATCH 11/20] `condition` shouldn't determine when to incremente the number of requests --- .../cartodb/common/background_polling/models/poller.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/assets/javascripts/cartodb/common/background_polling/models/poller.js b/lib/assets/javascripts/cartodb/common/background_polling/models/poller.js index f021b91a68da..cedfb0d8ef40 100644 --- a/lib/assets/javascripts/cartodb/common/background_polling/models/poller.js +++ b/lib/assets/javascripts/cartodb/common/background_polling/models/poller.js @@ -55,8 +55,8 @@ Poller.prototype._fetch = function() { self.model.fetch({ success: function() { self.polling = false; + self.numberOfRequests++; if (!self.condition || (self.condition && !self.condition(self.model))) { - self.numberOfRequests++; self._scheduleFetch(); } }, From ce9aa71bdbe613e65abaace3c748967024b315ec Mon Sep 17 00:00:00 2001 From: Pablo Alonso Garcia Date: Tue, 11 Aug 2015 18:14:35 +0200 Subject: [PATCH 12/20] Tests --- .../background_importer/imports_model.spec.js | 31 ++++++++++++++++--- 1 file changed, 27 insertions(+), 4 deletions(-) diff --git a/lib/assets/test/spec/cartodb/common/background_importer/imports_model.spec.js b/lib/assets/test/spec/cartodb/common/background_importer/imports_model.spec.js index d8e7d13dbe96..5400babf65e1 100644 --- a/lib/assets/test/spec/cartodb/common/background_importer/imports_model.spec.js +++ b/lib/assets/test/spec/cartodb/common/background_importer/imports_model.spec.js @@ -195,10 +195,6 @@ describe('common/background_polling/imports_model', function() { it('should have several change binds', function() { this.imp._initBinds(); var args_0 = this.imp.bind.calls.argsFor(0); - expect(args_0[0]).toEqual('change:state'); - var args_1 = this.imp.bind.calls.argsFor(1); - expect(args_1[0]).toEqual('change:success'); - var args_2 = this.imp.bind.calls.argsFor(2); expect(args_2[0]).toEqual('change:item_queue_id'); }); @@ -223,5 +219,32 @@ describe('common/background_polling/imports_model', function() { this.imp.set('item_queue_id', 'vamos-neno'); expect(this.imp.pollCheck).toHaveBeenCalled(); }); + + describe('polling', function() { + + beforeEach(function() { + this.imp.set('id', '1'); + spyOn(this.imp, 'fetch').and.callThrough(); + this.imp.pollCheck(); + }); + + it('should stop polling when geocoding has failed', function(done) { + var self = this; + setTimeout(function(){ + self.imp.set('state', 'failed'); + expect(self.imp.fetch.calls.count()).toBe(1); + done(); + }, 2000); + }); + + it('should stop polling when geocoding has completed', function(done) { + var self = this; + setTimeout(function(){ + self.imp.set('state', 'finished'); + expect(self.imp.fetch.calls.count()).toBe(1); + done(); + }, 2000); + }); + }); }); }); From 0066cb1cf3514b49e015629d5a4917be0d9225bc Mon Sep 17 00:00:00 2001 From: Pablo Alonso Garcia Date: Tue, 11 Aug 2015 18:24:06 +0200 Subject: [PATCH 13/20] Fix test --- .../cartodb/common/background_importer/imports_model.spec.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/assets/test/spec/cartodb/common/background_importer/imports_model.spec.js b/lib/assets/test/spec/cartodb/common/background_importer/imports_model.spec.js index 5400babf65e1..7c06901460b6 100644 --- a/lib/assets/test/spec/cartodb/common/background_importer/imports_model.spec.js +++ b/lib/assets/test/spec/cartodb/common/background_importer/imports_model.spec.js @@ -195,7 +195,7 @@ describe('common/background_polling/imports_model', function() { it('should have several change binds', function() { this.imp._initBinds(); var args_0 = this.imp.bind.calls.argsFor(0); - expect(args_2[0]).toEqual('change:item_queue_id'); + expect(args_0[0]).toEqual('change:item_queue_id'); }); it('should create a sync import when interval is greater than 0', function() { From 5d064c2c900cabc94fddadf75b63352314d7e7e3 Mon Sep 17 00:00:00 2001 From: Pablo Alonso Garcia Date: Tue, 11 Aug 2015 19:11:36 +0200 Subject: [PATCH 14/20] Fixed false positives and try to fix jasmine timeout --- .../geocoding_model.spec.js | 38 +++++++----- .../background_importer/imports_model.spec.js | 60 +++++++++++++------ 2 files changed, 66 insertions(+), 32 deletions(-) diff --git a/lib/assets/test/spec/cartodb/common/background_importer/geocoding_model.spec.js b/lib/assets/test/spec/cartodb/common/background_importer/geocoding_model.spec.js index 99dc01739741..57379e3a8c54 100644 --- a/lib/assets/test/spec/cartodb/common/background_importer/geocoding_model.spec.js +++ b/lib/assets/test/spec/cartodb/common/background_importer/geocoding_model.spec.js @@ -87,27 +87,35 @@ describe('common/background_polling/geocoding_model', function() { describe('polling', function() { beforeEach(function() { + jasmine.clock().install(); this.model.set('id', '1'); + this.model.sync = function(a,b,opts) { + opts.success(); + }; + spyOn(this.model, 'fetch').and.callThrough(); - this.model.pollCheck(); }); - it('should stop polling when geocoding has failed', function(done) { - var self = this; - setTimeout(function(){ - self.model.set('state', 'failed'); - expect(self.model.fetch.calls.count()).toBe(1); - done(); - }, 2000); + afterEach(function() { + jasmine.clock().uninstall(); }); - it('should stop polling when geocoding has completed', function(done) { - var self = this; - setTimeout(function(){ - self.model.set('state', 'finished'); - expect(self.model.fetch.calls.count()).toBe(1); - done(); - }, 2000); + it('should stop polling when geocoding has failed', function() { + this.model.set('state', 'failed'); + this.model.pollCheck(); + + jasmine.clock().tick(4000); + + expect(this.model.fetch.calls.count()).toBe(1); + }); + + it('should stop polling when geocoding has completed', function() { + this.model.set('state', 'finished'); + this.model.pollCheck(); + + jasmine.clock().tick(4000); + + expect(this.model.fetch.calls.count()).toBe(1); }); }); diff --git a/lib/assets/test/spec/cartodb/common/background_importer/imports_model.spec.js b/lib/assets/test/spec/cartodb/common/background_importer/imports_model.spec.js index 7c06901460b6..bc1fbd379e58 100644 --- a/lib/assets/test/spec/cartodb/common/background_importer/imports_model.spec.js +++ b/lib/assets/test/spec/cartodb/common/background_importer/imports_model.spec.js @@ -223,27 +223,53 @@ describe('common/background_polling/imports_model', function() { describe('polling', function() { beforeEach(function() { - this.imp.set('id', '1'); - spyOn(this.imp, 'fetch').and.callThrough(); - this.imp.pollCheck(); + jasmine.clock().install(); + this.model = this.imp; + this.model.set('id', '1'); + this.model.sync = function(a,b,opts) { + opts.success(); + }; + + spyOn(this.model, 'fetch').and.callThrough(); }); - it('should stop polling when geocoding has failed', function(done) { - var self = this; - setTimeout(function(){ - self.imp.set('state', 'failed'); - expect(self.imp.fetch.calls.count()).toBe(1); - done(); - }, 2000); + afterEach(function() { + jasmine.clock().uninstall(); }); - it('should stop polling when geocoding has completed', function(done) { - var self = this; - setTimeout(function(){ - self.imp.set('state', 'finished'); - expect(self.imp.fetch.calls.count()).toBe(1); - done(); - }, 2000); + it('should stop polling when geocoding has failed', function() { + this.model.set('state', 'failure'); + this.model.pollCheck(); + + jasmine.clock().tick(4000); + + expect(this.model.fetch.calls.count()).toBe(1); + }); + + it('should stop polling when geocoding has completed', function() { + this.model.set('state', 'complete'); + this.model.pollCheck(); + + jasmine.clock().tick(4000); + + expect(this.model.fetch.calls.count()).toBe(1); + }); + + it('should increment the interval after a number of requests', function() { + this.model.pollCheck(); + + // 2000 is the interval period and 30 the number of requests before + // the interval is incremented. + jasmine.clock().tick(2000 * 30); + + // 30 requests have been made + expect(this.model.fetch.calls.count()).toBe(30); + + // Interval has been incremented to 2000 * 2.5 + jasmine.clock().tick(2000 * 2.5 + 1); + + // Only one more request has been made (interval has been incremented) + expect(this.model.fetch.calls.count()).toBe(31); }); }); }); From c2d18b321f208315f91c842813ff474f35285dcb Mon Sep 17 00:00:00 2001 From: Pablo Alonso Garcia Date: Wed, 12 Aug 2015 11:24:30 +0200 Subject: [PATCH 15/20] Created a GeocodingModelPoller and ImportModelPoller with specific configuration for each type of model --- .../models/geocoding_model.js | 13 +--- .../models/geocoding_model_poller.js | 22 +++++++ .../background_polling/models/import_model.js | 18 +---- .../models/import_model_poller.js | 27 ++++++++ .../geocoding_model.spec.js | 48 -------------- .../geocoding_model_poller.spec.js | 66 +++++++++++++++++++ .../import_model_poller.spec.js | 64 ++++++++++++++++++ .../background_importer/imports_model.spec.js | 53 --------------- 8 files changed, 183 insertions(+), 128 deletions(-) create mode 100644 lib/assets/javascripts/cartodb/common/background_polling/models/geocoding_model_poller.js create mode 100644 lib/assets/javascripts/cartodb/common/background_polling/models/import_model_poller.js create mode 100644 lib/assets/test/spec/cartodb/common/background_importer/geocoding_model_poller.spec.js create mode 100644 lib/assets/test/spec/cartodb/common/background_importer/import_model_poller.spec.js diff --git a/lib/assets/javascripts/cartodb/common/background_polling/models/geocoding_model.js b/lib/assets/javascripts/cartodb/common/background_polling/models/geocoding_model.js index dffca3a0d83d..4ecc57d18ae5 100644 --- a/lib/assets/javascripts/cartodb/common/background_polling/models/geocoding_model.js +++ b/lib/assets/javascripts/cartodb/common/background_polling/models/geocoding_model.js @@ -1,7 +1,6 @@ var Backbone = require('backbone'); var _ = require('underscore'); -var Poller = require('./poller'); -var POLLING_INTERVAL = 2000; +var GeocodingModelPoller = require('./geocoding_model_poller'); /** * Geocoding model @@ -38,15 +37,7 @@ module.exports = cdb.core.Model.extend({ var self = this; this._initBinds(); _.extend(this.options, opts); - this.poller = new Poller(this, { - interval: POLLING_INTERVAL, - condition: function(model) { - return model.hasFailed() || model.hasCompleted(); - }, - error: function() { - self.trigger("change"); - } - }); + this.poller = new GeocodingModelPoller(this); if (this.options.startPollingAutomatically) { this._checkModel(); }; diff --git a/lib/assets/javascripts/cartodb/common/background_polling/models/geocoding_model_poller.js b/lib/assets/javascripts/cartodb/common/background_polling/models/geocoding_model_poller.js new file mode 100644 index 000000000000..157899f56921 --- /dev/null +++ b/lib/assets/javascripts/cartodb/common/background_polling/models/geocoding_model_poller.js @@ -0,0 +1,22 @@ +var Poller = require('./poller'); + +var GeocodingModelPoller = function(model) { + + var POLLING_INTERVAL = 2000; + + var options = { + interval: POLLING_INTERVAL, + condition: function(model) { + return model.hasFailed() || model.hasCompleted(); + }, + error: function() { + this.model.trigger("change"); + } + }; + + Poller.call(this, model, options); +}; + +GeocodingModelPoller.prototype = _.extend({}, Poller.prototype); + +module.exports = GeocodingModelPoller; diff --git a/lib/assets/javascripts/cartodb/common/background_polling/models/import_model.js b/lib/assets/javascripts/cartodb/common/background_polling/models/import_model.js index 4a060f0c05c8..ae96ead6b0d6 100644 --- a/lib/assets/javascripts/cartodb/common/background_polling/models/import_model.js +++ b/lib/assets/javascripts/cartodb/common/background_polling/models/import_model.js @@ -1,9 +1,6 @@ var cdb = require('cartodb.js'); var cdbAdmin = require('cdb.admin'); -var Poller = require('./poller'); -var POLLING_INTERVAL = 2000; // Interval time between poll checkings -var POLLING_INTERVAL_MULTIPLIER = 2.5; // Multiply interval by this number -var POLLING_REQUESTS_BEFORE_INTERVAL_CHANGE = 30; // Max tries until interval change +var ImportModelPoller = require('./import_model_poller'); /** * New import model that controls @@ -25,18 +22,7 @@ module.exports = cdb.core.Model.extend({ initialize: function() { this._initBinds(); - this.poller = new Poller(this, { - interval: function(numberOfRequests) { - if (numberOfRequests >= POLLING_REQUESTS_BEFORE_INTERVAL_CHANGE) { - return POLLING_INTERVAL * POLLING_INTERVAL_MULTIPLIER; - } - return POLLING_INTERVAL; - }, - condition: function(model) { - var state = model.get('state'); - return (state === "complete" || state === "failure"); - } - }); + this.poller = new ImportModelPoller(this); }, _initBinds: function() { diff --git a/lib/assets/javascripts/cartodb/common/background_polling/models/import_model_poller.js b/lib/assets/javascripts/cartodb/common/background_polling/models/import_model_poller.js new file mode 100644 index 000000000000..7b628f83796e --- /dev/null +++ b/lib/assets/javascripts/cartodb/common/background_polling/models/import_model_poller.js @@ -0,0 +1,27 @@ +var Poller = require('./poller'); + +var ImportModelPoller = function(model) { + + var POLLING_INTERVAL = 2000; // Interval time between poll checkings + var POLLING_INTERVAL_MULTIPLIER = 2.5; // Multiply interval by this number + var POLLING_REQUESTS_BEFORE_INTERVAL_CHANGE = 30; // Max tries until interval change + + var options = { + interval: function(numberOfRequests) { + if (numberOfRequests >= POLLING_REQUESTS_BEFORE_INTERVAL_CHANGE) { + return POLLING_INTERVAL * POLLING_INTERVAL_MULTIPLIER; + } + return POLLING_INTERVAL; + }, + condition: function(model) { + var state = model.get('state'); + return (state === "complete" || state === "failure"); + } + }; + + Poller.call(this, model, options); +}; + +ImportModelPoller.prototype = _.extend({}, Poller.prototype); + +module.exports = ImportModelPoller; diff --git a/lib/assets/test/spec/cartodb/common/background_importer/geocoding_model.spec.js b/lib/assets/test/spec/cartodb/common/background_importer/geocoding_model.spec.js index 57379e3a8c54..3ecb8bc07834 100644 --- a/lib/assets/test/spec/cartodb/common/background_importer/geocoding_model.spec.js +++ b/lib/assets/test/spec/cartodb/common/background_importer/geocoding_model.spec.js @@ -48,19 +48,6 @@ describe('common/background_polling/geocoding_model', function() { expect(this.model.poller.start).toHaveBeenCalled(); }); - - it('should trigger a change event if an error occurs', function() { - var onErrorCallback = jasmine.createSpy('error'); - - this.model.pollCheck(); - this.model.bind('change', onErrorCallback); - - jasmine.clock().tick(2001); - - this.model.fetch.calls.mostRecent().args[0].error(); - - expect(onErrorCallback).toHaveBeenCalled(); - }); }); describe('.destroyCheck', function() { @@ -84,41 +71,6 @@ describe('common/background_polling/geocoding_model', function() { }); }); - describe('polling', function() { - - beforeEach(function() { - jasmine.clock().install(); - this.model.set('id', '1'); - this.model.sync = function(a,b,opts) { - opts.success(); - }; - - spyOn(this.model, 'fetch').and.callThrough(); - }); - - afterEach(function() { - jasmine.clock().uninstall(); - }); - - it('should stop polling when geocoding has failed', function() { - this.model.set('state', 'failed'); - this.model.pollCheck(); - - jasmine.clock().tick(4000); - - expect(this.model.fetch.calls.count()).toBe(1); - }); - - it('should stop polling when geocoding has completed', function() { - this.model.set('state', 'finished'); - this.model.pollCheck(); - - jasmine.clock().tick(4000); - - expect(this.model.fetch.calls.count()).toBe(1); - }); - }); - it('should know when geocoding has finished', function() { expect(this.model.hasCompleted()).toBeFalsy(); this.model.set('state', 'finished'); diff --git a/lib/assets/test/spec/cartodb/common/background_importer/geocoding_model_poller.spec.js b/lib/assets/test/spec/cartodb/common/background_importer/geocoding_model_poller.spec.js new file mode 100644 index 000000000000..d3c8097128f7 --- /dev/null +++ b/lib/assets/test/spec/cartodb/common/background_importer/geocoding_model_poller.spec.js @@ -0,0 +1,66 @@ + +var GeocodingModelPoller = require('../../../../../javascripts/cartodb/common/background_polling/models/geocoding_model_poller'); + +describe('GeocodingModelPoller', function() { + + var originalFetch; + + beforeEach(function() { + + var Model = Backbone.Model.extend({ + url: '/hello' + }); + this.model = new Model(); + this.model.hasFailed = function() { return false }; + this.model.hasCompleted = function() { return false }; + this.model.sync = function(a,b,opts) { + opts.success(); + }; + originalFetch = this.model.fetch; + spyOn(this.model, 'fetch').and.callThrough(); + + this.poller = new GeocodingModelPoller(this.model); + + jasmine.clock().install(); + }); + + afterEach(function() { + jasmine.clock().uninstall(); + }); + + it('should stop polling when geocoding has failed', function() { + this.model.hasFailed = function(){ return true }; + + this.poller.start(); + + jasmine.clock().tick(4000); + + expect(this.model.fetch.calls.count()).toBe(1); + }); + + it('should stop polling when geocoding has completed', function() { + this.model.hasCompleted = function(){ return true }; + + this.poller.start(); + + jasmine.clock().tick(4000); + + expect(this.model.fetch.calls.count()).toBe(1); + }); + + it('should trigger a change event if an error occurs', function() { + this.model.fetch = originalFetch; + spyOn(this.model, 'fetch'); + + var onErrorCallback = jasmine.createSpy('error'); + this.model.bind('change', onErrorCallback); + + this.poller.start(); + + jasmine.clock().tick(2001); + + this.model.fetch.calls.mostRecent().args[0].error(); + + expect(onErrorCallback).toHaveBeenCalled(); + }); +}); \ No newline at end of file diff --git a/lib/assets/test/spec/cartodb/common/background_importer/import_model_poller.spec.js b/lib/assets/test/spec/cartodb/common/background_importer/import_model_poller.spec.js new file mode 100644 index 000000000000..10546e31754b --- /dev/null +++ b/lib/assets/test/spec/cartodb/common/background_importer/import_model_poller.spec.js @@ -0,0 +1,64 @@ +var ImportModelPoller = require('../../../../../javascripts/cartodb/common/background_polling/models/import_model_poller'); + +describe('ImportModelPoller', function() { + + var originalFetch; + + beforeEach(function() { + + var Model = Backbone.Model.extend({ + url: '/hello' + }); + this.model = new Model(); + this.model.sync = function(a,b,opts) { + opts.success(); + }; + originalFetch = this.model.fetch; + spyOn(this.model, 'fetch').and.callThrough(); + + this.poller = new ImportModelPoller(this.model); + + jasmine.clock().install(); + }); + + afterEach(function() { + jasmine.clock().uninstall(); + }); + + it('should stop polling when geocoding has failed', function() { + this.model.set('state', 'failure'); + + this.poller.start(); + + jasmine.clock().tick(4000); + + expect(this.model.fetch.calls.count()).toBe(1); + }); + + it('should stop polling when geocoding has completed', function() { + this.model.set('state', 'complete'); + + this.poller.start(); + + jasmine.clock().tick(4000); + + expect(this.model.fetch.calls.count()).toBe(1); + }); + + it('should increment the interval after a number of requests', function() { + this.poller.start(); + + // 2000 is the interval period and 30 the number of requests before + // the interval is incremented. + jasmine.clock().tick(2000 * 30); + + // 30 requests have been made + expect(this.model.fetch.calls.count()).toBe(30); + + // Interval has been incremented to 2000 * 2.5 + jasmine.clock().tick(2000 * 2.5 + 1); + + // Only one more request has been made (interval has been incremented) + expect(this.model.fetch.calls.count()).toBe(31); + }); +}); \ No newline at end of file diff --git a/lib/assets/test/spec/cartodb/common/background_importer/imports_model.spec.js b/lib/assets/test/spec/cartodb/common/background_importer/imports_model.spec.js index bc1fbd379e58..7000b9b9ee1d 100644 --- a/lib/assets/test/spec/cartodb/common/background_importer/imports_model.spec.js +++ b/lib/assets/test/spec/cartodb/common/background_importer/imports_model.spec.js @@ -219,58 +219,5 @@ describe('common/background_polling/imports_model', function() { this.imp.set('item_queue_id', 'vamos-neno'); expect(this.imp.pollCheck).toHaveBeenCalled(); }); - - describe('polling', function() { - - beforeEach(function() { - jasmine.clock().install(); - this.model = this.imp; - this.model.set('id', '1'); - this.model.sync = function(a,b,opts) { - opts.success(); - }; - - spyOn(this.model, 'fetch').and.callThrough(); - }); - - afterEach(function() { - jasmine.clock().uninstall(); - }); - - it('should stop polling when geocoding has failed', function() { - this.model.set('state', 'failure'); - this.model.pollCheck(); - - jasmine.clock().tick(4000); - - expect(this.model.fetch.calls.count()).toBe(1); - }); - - it('should stop polling when geocoding has completed', function() { - this.model.set('state', 'complete'); - this.model.pollCheck(); - - jasmine.clock().tick(4000); - - expect(this.model.fetch.calls.count()).toBe(1); - }); - - it('should increment the interval after a number of requests', function() { - this.model.pollCheck(); - - // 2000 is the interval period and 30 the number of requests before - // the interval is incremented. - jasmine.clock().tick(2000 * 30); - - // 30 requests have been made - expect(this.model.fetch.calls.count()).toBe(30); - - // Interval has been incremented to 2000 * 2.5 - jasmine.clock().tick(2000 * 2.5 + 1); - - // Only one more request has been made (interval has been incremented) - expect(this.model.fetch.calls.count()).toBe(31); - }); - }); }); }); From ecc6e00d88f7a00a1fae3b3d364e7fc77c0f1131 Mon Sep 17 00:00:00 2001 From: Pablo Alonso Garcia Date: Wed, 12 Aug 2015 13:11:27 +0200 Subject: [PATCH 16/20] Error callback receives the model as a parameter --- .../models/geocoding_model_poller.js | 4 ++-- .../background_polling/models/poller.js | 2 +- .../common/background_importer/poller.spec.js | 24 +++++++++++++++++-- 3 files changed, 25 insertions(+), 5 deletions(-) diff --git a/lib/assets/javascripts/cartodb/common/background_polling/models/geocoding_model_poller.js b/lib/assets/javascripts/cartodb/common/background_polling/models/geocoding_model_poller.js index 157899f56921..819fbba80abb 100644 --- a/lib/assets/javascripts/cartodb/common/background_polling/models/geocoding_model_poller.js +++ b/lib/assets/javascripts/cartodb/common/background_polling/models/geocoding_model_poller.js @@ -9,8 +9,8 @@ var GeocodingModelPoller = function(model) { condition: function(model) { return model.hasFailed() || model.hasCompleted(); }, - error: function() { - this.model.trigger("change"); + error: function(model) { + model.trigger("change"); } }; diff --git a/lib/assets/javascripts/cartodb/common/background_polling/models/poller.js b/lib/assets/javascripts/cartodb/common/background_polling/models/poller.js index cedfb0d8ef40..bb1b24a0f5b8 100644 --- a/lib/assets/javascripts/cartodb/common/background_polling/models/poller.js +++ b/lib/assets/javascripts/cartodb/common/background_polling/models/poller.js @@ -61,7 +61,7 @@ Poller.prototype._fetch = function() { } }, error: function(e) { - self.error && self.error(); + self.error && self.error(self.model); } }) } diff --git a/lib/assets/test/spec/cartodb/common/background_importer/poller.spec.js b/lib/assets/test/spec/cartodb/common/background_importer/poller.spec.js index 289b24c9043c..5f395ea06552 100644 --- a/lib/assets/test/spec/cartodb/common/background_importer/poller.spec.js +++ b/lib/assets/test/spec/cartodb/common/background_importer/poller.spec.js @@ -2,7 +2,7 @@ var Poller = require('../../../../../javascripts/cartodb/common/background_polli describe('Poller', function() { - var model, originalSync; + var model, originalSync, originalFetch; beforeEach(function() { var Model = Backbone.Model.extend({ @@ -15,8 +15,8 @@ describe('Poller', function() { }; jasmine.clock().install(); + originalFetch = model.fetch; spyOn(model, 'fetch').and.callThrough(); - }); afterEach(function() { @@ -145,6 +145,26 @@ describe('Poller', function() { expect(model.fetch.calls.count()).toEqual(2); }) + + it('should invoke the error callback if the request fails', function() { + model.fetch = originalFetch; + spyOn(model, 'fetch'); + + var errorCallback = jasmine.createSpy('error'); + + var poller = new Poller(model, { + interval: 100, + error: errorCallback + }); + + poller.start(); + + jasmine.clock().tick(100); + + model.fetch.calls.mostRecent().args[0].error(); + + expect(errorCallback).toHaveBeenCalledWith(model); + }) }); describe('.stop', function() { From c92bee4879aba383b81e8805cc383d4f79a08f73 Mon Sep 17 00:00:00 2001 From: Pablo Alonso Garcia Date: Wed, 12 Aug 2015 13:42:13 +0200 Subject: [PATCH 17/20] Whitespace --- .../common/background_importer/geocoding_model_poller.spec.js | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/assets/test/spec/cartodb/common/background_importer/geocoding_model_poller.spec.js b/lib/assets/test/spec/cartodb/common/background_importer/geocoding_model_poller.spec.js index d3c8097128f7..ac33e569746a 100644 --- a/lib/assets/test/spec/cartodb/common/background_importer/geocoding_model_poller.spec.js +++ b/lib/assets/test/spec/cartodb/common/background_importer/geocoding_model_poller.spec.js @@ -1,4 +1,3 @@ - var GeocodingModelPoller = require('../../../../../javascripts/cartodb/common/background_polling/models/geocoding_model_poller'); describe('GeocodingModelPoller', function() { From e23a4a33425890a0b53094717547174e61a8ab67 Mon Sep 17 00:00:00 2001 From: Pablo Alonso Garcia Date: Wed, 12 Aug 2015 13:49:20 +0200 Subject: [PATCH 18/20] Tried to make condition a bit more clear --- .../cartodb/common/background_polling/models/poller.js | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/lib/assets/javascripts/cartodb/common/background_polling/models/poller.js b/lib/assets/javascripts/cartodb/common/background_polling/models/poller.js index bb1b24a0f5b8..a9d1a87810fd 100644 --- a/lib/assets/javascripts/cartodb/common/background_polling/models/poller.js +++ b/lib/assets/javascripts/cartodb/common/background_polling/models/poller.js @@ -56,17 +56,22 @@ Poller.prototype._fetch = function() { success: function() { self.polling = false; self.numberOfRequests++; - if (!self.condition || (self.condition && !self.condition(self.model))) { + if (self._continuePolling()) { self._scheduleFetch(); } }, error: function(e) { - self.error && self.error(self.model); + _.isFunction(self.error) && self.error(self.model); } }) } } +Poller.prototype._continuePolling = function() { + return !this.condition || + (_.isFunction(this.condition) && !this.condition(this.model)); +} + Poller.prototype.stop = function() { this.polling = false; clearTimeout(this.timeout); From e077782dcef07c9028bc1b672c9b4d7f0a93f7a0 Mon Sep 17 00:00:00 2001 From: Pablo Alonso Garcia Date: Wed, 12 Aug 2015 15:20:58 +0200 Subject: [PATCH 19/20] Renamed "condition" option to "stopWhen" --- .../background_polling/models/geocoding_model_poller.js | 2 +- .../background_polling/models/import_model_poller.js | 2 +- .../cartodb/common/background_polling/models/poller.js | 8 ++++---- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/lib/assets/javascripts/cartodb/common/background_polling/models/geocoding_model_poller.js b/lib/assets/javascripts/cartodb/common/background_polling/models/geocoding_model_poller.js index 819fbba80abb..5e9f2e26a796 100644 --- a/lib/assets/javascripts/cartodb/common/background_polling/models/geocoding_model_poller.js +++ b/lib/assets/javascripts/cartodb/common/background_polling/models/geocoding_model_poller.js @@ -6,7 +6,7 @@ var GeocodingModelPoller = function(model) { var options = { interval: POLLING_INTERVAL, - condition: function(model) { + stopWhen: function(model) { return model.hasFailed() || model.hasCompleted(); }, error: function(model) { diff --git a/lib/assets/javascripts/cartodb/common/background_polling/models/import_model_poller.js b/lib/assets/javascripts/cartodb/common/background_polling/models/import_model_poller.js index 7b628f83796e..4f1962224b8e 100644 --- a/lib/assets/javascripts/cartodb/common/background_polling/models/import_model_poller.js +++ b/lib/assets/javascripts/cartodb/common/background_polling/models/import_model_poller.js @@ -13,7 +13,7 @@ var ImportModelPoller = function(model) { } return POLLING_INTERVAL; }, - condition: function(model) { + stopWhen: function(model) { var state = model.get('state'); return (state === "complete" || state === "failure"); } diff --git a/lib/assets/javascripts/cartodb/common/background_polling/models/poller.js b/lib/assets/javascripts/cartodb/common/background_polling/models/poller.js index a9d1a87810fd..60b097379535 100644 --- a/lib/assets/javascripts/cartodb/common/background_polling/models/poller.js +++ b/lib/assets/javascripts/cartodb/common/background_polling/models/poller.js @@ -7,7 +7,7 @@ * * var poller = new Poller(model, { * interval: 1000, - * condition: function(model) { + * stopWhen: function(model) { * return model.get('state') === 'completed'; * } * }); @@ -27,7 +27,7 @@ var Poller = function(model, options) { if (typeof this.interval !== "function") { this.interval = function() { return options['interval']; }; } - this.condition = options['condition']; + this.stopWhen = options['stopWhen']; this.error = options['error']; this.autoStart = options['autoStart']; @@ -68,8 +68,8 @@ Poller.prototype._fetch = function() { } Poller.prototype._continuePolling = function() { - return !this.condition || - (_.isFunction(this.condition) && !this.condition(this.model)); + return !this.stopWhen || + (_.isFunction(this.stopWhen) && !this.stopWhen(this.model)); } Poller.prototype.stop = function() { From 22d89804c395b6d8e58c8b26c762a61c4696c737 Mon Sep 17 00:00:00 2001 From: Pablo Alonso Garcia Date: Wed, 12 Aug 2015 15:30:46 +0200 Subject: [PATCH 20/20] Fixed test --- .../test/spec/cartodb/common/background_importer/poller.spec.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/assets/test/spec/cartodb/common/background_importer/poller.spec.js b/lib/assets/test/spec/cartodb/common/background_importer/poller.spec.js index 5f395ea06552..c563f57add5f 100644 --- a/lib/assets/test/spec/cartodb/common/background_importer/poller.spec.js +++ b/lib/assets/test/spec/cartodb/common/background_importer/poller.spec.js @@ -96,7 +96,7 @@ describe('Poller', function() { var poller = new Poller(model, { interval: 100, - condition: function(model) { + stopWhen: function(model) { return model.get('something') === 'wadus'; } });