[Security Solution][Detection Engine] fixes list index requests behaviour (#177416)

## Summary

- while working on https://github.com/elastic/security-team/issues/8589,
discovered list index requests fired when navigating between pages, even
though state of list index is unlikely to change
- fixes case when admin user opens Security for the first time, goes to
multiple pages and can see 409 error of multiple index create requests
- to avoid multiple index requests when user navigates to different
pages, I introduced react-query hooks with `Infinity` mins stale time,
which means, query result would be cached for entire user's session
before page reload or user logout/login.
Downside of this approach, if user does delete list index through API,
they would need to refresh browser page to update index status. But
delete index API is not event documented and its usage would affect
different aspect of the app.
Alternative, might be, to refresh index status when user opens lists
flyout. But given unlike scenario of user using undocumented delete
index API, I think we can skip this refresh call on every flyout open


### Before
#### Multiple lists/index requests


8c82a270-4578-4a98-9be9-1ca3d7b00d17


#### Multiple create index requests and 409 error


a7a1718c-5bb2-4427-9819-f72e98b55a9c


### After

#### Single lists/index request



d5060a31-469c-4fc3-bc40-b6e4d3e65a48

#### No multiple create index requests



3c5cb04f-8b26-47c0-ab79-2fe1c41671dd

---------

Co-authored-by: Ryland Herrick <ryalnd@gmail.com>

packages/kbn-securitysolution-list-hooks/src/constants.ts

@@ -0,0 +1,9 @@
+/*
+ * 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 and the Server Side Public License, v 1; you may not use this file except
+ * in compliance with, at your election, the Elastic License 2.0 or the Server
+ * Side Public License, v 1.
+ */
+
+export const READ_INDEX_QUERY_KEY = ['detectionEngine', 'listIndex'];

packages/kbn-securitysolution-list-hooks/src/use_create_list_index/index.ts

@@ -6,9 +6,37 @@
  * Side Public License, v 1.
  */
 
-import { createListIndex } from '@kbn/securitysolution-list-api';
-import { useAsync, withOptionalSignal } from '@kbn/securitysolution-hook-utils';
+import { useMutation, useQueryClient } from '@tanstack/react-query';
+
+import { createListIndex, ApiParams } from '@kbn/securitysolution-list-api';
+import { withOptionalSignal } from '@kbn/securitysolution-hook-utils';
+
+import { READ_INDEX_QUERY_KEY } from '../constants';
 
 const createListIndexWithOptionalSignal = withOptionalSignal(createListIndex);
 
-export const useCreateListIndex = () => useAsync(createListIndexWithOptionalSignal);
+export const useCreateListIndex = ({
+  http,
+  onError,
+}: {
+  http: ApiParams['http'];
+  onError?: (err: unknown) => void;
+}) => {
+  const queryClient = useQueryClient();
+
+  const { mutate, isLoading, error } = useMutation(
+    () => createListIndexWithOptionalSignal({ http }),
+    {
+      onSuccess: () => {
+        queryClient.invalidateQueries(READ_INDEX_QUERY_KEY);
+      },
+      onError,
+    }
+  );
+
+  return {
+    start: mutate,
+    loading: isLoading,
+    error,
+  };
+};

packages/kbn-securitysolution-list-hooks/src/use_read_list_index/index.ts

@@ -6,9 +6,45 @@
  * Side Public License, v 1.
  */
 
-import { readListIndex } from '@kbn/securitysolution-list-api';
-import { useAsync, withOptionalSignal } from '@kbn/securitysolution-hook-utils';
+import { useQuery } from '@tanstack/react-query';
+
+import { readListIndex, ApiParams } from '@kbn/securitysolution-list-api';
+import { withOptionalSignal } from '@kbn/securitysolution-hook-utils';
+
+import { READ_INDEX_QUERY_KEY } from '../constants';
 
 const readListIndexWithOptionalSignal = withOptionalSignal(readListIndex);
 
-export const useReadListIndex = () => useAsync(readListIndexWithOptionalSignal);
+export const useReadListIndex = ({
+  http,
+  isEnabled,
+  onError,
+}: {
+  isEnabled: boolean;
+  http: ApiParams['http'];
+  onError?: (err: unknown) => void;
+}) => {
+  const query = useQuery(
+    READ_INDEX_QUERY_KEY,
+    async ({ signal }) => {
+      if (!isEnabled) {
+        return null;
+      }
+
+      return readListIndexWithOptionalSignal({ http, signal });
+    },
+    {
+      onError,
+      retry: false,
+      refetchOnWindowFocus: false,
+      enabled: isEnabled,
+      staleTime: Infinity,
+    }
+  );
+
+  return {
+    result: query.data,
+    loading: query.isFetching,
+    error: query.error,
+  };
+};

x-pack/plugins/lists/public/lists/hooks/use_create_list_index.test.ts

@@ -11,11 +11,14 @@ import * as Api from '@kbn/securitysolution-list-api';
 import { httpServiceMock } from '@kbn/core/public/mocks';
 
 import { getAcknowledgeSchemaResponseMock } from '../../../common/schemas/response/acknowledge_schema.mock';
+import { createQueryWrapperMock } from '../mocks/query_wrapper';
 
 jest.mock('@kbn/securitysolution-list-api');
 
 // TODO: This test should be ported to the package: packages/kbn-securitysolution-list-hooks/src/use_create_list_index/index.test.ts once we have mocks in kbn packages
 
+const { wrapper: queryWrapper, queryClient } = createQueryWrapperMock();
+
 describe('useCreateListIndex', () => {
   let httpMock: ReturnType<typeof httpServiceMock.createStartContract>;
 
@@ -24,13 +27,62 @@ describe('useCreateListIndex', () => {
     (Api.createListIndex as jest.Mock).mockResolvedValue(getAcknowledgeSchemaResponseMock());
   });
 
-  it('invokes Api.createListIndex', async () => {
-    const { result, waitForNextUpdate } = renderHook(() => useCreateListIndex());
+  it('should call Api.createListIndex when start() executes', async () => {
+    const { result, waitForNextUpdate } = renderHook(() => useCreateListIndex({ http: httpMock }), {
+      wrapper: queryWrapper,
+    });
     act(() => {
-      result.current.start({ http: httpMock });
+      result.current.start();
     });
     await waitForNextUpdate();
 
     expect(Api.createListIndex).toHaveBeenCalledWith(expect.objectContaining({ http: httpMock }));
   });
+
+  it('should call onError callback when Api.createListIndex fails', async () => {
+    const onError = jest.fn();
+    jest.spyOn(Api, 'createListIndex').mockRejectedValue(new Error('Mocked error'));
+
+    const { result, waitForNextUpdate } = renderHook(
+      () => useCreateListIndex({ http: httpMock, onError }),
+      { wrapper: queryWrapper }
+    );
+
+    act(() => {
+      result.current.start();
+    });
+    await waitForNextUpdate();
+
+    expect(onError).toHaveBeenCalledWith(new Error('Mocked error'), undefined, undefined);
+  });
+
+  it('should not invalidate read index query on failure', async () => {
+    jest.spyOn(Api, 'createListIndex').mockRejectedValue(new Error('Mocked error'));
+    const invalidateQueriesSpy = jest.spyOn(queryClient, 'invalidateQueries');
+
+    const { result, waitForNextUpdate } = renderHook(() => useCreateListIndex({ http: httpMock }), {
+      wrapper: queryWrapper,
+    });
+
+    act(() => {
+      result.current.start();
+    });
+    await waitForNextUpdate();
+
+    expect(invalidateQueriesSpy).not.toHaveBeenCalled();
+  });
+
+  it('should invalidate read index query on success', async () => {
+    const { result, waitForNextUpdate } = renderHook(() => useCreateListIndex({ http: httpMock }), {
+      wrapper: queryWrapper,
+    });
+    const invalidateQueriesSpy = jest.spyOn(queryClient, 'invalidateQueries');
+
+    act(() => {
+      result.current.start();
+    });
+    await waitForNextUpdate();
+
+    expect(invalidateQueriesSpy).toHaveBeenCalledWith(['detectionEngine', 'listIndex']);
+  });
 });

x-pack/plugins/lists/public/lists/hooks/use_read_list_index.test.ts

@@ -5,15 +5,18 @@
  * 2.0.
  */
 
-import { act, renderHook } from '@testing-library/react-hooks';
+import { renderHook } from '@testing-library/react-hooks';
 import { useReadListIndex } from '@kbn/securitysolution-list-hooks';
 import * as Api from '@kbn/securitysolution-list-api';
 import { httpServiceMock } from '@kbn/core/public/mocks';
 
 import { getAcknowledgeSchemaResponseMock } from '../../../common/schemas/response/acknowledge_schema.mock';
+import { createQueryWrapperMock } from '../mocks/query_wrapper';
 
 jest.mock('@kbn/securitysolution-list-api');
 
+const { wrapper: queryWrapper } = createQueryWrapperMock();
+
 // TODO: Port this code over to the package: packages/kbn-securitysolution-list-hooks/src/use_read_list_index/index.test.ts once kibana has mocks in packages
 
 describe('useReadListIndex', () => {
@@ -22,15 +25,43 @@ describe('useReadListIndex', () => {
   beforeEach(() => {
     httpMock = httpServiceMock.createStartContract();
     (Api.readListIndex as jest.Mock).mockResolvedValue(getAcknowledgeSchemaResponseMock());
+    jest.resetAllMocks();
   });
 
-  it('invokes Api.readListIndex', async () => {
-    const { result, waitForNextUpdate } = renderHook(() => useReadListIndex());
-    act(() => {
-      result.current.start({ http: httpMock });
-    });
+  it('should call Api.readListIndex when is enabled', async () => {
+    const { waitForNextUpdate } = renderHook(
+      () => useReadListIndex({ http: httpMock, isEnabled: true }),
+      {
+        wrapper: queryWrapper,
+      }
+    );
+
     await waitForNextUpdate();
 
-    expect(Api.readListIndex).toHaveBeenCalledWith(expect.objectContaining({ http: httpMock }));
+    expect(Api.readListIndex).toHaveBeenCalled();
+  });
+
+  it('should not call Api.readListIndex when is not enabled', async () => {
+    renderHook(() => useReadListIndex({ http: httpMock, isEnabled: false }), {
+      wrapper: queryWrapper,
+    });
+
+    expect(Api.readListIndex).not.toHaveBeenCalled();
+  });
+
+  it('calls onError callback when apiCall fails', async () => {
+    const onError = jest.fn();
+    jest.spyOn(Api, 'readListIndex').mockRejectedValue(new Error('Mocked error'));
+
+    const { waitForNextUpdate } = renderHook(
+      () => useReadListIndex({ http: httpMock, isEnabled: true, onError }),
+      {
+        wrapper: queryWrapper,
+      }
+    );
+
+    await waitForNextUpdate();
+
+    expect(onError).toHaveBeenCalledWith(new Error('Mocked error'));
   });
 });

x-pack/plugins/lists/public/lists/mocks/query_wrapper.tsx

@@ -0,0 +1,34 @@
+/*
+ * 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 React from 'react';
+import { QueryClient, QueryClientProvider } from '@tanstack/react-query';
+
+export const createQueryWrapperMock = (): {
+  queryClient: QueryClient;
+  wrapper: React.FC;
+} => {
+  const queryClient = new QueryClient({
+    defaultOptions: {
+      queries: {
+        retry: false,
+      },
+    },
+    logger: {
+      error: () => undefined,
+      log: () => undefined,
+      warn: () => undefined,
+    },
+  });
+
+  return {
+    queryClient,
+    wrapper: ({ children }) => (
+      <QueryClientProvider client={queryClient}>{children}</QueryClientProvider>
+    ),
+  };
+};

x-pack/plugins/security_solution/public/detections/containers/detection_engine/lists/use_lists_config.test.tsx

@@ -88,6 +88,17 @@ describe('useListsConfig', () => {
       renderHook(() => useListsConfig());
       expect(listsIndexMock.createIndex).not.toHaveBeenCalled();
     });
+
+    it('does not call create index if the user can manage indexes and have cluster privilege, but index loading', () => {
+      useUserPrivilegesMock.mockReturnValue({
+        detectionEnginePrivileges: { result: { cluster: { manage: true } } },
+      });
+      listsPrivilegesMock.canManageIndex = true;
+      (useListsIndex as jest.Mock).mockReturnValue({ ...listsIndexMock, loading: true });
+
+      renderHook(() => useListsConfig());
+      expect(listsIndexMock.createIndex).not.toHaveBeenCalled();
+    });
   });
 
   describe('when lists are enabled and indexes exist', () => {

x-pack/plugins/security_solution/public/detections/containers/detection_engine/lists/use_lists_config.tsx

@@ -42,10 +42,10 @@ export const useListsConfig = (): UseListsConfigReturn => {
   const canCreateIndex = canManageIndex && canManageCluster;
 
   useEffect(() => {
-    if (needsIndex && canCreateIndex) {
+    if (needsIndex && canCreateIndex && !indexLoading) {
       createIndex();
     }
-  }, [createIndex, needsIndex, canCreateIndex]);
+  }, [createIndex, needsIndex, canCreateIndex, indexLoading]);
 
   return {
     canManageIndex,

x-pack/plugins/security_solution/public/detections/containers/detection_engine/lists/use_lists_index.tsx

@@ -5,7 +5,7 @@
  * 2.0.
  */
 
-import { useEffect, useState, useCallback } from 'react';
+import { useCallback, useMemo } from 'react';
 import { isSecurityAppError } from '@kbn/securitysolution-t-grid';
 import { useReadListIndex, useCreateListIndex } from '@kbn/securitysolution-list-hooks';
 import { useHttp, useKibana } from '../../../../common/lib/kibana';
@@ -13,6 +13,14 @@ import * as i18n from './translations';
 import { useAppToasts } from '../../../../common/hooks/use_app_toasts';
 import { useListsPrivileges } from './use_lists_privileges';
 
+/**
+ * Determines whether an error response from the `readListIndex`
+ * API call indicates that the index is not yet created.
+ */
+const isIndexNotCreatedError = (err: unknown) => {
+  return isSecurityAppError(err) && err.body.status_code === 404;
+};
+
 export interface UseListsIndexReturn {
   createIndex: () => void;
   indexExists: boolean | null;
@@ -21,82 +29,58 @@ export interface UseListsIndexReturn {
 }
 
 export const useListsIndex = (): UseListsIndexReturn => {
-  const [indexExists, setIndexExists] = useState<boolean | null>(null);
-  const [error, setError] = useState<unknown>(null);
   const { lists } = useKibana().services;
   const http = useHttp();
   const { addError } = useAppToasts();
   const { canReadIndex, canManageIndex, canWriteIndex } = useListsPrivileges();
-  const { loading: readLoading, start: readListIndex, ...readListIndexState } = useReadListIndex();
   const {
     loading: createLoading,
     start: createListIndex,
-    ...createListIndexState
-  } = useCreateListIndex();
-  const loading = readLoading || createLoading;
+    error: createListError,
+  } = useCreateListIndex({
+    http,
+    onError: (err) => {
+      if (err != null) {
+        addError(err, { title: i18n.LISTS_INDEX_CREATE_FAILURE });
+      }
+    },
+  });
 
-  // read route utilizes `esClient.indices.getAlias` which requires
-  // management privileges
-  const readIndex = useCallback(() => {
-    if (lists && canReadIndex && canManageIndex) {
-      readListIndex({ http });
-    }
-  }, [http, lists, readListIndex, canReadIndex, canManageIndex]);
+  const {
+    loading: readLoading,
+    result: readResult,
+    error: readError,
+  } = useReadListIndex({
+    http,
+    isEnabled: Boolean(lists && canReadIndex && canManageIndex && !createLoading),
+    onError: (err) => {
+      if (isIndexNotCreatedError(err)) {
+        return;
+      }
+
+      addError(err, { title: i18n.LISTS_INDEX_FETCH_FAILURE });
+    },
+  });
+
+  const loading = readLoading || createLoading;
 
   const createIndex = useCallback(() => {
     if (lists && canManageIndex && canWriteIndex) {
-      createListIndex({ http });
+      createListIndex();
     }
-  }, [createListIndex, http, lists, canManageIndex, canWriteIndex]);
+  }, [createListIndex, lists, canManageIndex, canWriteIndex]);
 
-  // initial read list
-  useEffect(() => {
-    if (!readLoading && !error && indexExists === null) {
-      readIndex();
+  const indexExists = useMemo(() => {
+    if (isIndexNotCreatedError(readError)) {
+      return false;
     }
-  }, [error, indexExists, readIndex, readLoading]);
 
-  // handle read result
-  useEffect(() => {
-    if (readListIndexState.result != null) {
-      setIndexExists(
-        readListIndexState.result.list_index && readListIndexState.result.list_item_index
-      );
-    }
-  }, [readListIndexState.result]);
-
-  // refetch index after creation
-  useEffect(() => {
-    if (createListIndexState.result != null) {
-      readIndex();
-    }
-  }, [createListIndexState.result, readIndex]);
-
-  // handle read error
-  useEffect(() => {
-    const err = readListIndexState.error;
-    if (err != null) {
-      if (isSecurityAppError(err) && err.body.status_code === 404) {
-        setIndexExists(false);
-      } else {
-        setError(err);
-        addError(err, { title: i18n.LISTS_INDEX_FETCH_FAILURE });
-      }
-    }
-  }, [addError, readListIndexState.error]);
-
-  // handle create error
-  useEffect(() => {
-    const err = createListIndexState.error;
-    if (err != null) {
-      setError(err);
-      addError(err, { title: i18n.LISTS_INDEX_CREATE_FAILURE });
-    }
-  }, [addError, createListIndexState.error]);
+    return readResult != null ? readResult.list_index && readResult.list_item_index : null;
+  }, [readError, readResult]);
 
   return {
     createIndex,
-    error,
+    error: createListError || isIndexNotCreatedError(readError) ? undefined : readError,
     indexExists,
     loading,
   };