Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ import { AlertsTable } from '@kbn/response-ops-alerts-table';
import type { PackageListItem } from '@kbn/fleet-plugin/common';
import type { QueryDslQueryContainer } from '@elastic/elasticsearch/lib/api/types';
import type { AlertsTableImperativeApi } from '@kbn/response-ops-alerts-table/types';
import { useBrowserFields } from '../../../../../../../data_view_manager/hooks/use_browser_fields';
import { DataViewManagerScopeName } from '../../../../../../../data_view_manager/constants';
import type { AdditionalTableContext } from '../../../../../../../detections/components/alert_summary/table/table';
import {
ACTION_COLUMN_WIDTH,
Expand All @@ -24,7 +26,6 @@ import {
TOOLBAR_VISIBILITY,
} from '../../../../../../../detections/components/alert_summary/table/table';
import { ActionsCell } from '../../../../../../../detections/components/alert_summary/table/actions_cell';
import { getDataViewStateFromIndexFields } from '../../../../../../../common/containers/source/use_data_view';
import { useKibana } from '../../../../../../../common/lib/kibana';
import { CellValue } from '../../../../../../../detections/components/alert_summary/table/render_cell';
import type { RuleResponse } from '../../../../../../../../common/api/detection_engine';
Expand Down Expand Up @@ -84,12 +85,7 @@ export const Table = memo(({ dataView, id, packages, query, ruleResponse }: Tabl
[application, cases, data, fieldFormats, http, licensing, notifications, settings]
);

const dataViewSpec = useMemo(() => dataView.toSpec(), [dataView]);

const { browserFields } = useMemo(
() => getDataViewStateFromIndexFields('', dataViewSpec.fields),
[dataViewSpec.fields]
);
const browserFields = useBrowserFields(DataViewManagerScopeName.detections, dataView);

const additionalContext: AdditionalTableContext = useMemo(
() => ({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,11 @@ import type {
} from '@kbn/response-ops-alerts-table/types';
import React, { memo, useCallback, useMemo, useRef } from 'react';
import type { RuleResponse } from '../../../../common/api/detection_engine';
import { getDataViewStateFromIndexFields } from '../../../common/containers/source/use_data_view';
import { useKibana } from '../../../common/lib/kibana';
import { ActionsCell } from '../../../detections/components/alert_summary/table/actions_cell';
import { CellValue } from '../../../detections/components/alert_summary/table/render_cell';
import { useBrowserFields } from '../../../data_view_manager/hooks/use_browser_fields';
import { DataViewManagerScopeName } from '../../../data_view_manager/constants';
import type { AdditionalTableContext } from '../../../detections/components/alert_summary/table/table';
import {
ACTION_COLUMN_WIDTH,
Expand Down Expand Up @@ -101,12 +102,7 @@ export const Table = memo(
[application, cases, data, fieldFormats, http, licensing, notifications, settings]
);

const dataViewSpec = useMemo(() => dataView.toSpec(), [dataView]);

const { browserFields } = useMemo(
() => getDataViewStateFromIndexFields('', dataViewSpec.fields),
[dataViewSpec.fields]
);
const browserFields = useBrowserFields(DataViewManagerScopeName.detections, dataView);

const additionalContext: AdditionalTableContext = useMemo(
() => ({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,9 @@ import type { BrowserFields } from '@kbn/timelines-plugin/common';
import type { FieldSpec, IIndexPatternFieldList } from '@kbn/data-views-plugin/common';
import type { DataViewSpec } from '@kbn/data-views-plugin/public';

import { browserFieldsManager } from '../../../data_view_manager/utils/security_browser_fields_manager';
import { useKibana } from '../../lib/kibana';
import * as i18n from './translations';
import { getDataViewStateFromIndexFields } from './use_data_view';
import { useAppToasts } from '../../hooks/use_app_toasts';
import type { ENDPOINT_FIELDS_SEARCH_STRATEGY } from '../../../../common/endpoint/constants';

Expand Down Expand Up @@ -115,10 +115,7 @@ export const useFetchIndex = (
abortCtrl.current = new AbortController();
const dv = await data.dataViews.create({ title: iNames.join(','), allowNoIndex: true });
const dataView = dv.toSpec();
const { browserFields } = getDataViewStateFromIndexFields(
iNames.join(','),
dataView.fields
);
const { browserFields } = browserFieldsManager.getBrowserFields(dv);
Comment thread
michaelolo24 marked this conversation as resolved.

previousIndexesName.current = dv.getIndexPattern().split(',');

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import { useCallback, useRef } from 'react';
import type { Subscription } from 'rxjs';
import { useDispatch } from 'react-redux';
import memoizeOne from 'memoize-one';
import deepEqual from 'fast-deep-equal';
import type { BrowserFields } from '@kbn/timelines-plugin/common';
import type { DataViewSpec } from '@kbn/data-views-plugin/public';
import type { FieldCategory } from '@kbn/timelines-plugin/common/search_strategy';
Expand Down Expand Up @@ -44,6 +45,9 @@ interface DataViewInfo {
/**
* HOT Code path where the fields can be 16087 in length or larger. This is
* VERY mutatious on purpose to improve the performance of the transform.
* TODO: newDataViewPickerEnabled - consider removing this in favor of the
* buildBrowserFieldsFromDataView util at x-pack/solutions/security/plugins/security_solution/public/data_view_manager/utils/build_browser_fields.ts
* which utilizes the less expensive DataView.fields instead of the DataViewSpec.fields which is much more expensive to build.
*/
export const getDataViewStateFromIndexFields = memoizeOne(
(_title: string, fields: DataViewSpec['fields']): DataViewInfo => {
Expand All @@ -66,7 +70,7 @@ export const getDataViewStateFromIndexFields = memoizeOne(
return { browserFields: browserFields as DangerCastForBrowserFieldsMutation };
}
},
(newArgs, lastArgs) => newArgs[0] === lastArgs[0] && newArgs[1]?.length === lastArgs[1]?.length
(newArgs, lastArgs) => deepEqual(newArgs, lastArgs) // DataViewSpec['fields'] is an object, so we cannot do a length check.
);

export const useDataView = (): {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import { TestProviders } from '../../../common/mock/test_providers';
import { useSelectDataView } from '../../hooks/use_select_data_view';
import { useUpdateUrlParam } from '../../../common/utils/global_query_string';
import { URL_PARAM_KEY } from '../../../common/hooks/constants';
import { useKibana as mockUseKibana } from '../../../common/lib/kibana/__mocks__';

jest.mock('../../../common/utils/global_query_string', () => ({
useUpdateUrlParam: jest.fn(),
Expand All @@ -39,9 +40,7 @@ jest.mock('react-redux', () => {
};
});

jest.mock('../../../common/lib/kibana', () => ({
useKibana: jest.fn(),
}));
jest.mock('../../../common/lib/kibana');

jest.mock('@kbn/unified-search-plugin/public', () => ({
...jest.requireActual('@kbn/unified-search-plugin/public'),
Expand Down Expand Up @@ -93,12 +92,12 @@ describe('DataViewPicker', () => {

jest.mocked(useKibana).mockReturnValue({
services: {
...mockUseKibana().services,
dataViewFieldEditor: { openEditor: jest.fn() },
dataViewEditor: {
openEditor: jest.fn(),
userPermissions: { editDataView: jest.fn().mockReturnValue(true) },
},
data: { dataViews: { get: jest.fn() } },
},
} as unknown as ReturnType<typeof useKibana>);
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import { useSavedDataViews } from '../../hooks/use_saved_data_views';
import { DEFAULT_SECURITY_DATA_VIEW, LOADING } from './translations';
import { DATA_VIEW_PICKER_TEST_ID } from './constants';
import { useDataView } from '../../hooks/use_data_view';
import { browserFieldsManager } from '../../utils/security_browser_fields_manager';

interface DataViewPickerProps {
/**
Expand Down Expand Up @@ -75,6 +76,7 @@ export const DataViewPicker = memo(({ scope, onClosePopover, disabled }: DataVie
// hence - it is the only place where we should update the url param for the data view selection.
const handleChangeDataView = useCallback(
(id: string, indexPattern: string = '') => {
browserFieldsManager.removeFromCache(scope);
selectDataView({ id, scope });

if (isDefaultSourcerer) {
Expand Down Expand Up @@ -111,6 +113,9 @@ export const DataViewPicker = memo(({ scope, onClosePopover, disabled }: DataVie
}

const dataViewInstance = await data.dataViews.get(dataViewId);
// Modifications to the fields do not trigger cache invalidation, but should as `fields` will be stale.
data.dataViews.clearInstanceCache(dataViewId);
browserFieldsManager.removeFromCache(scope);

closeFieldEditor.current = await dataViewFieldEditor.openEditor({
ctx: {
Expand All @@ -126,7 +131,7 @@ export const DataViewPicker = memo(({ scope, onClosePopover, disabled }: DataVie
},
});
},
[dataViewId, data.dataViews, dataViewFieldEditor, handleChangeDataView]
[dataViewId, data.dataViews, scope, dataViewFieldEditor, handleChangeDataView]
);

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,30 +9,43 @@ import { renderHook } from '@testing-library/react';
import { TestProviders } from '../../common/mock';
import { useBrowserFields } from './use_browser_fields';
import { DEFAULT_SECURITY_SOLUTION_DATA_VIEW_ID, DataViewManagerScopeName } from '../constants';
import { useDataViewSpec } from './use_data_view_spec';
import { type FieldSpec } from '@kbn/data-views-plugin/common';
import { useDataView } from './use_data_view';
import { DataView } from '@kbn/data-views-plugin/common';
import { useIsExperimentalFeatureEnabled } from '../../common/hooks/use_experimental_features';

jest.mock('./use_data_view_spec', () => ({
useDataViewSpec: jest.fn(),
jest.mock('../../common/hooks/use_experimental_features');

jest.mock('./use_data_view', () => ({
useDataView: jest.fn(),
}));

describe('useBrowserFields', () => {
beforeAll(() => {
jest.mocked(useDataViewSpec).mockReturnValue({
dataViewSpec: {
id: DEFAULT_SECURITY_SOLUTION_DATA_VIEW_ID,
fields: {
'@timestamp': {
type: 'date',
name: '@timestamp',
} as FieldSpec,
jest.mocked(useDataView).mockReturnValue({
dataView: new DataView({
spec: {
id: DEFAULT_SECURITY_SOLUTION_DATA_VIEW_ID,
title: 'security-solution-data-view',
fields: {
'@timestamp': {
name: '@timestamp',
type: 'date',
esTypes: ['date'],
aggregatable: true,
searchable: true,
scripted: false,
},
},
},
},
// @ts-expect-error: DataView constructor expects more, but this is enough for our test
fieldFormats: { getDefaultInstance: () => ({}) },
}),
status: 'ready',
});
});

it('should call the useDataView hook and return browser fields map', () => {
jest.mocked(useIsExperimentalFeatureEnabled).mockReturnValue(true);
const wrapper = renderHook(() => useBrowserFields(DataViewManagerScopeName.default), {
wrapper: TestProviders,
});
Expand All @@ -42,7 +55,62 @@ describe('useBrowserFields', () => {
"base": Object {
"fields": Object {
"@timestamp": Object {
"aggregatable": true,
"esTypes": Array [
"date",
],
"name": "@timestamp",
"scripted": false,
"searchable": true,
"shortDotsEnable": false,
"type": "date",
},
},
},
}
`);
});

it('should use the passed in dataView when the feature flag is disabled', () => {
jest.mocked(useIsExperimentalFeatureEnabled).mockReturnValue(false);
const oldDataView = new DataView({
spec: {
id: 'old-dataView',
title: 'security-solution-data-view-old',
fields: {
'@timestamp': {
name: '@timestamp',
type: 'date',
esTypes: ['date'],
aggregatable: true,
searchable: true,
scripted: false,
},
},
},
// @ts-expect-error: DataView constructor expects more, but this is enough for our test
fieldFormats: { getDefaultInstance: () => ({}) },
});
const wrapper = renderHook(
() => useBrowserFields(DataViewManagerScopeName.default, oldDataView),
{
wrapper: TestProviders,
}
);

expect(wrapper.result.current).toMatchInlineSnapshot(`
Object {
"base": Object {
"fields": Object {
"@timestamp": Object {
"aggregatable": true,
"esTypes": Array [
"date",
],
"name": "@timestamp",
"scripted": false,
"searchable": true,
"shortDotsEnable": false,
"type": "date",
},
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,25 +7,30 @@

import { useMemo } from 'react';
import type { BrowserFields } from '@kbn/timelines-plugin/common';
import type { DataView } from '@kbn/data-views-plugin/common';
import { DataViewManagerScopeName } from '../constants';
import { useDataViewSpec } from './use_data_view_spec';
import { getDataViewStateFromIndexFields } from '../../common/containers/source/use_data_view';
import { useDataView } from './use_data_view';
import { browserFieldsManager } from '../utils/security_browser_fields_manager';
import { useIsExperimentalFeatureEnabled } from '../../common/hooks/use_experimental_features';

export const useBrowserFields = (
scope: DataViewManagerScopeName = DataViewManagerScopeName.default
scope: DataViewManagerScopeName = DataViewManagerScopeName.default,
/**
* @deprecated remove when newDataViewPickerEnabled is removed
*/
oldDataView?: DataView
Comment thread
michaelolo24 marked this conversation as resolved.
): BrowserFields => {
const { dataViewSpec } = useDataViewSpec(scope);
const { dataView } = useDataView(scope);
const newDataViewPickerEnabled = useIsExperimentalFeatureEnabled('newDataViewPickerEnabled');
const activeDataView = newDataViewPickerEnabled ? dataView : oldDataView;

return useMemo(() => {
if (!dataViewSpec) {
if (!activeDataView) {
return {};
}

const { browserFields } = getDataViewStateFromIndexFields(
dataViewSpec?.title ?? '',
dataViewSpec.fields
);
const { browserFields } = browserFieldsManager.getBrowserFields(activeDataView, scope);

return browserFields;
}, [dataViewSpec]);
}, [activeDataView, scope]);
};
Loading