Skip to content

Commit

Permalink
[data.search.SearchSource] Remove legacy ES client APIs. (#75943)
Browse files Browse the repository at this point in the history
  • Loading branch information
lukeelmers authored Sep 3, 2020
1 parent c80a733 commit d494f1e
Show file tree
Hide file tree
Showing 32 changed files with 539 additions and 405 deletions.
1 change: 0 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -366,7 +366,6 @@
"dedent": "^0.7.0",
"deepmerge": "^4.2.2",
"delete-empty": "^2.0.0",
"elasticsearch-browser": "^16.7.0",
"enzyme": "^3.11.0",
"enzyme-adapter-react-16": "^1.15.2",
"enzyme-adapter-utils": "^1.13.0",
Expand Down
1 change: 0 additions & 1 deletion packages/kbn-optimizer/src/worker/webpack.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,6 @@ export function getWebpackConfig(bundle: Bundle, bundleRefs: BundleRefs, worker:
// or which have require() statements that should be ignored because the file is
// already bundled with all its necessary depedencies
noParse: [
/[\/\\]node_modules[\/\\]elasticsearch-browser[\/\\]/,
/[\/\\]node_modules[\/\\]lodash[\/\\]index\.js$/,
/[\/\\]node_modules[\/\\]vega[\/\\]build[\/\\]vega\.js$/,
],
Expand Down
3 changes: 0 additions & 3 deletions packages/kbn-ui-shared-deps/entry.js
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,3 @@ export const ElasticEuiChartsTheme = require('@elastic/eui/dist/eui_charts_theme

import * as Theme from './theme.ts';
export { Theme };

// massive deps that we should really get rid of or reduce in size substantially
export const ElasticsearchBrowser = require('elasticsearch-browser/elasticsearch.js');
7 changes: 0 additions & 7 deletions packages/kbn-ui-shared-deps/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -62,12 +62,5 @@ exports.externals = {
'@elastic/eui/dist/eui_charts_theme': '__kbnSharedDeps__.ElasticEuiChartsTheme',
'@elastic/eui/dist/eui_theme_light.json': '__kbnSharedDeps__.Theme.euiLightVars',
'@elastic/eui/dist/eui_theme_dark.json': '__kbnSharedDeps__.Theme.euiDarkVars',

/**
* massive deps that we should really get rid of or reduce in size substantially
*/
elasticsearch: '__kbnSharedDeps__.ElasticsearchBrowser',
'elasticsearch-browser': '__kbnSharedDeps__.ElasticsearchBrowser',
'elasticsearch-browser/elasticsearch': '__kbnSharedDeps__.ElasticsearchBrowser',
};
exports.publicPathLoader = require.resolve('./public_path_loader');
1 change: 0 additions & 1 deletion packages/kbn-ui-shared-deps/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
"compression-webpack-plugin": "^4.0.0",
"core-js": "^3.6.4",
"custom-event-polyfill": "^0.3.0",
"elasticsearch-browser": "^16.7.0",
"jquery": "^3.5.0",
"mini-css-extract-plugin": "0.8.0",
"moment": "^2.24.0",
Expand Down
11 changes: 1 addition & 10 deletions src/plugins/data/public/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,7 @@

import './index.scss';

import {
PluginInitializerContext,
CoreSetup,
CoreStart,
Plugin,
PackageInfo,
} from 'src/core/public';
import { PluginInitializerContext, CoreSetup, CoreStart, Plugin } from 'src/core/public';
import { ConfigSchema } from '../config';
import { Storage, IStorageWrapper, createStartServicesGetter } from '../../kibana_utils/public';
import {
Expand Down Expand Up @@ -100,15 +94,13 @@ export class DataPublicPlugin
private readonly fieldFormatsService: FieldFormatsService;
private readonly queryService: QueryService;
private readonly storage: IStorageWrapper;
private readonly packageInfo: PackageInfo;

constructor(initializerContext: PluginInitializerContext<ConfigSchema>) {
this.searchService = new SearchService();
this.queryService = new QueryService();
this.fieldFormatsService = new FieldFormatsService();
this.autocomplete = new AutocompleteService(initializerContext);
this.storage = new Storage(window.localStorage);
this.packageInfo = initializerContext.env.packageInfo;
}

public setup(
Expand Down Expand Up @@ -145,7 +137,6 @@ export class DataPublicPlugin

const searchService = this.searchService.setup(core, {
usageCollection,
packageInfo: this.packageInfo,
expressions,
});

Expand Down
1 change: 1 addition & 0 deletions src/plugins/data/public/public.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import { ExpressionAstFunction } from 'src/plugins/expressions/common';
import { ExpressionsSetup } from 'src/plugins/expressions/public';
import { History } from 'history';
import { Href } from 'history';
import { HttpStart } from 'src/core/public';
import { IconType } from '@elastic/eui';
import { InjectedIntl } from '@kbn/i18n/react';
import { ISearchOptions as ISearchOptions_2 } from 'src/plugins/data/public';
Expand Down
7 changes: 4 additions & 3 deletions src/plugins/data/public/search/fetch/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,9 @@
* under the License.
*/

import { HttpStart } from 'src/core/public';
import { BehaviorSubject } from 'rxjs';
import { GetConfigFn } from '../../../common';
import { ISearchStartLegacy } from '../types';

/**
* @internal
Expand All @@ -30,9 +31,9 @@ import { ISearchStartLegacy } from '../types';
export type SearchRequest = Record<string, any>;

export interface FetchHandlers {
legacySearchService: ISearchStartLegacy;
config: { get: GetConfigFn };
esShardTimeout: number;
http: HttpStart;
loadingCount$: BehaviorSubject<number>;
}

export interface SearchError {
Expand Down
10 changes: 9 additions & 1 deletion src/plugins/data/public/search/legacy/call_client.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,13 @@
* under the License.
*/

import { coreMock } from '../../../../../core/public/mocks';
import { callClient } from './call_client';
import { SearchStrategySearchParams } from './types';
import { defaultSearchStrategy } from './default_search_strategy';
import { FetchHandlers } from '../fetch';
import { handleResponse } from '../fetch/handle_response';
import { BehaviorSubject } from 'rxjs';

const mockAbortFn = jest.fn();
jest.mock('../fetch/handle_response', () => ({
Expand Down Expand Up @@ -54,7 +56,13 @@ describe('callClient', () => {

test('Passes the additional arguments it is given to the search strategy', () => {
const searchRequests = [{ _searchStrategyId: 0 }];
const args = { legacySearchService: {}, config: {}, esShardTimeout: 0 } as FetchHandlers;
const args = {
http: coreMock.createStart().http,
legacySearchService: {},
config: { get: jest.fn() },
esShardTimeout: 0,
loadingCount$: new BehaviorSubject(0),
} as FetchHandlers;

callClient(searchRequests, [], args);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,94 +17,54 @@
* under the License.
*/

import { IUiSettingsClient } from 'kibana/public';
import { HttpStart } from 'src/core/public';
import { coreMock } from '../../../../../core/public/mocks';
import { defaultSearchStrategy } from './default_search_strategy';
import { searchServiceMock } from '../mocks';
import { SearchStrategySearchParams } from './types';
import { UI_SETTINGS } from '../../../common';
import { BehaviorSubject } from 'rxjs';

const { search } = defaultSearchStrategy;

function getConfigStub(config: any = {}) {
return {
get: (key) => config[key],
} as IUiSettingsClient;
}

const msearchMockResponse: any = Promise.resolve([]);
msearchMockResponse.abort = jest.fn();
const msearchMock = jest.fn().mockReturnValue(msearchMockResponse);

const searchMockResponse: any = Promise.resolve([]);
searchMockResponse.abort = jest.fn();
const searchMock = jest.fn().mockReturnValue(searchMockResponse);
const msearchMock = jest.fn().mockResolvedValue({ body: { responses: [] } });

describe('defaultSearchStrategy', function () {
describe('search', function () {
let searchArgs: MockedKeys<Omit<SearchStrategySearchParams, 'config'>>;
let es: any;
let searchArgs: MockedKeys<SearchStrategySearchParams>;
let http: jest.Mocked<HttpStart>;

beforeEach(() => {
msearchMockResponse.abort.mockClear();
msearchMock.mockClear();

searchMockResponse.abort.mockClear();
searchMock.mockClear();

const searchService = searchServiceMock.createStartContract();
searchService.aggs.calculateAutoTimeExpression = jest.fn().mockReturnValue('1d');
searchService.__LEGACY.esClient.search = searchMock;
searchService.__LEGACY.esClient.msearch = msearchMock;
http = coreMock.createStart().http;
http.post.mockResolvedValue(msearchMock);

searchArgs = {
searchRequests: [
{
index: { title: 'foo' },
},
],
esShardTimeout: 0,
legacySearchService: searchService.__LEGACY,
http,
config: {
get: jest.fn(),
},
loadingCount$: new BehaviorSubject(0) as any,
};

es = searchArgs.legacySearchService.esClient;
});

test('does not send max_concurrent_shard_requests by default', async () => {
const config = getConfigStub({ [UI_SETTINGS.COURIER_BATCH_SEARCHES]: true });
await search({ ...searchArgs, config });
expect(es.msearch.mock.calls[0][0].max_concurrent_shard_requests).toBe(undefined);
});

test('allows configuration of max_concurrent_shard_requests', async () => {
const config = getConfigStub({
[UI_SETTINGS.COURIER_BATCH_SEARCHES]: true,
[UI_SETTINGS.COURIER_MAX_CONCURRENT_SHARD_REQUESTS]: 42,
});
await search({ ...searchArgs, config });
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({ [UI_SETTINGS.COURIER_BATCH_SEARCHES]: true });
await search({ ...searchArgs, config });
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 () => {
const config = getConfigStub({
[UI_SETTINGS.COURIER_BATCH_SEARCHES]: true,
[UI_SETTINGS.SEARCH_INCLUDE_FROZEN]: true,
});
await search({ ...searchArgs, config });
expect(es.msearch.mock.calls[0][0]).toHaveProperty('ignore_throttled', false);
});

test('should properly call abort with msearch', () => {
const config = getConfigStub({
[UI_SETTINGS.COURIER_BATCH_SEARCHES]: true,
});
search({ ...searchArgs, config }).abort();
expect(msearchMockResponse.abort).toHaveBeenCalled();
test('calls http.post with the correct arguments', async () => {
await search({ ...searchArgs });
expect(http.post.mock.calls).toMatchInlineSnapshot(`
Array [
Array [
"/internal/_msearch",
Object {
"body": "{\\"searches\\":[{\\"header\\":{\\"index\\":\\"foo\\"}}]}",
"signal": AbortSignal {},
},
],
]
`);
});
});
});
62 changes: 36 additions & 26 deletions src/plugins/data/public/search/legacy/default_search_strategy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,7 @@
* under the License.
*/

import { getPreference, getTimeout } from '../fetch';
import { getMSearchParams } from './get_msearch_params';
import { getPreference } from '../fetch';
import { SearchStrategyProvider, SearchStrategySearchParams } from './types';

// @deprecated
Expand All @@ -30,34 +29,45 @@ export const defaultSearchStrategy: SearchStrategyProvider = {
},
};

function msearch({
searchRequests,
legacySearchService,
config,
esShardTimeout,
}: SearchStrategySearchParams) {
const es = legacySearchService.esClient;
const inlineRequests = searchRequests.map(({ index, body, search_type: searchType }) => {
const inlineHeader = {
index: index.title || index,
search_type: searchType,
ignore_unavailable: true,
preference: getPreference(config.get),
function msearch({ searchRequests, config, http, loadingCount$ }: SearchStrategySearchParams) {
const requests = searchRequests.map(({ index, body }) => {
return {
header: {
index: index.title || index,
preference: getPreference(config.get),
},
body,
};
const inlineBody = {
...body,
timeout: getTimeout(esShardTimeout),
};
return `${JSON.stringify(inlineHeader)}\n${JSON.stringify(inlineBody)}`;
});

const searching = es.msearch({
...getMSearchParams(config.get),
body: `${inlineRequests.join('\n')}\n`,
});
const abortController = new AbortController();
let resolved = false;

// Start LoadingIndicator
loadingCount$.next(loadingCount$.getValue() + 1);

const cleanup = () => {
if (!resolved) {
resolved = true;
// Decrement loading counter & cleanup BehaviorSubject
loadingCount$.next(loadingCount$.getValue() - 1);
loadingCount$.complete();
}
};

const searching = http
.post('/internal/_msearch', {
body: JSON.stringify({ searches: requests }),
signal: abortController.signal,
})
.then(({ body }) => body?.responses)
.finally(() => cleanup());

return {
searching: searching.then(({ responses }: any) => responses),
abort: searching.abort,
abort: () => {
abortController.abort();
cleanup();
},
searching,
};
}
Loading

0 comments on commit d494f1e

Please sign in to comment.