From 957534cd8626395a76a82a4484e0a2cf7b056172 Mon Sep 17 00:00:00 2001 From: Sandeep Somavarapu Date: Sun, 17 May 2020 16:47:04 +0200 Subject: [PATCH] #91556 Check if there are changes to push to remote --- .../common/abstractSynchronizer.ts | 19 ++++++++++++--- .../userDataSync/common/extensionsSync.ts | 2 +- .../userDataSync/common/globalStateSync.ts | 2 +- .../userDataSync/common/snippetsSync.ts | 2 +- .../test/common/settingsSync.test.ts | 19 +++++++++++++++ .../test/common/synchronizer.test.ts | 23 +++++++++++++++++++ 6 files changed, 61 insertions(+), 6 deletions(-) diff --git a/src/vs/platform/userDataSync/common/abstractSynchronizer.ts b/src/vs/platform/userDataSync/common/abstractSynchronizer.ts index 47e10a2c34e..ab8b96308f1 100644 --- a/src/vs/platform/userDataSync/common/abstractSynchronizer.ts +++ b/src/vs/platform/userDataSync/common/abstractSynchronizer.ts @@ -10,7 +10,7 @@ import { URI } from 'vs/base/common/uri'; import { SyncResource, SyncStatus, IUserData, IUserDataSyncStoreService, UserDataSyncErrorCode, UserDataSyncError, IUserDataSyncLogService, IUserDataSyncUtilService, IUserDataSyncEnablementService, IUserDataSyncBackupStoreService, Conflict, ISyncResourceHandle, USER_DATA_SYNC_SCHEME, ISyncPreviewResult, IUserDataManifest } from 'vs/platform/userDataSync/common/userDataSync'; import { IEnvironmentService } from 'vs/platform/environment/common/environment'; import { joinPath, dirname, isEqual, basename } from 'vs/base/common/resources'; -import { CancelablePromise } from 'vs/base/common/async'; +import { CancelablePromise, RunOnceScheduler } from 'vs/base/common/async'; import { Emitter, Event } from 'vs/base/common/event'; import { ITelemetryService } from 'vs/platform/telemetry/common/telemetry'; import { ParseError, parse } from 'vs/base/common/json'; @@ -57,7 +57,8 @@ export abstract class AbstractSynchroniser extends Disposable { private _onDidChangeConflicts: Emitter = this._register(new Emitter()); readonly onDidChangeConflicts: Event = this._onDidChangeConflicts.event; - protected readonly _onDidChangeLocal: Emitter = this._register(new Emitter()); + private readonly localChangeTriggerScheduler = new RunOnceScheduler(() => this.doTriggerLocalChange(), 50); + private readonly _onDidChangeLocal: Emitter = this._register(new Emitter()); readonly onDidChangeLocal: Event = this._onDidChangeLocal.event; protected readonly lastSyncResource: URI; @@ -80,6 +81,18 @@ export abstract class AbstractSynchroniser extends Disposable { this.lastSyncResource = joinPath(this.syncFolder, `lastSync${this.resource}.json`); } + protected async triggerLocalChange(): Promise { + this.localChangeTriggerScheduler.schedule(); + } + + protected async doTriggerLocalChange(): Promise { + const lastSyncUserData = await this.getLastSyncUserData(); + const hasRemoteChanged = lastSyncUserData ? (await this.generatePreview(lastSyncUserData, lastSyncUserData)).hasRemoteChanged : true; + if (hasRemoteChanged) { + this._onDidChangeLocal.fire(); + } + } + protected setStatus(status: SyncStatus): void { if (this._status !== status) { const oldStatus = this._status; @@ -453,7 +466,7 @@ export abstract class AbstractFileSynchroniser extends AbstractSynchroniser { // Otherwise fire change event else { - this._onDidChangeLocal.fire(); + this.triggerLocalChange(); } } diff --git a/src/vs/platform/userDataSync/common/extensionsSync.ts b/src/vs/platform/userDataSync/common/extensionsSync.ts index 0aa08c4aa3a..364230de5c1 100644 --- a/src/vs/platform/userDataSync/common/extensionsSync.ts +++ b/src/vs/platform/userDataSync/common/extensionsSync.ts @@ -63,7 +63,7 @@ export class ExtensionsSynchroniser extends AbstractSynchroniser implements IUse Event.filter(this.extensionManagementService.onDidInstallExtension, (e => !!e.gallery)), Event.filter(this.extensionManagementService.onDidUninstallExtension, (e => !e.error)), this.extensionEnablementService.onDidChangeEnablement), - () => undefined, 500)(() => this._onDidChangeLocal.fire())); + () => undefined, 500)(() => this.triggerLocalChange())); } async pull(): Promise { diff --git a/src/vs/platform/userDataSync/common/globalStateSync.ts b/src/vs/platform/userDataSync/common/globalStateSync.ts index ad2acedf184..bc6355b88d1 100644 --- a/src/vs/platform/userDataSync/common/globalStateSync.ts +++ b/src/vs/platform/userDataSync/common/globalStateSync.ts @@ -66,7 +66,7 @@ export class GlobalStateSynchroniser extends AbstractSynchroniser implements IUs Event.filter(this.storageService.onDidChangeStorage, e => storageKeysSyncRegistryService.storageKeys.some(({ key }) => e.key === key)), /* Storage key registered */ this.storageKeysSyncRegistryService.onDidChangeStorageKeys - )((() => this._onDidChangeLocal.fire())) + )((() => this.triggerLocalChange())) ); } diff --git a/src/vs/platform/userDataSync/common/snippetsSync.ts b/src/vs/platform/userDataSync/common/snippetsSync.ts index 8871c78c7cd..5bbfe1cd019 100644 --- a/src/vs/platform/userDataSync/common/snippetsSync.ts +++ b/src/vs/platform/userDataSync/common/snippetsSync.ts @@ -70,7 +70,7 @@ export class SnippetsSynchroniser extends AbstractSynchroniser implements IUserD } // Otherwise fire change event else { - this._onDidChangeLocal.fire(); + this.triggerLocalChange(); } } diff --git a/src/vs/platform/userDataSync/test/common/settingsSync.test.ts b/src/vs/platform/userDataSync/test/common/settingsSync.test.ts index a9e62326673..4ccf9800534 100644 --- a/src/vs/platform/userDataSync/test/common/settingsSync.test.ts +++ b/src/vs/platform/userDataSync/test/common/settingsSync.test.ts @@ -15,6 +15,7 @@ import { VSBuffer } from 'vs/base/common/buffer'; import { ISyncData } from 'vs/platform/userDataSync/common/abstractSynchronizer'; import { Registry } from 'vs/platform/registry/common/platform'; import { IConfigurationRegistry, Extensions, ConfigurationScope } from 'vs/platform/configuration/common/configurationRegistry'; +import { Event } from 'vs/base/common/event'; suite('SettingsSync', () => { @@ -267,6 +268,24 @@ suite('SettingsSync', () => { }`); }); + test('local change event is triggered when settings are changed', async () => { + const content = + `{ + "files.autoSave": "afterDelay", + "files.simpleDialog.enable": true, +}`; + + await updateSettings(content); + await testObject.sync(await client.manifest()); + + const promise = Event.toPromise(testObject.onDidChangeLocal); + await updateSettings(`{ + "files.autoSave": "off", + "files.simpleDialog.enable": true, +}`); + await promise; + }); + test('do not sync ignored settings', async () => { const settingsContent = `{ diff --git a/src/vs/platform/userDataSync/test/common/synchronizer.test.ts b/src/vs/platform/userDataSync/test/common/synchronizer.test.ts index 08241ecb441..0a2664a4e60 100644 --- a/src/vs/platform/userDataSync/test/common/synchronizer.test.ts +++ b/src/vs/platform/userDataSync/test/common/synchronizer.test.ts @@ -51,6 +51,16 @@ class TestSynchroniser extends AbstractSynchroniser { this.syncBarrier.open(); } + async triggerLocalChange(): Promise { + super.triggerLocalChange(); + } + + onDidTriggerLocalChangeCall: Emitter = this._register(new Emitter()); + protected async doTriggerLocalChange(): Promise { + await super.doTriggerLocalChange(); + this.onDidTriggerLocalChangeCall.fire(); + } + protected async generatePreview(remoteUserData: IRemoteUserData, lastSyncUserData: IRemoteUserData | null): Promise { return { hasLocalChanged: false, hasRemoteChanged: false }; } @@ -203,5 +213,18 @@ suite('TestSynchronizer', () => { ]); }); + test('no requests are made to server when local change is triggered', async () => { + const testObject: TestSynchroniser = client.instantiationService.createInstance(TestSynchroniser, SyncResource.Settings); + testObject.syncBarrier.open(); + await testObject.sync(await client.manifest()); + + server.reset(); + const promise = Event.toPromise(testObject.onDidTriggerLocalChangeCall.event); + await testObject.triggerLocalChange(); + + await promise; + assert.deepEqual(server.requests, []); + }); + }); -- GitLab