add feedback from pr review

src/vs/workbench/services/backup/node/backupFileService.ts

@@ -14,8 +14,8 @@ import { IEnvironmentService } from 'vs/platform/environment/common/environment'
 import { IFileService } from 'vs/platform/files/common/files';
 import { TPromise } from 'vs/base/common/winjs.base';
 
-export interface IBackupsFileModel {
-	resolve(backupRoot: string): TPromise<IBackupsFileModel>;
+export interface IBackupFilesModel {
+	resolve(backupRoot: string): TPromise<IBackupFilesModel>;
 
 	add(resource: Uri, versionId?: number): void;
 	has(resource: Uri, versionId?: number): boolean;
@@ -24,10 +24,10 @@ export interface IBackupsFileModel {
 }
 
 // TODO@daniel this should resolve the backups with their file names once we have the metadata in place
-export class BackupsFileModel implements IBackupsFileModel {
+export class BackupFilesModel implements IBackupFilesModel {
 	private cache: { [resource: string]: number /* version ID */ } = Object.create(null);
 
-	resolve(backupRoot: string): TPromise<IBackupsFileModel> {
+	public resolve(backupRoot: string): TPromise<IBackupFilesModel> {
 		return pfs.readDirsInDir(backupRoot).then(backupSchemas => {
 
 			// For all supported schemas
@@ -47,11 +47,11 @@ export class BackupsFileModel implements IBackupsFileModel {
 		}).then(() => this, error => this);
 	}
 
-	add(resource: Uri, versionId = 0): void {
+	public add(resource: Uri, versionId = 0): void {
 		this.cache[resource.toString()] = versionId;
 	}
 
-	has(resource: Uri, versionId?: number): boolean {
+	public has(resource: Uri, versionId?: number): boolean {
 		const cachedVersionId = this.cache[resource.toString()];
 		if (typeof cachedVersionId !== 'number') {
 			return false; // unknown resource
@@ -64,11 +64,11 @@ export class BackupsFileModel implements IBackupsFileModel {
 		return true;
 	}
 
-	remove(resource: Uri): void {
+	public remove(resource: Uri): void {
 		delete this.cache[resource.toString()];
 	}
 
-	clear(): void {
+	public clear(): void {
 		this.cache = Object.create(null);
 	}
 }
@@ -81,7 +81,7 @@ export class BackupFileService implements IBackupFileService {
 	protected workspacesJsonPath: string;
 
 	private backupWorkspacePath: string;
-	private ready: TPromise<IBackupsFileModel>;
+	private ready: TPromise<IBackupFilesModel>;
 
 	constructor(
 		private currentWorkspace: Uri,
@@ -103,8 +103,8 @@ export class BackupFileService implements IBackupFileService {
 		return this.currentWorkspace && !this.environmentService.isExtensionDevelopment; // Hot exit is disabled for empty workspaces and when doing extension development
 	}
 
-	private init(): TPromise<IBackupsFileModel> {
-		const model = new BackupsFileModel();
+	private init(): TPromise<IBackupFilesModel> {
+		const model = new BackupFilesModel();
 
 		if (!this.backupEnabled) {
 			return TPromise.as(model);
@@ -140,11 +140,11 @@ export class BackupFileService implements IBackupFileService {
 		return this.ready.then(model => {
 			const backupResource = this.getBackupResource(resource);
 			if (!backupResource) {
-				return TPromise.as(void 0);
+				return void 0;
 			}
 
 			if (model.has(backupResource, versionId)) {
-				return TPromise.as(void 0); // return early if backup version id matches requested one
+				return void 0; // return early if backup version id matches requested one
 			}
 
 			return this.fileService.updateContent(backupResource, content, BACKUP_FILE_UPDATE_OPTIONS).then(() => model.add(backupResource, versionId));
@@ -155,7 +155,7 @@ export class BackupFileService implements IBackupFileService {
 		return this.ready.then(model => {
 			const backupResource = this.getBackupResource(resource);
 			if (!backupResource) {
-				return TPromise.as(void 0);
+				return void 0;
 			}
 
 			return this.fileService.del(backupResource).then(() => model.remove(backupResource));
@@ -165,14 +165,14 @@ export class BackupFileService implements IBackupFileService {
 	public discardAllWorkspaceBackups(): TPromise<void> {
 		return this.ready.then(model => {
 			if (!this.backupEnabled) {
-				return TPromise.as(void 0);
+				return void 0;
 			}
 
 			return this.fileService.del(Uri.file(this.backupWorkspacePath)).then(() => model.clear());
 		});
 	}
 
-	public getBackupResource(resource: Uri): Uri {
+	protected getBackupResource(resource: Uri): Uri {
 		if (!this.backupEnabled) {
 			return null;
 		}

src/vs/workbench/services/backup/test/backupFileService.test.ts

@@ -14,7 +14,7 @@ import path = require('path');
 import extfs = require('vs/base/node/extfs');
 import pfs = require('vs/base/node/pfs');
 import Uri from 'vs/base/common/uri';
-import { BackupFileService, BackupsFileModel } from 'vs/workbench/services/backup/node/backupFileService';
+import { BackupFileService, BackupFilesModel } from 'vs/workbench/services/backup/node/backupFileService';
 import { FileService } from 'vs/workbench/services/files/node/fileService';
 import { EnvironmentService } from 'vs/platform/environment/node/environmentService';
 import { parseArgs } from 'vs/platform/environment/node/argv';
@@ -37,6 +37,10 @@ class TestBackupFileService extends BackupFileService {
 
 		super(workspace, testEnvironmentService, fileService);
 	}
+
+	public getBackupResource(resource: Uri): Uri {
+		return super.getBackupResource(resource);
+	}
 }
 
 suite('BackupFileService', () => {
@@ -53,7 +57,7 @@ suite('BackupFileService', () => {
 	const barBackupPath = path.join(workspaceBackupPath, 'file', crypto.createHash('md5').update(barFile.fsPath).digest('hex'));
 	const untitledBackupPath = path.join(workspaceBackupPath, 'untitled', untitledFile.fsPath);
 
-	let service: BackupFileService;
+	let service: TestBackupFileService;
 
 	setup(done => {
 		service = new TestBackupFileService(workspaceResource, backupHome, workspacesJsonPath);
@@ -166,8 +170,8 @@ suite('BackupFileService', () => {
 		});
 	});
 
-	test('BackupsFileModel - simple', () => {
-		const model = new BackupsFileModel();
+	test('BackupFilesModel - simple', () => {
+		const model = new BackupFilesModel();
 
 		const resource1 = Uri.file('test.html');
 
@@ -213,11 +217,11 @@ suite('BackupFileService', () => {
 		assert.equal(model.has(resource4), true);
 	});
 
-	test('BackupsFileModel - resolve', (done) => {
+	test('BackupFilesModel - resolve', (done) => {
 		pfs.mkdirp(path.dirname(fooBackupPath)).then(() => {
 			fs.writeFileSync(fooBackupPath, 'foo');
 
-			const model = new BackupsFileModel();
+			const model = new BackupFilesModel();
 
 			model.resolve(workspaceBackupPath).then(model => {
 				assert.equal(model.has(Uri.file(fooBackupPath)), true);