Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ML] Transforms: Improve data grid memoization. #195394

Merged
merged 5 commits into from
Oct 11, 2024
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
4 changes: 3 additions & 1 deletion x-pack/packages/ml/data_grid/components/data_grid.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,9 @@ export const DataGrid: FC<Props> = memo(

const wrapperEl = useRef<HTMLDivElement>(null);

if (status === INDEX_STATUS.LOADED && data.length === 0) {
// `UNUSED` occurs if we were not able to identify populated fields, because
// then the query to fetch actual docs would not have triggered yet.
if ((status === INDEX_STATUS.UNUSED || status === INDEX_STATUS.LOADED) && data.length === 0) {
return (
<div data-test-subj={`${dataTestSubj} empty`}>
{isWithHeader(props) && <DataGridTitle title={props.title} />}
Expand Down
81 changes: 50 additions & 31 deletions x-pack/packages/ml/data_grid/hooks/use_data_grid.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -168,35 +168,54 @@ export const useDataGrid = (
}
}, [chartsVisible, rowCount, rowCountRelation]);

return {
ccsWarning,
chartsVisible,
chartsButtonVisible: true,
columnsWithCharts,
errorMessage,
invalidSortingColumnns,
noDataMessage,
onChangeItemsPerPage,
onChangePage,
onSort,
pagination,
resetPagination,
rowCount,
rowCountRelation,
setColumnCharts,
setCcsWarning,
setErrorMessage,
setNoDataMessage,
setPagination,
setRowCountInfo,
setSortingColumns,
setStatus,
setTableItems,
setVisibleColumns,
sortingColumns,
status,
tableItems,
toggleChartVisibility,
visibleColumns,
};
return useMemo(
() => ({
ccsWarning,
chartsVisible,
chartsButtonVisible: true,
columnsWithCharts,
errorMessage,
invalidSortingColumnns,
noDataMessage,
onChangeItemsPerPage,
onChangePage,
onSort,
pagination,
resetPagination,
rowCount,
rowCountRelation,
setColumnCharts,
setCcsWarning,
setErrorMessage,
setNoDataMessage,
setPagination,
setRowCountInfo,
setSortingColumns,
setStatus,
setTableItems,
setVisibleColumns,
sortingColumns,
status,
tableItems,
toggleChartVisibility,
visibleColumns,
}),
// custom comparison
// eslint-disable-next-line react-hooks/exhaustive-deps
[
ccsWarning,
chartsVisible,
columnsWithCharts,
errorMessage,
invalidSortingColumnns,
noDataMessage,
pagination,
rowCount,
rowCountRelation,
sortingColumns,
status,
tableItems,
visibleColumns,
]
);
};
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,9 @@ dataStart.search.search = jest.fn(({ params }: IKibanaSearchRequest) => {
});
}) as ISearchGeneric;

// Replace mock to support tests for `use_index_data`.
coreSetup.http.post = jest.fn().mockResolvedValue([]);

const appDependencies: AppDependencies = {
analytics: coreStart.analytics,
application: coreStart.application,
Expand Down
102 changes: 57 additions & 45 deletions x-pack/plugins/transform/public/app/hooks/use_index_data.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import { renderHook } from '@testing-library/react-hooks';

import { __IntlProvider as IntlProvider } from '@kbn/i18n-react';
import type { CoreSetup } from '@kbn/core/public';
import { DataGrid, type UseIndexDataReturnType } from '@kbn/ml-data-grid';
import { DataGrid, type UseIndexDataReturnType, INDEX_STATUS } from '@kbn/ml-data-grid';
import type { RuntimeMappings } from '@kbn/ml-runtime-field-utils';
import type { SimpleQuery } from '@kbn/ml-query-utils';

Expand Down Expand Up @@ -40,7 +40,7 @@ const runtimeMappings: RuntimeMappings = {
const queryClient = new QueryClient();

describe('Transform: useIndexData()', () => {
test('dataView set triggers loading', async () => {
test('empty populatedFields does not trigger loading', async () => {
const wrapper: FC<PropsWithChildren<unknown>> = ({ children }) => (
<QueryClientProvider client={queryClient}>
<IntlProvider locale="en">{children}</IntlProvider>
Expand All @@ -49,15 +49,16 @@ describe('Transform: useIndexData()', () => {

const { result, waitForNextUpdate } = renderHook(
() =>
useIndexData(
{
useIndexData({
dataView: {
id: 'the-id',
getIndexPattern: () => 'the-index-pattern',
fields: [],
} as unknown as SearchItems['dataView'],
query,
runtimeMappings
),
combinedRuntimeMappings: runtimeMappings,
populatedFields: [],
}),
{ wrapper }
);

Expand All @@ -66,60 +67,71 @@ describe('Transform: useIndexData()', () => {
await waitForNextUpdate();

expect(IndexObj.errorMessage).toBe('');
expect(IndexObj.status).toBe(1);
expect(IndexObj.status).toBe(INDEX_STATUS.UNUSED);
expect(IndexObj.tableItems).toEqual([]);
});
});

describe('Transform: <DataGrid /> with useIndexData()', () => {
test('Minimal initialization, no cross cluster search warning.', async () => {
// Arrange
const dataView = {
getIndexPattern: () => 'the-data-view-index-pattern',
fields: [] as any[],
} as SearchItems['dataView'];

const Wrapper = () => {
const props = {
...useIndexData(dataView, { match_all: {} }, runtimeMappings),
copyToClipboard: 'the-copy-to-clipboard-code',
copyToClipboardDescription: 'the-copy-to-clipboard-description',
dataTestSubj: 'the-data-test-subj',
title: 'the-index-preview-title',
toastNotifications: {} as CoreSetup['notifications']['toasts'],
};

return <DataGrid {...props} />;
};

render(
test('dataView set triggers loading', async () => {
const wrapper: FC<PropsWithChildren<unknown>> = ({ children }) => (
<QueryClientProvider client={queryClient}>
<IntlProvider locale="en">
<Wrapper />
</IntlProvider>
<IntlProvider locale="en">{children}</IntlProvider>
</QueryClientProvider>
);

// Act
// Assert
await waitFor(() => {
expect(screen.queryByText('the-index-preview-title')).toBeInTheDocument();
expect(
screen.queryByText('Cross-cluster search returned no fields data.')
).not.toBeInTheDocument();
});
const { result, waitForNextUpdate } = renderHook(
() =>
useIndexData({
dataView: {
id: 'the-id',
getIndexPattern: () => 'the-index-pattern',
metaFields: [],
// minimal mock of DataView fields (array with getByName method)
fields: new (class DataViewFields extends Array<{ name: string }> {
getByName(id: string) {
return this.find((d) => d.name === id);
}
})(
{
name: 'the-populated-field',
},
{
name: 'the-unpopulated-field',
}
),
} as unknown as SearchItems['dataView'],
query,
combinedRuntimeMappings: runtimeMappings,
populatedFields: ['the-populated-field'],
}),
{ wrapper }
);

const IndexObj: UseIndexDataReturnType = result.current;

await waitForNextUpdate();

expect(IndexObj.errorMessage).toBe('');
expect(IndexObj.status).toBe(INDEX_STATUS.LOADING);
expect(IndexObj.tableItems).toEqual([]);
});
});

test('Cross-cluster search warning', async () => {
describe('Transform: <DataGrid /> with useIndexData()', () => {
test('Minimal initialization, no cross cluster search warning.', async () => {
// Arrange
const dataView = {
getIndexPattern: () => 'remote:the-index-pattern-title',
getIndexPattern: () => 'the-data-view-index-pattern',
fields: [] as any[],
} as SearchItems['dataView'];

const Wrapper = () => {
const props = {
...useIndexData(dataView, { match_all: {} }, runtimeMappings),
...useIndexData({
dataView,
query: { match_all: {} },
combinedRuntimeMappings: runtimeMappings,
populatedFields: ['the-populated-field'],
}),
copyToClipboard: 'the-copy-to-clipboard-code',
copyToClipboardDescription: 'the-copy-to-clipboard-description',
dataTestSubj: 'the-data-test-subj',
Expand All @@ -144,7 +156,7 @@ describe('Transform: <DataGrid /> with useIndexData()', () => {
expect(screen.queryByText('the-index-preview-title')).toBeInTheDocument();
expect(
screen.queryByText('Cross-cluster search returned no fields data.')
).toBeInTheDocument();
).not.toBeInTheDocument();
});
});
});
Loading