Skip to content

Commit

Permalink
[Security Solution] getDataViewStateFromIndexFields was using wrong t…
Browse files Browse the repository at this point in the history
…ype as part of a cast (elastic#158594)

## Summary

Fixes an issue with the field browser where all types currently display
as unkown, this was because in a code path where a type cast happens, we
were using the wrong type. To see this, remove the as unknown from the
cast, and the typescript compiler will show the problem:
```
'BrowserField' is deprecated.ts(6385)
index.ts(70, 4): The declaration was marked as deprecated here.
Conversion of type 'DataViewField' to type 'BrowserField' may be a mistake because neither type sufficiently overlaps with the other. If this was intentional, convert the expression to 'unknown' first.
  Type 'DataViewField' is missing the following properties from type 'BrowserField': category, description, example, fields, and 2 more.ts(2352)
```
DataViewField actually only has spec and kbnFieldType properties, spec
is of type FieldSpec which is basically the same type as BrowserField,
and has sufficient overlap for the (still unsafe, but more safe than as
unknown) cast to occur.

Before:
<img width="338" alt="image"
src="https://github.com/elastic/kibana/assets/56408403/f31c1f9e-25f0-41ee-9e1c-a70171e41d29">

After:
<img width="555" alt="image"
src="https://github.com/elastic/kibana/assets/56408403/8b462477-2dce-41bb-9592-f34b20634b84">

---------

Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
(cherry picked from commit 1c75903)

# Conflicts:
#	x-pack/plugins/security_solution/public/detections/components/alerts_table/alerts_grouping.tsx
#	x-pack/plugins/security_solution/tsconfig.json
  • Loading branch information
kqualters-elastic committed May 31, 2023
1 parent 564ff93 commit 4483622
Show file tree
Hide file tree
Showing 27 changed files with 131 additions and 238 deletions.
Expand Up @@ -11,9 +11,14 @@ import type {
MappingRuntimeField,
MappingRuntimeFields,
} from '@elastic/elasticsearch/lib/api/typesWithBodyKey';
import type { RuntimeFieldSpec, RuntimePrimitiveTypes } from '@kbn/data-views-plugin/common';
import type { BoolQuery } from '@kbn/es-query';
import type * as estypes from '@elastic/elasticsearch/lib/api/typesWithBodyKey';

type RunTimeMappings =
| Record<string, Omit<RuntimeFieldSpec, 'type'> & { type: RuntimePrimitiveTypes }>
| undefined;

interface BoolAgg {
bool: BoolQuery;
}
Expand All @@ -29,7 +34,7 @@ export interface GroupingQueryArgs {
from: string;
groupByField: string;
rootAggregations?: NamedAggregation[];
runtimeMappings?: MappingRuntimeFields;
runtimeMappings?: RunTimeMappings;
additionalAggregationsRoot?: NamedAggregation[];
pageNumber?: number;
uniqueValue: string;
Expand Down
Expand Up @@ -27,8 +27,8 @@ import type {
TimelineRequestSortField,
TimelineStrategyResponseType,
} from '@kbn/timelines-plugin/common/search_strategy';
import type { MappingRuntimeFields } from '@elastic/elasticsearch/lib/api/types';
import { dataTableActions, Direction, TableId } from '@kbn/securitysolution-data-table';
import type { RunTimeMappings } from '../../store/sourcerer/model';
import { TimelineEventsQueries } from '../../../../common/search_strategy';
import type { KueryFilterQueryKind } from '../../../../common/types';
import type { ESQuery } from '../../../../common/typed_json';
Expand Down Expand Up @@ -77,7 +77,7 @@ export interface UseTimelineEventsProps {
indexNames: string[];
language?: KueryFilterQueryKind;
limit: number;
runtimeMappings: MappingRuntimeFields;
runtimeMappings: RunTimeMappings;
skip?: boolean;
sort?: TimelineRequestSortField[];
startDate: string;
Expand Down Expand Up @@ -321,7 +321,7 @@ export const useTimelineEventsHandler = ({
querySize: prevRequest?.pagination.querySize ?? 0,
sort: prevRequest?.sort ?? initSortDefault,
timerange: prevRequest?.timerange ?? {},
runtimeMappings: prevRequest?.runtimeMappings ?? {},
runtimeMappings: (prevRequest?.runtimeMappings ?? {}) as RunTimeMappings,
filterStatus: prevRequest?.filterStatus,
};

Expand Down
Expand Up @@ -32,6 +32,7 @@ import {
import numeral from '@elastic/numeral';
import { css } from '@emotion/react';
import type { EuiMarkdownEditorUiPluginEditorProps } from '@elastic/eui/src/components/markdown_editor/markdown_types';
import { DataView } from '@kbn/data-views-plugin/public';
import { FormattedMessage } from '@kbn/i18n-react';
import type { Filter } from '@kbn/es-query';
import { FilterStateStore } from '@kbn/es-query';
Expand Down Expand Up @@ -245,7 +246,16 @@ const InsightEditorComponent = ({
ui: { FiltersBuilderLazy },
},
uiSettings,
fieldFormats,
} = useKibana().services;
const dataView = useMemo(() => {
if (sourcererDataView != null) {
return new DataView({ spec: sourcererDataView, fieldFormats });
} else {
return null;
}
}, [sourcererDataView, fieldFormats]);

const [providers, setProviders] = useState<Provider[][]>([[]]);
const dateRangeChoices = useMemo(() => {
const settings: Array<{ from: string; to: string; display: string }> = uiSettings.get(
Expand Down Expand Up @@ -419,11 +429,11 @@ const InsightEditorComponent = ({
/>
</EuiFormRow>
<EuiFormRow label={i18n.FILTER_BUILDER} helpText={i18n.FILTER_BUILDER_TEXT} fullWidth>
{sourcererDataView ? (
{dataView ? (
<FiltersBuilderLazy
filters={filtersStub}
onChange={onChange}
dataView={sourcererDataView}
dataView={dataView}
maxDepth={2}
/>
) : (
Expand Down

This file was deleted.

Expand Up @@ -5,8 +5,6 @@
* 2.0.
*/

import type { IndexField } from '../../../../common/search_strategy/index_fields';
import { getBrowserFields, getAllBrowserFields } from '.';
import type { IndexFieldSearch } from './use_data_view';
import { useDataView } from './use_data_view';
import { mocksSource } from './mock';
Expand All @@ -27,32 +25,6 @@ jest.mock('../../lib/kibana');
jest.mock('../../lib/apm/use_track_http_request');

describe('source/index.tsx', () => {
describe('getAllBrowserFields', () => {
test('it returns an array of all fields in the BrowserFields argument', () => {
expect(
getAllBrowserFields(getBrowserFields('title 1', mocksSource.indexFields as IndexField[]))
).toMatchSnapshot();
});
});
describe('getBrowserFields', () => {
test('it returns an empty object given an empty array', () => {
const fields = getBrowserFields('title 1', []);
expect(fields).toEqual({});
});

test('it returns the same input given the same title and same fields length', () => {
const oldFields = getBrowserFields('title 1', mocksSource.indexFields as IndexField[]);
const newFields = getBrowserFields('title 1', mocksSource.indexFields as IndexField[]);
// Since it is memoized it will return the same object instance
expect(newFields).toBe(oldFields);
});

test('it transforms input into output as expected', () => {
const fields = getBrowserFields('title 2', mocksSource.indexFields as IndexField[]);
expect(fields).toMatchSnapshot();
});
});

describe('useDataView hook', () => {
const mockSearchResponse = {
...mocksSource,
Expand All @@ -74,8 +46,8 @@ describe('source/index.tsx', () => {
data: {
dataViews: {
...useKibana().services.data.dataViews,
get: async (dataViewId: string, displayErrors?: boolean, refreshFields = false) =>
Promise.resolve({
get: async (dataViewId: string, displayErrors?: boolean, refreshFields = false) => {
const dataViewMock = {
id: dataViewId,
matchedIndices: refreshFields
? ['hello', 'world', 'refreshed']
Expand All @@ -88,7 +60,12 @@ describe('source/index.tsx', () => {
type: 'keyword',
},
}),
}),
};
return Promise.resolve({
toSpec: () => dataViewMock,
...dataViewMock,
});
},
getFieldsForWildcard: async () => Promise.resolve(),
},
search: {
Expand Down Expand Up @@ -178,7 +155,6 @@ describe('source/index.tsx', () => {
const {
payload: { patternList: newPatternList },
} = mockDispatch.mock.calls[1][0];

expect(patternList).not.toBe(newPatternList);
expect(patternList).not.toContain('refreshed*');
expect(newPatternList).toContain('refreshed*');
Expand Down
Expand Up @@ -9,9 +9,9 @@ import { isEmpty, isEqual, keyBy, pick } from 'lodash/fp';
import memoizeOne from 'memoize-one';
import { useCallback, useEffect, useRef, useState } from 'react';
import type { DataViewBase } from '@kbn/es-query';
import type { BrowserField, BrowserFields, IndexField } from '@kbn/timelines-plugin/common';
import type { DataView, IIndexPatternFieldList } from '@kbn/data-views-plugin/common';
import { getCategory } from '@kbn/triggers-actions-ui-plugin/public';
import type { BrowserField, BrowserFields } from '@kbn/timelines-plugin/common';
import type { IIndexPatternFieldList } from '@kbn/data-views-plugin/common';
import type { DataViewSpec } from '@kbn/data-views-plugin/public';

import { useKibana } from '../../lib/kibana';
import * as i18n from './translations';
Expand Down Expand Up @@ -72,35 +72,6 @@ export const getIndexFields = memoizeOne(
newArgs[2] === lastArgs[2]
);

/**
* 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.
*/
export const getBrowserFields = memoizeOne(
(_title: string, fields: IndexField[]): BrowserFields => {
// Adds two dangerous casts to allow for mutations within this function
type DangerCastForMutation = Record<string, {}>;
type DangerCastForBrowserFieldsMutation = Record<
string,
Omit<BrowserField, 'fields'> & { fields: Record<string, BrowserField> }
>;

// We mutate this instead of using lodash/set to keep this as fast as possible
return fields.reduce<DangerCastForBrowserFieldsMutation>((accumulator, field) => {
const category = getCategory(field.name);
if (accumulator[category] == null) {
(accumulator as DangerCastForMutation)[category] = {};
}
if (accumulator[category].fields == null) {
accumulator[category].fields = {};
}
accumulator[category].fields[field.name] = field as unknown as BrowserField;
return accumulator;
}, {});
},
(newArgs, lastArgs) => newArgs[0] === lastArgs[0] && newArgs[1].length === lastArgs[1].length
);

const DEFAULT_BROWSER_FIELDS = {};
const DEFAULT_INDEX_PATTERNS = { fields: [], title: '' };
interface FetchIndexReturn {
Expand All @@ -115,7 +86,7 @@ interface FetchIndexReturn {
indexes: string[];
indexExists: boolean;
indexPatterns: DataViewBase;
dataView: DataView | undefined;
dataView: DataViewSpec | undefined;
}

/**
Expand Down Expand Up @@ -149,17 +120,18 @@ export const useFetchIndex = (
setState({ ...state, loading: true });
abortCtrl.current = new AbortController();
const dv = await data.dataViews.create({ title: iNames.join(','), allowNoIndex: true });
const dataView = dv.toSpec();
const { browserFields } = getDataViewStateFromIndexFields(
iNames,
dv.fields,
dataView.fields,
includeUnmapped
);

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

setState({
loading: false,
dataView: dv,
dataView,
browserFields,
indexes: dv.getIndexPattern().split(','),
indexExists: dv.getIndexPattern().split(',').length > 0,
Expand Down
Expand Up @@ -5,7 +5,7 @@
* 2.0.
*/

import type { MappingRuntimeFields } from '@elastic/elasticsearch/lib/api/typesWithBodyKey';
import type { MappingRuntimeFieldType } from '@elastic/elasticsearch/lib/api/typesWithBodyKey';
import { DEFAULT_INDEX_PATTERN } from '../../../../common/constants';
import type { BrowserFields } from '../../../../common/search_strategy/index_fields';

Expand Down Expand Up @@ -577,11 +577,13 @@ export const mockBrowserFields: BrowserFields = {
},
};

export const mockRuntimeMappings: MappingRuntimeFields = {
const runTimeType: MappingRuntimeFieldType = 'keyword' as const;

export const mockRuntimeMappings = {
'@a.runtime.field': {
script: {
source: 'emit("Radical dude: " + doc[\'host.name\'].value)',
},
type: 'keyword',
type: runTimeType,
},
};

0 comments on commit 4483622

Please sign in to comment.