From c6ec5afba1d1ed18d5f8b4924c5f31b8e53b0c13 Mon Sep 17 00:00:00 2001 From: Thibault Deckers Date: Tue, 29 Oct 2024 01:06:46 +0100 Subject: [PATCH] #1249 fixed copying content URI items --- CHANGELOG.md | 1 + .../aves/model/provider/ImageProvider.kt | 26 +++- .../model/provider/MediaStoreImageProvider.kt | 119 +++++++++--------- 3 files changed, 80 insertions(+), 66 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e444c6956..977f18f52 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,7 @@ All notable changes to this project will be documented in this file. ### Fixed - crash when loading large collection +- Viewer: copying content URI item ## [v1.11.16] - 2024-10-10 diff --git a/android/app/src/main/kotlin/deckers/thibault/aves/model/provider/ImageProvider.kt b/android/app/src/main/kotlin/deckers/thibault/aves/model/provider/ImageProvider.kt index 1d6d6a60a..9734cd497 100644 --- a/android/app/src/main/kotlin/deckers/thibault/aves/model/provider/ImageProvider.kt +++ b/android/app/src/main/kotlin/deckers/thibault/aves/model/provider/ImageProvider.kt @@ -137,8 +137,7 @@ abstract class ImageProvider { "success" to false, ) - // prevent naming with a `.` prefix as it would hide the file and remove it from the Media Store - if (sourcePath != null && !desiredName.startsWith('.')) { + if (sourcePath != null) { try { var newFields: FieldMap = skippedFieldMap if (!isCancelledOp()) { @@ -570,6 +569,20 @@ abstract class ImageProvider { } } + fun createTimeStampFileName() = Date().time.toString() + + private fun sanitizeDesiredFileName(desiredName: String): String { + var name = desiredName + // prevent creating hidden files + while (name.isNotEmpty() && name.startsWith(".")) { + name = name.substring(1) + } + if (name.isEmpty()) { + name = createTimeStampFileName() + } + return name + } + // returns available name to use, or `null` to skip it suspend fun resolveTargetFileNameWithoutExtension( contextWrapper: ContextWrapper, @@ -578,18 +591,19 @@ abstract class ImageProvider { mimeType: String, conflictStrategy: NameConflictStrategy, ): NameConflictResolution { - var resolvedName: String? = desiredNameWithoutExtension + val sanitizedNameWithoutExtension = sanitizeDesiredFileName(desiredNameWithoutExtension) + var resolvedName: String? = sanitizedNameWithoutExtension var replacementFile: File? = null val extension = extensionFor(mimeType) - val targetFile = File(dir, "$desiredNameWithoutExtension$extension") + val targetFile = File(dir, "$sanitizedNameWithoutExtension$extension") when (conflictStrategy) { NameConflictStrategy.RENAME -> { - var nameWithoutExtension = desiredNameWithoutExtension + var nameWithoutExtension = sanitizedNameWithoutExtension var i = 0 while (File(dir, "$nameWithoutExtension$extension").exists()) { i++ - nameWithoutExtension = "$desiredNameWithoutExtension ($i)" + nameWithoutExtension = "$sanitizedNameWithoutExtension ($i)" } resolvedName = nameWithoutExtension } diff --git a/android/app/src/main/kotlin/deckers/thibault/aves/model/provider/MediaStoreImageProvider.kt b/android/app/src/main/kotlin/deckers/thibault/aves/model/provider/MediaStoreImageProvider.kt index f05e6991f..fc921de8c 100644 --- a/android/app/src/main/kotlin/deckers/thibault/aves/model/provider/MediaStoreImageProvider.kt +++ b/android/app/src/main/kotlin/deckers/thibault/aves/model/provider/MediaStoreImageProvider.kt @@ -40,6 +40,7 @@ import java.io.FileOutputStream import java.io.IOException import java.io.OutputStream import java.io.SyncFailedException +import java.util.Date import java.util.Locale import java.util.concurrent.CompletableFuture import kotlin.coroutines.Continuation @@ -478,64 +479,62 @@ class MediaStoreImageProvider : ImageProvider() { "success" to false, ) - if (sourcePath != null) { - // on API 30 we cannot get access granted directly to a volume root from its document tree, - // but it is still less constraining to use tree document files than to rely on the Media Store - // - // Relying on `DocumentFile`, we can create an item via `DocumentFile.createFile()`, but: - // - we need to scan the file to get the Media Store content URI - // - the underlying document provider controls the new file name - // - // Relying on the Media Store, we can create an item via `ContentResolver.insert()` - // with a path, and retrieve its content URI, but: - // - the Media Store isolates content by storage volume (e.g. `MediaStore.Images.Media.getContentUri(volumeName)`) - // - the volume name should be lower case, not exactly as the `StorageVolume` UUID - // cf new method in API 30 `StorageVolume.getMediaStoreVolumeName()` - // - inserting on a removable volume works on API 29, but not on API 25 nor 26 (on which API/devices does it work?) - // - there is no documentation regarding support for usage with removable storage - // - the Media Store only allows inserting in specific primary directories ("DCIM", "Pictures") when using scoped storage - try { - val appDir = when { - toBin -> StorageUtils.trashDirFor(activity, sourcePath) - toVault -> File(targetDir) - else -> null - } - if (appDir != null) { - effectiveTargetDir = ensureTrailingSeparator(appDir.path) - targetDirDocFile = DocumentFileCompat.fromFile(appDir) - - if (toVault) { - appDir.mkdirs() - } - } - - if (effectiveTargetDir != null) { - val newFields = if (isCancelledOp()) skippedFieldMap else { - val sourceFile = File(sourcePath) - if (!sourceFile.exists() && toBin) { - delete(activity, sourceUri, sourcePath, mimeType = mimeType) - deletedFieldMap - } else { - moveSingle( - activity = activity, - sourceFile = sourceFile, - sourceUri = sourceUri, - targetDir = effectiveTargetDir, - targetDirDocFile = targetDirDocFile, - desiredName = desiredName ?: sourceFile.name, - nameConflictStrategy = nameConflictStrategy, - mimeType = mimeType, - copy = copy, - toBin = toBin, - ) - } - } - result["newFields"] = newFields - result["success"] = true - } - } catch (e: Exception) { - Log.w(LOG_TAG, "failed to move to targetDir=$targetDir entry with sourcePath=$sourcePath", e) + // on API 30 we cannot get access granted directly to a volume root from its document tree, + // but it is still less constraining to use tree document files than to rely on the Media Store + // + // Relying on `DocumentFile`, we can create an item via `DocumentFile.createFile()`, but: + // - we need to scan the file to get the Media Store content URI + // - the underlying document provider controls the new file name + // + // Relying on the Media Store, we can create an item via `ContentResolver.insert()` + // with a path, and retrieve its content URI, but: + // - the Media Store isolates content by storage volume (e.g. `MediaStore.Images.Media.getContentUri(volumeName)`) + // - the volume name should be lower case, not exactly as the `StorageVolume` UUID + // cf new method in API 30 `StorageVolume.getMediaStoreVolumeName()` + // - inserting on a removable volume works on API 29, but not on API 25 nor 26 (on which API/devices does it work?) + // - there is no documentation regarding support for usage with removable storage + // - the Media Store only allows inserting in specific primary directories ("DCIM", "Pictures") when using scoped storage + try { + val appDir = when { + toBin -> StorageUtils.trashDirFor(activity, sourcePath ?: StorageUtils.getPrimaryVolumePath(activity)) + toVault -> File(targetDir) + else -> null } + if (appDir != null) { + effectiveTargetDir = ensureTrailingSeparator(appDir.path) + targetDirDocFile = DocumentFileCompat.fromFile(appDir) + + if (toVault) { + appDir.mkdirs() + } + } + + if (effectiveTargetDir != null) { + val newFields = if (isCancelledOp()) skippedFieldMap else { + val sourceFile = if (sourcePath != null) File(sourcePath) else null + if (sourceFile != null && !sourceFile.exists() && toBin) { + delete(activity, sourceUri, sourcePath, mimeType = mimeType) + deletedFieldMap + } else { + moveSingle( + activity = activity, + sourceFile = sourceFile, + sourceUri = sourceUri, + targetDir = effectiveTargetDir, + targetDirDocFile = targetDirDocFile, + desiredName = desiredName ?: sourceFile?.name ?: sourceUri.lastPathSegment ?: createTimeStampFileName(), + nameConflictStrategy = nameConflictStrategy, + mimeType = mimeType, + copy = copy, + toBin = toBin, + ) + } + } + result["newFields"] = newFields + result["success"] = true + } + } catch (e: Exception) { + Log.w(LOG_TAG, "failed to move to targetDir=$targetDir entry with sourcePath=$sourcePath", e) } callback.onSuccess(result) } @@ -544,7 +543,7 @@ class MediaStoreImageProvider : ImageProvider() { private suspend fun moveSingle( activity: Activity, - sourceFile: File, + sourceFile: File?, sourceUri: Uri, targetDir: String, targetDirDocFile: DocumentFileCompat?, @@ -554,8 +553,8 @@ class MediaStoreImageProvider : ImageProvider() { copy: Boolean, toBin: Boolean, ): FieldMap { - val sourcePath = sourceFile.path - val sourceDir = sourceFile.parent?.let { ensureTrailingSeparator(it) } + val sourcePath = sourceFile?.path + val sourceDir = sourceFile?.parent?.let { ensureTrailingSeparator(it) } if (sourceDir == targetDir && !(copy && nameConflictStrategy == NameConflictStrategy.RENAME)) { // nothing to do unless it's a renamed copy return skippedFieldMap