mirror of
https://github.com/elastic/kibana.git
synced 2025-04-24 01:38:56 -04:00
[Discover] Use search cache when returning to a tab (#216742)
## Summary Resolves https://github.com/elastic/kibana/issues/214871. Related to https://github.com/elastic/kibana/issues/216475. This PR updates the behavior when returning to an existing tab in Discover to re-use the `searchSessionId` that was used prior to switching tabs. This has the effect of utilizing the `searchResponseCache` that is part of the data plugin, effectively making it so we don't re-issue the data requests: https://github.com/user-attachments/assets/9ffea23f-bc7d-4d39-90f2-57313d6f1e56 ### Checklist - [ ] [Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html) was added for features that require explanation or tutorials - [ ] [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 - [ ] If a plugin configuration key changed, check if it needs to be allowlisted in the cloud and added to the [docker list](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker) - [ ] This was checked for breaking HTTP API changes, and any breaking changes have been approved by the breaking-change committee. The `release_note:breaking` label should be applied in these situations. - [ ] [Flaky Test Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was used on any tests changed - [ ] The PR description includes the appropriate Release Notes section, and the correct `release_note:*` label is applied per the [guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process) ### Identify risks Does this PR introduce any risks? For example, consider risks like hard to test bugs, performance regression, potential of data loss. Describe the risk, its severity, and mitigation for each identified risk. Invite stakeholders and evaluate how to proceed before merging. - [ ] [See some risk examples](https://github.com/elastic/kibana/blob/main/RISK_MATRIX.mdx)
This commit is contained in:
parent
7419bc354c
commit
ff34965cc4
9 changed files with 41 additions and 63 deletions
|
@ -29,9 +29,7 @@ import { DiscoverHistogramLayout } from './discover_histogram_layout';
|
|||
import type { SavedSearch } from '@kbn/saved-search-plugin/public';
|
||||
import { VIEW_MODE } from '@kbn/saved-search-plugin/public';
|
||||
import type { Storage } from '@kbn/kibana-utils-plugin/public';
|
||||
import { createSearchSessionMock } from '../../../../__mocks__/search_session';
|
||||
import { searchSourceInstanceMock } from '@kbn/data-plugin/common/search/search_source/mocks';
|
||||
import { getSessionServiceMock } from '@kbn/data-plugin/public/search/session/mocks';
|
||||
import { getDiscoverStateMock } from '../../../../__mocks__/discover_state.mock';
|
||||
import { DiscoverMainProvider } from '../../state_management/discover_state_provider';
|
||||
import { act } from 'react-dom/test-utils';
|
||||
|
@ -42,9 +40,14 @@ import {
|
|||
RuntimeStateProvider,
|
||||
internalStateActions,
|
||||
} from '../../state_management/redux';
|
||||
import { TABS_ENABLED } from '../../discover_main_route';
|
||||
|
||||
function getStateContainer(savedSearch?: SavedSearch) {
|
||||
function getStateContainer({
|
||||
savedSearch,
|
||||
searchSessionId,
|
||||
}: {
|
||||
savedSearch?: SavedSearch;
|
||||
searchSessionId?: string | null;
|
||||
}) {
|
||||
const stateContainer = getDiscoverStateMock({ isTimeBased: true, savedSearch });
|
||||
const dataView = savedSearch?.searchSource?.getField('index') as DataView;
|
||||
const appState = {
|
||||
|
@ -69,6 +72,7 @@ function getStateContainer(savedSearch?: SavedSearch) {
|
|||
from: '2020-05-14T11:05:13.590',
|
||||
to: '2020-05-14T11:20:13.590',
|
||||
},
|
||||
...(searchSessionId && { searchSessionId }),
|
||||
},
|
||||
})
|
||||
);
|
||||
|
@ -121,11 +125,7 @@ const mountComponent = async ({
|
|||
totalHits$,
|
||||
};
|
||||
|
||||
const session = getSessionServiceMock();
|
||||
|
||||
session.getSession$.mockReturnValue(new BehaviorSubject(searchSessionId ?? undefined));
|
||||
|
||||
const stateContainer = getStateContainer(savedSearch);
|
||||
const stateContainer = getStateContainer({ savedSearch, searchSessionId });
|
||||
stateContainer.dataState.data$ = savedSearchData$;
|
||||
stateContainer.actions.undoSavedSearchChanges = jest.fn();
|
||||
|
||||
|
@ -151,7 +151,6 @@ const mountComponent = async ({
|
|||
/>
|
||||
),
|
||||
};
|
||||
stateContainer.searchSessionManager = createSearchSessionMock(session).searchSessionManager;
|
||||
|
||||
const component = mountWithIntl(
|
||||
<KibanaRenderContextProvider {...services.core}>
|
||||
|
@ -178,12 +177,10 @@ const mountComponent = async ({
|
|||
|
||||
describe('Discover histogram layout component', () => {
|
||||
describe('render', () => {
|
||||
if (!TABS_ENABLED) {
|
||||
it('should render null if there is no search session', async () => {
|
||||
const { component } = await mountComponent({ searchSessionId: null });
|
||||
expect(component.isEmptyRender()).toBe(true);
|
||||
});
|
||||
}
|
||||
it('should render null if there is no search session', async () => {
|
||||
const { component } = await mountComponent({ searchSessionId: null });
|
||||
expect(component.isEmptyRender()).toBe(true);
|
||||
});
|
||||
|
||||
it('should not render null if there is a search session', async () => {
|
||||
const { component } = await mountComponent();
|
||||
|
|
|
@ -10,12 +10,10 @@
|
|||
import React, { useCallback } from 'react';
|
||||
import { UnifiedHistogramContainer } from '@kbn/unified-histogram-plugin/public';
|
||||
import { css } from '@emotion/react';
|
||||
import useObservable from 'react-use/lib/useObservable';
|
||||
import { useDiscoverHistogram } from './use_discover_histogram';
|
||||
import { type DiscoverMainContentProps, DiscoverMainContent } from './discover_main_content';
|
||||
import { useAppStateSelector } from '../../state_management/discover_app_state_container';
|
||||
import { useIsEsqlMode } from '../../hooks/use_is_esql_mode';
|
||||
import { TABS_ENABLED } from '../../discover_main_route';
|
||||
|
||||
export interface DiscoverHistogramLayoutProps extends DiscoverMainContentProps {
|
||||
container: HTMLElement | null;
|
||||
|
@ -33,7 +31,6 @@ export const DiscoverHistogramLayout = ({
|
|||
...mainContentProps
|
||||
}: DiscoverHistogramLayoutProps) => {
|
||||
const { dataState } = stateContainer;
|
||||
const searchSessionId = useObservable(stateContainer.searchSessionManager.searchSessionId$);
|
||||
const hideChart = useAppStateSelector((state) => state.hideChart);
|
||||
const isEsqlMode = useIsEsqlMode();
|
||||
const unifiedHistogramProps = useDiscoverHistogram({
|
||||
|
@ -52,17 +49,13 @@ export const DiscoverHistogramLayout = ({
|
|||
|
||||
// Initialized when the first search has been requested or
|
||||
// when in ES|QL mode since search sessions are not supported
|
||||
// TODO: Handle this for tabs
|
||||
if (!TABS_ENABLED) {
|
||||
if (!searchSessionId && !isEsqlMode) {
|
||||
return null;
|
||||
}
|
||||
if (!unifiedHistogramProps.searchSessionId && !isEsqlMode) {
|
||||
return null;
|
||||
}
|
||||
|
||||
return (
|
||||
<UnifiedHistogramContainer
|
||||
{...unifiedHistogramProps}
|
||||
searchSessionId={searchSessionId}
|
||||
requestAdapter={dataState.inspectorAdapters.requests}
|
||||
container={container}
|
||||
css={histogramLayoutCss}
|
||||
|
|
|
@ -33,7 +33,6 @@ import { RequestAdapter } from '@kbn/inspector-plugin/common';
|
|||
import { KibanaContextProvider } from '@kbn/kibana-react-plugin/public';
|
||||
import { buildDataTableRecord } from '@kbn/discover-utils';
|
||||
import { getDiscoverStateMock } from '../../../../__mocks__/discover_state.mock';
|
||||
import { createSearchSessionMock } from '../../../../__mocks__/search_session';
|
||||
import { getSessionServiceMock } from '@kbn/data-plugin/public/search/session/mocks';
|
||||
import { DiscoverMainProvider } from '../../state_management/discover_state_provider';
|
||||
import { act } from 'react-dom/test-utils';
|
||||
|
@ -114,7 +113,11 @@ async function mountComponent(
|
|||
);
|
||||
stateContainer.internalState.dispatch(
|
||||
stateContainer.injectCurrentTab(internalStateActions.setDataRequestParams)({
|
||||
dataRequestParams: { timeRangeAbsolute: time, timeRangeRelative: time },
|
||||
dataRequestParams: {
|
||||
timeRangeAbsolute: time,
|
||||
timeRangeRelative: time,
|
||||
searchSessionId: '123',
|
||||
},
|
||||
})
|
||||
);
|
||||
|
||||
|
@ -131,7 +134,6 @@ async function mountComponent(
|
|||
setExpandedDoc: jest.fn(),
|
||||
updateDataViewList: jest.fn(),
|
||||
};
|
||||
stateContainer.searchSessionManager = createSearchSessionMock(session).searchSessionManager;
|
||||
|
||||
const component = mountWithIntl(
|
||||
<KibanaContextProvider services={services}>
|
||||
|
|
|
@ -229,7 +229,11 @@ export const useDiscoverHistogram = ({
|
|||
*/
|
||||
const { query, filters } = useQuerySubscriber({ data: services.data });
|
||||
const requestParams = useCurrentTabSelector((state) => state.dataRequestParams);
|
||||
const { timeRangeRelative: relativeTimeRange, timeRangeAbsolute: timeRange } = requestParams;
|
||||
const {
|
||||
timeRangeRelative: relativeTimeRange,
|
||||
timeRangeAbsolute: timeRange,
|
||||
searchSessionId,
|
||||
} = requestParams;
|
||||
// When in ES|QL mode, update the data view, query, and
|
||||
// columns only when documents are done fetching so the Lens suggestions
|
||||
// don't frequently change, such as when the user modifies the table
|
||||
|
@ -413,6 +417,7 @@ export const useDiscoverHistogram = ({
|
|||
onVisContextChanged: isEsqlMode ? onVisContextChanged : undefined,
|
||||
breakdownField,
|
||||
onBreakdownFieldChange,
|
||||
searchSessionId,
|
||||
};
|
||||
};
|
||||
|
||||
|
|
|
@ -35,7 +35,10 @@ export const TabsView = (props: DiscoverSessionViewProps) => {
|
|||
<UnifiedTabs
|
||||
services={services}
|
||||
initialItems={initialItems}
|
||||
onChanged={(updateState) => dispatch(internalStateActions.updateTabs(updateState))}
|
||||
onChanged={(updateState) => {
|
||||
const updateTabsAction = internalStateActions.updateTabs(updateState);
|
||||
return dispatch(updateTabsAction);
|
||||
}}
|
||||
createItem={() => createTabItem(allTabs)}
|
||||
getPreviewData={(item) => {
|
||||
const defaultQuery = { language: 'kuery', query: '(Empty query)' };
|
||||
|
|
|
@ -47,7 +47,6 @@ describe('test getDataStateContainer', () => {
|
|||
test('refetch$ triggers a search', async () => {
|
||||
const stateContainer = getDiscoverStateMock({ isTimeBased: true });
|
||||
jest.spyOn(stateContainer.searchSessionManager, 'getNextSearchSessionId');
|
||||
jest.spyOn(stateContainer.searchSessionManager, 'getCurrentSearchSessionId');
|
||||
expect(
|
||||
stateContainer.searchSessionManager.getNextSearchSessionId as jest.Mock
|
||||
).not.toHaveBeenCalled();
|
||||
|
@ -85,9 +84,6 @@ describe('test getDataStateContainer', () => {
|
|||
expect(
|
||||
stateContainer.searchSessionManager.getNextSearchSessionId as jest.Mock
|
||||
).toHaveBeenCalled();
|
||||
expect(
|
||||
stateContainer.searchSessionManager.getCurrentSearchSessionId as jest.Mock
|
||||
).not.toHaveBeenCalled();
|
||||
|
||||
unsubscribe();
|
||||
});
|
||||
|
@ -131,11 +127,6 @@ describe('test getDataStateContainer', () => {
|
|||
result: initialRecords,
|
||||
}) as DataDocuments$;
|
||||
|
||||
jest.spyOn(stateContainer.searchSessionManager, 'getCurrentSearchSessionId');
|
||||
expect(
|
||||
stateContainer.searchSessionManager.getCurrentSearchSessionId as jest.Mock
|
||||
).not.toHaveBeenCalled();
|
||||
|
||||
const dataState = stateContainer.dataState;
|
||||
const unsubscribe = dataState.subscribe();
|
||||
const resolveDataSourceProfileSpy = jest.spyOn(
|
||||
|
@ -157,10 +148,6 @@ describe('test getDataStateContainer', () => {
|
|||
if (hasLoadingMoreStarted && value.fetchStatus === FetchStatus.COMPLETE) {
|
||||
expect(resolveDataSourceProfileSpy).not.toHaveBeenCalled();
|
||||
expect(value.result).toEqual([...initialRecords, ...moreRecords]);
|
||||
// it uses the same current search session id
|
||||
expect(
|
||||
stateContainer.searchSessionManager.getCurrentSearchSessionId as jest.Mock
|
||||
).toHaveBeenCalled();
|
||||
unsubscribe();
|
||||
done();
|
||||
}
|
||||
|
|
|
@ -210,9 +210,6 @@ export function getDataStateContainer({
|
|||
reset: val === 'reset',
|
||||
fetchMore: val === 'fetch_more',
|
||||
},
|
||||
searchSessionId:
|
||||
(val === 'fetch_more' && searchSessionManager.getCurrentSearchSessionId()) ||
|
||||
searchSessionManager.getNextSearchSessionId(),
|
||||
})),
|
||||
share()
|
||||
);
|
||||
|
@ -222,7 +219,13 @@ export function getDataStateContainer({
|
|||
function subscribe() {
|
||||
const subscription = fetch$
|
||||
.pipe(
|
||||
mergeMap(async ({ options, searchSessionId }) => {
|
||||
mergeMap(async ({ options }) => {
|
||||
const { id: currentTabId, resetDefaultProfileState, dataRequestParams } = getCurrentTab();
|
||||
|
||||
const searchSessionId =
|
||||
(options.fetchMore && dataRequestParams.searchSessionId) ||
|
||||
searchSessionManager.getNextSearchSessionId();
|
||||
|
||||
const commonFetchDeps = {
|
||||
initialFetchStatus: getInitialFetchStatus(),
|
||||
inspectorAdapters,
|
||||
|
@ -259,6 +262,7 @@ export function getDataStateContainer({
|
|||
dataRequestParams: {
|
||||
timeRangeAbsolute: timefilter.getAbsoluteTime(),
|
||||
timeRangeRelative: timefilter.getTime(),
|
||||
searchSessionId,
|
||||
},
|
||||
})
|
||||
);
|
||||
|
@ -269,7 +273,6 @@ export function getDataStateContainer({
|
|||
query: appStateContainer.getState().query,
|
||||
});
|
||||
|
||||
const { id: currentTabId, resetDefaultProfileState } = getCurrentTab();
|
||||
const { currentDataView$ } = selectTabRuntimeState(runtimeStateManager, currentTabId);
|
||||
const dataView = currentDataView$.getValue();
|
||||
const defaultProfileState = dataView
|
||||
|
@ -354,7 +357,7 @@ export function getDataStateContainer({
|
|||
};
|
||||
}
|
||||
|
||||
const fetchQuery = async (resetQuery?: boolean) => {
|
||||
const fetchQuery = async () => {
|
||||
const query = appStateContainer.getState().query;
|
||||
const currentDataView = getSavedSearch().searchSource.getField('index');
|
||||
|
||||
|
@ -365,11 +368,7 @@ export function getDataStateContainer({
|
|||
}
|
||||
}
|
||||
|
||||
if (resetQuery) {
|
||||
refetch$.next('reset');
|
||||
} else {
|
||||
refetch$.next(undefined);
|
||||
}
|
||||
refetch$.next(undefined);
|
||||
|
||||
return refetch$;
|
||||
};
|
||||
|
|
|
@ -32,7 +32,6 @@ export class DiscoverSearchSessionManager {
|
|||
* skips if `searchSessionId` matches current search session id
|
||||
*/
|
||||
readonly newSearchSessionIdFromURL$: Rx.Observable<string | null>;
|
||||
readonly searchSessionId$: Rx.Observable<string | undefined>;
|
||||
|
||||
private readonly deps: DiscoverSearchSessionManagerDeps;
|
||||
|
||||
|
@ -47,7 +46,6 @@ export class DiscoverSearchSessionManager {
|
|||
return !this.deps.session.isCurrentSession(searchSessionId);
|
||||
})
|
||||
);
|
||||
this.searchSessionId$ = this.deps.session.getSession$();
|
||||
}
|
||||
|
||||
/**
|
||||
|
@ -71,13 +69,6 @@ export class DiscoverSearchSessionManager {
|
|||
return searchSessionIdFromURL ?? this.deps.session.start();
|
||||
}
|
||||
|
||||
/**
|
||||
* Get current search session id
|
||||
*/
|
||||
getCurrentSearchSessionId() {
|
||||
return this.deps.session.getSessionId();
|
||||
}
|
||||
|
||||
/**
|
||||
* Removes Discovers {@link SEARCH_SESSION_ID_QUERY_PARAM} from the URL
|
||||
* @param replace - methods to change the URL
|
||||
|
|
|
@ -41,6 +41,7 @@ export type ChartRequest = RequestState<{}>;
|
|||
export interface InternalStateDataRequestParams {
|
||||
timeRangeAbsolute?: TimeRange;
|
||||
timeRangeRelative?: TimeRange;
|
||||
searchSessionId?: string;
|
||||
}
|
||||
|
||||
export interface TabState extends TabItem {
|
||||
|
|
Loading…
Add table
Add a link
Reference in a new issue