diff --git a/features/viewfolder/impl/src/main/kotlin/io/element/android/features/viewfolder/impl/file/FileSave.kt b/features/viewfolder/impl/src/main/kotlin/io/element/android/features/viewfolder/impl/file/FileSave.kt index 03f442f36e..add55cee36 100644 --- a/features/viewfolder/impl/src/main/kotlin/io/element/android/features/viewfolder/impl/file/FileSave.kt +++ b/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) } } } diff --git a/features/viewfolder/impl/src/test/kotlin/io/element/android/features/viewfolder/impl/file/DefaultFileSaveTest.kt b/features/viewfolder/impl/src/test/kotlin/io/element/android/features/viewfolder/impl/file/DefaultFileSaveTest.kt new file mode 100644 index 0000000000..a991a11f76 --- /dev/null +++ b/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(), + ) +} diff --git a/libraries/androidutils/src/main/kotlin/io/element/android/libraries/androidutils/file/File.kt b/libraries/androidutils/src/main/kotlin/io/element/android/libraries/androidutils/file/File.kt index 21306ff192..7c5f0450e9 100644 --- a/libraries/androidutils/src/main/kotlin/io/element/android/libraries/androidutils/file/File.kt +++ b/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 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() } diff --git a/libraries/androidutils/src/test/kotlin/io/element/android/libraries/androidutils/file/FileTest.kt b/libraries/androidutils/src/test/kotlin/io/element/android/libraries/androidutils/file/FileTest.kt new file mode 100644 index 0000000000..b2e28f6249 --- /dev/null +++ b/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() + + 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() + + 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() + + 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() + } +} diff --git a/libraries/mediaviewer/impl/src/main/kotlin/io/element/android/libraries/mediaviewer/impl/local/AndroidLocalMediaActions.kt b/libraries/mediaviewer/impl/src/main/kotlin/io/element/android/libraries/mediaviewer/impl/local/AndroidLocalMediaActions.kt index 071e0dba64..4364020052 100644 --- a/libraries/mediaviewer/impl/src/main/kotlin/io/element/android/libraries/mediaviewer/impl/local/AndroidLocalMediaActions.kt +++ b/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) } } } diff --git a/libraries/mediaviewer/impl/src/test/kotlin/io/element/android/libraries/mediaviewer/impl/local/AndroidLocalMediaActionsTest.kt b/libraries/mediaviewer/impl/src/test/kotlin/io/element/android/libraries/mediaviewer/impl/local/AndroidLocalMediaActionsTest.kt index cdc5f7220d..ab1b193e26 100644 --- a/libraries/mediaviewer/impl/src/test/kotlin/io/element/android/libraries/mediaviewer/impl/local/AndroidLocalMediaActionsTest.kt +++ b/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(), diff --git a/tests/konsist/src/test/kotlin/io/element/android/tests/konsist/KonsistClassNameTest.kt b/tests/konsist/src/test/kotlin/io/element/android/tests/konsist/KonsistClassNameTest.kt index 53cdbbf225..14a3f24f3c 100644 --- a/tests/konsist/src/test/kotlin/io/element/android/tests/konsist/KonsistClassNameTest.kt +++ b/tests/konsist/src/test/kotlin/io/element/android/tests/konsist/KonsistClassNameTest.kt @@ -103,6 +103,7 @@ class KonsistClassNameTest { "FakeImageLoader", "FakeKeyStore", "FakeListenableFuture", + "FakeMediaStoreContentProvider", "FakeParentNode", ) .assertTrue { diff --git a/tests/testutils/src/main/kotlin/io/element/android/tests/testutils/fake/FakeMediaStoreContentProvider.kt b/tests/testutils/src/main/kotlin/io/element/android/tests/testutils/fake/FakeMediaStoreContentProvider.kt new file mode 100644 index 0000000000..13077db51f --- /dev/null +++ b/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() + + var conflictingDisplayNames: Set = emptySet() + + /** When true, an insert reports that it created nothing, the way a rejected insert does. */ + var returnsNoUriOnInsert: Boolean = false + + fun savedFileNames(): List = 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?, selection: String?, selectionArgs: Array?, sortOrder: String?): Cursor? = null + + override fun getType(uri: Uri): String? = null + + override fun update(uri: Uri, values: ContentValues?, selection: String?, selectionArgs: Array?) = 0 + + override fun delete(uri: Uri, selection: String?, selectionArgs: Array?) = 0 +} + +fun registerFakeMediaStoreContentProvider(): FakeMediaStoreContentProvider { + return FakeMediaStoreContentProvider().apply { + attachInfo( + ApplicationProvider.getApplicationContext(), + ProviderInfo().apply { authority = MediaStore.AUTHORITY } + ) + ShadowContentResolver.registerProviderInternal(MediaStore.AUTHORITY, this) + } +}