mirror of
https://github.com/immich-app/immich
synced 2026-08-15 13:03:57 +00:00
fix(mobile): invalidate stale view intent sessions
Discard stale view-intent resolution and upload results, and clean up session state when the viewer closes.
This commit is contained in:
parent
d0a1f22a38
commit
a1458f8714
5 changed files with 222 additions and 10 deletions
|
|
@ -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<void> 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<void> _uploadViewIntentFile({
|
||||
required LocalAsset asset,
|
||||
required String path,
|
||||
required ViewIntentPayload? activeViewIntent,
|
||||
required Completer<void> 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;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -12,6 +12,13 @@ class ViewIntentCurrentNotifier extends Notifier<ViewIntentPayload?> {
|
|||
void clear() {
|
||||
state = null;
|
||||
}
|
||||
|
||||
void clearIfMatch(ViewIntentPayload payload) {
|
||||
if (!identical(state, payload)) {
|
||||
return;
|
||||
}
|
||||
state = null;
|
||||
}
|
||||
}
|
||||
|
||||
final viewIntentCurrentProvider = NotifierProvider<ViewIntentCurrentNotifier, ViewIntentPayload?>(
|
||||
|
|
|
|||
|
|
@ -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<void> _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);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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<RemoteAsset?>.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<void>(),
|
||||
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<RemoteAsset?>.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<void>(),
|
||||
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));
|
||||
});
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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<String> managedTempPaths = [];
|
||||
final List<String> cleanedManagedTempPaths = [];
|
||||
|
||||
TestViewIntentService() : super(MockViewIntentHostApi());
|
||||
|
||||
|
|
@ -75,6 +77,11 @@ class TestViewIntentService extends ViewIntentService {
|
|||
Future<void> setManagedTempFilePath(String path) async {
|
||||
managedTempPaths.add(path);
|
||||
}
|
||||
|
||||
@override
|
||||
Future<void> 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<ViewIntentResolvedAsset>();
|
||||
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<Object?>(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<Object?>();
|
||||
when(() => router.push<Object?>(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);
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue