diff --git a/mobile/lib/domain/services/sync_linked_album.service.dart b/mobile/lib/domain/services/sync_linked_album.service.dart index 2f0004c505..493a090cf1 100644 --- a/mobile/lib/domain/services/sync_linked_album.service.dart +++ b/mobile/lib/domain/services/sync_linked_album.service.dart @@ -1,4 +1,5 @@ import 'package:hooks_riverpod/hooks_riverpod.dart'; +import 'package:immich_mobile/domain/models/album/album.model.dart'; import 'package:immich_mobile/domain/models/album/local_album.model.dart'; import 'package:immich_mobile/domain/models/store.model.dart'; import 'package:immich_mobile/domain/services/store.service.dart'; @@ -76,50 +77,62 @@ class SyncLinkedAlbumService { } Future manageLinkedAlbums(List localAlbums, String ownerId) async { + // fetch the server's authoritative owned-album list once and reconcile each + // local album against it. trusting only the local cache (previous behaviour) + // misses the case where the server lost an album that mobile still has + // cached (volume reset, soft-deleted user, etc). + final List serverAlbums; + try { + serverAlbums = await _albumApiRepository.getAllOwned(_storeService.get(StoreKey.currentUser)); + } catch (error, stackTrace) { + // soft-fail on network / server error so a flaky link doesn't destroy local state + _log.severe("Could not fetch server albums; deferring manageLinkedAlbums", error, stackTrace); + return; + } + + final serverById = {for (final a in serverAlbums) a.id: a}; + final serverByName = {for (final a in serverAlbums) a.name: a}; + try { for (final album in localAlbums) { - await _processLocalAlbum(album, ownerId); + await _processLocalAlbum(album, serverById, serverByName); } } catch (error, stackTrace) { _log.severe("Error managing linked albums", error, stackTrace); } } - /// Processes a single local album to ensure proper linking with remote albums - Future _processLocalAlbum(LocalAlbum localAlbum, String ownerId) { - final hasLinkedRemoteAlbum = localAlbum.linkedRemoteAlbumId != null; + /// Reconciles a single local album against the server's owned-album list. + Future _processLocalAlbum( + LocalAlbum localAlbum, + Map serverById, + Map serverByName, + ) async { + final linkedId = localAlbum.linkedRemoteAlbumId; + if (linkedId != null && serverById.containsKey(linkedId)) { + return; + } + if (linkedId != null) { + // server doesn't have this album anymore. drop the cached row; KeyAction.setNull + // on LocalAlbumEntity.linkedRemoteAlbumId nulls the link via FK cascade. + await _remoteAlbumRepository.deleteAlbum(linkedId); + } - if (hasLinkedRemoteAlbum) { - return _handleLinkedAlbum(localAlbum); + final byNameMatch = serverByName[localAlbum.name]; + if (byNameMatch != null) { + await _linkToExistingRemoteAlbum(localAlbum, byNameMatch); } else { - return _handleUnlinkedAlbum(localAlbum, ownerId); + await _createAndLinkNewRemoteAlbum(localAlbum); } } - /// Handles albums that are already linked to a remote album - Future _handleLinkedAlbum(LocalAlbum localAlbum) async { - final remoteAlbumId = localAlbum.linkedRemoteAlbumId!; - final remoteAlbum = await _remoteAlbumRepository.get(remoteAlbumId); - - final remoteAlbumExists = remoteAlbum != null; - if (!remoteAlbumExists) { - return _localAlbumRepository.unlinkRemoteAlbum(localAlbum.id); + /// Links a local album to an existing remote album, ensuring the cache row exists + /// so subsequent [syncLinkedAlbums] passes can find it without waiting for sync stream. + Future _linkToExistingRemoteAlbum(LocalAlbum localAlbum, RemoteAlbum existingRemoteAlbum) async { + final cached = await _remoteAlbumRepository.get(existingRemoteAlbum.id); + if (cached == null) { + await _remoteAlbumRepository.create(existingRemoteAlbum, []); } - } - - /// Handles albums that are not linked to any remote album - Future _handleUnlinkedAlbum(LocalAlbum localAlbum, String ownerId) async { - final existingRemoteAlbum = await _remoteAlbumRepository.getByName(localAlbum.name, ownerId); - - if (existingRemoteAlbum != null) { - return _linkToExistingRemoteAlbum(localAlbum, existingRemoteAlbum); - } else { - return _createAndLinkNewRemoteAlbum(localAlbum); - } - } - - /// Links a local album to an existing remote album - Future _linkToExistingRemoteAlbum(LocalAlbum localAlbum, dynamic existingRemoteAlbum) { return _localAlbumRepository.linkRemoteAlbum(localAlbum.id, existingRemoteAlbum.id); } diff --git a/mobile/lib/repositories/drift_album_api_repository.dart b/mobile/lib/repositories/drift_album_api_repository.dart index 2ae6e23891..0ea41c815b 100644 --- a/mobile/lib/repositories/drift_album_api_repository.dart +++ b/mobile/lib/repositories/drift_album_api_repository.dart @@ -15,6 +15,11 @@ class DriftAlbumApiRepository extends ApiRepository { DriftAlbumApiRepository(this._api); + Future> getAllOwned(UserDto owner) async { + final response = await checkNull(_api.getAllAlbums(isOwned: true)); + return response.map((dto) => dto.toRemoteAlbum(owner)).toList(); + } + Future createDriftAlbum( String name, UserDto owner, { diff --git a/mobile/lib/widgets/settings/backup_settings/drift_backup_settings.dart b/mobile/lib/widgets/settings/backup_settings/drift_backup_settings.dart index 2c179c42ea..a10a861161 100644 --- a/mobile/lib/widgets/settings/backup_settings/drift_backup_settings.dart +++ b/mobile/lib/widgets/settings/backup_settings/drift_backup_settings.dart @@ -69,6 +69,7 @@ class _AlbumSyncActionButtonState extends ConsumerState<_AlbumSyncActionButton> }); try { + await _manageLinkedAlbums(); await ref.read(backgroundSyncProvider).syncLinkedAlbum(); await ref.read(backgroundSyncProvider).syncRemote(); } catch (_) { diff --git a/mobile/test/domain/services/sync_linked_album_service_test.dart b/mobile/test/domain/services/sync_linked_album_service_test.dart new file mode 100644 index 0000000000..37d5629d5d --- /dev/null +++ b/mobile/test/domain/services/sync_linked_album_service_test.dart @@ -0,0 +1,186 @@ +import 'package:drift/drift.dart' as drift; +import 'package:drift/native.dart'; +import 'package:flutter/foundation.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:immich_mobile/domain/models/album/album.model.dart'; +import 'package:immich_mobile/domain/models/album/local_album.model.dart'; +import 'package:immich_mobile/domain/models/store.model.dart'; +import 'package:immich_mobile/domain/services/store.service.dart'; +import 'package:immich_mobile/domain/services/sync_linked_album.service.dart'; +import 'package:immich_mobile/entities/store.entity.dart'; +import 'package:immich_mobile/infrastructure/repositories/db.repository.dart'; +import 'package:immich_mobile/infrastructure/repositories/store.repository.dart'; +import 'package:immich_mobile/repositories/drift_album_api_repository.dart'; +import 'package:mocktail/mocktail.dart'; +import 'package:openapi/api.dart'; + +import '../../fixtures/album.stub.dart'; +import '../../fixtures/user.stub.dart'; +import '../../infrastructure/repository.mock.dart'; + +RemoteAlbum _remoteAlbumFor(LocalAlbum local, {required String id}) => RemoteAlbum( + id: id, + name: local.name, + ownerId: UserStub.admin.id, + ownerName: UserStub.admin.name, + description: '', + createdAt: DateTime(2024), + updatedAt: DateTime(2024), + isActivityEnabled: true, + order: AlbumAssetOrder.desc, + assetCount: 0, + isShared: false, +); + +LocalAlbum _localAlbum({required String id, required String name, String? linkedRemoteAlbumId}) => LocalAlbum( + id: id, + name: name, + updatedAt: DateTime(2024), + assetCount: 5, + backupSelection: BackupSelection.selected, + isIosSharedAlbum: false, + linkedRemoteAlbumId: linkedRemoteAlbumId, +); + +void main() { + late SyncLinkedAlbumService sut; + late MockLocalAlbumRepository mockLocalAlbumRepo; + late MockRemoteAlbumRepository mockRemoteAlbumRepo; + late MockDriftAlbumApiRepository mockAlbumApiRepo; + late Drift db; + + setUpAll(() async { + TestWidgetsFlutterBinding.ensureInitialized(); + debugDefaultTargetPlatformOverride = TargetPlatform.android; + db = Drift(drift.DatabaseConnection(NativeDatabase.memory(), closeStreamsSynchronously: true)); + await StoreService.init(storeRepository: DriftStoreRepository(db)); + await Store.put(StoreKey.currentUser, UserStub.admin); + registerFallbackValue(LocalAlbumStub.recent); + registerFallbackValue(UserStub.admin); + registerFallbackValue( + RemoteAlbum( + id: 'fallback', + name: 'fallback', + ownerId: 'u', + ownerName: 'u', + description: '', + createdAt: DateTime(2024), + updatedAt: DateTime(2024), + isActivityEnabled: true, + order: AlbumAssetOrder.desc, + assetCount: 0, + isShared: false, + ), + ); + }); + + tearDownAll(() async { + debugDefaultTargetPlatformOverride = null; + await Store.clear(); + await db.close(); + }); + + setUp(() { + mockLocalAlbumRepo = MockLocalAlbumRepository(); + mockRemoteAlbumRepo = MockRemoteAlbumRepository(); + mockAlbumApiRepo = MockDriftAlbumApiRepository(); + + sut = SyncLinkedAlbumService(mockLocalAlbumRepo, mockRemoteAlbumRepo, mockAlbumApiRepo, StoreService.I); + + when(() => mockLocalAlbumRepo.linkRemoteAlbum(any(), any())).thenAnswer((_) async {}); + when(() => mockLocalAlbumRepo.unlinkRemoteAlbum(any())).thenAnswer((_) async {}); + when(() => mockRemoteAlbumRepo.deleteAlbum(any())).thenAnswer((_) async {}); + when(() => mockRemoteAlbumRepo.create(any(), any())).thenAnswer((_) async {}); + }); + + group('manageLinkedAlbums', () { + test('soft-fails when server fetch throws, no destructive writes', () async { + final local = _localAlbum(id: 'l1', name: 'Movies', linkedRemoteAlbumId: 'stale'); + when(() => mockAlbumApiRepo.getAllOwned(any())).thenThrow(ApiException(503, 'down')); + + await sut.manageLinkedAlbums([local], UserStub.admin.id); + + verifyNever(() => mockRemoteAlbumRepo.deleteAlbum(any())); + verifyNever(() => mockLocalAlbumRepo.linkRemoteAlbum(any(), any())); + verifyNever(() => mockAlbumApiRepo.createDriftAlbum(any(), any(), assetIds: any(named: 'assetIds'))); + }); + + test('no-op when linked album still exists on server', () async { + final local = _localAlbum(id: 'l1', name: 'Movies', linkedRemoteAlbumId: 'r1'); + final remote = _remoteAlbumFor(local, id: 'r1'); + when(() => mockAlbumApiRepo.getAllOwned(any())).thenAnswer((_) async => [remote]); + + await sut.manageLinkedAlbums([local], UserStub.admin.id); + + verifyNever(() => mockRemoteAlbumRepo.deleteAlbum(any())); + verifyNever(() => mockLocalAlbumRepo.linkRemoteAlbum(any(), any())); + verifyNever(() => mockAlbumApiRepo.createDriftAlbum(any(), any(), assetIds: any(named: 'assetIds'))); + }); + + test('prunes stale link when server no longer has the album', () async { + final local = _localAlbum(id: 'l1', name: 'Movies', linkedRemoteAlbumId: 'stale-id'); + when(() => mockAlbumApiRepo.getAllOwned(any())).thenAnswer((_) async => []); + when( + () => mockAlbumApiRepo.createDriftAlbum(any(), any(), assetIds: any(named: 'assetIds')), + ).thenAnswer((_) async => _remoteAlbumFor(local, id: 'new-id')); + + await sut.manageLinkedAlbums([local], UserStub.admin.id); + + verify(() => mockRemoteAlbumRepo.deleteAlbum('stale-id')).called(1); + verify(() => mockAlbumApiRepo.createDriftAlbum('Movies', UserStub.admin, assetIds: [])).called(1); + }); + + test('links to existing server album by name when unlinked', () async { + final local = _localAlbum(id: 'l1', name: 'Movies'); + final existing = _remoteAlbumFor(local, id: 'r-existing'); + when(() => mockAlbumApiRepo.getAllOwned(any())).thenAnswer((_) async => [existing]); + when(() => mockRemoteAlbumRepo.get('r-existing')).thenAnswer((_) async => null); + + await sut.manageLinkedAlbums([local], UserStub.admin.id); + + verify(() => mockRemoteAlbumRepo.create(existing, [])).called(1); + verify(() => mockLocalAlbumRepo.linkRemoteAlbum('l1', 'r-existing')).called(1); + verifyNever(() => mockAlbumApiRepo.createDriftAlbum(any(), any(), assetIds: any(named: 'assetIds'))); + }); + + test('creates a new remote album when no match on server', () async { + final local = _localAlbum(id: 'l1', name: 'Movies'); + final created = _remoteAlbumFor(local, id: 'r-new'); + when(() => mockAlbumApiRepo.getAllOwned(any())).thenAnswer((_) async => []); + when( + () => mockAlbumApiRepo.createDriftAlbum(any(), any(), assetIds: any(named: 'assetIds')), + ).thenAnswer((_) async => created); + + await sut.manageLinkedAlbums([local], UserStub.admin.id); + + verify(() => mockAlbumApiRepo.createDriftAlbum('Movies', UserStub.admin, assetIds: [])).called(1); + verify(() => mockRemoteAlbumRepo.create(created, [])).called(1); + verify(() => mockLocalAlbumRepo.linkRemoteAlbum('l1', 'r-new')).called(1); + }); + }); + + group('syncLinkedAlbums', () { + test('prunes cache row when addAssets throws RemoteAlbumNotFoundException', () async { + final local = _localAlbum(id: 'l1', name: 'Movies', linkedRemoteAlbumId: 'r-stale'); + final remote = _remoteAlbumFor(local, id: 'r-stale'); + when(() => mockLocalAlbumRepo.getBackupAlbums()).thenAnswer((_) async => [local]); + when(() => mockRemoteAlbumRepo.get('r-stale')).thenAnswer((_) async => remote); + when(() => mockRemoteAlbumRepo.getLinkedAssetIds(any(), any(), any())).thenAnswer((_) async => ['a1']); + when(() => mockAlbumApiRepo.addAssets('r-stale', any())).thenThrow(const RemoteAlbumNotFoundException('r-stale')); + + await sut.syncLinkedAlbums(UserStub.admin.id); + + verify(() => mockRemoteAlbumRepo.deleteAlbum('r-stale')).called(1); + }); + + test('skips albums with null linked id without server calls', () async { + final local = _localAlbum(id: 'l1', name: 'Movies'); + when(() => mockLocalAlbumRepo.getBackupAlbums()).thenAnswer((_) async => [local]); + + await sut.syncLinkedAlbums(UserStub.admin.id); + + verifyNever(() => mockAlbumApiRepo.addAssets(any(), any())); + verifyNever(() => mockRemoteAlbumRepo.deleteAlbum(any())); + }); + }); +}