fix(mobile): make the iOS background batch wait for its remote sync (#32003)

fix(mobile): make the iOS background batch wait for its remote sync (#31942)

* fix(mobile): make the iOS background batch wait for its remote sync

* test onIosUpload itself, the worker reads its sync services from the providers

(cherry picked from commit ad93370f0287faafa7db615c726f32bd9e6d30e7)

Co-authored-by: Santo Shakil <santoshakil@users.noreply.github.com>

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<Override> 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<dynamic>([
         _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<bool> _handleBackup() async {
+  Future<bool> _handleBackup({required Future<bool> 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;
       }
 

mobile/lib/providers/backup/backup.provider.dart

@@ -251,7 +251,7 @@ class BackupNotifier extends StateNotifier<BackupState> {
     _uploadSpeedManager.removeTask(localAssetId);
   }
 
-  Future<void> startBackupWithURLSession(String userId) async {
+  Future<void> startBackupWithURLSession(String userId, Future<bool> remoteSync) async {
     if (!mounted) {
       _logger.warning("Skip handleBackupResume (pre-call): notifier disposed");
       return;
@@ -266,6 +266,10 @@ class BackupNotifier extends StateNotifier<BackupState> {
     _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);
     }

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<bool>();
+      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<bool>();
+        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<bool>();
+      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;
+    });
+  });
+}