Fix the mapping of indexes to body for msearch requests (#10913)

* Fix the mapping of indexes to body for msearch requests

Happened when one vis had no data in any indexes.  Pushing the index on
indexList caused subsequent checks to seem like they *did* have data.

* address code comments

src/ui/public/courier/fetch/strategy/__tests__/search.js

@@ -3,17 +3,21 @@ import expect from 'expect.js';
 import ngMock from 'ng_mock';
 
 import SearchStrategyProvider from '../search';
+import StubIndexPatternProvider from 'test_utils/stub_index_pattern';
 
 describe('ui/courier/fetch/strategy/search', () => {
-  let $rootScope;
+  require('test_utils/no_digest_promises').activateForSuite();
+
   let search;
   let reqsFetchParams;
+  let IndexPattern;
 
   beforeEach(ngMock.module('kibana'));
 
-  beforeEach(ngMock.inject((Private, $injector) => {
-    $rootScope = $injector.get('$rootScope');
+  beforeEach(ngMock.inject((Private) => {
     search = Private(SearchStrategyProvider);
+    IndexPattern = Private(StubIndexPatternProvider);
+
     reqsFetchParams = [
       {
         index: ['logstash-123'],
@@ -32,40 +36,74 @@ describe('ui/courier/fetch/strategy/search', () => {
 
   describe('#reqsFetchParamsToBody()', () => {
     it('filters out any body properties that begin with $', () => {
-      let value;
-      search.reqsFetchParamsToBody(reqsFetchParams).then(val => value = val);
-      $rootScope.$apply();
-      expect(_.includes(value, 'foo')).to.be(true);
-      expect(_.includes(value, '$foo')).to.be(false);
+      return search.reqsFetchParamsToBody(reqsFetchParams).then(value => {
+        expect(_.includes(value, 'foo')).to.be(true);
+        expect(_.includes(value, '$foo')).to.be(false);
+      });
     });
 
     context('when indexList is not empty', () => {
       it('includes the index', () => {
-        let value;
-        search.reqsFetchParamsToBody(reqsFetchParams).then(val => value = val);
-        $rootScope.$apply();
-        expect(_.includes(value, '"index":["logstash-123"]')).to.be(true);
+        return search.reqsFetchParamsToBody(reqsFetchParams).then(value => {
+          expect(_.includes(value, '"index":["logstash-123"]')).to.be(true);
+        });
       });
     });
 
     context('when indexList is empty', () => {
-      beforeEach(() => reqsFetchParams[0].index = []);
+      beforeEach(() => {
+        reqsFetchParams.forEach(request => request.index = []);
+      });
+      const emptyMustNotQuery = JSON.stringify({
+        query: {
+          bool: {
+            must_not: [
+              { match_all: {} }
+            ]
+          }
+        }
+      });
 
       it('queries the kibana index (.kibana) with a must_not match_all boolean', () => {
-        const query = JSON.stringify({
-          query: {
-            bool: {
-              must_not: [
-                { match_all: {} }
-              ]
-            }
-          }
+        return search.reqsFetchParamsToBody(reqsFetchParams).then(value => {
+          expect(_.includes(value, '"index":[".kibana"]')).to.be(true);
+          expect(_.includes(value, emptyMustNotQuery)).to.be(true);
+        });
+      });
+    });
+
+    context('when passed IndexPatterns', () => {
+      it(' that are out of range, queries .kibana', () => {
+        // Check out https://github.com/elastic/kibana/issues/10905 for the reasons behind this
+        // test. When an IndexPattern is out of time range, it returns an array that is then stored in a cache.  This
+        // cached object was being modified in a following function, which was a subtle side affect - it looked like
+        // only a local change.
+        const indexPattern = new IndexPattern('logstash-*', null, []);
+        // Stub the call so it looks like the request returns an empty list, which will happen if the time range
+        // selected doesn't contain any data for the particular index.
+        indexPattern.toIndexList = () => Promise.resolve([]);
+        const reqsFetchParams = [
+          {
+            index: indexPattern,
+            type: 'planet',
+            search_type: 'water',
+            body: { foo: 'earth' }
+          },
+          {
+            index: indexPattern,
+            type: 'planet',
+            search_type: 'rings',
+            body: { foo: 'saturn' }
+          }
+        ];
+        return search.reqsFetchParamsToBody(reqsFetchParams).then(value => {
+          const indexLineMatch = value.match(/"index":\[".kibana"\]/g);
+          expect(indexLineMatch).to.not.be(null);
+          expect(indexLineMatch.length).to.be(2);
+          const queryLineMatch = value.match(/"query":\{"bool":\{"must_not":\[\{"match_all":\{\}\}\]\}\}/g);
+          expect(queryLineMatch).to.not.be(null);
+          expect(queryLineMatch.length).to.be(2);
         });
-        let value;
-        search.reqsFetchParamsToBody(reqsFetchParams).then(val => value = val);
-        $rootScope.$apply();
-        expect(_.includes(value, '"index":[".kibana"]')).to.be(true);
-        expect(_.includes(value, query)).to.be(true);
       });
     });
   });

src/ui/public/courier/fetch/strategy/search.js

@@ -11,7 +11,7 @@ export default function FetchStrategyForSearch(Private, Promise, timefilter, kbn
     /**
      * Flatten a series of requests into as ES request body
      *
-     * @param  {array} requests - the requests to serialize
+     * @param  {array} reqsFetchParams - the requests to serialize
      * @return {Promise} - a promise that is fulfilled by the request body
      */
     reqsFetchParamsToBody: function (reqsFetchParams) {
@@ -29,10 +29,14 @@ export default function FetchStrategyForSearch(Private, Promise, timefilter, kbn
           if (!indexToListMapping[indexList.id]) {
             indexToListMapping[indexList.id] = indexList.toIndexList(timeBounds.min, timeBounds.max);
           }
-          return indexToListMapping[indexList.id];
+          return indexToListMapping[indexList.id].then(indexList => {
+            // Make sure the index list in the cache can't be subsequently updated.
+            return _.clone(indexList);
+          });
         })
         .then(function (indexList) {
           let body = fetchParams.body || {};
+          let index = [];
           // If we've reached this point and there are no indexes in the
           // index list at all, it means that we shouldn't expect any indexes
           // to contain the documents we're looking for, so we instead
@@ -42,11 +46,13 @@ export default function FetchStrategyForSearch(Private, Promise, timefilter, kbn
           // handle that request by querying *all* indexes, which is the
           // opposite of what we want in this case.
           if (_.isArray(indexList) && indexList.length === 0) {
-            indexList.push(kbnIndex);
+            index.push(kbnIndex);
             body = emptySearch();
+          } else {
+            index = indexList;
           }
           return angular.toJson({
-            index: indexList,
+            index,
             type: fetchParams.type,
             search_type: fetchParams.search_type,
             ignore_unavailable: true,