Skip to content

Commit ce60450

Browse files
committed
fix global state syncing
- do not remove if not registered - pull replaces local completely
1 parent 85119af commit ce60450

7 files changed

Lines changed: 50 additions & 14 deletions

File tree

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

Lines changed: 22 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -15,10 +15,13 @@ export interface IMergeResult {
1515
remote: IStringDictionary<IStorageValue> | null;
1616
}
1717

18-
export function merge(localStorage: IStringDictionary<IStorageValue>, remoteStorage: IStringDictionary<IStorageValue> | null, baseStorage: IStringDictionary<IStorageValue> | null, storageKeys: ReadonlyArray<IStorageKey>, logService: ILogService): IMergeResult {
18+
export function merge(localStorage: IStringDictionary<IStorageValue> | null, remoteStorage: IStringDictionary<IStorageValue> | null, baseStorage: IStringDictionary<IStorageValue> | null, storageKeys: ReadonlyArray<IStorageKey>, logService: ILogService): IMergeResult {
1919
if (!remoteStorage) {
2020
return { remote: localStorage, local: { added: {}, removed: [], updated: {} } };
2121
}
22+
if (!localStorage) {
23+
return { remote: null, local: { added: remoteStorage, removed: [], updated: {} } };
24+
}
2225

2326
const localToRemote = compare(localStorage, remoteStorage);
2427
if (localToRemote.added.size === 0 && localToRemote.removed.size === 0 && localToRemote.updated.size === 0) {
@@ -37,11 +40,11 @@ export function merge(localStorage: IStringDictionary<IStorageValue>, remoteStor
3740
const remoteValue = remoteStorage[key];
3841
const storageKey = storageKeys.filter(storageKey => storageKey.key === key)[0];
3942
if (!storageKey) {
40-
logService.info(`GlobalState: Skipped updating ${key} in storage. It is not registered to sync.`);
43+
logService.info(`GlobalState: Skipped adding ${key} in local storage as it is not registered.`);
4144
continue;
4245
}
4346
if (storageKey.version !== remoteValue.version) {
44-
logService.info(`GlobalState: Skipped updating ${key} in storage. Local version '${storageKey.version}' and remote version '${remoteValue.version} are not same.`);
47+
logService.info(`GlobalState: Skipped adding ${key} in local storage. Local version '${storageKey.version}' and remote version '${remoteValue.version} are not same.`);
4548
continue;
4649
}
4750
const localValue = localStorage[key];
@@ -60,11 +63,11 @@ export function merge(localStorage: IStringDictionary<IStorageValue>, remoteStor
6063
const remoteValue = remoteStorage[key];
6164
const storageKey = storageKeys.filter(storageKey => storageKey.key === key)[0];
6265
if (!storageKey) {
63-
logService.info(`GlobalState: Skipped updating ${key} in storage. It is not registered to sync.`);
66+
logService.info(`GlobalState: Skipped updating ${key} in local storage as is not registered.`);
6467
continue;
6568
}
6669
if (storageKey.version !== remoteValue.version) {
67-
logService.info(`GlobalState: Skipped updating ${key} in storage. Local version '${storageKey.version}' and remote version '${remoteValue.version} are not same.`);
70+
logService.info(`GlobalState: Skipped updating ${key} in local storage. Local version '${storageKey.version}' and remote version '${remoteValue.version} are not same.`);
6871
continue;
6972
}
7073
const localValue = localStorage[key];
@@ -78,7 +81,7 @@ export function merge(localStorage: IStringDictionary<IStorageValue>, remoteStor
7881
for (const key of values(baseToRemote.removed)) {
7982
const storageKey = storageKeys.filter(storageKey => storageKey.key === key)[0];
8083
if (!storageKey) {
81-
logService.info(`GlobalState: Skipped updating ${key} in storage. It is not registered to sync.`);
84+
logService.info(`GlobalState: Skipped removing ${key} in local storage. It is not registered to sync.`);
8285
continue;
8386
}
8487
local.removed.push(key);
@@ -99,6 +102,7 @@ export function merge(localStorage: IStringDictionary<IStorageValue>, remoteStor
99102
const remoteValue = remote[key];
100103
const localValue = localStorage[key];
101104
if (localValue.version < remoteValue.version) {
105+
logService.info(`GlobalState: Skipped updating ${key} in remote storage. Local version '${localValue.version}' and remote version '${remoteValue.version} are not same.`);
102106
continue;
103107
}
104108
remote[key] = localValue;
@@ -109,11 +113,21 @@ export function merge(localStorage: IStringDictionary<IStorageValue>, remoteStor
109113
if (baseToRemote.updated.has(key)) {
110114
continue;
111115
}
112-
const remoteValue = remote[key];
116+
113117
const storageKey = storageKeys.filter(storageKey => storageKey.key === key)[0];
114-
if (storageKey && storageKey.version < remoteValue.version) {
118+
// do not remove from remote if storage key is not found
119+
if (!storageKey) {
120+
logService.info(`GlobalState: Skipped removing ${key} in remote storage. It is not registered to sync.`);
115121
continue;
116122
}
123+
124+
const remoteValue = remote[key];
125+
// do not remove from remote if local data version is old
126+
if (storageKey.version < remoteValue.version) {
127+
logService.info(`GlobalState: Skipped updating ${key} in remote storage. Local version '${storageKey.version}' and remote version '${remoteValue.version} are not same.`);
128+
continue;
129+
}
130+
117131
delete remote[key];
118132
}
119133

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

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -51,8 +51,16 @@ export class GlobalStateSynchroniser extends AbstractSynchroniser implements IUs
5151
) {
5252
super(SyncResource.GlobalState, fileService, environmentService, userDataSyncStoreService, userDataSyncBackupStoreService, userDataSyncEnablementService, telemetryService, logService, configurationService);
5353
this._register(this.fileService.watch(dirname(this.environmentService.argvResource)));
54-
this._register(Event.filter(this.fileService.onDidFilesChange, e => e.contains(this.environmentService.argvResource))(() => this._onDidChangeLocal.fire()));
55-
this._register(Event.filter(this.storageService.onDidChangeStorage, e => storageKeysSyncRegistryService.storageKeys.some(({ key }) => e.key === key))(() => this._onDidChangeLocal.fire()));
54+
this._register(
55+
Event.any(
56+
/* Locale change */
57+
Event.filter(this.fileService.onDidFilesChange, e => e.contains(this.environmentService.argvResource)),
58+
/* Storage change */
59+
Event.filter(this.storageService.onDidChangeStorage, e => storageKeysSyncRegistryService.storageKeys.some(({ key }) => e.key === key)),
60+
/* Storage key registered */
61+
this.storageKeysSyncRegistryService.onDidChangeStorageKeys
62+
)((() => this._onDidChangeLocal.fire()))
63+
);
5664
}
5765

5866
async pull(): Promise<void> {

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,7 @@ export class UserDataSyncStoreService extends Disposable implements IUserDataSyn
7575

7676
const url = joinPath(this.userDataSyncStore.url, 'resource', resource, ref).toString();
7777
const headers: IHeaders = {};
78+
headers['Cache-Control'] = 'no-cache';
7879

7980
const context = await this.request({ type: 'GET', url, headers }, undefined, CancellationToken.None);
8081

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

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -364,4 +364,17 @@ suite('GlobalStateMerge', () => {
364364
assert.deepEqual(actual.remote, local);
365365
});
366366

367+
test('merge when a local value is not yet registered', async () => {
368+
const base = { 'a': { version: 1, value: 'a' }, 'b': { version: 1, value: 'b' } };
369+
const local = { 'a': { version: 1, value: 'a' } };
370+
const remote = { 'b': { version: 1, value: 'b' }, 'a': { version: 1, value: 'a' } };
371+
372+
const actual = merge(local, remote, base, [{ key: 'a', version: 1 }], new NullLogService());
373+
374+
assert.deepEqual(actual.local.added, {});
375+
assert.deepEqual(actual.local.updated, {});
376+
assert.deepEqual(actual.local.removed, []);
377+
assert.deepEqual(actual.remote, null);
378+
});
379+
367380
});

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -115,8 +115,8 @@ export class ActivitybarPart extends Part implements IActivityBarService {
115115
@IProductService private readonly productService: IProductService
116116
) {
117117
super(Parts.ACTIVITYBAR_PART, { hasTitle: false }, themeService, storageService, layoutService);
118-
this.migrateFromOldCachedViewletsValue();
119118
storageKeysSyncRegistryService.registerStorageKey({ key: ActivitybarPart.PINNED_VIEWLETS, version: 1 });
119+
this.migrateFromOldCachedViewletsValue();
120120

121121
this.cachedViewlets = this.getCachedViewlets();
122122
for (const cachedViewlet of this.cachedViewlets) {

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -359,9 +359,9 @@ export class StatusbarPart extends Part implements IStatusbarService {
359359
) {
360360
super(Parts.STATUSBAR_PART, { hasTitle: false }, themeService, storageService, layoutService);
361361

362+
storageKeysSyncRegistryService.registerStorageKey({ key: StatusbarViewModel.HIDDEN_ENTRIES_KEY, version: 1 });
362363
this.viewModel = this._register(new StatusbarViewModel(storageService));
363364
this.onDidChangeEntryVisibility = this.viewModel.onDidChangeEntryVisibility;
364-
storageKeysSyncRegistryService.registerStorageKey({ key: StatusbarViewModel.HIDDEN_ENTRIES_KEY, version: 1 });
365365

366366
this.registerListeners();
367367
}

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -338,13 +338,14 @@ export class PersistentContributableViewsModel extends ContributableViewsModel {
338338
@IStorageKeysSyncRegistryService storageKeysSyncRegistryService: IStorageKeysSyncRegistryService
339339
) {
340340
const globalViewsStateStorageId = `${viewletStateStorageId}.hidden`;
341+
storageKeysSyncRegistryService.registerStorageKey({ key: globalViewsStateStorageId, version: 1 });
341342
const viewStates = PersistentContributableViewsModel.loadViewsStates(viewletStateStorageId, globalViewsStateStorageId, storageService);
342343

343344
super(container, viewDescriptorService, viewStates);
344345

346+
this.storageService = storageService;
345347
this.workspaceViewsStateStorageId = viewletStateStorageId;
346348
this.globalViewsStateStorageId = globalViewsStateStorageId;
347-
this.storageService = storageService;
348349

349350
this._register(Event.any(
350351
this.onDidAdd,
@@ -353,7 +354,6 @@ export class PersistentContributableViewsModel extends ContributableViewsModel {
353354
Event.map(this.onDidChangeViewState, viewDescriptorRef => [viewDescriptorRef]))
354355
(viewDescriptorRefs => this.saveViewsStates()));
355356

356-
storageKeysSyncRegistryService.registerStorageKey({ key: this.globalViewsStateStorageId, version: 1 });
357357
this._globalViewsStatesValue = this.getStoredGlobalViewsStatesValue();
358358
this._register(this.storageService.onDidChangeStorage(e => this.onDidStorageChange(e)));
359359
}

0 commit comments

Comments
 (0)