Merge pull request #5182 from epixa/4342-fixing-master

Fix timing issue when fetching pre-flighted requests

src/plugins/kibana/public/discover/controllers/discover.js

@@ -217,7 +217,13 @@ define(function (require) {
             if (rows == null && oldRows == null) return status.LOADING;
 
             var rowsEmpty = _.isEmpty(rows);
-            if (rowsEmpty && fetchStatus) return status.LOADING;
+            // An undefined fetchStatus means the requests are still being
+            // prepared to be sent. When all requests are completed,
+            // fetchStatus is set to null, so it's important that we
+            // specifically check for undefined to determine a loading status.
+            var preparingForFetch = _.isUndefined(fetchStatus);
+            if (preparingForFetch) return status.LOADING;
+            else if (rowsEmpty && fetchStatus) return status.LOADING;
             else if (!rowsEmpty) return status.READY;
             else return status.NO_RESULTS;
           }

src/ui/public/courier/fetch/__tests__/fetch_these.js

@@ -0,0 +1,84 @@
+describe('ui/courier/fetch/_fetch_these', () => {
+  const _ = require('lodash');
+  const sinon = require('auto-release-sinon');
+  const expect = require('expect.js');
+  const ngMock = require('ngMock');
+
+  let Promise;
+  let $rootScope;
+  let fetchThese;
+  let request;
+  let requests;
+  let fakeResponses;
+
+  beforeEach(ngMock.module('kibana', (PrivateProvider) => {
+    function FakeResponsesProvider(Promise) {
+      fakeResponses = sinon.spy(function () {
+        return Promise.map(requests, mockRequest => {
+          return { mockRequest };
+        });
+      });
+      return fakeResponses;
+    }
+
+    PrivateProvider.swap(require('ui/courier/fetch/_call_client'), FakeResponsesProvider);
+    PrivateProvider.swap(require('ui/courier/fetch/_call_response_handlers'), FakeResponsesProvider);
+    PrivateProvider.swap(require('ui/courier/fetch/_continue_incomplete'), FakeResponsesProvider);
+  }));
+
+  beforeEach(ngMock.inject((Private, $injector) => {
+    $rootScope = $injector.get('$rootScope');
+    Promise = $injector.get('Promise');
+    fetchThese = Private(require('ui/courier/fetch/_fetch_these'));
+    request = mockRequest();
+    requests = [ request ];
+  }));
+
+  context('when request has not started', () => {
+    beforeEach(() => requests.forEach(req => req.started = false));
+
+    it('starts request', () => {
+      fetchThese(requests);
+      expect(request.start.called).to.be(true);
+      expect(request.continue.called).to.be(false);
+    });
+
+    it('waits for returned promise from start() to be fulfilled', () => {
+      request.start = sinon.stub().returns(Promise.resolve(request));
+      fetchThese(requests);
+
+      expect(request.start.callCount).to.be(1);
+      expect(fakeResponses.callCount).to.be(0);
+      $rootScope.$apply();
+      expect(fakeResponses.callCount).to.be(3);
+    });
+  });
+
+  context('when request has already started', () => {
+    it('continues request', () => {
+      fetchThese(requests);
+      expect(request.start.called).to.be(false);
+      expect(request.continue.called).to.be(true);
+    });
+    it('waits for returned promise to be fulfilled', () => {
+      request.continue = sinon.stub().returns(Promise.resolve(request));
+      fetchThese(requests);
+
+      expect(request.continue.callCount).to.be(1);
+      expect(fakeResponses.callCount).to.be(0);
+      $rootScope.$apply();
+      expect(fakeResponses.callCount).to.be(3);
+    });
+  });
+
+  function mockRequest() {
+    return {
+      strategy: 'mock',
+      started: true,
+      aborted: false,
+      retry: sinon.spy(function () { return this; }),
+      continue: sinon.spy(function () { return this; }),
+      start: sinon.spy(function () { return this; })
+    };
+  }
+});

src/ui/public/courier/fetch/_fetch_these.js

@@ -25,20 +25,10 @@ define(function (require) {
     function fetchWithStrategy(strategy, requests) {
 
       requests = requests.map(function (req) {
-        if (req.aborted) {
-          return ABORTED;
-        }
-
-        if (req.started) {
-          req.continue();
-        } else {
-          req.start();
-        }
-
-        return req;
+        return req.aborted ? ABORTED : req;
       });
 
-      return Promise.resolve()
+      return startRequests(requests)
       .then(function () {
         return callClient(strategy, requests);
       })
@@ -63,6 +53,19 @@ define(function (require) {
       });
     }
 
+    function startRequests(requests) {
+      return Promise.map(requests, function (req) {
+        if (req === ABORTED) {
+          return req;
+        }
+
+        return new Promise(function (resolve) {
+          var action = req.started ? req.continue : req.start;
+          resolve(action.call(req));
+        });
+      });
+    }
+
     return fetchThese;
   };
 });

src/ui/public/courier/fetch/request/__tests__/segmented.js

@@ -0,0 +1,55 @@
+describe('ui/courier/fetch/request/segmented', () => {
+  const sinon = require('auto-release-sinon');
+  const expect = require('expect.js');
+  const ngMock = require('ngMock');
+
+  let Promise;
+  let $rootScope;
+  let SegmentedReq;
+  let segmentedReq;
+  let searchReqStart;
+
+  beforeEach(ngMock.module('kibana'));
+
+  beforeEach(ngMock.inject((Private, $injector) => {
+    Promise = $injector.get('Promise');
+    $rootScope = $injector.get('$rootScope');
+    SegmentedReq = Private(require('ui/courier/fetch/request/segmented'));
+    searchReqStart = sinon.spy(Private(require('ui/courier/fetch/request/search')).prototype, 'start');
+  }));
+
+  describe('#start()', () => {
+    let returned;
+    beforeEach(() => {
+      init();
+      returned = segmentedReq.start();
+    });
+
+    it('returns promise', () => {
+      expect(returned.then).to.be.Function;
+    });
+
+    it('does not call super.start() until promise is resolved', () => {
+      expect(searchReqStart.called).to.be(false);
+      $rootScope.$apply();
+      expect(searchReqStart.called).to.be(true);
+    });
+  });
+
+  function init() {
+    segmentedReq = new SegmentedReq(mockSource());
+  }
+
+  function mockSource() {
+    return {
+      get: sinon.stub().returns(mockIndexPattern())
+    };
+  }
+
+  function mockIndexPattern() {
+    const queue = [1, 2, 3];
+    return {
+      toIndexList: sinon.stub().returns(Promise.resolve(queue))
+    };
+  }
+});

src/ui/public/courier/fetch/request/segmented.js

@@ -54,14 +54,14 @@ define(function (require) {
       // give the request consumer a chance to receive each segment and set
       // parameters via the handle
       if (_.isFunction(this._initFn)) this._initFn(this._handle);
-      this._createQueue().then(function (queue) {
+      return this._createQueue().then(function (queue) {
         self._all = queue.slice(0);
 
         // Send the initial fetch status
         self._reportStatus();
-      });
 
-      return SearchReq.prototype.start.call(this);
+        return SearchReq.prototype.start.call(self);
+      });
     };
 
     SegmentedReq.prototype.continue = function () {