From fb606192f278fdb5a4beba875cb7af7076bc6d49 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] refactor: reenable cloud ids # Conflicts: # mobile/lib/domain/utils/migrate_cloud_ids.dart --- 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 | 154 +++++++++--------- .../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, 293 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 e1d5fe451e..16ea53c38b 100644 --- a/mobile/lib/domain/utils/migrate_cloud_ids.dart +++ b/mobile/lib/domain/utils/migrate_cloud_ids.dart @@ -1,6 +1,8 @@ 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'; @@ -14,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'; @@ -21,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; @@ -28,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.supports(.cloudIdMetadata); @@ -55,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); @@ -69,7 +76,6 @@ Future _processCloudIdMappingsInBatches( Logger logger, Completer cancellation, ) async { - const pageSize = 20000; String? lastLocalId; final seenRemoteAssetIds = {}; @@ -78,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( @@ -202,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) { @@ -216,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); + }); + }); +}