move empty checks inside repo

mobile/lib/domain/interfaces/album_media.interface.dart

@@ -2,24 +2,17 @@ import 'package:immich_mobile/domain/models/asset/asset.model.dart';
 import 'package:immich_mobile/domain/models/local_album.model.dart';
 
 abstract interface class IAlbumMediaRepository {
-  Future<List<LocalAlbum>> getAll({
+  Future<List<LocalAlbum>> getAll();
-    bool withModifiedTime = false,
-    bool withAssetCount = false,
-    bool withAssetTitle = false,
-  });
 
   Future<List<LocalAsset>> getAssetsForAlbum(
     String albumId, {
-    bool withModifiedTime = false,
-    bool withAssetTitle = true,
     DateTimeFilter? updateTimeCond,
   });
 
   Future<LocalAlbum> refresh(
     String albumId, {
-    bool withModifiedTime = false,
+    bool withModifiedTime = true,
-    bool withAssetCount = false,
+    bool withAssetCount = true,
-    bool withAssetTitle = false,
   });
 }
 

mobile/lib/domain/services/device_sync.service.dart

@@ -11,13 +11,13 @@ import 'package:immich_mobile/utils/diff.dart';
 import 'package:immich_mobile/utils/nullable_value.dart';
 import 'package:logging/logging.dart';
 
-class SyncService {
+class DeviceSyncService {
   final IAlbumMediaRepository _albumMediaRepository;
   final ILocalAlbumRepository _localAlbumRepository;
   final ILocalAssetRepository _localAssetRepository;
   final Logger _log = Logger("SyncService");
 
-  SyncService({
+  DeviceSyncService({
     required IAlbumMediaRepository albumMediaRepository,
     required ILocalAlbumRepository localAlbumRepository,
     required ILocalAssetRepository localAssetRepository,
@@ -25,46 +25,41 @@ class SyncService {
         _localAlbumRepository = localAlbumRepository,
         _localAssetRepository = localAssetRepository;
 
-  Future<bool> syncLocalAlbums() async {
+  Future<void> syncAlbums() async {
     try {
       final Stopwatch stopwatch = Stopwatch()..start();
       // The deviceAlbums will not have the updatedAt field
       // and the assetCount will be 0. They are refreshed later
-      // after the comparison
+      // after the comparison. The orderby in the filter sorts the assets
+      // and not the albums.
       final deviceAlbums =
           (await _albumMediaRepository.getAll()).sortedBy((a) => a.id);
       final dbAlbums =
           await _localAlbumRepository.getAll(sortBy: SortLocalAlbumsBy.id);
 
-      final hasChange = await diffSortedLists(
+      await diffSortedLists(
         dbAlbums,
         deviceAlbums,
         compare: (a, b) => a.id.compareTo(b.id),
-        both: syncLocalAlbum,
+        both: updateAlbum,
-        onlyFirst: removeLocalAlbum,
+        onlyFirst: removeAlbum,
-        onlySecond: addLocalAlbum,
+        onlySecond: addAlbum,
       );
 
       stopwatch.stop();
       _log.info("Full device sync took - ${stopwatch.elapsedMilliseconds}ms");
-      return hasChange;
     } catch (e, s) {
       _log.severe("Error performing full device sync", e, s);
     }
-    return false;
   }
 
-  Future<void> addLocalAlbum(LocalAlbum newAlbum) async {
+  Future<void> addAlbum(LocalAlbum newAlbum) async {
     try {
       _log.info("Adding device album ${newAlbum.name}");
-      final deviceAlbum = await _albumMediaRepository.refresh(
+      final deviceAlbum = await _albumMediaRepository.refresh(newAlbum.id);
-        newAlbum.id,
-        withModifiedTime: true,
-        withAssetCount: true,
-      );
 
       final assets = deviceAlbum.assetCount > 0
-          ? (await _albumMediaRepository.getAssetsForAlbum(deviceAlbum.id))
+          ? await _albumMediaRepository.getAssetsForAlbum(deviceAlbum.id)
           : <LocalAsset>[];
 
       final album = deviceAlbum.copyWith(
@@ -73,13 +68,13 @@ class SyncService {
       );
 
       await _localAlbumRepository.insert(album, assets);
-      _log.info("Successfully added device album ${album.name}");
+      _log.fine("Successfully added device album ${album.name}");
     } catch (e, s) {
       _log.warning("Error while adding device album", e, s);
     }
   }
 
-  Future<void> removeLocalAlbum(LocalAlbum a) async {
+  Future<void> removeAlbum(LocalAlbum a) async {
     _log.info("Removing device album ${a.name}");
     try {
       // Asset deletion is handled in the repository
@@ -90,39 +85,26 @@ class SyncService {
   }
 
   // The deviceAlbum is ignored since we are going to refresh it anyways
-  FutureOr<bool> syncLocalAlbum(LocalAlbum dbAlbum, LocalAlbum _) async {
+  FutureOr<bool> updateAlbum(LocalAlbum dbAlbum, LocalAlbum _) async {
     try {
       _log.info("Syncing device album ${dbAlbum.name}");
 
-      final deviceAlbum = await _albumMediaRepository.refresh(
+      final deviceAlbum = await _albumMediaRepository.refresh(dbAlbum.id);
-        dbAlbum.id,
-        withModifiedTime: true,
-        withAssetCount: true,
-      );
 
       // Early return if album hasn't changed
       if (deviceAlbum.updatedAt.isAtSameMomentAs(dbAlbum.updatedAt) &&
           deviceAlbum.assetCount == dbAlbum.assetCount) {
-        _log.info(
+        _log.fine(
           "Device album ${dbAlbum.name} has not changed. Skipping sync.",
         );
         return false;
       }
 
-      // Skip empty albums that don't need syncing
+      _log.fine("Device album ${dbAlbum.name} has changed. Syncing...");
-      if (deviceAlbum.assetCount == 0 && dbAlbum.assetCount == 0) {
-        await _localAlbumRepository.update(
-          deviceAlbum.copyWith(backupSelection: dbAlbum.backupSelection),
-        );
-        _log.info("Album ${dbAlbum.name} is empty. Only metadata updated.");
-        return true;
-      }
 
-      _log.info("Device album ${dbAlbum.name} has changed. Syncing...");
+      // Faster path - only new assets added
-
+      if (await checkAddition(dbAlbum, deviceAlbum)) {
-      // Faster path - only assets added
+        _log.fine("Fast synced device album ${dbAlbum.name}");
-      if (await tryFastSync(dbAlbum, deviceAlbum)) {
-        _log.info("Fast synced device album ${dbAlbum.name}");
         return true;
       }
 
@@ -137,19 +119,15 @@ class SyncService {
   @visibleForTesting
   // The [deviceAlbum] is expected to be refreshed before calling this method
   // with modified time and asset count
-  Future<bool> tryFastSync(LocalAlbum dbAlbum, LocalAlbum deviceAlbum) async {
+  Future<bool> checkAddition(
+    LocalAlbum dbAlbum,
+    LocalAlbum deviceAlbum,
+  ) async {
     try {
-      _log.info("Fast syncing device album ${dbAlbum.name}");
+      _log.fine("Fast syncing device album ${dbAlbum.name}");
-      if (!deviceAlbum.updatedAt.isAfter(dbAlbum.updatedAt)) {
-        _log.info(
-          "Local album ${deviceAlbum.name} has modifications. Proceeding to full sync",
-        );
-        return false;
-      }
-
       // Assets has been modified
       if (deviceAlbum.assetCount <= dbAlbum.assetCount) {
-        _log.info("Local album has modifications. Proceeding to full sync");
+        _log.fine("Local album has modifications. Proceeding to full sync");
         return false;
       }
 
@@ -164,15 +142,15 @@ class SyncService {
 
       // Early return if no new assets were found
       if (newAssets.isEmpty) {
-        _log.info(
+        _log.fine(
-          "No new assets found despite album changes. Proceeding to full sync for ${dbAlbum.name}",
+          "No new assets found despite album having changes. Proceeding to full sync for ${dbAlbum.name}",
         );
         return false;
       }
 
       // Check whether there is only addition or if there has been deletions
       if (deviceAlbum.assetCount != dbAlbum.assetCount + newAssets.length) {
-        _log.info("Local album has modifications. Proceeding to full sync");
+        _log.fine("Local album has modifications. Proceeding to full sync");
         return false;
       }
 
@@ -190,7 +168,7 @@ class SyncService {
         }
       }
 
-      await _handleUpdate(
+      await _updateAlbum(
         deviceAlbum.copyWith(
           thumbnailId: NullableValue.valueOrEmpty(thumbnailId),
           backupSelection: dbAlbum.backupSelection,
@@ -221,7 +199,7 @@ class SyncService {
         _log.fine(
           "Device album ${deviceAlbum.name} is empty. Removing assets from DB.",
         );
-        await _handleUpdate(
+        await _updateAlbum(
           deviceAlbum.copyWith(
             // Clear thumbnail for empty album
             thumbnailId: const NullableValue.empty(),
@@ -246,37 +224,38 @@ class SyncService {
         _log.fine(
           "Device album ${deviceAlbum.name} is empty. Adding assets to DB.",
         );
-        await _handleUpdate(updatedDeviceAlbum, assetsToUpsert: assetsInDevice);
+        await _updateAlbum(updatedDeviceAlbum, assetsToUpsert: assetsInDevice);
         return true;
       }
 
-      // Sort assets by localId for the diffSortedLists function
+      assert(assetsInDb.isSortedBy((a) => a.localId));
-      assetsInDb.sort((a, b) => a.localId.compareTo(b.localId));
       assetsInDevice.sort((a, b) => a.localId.compareTo(b.localId));
 
-      final assetsToAddOrUpdate = <LocalAsset>[];
+      final assetsToUpsert = <LocalAsset>[];
-      final assetIdsToDelete = <String>[];
+      final assetsToDelete = <String>[];
 
       diffSortedListsSync(
         assetsInDb,
         assetsInDevice,
         compare: (a, b) => a.localId.compareTo(b.localId),
         both: (dbAsset, deviceAsset) {
+          // Custom comparison to check if the asset has been modified without
+          // comparing the checksum
           if (!_assetsEqual(dbAsset, deviceAsset)) {
-            assetsToAddOrUpdate.add(deviceAsset);
+            assetsToUpsert.add(deviceAsset);
             return true;
           }
           return false;
         },
-        onlyFirst: (dbAsset) => assetIdsToDelete.add(dbAsset.localId),
+        onlyFirst: (dbAsset) => assetsToDelete.add(dbAsset.localId),
-        onlySecond: (deviceAsset) => assetsToAddOrUpdate.add(deviceAsset),
+        onlySecond: (deviceAsset) => assetsToUpsert.add(deviceAsset),
       );
 
-      _log.info(
+      _log.fine(
-        "Syncing ${deviceAlbum.name}. ${assetsToAddOrUpdate.length} assets to add/update and ${assetIdsToDelete.length} assets to delete",
+        "Syncing ${deviceAlbum.name}. ${assetsToUpsert.length} assets to add/update and ${assetsToDelete.length} assets to delete",
       );
 
-      if (assetsToAddOrUpdate.isEmpty && assetIdsToDelete.isEmpty) {
+      if (assetsToUpsert.isEmpty && assetsToDelete.isEmpty) {
         _log.fine(
           "No asset changes detected in album ${deviceAlbum.name}. Updating metadata.",
         );
@@ -284,40 +263,30 @@ class SyncService {
         return true;
       }
 
-      await _handleUpdate(
+      await _updateAlbum(
         updatedDeviceAlbum,
-        assetsToUpsert: assetsToAddOrUpdate,
+        assetsToUpsert: assetsToUpsert,
-        assetIdsToDelete: assetIdsToDelete,
+        assetIdsToDelete: assetsToDelete,
       );
 
       return true;
     } catch (e, s) {
       _log.warning("Error on full syncing local album: ${dbAlbum.name}", e, s);
     }
-    return false;
+    return true;
   }
 
-  Future<void> _handleUpdate(
+  Future<void> _updateAlbum(
     LocalAlbum album, {
-    Iterable<LocalAsset>? assetsToUpsert,
+    Iterable<LocalAsset> assetsToUpsert = const [],
-    Iterable<String>? assetIdsToDelete,
+    Iterable<String> assetIdsToDelete = const [],
   }) =>
       _localAlbumRepository.transaction(() async {
-        if (assetsToUpsert != null && assetsToUpsert.isNotEmpty) {
         await _localAlbumRepository.addAssets(album.id, assetsToUpsert);
-        }
-
         await _localAlbumRepository.update(album);
-
+        await _localAlbumRepository.removeAssets(album.id, assetIdsToDelete);
-        if (assetIdsToDelete != null && assetIdsToDelete.isNotEmpty) {
-          await _localAlbumRepository.removeAssets(
-            album.id,
-            assetIdsToDelete,
-          );
-        }
       });
 
-  // Helper method to check if assets are equal without relying on the checksum
   bool _assetsEqual(LocalAsset a, LocalAsset b) {
     return a.updatedAt.isAtSameMomentAs(b.updatedAt) &&
         a.createdAt.isAtSameMomentAs(b.createdAt) &&

mobile/lib/domain/utils/background_sync.dart

@@ -29,7 +29,7 @@ class BackgroundSyncManager {
     }
 
     _deviceAlbumSyncTask = runInIsolateGentle(
-      computation: (ref) => ref.read(syncServiceProvider).syncLocalAlbums(),
+      computation: (ref) => ref.read(deviceSyncServiceProvider).syncAlbums(),
     );
     return _deviceAlbumSyncTask!.whenComplete(() {
       _deviceAlbumSyncTask = null;

mobile/lib/infrastructure/repositories/album_media.repository.dart

@@ -36,41 +36,24 @@ class AlbumMediaRepository implements IAlbumMediaRepository {
       );
 
   @override
-  Future<List<LocalAlbum>> getAll({
+  Future<List<LocalAlbum>> getAll() {
-    withModifiedTime = false,
+    final filter = AdvancedCustomFilter(
-    withAssetCount = false,
-    withAssetTitle = false,
-  }) async {
-    final filter = withModifiedTime || withAssetTitle
-        ? _getAlbumFilter(
-            withAssetTitle: withAssetTitle,
-            withModifiedTime: withModifiedTime,
-          )
-
-        // Use an AdvancedCustomFilter to get all albums faster
-        : AdvancedCustomFilter(
       orderBy: [OrderByItem.asc(CustomColumns.base.id)],
     );
 
-    final entities = await PhotoManager.getAssetPathList(
+    return PhotoManager.getAssetPathList(hasAll: true, filterOption: filter)
-      hasAll: true,
+        .then((e) => e.toDtoList());
-      filterOption: filter,
-    );
-    return entities.toDtoList(withAssetCount: withAssetCount);
   }
 
   @override
   Future<List<asset.LocalAsset>> getAssetsForAlbum(
     String albumId, {
-    withModifiedTime = false,
-    withAssetTitle = true,
     DateTimeFilter? updateTimeCond,
   }) async {
     final assetPathEntity = await AssetPathEntity.obtainPathFromProperties(
       id: albumId,
       optionGroup: _getAlbumFilter(
-        withAssetTitle: withAssetTitle,
+        withAssetTitle: true,
-        withModifiedTime: withModifiedTime,
         updateTimeCond: updateTimeCond,
       ),
     );
@@ -91,23 +74,17 @@ class AlbumMediaRepository implements IAlbumMediaRepository {
   @override
   Future<LocalAlbum> refresh(
     String albumId, {
-    withModifiedTime = false,
+    bool withModifiedTime = true,
-    withAssetCount = false,
+    bool withAssetCount = true,
-    withAssetTitle = false,
+  }) =>
-  }) async =>
+      AssetPathEntity.obtainPathFromProperties(
-      (await AssetPathEntity.obtainPathFromProperties(
         id: albumId,
-        optionGroup: _getAlbumFilter(
+        optionGroup: _getAlbumFilter(withModifiedTime: withModifiedTime),
-          withAssetTitle: withAssetTitle,
+      ).then((a) => a.toDto(withAssetCount: withAssetCount));
-          withModifiedTime: withModifiedTime,
-        ),
-      ))
-          .toDto(withAssetCount: withAssetCount);
 }
 
 extension on AssetEntity {
-  Future<asset.LocalAsset> toDto() async {
+  Future<asset.LocalAsset> toDto() async => asset.LocalAsset(
-    return asset.LocalAsset(
         localId: id,
         name: title ?? await titleAsync,
         type: switch (type) {
@@ -123,7 +100,6 @@ extension on AssetEntity {
         durationInSeconds: duration,
       );
 }
-}
 
 extension on List<AssetEntity> {
   Future<List<asset.LocalAsset>> toDtoList() =>

mobile/lib/infrastructure/repositories/local_album.repository.dart

@@ -52,9 +52,7 @@ class DriftLocalAlbumRepository extends DriftDatabaseRepository
         final assetsToDelete = _platform.isIOS
             ? await _getUniqueAssetsInAlbum(albumId)
             : await _getAssetsIdsInAlbum(albumId);
-        if (assetsToDelete.isNotEmpty) {
         await _deleteAssets(assetsToDelete);
-        }
 
         // All the other assets that are still associated will be unlinked automatically on-cascade
         await _db.managers.localAlbumEntity
@@ -65,35 +63,36 @@ class DriftLocalAlbumRepository extends DriftDatabaseRepository
   @override
   Future<void> insert(LocalAlbum localAlbum, Iterable<LocalAsset> assets) =>
       transaction(() async {
-        if (localAlbum.assetCount > 0) {
         await _upsertAssets(assets);
-        }
         // Needs to be after asset upsert to link the thumbnail
         await _upsertAlbum(localAlbum);
-
-        if (localAlbum.assetCount > 0) {
         await _linkAssetsToAlbum(localAlbum.id, assets);
-        }
       });
 
   @override
-  Future<void> addAssets(String albumId, Iterable<LocalAsset> assets) =>
+  Future<void> addAssets(String albumId, Iterable<LocalAsset> assets) {
-      transaction(() async {
+    if (assets.isEmpty) {
+      return Future.value();
+    }
+    return transaction(() async {
       await _upsertAssets(assets);
       await _linkAssetsToAlbum(albumId, assets);
     });
+  }
 
   @override
   Future<void> removeAssets(String albumId, Iterable<String> assetIds) async {
+    if (assetIds.isEmpty) {
+      return Future.value();
+    }
+
     if (_platform.isAndroid) {
-      await _deleteAssets(assetIds);
+      return _deleteAssets(assetIds);
-      return;
     }
 
     final uniqueAssets = await _getUniqueAssetsInAlbum(albumId);
     if (uniqueAssets.isEmpty) {
-      await _unlinkAssetsFromAlbum(albumId, assetIds);
+      return _unlinkAssetsFromAlbum(albumId, assetIds);
-      return;
     }
     // Delete unique assets and unlink others
     final uniqueSet = uniqueAssets.toSet();
@@ -106,8 +105,10 @@ class DriftLocalAlbumRepository extends DriftDatabaseRepository
         assetsToUnLink.add(assetId);
       }
     }
+    return transaction(() async {
       await _unlinkAssetsFromAlbum(albumId, assetsToUnLink);
       await _deleteAssets(assetsToDelete);
+    });
   }
 
   @override
@@ -123,7 +124,9 @@ class DriftLocalAlbumRepository extends DriftDatabaseRepository
               .equalsExp(_db.localAssetEntity.localId),
         ),
       ],
-    )..where(_db.localAlbumAssetEntity.albumId.equals(albumId));
+    )
+      ..where(_db.localAlbumAssetEntity.albumId.equals(albumId))
+      ..orderBy([OrderingTerm.desc(_db.localAssetEntity.localId)]);
     return query
         .map((row) => row.readTable(_db.localAssetEntity).toDto())
         .get();
@@ -147,8 +150,12 @@ class DriftLocalAlbumRepository extends DriftDatabaseRepository
   Future<void> _linkAssetsToAlbum(
     String albumId,
     Iterable<LocalAsset> assets,
-  ) =>
+  ) {
-      _db.batch(
+    if (assets.isEmpty) {
+      return Future.value();
+    }
+
+    return _db.batch(
       (batch) => batch.insertAll(
         _db.localAlbumAssetEntity,
         assets.map(
@@ -160,17 +167,23 @@ class DriftLocalAlbumRepository extends DriftDatabaseRepository
         mode: InsertMode.insertOrIgnore,
       ),
     );
+  }
 
   Future<void> _unlinkAssetsFromAlbum(
     String albumId,
     Iterable<String> assetIds,
-  ) =>
+  ) {
-      _db.batch(
+    if (assetIds.isEmpty) {
+      return Future.value();
+    }
+
+    return _db.batch(
       (batch) => batch.deleteWhere(
         _db.localAlbumAssetEntity,
         (f) => f.assetId.isIn(assetIds) & f.albumId.equals(albumId),
       ),
     );
+  }
 
   Future<List<String>> _getAssetsIdsInAlbum(String albumId) {
     final query = _db.localAlbumAssetEntity.select()
@@ -193,8 +206,12 @@ class DriftLocalAlbumRepository extends DriftDatabaseRepository
     return query.map((row) => row.read(assetId)!).get();
   }
 
-  Future<void> _upsertAssets(Iterable<LocalAsset> localAssets) =>
+  Future<void> _upsertAssets(Iterable<LocalAsset> localAssets) {
-      _db.batch((batch) async {
+    if (localAssets.isEmpty) {
+      return Future.value();
+    }
+
+    return _db.batch((batch) async {
       batch.insertAllOnConflictUpdate(
         _db.localAssetEntity,
         localAssets.map(
@@ -212,11 +229,18 @@ class DriftLocalAlbumRepository extends DriftDatabaseRepository
         ),
       );
     });
+  }
 
-  Future<void> _deleteAssets(Iterable<String> ids) => _db.batch(
+  Future<void> _deleteAssets(Iterable<String> ids) {
+    if (ids.isEmpty) {
+      return Future.value();
+    }
+
+    return _db.batch(
       (batch) => batch.deleteWhere(
         _db.localAssetEntity,
         (f) => f.localId.isIn(ids),
       ),
     );
   }
+}

mobile/lib/providers/infrastructure/sync.provider.dart

@@ -1,5 +1,5 @@
 import 'package:hooks_riverpod/hooks_riverpod.dart';
-import 'package:immich_mobile/domain/services/sync.service.dart';
+import 'package:immich_mobile/domain/services/device_sync.service.dart';
 import 'package:immich_mobile/domain/services/sync_stream.service.dart';
 import 'package:immich_mobile/infrastructure/repositories/sync_api.repository.dart';
 import 'package:immich_mobile/infrastructure/repositories/sync_stream.repository.dart';
@@ -9,8 +9,8 @@ import 'package:immich_mobile/providers/infrastructure/asset.provider.dart';
 import 'package:immich_mobile/providers/infrastructure/cancel.provider.dart';
 import 'package:immich_mobile/providers/infrastructure/db.provider.dart';
 
-final syncServiceProvider = Provider(
+final deviceSyncServiceProvider = Provider(
-  (ref) => SyncService(
+  (ref) => DeviceSyncService(
     albumMediaRepository: ref.watch(albumMediaRepositoryProvider),
     localAlbumRepository: ref.watch(localAlbumRepository),
     localAssetRepository: ref.watch(localAssetProvider),

mobile/test/fixtures/local_album.stub.dart

@@ -22,4 +22,14 @@ abstract final class LocalAlbumStub {
         backupSelection: BackupSelection.selected,
         isAll: true,
       );
+
+  static LocalAlbum get album3 => LocalAlbum(
+        id: "album3",
+        name: "Album 3",
+        updatedAt: DateTime(2020),
+        assetCount: 20,
+        thumbnailId: "123",
+        backupSelection: BackupSelection.excluded,
+        isAll: false,
+      );
 }