Skip to content

Commit 925b4ff

Browse files
sandy081sandy081
authored
store remote user data in last sync resource (microsoft#166229)
* include ref in stored last sync remote user data * migrate only if content exists
1 parent b53c173 commit 925b4ff

File tree

10 files changed

+122
-27
lines changed

10 files changed

+122
-27
lines changed

‎src/vs/platform/userDataSync/common/abstractSynchronizer.ts

Lines changed: 52 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,16 @@ type IncompatibleSyncSourceClassification = {
3535
source: { classification: 'SystemMetaData'; purpose: 'FeatureInsight'; isMeasurement: true; comment: 'settings sync resource. eg., settings, keybindings...' };
3636
};
3737

38+
export function isRemoteUserData(thing: any): thing is IRemoteUserData {
39+
if (thing
40+
&& (thing.ref !== undefined && typeof thing.ref === 'string' && thing.ref !== '')
41+
&& (thing.syncData !== undefined && (thing.syncData === null || isSyncData(thing.syncData)))) {
42+
return true;
43+
}
44+
45+
return false;
46+
}
47+
3848
export function isSyncData(thing: any): thing is ISyncData {
3949
if (thing
4050
&& (thing.version !== undefined && typeof thing.version === 'number')
@@ -595,11 +605,13 @@ export abstract class AbstractSynchroniser extends Disposable implements IUserDa
595605
let retrial = 1;
596606
while (syncData === undefined && retrial++ < 6 /* Retry 5 times */) {
597607
try {
598-
const content = (await this.fileService.readFile(this.lastSyncResource)).value.toString();
599-
try { syncData = content ? JSON.parse(content) : null; } catch (e) { /* Ignore */ }
600-
if (syncData && !isSyncData(syncData)) {
601-
this.logService.info(`${this.syncResourceLogLabel}: Last sync data stored locally is invalid.`);
602-
syncData = undefined;
608+
const lastSyncStoredRemoteUserData = await this.readLastSyncStoredRemoteUserData();
609+
if (lastSyncStoredRemoteUserData) {
610+
if (lastSyncStoredRemoteUserData.ref === lastSyncUserDataState.ref) {
611+
syncData = lastSyncStoredRemoteUserData.syncData;
612+
} else {
613+
this.logService.info(`${this.syncResourceLogLabel}: Last sync data stored locally is not same as the last sync state.`);
614+
}
603615
}
604616
break;
605617
} catch (error) {
@@ -620,10 +632,10 @@ export abstract class AbstractSynchroniser extends Disposable implements IUserDa
620632
try {
621633
const content = await this.userDataSyncStoreService.resolveResourceContent(this.resource, lastSyncUserDataState.ref, this.collection, this.syncHeaders);
622634
syncData = content === null ? null : this.parseSyncData(content);
623-
await this.fileService.writeFile(this.lastSyncResource, VSBuffer.fromString(syncData ? JSON.stringify(syncData) : ''));
635+
await this.writeLastSyncStoredRemoteUserData({ ref: lastSyncUserDataState.ref, syncData });
624636
} catch (error) {
625637
if (error instanceof UserDataSyncError && error.code === UserDataSyncErrorCode.NotFound) {
626-
this.logService.info(`${this.syncResourceLogLabel}: Last sync resource does not exist on the server.`);
638+
this.logService.info(`${this.syncResourceLogLabel}: .`);
627639
} else {
628640
throw error;
629641
}
@@ -654,21 +666,37 @@ export abstract class AbstractSynchroniser extends Disposable implements IUserDa
654666
};
655667

656668
this.storageService.store(this.lastSyncUserDataStateKey, JSON.stringify(lastSyncUserDataState), StorageScope.APPLICATION, StorageTarget.MACHINE);
657-
await this.fileService.writeFile(this.lastSyncResource, VSBuffer.fromString(lastSyncRemoteUserData.syncData ? JSON.stringify(lastSyncRemoteUserData.syncData) : ''));
669+
await this.writeLastSyncStoredRemoteUserData(lastSyncRemoteUserData);
670+
}
671+
672+
private async readLastSyncStoredRemoteUserData(): Promise<IRemoteUserData | undefined> {
673+
const content = (await this.fileService.readFile(this.lastSyncResource)).value.toString();
674+
try {
675+
const lastSyncStoredRemoteUserData = content ? JSON.parse(content) : undefined;
676+
if (isRemoteUserData(lastSyncStoredRemoteUserData)) {
677+
return lastSyncStoredRemoteUserData;
678+
}
679+
} catch (e) {
680+
this.logService.error(e);
681+
}
682+
return undefined;
683+
}
684+
685+
private async writeLastSyncStoredRemoteUserData(lastSyncRemoteUserData: IRemoteUserData): Promise<void> {
686+
await this.fileService.writeFile(this.lastSyncResource, VSBuffer.fromString(JSON.stringify(lastSyncRemoteUserData)));
658687
}
659688

660689
private async migrateLastSyncUserData(): Promise<string | undefined> {
661690
try {
662691
const content = await this.fileService.readFile(this.lastSyncResource);
663692
const userData = JSON.parse(content.value.toString());
664693
await this.fileService.del(this.lastSyncResource);
665-
if (userData.ref) {
694+
if (userData.ref && userData.content !== undefined) {
666695
this.storageService.store(this.lastSyncUserDataStateKey, JSON.stringify({
667696
...userData,
668697
content: undefined,
669698
}), StorageScope.APPLICATION, StorageTarget.MACHINE);
670-
await this.fileService.writeFile(this.lastSyncResource, VSBuffer.fromString(userData.content || ''));
671-
this.logService.info(`${this.syncResourceLogLabel}: Migrated data from last sync resource to last sync state.`);
699+
await this.writeLastSyncStoredRemoteUserData({ ref: userData.ref, syncData: userData.content === null ? null : JSON.parse(userData.content) });
672700
}
673701
} catch (error) {
674702
if (error instanceof FileOperationError && error.fileOperationResult === FileOperationResult.FILE_NOT_FOUND) {
@@ -878,6 +906,7 @@ export abstract class AbstractInitializer implements IUserDataInitializer {
878906
@IEnvironmentService protected readonly environmentService: IEnvironmentService,
879907
@ILogService protected readonly logService: ILogService,
880908
@IFileService protected readonly fileService: IFileService,
909+
@IStorageService protected readonly storageService: IStorageService,
881910
@IUriIdentityService uriIdentityService: IUriIdentityService,
882911
) {
883912
this.extUri = uriIdentityService.extUri;
@@ -916,8 +945,18 @@ export abstract class AbstractInitializer implements IUserDataInitializer {
916945
}
917946

918947
protected async updateLastSyncUserData(lastSyncRemoteUserData: IRemoteUserData, additionalProps: IStringDictionary<any> = {}): Promise<void> {
919-
const lastSyncUserData: IUserData = { ref: lastSyncRemoteUserData.ref, content: lastSyncRemoteUserData.syncData ? JSON.stringify(lastSyncRemoteUserData.syncData) : null, ...additionalProps };
920-
await this.fileService.writeFile(this.lastSyncResource, VSBuffer.fromString(JSON.stringify(lastSyncUserData)));
948+
if (additionalProps['ref'] || additionalProps['version']) {
949+
throw new Error('Cannot have core properties as additional');
950+
}
951+
952+
const lastSyncUserDataState: ILastSyncUserDataState = {
953+
ref: lastSyncRemoteUserData.ref,
954+
version: undefined,
955+
...additionalProps
956+
};
957+
958+
this.storageService.store(`${this.resource}.lastSyncUserData`, JSON.stringify(lastSyncUserDataState), StorageScope.APPLICATION, StorageTarget.MACHINE);
959+
await this.fileService.writeFile(this.lastSyncResource, VSBuffer.fromString(JSON.stringify(lastSyncRemoteUserData)));
921960
}
922961

923962
protected abstract doInitialize(remoteUserData: IRemoteUserData): Promise<void>;

‎src/vs/platform/userDataSync/common/extensionsSync.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -542,9 +542,10 @@ export abstract class AbstractExtensionsInitializer extends AbstractInitializer
542542
@IUserDataProfilesService userDataProfilesService: IUserDataProfilesService,
543543
@IEnvironmentService environmentService: IEnvironmentService,
544544
@ILogService logService: ILogService,
545+
@IStorageService storageService: IStorageService,
545546
@IUriIdentityService uriIdentityService: IUriIdentityService,
546547
) {
547-
super(SyncResource.Extensions, userDataProfilesService, environmentService, logService, fileService, uriIdentityService);
548+
super(SyncResource.Extensions, userDataProfilesService, environmentService, logService, fileService, storageService, uriIdentityService);
548549
}
549550

550551
protected async parseExtensions(remoteUserData: IRemoteUserData): Promise<ISyncExtension[] | null> {

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -398,14 +398,14 @@ export class LocalGlobalStateProvider {
398398
export class GlobalStateInitializer extends AbstractInitializer {
399399

400400
constructor(
401-
@IStorageService private readonly storageService: IStorageService,
401+
@IStorageService storageService: IStorageService,
402402
@IFileService fileService: IFileService,
403403
@IUserDataProfilesService userDataProfilesService: IUserDataProfilesService,
404404
@IEnvironmentService environmentService: IEnvironmentService,
405405
@IUserDataSyncLogService logService: IUserDataSyncLogService,
406406
@IUriIdentityService uriIdentityService: IUriIdentityService,
407407
) {
408-
super(SyncResource.GlobalState, userDataProfilesService, environmentService, logService, fileService, uriIdentityService);
408+
super(SyncResource.GlobalState, userDataProfilesService, environmentService, logService, fileService, storageService, uriIdentityService);
409409
}
410410

411411
async doInitialize(remoteUserData: IRemoteUserData): Promise<void> {

‎src/vs/platform/userDataSync/common/keybindingsSync.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -345,9 +345,10 @@ export class KeybindingsInitializer extends AbstractInitializer {
345345
@IUserDataProfilesService userDataProfilesService: IUserDataProfilesService,
346346
@IEnvironmentService environmentService: IEnvironmentService,
347347
@IUserDataSyncLogService logService: IUserDataSyncLogService,
348+
@IStorageService storageService: IStorageService,
348349
@IUriIdentityService uriIdentityService: IUriIdentityService,
349350
) {
350-
super(SyncResource.Keybindings, userDataProfilesService, environmentService, logService, fileService, uriIdentityService);
351+
super(SyncResource.Keybindings, userDataProfilesService, environmentService, logService, fileService, storageService, uriIdentityService);
351352
}
352353

353354
async doInitialize(remoteUserData: IRemoteUserData): Promise<void> {

‎src/vs/platform/userDataSync/common/settingsSync.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -348,9 +348,10 @@ export class SettingsInitializer extends AbstractInitializer {
348348
@IUserDataProfilesService userDataProfilesService: IUserDataProfilesService,
349349
@IEnvironmentService environmentService: IEnvironmentService,
350350
@IUserDataSyncLogService logService: IUserDataSyncLogService,
351+
@IStorageService storageService: IStorageService,
351352
@IUriIdentityService uriIdentityService: IUriIdentityService,
352353
) {
353-
super(SyncResource.Settings, userDataProfilesService, environmentService, logService, fileService, uriIdentityService);
354+
super(SyncResource.Settings, userDataProfilesService, environmentService, logService, fileService, storageService, uriIdentityService);
354355
}
355356

356357
async doInitialize(remoteUserData: IRemoteUserData): Promise<void> {

‎src/vs/platform/userDataSync/common/snippetsSync.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -509,9 +509,10 @@ export class SnippetsInitializer extends AbstractInitializer {
509509
@IUserDataProfilesService userDataProfilesService: IUserDataProfilesService,
510510
@IEnvironmentService environmentService: IEnvironmentService,
511511
@IUserDataSyncLogService logService: IUserDataSyncLogService,
512+
@IStorageService storageService: IStorageService,
512513
@IUriIdentityService uriIdentityService: IUriIdentityService,
513514
) {
514-
super(SyncResource.Snippets, userDataProfilesService, environmentService, logService, fileService, uriIdentityService);
515+
super(SyncResource.Snippets, userDataProfilesService, environmentService, logService, fileService, storageService, uriIdentityService);
515516
}
516517

517518
async doInitialize(remoteUserData: IRemoteUserData): Promise<void> {

‎src/vs/platform/userDataSync/common/tasksSync.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -254,9 +254,10 @@ export class TasksInitializer extends AbstractInitializer {
254254
@IUserDataProfilesService userDataProfilesService: IUserDataProfilesService,
255255
@IEnvironmentService environmentService: IEnvironmentService,
256256
@IUserDataSyncLogService logService: IUserDataSyncLogService,
257+
@IStorageService storageService: IStorageService,
257258
@IUriIdentityService uriIdentityService: IUriIdentityService,
258259
) {
259-
super(SyncResource.Tasks, userDataProfilesService, environmentService, logService, fileService, uriIdentityService);
260+
super(SyncResource.Tasks, userDataProfilesService, environmentService, logService, fileService, storageService, uriIdentityService);
260261
}
261262

262263
async doInitialize(remoteUserData: IRemoteUserData): Promise<void> {

‎src/vs/platform/userDataSync/test/common/synchronizer.test.ts

Lines changed: 50 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1110,7 +1110,7 @@ suite('TestSynchronizer - Last Sync Data', () => {
11101110
const actual = await testObject.getLastSyncUserData();
11111111

11121112
assert.deepStrictEqual(storageService.get('settings.lastSyncUserData', StorageScope.APPLICATION), JSON.stringify({ ref: '1' }));
1113-
assert.deepStrictEqual(JSON.parse((await fileService.readFile(testObject.getLastSyncResource())).value.toString()), { content: '0', machineId, version: 1 });
1113+
assert.deepStrictEqual(JSON.parse((await fileService.readFile(testObject.getLastSyncResource())).value.toString()), { ref: '1', syncData: { version: 1, machineId, content: '0' } });
11141114
assert.deepStrictEqual(actual, {
11151115
ref: '1',
11161116
syncData: {
@@ -1163,6 +1163,7 @@ suite('TestSynchronizer - Last Sync Data', () => {
11631163
foo: 'bar'
11641164
}
11651165
})));
1166+
server.reset();
11661167
const actual = await testObject.getLastSyncUserData();
11671168

11681169
assert.deepStrictEqual(storageService.get('settings.lastSyncUserData', StorageScope.APPLICATION), JSON.stringify({ ref: '1' }));
@@ -1174,6 +1175,38 @@ suite('TestSynchronizer - Last Sync Data', () => {
11741175
version: 1
11751176
},
11761177
});
1178+
assert.deepStrictEqual(server.requests, [{ headers: {}, type: 'GET', url: 'http://host:3000/v1/resource/settings/1' }]);
1179+
}));
1180+
1181+
test('last sync data is read from server after sync and stored sync data is tampered', () => runWithFakedTimers<void>({ useFakeTimers: true }, async () => {
1182+
const storageService = client.instantiationService.get(IStorageService);
1183+
const fileService = client.instantiationService.get(IFileService);
1184+
const testObject: TestSynchroniser = disposableStore.add(client.instantiationService.createInstance(TestSynchroniser, { syncResource: SyncResource.Settings, profile: client.instantiationService.get(IUserDataProfilesService).defaultProfile }, undefined));
1185+
testObject.syncBarrier.open();
1186+
1187+
await testObject.sync(await client.getResourceManifest());
1188+
const machineId = await testObject.getMachineId();
1189+
await fileService.writeFile(testObject.getLastSyncResource(), VSBuffer.fromString(JSON.stringify({
1190+
ref: '2',
1191+
syncData: {
1192+
content: '0',
1193+
machineId,
1194+
version: 1
1195+
}
1196+
})));
1197+
server.reset();
1198+
const actual = await testObject.getLastSyncUserData();
1199+
1200+
assert.deepStrictEqual(storageService.get('settings.lastSyncUserData', StorageScope.APPLICATION), JSON.stringify({ ref: '1' }));
1201+
assert.deepStrictEqual(actual, {
1202+
ref: '1',
1203+
syncData: {
1204+
content: '0',
1205+
machineId,
1206+
version: 1
1207+
}
1208+
});
1209+
assert.deepStrictEqual(server.requests, [{ headers: {}, type: 'GET', url: 'http://host:3000/v1/resource/settings/1' }]);
11771210
}));
11781211

11791212
test('reading last sync data: no requests are made to server when sync data is invalid', () => runWithFakedTimers<void>({ useFakeTimers: true }, async () => {
@@ -1202,6 +1235,22 @@ suite('TestSynchronizer - Last Sync Data', () => {
12021235
assert.deepStrictEqual(server.requests, []);
12031236
}));
12041237

1238+
test('reading last sync data: no requests are made to server when sync data is null', () => runWithFakedTimers<void>({ useFakeTimers: true }, async () => {
1239+
const fileService = client.instantiationService.get(IFileService);
1240+
const testObject: TestSynchroniser = disposableStore.add(client.instantiationService.createInstance(TestSynchroniser, { syncResource: SyncResource.Settings, profile: client.instantiationService.get(IUserDataProfilesService).defaultProfile }, undefined));
1241+
testObject.syncBarrier.open();
1242+
1243+
await testObject.sync(await client.getResourceManifest());
1244+
server.reset();
1245+
await fileService.writeFile(testObject.getLastSyncResource(), VSBuffer.fromString(JSON.stringify({
1246+
ref: '1',
1247+
syncData: null,
1248+
})));
1249+
await testObject.getLastSyncUserData();
1250+
1251+
assert.deepStrictEqual(server.requests, []);
1252+
}));
1253+
12051254
test('last sync data is null after sync if last sync state is deleted', () => runWithFakedTimers<void>({ useFakeTimers: true }, async () => {
12061255
const storageService = client.instantiationService.get(IStorageService);
12071256
const testObject: TestSynchroniser = disposableStore.add(client.instantiationService.createInstance(TestSynchroniser, { syncResource: SyncResource.Settings, profile: client.instantiationService.get(IUserDataProfilesService).defaultProfile }, undefined));

‎src/vs/workbench/contrib/extensions/electron-sandbox/remoteExtensionsInit.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -105,9 +105,10 @@ class RemoteExtensionsInitializer extends AbstractExtensionsInitializer {
105105
@ILogService logService: ILogService,
106106
@IUriIdentityService uriIdentityService: IUriIdentityService,
107107
@IExtensionGalleryService private readonly extensionGalleryService: IExtensionGalleryService,
108+
@IStorageService storageService: IStorageService,
108109
@IExtensionManifestPropertiesService private readonly extensionManifestPropertiesService: IExtensionManifestPropertiesService,
109110
) {
110-
super(extensionManagementService, ignoredExtensionsManagementService, fileService, userDataProfilesService, environmentService, logService, uriIdentityService);
111+
super(extensionManagementService, ignoredExtensionsManagementService, fileService, userDataProfilesService, environmentService, logService, storageService, uriIdentityService);
111112
}
112113

113114
protected override async doInitialize(remoteUserData: IRemoteUserData): Promise<void> {

‎src/vs/workbench/services/userData/browser/userDataInit.ts

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -272,10 +272,10 @@ export class UserDataInitializationService implements IUserDataInitializationSer
272272

273273
private createSyncResourceInitializer(syncResource: SyncResource): IUserDataInitializer {
274274
switch (syncResource) {
275-
case SyncResource.Settings: return new SettingsInitializer(this.fileService, this.userDataProfilesService, this.environmentService, this.logService, this.uriIdentityService);
276-
case SyncResource.Keybindings: return new KeybindingsInitializer(this.fileService, this.userDataProfilesService, this.environmentService, this.logService, this.uriIdentityService);
277-
case SyncResource.Tasks: return new TasksInitializer(this.fileService, this.userDataProfilesService, this.environmentService, this.logService, this.uriIdentityService);
278-
case SyncResource.Snippets: return new SnippetsInitializer(this.fileService, this.userDataProfilesService, this.environmentService, this.logService, this.uriIdentityService);
275+
case SyncResource.Settings: return new SettingsInitializer(this.fileService, this.userDataProfilesService, this.environmentService, this.logService, this.storageService, this.uriIdentityService);
276+
case SyncResource.Keybindings: return new KeybindingsInitializer(this.fileService, this.userDataProfilesService, this.environmentService, this.logService, this.storageService, this.uriIdentityService);
277+
case SyncResource.Tasks: return new TasksInitializer(this.fileService, this.userDataProfilesService, this.environmentService, this.logService, this.storageService, this.uriIdentityService);
278+
case SyncResource.Snippets: return new SnippetsInitializer(this.fileService, this.userDataProfilesService, this.environmentService, this.logService, this.storageService, this.uriIdentityService);
279279
case SyncResource.GlobalState: return new GlobalStateInitializer(this.storageService, this.fileService, this.userDataProfilesService, this.environmentService, this.logService, this.uriIdentityService);
280280
}
281281
throw new Error(`Cannot create initializer for ${syncResource}`);
@@ -296,9 +296,10 @@ class ExtensionsPreviewInitializer extends AbstractExtensionsInitializer {
296296
@IUserDataProfilesService userDataProfilesService: IUserDataProfilesService,
297297
@IEnvironmentService environmentService: IEnvironmentService,
298298
@IUserDataSyncLogService logService: IUserDataSyncLogService,
299+
@IStorageService storageService: IStorageService,
299300
@IUriIdentityService uriIdentityService: IUriIdentityService,
300301
) {
301-
super(extensionManagementService, ignoredExtensionsManagementService, fileService, userDataProfilesService, environmentService, logService, uriIdentityService);
302+
super(extensionManagementService, ignoredExtensionsManagementService, fileService, userDataProfilesService, environmentService, logService, storageService, uriIdentityService);
302303
}
303304

304305
getPreview(): Promise<IExtensionsInitializerPreviewResult | null> {

0 commit comments

Comments
 (0)