From 5181e59e22801bd4b6cd3234d2b3120621579fc5 Mon Sep 17 00:00:00 2001 From: Devon Thomson Date: Wed, 12 Oct 2022 12:48:10 -0400 Subject: [PATCH 1/4] Fix Initialization Race Condition --- .../embeddable/control_group_container.tsx | 27 ++++++++++++++++--- .../embeddable/dashboard_container.tsx | 2 +- .../viewport/dashboard_viewport.tsx | 4 ++- 3 files changed, 28 insertions(+), 5 deletions(-) diff --git a/src/plugins/controls/public/control_group/embeddable/control_group_container.tsx b/src/plugins/controls/public/control_group/embeddable/control_group_container.tsx index c07715007b46c..a1d8bc06f822a 100644 --- a/src/plugins/controls/public/control_group/embeddable/control_group_container.tsx +++ b/src/plugins/controls/public/control_group/embeddable/control_group_container.tsx @@ -10,7 +10,7 @@ import { skip, debounceTime, distinctUntilChanged } from 'rxjs/operators'; import React from 'react'; import ReactDOM from 'react-dom'; import { Filter, uniqFilters } from '@kbn/es-query'; -import { merge, Subject, Subscription } from 'rxjs'; +import { BehaviorSubject, merge, Subject, Subscription } from 'rxjs'; import { EuiContextMenuPanel } from '@elastic/eui'; import { @@ -57,6 +57,8 @@ export class ControlGroupContainer extends Container< public readonly type = CONTROL_GROUP_TYPE; public readonly anyControlOutputConsumerLoading$: Subject = new Subject(); + private initialized$ = new BehaviorSubject(false); + private subscriptions: Subscription = new Subscription(); private domNode?: HTMLElement; private recalculateFilters$: Subject; @@ -194,10 +196,11 @@ export class ControlGroupContainer extends Container< }); // when all children are ready setup subscriptions - this.untilReady().then(() => { + this.untilAllChildrenReady().then(() => { this.recalculateDataViews(); this.recalculateFilters(); this.setupSubscriptions(); + this.initialized$.next(true); }); } @@ -327,7 +330,7 @@ export class ControlGroupContainer extends Container< }; } - public untilReady = () => { + public untilAllChildrenReady = () => { const panelsLoading = () => Object.keys(this.getInput().panels).some( (panelId) => !this.getOutput().embeddableLoaded[panelId] @@ -349,6 +352,24 @@ export class ControlGroupContainer extends Container< return Promise.resolve(); }; + public untilInitialized = () => { + if (this.initialized$.value === false) { + return new Promise((resolve, reject) => { + const subscription = this.initialized$.subscribe((isInitialized) => { + if (this.destroyed) { + subscription.unsubscribe(); + reject(); + } + if (isInitialized) { + subscription.unsubscribe(); + resolve(); + } + }); + }); + } + return Promise.resolve(); + }; + public render(dom: HTMLElement) { if (this.domNode) { ReactDOM.unmountComponentAtNode(this.domNode); diff --git a/src/plugins/dashboard/public/application/embeddable/dashboard_container.tsx b/src/plugins/dashboard/public/application/embeddable/dashboard_container.tsx index d8c66155d8b30..a6aa2bfb44ccf 100644 --- a/src/plugins/dashboard/public/application/embeddable/dashboard_container.tsx +++ b/src/plugins/dashboard/public/application/embeddable/dashboard_container.tsx @@ -153,7 +153,7 @@ export class DashboardContainer extends Container { + this.controlGroup.untilInitialized().then(() => { if (!this.controlGroup || isErrorEmbeddable(this.controlGroup)) return; syncDashboardControlGroup({ dashboardContainer: this, diff --git a/src/plugins/dashboard/public/application/embeddable/viewport/dashboard_viewport.tsx b/src/plugins/dashboard/public/application/embeddable/viewport/dashboard_viewport.tsx index 52e76295eb998..aade1e29d8107 100644 --- a/src/plugins/dashboard/public/application/embeddable/viewport/dashboard_viewport.tsx +++ b/src/plugins/dashboard/public/application/embeddable/viewport/dashboard_viewport.tsx @@ -86,7 +86,9 @@ export class DashboardViewport extends React.Component this.setState({ controlGroupReady: true })); + this.props.controlGroup + ?.untilInitialized() + .then(() => this.setState({ controlGroupReady: true })); } } From 690dce6422fa6795e30fabb75593d18f4771d7fd Mon Sep 17 00:00:00 2001 From: Devon Thomson Date: Wed, 12 Oct 2022 12:49:11 -0400 Subject: [PATCH 2/4] Unskip Functional test which was made flaky --- .../group3/drilldowns/dashboard_to_dashboard_drilldown.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/x-pack/test/functional/apps/dashboard/group3/drilldowns/dashboard_to_dashboard_drilldown.ts b/x-pack/test/functional/apps/dashboard/group3/drilldowns/dashboard_to_dashboard_drilldown.ts index 1323f62f2a4f9..3b1f9c2065180 100644 --- a/x-pack/test/functional/apps/dashboard/group3/drilldowns/dashboard_to_dashboard_drilldown.ts +++ b/x-pack/test/functional/apps/dashboard/group3/drilldowns/dashboard_to_dashboard_drilldown.ts @@ -37,9 +37,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { const spaces = getService('spaces'); const elasticChart = getService('elasticChart'); - // Failing: See https://github.com/elastic/kibana/issues/142913 - // Failing: See https://github.com/elastic/kibana/issues/142912 - describe.skip('Dashboard to dashboard drilldown', function () { + describe('Dashboard to dashboard drilldown', function () { describe('Create & use drilldowns', () => { before(async () => { log.debug('Dashboard Drilldowns:initTests'); From e4b8e81499d5c339651ea1a6266564b2c13bd068 Mon Sep 17 00:00:00 2001 From: Devon Thomson Date: Wed, 12 Oct 2022 13:01:33 -0400 Subject: [PATCH 3/4] set initialized on next frame --- .../control_group/embeddable/control_group_container.tsx | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/plugins/controls/public/control_group/embeddable/control_group_container.tsx b/src/plugins/controls/public/control_group/embeddable/control_group_container.tsx index a1d8bc06f822a..c3cc6759c381e 100644 --- a/src/plugins/controls/public/control_group/embeddable/control_group_container.tsx +++ b/src/plugins/controls/public/control_group/embeddable/control_group_container.tsx @@ -200,7 +200,8 @@ export class ControlGroupContainer extends Container< this.recalculateDataViews(); this.recalculateFilters(); this.setupSubscriptions(); - this.initialized$.next(true); + // set initialized state on next frame to ensure initial filters have been published into output. + setTimeout(() => this.initialized$.next(true)); }); } From 0dd5ae8375a5c13dce85cbfff11e202e4ac3a61d Mon Sep 17 00:00:00 2001 From: Devon Thomson Date: Wed, 12 Oct 2022 14:30:13 -0400 Subject: [PATCH 4/4] Ensure control group is ready before building dashboard container. --- .../embeddable/control_group_container.tsx | 3 +- .../embeddable/dashboard_container.tsx | 17 +++++------ .../dashboard_container_factory.tsx | 28 +++++++++++++------ .../viewport/dashboard_viewport.tsx | 11 +------- 4 files changed, 29 insertions(+), 30 deletions(-) diff --git a/src/plugins/controls/public/control_group/embeddable/control_group_container.tsx b/src/plugins/controls/public/control_group/embeddable/control_group_container.tsx index c3cc6759c381e..a1d8bc06f822a 100644 --- a/src/plugins/controls/public/control_group/embeddable/control_group_container.tsx +++ b/src/plugins/controls/public/control_group/embeddable/control_group_container.tsx @@ -200,8 +200,7 @@ export class ControlGroupContainer extends Container< this.recalculateDataViews(); this.recalculateFilters(); this.setupSubscriptions(); - // set initialized state on next frame to ensure initial filters have been published into output. - setTimeout(() => this.initialized$.next(true)); + this.initialized$.next(true); }); } diff --git a/src/plugins/dashboard/public/application/embeddable/dashboard_container.tsx b/src/plugins/dashboard/public/application/embeddable/dashboard_container.tsx index a6aa2bfb44ccf..d4ee045a8d9a8 100644 --- a/src/plugins/dashboard/public/application/embeddable/dashboard_container.tsx +++ b/src/plugins/dashboard/public/application/embeddable/dashboard_container.tsx @@ -153,16 +153,13 @@ export class DashboardContainer extends Container { - if (!this.controlGroup || isErrorEmbeddable(this.controlGroup)) return; - syncDashboardControlGroup({ - dashboardContainer: this, - controlGroup: this.controlGroup, - }).then((result) => { - if (!result) return; - const { onDestroyControlGroup } = result; - this.onDestroyControlGroup = onDestroyControlGroup; - }); + syncDashboardControlGroup({ + dashboardContainer: this, + controlGroup: this.controlGroup, + }).then((result) => { + if (!result) return; + const { onDestroyControlGroup } = result; + this.onDestroyControlGroup = onDestroyControlGroup; }); } diff --git a/src/plugins/dashboard/public/application/embeddable/dashboard_container_factory.tsx b/src/plugins/dashboard/public/application/embeddable/dashboard_container_factory.tsx index 58a2c63492c09..40c25b454be9e 100644 --- a/src/plugins/dashboard/public/application/embeddable/dashboard_container_factory.tsx +++ b/src/plugins/dashboard/public/application/embeddable/dashboard_container_factory.tsx @@ -21,6 +21,7 @@ import { ContainerOutput, EmbeddableFactory, EmbeddableFactoryDefinition, + isErrorEmbeddable, } from '@kbn/embeddable-plugin/public'; import { getDefaultControlGroupInput } from '@kbn/controls-plugin/common'; @@ -73,15 +74,13 @@ export class DashboardContainerFactoryDefinition }; } - public create = async ( - initialInput: DashboardContainerInput, - parent?: Container - ): Promise => { + private buildControlGroup = async ( + initialInput: DashboardContainerInput + ): Promise => { const { pluginServices } = await import('../../services/plugin_services'); const { embeddable: { getEmbeddableFactory }, } = pluginServices.getServices(); - const controlsGroupFactory = getEmbeddableFactory< ControlGroupInput, ControlGroupOutput, @@ -97,10 +96,23 @@ export class DashboardContainerFactoryDefinition filters, query, }); + if (controlGroup && !isErrorEmbeddable(controlGroup)) { + await controlGroup.untilInitialized(); + } + return controlGroup; + }; + + public create = async ( + initialInput: DashboardContainerInput, + parent?: Container + ): Promise => { + const controlGroupPromise = this.buildControlGroup(initialInput); + const dashboardContainerPromise = import('./dashboard_container'); - const { DashboardContainer: DashboardContainerEmbeddable } = await import( - './dashboard_container' - ); + const [controlGroup, { DashboardContainer: DashboardContainerEmbeddable }] = await Promise.all([ + controlGroupPromise, + dashboardContainerPromise, + ]); return Promise.resolve(new DashboardContainerEmbeddable(initialInput, parent, controlGroup)); }; diff --git a/src/plugins/dashboard/public/application/embeddable/viewport/dashboard_viewport.tsx b/src/plugins/dashboard/public/application/embeddable/viewport/dashboard_viewport.tsx index aade1e29d8107..dff5ff80bcc27 100644 --- a/src/plugins/dashboard/public/application/embeddable/viewport/dashboard_viewport.tsx +++ b/src/plugins/dashboard/public/application/embeddable/viewport/dashboard_viewport.tsx @@ -33,7 +33,6 @@ export interface DashboardViewportProps { interface State { isFullScreenMode: boolean; - controlGroupReady: boolean; useMargins: boolean; title: string; description?: string; @@ -57,7 +56,6 @@ export class DashboardViewport extends React.Component this.setState({ controlGroupReady: true })); - } } public componentWillUnmount() { @@ -166,9 +159,7 @@ export class DashboardViewport extends React.Component )} - {this.state.controlGroupReady && ( - - )} + );