From 702827b42f022782dcaaa0649e9056b38bb22824 Mon Sep 17 00:00:00 2001 From: Devon Thomson Date: Wed, 12 Oct 2022 17:57:53 -0400 Subject: [PATCH] [Controls] Fix Initialization Race Condition (#143220) * Fix Initialization Race Condition --- .../embeddable/control_group_container.tsx | 27 ++++++++++++++++-- .../embeddable/dashboard_container.tsx | 17 +++++------ .../dashboard_container_factory.tsx | 28 +++++++++++++------ .../viewport/dashboard_viewport.tsx | 9 +----- .../dashboard_to_dashboard_drilldown.ts | 4 +-- 5 files changed, 53 insertions(+), 32 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..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 52e76295eb998..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() { @@ -164,9 +159,7 @@ export class DashboardViewport extends React.Component )} - {this.state.controlGroupReady && ( - - )} + ); 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');