From 830c7398573316dc2603ae0ab4001c505eee3ef0 Mon Sep 17 00:00:00 2001 From: Sandeep Somavarapu Date: Mon, 7 Jun 2021 19:49:50 +0200 Subject: [PATCH] Fix settings sync is removing extensions from web. - Do not use remote data as last sync data for extensions - Do not use cache and fetch extensions always --- resources/web/code-web.js | 3 - .../common/abstractSynchronizer.ts | 19 +- .../userDataSync/common/extensionsMerge.ts | 31 ++- .../userDataSync/common/extensionsSync.ts | 49 ++--- .../userDataSync/common/globalStateSync.ts | 5 +- .../userDataSync/common/keybindingsSync.ts | 5 +- .../userDataSync/common/settingsSync.ts | 5 +- .../userDataSync/common/snippetsSync.ts | 5 +- .../test/common/extensionsMerge.test.ts | 208 +++++++++--------- .../test/common/synchronizer.test.ts | 2 +- .../common/webExtensionsScannerService.ts | 12 +- 11 files changed, 174 insertions(+), 170 deletions(-) diff --git a/resources/web/code-web.js b/resources/web/code-web.js index f9c004501bf..2c2bbe88d08 100644 --- a/resources/web/code-web.js +++ b/resources/web/code-web.js @@ -423,9 +423,6 @@ async function handleRoot(req, res) { const webConfigJSON = { folderUri: folderUri, staticExtensions, - settingsSyncOptions: { - enabled: args['enable-sync'] - }, webWorkerExtensionHostIframeSrc: `${SCHEME}://${secondaryHost}/static/out/vs/workbench/services/extensions/worker/httpWebWorkerExtensionHostIframe.html` }; if (args['wrap-iframe']) { diff --git a/src/vs/platform/userDataSync/common/abstractSynchronizer.ts b/src/vs/platform/userDataSync/common/abstractSynchronizer.ts index 2931ef5e051..14dc0b792af 100644 --- a/src/vs/platform/userDataSync/common/abstractSynchronizer.ts +++ b/src/vs/platform/userDataSync/common/abstractSynchronizer.ts @@ -273,9 +273,10 @@ export abstract class AbstractSynchroniser extends Disposable { this.setStatus(SyncStatus.Syncing); const lastSyncUserData = await this.getLastSyncUserData(); const remoteUserData = await this.getLatestRemoteUserData(null, lastSyncUserData); + const isRemoteDataFromCurrentMachine = await this.isRemoteDataFromCurrentMachine(remoteUserData); /* use replace sync data */ - const resourcePreviewResults = await this.generateSyncPreview({ ref: remoteUserData.ref, syncData }, lastSyncUserData, CancellationToken.None); + const resourcePreviewResults = await this.generateSyncPreview({ ref: remoteUserData.ref, syncData }, lastSyncUserData, isRemoteDataFromCurrentMachine, CancellationToken.None); const resourcePreviews: [IResourcePreview, IAcceptResult][] = []; for (const resourcePreviewResult of resourcePreviewResults) { @@ -295,6 +296,11 @@ export abstract class AbstractSynchroniser extends Disposable { return true; } + private async isRemoteDataFromCurrentMachine(remoteUserData: IRemoteUserData): Promise { + const machineId = await this.currentMachineIdPromise; + return !!remoteUserData.syncData?.machineId && remoteUserData.syncData.machineId === machineId; + } + protected async getLatestRemoteUserData(manifest: IUserDataManifest | null, lastSyncUserData: IRemoteUserData | null): Promise { if (lastSyncUserData) { @@ -564,11 +570,8 @@ export abstract class AbstractSynchroniser extends Disposable { } private async doGenerateSyncResourcePreview(remoteUserData: IRemoteUserData, lastSyncUserData: IRemoteUserData | null, apply: boolean, token: CancellationToken): Promise { - const machineId = await this.currentMachineIdPromise; - const isLastSyncFromCurrentMachine = !!remoteUserData.syncData?.machineId && remoteUserData.syncData.machineId === machineId; - - const lastSyncUserDataForPreview = lastSyncUserData === null && isLastSyncFromCurrentMachine && !this.hasSyncResourceStateVersionChanged ? remoteUserData : lastSyncUserData; - const resourcePreviewResults = await this.generateSyncPreview(remoteUserData, lastSyncUserDataForPreview, token); + const isRemoteDataFromCurrentMachine = await this.isRemoteDataFromCurrentMachine(remoteUserData); + const resourcePreviewResults = await this.generateSyncPreview(remoteUserData, lastSyncUserData, isRemoteDataFromCurrentMachine, token); const resourcePreviews: IEditableResourcePreview[] = []; for (const resourcePreviewResult of resourcePreviewResults) { @@ -609,7 +612,7 @@ export abstract class AbstractSynchroniser extends Disposable { } } - return { remoteUserData, lastSyncUserData, resourcePreviews, isLastSyncFromCurrentMachine }; + return { remoteUserData, lastSyncUserData, resourcePreviews, isLastSyncFromCurrentMachine: isRemoteDataFromCurrentMachine }; } async getLastSyncUserData(): Promise { @@ -716,7 +719,7 @@ export abstract class AbstractSynchroniser extends Disposable { } protected abstract readonly version: number; - protected abstract generateSyncPreview(remoteUserData: IRemoteUserData, lastSyncUserData: IRemoteUserData | null, token: CancellationToken): Promise; + protected abstract generateSyncPreview(remoteUserData: IRemoteUserData, lastSyncUserData: IRemoteUserData | null, isRemoteDataFromCurrentMachine: boolean, token: CancellationToken): Promise; protected abstract getMergeResult(resourcePreview: IResourcePreview, token: CancellationToken): Promise; protected abstract getAcceptResult(resourcePreview: IResourcePreview, resource: URI, content: string | null | undefined, token: CancellationToken): Promise; protected abstract applyResult(remoteUserData: IRemoteUserData, lastSyncUserData: IRemoteUserData | null, result: [IResourcePreview, IAcceptResult][], force: boolean): Promise; diff --git a/src/vs/platform/userDataSync/common/extensionsMerge.ts b/src/vs/platform/userDataSync/common/extensionsMerge.ts index 8b10a4c30f1..46141af51df 100644 --- a/src/vs/platform/userDataSync/common/extensionsMerge.ts +++ b/src/vs/platform/userDataSync/common/extensionsMerge.ts @@ -10,10 +10,8 @@ import { IStringDictionary } from 'vs/base/common/collections'; import * as semver from 'vs/base/common/semver/semver'; export interface IMergeResult { - added: ISyncExtension[]; - removed: IExtensionIdentifier[]; - updated: ISyncExtensionWithVersion[]; - remote: ISyncExtension[] | null; + readonly local: { added: ISyncExtension[], removed: IExtensionIdentifier[], updated: ISyncExtension[] }; + readonly remote: { added: ISyncExtension[], removed: ISyncExtension[], updated: ISyncExtension[], all: ISyncExtension[] } | null; } export function merge(localExtensions: ISyncExtensionWithVersion[], remoteExtensions: ISyncExtension[] | null, lastSyncExtensions: ISyncExtension[] | null, skippedExtensions: ISyncExtension[], ignoredExtensions: string[]): IMergeResult { @@ -24,10 +22,17 @@ export function merge(localExtensions: ISyncExtensionWithVersion[], remoteExtens if (!remoteExtensions) { const remote = localExtensions.filter(({ identifier }) => ignoredExtensions.every(id => id.toLowerCase() !== identifier.id.toLowerCase())); return { - added, - removed, - updated, - remote: remote.length > 0 ? remote : null + local: { + added, + removed, + updated, + }, + remote: remote.length > 0 ? { + added: remote, + updated: [], + removed: [], + all: remote + } : null }; } @@ -179,7 +184,15 @@ export function merge(localExtensions: ISyncExtensionWithVersion[], remoteExtens newRemoteExtensionsMap.forEach((value, key) => remote.push(massageOutgoingExtension(value, key))); } - return { added, removed, updated, remote: remote.length ? remote : null }; + return { + local: { added, removed, updated }, + remote: remote.length ? { + added: [...remoteChanges.added].map(id => newRemoteExtensionsMap.get(id)!), + updated: [...remoteChanges.updated].map(id => newRemoteExtensionsMap.get(id)!), + removed: [...remoteChanges.removed].map(id => remoteExtensionsMap.get(id)!), + all: remote + } : null + }; } function compare(from: Map | null, to: Map, ignoredExtensions: Set, { checkInstalledProperty, checkVersionProperty }: { checkInstalledProperty: boolean, checkVersionProperty: boolean } = { checkInstalledProperty: false, checkVersionProperty: false }): { added: Set, removed: Set, updated: Set } { diff --git a/src/vs/platform/userDataSync/common/extensionsSync.ts b/src/vs/platform/userDataSync/common/extensionsSync.ts index f4a529b1e32..5f819266e95 100644 --- a/src/vs/platform/userDataSync/common/extensionsSync.ts +++ b/src/vs/platform/userDataSync/common/extensionsSync.ts @@ -14,7 +14,7 @@ import { ExtensionType, IExtensionIdentifier } from 'vs/platform/extensions/comm import { areSameExtensions, getExtensionId, getGalleryExtensionId } from 'vs/platform/extensionManagement/common/extensionManagementUtil'; import { IFileService } from 'vs/platform/files/common/files'; import { IConfigurationService } from 'vs/platform/configuration/common/configuration'; -import { merge } from 'vs/platform/userDataSync/common/extensionsMerge'; +import { merge, IMergeResult as IExtensionMergeResult } from 'vs/platform/userDataSync/common/extensionsMerge'; import { AbstractInitializer, AbstractSynchroniser, IAcceptResult, IMergeResult, IResourcePreview } from 'vs/platform/userDataSync/common/abstractSynchronizer'; import { ITelemetryService } from 'vs/platform/telemetry/common/telemetry'; import { URI } from 'vs/base/common/uri'; @@ -29,12 +29,7 @@ import { IStringDictionary } from 'vs/base/common/collections'; import { IExtensionsStorageSyncService } from 'vs/platform/userDataSync/common/extensionsStorageSync'; import { Promises } from 'vs/base/common/async'; -interface IExtensionResourceMergeResult extends IAcceptResult { - readonly added: ISyncExtension[]; - readonly removed: IExtensionIdentifier[]; - readonly updated: ISyncExtension[]; - readonly remote: ISyncExtension[] | null; -} +type IExtensionResourceMergeResult = IAcceptResult & IExtensionMergeResult; interface IExtensionResourcePreview extends IResourcePreview { readonly localExtensions: ISyncExtensionWithVersion[]; @@ -143,14 +138,11 @@ export class ExtensionsSynchroniser extends AbstractSynchroniser implements IUse this.logService.trace(`${this.syncResourceLogLabel}: Remote extensions does not exist. Synchronizing extensions for the first time.`); } - const { added, removed, updated, remote } = merge(localExtensions, remoteExtensions, lastSyncExtensions, skippedExtensions, ignoredExtensions); + const { local, remote } = merge(localExtensions, remoteExtensions, lastSyncExtensions, skippedExtensions, ignoredExtensions); const previewResult: IExtensionResourceMergeResult = { - added, - removed, - updated, - remote, - content: this.getPreviewContent(localExtensions, added, updated, removed), - localChange: added.length > 0 || removed.length > 0 || updated.length > 0 ? Change.Modified : Change.None, + local, remote, + content: this.getPreviewContent(localExtensions, local.added, local.updated, local.removed), + localChange: local.added.length > 0 || local.removed.length > 0 || local.updated.length > 0 ? Change.Modified : Change.None, remoteChange: remote !== null ? Change.Modified : Change.None, }; @@ -221,14 +213,12 @@ export class ExtensionsSynchroniser extends AbstractSynchroniser implements IUse const installedExtensions = await this.extensionManagementService.getInstalled(); const ignoredExtensions = this.ignoredExtensionsManagementService.getIgnoredExtensions(installedExtensions); const mergeResult = merge(resourcePreview.localExtensions, null, null, resourcePreview.skippedExtensions, ignoredExtensions); - const { added, removed, updated, remote } = mergeResult; + const { local, remote } = mergeResult; return { content: resourcePreview.localContent, - added, - removed, - updated, + local, remote, - localChange: added.length > 0 || removed.length > 0 || updated.length > 0 ? Change.Modified : Change.None, + localChange: local.added.length > 0 || local.removed.length > 0 || local.updated.length > 0 ? Change.Modified : Change.None, remoteChange: remote !== null ? Change.Modified : Change.None, }; } @@ -239,20 +229,19 @@ export class ExtensionsSynchroniser extends AbstractSynchroniser implements IUse const remoteExtensions = resourcePreview.remoteContent ? JSON.parse(resourcePreview.remoteContent) : null; if (remoteExtensions !== null) { const mergeResult = merge(resourcePreview.localExtensions, remoteExtensions, resourcePreview.localExtensions, [], ignoredExtensions); - const { added, removed, updated, remote } = mergeResult; + const { local, remote } = mergeResult; return { content: resourcePreview.remoteContent, - added, - removed, - updated, + local, remote, - localChange: added.length > 0 || removed.length > 0 || updated.length > 0 ? Change.Modified : Change.None, + localChange: local.added.length > 0 || local.removed.length > 0 || local.updated.length > 0 ? Change.Modified : Change.None, remoteChange: remote !== null ? Change.Modified : Change.None, }; } else { return { content: resourcePreview.remoteContent, - added: [], removed: [], updated: [], remote: null, + local: { added: [], removed: [], updated: [] }, + remote: null, localChange: Change.None, remoteChange: Change.None, }; @@ -261,7 +250,7 @@ export class ExtensionsSynchroniser extends AbstractSynchroniser implements IUse protected async applyResult(remoteUserData: IRemoteUserData, lastSyncUserData: IRemoteUserData | null, resourcePreviews: [IExtensionResourcePreview, IExtensionResourceMergeResult][], force: boolean): Promise { let { skippedExtensions, localExtensions } = resourcePreviews[0][0]; - let { added, removed, updated, remote, localChange, remoteChange } = resourcePreviews[0][1]; + let { local, remote, localChange, remoteChange } = resourcePreviews[0][1]; if (localChange === Change.None && remoteChange === Change.None) { this.logService.info(`${this.syncResourceLogLabel}: No changes found during synchronizing extensions.`); @@ -269,22 +258,22 @@ export class ExtensionsSynchroniser extends AbstractSynchroniser implements IUse if (localChange !== Change.None) { await this.backupLocal(JSON.stringify(localExtensions)); - skippedExtensions = await this.updateLocalExtensions(added, removed, updated, skippedExtensions); + skippedExtensions = await this.updateLocalExtensions(local.added, local.removed, local.updated, skippedExtensions); } if (remote) { // update remote this.logService.trace(`${this.syncResourceLogLabel}: Updating remote extensions...`); - const content = JSON.stringify(remote); + const content = JSON.stringify(remote.all); remoteUserData = await this.updateRemoteUserData(content, force ? null : remoteUserData.ref); - this.logService.info(`${this.syncResourceLogLabel}: Updated remote extensions`); + this.logService.info(`${this.syncResourceLogLabel}: Updated remote extensions.${remote.added.length ? ` Added: ${JSON.stringify(remote.added.map(e => e.identifier.id))}.` : ''}${remote.updated.length ? ` Updated: ${JSON.stringify(remote.updated.map(e => e.identifier.id))}.` : ''}${remote.removed.length ? ` Removed: ${JSON.stringify(remote.removed.map(e => e.identifier.id))}.` : ''}`); } if (lastSyncUserData?.ref !== remoteUserData.ref) { // update last sync this.logService.trace(`${this.syncResourceLogLabel}: Updating last synchronized extensions...`); await this.updateLastSyncUserData(remoteUserData, { skippedExtensions }); - this.logService.info(`${this.syncResourceLogLabel}: Updated last synchronized extensions`); + this.logService.info(`${this.syncResourceLogLabel}: Updated last synchronized extensions.${skippedExtensions.length ? ` Skipped: ${JSON.stringify(skippedExtensions.map(e => e.identifier.id))}.` : ''}`); } } diff --git a/src/vs/platform/userDataSync/common/globalStateSync.ts b/src/vs/platform/userDataSync/common/globalStateSync.ts index 545a690e23a..52b7938c189 100644 --- a/src/vs/platform/userDataSync/common/globalStateSync.ts +++ b/src/vs/platform/userDataSync/common/globalStateSync.ts @@ -101,8 +101,11 @@ export class GlobalStateSynchroniser extends AbstractSynchroniser implements IUs ); } - protected async generateSyncPreview(remoteUserData: IRemoteUserData, lastSyncUserData: IRemoteUserData | null, token: CancellationToken): Promise { + protected async generateSyncPreview(remoteUserData: IRemoteUserData, lastSyncUserData: IRemoteUserData | null, isRemoteDataFromCurrentMachine: boolean, token: CancellationToken): Promise { const remoteGlobalState: IGlobalState = remoteUserData.syncData ? JSON.parse(remoteUserData.syncData.content) : null; + + // Use remote data as last sync data if last sync data does not exist and remote data is from same machine + lastSyncUserData = lastSyncUserData === null && isRemoteDataFromCurrentMachine ? remoteUserData : lastSyncUserData; const lastSyncGlobalState: IGlobalState | null = lastSyncUserData && lastSyncUserData.syncData ? JSON.parse(lastSyncUserData.syncData.content) : null; const localGloablState = await this.getLocalGlobalState(); diff --git a/src/vs/platform/userDataSync/common/keybindingsSync.ts b/src/vs/platform/userDataSync/common/keybindingsSync.ts index 38371ccafa7..b2d8bc4eca6 100644 --- a/src/vs/platform/userDataSync/common/keybindingsSync.ts +++ b/src/vs/platform/userDataSync/common/keybindingsSync.ts @@ -80,8 +80,11 @@ export class KeybindingsSynchroniser extends AbstractJsonFileSynchroniser implem this._register(Event.filter(configurationService.onDidChangeConfiguration, e => e.affectsConfiguration('settingsSync.keybindingsPerPlatform'))(() => this.triggerLocalChange())); } - protected async generateSyncPreview(remoteUserData: IRemoteUserData, lastSyncUserData: ILastSyncUserData | null, token: CancellationToken): Promise { + protected async generateSyncPreview(remoteUserData: IRemoteUserData, lastSyncUserData: ILastSyncUserData | null, isRemoteDataFromCurrentMachine: boolean, token: CancellationToken): Promise { const remoteContent = remoteUserData.syncData ? this.getKeybindingsContentFromSyncContent(remoteUserData.syncData.content) : null; + + // Use remote data as last sync data if last sync data does not exist and remote data is from same machine + lastSyncUserData = lastSyncUserData === null && isRemoteDataFromCurrentMachine ? remoteUserData : lastSyncUserData; const lastSyncContent: string | null = lastSyncUserData ? this.getKeybindingsContentFromLastSyncUserData(lastSyncUserData) : null; // Get file content last to get the latest diff --git a/src/vs/platform/userDataSync/common/settingsSync.ts b/src/vs/platform/userDataSync/common/settingsSync.ts index 5a5266e328c..5cd2076be86 100644 --- a/src/vs/platform/userDataSync/common/settingsSync.ts +++ b/src/vs/platform/userDataSync/common/settingsSync.ts @@ -69,10 +69,13 @@ export class SettingsSynchroniser extends AbstractJsonFileSynchroniser implement super(environmentService.settingsResource, SyncResource.Settings, fileService, environmentService, storageService, userDataSyncStoreService, userDataSyncBackupStoreService, userDataSyncResourceEnablementService, telemetryService, logService, userDataSyncUtilService, configurationService); } - protected async generateSyncPreview(remoteUserData: IRemoteUserData, lastSyncUserData: IRemoteUserData | null, token: CancellationToken): Promise { + protected async generateSyncPreview(remoteUserData: IRemoteUserData, lastSyncUserData: IRemoteUserData | null, isRemoteDataFromCurrentMachine: boolean, token: CancellationToken): Promise { const fileContent = await this.getLocalFileContent(); const formattingOptions = await this.getFormattingOptions(); const remoteSettingsSyncContent = this.getSettingsSyncContent(remoteUserData); + + // Use remote data as last sync data if last sync data does not exist and remote data is from same machine + lastSyncUserData = lastSyncUserData === null && isRemoteDataFromCurrentMachine ? remoteUserData : lastSyncUserData; const lastSettingsSyncContent: ISettingsSyncContent | null = lastSyncUserData ? this.getSettingsSyncContent(lastSyncUserData) : null; const ignoredSettings = await this.getIgnoredSettings(); diff --git a/src/vs/platform/userDataSync/common/snippetsSync.ts b/src/vs/platform/userDataSync/common/snippetsSync.ts index adf6e9d1748..4f46c2e2da3 100644 --- a/src/vs/platform/userDataSync/common/snippetsSync.ts +++ b/src/vs/platform/userDataSync/common/snippetsSync.ts @@ -52,10 +52,13 @@ export class SnippetsSynchroniser extends AbstractSynchroniser implements IUserD this._register(Event.filter(this.fileService.onDidFilesChange, e => e.affects(this.snippetsFolder))(() => this.triggerLocalChange())); } - protected async generateSyncPreview(remoteUserData: IRemoteUserData, lastSyncUserData: IRemoteUserData | null, token: CancellationToken): Promise { + protected async generateSyncPreview(remoteUserData: IRemoteUserData, lastSyncUserData: IRemoteUserData | null, isRemoteDataFromCurrentMachine: boolean, token: CancellationToken): Promise { const local = await this.getSnippetsFileContents(); const localSnippets = this.toSnippetsContents(local); const remoteSnippets: IStringDictionary | null = remoteUserData.syncData ? this.parseSnippets(remoteUserData.syncData) : null; + + // Use remote data as last sync data if last sync data does not exist and remote data is from same machine + lastSyncUserData = lastSyncUserData === null && isRemoteDataFromCurrentMachine ? remoteUserData : lastSyncUserData; const lastSyncSnippets: IStringDictionary | null = lastSyncUserData && lastSyncUserData.syncData ? this.parseSnippets(lastSyncUserData.syncData) : null; if (remoteSnippets) { diff --git a/src/vs/platform/userDataSync/test/common/extensionsMerge.test.ts b/src/vs/platform/userDataSync/test/common/extensionsMerge.test.ts index ad643c1b85a..0036701c586 100644 --- a/src/vs/platform/userDataSync/test/common/extensionsMerge.test.ts +++ b/src/vs/platform/userDataSync/test/common/extensionsMerge.test.ts @@ -18,10 +18,10 @@ suite('ExtensionsMerge', () => { const actual = merge(localExtensions, null, null, [], []); - assert.deepStrictEqual(actual.added, []); - assert.deepStrictEqual(actual.removed, []); - assert.deepStrictEqual(actual.updated, []); - assert.deepStrictEqual(actual.remote, localExtensions); + assert.deepStrictEqual(actual.local.added, []); + assert.deepStrictEqual(actual.local.removed, []); + assert.deepStrictEqual(actual.local.updated, []); + assert.deepStrictEqual(actual.remote?.all, localExtensions); }); test('merge returns local extension if remote does not exist with ignored extensions', () => { @@ -37,10 +37,10 @@ suite('ExtensionsMerge', () => { const actual = merge(localExtensions, null, null, [], ['a']); - assert.deepStrictEqual(actual.added, []); - assert.deepStrictEqual(actual.removed, []); - assert.deepStrictEqual(actual.updated, []); - assert.deepStrictEqual(actual.remote, expected); + assert.deepStrictEqual(actual.local.added, []); + assert.deepStrictEqual(actual.local.removed, []); + assert.deepStrictEqual(actual.local.updated, []); + assert.deepStrictEqual(actual.remote?.all, expected); }); test('merge returns local extension if remote does not exist with ignored extensions (ignore case)', () => { @@ -56,10 +56,10 @@ suite('ExtensionsMerge', () => { const actual = merge(localExtensions, null, null, [], ['A']); - assert.deepStrictEqual(actual.added, []); - assert.deepStrictEqual(actual.removed, []); - assert.deepStrictEqual(actual.updated, []); - assert.deepStrictEqual(actual.remote, expected); + assert.deepStrictEqual(actual.local.added, []); + assert.deepStrictEqual(actual.local.removed, []); + assert.deepStrictEqual(actual.local.updated, []); + assert.deepStrictEqual(actual.remote?.all, expected); }); test('merge returns local extension if remote does not exist with skipped extensions', () => { @@ -79,10 +79,10 @@ suite('ExtensionsMerge', () => { const actual = merge(localExtensions, null, null, skippedExtension, []); - assert.deepStrictEqual(actual.added, []); - assert.deepStrictEqual(actual.removed, []); - assert.deepStrictEqual(actual.updated, []); - assert.deepStrictEqual(actual.remote, expected); + assert.deepStrictEqual(actual.local.added, []); + assert.deepStrictEqual(actual.local.removed, []); + assert.deepStrictEqual(actual.local.updated, []); + assert.deepStrictEqual(actual.remote?.all, expected); }); test('merge returns local extension if remote does not exist with skipped and ignored extensions', () => { @@ -101,10 +101,10 @@ suite('ExtensionsMerge', () => { const actual = merge(localExtensions, null, null, skippedExtension, ['a']); - assert.deepStrictEqual(actual.added, []); - assert.deepStrictEqual(actual.removed, []); - assert.deepStrictEqual(actual.updated, []); - assert.deepStrictEqual(actual.remote, expected); + assert.deepStrictEqual(actual.local.added, []); + assert.deepStrictEqual(actual.local.removed, []); + assert.deepStrictEqual(actual.local.updated, []); + assert.deepStrictEqual(actual.remote?.all, expected); }); test('merge local and remote extensions when there is no base', () => { @@ -125,10 +125,10 @@ suite('ExtensionsMerge', () => { const actual = merge(localExtensions, remoteExtensions, null, [], []); - assert.deepStrictEqual(actual.added, [{ identifier: { id: 'b', uuid: 'b' }, installed: true, version: '1.0.0' }, { identifier: { id: 'c', uuid: 'c' }, installed: true, version: '1.0.0' }]); - assert.deepStrictEqual(actual.removed, []); - assert.deepStrictEqual(actual.updated, []); - assert.deepStrictEqual(actual.remote, expected); + assert.deepStrictEqual(actual.local.added, [{ identifier: { id: 'b', uuid: 'b' }, installed: true, version: '1.0.0' }, { identifier: { id: 'c', uuid: 'c' }, installed: true, version: '1.0.0' }]); + assert.deepStrictEqual(actual.local.removed, []); + assert.deepStrictEqual(actual.local.updated, []); + assert.deepStrictEqual(actual.remote?.all, expected); }); test('merge local and remote extensions when there is no base and with ignored extensions', () => { @@ -148,10 +148,10 @@ suite('ExtensionsMerge', () => { const actual = merge(localExtensions, remoteExtensions, null, [], ['a']); - assert.deepStrictEqual(actual.added, [{ identifier: { id: 'b', uuid: 'b' }, installed: true, version: '1.0.0' }, { identifier: { id: 'c', uuid: 'c' }, installed: true, version: '1.0.0' }]); - assert.deepStrictEqual(actual.removed, []); - assert.deepStrictEqual(actual.updated, []); - assert.deepStrictEqual(actual.remote, expected); + assert.deepStrictEqual(actual.local.added, [{ identifier: { id: 'b', uuid: 'b' }, installed: true, version: '1.0.0' }, { identifier: { id: 'c', uuid: 'c' }, installed: true, version: '1.0.0' }]); + assert.deepStrictEqual(actual.local.removed, []); + assert.deepStrictEqual(actual.local.updated, []); + assert.deepStrictEqual(actual.remote?.all, expected); }); test('merge local and remote extensions when remote is moved forwarded', () => { @@ -170,9 +170,9 @@ suite('ExtensionsMerge', () => { const actual = merge(localExtensions, remoteExtensions, baseExtensions, [], []); - assert.deepStrictEqual(actual.added, [{ identifier: { id: 'b', uuid: 'b' }, installed: true, version: '1.0.0' }, { identifier: { id: 'c', uuid: 'c' }, installed: true, version: '1.0.0' }]); - assert.deepStrictEqual(actual.removed, [{ id: 'a', uuid: 'a' }, { id: 'd', uuid: 'd' }]); - assert.deepStrictEqual(actual.updated, []); + assert.deepStrictEqual(actual.local.added, [{ identifier: { id: 'b', uuid: 'b' }, installed: true, version: '1.0.0' }, { identifier: { id: 'c', uuid: 'c' }, installed: true, version: '1.0.0' }]); + assert.deepStrictEqual(actual.local.removed, [{ id: 'a', uuid: 'a' }, { id: 'd', uuid: 'd' }]); + assert.deepStrictEqual(actual.local.updated, []); assert.strictEqual(actual.remote, null); }); @@ -193,9 +193,9 @@ suite('ExtensionsMerge', () => { const actual = merge(localExtensions, remoteExtensions, baseExtensions, [], []); - assert.deepStrictEqual(actual.added, [{ identifier: { id: 'b', uuid: 'b' }, installed: true, version: '1.0.0' }, { identifier: { id: 'c', uuid: 'c' }, installed: true, version: '1.0.0' }]); - assert.deepStrictEqual(actual.removed, [{ id: 'a', uuid: 'a' }]); - assert.deepStrictEqual(actual.updated, [{ identifier: { id: 'd', uuid: 'd' }, disabled: true, installed: true, version: '1.0.0' }]); + assert.deepStrictEqual(actual.local.added, [{ identifier: { id: 'b', uuid: 'b' }, installed: true, version: '1.0.0' }, { identifier: { id: 'c', uuid: 'c' }, installed: true, version: '1.0.0' }]); + assert.deepStrictEqual(actual.local.removed, [{ id: 'a', uuid: 'a' }]); + assert.deepStrictEqual(actual.local.updated, [{ identifier: { id: 'd', uuid: 'd' }, disabled: true, installed: true, version: '1.0.0' }]); assert.strictEqual(actual.remote, null); }); @@ -215,9 +215,9 @@ suite('ExtensionsMerge', () => { const actual = merge(localExtensions, remoteExtensions, baseExtensions, [], ['a']); - assert.deepStrictEqual(actual.added, [{ identifier: { id: 'b', uuid: 'b' }, installed: true, version: '1.0.0' }, { identifier: { id: 'c', uuid: 'c' }, installed: true, version: '1.0.0' }]); - assert.deepStrictEqual(actual.removed, [{ id: 'd', uuid: 'd' }]); - assert.deepStrictEqual(actual.updated, []); + assert.deepStrictEqual(actual.local.added, [{ identifier: { id: 'b', uuid: 'b' }, installed: true, version: '1.0.0' }, { identifier: { id: 'c', uuid: 'c' }, installed: true, version: '1.0.0' }]); + assert.deepStrictEqual(actual.local.removed, [{ id: 'd', uuid: 'd' }]); + assert.deepStrictEqual(actual.local.updated, []); assert.strictEqual(actual.remote, null); }); @@ -239,9 +239,9 @@ suite('ExtensionsMerge', () => { const actual = merge(localExtensions, remoteExtensions, baseExtensions, skippedExtensions, []); - assert.deepStrictEqual(actual.added, [{ identifier: { id: 'b', uuid: 'b' }, installed: true, version: '1.0.0' }, { identifier: { id: 'c', uuid: 'c' }, installed: true, version: '1.0.0' }]); - assert.deepStrictEqual(actual.removed, [{ id: 'd', uuid: 'd' }]); - assert.deepStrictEqual(actual.updated, []); + assert.deepStrictEqual(actual.local.added, [{ identifier: { id: 'b', uuid: 'b' }, installed: true, version: '1.0.0' }, { identifier: { id: 'c', uuid: 'c' }, installed: true, version: '1.0.0' }]); + assert.deepStrictEqual(actual.local.removed, [{ id: 'd', uuid: 'd' }]); + assert.deepStrictEqual(actual.local.updated, []); assert.strictEqual(actual.remote, null); }); @@ -263,9 +263,9 @@ suite('ExtensionsMerge', () => { const actual = merge(localExtensions, remoteExtensions, baseExtensions, skippedExtensions, ['b']); - assert.deepStrictEqual(actual.added, [{ identifier: { id: 'c', uuid: 'c' }, installed: true, version: '1.0.0' }]); - assert.deepStrictEqual(actual.removed, [{ id: 'd', uuid: 'd' }]); - assert.deepStrictEqual(actual.updated, []); + assert.deepStrictEqual(actual.local.added, [{ identifier: { id: 'c', uuid: 'c' }, installed: true, version: '1.0.0' }]); + assert.deepStrictEqual(actual.local.removed, [{ id: 'd', uuid: 'd' }]); + assert.deepStrictEqual(actual.local.updated, []); assert.strictEqual(actual.remote, null); }); @@ -285,10 +285,10 @@ suite('ExtensionsMerge', () => { const actual = merge(localExtensions, remoteExtensions, baseExtensions, [], []); - assert.deepStrictEqual(actual.added, []); - assert.deepStrictEqual(actual.removed, []); - assert.deepStrictEqual(actual.updated, []); - assert.deepStrictEqual(actual.remote, localExtensions); + assert.deepStrictEqual(actual.local.added, []); + assert.deepStrictEqual(actual.local.removed, []); + assert.deepStrictEqual(actual.local.updated, []); + assert.deepStrictEqual(actual.remote?.all, localExtensions); }); test('merge local and remote extensions when local is moved forwarded with disabled extensions', () => { @@ -308,10 +308,10 @@ suite('ExtensionsMerge', () => { const actual = merge(localExtensions, remoteExtensions, baseExtensions, [], []); - assert.deepStrictEqual(actual.added, []); - assert.deepStrictEqual(actual.removed, []); - assert.deepStrictEqual(actual.updated, []); - assert.deepStrictEqual(actual.remote, localExtensions); + assert.deepStrictEqual(actual.local.added, []); + assert.deepStrictEqual(actual.local.removed, []); + assert.deepStrictEqual(actual.local.updated, []); + assert.deepStrictEqual(actual.remote?.all, localExtensions); }); test('merge local and remote extensions when local is moved forwarded with ignored settings', () => { @@ -330,10 +330,10 @@ suite('ExtensionsMerge', () => { const actual = merge(localExtensions, remoteExtensions, baseExtensions, [], ['b']); - assert.deepStrictEqual(actual.added, []); - assert.deepStrictEqual(actual.removed, []); - assert.deepStrictEqual(actual.updated, []); - assert.deepStrictEqual(actual.remote, [ + assert.deepStrictEqual(actual.local.added, []); + assert.deepStrictEqual(actual.local.removed, []); + assert.deepStrictEqual(actual.local.updated, []); + assert.deepStrictEqual(actual.remote?.all, [ { identifier: { id: 'c', uuid: 'c' }, installed: true, version: '1.0.0' }, ]); }); @@ -362,10 +362,10 @@ suite('ExtensionsMerge', () => { const actual = merge(localExtensions, remoteExtensions, baseExtensions, skippedExtensions, []); - assert.deepStrictEqual(actual.added, []); - assert.deepStrictEqual(actual.removed, []); - assert.deepStrictEqual(actual.updated, []); - assert.deepStrictEqual(actual.remote, expected); + assert.deepStrictEqual(actual.local.added, []); + assert.deepStrictEqual(actual.local.removed, []); + assert.deepStrictEqual(actual.local.updated, []); + assert.deepStrictEqual(actual.remote?.all, expected); }); test('merge local and remote extensions when local is moved forwarded with skipped and ignored extensions', () => { @@ -391,10 +391,10 @@ suite('ExtensionsMerge', () => { const actual = merge(localExtensions, remoteExtensions, baseExtensions, skippedExtensions, ['c']); - assert.deepStrictEqual(actual.added, []); - assert.deepStrictEqual(actual.removed, []); - assert.deepStrictEqual(actual.updated, []); - assert.deepStrictEqual(actual.remote, expected); + assert.deepStrictEqual(actual.local.added, []); + assert.deepStrictEqual(actual.local.removed, []); + assert.deepStrictEqual(actual.local.updated, []); + assert.deepStrictEqual(actual.remote?.all, expected); }); test('merge local and remote extensions when both moved forwarded', () => { @@ -420,10 +420,10 @@ suite('ExtensionsMerge', () => { const actual = merge(localExtensions, remoteExtensions, baseExtensions, [], []); - assert.deepStrictEqual(actual.added, [{ identifier: { id: 'e', uuid: 'e' }, installed: true, version: '1.0.0' }]); - assert.deepStrictEqual(actual.removed, [{ id: 'a', uuid: 'a' }]); - assert.deepStrictEqual(actual.updated, []); - assert.deepStrictEqual(actual.remote, expected); + assert.deepStrictEqual(actual.local.added, [{ identifier: { id: 'e', uuid: 'e' }, installed: true, version: '1.0.0' }]); + assert.deepStrictEqual(actual.local.removed, [{ id: 'a', uuid: 'a' }]); + assert.deepStrictEqual(actual.local.updated, []); + assert.deepStrictEqual(actual.remote?.all, expected); }); test('merge local and remote extensions when both moved forwarded with ignored extensions', () => { @@ -449,10 +449,10 @@ suite('ExtensionsMerge', () => { const actual = merge(localExtensions, remoteExtensions, baseExtensions, [], ['a', 'e']); - assert.deepStrictEqual(actual.added, []); - assert.deepStrictEqual(actual.removed, []); - assert.deepStrictEqual(actual.updated, []); - assert.deepStrictEqual(actual.remote, expected); + assert.deepStrictEqual(actual.local.added, []); + assert.deepStrictEqual(actual.local.removed, []); + assert.deepStrictEqual(actual.local.updated, []); + assert.deepStrictEqual(actual.remote?.all, expected); }); test('merge local and remote extensions when both moved forwarded with skipped extensions', () => { @@ -480,10 +480,10 @@ suite('ExtensionsMerge', () => { const actual = merge(localExtensions, remoteExtensions, baseExtensions, skippedExtensions, []); - assert.deepStrictEqual(actual.added, [{ identifier: { id: 'e', uuid: 'e' }, installed: true, version: '1.0.0' }]); - assert.deepStrictEqual(actual.removed, []); - assert.deepStrictEqual(actual.updated, []); - assert.deepStrictEqual(actual.remote, expected); + assert.deepStrictEqual(actual.local.added, [{ identifier: { id: 'e', uuid: 'e' }, installed: true, version: '1.0.0' }]); + assert.deepStrictEqual(actual.local.removed, []); + assert.deepStrictEqual(actual.local.updated, []); + assert.deepStrictEqual(actual.remote?.all, expected); }); test('merge local and remote extensions when both moved forwarded with skipped and ignoredextensions', () => { @@ -511,10 +511,10 @@ suite('ExtensionsMerge', () => { const actual = merge(localExtensions, remoteExtensions, baseExtensions, skippedExtensions, ['e']); - assert.deepStrictEqual(actual.added, []); - assert.deepStrictEqual(actual.removed, []); - assert.deepStrictEqual(actual.updated, []); - assert.deepStrictEqual(actual.remote, expected); + assert.deepStrictEqual(actual.local.added, []); + assert.deepStrictEqual(actual.local.removed, []); + assert.deepStrictEqual(actual.local.updated, []); + assert.deepStrictEqual(actual.remote?.all, expected); }); test('merge when remote extension has no uuid and different extension id case', () => { @@ -536,10 +536,10 @@ suite('ExtensionsMerge', () => { const actual = merge(localExtensions, remoteExtensions, null, [], []); - assert.deepStrictEqual(actual.added, [{ identifier: { id: 'd', uuid: 'd' }, installed: true, version: '1.0.0' }]); - assert.deepStrictEqual(actual.removed, []); - assert.deepStrictEqual(actual.updated, []); - assert.deepStrictEqual(actual.remote, expected); + assert.deepStrictEqual(actual.local.added, [{ identifier: { id: 'd', uuid: 'd' }, installed: true, version: '1.0.0' }]); + assert.deepStrictEqual(actual.local.removed, []); + assert.deepStrictEqual(actual.local.updated, []); + assert.deepStrictEqual(actual.remote?.all, expected); }); test('merge when remote extension is not an installed extension', () => { @@ -553,9 +553,9 @@ suite('ExtensionsMerge', () => { const actual = merge(localExtensions, remoteExtensions, null, [], []); - assert.deepStrictEqual(actual.added, []); - assert.deepStrictEqual(actual.removed, []); - assert.deepStrictEqual(actual.updated, []); + assert.deepStrictEqual(actual.local.added, []); + assert.deepStrictEqual(actual.local.removed, []); + assert.deepStrictEqual(actual.local.updated, []); assert.deepStrictEqual(actual.remote, null); }); @@ -569,10 +569,10 @@ suite('ExtensionsMerge', () => { const actual = merge(localExtensions, remoteExtensions, null, [], []); - assert.deepStrictEqual(actual.added, []); - assert.deepStrictEqual(actual.removed, []); - assert.deepStrictEqual(actual.updated, []); - assert.deepStrictEqual(actual.remote, localExtensions); + assert.deepStrictEqual(actual.local.added, []); + assert.deepStrictEqual(actual.local.removed, []); + assert.deepStrictEqual(actual.local.updated, []); + assert.deepStrictEqual(actual.remote?.all, localExtensions); }); test('merge when an extension is not an installed extension remotely and does not exist locally', () => { @@ -586,9 +586,9 @@ suite('ExtensionsMerge', () => { const actual = merge(localExtensions, remoteExtensions, remoteExtensions, [], []); - assert.deepStrictEqual(actual.added, []); - assert.deepStrictEqual(actual.removed, []); - assert.deepStrictEqual(actual.updated, []); + assert.deepStrictEqual(actual.local.added, []); + assert.deepStrictEqual(actual.local.removed, []); + assert.deepStrictEqual(actual.local.updated, []); assert.deepStrictEqual(actual.remote, null); }); @@ -605,10 +605,10 @@ suite('ExtensionsMerge', () => { const actual = merge(localExtensions, remoteExtensions, remoteExtensions, [], []); - assert.deepStrictEqual(actual.added, []); - assert.deepStrictEqual(actual.removed, []); - assert.deepStrictEqual(actual.updated, []); - assert.deepStrictEqual(actual.remote, expected); + assert.deepStrictEqual(actual.local.added, []); + assert.deepStrictEqual(actual.local.removed, []); + assert.deepStrictEqual(actual.local.updated, []); + assert.deepStrictEqual(actual.remote?.all, expected); }); test('merge when an extension is an installed extension remotely but not locally and updated remotely', () => { @@ -621,9 +621,9 @@ suite('ExtensionsMerge', () => { const actual = merge(localExtensions, remoteExtensions, localExtensions, [], []); - assert.deepStrictEqual(actual.added, []); - assert.deepStrictEqual(actual.removed, []); - assert.deepStrictEqual(actual.updated, remoteExtensions); + assert.deepStrictEqual(actual.local.added, []); + assert.deepStrictEqual(actual.local.removed, []); + assert.deepStrictEqual(actual.local.updated, remoteExtensions); assert.deepStrictEqual(actual.remote, null); }); @@ -641,10 +641,10 @@ suite('ExtensionsMerge', () => { const actual = merge(localExtensions, remoteExtensions, null, [], []); - assert.deepStrictEqual(actual.added, []); - assert.deepStrictEqual(actual.removed, []); - assert.deepStrictEqual(actual.updated, []); - assert.deepStrictEqual(actual.remote, expected); + assert.deepStrictEqual(actual.local.added, []); + assert.deepStrictEqual(actual.local.removed, []); + assert.deepStrictEqual(actual.local.updated, []); + assert.deepStrictEqual(actual.remote?.all, expected); }); }); diff --git a/src/vs/platform/userDataSync/test/common/synchronizer.test.ts b/src/vs/platform/userDataSync/test/common/synchronizer.test.ts index ee64a1cf279..8ccd8050ddf 100644 --- a/src/vs/platform/userDataSync/test/common/synchronizer.test.ts +++ b/src/vs/platform/userDataSync/test/common/synchronizer.test.ts @@ -53,7 +53,7 @@ class TestSynchroniser extends AbstractSynchroniser { return super.doSync(remoteUserData, lastSyncUserData, apply); } - protected async generateSyncPreview(remoteUserData: IRemoteUserData, lastSyncUserData: IRemoteUserData | null, token: CancellationToken): Promise { + protected override async generateSyncPreview(remoteUserData: IRemoteUserData, lastSyncUserData: IRemoteUserData | null, isRemoteDataFromCurrentMachine: boolean, token: CancellationToken): Promise { if (this.syncResult.hasError) { throw new Error('failed'); } diff --git a/src/vs/workbench/services/extensionManagement/common/webExtensionsScannerService.ts b/src/vs/workbench/services/extensionManagement/common/webExtensionsScannerService.ts index ff8f2cc6a04..f53f45652e8 100644 --- a/src/vs/workbench/services/extensionManagement/common/webExtensionsScannerService.ts +++ b/src/vs/workbench/services/extensionManagement/common/webExtensionsScannerService.ts @@ -21,7 +21,6 @@ import { groupByExtension, areSameExtensions, getGalleryExtensionId } from 'vs/p import { IConfigurationService } from 'vs/platform/configuration/common/configuration'; import type { IStaticExtension } from 'vs/workbench/workbench.web.api'; import { Disposable } from 'vs/base/common/lifecycle'; -import { Event } from 'vs/base/common/event'; import { localizeManifest } from 'vs/platform/extensionManagement/common/extensionNls'; import { localize } from 'vs/nls'; import * as semver from 'vs/base/common/semver/semver'; @@ -54,8 +53,6 @@ export class WebExtensionsScannerService extends Disposable implements IWebExten private readonly extensionsResource: URI | undefined = undefined; private readonly userExtensionsResourceLimiter: Queue = new Queue(); - private userExtensionsPromise: Promise | undefined; - constructor( @IWorkbenchEnvironmentService private readonly environmentService: IWorkbenchEnvironmentService, @IBuiltinExtensionsScannerService private readonly builtinExtensionsScannerService: IBuiltinExtensionsScannerService, @@ -69,9 +66,6 @@ export class WebExtensionsScannerService extends Disposable implements IWebExten this.extensionsResource = joinPath(environmentService.userRoamingDataHome, 'extensions.json'); this.systemExtensionsPromise = this.readSystemExtensions(); this.defaultExtensionsPromise = this.readDefaultExtensions(); - if (this.extensionsResource) { - this._register(Event.filter(this.fileService.onDidFilesChange, e => e.contains(this.extensionsResource!))(() => this.userExtensionsPromise = undefined)); - } } } @@ -191,10 +185,7 @@ export class WebExtensionsScannerService extends Disposable implements IWebExten if (type === undefined || type === ExtensionType.User) { const staticExtensions = await this.defaultExtensionsPromise; extensions.push(...staticExtensions); - if (!this.userExtensionsPromise) { - this.userExtensionsPromise = this.scanUserExtensions(); - } - const userExtensions = await this.userExtensionsPromise; + const userExtensions = await this.scanUserExtensions(); extensions.push(...userExtensions); } return extensions; @@ -383,7 +374,6 @@ export class WebExtensionsScannerService extends Disposable implements IWebExten packageNLSUri: e.packageNLSUri?.toJSON(), })); await this.fileService.writeFile(this.extensionsResource!, VSBuffer.fromString(JSON.stringify(storedUserExtensions))); - this.userExtensionsPromise = undefined; return userExtensions; }); }