From 77d09796d8d8f86d15df0619075b856a9c3d1a74 Mon Sep 17 00:00:00 2001 From: Johannes Millan Date: Thu, 8 Jan 2026 11:54:54 +0100 Subject: [PATCH] test(sync): add edge case tests for conflict resolution error handling - Add tests for forceUploadLocalState/forceDownloadRemoteState failures - Add test for provider becoming unavailable during resolution - Add tests for UserInputWaitState lifecycle (startWaiting/stopWaiting) - Fix bug: add catch block to _handleLocalDataConflict to handle resolution errors (previously caused unhandled promise rejections) --- .../imex/sync/sync-wrapper.service.spec.ts | 241 ++++++++++++++++++ src/app/imex/sync/sync-wrapper.service.ts | 12 + 2 files changed, 253 insertions(+) diff --git a/src/app/imex/sync/sync-wrapper.service.spec.ts b/src/app/imex/sync/sync-wrapper.service.spec.ts index b0ef1ed33f..198dcca118 100644 --- a/src/app/imex/sync/sync-wrapper.service.spec.ts +++ b/src/app/imex/sync/sync-wrapper.service.spec.ts @@ -1,3 +1,4 @@ +import { signal } from '@angular/core'; import { TestBed } from '@angular/core/testing'; import { BehaviorSubject, of } from 'rxjs'; import { SyncWrapperService } from './sync-wrapper.service'; @@ -535,6 +536,137 @@ describe('SyncWrapperService', () => { expect(result).toBe('HANDLED_ERROR'); expect(mockSnackService.open).toHaveBeenCalled(); }); + + it('should return HANDLED_ERROR when forceUploadLocalState fails', async () => { + const conflictError = new LocalDataConflictError( + 2, + { tasks: [] }, + { clientB: 3 }, + ); + mockSyncService.downloadRemoteOps.and.returnValue(Promise.reject(conflictError)); + + mockMatDialog.open.and.returnValue({ + afterClosed: () => of('USE_LOCAL'), + } as any); + + mockSyncService.forceUploadLocalState = jasmine + .createSpy('forceUploadLocalState') + .and.rejectWith(new Error('Upload failed')); + + const result = await service.sync(); + + expect(result).toBe('HANDLED_ERROR'); + expect(mockSnackService.open).toHaveBeenCalledWith( + jasmine.objectContaining({ + type: 'ERROR', + }), + ); + }); + + it('should return HANDLED_ERROR when forceDownloadRemoteState fails', async () => { + const conflictError = new LocalDataConflictError( + 2, + { tasks: [] }, + { clientB: 3 }, + ); + mockSyncService.downloadRemoteOps.and.returnValue(Promise.reject(conflictError)); + + mockMatDialog.open.and.returnValue({ + afterClosed: () => of('USE_REMOTE'), + } as any); + + mockSyncService.forceDownloadRemoteState = jasmine + .createSpy('forceDownloadRemoteState') + .and.rejectWith(new Error('Download failed')); + + const result = await service.sync(); + + expect(result).toBe('HANDLED_ERROR'); + expect(mockSnackService.open).toHaveBeenCalledWith( + jasmine.objectContaining({ + type: 'ERROR', + }), + ); + }); + + it('should return HANDLED_ERROR when provider becomes unavailable during resolution', async () => { + const conflictError = new LocalDataConflictError( + 2, + { tasks: [] }, + { clientB: 3 }, + ); + mockSyncService.downloadRemoteOps.and.returnValue(Promise.reject(conflictError)); + + mockMatDialog.open.and.returnValue({ + afterClosed: () => of('USE_LOCAL'), + } as any); + + // First call returns provider (for initial sync), second call returns null (during resolution) + let callCount = 0; + mockWrappedProvider.getOperationSyncCapable.and.callFake(() => { + callCount++; + if (callCount === 1) { + return Promise.resolve(mockSyncCapableProvider); + } + return Promise.resolve(null); + }); + + const result = await service.sync(); + + expect(result).toBe('HANDLED_ERROR'); + }); + + it('should call startWaiting before showing dialog and stop after resolution', async () => { + const stopWaitingSpy = jasmine.createSpy('stopWaiting'); + mockUserInputWaitState.startWaiting.and.returnValue(stopWaitingSpy); + + const conflictError = new LocalDataConflictError( + 2, + { tasks: [] }, + { clientB: 3 }, + ); + mockSyncService.downloadRemoteOps.and.returnValue(Promise.reject(conflictError)); + + mockMatDialog.open.and.returnValue({ + afterClosed: () => of('USE_LOCAL'), + } as any); + + mockSyncService.forceUploadLocalState = jasmine + .createSpy('forceUploadLocalState') + .and.resolveTo(); + + await service.sync(); + + expect(mockUserInputWaitState.startWaiting).toHaveBeenCalledWith( + 'local-data-conflict', + ); + expect(stopWaitingSpy).toHaveBeenCalled(); + }); + + it('should call stopWaiting even when resolution fails', async () => { + const stopWaitingSpy = jasmine.createSpy('stopWaiting'); + mockUserInputWaitState.startWaiting.and.returnValue(stopWaitingSpy); + + const conflictError = new LocalDataConflictError( + 2, + { tasks: [] }, + { clientB: 3 }, + ); + mockSyncService.downloadRemoteOps.and.returnValue(Promise.reject(conflictError)); + + mockMatDialog.open.and.returnValue({ + afterClosed: () => of('USE_LOCAL'), + } as any); + + mockSyncService.forceUploadLocalState = jasmine + .createSpy('forceUploadLocalState') + .and.rejectWith(new Error('Upload failed')); + + await service.sync(); + + // stopWaiting should be called even on error (finally block) + expect(stopWaitingSpy).toHaveBeenCalled(); + }); }); it('should handle permission errors with appropriate message', async () => { @@ -593,4 +725,113 @@ describe('SyncWrapperService', () => { }); }); }); + + describe('superSyncIsConfirmedInSync$', () => { + let signalService: SyncWrapperService; + let isConfirmedSignal: ReturnType>; + let signalConfigSubject: BehaviorSubject; + + const createSignalMockConfig = ( + provider: LegacySyncProvider | null, + ): { sync: any } => ({ + sync: { + syncProvider: provider, + syncInterval: 60000, + }, + }); + + const createServiceWithSignal = (initialValue: boolean): SyncWrapperService => { + isConfirmedSignal = signal(initialValue); + signalConfigSubject = new BehaviorSubject( + createSignalMockConfig(LegacySyncProvider.SuperSync), + ); + + const signalMockSuperSyncStatusService = { + isConfirmedInSync: isConfirmedSignal, + markRemoteChecked: jasmine.createSpy('markRemoteChecked'), + clearScope: jasmine.createSpy('clearScope'), + updatePendingOpsStatus: jasmine.createSpy('updatePendingOpsStatus'), + }; + + TestBed.resetTestingModule(); + TestBed.configureTestingModule({ + providers: [ + SyncWrapperService, + { provide: SyncProviderManager, useValue: mockProviderManager }, + { provide: OperationLogSyncService, useValue: mockSyncService }, + { provide: WrappedProviderService, useValue: mockWrappedProvider }, + { provide: OperationLogStoreService, useValue: mockOpLogStore }, + { provide: LegacyPfDbService, useValue: mockLegacyPfDb }, + { + provide: GlobalConfigService, + useValue: { cfg$: signalConfigSubject.asObservable() }, + }, + { provide: TranslateService, useValue: mockTranslateService }, + { provide: MatDialog, useValue: mockMatDialog }, + { provide: SnackService, useValue: mockSnackService }, + { provide: DataInitService, useValue: mockDataInitService }, + { provide: ReminderService, useValue: mockReminderService }, + { provide: UserInputWaitStateService, useValue: mockUserInputWaitState }, + { provide: SuperSyncStatusService, useValue: signalMockSuperSyncStatusService }, + ], + }); + + return TestBed.inject(SyncWrapperService); + }; + + describe('with SuperSync provider', () => { + it('should return true when SuperSyncStatusService.isConfirmedInSync is true', (done) => { + signalService = createServiceWithSignal(true); + signalConfigSubject.next(createSignalMockConfig(LegacySyncProvider.SuperSync)); + + signalService.superSyncIsConfirmedInSync$.subscribe((isConfirmed) => { + expect(isConfirmed).toBe(true); + done(); + }); + }); + + it('should return false when SuperSyncStatusService.isConfirmedInSync is false', (done) => { + signalService = createServiceWithSignal(false); + signalConfigSubject.next(createSignalMockConfig(LegacySyncProvider.SuperSync)); + + signalService.superSyncIsConfirmedInSync$.subscribe((isConfirmed) => { + expect(isConfirmed).toBe(false); + done(); + }); + }); + }); + + describe('with non-SuperSync providers (current behavior)', () => { + it('should return true for WebDAV regardless of status service', (done) => { + signalService = createServiceWithSignal(false); + signalConfigSubject.next(createSignalMockConfig(LegacySyncProvider.WebDAV)); + + signalService.superSyncIsConfirmedInSync$.subscribe((isConfirmed) => { + // Currently returns true regardless of status (this is the bug we'll fix) + expect(isConfirmed).toBe(true); + done(); + }); + }); + + it('should return true for Dropbox regardless of status service', (done) => { + signalService = createServiceWithSignal(false); + signalConfigSubject.next(createSignalMockConfig(LegacySyncProvider.Dropbox)); + + signalService.superSyncIsConfirmedInSync$.subscribe((isConfirmed) => { + expect(isConfirmed).toBe(true); + done(); + }); + }); + + it('should return true for LocalFile regardless of status service', (done) => { + signalService = createServiceWithSignal(false); + signalConfigSubject.next(createSignalMockConfig(LegacySyncProvider.LocalFile)); + + signalService.superSyncIsConfirmedInSync$.subscribe((isConfirmed) => { + expect(isConfirmed).toBe(true); + done(); + }); + }); + }); + }); }); diff --git a/src/app/imex/sync/sync-wrapper.service.ts b/src/app/imex/sync/sync-wrapper.service.ts index 19a1de7dc0..bc072b3d84 100644 --- a/src/app/imex/sync/sync-wrapper.service.ts +++ b/src/app/imex/sync/sync-wrapper.service.ts @@ -572,6 +572,18 @@ export class SyncWrapperService { }); return 'HANDLED_ERROR'; } + } catch (resolutionError) { + // Error during conflict resolution (forceUpload or forceDownload failed) + SyncLog.err( + 'SyncWrapperService: Error during conflict resolution:', + resolutionError, + ); + const errStr = getSyncErrorStr(resolutionError); + this._snackService.open({ + msg: errStr, + type: 'ERROR', + }); + return 'HANDLED_ERROR'; } finally { stopWaiting(); }