Introduce search interceptor (#60523)

* Add async search strategy * Add async search * Fix async strategy and add tests * Move types to separate file * Revert changes to demo search * Update demo search strategy to use async * Add async es search strategy * Return response as rawResponse * Poll after initial request * Add cancellation to search strategies * Add tests * Simplify async search strategy * Move loadingCount to search strategy * Update abort controller library * Bootstrap * Abort when the request is aborted * Add utility and update value suggestions route * Fix bad merge conflict * Update tests * Move to data_enhanced plugin * Remove bad merge * Revert switching abort controller libraries * Revert package.json in lib * Move to previous abort controller * Add support for frozen indices * Fix test to use fake timers to run debounced handlers * Revert changes to example plugin * Fix loading bar not going away when cancelling * Call getSearchStrategy instead of passing directly * Add async demo search strategy * Fix error with setting state * Update how aborting works * Fix type checks * Add test for loading count * Attempt to fix broken example test * Revert changes to test * Fix test * Update name to camelCase * Fix failing test * Don't require data_enhanced in example plugin * Actually send DELETE request * Use waitForCompletion parameter * Use default search params * Add support for rollups * Only make changes needed for frozen indices/rollups * Only make changes needed for frozen indices/rollups * Add back in async functionality * Fix tests/types * Fix issue with sending empty body in GET * Don't include skipped in loaded/total * Don't wait before polling the next time * Add search interceptor for bulk managing searches * Simplify search logic * Fix merge error * Review feedback * Add service for running beyond timeout * Refactor abort utils * Remove unneeded changes * Add tests * cleanup mocks * Update src/legacy/core_plugins/kibana/public/dashboard/np_ready/dashboard_app.html Co-Authored-By: Lukas Olson <[email protected]> Co-authored-by: Lukas Olson <[email protected]> Co-authored-by: Elastic Machine <[email protected]>
elastic · Mar 19, 2020 · 2eda06e · 2eda06e
1 parent 836b3d0
commit 2eda06e
Show file tree

Hide file tree

Showing 18 changed files with 543 additions and 67 deletions.
diff --git a/docs/development/plugins/data/server/kibana-plugin-plugins-data-server.md b/docs/development/plugins/data/server/kibana-plugin-plugins-data-server.md
@@ -60,7 +60,6 @@
 |  [esQuery](./kibana-plugin-plugins-data-server.esquery.md) |  |
 |  [fieldFormats](./kibana-plugin-plugins-data-server.fieldformats.md) |  |
 |  [indexPatterns](./kibana-plugin-plugins-data-server.indexpatterns.md) |  |
-|  [search](./kibana-plugin-plugins-data-server.search.md) |  |
 
 ## Type Aliases
 
@@ -70,6 +69,5 @@
 |  [IFieldFormatsRegistry](./kibana-plugin-plugins-data-server.ifieldformatsregistry.md) |  |
 |  [ISearch](./kibana-plugin-plugins-data-server.isearch.md) |  |
 |  [ISearchCancel](./kibana-plugin-plugins-data-server.isearchcancel.md) |  |
-|  [ParsedInterval](./kibana-plugin-plugins-data-server.parsedinterval.md) |  |
 |  [TSearchStrategyProvider](./kibana-plugin-plugins-data-server.tsearchstrategyprovider.md) | Search strategy provider creates an instance of a search strategy with the request handler context bound to it. This way every search strategy can use whatever information they require from the request context. |
 
diff --git a/src/plugins/data/common/index.ts b/src/plugins/data/common/index.ts
@@ -26,3 +26,4 @@ export * from './query';
 export * from './search';
 export * from './search/aggs';
 export * from './types';
+export * from './utils';
diff --git a/src/plugins/data/common/utils/abort_utils.test.ts b/src/plugins/data/common/utils/abort_utils.test.ts
@@ -0,0 +1,114 @@
+/*
+ * Licensed to Elasticsearch B.V. under one or more contributor
+ * license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright
+ * ownership. Elasticsearch B.V. licenses this file to you under
+ * the Apache License, Version 2.0 (the "License"); you may
+ * not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import { AbortError, toPromise, getCombinedSignal } from './abort_utils';
+
+jest.useFakeTimers();
+
+const flushPromises = () => new Promise(resolve => setImmediate(resolve));
+
+describe('AbortUtils', () => {
+  describe('AbortError', () => {
+    test('should preserve `message`', () => {
+      const message = 'my error message';
+      const error = new AbortError(message);
+      expect(error.message).toBe(message);
+    });
+
+    test('should have a name of "AbortError"', () => {
+      const error = new AbortError();
+      expect(error.name).toBe('AbortError');
+    });
+  });
+
+  describe('toPromise', () => {
+    describe('resolves', () => {
+      test('should not resolve if the signal does not abort', async () => {
+        const controller = new AbortController();
+        const promise = toPromise(controller.signal);
+        const whenResolved = jest.fn();
+        promise.then(whenResolved);
+        await flushPromises();
+        expect(whenResolved).not.toBeCalled();
+      });
+
+      test('should resolve if the signal does abort', async () => {
+        const controller = new AbortController();
+        const promise = toPromise(controller.signal);
+        const whenResolved = jest.fn();
+        promise.then(whenResolved);
+        controller.abort();
+        await flushPromises();
+        expect(whenResolved).toBeCalled();
+      });
+    });
+
+    describe('rejects', () => {
+      test('should not reject if the signal does not abort', async () => {
+        const controller = new AbortController();
+        const promise = toPromise(controller.signal, true);
+        const whenRejected = jest.fn();
+        promise.catch(whenRejected);
+        await flushPromises();
+        expect(whenRejected).not.toBeCalled();
+      });
+
+      test('should reject if the signal does abort', async () => {
+        const controller = new AbortController();
+        const promise = toPromise(controller.signal, true);
+        const whenRejected = jest.fn();
+        promise.catch(whenRejected);
+        controller.abort();
+        await flushPromises();
+        expect(whenRejected).toBeCalled();
+      });
+    });
+  });
+
+  describe('getCombinedSignal', () => {
+    test('should return an AbortSignal', () => {
+      const signal = getCombinedSignal([]);
+      expect(signal instanceof AbortSignal).toBe(true);
+    });
+
+    test('should not abort if none of the signals abort', async () => {
+      const controller1 = new AbortController();
+      const controller2 = new AbortController();
+      setTimeout(() => controller1.abort(), 2000);
+      setTimeout(() => controller2.abort(), 1000);
+      const signal = getCombinedSignal([controller1.signal, controller2.signal]);
+      expect(signal.aborted).toBe(false);
+      jest.advanceTimersByTime(500);
+      await flushPromises();
+      expect(signal.aborted).toBe(false);
+    });
+
+    test('should abort when the first signal aborts', async () => {
+      const controller1 = new AbortController();
+      const controller2 = new AbortController();
+      setTimeout(() => controller1.abort(), 2000);
+      setTimeout(() => controller2.abort(), 1000);
+      const signal = getCombinedSignal([controller1.signal, controller2.signal]);
+      expect(signal.aborted).toBe(false);
+      jest.advanceTimersByTime(1000);
+      await flushPromises();
+      expect(signal.aborted).toBe(true);
+    });
+  });
+});
diff --git a/src/plugins/data/common/utils/abort_utils.ts b/src/plugins/data/common/utils/abort_utils.ts
@@ -0,0 +1,56 @@
+/*
+ * Licensed to Elasticsearch B.V. under one or more contributor
+ * license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright
+ * ownership. Elasticsearch B.V. licenses this file to you under
+ * the Apache License, Version 2.0 (the "License"); you may
+ * not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+/**
+ * Class used to signify that something was aborted. Useful for applications to conditionally handle
+ * this type of error differently than other errors.
+ */
+export class AbortError extends Error {
+  constructor(message = 'Aborted') {
+    super(message);
+    this.message = message;
+    this.name = 'AbortError';
+  }
+}
+
+/**
+ * Returns a `Promise` corresponding with when the given `AbortSignal` is aborted. Useful for
+ * situations when you might need to `Promise.race` multiple `AbortSignal`s, or an `AbortSignal`
+ * with any other expected errors (or completions).
+ * @param signal The `AbortSignal` to generate the `Promise` from
+ * @param shouldReject If `false`, the promise will be resolved, otherwise it will be rejected
+ */
+export function toPromise(signal: AbortSignal, shouldReject = false) {
+  return new Promise((resolve, reject) => {
+    const action = shouldReject ? reject : resolve;
+    if (signal.aborted) action();
+    signal.addEventListener('abort', action);
+  });
+}
+
+/**
+ * Returns an `AbortSignal` that will be aborted when the first of the given signals aborts.
+ * @param signals
+ */
+export function getCombinedSignal(signals: AbortSignal[]) {
+  const promises = signals.map(signal => toPromise(signal));
+  const controller = new AbortController();
+  Promise.race(promises).then(() => controller.abort());
+  return controller.signal;
+}
diff --git a/src/plugins/data/common/utils/index.ts b/src/plugins/data/common/utils/index.ts
@@ -19,3 +19,4 @@
 
 /** @internal */
 export { shortenDottedString } from './shorten_dotted_string';
+export { AbortError, toPromise, getCombinedSignal } from './abort_utils';
diff --git a/src/plugins/data/public/mocks.ts b/src/plugins/data/public/mocks.ts
@@ -19,9 +19,7 @@
 
 import { Plugin, DataPublicPluginSetup, DataPublicPluginStart, IndexPatternsContract } from '.';
 import { fieldFormatsMock } from '../common/field_formats/mocks';
-import { searchSetupMock } from './search/mocks';
-import { AggTypeFieldFilters } from './search/aggs';
-import { searchAggsStartMock } from './search/aggs/mocks';
+import { searchSetupMock, searchStartMock } from './search/mocks';
 import { queryServiceMock } from './query/mocks';
 
 export type Setup = jest.Mocked<ReturnType<Plugin['setup']>>;
@@ -35,59 +33,28 @@ const autocompleteMock: any = {
 
 const createSetupContract = (): Setup => {
   const querySetupMock = queryServiceMock.createSetupContract();
-  const setupContract = {
+  return {
     autocomplete: autocompleteMock,
     search: searchSetupMock,
     fieldFormats: fieldFormatsMock as DataPublicPluginSetup['fieldFormats'],
     query: querySetupMock,
-    __LEGACY: {
-      esClient: {
-        search: jest.fn(),
-        msearch: jest.fn(),
-      },
-    },
   };
-
-  return setupContract;
 };
 
 const createStartContract = (): Start => {
   const queryStartMock = queryServiceMock.createStartContract();
-  const startContract = {
+  return {
     actions: {
       createFiltersFromEvent: jest.fn().mockResolvedValue(['yes']),
     },
     autocomplete: autocompleteMock,
-    getSuggestions: jest.fn(),
-    search: {
-      aggs: searchAggsStartMock(),
-      search: jest.fn(),
-      __LEGACY: {
-        AggConfig: jest.fn() as any,
-        AggType: jest.fn(),
-        aggTypeFieldFilters: new AggTypeFieldFilters(),
-        FieldParamType: jest.fn(),
-        MetricAggType: jest.fn(),
-        parentPipelineAggHelper: jest.fn() as any,
-        siblingPipelineAggHelper: jest.fn() as any,
-        esClient: {
-          search: jest.fn(),
-          msearch: jest.fn(),
-        },
-      },
-    },
+    search: searchStartMock,
     fieldFormats: fieldFormatsMock as DataPublicPluginStart['fieldFormats'],
     query: queryStartMock,
     ui: {
       IndexPatternSelect: jest.fn(),
       SearchBar: jest.fn(),
     },
-    __LEGACY: {
-      esClient: {
-        search: jest.fn(),
-        msearch: jest.fn(),
-      },
-    },
     indexPatterns: ({
       make: () => ({
         fieldsFetcher: {
@@ -97,7 +64,6 @@ const createStartContract = (): Start => {
       get: jest.fn().mockReturnValue(Promise.resolve({})),
     } as unknown) as IndexPatternsContract,
   };
-  return startContract;
 };
 
 export { searchSourceMock } from './search/mocks';

diff --git a/src/plugins/data/public/search/index.ts b/src/plugins/data/public/search/index.ts
@@ -56,4 +56,6 @@ export {
   SortDirection,
 } from './search_source';
 
+export { SearchInterceptor } from './search_interceptor';
+
 export { FetchOptions } from './fetch';
diff --git a/src/plugins/data/public/search/mocks.ts b/src/plugins/data/public/search/mocks.ts
@@ -17,7 +17,9 @@
  * under the License.
  */
 
-import { searchAggsSetupMock } from './aggs/mocks';
+import { searchAggsSetupMock, searchAggsStartMock } from './aggs/mocks';
+import { AggTypeFieldFilters } from './aggs/param_types/filter';
+import { ISearchStart } from './types';
 
 export * from './search_source/mocks';
 
@@ -26,3 +28,24 @@ export const searchSetupMock = {
   registerSearchStrategyContext: jest.fn(),
   registerSearchStrategyProvider: jest.fn(),
 };
+
+export const searchStartMock: jest.Mocked<ISearchStart> = {
+  aggs: searchAggsStartMock(),
+  search: jest.fn(),
+  cancel: jest.fn(),
+  getPendingCount$: jest.fn(),
+  runBeyondTimeout: jest.fn(),
+  __LEGACY: {
+    AggConfig: jest.fn() as any,
+    AggType: jest.fn(),
+    aggTypeFieldFilters: new AggTypeFieldFilters(),
+    FieldParamType: jest.fn(),
+    MetricAggType: jest.fn(),
+    parentPipelineAggHelper: jest.fn() as any,
+    siblingPipelineAggHelper: jest.fn() as any,
+    esClient: {
+      search: jest.fn(),
+      msearch: jest.fn(),
+    },
+  },
+};
diff --git a/src/plugins/data/public/search/request_timeout_error.ts b/src/plugins/data/public/search/request_timeout_error.ts
@@ -0,0 +1,30 @@
+/*
+ * Licensed to Elasticsearch B.V. under one or more contributor
+ * license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright
+ * ownership. Elasticsearch B.V. licenses this file to you under
+ * the Apache License, Version 2.0 (the "License"); you may
+ * not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+/**
+ * Class used to signify that a request timed out. Useful for applications to conditionally handle
+ * this type of error differently than other errors.
+ */
+export class RequestTimeoutError extends Error {
+  constructor(message = 'Request timed out') {
+    super(message);
+    this.message = message;
+    this.name = 'RequestTimeoutError';
+  }
+}
Original file line number	Diff line number	Diff line change
Expand Up		@@ -19,3 +19,4 @@

		/** @internal */
		export { shortenDottedString } from './shorten_dotted_string';
		export { AbortError, toPromise, getCombinedSignal } from './abort_utils';