From 5352ba33be8dde00a0cb1354711174ee5c1fcf0d Mon Sep 17 00:00:00 2001 From: Benjamin Pasero Date: Fri, 16 Aug 2019 12:10:43 +0200 Subject: [PATCH] web - synchronise global state changes --- .../storage/browser/storageService.ts | 159 +++++++++++++++--- src/vs/platform/storage/common/storage.ts | 74 -------- .../storage.test.ts | 10 +- 3 files changed, 143 insertions(+), 100 deletions(-) rename src/vs/platform/storage/test/{node => electron-browser}/storage.test.ts (92%) diff --git a/src/vs/platform/storage/browser/storageService.ts b/src/vs/platform/storage/browser/storageService.ts index a11505e32acd..cec2d7dc9c77 100644 --- a/src/vs/platform/storage/browser/storageService.ts +++ b/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; + 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 { @@ -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 = this._register(new Emitter()); + readonly onDidChangeItemsExternal: Event = this._onDidChangeItemsExternal.event; + + private cache: Map | undefined; + + private pendingUpdate: Promise = 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 { + 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 { + const items = await this.doGetItemsFromFile(); + + this.cache = items; + + this._onDidChangeItemsExternal.fire({ items }); + } + + async getItems(): Promise> { + if (!this.cache) { + try { + this.cache = await this.doGetItemsFromFile(); + } catch (error) { + this.cache = new Map(); + } + } + + return this.cache; + } + + private async doGetItemsFromFile(): Promise> { + 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 { + 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 { + return this.pendingUpdate; } } diff --git a/src/vs/platform/storage/common/storage.ts b/src/vs/platform/storage/common/storage.ts index 46fdd613d11c..8860535a54a6 100644 --- a/src/vs/platform/storage/common/storage.ts +++ b/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('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 | undefined; - - private pendingUpdate: Promise = Promise.resolve(); - - private _hasPendingUpdate = false; - get hasPendingUpdate(): boolean { - return this._hasPendingUpdate; - } - - constructor( - private readonly file: URI, - private readonly fileService: IFileService - ) { - super(); - } - - async getItems(): Promise> { - if (!this.cache) { - try { - this.cache = await this.doGetItemsFromFile(); - } catch (error) { - this.cache = new Map(); - } - } - - return this.cache; - } - - private async doGetItemsFromFile(): Promise> { - await this.pendingUpdate; - - const itemsRaw = await this.fileService.readFile(this.file); - - return serializableToMap(JSON.parse(itemsRaw.value.toString())); - } - - async updateItems(request: IUpdateRequest): Promise { - 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 { - return this.pendingUpdate; - } -} - export async function logStorage(global: Map, workspace: Map, globalPath: string, workspacePath: string): Promise { const safeParse = (value: string) => { try { diff --git a/src/vs/platform/storage/test/node/storage.test.ts b/src/vs/platform/storage/test/electron-browser/storage.test.ts similarity index 92% rename from src/vs/platform/storage/test/node/storage.test.ts rename to src/vs/platform/storage/test/electron-browser/storage.test.ts index 7c77b9e1c08f..4e5e10a4e13a 100644 --- a/src/vs/platform/storage/test/node/storage.test.ts +++ b/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 +});