mirror of
https://github.com/Novaloop-AG/element-x-android.git
synced 2026-11-02 12:07:28 +00:00
Keep saving files to Downloads once 32 files share the same name
MediaStore de-duplicates the display name of a new file itself, but its uniquifier gives up after 32 collisions and throws IllegalStateException. Saving then failed with "Sorry, an error occurred" and wrote nothing, and because image.png is the name most screenshots arrive with, a user who saves media regularly could reach a state where that name never saved again without deleting files from Downloads by hand. A new saveWithUniqueFileName helper in androidutils retries the insert once with a suffixed name when the platform reports it cannot build a unique one; a second failure propagates so genuine errors still surface. withFileNameSuffix builds that name by inserting the suffix before the extension, and handles names without one. Both call sites are fixed: AndroidLocalMediaActions and DefaultFileSave carried byte-identical logic and therefore the identical defect. While in AndroidLocalMediaActions, a null output uri from ContentResolver.insert now throws instead of reporting success without writing anything. The pre-Android-Q path is untouched: it overwrites rather than uniquifying, so it never reaches this limit. Relates to https://github.com/element-hq/element-x-android/issues/6371
This commit is contained in:
+35
@@ -42,6 +42,41 @@ fun File.safeRenameTo(dest: File) {
|
||||
)
|
||||
}
|
||||
|
||||
/**
|
||||
* Returns this file name with [suffix] inserted before its extension, i.e. `"image.png"` with the
|
||||
* suffix `"_1"` gives `"image_1.png"`. A name without extension simply gets the suffix appended.
|
||||
*/
|
||||
fun String.withFileNameSuffix(suffix: String): String {
|
||||
val extension = substringAfterLast('.', "")
|
||||
return if (extension.isEmpty()) {
|
||||
"$this$suffix"
|
||||
} else {
|
||||
"${substringBeforeLast('.')}$suffix.$extension"
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Runs [saveAs] with [fileName], and runs it once more with a suffixed name if the platform failed to
|
||||
* build a unique name for it.
|
||||
*
|
||||
* `MediaStore` de-duplicates the display name of a new file itself, but gives up after 32 conflicts
|
||||
* and throws `IllegalStateException("Failed to build unique file")`, which makes saving a file with a
|
||||
* common name such as `image.png` fail for good.
|
||||
* See https://github.com/element-hq/element-x-android/issues/6371
|
||||
*/
|
||||
fun <T> saveWithUniqueFileName(
|
||||
fileName: String,
|
||||
uniqueSuffix: () -> String = { "_${System.currentTimeMillis()}" },
|
||||
saveAs: (String) -> T,
|
||||
): T {
|
||||
return try {
|
||||
saveAs(fileName)
|
||||
} catch (nameConflict: IllegalStateException) {
|
||||
Timber.w(nameConflict, "Unable to build a unique file name, retrying with a suffixed one")
|
||||
saveAs(fileName.withFileNameSuffix(uniqueSuffix()))
|
||||
}
|
||||
}
|
||||
|
||||
fun Context.createTmpFile(baseDir: File = cacheDir, extension: String? = null): File {
|
||||
val suffix = extension?.let { ".$extension" }
|
||||
return File.createTempFile(UUID.randomUUID().toString(), suffix, baseDir).apply { mkdirs() }
|
||||
|
||||
+89
@@ -0,0 +1,89 @@
|
||||
/*
|
||||
* Copyright (c) 2026 Element Creations Ltd.
|
||||
*
|
||||
* SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Element-Commercial.
|
||||
* Please see LICENSE files in the repository root for full details.
|
||||
*/
|
||||
|
||||
package io.element.android.libraries.androidutils.file
|
||||
|
||||
import com.google.common.truth.Truth.assertThat
|
||||
import org.junit.Test
|
||||
|
||||
class FileTest {
|
||||
@Test
|
||||
fun `withFileNameSuffix - inserts the suffix before the extension`() {
|
||||
assertThat("image.png".withFileNameSuffix("_1")).isEqualTo("image_1.png")
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `withFileNameSuffix - appends the suffix when there is no extension`() {
|
||||
assertThat("README".withFileNameSuffix("_1")).isEqualTo("README_1")
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `withFileNameSuffix - only considers the last extension`() {
|
||||
assertThat("archive.tar.gz".withFileNameSuffix("_1")).isEqualTo("archive.tar_1.gz")
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `withFileNameSuffix - handles a name starting with a dot`() {
|
||||
assertThat(".hidden".withFileNameSuffix("_1")).isEqualTo("_1.hidden")
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `saveWithUniqueFileName - uses the original name when it works`() {
|
||||
val usedNames = mutableListOf<String>()
|
||||
|
||||
val result = saveWithUniqueFileName(
|
||||
fileName = "image.png",
|
||||
uniqueSuffix = { "_1" },
|
||||
) { name ->
|
||||
usedNames.add(name)
|
||||
"saved as $name"
|
||||
}
|
||||
|
||||
assertThat(result).isEqualTo("saved as image.png")
|
||||
assertThat(usedNames).containsExactly("image.png")
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `saveWithUniqueFileName - retries with a suffixed name when the name cannot be made unique`() {
|
||||
val usedNames = mutableListOf<String>()
|
||||
|
||||
val result = saveWithUniqueFileName(
|
||||
fileName = "image.png",
|
||||
uniqueSuffix = { "_1" },
|
||||
) { name ->
|
||||
usedNames.add(name)
|
||||
if (name == "image.png") {
|
||||
error("Failed to build unique file")
|
||||
}
|
||||
"saved as $name"
|
||||
}
|
||||
|
||||
assertThat(result).isEqualTo("saved as image_1.png")
|
||||
assertThat(usedNames).containsExactly("image.png", "image_1.png").inOrder()
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `saveWithUniqueFileName - does not retry more than once`() {
|
||||
val usedNames = mutableListOf<String>()
|
||||
|
||||
val error = try {
|
||||
saveWithUniqueFileName(
|
||||
fileName = "image.png",
|
||||
uniqueSuffix = { "_1" },
|
||||
) { name ->
|
||||
usedNames.add(name)
|
||||
error("Failed to build unique file")
|
||||
}
|
||||
null
|
||||
} catch (nameConflict: IllegalStateException) {
|
||||
nameConflict
|
||||
}
|
||||
|
||||
assertThat(error).isInstanceOf(IllegalStateException::class.java)
|
||||
assertThat(usedNames).containsExactly("image.png", "image_1.png").inOrder()
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user