From 8f6c44653365edad2e5e2db3ddb1b8084f21e893 Mon Sep 17 00:00:00 2001 From: Sandeep Somavarapu Date: Tue, 6 Mar 2018 12:11:39 +0100 Subject: [PATCH] #45122 - Use ILocalExtension as an input in IExtensionEnablementService --- .../common/extensionEnablementService.ts | 63 +++++++++---------- .../common/extensionManagement.ts | 6 +- .../common/extensionEnablementService.test.ts | 28 ++++----- .../extensions/browser/extensionsActions.ts | 2 +- .../node/extensionsWorkbenchService.ts | 8 +-- .../electron-browser/preferencesSearch.ts | 2 +- .../electron-browser/extensionService.ts | 1 - .../electron-browser/workbenchIssueService.ts | 3 +- 8 files changed, 52 insertions(+), 61 deletions(-) diff --git a/src/vs/platform/extensionManagement/common/extensionEnablementService.ts b/src/vs/platform/extensionManagement/common/extensionEnablementService.ts index 1a787fc24c1..4e4fbc25a3d 100644 --- a/src/vs/platform/extensionManagement/common/extensionEnablementService.ts +++ b/src/vs/platform/extensionManagement/common/extensionEnablementService.ts @@ -5,11 +5,10 @@ import { localize } from 'vs/nls'; import { TPromise } from 'vs/base/common/winjs.base'; -import { distinct, coalesce } from 'vs/base/common/arrays'; import Event, { Emitter } from 'vs/base/common/event'; import { IDisposable, dispose } from 'vs/base/common/lifecycle'; -import { IExtensionManagementService, DidUninstallExtensionEvent, IExtensionEnablementService, IExtensionIdentifier, EnablementState, ILocalExtension, isIExtensionIdentifier } from 'vs/platform/extensionManagement/common/extensionManagement'; -import { adoptToGalleryExtensionId, getIdFromLocalExtensionId, areSameExtensions, getGalleryExtensionIdFromLocal } from 'vs/platform/extensionManagement/common/extensionManagementUtil'; +import { IExtensionManagementService, DidUninstallExtensionEvent, IExtensionEnablementService, IExtensionIdentifier, EnablementState, ILocalExtension, isIExtensionIdentifier, LocalExtensionType } from 'vs/platform/extensionManagement/common/extensionManagement'; +import { getIdFromLocalExtensionId, areSameExtensions, getGalleryExtensionIdFromLocal } from 'vs/platform/extensionManagement/common/extensionManagementUtil'; import { IWorkspaceContextService, WorkbenchState } from 'vs/platform/workspace/common/workspace'; import { IStorageService, StorageScope } from 'vs/platform/storage/common/storage'; import { IEnvironmentService } from 'vs/platform/environment/common/environment'; @@ -58,10 +57,11 @@ export class ExtensionEnablementService implements IExtensionEnablementService { return TPromise.as(result); } - getEnablementState(identifier: IExtensionIdentifier): EnablementState { - if (this.environmentService.disableExtensions) { + getEnablementState(extension: ILocalExtension): EnablementState { + if (this.environmentService.disableExtensions && extension.type === LocalExtensionType.User) { return EnablementState.Disabled; } + const identifier = this._getIdentifier(extension); if (this.hasWorkspace) { if (this._getEnabledExtensions(StorageScope.WORKSPACE).filter(e => areSameExtensions(e, identifier))[0]) { return EnablementState.WorkspaceEnabled; @@ -82,14 +82,14 @@ export class ExtensionEnablementService implements IExtensionEnablementService { } setEnablement(arg: ILocalExtension | IExtensionIdentifier, newState: EnablementState): TPromise { - let identifier; + let identifier: IExtensionIdentifier; if (isIExtensionIdentifier(arg)) { identifier = arg; } else { if (!this.canChangeEnablement(arg)) { return TPromise.wrap(false); } - identifier = { id: getGalleryExtensionIdFromLocal(arg), uuid: arg.identifier.uuid }; + identifier = this._getIdentifier(arg); } const workspace = newState === EnablementState.WorkspaceDisabled || newState === EnablementState.WorkspaceEnabled; @@ -97,7 +97,7 @@ export class ExtensionEnablementService implements IExtensionEnablementService { return TPromise.wrapError(new Error(localize('noWorkspace', "No workspace."))); } - const currentState = this.getEnablementState(identifier); + const currentState = this._getEnablementState(identifier); if (currentState === newState) { return TPromise.as(false); @@ -123,16 +123,29 @@ export class ExtensionEnablementService implements IExtensionEnablementService { return TPromise.as(true); } - isEnabled(identifier: IExtensionIdentifier): boolean { - const enablementState = this.getEnablementState(identifier); + isEnabled(extension: ILocalExtension): boolean { + const enablementState = this.getEnablementState(extension); return enablementState === EnablementState.WorkspaceEnabled || enablementState === EnablementState.Enabled; } - migrateToIdentifiers(installed: IExtensionIdentifier[]): void { - this._migrateDisabledExtensions(installed, StorageScope.GLOBAL); + private _getEnablementState(identifier: IExtensionIdentifier): EnablementState { if (this.hasWorkspace) { - this._migrateDisabledExtensions(installed, StorageScope.WORKSPACE); + if (this._getEnabledExtensions(StorageScope.WORKSPACE).filter(e => areSameExtensions(e, identifier))[0]) { + return EnablementState.WorkspaceEnabled; + } + + if (this._getDisabledExtensions(StorageScope.WORKSPACE).filter(e => areSameExtensions(e, identifier))[0]) { + return EnablementState.WorkspaceDisabled; + } + } + if (this._getDisabledExtensions(StorageScope.GLOBAL).filter(e => areSameExtensions(e, identifier))[0]) { + return EnablementState.Disabled; } + return EnablementState.Enabled; + } + + private _getIdentifier(extension: ILocalExtension): IExtensionIdentifier { + return { id: getGalleryExtensionIdFromLocal(extension), uuid: extension.identifier.uuid }; } private _enableExtension(identifier: IExtensionIdentifier): void { @@ -227,8 +240,8 @@ export class ExtensionEnablementService implements IExtensionEnablementService { return this._getExtensions(DISABLED_EXTENSIONS_STORAGE_PATH, scope); } - private _setDisabledExtensions(disabledExtensions: IExtensionIdentifier[], scope: StorageScope, extension: IExtensionIdentifier, fireEvent = true): void { - this._setExtensions(DISABLED_EXTENSIONS_STORAGE_PATH, disabledExtensions, scope, extension, fireEvent); + private _setDisabledExtensions(disabledExtensions: IExtensionIdentifier[], scope: StorageScope, extension: IExtensionIdentifier): void { + this._setExtensions(DISABLED_EXTENSIONS_STORAGE_PATH, disabledExtensions, scope, extension); } private _getExtensions(storageId: string, scope: StorageScope): IExtensionIdentifier[] { @@ -239,30 +252,12 @@ export class ExtensionEnablementService implements IExtensionEnablementService { return value ? JSON.parse(value) : []; } - private _setExtensions(storageId: string, extensions: IExtensionIdentifier[], scope: StorageScope, extension: IExtensionIdentifier, fireEvent = true): void { + private _setExtensions(storageId: string, extensions: IExtensionIdentifier[], scope: StorageScope, extension: IExtensionIdentifier): void { if (extensions.length) { this.storageService.store(storageId, JSON.stringify(extensions.map(({ id, uuid }) => ({ id, uuid }))), scope); } else { this.storageService.remove(storageId, scope); } - if (fireEvent) { - this._onEnablementChanged.fire(extension); - } - } - - private _migrateDisabledExtensions(installedExtensions: IExtensionIdentifier[], scope: StorageScope): void { - const oldValue = this.storageService.get('extensions/disabled', scope, ''); - if (oldValue) { - const extensionIdentifiers = coalesce(distinct(oldValue.split(',')).map(id => { - id = adoptToGalleryExtensionId(id); - const matched = installedExtensions.filter(installed => areSameExtensions({ id }, { id: installed.id }))[0]; - return matched ? { id: matched.id, uuid: matched.uuid } : null; - })); - if (extensionIdentifiers.length) { - this.storageService.store(DISABLED_EXTENSIONS_STORAGE_PATH, JSON.stringify(extensionIdentifiers), scope); - } - } - this.storageService.remove('extensions/disabled', scope); } private _onDidUninstallExtension({ identifier, error }: DidUninstallExtensionEvent): void { diff --git a/src/vs/platform/extensionManagement/common/extensionManagement.ts b/src/vs/platform/extensionManagement/common/extensionManagement.ts index ba288ea5a04..70b424c6ca9 100644 --- a/src/vs/platform/extensionManagement/common/extensionManagement.ts +++ b/src/vs/platform/extensionManagement/common/extensionManagement.ts @@ -308,7 +308,7 @@ export interface IExtensionEnablementService { /** * Returns the enablement state for the given extension */ - getEnablementState(identifier: IExtensionIdentifier): EnablementState; + getEnablementState(extension: ILocalExtension): EnablementState; /** * Returns `true` if the enablement can be changed. @@ -318,7 +318,7 @@ export interface IExtensionEnablementService { /** * Returns `true` if the given extension identifier is enabled. */ - isEnabled(identifier: IExtensionIdentifier): boolean; + isEnabled(extension: ILocalExtension): boolean; /** * Enable or disable the given extension. @@ -334,8 +334,6 @@ export interface IExtensionEnablementService { * TODO: @Sandy. Use setEnablement(extension: ILocalExtension, state: EnablementState): TPromise. Use one model for extension management and runtime */ setEnablement(identifier: IExtensionIdentifier, state: EnablementState): TPromise; - - migrateToIdentifiers(installed: IExtensionIdentifier[]): void; } export const IExtensionTipsService = createDecorator('extensionTipsService'); diff --git a/src/vs/platform/extensionManagement/test/common/extensionEnablementService.test.ts b/src/vs/platform/extensionManagement/test/common/extensionEnablementService.test.ts index 8113bdd0f14..46f95ce498c 100644 --- a/src/vs/platform/extensionManagement/test/common/extensionEnablementService.test.ts +++ b/src/vs/platform/extensionManagement/test/common/extensionEnablementService.test.ts @@ -98,13 +98,13 @@ suite('ExtensionEnablementService Test', () => { test('test state of globally disabled extension', () => { return testObject.setEnablement(aLocalExtension('pub.a'), EnablementState.Disabled) - .then(() => assert.equal(testObject.getEnablementState({ id: 'pub.a' }), EnablementState.Disabled)); + .then(() => assert.equal(testObject.getEnablementState(aLocalExtension('pub.a')), EnablementState.Disabled)); }); test('test state of globally enabled extension', () => { return testObject.setEnablement(aLocalExtension('pub.a'), EnablementState.Disabled) .then(() => testObject.setEnablement(aLocalExtension('pub.a'), EnablementState.Enabled)) - .then(() => assert.equal(testObject.getEnablementState({ id: 'pub.a' }), EnablementState.Enabled)); + .then(() => assert.equal(testObject.getEnablementState(aLocalExtension('pub.a')), EnablementState.Enabled)); }); test('test disable an extension for workspace', () => { @@ -126,59 +126,59 @@ suite('ExtensionEnablementService Test', () => { test('test state of workspace disabled extension', () => { return testObject.setEnablement(aLocalExtension('pub.a'), EnablementState.WorkspaceDisabled) - .then(() => assert.equal(testObject.getEnablementState({ id: 'pub.a' }), EnablementState.WorkspaceDisabled)); + .then(() => assert.equal(testObject.getEnablementState(aLocalExtension('pub.a')), EnablementState.WorkspaceDisabled)); }); test('test state of workspace and globally disabled extension', () => { return testObject.setEnablement(aLocalExtension('pub.a'), EnablementState.Disabled) .then(() => testObject.setEnablement(aLocalExtension('pub.a'), EnablementState.WorkspaceDisabled)) - .then(() => assert.equal(testObject.getEnablementState({ id: 'pub.a' }), EnablementState.WorkspaceDisabled)); + .then(() => assert.equal(testObject.getEnablementState(aLocalExtension('pub.a')), EnablementState.WorkspaceDisabled)); }); test('test state of workspace enabled extension', () => { return testObject.setEnablement(aLocalExtension('pub.a'), EnablementState.WorkspaceDisabled) .then(() => testObject.setEnablement(aLocalExtension('pub.a'), EnablementState.WorkspaceEnabled)) - .then(() => assert.equal(testObject.getEnablementState({ id: 'pub.a' }), EnablementState.WorkspaceEnabled)); + .then(() => assert.equal(testObject.getEnablementState(aLocalExtension('pub.a')), EnablementState.WorkspaceEnabled)); }); test('test state of globally disabled and workspace enabled extension', () => { return testObject.setEnablement(aLocalExtension('pub.a'), EnablementState.Disabled) .then(() => testObject.setEnablement(aLocalExtension('pub.a'), EnablementState.WorkspaceDisabled)) .then(() => testObject.setEnablement(aLocalExtension('pub.a'), EnablementState.WorkspaceEnabled)) - .then(() => assert.equal(testObject.getEnablementState({ id: 'pub.a' }), EnablementState.WorkspaceEnabled)); + .then(() => assert.equal(testObject.getEnablementState(aLocalExtension('pub.a')), EnablementState.WorkspaceEnabled)); }); test('test state of an extension when disabled for workspace from workspace enabled', () => { return testObject.setEnablement(aLocalExtension('pub.a'), EnablementState.WorkspaceDisabled) .then(() => testObject.setEnablement(aLocalExtension('pub.a'), EnablementState.WorkspaceEnabled)) .then(() => testObject.setEnablement(aLocalExtension('pub.a'), EnablementState.WorkspaceDisabled)) - .then(() => assert.equal(testObject.getEnablementState({ id: 'pub.a' }), EnablementState.WorkspaceDisabled)); + .then(() => assert.equal(testObject.getEnablementState(aLocalExtension('pub.a')), EnablementState.WorkspaceDisabled)); }); test('test state of an extension when disabled globally from workspace enabled', () => { return testObject.setEnablement(aLocalExtension('pub.a'), EnablementState.WorkspaceDisabled) .then(() => testObject.setEnablement(aLocalExtension('pub.a'), EnablementState.WorkspaceEnabled)) .then(() => testObject.setEnablement(aLocalExtension('pub.a'), EnablementState.Disabled)) - .then(() => assert.equal(testObject.getEnablementState({ id: 'pub.a' }), EnablementState.Disabled)); + .then(() => assert.equal(testObject.getEnablementState(aLocalExtension('pub.a')), EnablementState.Disabled)); }); test('test state of an extension when disabled globally from workspace disabled', () => { return testObject.setEnablement(aLocalExtension('pub.a'), EnablementState.WorkspaceDisabled) .then(() => testObject.setEnablement(aLocalExtension('pub.a'), EnablementState.Disabled)) - .then(() => assert.equal(testObject.getEnablementState({ id: 'pub.a' }), EnablementState.Disabled)); + .then(() => assert.equal(testObject.getEnablementState(aLocalExtension('pub.a')), EnablementState.Disabled)); }); test('test state of an extension when enabled globally from workspace enabled', () => { return testObject.setEnablement(aLocalExtension('pub.a'), EnablementState.WorkspaceDisabled) .then(() => testObject.setEnablement(aLocalExtension('pub.a'), EnablementState.WorkspaceEnabled)) .then(() => testObject.setEnablement(aLocalExtension('pub.a'), EnablementState.Enabled)) - .then(() => assert.equal(testObject.getEnablementState({ id: 'pub.a' }), EnablementState.Enabled)); + .then(() => assert.equal(testObject.getEnablementState(aLocalExtension('pub.a')), EnablementState.Enabled)); }); test('test state of an extension when enabled globally from workspace disabled', () => { return testObject.setEnablement(aLocalExtension('pub.a'), EnablementState.WorkspaceDisabled) .then(() => testObject.setEnablement(aLocalExtension('pub.a'), EnablementState.Enabled)) - .then(() => assert.equal(testObject.getEnablementState({ id: 'pub.a' }), EnablementState.Enabled)); + .then(() => assert.equal(testObject.getEnablementState(aLocalExtension('pub.a')), EnablementState.Enabled)); }); test('test disable an extension for workspace and then globally', () => { @@ -311,18 +311,18 @@ suite('ExtensionEnablementService Test', () => { test('test isEnabled return false extension is disabled globally', () => { return testObject.setEnablement(aLocalExtension('pub.a'), EnablementState.Disabled) - .then(() => assert.ok(!testObject.isEnabled({ id: 'pub.a' }))); + .then(() => assert.ok(!testObject.isEnabled(aLocalExtension('pub.a')))); }); test('test isEnabled return false extension is disabled in workspace', () => { return testObject.setEnablement(aLocalExtension('pub.a'), EnablementState.WorkspaceDisabled) - .then(() => assert.ok(!testObject.isEnabled({ id: 'pub.a' }))); + .then(() => assert.ok(!testObject.isEnabled(aLocalExtension('pub.a')))); }); test('test isEnabled return true extension is not disabled', () => { return testObject.setEnablement(aLocalExtension('pub.a'), EnablementState.WorkspaceDisabled) .then(() => testObject.setEnablement(aLocalExtension('pub.c'), EnablementState.Disabled)) - .then(() => assert.ok(testObject.isEnabled({ id: 'pub.b' }))); + .then(() => assert.ok(testObject.isEnabled(aLocalExtension('pub.b')))); }); test('test canChangeEnablement return false for language packs', () => { diff --git a/src/vs/workbench/parts/extensions/browser/extensionsActions.ts b/src/vs/workbench/parts/extensions/browser/extensionsActions.ts index 6e300d32eed..05c57e64f98 100644 --- a/src/vs/workbench/parts/extensions/browser/extensionsActions.ts +++ b/src/vs/workbench/parts/extensions/browser/extensionsActions.ts @@ -926,7 +926,7 @@ export class ReloadAction extends Action { private computeReloadState(runningExtensions: IExtensionDescription[]): void { const isInstalled = this.extensionsWorkbenchService.local.some(e => e.id === this.extension.id); const isUninstalled = this.extension.state === ExtensionState.Uninstalled; - const isDisabled = !this.extensionEnablementService.isEnabled({ id: this.extension.id, uuid: this.extension.uuid }); + const isDisabled = this.extension.local ? !this.extensionEnablementService.isEnabled(this.extension.local) : false; const filteredExtensions = runningExtensions.filter(e => areSameExtensions(e, this.extension)); const isExtensionRunning = filteredExtensions.length > 0; diff --git a/src/vs/workbench/parts/extensions/node/extensionsWorkbenchService.ts b/src/vs/workbench/parts/extensions/node/extensionsWorkbenchService.ts index 22135b00648..c01694e75fd 100644 --- a/src/vs/workbench/parts/extensions/node/extensionsWorkbenchService.ts +++ b/src/vs/workbench/parts/extensions/node/extensionsWorkbenchService.ts @@ -419,7 +419,7 @@ export class ExtensionsWorkbenchService implements IExtensionsWorkbenchService { this.installed = result.map(local => { const extension = installedById[local.identifier.id] || new Extension(this.galleryService, this.stateProvider, local, null, this.telemetryService); extension.local = local; - extension.enablementState = this.extensionEnablementService.getEnablementState({ id: extension.id, uuid: extension.uuid }); + extension.enablementState = this.extensionEnablementService.getEnablementState(local); return extension; }); @@ -890,10 +890,10 @@ export class ExtensionsWorkbenchService implements IExtensionsWorkbenchService { this._onChange.fire(); } - private onEnablementChanged(extensionIdentifier: IExtensionIdentifier) { - const [extension] = this.local.filter(e => areSameExtensions(e, extensionIdentifier)); + private onEnablementChanged(identifier: IExtensionIdentifier) { + const [extension] = this.local.filter(e => areSameExtensions(e, identifier)); if (extension) { - const enablementState = this.extensionEnablementService.getEnablementState({ id: extension.id, uuid: extension.uuid }); + const enablementState = this.extensionEnablementService.getEnablementState(extension.local); if (enablementState !== extension.enablementState) { extension.enablementState = enablementState; this._onChange.fire(); diff --git a/src/vs/workbench/parts/preferences/electron-browser/preferencesSearch.ts b/src/vs/workbench/parts/preferences/electron-browser/preferencesSearch.ts index 17246c5d28c..aa9985fa576 100644 --- a/src/vs/workbench/parts/preferences/electron-browser/preferencesSearch.ts +++ b/src/vs/workbench/parts/preferences/electron-browser/preferencesSearch.ts @@ -44,7 +44,7 @@ export class PreferencesSearchService extends Disposable implements IPreferences this._installedExtensions = this.extensionManagementService.getInstalled(LocalExtensionType.User).then(exts => { // Filter to enabled extensions that have settings return exts - .filter(ext => this.extensionEnablementService.isEnabled(ext.identifier)) + .filter(ext => this.extensionEnablementService.isEnabled(ext)) .filter(ext => ext.manifest.contributes && ext.manifest.contributes.configuration) .filter(ext => !!ext.identifier.uuid); }); diff --git a/src/vs/workbench/services/extensions/electron-browser/extensionService.ts b/src/vs/workbench/services/extensions/electron-browser/extensionService.ts index 19c1b6a17f3..4b47d8317ea 100644 --- a/src/vs/workbench/services/extensions/electron-browser/extensionService.ts +++ b/src/vs/workbench/services/extensions/electron-browser/extensionService.ts @@ -456,7 +456,6 @@ export class ExtensionService extends Disposable implements IExtensionService { return ExtensionService._scanInstalledExtensions(this._windowService, this._choiceService, this._environmentService, log) .then(({ system, user, development }) => { - this._extensionEnablementService.migrateToIdentifiers(user); // TODO: @sandy Remove it after couple of milestones return this._extensionEnablementService.getDisabledExtensions() .then(disabledExtensions => { let result: { [extensionId: string]: IExtensionDescription; } = {}; diff --git a/src/vs/workbench/services/issue/electron-browser/workbenchIssueService.ts b/src/vs/workbench/services/issue/electron-browser/workbenchIssueService.ts index 3905d9542d1..7e78f2904a5 100644 --- a/src/vs/workbench/services/issue/electron-browser/workbenchIssueService.ts +++ b/src/vs/workbench/services/issue/electron-browser/workbenchIssueService.ts @@ -11,7 +11,6 @@ import { ITheme, IThemeService } from 'vs/platform/theme/common/themeService'; import { textLinkForeground, inputBackground, inputBorder, inputForeground, buttonBackground, buttonHoverBackground, buttonForeground, inputValidationErrorBorder, foreground, inputActiveOptionBorder, scrollbarSliderActiveBackground, scrollbarSliderBackground, scrollbarSliderHoverBackground } from 'vs/platform/theme/common/colorRegistry'; import { SIDE_BAR_BACKGROUND } from 'vs/workbench/common/theme'; import { IExtensionManagementService, IExtensionEnablementService, LocalExtensionType } from 'vs/platform/extensionManagement/common/extensionManagement'; -import { getGalleryExtensionIdFromLocal } from 'vs/platform/extensionManagement/common/extensionManagementUtil'; import { webFrame } from 'electron'; import { assign } from 'vs/base/common/objects'; import { IWorkbenchIssueService } from 'vs/workbench/services/issue/common/issue'; @@ -29,7 +28,7 @@ export class WorkbenchIssueService implements IWorkbenchIssueService { openReporter(dataOverrides: Partial = {}): TPromise { return this.extensionManagementService.getInstalled(LocalExtensionType.User).then(extensions => { - const enabledExtensions = extensions.filter(extension => this.extensionEnablementService.isEnabled({ id: getGalleryExtensionIdFromLocal(extension) })); + const enabledExtensions = extensions.filter(extension => this.extensionEnablementService.isEnabled(extension)); const theme = this.themeService.getTheme(); const issueReporterData: IssueReporterData = assign( { -- GitLab