From e2f5308cba21cdf28fbd1884aad869ddaefa5425 Mon Sep 17 00:00:00 2001 From: Peter Ombodi Date: Mon, 27 Apr 2026 13:28:53 +0300 Subject: [PATCH] refactor(mobile): decouple view intent asset resolver from providers --- .../view_intent_handler_android.dart | 17 +++- .../view_intent_asset_resolver.service.dart | 88 +++++++++++-------- .../view_intent_handler_android_test.dart | 69 +++++++++++++-- .../view_intent_asset_resolver_test.dart | 54 +++++------- 4 files changed, 151 insertions(+), 77 deletions(-) diff --git a/mobile/lib/providers/view_intent/view_intent_handler_android.dart b/mobile/lib/providers/view_intent/view_intent_handler_android.dart index 950b7a83fc..c6e6cdd46f 100644 --- a/mobile/lib/providers/view_intent/view_intent_handler_android.dart +++ b/mobile/lib/providers/view_intent/view_intent_handler_android.dart @@ -7,6 +7,7 @@ import 'package:immich_mobile/domain/services/timeline.service.dart'; import 'package:immich_mobile/platform/view_intent_api.g.dart'; import 'package:immich_mobile/providers/asset_viewer/asset_viewer.provider.dart'; import 'package:immich_mobile/providers/auth.provider.dart'; +import 'package:immich_mobile/providers/infrastructure/timeline.provider.dart'; import 'package:immich_mobile/providers/view_intent/view_intent_file_path.provider.dart'; import 'package:immich_mobile/providers/view_intent/view_intent_handler.provider.dart'; import 'package:immich_mobile/providers/view_intent/view_intent_main_timeline_ready.provider.dart'; @@ -82,7 +83,11 @@ class AndroidViewIntentHandler implements ViewIntentHandler { return; } - final resolvedAsset = await _viewIntentAssetResolver.resolve(attachment); + final resolvedAsset = await _viewIntentAssetResolver.resolve( + attachment, + timelineUsers: _resolveMainTimelineUsers(), + mainTimelineService: _ref.read(timelineServiceProvider), + ); _logger.fine('resolved view intent asset: ${resolvedAsset.asset}'); await _openAssetViewer( resolvedAsset.asset, @@ -92,6 +97,16 @@ class AndroidViewIntentHandler implements ViewIntentHandler { ); } + List _resolveMainTimelineUsers() { + final timelineUsers = _ref.read(timelineUsersProvider).valueOrNull; + final currentUserId = _ref.read(authProvider).userId; + final effectiveTimelineUsers = timelineUsers != null && timelineUsers.isNotEmpty ? timelineUsers : [currentUserId]; + _logger.fine( + 'resolve main timeline users source, timelineUsers=$timelineUsers, currentUserId=$currentUserId, effective=$effectiveTimelineUsers', + ); + return effectiveTimelineUsers; + } + Future _openAssetViewer( BaseAsset asset, TimelineService timelineService, diff --git a/mobile/lib/services/view_intent_asset_resolver.service.dart b/mobile/lib/services/view_intent_asset_resolver.service.dart index 932e136243..14e5907ee9 100644 --- a/mobile/lib/services/view_intent_asset_resolver.service.dart +++ b/mobile/lib/services/view_intent_asset_resolver.service.dart @@ -4,13 +4,13 @@ import 'package:hooks_riverpod/hooks_riverpod.dart'; import 'package:immich_mobile/domain/models/asset/base_asset.model.dart'; import 'package:immich_mobile/domain/services/timeline.service.dart'; import 'package:immich_mobile/infrastructure/repositories/local_asset.repository.dart'; +import 'package:immich_mobile/infrastructure/repositories/timeline.repository.dart'; import 'package:immich_mobile/models/view_intent/view_intent_payload.extension.dart'; import 'package:immich_mobile/platform/native_sync_api.g.dart'; import 'package:immich_mobile/platform/view_intent_api.g.dart'; import 'package:immich_mobile/providers/infrastructure/asset.provider.dart'; import 'package:immich_mobile/providers/infrastructure/platform.provider.dart'; import 'package:immich_mobile/providers/infrastructure/timeline.provider.dart'; -import 'package:immich_mobile/providers/user.provider.dart'; import 'package:logging/logging.dart'; class ViewIntentResolvedAsset { @@ -29,23 +29,35 @@ class ViewIntentResolvedAsset { final viewIntentAssetResolverProvider = Provider( (ref) => ViewIntentAssetResolver( - ref, - ref.read(localAssetRepository), - ref.read(nativeSyncApiProvider), - ref.read(timelineFactoryProvider), + localAssetRepository: ref.read(localAssetRepository), + nativeSyncApi: ref.read(nativeSyncApiProvider), + timelineFactory: ref.read(timelineFactoryProvider), + timelineRepository: ref.read(timelineRepositoryProvider), ), ); class ViewIntentAssetResolver { - final Ref _ref; final DriftLocalAssetRepository _localAssetRepository; final NativeSyncApi _nativeSyncApi; final TimelineFactory _timelineFactory; + final DriftTimelineRepository _timelineRepository; static final Logger _logger = Logger('ViewIntentAssetResolver'); - const ViewIntentAssetResolver(this._ref, this._localAssetRepository, this._nativeSyncApi, this._timelineFactory); + const ViewIntentAssetResolver({ + required DriftLocalAssetRepository localAssetRepository, + required NativeSyncApi nativeSyncApi, + required TimelineFactory timelineFactory, + required DriftTimelineRepository timelineRepository, + }) : _localAssetRepository = localAssetRepository, + _nativeSyncApi = nativeSyncApi, + _timelineFactory = timelineFactory, + _timelineRepository = timelineRepository; - Future resolve(ViewIntentPayload attachment) async { + Future resolve( + ViewIntentPayload attachment, { + required List timelineUsers, + required TimelineService mainTimelineService, + }) async { final localAssetId = attachment.localAssetId; final path = attachment.path; _logger.fine('resolve start, localAssetId=$localAssetId, path=$path, mimeType=${attachment.mimeType}'); @@ -56,7 +68,11 @@ class ViewIntentAssetResolver { if (localAssetId != null) { // Try the direct local-id match first when the intent resolves to a real // MediaStore asset. - final mainTimelineAsset = await _resolveMainTimelineAssetByLocalId(localAssetId); + final mainTimelineAsset = await _resolveMainTimelineAssetByLocalId( + localAssetId, + timelineUsers, + mainTimelineService, + ); if (mainTimelineAsset != null) { _logger.fine('presenting main timeline asset via localAssetId: ${mainTimelineAsset.asset}'); return mainTimelineAsset; @@ -69,7 +85,7 @@ class ViewIntentAssetResolver { final checksum = await _resolveChecksumForMatching(attachment, localAsset: localAsset); _logger.fine('resolve checksum for matching: $checksum'); if (checksum != null) { - final mainTimelineAsset = await _resolveMainTimelineAssetByChecksum(checksum); + final mainTimelineAsset = await _resolveMainTimelineAssetByChecksum(checksum, timelineUsers, mainTimelineService); if (mainTimelineAsset != null) { final lookupType = localAssetId != null ? 'checksum fallback' : 'checksum-only match'; _logger.fine('presenting main timeline asset via $lookupType: ${mainTimelineAsset.asset}'); @@ -92,61 +108,59 @@ class ViewIntentAssetResolver { ); } - Future _resolveMainTimelineAssetByLocalId(String localAssetId) async { + Future _resolveMainTimelineAssetByLocalId( + String localAssetId, + List timelineUsers, + TimelineService mainTimelineService, + ) async { _logger.fine('resolve main timeline by localId start: $localAssetId'); return _resolveMainTimelineAsset( - (effectiveTimelineUsers) => - _ref.read(timelineRepositoryProvider).getMainTimelineIndexByLocalId(effectiveTimelineUsers, localAssetId), + () => _timelineRepository.getMainTimelineIndexByLocalId(timelineUsers, localAssetId), + timelineUsers: timelineUsers, + mainTimelineService: mainTimelineService, lookupLabel: 'localId=$localAssetId', ); } - Future _resolveMainTimelineAssetByChecksum(String checksum) async { + Future _resolveMainTimelineAssetByChecksum( + String checksum, + List timelineUsers, + TimelineService mainTimelineService, + ) async { // Some ACTION_VIEW sources do not provide a local MediaStore id, so // checksum is the only way to match the incoming file to an existing // merged asset. _logger.fine('resolve main timeline by checksum start: $checksum'); return _resolveMainTimelineAsset( - (effectiveTimelineUsers) => - _ref.read(timelineRepositoryProvider).getMainTimelineIndexByChecksum(effectiveTimelineUsers, checksum), + () => _timelineRepository.getMainTimelineIndexByChecksum(timelineUsers, checksum), + timelineUsers: timelineUsers, + mainTimelineService: mainTimelineService, lookupLabel: 'checksum=$checksum', ); } Future _resolveMainTimelineAsset( - Future Function(List effectiveTimelineUsers) findIndex, { + Future Function() findIndex, { + required List timelineUsers, + required TimelineService mainTimelineService, required String lookupLabel, }) async { - final effectiveTimelineUsers = _resolveMainTimelineUsers(); - _logger.fine('resolve main timeline users for $lookupLabel: $effectiveTimelineUsers'); - if (effectiveTimelineUsers.isEmpty) { - _logger.fine('resolve main timeline aborted for $lookupLabel: effectiveTimelineUsers is empty'); + _logger.fine('resolve main timeline users for $lookupLabel: $timelineUsers'); + if (timelineUsers.isEmpty) { + _logger.fine('resolve main timeline aborted for $lookupLabel: timelineUsers is empty'); return null; } - final index = await findIndex(effectiveTimelineUsers); + final index = await findIndex(); _logger.fine('resolve main timeline index for $lookupLabel: $index'); if (index == null) { return null; } - return _resolveMainTimelineAssetAt(index); + return _resolveMainTimelineAssetAt(index, mainTimelineService); } - List _resolveMainTimelineUsers() { - final timelineUsers = _ref.read(timelineUsersProvider).valueOrNull; - final currentUserId = _ref.read(currentUserProvider)?.id; - final effectiveTimelineUsers = timelineUsers != null && timelineUsers.isNotEmpty - ? timelineUsers - : (currentUserId != null ? [currentUserId] : const []); - _logger.fine( - 'resolve main timeline users source, timelineUsers=$timelineUsers, currentUserId=$currentUserId, effective=$effectiveTimelineUsers', - ); - return effectiveTimelineUsers; - } - - Future _resolveMainTimelineAssetAt(int index) async { - final timelineService = _ref.read(timelineServiceProvider); + Future _resolveMainTimelineAssetAt(int index, TimelineService timelineService) async { _logger.fine( 'resolve main timeline asset at index start: index=$index, origin=${timelineService.origin}, totalAssets=${timelineService.totalAssets}', ); diff --git a/mobile/test/providers/view_intent/view_intent_handler_android_test.dart b/mobile/test/providers/view_intent/view_intent_handler_android_test.dart index 8353f473a2..6bf94d39d7 100644 --- a/mobile/test/providers/view_intent/view_intent_handler_android_test.dart +++ b/mobile/test/providers/view_intent/view_intent_handler_android_test.dart @@ -41,6 +41,8 @@ class MockWidgetService extends Mock implements WidgetService {} class FakePageRouteInfo extends Fake implements PageRouteInfo {} +class FakeTimelineService extends Fake implements TimelineService {} + class TestViewIntentService extends ViewIntentService { ViewIntentPayload? consumedAttachment; int cleanupStaleTempFilesCalls = 0; @@ -107,6 +109,8 @@ void main() { registerFallbackValue(FakePageRouteInfo()); registerFallbackValue(_pageRoutePredicate); registerFallbackValue(_localAsset(id: 'fallback')); + registerFallbackValue([]); + registerFallbackValue(FakeTimelineService()); }); setUp(() async { @@ -135,6 +139,7 @@ void main() { ); authNotifier = container.read(authProvider.notifier) as TestAuthNotifier; + await container.read(timelineUsersProvider.future); handler = container.read(_handlerProvider); addTearDown(() async { @@ -149,7 +154,13 @@ void main() { await handler.handle(payload); expect(container.read(viewIntentPendingProvider), payload); - verifyNever(() => resolver.resolve(payload)); + verifyNever( + () => resolver.resolve( + payload, + timelineUsers: any(named: 'timelineUsers'), + mainTimelineService: any(named: 'mainTimelineService'), + ), + ); }); testWidgets('flushDeferredViewIntent waits for main timeline readiness before flushing pending attachment', ( @@ -159,7 +170,13 @@ void main() { container.read(viewIntentPendingProvider.notifier).defer(payload); authNotifier.setAuthenticated(true); - when(() => resolver.resolve(payload)).thenAnswer((_) async { + when( + () => resolver.resolve( + payload, + timelineUsers: any(named: 'timelineUsers'), + mainTimelineService: any(named: 'mainTimelineService'), + ), + ).thenAnswer((_) async { return ViewIntentResolvedAsset(asset: deepLinkAsset, timelineService: deepLinkTimelineService, initialIndex: 0); }); @@ -167,7 +184,13 @@ void main() { await tester.pump(); expect(container.read(viewIntentPendingProvider), payload); - verifyNever(() => resolver.resolve(payload)); + verifyNever( + () => resolver.resolve( + payload, + timelineUsers: any(named: 'timelineUsers'), + mainTimelineService: any(named: 'mainTimelineService'), + ), + ); container.read(viewIntentMainTimelineReadyProvider.notifier).markMountedOnce(); await tester.pump(); @@ -175,13 +198,21 @@ void main() { await tester.idle(); expect(container.read(viewIntentPendingProvider), isNull); - verify(() => resolver.resolve(payload)).called(1); + verify( + () => resolver.resolve(payload, timelineUsers: ['user-1'], mainTimelineService: deepLinkTimelineService), + ).called(1); }); test('flushDeferredViewIntent does nothing when there is no pending attachment', () async { await handler.flushDeferredViewIntent(); - verifyNever(() => resolver.resolve(payload)); + verifyNever( + () => resolver.resolve( + payload, + timelineUsers: any(named: 'timelineUsers'), + mainTimelineService: any(named: 'mainTimelineService'), + ), + ); }); test('onAppResumed cleans stale temp files when no attachment is present', () async { @@ -190,7 +221,13 @@ void main() { await handler.onAppResumed(); expect(viewIntentService.cleanupStaleTempFilesCalls, 1); - verifyNever(() => resolver.resolve(payload)); + verifyNever( + () => resolver.resolve( + payload, + timelineUsers: any(named: 'timelineUsers'), + mainTimelineService: any(named: 'mainTimelineService'), + ), + ); }); test('onAppResumed does not clean stale temp files while pending attachment exists', () async { @@ -200,12 +237,24 @@ void main() { await handler.onAppResumed(); expect(viewIntentService.cleanupStaleTempFilesCalls, 0); - verifyNever(() => resolver.resolve(payload)); + verifyNever( + () => resolver.resolve( + payload, + timelineUsers: any(named: 'timelineUsers'), + mainTimelineService: any(named: 'mainTimelineService'), + ), + ); }); testWidgets('onAppResumed handles attachment immediately when authenticated', (tester) async { viewIntentService.consumedAttachment = payload; - when(() => resolver.resolve(payload)).thenAnswer( + when( + () => resolver.resolve( + payload, + timelineUsers: any(named: 'timelineUsers'), + mainTimelineService: any(named: 'mainTimelineService'), + ), + ).thenAnswer( (_) async => ViewIntentResolvedAsset(asset: deepLinkAsset, timelineService: deepLinkTimelineService, initialIndex: 0), ); @@ -216,7 +265,9 @@ void main() { await tester.pump(); await tester.idle(); - verify(() => resolver.resolve(payload)).called(1); + verify( + () => resolver.resolve(payload, timelineUsers: ['user-1'], mainTimelineService: deepLinkTimelineService), + ).called(1); }); } diff --git a/mobile/test/services/view_intent_asset_resolver_test.dart b/mobile/test/services/view_intent_asset_resolver_test.dart index a48562c41a..b6df1850c2 100644 --- a/mobile/test/services/view_intent_asset_resolver_test.dart +++ b/mobile/test/services/view_intent_asset_resolver_test.dart @@ -5,18 +5,15 @@ import 'package:hooks_riverpod/hooks_riverpod.dart'; import 'package:immich_mobile/domain/models/asset/base_asset.model.dart'; import 'package:immich_mobile/domain/models/timeline.model.dart'; import 'package:immich_mobile/domain/services/timeline.service.dart'; -import 'package:immich_mobile/domain/services/user.service.dart'; import 'package:immich_mobile/infrastructure/repositories/timeline.repository.dart'; import 'package:immich_mobile/platform/native_sync_api.g.dart'; import 'package:immich_mobile/platform/view_intent_api.g.dart'; import 'package:immich_mobile/providers/infrastructure/asset.provider.dart'; import 'package:immich_mobile/providers/infrastructure/platform.provider.dart'; import 'package:immich_mobile/providers/infrastructure/timeline.provider.dart'; -import 'package:immich_mobile/providers/user.provider.dart'; import 'package:immich_mobile/services/view_intent_asset_resolver.service.dart'; import 'package:mocktail/mocktail.dart'; -import '../fixtures/user.stub.dart'; import '../infrastructure/repository.mock.dart'; class MockTimelineRepository extends Mock implements DriftTimelineRepository {} @@ -25,20 +22,11 @@ class MockTimelineFactory extends Mock implements TimelineFactory {} class MockNativeSyncApi extends Mock implements NativeSyncApi {} -class MockUserService extends Mock implements UserService {} - -class _StaticCurrentUserProvider extends CurrentUserProvider { - _StaticCurrentUserProvider(UserService userService) : super(userService) { - state = userService.tryGetMyUser(); - } -} - void main() { late MockDriftLocalAssetRepository mockLocalAssetRepository; late MockNativeSyncApi nativeSyncApi; late MockTimelineRepository timelineRepository; late MockTimelineFactory timelineFactory; - late MockUserService userService; late TimelineService mainTimelineService; late List createdTimelineServices; late ProviderContainer container; @@ -48,13 +36,9 @@ void main() { nativeSyncApi = MockNativeSyncApi(); timelineRepository = MockTimelineRepository(); timelineFactory = MockTimelineFactory(); - userService = MockUserService(); createdTimelineServices = []; mainTimelineService = await _setMainTimelineService(const [], createdTimelineServices); - when(() => userService.tryGetMyUser()).thenReturn(UserStub.admin); - when(() => userService.watchMyUser()).thenAnswer((_) => Stream.value(UserStub.admin)); - when(() => timelineFactory.fromAssets(any(), TimelineOrigin.deepLink)).thenAnswer((invocation) { final assets = List.from(invocation.positionalArguments[0] as List); final timelineService = _timelineServiceFromAssets(assets, TimelineOrigin.deepLink); @@ -68,12 +52,8 @@ void main() { nativeSyncApiProvider.overrideWith((ref) => nativeSyncApi), timelineRepositoryProvider.overrideWith((ref) => timelineRepository), timelineFactoryProvider.overrideWith((ref) => timelineFactory), - timelineServiceProvider.overrideWith((ref) => mainTimelineService), - timelineUsersProvider.overrideWith((ref) => Stream.value(['user-1'])), - currentUserProvider.overrideWith((ref) => _StaticCurrentUserProvider(userService)), ], ); - await container.read(timelineUsersProvider.future); addTearDown(() async { for (final timelineService in createdTimelineServices) { @@ -91,7 +71,7 @@ void main() { when(() => mockLocalAssetRepository.getById('local-1')).thenAnswer((_) async => localAsset); when(() => timelineRepository.getMainTimelineIndexByLocalId(['user-1'], 'local-1')).thenAnswer((_) async => 0); - final result = await container.read(viewIntentAssetResolverProvider).resolve(_payload(localAssetId: 'local-1')); + final result = await _resolve(container, _payload(localAssetId: 'local-1'), mainTimelineService); expect(result.asset, same(mainAsset)); expect(result.timelineService, same(mainTimelineService)); @@ -110,7 +90,7 @@ void main() { when(() => timelineRepository.getMainTimelineIndexByLocalId(['user-1'], 'local-1')).thenAnswer((_) async => null); when(() => timelineRepository.getMainTimelineIndexByChecksum(['user-1'], 'checksum-1')).thenAnswer((_) async => 0); - final result = await container.read(viewIntentAssetResolverProvider).resolve(_payload(localAssetId: 'local-1')); + final result = await _resolve(container, _payload(localAssetId: 'local-1'), mainTimelineService); expect(result.asset, same(mainAsset)); expect(result.timelineService, same(mainTimelineService)); @@ -129,7 +109,7 @@ void main() { ).thenAnswer((_) async => [HashResult(assetId: 'local-1', hash: 'checksum-1')]); when(() => timelineRepository.getMainTimelineIndexByChecksum(['user-1'], 'checksum-1')).thenAnswer((_) async => 0); - final result = await container.read(viewIntentAssetResolverProvider).resolve(_payload(localAssetId: 'local-1')); + final result = await _resolve(container, _payload(localAssetId: 'local-1'), mainTimelineService); expect(result.asset, same(mainAsset)); expect(result.timelineService, same(mainTimelineService)); @@ -143,7 +123,7 @@ void main() { when(() => timelineRepository.getMainTimelineIndexByLocalId(['user-1'], 'local-1')).thenAnswer((_) async => null); when(() => nativeSyncApi.hashAssets(['local-1'], allowNetworkAccess: false)).thenThrow(Exception('hash failed')); - final result = await container.read(viewIntentAssetResolverProvider).resolve(_payload(localAssetId: 'local-1')); + final result = await _resolve(container, _payload(localAssetId: 'local-1'), mainTimelineService); expect(result.asset, equals(localAsset)); expect(result.timelineService.origin, TimelineOrigin.deepLink); @@ -160,9 +140,11 @@ void main() { ).thenAnswer((_) async => [HashResult(assetId: '/tmp/incoming.jpg', hash: 'checksum-2')]); when(() => timelineRepository.getMainTimelineIndexByChecksum(['user-1'], 'checksum-2')).thenAnswer((_) async => 0); - final result = await container - .read(viewIntentAssetResolverProvider) - .resolve(_payload(path: '/tmp/incoming.jpg', localAssetId: null)); + final result = await _resolve( + container, + _payload(path: '/tmp/incoming.jpg', localAssetId: null), + mainTimelineService, + ); expect(result.asset, same(mainAsset)); expect(result.timelineService, same(mainTimelineService)); @@ -172,9 +154,11 @@ void main() { test('returns transient deep-link asset for unmatched path-only attachment', () async { when(() => nativeSyncApi.hashFiles(['/tmp/incoming.webp'])).thenAnswer((_) async => const []); - final result = await container - .read(viewIntentAssetResolverProvider) - .resolve(_payload(path: '/tmp/incoming.webp', localAssetId: null, mimeType: 'image/webp')); + final result = await _resolve( + container, + _payload(path: '/tmp/incoming.webp', localAssetId: null, mimeType: 'image/webp'), + mainTimelineService, + ); expect(result.asset, isA()); expect(result.timelineService.origin, TimelineOrigin.deepLink); @@ -189,6 +173,16 @@ void main() { }); } +Future _resolve( + ProviderContainer container, + ViewIntentPayload payload, + TimelineService mainTimelineService, +) { + return container + .read(viewIntentAssetResolverProvider) + .resolve(payload, timelineUsers: const ['user-1'], mainTimelineService: mainTimelineService); +} + ViewIntentPayload _payload({String? localAssetId = 'local-1', String? path, String mimeType = 'image/jpeg'}) { return ViewIntentPayload(path: path, mimeType: mimeType, localAssetId: localAssetId); }