Merge pull request #3795 from spalger/fix/3654

[courier] don't restart doc requests unless necessary

src/kibana/components/courier/fetch/request/doc.js

@@ -17,18 +17,14 @@ define(function (require) {
       var parent = DocRequest.Super.prototype.canStart.call(this);
       if (!parent) return false;
 
-      // _getStoredVersion updates the internal
-      // cache returned by _getVersion, so _getVersion
-      // must be called first
-      var version = this.source._getVersion();
+      var version = this.source._version;
       var storedVersion = this.source._getStoredVersion();
 
       // conditions that equal "fetch This DOC!"
-      var unknownVersion = !version && !storedVersion;
-      var versionMismatch = version !== storedVersion;
-      var localVersionCleared = version && !storedVersion;
+      var unkown = !version && !storedVersion;
+      var mismatch = version !== storedVersion;
 
-      if (unknownVersion || versionMismatch || localVersionCleared) return true;
+      return Boolean(mismatch || (unkown && !this.started));
     };
 
     DocRequest.prototype.handleResponse = function (resp) {
@@ -43,4 +39,4 @@ define(function (require) {
 
     return DocRequest;
   };
-});
+});

src/kibana/components/courier/fetch/request/request.js

@@ -18,7 +18,7 @@ define(function (require) {
     }
 
     AbstractReq.prototype.canStart = function () {
-      return !this.stopped && !this.source._fetchDisabled;
+      return Boolean(!this.stopped && !this.source._fetchDisabled);
     };
 
     AbstractReq.prototype.start = function () {
@@ -115,4 +115,4 @@ define(function (require) {
 
     return AbstractReq;
   };
-});
+});

test/unit/specs/courier/fetch/doc.js

@@ -0,0 +1,105 @@
+define(function (require) {
+  describe('Courier DocFetchRequest class', function () {
+    var sinon = require('test_utils/auto_release_sinon');
+
+    var storage;
+    var source;
+    var defer;
+    var req;
+
+    var setVersion;
+
+    beforeEach(module('kibana'));
+    beforeEach(inject(function (Private, Promise, $injector) {
+      var DocSource = Private(require('components/courier/data_source/doc_source'));
+      var DocFetchRequest = Private(require('components/courier/fetch/request/doc'));
+
+      storage =
+      $injector.get('localStorage').store =
+      $injector.get('sessionStorage').store = {
+        getItem: sinon.stub(),
+        setItem: sinon.stub(),
+        removeItem: sinon.stub(),
+        clear: sinon.stub()
+      };
+
+      source = new DocSource({})
+      .set('index', 'doc-index')
+      .set('type', 'doc-type')
+      .set('id', 'doc-id');
+
+      defer = Promise.defer();
+
+      req = new DocFetchRequest(source, defer);
+
+      /**
+       * Setup the version numbers for tests. There are two versions for the
+       * purposes of these tests.
+       *
+       * @param {number} mine - the version that the DocSource most
+       *                      recently received from elasticsearch.
+       * @param {number} theirs - the version that other DocSources have
+       *                        received from elasticsearfch.
+       */
+      setVersion = function (mine, theirs) {
+        // mine – the version the source think is accurate
+        // theirs - the version other sources think is accurate
+
+        source._version = mine;
+        storage.getItem.withArgs(source._versionKey()).returns(theirs);
+      };
+    }));
+
+    describe('#canStart', function () {
+      it('can if the doc is unknown', function () {
+        setVersion(undefined, undefined);
+
+        expect(req.canStart()).to.be(true);
+      });
+
+      it('cannot if the doc is unknown but the request is already in progress', function () {
+        setVersion(undefined, undefined);
+        req.start();
+
+        expect(req.canStart()).to.be(false);
+      });
+
+      it('can if the doc is out of date', function () {
+        setVersion(1, 2);
+
+        expect(req.canStart()).to.be(true);
+      });
+
+      it('can if the doc is out of date and the request is in progress', function () {
+        setVersion(1, 2);
+        req.start();
+
+        expect(req.canStart()).to.be(true);
+      });
+
+      it('cannot if the doc is up to date', function () {
+        setVersion(2, 2);
+
+        expect(req.canStart()).to.be(false);
+      });
+
+      it('can if the doc is overdated', function () {
+        setVersion(5, 2);
+
+        expect(req.canStart()).to.be(true);
+      });
+
+      it('can if shared version is cleared', function () {
+        setVersion(10, undefined);
+
+        expect(req.canStart()).to.be(true);
+      });
+
+      it('can if everyone else has a doc', function () {
+        setVersion(undefined, 10);
+
+        expect(req.canStart()).to.be(true);
+      });
+    });
+  });
+});