From e475cf038bc285025ceece69702af11f9e200598 Mon Sep 17 00:00:00 2001 From: Benjamin Pasero Date: Mon, 3 May 2021 09:43:41 +0200 Subject: [PATCH] state service - allow multiple setItems at once --- src/bootstrap-node.js | 1 + .../electron-main/dialogMainService.ts | 1 - .../protocol/electron-main/protocol.ts | 6 +-- .../electron-main/protocolMainService.ts | 5 +- src/vs/platform/state/node/state.ts | 1 + src/vs/platform/state/node/stateService.ts | 51 +++++++++++++++---- src/vs/platform/state/test/node/state.test.ts | 28 ++++++++++ .../platform/windows/electron-main/window.ts | 6 +-- 8 files changed, 79 insertions(+), 20 deletions(-) diff --git a/src/bootstrap-node.js b/src/bootstrap-node.js index b9cc3f67b84..9a57b93333c 100644 --- a/src/bootstrap-node.js +++ b/src/bootstrap-node.js @@ -11,6 +11,7 @@ // - Posix: allow to change the current working dir via `VSCODE_CWD` if defined // - all OS: store the `process.cwd()` inside `VSCODE_CWD` for consistent lookups // TODO@bpasero revisit if chdir() on Windows is needed in the future still +// (find all users of `chdir` in code, there are more locations) function setupCurrentWorkingDirectory() { const path = require('path'); diff --git a/src/vs/platform/dialogs/electron-main/dialogMainService.ts b/src/vs/platform/dialogs/electron-main/dialogMainService.ts index 164e8d7694c..dbf1f60fd2a 100644 --- a/src/vs/platform/dialogs/electron-main/dialogMainService.ts +++ b/src/vs/platform/dialogs/electron-main/dialogMainService.ts @@ -91,7 +91,6 @@ export class DialogMainService implements IDialogMainService { // Ensure defaultPath dialogOptions.defaultPath = options.defaultPath || this.stateService.getItem(DialogMainService.workingDirPickerStorageKey); - // Ensure properties if (typeof options.pickFiles === 'boolean' || typeof options.pickFolders === 'boolean') { dialogOptions.properties = undefined; // let it override based on the booleans diff --git a/src/vs/platform/protocol/electron-main/protocol.ts b/src/vs/platform/protocol/electron-main/protocol.ts index e224973ab70..85a213cfa07 100644 --- a/src/vs/platform/protocol/electron-main/protocol.ts +++ b/src/vs/platform/protocol/electron-main/protocol.ts @@ -34,12 +34,8 @@ export interface IProtocolMainService { /** * Allows to make an object accessible to a renderer * via `ipcRenderer.invoke(resource.toString())`. - * - * @param obj the (optional) object to make accessible to the - * renderer. Can be updated later via the `IObjectUrl#update` - * method too. */ - createIPCObjectUrl(obj?: T): IIPCObjectUrl; + createIPCObjectUrl(): IIPCObjectUrl; /** * Adds a `URI` as root to the list of allowed diff --git a/src/vs/platform/protocol/electron-main/protocolMainService.ts b/src/vs/platform/protocol/electron-main/protocolMainService.ts index 9f93a6b7755..4215275cf05 100644 --- a/src/vs/platform/protocol/electron-main/protocolMainService.ts +++ b/src/vs/platform/protocol/electron-main/protocolMainService.ts @@ -142,7 +142,8 @@ export class ProtocolMainService extends Disposable implements IProtocolMainServ //#region IPC Object URLs - createIPCObjectUrl(obj: T): IIPCObjectUrl { + createIPCObjectUrl(): IIPCObjectUrl { + let obj: T | undefined = undefined; // Create unique URI const resource = URI.from({ @@ -152,7 +153,7 @@ export class ProtocolMainService extends Disposable implements IProtocolMainServ // Install IPC handler const channel = resource.toString(); - const handler = async (): Promise => obj; + const handler = async (): Promise => obj; ipcMain.handle(channel, handler); this.logService.trace(`IPC Object URL: Registered new channel ${channel}.`); diff --git a/src/vs/platform/state/node/state.ts b/src/vs/platform/state/node/state.ts index 9fd896f914c..2d8bc904840 100644 --- a/src/vs/platform/state/node/state.ts +++ b/src/vs/platform/state/node/state.ts @@ -14,6 +14,7 @@ export interface IStateService { getItem(key: string, defaultValue?: T): T | undefined; setItem(key: string, data?: object | string | number | boolean | undefined | null): void; + setItems(items: readonly { key: string, data?: object | string | number | boolean | undefined | null }[]): void; removeItem(key: string): void; } diff --git a/src/vs/platform/state/node/stateService.ts b/src/vs/platform/state/node/stateService.ts index 3799c66d4b0..4f12dc3122d 100644 --- a/src/vs/platform/state/node/stateService.ts +++ b/src/vs/platform/state/node/stateService.ts @@ -3,20 +3,20 @@ * Licensed under the MIT License. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ -import * as path from 'vs/base/common/path'; -import * as fs from 'fs'; +import { join } from 'vs/base/common/path'; +import { readFileSync, promises } from 'fs'; import { INativeEnvironmentService } from 'vs/platform/environment/common/environment'; import { writeFileSync } from 'vs/base/node/pfs'; import { isUndefined, isUndefinedOrNull } from 'vs/base/common/types'; import { IStateService } from 'vs/platform/state/node/state'; import { ILogService } from 'vs/platform/log/common/log'; -type StorageDatabase = { [key: string]: any; }; +type StorageDatabase = { [key: string]: unknown; }; export class FileStorage { - private _database: StorageDatabase | null = null; - private lastFlushedSerializedDatabase: string | null = null; + private _database: StorageDatabase | undefined = undefined; + private lastFlushedSerializedDatabase: string | undefined = undefined; constructor(private dbPath: string, private onError: (error: Error) => void) { } @@ -44,7 +44,7 @@ export class FileStorage { private loadSync(): StorageDatabase { try { - this.lastFlushedSerializedDatabase = fs.readFileSync(this.dbPath).toString(); + this.lastFlushedSerializedDatabase = readFileSync(this.dbPath).toString(); return JSON.parse(this.lastFlushedSerializedDatabase); } catch (error) { @@ -58,7 +58,7 @@ export class FileStorage { private async loadAsync(): Promise { try { - this.lastFlushedSerializedDatabase = (await fs.promises.readFile(this.dbPath)).toString(); + this.lastFlushedSerializedDatabase = (await promises.readFile(this.dbPath)).toString(); return JSON.parse(this.lastFlushedSerializedDatabase); } catch (error) { @@ -78,7 +78,7 @@ export class FileStorage { return defaultValue; } - return res; + return res as T; } setItem(key: string, data?: object | string | number | boolean | undefined | null): void { @@ -99,6 +99,35 @@ export class FileStorage { this.saveSync(); } + setItems(items: readonly { key: string, data?: object | string | number | boolean | undefined | null }[]): void { + let save = false; + + for (const { key, data } of items) { + + // Remove items when they are undefined or null + if (isUndefinedOrNull(data)) { + this.database[key] = undefined; + save = true; + } + + // Otherwise set items if changed + else { + if (typeof data === 'string' || typeof data === 'number' || typeof data === 'boolean') { + if (this.database[key] === data) { + continue; // Shortcut for primitives that did not change + } + } + + this.database[key] = data; + save = true; + } + } + + if (save) { + this.saveSync(); + } + } + removeItem(key: string): void { // Only update if the key is actually present (not undefined) @@ -135,7 +164,7 @@ export class StateService implements IStateService { @INativeEnvironmentService environmentService: INativeEnvironmentService, @ILogService logService: ILogService ) { - this.fileStorage = new FileStorage(path.join(environmentService.userDataPath, StateService.STATE_FILE), error => logService.error(error)); + this.fileStorage = new FileStorage(join(environmentService.userDataPath, StateService.STATE_FILE), error => logService.error(error)); } init(): Promise { @@ -152,6 +181,10 @@ export class StateService implements IStateService { this.fileStorage.setItem(key, data); } + setItems(items: readonly { key: string, data?: object | string | number | boolean | undefined | null }[]): void { + this.fileStorage.setItems(items); + } + removeItem(key: string): void { this.fileStorage.removeItem(key); } diff --git a/src/vs/platform/state/test/node/state.test.ts b/src/vs/platform/state/test/node/state.test.ts index 01d2b4e3942..c0ed56602d2 100644 --- a/src/vs/platform/state/test/node/state.test.ts +++ b/src/vs/platform/state/test/node/state.test.ts @@ -53,5 +53,33 @@ flakySuite('StateService', () => { service.setItem('some.null.key', null); assert.strictEqual(service.getItem('some.null.key', 'some.default'), 'some.default'); + + service.setItems([ + { key: 'some.setItems.key1', data: 'some.value' }, + { key: 'some.setItems.key2', data: 0 }, + { key: 'some.setItems.key3', data: true }, + { key: 'some.setItems.key4', data: null }, + { key: 'some.setItems.key5', data: undefined } + ]); + + assert.strictEqual(service.getItem('some.setItems.key1'), 'some.value'); + assert.strictEqual(service.getItem('some.setItems.key2'), 0); + assert.strictEqual(service.getItem('some.setItems.key3'), true); + assert.strictEqual(service.getItem('some.setItems.key4'), undefined); + assert.strictEqual(service.getItem('some.setItems.key5'), undefined); + + service.setItems([ + { key: 'some.setItems.key1', data: undefined }, + { key: 'some.setItems.key2', data: undefined }, + { key: 'some.setItems.key3', data: undefined }, + { key: 'some.setItems.key4', data: null }, + { key: 'some.setItems.key5', data: undefined } + ]); + + assert.strictEqual(service.getItem('some.setItems.key1'), undefined); + assert.strictEqual(service.getItem('some.setItems.key2'), undefined); + assert.strictEqual(service.getItem('some.setItems.key3'), undefined); + assert.strictEqual(service.getItem('some.setItems.key4'), undefined); + assert.strictEqual(service.getItem('some.setItems.key5'), undefined); }); }); diff --git a/src/vs/platform/windows/electron-main/window.ts b/src/vs/platform/windows/electron-main/window.ts index 103f6a6dad2..1a1ddef51b7 100644 --- a/src/vs/platform/windows/electron-main/window.ts +++ b/src/vs/platform/windows/electron-main/window.ts @@ -432,7 +432,7 @@ export class CodeWindow extends Disposable implements ICodeWindow { }); // Block all SVG requests from unsupported origins - const supportedSvgSchemes = new Set([Schemas.file, Schemas.vscodeFileResource, Schemas.vscodeRemoteResource, 'devtools']); // TODO: handle webview origin + const supportedSvgSchemes = new Set([Schemas.file, Schemas.vscodeFileResource, Schemas.vscodeRemoteResource, 'devtools']); // TODO@mjbvz: handle webview origin // But allow them if the are made from inside an webview const isSafeFrame = (requestFrame: WebFrameMain | undefined): boolean => { @@ -659,8 +659,8 @@ export class CodeWindow extends Disposable implements ICodeWindow { } private destroyWindow(): void { - this._onDidDestroy.fire(); // 'close' event will not be fired on destroy(), so signal crash via explicit event - this._win.destroy(); // make sure to destroy the window as it has crashed + this._onDidDestroy.fire(); // 'close' event will not be fired on destroy(), so signal crash via explicit event + this._win.destroy(); // make sure to destroy the window as it has crashed } private onDidDeleteUntitledWorkspace(workspace: IWorkspaceIdentifier): void {