[ML] Transforms: Improve data view checks. (#181892)

## 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! <img width="1672" alt="image" src="https://github.com/elastic/kibana/assets/230104/ff7a0bbf-21e2-48b8-be7e-d9344d478ae6"> After: We do just 1 `_search` request that gets the data view ids/titles: <img width="610" alt="image" src="https://github.com/elastic/kibana/assets/230104/985c4b7d-94f8-4d65-9c2f-79bacad29cda"> ### 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)
elastic · May 13, 2024 · f2945cf · f2945cf
1 parent 1c3ae4c
commit f2945cf
Show file tree

Hide file tree

Showing 9 changed files with 85 additions and 66 deletions.
diff --git 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
@@ -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
@@ -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<string, string>;
+
+export const useGetDataViewsTitleIdMap = () => {
+  const { data } = useAppDependencies();
+
+  return useQuery<DataViewListTitleIdMap, IHttpFetchError>(
+    [TRANSFORM_REACT_QUERY_KEYS.GET_DATA_VIEW_IDS_WITH_TITLE],
+    async () => {
+      return (await data.dataViews.getIdsWithTitle(true)).reduce<Record<string, string>>(
+        (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
@@ -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<unknown>;
-
-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
@@ -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/...orm/public/app/sections/transform_management/components/action_clone/use_clone_action.tsx b/...orm/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<typeof useCloneAction>;
 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) => <CloneActionName disabled={!canCreateTransform} />,
-      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/...blic/app/sections/transform_management/components/action_discover/use_action_discover.tsx b/...blic/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<typeof useDiscoverAction>;
 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/...sform/public/app/sections/transform_management/components/action_edit/use_edit_action.tsx b/...sform/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<typeof useEditAction>;
 export const useEditAction = (forceDisable: boolean, transformNodes: number) => {
+  const { data: dataViewsTitleIdMap } = useGetDataViewsTitleIdMap();
   const { canCreateTransform } = useTransformCapabilities();
 
   const [config, setConfig] = useState<TransformConfigUnion>();
@@ -27,29 +27,31 @@ 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:
                 'Unable to get the data view for the transform {transformId}. No data view exists for {dataViewTitle}.',
               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/...ublic/app/sections/transform_management/components/transform_list/transform_list.test.tsx b/...ublic/app/sections/transform_management/components/transform_list/transform_list.test.tsx
@@ -46,7 +46,7 @@ describe('Transform: Transform List <TransformList />', () => {
     );
 
     await waitFor(() => {
-      expect(useQueryMock).toHaveBeenCalledTimes(4);
+      expect(useQueryMock).toHaveBeenCalledTimes(5);
       expect(container.textContent).toContain('Create your first transform');
     });
   });