Skip to content

Commit

Permalink
Keep volatile information out of stored search source (#91517) (#91671)
Browse files Browse the repository at this point in the history
  • Loading branch information
flash1293 authored Feb 17, 2021
1 parent 995eed2 commit d41d82d
Show file tree
Hide file tree
Showing 12 changed files with 123 additions and 89 deletions.
47 changes: 23 additions & 24 deletions src/plugins/discover/public/application/angular/discover.js
Original file line number Diff line number Diff line change
Expand Up @@ -169,10 +169,10 @@ function discoverController($route, $scope, Promise) {
let inspectorRequest;
let isChangingIndexPattern = false;
const savedSearch = $route.current.locals.savedObjects.savedSearch;
$scope.searchSource = savedSearch.searchSource;
const persistentSearchSource = savedSearch.searchSource;
$scope.indexPattern = resolveIndexPattern(
$route.current.locals.savedObjects.ip,
$scope.searchSource,
persistentSearchSource,
toastNotifications
);
$scope.useNewFieldsApi = !config.get(SEARCH_FIELDS_FROM_SOURCE);
Expand Down Expand Up @@ -370,25 +370,19 @@ function discoverController($route, $scope, Promise) {
});
};

$scope.searchSource
.setField('index', $scope.indexPattern)
.setField('highlightAll', true)
.setField('version', true);

// Even when searching rollups, we want to use the default strategy so that we get back a
// document-like response.
$scope.searchSource.setPreferredSearchStrategyId('default');
persistentSearchSource.setField('index', $scope.indexPattern);

// searchSource which applies time range
const timeRangeSearchSource = savedSearch.searchSource.create();
const volatileSearchSource = savedSearch.searchSource.create();

if (isDefaultType($scope.indexPattern)) {
timeRangeSearchSource.setField('filter', () => {
volatileSearchSource.setField('filter', () => {
return timefilter.createFilter($scope.indexPattern);
});
}

$scope.searchSource.setParent(timeRangeSearchSource);
volatileSearchSource.setParent(persistentSearchSource);
$scope.volatileSearchSource = volatileSearchSource;

const pageTitleSuffix = savedSearch.id && savedSearch.title ? `: ${savedSearch.title}` : '';
chrome.docTitle.change(`Discover${pageTitleSuffix}`);
Expand All @@ -403,7 +397,8 @@ function discoverController($route, $scope, Promise) {
}

function getStateDefaults() {
const query = $scope.searchSource.getField('query') || data.query.queryString.getDefaultQuery();
const query =
persistentSearchSource.getField('query') || data.query.queryString.getDefaultQuery();
const sort = getSortArray(savedSearch.sort, $scope.indexPattern);
const columns = getDefaultColumns();

Expand All @@ -415,7 +410,7 @@ function discoverController($route, $scope, Promise) {
columns,
index: $scope.indexPattern.id,
interval: 'auto',
filters: _.cloneDeep($scope.searchSource.getOwnField('filter')),
filters: _.cloneDeep(persistentSearchSource.getOwnField('filter')),
};
if (savedSearch.grid) {
defaultState.grid = savedSearch.grid;
Expand Down Expand Up @@ -556,7 +551,7 @@ function discoverController($route, $scope, Promise) {
.then(function () {
$scope.fetchStatus = fetchStatuses.LOADING;
logInspectorRequest({ searchSessionId });
return $scope.searchSource.fetch({
return $scope.volatileSearchSource.fetch({
abortSignal: abortController.signal,
sessionId: searchSessionId,
});
Expand Down Expand Up @@ -603,11 +598,13 @@ function discoverController($route, $scope, Promise) {
}

function onResults(resp) {
inspectorRequest.stats(getResponseInspectorStats(resp, $scope.searchSource)).ok({ json: resp });
inspectorRequest
.stats(getResponseInspectorStats(resp, $scope.volatileSearchSource))
.ok({ json: resp });

if (getTimeField() && !$scope.state.hideChart) {
const tabifiedData = tabifyAggResponse($scope.opts.chartAggConfigs, resp);
$scope.searchSource.rawResponse = resp;
$scope.volatileSearchSource.rawResponse = resp;
$scope.histogramData = discoverResponseHandler(
tabifiedData,
getDimensions($scope.opts.chartAggConfigs.aggs, $scope.timeRange)
Expand Down Expand Up @@ -635,8 +632,8 @@ function discoverController($route, $scope, Promise) {
defaultMessage: 'This request queries Elasticsearch to fetch the data for the search.',
});
inspectorRequest = inspectorAdapters.requests.start(title, { description, searchSessionId });
inspectorRequest.stats(getRequestInspectorStats($scope.searchSource));
$scope.searchSource.getSearchRequestBody().then((body) => {
inspectorRequest.stats(getRequestInspectorStats($scope.volatileSearchSource));
$scope.volatileSearchSource.getSearchRequestBody().then((body) => {
inspectorRequest.json(body);
});
}
Expand Down Expand Up @@ -693,9 +690,11 @@ function discoverController($route, $scope, Promise) {
};

$scope.updateDataSource = () => {
const { indexPattern, searchSource, useNewFieldsApi } = $scope;
const { indexPattern, useNewFieldsApi } = $scope;
const { columns, sort } = $scope.state;
updateSearchSource(searchSource, {
updateSearchSource({
persistentSearchSource,
volatileSearchSource: $scope.volatileSearchSource,
indexPattern,
services,
sort,
Expand Down Expand Up @@ -731,12 +730,12 @@ function discoverController($route, $scope, Promise) {
visStateAggs
);

$scope.searchSource.onRequestStart((searchSource, options) => {
$scope.volatileSearchSource.onRequestStart((searchSource, options) => {
if (!$scope.opts.chartAggConfigs) return;
return $scope.opts.chartAggConfigs.onSearchRequestStart(searchSource, options);
});

$scope.searchSource.setField('aggs', function () {
$scope.volatileSearchSource.setField('aggs', function () {
if (!$scope.opts.chartAggConfigs) return;
return $scope.opts.chartAggConfigs.toDsl();
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
reset-query="resetQuery"
result-state="resultState"
rows="rows"
search-source="searchSource"
search-source="volatileSearchSource"
set-index-pattern="setIndexPattern"
show-save-query="showSaveQuery"
state="state"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
reset-query="resetQuery"
result-state="resultState"
rows="rows"
search-source="searchSource"
search-source="volatileSearchSource"
state="state"
time-range="timeRange"
top-nav-menu="topNavMenu"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,7 @@ export function Discover({
query={state.query}
savedQuery={state.savedQuery}
updateQuery={updateQuery}
searchSource={searchSource}
/>
<EuiPageBody className="dscPageBody" aria-describedby="savedSearchTitle">
<h1 id="savedSearchTitle" className="euiScreenReaderOnly">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import { SavedObject } from '../../../../../core/types';
import { DiscoverTopNav, DiscoverTopNavProps } from './discover_topnav';
import { RequestAdapter } from '../../../../inspector/common/adapters/request';
import { TopNavMenu } from '../../../../navigation/public';
import { Query } from '../../../../data/common';
import { ISearchSource, Query } from '../../../../data/common';
import { DiscoverSearchSessionManager } from '../angular/discover_search_session';
import { Subject } from 'rxjs';

Expand Down Expand Up @@ -61,6 +61,7 @@ function getProps(): DiscoverTopNavProps {
savedQuery: '',
updateQuery: jest.fn(),
onOpenInspector: jest.fn(),
searchSource: {} as ISearchSource,
};
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import { DiscoverProps } from './types';
import { getTopNavLinks } from './top_nav/get_top_nav_links';
import { Query, TimeRange } from '../../../../data/common/query';

export type DiscoverTopNavProps = Pick<DiscoverProps, 'indexPattern' | 'opts'> & {
export type DiscoverTopNavProps = Pick<DiscoverProps, 'indexPattern' | 'opts' | 'searchSource'> & {
onOpenInspector: () => void;
query?: Query;
savedQuery?: string;
Expand All @@ -24,6 +24,7 @@ export const DiscoverTopNav = ({
query,
savedQuery,
updateQuery,
searchSource,
}: DiscoverTopNavProps) => {
const showDatePicker = useMemo(() => indexPattern.isTimeBased(), [indexPattern]);
const { TopNavMenu } = opts.services.navigation.ui;
Expand All @@ -38,8 +39,9 @@ export const DiscoverTopNav = ({
services: opts.services,
state: opts.stateContainer,
onOpenInspector,
searchSource,
}),
[indexPattern, opts, onOpenInspector]
[indexPattern, opts, onOpenInspector, searchSource]
);

const updateSavedQueryId = (newSavedQueryId: string | undefined) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
* Side Public License, v 1.
*/

import { ISearchSource } from 'src/plugins/data/public';
import { getTopNavLinks } from './get_top_nav_links';
import { inspectorPluginMock } from '../../../../../inspector/public/mocks';
import { indexPatternMock } from '../../../__mocks__/index_pattern';
Expand Down Expand Up @@ -33,6 +34,7 @@ test('getTopNavLinks result', () => {
savedSearch: savedSearchMock,
services,
state,
searchSource: {} as ISearchSource,
});
expect(topNavLinks).toMatchInlineSnapshot(`
Array [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import { Adapters } from '../../../../../inspector/common/adapters';
import { SavedSearch } from '../../../saved_searches';
import { onSaveSearch } from './on_save_search';
import { GetStateReturn } from '../../angular/discover_state';
import { IndexPattern } from '../../../kibana_services';
import { IndexPattern, ISearchSource } from '../../../kibana_services';

/**
* Helper function to build the top nav links
Expand All @@ -29,6 +29,7 @@ export const getTopNavLinks = ({
services,
state,
onOpenInspector,
searchSource,
}: {
getFieldCounts: () => Promise<Record<string, number>>;
indexPattern: IndexPattern;
Expand All @@ -38,6 +39,7 @@ export const getTopNavLinks = ({
services: DiscoverServices;
state: GetStateReturn;
onOpenInspector: () => void;
searchSource: ISearchSource;
}) => {
const newSearch = {
id: 'new',
Expand Down Expand Up @@ -93,7 +95,7 @@ export const getTopNavLinks = ({
return;
}
const sharingData = await getSharingData(
savedSearch.searchSource,
searchSource,
state.appStateContainer.getState(),
services.uiSettings,
getFieldCounts
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
import { Capabilities, IUiSettingsClient } from 'kibana/public';
import { DOC_HIDE_TIME_COLUMN_SETTING, SORT_DEFAULT_ORDER_SETTING } from '../../../common';
import { getSortForSearchSource } from '../angular/doc_table';
import { SearchSource } from '../../../../data/common';
import { ISearchSource } from '../../../../data/common';
import { AppState } from '../angular/discover_state';
import { SortOrder } from '../../saved_searches/types';

Expand Down Expand Up @@ -39,7 +39,7 @@ const getSharingDataFields = async (
* Preparing data to share the current state as link or CSV/Report
*/
export async function getSharingData(
currentSearchSource: SearchSource,
currentSearchSource: ISearchSource,
state: AppState,
config: IUiSettingsClient,
getFieldCounts: () => Promise<Record<string, number>>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,8 @@ export async function persistSavedSearch(
state: AppState;
}
) {
updateSearchSource(savedSearch.searchSource, {
updateSearchSource({
persistentSearchSource: savedSearch.searchSource,
indexPattern,
services,
sort: state.sort as SortOrder[],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,12 @@ import { SortOrder } from '../../saved_searches/types';

describe('updateSearchSource', () => {
test('updates a given search source', async () => {
const searchSourceMock = createSearchSourceMock({});
const persistentSearchSourceMock = createSearchSourceMock({});
const volatileSearchSourceMock = createSearchSourceMock({});
const sampleSize = 250;
const result = updateSearchSource(searchSourceMock, {
updateSearchSource({
persistentSearchSource: persistentSearchSourceMock,
volatileSearchSource: volatileSearchSourceMock,
indexPattern: indexPatternMock,
services: ({
data: dataPluginMock.createStartContract(),
Expand All @@ -36,15 +39,18 @@ describe('updateSearchSource', () => {
columns: [],
useNewFieldsApi: false,
});
expect(result.getField('index')).toEqual(indexPatternMock);
expect(result.getField('size')).toEqual(sampleSize);
expect(result.getField('fields')).toBe(undefined);
expect(persistentSearchSourceMock.getField('index')).toEqual(indexPatternMock);
expect(volatileSearchSourceMock.getField('size')).toEqual(sampleSize);
expect(volatileSearchSourceMock.getField('fields')).toBe(undefined);
});

test('updates a given search source with the usage of the new fields api', async () => {
const searchSourceMock = createSearchSourceMock({});
const persistentSearchSourceMock = createSearchSourceMock({});
const volatileSearchSourceMock = createSearchSourceMock({});
const sampleSize = 250;
const result = updateSearchSource(searchSourceMock, {
updateSearchSource({
persistentSearchSource: persistentSearchSourceMock,
volatileSearchSource: volatileSearchSourceMock,
indexPattern: indexPatternMock,
services: ({
data: dataPluginMock.createStartContract(),
Expand All @@ -61,16 +67,19 @@ describe('updateSearchSource', () => {
columns: [],
useNewFieldsApi: true,
});
expect(result.getField('index')).toEqual(indexPatternMock);
expect(result.getField('size')).toEqual(sampleSize);
expect(result.getField('fields')).toEqual([{ field: '*' }]);
expect(result.getField('fieldsFromSource')).toBe(undefined);
expect(persistentSearchSourceMock.getField('index')).toEqual(indexPatternMock);
expect(volatileSearchSourceMock.getField('size')).toEqual(sampleSize);
expect(volatileSearchSourceMock.getField('fields')).toEqual([{ field: '*' }]);
expect(volatileSearchSourceMock.getField('fieldsFromSource')).toBe(undefined);
});

test('requests unmapped fields when the flag is provided, using the new fields api', async () => {
const searchSourceMock = createSearchSourceMock({});
const persistentSearchSourceMock = createSearchSourceMock({});
const volatileSearchSourceMock = createSearchSourceMock({});
const sampleSize = 250;
const result = updateSearchSource(searchSourceMock, {
updateSearchSource({
persistentSearchSource: persistentSearchSourceMock,
volatileSearchSource: volatileSearchSourceMock,
indexPattern: indexPatternMock,
services: ({
data: dataPluginMock.createStartContract(),
Expand All @@ -88,16 +97,21 @@ describe('updateSearchSource', () => {
useNewFieldsApi: true,
showUnmappedFields: true,
});
expect(result.getField('index')).toEqual(indexPatternMock);
expect(result.getField('size')).toEqual(sampleSize);
expect(result.getField('fields')).toEqual([{ field: '*', include_unmapped: 'true' }]);
expect(result.getField('fieldsFromSource')).toBe(undefined);
expect(persistentSearchSourceMock.getField('index')).toEqual(indexPatternMock);
expect(volatileSearchSourceMock.getField('size')).toEqual(sampleSize);
expect(volatileSearchSourceMock.getField('fields')).toEqual([
{ field: '*', include_unmapped: 'true' },
]);
expect(volatileSearchSourceMock.getField('fieldsFromSource')).toBe(undefined);
});

test('updates a given search source when showUnmappedFields option is set to true', async () => {
const searchSourceMock = createSearchSourceMock({});
const persistentSearchSourceMock = createSearchSourceMock({});
const volatileSearchSourceMock = createSearchSourceMock({});
const sampleSize = 250;
const result = updateSearchSource(searchSourceMock, {
updateSearchSource({
persistentSearchSource: persistentSearchSourceMock,
volatileSearchSource: volatileSearchSourceMock,
indexPattern: indexPatternMock,
services: ({
data: dataPluginMock.createStartContract(),
Expand All @@ -115,9 +129,11 @@ describe('updateSearchSource', () => {
useNewFieldsApi: true,
showUnmappedFields: true,
});
expect(result.getField('index')).toEqual(indexPatternMock);
expect(result.getField('size')).toEqual(sampleSize);
expect(result.getField('fields')).toEqual([{ field: '*', include_unmapped: 'true' }]);
expect(result.getField('fieldsFromSource')).toBe(undefined);
expect(persistentSearchSourceMock.getField('index')).toEqual(indexPatternMock);
expect(volatileSearchSourceMock.getField('size')).toEqual(sampleSize);
expect(volatileSearchSourceMock.getField('fields')).toEqual([
{ field: '*', include_unmapped: 'true' },
]);
expect(volatileSearchSourceMock.getField('fieldsFromSource')).toBe(undefined);
});
});
Loading

0 comments on commit d41d82d

Please sign in to comment.