diff --git a/mobile/lib/domain/services/background_worker.service.dart b/mobile/lib/domain/services/background_worker.service.dart index d7e76f6c2..2980597d7 100644 --- a/mobile/lib/domain/services/background_worker.service.dart +++ b/mobile/lib/domain/services/background_worker.service.dart @@ -1,5 +1,4 @@ import 'dart:async'; -import 'dart:io'; import 'dart:ui'; import 'package:background_downloader/background_downloader.dart'; @@ -18,14 +17,11 @@ import 'package:immich_mobile/extensions/platform_extensions.dart'; import 'package:immich_mobile/infrastructure/repositories/settings.repository.dart'; import 'package:immich_mobile/platform/background_worker_api.g.dart'; import 'package:immich_mobile/platform/background_worker_lock_api.g.dart'; -import 'package:immich_mobile/providers/api.provider.dart'; import 'package:immich_mobile/providers/backup/backup.provider.dart'; -import 'package:immich_mobile/providers/infrastructure/db.provider.dart'; +import 'package:immich_mobile/providers/infrastructure/cancel.provider.dart'; import 'package:immich_mobile/providers/infrastructure/platform.provider.dart'; import 'package:immich_mobile/providers/infrastructure/sync.provider.dart'; import 'package:immich_mobile/providers/user.provider.dart'; -import 'package:immich_mobile/repositories/asset_media.repository.dart'; -import 'package:immich_mobile/repositories/permission.repository.dart'; import 'package:immich_mobile/services/api.service.dart'; import 'package:immich_mobile/services/auth.service.dart'; import 'package:immich_mobile/services/foreground_upload.service.dart'; @@ -74,39 +70,22 @@ class BackgroundWorkerBgService extends BackgroundWorkerFlutterApi { bool _isCleanedUp = false; - BackgroundWorkerBgService({required this._dataController, required ApiService apiService}) - : _backgroundHostApi = BackgroundWorkerBgHostApi() { + BackgroundWorkerBgService({ + required this._dataController, + required ApiService apiService, + List overrides = const [], + }) : _backgroundHostApi = BackgroundWorkerBgHostApi() { final ref = ProviderContainer( - overrides: Store.overrideWith(dataController: _dataController, apiService: apiService), + overrides: [ + cancellationProvider.overrideWithValue(_cancellationToken), + ...Store.overrideWith(dataController: _dataController, apiService: apiService), + ...overrides, + ], ); _ref = ref; - final db = ref.read(driftProvider); - _localSyncService = LocalSyncService( - localAlbumRepository: db.localAlbumRepository, - nativeSyncApi: ref.read(nativeSyncApiProvider), - trashedLocalAssetRepository: db.trashedLocalAssetRepository, - assetMediaRepository: ref.read(assetMediaRepositoryProvider), - permissionRepository: ref.read(permissionRepositoryProvider), - cancellation: _cancellationToken, - ); - _remoteSyncService = SyncStreamService( - syncApiRepository: ref.read(syncApiRepositoryProvider), - syncStreamRepository: db.syncStreamRepository, - localAssetRepository: db.localAssetRepository, - trashedLocalAssetRepository: db.trashedLocalAssetRepository, - assetMediaRepository: ref.read(assetMediaRepositoryProvider), - permissionRepository: ref.read(permissionRepositoryProvider), - syncMigrationRepository: db.syncMigrationRepository, - api: ref.read(apiServiceProvider), - cancellation: _cancellationToken, - ); - _hashService = HashService( - localAlbumRepository: db.localAlbumRepository, - localAssetRepository: db.localAssetRepository, - nativeSyncApi: ref.read(nativeSyncApiProvider), - trashedLocalAssetRepository: db.trashedLocalAssetRepository, - cancellation: _cancellationToken, - ); + _localSyncService = ref.read(localSyncServiceProvider); + _remoteSyncService = ref.read(syncStreamServiceProvider); + _hashService = ref.read(hashServiceProvider); BackgroundWorkerFlutterApi.setUp(this); } @@ -169,15 +148,17 @@ class BackgroundWorkerBgService extends BackgroundWorkerFlutterApi { // Run sync local, sync remote, hash and backup concurrently so the bg // refresh task (20s budget) can make progress on all four instead of - // racing them sequentially. Phases are independent at the data layer: - // hash and handle_backup read drift state and tolerate stale reads - // (server-side dedup catches the rare race). The single budget caps the + // racing them sequentially. Hash tolerates stale reads, but a new backup + // batch waits for the remote sync. The single budget caps the // whole batch; no phase needs its own timeout. + final remoteSync = _remoteSyncService.sync(); final all = Future.wait([ _localSyncService.sync(), - _remoteSyncService.sync(), + remoteSync, _hashService.hashAssets(), - _handleBackup(), + _handleBackup( + remoteSync: remoteSync.then((ok) => ok && !_cancellationToken.isCompleted).catchError((_) => false), + ), ]); if (budget != null) { await all.timeout( @@ -217,7 +198,7 @@ class BackgroundWorkerBgService extends BackgroundWorkerFlutterApi { return true; } - final backupFuture = _handleBackup(); + final backupFuture = _handleBackup(remoteSync: Future.value(true)); Timer? cancelTimer; if (backupTimeout != null) { cancelTimer = Timer(backupTimeout, () { @@ -294,7 +275,7 @@ class BackgroundWorkerBgService extends BackgroundWorkerFlutterApi { } } - Future _handleBackup() async { + Future _handleBackup({required Future remoteSync}) async { final needsRetry = await runZonedGuarded(() async { if (_isCleanedUp) { return false; @@ -311,8 +292,8 @@ class BackgroundWorkerBgService extends BackgroundWorkerFlutterApi { return false; } - if (Platform.isIOS) { - await _ref?.read(backupProvider.notifier).startBackupWithURLSession(currentUser.id); + if (CurrentPlatform.isIOS) { + await _ref?.read(backupProvider.notifier).startBackupWithURLSession(currentUser.id, remoteSync); return false; } diff --git a/mobile/lib/providers/backup/backup.provider.dart b/mobile/lib/providers/backup/backup.provider.dart index c9df035b2..d05c9a868 100644 --- a/mobile/lib/providers/backup/backup.provider.dart +++ b/mobile/lib/providers/backup/backup.provider.dart @@ -251,7 +251,7 @@ class BackupNotifier extends StateNotifier { _uploadSpeedManager.removeTask(localAssetId); } - Future startBackupWithURLSession(String userId) async { + Future startBackupWithURLSession(String userId, Future remoteSync) async { if (!mounted) { _logger.warning("Skip handleBackupResume (pre-call): notifier disposed"); return; @@ -266,6 +266,10 @@ class BackupNotifier extends StateNotifier { _logger.info("Found ${tasks.length} pending tasks"); if (tasks.isEmpty) { + if (!await remoteSync) { + _logger.warning("Remote sync did not complete successfully, skipping new upload"); + return; + } _logger.info("No pending tasks, starting new upload"); return _backgroundUploadService.uploadBackupCandidates(userId); } diff --git a/mobile/test/domain/services/background_worker_service_test.dart b/mobile/test/domain/services/background_worker_service_test.dart new file mode 100644 index 000000000..4388f4d57 --- /dev/null +++ b/mobile/test/domain/services/background_worker_service_test.dart @@ -0,0 +1,136 @@ +import 'dart:async'; + +import 'package:background_downloader/background_downloader.dart'; +import 'package:drift/drift.dart' as drift; +import 'package:drift/native.dart'; +import 'package:fake_async/fake_async.dart'; +import 'package:flutter/foundation.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:immich_mobile/constants/constants.dart'; +import 'package:immich_mobile/data/data_controller.dart'; +import 'package:immich_mobile/data/db/main/database.dart'; +import 'package:immich_mobile/domain/models/settings_key.dart'; +import 'package:immich_mobile/domain/services/background_worker.service.dart'; +import 'package:immich_mobile/domain/services/hash.service.dart'; +import 'package:immich_mobile/domain/services/local_sync.service.dart'; +import 'package:immich_mobile/domain/services/sync_stream.service.dart'; +import 'package:immich_mobile/infrastructure/repositories/settings.repository.dart'; +import 'package:immich_mobile/providers/infrastructure/sync.provider.dart'; +import 'package:immich_mobile/providers/user.provider.dart'; +import 'package:immich_mobile/services/background_upload.service.dart'; +import 'package:immich_mobile/services/foreground_upload.service.dart'; +import 'package:mocktail/mocktail.dart'; + +import '../../fixtures/user.stub.dart'; +import '../../service.mocks.dart'; + +class MockDataController extends Mock implements DataController {} + +class MockLocalSyncService extends Mock implements LocalSyncService {} + +class MockSyncStreamService extends Mock implements SyncStreamService {} + +class MockHashService extends Mock implements HashService {} + +void main() { + late BackgroundWorkerBgService sut; + late MockSyncStreamService mockRemoteSyncService; + late MockBackgroundUploadService mockBackgroundUploadService; + late Drift db; + + setUpAll(() async { + TestWidgetsFlutterBinding.ensureInitialized(); + debugDefaultTargetPlatformOverride = TargetPlatform.iOS; + + db = Drift(drift.DatabaseConnection(NativeDatabase.memory(), closeStreamsSynchronously: true)); + await SettingsRepository.ensureInitialized(db); + await SettingsRepository.instance.write(SettingsKey.backupEnabled, true); + }); + + tearDownAll(() async { + debugDefaultTargetPlatformOverride = null; + await db.close(); + }); + + setUp(() { + final mockLocalSyncService = MockLocalSyncService(); + final mockHashService = MockHashService(); + final mockUserService = MockUserService(); + mockRemoteSyncService = MockSyncStreamService(); + mockBackgroundUploadService = MockBackgroundUploadService(); + + when(() => mockLocalSyncService.sync()).thenAnswer((_) async {}); + when(() => mockHashService.hashAssets()).thenAnswer((_) async {}); + when(() => mockUserService.tryGetMyUser()).thenReturn(UserStub.admin); + when(() => mockUserService.watchMyUser()).thenAnswer((_) => const Stream.empty()); + when(() => mockBackgroundUploadService.getActiveTasks(kBackupGroup)).thenAnswer((_) async => []); + when(() => mockBackgroundUploadService.uploadBackupCandidates(any())).thenAnswer((_) async {}); + when(() => mockBackgroundUploadService.resume()).thenAnswer((_) async {}); + + sut = BackgroundWorkerBgService( + dataController: MockDataController(), + apiService: MockApiService(), + overrides: [ + localSyncServiceProvider.overrideWithValue(mockLocalSyncService), + syncStreamServiceProvider.overrideWithValue(mockRemoteSyncService), + hashServiceProvider.overrideWithValue(mockHashService), + currentUserProvider.overrideWith((ref) => CurrentUserProvider(mockUserService)), + foregroundUploadServiceProvider.overrideWithValue(MockForegroundUploadService()), + backgroundUploadServiceProvider.overrideWithValue(mockBackgroundUploadService), + ], + ); + }); + + group('onIosUpload', () { + test('lists a new batch only after the remote sync finished', () async { + final remoteSync = Completer(); + when(() => mockRemoteSyncService.sync()).thenAnswer((_) => remoteSync.future); + + final upload = sut.onIosUpload(true, 20); + await pumpEventQueue(); + verifyNever(() => mockBackgroundUploadService.uploadBackupCandidates(any())); + + remoteSync.complete(true); + await upload; + verify(() => mockBackgroundUploadService.uploadBackupCandidates(UserStub.admin.id)).called(1); + }); + + test('does not list a new batch when the remote sync fails', () async { + when(() => mockRemoteSyncService.sync()).thenAnswer((_) async => false); + + await sut.onIosUpload(true, 20); + + verifyNever(() => mockBackgroundUploadService.uploadBackupCandidates(any())); + }); + + test('does not list a new batch when the run times out before the remote sync finished', () { + fakeAsync((async) { + final remoteSync = Completer(); + when(() => mockRemoteSyncService.sync()).thenAnswer((_) => remoteSync.future); + + unawaited(sut.onIosUpload(true, 20)); + async.elapse(const Duration(seconds: 20)); + // a cancelled sync can still return true + remoteSync.complete(true); + async.flushMicrotasks(); + + verifyNever(() => mockBackgroundUploadService.uploadBackupCandidates(any())); + }); + }); + + test('resumes pending uploads without waiting for the remote sync', () async { + final remoteSync = Completer(); + when(() => mockRemoteSyncService.sync()).thenAnswer((_) => remoteSync.future); + when( + () => mockBackgroundUploadService.getActiveTasks(kBackupGroup), + ).thenAnswer((_) async => [UploadTask(url: 'https://example.com', filename: 'photo.jpg')]); + + final upload = sut.onIosUpload(true, 20); + await pumpEventQueue(); + verify(() => mockBackgroundUploadService.resume()).called(1); + + remoteSync.complete(true); + await upload; + }); + }); +}