Skip to content

Commit

Permalink
[data.search] Clean up code around ES preference usage (#204076)
Browse files Browse the repository at this point in the history
## Summary

While refreshing my memory on how we are using the Elasticsearch
`preference` parameter, I noticed we had a method `getEsPreference` that
was exported but not used. We then had an additional method
`getPreference` that was being used to actually set the parameter. I
consolidated these into one place and moved the tests so that we can
utilize it when ES|QL supports this parameter.

### Checklist

- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios

---------

Co-authored-by: Marco Vettorello <[email protected]>
(cherry picked from commit df3665f)
  • Loading branch information
lukasolson committed Jan 6, 2025
1 parent c7dd64a commit 39b81f4
Show file tree
Hide file tree
Showing 7 changed files with 49 additions and 98 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@

import { UI_SETTINGS } from '../../../constants';
import { GetConfigFn } from '../../../types';
import { getSearchParams, 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 {
Expand All @@ -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');
});

Expand Down Expand Up @@ -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);
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -12,20 +12,16 @@ import { UI_SETTINGS } from '../../../constants';
import { GetConfigFn } from '../../../types';
import type { SearchRequest } from './types';

const sessionId = Date.now();
const defaultSessionId = `${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;
export function getEsPreference(
getConfigFn: GetConfigFn,
sessionId = defaultSessionId
): SearchRequest['preference'] {
const setPreference = getConfigFn<string>(UI_SETTINGS.COURIER_SET_REQUEST_PREFERENCE);
if (setPreference === 'sessionId') return sessionId;
const customPreference = getConfigFn<string>(UI_SETTINGS.COURIER_CUSTOM_REQUEST_PREFERENCE);
return setPreference === 'custom' ? customPreference : undefined;
}

/** @public */
Expand All @@ -36,7 +32,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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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, getEsPreference } from './get_search_params';
export { RequestFailure } from './request_error';
export * from './types';
49 changes: 0 additions & 49 deletions src/plugins/data/public/search/es_search/get_es_preference.test.ts

This file was deleted.

20 changes: 0 additions & 20 deletions src/plugins/data/public/search/es_search/get_es_preference.ts

This file was deleted.

10 changes: 0 additions & 10 deletions src/plugins/data/public/search/es_search/index.ts

This file was deleted.

1 change: 0 additions & 1 deletion src/plugins/data/public/search/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down

0 comments on commit 39b81f4

Please sign in to comment.