diff --git a/src/app/imex/sync/sync-wrapper.service.ts b/src/app/imex/sync/sync-wrapper.service.ts index 11048ab91..0d33be944 100644 --- a/src/app/imex/sync/sync-wrapper.service.ts +++ b/src/app/imex/sync/sync-wrapper.service.ts @@ -55,7 +55,6 @@ import { IS_ELECTRON } from '../../app.constants'; import { OperationLogStoreService } from '../../op-log/persistence/operation-log-store.service'; import { OperationLogSyncService } from '../../op-log/sync/operation-log-sync.service'; import { WrappedProviderService } from '../../op-log/sync-providers/wrapped-provider.service'; -import { isFileBasedProvider } from '../../op-log/sync/operation-sync.util'; /** * Converts LegacySyncProvider to SyncProviderId. @@ -237,15 +236,10 @@ export class SyncWrapperService { await this._opLogSyncService.uploadPendingOps(syncCapableProvider); } - // Mark as in-sync - only for SuperSync (API-based sync) - // File-based providers (Dropbox, WebDAV, LocalFile) don't get IN_SYNC status - // because we can't confirm real-time sync without server confirmation. - if (rawProvider && !isFileBasedProvider(rawProvider)) { - this._providerManager.setSyncStatus('IN_SYNC'); - SyncLog.log('SyncWrapperService: Sync complete, status=IN_SYNC'); - } else { - SyncLog.log('SyncWrapperService: Sync complete (file-based provider)'); - } + // 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; } catch (error) { SyncLog.err(error); diff --git a/src/app/op-log/sync/operation-log-download.service.spec.ts b/src/app/op-log/sync/operation-log-download.service.spec.ts index 5008a72e2..52287859d 100644 --- a/src/app/op-log/sync/operation-log-download.service.spec.ts +++ b/src/app/op-log/sync/operation-log-download.service.spec.ts @@ -22,6 +22,7 @@ describe('OperationLogDownloadService', () => { beforeEach(() => { mockOpLogStore = jasmine.createSpyObj('OperationLogStoreService', [ 'getAppliedOpIds', + 'hasSyncedOps', ]); mockLockService = jasmine.createSpyObj('LockService', ['request']); mockSnackService = jasmine.createSpyObj('SnackService', ['open']); @@ -36,6 +37,7 @@ describe('OperationLogDownloadService', () => { }, ); mockOpLogStore.getAppliedOpIds.and.returnValue(Promise.resolve(new Set())); + mockOpLogStore.hasSyncedOps.and.returnValue(Promise.resolve(false)); TestBed.configureTestingModule({ providers: [ @@ -1091,6 +1093,166 @@ describe('OperationLogDownloadService', () => { expect(mockApiProvider.setLastServerSeq).not.toHaveBeenCalled(); }); }); + + describe('server migration detection for file-based providers', () => { + it('should set needsFullStateUpload when empty server AND first connect AND has synced ops', async () => { + // Scenario: User switches from SuperSync to Dropbox + // - Server is empty (no sync-data.json file) + // - First time connecting (lastServerSeq === 0) + // - Client has previously synced ops (from SuperSync) + mockApiProvider.getLastServerSeq.and.returnValue(Promise.resolve(0)); + mockApiProvider.downloadOps.and.returnValue( + Promise.resolve({ + ops: [], + hasMore: false, + latestSeq: 0, // Empty server + }), + ); + // Client has synced ops from previous provider + mockOpLogStore.hasSyncedOps.and.returnValue(Promise.resolve(true)); + + const result = await service.downloadRemoteOps(mockApiProvider); + + expect(result.needsFullStateUpload).toBeTrue(); + expect(OpLog.warn).toHaveBeenCalledWith( + jasmine.stringContaining( + 'Server migration detected - empty server with synced ops', + ), + ); + }); + + it('should NOT set needsFullStateUpload when empty server AND first connect AND NO synced ops', async () => { + // Scenario: Fresh client connecting for the first time + // - Server is empty + // - First time connecting + // - No previous sync history (fresh client) + mockApiProvider.getLastServerSeq.and.returnValue(Promise.resolve(0)); + mockApiProvider.downloadOps.and.returnValue( + Promise.resolve({ + ops: [], + hasMore: false, + latestSeq: 0, + }), + ); + // Client has NO synced ops (fresh client) + mockOpLogStore.hasSyncedOps.and.returnValue(Promise.resolve(false)); + + const result = await service.downloadRemoteOps(mockApiProvider); + + expect(result.needsFullStateUpload).toBeFalsy(); + }); + + it('should NOT set needsFullStateUpload when server has data', async () => { + // Scenario: Joining existing sync group + // - Server has ops + // - Client may or may not have synced ops - irrelevant + mockApiProvider.getLastServerSeq.and.returnValue(Promise.resolve(0)); + mockApiProvider.downloadOps.and.returnValue( + Promise.resolve({ + ops: [ + { + serverSeq: 1, + receivedAt: Date.now(), + op: { + id: 'op-1', + clientId: 'c1', + actionType: '[Task] Add' as ActionType, + opType: OpType.Create, + entityType: 'TASK', + payload: {}, + vectorClock: {}, + timestamp: Date.now(), + schemaVersion: 1, + }, + }, + ], + hasMore: false, + latestSeq: 1, + }), + ); + mockOpLogStore.hasSyncedOps.and.returnValue(Promise.resolve(true)); + + const result = await service.downloadRemoteOps(mockApiProvider); + + expect(result.needsFullStateUpload).toBeFalsy(); + }); + + it('should NOT set needsFullStateUpload when already synced with this server (server has data)', async () => { + // Scenario: Regular sync with already-connected server + // - lastServerSeq > 0 means we've synced before + // - Server has data (latestSeq > 0) + mockApiProvider.getLastServerSeq.and.returnValue(Promise.resolve(50)); // Already synced + mockApiProvider.downloadOps.and.returnValue( + Promise.resolve({ + ops: [], + hasMore: false, + latestSeq: 50, // Server has data + }), + ); + mockOpLogStore.hasSyncedOps.and.returnValue(Promise.resolve(true)); + + const result = await service.downloadRemoteOps(mockApiProvider); + + expect(result.needsFullStateUpload).toBeFalsy(); + }); + + it('should set needsFullStateUpload when server was reset (previously synced, now empty)', async () => { + // Scenario: User previously synced with Dropbox, then cleared the Dropbox folder + // - lastServerSeq > 0 (had previous sync) + // - Server is now empty (user deleted sync-data.json) + // - Client has synced ops + // This is a server reset scenario - need full state upload + mockApiProvider.getLastServerSeq.and.returnValue(Promise.resolve(100)); // Had previous sync + mockApiProvider.downloadOps.and.returnValue( + Promise.resolve({ + ops: [], + hasMore: false, + latestSeq: 0, // Server is empty now + }), + ); + mockOpLogStore.hasSyncedOps.and.returnValue(Promise.resolve(true)); + + const result = await service.downloadRemoteOps(mockApiProvider); + + expect(result.needsFullStateUpload).toBeTrue(); + expect(OpLog.warn).toHaveBeenCalledWith( + jasmine.stringContaining( + 'Server migration detected - empty server with synced ops', + ), + ); + }); + + it('should NOT set needsFullStateUpload if gapDetected migration already triggered', async () => { + // Scenario: API-based provider with gap detection + // The gapDetected path should take precedence + mockApiProvider.getLastServerSeq.and.returnValue(Promise.resolve(100)); + mockApiProvider.downloadOps.and.returnValues( + // Gap detected on empty server - triggers migration via hasResetForGap path + Promise.resolve({ + ops: [], + hasMore: false, + latestSeq: 0, + gapDetected: true, + }), + // After reset, still empty + Promise.resolve({ + ops: [], + hasMore: false, + latestSeq: 0, + }), + ); + mockOpLogStore.hasSyncedOps.and.returnValue(Promise.resolve(true)); + + const result = await service.downloadRemoteOps(mockApiProvider); + + // Should have been triggered by the gapDetected path, not the alternative path + expect(result.needsFullStateUpload).toBeTrue(); + // The warning should mention the gap-based detection + expect(OpLog.warn).toHaveBeenCalledWith( + jasmine.stringContaining('gap on empty server'), + ); + }); + }); }); }); }); diff --git a/src/app/op-log/sync/operation-log-download.service.ts b/src/app/op-log/sync/operation-log-download.service.ts index d8d4a85f1..a1b18c84f 100644 --- a/src/app/op-log/sync/operation-log-download.service.ts +++ b/src/app/op-log/sync/operation-log-download.service.ts @@ -265,6 +265,34 @@ export class OperationLogDownloadService { ); } + // Alternative migration detection for file-based providers: + // When connecting to an empty server, check if the client has previously synced ops + // (from another provider like SuperSync OR from a previous sync with this provider). + // This handles: + // 1. Provider switch scenario (e.g., SuperSync → Dropbox) + // 2. Server reset scenario (e.g., user deleted sync-data.json in Dropbox) + // File-based providers don't return gapDetected, so we need this alternative check. + // NOTE: We check regardless of lastServerSeq because: + // - lastServerSeq might be non-zero from a previous sync with this provider + // - If server is empty but client has ops, we need to migrate regardless + OpLog.verbose( + `OperationLogDownloadService: Migration check - needsFullStateUpload=${needsFullStateUpload}, ` + + `allNewOps=${allNewOps.length}, finalLatestSeq=${finalLatestSeq}, lastServerSeq=${lastServerSeq}`, + ); + if (!needsFullStateUpload && allNewOps.length === 0 && finalLatestSeq === 0) { + const hasSyncedOps = await this.opLogStore.hasSyncedOps(); + OpLog.verbose( + `OperationLogDownloadService: Empty server detected, hasSyncedOps=${hasSyncedOps}`, + ); + if (hasSyncedOps) { + needsFullStateUpload = true; + OpLog.warn( + 'OperationLogDownloadService: Server migration detected - empty server with synced ops. ' + + 'Full state upload will be required.', + ); + } + } + OpLog.normal( `OperationLogDownloadService: Downloaded ${allNewOps.length} new operations via API.`, );