Skip to content

Commit 570b5f7

Browse files
joyceerhljoyceerhl
authored
Allow storage service to get/set objects (microsoft#181444)
* Allow storage service to get/set objects * Improve getObject and add tests * Use deepStrictEqual for objects * Fix getObject impl * Fix integration test * Close storage after test completes * Use parse from marshalling
1 parent 1b5e85f commit 570b5f7

File tree

6 files changed

+83
-13
lines changed

6 files changed

+83
-13
lines changed

‎src/vs/base/parts/storage/common/storage.ts

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,8 @@
66
import { ThrottledDelayer } from 'vs/base/common/async';
77
import { Emitter, Event } from 'vs/base/common/event';
88
import { Disposable, IDisposable } from 'vs/base/common/lifecycle';
9-
import { isUndefinedOrNull } from 'vs/base/common/types';
9+
import { parse, stringify } from 'vs/base/common/marshalling';
10+
import { isUndefinedOrNull, isObject } from 'vs/base/common/types';
1011

1112
export enum StorageHint {
1213

@@ -69,7 +70,10 @@ export interface IStorage extends IDisposable {
6970
getNumber(key: string, fallbackValue: number): number;
7071
getNumber(key: string, fallbackValue?: number): number | undefined;
7172

72-
set(key: string, value: string | boolean | number | undefined | null): Promise<void>;
73+
getObject<T extends object>(key: string, fallbackValue: T): T;
74+
getObject<T extends object>(key: string, fallbackValue?: T): T | undefined;
75+
76+
set(key: string, value: string | boolean | number | undefined | null | object): Promise<void>;
7377
delete(key: string): Promise<void>;
7478

7579
flush(delay?: number): Promise<void>;
@@ -213,7 +217,19 @@ export class Storage extends Disposable implements IStorage {
213217
return parseInt(value, 10);
214218
}
215219

216-
async set(key: string, value: string | boolean | number | null | undefined): Promise<void> {
220+
getObject(key: string, fallbackValue: object): object;
221+
getObject(key: string, fallbackValue?: object | undefined): object | undefined;
222+
getObject(key: string, fallbackValue?: object): object | undefined {
223+
const value = this.get(key);
224+
225+
if (isUndefinedOrNull(value)) {
226+
return fallbackValue;
227+
}
228+
229+
return parse(value);
230+
}
231+
232+
async set(key: string, value: string | boolean | number | null | undefined | object): Promise<void> {
217233
if (this.state === StorageState.Closed) {
218234
return; // Return early if we are already closed
219235
}
@@ -224,7 +240,7 @@ export class Storage extends Disposable implements IStorage {
224240
}
225241

226242
// Otherwise, convert to String and store
227-
const valueStr = String(value);
243+
const valueStr = isObject(value) ? stringify(value) : String(value);
228244

229245
// Return early if value already set
230246
const currentValue = this.cache.get(key);

‎src/vs/base/parts/storage/test/node/storage.integrationTest.ts

Lines changed: 30 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,12 +3,13 @@
33
* Licensed under the MIT License. See License.txt in the project root for license information.
44
*--------------------------------------------------------------------------------------------*/
55

6-
import { ok, strictEqual } from 'assert';
6+
import { deepStrictEqual, ok, strictEqual } from 'assert';
77
import { tmpdir } from 'os';
88
import { timeout } from 'vs/base/common/async';
99
import { Emitter, Event } from 'vs/base/common/event';
1010
import { join } from 'vs/base/common/path';
1111
import { isWindows } from 'vs/base/common/platform';
12+
import { URI } from 'vs/base/common/uri';
1213
import { generateUuid } from 'vs/base/common/uuid';
1314
import { Promises } from 'vs/base/node/pfs';
1415
import { isStorageItemsChangeEvent, IStorageDatabase, IStorageItemsChangeEvent, Storage } from 'vs/base/parts/storage/common/storage';
@@ -30,6 +31,21 @@ flakySuite('Storage Library', function () {
3031
return Promises.rm(testDir);
3132
});
3233

34+
test('objects', () => {
35+
return runWithFakedTimers({}, async function () {
36+
const storage = new Storage(new SQLiteStorageDatabase(join(testDir, 'storage.db')));
37+
38+
await storage.init();
39+
40+
ok(!storage.getObject('foo'));
41+
const uri = URI.file('path/to/folder');
42+
storage.set('foo', { 'bar': uri });
43+
deepStrictEqual(storage.getObject('foo'), { 'bar': uri });
44+
45+
await storage.close();
46+
});
47+
});
48+
3349
test('basics', () => {
3450
return runWithFakedTimers({}, async function () {
3551
const storage = new Storage(new SQLiteStorageDatabase(join(testDir, 'storage.db')));
@@ -40,6 +56,7 @@ flakySuite('Storage Library', function () {
4056
strictEqual(storage.get('foo', 'bar'), 'bar');
4157
strictEqual(storage.getNumber('foo', 55), 55);
4258
strictEqual(storage.getBoolean('foo', true), true);
59+
deepStrictEqual(storage.getObject('foo', { 'bar': 'baz' }), { 'bar': 'baz' });
4360

4461
let changes = new Set<string>();
4562
storage.onDidChangeStorage(key => {
@@ -52,21 +69,24 @@ flakySuite('Storage Library', function () {
5269
const set1Promise = storage.set('bar', 'foo');
5370
const set2Promise = storage.set('barNumber', 55);
5471
const set3Promise = storage.set('barBoolean', true);
72+
const set4Promise = storage.set('barObject', { 'bar': 'baz' });
5573

5674
let flushPromiseResolved = false;
5775
storage.whenFlushed().then(() => flushPromiseResolved = true);
5876

5977
strictEqual(storage.get('bar'), 'foo');
6078
strictEqual(storage.getNumber('barNumber'), 55);
6179
strictEqual(storage.getBoolean('barBoolean'), true);
80+
deepStrictEqual(storage.getObject('barObject'), { 'bar': 'baz' });
6281

63-
strictEqual(changes.size, 3);
82+
strictEqual(changes.size, 4);
6483
ok(changes.has('bar'));
6584
ok(changes.has('barNumber'));
6685
ok(changes.has('barBoolean'));
86+
ok(changes.has('barObject'));
6787

6888
let setPromiseResolved = false;
69-
await Promise.all([set1Promise, set2Promise, set3Promise]).then(() => setPromiseResolved = true);
89+
await Promise.all([set1Promise, set2Promise, set3Promise, set4Promise]).then(() => setPromiseResolved = true);
7090
strictEqual(setPromiseResolved, true);
7191
strictEqual(flushPromiseResolved, true);
7292

@@ -76,32 +96,37 @@ flakySuite('Storage Library', function () {
7696
storage.set('bar', 'foo');
7797
storage.set('barNumber', 55);
7898
storage.set('barBoolean', true);
99+
storage.set('barObject', { 'bar': 'baz' });
79100
strictEqual(changes.size, 0);
80101

81102
// Simple deletes
82103
const delete1Promise = storage.delete('bar');
83104
const delete2Promise = storage.delete('barNumber');
84105
const delete3Promise = storage.delete('barBoolean');
106+
const delete4Promise = storage.delete('barObject');
85107

86108
ok(!storage.get('bar'));
87109
ok(!storage.getNumber('barNumber'));
88110
ok(!storage.getBoolean('barBoolean'));
111+
ok(!storage.getObject('barObject'));
89112

90-
strictEqual(changes.size, 3);
113+
strictEqual(changes.size, 4);
91114
ok(changes.has('bar'));
92115
ok(changes.has('barNumber'));
93116
ok(changes.has('barBoolean'));
117+
ok(changes.has('barObject'));
94118

95119
changes = new Set<string>();
96120

97121
// Does not trigger events for same delete values
98122
storage.delete('bar');
99123
storage.delete('barNumber');
100124
storage.delete('barBoolean');
125+
storage.delete('barObject');
101126
strictEqual(changes.size, 0);
102127

103128
let deletePromiseResolved = false;
104-
await Promise.all([delete1Promise, delete2Promise, delete3Promise]).then(() => deletePromiseResolved = true);
129+
await Promise.all([delete1Promise, delete2Promise, delete3Promise, delete4Promise]).then(() => deletePromiseResolved = true);
105130
strictEqual(deletePromiseResolved, true);
106131

107132
await storage.close();

‎src/vs/editor/contrib/find/test/browser/findController.test.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,7 @@ suite('FindController', async () => {
7575
get: (key: string) => queryState[key],
7676
getBoolean: (key: string) => !!queryState[key],
7777
getNumber: (key: string) => undefined!,
78+
getObject: (key: string) => undefined!,
7879
store: (key: string, value: any) => { queryState[key] = value; return Promise.resolve(); },
7980
remove: () => undefined,
8081
isNew: () => false,
@@ -507,6 +508,7 @@ suite('FindController query options persistence', async () => {
507508
get: (key: string) => queryState[key],
508509
getBoolean: (key: string) => !!queryState[key],
509510
getNumber: (key: string) => undefined!,
511+
getObject: (key: string) => undefined!,
510512
store: (key: string, value: any) => { queryState[key] = value; return Promise.resolve(); },
511513
remove: () => undefined,
512514
isNew: () => false,

‎src/vs/editor/contrib/multicursor/test/browser/multicursor.test.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,7 @@ suite('Multicursor selection', () => {
9292
get: (key: string) => queryState[key],
9393
getBoolean: (key: string) => !!queryState[key],
9494
getNumber: (key: string) => undefined!,
95+
getObject: (key: string) => undefined!,
9596
store: (key: string, value: any) => { queryState[key] = value; return Promise.resolve(); },
9697
remove: (key) => undefined,
9798
log: () => undefined,

‎src/vs/platform/storage/common/storage.ts

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,17 @@ export interface IStorageService {
9797
getNumber(key: string, scope: StorageScope, fallbackValue: number): number;
9898
getNumber(key: string, scope: StorageScope, fallbackValue?: number): number | undefined;
9999

100+
/**
101+
* Retrieve an element stored with the given key from storage. Use
102+
* the provided `defaultValue` if the element is `null` or `undefined`.
103+
* The element will be converted to a `object` using `JSON.parse`.
104+
*
105+
* @param scope allows to define the scope of the storage operation
106+
* to either the current workspace only, all workspaces or all profiles.
107+
*/
108+
getObject<T extends object>(key: string, scope: StorageScope, fallbackValue: T): T;
109+
getObject<T extends object>(key: string, scope: StorageScope, fallbackValue?: T): T | undefined;
110+
100111
/**
101112
* Store a value under the given key to storage. The value will be
102113
* converted to a `string`. Storing either `undefined` or `null` will
@@ -108,7 +119,7 @@ export interface IStorageService {
108119
* @param target allows to define the target of the storage operation
109120
* to either the current machine or user.
110121
*/
111-
store(key: string, value: string | boolean | number | undefined | null, scope: StorageScope, target: StorageTarget): void;
122+
store(key: string, value: string | boolean | number | undefined | null | object, scope: StorageScope, target: StorageTarget): void;
112123

113124
/**
114125
* Delete an element stored under the provided key from storage.
@@ -371,7 +382,13 @@ export abstract class AbstractStorageService extends Disposable implements IStor
371382
return this.getStorage(scope)?.getNumber(key, fallbackValue);
372383
}
373384

374-
store(key: string, value: string | boolean | number | undefined | null, scope: StorageScope, target: StorageTarget): void {
385+
getObject(key: string, scope: StorageScope, fallbackValue: object): object;
386+
getObject(key: string, scope: StorageScope): object | undefined;
387+
getObject(key: string, scope: StorageScope, fallbackValue?: object): object | undefined {
388+
return this.getStorage(scope)?.getObject(key, fallbackValue);
389+
}
390+
391+
store(key: string, value: string | boolean | number | undefined | null | object, scope: StorageScope, target: StorageTarget): void {
375392

376393
// We remove the key for undefined/null values
377394
if (isUndefinedOrNull(value)) {

‎src/vs/platform/storage/test/common/storageService.test.ts

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
* Licensed under the MIT License. See License.txt in the project root for license information.
44
*--------------------------------------------------------------------------------------------*/
55

6-
import { ok, strictEqual } from 'assert';
6+
import { deepStrictEqual, ok, strictEqual } from 'assert';
77
import { InMemoryStorageService, IStorageService, IStorageTargetChangeEvent, IStorageValueChangeEvent, StorageScope, StorageTarget } from 'vs/platform/storage/common/storage';
88

99
export function createSuite<T extends IStorageService>(params: { setup: () => Promise<T>; teardown: (service: T) => Promise<void> }): void {
@@ -26,7 +26,7 @@ export function createSuite<T extends IStorageService>(params: { setup: () => Pr
2626
storeData(StorageScope.PROFILE);
2727
});
2828

29-
test('Get Data, Integer, Boolean (workspace)', () => {
29+
test('Get Data, Integer, Boolean, Object (workspace)', () => {
3030
storeData(StorageScope.WORKSPACE);
3131
});
3232

@@ -40,6 +40,8 @@ export function createSuite<T extends IStorageService>(params: { setup: () => Pr
4040
strictEqual(storageService.getNumber('test.getNumber', scope, 0), 0);
4141
strictEqual(storageService.getBoolean('test.getBoolean', scope, true), true);
4242
strictEqual(storageService.getBoolean('test.getBoolean', scope, false), false);
43+
deepStrictEqual(storageService.getObject('test.getObject', scope, { 'foo': 'bar' }), { 'foo': 'bar' });
44+
deepStrictEqual(storageService.getObject('test.getObject', scope, {}), {});
4345

4446
storageService.store('test.get', 'foobar', scope, StorageTarget.MACHINE);
4547
strictEqual(storageService.get('test.get', scope, (undefined)!), 'foobar');
@@ -66,9 +68,16 @@ export function createSuite<T extends IStorageService>(params: { setup: () => Pr
6668
storageService.store('test.getBoolean', false, scope, StorageTarget.MACHINE);
6769
strictEqual(storageService.getBoolean('test.getBoolean', scope, (undefined)!), false);
6870

71+
storageService.store('test.getObject', {}, scope, StorageTarget.MACHINE);
72+
deepStrictEqual(storageService.getObject('test.getObject', scope, (undefined)!), {});
73+
74+
storageService.store('test.getObject', { 'foo': {} }, scope, StorageTarget.MACHINE);
75+
deepStrictEqual(storageService.getObject('test.getObject', scope, (undefined)!), { 'foo': {} });
76+
6977
strictEqual(storageService.get('test.getDefault', scope, 'getDefault'), 'getDefault');
7078
strictEqual(storageService.getNumber('test.getNumberDefault', scope, 5), 5);
7179
strictEqual(storageService.getBoolean('test.getBooleanDefault', scope, true), true);
80+
deepStrictEqual(storageService.getObject('test.getObjectDefault', scope, { 'foo': 42 }), { 'foo': 42 });
7281
}
7382

7483
test('Remove Data (application)', () => {

0 commit comments

Comments
 (0)