From db60295e63a18bd9985c8f716309d767a181c9b3 Mon Sep 17 00:00:00 2001 From: shenlong-tanwen <139912620+shalong-tanwen@users.noreply.github.com> Date: Wed, 29 Jul 2026 01:00:29 +0530 Subject: [PATCH 1/5] refactor: reenable cloud ids --- mobile/ios/Runner/Sync/MessagesImpl.swift | 34 +++- .../services/background_worker.service.dart | 1 - mobile/lib/domain/services/hash.service.dart | 2 +- .../domain/services/local_sync.service.dart | 34 +--- .../lib/domain/utils/cloud_id_resolver.dart | 46 ++++++ .../lib/domain/utils/migrate_cloud_ids.dart | 155 +++++++++--------- .../repositories/local_asset.repository.dart | 5 - .../lib/pages/common/splash_screen.page.dart | 3 +- .../providers/app_life_cycle.provider.dart | 3 +- .../infrastructure/sync.provider.dart | 1 - mobile/pigeon/native_sync_api.dart | 14 +- .../services/local_sync_service_test.dart | 4 - .../local_asset_repository_test.dart | 4 +- mobile/test/medium/repository_context.dart | 3 +- .../medium/utils/migrate_cloud_ids_test.dart | 113 +++++++++++++ 15 files changed, 294 insertions(+), 128 deletions(-) create mode 100644 mobile/lib/domain/utils/cloud_id_resolver.dart create mode 100644 mobile/test/medium/utils/migrate_cloud_ids_test.dart diff --git a/mobile/ios/Runner/Sync/MessagesImpl.swift b/mobile/ios/Runner/Sync/MessagesImpl.swift index ddfd023690..5424721143 100644 --- a/mobile/ios/Runner/Sync/MessagesImpl.swift +++ b/mobile/ios/Runner/Sync/MessagesImpl.swift @@ -453,11 +453,30 @@ class NativeSyncApiImpl: ImmichPlugin, NativeSyncApi, FlutterPlugin { } } + + private func cloudIdErrorKind(for error: Error) -> CloudIdErrorKind { + let nsError = error as NSError + guard nsError.domain == PHPhotosErrorDomain else { + return .unknown + } + + switch nsError.code { + case PHPhotosError.identifierNotFound.rawValue: + return .notFound + case PHPhotosError.multipleIdentifiersFound.rawValue: + return .ambiguous + default: + return .unknown + } + } + func getCloudIdForAssetIds(assetIds: [String]) throws -> [CloudIdResult] { guard #available(iOS 16, *) else { - return assetIds.map { CloudIdResult(assetId: $0) } + return assetIds.map { + CloudIdResult(assetId: $0, error: "Cloud identifiers require iOS 16", errorKind: .unsupported) + } } - + var mappings: [CloudIdResult] = [] let result = PHPhotoLibrary.shared().cloudIdentifierMappings(forLocalIdentifiers: assetIds) for (key, value) in result { @@ -468,10 +487,17 @@ class NativeSyncApiImpl: ImmichPlugin, NativeSyncApi, FlutterPlugin { if !cloudId.hasSuffix(":") { mappings.append(CloudIdResult(assetId: key, cloudId: cloudId)) } else { - mappings.append(CloudIdResult(assetId: key, error: "Incomplete Cloud Id: \(cloudId)")) + mappings.append( + CloudIdResult(assetId: key, error: "Incomplete Cloud Id: \(cloudId)", errorKind: .incomplete)) } case .failure(let error): - mappings.append(CloudIdResult(assetId: key, error: "Error getting Cloud Id: \(error.localizedDescription)")) + let kind = cloudIdErrorKind(for: error) + var message = "Error getting Cloud Id: \(error.localizedDescription)" + if kind == .ambiguous, + let matches = (error as NSError).userInfo[PHLocalIdentifiersErrorKey] as? [String] { + message += " (matched: \(matches.joined(separator: ", ")))" + } + mappings.append(CloudIdResult(assetId: key, error: message, errorKind: kind)) } } return mappings; diff --git a/mobile/lib/domain/services/background_worker.service.dart b/mobile/lib/domain/services/background_worker.service.dart index 8161df5c51..b902f0f0f4 100644 --- a/mobile/lib/domain/services/background_worker.service.dart +++ b/mobile/lib/domain/services/background_worker.service.dart @@ -78,7 +78,6 @@ class BackgroundWorkerBgService extends BackgroundWorkerFlutterApi { _ref = ref; _localSyncService = LocalSyncService( localAlbumRepository: ref.read(localAlbumRepository), - localAssetRepository: ref.read(localAssetRepository), nativeSyncApi: ref.read(nativeSyncApiProvider), trashedLocalAssetRepository: ref.read(trashedLocalAssetRepository), assetMediaRepository: ref.read(assetMediaRepositoryProvider), diff --git a/mobile/lib/domain/services/hash.service.dart b/mobile/lib/domain/services/hash.service.dart index 8a573ee209..a6f87ae30d 100644 --- a/mobile/lib/domain/services/hash.service.dart +++ b/mobile/lib/domain/services/hash.service.dart @@ -42,7 +42,7 @@ class HashService { final Stopwatch stopwatch = Stopwatch()..start(); try { // Migrate hashes from cloud ID to local ID so we don't have to re-hash them - // await _localAssetRepository.reconcileHashesFromCloudId(); + await _localAssetRepository.reconcileHashesFromCloudId(); // Sorted by backupSelection followed by isCloud final localAlbums = await _localAlbumRepository.getBackupAlbums(); diff --git a/mobile/lib/domain/services/local_sync.service.dart b/mobile/lib/domain/services/local_sync.service.dart index 9e272ac65f..3abae181fe 100644 --- a/mobile/lib/domain/services/local_sync.service.dart +++ b/mobile/lib/domain/services/local_sync.service.dart @@ -6,10 +6,10 @@ import 'package:flutter/services.dart'; import 'package:immich_mobile/domain/models/album/local_album.model.dart'; import 'package:immich_mobile/domain/models/asset/base_asset.model.dart'; import 'package:immich_mobile/domain/models/store.model.dart'; +import 'package:immich_mobile/domain/utils/cloud_id_resolver.dart'; import 'package:immich_mobile/entities/store.entity.dart'; import 'package:immich_mobile/extensions/platform_extensions.dart'; import 'package:immich_mobile/infrastructure/repositories/local_album.repository.dart'; -import 'package:immich_mobile/infrastructure/repositories/local_asset.repository.dart'; import 'package:immich_mobile/infrastructure/repositories/trashed_local_asset.repository.dart'; import 'package:immich_mobile/platform/native_sync_api.g.dart'; import 'package:immich_mobile/repositories/asset_media.repository.dart'; @@ -22,8 +22,6 @@ const String _kSyncCancelledCode = "SYNC_CANCELLED"; class LocalSyncService { final DriftLocalAlbumRepository _localAlbumRepository; - // ignore: unused_field - final DriftLocalAssetRepository _localAssetRepository; final NativeSyncApi _nativeSyncApi; final DriftTrashedLocalAssetRepository _trashedLocalAssetRepository; final AssetMediaRepository _assetMediaRepository; @@ -33,7 +31,6 @@ class LocalSyncService { LocalSyncService({ required this._localAlbumRepository, - required this._localAssetRepository, required this._nativeSyncApi, required this._trashedLocalAssetRepository, required this._assetMediaRepository, @@ -57,11 +54,6 @@ class LocalSyncService { } } - if (CurrentPlatform.isIOS) { - // final assets = await _localAssetRepository.getEmptyCloudIdAssets(); - // await _mapIosCloudIds(assets); - } - if (full || await _nativeSyncApi.shouldFullSync()) { _log.fine("Full sync request from ${full ? "user" : "native"}"); return await fullSync(); @@ -336,28 +328,12 @@ class LocalSyncService { return true; } - // ignore: avoid-unused-parameters Future _mapIosCloudIds(List assets) async { - // if (!CurrentPlatform.isIOS || assets.isEmpty) { - return; - // } + if (!CurrentPlatform.isIOS || assets.isEmpty) { + return; + } - // final assetIds = assets.map((a) => a.id).toList(); - // final cloudMapping = {}; - // final cloudIds = await _nativeSyncApi.getCloudIdForAssetIds(assetIds); - // for (int i = 0; i < cloudIds.length; i++) { - // final cloudIdResult = cloudIds[i]; - // if (cloudIdResult.cloudId != null) { - // cloudMapping[cloudIdResult.assetId] = cloudIdResult.cloudId!; - // } else { - // final asset = assets.firstWhereOrNull((a) => a.id == cloudIdResult.assetId); - // _log.fine( - // "Cannot fetch cloudId for asset with id: ${cloudIdResult.assetId}, name: ${asset?.name}, createdAt: ${asset?.createdAt}. Error: ${cloudIdResult.error ?? "unknown"}", - // ); - // } - // } - - // await _localAlbumRepository.updateCloudMapping(cloudMapping); + await resolveCloudIds(_nativeSyncApi, _localAlbumRepository, assets.map((a) => a.id).toList()); } bool _assetsEqual(LocalAsset a, LocalAsset b) { diff --git a/mobile/lib/domain/utils/cloud_id_resolver.dart b/mobile/lib/domain/utils/cloud_id_resolver.dart new file mode 100644 index 0000000000..3f3ef1aeae --- /dev/null +++ b/mobile/lib/domain/utils/cloud_id_resolver.dart @@ -0,0 +1,46 @@ +import 'dart:async'; + +import 'package:immich_mobile/infrastructure/repositories/local_album.repository.dart'; +import 'package:immich_mobile/platform/native_sync_api.g.dart'; +import 'package:logging/logging.dart'; + +const kCloudIdChunkSize = 5000; + +Future resolveCloudIds( + NativeSyncApi nativeSyncApi, + DriftLocalAlbumRepository albumRepository, + List assetIds, { + Completer? cancellation, +}) async { + final logger = Logger('resolveCloudIds'); + + for (int offset = 0; offset < assetIds.length; offset += kCloudIdChunkSize) { + if (cancellation?.isCompleted ?? false) { + logger.warning('Cloud ID resolution cancelled after $offset of ${assetIds.length} assets'); + return; + } + + final end = offset + kCloudIdChunkSize; + final chunk = assetIds.sublist(offset, end > assetIds.length ? assetIds.length : end); + + final cloudMapping = {}; + for (final result in await nativeSyncApi.getCloudIdForAssetIds(chunk)) { + if (result.cloudId != null) { + cloudMapping[result.assetId] = result.cloudId!; + continue; + } + + if (result.errorKind == CloudIdErrorKind.unsupported) { + logger.warning('Cloud IDs unavailable: ${result.error ?? "unsupported"}'); + return; + } + + logger.fine( + 'Cannot fetch cloudId for asset with id: ${result.assetId}. ' + 'Reason: ${result.errorKind?.name ?? "unknown"}. Error: ${result.error ?? "unknown"}', + ); + } + + await albumRepository.updateCloudMapping(cloudMapping); + } +} diff --git a/mobile/lib/domain/utils/migrate_cloud_ids.dart b/mobile/lib/domain/utils/migrate_cloud_ids.dart index efef6e8327..af7e4666be 100644 --- a/mobile/lib/domain/utils/migrate_cloud_ids.dart +++ b/mobile/lib/domain/utils/migrate_cloud_ids.dart @@ -1,10 +1,13 @@ import 'dart:async'; +import 'dart:math' as math; import 'package:drift/drift.dart'; +import 'package:flutter/foundation.dart'; import 'package:hooks_riverpod/hooks_riverpod.dart'; import 'package:immich_mobile/constants/constants.dart'; import 'package:immich_mobile/domain/models/asset/asset_metadata.model.dart'; import 'package:immich_mobile/domain/models/asset/base_asset.model.dart'; +import 'package:immich_mobile/domain/utils/cloud_id_resolver.dart'; import 'package:immich_mobile/extensions/platform_extensions.dart'; import 'package:immich_mobile/infrastructure/entities/local_asset.entity.dart'; import 'package:immich_mobile/infrastructure/repositories/db.repository.dart'; @@ -13,6 +16,7 @@ import 'package:immich_mobile/platform/native_sync_api.g.dart'; import 'package:immich_mobile/providers/api.provider.dart'; import 'package:immich_mobile/providers/infrastructure/cancel.provider.dart'; import 'package:immich_mobile/providers/infrastructure/db.provider.dart'; +import 'package:immich_mobile/providers/infrastructure/platform.provider.dart'; import 'package:immich_mobile/providers/infrastructure/sync.provider.dart'; import 'package:immich_mobile/providers/server_info.provider.dart'; import 'package:immich_mobile/providers/user.provider.dart'; @@ -20,6 +24,10 @@ import 'package:logging/logging.dart'; // ignore: import_rule_openapi import 'package:openapi/api.dart' hide AssetVisibility; +const _kDbPageSize = 20000; + +const _kUploadBatchSize = 5000; + Future syncCloudIds(ProviderContainer ref) async { if (!CurrentPlatform.isIOS) { return; @@ -27,8 +35,9 @@ Future syncCloudIds(ProviderContainer ref) async { final logger = Logger('migrateCloudIds'); final db = ref.read(driftProvider); - // Populate cloud IDs for local assets that don't have one yet - await _populateCloudIds(db); + final cancellation = ref.read(cancellationProvider); + + await populateMissingCloudIds(db, ref.read(nativeSyncApiProvider), cancellation); final serverInfo = await ref.read(serverInfoProvider.notifier).getServerInfo(); final canUpdateMetadata = serverInfo.serverVersion.isAtLeast(major: 2, minor: 4); @@ -54,7 +63,6 @@ Future syncCloudIds(ProviderContainer ref) async { } final assetApi = ref.read(apiServiceProvider).assetsApi; - final cancellation = ref.read(cancellationProvider); // Process cloud IDs in paginated batches await _processCloudIdMappingsInBatches(db, currentUser.id, assetApi, canBulkUpdateMetadata, logger, cancellation); @@ -68,7 +76,6 @@ Future _processCloudIdMappingsInBatches( Logger logger, Completer cancellation, ) async { - const pageSize = 20000; String? lastLocalId; final seenRemoteAssetIds = {}; @@ -77,106 +84,69 @@ Future _processCloudIdMappingsInBatches( logger.warning('Cloud ID migration cancelled. Stopping batch processing.'); break; } - final mappings = await _fetchCloudIdMappings(drift, userId, pageSize, lastLocalId); + final mappings = await _fetchMapping(drift, userId, _kDbPageSize, lastLocalId); if (mappings.isEmpty) { break; } final items = []; for (final mapping in mappings) { - if (seenRemoteAssetIds.add(mapping.remoteAssetId)) { - items.add( - AssetMetadataBulkUpsertItemDto( - assetId: mapping.remoteAssetId, - key: kMobileMetadataKey, - value: Map.from( - RemoteAssetMobileAppMetadata( - cloudId: mapping.localAsset.cloudId, - createdAt: mapping.localAsset.createdAt.toIso8601String(), - adjustmentTime: mapping.localAsset.adjustmentTime?.toIso8601String(), - latitude: mapping.localAsset.latitude?.toString(), - longitude: mapping.localAsset.longitude?.toString(), - ).toJson(), - ), - ), - ); - } else { + if (!seenRemoteAssetIds.add(mapping.remoteAssetId)) { logger.fine('Duplicate remote asset ID found: ${mapping.remoteAssetId}. Skipping duplicate entry.'); + continue; } + + items.add( + .new( + assetId: mapping.remoteAssetId, + key: kMobileMetadataKey, + value: Map.from( + RemoteAssetMobileAppMetadata( + cloudId: mapping.localAsset.cloudId, + createdAt: mapping.localAsset.createdAt.toIso8601String(), + adjustmentTime: mapping.localAsset.adjustmentTime?.toIso8601String(), + latitude: mapping.localAsset.latitude?.toString(), + longitude: mapping.localAsset.longitude?.toString(), + ).toJson(), + ), + ), + ); } if (items.isNotEmpty) { if (canBulkUpdate) { - await _bulkUpdateCloudIds(assetsApi, items, cancellation.future); + for (int i = 0; i < items.length; i += _kUploadBatchSize) { + if (cancellation.isCompleted) { + break; + } + final end = math.min(i + _kUploadBatchSize, items.length); + await _bulkUpdate(assetsApi, items.sublist(i, end), cancellation.future); + } } else { - await _sequentialUpdateCloudIds(assetsApi, items, cancellation); + await _sequentialUpdate(assetsApi, items, cancellation); } } lastLocalId = mappings.last.localAsset.id; - if (mappings.length < pageSize) { + if (mappings.length < _kDbPageSize) { break; } } } -Future _sequentialUpdateCloudIds( - AssetsApi assetsApi, - List items, - Completer cancellation, -) async { - for (final item in items) { - if (cancellation.isCompleted) { - break; - } - final upsertItem = AssetMetadataUpsertItemDto(key: item.key, value: item.value); - try { - await assetsApi.updateAssetMetadata( - item.assetId, - AssetMetadataUpsertDto(items: [upsertItem]), - abortTrigger: cancellation.future, - ); - } catch (error, stack) { - Logger('migrateCloudIds').warning('Failed to update metadata for asset ${item.assetId}', error, stack); - } - } -} - -Future _bulkUpdateCloudIds( - AssetsApi assetsApi, - List items, - Future abortTrigger, -) async { - try { - await assetsApi.updateBulkAssetMetadata(AssetMetadataBulkUpsertDto(items: items), abortTrigger: abortTrigger); - } catch (error, stack) { - Logger('migrateCloudIds').warning('Failed to bulk update metadata', error, stack); - } -} - -Future _populateCloudIds(Drift drift) async { +@visibleForTesting +Future populateMissingCloudIds(Drift drift, NativeSyncApi nativeSyncApi, Completer cancellation) async { final query = drift.localAssetEntity.selectOnly() ..addColumns([drift.localAssetEntity.id]) ..where(drift.localAssetEntity.iCloudId.isNull()); final ids = await query.map((row) => row.read(drift.localAssetEntity.id)!).get(); - final cloudMapping = {}; - final cloudIds = await NativeSyncApi().getCloudIdForAssetIds(ids); - for (int i = 0; i < cloudIds.length; i++) { - final cloudIdResult = cloudIds[i]; - if (cloudIdResult.cloudId != null) { - cloudMapping[cloudIdResult.assetId] = cloudIdResult.cloudId!; - } else { - Logger('migrateCloudIds').fine( - "Cannot fetch cloudId for asset with id: ${cloudIdResult.assetId}. Error: ${cloudIdResult.error ?? "unknown"}", - ); - } - } - await DriftLocalAlbumRepository(drift).updateCloudMapping(cloudMapping); + + await resolveCloudIds(nativeSyncApi, DriftLocalAlbumRepository(drift), ids, cancellation: cancellation); } typedef _CloudIdMapping = ({String remoteAssetId, LocalAsset localAsset}); -Future> _fetchCloudIdMappings(Drift drift, String userId, int limit, String? lastLocalId) async { +Future> _fetchMapping(Drift drift, String userId, int limit, String? lastLocalId) async { final query = drift.localAssetEntity.select().join([ innerJoin( @@ -201,7 +171,7 @@ Future> _fetchCloudIdMappings(Drift drift, String userId, drift.remoteAssetCloudIdEntity.longitude.isNotExp(drift.localAssetEntity.longitude) | drift.remoteAssetCloudIdEntity.createdAt.isNotExp(drift.localAssetEntity.createdAt)), ) - ..orderBy([OrderingTerm.asc(drift.localAssetEntity.id)]) + ..orderBy([.asc(drift.localAssetEntity.id)]) ..limit(limit); if (lastLocalId != null) { @@ -215,3 +185,38 @@ Future> _fetchCloudIdMappings(Drift drift, String userId, ); }).get(); } + +Future _sequentialUpdate( + AssetsApi assetsApi, + List items, + Completer cancellation, +) async { + for (final item in items) { + if (cancellation.isCompleted) { + break; + } + try { + await assetsApi.updateAssetMetadata( + item.assetId, + .new( + items: [.new(key: item.key, value: item.value)], + ), + abortTrigger: cancellation.future, + ); + } catch (error, stack) { + Logger('migrateCloudIds').warning('Failed to update metadata for asset ${item.assetId}', error, stack); + } + } +} + +Future _bulkUpdate( + AssetsApi assetsApi, + List items, + Future abortTrigger, +) async { + try { + await assetsApi.updateBulkAssetMetadata(.new(items: items), abortTrigger: abortTrigger); + } catch (error, stack) { + Logger('migrateCloudIds').warning('Failed to bulk update metadata', error, stack); + } +} diff --git a/mobile/lib/infrastructure/repositories/local_asset.repository.dart b/mobile/lib/infrastructure/repositories/local_asset.repository.dart index 8396d6d2a6..8e5c683a6b 100644 --- a/mobile/lib/infrastructure/repositories/local_asset.repository.dart +++ b/mobile/lib/infrastructure/repositories/local_asset.repository.dart @@ -217,11 +217,6 @@ class DriftLocalAssetRepository extends DriftDatabaseRepository { return RemovalCandidatesResult(assets: assets, totalBytes: totalBytes); } - Future> getEmptyCloudIdAssets() { - final query = _db.localAssetEntity.select()..where((row) => row.iCloudId.isNull()); - return query.map((row) => row.toDto()).get(); - } - Future reconcileHashesFromCloudId() async { await _db.customUpdate( ''' diff --git a/mobile/lib/pages/common/splash_screen.page.dart b/mobile/lib/pages/common/splash_screen.page.dart index 65a78aed8a..8e8da3d9b1 100644 --- a/mobile/lib/pages/common/splash_screen.page.dart +++ b/mobile/lib/pages/common/splash_screen.page.dart @@ -337,8 +337,7 @@ class SplashScreenPageState extends ConsumerState { unawaited(_resumeBackup(backupProvider)); }), _resumeBackup(backupProvider), - // TODO: Bring back when the soft freeze issue is addressed - // backgroundManager.syncCloudIds(), + backgroundManager.syncCloudIds(), ]); } else { await backgroundManager.hashAssets(); diff --git a/mobile/lib/providers/app_life_cycle.provider.dart b/mobile/lib/providers/app_life_cycle.provider.dart index f369a37142..8ff7071c95 100644 --- a/mobile/lib/providers/app_life_cycle.provider.dart +++ b/mobile/lib/providers/app_life_cycle.provider.dart @@ -135,8 +135,7 @@ class AppLifeCycleNotifier extends StateNotifier { unawaited(_resumeBackup()); }), _resumeBackup(), - // TODO: Bring back when the soft freeze issue is addressed - // _safeRun(backgroundManager.syncCloudIds(), "syncCloudIds"), + _safeRun(backgroundManager.syncCloudIds(), "syncCloudIds"), ]); } else { await _safeRun(backgroundManager.hashAssets, "hashAssets"); diff --git a/mobile/lib/providers/infrastructure/sync.provider.dart b/mobile/lib/providers/infrastructure/sync.provider.dart index 700b51f12d..68ef9e39dc 100644 --- a/mobile/lib/providers/infrastructure/sync.provider.dart +++ b/mobile/lib/providers/infrastructure/sync.provider.dart @@ -37,7 +37,6 @@ final syncStreamRepositoryProvider = Provider((ref) => SyncStreamRepository(ref. final localSyncServiceProvider = Provider( (ref) => LocalSyncService( localAlbumRepository: ref.watch(localAlbumRepository), - localAssetRepository: ref.watch(localAssetRepository), trashedLocalAssetRepository: ref.watch(trashedLocalAssetRepository), assetMediaRepository: ref.watch(assetMediaRepositoryProvider), permissionRepository: ref.watch(permissionRepositoryProvider), diff --git a/mobile/pigeon/native_sync_api.dart b/mobile/pigeon/native_sync_api.dart index 433b154cd1..5aeff51515 100644 --- a/mobile/pigeon/native_sync_api.dart +++ b/mobile/pigeon/native_sync_api.dart @@ -95,12 +95,24 @@ class HashResult { const HashResult({required this.assetId, this.error, this.hash}); } +enum CloudIdErrorKind { + // PHPhotosErrorIdentifierNotFound + notFound, + // PHPhotosErrorMultipleIdentifiersFound + ambiguous, + // PhotoKit returned a partially synced identifier ("GUID:ID:" with no trailing hash) + incomplete, + unsupported, + unknown, +} + class CloudIdResult { final String assetId; final String? error; final String? cloudId; + final CloudIdErrorKind? errorKind; - const CloudIdResult({required this.assetId, this.error, this.cloudId}); + const CloudIdResult({required this.assetId, this.error, this.cloudId, this.errorKind}); } @HostApi() diff --git a/mobile/test/domain/services/local_sync_service_test.dart b/mobile/test/domain/services/local_sync_service_test.dart index 14277709da..00bea08b22 100644 --- a/mobile/test/domain/services/local_sync_service_test.dart +++ b/mobile/test/domain/services/local_sync_service_test.dart @@ -9,7 +9,6 @@ import 'package:immich_mobile/domain/services/store.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/local_album.repository.dart'; -import 'package:immich_mobile/infrastructure/repositories/local_asset.repository.dart'; import 'package:immich_mobile/infrastructure/repositories/store.repository.dart'; import 'package:immich_mobile/infrastructure/repositories/trashed_local_asset.repository.dart'; import 'package:immich_mobile/platform/native_sync_api.g.dart'; @@ -24,7 +23,6 @@ import '../../service.mocks.dart'; void main() { late LocalSyncService sut; late DriftLocalAlbumRepository mockLocalAlbumRepository; - late DriftLocalAssetRepository mockLocalAssetRepository; late DriftTrashedLocalAssetRepository mockTrashedLocalAssetRepository; late AssetMediaRepository mockAssetMediaRepository; late MockPermissionRepository mockPermissionRepository; @@ -47,7 +45,6 @@ void main() { setUp(() async { mockLocalAlbumRepository = MockLocalAlbumRepository(); - mockLocalAssetRepository = MockLocalAssetRepository(); mockTrashedLocalAssetRepository = MockTrashedLocalAssetRepository(); mockAssetMediaRepository = MockAssetMediaRepository(); mockPermissionRepository = MockPermissionRepository(); @@ -70,7 +67,6 @@ void main() { sut = LocalSyncService( localAlbumRepository: mockLocalAlbumRepository, - localAssetRepository: mockLocalAssetRepository, trashedLocalAssetRepository: mockTrashedLocalAssetRepository, assetMediaRepository: mockAssetMediaRepository, permissionRepository: mockPermissionRepository, diff --git a/mobile/test/medium/repositories/local_asset_repository_test.dart b/mobile/test/medium/repositories/local_asset_repository_test.dart index 2376445d1a..3d21e961d0 100644 --- a/mobile/test/medium/repositories/local_asset_repository_test.dart +++ b/mobile/test/medium/repositories/local_asset_repository_test.dart @@ -559,8 +559,8 @@ void main() { final remoteAsset = await ctx.newRemoteAsset(ownerId: userId); final cloudIdAsset = await ctx.newRemoteAssetCloudId(id: remoteAsset.id); final localAsset = await ctx.newLocalAsset( - checksumOption: const Option.none(), - iCloudId: null, + checksumOption: const .none(), + iCloudIdOption: const .none(), createdAt: cloudIdAsset.createdAt, adjustmentTime: cloudIdAsset.adjustmentTime, latitude: cloudIdAsset.latitude, diff --git a/mobile/test/medium/repository_context.dart b/mobile/test/medium/repository_context.dart index 09b8e2c7eb..0f02350a21 100644 --- a/mobile/test/medium/repository_context.dart +++ b/mobile/test/medium/repository_context.dart @@ -271,6 +271,7 @@ class MediumRepositoryContext { AssetType? type, bool? isFavorite, String? iCloudId, + Option? iCloudIdOption, DateTime? adjustmentTime, Option? adjustmentTimeOption, double? latitude, @@ -297,7 +298,7 @@ class MediumRepositoryContext { createdAt: .new(TestUtils.date(createdAt)), type: .new(type ?? .image), isFavorite: .new(isFavorite ?? false), - iCloudId: .new(TestUtils.uuid(iCloudId)), + iCloudId: _resolveUndefined(iCloudId, iCloudIdOption, TestUtils.uuid()), adjustmentTime: _resolveUndefined(adjustmentTime, adjustmentTimeOption, DateTime.now()), latitude: .new(latitude ?? TestUtils.randDouble(-90, 90)), longitude: .new(longitude ?? TestUtils.randDouble(-180, 180)), diff --git a/mobile/test/medium/utils/migrate_cloud_ids_test.dart b/mobile/test/medium/utils/migrate_cloud_ids_test.dart new file mode 100644 index 0000000000..c45d3b34e6 --- /dev/null +++ b/mobile/test/medium/utils/migrate_cloud_ids_test.dart @@ -0,0 +1,113 @@ +import 'dart:async'; + +import 'package:drift/drift.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:immich_mobile/domain/utils/cloud_id_resolver.dart'; +import 'package:immich_mobile/domain/utils/migrate_cloud_ids.dart'; +import 'package:immich_mobile/infrastructure/repositories/local_album.repository.dart'; +import 'package:immich_mobile/platform/native_sync_api.g.dart'; +import 'package:mocktail/mocktail.dart'; + +import '../../service.mocks.dart'; +import '../repository_context.dart'; + +void main() { + late MediumRepositoryContext ctx; + late MockNativeSyncApi mockNativeSyncApi; + late DriftLocalAlbumRepository albumRepository; + + setUp(() { + ctx = MediumRepositoryContext(); + mockNativeSyncApi = MockNativeSyncApi(); + albumRepository = DriftLocalAlbumRepository(ctx.db); + }); + + tearDown(() async { + await ctx.dispose(); + }); + + Future> readCloudIds() async { + final rows = await ctx.db.localAssetEntity.select().get(); + return rows.map((row) => row.iCloudId).toList(); + } + + void resolvingAllInputs() { + when(() => mockNativeSyncApi.getCloudIdForAssetIds(any())).thenAnswer( + (invocation) async => (invocation.positionalArguments.first as List) + .map((id) => CloudIdResult(assetId: id, cloudId: 'cloud-$id')) + .toList(), + ); + } + + group('populateCloudIds', () { + test('writes the cloud ID resolved for each asset', () async { + await ctx.newLocalAsset(id: 'asset-0', iCloudIdOption: const .none()); + resolvingAllInputs(); + + await populateMissingCloudIds(ctx.db, mockNativeSyncApi, .new()); + expect(await readCloudIds(), ['cloud-asset-0']); + }); + + test('skips assets that already have a cloud ID', () async { + await ctx.newLocalAsset(iCloudId: 'existing'); + + await populateMissingCloudIds(ctx.db, mockNativeSyncApi, .new()); + + verifyNever(() => mockNativeSyncApi.getCloudIdForAssetIds(any())); + expect(await readCloudIds(), ['existing']); + }); + + test('does not call the native API when already cancelled', () async { + await ctx.newLocalAsset(iCloudIdOption: const .none()); + + await populateMissingCloudIds(ctx.db, mockNativeSyncApi, .new()..complete()); + + verifyNever(() => mockNativeSyncApi.getCloudIdForAssetIds(any())); + expect(await readCloudIds(), [null]); + }); + }); + + group('resolveCloudIds', () { + test('resolves and stores more assets than fit in a single chunk', () async { + final ids = List.generate(kCloudIdChunkSize + 1, (i) => 'asset-$i'); + for (final id in ids) { + await ctx.newLocalAsset(id: id, iCloudIdOption: const .none()); + } + resolvingAllInputs(); + + await resolveCloudIds(mockNativeSyncApi, albumRepository, ids); + + verify(() => mockNativeSyncApi.getCloudIdForAssetIds(any())).called(2); + final stored = await ctx.db.localAssetEntity.select().get(); + expect(stored.every((row) => row.iCloudId == 'cloud-${row.id}'), isTrue); + }); + + test('stops after the first chunk when cloud IDs are unsupported', () async { + final ids = List.generate(kCloudIdChunkSize + 1, (i) => 'asset-$i'); + when(() => mockNativeSyncApi.getCloudIdForAssetIds(any())).thenAnswer( + (invocation) async => (invocation.positionalArguments.first as List) + .map((id) => CloudIdResult(assetId: id, error: 'needs iOS 16', errorKind: .unsupported)) + .toList(), + ); + + await resolveCloudIds(mockNativeSyncApi, albumRepository, ids); + + verify(() => mockNativeSyncApi.getCloudIdForAssetIds(any())).called(1); + }); + + test('stops between chunks once cancelled', () async { + final ids = List.generate(kCloudIdChunkSize + 1, (i) => 'asset-$i'); + final cancellation = Completer(); + when(() => mockNativeSyncApi.getCloudIdForAssetIds(any())).thenAnswer((invocation) async { + cancellation.complete(); + return (invocation.positionalArguments.first as List) + .map((id) => CloudIdResult(assetId: id, cloudId: 'cloud-$id')) + .toList(); + }); + + await resolveCloudIds(mockNativeSyncApi, albumRepository, ids, cancellation: cancellation); + + verify(() => mockNativeSyncApi.getCloudIdForAssetIds(any())).called(1); + }); + }); +} From ffdd360bd7a86f37afc1f9998b4d8dd086b54aa9 Mon Sep 17 00:00:00 2001 From: shenlong-tanwen <139912620+shalong-tanwen@users.noreply.github.com> Date: Thu, 13 Aug 2026 07:20:09 +0530 Subject: [PATCH 2/5] cleanup error formatting --- mobile/ios/Runner/Sync/MessagesImpl.swift | 22 +++++++++++----------- mobile/pigeon/native_sync_api.dart | 1 - 2 files changed, 11 insertions(+), 12 deletions(-) diff --git a/mobile/ios/Runner/Sync/MessagesImpl.swift b/mobile/ios/Runner/Sync/MessagesImpl.swift index 5424721143..1b1671dac8 100644 --- a/mobile/ios/Runner/Sync/MessagesImpl.swift +++ b/mobile/ios/Runner/Sync/MessagesImpl.swift @@ -454,19 +454,24 @@ class NativeSyncApiImpl: ImmichPlugin, NativeSyncApi, FlutterPlugin { } - private func cloudIdErrorKind(for error: Error) -> CloudIdErrorKind { + private func cloudIdError(for error: Error) -> (kind: CloudIdErrorKind, message: String) { let nsError = error as NSError + var message = "Error getting Cloud Id: \(error.localizedDescription)" + guard nsError.domain == PHPhotosErrorDomain else { - return .unknown + return (.unknown, message) } switch nsError.code { case PHPhotosError.identifierNotFound.rawValue: - return .notFound + return (.notFound, message) case PHPhotosError.multipleIdentifiersFound.rawValue: - return .ambiguous + if let matches = nsError.userInfo[PHLocalIdentifiersErrorKey] as? [String] { + message += " (matched: \(matches.joined(separator: ", ")))" + } + return (.ambiguous, message) default: - return .unknown + return (.unknown, message) } } @@ -491,12 +496,7 @@ class NativeSyncApiImpl: ImmichPlugin, NativeSyncApi, FlutterPlugin { CloudIdResult(assetId: key, error: "Incomplete Cloud Id: \(cloudId)", errorKind: .incomplete)) } case .failure(let error): - let kind = cloudIdErrorKind(for: error) - var message = "Error getting Cloud Id: \(error.localizedDescription)" - if kind == .ambiguous, - let matches = (error as NSError).userInfo[PHLocalIdentifiersErrorKey] as? [String] { - message += " (matched: \(matches.joined(separator: ", ")))" - } + let (kind, message) = cloudIdError(for: error) mappings.append(CloudIdResult(assetId: key, error: message, errorKind: kind)) } } diff --git a/mobile/pigeon/native_sync_api.dart b/mobile/pigeon/native_sync_api.dart index 5aeff51515..1a16f79445 100644 --- a/mobile/pigeon/native_sync_api.dart +++ b/mobile/pigeon/native_sync_api.dart @@ -102,7 +102,6 @@ enum CloudIdErrorKind { ambiguous, // PhotoKit returned a partially synced identifier ("GUID:ID:" with no trailing hash) incomplete, - unsupported, unknown, } From de28149d7d359521f9db56c2d4c646523ad11ef0 Mon Sep 17 00:00:00 2001 From: shenlong-tanwen <139912620+shalong-tanwen@users.noreply.github.com> Date: Thu, 13 Aug 2026 08:13:47 +0530 Subject: [PATCH 3/5] throw on unsupported --- mobile/ios/Runner/Sync/MessagesImpl.swift | 8 ++-- .../domain/services/local_sync.service.dart | 19 +++++--- .../lib/domain/utils/cloud_id_resolver.dart | 45 +++++++++++-------- .../providers/app_life_cycle.provider.dart | 2 +- mobile/pigeon/native_sync_api.dart | 2 + mobile/pubspec.lock | 4 +- mobile/pubspec.yaml | 2 +- .../medium/utils/migrate_cloud_ids_test.dart | 9 ++-- 8 files changed, 52 insertions(+), 39 deletions(-) diff --git a/mobile/ios/Runner/Sync/MessagesImpl.swift b/mobile/ios/Runner/Sync/MessagesImpl.swift index 1b1671dac8..181b343c6e 100644 --- a/mobile/ios/Runner/Sync/MessagesImpl.swift +++ b/mobile/ios/Runner/Sync/MessagesImpl.swift @@ -144,7 +144,7 @@ class NativeSyncApiImpl: ImmichPlugin, NativeSyncApi, FlutterPlugin { private func getMediaChanges() throws -> SyncDelta { guard #available(iOS 16, *) else { - throw PigeonError(code: "UNSUPPORTED_OS", message: "This feature requires iOS 16 or later.", details: nil) + throw PigeonError(code: kUnSupportedOSError, message: "This feature requires iOS 16 or later.", details: nil) } guard PHPhotoLibrary.authorizationStatus(for: .readWrite) == .authorized else { @@ -437,7 +437,7 @@ class NativeSyncApiImpl: ImmichPlugin, NativeSyncApi, FlutterPlugin { } func getTrashedAssets() throws -> [String: [PlatformAsset]] { - throw PigeonError(code: "UNSUPPORTED_OS", message: "This feature not supported on iOS.", details: nil) + throw PigeonError(code: kUnSupportedOSError, message: "This feature not supported on iOS.", details: nil) } func restoreFromTrashById(mediaId: String, type: Int64, completion: @escaping (Result) -> Void) { @@ -477,9 +477,7 @@ class NativeSyncApiImpl: ImmichPlugin, NativeSyncApi, FlutterPlugin { func getCloudIdForAssetIds(assetIds: [String]) throws -> [CloudIdResult] { guard #available(iOS 16, *) else { - return assetIds.map { - CloudIdResult(assetId: $0, error: "Cloud identifiers require iOS 16", errorKind: .unsupported) - } + throw PigeonError(code: kUnSupportedOSError, message: "This feature requires iOS 16 or later.", details: nil) } var mappings: [CloudIdResult] = [] diff --git a/mobile/lib/domain/services/local_sync.service.dart b/mobile/lib/domain/services/local_sync.service.dart index 3abae181fe..250015acda 100644 --- a/mobile/lib/domain/services/local_sync.service.dart +++ b/mobile/lib/domain/services/local_sync.service.dart @@ -108,7 +108,7 @@ class LocalSyncService { await updateAlbum(dbAlbum, album); } - await _mapIosCloudIds(newAssets); + await _mapCloudIds(newAssets); } await _nativeSyncApi.checkpointSync(); } on PlatformException catch (e, s) { @@ -167,7 +167,7 @@ class LocalSyncService { : []; await _localAlbumRepository.upsert(album, toUpsert: assets); - await _mapIosCloudIds(assets); + await _mapCloudIds(assets); _log.fine("Successfully added device album ${album.name}"); } catch (e, s) { _log.warning("Error while adding device album", e, s); @@ -249,7 +249,7 @@ class LocalSyncService { toUpsert: newAssets, ); - await _mapIosCloudIds(newAssets); + await _mapCloudIds(newAssets); return true; } catch (e, s) { _log.warning("Error on fast syncing local album: ${dbAlbum.name}", e, s); @@ -281,7 +281,7 @@ class LocalSyncService { if (dbAlbum.assetCount == 0) { _log.fine("Device album ${deviceAlbum.name} is empty. Adding assets to DB."); await _localAlbumRepository.upsert(updatedDeviceAlbum, toUpsert: assetsInDevice); - await _mapIosCloudIds(assetsInDevice); + await _mapCloudIds(assetsInDevice); return true; } @@ -319,7 +319,7 @@ class LocalSyncService { } await _localAlbumRepository.upsert(updatedDeviceAlbum, toUpsert: assetsToUpsert, toDelete: assetsToDelete); - await _mapIosCloudIds(assetsToUpsert); + await _mapCloudIds(assetsToUpsert); return true; } catch (e, s) { @@ -328,12 +328,17 @@ class LocalSyncService { return true; } - Future _mapIosCloudIds(List assets) async { + Future _mapCloudIds(List assets) async { if (!CurrentPlatform.isIOS || assets.isEmpty) { return; } - await resolveCloudIds(_nativeSyncApi, _localAlbumRepository, assets.map((a) => a.id).toList()); + await resolveCloudIds( + _nativeSyncApi, + _localAlbumRepository, + assets.map((a) => a.id).toList(), + cancellation: _cancellation, + ); } bool _assetsEqual(LocalAsset a, LocalAsset b) { diff --git a/mobile/lib/domain/utils/cloud_id_resolver.dart b/mobile/lib/domain/utils/cloud_id_resolver.dart index 3f3ef1aeae..21e50e8da4 100644 --- a/mobile/lib/domain/utils/cloud_id_resolver.dart +++ b/mobile/lib/domain/utils/cloud_id_resolver.dart @@ -1,9 +1,13 @@ import 'dart:async'; +import 'package:collection/collection.dart'; +import 'package:flutter/foundation.dart'; +import 'package:flutter/services.dart'; import 'package:immich_mobile/infrastructure/repositories/local_album.repository.dart'; import 'package:immich_mobile/platform/native_sync_api.g.dart'; import 'package:logging/logging.dart'; +@visibleForTesting const kCloudIdChunkSize = 5000; Future resolveCloudIds( @@ -14,31 +18,36 @@ Future resolveCloudIds( }) async { final logger = Logger('resolveCloudIds'); - for (int offset = 0; offset < assetIds.length; offset += kCloudIdChunkSize) { + for (final batch in assetIds.slices(kCloudIdChunkSize)) { if (cancellation?.isCompleted ?? false) { - logger.warning('Cloud ID resolution cancelled after $offset of ${assetIds.length} assets'); + logger.warning('Cloud ID resolution cancelled'); return; } - final end = offset + kCloudIdChunkSize; - final chunk = assetIds.sublist(offset, end > assetIds.length ? assetIds.length : end); - - final cloudMapping = {}; - for (final result in await nativeSyncApi.getCloudIdForAssetIds(chunk)) { - if (result.cloudId != null) { - cloudMapping[result.assetId] = result.cloudId!; - continue; - } - - if (result.errorKind == CloudIdErrorKind.unsupported) { - logger.warning('Cloud IDs unavailable: ${result.error ?? "unsupported"}'); + final List results; + try { + results = await nativeSyncApi.getCloudIdForAssetIds(batch); + } on PlatformException catch (error, stack) { + if (error.code == kUnSupportedOSError) { + logger.warning('Cloud IDs are unavailable on this device. Skipping resolution.', error, stack); return; } - logger.fine( - 'Cannot fetch cloudId for asset with id: ${result.assetId}. ' - 'Reason: ${result.errorKind?.name ?? "unknown"}. Error: ${result.error ?? "unknown"}', - ); + logger.warning('Cannot fetch cloudIds for ${batch.length} assets. Skipping batch.', error, stack); + continue; + } + + final cloudMapping = {}; + for (final CloudIdResult(:assetId, :cloudId, :error, :errorKind) in results) { + if (cloudId == null) { + logger.fine( + 'Cannot fetch cloudId for asset with id: $assetId. ' + 'Reason: ${errorKind?.name ?? "unknown"}. Error: ${error ?? "unknown"}', + ); + continue; + } + + cloudMapping[assetId] = cloudId; } await albumRepository.updateCloudMapping(cloudMapping); diff --git a/mobile/lib/providers/app_life_cycle.provider.dart b/mobile/lib/providers/app_life_cycle.provider.dart index 8ff7071c95..de0daf8aa6 100644 --- a/mobile/lib/providers/app_life_cycle.provider.dart +++ b/mobile/lib/providers/app_life_cycle.provider.dart @@ -135,7 +135,7 @@ class AppLifeCycleNotifier extends StateNotifier { unawaited(_resumeBackup()); }), _resumeBackup(), - _safeRun(backgroundManager.syncCloudIds(), "syncCloudIds"), + _safeRun(backgroundManager.syncCloudIds, "syncCloudIds"), ]); } else { await _safeRun(backgroundManager.hashAssets, "hashAssets"); diff --git a/mobile/pigeon/native_sync_api.dart b/mobile/pigeon/native_sync_api.dart index 1a16f79445..f5a6d55e02 100644 --- a/mobile/pigeon/native_sync_api.dart +++ b/mobile/pigeon/native_sync_api.dart @@ -11,6 +11,8 @@ import 'package:pigeon/pigeon.dart'; dartPackageName: 'immich_mobile', ), ) +const String kUnSupportedOSError = 'UNSUPPORTED_OS'; + enum PlatformAssetPlaybackStyle { unknown, image, video, imageAnimated, livePhoto, videoLooping } class PlatformAsset { diff --git a/mobile/pubspec.lock b/mobile/pubspec.lock index ac19481079..d7d03c1177 100644 --- a/mobile/pubspec.lock +++ b/mobile/pubspec.lock @@ -1362,10 +1362,10 @@ packages: dependency: "direct dev" description: name: pigeon - sha256: "04cfefc8add8b47ddf9ccac8b92bb4edeb67c87f185c623ba0db118ac99334ad" + sha256: f90254ef7b22db026fd8d5fd44718fd60697d50968a144a8bc251e5e95d79e10 url: "https://pub.dev" source: hosted - version: "26.3.4" + version: "27.3.0" pinput: dependency: "direct main" description: diff --git a/mobile/pubspec.yaml b/mobile/pubspec.yaml index 32e67de4f6..10d8e9392a 100644 --- a/mobile/pubspec.yaml +++ b/mobile/pubspec.yaml @@ -110,7 +110,7 @@ dev_dependencies: sdk: flutter mocktail: ^1.0.5 # Type safe platform code - pigeon: ^26.3.4 + pigeon: ^27.3.0 # cast 2.1.0 declares a loose bonsoir range but its code targets the 5.x API. # Pin bonsoir to 5.x until cast releases a version compatible with bonsoir 6.x. diff --git a/mobile/test/medium/utils/migrate_cloud_ids_test.dart b/mobile/test/medium/utils/migrate_cloud_ids_test.dart index c45d3b34e6..38585dd755 100644 --- a/mobile/test/medium/utils/migrate_cloud_ids_test.dart +++ b/mobile/test/medium/utils/migrate_cloud_ids_test.dart @@ -1,6 +1,7 @@ import 'dart:async'; import 'package:drift/drift.dart'; +import 'package:flutter/services.dart'; import 'package:flutter_test/flutter_test.dart'; import 'package:immich_mobile/domain/utils/cloud_id_resolver.dart'; import 'package:immich_mobile/domain/utils/migrate_cloud_ids.dart'; @@ -84,11 +85,9 @@ void main() { test('stops after the first chunk when cloud IDs are unsupported', () async { final ids = List.generate(kCloudIdChunkSize + 1, (i) => 'asset-$i'); - when(() => mockNativeSyncApi.getCloudIdForAssetIds(any())).thenAnswer( - (invocation) async => (invocation.positionalArguments.first as List) - .map((id) => CloudIdResult(assetId: id, error: 'needs iOS 16', errorKind: .unsupported)) - .toList(), - ); + when( + () => mockNativeSyncApi.getCloudIdForAssetIds(any()), + ).thenThrow(PlatformException(code: kUnSupportedOSError)); await resolveCloudIds(mockNativeSyncApi, albumRepository, ids); From df060eec4f4c409e752e9b7f5c8b29d62c8dfbfd Mon Sep 17 00:00:00 2001 From: shenlong-tanwen <139912620+shalong-tanwen@users.noreply.github.com> Date: Thu, 13 Aug 2026 08:26:50 +0530 Subject: [PATCH 4/5] move to freezed and slice http batching --- .../models/asset/asset_metadata.model.dart | 54 ++++++++----------- .../lib/domain/utils/migrate_cloud_ids.dart | 8 +-- 2 files changed, 25 insertions(+), 37 deletions(-) diff --git a/mobile/lib/domain/models/asset/asset_metadata.model.dart b/mobile/lib/domain/models/asset/asset_metadata.model.dart index fc29da3db0..ad326da249 100644 --- a/mobile/lib/domain/models/asset/asset_metadata.model.dart +++ b/mobile/lib/domain/models/asset/asset_metadata.model.dart @@ -1,3 +1,7 @@ +import 'package:freezed_annotation/freezed_annotation.dart'; + +part 'asset_metadata.model.freezed.dart'; + enum RemoteAssetMetadataKey { mobileApp("mobile-app"); @@ -23,40 +27,24 @@ class RemoteAssetMetadataItem { } } -class RemoteAssetMobileAppMetadata extends RemoteAssetMetadataValue { - final String? cloudId; - final String? createdAt; - final String? adjustmentTime; - final String? latitude; - final String? longitude; +@freezed +abstract class RemoteAssetMobileAppMetadata extends RemoteAssetMetadataValue with _$RemoteAssetMobileAppMetadata { + const factory RemoteAssetMobileAppMetadata({ + String? cloudId, + String? createdAt, + String? adjustmentTime, + String? latitude, + String? longitude, + }) = _RemoteAssetMobileAppMetadata; - const RemoteAssetMobileAppMetadata({ - this.cloudId, - this.createdAt, - this.adjustmentTime, - this.latitude, - this.longitude, - }); + const RemoteAssetMobileAppMetadata._(); @override - Map toJson() { - final map = {}; - if (cloudId != null) { - map["iCloudId"] = cloudId; - } - if (createdAt != null) { - map["createdAt"] = createdAt; - } - if (adjustmentTime != null) { - map["adjustmentTime"] = adjustmentTime; - } - if (latitude != null) { - map["latitude"] = latitude; - } - if (longitude != null) { - map["longitude"] = longitude; - } - - return map; - } + Map toJson() => { + 'iCloudId': ?cloudId, + 'createdAt': ?createdAt, + 'adjustmentTime': ?adjustmentTime, + 'latitude': ?latitude, + 'longitude': ?longitude, + }; } diff --git a/mobile/lib/domain/utils/migrate_cloud_ids.dart b/mobile/lib/domain/utils/migrate_cloud_ids.dart index af7e4666be..074fd95e74 100644 --- a/mobile/lib/domain/utils/migrate_cloud_ids.dart +++ b/mobile/lib/domain/utils/migrate_cloud_ids.dart @@ -1,6 +1,6 @@ import 'dart:async'; -import 'dart:math' as math; +import 'package:collection/collection.dart'; import 'package:drift/drift.dart'; import 'package:flutter/foundation.dart'; import 'package:hooks_riverpod/hooks_riverpod.dart'; @@ -115,12 +115,12 @@ Future _processCloudIdMappingsInBatches( if (items.isNotEmpty) { if (canBulkUpdate) { - for (int i = 0; i < items.length; i += _kUploadBatchSize) { + for (final batch in items.slices(_kUploadBatchSize)) { if (cancellation.isCompleted) { break; } - final end = math.min(i + _kUploadBatchSize, items.length); - await _bulkUpdate(assetsApi, items.sublist(i, end), cancellation.future); + + await _bulkUpdate(assetsApi, batch, cancellation.future); } } else { await _sequentialUpdate(assetsApi, items, cancellation); From 010230b5f3d777554b255b58d2917bd683c37d5d Mon Sep 17 00:00:00 2001 From: shenlong-tanwen <139912620+shalong-tanwen@users.noreply.github.com> Date: Thu, 13 Aug 2026 10:29:31 +0530 Subject: [PATCH 5/5] simplify remote asset duplication handling --- .../lib/domain/utils/migrate_cloud_ids.dart | 111 ++++++++++-------- 1 file changed, 65 insertions(+), 46 deletions(-) diff --git a/mobile/lib/domain/utils/migrate_cloud_ids.dart b/mobile/lib/domain/utils/migrate_cloud_ids.dart index 074fd95e74..d161dded59 100644 --- a/mobile/lib/domain/utils/migrate_cloud_ids.dart +++ b/mobile/lib/domain/utils/migrate_cloud_ids.dart @@ -9,7 +9,6 @@ import 'package:immich_mobile/domain/models/asset/asset_metadata.model.dart'; import 'package:immich_mobile/domain/models/asset/base_asset.model.dart'; import 'package:immich_mobile/domain/utils/cloud_id_resolver.dart'; import 'package:immich_mobile/extensions/platform_extensions.dart'; -import 'package:immich_mobile/infrastructure/entities/local_asset.entity.dart'; import 'package:immich_mobile/infrastructure/repositories/db.repository.dart'; import 'package:immich_mobile/infrastructure/repositories/local_album.repository.dart'; import 'package:immich_mobile/platform/native_sync_api.g.dart'; @@ -76,37 +75,31 @@ Future _processCloudIdMappingsInBatches( Logger logger, Completer cancellation, ) async { - String? lastLocalId; - final seenRemoteAssetIds = {}; + String? lastRemoteId; while (true) { if (cancellation.isCompleted) { logger.warning('Cloud ID migration cancelled. Stopping batch processing.'); break; } - final mappings = await _fetchMapping(drift, userId, _kDbPageSize, lastLocalId); + final mappings = await fetchMapping(drift, userId, _kDbPageSize, lastRemoteId); if (mappings.isEmpty) { break; } final items = []; for (final mapping in mappings) { - if (!seenRemoteAssetIds.add(mapping.remoteAssetId)) { - logger.fine('Duplicate remote asset ID found: ${mapping.remoteAssetId}. Skipping duplicate entry.'); - continue; - } - items.add( .new( assetId: mapping.remoteAssetId, key: kMobileMetadataKey, value: Map.from( RemoteAssetMobileAppMetadata( - cloudId: mapping.localAsset.cloudId, - createdAt: mapping.localAsset.createdAt.toIso8601String(), - adjustmentTime: mapping.localAsset.adjustmentTime?.toIso8601String(), - latitude: mapping.localAsset.latitude?.toString(), - longitude: mapping.localAsset.longitude?.toString(), + cloudId: mapping.cloudId, + createdAt: mapping.createdAt.toIso8601String(), + adjustmentTime: mapping.adjustmentTime?.toIso8601String(), + latitude: mapping.latitude?.toString(), + longitude: mapping.longitude?.toString(), ).toJson(), ), ), @@ -127,7 +120,7 @@ Future _processCloudIdMappingsInBatches( } } - lastLocalId = mappings.last.localAsset.id; + lastRemoteId = mappings.last.remoteAssetId; if (mappings.length < _kDbPageSize) { break; } @@ -144,44 +137,70 @@ Future populateMissingCloudIds(Drift drift, NativeSyncApi nativeSyncApi, C await resolveCloudIds(nativeSyncApi, DriftLocalAlbumRepository(drift), ids, cancellation: cancellation); } -typedef _CloudIdMapping = ({String remoteAssetId, LocalAsset localAsset}); +@visibleForTesting +typedef CloudIdMapping = ({ + String remoteAssetId, + String cloudId, + DateTime createdAt, + DateTime? adjustmentTime, + double? latitude, + double? longitude, +}); -Future> _fetchMapping(Drift drift, String userId, int limit, String? lastLocalId) async { - final query = - drift.localAssetEntity.select().join([ - innerJoin( - drift.remoteAssetEntity, - drift.localAssetEntity.checksum.equalsExp(drift.remoteAssetEntity.checksum), - ), - leftOuterJoin( - drift.remoteAssetCloudIdEntity, - drift.remoteAssetEntity.id.equalsExp(drift.remoteAssetCloudIdEntity.assetId), - useColumns: false, - ), - ]) - ..where( +@visibleForTesting +Future> fetchMapping(Drift db, String userId, int limit, String? lastRemoteId) async { + final query = db.remoteAssetEntity.selectOnly() + ..addColumns([ + db.remoteAssetEntity.id, + db.localAssetEntity.iCloudId, + db.localAssetEntity.createdAt, + db.localAssetEntity.adjustmentTime, + db.localAssetEntity.latitude, + db.localAssetEntity.longitude, + ]) + ..join([ + innerJoin( + db.localAssetEntity, + db.localAssetEntity.id.isInQuery( + db.localAssetEntity.selectOnly() + ..addColumns([db.localAssetEntity.id.min()]) + ..where(db.localAssetEntity.checksum.equalsExp(db.remoteAssetEntity.checksum)), + ), + useColumns: false, + ), + leftOuterJoin( + db.remoteAssetCloudIdEntity, + db.remoteAssetEntity.id.equalsExp(db.remoteAssetCloudIdEntity.assetId), + useColumns: false, + ), + ]) + ..where( + db.remoteAssetEntity.ownerId.equals(userId) & + // Skip locked assets as we cannot update them without unlocking first + db.remoteAssetEntity.visibility.isNotValue(AssetVisibility.locked.index) & + db.localAssetEntity.iCloudId.isNotNull() & // Only select assets that have a local cloud ID but either no remote cloud ID or a mismatched eTag - drift.localAssetEntity.iCloudId.isNotNull() & - drift.remoteAssetEntity.ownerId.equals(userId) & - // Skip locked assets as we cannot update them without unlocking first - drift.remoteAssetEntity.visibility.isNotValue(AssetVisibility.locked.index) & - (drift.remoteAssetCloudIdEntity.cloudId.isNull() | - drift.remoteAssetCloudIdEntity.adjustmentTime.isNotExp(drift.localAssetEntity.adjustmentTime) | - drift.remoteAssetCloudIdEntity.latitude.isNotExp(drift.localAssetEntity.latitude) | - drift.remoteAssetCloudIdEntity.longitude.isNotExp(drift.localAssetEntity.longitude) | - drift.remoteAssetCloudIdEntity.createdAt.isNotExp(drift.localAssetEntity.createdAt)), - ) - ..orderBy([.asc(drift.localAssetEntity.id)]) - ..limit(limit); + (db.remoteAssetCloudIdEntity.cloudId.isNull() | + db.remoteAssetCloudIdEntity.adjustmentTime.isNotExp(db.localAssetEntity.adjustmentTime) | + db.remoteAssetCloudIdEntity.latitude.isNotExp(db.localAssetEntity.latitude) | + db.remoteAssetCloudIdEntity.longitude.isNotExp(db.localAssetEntity.longitude) | + db.remoteAssetCloudIdEntity.createdAt.isNotExp(db.localAssetEntity.createdAt)), + ) + ..orderBy([.asc(db.remoteAssetEntity.id)]) + ..limit(limit); - if (lastLocalId != null) { - query.where(drift.localAssetEntity.id.isBiggerThanValue(lastLocalId)); + if (lastRemoteId != null) { + query.where(db.remoteAssetEntity.id.isBiggerThanValue(lastRemoteId)); } return query.map((row) { return ( - remoteAssetId: row.read(drift.remoteAssetEntity.id)!, - localAsset: row.readTable(drift.localAssetEntity).toDto(), + remoteAssetId: row.read(db.remoteAssetEntity.id)!, + cloudId: row.read(db.localAssetEntity.iCloudId)!, + createdAt: row.read(db.localAssetEntity.createdAt)!, + adjustmentTime: row.read(db.localAssetEntity.adjustmentTime), + latitude: row.read(db.localAssetEntity.latitude), + longitude: row.read(db.localAssetEntity.longitude), ); }).get(); }