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 ca9dea0. * Revert "Cleanup search params test" This reverts commit 30b9478. * 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 <[email protected]>
elastic · Jan 21, 2020 · f265961 · f265961
1 parent ce2930e
commit f265961
Show file tree

Hide file tree

Showing 30 changed files with 474 additions and 104 deletions.
diff --git 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
@@ -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<void, DataStart, {}, DataPluginStartDependencies> {
+export class DataPlugin
+  implements Plugin<void, DataStart, DataPluginSetupDependencies, DataPluginStartDependencies> {
   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/...inspector/build_tabular_inspector_data.ts → ...pressions/build_tabular_inspector_data.ts b/...inspector/build_tabular_inspector_data.ts → ...pressions/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
@@ -34,21 +34,17 @@ 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';
 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
@@ -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
@@ -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
@@ -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<SearchResponse> | 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
@@ -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
@@ -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
@@ -70,19 +70,21 @@
  */
 
 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';
 import { getHighlightRequest, esFilters, esQuery } from '../../../../../../plugins/data/public';
 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<SearchSource, keyof SearchSource>;
 
@@ -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;
     }