web - synchronise global state changes

src/vs/platform/storage/browser/storageService.ts

@@ -5,15 +5,17 @@
 
 import { Disposable } from 'vs/base/common/lifecycle';
 import { Event, Emitter } from 'vs/base/common/event';
-import { IWorkspaceStorageChangeEvent, IStorageService, StorageScope, IWillSaveStateEvent, WillSaveStateReason, logStorage, FileStorageDatabase } from 'vs/platform/storage/common/storage';
+import { IWorkspaceStorageChangeEvent, IStorageService, StorageScope, IWillSaveStateEvent, WillSaveStateReason, logStorage } from 'vs/platform/storage/common/storage';
 import { IEnvironmentService } from 'vs/platform/environment/common/environment';
 import { IWorkspaceInitializationPayload } from 'vs/platform/workspaces/common/workspaces';
 import { ServiceIdentifier } from 'vs/platform/instantiation/common/instantiation';
-import { IFileService } from 'vs/platform/files/common/files';
-import { IStorage, Storage } from 'vs/base/parts/storage/common/storage';
+import { IFileService, FileChangeType } from 'vs/platform/files/common/files';
+import { IStorage, Storage, IStorageDatabase, IStorageItemsChangeEvent, IUpdateRequest } from 'vs/base/parts/storage/common/storage';
 import { URI } from 'vs/base/common/uri';
 import { joinPath } from 'vs/base/common/resources';
-import { runWhenIdle } from 'vs/base/common/async';
+import { runWhenIdle, RunOnceScheduler } from 'vs/base/common/async';
+import { serializableToMap, mapToSerializable } from 'vs/base/common/map';
+import { VSBuffer } from 'vs/base/common/buffer';
 
 export class BrowserStorageService extends Disposable implements IStorageService {
 
@@ -35,6 +37,7 @@ export class BrowserStorageService extends Disposable implements IStorageService
 	private workspaceStorageFile: URI;
 
 	private initializePromise: Promise<void>;
+	private periodicSaveScheduler = this._register(new RunOnceScheduler(() => this.saveState(), 5000));
 
 	get hasPendingUpdate(): boolean {
 		return this.globalStorageDatabase.hasPendingUpdate || this.workspaceStorageDatabase.hasPendingUpdate;
@@ -51,20 +54,23 @@ export class BrowserStorageService extends Disposable implements IStorageService
 		// long running operation.
 		// Instead, periodically ask customers to save save. The library will be clever enough
 		// to only save state that has actually changed.
-		this.saveStatePeriodically();
+		this.periodicSaveScheduler.schedule();
 	}
 
-	private saveStatePeriodically(): void {
-		setTimeout(() => {
-			runWhenIdle(() => {
+	private saveState(): void {
+		runWhenIdle(() => {
 
-				// this event will potentially cause new state to be stored
+			// this event will potentially cause new state to be stored
+			// since new state will only be created while the document
+			// has focus, one optimization is to not run this when the
+			// document has no focus, assuming that state has not changed
+			if (document.hasFocus()) {
 				this._onWillSaveState.fire({ reason: WillSaveStateReason.NONE });
+			}
 
-				// repeat
-				this.saveStatePeriodically();
-			});
-		}, 5000);
+			// repeat
+			this.periodicSaveScheduler.schedule();
+		});
 	}
 
 	initialize(payload: IWorkspaceInitializationPayload): Promise<void> {
@@ -83,14 +89,14 @@ export class BrowserStorageService extends Disposable implements IStorageService
 
 		// Workspace Storage
 		this.workspaceStorageFile = joinPath(stateRoot, `${payload.id}.json`);
-		this.workspaceStorageDatabase = this._register(new FileStorageDatabase(this.workspaceStorageFile, this.fileService));
-		this.workspaceStorage = new Storage(this.workspaceStorageDatabase);
+		this.workspaceStorageDatabase = this._register(new FileStorageDatabase(this.workspaceStorageFile, false /* do not watch for external changes */, this.fileService));
+		this.workspaceStorage = this._register(new Storage(this.workspaceStorageDatabase));
 		this._register(this.workspaceStorage.onDidChangeStorage(key => this._onDidChangeStorage.fire({ key, scope: StorageScope.WORKSPACE })));
 
 		// Global Storage
 		this.globalStorageFile = joinPath(stateRoot, 'global.json');
-		this.globalStorageDatabase = this._register(new FileStorageDatabase(this.globalStorageFile, this.fileService));
-		this.globalStorage = new Storage(this.globalStorageDatabase);
+		this.globalStorageDatabase = this._register(new FileStorageDatabase(this.globalStorageFile, true /* watch for external changes */, this.fileService));
+		this.globalStorage = this._register(new Storage(this.globalStorageDatabase));
 		this._register(this.globalStorage.onDidChangeStorage(key => this._onDidChangeStorage.fire({ key, scope: StorageScope.GLOBAL })));
 
 		// Init both
@@ -140,14 +146,125 @@ export class BrowserStorageService extends Disposable implements IStorageService
 	}
 
 	close(): void {
-
-		// Signal as event so that clients can still store data
-		this._onWillSaveState.fire({ reason: WillSaveStateReason.SHUTDOWN });
-
 		// We explicitly do not close our DBs because writing data onBeforeUnload()
 		// can result in unexpected results. Namely, it seems that - even though this
 		// operation is async - sometimes it is being triggered on unload and
 		// succeeds. Often though, the DBs turn out to be empty because the write
 		// never had a chance to complete.
+		//
+		// Instead we trigger dispose() to ensure that no timeouts or callbacks
+		// get triggered in this phase.
+		this.dispose();
+	}
+}
+
+export class FileStorageDatabase extends Disposable implements IStorageDatabase {
+
+	private readonly _onDidChangeItemsExternal: Emitter<IStorageItemsChangeEvent> = this._register(new Emitter<IStorageItemsChangeEvent>());
+	readonly onDidChangeItemsExternal: Event<IStorageItemsChangeEvent> = this._onDidChangeItemsExternal.event;
+
+	private cache: Map<string, string> | undefined;
+
+	private pendingUpdate: Promise<void> = Promise.resolve();
+
+	private _hasPendingUpdate = false;
+	get hasPendingUpdate(): boolean {
+		return this._hasPendingUpdate;
+	}
+
+	private isWatching = false;
+
+	constructor(
+		private readonly file: URI,
+		private readonly watchForExternalChanges: boolean,
+		@IFileService private readonly fileService: IFileService
+	) {
+		super();
+	}
+
+	private async ensureWatching(): Promise<void> {
+		if (this.isWatching || !this.watchForExternalChanges) {
+			return;
+		}
+
+		const exists = await this.fileService.exists(this.file);
+		if (this.isWatching || !exists) {
+			return; // file must exist to be watched
+		}
+
+		this.isWatching = true;
+
+		this._register(this.fileService.watch(this.file));
+		this._register(this.fileService.onFileChanges(e => {
+			if (document.hasFocus()) {
+				return; // ignore changes from ourselves by checking for focus
+			}
+
+			if (!e.contains(this.file, FileChangeType.UPDATED)) {
+				return; // not our file
+			}
+
+			this.onDidStorageChangeExternal();
+		}));
+	}
+
+	private async onDidStorageChangeExternal(): Promise<void> {
+		const items = await this.doGetItemsFromFile();
+
+		this.cache = items;
+
+		this._onDidChangeItemsExternal.fire({ items });
+	}
+
+	async getItems(): Promise<Map<string, string>> {
+		if (!this.cache) {
+			try {
+				this.cache = await this.doGetItemsFromFile();
+			} catch (error) {
+				this.cache = new Map();
+			}
+		}
+
+		return this.cache;
+	}
+
+	private async doGetItemsFromFile(): Promise<Map<string, string>> {
+		await this.pendingUpdate;
+
+		const itemsRaw = await this.fileService.readFile(this.file);
+
+		this.ensureWatching(); // now that the file must exist, ensure we watch it for changes
+
+		return serializableToMap(JSON.parse(itemsRaw.value.toString()));
+	}
+
+	async updateItems(request: IUpdateRequest): Promise<void> {
+		const items = await this.getItems();
+
+		if (request.insert) {
+			request.insert.forEach((value, key) => items.set(key, value));
+		}
+
+		if (request.delete) {
+			request.delete.forEach(key => items.delete(key));
+		}
+
+		await this.pendingUpdate;
+
+		this._hasPendingUpdate = true;
+
+		this.pendingUpdate = this.fileService.writeFile(this.file, VSBuffer.fromString(JSON.stringify(mapToSerializable(items))))
+			.then(() => {
+				this.ensureWatching(); // now that the file must exist, ensure we watch it for changes
+			})
+			.finally(() => {
+				this._hasPendingUpdate = false;
+			});
+
+		return this.pendingUpdate;
+	}
+
+	close(): Promise<void> {
+		return this.pendingUpdate;
 	}
 }

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

@@ -7,11 +7,6 @@ import { createDecorator, ServiceIdentifier } from 'vs/platform/instantiation/co
 import { Event, Emitter } from 'vs/base/common/event';
 import { Disposable } from 'vs/base/common/lifecycle';
 import { isUndefinedOrNull } from 'vs/base/common/types';
-import { IUpdateRequest, IStorageDatabase } from 'vs/base/parts/storage/common/storage';
-import { serializableToMap, mapToSerializable } from 'vs/base/common/map';
-import { VSBuffer } from 'vs/base/common/buffer';
-import { URI } from 'vs/base/common/uri';
-import { IFileService } from 'vs/platform/files/common/files';
 
 export const IStorageService = createDecorator<IStorageService>('storageService');
 
@@ -212,75 +207,6 @@ export class InMemoryStorageService extends Disposable implements IStorageServic
 	}
 }
 
-export class FileStorageDatabase extends Disposable implements IStorageDatabase {
-
-	readonly onDidChangeItemsExternal = Event.None; // TODO@Ben implement global UI storage events
-
-	private cache: Map<string, string> | undefined;
-
-	private pendingUpdate: Promise<void> = Promise.resolve();
-
-	private _hasPendingUpdate = false;
-	get hasPendingUpdate(): boolean {
-		return this._hasPendingUpdate;
-	}
-
-	constructor(
-		private readonly file: URI,
-		private readonly fileService: IFileService
-	) {
-		super();
-	}
-
-	async getItems(): Promise<Map<string, string>> {
-		if (!this.cache) {
-			try {
-				this.cache = await this.doGetItemsFromFile();
-			} catch (error) {
-				this.cache = new Map();
-			}
-		}
-
-		return this.cache;
-	}
-
-	private async doGetItemsFromFile(): Promise<Map<string, string>> {
-		await this.pendingUpdate;
-
-		const itemsRaw = await this.fileService.readFile(this.file);
-
-		return serializableToMap(JSON.parse(itemsRaw.value.toString()));
-	}
-
-	async updateItems(request: IUpdateRequest): Promise<void> {
-		const items = await this.getItems();
-
-		if (request.insert) {
-			request.insert.forEach((value, key) => items.set(key, value));
-		}
-
-		if (request.delete) {
-			request.delete.forEach(key => items.delete(key));
-		}
-
-		await this.pendingUpdate;
-
-		this._hasPendingUpdate = true;
-
-		this.pendingUpdate = this.fileService.writeFile(this.file, VSBuffer.fromString(JSON.stringify(mapToSerializable(items))))
-			.then(() => undefined)
-			.finally(() => {
-				this._hasPendingUpdate = false;
-			});
-
-		return this.pendingUpdate;
-	}
-
-	close(): Promise<void> {
-		return this.pendingUpdate;
-	}
-}
-
 export async function logStorage(global: Map<string, string>, workspace: Map<string, string>, globalPath: string, workspacePath: string): Promise<void> {
 	const safeParse = (value: string) => {
 		try {

src/vs/platform/storage/test/node/storage.test.ts → src/vs/platform/storage/test/electron-browser/storage.test.ts

@@ -4,7 +4,7 @@
  *--------------------------------------------------------------------------------------------*/
 
 import { equal } from 'assert';
-import { FileStorageDatabase } from 'vs/platform/storage/common/storage';
+import { FileStorageDatabase } from 'vs/platform/storage/browser/storageService';
 import { generateUuid } from 'vs/base/common/uuid';
 import { join } from 'vs/base/common/path';
 import { tmpdir } from 'os';
@@ -49,7 +49,7 @@ suite('Storage', () => {
 	});
 
 	test('File Based Storage', async () => {
-		let storage = new Storage(new FileStorageDatabase(URI.file(join(testDir, 'storage.json')), fileService));
+		let storage = new Storage(new FileStorageDatabase(URI.file(join(testDir, 'storage.json')), false, fileService));
 
 		await storage.init();
 
@@ -63,7 +63,7 @@ suite('Storage', () => {
 
 		await storage.close();
 
-		storage = new Storage(new FileStorageDatabase(URI.file(join(testDir, 'storage.json')), fileService));
+		storage = new Storage(new FileStorageDatabase(URI.file(join(testDir, 'storage.json')), false, fileService));
 
 		await storage.init();
 
@@ -81,7 +81,7 @@ suite('Storage', () => {
 
 		await storage.close();
 
-		storage = new Storage(new FileStorageDatabase(URI.file(join(testDir, 'storage.json')), fileService));
+		storage = new Storage(new FileStorageDatabase(URI.file(join(testDir, 'storage.json')), false, fileService));
 
 		await storage.init();
 
@@ -89,4 +89,4 @@ suite('Storage', () => {
 		equal(storage.get('barNumber', 'undefinedNumber'), 'undefinedNumber');
 		equal(storage.get('barBoolean', 'undefinedBoolean'), 'undefinedBoolean');
 	});
-});
\ No newline at end of file
+});