From ead1d1eb63e5f2f7895b8e0e4c59acd00beeaaae Mon Sep 17 00:00:00 2001 From: Tim Roes Date: Mon, 16 Apr 2018 10:25:27 +0200 Subject: [PATCH 1/3] Fix issue with overwritten filters --- .../courier/data_source/search_source.js | 17 +++++++++-- src/ui/public/vis/request_handlers/courier.js | 29 ++++++++++++++++--- 2 files changed, 40 insertions(+), 6 deletions(-) diff --git a/src/ui/public/courier/data_source/search_source.js b/src/ui/public/courier/data_source/search_source.js index 774891622a3e5..b95d38424a29c 100644 --- a/src/ui/public/courier/data_source/search_source.js +++ b/src/ui/public/courier/data_source/search_source.js @@ -134,6 +134,7 @@ export function SearchSourceProvider(Promise, PromiseEmitter, Private, config) { this.history = []; this._requestStartHandlers = []; + this._inheritOptions = {}; this._filterPredicates = [ (filter) => { @@ -197,8 +198,9 @@ export function SearchSourceProvider(Promise, PromiseEmitter, Private, config) { * @param {SearchSource} searchSource - the parent searchSource * @return {this} - chainable */ - inherits(parent) { + inherits(parent, options = {}) { this._parent = parent; + this._inheritOptions = options; return this; } @@ -458,8 +460,19 @@ export function SearchSourceProvider(Promise, PromiseEmitter, Private, config) { this.activeFetchCount = (this.activeFetchCount || 0) + 1; this.history = [request]; + const handlers = [...this._requestStartHandlers]; + // If callparentStartHandlers has been set to true, we also call all + // handlers of parent search sources. + if (this._inheritOptions.callParentStartHandlers) { + let searchSource = this.getParent(); + while (searchSource) { + handlers.push(...searchSource._requestStartHandlers); + searchSource = searchSource.getParent(); + } + } + return Promise - .map(this._requestStartHandlers, fn => fn(this, request)) + .map(handlers, fn => fn(this, request)) .then(_.noop); } diff --git a/src/ui/public/vis/request_handlers/courier.js b/src/ui/public/vis/request_handlers/courier.js index c4868a58d9232..ef08d0f48cd47 100644 --- a/src/ui/public/vis/request_handlers/courier.js +++ b/src/ui/public/vis/request_handlers/courier.js @@ -40,11 +40,32 @@ const CourierRequestHandlerProvider = function (Private, courier, timefilter) { name: 'courier', handler: function (vis, { appState, queryFilter, searchSource, timeRange }) { - searchSource.filter(() => { + // Create a new search source that inherits the original search source + // but has the propriate timeRange applied via a filter. + // This is a temporary solution until we properly pass down all required + // information for the request to the request handler (https://github.com/elastic/kibana/issues/16641). + // Using callParentStartHandlers: true we make sure, that the parent searchSource + // onSearchRequestStart will be called properly even though we use an inherited + // search source. + const requestSearchSource = new SearchSource().inherits(searchSource, { callParentStartHandlers: true }); + + // For now we need to mirror the history of the passed search source, since + // the spy panel wouldn't work otherwise. + Object.defineProperty(requestSearchSource, 'history', { + get() { + return requestSearchSource._parent.history; + }, + set(history) { + return requestSearchSource._parent.history = history; + } + }); + + // Add the explicit passed timeRange as a filter to the requestSearchSource. + requestSearchSource.filter(() => { return timefilter.get(searchSource.index(), timeRange); }); - removeSearchSourceParentTimefilter(searchSource, timeRange); + removeSearchSourceParentTimefilter(requestSearchSource); if (queryFilter && vis.editorMode) { searchSource.set('filter', queryFilter.getFilters()); @@ -75,7 +96,7 @@ const CourierRequestHandlerProvider = function (Private, courier, timefilter) { return new Promise((resolve, reject) => { if (shouldQuery()) { delete vis.reload; - searchSource.onResults().then(resp => { + requestSearchSource.onResults().then(resp => { searchSource.lastQuery = { filter: _.cloneDeep(searchSource.get('filter')), query: _.cloneDeep(searchSource.get('query')), @@ -88,7 +109,7 @@ const CourierRequestHandlerProvider = function (Private, courier, timefilter) { return _.cloneDeep(resp); }).then(async resp => { for (const agg of vis.getAggConfig()) { - const nestedSearchSource = new SearchSource().inherits(searchSource); + const nestedSearchSource = new SearchSource().inherits(requestSearchSource); resp = await agg.type.postFlightRequest(resp, vis.aggs, agg, nestedSearchSource); } From fd86ca141c277a197412cabaead56d45f5a4d8b1 Mon Sep 17 00:00:00 2001 From: Tim Roes Date: Mon, 16 Apr 2018 10:51:51 +0200 Subject: [PATCH 2/3] Add tests --- .../data_source/__tests__/search_source.js | 48 +++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/src/ui/public/courier/data_source/__tests__/search_source.js b/src/ui/public/courier/data_source/__tests__/search_source.js index 4230437d5b452..3db1e4ca78c31 100644 --- a/src/ui/public/courier/data_source/__tests__/search_source.js +++ b/src/ui/public/courier/data_source/__tests__/search_source.js @@ -6,6 +6,12 @@ import { requestQueue } from '../../_request_queue'; import { SearchSourceProvider } from '../search_source'; import StubIndexPatternProv from 'test_utils/stub_index_pattern'; +function timeout() { + return new Promise(resolve => { + setTimeout(resolve); + }); +} + describe('SearchSource', function () { require('test_utils/no_digest_promises').activateForSuite(); @@ -154,6 +160,48 @@ describe('SearchSource', function () { }); }); + describe('#onRequestStart()', () => { + it('should be called when starting a request', async () => { + const source = new SearchSource(); + const fn = sinon.spy(); + source.onRequestStart(fn); + const request = {}; + source.requestIsStarting(request); + await timeout(); + expect(fn.calledWith(source, request)).to.be(true); + }); + + it('should not be called on parent searchSource', async () => { + const parent = new SearchSource(); + const source = new SearchSource().inherits(parent); + + const fn = sinon.spy(); + source.onRequestStart(fn); + const parentFn = sinon.spy(); + parent.onRequestStart(parentFn); + const request = {}; + source.requestIsStarting(request); + await timeout(); + expect(fn.calledWith(source, request)).to.be(true); + expect(parentFn.notCalled).to.be(true); + }); + + it('should be called on parent searchSource if callParentStartHandlers is true', async () => { + const parent = new SearchSource(); + const source = new SearchSource().inherits(parent, { callParentStartHandlers: true }); + + const fn = sinon.spy(); + source.onRequestStart(fn); + const parentFn = sinon.spy(); + parent.onRequestStart(parentFn); + const request = {}; + source.requestIsStarting(request); + await timeout(); + expect(fn.calledWith(source, request)).to.be(true); + expect(parentFn.calledWith(source, request)).to.be(true); + }); + }); + describe('#_mergeProp', function () { describe('filter', function () { let source; From 71a4ff98835fe58615c7695f580ba7b99d63e5a9 Mon Sep 17 00:00:00 2001 From: Tim Roes Date: Mon, 16 Apr 2018 12:40:22 +0200 Subject: [PATCH 3/3] Only add timerange filter predicate if index exists --- src/ui/public/vis/request_handlers/courier.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/ui/public/vis/request_handlers/courier.js b/src/ui/public/vis/request_handlers/courier.js index ef08d0f48cd47..5d37b2b9d7884 100644 --- a/src/ui/public/vis/request_handlers/courier.js +++ b/src/ui/public/vis/request_handlers/courier.js @@ -20,8 +20,8 @@ const CourierRequestHandlerProvider = function (Private, courier, timefilter) { } const index = searchSource.index() || searchSource.getParent().index(); - const timeFieldName = index.timeFieldName; - if (!timeFieldName) { + const timeFieldName = index && index.timeFieldName; + if (!index || !timeFieldName) { return true; }