fix global state syncing

- do not remove if not registered
- pull replaces local completely

src/vs/platform/userDataSync/common/globalStateMerge.ts

@@ -15,10 +15,13 @@ export interface IMergeResult {
 	remote: IStringDictionary<IStorageValue> | null;
 }
 
-export function merge(localStorage: IStringDictionary<IStorageValue>, remoteStorage: IStringDictionary<IStorageValue> | null, baseStorage: IStringDictionary<IStorageValue> | null, storageKeys: ReadonlyArray<IStorageKey>, logService: ILogService): IMergeResult {
+export function merge(localStorage: IStringDictionary<IStorageValue> | null, remoteStorage: IStringDictionary<IStorageValue> | null, baseStorage: IStringDictionary<IStorageValue> | null, storageKeys: ReadonlyArray<IStorageKey>, logService: ILogService): IMergeResult {
 	if (!remoteStorage) {
 		return { remote: localStorage, local: { added: {}, removed: [], updated: {} } };
 	}
+	if (!localStorage) {
+		return { remote: null, local: { added: remoteStorage, removed: [], updated: {} } };
+	}
 
 	const localToRemote = compare(localStorage, remoteStorage);
 	if (localToRemote.added.size === 0 && localToRemote.removed.size === 0 && localToRemote.updated.size === 0) {
@@ -37,11 +40,11 @@ export function merge(localStorage: IStringDictionary<IStorageValue>, remoteStor
 		const remoteValue = remoteStorage[key];
 		const storageKey = storageKeys.filter(storageKey => storageKey.key === key)[0];
 		if (!storageKey) {
-			logService.info(`GlobalState: Skipped updating ${key} in storage. It is not registered to sync.`);
+			logService.info(`GlobalState: Skipped adding ${key} in local storage as it is not registered.`);
 			continue;
 		}
 		if (storageKey.version !== remoteValue.version) {
-			logService.info(`GlobalState: Skipped updating ${key} in storage. Local version '${storageKey.version}' and remote version '${remoteValue.version} are not same.`);
+			logService.info(`GlobalState: Skipped adding ${key} in local storage. Local version '${storageKey.version}' and remote version '${remoteValue.version} are not same.`);
 			continue;
 		}
 		const localValue = localStorage[key];
@@ -60,11 +63,11 @@ export function merge(localStorage: IStringDictionary<IStorageValue>, remoteStor
 		const remoteValue = remoteStorage[key];
 		const storageKey = storageKeys.filter(storageKey => storageKey.key === key)[0];
 		if (!storageKey) {
-			logService.info(`GlobalState: Skipped updating ${key} in storage. It is not registered to sync.`);
+			logService.info(`GlobalState: Skipped updating ${key} in local storage as is not registered.`);
 			continue;
 		}
 		if (storageKey.version !== remoteValue.version) {
-			logService.info(`GlobalState: Skipped updating ${key} in storage. Local version '${storageKey.version}' and remote version '${remoteValue.version} are not same.`);
+			logService.info(`GlobalState: Skipped updating ${key} in local storage. Local version '${storageKey.version}' and remote version '${remoteValue.version} are not same.`);
 			continue;
 		}
 		const localValue = localStorage[key];
@@ -78,7 +81,7 @@ export function merge(localStorage: IStringDictionary<IStorageValue>, remoteStor
 	for (const key of values(baseToRemote.removed)) {
 		const storageKey = storageKeys.filter(storageKey => storageKey.key === key)[0];
 		if (!storageKey) {
-			logService.info(`GlobalState: Skipped updating ${key} in storage. It is not registered to sync.`);
+			logService.info(`GlobalState: Skipped removing ${key} in local storage. It is not registered to sync.`);
 			continue;
 		}
 		local.removed.push(key);
@@ -99,6 +102,7 @@ export function merge(localStorage: IStringDictionary<IStorageValue>, remoteStor
 		const remoteValue = remote[key];
 		const localValue = localStorage[key];
 		if (localValue.version < remoteValue.version) {
+			logService.info(`GlobalState: Skipped updating ${key} in remote storage. Local version '${localValue.version}' and remote version '${remoteValue.version} are not same.`);
 			continue;
 		}
 		remote[key] = localValue;
@@ -109,11 +113,21 @@ export function merge(localStorage: IStringDictionary<IStorageValue>, remoteStor
 		if (baseToRemote.updated.has(key)) {
 			continue;
 		}
-		const remoteValue = remote[key];
+
 		const storageKey = storageKeys.filter(storageKey => storageKey.key === key)[0];
-		if (storageKey && storageKey.version < remoteValue.version) {
+		// do not remove from remote if storage key is not found
+		if (!storageKey) {
+			logService.info(`GlobalState: Skipped removing ${key} in remote storage. It is not registered to sync.`);
 			continue;
 		}
+
+		const remoteValue = remote[key];
+		// do not remove from remote if local data version is old
+		if (storageKey.version < remoteValue.version) {
+			logService.info(`GlobalState: Skipped updating ${key} in remote storage. Local version '${storageKey.version}' and remote version '${remoteValue.version} are not same.`);
+			continue;
+		}
+
 		delete remote[key];
 	}
 

src/vs/platform/userDataSync/common/globalStateSync.ts

@@ -51,8 +51,16 @@ export class GlobalStateSynchroniser extends AbstractSynchroniser implements IUs
 	) {
 		super(SyncResource.GlobalState, fileService, environmentService, userDataSyncStoreService, userDataSyncBackupStoreService, userDataSyncEnablementService, telemetryService, logService, configurationService);
 		this._register(this.fileService.watch(dirname(this.environmentService.argvResource)));
-		this._register(Event.filter(this.fileService.onDidFilesChange, e => e.contains(this.environmentService.argvResource))(() => this._onDidChangeLocal.fire()));
-		this._register(Event.filter(this.storageService.onDidChangeStorage, e => storageKeysSyncRegistryService.storageKeys.some(({ key }) => e.key === key))(() => this._onDidChangeLocal.fire()));
+		this._register(
+			Event.any(
+				/* Locale change */
+				Event.filter(this.fileService.onDidFilesChange, e => e.contains(this.environmentService.argvResource)),
+				/* Storage change */
+				Event.filter(this.storageService.onDidChangeStorage, e => storageKeysSyncRegistryService.storageKeys.some(({ key }) => e.key === key)),
+				/* Storage key registered */
+				this.storageKeysSyncRegistryService.onDidChangeStorageKeys
+			)((() => this._onDidChangeLocal.fire()))
+		);
 	}
 
 	async pull(): Promise<void> {

src/vs/platform/userDataSync/common/userDataSyncStoreService.ts

@@ -75,6 +75,7 @@ export class UserDataSyncStoreService extends Disposable implements IUserDataSyn
 
 		const url = joinPath(this.userDataSyncStore.url, 'resource', resource, ref).toString();
 		const headers: IHeaders = {};
+		headers['Cache-Control'] = 'no-cache';
 
 		const context = await this.request({ type: 'GET', url, headers }, undefined, CancellationToken.None);
 

src/vs/platform/userDataSync/test/common/globalStateMerge.test.ts

@@ -364,4 +364,17 @@ suite('GlobalStateMerge', () => {
 		assert.deepEqual(actual.remote, local);
 	});
 
+	test('merge when a local value is not yet registered', async () => {
+		const base = { 'a': { version: 1, value: 'a' }, 'b': { version: 1, value: 'b' } };
+		const local = { 'a': { version: 1, value: 'a' } };
+		const remote = { 'b': { version: 1, value: 'b' }, 'a': { version: 1, value: 'a' } };
+
+		const actual = merge(local, remote, base, [{ key: 'a', version: 1 }], new NullLogService());
+
+		assert.deepEqual(actual.local.added, {});
+		assert.deepEqual(actual.local.updated, {});
+		assert.deepEqual(actual.local.removed, []);
+		assert.deepEqual(actual.remote, null);
+	});
+
 });

src/vs/workbench/browser/parts/activitybar/activitybarPart.ts

@@ -115,8 +115,8 @@ export class ActivitybarPart extends Part implements IActivityBarService {
 		@IProductService private readonly productService: IProductService
 	) {
 		super(Parts.ACTIVITYBAR_PART, { hasTitle: false }, themeService, storageService, layoutService);
-		this.migrateFromOldCachedViewletsValue();
 		storageKeysSyncRegistryService.registerStorageKey({ key: ActivitybarPart.PINNED_VIEWLETS, version: 1 });
+		this.migrateFromOldCachedViewletsValue();
 
 		this.cachedViewlets = this.getCachedViewlets();
 		for (const cachedViewlet of this.cachedViewlets) {

src/vs/workbench/browser/parts/statusbar/statusbarPart.ts

@@ -359,9 +359,9 @@ export class StatusbarPart extends Part implements IStatusbarService {
 	) {
 		super(Parts.STATUSBAR_PART, { hasTitle: false }, themeService, storageService, layoutService);
 
+		storageKeysSyncRegistryService.registerStorageKey({ key: StatusbarViewModel.HIDDEN_ENTRIES_KEY, version: 1 });
 		this.viewModel = this._register(new StatusbarViewModel(storageService));
 		this.onDidChangeEntryVisibility = this.viewModel.onDidChangeEntryVisibility;
-		storageKeysSyncRegistryService.registerStorageKey({ key: StatusbarViewModel.HIDDEN_ENTRIES_KEY, version: 1 });
 
 		this.registerListeners();
 	}

src/vs/workbench/browser/parts/views/views.ts

@@ -338,13 +338,14 @@ export class PersistentContributableViewsModel extends ContributableViewsModel {
 		@IStorageKeysSyncRegistryService storageKeysSyncRegistryService: IStorageKeysSyncRegistryService
 	) {
 		const globalViewsStateStorageId = `${viewletStateStorageId}.hidden`;
+		storageKeysSyncRegistryService.registerStorageKey({ key: globalViewsStateStorageId, version: 1 });
 		const viewStates = PersistentContributableViewsModel.loadViewsStates(viewletStateStorageId, globalViewsStateStorageId, storageService);
 
 		super(container, viewDescriptorService, viewStates);
 
+		this.storageService = storageService;
 		this.workspaceViewsStateStorageId = viewletStateStorageId;
 		this.globalViewsStateStorageId = globalViewsStateStorageId;
-		this.storageService = storageService;
 
 		this._register(Event.any(
 			this.onDidAdd,
@@ -353,7 +354,6 @@ export class PersistentContributableViewsModel extends ContributableViewsModel {
 			Event.map(this.onDidChangeViewState, viewDescriptorRef => [viewDescriptorRef]))
 			(viewDescriptorRefs => this.saveViewsStates()));
 
-		storageKeysSyncRegistryService.registerStorageKey({ key: this.globalViewsStateStorageId, version: 1 });
 		this._globalViewsStatesValue = this.getStoredGlobalViewsStatesValue();
 		this._register(this.storageService.onDidChangeStorage(e => this.onDidStorageChange(e)));
 	}