From 9aa6e48b8fa01c34d0195f5d3841ef7cf138c88f Mon Sep 17 00:00:00 2001 From: Lukas Olson Date: Mon, 9 Sep 2019 20:31:49 -0700 Subject: [PATCH 01/25] Initial refactor of search source --- src/legacy/ui/public/courier/courier.js | 12 +- .../courier/fetch/__tests__/call_client.js | 359 ------------------ .../courier/fetch/__tests__/fetch_now.js | 122 ------ .../ui/public/courier/fetch/call_client.js | 240 ++++-------- .../fetch/call_client.test.js} | 2 - .../courier/fetch/call_response_handlers.js | 104 ----- .../courier/fetch/continue_incomplete.js | 51 --- .../ui/public/courier/fetch/fetch_now.js | 124 ------ .../ui/public/courier/fetch/fetch_soon.js | 64 ++-- .../{request/index.js => fetch_soon.test.js} | 2 +- .../ui/public/courier/fetch/is_request.js | 28 -- .../ui/public/courier/fetch/req_status.js | 23 -- .../__tests__/search_request.js | 78 ---- .../request/search_request/search_request.js | 205 ---------- .../request/serialize_fetch_params/index.js | 20 - .../serialize_fetch_params.js | 60 --- .../serialize_fetch_params.test.js | 152 -------- .../serialize_fetch_params_provider.js | 31 -- .../ui/public/courier/search_poll/index.js | 2 +- .../public/courier/search_poll/search_poll.js | 61 +-- .../search_poll.test.js} | 2 +- .../__tests__/search_request_queue.js | 57 --- .../search_request_queue.js | 70 ---- .../search_source/__tests__/search_source.js | 352 ----------------- .../courier/search_source/search_source.js | 86 +---- .../search_source.test.js} | 2 +- .../default_search_strategy.js | 124 ++---- .../public/courier/search_strategy/index.js | 3 +- .../search_strategy_registry.js | 53 +-- .../search_strategy_registry.test.js | 74 ---- .../_error_allow_explicit_index.scss | 3 - .../error_allow_explicit_index/_index.scss | 1 - .../error_allow_explicit_index.html | 48 --- .../error_allow_explicit_index.js | 57 --- .../public/search/rollup_search_strategy.js | 51 +-- .../search/rollup_search_strategy.test.js | 7 + .../translations/translations/ja-JP.json | 8 - .../translations/translations/zh-CN.json | 8 - 38 files changed, 170 insertions(+), 2576 deletions(-) delete mode 100644 src/legacy/ui/public/courier/fetch/__tests__/call_client.js delete mode 100644 src/legacy/ui/public/courier/fetch/__tests__/fetch_now.js rename src/legacy/ui/public/{error_allow_explicit_index/index.js => courier/fetch/call_client.test.js} (90%) delete mode 100644 src/legacy/ui/public/courier/fetch/call_response_handlers.js delete mode 100644 src/legacy/ui/public/courier/fetch/continue_incomplete.js delete mode 100644 src/legacy/ui/public/courier/fetch/fetch_now.js rename src/legacy/ui/public/courier/fetch/{request/index.js => fetch_soon.test.js} (93%) delete mode 100644 src/legacy/ui/public/courier/fetch/is_request.js delete mode 100644 src/legacy/ui/public/courier/fetch/req_status.js delete mode 100644 src/legacy/ui/public/courier/fetch/request/search_request/__tests__/search_request.js delete mode 100644 src/legacy/ui/public/courier/fetch/request/search_request/search_request.js delete mode 100644 src/legacy/ui/public/courier/fetch/request/serialize_fetch_params/index.js delete mode 100644 src/legacy/ui/public/courier/fetch/request/serialize_fetch_params/serialize_fetch_params.js delete mode 100644 src/legacy/ui/public/courier/fetch/request/serialize_fetch_params/serialize_fetch_params.test.js delete mode 100644 src/legacy/ui/public/courier/fetch/request/serialize_fetch_params/serialize_fetch_params_provider.js rename src/legacy/ui/public/courier/{fetch/request/search_request/index.js => search_poll/search_poll.test.js} (93%) delete mode 100644 src/legacy/ui/public/courier/search_request_queue/__tests__/search_request_queue.js delete mode 100644 src/legacy/ui/public/courier/search_request_queue/search_request_queue.js delete mode 100644 src/legacy/ui/public/courier/search_source/__tests__/search_source.js rename src/legacy/ui/public/courier/{search_request_queue/index.js => search_source/search_source.test.js} (92%) delete mode 100644 src/legacy/ui/public/error_allow_explicit_index/_error_allow_explicit_index.scss delete mode 100644 src/legacy/ui/public/error_allow_explicit_index/_index.scss delete mode 100644 src/legacy/ui/public/error_allow_explicit_index/error_allow_explicit_index.html delete mode 100644 src/legacy/ui/public/error_allow_explicit_index/error_allow_explicit_index.js create mode 100644 x-pack/legacy/plugins/rollup/public/search/rollup_search_strategy.test.js diff --git a/src/legacy/ui/public/courier/courier.js b/src/legacy/ui/public/courier/courier.js index a317932e51118..ec599b1653a93 100644 --- a/src/legacy/ui/public/courier/courier.js +++ b/src/legacy/ui/public/courier/courier.js @@ -27,15 +27,14 @@ import { uiModules } from '../modules'; import { addFatalErrorCallback } from '../notify'; import '../promises'; -import { searchRequestQueue } from './search_request_queue'; import { FetchSoonProvider } from './fetch'; -import { SearchPollProvider } from './search_poll'; +import { SearchPoll } from './search_poll'; uiModules.get('kibana/courier').service('courier', ($rootScope, Private) => { const fetchSoon = Private(FetchSoonProvider); // This manages the doc fetch interval. - const searchPoll = Private(SearchPollProvider); + const searchPoll = new SearchPoll(); class Courier { constructor() { @@ -62,13 +61,6 @@ uiModules.get('kibana/courier').service('courier', ($rootScope, Private) => { // resume polling. searchPoll.pause(); - // And abort all pending requests. - searchRequestQueue.abortAll(); - - if (searchRequestQueue.getCount()) { - throw new Error('Aborting all pending requests failed.'); - } - refreshIntervalSubscription.unsubscribe(); }); diff --git a/src/legacy/ui/public/courier/fetch/__tests__/call_client.js b/src/legacy/ui/public/courier/fetch/__tests__/call_client.js deleted file mode 100644 index 895784ee4e6b3..0000000000000 --- a/src/legacy/ui/public/courier/fetch/__tests__/call_client.js +++ /dev/null @@ -1,359 +0,0 @@ -/* - * Licensed to Elasticsearch B.V. under one or more contributor - * license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright - * ownership. Elasticsearch B.V. licenses this file to you under - * the Apache License, Version 2.0 (the "License"); you may - * not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ - -import sinon from 'sinon'; -import expect from '@kbn/expect'; -import ngMock from 'ng_mock'; -import NoDigestPromises from 'test_utils/no_digest_promises'; -import { delay } from 'bluebird'; - -import { CallClientProvider } from '../call_client'; -import { RequestStatus } from '../req_status'; -import { SearchRequestProvider } from '../request'; -import { addSearchStrategy } from '../../search_strategy'; - -describe('callClient', () => { - NoDigestPromises.activateForSuite(); - - const ABORTED = RequestStatus.ABORTED; - - let SearchRequest; - let callClient; - let fakeSearch; - let searchRequests; - let esRequestDelay; - let esShouldError; - let esPromiseAbortSpy; - - const createSearchRequest = (id, overrides = {}, errorHandler = () => {}) => { - const { source: overrideSource, ...rest } = overrides; - - const source = { - _flatten: () => ({}), - requestIsStopped: () => {}, - getField: () => 'indexPattern', - getPreferredSearchStrategyId: () => undefined, - ...overrideSource - }; - - const searchRequest = new SearchRequest({ source, errorHandler, ...rest }); - searchRequest.__testId__ = id; - return searchRequest; - }; - - beforeEach(ngMock.module('kibana')); - - beforeEach(ngMock.module(function stubEs($provide) { - esRequestDelay = 0; - esShouldError = false; - - $provide.service('es', (Promise) => { - fakeSearch = sinon.spy(() => { - const esPromise = new Promise((resolve, reject) => { - if (esShouldError) { - return reject('fake es error'); - } - - setTimeout(() => { - resolve({ - responses: searchRequests.map(searchRequest => searchRequest.__testId__), - }); - }, esRequestDelay); - }); - - esPromise.abort = esPromiseAbortSpy = sinon.spy(); - return esPromise; - }); - - return { - msearch: fakeSearch, - }; - }); - })); - - beforeEach(ngMock.inject(Private => { - callClient = Private(CallClientProvider); - SearchRequest = Private(SearchRequestProvider); - })); - - describe('basic contract', () => { - it('returns a promise', () => { - searchRequests = [ createSearchRequest() ]; - const callingClient = callClient(searchRequests); - expect(callingClient.then).to.be.a('function'); - }); - - it(`resolves the promise with the 'responses' property of the es.msearch() result`, () => { - searchRequests = [ createSearchRequest(1) ]; - - return callClient(searchRequests).then(results => { - expect(results).to.eql([1]); - }); - }); - - describe('for failing requests', () => { - beforeEach(() => { - addSearchStrategy({ - id: 'fail', - isViable: indexPattern => { - return indexPattern.type === 'fail'; - }, - search: () => { - return { - searching: Promise.reject(new Error('Search failed')), - failedSearchRequests: [], - abort: () => {}, - }; - }, - }); - }); - - it(`still bubbles up the failure`, () => { - const searchRequestFail1 = createSearchRequest('fail1', { - source: { - getField: () => ({ type: 'fail' }), - }, - }); - - const searchRequestFail2 = createSearchRequest('fail2', { - source: { - getField: () => ({ type: 'fail' }), - }, - }); - - searchRequests = [ searchRequestFail1, searchRequestFail2 ]; - - return callClient(searchRequests).then(results => { - expect(results).to.eql([ - { error: new Error('Search failed') }, - { error: new Error('Search failed') }, - ]); - }); - }); - }); - }); - - describe('implementation', () => { - it('calls es.msearch() once, regardless of number of searchRequests', () => { - expect(fakeSearch.callCount).to.be(0); - searchRequests = [ createSearchRequest(), createSearchRequest(), createSearchRequest() ]; - - return callClient(searchRequests).then(() => { - expect(fakeSearch.callCount).to.be(1); - }); - }); - - it('calls searchRequest.whenAborted() as part of setup', async () => { - const whenAbortedSpy = sinon.spy(); - const searchRequest = createSearchRequest(); - searchRequest.whenAborted = whenAbortedSpy; - searchRequests = [ searchRequest ]; - - return callClient(searchRequests).then(() => { - expect(whenAbortedSpy.callCount).to.be(1); - }); - }); - }); - - describe('aborting at different points in the request lifecycle:', () => { - - it('while the search body is being formed resolves with an ABORTED response', () => { - const searchRequest = createSearchRequest(1, { - source: { - _flatten: () => { - return new Promise(resolve => { - setTimeout(() => { - resolve({}); - }, 100); - }); - }, - requestIsStopped: () => {}, - }, - }); - - searchRequests = [ searchRequest ]; - const callingClient = callClient(searchRequests); - - // Abort the request while the search body is being formed. - setTimeout(() => { - searchRequest.abort(); - }, 20); - - return callingClient.then(results => { - expect(results).to.eql([ ABORTED ]); - }); - }); - - it('while the search is in flight resolves with an ABORTED response', () => { - esRequestDelay = 100; - - const searchRequest = createSearchRequest(); - searchRequests = [ searchRequest ]; - const callingClient = callClient(searchRequests); - - // Abort the request while the search is in flight.. - setTimeout(() => { - searchRequest.abort(); - }, 80); - - return callingClient.then(results => { - expect(results).to.eql([ ABORTED ]); - }); - }); - }); - - describe('aborting number of requests:', () => { - it(`aborting all searchRequests resolves with ABORTED responses`, () => { - const searchRequest1 = createSearchRequest(); - const searchRequest2 = createSearchRequest(); - searchRequests = [ searchRequest1, searchRequest2 ]; - const callingClient = callClient(searchRequests); - - searchRequest1.abort(); - searchRequest2.abort(); - - return callingClient.then(results => { - expect(results).to.eql([ABORTED, ABORTED]); - }); - }); - - it(`aborting all searchRequests calls abort() on the promise returned by searchStrategy.search()`, () => { - esRequestDelay = 100; - - const searchRequest1 = createSearchRequest(); - const searchRequest2 = createSearchRequest(); - searchRequests = [ searchRequest1, searchRequest2 ]; - - const callingClient = callClient(searchRequests); - - return Promise.all([ - delay(70).then(() => { - // At this point we expect the request to be in flight. - expect(esPromiseAbortSpy.callCount).to.be(0); - searchRequest1.abort(); - searchRequest2.abort(); - }), - callingClient.then(() => { - expect(esPromiseAbortSpy.callCount).to.be(1); - }), - ]); - }); - - it('aborting some searchRequests resolves those with ABORTED responses', () => { - const searchRequest1 = createSearchRequest(1); - const searchRequest2 = createSearchRequest(2); - searchRequests = [ searchRequest1, searchRequest2 ]; - const callingClient = callClient(searchRequests); - searchRequest2.abort(); - - return callingClient.then(results => { - expect(results).to.eql([ 1, ABORTED ]); - }); - }); - }); - - describe('searchRequests with multiple searchStrategies map correctly to their responses', () => { - const search = ({ searchRequests }) => { - return { - searching: Promise.resolve(searchRequests.map(searchRequest => searchRequest.__testId__)), - failedSearchRequests: [], - abort: () => {}, - }; - }; - - const searchStrategyA = { - id: 'a', - isViable: indexPattern => { - return indexPattern.type === 'a'; - }, - search, - }; - - const searchStrategyB = { - id: 'b', - isViable: indexPattern => { - return indexPattern.type === 'b'; - }, - search, - }; - - let searchRequestA; - let searchRequestB; - let searchRequestA2; - - beforeEach(() => { - addSearchStrategy(searchStrategyA); - addSearchStrategy(searchStrategyB); - - searchRequestA = createSearchRequest('a', { - source: { - getField: () => ({ type: 'a' }), - getSearchStrategyForSearchRequest: () => {}, - getPreferredSearchStrategyId: () => {}, - }, - }); - - searchRequestB = createSearchRequest('b', { - source: { - getField: () => ({ type: 'b' }), - getSearchStrategyForSearchRequest: () => {}, - getPreferredSearchStrategyId: () => {}, - }, - }); - - searchRequestA2 = createSearchRequest('a2', { - source: { - getField: () => ({ type: 'a' }), - getSearchStrategyForSearchRequest: () => {}, - getPreferredSearchStrategyId: () => {}, - }, - }); - }); - - it('if the searchRequests are reordered by the searchStrategies', () => { - // Add requests in an order which will be reordered by the strategies. - searchRequests = [ searchRequestA, searchRequestB, searchRequestA2 ]; - const callingClient = callClient(searchRequests); - - return callingClient.then(results => { - expect(results).to.eql(['a', 'b', 'a2']); - }); - }); - - it('if one is aborted after being provided', () => { - // Add requests in an order which will be reordered by the strategies. - searchRequests = [ searchRequestA, searchRequestB, searchRequestA2 ]; - const callingClient = callClient(searchRequests); - searchRequestA2.abort(); - - return callingClient.then(results => { - expect(results).to.eql(['a', 'b', ABORTED]); - }); - }); - - it(`if one is already aborted when it's provided`, () => { - searchRequests = [ searchRequestA, searchRequestB, ABORTED, searchRequestA2 ]; - const callingClient = callClient(searchRequests); - - return callingClient.then(results => { - expect(results).to.eql(['a', 'b', ABORTED, 'a2']); - }); - }); - }); -}); diff --git a/src/legacy/ui/public/courier/fetch/__tests__/fetch_now.js b/src/legacy/ui/public/courier/fetch/__tests__/fetch_now.js deleted file mode 100644 index 19032ce1f4ca3..0000000000000 --- a/src/legacy/ui/public/courier/fetch/__tests__/fetch_now.js +++ /dev/null @@ -1,122 +0,0 @@ -/* - * Licensed to Elasticsearch B.V. under one or more contributor - * license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright - * ownership. Elasticsearch B.V. licenses this file to you under - * the Apache License, Version 2.0 (the "License"); you may - * not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ - -import sinon from 'sinon'; -import expect from '@kbn/expect'; -import ngMock from 'ng_mock'; - -import { CallClientProvider } from '../call_client'; -import { CallResponseHandlersProvider } from '../call_response_handlers'; -import { ContinueIncompleteProvider } from '../continue_incomplete'; -import { FetchNowProvider } from '../fetch_now'; - -function mockRequest() { - return { - strategy: 'mock', - started: true, - aborted: false, - handleFailure: sinon.spy(), - retry: sinon.spy(function () { return this; }), - continue: sinon.spy(function () { return this; }), - start: sinon.spy(function () { return this; }) - }; -} - -describe('FetchNowProvider', () => { - - let Promise; - let $rootScope; - let fetchNow; - 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(CallClientProvider, FakeResponsesProvider); - PrivateProvider.swap(CallResponseHandlersProvider, FakeResponsesProvider); - PrivateProvider.swap(ContinueIncompleteProvider, FakeResponsesProvider); - })); - - beforeEach(ngMock.inject((Private, $injector) => { - $rootScope = $injector.get('$rootScope'); - Promise = $injector.get('Promise'); - fetchNow = Private(FetchNowProvider); - request = mockRequest(); - requests = [ request ]; - })); - - describe('when request has not started', () => { - beforeEach(() => requests.forEach(req => req.started = false)); - - it('starts request', () => { - fetchNow(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)); - fetchNow(requests); - - expect(request.start.callCount).to.be(1); - expect(fakeResponses.callCount).to.be(0); - $rootScope.$apply(); - expect(fakeResponses.callCount).to.be(3); - }); - - it('invokes request failure handler if starting fails', () => { - request.start = sinon.stub().returns(Promise.reject('some error')); - fetchNow(requests); - $rootScope.$apply(); - sinon.assert.calledWith(request.handleFailure, 'some error'); - }); - }); - - describe('when request has already started', () => { - it('continues request', () => { - fetchNow(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)); - fetchNow(requests); - - expect(request.continue.callCount).to.be(1); - expect(fakeResponses.callCount).to.be(0); - $rootScope.$apply(); - expect(fakeResponses.callCount).to.be(3); - }); - it('invokes request failure handler if continuing fails', () => { - request.continue = sinon.stub().returns(Promise.reject('some error')); - fetchNow(requests); - $rootScope.$apply(); - sinon.assert.calledWith(request.handleFailure, 'some error'); - }); - }); -}); diff --git a/src/legacy/ui/public/courier/fetch/call_client.js b/src/legacy/ui/public/courier/fetch/call_client.js index 7ba73e741c074..8a6546c20e126 100644 --- a/src/legacy/ui/public/courier/fetch/call_client.js +++ b/src/legacy/ui/public/courier/fetch/call_client.js @@ -17,187 +17,81 @@ * under the License. */ -import { ErrorAllowExplicitIndexProvider } from '../../error_allow_explicit_index'; -import { assignSearchRequestsToSearchStrategies } from '../search_strategy'; -import { IsRequestProvider } from './is_request'; -import { RequestStatus } from './req_status'; -import { SerializeFetchParamsProvider } from './request/serialize_fetch_params'; +import { groupBy } from 'lodash'; +import { getSearchStrategyForSearchRequest, getSearchStrategyById } from '../search_strategy'; +import { toastNotifications } from '../../notify/toasts'; import { i18n } from '@kbn/i18n'; -import { createDefer } from 'ui/promises'; - -export function CallClientProvider(Private, Promise, es, config, sessionId, esShardTimeout) { - const errorAllowExplicitIndex = Private(ErrorAllowExplicitIndexProvider); - const isRequest = Private(IsRequestProvider); - const serializeFetchParams = Private(SerializeFetchParamsProvider); - - const ABORTED = RequestStatus.ABORTED; - - function callClient(searchRequests) { - // get the actual list of requests that we will be fetching - const requestsToFetch = searchRequests.filter(isRequest); - let requestsToFetchCount = requestsToFetch.length; - - if (requestsToFetchCount === 0) { - return Promise.resolve([]); - } - - // This is how we'll provide the consumer with search responses. Resolved by - // respondToSearchRequests. - const defer = createDefer(Promise); - - const abortableSearches = []; - let areAllSearchRequestsAborted = false; - - // When we traverse our search requests and send out searches, some of them may fail. We'll - // store those that don't fail here. - const activeSearchRequests = []; - - // Respond to each searchRequest with the response or ABORTED. - const respondToSearchRequests = (responsesInOriginalRequestOrder = []) => { - // We map over searchRequests because if we were originally provided an ABORTED - // request then we'll return that value. - return Promise.map(searchRequests, function (searchRequest, searchRequestIndex) { - if (searchRequest.aborted) { - return ABORTED; - } - - const status = searchRequests[searchRequestIndex]; - - if (status === ABORTED) { - return ABORTED; - } - - const activeSearchRequestIndex = activeSearchRequests.indexOf(searchRequest); - const isFailedSearchRequest = activeSearchRequestIndex === -1; - - if (isFailedSearchRequest) { - return ABORTED; - } - - return responsesInOriginalRequestOrder[searchRequestIndex]; - }) - .then( - (res) => defer.resolve(res), - (err) => defer.reject(err) - ); - }; - - // handle a request being aborted while being fetched - const requestWasAborted = Promise.method(function (searchRequest, index) { - if (searchRequests[index] === ABORTED) { - defer.reject(new Error( - i18n.translate('common.ui.courier.fetch.requestWasAbortedTwiceErrorMessage', { - defaultMessage: 'Request was aborted twice?', - }) - )); - } - - requestsToFetchCount--; - - if (requestsToFetchCount !== 0) { - // We can't resolve early unless all searchRequests have been aborted. - return; - } - - abortableSearches.forEach(({ abort }) => { - abort(); - }); - - areAllSearchRequestsAborted = true; - - return respondToSearchRequests(); +import { EuiSpacer } from '@elastic/eui'; +import React from 'react'; +import { ShardFailureOpenModalButton } from './components/shard_failure_open_modal_button'; + +export function callClient(searchRequests, requestsOptions = [], { Promise, es, config, sessionId, esShardTimeout }) { + // Correlate the options with the request that they're associated with + const requestOptionEntries = searchRequests.map((request, i) => [request, requestsOptions[i]]); + const requestOptionsMap = new Map(requestOptionEntries); + + // Group the requests by the strategy used to search that specific request + const searchStrategyMap = groupBy(searchRequests, (request, i) => { + const searchStrategy = getSearchStrategyForSearchRequest(request, requestsOptions[i]); + return searchStrategy.id; + }); + + // Execute each search strategy with the group of requests, but return the responses in the same + // order in which they were received. We use a map to correlate the original request with its + // response. + const requestResponseMap = new Map(); + Object.keys(searchStrategyMap).forEach(searchStrategyId => { + const searchStrategy = getSearchStrategyById(searchStrategyId); + const requests = searchStrategyMap[searchStrategyId]; + const { searching, abort } = searchStrategy.search({ searchRequests: requests, es, Promise, config, sessionId, esShardTimeout }); + requests.forEach((request, i) => { + const response = searching.then(results => handleResponse(request, results[i])); + const { abortSignal } = requestOptionsMap.get(request) || {}; + if (abortSignal) abortSignal.addEventListener('abort', abort); + requestResponseMap.set(request, response); }); + }, []); + return searchRequests.map(request => requestResponseMap.get(request)); +} - // attach abort handlers, close over request index - searchRequests.forEach(function (searchRequest, index) { - if (!isRequest(searchRequest)) { - return; - } - - searchRequest.whenAborted(function () { - requestWasAborted(searchRequest, index).catch(defer.reject); - }); +export function handleResponse(request, response) { + if (response.timed_out) { + toastNotifications.addWarning({ + title: i18n.translate('common.ui.courier.fetch.requestTimedOutNotificationMessage', { + defaultMessage: 'Data might be incomplete because your request timed out', + }), }); + } - const searchStrategiesWithRequests = assignSearchRequestsToSearchStrategies(requestsToFetch); - - // We're going to create a new async context here, so that the logic within it can execute - // asynchronously after we've returned a reference to defer.promise. - Promise.resolve().then(async () => { - // Execute each request using its search strategy. - for (let i = 0; i < searchStrategiesWithRequests.length; i++) { - const searchStrategyWithSearchRequests = searchStrategiesWithRequests[i]; - const { searchStrategy, searchRequests } = searchStrategyWithSearchRequests; - const { - searching, - abort, - failedSearchRequests, - } = await searchStrategy.search({ searchRequests, es, Promise, serializeFetchParams, config, sessionId, esShardTimeout }); - - // Collect searchRequests which have successfully been sent. - searchRequests.forEach(searchRequest => { - if (failedSearchRequests.includes(searchRequest)) { - return; - } - - activeSearchRequests.push(searchRequest); - }); - - abortableSearches.push({ - searching, - abort, - requestsCount: searchRequests.length, - }); - } - - try { - // The request was aborted while we were doing the above logic. - if (areAllSearchRequestsAborted) { - return; - } - - const segregatedResponses = await Promise.all(abortableSearches.map(async ({ searching, requestsCount }) => { - return searching.catch((e) => { - // Duplicate errors so that they correspond to the original requests. - return new Array(requestsCount).fill({ error: e }); - }); - })); - - // Assigning searchRequests to strategies means that the responses come back in a different - // order than the original searchRequests. So we'll put them back in order so that we can - // use the order to associate each response with the original request. - const responsesInOriginalRequestOrder = new Array(searchRequests.length); - segregatedResponses.forEach((responses, strategyIndex) => { - responses.forEach((response, responseIndex) => { - const searchRequest = searchStrategiesWithRequests[strategyIndex].searchRequests[responseIndex]; - const requestIndex = searchRequests.indexOf(searchRequest); - responsesInOriginalRequestOrder[requestIndex] = response; - }); - }); - - await respondToSearchRequests(responsesInOriginalRequestOrder); - } catch(error) { - if (errorAllowExplicitIndex.test(error)) { - return errorAllowExplicitIndex.takeover(); - } - - defer.reject(error); - } + if (response._shards && response._shards.failed) { + const title = i18n.translate('common.ui.courier.fetch.shardsFailedNotificationMessage', { + defaultMessage: '{shardsFailed} of {shardsTotal} shards failed', + values: { + shardsFailed: response._shards.failed, + shardsTotal: response._shards.total, + }, + }); + const description = i18n.translate('common.ui.courier.fetch.shardsFailedNotificationDescription', { + defaultMessage: 'The data you are seeing might be incomplete or wrong.', }); - // Return the promise which acts as our vehicle for providing search responses to the consumer. - // However, if there are any errors, notify the searchRequests of them *instead* of bubbling - // them up to the consumer. - return defer.promise.catch((err) => { - // By returning the return value of this catch() without rethrowing the error, we delegate - // error-handling to the searchRequest instead of the consumer. - searchRequests.forEach((searchRequest, index) => { - if (searchRequests[index] !== ABORTED) { - searchRequest.handleFailure(err); - } - }); + const text = ( + <> + {description} + + + + ); + + toastNotifications.addWarning({ + title, + text, }); } - return callClient; + return response; } diff --git a/src/legacy/ui/public/error_allow_explicit_index/index.js b/src/legacy/ui/public/courier/fetch/call_client.test.js similarity index 90% rename from src/legacy/ui/public/error_allow_explicit_index/index.js rename to src/legacy/ui/public/courier/fetch/call_client.test.js index a832fde31c987..9880b336e76e5 100644 --- a/src/legacy/ui/public/error_allow_explicit_index/index.js +++ b/src/legacy/ui/public/courier/fetch/call_client.test.js @@ -16,5 +16,3 @@ * specific language governing permissions and limitations * under the License. */ - -export { ErrorAllowExplicitIndexProvider } from './error_allow_explicit_index'; diff --git a/src/legacy/ui/public/courier/fetch/call_response_handlers.js b/src/legacy/ui/public/courier/fetch/call_response_handlers.js deleted file mode 100644 index 379ea68a99b51..0000000000000 --- a/src/legacy/ui/public/courier/fetch/call_response_handlers.js +++ /dev/null @@ -1,104 +0,0 @@ -/* - * Licensed to Elasticsearch B.V. under one or more contributor - * license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright - * ownership. Elasticsearch B.V. licenses this file to you under - * the Apache License, Version 2.0 (the "License"); you may - * not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ -import React from 'react'; -import { i18n } from '@kbn/i18n'; -import { EuiSpacer } from '@elastic/eui'; -import { toastNotifications } from '../../notify'; -import { RequestFailure } from '../../errors'; -import { RequestStatus } from './req_status'; -import { SearchError } from '../search_strategy/search_error'; -import { ShardFailureOpenModalButton } from './components/shard_failure_open_modal_button'; - -export function CallResponseHandlersProvider(Promise) { - const ABORTED = RequestStatus.ABORTED; - const INCOMPLETE = RequestStatus.INCOMPLETE; - - function callResponseHandlers(searchRequests, responses) { - return Promise.map(searchRequests, function (searchRequest, index) { - if (searchRequest === ABORTED || searchRequest.aborted) { - return ABORTED; - } - - const response = responses[index]; - - if (response.timed_out) { - toastNotifications.addWarning({ - title: i18n.translate('common.ui.courier.fetch.requestTimedOutNotificationMessage', { - defaultMessage: 'Data might be incomplete because your request timed out', - }), - }); - } - - if (response._shards && response._shards.failed) { - const title = i18n.translate('common.ui.courier.fetch.shardsFailedNotificationMessage', { - defaultMessage: '{shardsFailed} of {shardsTotal} shards failed', - values: { - shardsFailed: response._shards.failed, - shardsTotal: response._shards.total, - }, - }); - const description = i18n.translate('common.ui.courier.fetch.shardsFailedNotificationDescription', { - defaultMessage: 'The data you are seeing might be incomplete or wrong.', - }); - - const text = ( - <> - {description} - - - - ); - - toastNotifications.addWarning({ - title, - text, - }); - } - - function progress() { - if (searchRequest.isIncomplete()) { - return INCOMPLETE; - } - - searchRequest.complete(); - return response; - } - - if (response.error) { - if (searchRequest.filterError(response)) { - return progress(); - } else { - return searchRequest.handleFailure( - response.error instanceof SearchError - ? response.error - : new RequestFailure(null, response) - ); - } - } - - return Promise.try(() => searchRequest.handleResponse(response)).then(progress); - }); - } - - return callResponseHandlers; -} diff --git a/src/legacy/ui/public/courier/fetch/continue_incomplete.js b/src/legacy/ui/public/courier/fetch/continue_incomplete.js deleted file mode 100644 index b40ebdb886748..0000000000000 --- a/src/legacy/ui/public/courier/fetch/continue_incomplete.js +++ /dev/null @@ -1,51 +0,0 @@ -/* - * Licensed to Elasticsearch B.V. under one or more contributor - * license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright - * ownership. Elasticsearch B.V. licenses this file to you under - * the Apache License, Version 2.0 (the "License"); you may - * not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ - -import { RequestStatus } from './req_status'; - -export function ContinueIncompleteProvider() { - const INCOMPLETE = RequestStatus.INCOMPLETE; - - function continueIncompleteRequests(searchRequests, responses, fetchSearchResults) { - const incompleteSearchRequests = []; - - responses.forEach(function (response, index) { - if (response === INCOMPLETE) { - incompleteSearchRequests.push(searchRequests[index]); - } - }); - - if (!incompleteSearchRequests.length) { - return responses; - } - - return fetchSearchResults(incompleteSearchRequests) - .then(function (completedResponses) { - return responses.map(function (prevResponse) { - if (prevResponse !== INCOMPLETE) { - return prevResponse; - } - - return completedResponses.shift(); - }); - }); - } - - return continueIncompleteRequests; -} diff --git a/src/legacy/ui/public/courier/fetch/fetch_now.js b/src/legacy/ui/public/courier/fetch/fetch_now.js deleted file mode 100644 index de5704d4380f4..0000000000000 --- a/src/legacy/ui/public/courier/fetch/fetch_now.js +++ /dev/null @@ -1,124 +0,0 @@ -/* - * Licensed to Elasticsearch B.V. under one or more contributor - * license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright - * ownership. Elasticsearch B.V. licenses this file to you under - * the Apache License, Version 2.0 (the "License"); you may - * not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ - -import { fatalError } from '../../notify'; -import { CallClientProvider } from './call_client'; -import { CallResponseHandlersProvider } from './call_response_handlers'; -import { ContinueIncompleteProvider } from './continue_incomplete'; -import { RequestStatus } from './req_status'; -import { i18n } from '@kbn/i18n'; - -/** - * Fetch now provider should be used if you want the results searched and returned immediately. - * This can be slightly inefficient if a large number of requests are queued up, we can batch these - * by using fetchSoon. This introduces a slight delay which allows other requests to queue up before - * sending out requests in a batch. - * - * @param Private - * @param Promise - * @return {fetchNow} - * @constructor - */ -export function FetchNowProvider(Private, Promise) { - // core tasks - const callClient = Private(CallClientProvider); - const callResponseHandlers = Private(CallResponseHandlersProvider); - const continueIncomplete = Private(ContinueIncompleteProvider); - - const ABORTED = RequestStatus.ABORTED; - const INCOMPLETE = RequestStatus.INCOMPLETE; - - function fetchNow(searchRequests) { - return fetchSearchResults(searchRequests.map(function (searchRequest) { - if (!searchRequest.started) { - return searchRequest; - } - - return searchRequest.retry(); - })) - .catch(error => { - // If any errors occur after the search requests have resolved, then we kill Kibana. - fatalError(error, 'Courier fetch'); - }); - } - - function fetchSearchResults(searchRequests) { - function replaceAbortedRequests() { - searchRequests = searchRequests.map(searchRequest => { - if (searchRequest.aborted) { - return ABORTED; - } - - return searchRequest; - }); - } - - replaceAbortedRequests(); - return startRequests(searchRequests) - .then(function () { - replaceAbortedRequests(); - return callClient(searchRequests) - .catch(() => { - // Silently swallow errors that result from search requests so the consumer can surface - // them as notifications instead of courier forcing fatal errors. - }); - }) - .then(function (responses) { - replaceAbortedRequests(); - return callResponseHandlers(searchRequests, responses); - }) - .then(function (responses) { - replaceAbortedRequests(); - return continueIncomplete(searchRequests, responses, fetchSearchResults); - }) - .then(function (responses) { - replaceAbortedRequests(); - return responses.map(function (resp) { - switch (resp) { - case ABORTED: - return null; - case INCOMPLETE: - throw new Error( - i18n.translate('common.ui.courier.fetch.failedToClearRequestErrorMessage', { - defaultMessage: 'Failed to clear incomplete or duplicate request from responses.', - }) - ); - default: - return resp; - } - }); - }); - } - - function startRequests(searchRequests) { - return Promise.map(searchRequests, function (searchRequest) { - if (searchRequest === ABORTED) { - return searchRequest; - } - - return new Promise(function (resolve) { - const action = searchRequest.started ? searchRequest.continue : searchRequest.start; - resolve(action.call(searchRequest)); - }) - .catch(err => searchRequest.handleFailure(err)); - }); - } - - return fetchNow; -} diff --git a/src/legacy/ui/public/courier/fetch/fetch_soon.js b/src/legacy/ui/public/courier/fetch/fetch_soon.js index 266d4a6d3c9e6..6f8a31bf1f976 100644 --- a/src/legacy/ui/public/courier/fetch/fetch_soon.js +++ b/src/legacy/ui/public/courier/fetch/fetch_soon.js @@ -17,41 +17,57 @@ * under the License. */ -import _ from 'lodash'; -import { searchRequestQueue } from '../search_request_queue'; -import { FetchNowProvider } from './fetch_now'; +import { callClient } from './call_client'; /** * This is usually the right fetch provider to use, rather than FetchNowProvider, as this class introduces * a slight delay in the request process to allow multiple requests to queue up (e.g. when a dashboard * is loading). */ -export function FetchSoonProvider(Private, Promise, config) { +export function FetchSoonProvider(Promise, es, config, sessionId, esShardTimeout) { + /** + * Delays executing a function for a given amount of time, and returns a promise that resolves + * with the result. + * @param fn The function to invoke + * @param ms The number of milliseconds to wait + * @return Promise A promise that resolves with the result of executing the function + */ + function delay(fn, ms) { + return new Promise(resolve => { + setTimeout(() => resolve(fn()), ms); + }); + } - const fetchNow = Private(FetchNowProvider); + // The current batch/queue of requests to fetch + let requestsToFetch = []; + let requestOptions = []; - const fetch = () => fetchNow(searchRequestQueue.getPending()); - const debouncedFetch = _.debounce(fetch, { - wait: 10, - maxWait: 50 - }); + // The in-progress fetch (if there is one) + let fetchInProgress = null; /** - * Fetch a list of requests - * @param {array} requests - the requests to fetch - * @async + * Delay fetching for a given amount of time, while batching up the requests to be fetched. + * Returns a promise that resolves with the response for the given request. + * @param request The request to fetch + * @param ms The number of milliseconds to wait (and batch requests) + * @return Promise The response for the given request */ - this.fetchSearchRequests = (requests) => { - requests.forEach(req => req._setFetchRequested()); - config.get('courier:batchSearches') ? debouncedFetch() : fetch(); - return Promise.all(requests.map(req => req.getCompletePromise())); - }; + async function delayedFetch(request, options, ms) { + const i = requestsToFetch.length; + requestsToFetch = [...requestsToFetch, request]; + requestOptions = [...requestOptions, options]; + const responses = await (fetchInProgress = fetchInProgress || delay(() => { + const response = callClient(requestsToFetch, requestOptions, { Promise, es, config, sessionId, esShardTimeout }); + requestsToFetch = []; + requestOptions = []; + fetchInProgress = null; + return response; + }, ms)); + return responses[i]; + } - /** - * Return a promise that resembles the success of the fetch completing so we can execute - * logic based on this state change. Individual errors are routed to their respective requests. - */ - this.fetchQueued = () => { - return this.fetchSearchRequests(searchRequestQueue.getStartable()); + this.fetch = (request, options) => { + const delay = config.get('courier:batchSearches') ? 50 : 0; + return delayedFetch(request, options, delay); }; } diff --git a/src/legacy/ui/public/courier/fetch/request/index.js b/src/legacy/ui/public/courier/fetch/fetch_soon.test.js similarity index 93% rename from src/legacy/ui/public/courier/fetch/request/index.js rename to src/legacy/ui/public/courier/fetch/fetch_soon.test.js index 6647d0e5b2e10..81af3922ba9b3 100644 --- a/src/legacy/ui/public/courier/fetch/request/index.js +++ b/src/legacy/ui/public/courier/fetch/fetch_soon.test.js @@ -17,4 +17,4 @@ * under the License. */ -export { SearchRequestProvider } from './search_request'; + diff --git a/src/legacy/ui/public/courier/fetch/is_request.js b/src/legacy/ui/public/courier/fetch/is_request.js deleted file mode 100644 index 73c54d6f4bca1..0000000000000 --- a/src/legacy/ui/public/courier/fetch/is_request.js +++ /dev/null @@ -1,28 +0,0 @@ -/* - * Licensed to Elasticsearch B.V. under one or more contributor - * license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright - * ownership. Elasticsearch B.V. licenses this file to you under - * the Apache License, Version 2.0 (the "License"); you may - * not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ - -import { SearchRequestProvider } from './request'; - -export function IsRequestProvider(Private) { - const SearchRequest = Private(SearchRequestProvider); - - return function isRequest(obj) { - return obj instanceof SearchRequest; - }; -} diff --git a/src/legacy/ui/public/courier/fetch/req_status.js b/src/legacy/ui/public/courier/fetch/req_status.js deleted file mode 100644 index d56bc6d3ad360..0000000000000 --- a/src/legacy/ui/public/courier/fetch/req_status.js +++ /dev/null @@ -1,23 +0,0 @@ -/* - * Licensed to Elasticsearch B.V. under one or more contributor - * license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright - * ownership. Elasticsearch B.V. licenses this file to you under - * the Apache License, Version 2.0 (the "License"); you may - * not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ - -export const RequestStatus = { - ABORTED: 'aborted', - INCOMPLETE: 'incomplete', -}; diff --git a/src/legacy/ui/public/courier/fetch/request/search_request/__tests__/search_request.js b/src/legacy/ui/public/courier/fetch/request/search_request/__tests__/search_request.js deleted file mode 100644 index ecac8cd474098..0000000000000 --- a/src/legacy/ui/public/courier/fetch/request/search_request/__tests__/search_request.js +++ /dev/null @@ -1,78 +0,0 @@ -/* - * Licensed to Elasticsearch B.V. under one or more contributor - * license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright - * ownership. Elasticsearch B.V. licenses this file to you under - * the Apache License, Version 2.0 (the "License"); you may - * not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ - -import ngMock from 'ng_mock'; -import sinon from 'sinon'; -import expect from '@kbn/expect'; - -import { SearchRequestProvider } from '../search_request'; -import { searchRequestQueue } from '../../../../search_request_queue'; - -describe('ui/courier/fetch search request', () => { - beforeEach(ngMock.module('kibana')); - - afterEach(() => { - searchRequestQueue.removeAll(); - }); - - it('throws exception when created without errorHandler', ngMock.inject((Private) => { - const SearchReq = Private(SearchRequestProvider); - - let caughtError = false; - try { - new SearchReq({ source: {} }); - } catch(error) { - caughtError = true; - } - expect(caughtError).to.be(true); - })); - - 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, errorHandler: () => {} }); - expect(req.start()).to.have.property('then').a('function'); - sinon.assert.calledOnce(spy); - sinon.assert.calledWithExactly(spy, req); - })); - }); - - describe('clone', () => { - it('returns a search request with identical constructor arguments', ngMock.inject((Private) => { - const SearchRequest = Private(SearchRequestProvider); - - const source = {}; - const errorHandler = () => {}; - const defer = {}; - - const originalRequest = new SearchRequest({ source, errorHandler, defer }); - const clonedRequest = originalRequest.clone(); - - expect(clonedRequest).not.to.be(originalRequest); - expect(clonedRequest.source).to.be(source); - expect(clonedRequest.errorHandler).to.be(errorHandler); - expect(clonedRequest.defer).to.be(defer); - })); - - }); -}); diff --git a/src/legacy/ui/public/courier/fetch/request/search_request/search_request.js b/src/legacy/ui/public/courier/fetch/request/search_request/search_request.js deleted file mode 100644 index a6ce562e462d8..0000000000000 --- a/src/legacy/ui/public/courier/fetch/request/search_request/search_request.js +++ /dev/null @@ -1,205 +0,0 @@ -/* - * Licensed to Elasticsearch B.V. under one or more contributor - * license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright - * ownership. Elasticsearch B.V. licenses this file to you under - * the Apache License, Version 2.0 (the "License"); you may - * not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ - -import moment from 'moment'; - -import { searchRequestQueue } from '../../../search_request_queue'; - -import { createDefer } from 'ui/promises'; -import { i18n } from '@kbn/i18n'; - -export function SearchRequestProvider(Promise) { - class SearchRequest { - constructor({ source, defer, errorHandler }) { - if (!errorHandler) { - throw new Error( - i18n.translate('common.ui.courier.fetch.requireErrorHandlerErrorMessage', { - defaultMessage: '{errorHandler} is required', - values: { errorHandler: 'errorHandler' } - }) - ); - } - - this.errorHandler = errorHandler; - this.source = source; - this.defer = defer || createDefer(Promise); - this.abortedDefer = createDefer(Promise); - this.type = 'search'; - - // Track execution time. - this.moment = undefined; - this.ms = undefined; - - // Lifecycle state. - this.started = false; - this.stopped = false; - this._isFetchRequested = false; - - searchRequestQueue.add(this); - } - - /** - * Called by the searchPoll to find requests that should be sent to the - * fetchSoon module. When a module is sent to fetchSoon its _isFetchRequested flag - * is set, and this consults that flag so requests are not send to fetchSoon - * multiple times. - * - * @return {Boolean} - */ - canStart() { - if (this.source._fetchDisabled) { - return false; - } - - if (this.stopped) { - return false; - } - - if (this._isFetchRequested) { - return false; - } - - return true; - } - - /** - * Used to find requests that were previously sent to the fetchSoon module but - * have not been started yet, so they can be started. - * - * @return {Boolean} - */ - isFetchRequestedAndPending() { - if (this.started) { - return false; - } - - return this._isFetchRequested; - } - - /** - * Called by the fetchSoon module when this request has been sent to - * be fetched. At that point the request is somewhere between `ready-to-start` - * and `started`. The fetch module then waits a short period of time to - * allow requests to build up in the request queue, and then immediately - * fetches all requests that return true from `isFetchRequestedAndPending()` - * - * @return {undefined} - */ - _setFetchRequested() { - this._isFetchRequested = true; - } - - start() { - if (this.started) { - throw new TypeError( - i18n.translate('common.ui.courier.fetch.unableStartRequestErrorMessage', { - defaultMessage: 'Unable to start request because it has already started', - }) - ); - } - - this.started = true; - this.moment = moment(); - - return this.source.requestIsStarting(this); - } - - getFetchParams() { - return this.source._flatten(); - } - - filterError() { - return false; - } - - handleResponse(resp) { - this.success = true; - this.resp = resp; - } - - handleFailure(error) { - this.success = false; - this.resp = error; - this.resp = (error && error.resp) || error; - return this.errorHandler(this, error); - } - - isIncomplete() { - return false; - } - - continue() { - throw new Error( - i18n.translate('common.ui.courier.fetch.unableContinueRequestErrorMessage', { - defaultMessage: 'Unable to continue {type} request', - values: { type: this.type } - }) - ); - } - - retry() { - const clone = this.clone(); - this.abort(); - return clone; - } - - _markStopped() { - if (this.stopped) return; - this.stopped = true; - this.source.requestIsStopped(this); - searchRequestQueue.remove(this); - } - - abort() { - this._markStopped(); - this.aborted = true; - const error = new Error('The request was aborted.'); - error.name = 'AbortError'; - this.abortedDefer.resolve(error); - this.abortedDefer = null; - this.defer.reject(error); - this.defer = null; - } - - whenAborted(cb) { - this.abortedDefer.promise.then(cb); - } - - complete() { - this._markStopped(); - this.ms = this.moment.diff() * -1; - this.defer.resolve(this.resp); - } - - getCompletePromise() { - return this.defer.promise; - } - - getCompleteOrAbortedPromise() { - return Promise.race([ this.defer.promise, this.abortedDefer.promise ]); - } - - clone = () => { - const { source, defer, errorHandler } = this; - return new SearchRequest({ source, defer, errorHandler }); - }; - } - - return SearchRequest; -} diff --git a/src/legacy/ui/public/courier/fetch/request/serialize_fetch_params/index.js b/src/legacy/ui/public/courier/fetch/request/serialize_fetch_params/index.js deleted file mode 100644 index 807d53086e106..0000000000000 --- a/src/legacy/ui/public/courier/fetch/request/serialize_fetch_params/index.js +++ /dev/null @@ -1,20 +0,0 @@ -/* - * Licensed to Elasticsearch B.V. under one or more contributor - * license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright - * ownership. Elasticsearch B.V. licenses this file to you under - * the Apache License, Version 2.0 (the "License"); you may - * not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ - -export { SerializeFetchParamsProvider } from './serialize_fetch_params_provider'; diff --git a/src/legacy/ui/public/courier/fetch/request/serialize_fetch_params/serialize_fetch_params.js b/src/legacy/ui/public/courier/fetch/request/serialize_fetch_params/serialize_fetch_params.js deleted file mode 100644 index eed28d0a05b90..0000000000000 --- a/src/legacy/ui/public/courier/fetch/request/serialize_fetch_params/serialize_fetch_params.js +++ /dev/null @@ -1,60 +0,0 @@ -/* - * Licensed to Elasticsearch B.V. under one or more contributor - * license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright - * ownership. Elasticsearch B.V. licenses this file to you under - * the Apache License, Version 2.0 (the "License"); you may - * not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ - -import { getPreference, getTimeout } from '../../get_search_params'; - -/** - * - * @param requestsFetchParams {Array.} - * @param Promise - * @param sessionId - * @return {Promise.} - */ -export function serializeFetchParams( - requestsFetchParams, - Promise, - sessionId, - config, - esShardTimeout) { - const promises = requestsFetchParams.map(function (fetchParams) { - return Promise.resolve(fetchParams.index) - .then(function (indexPattern) { - const body = { - timeout: getTimeout(esShardTimeout), - ...fetchParams.body || {}, - }; - - const index = (indexPattern && indexPattern.title) ? indexPattern.title : indexPattern; - - const header = { - index, - search_type: fetchParams.search_type, - ignore_unavailable: true, - preference: getPreference(config, sessionId) - }; - - return `${JSON.stringify(header)}\n${JSON.stringify(body)}`; - }); - }); - - return Promise.all(promises).then(function (requests) { - return requests.join('\n') + '\n'; - }); -} - diff --git a/src/legacy/ui/public/courier/fetch/request/serialize_fetch_params/serialize_fetch_params.test.js b/src/legacy/ui/public/courier/fetch/request/serialize_fetch_params/serialize_fetch_params.test.js deleted file mode 100644 index 5f4c5bf9ef45a..0000000000000 --- a/src/legacy/ui/public/courier/fetch/request/serialize_fetch_params/serialize_fetch_params.test.js +++ /dev/null @@ -1,152 +0,0 @@ -/* - * Licensed to Elasticsearch B.V. under one or more contributor - * license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright - * ownership. Elasticsearch B.V. licenses this file to you under - * the Apache License, Version 2.0 (the "License"); you may - * not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ - -import { serializeFetchParams } from './serialize_fetch_params'; -import _ from 'lodash'; - -const DEFAULT_SESSION_ID = '1'; - -function serializeFetchParamsWithDefaults(paramOverrides) { - const paramDefaults = { - requestFetchParams: [], - Promise, - sessionId: DEFAULT_SESSION_ID, - config: { - get: () => { - return 'sessionId'; - } - }, - timeout: 100, - }; - const params = { ...paramDefaults, ...paramOverrides }; - - return serializeFetchParams( - params.requestFetchParams, - Promise, - params.sessionId, - params.config, - params.timeout, - ); -} - -describe('when indexList is not empty', () => { - test('includes the index', () => { - const requestFetchParams = [ - { - index: ['logstash-123'], - type: 'blah', - search_type: 'blah2', - body: { foo: 'bar', $foo: 'bar' } - } - ]; - return serializeFetchParamsWithDefaults({ requestFetchParams }).then(value => { - expect(_.includes(value, '"index":["logstash-123"]')).toBe(true); - }); - }); -}); - -describe('headers', () => { - - const requestFetchParams = [ - { - index: ['logstash-123'], - type: 'blah', - search_type: 'blah2', - body: { foo: 'bar' } - } - ]; - - const getHeader = async (paramOverrides) => { - const request = await serializeFetchParamsWithDefaults(paramOverrides); - const requestParts = request.split('\n'); - if (requestParts.length < 2) { - throw new Error('fetch Body does not contain expected format header newline body.'); - } - return JSON.parse(requestParts[0]); - }; - - describe('search request preference', () => { - test('should be set to sessionId when courier:setRequestPreference is "sessionId"', async () => { - const config = { - get: () => { - return 'sessionId'; - } - }; - const header = await getHeader({ requestFetchParams, config }); - expect(header.preference).toBe(DEFAULT_SESSION_ID); - }); - - test('should be set to custom string when courier:setRequestPreference is "custom"', async () => { - const CUSTOM_PREFERENCE = '_local'; - const config = { - get: (key) => { - if (key === 'courier:setRequestPreference') { - return 'custom'; - } else if (key === 'courier:customRequestPreference') { - return CUSTOM_PREFERENCE; - } - } - }; - const header = await getHeader({ requestFetchParams, config }); - expect(header.preference).toBe(CUSTOM_PREFERENCE); - }); - - test('should not be set when courier:setRequestPreference is "none"', async () => { - const config = { - get: () => { - return 'none'; - } - }; - const header = await getHeader({ requestFetchParams, config }); - expect(header.preference).toBe(undefined); - }); - }); -}); - -describe('body', () => { - const requestFetchParams = [ - { - index: ['logstash-123'], - type: 'blah', - search_type: 'blah2', - body: { foo: 'bar' } - } - ]; - - const getBody = async (paramOverrides) => { - const request = await serializeFetchParamsWithDefaults(paramOverrides); - const requestParts = request.split('\n'); - if (requestParts.length < 2) { - throw new Error('fetch Body does not contain expected format: header newline body.'); - } - return JSON.parse(requestParts[1]); - }; - - describe('timeout', () => { - test('should set a timeout as specified', async () => { - const request = await getBody({ requestFetchParams, timeout: 200 }); - expect(request).toHaveProperty('timeout', '200ms'); - }); - - test('should not set a timeout when timeout is 0', async () => { - const request = await getBody({ requestFetchParams, timeout: 0 }); - expect(request.timeout).toBe(undefined); - }); - }); -}); diff --git a/src/legacy/ui/public/courier/fetch/request/serialize_fetch_params/serialize_fetch_params_provider.js b/src/legacy/ui/public/courier/fetch/request/serialize_fetch_params/serialize_fetch_params_provider.js deleted file mode 100644 index 4ddcc05b927ff..0000000000000 --- a/src/legacy/ui/public/courier/fetch/request/serialize_fetch_params/serialize_fetch_params_provider.js +++ /dev/null @@ -1,31 +0,0 @@ -/* - * Licensed to Elasticsearch B.V. under one or more contributor - * license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright - * ownership. Elasticsearch B.V. licenses this file to you under - * the Apache License, Version 2.0 (the "License"); you may - * not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ - -import { serializeFetchParams } from './serialize_fetch_params'; - -export function SerializeFetchParamsProvider(Promise, sessionId, config, esShardTimeout) { - return (fetchParams) => ( - serializeFetchParams( - fetchParams, - Promise, - sessionId, - config, - esShardTimeout) - ); -} diff --git a/src/legacy/ui/public/courier/search_poll/index.js b/src/legacy/ui/public/courier/search_poll/index.js index 24813e3b58ea1..dabb1244da8fc 100644 --- a/src/legacy/ui/public/courier/search_poll/index.js +++ b/src/legacy/ui/public/courier/search_poll/index.js @@ -17,4 +17,4 @@ * under the License. */ -export { SearchPollProvider } from './search_poll'; +export { SearchPoll } from './search_poll'; diff --git a/src/legacy/ui/public/courier/search_poll/search_poll.js b/src/legacy/ui/public/courier/search_poll/search_poll.js index 91c866c14aa49..6b28ac9d29b23 100644 --- a/src/legacy/ui/public/courier/search_poll/search_poll.js +++ b/src/legacy/ui/public/courier/search_poll/search_poll.js @@ -19,23 +19,15 @@ import _ from 'lodash'; -import { fatalError } from '../../notify'; import '../../promises'; -import { searchRequestQueue } from '../search_request_queue'; -import { FetchSoonProvider } from '../fetch'; import { timefilter } from 'ui/timefilter'; -export function SearchPollProvider(Private, Promise) { - const fetchSoon = Private(FetchSoonProvider); - - class SearchPoll { - constructor() { - this._isPolling = false; - this._intervalInMs = undefined; - this._timerId = null; - this._searchPromise = null; - this._isIntervalFasterThanSearch = false; - } +export class SearchPoll { + constructor() { + this._isPolling = false; + this._intervalInMs = undefined; + this._timerId = null; + } setIntervalInMs = intervalInMs => { this._intervalInMs = _.parseInt(intervalInMs); @@ -69,48 +61,9 @@ export function SearchPollProvider(Private, Promise) { }; _search = () => { - // If our interval is faster than the rate at which searches return results, then trigger - // a new search as soon as the results come back. - if (this._searchPromise) { - this._isIntervalFasterThanSearch = true; - return; - } - // Schedule another search. this.resetTimer(); - // We use resolve() here instead of try() because the latter won't trigger a $digest - // when the promise resolves. - this._searchPromise = Promise.resolve().then(() => { - timefilter.notifyShouldFetch(); - const requests = searchRequestQueue.getInactive(); - - // The promise returned from fetchSearchRequests() only resolves when the requests complete. - // We want to continue even if the requests abort so we return a different promise. - fetchSoon.fetchSearchRequests(requests); - - return Promise.all( - requests.map(request => request.getCompleteOrAbortedPromise()) - ); - }) - .then(() => { - this._searchPromise = null; - - // If the search response comes back before the interval fires, then we'll wait - // for the interval and let it kick off the next search. But if the interval fires before - // the search returns results, then we'll need to wait for the search to return results - // and then kick off another search again. A new search will also reset the interval. - if (this._isIntervalFasterThanSearch) { - this._isIntervalFasterThanSearch = false; - this._search(); - } - }) - .catch(err => { - // If there was a problem, then kill Kibana. - fatalError(err); - }); + timefilter.notifyShouldFetch(); }; - } - - return new SearchPoll(); } diff --git a/src/legacy/ui/public/courier/fetch/request/search_request/index.js b/src/legacy/ui/public/courier/search_poll/search_poll.test.js similarity index 93% rename from src/legacy/ui/public/courier/fetch/request/search_request/index.js rename to src/legacy/ui/public/courier/search_poll/search_poll.test.js index 6647d0e5b2e10..81af3922ba9b3 100644 --- a/src/legacy/ui/public/courier/fetch/request/search_request/index.js +++ b/src/legacy/ui/public/courier/search_poll/search_poll.test.js @@ -17,4 +17,4 @@ * under the License. */ -export { SearchRequestProvider } from './search_request'; + diff --git a/src/legacy/ui/public/courier/search_request_queue/__tests__/search_request_queue.js b/src/legacy/ui/public/courier/search_request_queue/__tests__/search_request_queue.js deleted file mode 100644 index f6b4e4bef20c2..0000000000000 --- a/src/legacy/ui/public/courier/search_request_queue/__tests__/search_request_queue.js +++ /dev/null @@ -1,57 +0,0 @@ -/* - * Licensed to Elasticsearch B.V. under one or more contributor - * license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright - * ownership. Elasticsearch B.V. licenses this file to you under - * the Apache License, Version 2.0 (the "License"); you may - * not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ - -import ngMock from 'ng_mock'; -import expect from '@kbn/expect'; -import sinon from 'sinon'; - -import { searchRequestQueue } from '../search_request_queue'; - -describe('Courier Request Queue', function () { - beforeEach(ngMock.module('kibana')); - beforeEach(() => searchRequestQueue.removeAll()); - after(() => searchRequestQueue.removeAll()); - - class MockReq { - constructor(startable = true) { - this.source = {}; - this.canStart = sinon.stub().returns(startable); - } - } - - describe('#getStartable()', function () { - it('returns only startable requests', function () { - searchRequestQueue.add(new MockReq(false)); - searchRequestQueue.add(new MockReq(true)); - expect(searchRequestQueue.getStartable()).to.have.length(1); - }); - }); - - // Note: I'm not convinced this discrepancy between how we calculate startable vs inactive requests makes any sense. - // I'm only testing here that the current, (very old) code continues to behave how it always did, but it may turn out - // that we can clean this up, or remove this. - describe('#getInactive()', function () { - it('returns only requests with started = false', function () { - searchRequestQueue.add({ started: true }); - searchRequestQueue.add({ started: false }); - searchRequestQueue.add({ started: true }); - expect(searchRequestQueue.getInactive()).to.have.length(1); - }); - }); -}); diff --git a/src/legacy/ui/public/courier/search_request_queue/search_request_queue.js b/src/legacy/ui/public/courier/search_request_queue/search_request_queue.js deleted file mode 100644 index 80d74cdad94fe..0000000000000 --- a/src/legacy/ui/public/courier/search_request_queue/search_request_queue.js +++ /dev/null @@ -1,70 +0,0 @@ -/* - * Licensed to Elasticsearch B.V. under one or more contributor - * license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright - * ownership. Elasticsearch B.V. licenses this file to you under - * the Apache License, Version 2.0 (the "License"); you may - * not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ - -class SearchRequestQueue { - constructor() { - // Queue of pending requests, requests are removed as they are processed by fetch.[sourceType](). - this._searchRequests = []; - } - - getCount() { - return this._searchRequests.length; - } - - add(searchRequest) { - this._searchRequests.push(searchRequest); - } - - remove(searchRequest) { - // Remove all matching search requests. - this._searchRequests = this._searchRequests.filter( - existingSearchRequest => existingSearchRequest !== searchRequest - ); - } - - removeAll() { - this._searchRequests.length = 0; - } - - abortAll() { - this._searchRequests.forEach(searchRequest => searchRequest.abort()); - } - - getAll() { - return this._searchRequests; - } - - getSearchRequestAt(index) { - return this._searchRequests[index]; - } - - getInactive() { - return this._searchRequests.filter(searchRequest => !searchRequest.started); - } - - getStartable() { - return this._searchRequests.filter(searchRequest => searchRequest.canStart()); - } - - getPending() { - return this._searchRequests.filter(searchRequest => searchRequest.isFetchRequestedAndPending()); - } -} - -export const searchRequestQueue = new SearchRequestQueue(); diff --git a/src/legacy/ui/public/courier/search_source/__tests__/search_source.js b/src/legacy/ui/public/courier/search_source/__tests__/search_source.js deleted file mode 100644 index 727463bc8b86b..0000000000000 --- a/src/legacy/ui/public/courier/search_source/__tests__/search_source.js +++ /dev/null @@ -1,352 +0,0 @@ -/* - * Licensed to Elasticsearch B.V. under one or more contributor - * license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright - * ownership. Elasticsearch B.V. licenses this file to you under - * the Apache License, Version 2.0 (the "License"); you may - * not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ - -import ngMock from 'ng_mock'; -import expect from '@kbn/expect'; -import sinon from 'sinon'; - -import { searchRequestQueue } from '../../search_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(); - - let config; - let SearchSource; - let indexPattern; - let indexPattern2; - - beforeEach(ngMock.module('kibana')); - beforeEach(ngMock.inject(function (Private, _config_) { - config = _config_; - SearchSource = Private(SearchSourceProvider); - - const IndexPattern = Private(StubIndexPatternProv); - indexPattern = new IndexPattern('test-*', cfg => cfg, null, []); - indexPattern2 = new IndexPattern('test2-*', cfg => cfg, null, []); - expect(indexPattern).to.not.be(indexPattern2); - })); - beforeEach(() => searchRequestQueue.removeAll()); - after(() => searchRequestQueue.removeAll()); - - describe('#onResults()', function () { - it('adds a request to the searchRequestQueue', function () { - const searchSource = new SearchSource(); - - expect(searchRequestQueue.getCount()).to.be(0); - searchSource.onResults(); - expect(searchRequestQueue.getCount()).to.be(1); - }); - - it('returns a promise that is resolved with the results', function () { - const searchSource = new SearchSource(); - const fakeResults = {}; - - const promise = searchSource.onResults().then((results) => { - expect(results).to.be(fakeResults); - }); - - const searchRequest = searchRequestQueue.getSearchRequestAt(0); - searchRequest.defer.resolve(fakeResults); - return promise; - }); - }); - - describe('#destroy()', function () { - it('aborts all startable requests', function () { - const searchSource = new SearchSource(); - searchSource.onResults(); - const searchRequest = searchRequestQueue.getSearchRequestAt(0); - sinon.stub(searchRequest, 'canStart').returns(true); - searchSource.destroy(); - expect(searchRequestQueue.getCount()).to.be(0); - }); - - it('aborts all non-startable requests', function () { - const searchSource = new SearchSource(); - searchSource.onResults(); - const searchRequest = searchRequestQueue.getSearchRequestAt(0); - sinon.stub(searchRequest, 'canStart').returns(false); - searchSource.destroy(); - expect(searchRequestQueue.getCount()).to.be(0); - }); - }); - - describe('#setField()', function () { - it('sets the value for the property', function () { - const searchSource = new SearchSource(); - searchSource.setField('aggs', 5); - expect(searchSource.getField('aggs')).to.be(5); - }); - - it('throws an error if the property is not accepted', function () { - const searchSource = new SearchSource(); - expect(() => searchSource.setField('index', 5)).to.throwError(); - }); - }); - - describe('#getField()', function () { - it('gets the value for the property', function () { - const searchSource = new SearchSource(); - searchSource.setField('aggs', 5); - expect(searchSource.getField('aggs')).to.be(5); - }); - - it('throws an error if the property is not accepted', function () { - const searchSource = new SearchSource(); - expect(() => searchSource.getField('unacceptablePropName')).to.throwError(); - }); - }); - - describe(`#setField('index')`, function () { - describe('auto-sourceFiltering', function () { - describe('new index pattern assigned', function () { - it('generates a searchSource filter', function () { - const searchSource = new SearchSource(); - expect(searchSource.getField('index')).to.be(undefined); - expect(searchSource.getField('source')).to.be(undefined); - searchSource.setField('index', indexPattern); - expect(searchSource.getField('index')).to.be(indexPattern); - expect(searchSource.getField('source')).to.be.a('function'); - }); - - it('removes created searchSource filter on removal', function () { - const searchSource = new SearchSource(); - searchSource.setField('index', indexPattern); - searchSource.setField('index', null); - expect(searchSource.getField('index')).to.be(undefined); - expect(searchSource.getField('source')).to.be(undefined); - }); - }); - - describe('new index pattern assigned over another', function () { - it('replaces searchSource filter with new', function () { - const searchSource = new SearchSource(); - searchSource.setField('index', indexPattern); - const searchSourceFilter1 = searchSource.getField('source'); - searchSource.setField('index', indexPattern2); - expect(searchSource.getField('index')).to.be(indexPattern2); - expect(searchSource.getField('source')).to.be.a('function'); - expect(searchSource.getField('source')).to.not.be(searchSourceFilter1); - }); - - it('removes created searchSource filter on removal', function () { - const searchSource = new SearchSource(); - searchSource.setField('index', indexPattern); - searchSource.setField('index', indexPattern2); - searchSource.setField('index', null); - expect(searchSource.getField('index')).to.be(undefined); - expect(searchSource.getField('source')).to.be(undefined); - }); - }); - - describe('ip assigned before custom searchSource filter', function () { - it('custom searchSource filter becomes new searchSource', function () { - const searchSource = new SearchSource(); - const football = {}; - searchSource.setField('index', indexPattern); - expect(searchSource.getField('source')).to.be.a('function'); - searchSource.setField('source', football); - expect(searchSource.getField('index')).to.be(indexPattern); - expect(searchSource.getField('source')).to.be(football); - }); - - it('custom searchSource stays after removal', function () { - const searchSource = new SearchSource(); - const football = {}; - searchSource.setField('index', indexPattern); - searchSource.setField('source', football); - searchSource.setField('index', null); - expect(searchSource.getField('index')).to.be(undefined); - expect(searchSource.getField('source')).to.be(football); - }); - }); - - describe('ip assigned after custom searchSource filter', function () { - it('leaves the custom filter in place', function () { - const searchSource = new SearchSource(); - const football = {}; - searchSource.setField('source', football); - searchSource.setField('index', indexPattern); - expect(searchSource.getField('index')).to.be(indexPattern); - expect(searchSource.getField('source')).to.be(football); - }); - - it('custom searchSource stays after removal', function () { - const searchSource = new SearchSource(); - const football = {}; - searchSource.setField('source', football); - searchSource.setField('index', indexPattern); - searchSource.setField('index', null); - expect(searchSource.getField('index')).to.be(undefined); - expect(searchSource.getField('source')).to.be(football); - }); - }); - }); - }); - - describe('#onRequestStart()', () => { - it('should be called when starting a request', async () => { - const searchSource = new SearchSource(); - const fn = sinon.spy(); - searchSource.onRequestStart(fn); - const request = {}; - searchSource.requestIsStarting(request); - await timeout(); - expect(fn.calledWith(searchSource, request)).to.be(true); - }); - - it('should not be called on parent searchSource', async () => { - const parent = new SearchSource(); - const searchSource = new SearchSource().setParent(parent); - - const fn = sinon.spy(); - searchSource.onRequestStart(fn); - const parentFn = sinon.spy(); - parent.onRequestStart(parentFn); - const request = {}; - searchSource.requestIsStarting(request); - await timeout(); - expect(fn.calledWith(searchSource, 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 searchSource = new SearchSource().setParent(parent, { callParentStartHandlers: true }); - - const fn = sinon.spy(); - searchSource.onRequestStart(fn); - const parentFn = sinon.spy(); - parent.onRequestStart(parentFn); - const request = {}; - searchSource.requestIsStarting(request); - await timeout(); - expect(fn.calledWith(searchSource, request)).to.be(true); - expect(parentFn.calledWith(searchSource, request)).to.be(true); - }); - }); - - describe('#_mergeProp', function () { - describe('filter', function () { - let searchSource; - let state; - - beforeEach(function () { - searchSource = new SearchSource(); - state = {}; - }); - - [null, undefined].forEach(falsyValue => { - it(`ignores ${falsyValue} filter`, function () { - searchSource._mergeProp(state, falsyValue, 'filter'); - expect(state.filters).to.be(undefined); - }); - }); - - [false, 0, '', NaN].forEach(falsyValue => { - it(`doesn't add ${falsyValue} filter`, function () { - searchSource._mergeProp(state, falsyValue, 'filter'); - expect(state.filters).to.be.empty(); - }); - }); - - it('adds "meta.disabled: undefined" filter', function () { - const filter = { - meta: {} - }; - searchSource._mergeProp(state, filter, 'filter'); - expect(state.filters).to.eql([filter]); - }); - - it('adds "meta.disabled: false" filter', function () { - const filter = { - meta: { - disabled: false - } - }; - searchSource._mergeProp(state, filter, 'filter'); - expect(state.filters).to.eql([filter]); - }); - - it(`doesn't add "meta.disabled: true" filter`, function () { - const filter = { - meta: { - disabled: true - } - }; - searchSource._mergeProp(state, filter, 'filter'); - expect(state.filters).to.be.empty(); - }); - - describe('when courier:ignoreFilterIfFieldNotInIndex is false', function () { - it('adds filter for non-existent field', function () { - config.set('courier:ignoreFilterIfFieldNotInIndex', false); - const filter = { - meta: { - key: 'bar' - } - }; - state.index = { - fields: [] - }; - searchSource._mergeProp(state, filter, 'filter'); - expect(state.filters).to.eql([ filter ]); - }); - }); - - describe('when courier:ignoreFilterIfFieldNotInIndex is true', function () { - it(`doesn't add filter for non-existent field`, function () { - config.set('courier:ignoreFilterIfFieldNotInIndex', true); - const filter = { - meta: { - key: 'bar' - } - }; - state.index = { - fields: [] - }; - searchSource._mergeProp(state, filter, 'filter'); - expect(state.filters).to.be.empty(); - }); - - it(`adds filter for existent field`, function () { - config.set('courier:ignoreFilterIfFieldNotInIndex', true); - const filter = { - meta: { - key: 'bar' - } - }; - state.index = { - fields: [{ name: 'bar' }] - }; - searchSource._mergeProp(state, filter, 'filter'); - expect(state.filters).to.eql([ filter ]); - }); - }); - }); - }); -}); diff --git a/src/legacy/ui/public/courier/search_source/search_source.js b/src/legacy/ui/public/courier/search_source/search_source.js index 4a2c441012b50..007c5ee617c88 100644 --- a/src/legacy/ui/public/courier/search_source/search_source.js +++ b/src/legacy/ui/public/courier/search_source/search_source.js @@ -73,11 +73,8 @@ import _ from 'lodash'; import angular from 'angular'; import { buildEsQuery, getEsQueryConfig, filterMatchesIndex } from '@kbn/es-query'; -import { createDefer } from 'ui/promises'; import { NormalizeSortRequestProvider } from './_normalize_sort_request'; -import { SearchRequestProvider } from '../fetch/request'; -import { searchRequestQueue } from '../search_request_queue'; import { FetchSoonProvider } from '../fetch'; import { FieldWildcardProvider } from '../../field_wildcard'; import { getHighlightRequest } from '../../../../core_plugins/kibana/common/highlight'; @@ -114,7 +111,6 @@ function isIndexPattern(val) { } export function SearchSourceProvider(Promise, Private, config) { - const SearchRequest = Private(SearchRequestProvider); const normalizeSortRequest = Private(NormalizeSortRequestProvider); const fetchSoon = Private(FetchSoonProvider); const { fieldWildcardFilter } = Private(FieldWildcardProvider); @@ -299,39 +295,19 @@ export function SearchSourceProvider(Promise, Private, config) { * * @async */ - fetch() { - const self = this; - let req = _.first(self._myStartableQueued()); - - if (!req) { - const errorHandler = (request, error) => { - request.defer.reject(error); - request.abort(); - }; - req = self._createRequest({ errorHandler }); - } - - fetchSoon.fetchSearchRequests([req]); - return req.getCompletePromise(); - } - - /** - * Fetch all pending requests for this source ASAP - * @async - */ - fetchQueued() { - return fetchSoon.fetchSearchRequests(this._myStartableQueued()); + async fetch(options) { + const searchRequest = await this._flatten(); + return fetchSoon.fetch(searchRequest, { + ...(this._searchStrategyId && { searchStrategyId: this._searchStrategyId }), + ...options, + }); } /** * Cancel all pending requests for this searchSource * @return {undefined} */ - cancelQueued() { - searchRequestQueue.getAll() - .filter(req => req.source === this) - .forEach(req => req.abort()); - } + cancelQueued() {} /** * Add a handler that will be notified whenever requests start @@ -367,40 +343,11 @@ export function SearchSourceProvider(Promise, Private, config) { .then(_.noop); } - /** - * Put a request in to the courier that this Source should - * be fetched on the next run of the courier - * @return {Promise} - */ - onResults() { - const self = this; - - return new Promise(function (resolve, reject) { - const defer = createDefer(Promise); - defer.promise.then(resolve, reject); - - const errorHandler = (request, error) => { - reject(error); - request.abort(); - }; - self._createRequest({ defer, errorHandler }); - }); - } - async getSearchRequestBody() { const searchRequest = await this._flatten(); return searchRequest.body; } - /** - * Called by requests of this search source when they are done - * @param {Courier.Request} request - * @return {undefined} - */ - requestIsStopped() { - this.activeFetchCount -= 1; - } - /** * Completely destroy the SearchSource. * @return {undefined} @@ -414,25 +361,6 @@ export function SearchSourceProvider(Promise, Private, config) { * PRIVATE APIS ******/ - _myStartableQueued() { - return searchRequestQueue - .getStartable() - .filter(req => req.source === this); - } - - /** - * Create a common search request object, which should - * be put into the pending request queue, for this search - * source - * - * @param {Deferred} defer - the deferred object that should be resolved - * when the request is complete - * @return {SearchRequest} - */ - _createRequest({ defer, errorHandler }) { - return new SearchRequest({ source: this, defer, errorHandler }); - } - /** * Used to merge properties into the data within ._flatten(). * The data is passed in and modified by the function diff --git a/src/legacy/ui/public/courier/search_request_queue/index.js b/src/legacy/ui/public/courier/search_source/search_source.test.js similarity index 92% rename from src/legacy/ui/public/courier/search_request_queue/index.js rename to src/legacy/ui/public/courier/search_source/search_source.test.js index 785a59fce73d5..81af3922ba9b3 100644 --- a/src/legacy/ui/public/courier/search_request_queue/index.js +++ b/src/legacy/ui/public/courier/search_source/search_source.test.js @@ -17,4 +17,4 @@ * under the License. */ -export { searchRequestQueue } from './search_request_queue'; + diff --git a/src/legacy/ui/public/courier/search_strategy/default_search_strategy.js b/src/legacy/ui/public/courier/search_strategy/default_search_strategy.js index 4b1f488ece128..27dd705b566b9 100644 --- a/src/legacy/ui/public/courier/search_strategy/default_search_strategy.js +++ b/src/legacy/ui/public/courier/search_strategy/default_search_strategy.js @@ -19,48 +19,13 @@ import { addSearchStrategy } from './search_strategy_registry'; import { isDefaultTypeIndexPattern } from './is_default_type_index_pattern'; -import { SearchError } from './search_error'; -import { getSearchParams, getMSearchParams } from '../fetch/get_search_params'; - -function getAllFetchParams(searchRequests, Promise) { - return Promise.map(searchRequests, (searchRequest) => { - return Promise.try(searchRequest.getFetchParams, void 0, searchRequest) - .then((fetchParams) => { - return (searchRequest.fetchParams = fetchParams); - }) - .then(value => ({ resolved: value })) - .catch(error => ({ rejected: error })); - }); -} - -async function serializeAllFetchParams(fetchParams, searchRequests, serializeFetchParams) { - const searchRequestsWithFetchParams = []; - const failedSearchRequests = []; - - // Gather the fetch param responses from all the successful requests. - fetchParams.forEach((result, index) => { - if (result.resolved) { - searchRequestsWithFetchParams.push(result.resolved); - } else { - const searchRequest = searchRequests[index]; - - searchRequest.handleFailure(result.rejected); - failedSearchRequests.push(searchRequest); - } - }); - - return { - serializedFetchParams: await serializeFetchParams(searchRequestsWithFetchParams), - failedSearchRequests, - }; -} +import { getSearchParams, getMSearchParams, getPreference, getTimeout } from '../fetch/get_search_params'; export const defaultSearchStrategy = { id: 'default', search: params => { - const { config } = params; - return config.get('courier:batchSearches') ? msearch(params) : search(params); + return params.config.get('courier:batchSearches') ? msearch(params) : search(params); }, isViable: (indexPattern) => { @@ -72,79 +37,40 @@ export const defaultSearchStrategy = { }, }; -async function msearch({ searchRequests, es, Promise, serializeFetchParams, config }) { - // Flatten the searchSource within each searchRequest to get the fetch params, - // e.g. body, filters, index pattern, query. - const allFetchParams = await getAllFetchParams(searchRequests, Promise); - - // Serialize the fetch params into a format suitable for the body of an ES query. - const { - serializedFetchParams, - failedSearchRequests, - } = await serializeAllFetchParams(allFetchParams, searchRequests, serializeFetchParams); - - if (serializedFetchParams.trim() === '') { - return { - failedSearchRequests, +function msearch({ searchRequests, es, config, sessionId, esShardTimeout }) { + const inlineRequests = searchRequests.map(({ index, body, search_type: searchType }) => { + const inlineHeader = { + index: index.title || index, + search_type: searchType, + ignore_unavailable: true, + preference: getPreference(config, sessionId) }; - } - const msearchParams = { - ...getMSearchParams(config), - body: serializedFetchParams, - }; - - const searching = es.msearch(msearchParams); - - return { - // Munge data into shape expected by consumer. - searching: new Promise((resolve, reject) => { - // Unwrap the responses object returned by the ES client. - searching.then(({ responses }) => { - resolve(responses); - }).catch(error => { - // Format ES client error as a SearchError. - const { statusCode, displayName, message, path } = error; - - const searchError = new SearchError({ - status: statusCode, - title: displayName, - message, - path, - }); + const inlineBody = { + ...body, + timeout: getTimeout(esShardTimeout) + }; + return `${JSON.stringify(inlineHeader)}\n${JSON.stringify(inlineBody)}`; + }); - reject(searchError); - }); - }), - abort: searching.abort, - failedSearchRequests, - }; + const searching = es.msearch({ + ...getMSearchParams(config), + body: `${inlineRequests.join('\n')}\n`, + }).then(({ responses }) => responses); + return { searching, abort: searching.abort }; } function search({ searchRequests, es, Promise, config, sessionId, esShardTimeout }) { - const failedSearchRequests = []; const abortController = new AbortController(); const searchParams = getSearchParams(config, sessionId, esShardTimeout); - const promises = searchRequests.map(async searchRequest => { - return searchRequest.getFetchParams() - .then(fetchParams => { - const { index, body } = searchRequest.fetchParams = fetchParams; - const promise = es.search({ index: index.title || index, body, ...searchParams }); - abortController.signal.addEventListener('abort', promise.abort); - return promise; - }, error => { - searchRequest.handleFailure(error); - failedSearchRequests.push(searchRequest); - }) - .catch(({ response }) => { - // Copying the _msearch behavior where the errors for individual requests are returned - // instead of thrown - return JSON.parse(response); - }); + const promises = searchRequests.map(({ index, body }) => { + const searching = es.search({ index: index.title || index, body, ...searchParams }) + .catch(({ response }) => JSON.parse(response)); + abortController.signal.addEventListener('abort', searching.abort); + return searching; }); return { searching: Promise.all(promises), abort: () => abortController.abort(), - failedSearchRequests }; } diff --git a/src/legacy/ui/public/courier/search_strategy/index.js b/src/legacy/ui/public/courier/search_strategy/index.js index 3f6d172426d0d..229d0cbb1da5d 100644 --- a/src/legacy/ui/public/courier/search_strategy/index.js +++ b/src/legacy/ui/public/courier/search_strategy/index.js @@ -18,9 +18,10 @@ */ export { - assignSearchRequestsToSearchStrategies, addSearchStrategy, hasSearchStategyForIndexPattern, + getSearchStrategyById, + getSearchStrategyForSearchRequest, } from './search_strategy_registry'; export { isDefaultTypeIndexPattern } from './is_default_type_index_pattern'; diff --git a/src/legacy/ui/public/courier/search_strategy/search_strategy_registry.js b/src/legacy/ui/public/courier/search_strategy/search_strategy_registry.js index 3af93e4f16509..565a7d1c52927 100644 --- a/src/legacy/ui/public/courier/search_strategy/search_strategy_registry.js +++ b/src/legacy/ui/public/courier/search_strategy/search_strategy_registry.js @@ -35,22 +35,20 @@ const getSearchStrategyByViability = indexPattern => { }); }; -const getSearchStrategyById = searchStrategyId => { +export const getSearchStrategyById = searchStrategyId => { return searchStrategies.find(searchStrategy => { return searchStrategy.id === searchStrategyId; }); }; -const getSearchStrategyForSearchRequest = searchRequest => { +export const getSearchStrategyForSearchRequest = (searchRequest, { searchStrategyId } = {}) => { // Allow the searchSource to declare the correct strategy with which to execute its searches. - const preferredSearchStrategyId = searchRequest.source.getPreferredSearchStrategyId(); - if (preferredSearchStrategyId != null) { - return getSearchStrategyById(preferredSearchStrategyId); + if (searchStrategyId != null) { + return getSearchStrategyById(searchStrategyId); } // Otherwise try to match it to a strategy. - const indexPattern = searchRequest.source.getField('index'); - const viableSearchStrategy = getSearchStrategyByViability(indexPattern); + const viableSearchStrategy = getSearchStrategyByViability(searchRequest.index); if (viableSearchStrategy) { return viableSearchStrategy; @@ -60,47 +58,6 @@ const getSearchStrategyForSearchRequest = searchRequest => { return noOpSearchStrategy; }; - -/** - * Build a structure like this: - * - * [{ - * searchStrategy: rollupSearchStrategy, - * searchRequests: [], - * }, { - * searchStrategy: defaultSearchStrategy, - * searchRequests: [], - * }] - * - * We use an array of objects to preserve the order of the search requests, which we use to - * deterministically associate each response with the originating request. - */ -export const assignSearchRequestsToSearchStrategies = searchRequests => { - const searchStrategiesWithRequests = []; - const searchStrategyById = {}; - - searchRequests.forEach(searchRequest => { - const matchingSearchStrategy = getSearchStrategyForSearchRequest(searchRequest); - const { id } = matchingSearchStrategy; - let searchStrategyWithRequest = searchStrategyById[id]; - - // Create the data structure if we don't already have it. - if (!searchStrategyWithRequest) { - searchStrategyWithRequest = { - searchStrategy: matchingSearchStrategy, - searchRequests: [], - }; - - searchStrategyById[id] = searchStrategyWithRequest; - searchStrategiesWithRequests.push(searchStrategyWithRequest); - } - - searchStrategyWithRequest.searchRequests.push(searchRequest); - }); - - return searchStrategiesWithRequests; -}; - export const hasSearchStategyForIndexPattern = indexPattern => { return Boolean(getSearchStrategyByViability(indexPattern)); }; diff --git a/src/legacy/ui/public/courier/search_strategy/search_strategy_registry.test.js b/src/legacy/ui/public/courier/search_strategy/search_strategy_registry.test.js index 5f7e14082d577..24b8e23e6a1cc 100644 --- a/src/legacy/ui/public/courier/search_strategy/search_strategy_registry.test.js +++ b/src/legacy/ui/public/courier/search_strategy/search_strategy_registry.test.js @@ -17,79 +17,5 @@ * under the License. */ -import { - assignSearchRequestsToSearchStrategies, - addSearchStrategy, -} from './search_strategy_registry'; - -import { noOpSearchStrategy } from './no_op_search_strategy'; - describe('SearchStrategyRegistry', () => { - describe('assignSearchRequestsToSearchStrategies', () => { - test('associates search requests with valid search strategies', () => { - const searchStrategyA = { - id: 'a', - isViable: indexPattern => { - return indexPattern === 'a'; - }, - }; - - addSearchStrategy(searchStrategyA); - - const searchStrategyB = { - id: 'b', - isViable: indexPattern => { - return indexPattern === 'b'; - }, - }; - - addSearchStrategy(searchStrategyB); - - const searchRequest0 = { - id: 0, - source: { getField: () => 'b', getPreferredSearchStrategyId: () => {} }, - }; - - const searchRequest1 = { - id: 1, - source: { getField: () => 'a', getPreferredSearchStrategyId: () => {} }, - }; - - const searchRequest2 = { - id: 2, - source: { getField: () => 'a', getPreferredSearchStrategyId: () => {} }, - }; - - const searchRequest3 = { - id: 3, - source: { getField: () => 'b', getPreferredSearchStrategyId: () => {} }, - }; - - const searchRequests = [ searchRequest0, searchRequest1, searchRequest2, searchRequest3]; - const searchStrategiesWithSearchRequests = assignSearchRequestsToSearchStrategies(searchRequests); - - expect(searchStrategiesWithSearchRequests).toEqual([{ - searchStrategy: searchStrategyB, - searchRequests: [ searchRequest0, searchRequest3 ], - }, { - searchStrategy: searchStrategyA, - searchRequests: [ searchRequest1, searchRequest2 ], - }]); - }); - - test(`associates search requests with noOpSearchStrategy when a viable one can't be found`, () => { - const searchRequest0 = { - id: 0, - source: { getField: () => {}, getPreferredSearchStrategyId: () => {} }, - }; - - const searchRequests = [ searchRequest0 ]; - const searchStrategiesWithSearchRequests = assignSearchRequestsToSearchStrategies(searchRequests); - - expect(searchStrategiesWithSearchRequests).toEqual([{ - searchStrategy: noOpSearchStrategy, - searchRequests: [ searchRequest0 ], - }]); - }); - }); }); diff --git a/src/legacy/ui/public/error_allow_explicit_index/_error_allow_explicit_index.scss b/src/legacy/ui/public/error_allow_explicit_index/_error_allow_explicit_index.scss deleted file mode 100644 index 769abea150199..0000000000000 --- a/src/legacy/ui/public/error_allow_explicit_index/_error_allow_explicit_index.scss +++ /dev/null @@ -1,3 +0,0 @@ -.kbnError--multi-allow-explicit-index { - padding: $euiSizeL; -} diff --git a/src/legacy/ui/public/error_allow_explicit_index/_index.scss b/src/legacy/ui/public/error_allow_explicit_index/_index.scss deleted file mode 100644 index 84cb111127679..0000000000000 --- a/src/legacy/ui/public/error_allow_explicit_index/_index.scss +++ /dev/null @@ -1 +0,0 @@ -@import './error_allow_explicit_index'; diff --git a/src/legacy/ui/public/error_allow_explicit_index/error_allow_explicit_index.html b/src/legacy/ui/public/error_allow_explicit_index/error_allow_explicit_index.html deleted file mode 100644 index e61383b11101a..0000000000000 --- a/src/legacy/ui/public/error_allow_explicit_index/error_allow_explicit_index.html +++ /dev/null @@ -1,48 +0,0 @@ -
-

- - - -

- -

- -

- -

-
    -
  1. -
  2. -
  3. -
-
diff --git a/src/legacy/ui/public/error_allow_explicit_index/error_allow_explicit_index.js b/src/legacy/ui/public/error_allow_explicit_index/error_allow_explicit_index.js deleted file mode 100644 index 35763d8dd0385..0000000000000 --- a/src/legacy/ui/public/error_allow_explicit_index/error_allow_explicit_index.js +++ /dev/null @@ -1,57 +0,0 @@ -/* - * Licensed to Elasticsearch B.V. under one or more contributor - * license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright - * ownership. Elasticsearch B.V. licenses this file to you under - * the Apache License, Version 2.0 (the "License"); you may - * not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ - -import { i18n } from '@kbn/i18n'; -import { get } from 'lodash'; - -import uiRoutes from '../routes'; -import { KbnUrlProvider } from '../url'; - -import template from './error_allow_explicit_index.html'; - -uiRoutes - .when('/error/multi.allow_explicit_index', { - template, - k7Breadcrumbs: () => [{ text: i18n.translate('common.ui.errorAllowExplicitIndex.breadcrumbs.errorText', { defaultMessage: 'Error' }) }], - }); - -export function ErrorAllowExplicitIndexProvider(Private, Promise) { - const kbnUrl = Private(KbnUrlProvider); - - return new (class ErrorAllowExplicitIndex { - test(error) { - if (!error || error.status !== 400) { - return false; - } - - const type = get(error, 'body.error.type'); - const reason = get(error, 'body.error.reason'); - - return ( - type === 'illegal_argument_exception' && - String(reason).includes('explicit index') - ); - } - - takeover() { - kbnUrl.change('/error/multi.allow_explicit_index'); - return Promise.halt(); - } - }); -} diff --git a/x-pack/legacy/plugins/rollup/public/search/rollup_search_strategy.js b/x-pack/legacy/plugins/rollup/public/search/rollup_search_strategy.js index abc0bc620b81a..ab24a37a2ecec 100644 --- a/x-pack/legacy/plugins/rollup/public/search/rollup_search_strategy.js +++ b/x-pack/legacy/plugins/rollup/public/search/rollup_search_strategy.js @@ -7,43 +7,8 @@ import { kfetch } from 'ui/kfetch'; import { SearchError, getSearchErrorType } from 'ui/courier'; -function getAllFetchParams(searchRequests, Promise) { - return Promise.map(searchRequests, (searchRequest) => { - return Promise.try(searchRequest.getFetchParams, void 0, searchRequest) - .then((fetchParams) => { - return (searchRequest.fetchParams = fetchParams); - }) - .then(value => ({ resolved: value })) - .catch(error => ({ rejected: error })); - }); -} - -function serializeAllFetchParams(fetchParams, searchRequests) { - const searchRequestsWithFetchParams = []; - const failedSearchRequests = []; - - // Gather the fetch param responses from all the successful requests. - fetchParams.forEach((result, index) => { - if (result.resolved) { - searchRequestsWithFetchParams.push(result.resolved); - } else { - const searchRequest = searchRequests[index]; - - searchRequest.handleFailure(result.rejected); - failedSearchRequests.push(searchRequest); - } - }); - - const serializedFetchParams = serializeFetchParams(searchRequestsWithFetchParams); - - return { - serializedFetchParams, - failedSearchRequests, - }; -} - -function serializeFetchParams(searchRequestsWithFetchParams) { - return JSON.stringify(searchRequestsWithFetchParams.map(searchRequestWithFetchParams => { +function serializeFetchParams(searchRequests) { + return JSON.stringify(searchRequests.map(searchRequestWithFetchParams => { const indexPattern = searchRequestWithFetchParams.index.title || searchRequestWithFetchParams.index; const { body: { @@ -84,16 +49,9 @@ function shimHitsInFetchResponse(response) { export const rollupSearchStrategy = { id: 'rollup', - search: async ({ searchRequests, Promise }) => { - // Flatten the searchSource within each searchRequest to get the fetch params, - // e.g. body, filters, index pattern, query. - const allFetchParams = await getAllFetchParams(searchRequests, Promise); - + search: ({ searchRequests, Promise }) => { // Serialize the fetch params into a format suitable for the body of an ES query. - const { - serializedFetchParams, - failedSearchRequests, - } = await serializeAllFetchParams(allFetchParams, searchRequests); + const serializedFetchParams = serializeFetchParams(searchRequests); const controller = new AbortController(); const promise = kfetch({ @@ -124,7 +82,6 @@ export const rollupSearchStrategy = { return Promise.reject(searchError); }), abort: () => controller.abort(), - failedSearchRequests, }; }, diff --git a/x-pack/legacy/plugins/rollup/public/search/rollup_search_strategy.test.js b/x-pack/legacy/plugins/rollup/public/search/rollup_search_strategy.test.js new file mode 100644 index 0000000000000..8171e62b95585 --- /dev/null +++ b/x-pack/legacy/plugins/rollup/public/search/rollup_search_strategy.test.js @@ -0,0 +1,7 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + + diff --git a/x-pack/plugins/translations/translations/ja-JP.json b/x-pack/plugins/translations/translations/ja-JP.json index b8938d2cae174..177ad62933e1a 100644 --- a/x-pack/plugins/translations/translations/ja-JP.json +++ b/x-pack/plugins/translations/translations/ja-JP.json @@ -301,14 +301,6 @@ "common.ui.dualRangeControl.mustSetBothErrorMessage": "下と上の値の両方を設定する必要があります", "common.ui.dualRangeControl.outsideOfRangeErrorMessage": "値は {min} と {max} の間でなければなりません", "common.ui.dualRangeControl.upperValidErrorMessage": "上の値は下の値以上でなければなりません", - "common.ui.errorAllowExplicitIndex.breadcrumbs.errorText": "エラー", - "common.ui.errorAllowExplicitIndex.errorDescription": "ご使用の Elasticsearch クラスターの {allowExplicitIndexConfig} 設定が {allowExplicitIndexValue} に設定されているようです。これにより Kibana が検索リクエストを行うことができません。この機能は、ダッシュボードに多数のパネルがある際に素早く一貫して読み込まれるように、Elasticsearch に複数インデックスを検索する単独のリクエストを送るのに使用します。", - "common.ui.errorAllowExplicitIndex.errorDisclaimer": "申し訳ございませんが、この問題が解決されるまでディスカバリ、可視化、ダッシュボードなどの Kibana の特定のアプリはご利用いただけません。", - "common.ui.errorAllowExplicitIndex.errorTitle": "おっと！", - "common.ui.errorAllowExplicitIndex.howToFix.goBackText": "ブラウザの戻るボタンで前の画面に戻ります。", - "common.ui.errorAllowExplicitIndex.howToFix.removeConfigItemText": "Elasticsearch の構成ファイルから {allowExplicitIndexConfig} を削除します。", - "common.ui.errorAllowExplicitIndex.howToFix.restartText": "Elasticsearch を再起動します。", - "common.ui.errorAllowExplicitIndex.howToFixErrorTitle": "どうすれば良いのでしょう？", "common.ui.errorAutoCreateIndex.breadcrumbs.errorText": "エラー", "common.ui.errorAutoCreateIndex.errorDescription": "Elasticsearch クラスターの {autoCreateIndexActionConfig} 設定が原因で、Kibana が保存されたオブジェクトを格納するインデックスを自動的に作成できないようです。Kibana は、保存されたオブジェクトインデックスが適切なマッピング/スキーマを使用し Kibana から Elasticsearch へのポーリングの回数を減らすための最適な手段であるため、この Elasticsearch の機能を使用します。", "common.ui.errorAutoCreateIndex.errorDisclaimer": "申し訳ございませんが、この問題が解決されるまで Kibana で何も保存することができません。", diff --git a/x-pack/plugins/translations/translations/zh-CN.json b/x-pack/plugins/translations/translations/zh-CN.json index 950fe053ca5e6..18c0987baf17a 100644 --- a/x-pack/plugins/translations/translations/zh-CN.json +++ b/x-pack/plugins/translations/translations/zh-CN.json @@ -301,14 +301,6 @@ "common.ui.dualRangeControl.mustSetBothErrorMessage": "下限值和上限值都须设置", "common.ui.dualRangeControl.outsideOfRangeErrorMessage": "值必须是在 {min} 到 {max} 的范围内", "common.ui.dualRangeControl.upperValidErrorMessage": "上限值必须大于或等于下限值", - "common.ui.errorAllowExplicitIndex.breadcrumbs.errorText": "错误", - "common.ui.errorAllowExplicitIndex.errorDescription": "似乎您的 Elasticsearch 集群已将设置 {allowExplicitIndexConfig} 设置为 {allowExplicitIndexValue}，这使 Kibana 无法执行搜索请求。使用此功能，我们可以向 Elasticsearch 发送单个请求来搜索多个索引，这样，当仪表板上有多个面板时，面板可快速且一致地加载。", - "common.ui.errorAllowExplicitIndex.errorDisclaimer": "但是，只有解决了此问题后，您才能使用 Kibana 中的某些应用，如 Discover、Visualize 和仪表板。", - "common.ui.errorAllowExplicitIndex.errorTitle": "糟糕！", - "common.ui.errorAllowExplicitIndex.howToFix.goBackText": "使用浏览器的后退按钮返回您之前正做的工作。", - "common.ui.errorAllowExplicitIndex.howToFix.removeConfigItemText": "从 Elasticsearch 配置文件中删除 {allowExplicitIndexConfig}", - "common.ui.errorAllowExplicitIndex.howToFix.restartText": "重新启动 Elasticsearch。", - "common.ui.errorAllowExplicitIndex.howToFixErrorTitle": "那么，我如何解决此问题？", "common.ui.errorAutoCreateIndex.breadcrumbs.errorText": "错误", "common.ui.errorAutoCreateIndex.errorDescription": "似乎 Elasticsearch 集群的 {autoCreateIndexActionConfig} 设置使 Kibana 无法自动创建用于存储已保存对象的索引。Kibana 将使用此 Elasticsearch 功能，因为这是确保已保存对象索引使用正确映射/架构的最好方式，而且其允许 Kibana 较少地轮询 Elasticsearch。", "common.ui.errorAutoCreateIndex.errorDisclaimer": "但是，只有解决了此问题后，您才能在 Kibana 保存内容。", From 039c507162895812f802c5561da5e510e51c621e Mon Sep 17 00:00:00 2001 From: Lukas Olson Date: Tue, 10 Sep 2019 14:20:19 -0700 Subject: [PATCH 02/25] Add abort signal to search source fetch and remove cancel queued --- .../public/control/list_control_factory.js | 4 ++-- .../public/control/range_control_factory.js | 4 ++-- .../public/discover/controllers/discover.js | 14 ++++++++++---- .../discover/embeddable/search_embeddable.ts | 10 +++++++--- src/legacy/ui/public/_index.scss | 1 - .../ui/public/agg_types/buckets/histogram.js | 6 ++---- src/legacy/ui/public/agg_types/buckets/terms.js | 5 +---- .../ui/public/agg_types/param_types/base.ts | 7 ++++--- .../public/courier/search_source/search_source.js | 13 ++++--------- .../search_strategy/default_search_strategy.js | 7 +++++-- .../ui/public/saved_objects/saved_object.js | 6 +----- src/legacy/ui/public/vis/agg_config.ts | 4 ++-- src/legacy/ui/public/vis/agg_configs.ts | 4 ++-- .../ui/public/vis/request_handlers/courier.js | 11 +++-------- .../loader/embedded_visualize_handler.ts | 6 +++--- .../loader/pipeline_helpers/build_pipeline.ts | 15 +++------------ 16 files changed, 51 insertions(+), 66 deletions(-) diff --git a/src/legacy/core_plugins/input_control_vis/public/control/list_control_factory.js b/src/legacy/core_plugins/input_control_vis/public/control/list_control_factory.js index 105bbe5dad501..68ed31598a4cf 100644 --- a/src/legacy/core_plugins/input_control_vis/public/control/list_control_factory.js +++ b/src/legacy/core_plugins/input_control_vis/public/control/list_control_factory.js @@ -122,12 +122,12 @@ class ListControl extends Control { this.useTimeFilter, ancestorFilters ); - this.abortController.signal.addEventListener('abort', () => searchSource.cancelQueued()); + const abortSignal = this.abortController.signal; this.lastQuery = query; let resp; try { - resp = await searchSource.fetch(); + resp = await searchSource.fetch({ abortSignal }); } catch(error) { // If the fetch was aborted then no need to surface this error in the UI if (error.name === 'AbortError') return; diff --git a/src/legacy/core_plugins/input_control_vis/public/control/range_control_factory.js b/src/legacy/core_plugins/input_control_vis/public/control/range_control_factory.js index 8a157769165ca..3aabd185b3b7c 100644 --- a/src/legacy/core_plugins/input_control_vis/public/control/range_control_factory.js +++ b/src/legacy/core_plugins/input_control_vis/public/control/range_control_factory.js @@ -66,11 +66,11 @@ class RangeControl extends Control { const aggs = minMaxAgg(indexPattern.fields.byName[fieldName]); const searchSource = createSearchSource(this.kbnApi, null, indexPattern, aggs, this.useTimeFilter); - this.abortController.signal.addEventListener('abort', () => searchSource.cancelQueued()); + const abortSignal = this.abortController.signal; let resp; try { - resp = await searchSource.fetch(); + resp = await searchSource.fetch({ abortSignal }); } catch(error) { // If the fetch was aborted then no need to surface this error in the UI if (error.name === 'AbortError') return; diff --git a/src/legacy/core_plugins/kibana/public/discover/controllers/discover.js b/src/legacy/core_plugins/kibana/public/discover/controllers/discover.js index 54620e7f86ac4..b309e74f296dc 100644 --- a/src/legacy/core_plugins/kibana/public/discover/controllers/discover.js +++ b/src/legacy/core_plugins/kibana/public/discover/controllers/discover.js @@ -234,7 +234,10 @@ function discoverController( // the saved savedSearch const savedSearch = $route.current.locals.savedSearch; + + let abortController; $scope.$on('$destroy', () => { + if (abortController) abortController.abort(); savedSearch.destroy(); subscriptions.unsubscribe(); }); @@ -749,7 +752,8 @@ function discoverController( $scope.updateTime(); // Abort any in-progress requests before fetching again - $scope.searchSource.cancelQueued(); + if (abortController) abortController.abort(); + abortController = new AbortController(); $scope.updateDataSource() .then(setupVisualization) @@ -757,7 +761,9 @@ function discoverController( $state.save(); $scope.fetchStatus = fetchStatuses.LOADING; logInspectorRequest(); - return $scope.searchSource.fetch(); + return $scope.searchSource.fetch({ + abortSignal: abortController.signal + }); }) .then(onResults) .catch((error) => { @@ -1040,8 +1046,8 @@ function discoverController( ); visSavedObject.vis = $scope.vis; - $scope.searchSource.onRequestStart((searchSource, searchRequest) => { - return $scope.vis.getAggConfig().onSearchRequestStart(searchSource, searchRequest); + $scope.searchSource.onRequestStart((searchSource, searchRequest, options) => { + return $scope.vis.getAggConfig().onSearchRequestStart(searchSource, options); }); $scope.searchSource.setField('aggs', function () { diff --git a/src/legacy/core_plugins/kibana/public/discover/embeddable/search_embeddable.ts b/src/legacy/core_plugins/kibana/public/discover/embeddable/search_embeddable.ts index 92fad1713177a..a40674ad6e94d 100644 --- a/src/legacy/core_plugins/kibana/public/discover/embeddable/search_embeddable.ts +++ b/src/legacy/core_plugins/kibana/public/discover/embeddable/search_embeddable.ts @@ -107,6 +107,7 @@ export class SearchEmbeddable extends Embeddable private subscription?: Subscription; public readonly type = SEARCH_EMBEDDABLE_TYPE; private filterGen: FilterManager; + private abortController?: AbortController; private prevTimeRange?: TimeRange; private prevFilters?: Filter[]; @@ -191,7 +192,7 @@ export class SearchEmbeddable extends Embeddable if (this.autoRefreshFetchSubscription) { this.autoRefreshFetchSubscription.unsubscribe(); } - this.savedSearch.searchSource.cancelQueued(); + if (this.abortController) this.abortController.abort(); } private initializeSearchScope() { @@ -273,7 +274,8 @@ export class SearchEmbeddable extends Embeddable const { searchSource } = this.savedSearch; // Abort any in-progress requests - searchSource.cancelQueued(); + if (this.abortController) this.abortController.abort(); + this.abortController = new AbortController(); searchSource.setField('size', config.get('discover:sampleSize')); searchSource.setField('sort', getSort(this.searchScope.sort, this.searchScope.indexPattern)); @@ -296,7 +298,9 @@ export class SearchEmbeddable extends Embeddable try { // Make the request - const resp = await searchSource.fetch(); + const resp = await searchSource.fetch({ + abortSignal: this.abortController.signal, + }); this.searchScope.isLoading = false; diff --git a/src/legacy/ui/public/_index.scss b/src/legacy/ui/public/_index.scss index 1c3a9c006acfd..6f1243e178f7f 100644 --- a/src/legacy/ui/public/_index.scss +++ b/src/legacy/ui/public/_index.scss @@ -13,7 +13,6 @@ @import './courier/index'; @import './collapsible_sidebar/index'; @import './directives/index'; -@import './error_allow_explicit_index/index'; @import './error_auto_create_index/index'; @import './error_url_overflow/index'; @import './exit_full_screen/index'; diff --git a/src/legacy/ui/public/agg_types/buckets/histogram.js b/src/legacy/ui/public/agg_types/buckets/histogram.js index 418604e48dc57..65dd4f56f41e8 100644 --- a/src/legacy/ui/public/agg_types/buckets/histogram.js +++ b/src/legacy/ui/public/agg_types/buckets/histogram.js @@ -76,7 +76,7 @@ export const histogramBucketAgg = new BucketAggType({ { name: 'interval', editorComponent: NumberIntervalParamEditor, - modifyAggConfigOnSearchRequestStart(aggConfig, searchSource, searchRequest) { + modifyAggConfigOnSearchRequestStart(aggConfig, searchSource, searchRequest, options) { const field = aggConfig.getField(); const aggBody = field.scripted ? { script: { source: field.script, lang: field.lang } } @@ -94,9 +94,7 @@ export const histogramBucketAgg = new BucketAggType({ } }); - searchRequest.whenAborted(() => childSearchSource.cancelQueued()); - - return childSearchSource.fetch() + return childSearchSource.fetch(options) .then((resp) => { aggConfig.setAutoBounds({ min: _.get(resp, 'aggregations.minAgg.value'), diff --git a/src/legacy/ui/public/agg_types/buckets/terms.js b/src/legacy/ui/public/agg_types/buckets/terms.js index fb2d7f9e1d47b..753cd482c36fd 100644 --- a/src/legacy/ui/public/agg_types/buckets/terms.js +++ b/src/legacy/ui/public/agg_types/buckets/terms.js @@ -81,9 +81,6 @@ export const termsBucketAgg = new BucketAggType({ if (aggConfig.params.otherBucket) { const filterAgg = buildOtherBucketAgg(aggConfigs, aggConfig, resp); if (!filterAgg) return resp; - if (abortSignal) { - abortSignal.addEventListener('abort', () => nestedSearchSource.cancelQueued()); - } nestedSearchSource.setField('aggs', filterAgg); @@ -101,7 +98,7 @@ export const termsBucketAgg = new BucketAggType({ }); request.stats(getRequestInspectorStats(nestedSearchSource)); - const response = await nestedSearchSource.fetch(); + const response = await nestedSearchSource.fetch({ abortSignal }); request .stats(getResponseInspectorStats(nestedSearchSource, response)) .ok({ json: response }); diff --git a/src/legacy/ui/public/agg_types/param_types/base.ts b/src/legacy/ui/public/agg_types/param_types/base.ts index c1e6c52d76b1e..6f67a654a6cb0 100644 --- a/src/legacy/ui/public/agg_types/param_types/base.ts +++ b/src/legacy/ui/public/agg_types/param_types/base.ts @@ -49,15 +49,16 @@ export class BaseParamType implements AggParam { * Allows aggConfig to retrieve values needed for serialization by creating a {SearchRequest} * Example usage: an aggregation needs to know the min/max of a field to determine an appropriate interval * - * @param {AggConfig} aggconfig + * @param {AggConfig} aggConfig * @param {Courier.SearchSource} searchSource * @param {Courier.SearchRequest} searchRequest * @returns {Promise|undefined} */ modifyAggConfigOnSearchRequestStart: ( - aggconfig: AggConfig, + aggConfig: AggConfig, searchSource: SearchSource, - searchRequest: any + searchRequest: any, + options: any ) => void; constructor(config: Record) { diff --git a/src/legacy/ui/public/courier/search_source/search_source.js b/src/legacy/ui/public/courier/search_source/search_source.js index 007c5ee617c88..22c83e4f635c4 100644 --- a/src/legacy/ui/public/courier/search_source/search_source.js +++ b/src/legacy/ui/public/courier/search_source/search_source.js @@ -297,18 +297,13 @@ export function SearchSourceProvider(Promise, Private, config) { */ async fetch(options) { const searchRequest = await this._flatten(); + await this.requestIsStarting(searchRequest, options); return fetchSoon.fetch(searchRequest, { ...(this._searchStrategyId && { searchStrategyId: this._searchStrategyId }), ...options, }); } - /** - * Cancel all pending requests for this searchSource - * @return {undefined} - */ - cancelQueued() {} - /** * Add a handler that will be notified whenever requests start * @param {Function} handler @@ -321,9 +316,10 @@ export function SearchSourceProvider(Promise, Private, config) { /** * Called by requests of this search source when they are started * @param {Courier.Request} request + * @param options * @return {Promise} */ - requestIsStarting(request) { + requestIsStarting(request, options) { this.activeFetchCount = (this.activeFetchCount || 0) + 1; this.history = [request]; @@ -339,7 +335,7 @@ export function SearchSourceProvider(Promise, Private, config) { } return Promise - .map(handlers, fn => fn(this, request)) + .map(handlers, fn => fn(this, request, options)) .then(_.noop); } @@ -353,7 +349,6 @@ export function SearchSourceProvider(Promise, Private, config) { * @return {undefined} */ destroy() { - this.cancelQueued(); this._requestStartHandlers.length = 0; } diff --git a/src/legacy/ui/public/courier/search_strategy/default_search_strategy.js b/src/legacy/ui/public/courier/search_strategy/default_search_strategy.js index 27dd705b566b9..a5ddc9239b357 100644 --- a/src/legacy/ui/public/courier/search_strategy/default_search_strategy.js +++ b/src/legacy/ui/public/courier/search_strategy/default_search_strategy.js @@ -55,8 +55,11 @@ function msearch({ searchRequests, es, config, sessionId, esShardTimeout }) { const searching = es.msearch({ ...getMSearchParams(config), body: `${inlineRequests.join('\n')}\n`, - }).then(({ responses }) => responses); - return { searching, abort: searching.abort }; + }); + return { + searching: searching.then(({ responses }) => responses), + abort: searching.abort + }; } function search({ searchRequests, es, Promise, config, sessionId, esShardTimeout }) { diff --git a/src/legacy/ui/public/saved_objects/saved_object.js b/src/legacy/ui/public/saved_objects/saved_object.js index 74d273119125a..f5c62372db10f 100644 --- a/src/legacy/ui/public/saved_objects/saved_object.js +++ b/src/legacy/ui/public/saved_objects/saved_object.js @@ -527,11 +527,7 @@ export function SavedObjectProvider(Promise, Private, confirmModalPromise, index }); }; - this.destroy = () => { - if (this.searchSource) { - this.searchSource.cancelQueued(); - } - }; + this.destroy = () => {}; /** * Delete this object from Elasticsearch diff --git a/src/legacy/ui/public/vis/agg_config.ts b/src/legacy/ui/public/vis/agg_config.ts index dd3489e93243d..01a735580e9af 100644 --- a/src/legacy/ui/public/vis/agg_config.ts +++ b/src/legacy/ui/public/vis/agg_config.ts @@ -227,14 +227,14 @@ export class AggConfig { * @param {Courier.SearchRequest} searchRequest * @return {Promise} */ - onSearchRequestStart(searchSource: any, searchRequest: any) { + onSearchRequestStart(searchSource: any, searchRequest: any, options: any) { if (!this.type) { return Promise.resolve(); } return Promise.all( this.type.params.map((param: any) => - param.modifyAggConfigOnSearchRequestStart(this, searchSource, searchRequest) + param.modifyAggConfigOnSearchRequestStart(this, searchSource, searchRequest, options) ) ); } diff --git a/src/legacy/ui/public/vis/agg_configs.ts b/src/legacy/ui/public/vis/agg_configs.ts index e3edbef40c654..fd31c515ccf21 100644 --- a/src/legacy/ui/public/vis/agg_configs.ts +++ b/src/legacy/ui/public/vis/agg_configs.ts @@ -307,11 +307,11 @@ export class AggConfigs { return _.find(reqAgg.getResponseAggs(), { id }); } - onSearchRequestStart(searchSource: any, searchRequest: any) { + onSearchRequestStart(searchSource: any, searchRequest: any, options: any) { return Promise.all( // @ts-ignore this.getRequestAggs().map((agg: AggConfig) => - agg.onSearchRequestStart(searchSource, searchRequest) + agg.onSearchRequestStart(searchSource, searchRequest, options) ) ); } diff --git a/src/legacy/ui/public/vis/request_handlers/courier.js b/src/legacy/ui/public/vis/request_handlers/courier.js index cdd5158c5701d..88b27f33b47da 100644 --- a/src/legacy/ui/public/vis/request_handlers/courier.js +++ b/src/legacy/ui/public/vis/request_handlers/courier.js @@ -70,8 +70,8 @@ const CourierRequestHandlerProvider = function () { return aggs.toDsl(metricsAtAllLevels); }); - requestSearchSource.onRequestStart((searchSource, searchRequest) => { - return aggs.onSearchRequestStart(searchSource, searchRequest); + requestSearchSource.onRequestStart((searchSource, searchRequest, options) => { + return aggs.onSearchRequestStart(searchSource, searchRequest, options); }); if (timeRange) { @@ -102,12 +102,7 @@ const CourierRequestHandlerProvider = function () { request.stats(getRequestInspectorStats(requestSearchSource)); try { - // Abort any in-progress requests before fetching again - if (abortSignal) { - abortSignal.addEventListener('abort', () => requestSearchSource.cancelQueued()); - } - - const response = await requestSearchSource.fetch(); + const response = await requestSearchSource.fetch({ abortSignal }); searchSource.lastQuery = queryHash; diff --git a/src/legacy/ui/public/visualize/loader/embedded_visualize_handler.ts b/src/legacy/ui/public/visualize/loader/embedded_visualize_handler.ts index 2d50831a21041..fe460167c9c44 100644 --- a/src/legacy/ui/public/visualize/loader/embedded_visualize_handler.ts +++ b/src/legacy/ui/public/visualize/loader/embedded_visualize_handler.ts @@ -494,9 +494,9 @@ export class EmbeddedVisualizeHandler { // If the data loader was aborted then no need to surface this error in the UI if (error && error.name === 'AbortError') return; - // TODO: come up with a general way to cancel execution of pipeline expressions. - if (this.dataLoaderParams.searchSource && this.dataLoaderParams.searchSource.cancelQueued) { - this.dataLoaderParams.searchSource.cancelQueued(); + // Cancel execution of pipeline expressions + if (this.abortController) { + this.abortController.abort(); } this.vis.requestError = error; diff --git a/src/legacy/ui/public/visualize/loader/pipeline_helpers/build_pipeline.ts b/src/legacy/ui/public/visualize/loader/pipeline_helpers/build_pipeline.ts index 6dcd326af00cb..2d22f39af7594 100644 --- a/src/legacy/ui/public/visualize/loader/pipeline_helpers/build_pipeline.ts +++ b/src/legacy/ui/public/visualize/loader/pipeline_helpers/build_pipeline.ts @@ -441,18 +441,9 @@ export const buildVislibDimensions = async ( } else if (xAgg.type.name === 'histogram') { const intervalParam = xAgg.type.paramByName('interval'); const output = { params: {} as any }; - const searchRequest = { - whenAborted: (fn: any) => { - if (params.abortSignal) { - params.abortSignal.addEventListener('abort', fn); - } - }, - }; - await intervalParam.modifyAggConfigOnSearchRequestStart( - xAgg, - params.searchSource, - searchRequest - ); + await intervalParam.modifyAggConfigOnSearchRequestStart(xAgg, params.searchSource, null, { + abortSignal: params.abortSignal, + }); intervalParam.write(xAgg, output); dimensions.x.params.interval = output.params.interval; } From fb1f95932f9e97b4448c7a92177ca8ded584740b Mon Sep 17 00:00:00 2001 From: Lukas Olson Date: Wed, 11 Sep 2019 15:53:08 -0700 Subject: [PATCH 03/25] Remove usages of Angular Promises --- src/legacy/ui/public/courier/courier.js | 1 - src/legacy/ui/public/courier/fetch/call_client.js | 4 ++-- src/legacy/ui/public/courier/fetch/fetch_soon.js | 4 ++-- .../ui/public/courier/search_poll/search_poll.js | 1 - .../ui/public/courier/search_source/search_source.js | 12 +++++------- .../search_strategy/default_search_strategy.js | 2 +- .../search_strategy/default_search_strategy.test.js | 2 -- 7 files changed, 10 insertions(+), 16 deletions(-) diff --git a/src/legacy/ui/public/courier/courier.js b/src/legacy/ui/public/courier/courier.js index ec599b1653a93..75393e903d5d7 100644 --- a/src/legacy/ui/public/courier/courier.js +++ b/src/legacy/ui/public/courier/courier.js @@ -25,7 +25,6 @@ import '../es'; import '../directives/listen'; import { uiModules } from '../modules'; import { addFatalErrorCallback } from '../notify'; -import '../promises'; import { FetchSoonProvider } from './fetch'; import { SearchPoll } from './search_poll'; diff --git a/src/legacy/ui/public/courier/fetch/call_client.js b/src/legacy/ui/public/courier/fetch/call_client.js index 8a6546c20e126..e8e847e6418e2 100644 --- a/src/legacy/ui/public/courier/fetch/call_client.js +++ b/src/legacy/ui/public/courier/fetch/call_client.js @@ -25,7 +25,7 @@ import { EuiSpacer } from '@elastic/eui'; import React from 'react'; import { ShardFailureOpenModalButton } from './components/shard_failure_open_modal_button'; -export function callClient(searchRequests, requestsOptions = [], { Promise, es, config, sessionId, esShardTimeout }) { +export function callClient(searchRequests, requestsOptions = [], { es, config, sessionId, esShardTimeout }) { // Correlate the options with the request that they're associated with const requestOptionEntries = searchRequests.map((request, i) => [request, requestsOptions[i]]); const requestOptionsMap = new Map(requestOptionEntries); @@ -43,7 +43,7 @@ export function callClient(searchRequests, requestsOptions = [], { Promise, es, Object.keys(searchStrategyMap).forEach(searchStrategyId => { const searchStrategy = getSearchStrategyById(searchStrategyId); const requests = searchStrategyMap[searchStrategyId]; - const { searching, abort } = searchStrategy.search({ searchRequests: requests, es, Promise, config, sessionId, esShardTimeout }); + const { searching, abort } = searchStrategy.search({ searchRequests: requests, es, config, sessionId, esShardTimeout }); requests.forEach((request, i) => { const response = searching.then(results => handleResponse(request, results[i])); const { abortSignal } = requestOptionsMap.get(request) || {}; diff --git a/src/legacy/ui/public/courier/fetch/fetch_soon.js b/src/legacy/ui/public/courier/fetch/fetch_soon.js index 6f8a31bf1f976..4bed4ea115346 100644 --- a/src/legacy/ui/public/courier/fetch/fetch_soon.js +++ b/src/legacy/ui/public/courier/fetch/fetch_soon.js @@ -24,7 +24,7 @@ import { callClient } from './call_client'; * a slight delay in the request process to allow multiple requests to queue up (e.g. when a dashboard * is loading). */ -export function FetchSoonProvider(Promise, es, config, sessionId, esShardTimeout) { +export function FetchSoonProvider(es, config, sessionId, esShardTimeout) { /** * Delays executing a function for a given amount of time, and returns a promise that resolves * with the result. @@ -57,7 +57,7 @@ export function FetchSoonProvider(Promise, es, config, sessionId, esShardTimeout requestsToFetch = [...requestsToFetch, request]; requestOptions = [...requestOptions, options]; const responses = await (fetchInProgress = fetchInProgress || delay(() => { - const response = callClient(requestsToFetch, requestOptions, { Promise, es, config, sessionId, esShardTimeout }); + const response = callClient(requestsToFetch, requestOptions, { es, config, sessionId, esShardTimeout }); requestsToFetch = []; requestOptions = []; fetchInProgress = null; diff --git a/src/legacy/ui/public/courier/search_poll/search_poll.js b/src/legacy/ui/public/courier/search_poll/search_poll.js index 6b28ac9d29b23..81eb40ca1de87 100644 --- a/src/legacy/ui/public/courier/search_poll/search_poll.js +++ b/src/legacy/ui/public/courier/search_poll/search_poll.js @@ -19,7 +19,6 @@ import _ from 'lodash'; -import '../../promises'; import { timefilter } from 'ui/timefilter'; export class SearchPoll { diff --git a/src/legacy/ui/public/courier/search_source/search_source.js b/src/legacy/ui/public/courier/search_source/search_source.js index 22c83e4f635c4..74a22036bc860 100644 --- a/src/legacy/ui/public/courier/search_source/search_source.js +++ b/src/legacy/ui/public/courier/search_source/search_source.js @@ -110,7 +110,7 @@ function isIndexPattern(val) { return Boolean(val && typeof val.title === 'string'); } -export function SearchSourceProvider(Promise, Private, config) { +export function SearchSourceProvider(Private, config) { const normalizeSortRequest = Private(NormalizeSortRequestProvider); const fetchSoon = Private(FetchSoonProvider); const { fieldWildcardFilter } = Private(FieldWildcardProvider); @@ -334,9 +334,7 @@ export function SearchSourceProvider(Promise, Private, config) { } } - return Promise - .map(handlers, fn => fn(this, request, options)) - .then(_.noop); + return Promise.all(handlers.map(fn => fn(this, request, options))); } async getSearchRequestBody() { @@ -368,7 +366,7 @@ export function SearchSourceProvider(Promise, Private, config) { _mergeProp(data, val, key) { if (typeof val === 'function') { const source = this; - return Promise.cast(val(this)) + return Promise.resolve(val(this)) .then(function (newVal) { return source._mergeProp(data, newVal, key); }); @@ -450,14 +448,14 @@ export function SearchSourceProvider(Promise, Private, config) { // pass each key:value pair to source._mergeProp. if _mergeProp // returns a promise, then wait for it to complete and call _mergeProp again return Promise.all(_.map(current._fields, function ittr(value, key) { - if (Promise.is(value)) { + if (value instanceof Promise) { return value.then(function (value) { return ittr(value, key); }); } const prom = root._mergeProp(flatData, value, key); - return Promise.is(prom) ? prom : null; + return prom instanceof Promise ? prom : null; })) .then(function () { // move to this sources parent diff --git a/src/legacy/ui/public/courier/search_strategy/default_search_strategy.js b/src/legacy/ui/public/courier/search_strategy/default_search_strategy.js index a5ddc9239b357..a29ef416d6408 100644 --- a/src/legacy/ui/public/courier/search_strategy/default_search_strategy.js +++ b/src/legacy/ui/public/courier/search_strategy/default_search_strategy.js @@ -62,7 +62,7 @@ function msearch({ searchRequests, es, config, sessionId, esShardTimeout }) { }; } -function search({ searchRequests, es, Promise, config, sessionId, esShardTimeout }) { +function search({ searchRequests, es, config, sessionId, esShardTimeout }) { const abortController = new AbortController(); const searchParams = getSearchParams(config, sessionId, esShardTimeout); const promises = searchRequests.map(({ index, body }) => { diff --git a/src/legacy/ui/public/courier/search_strategy/default_search_strategy.test.js b/src/legacy/ui/public/courier/search_strategy/default_search_strategy.test.js index 341a3268f6f3d..27e2c47c9a2d9 100644 --- a/src/legacy/ui/public/courier/search_strategy/default_search_strategy.test.js +++ b/src/legacy/ui/public/courier/search_strategy/default_search_strategy.test.js @@ -18,7 +18,6 @@ */ import { defaultSearchStrategy } from './default_search_strategy'; -import { Promise } from 'bluebird'; const { search } = defaultSearchStrategy; @@ -44,7 +43,6 @@ describe('defaultSearchStrategy', function () { msearch: msearchMock, search: searchMock, }, - Promise, serializeFetchParams: () => Promise.resolve('pretend this is a valid request body'), }; }); From 1a40af42c7785e09b6432db5573468352b33e9dc Mon Sep 17 00:00:00 2001 From: Lukas Olson Date: Wed, 11 Sep 2019 16:07:21 -0700 Subject: [PATCH 04/25] Remove usages of angular "sessionId" service --- src/legacy/ui/public/courier/fetch/call_client.js | 4 ++-- src/legacy/ui/public/courier/fetch/fetch_soon.js | 4 ++-- src/legacy/ui/public/courier/fetch/get_search_params.js | 8 +++++--- .../courier/search_strategy/default_search_strategy.js | 8 ++++---- src/legacy/ui/public/legacy_compat/angular_config.tsx | 1 - 5 files changed, 13 insertions(+), 12 deletions(-) diff --git a/src/legacy/ui/public/courier/fetch/call_client.js b/src/legacy/ui/public/courier/fetch/call_client.js index e8e847e6418e2..615eddb52f2a6 100644 --- a/src/legacy/ui/public/courier/fetch/call_client.js +++ b/src/legacy/ui/public/courier/fetch/call_client.js @@ -25,7 +25,7 @@ import { EuiSpacer } from '@elastic/eui'; import React from 'react'; import { ShardFailureOpenModalButton } from './components/shard_failure_open_modal_button'; -export function callClient(searchRequests, requestsOptions = [], { es, config, sessionId, esShardTimeout }) { +export function callClient(searchRequests, requestsOptions = [], { es, config, esShardTimeout }) { // Correlate the options with the request that they're associated with const requestOptionEntries = searchRequests.map((request, i) => [request, requestsOptions[i]]); const requestOptionsMap = new Map(requestOptionEntries); @@ -43,7 +43,7 @@ export function callClient(searchRequests, requestsOptions = [], { es, config, s Object.keys(searchStrategyMap).forEach(searchStrategyId => { const searchStrategy = getSearchStrategyById(searchStrategyId); const requests = searchStrategyMap[searchStrategyId]; - const { searching, abort } = searchStrategy.search({ searchRequests: requests, es, config, sessionId, esShardTimeout }); + const { searching, abort } = searchStrategy.search({ searchRequests: requests, es, config, esShardTimeout }); requests.forEach((request, i) => { const response = searching.then(results => handleResponse(request, results[i])); const { abortSignal } = requestOptionsMap.get(request) || {}; diff --git a/src/legacy/ui/public/courier/fetch/fetch_soon.js b/src/legacy/ui/public/courier/fetch/fetch_soon.js index 4bed4ea115346..134145808dd7b 100644 --- a/src/legacy/ui/public/courier/fetch/fetch_soon.js +++ b/src/legacy/ui/public/courier/fetch/fetch_soon.js @@ -24,7 +24,7 @@ import { callClient } from './call_client'; * a slight delay in the request process to allow multiple requests to queue up (e.g. when a dashboard * is loading). */ -export function FetchSoonProvider(es, config, sessionId, esShardTimeout) { +export function FetchSoonProvider(es, config, esShardTimeout) { /** * Delays executing a function for a given amount of time, and returns a promise that resolves * with the result. @@ -57,7 +57,7 @@ export function FetchSoonProvider(es, config, sessionId, esShardTimeout) { requestsToFetch = [...requestsToFetch, request]; requestOptions = [...requestOptions, options]; const responses = await (fetchInProgress = fetchInProgress || delay(() => { - const response = callClient(requestsToFetch, requestOptions, { es, config, sessionId, esShardTimeout }); + const response = callClient(requestsToFetch, requestOptions, { es, config, esShardTimeout }); requestsToFetch = []; requestOptions = []; fetchInProgress = null; diff --git a/src/legacy/ui/public/courier/fetch/get_search_params.js b/src/legacy/ui/public/courier/fetch/get_search_params.js index 7561661d321fa..dd55201ba5540 100644 --- a/src/legacy/ui/public/courier/fetch/get_search_params.js +++ b/src/legacy/ui/public/courier/fetch/get_search_params.js @@ -17,6 +17,8 @@ * under the License. */ +const sessionId = Date.now(); + export function getMSearchParams(config) { return { rest_total_hits_as_int: true, @@ -25,13 +27,13 @@ export function getMSearchParams(config) { }; } -export function getSearchParams(config, sessionId, esShardTimeout) { +export function getSearchParams(config, esShardTimeout) { return { rest_total_hits_as_int: true, ignore_unavailable: true, ignore_throttled: getIgnoreThrottled(config), max_concurrent_shard_requests: getMaxConcurrentShardRequests(config), - preference: getPreference(config, sessionId), + preference: getPreference(config), timeout: getTimeout(esShardTimeout), }; } @@ -45,7 +47,7 @@ export function getMaxConcurrentShardRequests(config) { return maxConcurrentShardRequests > 0 ? maxConcurrentShardRequests : undefined; } -export function getPreference(config, sessionId) { +export function getPreference(config) { const setRequestPreference = config.get('courier:setRequestPreference'); if (setRequestPreference === 'sessionId') return sessionId; return setRequestPreference === 'custom' ? config.get('courier:customRequestPreference') : undefined; diff --git a/src/legacy/ui/public/courier/search_strategy/default_search_strategy.js b/src/legacy/ui/public/courier/search_strategy/default_search_strategy.js index a29ef416d6408..7d9865c137e62 100644 --- a/src/legacy/ui/public/courier/search_strategy/default_search_strategy.js +++ b/src/legacy/ui/public/courier/search_strategy/default_search_strategy.js @@ -37,13 +37,13 @@ export const defaultSearchStrategy = { }, }; -function msearch({ searchRequests, es, config, sessionId, esShardTimeout }) { +function msearch({ searchRequests, es, config, esShardTimeout }) { const inlineRequests = searchRequests.map(({ index, body, search_type: searchType }) => { const inlineHeader = { index: index.title || index, search_type: searchType, ignore_unavailable: true, - preference: getPreference(config, sessionId) + preference: getPreference(config) }; const inlineBody = { ...body, @@ -62,9 +62,9 @@ function msearch({ searchRequests, es, config, sessionId, esShardTimeout }) { }; } -function search({ searchRequests, es, config, sessionId, esShardTimeout }) { +function search({ searchRequests, es, config, esShardTimeout }) { const abortController = new AbortController(); - const searchParams = getSearchParams(config, sessionId, esShardTimeout); + const searchParams = getSearchParams(config, esShardTimeout); const promises = searchRequests.map(({ index, body }) => { const searching = es.search({ index: index.title || index, body, ...searchParams }) .catch(({ response }) => JSON.parse(response)); diff --git a/src/legacy/ui/public/legacy_compat/angular_config.tsx b/src/legacy/ui/public/legacy_compat/angular_config.tsx index 1e22003b32833..3b93e70a5667e 100644 --- a/src/legacy/ui/public/legacy_compat/angular_config.tsx +++ b/src/legacy/ui/public/legacy_compat/angular_config.tsx @@ -64,7 +64,6 @@ export const configureAppAngularModule = (angularModule: IModule) => { .value('buildNum', legacyMetadata.buildNum) .value('buildSha', legacyMetadata.buildSha) .value('serverName', legacyMetadata.serverName) - .value('sessionId', Date.now()) .value('esUrl', getEsUrl(newPlatform)) .value('uiCapabilities', capabilities.get()) .config(setupCompileProvider(newPlatform)) From 8aa64967a31b27b994c33a213530b1e8d0a9b590 Mon Sep 17 00:00:00 2001 From: Lukas Olson Date: Fri, 13 Sep 2019 16:40:14 -0700 Subject: [PATCH 05/25] Remove config as dependency --- .../edit_index_pattern/edit_index_pattern.js | 5 +- src/legacy/ui/public/courier/courier.js | 15 +--- .../ui/public/courier/fetch/fetch_soon.js | 2 +- .../__tests__/normalize_sort_request.js | 4 +- .../search_source/_normalize_sort_request.js | 76 +++++++++---------- .../courier/search_source/search_source.js | 23 +++--- .../__tests__/field_wildcard.js | 19 ++--- .../public/field_wildcard/field_wildcard.js | 44 +++++------ src/legacy/ui/public/field_wildcard/index.js | 2 +- 9 files changed, 78 insertions(+), 112 deletions(-) diff --git a/src/legacy/core_plugins/kibana/public/management/sections/index_patterns/edit_index_pattern/edit_index_pattern.js b/src/legacy/core_plugins/kibana/public/management/sections/index_patterns/edit_index_pattern/edit_index_pattern.js index 1eb56403d3a78..6ae84b9c641c2 100644 --- a/src/legacy/core_plugins/kibana/public/management/sections/index_patterns/edit_index_pattern/edit_index_pattern.js +++ b/src/legacy/core_plugins/kibana/public/management/sections/index_patterns/edit_index_pattern/edit_index_pattern.js @@ -27,7 +27,7 @@ import { fatalError, toastNotifications } from 'ui/notify'; import uiRoutes from 'ui/routes'; import { uiModules } from 'ui/modules'; import template from './edit_index_pattern.html'; -import { FieldWildcardProvider } from 'ui/field_wildcard'; +import { fieldWildcardMatcher } from 'ui/field_wildcard'; import { IndexPatternListFactory } from 'ui/management/index_pattern_list'; import React from 'react'; import { render, unmountComponentAtNode } from 'react-dom'; @@ -173,10 +173,9 @@ uiModules.get('apps/management') .controller('managementIndexPatternsEdit', function ( $scope, $location, $route, Promise, config, indexPatterns, Private, AppState, confirmModal) { const $state = $scope.state = new AppState(); - const { fieldWildcardMatcher } = Private(FieldWildcardProvider); const indexPatternListProvider = Private(IndexPatternListFactory)(); - $scope.fieldWildcardMatcher = fieldWildcardMatcher; + $scope.fieldWildcardMatcher = (...args) => fieldWildcardMatcher(...args, config.get('metaFields')); $scope.editSectionsProvider = Private(IndicesEditSectionsProvider); $scope.kbnUrl = Private(KbnUrlProvider); $scope.indexPattern = $route.current.locals.indexPattern; diff --git a/src/legacy/ui/public/courier/courier.js b/src/legacy/ui/public/courier/courier.js index 75393e903d5d7..a050741b069c7 100644 --- a/src/legacy/ui/public/courier/courier.js +++ b/src/legacy/ui/public/courier/courier.js @@ -26,12 +26,9 @@ import '../directives/listen'; import { uiModules } from '../modules'; import { addFatalErrorCallback } from '../notify'; -import { FetchSoonProvider } from './fetch'; import { SearchPoll } from './search_poll'; -uiModules.get('kibana/courier').service('courier', ($rootScope, Private) => { - const fetchSoon = Private(FetchSoonProvider); - +uiModules.get('kibana/courier').service('courier', () => { // This manages the doc fetch interval. const searchPoll = new SearchPoll(); @@ -66,16 +63,6 @@ uiModules.get('kibana/courier').service('courier', ($rootScope, Private) => { addFatalErrorCallback(closeOnFatal); updateRefreshInterval(); } - - /** - * Fetch the pending requests. - */ - fetch() { - fetchSoon.fetchQueued().then(() => { - // Reset the timer using the time that we get this response as the starting point. - searchPoll.resetTimer(); - }); - } } return new Courier(); diff --git a/src/legacy/ui/public/courier/fetch/fetch_soon.js b/src/legacy/ui/public/courier/fetch/fetch_soon.js index 134145808dd7b..f93d383c5779e 100644 --- a/src/legacy/ui/public/courier/fetch/fetch_soon.js +++ b/src/legacy/ui/public/courier/fetch/fetch_soon.js @@ -25,7 +25,7 @@ import { callClient } from './call_client'; * is loading). */ export function FetchSoonProvider(es, config, esShardTimeout) { - /** +/** * Delays executing a function for a given amount of time, and returns a promise that resolves * with the result. * @param fn The function to invoke diff --git a/src/legacy/ui/public/courier/search_source/__tests__/normalize_sort_request.js b/src/legacy/ui/public/courier/search_source/__tests__/normalize_sort_request.js index 2358fdb520715..33d049315e41a 100644 --- a/src/legacy/ui/public/courier/search_source/__tests__/normalize_sort_request.js +++ b/src/legacy/ui/public/courier/search_source/__tests__/normalize_sort_request.js @@ -20,18 +20,16 @@ import '../../../private'; import ngMock from 'ng_mock'; import expect from '@kbn/expect'; -import { NormalizeSortRequestProvider } from '../_normalize_sort_request'; +import { normalizeSortRequest } from '../_normalize_sort_request'; import FixturesStubbedLogstashIndexPatternProvider from 'fixtures/stubbed_logstash_index_pattern'; import _ from 'lodash'; describe('SearchSource#normalizeSortRequest', function () { - let normalizeSortRequest; let indexPattern; let normalizedSort; beforeEach(ngMock.module('kibana')); beforeEach(ngMock.inject(function (Private) { - normalizeSortRequest = Private(NormalizeSortRequestProvider); indexPattern = Private(FixturesStubbedLogstashIndexPatternProvider); normalizedSort = [{ diff --git a/src/legacy/ui/public/courier/search_source/_normalize_sort_request.js b/src/legacy/ui/public/courier/search_source/_normalize_sort_request.js index d5943771f83b8..219ff4a9abb3f 100644 --- a/src/legacy/ui/public/courier/search_source/_normalize_sort_request.js +++ b/src/legacy/ui/public/courier/search_source/_normalize_sort_request.js @@ -19,59 +19,55 @@ import _ from 'lodash'; -export function NormalizeSortRequestProvider(config) { - const defaultSortOptions = config.get('sort:options'); - - /** +/** * Decorate queries with default parameters * @param {query} query object * @returns {object} */ - return function (sortObject, indexPattern) { - // [].concat({}) -> [{}], [].concat([{}]) -> [{}] - return [].concat(sortObject).map(function (sortable) { - return normalize(sortable, indexPattern); - }); - }; +export function normalizeSortRequest(sortObject, indexPattern, defaultSortOptions) { + // [].concat({}) -> [{}], [].concat([{}]) -> [{}] + return [].concat(sortObject).map(function (sortable) { + return normalize(sortable, indexPattern, defaultSortOptions); + }); +} - /* +/* Normalize the sort description to the more verbose format: { someField: "desc" } into { someField: { "order": "desc"}} */ - function normalize(sortable, indexPattern) { - const normalized = {}; - let sortField = _.keys(sortable)[0]; - let sortValue = sortable[sortField]; - const indexField = indexPattern.fields.byName[sortField]; +function normalize(sortable, indexPattern, defaultSortOptions) { + const normalized = {}; + let sortField = _.keys(sortable)[0]; + let sortValue = sortable[sortField]; + const indexField = indexPattern.fields.byName[sortField]; - if (indexField && indexField.scripted && indexField.sortable) { - let direction; - if (_.isString(sortValue)) direction = sortValue; - if (_.isObject(sortValue) && sortValue.order) direction = sortValue.order; + if (indexField && indexField.scripted && indexField.sortable) { + let direction; + if (_.isString(sortValue)) direction = sortValue; + if (_.isObject(sortValue) && sortValue.order) direction = sortValue.order; - sortField = '_script'; - sortValue = { - script: { - source: indexField.script, - lang: indexField.lang - }, - type: castSortType(indexField.type), - order: direction - }; - } else { - if (_.isString(sortValue)) { - sortValue = { order: sortValue }; - } - sortValue = _.defaults({}, sortValue, defaultSortOptions); - - if (sortField === '_score') { - delete sortValue.unmapped_type; - } + sortField = '_script'; + sortValue = { + script: { + source: indexField.script, + lang: indexField.lang + }, + type: castSortType(indexField.type), + order: direction + }; + } else { + if (_.isString(sortValue)) { + sortValue = { order: sortValue }; } + sortValue = _.defaults({}, sortValue, defaultSortOptions); - normalized[sortField] = sortValue; - return normalized; + if (sortField === '_score') { + delete sortValue.unmapped_type; + } } + + normalized[sortField] = sortValue; + return normalized; } // The ES API only supports sort scripts of type 'number' and 'string' diff --git a/src/legacy/ui/public/courier/search_source/search_source.js b/src/legacy/ui/public/courier/search_source/search_source.js index c9b0d28a7add2..a17a7aae86a09 100644 --- a/src/legacy/ui/public/courier/search_source/search_source.js +++ b/src/legacy/ui/public/courier/search_source/search_source.js @@ -73,11 +73,12 @@ import _ from 'lodash'; import angular from 'angular'; import { buildEsQuery, getEsQueryConfig, filterMatchesIndex } from '@kbn/es-query'; -import { NormalizeSortRequestProvider } from './_normalize_sort_request'; +import { normalizeSortRequest } from './_normalize_sort_request'; import { FetchSoonProvider } from '../fetch'; -import { FieldWildcardProvider } from '../../field_wildcard'; +import { fieldWildcardFilter } from '../../field_wildcard'; import { getHighlightRequest } from '../../../../../plugins/data/common/highlight'; +import chrome from '../../chrome'; const FIELDS = [ 'type', @@ -110,14 +111,12 @@ function isIndexPattern(val) { return Boolean(val && typeof val.title === 'string'); } -export function SearchSourceProvider(Private, config) { - const normalizeSortRequest = Private(NormalizeSortRequestProvider); - const fetchSoon = Private(FetchSoonProvider); - const { fieldWildcardFilter } = Private(FieldWildcardProvider); - const getConfig = (...args) => config.get(...args); - - const forIp = Symbol('for which index pattern?'); +const config = chrome.getUiSettingsClient(); +const getConfig = (...args) => config.get(...args); +const forIp = Symbol('for which index pattern?'); +export function SearchSourceProvider(Private) { + const fetchSoon = Private(FetchSoonProvider); class SearchSource { constructor(initialFields) { this._id = _.uniqueId('data_source'); @@ -132,7 +131,7 @@ export function SearchSourceProvider(Private, config) { this._filterPredicates = [ (filter) => { - // remove null/undefined filters + // remove null/undefined filters return filter; }, (filter) => { @@ -401,7 +400,7 @@ export function SearchSourceProvider(Private, config) { addToBody(); break; case 'sort': - val = normalizeSortRequest(val, this.getField('index')); + val = normalizeSortRequest(val, this.getField('index'), config.get('sort:options')); addToBody(); break; case 'query': @@ -481,7 +480,7 @@ export function SearchSourceProvider(Private, config) { if (flatData.body._source) { // exclude source fields for this index pattern specified by the user - const filter = fieldWildcardFilter(flatData.body._source.excludes); + const filter = fieldWildcardFilter(flatData.body._source.excludes, config.get('metaFields')); flatData.body.docvalue_fields = flatData.body.docvalue_fields.filter( docvalueField => filter(docvalueField.field) ); diff --git a/src/legacy/ui/public/field_wildcard/__tests__/field_wildcard.js b/src/legacy/ui/public/field_wildcard/__tests__/field_wildcard.js index aeffdbc8bfa6c..a15c602b7ba83 100644 --- a/src/legacy/ui/public/field_wildcard/__tests__/field_wildcard.js +++ b/src/legacy/ui/public/field_wildcard/__tests__/field_wildcard.js @@ -20,19 +20,12 @@ import expect from '@kbn/expect'; import ngMock from 'ng_mock'; -import { FieldWildcardProvider } from '../../field_wildcard'; +import { fieldWildcardFilter, makeRegEx } from '../../field_wildcard'; describe('fieldWildcard', function () { - let fieldWildcardFilter; - let makeRegEx; + const metaFields = ['_id', '_type', '_source']; beforeEach(ngMock.module('kibana')); - beforeEach(ngMock.inject(function (config, Private) { - config.set('metaFields', ['_id', '_type', '_source']); - const fieldWildcard = Private(FieldWildcardProvider); - fieldWildcardFilter = fieldWildcard.fieldWildcardFilter; - makeRegEx = fieldWildcard.makeRegEx; - })); describe('makeRegEx', function () { it('matches * in any position', function () { @@ -70,7 +63,7 @@ describe('fieldWildcard', function () { }); it('filters nothing when given an empty array', function () { - const filter = fieldWildcardFilter([]); + const filter = fieldWildcardFilter([], metaFields); const original = [ 'foo', 'bar', @@ -82,7 +75,7 @@ describe('fieldWildcard', function () { }); it('does not filter metaFields', function () { - const filter = fieldWildcardFilter([ '_*' ]); + const filter = fieldWildcardFilter([ '_*' ], metaFields); const original = [ '_id', @@ -97,7 +90,7 @@ describe('fieldWildcard', function () { const filter = fieldWildcardFilter([ 'f*', '*4' - ]); + ], metaFields); const original = [ 'foo', @@ -114,7 +107,7 @@ describe('fieldWildcard', function () { 'f*', '*4', 'undefined' - ]); + ], metaFields); const original = [ 'foo', diff --git a/src/legacy/ui/public/field_wildcard/field_wildcard.js b/src/legacy/ui/public/field_wildcard/field_wildcard.js index f73997d40a4e4..ffc78405c88bd 100644 --- a/src/legacy/ui/public/field_wildcard/field_wildcard.js +++ b/src/legacy/ui/public/field_wildcard/field_wildcard.js @@ -19,31 +19,25 @@ import { escapeRegExp, memoize } from 'lodash'; -export function FieldWildcardProvider(config) { - const metaFields = config.get('metaFields'); +export const makeRegEx = memoize(function makeRegEx(glob) { + return new RegExp('^' + glob.split('*').map(escapeRegExp).join('.*') + '$'); +}); - const makeRegEx = memoize(function makeRegEx(glob) { - return new RegExp('^' + glob.split('*').map(escapeRegExp).join('.*') + '$'); - }); - - // Note that this will return an essentially noop function if globs is undefined. - function fieldWildcardMatcher(globs = []) { - return function matcher(val) { - // do not test metaFields or keyword - if (metaFields.indexOf(val) !== -1) { - return false; - } - return globs.some(p => makeRegEx(p).test(val)); - }; - } - - // Note that this will return an essentially noop function if globs is undefined. - function fieldWildcardFilter(globs = []) { - const matcher = fieldWildcardMatcher(globs); - return function filter(val) { - return !matcher(val); - }; - } +// Note that this will return an essentially noop function if globs is undefined. +export function fieldWildcardMatcher(globs = [], metaFields) { + return function matcher(val) { + // do not test metaFields or keyword + if (metaFields.indexOf(val) !== -1) { + return false; + } + return globs.some(p => makeRegEx(p).test(val)); + }; +} - return { makeRegEx, fieldWildcardMatcher, fieldWildcardFilter }; +// Note that this will return an essentially noop function if globs is undefined. +export function fieldWildcardFilter(globs = [], metaFields) { + const matcher = fieldWildcardMatcher(globs, metaFields); + return function filter(val) { + return !matcher(val); + }; } diff --git a/src/legacy/ui/public/field_wildcard/index.js b/src/legacy/ui/public/field_wildcard/index.js index d03643f8804d8..db9f830e450b8 100644 --- a/src/legacy/ui/public/field_wildcard/index.js +++ b/src/legacy/ui/public/field_wildcard/index.js @@ -17,4 +17,4 @@ * under the License. */ -export { FieldWildcardProvider } from './field_wildcard'; +export * from './field_wildcard'; From 027f6b6b11d2a52b6acd880f79b0431b35d72bf7 Mon Sep 17 00:00:00 2001 From: Lukas Olson Date: Mon, 23 Sep 2019 13:30:24 -0700 Subject: [PATCH 06/25] Update deps on config and esShardTimeout --- src/legacy/ui/public/courier/fetch/fetch_soon.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/legacy/ui/public/courier/fetch/fetch_soon.js b/src/legacy/ui/public/courier/fetch/fetch_soon.js index f93d383c5779e..a48db3b242be6 100644 --- a/src/legacy/ui/public/courier/fetch/fetch_soon.js +++ b/src/legacy/ui/public/courier/fetch/fetch_soon.js @@ -18,13 +18,16 @@ */ import { callClient } from './call_client'; +import { npSetup } from 'ui/new_platform'; +const config = npSetup.core.uiSettings; +const esShardTimeout = npSetup.core.injectedMetadata.getInjectedVar('esShardTimeout'); /** * This is usually the right fetch provider to use, rather than FetchNowProvider, as this class introduces * a slight delay in the request process to allow multiple requests to queue up (e.g. when a dashboard * is loading). */ -export function FetchSoonProvider(es, config, esShardTimeout) { +export function FetchSoonProvider(es) { /** * Delays executing a function for a given amount of time, and returns a promise that resolves * with the result. From ae21ec842b1c07d99997ab42b517678520798f0b Mon Sep 17 00:00:00 2001 From: Lukas Olson Date: Tue, 24 Sep 2019 10:48:52 -0700 Subject: [PATCH 07/25] Remove remaining angular dependencies from SearchSource --- .../interpreter/public/functions/esaggs.ts | 27 +- .../public/context/api/__tests__/_stubs.js | 42 +- .../public/context/api/__tests__/anchor.js | 49 +- .../context/api/__tests__/predecessors.js | 20 +- .../context/api/__tests__/successors.js | 19 +- .../kibana/public/context/api/anchor.js | 6 +- .../kibana/public/context/api/context.ts | 9 +- .../discover/embeddable/search_embeddable.ts | 6 +- .../ui/public/courier/fetch/fetch_soon.js | 67 +- src/legacy/ui/public/courier/fetch/index.js | 2 +- src/legacy/ui/public/courier/index.js | 2 +- .../ui/public/courier/search_source/index.js | 2 +- .../courier/search_source/search_source.d.ts | 23 +- .../courier/search_source/search_source.js | 635 +++++++++--------- .../ui/public/saved_objects/saved_object.js | 3 +- .../ui/public/vis/request_handlers/courier.js | 173 ----- src/legacy/ui/public/vis/vis.js | 3 +- .../plugins/maps/public/kibana_services.js | 9 +- 18 files changed, 453 insertions(+), 644 deletions(-) delete mode 100644 src/legacy/ui/public/vis/request_handlers/courier.js diff --git a/src/legacy/core_plugins/interpreter/public/functions/esaggs.ts b/src/legacy/core_plugins/interpreter/public/functions/esaggs.ts index fe1124c321f9e..8381853a38479 100644 --- a/src/legacy/core_plugins/interpreter/public/functions/esaggs.ts +++ b/src/legacy/core_plugins/interpreter/public/functions/esaggs.ts @@ -29,7 +29,6 @@ import chrome from 'ui/chrome'; import { TimeRange } from 'src/plugins/data/public'; import { SearchSource } from '../../../../ui/public/courier/search_source'; // @ts-ignore -import { SearchSourceProvider } from '../../../../ui/public/courier/search_source'; import { FilterBarQueryFilterProvider } from '../../../../ui/public/filter_manager/query_filter'; import { buildTabularInspectorData } from '../../../../ui/public/inspector/build_tabular_inspector_data'; @@ -120,7 +119,7 @@ const handleCourierRequest = async ({ const queryHash = calculateObjectHash(reqBody); // We only need to reexecute the query, if forceFetch was true or the hash of the request body has changed // since the last request - const shouldQuery = forceFetch || searchSource.lastQuery !== queryHash; + const shouldQuery = forceFetch || (searchSource as any).lastQuery !== queryHash; if (shouldQuery) { inspectorAdapters.requests.reset(); @@ -143,11 +142,11 @@ const handleCourierRequest = async ({ try { const response = await requestSearchSource.fetch({ abortSignal }); - searchSource.lastQuery = queryHash; + (searchSource as any).lastQuery = queryHash; request.stats(getResponseInspectorStats(searchSource, response)).ok({ json: response }); - searchSource.rawResponse = response; + (searchSource as any).rawResponse = response; } catch (e) { // Log any error during request to the inspector request.error({ json: e }); @@ -163,7 +162,7 @@ const handleCourierRequest = async ({ // Note that rawResponse is not deeply cloned here, so downstream applications using courier // must take care not to mutate it, or it could have unintended side effects, e.g. displaying // response data incorrectly in the inspector. - let resp = searchSource.rawResponse; + let resp = (searchSource as any).rawResponse; for (const agg of aggs.aggs) { if (has(agg, 'type.postFlightRequest')) { resp = await agg.type.postFlightRequest( @@ -177,7 +176,7 @@ const handleCourierRequest = async ({ } } - searchSource.finalResponse = resp; + (searchSource as any).finalResponse = resp; const parsedTimeRange = timeRange ? getTime(aggs.indexPattern, timeRange) : null; const tabifyParams = { @@ -188,23 +187,24 @@ const handleCourierRequest = async ({ const tabifyCacheHash = calculateObjectHash({ tabifyAggs: aggs, ...tabifyParams }); // We only need to reexecute tabify, if either we did a new request or some input params to tabify changed - const shouldCalculateNewTabify = shouldQuery || searchSource.lastTabifyHash !== tabifyCacheHash; + const shouldCalculateNewTabify = + shouldQuery || (searchSource as any).lastTabifyHash !== tabifyCacheHash; if (shouldCalculateNewTabify) { - searchSource.lastTabifyHash = tabifyCacheHash; - searchSource.tabifiedResponse = tabifyAggResponse( + (searchSource as any).lastTabifyHash = tabifyCacheHash; + (searchSource as any).tabifiedResponse = tabifyAggResponse( aggs, - searchSource.finalResponse, + (searchSource as any).finalResponse, tabifyParams ); } inspectorAdapters.data.setTabularLoader( - () => buildTabularInspectorData(searchSource.tabifiedResponse, queryFilter), + () => buildTabularInspectorData((searchSource as any).tabifiedResponse, queryFilter), { returnsFormattedValues: true } ); - return searchSource.tabifiedResponse; + return (searchSource as any).tabifiedResponse; }; export const esaggs = (): ExpressionFunction => ({ @@ -247,7 +247,6 @@ export const esaggs = (): ExpressionFunction { + searchSourceStub.setParent = sinon.stub(SearchSource.prototype, 'setParent').returns(searchSourceStub); + searchSourceStub.setField = sinon.stub(SearchSource.prototype, 'setField').returns(searchSourceStub); + searchSourceStub.getField = sinon.stub(SearchSource.prototype, 'getField').callsFake(key => { const previousSetCall = searchSourceStub.setField.withArgs(key).lastCall; return previousSetCall ? previousSetCall.args[1] : null; }); - searchSourceStub.fetch = sinon.spy(() => { + searchSourceStub.fetch = sinon.stub(SearchSource.prototype, 'fetch').callsFake(() => Promise.resolve({ + hits: { + hits: searchSourceStub._stubHits, + total: searchSourceStub._stubHits.length, + }, + })); + + searchSourceStub._restore = () => { + searchSourceStub.setParent.restore(); + searchSourceStub.setField.restore(); + searchSourceStub.getField.restore(); + searchSourceStub.fetch.restore(); + }; + + return searchSourceStub; +} + +/** + * A stubbed search source with a `fetch` method that returns a filtered set of `_stubHits`. + */ +export function createContextSearchSourceStub(hits, timeField = '@timestamp') { + const searchSourceStub = createSearchSourceStub(hits, timeField); + + searchSourceStub.fetch.restore(); + searchSourceStub.fetch = sinon.stub(SearchSource.prototype, 'fetch').callsFake(() => { const timeField = searchSourceStub._stubTimeField; const lastQuery = searchSourceStub.setField.withArgs('query').lastCall.args[1]; const timeRange = lastQuery.query.constant_score.filter.range[timeField]; @@ -71,7 +99,5 @@ export function createSearchSourceStubProvider(hits, timeField) { }); }); - return function SearchSourceStubProvider() { - return searchSourceStub; - }; + return searchSourceStub; } diff --git a/src/legacy/core_plugins/kibana/public/context/api/__tests__/anchor.js b/src/legacy/core_plugins/kibana/public/context/api/__tests__/anchor.js index 582de1c8fa74c..46e66177b516a 100644 --- a/src/legacy/core_plugins/kibana/public/context/api/__tests__/anchor.js +++ b/src/legacy/core_plugins/kibana/public/context/api/__tests__/anchor.js @@ -19,55 +19,34 @@ import expect from '@kbn/expect'; import ngMock from 'ng_mock'; -import sinon from 'sinon'; -import { createIndexPatternsStub } from './_stubs'; -import { SearchSourceProvider } from 'ui/courier'; +import { createIndexPatternsStub, createSearchSourceStub } from './_stubs'; import { fetchAnchorProvider } from '../anchor'; -function createSearchSourceStubProvider(hits) { - const searchSourceStub = { - _stubHits: hits, - }; - - searchSourceStub.setParent = sinon.stub().returns(searchSourceStub); - searchSourceStub.setField = sinon.stub().returns(searchSourceStub); - searchSourceStub.fetch = sinon.spy(() => Promise.resolve({ - hits: { - hits: searchSourceStub._stubHits, - total: searchSourceStub._stubHits.length, - }, - })); - - return function SearchSourceStubProvider() { - return searchSourceStub; - }; -} - describe('context app', function () { beforeEach(ngMock.module('kibana')); describe('function fetchAnchor', function () { let fetchAnchor; - let SearchSourceStub; + let searchSourceStub; beforeEach(ngMock.module(function createServiceStubs($provide) { $provide.value('indexPatterns', createIndexPatternsStub()); })); beforeEach(ngMock.inject(function createPrivateStubs(Private) { - SearchSourceStub = createSearchSourceStubProvider([ + searchSourceStub = createSearchSourceStub([ { _id: 'hit1' }, ]); - Private.stub(SearchSourceProvider, SearchSourceStub); - fetchAnchor = Private(fetchAnchorProvider); })); - it('should use the `fetch` method of the SearchSource', function () { - const searchSourceStub = new SearchSourceStub(); + afterEach(() => { + searchSourceStub._restore(); + }); + it('should use the `fetch` method of the SearchSource', function () { return fetchAnchor('INDEX_PATTERN_ID', 'id', [{ '@timestamp': 'desc' }, { '_doc': 'desc' }]) .then(() => { expect(searchSourceStub.fetch.calledOnce).to.be(true); @@ -75,8 +54,6 @@ describe('context app', function () { }); it('should configure the SearchSource to not inherit from the implicit root', function () { - const searchSourceStub = new SearchSourceStub(); - return fetchAnchor('INDEX_PATTERN_ID', 'id', [{ '@timestamp': 'desc' }, { '_doc': 'desc' }]) .then(() => { const setParentSpy = searchSourceStub.setParent; @@ -86,8 +63,6 @@ describe('context app', function () { }); it('should set the SearchSource index pattern', function () { - const searchSourceStub = new SearchSourceStub(); - return fetchAnchor('INDEX_PATTERN_ID', 'id', [{ '@timestamp': 'desc' }, { '_doc': 'desc' }]) .then(() => { const setFieldSpy = searchSourceStub.setField; @@ -96,8 +71,6 @@ describe('context app', function () { }); it('should set the SearchSource version flag to true', function () { - const searchSourceStub = new SearchSourceStub(); - return fetchAnchor('INDEX_PATTERN_ID', 'id', [{ '@timestamp': 'desc' }, { '_doc': 'desc' }]) .then(() => { const setVersionSpy = searchSourceStub.setField.withArgs('version'); @@ -107,8 +80,6 @@ describe('context app', function () { }); it('should set the SearchSource size to 1', function () { - const searchSourceStub = new SearchSourceStub(); - return fetchAnchor('INDEX_PATTERN_ID', 'id', [{ '@timestamp': 'desc' }, { '_doc': 'desc' }]) .then(() => { const setSizeSpy = searchSourceStub.setField.withArgs('size'); @@ -118,8 +89,6 @@ describe('context app', function () { }); it('should set the SearchSource query to an ids query', function () { - const searchSourceStub = new SearchSourceStub(); - return fetchAnchor('INDEX_PATTERN_ID', 'id', [{ '@timestamp': 'desc' }, { '_doc': 'desc' }]) .then(() => { const setQuerySpy = searchSourceStub.setField.withArgs('query'); @@ -140,8 +109,6 @@ describe('context app', function () { }); it('should set the SearchSource sort order', function () { - const searchSourceStub = new SearchSourceStub(); - return fetchAnchor('INDEX_PATTERN_ID', 'id', [{ '@timestamp': 'desc' }, { '_doc': 'desc' }]) .then(() => { const setSortSpy = searchSourceStub.setField.withArgs('sort'); @@ -154,7 +121,6 @@ describe('context app', function () { }); it('should reject with an error when no hits were found', function () { - const searchSourceStub = new SearchSourceStub(); searchSourceStub._stubHits = []; return fetchAnchor('INDEX_PATTERN_ID', 'id', [{ '@timestamp': 'desc' }, { '_doc': 'desc' }]) @@ -169,7 +135,6 @@ describe('context app', function () { }); it('should return the first hit after adding an anchor marker', function () { - const searchSourceStub = new SearchSourceStub(); searchSourceStub._stubHits = [ { property1: 'value1' }, { property2: 'value2' }, diff --git a/src/legacy/core_plugins/kibana/public/context/api/__tests__/predecessors.js b/src/legacy/core_plugins/kibana/public/context/api/__tests__/predecessors.js index 88efc8efc5d30..2bf3da42e24e5 100644 --- a/src/legacy/core_plugins/kibana/public/context/api/__tests__/predecessors.js +++ b/src/legacy/core_plugins/kibana/public/context/api/__tests__/predecessors.js @@ -22,8 +22,7 @@ import ngMock from 'ng_mock'; import moment from 'moment'; import * as _ from 'lodash'; -import { createIndexPatternsStub, createSearchSourceStubProvider } from './_stubs'; -import { SearchSourceProvider } from 'ui/courier'; +import { createIndexPatternsStub, createContextSearchSourceStub } from './_stubs'; import { fetchContextProvider } from '../context'; @@ -38,16 +37,14 @@ describe('context app', function () { describe('function fetchPredecessors', function () { let fetchPredecessors; - let getSearchSourceStub; + let searchSourceStub; beforeEach(ngMock.module(function createServiceStubs($provide) { $provide.value('indexPatterns', createIndexPatternsStub()); })); beforeEach(ngMock.inject(function createPrivateStubs(Private) { - getSearchSourceStub = createSearchSourceStubProvider([], '@timestamp', MS_PER_DAY * 8); - Private.stub(SearchSourceProvider, getSearchSourceStub); - + searchSourceStub = createContextSearchSourceStub([], '@timestamp', MS_PER_DAY * 8); fetchPredecessors = (indexPatternId, timeField, sortDir, timeValIso, timeValNr, tieBreakerField, tieBreakerValue, size) => { const anchor = { _source: { @@ -69,8 +66,11 @@ describe('context app', function () { }; })); + afterEach(() => { + searchSourceStub._restore(); + }); + it('should perform exactly one query when enough hits are returned', function () { - const searchSourceStub = getSearchSourceStub(); searchSourceStub._stubHits = [ searchSourceStub._createStubHit(MS_PER_DAY * 3000 + 2), searchSourceStub._createStubHit(MS_PER_DAY * 3000 + 1), @@ -97,7 +97,6 @@ describe('context app', function () { }); it('should perform multiple queries with the last being unrestricted when too few hits are returned', function () { - const searchSourceStub = getSearchSourceStub(); searchSourceStub._stubHits = [ searchSourceStub._createStubHit(MS_PER_DAY * 3010), searchSourceStub._createStubHit(MS_PER_DAY * 3002), @@ -134,7 +133,6 @@ describe('context app', function () { }); it('should perform multiple queries until the expected hit count is returned', function () { - const searchSourceStub = getSearchSourceStub(); searchSourceStub._stubHits = [ searchSourceStub._createStubHit(MS_PER_DAY * 1700), searchSourceStub._createStubHit(MS_PER_DAY * 1200), @@ -185,8 +183,6 @@ describe('context app', function () { }); it('should configure the SearchSource to not inherit from the implicit root', function () { - const searchSourceStub = getSearchSourceStub(); - return fetchPredecessors( 'INDEX_PATTERN_ID', '@timestamp', @@ -206,8 +202,6 @@ describe('context app', function () { }); it('should set the tiebreaker sort order to the opposite as the time field', function () { - const searchSourceStub = getSearchSourceStub(); - return fetchPredecessors( 'INDEX_PATTERN_ID', '@timestamp', diff --git a/src/legacy/core_plugins/kibana/public/context/api/__tests__/successors.js b/src/legacy/core_plugins/kibana/public/context/api/__tests__/successors.js index 57f7673d31183..b8bec40f2859c 100644 --- a/src/legacy/core_plugins/kibana/public/context/api/__tests__/successors.js +++ b/src/legacy/core_plugins/kibana/public/context/api/__tests__/successors.js @@ -22,8 +22,7 @@ import ngMock from 'ng_mock'; import moment from 'moment'; import * as _ from 'lodash'; -import { createIndexPatternsStub, createSearchSourceStubProvider } from './_stubs'; -import { SearchSourceProvider } from 'ui/courier'; +import { createIndexPatternsStub, createContextSearchSourceStub } from './_stubs'; import { fetchContextProvider } from '../context'; @@ -37,15 +36,14 @@ describe('context app', function () { describe('function fetchSuccessors', function () { let fetchSuccessors; - let getSearchSourceStub; + let searchSourceStub; beforeEach(ngMock.module(function createServiceStubs($provide) { $provide.value('indexPatterns', createIndexPatternsStub()); })); beforeEach(ngMock.inject(function createPrivateStubs(Private) { - getSearchSourceStub = createSearchSourceStubProvider([], '@timestamp'); - Private.stub(SearchSourceProvider, getSearchSourceStub); + searchSourceStub = createContextSearchSourceStub([], '@timestamp'); fetchSuccessors = (indexPatternId, timeField, sortDir, timeValIso, timeValNr, tieBreakerField, tieBreakerValue, size) => { const anchor = { @@ -68,8 +66,11 @@ describe('context app', function () { }; })); + afterEach(() => { + searchSourceStub._restore(); + }); + it('should perform exactly one query when enough hits are returned', function () { - const searchSourceStub = getSearchSourceStub(); searchSourceStub._stubHits = [ searchSourceStub._createStubHit(MS_PER_DAY * 5000), searchSourceStub._createStubHit(MS_PER_DAY * 4000), @@ -96,7 +97,6 @@ describe('context app', function () { }); it('should perform multiple queries with the last being unrestricted when too few hits are returned', function () { - const searchSourceStub = getSearchSourceStub(); searchSourceStub._stubHits = [ searchSourceStub._createStubHit(MS_PER_DAY * 3010), searchSourceStub._createStubHit(MS_PER_DAY * 3002), @@ -133,7 +133,6 @@ describe('context app', function () { }); it('should perform multiple queries until the expected hit count is returned', function () { - const searchSourceStub = getSearchSourceStub(); searchSourceStub._stubHits = [ searchSourceStub._createStubHit(MS_PER_DAY * 3000), searchSourceStub._createStubHit(MS_PER_DAY * 3000 - 1), @@ -187,8 +186,6 @@ describe('context app', function () { }); it('should configure the SearchSource to not inherit from the implicit root', function () { - const searchSourceStub = getSearchSourceStub(); - return fetchSuccessors( 'INDEX_PATTERN_ID', '@timestamp', @@ -208,8 +205,6 @@ describe('context app', function () { }); it('should set the tiebreaker sort order to the same as the time field', function () { - const searchSourceStub = getSearchSourceStub(); - return fetchSuccessors( 'INDEX_PATTERN_ID', '@timestamp', diff --git a/src/legacy/core_plugins/kibana/public/context/api/anchor.js b/src/legacy/core_plugins/kibana/public/context/api/anchor.js index bab75e14e8ed3..02a309eaa0165 100644 --- a/src/legacy/core_plugins/kibana/public/context/api/anchor.js +++ b/src/legacy/core_plugins/kibana/public/context/api/anchor.js @@ -21,11 +21,9 @@ import _ from 'lodash'; import { i18n } from '@kbn/i18n'; -import { SearchSourceProvider } from 'ui/courier'; - -export function fetchAnchorProvider(indexPatterns, Private) { - const SearchSource = Private(SearchSourceProvider); +import { SearchSource } from 'ui/courier'; +export function fetchAnchorProvider(indexPatterns) { return async function fetchAnchor( indexPatternId, anchorId, diff --git a/src/legacy/core_plugins/kibana/public/context/api/context.ts b/src/legacy/core_plugins/kibana/public/context/api/context.ts index baecf8a673521..48ac59f1f0855 100644 --- a/src/legacy/core_plugins/kibana/public/context/api/context.ts +++ b/src/legacy/core_plugins/kibana/public/context/api/context.ts @@ -18,8 +18,7 @@ */ // @ts-ignore -import { SearchSourceProvider, SearchSource } from 'ui/courier'; -import { IPrivate } from 'ui/private'; +import { SearchSource } from 'ui/courier'; import { Filter } from '@kbn/es-query'; import { IndexPatterns, IndexPattern } from 'ui/index_patterns'; import { reverseSortDir, SortDirection } from './utils/sorting'; @@ -42,9 +41,7 @@ const DAY_MILLIS = 24 * 60 * 60 * 1000; // look from 1 day up to 10000 days into the past and future const LOOKUP_OFFSETS = [0, 1, 7, 30, 365, 10000].map(days => days * DAY_MILLIS); -function fetchContextProvider(indexPatterns: IndexPatterns, Private: IPrivate) { - const SearchSourcePrivate: any = Private(SearchSourceProvider); - +function fetchContextProvider(indexPatterns: IndexPatterns) { return { fetchSurroundingDocs, }; @@ -116,7 +113,7 @@ function fetchContextProvider(indexPatterns: IndexPatterns, Private: IPrivate) { } async function createSearchSource(indexPattern: IndexPattern, filters: Filter[]) { - return new SearchSourcePrivate() + return new SearchSource() .setParent(false) .setField('index', indexPattern) .setField('filter', filters); diff --git a/src/legacy/core_plugins/kibana/public/discover/embeddable/search_embeddable.ts b/src/legacy/core_plugins/kibana/public/discover/embeddable/search_embeddable.ts index fe2ac0068139c..6250ad9d4ab9a 100644 --- a/src/legacy/core_plugins/kibana/public/discover/embeddable/search_embeddable.ts +++ b/src/legacy/core_plugins/kibana/public/discover/embeddable/search_embeddable.ts @@ -102,7 +102,7 @@ export class SearchEmbeddable extends Embeddable private inspectorAdaptors: Adapters; private searchScope?: SearchScope; private panelTitle: string = ''; - private filtersSearchSource: SearchSource; + private filtersSearchSource?: SearchSource; private searchInstance?: JQLite; private autoRefreshFetchSubscription?: Subscription; private subscription?: Subscription; @@ -337,8 +337,8 @@ export class SearchEmbeddable extends Embeddable searchScope.sharedItemTitle = this.panelTitle; if (isFetchRequired) { - this.filtersSearchSource.setField('filter', this.input.filters); - this.filtersSearchSource.setField('query', this.input.query); + this.filtersSearchSource!.setField('filter', this.input.filters); + this.filtersSearchSource!.setField('query', this.input.query); this.fetch(); diff --git a/src/legacy/ui/public/courier/fetch/fetch_soon.js b/src/legacy/ui/public/courier/fetch/fetch_soon.js index a48db3b242be6..8e9a16f817eaf 100644 --- a/src/legacy/ui/public/courier/fetch/fetch_soon.js +++ b/src/legacy/ui/public/courier/fetch/fetch_soon.js @@ -19,15 +19,20 @@ import { callClient } from './call_client'; import { npSetup } from 'ui/new_platform'; +import chrome from '../../chrome'; const config = npSetup.core.uiSettings; const esShardTimeout = npSetup.core.injectedMetadata.getInjectedVar('esShardTimeout'); + /** - * This is usually the right fetch provider to use, rather than FetchNowProvider, as this class introduces - * a slight delay in the request process to allow multiple requests to queue up (e.g. when a dashboard - * is loading). + * This function introduces a slight delay in the request process to allow multiple requests to queue + * up (e.g. when a dashboard is loading). */ -export function FetchSoonProvider(es) { +export async function fetchSoon(request, options) { + const delay = config.get('courier:batchSearches') ? 50 : 0; + return delayedFetch(request, options, delay); +} + /** * Delays executing a function for a given amount of time, and returns a promise that resolves * with the result. @@ -35,42 +40,38 @@ export function FetchSoonProvider(es) { * @param ms The number of milliseconds to wait * @return Promise A promise that resolves with the result of executing the function */ - function delay(fn, ms) { - return new Promise(resolve => { - setTimeout(() => resolve(fn()), ms); - }); - } +function delay(fn, ms) { + return new Promise(resolve => { + setTimeout(() => resolve(fn()), ms); + }); +} - // The current batch/queue of requests to fetch - let requestsToFetch = []; - let requestOptions = []; +// The current batch/queue of requests to fetch +let requestsToFetch = []; +let requestOptions = []; - // The in-progress fetch (if there is one) - let fetchInProgress = null; +// The in-progress fetch (if there is one) +let fetchInProgress = null; - /** +/** * Delay fetching for a given amount of time, while batching up the requests to be fetched. * Returns a promise that resolves with the response for the given request. * @param request The request to fetch * @param ms The number of milliseconds to wait (and batch requests) * @return Promise The response for the given request */ - async function delayedFetch(request, options, ms) { - const i = requestsToFetch.length; - requestsToFetch = [...requestsToFetch, request]; - requestOptions = [...requestOptions, options]; - const responses = await (fetchInProgress = fetchInProgress || delay(() => { - const response = callClient(requestsToFetch, requestOptions, { es, config, esShardTimeout }); - requestsToFetch = []; - requestOptions = []; - fetchInProgress = null; - return response; - }, ms)); - return responses[i]; - } - - this.fetch = (request, options) => { - const delay = config.get('courier:batchSearches') ? 50 : 0; - return delayedFetch(request, options, delay); - }; +async function delayedFetch(request, options, ms) { + const $injector = await chrome.dangerouslyGetActiveInjector(); + const es = $injector.get('es'); + const i = requestsToFetch.length; + requestsToFetch = [...requestsToFetch, request]; + requestOptions = [...requestOptions, options]; + const responses = await (fetchInProgress = fetchInProgress || delay(() => { + const response = callClient(requestsToFetch, requestOptions, { es, config, esShardTimeout }); + requestsToFetch = []; + requestOptions = []; + fetchInProgress = null; + return response; + }, ms)); + return responses[i]; } diff --git a/src/legacy/ui/public/courier/fetch/index.js b/src/legacy/ui/public/courier/fetch/index.js index a5daaca5cb2c3..7b89dea1a110c 100644 --- a/src/legacy/ui/public/courier/fetch/index.js +++ b/src/legacy/ui/public/courier/fetch/index.js @@ -17,5 +17,5 @@ * under the License. */ -export { FetchSoonProvider } from './fetch_soon'; +export * from './fetch_soon'; export * from './get_search_params'; diff --git a/src/legacy/ui/public/courier/index.js b/src/legacy/ui/public/courier/index.js index 01ef07df26670..02fce2caf1273 100644 --- a/src/legacy/ui/public/courier/index.js +++ b/src/legacy/ui/public/courier/index.js @@ -19,7 +19,7 @@ import './courier'; -export { SearchSourceProvider } from './search_source'; +export { SearchSource } from './search_source'; export { addSearchStrategy, diff --git a/src/legacy/ui/public/courier/search_source/index.js b/src/legacy/ui/public/courier/search_source/index.js index 5ec7cc315db1c..dcae7b3d2ff05 100644 --- a/src/legacy/ui/public/courier/search_source/index.js +++ b/src/legacy/ui/public/courier/search_source/index.js @@ -17,4 +17,4 @@ * under the License. */ -export { SearchSourceProvider } from './search_source'; +export { SearchSource } from './search_source'; diff --git a/src/legacy/ui/public/courier/search_source/search_source.d.ts b/src/legacy/ui/public/courier/search_source/search_source.d.ts index 11406ff3da824..6e7d11f43f052 100644 --- a/src/legacy/ui/public/courier/search_source/search_source.d.ts +++ b/src/legacy/ui/public/courier/search_source/search_source.d.ts @@ -17,4 +17,25 @@ * under the License. */ -export type SearchSource = any; +export declare class SearchSource { + setPreferredSearchStrategyId: (searchStrategyId: string) => void; + getPreferredSearchStrategyId: () => string; + setFields: (newFields: any) => SearchSource; + setField: (field: string, value: any) => SearchSource; + getId: () => string; + getFields: () => any; + getField: (field: string) => any; + getOwnField: () => any; + create: () => SearchSource; + createCopy: () => SearchSource; + createChild: (options?: any) => SearchSource; + setParent: (parent: SearchSource | boolean) => SearchSource; + getParent: () => SearchSource | undefined; + fetch: (options: any) => Promise; + onRequestStart: ( + handler: (searchSource: SearchSource, request: any, options: any) => void + ) => void; + getSearchRequestBody: () => any; + destroy: () => void; + history: any[]; +} diff --git a/src/legacy/ui/public/courier/search_source/search_source.js b/src/legacy/ui/public/courier/search_source/search_source.js index a17a7aae86a09..f753ba330db18 100644 --- a/src/legacy/ui/public/courier/search_source/search_source.js +++ b/src/legacy/ui/public/courier/search_source/search_source.js @@ -75,10 +75,10 @@ import { buildEsQuery, getEsQueryConfig, filterMatchesIndex } from '@kbn/es-quer import { normalizeSortRequest } from './_normalize_sort_request'; -import { FetchSoonProvider } from '../fetch'; +import { fetchSoon } from '../fetch'; import { fieldWildcardFilter } from '../../field_wildcard'; import { getHighlightRequest } from '../../../../../plugins/data/common/highlight'; -import chrome from '../../chrome'; +import { npSetup } from 'ui/new_platform'; const FIELDS = [ 'type', @@ -111,249 +111,247 @@ function isIndexPattern(val) { return Boolean(val && typeof val.title === 'string'); } -const config = chrome.getUiSettingsClient(); +const config = npSetup.core.uiSettings; const getConfig = (...args) => config.get(...args); const forIp = Symbol('for which index pattern?'); -export function SearchSourceProvider(Private) { - const fetchSoon = Private(FetchSoonProvider); - class SearchSource { - constructor(initialFields) { - this._id = _.uniqueId('data_source'); +export class SearchSource { + constructor(initialFields) { + this._id = _.uniqueId('data_source'); - this._searchStrategyId = undefined; - this._fields = parseInitialFields(initialFields); - this._parent = undefined; + this._searchStrategyId = undefined; + this._fields = parseInitialFields(initialFields); + this._parent = undefined; - this.history = []; - this._requestStartHandlers = []; - this._inheritOptions = {}; + this.history = []; + this._requestStartHandlers = []; + this._inheritOptions = {}; - this._filterPredicates = [ - (filter) => { + this._filterPredicates = [ + (filter) => { // remove null/undefined filters - return filter; - }, - (filter) => { - const disabled = _.get(filter, 'meta.disabled'); - return disabled === undefined || disabled === false; - }, - (filter, data) => { - const index = data.index || this.getField('index'); - return !config.get('courier:ignoreFilterIfFieldNotInIndex') || filterMatchesIndex(filter, index); - } - ]; - } + return filter; + }, + (filter) => { + const disabled = _.get(filter, 'meta.disabled'); + return disabled === undefined || disabled === false; + }, + (filter, data) => { + const index = data.index || this.getField('index'); + return !config.get('courier:ignoreFilterIfFieldNotInIndex') || filterMatchesIndex(filter, index); + } + ]; + } - /***** + /***** * PUBLIC API *****/ - setPreferredSearchStrategyId(searchStrategyId) { - this._searchStrategyId = searchStrategyId; - } - - getPreferredSearchStrategyId() { - return this._searchStrategyId; - } - - setFields(newFields) { - this._fields = newFields; - return this; - } - - setField = (field, value) => { - if (!FIELDS.includes(field)) { - throw new Error(`Can't set field '${field}' on SearchSource. Acceptable fields are: ${FIELDS.join(', ')}.`); - } + setPreferredSearchStrategyId(searchStrategyId) { + this._searchStrategyId = searchStrategyId; + } - if (field === 'index') { - const fields = this._fields; + getPreferredSearchStrategyId() { + return this._searchStrategyId; + } - const hasSource = fields.source; - const sourceCameFromIp = hasSource && fields.source.hasOwnProperty(forIp); - const sourceIsForOurIp = sourceCameFromIp && fields.source[forIp] === fields.index; - if (sourceIsForOurIp) { - delete fields.source; - } + setFields(newFields) { + this._fields = newFields; + return this; + } - if (value === null || value === undefined) { - delete fields.index; - return this; - } + setField(field, value) { + if (!FIELDS.includes(field)) { + throw new Error(`Can't set field '${field}' on SearchSource. Acceptable fields are: ${FIELDS.join(', ')}.`); + } - if (!isIndexPattern(value)) { - throw new TypeError('expected indexPattern to be an IndexPattern duck.'); - } + if (field === 'index') { + const fields = this._fields; - fields[field] = value; - if (!fields.source) { - // imply source filtering based on the index pattern, but allow overriding - // it by simply setting another field for "source". When index is changed - fields.source = function () { - return value.getSourceFiltering(); - }; - fields.source[forIp] = value; - } + const hasSource = fields.source; + const sourceCameFromIp = hasSource && fields.source.hasOwnProperty(forIp); + const sourceIsForOurIp = sourceCameFromIp && fields.source[forIp] === fields.index; + if (sourceIsForOurIp) { + delete fields.source; + } + if (value === null || value === undefined) { + delete fields.index; return this; } - if (value == null) { - delete this._fields[field]; - return this; + if (!isIndexPattern(value)) { + throw new TypeError('expected indexPattern to be an IndexPattern duck.'); } - this._fields[field] = value; - return this; - }; + fields[field] = value; + if (!fields.source) { + // imply source filtering based on the index pattern, but allow overriding + // it by simply setting another field for "source". When index is changed + fields.source = function () { + return value.getSourceFiltering(); + }; + fields.source[forIp] = value; + } - getId() { - return this._id; + return this; } - getFields() { - return _.clone(this._fields); + if (value == null) { + delete this._fields[field]; + return this; } - /** - * Get fields from the fields - */ - getField = field => { - if (!FIELDS.includes(field)) { - throw new Error(`Can't get field '${field}' from SearchSource. Acceptable fields are: ${FIELDS.join(', ')}.`); - } + this._fields[field] = value; + return this; + } - let searchSource = this; + getId() { + return this._id; + } - while (searchSource) { - const value = searchSource._fields[field]; - if (value !== void 0) { - return value; - } + getFields() { + return _.clone(this._fields); + } - searchSource = searchSource.getParent(); - } - }; + /** + * Get fields from the fields + */ + getField(field) { + if (!FIELDS.includes(field)) { + throw new Error(`Can't get field '${field}' from SearchSource. Acceptable fields are: ${FIELDS.join(', ')}.`); + } - /** - * Get the field from our own fields, don't traverse up the chain - */ - getOwnField(field) { - if (!FIELDS.includes(field)) { - throw new Error(`Can't get field '${field}' from SearchSource. Acceptable fields are: ${FIELDS.join(', ')}.`); - } + let searchSource = this; - const value = this._fields[field]; + while (searchSource) { + const value = searchSource._fields[field]; if (value !== void 0) { return value; } - } - create() { - return new SearchSource(); + searchSource = searchSource.getParent(); } + } - createCopy() { - const json = angular.toJson(this._fields); - const newSearchSource = new SearchSource(json); - // when serializing the internal fields we lose the internal classes used in the index - // pattern, so we have to set it again to workaround this behavior - newSearchSource.setField('index', this.getField('index')); - newSearchSource.setParent(this.getParent()); - return newSearchSource; + /** + * Get the field from our own fields, don't traverse up the chain + */ + getOwnField(field) { + if (!FIELDS.includes(field)) { + throw new Error(`Can't get field '${field}' from SearchSource. Acceptable fields are: ${FIELDS.join(', ')}.`); } - createChild(options = {}) { - const childSearchSource = new SearchSource(); - childSearchSource.setParent(this, options); - return childSearchSource; + const value = this._fields[field]; + if (value !== void 0) { + return value; } + } - /** + create() { + return new SearchSource(); + } + + createCopy() { + const json = angular.toJson(this._fields); + const newSearchSource = new SearchSource(json); + // when serializing the internal fields we lose the internal classes used in the index + // pattern, so we have to set it again to workaround this behavior + newSearchSource.setField('index', this.getField('index')); + newSearchSource.setParent(this.getParent()); + return newSearchSource; + } + + createChild(options = {}) { + const childSearchSource = new SearchSource(); + childSearchSource.setParent(this, options); + return childSearchSource; + } + + /** * Set a searchSource that this source should inherit from * @param {SearchSource} searchSource - the parent searchSource * @return {this} - chainable */ - setParent(parent, options = {}) { - this._parent = parent; - this._inheritOptions = options; - return this; - } + setParent(parent, options = {}) { + this._parent = parent; + this._inheritOptions = options; + return this; + } - /** + /** * Get the parent of this SearchSource * @return {undefined|searchSource} */ - getParent() { - return this._parent || undefined; - } + getParent() { + return this._parent || undefined; + } - /** + /** * Fetch this source and reject the returned Promise on error * * @async */ - async fetch(options) { - const searchRequest = await this._flatten(); - await this.requestIsStarting(searchRequest, options); - return fetchSoon.fetch(searchRequest, { - ...(this._searchStrategyId && { searchStrategyId: this._searchStrategyId }), - ...options, - }); - } + async fetch(options) { + const searchRequest = await this._flatten(); + await this.requestIsStarting(searchRequest, options); + return fetchSoon(searchRequest, { + ...(this._searchStrategyId && { searchStrategyId: this._searchStrategyId }), + ...options, + }); + } - /** + /** * Add a handler that will be notified whenever requests start * @param {Function} handler * @return {undefined} */ - onRequestStart(handler) { - this._requestStartHandlers.push(handler); - } + onRequestStart(handler) { + this._requestStartHandlers.push(handler); + } - /** + /** * Called by requests of this search source when they are started * @param {Courier.Request} request * @param options * @return {Promise} */ - requestIsStarting(request, options) { - 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(); - } + requestIsStarting(request, options) { + 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.all(handlers.map(fn => fn(this, request, options))); } - async getSearchRequestBody() { - const searchRequest = await this._flatten(); - return searchRequest.body; - } + return Promise.all(handlers.map(fn => fn(this, request, options))); + } - /** + async getSearchRequestBody() { + const searchRequest = await this._flatten(); + return searchRequest.body; + } + + /** * Completely destroy the SearchSource. * @return {undefined} */ - destroy() { - this._requestStartHandlers.length = 0; - } + destroy() { + this._requestStartHandlers.length = 0; + } - /****** + /****** * PRIVATE APIS ******/ - /** + /** * Used to merge properties into the data within ._flatten(). * The data is passed in and modified by the function * @@ -362,191 +360,188 @@ export function SearchSourceProvider(Private) { * @param {*} key - The key of `val` * @return {undefined} */ - _mergeProp(data, val, key) { - if (typeof val === 'function') { - const source = this; - return Promise.resolve(val(this)) - .then(function (newVal) { - return source._mergeProp(data, newVal, key); - }); - } + _mergeProp(data, val, key) { + if (typeof val === 'function') { + const source = this; + return Promise.resolve(val(this)) + .then(function (newVal) { + return source._mergeProp(data, newVal, key); + }); + } - if (val == null || !key || !_.isString(key)) return; + if (val == null || !key || !_.isString(key)) return; - switch (key) { - case 'filter': - let filters = Array.isArray(val) ? val : [val]; + switch (key) { + case 'filter': + let filters = Array.isArray(val) ? val : [val]; - filters = filters.filter(filter => { - return this._filterPredicates.every(predicate => predicate(filter, data)); - }); + filters = filters.filter(filter => { + return this._filterPredicates.every(predicate => predicate(filter, data)); + }); - data.filters = [...(data.filters || []), ...filters]; - return; - case 'index': - case 'type': - case 'id': - case 'highlightAll': - if (key && data[key] == null) { - data[key] = val; - } - return; - case 'searchAfter': - key = 'search_after'; - addToBody(); - break; - case 'source': - key = '_source'; - addToBody(); - break; - case 'sort': - val = normalizeSortRequest(val, this.getField('index'), config.get('sort:options')); - addToBody(); - break; - case 'query': - data.query = (data.query || []).concat(val); - break; - case 'fields': - data[key] = _.uniq([...(data[key] || []), ...val]); - break; - default: - addToBody(); - } + data.filters = [...(data.filters || []), ...filters]; + return; + case 'index': + case 'type': + case 'id': + case 'highlightAll': + if (key && data[key] == null) { + data[key] = val; + } + return; + case 'searchAfter': + key = 'search_after'; + addToBody(); + break; + case 'source': + key = '_source'; + addToBody(); + break; + case 'sort': + val = normalizeSortRequest(val, this.getField('index'), config.get('sort:options')); + addToBody(); + break; + case 'query': + data.query = (data.query || []).concat(val); + break; + case 'fields': + data[key] = _.uniq([...(data[key] || []), ...val]); + break; + default: + addToBody(); + } - /** + /** * Add the key and val to the body of the request */ - function addToBody() { - data.body = data.body || {}; - // ignore if we already have a value - if (data.body[key] == null) { - data.body[key] = val; - } + function addToBody() { + data.body = data.body || {}; + // ignore if we already have a value + if (data.body[key] == null) { + data.body[key] = val; } } + } - /** + /** * Walk the inheritance chain of a source and return it's * flat representation (taking into account merging rules) * @returns {Promise} * @resolved {Object|null} - the flat data of the SearchSource */ - _flatten() { - // the merged data of this dataSource and it's ancestors - const flatData = {}; - - // function used to write each property from each data object in the chain to flat data - const root = this; - - // start the chain at this source - let current = this; - - // call the ittr and return it's promise - return (function ittr() { - // iterate the _fields object (not array) and - // pass each key:value pair to source._mergeProp. if _mergeProp - // returns a promise, then wait for it to complete and call _mergeProp again - return Promise.all(_.map(current._fields, function ittr(value, key) { - if (value instanceof Promise) { - return value.then(function (value) { - return ittr(value, key); - }); - } - - const prom = root._mergeProp(flatData, value, key); - return prom instanceof Promise ? prom : null; - })) - .then(function () { - // move to this sources parent - const parent = current.getParent(); - // keep calling until we reach the top parent - if (parent) { - current = parent; - return ittr(); - } + _flatten() { + // the merged data of this dataSource and it's ancestors + const flatData = {}; + + // function used to write each property from each data object in the chain to flat data + const root = this; + + // start the chain at this source + let current = this; + + // call the ittr and return it's promise + return (function ittr() { + // iterate the _fields object (not array) and + // pass each key:value pair to source._mergeProp. if _mergeProp + // returns a promise, then wait for it to complete and call _mergeProp again + return Promise.all(_.map(current._fields, function ittr(value, key) { + if (value instanceof Promise) { + return value.then(function (value) { + return ittr(value, key); }); - }()) + } + + const prom = root._mergeProp(flatData, value, key); + return prom instanceof Promise ? prom : null; + })) .then(function () { - // This is down here to prevent the circular dependency - flatData.body = flatData.body || {}; - - const computedFields = flatData.index.getComputedFields(); - flatData.body.stored_fields = computedFields.storedFields; - flatData.body.script_fields = flatData.body.script_fields || {}; - flatData.body.docvalue_fields = flatData.body.docvalue_fields || []; - - _.extend(flatData.body.script_fields, computedFields.scriptFields); - flatData.body.docvalue_fields = _.union(flatData.body.docvalue_fields, computedFields.docvalueFields); - - if (flatData.body._source) { - // exclude source fields for this index pattern specified by the user - const filter = fieldWildcardFilter(flatData.body._source.excludes, config.get('metaFields')); - flatData.body.docvalue_fields = flatData.body.docvalue_fields.filter( - docvalueField => filter(docvalueField.field) - ); + // move to this sources parent + const parent = current.getParent(); + // keep calling until we reach the top parent + if (parent) { + current = parent; + return ittr(); } + }); + }()) + .then(function () { + // This is down here to prevent the circular dependency + flatData.body = flatData.body || {}; + + const computedFields = flatData.index.getComputedFields(); + flatData.body.stored_fields = computedFields.storedFields; + flatData.body.script_fields = flatData.body.script_fields || {}; + flatData.body.docvalue_fields = flatData.body.docvalue_fields || []; + + _.extend(flatData.body.script_fields, computedFields.scriptFields); + flatData.body.docvalue_fields = _.union(flatData.body.docvalue_fields, computedFields.docvalueFields); + + if (flatData.body._source) { + // exclude source fields for this index pattern specified by the user + const filter = fieldWildcardFilter(flatData.body._source.excludes, config.get('metaFields')); + flatData.body.docvalue_fields = flatData.body.docvalue_fields.filter( + docvalueField => filter(docvalueField.field) + ); + } - // if we only want to search for certain fields - const fields = flatData.fields; - if (fields) { - // filter out the docvalue_fields, and script_fields to only include those that we are concerned with - flatData.body.docvalue_fields = _.intersection(flatData.body.docvalue_fields, fields); - flatData.body.script_fields = _.pick(flatData.body.script_fields, fields); - - // request the remaining fields from both stored_fields and _source - const remainingFields = _.difference(fields, _.keys(flatData.body.script_fields)); - flatData.body.stored_fields = remainingFields; - _.set(flatData.body, '_source.includes', remainingFields); - } + // if we only want to search for certain fields + const fields = flatData.fields; + if (fields) { + // filter out the docvalue_fields, and script_fields to only include those that we are concerned with + flatData.body.docvalue_fields = _.intersection(flatData.body.docvalue_fields, fields); + flatData.body.script_fields = _.pick(flatData.body.script_fields, fields); + + // request the remaining fields from both stored_fields and _source + const remainingFields = _.difference(fields, _.keys(flatData.body.script_fields)); + flatData.body.stored_fields = remainingFields; + _.set(flatData.body, '_source.includes', remainingFields); + } - const esQueryConfigs = getEsQueryConfig(config); - flatData.body.query = buildEsQuery(flatData.index, flatData.query, flatData.filters, esQueryConfigs); + const esQueryConfigs = getEsQueryConfig(config); + flatData.body.query = buildEsQuery(flatData.index, flatData.query, flatData.filters, esQueryConfigs); - if (flatData.highlightAll != null) { - if (flatData.highlightAll && flatData.body.query) { - flatData.body.highlight = getHighlightRequest(flatData.body.query, getConfig); - } - delete flatData.highlightAll; + if (flatData.highlightAll != null) { + if (flatData.highlightAll && flatData.body.query) { + flatData.body.highlight = getHighlightRequest(flatData.body.query, getConfig); } + delete flatData.highlightAll; + } - /** + /** * Translate a filter into a query to support es 3+ * @param {Object} filter - The filter to translate * @return {Object} the query version of that filter */ - const translateToQuery = function (filter) { - if (!filter) return; + const translateToQuery = function (filter) { + if (!filter) return; - if (filter.query) { - return filter.query; - } + if (filter.query) { + return filter.query; + } - return filter; - }; + return filter; + }; - // re-write filters within filter aggregations - (function recurse(aggBranch) { - if (!aggBranch) return; - Object.keys(aggBranch).forEach(function (id) { - const agg = aggBranch[id]; + // re-write filters within filter aggregations + (function recurse(aggBranch) { + if (!aggBranch) return; + Object.keys(aggBranch).forEach(function (id) { + const agg = aggBranch[id]; - if (agg.filters) { - // translate filters aggregations - const filters = agg.filters.filters; + if (agg.filters) { + // translate filters aggregations + const filters = agg.filters.filters; - Object.keys(filters).forEach(function (filterId) { - filters[filterId] = translateToQuery(filters[filterId]); - }); - } + Object.keys(filters).forEach(function (filterId) { + filters[filterId] = translateToQuery(filters[filterId]); + }); + } - recurse(agg.aggs || agg.aggregations); - }); - }(flatData.body.aggs || flatData.body.aggregations)); + recurse(agg.aggs || agg.aggregations); + }); + }(flatData.body.aggs || flatData.body.aggregations)); - return flatData; - }); - } + return flatData; + }); } - - return SearchSource; } diff --git a/src/legacy/ui/public/saved_objects/saved_object.js b/src/legacy/ui/public/saved_objects/saved_object.js index f5c62372db10f..d3727474a11fa 100644 --- a/src/legacy/ui/public/saved_objects/saved_object.js +++ b/src/legacy/ui/public/saved_objects/saved_object.js @@ -34,7 +34,7 @@ import _ from 'lodash'; import { InvalidJSONProperty, SavedObjectNotFound } from '../errors'; import { expandShorthand } from '../utils/mapping_setup'; -import { SearchSourceProvider } from '../courier/search_source'; +import { SearchSource } from '../courier'; import { findObjectByTitle } from './find_object_by_title'; import { SavedObjectsClientProvider } from './saved_objects_client_provider'; import { migrateLegacyQuery } from '../utils/migrate_legacy_query'; @@ -68,7 +68,6 @@ function isErrorNonFatal(error) { export function SavedObjectProvider(Promise, Private, confirmModalPromise, indexPatterns) { const savedObjectsClient = Private(SavedObjectsClientProvider); - const SearchSource = Private(SearchSourceProvider); /** * The SavedObject class is a base class for saved objects loaded from the server and diff --git a/src/legacy/ui/public/vis/request_handlers/courier.js b/src/legacy/ui/public/vis/request_handlers/courier.js deleted file mode 100644 index 88b27f33b47da..0000000000000 --- a/src/legacy/ui/public/vis/request_handlers/courier.js +++ /dev/null @@ -1,173 +0,0 @@ -/* - * Licensed to Elasticsearch B.V. under one or more contributor - * license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright - * ownership. Elasticsearch B.V. licenses this file to you under - * the Apache License, Version 2.0 (the "License"); you may - * not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ - -import { has } from 'lodash'; -import { i18n } from '@kbn/i18n'; -import { VisRequestHandlersRegistryProvider } from '../../registry/vis_request_handlers'; -import { calculateObjectHash } from '../lib/calculate_object_hash'; -import { getRequestInspectorStats, getResponseInspectorStats } from '../../courier/utils/courier_inspector_utils'; -import { tabifyAggResponse } from '../../agg_response/tabify/tabify'; -import { buildTabularInspectorData } from '../../inspector/build_tabular_inspector_data'; -import { getTime } from '../../timefilter/get_time'; - -const CourierRequestHandlerProvider = function () { - - return { - name: 'courier', - handler: async function ({ - searchSource, - aggs, - timeRange, - query, - filters, - forceFetch, - partialRows, - metricsAtAllLevels, - inspectorAdapters, - queryFilter, - abortSignal, - }) { - // Create a new search source that inherits the original search source - // but has the appropriate 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 timeFilterSearchSource = searchSource.createChild({ callParentStartHandlers: true }); - const requestSearchSource = timeFilterSearchSource.createChild({ callParentStartHandlers: true }); - - aggs.setTimeRange(timeRange); - - // For now we need to mirror the history of the passed search source, since - // the request inspector wouldn't work otherwise. - Object.defineProperty(requestSearchSource, 'history', { - get() { - return searchSource.history; - }, - set(history) { - return searchSource.history = history; - } - }); - - requestSearchSource.setField('aggs', function () { - return aggs.toDsl(metricsAtAllLevels); - }); - - requestSearchSource.onRequestStart((searchSource, searchRequest, options) => { - return aggs.onSearchRequestStart(searchSource, searchRequest, options); - }); - - if (timeRange) { - timeFilterSearchSource.setField('filter', () => { - return getTime(searchSource.getField('index'), timeRange); - }); - } - - requestSearchSource.setField('filter', filters); - requestSearchSource.setField('query', query); - - const reqBody = await requestSearchSource.getSearchRequestBody(); - - const queryHash = calculateObjectHash(reqBody); - // We only need to reexecute the query, if forceFetch was true or the hash of the request body has changed - // since the last request - const shouldQuery = forceFetch || (searchSource.lastQuery !== queryHash); - - if (shouldQuery) { - inspectorAdapters.requests.reset(); - const request = inspectorAdapters.requests.start( - i18n.translate('common.ui.vis.courier.inspector.dataRequest.title', { defaultMessage: 'Data' }), - { - description: i18n.translate('common.ui.vis.courier.inspector.dataRequest.description', - { defaultMessage: 'This request queries Elasticsearch to fetch the data for the visualization.' }), - } - ); - request.stats(getRequestInspectorStats(requestSearchSource)); - - try { - const response = await requestSearchSource.fetch({ abortSignal }); - - searchSource.lastQuery = queryHash; - - request - .stats(getResponseInspectorStats(searchSource, response)) - .ok({ json: response }); - - searchSource.rawResponse = response; - } catch(e) { - // Log any error during request to the inspector - request.error({ json: e }); - throw e; - } finally { - // Add the request body no matter if things went fine or not - requestSearchSource.getSearchRequestBody().then(req => { - request.json(req); - }); - } - } - - // Note that rawResponse is not deeply cloned here, so downstream applications using courier - // must take care not to mutate it, or it could have unintended side effects, e.g. displaying - // response data incorrectly in the inspector. - let resp = searchSource.rawResponse; - for (const agg of aggs.aggs) { - if (has(agg, 'type.postFlightRequest')) { - resp = await agg.type.postFlightRequest( - resp, - aggs, - agg, - requestSearchSource, - inspectorAdapters, - abortSignal, - ); - } - } - - searchSource.finalResponse = resp; - - const parsedTimeRange = timeRange ? getTime(aggs.indexPattern, timeRange) : null; - const tabifyParams = { - metricsAtAllLevels, - partialRows, - timeRange: parsedTimeRange ? parsedTimeRange.range : undefined, - }; - - const tabifyCacheHash = calculateObjectHash({ tabifyAggs: aggs, ...tabifyParams }); - // We only need to reexecute tabify, if either we did a new request or some input params to tabify changed - const shouldCalculateNewTabify = shouldQuery || (searchSource.lastTabifyHash !== tabifyCacheHash); - - if (shouldCalculateNewTabify) { - searchSource.lastTabifyHash = tabifyCacheHash; - searchSource.tabifiedResponse = tabifyAggResponse(aggs, searchSource.finalResponse, tabifyParams); - } - - inspectorAdapters.data.setTabularLoader( - () => buildTabularInspectorData(searchSource.tabifiedResponse, queryFilter), - { returnsFormattedValues: true } - ); - - return searchSource.tabifiedResponse; - } - }; -}; - -VisRequestHandlersRegistryProvider.register(CourierRequestHandlerProvider); - -export { CourierRequestHandlerProvider }; diff --git a/src/legacy/ui/public/vis/vis.js b/src/legacy/ui/public/vis/vis.js index 16c8d05c2d062..644c702cb0dcd 100644 --- a/src/legacy/ui/public/vis/vis.js +++ b/src/legacy/ui/public/vis/vis.js @@ -34,7 +34,7 @@ import { AggConfigs } from './agg_configs'; import { PersistedState } from '../persisted_state'; import { FilterBarQueryFilterProvider } from '../filter_manager/query_filter'; import { updateVisualizationConfig } from './vis_update'; -import { SearchSourceProvider } from '../courier/search_source'; +import { SearchSource } from '../courier'; import { SavedObjectsClientProvider } from '../saved_objects'; import { timefilter } from '../timefilter'; @@ -43,7 +43,6 @@ import '../directives/bind'; export function VisProvider(Private, indexPatterns, getAppState) { const visTypes = Private(VisTypesRegistryProvider); const queryFilter = Private(FilterBarQueryFilterProvider); - const SearchSource = Private(SearchSourceProvider); const savedObjectsClient = Private(SavedObjectsClientProvider); class Vis extends EventEmitter { diff --git a/x-pack/legacy/plugins/maps/public/kibana_services.js b/x-pack/legacy/plugins/maps/public/kibana_services.js index 23a1380181c73..1adffa2fb2664 100644 --- a/x-pack/legacy/plugins/maps/public/kibana_services.js +++ b/x-pack/legacy/plugins/maps/public/kibana_services.js @@ -4,15 +4,13 @@ * you may not use this file except in compliance with the Elastic License. */ -import { uiModules } from 'ui/modules'; -import { SearchSourceProvider } from 'ui/courier'; import { getRequestInspectorStats, getResponseInspectorStats } from 'ui/courier/utils/courier_inspector_utils'; export { xpackInfo } from 'plugins/xpack_main/services/xpack_info'; import { setup as data } from '../../../../../src/legacy/core_plugins/data/public/legacy'; +export { SearchSource } from 'ui/courier'; export const indexPatternService = data.indexPatterns.indexPatterns; -export let SearchSource; export async function fetchSearchSourceAndRecordWithInspector({ searchSource, requestId, requestName, requestDesc, inspectorAdapters }) { const inspectorRequest = inspectorAdapters.requests.start( @@ -35,8 +33,3 @@ export async function fetchSearchSourceAndRecordWithInspector({ searchSource, re return resp; } - -uiModules.get('app/maps').run(($injector) => { - const Private = $injector.get('Private'); - SearchSource = Private(SearchSourceProvider); -}); From 493f88220c5c0e5559454ffa5610a7533895a9f1 Mon Sep 17 00:00:00 2001 From: Lukas Olson Date: Tue, 24 Sep 2019 14:54:50 -0700 Subject: [PATCH 08/25] Fix Karma tests --- .../__tests__/normalize_sort_request.js | 15 ++++++++------- .../ui/public/field_wildcard/field_wildcard.js | 2 +- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/src/legacy/ui/public/courier/search_source/__tests__/normalize_sort_request.js b/src/legacy/ui/public/courier/search_source/__tests__/normalize_sort_request.js index 33d049315e41a..7cffcd6d646b1 100644 --- a/src/legacy/ui/public/courier/search_source/__tests__/normalize_sort_request.js +++ b/src/legacy/ui/public/courier/search_source/__tests__/normalize_sort_request.js @@ -27,6 +27,7 @@ import _ from 'lodash'; describe('SearchSource#normalizeSortRequest', function () { let indexPattern; let normalizedSort; + const defaultSortOptions = { unmapped_type: 'boolean' }; beforeEach(ngMock.module('kibana')); beforeEach(ngMock.inject(function (Private) { @@ -42,7 +43,7 @@ describe('SearchSource#normalizeSortRequest', function () { it('should return an array', function () { const sortable = { someField: 'desc' }; - const result = normalizeSortRequest(sortable, indexPattern); + const result = normalizeSortRequest(sortable, indexPattern, defaultSortOptions); expect(result).to.be.an(Array); expect(result).to.eql(normalizedSort); // ensure object passed in is not mutated @@ -51,7 +52,7 @@ describe('SearchSource#normalizeSortRequest', function () { }); it('should make plain string sort into the more verbose format', function () { - const result = normalizeSortRequest([{ someField: 'desc' }], indexPattern); + const result = normalizeSortRequest([{ someField: 'desc' }], indexPattern, defaultSortOptions); expect(result).to.eql(normalizedSort); }); @@ -62,7 +63,7 @@ describe('SearchSource#normalizeSortRequest', function () { unmapped_type: 'boolean' } }]; - const result = normalizeSortRequest(sortState, indexPattern); + const result = normalizeSortRequest(sortState, indexPattern, defaultSortOptions); expect(result).to.eql(normalizedSort); }); @@ -84,11 +85,11 @@ describe('SearchSource#normalizeSortRequest', function () { } }; - let result = normalizeSortRequest(sortState, indexPattern); + let result = normalizeSortRequest(sortState, indexPattern, defaultSortOptions); expect(result).to.eql([normalizedSort]); sortState[fieldName] = { order: direction }; - result = normalizeSortRequest([sortState], indexPattern); + result = normalizeSortRequest([sortState], indexPattern, defaultSortOptions); expect(result).to.eql([normalizedSort]); }); @@ -103,7 +104,7 @@ describe('SearchSource#normalizeSortRequest', function () { order: direction, unmapped_type: 'boolean' }; - const result = normalizeSortRequest([sortState], indexPattern); + const result = normalizeSortRequest([sortState], indexPattern, defaultSortOptions); expect(result).to.eql([normalizedSort]); }); @@ -116,7 +117,7 @@ describe('SearchSource#normalizeSortRequest', function () { } }]; - const result = normalizeSortRequest(sortable, indexPattern); + const result = normalizeSortRequest(sortable, indexPattern, defaultSortOptions); expect(_.isEqual(result, expected)).to.be.ok(); }); diff --git a/src/legacy/ui/public/field_wildcard/field_wildcard.js b/src/legacy/ui/public/field_wildcard/field_wildcard.js index ffc78405c88bd..656641b20a98c 100644 --- a/src/legacy/ui/public/field_wildcard/field_wildcard.js +++ b/src/legacy/ui/public/field_wildcard/field_wildcard.js @@ -35,7 +35,7 @@ export function fieldWildcardMatcher(globs = [], metaFields) { } // Note that this will return an essentially noop function if globs is undefined. -export function fieldWildcardFilter(globs = [], metaFields) { +export function fieldWildcardFilter(globs = [], metaFields = []) { const matcher = fieldWildcardMatcher(globs, metaFields); return function filter(val) { return !matcher(val); From 68e752937b793f2fa39aa5192a97f948d5c3127e Mon Sep 17 00:00:00 2001 From: Lukas Olson Date: Wed, 25 Sep 2019 11:28:36 -0700 Subject: [PATCH 09/25] Separate callClient and handleResponse and add tests for each --- .../ui/public/courier/fetch/call_client.js | 48 +------- .../public/courier/fetch/call_client.test.js | 110 ++++++++++++++++++ .../public/courier/fetch/handle_response.js | 67 +++++++++++ .../courier/fetch/handle_response.test.js | 74 ++++++++++++ 4 files changed, 253 insertions(+), 46 deletions(-) create mode 100644 src/legacy/ui/public/courier/fetch/handle_response.js create mode 100644 src/legacy/ui/public/courier/fetch/handle_response.test.js diff --git a/src/legacy/ui/public/courier/fetch/call_client.js b/src/legacy/ui/public/courier/fetch/call_client.js index 615eddb52f2a6..971ae4c49a604 100644 --- a/src/legacy/ui/public/courier/fetch/call_client.js +++ b/src/legacy/ui/public/courier/fetch/call_client.js @@ -19,13 +19,9 @@ import { groupBy } from 'lodash'; import { getSearchStrategyForSearchRequest, getSearchStrategyById } from '../search_strategy'; -import { toastNotifications } from '../../notify/toasts'; -import { i18n } from '@kbn/i18n'; -import { EuiSpacer } from '@elastic/eui'; -import React from 'react'; -import { ShardFailureOpenModalButton } from './components/shard_failure_open_modal_button'; +import { handleResponse } from './handle_response'; -export function callClient(searchRequests, requestsOptions = [], { es, config, esShardTimeout }) { +export function callClient(searchRequests, requestsOptions = [], { es, config, esShardTimeout } = {}) { // Correlate the options with the request that they're associated with const requestOptionEntries = searchRequests.map((request, i) => [request, requestsOptions[i]]); const requestOptionsMap = new Map(requestOptionEntries); @@ -54,44 +50,4 @@ export function callClient(searchRequests, requestsOptions = [], { es, config, e return searchRequests.map(request => requestResponseMap.get(request)); } -export function handleResponse(request, response) { - if (response.timed_out) { - toastNotifications.addWarning({ - title: i18n.translate('common.ui.courier.fetch.requestTimedOutNotificationMessage', { - defaultMessage: 'Data might be incomplete because your request timed out', - }), - }); - } - - if (response._shards && response._shards.failed) { - const title = i18n.translate('common.ui.courier.fetch.shardsFailedNotificationMessage', { - defaultMessage: '{shardsFailed} of {shardsTotal} shards failed', - values: { - shardsFailed: response._shards.failed, - shardsTotal: response._shards.total, - }, - }); - const description = i18n.translate('common.ui.courier.fetch.shardsFailedNotificationDescription', { - defaultMessage: 'The data you are seeing might be incomplete or wrong.', - }); - const text = ( - <> - {description} - - - - ); - - toastNotifications.addWarning({ - title, - text, - }); - } - - return response; -} diff --git a/src/legacy/ui/public/courier/fetch/call_client.test.js b/src/legacy/ui/public/courier/fetch/call_client.test.js index 9880b336e76e5..463d6c59e479e 100644 --- a/src/legacy/ui/public/courier/fetch/call_client.test.js +++ b/src/legacy/ui/public/courier/fetch/call_client.test.js @@ -16,3 +16,113 @@ * specific language governing permissions and limitations * under the License. */ + +import { callClient } from './call_client'; +import { handleResponse } from './handle_response'; + +const mockResponses = [{}, {}]; +const mockAbortFns = [jest.fn(), jest.fn()]; +const mockSearchFns = [ + jest.fn(({ searchRequests }) => ({ + searching: Promise.resolve(Array(searchRequests.length).fill(mockResponses[0])), + abort: mockAbortFns[0] + })), + jest.fn(({ searchRequests }) => ({ + searching: Promise.resolve(Array(searchRequests.length).fill(mockResponses[1])), + abort: mockAbortFns[1] + })) +]; +const mockSearchStrategies = mockSearchFns.map((search, i) => ({ search, id: i })); + +jest.mock('./handle_response', () => ({ + handleResponse: jest.fn((request, response) => response) +})); + +jest.mock('../search_strategy', () => ({ + getSearchStrategyForSearchRequest: request => mockSearchStrategies[request._searchStrategyId], + getSearchStrategyById: id => mockSearchStrategies[id] +})); + +describe('callClient', () => { + beforeEach(() => { + handleResponse.mockClear(); + mockAbortFns.forEach(fn => fn.mockClear()); + mockSearchFns.forEach(fn => fn.mockClear()); + }); + + test('Executes each search strategy with its group of matching requests', () => { + const searchRequests = [{ + _searchStrategyId: 0 + }, { + _searchStrategyId: 1 + }, { + _searchStrategyId: 0 + }, { + _searchStrategyId: 1 + }]; + + callClient(searchRequests); + + expect(mockSearchFns[0]).toBeCalled(); + expect(mockSearchFns[0].mock.calls[0][0].searchRequests).toEqual([searchRequests[0], searchRequests[2]]); + expect(mockSearchFns[1]).toBeCalled(); + expect(mockSearchFns[1].mock.calls[0][0].searchRequests).toEqual([searchRequests[1], searchRequests[3]]); + }); + + test('Passes the additional arguments it is given to the search strategy', () => { + const searchRequests = [{ + _searchStrategyId: 0 + }]; + const args = { es: {}, config: {}, esShardTimeout: 0 }; + + callClient(searchRequests, [], args); + + expect(mockSearchFns[0]).toBeCalled(); + expect(mockSearchFns[0].mock.calls[0][0]).toEqual({ searchRequests, ...args }); + }); + + test('Returns the responses in the original order', async () => { + const searchRequests = [{ + _searchStrategyId: 1 + }, { + _searchStrategyId: 0 + }]; + + const responses = await Promise.all(callClient(searchRequests)); + + expect(responses).toEqual([mockResponses[1], mockResponses[0]]); + }); + + test('Calls handleResponse with each request and response', async () => { + const searchRequests = [{ + _searchStrategyId: 0 + }, { + _searchStrategyId: 1 + }]; + + const responses = callClient(searchRequests); + await Promise.all(responses); + + expect(handleResponse).toBeCalledTimes(2); + expect(handleResponse).toBeCalledWith(searchRequests[0], mockResponses[0]); + expect(handleResponse).toBeCalledWith(searchRequests[1], mockResponses[1]); + }); + + test('If passed an abortSignal, calls abort on the strategy if the signal is aborted', () => { + const searchRequests = [{ + _searchStrategyId: 0 + }, { + _searchStrategyId: 1 + }]; + const abortController = new AbortController(); + const requestOptions = [{ + abortSignal: abortController.signal + }]; + + callClient(searchRequests, requestOptions); + abortController.abort(); + + expect(mockAbortFns[0]).toBeCalled(); + expect(mockAbortFns[1]).not.toBeCalled(); + }); +}); diff --git a/src/legacy/ui/public/courier/fetch/handle_response.js b/src/legacy/ui/public/courier/fetch/handle_response.js new file mode 100644 index 0000000000000..fb2797369d78f --- /dev/null +++ b/src/legacy/ui/public/courier/fetch/handle_response.js @@ -0,0 +1,67 @@ +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + + +import React from 'react'; +import { toastNotifications } from '../../notify/toasts'; +import { i18n } from '@kbn/i18n'; +import { EuiSpacer } from '@elastic/eui'; +import { ShardFailureOpenModalButton } from './components/shard_failure_open_modal_button'; + +export function handleResponse(request, response) { + if (response.timed_out) { + toastNotifications.addWarning({ + title: i18n.translate('common.ui.courier.fetch.requestTimedOutNotificationMessage', { + defaultMessage: 'Data might be incomplete because your request timed out', + }), + }); + } + + if (response._shards && response._shards.failed) { + const title = i18n.translate('common.ui.courier.fetch.shardsFailedNotificationMessage', { + defaultMessage: '{shardsFailed} of {shardsTotal} shards failed', + values: { + shardsFailed: response._shards.failed, + shardsTotal: response._shards.total, + }, + }); + const description = i18n.translate('common.ui.courier.fetch.shardsFailedNotificationDescription', { + defaultMessage: 'The data you are seeing might be incomplete or wrong.', + }); + + const text = ( + <> + {description} + + + + ); + + toastNotifications.addWarning({ + title, + text, + }); + } + + return response; +} diff --git a/src/legacy/ui/public/courier/fetch/handle_response.test.js b/src/legacy/ui/public/courier/fetch/handle_response.test.js new file mode 100644 index 0000000000000..0836832e6c05a --- /dev/null +++ b/src/legacy/ui/public/courier/fetch/handle_response.test.js @@ -0,0 +1,74 @@ +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import { handleResponse } from './handle_response'; +import { toastNotifications } from '../../notify/toasts'; + +jest.mock('../../notify/toasts', () => { + return { + toastNotifications: { + addWarning: jest.fn() + } + }; +}); + +jest.mock('@kbn/i18n', () => { + return { + i18n: { + translate: (id, { defaultMessage }) => defaultMessage + } + }; +}); + +describe('handleResponse', () => { + beforeEach(() => { + toastNotifications.addWarning.mockReset(); + }); + + test('should notify if timed out', () => { + const request = { body: {} }; + const response = { + timed_out: true + }; + const result = handleResponse(request, response); + expect(result).toBe(response); + expect(toastNotifications.addWarning).toBeCalled(); + expect(toastNotifications.addWarning.mock.calls[0][0].title).toMatch('request timed out'); + }); + + test('should notify if shards failed', () => { + const request = { body: {} }; + const response = { + _shards: { + failed: true + } + }; + const result = handleResponse(request, response); + expect(result).toBe(response); + expect(toastNotifications.addWarning).toBeCalled(); + expect(toastNotifications.addWarning.mock.calls[0][0].title).toMatch('shards failed'); + }); + + test('returns the response', () => { + const request = {}; + const response = {}; + const result = handleResponse(request, response); + expect(result).toBe(response); + }); +}); From ca27f89fdb88701688c75142693b77012c60291f Mon Sep 17 00:00:00 2001 From: Lukas Olson Date: Wed, 25 Sep 2019 15:50:22 -0700 Subject: [PATCH 10/25] Add tests for fetchSoon --- .../ui/public/courier/fetch/fetch_soon.js | 13 +- .../public/courier/fetch/fetch_soon.test.js | 120 ++++++++++++++++++ .../courier/search_source/search_source.js | 18 ++- 3 files changed, 140 insertions(+), 11 deletions(-) diff --git a/src/legacy/ui/public/courier/fetch/fetch_soon.js b/src/legacy/ui/public/courier/fetch/fetch_soon.js index 8e9a16f817eaf..ef02beddcb59a 100644 --- a/src/legacy/ui/public/courier/fetch/fetch_soon.js +++ b/src/legacy/ui/public/courier/fetch/fetch_soon.js @@ -18,19 +18,14 @@ */ import { callClient } from './call_client'; -import { npSetup } from 'ui/new_platform'; -import chrome from '../../chrome'; - -const config = npSetup.core.uiSettings; -const esShardTimeout = npSetup.core.injectedMetadata.getInjectedVar('esShardTimeout'); /** * This function introduces a slight delay in the request process to allow multiple requests to queue * up (e.g. when a dashboard is loading). */ -export async function fetchSoon(request, options) { +export async function fetchSoon(request, options, { es, config, esShardTimeout }) { const delay = config.get('courier:batchSearches') ? 50 : 0; - return delayedFetch(request, options, delay); + return delayedFetch(request, options, { es, config, esShardTimeout }, delay); } /** @@ -60,9 +55,7 @@ let fetchInProgress = null; * @param ms The number of milliseconds to wait (and batch requests) * @return Promise The response for the given request */ -async function delayedFetch(request, options, ms) { - const $injector = await chrome.dangerouslyGetActiveInjector(); - const es = $injector.get('es'); +async function delayedFetch(request, options, { es, config, esShardTimeout }, ms) { const i = requestsToFetch.length; requestsToFetch = [...requestsToFetch, request]; requestOptions = [...requestOptions, options]; diff --git a/src/legacy/ui/public/courier/fetch/fetch_soon.test.js b/src/legacy/ui/public/courier/fetch/fetch_soon.test.js index 81af3922ba9b3..824a4ab7e12e3 100644 --- a/src/legacy/ui/public/courier/fetch/fetch_soon.test.js +++ b/src/legacy/ui/public/courier/fetch/fetch_soon.test.js @@ -17,4 +17,124 @@ * under the License. */ +import { fetchSoon } from './fetch_soon'; +import { callClient } from './call_client'; +function getMockConfig(config) { + const entries = Object.entries(config); + return new Map(entries); +} + +const mockResponses = { + 'foo': {}, + 'bar': {}, + 'baz': {}, +}; + +jest.useFakeTimers(); + +jest.mock('./call_client', () => ({ + callClient: jest.fn(requests => { + // Allow a request object to specify which mockResponse it wants to receive (_mockResponseId) + // in addition to how long to simulate waiting before returning a response (_waitMs) + const responses = requests.map(request => { + const waitMs = requests.reduce((total, request) => request._waitMs || 0, 0); + return new Promise(resolve => { + resolve(mockResponses[request._mockResponseId]); + }, waitMs); + }); + return Promise.resolve(responses); + }) +})); + +describe('fetchSoon', () => { + beforeEach(() => { + callClient.mockClear(); + }); + + test('should delay by 0ms if config is set to not batch searches', () => { + const config = getMockConfig({ + 'courier:batchSearches': false + }); + const request = {}; + const options = {}; + + fetchSoon(request, options, { config }); + + expect(callClient).not.toBeCalled(); + jest.advanceTimersByTime(0); + expect(callClient).toBeCalled(); + }); + + test('should delay by 50ms if config is set to batch searches', () => { + const config = getMockConfig({ + 'courier:batchSearches': true + }); + const request = {}; + const options = {}; + + fetchSoon(request, options, { config }); + + expect(callClient).not.toBeCalled(); + jest.advanceTimersByTime(0); + expect(callClient).not.toBeCalled(); + jest.advanceTimersByTime(50); + expect(callClient).toBeCalled(); + }); + + test('should send a batch of requests to callClient', () => { + const config = getMockConfig({ + 'courier:batchSearches': true + }); + const requests = [{ foo: 1 }, { foo: 2 }]; + const options = [{ bar: 1 }, { bar: 2 }]; + + requests.forEach((request, i) => { + fetchSoon(request, options[i], { config }); + }); + + jest.advanceTimersByTime(50); + expect(callClient).toBeCalledTimes(1); + expect(callClient.mock.calls[0][0]).toEqual(requests); + expect(callClient.mock.calls[0][1]).toEqual(options); + }); + + test('should return the response to the corresponding call for multiple batched requests', async () => { + const config = getMockConfig({ + 'courier:batchSearches': true + }); + const requests = [{ _mockResponseId: 'foo' }, { _mockResponseId: 'bar' }]; + + const promises = requests.map(request => { + return fetchSoon(request, {}, { config }); + }); + jest.advanceTimersByTime(50); + const results = await Promise.all(promises); + + expect(results).toEqual([mockResponses.foo, mockResponses.bar]); + }); + + test('should wait for the previous batch to start before starting a new batch', () => { + const config = getMockConfig({ + 'courier:batchSearches': true + }); + const firstBatch = [{ foo: 1 }, { foo: 2 }]; + const secondBatch = [{ bar: 1 }, { bar: 2 }]; + + firstBatch.forEach(request => { + fetchSoon(request, {}, { config }); + }); + jest.advanceTimersByTime(50); + secondBatch.forEach(request => { + fetchSoon(request, {}, { config }); + }); + + expect(callClient).toBeCalledTimes(1); + expect(callClient.mock.calls[0][0]).toEqual(firstBatch); + + jest.advanceTimersByTime(50); + + expect(callClient).toBeCalledTimes(2); + expect(callClient.mock.calls[1][0]).toEqual(secondBatch); + }); +}); diff --git a/src/legacy/ui/public/courier/search_source/search_source.js b/src/legacy/ui/public/courier/search_source/search_source.js index 1da617fbbd3ec..1910e02805104 100644 --- a/src/legacy/ui/public/courier/search_source/search_source.js +++ b/src/legacy/ui/public/courier/search_source/search_source.js @@ -79,6 +79,7 @@ import { fetchSoon } from '../fetch'; import { fieldWildcardFilter } from '../../field_wildcard'; import { getHighlightRequest } from '../../../../../plugins/data/common/field_formats'; import { npSetup } from 'ui/new_platform'; +import chrome from '../../chrome'; const FIELDS = [ 'type', @@ -111,6 +112,7 @@ function isIndexPattern(val) { return Boolean(val && typeof val.title === 'string'); } +const esShardTimeout = npSetup.core.injectedMetadata.getInjectedVar('esShardTimeout'); const config = npSetup.core.uiSettings; const getConfig = (...args) => config.get(...args); const forIp = Symbol('for which index pattern?'); @@ -293,12 +295,16 @@ export class SearchSource { * @async */ async fetch(options) { + const $injector = await chrome.dangerouslyGetActiveInjector(); + const es = $injector.get('es'); + const searchRequest = await this._flatten(); await this.requestIsStarting(searchRequest, options); + return fetchSoon(searchRequest, { ...(this._searchStrategyId && { searchStrategyId: this._searchStrategyId }), ...options, - }); + }, { es, config, esShardTimeout }); } /** @@ -351,6 +357,16 @@ export class SearchSource { * PRIVATE APIS ******/ + _getSearchConfig() { + return { + esShardTimeout, + includeFrozen: config.get('search:includeFrozen'), + maxConcurrentShardRequests: config.get('courier:maxConcurrentShardRequests'), + setRequestPreference: config.get('courier:setRequestPreference'), + customRequestPreference: config.get('courier:customRequestPreference'), + }; + } + /** * Used to merge properties into the data within ._flatten(). * The data is passed in and modified by the function From 9ff10c47a6a0cdcd89ac498986583e592e6e43ef Mon Sep 17 00:00:00 2001 From: Lukas Olson Date: Wed, 25 Sep 2019 16:34:43 -0700 Subject: [PATCH 11/25] Add back search source test and convert to jest --- .../public/courier/search_poll/search_poll.js | 60 ++-- .../courier/search_poll/search_poll.test.js | 20 -- .../search_source/search_source.test.js | 287 ++++++++++++++++++ 3 files changed, 317 insertions(+), 50 deletions(-) delete mode 100644 src/legacy/ui/public/courier/search_poll/search_poll.test.js diff --git a/src/legacy/ui/public/courier/search_poll/search_poll.js b/src/legacy/ui/public/courier/search_poll/search_poll.js index 81eb40ca1de87..f00c2a32e0ec6 100644 --- a/src/legacy/ui/public/courier/search_poll/search_poll.js +++ b/src/legacy/ui/public/courier/search_poll/search_poll.js @@ -28,41 +28,41 @@ export class SearchPoll { this._timerId = null; } - setIntervalInMs = intervalInMs => { - this._intervalInMs = _.parseInt(intervalInMs); - }; + setIntervalInMs = intervalInMs => { + this._intervalInMs = _.parseInt(intervalInMs); + }; - resume = () => { - this._isPolling = true; - this.resetTimer(); - }; + resume = () => { + this._isPolling = true; + this.resetTimer(); + }; - pause = () => { - this._isPolling = false; - this.clearTimer(); - }; + pause = () => { + this._isPolling = false; + this.clearTimer(); + }; - resetTimer = () => { - // Cancel the pending search and schedule a new one. - this.clearTimer(); + resetTimer = () => { + // Cancel the pending search and schedule a new one. + this.clearTimer(); - if (this._isPolling) { - this._timerId = setTimeout(this._search, this._intervalInMs); - } - }; + if (this._isPolling) { + this._timerId = setTimeout(this._search, this._intervalInMs); + } + }; - clearTimer = () => { - // Cancel the pending search, if there is one. - if (this._timerId) { - clearTimeout(this._timerId); - this._timerId = null; - } - }; + clearTimer = () => { + // Cancel the pending search, if there is one. + if (this._timerId) { + clearTimeout(this._timerId); + this._timerId = null; + } + }; - _search = () => { - // Schedule another search. - this.resetTimer(); + _search = () => { + // Schedule another search. + this.resetTimer(); - timefilter.notifyShouldFetch(); - }; + timefilter.notifyShouldFetch(); + }; } diff --git a/src/legacy/ui/public/courier/search_poll/search_poll.test.js b/src/legacy/ui/public/courier/search_poll/search_poll.test.js deleted file mode 100644 index 81af3922ba9b3..0000000000000 --- a/src/legacy/ui/public/courier/search_poll/search_poll.test.js +++ /dev/null @@ -1,20 +0,0 @@ -/* - * Licensed to Elasticsearch B.V. under one or more contributor - * license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright - * ownership. Elasticsearch B.V. licenses this file to you under - * the Apache License, Version 2.0 (the "License"); you may - * not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ - - diff --git a/src/legacy/ui/public/courier/search_source/search_source.test.js b/src/legacy/ui/public/courier/search_source/search_source.test.js index 81af3922ba9b3..828725c3f2d9f 100644 --- a/src/legacy/ui/public/courier/search_source/search_source.test.js +++ b/src/legacy/ui/public/courier/search_source/search_source.test.js @@ -17,4 +17,291 @@ * under the License. */ +import { SearchSource } from '../search_source'; +let mockConfig = {}; + +jest.mock('ui/new_platform', () => ({ + npSetup: { + core: { + uiSettings: { + get: key => mockConfig[key] + }, + injectedMetadata: { + getInjectedVar: () => 0, + } + } + } +})); + +jest.mock('../fetch', () => ({ + fetchSoon: jest.fn(), +})); + +const indexPattern = { title: 'foo' }; +const indexPattern2 = { title: 'foo' }; + +describe('SearchSource', function () { + describe('#setField()', function () { + it('sets the value for the property', function () { + const searchSource = new SearchSource(); + searchSource.setField('aggs', 5); + expect(searchSource.getField('aggs')).toBe(5); + }); + + it('throws an error if the property is not accepted', function () { + const searchSource = new SearchSource(); + expect(() => searchSource.setField('index', 5)).toThrow(); + }); + }); + + describe('#getField()', function () { + it('gets the value for the property', function () { + const searchSource = new SearchSource(); + searchSource.setField('aggs', 5); + expect(searchSource.getField('aggs')).toBe(5); + }); + + it('throws an error if the property is not accepted', function () { + const searchSource = new SearchSource(); + expect(() => searchSource.getField('unacceptablePropName')).toThrow(); + }); + }); + + describe(`#setField('index')`, function () { + describe('auto-sourceFiltering', function () { + describe('new index pattern assigned', function () { + it('generates a searchSource filter', function () { + const searchSource = new SearchSource(); + expect(searchSource.getField('index')).toBe(undefined); + expect(searchSource.getField('source')).toBe(undefined); + searchSource.setField('index', indexPattern); + expect(searchSource.getField('index')).toBe(indexPattern); + expect(typeof searchSource.getField('source')).toBe('function'); + }); + + it('removes created searchSource filter on removal', function () { + const searchSource = new SearchSource(); + searchSource.setField('index', indexPattern); + searchSource.setField('index', null); + expect(searchSource.getField('index')).toBe(undefined); + expect(searchSource.getField('source')).toBe(undefined); + }); + }); + + describe('new index pattern assigned over another', function () { + it('replaces searchSource filter with new', function () { + const searchSource = new SearchSource(); + searchSource.setField('index', indexPattern); + const searchSourceFilter1 = searchSource.getField('source'); + searchSource.setField('index', indexPattern2); + expect(searchSource.getField('index')).toBe(indexPattern2); + expect(typeof searchSource.getField('source')).toBe('function'); + expect(searchSource.getField('source')).not.toBe(searchSourceFilter1); + }); + + it('removes created searchSource filter on removal', function () { + const searchSource = new SearchSource(); + searchSource.setField('index', indexPattern); + searchSource.setField('index', indexPattern2); + searchSource.setField('index', null); + expect(searchSource.getField('index')).toBe(undefined); + expect(searchSource.getField('source')).toBe(undefined); + }); + }); + + describe('ip assigned before custom searchSource filter', function () { + it('custom searchSource filter becomes new searchSource', function () { + const searchSource = new SearchSource(); + const football = {}; + searchSource.setField('index', indexPattern); + expect(typeof searchSource.getField('source')).toBe('function'); + searchSource.setField('source', football); + expect(searchSource.getField('index')).toBe(indexPattern); + expect(searchSource.getField('source')).toBe(football); + }); + + it('custom searchSource stays after removal', function () { + const searchSource = new SearchSource(); + const football = {}; + searchSource.setField('index', indexPattern); + searchSource.setField('source', football); + searchSource.setField('index', null); + expect(searchSource.getField('index')).toBe(undefined); + expect(searchSource.getField('source')).toBe(football); + }); + }); + + describe('ip assigned after custom searchSource filter', function () { + it('leaves the custom filter in place', function () { + const searchSource = new SearchSource(); + const football = {}; + searchSource.setField('source', football); + searchSource.setField('index', indexPattern); + expect(searchSource.getField('index')).toBe(indexPattern); + expect(searchSource.getField('source')).toBe(football); + }); + + it('custom searchSource stays after removal', function () { + const searchSource = new SearchSource(); + const football = {}; + searchSource.setField('source', football); + searchSource.setField('index', indexPattern); + searchSource.setField('index', null); + expect(searchSource.getField('index')).toBe(undefined); + expect(searchSource.getField('source')).toBe(football); + }); + }); + }); + }); + + describe('#onRequestStart()', () => { + it('should be called when starting a request', async () => { + const searchSource = new SearchSource(); + const fn = jest.fn(); + searchSource.onRequestStart(fn); + const request = {}; + const options = {}; + searchSource.requestIsStarting(request, options); + expect(fn).toBeCalledWith(searchSource, request, options); + }); + + it('should not be called on parent searchSource', async () => { + const parent = new SearchSource(); + const searchSource = new SearchSource().setParent(parent); + + const fn = jest.fn(); + searchSource.onRequestStart(fn); + const parentFn = jest.fn(); + parent.onRequestStart(parentFn); + const request = {}; + const options = {}; + searchSource.requestIsStarting(request, options); + + expect(fn).toBeCalledWith(searchSource, request, options); + expect(parentFn).not.toBeCalled(); + }); + + it('should be called on parent searchSource if callParentStartHandlers is true', async () => { + const parent = new SearchSource(); + const searchSource = new SearchSource().setParent(parent, { callParentStartHandlers: true }); + + const fn = jest.fn(); + searchSource.onRequestStart(fn); + const parentFn = jest.fn(); + parent.onRequestStart(parentFn); + const request = {}; + const options = {}; + searchSource.requestIsStarting(request, options); + + expect(fn).toBeCalledWith(searchSource, request, options); + expect(parentFn).toBeCalledWith(searchSource, request, options); + }); + }); + + describe('#_mergeProp', function () { + describe('filter', function () { + let previousMockConfig; + let searchSource; + let state; + + beforeEach(function () { + previousMockConfig = mockConfig; + searchSource = new SearchSource(); + state = {}; + }); + + afterEach(() => { + mockConfig = previousMockConfig; + }); + + [null, undefined].forEach(falsyValue => { + it(`ignores ${falsyValue} filter`, function () { + searchSource._mergeProp(state, falsyValue, 'filter'); + expect(state.filters).toBe(undefined); + }); + }); + + [false, 0, '', NaN].forEach(falsyValue => { + it(`doesn't add ${falsyValue} filter`, function () { + searchSource._mergeProp(state, falsyValue, 'filter'); + expect(state.filters).toEqual([]); + }); + }); + + it('adds "meta.disabled: undefined" filter', function () { + const filter = { + meta: {} + }; + searchSource._mergeProp(state, filter, 'filter'); + expect(state.filters).toEqual([filter]); + }); + + it('adds "meta.disabled: false" filter', function () { + const filter = { + meta: { + disabled: false + } + }; + searchSource._mergeProp(state, filter, 'filter'); + expect(state.filters).toEqual([filter]); + }); + + it(`doesn't add "meta.disabled: true" filter`, function () { + const filter = { + meta: { + disabled: true + } + }; + searchSource._mergeProp(state, filter, 'filter'); + expect(state.filters).toEqual([]); + }); + + describe('when courier:ignoreFilterIfFieldNotInIndex is false', function () { + it('adds filter for non-existent field', function () { + mockConfig = { 'courier:ignoreFilterIfFieldNotInIndex': false }; + const filter = { + meta: { + key: 'bar' + } + }; + state.index = { + fields: [] + }; + searchSource._mergeProp(state, filter, 'filter'); + expect(state.filters).toEqual([ filter ]); + }); + }); + + describe('when courier:ignoreFilterIfFieldNotInIndex is true', function () { + it(`doesn't add filter for non-existent field`, function () { + mockConfig = { 'courier:ignoreFilterIfFieldNotInIndex': true }; + const filter = { + meta: { + key: 'bar' + } + }; + state.index = { + fields: [] + }; + searchSource._mergeProp(state, filter, 'filter'); + expect(state.filters).toEqual([]); + }); + + it(`adds filter for existent field`, function () { + mockConfig = { 'courier:ignoreFilterIfFieldNotInIndex': true }; + const filter = { + meta: { + key: 'bar' + } + }; + state.index = { + fields: [{ name: 'bar' }] + }; + searchSource._mergeProp(state, filter, 'filter'); + expect(state.filters).toEqual([ filter ]); + }); + }); + }); + }); +}); From fa1ad58d71b1c49125b5eab8a99b8bd4919964f3 Mon Sep 17 00:00:00 2001 From: Lukas Olson Date: Wed, 25 Sep 2019 16:57:31 -0700 Subject: [PATCH 12/25] Create search strategy registry test --- .../default_search_strategy.test.js | 4 - .../search_strategy_registry.js | 4 +- .../search_strategy_registry.test.js | 95 ++++++++++++++++++- 3 files changed, 96 insertions(+), 7 deletions(-) diff --git a/src/legacy/ui/public/courier/search_strategy/default_search_strategy.test.js b/src/legacy/ui/public/courier/search_strategy/default_search_strategy.test.js index 27e2c47c9a2d9..dedddd444a933 100644 --- a/src/legacy/ui/public/courier/search_strategy/default_search_strategy.test.js +++ b/src/legacy/ui/public/courier/search_strategy/default_search_strategy.test.js @@ -28,9 +28,7 @@ function getConfigStub(config = {}) { } describe('defaultSearchStrategy', function () { - describe('search', function () { - let searchArgs; beforeEach(() => { @@ -76,7 +74,5 @@ describe('defaultSearchStrategy', function () { await search(searchArgs); expect(searchArgs.es.msearch.mock.calls[0][0]).toHaveProperty('ignore_throttled', false); }); - }); - }); diff --git a/src/legacy/ui/public/courier/search_strategy/search_strategy_registry.js b/src/legacy/ui/public/courier/search_strategy/search_strategy_registry.js index 565a7d1c52927..e67d39ea27aa6 100644 --- a/src/legacy/ui/public/courier/search_strategy/search_strategy_registry.js +++ b/src/legacy/ui/public/courier/search_strategy/search_strategy_registry.js @@ -19,7 +19,7 @@ import { noOpSearchStrategy } from './no_op_search_strategy'; -const searchStrategies = []; +export const searchStrategies = []; export const addSearchStrategy = searchStrategy => { if (searchStrategies.includes(searchStrategy)) { @@ -29,7 +29,7 @@ export const addSearchStrategy = searchStrategy => { searchStrategies.push(searchStrategy); }; -const getSearchStrategyByViability = indexPattern => { +export const getSearchStrategyByViability = indexPattern => { return searchStrategies.find(searchStrategy => { return searchStrategy.isViable(indexPattern); }); diff --git a/src/legacy/ui/public/courier/search_strategy/search_strategy_registry.test.js b/src/legacy/ui/public/courier/search_strategy/search_strategy_registry.test.js index 24b8e23e6a1cc..362d303eb6203 100644 --- a/src/legacy/ui/public/courier/search_strategy/search_strategy_registry.test.js +++ b/src/legacy/ui/public/courier/search_strategy/search_strategy_registry.test.js @@ -17,5 +17,98 @@ * under the License. */ -describe('SearchStrategyRegistry', () => { +import { noOpSearchStrategy } from './no_op_search_strategy'; +import { + searchStrategies, + addSearchStrategy, + getSearchStrategyByViability, + getSearchStrategyById, + getSearchStrategyForSearchRequest, + hasSearchStategyForIndexPattern +} from './search_strategy_registry'; + +const mockSearchStrategies = [{ + id: 0, + isViable: index => index === 0 +}, { + id: 1, + isViable: index => index === 1 +}]; + +describe('Search strategy registry', () => { + beforeEach(() => { + searchStrategies.length = 0; + }); + + describe('addSearchStrategy', () => { + it('adds a search strategy', () => { + addSearchStrategy(mockSearchStrategies[0]); + expect(searchStrategies.length).toBe(1); + }); + + it('does not add a search strategy if it is already included', () => { + addSearchStrategy(mockSearchStrategies[0]); + addSearchStrategy(mockSearchStrategies[0]); + expect(searchStrategies.length).toBe(1); + }); + }); + + describe('getSearchStrategyByViability', () => { + beforeEach(() => { + mockSearchStrategies.forEach(addSearchStrategy); + }); + + it('returns the viable strategy', () => { + expect(getSearchStrategyByViability(0)).toBe(mockSearchStrategies[0]); + expect(getSearchStrategyByViability(1)).toBe(mockSearchStrategies[1]); + }); + + it('returns undefined if there is no viable strategy', () => { + expect(getSearchStrategyByViability(-1)).toBe(undefined); + }); + }); + + describe('getSearchStrategyById', () => { + beforeEach(() => { + mockSearchStrategies.forEach(addSearchStrategy); + }); + + it('returns the strategy by ID', () => { + expect(getSearchStrategyById(0)).toBe(mockSearchStrategies[0]); + expect(getSearchStrategyById(1)).toBe(mockSearchStrategies[1]); + }); + + it('returns undefined if there is no strategy with that ID', () => { + expect(getSearchStrategyById(-1)).toBe(undefined); + }); + }); + + describe('getSearchStrategyForSearchRequest', () => { + beforeEach(() => { + mockSearchStrategies.forEach(addSearchStrategy); + }); + + it('returns the strategy by ID if provided', () => { + expect(getSearchStrategyForSearchRequest({}, { searchStrategyId: 1 })).toBe(mockSearchStrategies[1]); + }); + + it('returns the strategy by viability if there is one', () => { + expect(getSearchStrategyForSearchRequest({ index: 1 })).toBe(mockSearchStrategies[1]); + }); + + it('returns the no op strategy if there is no viable strategy', () => { + expect(getSearchStrategyForSearchRequest({ index: 3 })).toBe(noOpSearchStrategy); + }); + }); + + describe('hasSearchStategyForIndexPattern', () => { + beforeEach(() => { + mockSearchStrategies.forEach(addSearchStrategy); + }); + + it('returns whether there is a search strategy for this index pattern', () => { + expect(hasSearchStategyForIndexPattern(0)).toBe(true); + expect(hasSearchStategyForIndexPattern(-1)).toBe(false); + }); + }); }); From aa8751175c98e80ce5d0912e6849b6bc670fd453 Mon Sep 17 00:00:00 2001 From: Lukas Olson Date: Wed, 25 Sep 2019 16:59:45 -0700 Subject: [PATCH 13/25] Revert empty test --- .../rollup/public/search/rollup_search_strategy.test.js | 7 ------- 1 file changed, 7 deletions(-) delete mode 100644 x-pack/legacy/plugins/rollup/public/search/rollup_search_strategy.test.js diff --git a/x-pack/legacy/plugins/rollup/public/search/rollup_search_strategy.test.js b/x-pack/legacy/plugins/rollup/public/search/rollup_search_strategy.test.js deleted file mode 100644 index 8171e62b95585..0000000000000 --- a/x-pack/legacy/plugins/rollup/public/search/rollup_search_strategy.test.js +++ /dev/null @@ -1,7 +0,0 @@ -/* - * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one - * or more contributor license agreements. Licensed under the Elastic License; - * you may not use this file except in compliance with the Elastic License. - */ - - From 43305ecd3d0df1f42204c74a4821d7eb9c1ffe2b Mon Sep 17 00:00:00 2001 From: Lukas Olson Date: Thu, 26 Sep 2019 12:49:20 -0700 Subject: [PATCH 14/25] Remove filter predicates from search source --- .../src/es_query/__tests__/from_filters.js | 26 ++++ .../kbn-es-query/src/es_query/from_filters.js | 1 + .../courier/search_source/search_source.js | 34 +----- .../search_source/search_source.test.js | 111 ------------------ 4 files changed, 29 insertions(+), 143 deletions(-) diff --git a/packages/kbn-es-query/src/es_query/__tests__/from_filters.js b/packages/kbn-es-query/src/es_query/__tests__/from_filters.js index 59e5f4d6faf8a..676992e4dddc8 100644 --- a/packages/kbn-es-query/src/es_query/__tests__/from_filters.js +++ b/packages/kbn-es-query/src/es_query/__tests__/from_filters.js @@ -55,6 +55,32 @@ describe('build query', function () { expect(result.filter).to.eql(expectedESQueries); }); + it('should remove disabled filters', function () { + const filters = [ + { + match_all: {}, + meta: { type: 'match_all', negate: true, disabled: true }, + }, + ]; + + const expectedESQueries = []; + + const result = buildQueryFromFilters(filters); + + expect(result.must_not).to.eql(expectedESQueries); + }); + + it('should remove falsy filters', function () { + const filters = [null, undefined]; + + const expectedESQueries = []; + + const result = buildQueryFromFilters(filters); + + expect(result.must_not).to.eql(expectedESQueries); + expect(result.must).to.eql(expectedESQueries); + }); + it('should place negated filters in the must_not clause', function () { const filters = [ { diff --git a/packages/kbn-es-query/src/es_query/from_filters.js b/packages/kbn-es-query/src/es_query/from_filters.js index b8193b7469a20..10f9cf82fc972 100644 --- a/packages/kbn-es-query/src/es_query/from_filters.js +++ b/packages/kbn-es-query/src/es_query/from_filters.js @@ -60,6 +60,7 @@ const cleanFilter = function (filter) { }; export function buildQueryFromFilters(filters = [], indexPattern, ignoreFilterIfFieldNotInIndex) { + filters = filters.filter(filter => filter && !_.get(filter, ['meta', 'disabled'])); return { must: [], filter: filters diff --git a/src/legacy/ui/public/courier/search_source/search_source.js b/src/legacy/ui/public/courier/search_source/search_source.js index 1910e02805104..709470f5dff85 100644 --- a/src/legacy/ui/public/courier/search_source/search_source.js +++ b/src/legacy/ui/public/courier/search_source/search_source.js @@ -71,7 +71,7 @@ import _ from 'lodash'; import angular from 'angular'; -import { buildEsQuery, getEsQueryConfig, filterMatchesIndex } from '@kbn/es-query'; +import { buildEsQuery, getEsQueryConfig } from '@kbn/es-query'; import { normalizeSortRequest } from './_normalize_sort_request'; @@ -128,21 +128,6 @@ export class SearchSource { this.history = []; this._requestStartHandlers = []; this._inheritOptions = {}; - - this._filterPredicates = [ - (filter) => { - // remove null/undefined filters - return filter; - }, - (filter) => { - const disabled = _.get(filter, 'meta.disabled'); - return disabled === undefined || disabled === false; - }, - (filter, data) => { - const index = data.index || this.getField('index'); - return !config.get('courier:ignoreFilterIfFieldNotInIndex') || filterMatchesIndex(filter, index); - } - ]; } /***** @@ -357,16 +342,6 @@ export class SearchSource { * PRIVATE APIS ******/ - _getSearchConfig() { - return { - esShardTimeout, - includeFrozen: config.get('search:includeFrozen'), - maxConcurrentShardRequests: config.get('courier:maxConcurrentShardRequests'), - setRequestPreference: config.get('courier:setRequestPreference'), - customRequestPreference: config.get('courier:customRequestPreference'), - }; - } - /** * Used to merge properties into the data within ._flatten(). * The data is passed in and modified by the function @@ -389,12 +364,7 @@ export class SearchSource { switch (key) { case 'filter': - let filters = Array.isArray(val) ? val : [val]; - - filters = filters.filter(filter => { - return this._filterPredicates.every(predicate => predicate(filter, data)); - }); - + const filters = Array.isArray(val) ? val : [val]; data.filters = [...(data.filters || []), ...filters]; return; case 'index': diff --git a/src/legacy/ui/public/courier/search_source/search_source.test.js b/src/legacy/ui/public/courier/search_source/search_source.test.js index 828725c3f2d9f..05f753f27f5af 100644 --- a/src/legacy/ui/public/courier/search_source/search_source.test.js +++ b/src/legacy/ui/public/courier/search_source/search_source.test.js @@ -19,14 +19,9 @@ import { SearchSource } from '../search_source'; -let mockConfig = {}; - jest.mock('ui/new_platform', () => ({ npSetup: { core: { - uiSettings: { - get: key => mockConfig[key] - }, injectedMetadata: { getInjectedVar: () => 0, } @@ -198,110 +193,4 @@ describe('SearchSource', function () { expect(parentFn).toBeCalledWith(searchSource, request, options); }); }); - - describe('#_mergeProp', function () { - describe('filter', function () { - let previousMockConfig; - let searchSource; - let state; - - beforeEach(function () { - previousMockConfig = mockConfig; - searchSource = new SearchSource(); - state = {}; - }); - - afterEach(() => { - mockConfig = previousMockConfig; - }); - - [null, undefined].forEach(falsyValue => { - it(`ignores ${falsyValue} filter`, function () { - searchSource._mergeProp(state, falsyValue, 'filter'); - expect(state.filters).toBe(undefined); - }); - }); - - [false, 0, '', NaN].forEach(falsyValue => { - it(`doesn't add ${falsyValue} filter`, function () { - searchSource._mergeProp(state, falsyValue, 'filter'); - expect(state.filters).toEqual([]); - }); - }); - - it('adds "meta.disabled: undefined" filter', function () { - const filter = { - meta: {} - }; - searchSource._mergeProp(state, filter, 'filter'); - expect(state.filters).toEqual([filter]); - }); - - it('adds "meta.disabled: false" filter', function () { - const filter = { - meta: { - disabled: false - } - }; - searchSource._mergeProp(state, filter, 'filter'); - expect(state.filters).toEqual([filter]); - }); - - it(`doesn't add "meta.disabled: true" filter`, function () { - const filter = { - meta: { - disabled: true - } - }; - searchSource._mergeProp(state, filter, 'filter'); - expect(state.filters).toEqual([]); - }); - - describe('when courier:ignoreFilterIfFieldNotInIndex is false', function () { - it('adds filter for non-existent field', function () { - mockConfig = { 'courier:ignoreFilterIfFieldNotInIndex': false }; - const filter = { - meta: { - key: 'bar' - } - }; - state.index = { - fields: [] - }; - searchSource._mergeProp(state, filter, 'filter'); - expect(state.filters).toEqual([ filter ]); - }); - }); - - describe('when courier:ignoreFilterIfFieldNotInIndex is true', function () { - it(`doesn't add filter for non-existent field`, function () { - mockConfig = { 'courier:ignoreFilterIfFieldNotInIndex': true }; - const filter = { - meta: { - key: 'bar' - } - }; - state.index = { - fields: [] - }; - searchSource._mergeProp(state, filter, 'filter'); - expect(state.filters).toEqual([]); - }); - - it(`adds filter for existent field`, function () { - mockConfig = { 'courier:ignoreFilterIfFieldNotInIndex': true }; - const filter = { - meta: { - key: 'bar' - } - }; - state.index = { - fields: [{ name: 'bar' }] - }; - searchSource._mergeProp(state, filter, 'filter'); - expect(state.filters).toEqual([ filter ]); - }); - }); - }); - }); }); From df0a353893575c3e1d42767a199924348d63885b Mon Sep 17 00:00:00 2001 From: Lukas Olson Date: Wed, 2 Oct 2019 16:33:54 -0700 Subject: [PATCH 15/25] Update typings and throw response errors --- .../__tests__/get_saved_dashboard_mock.ts | 6 +- .../ui/public/courier/search_source/mocks.ts | 58 +++++++++++++++++++ .../courier/search_source/search_source.d.ts | 2 +- .../courier/search_source/search_source.js | 9 ++- .../loader/embedded_visualize_handler.test.ts | 7 ++- .../pipeline_helpers/build_pipeline.test.ts | 7 +-- .../contexts/kibana/__mocks__/saved_search.ts | 4 +- 7 files changed, 78 insertions(+), 15 deletions(-) create mode 100644 src/legacy/ui/public/courier/search_source/mocks.ts diff --git a/src/legacy/core_plugins/kibana/public/dashboard/__tests__/get_saved_dashboard_mock.ts b/src/legacy/core_plugins/kibana/public/dashboard/__tests__/get_saved_dashboard_mock.ts index f9f5cfe0214b2..01468eadffb84 100644 --- a/src/legacy/core_plugins/kibana/public/dashboard/__tests__/get_saved_dashboard_mock.ts +++ b/src/legacy/core_plugins/kibana/public/dashboard/__tests__/get_saved_dashboard_mock.ts @@ -17,6 +17,7 @@ * under the License. */ +import { searchSourceMock } from '../../../../../ui/public/courier/search_source/mocks'; import { SavedObjectDashboard } from '../saved_dashboard/saved_dashboard'; export function getSavedDashboardMock( @@ -26,10 +27,7 @@ export function getSavedDashboardMock( id: '123', title: 'my dashboard', panelsJSON: '[]', - searchSource: { - getOwnField: (param: any) => param, - setField: () => {}, - }, + searchSource: searchSourceMock, copyOnSave: false, timeRestore: false, timeTo: 'now', diff --git a/src/legacy/ui/public/courier/search_source/mocks.ts b/src/legacy/ui/public/courier/search_source/mocks.ts new file mode 100644 index 0000000000000..bf546c1b9e7c2 --- /dev/null +++ b/src/legacy/ui/public/courier/search_source/mocks.ts @@ -0,0 +1,58 @@ +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you under + * the Apache License, Version 2.0 (the "License"), you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +export const searchSourceMock = { + setPreferredSearchStrategyId: jest.fn(), + getPreferredSearchStrategyId: jest.fn(), + setFields: jest.fn(), + setField: jest.fn(), + getId: jest.fn(), + getFields: jest.fn(), + getField: jest.fn(), + getOwnField: jest.fn(), + create: jest.fn(), + createCopy: jest.fn(), + createChild: jest.fn(), + setParent: jest.fn(), + getParent: jest.fn(), + fetch: jest.fn(), + onRequestStart: jest.fn(), + getSearchRequestBody: jest.fn(), + destroy: jest.fn(), + history: [], +}; diff --git a/src/legacy/ui/public/courier/search_source/search_source.d.ts b/src/legacy/ui/public/courier/search_source/search_source.d.ts index 6e7d11f43f052..0ffddc4fe84dd 100644 --- a/src/legacy/ui/public/courier/search_source/search_source.d.ts +++ b/src/legacy/ui/public/courier/search_source/search_source.d.ts @@ -31,7 +31,7 @@ export declare class SearchSource { createChild: (options?: any) => SearchSource; setParent: (parent: SearchSource | boolean) => SearchSource; getParent: () => SearchSource | undefined; - fetch: (options: any) => Promise; + fetch: (options?: any) => Promise; onRequestStart: ( handler: (searchSource: SearchSource, request: any, options: any) => void ) => void; diff --git a/src/legacy/ui/public/courier/search_source/search_source.js b/src/legacy/ui/public/courier/search_source/search_source.js index 709470f5dff85..95812f46127b0 100644 --- a/src/legacy/ui/public/courier/search_source/search_source.js +++ b/src/legacy/ui/public/courier/search_source/search_source.js @@ -80,6 +80,7 @@ import { fieldWildcardFilter } from '../../field_wildcard'; import { getHighlightRequest } from '../../../../../plugins/data/common/field_formats'; import { npSetup } from 'ui/new_platform'; import chrome from '../../chrome'; +import { RequestFailure } from '../fetch/errors'; const FIELDS = [ 'type', @@ -286,10 +287,16 @@ export class SearchSource { const searchRequest = await this._flatten(); await this.requestIsStarting(searchRequest, options); - return fetchSoon(searchRequest, { + const response = await fetchSoon(searchRequest, { ...(this._searchStrategyId && { searchStrategyId: this._searchStrategyId }), ...options, }, { es, config, esShardTimeout }); + + if (response.error) { + throw new RequestFailure(null, response); + } + + return response; } /** diff --git a/src/legacy/ui/public/visualize/loader/embedded_visualize_handler.test.ts b/src/legacy/ui/public/visualize/loader/embedded_visualize_handler.test.ts index 9d6b56c32f1cb..c73f787457a03 100644 --- a/src/legacy/ui/public/visualize/loader/embedded_visualize_handler.test.ts +++ b/src/legacy/ui/public/visualize/loader/embedded_visualize_handler.test.ts @@ -18,6 +18,7 @@ */ jest.mock('ui/new_platform'); +import { searchSourceMock } from '../../courier/search_source/mocks'; import { mockDataLoaderFetch, timefilter } from './embedded_visualize_handler.test.mocks'; import _ from 'lodash'; @@ -85,7 +86,7 @@ describe('EmbeddedVisualizeHandler', () => { inspectorAdapters: {}, query: undefined, queryFilter: null, - searchSource: undefined, + searchSource: searchSourceMock, timeRange: undefined, uiState: undefined, }; @@ -96,7 +97,7 @@ describe('EmbeddedVisualizeHandler', () => { { vis: mockVis, title: 'My Vis', - searchSource: undefined, + searchSource: searchSourceMock, destroy: () => ({}), copyOnSave: false, save: () => Promise.resolve('123'), @@ -128,7 +129,7 @@ describe('EmbeddedVisualizeHandler', () => { { vis: mockVis, title: 'My Vis', - searchSource: undefined, + searchSource: searchSourceMock, destroy: () => ({}), copyOnSave: false, save: () => Promise.resolve('123'), diff --git a/src/legacy/ui/public/visualize/loader/pipeline_helpers/build_pipeline.test.ts b/src/legacy/ui/public/visualize/loader/pipeline_helpers/build_pipeline.test.ts index 1c8621848dbc1..a08aa77fac3b8 100644 --- a/src/legacy/ui/public/visualize/loader/pipeline_helpers/build_pipeline.test.ts +++ b/src/legacy/ui/public/visualize/loader/pipeline_helpers/build_pipeline.test.ts @@ -28,7 +28,7 @@ import { } from './build_pipeline'; import { Vis, VisState } from 'ui/vis'; import { AggConfig } from 'ui/agg_types/agg_config'; -import { SearchSource } from 'ui/courier'; +import { searchSourceMock } from 'ui/courier/search_source/mocks'; jest.mock('ui/new_platform'); jest.mock('ui/agg_types/buckets/date_histogram', () => ({ @@ -347,10 +347,7 @@ describe('visualize loader pipeline helpers: build pipeline', () => { toExpression: () => 'testing custom expressions', }, }; - const searchSource: SearchSource = { - getField: () => null, - }; - const expression = await buildPipeline(vis, { searchSource }); + const expression = await buildPipeline(vis, { searchSource: searchSourceMock }); expect(expression).toMatchSnapshot(); }); }); diff --git a/x-pack/legacy/plugins/ml/public/contexts/kibana/__mocks__/saved_search.ts b/x-pack/legacy/plugins/ml/public/contexts/kibana/__mocks__/saved_search.ts index 311e6688f7aa9..07979d7c1bd11 100644 --- a/x-pack/legacy/plugins/ml/public/contexts/kibana/__mocks__/saved_search.ts +++ b/x-pack/legacy/plugins/ml/public/contexts/kibana/__mocks__/saved_search.ts @@ -4,10 +4,12 @@ * you may not use this file except in compliance with the Elastic License. */ +import { searchSourceMock } from '../../../../../../../../src/legacy/ui/public/courier/search_source/mocks'; + export const savedSearchMock = { id: 'the-saved-search-id', title: 'the-saved-search-title', - searchSource: {}, + searchSource: searchSourceMock, columns: [], sort: [], destroy: () => {}, From 5496403a5285767c6f07cb8a3225b560b121d3f9 Mon Sep 17 00:00:00 2001 From: Lukas Olson Date: Wed, 2 Oct 2019 11:54:28 -0700 Subject: [PATCH 16/25] Fix proxy to properly return response from ES --- src/legacy/core_plugins/elasticsearch/lib/create_proxy.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/legacy/core_plugins/elasticsearch/lib/create_proxy.js b/src/legacy/core_plugins/elasticsearch/lib/create_proxy.js index cc4438ba29b80..266ed7bafdf90 100644 --- a/src/legacy/core_plugins/elasticsearch/lib/create_proxy.js +++ b/src/legacy/core_plugins/elasticsearch/lib/create_proxy.js @@ -19,7 +19,6 @@ import Joi from 'joi'; import { abortableRequestHandler } from './abortable_request_handler'; -import { handleESError } from './handle_es_error'; export function createProxy(server) { const { callWithRequest } = server.plugins.elasticsearch.getCluster('data'); @@ -63,7 +62,7 @@ export function createProxy(server) { body }, { signal }); } catch (error) { - throw handleESError(error); + return JSON.parse(error.response); } }) }); From 41567df6bb6ab47e2299bf163cd5aa69cb59a0d3 Mon Sep 17 00:00:00 2001 From: Lukas Olson Date: Thu, 3 Oct 2019 10:39:19 -0700 Subject: [PATCH 17/25] Update jest snapshots --- .../source_index_preview.test.tsx.snap | 21 ++++++++++++++++++- .../step_create_form.test.tsx.snap | 21 ++++++++++++++++++- .../__snapshots__/pivot_preview.test.tsx.snap | 21 ++++++++++++++++++- .../step_define_form.test.tsx.snap | 21 ++++++++++++++++++- .../step_define_summary.test.tsx.snap | 21 ++++++++++++++++++- 5 files changed, 100 insertions(+), 5 deletions(-) diff --git a/x-pack/legacy/plugins/ml/public/data_frame/pages/data_frame_new_pivot/components/source_index_preview/__snapshots__/source_index_preview.test.tsx.snap b/x-pack/legacy/plugins/ml/public/data_frame/pages/data_frame_new_pivot/components/source_index_preview/__snapshots__/source_index_preview.test.tsx.snap index 9179d9c8bcf61..644746f4d8387 100644 --- a/x-pack/legacy/plugins/ml/public/data_frame/pages/data_frame_new_pivot/components/source_index_preview/__snapshots__/source_index_preview.test.tsx.snap +++ b/x-pack/legacy/plugins/ml/public/data_frame/pages/data_frame_new_pivot/components/source_index_preview/__snapshots__/source_index_preview.test.tsx.snap @@ -18,7 +18,26 @@ exports[`Data Frame: Minimal initialization 1`] = ` "columns": Array [], "destroy": [Function], "id": "the-saved-search-id", - "searchSource": Object {}, + "searchSource": Object { + "create": [MockFunction], + "createChild": [MockFunction], + "createCopy": [MockFunction], + "destroy": [MockFunction], + "fetch": [MockFunction], + "getField": [MockFunction], + "getFields": [MockFunction], + "getId": [MockFunction], + "getOwnField": [MockFunction], + "getParent": [MockFunction], + "getPreferredSearchStrategyId": [MockFunction], + "getSearchRequestBody": [MockFunction], + "history": Array [], + "onRequestStart": [MockFunction], + "setField": [MockFunction], + "setFields": [MockFunction], + "setParent": [MockFunction], + "setPreferredSearchStrategyId": [MockFunction], + }, "sort": Array [], "title": "the-saved-search-title", }, diff --git a/x-pack/legacy/plugins/ml/public/data_frame/pages/data_frame_new_pivot/components/step_create/__snapshots__/step_create_form.test.tsx.snap b/x-pack/legacy/plugins/ml/public/data_frame/pages/data_frame_new_pivot/components/step_create/__snapshots__/step_create_form.test.tsx.snap index 26844efb711e5..54ca1cb949b1b 100644 --- a/x-pack/legacy/plugins/ml/public/data_frame/pages/data_frame_new_pivot/components/step_create/__snapshots__/step_create_form.test.tsx.snap +++ b/x-pack/legacy/plugins/ml/public/data_frame/pages/data_frame_new_pivot/components/step_create/__snapshots__/step_create_form.test.tsx.snap @@ -18,7 +18,26 @@ exports[`Data Frame: Minimal initialization 1`] = ` "columns": Array [], "destroy": [Function], "id": "the-saved-search-id", - "searchSource": Object {}, + "searchSource": Object { + "create": [MockFunction], + "createChild": [MockFunction], + "createCopy": [MockFunction], + "destroy": [MockFunction], + "fetch": [MockFunction], + "getField": [MockFunction], + "getFields": [MockFunction], + "getId": [MockFunction], + "getOwnField": [MockFunction], + "getParent": [MockFunction], + "getPreferredSearchStrategyId": [MockFunction], + "getSearchRequestBody": [MockFunction], + "history": Array [], + "onRequestStart": [MockFunction], + "setField": [MockFunction], + "setFields": [MockFunction], + "setParent": [MockFunction], + "setPreferredSearchStrategyId": [MockFunction], + }, "sort": Array [], "title": "the-saved-search-title", }, diff --git a/x-pack/legacy/plugins/ml/public/data_frame/pages/data_frame_new_pivot/components/step_define/__snapshots__/pivot_preview.test.tsx.snap b/x-pack/legacy/plugins/ml/public/data_frame/pages/data_frame_new_pivot/components/step_define/__snapshots__/pivot_preview.test.tsx.snap index 192d9d2cff625..dd3a10689272f 100644 --- a/x-pack/legacy/plugins/ml/public/data_frame/pages/data_frame_new_pivot/components/step_define/__snapshots__/pivot_preview.test.tsx.snap +++ b/x-pack/legacy/plugins/ml/public/data_frame/pages/data_frame_new_pivot/components/step_define/__snapshots__/pivot_preview.test.tsx.snap @@ -18,7 +18,26 @@ exports[`Data Frame: Minimal initialization 1`] = ` "columns": Array [], "destroy": [Function], "id": "the-saved-search-id", - "searchSource": Object {}, + "searchSource": Object { + "create": [MockFunction], + "createChild": [MockFunction], + "createCopy": [MockFunction], + "destroy": [MockFunction], + "fetch": [MockFunction], + "getField": [MockFunction], + "getFields": [MockFunction], + "getId": [MockFunction], + "getOwnField": [MockFunction], + "getParent": [MockFunction], + "getPreferredSearchStrategyId": [MockFunction], + "getSearchRequestBody": [MockFunction], + "history": Array [], + "onRequestStart": [MockFunction], + "setField": [MockFunction], + "setFields": [MockFunction], + "setParent": [MockFunction], + "setPreferredSearchStrategyId": [MockFunction], + }, "sort": Array [], "title": "the-saved-search-title", }, diff --git a/x-pack/legacy/plugins/ml/public/data_frame/pages/data_frame_new_pivot/components/step_define/__snapshots__/step_define_form.test.tsx.snap b/x-pack/legacy/plugins/ml/public/data_frame/pages/data_frame_new_pivot/components/step_define/__snapshots__/step_define_form.test.tsx.snap index 033eea8cf290e..a45184587437e 100644 --- a/x-pack/legacy/plugins/ml/public/data_frame/pages/data_frame_new_pivot/components/step_define/__snapshots__/step_define_form.test.tsx.snap +++ b/x-pack/legacy/plugins/ml/public/data_frame/pages/data_frame_new_pivot/components/step_define/__snapshots__/step_define_form.test.tsx.snap @@ -18,7 +18,26 @@ exports[`Data Frame: Minimal initialization 1`] = ` "columns": Array [], "destroy": [Function], "id": "the-saved-search-id", - "searchSource": Object {}, + "searchSource": Object { + "create": [MockFunction], + "createChild": [MockFunction], + "createCopy": [MockFunction], + "destroy": [MockFunction], + "fetch": [MockFunction], + "getField": [MockFunction], + "getFields": [MockFunction], + "getId": [MockFunction], + "getOwnField": [MockFunction], + "getParent": [MockFunction], + "getPreferredSearchStrategyId": [MockFunction], + "getSearchRequestBody": [MockFunction], + "history": Array [], + "onRequestStart": [MockFunction], + "setField": [MockFunction], + "setFields": [MockFunction], + "setParent": [MockFunction], + "setPreferredSearchStrategyId": [MockFunction], + }, "sort": Array [], "title": "the-saved-search-title", }, diff --git a/x-pack/legacy/plugins/ml/public/data_frame/pages/data_frame_new_pivot/components/step_define/__snapshots__/step_define_summary.test.tsx.snap b/x-pack/legacy/plugins/ml/public/data_frame/pages/data_frame_new_pivot/components/step_define/__snapshots__/step_define_summary.test.tsx.snap index c3b75584f0a51..0c34c1f979000 100644 --- a/x-pack/legacy/plugins/ml/public/data_frame/pages/data_frame_new_pivot/components/step_define/__snapshots__/step_define_summary.test.tsx.snap +++ b/x-pack/legacy/plugins/ml/public/data_frame/pages/data_frame_new_pivot/components/step_define/__snapshots__/step_define_summary.test.tsx.snap @@ -18,7 +18,26 @@ exports[`Data Frame: Minimal initialization 1`] = ` "columns": Array [], "destroy": [Function], "id": "the-saved-search-id", - "searchSource": Object {}, + "searchSource": Object { + "create": [MockFunction], + "createChild": [MockFunction], + "createCopy": [MockFunction], + "destroy": [MockFunction], + "fetch": [MockFunction], + "getField": [MockFunction], + "getFields": [MockFunction], + "getId": [MockFunction], + "getOwnField": [MockFunction], + "getParent": [MockFunction], + "getPreferredSearchStrategyId": [MockFunction], + "getSearchRequestBody": [MockFunction], + "history": Array [], + "onRequestStart": [MockFunction], + "setField": [MockFunction], + "setFields": [MockFunction], + "setParent": [MockFunction], + "setPreferredSearchStrategyId": [MockFunction], + }, "sort": Array [], "title": "the-saved-search-title", }, From c1e1ddbde15a41a61f5233b15d42c43c775b1a8a Mon Sep 17 00:00:00 2001 From: Lukas Olson Date: Thu, 3 Oct 2019 10:41:35 -0700 Subject: [PATCH 18/25] Remove unused translations --- x-pack/plugins/translations/translations/ja-JP.json | 7 +------ x-pack/plugins/translations/translations/zh-CN.json | 7 +------ 2 files changed, 2 insertions(+), 12 deletions(-) diff --git a/x-pack/plugins/translations/translations/ja-JP.json b/x-pack/plugins/translations/translations/ja-JP.json index 68d537b7e3abd..a72b48063bb92 100644 --- a/x-pack/plugins/translations/translations/ja-JP.json +++ b/x-pack/plugins/translations/translations/ja-JP.json @@ -263,13 +263,8 @@ "common.ui.aggTypes.timeInterval.scaledHelpText": "現在 {bucketDescription} にスケーリングされています", "common.ui.aggTypes.timeInterval.selectIntervalPlaceholder": "間隔を選択", "common.ui.aggTypes.timeInterval.selectOptionHelpText": "オプションを選択するかカスタム値を作成します。例30s、20m、24h、2d、1w、1M", - "common.ui.courier.fetch.failedToClearRequestErrorMessage": "返答から未完全または重複のリクエストを消去できませんでした。", "common.ui.courier.fetch.requestTimedOutNotificationMessage": "リクエストがタイムアウトしたため、データが不完全な可能性があります", - "common.ui.courier.fetch.requestWasAbortedTwiceErrorMessage": "リクエストが 2 度中断されましたか？", - "common.ui.courier.fetch.requireErrorHandlerErrorMessage": "{errorHandler} が必要です", "common.ui.courier.fetch.shardsFailedNotificationMessage": "{shardsTotal} 件中 {shardsFailed} 件のシャードでエラーが発生しました", - "common.ui.courier.fetch.unableContinueRequestErrorMessage": "{type} リクエストを続行できません", - "common.ui.courier.fetch.unableStartRequestErrorMessage": "既に開始済みのためリクエストは開始できません", "common.ui.courier.hitsDescription": "クエリにより返されたドキュメントの数です。", "common.ui.courier.hitsLabel": "ヒット数", "common.ui.courier.hitsTotalDescription": "クエリに一致するドキュメントの数です。", @@ -11381,4 +11376,4 @@ "xpack.fileUpload.fileParser.noFileProvided": "エラー、ファイルが提供されていません", "xpack.fileUpload.jsonIndexFilePicker.errorGettingIndexName": "インデックス名の取得中にエラーが発生: {errorMessage}" } -} \ No newline at end of file +} diff --git a/x-pack/plugins/translations/translations/zh-CN.json b/x-pack/plugins/translations/translations/zh-CN.json index ac738716b1491..95a4c8c74d0f9 100644 --- a/x-pack/plugins/translations/translations/zh-CN.json +++ b/x-pack/plugins/translations/translations/zh-CN.json @@ -263,13 +263,8 @@ "common.ui.aggTypes.timeInterval.scaledHelpText": "当前缩放至 {bucketDescription}", "common.ui.aggTypes.timeInterval.selectIntervalPlaceholder": "选择时间间隔", "common.ui.aggTypes.timeInterval.selectOptionHelpText": "选择选项或创建定制值示例:30s、20m、24h、2d、1w、1M", - "common.ui.courier.fetch.failedToClearRequestErrorMessage": "无法从响应中清除不完整或重复的请求。", "common.ui.courier.fetch.requestTimedOutNotificationMessage": "由于您的请求超时，因此数据可能不完整", - "common.ui.courier.fetch.requestWasAbortedTwiceErrorMessage": "请求已中止两次？", - "common.ui.courier.fetch.requireErrorHandlerErrorMessage": "“{errorHandler}” 必填", "common.ui.courier.fetch.shardsFailedNotificationMessage": "{shardsTotal} 个分片有 {shardsFailed} 个失败", - "common.ui.courier.fetch.unableContinueRequestErrorMessage": "无法继续 {type} 请求", - "common.ui.courier.fetch.unableStartRequestErrorMessage": "无法启动请求，因此其已启动", "common.ui.courier.hitsDescription": "查询返回的文档数目。", "common.ui.courier.hitsLabel": "命中", "common.ui.courier.hitsTotalDescription": "匹配查询的文档数目。", @@ -11383,4 +11378,4 @@ "xpack.fileUpload.fileParser.noFileProvided": "错误，未提供任何文件", "xpack.fileUpload.jsonIndexFilePicker.errorGettingIndexName": "检索索引名称时出错：{errorMessage}" } -} \ No newline at end of file +} From b3e8d118b3d282aaf4c33bdf265e801abe15b9d5 Mon Sep 17 00:00:00 2001 From: Lukas Olson Date: Thu, 3 Oct 2019 13:13:32 -0700 Subject: [PATCH 19/25] Don't pass search request to onRequestStart, create it afterwards --- .../interpreter/public/functions/esaggs.ts | 8 +++----- .../kibana/public/discover/controllers/discover.js | 2 +- .../agg_types/__tests__/metrics/parent_pipeline.js | 5 ++--- .../agg_types/__tests__/metrics/sibling_pipeline.js | 7 +++---- src/legacy/ui/public/agg_types/agg_config.ts | 4 ++-- src/legacy/ui/public/agg_types/agg_configs.ts | 6 ++---- src/legacy/ui/public/agg_types/buckets/histogram.js | 2 +- src/legacy/ui/public/agg_types/param_types/base.ts | 4 +--- .../ui/public/courier/search_source/search_source.js | 11 +++++------ .../loader/pipeline_helpers/build_pipeline.ts | 2 +- 10 files changed, 21 insertions(+), 30 deletions(-) diff --git a/src/legacy/core_plugins/interpreter/public/functions/esaggs.ts b/src/legacy/core_plugins/interpreter/public/functions/esaggs.ts index 18277ff27b17e..5d5c3e07a8ee9 100644 --- a/src/legacy/core_plugins/interpreter/public/functions/esaggs.ts +++ b/src/legacy/core_plugins/interpreter/public/functions/esaggs.ts @@ -99,11 +99,9 @@ const handleCourierRequest = async ({ return aggs.toDsl(metricsAtAllLevels); }); - requestSearchSource.onRequestStart( - (paramSearchSource: SearchSource, searchRequest: unknown, options: any) => { - return aggs.onSearchRequestStart(paramSearchSource, searchRequest, options); - } - ); + requestSearchSource.onRequestStart((paramSearchSource: SearchSource, options: any) => { + return aggs.onSearchRequestStart(paramSearchSource, options); + }); if (timeRange) { timeFilterSearchSource.setField('filter', () => { diff --git a/src/legacy/core_plugins/kibana/public/discover/controllers/discover.js b/src/legacy/core_plugins/kibana/public/discover/controllers/discover.js index 537263ea25a51..09b44a192f4e0 100644 --- a/src/legacy/core_plugins/kibana/public/discover/controllers/discover.js +++ b/src/legacy/core_plugins/kibana/public/discover/controllers/discover.js @@ -1037,7 +1037,7 @@ function discoverController( ); visSavedObject.vis = $scope.vis; - $scope.searchSource.onRequestStart((searchSource, searchRequest, options) => { + $scope.searchSource.onRequestStart((searchSource, options) => { return $scope.vis.getAggConfig().onSearchRequestStart(searchSource, options); }); diff --git a/src/legacy/ui/public/agg_types/__tests__/metrics/parent_pipeline.js b/src/legacy/ui/public/agg_types/__tests__/metrics/parent_pipeline.js index 3c8fde7eb7135..e4ca6075c624b 100644 --- a/src/legacy/ui/public/agg_types/__tests__/metrics/parent_pipeline.js +++ b/src/legacy/ui/public/agg_types/__tests__/metrics/parent_pipeline.js @@ -203,7 +203,6 @@ describe('parent pipeline aggs', function () { }); const searchSource = {}; - const request = {}; const customMetricSpy = sinon.spy(); const customMetric = aggConfig.params.customMetric; @@ -211,9 +210,9 @@ describe('parent pipeline aggs', function () { customMetric.type.params[0].modifyAggConfigOnSearchRequestStart = customMetricSpy; aggConfig.type.params.forEach(param => { - param.modifyAggConfigOnSearchRequestStart(aggConfig, searchSource, request); + param.modifyAggConfigOnSearchRequestStart(aggConfig, searchSource); }); - expect(customMetricSpy.calledWith(customMetric, searchSource, request)).to.be(true); + expect(customMetricSpy.calledWith(customMetric, searchSource)).to.be(true); }); }); }); diff --git a/src/legacy/ui/public/agg_types/__tests__/metrics/sibling_pipeline.js b/src/legacy/ui/public/agg_types/__tests__/metrics/sibling_pipeline.js index fef69155d2351..aba5db9cedadf 100644 --- a/src/legacy/ui/public/agg_types/__tests__/metrics/sibling_pipeline.js +++ b/src/legacy/ui/public/agg_types/__tests__/metrics/sibling_pipeline.js @@ -145,7 +145,6 @@ describe('sibling pipeline aggs', function () { init(); const searchSource = {}; - const request = {}; const customMetricSpy = sinon.spy(); const customBucketSpy = sinon.spy(); const { customMetric, customBucket } = aggConfig.params; @@ -155,10 +154,10 @@ describe('sibling pipeline aggs', function () { customBucket.type.params[0].modifyAggConfigOnSearchRequestStart = customBucketSpy; aggConfig.type.params.forEach(param => { - param.modifyAggConfigOnSearchRequestStart(aggConfig, searchSource, request); + param.modifyAggConfigOnSearchRequestStart(aggConfig, searchSource); }); - expect(customMetricSpy.calledWith(customMetric, searchSource, request)).to.be(true); - expect(customBucketSpy.calledWith(customBucket, searchSource, request)).to.be(true); + expect(customMetricSpy.calledWith(customMetric, searchSource)).to.be(true); + expect(customBucketSpy.calledWith(customBucket, searchSource)).to.be(true); }); }); diff --git a/src/legacy/ui/public/agg_types/agg_config.ts b/src/legacy/ui/public/agg_types/agg_config.ts index 5b8dc6f65a900..50c96ab083242 100644 --- a/src/legacy/ui/public/agg_types/agg_config.ts +++ b/src/legacy/ui/public/agg_types/agg_config.ts @@ -226,14 +226,14 @@ export class AggConfig { * @param {Courier.SearchRequest} searchRequest * @return {Promise} */ - onSearchRequestStart(searchSource: any, searchRequest: any, options: any) { + onSearchRequestStart(searchSource: any, options: any) { if (!this.type) { return Promise.resolve(); } return Promise.all( this.type.params.map((param: any) => - param.modifyAggConfigOnSearchRequestStart(this, searchSource, searchRequest, options) + param.modifyAggConfigOnSearchRequestStart(this, searchSource, options) ) ); } diff --git a/src/legacy/ui/public/agg_types/agg_configs.ts b/src/legacy/ui/public/agg_types/agg_configs.ts index 132dfd7dbe515..675d37d05c33c 100644 --- a/src/legacy/ui/public/agg_types/agg_configs.ts +++ b/src/legacy/ui/public/agg_types/agg_configs.ts @@ -307,12 +307,10 @@ export class AggConfigs { return _.find(reqAgg.getResponseAggs(), { id }); } - onSearchRequestStart(searchSource: any, searchRequest: any, options: any) { + onSearchRequestStart(searchSource: any, options: any) { return Promise.all( // @ts-ignore - this.getRequestAggs().map((agg: AggConfig) => - agg.onSearchRequestStart(searchSource, searchRequest, options) - ) + this.getRequestAggs().map((agg: AggConfig) => agg.onSearchRequestStart(searchSource, options)) ); } } diff --git a/src/legacy/ui/public/agg_types/buckets/histogram.js b/src/legacy/ui/public/agg_types/buckets/histogram.js index d1877413a3f9a..91ab399b05487 100644 --- a/src/legacy/ui/public/agg_types/buckets/histogram.js +++ b/src/legacy/ui/public/agg_types/buckets/histogram.js @@ -76,7 +76,7 @@ export const histogramBucketAgg = new BucketAggType({ { name: 'interval', editorComponent: NumberIntervalParamEditor, - modifyAggConfigOnSearchRequestStart(aggConfig, searchSource, searchRequest, options) { + modifyAggConfigOnSearchRequestStart(aggConfig, searchSource, options) { const field = aggConfig.getField(); const aggBody = field.scripted ? { script: { source: field.script, lang: field.lang } } diff --git a/src/legacy/ui/public/agg_types/param_types/base.ts b/src/legacy/ui/public/agg_types/param_types/base.ts index 21387ac0761cb..2a9ec5d7d4b35 100644 --- a/src/legacy/ui/public/agg_types/param_types/base.ts +++ b/src/legacy/ui/public/agg_types/param_types/base.ts @@ -46,18 +46,16 @@ export class BaseParamType implements AggParam { /** * A function that will be called before an aggConfig is serialized and sent to ES. - * Allows aggConfig to retrieve values needed for serialization by creating a {SearchRequest} + * Allows aggConfig to retrieve values needed for serialization * Example usage: an aggregation needs to know the min/max of a field to determine an appropriate interval * * @param {AggConfig} aggConfig * @param {Courier.SearchSource} searchSource - * @param {Courier.SearchRequest} searchRequest * @returns {Promise|undefined} */ modifyAggConfigOnSearchRequestStart: ( aggConfig: AggConfig, searchSource: SearchSource, - searchRequest: any, options: any ) => void; diff --git a/src/legacy/ui/public/courier/search_source/search_source.js b/src/legacy/ui/public/courier/search_source/search_source.js index 95812f46127b0..ccc0ba37fa36c 100644 --- a/src/legacy/ui/public/courier/search_source/search_source.js +++ b/src/legacy/ui/public/courier/search_source/search_source.js @@ -284,8 +284,10 @@ export class SearchSource { const $injector = await chrome.dangerouslyGetActiveInjector(); const es = $injector.get('es'); + await this.requestIsStarting(options); + const searchRequest = await this._flatten(); - await this.requestIsStarting(searchRequest, options); + this.history = [searchRequest]; const response = await fetchSoon(searchRequest, { ...(this._searchStrategyId && { searchStrategyId: this._searchStrategyId }), @@ -314,10 +316,7 @@ export class SearchSource { * @param options * @return {Promise} */ - requestIsStarting(request, options) { - this.activeFetchCount = (this.activeFetchCount || 0) + 1; - this.history = [request]; - + requestIsStarting(options) { const handlers = [...this._requestStartHandlers]; // If callparentStartHandlers has been set to true, we also call all // handlers of parent search sources. @@ -329,7 +328,7 @@ export class SearchSource { } } - return Promise.all(handlers.map(fn => fn(this, request, options))); + return Promise.all(handlers.map(fn => fn(this, options))); } async getSearchRequestBody() { diff --git a/src/legacy/ui/public/visualize/loader/pipeline_helpers/build_pipeline.ts b/src/legacy/ui/public/visualize/loader/pipeline_helpers/build_pipeline.ts index c4444ec1b56df..154fce46897eb 100644 --- a/src/legacy/ui/public/visualize/loader/pipeline_helpers/build_pipeline.ts +++ b/src/legacy/ui/public/visualize/loader/pipeline_helpers/build_pipeline.ts @@ -441,7 +441,7 @@ export const buildVislibDimensions = async ( } else if (xAgg.type.name === 'histogram') { const intervalParam = xAgg.type.paramByName('interval'); const output = { params: {} as any }; - await intervalParam.modifyAggConfigOnSearchRequestStart(xAgg, params.searchSource, null, { + await intervalParam.modifyAggConfigOnSearchRequestStart(xAgg, params.searchSource, { abortSignal: params.abortSignal, }); intervalParam.write(xAgg, output); From 649e6faf94a39a5faa27d94ad1f891ba77080f0e Mon Sep 17 00:00:00 2001 From: Lukas Olson Date: Thu, 3 Oct 2019 14:06:27 -0700 Subject: [PATCH 20/25] Fix search source & get search params tests --- .../courier/fetch/get_search_params.test.js | 4 ++-- .../courier/search_source/search_source.d.ts | 4 +--- .../search_source/search_source.test.js | 23 ++++++++----------- 3 files changed, 13 insertions(+), 18 deletions(-) diff --git a/src/legacy/ui/public/courier/fetch/get_search_params.test.js b/src/legacy/ui/public/courier/fetch/get_search_params.test.js index 9129aea05f428..380d1da963ddf 100644 --- a/src/legacy/ui/public/courier/fetch/get_search_params.test.js +++ b/src/legacy/ui/public/courier/fetch/get_search_params.test.js @@ -99,10 +99,10 @@ describe('getSearchParams', () => { test('includes timeout according to esShardTimeout if greater than 0', () => { const config = getConfigStub(); - let searchParams = getSearchParams(config, null, 0); + let searchParams = getSearchParams(config, 0); expect(searchParams.timeout).toBe(undefined); - searchParams = getSearchParams(config, null, 100); + searchParams = getSearchParams(config, 100); expect(searchParams.timeout).toBe('100ms'); }); }); diff --git a/src/legacy/ui/public/courier/search_source/search_source.d.ts b/src/legacy/ui/public/courier/search_source/search_source.d.ts index 0ffddc4fe84dd..674e7ace0594c 100644 --- a/src/legacy/ui/public/courier/search_source/search_source.d.ts +++ b/src/legacy/ui/public/courier/search_source/search_source.d.ts @@ -32,9 +32,7 @@ export declare class SearchSource { setParent: (parent: SearchSource | boolean) => SearchSource; getParent: () => SearchSource | undefined; fetch: (options?: any) => Promise; - onRequestStart: ( - handler: (searchSource: SearchSource, request: any, options: any) => void - ) => void; + onRequestStart: (handler: (searchSource: SearchSource, options: any) => void) => void; getSearchRequestBody: () => any; destroy: () => void; history: any[]; diff --git a/src/legacy/ui/public/courier/search_source/search_source.test.js b/src/legacy/ui/public/courier/search_source/search_source.test.js index 05f753f27f5af..800f4e4308671 100644 --- a/src/legacy/ui/public/courier/search_source/search_source.test.js +++ b/src/legacy/ui/public/courier/search_source/search_source.test.js @@ -151,17 +151,16 @@ describe('SearchSource', function () { }); describe('#onRequestStart()', () => { - it('should be called when starting a request', async () => { + it('should be called when starting a request', () => { const searchSource = new SearchSource(); const fn = jest.fn(); searchSource.onRequestStart(fn); - const request = {}; const options = {}; - searchSource.requestIsStarting(request, options); - expect(fn).toBeCalledWith(searchSource, request, options); + searchSource.requestIsStarting(options); + expect(fn).toBeCalledWith(searchSource, options); }); - it('should not be called on parent searchSource', async () => { + it('should not be called on parent searchSource', () => { const parent = new SearchSource(); const searchSource = new SearchSource().setParent(parent); @@ -169,15 +168,14 @@ describe('SearchSource', function () { searchSource.onRequestStart(fn); const parentFn = jest.fn(); parent.onRequestStart(parentFn); - const request = {}; const options = {}; - searchSource.requestIsStarting(request, options); + searchSource.requestIsStarting(options); - expect(fn).toBeCalledWith(searchSource, request, options); + expect(fn).toBeCalledWith(searchSource, options); expect(parentFn).not.toBeCalled(); }); - it('should be called on parent searchSource if callParentStartHandlers is true', async () => { + it('should be called on parent searchSource if callParentStartHandlers is true', () => { const parent = new SearchSource(); const searchSource = new SearchSource().setParent(parent, { callParentStartHandlers: true }); @@ -185,12 +183,11 @@ describe('SearchSource', function () { searchSource.onRequestStart(fn); const parentFn = jest.fn(); parent.onRequestStart(parentFn); - const request = {}; const options = {}; - searchSource.requestIsStarting(request, options); + searchSource.requestIsStarting(options); - expect(fn).toBeCalledWith(searchSource, request, options); - expect(parentFn).toBeCalledWith(searchSource, request, options); + expect(fn).toBeCalledWith(searchSource, options); + expect(parentFn).toBeCalledWith(searchSource, options); }); }); }); From 0d7c97d4066427310eeade1015556df0eada2d6e Mon Sep 17 00:00:00 2001 From: Lukas Olson Date: Tue, 8 Oct 2019 09:56:16 -0700 Subject: [PATCH 21/25] Fix issue with angular scope not firing after setting state on vis --- .../interpreter/public/renderers/visualization.ts | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/legacy/core_plugins/interpreter/public/renderers/visualization.ts b/src/legacy/core_plugins/interpreter/public/renderers/visualization.ts index 960e925b13221..a4cd95cd3bd57 100644 --- a/src/legacy/core_plugins/interpreter/public/renderers/visualization.ts +++ b/src/legacy/core_plugins/interpreter/public/renderers/visualization.ts @@ -30,13 +30,16 @@ export const visualization = () => ({ const { visData, visConfig, params } = config; const visType = config.visType || visConfig.type; const $injector = await chrome.dangerouslyGetActiveInjector(); + const $timeout = $injector.get('$timeout') as any; const Private = $injector.get('Private') as any; const Vis = Private(VisProvider); if (handlers.vis) { // special case in visualize, we need to render first (without executing the expression), for maps to work - if (visConfig) { - handlers.vis.setCurrentState({ type: visType, params: visConfig }); + if (visConfig && ['tile_map', 'region_map'].includes(visConfig.type)) { + $timeout(() => { + handlers.vis.setCurrentState({ type: visType, params: visConfig }); + }); } } else { handlers.vis = new Vis({ From 60122b47d95f4ffd121864168698e42cf07f2b21 Mon Sep 17 00:00:00 2001 From: Lukas Olson Date: Mon, 14 Oct 2019 12:28:26 -0700 Subject: [PATCH 22/25] Fix tag cloud vis --- .../core_plugins/interpreter/public/renderers/visualization.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/legacy/core_plugins/interpreter/public/renderers/visualization.ts b/src/legacy/core_plugins/interpreter/public/renderers/visualization.ts index a4cd95cd3bd57..8dcd2992cd7ca 100644 --- a/src/legacy/core_plugins/interpreter/public/renderers/visualization.ts +++ b/src/legacy/core_plugins/interpreter/public/renderers/visualization.ts @@ -49,12 +49,13 @@ export const visualization = () => ({ handlers.vis.eventsSubject = handlers.eventsSubject; } + const visParams = { ...handlers.vis.params, ...visConfig }; const uiState = handlers.uiState || handlers.vis.getUiState(); handlers.onDestroy(() => visualizationLoader.destroy()); await visualizationLoader - .render(domNode, handlers.vis, visData, handlers.vis.params, uiState, params) + .render(domNode, handlers.vis, visData, visParams, uiState, params) .then(() => { if (handlers.done) handlers.done(); }); From eca50a9f7328d780460cc79f9826301e410239b6 Mon Sep 17 00:00:00 2001 From: Lukas Olson Date: Mon, 14 Oct 2019 13:14:47 -0700 Subject: [PATCH 23/25] Fix setting of visConfig to not happen async --- .../interpreter/public/renderers/visualization.ts | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/legacy/core_plugins/interpreter/public/renderers/visualization.ts b/src/legacy/core_plugins/interpreter/public/renderers/visualization.ts index 8dcd2992cd7ca..90020f819dbe2 100644 --- a/src/legacy/core_plugins/interpreter/public/renderers/visualization.ts +++ b/src/legacy/core_plugins/interpreter/public/renderers/visualization.ts @@ -30,14 +30,14 @@ export const visualization = () => ({ const { visData, visConfig, params } = config; const visType = config.visType || visConfig.type; const $injector = await chrome.dangerouslyGetActiveInjector(); - const $timeout = $injector.get('$timeout') as any; + const $rootScope = $injector.get('$rootScope') as any; const Private = $injector.get('Private') as any; const Vis = Private(VisProvider); if (handlers.vis) { // special case in visualize, we need to render first (without executing the expression), for maps to work - if (visConfig && ['tile_map', 'region_map'].includes(visConfig.type)) { - $timeout(() => { + if (visConfig) { + $rootScope.$apply(() => { handlers.vis.setCurrentState({ type: visType, params: visConfig }); }); } @@ -49,13 +49,12 @@ export const visualization = () => ({ handlers.vis.eventsSubject = handlers.eventsSubject; } - const visParams = { ...handlers.vis.params, ...visConfig }; const uiState = handlers.uiState || handlers.vis.getUiState(); handlers.onDestroy(() => visualizationLoader.destroy()); await visualizationLoader - .render(domNode, handlers.vis, visData, visParams, uiState, params) + .render(domNode, handlers.vis, visData, handlers.vis.params, uiState, params) .then(() => { if (handlers.done) handlers.done(); }); From 04a423c69a5ef605c9e3402adc850566b39e7d88 Mon Sep 17 00:00:00 2001 From: Lukas Olson Date: Mon, 14 Oct 2019 13:20:11 -0700 Subject: [PATCH 24/25] Remove unused snapshots --- .../source_index_preview.test.tsx.snap | 78 --------------- .../step_create_form.test.tsx.snap | 81 --------------- .../__snapshots__/pivot_preview.test.tsx.snap | 98 ------------------- .../step_define_form.test.tsx.snap | 71 -------------- .../step_define_summary.test.tsx.snap | 96 ------------------ 5 files changed, 424 deletions(-) delete mode 100644 x-pack/legacy/plugins/ml/public/data_frame/pages/data_frame_new_pivot/components/source_index_preview/__snapshots__/source_index_preview.test.tsx.snap delete mode 100644 x-pack/legacy/plugins/ml/public/data_frame/pages/data_frame_new_pivot/components/step_create/__snapshots__/step_create_form.test.tsx.snap delete mode 100644 x-pack/legacy/plugins/ml/public/data_frame/pages/data_frame_new_pivot/components/step_define/__snapshots__/pivot_preview.test.tsx.snap delete mode 100644 x-pack/legacy/plugins/ml/public/data_frame/pages/data_frame_new_pivot/components/step_define/__snapshots__/step_define_form.test.tsx.snap delete mode 100644 x-pack/legacy/plugins/ml/public/data_frame/pages/data_frame_new_pivot/components/step_define/__snapshots__/step_define_summary.test.tsx.snap diff --git a/x-pack/legacy/plugins/ml/public/data_frame/pages/data_frame_new_pivot/components/source_index_preview/__snapshots__/source_index_preview.test.tsx.snap b/x-pack/legacy/plugins/ml/public/data_frame/pages/data_frame_new_pivot/components/source_index_preview/__snapshots__/source_index_preview.test.tsx.snap deleted file mode 100644 index 644746f4d8387..0000000000000 --- a/x-pack/legacy/plugins/ml/public/data_frame/pages/data_frame_new_pivot/components/source_index_preview/__snapshots__/source_index_preview.test.tsx.snap +++ /dev/null @@ -1,78 +0,0 @@ -// Jest Snapshot v1, https://goo.gl/fbAQLP - -exports[`Data Frame: Minimal initialization 1`] = ` -
- - - -
-`; diff --git a/x-pack/legacy/plugins/ml/public/data_frame/pages/data_frame_new_pivot/components/step_create/__snapshots__/step_create_form.test.tsx.snap b/x-pack/legacy/plugins/ml/public/data_frame/pages/data_frame_new_pivot/components/step_create/__snapshots__/step_create_form.test.tsx.snap deleted file mode 100644 index 54ca1cb949b1b..0000000000000 --- a/x-pack/legacy/plugins/ml/public/data_frame/pages/data_frame_new_pivot/components/step_create/__snapshots__/step_create_form.test.tsx.snap +++ /dev/null @@ -1,81 +0,0 @@ -// Jest Snapshot v1, https://goo.gl/fbAQLP - -exports[`Data Frame: Minimal initialization 1`] = ` -
- - - -
-`; diff --git a/x-pack/legacy/plugins/ml/public/data_frame/pages/data_frame_new_pivot/components/step_define/__snapshots__/pivot_preview.test.tsx.snap b/x-pack/legacy/plugins/ml/public/data_frame/pages/data_frame_new_pivot/components/step_define/__snapshots__/pivot_preview.test.tsx.snap deleted file mode 100644 index dd3a10689272f..0000000000000 --- a/x-pack/legacy/plugins/ml/public/data_frame/pages/data_frame_new_pivot/components/step_define/__snapshots__/pivot_preview.test.tsx.snap +++ /dev/null @@ -1,98 +0,0 @@ -// Jest Snapshot v1, https://goo.gl/fbAQLP - -exports[`Data Frame: Minimal initialization 1`] = ` -
- - - -
-`; diff --git a/x-pack/legacy/plugins/ml/public/data_frame/pages/data_frame_new_pivot/components/step_define/__snapshots__/step_define_form.test.tsx.snap b/x-pack/legacy/plugins/ml/public/data_frame/pages/data_frame_new_pivot/components/step_define/__snapshots__/step_define_form.test.tsx.snap deleted file mode 100644 index a45184587437e..0000000000000 --- a/x-pack/legacy/plugins/ml/public/data_frame/pages/data_frame_new_pivot/components/step_define/__snapshots__/step_define_form.test.tsx.snap +++ /dev/null @@ -1,71 +0,0 @@ -// Jest Snapshot v1, https://goo.gl/fbAQLP - -exports[`Data Frame: Minimal initialization 1`] = ` -
- - - -
-`; diff --git a/x-pack/legacy/plugins/ml/public/data_frame/pages/data_frame_new_pivot/components/step_define/__snapshots__/step_define_summary.test.tsx.snap b/x-pack/legacy/plugins/ml/public/data_frame/pages/data_frame_new_pivot/components/step_define/__snapshots__/step_define_summary.test.tsx.snap deleted file mode 100644 index 0c34c1f979000..0000000000000 --- a/x-pack/legacy/plugins/ml/public/data_frame/pages/data_frame_new_pivot/components/step_define/__snapshots__/step_define_summary.test.tsx.snap +++ /dev/null @@ -1,96 +0,0 @@ -// Jest Snapshot v1, https://goo.gl/fbAQLP - -exports[`Data Frame: Minimal initialization 1`] = ` -
- - - -
-`; From 441aa2ba530d7dfe55fc36127a6f8c36d4eabad6 Mon Sep 17 00:00:00 2001 From: Lukas Olson Date: Wed, 16 Oct 2019 16:38:09 -0700 Subject: [PATCH 25/25] Remove unused reference to IPrivate --- .../core_plugins/kibana/public/discover/context/api/context.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/legacy/core_plugins/kibana/public/discover/context/api/context.ts b/src/legacy/core_plugins/kibana/public/discover/context/api/context.ts index b1c20a1680224..48ac59f1f0855 100644 --- a/src/legacy/core_plugins/kibana/public/discover/context/api/context.ts +++ b/src/legacy/core_plugins/kibana/public/discover/context/api/context.ts @@ -19,7 +19,6 @@ // @ts-ignore import { SearchSource } from 'ui/courier'; -import { IPrivate } from 'ui/private'; import { Filter } from '@kbn/es-query'; import { IndexPatterns, IndexPattern } from 'ui/index_patterns'; import { reverseSortDir, SortDirection } from './utils/sorting';