From 44f1b6866c338678117b88424581242978400f56 Mon Sep 17 00:00:00 2001
From: Alex Dima <alexdima@microsoft.com>
Date: Tue, 24 Jul 2018 11:00:10 +0200
Subject: [PATCH] Fixes #54773: Check model version id right before applying
 the edits

---
 .../electron-browser/bulkEditService.ts       | 26 +++++++++++
 .../api/mainThreadEditors.test.ts             | 44 ++++++++++++++++++-
 2 files changed, 69 insertions(+), 1 deletion(-)

diff --git a/src/vs/workbench/services/bulkEdit/electron-browser/bulkEditService.ts b/src/vs/workbench/services/bulkEdit/electron-browser/bulkEditService.ts
index a0a69bf0c88..dd3d391644c 100644
--- a/src/vs/workbench/services/bulkEdit/electron-browser/bulkEditService.ts
+++ b/src/vs/workbench/services/bulkEdit/electron-browser/bulkEditService.ts
@@ -47,11 +47,14 @@ abstract class Recording {
 	abstract hasChanged(resource: URI): boolean;
 }
 
+type ValidationResult = { canApply: true } | { canApply: false, reason: URI };
+
 class ModelEditTask implements IDisposable {
 
 	private readonly _model: ITextModel;
 
 	protected _edits: IIdentifiedSingleEditOperation[];
+	private _expectedModelVersionId: number | undefined;
 	protected _newEol: EndOfLineSequence;
 
 	constructor(private readonly _modelReference: IReference<ITextEditorModel>) {
@@ -64,6 +67,7 @@ class ModelEditTask implements IDisposable {
 	}
 
 	addEdit(resourceEdit: ResourceTextEdit): void {
+		this._expectedModelVersionId = resourceEdit.modelVersionId;
 		for (const edit of resourceEdit.edits) {
 			if (typeof edit.eol === 'number') {
 				// honor eol-change
@@ -82,6 +86,13 @@ class ModelEditTask implements IDisposable {
 		}
 	}
 
+	validate(): ValidationResult {
+		if (typeof this._expectedModelVersionId === 'undefined' || this._model.getVersionId() === this._expectedModelVersionId) {
+			return { canApply: true };
+		}
+		return { canApply: false, reason: this._model.uri };
+	}
+
 	apply(): void {
 		if (this._edits.length > 0) {
 			this._edits = mergeSort(this._edits, (a, b) => Range.compareRangesUsingStarts(a.range, b.range));
@@ -191,6 +202,16 @@ class BulkEditModel implements IDisposable {
 		return this;
 	}
 
+	validate(): ValidationResult {
+		for (const task of this._tasks) {
+			const result = task.validate();
+			if (!result.canApply) {
+				return result;
+			}
+		}
+		return { canApply: true };
+	}
+
 	apply(): void {
 		for (const task of this._tasks) {
 			task.apply();
@@ -330,6 +351,11 @@ export class BulkEdit {
 			throw new Error(localize('conflict', "These files have changed in the meantime: {0}", conflicts.join(', ')));
 		}
 
+		const validationResult = model.validate();
+		if (validationResult.canApply === false) {
+			throw new Error(`${validationResult.reason.toString()} has changed in the meantime`);
+		}
+
 		await model.apply();
 		model.dispose();
 	}
diff --git a/src/vs/workbench/test/electron-browser/api/mainThreadEditors.test.ts b/src/vs/workbench/test/electron-browser/api/mainThreadEditors.test.ts
index 39d428c8f1d..fdb4b5575c3 100644
--- a/src/vs/workbench/test/electron-browser/api/mainThreadEditors.test.ts
+++ b/src/vs/workbench/test/electron-browser/api/mainThreadEditors.test.ts
@@ -26,6 +26,8 @@ import { TPromise } from 'vs/base/common/winjs.base';
 import { ResourceTextEdit } from 'vs/editor/common/modes';
 import { BulkEditService } from 'vs/workbench/services/bulkEdit/electron-browser/bulkEditService';
 import { NullLogService } from 'vs/platform/log/common/log';
+import { ITextModelService, ITextEditorModel } from 'vs/editor/common/services/resolverService';
+import { IReference, ImmortalReference } from 'vs/base/common/lifecycle';
 
 suite('MainThreadEditors', () => {
 
@@ -71,8 +73,16 @@ suite('MainThreadEditors', () => {
 		};
 		const workbenchEditorService = new TestEditorService();
 		const editorGroupService = new TestEditorGroupsService();
+		const textModelService = new class extends mock<ITextModelService>() {
+			createModelReference(resource: URI): TPromise<IReference<ITextEditorModel>> {
+				const textEditorModel: ITextEditorModel = new class extends mock<ITextEditorModel>() {
+					textEditorModel = modelService.getModel(resource);
+				};
+				return TPromise.as(new ImmortalReference(textEditorModel));
+			}
+		};
 
-		const bulkEditService = new BulkEditService(new NullLogService(), modelService, new TestEditorService(), null, new TestFileService(), textFileService, TestEnvironmentService, new TestContextService());
+		const bulkEditService = new BulkEditService(new NullLogService(), modelService, new TestEditorService(), textModelService, new TestFileService(), textFileService, TestEnvironmentService, new TestContextService());
 
 		const rpcProtocol = new TestRPCProtocol();
 		rpcProtocol.set(ExtHostContext.ExtHostDocuments, new class extends mock<ExtHostDocumentsShape>() {
@@ -129,6 +139,38 @@ suite('MainThreadEditors', () => {
 		});
 	});
 
+	test(`issue #54773: applyWorkspaceEdit checks model version in race situation`, () => {
+
+		let model = modelService.createModel('something', null, resource);
+
+		let workspaceResourceEdit1: ResourceTextEdit = {
+			resource: resource,
+			modelVersionId: model.getVersionId(),
+			edits: [{
+				text: 'asdfg',
+				range: new Range(1, 1, 1, 1)
+			}]
+		};
+		let workspaceResourceEdit2: ResourceTextEdit = {
+			resource: resource,
+			modelVersionId: model.getVersionId(),
+			edits: [{
+				text: 'asdfg',
+				range: new Range(1, 1, 1, 1)
+			}]
+		};
+
+		let p1 = editors.$tryApplyWorkspaceEdit({ edits: [workspaceResourceEdit1] }).then((result) => {
+			// first edit request succeeds
+			assert.equal(result, true);
+		});
+		let p2 = editors.$tryApplyWorkspaceEdit({ edits: [workspaceResourceEdit2] }).then((result) => {
+			// second edit request fails
+			assert.equal(result, false);
+		});
+		return TPromise.join([p1, p2]);
+	});
+
 	test(`applyWorkspaceEdit with only resource edit`, () => {
 		return editors.$tryApplyWorkspaceEdit({
 			edits: [
-- 
GitLab