mirror of
https://github.com/immich-app/immich.git
synced 2026-07-28 14:47:30 -07:00
fix(mobile): keep the original filename when sharing downloaded assets
This commit is contained in:
@@ -22,7 +22,7 @@ import 'package:path/path.dart' as p;
|
||||
import 'package:photo_manager/photo_manager.dart';
|
||||
import 'package:share_plus/share_plus.dart';
|
||||
|
||||
typedef _ShareFile = ({File file, bool cleanup, String displayName});
|
||||
typedef _ShareFile = ({File file, FileSystemEntity? tempEntity, String displayName});
|
||||
|
||||
final assetMediaRepositoryProvider = Provider(
|
||||
(ref) => AssetMediaRepository(ref.watch(nativeSyncApiProvider), ref.watch(storageRepositoryProvider)),
|
||||
@@ -102,30 +102,60 @@ class AssetMediaRepository {
|
||||
}
|
||||
}
|
||||
|
||||
/// Deletes temporary files in parallel
|
||||
Future<void> _cleanupTempFiles(List<File> tempFiles) async {
|
||||
/// Deletes temporary entities in parallel, removing download task directories recursively
|
||||
@visibleForTesting
|
||||
static Future<void> cleanupShareTempFiles(List<FileSystemEntity> tempEntities) async {
|
||||
await Future.wait(
|
||||
tempFiles.map((file) async {
|
||||
tempEntities.map((entity) async {
|
||||
try {
|
||||
await file.delete();
|
||||
if (entity is Directory) {
|
||||
await entity.delete(recursive: true);
|
||||
} else {
|
||||
await entity.delete();
|
||||
}
|
||||
} catch (e) {
|
||||
_log.warning("Failed to delete temporary file: ${file.path}", e);
|
||||
_log.warning("Failed to delete temporary file: ${entity.path}", e);
|
||||
}
|
||||
}),
|
||||
);
|
||||
}
|
||||
|
||||
String _sanitizeFilename(String filename) {
|
||||
return filename.replaceAll(RegExp(r'[\\/]'), '_');
|
||||
static final RegExp _pathSeparators = RegExp(r'[\\/]');
|
||||
|
||||
static String _sanitizeFilename(String filename) {
|
||||
return filename.replaceAll(_pathSeparators, '_');
|
||||
}
|
||||
|
||||
String _getPreviewFilename(BaseAsset asset) {
|
||||
@visibleForTesting
|
||||
static String getOriginalShareFilename(BaseAsset asset) {
|
||||
final fallbackName = asset.remoteId ?? asset.localId ?? 'asset';
|
||||
final hasUsableName = asset.name.replaceAll(_pathSeparators, '').isNotEmpty;
|
||||
return hasUsableName ? _sanitizeFilename(asset.name) : fallbackName;
|
||||
}
|
||||
|
||||
static String _getPreviewFilename(BaseAsset asset) {
|
||||
final sanitizedFilename = _sanitizeFilename(asset.name);
|
||||
final baseName = p.basenameWithoutExtension(sanitizedFilename);
|
||||
final fallbackName = asset.remoteId ?? asset.localId ?? 'asset';
|
||||
return '${baseName.isEmpty ? fallbackName : baseName}-preview.jpg';
|
||||
}
|
||||
|
||||
static String _shareDisplayName(BaseAsset asset, ShareAssetType fileType) =>
|
||||
switch (asset.isVideo ? ShareAssetType.original : fileType) {
|
||||
ShareAssetType.original => getOriginalShareFilename(asset),
|
||||
ShareAssetType.preview => _getPreviewFilename(asset),
|
||||
};
|
||||
|
||||
@visibleForTesting
|
||||
static Map<String, int> countShareDisplayNames(List<BaseAsset> assets, ShareAssetType fileType) {
|
||||
final counts = <String, int>{};
|
||||
for (final asset in assets) {
|
||||
final name = _shareDisplayName(asset, fileType);
|
||||
counts[name] = (counts[name] ?? 0) + 1;
|
||||
}
|
||||
return counts;
|
||||
}
|
||||
|
||||
bool _isCancelled(Completer<void>? cancelCompleter) => cancelCompleter?.isCompleted ?? false;
|
||||
|
||||
Future<_ShareFile?> _getLocalOriginalShareFile(BaseAsset asset, String localId) async {
|
||||
@@ -135,24 +165,44 @@ class AssetMediaRepository {
|
||||
return null;
|
||||
}
|
||||
|
||||
return (file: file, cleanup: CurrentPlatform.isIOS, displayName: _sanitizeFilename(asset.name));
|
||||
return (file: file, tempEntity: CurrentPlatform.isIOS ? file : null, displayName: getOriginalShareFilename(asset));
|
||||
}
|
||||
|
||||
@visibleForTesting
|
||||
static DownloadTask buildShareDownloadTask({
|
||||
required String taskId,
|
||||
required String url,
|
||||
required Map<String, String> headers,
|
||||
required String displayName,
|
||||
required bool isDuplicate,
|
||||
}) {
|
||||
return DownloadTask(
|
||||
taskId: taskId,
|
||||
url: url,
|
||||
headers: headers,
|
||||
// receiving apps show the shared file's own name, so duplicates keep the task id prefix to stay unique
|
||||
filename: isDuplicate ? '$taskId-$displayName' : displayName,
|
||||
directory: taskId,
|
||||
baseDirectory: BaseDirectory.temporary,
|
||||
group: kShareDownloadGroup,
|
||||
updates: Updates.statusAndProgress,
|
||||
);
|
||||
}
|
||||
|
||||
Future<_ShareFile?> _downloadRemoteShareFile({
|
||||
required String taskId,
|
||||
required String url,
|
||||
required String displayName,
|
||||
required bool isDuplicate,
|
||||
Completer<void>? cancelCompleter,
|
||||
required void Function(double progress) onProgress,
|
||||
}) async {
|
||||
final task = DownloadTask(
|
||||
final task = buildShareDownloadTask(
|
||||
taskId: taskId,
|
||||
url: url,
|
||||
headers: ApiService.getRequestHeaders(),
|
||||
filename: '$taskId-$displayName',
|
||||
baseDirectory: BaseDirectory.temporary,
|
||||
group: kShareDownloadGroup,
|
||||
updates: Updates.statusAndProgress,
|
||||
displayName: displayName,
|
||||
isDuplicate: isDuplicate,
|
||||
);
|
||||
final downloader = FileDownloader();
|
||||
final statusUpdate = await downloader.download(
|
||||
@@ -171,7 +221,8 @@ class AssetMediaRepository {
|
||||
}
|
||||
|
||||
if (statusUpdate.status == TaskStatus.complete) {
|
||||
return (file: File(await task.filePath()), cleanup: true, displayName: displayName);
|
||||
final file = File(await task.filePath());
|
||||
return (file: file, tempEntity: file.parent, displayName: displayName);
|
||||
}
|
||||
|
||||
_log.severe("Download for $displayName failed with status ${statusUpdate.status}", statusUpdate.exception);
|
||||
@@ -181,13 +232,15 @@ class AssetMediaRepository {
|
||||
Future<_ShareFile?> _getRemoteOriginalShareFile(
|
||||
BaseAsset asset,
|
||||
String remoteId, {
|
||||
required bool isDuplicate,
|
||||
Completer<void>? cancelCompleter,
|
||||
required void Function(double progress) onProgress,
|
||||
}) {
|
||||
return _downloadRemoteShareFile(
|
||||
taskId: 'share-original-$remoteId-${DateTime.now().microsecondsSinceEpoch}',
|
||||
url: getOriginalUrlForRemoteId(remoteId, edited: asset.isEdited),
|
||||
displayName: _sanitizeFilename(asset.name),
|
||||
displayName: getOriginalShareFilename(asset),
|
||||
isDuplicate: isDuplicate,
|
||||
cancelCompleter: cancelCompleter,
|
||||
onProgress: onProgress,
|
||||
);
|
||||
@@ -196,6 +249,7 @@ class AssetMediaRepository {
|
||||
Future<_ShareFile?> _getRemotePreviewShareFile(
|
||||
BaseAsset asset,
|
||||
String remoteId, {
|
||||
required bool isDuplicate,
|
||||
Completer<void>? cancelCompleter,
|
||||
required void Function(double progress) onProgress,
|
||||
}) {
|
||||
@@ -203,6 +257,7 @@ class AssetMediaRepository {
|
||||
taskId: 'share-preview-$remoteId-${DateTime.now().microsecondsSinceEpoch}',
|
||||
url: getThumbnailUrlForRemoteId(remoteId, type: AssetMediaSize.preview, edited: asset.isEdited),
|
||||
displayName: _getPreviewFilename(asset),
|
||||
isDuplicate: isDuplicate,
|
||||
cancelCompleter: cancelCompleter,
|
||||
onProgress: onProgress,
|
||||
);
|
||||
@@ -210,6 +265,7 @@ class AssetMediaRepository {
|
||||
|
||||
Future<_ShareFile?> _getOriginalShareFile(
|
||||
BaseAsset asset, {
|
||||
required bool isDuplicate,
|
||||
Completer<void>? cancelCompleter,
|
||||
required void Function(double progress) onProgress,
|
||||
}) {
|
||||
@@ -224,11 +280,18 @@ class AssetMediaRepository {
|
||||
return Future.value(null);
|
||||
}
|
||||
|
||||
return _getRemoteOriginalShareFile(asset, remoteId, cancelCompleter: cancelCompleter, onProgress: onProgress);
|
||||
return _getRemoteOriginalShareFile(
|
||||
asset,
|
||||
remoteId,
|
||||
isDuplicate: isDuplicate,
|
||||
cancelCompleter: cancelCompleter,
|
||||
onProgress: onProgress,
|
||||
);
|
||||
}
|
||||
|
||||
Future<_ShareFile?> _getPreviewShareFile(
|
||||
BaseAsset asset, {
|
||||
required bool isDuplicate,
|
||||
Completer<void>? cancelCompleter,
|
||||
required void Function(double progress) onProgress,
|
||||
}) async {
|
||||
@@ -237,6 +300,7 @@ class AssetMediaRepository {
|
||||
final remotePreview = await _getRemotePreviewShareFile(
|
||||
asset,
|
||||
remoteId,
|
||||
isDuplicate: isDuplicate,
|
||||
cancelCompleter: cancelCompleter,
|
||||
onProgress: onProgress,
|
||||
);
|
||||
@@ -262,7 +326,7 @@ class AssetMediaRepository {
|
||||
void Function(double progress)? onAssetDownloadProgress,
|
||||
}) async {
|
||||
final downloadedXFiles = <XFile>[];
|
||||
final tempFiles = <File>[];
|
||||
final tempFiles = <FileSystemEntity>[];
|
||||
final totalAssets = assets.length;
|
||||
var processedAssets = 0;
|
||||
|
||||
@@ -279,29 +343,34 @@ class AssetMediaRepository {
|
||||
|
||||
updateProgress();
|
||||
|
||||
final displayNameCounts = countShareDisplayNames(assets, fileType);
|
||||
|
||||
for (final asset in assets) {
|
||||
if (_isCancelled(cancelCompleter)) {
|
||||
await _cleanupTempFiles(tempFiles);
|
||||
await cleanupShareTempFiles(tempFiles);
|
||||
return 0;
|
||||
}
|
||||
|
||||
final effectiveFileType = asset.isVideo ? ShareAssetType.original : fileType;
|
||||
final isDuplicate = displayNameCounts[_shareDisplayName(asset, fileType)]! > 1;
|
||||
|
||||
final shareFile = switch (effectiveFileType) {
|
||||
ShareAssetType.original => await _getOriginalShareFile(
|
||||
asset,
|
||||
isDuplicate: isDuplicate,
|
||||
cancelCompleter: cancelCompleter,
|
||||
onProgress: updateProgress,
|
||||
),
|
||||
ShareAssetType.preview => await _getPreviewShareFile(
|
||||
asset,
|
||||
isDuplicate: isDuplicate,
|
||||
cancelCompleter: cancelCompleter,
|
||||
onProgress: updateProgress,
|
||||
),
|
||||
};
|
||||
|
||||
if (_isCancelled(cancelCompleter)) {
|
||||
await _cleanupTempFiles(tempFiles);
|
||||
await cleanupShareTempFiles(tempFiles);
|
||||
return 0;
|
||||
}
|
||||
|
||||
@@ -312,8 +381,9 @@ class AssetMediaRepository {
|
||||
}
|
||||
|
||||
downloadedXFiles.add(XFile(shareFile.file.path, name: shareFile.displayName));
|
||||
if (shareFile.cleanup) {
|
||||
tempFiles.add(shareFile.file);
|
||||
final tempEntity = shareFile.tempEntity;
|
||||
if (tempEntity != null) {
|
||||
tempFiles.add(tempEntity);
|
||||
}
|
||||
processedAssets++;
|
||||
updateProgress();
|
||||
@@ -325,7 +395,7 @@ class AssetMediaRepository {
|
||||
}
|
||||
|
||||
if (_isCancelled(cancelCompleter)) {
|
||||
await _cleanupTempFiles(tempFiles);
|
||||
await cleanupShareTempFiles(tempFiles);
|
||||
return 0;
|
||||
}
|
||||
|
||||
@@ -337,7 +407,7 @@ class AssetMediaRepository {
|
||||
downloadedXFiles,
|
||||
sharePositionOrigin: Rect.fromPoints(Offset.zero, Offset(size.width / 3, size.height)),
|
||||
).then((result) async {
|
||||
await _cleanupTempFiles(tempFiles);
|
||||
await cleanupShareTempFiles(tempFiles);
|
||||
}),
|
||||
);
|
||||
|
||||
|
||||
@@ -0,0 +1,141 @@
|
||||
import 'dart:io';
|
||||
|
||||
import 'package:background_downloader/background_downloader.dart';
|
||||
import 'package:flutter/services.dart';
|
||||
import 'package:flutter_test/flutter_test.dart';
|
||||
import 'package:immich_mobile/constants/enums.dart';
|
||||
import 'package:immich_mobile/repositories/asset_media.repository.dart';
|
||||
import 'package:path/path.dart' as p;
|
||||
|
||||
import '../test_utils.dart';
|
||||
|
||||
void main() {
|
||||
TestWidgetsFlutterBinding.ensureInitialized();
|
||||
|
||||
setUpAll(() {
|
||||
TestDefaultBinaryMessengerBinding.instance.defaultBinaryMessenger.setMockMethodCallHandler(
|
||||
const MethodChannel('plugins.flutter.io/path_provider'),
|
||||
(methodCall) async => '/tmp/immich-share-test',
|
||||
);
|
||||
});
|
||||
|
||||
DownloadTask buildTask(String taskId, String displayName, {bool isDuplicate = false}) =>
|
||||
AssetMediaRepository.buildShareDownloadTask(
|
||||
taskId: taskId,
|
||||
url: 'https://example.com/api/assets/some-id/original',
|
||||
headers: const {},
|
||||
displayName: displayName,
|
||||
isDuplicate: isDuplicate,
|
||||
);
|
||||
|
||||
group('buildShareDownloadTask', () {
|
||||
test('saves a unique original under the asset name, without the task id prefix (#29468)', () async {
|
||||
final task = buildTask('share-original-some-remote-id-123456', 'IMG-0001.jpg');
|
||||
|
||||
expect(task.filename, 'IMG-0001.jpg');
|
||||
expect(p.basename(await task.filePath()), 'IMG-0001.jpg');
|
||||
});
|
||||
|
||||
test('preview shares keep their -preview name', () async {
|
||||
final task = buildTask('share-preview-some-remote-id-123456', 'IMG-0001-preview.jpg');
|
||||
|
||||
expect(p.basename(await task.filePath()), 'IMG-0001-preview.jpg');
|
||||
});
|
||||
|
||||
test('names with spaces and unusual characters pass through untouched', () {
|
||||
const name = 'photo 1 (final) 名字.jpg';
|
||||
final task = buildTask('share-original-some-remote-id-123456', name);
|
||||
|
||||
expect(task.filename, name);
|
||||
});
|
||||
|
||||
test('two same-named assets download to two distinct prefixed names', () async {
|
||||
final assets = [
|
||||
TestUtils.createRemoteAsset(id: 'remote-1').copyWith(name: 'IMG-0001.jpg'),
|
||||
TestUtils.createRemoteAsset(id: 'remote-2').copyWith(name: 'IMG-0001.jpg'),
|
||||
];
|
||||
final counts = AssetMediaRepository.countShareDisplayNames(assets, ShareAssetType.original);
|
||||
final isDuplicate = counts['IMG-0001.jpg']! > 1;
|
||||
|
||||
final first = buildTask('share-original-remote-1-111', 'IMG-0001.jpg', isDuplicate: isDuplicate);
|
||||
final second = buildTask('share-original-remote-2-222', 'IMG-0001.jpg', isDuplicate: isDuplicate);
|
||||
|
||||
expect(p.basename(await first.filePath()), 'share-original-remote-1-111-IMG-0001.jpg');
|
||||
expect(p.basename(await second.filePath()), 'share-original-remote-2-222-IMG-0001.jpg');
|
||||
});
|
||||
|
||||
test('sharing the same asset twice downloads to distinct paths', () async {
|
||||
final asset = TestUtils.createRemoteAsset(id: 'remote-1').copyWith(name: 'IMG-0001.jpg');
|
||||
final counts = AssetMediaRepository.countShareDisplayNames([asset, asset], ShareAssetType.original);
|
||||
final isDuplicate = counts['IMG-0001.jpg']! > 1;
|
||||
|
||||
final first = buildTask('share-original-remote-1-111111', 'IMG-0001.jpg', isDuplicate: isDuplicate);
|
||||
final second = buildTask('share-original-remote-1-222222', 'IMG-0001.jpg', isDuplicate: isDuplicate);
|
||||
|
||||
expect(await first.filePath(), isNot(await second.filePath()));
|
||||
});
|
||||
|
||||
test('a failed duplicate leaves the survivor with the prefixed name', () {
|
||||
final assets = [
|
||||
TestUtils.createRemoteAsset(id: 'remote-1').copyWith(name: 'IMG-0001.jpg'),
|
||||
TestUtils.createRemoteAsset(id: 'remote-2').copyWith(name: 'IMG-0001.jpg'),
|
||||
];
|
||||
final counts = AssetMediaRepository.countShareDisplayNames(assets, ShareAssetType.original);
|
||||
|
||||
final survivor = buildTask(
|
||||
'share-original-remote-1-111',
|
||||
'IMG-0001.jpg',
|
||||
isDuplicate: counts['IMG-0001.jpg']! > 1,
|
||||
);
|
||||
|
||||
expect(survivor.filename, 'share-original-remote-1-111-IMG-0001.jpg');
|
||||
});
|
||||
});
|
||||
|
||||
group('getOriginalShareFilename', () {
|
||||
test('falls back to the remote id when the name is empty', () {
|
||||
final asset = TestUtils.createRemoteAsset(id: 'remote-1').copyWith(name: '');
|
||||
|
||||
expect(AssetMediaRepository.getOriginalShareFilename(asset), 'remote-1');
|
||||
});
|
||||
|
||||
test('falls back when the name is only path separators', () {
|
||||
final asset = TestUtils.createRemoteAsset(id: 'remote-1').copyWith(name: r'\/');
|
||||
|
||||
expect(AssetMediaRepository.getOriginalShareFilename(asset), 'remote-1');
|
||||
});
|
||||
|
||||
test('falls back to the local id when there is no remote id', () {
|
||||
final asset = TestUtils.createLocalAsset(id: 'local-1').copyWith(name: '');
|
||||
|
||||
expect(AssetMediaRepository.getOriginalShareFilename(asset), 'local-1');
|
||||
});
|
||||
|
||||
test('sanitizes separators in a real name instead of falling back', () {
|
||||
final asset = TestUtils.createRemoteAsset(id: 'remote-1').copyWith(name: 'holiday/IMG-0001.jpg');
|
||||
|
||||
expect(AssetMediaRepository.getOriginalShareFilename(asset), 'holiday_IMG-0001.jpg');
|
||||
});
|
||||
});
|
||||
|
||||
group('cleanupShareTempFiles', () {
|
||||
test('removes only the owned task directories and temp files', () async {
|
||||
final tempRoot = Directory.systemTemp.createTempSync('immich-share-cleanup');
|
||||
addTearDown(() => tempRoot.deleteSync(recursive: true));
|
||||
final taskDir = Directory(p.join(tempRoot.path, 'share-original-remote-1-111'))..createSync();
|
||||
final downloaded = File(p.join(taskDir.path, 'IMG-0001.jpg'))..createSync();
|
||||
final iosLocalTemp = File(p.join(tempRoot.path, 'local-original.jpg'))..createSync();
|
||||
final foreignDir = Directory(p.join(tempRoot.path, 'unrelated'))..createSync();
|
||||
final foreignFile = File(p.join(foreignDir.path, 'keep.jpg'))..createSync();
|
||||
|
||||
await AssetMediaRepository.cleanupShareTempFiles([taskDir, iosLocalTemp]);
|
||||
|
||||
expect(taskDir.existsSync(), isFalse);
|
||||
expect(downloaded.existsSync(), isFalse);
|
||||
expect(iosLocalTemp.existsSync(), isFalse);
|
||||
expect(foreignDir.existsSync(), isTrue);
|
||||
expect(foreignFile.existsSync(), isTrue);
|
||||
expect(tempRoot.existsSync(), isTrue);
|
||||
});
|
||||
});
|
||||
}
|
||||
Reference in New Issue
Block a user