diff --git a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/voicemessages/timeline/VoiceMessageMediaRepo.kt b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/voicemessages/timeline/VoiceMessageMediaRepo.kt index a559183261..98b43ae415 100644 --- a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/voicemessages/timeline/VoiceMessageMediaRepo.kt +++ b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/voicemessages/timeline/VoiceMessageMediaRepo.kt @@ -83,13 +83,15 @@ class DefaultVoiceMessageMediaRepo @AssistedInject constructor( ): DefaultVoiceMessageMediaRepo } - override suspend fun getMediaFile(): Result = if (!isInCache()) { - matrixMediaLoader.downloadMediaFile( + override suspend fun getMediaFile(): Result = when { + cachedFile == null -> Result.failure(IllegalStateException("Invalid mxcUri.")) + cachedFile.exists() -> Result.success(cachedFile) + else -> matrixMediaLoader.downloadMediaFile( source = mediaSource, mimeType = mimeType, body = body, ).mapCatching { - val dest = cachedFilePath.apply { parentFile?.mkdirs() } + val dest = cachedFile.apply { parentFile?.mkdirs() } // TODO By not closing the MediaFile we're leaking the rust file handle here. // Not that big of a deal but better to avoid it someday. if (it.toFile().renameTo(dest)) { @@ -98,13 +100,11 @@ class DefaultVoiceMessageMediaRepo @AssistedInject constructor( error("Failed to move file to cache.") } } - } else { - Result.success(cachedFilePath) } - private val cachedFilePath: File = File("${cacheDir.path}/$CACHE_VOICE_SUBDIR/${mxcUri2FilePath(mediaSource.url)}") - - private fun isInCache(): Boolean = cachedFilePath.exists() + private val cachedFile: File? = mxcUri2FilePath(mediaSource.url)?.let { + File("${cacheDir.path}/$CACHE_VOICE_SUBDIR/$it") + } } /** @@ -123,12 +123,9 @@ private val mxcRegex = Regex("""^mxc:\/\/([^\/]+)\/([^\/]+)$""") * Sanitizes an mxcUri to be used as a relative file path. * * @param mxcUri the Matrix Content (mxc://) URI of the voice message. - * @return the relative file path as "/". - * @throws IllegalStateException if the mxcUri is invalid. + * @return the relative file path as "/" or null if the mxcUri is invalid. */ -private fun mxcUri2FilePath(mxcUri: String): String = checkNotNull(mxcRegex.matchEntire(mxcUri)) { - "mxcUri2FilePath: Invalid mxcUri: $mxcUri" -}.let { match -> +private fun mxcUri2FilePath(mxcUri: String): String? = mxcRegex.matchEntire(mxcUri)?.let { match -> buildString { append(match.groupValues[1]) append("/") diff --git a/features/messages/impl/src/test/kotlin/io/element/android/features/messages/voicemessages/timeline/DefaultVoiceMessageMediaRepoTest.kt b/features/messages/impl/src/test/kotlin/io/element/android/features/messages/voicemessages/timeline/DefaultVoiceMessageMediaRepoTest.kt index 3b19e66450..152090b14c 100644 --- a/features/messages/impl/src/test/kotlin/io/element/android/features/messages/voicemessages/timeline/DefaultVoiceMessageMediaRepoTest.kt +++ b/features/messages/impl/src/test/kotlin/io/element/android/features/messages/voicemessages/timeline/DefaultVoiceMessageMediaRepoTest.kt @@ -63,7 +63,7 @@ class DefaultVoiceMessageMediaRepoTest { repo.getMediaFile().let { result -> Truth.assertThat(result.isFailure).isTrue() - result.exceptionOrNull()?.let { exception -> + result.exceptionOrNull()!!.let { exception -> Truth.assertThat(exception).isInstanceOf(RuntimeException::class.java) } } @@ -116,16 +116,32 @@ class DefaultVoiceMessageMediaRepoTest { } } } + + @Test + fun `invalid mxc uri returns a failure`() = runTest { + val repo = createDefaultVoiceMessageMediaRepo( + temporaryFolder = temporaryFolder, + mxcUri = INVALID_MXC_URI, + ) + repo.getMediaFile().let { result -> + Truth.assertThat(result.isFailure).isTrue() + result.exceptionOrNull()!!.let { exception -> + Truth.assertThat(exception).isInstanceOf(RuntimeException::class.java) + Truth.assertThat(exception).hasMessageThat().isEqualTo("Invalid mxcUri.") + } + } + } } private fun createDefaultVoiceMessageMediaRepo( temporaryFolder: TemporaryFolder, matrixMediaLoader: MatrixMediaLoader = FakeMediaLoader(), + mxcUri: String = MXC_URI, ) = DefaultVoiceMessageMediaRepo( cacheDir = temporaryFolder.root, matrixMediaLoader = matrixMediaLoader, mediaSource = MediaSource( - url = MXC_URI, + url = mxcUri, json = null ), mimeType = "audio/ogg", @@ -133,6 +149,7 @@ private fun createDefaultVoiceMessageMediaRepo( ) private const val MXC_URI = "mxc://matrix.org/1234567890abcdefg" +private const val INVALID_MXC_URI = "notAnMxcUri" private val TemporaryFolder.cachedFilePath get() = "${this.root.path}/temp/voice/matrix.org/1234567890abcdefg" private fun TemporaryFolder.createCachedFile() = File(cachedFilePath).apply { parentFile?.mkdirs()