Skip to content

Commit

Permalink
[UnifiedFieldList] Add new fields ingested in background with valid m…
Browse files Browse the repository at this point in the history
…appings (elastic#172329)

Improves UnifiedFieldList by adding a newly ingested field to the list with the right type. On top of
that, it triggers a refresh of the selected DataView fields, so the new field be available by consumers of the DataView.

Co-authored-by: Julia Rechkunova <[email protected]>
  • Loading branch information
2 people authored and nreese committed Jan 10, 2024
1 parent 9a34454 commit 326bd1a
Show file tree
Hide file tree
Showing 16 changed files with 649 additions and 131 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import {
useEuiTheme,
} from '@elastic/eui';
import { ToolbarButton } from '@kbn/shared-ux-button-toolbar';
import { type DataViewField } from '@kbn/data-views-plugin/public';
import { DataViewField, type FieldSpec } from '@kbn/data-views-plugin/common';
import { getDataViewFieldSubtypeMulti } from '@kbn/es-query/src/utils';
import { FIELDS_LIMIT_SETTING, SEARCH_FIELDS_FROM_SOURCE } from '@kbn/discover-utils';
import { FieldList } from '../../components/field_list';
Expand Down Expand Up @@ -191,25 +191,6 @@ export const UnifiedFieldListSidebarComponent: React.FC<UnifiedFieldListSidebarP
onSelectedFieldFilter,
]);

useEffect(() => {
if (
searchMode !== 'documents' ||
!useNewFieldsApi ||
stateService.creationOptions.disableMultiFieldsGroupingByParent
) {
setMultiFieldsMap(undefined); // we don't have to calculate multifields in this case
} else {
setMultiFieldsMap(calculateMultiFields(allFields, selectedFieldsState.selectedFieldsMap));
}
}, [
stateService.creationOptions.disableMultiFieldsGroupingByParent,
selectedFieldsState.selectedFieldsMap,
allFields,
useNewFieldsApi,
setMultiFieldsMap,
searchMode,
]);

const popularFieldsLimit = useMemo(
() => core.uiSettings.get(FIELDS_LIMIT_SETTING),
[core.uiSettings]
Expand All @@ -226,24 +207,47 @@ export const UnifiedFieldListSidebarComponent: React.FC<UnifiedFieldListSidebarP
[searchMode, stateService.creationOptions.disableMultiFieldsGroupingByParent]
);

const { fieldListFiltersProps, fieldListGroupedProps } = useGroupedFields<DataViewField>({
dataViewId: (searchMode === 'documents' && dataView?.id) || null, // passing `null` for text-based queries
allFields,
popularFieldsLimit:
searchMode !== 'documents' || stateService.creationOptions.disablePopularFields
? 0
: popularFieldsLimit,
isAffectedByGlobalFilter,
services: {
dataViews,
core,
},
sortedSelectedFields: onSelectedFieldFilter ? undefined : selectedFieldsState.selectedFields,
onSelectedFieldFilter,
onSupportedFieldFilter:
stateService.creationOptions.onSupportedFieldFilter ?? onSupportedFieldFilter,
onOverrideFieldGroupDetails: stateService.creationOptions.onOverrideFieldGroupDetails,
});
const { fieldListFiltersProps, fieldListGroupedProps, allFieldsModified } =
useGroupedFields<DataViewField>({
dataViewId: (searchMode === 'documents' && dataView?.id) || null, // passing `null` for text-based queries
allFields,
popularFieldsLimit:
searchMode !== 'documents' || stateService.creationOptions.disablePopularFields
? 0
: popularFieldsLimit,
isAffectedByGlobalFilter,
services: {
dataViews,
core,
},
sortedSelectedFields: onSelectedFieldFilter ? undefined : selectedFieldsState.selectedFields,
onSelectedFieldFilter,
onSupportedFieldFilter:
stateService.creationOptions.onSupportedFieldFilter ?? onSupportedFieldFilter,
onOverrideFieldGroupDetails: stateService.creationOptions.onOverrideFieldGroupDetails,
getNewFieldsBySpec,
});

useEffect(() => {
if (
searchMode !== 'documents' ||
!useNewFieldsApi ||
stateService.creationOptions.disableMultiFieldsGroupingByParent
) {
setMultiFieldsMap(undefined); // we don't have to calculate multifields in this case
} else {
setMultiFieldsMap(
calculateMultiFields(allFieldsModified, selectedFieldsState.selectedFieldsMap)
);
}
}, [
stateService.creationOptions.disableMultiFieldsGroupingByParent,
selectedFieldsState.selectedFieldsMap,
allFieldsModified,
useNewFieldsApi,
setMultiFieldsMap,
searchMode,
]);

const renderFieldItem: FieldListGroupedProps<DataViewField>['renderFieldItem'] = useCallback(
({ field, groupName, groupIndex, itemIndex, fieldSearchHighlight }) => (
Expand Down Expand Up @@ -456,3 +460,7 @@ function calculateMultiFields(
});
return map;
}

function getNewFieldsBySpec(fieldSpecArr: FieldSpec[]): DataViewField[] {
return fieldSpecArr.map((fieldSpec) => new DataViewField(fieldSpec));
}
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,7 @@ describe('UnifiedFieldList useExistingFields', () => {
expect(hookReader.result.current.getFieldsExistenceStatus(dataViewId)).toBe(
ExistenceFetchStatus.succeeded
);
expect(hookReader.result.current.getNewFields(dataViewId)).toStrictEqual([]);

// does not have existence info => works less restrictive
const anotherDataViewId = 'test-id';
Expand All @@ -140,6 +141,7 @@ describe('UnifiedFieldList useExistingFields', () => {
expect(hookReader.result.current.getFieldsExistenceStatus(anotherDataViewId)).toBe(
ExistenceFetchStatus.unknown
);
expect(hookReader.result.current.getNewFields(dataViewId)).toStrictEqual([]);
});

it('should work correctly with multiple readers', async () => {
Expand Down Expand Up @@ -217,6 +219,7 @@ describe('UnifiedFieldList useExistingFields', () => {
expect(currentResult.isFieldsExistenceInfoUnavailable(dataViewId)).toBe(true);
expect(currentResult.hasFieldData(dataViewId, dataView.fields[0].name)).toBe(true);
expect(currentResult.getFieldsExistenceStatus(dataViewId)).toBe(ExistenceFetchStatus.failed);
expect(currentResult.getNewFields(dataViewId)).toStrictEqual([]);
});

it('should work correctly for multiple data views', async () => {
Expand Down Expand Up @@ -533,4 +536,49 @@ describe('UnifiedFieldList useExistingFields', () => {

expect(params.onNoData).toHaveBeenCalledTimes(1); // still 1 time
});

it('should include newFields', async () => {
const newFields = [{ name: 'test', type: 'keyword', searchable: true, aggregatable: true }];

(ExistingFieldsServiceApi.loadFieldExisting as jest.Mock).mockImplementation(
async ({ dataView: currentDataView }) => {
return {
existingFieldNames: [currentDataView.fields[0].name],
newFields,
};
}
);

const params: ExistingFieldsFetcherParams = {
dataViews: [dataView],
services: mockedServices,
fromDate: '2019-01-01',
toDate: '2020-01-01',
query: { query: '', language: 'lucene' },
filters: [],
};
const hookFetcher = renderHook(useExistingFieldsFetcher, {
initialProps: params,
});

const hookReader = renderHook(useExistingFieldsReader);
await hookFetcher.waitForNextUpdate();

expect(ExistingFieldsServiceApi.loadFieldExisting).toHaveBeenCalledWith(
expect.objectContaining({
fromDate: '2019-01-01',
toDate: '2020-01-01',
dslQuery,
dataView,
timeFieldName: dataView.timeFieldName,
})
);

expect(hookReader.result.current.getFieldsExistenceStatus(dataView.id!)).toBe(
ExistenceFetchStatus.succeeded
);

expect(hookReader.result.current.getNewFields(dataView.id!)).toBe(newFields);
expect(hookReader.result.current.getNewFields('another-id')).toStrictEqual([]);
});
});
22 changes: 20 additions & 2 deletions packages/kbn-unified-field-list/src/hooks/use_existing_fields.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,18 +12,20 @@ import { BehaviorSubject } from 'rxjs';
import type { CoreStart } from '@kbn/core/public';
import type { AggregateQuery, EsQueryConfig, Filter, Query } from '@kbn/es-query';
import { type DataPublicPluginStart } from '@kbn/data-plugin/public';
import type { DataView, DataViewsContract } from '@kbn/data-views-plugin/common';
import type { DataView, DataViewsContract, FieldSpec } from '@kbn/data-views-plugin/common';
import { getEsQueryConfig } from '@kbn/data-service/src/es_query';
import { reportPerformanceMetricEvent } from '@kbn/ebt-tools';
import { loadFieldExisting } from '../services/field_existing';
import { ExistenceFetchStatus } from '../types';

const getBuildEsQueryAsync = async () => (await import('@kbn/es-query')).buildEsQuery;
const generateId = htmlIdGenerator();
const DEFAULT_EMPTY_NEW_FIELDS: FieldSpec[] = [];

export interface ExistingFieldsInfo {
fetchStatus: ExistenceFetchStatus;
existingFieldsByFieldNameMap: Record<string, boolean>;
newFields?: FieldSpec[];
numberOfFetches: number;
hasDataViewRestrictions?: boolean;
}
Expand Down Expand Up @@ -54,6 +56,7 @@ export interface ExistingFieldsReader {
hasFieldData: (dataViewId: string, fieldName: string) => boolean;
getFieldsExistenceStatus: (dataViewId: string) => ExistenceFetchStatus;
isFieldsExistenceInfoUnavailable: (dataViewId: string) => boolean;
getNewFields: (dataViewId: string) => FieldSpec[];
}

const initialData: ExistingFieldsByDataViewMap = {};
Expand Down Expand Up @@ -157,6 +160,7 @@ export const useExistingFieldsFetcher = (
}

info.existingFieldsByFieldNameMap = booleanMap(existingFieldNames);
info.newFields = result.newFields;
info.fetchStatus = ExistenceFetchStatus.succeeded;
} catch (error) {
info.fetchStatus = ExistenceFetchStatus.failed;
Expand Down Expand Up @@ -286,6 +290,19 @@ export const useExistingFieldsReader: () => ExistingFieldsReader = () => {
[existingFieldsByDataViewMap]
);

const getNewFields = useCallback(
(dataViewId: string) => {
const info = existingFieldsByDataViewMap[dataViewId];

if (info?.fetchStatus === ExistenceFetchStatus.succeeded) {
return info?.newFields ?? DEFAULT_EMPTY_NEW_FIELDS;
}

return DEFAULT_EMPTY_NEW_FIELDS;
},
[existingFieldsByDataViewMap]
);

const getFieldsExistenceInfo = useCallback(
(dataViewId: string) => {
return dataViewId ? existingFieldsByDataViewMap[dataViewId] : unknownInfo;
Expand Down Expand Up @@ -321,8 +338,9 @@ export const useExistingFieldsReader: () => ExistingFieldsReader = () => {
hasFieldData,
getFieldsExistenceStatus,
isFieldsExistenceInfoUnavailable,
getNewFields,
}),
[hasFieldData, getFieldsExistenceStatus, isFieldsExistenceInfoUnavailable]
[hasFieldData, getFieldsExistenceStatus, isFieldsExistenceInfoUnavailable, getNewFields]
);
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ describe('UnifiedFieldList useGroupedFields()', () => {
? ExistenceFetchStatus.succeeded
: ExistenceFetchStatus.unknown,
isFieldsExistenceInfoUnavailable: (dataViewId) => dataViewId !== props.dataViewId,
getNewFields: () => [],
})
);

Expand Down Expand Up @@ -156,6 +157,7 @@ describe('UnifiedFieldList useGroupedFields()', () => {
? ExistenceFetchStatus.succeeded
: ExistenceFetchStatus.unknown,
isFieldsExistenceInfoUnavailable: (dataViewId) => dataViewId !== props.dataViewId,
getNewFields: () => [],
})
);

Expand Down Expand Up @@ -185,6 +187,8 @@ describe('UnifiedFieldList useGroupedFields()', () => {

expect(fieldListGroupedProps.fieldsExistenceStatus).toBe(ExistenceFetchStatus.succeeded);
expect(fieldListGroupedProps.fieldsExistInIndex).toBe(true);
expect(result.current.allFieldsModified).toBe(allFields);
expect(result.current.hasNewFields).toBe(false);

rerender({
...props,
Expand All @@ -198,6 +202,82 @@ describe('UnifiedFieldList useGroupedFields()', () => {
expect(result.current.fieldListGroupedProps.scrollToTopResetCounter).not.toBe(
scrollToTopResetCounter1
);
expect(result.current.allFieldsModified).toBe(allFields);
expect(result.current.hasNewFields).toBe(false);

(ExistenceApi.useExistingFieldsReader as jest.Mock).mockRestore();
});

it('should work correctly with new fields', async () => {
const props: GroupedFieldsParams<DataViewField> = {
dataViewId: dataView.id!,
allFields,
services: mockedServices,
getNewFieldsBySpec: (spec) => spec.map((field) => new DataViewField(field)),
};

const newField = { name: 'test', type: 'keyword', searchable: true, aggregatable: true };

jest.spyOn(ExistenceApi, 'useExistingFieldsReader').mockImplementation(
(): ExistingFieldsReader => ({
hasFieldData: (dataViewId) => {
return dataViewId === props.dataViewId;
},
getFieldsExistenceStatus: (dataViewId) =>
dataViewId === props.dataViewId
? ExistenceFetchStatus.succeeded
: ExistenceFetchStatus.unknown,
isFieldsExistenceInfoUnavailable: (dataViewId) => dataViewId !== props.dataViewId,
getNewFields: () => [newField],
})
);

const { result, waitForNextUpdate, rerender } = renderHook(useGroupedFields, {
initialProps: props,
});

await waitForNextUpdate();

let fieldListGroupedProps = result.current.fieldListGroupedProps;
const fieldGroups = fieldListGroupedProps.fieldGroups;
const scrollToTopResetCounter1 = fieldListGroupedProps.scrollToTopResetCounter;

expect(
Object.keys(fieldGroups!).map(
(key) => `${key}-${fieldGroups![key as FieldsGroupNames]?.fields.length}`
)
).toStrictEqual([
'SpecialFields-0',
'SelectedFields-0',
'PopularFields-0',
'AvailableFields-25',
'UnmappedFields-1',
'EmptyFields-0',
'MetaFields-3',
]);

expect(fieldListGroupedProps.fieldsExistenceStatus).toBe(ExistenceFetchStatus.succeeded);
expect(fieldListGroupedProps.fieldsExistInIndex).toBe(true);
expect(result.current.allFieldsModified).toStrictEqual([
...allFields,
new DataViewField(newField),
]);
expect(result.current.hasNewFields).toBe(true);

rerender({
...props,
dataViewId: null, // for text-based queries
allFields,
});

fieldListGroupedProps = result.current.fieldListGroupedProps;
expect(fieldListGroupedProps.fieldsExistenceStatus).toBe(ExistenceFetchStatus.succeeded);
expect(fieldListGroupedProps.fieldsExistInIndex).toBe(true);
expect(result.current.fieldListGroupedProps.scrollToTopResetCounter).not.toBe(
scrollToTopResetCounter1
);
expect(result.current.allFieldsModified).toBe(allFields);
expect(result.current.hasNewFields).toBe(false);

(ExistenceApi.useExistingFieldsReader as jest.Mock).mockRestore();
});
Expand Down Expand Up @@ -438,6 +518,7 @@ describe('UnifiedFieldList useGroupedFields()', () => {
? ExistenceFetchStatus.succeeded
: ExistenceFetchStatus.unknown,
isFieldsExistenceInfoUnavailable: (dataViewId) => dataViewId !== knownDataViewId,
getNewFields: () => [],
})
);

Expand Down
Loading

0 comments on commit 326bd1a

Please sign in to comment.