From 34a145ecb05db9b75d672936df6afc9a06067961 Mon Sep 17 00:00:00 2001 From: Liza Katz Date: Tue, 21 Jan 2020 18:49:57 +0200 Subject: [PATCH] Clean up search service (#53766) * deprecate msearch * Missing export * adjust tests, revert loading method of esaggs/boot * getInjectedMetadata * Fix jest tests * update default strategy abort test * notice update * Allow running discover errors test independently * Remove batchSearches * Detect painless script error * don't show notifications for aborted requests * Fix jest tests * Restore loader indicator * Decreace loading count on error * update search test * Trigger digest after fetching fresh index patterns * Revert isEqual * accurate revert * Return full error details to client from search endpoint * Re-throw AbortError from http when user aborts request. * fix typo * typo * Adjust routes jest test * Restore msearch using a separate es connection * typescript fixes * set http service mock * Move es client to dat aplugin, for follow up PR * Add karma mock * krma mock * fix tests * ts * Pass in version dynamically * add headers to esClient host * Restored fetch soon test Use tap for loadingCount side effects * Cleanup search params * Cleanup search params test * Revert "Cleanup search params" This reverts commit ca9dea01d59fddd10b43513450707421248b96d5. * Revert "Cleanup search params test" This reverts commit 30b94786122181f41ac44606899655ed85c0dc52. * Revert code to use old es client until #44302 is resolved * Revert changes to getPainlessError * Fix jest test * Refactor esClient to trigger loadingIndicator * fixing tests * use esClient from searchService * git remove comment * fix jest Co-authored-by: Elastic Machine --- src/core/public/http/fetch.ts | 6 +- src/legacy/core_plugins/data/public/plugin.ts | 19 +++- .../build_tabular_inspector_data.ts | 4 +- .../data/public/search/expressions/esaggs.ts | 10 +- .../public/search/fetch/call_client.test.ts | 2 +- .../data/public/search/fetch/call_client.ts | 6 +- .../data/public/search/fetch/fetch_soon.ts | 10 +- .../data/public/search/fetch/types.ts | 3 +- .../search_source/search_source.test.ts | 31 +++++-- .../search/search_source/search_source.ts | 34 ++++--- .../default_search_strategy.test.ts | 44 ++++++--- .../default_search_strategy.ts | 35 ++++++- .../np_ready/angular/get_painless_error.ts | 3 +- .../visualizations/public/legacy_imports.ts | 1 - .../components/visualization_requesterror.tsx | 2 +- .../public/np_ready/public/index.ts | 2 + .../new_platform/new_platform.karma_mock.js | 8 ++ src/plugins/data/public/mocks.ts | 17 +++- src/plugins/data/public/plugin.ts | 21 ++++- .../create_app_mount_context_search.test.ts | 50 +++++----- .../search/create_app_mount_context_search.ts | 15 ++- .../public/search/es_client/get_es_client.ts | 93 +++++++++++++++++++ .../data/public/search/es_client/index.ts | 20 ++++ .../data/public/search/search_service.ts | 22 ++++- src/plugins/data/public/services.ts | 19 +++- .../query_string_input.test.tsx.snap | 72 ++++++++++++++ src/plugins/data/server/search/routes.test.ts | 15 ++- src/plugins/data/server/search/routes.ts | 10 +- test/functional/apps/discover/_errors.js | 3 +- .../maps/public/angular/map_controller.js | 4 +- 30 files changed, 475 insertions(+), 106 deletions(-) rename src/legacy/{ui/public/inspector => core_plugins/data/public/search/expressions}/build_tabular_inspector_data.ts (95%) create mode 100644 src/plugins/data/public/search/es_client/get_es_client.ts create mode 100644 src/plugins/data/public/search/es_client/index.ts diff --git a/src/core/public/http/fetch.ts b/src/core/public/http/fetch.ts index b86f1f5c08029..b7ceaed6e56a7 100644 --- a/src/core/public/http/fetch.ts +++ b/src/core/public/http/fetch.ts @@ -133,7 +133,11 @@ export class Fetch { try { response = await window.fetch(request); } catch (err) { - throw new HttpFetchError(err.message, request); + if (err.name === 'AbortError') { + throw err; + } else { + throw new HttpFetchError(err.message, request); + } } const contentType = response.headers.get('Content-Type') || ''; diff --git a/src/legacy/core_plugins/data/public/plugin.ts b/src/legacy/core_plugins/data/public/plugin.ts index 893e477b38583..5329702348207 100644 --- a/src/legacy/core_plugins/data/public/plugin.ts +++ b/src/legacy/core_plugins/data/public/plugin.ts @@ -20,15 +20,24 @@ import { CoreSetup, CoreStart, Plugin } from 'kibana/public'; import { SearchService, SearchStart } from './search'; import { DataPublicPluginStart } from '../../../../plugins/data/public'; +import { ExpressionsSetup } from '../../../../plugins/expressions/public'; import { setFieldFormats, setNotifications, setIndexPatterns, setQueryService, + setSearchService, + setUiSettings, + setInjectedMetadata, + setHttp, // eslint-disable-next-line @kbn/eslint/no-restricted-paths } from '../../../../plugins/data/public/services'; +export interface DataPluginSetupDependencies { + expressions: ExpressionsSetup; +} + export interface DataPluginStartDependencies { data: DataPublicPluginStart; } @@ -54,18 +63,24 @@ export interface DataStart { * or static code. */ -export class DataPlugin implements Plugin { +export class DataPlugin + implements Plugin { private readonly search = new SearchService(); - public setup(core: CoreSetup) {} + public setup(core: CoreSetup) { + setInjectedMetadata(core.injectedMetadata); + } public start(core: CoreStart, { data }: DataPluginStartDependencies): DataStart { // This is required for when Angular code uses Field and FieldList. setFieldFormats(data.fieldFormats); setQueryService(data.query); + setSearchService(data.search); setIndexPatterns(data.indexPatterns); setFieldFormats(data.fieldFormats); setNotifications(core.notifications); + setUiSettings(core.uiSettings); + setHttp(core.http); return { search: this.search.start(core), diff --git a/src/legacy/ui/public/inspector/build_tabular_inspector_data.ts b/src/legacy/core_plugins/data/public/search/expressions/build_tabular_inspector_data.ts similarity index 95% rename from src/legacy/ui/public/inspector/build_tabular_inspector_data.ts rename to src/legacy/core_plugins/data/public/search/expressions/build_tabular_inspector_data.ts index b09ed60e7186f..6e6d2a15fa2ac 100644 --- a/src/legacy/ui/public/inspector/build_tabular_inspector_data.ts +++ b/src/legacy/core_plugins/data/public/search/expressions/build_tabular_inspector_data.ts @@ -19,8 +19,8 @@ import { set } from 'lodash'; // @ts-ignore -import { createFilter } from '../../../core_plugins/visualizations/public'; -import { FormattedData } from './adapters'; +import { createFilter } from '../../../../visualizations/public'; +import { FormattedData } from '../../../../../../plugins/inspector/public'; interface Column { id: string; diff --git a/src/legacy/core_plugins/data/public/search/expressions/esaggs.ts b/src/legacy/core_plugins/data/public/search/expressions/esaggs.ts index 4ec4dbd7f88d6..889c747c9a62e 100644 --- a/src/legacy/core_plugins/data/public/search/expressions/esaggs.ts +++ b/src/legacy/core_plugins/data/public/search/expressions/esaggs.ts @@ -34,14 +34,8 @@ import { getTime, FilterManager, } from '../../../../../../plugins/data/public'; -import { - SearchSource, - ISearchSource, - getRequestInspectorStats, - getResponseInspectorStats, -} from '../../../../../ui/public/courier'; -import { buildTabularInspectorData } from '../../../../../ui/public/inspector/build_tabular_inspector_data'; +import { buildTabularInspectorData } from './build_tabular_inspector_data'; import { calculateObjectHash } from '../../../../visualizations/public'; // @ts-ignore import { tabifyAggResponse } from '../../../../../ui/public/agg_response/tabify/tabify'; @@ -49,6 +43,8 @@ import { PersistedState } from '../../../../../ui/public/persisted_state'; import { Adapters } from '../../../../../../plugins/inspector/public'; // eslint-disable-next-line @kbn/eslint/no-restricted-paths import { getQueryService, getIndexPatterns } from '../../../../../../plugins/data/public/services'; +import { ISearchSource, getRequestInspectorStats, getResponseInspectorStats } from '../..'; +import { SearchSource } from '../search_source'; export interface RequestHandlerParams { searchSource: ISearchSource; diff --git a/src/legacy/core_plugins/data/public/search/fetch/call_client.test.ts b/src/legacy/core_plugins/data/public/search/fetch/call_client.test.ts index 74c87d77dd4fd..24a36c9db9df7 100644 --- a/src/legacy/core_plugins/data/public/search/fetch/call_client.test.ts +++ b/src/legacy/core_plugins/data/public/search/fetch/call_client.test.ts @@ -76,7 +76,7 @@ describe('callClient', () => { test('Passes the additional arguments it is given to the search strategy', () => { const searchRequests = [{ _searchStrategyId: 0 }]; - const args = { es: {}, config: {}, esShardTimeout: 0 } as FetchHandlers; + const args = { searchService: {}, config: {}, esShardTimeout: 0 } as FetchHandlers; callClient(searchRequests, [], args); diff --git a/src/legacy/core_plugins/data/public/search/fetch/call_client.ts b/src/legacy/core_plugins/data/public/search/fetch/call_client.ts index 43da27f941e4e..ad18775d5f144 100644 --- a/src/legacy/core_plugins/data/public/search/fetch/call_client.ts +++ b/src/legacy/core_plugins/data/public/search/fetch/call_client.ts @@ -26,7 +26,7 @@ import { SearchRequest } from '../types'; export function callClient( searchRequests: SearchRequest[], requestsOptions: FetchOptions[] = [], - { es, config, esShardTimeout }: FetchHandlers + fetchHandlers: FetchHandlers ) { // Correlate the options with the request that they're associated with const requestOptionEntries: Array<[ @@ -53,9 +53,7 @@ export function callClient( // then an error would have been thrown above const { searching, abort } = searchStrategy!.search({ searchRequests: requests, - es, - config, - esShardTimeout, + ...fetchHandlers, }); requests.forEach((request, i) => { diff --git a/src/legacy/core_plugins/data/public/search/fetch/fetch_soon.ts b/src/legacy/core_plugins/data/public/search/fetch/fetch_soon.ts index 75de85e02a1a2..4830464047ad6 100644 --- a/src/legacy/core_plugins/data/public/search/fetch/fetch_soon.ts +++ b/src/legacy/core_plugins/data/public/search/fetch/fetch_soon.ts @@ -28,10 +28,10 @@ import { SearchRequest, SearchResponse } from '../types'; export async function fetchSoon( request: SearchRequest, options: FetchOptions, - { es, config, esShardTimeout }: FetchHandlers + fetchHandlers: FetchHandlers ) { - const msToDelay = config.get('courier:batchSearches') ? 50 : 0; - return delayedFetch(request, options, { es, config, esShardTimeout }, msToDelay); + const msToDelay = fetchHandlers.config.get('courier:batchSearches') ? 50 : 0; + return delayedFetch(request, options, fetchHandlers, msToDelay); } /** @@ -64,7 +64,7 @@ let fetchInProgress: Promise | null = null; async function delayedFetch( request: SearchRequest, options: FetchOptions, - { es, config, esShardTimeout }: FetchHandlers, + fetchHandlers: FetchHandlers, ms: number ) { const i = requestsToFetch.length; @@ -73,7 +73,7 @@ async function delayedFetch( const responses = await (fetchInProgress = fetchInProgress || delay(() => { - const response = callClient(requestsToFetch, requestOptions, { es, config, esShardTimeout }); + const response = callClient(requestsToFetch, requestOptions, fetchHandlers); requestsToFetch = []; requestOptions = []; fetchInProgress = null; diff --git a/src/legacy/core_plugins/data/public/search/fetch/types.ts b/src/legacy/core_plugins/data/public/search/fetch/types.ts index 0887a1f84c7c8..fba14119d83c3 100644 --- a/src/legacy/core_plugins/data/public/search/fetch/types.ts +++ b/src/legacy/core_plugins/data/public/search/fetch/types.ts @@ -17,6 +17,7 @@ * under the License. */ +import { ISearchStart } from 'src/plugins/data/public'; import { IUiSettingsClient } from '../../../../../../core/public'; import { SearchRequest, SearchResponse } from '../types'; @@ -35,7 +36,7 @@ export interface FetchOptions { } export interface FetchHandlers { - es: ApiCaller; + searchService: ISearchStart; config: IUiSettingsClient; esShardTimeout: number; } diff --git a/src/legacy/core_plugins/data/public/search/search_source/search_source.test.ts b/src/legacy/core_plugins/data/public/search/search_source/search_source.test.ts index 28f8dba9a75de..ebeee60b67c8a 100644 --- a/src/legacy/core_plugins/data/public/search/search_source/search_source.test.ts +++ b/src/legacy/core_plugins/data/public/search/search_source/search_source.test.ts @@ -19,19 +19,34 @@ import { SearchSource } from '../search_source'; import { IndexPattern } from '../../../../../../plugins/data/public'; - -jest.mock('ui/new_platform'); +import { + setSearchService, + setUiSettings, + setInjectedMetadata, + // eslint-disable-next-line @kbn/eslint/no-restricted-paths +} from '../../../../../../plugins/data/public/services'; + +import { + injectedMetadataServiceMock, + uiSettingsServiceMock, +} from '../../../../../../core/public/mocks'; + +setUiSettings(uiSettingsServiceMock.createStartContract()); +setInjectedMetadata(injectedMetadataServiceMock.createSetupContract()); +setSearchService({ + search: jest.fn(), + __LEGACY: { + esClient: { + search: jest.fn(), + msearch: jest.fn(), + }, + }, +}); jest.mock('../fetch', () => ({ fetchSoon: jest.fn().mockResolvedValue({}), })); -jest.mock('ui/chrome', () => ({ - dangerouslyGetActiveInjector: () => ({ - get: jest.fn(), - }), -})); - const getComputedFields = () => ({ storedFields: [], scriptFields: [], diff --git a/src/legacy/core_plugins/data/public/search/search_source/search_source.ts b/src/legacy/core_plugins/data/public/search/search_source/search_source.ts index 6efcae4d4b88d..e977db713ebaa 100644 --- a/src/legacy/core_plugins/data/public/search/search_source/search_source.ts +++ b/src/legacy/core_plugins/data/public/search/search_source/search_source.ts @@ -70,8 +70,6 @@ */ import _ from 'lodash'; -import { npSetup } from 'ui/new_platform'; -import chrome from 'ui/chrome'; import { normalizeSortRequest } from './normalize_sort_request'; import { fetchSoon } from '../fetch'; import { fieldWildcardFilter } from '../../../../../../plugins/kibana_utils/public'; @@ -79,10 +77,14 @@ import { getHighlightRequest, esFilters, esQuery } from '../../../../../../plugi import { RequestFailure } from '../fetch/errors'; import { filterDocvalueFields } from './filter_docvalue_fields'; import { SearchSourceOptions, SearchSourceFields, SearchRequest } from './types'; -import { FetchOptions, ApiCaller } from '../fetch/types'; +import { FetchOptions } from '../fetch/types'; -const esShardTimeout = npSetup.core.injectedMetadata.getInjectedVar('esShardTimeout') as number; -const config = npSetup.core.uiSettings; +import { + getSearchService, + getUiSettings, + getInjectedMetadata, + // eslint-disable-next-line @kbn/eslint/no-restricted-paths +} from '../../../../../../plugins/data/public/services'; export type ISearchSource = Pick; @@ -192,21 +194,23 @@ export class SearchSource { * @async */ async fetch(options: FetchOptions = {}) { - const $injector = await chrome.dangerouslyGetActiveInjector(); - const es = $injector.get('es') as ApiCaller; - await this.requestIsStarting(options); const searchRequest = await this.flatten(); this.history = [searchRequest]; + const esShardTimeout = getInjectedMetadata().getInjectedVar('esShardTimeout') as number; const response = await fetchSoon( searchRequest, { ...(this.searchStrategyId && { searchStrategyId: this.searchStrategyId }), ...options, }, - { es, config, esShardTimeout } + { + searchService: getSearchService(), + config: getUiSettings(), + esShardTimeout, + } ); if (response.error) { @@ -313,7 +317,11 @@ export class SearchSource { case 'source': return addToBody('_source', val); case 'sort': - const sort = normalizeSortRequest(val, this.getField('index'), config.get('sort:options')); + const sort = normalizeSortRequest( + val, + this.getField('index'), + getUiSettings().get('sort:options') + ); return addToBody(key, sort); default: return addToBody(key, val); @@ -359,7 +367,7 @@ export class SearchSource { if (body._source) { // exclude source fields for this index pattern specified by the user - const filter = fieldWildcardFilter(body._source.excludes, config.get('metaFields')); + const filter = fieldWildcardFilter(body._source.excludes, getUiSettings().get('metaFields')); body.docvalue_fields = body.docvalue_fields.filter((docvalueField: any) => filter(docvalueField.field) ); @@ -377,11 +385,11 @@ export class SearchSource { _.set(body, '_source.includes', remainingFields); } - const esQueryConfigs = esQuery.getEsQueryConfig(config); + const esQueryConfigs = esQuery.getEsQueryConfig(getUiSettings()); body.query = esQuery.buildEsQuery(index, query, filters, esQueryConfigs); if (highlightAll && body.query) { - body.highlight = getHighlightRequest(body.query, config.get('doc_table:highlight')); + body.highlight = getHighlightRequest(body.query, getUiSettings().get('doc_table:highlight')); delete searchRequest.highlightAll; } diff --git a/src/legacy/core_plugins/data/public/search/search_strategy/default_search_strategy.test.ts b/src/legacy/core_plugins/data/public/search/search_strategy/default_search_strategy.test.ts index 0ec6a6c2e143e..8caf20c50cd3a 100644 --- a/src/legacy/core_plugins/data/public/search/search_strategy/default_search_strategy.test.ts +++ b/src/legacy/core_plugins/data/public/search/search_strategy/default_search_strategy.test.ts @@ -37,9 +37,16 @@ const searchMockResponse: any = Promise.resolve([]); searchMockResponse.abort = jest.fn(); const searchMock = jest.fn().mockReturnValue(searchMockResponse); +const newSearchMockResponse: any = Promise.resolve([]); +newSearchMockResponse.abort = jest.fn(); +const newSearchMock = jest.fn().mockReturnValue({ + toPromise: () => searchMockResponse, +}); + describe('defaultSearchStrategy', function() { describe('search', function() { let searchArgs: MockedKeys>; + let es: any; beforeEach(() => { msearchMockResponse.abort.mockClear(); @@ -55,17 +62,24 @@ describe('defaultSearchStrategy', function() { }, ], esShardTimeout: 0, - es: { - msearch: msearchMock, - search: searchMock, + searchService: { + search: newSearchMock, + __LEGACY: { + esClient: { + search: searchMock, + msearch: msearchMock, + }, + }, }, }; + + es = searchArgs.searchService.__LEGACY.esClient; }); test('does not send max_concurrent_shard_requests by default', async () => { const config = getConfigStub({ 'courier:batchSearches': true }); await search({ ...searchArgs, config }); - expect(searchArgs.es.msearch.mock.calls[0][0].max_concurrent_shard_requests).toBe(undefined); + expect(es.msearch.mock.calls[0][0].max_concurrent_shard_requests).toBe(undefined); }); test('allows configuration of max_concurrent_shard_requests', async () => { @@ -74,13 +88,13 @@ describe('defaultSearchStrategy', function() { 'courier:maxConcurrentShardRequests': 42, }); await search({ ...searchArgs, config }); - expect(searchArgs.es.msearch.mock.calls[0][0].max_concurrent_shard_requests).toBe(42); + expect(es.msearch.mock.calls[0][0].max_concurrent_shard_requests).toBe(42); }); test('should set rest_total_hits_as_int to true on a request', async () => { const config = getConfigStub({ 'courier:batchSearches': true }); await search({ ...searchArgs, config }); - expect(searchArgs.es.msearch.mock.calls[0][0]).toHaveProperty('rest_total_hits_as_int', true); + expect(es.msearch.mock.calls[0][0]).toHaveProperty('rest_total_hits_as_int', true); }); test('should set ignore_throttled=false when including frozen indices', async () => { @@ -89,7 +103,7 @@ describe('defaultSearchStrategy', function() { 'search:includeFrozen': true, }); await search({ ...searchArgs, config }); - expect(searchArgs.es.msearch.mock.calls[0][0]).toHaveProperty('ignore_throttled', false); + expect(es.msearch.mock.calls[0][0]).toHaveProperty('ignore_throttled', false); }); test('should properly call abort with msearch', () => { @@ -100,12 +114,18 @@ describe('defaultSearchStrategy', function() { expect(msearchMockResponse.abort).toHaveBeenCalled(); }); - test('should properly abort with search', async () => { - const config = getConfigStub({ - 'courier:batchSearches': false, - }); + test('should call new search service', () => { + const config = getConfigStub(); + search({ ...searchArgs, config }); + expect(searchMock).toHaveBeenCalled(); + expect(newSearchMock).toHaveBeenCalledTimes(0); + }); + + test('should properly abort with new search service', async () => { + const abortSpy = jest.spyOn(AbortController.prototype, 'abort'); + const config = getConfigStub({}); search({ ...searchArgs, config }).abort(); - expect(searchMockResponse.abort).toHaveBeenCalled(); + expect(abortSpy).toHaveBeenCalled(); }); }); }); diff --git a/src/legacy/core_plugins/data/public/search/search_strategy/default_search_strategy.ts b/src/legacy/core_plugins/data/public/search/search_strategy/default_search_strategy.ts index 9bfa1df71aa81..39789504de0a7 100644 --- a/src/legacy/core_plugins/data/public/search/search_strategy/default_search_strategy.ts +++ b/src/legacy/core_plugins/data/public/search/search_strategy/default_search_strategy.ts @@ -38,7 +38,14 @@ export const defaultSearchStrategy: SearchStrategyProvider = { }, }; -function msearch({ searchRequests, es, config, esShardTimeout }: SearchStrategySearchParams) { +// @deprecated +function msearch({ + searchRequests, + searchService, + config, + esShardTimeout, +}: SearchStrategySearchParams) { + const es = searchService.__LEGACY.esClient; const inlineRequests = searchRequests.map(({ index, body, search_type: searchType }) => { const inlineHeader = { index: index.title || index, @@ -57,19 +64,39 @@ function msearch({ searchRequests, es, config, esShardTimeout }: SearchStrategyS ...getMSearchParams(config), body: `${inlineRequests.join('\n')}\n`, }); + return { - searching: searching.then(({ responses }) => responses), + searching: searching.then(({ responses }: any) => responses), abort: searching.abort, }; } -function search({ searchRequests, es, config, esShardTimeout }: SearchStrategySearchParams) { +function search({ + searchRequests, + searchService, + config, + esShardTimeout, +}: SearchStrategySearchParams) { const abortController = new AbortController(); const searchParams = getSearchParams(config, esShardTimeout); + const es = searchService.__LEGACY.esClient; const promises = searchRequests.map(({ index, body }) => { const searching = es.search({ index: index.title || index, body, ...searchParams }); abortController.signal.addEventListener('abort', searching.abort); - return searching.catch(({ response }) => JSON.parse(response)); + return searching.catch(({ response }: any) => JSON.parse(response)); + /* + * Once #44302 is resolved, replace the old implementation with this one - + * const params = { + * index: index.title || index, + * body, + * ...searchParams, + * }; + * const { signal } = abortController; + * return searchService + * .search({ params }, { signal }) + * .toPromise() + * .then(({ rawResponse }) => rawResponse); + */ }); return { searching: Promise.all(promises), diff --git a/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/get_painless_error.ts b/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/get_painless_error.ts index 212fd870a5aeb..2bbeea9d675c7 100644 --- a/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/get_painless_error.ts +++ b/src/legacy/core_plugins/kibana/public/discover/np_ready/angular/get_painless_error.ts @@ -25,6 +25,7 @@ export function getPainlessError(error: Error) { error, 'resp.error.root_cause' ); + const message: string = get(error, 'message'); if (!rootCause) { return; @@ -43,6 +44,6 @@ export function getPainlessError(error: Error) { defaultMessage: "Error with Painless scripted field '{script}'.", values: { script }, }), - error: error.message, + error: message, }; } diff --git a/src/legacy/core_plugins/visualizations/public/legacy_imports.ts b/src/legacy/core_plugins/visualizations/public/legacy_imports.ts index b750557c24b94..fd40c831ce0ef 100644 --- a/src/legacy/core_plugins/visualizations/public/legacy_imports.ts +++ b/src/legacy/core_plugins/visualizations/public/legacy_imports.ts @@ -18,7 +18,6 @@ */ export { PersistedState } from '../../../ui/public/persisted_state'; -export { SearchError } from '../../../ui/public/courier/search_strategy/search_error'; export { AggConfig } from '../../../ui/public/agg_types/agg_config'; export { AggConfigs } from '../../../ui/public/agg_types/agg_configs'; export { diff --git a/src/legacy/core_plugins/visualizations/public/np_ready/public/components/visualization_requesterror.tsx b/src/legacy/core_plugins/visualizations/public/np_ready/public/components/visualization_requesterror.tsx index 8b9fded919f13..1af9aa3c3e602 100644 --- a/src/legacy/core_plugins/visualizations/public/np_ready/public/components/visualization_requesterror.tsx +++ b/src/legacy/core_plugins/visualizations/public/np_ready/public/components/visualization_requesterror.tsx @@ -19,7 +19,7 @@ import { EuiIcon, EuiSpacer, EuiText } from '@elastic/eui'; import React from 'react'; -import { SearchError } from '../../../legacy_imports'; +import { SearchError } from '../../../../../data/public/search/search_strategy'; interface VisualizationRequestErrorProps { onInit?: () => void; diff --git a/src/legacy/core_plugins/visualizations/public/np_ready/public/index.ts b/src/legacy/core_plugins/visualizations/public/np_ready/public/index.ts index 2e9d055858a48..29ff812b95473 100644 --- a/src/legacy/core_plugins/visualizations/public/np_ready/public/index.ts +++ b/src/legacy/core_plugins/visualizations/public/np_ready/public/index.ts @@ -52,3 +52,5 @@ export { buildPipeline, buildVislibDimensions, SchemaConfig } from './legacy/bui // @ts-ignore export { updateOldState } from './legacy/vis_update_state'; export { calculateObjectHash } from './legacy/calculate_object_hash'; +// @ts-ignore +export { createFiltersFromEvent } from './filters/vis_filters'; diff --git a/src/legacy/ui/public/new_platform/new_platform.karma_mock.js b/src/legacy/ui/public/new_platform/new_platform.karma_mock.js index d3f74a540b960..dfd26bc4be039 100644 --- a/src/legacy/ui/public/new_platform/new_platform.karma_mock.js +++ b/src/legacy/ui/public/new_platform/new_platform.karma_mock.js @@ -236,6 +236,14 @@ export const npStart = { history: sinon.fake(), }, }, + search: { + __LEGACY: { + esClient: { + search: sinon.fake(), + msearch: sinon.fake(), + }, + }, + }, fieldFormats: getFieldFormatsRegistry(mockCore), }, share: { diff --git a/src/plugins/data/public/mocks.ts b/src/plugins/data/public/mocks.ts index 08a7a3ef11537..f44d40f533eed 100644 --- a/src/plugins/data/public/mocks.ts +++ b/src/plugins/data/public/mocks.ts @@ -69,13 +69,28 @@ const createStartContract = (): Start => { const startContract = { autocomplete: autocompleteMock, getSuggestions: jest.fn(), - search: { search: jest.fn() }, + search: { + search: jest.fn(), + + __LEGACY: { + esClient: { + search: jest.fn(), + msearch: jest.fn(), + }, + }, + }, fieldFormats: fieldFormatsMock as FieldFormatsStart, query: queryStartMock, ui: { IndexPatternSelect: jest.fn(), SearchBar: jest.fn(), }, + __LEGACY: { + esClient: { + search: jest.fn(), + msearch: jest.fn(), + }, + }, indexPatterns: {} as IndexPatternsContract, }; return startContract; diff --git a/src/plugins/data/public/plugin.ts b/src/plugins/data/public/plugin.ts index 78abcbcfec467..ce8ca0317bd7d 100644 --- a/src/plugins/data/public/plugin.ts +++ b/src/plugins/data/public/plugin.ts @@ -17,7 +17,13 @@ * under the License. */ -import { PluginInitializerContext, CoreSetup, CoreStart, Plugin } from 'src/core/public'; +import { + PluginInitializerContext, + CoreSetup, + CoreStart, + Plugin, + PackageInfo, +} from 'src/core/public'; import { Storage, IStorageWrapper } from '../../kibana_utils/public'; import { DataPublicPluginSetup, @@ -31,7 +37,13 @@ import { FieldFormatsService } from './field_formats_provider'; import { QueryService } from './query'; import { createIndexPatternSelect } from './ui/index_pattern_select'; import { IndexPatterns } from './index_patterns'; -import { setNotifications, setFieldFormats, setOverlays, setIndexPatterns } from './services'; +import { + setNotifications, + setFieldFormats, + setOverlays, + setIndexPatterns, + setHttp, +} from './services'; import { createFilterAction, GLOBAL_APPLY_FILTER_ACTION } from './actions'; import { APPLY_FILTER_TRIGGER } from '../../embeddable/public'; import { createSearchBar } from './ui/search_bar/create_search_bar'; @@ -42,12 +54,14 @@ export class DataPublicPlugin implements Plugin { it('Returns search fn when there are no strategies', () => { - const context = createAppMountSearchContext({}); + const context = createAppMountSearchContext({}, new BehaviorSubject(0)); expect(context.search).toBeDefined(); }); it(`Search throws an error when the strategy doesn't exist`, () => { - const context = createAppMountSearchContext({}); + const context = createAppMountSearchContext({}, new BehaviorSubject(0)); expect(() => context.search({}, {}, 'noexist').toPromise()).toThrowErrorMatchingInlineSnapshot( `"Strategy with name noexist does not exist"` ); }); it(`Search fn is called on appropriate strategy name`, done => { - const context = createAppMountSearchContext({ - mysearch: search => - Promise.resolve({ - search: () => from(Promise.resolve({ percentComplete: 98 })), - }), - anothersearch: search => - Promise.resolve({ - search: () => from(Promise.resolve({ percentComplete: 0 })), - }), - }); + const context = createAppMountSearchContext( + { + mysearch: search => + Promise.resolve({ + search: () => from(Promise.resolve({ percentComplete: 98 })), + }), + anothersearch: search => + Promise.resolve({ + search: () => from(Promise.resolve({ percentComplete: 0 })), + }), + }, + new BehaviorSubject(0) + ); context.search({}, {}, 'mysearch').subscribe(response => { expect(response).toEqual({ percentComplete: 98 }); @@ -52,16 +55,19 @@ describe('Create app mount search context', () => { }); it(`Search fn is called with the passed in request object`, done => { - const context = createAppMountSearchContext({ - mysearch: search => { - return Promise.resolve({ - search: request => { - expect(request).toEqual({ greeting: 'hi' }); - return from(Promise.resolve({})); - }, - }); + const context = createAppMountSearchContext( + { + mysearch: search => { + return Promise.resolve({ + search: request => { + expect(request).toEqual({ greeting: 'hi' }); + return from(Promise.resolve({})); + }, + }); + }, }, - }); + new BehaviorSubject(0) + ); context.search({ greeting: 'hi' } as any, {}, 'mysearch').subscribe( response => {}, () => {}, diff --git a/src/plugins/data/public/search/create_app_mount_context_search.ts b/src/plugins/data/public/search/create_app_mount_context_search.ts index 5659a9c863dc1..f480b8f3e042e 100644 --- a/src/plugins/data/public/search/create_app_mount_context_search.ts +++ b/src/plugins/data/public/search/create_app_mount_context_search.ts @@ -17,8 +17,8 @@ * under the License. */ -import { mergeMap } from 'rxjs/operators'; -import { from } from 'rxjs'; +import { mergeMap, tap } from 'rxjs/operators'; +import { from, BehaviorSubject } from 'rxjs'; import { ISearchAppMountContext } from './i_search_app_mount_context'; import { ISearchGeneric } from './i_search'; import { @@ -30,7 +30,8 @@ import { TStrategyTypes } from './strategy_types'; import { DEFAULT_SEARCH_STRATEGY } from '../../common/search'; export const createAppMountSearchContext = ( - searchStrategies: TSearchStrategiesMap + searchStrategies: TSearchStrategiesMap, + loadingCount$: BehaviorSubject ): ISearchAppMountContext => { const getSearchStrategy = ( strategyName?: K @@ -48,7 +49,13 @@ export const createAppMountSearchContext = ( const strategyPromise = getSearchStrategy(strategyName); return from(strategyPromise).pipe( mergeMap(strategy => { - return strategy.search(request, options); + loadingCount$.next(loadingCount$.getValue() + 1); + return strategy.search(request, options).pipe( + tap( + error => loadingCount$.next(loadingCount$.getValue() - 1), + complete => loadingCount$.next(loadingCount$.getValue() - 1) + ) + ); }) ); }; diff --git a/src/plugins/data/public/search/es_client/get_es_client.ts b/src/plugins/data/public/search/es_client/get_es_client.ts new file mode 100644 index 0000000000000..6c271643ba012 --- /dev/null +++ b/src/plugins/data/public/search/es_client/get_es_client.ts @@ -0,0 +1,93 @@ +/* + * 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. + */ + +// @ts-ignore +import { default as es } from 'elasticsearch-browser/elasticsearch'; +import { CoreStart, PackageInfo } from 'kibana/public'; +import { BehaviorSubject } from 'rxjs'; + +export function getEsClient( + injectedMetadata: CoreStart['injectedMetadata'], + http: CoreStart['http'], + packageInfo: PackageInfo, + loadingCount$: BehaviorSubject +) { + const esRequestTimeout = injectedMetadata.getInjectedVar('esRequestTimeout') as number; + const esApiVersion = injectedMetadata.getInjectedVar('esApiVersion') as string; + + // Use legacy es client for msearch. + const client = es.Client({ + host: getEsUrl(http, packageInfo), + log: 'info', + requestTimeout: esRequestTimeout, + apiVersion: esApiVersion, + }); + + return { + search: wrapEsClientMethod(client, 'search', loadingCount$), + msearch: wrapEsClientMethod(client, 'msearch', loadingCount$), + create: wrapEsClientMethod(client, 'create', loadingCount$), + }; +} + +function wrapEsClientMethod(esClient: any, method: string, loadingCount$: BehaviorSubject) { + return (args: any) => { + // esClient returns a promise, with an additional abort handler + // To tap into the abort handling, we have to override that abort handler. + const customPromiseThingy = esClient[method](args); + const { abort } = customPromiseThingy; + let resolved = false; + + // Start LoadingIndicator + loadingCount$.next(loadingCount$.getValue() + 1); + + // Stop LoadingIndicator when user aborts + customPromiseThingy.abort = () => { + abort(); + if (!resolved) { + resolved = true; + loadingCount$.next(loadingCount$.getValue() - 1); + } + }; + + // Stop LoadingIndicator when promise finishes + customPromiseThingy.finally(() => { + resolved = true; + loadingCount$.next(loadingCount$.getValue() - 1); + }); + + return customPromiseThingy; + }; +} + +function getEsUrl(http: CoreStart['http'], packageInfo: PackageInfo) { + const a = document.createElement('a'); + a.href = http.basePath.prepend('/elasticsearch'); + const protocolPort = /https/.test(a.protocol) ? 443 : 80; + const port = a.port || protocolPort; + return { + host: a.hostname, + port, + protocol: a.protocol, + pathname: a.pathname, + headers: { + 'kbn-version': packageInfo.version, + }, + }; +} diff --git a/src/plugins/data/public/search/es_client/index.ts b/src/plugins/data/public/search/es_client/index.ts new file mode 100644 index 0000000000000..bf1a3f5d6e7c4 --- /dev/null +++ b/src/plugins/data/public/search/es_client/index.ts @@ -0,0 +1,20 @@ +/* + * 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 { getEsClient } from './get_es_client'; diff --git a/src/plugins/data/public/search/search_service.ts b/src/plugins/data/public/search/search_service.ts index 6030884c9f6b1..6f3e228939d6d 100644 --- a/src/plugins/data/public/search/search_service.ts +++ b/src/plugins/data/public/search/search_service.ts @@ -16,6 +16,7 @@ * specific language governing permissions and limitations * under the License. */ +import { BehaviorSubject } from 'rxjs'; import { Plugin, CoreSetup, @@ -23,6 +24,7 @@ import { CoreStart, IContextContainer, PluginOpaqueId, + PackageInfo, } from '../../../../core/public'; import { ISearchAppMountContext } from './i_search_app_mount_context'; @@ -37,6 +39,7 @@ import { import { TStrategyTypes } from './strategy_types'; import { esSearchService } from './es_search'; import { ISearchGeneric } from './i_search'; +import { getEsClient } from './es_client'; /** * Extends the AppMountContext so other plugins have access @@ -50,6 +53,9 @@ declare module 'kibana/public' { export interface ISearchStart { search: ISearchGeneric; + __LEGACY: { + esClient: any; + }; } /** @@ -74,11 +80,16 @@ export class SearchService implements Plugin { private contextContainer?: IContextContainer>; private search?: ISearchGeneric; + private readonly loadingCount$ = new BehaviorSubject(0); constructor(private initializerContext: PluginInitializerContext) {} public setup(core: CoreSetup): ISearchSetup { - const search = (this.search = createAppMountSearchContext(this.searchStrategies).search); + core.http.addLoadingCountSource(this.loadingCount$); + const search = (this.search = createAppMountSearchContext( + this.searchStrategies, + this.loadingCount$ + ).search); core.application.registerMountContext<'search'>('search', () => { return { search }; }); @@ -115,11 +126,16 @@ export class SearchService implements Plugin { return api; } - public start(core: CoreStart) { + public start(core: CoreStart, packageInfo: PackageInfo) { if (!this.search) { throw new Error('Search should always be defined'); } - return { search: this.search }; + return { + search: this.search, + __LEGACY: { + esClient: getEsClient(core.injectedMetadata, core.http, packageInfo, this.loadingCount$), + }, + }; } public stop() {} diff --git a/src/plugins/data/public/services.ts b/src/plugins/data/public/services.ts index 16347b752b30a..6a15893f573d8 100644 --- a/src/plugins/data/public/services.ts +++ b/src/plugins/data/public/services.ts @@ -17,8 +17,8 @@ * under the License. */ -import { NotificationsStart, HttpStart } from 'src/core/public'; -import { CoreStart } from 'kibana/public'; +import { NotificationsStart } from 'src/core/public'; +import { CoreSetup, CoreStart } from 'kibana/public'; import { FieldFormatsStart } from '.'; import { createGetterSetter } from '../../kibana_utils/public'; import { IndexPatternsContract } from './index_patterns'; @@ -28,11 +28,16 @@ export const [getNotifications, setNotifications] = createGetterSetter( + 'UiSettings' +); + +export const [getHttp, setHttp] = createGetterSetter('Http'); + export const [getFieldFormats, setFieldFormats] = createGetterSetter( 'FieldFormats' ); -export const [getHttp, setHttp] = createGetterSetter('Http'); export const [getOverlays, setOverlays] = createGetterSetter('Overlays'); export const [getIndexPatterns, setIndexPatterns] = createGetterSetter( @@ -42,3 +47,11 @@ export const [getIndexPatterns, setIndexPatterns] = createGetterSetter('Query'); + +export const [getInjectedMetadata, setInjectedMetadata] = createGetterSetter< + CoreSetup['injectedMetadata'] +>('InjectedMetadata'); + +export const [getSearchService, setSearchService] = createGetterSetter< + DataPublicPluginStart['search'] +>('Search'); diff --git a/src/plugins/data/public/ui/query_string_input/__snapshots__/query_string_input.test.tsx.snap b/src/plugins/data/public/ui/query_string_input/__snapshots__/query_string_input.test.tsx.snap index d8db53d4c6020..f38adff892099 100644 --- a/src/plugins/data/public/ui/query_string_input/__snapshots__/query_string_input.test.tsx.snap +++ b/src/plugins/data/public/ui/query_string_input/__snapshots__/query_string_input.test.tsx.snap @@ -150,6 +150,12 @@ exports[`QueryStringInput Should disable autoFocus on EuiFieldText when disableA "setIsVisible": [MockFunction], }, "data": Object { + "__LEGACY": Object { + "esClient": Object { + "msearch": [MockFunction], + "search": [MockFunction], + }, + }, "autocomplete": Object { "getQuerySuggestions": [MockFunction], "getValueSuggestions": [MockFunction], @@ -205,6 +211,12 @@ exports[`QueryStringInput Should disable autoFocus on EuiFieldText when disableA }, }, "search": Object { + "__LEGACY": Object { + "esClient": Object { + "msearch": [MockFunction], + "search": [MockFunction], + }, + }, "search": [MockFunction], }, "ui": Object { @@ -780,6 +792,12 @@ exports[`QueryStringInput Should disable autoFocus on EuiFieldText when disableA "setIsVisible": [MockFunction], }, "data": Object { + "__LEGACY": Object { + "esClient": Object { + "msearch": [MockFunction], + "search": [MockFunction], + }, + }, "autocomplete": Object { "getQuerySuggestions": [MockFunction], "getValueSuggestions": [MockFunction], @@ -835,6 +853,12 @@ exports[`QueryStringInput Should disable autoFocus on EuiFieldText when disableA }, }, "search": Object { + "__LEGACY": Object { + "esClient": Object { + "msearch": [MockFunction], + "search": [MockFunction], + }, + }, "search": [MockFunction], }, "ui": Object { @@ -1392,6 +1416,12 @@ exports[`QueryStringInput Should pass the query language to the language switche "setIsVisible": [MockFunction], }, "data": Object { + "__LEGACY": Object { + "esClient": Object { + "msearch": [MockFunction], + "search": [MockFunction], + }, + }, "autocomplete": Object { "getQuerySuggestions": [MockFunction], "getValueSuggestions": [MockFunction], @@ -1447,6 +1477,12 @@ exports[`QueryStringInput Should pass the query language to the language switche }, }, "search": Object { + "__LEGACY": Object { + "esClient": Object { + "msearch": [MockFunction], + "search": [MockFunction], + }, + }, "search": [MockFunction], }, "ui": Object { @@ -2019,6 +2055,12 @@ exports[`QueryStringInput Should pass the query language to the language switche "setIsVisible": [MockFunction], }, "data": Object { + "__LEGACY": Object { + "esClient": Object { + "msearch": [MockFunction], + "search": [MockFunction], + }, + }, "autocomplete": Object { "getQuerySuggestions": [MockFunction], "getValueSuggestions": [MockFunction], @@ -2074,6 +2116,12 @@ exports[`QueryStringInput Should pass the query language to the language switche }, }, "search": Object { + "__LEGACY": Object { + "esClient": Object { + "msearch": [MockFunction], + "search": [MockFunction], + }, + }, "search": [MockFunction], }, "ui": Object { @@ -2631,6 +2679,12 @@ exports[`QueryStringInput Should render the given query 1`] = ` "setIsVisible": [MockFunction], }, "data": Object { + "__LEGACY": Object { + "esClient": Object { + "msearch": [MockFunction], + "search": [MockFunction], + }, + }, "autocomplete": Object { "getQuerySuggestions": [MockFunction], "getValueSuggestions": [MockFunction], @@ -2686,6 +2740,12 @@ exports[`QueryStringInput Should render the given query 1`] = ` }, }, "search": Object { + "__LEGACY": Object { + "esClient": Object { + "msearch": [MockFunction], + "search": [MockFunction], + }, + }, "search": [MockFunction], }, "ui": Object { @@ -3258,6 +3318,12 @@ exports[`QueryStringInput Should render the given query 1`] = ` "setIsVisible": [MockFunction], }, "data": Object { + "__LEGACY": Object { + "esClient": Object { + "msearch": [MockFunction], + "search": [MockFunction], + }, + }, "autocomplete": Object { "getQuerySuggestions": [MockFunction], "getValueSuggestions": [MockFunction], @@ -3313,6 +3379,12 @@ exports[`QueryStringInput Should render the given query 1`] = ` }, }, "search": Object { + "__LEGACY": Object { + "esClient": Object { + "msearch": [MockFunction], + "search": [MockFunction], + }, + }, "search": [MockFunction], }, "ui": Object { diff --git a/src/plugins/data/server/search/routes.test.ts b/src/plugins/data/server/search/routes.test.ts index a2394d88f3931..6ea0799f790fc 100644 --- a/src/plugins/data/server/search/routes.test.ts +++ b/src/plugins/data/server/search/routes.test.ts @@ -65,8 +65,13 @@ describe('Search service', () => { expect(mockResponse.ok.mock.calls[0][0]).toEqual({ body: 'yay' }); }); - it('handler throws internal error if the search throws an error', async () => { - const mockSearch = jest.fn().mockRejectedValue('oh no'); + it('handler throws an error if the search throws an error', async () => { + const mockSearch = jest.fn().mockRejectedValue({ + message: 'oh no', + body: { + error: 'oops', + }, + }); const mockContext = { core: { elasticsearch: { @@ -93,7 +98,9 @@ describe('Search service', () => { expect(mockSearch).toBeCalled(); expect(mockSearch.mock.calls[0][0]).toStrictEqual(mockBody); expect(mockSearch.mock.calls[0][2]).toBe(mockParams.strategy); - expect(mockResponse.internalError).toBeCalled(); - expect(mockResponse.internalError.mock.calls[0][0]).toEqual({ body: 'oh no' }); + expect(mockResponse.customError).toBeCalled(); + const error: any = mockResponse.customError.mock.calls[0][0]; + expect(error.body.message).toBe('oh no'); + expect(error.body.attributes.error).toBe('oops'); }); }); diff --git a/src/plugins/data/server/search/routes.ts b/src/plugins/data/server/search/routes.ts index eaa72548e08ee..6f726771c41b2 100644 --- a/src/plugins/data/server/search/routes.ts +++ b/src/plugins/data/server/search/routes.ts @@ -39,7 +39,15 @@ export function registerSearchRoute(router: IRouter): void { const response = await context.search!.search(searchRequest, {}, strategy); return res.ok({ body: response }); } catch (err) { - return res.internalError({ body: err }); + return res.customError({ + statusCode: err.statusCode, + body: { + message: err.message, + attributes: { + error: err.body.error, + }, + }, + }); } } ); diff --git a/test/functional/apps/discover/_errors.js b/test/functional/apps/discover/_errors.js index 53dcd8cc9e5c1..7dbb93c884f46 100644 --- a/test/functional/apps/discover/_errors.js +++ b/test/functional/apps/discover/_errors.js @@ -22,10 +22,11 @@ import expect from '@kbn/expect'; export default function({ getService, getPageObjects }) { const esArchiver = getService('esArchiver'); const testSubjects = getService('testSubjects'); - const PageObjects = getPageObjects(['common']); + const PageObjects = getPageObjects(['common', 'discover']); describe('errors', function describeIndexTests() { before(async function() { + await esArchiver.loadIfNeeded('logstash_functional'); await esArchiver.load('invalid_scripted_field'); await PageObjects.common.navigateToApp('discover'); }); diff --git a/x-pack/legacy/plugins/maps/public/angular/map_controller.js b/x-pack/legacy/plugins/maps/public/angular/map_controller.js index 75971d5dfe2a8..eec97dc5c71e9 100644 --- a/x-pack/legacy/plugins/maps/public/angular/map_controller.js +++ b/x-pack/legacy/plugins/maps/public/angular/map_controller.js @@ -367,7 +367,9 @@ app.controller( if (prevIndexPatternIds !== nextIndexPatternIds) { return; } - $scope.indexPatterns = indexPatterns; + $scope.$evalAsync(() => { + $scope.indexPatterns = indexPatterns; + }); } $scope.isFullScreen = false;