From 2bd4962e3f43653339773145d4022f9bb09a5937 Mon Sep 17 00:00:00 2001 From: CJ Cenizal Date: Thu, 5 Jul 2018 12:24:35 -0700 Subject: [PATCH 01/28] Add SearchStrategyRegistry and defaultSearchStrategy to support existing search behavior, and integrate it with CallClient. --- src/ui/public/courier/courier.js | 7 +- src/ui/public/courier/fetch/call_client.js | 56 +++++++++---- .../default_search_strategy.js | 32 +++++++ .../public/courier/search_strategy/index.js | 27 ++++++ .../search_strategy_registry.js | 69 +++++++++++++++ .../search_strategy_registry.test.js | 83 +++++++++++++++++++ 6 files changed, 255 insertions(+), 19 deletions(-) create mode 100644 src/ui/public/courier/search_strategy/default_search_strategy.js create mode 100644 src/ui/public/courier/search_strategy/index.js create mode 100644 src/ui/public/courier/search_strategy/search_strategy_registry.js create mode 100644 src/ui/public/courier/search_strategy/search_strategy_registry.test.js diff --git a/src/ui/public/courier/courier.js b/src/ui/public/courier/courier.js index c468182cbb6fc9..66d3c406599134 100644 --- a/src/ui/public/courier/courier.js +++ b/src/ui/public/courier/courier.js @@ -30,6 +30,9 @@ import '../promises'; import { searchRequestQueue } from './search_request_queue'; import { FetchSoonProvider } from './fetch'; import { SearchPollProvider } from './search_poll'; +import { addSearchStrategy, defaultSearchStrategy } from './search_strategy'; + +addSearchStrategy(defaultSearchStrategy); uiModules.get('kibana/courier').service('courier', ($rootScope, Private) => { const fetchSoon = Private(FetchSoonProvider); @@ -74,12 +77,12 @@ uiModules.get('kibana/courier').service('courier', ($rootScope, Private) => { /** * Fetch the pending requests. */ - fetch = () => { + 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/ui/public/courier/fetch/call_client.js b/src/ui/public/courier/fetch/call_client.js index 93453c3a3f8eec..40774620c034f8 100644 --- a/src/ui/public/courier/fetch/call_client.js +++ b/src/ui/public/courier/fetch/call_client.js @@ -20,6 +20,7 @@ import _ from 'lodash'; import { ErrorAllowExplicitIndexProvider } from '../../error_allow_explicit_index'; +import { assignSearchRequestsToSearchStrategies } from '../search_strategy'; import { IsRequestProvider } from './is_request'; import { MergeDuplicatesRequestProvider } from './merge_duplicate_requests'; import { RequestStatus } from './req_status'; @@ -39,10 +40,14 @@ export function CallClientProvider(Private, Promise, es) { const statuses = mergeDuplicateRequests(requests); // get the actual list of requests that we will be fetching - let requestsToFetch = statuses.filter(isRequest); + const requestsToFetch = statuses.filter(isRequest); let execCount = requestsToFetch.length; - if (!execCount) return Promise.resolve([]); + if (!execCount) { + return Promise.resolve([]); + } + + const searchStrategiesWithRequests = assignSearchRequestsToSearchStrategies(requestsToFetch); // resolved by respond() let esPromise = undefined; @@ -52,14 +57,24 @@ export function CallClientProvider(Private, Promise, es) { // for each respond with either the response or ABORTED const respond = function (responses) { responses = responses || []; - return Promise.map(requests, function (request, i) { + const activeSearchRequests = searchStrategiesWithRequests.reduce((allSearchRequests, { searchRequests }) => { + return allSearchRequests.concat(searchRequests); + }, []) + .filter(request => { + // We'll use the index of the request to map it to its response. If a request has already + // failed then it won't generate a response. In this case we need to remove the request + // to maintain parity between the list of requests and the list of correspoding responses. + return request !== undefined; + }); + + return Promise.map(activeSearchRequests, function (request, i) { switch (statuses[i]) { case ABORTED: return ABORTED; case DUPLICATE: return request._uniq.resp; default: - const index = _.findIndex(requestsToFetch, request); + const index = _.findIndex(activeSearchRequests, request); if (index < 0) { // This means the request failed. return ABORTED; @@ -106,18 +121,19 @@ export function CallClientProvider(Private, Promise, es) { // 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 () => { - // Flatten the searchSource within each searchRequest to get the fetch params, - // e.g. body, filters, index pattern, query. - const allFetchParams = await getAllFetchParams(requestsToFetch); + // Execute each request using its search strategy. + const esPromises = searchStrategiesWithRequests.map(async searchStrategyWithSearchRequests => { + const { searchStrategy, searchRequests: searchStrategyRequests } = searchStrategyWithSearchRequests; - // Serialize the fetch params into a format suitable for the body of an ES query. - const serializedFetchParams = await serializeAllFetchParams(allFetchParams, requestsToFetch); + // Flatten the searchSource within each searchRequest to get the fetch params, + // e.g. body, filters, index pattern, query. + const allFetchParams = await getAllFetchParams(searchStrategyRequests); - // The index of the request inside requestsToFetch determines which response is mapped to it. - // If a request won't generate a response, since it already failed, we need to remove the - // request from the requestsToFetch array so the indexes will continue to match up to the - // responses correctly. - requestsToFetch = requestsToFetch.filter(request => request !== undefined); + // Serialize the fetch params into a format suitable for the body of an ES query. + const serializedFetchParams = await serializeAllFetchParams(allFetchParams, searchStrategyRequests); + + return searchStrategy.search({ body: serializedFetchParams, es }); + }); try { // The request was aborted while we were doing the above logic. @@ -125,9 +141,15 @@ export function CallClientProvider(Private, Promise, es) { throw ABORTED; } - esPromise = es.msearch({ body: serializedFetchParams }); - const clientResponse = await esPromise; - await respond(clientResponse.responses); + esPromise = Promise.all(esPromises); + const segregatedResponses = await esPromise; + + // Aggregate the responses returned by all of the search strategies. + const aggregatedResponses = segregatedResponses.reduce((aggregation, responses) => { + return aggregation.concat(responses.responses); + }, []); + + await respond(aggregatedResponses); } catch(error) { if (error === ABORTED) { return await respond(); diff --git a/src/ui/public/courier/search_strategy/default_search_strategy.js b/src/ui/public/courier/search_strategy/default_search_strategy.js new file mode 100644 index 00000000000000..1f3597b1e67568 --- /dev/null +++ b/src/ui/public/courier/search_strategy/default_search_strategy.js @@ -0,0 +1,32 @@ +/* + * 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 defaultSearchStrategy = { + id: 'default', + + search: ({ body, es }) => { + return es.msearch({ body }); + }, + + isValidForSearchRequest: searchRequest => { + // Basic index patterns don't have `type` defined. + const indexPattern = searchRequest.source.getField('index'); + return indexPattern.type == null; + }, +}; diff --git a/src/ui/public/courier/search_strategy/index.js b/src/ui/public/courier/search_strategy/index.js new file mode 100644 index 00000000000000..533acd17c481e2 --- /dev/null +++ b/src/ui/public/courier/search_strategy/index.js @@ -0,0 +1,27 @@ +/* + * 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 { + assignSearchRequestsToSearchStrategies, + addSearchStrategy, +} from './search_strategy_registry'; + +export { + defaultSearchStrategy, +} from './default_search_strategy'; diff --git a/src/ui/public/courier/search_strategy/search_strategy_registry.js b/src/ui/public/courier/search_strategy/search_strategy_registry.js new file mode 100644 index 00000000000000..3de2042666ccce --- /dev/null +++ b/src/ui/public/courier/search_strategy/search_strategy_registry.js @@ -0,0 +1,69 @@ +/* + * 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. + */ + +const searchStrategies = []; + +const addSearchStrategy = searchStrategy => { + searchStrategies.push(searchStrategy); +}; + +/** + * Build a structure like this: + * + * [{ + * searchStrategy: , + * searchRequests: [], + * }, { + * searchStrategy: , + * searchRequests: [], + * }] + * + * We use an array of objects to preserve the order of the search requests, which use to + * deterministically associate each response with the originating request. + */ +const assignSearchRequestsToSearchStrategies = searchRequests => { + const searchStrategiesWithRequests = []; + const searchStrategyById = {}; + + searchRequests.forEach(searchRequest => { + const matchingSearchStrategy = searchStrategies.find(searchStrategy => searchStrategy.isValidForSearchRequest(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 { + assignSearchRequestsToSearchStrategies, + addSearchStrategy, +}; diff --git a/src/ui/public/courier/search_strategy/search_strategy_registry.test.js b/src/ui/public/courier/search_strategy/search_strategy_registry.test.js new file mode 100644 index 00000000000000..d83625964304cd --- /dev/null +++ b/src/ui/public/courier/search_strategy/search_strategy_registry.test.js @@ -0,0 +1,83 @@ +/* + * 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 { + assignSearchRequestsToSearchStrategies, + addSearchStrategy, +} from './search_strategy_registry'; + +describe('SearchStrategyRegistry', () => { + describe('assignSearchRequestsToSearchStrategies', () => { + test('associates search requests with valid search strategies', () => { + const searchStrategyA = { + id: 'a', + isValidForSearchRequest: searchRequest => { + return searchRequest.type === 'a'; + }, + }; + + addSearchStrategy(searchStrategyA); + + const searchStrategyB = { + id: 'b', + isValidForSearchRequest: searchRequest => { + return searchRequest.type === 'b'; + }, + }; + + addSearchStrategy(searchStrategyB); + + const searchRequests = [{ + id: 0, + type: 'b', + }, { + id: 1, + type: 'a', + }, { + id: 2, + type: 'a', + }, { + id: 3, + type: 'b', + }]; + + const searchStrategiesWithSearchRequests = assignSearchRequestsToSearchStrategies(searchRequests); + + expect(searchStrategiesWithSearchRequests).toEqual([{ + searchStrategy: searchStrategyB, + searchRequests: [{ + id: 0, + type: 'b', + }, { + id: 3, + type: 'b', + }], + }, { + searchStrategy: searchStrategyA, + searchRequests: [{ + id: 1, + type: 'a', + }, { + id: 2, + type: 'a', + }], + }]); + }); + }); +}); From 1c1b99aedfbb6ed48a349b8e48f042e62e76bf92 Mon Sep 17 00:00:00 2001 From: CJ Cenizal Date: Thu, 5 Jul 2018 15:23:27 -0700 Subject: [PATCH 02/28] Fix typo. --- .../public/courier/search_strategy/search_strategy_registry.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ui/public/courier/search_strategy/search_strategy_registry.js b/src/ui/public/courier/search_strategy/search_strategy_registry.js index 3de2042666ccce..5954b711bb84ec 100644 --- a/src/ui/public/courier/search_strategy/search_strategy_registry.js +++ b/src/ui/public/courier/search_strategy/search_strategy_registry.js @@ -34,7 +34,7 @@ const addSearchStrategy = searchStrategy => { * searchRequests: [], * }] * - * We use an array of objects to preserve the order of the search requests, which use to + * 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. */ const assignSearchRequestsToSearchStrategies = searchRequests => { From a0386eaf16ed845e21d256cf6e0221e0cb9e80ae Mon Sep 17 00:00:00 2001 From: CJ Cenizal Date: Thu, 5 Jul 2018 21:06:42 -0700 Subject: [PATCH 03/28] Move fetch param logic from CallClient into defaultSearchStrategy. --- src/ui/public/courier/fetch/call_client.js | 41 ++----------------- .../default_search_strategy.js | 39 +++++++++++++++++- 2 files changed, 40 insertions(+), 40 deletions(-) diff --git a/src/ui/public/courier/fetch/call_client.js b/src/ui/public/courier/fetch/call_client.js index 40774620c034f8..193bf3adfe5819 100644 --- a/src/ui/public/courier/fetch/call_client.js +++ b/src/ui/public/courier/fetch/call_client.js @@ -122,17 +122,9 @@ export function CallClientProvider(Private, Promise, es) { // asynchronously after we've returned a reference to defer.promise. Promise.resolve().then(async () => { // Execute each request using its search strategy. - const esPromises = searchStrategiesWithRequests.map(async searchStrategyWithSearchRequests => { - const { searchStrategy, searchRequests: searchStrategyRequests } = searchStrategyWithSearchRequests; - - // Flatten the searchSource within each searchRequest to get the fetch params, - // e.g. body, filters, index pattern, query. - const allFetchParams = await getAllFetchParams(searchStrategyRequests); - - // Serialize the fetch params into a format suitable for the body of an ES query. - const serializedFetchParams = await serializeAllFetchParams(allFetchParams, searchStrategyRequests); - - return searchStrategy.search({ body: serializedFetchParams, es }); + const esPromises = searchStrategiesWithRequests.map(searchStrategyWithSearchRequests => { + const { searchStrategy, searchRequests } = searchStrategyWithSearchRequests; + return searchStrategy.search({ searchRequests, es, Promise, serializeFetchParams }); }); try { @@ -175,32 +167,5 @@ export function CallClientProvider(Private, Promise, es) { }); } - function getAllFetchParams(requests) { - return Promise.map(requests, (request) => { - return Promise.try(request.getFetchParams, void 0, request) - .then((fetchParams) => { - return (request.fetchParams = fetchParams); - }) - .then(value => ({ resolved: value })) - .catch(error => ({ rejected: error })); - }); - } - - function serializeAllFetchParams(fetchParams, requestsToFetch) { - const requestsWithFetchParams = []; - - // Gather the fetch param responses from all the successful requests. - fetchParams.forEach((result, index) => { - if (result.resolved) { - requestsWithFetchParams.push(result.resolved); - } else { - const request = requestsToFetch[index]; - request.handleFailure(result.rejected); - requestsToFetch[index] = undefined; - } - }); - return serializeFetchParams(requestsWithFetchParams); - } - return callClient; } diff --git a/src/ui/public/courier/search_strategy/default_search_strategy.js b/src/ui/public/courier/search_strategy/default_search_strategy.js index 1f3597b1e67568..a300ce69177158 100644 --- a/src/ui/public/courier/search_strategy/default_search_strategy.js +++ b/src/ui/public/courier/search_strategy/default_search_strategy.js @@ -17,11 +17,46 @@ * under the License. */ +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, serializeFetchParams) { + const requestsWithFetchParams = []; + + // Gather the fetch param responses from all the successful requests. + fetchParams.forEach((result, index) => { + if (result.resolved) { + requestsWithFetchParams.push(result.resolved); + } else { + const request = searchRequests[index]; + request.handleFailure(result.rejected); + searchRequests[index] = undefined; + } + }); + + return serializeFetchParams(requestsWithFetchParams); +} + export const defaultSearchStrategy = { id: 'default', - search: ({ body, es }) => { - return es.msearch({ body }); + search: async ({ searchRequests, es, Promise, serializeFetchParams }) => { + // 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 = await serializeAllFetchParams(allFetchParams, searchRequests, serializeFetchParams); + + return es.msearch({ body: serializedFetchParams }); }, isValidForSearchRequest: searchRequest => { From ebd354459146e653d75e35595a02e9fab2cde22d Mon Sep 17 00:00:00 2001 From: CJ Cenizal Date: Fri, 6 Jul 2018 10:08:27 -0700 Subject: [PATCH 04/28] Move defaultSearchStrategy configuration into kibana plugin via search uiExport to avoid race conditions with other plugins. --- src/core_plugins/kibana/public/kibana.js | 1 + src/ui/public/courier/courier.js | 3 --- .../courier/search_strategy/default_search_strategy.js | 4 ++++ src/ui/public/courier/search_strategy/index.js | 4 ---- src/ui/ui_exports/ui_export_defaults.js | 5 ++++- src/ui/ui_exports/ui_export_types/index.js | 3 ++- src/ui/ui_exports/ui_export_types/ui_app_extensions.js | 1 + 7 files changed, 12 insertions(+), 9 deletions(-) diff --git a/src/core_plugins/kibana/public/kibana.js b/src/core_plugins/kibana/public/kibana.js index aa5a563ca710e8..ee0da07aa8d3ee 100644 --- a/src/core_plugins/kibana/public/kibana.js +++ b/src/core_plugins/kibana/public/kibana.js @@ -41,6 +41,7 @@ import 'uiExports/devTools'; import 'uiExports/docViews'; import 'uiExports/embeddableFactories'; import 'uiExports/inspectorViews'; +import 'uiExports/search'; import 'ui/autoload/all'; import './home'; diff --git a/src/ui/public/courier/courier.js b/src/ui/public/courier/courier.js index 66d3c406599134..a969f09ad6481e 100644 --- a/src/ui/public/courier/courier.js +++ b/src/ui/public/courier/courier.js @@ -30,9 +30,6 @@ import '../promises'; import { searchRequestQueue } from './search_request_queue'; import { FetchSoonProvider } from './fetch'; import { SearchPollProvider } from './search_poll'; -import { addSearchStrategy, defaultSearchStrategy } from './search_strategy'; - -addSearchStrategy(defaultSearchStrategy); uiModules.get('kibana/courier').service('courier', ($rootScope, Private) => { const fetchSoon = Private(FetchSoonProvider); diff --git a/src/ui/public/courier/search_strategy/default_search_strategy.js b/src/ui/public/courier/search_strategy/default_search_strategy.js index a300ce69177158..b96e421a5cce48 100644 --- a/src/ui/public/courier/search_strategy/default_search_strategy.js +++ b/src/ui/public/courier/search_strategy/default_search_strategy.js @@ -17,6 +17,8 @@ * under the License. */ +import { addSearchStrategy } from './search_strategy_registry'; + function getAllFetchParams(searchRequests, Promise) { return Promise.map(searchRequests, (searchRequest) => { return Promise.try(searchRequest.getFetchParams, void 0, searchRequest) @@ -65,3 +67,5 @@ export const defaultSearchStrategy = { return indexPattern.type == null; }, }; + +addSearchStrategy(defaultSearchStrategy); diff --git a/src/ui/public/courier/search_strategy/index.js b/src/ui/public/courier/search_strategy/index.js index 533acd17c481e2..af37b9b1594559 100644 --- a/src/ui/public/courier/search_strategy/index.js +++ b/src/ui/public/courier/search_strategy/index.js @@ -21,7 +21,3 @@ export { assignSearchRequestsToSearchStrategies, addSearchStrategy, } from './search_strategy_registry'; - -export { - defaultSearchStrategy, -} from './default_search_strategy'; diff --git a/src/ui/ui_exports/ui_export_defaults.js b/src/ui/ui_exports/ui_export_defaults.js index ae8f175d0e846c..d12f14bb4b10eb 100644 --- a/src/ui/ui_exports/ui_export_defaults.js +++ b/src/ui/ui_exports/ui_export_defaults.js @@ -56,6 +56,9 @@ export const UI_EXPORT_DEFAULTS = { embeddableFactories: [ 'plugins/kibana/visualize/embeddable/visualize_embeddable_factory_provider', 'plugins/kibana/discover/embeddable/search_embeddable_factory_provider', - ] + ], + search: [ + 'ui/courier/search_strategy/default_search_strategy', + ], }, }; diff --git a/src/ui/ui_exports/ui_export_types/index.js b/src/ui/ui_exports/ui_export_types/index.js index b584cdbc27ec26..08d03bc04b14bd 100644 --- a/src/ui/ui_exports/ui_export_types/index.js +++ b/src/ui/ui_exports/ui_export_types/index.js @@ -51,7 +51,8 @@ export { home, visTypeEnhancers, aliases, - visualize + visualize, + search, } from './ui_app_extensions'; export { diff --git a/src/ui/ui_exports/ui_export_types/ui_app_extensions.js b/src/ui/ui_exports/ui_export_types/ui_app_extensions.js index a01e7059d7945b..738b794cb31070 100644 --- a/src/ui/ui_exports/ui_export_types/ui_app_extensions.js +++ b/src/ui/ui_exports/ui_export_types/ui_app_extensions.js @@ -50,6 +50,7 @@ export const docViews = appExtension; export const hacks = appExtension; export const home = appExtension; export const inspectorViews = appExtension; +export const search = appExtension; // Add a visualize app extension that should be used for visualize specific stuff export const visualize = appExtension; From 27ba85c8e0cb4c6492dd36443e8c212db163c302 Mon Sep 17 00:00:00 2001 From: CJ Cenizal Date: Fri, 6 Jul 2018 11:05:47 -0700 Subject: [PATCH 05/28] Fix typo in comment. --- src/ui/public/courier/fetch/call_client.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/ui/public/courier/fetch/call_client.js b/src/ui/public/courier/fetch/call_client.js index 193bf3adfe5819..4f95ad7175872e 100644 --- a/src/ui/public/courier/fetch/call_client.js +++ b/src/ui/public/courier/fetch/call_client.js @@ -63,7 +63,8 @@ export function CallClientProvider(Private, Promise, es) { .filter(request => { // We'll use the index of the request to map it to its response. If a request has already // failed then it won't generate a response. In this case we need to remove the request - // to maintain parity between the list of requests and the list of correspoding responses. + // to maintain cardinality between the list of requests and the list of corresponding + // responses. return request !== undefined; }); From 8a7e05fa86467ef564dd3319b7d38c6d7ecedc99 Mon Sep 17 00:00:00 2001 From: CJ Cenizal Date: Fri, 6 Jul 2018 15:18:19 -0700 Subject: [PATCH 06/28] Fix bug where Dashboard Only Mode was broken. --- x-pack/plugins/dashboard_mode/public/dashboard_viewer.js | 1 + 1 file changed, 1 insertion(+) diff --git a/x-pack/plugins/dashboard_mode/public/dashboard_viewer.js b/x-pack/plugins/dashboard_mode/public/dashboard_viewer.js index dcd8eb41e52efe..3da539144771e7 100644 --- a/x-pack/plugins/dashboard_mode/public/dashboard_viewer.js +++ b/x-pack/plugins/dashboard_mode/public/dashboard_viewer.js @@ -24,6 +24,7 @@ import 'uiExports/embeddableFactories'; import 'uiExports/navbarExtensions'; import 'uiExports/docViews'; import 'uiExports/fieldFormats'; +import 'uiExports/search'; import _ from 'lodash'; import 'ui/autoload/all'; From 0a8442b9f47fd16385eaaf4c21703830af2461e4 Mon Sep 17 00:00:00 2001 From: CJ Cenizal Date: Mon, 9 Jul 2018 11:18:31 -0700 Subject: [PATCH 07/28] Don't throw ABORTED. --- src/ui/public/courier/fetch/call_client.js | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/src/ui/public/courier/fetch/call_client.js b/src/ui/public/courier/fetch/call_client.js index 4f95ad7175872e..f047d5f1e2f113 100644 --- a/src/ui/public/courier/fetch/call_client.js +++ b/src/ui/public/courier/fetch/call_client.js @@ -55,8 +55,7 @@ export function CallClientProvider(Private, Promise, es) { const defer = Promise.defer(); // for each respond with either the response or ABORTED - const respond = function (responses) { - responses = responses || []; + const respond = function (responses = []) { const activeSearchRequests = searchStrategiesWithRequests.reduce((allSearchRequests, { searchRequests }) => { return allSearchRequests.concat(searchRequests); }, []) @@ -131,7 +130,7 @@ export function CallClientProvider(Private, Promise, es) { try { // The request was aborted while we were doing the above logic. if (isRequestAborted) { - throw ABORTED; + return await respond(); } esPromise = Promise.all(esPromises); @@ -144,10 +143,6 @@ export function CallClientProvider(Private, Promise, es) { await respond(aggregatedResponses); } catch(error) { - if (error === ABORTED) { - return await respond(); - } - if (errorAllowExplicitIndex.test(error)) { return errorAllowExplicitIndex.takeover(); } From 9777a6083f16e6283d0e41e7860843d8ef18d735 Mon Sep 17 00:00:00 2001 From: CJ Cenizal Date: Mon, 9 Jul 2018 13:18:04 -0700 Subject: [PATCH 08/28] Add comments and refactor for clarity. --- src/ui/public/courier/fetch/call_client.js | 89 ++++++++++++---------- 1 file changed, 49 insertions(+), 40 deletions(-) diff --git a/src/ui/public/courier/fetch/call_client.js b/src/ui/public/courier/fetch/call_client.js index f047d5f1e2f113..d4aa16b52fc10c 100644 --- a/src/ui/public/courier/fetch/call_client.js +++ b/src/ui/public/courier/fetch/call_client.js @@ -35,9 +35,9 @@ export function CallClientProvider(Private, Promise, es) { const ABORTED = RequestStatus.ABORTED; const DUPLICATE = RequestStatus.DUPLICATE; - function callClient(requests) { + function callClient(searchRequests) { // merging docs can change status to DUPLICATE, capture new statuses - const statuses = mergeDuplicateRequests(requests); + const statuses = mergeDuplicateRequests(searchRequests); // get the actual list of requests that we will be fetching const requestsToFetch = statuses.filter(isRequest); @@ -47,40 +47,48 @@ export function CallClientProvider(Private, Promise, es) { return Promise.resolve([]); } + // This is how we'll provide the consumer with search responses. + const defer = Promise.defer(); + const searchStrategiesWithRequests = assignSearchRequestsToSearchStrategies(requestsToFetch); - // resolved by respond() + // resolved by respondToSearchRequests() let esPromise = undefined; let isRequestAborted = false; - const defer = Promise.defer(); // for each respond with either the response or ABORTED - const respond = function (responses = []) { - const activeSearchRequests = searchStrategiesWithRequests.reduce((allSearchRequests, { searchRequests }) => { + const respondToSearchRequests = (responses = []) => { + const aggregatedSearchRequests = searchStrategiesWithRequests.reduce((allSearchRequests, { searchRequests }) => { return allSearchRequests.concat(searchRequests); - }, []) - .filter(request => { - // We'll use the index of the request to map it to its response. If a request has already - // failed then it won't generate a response. In this case we need to remove the request - // to maintain cardinality between the list of requests and the list of corresponding - // responses. - return request !== undefined; - }); - - return Promise.map(activeSearchRequests, function (request, i) { - switch (statuses[i]) { - case ABORTED: - return ABORTED; - case DUPLICATE: - return request._uniq.resp; - default: - const index = _.findIndex(activeSearchRequests, request); - if (index < 0) { - // This means the request failed. - return ABORTED; - } - return responses[index]; + }, []); + + const activeSearchRequests = aggregatedSearchRequests.filter(request => { + // We'll use the index of the request to map it to its response. If a request has already + // failed then it won't generate a response. In this case we need to remove the request + // to maintain cardinality between the list of requests and the list of corresponding + // responses. + return request !== undefined; + }); + + return Promise.map(activeSearchRequests, function (searchRequest, searchRequestIndex) { + const status = statuses[searchRequestIndex]; + + if (status === ABORTED) { + return ABORTED; + } + + if (status === DUPLICATE) { + return searchRequest._uniq.resp; } + + const index = _.findIndex(activeSearchRequests, searchRequest); + + if (index < 0) { + // This means the searchRequest failed. + return ABORTED; + } + + return responses[index]; }) .then( (res) => defer.resolve(res), @@ -107,7 +115,7 @@ export function CallClientProvider(Private, Promise, es) { esPromise = undefined; isRequestAborted = true; - return respond(); + return respondToSearchRequests(); }); // attach abort handlers, close over request index @@ -130,7 +138,7 @@ export function CallClientProvider(Private, Promise, es) { try { // The request was aborted while we were doing the above logic. if (isRequestAborted) { - return await respond(); + return await respondToSearchRequests(); } esPromise = Promise.all(esPromises); @@ -141,7 +149,7 @@ export function CallClientProvider(Private, Promise, es) { return aggregation.concat(responses.responses); }, []); - await respond(aggregatedResponses); + await respondToSearchRequests(aggregatedResponses); } catch(error) { if (errorAllowExplicitIndex.test(error)) { return errorAllowExplicitIndex.takeover(); @@ -151,16 +159,17 @@ export function CallClientProvider(Private, Promise, es) { } }); - // return our promise, but catch any errors we create and - // send them to the requests - return defer.promise - .catch((err) => { - requests.forEach((req, index) => { - if (statuses[index] !== ABORTED) { - req.handleFailure(err); - } - }); + // If there are any errors, notify the searchRequests of them. + defer.promise.catch((err) => { + searchRequests.forEach((searchRequest, index) => { + if (statuses[index] !== ABORTED) { + searchRequest.handleFailure(err); + } }); + }); + + // Return the promise which acts as our vehicle for providing search responses to the consumer. + return defer.promise; } return callClient; From 31925d51f157280a79df082b8b0befd7a940dc43 Mon Sep 17 00:00:00 2001 From: CJ Cenizal Date: Tue, 10 Jul 2018 13:59:34 -0700 Subject: [PATCH 09/28] Refactor strategy interface to be more flexible with the arguments it accepts. --- .../search_strategy/default_search_strategy.js | 12 +++++++++--- .../search_strategy/search_strategy_registry.js | 14 +++++++++++++- .../search_strategy_registry.test.js | 4 ++-- 3 files changed, 24 insertions(+), 6 deletions(-) diff --git a/src/ui/public/courier/search_strategy/default_search_strategy.js b/src/ui/public/courier/search_strategy/default_search_strategy.js index b96e421a5cce48..32af5168ff0ef8 100644 --- a/src/ui/public/courier/search_strategy/default_search_strategy.js +++ b/src/ui/public/courier/search_strategy/default_search_strategy.js @@ -61,10 +61,16 @@ export const defaultSearchStrategy = { return es.msearch({ body: serializedFetchParams }); }, - isValidForSearchRequest: searchRequest => { + // Accept multiple criteria for determining viability to be as flexible as possible. + isViable: ({ indexPattern, searchRequest }) => { + if (!indexPattern && !searchRequest) { + return false; + } + + const derivedIndexPattern = indexPattern || searchRequest.source.getField('index'); + // Basic index patterns don't have `type` defined. - const indexPattern = searchRequest.source.getField('index'); - return indexPattern.type == null; + return derivedIndexPattern.type == null; }, }; diff --git a/src/ui/public/courier/search_strategy/search_strategy_registry.js b/src/ui/public/courier/search_strategy/search_strategy_registry.js index 5954b711bb84ec..840e6d118363ba 100644 --- a/src/ui/public/courier/search_strategy/search_strategy_registry.js +++ b/src/ui/public/courier/search_strategy/search_strategy_registry.js @@ -23,6 +23,12 @@ const addSearchStrategy = searchStrategy => { searchStrategies.push(searchStrategy); }; +const getSearchStrategy = (...rest) => { + return searchStrategies.find(searchStrategy => { + return searchStrategy.isViable(...rest); + }); +}; + /** * Build a structure like this: * @@ -42,7 +48,8 @@ const assignSearchRequestsToSearchStrategies = searchRequests => { const searchStrategyById = {}; searchRequests.forEach(searchRequest => { - const matchingSearchStrategy = searchStrategies.find(searchStrategy => searchStrategy.isValidForSearchRequest(searchRequest)); + const matchingSearchStrategy = getSearchStrategy({ searchRequest }); + const { id } = matchingSearchStrategy; let searchStrategyWithRequest = searchStrategyById[id]; @@ -63,7 +70,12 @@ const assignSearchRequestsToSearchStrategies = searchRequests => { return searchStrategiesWithRequests; }; +const hasSearchStategyForIndexPattern = indexPattern => { + return Boolean(getSearchStrategy({ indexPattern })); +}; + export { assignSearchRequestsToSearchStrategies, addSearchStrategy, + hasSearchStategyForIndexPattern, }; diff --git a/src/ui/public/courier/search_strategy/search_strategy_registry.test.js b/src/ui/public/courier/search_strategy/search_strategy_registry.test.js index d83625964304cd..d33d5b253984a7 100644 --- a/src/ui/public/courier/search_strategy/search_strategy_registry.test.js +++ b/src/ui/public/courier/search_strategy/search_strategy_registry.test.js @@ -27,7 +27,7 @@ describe('SearchStrategyRegistry', () => { test('associates search requests with valid search strategies', () => { const searchStrategyA = { id: 'a', - isValidForSearchRequest: searchRequest => { + isViable: ({ searchRequest }) => { return searchRequest.type === 'a'; }, }; @@ -36,7 +36,7 @@ describe('SearchStrategyRegistry', () => { const searchStrategyB = { id: 'b', - isValidForSearchRequest: searchRequest => { + isViable: ({ searchRequest }) => { return searchRequest.type === 'b'; }, }; From b74224cacbb91907a885f5641f919323597706c5 Mon Sep 17 00:00:00 2001 From: CJ Cenizal Date: Tue, 10 Jul 2018 15:18:50 -0700 Subject: [PATCH 10/28] Add call-out react directive. --- src/ui/public/react_components.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/ui/public/react_components.js b/src/ui/public/react_components.js index 9bbfb2c366a2c3..521ecd52920e7b 100644 --- a/src/ui/public/react_components.js +++ b/src/ui/public/react_components.js @@ -28,6 +28,7 @@ import { EuiIcon, EuiColorPicker, EuiIconTip, + EuiCallOut, } from '@elastic/eui'; import { uiModules } from './modules'; @@ -43,3 +44,5 @@ app.directive('icon', reactDirective => reactDirective(EuiIcon)); app.directive('colorPicker', reactDirective => reactDirective(EuiColorPicker)); app.directive('iconTip', reactDirective => reactDirective(EuiIconTip, ['content', 'type', 'position', 'title'])); + +app.directive('callOut', reactDirective => reactDirective(EuiCallOut, ['title', 'color', 'size', 'iconType', 'children'])); From 9d02ff2ee4c56392ef3be6b57fd086997728ea0f Mon Sep 17 00:00:00 2001 From: CJ Cenizal Date: Tue, 10 Jul 2018 15:19:58 -0700 Subject: [PATCH 11/28] Show error in Discover if user tries to access a rollup index pattern without the right search strategy. --- .../public/discover/controllers/discover.js | 26 ++++++++++++++----- .../kibana/public/discover/index.html | 7 +++++ src/ui/public/courier/index.js | 7 +++++ .../public/courier/search_strategy/index.js | 2 ++ .../search_strategy_registry.js | 9 +++++-- 5 files changed, 43 insertions(+), 8 deletions(-) diff --git a/src/core_plugins/kibana/public/discover/controllers/discover.js b/src/core_plugins/kibana/public/discover/controllers/discover.js index d7c6d982eb8281..eb4321b0181853 100644 --- a/src/core_plugins/kibana/public/discover/controllers/discover.js +++ b/src/core_plugins/kibana/public/discover/controllers/discover.js @@ -28,12 +28,12 @@ import 'ui/visualize'; import 'ui/fixed_scroll'; import 'ui/directives/validate_json'; import 'ui/filters/moment'; -import 'ui/courier'; import 'ui/index_patterns'; import 'ui/state_management/app_state'; import { timefilter } from 'ui/timefilter'; import 'ui/share'; import 'ui/query_bar'; +import { hasSearchStategyForIndexPattern, isRollupIndexPattern } from 'ui/courier'; import { toastNotifications, getPainlessError } from 'ui/notify'; import { VisProvider } from 'ui/vis'; import { BasicResponseHandlerProvider } from 'ui/vis/response_handlers/basic'; @@ -195,6 +195,7 @@ function discoverController( // the actual courier.SearchSource $scope.searchSource = savedSearch.searchSource; $scope.indexPattern = resolveIndexPatternLoading(); + $scope.searchSource .setField('index', $scope.indexPattern) .setField('highlightAll', true) @@ -237,7 +238,6 @@ function discoverController( }); }; - const getSharingDataFields = async () => { const selectedFields = $state.columns; if (selectedFields.length === 1 && selectedFields[0] === '_source') { @@ -752,18 +752,32 @@ function discoverController( const own = $scope.searchSource.getOwnField('index'); - if (own && !stateVal) return own; + if (own && !stateVal) { + return own; + } + if (stateVal && !stateValFound) { - const err = '"' + stateVal + '" is not a configured pattern ID. '; if (own) { - notify.warning(`${err} Using the saved index pattern: "${own.title}" (${own.id})`); + notify.warning(`"${stateVal}" is not a configured pattern ID. Using the saved index pattern: "${own.title}" (${own.id})`); return own; } - notify.warning(`${err} Using the default index pattern: "${loaded.title}" (${loaded.id})`); + notify.warning(`"${stateVal}" is not a configured pattern ID. Using the default index pattern: "${loaded.title}" (${loaded.id})`); } + return loaded; } + // Block the UI from loading if the user has loaded a rollup index pattern but it isn't + // supported. + $scope.isUnsupportedRollup = ( + isRollupIndexPattern($route.current.locals.ip.loaded) + && !hasSearchStategyForIndexPattern($route.current.locals.ip.loaded) + ); + + if ($scope.isUnsupportedRollup) { + return; + } + init(); } diff --git a/src/core_plugins/kibana/public/discover/index.html b/src/core_plugins/kibana/public/discover/index.html index a8fde71b987fb1..78955a8c9de94b 100644 --- a/src/core_plugins/kibana/public/discover/index.html +++ b/src/core_plugins/kibana/public/discover/index.html @@ -60,6 +60,13 @@

+ + { * Build a structure like this: * * [{ - * searchStrategy: , + * searchStrategy: rollupSearchStrategy, * searchRequests: [], * }, { - * searchStrategy: , + * searchStrategy: defaultSearchStrategy, * searchRequests: [], * }] * @@ -74,8 +74,13 @@ const hasSearchStategyForIndexPattern = indexPattern => { return Boolean(getSearchStrategy({ indexPattern })); }; +const isRollupIndexPattern = indexPattern => { + return indexPattern.type === 'rollup'; +}; + export { assignSearchRequestsToSearchStrategies, addSearchStrategy, hasSearchStategyForIndexPattern, + isRollupIndexPattern, }; From d48f65b9e3aeaf387a2fc80e951eba81f82be237 Mon Sep 17 00:00:00 2001 From: CJ Cenizal Date: Wed, 11 Jul 2018 13:56:30 -0700 Subject: [PATCH 12/28] Improve logic for resolving an early response if all searchRequests have been aborted. --- src/ui/public/courier/fetch/call_client.js | 32 +++++++++++----------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/src/ui/public/courier/fetch/call_client.js b/src/ui/public/courier/fetch/call_client.js index d4aa16b52fc10c..2f5cbc38fea20c 100644 --- a/src/ui/public/courier/fetch/call_client.js +++ b/src/ui/public/courier/fetch/call_client.js @@ -41,9 +41,9 @@ export function CallClientProvider(Private, Promise, es) { // get the actual list of requests that we will be fetching const requestsToFetch = statuses.filter(isRequest); - let execCount = requestsToFetch.length; + let requestsToFetchCount = requestsToFetch.length; - if (!execCount) { + if (requestsToFetchCount === 0) { return Promise.resolve([]); } @@ -53,7 +53,7 @@ export function CallClientProvider(Private, Promise, es) { const searchStrategiesWithRequests = assignSearchRequestsToSearchStrategies(requestsToFetch); // resolved by respondToSearchRequests() - let esPromise = undefined; + let searchRequestPromises = undefined; let isRequestAborted = false; // for each respond with either the response or ABORTED @@ -97,22 +97,23 @@ export function CallClientProvider(Private, Promise, es) { }; // handle a request being aborted while being fetched - const requestWasAborted = Promise.method(function (req, i) { - if (statuses[i] === ABORTED) { + const requestWasAborted = Promise.method(function (searchRequest, index) { + if (statuses[index] === ABORTED) { defer.reject(new Error('Request was aborted twice?')); } - execCount -= 1; - if (execCount > 0) { - // the multi-request still contains other requests + requestsToFetchCount--; + + if (requestsToFetchCount !== 0) { + // We can't resolve early unless all searchRequests have been aborted. return; } - if (esPromise && _.isFunction(esPromise.abort)) { - esPromise.abort(); + if (searchRequestPromises) { + searchRequestPromises.forEach(searchRequestPromise => searchRequestPromise.abort()); } - esPromise = undefined; + searchRequestPromises = undefined; isRequestAborted = true; return respondToSearchRequests(); @@ -130,7 +131,7 @@ export function CallClientProvider(Private, Promise, es) { // asynchronously after we've returned a reference to defer.promise. Promise.resolve().then(async () => { // Execute each request using its search strategy. - const esPromises = searchStrategiesWithRequests.map(searchStrategyWithSearchRequests => { + searchRequestPromises = searchStrategiesWithRequests.map(searchStrategyWithSearchRequests => { const { searchStrategy, searchRequests } = searchStrategyWithSearchRequests; return searchStrategy.search({ searchRequests, es, Promise, serializeFetchParams }); }); @@ -141,12 +142,11 @@ export function CallClientProvider(Private, Promise, es) { return await respondToSearchRequests(); } - esPromise = Promise.all(esPromises); - const segregatedResponses = await esPromise; + const segregatedResponses = await Promise.all(searchRequestPromises); // Aggregate the responses returned by all of the search strategies. - const aggregatedResponses = segregatedResponses.reduce((aggregation, responses) => { - return aggregation.concat(responses.responses); + const aggregatedResponses = segregatedResponses.reduce((allResponses, responses) => { + return allResponses.concat(responses.responses); }, []); await respondToSearchRequests(aggregatedResponses); From 5c7666cdf884de3622474c54d73ae59202a6c689 Mon Sep 17 00:00:00 2001 From: CJ Cenizal Date: Thu, 12 Jul 2018 15:08:57 -0700 Subject: [PATCH 13/28] Fix bugs in callClient using the tests as a guide. --- .../courier/fetch/__tests__/call_client.js | 15 +++-- src/ui/public/courier/fetch/call_client.js | 57 +++++++++++-------- .../default_search_strategy.js | 28 ++++++--- 3 files changed, 64 insertions(+), 36 deletions(-) diff --git a/src/ui/public/courier/fetch/__tests__/call_client.js b/src/ui/public/courier/fetch/__tests__/call_client.js index 7b0439b43f3922..cdf7db72431bea 100644 --- a/src/ui/public/courier/fetch/__tests__/call_client.js +++ b/src/ui/public/courier/fetch/__tests__/call_client.js @@ -42,14 +42,18 @@ describe('callClient', () => { let esPromiseAbortSpy; const createSearchRequest = (id, overrides = {}) => { + const { source: overrideSource, ...rest } = overrides; + const source = { _flatten: () => ({}), requestIsStopped: () => {}, + getField: () => 'indexPattern', + ...overrideSource }; const errorHandler = () => {}; - const searchRequest = new SearchRequest({ source, errorHandler, ...overrides }); + const searchRequest = new SearchRequest({ source, errorHandler, ...rest }); searchRequest.__testId__ = id; return searchRequest; }; @@ -145,8 +149,11 @@ describe('callClient', () => { searchRequest.handleFailure = handleFailureSpy; searchRequests = [ searchRequest ]; - await callClient(searchRequests); - sinon.assert.calledWith(handleFailureSpy, 'fake es error'); + try { + await callClient(searchRequests); + } catch(e) { + sinon.assert.calledWith(handleFailureSpy, 'fake es error'); + } }); }); @@ -227,7 +234,7 @@ describe('callClient', () => { }); }); - it(`aborting all searchRequests calls abort() on the promise returned by es.msearch()`, () => { + it(`aborting all searchRequests calls abort() on the promise returned by searchStrategy.search()`, () => { esRequestDelay = 100; const searchRequest1 = createSearchRequest(); diff --git a/src/ui/public/courier/fetch/call_client.js b/src/ui/public/courier/fetch/call_client.js index 55df5959311294..4f2efcfc34995d 100644 --- a/src/ui/public/courier/fetch/call_client.js +++ b/src/ui/public/courier/fetch/call_client.js @@ -17,8 +17,6 @@ * under the License. */ -import _ from 'lodash'; - import { ErrorAllowExplicitIndexProvider } from '../../error_allow_explicit_index'; import { assignSearchRequestsToSearchStrategies } from '../search_strategy'; import { IsRequestProvider } from './is_request'; @@ -53,8 +51,12 @@ export function CallClientProvider(Private, Promise, es) { const searchStrategiesWithRequests = assignSearchRequestsToSearchStrategies(requestsToFetch); // resolved by respondToSearchRequests() - let searchRequestPromises = undefined; - let isRequestAborted = false; + const searchRequestPromises = []; + 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 = []; // for each respond with either the response or ABORTED const respondToSearchRequests = (responses = []) => { @@ -62,15 +64,7 @@ export function CallClientProvider(Private, Promise, es) { return allSearchRequests.concat(searchRequests); }, []); - const activeSearchRequests = aggregatedSearchRequests.filter(request => { - // We'll use the index of the request to map it to its response. If a request has already - // failed then it won't generate a response. In this case we need to remove the request - // to maintain cardinality between the list of requests and the list of corresponding - // responses. - return request !== undefined; - }); - - return Promise.map(activeSearchRequests, function (searchRequest, searchRequestIndex) { + return Promise.map(aggregatedSearchRequests, function (searchRequest, searchRequestIndex) { if (searchRequest.aborted) { return ABORTED; } @@ -85,14 +79,14 @@ export function CallClientProvider(Private, Promise, es) { return searchRequest._uniq.resp; } - const index = _.findIndex(activeSearchRequests, searchRequest); + const activeSearchRequestIndex = activeSearchRequests.indexOf(searchRequest); + const isFailedSearchRequest = activeSearchRequestIndex === -1; - if (index < 0) { - // This means the searchRequest failed. + if (isFailedSearchRequest) { return ABORTED; } - return responses[index]; + return responses[activeSearchRequestIndex]; }) .then( (res) => defer.resolve(res), @@ -114,11 +108,12 @@ export function CallClientProvider(Private, Promise, es) { } if (searchRequestPromises) { - searchRequestPromises.forEach(searchRequestPromise => searchRequestPromise.abort()); + searchRequestPromises.forEach(searchRequestPromise => { + searchRequestPromise.abort(); + }); } - searchRequestPromises = undefined; - isRequestAborted = true; + areAllSearchRequestsAborted = true; return respondToSearchRequests(); }); @@ -135,14 +130,28 @@ export function CallClientProvider(Private, Promise, es) { // asynchronously after we've returned a reference to defer.promise. Promise.resolve().then(async () => { // Execute each request using its search strategy. - searchRequestPromises = searchStrategiesWithRequests.map(searchStrategyWithSearchRequests => { + for (let i = 0; i < searchStrategiesWithRequests.length; i++) { + const searchStrategyWithSearchRequests = searchStrategiesWithRequests[i]; const { searchStrategy, searchRequests } = searchStrategyWithSearchRequests; - return searchStrategy.search({ searchRequests, es, Promise, serializeFetchParams }); - }); + const { + searching, + failedSearchRequests, + } = await searchStrategy.search({ searchRequests, es, Promise, serializeFetchParams }); + + searchRequests.forEach(searchRequest => { + if (failedSearchRequests.includes(searchRequest)) { + return; + } + + activeSearchRequests.push(searchRequest); + }); + + searchRequestPromises.push(searching); + } try { // The request was aborted while we were doing the above logic. - if (isRequestAborted) { + if (areAllSearchRequestsAborted) { return await respondToSearchRequests(); } diff --git a/src/ui/public/courier/search_strategy/default_search_strategy.js b/src/ui/public/courier/search_strategy/default_search_strategy.js index 32af5168ff0ef8..d0094695350300 100644 --- a/src/ui/public/courier/search_strategy/default_search_strategy.js +++ b/src/ui/public/courier/search_strategy/default_search_strategy.js @@ -31,20 +31,26 @@ function getAllFetchParams(searchRequests, Promise) { } function serializeAllFetchParams(fetchParams, searchRequests, serializeFetchParams) { - const requestsWithFetchParams = []; + const searcRequestsWithFetchParams = []; + const failedSearchRequests = []; // Gather the fetch param responses from all the successful requests. fetchParams.forEach((result, index) => { if (result.resolved) { - requestsWithFetchParams.push(result.resolved); + searcRequestsWithFetchParams.push(result.resolved); } else { - const request = searchRequests[index]; - request.handleFailure(result.rejected); - searchRequests[index] = undefined; + const searchRequest = searchRequests[index]; + + // TODO: All strategies will need to implement this. + searchRequest.handleFailure(result.rejected); + failedSearchRequests.push(searchRequest); } }); - return serializeFetchParams(requestsWithFetchParams); + return { + serializeAllFetchParams: serializeFetchParams(searcRequestsWithFetchParams), + failedSearchRequests, + }; } export const defaultSearchStrategy = { @@ -56,9 +62,15 @@ export const defaultSearchStrategy = { const allFetchParams = await getAllFetchParams(searchRequests, Promise); // Serialize the fetch params into a format suitable for the body of an ES query. - const serializedFetchParams = await serializeAllFetchParams(allFetchParams, searchRequests, serializeFetchParams); + const { + serializedFetchParams, + failedSearchRequests, + } = await serializeAllFetchParams(allFetchParams, searchRequests, serializeFetchParams); - return es.msearch({ body: serializedFetchParams }); + return { + searching: es.msearch({ body: serializedFetchParams }), + failedSearchRequests, + }; }, // Accept multiple criteria for determining viability to be as flexible as possible. From 6b97760ebea578f783ee6ea14c5adab10a878d4c Mon Sep 17 00:00:00 2001 From: CJ Cenizal Date: Thu, 12 Jul 2018 17:36:29 -0700 Subject: [PATCH 14/28] Fix bug caused by typo in defaultSearchStrategy. --- .../public/courier/search_strategy/default_search_strategy.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/ui/public/courier/search_strategy/default_search_strategy.js b/src/ui/public/courier/search_strategy/default_search_strategy.js index d0094695350300..2d7e3691927359 100644 --- a/src/ui/public/courier/search_strategy/default_search_strategy.js +++ b/src/ui/public/courier/search_strategy/default_search_strategy.js @@ -30,7 +30,7 @@ function getAllFetchParams(searchRequests, Promise) { }); } -function serializeAllFetchParams(fetchParams, searchRequests, serializeFetchParams) { +async function serializeAllFetchParams(fetchParams, searchRequests, serializeFetchParams) { const searcRequestsWithFetchParams = []; const failedSearchRequests = []; @@ -48,7 +48,7 @@ function serializeAllFetchParams(fetchParams, searchRequests, serializeFetchPara }); return { - serializeAllFetchParams: serializeFetchParams(searcRequestsWithFetchParams), + serializedFetchParams: await serializeFetchParams(searcRequestsWithFetchParams), failedSearchRequests, }; } From 3964b3af070bcb518db86e73a4d12a4adacfcd38 Mon Sep 17 00:00:00 2001 From: CJ Cenizal Date: Thu, 12 Jul 2018 17:41:28 -0700 Subject: [PATCH 15/28] Improve appearance of Discover when an unsupported rollup index pattern is being searched. Sentence-case copy in field chooser. --- .../field_chooser/field_chooser.html | 14 +++--- .../public/discover/directives/index.js | 6 +++ .../discover/directives/unsupported_rollup.js | 43 +++++++++++++++++++ .../kibana/public/discover/index.html | 7 +-- 4 files changed, 59 insertions(+), 11 deletions(-) create mode 100644 src/core_plugins/kibana/public/discover/directives/unsupported_rollup.js diff --git a/src/core_plugins/kibana/public/discover/components/field_chooser/field_chooser.html b/src/core_plugins/kibana/public/discover/components/field_chooser/field_chooser.html index 010cad19d1a969..3820555d7b7000 100644 --- a/src/core_plugins/kibana/public/discover/components/field_chooser/field_chooser.html +++ b/src/core_plugins/kibana/public/discover/components/field_chooser/field_chooser.html @@ -31,8 +31,10 @@
-
- +
+
    Selected
-
+
@@ -128,7 +130,7 @@
diff --git a/src/core_plugins/kibana/public/discover/directives/index.js b/src/core_plugins/kibana/public/discover/directives/index.js index 6f1447fa672d38..29d37010a55ad1 100644 --- a/src/core_plugins/kibana/public/discover/directives/index.js +++ b/src/core_plugins/kibana/public/discover/directives/index.js @@ -25,8 +25,14 @@ import { DiscoverNoResults, } from './no_results'; +import { + DiscoverUnsupportedRollup, +} from './unsupported_rollup'; + import './timechart'; const app = uiModules.get('apps/discover', ['react']); app.directive('discoverNoResults', reactDirective => reactDirective(DiscoverNoResults)); + +app.directive('discoverUnsupportedRollup', reactDirective => reactDirective(DiscoverUnsupportedRollup)); diff --git a/src/core_plugins/kibana/public/discover/directives/unsupported_rollup.js b/src/core_plugins/kibana/public/discover/directives/unsupported_rollup.js new file mode 100644 index 00000000000000..d33728ad8babec --- /dev/null +++ b/src/core_plugins/kibana/public/discover/directives/unsupported_rollup.js @@ -0,0 +1,43 @@ +/* + * 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, { Fragment } from 'react'; + +import { + EuiCallOut, + EuiFlexGroup, + EuiFlexItem, + EuiSpacer, +} from '@elastic/eui'; + +export const DiscoverUnsupportedRollup = () => ( + + + + + + + + + +); diff --git a/src/core_plugins/kibana/public/discover/index.html b/src/core_plugins/kibana/public/discover/index.html index 78955a8c9de94b..9e4e88bd9f63f3 100644 --- a/src/core_plugins/kibana/public/discover/index.html +++ b/src/core_plugins/kibana/public/discover/index.html @@ -60,12 +60,9 @@

- + > Date: Fri, 13 Jul 2018 12:17:45 -0700 Subject: [PATCH 16/28] Remove unnecessary if. --- src/ui/public/courier/fetch/call_client.js | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/ui/public/courier/fetch/call_client.js b/src/ui/public/courier/fetch/call_client.js index 4f2efcfc34995d..35267af7ff16de 100644 --- a/src/ui/public/courier/fetch/call_client.js +++ b/src/ui/public/courier/fetch/call_client.js @@ -107,11 +107,9 @@ export function CallClientProvider(Private, Promise, es) { return; } - if (searchRequestPromises) { - searchRequestPromises.forEach(searchRequestPromise => { - searchRequestPromise.abort(); - }); - } + searchRequestPromises.forEach(searchRequestPromise => { + searchRequestPromise.abort(); + }); areAllSearchRequestsAborted = true; From dcddc1c929c79ebd7eaeb37cd95d48614b40e017 Mon Sep 17 00:00:00 2001 From: CJ Cenizal Date: Fri, 13 Jul 2018 12:55:26 -0700 Subject: [PATCH 17/28] Return separate searching and abort properties from strategy.search(). --- src/ui/public/courier/fetch/call_client.js | 14 +++++++++----- .../search_strategy/default_search_strategy.js | 6 +++++- 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/src/ui/public/courier/fetch/call_client.js b/src/ui/public/courier/fetch/call_client.js index 35267af7ff16de..32ab92abb0dbbb 100644 --- a/src/ui/public/courier/fetch/call_client.js +++ b/src/ui/public/courier/fetch/call_client.js @@ -51,7 +51,7 @@ export function CallClientProvider(Private, Promise, es) { const searchStrategiesWithRequests = assignSearchRequestsToSearchStrategies(requestsToFetch); // resolved by respondToSearchRequests() - const searchRequestPromises = []; + const abortableSearches = []; let areAllSearchRequestsAborted = false; // When we traverse our search requests and send out searches, some of them may fail. We'll @@ -107,8 +107,8 @@ export function CallClientProvider(Private, Promise, es) { return; } - searchRequestPromises.forEach(searchRequestPromise => { - searchRequestPromise.abort(); + abortableSearches.forEach(({ abort }) => { + abort(); }); areAllSearchRequestsAborted = true; @@ -133,6 +133,7 @@ export function CallClientProvider(Private, Promise, es) { const { searchStrategy, searchRequests } = searchStrategyWithSearchRequests; const { searching, + abort, failedSearchRequests, } = await searchStrategy.search({ searchRequests, es, Promise, serializeFetchParams }); @@ -144,7 +145,10 @@ export function CallClientProvider(Private, Promise, es) { activeSearchRequests.push(searchRequest); }); - searchRequestPromises.push(searching); + abortableSearches.push({ + searching, + abort, + }); } try { @@ -153,7 +157,7 @@ export function CallClientProvider(Private, Promise, es) { return await respondToSearchRequests(); } - const segregatedResponses = await Promise.all(searchRequestPromises); + const segregatedResponses = await Promise.all(abortableSearches.map(({ searching }) => searching)); // Aggregate the responses returned by all of the search strategies. const aggregatedResponses = segregatedResponses.reduce((allResponses, responses) => { diff --git a/src/ui/public/courier/search_strategy/default_search_strategy.js b/src/ui/public/courier/search_strategy/default_search_strategy.js index 2d7e3691927359..7236aac1d890ff 100644 --- a/src/ui/public/courier/search_strategy/default_search_strategy.js +++ b/src/ui/public/courier/search_strategy/default_search_strategy.js @@ -67,8 +67,12 @@ export const defaultSearchStrategy = { failedSearchRequests, } = await serializeAllFetchParams(allFetchParams, searchRequests, serializeFetchParams); + const searching = es.msearch({ body: serializedFetchParams }); + const abort = searching.abort; + return { - searching: es.msearch({ body: serializedFetchParams }), + searching, + abort, failedSearchRequests, }; }, From 00a8fe5f21a9f813a9bdb0a63453a637879df145 Mon Sep 17 00:00:00 2001 From: CJ Cenizal Date: Fri, 13 Jul 2018 13:02:17 -0700 Subject: [PATCH 18/28] Simplify isViable to only expect an indexPattern. --- .../courier/search_strategy/default_search_strategy.js | 8 +++----- .../search_strategy/search_strategy_registry.js | 10 ++++++---- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/ui/public/courier/search_strategy/default_search_strategy.js b/src/ui/public/courier/search_strategy/default_search_strategy.js index 7236aac1d890ff..f03ad7da9ff62f 100644 --- a/src/ui/public/courier/search_strategy/default_search_strategy.js +++ b/src/ui/public/courier/search_strategy/default_search_strategy.js @@ -78,15 +78,13 @@ export const defaultSearchStrategy = { }, // Accept multiple criteria for determining viability to be as flexible as possible. - isViable: ({ indexPattern, searchRequest }) => { - if (!indexPattern && !searchRequest) { + isViable: (indexPattern) => { + if (!indexPattern) { return false; } - const derivedIndexPattern = indexPattern || searchRequest.source.getField('index'); - // Basic index patterns don't have `type` defined. - return derivedIndexPattern.type == null; + return indexPattern.type == null; }, }; diff --git a/src/ui/public/courier/search_strategy/search_strategy_registry.js b/src/ui/public/courier/search_strategy/search_strategy_registry.js index 1ea7a1fa1e0d2e..8b121598d3adbe 100644 --- a/src/ui/public/courier/search_strategy/search_strategy_registry.js +++ b/src/ui/public/courier/search_strategy/search_strategy_registry.js @@ -23,9 +23,9 @@ const addSearchStrategy = searchStrategy => { searchStrategies.push(searchStrategy); }; -const getSearchStrategy = (...rest) => { +const getSearchStrategy = indexPattern => { return searchStrategies.find(searchStrategy => { - return searchStrategy.isViable(...rest); + return searchStrategy.isViable(indexPattern); }); }; @@ -48,7 +48,9 @@ const assignSearchRequestsToSearchStrategies = searchRequests => { const searchStrategyById = {}; searchRequests.forEach(searchRequest => { - const matchingSearchStrategy = getSearchStrategy({ searchRequest }); + + const indexPattern = searchRequest.source.getField('index'); + const matchingSearchStrategy = getSearchStrategy(indexPattern); const { id } = matchingSearchStrategy; let searchStrategyWithRequest = searchStrategyById[id]; @@ -71,7 +73,7 @@ const assignSearchRequestsToSearchStrategies = searchRequests => { }; const hasSearchStategyForIndexPattern = indexPattern => { - return Boolean(getSearchStrategy({ indexPattern })); + return Boolean(getSearchStrategy(indexPattern)); }; const isRollupIndexPattern = indexPattern => { From ae033f2b508c370ae50df1f306b9f357cfbf3838 Mon Sep 17 00:00:00 2001 From: CJ Cenizal Date: Fri, 13 Jul 2018 13:05:21 -0700 Subject: [PATCH 19/28] Remove duplicate call to respondToSearchRequests(). --- src/ui/public/courier/fetch/call_client.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/ui/public/courier/fetch/call_client.js b/src/ui/public/courier/fetch/call_client.js index 32ab92abb0dbbb..2b96822a9a85dd 100644 --- a/src/ui/public/courier/fetch/call_client.js +++ b/src/ui/public/courier/fetch/call_client.js @@ -137,6 +137,7 @@ export function CallClientProvider(Private, Promise, es) { failedSearchRequests, } = await searchStrategy.search({ searchRequests, es, Promise, serializeFetchParams }); + // Collect searchRequests which have successfully been sent. searchRequests.forEach(searchRequest => { if (failedSearchRequests.includes(searchRequest)) { return; @@ -154,7 +155,7 @@ export function CallClientProvider(Private, Promise, es) { try { // The request was aborted while we were doing the above logic. if (areAllSearchRequestsAborted) { - return await respondToSearchRequests(); + return; } const segregatedResponses = await Promise.all(abortableSearches.map(({ searching }) => searching)); From 5bd4b012632385dacace7a63dcc32ecec2df226c Mon Sep 17 00:00:00 2001 From: CJ Cenizal Date: Fri, 13 Jul 2018 14:44:36 -0700 Subject: [PATCH 20/28] Send responses in original order to respondToSearchRequests. --- src/ui/public/courier/fetch/call_client.js | 58 ++++++++++++---------- 1 file changed, 32 insertions(+), 26 deletions(-) diff --git a/src/ui/public/courier/fetch/call_client.js b/src/ui/public/courier/fetch/call_client.js index 2b96822a9a85dd..3035fab1a04421 100644 --- a/src/ui/public/courier/fetch/call_client.js +++ b/src/ui/public/courier/fetch/call_client.js @@ -35,22 +35,20 @@ export function CallClientProvider(Private, Promise, es) { function callClient(searchRequests) { // merging docs can change status to DUPLICATE, capture new statuses - const statuses = mergeDuplicateRequests(searchRequests); + const searchRequestsAndStatuses = mergeDuplicateRequests(searchRequests); // get the actual list of requests that we will be fetching - const requestsToFetch = statuses.filter(isRequest); + const requestsToFetch = searchRequestsAndStatuses.filter(isRequest); let requestsToFetchCount = requestsToFetch.length; if (requestsToFetchCount === 0) { return Promise.resolve([]); } - // This is how we'll provide the consumer with search responses. + // This is how we'll provide the consumer with search responses. Resolved by + // respondToSearchRequests. const defer = Promise.defer(); - const searchStrategiesWithRequests = assignSearchRequestsToSearchStrategies(requestsToFetch); - - // resolved by respondToSearchRequests() const abortableSearches = []; let areAllSearchRequestsAborted = false; @@ -58,18 +56,14 @@ export function CallClientProvider(Private, Promise, es) { // store those that don't fail here. const activeSearchRequests = []; - // for each respond with either the response or ABORTED - const respondToSearchRequests = (responses = []) => { - const aggregatedSearchRequests = searchStrategiesWithRequests.reduce((allSearchRequests, { searchRequests }) => { - return allSearchRequests.concat(searchRequests); - }, []); - - return Promise.map(aggregatedSearchRequests, function (searchRequest, searchRequestIndex) { + // Respond to each searchRequest with the response or ABORTED. + const respondToSearchRequests = (responsesInOriginalRequestOrder = []) => { + return Promise.map(searchRequests, function (searchRequest, searchRequestIndex) { if (searchRequest.aborted) { return ABORTED; } - const status = statuses[searchRequestIndex]; + const status = searchRequestsAndStatuses[searchRequestIndex]; if (status === ABORTED) { return ABORTED; @@ -86,7 +80,7 @@ export function CallClientProvider(Private, Promise, es) { return ABORTED; } - return responses[activeSearchRequestIndex]; + return responsesInOriginalRequestOrder[activeSearchRequestIndex]; }) .then( (res) => defer.resolve(res), @@ -96,7 +90,7 @@ export function CallClientProvider(Private, Promise, es) { // handle a request being aborted while being fetched const requestWasAborted = Promise.method(function (searchRequest, index) { - if (statuses[index] === ABORTED) { + if (searchRequestsAndStatuses[index] === ABORTED) { defer.reject(new Error('Request was aborted twice?')); } @@ -117,13 +111,18 @@ export function CallClientProvider(Private, Promise, es) { }); // attach abort handlers, close over request index - statuses.forEach(function (req, i) { - if (!isRequest(req)) return; - req.whenAborted(function () { - requestWasAborted(req, i).catch(defer.reject); + searchRequestsAndStatuses.forEach(function (searchRequest, index) { + if (!isRequest(searchRequest)) { + return; + } + + searchRequest.whenAborted(function () { + requestWasAborted(searchRequest, index).catch(defer.reject); }); }); + 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 () => { @@ -160,12 +159,19 @@ export function CallClientProvider(Private, Promise, es) { const segregatedResponses = await Promise.all(abortableSearches.map(({ searching }) => searching)); - // Aggregate the responses returned by all of the search strategies. - const aggregatedResponses = segregatedResponses.reduce((allResponses, responses) => { - return allResponses.concat(responses.responses); - }, []); + // 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(requestsToFetch.length); + segregatedResponses.forEach((responses, strategyIndex) => { + responses.responses.forEach((response, responseIndex) => { + const searchRequest = searchStrategiesWithRequests[strategyIndex].searchRequests[responseIndex]; + const requestIndex = requestsToFetch.indexOf(searchRequest); + responsesInOriginalRequestOrder[requestIndex] = response; + }); + }); - await respondToSearchRequests(aggregatedResponses); + await respondToSearchRequests(responsesInOriginalRequestOrder); } catch(error) { if (errorAllowExplicitIndex.test(error)) { return errorAllowExplicitIndex.takeover(); @@ -178,7 +184,7 @@ export function CallClientProvider(Private, Promise, es) { // If there are any errors, notify the searchRequests of them. defer.promise.catch((err) => { searchRequests.forEach((searchRequest, index) => { - if (statuses[index] !== ABORTED) { + if (searchRequestsAndStatuses[index] !== ABORTED) { searchRequest.handleFailure(err); } }); From d867a3ff5a6d3b1f1173715f3cc741402ab8b6fe Mon Sep 17 00:00:00 2001 From: CJ Cenizal Date: Fri, 13 Jul 2018 14:50:31 -0700 Subject: [PATCH 21/28] Fix Jest tests. --- .../search_strategy_registry.test.js | 50 +++++++++---------- 1 file changed, 23 insertions(+), 27 deletions(-) diff --git a/src/ui/public/courier/search_strategy/search_strategy_registry.test.js b/src/ui/public/courier/search_strategy/search_strategy_registry.test.js index d33d5b253984a7..69eca6c271b43e 100644 --- a/src/ui/public/courier/search_strategy/search_strategy_registry.test.js +++ b/src/ui/public/courier/search_strategy/search_strategy_registry.test.js @@ -27,8 +27,8 @@ describe('SearchStrategyRegistry', () => { test('associates search requests with valid search strategies', () => { const searchStrategyA = { id: 'a', - isViable: ({ searchRequest }) => { - return searchRequest.type === 'a'; + isViable: indexPattern => { + return indexPattern === 'a'; }, }; @@ -36,47 +36,43 @@ describe('SearchStrategyRegistry', () => { const searchStrategyB = { id: 'b', - isViable: ({ searchRequest }) => { - return searchRequest.type === 'b'; + isViable: indexPattern => { + return indexPattern === 'b'; }, }; addSearchStrategy(searchStrategyB); - const searchRequests = [{ + const searchRequest0 = { id: 0, - type: 'b', - }, { + source: { getField: () => 'b' }, + }; + + const searchRequest1 = { id: 1, - type: 'a', - }, { + source: { getField: () => 'a' }, + }; + + const searchRequest2 = { id: 2, - type: 'a', - }, { + source: { getField: () => 'a' }, + }; + + const searchRequest3 = { id: 3, - type: 'b', - }]; + source: { getField: () => 'b' }, + }; + + const searchRequests = [ searchRequest0, searchRequest1, searchRequest2, searchRequest3]; const searchStrategiesWithSearchRequests = assignSearchRequestsToSearchStrategies(searchRequests); expect(searchStrategiesWithSearchRequests).toEqual([{ searchStrategy: searchStrategyB, - searchRequests: [{ - id: 0, - type: 'b', - }, { - id: 3, - type: 'b', - }], + searchRequests: [ searchRequest0, searchRequest3 ], }, { searchStrategy: searchStrategyA, - searchRequests: [{ - id: 1, - type: 'a', - }, { - id: 2, - type: 'a', - }], + searchRequests: [ searchRequest1, searchRequest2 ], }]); }); }); From 714990cb79ebe83cad0c91bc87b7e77ace19fa17 Mon Sep 17 00:00:00 2001 From: CJ Cenizal Date: Fri, 13 Jul 2018 16:06:26 -0700 Subject: [PATCH 22/28] Add tests with multiple searchStrategies and fix errors in logic. --- .../courier/fetch/__tests__/call_client.js | 86 +++++++++++++++++++ src/ui/public/courier/fetch/call_client.js | 10 ++- .../search_strategy_registry.js | 4 + 3 files changed, 96 insertions(+), 4 deletions(-) diff --git a/src/ui/public/courier/fetch/__tests__/call_client.js b/src/ui/public/courier/fetch/__tests__/call_client.js index cdf7db72431bea..b1d72f92fd24fc 100644 --- a/src/ui/public/courier/fetch/__tests__/call_client.js +++ b/src/ui/public/courier/fetch/__tests__/call_client.js @@ -27,6 +27,7 @@ import { CallClientProvider } from '../call_client'; import { RequestStatus } from '../req_status'; import { SearchRequestProvider } from '../request'; import { MergeDuplicatesRequestProvider } from '../merge_duplicate_requests'; +import { addSearchStrategy } from '../../search_strategy'; describe('callClient', () => { NoDigestPromises.activateForSuite(); @@ -268,4 +269,89 @@ describe('callClient', () => { }); }); }); + + describe('searchRequests with multiple searchStrategies', () => { + const search = ({ searchRequests }) => { + return { + searching: Promise.resolve({ + responses: 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' }), + }, + }); + + searchRequestB = createSearchRequest('b', { + source: { + getField: () => ({ type: 'b' }), + }, + }); + + searchRequestA2 = createSearchRequest('a2', { + source: { + getField: () => ({ type: 'a' }), + }, + }); + }); + + it('map correctly 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('map correctly if once is aborted', () => { + // 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('map correctly if one is already aborted', () => { + 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/ui/public/courier/fetch/call_client.js b/src/ui/public/courier/fetch/call_client.js index 3035fab1a04421..db8f0382c7474c 100644 --- a/src/ui/public/courier/fetch/call_client.js +++ b/src/ui/public/courier/fetch/call_client.js @@ -58,7 +58,9 @@ export function CallClientProvider(Private, Promise, es) { // Respond to each searchRequest with the response or ABORTED. const respondToSearchRequests = (responsesInOriginalRequestOrder = []) => { - return Promise.map(searchRequests, function (searchRequest, searchRequestIndex) { + // We map over searchRequestsAndStatuses because if we were originally provided an ABORTED + // request then we'll return that value. + return Promise.map(searchRequestsAndStatuses, function (searchRequest, searchRequestIndex) { if (searchRequest.aborted) { return ABORTED; } @@ -80,7 +82,7 @@ export function CallClientProvider(Private, Promise, es) { return ABORTED; } - return responsesInOriginalRequestOrder[activeSearchRequestIndex]; + return responsesInOriginalRequestOrder[searchRequestIndex]; }) .then( (res) => defer.resolve(res), @@ -162,11 +164,11 @@ export function CallClientProvider(Private, Promise, es) { // 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(requestsToFetch.length); + const responsesInOriginalRequestOrder = new Array(searchRequestsAndStatuses.length); segregatedResponses.forEach((responses, strategyIndex) => { responses.responses.forEach((response, responseIndex) => { const searchRequest = searchStrategiesWithRequests[strategyIndex].searchRequests[responseIndex]; - const requestIndex = requestsToFetch.indexOf(searchRequest); + const requestIndex = searchRequestsAndStatuses.indexOf(searchRequest); responsesInOriginalRequestOrder[requestIndex] = response; }); }); diff --git a/src/ui/public/courier/search_strategy/search_strategy_registry.js b/src/ui/public/courier/search_strategy/search_strategy_registry.js index 8b121598d3adbe..286f08b6a1bf3c 100644 --- a/src/ui/public/courier/search_strategy/search_strategy_registry.js +++ b/src/ui/public/courier/search_strategy/search_strategy_registry.js @@ -20,6 +20,10 @@ const searchStrategies = []; const addSearchStrategy = searchStrategy => { + if (searchStrategies.includes(searchStrategy)) { + return; + } + searchStrategies.push(searchStrategy); }; From 5ad6a55c97dc12878df95e469b7d707e9ec53796 Mon Sep 17 00:00:00 2001 From: CJ Cenizal Date: Fri, 13 Jul 2018 16:09:49 -0700 Subject: [PATCH 23/28] Rename describe block. --- src/ui/public/courier/fetch/__tests__/call_client.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/ui/public/courier/fetch/__tests__/call_client.js b/src/ui/public/courier/fetch/__tests__/call_client.js index b1d72f92fd24fc..0fb61c3228a459 100644 --- a/src/ui/public/courier/fetch/__tests__/call_client.js +++ b/src/ui/public/courier/fetch/__tests__/call_client.js @@ -270,7 +270,7 @@ describe('callClient', () => { }); }); - describe('searchRequests with multiple searchStrategies', () => { + describe('searchRequests with multiple searchStrategies map correctly to their responses', () => { const search = ({ searchRequests }) => { return { searching: Promise.resolve({ @@ -324,7 +324,7 @@ describe('callClient', () => { }); }); - it('map correctly if the searchRequests are reordered by the searchStrategies', () => { + 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); @@ -334,7 +334,7 @@ describe('callClient', () => { }); }); - it('map correctly if once is aborted', () => { + it('if once is aborted', () => { // Add requests in an order which will be reordered by the strategies. searchRequests = [ searchRequestA, searchRequestB, searchRequestA2 ]; const callingClient = callClient(searchRequests); @@ -345,7 +345,7 @@ describe('callClient', () => { }); }); - it('map correctly if one is already aborted', () => { + it('if one is already aborted', () => { searchRequests = [ searchRequestA, searchRequestB, ABORTED, searchRequestA2 ]; const callingClient = callClient(searchRequests); From 464b568b56c92cdfb2f07eacb88c1fdd8c4a47d6 Mon Sep 17 00:00:00 2001 From: CJ Cenizal Date: Fri, 13 Jul 2018 16:10:40 -0700 Subject: [PATCH 24/28] Rename describe block. --- src/ui/public/courier/fetch/__tests__/call_client.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/ui/public/courier/fetch/__tests__/call_client.js b/src/ui/public/courier/fetch/__tests__/call_client.js index 0fb61c3228a459..0d63ce5b9107bf 100644 --- a/src/ui/public/courier/fetch/__tests__/call_client.js +++ b/src/ui/public/courier/fetch/__tests__/call_client.js @@ -334,7 +334,7 @@ describe('callClient', () => { }); }); - it('if once is aborted', () => { + 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); @@ -345,7 +345,7 @@ describe('callClient', () => { }); }); - it('if one is already aborted', () => { + it(`if one is already aborted when it's provided`, () => { searchRequests = [ searchRequestA, searchRequestB, ABORTED, searchRequestA2 ]; const callingClient = callClient(searchRequests); From 4aba1fa9276a2f21941bb809f5137ff4df431cf2 Mon Sep 17 00:00:00 2001 From: CJ Cenizal Date: Mon, 16 Jul 2018 15:33:19 -0700 Subject: [PATCH 25/28] Check for default type index pattern instead of rollup type. --- .../public/discover/controllers/discover.js | 9 ++-- .../public/discover/directives/index.js | 9 ++-- .../directives/unsupported_index_pattern.js | 50 +++++++++++++++++++ .../kibana/public/discover/index.html | 7 +-- src/ui/public/courier/index.js | 2 +- .../default_search_strategy.js | 4 +- .../public/courier/search_strategy/index.js | 3 +- .../is_default_type_index_pattern.js} | 28 ++--------- .../search_strategy_registry.js | 5 -- 9 files changed, 74 insertions(+), 43 deletions(-) create mode 100644 src/core_plugins/kibana/public/discover/directives/unsupported_index_pattern.js rename src/{core_plugins/kibana/public/discover/directives/unsupported_rollup.js => ui/public/courier/search_strategy/is_default_type_index_pattern.js} (57%) diff --git a/src/core_plugins/kibana/public/discover/controllers/discover.js b/src/core_plugins/kibana/public/discover/controllers/discover.js index ee0c880ef50211..068ed4e4ce8fb2 100644 --- a/src/core_plugins/kibana/public/discover/controllers/discover.js +++ b/src/core_plugins/kibana/public/discover/controllers/discover.js @@ -33,7 +33,7 @@ import 'ui/state_management/app_state'; import { timefilter } from 'ui/timefilter'; import 'ui/share'; import 'ui/query_bar'; -import { hasSearchStategyForIndexPattern, isRollupIndexPattern } from 'ui/courier'; +import { hasSearchStategyForIndexPattern, isDefaultTypeIndexPattern } from 'ui/courier'; import { toastNotifications, getPainlessError } from 'ui/notify'; import { VisProvider } from 'ui/vis'; import { BasicResponseHandlerProvider } from 'ui/vis/response_handlers/basic'; @@ -779,12 +779,13 @@ function discoverController( // Block the UI from loading if the user has loaded a rollup index pattern but it isn't // supported. - $scope.isUnsupportedRollup = ( - isRollupIndexPattern($route.current.locals.ip.loaded) + $scope.isUnsupportedIndexPattern = ( + !isDefaultTypeIndexPattern($route.current.locals.ip.loaded) && !hasSearchStategyForIndexPattern($route.current.locals.ip.loaded) ); - if ($scope.isUnsupportedRollup) { + if ($scope.isUnsupportedIndexPattern) { + $scope.unsupportedIndexPatternType = $route.current.locals.ip.loaded.type; return; } diff --git a/src/core_plugins/kibana/public/discover/directives/index.js b/src/core_plugins/kibana/public/discover/directives/index.js index 29d37010a55ad1..a6fc851329340f 100644 --- a/src/core_plugins/kibana/public/discover/directives/index.js +++ b/src/core_plugins/kibana/public/discover/directives/index.js @@ -26,8 +26,8 @@ import { } from './no_results'; import { - DiscoverUnsupportedRollup, -} from './unsupported_rollup'; + DiscoverUnsupportedIndexPattern, +} from './unsupported_index_pattern'; import './timechart'; @@ -35,4 +35,7 @@ const app = uiModules.get('apps/discover', ['react']); app.directive('discoverNoResults', reactDirective => reactDirective(DiscoverNoResults)); -app.directive('discoverUnsupportedRollup', reactDirective => reactDirective(DiscoverUnsupportedRollup)); +app.directive( + 'discoverUnsupportedIndexPattern', + reactDirective => reactDirective(DiscoverUnsupportedIndexPattern, ['unsupportedType']) +); diff --git a/src/core_plugins/kibana/public/discover/directives/unsupported_index_pattern.js b/src/core_plugins/kibana/public/discover/directives/unsupported_index_pattern.js new file mode 100644 index 00000000000000..b1be10781b2b8a --- /dev/null +++ b/src/core_plugins/kibana/public/discover/directives/unsupported_index_pattern.js @@ -0,0 +1,50 @@ +/* + * 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, { Fragment } from 'react'; + +import { + EuiCallOut, + EuiFlexGroup, + EuiFlexItem, + EuiSpacer, +} from '@elastic/eui'; + +export const DiscoverUnsupportedIndexPattern = ({ unsupportedType }) => { + // This message makes the assumption that X-Pack will support this type, as is the case with + // rollup index patterns. + const message = `Index patterns based on ${unsupportedType} indices require the` + + ` ${unsupportedType} plugin from X-Pack which is either not installed or disabled`; + + return ( + + + + + + + + + + ); +}; diff --git a/src/core_plugins/kibana/public/discover/index.html b/src/core_plugins/kibana/public/discover/index.html index 9e4e88bd9f63f3..ee88da2c1b3b3c 100644 --- a/src/core_plugins/kibana/public/discover/index.html +++ b/src/core_plugins/kibana/public/discover/index.html @@ -60,9 +60,10 @@

- + { @@ -83,8 +84,7 @@ export const defaultSearchStrategy = { return false; } - // Basic index patterns don't have `type` defined. - return indexPattern.type == null; + return isDefaultTypeIndexPattern(indexPattern); }, }; diff --git a/src/ui/public/courier/search_strategy/index.js b/src/ui/public/courier/search_strategy/index.js index 2613b64df8778e..41991b63f950d6 100644 --- a/src/ui/public/courier/search_strategy/index.js +++ b/src/ui/public/courier/search_strategy/index.js @@ -21,5 +21,6 @@ export { assignSearchRequestsToSearchStrategies, addSearchStrategy, hasSearchStategyForIndexPattern, - isRollupIndexPattern, } from './search_strategy_registry'; + +export { isDefaultTypeIndexPattern } from './is_default_type_index_pattern'; diff --git a/src/core_plugins/kibana/public/discover/directives/unsupported_rollup.js b/src/ui/public/courier/search_strategy/is_default_type_index_pattern.js similarity index 57% rename from src/core_plugins/kibana/public/discover/directives/unsupported_rollup.js rename to src/ui/public/courier/search_strategy/is_default_type_index_pattern.js index d33728ad8babec..ec2a54239a880b 100644 --- a/src/core_plugins/kibana/public/discover/directives/unsupported_rollup.js +++ b/src/ui/public/courier/search_strategy/is_default_type_index_pattern.js @@ -17,27 +17,7 @@ * under the License. */ -import React, { Fragment } from 'react'; - -import { - EuiCallOut, - EuiFlexGroup, - EuiFlexItem, - EuiSpacer, -} from '@elastic/eui'; - -export const DiscoverUnsupportedRollup = () => ( - - - - - - - - - -); +export const isDefaultTypeIndexPattern = indexPattern => { + // Default index patterns don't have `type` defined. + return indexPattern.type == null; +}; diff --git a/src/ui/public/courier/search_strategy/search_strategy_registry.js b/src/ui/public/courier/search_strategy/search_strategy_registry.js index 286f08b6a1bf3c..a0341932565da9 100644 --- a/src/ui/public/courier/search_strategy/search_strategy_registry.js +++ b/src/ui/public/courier/search_strategy/search_strategy_registry.js @@ -80,13 +80,8 @@ const hasSearchStategyForIndexPattern = indexPattern => { return Boolean(getSearchStrategy(indexPattern)); }; -const isRollupIndexPattern = indexPattern => { - return indexPattern.type === 'rollup'; -}; - export { assignSearchRequestsToSearchStrategies, addSearchStrategy, hasSearchStategyForIndexPattern, - isRollupIndexPattern, }; From 4c51a51cff014f0c07f1234549295788b3251a95 Mon Sep 17 00:00:00 2001 From: CJ Cenizal Date: Mon, 16 Jul 2018 15:35:54 -0700 Subject: [PATCH 26/28] CallClient now expects search to resolve with responses array. --- src/ui/public/courier/fetch/call_client.js | 2 +- .../public/courier/search_strategy/default_search_strategy.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/ui/public/courier/fetch/call_client.js b/src/ui/public/courier/fetch/call_client.js index db8f0382c7474c..fd8266eb6e122a 100644 --- a/src/ui/public/courier/fetch/call_client.js +++ b/src/ui/public/courier/fetch/call_client.js @@ -166,7 +166,7 @@ export function CallClientProvider(Private, Promise, es) { // use the order to associate each response with the original request. const responsesInOriginalRequestOrder = new Array(searchRequestsAndStatuses.length); segregatedResponses.forEach((responses, strategyIndex) => { - responses.responses.forEach((response, responseIndex) => { + responses.forEach((response, responseIndex) => { const searchRequest = searchStrategiesWithRequests[strategyIndex].searchRequests[responseIndex]; const requestIndex = searchRequestsAndStatuses.indexOf(searchRequest); responsesInOriginalRequestOrder[requestIndex] = response; diff --git a/src/ui/public/courier/search_strategy/default_search_strategy.js b/src/ui/public/courier/search_strategy/default_search_strategy.js index 4ad4fc5f2003c8..164625c903d99c 100644 --- a/src/ui/public/courier/search_strategy/default_search_strategy.js +++ b/src/ui/public/courier/search_strategy/default_search_strategy.js @@ -68,7 +68,7 @@ export const defaultSearchStrategy = { failedSearchRequests, } = await serializeAllFetchParams(allFetchParams, searchRequests, serializeFetchParams); - const searching = es.msearch({ body: serializedFetchParams }); + const searching = es.msearch({ body: serializedFetchParams }).then(({ responses }) => responses); const abort = searching.abort; return { From 1c4e668a09c98d90fd25b08217b181ee191db0b5 Mon Sep 17 00:00:00 2001 From: CJ Cenizal Date: Mon, 16 Jul 2018 17:06:43 -0700 Subject: [PATCH 27/28] Fix tests. --- src/ui/public/courier/fetch/__tests__/call_client.js | 4 +--- .../courier/search_strategy/default_search_strategy.js | 8 ++++---- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/src/ui/public/courier/fetch/__tests__/call_client.js b/src/ui/public/courier/fetch/__tests__/call_client.js index 0d63ce5b9107bf..565af41f23ebfb 100644 --- a/src/ui/public/courier/fetch/__tests__/call_client.js +++ b/src/ui/public/courier/fetch/__tests__/call_client.js @@ -273,9 +273,7 @@ describe('callClient', () => { describe('searchRequests with multiple searchStrategies map correctly to their responses', () => { const search = ({ searchRequests }) => { return { - searching: Promise.resolve({ - responses: searchRequests.map(searchRequest => searchRequest.__testId__), - }), + searching: Promise.resolve(searchRequests.map(searchRequest => searchRequest.__testId__)), failedSearchRequests: [], abort: () => {}, }; diff --git a/src/ui/public/courier/search_strategy/default_search_strategy.js b/src/ui/public/courier/search_strategy/default_search_strategy.js index 164625c903d99c..05fc45c5c67d1c 100644 --- a/src/ui/public/courier/search_strategy/default_search_strategy.js +++ b/src/ui/public/courier/search_strategy/default_search_strategy.js @@ -68,12 +68,12 @@ export const defaultSearchStrategy = { failedSearchRequests, } = await serializeAllFetchParams(allFetchParams, searchRequests, serializeFetchParams); - const searching = es.msearch({ body: serializedFetchParams }).then(({ responses }) => responses); - const abort = searching.abort; + const searching = es.msearch({ body: serializedFetchParams }); return { - searching, - abort, + // Unwrap the responses object returned by the es client. + searching: searching.then(({ responses }) => responses), + abort: searching.abort, failedSearchRequests, }; }, From ff73efba0476c9d519f24f96d9bc3fc85023408a Mon Sep 17 00:00:00 2001 From: CJ Cenizal Date: Tue, 17 Jul 2018 16:11:01 -0700 Subject: [PATCH 28/28] Tweak copy. --- .../public/discover/directives/unsupported_index_pattern.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core_plugins/kibana/public/discover/directives/unsupported_index_pattern.js b/src/core_plugins/kibana/public/discover/directives/unsupported_index_pattern.js index b1be10781b2b8a..d5e143fc3bcd90 100644 --- a/src/core_plugins/kibana/public/discover/directives/unsupported_index_pattern.js +++ b/src/core_plugins/kibana/public/discover/directives/unsupported_index_pattern.js @@ -30,7 +30,7 @@ export const DiscoverUnsupportedIndexPattern = ({ unsupportedType }) => { // This message makes the assumption that X-Pack will support this type, as is the case with // rollup index patterns. const message = `Index patterns based on ${unsupportedType} indices require the` + - ` ${unsupportedType} plugin from X-Pack which is either not installed or disabled`; + ` ${unsupportedType} plugin from X-Pack, which is not installed or disabled`; return (