From dcaacca5bd3017833f7833e1d5be81522dd3980a Mon Sep 17 00:00:00 2001 From: Anton Dosov Date: Mon, 11 Jan 2021 16:57:47 +0100 Subject: [PATCH 1/6] fix search sesion integration in vega, timelion, timeseries --- .../helpers/timelion_request_handler.ts | 21 ++++++++- .../public/timelion_vis_fn.ts | 3 +- .../vis_type_timelion/server/routes/run.ts | 2 + .../server/series_functions/es/es.test.js | 11 +++-- .../server/series_functions/es/index.js | 2 + .../vis_type_timeseries/common/vis_schema.ts | 5 +++ .../vis_type_timeseries/public/metrics_fn.ts | 3 +- .../public/request_handler.ts | 44 +++++++++++++------ .../strategies/abstract_search_strategy.ts | 4 +- .../public/data_model/search_api.ts | 8 ++-- src/plugins/vis_type_vega/public/vega_fn.ts | 1 + .../public/vega_request_handler.ts | 5 ++- 12 files changed, 82 insertions(+), 27 deletions(-) diff --git a/src/plugins/vis_type_timelion/public/helpers/timelion_request_handler.ts b/src/plugins/vis_type_timelion/public/helpers/timelion_request_handler.ts index 7a630f36b51f4..2b8e47a8c4893 100644 --- a/src/plugins/vis_type_timelion/public/helpers/timelion_request_handler.ts +++ b/src/plugins/vis_type_timelion/public/helpers/timelion_request_handler.ts @@ -73,12 +73,15 @@ export function getTimelionRequestHandler({ filters, query, visParams, + searchSessionId, }: { timeRange: TimeRange; filters: Filter[]; query: Query; visParams: TimelionVisParams; + searchSessionId?: string; }): Promise { + const dataSearch = getDataSearch(); const expression = visParams.expression; if (!expression) { @@ -93,7 +96,15 @@ export function getTimelionRequestHandler({ // parse the time range client side to make sure it behaves like other charts const timeRangeBounds = timefilter.calculateBounds(timeRange); - const sessionId = getDataSearch().session.getSessionId(); + const isCurrentSession = () => + !!searchSessionId && searchSessionId === dataSearch.session.getSessionId(); + const untrackSearch = + isCurrentSession() && + dataSearch.session.trackSearch({ + abort: () => { + // TODO: support search cancellations + }, + }); try { return await http.post('/api/timelion/run', { @@ -110,7 +121,9 @@ export function getTimelionRequestHandler({ interval: visParams.interval, timezone, }, - sessionId, + sessionId: searchSessionId, + isRestore: isCurrentSession() ? dataSearch.session.isRestore() : false, + isStored: isCurrentSession() ? dataSearch.session.isStored() : false, }), }); } catch (e) { @@ -125,6 +138,10 @@ export function getTimelionRequestHandler({ } else { throw e; } + } finally { + if (untrackSearch && isCurrentSession()) { + untrackSearch(); + } } }; } diff --git a/src/plugins/vis_type_timelion/public/timelion_vis_fn.ts b/src/plugins/vis_type_timelion/public/timelion_vis_fn.ts index 2e8878b11e915..6cc3f7fa05318 100644 --- a/src/plugins/vis_type_timelion/public/timelion_vis_fn.ts +++ b/src/plugins/vis_type_timelion/public/timelion_vis_fn.ts @@ -72,7 +72,7 @@ export const getTimelionVisualizationConfig = ( help: '', }, }, - async fn(input, args) { + async fn(input, args, { getSearchSessionId }) { const timelionRequestHandler = getTimelionRequestHandler(dependencies); const visParams = { expression: args.expression, interval: args.interval }; @@ -82,6 +82,7 @@ export const getTimelionVisualizationConfig = ( query: get(input, 'query') as Query, filters: get(input, 'filters') as Filter[], visParams, + searchSessionId: getSearchSessionId(), }); response.visType = TIMELION_VIS_NAME; diff --git a/src/plugins/vis_type_timelion/server/routes/run.ts b/src/plugins/vis_type_timelion/server/routes/run.ts index 5766705d9873d..8bb55b2f3215f 100644 --- a/src/plugins/vis_type_timelion/server/routes/run.ts +++ b/src/plugins/vis_type_timelion/server/routes/run.ts @@ -76,6 +76,8 @@ export function runRoute( }) ), sessionId: schema.maybe(schema.string()), + isRestore: schema.maybe(schema.boolean()), + isStored: schema.maybe(schema.boolean()), }), }, }, diff --git a/src/plugins/vis_type_timelion/server/series_functions/es/es.test.js b/src/plugins/vis_type_timelion/server/series_functions/es/es.test.js index f4ba36e4fdd67..8bf621bd55648 100644 --- a/src/plugins/vis_type_timelion/server/series_functions/es/es.test.js +++ b/src/plugins/vis_type_timelion/server/series_functions/es/es.test.js @@ -60,19 +60,24 @@ describe('es', () => { }); }); - test('should call data search with sessionId', async () => { + test('should call data search with sessionId, isRestore and isStored', async () => { tlConfig = { ...stubRequestAndServer({ rawResponse: esResponse }), request: { body: { - sessionId: 1, + sessionId: '1', + isRestore: true, + isStored: false, }, }, }; await invoke(es, [5], tlConfig); - expect(tlConfig.context.search.search.mock.calls[0][1]).toHaveProperty('sessionId', 1); + const res = tlConfig.context.search.search.mock.calls[0][1]; + expect(res).toHaveProperty('sessionId', '1'); + expect(res).toHaveProperty('isRestore', true); + expect(res).toHaveProperty('isStored', false); }); test('returns a seriesList', () => { diff --git a/src/plugins/vis_type_timelion/server/series_functions/es/index.js b/src/plugins/vis_type_timelion/server/series_functions/es/index.js index 24b3668b5cd3c..64e110a7f14f4 100644 --- a/src/plugins/vis_type_timelion/server/series_functions/es/index.js +++ b/src/plugins/vis_type_timelion/server/series_functions/es/index.js @@ -134,6 +134,8 @@ export default new Datasource('es', { body, { sessionId: tlConfig.request?.body.sessionId, + isRestore: tlConfig.request?.body.isRestore, + isStored: tlConfig.request?.body.isStored, }, tlConfig.context ) diff --git a/src/plugins/vis_type_timeseries/common/vis_schema.ts b/src/plugins/vis_type_timeseries/common/vis_schema.ts index a90fa752ad7dc..42e9b0aedaaed 100644 --- a/src/plugins/vis_type_timeseries/common/vis_schema.ts +++ b/src/plugins/vis_type_timeseries/common/vis_schema.ts @@ -281,5 +281,10 @@ export const visPayloadSchema = schema.object({ min: stringRequired, max: stringRequired, }), + + // search sessions integration start sessionId: schema.maybe(schema.string()), + isRestore: schema.maybe(schema.boolean()), + isStored: schema.maybe(schema.boolean()), + // search sessions integration end }); diff --git a/src/plugins/vis_type_timeseries/public/metrics_fn.ts b/src/plugins/vis_type_timeseries/public/metrics_fn.ts index 60acd35b22402..825b964270794 100644 --- a/src/plugins/vis_type_timeseries/public/metrics_fn.ts +++ b/src/plugins/vis_type_timeseries/public/metrics_fn.ts @@ -65,7 +65,7 @@ export const createMetricsFn = (): TimeseriesExpressionFunctionDefinition => ({ help: '', }, }, - async fn(input, args) { + async fn(input, args, { getSearchSessionId }) { const visParams: TimeseriesVisParams = JSON.parse(args.params); const uiState = JSON.parse(args.uiState); @@ -73,6 +73,7 @@ export const createMetricsFn = (): TimeseriesExpressionFunctionDefinition => ({ input, visParams, uiState, + searchSessionId: getSearchSessionId(), }); return { diff --git a/src/plugins/vis_type_timeseries/public/request_handler.ts b/src/plugins/vis_type_timeseries/public/request_handler.ts index aa45453515277..dfd6a912a606c 100644 --- a/src/plugins/vis_type_timeseries/public/request_handler.ts +++ b/src/plugins/vis_type_timeseries/public/request_handler.ts @@ -29,12 +29,14 @@ interface MetricsRequestHandlerParams { input: KibanaContext | null; uiState: Record; visParams: TimeseriesVisParams; + searchSessionId?: string; } export const metricsRequestHandler = async ({ input, uiState, visParams, + searchSessionId, }: MetricsRequestHandlerParams): Promise => { const config = getUISettings(); const timezone = getTimezone(config); @@ -47,21 +49,37 @@ export const metricsRequestHandler = async ({ validateInterval(parsedTimeRange, visParams, maxBuckets); - const resp = await getCoreStart().http.post(ROUTES.VIS_DATA, { - body: JSON.stringify({ - timerange: { - timezone, - ...parsedTimeRange, + const isCurrentSession = () => + !!searchSessionId && searchSessionId === dataSearch.search.session.getSessionId(); + const untrackSearch = + isCurrentSession() && + dataSearch.search.session.trackSearch({ + abort: () => { + // TODO: support search cancellations }, - query: input?.query, - filters: input?.filters, - panels: [visParams], - state: uiStateObj, - sessionId: dataSearch.search.session.getSessionId(), - }), - }); + }); - return resp; + try { + return await getCoreStart().http.post(ROUTES.VIS_DATA, { + body: JSON.stringify({ + timerange: { + timezone, + ...parsedTimeRange, + }, + query: input?.query, + filters: input?.filters, + panels: [visParams], + state: uiStateObj, + sessionId: searchSessionId, + isRestore: isCurrentSession() ? dataSearch.search.session.isRestore() : false, + isStored: isCurrentSession() ? dataSearch.search.session.isStored() : false, + }), + }); + } finally { + if (untrackSearch && isCurrentSession()) { + untrackSearch(); + } + } } return {}; diff --git a/src/plugins/vis_type_timeseries/server/lib/search_strategies/strategies/abstract_search_strategy.ts b/src/plugins/vis_type_timeseries/server/lib/search_strategies/strategies/abstract_search_strategy.ts index 71461d319f2b6..113eac49ea560 100644 --- a/src/plugins/vis_type_timeseries/server/lib/search_strategies/strategies/abstract_search_strategy.ts +++ b/src/plugins/vis_type_timeseries/server/lib/search_strategies/strategies/abstract_search_strategy.ts @@ -49,7 +49,7 @@ export interface ReqFacade extends FakeRequest { export abstract class AbstractSearchStrategy { async search(req: ReqFacade, bodies: any[], indexType?: string) { const requests: any[] = []; - const { sessionId } = req.payload; + const { sessionId, isRestore, isStored } = req.payload; bodies.forEach((body) => { requests.push( @@ -63,6 +63,8 @@ export abstract class AbstractSearchStrategy { }, { sessionId, + isRestore, + isStored, } ) .toPromise() diff --git a/src/plugins/vis_type_vega/public/data_model/search_api.ts b/src/plugins/vis_type_vega/public/data_model/search_api.ts index be58e3e3951eb..1a91079848eba 100644 --- a/src/plugins/vis_type_vega/public/data_model/search_api.ts +++ b/src/plugins/vis_type_vega/public/data_model/search_api.ts @@ -40,7 +40,8 @@ export class SearchAPI { constructor( private readonly dependencies: SearchAPIDependencies, private readonly abortSignal?: AbortSignal, - public readonly inspectorAdapters?: VegaInspectorAdapters + public readonly inspectorAdapters?: VegaInspectorAdapters, + private readonly searchSessionId?: string ) {} search(searchRequests: SearchRequest[]) { @@ -60,10 +61,7 @@ export class SearchAPI { } return search - .search( - { params }, - { abortSignal: this.abortSignal, sessionId: search.session.getSessionId() } - ) + .search({ params }, { abortSignal: this.abortSignal, sessionId: this.searchSessionId }) .pipe( tap((data) => this.inspectSearchResult(data, requestResponders[requestId])), map((data) => ({ diff --git a/src/plugins/vis_type_vega/public/vega_fn.ts b/src/plugins/vis_type_vega/public/vega_fn.ts index 5a8113aeeea11..70f01aa65f822 100644 --- a/src/plugins/vis_type_vega/public/vega_fn.ts +++ b/src/plugins/vis_type_vega/public/vega_fn.ts @@ -74,6 +74,7 @@ export const createVegaFn = ( query: get(input, 'query') as Query, filters: get(input, 'filters') as any, visParams: { spec: args.spec }, + searchSessionId: context.getSearchSessionId(), }); return { diff --git a/src/plugins/vis_type_vega/public/vega_request_handler.ts b/src/plugins/vis_type_vega/public/vega_request_handler.ts index f48b61ed70822..1f60feebe7efa 100644 --- a/src/plugins/vis_type_vega/public/vega_request_handler.ts +++ b/src/plugins/vis_type_vega/public/vega_request_handler.ts @@ -32,6 +32,7 @@ interface VegaRequestHandlerParams { filters: Filter; timeRange: TimeRange; visParams: VisParams; + searchSessionId?: string; } interface VegaRequestHandlerContext { @@ -52,6 +53,7 @@ export function createVegaRequestHandler( filters, query, visParams, + searchSessionId, }: VegaRequestHandlerParams) { if (!searchAPI) { searchAPI = new SearchAPI( @@ -61,7 +63,8 @@ export function createVegaRequestHandler( injectedMetadata: getInjectedMetadata(), }, context.abortSignal, - context.inspectorAdapters + context.inspectorAdapters, + searchSessionId ); } From 48c26bacf6e2674f7b0162571f8123f6031a4d1e Mon Sep 17 00:00:00 2001 From: Anton Dosov Date: Tue, 12 Jan 2021 11:05:18 +0100 Subject: [PATCH 2/6] review improvements --- src/plugins/data/public/public.api.md | 2 +- .../data/public/search/search_interceptor.ts | 6 +---- .../data/public/search/session/mocks.ts | 2 ++ .../public/search/session/session_service.ts | 24 +++++++++++++++++++ .../helpers/timelion_request_handler.ts | 11 ++++----- .../vis_type_timelion/server/routes/run.ts | 10 +++++--- .../server/series_functions/es/es.test.js | 8 ++++--- .../server/series_functions/es/index.js | 4 +--- .../vis_type_timeseries/common/vis_schema.ts | 12 ++++++---- .../public/request_handler.ts | 18 +++++++------- .../abstract_search_strategy.test.js | 10 ++++++-- .../strategies/abstract_search_strategy.ts | 7 +----- .../public/search/search_interceptor.ts | 17 ++++++++----- 13 files changed, 80 insertions(+), 51 deletions(-) diff --git a/src/plugins/data/public/public.api.md b/src/plugins/data/public/public.api.md index 27a40b4e5ffcb..05310925d6fc6 100644 --- a/src/plugins/data/public/public.api.md +++ b/src/plugins/data/public/public.api.md @@ -2620,7 +2620,7 @@ export const UI_SETTINGS: { // src/plugins/data/public/index.ts:433:1 - (ae-forgotten-export) The symbol "propFilter" needs to be exported by the entry point index.d.ts // src/plugins/data/public/index.ts:436:1 - (ae-forgotten-export) The symbol "toAbsoluteDates" needs to be exported by the entry point index.d.ts // src/plugins/data/public/query/state_sync/connect_to_query_state.ts:45:5 - (ae-forgotten-export) The symbol "FilterStateStore" needs to be exported by the entry point index.d.ts -// src/plugins/data/public/search/session/session_service.ts:50:5 - (ae-forgotten-export) The symbol "UrlGeneratorStateMapping" needs to be exported by the entry point index.d.ts +// src/plugins/data/public/search/session/session_service.ts:51:5 - (ae-forgotten-export) The symbol "UrlGeneratorStateMapping" needs to be exported by the entry point index.d.ts // (No @packageDocumentation comment for this package) diff --git a/src/plugins/data/public/search/search_interceptor.ts b/src/plugins/data/public/search/search_interceptor.ts index 8548a2a9f2b2a..584ec3c3e3dba 100644 --- a/src/plugins/data/public/search/search_interceptor.ts +++ b/src/plugins/data/public/search/search_interceptor.ts @@ -130,16 +130,12 @@ export class SearchInterceptor { ): Promise { const { abortSignal, ...requestOptions } = options || {}; - const isCurrentSession = - options?.sessionId && this.deps.session.getSessionId() === options.sessionId; - return this.batchedFetch( { request, options: { ...requestOptions, - isStored: isCurrentSession ? this.deps.session.isStored() : false, - isRestore: isCurrentSession ? this.deps.session.isRestore() : false, + ...this.deps.session.getSearchOptions(options?.sessionId), }, }, abortSignal diff --git a/src/plugins/data/public/search/session/mocks.ts b/src/plugins/data/public/search/session/mocks.ts index ea0cd8be03f27..13dd054c122d5 100644 --- a/src/plugins/data/public/search/session/mocks.ts +++ b/src/plugins/data/public/search/session/mocks.ts @@ -49,5 +49,7 @@ export function getSessionServiceMock(): jest.Mocked { isStored: jest.fn(), isRestore: jest.fn(), save: jest.fn(), + isCurrentSession: jest.fn(), + getSearchOptions: jest.fn(), }; } diff --git a/src/plugins/data/public/search/session/session_service.ts b/src/plugins/data/public/search/session/session_service.ts index 2bbb762fcfe9f..ac15958a87070 100644 --- a/src/plugins/data/public/search/session/session_service.ts +++ b/src/plugins/data/public/search/session/session_service.ts @@ -29,6 +29,7 @@ import { SessionStateContainer, } from './search_session_state'; import { ISessionsClient } from './sessions_client'; +import { ISearchOptions } from '../../../common'; export type ISessionService = PublicContract; @@ -243,4 +244,27 @@ export class SessionService { this.state.transitions.store(); } } + + /** + * Checks if passed sessionId is a current sessionId + * @param sessionId + */ + public isCurrentSession(sessionId?: string): boolean { + return !!sessionId && this.getSessionId() === sessionId; + } + + /** + * Infers search session options for sessionId using current session state + * @param sessionId + */ + public getSearchOptions( + sessionId?: string + ): Pick { + const currentSessionId = this.getSessionId(); + return { + sessionId, + isRestore: currentSessionId ? this.isRestore() : false, + isStored: currentSessionId ? this.isStored() : false, + }; + } } diff --git a/src/plugins/vis_type_timelion/public/helpers/timelion_request_handler.ts b/src/plugins/vis_type_timelion/public/helpers/timelion_request_handler.ts index 2b8e47a8c4893..4c336a5ee2c18 100644 --- a/src/plugins/vis_type_timelion/public/helpers/timelion_request_handler.ts +++ b/src/plugins/vis_type_timelion/public/helpers/timelion_request_handler.ts @@ -96,10 +96,8 @@ export function getTimelionRequestHandler({ // parse the time range client side to make sure it behaves like other charts const timeRangeBounds = timefilter.calculateBounds(timeRange); - const isCurrentSession = () => - !!searchSessionId && searchSessionId === dataSearch.session.getSessionId(); const untrackSearch = - isCurrentSession() && + dataSearch.session.isCurrentSession(searchSessionId) && dataSearch.session.trackSearch({ abort: () => { // TODO: support search cancellations @@ -121,9 +119,7 @@ export function getTimelionRequestHandler({ interval: visParams.interval, timezone, }, - sessionId: searchSessionId, - isRestore: isCurrentSession() ? dataSearch.session.isRestore() : false, - isStored: isCurrentSession() ? dataSearch.session.isStored() : false, + searchSession: dataSearch.session.getSearchOptions(searchSessionId), }), }); } catch (e) { @@ -139,7 +135,8 @@ export function getTimelionRequestHandler({ throw e; } } finally { - if (untrackSearch && isCurrentSession()) { + if (untrackSearch && dataSearch.session.isCurrentSession(searchSessionId)) { + // call `untrack` if this search still belongs to current session untrackSearch(); } } diff --git a/src/plugins/vis_type_timelion/server/routes/run.ts b/src/plugins/vis_type_timelion/server/routes/run.ts index 8bb55b2f3215f..e801bdd108be5 100644 --- a/src/plugins/vis_type_timelion/server/routes/run.ts +++ b/src/plugins/vis_type_timelion/server/routes/run.ts @@ -75,9 +75,13 @@ export function runRoute( to: schema.maybe(schema.string()), }) ), - sessionId: schema.maybe(schema.string()), - isRestore: schema.maybe(schema.boolean()), - isStored: schema.maybe(schema.boolean()), + searchSession: schema.maybe( + schema.object({ + sessionId: schema.maybe(schema.string()), + isRestore: schema.maybe(schema.boolean()), + isStored: schema.maybe(schema.boolean()), + }) + ), }), }, }, diff --git a/src/plugins/vis_type_timelion/server/series_functions/es/es.test.js b/src/plugins/vis_type_timelion/server/series_functions/es/es.test.js index 8bf621bd55648..b02baefea9919 100644 --- a/src/plugins/vis_type_timelion/server/series_functions/es/es.test.js +++ b/src/plugins/vis_type_timelion/server/series_functions/es/es.test.js @@ -65,9 +65,11 @@ describe('es', () => { ...stubRequestAndServer({ rawResponse: esResponse }), request: { body: { - sessionId: '1', - isRestore: true, - isStored: false, + searchSession: { + sessionId: '1', + isRestore: true, + isStored: false, + }, }, }, }; diff --git a/src/plugins/vis_type_timelion/server/series_functions/es/index.js b/src/plugins/vis_type_timelion/server/series_functions/es/index.js index 64e110a7f14f4..579563088a0a8 100644 --- a/src/plugins/vis_type_timelion/server/series_functions/es/index.js +++ b/src/plugins/vis_type_timelion/server/series_functions/es/index.js @@ -133,9 +133,7 @@ export default new Datasource('es', { .search( body, { - sessionId: tlConfig.request?.body.sessionId, - isRestore: tlConfig.request?.body.isRestore, - isStored: tlConfig.request?.body.isStored, + ...tlConfig.request?.body.searchSession, }, tlConfig.context ) diff --git a/src/plugins/vis_type_timeseries/common/vis_schema.ts b/src/plugins/vis_type_timeseries/common/vis_schema.ts index 42e9b0aedaaed..72ea17572bb9c 100644 --- a/src/plugins/vis_type_timeseries/common/vis_schema.ts +++ b/src/plugins/vis_type_timeseries/common/vis_schema.ts @@ -282,9 +282,11 @@ export const visPayloadSchema = schema.object({ max: stringRequired, }), - // search sessions integration start - sessionId: schema.maybe(schema.string()), - isRestore: schema.maybe(schema.boolean()), - isStored: schema.maybe(schema.boolean()), - // search sessions integration end + searchSession: schema.maybe( + schema.object({ + sessionId: schema.maybe(schema.string()), + isRestore: schema.maybe(schema.boolean()), + isStored: schema.maybe(schema.boolean()), + }) + ), }); diff --git a/src/plugins/vis_type_timeseries/public/request_handler.ts b/src/plugins/vis_type_timeseries/public/request_handler.ts index dfd6a912a606c..e9b8d2ef28983 100644 --- a/src/plugins/vis_type_timeseries/public/request_handler.ts +++ b/src/plugins/vis_type_timeseries/public/request_handler.ts @@ -41,19 +41,18 @@ export const metricsRequestHandler = async ({ const config = getUISettings(); const timezone = getTimezone(config); const uiStateObj = uiState[visParams.type] ?? {}; - const dataSearch = getDataStart(); - const parsedTimeRange = dataSearch.query.timefilter.timefilter.calculateBounds(input?.timeRange!); + const data = getDataStart(); + const dataSearch = getDataStart().search; + const parsedTimeRange = data.query.timefilter.timefilter.calculateBounds(input?.timeRange!); if (visParams && visParams.id && !visParams.isModelInvalid) { const maxBuckets = config.get(MAX_BUCKETS_SETTING); validateInterval(parsedTimeRange, visParams, maxBuckets); - const isCurrentSession = () => - !!searchSessionId && searchSessionId === dataSearch.search.session.getSessionId(); const untrackSearch = - isCurrentSession() && - dataSearch.search.session.trackSearch({ + dataSearch.session.isCurrentSession(searchSessionId) && + dataSearch.session.trackSearch({ abort: () => { // TODO: support search cancellations }, @@ -70,13 +69,12 @@ export const metricsRequestHandler = async ({ filters: input?.filters, panels: [visParams], state: uiStateObj, - sessionId: searchSessionId, - isRestore: isCurrentSession() ? dataSearch.search.session.isRestore() : false, - isStored: isCurrentSession() ? dataSearch.search.session.isStored() : false, + searchSession: dataSearch.session.getSearchOptions(searchSessionId), }), }); } finally { - if (untrackSearch && isCurrentSession()) { + if (untrackSearch && dataSearch.session.isCurrentSession(searchSessionId)) { + // untrack if this search still belongs to current session untrackSearch(); } } diff --git a/src/plugins/vis_type_timeseries/server/lib/search_strategies/strategies/abstract_search_strategy.test.js b/src/plugins/vis_type_timeseries/server/lib/search_strategies/strategies/abstract_search_strategy.test.js index 2c38e883cd69f..800db39cdecd9 100644 --- a/src/plugins/vis_type_timeseries/server/lib/search_strategies/strategies/abstract_search_strategy.test.js +++ b/src/plugins/vis_type_timeseries/server/lib/search_strategies/strategies/abstract_search_strategy.test.js @@ -62,7 +62,11 @@ describe('AbstractSearchStrategy', () => { const responses = await abstractSearchStrategy.search( { payload: { - sessionId: 1, + searchSession: { + sessionId: '1', + isRestore: false, + isStored: true, + }, }, requestContext: { search: { search: searchFn }, @@ -81,7 +85,9 @@ describe('AbstractSearchStrategy', () => { indexType: undefined, }, { - sessionId: 1, + sessionId: '1', + isRestore: false, + isStored: true, } ); }); diff --git a/src/plugins/vis_type_timeseries/server/lib/search_strategies/strategies/abstract_search_strategy.ts b/src/plugins/vis_type_timeseries/server/lib/search_strategies/strategies/abstract_search_strategy.ts index 113eac49ea560..bf9769fef84ba 100644 --- a/src/plugins/vis_type_timeseries/server/lib/search_strategies/strategies/abstract_search_strategy.ts +++ b/src/plugins/vis_type_timeseries/server/lib/search_strategies/strategies/abstract_search_strategy.ts @@ -49,7 +49,6 @@ export interface ReqFacade extends FakeRequest { export abstract class AbstractSearchStrategy { async search(req: ReqFacade, bodies: any[], indexType?: string) { const requests: any[] = []; - const { sessionId, isRestore, isStored } = req.payload; bodies.forEach((body) => { requests.push( @@ -61,11 +60,7 @@ export abstract class AbstractSearchStrategy { ...body, }, }, - { - sessionId, - isRestore, - isStored, - } + req.payload.searchSession ) .toPromise() ); diff --git a/x-pack/plugins/data_enhanced/public/search/search_interceptor.ts b/x-pack/plugins/data_enhanced/public/search/search_interceptor.ts index b0f194115f0b8..72d2cce49477b 100644 --- a/x-pack/plugins/data_enhanced/public/search/search_interceptor.ts +++ b/x-pack/plugins/data_enhanced/public/search/search_interceptor.ts @@ -64,20 +64,24 @@ export class EnhancedSearchInterceptor extends SearchInterceptor { const search = () => this.runSearch({ id, ...request }, searchOptions); this.pendingCount$.next(this.pendingCount$.getValue() + 1); - const isCurrentSession = () => - !!options.sessionId && options.sessionId === this.deps.session.getSessionId(); - const untrackSearch = isCurrentSession() && this.deps.session.trackSearch({ abort }); + const untrackSearch = + this.deps.session.isCurrentSession(options.sessionId) && + this.deps.session.trackSearch({ abort }); // track if this search's session will be send to background // if yes, then we don't need to cancel this search when it is aborted let isSavedToBackground = false; const savedToBackgroundSub = - isCurrentSession() && + this.deps.session.isCurrentSession(options.sessionId) && this.deps.session.state$ .pipe( skip(1), // ignore any state, we are only interested in transition x -> BackgroundLoading - filter((state) => isCurrentSession() && state === SearchSessionState.BackgroundLoading), + filter( + (state) => + this.deps.session.isCurrentSession(options.sessionId) && + state === SearchSessionState.BackgroundLoading + ), take(1) ) .subscribe(() => { @@ -93,7 +97,8 @@ export class EnhancedSearchInterceptor extends SearchInterceptor { finalize(() => { this.pendingCount$.next(this.pendingCount$.getValue() - 1); cleanup(); - if (untrackSearch && isCurrentSession()) { + if (untrackSearch && this.deps.session.isCurrentSession(options.sessionId)) { + // untrack if this search still belongs to current session untrackSearch(); } if (savedToBackgroundSub) { From 0b24d189618973fed9068a2bf1d7e4696633d441 Mon Sep 17 00:00:00 2001 From: Anton Dosov Date: Tue, 12 Jan 2021 12:54:06 +0100 Subject: [PATCH 3/6] tests --- .../public/search/search_interceptor.test.ts | 64 +++----------- .../search/session/session_service.test.ts | 85 ++++++++++++++++++- .../public/search/session/session_service.ts | 6 +- .../public/search/search_interceptor.test.ts | 6 +- 4 files changed, 103 insertions(+), 58 deletions(-) diff --git a/src/plugins/data/public/search/search_interceptor.test.ts b/src/plugins/data/public/search/search_interceptor.test.ts index 947dac1b32640..04361281d9419 100644 --- a/src/plugins/data/public/search/search_interceptor.test.ts +++ b/src/plugins/data/public/search/search_interceptor.test.ts @@ -118,9 +118,11 @@ describe('SearchInterceptor', () => { sessionId?: string; }) => { const sessionServiceMock = searchMock.session as jest.Mocked; - sessionServiceMock.getSessionId.mockImplementation(() => sessionId); - sessionServiceMock.isRestore.mockImplementation(() => isRestore); - sessionServiceMock.isStored.mockImplementation(() => isStored); + sessionServiceMock.getSearchOptions.mockImplementation(() => ({ + sessionId, + isRestore, + isStored, + })); fetchMock.mockResolvedValue({ result: 200 }); }; @@ -130,30 +132,14 @@ describe('SearchInterceptor', () => { afterEach(() => { const sessionServiceMock = searchMock.session as jest.Mocked; - sessionServiceMock.getSessionId.mockReset(); - sessionServiceMock.isRestore.mockReset(); - sessionServiceMock.isStored.mockReset(); + sessionServiceMock.getSearchOptions.mockReset(); fetchMock.mockReset(); }); - test('infers isRestore from session service state', async () => { + test('gets session search options from session service', async () => { const sessionId = 'sid'; setup({ isRestore: true, - sessionId, - }); - - await searchInterceptor.search(mockRequest, { sessionId }).toPromise(); - expect(fetchMock.mock.calls[0][0]).toEqual( - expect.objectContaining({ - options: { sessionId: 'sid', isStored: false, isRestore: true }, - }) - ); - }); - - test('infers isStored from session service state', async () => { - const sessionId = 'sid'; - setup({ isStored: true, sessionId, }); @@ -161,41 +147,13 @@ describe('SearchInterceptor', () => { await searchInterceptor.search(mockRequest, { sessionId }).toPromise(); expect(fetchMock.mock.calls[0][0]).toEqual( expect.objectContaining({ - options: { sessionId: 'sid', isStored: true, isRestore: false }, - }) - ); - }); - - test('skips isRestore & isStore in case not a current session Id', async () => { - setup({ - isStored: true, - isRestore: true, - sessionId: 'session id', - }); - - await searchInterceptor - .search(mockRequest, { sessionId: 'different session id' }) - .toPromise(); - expect(fetchMock.mock.calls[0][0]).toEqual( - expect.objectContaining({ - options: { sessionId: 'different session id', isStored: false, isRestore: false }, + options: { sessionId, isStored: true, isRestore: true }, }) ); - }); - test('skips isRestore & isStore in case no session Id', async () => { - setup({ - isStored: true, - isRestore: true, - sessionId: undefined, - }); - - await searchInterceptor.search(mockRequest, { sessionId: 'sessionId' }).toPromise(); - expect(fetchMock.mock.calls[0][0]).toEqual( - expect.objectContaining({ - options: { sessionId: 'sessionId', isStored: false, isRestore: false }, - }) - ); + expect( + (searchMock.session as jest.Mocked).getSearchOptions + ).toHaveBeenCalledWith(sessionId); }); }); diff --git a/src/plugins/data/public/search/session/session_service.test.ts b/src/plugins/data/public/search/session/session_service.test.ts index cf083239b1571..6e2f1efaf2f9e 100644 --- a/src/plugins/data/public/search/session/session_service.test.ts +++ b/src/plugins/data/public/search/session/session_service.test.ts @@ -30,9 +30,19 @@ describe('Session service', () => { beforeEach(() => { const initializerContext = coreMock.createPluginInitializerContext(); + const startService = coreMock.createSetup().getStartServices; sessionService = new SessionService( initializerContext, - coreMock.createSetup().getStartServices, + () => + startService().then(([coreStart, ...rest]) => [ + { + ...{ + ...coreStart, + application: { ...coreStart.application, currentAppId$: new BehaviorSubject('app') }, + }, + }, + ...rest, + ]), getSessionsClientMock(), { freezeState: false } // needed to use mocks inside state container ); @@ -93,4 +103,77 @@ describe('Session service', () => { expect(abort).toBeCalledTimes(3); }); }); + + test('getSearchOptions infers isRestore & isStored from state', async () => { + const sessionId = sessionService.start(); + const someOtherId = 'some-other-id'; + expect(sessionService.getSearchOptions()).toEqual({ + isStored: false, + isRestore: false, + sessionId: undefined, + }); + expect(sessionService.getSearchOptions(someOtherId)).toEqual({ + isStored: false, + isRestore: false, + sessionId: someOtherId, + }); + expect(sessionService.getSearchOptions(sessionId)).toEqual({ + isStored: false, + isRestore: false, + sessionId, + }); + + sessionService.setSearchSessionInfoProvider({ + getName: async () => 'Name', + getUrlGeneratorData: async () => ({ + urlGeneratorId: 'id', + initialState: {}, + restoreState: {}, + }), + }); + await sessionService.save(); + + expect(sessionService.getSearchOptions()).toEqual({ + isStored: false, + isRestore: false, + sessionId: undefined, + }); + expect(sessionService.getSearchOptions(someOtherId)).toEqual({ + isStored: false, + isRestore: false, + sessionId: someOtherId, + }); + expect(sessionService.getSearchOptions(sessionId)).toEqual({ + isStored: true, + isRestore: false, + sessionId, + }); + + await sessionService.restore(sessionId); + + expect(sessionService.getSearchOptions()).toEqual({ + isStored: false, + isRestore: false, + sessionId: undefined, + }); + expect(sessionService.getSearchOptions(someOtherId)).toEqual({ + isStored: false, + isRestore: false, + sessionId: someOtherId, + }); + expect(sessionService.getSearchOptions(sessionId)).toEqual({ + isStored: true, + isRestore: true, + sessionId, + }); + }); + test('isCurrentSession', () => { + expect(sessionService.isCurrentSession()).toBeFalsy(); + + const sessionId = sessionService.start(); + + expect(sessionService.isCurrentSession()).toBeFalsy(); + expect(sessionService.isCurrentSession('some-other')).toBeFalsy(); + expect(sessionService.isCurrentSession(sessionId)).toBeTruthy(); + }); }); diff --git a/src/plugins/data/public/search/session/session_service.ts b/src/plugins/data/public/search/session/session_service.ts index ac15958a87070..cfdeb047a5f6a 100644 --- a/src/plugins/data/public/search/session/session_service.ts +++ b/src/plugins/data/public/search/session/session_service.ts @@ -260,11 +260,11 @@ export class SessionService { public getSearchOptions( sessionId?: string ): Pick { - const currentSessionId = this.getSessionId(); + const isCurrentSession = this.isCurrentSession(sessionId); return { sessionId, - isRestore: currentSessionId ? this.isRestore() : false, - isStored: currentSessionId ? this.isStored() : false, + isRestore: isCurrentSession ? this.isRestore() : false, + isStored: isCurrentSession ? this.isStored() : false, }; } } diff --git a/x-pack/plugins/data_enhanced/public/search/search_interceptor.test.ts b/x-pack/plugins/data_enhanced/public/search/search_interceptor.test.ts index fc6c860f907f6..d1bb672b985f4 100644 --- a/x-pack/plugins/data_enhanced/public/search/search_interceptor.test.ts +++ b/x-pack/plugins/data_enhanced/public/search/search_interceptor.test.ts @@ -373,7 +373,7 @@ describe('EnhancedSearchInterceptor', () => { test('should NOT DELETE a running SAVED async search on abort', async () => { const sessionId = 'sessionId'; - sessionService.getSessionId.mockImplementation(() => sessionId); + sessionService.isCurrentSession.mockImplementation((_sessionId) => _sessionId === sessionId); const responses = [ { time: 10, @@ -479,6 +479,7 @@ describe('EnhancedSearchInterceptor', () => { test('should track searches', async () => { const sessionId = 'sessionId'; + sessionService.isCurrentSession.mockImplementation((_sessionId) => _sessionId === sessionId); sessionService.getSessionId.mockImplementation(() => sessionId); const untrack = jest.fn(); @@ -496,6 +497,7 @@ describe('EnhancedSearchInterceptor', () => { test('session service should be able to cancel search', async () => { const sessionId = 'sessionId'; + sessionService.isCurrentSession.mockImplementation((_sessionId) => _sessionId === sessionId); sessionService.getSessionId.mockImplementation(() => sessionId); const untrack = jest.fn(); @@ -519,6 +521,7 @@ describe('EnhancedSearchInterceptor', () => { test("don't track non current session searches", async () => { const sessionId = 'sessionId'; + sessionService.isCurrentSession.mockImplementation((_sessionId) => _sessionId === sessionId); sessionService.getSessionId.mockImplementation(() => sessionId); const untrack = jest.fn(); @@ -539,6 +542,7 @@ describe('EnhancedSearchInterceptor', () => { test("don't track if no current session", async () => { sessionService.getSessionId.mockImplementation(() => undefined); + sessionService.isCurrentSession.mockImplementation((_sessionId) => false); const untrack = jest.fn(); sessionService.trackSearch.mockImplementation(() => untrack); From 2edf0e2b795748162479538cc9611e595fcd9f42 Mon Sep 17 00:00:00 2001 From: Anton Dosov Date: Tue, 12 Jan 2021 13:50:33 +0100 Subject: [PATCH 4/6] fix nit in tests --- .../data/public/search/session/session_service.test.ts | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/plugins/data/public/search/session/session_service.test.ts b/src/plugins/data/public/search/session/session_service.test.ts index 6e2f1efaf2f9e..7503f151e05d8 100644 --- a/src/plugins/data/public/search/session/session_service.test.ts +++ b/src/plugins/data/public/search/session/session_service.test.ts @@ -36,10 +36,8 @@ describe('Session service', () => { () => startService().then(([coreStart, ...rest]) => [ { - ...{ - ...coreStart, - application: { ...coreStart.application, currentAppId$: new BehaviorSubject('app') }, - }, + ...coreStart, + application: { ...coreStart.application, currentAppId$: new BehaviorSubject('app') }, }, ...rest, ]), From 3f6ec66968202c1ec8327cace0ebf3b991cc0299 Mon Sep 17 00:00:00 2001 From: Anton Dosov Date: Tue, 12 Jan 2021 15:53:04 +0100 Subject: [PATCH 5/6] improve api schema --- .../public/search/search_interceptor.test.ts | 2 +- .../data/public/search/search_interceptor.ts | 2 +- .../search/session/session_service.test.ts | 16 +--------------- .../public/search/session/session_service.ts | 4 ++-- .../public/helpers/timelion_request_handler.ts | 4 +++- .../vis_type_timelion/server/routes/run.ts | 6 +++--- .../vis_type_timeseries/common/vis_schema.ts | 6 +++--- .../public/request_handler.ts | 4 +++- 8 files changed, 17 insertions(+), 27 deletions(-) diff --git a/src/plugins/data/public/search/search_interceptor.test.ts b/src/plugins/data/public/search/search_interceptor.test.ts index 04361281d9419..0656d3ac3aa03 100644 --- a/src/plugins/data/public/search/search_interceptor.test.ts +++ b/src/plugins/data/public/search/search_interceptor.test.ts @@ -115,7 +115,7 @@ describe('SearchInterceptor', () => { }: { isRestore?: boolean; isStored?: boolean; - sessionId?: string; + sessionId: string; }) => { const sessionServiceMock = searchMock.session as jest.Mocked; sessionServiceMock.getSearchOptions.mockImplementation(() => ({ diff --git a/src/plugins/data/public/search/search_interceptor.ts b/src/plugins/data/public/search/search_interceptor.ts index 584ec3c3e3dba..b81a6c4450cc4 100644 --- a/src/plugins/data/public/search/search_interceptor.ts +++ b/src/plugins/data/public/search/search_interceptor.ts @@ -135,7 +135,7 @@ export class SearchInterceptor { request, options: { ...requestOptions, - ...this.deps.session.getSearchOptions(options?.sessionId), + ...(options?.sessionId && this.deps.session.getSearchOptions(options.sessionId)), }, }, abortSignal diff --git a/src/plugins/data/public/search/session/session_service.test.ts b/src/plugins/data/public/search/session/session_service.test.ts index 7503f151e05d8..17e4fe0be5982 100644 --- a/src/plugins/data/public/search/session/session_service.test.ts +++ b/src/plugins/data/public/search/session/session_service.test.ts @@ -105,11 +105,7 @@ describe('Session service', () => { test('getSearchOptions infers isRestore & isStored from state', async () => { const sessionId = sessionService.start(); const someOtherId = 'some-other-id'; - expect(sessionService.getSearchOptions()).toEqual({ - isStored: false, - isRestore: false, - sessionId: undefined, - }); + expect(sessionService.getSearchOptions(someOtherId)).toEqual({ isStored: false, isRestore: false, @@ -131,11 +127,6 @@ describe('Session service', () => { }); await sessionService.save(); - expect(sessionService.getSearchOptions()).toEqual({ - isStored: false, - isRestore: false, - sessionId: undefined, - }); expect(sessionService.getSearchOptions(someOtherId)).toEqual({ isStored: false, isRestore: false, @@ -149,11 +140,6 @@ describe('Session service', () => { await sessionService.restore(sessionId); - expect(sessionService.getSearchOptions()).toEqual({ - isStored: false, - isRestore: false, - sessionId: undefined, - }); expect(sessionService.getSearchOptions(someOtherId)).toEqual({ isStored: false, isRestore: false, diff --git a/src/plugins/data/public/search/session/session_service.ts b/src/plugins/data/public/search/session/session_service.ts index cfdeb047a5f6a..11e0377c212ef 100644 --- a/src/plugins/data/public/search/session/session_service.ts +++ b/src/plugins/data/public/search/session/session_service.ts @@ -258,8 +258,8 @@ export class SessionService { * @param sessionId */ public getSearchOptions( - sessionId?: string - ): Pick { + sessionId: string + ): Required> { const isCurrentSession = this.isCurrentSession(sessionId); return { sessionId, diff --git a/src/plugins/vis_type_timelion/public/helpers/timelion_request_handler.ts b/src/plugins/vis_type_timelion/public/helpers/timelion_request_handler.ts index 4c336a5ee2c18..896a827f6248c 100644 --- a/src/plugins/vis_type_timelion/public/helpers/timelion_request_handler.ts +++ b/src/plugins/vis_type_timelion/public/helpers/timelion_request_handler.ts @@ -119,7 +119,9 @@ export function getTimelionRequestHandler({ interval: visParams.interval, timezone, }, - searchSession: dataSearch.session.getSearchOptions(searchSessionId), + ...(searchSessionId && { + searchSession: dataSearch.session.getSearchOptions(searchSessionId), + }), }), }); } catch (e) { diff --git a/src/plugins/vis_type_timelion/server/routes/run.ts b/src/plugins/vis_type_timelion/server/routes/run.ts index e801bdd108be5..af5af35e53b3f 100644 --- a/src/plugins/vis_type_timelion/server/routes/run.ts +++ b/src/plugins/vis_type_timelion/server/routes/run.ts @@ -77,9 +77,9 @@ export function runRoute( ), searchSession: schema.maybe( schema.object({ - sessionId: schema.maybe(schema.string()), - isRestore: schema.maybe(schema.boolean()), - isStored: schema.maybe(schema.boolean()), + sessionId: schema.string(), + isRestore: schema.boolean({ defaultValue: false }), + isStored: schema.boolean({ defaultValue: false }), }) ), }), diff --git a/src/plugins/vis_type_timeseries/common/vis_schema.ts b/src/plugins/vis_type_timeseries/common/vis_schema.ts index 72ea17572bb9c..a3e128bbb8554 100644 --- a/src/plugins/vis_type_timeseries/common/vis_schema.ts +++ b/src/plugins/vis_type_timeseries/common/vis_schema.ts @@ -284,9 +284,9 @@ export const visPayloadSchema = schema.object({ searchSession: schema.maybe( schema.object({ - sessionId: schema.maybe(schema.string()), - isRestore: schema.maybe(schema.boolean()), - isStored: schema.maybe(schema.boolean()), + sessionId: schema.string(), + isRestore: schema.boolean({ defaultValue: false }), + isStored: schema.boolean({ defaultValue: false }), }) ), }); diff --git a/src/plugins/vis_type_timeseries/public/request_handler.ts b/src/plugins/vis_type_timeseries/public/request_handler.ts index e9b8d2ef28983..16b5e4c2dc4fa 100644 --- a/src/plugins/vis_type_timeseries/public/request_handler.ts +++ b/src/plugins/vis_type_timeseries/public/request_handler.ts @@ -69,7 +69,9 @@ export const metricsRequestHandler = async ({ filters: input?.filters, panels: [visParams], state: uiStateObj, - searchSession: dataSearch.session.getSearchOptions(searchSessionId), + ...(searchSessionId && { + searchSession: dataSearch.session.getSearchOptions(searchSessionId), + }), }), }); } finally { From a5c7548bbce26c2ca8609827e2408e73f3f759a1 Mon Sep 17 00:00:00 2001 From: Anton Dosov Date: Tue, 12 Jan 2021 16:10:07 +0100 Subject: [PATCH 6/6] docs --- src/plugins/data/public/public.api.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/plugins/data/public/public.api.md b/src/plugins/data/public/public.api.md index 935cb945678de..50b6b2223bd0a 100644 --- a/src/plugins/data/public/public.api.md +++ b/src/plugins/data/public/public.api.md @@ -2624,7 +2624,7 @@ export const UI_SETTINGS: { // src/plugins/data/public/index.ts:433:1 - (ae-forgotten-export) The symbol "propFilter" needs to be exported by the entry point index.d.ts // src/plugins/data/public/index.ts:436:1 - (ae-forgotten-export) The symbol "toAbsoluteDates" needs to be exported by the entry point index.d.ts // src/plugins/data/public/query/state_sync/connect_to_query_state.ts:45:5 - (ae-forgotten-export) The symbol "FilterStateStore" needs to be exported by the entry point index.d.ts -// src/plugins/data/public/search/session/session_service.ts:51:5 - (ae-forgotten-export) The symbol "UrlGeneratorStateMapping" needs to be exported by the entry point index.d.ts +// src/plugins/data/public/search/session/session_service.ts:52:5 - (ae-forgotten-export) The symbol "UrlGeneratorStateMapping" needs to be exported by the entry point index.d.ts // (No @packageDocumentation comment for this package)