[Security Solution] getDataViewStateFromIndexFields was using wrong t…

…ype as part of a cast (#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 <[email protected]>
elastic · May 31, 2023 · 1c75903 · 1c75903
1 parent a21ddd0
commit 1c75903
Show file tree

Hide file tree

Showing 27 changed files with 131 additions and 239 deletions.
diff --git a/packages/kbn-securitysolution-grouping/src/containers/query/types.ts b/packages/kbn-securitysolution-grouping/src/containers/query/types.ts
@@ -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;
 }
@@ -29,7 +34,7 @@ export interface GroupingQueryArgs {
   from: string;
   groupByField: string;
   rootAggregations?: NamedAggregation[];
-  runtimeMappings?: MappingRuntimeFields;
+  runtimeMappings?: RunTimeMappings;
   additionalAggregationsRoot?: NamedAggregation[];
   pageNumber?: number;
   uniqueValue: string;

diff --git a/...plugins/security_solution/public/common/components/events_viewer/use_timelines_events.tsx b/...plugins/security_solution/public/common/components/events_viewer/use_timelines_events.tsx
@@ -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';
@@ -77,7 +77,7 @@ export interface UseTimelineEventsProps {
   indexNames: string[];
   language?: KueryFilterQueryKind;
   limit: number;
-  runtimeMappings: MappingRuntimeFields;
+  runtimeMappings: RunTimeMappings;
   skip?: boolean;
   sort?: TimelineRequestSortField[];
   startDate: string;
@@ -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,
       };
 

diff --git a/...gins/security_solution/public/common/components/markdown_editor/plugins/insight/index.tsx b/...gins/security_solution/public/common/components/markdown_editor/plugins/insight/index.tsx
@@ -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';
@@ -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(
@@ -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}
                 />
               ) : (

diff --git a/...ugins/security_solution/public/common/containers/source/__snapshots__/index.test.tsx.snap b/...ugins/security_solution/public/common/containers/source/__snapshots__/index.test.tsx.snap
diff --git a/x-pack/plugins/security_solution/public/common/containers/source/index.test.tsx b/x-pack/plugins/security_solution/public/common/containers/source/index.test.tsx
@@ -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';
@@ -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,
@@ -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']
@@ -88,7 +60,12 @@ describe('source/index.tsx', () => {
                       type: 'keyword',
                     },
                   }),
-                }),
+                };
+                return Promise.resolve({
+                  toSpec: () => dataViewMock,
+                  ...dataViewMock,
+                });
+              },
               getFieldsForWildcard: async () => Promise.resolve(),
             },
             search: {
@@ -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*');

diff --git a/x-pack/plugins/security_solution/public/common/containers/source/index.tsx b/x-pack/plugins/security_solution/public/common/containers/source/index.tsx
@@ -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';
@@ -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 {
@@ -115,7 +86,7 @@ interface FetchIndexReturn {
   indexes: string[];
   indexExists: boolean;
   indexPatterns: DataViewBase;
-  dataView: DataView | undefined;
+  dataView: DataViewSpec | undefined;
 }
 
 /**
@@ -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,

diff --git a/x-pack/plugins/security_solution/public/common/containers/source/mock.ts b/x-pack/plugins/security_solution/public/common/containers/source/mock.ts
@@ -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';
 
@@ -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,
   },
 };