From b83da2dfa839efaba1c7f1d7f0d7bc2b730dfc0e Mon Sep 17 00:00:00 2001 From: Charles Gagnon Date: Tue, 10 Nov 2020 17:00:16 -0800 Subject: [PATCH] Run initial modelview actions first (#13317) * Run initial modelview actions first * add param * Comments and typings * Catch promise error * fix db projects test * remove extra calls --- .../addDatabaseReferenceDialog.test.ts | 4 +- .../platform/dashboard/browser/interfaces.ts | 2 +- .../model/browser/modelViewService.ts | 6 +- .../browser/modelComponents/componentBase.ts | 2 +- .../browser/modelComponents/modelStore.ts | 60 ++++++++++++++----- .../browser/modelComponents/viewBase.ts | 38 ++++++------ 6 files changed, 70 insertions(+), 42 deletions(-) diff --git a/extensions/sql-database-projects/src/test/dialogs/addDatabaseReferenceDialog.test.ts b/extensions/sql-database-projects/src/test/dialogs/addDatabaseReferenceDialog.test.ts index 4e7cf495b5..e1554d6ce8 100644 --- a/extensions/sql-database-projects/src/test/dialogs/addDatabaseReferenceDialog.test.ts +++ b/extensions/sql-database-projects/src/test/dialogs/addDatabaseReferenceDialog.test.ts @@ -29,10 +29,8 @@ describe('Add Database Reference Dialog', () => { const dialog = new AddDatabaseReferenceDialog(project); await dialog.openDialog(); - should(dialog.dialog.okButton.enabled).equal(false); + should(dialog.dialog.okButton.enabled).equal(true, 'Ok button should be enabled since initial type of systemDb has default values filled'); should(dialog.currentReferenceType).equal(ReferenceType.systemDb); - dialog.tryEnableAddReferenceButton(); - should(dialog.dialog.okButton.enabled).equal(true, 'Ok button should be enabled because there is a default value in the database name textbox'); // empty db name textbox dialog.databaseNameTextbox!.value = ''; diff --git a/src/sql/platform/dashboard/browser/interfaces.ts b/src/sql/platform/dashboard/browser/interfaces.ts index c2a91d2385..5ed5a83068 100644 --- a/src/sql/platform/dashboard/browser/interfaces.ts +++ b/src/sql/platform/dashboard/browser/interfaces.ts @@ -69,7 +69,7 @@ export interface IModelStore { * @param componentId unique identifier of the component * @param action some action to perform */ - eventuallyRunOnComponent(componentId: string, action: (component: IComponent) => T): Promise; + eventuallyRunOnComponent(componentId: string, action: (component: IComponent) => T, initial: boolean): void; /** * Register a callback that will validate components when given a component ID */ diff --git a/src/sql/platform/model/browser/modelViewService.ts b/src/sql/platform/model/browser/modelViewService.ts index 73b3d95c42..5fb75a0430 100644 --- a/src/sql/platform/model/browser/modelViewService.ts +++ b/src/sql/platform/model/browser/modelViewService.ts @@ -35,12 +35,12 @@ export interface IModelView extends IView { clearContainer(componentId: string): void; addToContainer(containerId: string, item: IItemConfig, index?: number): void; removeFromContainer(containerId: string, item: IItemConfig): void; - setLayout(componentId: string, layout: any): void; + setLayout(componentId: string, layout: any, initial?: boolean): void; setItemLayout(componentId: string, item: IItemConfig): void; - setProperties(componentId: string, properties: { [key: string]: any }): void; + setProperties(componentId: string, properties: { [key: string]: any }, initial?: boolean): void; setDataProvider(handle: number, componentId: string, context: any): void; refreshDataProvider(componentId: string, item: any): void; - registerEvent(componentId: string): void; + registerEvent(componentId: string, initial?: boolean): void; onEvent: Event; validate(componentId: string): Thenable; readonly onDestroy: Event; diff --git a/src/sql/workbench/browser/modelComponents/componentBase.ts b/src/sql/workbench/browser/modelComponents/componentBase.ts index c89d4d68d8..0d180d4c3b 100644 --- a/src/sql/workbench/browser/modelComponents/componentBase.ts +++ b/src/sql/workbench/browser/modelComponents/componentBase.ts @@ -308,7 +308,7 @@ export abstract class ContainerBase = (component: IComponent) => T; + export class ModelStore implements IModelStore { private _descriptorMappings: { [x: string]: IComponentDescriptor } = {}; private _componentMappings: { [x: string]: IComponent } = {}; - private _componentActions: { [x: string]: Deferred } = {}; + private _componentActions: { [x: string]: { initial: ModelStoreAction[], actions: Deferred } } = {}; private _validationCallbacks: ((componentId: string) => Thenable)[] = []; constructor() { } @@ -50,12 +53,19 @@ export class ModelStore implements IModelStore { return this._componentMappings[componentId]; } - eventuallyRunOnComponent(componentId: string, action: (component: IComponent) => T): Promise { + /** + * Queues up an action to run once a component is created and registered. This will run immediately if the component is + * already registered. + * @param componentId The ID of the component to queue up the action for + * @param action The action to run when the component is registered + * @param initial Whether this is an initial setup action that should be done before other post-setup actions + */ + eventuallyRunOnComponent(componentId: string, action: (component: IComponent) => T, initial: boolean = false): void { let component = this.getComponent(componentId); if (component) { - return Promise.resolve(action(component)); + action(component); } else { - return this.addPendingAction(componentId, action); + this.addPendingAction(componentId, action, initial); } } @@ -68,24 +78,46 @@ export class ModelStore implements IModelStore { return Promise.all(this._validationCallbacks.map(callback => callback(componentId))).then(validations => validations.every(validation => validation === true)); } - private addPendingAction(componentId: string, action: (component: IComponent) => T): Promise { + /** + * Adds the specified action to the list of actions to run once the specified component is created and registered. + * @param componentId The ID of the component to add the action to + * @param action The action to run once the component is registered + * @param initial Whether this is an initial setup action that should be ran before other post-setup actions + */ + private addPendingAction(componentId: string, action: ModelStoreAction, initial: boolean): void { // We create a promise and chain it onto a tracking promise whose resolve method // will only be called once the component is created - let deferredPromise = this._componentActions[componentId]; - if (!deferredPromise) { - deferredPromise = new Deferred(); - this._componentActions[componentId] = deferredPromise; + let deferredActions = this._componentActions[componentId]; + if (!deferredActions) { + deferredActions = { initial: [], actions: new Deferred() }; + this._componentActions[componentId] = deferredActions; + } + if (initial) { + deferredActions.initial.push(action); + } else { + deferredActions.actions.promise.then((component) => { + return action(component); + }); } - let promise = deferredPromise.promise.then((component) => { - return action(component); - }); - return promise; } + /** + * Runs the set of pending actions for a given component. This will run the initial setup actions + * first and then run all the other actions afterwards. + * @param componentId The ID of the component to run the currently pending actions for + * @param component The component object to run the actions against + */ private runPendingActions(componentId: string, component: IComponent) { let promiseTracker = this._componentActions[componentId]; if (promiseTracker) { - promiseTracker.resolve(component); + // Run initial actions first to ensure they're done before later actions (and thus don't overwrite following actions) + new Promise(resolve => { + promiseTracker.initial.forEach(action => action(component)); + resolve(); + }).then(() => { + promiseTracker.actions.resolve(component); + }).catch(onUnexpectedError); + this._componentActions[componentId] = undefined; } } } diff --git a/src/sql/workbench/browser/modelComponents/viewBase.ts b/src/sql/workbench/browser/modelComponents/viewBase.ts index a676fbca04..281a289010 100644 --- a/src/sql/workbench/browser/modelComponents/viewBase.ts +++ b/src/sql/workbench/browser/modelComponents/viewBase.ts @@ -60,12 +60,12 @@ export abstract class ViewBase extends AngularDisposable implements IModelView { throw new Error(nls.localize('componentTypeNotRegistered', "Could not find component for type {0}", ModelComponentTypes[component.type])); } let descriptor = this.modelStore.createComponentDescriptor(typeId, component.id); - this.setProperties(component.id, component.properties); - this.setLayout(component.id, component.layout); - this.registerEvent(component.id); + this.setProperties(component.id, component.properties, true); + this.setLayout(component.id, component.layout, true); + this.registerEvent(component.id, true); if (component.itemConfigs) { for (let item of component.itemConfigs) { - this.addToContainer(component.id, item); + this.addToContainer(component.id, item, undefined, true); } } @@ -84,12 +84,12 @@ export abstract class ViewBase extends AngularDisposable implements IModelView { this.queueAction(componentId, (component) => component.clearContainer()); } - addToContainer(containerId: string, itemConfig: IItemConfig, index?: number): void { + addToContainer(containerId: string, itemConfig: IItemConfig, index?: number, initial: boolean = false): void { // Do not return the promise as this should be non-blocking this.queueAction(containerId, (component) => { let childDescriptor = this.defineComponent(itemConfig.componentShape); component.addToContainer(childDescriptor, itemConfig.config, index); - }); + }, initial); } removeFromContainer(containerId: string, itemConfig: IItemConfig): void { @@ -100,11 +100,11 @@ export abstract class ViewBase extends AngularDisposable implements IModelView { }); } - setLayout(componentId: string, layout: any): void { + setLayout(componentId: string, layout: any, initial: boolean = false): void { if (!layout) { return; } - this.queueAction(componentId, (component) => component.setLayout(layout)); + this.queueAction(componentId, (component) => component.setLayout(layout), initial); } setItemLayout(containerId: string, itemConfig: IItemConfig): void { @@ -114,24 +114,22 @@ export abstract class ViewBase extends AngularDisposable implements IModelView { }); } - setProperties(componentId: string, properties: { [key: string]: any; }): void { + setProperties(componentId: string, properties: { [key: string]: any; }, initial: boolean = false): void { if (!properties) { return; } - this.queueAction(componentId, (component) => component.setProperties(properties)); + this.queueAction(componentId, (component) => component.setProperties(properties), initial); } refreshDataProvider(componentId: string, item: any): void { this.queueAction(componentId, (component) => component.refreshDataProvider(item)); } - private queueAction(componentId: string, action: (component: IComponent) => T): void { - this.modelStore.eventuallyRunOnComponent(componentId, action).catch(err => { - // TODO add error handling - }); + private queueAction(componentId: string, action: (component: IComponent) => T, initial: boolean = false): void { + this.modelStore.eventuallyRunOnComponent(componentId, action, initial); } - registerEvent(componentId: string) { + registerEvent(componentId: string, initial: boolean = false) { this.queueAction(componentId, (component) => { this._register(component.registerEventHandler(e => { let modelViewEvent: IModelViewEventArgs = assign({ @@ -140,7 +138,7 @@ export abstract class ViewBase extends AngularDisposable implements IModelView { }, e); this._onEventEmitter.fire(modelViewEvent); })); - }); + }, initial); } public get onEvent(): Event { @@ -148,18 +146,18 @@ export abstract class ViewBase extends AngularDisposable implements IModelView { } public validate(componentId: string): Thenable { - return new Promise(resolve => this.modelStore.eventuallyRunOnComponent(componentId, component => resolve(component.validate()))); + return new Promise(resolve => this.modelStore.eventuallyRunOnComponent(componentId, component => resolve(component.validate()), false)); } public setDataProvider(handle: number, componentId: string, context: any): any { - return this.queueAction(componentId, (component) => component.setDataProvider(handle, componentId, context)); + return this.queueAction(componentId, (component) => component.setDataProvider(handle, componentId, context), false); } public focus(componentId: string): void { - return this.queueAction(componentId, (component) => component.focus()); + return this.queueAction(componentId, (component) => component.focus(), false); } public doAction(componentId: string, action: string, ...args: any[]): void { - return this.queueAction(componentId, (component) => component.doAction(action, ...args)); + return this.queueAction(componentId, (component) => component.doAction(action, ...args), false); } }