From f86d02e753d2fca53905402e87b09b2f43340855 Mon Sep 17 00:00:00 2001 From: Alan Ren Date: Thu, 25 Aug 2022 08:10:23 -0700 Subject: [PATCH] handle edit and save race condition (#20462) * handle edit and save race condition * handle more race condition scenarios * fix error --- .../workbench/browser/designer/designer.ts | 17 ++- .../workbench/browser/designer/interfaces.ts | 5 + .../browser/designer/tableActions.ts | 14 ++- .../browser/tableDesignerComponentInput.ts | 117 ++++++++++++------ 4 files changed, 110 insertions(+), 43 deletions(-) diff --git a/src/sql/workbench/browser/designer/designer.ts b/src/sql/workbench/browser/designer/designer.ts index b4714f5179..7d8173d8f3 100644 --- a/src/sql/workbench/browser/designer/designer.ts +++ b/src/sql/workbench/browser/designer/designer.ts @@ -51,6 +51,8 @@ import { RowMoveManager, RowMoveOnDragEventData } from 'sql/base/browser/ui/tabl import { ITaskbarContent, Taskbar } from 'sql/base/browser/ui/taskbar/taskbar'; import { RowSelectionModel } from 'sql/base/browser/ui/table/plugins/rowSelectionModel.plugin'; import { listFocusAndSelectionBackground } from 'sql/platform/theme/common/colors'; +import { timeout } from 'vs/base/common/async'; +import { onUnexpectedError } from 'vs/base/common/errors'; export interface IDesignerStyle { tabbedPanelStyles?: ITabbedPanelStyles; @@ -282,6 +284,9 @@ export class Designer extends Disposable implements IThemable { public setInput(input: DesignerComponentInput): void { + if (this._input) { + void this.submitPendingChanges().catch(onUnexpectedError); + } this.saveUIState(); if (this._loadingTimeoutHandle) { this.stopLoading(); @@ -303,7 +308,9 @@ export class Designer extends Disposable implements IThemable { this._inputDisposable.add(this._input.onRefreshRequested(() => { this.refresh(); })); - + this._inputDisposable.add(this._input.onSubmitPendingEditRequested(async () => { + await this.submitPendingChanges(); + })); if (this._input.view === undefined) { this._input.initialize(); } else { @@ -319,6 +326,14 @@ export class Designer extends Disposable implements IThemable { this._inputDisposable?.dispose(); } + public async submitPendingChanges(): Promise { + if (this._container.contains(document.activeElement) && document.activeElement instanceof HTMLInputElement) { + // Force the elements to fire the blur event to submit the pending changes. + document.activeElement.blur(); + return timeout(10); + } + } + private clearUI(): void { this._componentMap.forEach(item => item.component.dispose()); this._componentMap.clear(); diff --git a/src/sql/workbench/browser/designer/interfaces.ts b/src/sql/workbench/browser/designer/interfaces.ts index 6c1526a67a..7c58af3579 100644 --- a/src/sql/workbench/browser/designer/interfaces.ts +++ b/src/sql/workbench/browser/designer/interfaces.ts @@ -28,6 +28,11 @@ export interface DesignerComponentInput { */ readonly onRefreshRequested: Event; + /** + * The event that is triggerd when force submit of the pending edit is requested. + */ + readonly onSubmitPendingEditRequested: Event; + /** * Gets the object type display name. */ diff --git a/src/sql/workbench/browser/designer/tableActions.ts b/src/sql/workbench/browser/designer/tableActions.ts index fa9b5efb88..1b78286150 100644 --- a/src/sql/workbench/browser/designer/tableActions.ts +++ b/src/sql/workbench/browser/designer/tableActions.ts @@ -21,6 +21,7 @@ export class DesignerTableAction extends Action { protected _table: Table; constructor( + private _designer: Designer, id: string, label: string, icon: string, @@ -43,6 +44,10 @@ export class DesignerTableAction extends Action { } } } + + public override async run(context: DesignerTableActionContext): Promise { + await this._designer.submitPendingChanges(); + } } export class AddRowAction extends DesignerTableAction { @@ -54,12 +59,13 @@ export class AddRowAction extends DesignerTableAction { private designer: Designer, tableProperties: DesignerTableProperties, ) { - super(AddRowAction.ID, tableProperties.labelForAddNewButton || AddRowAction.LABEL, AddRowAction.ICON, false); + super(designer, AddRowAction.ID, tableProperties.labelForAddNewButton || AddRowAction.LABEL, AddRowAction.ICON, false); this.designer = designer; this._tooltip = localize('designer.newRowButtonAriaLabel', "Add new row to '{0}' table", tableProperties.ariaLabel); } public override async run(context: DesignerTableActionContext): Promise { + await super.run(context); const lastIndex = context.table.getData().getItems().length; return new Promise((resolve) => { this.designer.handleEdit({ @@ -78,13 +84,14 @@ export class MoveRowUpAction extends DesignerTableAction { public static LABEL = localize('designer.moveRowUpAction', 'Move Up'); constructor(private designer: Designer) { - super(MoveRowUpAction.ID, MoveRowUpAction.LABEL, MoveRowUpAction.ICON, true); + super(designer, MoveRowUpAction.ID, MoveRowUpAction.LABEL, MoveRowUpAction.ICON, true); this.designer = designer; this._tooltip = localize('designer.moveRowUpButtonAriaLabel', "Move selected row up one position"); this.enabled = false; } public override async run(context: DesignerTableActionContext): Promise { + await super.run(context); let rowIndex = context.selectedRow ?? context.table.getSelectedRows()[0]; if (rowIndex - 1 < 0) { return; @@ -116,13 +123,14 @@ export class MoveRowDownAction extends DesignerTableAction { public static LABEL = localize('designer.moveRowDownAction', 'Move Down'); constructor(private designer: Designer) { - super(MoveRowDownAction.ID, MoveRowDownAction.LABEL, MoveRowDownAction.ICON, true); + super(designer, MoveRowDownAction.ID, MoveRowDownAction.LABEL, MoveRowDownAction.ICON, true); this.designer = designer; this._tooltip = localize('designer.moveRowDownButtonAriaLabel', "Move selected row down one position"); this.enabled = false; } public override async run(context: DesignerTableActionContext): Promise { + await super.run(context); let rowIndex = context.selectedRow ?? context.table.getSelectedRows()[0]; const tableData = context.table.getData().getItems(); if (rowIndex + 1 >= tableData.length) { diff --git a/src/sql/workbench/services/tableDesigner/browser/tableDesignerComponentInput.ts b/src/sql/workbench/services/tableDesigner/browser/tableDesignerComponentInput.ts index 24923f9e6f..ccdba64600 100644 --- a/src/sql/workbench/services/tableDesigner/browser/tableDesignerComponentInput.ts +++ b/src/sql/workbench/services/tableDesigner/browser/tableDesignerComponentInput.ts @@ -4,7 +4,7 @@ *--------------------------------------------------------------------------------------------*/ import * as azdata from 'azdata'; -import { DesignerViewModel, DesignerEdit, DesignerComponentInput, DesignerView, DesignerTab, DesignerDataPropertyInfo, DropDownProperties, DesignerTableProperties, DesignerEditProcessedEventArgs, DesignerAction, DesignerStateChangedEventArgs, DesignerPropertyPath, DesignerIssue, ScriptProperty } from 'sql/workbench/browser/designer/interfaces'; +import { DesignerViewModel, DesignerEdit, DesignerComponentInput, DesignerView, DesignerTab, DesignerDataPropertyInfo, DropDownProperties, DesignerTableProperties, DesignerEditProcessedEventArgs, DesignerAction, DesignerStateChangedEventArgs, DesignerPropertyPath, DesignerIssue, ScriptProperty, DesignerUIState } from 'sql/workbench/browser/designer/interfaces'; import { TableDesignerProvider } from 'sql/workbench/services/tableDesigner/common/interface'; import { localize } from 'vs/nls'; import { designers } from 'sql/workbench/api/common/sqlExtHostTypes'; @@ -18,6 +18,7 @@ import { IAdsTelemetryService, ITelemetryEventProperties } from 'sql/platform/te import { TelemetryAction, TelemetryView } from 'sql/platform/telemetry/common/telemetryKeys'; import { IErrorMessageService } from 'sql/platform/errorMessage/common/errorMessageService'; import { TableDesignerMetadata } from 'sql/workbench/services/tableDesigner/browser/tableDesignerMetadata'; +import { Queue, timeout } from 'vs/base/common/async'; const ErrorDialogTitle: string = localize('tableDesigner.ErrorDialogTitle', "Table Designer Error"); export class TableDesignerComponentInput implements DesignerComponentInput { @@ -32,13 +33,20 @@ export class TableDesignerComponentInput implements DesignerComponentInput { private _onInitialized = new Emitter(); private _onEditProcessed = new Emitter(); private _onRefreshRequested = new Emitter(); + private _onSubmitPendingEditRequested = new Emitter(); private _originalViewModel: DesignerViewModel; private _tableDesignerView: azdata.designers.TableDesignerView; + private _activeEditPromise: Promise; + private _isEditInProgress: boolean = false; + private _recentEditAccepted: boolean = true; + private _editQueue: Queue = new Queue(); public readonly onInitialized: Event = this._onInitialized.event; public readonly onEditProcessed: Event = this._onEditProcessed.event; public readonly onStateChange: Event = this._onStateChange.event; public readonly onRefreshRequested: Event = this._onRefreshRequested.event; + public readonly onSubmitPendingEditRequested: Event = this._onSubmitPendingEditRequested.event; + private readonly designerEditTypeDisplayValue: { [key: number]: string } = { 0: 'Add', 1: 'Remove', 2: 'Update' @@ -54,6 +62,8 @@ export class TableDesignerComponentInput implements DesignerComponentInput { @IErrorMessageService private readonly _errorMessageService: IErrorMessageService) { } + public designerUIState?: DesignerUIState = undefined; + get valid(): boolean { return this._valid; } @@ -87,44 +97,11 @@ export class TableDesignerComponentInput implements DesignerComponentInput { } processEdit(edit: DesignerEdit): void { - const telemetryInfo = this.createTelemetryInfo(); - telemetryInfo.tableObjectType = this.getObjectTypeFromPath(edit.path); - const editAction = this._adsTelemetryService.createActionEvent(TelemetryView.TableDesigner, - this.designerEditTypeDisplayValue[edit.type]).withAdditionalProperties(telemetryInfo); - const startTime = new Date().getTime(); - this.updateState(this.valid, this.dirty, 'processEdit'); - this._provider.processTableEdit(this.tableInfo, edit).then( - result => { - if (result.inputValidationError) { - this._errorMessageService.showDialog(Severity.Error, ErrorDialogTitle, localize('tableDesigner.inputValidationError', "The input validation failed with error: {0}", result.inputValidationError)); - } - this._viewModel = result.viewModel; - if (result.view) { - this.setDesignerView(result.view); - } - this._issues = result.issues; - this.updateState(result.isValid, this.isDirty(), undefined); - - this._onEditProcessed.fire({ - edit: edit, - result: { - isValid: result.isValid, - issues: result.issues, - refreshView: !!result.view - } - }); - const metadataTelemetryInfo = TableDesignerMetadata.getTelemetryInfo(this._provider.providerId, result.metadata); - editAction.withAdditionalMeasurements({ - 'elapsedTimeMs': new Date().getTime() - startTime - }).withAdditionalProperties(metadataTelemetryInfo).send(); - }, - error => { - this._errorMessageService.showDialog(Severity.Error, ErrorDialogTitle, localize('tableDesigner.errorProcessingEdit', "An error occured while processing the change: {0}", error?.message ?? error), error?.data); - this.updateState(this.valid, this.dirty); - this._adsTelemetryService.createErrorEvent(TelemetryView.TableDesigner, - this.designerEditTypeDisplayValue[edit.type]).withAdditionalProperties(telemetryInfo).send(); - } - ); + // If there is already an edit being processed, the new edit will be skipped if the previous edit is not accepted. + const checkPreviousEditResult = this._editQueue.size !== 0; + this._editQueue.queue(async () => { + await this.doProcessEdit(edit, checkPreviousEditResult); + }); } async generateScript(): Promise { @@ -183,6 +160,14 @@ export class TableDesignerComponentInput implements DesignerComponentInput { } async save(): Promise { + this._onSubmitPendingEditRequested.fire(); + await timeout(10); + if (this._isEditInProgress) { + await this._activeEditPromise; + } + if (!this.valid || !this._recentEditAccepted) { + return; + } if (!this.isDirty()) { return; } @@ -235,6 +220,60 @@ export class TableDesignerComponentInput implements DesignerComponentInput { this.updateState(true, false); } + private async doProcessEdit(edit: DesignerEdit, checkPreviousEditResult: boolean): Promise { + if (checkPreviousEditResult && !this._recentEditAccepted) { + return; + } + const telemetryInfo = this.createTelemetryInfo(); + telemetryInfo.tableObjectType = this.getObjectTypeFromPath(edit.path); + const editAction = this._adsTelemetryService.createActionEvent(TelemetryView.TableDesigner, + this.designerEditTypeDisplayValue[edit.type]).withAdditionalProperties(telemetryInfo); + const startTime = new Date().getTime(); + this.updateState(this.valid, this.dirty, 'processEdit'); + this._activeEditPromise = new Promise(async (resolve) => { + this._isEditInProgress = true; + this._recentEditAccepted = true; + try { + const result = await this._provider.processTableEdit(this.tableInfo, edit); + if (result.inputValidationError) { + this._recentEditAccepted = false; + this._errorMessageService.showDialog(Severity.Error, ErrorDialogTitle, localize('tableDesigner.inputValidationError', "The input validation failed with error: {0}", result.inputValidationError)); + } + this._viewModel = result.viewModel; + if (result.view) { + this.setDesignerView(result.view); + } + this._issues = result.issues; + this.updateState(result.isValid, this.isDirty(), undefined); + + this._onEditProcessed.fire({ + edit: edit, + result: { + isValid: result.isValid, + issues: result.issues, + refreshView: !!result.view + } + }); + const metadataTelemetryInfo = TableDesignerMetadata.getTelemetryInfo(this._provider.providerId, result.metadata); + editAction.withAdditionalMeasurements({ + 'elapsedTimeMs': new Date().getTime() - startTime + }).withAdditionalProperties(metadataTelemetryInfo).send(); + } + catch (error) { + this._errorMessageService.showDialog(Severity.Error, ErrorDialogTitle, localize('tableDesigner.errorProcessingEdit', "An error occured while processing the change: {0}", error?.message ?? error), error?.data); + this.updateState(this.valid, this.dirty); + this._adsTelemetryService.createErrorEvent(TelemetryView.TableDesigner, + this.designerEditTypeDisplayValue[edit.type]).withAdditionalProperties(telemetryInfo).send(); + this._recentEditAccepted = false; + } + finally { + this._isEditInProgress = false; + resolve(); + } + }); + return this._activeEditPromise; + } + private updateState(valid: boolean, dirty: boolean, pendingAction?: DesignerAction): void { if (this._dirty !== dirty || this._valid !== valid || this._pendingAction !== pendingAction) { const previousState = {