Skip to content

Commit 793c1b2

Browse files
sandy081sandy081
committed
microsoft#100346 Use resource previews and remove conflict type
1 parent 8025723 commit 793c1b2

8 files changed

Lines changed: 151 additions & 129 deletions

File tree

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

Lines changed: 62 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ import { VSBuffer } from 'vs/base/common/buffer';
99
import { URI } from 'vs/base/common/uri';
1010
import {
1111
SyncResource, SyncStatus, IUserData, IUserDataSyncStoreService, UserDataSyncErrorCode, UserDataSyncError, IUserDataSyncLogService, IUserDataSyncUtilService,
12-
IUserDataSyncResourceEnablementService, IUserDataSyncBackupStoreService, Conflict, ISyncResourceHandle, USER_DATA_SYNC_SCHEME, ISyncResourcePreview as IBaseSyncResourcePreview,
12+
IUserDataSyncResourceEnablementService, IUserDataSyncBackupStoreService, ISyncResourceHandle, USER_DATA_SYNC_SCHEME, ISyncResourcePreview as IBaseSyncResourcePreview,
1313
IUserDataManifest, ISyncData, IRemoteUserData, PREVIEW_DIR_NAME, IResourcePreview as IBaseResourcePreview, Change
1414
} from 'vs/platform/userDataSync/common/userDataSync';
1515
import { IEnvironmentService } from 'vs/platform/environment/common/environment';
@@ -78,10 +78,11 @@ export abstract class AbstractSynchroniser extends Disposable {
7878
private _onDidChangStatus: Emitter<SyncStatus> = this._register(new Emitter<SyncStatus>());
7979
readonly onDidChangeStatus: Event<SyncStatus> = this._onDidChangStatus.event;
8080

81-
private _conflicts: Conflict[] = [];
82-
get conflicts(): Conflict[] { return this._conflicts; }
83-
private _onDidChangeConflicts: Emitter<Conflict[]> = this._register(new Emitter<Conflict[]>());
84-
readonly onDidChangeConflicts: Event<Conflict[]> = this._onDidChangeConflicts.event;
81+
private _resourcePreviews: IResourcePreview[] = [];
82+
get resourcePreviews(): IResourcePreview[] { return this._resourcePreviews; }
83+
get conflicts(): IResourcePreview[] { return this._resourcePreviews.filter(({ hasConflicts }) => hasConflicts); }
84+
private _onDidChangeConflicts: Emitter<IResourcePreview[]> = this._register(new Emitter<IResourcePreview[]>());
85+
readonly onDidChangeConflicts: Event<IResourcePreview[]> = this._onDidChangeConflicts.event;
8586

8687
private readonly localChangeTriggerScheduler = new RunOnceScheduler(() => this.doTriggerLocalChange(), 50);
8788
private readonly _onDidChangeLocal: Emitter<void> = this._register(new Emitter<void>());
@@ -155,16 +156,6 @@ export abstract class AbstractSynchroniser extends Disposable {
155156
// Log to telemetry when conflicts are resolved
156157
this.telemetryService.publicLog2<{ source: string }, SyncSourceClassification>('sync/conflictsResolved', { source: this.resource });
157158
}
158-
if (this.status !== SyncStatus.HasConflicts) {
159-
this.setConflicts([]);
160-
}
161-
}
162-
}
163-
164-
private setConflicts(conflicts: Conflict[]) {
165-
if (!equals(this._conflicts, conflicts, (a, b) => isEqual(a.local, b.local) && isEqual(a.remote, b.remote))) {
166-
this._conflicts = conflicts;
167-
this._onDidChangeConflicts.fire(this._conflicts);
168159
}
169160
}
170161

@@ -364,6 +355,9 @@ export abstract class AbstractSynchroniser extends Disposable {
364355
// reset preview
365356
this.syncPreviewPromise = null;
366357

358+
// reset resource previews
359+
await this.updateResourcePreviews([], CancellationToken.None);
360+
367361
return SyncStatus.Idle;
368362
} catch (error) {
369363

@@ -374,47 +368,50 @@ export abstract class AbstractSynchroniser extends Disposable {
374368
}
375369
}
376370

377-
async acceptConflict(conflictUri: URI, conflictContent: string): Promise<void> {
378-
let preview = this.syncPreviewPromise ? await this.syncPreviewPromise : null;
379-
380-
if (!preview || !preview.resourcePreviews.some(({ hasConflicts }) => hasConflicts)) {
371+
async acceptPreviewContent(resource: URI, content: string): Promise<void> {
372+
if (!this.syncPreviewPromise) {
381373
return;
382374
}
383375

376+
let preview = await this.syncPreviewPromise;
384377
this.syncPreviewPromise = createCancelablePromise(async token => {
385-
const newPreview = await this.updateSyncResourcePreviewWithConflict(preview!, conflictUri, conflictContent, token);
386-
await this.updateConflicts(newPreview.resourcePreviews, token);
378+
const newPreview = await this.updateSyncResourcePreviewContent(preview, resource, content, token);
379+
380+
if (!token.isCancellationRequested) {
381+
await this.updateResourcePreviews(newPreview.resourcePreviews, token);
382+
}
383+
387384
return newPreview;
388385
});
389386
preview = await this.syncPreviewPromise;
390387

391388
if (!preview.resourcePreviews.some(({ hasConflicts }) => hasConflicts)) {
392-
// apply preview
389+
// Apply preview if there are no conflicts
393390
await this.applyPreview(preview.remoteUserData, preview.lastSyncUserData, preview.resourcePreviews, false);
394391

395392
// reset preview
396393
this.syncPreviewPromise = null;
397394

395+
// reset resource previews
396+
await this.updateResourcePreviews([], CancellationToken.None);
397+
398+
// reset status
398399
this.setStatus(SyncStatus.Idle);
399400
}
400401
}
401402

402-
private async updateSyncResourcePreviewWithConflict(preview: ISyncResourcePreview, conflictResource: URI, previewContent: string, token: CancellationToken): Promise<ISyncResourcePreview> {
403-
const conflict = this.conflicts.find(({ local, remote }) => isEqual(local, conflictResource) || isEqual(remote, conflictResource));
404-
if (!conflict) {
405-
return preview;
406-
}
407-
const index = preview.resourcePreviews.findIndex(({ previewResource }) => previewResource && isEqual(previewResource, conflict.local));
408-
if (index === -1) {
409-
return preview;
403+
private async updateSyncResourcePreviewContent(preview: ISyncResourcePreview, resource: URI, previewContent: string, token: CancellationToken): Promise<ISyncResourcePreview> {
404+
const index = preview.resourcePreviews.findIndex(({ localResource, remoteResource, previewResource }) => isEqual(localResource, resource) || isEqual(remoteResource, resource) || isEqual(previewResource, resource));
405+
if (index !== -1) {
406+
const resourcePreviews = [...preview.resourcePreviews];
407+
const resourcePreview = await this.updateResourcePreviewContent(resourcePreviews[index], resource, previewContent, token);
408+
resourcePreviews[index] = resourcePreview;
409+
preview = {
410+
...preview,
411+
resourcePreviews
412+
};
410413
}
411-
const resourcePreviews = [...preview.resourcePreviews];
412-
const resourcePreview = await this.updateResourcePreviewContent(resourcePreviews[index], conflictResource, previewContent, token);
413-
resourcePreviews[index] = resourcePreview;
414-
return {
415-
...preview,
416-
resourcePreviews
417-
};
414+
return preview;
418415
}
419416

420417
protected async updateResourcePreviewContent(resourcePreview: IResourcePreview, resource: URI, previewContent: string, token: CancellationToken): Promise<IResourcePreview> {
@@ -427,27 +424,25 @@ export abstract class AbstractSynchroniser extends Disposable {
427424
};
428425
}
429426

430-
private async updateConflicts(resourcePreviews: IResourcePreview[], token: CancellationToken): Promise<void> {
431-
const conflicts: Conflict[] = [];
432-
for (const resourcePreview of resourcePreviews) {
433-
if (resourcePreview.hasConflicts) {
434-
conflicts.push({ local: resourcePreview.previewResource!, remote: resourcePreview.remoteResource! });
435-
}
436-
}
427+
private async updateResourcePreviews(resourcePreviews: IResourcePreview[], token: CancellationToken): Promise<void> {
428+
const oldConflicts = this.conflicts;
429+
const oldPreviews = this._resourcePreviews;
430+
this._resourcePreviews = resourcePreviews;
437431

438-
for (const conflict of this.conflicts) {
439-
// clear obsolete conflicts
440-
if (!conflicts.some(({ local }) => isEqual(local, conflict.local))) {
432+
// clear obsolete previews
433+
for (const resourcePreview of oldPreviews) {
434+
if (!this._resourcePreviews.some(({ previewResource }) => isEqual(previewResource, resourcePreview.previewResource))) {
441435
try {
442-
await this.fileService.del(conflict.local);
443-
} catch (error) {
444-
// Ignore & log
445-
this.logService.error(error);
446-
}
436+
await this.fileService.del(resourcePreview.previewResource);
437+
} catch (error) { /* Ignore */ }
447438
}
448439
}
449440

450-
this.setConflicts(conflicts);
441+
// update conflicts
442+
const newConflicts = this.conflicts;
443+
if (!equals(oldConflicts, newConflicts, (a, b) => isEqual(a.previewResource, b.previewResource))) {
444+
this._onDidChangeConflicts.fire(newConflicts);
445+
}
451446
}
452447

453448
async hasPreviouslySynced(): Promise<boolean> {
@@ -528,7 +523,11 @@ export abstract class AbstractSynchroniser extends Disposable {
528523
// For preview, use remoteUserData if lastSyncUserData does not exists and last sync is from current machine
529524
const lastSyncUserDataForPreview = lastSyncUserData === null && isLastSyncFromCurrentMachine ? remoteUserData : lastSyncUserData;
530525
const resourcePreviews = await this.generateSyncPreview(remoteUserData, lastSyncUserDataForPreview, token);
531-
await this.updateConflicts(resourcePreviews, token);
526+
527+
if (!token.isCancellationRequested) {
528+
await this.updateResourcePreviews(resourcePreviews, token);
529+
}
530+
532531
return { remoteUserData, lastSyncUserData, resourcePreviews, isLastSyncFromCurrentMachine };
533532
}
534533

@@ -605,23 +604,20 @@ export abstract class AbstractSynchroniser extends Disposable {
605604
}
606605

607606
async stop(): Promise<void> {
608-
this.logService.info(`${this.syncResourceLogLabel}: Stopped synchronizing ${this.resource.toLowerCase()}.`);
607+
if (this.status === SyncStatus.Idle) {
608+
return;
609+
}
610+
611+
this.logService.trace(`${this.syncResourceLogLabel}: Stopping synchronizing ${this.resource.toLowerCase()}.`);
609612
if (this.syncPreviewPromise) {
610613
this.syncPreviewPromise.cancel();
611614
this.syncPreviewPromise = null;
612615
}
613-
if (this.conflicts.length) {
614-
await Promise.all(this.conflicts.map(async ({ local }) => {
615-
try {
616-
this.fileService.del(local);
617-
} catch (error) {
618-
// Ignore & log
619-
this.logService.error(error);
620-
}
621-
}));
622-
this.setConflicts([]);
623-
}
616+
617+
await this.updateResourcePreviews([], CancellationToken.None);
618+
624619
this.setStatus(SyncStatus.Idle);
620+
this.logService.info(`${this.syncResourceLogLabel}: Stopped synchronizing ${this.resource.toLowerCase()}.`);
625621
}
626622

627623
protected abstract readonly version: number;

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

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -144,6 +144,7 @@ export class SettingsSynchroniser extends AbstractJsonFileSynchroniser implement
144144
const formattingOptions = await this.getFormattingOptions();
145145
const remoteSettingsSyncContent = this.getSettingsSyncContent(remoteUserData);
146146
const lastSettingsSyncContent: ISettingsSyncContent | null = lastSyncUserData ? this.getSettingsSyncContent(lastSyncUserData) : null;
147+
const ignoredSettings = await this.getIgnoredSettings();
147148

148149
let previewContent: string | null = null;
149150
let hasLocalChanged: boolean = false;
@@ -154,7 +155,6 @@ export class SettingsSynchroniser extends AbstractJsonFileSynchroniser implement
154155
const localContent: string = fileContent ? fileContent.value.toString() : '{}';
155156
this.validateContent(localContent);
156157
this.logService.trace(`${this.syncResourceLogLabel}: Merging remote settings with local settings...`);
157-
const ignoredSettings = await this.getIgnoredSettings();
158158
const result = merge(localContent, remoteSettingsSyncContent.settings, lastSettingsSyncContent ? lastSettingsSyncContent.settings : null, ignoredSettings, [], formattingOptions);
159159
previewContent = result.localContent || result.remoteContent;
160160
hasLocalChanged = result.localContent !== null;
@@ -171,7 +171,6 @@ export class SettingsSynchroniser extends AbstractJsonFileSynchroniser implement
171171

172172
if (previewContent && !token.isCancellationRequested) {
173173
// Remove the ignored settings from the preview.
174-
const ignoredSettings = await this.getIgnoredSettings();
175174
const content = updateIgnoredSettings(previewContent, '{}', ignoredSettings, formattingOptions);
176175
await this.fileService.writeFile(this.localPreviewResource, VSBuffer.fromString(content));
177176
}

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

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -281,8 +281,6 @@ export interface ISyncResourceHandle {
281281
uri: URI;
282282
}
283283

284-
export type Conflict = { remote: URI, local: URI };
285-
286284
export interface IRemoteUserData {
287285
ref: string;
288286
syncData: ISyncData | null;
@@ -320,8 +318,9 @@ export interface IUserDataSynchroniser {
320318
readonly resource: SyncResource;
321319
readonly status: SyncStatus;
322320
readonly onDidChangeStatus: Event<SyncStatus>;
323-
readonly conflicts: Conflict[];
324-
readonly onDidChangeConflicts: Event<Conflict[]>;
321+
readonly resourcePreviews: IResourcePreview[];
322+
readonly conflicts: IResourcePreview[];
323+
readonly onDidChangeConflicts: Event<IResourcePreview[]>;
325324
readonly onDidChangeLocal: Event<void>;
326325

327326
pull(): Promise<void>;
@@ -336,7 +335,7 @@ export interface IUserDataSynchroniser {
336335
resetLocal(): Promise<void>;
337336

338337
resolveContent(resource: URI): Promise<string | null>;
339-
acceptConflict(conflictResource: URI, content: string): Promise<void>;
338+
acceptPreviewContent(resource: URI, content: string): Promise<void>;
340339

341340
getRemoteSyncResourceHandles(): Promise<ISyncResourceHandle[]>;
342341
getLocalSyncResourceHandles(): Promise<ISyncResourceHandle[]>;
@@ -357,7 +356,7 @@ export interface IUserDataSyncResourceEnablementService {
357356
setResourceEnablement(resource: SyncResource, enabled: boolean): void;
358357
}
359358

360-
export type SyncResourceConflicts = { syncResource: SyncResource, conflicts: Conflict[] };
359+
export type SyncResourceConflicts = { syncResource: SyncResource, conflicts: IResourcePreview[] };
361360

362361
export interface ISyncTask {
363362
manifest: IUserDataManifest | null;
@@ -393,7 +392,7 @@ export interface IUserDataSyncService {
393392
isFirstTimeSyncingWithAnotherMachine(): Promise<boolean>;
394393
hasPreviouslySynced(): Promise<boolean>;
395394
resolveContent(resource: URI): Promise<string | null>;
396-
acceptConflict(conflictResource: URI, content: string): Promise<void>;
395+
acceptPreviewContent(conflictResource: URI, content: string): Promise<void>;
397396

398397
getLocalSyncResourceHandles(resource: SyncResource): Promise<ISyncResourceHandle[]>;
399398
getRemoteSyncResourceHandles(resource: SyncResource): Promise<ISyncResourceHandle[]>;

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ export class UserDataSyncChannel implements IServerChannel {
5252
case 'resetLocal': return this.service.resetLocal();
5353
case 'hasPreviouslySynced': return this.service.hasPreviouslySynced();
5454
case 'isFirstTimeSyncingWithAnotherMachine': return this.service.isFirstTimeSyncingWithAnotherMachine();
55-
case 'acceptConflict': return this.service.acceptConflict(URI.revive(args[0]), args[1]);
55+
case 'acceptPreviewContent': return this.service.acceptPreviewContent(URI.revive(args[0]), args[1]);
5656
case 'resolveContent': return this.service.resolveContent(URI.revive(args[0]));
5757
case 'getLocalSyncResourceHandles': return this.service.getLocalSyncResourceHandles(args[0]);
5858
case 'getRemoteSyncResourceHandles': return this.service.getRemoteSyncResourceHandles(args[0]);

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

Lines changed: 19 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
* Licensed under the MIT License. See License.txt in the project root for license information.
44
*--------------------------------------------------------------------------------------------*/
55

6-
import { IUserDataSyncService, SyncStatus, IUserDataSyncStoreService, SyncResource, IUserDataSyncLogService, IUserDataSynchroniser, UserDataSyncErrorCode, UserDataSyncError, SyncResourceConflicts, ISyncResourceHandle, IUserDataManifest, ISyncTask, Change } from 'vs/platform/userDataSync/common/userDataSync';
6+
import { IUserDataSyncService, SyncStatus, IUserDataSyncStoreService, SyncResource, IUserDataSyncLogService, IUserDataSynchroniser, UserDataSyncErrorCode, UserDataSyncError, SyncResourceConflicts, ISyncResourceHandle, IUserDataManifest, ISyncTask, Change, IResourcePreview } from 'vs/platform/userDataSync/common/userDataSync';
77
import { Disposable } from 'vs/base/common/lifecycle';
88
import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation';
99
import { Emitter, Event } from 'vs/base/common/event';
@@ -239,12 +239,12 @@ export class UserDataSyncService extends Disposable implements IUserDataSyncServ
239239

240240
}
241241

242-
async acceptConflict(conflict: URI, content: string): Promise<void> {
242+
async acceptPreviewContent(resource: URI, content: string): Promise<void> {
243243
await this.checkEnablement();
244-
const syncResourceConflict = this.conflicts.filter(({ conflicts }) => conflicts.some(({ local, remote }) => isEqual(conflict, local) || isEqual(conflict, remote)))[0];
245-
if (syncResourceConflict) {
246-
const synchroniser = this.getSynchroniser(syncResourceConflict.syncResource);
247-
await synchroniser.acceptConflict(conflict, content);
244+
const synchroniser = this.synchronisers.find(synchroniser => synchroniser.resourcePreviews.some(({ localResource, previewResource, remoteResource }) =>
245+
isEqual(resource, localResource) || isEqual(resource, previewResource) || isEqual(resource, remoteResource)));
246+
if (synchroniser) {
247+
await synchroniser.acceptPreviewContent(resource, content);
248248
}
249249
}
250250

@@ -365,7 +365,7 @@ export class UserDataSyncService extends Disposable implements IUserDataSyncServ
365365

366366
private updateConflicts(): void {
367367
const conflicts = this.computeConflicts();
368-
if (!equals(this._conflicts, conflicts, (a, b) => a.syncResource === b.syncResource && equals(a.conflicts, b.conflicts, (a, b) => isEqual(a.local, b.local) && isEqual(a.remote, b.remote)))) {
368+
if (!equals(this._conflicts, conflicts, (a, b) => a.syncResource === b.syncResource && equals(a.conflicts, b.conflicts, (a, b) => isEqual(a.previewResource, b.previewResource)))) {
369369
this._conflicts = this.computeConflicts();
370370
this._onDidChangeConflicts.fire(conflicts);
371371
}
@@ -412,7 +412,18 @@ export class UserDataSyncService extends Disposable implements IUserDataSyncServ
412412

413413
private computeConflicts(): SyncResourceConflicts[] {
414414
return this.synchronisers.filter(s => s.status === SyncStatus.HasConflicts)
415-
.map(s => ({ syncResource: s.resource, conflicts: s.conflicts }));
415+
.map(s => ({ syncResource: s.resource, conflicts: s.conflicts.map(r => this.toStrictResourcePreview(r)) }));
416+
}
417+
418+
private toStrictResourcePreview(resourcePreview: IResourcePreview): IResourcePreview {
419+
return {
420+
localResource: resourcePreview.localResource,
421+
previewResource: resourcePreview.previewResource,
422+
remoteResource: resourcePreview.remoteResource,
423+
localChange: resourcePreview.localChange,
424+
remoteChange: resourcePreview.remoteChange,
425+
hasConflicts: resourcePreview.hasConflicts,
426+
};
416427
}
417428

418429
getSynchroniser(source: SyncResource): IUserDataSynchroniser {

0 commit comments

Comments
 (0)