From 7df4ebd0b3fec4f45f3a99ada0323a968a12f881 Mon Sep 17 00:00:00 2001 From: Sandeep Somavarapu Date: Tue, 16 Jun 2020 18:57:25 +0200 Subject: [PATCH] Refactor auto sync service --- .../common/userDataAutoSyncService.ts | 32 +++++++++---------- .../userDataSync/common/userDataSync.ts | 6 ++-- .../userDataSync/common/userDataSyncIpc.ts | 6 ++-- .../userDataAutoSyncService.ts | 3 +- .../common/userDataAutoSyncService.test.ts | 8 ++--- .../browser/userDataAutoSyncService.ts | 3 +- .../userDataSync/browser/userDataSync.ts | 5 ++- .../userDataAutoSyncService.ts | 14 ++++---- .../browser/userDataSyncWorkbenchService.ts | 18 +++++------ 9 files changed, 45 insertions(+), 50 deletions(-) diff --git a/src/vs/platform/userDataSync/common/userDataAutoSyncService.ts b/src/vs/platform/userDataSync/common/userDataAutoSyncService.ts index 5f0a3f2bb70..23a10b31531 100644 --- a/src/vs/platform/userDataSync/common/userDataAutoSyncService.ts +++ b/src/vs/platform/userDataSync/common/userDataAutoSyncService.ts @@ -6,7 +6,7 @@ import { Delayer, disposableTimeout } from 'vs/base/common/async'; import { Event, Emitter } from 'vs/base/common/event'; import { Disposable, toDisposable, MutableDisposable, IDisposable } from 'vs/base/common/lifecycle'; -import { IUserDataSyncLogService, IUserDataSyncService, IUserDataAutoSyncService, UserDataSyncError, UserDataSyncErrorCode, IUserDataSyncEnablementService, ALL_SYNC_RESOURCES, IUserDataSyncStoreService } from 'vs/platform/userDataSync/common/userDataSync'; +import { IUserDataSyncLogService, IUserDataSyncService, IUserDataAutoSyncService, UserDataSyncError, UserDataSyncErrorCode, IUserDataSyncEnablementService, IUserDataSyncStoreService } from 'vs/platform/userDataSync/common/userDataSync'; import { IUserDataSyncAccountService } from 'vs/platform/userDataSync/common/userDataSyncAccount'; import { ITelemetryService } from 'vs/platform/telemetry/common/telemetry'; @@ -14,8 +14,6 @@ type AutoSyncClassification = { sources: { classification: 'SystemMetaData', purpose: 'FeatureInsight', isMeasurement: true }; }; -export const RESOURCE_ENABLEMENT_SOURCE = 'resourceEnablement'; - export class UserDataAutoSyncService extends Disposable implements IUserDataAutoSyncService { _serviceBrand: any; @@ -42,7 +40,8 @@ export class UserDataAutoSyncService extends Disposable implements IUserDataAuto if (userDataSyncStoreService.userDataSyncStore) { this.updateAutoSync(); this._register(Event.any(authTokenService.onDidChangeAccount, this.userDataSyncEnablementService.onDidChangeEnablement)(() => this.updateAutoSync())); - this._register(Event.filter(this.userDataSyncEnablementService.onDidChangeResourceEnablement, ([, enabled]) => enabled)(() => this.triggerAutoSync([RESOURCE_ENABLEMENT_SOURCE]))); + this._register(Event.debounce(userDataSyncService.onDidChangeLocal, (last, source) => last ? [...last, source] : [source], 1000)(sources => this.triggerSync(sources, false))); + this._register(Event.filter(this.userDataSyncEnablementService.onDidChangeResourceEnablement, ([, enabled]) => enabled)(() => this.triggerSync(['resourceEnablement'], false))); } } @@ -66,12 +65,18 @@ export class UserDataAutoSyncService extends Disposable implements IUserDataAuto } } - enable(): void { + async turnOn(pullFirst: boolean): Promise { + if (pullFirst) { + await this.userDataSyncService.pull(); + } else { + await this.userDataSyncService.sync(); + } + this.userDataSyncEnablementService.setEnablement(true); this.updateAutoSync(); } - disable(): void { + async turnOff(): Promise { this.userDataSyncEnablementService.setEnablement(false); this.updateAutoSync(); } @@ -101,10 +106,10 @@ export class UserDataAutoSyncService extends Disposable implements IUserDataAuto if (userDataSyncError.code === UserDataSyncErrorCode.TurnedOff || userDataSyncError.code === UserDataSyncErrorCode.SessionExpired) { this.logService.info('Auto Sync: Sync is turned off in the cloud.'); await this.userDataSyncService.resetLocal(); - this.disable(); + this.turnOff(); this.logService.info('Auto Sync: Turned off sync because sync is turned off in the cloud'); } else if (userDataSyncError.code === UserDataSyncErrorCode.LocalTooManyRequests) { - this.disable(); + this.turnOff(); this.logService.info('Auto Sync: Turned off sync because of making too many requests to server'); } else { this.logService.error(userDataSyncError); @@ -114,19 +119,14 @@ export class UserDataAutoSyncService extends Disposable implements IUserDataAuto } private sources: string[] = []; - async triggerAutoSync(sources: string[]): Promise { + async triggerSync(sources: string[], skipIfSyncedRecently: boolean): Promise { if (this.autoSync.value === undefined) { return this.syncTriggerDelayer.cancel(); } - /* - If sync is not triggered by sync resource (triggered by other sources like window focus etc.,) or by resource enablement - then limit sync to once per 10s - */ - const hasToLimitSync = sources.indexOf(RESOURCE_ENABLEMENT_SOURCE) === -1 && ALL_SYNC_RESOURCES.every(syncResource => sources.indexOf(syncResource) === -1); - if (hasToLimitSync && this.lastSyncTriggerTime + if (skipIfSyncedRecently && this.lastSyncTriggerTime && Math.round((new Date().getTime() - this.lastSyncTriggerTime) / 1000) < 10) { - this.logService.debug('Auto Sync Skipped: Limited to once per 10 seconds.'); + this.logService.debug('Auto Sync: Skipped. Limited to once per 10 seconds.'); return; } diff --git a/src/vs/platform/userDataSync/common/userDataSync.ts b/src/vs/platform/userDataSync/common/userDataSync.ts index 49c89671624..30109f77b14 100644 --- a/src/vs/platform/userDataSync/common/userDataSync.ts +++ b/src/vs/platform/userDataSync/common/userDataSync.ts @@ -382,9 +382,9 @@ export const IUserDataAutoSyncService = createDecorator; - enable(): void; - disable(): void; - triggerAutoSync(sources: string[]): Promise; + turnOn(pullFirst: boolean): Promise; + turnOff(): Promise; + triggerSync(sources: string[], hasToLimitSync: boolean): Promise; } export const IUserDataSyncUtilService = createDecorator('IUserDataSyncUtilService'); diff --git a/src/vs/platform/userDataSync/common/userDataSyncIpc.ts b/src/vs/platform/userDataSync/common/userDataSyncIpc.ts index 63b29ad6830..7ec8134934d 100644 --- a/src/vs/platform/userDataSync/common/userDataSyncIpc.ts +++ b/src/vs/platform/userDataSync/common/userDataSyncIpc.ts @@ -74,9 +74,9 @@ export class UserDataAutoSyncChannel implements IServerChannel { call(context: any, command: string, args?: any): Promise { switch (command) { - case 'triggerAutoSync': return this.service.triggerAutoSync(args[0]); - case 'enable': return Promise.resolve(this.service.enable()); - case 'disable': return Promise.resolve(this.service.disable()); + case 'triggerSync': return this.service.triggerSync(args[0], args[1]); + case 'turnOn': return this.service.turnOn(args[0]); + case 'turnOff': return this.service.turnOff(); } throw new Error('Invalid call'); } diff --git a/src/vs/platform/userDataSync/electron-browser/userDataAutoSyncService.ts b/src/vs/platform/userDataSync/electron-browser/userDataAutoSyncService.ts index f9cf9b1ffd4..9e41ea3f636 100644 --- a/src/vs/platform/userDataSync/electron-browser/userDataAutoSyncService.ts +++ b/src/vs/platform/userDataSync/electron-browser/userDataAutoSyncService.ts @@ -26,8 +26,7 @@ export class UserDataAutoSyncService extends BaseUserDataAutoSyncService { this._register(Event.debounce(Event.any( Event.map(electronService.onWindowFocus, () => 'windowFocus'), Event.map(electronService.onWindowOpen, () => 'windowOpen'), - userDataSyncService.onDidChangeLocal, - ), (last, source) => last ? [...last, source] : [source], 1000)(sources => this.triggerAutoSync(sources))); + ), (last, source) => last ? [...last, source] : [source], 1000)(sources => this.triggerSync(sources, true))); } } diff --git a/src/vs/platform/userDataSync/test/common/userDataAutoSyncService.test.ts b/src/vs/platform/userDataSync/test/common/userDataAutoSyncService.test.ts index f2c772fb7ae..aee0d8fce79 100644 --- a/src/vs/platform/userDataSync/test/common/userDataAutoSyncService.test.ts +++ b/src/vs/platform/userDataSync/test/common/userDataAutoSyncService.test.ts @@ -34,7 +34,7 @@ suite('UserDataAutoSyncService', () => { const testObject: UserDataAutoSyncService = client.instantiationService.createInstance(TestUserDataAutoSyncService); // Trigger auto sync with settings change - await testObject.triggerAutoSync([SyncResource.Settings]); + await testObject.triggerSync([SyncResource.Settings], false); // Make sure only one request is made assert.deepEqual(target.requests, [{ type: 'GET', url: `${target.url}/v1/manifest`, headers: {} }]); @@ -55,7 +55,7 @@ suite('UserDataAutoSyncService', () => { // Trigger auto sync with settings change multiple times for (let counter = 0; counter < 2; counter++) { - await testObject.triggerAutoSync([SyncResource.Settings]); + await testObject.triggerSync([SyncResource.Settings], false); } assert.deepEqual(target.requests, [ @@ -78,7 +78,7 @@ suite('UserDataAutoSyncService', () => { const testObject: UserDataAutoSyncService = client.instantiationService.createInstance(TestUserDataAutoSyncService); // Trigger auto sync with window focus once - await testObject.triggerAutoSync(['windowFocus']); + await testObject.triggerSync(['windowFocus'], true); // Make sure only one request is made assert.deepEqual(target.requests, [{ type: 'GET', url: `${target.url}/v1/manifest`, headers: {} }]); @@ -99,7 +99,7 @@ suite('UserDataAutoSyncService', () => { // Trigger auto sync with window focus multiple times for (let counter = 0; counter < 2; counter++) { - await testObject.triggerAutoSync(['windowFocus']); + await testObject.triggerSync(['windowFocus'], true); } // Make sure only one request is made diff --git a/src/vs/workbench/contrib/userDataSync/browser/userDataAutoSyncService.ts b/src/vs/workbench/contrib/userDataSync/browser/userDataAutoSyncService.ts index 7529104c1ca..2d35383d1f0 100644 --- a/src/vs/workbench/contrib/userDataSync/browser/userDataAutoSyncService.ts +++ b/src/vs/workbench/contrib/userDataSync/browser/userDataAutoSyncService.ts @@ -29,8 +29,7 @@ export class UserDataAutoSyncService extends BaseUserDataAutoSyncService { this._register(Event.debounce(Event.any( Event.map(hostService.onDidChangeFocus, () => 'windowFocus'), instantiationService.createInstance(UserDataSyncTrigger).onDidTriggerSync, - userDataSyncService.onDidChangeLocal, - ), (last, source) => last ? [...last, source] : [source], 1000)(sources => this.triggerAutoSync(sources))); + ), (last, source) => last ? [...last, source] : [source], 1000)(sources => this.triggerSync(sources, true))); } } diff --git a/src/vs/workbench/contrib/userDataSync/browser/userDataSync.ts b/src/vs/workbench/contrib/userDataSync/browser/userDataSync.ts index 6dda6b23ef3..c608842be71 100644 --- a/src/vs/workbench/contrib/userDataSync/browser/userDataSync.ts +++ b/src/vs/workbench/contrib/userDataSync/browser/userDataSync.ts @@ -110,7 +110,7 @@ export class UserDataSyncWorkbenchContribution extends Disposable implements IWo @IInstantiationService private readonly instantiationService: IInstantiationService, @IOutputService private readonly outputService: IOutputService, @IUserDataSyncAccountService readonly authTokenService: IUserDataSyncAccountService, - @IUserDataAutoSyncService userDataAutoSyncService: IUserDataAutoSyncService, + @IUserDataAutoSyncService private readonly userDataAutoSyncService: IUserDataAutoSyncService, @ITextModelService private readonly textModelResolverService: ITextModelService, @IPreferencesService private readonly preferencesService: IPreferencesService, @ITelemetryService private readonly telemetryService: ITelemetryService, @@ -945,8 +945,7 @@ export class UserDataSyncWorkbenchContribution extends Disposable implements IWo }); } run(accessor: ServicesAccessor): Promise { - accessor.get(ITelemetryService).publicLog2(`sync/actions/${syncNowCommand.id}`); - return that.userDataSyncService.sync(); + return that.userDataAutoSyncService.triggerSync([syncNowCommand.id], false); } })); } diff --git a/src/vs/workbench/contrib/userDataSync/electron-browser/userDataAutoSyncService.ts b/src/vs/workbench/contrib/userDataSync/electron-browser/userDataAutoSyncService.ts index bb6b3b63107..45c6df0784a 100644 --- a/src/vs/workbench/contrib/userDataSync/electron-browser/userDataAutoSyncService.ts +++ b/src/vs/workbench/contrib/userDataSync/electron-browser/userDataAutoSyncService.ts @@ -24,19 +24,19 @@ export class UserDataAutoSyncService extends Disposable implements IUserDataAuto ) { super(); this.channel = sharedProcessService.getChannel('userDataAutoSync'); - this._register(instantiationService.createInstance(UserDataSyncTrigger).onDidTriggerSync(source => this.triggerAutoSync([source]))); + this._register(instantiationService.createInstance(UserDataSyncTrigger).onDidTriggerSync(source => this.triggerSync([source], true))); } - triggerAutoSync(sources: string[]): Promise { - return this.channel.call('triggerAutoSync', [sources]); + triggerSync(sources: string[], hasToLimitSync: boolean): Promise { + return this.channel.call('triggerSync', [sources, hasToLimitSync]); } - enable(): void { - this.channel.call('enable'); + turnOn(pullFirst: boolean): Promise { + return this.channel.call('turnOn', [pullFirst]); } - disable(): void { - this.channel.call('disable'); + turnOff(): Promise { + return this.channel.call('turnOff'); } } diff --git a/src/vs/workbench/services/userDataSync/browser/userDataSyncWorkbenchService.ts b/src/vs/workbench/services/userDataSync/browser/userDataSyncWorkbenchService.ts index cb159bc240b..3a7d58a0af6 100644 --- a/src/vs/workbench/services/userDataSync/browser/userDataSyncWorkbenchService.ts +++ b/src/vs/workbench/services/userDataSync/browser/userDataSyncWorkbenchService.ts @@ -220,13 +220,13 @@ export class UserDataSyncWorkbenchService extends Disposable implements IUserDat throw new Error(localize('no account', "No account available")); } - await this.handleFirstTimeSync(); - this.userDataAutoSyncService.enable(); + const pullFirst = await this.handleFirstTimeSync(); + await this.userDataAutoSyncService.turnOn(pullFirst); this.notificationService.info(localize('sync turned on', "Preferences sync is turned on")); } async turnoff(everywhere: boolean): Promise { - this.userDataAutoSyncService.disable(); + await this.userDataAutoSyncService.turnOff(); if (everywhere) { this.telemetryService.publicLog2('sync/turnOffEveryWhere'); @@ -236,11 +236,10 @@ export class UserDataSyncWorkbenchService extends Disposable implements IUserDat } } - private async handleFirstTimeSync(): Promise { + private async handleFirstTimeSync(): Promise { const isFirstTimeSyncingWithAnotherMachine = await this.userDataSyncService.isFirstTimeSyncingWithAnotherMachine(); if (!isFirstTimeSyncingWithAnotherMachine) { - await this.userDataSyncService.sync(); - return; + return false; } const result = await this.dialogService.show( @@ -264,16 +263,15 @@ export class UserDataSyncWorkbenchService extends Disposable implements IUserDat throw canceled(); case 1: this.telemetryService.publicLog2<{ action: string }, FirstTimeSyncClassification>('sync/firstTimeSync', { action: 'merge' }); - await this.userDataSyncService.sync(); - break; + return false; case 2: this.telemetryService.publicLog2<{ action: string }, FirstTimeSyncClassification>('sync/firstTimeSync', { action: 'replace-local' }); - await this.userDataSyncService.pull(); - break; + return true; case 3: this.telemetryService.publicLog2<{ action: string }, FirstTimeSyncClassification>('sync/firstTimeSync', { action: 'cancelled' }); throw canceled(); } + return false; } private isSupportedAuthenticationProviderId(authenticationProviderId: string): boolean {