Merge pull request #7344 from hayaksi1/fix/6371-save-file-name-conflicts

Keep saving files to Downloads once 32 files share the same name

features/viewfolder/impl/src/main/kotlin/io/element/android/features/viewfolder/impl/file/FileSave.kt

@@ -16,6 +16,7 @@ import android.provider.MediaStore
 import androidx.annotation.RequiresApi
 import dev.zacsweers.metro.AppScope
 import dev.zacsweers.metro.ContributesBinding
+import io.element.android.libraries.androidutils.file.saveWithUniqueFileName
 import io.element.android.libraries.androidutils.system.toast
 import io.element.android.libraries.core.coroutine.CoroutineDispatchers
 import io.element.android.libraries.core.extensions.runCatchingExceptions
@@ -25,6 +26,7 @@ import kotlinx.coroutines.withContext
 import timber.log.Timber
 import java.io.File
 import java.io.FileOutputStream
+import java.io.IOException
 
 interface FileSave {
     suspend fun save(
@@ -61,18 +63,18 @@ class DefaultFileSave(
     @RequiresApi(Build.VERSION_CODES.Q)
     private fun saveOnDiskUsingMediaStore(path: String) {
         val file = File(path)
-        val contentValues = ContentValues().apply {
-            put(MediaStore.MediaColumns.DISPLAY_NAME, file.name)
-            put(MediaStore.MediaColumns.MIME_TYPE, MimeTypes.OctetStream)
-            put(MediaStore.MediaColumns.RELATIVE_PATH, Environment.DIRECTORY_DOWNLOADS)
-        }
         val resolver = context.contentResolver
-        val outputUri = resolver.insert(MediaStore.Downloads.EXTERNAL_CONTENT_URI, contentValues)
-        if (outputUri != null) {
-            file.inputStream().use { input ->
-                resolver.openOutputStream(outputUri).use { output ->
-                    input.copyTo(output!!, DEFAULT_BUFFER_SIZE)
-                }
+        val outputUri = saveWithUniqueFileName(file.name) { displayName ->
+            val contentValues = ContentValues().apply {
+                put(MediaStore.MediaColumns.DISPLAY_NAME, displayName)
+                put(MediaStore.MediaColumns.MIME_TYPE, MimeTypes.OctetStream)
+                put(MediaStore.MediaColumns.RELATIVE_PATH, Environment.DIRECTORY_DOWNLOADS)
+            }
+            resolver.insert(MediaStore.Downloads.EXTERNAL_CONTENT_URI, contentValues)
+        } ?: throw IOException("Unable to create the destination file")
+        file.inputStream().use { input ->
+            resolver.openOutputStream(outputUri).use { output ->
+                input.copyTo(output!!, DEFAULT_BUFFER_SIZE)
             }
         }
     }

features/viewfolder/impl/src/test/kotlin/io/element/android/features/viewfolder/impl/file/DefaultFileSaveTest.kt

@@ -0,0 +1,60 @@
+/*
+ * 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.features.viewfolder.impl.file
+
+import com.google.common.truth.Truth.assertThat
+import io.element.android.tests.testutils.fake.registerFakeMediaStoreContentProvider
+import io.element.android.tests.testutils.robolectric.RobolectricTest
+import io.element.android.tests.testutils.testCoroutineDispatchers
+import kotlinx.coroutines.test.TestScope
+import kotlinx.coroutines.test.runTest
+import org.junit.Test
+import org.robolectric.RuntimeEnvironment
+import java.io.File
+
+class DefaultFileSaveTest : RobolectricTest() {
+    @Test
+    fun `save inserts the file in the Downloads collection under its own name`() = runTest {
+        val mediaStore = registerFakeMediaStoreContentProvider()
+        createFileSave().save(aFileToSave().path)
+        assertThat(mediaStore.insertedDisplayNames).containsExactly("a-file.txt")
+        assertThat(mediaStore.savedContent("a-file.txt")).isEqualTo("Some content")
+    }
+
+    @Test
+    fun `save retries with a suffixed name when the name cannot be made unique`() = runTest {
+        val mediaStore = registerFakeMediaStoreContentProvider().apply {
+            conflictingDisplayNames = setOf("a-file.txt")
+        }
+        createFileSave().save(aFileToSave().path)
+        assertThat(mediaStore.insertedDisplayNames).hasSize(2)
+        assertThat(mediaStore.insertedDisplayNames.last()).matches("a-file_\\d+\\.txt")
+        assertThat(mediaStore.savedContent(mediaStore.insertedDisplayNames.last())).isEqualTo("Some content")
+    }
+
+    @Test
+    fun `save writes nothing when the destination cannot be created`() = runTest {
+        val mediaStore = registerFakeMediaStoreContentProvider().apply {
+            returnsNoUriOnInsert = true
+        }
+        createFileSave().save(aFileToSave().path)
+        assertThat(mediaStore.insertedDisplayNames).containsExactly("a-file.txt")
+        assertThat(mediaStore.savedFileNames()).isEmpty()
+    }
+
+    private fun aFileToSave(): File {
+        return File(RuntimeEnvironment.getApplication().cacheDir, "a-file.txt").apply {
+            writeText("Some content")
+        }
+    }
+
+    private fun TestScope.createFileSave() = DefaultFileSave(
+        context = RuntimeEnvironment.getApplication(),
+        dispatchers = testCoroutineDispatchers(),
+    )
+}

libraries/androidutils/src/main/kotlin/io/element/android/libraries/androidutils/file/File.kt

@@ -42,6 +42,32 @@ 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"
+    }
+}
+
+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() }

libraries/androidutils/src/test/kotlin/io/element/android/libraries/androidutils/file/FileTest.kt

@@ -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()
+    }
+}

libraries/mediaviewer/impl/src/main/kotlin/io/element/android/libraries/mediaviewer/impl/local/AndroidLocalMediaActions.kt

@@ -32,6 +32,7 @@ import androidx.core.content.PermissionChecker
 import androidx.core.net.toFile
 import dev.zacsweers.metro.AppScope
 import dev.zacsweers.metro.ContributesBinding
+import io.element.android.libraries.androidutils.file.saveWithUniqueFileName
 import io.element.android.libraries.androidutils.system.startInstallFromSourceIntent
 import io.element.android.libraries.core.coroutine.CoroutineDispatchers
 import io.element.android.libraries.core.extensions.runCatchingExceptions
@@ -44,6 +45,7 @@ import kotlinx.coroutines.withContext
 import timber.log.Timber
 import java.io.File
 import java.io.FileOutputStream
+import java.io.IOException
 import java.io.InputStream
 
 @ContributesBinding(AppScope::class)
@@ -172,18 +174,18 @@ class AndroidLocalMediaActions(
 
     @RequiresApi(Build.VERSION_CODES.Q)
     private fun saveOnDiskUsingMediaStore(localMedia: LocalMedia) {
-        val contentValues = ContentValues().apply {
-            put(MediaStore.MediaColumns.DISPLAY_NAME, localMedia.info.filename)
-            put(MediaStore.MediaColumns.MIME_TYPE, localMedia.info.mimeType)
-            put(MediaStore.MediaColumns.RELATIVE_PATH, Environment.DIRECTORY_DOWNLOADS)
-        }
         val resolver = context.contentResolver
-        val outputUri = resolver.insert(MediaStore.Downloads.EXTERNAL_CONTENT_URI, contentValues)
-        if (outputUri != null) {
-            localMedia.openStream()?.use { input ->
-                resolver.openOutputStream(outputUri).use { output ->
-                    input.copyTo(output!!, DEFAULT_BUFFER_SIZE)
-                }
+        val outputUri = saveWithUniqueFileName(localMedia.info.filename) { displayName ->
+            val contentValues = ContentValues().apply {
+                put(MediaStore.MediaColumns.DISPLAY_NAME, displayName)
+                put(MediaStore.MediaColumns.MIME_TYPE, localMedia.info.mimeType)
+                put(MediaStore.MediaColumns.RELATIVE_PATH, Environment.DIRECTORY_DOWNLOADS)
+            }
+            resolver.insert(MediaStore.Downloads.EXTERNAL_CONTENT_URI, contentValues)
+        } ?: throw IOException("Unable to create the destination file")
+        localMedia.openStream()?.use { input ->
+            resolver.openOutputStream(outputUri).use { output ->
+                input.copyTo(output!!, DEFAULT_BUFFER_SIZE)
             }
         }
     }

libraries/mediaviewer/impl/src/test/kotlin/io/element/android/libraries/mediaviewer/impl/local/AndroidLocalMediaActionsTest.kt

@@ -12,18 +12,22 @@ import android.net.Uri
 import androidx.activity.compose.LocalActivityResultRegistryOwner
 import androidx.compose.runtime.CompositionLocalProvider
 import androidx.compose.ui.platform.LocalContext
+import androidx.core.net.toUri
 import app.cash.molecule.RecompositionMode
 import app.cash.molecule.moleculeFlow
 import app.cash.turbine.test
 import com.google.common.truth.Truth.assertThat
 import io.element.android.libraries.matrix.test.core.aBuildMeta
 import io.element.android.libraries.mediaviewer.test.viewer.aLocalMedia
+import io.element.android.tests.testutils.fake.registerFakeMediaStoreContentProvider
 import io.element.android.tests.testutils.robolectric.RobolectricTest
 import io.element.android.tests.testutils.testCoroutineDispatchers
 import kotlinx.coroutines.test.TestScope
 import kotlinx.coroutines.test.runTest
 import org.junit.Test
 import org.robolectric.RuntimeEnvironment
+import java.io.File
+import java.io.IOException
 
 class AndroidLocalMediaActionsTest : RobolectricTest() {
     @Test
@@ -62,6 +66,43 @@ class AndroidLocalMediaActionsTest : RobolectricTest() {
         assertThat(result.exceptionOrNull()).isNotNull()
     }
 
+    @Test
+    fun `test AndroidLocalMediaAction save on disk inserts the media under its own name`() = runTest {
+        val mediaStore = registerFakeMediaStoreContentProvider()
+        val sut = createAndroidLocalMediaActions()
+        val result = sut.saveOnDisk(aLocalMedia(aMediaFile().toUri()))
+        assertThat(result.exceptionOrNull()).isNull()
+        assertThat(mediaStore.insertedDisplayNames).containsExactly("an image file.jpg")
+    }
+
+    @Test
+    fun `test AndroidLocalMediaAction save on disk retries with a suffixed name when the name cannot be made unique`() = runTest {
+        val mediaStore = registerFakeMediaStoreContentProvider().apply {
+            conflictingDisplayNames = setOf("an image file.jpg")
+        }
+        val sut = createAndroidLocalMediaActions()
+        val result = sut.saveOnDisk(aLocalMedia(aMediaFile().toUri()))
+        assertThat(result.exceptionOrNull()).isNull()
+        assertThat(mediaStore.insertedDisplayNames).hasSize(2)
+        assertThat(mediaStore.insertedDisplayNames.last()).matches("an image file_\\d+\\.jpg")
+    }
+
+    @Test
+    fun `test AndroidLocalMediaAction save on disk fails when the destination cannot be created`() = runTest {
+        registerFakeMediaStoreContentProvider().apply {
+            returnsNoUriOnInsert = true
+        }
+        val sut = createAndroidLocalMediaActions()
+        val result = sut.saveOnDisk(aLocalMedia(aMediaFile().toUri()))
+        assertThat(result.exceptionOrNull()).isInstanceOf(IOException::class.java)
+    }
+
+    private fun aMediaFile(): File {
+        return File(RuntimeEnvironment.getApplication().cacheDir, "an image file.jpg").apply {
+            writeText("Some content")
+        }
+    }
+
     private fun TestScope.createAndroidLocalMediaActions() = AndroidLocalMediaActions(
         RuntimeEnvironment.getApplication(),
         testCoroutineDispatchers(),

tests/konsist/src/test/kotlin/io/element/android/tests/konsist/KonsistClassNameTest.kt

@@ -103,6 +103,7 @@ class KonsistClassNameTest {
                 "FakeImageLoader",
                 "FakeKeyStore",
                 "FakeListenableFuture",
+                "FakeMediaStoreContentProvider",
                 "FakeParentNode",
             )
             .assertTrue {

tests/testutils/src/main/kotlin/io/element/android/tests/testutils/fake/FakeMediaStoreContentProvider.kt

@@ -0,0 +1,74 @@
+/*
+ * 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.tests.testutils.fake
+
+import android.content.ContentProvider
+import android.content.ContentValues
+import android.content.pm.ProviderInfo
+import android.database.Cursor
+import android.net.Uri
+import android.os.ParcelFileDescriptor
+import android.provider.MediaStore
+import androidx.test.core.app.ApplicationProvider
+import org.robolectric.shadows.ShadowContentResolver
+import java.io.File
+
+/**
+ * Stands in for the MediaStore provider: it records the display names an insert is attempted with,
+ * writes what is copied into it to a temporary directory, and can make any display name collide the
+ * way MediaStore does once a directory already holds 32 files with the same name.
+ */
+class FakeMediaStoreContentProvider : ContentProvider() {
+    private val directory: File = File.createTempFile("fake-media-store", null).apply {
+        delete()
+        mkdir()
+    }
+
+    val insertedDisplayNames = mutableListOf<String>()
+
+    var conflictingDisplayNames: Set<String> = emptySet()
+
+    /** When true, an insert reports that it created nothing, the way a rejected insert does. */
+    var returnsNoUriOnInsert: Boolean = false
+
+    fun savedFileNames(): List<String> = directory.list().orEmpty().toList()
+
+    fun savedContent(displayName: String): String = File(directory, displayName).readText()
+
+    override fun onCreate() = true
+
+    override fun insert(uri: Uri, values: ContentValues?): Uri? {
+        val displayName = values?.getAsString(MediaStore.MediaColumns.DISPLAY_NAME).orEmpty()
+        insertedDisplayNames += displayName
+        check(displayName !in conflictingDisplayNames) { "Failed to build unique file" }
+        return if (returnsNoUriOnInsert) null else uri.buildUpon().appendPath(displayName).build()
+    }
+
+    override fun openFile(uri: Uri, mode: String): ParcelFileDescriptor {
+        val file = File(directory, uri.lastPathSegment.orEmpty())
+        return ParcelFileDescriptor.open(file, ParcelFileDescriptor.MODE_CREATE or ParcelFileDescriptor.MODE_READ_WRITE)
+    }
+
+    override fun query(uri: Uri, projection: Array<out String>?, selection: String?, selectionArgs: Array<out String>?, sortOrder: String?): Cursor? = null
+
+    override fun getType(uri: Uri): String? = null
+
+    override fun update(uri: Uri, values: ContentValues?, selection: String?, selectionArgs: Array<out String>?) = 0
+
+    override fun delete(uri: Uri, selection: String?, selectionArgs: Array<out String>?) = 0
+}
+
+fun registerFakeMediaStoreContentProvider(): FakeMediaStoreContentProvider {
+    return FakeMediaStoreContentProvider().apply {
+        attachInfo(
+            ApplicationProvider.getApplicationContext(),
+            ProviderInfo().apply { authority = MediaStore.AUTHORITY }
+        )
+        ShadowContentResolver.registerProviderInternal(MediaStore.AUTHORITY, this)
+    }
+}