From efaffe97a5eda2c1cd2e387e314de1d29ac1f32b Mon Sep 17 00:00:00 2001 From: Johannes Millan Date: Fri, 6 Mar 2026 12:38:09 +0100 Subject: [PATCH] refactor(sync): use discriminated unions for sync orchestrator results Replace flag-based result objects with DownloadOutcome and UploadOutcome discriminated unions at the orchestrator level. This eliminates null returns, optional boolean checks, and ambiguous flag combinations. - Add DownloadOutcome (5 variants) and UploadOutcome (3 variants) types - Update OperationLogSyncService to return discriminated unions - Update SyncWrapperService and ImmediateUploadService consumers - Use exhaustive switch in downloadCallback adapter - Replace piggybackedOps array pass-through with piggybackedOpsCount --- .../imex/sync/sync-wrapper.service.spec.ts | 179 ++++++++++-------- src/app/imex/sync/sync-wrapper.service.ts | 51 ++--- .../op-log/core/types/sync-results.types.ts | 68 +++++++ .../sync/immediate-upload.service.spec.ts | 151 +++++---------- .../op-log/sync/immediate-upload.service.ts | 19 +- .../sync/operation-log-sync.service.spec.ts | 49 +++-- .../op-log/sync/operation-log-sync.service.ts | 112 +++++------ 7 files changed, 326 insertions(+), 303 deletions(-) diff --git a/src/app/imex/sync/sync-wrapper.service.spec.ts b/src/app/imex/sync/sync-wrapper.service.spec.ts index c7c2d294e2..4b97a17e32 100644 --- a/src/app/imex/sync/sync-wrapper.service.spec.ts +++ b/src/app/imex/sync/sync-wrapper.service.spec.ts @@ -92,18 +92,18 @@ describe('SyncWrapperService', () => { ]); mockSyncService.downloadRemoteOps.and.returnValue( Promise.resolve({ - newOpsCount: 0, - serverMigrationHandled: false, - localWinOpsCreated: 0, + kind: 'no_new_ops' as const, }), ); mockSyncService.uploadPendingOps.and.returnValue( Promise.resolve({ + kind: 'completed' as const, uploadedCount: 0, - rejectedCount: 0, - piggybackedOps: [], - rejectedOps: [], + piggybackedOpsCount: 0, localWinOpsCreated: 0, + permanentRejectionCount: 0, + hasMorePiggyback: false, + rejectedOps: [], }), ); @@ -335,10 +335,7 @@ describe('SyncWrapperService', () => { it('should NOT update lastSyncedProviderId when download is cancelled', async () => { mockSyncService.downloadRemoteOps.and.returnValue( Promise.resolve({ - newOpsCount: 0, - serverMigrationHandled: false, - localWinOpsCreated: 0, - cancelled: true, + kind: 'cancelled' as const, }), ); @@ -353,16 +350,18 @@ describe('SyncWrapperService', () => { const callOrder: string[] = []; mockSyncService.downloadRemoteOps.and.callFake(async () => { callOrder.push('download'); - return { newOpsCount: 0, serverMigrationHandled: false, localWinOpsCreated: 0 }; + return { kind: 'no_new_ops' as const }; }); mockSyncService.uploadPendingOps.and.callFake(async () => { callOrder.push('upload'); return { + kind: 'completed' as const, uploadedCount: 0, - rejectedCount: 0, - piggybackedOps: [], - rejectedOps: [], + piggybackedOpsCount: 0, localWinOpsCreated: 0, + permanentRejectionCount: 0, + hasMorePiggyback: false, + rejectedOps: [], }; }); @@ -374,18 +373,20 @@ describe('SyncWrapperService', () => { it('should re-upload when localWinOpsCreated > 0 from download', async () => { mockSyncService.downloadRemoteOps.and.returnValue( Promise.resolve({ + kind: 'ops_processed' as const, newOpsCount: 5, - serverMigrationHandled: false, localWinOpsCreated: 3, // LWW created 3 local-win ops }), ); mockSyncService.uploadPendingOps.and.returnValue( Promise.resolve({ + kind: 'completed' as const, uploadedCount: 3, - rejectedCount: 0, - piggybackedOps: [], - rejectedOps: [], + piggybackedOpsCount: 0, localWinOpsCreated: 0, + permanentRejectionCount: 0, + hasMorePiggyback: false, + rejectedOps: [], }), ); @@ -398,9 +399,7 @@ describe('SyncWrapperService', () => { it('should re-upload when localWinOpsCreated > 0 from upload', async () => { mockSyncService.downloadRemoteOps.and.returnValue( Promise.resolve({ - newOpsCount: 0, - serverMigrationHandled: false, - localWinOpsCreated: 0, + kind: 'no_new_ops' as const, }), ); // First upload returns localWinOpsCreated > 0 @@ -409,19 +408,23 @@ describe('SyncWrapperService', () => { uploadCallCount++; if (uploadCallCount === 1) { return { + kind: 'completed' as const, uploadedCount: 2, - rejectedCount: 0, - piggybackedOps: [], - rejectedOps: [], + piggybackedOpsCount: 0, localWinOpsCreated: 2, // LWW created ops from piggybacked + permanentRejectionCount: 0, + hasMorePiggyback: false, + rejectedOps: [], }; } return { + kind: 'completed' as const, uploadedCount: 2, - rejectedCount: 0, - piggybackedOps: [], - rejectedOps: [], + piggybackedOpsCount: 0, localWinOpsCreated: 0, + permanentRejectionCount: 0, + hasMorePiggyback: false, + rejectedOps: [], }; }); @@ -434,18 +437,20 @@ describe('SyncWrapperService', () => { it('should NOT re-upload when no localWinOpsCreated', async () => { mockSyncService.downloadRemoteOps.and.returnValue( Promise.resolve({ + kind: 'ops_processed' as const, newOpsCount: 5, - serverMigrationHandled: false, localWinOpsCreated: 0, }), ); mockSyncService.uploadPendingOps.and.returnValue( Promise.resolve({ + kind: 'completed' as const, uploadedCount: 3, - rejectedCount: 0, - piggybackedOps: [], - rejectedOps: [], + piggybackedOpsCount: 0, localWinOpsCreated: 0, + permanentRejectionCount: 0, + hasMorePiggyback: false, + rejectedOps: [], }), ); @@ -477,11 +482,13 @@ describe('SyncWrapperService', () => { it('should set ERROR and return HANDLED_ERROR when upload has rejected ops with "Payload too complex"', async () => { mockSyncService.uploadPendingOps.and.returnValue( Promise.resolve({ + kind: 'completed' as const, uploadedCount: 0, - rejectedCount: 1, - piggybackedOps: [], - rejectedOps: [{ opId: 'test-op', error: 'Payload too complex (max depth 50)' }], + piggybackedOpsCount: 0, localWinOpsCreated: 0, + permanentRejectionCount: 1, + hasMorePiggyback: false, + rejectedOps: [{ opId: 'test-op', error: 'Payload too complex (max depth 50)' }], }), ); @@ -495,11 +502,13 @@ describe('SyncWrapperService', () => { it('should set ERROR and return HANDLED_ERROR when upload has rejected ops with "Payload too large"', async () => { mockSyncService.uploadPendingOps.and.returnValue( Promise.resolve({ + kind: 'completed' as const, uploadedCount: 0, - rejectedCount: 1, - piggybackedOps: [], - rejectedOps: [{ opId: 'test-op', error: 'Payload too large' }], + piggybackedOpsCount: 0, localWinOpsCreated: 0, + permanentRejectionCount: 1, + hasMorePiggyback: false, + rejectedOps: [{ opId: 'test-op', error: 'Payload too large' }], }), ); @@ -513,11 +522,13 @@ describe('SyncWrapperService', () => { it('should set ERROR for non-payload rejected ops', async () => { mockSyncService.uploadPendingOps.and.returnValue( Promise.resolve({ + kind: 'completed' as const, uploadedCount: 0, - rejectedCount: 1, - piggybackedOps: [], - rejectedOps: [{ opId: 'test-op', error: 'Some other rejection' }], + piggybackedOpsCount: 0, localWinOpsCreated: 0, + permanentRejectionCount: 1, + hasMorePiggyback: false, + rejectedOps: [{ opId: 'test-op', error: 'Some other rejection' }], }), ); @@ -530,11 +541,13 @@ describe('SyncWrapperService', () => { it('should set IN_SYNC when some ops uploaded and none rejected', async () => { mockSyncService.uploadPendingOps.and.returnValue( Promise.resolve({ + kind: 'completed' as const, uploadedCount: 5, - rejectedCount: 0, - piggybackedOps: [], - rejectedOps: [], + piggybackedOpsCount: 0, localWinOpsCreated: 0, + permanentRejectionCount: 0, + hasMorePiggyback: false, + rejectedOps: [], }), ); @@ -547,14 +560,16 @@ describe('SyncWrapperService', () => { it('should set ERROR when multiple ops rejected', async () => { mockSyncService.uploadPendingOps.and.returnValue( Promise.resolve({ + kind: 'completed' as const, uploadedCount: 3, - rejectedCount: 2, - piggybackedOps: [], + piggybackedOpsCount: 0, + localWinOpsCreated: 0, + permanentRejectionCount: 2, + hasMorePiggyback: false, rejectedOps: [ { opId: 'op1', error: 'Conflict' }, { opId: 'op2', error: 'Validation failed' }, ], - localWinOpsCreated: 0, }), ); @@ -564,8 +579,10 @@ describe('SyncWrapperService', () => { expect(mockProviderManager.setSyncStatus).toHaveBeenCalledWith('ERROR'); }); - it('should set IN_SYNC when uploadResult is null (fresh client)', async () => { - mockSyncService.uploadPendingOps.and.returnValue(Promise.resolve(null)); + it('should set IN_SYNC when uploadResult is blocked_fresh_client', async () => { + mockSyncService.uploadPendingOps.and.returnValue( + Promise.resolve({ kind: 'blocked_fresh_client' as const }), + ); const result = await service.sync(); @@ -573,14 +590,16 @@ describe('SyncWrapperService', () => { expect(mockProviderManager.setSyncStatus).toHaveBeenCalledWith('IN_SYNC'); }); - it('should set IN_SYNC when rejectedCount is 0 even with empty rejectedOps array', async () => { + it('should set IN_SYNC when permanentRejectionCount is 0 even with empty rejectedOps array', async () => { mockSyncService.uploadPendingOps.and.returnValue( Promise.resolve({ + kind: 'completed' as const, uploadedCount: 0, - rejectedCount: 0, - piggybackedOps: [], - rejectedOps: [], + piggybackedOpsCount: 0, localWinOpsCreated: 0, + permanentRejectionCount: 0, + hasMorePiggyback: false, + rejectedOps: [], }), ); @@ -1088,7 +1107,7 @@ describe('SyncWrapperService', () => { callOrder.push('sync-download-start'); await syncPromise; callOrder.push('sync-download-end'); - return { newOpsCount: 0, serverMigrationHandled: false, localWinOpsCreated: 0 }; + return { kind: 'no_new_ops' as const }; }); // Start sync @@ -1415,19 +1434,19 @@ describe('SyncWrapperService', () => { it('should stop after MAX_LWW_REUPLOAD_RETRIES when upload always returns localWinOpsCreated', async () => { mockSyncService.downloadRemoteOps.and.returnValue( Promise.resolve({ - newOpsCount: 0, - serverMigrationHandled: false, - localWinOpsCreated: 0, + kind: 'no_new_ops' as const, }), ); // Upload always returns localWinOpsCreated: 2 (never resolves) mockSyncService.uploadPendingOps.and.returnValue( Promise.resolve({ + kind: 'completed' as const, uploadedCount: 2, - rejectedCount: 0, - piggybackedOps: [], - rejectedOps: [], + piggybackedOpsCount: 0, localWinOpsCreated: 2, + permanentRejectionCount: 0, + hasMorePiggyback: false, + rejectedOps: [], }), ); @@ -1448,9 +1467,7 @@ describe('SyncWrapperService', () => { it('should exit early when retry returns localWinOpsCreated: 0', async () => { mockSyncService.downloadRemoteOps.and.returnValue( Promise.resolve({ - newOpsCount: 0, - serverMigrationHandled: false, - localWinOpsCreated: 0, + kind: 'no_new_ops' as const, }), ); @@ -1458,12 +1475,14 @@ describe('SyncWrapperService', () => { mockSyncService.uploadPendingOps.and.callFake(async () => { uploadCallCount++; return { + kind: 'completed' as const, uploadedCount: 2, - rejectedCount: 0, - piggybackedOps: [], - rejectedOps: [], + piggybackedOpsCount: 0, // First call returns 1, second call returns 0 -> exits loop localWinOpsCreated: uploadCallCount <= 1 ? 1 : 0, + permanentRejectionCount: 0, + hasMorePiggyback: false, + rejectedOps: [], }; }); @@ -1475,12 +1494,10 @@ describe('SyncWrapperService', () => { expect(result).toBe(SyncStatus.InSync); }); - it('should treat null reupload result as 0 localWinOpsCreated and exit loop', async () => { + it('should treat blocked_fresh_client reupload result as 0 localWinOpsCreated and exit loop', async () => { mockSyncService.downloadRemoteOps.and.returnValue( Promise.resolve({ - newOpsCount: 0, - serverMigrationHandled: false, - localWinOpsCreated: 0, + kind: 'no_new_ops' as const, }), ); @@ -1489,20 +1506,22 @@ describe('SyncWrapperService', () => { uploadCallCount++; if (uploadCallCount === 1) { return { + kind: 'completed' as const, uploadedCount: 1, - rejectedCount: 0, - piggybackedOps: [], - rejectedOps: [], + piggybackedOpsCount: 0, localWinOpsCreated: 2, + permanentRejectionCount: 0, + hasMorePiggyback: false, + rejectedOps: [], }; } - // Second call returns null (e.g., fresh client scenario) - return null; + // Second call returns blocked_fresh_client (treated as 0 localWinOpsCreated) + return { kind: 'blocked_fresh_client' as const }; }); const result = await service.sync(); - // 1 initial + 1 retry (returns null -> treated as 0) = 2 total + // 1 initial + 1 retry (returns blocked_fresh_client -> treated as 0) = 2 total expect(mockSyncService.uploadPendingOps).toHaveBeenCalledTimes(2); expect(result).toBe(SyncStatus.InSync); }); @@ -1510,8 +1529,8 @@ describe('SyncWrapperService', () => { it('should enter while loop when both download and upload produce LWW ops', async () => { mockSyncService.downloadRemoteOps.and.returnValue( Promise.resolve({ + kind: 'ops_processed' as const, newOpsCount: 5, - serverMigrationHandled: false, localWinOpsCreated: 2, // download produced LWW ops }), ); @@ -1520,12 +1539,14 @@ describe('SyncWrapperService', () => { mockSyncService.uploadPendingOps.and.callFake(async () => { uploadCallCount++; return { + kind: 'completed' as const, uploadedCount: 3, - rejectedCount: 0, - piggybackedOps: [], - rejectedOps: [], + piggybackedOpsCount: 0, // First upload also produces LWW ops, subsequent do not localWinOpsCreated: uploadCallCount === 1 ? 1 : 0, + permanentRejectionCount: 0, + hasMorePiggyback: false, + rejectedOps: [], }; }); diff --git a/src/app/imex/sync/sync-wrapper.service.ts b/src/app/imex/sync/sync-wrapper.service.ts index 8c45cb3c47..3c8ff26b74 100644 --- a/src/app/imex/sync/sync-wrapper.service.ts +++ b/src/app/imex/sync/sync-wrapper.service.ts @@ -324,14 +324,12 @@ export class SyncWrapperService { syncCapableProvider, isProviderSwitch ? { forceFromSeq0: true } : undefined, ); - SyncLog.log( - `SyncWrapperService: Download complete. newOps=${downloadResult.newOpsCount}, migration=${downloadResult.serverMigrationHandled}`, - ); + SyncLog.log(`SyncWrapperService: Download complete. kind=${downloadResult.kind}`); // If user cancelled the sync import conflict dialog, skip upload entirely. // This keeps the local state unchanged and doesn't push it to the server. // Don't update lastSyncedProvider so the next sync retries with forceFromSeq0. - if (downloadResult.cancelled) { + if (downloadResult.kind === 'cancelled') { SyncLog.log('SyncWrapperService: Sync cancelled by user. Skipping upload phase.'); this._providerManager.setSyncStatus('UNKNOWN_OR_CHANGED'); return 'HANDLED_ERROR'; @@ -343,14 +341,14 @@ export class SyncWrapperService { // 2. Upload pending local ops const uploadResult = await this._opLogSyncService.uploadPendingOps(syncCapableProvider); - if (uploadResult) { + if (uploadResult.kind === 'completed') { SyncLog.log( - `SyncWrapperService: Upload complete. uploaded=${uploadResult.uploadedCount}, piggybacked=${uploadResult.piggybackedOps.length}`, + `SyncWrapperService: Upload complete. uploaded=${uploadResult.uploadedCount}, piggybacked=${uploadResult.piggybackedOpsCount}`, ); } // If upload was cancelled (piggybacked SYNC_IMPORT conflict dialog), skip LWW re-upload - if (uploadResult?.cancelled) { + if (uploadResult.kind === 'cancelled') { SyncLog.log( 'SyncWrapperService: Upload cancelled by user (piggybacked SYNC_IMPORT). Skipping LWW re-upload.', ); @@ -359,10 +357,12 @@ export class SyncWrapperService { } // 3. If LWW created local-win ops, upload them (with retry limit to prevent infinite loops) + const downloadLwwOps = + downloadResult.kind === 'ops_processed' ? downloadResult.localWinOpsCreated : 0; + const uploadLwwOps = + uploadResult.kind === 'completed' ? uploadResult.localWinOpsCreated : 0; let lwwRetries = 0; - let pendingLwwOps = - (downloadResult.localWinOpsCreated ?? 0) + - (uploadResult?.localWinOpsCreated ?? 0); + let pendingLwwOps = downloadLwwOps + uploadLwwOps; while (pendingLwwOps > 0 && lwwRetries < MAX_LWW_REUPLOAD_RETRIES) { lwwRetries++; SyncLog.log( @@ -371,7 +371,8 @@ export class SyncWrapperService { ); const reuploadResult = await this._opLogSyncService.uploadPendingOps(syncCapableProvider); - pendingLwwOps = reuploadResult?.localWinOpsCreated ?? 0; + pendingLwwOps = + reuploadResult.kind === 'completed' ? reuploadResult.localWinOpsCreated : 0; } if (pendingLwwOps > 0) { SyncLog.warn( @@ -383,22 +384,9 @@ export class SyncWrapperService { return SyncStatus.UpdateRemote; } - // 4. Check for permanent rejection failures - these are critical failures that should - // NOT be reported as "in sync" to the user. - // - // IMPORTANT: We check permanentRejectionCount, NOT rejectedCount. - // - Transient errors (INTERNAL_ERROR) will retry on next sync - // - Resolved conflicts (CONFLICT_CONCURRENT merged successfully) are not failures - // - Duplicate operations (already synced) are not failures - // Only permanent rejections (VALIDATION_ERROR, payload too large) should block success. - // - // Fall back to rejectedCount for backward compatibility if permanentRejectionCount is undefined. - const permanentFailures = - uploadResult?.permanentRejectionCount ?? uploadResult?.rejectedCount ?? 0; - - if (permanentFailures > 0) { - // Check for payload errors first (special handling with alert) - const hasPayloadError = uploadResult?.rejectedOps?.some( + // 4. Check for permanent rejection failures + if (uploadResult.kind === 'completed' && uploadResult.permanentRejectionCount > 0) { + const hasPayloadError = uploadResult.rejectedOps.some( (r) => r.error?.includes('Payload too complex') || r.error?.includes('Payload too large'), @@ -407,25 +395,22 @@ export class SyncWrapperService { if (hasPayloadError) { SyncLog.err( 'SyncWrapperService: Upload rejected - payload too large/complex', - uploadResult?.rejectedOps, + uploadResult.rejectedOps, ); this._providerManager.setSyncStatus('ERROR'); - // Use alertDialog for maximum visibility - this is a critical error alertDialog(this._translateService.instant(T.F.SYNC.S.ERROR_PAYLOAD_TOO_LARGE)); return 'HANDLED_ERROR'; } - // Other permanent rejections - still shouldn't claim success SyncLog.err( - `SyncWrapperService: Upload had ${permanentFailures} permanent rejection(s), not marking as IN_SYNC`, - uploadResult?.rejectedOps, + `SyncWrapperService: Upload had ${uploadResult.permanentRejectionCount} permanent rejection(s), not marking as IN_SYNC`, + uploadResult.rejectedOps, ); this._providerManager.setSyncStatus('ERROR'); return 'HANDLED_ERROR'; } // Mark as in-sync for all providers after successful sync - // This indicates the sync operation completed successfully this._providerManager.setSyncStatus('IN_SYNC'); SyncLog.log('SyncWrapperService: Sync complete, status=IN_SYNC'); return SyncStatus.InSync; diff --git a/src/app/op-log/core/types/sync-results.types.ts b/src/app/op-log/core/types/sync-results.types.ts index ae019817dc..c63ca26fdc 100644 --- a/src/app/op-log/core/types/sync-results.types.ts +++ b/src/app/op-log/core/types/sync-results.types.ts @@ -157,3 +157,71 @@ export interface DownloadResultForRejection { export type DownloadCallback = (options?: { forceFromSeq0?: boolean; }) => Promise; + +// ───────────────────────────────────────────────────────────────────────────── +// Orchestrator-level result types (discriminated unions) +// +// These replace the ad-hoc return types from OperationLogSyncService methods. +// Transport-level types (DownloadResult, UploadResult) remain unchanged. +// ───────────────────────────────────────────────────────────────────────────── + +/** + * Outcome of OperationLogSyncService.downloadRemoteOps(). + * + * Each variant represents a distinct terminal state — callers switch on `kind` + * instead of checking boolean flag combinations. + */ +export type DownloadOutcome = + | { + /** Server was empty/reset — a SYNC_IMPORT was created. Caller must upload. */ + kind: 'server_migration_handled'; + } + | { + /** No new operations on server. */ + kind: 'no_new_ops'; + allOpClocks?: VectorClock[]; + snapshotVectorClock?: VectorClock; + } + | { + /** Incremental ops were downloaded and processed. */ + kind: 'ops_processed'; + newOpsCount: number; + localWinOpsCreated: number; + allOpClocks?: VectorClock[]; + snapshotVectorClock?: VectorClock; + } + | { + /** File-based snapshot was hydrated (fresh download from file provider). */ + kind: 'snapshot_hydrated'; + allOpClocks?: VectorClock[]; + snapshotVectorClock?: VectorClock; + } + | { + /** User cancelled a SYNC_IMPORT conflict dialog. */ + kind: 'cancelled'; + }; + +/** + * Outcome of OperationLogSyncService.uploadPendingOps(). + * + * Each variant represents a distinct terminal state. + */ +export type UploadOutcome = + | { + /** Upload was blocked because this is a fresh client with no history. */ + kind: 'blocked_fresh_client'; + } + | { + /** Upload completed (ops may have been accepted, rejected, or piggybacked). */ + kind: 'completed'; + uploadedCount: number; + piggybackedOpsCount: number; + localWinOpsCreated: number; + permanentRejectionCount: number; + hasMorePiggyback: boolean; + rejectedOps: RejectedOpInfo[]; + } + | { + /** User cancelled a piggybacked SYNC_IMPORT conflict dialog. */ + kind: 'cancelled'; + }; diff --git a/src/app/op-log/sync/immediate-upload.service.spec.ts b/src/app/op-log/sync/immediate-upload.service.spec.ts index 9c0591cc13..ff83705446 100644 --- a/src/app/op-log/sync/immediate-upload.service.spec.ts +++ b/src/app/op-log/sync/immediate-upload.service.spec.ts @@ -2,11 +2,11 @@ import { TestBed, fakeAsync, tick } from '@angular/core/testing'; import { ImmediateUploadService } from './immediate-upload.service'; import { SyncProviderManager } from '../sync-providers/provider-manager.service'; import { OperationLogSyncService } from './operation-log-sync.service'; -import { ActionType, Operation, OpType } from '../core/operation.types'; import { SyncProviderId } from '../sync-providers/provider.const'; import { DataInitStateService } from '../../core/data-init/data-init-state.service'; import { SyncWrapperService } from '../../imex/sync/sync-wrapper.service'; import { BehaviorSubject } from 'rxjs'; +import { RejectedOpInfo } from '../core/types/sync-results.types'; describe('ImmediateUploadService', () => { let service: ImmediateUploadService; @@ -16,17 +16,32 @@ describe('ImmediateUploadService', () => { let mockSyncWrapperService: { isEncryptionOperationInProgress: boolean }; let mockProvider: any; - const createMockOp = (id: string): Operation => ({ - id, - clientId: 'clientA', - actionType: '[Task] Add' as ActionType, - opType: OpType.Create, - entityType: 'TASK', - entityId: `task-${id}`, - payload: {}, - vectorClock: { clientA: 1 }, - timestamp: Date.now(), - schemaVersion: 1, + const completedResult = ( + overrides: Partial<{ + uploadedCount: number; + piggybackedOpsCount: number; + localWinOpsCreated: number; + permanentRejectionCount: number; + hasMorePiggyback: boolean; + rejectedOps: RejectedOpInfo[]; + }> = {}, + ): { + kind: 'completed'; + uploadedCount: number; + piggybackedOpsCount: number; + localWinOpsCreated: number; + permanentRejectionCount: number; + hasMorePiggyback: boolean; + rejectedOps: RejectedOpInfo[]; + } => ({ + kind: 'completed', + uploadedCount: 0, + piggybackedOpsCount: 0, + localWinOpsCreated: 0, + permanentRejectionCount: 0, + hasMorePiggyback: false, + rejectedOps: [], + ...overrides, }); beforeEach(() => { @@ -86,12 +101,7 @@ describe('ImmediateUploadService', () => { describe('checkmark (IN_SYNC) behavior', () => { it('should show checkmark when upload succeeds and no piggybacked ops', fakeAsync(() => { mockSyncService.uploadPendingOps.and.returnValue( - Promise.resolve({ - uploadedCount: 3, - rejectedCount: 0, - piggybackedOps: [], // No remote ops = confirmed in sync - rejectedOps: [], - }), + Promise.resolve(completedResult({ uploadedCount: 3 })), ); service.initialize(); @@ -102,14 +112,8 @@ describe('ImmediateUploadService', () => { })); it('should NOT show checkmark when piggybacked ops exist', fakeAsync(() => { - const piggybackedOp = createMockOp('piggybacked-1'); mockSyncService.uploadPendingOps.and.returnValue( - Promise.resolve({ - uploadedCount: 2, - rejectedCount: 0, - piggybackedOps: [piggybackedOp], // Remote ops exist = may not be fully in sync - rejectedOps: [], - }), + Promise.resolve(completedResult({ uploadedCount: 2, piggybackedOpsCount: 1 })), ); service.initialize(); @@ -123,12 +127,7 @@ describe('ImmediateUploadService', () => { it('should NOT show checkmark when nothing was uploaded', fakeAsync(() => { mockSyncService.uploadPendingOps.and.returnValue( - Promise.resolve({ - uploadedCount: 0, // Nothing uploaded - rejectedCount: 0, - piggybackedOps: [], - rejectedOps: [], - }), + Promise.resolve(completedResult({ uploadedCount: 0 })), ); service.initialize(); @@ -154,18 +153,8 @@ describe('ImmediateUploadService', () => { }); it('should NOT show checkmark when piggybacked ops exist (multiple)', fakeAsync(() => { - const piggybackedOps = [ - createMockOp('piggybacked-1'), - createMockOp('piggybacked-2'), - createMockOp('piggybacked-3'), - ]; mockSyncService.uploadPendingOps.and.returnValue( - Promise.resolve({ - uploadedCount: 5, - rejectedCount: 0, - piggybackedOps, - rejectedOps: [], - }), + Promise.resolve(completedResult({ uploadedCount: 5, piggybackedOpsCount: 3 })), ); service.initialize(); @@ -182,12 +171,7 @@ describe('ImmediateUploadService', () => { // Need to re-create the mock with isSyncInProgress = true Object.defineProperty(mockProviderManager, 'isSyncInProgress', { value: true }); mockSyncService.uploadPendingOps.and.returnValue( - Promise.resolve({ - uploadedCount: 1, - rejectedCount: 0, - piggybackedOps: [], - rejectedOps: [], - }), + Promise.resolve(completedResult({ uploadedCount: 1 })), ); service.initialize(); @@ -201,12 +185,7 @@ describe('ImmediateUploadService', () => { // Set encryption operation in progress (e.g., password change) mockSyncWrapperService.isEncryptionOperationInProgress = true; mockSyncService.uploadPendingOps.and.returnValue( - Promise.resolve({ - uploadedCount: 1, - rejectedCount: 0, - piggybackedOps: [], - rejectedOps: [], - }), + Promise.resolve(completedResult({ uploadedCount: 1 })), ); service.initialize(); @@ -218,14 +197,16 @@ describe('ImmediateUploadService', () => { it('should handle fresh client (syncService returns null)', fakeAsync(() => { // Fresh client handling is now done inside syncService.uploadPendingOps() - // which returns null for fresh clients - mockSyncService.uploadPendingOps.and.returnValue(Promise.resolve(null)); + // which returns { kind: 'blocked_fresh_client' } for fresh clients + mockSyncService.uploadPendingOps.and.returnValue( + Promise.resolve({ kind: 'blocked_fresh_client' as const }), + ); service.initialize(); service.trigger(); tick(2100); - // Upload was called, but returned null - no checkmark shown + // Upload was called, but returned blocked_fresh_client - no checkmark shown expect(mockSyncService.uploadPendingOps).toHaveBeenCalled(); expect(mockProviderManager.setSyncStatus).not.toHaveBeenCalled(); })); @@ -233,12 +214,7 @@ describe('ImmediateUploadService', () => { it('should skip upload when provider is not ready', fakeAsync(() => { mockProvider.isReady.and.returnValue(Promise.resolve(false)); mockSyncService.uploadPendingOps.and.returnValue( - Promise.resolve({ - uploadedCount: 1, - rejectedCount: 0, - piggybackedOps: [], - rejectedOps: [], - }), + Promise.resolve(completedResult({ uploadedCount: 1 })), ); service.initialize(); @@ -252,12 +228,7 @@ describe('ImmediateUploadService', () => { // File-based providers use periodic sync, not immediate upload mockProvider.id = SyncProviderId.Dropbox; mockSyncService.uploadPendingOps.and.returnValue( - Promise.resolve({ - uploadedCount: 1, - rejectedCount: 0, - piggybackedOps: [], - rejectedOps: [], - }), + Promise.resolve(completedResult({ uploadedCount: 1 })), ); service.initialize(); @@ -270,12 +241,7 @@ describe('ImmediateUploadService', () => { it('should skip upload for WebDAV (file-based provider)', fakeAsync(() => { mockProvider.id = SyncProviderId.WebDAV; mockSyncService.uploadPendingOps.and.returnValue( - Promise.resolve({ - uploadedCount: 1, - rejectedCount: 0, - piggybackedOps: [], - rejectedOps: [], - }), + Promise.resolve(completedResult({ uploadedCount: 1 })), ); service.initialize(); @@ -288,12 +254,7 @@ describe('ImmediateUploadService', () => { it('should skip upload for LocalFile (file-based provider)', fakeAsync(() => { mockProvider.id = SyncProviderId.LocalFile; mockSyncService.uploadPendingOps.and.returnValue( - Promise.resolve({ - uploadedCount: 1, - rejectedCount: 0, - piggybackedOps: [], - rejectedOps: [], - }), + Promise.resolve(completedResult({ uploadedCount: 1 })), ); service.initialize(); @@ -307,12 +268,7 @@ describe('ImmediateUploadService', () => { describe('debouncing', () => { it('should debounce rapid triggers into single upload', fakeAsync(() => { mockSyncService.uploadPendingOps.and.returnValue( - Promise.resolve({ - uploadedCount: 1, - rejectedCount: 0, - piggybackedOps: [], - rejectedOps: [], - }), + Promise.resolve(completedResult({ uploadedCount: 1 })), ); service.initialize(); @@ -334,12 +290,7 @@ describe('ImmediateUploadService', () => { describe('constructor initialization', () => { it('should auto-initialize when data is loaded', fakeAsync(() => { mockSyncService.uploadPendingOps.and.returnValue( - Promise.resolve({ - uploadedCount: 1, - rejectedCount: 0, - piggybackedOps: [], - rejectedOps: [], - }), + Promise.resolve(completedResult({ uploadedCount: 1 })), ); // Simulate data loading complete @@ -355,12 +306,7 @@ describe('ImmediateUploadService', () => { it('should not auto-initialize before data is loaded', fakeAsync(() => { mockSyncService.uploadPendingOps.and.returnValue( - Promise.resolve({ - uploadedCount: 1, - rejectedCount: 0, - piggybackedOps: [], - rejectedOps: [], - }), + Promise.resolve(completedResult({ uploadedCount: 1 })), ); // Data not loaded (still false) @@ -373,12 +319,7 @@ describe('ImmediateUploadService', () => { it('should queue triggers before initialization and replay them when initialized', fakeAsync(() => { mockSyncService.uploadPendingOps.and.returnValue( - Promise.resolve({ - uploadedCount: 1, - rejectedCount: 0, - piggybackedOps: [], - rejectedOps: [], - }), + Promise.resolve(completedResult({ uploadedCount: 1 })), ); // Trigger multiple times before initialization (data not loaded) diff --git a/src/app/op-log/sync/immediate-upload.service.ts b/src/app/op-log/sync/immediate-upload.service.ts index 2167cf51ef..02d1d69bca 100644 --- a/src/app/op-log/sync/immediate-upload.service.ts +++ b/src/app/op-log/sync/immediate-upload.service.ts @@ -182,27 +182,24 @@ export class ImmediateUploadService implements OnDestroy { // Use sync service's uploadPendingOps which includes migration detection callback. // This ensures SYNC_IMPORT is created when switching to a new/empty server. - // Returns null if fresh client (blocked from upload). const result = await this._syncService.uploadPendingOps(syncCapableProvider); - if (!result) { - OpLog.verbose('ImmediateUploadService: Upload returned null (fresh client)'); + if (result.kind === 'blocked_fresh_client') { + OpLog.verbose('ImmediateUploadService: Upload blocked (fresh client)'); return; } - // If cancelled (piggybacked SYNC_IMPORT conflict dialog), skip post-upload logic - if (result.cancelled) { + if (result.kind === 'cancelled') { OpLog.verbose( 'ImmediateUploadService: Upload cancelled (piggybacked SYNC_IMPORT conflict)', ); return; } - // Note: piggybacked ops and rejected ops are already handled by _syncService.uploadPendingOps() - // We just need to handle the sync status here. + // result.kind === 'completed' from here // If LWW local-wins created new update ops from piggybacked ops, // do a follow-up upload to push them to the server immediately - if ((result.localWinOpsCreated ?? 0) > 0) { + if (result.localWinOpsCreated > 0) { OpLog.verbose( `ImmediateUploadService: LWW created ${result.localWinOpsCreated} local-win op(s), re-uploading`, ); @@ -211,17 +208,17 @@ export class ImmediateUploadService implements OnDestroy { // Don't show checkmark when piggybacked ops exist - there may be more // remote ops pending. Let normal sync cycle confirm full sync state. - if (result.piggybackedOps.length > 0) { + if (result.piggybackedOpsCount > 0) { OpLog.verbose( `ImmediateUploadService: Uploaded ${result.uploadedCount} ops, ` + - `processed ${result.piggybackedOps.length} piggybacked (checkmark deferred)`, + `processed ${result.piggybackedOpsCount} piggybacked (checkmark deferred)`, ); return; } // Show checkmark ONLY when server confirms no pending remote ops // (empty piggybackedOps means we're confirmed in sync) - if (result.uploadedCount > 0 || (result.localWinOpsCreated ?? 0) > 0) { + if (result.uploadedCount > 0 || result.localWinOpsCreated > 0) { this._providerManager.setSyncStatus('IN_SYNC'); OpLog.verbose( `ImmediateUploadService: Uploaded ${result.uploadedCount} ops, confirmed in sync`, diff --git a/src/app/op-log/sync/operation-log-sync.service.spec.ts b/src/app/op-log/sync/operation-log-sync.service.spec.ts index a22a728008..8d40745f04 100644 --- a/src/app/op-log/sync/operation-log-sync.service.spec.ts +++ b/src/app/op-log/sync/operation-log-sync.service.spec.ts @@ -267,7 +267,10 @@ describe('OperationLogSyncService', () => { const result = await service.uploadPendingOps(mockProvider); - expect(result?.localWinOpsCreated).toBe(0); + expect(result.kind).toBe('completed'); + if (result.kind === 'completed') { + expect(result.localWinOpsCreated).toBe(0); + } }); it('should return localWinOpsCreated count from piggybacked ops processing', async () => { @@ -309,7 +312,10 @@ describe('OperationLogSyncService', () => { const result = await service.uploadPendingOps(mockProvider); - expect(result?.localWinOpsCreated).toBe(2); + expect(result.kind).toBe('completed'); + if (result.kind === 'completed') { + expect(result.localWinOpsCreated).toBe(2); + } }); describe('rejected ops handling delegation', () => { @@ -373,9 +379,7 @@ describe('OperationLogSyncService', () => { const downloadSpy = spyOn(service, 'downloadRemoteOps').and.returnValue( Promise.resolve({ - serverMigrationHandled: false, - localWinOpsCreated: 0, - newOpsCount: 0, + kind: 'no_new_ops' as const, }), ); @@ -439,7 +443,10 @@ describe('OperationLogSyncService', () => { const result = await service.uploadPendingOps(mockProvider); // Total should be 2 + 3 = 5 - expect(result?.localWinOpsCreated).toBe(5); + expect(result.kind).toBe('completed'); + if (result.kind === 'completed') { + expect(result.localWinOpsCreated).toBe(5); + } }); it('should not call handleRejectedOps if processRemoteOps throws', async () => { @@ -518,8 +525,7 @@ describe('OperationLogSyncService', () => { const result = await service.downloadRemoteOps(mockProvider); - expect(result.localWinOpsCreated).toBe(0); - expect(result.newOpsCount).toBe(0); + expect(result.kind).toBe('no_new_ops'); }); it('should return localWinOpsCreated count and newOpsCount from processing remote ops', async () => { @@ -563,8 +569,11 @@ describe('OperationLogSyncService', () => { const result = await service.downloadRemoteOps(mockProvider); - expect(result.localWinOpsCreated).toBe(1); - expect(result.newOpsCount).toBe(1); + expect(result.kind).toBe('ops_processed'); + if (result.kind === 'ops_processed') { + expect(result.localWinOpsCreated).toBe(1); + expect(result.newOpsCount).toBe(1); + } }); it('should return localWinOpsCreated: 0 and newOpsCount: 0 on server migration', async () => { @@ -587,9 +596,7 @@ describe('OperationLogSyncService', () => { const result = await service.downloadRemoteOps(mockProvider); - expect(result.serverMigrationHandled).toBe(true); - expect(result.localWinOpsCreated).toBe(0); - expect(result.newOpsCount).toBe(0); + expect(result.kind).toBe('server_migration_handled'); }); describe('lastServerSeq persistence', () => { @@ -1736,7 +1743,7 @@ describe('OperationLogSyncService', () => { mockProvider, { syncImportReason: 'SERVER_MIGRATION' }, ); - expect(result.serverMigrationHandled).toBe(true); + expect(result.kind).toBe('server_migration_handled'); }); it('should NOT create SYNC_IMPORT when fresh client has no meaningful data on empty server', async () => { @@ -1764,7 +1771,7 @@ describe('OperationLogSyncService', () => { const result = await service.downloadRemoteOps(mockProvider); expect(serverMigrationServiceSpy.handleServerMigration).not.toHaveBeenCalled(); - expect(result.serverMigrationHandled).toBe(false); + expect(result.kind).not.toBe('server_migration_handled'); }); it('should NOT create SYNC_IMPORT when server is not empty (latestServerSeq > 0)', async () => { @@ -1792,7 +1799,7 @@ describe('OperationLogSyncService', () => { const result = await service.downloadRemoteOps(mockProvider); expect(serverMigrationServiceSpy.handleServerMigration).not.toHaveBeenCalled(); - expect(result.serverMigrationHandled).toBe(false); + expect(result.kind).not.toBe('server_migration_handled'); }); it('should NOT create SYNC_IMPORT when client is not fresh (has op-log history)', async () => { @@ -1830,7 +1837,7 @@ describe('OperationLogSyncService', () => { // Should NOT call handleServerMigration - client is not fresh expect(serverMigrationServiceSpy.handleServerMigration).not.toHaveBeenCalled(); - expect(result.serverMigrationHandled).toBe(false); + expect(result.kind).not.toBe('server_migration_handled'); }); }); @@ -1907,7 +1914,7 @@ describe('OperationLogSyncService', () => { syncImportReason: 'SERVER_MIGRATION', }), ); - expect(result?.cancelled).toBe(true); + expect(result.kind).toBe('cancelled'); }); it('should show conflict dialog when piggybacked ops contain SYNC_IMPORT and client has meaningful data', async () => { @@ -1948,7 +1955,7 @@ describe('OperationLogSyncService', () => { const result = await service.uploadPendingOps(mockProvider); expect(syncImportConflictDialogServiceSpy.showConflictDialog).toHaveBeenCalled(); - expect(result?.cancelled).toBe(true); + expect(result.kind).toBe('cancelled'); }); it('should process piggybacked SYNC_IMPORT silently when no meaningful local data', async () => { @@ -1994,7 +2001,7 @@ describe('OperationLogSyncService', () => { expect(remoteOpsProcessingServiceSpy.processRemoteOps).toHaveBeenCalledWith([ piggybackedSyncImport, ]); - expect(result?.cancelled).toBeUndefined(); + expect(result.kind).not.toBe('cancelled'); }); it('should process piggybacked ops normally when no SYNC_IMPORT present', async () => { @@ -2034,7 +2041,7 @@ describe('OperationLogSyncService', () => { expect(remoteOpsProcessingServiceSpy.processRemoteOps).toHaveBeenCalledWith([ piggybackedOp, ]); - expect(result?.cancelled).toBeUndefined(); + expect(result.kind).not.toBe('cancelled'); }); it('should call forceUploadLocalState when user chooses USE_LOCAL', async () => { diff --git a/src/app/op-log/sync/operation-log-sync.service.ts b/src/app/op-log/sync/operation-log-sync.service.ts index 214dd673e3..8ac24d05df 100644 --- a/src/app/op-log/sync/operation-log-sync.service.ts +++ b/src/app/op-log/sync/operation-log-sync.service.ts @@ -2,19 +2,15 @@ import { inject, Injectable } from '@angular/core'; import { Store } from '@ngrx/store'; import { TranslateService } from '@ngx-translate/core'; import { OperationLogStoreService } from '../persistence/operation-log-store.service'; -import { - OperationLogEntry, - OpType, - VectorClock, - FULL_STATE_OP_TYPES, -} from '../core/operation.types'; +import { OperationLogEntry, OpType, FULL_STATE_OP_TYPES } from '../core/operation.types'; import { OpLog } from '../../core/log'; import { OperationSyncCapable, SyncProviderBase, } from '../sync-providers/provider.interface'; import { SyncProviderId } from '../sync-providers/provider.const'; -import { OperationLogUploadService, UploadResult } from './operation-log-upload.service'; +import { OperationLogUploadService } from './operation-log-upload.service'; +import { DownloadOutcome, UploadOutcome } from '../core/types/sync-results.types'; import { OperationLogDownloadService } from './operation-log-download.service'; import { SnackService } from '../../core/snack/snack.service'; import { T } from '../../t.const'; @@ -248,7 +244,7 @@ export class OperationLogSyncService { async uploadPendingOps( syncProvider: OperationSyncCapable, options?: { skipPiggybackProcessing?: boolean; skipServerMigrationCheck?: boolean }, - ): Promise { + ): Promise { // CRITICAL: Ensure all pending write operations have completed before uploading. // The effect that writes operations uses concatMap for sequential processing, // but if sync is triggered before all operations are written to IndexedDB, @@ -264,7 +260,7 @@ export class OperationLogSyncService { 'OperationLogSyncService: Upload blocked - this is a fresh client with no history. ' + 'Download remote data first before uploading.', ); - return null; + return { kind: 'blocked_fresh_client' }; } // SERVER MIGRATION CHECK: Passed as callback to execute INSIDE the upload lock. @@ -332,11 +328,18 @@ export class OperationLogSyncService { }, 'OperationLogSyncService (piggybacked SYNC_IMPORT)', ); + if (resolution === 'CANCEL') { + return { kind: 'cancelled' }; + } + // USE_LOCAL or USE_REMOTE was handled — report as completed with no further work return { - ...result, + kind: 'completed', + uploadedCount: result.uploadedCount, + piggybackedOpsCount: result.piggybackedOps.length, localWinOpsCreated: 0, permanentRejectionCount: 0, - cancelled: resolution === 'CANCEL', + hasMorePiggyback: false, + rejectedOps: [], }; } } @@ -354,10 +357,29 @@ export class OperationLogSyncService { // // NOTE: This must NOT run after a SYNC_IMPORT conflict dialog resolution (USE_LOCAL, // USE_REMOTE, CANCEL) — those paths return early above to avoid stale rejection handling. - const downloadCallback = (downloadOptions?: { + const downloadCallback = async (downloadOptions?: { forceFromSeq0?: boolean; - }): Promise => - this.downloadRemoteOps(syncProvider, downloadOptions); + }): Promise => { + const outcome = await this.downloadRemoteOps(syncProvider, downloadOptions); + switch (outcome.kind) { + case 'ops_processed': + return { + newOpsCount: outcome.newOpsCount, + allOpClocks: outcome.allOpClocks, + snapshotVectorClock: outcome.snapshotVectorClock, + }; + case 'no_new_ops': + case 'snapshot_hydrated': + return { + newOpsCount: 0, + allOpClocks: outcome.allOpClocks, + snapshotVectorClock: outcome.snapshotVectorClock, + }; + case 'server_migration_handled': + case 'cancelled': + return { newOpsCount: 0 }; + } + }; try { rejectionResult = await this.rejectedOpsHandlerService.handleRejectedOps( result.rejectedOps, @@ -379,9 +401,13 @@ export class OperationLogSyncService { ); return { - ...result, + kind: 'completed', + uploadedCount: result.uploadedCount, + piggybackedOpsCount: result.piggybackedOps.length, localWinOpsCreated, permanentRejectionCount: rejectionResult.permanentRejectionCount, + hasMorePiggyback: result.hasMorePiggyback ?? false, + rejectedOps: result.rejectedOps, }; } @@ -401,14 +427,7 @@ export class OperationLogSyncService { async downloadRemoteOps( syncProvider: OperationSyncCapable, options?: { forceFromSeq0?: boolean }, - ): Promise<{ - serverMigrationHandled: boolean; - localWinOpsCreated: number; - newOpsCount: number; - allOpClocks?: VectorClock[]; - snapshotVectorClock?: VectorClock; - cancelled?: boolean; - }> { + ): Promise { const result = await this.downloadService.downloadRemoteOps(syncProvider, options); // Server migration detected: gap on empty server @@ -419,8 +438,7 @@ export class OperationLogSyncService { if (result.latestServerSeq !== undefined) { await syncProvider.setLastServerSeq(result.latestServerSeq); } - // Return with flag indicating migration was handled - caller should upload the SYNC_IMPORT - return { serverMigrationHandled: true, localWinOpsCreated: 0, newOpsCount: 0 }; + return { kind: 'server_migration_handled' }; } // FILE-BASED SYNC: Handle full state snapshot from fresh download @@ -499,11 +517,7 @@ export class OperationLogSyncService { this.snackService.open({ msg: T.F.SYNC.S.FRESH_CLIENT_SYNC_CANCELLED, }); - return { - serverMigrationHandled: false, - localWinOpsCreated: 0, - newOpsCount: 0, - }; + return { kind: 'cancelled' }; } OpLog.normal( @@ -549,9 +563,7 @@ export class OperationLogSyncService { OpLog.normal('OperationLogSyncService: Snapshot hydration complete.'); return { - serverMigrationHandled: false, - localWinOpsCreated: 0, - newOpsCount: 0, // Snapshot applied, not incremental ops + kind: 'snapshot_hydrated', allOpClocks: result.allOpClocks, snapshotVectorClock: result.snapshotVectorClock, }; @@ -577,7 +589,7 @@ export class OperationLogSyncService { }); // After SYNC_IMPORT is created, isWhollyFreshClient() returns false // and upload phase will proceed normally. - return { serverMigrationHandled: true, localWinOpsCreated: 0, newOpsCount: 0 }; + return { kind: 'server_migration_handled' }; } } @@ -591,12 +603,8 @@ export class OperationLogSyncService { await syncProvider.setLastServerSeq(result.latestServerSeq); } return { - serverMigrationHandled: false, - localWinOpsCreated: 0, - newOpsCount: 0, - // Include all op clocks from forced download (even though no new ops) + kind: 'no_new_ops', allOpClocks: result.allOpClocks, - // Include snapshot vector clock for superseded op resolution snapshotVectorClock: result.snapshotVectorClock, }; } @@ -627,7 +635,7 @@ export class OperationLogSyncService { this.snackService.open({ msg: T.F.SYNC.S.FRESH_CLIENT_SYNC_CANCELLED, }); - return { serverMigrationHandled: false, localWinOpsCreated: 0, newOpsCount: 0 }; + return { kind: 'cancelled' }; } OpLog.normal( @@ -676,12 +684,10 @@ export class OperationLogSyncService { }, 'OperationLogSyncService (incoming SYNC_IMPORT)', ); - return { - serverMigrationHandled: false, - localWinOpsCreated: 0, - newOpsCount: 0, - cancelled: resolution === 'CANCEL', - }; + if (resolution === 'CANCEL') { + return { kind: 'cancelled' }; + } + return { kind: 'no_new_ops' }; } } @@ -718,12 +724,10 @@ export class OperationLogSyncService { }, 'OperationLogSyncService (local SYNC_IMPORT filters remote)', ); - return { - serverMigrationHandled: false, - localWinOpsCreated: 0, - newOpsCount: 0, - cancelled: resolution === 'CANCEL', - }; + if (resolution === 'CANCEL') { + return { kind: 'cancelled' }; + } + return { kind: 'no_new_ops' }; } else if ( processResult.allOpsFilteredBySyncImport && processResult.filteredOpCount > 0 @@ -757,9 +761,9 @@ export class OperationLogSyncService { ); return { - serverMigrationHandled: false, - localWinOpsCreated: processResult.localWinOpsCreated, + kind: 'ops_processed', newOpsCount: result.newOps.length, + localWinOpsCreated: processResult.localWinOpsCreated, allOpClocks: result.allOpClocks, snapshotVectorClock: result.snapshotVectorClock, };