From c860809aa101760ad06e686474b764e2f38008e0 Mon Sep 17 00:00:00 2001 From: shenlong <139912620+shenlong-tanwen@users.noreply.github.com> Date: Mon, 24 Nov 2025 21:53:17 +0530 Subject: [PATCH] fix: getAspectRatio fallback to db width and height (#24131) fix: getExif fallback to db width and height Co-authored-by: shenlong-tanwen <139912620+shalong-tanwen@users.noreply.github.com> --- mobile/lib/domain/services/asset.service.dart | 14 ++ .../domain/services/asset.service_test.dart | 165 ++++++++++++++++++ .../test/infrastructure/repository.mock.dart | 3 + mobile/test/test_utils.dart | 40 +++++ 4 files changed, 222 insertions(+) create mode 100644 mobile/test/domain/services/asset.service_test.dart diff --git a/mobile/lib/domain/services/asset.service.dart b/mobile/lib/domain/services/asset.service.dart index 33661105e4..3d8fddc9b7 100644 --- a/mobile/lib/domain/services/asset.service.dart +++ b/mobile/lib/domain/services/asset.service.dart @@ -75,6 +75,20 @@ class AssetService { isFlipped = false; } + if (width == null || height == null) { + if (asset.hasRemote) { + final id = asset is LocalAsset ? asset.remoteId! : (asset as RemoteAsset).id; + final remoteAsset = await _remoteAssetRepository.get(id); + width = remoteAsset?.width?.toDouble(); + height = remoteAsset?.height?.toDouble(); + } else { + final id = asset is LocalAsset ? asset.id : (asset as RemoteAsset).localId!; + final localAsset = await _localAssetRepository.get(id); + width = localAsset?.width?.toDouble(); + height = localAsset?.height?.toDouble(); + } + } + final orientedWidth = isFlipped ? height : width; final orientedHeight = isFlipped ? width : height; if (orientedWidth != null && orientedHeight != null && orientedHeight > 0) { diff --git a/mobile/test/domain/services/asset.service_test.dart b/mobile/test/domain/services/asset.service_test.dart new file mode 100644 index 0000000000..5e7179ffa6 --- /dev/null +++ b/mobile/test/domain/services/asset.service_test.dart @@ -0,0 +1,165 @@ +import 'package:flutter/foundation.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:immich_mobile/domain/models/exif.model.dart'; +import 'package:immich_mobile/domain/services/asset.service.dart'; +import 'package:mocktail/mocktail.dart'; + +import '../../infrastructure/repository.mock.dart'; +import '../../test_utils.dart'; + +void main() { + late AssetService sut; + late MockRemoteAssetRepository mockRemoteAssetRepository; + late MockDriftLocalAssetRepository mockLocalAssetRepository; + + setUp(() { + mockRemoteAssetRepository = MockRemoteAssetRepository(); + mockLocalAssetRepository = MockDriftLocalAssetRepository(); + sut = AssetService( + remoteAssetRepository: mockRemoteAssetRepository, + localAssetRepository: mockLocalAssetRepository, + ); + }); + + group('getAspectRatio', () { + test('flips dimensions on Android for 90° and 270° orientations', () async { + debugDefaultTargetPlatformOverride = TargetPlatform.android; + addTearDown(() => debugDefaultTargetPlatformOverride = null); + + for (final orientation in [90, 270]) { + final localAsset = TestUtils.createLocalAsset( + id: 'local-$orientation', + width: 1920, + height: 1080, + orientation: orientation, + ); + + final result = await sut.getAspectRatio(localAsset); + + expect(result, 1080 / 1920, reason: 'Orientation $orientation should flip on Android'); + } + }); + + test('does not flip dimensions on iOS regardless of orientation', () async { + debugDefaultTargetPlatformOverride = TargetPlatform.iOS; + addTearDown(() => debugDefaultTargetPlatformOverride = null); + + for (final orientation in [0, 90, 270]) { + final localAsset = TestUtils.createLocalAsset( + id: 'local-$orientation', + width: 1920, + height: 1080, + orientation: orientation, + ); + + final result = await sut.getAspectRatio(localAsset); + + expect(result, 1920 / 1080, reason: 'iOS should never flip dimensions'); + } + }); + + test('fetches dimensions from remote repository when missing from asset', () async { + final remoteAsset = TestUtils.createRemoteAsset(id: 'remote-1', width: null, height: null); + + final exif = const ExifInfo(orientation: '1'); + + final fetchedAsset = TestUtils.createRemoteAsset(id: 'remote-1', width: 1920, height: 1080); + + when(() => mockRemoteAssetRepository.getExif('remote-1')).thenAnswer((_) async => exif); + when(() => mockRemoteAssetRepository.get('remote-1')).thenAnswer((_) async => fetchedAsset); + + final result = await sut.getAspectRatio(remoteAsset); + + expect(result, 1920 / 1080); + verify(() => mockRemoteAssetRepository.get('remote-1')).called(1); + }); + + test('fetches dimensions from local repository when missing from local asset', () async { + final localAsset = TestUtils.createLocalAsset(id: 'local-1', width: null, height: null, orientation: 0); + + final fetchedAsset = TestUtils.createLocalAsset(id: 'local-1', width: 1920, height: 1080, orientation: 0); + + when(() => mockLocalAssetRepository.get('local-1')).thenAnswer((_) async => fetchedAsset); + + final result = await sut.getAspectRatio(localAsset); + + expect(result, 1920 / 1080); + verify(() => mockLocalAssetRepository.get('local-1')).called(1); + }); + + test('returns 1.0 when dimensions are still unavailable after fetching', () async { + final remoteAsset = TestUtils.createRemoteAsset(id: 'remote-1', width: null, height: null); + + final exif = const ExifInfo(orientation: '1'); + + when(() => mockRemoteAssetRepository.getExif('remote-1')).thenAnswer((_) async => exif); + when(() => mockRemoteAssetRepository.get('remote-1')).thenAnswer((_) async => null); + + final result = await sut.getAspectRatio(remoteAsset); + + expect(result, 1.0); + }); + + test('returns 1.0 when height is zero', () async { + final remoteAsset = TestUtils.createRemoteAsset(id: 'remote-1', width: 1920, height: 0); + + final exif = const ExifInfo(orientation: '1'); + + when(() => mockRemoteAssetRepository.getExif('remote-1')).thenAnswer((_) async => exif); + + final result = await sut.getAspectRatio(remoteAsset); + + expect(result, 1.0); + }); + + test('handles local asset with remoteId and uses exif from remote', () async { + final localAsset = TestUtils.createLocalAsset( + id: 'local-1', + remoteId: 'remote-1', + width: 1920, + height: 1080, + orientation: 0, + ); + + final exif = const ExifInfo(orientation: '6'); + + when(() => mockRemoteAssetRepository.getExif('remote-1')).thenAnswer((_) async => exif); + + final result = await sut.getAspectRatio(localAsset); + + expect(result, 1080 / 1920); + }); + + test('handles various flipped EXIF orientations correctly', () async { + final flippedOrientations = ['5', '6', '7', '8', '90', '-90']; + + for (final orientation in flippedOrientations) { + final remoteAsset = TestUtils.createRemoteAsset(id: 'remote-$orientation', width: 1920, height: 1080); + + final exif = ExifInfo(orientation: orientation); + + when(() => mockRemoteAssetRepository.getExif('remote-$orientation')).thenAnswer((_) async => exif); + + final result = await sut.getAspectRatio(remoteAsset); + + expect(result, 1080 / 1920, reason: 'Orientation $orientation should flip dimensions'); + } + }); + + test('handles various non-flipped EXIF orientations correctly', () async { + final nonFlippedOrientations = ['1', '2', '3', '4']; + + for (final orientation in nonFlippedOrientations) { + final remoteAsset = TestUtils.createRemoteAsset(id: 'remote-$orientation', width: 1920, height: 1080); + + final exif = ExifInfo(orientation: orientation); + + when(() => mockRemoteAssetRepository.getExif('remote-$orientation')).thenAnswer((_) async => exif); + + final result = await sut.getAspectRatio(remoteAsset); + + expect(result, 1920 / 1080, reason: 'Orientation $orientation should NOT flip dimensions'); + } + }); + }); +} diff --git a/mobile/test/infrastructure/repository.mock.dart b/mobile/test/infrastructure/repository.mock.dart index becfafe33d..aac384c29e 100644 --- a/mobile/test/infrastructure/repository.mock.dart +++ b/mobile/test/infrastructure/repository.mock.dart @@ -4,6 +4,7 @@ import 'package:immich_mobile/infrastructure/repositories/local_album.repository import 'package:immich_mobile/infrastructure/repositories/local_asset.repository.dart'; import 'package:immich_mobile/infrastructure/repositories/log.repository.dart'; import 'package:immich_mobile/infrastructure/repositories/remote_album.repository.dart'; +import 'package:immich_mobile/infrastructure/repositories/remote_asset.repository.dart'; import 'package:immich_mobile/infrastructure/repositories/storage.repository.dart'; import 'package:immich_mobile/infrastructure/repositories/store.repository.dart'; import 'package:immich_mobile/infrastructure/repositories/sync_api.repository.dart'; @@ -35,6 +36,8 @@ class MockLocalAssetRepository extends Mock implements DriftLocalAssetRepository class MockDriftLocalAssetRepository extends Mock implements DriftLocalAssetRepository {} +class MockRemoteAssetRepository extends Mock implements RemoteAssetRepository {} + class MockTrashedLocalAssetRepository extends Mock implements DriftTrashedLocalAssetRepository {} class MockStorageRepository extends Mock implements StorageRepository {} diff --git a/mobile/test/test_utils.dart b/mobile/test/test_utils.dart index 9b59773d3b..498607e3d2 100644 --- a/mobile/test/test_utils.dart +++ b/mobile/test/test_utils.dart @@ -5,6 +5,7 @@ import 'package:easy_localization/easy_localization.dart'; import 'package:fake_async/fake_async.dart'; import 'package:flutter_test/flutter_test.dart'; import 'package:hooks_riverpod/hooks_riverpod.dart'; +import 'package:immich_mobile/domain/models/asset/base_asset.model.dart' as domain; import 'package:immich_mobile/entities/album.entity.dart'; import 'package:immich_mobile/entities/android_device_asset.entity.dart'; import 'package:immich_mobile/entities/asset.entity.dart'; @@ -116,4 +117,43 @@ abstract final class TestUtils { } return result; } + + static domain.RemoteAsset createRemoteAsset({required String id, int? width, int? height, String? ownerId}) { + return domain.RemoteAsset( + id: id, + checksum: 'checksum1', + ownerId: ownerId ?? 'owner1', + name: 'test.jpg', + type: domain.AssetType.image, + createdAt: DateTime(2024, 1, 1), + updatedAt: DateTime(2024, 1, 1), + durationInSeconds: 0, + isFavorite: false, + width: width, + height: height, + ); + } + + static domain.LocalAsset createLocalAsset({ + required String id, + String? remoteId, + int? width, + int? height, + int orientation = 0, + }) { + return domain.LocalAsset( + id: id, + remoteId: remoteId, + checksum: 'checksum1', + name: 'test.jpg', + type: domain.AssetType.image, + createdAt: DateTime(2024, 1, 1), + updatedAt: DateTime(2024, 1, 1), + durationInSeconds: 0, + isFavorite: false, + width: width, + height: height, + orientation: orientation, + ); + } }