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

Don't overwrite sync strategy in xpack #75556

Merged
merged 7 commits into from
Aug 25, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

```typescript
setup(core: CoreSetup<DataPluginStartDependencies, DataPluginStart>, { expressions, usageCollection }: DataPluginSetupDependencies): {
__enhance: (enhancements: DataEnhancements) => void;
search: ISearchSetup;
fieldFormats: {
register: (customFieldFormat: import("../public").FieldFormatInstanceType) => number;
Expand All @@ -25,6 +26,7 @@ setup(core: CoreSetup<DataPluginStartDependencies, DataPluginStart>, { expressio
<b>Returns:</b>

`{
__enhance: (enhancements: DataEnhancements) => void;
search: ISearchSetup;
fieldFormats: {
register: (customFieldFormat: import("../public").FieldFormatInstanceType) => number;
Expand Down
3 changes: 0 additions & 3 deletions src/plugins/data/common/search/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,6 @@ export * from './expressions';
export * from './tabify';
export * from './types';

import { ES_SEARCH_STRATEGY } from './es_search';
export const DEFAULT_SEARCH_STRATEGY = ES_SEARCH_STRATEGY;

export {
IEsSearchRequest,
IEsSearchResponse,
Expand Down
23 changes: 18 additions & 5 deletions src/plugins/data/server/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import { PluginInitializerContext, CoreSetup, CoreStart, Plugin, Logger } from '
import { ExpressionsServerSetup } from 'src/plugins/expressions/server';
import { ConfigSchema } from '../config';
import { IndexPatternsService, IndexPatternsServiceStart } from './index_patterns';
import { ISearchSetup, ISearchStart } from './search';
import { ISearchSetup, ISearchStart, SearchEnhancements } from './search';
import { SearchService } from './search/search_service';
import { QueryService } from './query/query_service';
import { ScriptsService } from './scripts';
Expand All @@ -31,9 +31,17 @@ import { AutocompleteService } from './autocomplete';
import { FieldFormatsService, FieldFormatsSetup, FieldFormatsStart } from './field_formats';
import { getUiSettings } from './ui_settings';

export interface DataEnhancements {
search: SearchEnhancements;
}

export interface DataPluginSetup {
search: ISearchSetup;
fieldFormats: FieldFormatsSetup;
/**
* @internal
*/
__enhance: (enhancements: DataEnhancements) => void;
}

export interface DataPluginStart {
Expand Down Expand Up @@ -87,11 +95,16 @@ export class DataServerPlugin

core.uiSettings.register(getUiSettings());

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

return {
search: this.searchService.setup(core, {
registerFunction: expressions.registerFunction,
usageCollection,
}),
__enhance: (enhancements: DataEnhancements) => {
searchSetup.__enhance(enhancements.search);
},
search: searchSetup,
fieldFormats: this.fieldFormats.setup(),
};
}
Expand Down
8 changes: 7 additions & 1 deletion src/plugins/data/server/search/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,13 @@
* under the License.
*/

export { ISearchStrategy, ISearchOptions, ISearchSetup, ISearchStart } from './types';
export {
ISearchStrategy,
ISearchOptions,
ISearchSetup,
ISearchStart,
SearchEnhancements,
} from './types';

export { getDefaultSearchParams, getTotalLoaded } from './es_search';

Expand Down
1 change: 1 addition & 0 deletions src/plugins/data/server/search/mocks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ export function createSearchSetupMock(): jest.Mocked<ISearchSetup> {
return {
aggs: searchAggsSetupMock(),
registerSearchStrategy: jest.fn(),
__enhance: jest.fn(),
};
}

Expand Down
16 changes: 10 additions & 6 deletions src/plugins/data/server/search/search_service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import {
PluginInitializerContext,
RequestHandlerContext,
} from '../../../../core/server';
import { ISearchSetup, ISearchStart, ISearchStrategy } from './types';
import { ISearchSetup, ISearchStart, ISearchStrategy, SearchEnhancements } from './types';

import { AggsService, AggsSetupDependencies } from './aggs';

Expand Down Expand Up @@ -57,6 +57,7 @@ export interface SearchServiceStartDependencies {

export class SearchService implements Plugin<ISearchSetup, ISearchStart> {
private readonly aggsService = new AggsService();
private defaultSearchStrategyName: string = ES_SEARCH_STRATEGY;
private searchStrategies: StrategyMap<any, any> = {};

constructor(
Expand Down Expand Up @@ -87,6 +88,11 @@ export class SearchService implements Plugin<ISearchSetup, ISearchStart> {
registerSearchRoute(core);

return {
__enhance: (enhancements: SearchEnhancements) => {
if (this.searchStrategies.hasOwnProperty(enhancements.defaultStrategy)) {
this.defaultSearchStrategyName = enhancements.defaultStrategy;
}
},
aggs: this.aggsService.setup({ registerFunction }),
registerSearchStrategy: this.registerSearchStrategy,
usage,
Expand All @@ -98,11 +104,9 @@ export class SearchService implements Plugin<ISearchSetup, ISearchStart> {
searchRequest: IEsSearchRequest,
options: Record<string, any>
) {
return this.getSearchStrategy(options.strategy || ES_SEARCH_STRATEGY).search(
context,
searchRequest,
{ signal: options.signal }
);
return this.getSearchStrategy(
options.strategy || this.defaultSearchStrategyName
).search(context, searchRequest, { signal: options.signal });
}

public start(
Expand Down
9 changes: 9 additions & 0 deletions src/plugins/data/server/search/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,10 @@ import { AggsSetup, AggsStart } from './aggs';
import { SearchUsage } from './collectors/usage';
import { IEsSearchRequest, IEsSearchResponse } from './es_search';

export interface SearchEnhancements {
defaultStrategy: string;
}

export interface ISearchOptions {
/**
* An `AbortSignal` that allows the caller of `search` to abort a search request.
Expand All @@ -49,6 +53,11 @@ export interface ISearchSetup {
* Used internally for telemetry
*/
usage?: SearchUsage;

/**
* @internal
*/
__enhance: (enhancements: SearchEnhancements) => void;
}

export interface ISearchStart<
Expand Down
8 changes: 8 additions & 0 deletions src/plugins/data/server/server.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -685,6 +685,10 @@ export interface ISearchOptions {
//
// @public (undocumented)
export interface ISearchSetup {
// Warning: (ae-forgotten-export) The symbol "SearchEnhancements" needs to be exported by the entry point index.d.ts
//
// @internal (undocumented)
__enhance: (enhancements: SearchEnhancements) => void;
// Warning: (ae-forgotten-export) The symbol "AggsSetup" needs to be exported by the entry point index.d.ts
//
// (undocumented)
Expand Down Expand Up @@ -855,6 +859,7 @@ export class Plugin implements Plugin_2<PluginSetup, PluginStart, DataPluginSetu
constructor(initializerContext: PluginInitializerContext_2<ConfigSchema>);
// (undocumented)
setup(core: CoreSetup<DataPluginStartDependencies, PluginStart>, { expressions, usageCollection }: DataPluginSetupDependencies): {
__enhance: (enhancements: DataEnhancements) => void;
search: ISearchSetup;
fieldFormats: {
register: (customFieldFormat: import("../public").FieldFormatInstanceType) => number;
Expand Down Expand Up @@ -883,6 +888,8 @@ export function plugin(initializerContext: PluginInitializerContext<ConfigSchema
//
// @public (undocumented)
export interface PluginSetup {
// @internal (undocumented)
__enhance: (enhancements: DataEnhancements) => void;
// Warning: (ae-forgotten-export) The symbol "FieldFormatsSetup" needs to be exported by the entry point index.d.ts
//
// (undocumented)
Expand Down Expand Up @@ -1090,6 +1097,7 @@ export function usageProvider(core: CoreSetup_2): SearchUsage;
// src/plugins/data/server/index.ts:240:1 - (ae-forgotten-export) The symbol "isValidInterval" needs to be exported by the entry point index.d.ts
// src/plugins/data/server/index.ts:244:1 - (ae-forgotten-export) The symbol "propFilter" needs to be exported by the entry point index.d.ts
// src/plugins/data/server/index.ts:247:1 - (ae-forgotten-export) The symbol "toAbsoluteDates" needs to be exported by the entry point index.d.ts
// src/plugins/data/server/plugin.ts:88:66 - (ae-forgotten-export) The symbol "DataEnhancements" needs to be exported by the entry point index.d.ts

// (No @packageDocumentation comment for this package)

Expand Down
7 changes: 6 additions & 1 deletion x-pack/plugins/data_enhanced/common/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,9 @@
* you may not use this file except in compliance with the Elastic License.
*/

export { EnhancedSearchParams, IEnhancedEsSearchRequest, IAsyncSearchRequest } from './search';
export {
EnhancedSearchParams,
IEnhancedEsSearchRequest,
IAsyncSearchRequest,
ENHANCED_ES_SEARCH_STRATEGY,
} from './search';
7 changes: 6 additions & 1 deletion x-pack/plugins/data_enhanced/common/search/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,9 @@
* you may not use this file except in compliance with the Elastic License.
*/

export { EnhancedSearchParams, IEnhancedEsSearchRequest, IAsyncSearchRequest } from './types';
export {
EnhancedSearchParams,
IEnhancedEsSearchRequest,
IAsyncSearchRequest,
ENHANCED_ES_SEARCH_STRATEGY,
} from './types';
2 changes: 2 additions & 0 deletions x-pack/plugins/data_enhanced/common/search/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@

import { IEsSearchRequest, ISearchRequestParams } from '../../../../../src/plugins/data/common';

export const ENHANCED_ES_SEARCH_STRATEGY = 'ese';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we give this a more meaningful name like enhanced_es or something?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wanted to keep it shorter, as this is what gets sent in the URL.
I think keeping it short is clearer, and we use the constant name elsewhere anyway.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And I did change it to use the enhance pattern. I agree it makes more sense this way.


export interface EnhancedSearchParams extends ISearchRequestParams {
ignoreThrottled: boolean;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import {
} from '../../../../../src/plugins/data/public';
import { AbortError, toPromise } from '../../../../../src/plugins/data/common';
import { IAsyncSearchOptions } from '.';
import { IAsyncSearchRequest } from '../../common';
import { IAsyncSearchRequest, ENHANCED_ES_SEARCH_STRATEGY } from '../../common';

export class EnhancedSearchInterceptor extends SearchInterceptor {
/**
Expand Down Expand Up @@ -76,10 +76,11 @@ export class EnhancedSearchInterceptor extends SearchInterceptor {

const { combinedSignal, cleanup } = this.setupTimers(options);
const aborted$ = from(toPromise(combinedSignal));
const strategy = options?.strategy || ENHANCED_ES_SEARCH_STRATEGY;

this.pendingCount$.next(this.pendingCount$.getValue() + 1);

return this.runSearch(request, combinedSignal, options?.strategy).pipe(
return this.runSearch(request, combinedSignal, strategy).pipe(
expand((response) => {
// If the response indicates of an error, stop polling and complete the observable
if (!response || (!response.isRunning && response.isPartial)) {
Expand All @@ -96,7 +97,7 @@ export class EnhancedSearchInterceptor extends SearchInterceptor {
return timer(pollInterval).pipe(
// Send future requests using just the ID from the response
mergeMap(() => {
return this.runSearch({ ...request, id }, combinedSignal, options?.strategy);
return this.runSearch({ ...request, id }, combinedSignal, strategy);
})
);
}),
Expand Down
10 changes: 8 additions & 2 deletions x-pack/plugins/data_enhanced/server/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,14 @@ import {
Plugin,
Logger,
} from '../../../../src/core/server';
import { ES_SEARCH_STRATEGY } from '../../../../src/plugins/data/common';
import {
PluginSetup as DataPluginSetup,
PluginStart as DataPluginStart,
usageProvider,
} from '../../../../src/plugins/data/server';
import { enhancedEsSearchStrategyProvider } from './search';
import { UsageCollectionSetup } from '../../../../src/plugins/usage_collection/server';
import { ENHANCED_ES_SEARCH_STRATEGY } from '../common';

interface SetupDependencies {
data: DataPluginSetup;
Expand All @@ -36,13 +36,19 @@ export class EnhancedDataServerPlugin implements Plugin<void, void, SetupDepende
const usage = deps.usageCollection ? usageProvider(core) : undefined;

deps.data.search.registerSearchStrategy(
ES_SEARCH_STRATEGY,
ENHANCED_ES_SEARCH_STRATEGY,
enhancedEsSearchStrategyProvider(
this.initializerContext.config.legacy.globalConfig$,
this.logger,
usage
)
);

deps.data.__enhance({
search: {
defaultStrategy: ENHANCED_ES_SEARCH_STRATEGY,
},
});
}

public start(core: CoreStart) {}
Expand Down