From f2945cffb5f1f40d14ed447ac79c7d8fe031c99b Mon Sep 17 00:00:00 2001 From: Walter Rafelsberger Date: Mon, 13 May 2024 13:26:29 +0200 Subject: [PATCH] [ML] Transforms: Improve data view checks. (#181892) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary Part of #181603. For some of the actions in the transform list we need to identify if there's a data view for the target index. The way we identified this was quite inefficient. We had poor caching in place and fetched info for all data views including fields — this can be quite expensive queries! This update fixes the approach and switches to using `dataViews.getIdsWithTitle()` in combination with `useQuery()` to bring the caching in line with how we load and refresh the rest of the transform list. Before: Lots of field caps requests! image After: We do just 1 `_search` request that gets the data view ids/titles: image ### Checklist - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios - [x] This was checked for breaking API changes and was [labeled appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process) --- x-pack/plugins/transform/common/constants.ts | 1 + .../transform/public/app/hooks/index.ts | 1 + .../hooks/use_get_data_views_title_id_map.ts | 33 +++++++++++++ .../app/hooks/use_search_items/common.ts | 15 +----- .../use_search_items/use_search_items.ts | 4 +- .../action_clone/use_clone_action.tsx | 26 ++++++----- .../action_discover/use_action_discover.tsx | 46 ++++++++----------- .../action_edit/use_edit_action.tsx | 23 +++++----- .../transform_list/transform_list.test.tsx | 2 +- 9 files changed, 85 insertions(+), 66 deletions(-) create mode 100644 x-pack/plugins/transform/public/app/hooks/use_get_data_views_title_id_map.ts diff --git a/x-pack/plugins/transform/common/constants.ts b/x-pack/plugins/transform/common/constants.ts index 761d9e4c79c72a..8b69c4447431f3 100644 --- a/x-pack/plugins/transform/common/constants.ts +++ b/x-pack/plugins/transform/common/constants.ts @@ -37,6 +37,7 @@ export const addExternalBasePath = (uri: string): string => `${EXTERNAL_API_BASE export const TRANSFORM_REACT_QUERY_KEYS = { DATA_SEARCH: 'transform.data_search', DATA_VIEW_EXISTS: 'transform.data_view_exists', + GET_DATA_VIEW_IDS_WITH_TITLE: 'transform.get_data_view_ids_with_title', GET_DATA_VIEW_TITLES: 'transform.get_data_view_titles', GET_ES_INDICES: 'transform.get_es_indices', GET_ES_INGEST_PIPELINES: 'transform.get_es_ingest_pipelines', diff --git a/x-pack/plugins/transform/public/app/hooks/index.ts b/x-pack/plugins/transform/public/app/hooks/index.ts index 749706f97cd70d..6af5f2201b0278 100644 --- a/x-pack/plugins/transform/public/app/hooks/index.ts +++ b/x-pack/plugins/transform/public/app/hooks/index.ts @@ -7,6 +7,7 @@ export { useCreateTransform } from './use_create_transform'; export { useDocumentationLinks } from './use_documentation_links'; +export { useGetDataViewsTitleIdMap } from './use_get_data_views_title_id_map'; export { useGetDataViewTitles } from './use_get_data_view_titles'; export { useGetEsIndices } from './use_get_es_indices'; export { useGetEsIngestPipelines } from './use_get_es_ingest_pipelines'; diff --git a/x-pack/plugins/transform/public/app/hooks/use_get_data_views_title_id_map.ts b/x-pack/plugins/transform/public/app/hooks/use_get_data_views_title_id_map.ts new file mode 100644 index 00000000000000..d7454ffc63a151 --- /dev/null +++ b/x-pack/plugins/transform/public/app/hooks/use_get_data_views_title_id_map.ts @@ -0,0 +1,33 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import { useQuery } from '@tanstack/react-query'; + +import type { IHttpFetchError } from '@kbn/core-http-browser'; + +import { TRANSFORM_REACT_QUERY_KEYS } from '../../../common/constants'; + +import { useAppDependencies } from '../app_dependencies'; + +type DataViewListTitleIdMap = Record; + +export const useGetDataViewsTitleIdMap = () => { + const { data } = useAppDependencies(); + + return useQuery( + [TRANSFORM_REACT_QUERY_KEYS.GET_DATA_VIEW_IDS_WITH_TITLE], + async () => { + return (await data.dataViews.getIdsWithTitle(true)).reduce>( + (acc, { id, title }) => { + acc[title] = id; + return acc; + }, + {} + ); + } + ); +}; diff --git a/x-pack/plugins/transform/public/app/hooks/use_search_items/common.ts b/x-pack/plugins/transform/public/app/hooks/use_search_items/common.ts index 0432dca913f8f7..2277ca7c48e116 100644 --- a/x-pack/plugins/transform/public/app/hooks/use_search_items/common.ts +++ b/x-pack/plugins/transform/public/app/hooks/use_search_items/common.ts @@ -8,24 +8,11 @@ import { buildEsQuery } from '@kbn/es-query'; import type { IUiSettingsClient } from '@kbn/core/public'; import { getEsQueryConfig } from '@kbn/data-plugin/public'; -import type { DataView, DataViewsContract } from '@kbn/data-views-plugin/public'; +import type { DataView } from '@kbn/data-views-plugin/public'; import { matchAllQuery } from '@kbn/ml-query-utils'; import { isDataView } from '../../../../common/types/data_view'; -let dataViewCache: DataView[] = []; - -export let refreshDataViews: () => Promise; - -export async function loadDataViews(dataViewsContract: DataViewsContract) { - dataViewCache = await dataViewsContract.find('*', 10000); - return dataViewCache; -} - -export function getDataViewIdByTitle(dataViewTitle: string): string | undefined { - return dataViewCache.find(({ title }) => title === dataViewTitle)?.id; -} - type CombinedQuery = Record<'bool', any> | object; export interface SearchItems { diff --git a/x-pack/plugins/transform/public/app/hooks/use_search_items/use_search_items.ts b/x-pack/plugins/transform/public/app/hooks/use_search_items/use_search_items.ts index 502f167582a98a..ed3b291a2d7f89 100644 --- a/x-pack/plugins/transform/public/app/hooks/use_search_items/use_search_items.ts +++ b/x-pack/plugins/transform/public/app/hooks/use_search_items/use_search_items.ts @@ -10,7 +10,7 @@ import { i18n } from '@kbn/i18n'; import { isDataView } from '../../../../common/types/data_view'; import { useAppDependencies } from '../../app_dependencies'; import type { SearchItems } from './common'; -import { createSearchItems, getDataViewIdByTitle, loadDataViews } from './common'; +import { createSearchItems } from './common'; export const useSearchItems = (defaultSavedObjectId: string | undefined) => { const [savedObjectId, setSavedObjectId] = useState(defaultSavedObjectId); @@ -73,8 +73,6 @@ export const useSearchItems = (defaultSavedObjectId: string | undefined) => { return { error, - getDataViewIdByTitle, - loadDataViews, searchItems, setSavedObjectId, }; diff --git a/x-pack/plugins/transform/public/app/sections/transform_management/components/action_clone/use_clone_action.tsx b/x-pack/plugins/transform/public/app/sections/transform_management/components/action_clone/use_clone_action.tsx index a33afe14bf04f0..6c1451da3dec95 100644 --- a/x-pack/plugins/transform/public/app/sections/transform_management/components/action_clone/use_clone_action.tsx +++ b/x-pack/plugins/transform/public/app/sections/transform_management/components/action_clone/use_clone_action.tsx @@ -11,30 +11,30 @@ import { i18n } from '@kbn/i18n'; import type { TransformListAction, TransformListRow } from '../../../../common'; import { SECTION_SLUG } from '../../../../common/constants'; -import { useTransformCapabilities, useSearchItems } from '../../../../hooks'; -import { useAppDependencies, useToastNotifications } from '../../../../app_dependencies'; +import { useGetDataViewsTitleIdMap, useTransformCapabilities } from '../../../../hooks'; +import { useToastNotifications } from '../../../../app_dependencies'; import { cloneActionNameText, CloneActionName } from './clone_action_name'; export type CloneAction = ReturnType; export const useCloneAction = (forceDisable: boolean, transformNodes: number) => { const history = useHistory(); - const appDeps = useAppDependencies(); - const dataViewsContract = appDeps.data.dataViews; const toastNotifications = useToastNotifications(); - const { getDataViewIdByTitle, loadDataViews } = useSearchItems(undefined); - + const { data: dataViewsTitleIdMap } = useGetDataViewsTitleIdMap(); const { canCreateTransform } = useTransformCapabilities(); const clickHandler = useCallback( async (item: TransformListRow) => { try { - await loadDataViews(dataViewsContract); + if (!dataViewsTitleIdMap) { + return; + } + const dataViewTitle = Array.isArray(item.config.source.index) ? item.config.source.index.join(',') : item.config.source.index; - const dataViewId = getDataViewIdByTitle(dataViewTitle); + const dataViewId = dataViewsTitleIdMap[dataViewTitle]; if (dataViewId === undefined) { toastNotifications.addDanger( @@ -55,20 +55,24 @@ export const useCloneAction = (forceDisable: boolean, transformNodes: number) => }); } }, - [history, dataViewsContract, toastNotifications, loadDataViews, getDataViewIdByTitle] + [dataViewsTitleIdMap, history, toastNotifications] ); const action: TransformListAction = useMemo( () => ({ name: (item: TransformListRow) => , - enabled: () => canCreateTransform && !forceDisable && transformNodes > 0, + enabled: () => + dataViewsTitleIdMap !== undefined && + canCreateTransform && + !forceDisable && + transformNodes > 0, description: cloneActionNameText, icon: 'copy', type: 'icon', onClick: clickHandler, 'data-test-subj': 'transformActionClone', }), - [canCreateTransform, forceDisable, clickHandler, transformNodes] + [canCreateTransform, dataViewsTitleIdMap, forceDisable, clickHandler, transformNodes] ); return { action }; diff --git a/x-pack/plugins/transform/public/app/sections/transform_management/components/action_discover/use_action_discover.tsx b/x-pack/plugins/transform/public/app/sections/transform_management/components/action_discover/use_action_discover.tsx index 2d4515b479f178..ca78630a6730ea 100644 --- a/x-pack/plugins/transform/public/app/sections/transform_management/components/action_discover/use_action_discover.tsx +++ b/x-pack/plugins/transform/public/app/sections/transform_management/components/action_discover/use_action_discover.tsx @@ -5,13 +5,13 @@ * 2.0. */ -import React, { useCallback, useEffect, useMemo, useState } from 'react'; +import React, { useCallback, useMemo } from 'react'; import { DISCOVER_APP_LOCATOR } from '@kbn/discover-plugin/common'; import type { TransformListAction, TransformListRow } from '../../../../common'; -import { useSearchItems } from '../../../../hooks/use_search_items'; import { useAppDependencies } from '../../../../app_dependencies'; +import { useGetDataViewsTitleIdMap } from '../../../../hooks'; import { isDiscoverActionDisabled, @@ -23,42 +23,36 @@ export type DiscoverAction = ReturnType; export const useDiscoverAction = (forceDisable: boolean) => { const { share, - data: { dataViews: dataViewsContract }, application: { capabilities }, } = useAppDependencies(); const isDiscoverAvailable = !!capabilities.discover?.show; - const { getDataViewIdByTitle, loadDataViews } = useSearchItems(undefined); - - const [dataViewsLoaded, setDataViewsLoaded] = useState(false); - - useEffect(() => { - async function checkDataViewAvailability() { - await loadDataViews(dataViewsContract); - setDataViewsLoaded(true); - } - - checkDataViewAvailability(); - }, [loadDataViews, dataViewsContract]); + const { data: dataViewsTitleIdMap } = useGetDataViewsTitleIdMap(); const clickHandler = useCallback( (item: TransformListRow) => { const locator = share.url.locators.get(DISCOVER_APP_LOCATOR); - if (!locator) return; - const dataViewId = getDataViewIdByTitle(item.config.dest.index); - locator.navigateSync({ - indexPatternId: dataViewId, - }); + if (!locator || !dataViewsTitleIdMap) return; + const dataViewId = dataViewsTitleIdMap[item.config.dest.index]; + if (dataViewId) { + locator.navigateSync({ + indexPatternId: dataViewId, + }); + } }, - [getDataViewIdByTitle, share] + [dataViewsTitleIdMap, share] ); const dataViewExists = useCallback( - (item: TransformListRow) => { - const dataViewId = getDataViewIdByTitle(item.config.dest.index); + (item: TransformListRow): boolean => { + if (!dataViewsTitleIdMap) { + return false; + } + + const dataViewId = dataViewsTitleIdMap[item.config.dest.index]; return dataViewId !== undefined; }, - [getDataViewIdByTitle] + [dataViewsTitleIdMap] ); const action: TransformListAction = useMemo( @@ -68,14 +62,14 @@ export const useDiscoverAction = (forceDisable: boolean) => { }, available: () => isDiscoverAvailable, enabled: (item: TransformListRow) => - dataViewsLoaded && !isDiscoverActionDisabled([item], forceDisable, dataViewExists(item)), + !isDiscoverActionDisabled([item], forceDisable, dataViewExists(item)), description: discoverActionNameText, icon: 'visTable', type: 'icon', onClick: clickHandler, 'data-test-subj': 'transformActionDiscover', }), - [forceDisable, dataViewExists, dataViewsLoaded, isDiscoverAvailable, clickHandler] + [forceDisable, dataViewExists, isDiscoverAvailable, clickHandler] ); return { action }; diff --git a/x-pack/plugins/transform/public/app/sections/transform_management/components/action_edit/use_edit_action.tsx b/x-pack/plugins/transform/public/app/sections/transform_management/components/action_edit/use_edit_action.tsx index d54abfa498a9a4..2b15d830651973 100644 --- a/x-pack/plugins/transform/public/app/sections/transform_management/components/action_edit/use_edit_action.tsx +++ b/x-pack/plugins/transform/public/app/sections/transform_management/components/action_edit/use_edit_action.tsx @@ -10,15 +10,15 @@ import React, { useCallback, useMemo, useState } from 'react'; import { i18n } from '@kbn/i18n'; import type { TransformListAction, TransformListRow } from '../../../../common'; -import { useTransformCapabilities } from '../../../../hooks'; +import { useGetDataViewsTitleIdMap, useTransformCapabilities } from '../../../../hooks'; import { editActionNameText, EditActionName } from './edit_action_name'; -import { useSearchItems } from '../../../../hooks/use_search_items'; -import { useAppDependencies, useToastNotifications } from '../../../../app_dependencies'; +import { useToastNotifications } from '../../../../app_dependencies'; import type { TransformConfigUnion } from '../../../../../../common/types/transform'; export type EditAction = ReturnType; export const useEditAction = (forceDisable: boolean, transformNodes: number) => { + const { data: dataViewsTitleIdMap } = useGetDataViewsTitleIdMap(); const { canCreateTransform } = useTransformCapabilities(); const [config, setConfig] = useState(); @@ -27,20 +27,21 @@ export const useEditAction = (forceDisable: boolean, transformNodes: number) => const closeFlyout = () => setIsFlyoutVisible(false); - const { getDataViewIdByTitle } = useSearchItems(undefined); const toastNotifications = useToastNotifications(); - const appDeps = useAppDependencies(); - const dataViews = appDeps.data.dataViews; const clickHandler = useCallback( async (item: TransformListRow) => { try { + if (!dataViewsTitleIdMap) { + return; + } + const dataViewTitle = Array.isArray(item.config.source.index) ? item.config.source.index.join(',') : item.config.source.index; - const currentDataViewId = getDataViewIdByTitle(dataViewTitle); + const newDataViewId = dataViewsTitleIdMap[dataViewTitle]; - if (currentDataViewId === undefined) { + if (newDataViewId === undefined) { toastNotifications.addWarning( i18n.translate('xpack.transform.edit.noDataViewErrorPromptText', { defaultMessage: @@ -48,8 +49,9 @@ export const useEditAction = (forceDisable: boolean, transformNodes: number) => values: { dataViewTitle, transformId: item.id }, }) ); + } else { + setDataViewId(newDataViewId); } - setDataViewId(currentDataViewId); setConfig(item.config); setIsFlyoutVisible(true); } catch (e) { @@ -60,8 +62,7 @@ export const useEditAction = (forceDisable: boolean, transformNodes: number) => }); } }, - // eslint-disable-next-line react-hooks/exhaustive-deps - [dataViews, toastNotifications, getDataViewIdByTitle] + [dataViewsTitleIdMap, toastNotifications] ); const action: TransformListAction = useMemo( diff --git a/x-pack/plugins/transform/public/app/sections/transform_management/components/transform_list/transform_list.test.tsx b/x-pack/plugins/transform/public/app/sections/transform_management/components/transform_list/transform_list.test.tsx index e3f98beea9e7e3..c47c0f0add99f7 100644 --- a/x-pack/plugins/transform/public/app/sections/transform_management/components/transform_list/transform_list.test.tsx +++ b/x-pack/plugins/transform/public/app/sections/transform_management/components/transform_list/transform_list.test.tsx @@ -46,7 +46,7 @@ describe('Transform: Transform List ', () => { ); await waitFor(() => { - expect(useQueryMock).toHaveBeenCalledTimes(4); + expect(useQueryMock).toHaveBeenCalledTimes(5); expect(container.textContent).toContain('Create your first transform'); }); });