From a1458f8714487b27363c554d987412cf6c39d4b0 Mon Sep 17 00:00:00 2001 From: Peter Ombodi Date: Thu, 6 Aug 2026 14:07:02 +0300 Subject: [PATCH] fix(mobile): invalidate stale view intent sessions Discard stale view-intent resolution and upload results, and clean up session state when the viewer closes. --- .../asset_upload_coordinator.provider.dart | 20 ++++- .../view_intent_current.provider.dart | 7 ++ .../view_intent_handler_android.dart | 50 +++++++++-- ...sset_upload_coordinator_provider_test.dart | 89 +++++++++++++++++++ .../view_intent_handler_android_test.dart | 66 +++++++++++++- 5 files changed, 222 insertions(+), 10 deletions(-) diff --git a/mobile/lib/providers/asset_upload_coordinator.provider.dart b/mobile/lib/providers/asset_upload_coordinator.provider.dart index 00a912b95e..caab17ad5e 100644 --- a/mobile/lib/providers/asset_upload_coordinator.provider.dart +++ b/mobile/lib/providers/asset_upload_coordinator.provider.dart @@ -4,8 +4,10 @@ import 'dart:io'; import 'package:hooks_riverpod/hooks_riverpod.dart'; import 'package:immich_mobile/constants/enums.dart'; import 'package:immich_mobile/domain/models/asset/base_asset.model.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/infrastructure/asset.provider.dart'; +import 'package:immich_mobile/providers/view_intent/view_intent_current.provider.dart'; import 'package:immich_mobile/providers/view_intent/view_intent_file_path.provider.dart'; import 'package:immich_mobile/services/foreground_upload.service.dart'; import 'package:immich_mobile/services/view_intent.service.dart'; @@ -25,6 +27,7 @@ class AssetUploadCoordinator { required Completer cancelToken, required UploadCallbacks callbacks, }) async { + final activeViewIntent = source == ActionSource.viewer ? _ref.read(viewIntentCurrentProvider) : null; final viewIntentFilePath = source == ActionSource.viewer ? _ref.read(viewIntentFilePathProvider) : null; if (viewIntentFilePath == null) { final viewerAsset = source == ActionSource.viewer && assets.length == 1 ? assets.single : null; @@ -55,7 +58,12 @@ class AssetUploadCoordinator { final remoteAsset = await _waitForRemoteAsset(remoteAssetId); final latestAsset = _ref.read(assetViewerProvider).currentAsset; - if (remoteAsset == null || latestAsset == null || !latestAsset.refersToSameAsset(viewerAsset)) { + final isCurrentViewIntent = + activeViewIntent == null || identical(_ref.read(viewIntentCurrentProvider), activeViewIntent); + if (remoteAsset == null || + latestAsset == null || + !latestAsset.refersToSameAsset(viewerAsset) || + !isCurrentViewIntent) { return; } @@ -71,6 +79,7 @@ class AssetUploadCoordinator { await _uploadViewIntentFile( asset: assets.single, path: viewIntentFilePath, + activeViewIntent: activeViewIntent, cancelToken: cancelToken, callbacks: callbacks, ); @@ -79,6 +88,7 @@ class AssetUploadCoordinator { Future _uploadViewIntentFile({ required LocalAsset asset, required String path, + required ViewIntentPayload? activeViewIntent, required Completer cancelToken, required UploadCallbacks callbacks, }) async { @@ -106,7 +116,7 @@ class AssetUploadCoordinator { } final remoteAsset = await _waitForRemoteAsset(uploadedRemoteAssetId); - if (remoteAsset == null || !_isCurrentUpload(asset, path)) { + if (remoteAsset == null || !_isCurrentUpload(asset, path, activeViewIntent)) { return; } @@ -136,7 +146,11 @@ class AssetUploadCoordinator { } } - bool _isCurrentUpload(LocalAsset asset, String path) { + bool _isCurrentUpload(LocalAsset asset, String path, ViewIntentPayload? activeViewIntent) { + if (activeViewIntent != null && !identical(_ref.read(viewIntentCurrentProvider), activeViewIntent)) { + return false; + } + if (_ref.read(viewIntentFilePathProvider) != path) { return false; } diff --git a/mobile/lib/providers/view_intent/view_intent_current.provider.dart b/mobile/lib/providers/view_intent/view_intent_current.provider.dart index 9a7af72873..4a64380c9d 100644 --- a/mobile/lib/providers/view_intent/view_intent_current.provider.dart +++ b/mobile/lib/providers/view_intent/view_intent_current.provider.dart @@ -12,6 +12,13 @@ class ViewIntentCurrentNotifier extends Notifier { void clear() { state = null; } + + void clearIfMatch(ViewIntentPayload payload) { + if (!identical(state, payload)) { + return; + } + state = null; + } } final viewIntentCurrentProvider = NotifierProvider( 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 311241aa1c..34b4fb7c67 100644 --- a/mobile/lib/providers/view_intent/view_intent_handler_android.dart +++ b/mobile/lib/providers/view_intent/view_intent_handler_android.dart @@ -69,11 +69,25 @@ class AndroidViewIntentHandler implements ViewIntentHandler { ); if (!_ref.read(authProvider).isAuthenticated) { + _clearCurrentViewIntent(); _ref.read(viewIntentPendingProvider.notifier).defer(attachment); return; } - final resolvedAsset = await _viewIntentAssetResolver.resolve(attachment); + _activateViewIntent(attachment); + + final ViewIntentResolvedAsset resolvedAsset; + try { + resolvedAsset = await _viewIntentAssetResolver.resolve(attachment); + } catch (_) { + _ref.read(viewIntentCurrentProvider.notifier).clearIfMatch(attachment); + rethrow; + } + if (!identical(_ref.read(viewIntentCurrentProvider), attachment)) { + await resolvedAsset.timelineService.dispose(); + return; + } + _logger.fine('resolved view intent asset: ${resolvedAsset.asset}'); await _openAssetViewer( asset: resolvedAsset.asset, @@ -95,10 +109,17 @@ class AndroidViewIntentHandler implements ViewIntentHandler { return false; } + final reopenedAttachment = ViewIntentPayload( + path: attachment.path, + mimeType: attachment.mimeType, + localAssetId: attachment.localAssetId, + ); + _activateViewIntent(reopenedAttachment); + final origin = asset.isTrashed ? TimelineOrigin.deepLinkTrash : TimelineOrigin.deepLink; final timelineService = _ref.read(timelineFactoryProvider).fromAssets([asset], origin); unawaited( - _openAssetViewer(asset: asset, timelineService: timelineService, attachment: attachment).catchError(( + _openAssetViewer(asset: asset, timelineService: timelineService, attachment: reopenedAttachment).catchError(( Object error, StackTrace stackTrace, ) { @@ -108,6 +129,19 @@ class AndroidViewIntentHandler implements ViewIntentHandler { return true; } + void _activateViewIntent(ViewIntentPayload attachment) { + _ref.read(viewIntentCurrentProvider.notifier).setPayload(attachment); + _ref.read(viewIntentFilePathProvider.notifier).clear(); + unawaited(_viewIntentService.cleanupManagedTempFile()); + _router.popUntilRoot(); + } + + void _clearCurrentViewIntent() { + _ref.read(viewIntentCurrentProvider.notifier).clear(); + _ref.read(viewIntentFilePathProvider.notifier).clear(); + unawaited(_viewIntentService.cleanupManagedTempFile()); + } + Future _openAssetViewer({ required BaseAsset asset, required TimelineService timelineService, @@ -119,7 +153,6 @@ class AndroidViewIntentHandler implements ViewIntentHandler { if (asset.isVideo) { notifier.setControls(false); } - _ref.read(viewIntentCurrentProvider.notifier).setPayload(attachment); notifier.setAsset(asset); if (viewIntentFilePath != null) { @@ -130,7 +163,14 @@ class AndroidViewIntentHandler implements ViewIntentHandler { unawaited(_viewIntentService.cleanupManagedTempFile()); } - _router.popUntilRoot(); - await _router.push(AssetViewerRoute(initialIndex: 0, timelineService: timelineService)); + try { + await _router.push(AssetViewerRoute(initialIndex: 0, timelineService: timelineService)); + } finally { + _ref.read(viewIntentCurrentProvider.notifier).clearIfMatch(attachment); + if (viewIntentFilePath != null) { + _ref.read(viewIntentFilePathProvider.notifier).clearIfMatch(viewIntentFilePath); + await _viewIntentService.cleanupManagedTempFileIfCurrent(viewIntentFilePath); + } + } } } diff --git a/mobile/test/providers/asset_upload_coordinator_provider_test.dart b/mobile/test/providers/asset_upload_coordinator_provider_test.dart index ba5b04d3b7..882b54c8f4 100644 --- a/mobile/test/providers/asset_upload_coordinator_provider_test.dart +++ b/mobile/test/providers/asset_upload_coordinator_provider_test.dart @@ -5,9 +5,11 @@ import 'package:flutter_test/flutter_test.dart'; import 'package:hooks_riverpod/hooks_riverpod.dart'; import 'package:immich_mobile/constants/enums.dart'; import 'package:immich_mobile/domain/models/asset/base_asset.model.dart'; +import 'package:immich_mobile/platform/view_intent_api.g.dart'; import 'package:immich_mobile/providers/asset_upload_coordinator.provider.dart'; import 'package:immich_mobile/providers/asset_viewer/asset_viewer.provider.dart'; import 'package:immich_mobile/providers/infrastructure/asset.provider.dart'; +import 'package:immich_mobile/providers/view_intent/view_intent_current.provider.dart'; import 'package:immich_mobile/providers/view_intent/view_intent_file_path.provider.dart'; import 'package:immich_mobile/services/foreground_upload.service.dart'; import 'package:immich_mobile/services/view_intent.service.dart'; @@ -80,6 +82,45 @@ void main() { expect(currentAsset?.isMerged, isTrue); }); + test('does not let a device-backed upload from an older view intent replace the viewer', () async { + final oldPayload = ViewIntentPayload(path: '/tmp/old.jpg', mimeType: 'image/jpeg', localAssetId: 'local-old'); + final newPayload = ViewIntentPayload(path: '/tmp/new.jpg', mimeType: 'image/jpeg', localAssetId: 'local-new'); + final localAsset = LocalAssetFactory.create(id: 'local-old'); + final remoteAsset = RemoteAssetFactory.create(id: 'remote-old'); + final remoteController = StreamController.broadcast(); + addTearDown(remoteController.close); + + container.read(viewIntentCurrentProvider.notifier).setPayload(oldPayload); + container.read(assetViewerProvider.notifier).setAsset(localAsset); + when(() => assetService.watchRemoteAsset(remoteAsset.id)).thenAnswer((_) => remoteController.stream); + when( + () => uploadService.uploadManual( + any(), + cancelToken: any(named: 'cancelToken'), + callbacks: any(named: 'callbacks'), + ), + ).thenAnswer((invocation) async { + final callbacks = invocation.namedArguments[#callbacks] as UploadCallbacks; + callbacks.onSuccess?.call(localAsset.id, remoteAsset.id); + }); + + final upload = container + .read(assetUploadCoordinatorProvider) + .upload( + source: ActionSource.viewer, + assets: [localAsset], + cancelToken: Completer(), + callbacks: const UploadCallbacks(), + ); + await pumpEventQueue(); + + container.read(viewIntentCurrentProvider.notifier).setPayload(newPayload); + remoteController.add(remoteAsset); + await upload; + + expect(container.read(assetViewerProvider).currentAsset, same(localAsset)); + }); + test('uploads a path-only viewer asset as a file and replaces it with the synchronized remote asset', () async { const path = 'C:/cache/view_intent_1.jpg'; final localAsset = LocalAssetFactory.create(id: '-1'); @@ -273,4 +314,52 @@ void main() { verifyNever(() => viewIntentService.cleanupManagedTempFileIfCurrent(oldPath)); verify(() => viewIntentService.markUploadInactive(oldPath)).called(1); }); + + test('does not let an older file upload replace a newer session for the same asset', () async { + const path = 'C:/cache/view_intent_same.jpg'; + final oldPayload = ViewIntentPayload(path: path, mimeType: 'image/jpeg'); + final newPayload = ViewIntentPayload(path: path, mimeType: 'image/jpeg'); + final localAsset = LocalAssetFactory.create(id: '-6'); + final uploadedRemote = RemoteAssetFactory.create(id: 'remote-same'); + final remoteController = StreamController.broadcast(); + addTearDown(remoteController.close); + + container.read(viewIntentCurrentProvider.notifier).setPayload(oldPayload); + container.read(viewIntentFilePathProvider.notifier).setPath(path); + container.read(assetViewerProvider.notifier).setAsset(localAsset); + when(() => viewIntentService.markUploadActive(path)).thenReturn(null); + when(() => viewIntentService.cleanupManagedTempFileIfCurrent(path)).thenAnswer((_) async {}); + when(() => viewIntentService.markUploadInactive(path)).thenAnswer((_) async {}); + when(() => assetService.watchRemoteAsset(uploadedRemote.id)).thenAnswer((_) => remoteController.stream); + when( + () => uploadService.uploadShareIntent( + any(), + cancelToken: any(named: 'cancelToken'), + onProgress: any(named: 'onProgress'), + onSuccess: any(named: 'onSuccess'), + onError: any(named: 'onError'), + ), + ).thenAnswer((invocation) async { + final onSuccess = invocation.namedArguments[#onSuccess] as void Function(String, String)?; + onSuccess?.call('file-id', uploadedRemote.id); + }); + + final upload = container + .read(assetUploadCoordinatorProvider) + .upload( + source: ActionSource.viewer, + assets: [localAsset], + cancelToken: Completer(), + callbacks: const UploadCallbacks(), + ); + await pumpEventQueue(); + + container.read(viewIntentCurrentProvider.notifier).setPayload(newPayload); + remoteController.add(uploadedRemote); + await upload; + + expect(container.read(assetViewerProvider).currentAsset, same(localAsset)); + expect(container.read(viewIntentFilePathProvider), path); + verifyNever(() => viewIntentService.cleanupManagedTempFileIfCurrent(path)); + }); } 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 5d6d24698c..f27f5b62ce 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 @@ -15,6 +15,7 @@ import 'package:immich_mobile/providers/auth.provider.dart'; import 'package:immich_mobile/providers/infrastructure/asset.provider.dart'; import 'package:immich_mobile/providers/infrastructure/timeline.provider.dart'; import 'package:immich_mobile/providers/view_intent/view_intent_current.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_android.dart'; import 'package:immich_mobile/providers/view_intent/view_intent_pending.provider.dart'; import 'package:immich_mobile/routing/router.dart'; @@ -55,6 +56,7 @@ class TestViewIntentService extends ViewIntentService { int cleanupStaleTempFilesCalls = 0; int cleanupManagedTempFileCalls = 0; final List managedTempPaths = []; + final List cleanedManagedTempPaths = []; TestViewIntentService() : super(MockViewIntentHostApi()); @@ -75,6 +77,11 @@ class TestViewIntentService extends ViewIntentService { Future setManagedTempFilePath(String path) async { managedTempPaths.add(path); } + + @override + Future cleanupManagedTempFileIfCurrent(String path) async { + cleanedManagedTempPaths.add(path); + } } class TestAuthNotifier extends AuthNotifier { @@ -252,12 +259,12 @@ void main() { await handler.handle(payload); expect(container.read(assetViewerProvider).currentAsset, deepLinkAsset); - expect(container.read(viewIntentCurrentProvider), payload); + expect(container.read(viewIntentCurrentProvider), isNull); await handler.handle(secondPayload); expect(container.read(assetViewerProvider).currentAsset, secondAsset); - expect(container.read(viewIntentCurrentProvider), secondPayload); + expect(container.read(viewIntentCurrentProvider), isNull); verify(() => resolver.resolve(payload)).called(1); verify(() => resolver.resolve(secondPayload)).called(1); verify(() => router.popUntilRoot()).called(2); @@ -266,6 +273,61 @@ void main() { verifyNever(() => router.replaceAll(any())); }); + test('a slower view intent cannot replace a newer one', () async { + final firstResolution = Completer(); + final secondPayload = ViewIntentPayload( + path: '/tmp/incoming-b.jpg', + mimeType: 'image/jpeg', + localAssetId: 'local-2', + ); + final secondAsset = _localAsset(id: 'local-2'); + final secondTimelineService = await _createReadyTimelineService([secondAsset], TimelineOrigin.deepLink); + addTearDown(secondTimelineService.dispose); + + when(() => resolver.resolve(payload)).thenAnswer((_) => firstResolution.future); + when( + () => resolver.resolve(secondPayload), + ).thenAnswer((_) async => ViewIntentResolvedAsset(asset: secondAsset, timelineService: secondTimelineService)); + + final firstHandle = handler.handle(payload); + await pumpEventQueue(); + await handler.handle(secondPayload); + + firstResolution.complete(ViewIntentResolvedAsset(asset: deepLinkAsset, timelineService: deepLinkTimelineService)); + await firstHandle; + + expect(container.read(assetViewerProvider).currentAsset, secondAsset); + expect(container.read(viewIntentCurrentProvider), isNull); + verify(() => router.popUntilRoot()).called(2); + verify(() => router.push(any())).called(1); + }); + + test('closing a file-backed view intent clears only its session state', () async { + const path = '/tmp/view_intent_1.jpg'; + final routeClosed = Completer(); + when(() => router.push(any())).thenAnswer((_) => routeClosed.future); + when(() => resolver.resolve(payload)).thenAnswer( + (_) async => ViewIntentResolvedAsset( + asset: deepLinkAsset, + timelineService: deepLinkTimelineService, + viewIntentFilePath: path, + ), + ); + + final handling = handler.handle(payload); + await pumpEventQueue(); + + expect(container.read(viewIntentCurrentProvider), same(payload)); + expect(container.read(viewIntentFilePathProvider), path); + + routeClosed.complete(null); + await handling; + + expect(container.read(viewIntentCurrentProvider), isNull); + expect(container.read(viewIntentFilePathProvider), isNull); + expect(viewIntentService.cleanedManagedTempPaths, [path]); + }); + test('reopenRemoteAsset opens the restored asset in a regular deep-link timeline', () async { final restoredAsset = _remoteAsset(id: 'remote-1', localId: 'local-1'); final restoredTimeline = await _createReadyTimelineService([restoredAsset], TimelineOrigin.deepLink);