From d8affc039ab669c3009aad5bab4e8e2f9de299a9 Mon Sep 17 00:00:00 2001 From: Sandeep Somavarapu Date: Tue, 22 Aug 2023 12:34:24 +0200 Subject: [PATCH] fix #188582 (#190957) --- .../common/extensionsProfileScannerService.ts | 19 +- .../extensionsProfileScannerService.test.ts | 252 +++++++++++++++++- 2 files changed, 259 insertions(+), 12 deletions(-) diff --git a/src/vs/platform/extensionManagement/common/extensionsProfileScannerService.ts b/src/vs/platform/extensionManagement/common/extensionsProfileScannerService.ts index 30e749ac312..40eb2584305 100644 --- a/src/vs/platform/extensionManagement/common/extensionsProfileScannerService.ts +++ b/src/vs/platform/extensionManagement/common/extensionsProfileScannerService.ts @@ -124,22 +124,24 @@ export abstract class AbstractExtensionsProfileScannerService extends Disposable const extensionsToRemove: IScannedProfileExtension[] = []; const extensionsToAdd: IScannedProfileExtension[] = []; try { - await this.withProfileExtensions(profileLocation, profileExtensions => { + await this.withProfileExtensions(profileLocation, existingExtensions => { const result: IScannedProfileExtension[] = []; - for (const extension of profileExtensions) { - if (extensions.some(([e]) => areSameExtensions(e.identifier, extension.identifier) && e.manifest.version !== extension.version)) { + for (const existing of existingExtensions) { + if (extensions.some(([e]) => areSameExtensions(e.identifier, existing.identifier) && e.manifest.version !== existing.version)) { // Remove the existing extension with different version - extensionsToRemove.push(extension); + extensionsToRemove.push(existing); } else { - result.push(extension); + result.push(existing); } } for (const [extension, metadata] of extensions) { - if (!result.some(e => areSameExtensions(e.identifier, extension.identifier) && e.version === extension.manifest.version)) { - // Add only if the same version of the extension is not already added - const extensionToAdd = { identifier: extension.identifier, version: extension.manifest.version, location: extension.location, metadata }; + const index = result.findIndex(e => areSameExtensions(e.identifier, extension.identifier) && e.version === extension.manifest.version); + const extensionToAdd = { identifier: extension.identifier, version: extension.manifest.version, location: extension.location, metadata }; + if (index === -1) { extensionsToAdd.push(extensionToAdd); result.push(extensionToAdd); + } else { + result.splice(index, 1, extensionToAdd); } } if (extensionsToAdd.length) { @@ -189,7 +191,6 @@ export abstract class AbstractExtensionsProfileScannerService extends Disposable async removeExtensionFromProfile(extension: IExtension, profileLocation: URI): Promise { const extensionsToRemove: IScannedProfileExtension[] = []; - this._onRemoveExtensions.fire({ extensions: extensionsToRemove, profileLocation }); try { await this.withProfileExtensions(profileLocation, profileExtensions => { const result: IScannedProfileExtension[] = []; diff --git a/src/vs/platform/extensionManagement/test/common/extensionsProfileScannerService.test.ts b/src/vs/platform/extensionManagement/test/common/extensionsProfileScannerService.test.ts index 2dcd920d3e2..b2fa0668e72 100644 --- a/src/vs/platform/extensionManagement/test/common/extensionsProfileScannerService.test.ts +++ b/src/vs/platform/extensionManagement/test/common/extensionsProfileScannerService.test.ts @@ -4,13 +4,14 @@ *--------------------------------------------------------------------------------------------*/ import * as assert from 'assert'; +import * as sinon from 'sinon'; import { VSBuffer } from 'vs/base/common/buffer'; import { DisposableStore } from 'vs/base/common/lifecycle'; import { joinPath } from 'vs/base/common/resources'; import { URI } from 'vs/base/common/uri'; import { IEnvironmentService } from 'vs/platform/environment/common/environment'; -import { AbstractExtensionsProfileScannerService } from 'vs/platform/extensionManagement/common/extensionsProfileScannerService'; -import { ExtensionType, IExtension, TargetPlatform } from 'vs/platform/extensions/common/extensions'; +import { AbstractExtensionsProfileScannerService, ProfileExtensionsEvent } from 'vs/platform/extensionManagement/common/extensionsProfileScannerService'; +import { ExtensionType, IExtension, IExtensionManifest, TargetPlatform } from 'vs/platform/extensions/common/extensions'; import { FileService } from 'vs/platform/files/common/fileService'; import { IFileService } from 'vs/platform/files/common/files'; import { InMemoryFileSystemProvider } from 'vs/platform/files/common/inMemoryFilesystemProvider'; @@ -47,6 +48,10 @@ suite('ExtensionsProfileScannerService', () => { instantiationService.stub(IUserDataProfilesService, userDataProfilesService); }); + teardown(() => disposables.clear()); + + suiteTeardown(() => sinon.restore()); + test('write extensions located in the same extensions folder', async () => { const testObject = instantiationService.createInstance(TestObject, extensionsLocation); @@ -442,7 +447,247 @@ suite('ExtensionsProfileScannerService', () => { assert.deepStrictEqual(manifestContent, [{ identifier: extension.identifier, location: extension.location.toJSON(), relativeLocation: 'pub.a-1.0.0', version: extension.manifest.version }]); }); - function aExtension(id: string, location: URI, e?: Partial): IExtension { + test('add extension trigger events', async () => { + const testObject = instantiationService.createInstance(TestObject, extensionsLocation); + const target1 = sinon.stub(); + const target2 = sinon.stub(); + testObject.onAddExtensions(target1); + testObject.onDidAddExtensions(target2); + + const extensionsManifest = joinPath(extensionsLocation, 'extensions.json'); + const extension = aExtension('pub.a', joinPath(ROOT, 'foo', 'pub.a-1.0.0')); + await testObject.addExtensionsToProfile([[extension, undefined]], extensionsManifest); + + const actual = await testObject.scanProfileExtensions(extensionsManifest); + assert.deepStrictEqual(actual.map(a => ({ ...a, location: a.location.toJSON() })), [{ identifier: extension.identifier, location: extension.location.toJSON(), version: extension.manifest.version, metadata: undefined }]); + + assert.ok(target1.calledOnce); + assert.deepStrictEqual(((target1.args[0][0])).profileLocation.toString(), extensionsManifest.toString()); + assert.deepStrictEqual(((target1.args[0][0])).extensions.length, 1); + assert.deepStrictEqual(((target1.args[0][0])).extensions[0].identifier, extension.identifier); + assert.deepStrictEqual(((target1.args[0][0])).extensions[0].version, extension.manifest.version); + assert.deepStrictEqual(((target1.args[0][0])).extensions[0].location.toString(), extension.location.toString()); + + assert.ok(target2.calledOnce); + assert.deepStrictEqual(((target2.args[0][0])).profileLocation.toString(), extensionsManifest.toString()); + assert.deepStrictEqual(((target2.args[0][0])).extensions.length, 1); + assert.deepStrictEqual(((target2.args[0][0])).extensions[0].identifier, extension.identifier); + assert.deepStrictEqual(((target2.args[0][0])).extensions[0].version, extension.manifest.version); + assert.deepStrictEqual(((target2.args[0][0])).extensions[0].location.toString(), extension.location.toString()); + }); + + test('remove extension trigger events', async () => { + const testObject = instantiationService.createInstance(TestObject, extensionsLocation); + const target1 = sinon.stub(); + const target2 = sinon.stub(); + testObject.onRemoveExtensions(target1); + testObject.onDidRemoveExtensions(target2); + + const extensionsManifest = joinPath(extensionsLocation, 'extensions.json'); + const extension = aExtension('pub.a', joinPath(ROOT, 'foo', 'pub.a-1.0.0')); + await testObject.addExtensionsToProfile([[extension, undefined]], extensionsManifest); + await testObject.removeExtensionFromProfile(extension, extensionsManifest); + + const actual = await testObject.scanProfileExtensions(extensionsManifest); + assert.deepStrictEqual(actual.length, 0); + + assert.ok(target1.calledOnce); + assert.deepStrictEqual(((target1.args[0][0])).profileLocation.toString(), extensionsManifest.toString()); + assert.deepStrictEqual(((target1.args[0][0])).extensions.length, 1); + assert.deepStrictEqual(((target1.args[0][0])).extensions[0].identifier, extension.identifier); + assert.deepStrictEqual(((target1.args[0][0])).extensions[0].version, extension.manifest.version); + assert.deepStrictEqual(((target1.args[0][0])).extensions[0].location.toString(), extension.location.toString()); + + assert.ok(target2.calledOnce); + assert.deepStrictEqual(((target2.args[0][0])).profileLocation.toString(), extensionsManifest.toString()); + assert.deepStrictEqual(((target2.args[0][0])).extensions.length, 1); + assert.deepStrictEqual(((target2.args[0][0])).extensions[0].identifier, extension.identifier); + assert.deepStrictEqual(((target2.args[0][0])).extensions[0].version, extension.manifest.version); + assert.deepStrictEqual(((target2.args[0][0])).extensions[0].location.toString(), extension.location.toString()); + }); + + test('add extension with same id but different version', async () => { + const testObject = instantiationService.createInstance(TestObject, extensionsLocation); + + const extensionsManifest = joinPath(extensionsLocation, 'extensions.json'); + + const extension1 = aExtension('pub.a', joinPath(ROOT, 'pub.a-1.0.0')); + await testObject.addExtensionsToProfile([[extension1, undefined]], extensionsManifest); + + const target1 = sinon.stub(); + const target2 = sinon.stub(); + const target3 = sinon.stub(); + const target4 = sinon.stub(); + testObject.onAddExtensions(target1); + testObject.onRemoveExtensions(target2); + testObject.onDidAddExtensions(target3); + testObject.onDidRemoveExtensions(target4); + const extension2 = aExtension('pub.a', joinPath(ROOT, 'pub.a-2.0.0'), undefined, { version: '2.0.0' }); + await testObject.addExtensionsToProfile([[extension2, undefined]], extensionsManifest); + + const actual = await testObject.scanProfileExtensions(extensionsManifest); + assert.deepStrictEqual(actual.map(a => ({ ...a, location: a.location.toJSON() })), [{ identifier: extension2.identifier, location: extension2.location.toJSON(), version: extension2.manifest.version, metadata: undefined }]); + + assert.ok(target1.calledOnce); + assert.deepStrictEqual(((target1.args[0][0])).profileLocation.toString(), extensionsManifest.toString()); + assert.deepStrictEqual(((target1.args[0][0])).extensions.length, 1); + assert.deepStrictEqual(((target1.args[0][0])).extensions[0].identifier, extension2.identifier); + assert.deepStrictEqual(((target1.args[0][0])).extensions[0].version, extension2.manifest.version); + assert.deepStrictEqual(((target1.args[0][0])).extensions[0].location.toString(), extension2.location.toString()); + + assert.ok(target2.calledOnce); + assert.deepStrictEqual(((target2.args[0][0])).profileLocation.toString(), extensionsManifest.toString()); + assert.deepStrictEqual(((target2.args[0][0])).extensions.length, 1); + assert.deepStrictEqual(((target2.args[0][0])).extensions[0].identifier, extension1.identifier); + assert.deepStrictEqual(((target2.args[0][0])).extensions[0].version, extension1.manifest.version); + assert.deepStrictEqual(((target2.args[0][0])).extensions[0].location.toString(), extension1.location.toString()); + + assert.ok(target3.calledOnce); + assert.deepStrictEqual(((target1.args[0][0])).profileLocation.toString(), extensionsManifest.toString()); + assert.deepStrictEqual(((target1.args[0][0])).extensions.length, 1); + assert.deepStrictEqual(((target1.args[0][0])).extensions[0].identifier, extension2.identifier); + assert.deepStrictEqual(((target1.args[0][0])).extensions[0].version, extension2.manifest.version); + assert.deepStrictEqual(((target1.args[0][0])).extensions[0].location.toString(), extension2.location.toString()); + + assert.ok(target4.calledOnce); + assert.deepStrictEqual(((target2.args[0][0])).profileLocation.toString(), extensionsManifest.toString()); + assert.deepStrictEqual(((target2.args[0][0])).extensions.length, 1); + assert.deepStrictEqual(((target2.args[0][0])).extensions[0].identifier, extension1.identifier); + assert.deepStrictEqual(((target2.args[0][0])).extensions[0].version, extension1.manifest.version); + assert.deepStrictEqual(((target2.args[0][0])).extensions[0].location.toString(), extension1.location.toString()); + }); + + test('add same extension', async () => { + const testObject = instantiationService.createInstance(TestObject, extensionsLocation); + + const extensionsManifest = joinPath(extensionsLocation, 'extensions.json'); + + const extension = aExtension('pub.a', joinPath(ROOT, 'pub.a-1.0.0')); + await testObject.addExtensionsToProfile([[extension, undefined]], extensionsManifest); + + const target1 = sinon.stub(); + const target2 = sinon.stub(); + const target3 = sinon.stub(); + const target4 = sinon.stub(); + testObject.onAddExtensions(target1); + testObject.onRemoveExtensions(target2); + testObject.onDidAddExtensions(target3); + testObject.onDidRemoveExtensions(target4); + await testObject.addExtensionsToProfile([[extension, undefined]], extensionsManifest); + + const actual = await testObject.scanProfileExtensions(extensionsManifest); + assert.deepStrictEqual(actual.map(a => ({ ...a, location: a.location.toJSON() })), [{ identifier: extension.identifier, location: extension.location.toJSON(), version: extension.manifest.version, metadata: undefined }]); + assert.ok(target1.notCalled); + assert.ok(target2.notCalled); + assert.ok(target3.notCalled); + assert.ok(target4.notCalled); + }); + + test('add same extension with different metadata', async () => { + const testObject = instantiationService.createInstance(TestObject, extensionsLocation); + + const extensionsManifest = joinPath(extensionsLocation, 'extensions.json'); + + const extension = aExtension('pub.a', joinPath(ROOT, 'pub.a-1.0.0')); + await testObject.addExtensionsToProfile([[extension, undefined]], extensionsManifest); + + const target1 = sinon.stub(); + const target2 = sinon.stub(); + const target3 = sinon.stub(); + const target4 = sinon.stub(); + testObject.onAddExtensions(target1); + testObject.onRemoveExtensions(target2); + testObject.onDidAddExtensions(target3); + testObject.onDidRemoveExtensions(target4); + await testObject.addExtensionsToProfile([[extension, { isApplicationScoped: true }]], extensionsManifest); + + const actual = await testObject.scanProfileExtensions(extensionsManifest); + assert.deepStrictEqual(actual.map(a => ({ ...a, location: a.location.toJSON(), metadata: a.metadata })), [{ identifier: extension.identifier, location: extension.location.toJSON(), version: extension.manifest.version, metadata: { isApplicationScoped: true } }]); + assert.ok(target1.notCalled); + assert.ok(target2.notCalled); + assert.ok(target3.notCalled); + assert.ok(target4.notCalled); + }); + + test('add extension with different version and metadata', async () => { + const testObject = instantiationService.createInstance(TestObject, extensionsLocation); + + const extensionsManifest = joinPath(extensionsLocation, 'extensions.json'); + + const extension1 = aExtension('pub.a', joinPath(ROOT, 'pub.a-1.0.0')); + await testObject.addExtensionsToProfile([[extension1, undefined]], extensionsManifest); + const extension2 = aExtension('pub.a', joinPath(ROOT, 'pub.a-2.0.0'), undefined, { version: '2.0.0' }); + + const target1 = sinon.stub(); + const target2 = sinon.stub(); + const target3 = sinon.stub(); + const target4 = sinon.stub(); + testObject.onAddExtensions(target1); + testObject.onRemoveExtensions(target2); + testObject.onDidAddExtensions(target3); + testObject.onDidRemoveExtensions(target4); + await testObject.addExtensionsToProfile([[extension2, { isApplicationScoped: true }]], extensionsManifest); + + const actual = await testObject.scanProfileExtensions(extensionsManifest); + assert.deepStrictEqual(actual.map(a => ({ ...a, location: a.location.toJSON(), metadata: a.metadata })), [{ identifier: extension2.identifier, location: extension2.location.toJSON(), version: extension2.manifest.version, metadata: { isApplicationScoped: true } }]); + + assert.ok(target1.calledOnce); + assert.deepStrictEqual(((target1.args[0][0])).profileLocation.toString(), extensionsManifest.toString()); + assert.deepStrictEqual(((target1.args[0][0])).extensions.length, 1); + assert.deepStrictEqual(((target1.args[0][0])).extensions[0].identifier, extension2.identifier); + assert.deepStrictEqual(((target1.args[0][0])).extensions[0].version, extension2.manifest.version); + assert.deepStrictEqual(((target1.args[0][0])).extensions[0].location.toString(), extension2.location.toString()); + + assert.ok(target2.calledOnce); + assert.deepStrictEqual(((target2.args[0][0])).profileLocation.toString(), extensionsManifest.toString()); + assert.deepStrictEqual(((target2.args[0][0])).extensions.length, 1); + assert.deepStrictEqual(((target2.args[0][0])).extensions[0].identifier, extension1.identifier); + assert.deepStrictEqual(((target2.args[0][0])).extensions[0].version, extension1.manifest.version); + assert.deepStrictEqual(((target2.args[0][0])).extensions[0].location.toString(), extension1.location.toString()); + + assert.ok(target3.calledOnce); + assert.deepStrictEqual(((target1.args[0][0])).profileLocation.toString(), extensionsManifest.toString()); + assert.deepStrictEqual(((target1.args[0][0])).extensions.length, 1); + assert.deepStrictEqual(((target1.args[0][0])).extensions[0].identifier, extension2.identifier); + assert.deepStrictEqual(((target1.args[0][0])).extensions[0].version, extension2.manifest.version); + assert.deepStrictEqual(((target1.args[0][0])).extensions[0].location.toString(), extension2.location.toString()); + + assert.ok(target4.calledOnce); + assert.deepStrictEqual(((target2.args[0][0])).profileLocation.toString(), extensionsManifest.toString()); + assert.deepStrictEqual(((target2.args[0][0])).extensions.length, 1); + assert.deepStrictEqual(((target2.args[0][0])).extensions[0].identifier, extension1.identifier); + assert.deepStrictEqual(((target2.args[0][0])).extensions[0].version, extension1.manifest.version); + assert.deepStrictEqual(((target2.args[0][0])).extensions[0].location.toString(), extension1.location.toString()); + }); + + test('add extension with same id and version located in the different folder', async () => { + const testObject = instantiationService.createInstance(TestObject, extensionsLocation); + + const extensionsManifest = joinPath(extensionsLocation, 'extensions.json'); + + let extension = aExtension('pub.a', joinPath(ROOT, 'foo', 'pub.a-1.0.0')); + await testObject.addExtensionsToProfile([[extension, undefined]], extensionsManifest); + + const target1 = sinon.stub(); + const target2 = sinon.stub(); + const target3 = sinon.stub(); + const target4 = sinon.stub(); + testObject.onAddExtensions(target1); + testObject.onRemoveExtensions(target2); + testObject.onDidAddExtensions(target3); + testObject.onDidRemoveExtensions(target4); + extension = aExtension('pub.a', joinPath(ROOT, 'pub.a-1.0.0')); + await testObject.addExtensionsToProfile([[extension, undefined]], extensionsManifest); + + const actual = await testObject.scanProfileExtensions(extensionsManifest); + assert.deepStrictEqual(actual.map(a => ({ ...a, location: a.location.toJSON() })), [{ identifier: extension.identifier, location: extension.location.toJSON(), version: extension.manifest.version, metadata: undefined }]); + assert.ok(target1.notCalled); + assert.ok(target2.notCalled); + assert.ok(target3.notCalled); + assert.ok(target4.notCalled); + }); + + function aExtension(id: string, location: URI, e?: Partial, manifest?: Partial): IExtension { return { identifier: { id }, location, @@ -454,6 +699,7 @@ suite('ExtensionsProfileScannerService', () => { publisher: 'publisher', version: '1.0.0', engines: { vscode: '1.0.0' }, + ...manifest, }, isValid: true, validations: [],