From 1120caca1099d47338775fce5bff57903b993722 Mon Sep 17 00:00:00 2001 From: Santo Shakil Date: Sat, 9 May 2026 00:33:04 +0600 Subject: [PATCH 1/3] fix(mobile): self-heal stale linked album cache on 400 if the server forgets an album that mobile still has cached, every upload hits 400 on addAssets and spams severe forever. catch that 400, drop the cache row, fk cascade nulls the link. next manage pass recreates or re-links by name. --- .../services/sync_linked_album.service.dart | 43 ++++++++++++------- .../drift_album_api_repository.dart | 33 ++++++++++---- 2 files changed, 51 insertions(+), 25 deletions(-) diff --git a/mobile/lib/domain/services/sync_linked_album.service.dart b/mobile/lib/domain/services/sync_linked_album.service.dart index 3bc76083b8..ff60e0ef74 100644 --- a/mobile/lib/domain/services/sync_linked_album.service.dart +++ b/mobile/lib/domain/services/sync_linked_album.service.dart @@ -39,24 +39,35 @@ class SyncLinkedAlbumService { await Future.wait( selectedAlbums.map((localAlbum) async { - final linkedRemoteAlbumId = localAlbum.linkedRemoteAlbumId; - if (linkedRemoteAlbumId == null) { - _log.warning("No linked remote album ID found for local album: ${localAlbum.name}"); - return; - } + try { + final linkedRemoteAlbumId = localAlbum.linkedRemoteAlbumId; + if (linkedRemoteAlbumId == null) { + _log.warning("No linked remote album ID found for local album: ${localAlbum.name}"); + return; + } - final remoteAlbum = await _remoteAlbumRepository.get(linkedRemoteAlbumId); - if (remoteAlbum == null) { - _log.warning("Linked remote album not found for ID: $linkedRemoteAlbumId"); - return; - } + final remoteAlbum = await _remoteAlbumRepository.get(linkedRemoteAlbumId); + if (remoteAlbum == null) { + _log.warning("Linked remote album not found for ID: $linkedRemoteAlbumId"); + return; + } - // get assets that are uploaded but not in the remote album - final assetIds = await _remoteAlbumRepository.getLinkedAssetIds(userId, localAlbum.id, linkedRemoteAlbumId); - _log.fine("Syncing ${assetIds.length} assets to remote album: ${remoteAlbum.name}"); - if (assetIds.isNotEmpty) { - final album = await _albumApiRepository.addAssets(remoteAlbum.id, assetIds); - await _remoteAlbumRepository.addAssets(remoteAlbum.id, album.added); + // get assets that are uploaded but not in the remote album + final assetIds = await _remoteAlbumRepository.getLinkedAssetIds(userId, localAlbum.id, linkedRemoteAlbumId); + _log.fine("Syncing ${assetIds.length} assets to remote album: ${remoteAlbum.name}"); + if (assetIds.isNotEmpty) { + final album = await _albumApiRepository.addAssets(remoteAlbum.id, assetIds); + await _remoteAlbumRepository.addAssets(remoteAlbum.id, album.added); + } + } on RemoteAlbumNotFoundException catch (e) { + // server doesn't have the linked album anymore. drop the cached row; + // KeyAction.setNull on LocalAlbumEntity.linkedRemoteAlbumId nulls + // the link via FK cascade, and the next manageLinkedAlbums run + // will recreate or re-link by name. + _log.warning("Pruning stale linked album for ${localAlbum.name} (server returned 'Album not found' for ${e.albumId})"); + await _remoteAlbumRepository.deleteAlbum(e.albumId); + } catch (error, stack) { + _log.severe("Linked album sync failed for ${localAlbum.name}", error, stack); } }), ); diff --git a/mobile/lib/repositories/drift_album_api_repository.dart b/mobile/lib/repositories/drift_album_api_repository.dart index a0c7a3732a..2ae6e23891 100644 --- a/mobile/lib/repositories/drift_album_api_repository.dart +++ b/mobile/lib/repositories/drift_album_api_repository.dart @@ -42,17 +42,24 @@ class DriftAlbumApiRepository extends ApiRepository { } Future<({List added, List failed})> addAssets(String albumId, Iterable assetIds) async { - final response = await checkNull(_api.addAssetsToAlbum(albumId, BulkIdsDto(ids: assetIds.toList()))); - final List added = [], failed = []; - for (final dto in response) { - if (dto.success) { - added.add(dto.id); - } else { - failed.add(dto.id); + try { + final response = await checkNull(_api.addAssetsToAlbum(albumId, BulkIdsDto(ids: assetIds.toList()))); + final List added = [], failed = []; + for (final dto in response) { + if (dto.success) { + added.add(dto.id); + } else { + failed.add(dto.id); + } } - } - return (added: added, failed: failed); + return (added: added, failed: failed); + } on ApiException catch (e) { + if (e.code == 400 && (e.message?.contains('"message":"Album not found"') ?? false)) { + throw RemoteAlbumNotFoundException(albumId); + } + rethrow; + } } Future updateAlbum( @@ -104,6 +111,14 @@ class DriftAlbumApiRepository extends ApiRepository { } } +class RemoteAlbumNotFoundException implements Exception { + final String albumId; + const RemoteAlbumNotFoundException(this.albumId); + + @override + String toString() => 'RemoteAlbumNotFoundException: $albumId'; +} + extension on AlbumResponseDto { RemoteAlbum toRemoteAlbum(final UserDto user) { return RemoteAlbum( From 97430bec14ff8ddf3fab46ed2ff3e7c5f2f0a1ab Mon Sep 17 00:00:00 2001 From: Santo Shakil Date: Sat, 9 May 2026 00:46:09 +0600 Subject: [PATCH 2/3] fix(mobile): wrap long log line for dart format --- mobile/lib/domain/services/sync_linked_album.service.dart | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/mobile/lib/domain/services/sync_linked_album.service.dart b/mobile/lib/domain/services/sync_linked_album.service.dart index ff60e0ef74..2f0004c505 100644 --- a/mobile/lib/domain/services/sync_linked_album.service.dart +++ b/mobile/lib/domain/services/sync_linked_album.service.dart @@ -64,7 +64,9 @@ class SyncLinkedAlbumService { // KeyAction.setNull on LocalAlbumEntity.linkedRemoteAlbumId nulls // the link via FK cascade, and the next manageLinkedAlbums run // will recreate or re-link by name. - _log.warning("Pruning stale linked album for ${localAlbum.name} (server returned 'Album not found' for ${e.albumId})"); + _log.warning( + "Pruning stale linked album for ${localAlbum.name} (server returned 'Album not found' for ${e.albumId})", + ); await _remoteAlbumRepository.deleteAlbum(e.albumId); } catch (error, stack) { _log.severe("Linked album sync failed for ${localAlbum.name}", error, stack); From 8d918a65ef354dc8d47773e0cde3df8da180f3e7 Mon Sep 17 00:00:00 2001 From: Santo Shakil Date: Mon, 11 May 2026 21:03:57 +0600 Subject: [PATCH 3/3] fix(mobile): server side check in manageLinkedAlbums + wire to organize mobile cache only trusts SyncStream events. if the server looses an album silently (audit table empty) manage never spots the stale id. now fetch getAllOwned first, prune the cached row, then recreate or relink by name. also the organize button only ran sync passes before. now it runs manageLinkedAlbums too so toggle off and on actually reconciles the cache. added unit tests for both paths. verifed on pixel 9a with a sql soft delete on a local v3 server, audit row stays empty just like users 'clean reinstall' case. --- .../services/sync_linked_album.service.dart | 73 ++++--- .../drift_album_api_repository.dart | 5 + .../drift_backup_settings.dart | 1 + .../sync_linked_album_service_test.dart | 186 ++++++++++++++++++ 4 files changed, 235 insertions(+), 30 deletions(-) create mode 100644 mobile/test/domain/services/sync_linked_album_service_test.dart 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())); + }); + }); +}