From 820a25621b8c6c1c165468671b700cad608f0ce3 Mon Sep 17 00:00:00 2001 From: Liza K Date: Thu, 5 Mar 2020 16:09:58 +0200 Subject: [PATCH] Old search strategy cleanup --- src/legacy/core_plugins/data/public/plugin.ts | 10 +- .../data/public/search/fetch/call_client.ts | 39 ++--- src/plugins/data/public/search/index.ts | 2 - .../public/search/search_strategy/index.ts | 7 +- .../search_strategy_registry.test.ts | 149 ------------------ .../search_strategy_registry.ts | 49 +----- .../vis_type_timeseries/server/plugin.ts | 2 - x-pack/legacy/plugins/rollup/public/legacy.ts | 2 - x-pack/legacy/plugins/rollup/public/plugin.ts | 4 +- .../server/lib/search_strategies/index.ts | 2 - .../register_rollup_search_strategy.ts | 29 ---- x-pack/legacy/plugins/rollup/server/plugin.ts | 6 - 12 files changed, 15 insertions(+), 286 deletions(-) delete mode 100644 src/plugins/data/public/search/search_strategy/search_strategy_registry.test.ts delete mode 100644 x-pack/legacy/plugins/rollup/server/lib/search_strategies/register_rollup_search_strategy.ts diff --git a/src/legacy/core_plugins/data/public/plugin.ts b/src/legacy/core_plugins/data/public/plugin.ts index 18230646ab412..f40cda8760bc7 100644 --- a/src/legacy/core_plugins/data/public/plugin.ts +++ b/src/legacy/core_plugins/data/public/plugin.ts @@ -18,12 +18,7 @@ */ import { CoreSetup, CoreStart, Plugin } from 'kibana/public'; -import { - DataPublicPluginStart, - addSearchStrategy, - defaultSearchStrategy, - DataPublicPluginSetup, -} from '../../../../plugins/data/public'; +import { DataPublicPluginStart, DataPublicPluginSetup } from '../../../../plugins/data/public'; import { ExpressionsSetup } from '../../../../plugins/expressions/public'; import { @@ -111,9 +106,6 @@ export class DataPlugin public setup(core: CoreSetup, { data, uiActions }: DataPluginSetupDependencies) { setInjectedMetadata(core.injectedMetadata); - // This is to be deprecated once we switch to the new search service fully - addSearchStrategy(defaultSearchStrategy); - uiActions.attachAction( SELECT_RANGE_TRIGGER, selectRangeAction(data.query.filterManager, data.query.timefilter.timefilter) diff --git a/src/plugins/data/public/search/fetch/call_client.ts b/src/plugins/data/public/search/fetch/call_client.ts index 6cc58b05ea183..b3c4c682fa60c 100644 --- a/src/plugins/data/public/search/fetch/call_client.ts +++ b/src/plugins/data/public/search/fetch/call_client.ts @@ -17,10 +17,9 @@ * under the License. */ -import { groupBy } from 'lodash'; import { handleResponse } from './handle_response'; import { FetchOptions, FetchHandlers } from './types'; -import { getSearchStrategyForSearchRequest, getSearchStrategyById } from '../search_strategy'; +import { defaultSearchStrategy } from '../search_strategy'; import { SearchRequest } from '..'; export function callClient( @@ -34,34 +33,18 @@ export function callClient( FetchOptions ]> = searchRequests.map((request, i) => [request, requestsOptions[i]]); const requestOptionsMap = new Map(requestOptionEntries); - - // Group the requests by the strategy used to search that specific request - const searchStrategyMap = groupBy(searchRequests, (request, i) => { - const searchStrategy = getSearchStrategyForSearchRequest(request, requestsOptions[i]); - return searchStrategy.id; - }); - - // Execute each search strategy with the group of requests, but return the responses in the same - // order in which they were received. We use a map to correlate the original request with its - // response. const requestResponseMap = new Map(); - Object.keys(searchStrategyMap).forEach(searchStrategyId => { - const searchStrategy = getSearchStrategyById(searchStrategyId); - const requests = searchStrategyMap[searchStrategyId]; - // There's no way `searchStrategy` could be undefined here because if we didn't get a matching strategy for this ID - // then an error would have been thrown above - const { searching, abort } = searchStrategy!.search({ - searchRequests: requests, - ...fetchHandlers, - }); + const { searching, abort } = defaultSearchStrategy.search({ + searchRequests, + ...fetchHandlers, + }); - requests.forEach((request, i) => { - const response = searching.then(results => handleResponse(request, results[i])); - const { abortSignal = null } = requestOptionsMap.get(request) || {}; - if (abortSignal) abortSignal.addEventListener('abort', abort); - requestResponseMap.set(request, response); - }); - }, []); + searchRequests.forEach((request, i) => { + const response = searching.then(results => handleResponse(request, results[i])); + const { abortSignal = null } = requestOptionsMap.get(request) || {}; + if (abortSignal) abortSignal.addEventListener('abort', abort); + requestResponseMap.set(request, response); + }); return searchRequests.map(request => requestResponseMap.get(request)); } diff --git a/src/plugins/data/public/search/index.ts b/src/plugins/data/public/search/index.ts index 2a54cfe2be785..ca586bb09a8d1 100644 --- a/src/plugins/data/public/search/index.ts +++ b/src/plugins/data/public/search/index.ts @@ -43,9 +43,7 @@ export { IKibanaSearchResponse, IKibanaSearchRequest } from '../../common/search export { LegacyApiCaller, SearchRequest, SearchResponse } from './es_client'; export { - addSearchStrategy, hasSearchStategyForIndexPattern, - defaultSearchStrategy, SearchError, SearchStrategyProvider, getSearchErrorType, diff --git a/src/plugins/data/public/search/search_strategy/index.ts b/src/plugins/data/public/search/search_strategy/index.ts index 330e10d7d30e4..5feef5604582a 100644 --- a/src/plugins/data/public/search/search_strategy/index.ts +++ b/src/plugins/data/public/search/search_strategy/index.ts @@ -17,12 +17,7 @@ * under the License. */ -export { - addSearchStrategy, - hasSearchStategyForIndexPattern, - getSearchStrategyById, - getSearchStrategyForSearchRequest, -} from './search_strategy_registry'; +export { hasSearchStategyForIndexPattern } from './search_strategy_registry'; export { SearchError, getSearchErrorType } from './search_error'; diff --git a/src/plugins/data/public/search/search_strategy/search_strategy_registry.test.ts b/src/plugins/data/public/search/search_strategy/search_strategy_registry.test.ts deleted file mode 100644 index eaf86e1b270d5..0000000000000 --- a/src/plugins/data/public/search/search_strategy/search_strategy_registry.test.ts +++ /dev/null @@ -1,149 +0,0 @@ -/* - * 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 { IndexPattern } from '../..'; -import { noOpSearchStrategy } from './no_op_search_strategy'; -import { - searchStrategies, - addSearchStrategy, - getSearchStrategyByViability, - getSearchStrategyById, - getSearchStrategyForSearchRequest, - hasSearchStategyForIndexPattern, -} from './search_strategy_registry'; -import { SearchStrategyProvider } from './types'; - -const mockSearchStrategies: SearchStrategyProvider[] = [ - { - id: '0', - isViable: (index: IndexPattern) => index.id === '0', - search: () => ({ - searching: Promise.resolve([]), - abort: () => void 0, - }), - }, - { - id: '1', - isViable: (index: IndexPattern) => index.id === '1', - search: () => ({ - searching: Promise.resolve([]), - abort: () => void 0, - }), - }, -]; - -describe('Search strategy registry', () => { - beforeEach(() => { - searchStrategies.length = 0; - }); - - describe('addSearchStrategy', () => { - it('adds a search strategy', () => { - addSearchStrategy(mockSearchStrategies[0]); - expect(searchStrategies.length).toBe(1); - }); - - it('does not add a search strategy if it is already included', () => { - addSearchStrategy(mockSearchStrategies[0]); - addSearchStrategy(mockSearchStrategies[0]); - expect(searchStrategies.length).toBe(1); - }); - }); - - describe('getSearchStrategyByViability', () => { - beforeEach(() => { - mockSearchStrategies.forEach(addSearchStrategy); - }); - - it('returns the viable strategy', () => { - expect(getSearchStrategyByViability({ id: '0' } as IndexPattern)).toBe( - mockSearchStrategies[0] - ); - expect(getSearchStrategyByViability({ id: '1' } as IndexPattern)).toBe( - mockSearchStrategies[1] - ); - }); - - it('returns undefined if there is no viable strategy', () => { - expect(getSearchStrategyByViability({ id: '-1' } as IndexPattern)).toBe(undefined); - }); - }); - - describe('getSearchStrategyById', () => { - beforeEach(() => { - mockSearchStrategies.forEach(addSearchStrategy); - }); - - it('returns the strategy by ID', () => { - expect(getSearchStrategyById('0')).toBe(mockSearchStrategies[0]); - expect(getSearchStrategyById('1')).toBe(mockSearchStrategies[1]); - }); - - it('returns undefined if there is no strategy with that ID', () => { - expect(getSearchStrategyById('-1')).toBe(undefined); - }); - - it('returns the noOp search strategy if passed that ID', () => { - expect(getSearchStrategyById('noOp')).toBe(noOpSearchStrategy); - }); - }); - - describe('getSearchStrategyForSearchRequest', () => { - beforeEach(() => { - mockSearchStrategies.forEach(addSearchStrategy); - }); - - it('returns the strategy by ID if provided', () => { - expect(getSearchStrategyForSearchRequest({}, { searchStrategyId: '1' })).toBe( - mockSearchStrategies[1] - ); - }); - - it('throws if there is no strategy by provided ID', () => { - expect(() => - getSearchStrategyForSearchRequest({}, { searchStrategyId: '-1' }) - ).toThrowErrorMatchingInlineSnapshot(`"No strategy with ID -1"`); - }); - - it('returns the strategy by viability if there is one', () => { - expect( - getSearchStrategyForSearchRequest({ - index: { - id: '1', - }, - }) - ).toBe(mockSearchStrategies[1]); - }); - - it('returns the no op strategy if there is no viable strategy', () => { - expect(getSearchStrategyForSearchRequest({ index: '3' })).toBe(noOpSearchStrategy); - }); - }); - - describe('hasSearchStategyForIndexPattern', () => { - beforeEach(() => { - mockSearchStrategies.forEach(addSearchStrategy); - }); - - it('returns whether there is a search strategy for this index pattern', () => { - expect(hasSearchStategyForIndexPattern({ id: '0' } as IndexPattern)).toBe(true); - expect(hasSearchStategyForIndexPattern({ id: '-1' } as IndexPattern)).toBe(false); - }); - }); -}); diff --git a/src/plugins/data/public/search/search_strategy/search_strategy_registry.ts b/src/plugins/data/public/search/search_strategy/search_strategy_registry.ts index 1ab6f7d4e1eff..77c1d0c8dec23 100644 --- a/src/plugins/data/public/search/search_strategy/search_strategy_registry.ts +++ b/src/plugins/data/public/search/search_strategy/search_strategy_registry.ts @@ -18,54 +18,7 @@ */ import { IndexPattern } from '../..'; -import { SearchStrategyProvider } from './types'; -import { noOpSearchStrategy } from './no_op_search_strategy'; -import { SearchResponse } from '..'; - -export const searchStrategies: SearchStrategyProvider[] = []; - -export const addSearchStrategy = (searchStrategy: SearchStrategyProvider) => { - if (searchStrategies.includes(searchStrategy)) { - return; - } - - searchStrategies.push(searchStrategy); -}; - -export const getSearchStrategyByViability = (indexPattern: IndexPattern) => { - return searchStrategies.find(searchStrategy => { - return searchStrategy.isViable(indexPattern); - }); -}; - -export const getSearchStrategyById = (searchStrategyId: string) => { - return [...searchStrategies, noOpSearchStrategy].find(searchStrategy => { - return searchStrategy.id === searchStrategyId; - }); -}; - -export const getSearchStrategyForSearchRequest = ( - searchRequest: SearchResponse, - { searchStrategyId }: { searchStrategyId?: string } = {} -) => { - // Allow the searchSource to declare the correct strategy with which to execute its searches. - if (searchStrategyId != null) { - const strategy = getSearchStrategyById(searchStrategyId); - if (!strategy) throw Error(`No strategy with ID ${searchStrategyId}`); - return strategy; - } - - // Otherwise try to match it to a strategy. - const viableSearchStrategy = getSearchStrategyByViability(searchRequest.index); - - if (viableSearchStrategy) { - return viableSearchStrategy; - } - - // This search strategy automatically rejects with an error. - return noOpSearchStrategy; -}; export const hasSearchStategyForIndexPattern = (indexPattern: IndexPattern) => { - return Boolean(getSearchStrategyByViability(indexPattern)); + return true; }; diff --git a/src/plugins/vis_type_timeseries/server/plugin.ts b/src/plugins/vis_type_timeseries/server/plugin.ts index 6ef6362c6e37b..70ca41796dadb 100644 --- a/src/plugins/vis_type_timeseries/server/plugin.ts +++ b/src/plugins/vis_type_timeseries/server/plugin.ts @@ -52,7 +52,6 @@ export interface VisTypeTimeseriesSetup { fakeRequest: FakeRequest, options: GetVisDataOptions ) => ReturnType; - addSearchStrategy: SearchStrategyRegistry['addStrategy']; } export interface Framework { @@ -110,7 +109,6 @@ export class VisTypeTimeseriesPlugin implements Plugin { ) => { return await getVisData(requestContext, { ...fakeRequest, body: options }, framework); }, - addSearchStrategy: searchStrategyRegistry.addStrategy.bind(searchStrategyRegistry), }; } diff --git a/x-pack/legacy/plugins/rollup/public/legacy.ts b/x-pack/legacy/plugins/rollup/public/legacy.ts index e3e663ac7b0f4..e137799bd34fe 100644 --- a/x-pack/legacy/plugins/rollup/public/legacy.ts +++ b/x-pack/legacy/plugins/rollup/public/legacy.ts @@ -7,7 +7,6 @@ import { npSetup, npStart } from 'ui/new_platform'; import { aggTypeFilters } from 'ui/agg_types'; import { aggTypeFieldFilters } from 'ui/agg_types'; -import { addSearchStrategy } from '../../../../../src/plugins/data/public'; import { RollupPlugin } from './plugin'; import { setup as management } from '../../../../../src/legacy/core_plugins/management/public/legacy'; @@ -18,7 +17,6 @@ export const setup = plugin.setup(npSetup.core, { __LEGACY: { aggTypeFilters, aggTypeFieldFilters, - addSearchStrategy, managementLegacy: management, }, }); diff --git a/x-pack/legacy/plugins/rollup/public/plugin.ts b/x-pack/legacy/plugins/rollup/public/plugin.ts index a01383f4733ef..742393e57ae3b 100644 --- a/x-pack/legacy/plugins/rollup/public/plugin.ts +++ b/x-pack/legacy/plugins/rollup/public/plugin.ts @@ -37,7 +37,6 @@ export interface RollupPluginSetupDependencies { __LEGACY: { aggTypeFilters: AggTypeFilters; aggTypeFieldFilters: AggTypeFieldFilters; - addSearchStrategy: (searchStrategy: SearchStrategyProvider) => void; managementLegacy: ManagementSetupLegacy; }; home?: HomePublicPluginSetup; @@ -49,7 +48,7 @@ export class RollupPlugin implements Plugin { setup( core: CoreSetup, { - __LEGACY: { aggTypeFilters, aggTypeFieldFilters, addSearchStrategy, managementLegacy }, + __LEGACY: { aggTypeFilters, aggTypeFieldFilters, managementLegacy }, home, management, indexManagement, @@ -67,7 +66,6 @@ export class RollupPlugin implements Plugin { if (isRollupIndexPatternsEnabled) { managementLegacy.indexPattern.creation.add(RollupIndexPatternCreationConfig); managementLegacy.indexPattern.list.add(RollupIndexPatternListConfig); - addSearchStrategy(getRollupSearchStrategy(core.http.fetch)); initAggTypeFilter(aggTypeFilters); initAggTypeFieldFilter(aggTypeFieldFilters); } diff --git a/x-pack/legacy/plugins/rollup/server/lib/search_strategies/index.ts b/x-pack/legacy/plugins/rollup/server/lib/search_strategies/index.ts index 7db0b38ea29dd..41bc2aa258807 100644 --- a/x-pack/legacy/plugins/rollup/server/lib/search_strategies/index.ts +++ b/x-pack/legacy/plugins/rollup/server/lib/search_strategies/index.ts @@ -3,5 +3,3 @@ * or more contributor license agreements. Licensed under the Elastic License; * you may not use this file except in compliance with the Elastic License. */ - -export { registerRollupSearchStrategy } from './register_rollup_search_strategy'; diff --git a/x-pack/legacy/plugins/rollup/server/lib/search_strategies/register_rollup_search_strategy.ts b/x-pack/legacy/plugins/rollup/server/lib/search_strategies/register_rollup_search_strategy.ts deleted file mode 100644 index 93c4c1b52140b..0000000000000 --- a/x-pack/legacy/plugins/rollup/server/lib/search_strategies/register_rollup_search_strategy.ts +++ /dev/null @@ -1,29 +0,0 @@ -/* - * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one - * or more contributor license agreements. Licensed under the Elastic License; - * you may not use this file except in compliance with the Elastic License. - */ -import { getRollupSearchStrategy } from './rollup_search_strategy'; -import { getRollupSearchRequest } from './rollup_search_request'; -import { getRollupSearchCapabilities } from './rollup_search_capabilities'; -import { - AbstractSearchRequest, - DefaultSearchCapabilities, - AbstractSearchStrategy, -} from '../../../../../../../src/plugins/vis_type_timeseries/server'; -import { RouteDependencies } from '../../types'; - -export const registerRollupSearchStrategy = ( - { elasticsearchService }: RouteDependencies, - addSearchStrategy: (searchStrategy: any) => void -) => { - const RollupSearchRequest = getRollupSearchRequest(AbstractSearchRequest); - const RollupSearchCapabilities = getRollupSearchCapabilities(DefaultSearchCapabilities); - const RollupSearchStrategy = getRollupSearchStrategy( - AbstractSearchStrategy, - RollupSearchRequest, - RollupSearchCapabilities - ); - - addSearchStrategy(new RollupSearchStrategy(elasticsearchService)); -}; diff --git a/x-pack/legacy/plugins/rollup/server/plugin.ts b/x-pack/legacy/plugins/rollup/server/plugin.ts index 090cb8a47377a..94af3d723c88f 100644 --- a/x-pack/legacy/plugins/rollup/server/plugin.ts +++ b/x-pack/legacy/plugins/rollup/server/plugin.ts @@ -24,7 +24,6 @@ import { import { registerRollupUsageCollector } from './collectors'; import { rollupDataEnricher } from './rollup_data_enricher'; -import { registerRollupSearchStrategy } from './lib/search_strategies'; export class RollupsServerPlugin implements Plugin { log: Logger; @@ -82,11 +81,6 @@ export class RollupsServerPlugin implements Plugin { if (indexManagement && indexManagement.indexDataEnricher) { indexManagement.indexDataEnricher.add(rollupDataEnricher); } - - if (metrics) { - const { addSearchStrategy } = metrics; - registerRollupSearchStrategy(routeDependencies, addSearchStrategy); - } } start() {}