Refactor search session status on-the-fly calculation and other fixes (…

…#136296)

src/plugins/data/common/search/session/types.ts

@@ -6,6 +6,8 @@
  * Side Public License, v 1.
  */
 
+// eslint-disable-next-line @kbn/eslint/no-restricted-paths
+import type { SavedObjectsFindResponse } from '@kbn/core/server';
 import { SerializableRecord } from '@kbn/utility-types';
 import { SearchSessionStatus } from './status';
 
@@ -87,10 +89,20 @@ export interface SearchSessionRequestInfo {
   error?: string;
 }
 
-export interface SearchSessionFindOptions {
-  page?: number;
-  perPage?: number;
-  sortField?: string;
-  sortOrder?: string;
-  filter?: string;
+/**
+ * On-the-fly calculated search session status
+ */
+export interface SearchSessionStatusResponse {
+  status: SearchSessionStatus;
+}
+
+/**
+ * List of search session objects with on-the-fly calculated search session statuses
+ */
+export interface SearchSessionsFindResponse
+  extends SavedObjectsFindResponse<SearchSessionSavedObjectAttributes> {
+  /**
+   * Map containing calculated statuses of search sessions from the find response
+   */
+  statuses: Record<string, SearchSessionStatusResponse>;
 }

src/plugins/data/config.ts

@@ -60,15 +60,15 @@ export const searchSessionsConfigSchema = schema.object({
   defaultExpiration: schema.duration({ defaultValue: '7d' }),
   management: schema.object({
     /**
-     * maxSessions controls how many saved search sessions we display per page on the management screen.
+     * maxSessions controls how many saved search sessions we load on the management screen.
      */
-    maxSessions: schema.number({ defaultValue: 10000 }),
+    maxSessions: schema.number({ defaultValue: 100 }),
     /**
-     * refreshInterval controls how often we refresh the management screen.
+     * refreshInterval controls how often we refresh the management screen. 0s as duration means that auto-refresh is turned off.
      */
-    refreshInterval: schema.duration({ defaultValue: '10s' }),
+    refreshInterval: schema.duration({ defaultValue: '0s' }),
     /**
-     * refreshTimeout controls how often we refresh the management screen.
+     * refreshTimeout controls the timeout for loading search sessions on mgmt screen
      */
     refreshTimeout: schema.duration({ defaultValue: '1m' }),
     expiresSoonWarning: schema.duration({ defaultValue: '1d' }),

src/plugins/data/public/search/search_interceptor/search_interceptor.test.ts

@@ -291,7 +291,7 @@ describe('SearchInterceptor', () => {
       expect(error.mock.calls[0][0]).toBeInstanceOf(AbortError);
     });
 
-    test.skip('should DELETE a running async search on abort', async () => {
+    test('should DELETE a running async search on abort', async () => {
       const responses = [
         {
           time: 10,
@@ -362,7 +362,7 @@ describe('SearchInterceptor', () => {
       expect(mockCoreSetup.http.delete).not.toHaveBeenCalled();
     });
 
-    test.skip('should DELETE a running async search on async timeout after first response', async () => {
+    test('should DELETE a running async search on async timeout after first response', async () => {
       const responses = [
         {
           time: 10,
@@ -404,7 +404,7 @@ describe('SearchInterceptor', () => {
       expect(mockCoreSetup.http.delete).toHaveBeenCalledTimes(1);
     });
 
-    test.skip('should DELETE a running async search on async timeout on error from fetch', async () => {
+    test('should DELETE a running async search on async timeout on error from fetch', async () => {
       const responses = [
         {
           time: 10,
@@ -447,7 +447,7 @@ describe('SearchInterceptor', () => {
       expect(mockCoreSetup.http.delete).toHaveBeenCalledTimes(1);
     });
 
-    test.skip('should NOT DELETE a running SAVED async search on abort', async () => {
+    test('should NOT DELETE a running SAVED async search on abort', async () => {
       const sessionId = 'sessionId';
       sessionService.isCurrentSession.mockImplementation((_sessionId) => _sessionId === sessionId);
       const responses = [
@@ -907,16 +907,22 @@ describe('SearchInterceptor', () => {
         expect(fetchMock).toBeCalledTimes(2);
       });
 
-      // TODO: changed, not cached searches are not tracked, double check edge cases.
-      test.skip('should track searches that come from cache', async () => {
+      test('should not track searches that come from cache', async () => {
         mockFetchImplementation(partialCompleteResponse);
         sessionService.isCurrentSession.mockImplementation(
           (_sessionId) => _sessionId === sessionId
         );
         sessionService.getSessionId.mockImplementation(() => sessionId);
 
-        // const untrack = jest.fn();
-        // sessionService.trackSearch.mockImplementation(() => untrack);
+        const completeSearch = jest.fn();
+
+        sessionService.trackSearch.mockImplementation((params) => ({
+          complete: completeSearch,
+          error: jest.fn(),
+          beforePoll: jest.fn(() => {
+            return [{ isSearchStored: false }, () => {}];
+          }),
+        }));
 
         const req = {
           params: {
@@ -929,14 +935,15 @@ describe('SearchInterceptor', () => {
         response.subscribe({ next, error, complete });
         response2.subscribe({ next, error, complete });
         await timeTravel(10);
+
         expect(fetchMock).toBeCalledTimes(1);
-        expect(sessionService.trackSearch).toBeCalledTimes(2);
-        // expect(untrack).not.toBeCalled();
+        expect(sessionService.trackSearch).toBeCalledTimes(1);
+        expect(completeSearch).not.toBeCalled();
         await timeTravel(300);
         // Should be called only 2 times (once per partial response)
         expect(fetchMock).toBeCalledTimes(2);
-        expect(sessionService.trackSearch).toBeCalledTimes(2);
-        // expect(untrack).toBeCalledTimes(2);
+        expect(sessionService.trackSearch).toBeCalledTimes(1);
+        expect(completeSearch).toBeCalledTimes(1);
 
         expect(next).toBeCalledTimes(4);
         expect(error).toBeCalledTimes(0);
@@ -1115,15 +1122,22 @@ describe('SearchInterceptor', () => {
         expect(complete2).toBeCalledTimes(1);
       });
 
-      test.skip('aborting a running first search shouldnt clear cache', async () => {
+      test('aborting a running first search shouldnt clear cache', async () => {
         mockFetchImplementation(partialCompleteResponse);
         sessionService.isCurrentSession.mockImplementation(
           (_sessionId) => _sessionId === sessionId
         );
         sessionService.getSessionId.mockImplementation(() => sessionId);
 
-        // const untrack = jest.fn();
-        // sessionService.trackSearch.mockImplementation(() => untrack);
+        const completeSearch = jest.fn();
+
+        sessionService.trackSearch.mockImplementation((params) => ({
+          complete: completeSearch,
+          error: jest.fn(),
+          beforePoll: jest.fn(() => {
+            return [{ isSearchStored: false }, () => {}];
+          }),
+        }));
 
         const req = {
           params: {
@@ -1157,9 +1171,9 @@ describe('SearchInterceptor', () => {
         abortController.abort();
 
         await timeTravel(300);
-        // Both searches should be tracked and untracked
-        expect(sessionService.trackSearch).toBeCalledTimes(2);
-        // expect(untrack).toBeCalledTimes(2);
+        // Only first searches should be tracked and untracked
+        expect(sessionService.trackSearch).toBeCalledTimes(1);
+        expect(completeSearch).toBeCalledTimes(1);
 
         // First search should error
         expect(next).toBeCalledTimes(1);
@@ -1175,15 +1189,22 @@ describe('SearchInterceptor', () => {
         expect(fetchMock).toBeCalledTimes(2);
       });
 
-      test.skip('aborting a running second search shouldnt clear cache', async () => {
+      test('aborting a running second search shouldnt clear cache', async () => {
         mockFetchImplementation(partialCompleteResponse);
         sessionService.isCurrentSession.mockImplementation(
           (_sessionId) => _sessionId === sessionId
         );
         sessionService.getSessionId.mockImplementation(() => sessionId);
 
-        // const untrack = jest.fn();
-        // sessionService.trackSearch.mockImplementation(() => untrack);
+        const completeSearch = jest.fn();
+
+        sessionService.trackSearch.mockImplementation((params) => ({
+          complete: completeSearch,
+          error: jest.fn(),
+          beforePoll: jest.fn(() => {
+            return [{ isSearchStored: false }, () => {}];
+          }),
+        }));
 
         const req = {
           params: {
@@ -1202,7 +1223,7 @@ describe('SearchInterceptor', () => {
         expect(error).toBeCalledTimes(0);
         expect(complete).toBeCalledTimes(0);
         expect(sessionService.trackSearch).toBeCalledTimes(1);
-        // expect(untrack).not.toBeCalled();
+        expect(completeSearch).not.toBeCalled();
 
         const next2 = jest.fn();
         const error2 = jest.fn();
@@ -1218,8 +1239,8 @@ describe('SearchInterceptor', () => {
         abortController.abort();
 
         await timeTravel(300);
-        expect(sessionService.trackSearch).toBeCalledTimes(2);
-        // expect(untrack).toBeCalledTimes(2);
+        expect(sessionService.trackSearch).toBeCalledTimes(1);
+        expect(completeSearch).toBeCalledTimes(1);
 
         expect(next).toBeCalledTimes(2);
         expect(error).toBeCalledTimes(0);
@@ -1233,7 +1254,7 @@ describe('SearchInterceptor', () => {
         expect(fetchMock).toBeCalledTimes(2);
       });
 
-      test.skip('aborting both requests should cancel underlaying search only once', async () => {
+      test('aborting both requests should cancel underlaying search only once', async () => {
         mockFetchImplementation(partialCompleteResponse);
         sessionService.isCurrentSession.mockImplementation(
           (_sessionId) => _sessionId === sessionId
@@ -1270,15 +1291,22 @@ describe('SearchInterceptor', () => {
         expect(mockCoreSetup.http.delete).toHaveBeenCalledTimes(1);
       });
 
-      test.skip('aborting both searches should stop searching and clear cache', async () => {
+      test('aborting both searches should stop searching and clear cache', async () => {
         mockFetchImplementation(partialCompleteResponse);
         sessionService.isCurrentSession.mockImplementation(
           (_sessionId) => _sessionId === sessionId
         );
         sessionService.getSessionId.mockImplementation(() => sessionId);
 
-        // const untrack = jest.fn();
-        // sessionService.trackSearch.mockImplementation(() => untrack);
+        const completeSearch = jest.fn();
+
+        sessionService.trackSearch.mockImplementation((params) => ({
+          complete: completeSearch,
+          error: jest.fn(),
+          beforePoll: jest.fn(() => {
+            return [{ isSearchStored: false }, () => {}];
+          }),
+        }));
 
         const req = {
           params: {

src/plugins/data/public/search/search_interceptor/search_interceptor.ts

@@ -17,7 +17,18 @@ import {
   Subscription,
   throwError,
 } from 'rxjs';
-import { catchError, finalize, map, shareReplay, switchMap, takeUntil, tap } from 'rxjs/operators';
+import {
+  catchError,
+  filter,
+  finalize,
+  map,
+  shareReplay,
+  skip,
+  switchMap,
+  take,
+  takeUntil,
+  tap,
+} from 'rxjs/operators';
 import { PublicMethodsOf } from '@kbn/utility-types';
 import type { HttpSetup, IHttpFetchError } from '@kbn/core-http-browser';
 
@@ -57,7 +68,7 @@ import {
   TimeoutErrorMode,
   SearchSessionIncompleteWarning,
 } from '../errors';
-import { ISessionService } from '../session';
+import { ISessionService, SearchSessionState } from '../session';
 import { SearchResponseCache } from './search_response_cache';
 import { createRequestHash } from './utils';
 import { SearchAbortController } from './search_abort_controller';
@@ -266,10 +277,27 @@ export class SearchInterceptor {
         })
       : undefined;
 
+    // track if this search's session will be send to background
+    // if yes, then we don't need to cancel this search when it is aborted
+    let isSavedToBackground = false;
+    const savedToBackgroundSub =
+      this.deps.session.isCurrentSession(sessionId) &&
+      this.deps.session.state$
+        .pipe(
+          skip(1), // ignore any state, we are only interested in transition x -> BackgroundLoading
+          filter(
+            (state) =>
+              this.deps.session.isCurrentSession(sessionId) &&
+              state === SearchSessionState.BackgroundLoading
+          ),
+          take(1)
+        )
+        .subscribe(() => {
+          isSavedToBackground = true;
+        });
+
     const cancel = once(() => {
-      if (this.deps.session.isCurrentSession(sessionId) && !this.deps.session.isStored()) {
-        this.deps.http.delete(`/internal/search/${strategy}/${id}`);
-      }
+      if (id && !isSavedToBackground) this.deps.http.delete(`/internal/search/${strategy}/${id}`);
     });
 
     return pollSearch(search, cancel, {
@@ -290,6 +318,9 @@ export class SearchInterceptor {
       }),
       finalize(() => {
         searchAbortController.cleanup();
+        if (savedToBackgroundSub) {
+          savedToBackgroundSub.unsubscribe();
+        }
       }),
       // This observable is cached in the responseCache.
       // Using shareReplay makes sure that future subscribers will get the final response

src/plugins/data/public/search/session/mocks.ts

@@ -34,8 +34,9 @@ export function getSessionServiceMock(): jest.Mocked<ISessionService> {
     state$: new BehaviorSubject<SearchSessionState>(SearchSessionState.None).asObservable(),
     sessionMeta$: new BehaviorSubject<SessionMeta>({
       state: SearchSessionState.None,
+      isContinued: false,
     }).asObservable(),
-    disableSaveAfterSessionCompleteTimedOut$: of(false),
+    disableSaveAfterSearchesExpire$: of(false),
     renameCurrentSession: jest.fn(),
     trackSearch: jest.fn((searchDescriptor) => ({
       complete: jest.fn(),

src/plugins/data/public/search/session/search_session_state.ts

@@ -121,6 +121,17 @@ export interface SessionStateInternal<SearchDescriptor = unknown, SearchMeta ext
    */
   isCanceled: boolean;
 
+  /**
+   * If session was continued from a different app,
+   * If session continued from a different app, then it is very likely that `trackedSearches`
+   * doesn't have all the search that were included into the session.
+   * Session that was continued can't be saved because we can't guarantee all the searches saved.
+   * This limitation should be fixed in https://github.com/elastic/kibana/issues/121543
+   *
+   * @deprecated - https://github.com/elastic/kibana/issues/121543
+   */
+  isContinued: boolean;
+
   /**
    * Start time of the current session (from browser perspective)
    */
@@ -146,6 +157,7 @@ const createSessionDefaultState: <
   isStored: false,
   isRestore: false,
   isCanceled: false,
+  isContinued: false,
   isStarted: false,
   trackedSearches: [],
 });
@@ -309,6 +321,11 @@ export interface SessionMeta {
   startTime?: Date;
   canceledTime?: Date;
   completedTime?: Date;
+
+  /**
+   * @deprecated - see remarks in {@link SessionStateInternal}
+   */
+  isContinued: boolean;
 }
 
 export interface SessionPureSelectors<
@@ -360,6 +377,7 @@ export const sessionPureSelectors: SessionPureSelectors = {
         : state.startTime,
       completedTime: state.completedTime,
       canceledTime: state.canceledTime,
+      isContinued: state.isContinued,
     });
   },
   getSearch(state) {