Skip to content

Commit

Permalink
feedback on auto update by extension (#199163)
Browse files Browse the repository at this point in the history
  • Loading branch information
@sandy081
sandy081 authored Nov 27, 2023
1 parent b798fd8 commit 0e16324
Show file tree
Hide file tree
Showing 6 changed files with 46 additions and 96 deletions.
1 change: 0 additions & 1 deletion src/vs/base/common/product.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,6 @@ export interface IProductConfiguration {
readonly nlsBaseUrl: string;
};

readonly publishersByOrganisation?: IStringDictionary<string[]>;
readonly extensionRecommendations?: IStringDictionary<IExtensionRecommendations>;
readonly configBasedExtensionTips?: IStringDictionary<IConfigBasedExtensionTip>;
readonly exeBasedExtensionTips?: IStringDictionary<IExeBasedExtensionTip>;
Expand Down
2 changes: 1 addition & 1 deletion src/vs/workbench/contrib/extensions/browser/extensionEditor.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -369,7 +369,7 @@ export class ExtensionEditor extends EditorPane {
this.instantiationService.createInstance(SwitchToPreReleaseVersionAction, false),
this.instantiationService.createInstance(SwitchToReleasedVersionAction, false),
this.instantiationService.createInstance(ToggleSyncExtensionAction),
this.instantiationService.createInstance(ToggleAutoUpdateForExtensionAction, false, ['onlySelectedExtensions']),
this.instantiationService.createInstance(ToggleAutoUpdateForExtensionAction, false, [false, 'onlySelectedExtensions']),
new ExtensionEditorManageExtensionAction(this.scopedContextKeyService || this.contextKeyService, this.instantiationService),
];

Expand Down
4 changes: 2 additions & 2 deletions src/vs/workbench/contrib/extensions/browser/extensions.contribution.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1335,7 +1335,7 @@ class ExtensionsContributions extends Disposable implements IWorkbenchContributi
id: MenuId.ExtensionContext,
group: UPDATE_ACTIONS_GROUP,
order: 1,
when: ContextKeyExpr.and(ContextKeyExpr.not('inExtensionEditor'), ContextKeyExpr.equals('extensionStatus', 'installed'), ContextKeyExpr.not('isBuiltinExtension'), ContextKeyExpr.equals(`config.${AutoUpdateConfigurationKey}`, 'onlySelectedExtensions'),)
when: ContextKeyExpr.and(ContextKeyExpr.not('inExtensionEditor'), ContextKeyExpr.equals('extensionStatus', 'installed'), ContextKeyExpr.not('isBuiltinExtension'), ContextKeyExpr.or(ContextKeyExpr.equals(`config.${AutoUpdateConfigurationKey}`, 'onlySelectedExtensions'), ContextKeyExpr.equals(`config.${AutoUpdateConfigurationKey}`, false)),)
},
run: async (accessor: ServicesAccessor, id: string) => {
const instantiationService = accessor.get(IInstantiationService);
Expand All @@ -1357,7 +1357,7 @@ class ExtensionsContributions extends Disposable implements IWorkbenchContributi
id: MenuId.ExtensionContext,
group: UPDATE_ACTIONS_GROUP,
order: 2,
when: ContextKeyExpr.and(ContextKeyExpr.equals('extensionStatus', 'installed'), ContextKeyExpr.not('isBuiltinExtension'), ContextKeyExpr.equals(`config.${AutoUpdateConfigurationKey}`, 'onlySelectedExtensions'),)
when: ContextKeyExpr.and(ContextKeyExpr.equals('extensionStatus', 'installed'), ContextKeyExpr.not('isBuiltinExtension'), ContextKeyExpr.or(ContextKeyExpr.equals(`config.${AutoUpdateConfigurationKey}`, 'onlySelectedExtensions'), ContextKeyExpr.equals(`config.${AutoUpdateConfigurationKey}`, false)),)
},
run: async (accessor: ServicesAccessor, id: string) => {
const instantiationService = accessor.get(IInstantiationService);
Expand Down
7 changes: 1 addition & 6 deletions src/vs/workbench/contrib/extensions/browser/extensionsActions.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -889,11 +889,7 @@ export class ToggleAutoUpdateForExtensionAction extends ExtensionAction {
export class ToggleAutoUpdatesForPublisherAction extends ExtensionAction {

static readonly ID = 'workbench.extensions.action.toggleAutoUpdatesForPublisher';
static readonly LABEL = localize('toggleAutoUpdatesForPublisherLabel', "Auto Update (Publisher)");

static getLabel(extension: IExtension): string {
return localize('toggleAutoUpdatesForPublisherLabel2', "Auto Update All from {0} (Publisher)", extension.publisherDisplayName);
}
static readonly LABEL = localize('toggleAutoUpdatesForPublisherLabel', "Auto Update All (From Publisher)");

constructor(
@IExtensionsWorkbenchService private readonly extensionsWorkbenchService: IExtensionsWorkbenchService
Expand Down Expand Up @@ -1235,7 +1231,6 @@ export class MenuItemExtensionAction extends ExtensionAction {
} else if (this.action.id === ToggleAutoUpdateForExtensionAction.ID) {
this.checked = this.extensionsWorkbenchService.isAutoUpdateEnabledFor(this.extension);
} else if (this.action.id === ToggleAutoUpdatesForPublisherAction.ID) {
this.label = ToggleAutoUpdatesForPublisherAction.getLabel(this.extension);
this.checked = this.extensionsWorkbenchService.isAutoUpdateEnabledFor(this.extension.publisher);
} else {
this.checked = this.action.checked;
Expand Down
37 changes: 8 additions & 29 deletions src/vs/workbench/contrib/extensions/browser/extensionsWorkbenchService.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -738,8 +738,6 @@ export class ExtensionsWorkbenchService extends Disposable implements IExtension

readonly whenInitialized: Promise<void>;

private readonly organizationByPublisher = new Map<string, string>();

constructor(
@IInstantiationService private readonly instantiationService: IInstantiationService,
@IEditorService private readonly editorService: IEditorService,
Expand Down Expand Up @@ -801,14 +799,6 @@ export class ExtensionsWorkbenchService extends Disposable implements IExtension

urlService.registerHandler(this);

if (this.productService.publishersByOrganisation) {
for (const organization of Object.keys(this.productService.publishersByOrganisation)) {
for (const publisher of this.productService.publishersByOrganisation[organization]) {
this.organizationByPublisher.set(publisher, organization);
}
}
}

this.whenInitialized = this.initialize();
}

Expand Down Expand Up @@ -1509,25 +1499,13 @@ export class ExtensionsWorkbenchService extends Disposable implements IExtension
}

private isAutoUpdateEnabledForPublisher(publisher: string): boolean {
publisher = publisher.toLowerCase();
const publishersToAutoUpdate = this.getPublishersToAutoUpdate();
if (publishersToAutoUpdate.includes(publisher)) {
return true;
}
const publisherOrganization = this.organizationByPublisher.get(publisher);
if (publisherOrganization) {
return publishersToAutoUpdate.some(p => this.organizationByPublisher.get(publisher) === publisherOrganization);
}
return false;
return publishersToAutoUpdate.includes(publisher.toLowerCase());
}

async updateAutoUpdateEnablementFor(extensionOrPublisher: IExtension | string, enable: boolean): Promise<void> {
const autoUpdateValue = this.getAutoUpdateValue();

if (autoUpdateValue === false) {
throw new Error('Auto update is disabled');
}

if (autoUpdateValue === true || autoUpdateValue === 'onlyEnabledExtensions') {
if (isString(extensionOrPublisher)) {
throw new Error('Expected extension, found publisher string');
Expand All @@ -1542,6 +1520,10 @@ export class ExtensionsWorkbenchService extends Disposable implements IExtension
return;
}

if (autoUpdateValue === false && enable) {
await this.configurationService.updateValue(AutoUpdateConfigurationKey, 'onlySelectedExtensions');
}

let update = false;
const autoUpdateExtensions = this.getSelectedExtensionsToAutoUpdate();
if (isString(extensionOrPublisher)) {
Expand All @@ -1557,12 +1539,6 @@ export class ExtensionsWorkbenchService extends Disposable implements IExtension
if (autoUpdateExtensions.includes(extensionOrPublisher)) {
autoUpdateExtensions.splice(autoUpdateExtensions.indexOf(extensionOrPublisher), 1);
}
const publisherOrganization = this.organizationByPublisher.get(extensionOrPublisher);
if (publisherOrganization) {
for (const publisher of this.productService.publishersByOrganisation?.[publisherOrganization] ?? []) {
autoUpdateExtensions.splice(autoUpdateExtensions.indexOf(publisher), 1);
}
}
}
}
} else {
Expand Down Expand Up @@ -1610,6 +1586,9 @@ export class ExtensionsWorkbenchService extends Disposable implements IExtension
if (update) {
this.setSelectedExtensionsToAutoUpdate(autoUpdateExtensions);
await this.onDidPinnedViewContainersStorageValueChange(true);
if (autoUpdateValue === 'onlySelectedExtensions' && autoUpdateExtensions.length === 0) {
await this.configurationService.updateValue(AutoUpdateConfigurationKey, false);
}
}
}

Expand Down
91 changes: 34 additions & 57 deletions src/vs/workbench/contrib/extensions/test/electron-sandbox/extensionsWorkbenchService.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,6 @@ import { IExtensionService } from 'vs/workbench/services/extensions/common/exten
import { toDisposable } from 'vs/base/common/lifecycle';
import { ensureNoDisposablesAreLeakedInTestSuite } from 'vs/base/test/common/utils';
import { Mutable } from 'vs/base/common/types';
import { IProductConfiguration } from 'vs/base/common/product';

suite('ExtensionsWorkbenchServiceTest', () => {

Expand Down Expand Up @@ -1516,6 +1515,31 @@ suite('ExtensionsWorkbenchServiceTest', () => {
assert.strictEqual(testObject.local[1].local?.pinned, true);

assert.deepStrictEqual(testObject.getSelectedExtensionsToAutoUpdate(), ['pub.a']);
assert.equal(instantiationService.get(IConfigurationService).getValue(AutoUpdateConfigurationKey), 'onlySelectedExtensions');
});

test('Test enable autoupdate for extension when auto update is disabled', async () => {
stubConfiguration(false);

const extension1 = aLocalExtension('a', undefined, { pinned: true });
const extension2 = aLocalExtension('b', undefined, { pinned: true });
instantiationService.stubPromise(IExtensionManagementService, 'getInstalled', [extension1, extension2]);
instantiationService.stub(IExtensionManagementService, 'updateMetadata', (local: Mutable<ILocalExtension>, metadata: Partial<Metadata>) => {
local.pinned = !!metadata.pinned;
return local;
});
testObject = await aWorkbenchService();

assert.strictEqual(testObject.local[0].local?.pinned, true);
assert.strictEqual(testObject.local[1].local?.pinned, true);

await testObject.updateAutoUpdateEnablementFor(testObject.local[0], true);

assert.strictEqual(testObject.local[0].local?.pinned, false);
assert.strictEqual(testObject.local[1].local?.pinned, true);

assert.deepStrictEqual(testObject.getSelectedExtensionsToAutoUpdate(), ['pub.a']);
assert.equal(instantiationService.get(IConfigurationService).getValue(AutoUpdateConfigurationKey), 'onlySelectedExtensions');
});

test('Test disable autoupdate for extension when auto update is set to onlySelectedExtensions', async () => {
Expand All @@ -1536,6 +1560,7 @@ suite('ExtensionsWorkbenchServiceTest', () => {
assert.strictEqual(testObject.local[0].local?.pinned, true);
assert.strictEqual(testObject.local[1].local?.pinned, true);
assert.deepStrictEqual(testObject.getSelectedExtensionsToAutoUpdate(), []);
assert.equal(instantiationService.get(IConfigurationService).getValue(AutoUpdateConfigurationKey), false);
});

test('Test enable auto update for publisher when auto update mode is onlySelectedExtensions', async () => {
Expand Down Expand Up @@ -1626,76 +1651,28 @@ suite('ExtensionsWorkbenchServiceTest', () => {
assert.deepStrictEqual(testObject.getSelectedExtensionsToAutoUpdate(), ['pub']);
});

test('Test enable auto update for publisher enables for the associated organisation', async () => {
stubConfiguration('onlySelectedExtensions');
stubProductConfiguration({ publishersByOrganisation: { 'org1': ['pub', 'pub1'] } });

const extension1 = aLocalExtension('a', undefined, { pinned: true });
const extension2 = aLocalExtension('b', { publisher: 'pub1' }, { pinned: true });
const extension3 = aLocalExtension('a', { publisher: 'pub2' }, { pinned: true });
instantiationService.stubPromise(IExtensionManagementService, 'getInstalled', [extension1, extension2, extension3]);
instantiationService.stub(IExtensionManagementService, 'updateMetadata', (local: ILocalExtension, metadata: Partial<Metadata>) => {
local.pinned = !!metadata.pinned;
return local;
});
testObject = await aWorkbenchService();

await testObject.updateAutoUpdateEnablementFor(testObject.local[0].publisher, true);

assert.strictEqual(testObject.isAutoUpdateEnabledFor('pub'), true);
assert.strictEqual(testObject.isAutoUpdateEnabledFor('pub1'), true);
assert.strictEqual(testObject.isAutoUpdateEnabledFor('pub2'), false);
assert.strictEqual(testObject.isAutoUpdateEnabledFor(testObject.local[0]), true);
assert.strictEqual(testObject.isAutoUpdateEnabledFor(testObject.local[1]), true);
assert.strictEqual(testObject.isAutoUpdateEnabledFor(testObject.local[2]), false);
});

test('Test disable auto update for publisher disables for the associated organisation', async () => {
stubConfiguration('onlySelectedExtensions');
stubProductConfiguration({ publishersByOrganisation: { 'org1': ['pub', 'pub1'] } });

const extension1 = aLocalExtension('a', undefined, { pinned: true });
const extension2 = aLocalExtension('b', { publisher: 'pub1' }, { pinned: true });
const extension3 = aLocalExtension('a', { publisher: 'pub2' }, { pinned: true });
instantiationService.stubPromise(IExtensionManagementService, 'getInstalled', [extension1, extension2, extension3]);
instantiationService.stub(IExtensionManagementService, 'updateMetadata', (local: ILocalExtension, metadata: Partial<Metadata>) => {
local.pinned = !!metadata.pinned;
return local;
});
testObject = await aWorkbenchService();

await testObject.updateAutoUpdateEnablementFor(testObject.local[0].publisher, true);
await testObject.updateAutoUpdateEnablementFor(testObject.local[1].publisher, false);

assert.strictEqual(testObject.isAutoUpdateEnabledFor('pub'), false);
assert.strictEqual(testObject.isAutoUpdateEnabledFor('pub1'), false);
assert.strictEqual(testObject.isAutoUpdateEnabledFor('pub2'), false);
assert.strictEqual(testObject.isAutoUpdateEnabledFor(testObject.local[0]), false);
assert.strictEqual(testObject.isAutoUpdateEnabledFor(testObject.local[1]), false);
assert.strictEqual(testObject.isAutoUpdateEnabledFor(testObject.local[2]), false);
});

async function aWorkbenchService(): Promise<ExtensionsWorkbenchService> {
const workbenchService: ExtensionsWorkbenchService = disposableStore.add(instantiationService.createInstance(ExtensionsWorkbenchService));
await workbenchService.queryLocal();
return workbenchService;
}

function stubConfiguration(autoUpdateValue?: any, autoCheckUpdatesValue?: any): void {
const values: any = {
[AutoUpdateConfigurationKey]: autoUpdateValue ?? true,
[AutoCheckUpdatesConfigurationKey]: autoCheckUpdatesValue ?? true
};
instantiationService.stub(IConfigurationService, <Partial<IConfigurationService>>{
onDidChangeConfiguration: () => { return undefined!; },
getValue: (key?: string) => {
return key === AutoUpdateConfigurationKey ? autoUpdateValue ?? true
: key === AutoCheckUpdatesConfigurationKey ? autoCheckUpdatesValue ?? true
: undefined;
return key ? values[key] : undefined;
},
updateValue: async (key: string, value: any) => {
values[key] = value;
}
});
}

function stubProductConfiguration(productConfiguration: Partial<IProductConfiguration>): void {
instantiationService.stub(IProductService, productConfiguration);
}

function aLocalExtension(name: string = 'someext', manifest: any = {}, properties: any = {}): ILocalExtension {
manifest = { name, publisher: 'pub', version: '1.0.0', ...manifest };
properties = {
Expand Down

0 comments on commit 0e16324

Please sign in to comment.