diff --git a/mobile/android/app/src/main/kotlin/app/alextran/immich/sync/Messages.g.kt b/mobile/android/app/src/main/kotlin/app/alextran/immich/sync/Messages.g.kt index 02f1cb237d..b048c73db7 100644 --- a/mobile/android/app/src/main/kotlin/app/alextran/immich/sync/Messages.g.kt +++ b/mobile/android/app/src/main/kotlin/app/alextran/immich/sync/Messages.g.kt @@ -207,6 +207,20 @@ enum class PlatformAssetPlaybackStyle(val raw: Int) { } } +enum class CloudIdErrorKind(val raw: Int) { + NOT_FOUND(0), + AMBIGUOUS(1), + INCOMPLETE(2), + UNSUPPORTED(3), + UNKNOWN(4); + + companion object { + fun ofRaw(raw: Int): CloudIdErrorKind? { + return values().firstOrNull { it.raw == raw } + } + } +} + /** Generated class from Pigeon that represents data sent in messages. */ data class PlatformAsset ( val id: String, @@ -435,7 +449,8 @@ data class HashResult ( data class CloudIdResult ( val assetId: String, val error: String? = null, - val cloudId: String? = null + val cloudId: String? = null, + val errorKind: CloudIdErrorKind? = null ) { companion object { @@ -443,7 +458,8 @@ data class CloudIdResult ( val assetId = pigeonVar_list[0] as String val error = pigeonVar_list[1] as String? val cloudId = pigeonVar_list[2] as String? - return CloudIdResult(assetId, error, cloudId) + val errorKind = pigeonVar_list[3] as CloudIdErrorKind? + return CloudIdResult(assetId, error, cloudId, errorKind) } } fun toList(): List { @@ -451,6 +467,7 @@ data class CloudIdResult ( assetId, error, cloudId, + errorKind, ) } override fun equals(other: Any?): Boolean { @@ -461,7 +478,7 @@ data class CloudIdResult ( return true } val other = other as CloudIdResult - return MessagesPigeonUtils.deepEquals(this.assetId, other.assetId) && MessagesPigeonUtils.deepEquals(this.error, other.error) && MessagesPigeonUtils.deepEquals(this.cloudId, other.cloudId) + return MessagesPigeonUtils.deepEquals(this.assetId, other.assetId) && MessagesPigeonUtils.deepEquals(this.error, other.error) && MessagesPigeonUtils.deepEquals(this.cloudId, other.cloudId) && MessagesPigeonUtils.deepEquals(this.errorKind, other.errorKind) } override fun hashCode(): Int { @@ -469,6 +486,7 @@ data class CloudIdResult ( result = 31 * result + MessagesPigeonUtils.deepHash(this.assetId) result = 31 * result + MessagesPigeonUtils.deepHash(this.error) result = 31 * result + MessagesPigeonUtils.deepHash(this.cloudId) + result = 31 * result + MessagesPigeonUtils.deepHash(this.errorKind) return result } } @@ -481,26 +499,31 @@ private open class MessagesPigeonCodec : StandardMessageCodec() { } } 130.toByte() -> { - return (readValue(buffer) as? List)?.let { - PlatformAsset.fromList(it) + return (readValue(buffer) as Long?)?.let { + CloudIdErrorKind.ofRaw(it.toInt()) } } 131.toByte() -> { return (readValue(buffer) as? List)?.let { - PlatformAlbum.fromList(it) + PlatformAsset.fromList(it) } } 132.toByte() -> { return (readValue(buffer) as? List)?.let { - SyncDelta.fromList(it) + PlatformAlbum.fromList(it) } } 133.toByte() -> { return (readValue(buffer) as? List)?.let { - HashResult.fromList(it) + SyncDelta.fromList(it) } } 134.toByte() -> { + return (readValue(buffer) as? List)?.let { + HashResult.fromList(it) + } + } + 135.toByte() -> { return (readValue(buffer) as? List)?.let { CloudIdResult.fromList(it) } @@ -514,26 +537,30 @@ private open class MessagesPigeonCodec : StandardMessageCodec() { stream.write(129) writeValue(stream, value.raw.toLong()) } - is PlatformAsset -> { + is CloudIdErrorKind -> { stream.write(130) - writeValue(stream, value.toList()) + writeValue(stream, value.raw.toLong()) } - is PlatformAlbum -> { + is PlatformAsset -> { stream.write(131) writeValue(stream, value.toList()) } - is SyncDelta -> { + is PlatformAlbum -> { stream.write(132) writeValue(stream, value.toList()) } - is HashResult -> { + is SyncDelta -> { stream.write(133) writeValue(stream, value.toList()) } - is CloudIdResult -> { + is HashResult -> { stream.write(134) writeValue(stream, value.toList()) } + is CloudIdResult -> { + stream.write(135) + writeValue(stream, value.toList()) + } else -> super.writeValue(stream, value) } } diff --git a/mobile/ios/Runner/Sync/Messages.g.swift b/mobile/ios/Runner/Sync/Messages.g.swift index a752785c5b..3795db6ad5 100644 --- a/mobile/ios/Runner/Sync/Messages.g.swift +++ b/mobile/ios/Runner/Sync/Messages.g.swift @@ -183,6 +183,14 @@ enum PlatformAssetPlaybackStyle: Int { case videoLooping = 5 } +enum CloudIdErrorKind: Int { + case notFound = 0 + case ambiguous = 1 + case incomplete = 2 + case unsupported = 3 + case unknown = 4 +} + /// Generated class from Pigeon that represents data sent in messages. struct PlatformAsset: Hashable { var id: String @@ -422,6 +430,7 @@ struct CloudIdResult: Hashable { var assetId: String var error: String? = nil var cloudId: String? = nil + var errorKind: CloudIdErrorKind? = nil // swift-format-ignore: AlwaysUseLowerCamelCase @@ -429,11 +438,13 @@ struct CloudIdResult: Hashable { let assetId = pigeonVar_list[0] as! String let error: String? = nilOrValue(pigeonVar_list[1]) let cloudId: String? = nilOrValue(pigeonVar_list[2]) + let errorKind: CloudIdErrorKind? = nilOrValue(pigeonVar_list[3]) return CloudIdResult( assetId: assetId, error: error, - cloudId: cloudId + cloudId: cloudId, + errorKind: errorKind ) } func toList() -> [Any?] { @@ -441,13 +452,14 @@ struct CloudIdResult: Hashable { assetId, error, cloudId, + errorKind, ] } static func == (lhs: CloudIdResult, rhs: CloudIdResult) -> Bool { if Swift.type(of: lhs) != Swift.type(of: rhs) { return false } - return deepEqualsMessages(lhs.assetId, rhs.assetId) && deepEqualsMessages(lhs.error, rhs.error) && deepEqualsMessages(lhs.cloudId, rhs.cloudId) + return deepEqualsMessages(lhs.assetId, rhs.assetId) && deepEqualsMessages(lhs.error, rhs.error) && deepEqualsMessages(lhs.cloudId, rhs.cloudId) && deepEqualsMessages(lhs.errorKind, rhs.errorKind) } func hash(into hasher: inout Hasher) { @@ -455,6 +467,7 @@ struct CloudIdResult: Hashable { deepHashMessages(value: assetId, hasher: &hasher) deepHashMessages(value: error, hasher: &hasher) deepHashMessages(value: cloudId, hasher: &hasher) + deepHashMessages(value: errorKind, hasher: &hasher) } } @@ -468,14 +481,20 @@ private class MessagesPigeonCodecReader: FlutterStandardReader { } return nil case 130: - return PlatformAsset.fromList(self.readValue() as! [Any?]) + let enumResultAsInt: Int? = nilOrValue(self.readValue() as! Int?) + if let enumResultAsInt = enumResultAsInt { + return CloudIdErrorKind(rawValue: enumResultAsInt) + } + return nil case 131: - return PlatformAlbum.fromList(self.readValue() as! [Any?]) + return PlatformAsset.fromList(self.readValue() as! [Any?]) case 132: - return SyncDelta.fromList(self.readValue() as! [Any?]) + return PlatformAlbum.fromList(self.readValue() as! [Any?]) case 133: - return HashResult.fromList(self.readValue() as! [Any?]) + return SyncDelta.fromList(self.readValue() as! [Any?]) case 134: + return HashResult.fromList(self.readValue() as! [Any?]) + case 135: return CloudIdResult.fromList(self.readValue() as! [Any?]) default: return super.readValue(ofType: type) @@ -488,21 +507,24 @@ private class MessagesPigeonCodecWriter: FlutterStandardWriter { if let value = value as? PlatformAssetPlaybackStyle { super.writeByte(129) super.writeValue(value.rawValue) - } else if let value = value as? PlatformAsset { + } else if let value = value as? CloudIdErrorKind { super.writeByte(130) - super.writeValue(value.toList()) - } else if let value = value as? PlatformAlbum { + super.writeValue(value.rawValue) + } else if let value = value as? PlatformAsset { super.writeByte(131) super.writeValue(value.toList()) - } else if let value = value as? SyncDelta { + } else if let value = value as? PlatformAlbum { super.writeByte(132) super.writeValue(value.toList()) - } else if let value = value as? HashResult { + } else if let value = value as? SyncDelta { super.writeByte(133) super.writeValue(value.toList()) - } else if let value = value as? CloudIdResult { + } else if let value = value as? HashResult { super.writeByte(134) super.writeValue(value.toList()) + } else if let value = value as? CloudIdResult { + super.writeByte(135) + super.writeValue(value.toList()) } else { super.writeValue(value) } 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 e4c332b283..96e663b598 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 feb104f90d..16b7e123a2 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 c34d2c4697..41265a78f0 100644 --- a/mobile/lib/infrastructure/repositories/local_asset.repository.dart +++ b/mobile/lib/infrastructure/repositories/local_asset.repository.dart @@ -209,11 +209,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 0d423875cb..c86ec8b707 100644 --- a/mobile/lib/pages/common/splash_screen.page.dart +++ b/mobile/lib/pages/common/splash_screen.page.dart @@ -330,8 +330,7 @@ class SplashScreenPageState extends ConsumerState { _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/platform/native_sync_api.g.dart b/mobile/lib/platform/native_sync_api.g.dart index bd979af87b..52f60dd463 100644 --- a/mobile/lib/platform/native_sync_api.g.dart +++ b/mobile/lib/platform/native_sync_api.g.dart @@ -88,6 +88,8 @@ int _deepHash(Object? value) { enum PlatformAssetPlaybackStyle { unknown, image, video, imageAnimated, livePhoto, videoLooping } +enum CloudIdErrorKind { notFound, ambiguous, incomplete, unsupported, unknown } + class PlatformAsset { PlatformAsset({ required this.id, @@ -355,7 +357,7 @@ class HashResult { } class CloudIdResult { - CloudIdResult({required this.assetId, this.error, this.cloudId}); + CloudIdResult({required this.assetId, this.error, this.cloudId, this.errorKind}); String assetId; @@ -363,8 +365,10 @@ class CloudIdResult { String? cloudId; + CloudIdErrorKind? errorKind; + List _toList() { - return [assetId, error, cloudId]; + return [assetId, error, cloudId, errorKind]; } Object encode() { @@ -373,7 +377,12 @@ class CloudIdResult { static CloudIdResult decode(Object result) { result as List; - return CloudIdResult(assetId: result[0]! as String, error: result[1] as String?, cloudId: result[2] as String?); + return CloudIdResult( + assetId: result[0]! as String, + error: result[1] as String?, + cloudId: result[2] as String?, + errorKind: result[3] as CloudIdErrorKind?, + ); } @override @@ -387,7 +396,8 @@ class CloudIdResult { } return _deepEquals(assetId, other.assetId) && _deepEquals(error, other.error) && - _deepEquals(cloudId, other.cloudId); + _deepEquals(cloudId, other.cloudId) && + _deepEquals(errorKind, other.errorKind); } @override @@ -405,21 +415,24 @@ class _PigeonCodec extends StandardMessageCodec { } else if (value is PlatformAssetPlaybackStyle) { buffer.putUint8(129); writeValue(buffer, value.index); - } else if (value is PlatformAsset) { + } else if (value is CloudIdErrorKind) { buffer.putUint8(130); - writeValue(buffer, value.encode()); - } else if (value is PlatformAlbum) { + writeValue(buffer, value.index); + } else if (value is PlatformAsset) { buffer.putUint8(131); writeValue(buffer, value.encode()); - } else if (value is SyncDelta) { + } else if (value is PlatformAlbum) { buffer.putUint8(132); writeValue(buffer, value.encode()); - } else if (value is HashResult) { + } else if (value is SyncDelta) { buffer.putUint8(133); writeValue(buffer, value.encode()); - } else if (value is CloudIdResult) { + } else if (value is HashResult) { buffer.putUint8(134); writeValue(buffer, value.encode()); + } else if (value is CloudIdResult) { + buffer.putUint8(135); + writeValue(buffer, value.encode()); } else { super.writeValue(buffer, value); } @@ -432,14 +445,17 @@ class _PigeonCodec extends StandardMessageCodec { final value = readValue(buffer) as int?; return value == null ? null : PlatformAssetPlaybackStyle.values[value]; case 130: - return PlatformAsset.decode(readValue(buffer)!); + final value = readValue(buffer) as int?; + return value == null ? null : CloudIdErrorKind.values[value]; case 131: - return PlatformAlbum.decode(readValue(buffer)!); + return PlatformAsset.decode(readValue(buffer)!); case 132: - return SyncDelta.decode(readValue(buffer)!); + return PlatformAlbum.decode(readValue(buffer)!); case 133: - return HashResult.decode(readValue(buffer)!); + return SyncDelta.decode(readValue(buffer)!); case 134: + return HashResult.decode(readValue(buffer)!); + case 135: return CloudIdResult.decode(readValue(buffer)!); default: return super.readValueOfType(type, buffer); diff --git a/mobile/lib/providers/app_life_cycle.provider.dart b/mobile/lib/providers/app_life_cycle.provider.dart index ad0940b776..07d3961107 100644 --- a/mobile/lib/providers/app_life_cycle.provider.dart +++ b/mobile/lib/providers/app_life_cycle.provider.dart @@ -123,8 +123,7 @@ class AppLifeCycleNotifier extends StateNotifier { _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 bc74728346..7e27e81d88 100644 --- a/mobile/test/medium/repositories/local_asset_repository_test.dart +++ b/mobile/test/medium/repositories/local_asset_repository_test.dart @@ -492,8 +492,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 ed06774e82..ea9752b596 100644 --- a/mobile/test/medium/repository_context.dart +++ b/mobile/test/medium/repository_context.dart @@ -252,6 +252,7 @@ class MediumRepositoryContext { AssetType? type, bool? isFavorite, String? iCloudId, + Option? iCloudIdOption, DateTime? adjustmentTime, Option? adjustmentTimeOption, double? latitude, @@ -278,7 +279,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); + }); + }); +}