Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[data.search.SearchSource] Remove legacy ES client APIs. #75943

Merged
merged 22 commits into from
Sep 3, 2020
Merged
Show file tree
Hide file tree
Changes from 15 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -363,7 +363,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 @@ -29,6 +29,7 @@ import { ExpressionsSetup } from 'src/plugins/expressions/public';
import { FetchOptions as FetchOptions_2 } from 'src/plugins/data/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 { ISearchSource as ISearchSource_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 @@ -35,9 +36,9 @@ export interface FetchOptions {
}

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,
lizozom marked this conversation as resolved.
Show resolved Hide resolved
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