From 3bf3f69a08209bedac44dffdc05a720e16e7fefd Mon Sep 17 00:00:00 2001 From: Benjamin Pasero Date: Wed, 14 Jun 2017 15:53:13 +0200 Subject: [PATCH] multiroot - get rid of workspace.uid (only used for local storage) --- .../browser/standalone/simpleServices.ts | 5 +- .../platform/storage/common/storageService.ts | 15 +-- .../storage/test/storageService.test.ts | 7 +- src/vs/platform/workspace/common/workspace.ts | 15 +-- .../workspace/test/common/testWorkspace.ts | 1 - src/vs/workbench/electron-browser/main.ts | 125 +++++++++++------- src/vs/workbench/electron-browser/shell.ts | 22 +-- .../configuration/node/configuration.ts | 2 +- .../workbench/test/workbenchTestServices.ts | 5 +- 9 files changed, 101 insertions(+), 96 deletions(-) diff --git a/src/vs/editor/browser/standalone/simpleServices.ts b/src/vs/editor/browser/standalone/simpleServices.ts index c063ec34f7b..7d6d9ca4fa2 100644 --- a/src/vs/editor/browser/standalone/simpleServices.ts +++ b/src/vs/editor/browser/standalone/simpleServices.ts @@ -37,6 +37,7 @@ import { ResolvedKeybinding, Keybinding, createKeybinding, SimpleKeybinding } fr import { ResolvedKeybindingItem } from 'vs/platform/keybinding/common/resolvedKeybindingItem'; import { OS } from 'vs/base/common/platform'; import { IRange } from 'vs/editor/common/core/range'; +import { generateUuid } from "vs/base/common/uuid"; export class SimpleEditor implements IEditor { @@ -499,9 +500,11 @@ export class SimpleWorkspaceContextService implements IWorkspaceContextService { public readonly onDidChangeWorkspaceRoots: Event = this._onDidChangeWorkspaceRoots.event; private readonly folders: URI[]; + private readonly id: string; constructor(private workspace?: Workspace) { this.folders = workspace ? [workspace.resource] : []; + this.id = generateUuid(); } public getFolders(): URI[] { @@ -513,7 +516,7 @@ export class SimpleWorkspaceContextService implements IWorkspaceContextService { } public getWorkspace2(): IWorkspace2 { - return this.workspace ? { id: `${this.workspace.uid}`, roots: [this.workspace.resource] } : void 0; + return this.workspace ? { id: `${this.id}`, roots: [this.workspace.resource] } : void 0; } public hasWorkspace(): boolean { diff --git a/src/vs/platform/storage/common/storageService.ts b/src/vs/platform/storage/common/storageService.ts index 82952ee5dbd..83049e9736f 100644 --- a/src/vs/platform/storage/common/storageService.ts +++ b/src/vs/platform/storage/common/storageService.ts @@ -23,7 +23,6 @@ export interface IStorage { export interface IWorkspaceStorageIdentifier { resource: URI; uid?: number; - name?: string; } export class StorageService implements IStorageService { @@ -55,7 +54,7 @@ export class StorageService implements IStorageService { // Make sure to delete all workspace storage if the workspace has been recreated meanwhile // which is only possible if a UID property is provided that we can check on if (workspaceIdentifier && types.isNumber(workspaceIdentifier.uid)) { - this.cleanupWorkspaceScope(workspaceIdentifier.uid, workspaceIdentifier.name); + this.cleanupWorkspaceScope(workspaceIdentifier.uid); } } @@ -76,13 +75,13 @@ export class StorageService implements IStorageService { return `${strings.rtrim(workspaceIdStr, '/')}/`; } - private cleanupWorkspaceScope(workspaceId: number, workspaceName: string): void { + private cleanupWorkspaceScope(workspaceUid: number): void { // Get stored identifier from storage const id = this.getInteger(StorageService.WORKSPACE_IDENTIFIER, StorageScope.WORKSPACE); // If identifier differs, assume the workspace got recreated and thus clean all storage for this workspace - if (types.isNumber(id) && workspaceId !== id) { + if (types.isNumber(id) && workspaceUid !== id) { const keyPrefix = this.toStorageKey('', StorageScope.WORKSPACE); const toDelete: string[] = []; const length = this.workspaceStorage.length; @@ -99,10 +98,6 @@ export class StorageService implements IStorageService { } } - if (toDelete.length > 0) { - console.warn('Clearing previous version of local storage for workspace ', workspaceName); - } - // Run the delete toDelete.forEach((keyToDelete) => { this.workspaceStorage.removeItem(keyToDelete); @@ -110,8 +105,8 @@ export class StorageService implements IStorageService { } // Store workspace identifier now - if (workspaceId !== id) { - this.store(StorageService.WORKSPACE_IDENTIFIER, workspaceId, StorageScope.WORKSPACE); + if (workspaceUid !== id) { + this.store(StorageService.WORKSPACE_IDENTIFIER, workspaceUid, StorageScope.WORKSPACE); } } diff --git a/src/vs/platform/storage/test/storageService.test.ts b/src/vs/platform/storage/test/storageService.test.ts index 2be5dff0a16..2978c54f134 100644 --- a/src/vs/platform/storage/test/storageService.test.ts +++ b/src/vs/platform/storage/test/storageService.test.ts @@ -77,7 +77,9 @@ suite('Workbench StorageSevice', () => { test('StorageSevice cleans up when workspace changes', () => { let storageImpl = new InMemoryLocalStorage(); - let s = new StorageService(storageImpl, null, contextService.getWorkspace()); + let ws = contextService.getWorkspace(); + ws.uid = new Date().getTime(); + let s = new StorageService(storageImpl, null, ws); s.store('key1', 'foobar'); s.store('key2', 'something'); @@ -93,7 +95,8 @@ suite('Workbench StorageSevice', () => { assert.strictEqual(s.get('wkey1', StorageScope.WORKSPACE), 'foo'); assert.strictEqual(s.get('wkey2', StorageScope.WORKSPACE), 'foo2'); - let ws: any = new Workspace(TestWorkspace.resource, new Date().getTime() + 100, TestWorkspace.name); + ws = new Workspace(TestWorkspace.resource, TestWorkspace.name); + ws.uid = new Date().getTime() + 100; s = new StorageService(storageImpl, null, ws); assert.strictEqual(s.get('key1', StorageScope.GLOBAL), 'foobar'); diff --git a/src/vs/platform/workspace/common/workspace.ts b/src/vs/platform/workspace/common/workspace.ts index 8e1b08ed143..b133e6aea4e 100644 --- a/src/vs/platform/workspace/common/workspace.ts +++ b/src/vs/platform/workspace/common/workspace.ts @@ -65,13 +65,6 @@ export interface IWorkspace { */ resource: URI; - /** - * the unique identifier of the workspace. if the workspace is deleted and recreated - * the identifier also changes. this makes the uid more unique compared to the id which - * is just derived from the workspace name. - */ - uid?: number; - /** * the name of the workspace */ @@ -94,17 +87,13 @@ export interface IWorkspace2 { export class Workspace implements IWorkspace { - constructor(private _resource: URI, private _uid?: number, private _name?: string) { + constructor(private _resource: URI, private _name?: string) { } public get resource(): URI { return this._resource; } - public get uid(): number { - return this._uid; - } - public get name(): string { return this._name; } @@ -134,6 +123,6 @@ export class Workspace implements IWorkspace { } public toJSON() { - return { resource: this._resource, uid: this._uid, name: this._name }; + return { resource: this._resource, name: this._name }; } } \ No newline at end of file diff --git a/src/vs/platform/workspace/test/common/testWorkspace.ts b/src/vs/platform/workspace/test/common/testWorkspace.ts index 18926c9330d..4400e7544ea 100644 --- a/src/vs/platform/workspace/test/common/testWorkspace.ts +++ b/src/vs/platform/workspace/test/common/testWorkspace.ts @@ -8,6 +8,5 @@ import URI from 'vs/base/common/uri'; export const TestWorkspace = new Workspace( URI.file('C:\\testWorkspace'), - Date.now(), 'Test Workspace' ); diff --git a/src/vs/workbench/electron-browser/main.ts b/src/vs/workbench/electron-browser/main.ts index cf9c5e45db7..cf8ae033c6d 100644 --- a/src/vs/workbench/electron-browser/main.ts +++ b/src/vs/workbench/electron-browser/main.ts @@ -28,6 +28,9 @@ import { IInitData } from 'vs/workbench/services/timer/common/timerService'; import { TimerService } from 'vs/workbench/services/timer/node/timerService'; import { KeyboardMapperFactory } from "vs/workbench/services/keybinding/electron-browser/keybindingService"; import { IWindowConfiguration, IPath } from "vs/platform/windows/common/windows"; +import { IStorageService } from "vs/platform/storage/common/storage"; +import { IEnvironmentService } from "vs/platform/environment/common/environment"; +import { StorageService, inMemoryLocalStorageInstance, IWorkspaceStorageIdentifier } from "vs/platform/storage/common/storageService"; import { webFrame } from 'electron'; @@ -62,12 +65,8 @@ export function startup(configuration: IWindowConfiguration): TPromise { filesToDiff }; - // Resolve workspace - return getWorkspace(configuration.workspacePath).then(workspace => { - - // Open workbench - return openWorkbench(configuration, workspace, shellOptions); - }); + // Open workbench + return openWorkbench(configuration, shellOptions); } function toInputs(paths: IPath[], isUntitledFile?: boolean): IResourceInput[] { @@ -95,12 +94,53 @@ function toInputs(paths: IPath[], isUntitledFile?: boolean): IResourceInput[] { }); } -function getWorkspace(workspacePath: string): TPromise { - if (!workspacePath) { +function openWorkbench(configuration: IWindowConfiguration, options: IOptions): TPromise { + return getWorkspace(configuration).then(workspace => { + const environmentService = new EnvironmentService(configuration, configuration.execPath); + const workspaceConfigurationService = new WorkspaceConfigurationService(environmentService, workspace); + const timerService = new TimerService((window).MonacoEnvironment.timers as IInitData, !workspaceConfigurationService.hasWorkspace()); + + return createStorageService(configuration, environmentService).then(storageService => { + + // Since the configuration service is one of the core services that is used in so many places, we initialize it + // right before startup of the workbench shell to have its data ready for consumers + return workspaceConfigurationService.initialize().then(() => { + timerService.beforeDOMContentLoaded = Date.now(); + + return domContentLoaded().then(() => { + timerService.afterDOMContentLoaded = Date.now(); + + // Open Shell + timerService.beforeWorkbenchOpen = Date.now(); + const shell = new WorkbenchShell(document.body, { + contextService: workspaceConfigurationService, + configurationService: workspaceConfigurationService, + environmentService, + timerService, + storageService + }, configuration, options); + shell.open(); + + // Inform user about loading issues from the loader + (self).require.config({ + onError: (err: any) => { + if (err.errorCode === 'load') { + shell.onUnexpectedError(loaderError(err)); + } + } + }); + }); + }); + }); + }); +} + +function getWorkspace(configuration: IWindowConfiguration): TPromise { + if (!configuration.workspacePath) { return TPromise.as(null); } - return realpath(workspacePath).then(realWorkspacePath => { + return realpath(configuration.workspacePath).then(realWorkspacePath => { // for some weird reason, node adds a trailing slash to UNC paths // we never ever want trailing slashes as our workspace path unless @@ -110,16 +150,13 @@ function getWorkspace(workspacePath: string): TPromise { realWorkspacePath = strings.rtrim(realWorkspacePath, paths.nativeSep); } + // update config + configuration.workspacePath = realWorkspacePath; + const workspaceResource = uri.file(realWorkspacePath); const folderName = path.basename(realWorkspacePath) || realWorkspacePath; - return stat(realWorkspacePath).then(folderStat => { - return new Workspace( - workspaceResource, - platform.isLinux ? folderStat.ino : folderStat.birthtime.getTime(), - folderName // On Linux, birthtime is ctime, so we cannot use it! We use the ino instead! - ); - }); + return new Workspace(workspaceResource, folderName); }, error => { errors.onUnexpectedError(error); @@ -127,38 +164,30 @@ function getWorkspace(workspacePath: string): TPromise { }); } -function openWorkbench(configuration: IWindowConfiguration, workspace: Workspace, options: IOptions): TPromise { - const environmentService = new EnvironmentService(configuration, configuration.execPath); - const workspaceConfigurationService = new WorkspaceConfigurationService(environmentService, workspace); - const timerService = new TimerService((window).MonacoEnvironment.timers as IInitData, !workspaceConfigurationService.hasWorkspace()); - - // Since the configuration service is one of the core services that is used in so many places, we initialize it - // right before startup of the workbench shell to have its data ready for consumers - return workspaceConfigurationService.initialize().then(() => { - timerService.beforeDOMContentLoaded = Date.now(); - - return domContentLoaded().then(() => { - timerService.afterDOMContentLoaded = Date.now(); - - // Open Shell - timerService.beforeWorkbenchOpen = Date.now(); - const shell = new WorkbenchShell(document.body, { - contextService: workspaceConfigurationService, - configurationService: workspaceConfigurationService, - environmentService, - timerService - }, configuration, options); - shell.open(); - - // Inform user about loading issues from the loader - (self).require.config({ - onError: (err: any) => { - if (err.errorCode === 'load') { - shell.onUnexpectedError(loaderError(err)); - } - } - }); - }); +function createStorageService(configuration: IWindowConfiguration, environmentService: IEnvironmentService): TPromise { + let workspaceStatPromise: TPromise = TPromise.as(null); + if (configuration.workspacePath) { + workspaceStatPromise = stat(configuration.workspacePath); + } + + return workspaceStatPromise.then(stat => { + let id: IWorkspaceStorageIdentifier; + if (stat) { + id = { resource: uri.file(configuration.workspacePath), uid: platform.isLinux ? stat.ino : stat.birthtime.getTime() }; + } else if (configuration.backupPath) { + // if we do not have a workspace open, we need to find another identifier for the window to store + // workspace UI state. if we have a backup path in the configuration we can use that because this + // will be a unique identifier per window that is stable between restarts as long as there are + // dirty files in the workspace. + // We use basename() to produce a short identifier, we do not need the full path. We use a custom + // scheme so that we can later distinguish these identifiers from the workspace one. + id = { resource: uri.from({ path: path.basename(configuration.backupPath), scheme: 'empty' }) }; + } + + const disableStorage = !!environmentService.extensionTestsPath; // never keep any state when running extension tests! + const storage = disableStorage ? inMemoryLocalStorageInstance : window.localStorage; + + return new StorageService(storage, storage, id); }); } diff --git a/src/vs/workbench/electron-browser/shell.ts b/src/vs/workbench/electron-browser/shell.ts index 141b3285c0a..015cc5f7bbd 100644 --- a/src/vs/workbench/electron-browser/shell.ts +++ b/src/vs/workbench/electron-browser/shell.ts @@ -21,7 +21,6 @@ import { SyncDescriptor } from 'vs/platform/instantiation/common/descriptors'; import pkg from 'vs/platform/node/package'; import { ContextViewService } from 'vs/platform/contextview/browser/contextViewService'; import { Workbench, IWorkbenchStartedInfo } from 'vs/workbench/electron-browser/workbench'; -import { StorageService, inMemoryLocalStorageInstance } from 'vs/platform/storage/common/storageService'; import { ITelemetryService } from 'vs/platform/telemetry/common/telemetry'; import { NullTelemetryService, configurationTelemetry, loadExperiments, lifecycleTelemetry } from 'vs/platform/telemetry/common/telemetryUtils'; import { ITelemetryAppenderChannel, TelemetryAppenderClient } from 'vs/platform/telemetry/common/telemetryIpc'; @@ -89,8 +88,6 @@ import { IURLService } from 'vs/platform/url/common/url'; import { ExtensionHostProcessWorker } from 'vs/workbench/electron-browser/extensionHost'; import { ITimerService } from 'vs/workbench/services/timer/common/timerService'; import { remote, ipcRenderer as ipc } from 'electron'; -import URI from "vs/base/common/uri"; -import { basename } from "path"; import { ITextMateService } from 'vs/editor/node/textMate/textMateService'; import { MainProcessTextMateSyntax } from 'vs/editor/electron-browser/textMate/TMSyntax'; import { BareFontInfo } from 'vs/editor/common/config/fontInfo'; @@ -110,6 +107,7 @@ export interface ICoreServices { configurationService: IConfigurationService; environmentService: IEnvironmentService; timerService: ITimerService; + storageService: IStorageService; } const currentWindow = remote.getCurrentWindow(); @@ -155,6 +153,7 @@ export class WorkbenchShell { this.configurationService = services.configurationService; this.environmentService = services.environmentService; this.timerService = services.timerService; + this.storageService = services.storageService; this.toUnbind = []; this.previousErrorTime = 0; @@ -251,6 +250,7 @@ export class WorkbenchShell { serviceCollection.set(IConfigurationService, this.configurationService); serviceCollection.set(IEnvironmentService, this.environmentService); serviceCollection.set(ITimerService, this.timerService); + serviceCollection.set(IStorageService, this.storageService); const instantiationService: IInstantiationService = new InstantiationService(serviceCollection, true); @@ -273,22 +273,6 @@ export class WorkbenchShell { sharedProcess .done(client => client.registerChannel('choice', instantiationService.createInstance(ChoiceChannel))); - // Storage Sevice - let workspaceIdentifier = this.contextService.getWorkspace(); - if (!workspaceIdentifier && !!this.configuration.backupPath) { - // if we do not have a workspace open, we need to find another identifier for the window to store - // workspace UI state. if we have a backup path in the configuration we can use that because this - // will be a unique identifier per window that is stable between restarts as long as there are - // dirty files in the workspace. - // We use basename() to produce a short identifier, we do not need the full path. We use a custom - // scheme so that we can later distinguish these identifiers from the workspace one. - workspaceIdentifier = { resource: URI.from({ path: basename(this.configuration.backupPath), scheme: 'empty' }) }; - } - const disableStorage = !!this.environmentService.extensionTestsPath; // never keep any state when running extension tests! - const storage = disableStorage ? inMemoryLocalStorageInstance : window.localStorage; - this.storageService = new StorageService(storage, storage, workspaceIdentifier); - serviceCollection.set(IStorageService, this.storageService); - // Warm up font cache information before building up too many dom elements restoreFontInfo(this.storageService); readFontInfo(BareFontInfo.createFromRawSettings(this.configurationService.getConfiguration('editor'), browser.getZoomLevel())); diff --git a/src/vs/workbench/services/configuration/node/configuration.ts b/src/vs/workbench/services/configuration/node/configuration.ts index 318ffc68d59..27661ffa4cb 100644 --- a/src/vs/workbench/services/configuration/node/configuration.ts +++ b/src/vs/workbench/services/configuration/node/configuration.ts @@ -80,7 +80,7 @@ export class WorkspaceConfigurationService extends Disposable implements IWorksp constructor(private environmentService: IEnvironmentService, private singleRootWorkspace?: SingleRootWorkspace, private workspaceSettingsRootFolder: string = WORKSPACE_CONFIG_FOLDER_DEFAULT_NAME) { super(); - this.workspace = singleRootWorkspace ? new Workspace(`${singleRootWorkspace.uid}`, [singleRootWorkspace.resource]) : null; + this.workspace = singleRootWorkspace ? new Workspace(singleRootWorkspace.resource.toString(), [singleRootWorkspace.resource]) : null; this.workspaceFilePathToConfiguration = Object.create(null); this.cachedConfig = new ConfigModel(null); diff --git a/src/vs/workbench/test/workbenchTestServices.ts b/src/vs/workbench/test/workbenchTestServices.ts index dc3c013b404..d38d290f463 100644 --- a/src/vs/workbench/test/workbenchTestServices.ts +++ b/src/vs/workbench/test/workbenchTestServices.ts @@ -53,6 +53,7 @@ import { IEnvironmentService } from 'vs/platform/environment/common/environment' import { IThemeService, ITheme, DARK } from 'vs/platform/theme/common/themeService'; import { Color } from 'vs/base/common/color'; import { isLinux } from 'vs/base/common/platform'; +import { generateUuid } from "vs/base/common/uuid"; export function createFileInput(instantiationService: IInstantiationService, resource: URI): FileEditorInput { return instantiationService.createInstance(FileEditorInput, resource, void 0); @@ -64,12 +65,14 @@ export class TestContextService implements IWorkspaceContextService { public _serviceBrand: any; private workspace: any; + private id: string; private options: any; private _onDidChangeWorkspaceRoots: Emitter; constructor(workspace: any = TestWorkspace, options: any = null) { this.workspace = workspace; + this.id = generateUuid(); this.options = options || Object.create(null); this._onDidChangeWorkspaceRoots = new Emitter(); } @@ -91,7 +94,7 @@ export class TestContextService implements IWorkspaceContextService { } public getWorkspace2(): IWorkspace2 { - return this.workspace ? { id: `${this.workspace.uid}`, roots: [this.workspace.resource] } : void 0; + return this.workspace ? { id: this.id, roots: [this.workspace.resource] } : void 0; } public setWorkspace(workspace: any): void { -- GitLab