more cleanup of file storage

Benjamin Pasero · Benjamin Pasero · commit a0a01738631b · 2017-11-23T11:43:30.000+01:00
diff --git a/src/vs/platform/storage/node/storage.ts b/src/vs/platform/storage/node/storage.ts
@@ -10,6 +10,7 @@ import * as fs from 'original-fs';
 import { IEnvironmentService } from 'vs/platform/environment/common/environment';
 import { createDecorator } from 'vs/platform/instantiation/common/instantiation';
 import { writeFileAndFlushSync } from 'vs/base/node/extfs';
+import { isUndefined, isUndefinedOrNull } from 'vs/base/common/types';
 
 export const IStorageService = createDecorator<IStorageService>('storageService');
 
@@ -33,7 +34,7 @@ export class FileStorage {
 		}
 
 		const res = this.database[key];
-		if (typeof res === 'undefined') {
+		if (isUndefinedOrNull(res)) {
 			return defaultValue;
 		}
 
@@ -45,6 +46,11 @@ export class FileStorage {
 			this.database = this.load();
 		}
 
+		// Remove an item when it is undefined or null
+		if (isUndefinedOrNull(data)) {
+			return this.removeItem(key);
+		}
+
 		// Shortcut for primitives that did not change
 		if (typeof data === 'string' || typeof data === 'number' || typeof data === 'boolean') {
 			if (this.database[key] === data) {
@@ -61,8 +67,9 @@ export class FileStorage {
 			this.database = this.load();
 		}
 
-		if (this.database[key]) {
-			delete this.database[key];
+		// Only update if the key is actually present (not undefined)
+		if (!isUndefined(this.database[key])) {
+			this.database[key] = void 0;
 			this.save();
 		}
 	}
diff --git a/src/vs/platform/storage/test/node/storage.test.ts b/src/vs/platform/storage/test/node/storage.test.ts
@@ -44,6 +44,12 @@ suite('StorageService', () => {
 			service.setItem('some.other.key', 'some.other.value');
 			assert.equal(service.getItem('some.other.key'), 'some.other.value');
 
+			service.setItem('some.undefined.key', void 0);
+			assert.equal(service.getItem('some.undefined.key', 'some.default'), 'some.default');
+
+			service.setItem('some.null.key', null);
+			assert.equal(service.getItem('some.null.key', 'some.default'), 'some.default');
+
 			done();
 		});
 	});

Original file line number	Diff line number	Diff line change
`@@ -10,6 +10,7 @@ import * as fs from 'original-fs';`
`10`	`10`	`import { IEnvironmentService } from 'vs/platform/environment/common/environment';`
`11`	`11`	`import { createDecorator } from 'vs/platform/instantiation/common/instantiation';`
`12`	`12`	`import { writeFileAndFlushSync } from 'vs/base/node/extfs';`
	`13`	`+import { isUndefined, isUndefinedOrNull } from 'vs/base/common/types';`
`13`	`14`
`14`	`15`	`export const IStorageService = createDecorator<IStorageService>('storageService');`
`15`	`16`
`@@ -33,7 +34,7 @@ export class FileStorage {`
`33`	`34`	`}`
`34`	`35`
`35`	`36`	`const res = this.database[key];`
`36`		`- if (typeof res === 'undefined') {`
	`37`	`+ if (isUndefinedOrNull(res)) {`
`37`	`38`	`return defaultValue;`
`38`	`39`	`}`
`39`	`40`
`@@ -45,6 +46,11 @@ export class FileStorage {`
`45`	`46`	`this.database = this.load();`
`46`	`47`	`}`
`47`	`48`
	`49`	`+ // Remove an item when it is undefined or null`
	`50`	`+ if (isUndefinedOrNull(data)) {`
	`51`	`+ return this.removeItem(key);`
	`52`	`+ }`
	`53`	`+`
`48`	`54`	`// Shortcut for primitives that did not change`
`49`	`55`	`if (typeof data === 'string' \|\| typeof data === 'number' \|\| typeof data === 'boolean') {`
`50`	`56`	`if (this.database[key] === data) {`
`@@ -61,8 +67,9 @@ export class FileStorage {`
`61`	`67`	`this.database = this.load();`
`62`	`68`	`}`
`63`	`69`
`64`		`- if (this.database[key]) {`
`65`		`- delete this.database[key];`
	`70`	`+ // Only update if the key is actually present (not undefined)`
	`71`	`+ if (!isUndefined(this.database[key])) {`
	`72`	`+ this.database[key] = void 0;`
`66`	`73`	`this.save();`
`67`	`74`	`}`
`68`	`75`	`}`