Reload resource deployment types on extension changes (#14875)

* Reload resource deployment types on extension changes

* add comments

extensions/resource-deployment/src/main.ts

@@ -24,14 +24,10 @@ export async function activate(context: vscode.ExtensionContext): Promise<rd.IEx
 	const toolsService = new ToolsService(platformService);
 	const notebookService = new NotebookService(platformService, context.extensionPath);
 	const resourceTypeService = new ResourceTypeService(platformService, toolsService, notebookService);
-	const resourceTypes = resourceTypeService.getResourceTypes();
+	resourceTypeService.loadResourceTypes();
-	const validationFailures = resourceTypeService.validateResourceTypes(resourceTypes);
+	context.subscriptions.push(vscode.extensions.onDidChange(() => {
-	if (validationFailures.length !== 0) {
+		resourceTypeService.loadResourceTypes();
-		const errorMessage = localize('resourceDeployment.FailedToLoadExtension', "Failed to load extension: {0}, Error detected in the resource type definition in package.json, check debug console for details.", context.extensionPath);
+	}));
-		vscode.window.showErrorMessage(errorMessage);
-		validationFailures.forEach(message => console.error(message));
-		return <any>undefined;
-	}
 	const uriHandlerService = new UriHandlerService(resourceTypeService);
 	vscode.window.registerUriHandler(uriHandlerService);
 	/**
@@ -41,7 +37,7 @@ export async function activate(context: vscode.ExtensionContext): Promise<rd.IEx
 	 * resource types will be displayed
 	 */
 	const openDialog = (defaultResourceTypeName: string, resourceTypeNameFilters?: string[], optionValuesFilter?: OptionValuesFilter, initialVariableValues?: InitialVariableValues) => {
-		const defaultResourceType = resourceTypes.find(resourceType => resourceType.name === defaultResourceTypeName);
+		const defaultResourceType = resourceTypeService.getResourceTypes().find(resourceType => resourceType.name === defaultResourceTypeName);
 		if (!defaultResourceType) {
 			vscode.window.showErrorMessage(localize('resourceDeployment.UnknownResourceType', "The resource type: {0} is not defined", defaultResourceTypeName));
 		} else {

extensions/resource-deployment/src/services/resourceTypeService.ts

@@ -29,8 +29,28 @@ export interface OptionValuesFilter {
 }
 
 export interface IResourceTypeService {
+	/**
+	 * Loads the resource types contributed by all extensions, this should only be called on startup or when
+	 * changes to the installed extensions are made.
+	 */
+	loadResourceTypes(): void;
+	/**
+	 * Gets the set of contributed resource types
+	 * @param filterByPlatform Whether to filter to just types valid for the current platform
+	 */
 	getResourceTypes(filterByPlatform?: boolean): ResourceType[];
-	validateResourceTypes(resourceTypes: ResourceType[]): string[];
+	/**
+	 * Validates that the given resource type matches the schema we expect
+	 * @param resourceType The resource type to validate
+	 * @param positionInfo A string to use to identify the resource type (in case it doesn't have a name or other expected properties)
+	 */
+	validateResourceType(resourceType: ResourceType, positionInfo: string): string[];
+	/**
+	 * Starts a deployment for the given resource type
+	 * @param resourceType The resource type to start the deployment for
+	 * @param optionValuesFilter The resource type options to filter the list of selectable options to
+	 * @param initialVariableValues The set of initial key/value pairs to assign to the variables for the deployment
+	 */
 	startDeployment(resourceType: ResourceType, optionValuesFilter?: OptionValuesFilter, initialVariableValues?: InitialVariableValues): void;
 }
 
@@ -44,34 +64,40 @@ export class ResourceTypeService implements IResourceTypeService {
 	 * @param filterByPlatform indicates whether to return the resource types supported on current platform.
 	 */
 	getResourceTypes(filterByPlatform: boolean = true): ResourceType[] {
-		if (this._resourceTypes.length === 0) {
-			vscode.extensions.all.forEach((extension) => {
-				const extensionResourceTypes = extension.packageJSON.contributes?.resourceDeploymentTypes as ResourceType[];
-				extensionResourceTypes?.forEach((extensionResourceType: ResourceType) => {
-					// Clone the object - we modify it by adding complex types and so if we modify the original contribution then
-					// we can break VS Code functionality since it will sometimes pass this object over the RPC layer which requires
-					// stringifying it - which can break with some of the complex types we add.
-					const resourceType = deepClone(extensionResourceType);
-					this.updatePathProperties(resourceType, extension.extensionPath);
-					resourceType.getProvider = (selectedOptions) => { return this.getProvider(resourceType, selectedOptions); };
-					resourceType.getOkButtonText = (selectedOptions) => { return this.getOkButtonText(resourceType, selectedOptions); };
-					resourceType.getAgreementInfo = (selectedOptions) => { return this.getAgreementInfo(resourceType, selectedOptions); };
-					resourceType.getHelpText = (selectedOptions) => { return this.getHelpText(resourceType, selectedOptions); };
-					this.getResourceSubTypes(resourceType);
-					this._resourceTypes.push(resourceType);
-				});
-
-			});
-		}
-
 		let resourceTypes = this._resourceTypes;
 		if (filterByPlatform) {
 			resourceTypes = resourceTypes.filter(resourceType => (typeof resourceType.platforms === 'string' && resourceType.platforms === '*') || resourceType.platforms.includes(this.platformService.platform()));
 		}
-
 		return resourceTypes;
 	}
 
+	public loadResourceTypes(): void {
+		const resourceTypes: ResourceType[] = [];
+		vscode.extensions.all.forEach((extension) => {
+			const extensionResourceTypes = extension.packageJSON.contributes?.resourceDeploymentTypes as ResourceType[];
+			extensionResourceTypes?.forEach((extensionResourceType: ResourceType, index: number) => {
+				// Clone the object - we modify it by adding complex types and so if we modify the original contribution then
+				// we can break VS Code functionality since it will sometimes pass this object over the RPC layer which requires
+				// stringifying it - which can break with some of the complex types we add.
+				const resourceType = deepClone(extensionResourceType);
+				this.updatePathProperties(resourceType, extension.extensionPath);
+				resourceType.getProvider = (selectedOptions) => { return this.getProvider(resourceType, selectedOptions); };
+				resourceType.getOkButtonText = (selectedOptions) => { return this.getOkButtonText(resourceType, selectedOptions); };
+				resourceType.getAgreementInfo = (selectedOptions) => { return this.getAgreementInfo(resourceType, selectedOptions); };
+				resourceType.getHelpText = (selectedOptions) => { return this.getHelpText(resourceType, selectedOptions); };
+				this.getResourceSubTypes(resourceType);
+				// Validate the resource type, only adding it to our overall list if it's valid
+				const errors = this.validateResourceType(resourceType, `resource type index: ${index}`);
+				if (errors.length > 0) {
+					console.log(`Found errors validating resource type at index ${index} in extension ${extension.id}\n${errors.join('\n')}`);
+				} else {
+					resourceTypes.push(resourceType);
+				}
+			});
+		});
+		this._resourceTypes = resourceTypes;
+	}
+
 	private updatePathProperties(resourceType: ResourceType, extensionPath: string): void {
 		if (typeof resourceType.icon === 'string') {
 			resourceType.icon = path.join(extensionPath, resourceType.icon);
@@ -127,14 +153,12 @@ export class ResourceTypeService implements IResourceTypeService {
 	}
 
 	private getResourceSubTypes(resourceType: ResourceType): void {
-		const resourceSubTypes: ResourceSubType[] = [];
 		vscode.extensions.all.forEach((extension) => {
 			const extensionResourceSubTypes = extension.packageJSON.contributes?.resourceDeploymentSubTypes as ResourceSubType[];
 			extensionResourceSubTypes?.forEach((extensionResourceSubType: ResourceSubType) => {
 				const resourceSubType = deepClone(extensionResourceSubType);
 				if (resourceSubType.resourceName === resourceType.name) {
 					this.updateProviderPathProperties(resourceSubType.provider, extension.extensionPath);
-					resourceSubTypes.push(resourceSubType);
 					const tagSet = new Set(resourceType.tags);
 					resourceSubType.tags?.forEach(tag => tagSet.add(tag));
 					resourceType.tags = Array.from(tagSet);
@@ -162,27 +186,8 @@ export class ResourceTypeService implements IResourceTypeService {
 		});
 	}
 
-	/**
+	public validateResourceType(resourceType: ResourceType, positionInfo: string): string[] {
-	 * Validate the resource types and returns validation error messages if any.
-	 * @param resourceTypes resource types to be validated
-	 */
-	validateResourceTypes(resourceTypes: ResourceType[]): string[] {
-		// NOTE: The validation error messages do not need to be localized as it is only meant for the developer's use.
 		const errorMessages: string[] = [];
-		if (!resourceTypes || resourceTypes.length === 0) {
-			errorMessages.push('Resource type list is empty');
-		} else {
-			let resourceTypeIndex = 1;
-			resourceTypes.forEach(resourceType => {
-				this.validateResourceType(resourceType, `resource type index: ${resourceTypeIndex}`, errorMessages);
-				resourceTypeIndex++;
-			});
-		}
-
-		return errorMessages;
-	}
-
-	private validateResourceType(resourceType: ResourceType, positionInfo: string, errorMessages: string[]): void {
 		this.validateNameDisplayName(resourceType, 'resource type', positionInfo, errorMessages);
 		if (!resourceType.icon || (typeof resourceType.icon === 'object' && (!resourceType.icon.dark || !resourceType.icon.light))) {
 			errorMessages.push(`Icon for resource type is not specified properly. ${positionInfo} `);
@@ -196,8 +201,8 @@ export class ResourceTypeService implements IResourceTypeService {
 				optionIndex++;
 			});
 		}
-
 		this.validateProviders(resourceType, positionInfo, errorMessages);
+		return errorMessages;
 	}
 
 	private validateResourceTypeOption(option: ResourceTypeOption, positionInfo: string, errorMessages: string[]): void {

extensions/resource-deployment/src/test/services/resourceTypeService.test.ts

@@ -12,6 +12,7 @@ import { ResourceTypeService, processWhenClause } from '../../services/resourceT
 import { IPlatformService } from '../../services/platformService';
 import { ToolsService } from '../../services/toolsService';
 import { NotebookService } from '../../services/notebookService';
+import { ResourceType } from '../../interfaces';
 
 describe('Resource Type Service Tests', function (): void {
 
@@ -36,15 +37,18 @@ describe('Resource Type Service Tests', function (): void {
 			mockPlatformService.reset();
 			mockPlatformService.setup(service => service.platform()).returns(() => platformInfo.platform);
 			mockPlatformService.setup(service => service.showErrorMessage(TypeMoq.It.isAnyString()));
+			resourceTypeService.loadResourceTypes();
 			const resourceTypes = resourceTypeService.getResourceTypes(true).map(rt => rt.name);
 			for (let i = 0; i < platformInfo.resourceTypes.length; i++) {
 				assert(resourceTypes.indexOf(platformInfo.resourceTypes[i]) !== -1, `resource type '${platformInfo.resourceTypes[i]}' should be available for platform: ${platformInfo.platform}.`);
 			}
 		});
 
-		const allResourceTypes = resourceTypeService.getResourceTypes(false);
+		resourceTypeService.loadResourceTypes();
-		const validationErrors = resourceTypeService.validateResourceTypes(allResourceTypes);
+		resourceTypeService.getResourceTypes().forEach((resourceType: ResourceType, index: number) => {
-		assert(validationErrors.length === 0, `Validation errors detected in the package.json: ${validationErrors.join(EOL)}.`);
+			const validationErrors = resourceTypeService.validateResourceType(resourceType, `resource type index ${index}`);
+			assert(validationErrors.length === 0, `Validation errors detected in the package.json: ${validationErrors.join(EOL)}.`);
+		});
 	});
 
 	it('Selected options containing all when clauses should return true', () => {