From 4ce2a4ec336a32874a0a79ce6d7092ee9bd27850 Mon Sep 17 00:00:00 2001 From: Lukas Olson Date: Thu, 12 Dec 2024 09:20:36 -0700 Subject: [PATCH 1/4] Clean up code around ES preference usage --- .../fetch/get_search_params.test.ts | 6 +-- .../search_source/fetch/get_search_params.ts | 20 +------- src/plugins/data/common/search/utils.test.ts | 38 +++++++++++++- src/plugins/data/common/search/utils.ts | 15 +++++- .../es_search/get_es_preference.test.ts | 49 ------------------- .../search/es_search/get_es_preference.ts | 20 -------- .../data/public/search/es_search/index.ts | 10 ---- src/plugins/data/public/search/index.ts | 1 - 8 files changed, 56 insertions(+), 103 deletions(-) delete mode 100644 src/plugins/data/public/search/es_search/get_es_preference.test.ts delete mode 100644 src/plugins/data/public/search/es_search/get_es_preference.ts delete mode 100644 src/plugins/data/public/search/es_search/index.ts diff --git a/src/plugins/data/common/search/search_source/fetch/get_search_params.test.ts b/src/plugins/data/common/search/search_source/fetch/get_search_params.test.ts index 99f813294fe5a..189802921cd47 100644 --- a/src/plugins/data/common/search/search_source/fetch/get_search_params.test.ts +++ b/src/plugins/data/common/search/search_source/fetch/get_search_params.test.ts @@ -9,7 +9,7 @@ import { UI_SETTINGS } from '../../../constants'; import { GetConfigFn } from '../../../types'; -import { getSearchParams, getSearchParamsFromRequest } from './get_search_params'; +import { getSearchParamsFromRequest } from './get_search_params'; import { createStubDataView } from '@kbn/data-views-plugin/common/data_views/data_view.stub'; function getConfigStub(config: any = {}): GetConfigFn { @@ -18,11 +18,11 @@ function getConfigStub(config: any = {}): GetConfigFn { describe('getSearchParams', () => { test('includes custom preference', () => { - const config = getConfigStub({ + const getConfig = getConfigStub({ [UI_SETTINGS.COURIER_SET_REQUEST_PREFERENCE]: 'custom', [UI_SETTINGS.COURIER_CUSTOM_REQUEST_PREFERENCE]: 'aaa', }); - const searchParams = getSearchParams(config); + const searchParams = getSearchParamsFromRequest({ index: 'abc', body: {} }, { getConfig }); expect(searchParams.preference).toBe('aaa'); }); diff --git a/src/plugins/data/common/search/search_source/fetch/get_search_params.ts b/src/plugins/data/common/search/search_source/fetch/get_search_params.ts index 221e670dae6dc..6e3f8be2b9439 100644 --- a/src/plugins/data/common/search/search_source/fetch/get_search_params.ts +++ b/src/plugins/data/common/search/search_source/fetch/get_search_params.ts @@ -8,25 +8,9 @@ */ import type { ISearchRequestParams } from '@kbn/search-types'; -import { UI_SETTINGS } from '../../../constants'; import { GetConfigFn } from '../../../types'; import type { SearchRequest } from './types'; - -const sessionId = Date.now(); - -export function getSearchParams(getConfig: GetConfigFn) { - return { - preference: getPreference(getConfig), - }; -} - -export function getPreference(getConfig: GetConfigFn) { - const setRequestPreference = getConfig(UI_SETTINGS.COURIER_SET_REQUEST_PREFERENCE); - if (setRequestPreference === 'sessionId') return sessionId; - return setRequestPreference === 'custom' - ? getConfig(UI_SETTINGS.COURIER_CUSTOM_REQUEST_PREFERENCE) - : undefined; -} +import { getEsPreference } from '../../utils'; /** @public */ // TODO: Could provide this on runtime contract with dependencies @@ -36,7 +20,7 @@ export function getSearchParamsFromRequest( dependencies: { getConfig: GetConfigFn } ): ISearchRequestParams { const { getConfig } = dependencies; - const searchParams = getSearchParams(getConfig); + const searchParams = { preference: getEsPreference(getConfig) }; // eslint-disable-next-line @typescript-eslint/naming-convention const { track_total_hits, ...body } = searchRequest.body; const dataView = typeof searchRequest.index !== 'string' ? searchRequest.index : undefined; diff --git a/src/plugins/data/common/search/utils.test.ts b/src/plugins/data/common/search/utils.test.ts index 5fd178f6080b5..87d42f52feddd 100644 --- a/src/plugins/data/common/search/utils.test.ts +++ b/src/plugins/data/common/search/utils.test.ts @@ -8,7 +8,8 @@ */ import type { IKibanaSearchResponse } from '@kbn/search-types'; -import { isAbortResponse, isRunningResponse } from './utils'; +import { isAbortResponse, isRunningResponse, getEsPreference } from './utils'; +import { UI_SETTINGS } from '..'; describe('utils', () => { describe('isAbortResponse', () => { @@ -45,4 +46,39 @@ describe('utils', () => { expect(isRunning).toBe(false); }); }); + + describe('getEsPreference', () => { + const mockConfigGet = jest.fn(); + + beforeEach(() => { + mockConfigGet.mockClear(); + }); + + test('returns the session ID if set to sessionId', () => { + mockConfigGet.mockImplementation((key: string) => { + if (key === UI_SETTINGS.COURIER_SET_REQUEST_PREFERENCE) return 'sessionId'; + if (key === UI_SETTINGS.COURIER_CUSTOM_REQUEST_PREFERENCE) return 'foobar'; + }); + const preference = getEsPreference(mockConfigGet, 'my_session_id'); + expect(preference).toBe('my_session_id'); + }); + + test('returns the custom preference if set to custom', () => { + mockConfigGet.mockImplementation((key: string) => { + if (key === UI_SETTINGS.COURIER_SET_REQUEST_PREFERENCE) return 'custom'; + if (key === UI_SETTINGS.COURIER_CUSTOM_REQUEST_PREFERENCE) return 'foobar'; + }); + const preference = getEsPreference(mockConfigGet); + expect(preference).toBe('foobar'); + }); + + test('returns undefined if set to none', () => { + mockConfigGet.mockImplementation((key: string) => { + if (key === UI_SETTINGS.COURIER_SET_REQUEST_PREFERENCE) return 'none'; + if (key === UI_SETTINGS.COURIER_CUSTOM_REQUEST_PREFERENCE) return 'foobar'; + }); + const preference = getEsPreference(mockConfigGet); + expect(preference).toBe(undefined); + }); + }); }); diff --git a/src/plugins/data/common/search/utils.ts b/src/plugins/data/common/search/utils.ts index baf3fe79b1ac2..cf7c91635f839 100644 --- a/src/plugins/data/common/search/utils.ts +++ b/src/plugins/data/common/search/utils.ts @@ -9,7 +9,8 @@ import moment from 'moment-timezone'; import type { IKibanaSearchResponse } from '@kbn/search-types'; -import { AggTypesDependencies } from '..'; +import type { SearchRequest } from '@elastic/elasticsearch/lib/api/types'; +import { AggTypesDependencies, GetConfigFn, UI_SETTINGS } from '..'; // TODO - investigate if this check is still needed // There are no documented work flows where response or rawResponse is not returned @@ -49,3 +50,15 @@ export const getUserTimeZone = ( return userTimeZone ?? defaultTimeZone; }; + +const defaultSessionId = `${Date.now()}`; + +export function getEsPreference( + getConfigFn: GetConfigFn, + sessionId = defaultSessionId +): SearchRequest['preference'] { + const setPreference = getConfigFn(UI_SETTINGS.COURIER_SET_REQUEST_PREFERENCE); + if (setPreference === 'sessionId') return `${sessionId}`; + const customPreference = getConfigFn(UI_SETTINGS.COURIER_CUSTOM_REQUEST_PREFERENCE); + return setPreference === 'custom' ? customPreference : undefined; +} diff --git a/src/plugins/data/public/search/es_search/get_es_preference.test.ts b/src/plugins/data/public/search/es_search/get_es_preference.test.ts deleted file mode 100644 index e9edbdac475d8..0000000000000 --- a/src/plugins/data/public/search/es_search/get_es_preference.test.ts +++ /dev/null @@ -1,49 +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 - * 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side - * Public License v 1"; you may not use this file except in compliance with, at - * your election, the "Elastic License 2.0", the "GNU Affero General Public - * License v3.0 only", or the "Server Side Public License, v 1". - */ - -import type { MockedKeys } from '@kbn/utility-types-jest'; -import { getEsPreference } from './get_es_preference'; -import { CoreStart } from '@kbn/core/public'; -import { coreMock } from '@kbn/core/public/mocks'; -import { UI_SETTINGS } from '../../../common'; - -describe('Get ES preference', () => { - let mockCoreStart: MockedKeys; - - beforeEach(() => { - mockCoreStart = coreMock.createStart(); - }); - - test('returns the session ID if set to sessionId', () => { - mockCoreStart.uiSettings.get.mockImplementation((key: string) => { - if (key === UI_SETTINGS.COURIER_SET_REQUEST_PREFERENCE) return 'sessionId'; - if (key === UI_SETTINGS.COURIER_CUSTOM_REQUEST_PREFERENCE) return 'foobar'; - }); - const preference = getEsPreference(mockCoreStart.uiSettings, 'my_session_id'); - expect(preference).toBe('my_session_id'); - }); - - test('returns the custom preference if set to custom', () => { - mockCoreStart.uiSettings.get.mockImplementation((key: string) => { - if (key === UI_SETTINGS.COURIER_SET_REQUEST_PREFERENCE) return 'custom'; - if (key === UI_SETTINGS.COURIER_CUSTOM_REQUEST_PREFERENCE) return 'foobar'; - }); - const preference = getEsPreference(mockCoreStart.uiSettings); - expect(preference).toBe('foobar'); - }); - - test('returns undefined if set to none', () => { - mockCoreStart.uiSettings.get.mockImplementation((key: string) => { - if (key === UI_SETTINGS.COURIER_SET_REQUEST_PREFERENCE) return 'none'; - if (key === UI_SETTINGS.COURIER_CUSTOM_REQUEST_PREFERENCE) return 'foobar'; - }); - const preference = getEsPreference(mockCoreStart.uiSettings); - expect(preference).toBe(undefined); - }); -}); diff --git a/src/plugins/data/public/search/es_search/get_es_preference.ts b/src/plugins/data/public/search/es_search/get_es_preference.ts deleted file mode 100644 index e07aaf0ce99a8..0000000000000 --- a/src/plugins/data/public/search/es_search/get_es_preference.ts +++ /dev/null @@ -1,20 +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 - * 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side - * Public License v 1"; you may not use this file except in compliance with, at - * your election, the "Elastic License 2.0", the "GNU Affero General Public - * License v3.0 only", or the "Server Side Public License, v 1". - */ - -import { IUiSettingsClient } from '@kbn/core/public'; -import { UI_SETTINGS } from '../../../common'; - -const defaultSessionId = `${Date.now()}`; - -export function getEsPreference(uiSettings: IUiSettingsClient, sessionId = defaultSessionId) { - const setPreference = uiSettings.get(UI_SETTINGS.COURIER_SET_REQUEST_PREFERENCE); - if (setPreference === 'sessionId') return `${sessionId}`; - const customPreference = uiSettings.get(UI_SETTINGS.COURIER_CUSTOM_REQUEST_PREFERENCE); - return setPreference === 'custom' ? customPreference : undefined; -} diff --git a/src/plugins/data/public/search/es_search/index.ts b/src/plugins/data/public/search/es_search/index.ts deleted file mode 100644 index 8084588003204..0000000000000 --- a/src/plugins/data/public/search/es_search/index.ts +++ /dev/null @@ -1,10 +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 - * 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side - * Public License v 1"; you may not use this file except in compliance with, at - * your election, the "Elastic License 2.0", the "GNU Affero General Public - * License v3.0 only", or the "Server Side Public License, v 1". - */ - -export { getEsPreference } from './get_es_preference'; diff --git a/src/plugins/data/public/search/index.ts b/src/plugins/data/public/search/index.ts index a2f5c6fe1a3b5..a678d2d4f7521 100644 --- a/src/plugins/data/public/search/index.ts +++ b/src/plugins/data/public/search/index.ts @@ -48,7 +48,6 @@ export { SEARCH_SESSIONS_MANAGEMENT_ID, waitUntilNextSessionCompletes$, } from './session'; -export { getEsPreference } from './es_search'; export type { SearchInterceptorDeps } from './search_interceptor'; export { SearchInterceptor } from './search_interceptor'; From cbfb68cb0f267fb7e85d6d1ad6870880a15c1d8d Mon Sep 17 00:00:00 2001 From: Lukas Olson Date: Thu, 12 Dec 2024 10:00:41 -0700 Subject: [PATCH 2/4] Remove unused exports --- src/plugins/data/common/search/search_source/fetch/index.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/plugins/data/common/search/search_source/fetch/index.ts b/src/plugins/data/common/search/search_source/fetch/index.ts index f097189fe4bae..3897d7e25d200 100644 --- a/src/plugins/data/common/search/search_source/fetch/index.ts +++ b/src/plugins/data/common/search/search_source/fetch/index.ts @@ -7,6 +7,6 @@ * License v3.0 only", or the "Server Side Public License, v 1". */ -export { getSearchParams, getSearchParamsFromRequest, getPreference } from './get_search_params'; +export { getSearchParamsFromRequest } from './get_search_params'; export { RequestFailure } from './request_error'; export * from './types'; From 4fa80d8c254d349a2cd29521603e664454332196 Mon Sep 17 00:00:00 2001 From: Lukas Olson Date: Fri, 13 Dec 2024 13:58:29 -0700 Subject: [PATCH 3/4] Move files back to where they originally were --- .../fetch/get_search_params.test.ts | 37 +++++++++++++++++- .../search_source/fetch/get_search_params.ts | 14 ++++++- .../search/search_source/fetch/index.ts | 2 +- src/plugins/data/common/search/utils.test.ts | 38 +------------------ src/plugins/data/common/search/utils.ts | 15 +------- 5 files changed, 52 insertions(+), 54 deletions(-) diff --git a/src/plugins/data/common/search/search_source/fetch/get_search_params.test.ts b/src/plugins/data/common/search/search_source/fetch/get_search_params.test.ts index 189802921cd47..68b9d234e6667 100644 --- a/src/plugins/data/common/search/search_source/fetch/get_search_params.test.ts +++ b/src/plugins/data/common/search/search_source/fetch/get_search_params.test.ts @@ -9,7 +9,7 @@ import { UI_SETTINGS } from '../../../constants'; import { GetConfigFn } from '../../../types'; -import { getSearchParamsFromRequest } from './get_search_params'; +import { getSearchParamsFromRequest, getEsPreference } from './get_search_params'; import { createStubDataView } from '@kbn/data-views-plugin/common/data_views/data_view.stub'; function getConfigStub(config: any = {}): GetConfigFn { @@ -94,4 +94,39 @@ describe('getSearchParams', () => { ); expect(searchParams).not.toHaveProperty('expand_wildcards', 'all'); }); + + describe('getEsPreference', () => { + const mockConfigGet = jest.fn(); + + beforeEach(() => { + mockConfigGet.mockClear(); + }); + + test('returns the session ID if set to sessionId', () => { + mockConfigGet.mockImplementation((key: string) => { + if (key === UI_SETTINGS.COURIER_SET_REQUEST_PREFERENCE) return 'sessionId'; + if (key === UI_SETTINGS.COURIER_CUSTOM_REQUEST_PREFERENCE) return 'foobar'; + }); + const preference = getEsPreference(mockConfigGet, 'my_session_id'); + expect(preference).toBe('my_session_id'); + }); + + test('returns the custom preference if set to custom', () => { + mockConfigGet.mockImplementation((key: string) => { + if (key === UI_SETTINGS.COURIER_SET_REQUEST_PREFERENCE) return 'custom'; + if (key === UI_SETTINGS.COURIER_CUSTOM_REQUEST_PREFERENCE) return 'foobar'; + }); + const preference = getEsPreference(mockConfigGet); + expect(preference).toBe('foobar'); + }); + + test('returns undefined if set to none', () => { + mockConfigGet.mockImplementation((key: string) => { + if (key === UI_SETTINGS.COURIER_SET_REQUEST_PREFERENCE) return 'none'; + if (key === UI_SETTINGS.COURIER_CUSTOM_REQUEST_PREFERENCE) return 'foobar'; + }); + const preference = getEsPreference(mockConfigGet); + expect(preference).toBe(undefined); + }); + }); }); diff --git a/src/plugins/data/common/search/search_source/fetch/get_search_params.ts b/src/plugins/data/common/search/search_source/fetch/get_search_params.ts index 6e3f8be2b9439..f68cf3b66a90c 100644 --- a/src/plugins/data/common/search/search_source/fetch/get_search_params.ts +++ b/src/plugins/data/common/search/search_source/fetch/get_search_params.ts @@ -8,9 +8,21 @@ */ import type { ISearchRequestParams } from '@kbn/search-types'; +import { UI_SETTINGS } from '../../../constants'; import { GetConfigFn } from '../../../types'; import type { SearchRequest } from './types'; -import { getEsPreference } from '../../utils'; + +const defaultSessionId = `${Date.now()}`; + +export function getEsPreference( + getConfigFn: GetConfigFn, + sessionId = defaultSessionId +): SearchRequest['preference'] { + const setPreference = getConfigFn(UI_SETTINGS.COURIER_SET_REQUEST_PREFERENCE); + if (setPreference === 'sessionId') return `${sessionId}`; + const customPreference = getConfigFn(UI_SETTINGS.COURIER_CUSTOM_REQUEST_PREFERENCE); + return setPreference === 'custom' ? customPreference : undefined; +} /** @public */ // TODO: Could provide this on runtime contract with dependencies diff --git a/src/plugins/data/common/search/search_source/fetch/index.ts b/src/plugins/data/common/search/search_source/fetch/index.ts index 3897d7e25d200..3f621c66eeaa3 100644 --- a/src/plugins/data/common/search/search_source/fetch/index.ts +++ b/src/plugins/data/common/search/search_source/fetch/index.ts @@ -7,6 +7,6 @@ * License v3.0 only", or the "Server Side Public License, v 1". */ -export { getSearchParamsFromRequest } from './get_search_params'; +export { getSearchParamsFromRequest, getEsPreference } from './get_search_params'; export { RequestFailure } from './request_error'; export * from './types'; diff --git a/src/plugins/data/common/search/utils.test.ts b/src/plugins/data/common/search/utils.test.ts index 87d42f52feddd..5fd178f6080b5 100644 --- a/src/plugins/data/common/search/utils.test.ts +++ b/src/plugins/data/common/search/utils.test.ts @@ -8,8 +8,7 @@ */ import type { IKibanaSearchResponse } from '@kbn/search-types'; -import { isAbortResponse, isRunningResponse, getEsPreference } from './utils'; -import { UI_SETTINGS } from '..'; +import { isAbortResponse, isRunningResponse } from './utils'; describe('utils', () => { describe('isAbortResponse', () => { @@ -46,39 +45,4 @@ describe('utils', () => { expect(isRunning).toBe(false); }); }); - - describe('getEsPreference', () => { - const mockConfigGet = jest.fn(); - - beforeEach(() => { - mockConfigGet.mockClear(); - }); - - test('returns the session ID if set to sessionId', () => { - mockConfigGet.mockImplementation((key: string) => { - if (key === UI_SETTINGS.COURIER_SET_REQUEST_PREFERENCE) return 'sessionId'; - if (key === UI_SETTINGS.COURIER_CUSTOM_REQUEST_PREFERENCE) return 'foobar'; - }); - const preference = getEsPreference(mockConfigGet, 'my_session_id'); - expect(preference).toBe('my_session_id'); - }); - - test('returns the custom preference if set to custom', () => { - mockConfigGet.mockImplementation((key: string) => { - if (key === UI_SETTINGS.COURIER_SET_REQUEST_PREFERENCE) return 'custom'; - if (key === UI_SETTINGS.COURIER_CUSTOM_REQUEST_PREFERENCE) return 'foobar'; - }); - const preference = getEsPreference(mockConfigGet); - expect(preference).toBe('foobar'); - }); - - test('returns undefined if set to none', () => { - mockConfigGet.mockImplementation((key: string) => { - if (key === UI_SETTINGS.COURIER_SET_REQUEST_PREFERENCE) return 'none'; - if (key === UI_SETTINGS.COURIER_CUSTOM_REQUEST_PREFERENCE) return 'foobar'; - }); - const preference = getEsPreference(mockConfigGet); - expect(preference).toBe(undefined); - }); - }); }); diff --git a/src/plugins/data/common/search/utils.ts b/src/plugins/data/common/search/utils.ts index cf7c91635f839..baf3fe79b1ac2 100644 --- a/src/plugins/data/common/search/utils.ts +++ b/src/plugins/data/common/search/utils.ts @@ -9,8 +9,7 @@ import moment from 'moment-timezone'; import type { IKibanaSearchResponse } from '@kbn/search-types'; -import type { SearchRequest } from '@elastic/elasticsearch/lib/api/types'; -import { AggTypesDependencies, GetConfigFn, UI_SETTINGS } from '..'; +import { AggTypesDependencies } from '..'; // TODO - investigate if this check is still needed // There are no documented work flows where response or rawResponse is not returned @@ -50,15 +49,3 @@ export const getUserTimeZone = ( return userTimeZone ?? defaultTimeZone; }; - -const defaultSessionId = `${Date.now()}`; - -export function getEsPreference( - getConfigFn: GetConfigFn, - sessionId = defaultSessionId -): SearchRequest['preference'] { - const setPreference = getConfigFn(UI_SETTINGS.COURIER_SET_REQUEST_PREFERENCE); - if (setPreference === 'sessionId') return `${sessionId}`; - const customPreference = getConfigFn(UI_SETTINGS.COURIER_CUSTOM_REQUEST_PREFERENCE); - return setPreference === 'custom' ? customPreference : undefined; -} From 4a3f34fc4642b388726166831b90bce0649041f6 Mon Sep 17 00:00:00 2001 From: Lukas Olson Date: Thu, 19 Dec 2024 10:50:55 -0700 Subject: [PATCH 4/4] Update src/plugins/data/common/search/search_source/fetch/get_search_params.ts Co-authored-by: Marco Vettorello --- .../data/common/search/search_source/fetch/get_search_params.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/plugins/data/common/search/search_source/fetch/get_search_params.ts b/src/plugins/data/common/search/search_source/fetch/get_search_params.ts index f68cf3b66a90c..f1784770fe3ba 100644 --- a/src/plugins/data/common/search/search_source/fetch/get_search_params.ts +++ b/src/plugins/data/common/search/search_source/fetch/get_search_params.ts @@ -19,7 +19,7 @@ export function getEsPreference( sessionId = defaultSessionId ): SearchRequest['preference'] { const setPreference = getConfigFn(UI_SETTINGS.COURIER_SET_REQUEST_PREFERENCE); - if (setPreference === 'sessionId') return `${sessionId}`; + if (setPreference === 'sessionId') return sessionId; const customPreference = getConfigFn(UI_SETTINGS.COURIER_CUSTOM_REQUEST_PREFERENCE); return setPreference === 'custom' ? customPreference : undefined; }