[courier/request] move source mutation logic out of request

src/fixtures/stubbed_search_source.js

@@ -38,6 +38,13 @@ export default function stubSearchSource(Private, $q, Promise) {
     onError: function () { return $q.defer().promise; },
     _flatten: function () {
       return Promise.resolve({ index: indexPattern, body: {} });
+    },
+    _requestStartHandlers: [],
+    onRequestStart(fn) {
+      this._requestStartHandlers.push(fn);
+    },
+    requestIsStarting(req) {
+      return Promise.map(this._requestStartHandlers, fn => fn(req));
     }
   };
 

src/ui/public/courier/data_source/_abstract.js

@@ -238,12 +238,41 @@ export function AbstractDataSourceProvider(Private, Promise, PromiseEmitter, con
    */
   SourceAbstract.prototype.destroy = function () {
     this.cancelQueued();
+    this._requestStartHandlers.length = 0;
   };
 
+  /**
+   *  Add a handler that will be notified whenever requests start
+   *  @param  {Function} handler
+   *  @return {undefined}
+   */
   SourceAbstract.prototype.onRequestStart = function (handler) {
     this._requestStartHandlers.push(handler);
   };
 
+  /**
+   *  Called by requests of this search source when they are started
+   *  @param  {Courier.Request} request
+   *  @return {Promise<undefined>}
+   */
+  SourceAbstract.prototype.requestIsStarting = function (request) {
+    this.activeFetchCount = (this.activeFetchCount || 0) + 1;
+    this.history = [request];
+
+    return Promise
+      .map(this._requestStartHandlers, fn => fn(this, request))
+      .then(_.noop);
+  };
+
+  /**
+   *  Called by requests of this search source when they are done
+   *  @param  {Courier.Request} request
+   *  @return {Promise<undefined>}
+   */
+  SourceAbstract.prototype.requestIsStopped = function (/* request */) {
+    this.activeFetchCount -= 1;
+  };
+
   /*****
    * PRIVATE API
    *****/
@@ -258,12 +287,6 @@ export function AbstractDataSourceProvider(Private, Promise, PromiseEmitter, con
     throw new Error('_createRequest must be implemented by subclass');
   };
 
-  SourceAbstract.prototype._triggerRequestStart = function (request) {
-    return Promise.map(this._requestStartHandlers, fn => (
-      fn(this, request)
-    ));
-  };
-
   /**
    * Walk the inheritance chain of a source and return it's
    * flat representaion (taking into account merging rules)

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

@@ -0,0 +1,27 @@
+import ngMock from 'ng_mock';
+import sinon from 'sinon';
+
+import { AbstractRequestProvider } from '../request';
+import { requestQueue } from '../../../_request_queue';
+
+describe('courier/fetch abstract request', () => {
+  beforeEach(ngMock.module('kibana'));
+
+  afterEach(() => {
+    requestQueue.clear();
+  });
+
+  describe('#start()', () => {
+    it('calls this.source.requestIsStarting(request)', ngMock.inject((Private) => {
+      const AbstractReq = Private(AbstractRequestProvider);
+
+      const spy = sinon.spy(() => Promise.resolve());
+      const source = { requestIsStarting: spy };
+
+      const req = new AbstractReq(source);
+      expect(req.start()).to.have.property('then').a('function');
+      sinon.assert.calledOnce(spy);
+      sinon.assert.calledWithExactly(spy, req);
+    }));
+  });
+});

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

@@ -0,0 +1,27 @@
+import ngMock from 'ng_mock';
+import sinon from 'sinon';
+
+import { SearchRequestProvider } from '../search';
+import { requestQueue } from '../../../_request_queue';
+
+describe('ui/courier/fetch search request', () => {
+  beforeEach(ngMock.module('kibana'));
+
+  afterEach(() => {
+    requestQueue.clear();
+  });
+
+  describe('#start()', () => {
+    it('calls this.source.requestIsStarting(request)', ngMock.inject((Private) => {
+      const SearchReq = Private(SearchRequestProvider);
+
+      const spy = sinon.spy(() => Promise.resolve());
+      const source = { requestIsStarting: spy };
+
+      const req = new SearchReq(source);
+      expect(req.start()).to.have.property('then').a('function');
+      sinon.assert.calledOnce(spy);
+      sinon.assert.calledWithExactly(spy, req);
+    }));
+  });
+});

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

@@ -3,20 +3,26 @@ import expect from 'expect.js';
 import ngMock from 'ng_mock';
 
 import { SegmentedRequestProvider } from '../segmented';
-import { SearchRequestProvider } from '../search';
+import { AbstractRequestProvider } from '../request';
 
 describe('SegmentedRequestProvider', () => {
   let Promise;
   let SegmentedReq;
   let segmentedReq;
-  let searchReqStart;
+  let abstractReqStart;
 
   beforeEach(ngMock.module('kibana'));
 
   beforeEach(ngMock.inject((Private, $injector) => {
     Promise = $injector.get('Promise');
     SegmentedReq = Private(SegmentedRequestProvider);
-    searchReqStart = sinon.spy(Private(SearchRequestProvider).prototype, 'start');
+
+    const AbstractReq = Private(AbstractRequestProvider);
+    abstractReqStart = sinon.stub(AbstractReq.prototype, 'start', () => {
+      const promise = Promise.resolve();
+      sinon.spy(promise, 'then');
+      return promise;
+    });
   }));
 
   describe('#start()', () => {
@@ -30,8 +36,14 @@ describe('SegmentedRequestProvider', () => {
       expect(returned.then).to.be.Function;
     });
 
-    it('calls super.start() synchronously', () => {
-      expect(searchReqStart.called).to.be(true);
+    it('calls AbstractReq#start()', () => {
+      sinon.assert.calledOnce(abstractReqStart);
+    });
+
+    it('listens to promise from super.start()', () => {
+      sinon.assert.calledOnce(abstractReqStart);
+      const promise = abstractReqStart.firstCall.returnValue;
+      sinon.assert.calledOnce(promise.then);
     });
   });
 
@@ -41,7 +53,7 @@ describe('SegmentedRequestProvider', () => {
 
   function mockSource() {
     return {
-      get: sinon.stub().returns(mockIndexPattern())
+      get: sinon.stub().returns(mockIndexPattern()),
     };
   }
 

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

@@ -63,16 +63,7 @@ export function AbstractRequestProvider(Private, Promise) {
       this.started = true;
       this.moment = moment();
 
-      const source = this.source;
-      if (source.activeFetchCount) {
-        source.activeFetchCount += 1;
-      } else {
-        source.activeFetchCount = 1;
-      }
-
-      source.history = [this];
-
-      return this.source._triggerRequestStart(this);
+      return this.source.requestIsStarting(this);
     }
 
     getFetchParams() {
@@ -115,7 +106,7 @@ export function AbstractRequestProvider(Private, Promise) {
     _markStopped() {
       if (this.stopped) return;
       this.stopped = true;
-      this.source.activeFetchCount -= 1;
+      this.source.requestIsStopped(this);
       _.pull(requestQueue, this);
     }