diff --git a/mobile/lib/domain/services/sync.service.dart b/mobile/lib/domain/services/sync.service.dart index 7a059fdbbb..0dd6f9b9b5 100644 --- a/mobile/lib/domain/services/sync.service.dart +++ b/mobile/lib/domain/services/sync.service.dart @@ -5,7 +5,8 @@ import 'package:immich_mobile/domain/interfaces/album_media.interface.dart'; import 'package:immich_mobile/domain/interfaces/local_album.interface.dart'; import 'package:immich_mobile/domain/interfaces/local_album_asset.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/asset/asset.model.dart' + hide AssetType; import 'package:immich_mobile/domain/models/local_album.model.dart'; import 'package:immich_mobile/utils/diff.dart'; import 'package:logging/logging.dart'; @@ -114,18 +115,43 @@ class SyncService { } } - Future removeLocalAlbum(LocalAlbum album) async { - _log.info("Removing device album ${album.name}"); + Future removeLocalAlbum(LocalAlbum a) async { + _log.info("Removing device album ${a.name}"); try { + // Do not request title to speed things up on iOS + final filter = albumFilter; + filter.setOption( + AssetType.image, + filter.getOption(AssetType.image).copyWith(needTitle: false), + ); + filter.setOption( + AssetType.video, + filter.getOption(AssetType.video).copyWith(needTitle: false), + ); + final deviceAlbum = + await _albumMediaRepository.refresh(a.id, filter: filter); + final assetsToDelete = + (await _localAlbumAssetRepository.getAssetsForAlbum(deviceAlbum.id)) + .map((asset) => asset.localId) + .toSet(); + // Remove all assets that are only in this particular album // We cannot remove all assets in the album because they might be in other albums in iOS - final assetsToDelete = - await _localAlbumRepository.getAssetIdsOnlyInAlbum(album.id); + final assetsOnlyInAlbum = assetsToDelete.isEmpty + ? {} + : (await _localAlbumRepository.getAssetIdsOnlyInAlbum(deviceAlbum.id)) + .toSet(); await _localAlbumRepository.transaction(() async { - if (assetsToDelete.isNotEmpty) { - await _localAssetRepository.deleteIds(assetsToDelete); - } - await _localAlbumRepository.delete(album.id); + // Delete all assets that are only in this particular album + await _localAssetRepository.deleteIds( + assetsToDelete.intersection(assetsOnlyInAlbum), + ); + // Unlink the others + await _localAlbumAssetRepository.unlinkAssetsFromAlbum( + deviceAlbum.id, + assetsToDelete.difference(assetsOnlyInAlbum), + ); + await _localAlbumRepository.delete(deviceAlbum.id); }); } catch (e, s) { _log.warning("Error while removing device album", e, s); @@ -312,7 +338,8 @@ class SyncService { backupSelection: dbAlbum.backupSelection, ); - // Only query for assets unique to album if we have assets to delete + // Remove all assets that are only in this particular album + // We cannot remove all assets in the album because they might be in other albums in iOS final assetsOnlyInAlbum = assetsToDelete.isEmpty ? {} : (await _localAlbumRepository.getAssetIdsOnlyInAlbum(deviceAlbum.id)) @@ -326,8 +353,7 @@ class SyncService { assetsToAdd.map((a) => a.localId), ); await _localAlbumRepository.upsert(updatedAlbum); - // Remove all assets that are only in this particular album - // We cannot remove all assets in the album because they might be in other albums in iOS + // Delete all assets that are only in this particular album await _localAssetRepository.deleteIds( assetsToDelete.intersection(assetsOnlyInAlbum), ); diff --git a/mobile/test/domain/services/sync_service_test.dart b/mobile/test/domain/services/sync_service_test.dart index 3c7f253743..ed8a5a167f 100644 --- a/mobile/test/domain/services/sync_service_test.dart +++ b/mobile/test/domain/services/sync_service_test.dart @@ -1,3 +1,4 @@ +import 'package:collection/collection.dart'; import 'package:flutter_test/flutter_test.dart'; import 'package:immich_mobile/domain/interfaces/album_media.interface.dart'; import 'package:immich_mobile/domain/interfaces/local_album.interface.dart'; @@ -120,8 +121,16 @@ void main() { when(() => mockLocalAlbumAssetRepo.linkAssetsToAlbum(any(), any())) .thenAnswer((_) async => {}); + when(() => mockLocalAlbumAssetRepo.unlinkAssetsFromAlbum(any(), any())) + .thenAnswer((_) async => {}); + when(() => mockLocalAlbumRepo.upsert(any())).thenAnswer((_) async => {}); + when(() => mockLocalAlbumRepo.delete(any())).thenAnswer((_) async => {}); + + when(() => mockLocalAssetRepo.deleteIds(any())) + .thenAnswer((_) async => {}); + when(() => mockLocalAlbumRepo.transaction(any())) .thenAnswer((_) async { final capturedCallback = verify( @@ -136,28 +145,18 @@ void main() { test( 'album filter should be properly configured with expected settings', () { - // Access the album filter from the service final albumFilter = sut.albumFilter; - // Verify image option settings final imageOption = albumFilter.getOption(AssetType.image); expect(imageOption.needTitle, isTrue); expect(imageOption.sizeConstraint.ignoreSize, isTrue); - - // Verify video option settings final videoOption = albumFilter.getOption(AssetType.video); expect(videoOption.needTitle, isTrue); expect(videoOption.sizeConstraint.ignoreSize, isTrue); expect(videoOption.durationConstraint.allowNullable, isTrue); - - // Verify containsPathModified flag expect(albumFilter.containsPathModified, isTrue); - - // Verify time conditions are ignored expect(albumFilter.createTimeCond.ignore, isTrue); expect(albumFilter.updateTimeCond.ignore, isTrue); - - // Verify ordering expect(albumFilter.orders.length, 1); expect( albumFilter.orders.firstOrNull?.type, @@ -183,7 +182,6 @@ void main() { deviceAlbum.copyWith(updatedAt: earlier), ); - // Verify: method returns without making any changes expect(result, isFalse); verifyNever(() => mockAlbumMediaRepo.getAssetsForAlbum(any())); verifyNever(() => mockLocalAssetRepo.upsertAll(any())); @@ -197,7 +195,6 @@ void main() { test( 'early return when device album has fewer assets than DB album', () async { - // Execute final result = await sut.handleOnlyAssetsAdded( dbAlbum, deviceAlbum.copyWith(assetCount: dbAlbum.assetCount - 1), @@ -345,9 +342,9 @@ void main() { verify(() => mockLocalAlbumRepo.upsert(captureAny())); albumUpsertCall.called(1); verify(() => mockLocalAssetRepo.upsertAll(any())).called(1); - verify( - () => mockLocalAlbumAssetRepo.linkAssetsToAlbum(albumId, any()), - ).called(1); + verify(() => + mockLocalAlbumAssetRepo.linkAssetsToAlbum(albumId, any())) + .called(1); final capturedAlbum = albumUpsertCall.captured.singleOrNull as LocalAlbum?; @@ -357,5 +354,83 @@ void main() { }, ); }); + + group('removeLocalAlbum: ', () { + test( + 'removing album with no assets correctly calls delete', + () async { + when(() => mockLocalAlbumAssetRepo.getAssetsForAlbum(albumId)) + .thenAnswer((_) async => []); + when(() => mockLocalAlbumRepo.getAssetIdsOnlyInAlbum(albumId)) + .thenAnswer((_) async => []); + + await sut.removeLocalAlbum(deviceAlbum); + + verify(() => mockLocalAssetRepo.deleteIds([])).called(1); + verify(() => mockLocalAlbumRepo.delete(albumId)).called(1); + verify( + () => mockLocalAlbumAssetRepo.unlinkAssetsFromAlbum(albumId, {}), + ).called(1); + }, + ); + + test( + 'removing album with assets unique to that album deletes those assets', + () async { + final uniqueAssetIds = deviceAssets.map((a) => a.localId).toList(); + when(() => mockLocalAlbumRepo.getAssetIdsOnlyInAlbum(albumId)) + .thenAnswer((_) async => uniqueAssetIds); + + await sut.removeLocalAlbum(deviceAlbum); + + verify(() => mockLocalAssetRepo.deleteIds(uniqueAssetIds)).called(1); + verify(() => mockLocalAlbumRepo.delete(albumId)).called(1); + verify(() => mockLocalAlbumAssetRepo.unlinkAssetsFromAlbum(any(), {})) + .called(1); + }, + ); + + test( + 'removing album with shared assets unlinks those assets', + () async { + final assetIds = deviceAssets.map((a) => a.localId).toList(); + when(() => mockLocalAlbumRepo.getAssetIdsOnlyInAlbum(albumId)) + .thenAnswer((_) async => []); + + await sut.removeLocalAlbum(deviceAlbum); + + verify(() => mockLocalAssetRepo.deleteIds([])).called(1); + verify( + () => mockLocalAlbumAssetRepo.unlinkAssetsFromAlbum( + albumId, + assetIds, + ), + ).called(1); + verify(() => mockLocalAlbumRepo.delete(albumId)).called(1); + }, + ); + + test( + 'removing album with mixed assets (some unique, some shared)', + () async { + final uniqueAssetIds = ['asset-1', 'asset-2']; + final sharedAssetIds = ['asset-0', 'asset-3', 'asset-4']; + + when(() => mockLocalAlbumRepo.getAssetIdsOnlyInAlbum(albumId)) + .thenAnswer((_) async => uniqueAssetIds); + + await sut.removeLocalAlbum(deviceAlbum); + + verify(() => mockLocalAssetRepo.deleteIds(uniqueAssetIds)).called(1); + verify( + () => mockLocalAlbumAssetRepo.unlinkAssetsFromAlbum( + albumId, + sharedAssetIds, + ), + ).called(1); + verify(() => mockLocalAlbumRepo.delete(albumId)).called(1); + }, + ); + }); }); }