Skip to content

Commit

Permalink
Fix #104496
Browse files Browse the repository at this point in the history
  • Loading branch information
sandy081 committed Aug 26, 2020
1 parent 3613fc9 commit 6be16f9
Show file tree
Hide file tree
Showing 10 changed files with 55 additions and 27 deletions.
20 changes: 10 additions & 10 deletions src/vs/platform/userDataSync/common/userDataAutoSyncService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -131,8 +131,8 @@ export class UserDataAutoSyncService extends UserDataAutoSyncEnablementService i

this._register(userDataSyncAccountService.onDidChangeAccount(() => this.updateAutoSync()));
this._register(userDataSyncStoreService.onDidChangeDonotMakeRequestsUntil(() => this.updateAutoSync()));
this._register(Event.debounce<string, string[]>(userDataSyncService.onDidChangeLocal, (last, source) => last ? [...last, source] : [source], 1000)(sources => this.triggerSync(sources, false)));
this._register(Event.filter(this.userDataSyncResourceEnablementService.onDidChangeResourceEnablement, ([, enabled]) => enabled)(() => this.triggerSync(['resourceEnablement'], false)));
this._register(Event.debounce<string, string[]>(userDataSyncService.onDidChangeLocal, (last, source) => last ? [...last, source] : [source], 1000)(sources => this.triggerSync(sources, false, false)));
this._register(Event.filter(this.userDataSyncResourceEnablementService.onDidChangeResourceEnablement, ([, enabled]) => enabled)(() => this.triggerSync(['resourceEnablement'], false, false)));
}
}

Expand Down Expand Up @@ -320,7 +320,7 @@ export class UserDataAutoSyncService extends UserDataAutoSyncEnablementService i
}

private sources: string[] = [];
async triggerSync(sources: string[], skipIfSyncedRecently: boolean): Promise<void> {
async triggerSync(sources: string[], skipIfSyncedRecently: boolean, disableCache: boolean): Promise<void> {
if (this.autoSync.value === undefined) {
return this.syncTriggerDelayer.cancel();
}
Expand All @@ -337,7 +337,7 @@ export class UserDataAutoSyncService extends UserDataAutoSyncEnablementService i
this.telemetryService.publicLog2<{ sources: string[] }, AutoSyncClassification>('sync/triggered', { sources: this.sources });
this.sources = [];
if (this.autoSync.value) {
await this.autoSync.value.sync('Activity');
await this.autoSync.value.sync('Activity', disableCache);
}
}, this.successiveFailures
? this.getSyncTriggerDelayTime() * 1 * Math.min(Math.pow(2, this.successiveFailures), 60) /* Delay exponentially until max 1 minute */
Expand Down Expand Up @@ -393,14 +393,14 @@ class AutoSync extends Disposable {
this.logService.info('Auto Sync: Stopped');
}));
this.logService.info('Auto Sync: Started');
this.sync(AutoSync.INTERVAL_SYNCING);
this.sync(AutoSync.INTERVAL_SYNCING, false);
}

private waitUntilNextIntervalAndSync(): void {
this.intervalHandler.value = disposableTimeout(() => this.sync(AutoSync.INTERVAL_SYNCING), this.interval);
this.intervalHandler.value = disposableTimeout(() => this.sync(AutoSync.INTERVAL_SYNCING, false), this.interval);
}

sync(reason: string): Promise<void> {
sync(reason: string, disableCache: boolean): Promise<void> {
const syncPromise = createCancelablePromise(async token => {
if (this.syncPromise) {
try {
Expand All @@ -414,7 +414,7 @@ class AutoSync extends Disposable {
}
}
}
return this.doSync(reason, token);
return this.doSync(reason, disableCache, token);
});
this.syncPromise = syncPromise;
this.syncPromise.finally(() => this.syncPromise = undefined);
Expand All @@ -435,12 +435,12 @@ class AutoSync extends Disposable {
!isEqual(current.stableUrl, previous.stableUrl));
}

private async doSync(reason: string, token: CancellationToken): Promise<void> {
private async doSync(reason: string, disableCache: boolean, token: CancellationToken): Promise<void> {
this.logService.info(`Auto Sync: Triggered by ${reason}`);
this._onDidStartSync.fire();
let error: Error | undefined;
try {
this.syncTask = await this.userDataSyncService.createSyncTask();
this.syncTask = await this.userDataSyncService.createSyncTask(disableCache);
if (token.isCancellationRequested) {
return;
}
Expand Down
4 changes: 2 additions & 2 deletions src/vs/platform/userDataSync/common/userDataSync.ts
Original file line number Diff line number Diff line change
Expand Up @@ -437,7 +437,7 @@ export interface IUserDataSyncService {
readonly onDidResetRemote: Event<void>;
readonly onDidResetLocal: Event<void>;

createSyncTask(): Promise<ISyncTask>;
createSyncTask(disableCache?: boolean): Promise<ISyncTask>;
createManualSyncTask(): Promise<IManualSyncTask>;

replace(uri: URI): Promise<void>;
Expand Down Expand Up @@ -465,7 +465,7 @@ export interface IUserDataAutoSyncService {
canToggleEnablement(): boolean;
turnOn(): Promise<void>;
turnOff(everywhere: boolean): Promise<void>;
triggerSync(sources: string[], hasToLimitSync: boolean): Promise<void>;
triggerSync(sources: string[], hasToLimitSync: boolean, disableCache: boolean): Promise<void>;
}

export const IUserDataSyncUtilService = createDecorator<IUserDataSyncUtilService>('IUserDataSyncUtilService');
Expand Down
2 changes: 1 addition & 1 deletion src/vs/platform/userDataSync/common/userDataSyncIpc.ts
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ export class UserDataAutoSyncChannel implements IServerChannel {

call(context: any, command: string, args?: any): Promise<any> {
switch (command) {
case 'triggerSync': return this.service.triggerSync(args[0], args[1]);
case 'triggerSync': return this.service.triggerSync(args[0], args[1], args[2]);
case 'turnOn': return this.service.turnOn();
case 'turnOff': return this.service.turnOff(args[0]);
}
Expand Down
8 changes: 6 additions & 2 deletions src/vs/platform/userDataSync/common/userDataSyncService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -106,13 +106,17 @@ export class UserDataSyncService extends Disposable implements IUserDataSyncServ
this.onDidChangeLocal = Event.any(...this.synchronisers.map(s => Event.map(s.onDidChangeLocal, () => s.resource)));
}

async createSyncTask(): Promise<ISyncTask> {
async createSyncTask(disableCache?: boolean): Promise<ISyncTask> {
await this.checkEnablement();

const executionId = generateUuid();
let manifest: IUserDataManifest | null;
try {
manifest = await this.userDataSyncStoreService.manifest(createSyncHeaders(executionId));
const syncHeaders = createSyncHeaders(executionId);
if (disableCache) {
syncHeaders['Cache-Control'] = 'no-cache';
}
manifest = await this.userDataSyncStoreService.manifest(syncHeaders);
} catch (error) {
error = UserDataSyncError.toUserDataSyncError(error);
this.telemetryService.publicLog2<{ code: string, service: string, resource?: string, executionId?: string }, SyncErrorClassification>('sync/error', { code: error.code, resource: error.resource, executionId, service: this.userDataSyncStoreManagementService.userDataSyncStore!.url.toString() });
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ export class UserDataAutoSyncService extends BaseUserDataAutoSyncService {
this._register(Event.debounce<string, string[]>(Event.any<string>(
Event.map(electronService.onWindowFocus, () => 'windowFocus'),
Event.map(electronService.onWindowOpen, () => 'windowOpen'),
), (last, source) => last ? [...last, source] : [source], 1000)(sources => this.triggerSync(sources, true)));
), (last, source) => last ? [...last, source] : [source], 1000)(sources => this.triggerSync(sources, true, false)));
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ class TestUserDataAutoSyncService extends UserDataAutoSyncService {
protected getSyncTriggerDelayTime(): number { return 50; }

sync(): Promise<void> {
return this.triggerSync(['sync'], false);
return this.triggerSync(['sync'], false, false);
}
}

Expand All @@ -43,7 +43,7 @@ suite('UserDataAutoSyncService', () => {
const testObject: UserDataAutoSyncService = client.instantiationService.createInstance(TestUserDataAutoSyncService);

// Trigger auto sync with settings change
await testObject.triggerSync([SyncResource.Settings], false);
await testObject.triggerSync([SyncResource.Settings], false, false);

// Filter out machine requests
const actual = target.requests.filter(request => !request.url.startsWith(`${target.url}/v1/resource/machines`));
Expand All @@ -66,7 +66,7 @@ suite('UserDataAutoSyncService', () => {

// Trigger auto sync with settings change multiple times
for (let counter = 0; counter < 2; counter++) {
await testObject.triggerSync([SyncResource.Settings], false);
await testObject.triggerSync([SyncResource.Settings], false, false);
}

// Filter out machine requests
Expand All @@ -91,7 +91,7 @@ suite('UserDataAutoSyncService', () => {
const testObject: UserDataAutoSyncService = client.instantiationService.createInstance(TestUserDataAutoSyncService);

// Trigger auto sync with window focus once
await testObject.triggerSync(['windowFocus'], true);
await testObject.triggerSync(['windowFocus'], true, false);

// Filter out machine requests
const actual = target.requests.filter(request => !request.url.startsWith(`${target.url}/v1/resource/machines`));
Expand All @@ -114,7 +114,7 @@ suite('UserDataAutoSyncService', () => {

// Trigger auto sync with window focus multiple times
for (let counter = 0; counter < 2; counter++) {
await testObject.triggerSync(['windowFocus'], true);
await testObject.triggerSync(['windowFocus'], true, false);
}

// Filter out machine requests
Expand Down Expand Up @@ -401,4 +401,28 @@ suite('UserDataAutoSyncService', () => {
assert.deepEqual(target.requests, []);
});

test('test cache control header with no cache is sent when triggered with disable cache option', async () => {
const target = new UserDataSyncTestServer(5, 1);

// Set up and sync from the test client
const testClient = disposableStore.add(new UserDataSyncClient(target));
await testClient.setUp();
const testObject: TestUserDataAutoSyncService = testClient.instantiationService.createInstance(TestUserDataAutoSyncService);

await testObject.triggerSync(['some reason'], true, true);
assert.equal(target.requestsWithAllHeaders[0].headers!['Cache-Control'], 'no-cache');
});

test('test cache control header is not sent when triggered without disable cache option', async () => {
const target = new UserDataSyncTestServer(5, 1);

// Set up and sync from the test client
const testClient = disposableStore.add(new UserDataSyncClient(target));
await testClient.setUp();
const testObject: TestUserDataAutoSyncService = testClient.instantiationService.createInstance(TestUserDataAutoSyncService);

await testObject.triggerSync(['some reason'], true, false);
assert.equal(target.requestsWithAllHeaders[0].headers!['Cache-Control'], undefined);
});

});
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ export class UserDataAutoSyncService extends BaseUserDataAutoSyncService {
this._register(Event.debounce<string, string[]>(Event.any<string>(
Event.map(hostService.onDidChangeFocus, () => 'windowFocus'),
instantiationService.createInstance(UserDataSyncTrigger).onDidTriggerSync,
), (last, source) => last ? [...last, source] : [source], 1000)(sources => this.triggerSync(sources, true)));
), (last, source) => last ? [...last, source] : [source], 1000)(sources => this.triggerSync(sources, true, false)));
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -1031,7 +1031,7 @@ export class UserDataSyncWorkbenchContribution extends Disposable implements IWo
});
}
run(accessor: ServicesAccessor): Promise<any> {
return that.userDataAutoSyncService.triggerSync([syncNowCommand.id], false);
return that.userDataAutoSyncService.triggerSync([syncNowCommand.id], false, true);
}
}));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,11 @@ export class UserDataAutoSyncService extends UserDataAutoSyncEnablementService i
) {
super(storageService, environmentService, userDataSyncStoreManagementService);
this.channel = sharedProcessService.getChannel('userDataAutoSync');
this._register(instantiationService.createInstance(UserDataSyncTrigger).onDidTriggerSync(source => this.triggerSync([source], true)));
this._register(instantiationService.createInstance(UserDataSyncTrigger).onDidTriggerSync(source => this.triggerSync([source], true, false)));
}

triggerSync(sources: string[], hasToLimitSync: boolean): Promise<void> {
return this.channel.call('triggerSync', [sources, hasToLimitSync]);
triggerSync(sources: string[], hasToLimitSync: boolean, disableCache: boolean): Promise<void> {
return this.channel.call('triggerSync', [sources, hasToLimitSync, disableCache]);
}

turnOn(): Promise<void> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -718,7 +718,7 @@ class SimpleUserDataAutoSyncAccountService implements IUserDataAutoSyncService {
canToggleEnablement(): boolean { return false; }
async turnOn(): Promise<void> { }
async turnOff(everywhere: boolean): Promise<void> { }
async triggerSync(sources: string[], hasToLimitSync: boolean): Promise<void> { }
async triggerSync(sources: string[], hasToLimitSync: boolean, disableCache: boolean): Promise<void> { }
}

registerSingleton(IUserDataAutoSyncService, SimpleUserDataAutoSyncAccountService);
Expand Down

1 comment on commit 6be16f9

@sandy081
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Attached fix to wrong issue. This fix is meant for #105366

Please sign in to comment.