[Search service] Move loadingCount to sync search strategy (#56335)

* Move loadingCount to search strategy * Only register observable once Co-authored-by: Elastic Machine <[email protected]>
elastic · Feb 4, 2020 · e510484 · e510484
1 parent 22e66a1
commit e510484
Show file tree

Hide file tree

Showing 5 changed files with 42 additions and 60 deletions.
diff --git a/src/plugins/data/public/search/create_app_mount_context_search.test.ts b/src/plugins/data/public/search/create_app_mount_context_search.test.ts
@@ -18,35 +18,32 @@
  */
 
 import { createAppMountSearchContext } from './create_app_mount_context_search';
-import { from, BehaviorSubject } from 'rxjs';
+import { from } from 'rxjs';
 
 describe('Create app mount search context', () => {
   it('Returns search fn when there are no strategies', () => {
-    const context = createAppMountSearchContext({}, new BehaviorSubject(0));
+    const context = createAppMountSearchContext({});
     expect(context.search).toBeDefined();
   });
 
   it(`Search throws an error when the strategy doesn't exist`, () => {
-    const context = createAppMountSearchContext({}, new BehaviorSubject(0));
+    const context = createAppMountSearchContext({});
     expect(() => context.search({}, {}, 'noexist').toPromise()).toThrowErrorMatchingInlineSnapshot(
       `"Strategy with name noexist does not exist"`
     );
   });
 
   it(`Search fn is called on appropriate strategy name`, done => {
-    const context = createAppMountSearchContext(
-      {
-        mysearch: search =>
-          Promise.resolve({
-            search: () => from(Promise.resolve({ percentComplete: 98 })),
-          }),
-        anothersearch: search =>
-          Promise.resolve({
-            search: () => from(Promise.resolve({ percentComplete: 0 })),
-          }),
-      },
-      new BehaviorSubject(0)
-    );
+    const context = createAppMountSearchContext({
+      mysearch: search =>
+        Promise.resolve({
+          search: () => from(Promise.resolve({ percentComplete: 98 })),
+        }),
+      anothersearch: search =>
+        Promise.resolve({
+          search: () => from(Promise.resolve({ percentComplete: 0 })),
+        }),
+    });
 
     context.search({}, {}, 'mysearch').subscribe(response => {
       expect(response).toEqual({ percentComplete: 98 });
@@ -55,19 +52,16 @@ describe('Create app mount search context', () => {
   });
 
   it(`Search fn is called with the passed in request object`, done => {
-    const context = createAppMountSearchContext(
-      {
-        mysearch: search => {
-          return Promise.resolve({
-            search: request => {
-              expect(request).toEqual({ greeting: 'hi' });
-              return from(Promise.resolve({}));
-            },
-          });
-        },
+    const context = createAppMountSearchContext({
+      mysearch: search => {
+        return Promise.resolve({
+          search: request => {
+            expect(request).toEqual({ greeting: 'hi' });
+            return from(Promise.resolve({}));
+          },
+        });
       },
-      new BehaviorSubject(0)
-    );
+    });
     context.search({ greeting: 'hi' } as any, {}, 'mysearch').subscribe(
       response => {},
       () => {},

diff --git a/src/plugins/data/public/search/create_app_mount_context_search.ts b/src/plugins/data/public/search/create_app_mount_context_search.ts
@@ -17,8 +17,8 @@
  * under the License.
  */
 
-import { mergeMap, tap } from 'rxjs/operators';
-import { from, BehaviorSubject } from 'rxjs';
+import { mergeMap } from 'rxjs/operators';
+import { from } from 'rxjs';
 import { ISearchAppMountContext } from './i_search_app_mount_context';
 import { ISearchGeneric } from './i_search';
 import {
@@ -30,8 +30,7 @@ import { TStrategyTypes } from './strategy_types';
 import { DEFAULT_SEARCH_STRATEGY } from '../../common/search';
 
 export const createAppMountSearchContext = (
-  searchStrategies: TSearchStrategiesMap,
-  loadingCount$: BehaviorSubject<number>
+  searchStrategies: TSearchStrategiesMap
 ): ISearchAppMountContext => {
   const getSearchStrategy = <K extends TStrategyTypes = typeof DEFAULT_SEARCH_STRATEGY>(
     strategyName?: K
@@ -47,17 +46,7 @@ export const createAppMountSearchContext = (
 
   const search: ISearchGeneric = (request, options, strategyName) => {
     const strategyPromise = getSearchStrategy(strategyName);
-    return from(strategyPromise).pipe(
-      mergeMap(strategy => {
-        loadingCount$.next(loadingCount$.getValue() + 1);
-        return strategy.search(request, options).pipe(
-          tap(
-            error => loadingCount$.next(loadingCount$.getValue() - 1),
-            complete => loadingCount$.next(loadingCount$.getValue() - 1)
-          )
-        );
-      })
-    );
+    return from(strategyPromise).pipe(mergeMap(strategy => strategy.search(request, options)));
   };
 
   return { search };

diff --git a/src/plugins/data/public/search/es_client/get_es_client.ts b/src/plugins/data/public/search/es_client/get_es_client.ts
@@ -25,8 +25,7 @@ import { BehaviorSubject } from 'rxjs';
 export function getEsClient(
   injectedMetadata: CoreStart['injectedMetadata'],
   http: CoreStart['http'],
-  packageInfo: PackageInfo,
-  loadingCount$: BehaviorSubject<number>
+  packageInfo: PackageInfo
 ) {
   const esRequestTimeout = injectedMetadata.getInjectedVar('esRequestTimeout') as number;
   const esApiVersion = injectedMetadata.getInjectedVar('esApiVersion') as string;
@@ -39,6 +38,9 @@ export function getEsClient(
     apiVersion: esApiVersion,
   });
 
+  const loadingCount$ = new BehaviorSubject(0);
+  http.addLoadingCountSource(loadingCount$);
+
   return {
     search: wrapEsClientMethod(client, 'search', loadingCount$),
     msearch: wrapEsClientMethod(client, 'msearch', loadingCount$),

diff --git a/src/plugins/data/public/search/search_service.ts b/src/plugins/data/public/search/search_service.ts
@@ -16,7 +16,6 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-import { BehaviorSubject } from 'rxjs';
 import {
   Plugin,
   CoreSetup,
@@ -80,22 +79,17 @@ export class SearchService implements Plugin<ISearchSetup, ISearchStart> {
   private contextContainer?: IContextContainer<TSearchStrategyProvider<any>>;
   private esClient?: LegacyApiCaller;
   private search?: ISearchGeneric;
-  private readonly loadingCount$ = new BehaviorSubject(0);
 
   constructor(private initializerContext: PluginInitializerContext) {}
 
   public setup(core: CoreSetup, packageInfo: PackageInfo): ISearchSetup {
-    core.http.addLoadingCountSource(this.loadingCount$);
-    const search = (this.search = createAppMountSearchContext(
-      this.searchStrategies,
-      this.loadingCount$
-    ).search);
+    const search = (this.search = createAppMountSearchContext(this.searchStrategies).search);
     core.application.registerMountContext<'search'>('search', () => {
       return { search };
     });
 
     this.contextContainer = core.context.createContextContainer();
-    this.esClient = getEsClient(core.injectedMetadata, core.http, packageInfo, this.loadingCount$);
+    this.esClient = getEsClient(core.injectedMetadata, core.http, packageInfo);
 
     const registerSearchStrategyProvider: TRegisterSearchStrategyProvider = <
       T extends TStrategyTypes

diff --git a/src/plugins/data/public/search/sync_search_strategy.ts b/src/plugins/data/public/search/sync_search_strategy.ts
@@ -17,7 +17,7 @@
  * under the License.
  */
 
-import { from } from 'rxjs';
+import { BehaviorSubject, from } from 'rxjs';
 import { IKibanaSearchRequest, IKibanaSearchResponse } from '../../common/search';
 import { ISearchContext } from './i_search_context';
 import { ISearch, ISearchOptions } from './i_search';
@@ -31,24 +31,27 @@ export interface ISyncSearchRequest extends IKibanaSearchRequest {
 
 export const syncSearchStrategyProvider: TSearchStrategyProvider<typeof SYNC_SEARCH_STRATEGY> = (
   context: ISearchContext
-) => {
+): ISearchStrategy<typeof SYNC_SEARCH_STRATEGY> => {
+  const loadingCount$ = new BehaviorSubject(0);
+  context.core.http.addLoadingCountSource(loadingCount$);
+
   const search: ISearch<typeof SYNC_SEARCH_STRATEGY> = (
     request: ISyncSearchRequest,
     options: ISearchOptions = {}
   ) => {
+    loadingCount$.next(loadingCount$.getValue() + 1);
+
     const response: Promise<IKibanaSearchResponse> = context.core.http.fetch({
       path: `/internal/search/${request.serverStrategy}`,
       method: 'POST',
       body: JSON.stringify(request),
       signal: options.signal,
     });
 
-    return from(response);
-  };
+    response.then(() => loadingCount$.next(loadingCount$.getValue() - 1));
 
-  const strategy: ISearchStrategy<typeof SYNC_SEARCH_STRATEGY> = {
-    search,
+    return from(response);
   };
 
-  return strategy;
+  return { search };
 };