Skip to content

Commit 7fb2e93

Browse files
bpaserobpasero
committed
debt - avoid sync fs in registerWorkspaceBackup 
1 parent 099445b commit 7fb2e93

File tree

5 files changed

+25
-33
lines changed

5 files changed

+25
-33
lines changed

‎src/vs/platform/backup/electron-main/backup.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,8 @@ export interface IBackupMainService {
1717

1818
getEmptyWindowBackups(): IEmptyWindowBackupInfo[];
1919

20-
registerWorkspaceBackup(workspaceInfo: IWorkspaceBackupInfo, migrateFrom?: string): string;
20+
registerWorkspaceBackup(workspaceInfo: IWorkspaceBackupInfo): string;
21+
registerWorkspaceBackup(workspaceInfo: IWorkspaceBackupInfo, migrateFrom: string): Promise<string>;
2122
registerFolderBackup(folderInfo: IFolderBackupInfo): string;
2223
registerEmptyWindowBackup(emptyWindowInfo: IEmptyWindowBackupInfo): string;
2324

‎src/vs/platform/backup/electron-main/backupMainService.ts

Lines changed: 9 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@
44
*--------------------------------------------------------------------------------------------*/
55

66
import { createHash } from 'crypto';
7-
import * as fs from 'fs';
87
import { isEqual } from 'vs/base/common/extpath';
98
import { Schemas } from 'vs/base/common/network';
109
import { join } from 'vs/base/common/path';
@@ -134,7 +133,9 @@ export class BackupMainService implements IBackupMainService {
134133
return this.emptyWindows.slice(0); // return a copy
135134
}
136135

137-
registerWorkspaceBackup(workspaceInfo: IWorkspaceBackupInfo, migrateFrom?: string): string {
136+
registerWorkspaceBackup(workspaceInfo: IWorkspaceBackupInfo): string;
137+
registerWorkspaceBackup(workspaceInfo: IWorkspaceBackupInfo, migrateFrom: string): Promise<string>;
138+
registerWorkspaceBackup(workspaceInfo: IWorkspaceBackupInfo, migrateFrom?: string): string | Promise<string> {
138139
if (!this.workspaces.some(workspace => workspaceInfo.workspace.id === workspace.workspace.id)) {
139140
this.workspaces.push(workspaceInfo);
140141
this.storeWorkspacesMetadata();
@@ -143,23 +144,23 @@ export class BackupMainService implements IBackupMainService {
143144
const backupPath = join(this.backupHome, workspaceInfo.workspace.id);
144145

145146
if (migrateFrom) {
146-
this.moveBackupFolderSync(backupPath, migrateFrom);
147+
return this.moveBackupFolder(backupPath, migrateFrom).then(() => backupPath);
147148
}
148149

149150
return backupPath;
150151
}
151152

152-
private moveBackupFolderSync(backupPath: string, moveFromPath: string): void {
153+
private async moveBackupFolder(backupPath: string, moveFromPath: string): Promise<void> {
153154

154155
// Target exists: make sure to convert existing backups to empty window backups
155-
if (fs.existsSync(backupPath)) {
156-
this.convertToEmptyWindowBackupSync(backupPath);
156+
if (await Promises.exists(backupPath)) {
157+
await this.convertToEmptyWindowBackup(backupPath);
157158
}
158159

159160
// When we have data to migrate from, move it over to the target location
160-
if (fs.existsSync(moveFromPath)) {
161+
if (await Promises.exists(moveFromPath)) {
161162
try {
162-
fs.renameSync(moveFromPath, backupPath);
163+
await Promises.rename(moveFromPath, backupPath);
163164
} catch (error) {
164165
this.logService.error(`Backup: Could not move backup folder to new location: ${error.toString()}`);
165166
}
@@ -324,22 +325,6 @@ export class BackupMainService implements IBackupMainService {
324325
return true;
325326
}
326327

327-
private convertToEmptyWindowBackupSync(backupPath: string): boolean {
328-
const newEmptyWindowBackupInfo = this.prepareNewEmptyWindowBackup();
329-
330-
// Rename backupPath to new empty window backup path
331-
const newEmptyWindowBackupPath = join(this.backupHome, newEmptyWindowBackupInfo.backupFolder);
332-
try {
333-
fs.renameSync(backupPath, newEmptyWindowBackupPath);
334-
} catch (error) {
335-
this.logService.error(`Backup: Could not rename backup folder: ${error.toString()}`);
336-
return false;
337-
}
338-
this.emptyWindows.push(newEmptyWindowBackupInfo);
339-
340-
return true;
341-
}
342-
343328
async getDirtyWorkspaces(): Promise<Array<IWorkspaceBackupInfo | IFolderBackupInfo>> {
344329
const dirtyWorkspaces: Array<IWorkspaceBackupInfo | IFolderBackupInfo> = [];
345330

‎src/vs/platform/backup/test/electron-main/backupMainService.test.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -328,13 +328,13 @@ flakySuite('BackupMainService', () => {
328328
assert.strictEqual(service.getEmptyWindowBackups().length, 1);
329329
});
330330

331-
test('service supports to migrate backup data from another location', () => {
331+
test('service supports to migrate backup data from another location', async () => {
332332
const backupPathToMigrate = service.toBackupPath(fooFile);
333333
fs.mkdirSync(backupPathToMigrate);
334334
fs.writeFileSync(path.join(backupPathToMigrate, 'backup.txt'), 'Some Data');
335335
service.registerFolderBackup(toFolderBackupInfo(URI.file(backupPathToMigrate)));
336336

337-
const workspaceBackupPath = service.registerWorkspaceBackup(toWorkspaceBackupInfo(barFile.fsPath), backupPathToMigrate);
337+
const workspaceBackupPath = await service.registerWorkspaceBackup(toWorkspaceBackupInfo(barFile.fsPath), backupPathToMigrate);
338338

339339
assert.ok(fs.existsSync(workspaceBackupPath));
340340
assert.ok(fs.existsSync(path.join(workspaceBackupPath, 'backup.txt')));
@@ -344,7 +344,7 @@ flakySuite('BackupMainService', () => {
344344
assert.strictEqual(0, emptyBackups.length);
345345
});
346346

347-
test('service backup migration makes sure to preserve existing backups', () => {
347+
test('service backup migration makes sure to preserve existing backups', async () => {
348348
const backupPathToMigrate = service.toBackupPath(fooFile);
349349
fs.mkdirSync(backupPathToMigrate);
350350
fs.writeFileSync(path.join(backupPathToMigrate, 'backup.txt'), 'Some Data');
@@ -355,7 +355,7 @@ flakySuite('BackupMainService', () => {
355355
fs.writeFileSync(path.join(backupPathToPreserve, 'backup.txt'), 'Some Data');
356356
service.registerFolderBackup(toFolderBackupInfo(URI.file(backupPathToPreserve)));
357357

358-
const workspaceBackupPath = service.registerWorkspaceBackup(toWorkspaceBackupInfo(barFile.fsPath), backupPathToMigrate);
358+
const workspaceBackupPath = await service.registerWorkspaceBackup(toWorkspaceBackupInfo(barFile.fsPath), backupPathToMigrate);
359359

360360
assert.ok(fs.existsSync(workspaceBackupPath));
361361
assert.ok(fs.existsSync(path.join(workspaceBackupPath, 'backup.txt')));

‎src/vs/platform/workspaces/electron-main/workspacesManagementMainService.ts

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -281,7 +281,7 @@ export class WorkspacesManagementMainService extends Disposable implements IWork
281281
return undefined; // return early if the workspace is not valid
282282
}
283283

284-
const result = this.doEnterWorkspace(window, getWorkspaceIdentifier(path));
284+
const result = await this.doEnterWorkspace(window, getWorkspaceIdentifier(path));
285285
if (!result) {
286286
return undefined;
287287
}
@@ -321,7 +321,7 @@ export class WorkspacesManagementMainService extends Disposable implements IWork
321321
return true; // OK
322322
}
323323

324-
private doEnterWorkspace(window: ICodeWindow, workspace: IWorkspaceIdentifier): IEnterWorkspaceResult | undefined {
324+
private async doEnterWorkspace(window: ICodeWindow, workspace: IWorkspaceIdentifier): Promise<IEnterWorkspaceResult | undefined> {
325325
if (!window.config) {
326326
return undefined;
327327
}
@@ -331,7 +331,11 @@ export class WorkspacesManagementMainService extends Disposable implements IWork
331331
// Register window for backups and migrate current backups over
332332
let backupPath: string | undefined;
333333
if (!window.config.extensionDevelopmentPath) {
334-
backupPath = this.backupMainService.registerWorkspaceBackup({ workspace, remoteAuthority: window.remoteAuthority }, window.config.backupPath);
334+
if (window.config.backupPath) {
335+
backupPath = await this.backupMainService.registerWorkspaceBackup({ workspace, remoteAuthority: window.remoteAuthority }, window.config.backupPath);
336+
} else {
337+
backupPath = this.backupMainService.registerWorkspaceBackup({ workspace, remoteAuthority: window.remoteAuthority });
338+
}
335339
}
336340

337341
// if the window was opened on an untitled workspace, delete it.

‎src/vs/platform/workspaces/test/electron-main/workspacesManagementMainService.test.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,9 @@ flakySuite('WorkspacesManagementMainService', () => {
5353

5454
isHotExitEnabled(): boolean { throw new Error('Method not implemented.'); }
5555
getEmptyWindowBackups(): IEmptyWindowBackupInfo[] { throw new Error('Method not implemented.'); }
56-
registerWorkspaceBackup(workspace: IWorkspaceBackupInfo, migrateFrom?: string | undefined): string { throw new Error('Method not implemented.'); }
56+
registerWorkspaceBackup(workspaceInfo: IWorkspaceBackupInfo): string;
57+
registerWorkspaceBackup(workspaceInfo: IWorkspaceBackupInfo, migrateFrom: string): Promise<string>;
58+
registerWorkspaceBackup(workspaceInfo: unknown, migrateFrom?: unknown): string | Promise<string> { throw new Error('Method not implemented.'); }
5759
registerFolderBackup(folder: IFolderBackupInfo): string { throw new Error('Method not implemented.'); }
5860
registerEmptyWindowBackup(empty: IEmptyWindowBackupInfo): string { throw new Error('Method not implemented.'); }
5961
async getDirtyWorkspaces(): Promise<(IWorkspaceBackupInfo | IFolderBackupInfo)[]> { return []; }

0 commit comments

Comments
 (0)