From b2eb62dfa54dedb9d1b103984f030f43a6d986bc Mon Sep 17 00:00:00 2001 From: Santo Shakil Date: Wed, 5 Aug 2026 20:47:05 +0600 Subject: [PATCH] fix(mobile): keep backup remainder from going negative (#29011) * recount the backup counters when a run starts * clear the error before the recount * create the cancel token before the recount --- .../backup/drift_backup.provider.dart | 16 ++- .../backup/drift_backup_provider_test.dart | 109 ++++++++++++++++++ 2 files changed, 122 insertions(+), 3 deletions(-) create mode 100644 mobile/test/providers/backup/drift_backup_provider_test.dart diff --git a/mobile/lib/providers/backup/drift_backup.provider.dart b/mobile/lib/providers/backup/drift_backup.provider.dart index b43fec023c..e841462d01 100644 --- a/mobile/lib/providers/backup/drift_backup.provider.dart +++ b/mobile/lib/providers/backup/drift_backup.provider.dart @@ -255,7 +255,7 @@ class DriftBackupNotifier extends StateNotifier { state = state.copyWith(isSyncing: isSyncing); } - Future startForegroundBackup(String userId) { + Future startForegroundBackup(String userId) async { // Cancel any existing backup before starting a new one if (_cancelToken != null) { stopForegroundBackup(); @@ -263,11 +263,17 @@ class DriftBackupNotifier extends StateNotifier { state = state.copyWith(error: BackupError.none); - _cancelToken = Completer(); + // A pause during the recount below nulls _cancelToken, so the run keeps its own reference. + final cancelToken = Completer(); + _cancelToken = cancelToken; + + // Re-baseline the counters against the same DB read that feeds this run's candidate list, + // otherwise a resume counts duplicate successes against the old baseline (#26215). + await getBackupStatus(userId); return _foregroundUploadService.uploadCandidates( userId, - _cancelToken!, + cancelToken, callbacks: UploadCallbacks( onProgress: _handleForegroundBackupProgress, onSuccess: _handleForegroundBackupSuccess, @@ -333,6 +339,10 @@ class DriftBackupNotifier extends StateNotifier { } void _handleForegroundBackupSuccess(String localAssetId, String remoteAssetId) { + if (!mounted) { + _logger.warning("Skip _handleForegroundBackupSuccess: notifier disposed"); + return; + } state = state.copyWith(backupCount: state.backupCount + 1, remainderCount: state.remainderCount - 1); _uploadSpeedManager.removeTask(localAssetId); diff --git a/mobile/test/providers/backup/drift_backup_provider_test.dart b/mobile/test/providers/backup/drift_backup_provider_test.dart new file mode 100644 index 0000000000..9a5f222301 --- /dev/null +++ b/mobile/test/providers/backup/drift_backup_provider_test.dart @@ -0,0 +1,109 @@ +import 'dart:async'; + +import 'package:flutter_test/flutter_test.dart'; +import 'package:immich_mobile/providers/backup/drift_backup.provider.dart'; +import 'package:immich_mobile/services/background_upload.service.dart'; +import 'package:immich_mobile/services/foreground_upload.service.dart'; +import 'package:immich_mobile/utils/upload_speed_calculator.dart'; +import 'package:mocktail/mocktail.dart'; + +class MockForegroundUploadService extends Mock implements ForegroundUploadService {} + +class MockBackgroundUploadService extends Mock implements BackgroundUploadService {} + +void main() { + late MockForegroundUploadService foregroundUploadService; + late MockBackgroundUploadService backgroundUploadService; + late DriftBackupNotifier notifier; + + setUpAll(() { + registerFallbackValue(Completer()); + registerFallbackValue(const UploadCallbacks()); + }); + + setUp(() { + foregroundUploadService = MockForegroundUploadService(); + backgroundUploadService = MockBackgroundUploadService(); + notifier = DriftBackupNotifier(foregroundUploadService, backgroundUploadService, UploadSpeedManager()); + addTearDown(() { + if (notifier.mounted) { + notifier.dispose(); + } + }); + }); + + void mockCounts({required int total, required int remainder, int processing = 0}) { + when( + () => foregroundUploadService.getBackupCounts('user-1'), + ).thenAnswer((_) async => (total: total, remainder: remainder, processing: processing)); + } + + // Drives a backup run so we can grab the onSuccess callback the notifier wires up. + Future startAndCaptureOnSuccess() async { + void Function(String, String)? onSuccess; + when(() => foregroundUploadService.uploadCandidates(any(), any(), callbacks: any(named: 'callbacks'))).thenAnswer(( + invocation, + ) async { + onSuccess = (invocation.namedArguments[#callbacks] as UploadCallbacks).onSuccess; + }); + await notifier.startForegroundBackup('user-1'); + return onSuccess!; + } + + group('foreground backup counts', () { + test('successes move one asset from remainder to backup', () async { + mockCounts(total: 25, remainder: 25); + final onSuccess = await startAndCaptureOnSuccess(); + + for (var i = 0; i < 10; i++) { + onSuccess('asset-$i', 'remote-$i'); + } + + expect(notifier.state.remainderCount, 15); + expect(notifier.state.backupCount, 10); + expect(notifier.state.backupCount + notifier.state.remainderCount, notifier.state.totalCount); + }); + + test('a duplicate success after pause and resume cannot go below zero', () async { + // #26215: app pauses mid-backup, sync has not recorded the upload yet, so the + // resumed run re-uploads the same asset and the server answers 200 duplicate. + // The start of each run re-baselines the counters from the DB, so the duplicate + // success is counted against a baseline that includes the asset again. + mockCounts(total: 1, remainder: 1); + + final firstRun = await startAndCaptureOnSuccess(); + expect(notifier.state.remainderCount, 1); + firstRun('asset-1', 'remote-1'); + expect(notifier.state.remainderCount, 0); + + notifier.stopForegroundBackup(); + + final resumedRun = await startAndCaptureOnSuccess(); + expect(notifier.state.remainderCount, 1); + verify(() => foregroundUploadService.getBackupCounts('user-1')).called(2); + + resumedRun('asset-1', 'remote-1'); + expect(notifier.state.remainderCount, 0); + expect(notifier.state.backupCount, 1); + }); + + test('a drifted counter state heals at run start', () async { + mockCounts(total: 91, remainder: 7); + notifier.state = notifier.state.copyWith(totalCount: 91, backupCount: 103, remainderCount: -12); + + await startAndCaptureOnSuccess(); + + expect(notifier.state.totalCount, 91); + expect(notifier.state.remainderCount, 7); + expect(notifier.state.backupCount, 84); + }); + + test('a late success after dispose does not throw', () async { + mockCounts(total: 2, remainder: 2); + final onSuccess = await startAndCaptureOnSuccess(); + notifier.dispose(); + + onSuccess('asset-1', 'remote-1'); + }); + }); +}