Microsoft/vscode
1
0
Fork 0
mirror of https://github.com/microsoft/vscode.git synced 2026-05-18 14:19:42 +01:00
Code Issues Packages Projects Releases Wiki Activity

backup - remove discardAllBackups() method

Browse Source 
This method is actually deleting the entire backup home
folder for the workspace and it is possible that this could
lead to race conditions when a backup is running at the same
time.
This commit is contained in:
Benjamin Pasero
2020-01-23 12:14:27 +01:00
parent 9bec0160f6
commit 5ec65bdc87
6 changed files with 10 additions and 81 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/vs/workbench/contrib/backup/electron-browser/backupTracker.ts
+4 -15
View File
@@ -182,15 +182,14 @@ export class NativeBackupTracker extends BackupTracker implements IWorkbenchCont
return true; // veto (save failed or was canceled)
}
return this.noVeto(dirtyCount === workingCopies.length ? true /* all */ : workingCopies); // no veto (dirty saved)
return this.noVeto(workingCopies); // no veto (dirty saved)
}
// Don't Save
else if (confirm === ConfirmResult.DONT_SAVE) {
const dirtyCount = this.workingCopyService.dirtyCount;
await this.doRevertAllBeforeShutdown(workingCopies);
return this.noVeto(dirtyCount === workingCopies.length ? true /* all */ : workingCopies); // no veto (dirty reverted)
return this.noVeto(workingCopies); // no veto (dirty reverted)
}
// Cancel
@@ -243,7 +242,7 @@ export class NativeBackupTracker extends BackupTracker implements IWorkbenchCont
}
}
private noVeto(backupsToDiscardOrAll: IWorkingCopy[] | boolean): boolean | Promise<boolean> {
private noVeto(backupsToDiscard: IWorkingCopy[]): boolean | Promise<boolean> {
if (this.lifecycleService.phase < LifecyclePhase.Restored) {
return false; // if editors have not restored, we are not up to speed with backups and thus should not discard them
}
@@ -252,16 +251,6 @@ export class NativeBackupTracker extends BackupTracker implements IWorkbenchCont
return false; // extension development does not track any backups
}
if (backupsToDiscardOrAll === true) {
// discard all backups
return this.backupFileService.discardAllBackups().then(() => false, () => false);
}
if (Array.isArray(backupsToDiscardOrAll)) {
// otherwise, discard individually
return Promise.all(backupsToDiscardOrAll.map(workingCopy => this.backupFileService.discardBackup(workingCopy.resource))).then(() => false, () => false);
}
return false;
return Promise.all(backupsToDiscard.map(workingCopy => this.backupFileService.discardBackup(workingCopy.resource))).then(() => false, () => false);
}
}
src/vs/workbench/contrib/backup/test/electron-browser/backupTracker.test.ts
+3 -3
View File
@@ -279,11 +279,11 @@ suite('BackupTracker', () => {
let veto = event.value;
if (typeof veto === 'boolean') {
assert.ok(accessor.backupFileService.didDiscardAllBackups);
assert.ok(accessor.backupFileService.discardedBackups.length > 0);
assert.ok(!veto);
} else {
veto = await veto;
assert.ok(accessor.backupFileService.didDiscardAllBackups);
assert.ok(accessor.backupFileService.discardedBackups.length > 0);
assert.ok(!veto);
}
@@ -452,7 +452,7 @@ suite('BackupTracker', () => {
accessor.lifecycleService.fireWillShutdown(event);
const veto = await (<Promise<boolean>>event.value);
assert.ok(!accessor.backupFileService.didDiscardAllBackups); // When hot exit is set, backups should never be cleaned since the confirm result is cancel
assert.equal(accessor.backupFileService.discardedBackups.length, 0); // When hot exit is set, backups should never be cleaned since the confirm result is cancel
assert.equal(veto, shouldVeto);
part.dispose();
src/vs/workbench/services/backup/common/backup.ts
-5
View File
@@ -68,9 +68,4 @@ export interface IBackupFileService {
* @param resource The resource whose backup is being discarded discard to back up.
*/
discardBackup(resource: URI): Promise<void>;
/**
* Discards all backups that exist.
*/
discardAllBackups(): Promise<void>;
}
src/vs/workbench/services/backup/common/backupFileService.ts
-23
View File
@@ -163,10 +163,6 @@ export class BackupFileService implements IBackupFileService {
return this.impl.discardBackup(resource);
}
discardAllBackups(): Promise<void> {
return this.impl.discardAllBackups();
}
getBackups(): Promise<URI[]> {
return this.impl.getBackups();
}
@@ -277,21 +273,6 @@ class BackupFileServiceImpl extends Disposable implements IBackupFileService {
});
}
async discardAllBackups(): Promise<void> {
// Discard each backup and clear model
// We go through the doDiscardBackup()
// method to benefit from the IO queue
const model = await this.ready;
await Promise.all(model.get().map(backupResource => this.doDiscardBackup(backupResource)));
model.clear();
// Delete the backup home for this workspace
// It will automatically be populated again
// once another backup is made
await this.deleteIgnoreFileNotFound(this.backupWorkspacePath);
}
private async deleteIgnoreFileNotFound(resource: URI): Promise<void> {
try {
await this.fileService.del(resource, { recursive: true });
@@ -443,10 +424,6 @@ export class InMemoryBackupFileService implements IBackupFileService {
this.backups.delete(this.toBackupResource(resource).toString());
}
async discardAllBackups(): Promise<void> {
this.backups.clear();
}
toBackupResource(resource: URI): URI {
return URI.file(join(resource.scheme, this.hashPath(resource)));
}
src/vs/workbench/services/backup/test/electron-browser/backupFileService.test.ts
+3 -31
View File
@@ -42,7 +42,6 @@ const barFile = URI.file(platform.isWindows ? 'c:\\Bar' : '/Bar');
const fooBarFile = URI.file(platform.isWindows ? 'c:\\Foo Bar' : '/Foo Bar');
const untitledFile = URI.from({ scheme: Schemas.untitled, path: 'Untitled-1' });
const fooBackupPath = path.join(workspaceBackupPath, 'file', hashPath(fooFile));
const barBackupPath = path.join(workspaceBackupPath, 'file', hashPath(barFile));
const untitledBackupPath = path.join(workspaceBackupPath, 'untitled', hashPath(untitledFile));
class TestBackupEnvironmentService extends NativeWorkbenchEnvironmentService {
@@ -58,6 +57,7 @@ export class NodeTestBackupFileService extends BackupFileService {
private backupResourceJoiners: Function[];
private discardBackupJoiners: Function[];
discardedBackups: URI[];
constructor(workspaceBackupPath: string) {
const environmentService = new TestBackupEnvironmentService(workspaceBackupPath);
@@ -71,7 +71,7 @@ export class NodeTestBackupFileService extends BackupFileService {
this.fileService = fileService;
this.backupResourceJoiners = [];
this.discardBackupJoiners = [];
this.didDiscardAllBackups = false;
this.discardedBackups = [];
}
joinBackupResource(): Promise<void> {
@@ -92,19 +92,12 @@ export class NodeTestBackupFileService extends BackupFileService {
async discardBackup(resource: URI): Promise<void> {
await super.discardBackup(resource);
this.discardedBackups.push(resource);
while (this.discardBackupJoiners.length) {
this.discardBackupJoiners.pop()!();
}
}
didDiscardAllBackups: boolean;
discardAllBackups(): Promise<void> {
this.didDiscardAllBackups = true;
return super.discardAllBackups();
}
}
suite('BackupFileService', () => {
@@ -282,27 +275,6 @@ suite('BackupFileService', () => {
});
});
suite('discardAllBackups', () => {
test('text file', async () => {
await service.backup(fooFile, createTextBufferFactory('test').create(DefaultEndOfLine.LF).createSnapshot(false));
assert.equal(fs.readdirSync(path.join(workspaceBackupPath, 'file')).length, 1);
await service.backup(barFile, createTextBufferFactory('test').create(DefaultEndOfLine.LF).createSnapshot(false));
assert.equal(fs.readdirSync(path.join(workspaceBackupPath, 'file')).length, 2);
await service.discardAllBackups();
assert.equal(fs.existsSync(fooBackupPath), false);
assert.equal(fs.existsSync(barBackupPath), false);
assert.equal(fs.existsSync(path.join(workspaceBackupPath, 'file')), false);
});
test('untitled file', async () => {
await service.backup(untitledFile, createTextBufferFactory('test').create(DefaultEndOfLine.LF).createSnapshot(false));
assert.equal(fs.readdirSync(path.join(workspaceBackupPath, 'untitled')).length, 1);
await service.discardAllBackups();
assert.equal(fs.existsSync(untitledBackupPath), false);
assert.equal(fs.existsSync(path.join(workspaceBackupPath, 'untitled')), false);
});
});
suite('getBackups', () => {
test('("file") - text file', async () => {
await service.backup(fooFile, createTextBufferFactory('test').create(DefaultEndOfLine.LF).createSnapshot(false));
src/vs/workbench/test/workbenchTestServices.ts
-4
View File
@@ -1209,10 +1209,6 @@ export class TestBackupFileService implements IBackupFileService {
discardBackup(_resource: URI): Promise<void> {
return Promise.resolve();
}
discardAllBackups(): Promise<void> {
return Promise.resolve();
}
}
export class TestCodeEditorService implements ICodeEditorService {