Skip to content

Commit

Permalink
[Search] Refactor service to register search strategies, not providers (
Browse files Browse the repository at this point in the history
elastic#60342) (elastic#68545)

* Add async search strategy

* Add async search

* Fix async strategy and add tests

* Move types to separate file

* Revert changes to demo search

* Update demo search strategy to use async

* Add async es search strategy

* Return response as rawResponse

* Poll after initial request

* Add cancellation to search strategies

* Add tests

* Simplify async search strategy

* Move loadingCount to search strategy

* Update abort controller library

* Bootstrap

* Abort when the request is aborted

* Add utility and update value suggestions route

* Fix bad merge conflict

* Update tests

* Move to data_enhanced plugin

* Remove bad merge

* Revert switching abort controller libraries

* Revert package.json in lib

* Move to previous abort controller

* Add support for frozen indices

* Fix test to use fake timers to run debounced handlers

* Revert changes to example plugin

* Fix loading bar not going away when cancelling

* Call getSearchStrategy instead of passing  directly

* Add async demo search strategy

* Fix error with setting state

* Update how aborting works

* Fix type checks

* Add test for loading count

* Attempt to fix broken example test

* Revert changes to test

* Fix test

* Update name to camelCase

* Fix failing test

* Don't require data_enhanced in example plugin

* Actually send DELETE request

* Use waitForCompletion parameter

* Use default search params

* Add support for rollups

* Only make changes needed for frozen indices/rollups

* Only make changes needed for frozen indices/rollups

* Add back in async functionality

* Fix tests/types

* Fix issue with sending empty body in GET

* Don't include skipped in loaded/total

* Don't wait before polling the next time

* Add search interceptor for bulk managing searches

* Simplify search logic

* Fix merge error

* Review feedback

* UI to stop async searches

* Add service for running beyond timeout

* Refactor abort utils

* Remove unneeded changes

* Add tests

* Refactor search service to register strategies directly

* Remove accidental change

* re-generate docs

* Fix merge

* types

* doc

* eslint

* Fix async strategy jest test

* type fix

* Use getStartServices in search strategies

* Code review + snapshot

* eslint

* Type script

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
Co-authored-by: Liza K <liza.katz@elastic.co>

Co-authored-by: Lukas Olson <olson.lukas@gmail.com>
Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
  • Loading branch information
3 people authored Jun 8, 2020
1 parent 6db2907 commit 503321b
Show file tree
Hide file tree
Showing 27 changed files with 329 additions and 434 deletions.

This file was deleted.

This file was deleted.

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,6 @@
| [IndexPatternTypeMeta](./kibana-plugin-plugins-data-public.indexpatterntypemeta.md) | |
| [IRequestTypesMap](./kibana-plugin-plugins-data-public.irequesttypesmap.md) | |
| [IResponseTypesMap](./kibana-plugin-plugins-data-public.iresponsetypesmap.md) | |
| [ISearchContext](./kibana-plugin-plugins-data-public.isearchcontext.md) | |
| [ISearchOptions](./kibana-plugin-plugins-data-public.isearchoptions.md) | |
| [ISearchStrategy](./kibana-plugin-plugins-data-public.isearchstrategy.md) | Search strategy interface contains a search method that takes in a request and returns a promise that resolves to a response. |
| [ISyncSearchRequest](./kibana-plugin-plugins-data-public.isyncsearchrequest.md) | |
Expand Down Expand Up @@ -157,5 +156,4 @@
| [TabbedAggRow](./kibana-plugin-plugins-data-public.tabbedaggrow.md) | \* |
| [TimefilterContract](./kibana-plugin-plugins-data-public.timefiltercontract.md) | |
| [TimeHistoryContract](./kibana-plugin-plugins-data-public.timehistorycontract.md) | |
| [TSearchStrategyProvider](./kibana-plugin-plugins-data-public.tsearchstrategyprovider.md) | Search strategy provider creates an instance of a search strategy with the request handler context bound to it. This way every search strategy can use whatever information they require from the request context. |

This file was deleted.

68 changes: 21 additions & 47 deletions examples/demo_search/public/async_demo_search_strategy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,53 +17,27 @@
* under the License.
*/

import { Observable } from 'rxjs';
import {
ISearchContext,
TSearchStrategyProvider,
ISearchStrategy,
} from '../../../src/plugins/data/public';

import { ASYNC_DEMO_SEARCH_STRATEGY, IAsyncDemoResponse } from '../common';
import { Observable, from } from 'rxjs';
import { CoreSetup } from 'kibana/public';
import { flatMap } from 'rxjs/operators';
import { ISearch } from '../../../src/plugins/data/public';
import { ASYNC_SEARCH_STRATEGY } from '../../../x-pack/plugins/data_enhanced/public';
import { ASYNC_DEMO_SEARCH_STRATEGY, IAsyncDemoResponse } from '../common';
import { DemoDataSearchStartDependencies } from './types';

/**
* This demo search strategy provider simply provides a shortcut for calling the DEMO_ASYNC_SEARCH_STRATEGY
* on the server side, without users having to pass it in explicitly, and it takes advantage of the
* already registered ASYNC_SEARCH_STRATEGY that exists on the client.
*
* so instead of callers having to do:
*
* ```
* search(
* { ...request, serverStrategy: DEMO_ASYNC_SEARCH_STRATEGY },
* options,
* ASYNC_SEARCH_STRATEGY
* ) as Observable<IDemoResponse>,
*```
* They can instead just do
*
* ```
* search(request, options, DEMO_ASYNC_SEARCH_STRATEGY);
* ```
*
* and are ensured type safety in regard to the request and response objects.
*
* @param context - context supplied by other plugins.
* @param search - a search function to access other strategies that have already been registered.
*/
export const asyncDemoClientSearchStrategyProvider: TSearchStrategyProvider<typeof ASYNC_DEMO_SEARCH_STRATEGY> = (
context: ISearchContext
): ISearchStrategy<typeof ASYNC_DEMO_SEARCH_STRATEGY> => {
const asyncStrategyProvider = context.getSearchStrategy(ASYNC_SEARCH_STRATEGY);
const { search } = asyncStrategyProvider(context);
return {
search: (request, options) => {
return search(
{ ...request, serverStrategy: ASYNC_DEMO_SEARCH_STRATEGY },
options
) as Observable<IAsyncDemoResponse>;
},
export function asyncDemoClientSearchStrategyProvider(core: CoreSetup) {
const search: ISearch<typeof ASYNC_DEMO_SEARCH_STRATEGY> = (request, options) => {
return from(core.getStartServices()).pipe(
flatMap((startServices) => {
const asyncStrategy = (startServices[1] as DemoDataSearchStartDependencies).data.search.getSearchStrategy(
ASYNC_SEARCH_STRATEGY
);
return asyncStrategy.search(
{ ...request, serverStrategy: ASYNC_DEMO_SEARCH_STRATEGY },
options
) as Observable<IAsyncDemoResponse>;
})
);
};
};
return { search };
}
43 changes: 22 additions & 21 deletions examples/demo_search/public/demo_search_strategy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,12 @@
* under the License.
*/

import { Observable } from 'rxjs';
import { ISearchContext, SYNC_SEARCH_STRATEGY } from '../../../src/plugins/data/public';
import { TSearchStrategyProvider, ISearchStrategy } from '../../../src/plugins/data/public';

import { Observable, from } from 'rxjs';
import { flatMap } from 'rxjs/operators';
import { CoreSetup } from 'kibana/public';
import { ISearch, SYNC_SEARCH_STRATEGY } from '../../../src/plugins/data/public';
import { DEMO_SEARCH_STRATEGY, IDemoResponse } from '../common';
import { DemoDataSearchStartDependencies } from './types';

/**
* This demo search strategy provider simply provides a shortcut for calling the DEMO_SEARCH_STRATEGY
Expand All @@ -31,7 +32,7 @@ import { DEMO_SEARCH_STRATEGY, IDemoResponse } from '../common';
* so instead of callers having to do:
*
* ```
* context.search(
* data.search.search(
* { ...request, serverStrategy: DEMO_SEARCH_STRATEGY },
* options,
* SYNC_SEARCH_STRATEGY
Expand All @@ -41,24 +42,24 @@ import { DEMO_SEARCH_STRATEGY, IDemoResponse } from '../common';
* They can instead just do
*
* ```
* context.search(request, options, DEMO_SEARCH_STRATEGY);
* data.search.search(request, options, DEMO_SEARCH_STRATEGY);
* ```
*
* and are ensured type safety in regard to the request and response objects.
*
* @param context - context supplied by other plugins.
* @param search - a search function to access other strategies that have already been registered.
*/
export const demoClientSearchStrategyProvider: TSearchStrategyProvider<typeof DEMO_SEARCH_STRATEGY> = (
context: ISearchContext
): ISearchStrategy<typeof DEMO_SEARCH_STRATEGY> => {
const syncStrategyProvider = context.getSearchStrategy(SYNC_SEARCH_STRATEGY);
const { search } = syncStrategyProvider(context);
return {
search: (request, options) => {
return search({ ...request, serverStrategy: DEMO_SEARCH_STRATEGY }, options) as Observable<
IDemoResponse
>;
},
export function demoClientSearchStrategyProvider(core: CoreSetup) {
const search: ISearch<typeof DEMO_SEARCH_STRATEGY> = (request, options) => {
return from(core.getStartServices()).pipe(
flatMap((startServices) => {
const syncStrategy = (startServices[1] as DemoDataSearchStartDependencies).data.search.getSearchStrategy(
SYNC_SEARCH_STRATEGY
);
return syncStrategy.search(
{ ...request, serverStrategy: DEMO_SEARCH_STRATEGY },
options
) as Observable<IDemoResponse>;
})
);
};
};
return { search };
}
23 changes: 8 additions & 15 deletions examples/demo_search/public/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
* under the License.
*/

import { DataPublicPluginSetup } from '../../../src/plugins/data/public';
import { Plugin, CoreSetup } from '../../../src/core/public';
import {
DEMO_SEARCH_STRATEGY,
Expand All @@ -29,10 +28,7 @@ import {
} from '../common';
import { demoClientSearchStrategyProvider } from './demo_search_strategy';
import { asyncDemoClientSearchStrategyProvider } from './async_demo_search_strategy';

interface DemoDataSearchSetupDependencies {
data: DataPublicPluginSetup;
}
import { DemoDataSearchSetupDependencies, DemoDataSearchStartDependencies } from './types';

/**
* Add the typescript mappings for our search strategy to the request and
Expand All @@ -55,16 +51,13 @@ declare module '../../../src/plugins/data/public' {
}
}

export class DemoDataPlugin implements Plugin {
public setup(core: CoreSetup, deps: DemoDataSearchSetupDependencies) {
deps.data.search.registerSearchStrategyProvider(
DEMO_SEARCH_STRATEGY,
demoClientSearchStrategyProvider
);
deps.data.search.registerSearchStrategyProvider(
ASYNC_DEMO_SEARCH_STRATEGY,
asyncDemoClientSearchStrategyProvider
);
export class DemoDataPlugin
implements Plugin<void, void, DemoDataSearchSetupDependencies, DemoDataSearchStartDependencies> {
public setup(core: CoreSetup, { data }: DemoDataSearchSetupDependencies) {
const demoClientSearchStrategy = demoClientSearchStrategyProvider(core);
const asyncDemoClientSearchStrategy = asyncDemoClientSearchStrategyProvider(core);
data.search.registerSearchStrategy(DEMO_SEARCH_STRATEGY, demoClientSearchStrategy);
data.search.registerSearchStrategy(ASYNC_DEMO_SEARCH_STRATEGY, asyncDemoClientSearchStrategy);
}

public start() {}
Expand Down
28 changes: 28 additions & 0 deletions examples/demo_search/public/types.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
/*
* 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.
*/

import { DataPublicPluginStart, DataPublicPluginSetup } from '../../../src/plugins/data/public';

export interface DemoDataSearchSetupDependencies {
data: DataPublicPluginSetup;
}

export interface DemoDataSearchStartDependencies {
data: DataPublicPluginStart;
}
2 changes: 0 additions & 2 deletions src/plugins/data/public/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -341,8 +341,6 @@ export {
SYNC_SEARCH_STRATEGY,
getEsPreference,
getSearchErrorType,
ISearchContext,
TSearchStrategyProvider,
ISearchStrategy,
ISearch,
ISearchOptions,
Expand Down
12 changes: 9 additions & 3 deletions src/plugins/data/public/mocks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,17 @@ import { Plugin, IndexPatternsContract } from '.';
import { fieldFormatsServiceMock } from './field_formats/mocks';
import { searchSetupMock, searchStartMock } from './search/mocks';
import { queryServiceMock } from './query/mocks';
import { AutocompleteStart, AutocompleteSetup } from './autocomplete';

export type Setup = jest.Mocked<ReturnType<Plugin['setup']>>;
export type Start = jest.Mocked<ReturnType<Plugin['start']>>;

const autocompleteMock: any = {
const automcompleteSetupMock: jest.Mocked<AutocompleteSetup> = {
addQuerySuggestionProvider: jest.fn(),
getQuerySuggestions: jest.fn(),
};

const autocompleteStartMock: jest.Mocked<AutocompleteStart> = {
getValueSuggestions: jest.fn(),
getQuerySuggestions: jest.fn(),
hasQuerySuggestions: jest.fn(),
Expand All @@ -34,7 +40,7 @@ const autocompleteMock: any = {
const createSetupContract = (): Setup => {
const querySetupMock = queryServiceMock.createSetupContract();
return {
autocomplete: autocompleteMock,
autocomplete: automcompleteSetupMock,
search: searchSetupMock,
fieldFormats: fieldFormatsServiceMock.createSetupContract(),
query: querySetupMock,
Expand All @@ -48,7 +54,7 @@ const createStartContract = (): Start => {
createFiltersFromValueClickAction: jest.fn().mockResolvedValue(['yes']),
createFiltersFromRangeSelectAction: jest.fn(),
},
autocomplete: autocompleteMock,
autocomplete: autocompleteStartMock,
search: searchStartMock,
fieldFormats: fieldFormatsServiceMock.createStartContract(),
query: queryStartMock,
Expand Down
Loading

0 comments on commit 503321b

Please sign in to comment.