Skip to content

Commit

Permalink
Introduce search interceptor (elastic#60523)
Browse files Browse the repository at this point in the history
* 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 <olson.lukas@gmail.com>

Co-authored-by: Lukas Olson <olson.lukas@gmail.com>
Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
  • Loading branch information
3 people authored and Liza K committed Mar 20, 2020
1 parent 3bfabb0 commit 2f1055f
Show file tree
Hide file tree
Showing 18 changed files with 543 additions and 67 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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. |

1 change: 1 addition & 0 deletions src/plugins/data/common/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,3 +26,4 @@ export * from './query';
export * from './search';
export * from './search/aggs';
export * from './types';
export * from './utils';
114 changes: 114 additions & 0 deletions src/plugins/data/common/utils/abort_utils.test.ts
Original file line number Diff line number Diff line change
@@ -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);
});
});
});
56 changes: 56 additions & 0 deletions src/plugins/data/common/utils/abort_utils.ts
Original file line number Diff line number Diff line change
@@ -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;
}
1 change: 1 addition & 0 deletions src/plugins/data/common/utils/index.ts
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';
42 changes: 4 additions & 38 deletions src/plugins/data/public/mocks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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']>>;
Expand All @@ -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: {
Expand All @@ -97,7 +64,6 @@ const createStartContract = (): Start => {
get: jest.fn().mockReturnValue(Promise.resolve({})),
} as unknown) as IndexPatternsContract,
};
return startContract;
};

export { searchSourceMock } from './search/mocks';
Expand Down
2 changes: 2 additions & 0 deletions src/plugins/data/public/search/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,4 +56,6 @@ export {
SortDirection,
} from './search_source';

export { SearchInterceptor } from './search_interceptor';

export { FetchOptions } from './fetch';
25 changes: 24 additions & 1 deletion src/plugins/data/public/search/mocks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand All @@ -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(),
},
},
};
30 changes: 30 additions & 0 deletions src/plugins/data/public/search/request_timeout_error.ts
Original file line number Diff line number Diff line change
@@ -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';
}
}
Loading

0 comments on commit 2f1055f

Please sign in to comment.