Support removing and adding the same extension but at different versions (microsoft#153761)

Fixes microsoft#153646: Support removing and adding the same extension but at different versions

Author: alexdima

src/vs/workbench/services/extensions/common/abstractExtensionService.ts

@@ -546,23 +546,6 @@ export abstract class AbstractExtensionService extends Disposable implements IEx
 	}
 
 	private async _deltaExtensions(_toAdd: IExtension[], _toRemove: string[] | IExtension[]): Promise<void> {
-		const toAdd: IExtensionDescription[] = [];
-		for (let i = 0, len = _toAdd.length; i < len; i++) {
-			const extension = _toAdd[i];
-
-			const extensionDescription = await this._scanSingleExtension(extension);
-			if (!extensionDescription) {
-				// could not scan extension...
-				continue;
-			}
-
-			if (!this.canAddExtension(extensionDescription)) {
-				continue;
-			}
-
-			toAdd.push(extensionDescription);
-		}
-
 		let toRemove: IExtensionDescription[] = [];
 		for (let i = 0, len = _toRemove.length; i < len; i++) {
 			const extensionOrId = _toRemove[i];
@@ -587,6 +570,23 @@ export abstract class AbstractExtensionService extends Disposable implements IEx
 			toRemove.push(extensionDescription);
 		}
 
+		const toAdd: IExtensionDescription[] = [];
+		for (let i = 0, len = _toAdd.length; i < len; i++) {
+			const extension = _toAdd[i];
+
+			const extensionDescription = await this._scanSingleExtension(extension);
+			if (!extensionDescription) {
+				// could not scan extension...
+				continue;
+			}
+
+			if (!this._canAddExtension(extensionDescription, toRemove)) {
+				continue;
+			}
+
+			toAdd.push(extensionDescription);
+		}
+
 		if (toAdd.length === 0 && toRemove.length === 0) {
 			return;
 		}
@@ -643,15 +643,19 @@ export abstract class AbstractExtensionService extends Disposable implements IEx
 	}
 
 	public canAddExtension(extension: IExtensionDescription): boolean {
-		const existing = this._registry.getExtensionDescription(extension.identifier);
-		if (existing) {
-			// this extension is already running (most likely at a different version)
-			return false;
-		}
+		return this._canAddExtension(extension, []);
+	}
 
-		// Check if extension is renamed
-		if (extension.uuid && this._registry.getAllExtensionDescriptions().some(e => e.uuid === extension.uuid)) {
-			return false;
+	private _canAddExtension(extension: IExtensionDescription, extensionsBeingRemoved: IExtensionDescription[]): boolean {
+		// (Also check for renamed extensions)
+		const existing = this._registry.getExtensionDescriptionByIdOrUUID(extension.identifier, extension.id);
+		if (existing) {
+			// This extension is already known (most likely at a different version)
+			// so it cannot be added again unless it is removed first
+			const isBeingRemoved = extensionsBeingRemoved.some((extensionDescription) => ExtensionIdentifier.equals(extension.identifier, extensionDescription.identifier));
+			if (!isBeingRemoved) {
+				return false;
+			}
 		}
 
 		const extensionKind = this._getExtensionKind(extension);
@@ -667,7 +671,7 @@ export abstract class AbstractExtensionService extends Disposable implements IEx
 	public canRemoveExtension(extension: IExtensionDescription): boolean {
 		const extensionDescription = this._registry.getExtensionDescription(extension.identifier);
 		if (!extensionDescription) {
-			// ignore removing an extension which is not running
+			// Can't remove an extension that is unknown!
 			return false;
 		}
 

src/vs/workbench/services/extensions/common/extensionDescriptionRegistry.ts

@@ -68,19 +68,16 @@ export class ExtensionDescriptionRegistry {
 	}
 
 	public deltaExtensions(toAdd: IExtensionDescription[], toRemove: ExtensionIdentifier[]): DeltaExtensionsResult {
-		if (toAdd.length > 0) {
-			this._extensionDescriptions = this._extensionDescriptions.concat(toAdd);
-		}
+		// It is possible that an extension is removed, only to be added again at a different version
+		// so we will first handle removals
+		this._extensionDescriptions = removeExtensions(this._extensionDescriptions, toRemove);
+
+		// Then, handle the extensions to add
+		this._extensionDescriptions = this._extensionDescriptions.concat(toAdd);
 
 		// Immediately remove looping extensions!
 		const looping = ExtensionDescriptionRegistry._findLoopingExtensions(this._extensionDescriptions);
-		toRemove = toRemove.concat(looping.map(ext => ext.identifier));
-
-		if (toRemove.length > 0) {
-			const toRemoveSet = new Set<string>();
-			toRemove.forEach(extensionId => toRemoveSet.add(ExtensionIdentifier.toKey(extensionId)));
-			this._extensionDescriptions = this._extensionDescriptions.filter(extension => !toRemoveSet.has(ExtensionIdentifier.toKey(extension.identifier)));
-		}
+		this._extensionDescriptions = removeExtensions(this._extensionDescriptions, looping.map(ext => ext.identifier));
 
 		this._initialize();
 		this._onDidChange.fire(undefined);
@@ -194,6 +191,22 @@ export class ExtensionDescriptionRegistry {
 		const extension = this._extensionsMap.get(ExtensionIdentifier.toKey(extensionId));
 		return extension ? extension : undefined;
 	}
+
+	public getExtensionDescriptionByUUID(uuid: string): IExtensionDescription | undefined {
+		for (const extensionDescription of this._extensionsArr) {
+			if (extensionDescription.uuid === uuid) {
+				return extensionDescription;
+			}
+		}
+		return undefined;
+	}
+
+	public getExtensionDescriptionByIdOrUUID(extensionId: ExtensionIdentifier | string, uuid: string | undefined): IExtensionDescription | undefined {
+		return (
+			this.getExtensionDescription(extensionId)
+			?? (uuid ? this.getExtensionDescriptionByUUID(uuid) : undefined)
+		);
+	}
 }
 
 const enum SortBucket {
@@ -226,3 +239,9 @@ function extensionCmp(a: IExtensionDescription, b: IExtensionDescription): numbe
 	}
 	return 0;
 }
+
+function removeExtensions(arr: IExtensionDescription[], toRemove: ExtensionIdentifier[]): IExtensionDescription[] {
+	const toRemoveSet = new Set<string>();
+	toRemove.forEach(extensionId => toRemoveSet.add(ExtensionIdentifier.toKey(extensionId)));
+	return arr.filter(extension => !toRemoveSet.has(ExtensionIdentifier.toKey(extension.identifier)));
+}

src/vs/workbench/services/extensions/test/common/extensionDescriptionRegistry.test.ts

@@ -0,0 +1,40 @@
+/*---------------------------------------------------------------------------------------------
+ *  Copyright (c) Microsoft Corporation. All rights reserved.
+ *  Licensed under the MIT License. See License.txt in the project root for license information.
+ *--------------------------------------------------------------------------------------------*/
+
+import * as assert from 'assert';
+import { URI } from 'vs/base/common/uri';
+import { ExtensionIdentifier, IExtensionDescription, TargetPlatform } from 'vs/platform/extensions/common/extensions';
+import { ExtensionDescriptionRegistry } from 'vs/workbench/services/extensions/common/extensionDescriptionRegistry';
+
+suite('ExtensionDescriptionRegistry', () => {
+	test('allow removing and adding the same extension at a different version', () => {
+		const idA = new ExtensionIdentifier('a');
+		const extensionA1 = desc(idA, '1.0.0');
+		const extensionA2 = desc(idA, '2.0.0');
+
+		const registry = new ExtensionDescriptionRegistry([extensionA1]);
+		registry.deltaExtensions([extensionA2], [idA]);
+
+		assert.deepStrictEqual(registry.getAllExtensionDescriptions(), [extensionA2]);
+	});
+
+	function desc(id: ExtensionIdentifier, version: string, activationEvents: string[] = ['*']): IExtensionDescription {
+		return {
+			name: id.value,
+			publisher: 'test',
+			version: '0.0.0',
+			engines: { vscode: '^1.0.0' },
+			identifier: id,
+			extensionLocation: URI.parse(`nothing://nowhere`),
+			isBuiltin: false,
+			isUnderDevelopment: false,
+			isUserBuiltin: false,
+			activationEvents,
+			main: 'index.js',
+			targetPlatform: TargetPlatform.UNDEFINED,
+			extensionDependencies: []
+		};
+	}
+});