From f15f138c338d5db9423be2bc53bd196d8cc80cce Mon Sep 17 00:00:00 2001 From: Lukas Olson Date: Mon, 3 Feb 2020 11:13:52 -0700 Subject: [PATCH 01/13] Update abort controller library --- package.json | 2 +- packages/kbn-ui-shared-deps/package.json | 7 +++---- packages/kbn-ui-shared-deps/polyfills.js | 2 +- .../elasticsearch/server/lib/abortable_request_handler.js | 2 +- .../server/lib/abortable_request_handler.test.js | 2 +- yarn.lock | 5 ----- 6 files changed, 7 insertions(+), 13 deletions(-) diff --git a/package.json b/package.json index e249546e71581..4a98559b92505 100644 --- a/package.json +++ b/package.json @@ -144,7 +144,7 @@ "@types/react-grid-layout": "^0.16.7", "@types/recompose": "^0.30.5", "JSONStream": "1.3.5", - "abortcontroller-polyfill": "^1.3.0", + "abort-controller": "^3.0.0", "angular": "^1.7.9", "angular-aria": "^1.7.8", "angular-elastic": "^2.5.1", diff --git a/packages/kbn-ui-shared-deps/package.json b/packages/kbn-ui-shared-deps/package.json index 47a47449927e4..08c4edd8510aa 100644 --- a/packages/kbn-ui-shared-deps/package.json +++ b/packages/kbn-ui-shared-deps/package.json @@ -9,12 +9,11 @@ "kbn:watch": "node scripts/build --watch" }, "devDependencies": { - "@elastic/eui": "18.2.1", "@elastic/charts": "^16.1.0", + "@elastic/eui": "18.2.1", "@kbn/dev-utils": "1.0.0", "@kbn/i18n": "1.0.0", "@yarnpkg/lockfile": "^1.1.0", - "abortcontroller-polyfill": "^1.3.0", "angular": "^1.7.9", "core-js": "^3.2.1", "css-loader": "^2.1.1", @@ -24,13 +23,13 @@ "mini-css-extract-plugin": "0.8.0", "moment": "^2.24.0", "moment-timezone": "^0.5.27", + "react": "^16.12.0", "react-dom": "^16.12.0", "react-intl": "^2.8.0", - "react": "^16.12.0", "read-pkg": "^5.2.0", "regenerator-runtime": "^0.13.3", "symbol-observable": "^1.2.0", "webpack": "4.41.0", "whatwg-fetch": "^3.0.0" } -} \ No newline at end of file +} diff --git a/packages/kbn-ui-shared-deps/polyfills.js b/packages/kbn-ui-shared-deps/polyfills.js index d2305d643e4d2..612fbb9a78b50 100644 --- a/packages/kbn-ui-shared-deps/polyfills.js +++ b/packages/kbn-ui-shared-deps/polyfills.js @@ -21,6 +21,6 @@ require('core-js/stable'); require('regenerator-runtime/runtime'); require('custom-event-polyfill'); require('whatwg-fetch'); -require('abortcontroller-polyfill/dist/polyfill-patch-fetch'); +require('abort-controller/polyfill'); require('./vendor/childnode_remove_polyfill'); require('symbol-observable'); diff --git a/src/legacy/core_plugins/elasticsearch/server/lib/abortable_request_handler.js b/src/legacy/core_plugins/elasticsearch/server/lib/abortable_request_handler.js index 0b8786f0c2841..68216b92840fc 100644 --- a/src/legacy/core_plugins/elasticsearch/server/lib/abortable_request_handler.js +++ b/src/legacy/core_plugins/elasticsearch/server/lib/abortable_request_handler.js @@ -17,7 +17,7 @@ * under the License. */ -import { AbortController } from 'abortcontroller-polyfill/dist/cjs-ponyfill'; +import { AbortController } from 'abort-controller'; /* * A simple utility for generating a handler that provides a signal to the handler that signals when diff --git a/src/legacy/core_plugins/elasticsearch/server/lib/abortable_request_handler.test.js b/src/legacy/core_plugins/elasticsearch/server/lib/abortable_request_handler.test.js index d79dd4ae4e449..1c154370d1674 100644 --- a/src/legacy/core_plugins/elasticsearch/server/lib/abortable_request_handler.test.js +++ b/src/legacy/core_plugins/elasticsearch/server/lib/abortable_request_handler.test.js @@ -17,7 +17,7 @@ * under the License. */ -import { AbortSignal } from 'abortcontroller-polyfill/dist/cjs-ponyfill'; +import { AbortSignal } from 'abort-controller'; import { abortableRequestHandler } from './abortable_request_handler'; describe('abortableRequestHandler', () => { diff --git a/yarn.lock b/yarn.lock index a3acc2ae216c5..40aad5c08c914 100644 --- a/yarn.lock +++ b/yarn.lock @@ -5655,11 +5655,6 @@ abort-controller@^2.0.3: dependencies: event-target-shim "^5.0.0" -abortcontroller-polyfill@^1.3.0: - version "1.3.0" - resolved "https://registry.yarnpkg.com/abortcontroller-polyfill/-/abortcontroller-polyfill-1.3.0.tgz#de69af32ae926c210b7efbcc29bf644ee4838b00" - integrity sha512-lbWQgf+eRvku3va8poBlDBO12FigTQr9Zb7NIjXrePrhxWVKdCP2wbDl1tLDaYa18PWTom3UEWwdH13S46I+yA== - accept@3.x.x: version "3.0.2" resolved "https://registry.yarnpkg.com/accept/-/accept-3.0.2.tgz#83e41cec7e1149f3fd474880423873db6c6cc9ac" From 4d36124efb511963b9e0ea6a1ce521c32dc8b35f Mon Sep 17 00:00:00 2001 From: Lukas Olson Date: Mon, 3 Feb 2020 11:54:57 -0700 Subject: [PATCH 02/13] Bootstrap --- yarn.lock | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/yarn.lock b/yarn.lock index 40aad5c08c914..752e39abf8c90 100644 --- a/yarn.lock +++ b/yarn.lock @@ -5655,6 +5655,13 @@ abort-controller@^2.0.3: dependencies: event-target-shim "^5.0.0" +abort-controller@^3.0.0: + version "3.0.0" + resolved "https://registry.yarnpkg.com/abort-controller/-/abort-controller-3.0.0.tgz#eaf54d53b62bae4138e809ca225c8439a6efb392" + integrity sha512-h8lQ8tacZYnR3vNQTgibj+tODHI5/+l06Au2Pcriv/Gmet0eaj4TwWH41sO9wnHDiQsEj19q0drzdWdeAHtweg== + dependencies: + event-target-shim "^5.0.0" + accept@3.x.x: version "3.0.2" resolved "https://registry.yarnpkg.com/accept/-/accept-3.0.2.tgz#83e41cec7e1149f3fd474880423873db6c6cc9ac" From 491e8bb9d3dfc1e98cd2fc4aed7ada345625db40 Mon Sep 17 00:00:00 2001 From: Lukas Olson Date: Tue, 4 Feb 2020 10:39:29 -0700 Subject: [PATCH 03/13] Abort when the request is aborted --- src/plugins/data/server/search/routes.ts | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/plugins/data/server/search/routes.ts b/src/plugins/data/server/search/routes.ts index 6f726771c41b2..5e54f0237d7dc 100644 --- a/src/plugins/data/server/search/routes.ts +++ b/src/plugins/data/server/search/routes.ts @@ -18,6 +18,7 @@ */ import { schema } from '@kbn/config-schema'; +import { AbortController } from 'abort-controller'; import { IRouter } from '../../../../core/server'; export function registerSearchRoute(router: IRouter): void { @@ -35,8 +36,15 @@ export function registerSearchRoute(router: IRouter): void { async (context, request, res) => { const searchRequest = request.body; const strategy = request.params.strategy; + + const controller = new AbortController(); + const { signal } = controller; + request.events.aborted$.subscribe(() => { + controller.abort(); + }); + try { - const response = await context.search!.search(searchRequest, {}, strategy); + const response = await context.search!.search(searchRequest, { signal }, strategy); return res.ok({ body: response }); } catch (err) { return res.customError({ From 0db725745b15b83cd06fe19f52e05362a48bc7f3 Mon Sep 17 00:00:00 2001 From: Lukas Olson Date: Tue, 4 Feb 2020 12:48:59 -0700 Subject: [PATCH 04/13] Add utility and update value suggestions route --- .../autocomplete/value_suggestions_route.ts | 4 +- .../lib/get_request_aborted_signal.test.ts | 42 +++++++++++++++++++ .../server/lib/get_request_aborted_signal.ts | 32 ++++++++++++++ src/plugins/data/server/lib/index.ts | 20 +++++++++ src/plugins/data/server/search/routes.ts | 9 +--- 5 files changed, 99 insertions(+), 8 deletions(-) create mode 100644 src/plugins/data/server/lib/get_request_aborted_signal.test.ts create mode 100644 src/plugins/data/server/lib/get_request_aborted_signal.ts create mode 100644 src/plugins/data/server/lib/index.ts diff --git a/src/plugins/data/server/autocomplete/value_suggestions_route.ts b/src/plugins/data/server/autocomplete/value_suggestions_route.ts index b415e83becf93..fa847368742cf 100644 --- a/src/plugins/data/server/autocomplete/value_suggestions_route.ts +++ b/src/plugins/data/server/autocomplete/value_suggestions_route.ts @@ -22,6 +22,7 @@ import { schema } from '@kbn/config-schema'; import { IRouter } from 'kibana/server'; import { IFieldType, indexPatterns, esFilters } from '../index'; +import { getRequestAbortedSignal } from '../lib'; export function registerValueSuggestionsRoute(router: IRouter) { router.post( @@ -49,6 +50,7 @@ export function registerValueSuggestionsRoute(router: IRouter) { const { field: fieldName, query, boolFilter } = request.body; const { index } = request.params; const { dataClient } = context.core.elasticsearch; + const signal = getRequestAbortedSignal(request.events.aborted$); const autocompleteSearchOptions = { timeout: await uiSettings.get('kibana.autocompleteTimeout'), @@ -64,7 +66,7 @@ export function registerValueSuggestionsRoute(router: IRouter) { const body = await getBody(autocompleteSearchOptions, field || fieldName, query, boolFilter); try { - const result = await dataClient.callAsCurrentUser('search', { index, body }); + const result = await dataClient.callAsCurrentUser('search', { index, body }, { signal }); const buckets: any[] = get(result, 'aggregations.suggestions.buckets') || diff --git a/src/plugins/data/server/lib/get_request_aborted_signal.test.ts b/src/plugins/data/server/lib/get_request_aborted_signal.test.ts new file mode 100644 index 0000000000000..2792452279161 --- /dev/null +++ b/src/plugins/data/server/lib/get_request_aborted_signal.test.ts @@ -0,0 +1,42 @@ +/* + * 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 { Subject } from 'rxjs'; +import { getRequestAbortedSignal } from './get_request_aborted_signal'; + +describe('abortableRequestHandler', () => { + it('should call abort if disconnected', () => { + const abortedSubject = new Subject(); + const aborted$ = abortedSubject.asObservable(); + const onAborted = jest.fn(); + + const signal = getRequestAbortedSignal(aborted$); + signal.addEventListener('abort', onAborted); + + // Shouldn't be aborted or call onAborted prior to disconnecting + expect(signal.aborted).toBe(false); + expect(onAborted).not.toBeCalled(); + + abortedSubject.next(); + + // Should be aborted and call onAborted after disconnecting + expect(signal.aborted).toBe(true); + expect(onAborted).toBeCalled(); + }); +}); diff --git a/src/plugins/data/server/lib/get_request_aborted_signal.ts b/src/plugins/data/server/lib/get_request_aborted_signal.ts new file mode 100644 index 0000000000000..60f56cf406b00 --- /dev/null +++ b/src/plugins/data/server/lib/get_request_aborted_signal.ts @@ -0,0 +1,32 @@ +/* + * 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 { Observable } from 'rxjs'; +import { AbortController } from 'abort-controller'; + +/** + * A simple utility function that returns an `AbortSignal` corresponding to an `AbortController` + * which aborts when the given request is aborted. + * @param aborted$ The observable of abort events (usually `request.events.aborted$`) + */ +export function getRequestAbortedSignal(aborted$: Observable): AbortSignal { + const controller = new AbortController(); + aborted$.subscribe(() => controller.abort()); + return controller.signal; +} diff --git a/src/plugins/data/server/lib/index.ts b/src/plugins/data/server/lib/index.ts new file mode 100644 index 0000000000000..a2af456846e14 --- /dev/null +++ b/src/plugins/data/server/lib/index.ts @@ -0,0 +1,20 @@ +/* + * 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. + */ + +export { getRequestAbortedSignal } from './get_request_aborted_signal'; diff --git a/src/plugins/data/server/search/routes.ts b/src/plugins/data/server/search/routes.ts index 5e54f0237d7dc..11879d14931ae 100644 --- a/src/plugins/data/server/search/routes.ts +++ b/src/plugins/data/server/search/routes.ts @@ -18,8 +18,8 @@ */ import { schema } from '@kbn/config-schema'; -import { AbortController } from 'abort-controller'; import { IRouter } from '../../../../core/server'; +import { getRequestAbortedSignal } from '../lib'; export function registerSearchRoute(router: IRouter): void { router.post( @@ -36,12 +36,7 @@ export function registerSearchRoute(router: IRouter): void { async (context, request, res) => { const searchRequest = request.body; const strategy = request.params.strategy; - - const controller = new AbortController(); - const { signal } = controller; - request.events.aborted$.subscribe(() => { - controller.abort(); - }); + const signal = getRequestAbortedSignal(request.events.aborted$); try { const response = await context.search!.search(searchRequest, { signal }, strategy); From ad5f18875523c11b63a42778c7ce2e943043e6b3 Mon Sep 17 00:00:00 2001 From: Lukas Olson Date: Thu, 6 Feb 2020 11:05:47 -0700 Subject: [PATCH 05/13] Remove bad merge --- packages/kbn-ui-shared-deps/package.json | 1 - yarn.lock | 27 ------------------------ 2 files changed, 28 deletions(-) diff --git a/packages/kbn-ui-shared-deps/package.json b/packages/kbn-ui-shared-deps/package.json index 58bb6eb0725a7..8c8b8b8a21488 100644 --- a/packages/kbn-ui-shared-deps/package.json +++ b/packages/kbn-ui-shared-deps/package.json @@ -12,7 +12,6 @@ "abort-controller": "^3.0.0", "@elastic/eui": "18.3.0", "@elastic/charts": "^16.1.0", - "@elastic/eui": "18.2.1", "@kbn/dev-utils": "1.0.0", "@kbn/i18n": "1.0.0", "@yarnpkg/lockfile": "^1.1.0", diff --git a/yarn.lock b/yarn.lock index b3f9d85b5524f..4a787532b1e1a 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1953,33 +1953,6 @@ tabbable "^1.1.0" uuid "^3.1.0" -"@elastic/eui@18.2.1": - version "18.2.1" - resolved "https://registry.yarnpkg.com/@elastic/eui/-/eui-18.2.1.tgz#6ce6d0bd1d0541052d21f2918305524d71e91678" - integrity sha512-6C5tnWJTlBB++475i0vRoCsnz4JaYznb4zMNFLc+z5GY3vA3/E3AXTjmmBwybEicCCi3h1SnpJxZsgMakiZwRA== - dependencies: - "@types/chroma-js" "^1.4.3" - "@types/lodash" "^4.14.116" - "@types/numeral" "^0.0.25" - "@types/react-beautiful-dnd" "^10.1.0" - chroma-js "^2.0.4" - classnames "^2.2.5" - highlight.js "^9.12.0" - html "^1.0.0" - keymirror "^0.1.1" - lodash "^4.17.11" - numeral "^2.0.6" - prop-types "^15.6.0" - react-ace "^7.0.5" - react-beautiful-dnd "^10.1.0" - react-focus-lock "^1.17.7" - react-input-autosize "^2.2.2" - react-is "~16.3.0" - react-virtualized "^9.21.2" - resize-observer-polyfill "^1.5.0" - tabbable "^3.0.0" - uuid "^3.1.0" - "@elastic/eui@18.3.0": version "18.3.0" resolved "https://registry.yarnpkg.com/@elastic/eui/-/eui-18.3.0.tgz#e21c6246624f694e2ae1c7c1f1a11b612faf260a" From 48b88c0d0c76a8cc781634fa002e1e00c749d097 Mon Sep 17 00:00:00 2001 From: Lukas Olson Date: Mon, 10 Feb 2020 09:32:45 -0700 Subject: [PATCH 06/13] Revert switching abort controller libraries --- package.json | 2 +- packages/kbn-ui-shared-deps/polyfills.js | 2 +- .../elasticsearch/server/lib/abortable_request_handler.js | 2 +- .../server/lib/abortable_request_handler.test.js | 2 +- yarn.lock | 5 +++++ 5 files changed, 9 insertions(+), 4 deletions(-) diff --git a/package.json b/package.json index 539d3a067665f..4d7c31f8227b8 100644 --- a/package.json +++ b/package.json @@ -144,7 +144,7 @@ "@types/react-grid-layout": "^0.16.7", "@types/recompose": "^0.30.5", "JSONStream": "1.3.5", - "abort-controller": "^3.0.0", + "abortcontroller-polyfill": "^1.4.0", "angular": "^1.7.9", "angular-aria": "^1.7.9", "angular-elastic": "^2.5.1", diff --git a/packages/kbn-ui-shared-deps/polyfills.js b/packages/kbn-ui-shared-deps/polyfills.js index 612fbb9a78b50..d2305d643e4d2 100644 --- a/packages/kbn-ui-shared-deps/polyfills.js +++ b/packages/kbn-ui-shared-deps/polyfills.js @@ -21,6 +21,6 @@ require('core-js/stable'); require('regenerator-runtime/runtime'); require('custom-event-polyfill'); require('whatwg-fetch'); -require('abort-controller/polyfill'); +require('abortcontroller-polyfill/dist/polyfill-patch-fetch'); require('./vendor/childnode_remove_polyfill'); require('symbol-observable'); diff --git a/src/legacy/core_plugins/elasticsearch/server/lib/abortable_request_handler.js b/src/legacy/core_plugins/elasticsearch/server/lib/abortable_request_handler.js index 68216b92840fc..0b8786f0c2841 100644 --- a/src/legacy/core_plugins/elasticsearch/server/lib/abortable_request_handler.js +++ b/src/legacy/core_plugins/elasticsearch/server/lib/abortable_request_handler.js @@ -17,7 +17,7 @@ * under the License. */ -import { AbortController } from 'abort-controller'; +import { AbortController } from 'abortcontroller-polyfill/dist/cjs-ponyfill'; /* * A simple utility for generating a handler that provides a signal to the handler that signals when diff --git a/src/legacy/core_plugins/elasticsearch/server/lib/abortable_request_handler.test.js b/src/legacy/core_plugins/elasticsearch/server/lib/abortable_request_handler.test.js index 1c154370d1674..d79dd4ae4e449 100644 --- a/src/legacy/core_plugins/elasticsearch/server/lib/abortable_request_handler.test.js +++ b/src/legacy/core_plugins/elasticsearch/server/lib/abortable_request_handler.test.js @@ -17,7 +17,7 @@ * under the License. */ -import { AbortSignal } from 'abort-controller'; +import { AbortSignal } from 'abortcontroller-polyfill/dist/cjs-ponyfill'; import { abortableRequestHandler } from './abortable_request_handler'; describe('abortableRequestHandler', () => { diff --git a/yarn.lock b/yarn.lock index 7ebf896964378..a6787f37be737 100644 --- a/yarn.lock +++ b/yarn.lock @@ -5664,6 +5664,11 @@ abort-controller@^3.0.0: dependencies: event-target-shim "^5.0.0" +abortcontroller-polyfill@^1.4.0: + version "1.4.0" + resolved "https://registry.yarnpkg.com/abortcontroller-polyfill/-/abortcontroller-polyfill-1.4.0.tgz#0d5eb58e522a461774af8086414f68e1dda7a6c4" + integrity sha512-3ZFfCRfDzx3GFjO6RAkYx81lPGpUS20ISxux9gLxuKnqafNcFQo59+IoZqpO2WvQlyc287B62HDnDdNYRmlvWA== + accept@3.x.x: version "3.0.2" resolved "https://registry.yarnpkg.com/accept/-/accept-3.0.2.tgz#83e41cec7e1149f3fd474880423873db6c6cc9ac" From 21dd1c8042224a4c4cf3cd7fa055dfc23c4f68c5 Mon Sep 17 00:00:00 2001 From: Lukas Olson Date: Mon, 10 Feb 2020 09:46:19 -0700 Subject: [PATCH 07/13] Revert package.json in lib --- packages/kbn-ui-shared-deps/package.json | 2 +- yarn.lock | 7 ------- 2 files changed, 1 insertion(+), 8 deletions(-) diff --git a/packages/kbn-ui-shared-deps/package.json b/packages/kbn-ui-shared-deps/package.json index fc9d159ea9b95..fa691e1f07775 100644 --- a/packages/kbn-ui-shared-deps/package.json +++ b/packages/kbn-ui-shared-deps/package.json @@ -10,7 +10,7 @@ }, "devDependencies": { "@elastic/charts": "^17.0.2", - "abort-controller": "^3.0.0", + "abortcontroller-polyfill": "^1.4.0", "@elastic/eui": "18.3.0", "@kbn/dev-utils": "1.0.0", "@kbn/i18n": "1.0.0", diff --git a/yarn.lock b/yarn.lock index a6787f37be737..db6c4127c3519 100644 --- a/yarn.lock +++ b/yarn.lock @@ -5657,13 +5657,6 @@ abort-controller@^2.0.3: dependencies: event-target-shim "^5.0.0" -abort-controller@^3.0.0: - version "3.0.0" - resolved "https://registry.yarnpkg.com/abort-controller/-/abort-controller-3.0.0.tgz#eaf54d53b62bae4138e809ca225c8439a6efb392" - integrity sha512-h8lQ8tacZYnR3vNQTgibj+tODHI5/+l06Au2Pcriv/Gmet0eaj4TwWH41sO9wnHDiQsEj19q0drzdWdeAHtweg== - dependencies: - event-target-shim "^5.0.0" - abortcontroller-polyfill@^1.4.0: version "1.4.0" resolved "https://registry.yarnpkg.com/abortcontroller-polyfill/-/abortcontroller-polyfill-1.4.0.tgz#0d5eb58e522a461774af8086414f68e1dda7a6c4" From 0b87a0bd71a77e35b4acff9e507f2da1e838c09e Mon Sep 17 00:00:00 2001 From: Lukas Olson Date: Mon, 10 Feb 2020 10:29:30 -0700 Subject: [PATCH 08/13] Move to previous abort controller --- src/plugins/data/server/lib/get_request_aborted_signal.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/plugins/data/server/lib/get_request_aborted_signal.ts b/src/plugins/data/server/lib/get_request_aborted_signal.ts index 60f56cf406b00..d1541f1df9384 100644 --- a/src/plugins/data/server/lib/get_request_aborted_signal.ts +++ b/src/plugins/data/server/lib/get_request_aborted_signal.ts @@ -18,7 +18,8 @@ */ import { Observable } from 'rxjs'; -import { AbortController } from 'abort-controller'; +// @ts-ignore not typed +import { AbortController } from 'abortcontroller-polyfill/dist/cjs-ponyfill'; /** * A simple utility function that returns an `AbortSignal` corresponding to an `AbortController` From 35b1abb1eaf2f18a7067bdf1533af6bc3322fc11 Mon Sep 17 00:00:00 2001 From: Lukas Olson Date: Fri, 14 Feb 2020 13:25:21 -0700 Subject: [PATCH 09/13] Fix test to use fake timers to run debounced handlers --- src/plugins/data/server/lib/get_request_aborted_signal.test.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/plugins/data/server/lib/get_request_aborted_signal.test.ts b/src/plugins/data/server/lib/get_request_aborted_signal.test.ts index 2792452279161..3c1e20dbcb158 100644 --- a/src/plugins/data/server/lib/get_request_aborted_signal.test.ts +++ b/src/plugins/data/server/lib/get_request_aborted_signal.test.ts @@ -21,6 +21,8 @@ import { Subject } from 'rxjs'; import { getRequestAbortedSignal } from './get_request_aborted_signal'; describe('abortableRequestHandler', () => { + jest.useFakeTimers(); + it('should call abort if disconnected', () => { const abortedSubject = new Subject(); const aborted$ = abortedSubject.asObservable(); @@ -34,6 +36,7 @@ describe('abortableRequestHandler', () => { expect(onAborted).not.toBeCalled(); abortedSubject.next(); + jest.runAllTimers(); // Should be aborted and call onAborted after disconnecting expect(signal.aborted).toBe(true); From 914d3300055a62a93b186da658261407515e92f9 Mon Sep 17 00:00:00 2001 From: Lukas Olson Date: Tue, 18 Feb 2020 15:00:39 -0700 Subject: [PATCH 10/13] Fix loading bar not going away when cancelling --- src/plugins/data/public/search/sync_search_strategy.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/plugins/data/public/search/sync_search_strategy.ts b/src/plugins/data/public/search/sync_search_strategy.ts index b895a177d8608..dc4bc7462709d 100644 --- a/src/plugins/data/public/search/sync_search_strategy.ts +++ b/src/plugins/data/public/search/sync_search_strategy.ts @@ -48,7 +48,7 @@ export const syncSearchStrategyProvider: TSearchStrategyProvider loadingCount$.next(loadingCount$.getValue() - 1)); + response.finally(() => loadingCount$.next(loadingCount$.getValue() - 1)); return from(response); }; From 159f5f04b68010d23784175ae34b28eaf4b02196 Mon Sep 17 00:00:00 2001 From: Lukas Olson Date: Mon, 24 Feb 2020 16:43:47 -0700 Subject: [PATCH 11/13] Add test for loading count --- .../search/sync_search_strategy.test.ts | 54 +++++++++++++++++++ 1 file changed, 54 insertions(+) diff --git a/src/plugins/data/public/search/sync_search_strategy.test.ts b/src/plugins/data/public/search/sync_search_strategy.test.ts index 9378e5833f8d7..ca64690e65a33 100644 --- a/src/plugins/data/public/search/sync_search_strategy.test.ts +++ b/src/plugins/data/public/search/sync_search_strategy.test.ts @@ -50,4 +50,58 @@ describe('Sync search strategy', () => { signal: undefined, }); }); + + it('increments and decrements loading count on success', async () => { + const expectedLoadingCountValues = [0, 1, 0]; + expect.assertions(expectedLoadingCountValues.length); + let i = 0; + + mockCoreStart.http.fetch.mockImplementationOnce(() => Promise.resolve()); + + const syncSearch = syncSearchStrategyProvider({ + core: mockCoreStart, + getSearchStrategy: jest.fn(), + }); + + const loadingCount$ = mockCoreStart.http.addLoadingCountSource.mock.calls[0][0]; + loadingCount$.subscribe(value => { + expect(value).toBe(expectedLoadingCountValues[i++]); + }); + + await syncSearch.search( + { + serverStrategy: SYNC_SEARCH_STRATEGY, + }, + {} + ); + }); + + it('increments and decrements loading count on failure', async () => { + const expectedLoadingCountValues = [0, 1, 0]; + expect.assertions(expectedLoadingCountValues.length); + let i = 0; + + mockCoreStart.http.fetch.mockImplementationOnce(() => Promise.reject()); + + const syncSearch = syncSearchStrategyProvider({ + core: mockCoreStart, + getSearchStrategy: jest.fn(), + }); + + const loadingCount$ = mockCoreStart.http.addLoadingCountSource.mock.calls[0][0]; + loadingCount$.subscribe(value => { + expect(value).toBe(expectedLoadingCountValues[i++]); + }); + + try { + await syncSearch.search( + { + serverStrategy: SYNC_SEARCH_STRATEGY, + }, + {} + ); + } catch (e) { + // Just swallow the error silently (because of the expect.assertions) + } + }); }); From efa6cf6c7a2fc5abe1c16f37bcc63eddae2f62ba Mon Sep 17 00:00:00 2001 From: Lukas Olson Date: Tue, 25 Feb 2020 07:32:31 -0700 Subject: [PATCH 12/13] Fix test --- .../search/sync_search_strategy.test.ts | 47 ++++++++++--------- 1 file changed, 24 insertions(+), 23 deletions(-) diff --git a/src/plugins/data/public/search/sync_search_strategy.test.ts b/src/plugins/data/public/search/sync_search_strategy.test.ts index ca64690e65a33..b118f737f56e3 100644 --- a/src/plugins/data/public/search/sync_search_strategy.test.ts +++ b/src/plugins/data/public/search/sync_search_strategy.test.ts @@ -53,8 +53,7 @@ describe('Sync search strategy', () => { it('increments and decrements loading count on success', async () => { const expectedLoadingCountValues = [0, 1, 0]; - expect.assertions(expectedLoadingCountValues.length); - let i = 0; + const receivedLoadingCountValues: number[] = []; mockCoreStart.http.fetch.mockImplementationOnce(() => Promise.resolve()); @@ -64,22 +63,24 @@ describe('Sync search strategy', () => { }); const loadingCount$ = mockCoreStart.http.addLoadingCountSource.mock.calls[0][0]; - loadingCount$.subscribe(value => { - expect(value).toBe(expectedLoadingCountValues[i++]); - }); + loadingCount$.subscribe(value => receivedLoadingCountValues.push(value)); - await syncSearch.search( - { - serverStrategy: SYNC_SEARCH_STRATEGY, - }, - {} - ); + await syncSearch + .search( + { + serverStrategy: SYNC_SEARCH_STRATEGY, + }, + {} + ) + .toPromise(); + + expect(receivedLoadingCountValues).toEqual(expectedLoadingCountValues); }); it('increments and decrements loading count on failure', async () => { + expect.assertions(1); const expectedLoadingCountValues = [0, 1, 0]; - expect.assertions(expectedLoadingCountValues.length); - let i = 0; + const receivedLoadingCountValues: number[] = []; mockCoreStart.http.fetch.mockImplementationOnce(() => Promise.reject()); @@ -89,19 +90,19 @@ describe('Sync search strategy', () => { }); const loadingCount$ = mockCoreStart.http.addLoadingCountSource.mock.calls[0][0]; - loadingCount$.subscribe(value => { - expect(value).toBe(expectedLoadingCountValues[i++]); - }); + loadingCount$.subscribe(value => receivedLoadingCountValues.push(value)); try { - await syncSearch.search( - { - serverStrategy: SYNC_SEARCH_STRATEGY, - }, - {} - ); + await syncSearch + .search( + { + serverStrategy: SYNC_SEARCH_STRATEGY, + }, + {} + ) + .toPromise(); } catch (e) { - // Just swallow the error silently (because of the expect.assertions) + expect(receivedLoadingCountValues).toEqual(expectedLoadingCountValues); } }); }); From 2ec0d0be726a4d87e1457942552ba231a539584c Mon Sep 17 00:00:00 2001 From: Lukas Olson Date: Tue, 25 Feb 2020 11:15:38 -0700 Subject: [PATCH 13/13] Fix failing test --- .../search/sync_search_strategy.test.ts | 33 +++++-------------- .../public/search/sync_search_strategy.ts | 21 ++++++------ 2 files changed, 19 insertions(+), 35 deletions(-) diff --git a/src/plugins/data/public/search/sync_search_strategy.test.ts b/src/plugins/data/public/search/sync_search_strategy.test.ts index b118f737f56e3..31a1adfa01c75 100644 --- a/src/plugins/data/public/search/sync_search_strategy.test.ts +++ b/src/plugins/data/public/search/sync_search_strategy.test.ts @@ -35,12 +35,9 @@ describe('Sync search strategy', () => { core: mockCoreStart, getSearchStrategy: jest.fn(), }); - syncSearch.search( - { - serverStrategy: SYNC_SEARCH_STRATEGY, - }, - {} - ); + const request = { serverStrategy: SYNC_SEARCH_STRATEGY }; + syncSearch.search(request, {}); + expect(mockCoreStart.http.fetch.mock.calls[0][0]).toEqual({ path: `/internal/search/${SYNC_SEARCH_STRATEGY}`, body: JSON.stringify({ @@ -55,24 +52,18 @@ describe('Sync search strategy', () => { const expectedLoadingCountValues = [0, 1, 0]; const receivedLoadingCountValues: number[] = []; - mockCoreStart.http.fetch.mockImplementationOnce(() => Promise.resolve()); + mockCoreStart.http.fetch.mockResolvedValueOnce('response'); const syncSearch = syncSearchStrategyProvider({ core: mockCoreStart, getSearchStrategy: jest.fn(), }); + const request = { serverStrategy: SYNC_SEARCH_STRATEGY }; const loadingCount$ = mockCoreStart.http.addLoadingCountSource.mock.calls[0][0]; loadingCount$.subscribe(value => receivedLoadingCountValues.push(value)); - await syncSearch - .search( - { - serverStrategy: SYNC_SEARCH_STRATEGY, - }, - {} - ) - .toPromise(); + await syncSearch.search(request, {}).toPromise(); expect(receivedLoadingCountValues).toEqual(expectedLoadingCountValues); }); @@ -82,25 +73,19 @@ describe('Sync search strategy', () => { const expectedLoadingCountValues = [0, 1, 0]; const receivedLoadingCountValues: number[] = []; - mockCoreStart.http.fetch.mockImplementationOnce(() => Promise.reject()); + mockCoreStart.http.fetch.mockRejectedValueOnce('error'); const syncSearch = syncSearchStrategyProvider({ core: mockCoreStart, getSearchStrategy: jest.fn(), }); + const request = { serverStrategy: SYNC_SEARCH_STRATEGY }; const loadingCount$ = mockCoreStart.http.addLoadingCountSource.mock.calls[0][0]; loadingCount$.subscribe(value => receivedLoadingCountValues.push(value)); try { - await syncSearch - .search( - { - serverStrategy: SYNC_SEARCH_STRATEGY, - }, - {} - ) - .toPromise(); + await syncSearch.search(request, {}).toPromise(); } catch (e) { expect(receivedLoadingCountValues).toEqual(expectedLoadingCountValues); } diff --git a/src/plugins/data/public/search/sync_search_strategy.ts b/src/plugins/data/public/search/sync_search_strategy.ts index 4b66b0e32894a..860ce593ae217 100644 --- a/src/plugins/data/public/search/sync_search_strategy.ts +++ b/src/plugins/data/public/search/sync_search_strategy.ts @@ -18,7 +18,8 @@ */ import { BehaviorSubject, from } from 'rxjs'; -import { IKibanaSearchRequest, IKibanaSearchResponse } from '../../common/search'; +import { finalize } from 'rxjs/operators'; +import { IKibanaSearchRequest } from '../../common/search'; import { ISearch, ISearchOptions } from './i_search'; import { TSearchStrategyProvider, ISearchStrategy, ISearchContext } from './types'; @@ -40,16 +41,14 @@ export const syncSearchStrategyProvider: TSearchStrategyProvider { loadingCount$.next(loadingCount$.getValue() + 1); - const response: Promise = context.core.http.fetch({ - path: `/internal/search/${request.serverStrategy}`, - method: 'POST', - body: JSON.stringify(request), - signal: options.signal, - }); - - response.finally(() => loadingCount$.next(loadingCount$.getValue() - 1)); - - return from(response); + return from( + context.core.http.fetch({ + path: `/internal/search/${request.serverStrategy}`, + method: 'POST', + body: JSON.stringify(request), + signal: options.signal, + }) + ).pipe(finalize(() => loadingCount$.next(loadingCount$.getValue() - 1))); }; return { search };