From 0113b40fd1dab5f71eebbcee5e25f55b5c779059 Mon Sep 17 00:00:00 2001 From: Benjamin Pasero Date: Tue, 19 Jun 2018 19:04:11 +0200 Subject: [PATCH] fix #52212 --- .../src/singlefolder-tests/workspace.test.ts | 18 ++-- .../workbench/electron-browser/workbench.ts | 8 +- .../parts/backup/common/backupModelTracker.ts | 3 - .../parts/backup/common/backupRestorer.ts | 8 +- .../services/backup/common/backup.ts | 5 - .../services/backup/node/backupFileService.ts | 94 +++++++++++++------ .../workspace/node/workspaceEditingService.ts | 5 +- .../workbench/test/workbenchTestServices.ts | 2 - 8 files changed, 83 insertions(+), 60 deletions(-) diff --git a/extensions/vscode-api-tests/src/singlefolder-tests/workspace.test.ts b/extensions/vscode-api-tests/src/singlefolder-tests/workspace.test.ts index ddf664ff163..407f8dbf2e5 100644 --- a/extensions/vscode-api-tests/src/singlefolder-tests/workspace.test.ts +++ b/extensions/vscode-api-tests/src/singlefolder-tests/workspace.test.ts @@ -564,9 +564,8 @@ suite('workspace-namespace', () => { let success = await vscode.workspace.applyEdit(edit); assert.equal(success, true); - // todo@ben https://github.com/Microsoft/vscode/issues/52212 - // let doc = await vscode.workspace.openTextDocument(newUri); - // assert.equal(doc.getText(), 'AFTERBEFORE'); + let doc = await vscode.workspace.openTextDocument(newUri); + assert.equal(doc.getText(), 'AFTERBEFORE'); }); function nameWithUnderscore(uri: vscode.Uri) { @@ -584,9 +583,8 @@ suite('workspace-namespace', () => { we.insert(newUri, new vscode.Position(0, 0), 'Bar'); assert.ok(await vscode.workspace.applyEdit(we)); - // todo@ben https://github.com/Microsoft/vscode/issues/52212 - // let doc = await vscode.workspace.openTextDocument(newUri); - // assert.equal(doc.getText(), 'BarHelloFoo'); + let doc = await vscode.workspace.openTextDocument(newUri); + assert.equal(doc.getText(), 'BarHelloFoo'); }); test('WorkspaceEdit: Problem recreating a renamed resource #42634', async function () { @@ -603,9 +601,8 @@ suite('workspace-namespace', () => { assert.ok(await vscode.workspace.applyEdit(we)); - // todo@ben https://github.com/Microsoft/vscode/issues/52212 - // let newDoc = await vscode.workspace.openTextDocument(newUri); - // assert.equal(newDoc.getText(), 'HelloFoo'); + let newDoc = await vscode.workspace.openTextDocument(newUri); + assert.equal(newDoc.getText(), 'HelloFoo'); // let doc = await vscode.workspace.openTextDocument(docUri); // assert.equal(doc.getText(), 'Bar'); }); @@ -653,8 +650,7 @@ suite('workspace-namespace', () => { let doc = await vscode.workspace.openTextDocument(newUri); assert.ok(doc); - // todo@ben https://github.com/Microsoft/vscode/issues/52212 - // assert.equal(doc.getText(), 'Hello'); + assert.equal(doc.getText(), 'Hello'); }); test('WorkspaceEdit: rename resource followed by edit does not work #42638', async function () { diff --git a/src/vs/workbench/electron-browser/workbench.ts b/src/vs/workbench/electron-browser/workbench.ts index 1dd8ea0391a..b53a4c8f1d0 100644 --- a/src/vs/workbench/electron-browser/workbench.ts +++ b/src/vs/workbench/electron-browser/workbench.ts @@ -17,7 +17,7 @@ import { RunOnceScheduler } from 'vs/base/common/async'; import * as browser from 'vs/base/browser/browser'; import * as perf from 'vs/base/common/performance'; import * as errors from 'vs/base/common/errors'; -import { BackupFileService } from 'vs/workbench/services/backup/node/backupFileService'; +import { BackupFileService, InMemoryBackupFileService } from 'vs/workbench/services/backup/node/backupFileService'; import { IBackupFileService } from 'vs/workbench/services/backup/common/backup'; import { Registry } from 'vs/platform/registry/common/platform'; import { isWindows, isLinux, isMacintosh } from 'vs/base/common/platform'; @@ -415,7 +415,11 @@ export class Workbench extends Disposable implements IPartService { this.menubarPart = this.instantiationService.createInstance(MenubarPart, Identifiers.MENUBAR_PART); // Backup File Service - this.backupFileService = this.instantiationService.createInstance(BackupFileService, this.workbenchParams.configuration.backupPath); + if (this.workbenchParams.configuration.backupPath) { + this.backupFileService = this.instantiationService.createInstance(BackupFileService, this.workbenchParams.configuration.backupPath); + } else { + this.backupFileService = new InMemoryBackupFileService(); + } serviceCollection.set(IBackupFileService, this.backupFileService); // Text File Service diff --git a/src/vs/workbench/parts/backup/common/backupModelTracker.ts b/src/vs/workbench/parts/backup/common/backupModelTracker.ts index 4cb9b8c7670..d4b296540d1 100644 --- a/src/vs/workbench/parts/backup/common/backupModelTracker.ts +++ b/src/vs/workbench/parts/backup/common/backupModelTracker.ts @@ -36,9 +36,6 @@ export class BackupModelTracker implements IWorkbenchContribution { } private registerListeners() { - if (!this.backupFileService.backupEnabled) { - return; - } // Listen for text file model changes this.toDispose.push(this.textFileService.models.onModelContentChanged((e) => this.onTextFileModelChanged(e))); diff --git a/src/vs/workbench/parts/backup/common/backupRestorer.ts b/src/vs/workbench/parts/backup/common/backupRestorer.ts index c9fba97e14b..70b860ad8de 100644 --- a/src/vs/workbench/parts/backup/common/backupRestorer.ts +++ b/src/vs/workbench/parts/backup/common/backupRestorer.ts @@ -35,11 +35,9 @@ export class BackupRestorer implements IWorkbenchContribution { } private restoreBackups(): void { - if (this.backupFileService.backupEnabled) { - this.lifecycleService.when(LifecyclePhase.Running).then(() => { - this.doRestoreBackups().done(null, errors.onUnexpectedError); - }); - } + this.lifecycleService.when(LifecyclePhase.Running).then(() => { + this.doRestoreBackups().done(null, errors.onUnexpectedError); + }); } private doRestoreBackups(): TPromise { diff --git a/src/vs/workbench/services/backup/common/backup.ts b/src/vs/workbench/services/backup/common/backup.ts index 86d5326107e..be76ff6c428 100644 --- a/src/vs/workbench/services/backup/common/backup.ts +++ b/src/vs/workbench/services/backup/common/backup.ts @@ -22,11 +22,6 @@ export const BACKUP_FILE_UPDATE_OPTIONS: IUpdateContentOptions = { encoding: 'ut export interface IBackupFileService { _serviceBrand: any; - /** - * If backups are enabled. - */ - backupEnabled: boolean; - /** * Finds out if there are any backups stored. */ diff --git a/src/vs/workbench/services/backup/node/backupFileService.ts b/src/vs/workbench/services/backup/node/backupFileService.ts index fdeea689752..8b2c585dc49 100644 --- a/src/vs/workbench/services/backup/node/backupFileService.ts +++ b/src/vs/workbench/services/backup/node/backupFileService.ts @@ -15,7 +15,8 @@ import { IFileService, ITextSnapshot } from 'vs/platform/files/common/files'; import { TPromise } from 'vs/base/common/winjs.base'; import { readToMatchingString } from 'vs/base/node/stream'; import { ITextBufferFactory } from 'vs/editor/common/model'; -import { createTextBufferFactoryFromStream } from 'vs/editor/common/model/textModel'; +import { createTextBufferFactoryFromStream, createTextBufferFactoryFromSnapshot } from 'vs/editor/common/model/textModel'; +import { keys } from 'vs/base/common/map'; export interface IBackupFilesModel { resolve(backupRoot: string): TPromise; @@ -135,17 +136,9 @@ export class BackupFileService implements IBackupFileService { this.ready = this.init(); } - public get backupEnabled(): boolean { - return !!this.backupWorkspacePath; // Hot exit requires a backup path - } - private init(): TPromise { const model = new BackupFilesModel(); - if (!this.backupEnabled) { - return TPromise.as(model); - } - return model.resolve(this.backupWorkspacePath); } @@ -157,12 +150,9 @@ export class BackupFileService implements IBackupFileService { public loadBackupResource(resource: Uri): TPromise { return this.ready.then(model => { - const backupResource = this.toBackupResource(resource); - if (!backupResource) { - return void 0; - } // Return directly if we have a known backup with that resource + const backupResource = this.toBackupResource(resource); if (model.has(backupResource)) { return backupResource; } @@ -178,10 +168,6 @@ export class BackupFileService implements IBackupFileService { return this.ready.then(model => { const backupResource = this.toBackupResource(resource); - if (!backupResource) { - return void 0; - } - if (model.has(backupResource, versionId)) { return void 0; // return early if backup version id matches requested one } @@ -198,9 +184,6 @@ export class BackupFileService implements IBackupFileService { public discardResourceBackup(resource: Uri): TPromise { return this.ready.then(model => { const backupResource = this.toBackupResource(resource); - if (!backupResource) { - return void 0; - } return this.ioOperationQueues.queueFor(backupResource).queue(() => { return pfs.del(backupResource.fsPath).then(() => model.remove(backupResource)); @@ -212,10 +195,6 @@ export class BackupFileService implements IBackupFileService { this.isShuttingDown = true; return this.ready.then(model => { - if (!this.backupEnabled) { - return void 0; - } - return pfs.del(this.backupWorkspacePath).then(() => model.clear()); }); } @@ -226,8 +205,7 @@ export class BackupFileService implements IBackupFileService { model.get().forEach(fileBackup => { readPromises.push( - readToMatchingString(fileBackup.fsPath, BackupFileService.META_MARKER, 2000, 10000) - .then(Uri.parse) + readToMatchingString(fileBackup.fsPath, BackupFileService.META_MARKER, 2000, 10000).then(Uri.parse) ); }); @@ -259,10 +237,6 @@ export class BackupFileService implements IBackupFileService { } public toBackupResource(resource: Uri): Uri { - if (!this.backupEnabled) { - return null; - } - return Uri.file(path.join(this.backupWorkspacePath, resource.scheme, this.hashPath(resource))); } @@ -270,3 +244,63 @@ export class BackupFileService implements IBackupFileService { return crypto.createHash('md5').update(resource.fsPath).digest('hex'); } } + +export class InMemoryBackupFileService implements IBackupFileService { + + public _serviceBrand: any; + + private backups: Map = new Map(); + + hasBackups(): TPromise { + return TPromise.as(this.backups.size > 0); + } + + loadBackupResource(resource: Uri): TPromise { + const backupResource = this.toBackupResource(resource); + if (this.backups.has(backupResource.toString())) { + return TPromise.as(backupResource); + } + + return TPromise.as(void 0); + } + + backupResource(resource: Uri, content: ITextSnapshot, versionId?: number): TPromise { + const backupResource = this.toBackupResource(resource); + this.backups.set(backupResource.toString(), content); + + return TPromise.as(void 0); + } + + resolveBackupContent(backupResource: Uri): TPromise { + const snapshot = this.backups.get(backupResource.toString()); + if (snapshot) { + return TPromise.as(createTextBufferFactoryFromSnapshot(snapshot)); + } + + return TPromise.as(void 0); + } + + getWorkspaceFileBackups(): TPromise { + return TPromise.as(keys(this.backups).map(key => Uri.parse(key))); + } + + discardResourceBackup(resource: Uri): TPromise { + this.backups.delete(this.toBackupResource(resource).toString()); + + return TPromise.as(void 0); + } + + discardAllWorkspaceBackups(): TPromise { + this.backups.clear(); + + return TPromise.as(void 0); + } + + toBackupResource(resource: Uri): Uri { + return Uri.file(path.join(resource.scheme, this.hashPath(resource))); + } + + private hashPath(resource: Uri): string { + return crypto.createHash('md5').update(resource.fsPath).digest('hex'); + } +} \ No newline at end of file diff --git a/src/vs/workbench/services/workspace/node/workspaceEditingService.ts b/src/vs/workbench/services/workspace/node/workspaceEditingService.ts index 5e8f0fb6ca9..b9c500f1c5a 100644 --- a/src/vs/workbench/services/workspace/node/workspaceEditingService.ts +++ b/src/vs/workbench/services/workspace/node/workspaceEditingService.ts @@ -202,8 +202,9 @@ export class WorkspaceEditingService implements IWorkspaceEditingService { // TODO@Ben TODO@Sandeep the following requires ugly casts and should probably have a service interface // Reinitialize backup service - const backupFileService = this.backupFileService as BackupFileService; - backupFileService.initialize(result.backupPath); + if (this.backupFileService instanceof BackupFileService) { + this.backupFileService.initialize(result.backupPath); + } // Reinitialize configuration service const workspaceImpl = this.contextService as WorkspaceService; diff --git a/src/vs/workbench/test/workbenchTestServices.ts b/src/vs/workbench/test/workbenchTestServices.ts index 461dcf4daf8..7348c180e15 100644 --- a/src/vs/workbench/test/workbenchTestServices.ts +++ b/src/vs/workbench/test/workbenchTestServices.ts @@ -871,8 +871,6 @@ export class TestFileService implements IFileService { export class TestBackupFileService implements IBackupFileService { public _serviceBrand: any; - public backupEnabled: boolean; - public hasBackups(): TPromise { return TPromise.as(false); } -- GitLab