Support object type policy (#234428)

* #84756 Support object type policy * add new policy object for array and object * add new policy object type * checkin policies.js file * review * fix warning --------- Co-authored-by: João Moreno <[email protected]>
microsoft · Nov 25, 2024 · 6c6eb1e · 6c6eb1e
1 parent b98bc64
commit 6c6eb1e
Show file tree

Hide file tree

Showing 7 changed files with 125 additions and 25 deletions.
diff --git a/build/lib/policies.js b/build/lib/policies.js
diff --git a/build/lib/policies.ts b/build/lib/policies.ts
@@ -59,7 +59,7 @@ function renderADMLString(prefix: string, moduleName: string, nlsString: NlsStri
 		value = nlsString.value;
 	}
 
-	return `<string id="${prefix}_${nlsString.nlsKey}">${value}</string>`;
+	return `<string id="${prefix}_${nlsString.nlsKey.replace(/\./g, '_')}">${value}</string>`;
 }
 
 abstract class BasePolicy implements Policy {
@@ -78,7 +78,7 @@ abstract class BasePolicy implements Policy {
 
 	renderADMX(regKey: string) {
 		return [
-			`<policy name="${this.name}" class="Both" displayName="$(string.${this.name})" explainText="$(string.${this.name}_${this.description.nlsKey})" key="Software\\Policies\\Microsoft\\${regKey}" presentation="$(presentation.${this.name})">`,
+			`<policy name="${this.name}" class="Both" displayName="$(string.${this.name})" explainText="$(string.${this.name}_${this.description.nlsKey.replace(/\./g, '_')})" key="Software\\Policies\\Microsoft\\${regKey}" presentation="$(presentation.${this.name})">`,
 			`	<parentCategory ref="${this.category.name.nlsKey}" />`,
 			`	<supportedOn ref="Supported_${this.minimumVersion.replace(/\./g, '_')}" />`,
 			`	<elements>`,
@@ -232,6 +232,44 @@ class StringPolicy extends BasePolicy {
 	}
 }
 
+class ObjectPolicy extends BasePolicy {
+
+	static from(
+		name: string,
+		category: Category,
+		minimumVersion: string,
+		description: NlsString,
+		moduleName: string,
+		settingNode: Parser.SyntaxNode
+	): ObjectPolicy | undefined {
+		const type = getStringProperty(settingNode, 'type');
+
+		if (type !== 'object' && type !== 'array') {
+			return undefined;
+		}
+
+		return new ObjectPolicy(name, category, minimumVersion, description, moduleName);
+	}
+
+	private constructor(
+		name: string,
+		category: Category,
+		minimumVersion: string,
+		description: NlsString,
+		moduleName: string,
+	) {
+		super(PolicyType.StringEnum, name, category, minimumVersion, description, moduleName);
+	}
+
+	protected renderADMXElements(): string[] {
+		return [`<multiText id="${this.name}" valueName="${this.name}" required="true" />`];
+	}
+
+	renderADMLPresentationContents() {
+		return `<multiTextBox refId="${this.name}" />`;
+	}
+}
+
 class StringEnumPolicy extends BasePolicy {
 
 	static from(
@@ -402,6 +440,7 @@ const PolicyTypes = [
 	IntPolicy,
 	StringEnumPolicy,
 	StringPolicy,
+	ObjectPolicy
 ];
 
 function getPolicy(
@@ -474,7 +513,7 @@ function getPolicies(moduleName: string, node: Parser.SyntaxNode): Policy[] {
 				arguments: (arguments	(object	(pair
 					key: [(property_identifier)(string)] @propertiesKey (#eq? @propertiesKey properties)
 					value: (object (pair
-						key: [(property_identifier)(string)]
+						key: [(property_identifier)(string)(computed_property_name)]
 						value: (object (pair
 							key: [(property_identifier)(string)] @policyKey (#eq? @policyKey policy)
 							value: (object) @policy

diff --git a/src/vs/platform/configuration/common/configurations.ts b/src/vs/platform/configuration/common/configurations.ts
@@ -8,12 +8,13 @@ import { IStringDictionary } from '../../../base/common/collections.js';
 import { Emitter, Event } from '../../../base/common/event.js';
 import { Disposable } from '../../../base/common/lifecycle.js';
 import { equals } from '../../../base/common/objects.js';
-import { isEmptyObject } from '../../../base/common/types.js';
+import { isEmptyObject, isString } from '../../../base/common/types.js';
 import { ConfigurationModel } from './configurationModels.js';
 import { Extensions, IConfigurationRegistry, IRegisteredConfigurationPropertySchema } from './configurationRegistry.js';
 import { ILogService, NullLogService } from '../../log/common/log.js';
-import { IPolicyService, PolicyDefinition, PolicyName, PolicyValue } from '../../policy/common/policy.js';
+import { IPolicyService, PolicyDefinition, PolicyName } from '../../policy/common/policy.js';
 import { Registry } from '../../registry/common/platform.js';
+import { getErrorMessage } from '../../../base/common/errors.js';
 
 export class DefaultConfiguration extends Disposable {
 
@@ -122,12 +123,12 @@ export class PolicyConfiguration extends Disposable implements IPolicyConfigurat
 				continue;
 			}
 			if (config.policy) {
-				if (config.type !== 'string' && config.type !== 'number') {
+				if (config.type !== 'string' && config.type !== 'number' && config.type !== 'array' && config.type !== 'object') {
 					this.logService.warn(`Policy ${config.policy.name} has unsupported type ${config.type}`);
 					continue;
 				}
 				keys.push(key);
-				policyDefinitions[config.policy.name] = { type: config.type };
+				policyDefinitions[config.policy.name] = { type: config.type === 'number' ? 'number' : 'string' };
 			}
 		}
 
@@ -148,13 +149,22 @@ export class PolicyConfiguration extends Disposable implements IPolicyConfigurat
 	private update(keys: string[], trigger: boolean): void {
 		this.logService.trace('PolicyConfiguration#update', keys);
 		const configurationProperties = Registry.as<IConfigurationRegistry>(Extensions.Configuration).getConfigurationProperties();
-		const changed: [string, PolicyValue | undefined][] = [];
+		const changed: [string, any][] = [];
 		const wasEmpty = this._configurationModel.isEmpty();
 
 		for (const key of keys) {
-			const policyName = configurationProperties[key]?.policy?.name;
+			const proprety = configurationProperties[key];
+			const policyName = proprety?.policy?.name;
 			if (policyName) {
-				const policyValue = this.policyService.getPolicyValue(policyName);
+				let policyValue = this.policyService.getPolicyValue(policyName);
+				if (isString(policyValue) && proprety.type !== 'string') {
+					try {
+						policyValue = JSON.parse(policyValue);
+					} catch (e) {
+						this.logService.error(`Error parsing policy value ${policyName}:`, getErrorMessage(e));
+						continue;
+					}
+				}
 				if (wasEmpty ? policyValue !== undefined : !equals(this._configurationModel.getValue(key), policyValue)) {
 					changed.push([key, policyValue]);
 				}

diff --git a/src/vs/platform/configuration/test/common/policyConfiguration.test.ts b/src/vs/platform/configuration/test/common/policyConfiguration.test.ts
@@ -50,6 +50,22 @@ suite('PolicyConfiguration', () => {
 					minimumVersion: '1.0.0',
 				}
 			},
+			'policy.objectSetting': {
+				'type': 'object',
+				'default': {},
+				policy: {
+					name: 'PolicyObjectSetting',
+					minimumVersion: '1.0.0',
+				}
+			},
+			'policy.arraySetting': {
+				'type': 'object',
+				'default': [],
+				policy: {
+					name: 'PolicyArraySetting',
+					minimumVersion: '1.0.0',
+				}
+			},
 			'nonPolicy.setting': {
 				'type': 'boolean',
 				'default': true
@@ -107,6 +123,24 @@ suite('PolicyConfiguration', () => {
 		assert.deepStrictEqual(acutal.overrides, []);
 	});
 
+	test('initialize: with object type policy', async () => {
+		await fileService.writeFile(policyFile, VSBuffer.fromString(JSON.stringify({ 'PolicyObjectSetting': JSON.stringify({ 'a': 'b' }) })));
+
+		await testObject.initialize();
+		const acutal = testObject.configurationModel;
+
+		assert.deepStrictEqual(acutal.getValue('policy.objectSetting'), { 'a': 'b' });
+	});
+
+	test('initialize: with array type policy', async () => {
+		await fileService.writeFile(policyFile, VSBuffer.fromString(JSON.stringify({ 'PolicyArraySetting': JSON.stringify([1]) })));
+
+		await testObject.initialize();
+		const acutal = testObject.configurationModel;
+
+		assert.deepStrictEqual(acutal.getValue('policy.arraySetting'), [1]);
+	});
+
 	test('change: when policy is added', async () => {
 		await fileService.writeFile(policyFile, VSBuffer.fromString(JSON.stringify({ 'PolicySettingA': 'policyValueA' })));
 		await testObject.initialize();

diff --git a/src/vs/workbench/contrib/extensions/browser/extensions.contribution.ts b/src/vs/workbench/contrib/extensions/browser/extensions.contribution.ts
@@ -9,7 +9,7 @@ import { Registry } from '../../../../platform/registry/common/platform.js';
 import { MenuRegistry, MenuId, registerAction2, Action2, IMenuItem, IAction2Options } from '../../../../platform/actions/common/actions.js';
 import { InstantiationType, registerSingleton } from '../../../../platform/instantiation/common/extensions.js';
 import { ExtensionsLocalizedLabel, IExtensionManagementService, IExtensionGalleryService, PreferencesLocalizedLabel, EXTENSION_INSTALL_SOURCE_CONTEXT, ExtensionInstallSource, UseUnpkgResourceApiConfigKey, AllowedExtensionsConfigKey } from '../../../../platform/extensionManagement/common/extensionManagement.js';
-import { EnablementState, IExtensionManagementServerService, IWorkbenchExtensionEnablementService, IWorkbenchExtensionManagementService, extensionsConfigurationNodeBase } from '../../../services/extensionManagement/common/extensionManagement.js';
+import { EnablementState, IExtensionManagementServerService, IWorkbenchExtensionEnablementService, IWorkbenchExtensionManagementService } from '../../../services/extensionManagement/common/extensionManagement.js';
 import { IExtensionIgnoredRecommendationsService, IExtensionRecommendationsService } from '../../../services/extensionRecommendations/common/extensionRecommendations.js';
 import { IWorkbenchContributionsRegistry, Extensions as WorkbenchExtensions, IWorkbenchContribution } from '../../../common/contributions.js';
 import { SyncDescriptor } from '../../../../platform/instantiation/common/descriptors.js';
@@ -122,7 +122,10 @@ Registry.as<IViewContainersRegistry>(ViewContainerExtensions.ViewContainersRegis
 
 Registry.as<IConfigurationRegistry>(ConfigurationExtensions.Configuration)
 	.registerConfiguration({
-		...extensionsConfigurationNodeBase,
+		id: 'extensions',
+		order: 30,
+		title: localize('extensionsConfigurationTitle', "Extensions"),
+		type: 'object',
 		properties: {
 			'extensions.autoUpdate': {
 				enum: [true, 'onlyEnabledExtensions', false,],

diff --git a/src/vs/workbench/contrib/extensions/browser/extensionsWorkbenchService.ts b/src/vs/workbench/contrib/extensions/browser/extensionsWorkbenchService.ts
@@ -21,7 +21,7 @@ import {
 	TargetPlatformToString,
 	IAllowedExtensionsService
 } from '../../../../platform/extensionManagement/common/extensionManagement.js';
-import { IWorkbenchExtensionEnablementService, EnablementState, IExtensionManagementServerService, IExtensionManagementServer, IWorkbenchExtensionManagementService, DefaultIconPath, IResourceExtension, extensionsConfigurationNodeBase } from '../../../services/extensionManagement/common/extensionManagement.js';
+import { IWorkbenchExtensionEnablementService, EnablementState, IExtensionManagementServerService, IExtensionManagementServer, IWorkbenchExtensionManagementService, DefaultIconPath, IResourceExtension } from '../../../services/extensionManagement/common/extensionManagement.js';
 import { getGalleryExtensionTelemetryData, getLocalExtensionTelemetryData, areSameExtensions, groupByExtension, getGalleryExtensionId } from '../../../../platform/extensionManagement/common/extensionManagementUtil.js';
 import { IInstantiationService } from '../../../../platform/instantiation/common/instantiation.js';
 import { IConfigurationService } from '../../../../platform/configuration/common/configuration.js';
@@ -1033,7 +1033,10 @@ export class ExtensionsWorkbenchService extends Disposable implements IExtension
 	private registerAutoRestartConfig(): void {
 		Registry.as<IConfigurationRegistry>(ConfigurationExtensions.Configuration)
 			.registerConfiguration({
-				...extensionsConfigurationNodeBase,
+				id: 'extensions',
+				order: 30,
+				title: nls.localize('extensionsConfigurationTitle', "Extensions"),
+				type: 'object',
 				properties: {
 					[AutoRestartConfigurationKey]: {
 						type: 'boolean',

diff --git a/src/vs/workbench/services/extensionManagement/common/extensionManagement.ts b/src/vs/workbench/services/extensionManagement/common/extensionManagement.ts
@@ -9,7 +9,6 @@ import { IExtension, ExtensionType, IExtensionManifest, IExtensionIdentifier } f
 import { IExtensionManagementService, IGalleryExtension, ILocalExtension, InstallOptions, InstallExtensionEvent, DidUninstallExtensionEvent, InstallExtensionResult, Metadata, UninstallExtensionEvent, DidUpdateExtensionMetadata } from '../../../../platform/extensionManagement/common/extensionManagement.js';
 import { URI } from '../../../../base/common/uri.js';
 import { FileAccess } from '../../../../base/common/network.js';
-import { localize } from '../../../../nls.js';
 import { IMarkdownString } from '../../../../base/common/htmlContent.js';
 
 export type DidChangeProfileEvent = { readonly added: ILocalExtension[]; readonly removed: ILocalExtension[] };
@@ -89,13 +88,6 @@ export interface IWorkbenchExtensionManagementService extends IProfileAwareExten
 	updateMetadata(local: ILocalExtension, metadata: Partial<Metadata>): Promise<ILocalExtension>;
 }
 
-export const extensionsConfigurationNodeBase = {
-	id: 'extensions',
-	order: 30,
-	title: localize('extensionsConfigurationTitle', "Extensions"),
-	type: 'object'
-};
-
 export const enum EnablementState {
 	DisabledByTrustRequirement,
 	DisabledByExtensionKind,