diff --git a/mobile/lib/services/background_upload.service.dart b/mobile/lib/services/background_upload.service.dart index 9de4d4b298..4a5129109a 100644 --- a/mobile/lib/services/background_upload.service.dart +++ b/mobile/lib/services/background_upload.service.dart @@ -317,6 +317,8 @@ class BackgroundUploadService { priority: priority, isFavorite: asset.isFavorite, requiresWiFi: requiresWiFi, + // Visibility hidden on upload to prevent the server from running regular jobs on the live photo asset + fields: entity.isLivePhoto ? {'visibility': api.AssetVisibility.hidden.value} : null, cloudId: entity.isLivePhoto ? null : asset.cloudId, adjustmentTime: entity.isLivePhoto ? null : asset.adjustmentTime?.toIso8601String(), latitude: entity.isLivePhoto ? null : asset.latitude?.toString(), @@ -336,8 +338,7 @@ class BackgroundUploadService { return null; } - // Visibility hidden on upload to prevent the server from running regular jobs on the live photo asset - final fields = {'livePhotoVideoId': livePhotoVideoId, 'visibility': api.AssetVisibility.hidden.value}; + final fields = {'livePhotoVideoId': livePhotoVideoId}; final requiresWiFi = _shouldRequireWiFi(asset); final originalFileName = await _assetMediaRepository.getOriginalFilename(asset.id) ?? asset.name; diff --git a/mobile/lib/services/foreground_upload.service.dart b/mobile/lib/services/foreground_upload.service.dart index e3784fa043..50544ba77d 100644 --- a/mobile/lib/services/foreground_upload.service.dart +++ b/mobile/lib/services/foreground_upload.service.dart @@ -2,6 +2,7 @@ import 'dart:async'; import 'dart:convert'; import 'dart:io'; +import 'package:flutter/foundation.dart'; import 'package:hooks_riverpod/hooks_riverpod.dart'; import 'package:immich_mobile/domain/models/asset/asset_metadata.model.dart'; import 'package:immich_mobile/domain/models/asset/base_asset.model.dart' hide AssetVisibility; @@ -100,7 +101,7 @@ class ForegroundUploadService { final requireWifi = _shouldRequireWiFi(asset); return requireWifi && !hasWifi; }, - processItem: (asset) => _uploadSingleAsset(asset, cancelToken, callbacks: callbacks), + processItem: (asset) => uploadSingleAsset(asset, cancelToken, callbacks: callbacks), ); } } @@ -126,7 +127,7 @@ class ForegroundUploadService { continue; } - await _uploadSingleAsset(asset, cancelToken, callbacks: callbacks); + await uploadSingleAsset(asset, cancelToken, callbacks: callbacks); } } @@ -143,7 +144,7 @@ class ForegroundUploadService { await _executeWithWorkerPool( items: localAssets, cancelToken: cancelToken, - processItem: (asset) => _uploadSingleAsset(asset, cancelToken, callbacks: callbacks), + processItem: (asset) => uploadSingleAsset(asset, cancelToken, callbacks: callbacks), ); } @@ -233,7 +234,8 @@ class ForegroundUploadService { await Future.wait(workerFutures); } - Future _uploadSingleAsset( + @visibleForTesting + Future uploadSingleAsset( LocalAsset asset, Completer? cancelToken, { required UploadCallbacks callbacks, diff --git a/mobile/test/api.mocks.dart b/mobile/test/api.mocks.dart index 91e27735ae..40cdd29143 100644 --- a/mobile/test/api.mocks.dart +++ b/mobile/test/api.mocks.dart @@ -1,3 +1,4 @@ +import 'package:immich_mobile/platform/connectivity_api.g.dart'; import 'package:immich_mobile/repositories/partner_api.repository.dart'; import 'package:mocktail/mocktail.dart'; import 'package:openapi/api.dart'; @@ -7,3 +8,5 @@ class MockSyncApi extends Mock implements SyncApi {} class MockServerApi extends Mock implements ServerApi {} class MockPartnerApiRepository extends Mock implements PartnerApiRepository {} + +class MockConnectivityApi extends Mock implements ConnectivityApi {} diff --git a/mobile/test/services/background_upload.service_test.dart b/mobile/test/services/background_upload.service_test.dart index 36c951702d..527da1bf1b 100644 --- a/mobile/test/services/background_upload.service_test.dart +++ b/mobile/test/services/background_upload.service_test.dart @@ -118,8 +118,24 @@ void main() { expect(task, isNotNull); // For live photos, extension should be changed to match the video file expect(task!.fields['filename'], equals('OriginalLivePhoto.mov')); + expect(task.fields['visibility'], equals('hidden')); verify(() => mockAssetMediaRepository.getOriginalFilename(asset.id)).called(1); }); + + test('should not set visibility for a regular photo', () async { + final asset = LocalAssetStub.image1; + final mockEntity = MockAssetEntity(); + final mockFile = File('/path/to/file.jpg'); + + when(() => mockEntity.isLivePhoto).thenReturn(false); + when(() => mockStorageRepository.getAssetEntityForAsset(asset)).thenAnswer((_) async => mockEntity); + when(() => mockStorageRepository.getFileForAsset(asset.id)).thenAnswer((_) async => mockFile); + when(() => mockAssetMediaRepository.getOriginalFilename(asset.id)).thenAnswer((_) async => 'Regular.jpg'); + + final task = await sut.getUploadTask(asset); + expect(task, isNotNull); + expect(task!.fields.containsKey('visibility'), isFalse); + }); }); group('getLivePhotoUploadTask', () { @@ -140,7 +156,7 @@ void main() { expect(task, isNotNull); expect(task!.fields['filename'], equals('OriginalLivePhoto.HEIC')); expect(task.fields['livePhotoVideoId'], equals('video-id-123')); - expect(task.fields['visibility'], equals('hidden')); + expect(task.fields.containsKey('visibility'), isFalse); verify(() => mockAssetMediaRepository.getOriginalFilename(asset.id)).called(1); }); @@ -334,7 +350,7 @@ void main() { expect(task, isNotNull); expect(task!.fields.containsKey('metadata'), isTrue); expect(task.fields['livePhotoVideoId'], equals('video-123')); - expect(task.fields['visibility'], equals('hidden')); + expect(task.fields.containsKey('visibility'), isFalse); final metadata = jsonDecode(task.fields['metadata']!) as List; expect(metadata, hasLength(1)); diff --git a/mobile/test/services/foreground_upload.service_test.dart b/mobile/test/services/foreground_upload.service_test.dart new file mode 100644 index 0000000000..ed48270b87 --- /dev/null +++ b/mobile/test/services/foreground_upload.service_test.dart @@ -0,0 +1,128 @@ +import 'dart:io'; + +import 'package:drift/drift.dart' hide isNull, isNotNull; +import 'package:drift/native.dart'; +import 'package:flutter/services.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:immich_mobile/domain/models/store.model.dart'; +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/settings.repository.dart'; +import 'package:immich_mobile/infrastructure/repositories/store.repository.dart'; +import 'package:immich_mobile/repositories/upload.repository.dart'; +import 'package:immich_mobile/services/foreground_upload.service.dart'; +import 'package:mocktail/mocktail.dart'; + +import '../api.mocks.dart'; +import '../fixtures/asset.stub.dart'; +import '../infrastructure/repository.mock.dart'; +import '../mocks/asset_entity.mock.dart'; +import '../repository.mocks.dart'; + +void main() { + late ForegroundUploadService sut; + late MockUploadRepository mockUploadRepository; + late MockStorageRepository mockStorageRepository; + late MockDriftBackupRepository mockBackupRepository; + late MockConnectivityApi mockConnectivityApi; + late MockAssetMediaRepository mockAssetMediaRepository; + late Drift db; + + setUpAll(() async { + TestWidgetsFlutterBinding.ensureInitialized(); + TestDefaultBinaryMessengerBinding.instance.defaultBinaryMessenger.setMockMethodCallHandler( + const MethodChannel('plugins.flutter.io/path_provider'), + (MethodCall methodCall) async => 'test', + ); + db = Drift(DatabaseConnection(NativeDatabase.memory(), closeStreamsSynchronously: true)); + await StoreService.init(storeRepository: DriftStoreRepository(db)); + await SettingsRepository.ensureInitialized(db); + + await Store.put(StoreKey.serverEndpoint, 'http://demo.immich.app'); + await Store.put(StoreKey.deviceId, 'device-id'); + + registerFallbackValue(File('file')); + registerFallbackValue({}); + }); + + setUp(() { + mockUploadRepository = MockUploadRepository(); + mockStorageRepository = MockStorageRepository(); + mockBackupRepository = MockDriftBackupRepository(); + mockConnectivityApi = MockConnectivityApi(); + mockAssetMediaRepository = MockAssetMediaRepository(); + + sut = ForegroundUploadService( + mockUploadRepository, + mockStorageRepository, + mockBackupRepository, + mockConnectivityApi, + mockAssetMediaRepository, + ); + }); + + List> captureFields() { + final captured = >[]; + when( + () => mockUploadRepository.uploadFile( + file: any(named: 'file'), + originalFileName: any(named: 'originalFileName'), + fields: any(named: 'fields'), + cancelToken: any(named: 'cancelToken'), + onProgress: any(named: 'onProgress'), + logContext: any(named: 'logContext'), + ), + ).thenAnswer((invocation) async { + final fields = invocation.namedArguments[#fields] as Map; + captured.add(Map.of(fields)); + return UploadResult.success(remoteAssetId: 'remote-${captured.length}'); + }); + return captured; + } + + group('uploadSingleAsset', () { + test('should upload the motion part hidden and keep the still image visible', () async { + final asset = LocalAssetStub.image1; + final mockEntity = MockAssetEntity(); + final stillFile = File('/path/to/still.heic'); + final videoFile = File('/path/to/motion.mov'); + + when(() => mockEntity.isLivePhoto).thenReturn(true); + when(() => mockStorageRepository.getAssetEntityForAsset(asset)).thenAnswer((_) async => mockEntity); + when(() => mockStorageRepository.isAssetAvailableLocally(asset.id)).thenAnswer((_) async => true); + when(() => mockStorageRepository.getFileForAsset(asset.id)).thenAnswer((_) async => stillFile); + when(() => mockStorageRepository.getMotionFileForAsset(asset)).thenAnswer((_) async => videoFile); + when(() => mockAssetMediaRepository.getOriginalFilename(asset.id)).thenAnswer((_) async => 'live.heic'); + + final captured = captureFields(); + + await sut.uploadSingleAsset(asset, null, callbacks: const UploadCallbacks()); + + expect(captured, hasLength(2)); + expect(captured[0]['visibility'], equals('hidden')); + expect(captured[0].containsKey('livePhotoVideoId'), isFalse); + expect(captured[1].containsKey('visibility'), isFalse); + expect(captured[1]['livePhotoVideoId'], equals('remote-1')); + }); + + test('should not set visibility for a regular photo', () async { + final asset = LocalAssetStub.image1; + final mockEntity = MockAssetEntity(); + final stillFile = File('/path/to/photo.jpg'); + + when(() => mockEntity.isLivePhoto).thenReturn(false); + when(() => mockStorageRepository.getAssetEntityForAsset(asset)).thenAnswer((_) async => mockEntity); + when(() => mockStorageRepository.isAssetAvailableLocally(asset.id)).thenAnswer((_) async => true); + when(() => mockStorageRepository.getFileForAsset(asset.id)).thenAnswer((_) async => stillFile); + when(() => mockAssetMediaRepository.getOriginalFilename(asset.id)).thenAnswer((_) async => 'photo.jpg'); + + final captured = captureFields(); + + await sut.uploadSingleAsset(asset, null, callbacks: const UploadCallbacks()); + + expect(captured, hasLength(1)); + expect(captured[0].containsKey('visibility'), isFalse); + }); + }); +} diff --git a/server/src/schema/migrations/1782500000000-RestoreLivePhotoStillVisibility.ts b/server/src/schema/migrations/1782500000000-RestoreLivePhotoStillVisibility.ts new file mode 100644 index 0000000000..f8525b51c6 --- /dev/null +++ b/server/src/schema/migrations/1782500000000-RestoreLivePhotoStillVisibility.ts @@ -0,0 +1,16 @@ +import { Kysely, sql } from 'kysely'; + +export async function up(db: Kysely): Promise { + // (#29836) Reset the visibility of any still images that are hidden and have a motion part + await sql` + UPDATE "asset" + SET "visibility" = 'timeline' + WHERE "type" = 'IMAGE' + AND "visibility" = 'hidden' + AND "livePhotoVideoId" IS NOT NULL + `.execute(db); +} + +export async function down(): Promise { + // Not implemented: the previous 'hidden' value was itself the bug. +}