From 8a17a70dff421d17a073236f2335edb4bdbb72ca Mon Sep 17 00:00:00 2001 From: Sandeep Somavarapu Date: Tue, 16 Jun 2020 10:18:27 +0200 Subject: [PATCH] Fix #100220 --- .../common/abstractSynchronizer.ts | 72 ++++++++++--------- .../userDataSync/common/userDataSync.ts | 15 ++-- .../common/userDataSyncService.ts | 9 ++- .../common/userDataSyncStoreService.ts | 14 ++-- .../test/common/userDataSyncService.test.ts | 20 ++++++ 5 files changed, 84 insertions(+), 46 deletions(-) diff --git a/src/vs/platform/userDataSync/common/abstractSynchronizer.ts b/src/vs/platform/userDataSync/common/abstractSynchronizer.ts index 559b89fed26..0974db02070 100644 --- a/src/vs/platform/userDataSync/common/abstractSynchronizer.ts +++ b/src/vs/platform/userDataSync/common/abstractSynchronizer.ts @@ -27,6 +27,7 @@ import { equals } from 'vs/base/common/arrays'; import { getServiceMachineId } from 'vs/platform/serviceMachineId/common/serviceMachineId'; import { IStorageService } from 'vs/platform/storage/common/storage'; import { CancellationToken } from 'vs/base/common/cancellation'; +import { IHeaders } from 'vs/base/parts/request/common/request'; type SyncSourceClassification = { source?: { classification: 'SystemMetaData', purpose: 'FeatureInsight', isMeasurement: true }; @@ -75,6 +76,8 @@ export abstract class AbstractSynchroniser extends Disposable { protected readonly lastSyncResource: URI; protected readonly syncResourceLogLabel: string; + private syncHeaders: IHeaders = {}; + constructor( readonly resource: SyncResource, @IFileService protected readonly fileService: IFileService, @@ -150,39 +153,44 @@ export abstract class AbstractSynchroniser extends Disposable { } } - async sync(manifest: IUserDataManifest | null): Promise { - if (!this.isEnabled()) { - if (this.status !== SyncStatus.Idle) { - await this.stop(); - } - this.logService.info(`${this.syncResourceLogLabel}: Skipped synchronizing ${this.resource.toLowerCase()} as it is disabled.`); - return; - } - if (this.status === SyncStatus.HasConflicts) { - this.logService.info(`${this.syncResourceLogLabel}: Skipped synchronizing ${this.resource.toLowerCase()} as there are conflicts.`); - return; - } - if (this.status === SyncStatus.Syncing) { - this.logService.info(`${this.syncResourceLogLabel}: Skipped synchronizing ${this.resource.toLowerCase()} as it is running already.`); - return; - } - - this.logService.trace(`${this.syncResourceLogLabel}: Started synchronizing ${this.resource.toLowerCase()}...`); - this.setStatus(SyncStatus.Syncing); - - const lastSyncUserData = await this.getLastSyncUserData(); - const remoteUserData = await this.getLatestRemoteUserData(manifest, lastSyncUserData); - - let status: SyncStatus = SyncStatus.Idle; + async sync(manifest: IUserDataManifest | null, headers: IHeaders = {}): Promise { try { - status = await this.performSync(remoteUserData, lastSyncUserData); - if (status === SyncStatus.HasConflicts) { - this.logService.info(`${this.syncResourceLogLabel}: Detected conflicts while synchronizing ${this.resource.toLowerCase()}.`); - } else if (status === SyncStatus.Idle) { - this.logService.trace(`${this.syncResourceLogLabel}: Finished synchronizing ${this.resource.toLowerCase()}.`); + this.syncHeaders = { ...headers }; + if (!this.isEnabled()) { + if (this.status !== SyncStatus.Idle) { + await this.stop(); + } + this.logService.info(`${this.syncResourceLogLabel}: Skipped synchronizing ${this.resource.toLowerCase()} as it is disabled.`); + return; + } + if (this.status === SyncStatus.HasConflicts) { + this.logService.info(`${this.syncResourceLogLabel}: Skipped synchronizing ${this.resource.toLowerCase()} as there are conflicts.`); + return; + } + if (this.status === SyncStatus.Syncing) { + this.logService.info(`${this.syncResourceLogLabel}: Skipped synchronizing ${this.resource.toLowerCase()} as it is running already.`); + return; + } + + this.logService.trace(`${this.syncResourceLogLabel}: Started synchronizing ${this.resource.toLowerCase()}...`); + this.setStatus(SyncStatus.Syncing); + + const lastSyncUserData = await this.getLastSyncUserData(); + const remoteUserData = await this.getLatestRemoteUserData(manifest, lastSyncUserData); + + let status: SyncStatus = SyncStatus.Idle; + try { + status = await this.performSync(remoteUserData, lastSyncUserData); + if (status === SyncStatus.HasConflicts) { + this.logService.info(`${this.syncResourceLogLabel}: Detected conflicts while synchronizing ${this.resource.toLowerCase()}.`); + } else if (status === SyncStatus.Idle) { + this.logService.trace(`${this.syncResourceLogLabel}: Finished synchronizing ${this.resource.toLowerCase()}.`); + } + } finally { + this.setStatus(status); } } finally { - this.setStatus(status); + this.syncHeaders = {}; } } @@ -446,14 +454,14 @@ export abstract class AbstractSynchroniser extends Disposable { return { ref: refOrLastSyncData, content }; } else { const lastSyncUserData: IUserData | null = refOrLastSyncData ? { ref: refOrLastSyncData.ref, content: refOrLastSyncData.syncData ? JSON.stringify(refOrLastSyncData.syncData) : null } : null; - return this.userDataSyncStoreService.read(this.resource, lastSyncUserData); + return this.userDataSyncStoreService.read(this.resource, lastSyncUserData, this.syncHeaders); } } protected async updateRemoteUserData(content: string, ref: string | null): Promise { const machineId = await this.currentMachineIdPromise; const syncData: ISyncData = { version: this.version, machineId, content }; - ref = await this.userDataSyncStoreService.write(this.resource, JSON.stringify(syncData), ref); + ref = await this.userDataSyncStoreService.write(this.resource, JSON.stringify(syncData), ref, this.syncHeaders); return { ref, syncData }; } diff --git a/src/vs/platform/userDataSync/common/userDataSync.ts b/src/vs/platform/userDataSync/common/userDataSync.ts index b630278afdb..49c89671624 100644 --- a/src/vs/platform/userDataSync/common/userDataSync.ts +++ b/src/vs/platform/userDataSync/common/userDataSync.ts @@ -22,6 +22,7 @@ import { IEnvironmentService } from 'vs/platform/environment/common/environment' import { IProductService, ConfigurationSyncStore } from 'vs/platform/product/common/productService'; import { distinct } from 'vs/base/common/arrays'; import { isArray, isString, isObject } from 'vs/base/common/types'; +import { IHeaders } from 'vs/base/parts/request/common/request'; export const CONFIGURATION_SYNC_STORE_KEY = 'configurationSync.store'; @@ -162,16 +163,20 @@ export type ServerResource = SyncResource | 'machines'; export interface IUserDataSyncStoreService { readonly _serviceBrand: undefined; readonly userDataSyncStore: IUserDataSyncStore | undefined; + readonly onTokenFailed: Event; readonly onTokenSucceed: Event; setAuthToken(token: string, type: string): void; - read(resource: ServerResource, oldValue: IUserData | null): Promise; - write(resource: ServerResource, content: string, ref: string | null): Promise; - manifest(): Promise; + + // Sync requests + manifest(headers?: IHeaders): Promise; + read(resource: ServerResource, oldValue: IUserData | null, headers?: IHeaders): Promise; + write(resource: ServerResource, content: string, ref: string | null, headers?: IHeaders): Promise; clear(): Promise; + delete(resource: ServerResource): Promise; + getAllRefs(resource: ServerResource): Promise; resolveContent(resource: ServerResource, ref: string): Promise; - delete(resource: ServerResource): Promise; } export const IUserDataSyncBackupStoreService = createDecorator('IUserDataSyncBackupStoreService'); @@ -301,7 +306,7 @@ export interface IUserDataSynchroniser { pull(): Promise; push(): Promise; - sync(manifest: IUserDataManifest | null): Promise; + sync(manifest: IUserDataManifest | null, headers?: IHeaders): Promise; replace(uri: URI): Promise; stop(): Promise; diff --git a/src/vs/platform/userDataSync/common/userDataSyncService.ts b/src/vs/platform/userDataSync/common/userDataSyncService.ts index a273bc40004..35a8be4409d 100644 --- a/src/vs/platform/userDataSync/common/userDataSyncService.ts +++ b/src/vs/platform/userDataSync/common/userDataSyncService.ts @@ -25,6 +25,8 @@ import { IProductService } from 'vs/platform/product/common/productService'; import { platform, PlatformToString, isWeb, Platform } from 'vs/base/common/platform'; import { escapeRegExpCharacters } from 'vs/base/common/strings'; import { CancellationToken } from 'vs/base/common/cancellation'; +import { generateUuid } from 'vs/base/common/uuid'; +import { IHeaders } from 'vs/base/parts/request/common/request'; type SyncClassification = { resource?: { classification: 'SystemMetaData', purpose: 'FeatureInsight', isMeasurement: true }; @@ -159,7 +161,8 @@ export class UserDataSyncService extends Disposable implements IUserDataSyncServ } this.telemetryService.publicLog2('sync/getmanifest'); - let manifest = await this.userDataSyncStoreService.manifest(); + const syncHeaders: IHeaders = { 'X-Execution-Id': generateUuid() }; + let manifest = await this.userDataSyncStoreService.manifest(syncHeaders); // Server has no data but this machine was synced before if (manifest === null && await this.hasPreviouslySynced()) { @@ -195,7 +198,7 @@ export class UserDataSyncService extends Disposable implements IUserDataSyncServ return; } try { - await synchroniser.sync(manifest); + await synchroniser.sync(manifest, syncHeaders); } catch (e) { this.handleSynchronizerError(e, synchroniser.resource); this._syncErrors.push([synchroniser.resource, UserDataSyncError.toUserDataSyncError(e)]); @@ -204,7 +207,7 @@ export class UserDataSyncService extends Disposable implements IUserDataSyncServ // After syncing, get the manifest if it was not available before if (manifest === null) { - manifest = await this.userDataSyncStoreService.manifest(); + manifest = await this.userDataSyncStoreService.manifest(syncHeaders); } // Return if cancellation is requested diff --git a/src/vs/platform/userDataSync/common/userDataSyncStoreService.ts b/src/vs/platform/userDataSync/common/userDataSyncStoreService.ts index 320a11ed626..199ac53f228 100644 --- a/src/vs/platform/userDataSync/common/userDataSyncStoreService.ts +++ b/src/vs/platform/userDataSync/common/userDataSyncStoreService.ts @@ -124,13 +124,13 @@ export class UserDataSyncStoreService extends Disposable implements IUserDataSyn } } - async read(resource: ServerResource, oldValue: IUserData | null): Promise { + async read(resource: ServerResource, oldValue: IUserData | null, headers: IHeaders = {}): Promise { if (!this.userDataSyncStore) { throw new Error('No settings sync store url configured.'); } const url = joinPath(this.userDataSyncStore.url, 'resource', resource, 'latest').toString(); - const headers: IHeaders = {}; + headers = { ...headers }; // Disable caching as they are cached by synchronisers headers['Cache-Control'] = 'no-cache'; if (oldValue) { @@ -156,13 +156,14 @@ export class UserDataSyncStoreService extends Disposable implements IUserDataSyn return { ref, content }; } - async write(resource: ServerResource, data: string, ref: string | null): Promise { + async write(resource: ServerResource, data: string, ref: string | null, headers: IHeaders = {}): Promise { if (!this.userDataSyncStore) { throw new Error('No settings sync store url configured.'); } const url = joinPath(this.userDataSyncStore.url, 'resource', resource).toString(); - const headers: IHeaders = { 'Content-Type': 'text/plain' }; + headers = { ...headers }; + headers['Content-Type'] = 'text/plain'; if (ref) { headers['If-Match'] = ref; } @@ -180,13 +181,14 @@ export class UserDataSyncStoreService extends Disposable implements IUserDataSyn return newRef; } - async manifest(): Promise { + async manifest(headers: IHeaders = {}): Promise { if (!this.userDataSyncStore) { throw new Error('No settings sync store url configured.'); } const url = joinPath(this.userDataSyncStore.url, 'manifest').toString(); - const headers: IHeaders = { 'Content-Type': 'application/json' }; + headers = { ...headers }; + headers['Content-Type'] = 'application/json'; const context = await this.request({ type: 'GET', url, headers }, CancellationToken.None); if (!isSuccess(context)) { diff --git a/src/vs/platform/userDataSync/test/common/userDataSyncService.test.ts b/src/vs/platform/userDataSync/test/common/userDataSyncService.test.ts index 0493a52be3f..87a2da946a5 100644 --- a/src/vs/platform/userDataSync/test/common/userDataSyncService.test.ts +++ b/src/vs/platform/userDataSync/test/common/userDataSyncService.test.ts @@ -583,4 +583,24 @@ suite('UserDataSyncService', () => { assert.deepEqual(testObject.conflicts, []); }); + test('test sync send execution id header', async () => { + // Setup the client + const target = new UserDataSyncTestServer(); + const client = disposableStore.add(new UserDataSyncClient(target)); + await client.setUp(); + const testObject = client.instantiationService.get(IUserDataSyncService); + + await testObject.sync(); + + for (const request of target.requestsWithAllHeaders) { + const hasExecutionIdHeader = request.headers && request.headers['X-Execution-Id'] && request.headers['X-Execution-Id'].length > 0; + if (request.url.startsWith(`${target.url}/v1/resource/machines`)) { + assert.ok(!hasExecutionIdHeader, `Should not have execution header: ${request.url}`); + } else { + assert.ok(hasExecutionIdHeader, `Should have execution header: ${request.url}`); + } + } + + }); + });