diff --git a/mobile/lib/domain/interfaces/album_media.interface.dart b/mobile/lib/domain/interfaces/album_media.interface.dart index ab8f659e9e..6257de9f17 100644 --- a/mobile/lib/domain/interfaces/album_media.interface.dart +++ b/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> getAll({ - bool withModifiedTime = false, - bool withAssetCount = false, - bool withAssetTitle = false, - }); + Future> getAll(); Future> getAssetsForAlbum( String albumId, { - bool withModifiedTime = false, - bool withAssetTitle = true, DateTimeFilter? updateTimeCond, }); Future refresh( String albumId, { - bool withModifiedTime = false, - bool withAssetCount = false, - bool withAssetTitle = false, + bool withModifiedTime = true, + bool withAssetCount = true, }); } diff --git a/mobile/lib/domain/services/sync.service.dart b/mobile/lib/domain/services/device_sync.service.dart similarity index 68% rename from mobile/lib/domain/services/sync.service.dart rename to mobile/lib/domain/services/device_sync.service.dart index 6c8b4cc593..7e7d842983 100644 --- a/mobile/lib/domain/services/sync.service.dart +++ b/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 syncLocalAlbums() async { + Future 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, - onlyFirst: removeLocalAlbum, - onlySecond: addLocalAlbum, + both: updateAlbum, + onlyFirst: removeAlbum, + 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 addLocalAlbum(LocalAlbum newAlbum) async { + Future addAlbum(LocalAlbum newAlbum) async { try { _log.info("Adding device album ${newAlbum.name}"); - final deviceAlbum = await _albumMediaRepository.refresh( - newAlbum.id, - withModifiedTime: true, - withAssetCount: true, - ); + final deviceAlbum = await _albumMediaRepository.refresh(newAlbum.id); final assets = deviceAlbum.assetCount > 0 - ? (await _albumMediaRepository.getAssetsForAlbum(deviceAlbum.id)) + ? await _albumMediaRepository.getAssetsForAlbum(deviceAlbum.id) : []; 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 removeLocalAlbum(LocalAlbum a) async { + Future 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 syncLocalAlbum(LocalAlbum dbAlbum, LocalAlbum _) async { + FutureOr updateAlbum(LocalAlbum dbAlbum, LocalAlbum _) async { try { _log.info("Syncing device album ${dbAlbum.name}"); - final deviceAlbum = await _albumMediaRepository.refresh( - dbAlbum.id, - withModifiedTime: true, - withAssetCount: true, - ); + final deviceAlbum = await _albumMediaRepository.refresh(dbAlbum.id); // 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 - 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.fine("Device album ${dbAlbum.name} has changed. Syncing..."); - _log.info("Device album ${dbAlbum.name} has changed. Syncing..."); - - // Faster path - only assets added - if (await tryFastSync(dbAlbum, deviceAlbum)) { - _log.info("Fast synced device album ${dbAlbum.name}"); + // Faster path - only new assets added + if (await checkAddition(dbAlbum, deviceAlbum)) { + _log.fine("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 tryFastSync(LocalAlbum dbAlbum, LocalAlbum deviceAlbum) async { + Future checkAddition( + LocalAlbum dbAlbum, + LocalAlbum deviceAlbum, + ) async { try { - _log.info("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; - } - + _log.fine("Fast syncing device album ${dbAlbum.name}"); // 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( - "No new assets found despite album changes. Proceeding to full sync for ${dbAlbum.name}", + _log.fine( + "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 - assetsInDb.sort((a, b) => a.localId.compareTo(b.localId)); + assert(assetsInDb.isSortedBy((a) => a.localId)); assetsInDevice.sort((a, b) => a.localId.compareTo(b.localId)); - final assetsToAddOrUpdate = []; - final assetIdsToDelete = []; + final assetsToUpsert = []; + final assetsToDelete = []; 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), - onlySecond: (deviceAsset) => assetsToAddOrUpdate.add(deviceAsset), + onlyFirst: (dbAsset) => assetsToDelete.add(dbAsset.localId), + onlySecond: (deviceAsset) => assetsToUpsert.add(deviceAsset), ); - _log.info( - "Syncing ${deviceAlbum.name}. ${assetsToAddOrUpdate.length} assets to add/update and ${assetIdsToDelete.length} assets to delete", + _log.fine( + "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, - assetIdsToDelete: assetIdsToDelete, + assetsToUpsert: assetsToUpsert, + assetIdsToDelete: assetsToDelete, ); return true; } catch (e, s) { _log.warning("Error on full syncing local album: ${dbAlbum.name}", e, s); } - return false; + return true; } - Future _handleUpdate( + Future _updateAlbum( LocalAlbum album, { - Iterable? assetsToUpsert, - Iterable? assetIdsToDelete, + Iterable assetsToUpsert = const [], + Iterable assetIdsToDelete = const [], }) => _localAlbumRepository.transaction(() async { - if (assetsToUpsert != null && assetsToUpsert.isNotEmpty) { - await _localAlbumRepository.addAssets(album.id, assetsToUpsert); - } - + await _localAlbumRepository.addAssets(album.id, assetsToUpsert); await _localAlbumRepository.update(album); - - if (assetIdsToDelete != null && assetIdsToDelete.isNotEmpty) { - await _localAlbumRepository.removeAssets( - album.id, - assetIdsToDelete, - ); - } + 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) && diff --git a/mobile/lib/domain/utils/background_sync.dart b/mobile/lib/domain/utils/background_sync.dart index 727ffa5147..4877009eb0 100644 --- a/mobile/lib/domain/utils/background_sync.dart +++ b/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; diff --git a/mobile/lib/infrastructure/repositories/album_media.repository.dart b/mobile/lib/infrastructure/repositories/album_media.repository.dart index cef5c73297..c834169852 100644 --- a/mobile/lib/infrastructure/repositories/album_media.repository.dart +++ b/mobile/lib/infrastructure/repositories/album_media.repository.dart @@ -36,41 +36,24 @@ class AlbumMediaRepository implements IAlbumMediaRepository { ); @override - Future> getAll({ - withModifiedTime = false, - 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( - hasAll: true, - filterOption: filter, + Future> getAll() { + final filter = AdvancedCustomFilter( + orderBy: [OrderByItem.asc(CustomColumns.base.id)], ); - return entities.toDtoList(withAssetCount: withAssetCount); + + return PhotoManager.getAssetPathList(hasAll: true, filterOption: filter) + .then((e) => e.toDtoList()); } @override Future> getAssetsForAlbum( String albumId, { - withModifiedTime = false, - withAssetTitle = true, DateTimeFilter? updateTimeCond, }) async { final assetPathEntity = await AssetPathEntity.obtainPathFromProperties( id: albumId, optionGroup: _getAlbumFilter( - withAssetTitle: withAssetTitle, - withModifiedTime: withModifiedTime, + withAssetTitle: true, updateTimeCond: updateTimeCond, ), ); @@ -91,38 +74,31 @@ class AlbumMediaRepository implements IAlbumMediaRepository { @override Future refresh( String albumId, { - withModifiedTime = false, - withAssetCount = false, - withAssetTitle = false, - }) async => - (await AssetPathEntity.obtainPathFromProperties( + bool withModifiedTime = true, + bool withAssetCount = true, + }) => + AssetPathEntity.obtainPathFromProperties( id: albumId, - optionGroup: _getAlbumFilter( - withAssetTitle: withAssetTitle, - withModifiedTime: withModifiedTime, - ), - )) - .toDto(withAssetCount: withAssetCount); + optionGroup: _getAlbumFilter(withModifiedTime: withModifiedTime), + ).then((a) => a.toDto(withAssetCount: withAssetCount)); } extension on AssetEntity { - Future toDto() async { - return asset.LocalAsset( - localId: id, - name: title ?? await titleAsync, - type: switch (type) { - AssetType.other => asset.AssetType.other, - AssetType.image => asset.AssetType.image, - AssetType.video => asset.AssetType.video, - AssetType.audio => asset.AssetType.audio, - }, - createdAt: createDateTime, - updatedAt: modifiedDateTime, - width: width, - height: height, - durationInSeconds: duration, - ); - } + Future toDto() async => asset.LocalAsset( + localId: id, + name: title ?? await titleAsync, + type: switch (type) { + AssetType.other => asset.AssetType.other, + AssetType.image => asset.AssetType.image, + AssetType.video => asset.AssetType.video, + AssetType.audio => asset.AssetType.audio, + }, + createdAt: createDateTime, + updatedAt: modifiedDateTime, + width: width, + height: height, + durationInSeconds: duration, + ); } extension on List { diff --git a/mobile/lib/infrastructure/repositories/local_album.repository.dart b/mobile/lib/infrastructure/repositories/local_album.repository.dart index 7833d45273..557c264409 100644 --- a/mobile/lib/infrastructure/repositories/local_album.repository.dart +++ b/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); - } + 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 insert(LocalAlbum localAlbum, Iterable assets) => transaction(() async { - if (localAlbum.assetCount > 0) { - await _upsertAssets(assets); - } + 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); - } + await _linkAssetsToAlbum(localAlbum.id, assets); }); @override - Future addAssets(String albumId, Iterable assets) => - transaction(() async { - await _upsertAssets(assets); - await _linkAssetsToAlbum(albumId, assets); - }); + Future addAssets(String albumId, Iterable assets) { + if (assets.isEmpty) { + return Future.value(); + } + return transaction(() async { + await _upsertAssets(assets); + await _linkAssetsToAlbum(albumId, assets); + }); + } @override Future removeAssets(String albumId, Iterable assetIds) async { + if (assetIds.isEmpty) { + return Future.value(); + } + if (_platform.isAndroid) { - await _deleteAssets(assetIds); - return; + return _deleteAssets(assetIds); } final uniqueAssets = await _getUniqueAssetsInAlbum(albumId); if (uniqueAssets.isEmpty) { - await _unlinkAssetsFromAlbum(albumId, assetIds); - return; + return _unlinkAssetsFromAlbum(albumId, assetIds); } // Delete unique assets and unlink others final uniqueSet = uniqueAssets.toSet(); @@ -106,8 +105,10 @@ class DriftLocalAlbumRepository extends DriftDatabaseRepository assetsToUnLink.add(assetId); } } - await _unlinkAssetsFromAlbum(albumId, assetsToUnLink); - await _deleteAssets(assetsToDelete); + 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,30 +150,40 @@ class DriftLocalAlbumRepository extends DriftDatabaseRepository Future _linkAssetsToAlbum( String albumId, Iterable assets, - ) => - _db.batch( - (batch) => batch.insertAll( - _db.localAlbumAssetEntity, - assets.map( - (a) => LocalAlbumAssetEntityCompanion.insert( - assetId: a.localId, - albumId: albumId, - ), + ) { + if (assets.isEmpty) { + return Future.value(); + } + + return _db.batch( + (batch) => batch.insertAll( + _db.localAlbumAssetEntity, + assets.map( + (a) => LocalAlbumAssetEntityCompanion.insert( + assetId: a.localId, + albumId: albumId, ), - mode: InsertMode.insertOrIgnore, ), - ); + mode: InsertMode.insertOrIgnore, + ), + ); + } Future _unlinkAssetsFromAlbum( String albumId, Iterable assetIds, - ) => - _db.batch( - (batch) => batch.deleteWhere( - _db.localAlbumAssetEntity, - (f) => f.assetId.isIn(assetIds) & f.albumId.equals(albumId), - ), - ); + ) { + if (assetIds.isEmpty) { + return Future.value(); + } + + return _db.batch( + (batch) => batch.deleteWhere( + _db.localAlbumAssetEntity, + (f) => f.assetId.isIn(assetIds) & f.albumId.equals(albumId), + ), + ); + } Future> _getAssetsIdsInAlbum(String albumId) { final query = _db.localAlbumAssetEntity.select() @@ -193,30 +206,41 @@ class DriftLocalAlbumRepository extends DriftDatabaseRepository return query.map((row) => row.read(assetId)!).get(); } - Future _upsertAssets(Iterable localAssets) => - _db.batch((batch) async { - batch.insertAllOnConflictUpdate( - _db.localAssetEntity, - localAssets.map( - (a) => LocalAssetEntityCompanion.insert( - name: a.name, - type: a.type, - createdAt: Value(a.createdAt), - updatedAt: Value(a.updatedAt), - width: Value.absentIfNull(a.width), - height: Value.absentIfNull(a.height), - durationInSeconds: Value.absentIfNull(a.durationInSeconds), - localId: a.localId, - checksum: Value.absentIfNull(a.checksum), - ), - ), - ); - }); + Future _upsertAssets(Iterable localAssets) { + if (localAssets.isEmpty) { + return Future.value(); + } - Future _deleteAssets(Iterable ids) => _db.batch( - (batch) => batch.deleteWhere( - _db.localAssetEntity, - (f) => f.localId.isIn(ids), + return _db.batch((batch) async { + batch.insertAllOnConflictUpdate( + _db.localAssetEntity, + localAssets.map( + (a) => LocalAssetEntityCompanion.insert( + name: a.name, + type: a.type, + createdAt: Value(a.createdAt), + updatedAt: Value(a.updatedAt), + width: Value.absentIfNull(a.width), + height: Value.absentIfNull(a.height), + durationInSeconds: Value.absentIfNull(a.durationInSeconds), + localId: a.localId, + checksum: Value.absentIfNull(a.checksum), + ), ), ); + }); + } + + Future _deleteAssets(Iterable ids) { + if (ids.isEmpty) { + return Future.value(); + } + + return _db.batch( + (batch) => batch.deleteWhere( + _db.localAssetEntity, + (f) => f.localId.isIn(ids), + ), + ); + } } diff --git a/mobile/lib/providers/infrastructure/sync.provider.dart b/mobile/lib/providers/infrastructure/sync.provider.dart index da776672d4..94b654c9d2 100644 --- a/mobile/lib/providers/infrastructure/sync.provider.dart +++ b/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( - (ref) => SyncService( +final deviceSyncServiceProvider = Provider( + (ref) => DeviceSyncService( albumMediaRepository: ref.watch(albumMediaRepositoryProvider), localAlbumRepository: ref.watch(localAlbumRepository), localAssetRepository: ref.watch(localAssetProvider), diff --git a/mobile/test/domain/services/sync_service_test.dart b/mobile/test/domain/services/device_sync_service_test.dart similarity index 62% rename from mobile/test/domain/services/sync_service_test.dart rename to mobile/test/domain/services/device_sync_service_test.dart index 2fd913b9b7..1eeeef2426 100644 --- a/mobile/test/domain/services/sync_service_test.dart +++ b/mobile/test/domain/services/device_sync_service_test.dart @@ -1,5 +1,3 @@ -// ignore_for_file: avoid-unsafe-collection-methods - import 'package:flutter/foundation.dart'; import 'package:flutter_test/flutter_test.dart'; import 'package:immich_mobile/domain/interfaces/album_media.interface.dart'; @@ -7,7 +5,7 @@ import 'package:immich_mobile/domain/interfaces/local_album.interface.dart'; import 'package:immich_mobile/domain/interfaces/local_asset.interface.dart'; import 'package:immich_mobile/domain/models/asset/asset.model.dart'; import 'package:immich_mobile/domain/models/local_album.model.dart'; -import 'package:immich_mobile/domain/services/sync.service.dart'; +import 'package:immich_mobile/domain/services/device_sync.service.dart'; import 'package:immich_mobile/utils/nullable_value.dart'; import 'package:mocktail/mocktail.dart'; @@ -19,18 +17,16 @@ void main() { late IAlbumMediaRepository mockAlbumMediaRepo; late ILocalAlbumRepository mockLocalAlbumRepo; late ILocalAssetRepository mockLocalAssetRepo; - late SyncService sut; + late DeviceSyncService sut; - Future mockTransaction(Future Function() action) async { - return await action(); - } + Future mockTransaction(Future Function() action) => action(); setUp(() { mockAlbumMediaRepo = MockAlbumMediaRepository(); mockLocalAlbumRepo = MockLocalAlbumRepository(); mockLocalAssetRepo = MockLocalAssetRepository(); - sut = SyncService( + sut = DeviceSyncService( albumMediaRepository: mockAlbumMediaRepo, localAlbumRepository: mockLocalAlbumRepo, localAssetRepository: mockLocalAssetRepo, @@ -40,6 +36,7 @@ void main() { registerFallbackValue(LocalAssetStub.image1); registerFallbackValue(SortLocalAlbumsBy.id); registerFallbackValue([]); + registerFallbackValue([]); when(() => mockAlbumMediaRepo.getAll()).thenAnswer((_) async => []); when(() => mockLocalAlbumRepo.getAll(sortBy: any(named: 'sortBy'))) @@ -54,16 +51,9 @@ void main() { .thenAnswer((_) async => true); when(() => mockLocalAlbumRepo.getAssetsForAlbum(any())) .thenAnswer((_) async => []); - when(() => mockLocalAssetRepo.get(any())).thenAnswer( - (_) async => LocalAssetStub.image1, - ); - when( - () => mockAlbumMediaRepo.refresh( - any(), - withModifiedTime: any(named: 'withModifiedTime'), - withAssetCount: any(named: 'withAssetCount'), - ), - ).thenAnswer( + when(() => mockLocalAssetRepo.get(any())) + .thenAnswer((_) async => LocalAssetStub.image1); + when(() => mockAlbumMediaRepo.refresh(any())).thenAnswer( (inv) async => LocalAlbumStub.album1.copyWith(id: inv.positionalArguments.first), ); @@ -83,76 +73,45 @@ void main() { ); }); - group('syncLocalAlbums', () { - test('should return false when no albums exist', () async { - final result = await sut.syncLocalAlbums(); - expect(result, isFalse); + group('syncAlbums', () { + test('should return when no albums exist', () async { + await sut.syncAlbums(); verify(() => mockAlbumMediaRepo.getAll()).called(1); verify(() => mockLocalAlbumRepo.getAll(sortBy: SortLocalAlbumsBy.id)) .called(1); verifyNever(() => mockLocalAlbumRepo.insert(any(), any())); verifyNever(() => mockLocalAlbumRepo.delete(any())); - verifyNever( - () => mockAlbumMediaRepo.refresh( - any(), - withModifiedTime: any(named: 'withModifiedTime'), - withAssetCount: any(named: 'withAssetCount'), - ), - ); + verifyNever(() => mockAlbumMediaRepo.refresh(any())); }); - test('should call addLocalAlbum for new device albums', () async { + test('should call addAlbum for new device albums', () async { final deviceAlbums = [LocalAlbumStub.album1, LocalAlbumStub.album2]; when(() => mockAlbumMediaRepo.getAll()) .thenAnswer((_) async => deviceAlbums); when(() => mockLocalAlbumRepo.getAll(sortBy: SortLocalAlbumsBy.id)) .thenAnswer((_) async => []); - when( - () => mockAlbumMediaRepo.refresh( - deviceAlbums.first.id, - withModifiedTime: true, - withAssetCount: true, - ), - ).thenAnswer( - (_) async => deviceAlbums.first - .copyWith(updatedAt: DateTime(2024), assetCount: 1), - ); - when( - () => mockAlbumMediaRepo.refresh( - deviceAlbums[1].id, - withModifiedTime: true, - withAssetCount: true, - ), - ).thenAnswer( - (_) async => - deviceAlbums[1].copyWith(updatedAt: DateTime(2024), assetCount: 0), - ); + final refreshedAlbum1 = + deviceAlbums.first.copyWith(updatedAt: DateTime(2024), assetCount: 1); + final refreshedAlbum2 = + deviceAlbums[1].copyWith(updatedAt: DateTime(2024), assetCount: 0); + + when(() => mockAlbumMediaRepo.refresh(deviceAlbums.first.id)) + .thenAnswer((_) async => refreshedAlbum1); + when(() => mockAlbumMediaRepo.refresh(deviceAlbums[1].id)) + .thenAnswer((_) async => refreshedAlbum2); when(() => mockAlbumMediaRepo.getAssetsForAlbum(deviceAlbums.first.id)) .thenAnswer((_) async => [LocalAssetStub.image1]); - final result = await sut.syncLocalAlbums(); + await sut.syncAlbums(); - expect(result, isTrue); verify(() => mockAlbumMediaRepo.getAll()).called(1); verify(() => mockLocalAlbumRepo.getAll(sortBy: SortLocalAlbumsBy.id)) .called(1); - // Verify addLocalAlbum logic was triggered twice - verify( - () => mockAlbumMediaRepo.refresh( - deviceAlbums.first.id, - withModifiedTime: true, - withAssetCount: true, - ), - ).called(1); - verify( - () => mockAlbumMediaRepo.refresh( - deviceAlbums[1].id, - withModifiedTime: true, - withAssetCount: true, - ), - ).called(1); + + verify(() => mockAlbumMediaRepo.refresh(deviceAlbums.first.id)).called(1); + verify(() => mockAlbumMediaRepo.refresh(deviceAlbums[1].id)).called(1); verify(() => mockAlbumMediaRepo.getAssetsForAlbum(deviceAlbums.first.id)) .called(1); verifyNever( @@ -162,38 +121,29 @@ void main() { verifyNever(() => mockLocalAlbumRepo.delete(any())); }); - test('should call removeLocalAlbum for albums only in DB', () async { + test('should call removeAlbum for albums only in DB', () async { final dbAlbums = [LocalAlbumStub.album1, LocalAlbumStub.album2]; when(() => mockAlbumMediaRepo.getAll()).thenAnswer((_) async => []); when(() => mockLocalAlbumRepo.getAll(sortBy: SortLocalAlbumsBy.id)) .thenAnswer((_) async => dbAlbums); - final result = await sut.syncLocalAlbums(); + await sut.syncAlbums(); - expect(result, isTrue); verify(() => mockAlbumMediaRepo.getAll()).called(1); verify(() => mockLocalAlbumRepo.getAll(sortBy: SortLocalAlbumsBy.id)) .called(1); verify(() => mockLocalAlbumRepo.delete(dbAlbums.first.id)).called(1); verify(() => mockLocalAlbumRepo.delete(dbAlbums[1].id)).called(1); verifyNever(() => mockLocalAlbumRepo.insert(any(), any())); - verifyNever( - () => mockAlbumMediaRepo.refresh( - any(), - withModifiedTime: any(named: 'withModifiedTime'), - withAssetCount: any(named: 'withAssetCount'), - ), - ); + verifyNever(() => mockAlbumMediaRepo.refresh(any())); }); test( - 'should call syncLocalAlbum for albums in both DB and device', + 'should call updateAlbum for albums in both DB and device', () async { final commonAlbum = LocalAlbumStub.album1; final deviceAlbums = [commonAlbum]; - final dbAlbums = [ - commonAlbum.copyWith(updatedAt: DateTime(2023)), - ]; // Different updatedAt to trigger sync + final dbAlbums = [commonAlbum.copyWith(updatedAt: DateTime(2023))]; when(() => mockAlbumMediaRepo.getAll()) .thenAnswer((_) async => deviceAlbums); when(() => mockLocalAlbumRepo.getAll(sortBy: SortLocalAlbumsBy.id)) @@ -201,34 +151,23 @@ void main() { final refreshedAlbum = commonAlbum.copyWith(updatedAt: DateTime(2024), assetCount: 1); - when( - () => mockAlbumMediaRepo.refresh( - commonAlbum.id, - withModifiedTime: true, - withAssetCount: true, - ), - ).thenAnswer((_) async => refreshedAlbum); + when(() => mockAlbumMediaRepo.refresh(commonAlbum.id)) + .thenAnswer((_) async => refreshedAlbum); when(() => mockAlbumMediaRepo.getAssetsForAlbum(commonAlbum.id)) .thenAnswer((_) async => [LocalAssetStub.image1]); when(() => mockLocalAlbumRepo.getAssetsForAlbum(commonAlbum.id)) - .thenAnswer((_) async => []); + .thenAnswer((_) async => []); // DB has no assets initially - final result = await sut.syncLocalAlbums(); + await sut.syncAlbums(); - expect(result, isTrue); verify(() => mockAlbumMediaRepo.getAll()).called(1); verify(() => mockLocalAlbumRepo.getAll(sortBy: SortLocalAlbumsBy.id)) .called(1); - verify( - () => mockAlbumMediaRepo.refresh( - commonAlbum.id, - withModifiedTime: true, - withAssetCount: true, - ), - ).called(1); + verify(() => mockAlbumMediaRepo.refresh(commonAlbum.id)).called(1); + // Verify fullSync path was likely taken verify(() => mockAlbumMediaRepo.getAssetsForAlbum(commonAlbum.id)) .called(1); verify(() => mockLocalAlbumRepo.getAssetsForAlbum(commonAlbum.id)) @@ -239,51 +178,77 @@ void main() { }, ); + test('should handle a mix of add, remove, and update', () async { + final albumToRemove = LocalAlbumStub.album1.copyWith(id: "remove_me"); + final albumToUpdate = LocalAlbumStub.album2.copyWith(id: "update_me"); + final albumToAdd = LocalAlbumStub.album3.copyWith(id: "add_me"); + + final dbAlbums = [ + albumToRemove, + albumToUpdate.copyWith(updatedAt: DateTime(2023)), + ]; + final deviceAlbums = [albumToUpdate, albumToAdd]; + + when(() => mockAlbumMediaRepo.getAll()) + .thenAnswer((_) async => deviceAlbums); + when(() => mockLocalAlbumRepo.getAll(sortBy: SortLocalAlbumsBy.id)) + .thenAnswer((_) async => dbAlbums); + + final refreshedUpdateAlbum = + albumToUpdate.copyWith(updatedAt: DateTime(2024), assetCount: 0); + when(() => mockAlbumMediaRepo.refresh(albumToUpdate.id)) + .thenAnswer((_) async => refreshedUpdateAlbum); + + final refreshedAddAlbum = + albumToAdd.copyWith(updatedAt: DateTime(2024), assetCount: 1); + when(() => mockAlbumMediaRepo.refresh(albumToAdd.id)) + .thenAnswer((_) async => refreshedAddAlbum); + when(() => mockAlbumMediaRepo.getAssetsForAlbum(albumToAdd.id)) + .thenAnswer((_) async => [LocalAssetStub.image1]); + + await sut.syncAlbums(); + + verify(() => mockAlbumMediaRepo.getAll()).called(1); + verify(() => mockLocalAlbumRepo.getAll(sortBy: SortLocalAlbumsBy.id)) + .called(1); + + verify(() => mockLocalAlbumRepo.delete(albumToRemove.id)).called(1); + + verify(() => mockAlbumMediaRepo.refresh(albumToAdd.id)).called(1); + verify(() => mockAlbumMediaRepo.getAssetsForAlbum(albumToAdd.id)) + .called(1); + verify( + () => mockLocalAlbumRepo.insert( + any(that: predicate((a) => a.id == albumToAdd.id)), + any(), + ), + ).called(1); + + verify(() => mockAlbumMediaRepo.refresh(albumToUpdate.id)).called(1); + verifyNever(() => mockAlbumMediaRepo.getAssetsForAlbum(albumToUpdate.id)); + verify(() => mockLocalAlbumRepo.getAssetsForAlbum(albumToUpdate.id)) + .called(1); + verify( + () => mockLocalAlbumRepo.update( + any(that: predicate((a) => a.id == albumToUpdate.id)), + ), + ).called(1); + }); + test('should handle errors during repository calls', () async { when(() => mockAlbumMediaRepo.getAll()) .thenThrow(Exception("Device error")); - final result = await sut.syncLocalAlbums(); + await sut.syncAlbums(); - expect(result, isFalse); verify(() => mockAlbumMediaRepo.getAll()).called(1); verifyNever( () => mockLocalAlbumRepo.getAll(sortBy: any(named: 'sortBy')), ); }); - - test('should handle errors during diff callbacks', () async { - final deviceAlbums = [LocalAlbumStub.album1]; - when(() => mockAlbumMediaRepo.getAll()) - .thenAnswer((_) async => deviceAlbums); - when(() => mockLocalAlbumRepo.getAll(sortBy: SortLocalAlbumsBy.id)) - .thenAnswer((_) async => []); - when( - () => mockAlbumMediaRepo.refresh( - any(), - withModifiedTime: any(named: 'withModifiedTime'), - withAssetCount: any(named: 'withAssetCount'), - ), - ).thenThrow(Exception("Refresh error")); - - final result = await sut.syncLocalAlbums(); - - expect(result, isTrue); - verify(() => mockAlbumMediaRepo.getAll()).called(1); - verify(() => mockLocalAlbumRepo.getAll(sortBy: SortLocalAlbumsBy.id)) - .called(1); - verify( - () => mockAlbumMediaRepo.refresh( - deviceAlbums.first.id, - withModifiedTime: true, - withAssetCount: true, - ), - ).called(1); - verifyNever(() => mockLocalAlbumRepo.insert(any(), any())); - }); }); - group('addLocalAlbum', () { + group('addAlbum', () { test( 'refreshes, gets assets, sets thumbnail, and inserts for non-empty album', () async { @@ -299,25 +264,14 @@ void main() { ), ]; - when( - () => mockAlbumMediaRepo.refresh( - newAlbum.id, - withModifiedTime: true, - withAssetCount: true, - ), - ).thenAnswer((_) async => refreshedAlbum); + when(() => mockAlbumMediaRepo.refresh(newAlbum.id)) + .thenAnswer((_) async => refreshedAlbum); when(() => mockAlbumMediaRepo.getAssetsForAlbum(newAlbum.id)) .thenAnswer((_) async => assets); - await sut.addLocalAlbum(newAlbum); + await sut.addAlbum(newAlbum); - verify( - () => mockAlbumMediaRepo.refresh( - newAlbum.id, - withModifiedTime: true, - withAssetCount: true, - ), - ).called(1); + verify(() => mockAlbumMediaRepo.refresh(newAlbum.id)).called(1); verify(() => mockAlbumMediaRepo.getAssetsForAlbum(newAlbum.id)) .called(1); @@ -342,23 +296,12 @@ void main() { final refreshedAlbum = newAlbum.copyWith(updatedAt: DateTime(2024), assetCount: 0); - when( - () => mockAlbumMediaRepo.refresh( - newAlbum.id, - withModifiedTime: true, - withAssetCount: true, - ), - ).thenAnswer((_) async => refreshedAlbum); + when(() => mockAlbumMediaRepo.refresh(newAlbum.id)) + .thenAnswer((_) async => refreshedAlbum); - await sut.addLocalAlbum(newAlbum); + await sut.addAlbum(newAlbum); - verify( - () => mockAlbumMediaRepo.refresh( - newAlbum.id, - withModifiedTime: true, - withAssetCount: true, - ), - ).called(1); + verify(() => mockAlbumMediaRepo.refresh(newAlbum.id)).called(1); verifyNever(() => mockAlbumMediaRepo.getAssetsForAlbum(newAlbum.id)); final captured = @@ -373,161 +316,43 @@ void main() { expect(capturedAssets, isEmpty); }, ); - - test('handles error during refresh', () async { - final newAlbum = LocalAlbumStub.album1; - when( - () => mockAlbumMediaRepo.refresh( - any(), - withModifiedTime: any(named: 'withModifiedTime'), - withAssetCount: any(named: 'withAssetCount'), - ), - ).thenThrow(Exception("Refresh failed")); - - await sut.addLocalAlbum(newAlbum); - - verify( - () => mockAlbumMediaRepo.refresh( - newAlbum.id, - withModifiedTime: true, - withAssetCount: true, - ), - ).called(1); - verifyNever(() => mockAlbumMediaRepo.getAssetsForAlbum(any())); - verifyNever(() => mockLocalAlbumRepo.insert(any(), any())); - }); - - test('handles error during getAssetsForAlbum', () async { - final newAlbum = LocalAlbumStub.album1.copyWith(assetCount: 0); - final refreshedAlbum = - newAlbum.copyWith(updatedAt: DateTime(2024), assetCount: 2); - - when( - () => mockAlbumMediaRepo.refresh( - newAlbum.id, - withModifiedTime: true, - withAssetCount: true, - ), - ).thenAnswer((_) async => refreshedAlbum); - when(() => mockAlbumMediaRepo.getAssetsForAlbum(newAlbum.id)) - .thenThrow(Exception("Get assets failed")); - - await sut.addLocalAlbum(newAlbum); - - verify( - () => mockAlbumMediaRepo.refresh( - newAlbum.id, - withModifiedTime: true, - withAssetCount: true, - ), - ).called(1); - verify(() => mockAlbumMediaRepo.getAssetsForAlbum(newAlbum.id)).called(1); - verifyNever(() => mockLocalAlbumRepo.insert(any(), any())); - }); }); - group('removeLocalAlbum', () { + group('removeAlbum', () { test('calls localAlbumRepository.delete', () async { final albumToDelete = LocalAlbumStub.album1; - await sut.removeLocalAlbum(albumToDelete); - verify(() => mockLocalAlbumRepo.delete(albumToDelete.id)).called(1); - }); - - test('handles error during delete', () async { - final albumToDelete = LocalAlbumStub.album1; - when(() => mockLocalAlbumRepo.delete(any())) - .thenThrow(Exception("Delete failed")); - - await expectLater(sut.removeLocalAlbum(albumToDelete), completes); + await sut.removeAlbum(albumToDelete); verify(() => mockLocalAlbumRepo.delete(albumToDelete.id)).called(1); }); }); - group('syncLocalAlbum', () { + group('updateAlbum', () { final dbAlbum = LocalAlbumStub.album1.copyWith( updatedAt: DateTime(2024, 1, 1), assetCount: 1, - backupSelection: BackupSelection.selected, ); - test('returns false if refresh shows no changes', () async { - final refreshedAlbum = dbAlbum; // Same updatedAt and assetCount - when( - () => mockAlbumMediaRepo.refresh( - dbAlbum.id, - withModifiedTime: true, - withAssetCount: true, - ), - ).thenAnswer((_) async => refreshedAlbum); + test('returns early if refresh shows no changes', () async { + final refreshedAlbum = dbAlbum; + when(() => mockAlbumMediaRepo.refresh(dbAlbum.id)) + .thenAnswer((_) async => refreshedAlbum); - final result = await sut.syncLocalAlbum(dbAlbum, LocalAlbumStub.album1); + final result = await sut.updateAlbum(dbAlbum, LocalAlbumStub.album1); - expect(result, isFalse); - verify( - () => mockAlbumMediaRepo.refresh( - dbAlbum.id, - withModifiedTime: true, - withAssetCount: true, - ), - ).called(1); + expect(result, false); + verify(() => mockAlbumMediaRepo.refresh(dbAlbum.id)).called(1); verifyNever(() => mockAlbumMediaRepo.getAssetsForAlbum(any())); verifyNever(() => mockLocalAlbumRepo.getAssetsForAlbum(any())); verifyNever(() => mockLocalAlbumRepo.transaction(any())); }); - test( - 'updates metadata and returns true for empty albums with no changes needed', - () async { - final emptyDbAlbum = - dbAlbum.copyWith(updatedAt: DateTime(2024, 1, 1), assetCount: 0); - final refreshedEmptyAlbum = emptyDbAlbum.copyWith( - updatedAt: DateTime(2024, 1, 2), - ); - - when( - () => mockAlbumMediaRepo.refresh( - emptyDbAlbum.id, - withModifiedTime: true, - withAssetCount: true, - ), - ).thenAnswer((_) async => refreshedEmptyAlbum); - - final result = - await sut.syncLocalAlbum(emptyDbAlbum, LocalAlbumStub.album1); - - expect(result, isTrue); - verify( - () => mockAlbumMediaRepo.refresh( - emptyDbAlbum.id, - withModifiedTime: true, - withAssetCount: true, - ), - ).called(1); - verify( - () => mockLocalAlbumRepo.update( - refreshedEmptyAlbum.copyWith( - backupSelection: emptyDbAlbum.backupSelection, - ), - ), - ).called(1); - verifyNever(() => mockAlbumMediaRepo.getAssetsForAlbum(any())); - verifyNever(() => mockLocalAlbumRepo.getAssetsForAlbum(any())); - verifyNever(() => mockLocalAlbumRepo.transaction(any())); - }, - ); - - test('calls tryFastSync and returns true if it succeeds', () async { + test('calls checkAddition and returns true if it succeeds', () async { final refreshedAlbum = dbAlbum.copyWith( updatedAt: DateTime(2024, 1, 2), assetCount: 2, - ); // Time and count increased - when( - () => mockAlbumMediaRepo.refresh( - dbAlbum.id, - withModifiedTime: true, - withAssetCount: true, - ), - ).thenAnswer((_) async => refreshedAlbum); + ); + when(() => mockAlbumMediaRepo.refresh(dbAlbum.id)) + .thenAnswer((_) async => refreshedAlbum); final newAsset = LocalAssetStub.image2.copyWith(localId: "new_asset"); when( @@ -536,23 +361,21 @@ void main() { updateTimeCond: any(named: 'updateTimeCond'), ), ).thenAnswer((_) async => [newAsset]); + final dbAlbumNoThumb = dbAlbum.copyWith(thumbnailId: null); - final result = await sut.syncLocalAlbum(dbAlbum, LocalAlbumStub.album1); + final result = + await sut.updateAlbum(dbAlbumNoThumb, LocalAlbumStub.album1); expect(result, isTrue); - verify( - () => mockAlbumMediaRepo.refresh( - dbAlbum.id, - withModifiedTime: true, - withAssetCount: true, - ), - ).called(1); + verify(() => mockAlbumMediaRepo.refresh(dbAlbum.id)).called(1); + verify( () => mockAlbumMediaRepo.getAssetsForAlbum( dbAlbum.id, updateTimeCond: any(named: 'updateTimeCond'), ), ).called(1); + verifyNever(() => mockLocalAssetRepo.get(any())); verify(() => mockLocalAlbumRepo.transaction(any())).called(1); verify(() => mockLocalAlbumRepo.addAssets(dbAlbum.id, [newAsset])) @@ -561,31 +384,30 @@ void main() { () => mockLocalAlbumRepo.update( any( that: predicate( - (a) => a.id == dbAlbum.id && a.assetCount == 2, + (a) => + a.id == dbAlbum.id && + a.assetCount == 2 && + a.thumbnailId == newAsset.localId, ), ), ), ).called(1); - verifyNever(() => mockLocalAlbumRepo.removeAssets(any(), any())); + verify(() => mockLocalAlbumRepo.removeAssets(any(), any(that: isEmpty))) + .called(1); verifyNever(() => mockAlbumMediaRepo.getAssetsForAlbum(dbAlbum.id)); verifyNever(() => mockLocalAlbumRepo.getAssetsForAlbum(dbAlbum.id)); }); test( - 'calls fullSync and returns true if tryFastSync returns false', + 'calls fullSync and returns true if checkAddition returns false', () async { final refreshedAlbum = dbAlbum.copyWith( updatedAt: DateTime(2024, 1, 2), assetCount: 0, - ); // Count decreased -> fast sync fails - when( - () => mockAlbumMediaRepo.refresh( - dbAlbum.id, - withModifiedTime: true, - withAssetCount: true, - ), - ).thenAnswer((_) async => refreshedAlbum); + ); + when(() => mockAlbumMediaRepo.refresh(dbAlbum.id)) + .thenAnswer((_) async => refreshedAlbum); when(() => mockAlbumMediaRepo.getAssetsForAlbum(dbAlbum.id)) .thenAnswer((_) async => []); @@ -593,29 +415,23 @@ void main() { (_) async => [LocalAssetStub.image1], ); - final result = await sut.syncLocalAlbum(dbAlbum, LocalAlbumStub.album1); + final result = await sut.updateAlbum(dbAlbum, LocalAlbumStub.album1); expect(result, isTrue); - verify( - () => mockAlbumMediaRepo.refresh( - dbAlbum.id, - withModifiedTime: true, - withAssetCount: true, - ), - ).called(1); - // Verify tryFastSync path was attempted but failed (no getAssetsForAlbum with timeCond) + verify(() => mockAlbumMediaRepo.refresh(dbAlbum.id)).called(1); verifyNever( () => mockAlbumMediaRepo.getAssetsForAlbum( dbAlbum.id, updateTimeCond: any(named: 'updateTimeCond'), ), ); - // Verify fullSync path was taken + verifyNever(() => mockAlbumMediaRepo.getAssetsForAlbum(dbAlbum.id)); verify(() => mockLocalAlbumRepo.getAssetsForAlbum(dbAlbum.id)) .called(1); - // Verify _handleUpdate was called via fullSync - verifyNever(() => mockLocalAlbumRepo.addAssets(any(), any())); + verify(() => mockLocalAlbumRepo.transaction(any())).called(1); + verify(() => mockLocalAlbumRepo.addAssets(any(), any(that: isEmpty))) + .called(1); verify( () => mockLocalAlbumRepo.update( any( @@ -635,31 +451,62 @@ void main() { }, ); - test('handles error during refresh', () async { + test('handles error during checkAddition', () async { + final refreshedAlbum = dbAlbum.copyWith( + updatedAt: DateTime(2024, 1, 2), + assetCount: 2, + ); + when(() => mockAlbumMediaRepo.refresh(dbAlbum.id)) + .thenAnswer((_) async => refreshedAlbum); when( - () => mockAlbumMediaRepo.refresh( - any(), - withModifiedTime: any(named: 'withModifiedTime'), - withAssetCount: any(named: 'withAssetCount'), + () => mockAlbumMediaRepo.getAssetsForAlbum( + dbAlbum.id, + updateTimeCond: any(named: 'updateTimeCond'), ), - ).thenThrow(Exception("Refresh failed")); + ).thenThrow(Exception("checkAddition failed")); - final result = await sut.syncLocalAlbum(dbAlbum, LocalAlbumStub.album1); + when(() => mockAlbumMediaRepo.getAssetsForAlbum(dbAlbum.id)) + .thenAnswer((_) async => []); + when(() => mockLocalAlbumRepo.getAssetsForAlbum(dbAlbum.id)) + .thenAnswer((_) async => []); + + final result = await sut.updateAlbum(dbAlbum, LocalAlbumStub.album1); expect(result, isTrue); + verify(() => mockAlbumMediaRepo.refresh(dbAlbum.id)).called(1); verify( - () => mockAlbumMediaRepo.refresh( + () => mockAlbumMediaRepo.getAssetsForAlbum( dbAlbum.id, - withModifiedTime: true, - withAssetCount: true, + updateTimeCond: any(named: 'updateTimeCond'), ), - ).called(1); - verifyNever(() => mockAlbumMediaRepo.getAssetsForAlbum(any())); + ).called(2); // One for checkAddition, one for fullSync + verify(() => mockLocalAlbumRepo.getAssetsForAlbum(dbAlbum.id)).called(1); + }); + + test('handles error during fullSync', () async { + final refreshedAlbum = dbAlbum.copyWith( + updatedAt: DateTime(2024, 1, 2), + assetCount: 1, + ); + when(() => mockAlbumMediaRepo.refresh(dbAlbum.id)) + .thenAnswer((_) async => refreshedAlbum); + when(() => mockAlbumMediaRepo.getAssetsForAlbum(dbAlbum.id)) + .thenThrow(Exception("fullSync failed")); + + final result = await sut.updateAlbum( + dbAlbum, + LocalAlbumStub.album1.copyWith(assetCount: 2), + ); + + expect(result, isTrue); + verify(() => mockAlbumMediaRepo.refresh(dbAlbum.id)).called(1); + verify(() => mockAlbumMediaRepo.getAssetsForAlbum(dbAlbum.id)); + verifyNever(() => mockLocalAlbumRepo.getAssetsForAlbum(any())); verifyNever(() => mockLocalAlbumRepo.transaction(any())); }); }); - group('tryFastSync', () { + group('checkAddition', () { final dbAlbum = LocalAlbumStub.album1.copyWith( updatedAt: DateTime(2024, 1, 1, 10, 0, 0), assetCount: 1, @@ -687,9 +534,9 @@ void main() { localId: "thumb1", createdAt: DateTime(2024, 1, 1, 9, 0, 0), ), - ); // Old thumb is older + ); - final result = await sut.tryFastSync(dbAlbum, refreshedAlbum); + final result = await sut.checkAddition(dbAlbum, refreshedAlbum); expect(result, isTrue); verify( @@ -709,12 +556,13 @@ void main() { a.id == dbAlbum.id && a.assetCount == 2 && a.updatedAt == refreshedAlbum.updatedAt && - a.thumbnailId == newAsset.localId, // Thumbnail updated + a.thumbnailId == newAsset.localId, ), ), ), ).called(1); - verifyNever(() => mockLocalAlbumRepo.removeAssets(any(), any())); + verify(() => mockLocalAlbumRepo.removeAssets(any(), any(that: isEmpty))) + .called(1); }); test('returns true and keeps old thumbnail if newer', () async { @@ -734,9 +582,9 @@ void main() { localId: "thumb1", createdAt: DateTime(2024, 1, 1, 9, 0, 0), ), - ); // Old thumb is newer + ); - final result = await sut.tryFastSync(dbAlbum, refreshedAlbum); + final result = await sut.checkAddition(dbAlbum, refreshedAlbum); expect(result, isTrue); verify( @@ -756,30 +604,56 @@ void main() { a.id == dbAlbum.id && a.assetCount == 2 && a.updatedAt == refreshedAlbum.updatedAt && - a.thumbnailId == "thumb1", // Thumbnail NOT updated + a.thumbnailId == "thumb1", ), ), ), ).called(1); }); - test('returns false if updatedAt is not after', () async { - final notUpdatedAlbum = - refreshedAlbum.copyWith(updatedAt: dbAlbum.updatedAt); // Same time - final result = await sut.tryFastSync(dbAlbum, notUpdatedAlbum); - expect(result, isFalse); - verifyNever( + test('returns true and sets new thumbnail if db thumb is null', () async { + final dbAlbumNoThumb = dbAlbum.copyWith(thumbnailId: null); + final newAsset = LocalAssetStub.image2.copyWith( + localId: "asset2", + createdAt: DateTime(2024, 1, 1, 10, 30, 0), + ); + when( () => mockAlbumMediaRepo.getAssetsForAlbum( - any(), + dbAlbum.id, updateTimeCond: any(named: 'updateTimeCond'), ), - ); - verifyNever(() => mockLocalAlbumRepo.transaction(any())); + ).thenAnswer((_) async => [newAsset]); + + final result = await sut.checkAddition(dbAlbumNoThumb, refreshedAlbum); + + expect(result, isTrue); + verify( + () => mockAlbumMediaRepo.getAssetsForAlbum( + dbAlbum.id, + updateTimeCond: any(named: 'updateTimeCond'), + ), + ).called(1); + verifyNever(() => mockLocalAssetRepo.get(any())); + verify(() => mockLocalAlbumRepo.addAssets(dbAlbum.id, [newAsset])) + .called(1); + verify( + () => mockLocalAlbumRepo.update( + any( + that: predicate( + (a) => + a.id == dbAlbum.id && + a.assetCount == 2 && + a.updatedAt == refreshedAlbum.updatedAt && + a.thumbnailId == newAsset.localId, + ), + ), + ), + ).called(1); }); test('returns false if assetCount decreased', () async { final decreasedCountAlbum = refreshedAlbum.copyWith(assetCount: 0); - final result = await sut.tryFastSync(dbAlbum, decreasedCountAlbum); + final result = await sut.checkAddition(dbAlbum, decreasedCountAlbum); expect(result, isFalse); verifyNever( () => mockAlbumMediaRepo.getAssetsForAlbum( @@ -793,7 +667,7 @@ void main() { test('returns false if assetCount is same', () async { final sameCountAlbum = refreshedAlbum.copyWith(assetCount: dbAlbum.assetCount); - final result = await sut.tryFastSync(dbAlbum, sameCountAlbum); + final result = await sut.checkAddition(dbAlbum, sameCountAlbum); expect(result, isFalse); verifyNever( () => mockAlbumMediaRepo.getAssetsForAlbum( @@ -811,7 +685,7 @@ void main() { updateTimeCond: any(named: 'updateTimeCond'), ), ).thenAnswer((_) async => []); - final result = await sut.tryFastSync(dbAlbum, refreshedAlbum); + final result = await sut.checkAddition(dbAlbum, refreshedAlbum); expect(result, isFalse); verify( () => mockAlbumMediaRepo.getAssetsForAlbum( @@ -833,27 +707,7 @@ void main() { updateTimeCond: any(named: 'updateTimeCond'), ), ).thenAnswer((_) async => newAssets); - final result = await sut.tryFastSync(dbAlbum, mismatchCountAlbum); - expect(result, isFalse); - verify( - () => mockAlbumMediaRepo.getAssetsForAlbum( - dbAlbum.id, - updateTimeCond: any(named: 'updateTimeCond'), - ), - ).called(1); - verifyNever(() => mockLocalAlbumRepo.transaction(any())); - }); - - test('handles error during getAssetsForAlbum', () async { - when( - () => mockAlbumMediaRepo.getAssetsForAlbum( - dbAlbum.id, - updateTimeCond: any(named: 'updateTimeCond'), - ), - ).thenThrow(Exception("Get assets failed")); - - final result = await sut.tryFastSync(dbAlbum, refreshedAlbum); - + final result = await sut.checkAddition(dbAlbum, mismatchCountAlbum); expect(result, isFalse); verify( () => mockAlbumMediaRepo.getAssetsForAlbum( @@ -876,28 +730,44 @@ void main() { assetCount: 2, ); - final dbAsset1 = LocalAssetStub.image1 - .copyWith(localId: "asset1", updatedAt: DateTime(2024)); + final dbAsset1 = LocalAssetStub.image1.copyWith( + localId: "asset1", + createdAt: DateTime(2024), + updatedAt: DateTime(2024), + ); final dbAsset2 = LocalAssetStub.image2.copyWith( localId: "asset2", + createdAt: DateTime(2024), updatedAt: DateTime(2024), ); // To be deleted - final deviceAsset1 = LocalAssetStub.image1 - .copyWith(localId: "asset1", updatedAt: DateTime(2025)); // Updated - final deviceAsset3 = LocalAssetStub.video1 - .copyWith(localId: "asset3", updatedAt: DateTime(2024)); // Added + final deviceAsset1 = LocalAssetStub.image1.copyWith( + localId: "asset1", + createdAt: DateTime(2024), + updatedAt: DateTime(2025), + ); // Updated + final deviceAsset3 = LocalAssetStub.video1.copyWith( + localId: "asset3", + createdAt: DateTime(2024), + updatedAt: DateTime(2024), + ); // Added test('handles empty device album -> deletes all DB assets', () async { final emptyRefreshedAlbum = refreshedAlbum.copyWith(assetCount: 0); + when(() => mockAlbumMediaRepo.getAssetsForAlbum(dbAlbum.id)) + .thenAnswer((_) async => []); when(() => mockLocalAlbumRepo.getAssetsForAlbum(dbAlbum.id)) .thenAnswer((_) async => [dbAsset1, dbAsset2]); final result = await sut.fullSync(dbAlbum, emptyRefreshedAlbum); expect(result, isTrue); - verifyNever(() => mockAlbumMediaRepo.getAssetsForAlbum(dbAlbum.id)); + verifyNever( + () => mockAlbumMediaRepo.getAssetsForAlbum(emptyRefreshedAlbum.id), + ); verify(() => mockLocalAlbumRepo.getAssetsForAlbum(dbAlbum.id)).called(1); - verifyNever(() => mockLocalAlbumRepo.addAssets(any(), any())); + verify(() => mockLocalAlbumRepo.transaction(any())).called(1); + verify(() => mockLocalAlbumRepo.addAssets(any(), any(that: isEmpty))) + .called(1); verify( () => mockLocalAlbumRepo.update( any( @@ -906,7 +776,7 @@ void main() { a.id == dbAlbum.id && a.assetCount == 0 && a.updatedAt == emptyRefreshedAlbum.updatedAt && - a.thumbnailId == null, // Thumbnail cleared + a.thumbnailId == null, ), ), ), @@ -922,6 +792,8 @@ void main() { thumbnailId: const NullableValue.empty(), ); final deviceAssets = [deviceAsset1, deviceAsset3]; + + deviceAssets.sort((a, b) => a.createdAt.compareTo(b.createdAt)); final refreshedWithAssets = refreshedAlbum.copyWith(assetCount: deviceAssets.length); @@ -936,6 +808,7 @@ void main() { verify(() => mockAlbumMediaRepo.getAssetsForAlbum(emptyDbAlbum.id)) .called(1); verifyNever(() => mockLocalAlbumRepo.getAssetsForAlbum(emptyDbAlbum.id)); + verify(() => mockLocalAlbumRepo.transaction(any())).called(1); verify(() => mockLocalAlbumRepo.addAssets(emptyDbAlbum.id, deviceAssets)) .called(1); verify( @@ -951,27 +824,31 @@ void main() { ), ), ).called(1); - verifyNever(() => mockLocalAlbumRepo.removeAssets(any(), any())); + verify(() => mockLocalAlbumRepo.removeAssets(any(), any(that: isEmpty))) + .called(1); }); test('handles mix of additions, updates, and deletions', () async { - final currentRefreshedAlbum = refreshedAlbum.copyWith( - assetCount: 2, - ); // asset1 updated, asset3 added, asset2 deleted + final currentRefreshedAlbum = refreshedAlbum.copyWith(assetCount: 2); + final deviceAssets = [deviceAsset1, deviceAsset3]; + deviceAssets.sort((a, b) => a.createdAt.compareTo(b.createdAt)); + final dbAssets = [dbAsset1, dbAsset2]; + dbAssets.sort((a, b) => a.localId.compareTo(b.localId)); + when(() => mockAlbumMediaRepo.getAssetsForAlbum(dbAlbum.id)).thenAnswer( - (_) async => [deviceAsset1, deviceAsset3], - ); // Device has asset1 (updated), asset3 (new) + (_) async => deviceAssets, + ); when(() => mockLocalAlbumRepo.getAssetsForAlbum(dbAlbum.id)).thenAnswer( - (_) async => [dbAsset1, dbAsset2], - ); // DB has asset1 (old), asset2 (to delete) + (_) async => dbAssets, + ); final result = await sut.fullSync(dbAlbum, currentRefreshedAlbum); expect(result, isTrue); verify(() => mockAlbumMediaRepo.getAssetsForAlbum(dbAlbum.id)).called(1); verify(() => mockLocalAlbumRepo.getAssetsForAlbum(dbAlbum.id)).called(1); + verify(() => mockLocalAlbumRepo.transaction(any())).called(1); - // Verify assets to upsert (updated asset1, new asset3) verify( () => mockLocalAlbumRepo.addAssets( dbAlbum.id, @@ -989,7 +866,6 @@ void main() { ), ).called(1); - // Verify metadata update (thumbnail should be asset1 as it's first in sorted device list) verify( () => mockLocalAlbumRepo.update( any( @@ -998,26 +874,25 @@ void main() { a.id == dbAlbum.id && a.assetCount == 2 && a.updatedAt == currentRefreshedAlbum.updatedAt && - a.thumbnailId == "asset1", + a.thumbnailId == deviceAssets.first.localId, ), ), ), ).called(1); - // Verify assets to delete (asset2) verify(() => mockLocalAlbumRepo.removeAssets(dbAlbum.id, ["asset2"])) .called(1); }); - test('handles no asset changes, only metadata update', () async { + test('handles identical assets, resulting in no DB changes', () async { final currentRefreshedAlbum = refreshedAlbum.copyWith( updatedAt: DateTime(2025), - assetCount: 1, - ); // Only updatedAt changed - final dbAssets = [dbAsset1]; - final deviceAssets = [ - dbAsset1, - ]; // Assets are identical based on _assetsEqual + assetCount: 2, + ); + final dbAssets = [dbAsset1, dbAsset2]; + final deviceAssets = [dbAsset1, dbAsset2]; + deviceAssets.sort((a, b) => a.createdAt.compareTo(b.createdAt)); + dbAssets.sort((a, b) => a.localId.compareTo(b.localId)); when(() => mockAlbumMediaRepo.getAssetsForAlbum(dbAlbum.id)) .thenAnswer((_) async => deviceAssets); @@ -1029,50 +904,22 @@ void main() { expect(result, isTrue); verify(() => mockAlbumMediaRepo.getAssetsForAlbum(dbAlbum.id)).called(1); verify(() => mockLocalAlbumRepo.getAssetsForAlbum(dbAlbum.id)).called(1); - // Transaction is NOT called because _handleUpdate isn't called if lists are empty verifyNever(() => mockLocalAlbumRepo.transaction(any())); - // Only metadata update is called directly + verifyNever(() => mockLocalAlbumRepo.addAssets(any(), any())); + verifyNever(() => mockLocalAlbumRepo.removeAssets(any(), any())); verify( () => mockLocalAlbumRepo.update( any( that: predicate( (a) => a.id == dbAlbum.id && - a.assetCount == 1 && + a.assetCount == 2 && a.updatedAt == currentRefreshedAlbum.updatedAt && - a.thumbnailId == "asset1", // Thumbnail remains + a.thumbnailId == deviceAssets.first.localId, ), ), ), ).called(1); - verifyNever(() => mockLocalAlbumRepo.addAssets(any(), any())); - verifyNever(() => mockLocalAlbumRepo.removeAssets(any(), any())); - }); - - test('handles error during getAssetsForAlbum (device)', () async { - when(() => mockAlbumMediaRepo.getAssetsForAlbum(dbAlbum.id)) - .thenThrow(Exception("Get device assets failed")); - - final result = await sut.fullSync(dbAlbum, refreshedAlbum); - - expect(result, isFalse); - verify(() => mockAlbumMediaRepo.getAssetsForAlbum(dbAlbum.id)).called(1); - verifyNever(() => mockLocalAlbumRepo.getAssetsForAlbum(any())); - verifyNever(() => mockLocalAlbumRepo.transaction(any())); - }); - - test('handles error during getAssetsForAlbum (db)', () async { - when(() => mockAlbumMediaRepo.getAssetsForAlbum(dbAlbum.id)) - .thenAnswer((_) async => [deviceAsset1]); - when(() => mockLocalAlbumRepo.getAssetsForAlbum(dbAlbum.id)) - .thenThrow(Exception("Get db assets failed")); - - final result = await sut.fullSync(dbAlbum, refreshedAlbum); - - expect(result, isFalse); - verify(() => mockAlbumMediaRepo.getAssetsForAlbum(dbAlbum.id)).called(1); - verify(() => mockLocalAlbumRepo.getAssetsForAlbum(dbAlbum.id)).called(1); - verifyNever(() => mockLocalAlbumRepo.transaction(any())); }); }); } diff --git a/mobile/test/fixtures/local_album.stub.dart b/mobile/test/fixtures/local_album.stub.dart index 76cc3a570a..32d25ce254 100644 --- a/mobile/test/fixtures/local_album.stub.dart +++ b/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, + ); }