diff --git a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/voicemessages/timeline/VoiceMessageCache.kt b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/voicemessages/timeline/VoiceMessageCache.kt deleted file mode 100644 index 8ced58132e..0000000000 --- a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/voicemessages/timeline/VoiceMessageCache.kt +++ /dev/null @@ -1,123 +0,0 @@ -/* - * Copyright (c) 2023 New Vector Ltd - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package io.element.android.features.messages.impl.voicemessages.timeline - -import com.squareup.anvil.annotations.ContributesBinding -import dagger.assisted.Assisted -import dagger.assisted.AssistedFactory -import dagger.assisted.AssistedInject -import io.element.android.libraries.di.AppScope -import io.element.android.libraries.di.CacheDirectory -import java.io.File - -/** - * Manages the local disk cache for a voice message. - */ -interface VoiceMessageCache { - - /** - * Factory for [VoiceMessageCache]. - */ - fun interface Factory { - /** - * Creates a [VoiceMessageCache] for the given Matrix Content (mxc://) URI. - * - * @param mxcUri the Matrix Content (mxc://) URI of the voice message. - */ - fun create(mxcUri: String): VoiceMessageCache - } - - /** - * The file path of the voice message in the cache directory. - * NB: This doesn't necessarily mean that the file exists. - * - * @return the file path of the voice message in the cache directory. - */ - val cachePath: String - - /** - * Checks if the voice message is in the cache directory. - * - * @return true if the voice message is in the cache directory. - */ - fun isInCache(): Boolean - - /** - * Moves the file to the voice cache directory. - * - * @return true if the file was successfully moved. - */ - fun moveToCache(file: File): Boolean -} - -/** - * Default implementation of [VoiceMessageCache]. - * - * NB: All methods will throw an [IllegalStateException] if the mxcUri is invalid. - * - * @param cacheDir the application's cache directory. - * @param mxcUri the Matrix Content (mxc://) URI of the voice message. - */ -class VoiceMessageCacheImpl @AssistedInject constructor( - @CacheDirectory private val cacheDir: File, - @Assisted private val mxcUri: String, -) : VoiceMessageCache { - - @ContributesBinding(AppScope::class) - @AssistedFactory - fun interface Factory : VoiceMessageCache.Factory { - override fun create(mxcUri: String): VoiceMessageCacheImpl - } - - override val cachePath: String = "${cacheDir.path}/$CACHE_VOICE_SUBDIR/${mxcUri2FilePath(mxcUri)}" - - override fun isInCache(): Boolean = File(cachePath).exists() - - override fun moveToCache(file: File): Boolean { - val dest = File(cachePath).apply { parentFile?.mkdirs() } - return file.renameTo(dest) - } -} - -/** - * Subdirectory of the application's cache directory where voice messages are stored. - */ -private const val CACHE_VOICE_SUBDIR = "temp/voice" - -/** - * Regex to match a Matrix Content (mxc://) URI. - * - * See: https://spec.matrix.org/v1.8/client-server-api/#matrix-content-mxc-uris - */ -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. - */ -private fun mxcUri2FilePath(mxcUri: String): String = checkNotNull(mxcRegex.matchEntire(mxcUri)) { - "mxcUri2FilePath: Invalid mxcUri: $mxcUri" -}.let { match -> - buildString { - append(match.groupValues[1]) - append("/") - append(match.groupValues[2]) - } -} 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 new file mode 100644 index 0000000000..a559183261 --- /dev/null +++ b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/voicemessages/timeline/VoiceMessageMediaRepo.kt @@ -0,0 +1,137 @@ +/* + * Copyright (c) 2023 New Vector Ltd + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package io.element.android.features.messages.impl.voicemessages.timeline + +import com.squareup.anvil.annotations.ContributesBinding +import dagger.assisted.Assisted +import dagger.assisted.AssistedFactory +import dagger.assisted.AssistedInject +import io.element.android.libraries.di.CacheDirectory +import io.element.android.libraries.di.RoomScope +import io.element.android.libraries.matrix.api.media.MatrixMediaLoader +import io.element.android.libraries.matrix.api.media.MediaSource +import io.element.android.libraries.matrix.api.media.toFile +import java.io.File + +/** + * Fetches the media file for a voice message. + * + * Media is downloaded from the rust sdk and stored in the application's cache directory. + * Media files are indexed by their Matrix Content (mxc://) URI and considered immutable. + * Whenever a given mxc is found in the cache, it is returned immediately. + */ +interface VoiceMessageMediaRepo { + + /** + * Factory for [VoiceMessageMediaRepo]. + */ + fun interface Factory { + /** + * Creates a [VoiceMessageMediaRepo]. + * + * @param mediaSource the media source of the voice message. + * @param mimeType the mime type of the voice message. + * @param body the body of the voice message. + */ + fun create( + mediaSource: MediaSource, + mimeType: String?, + body: String?, + ): VoiceMessageMediaRepo + } + + /** + * Returns the voice message media file. + * + * In case of a cache hit the file is returned immediately. + * In case of a cache miss the file is downloaded and then returned. + * + * @return A [Result] holding either the media [File] from the cache directory or an [Exception]. + */ + suspend fun getMediaFile(): Result +} + +class DefaultVoiceMessageMediaRepo @AssistedInject constructor( + @CacheDirectory private val cacheDir: File, + private val matrixMediaLoader: MatrixMediaLoader, + @Assisted private val mediaSource: MediaSource, + @Assisted("mimeType") private val mimeType: String?, + @Assisted("body") private val body: String?, +) : VoiceMessageMediaRepo { + + @ContributesBinding(RoomScope::class) + @AssistedFactory + fun interface Factory : VoiceMessageMediaRepo.Factory { + override fun create( + mediaSource: MediaSource, + @Assisted("mimeType") mimeType: String?, + @Assisted("body") body: String?, + ): DefaultVoiceMessageMediaRepo + } + + override suspend fun getMediaFile(): Result = if (!isInCache()) { + matrixMediaLoader.downloadMediaFile( + source = mediaSource, + mimeType = mimeType, + body = body, + ).mapCatching { + val dest = cachedFilePath.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)) { + dest + } else { + 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() +} + +/** + * Subdirectory of the application's cache directory where voice messages are stored. + */ +private const val CACHE_VOICE_SUBDIR = "temp/voice" + +/** + * Regex to match a Matrix Content (mxc://) URI. + * + * See: https://spec.matrix.org/v1.8/client-server-api/#matrix-content-mxc-uris + */ +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. + */ +private fun mxcUri2FilePath(mxcUri: String): String = checkNotNull(mxcRegex.matchEntire(mxcUri)) { + "mxcUri2FilePath: Invalid mxcUri: $mxcUri" +}.let { match -> + buildString { + append(match.groupValues[1]) + append("/") + append(match.groupValues[2]) + } +} diff --git a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/voicemessages/timeline/VoiceMessagePlayer.kt b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/voicemessages/timeline/VoiceMessagePlayer.kt index a2d4f0b655..3934c88b9c 100644 --- a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/voicemessages/timeline/VoiceMessagePlayer.kt +++ b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/voicemessages/timeline/VoiceMessagePlayer.kt @@ -20,6 +20,7 @@ import com.squareup.anvil.annotations.ContributesBinding import io.element.android.features.messages.impl.mediaplayer.MediaPlayer import io.element.android.libraries.di.RoomScope import io.element.android.libraries.matrix.api.core.EventId +import io.element.android.libraries.matrix.api.media.MediaSource import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.distinctUntilChanged import kotlinx.coroutines.flow.map @@ -37,14 +38,20 @@ interface VoiceMessagePlayer { * * NB: Different voice messages can use the same content uri (e.g. in case of * a forward of a voice message), - * therefore the media uri is not enough to uniquely identify a voice message. - * This is why we must provide the eventId as well. + * therefore the mxc:// uri in [mediaSource] is not enough to uniquely identify + * a voice message. This is why we must provide the eventId as well. * - * @param eventId The id of the voice message event. If null, a dummy - * player is returned. - * @param mediaPath The path to the voice message's media file. + * @param eventId The eventId of the voice message event. + * @param mediaSource The media source of the voice message. + * @param mimeType The mime type of the voice message. + * @param body The body of the voice message. */ - fun create(eventId: EventId?, mediaPath: String): VoiceMessagePlayer + fun create( + eventId: EventId?, + mediaSource: MediaSource, + mimeType: String?, + body: String?, + ): VoiceMessagePlayer } /** @@ -53,15 +60,14 @@ interface VoiceMessagePlayer { val state: Flow /** - * Start playing from the beginning acquiring control of the - * underlying [MediaPlayer]. + * Starts playing from the beginning + * acquiring control of the underlying [MediaPlayer]. + * If already in control of the underlying [MediaPlayer], starts playing from the + * current position. + * + * Will suspend whilst the media file is being downloaded. */ - fun acquireControlAndPlay() - - /** - * Start playing from the current position. - */ - fun play() + suspend fun play(): Result /** * Pause playback. @@ -92,32 +98,45 @@ interface VoiceMessagePlayer { } /** - * An implementation of [VoiceMessagePlayer] which is backed by a [MediaPlayer] - * usually shared among different [VoiceMessagePlayer] instances. - * - * @param mediaPlayer The [MediaPlayer] to use. - * @param eventId The id of the voice message event. If null, the player will behave as no-op. - * @param mediaPath The path to the voice message's media file. + * An implementation of [VoiceMessagePlayer] which is backed by a + * [VoiceMessageMediaRepo] to fetch and cache the media file and + * which uses a global [MediaPlayer] instance to play the media. */ -class VoiceMessagePlayerImpl( +class DefaultVoiceMessagePlayer( private val mediaPlayer: MediaPlayer, + voiceMessageMediaRepoFactory: VoiceMessageMediaRepo.Factory, private val eventId: EventId?, - private val mediaPath: String, + mediaSource: MediaSource, + mimeType: String?, + body: String?, ) : VoiceMessagePlayer { @ContributesBinding(RoomScope::class) // Scoped types can't use @AssistedInject. class Factory @Inject constructor( private val mediaPlayer: MediaPlayer, + private val voiceMessageMediaRepoFactory: VoiceMessageMediaRepo.Factory, ) : VoiceMessagePlayer.Factory { - override fun create(eventId: EventId?, mediaPath: String): VoiceMessagePlayerImpl { - return VoiceMessagePlayerImpl( - mediaPlayer = mediaPlayer, - eventId = eventId, - mediaPath = mediaPath, - ) - } + override fun create( + eventId: EventId?, + mediaSource: MediaSource, + mimeType: String?, + body: String?, + ): DefaultVoiceMessagePlayer = DefaultVoiceMessagePlayer( + mediaPlayer = mediaPlayer, + voiceMessageMediaRepoFactory = voiceMessageMediaRepoFactory, + eventId = eventId, + mediaSource = mediaSource, + mimeType = mimeType, + body = body, + ) } + private val repo = voiceMessageMediaRepoFactory.create( + mediaSource = mediaSource, + mimeType = mimeType, + body = body + ) + override val state: Flow = mediaPlayer.state.map { state -> VoiceMessagePlayer.State( isPlaying = state.mediaId.isMyTrack() && state.isPlaying, @@ -126,19 +145,20 @@ class VoiceMessagePlayerImpl( ) }.distinctUntilChanged() - override fun acquireControlAndPlay() { - eventId?.let { eventId -> - mediaPlayer.acquireControlAndPlay( - uri = mediaPath, - mediaId = eventId.value, - mimeType = "audio/ogg" // Files in the voice cache have no extension so we need to set the mime type manually. - ) - } - } - - override fun play() { - ifInControl { - mediaPlayer.play() + override suspend fun play(): Result = if (inControl()) { + mediaPlayer.play() + Result.success(Unit) + } else { + if (eventId != null) { + repo.getMediaFile().mapCatching { mediaFile -> + mediaPlayer.acquireControlAndPlay( + uri = mediaFile.path, + mediaId = eventId.value, + mimeType = "audio/ogg" // Files in the voice cache have no extension so we need to set the mime type manually. + ) + } + } else { + Result.failure(IllegalStateException("Cannot play a voice message with no eventId")) } } @@ -157,6 +177,8 @@ class VoiceMessagePlayerImpl( private fun String?.isMyTrack(): Boolean = if (eventId == null) false else this == eventId.value private inline fun ifInControl(block: () -> Unit) { - if (mediaPlayer.state.value.mediaId.isMyTrack()) block() + if (inControl()) block() } + + private fun inControl(): Boolean = mediaPlayer.state.value.mediaId.isMyTrack() } diff --git a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/voicemessages/timeline/VoiceMessagePresenter.kt b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/voicemessages/timeline/VoiceMessagePresenter.kt index f94176b601..6bff6ef26f 100644 --- a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/voicemessages/timeline/VoiceMessagePresenter.kt +++ b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/voicemessages/timeline/VoiceMessagePresenter.kt @@ -37,9 +37,6 @@ import io.element.android.libraries.architecture.Async import io.element.android.libraries.architecture.Presenter import io.element.android.libraries.architecture.runUpdatingState import io.element.android.libraries.di.RoomScope -import io.element.android.libraries.matrix.api.media.MatrixMediaLoader -import io.element.android.libraries.matrix.api.media.MediaFile -import io.element.android.libraries.matrix.api.media.toFile import io.element.android.libraries.ui.utils.time.formatShort import kotlinx.coroutines.launch import kotlin.time.Duration.Companion.milliseconds @@ -54,9 +51,7 @@ interface VoiceMessagePresenterModule { } class VoiceMessagePresenter @AssistedInject constructor( - private val mediaLoader: MatrixMediaLoader, voiceMessagePlayerFactory: VoiceMessagePlayer.Factory, - voiceMessageCacheFactory: VoiceMessageCache.Factory, @Assisted private val content: TimelineItemVoiceContent, ) : Presenter { @@ -65,11 +60,11 @@ class VoiceMessagePresenter @AssistedInject constructor( override fun create(content: TimelineItemVoiceContent): VoiceMessagePresenter } - private val voiceCache = voiceMessageCacheFactory.create(mxcUri = content.mediaSource.url) - private val player = voiceMessagePlayerFactory.create( eventId = content.eventId, - mediaPath = voiceCache.cachePath + mediaSource = content.mediaSource, + mimeType = content.mimeType, + body = content.body, ) @Composable @@ -78,15 +73,15 @@ class VoiceMessagePresenter @AssistedInject constructor( val scope = rememberCoroutineScope() val playerState by player.state.collectAsState(VoiceMessagePlayer.State(isPlaying = false, isMyMedia = false, currentPosition = 0L)) - val mediaFile = remember { mutableStateOf>(Async.Uninitialized) } + val play = remember { mutableStateOf>(Async.Uninitialized) } val button by remember { derivedStateOf { when { content.eventId == null -> VoiceMessageState.Button.Disabled playerState.isPlaying -> VoiceMessageState.Button.Pause - mediaFile.value is Async.Loading -> VoiceMessageState.Button.Downloading - mediaFile.value is Async.Failure -> VoiceMessageState.Button.Retry + play.value is Async.Loading -> VoiceMessageState.Button.Downloading + play.value is Async.Failure -> VoiceMessageState.Button.Retry else -> VoiceMessageState.Button.Play } } @@ -101,38 +96,13 @@ class VoiceMessagePresenter @AssistedInject constructor( } } - suspend fun downloadCacheAndPlay() { - mediaFile.runUpdatingState { - mediaLoader.downloadMediaFile( - source = content.mediaSource, - mimeType = content.mimeType, - body = content.body, - ).mapCatching { - if (voiceCache.moveToCache(it.toFile())) { - player.acquireControlAndPlay() - it - } else { - error("Failed to move file to cache.") - } - } - } - } - fun eventSink(event: VoiceMessageEvents) { when (event) { is VoiceMessageEvents.PlayPause -> { - if (playerState.isMyMedia) { - if (playerState.isPlaying) { - player.pause() - } else { - player.play() - } + if (playerState.isPlaying) { + player.pause() } else { - if (voiceCache.isInCache()) { - player.acquireControlAndPlay() - } else { - scope.launch { downloadCacheAndPlay() } - } + scope.launch { play.runUpdatingState { player.play() } } } } is VoiceMessageEvents.Seek -> { 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 new file mode 100644 index 0000000000..3b19e66450 --- /dev/null +++ b/features/messages/impl/src/test/kotlin/io/element/android/features/messages/voicemessages/timeline/DefaultVoiceMessageMediaRepoTest.kt @@ -0,0 +1,142 @@ +/* + * Copyright (c) 2023 New Vector Ltd + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package io.element.android.features.messages.voicemessages.timeline + +import com.google.common.truth.Truth +import io.element.android.features.messages.impl.voicemessages.timeline.DefaultVoiceMessageMediaRepo +import io.element.android.libraries.matrix.api.media.MatrixMediaLoader +import io.element.android.libraries.matrix.api.media.MediaSource +import io.element.android.libraries.matrix.test.media.FakeMediaLoader +import kotlinx.coroutines.test.runTest +import org.junit.Rule +import org.junit.Test +import org.junit.rules.TemporaryFolder +import java.io.File + +class DefaultVoiceMessageMediaRepoTest { + + @get:Rule + val temporaryFolder = TemporaryFolder() + + @Test + fun `cache miss - downloads and returns cached file successfully`() = runTest { + val fakeMediaLoader = FakeMediaLoader().apply { + path = temporaryFolder.createRustMediaFile().path + } + val repo = createDefaultVoiceMessageMediaRepo( + temporaryFolder = temporaryFolder, + matrixMediaLoader = fakeMediaLoader, + ) + + repo.getMediaFile().let { result -> + Truth.assertThat(result.isSuccess).isTrue() + result.getOrThrow().let { file -> + Truth.assertThat(file.path).isEqualTo(temporaryFolder.cachedFilePath) + Truth.assertThat(file.exists()).isTrue() + } + } + } + + @Test + fun `cache miss - download fails`() = runTest { + val fakeMediaLoader = FakeMediaLoader().apply { + shouldFail = true + } + val repo = createDefaultVoiceMessageMediaRepo( + temporaryFolder = temporaryFolder, + matrixMediaLoader = fakeMediaLoader, + ) + + repo.getMediaFile().let { result -> + Truth.assertThat(result.isFailure).isTrue() + result.exceptionOrNull()?.let { exception -> + Truth.assertThat(exception).isInstanceOf(RuntimeException::class.java) + } + } + } + + @Test + fun `cache miss - download succeeds but file move fails`() = runTest { + val fakeMediaLoader = FakeMediaLoader().apply { + path = temporaryFolder.createRustMediaFile().path + } + File(temporaryFolder.cachedFilePath).apply { + parentFile?.mkdirs() + // Deny access to parent folder so move to cache will fail. + parentFile?.setReadable(false) + parentFile?.setWritable(false) + parentFile?.setExecutable(false) + } + val repo = createDefaultVoiceMessageMediaRepo( + temporaryFolder = temporaryFolder, + matrixMediaLoader = fakeMediaLoader, + ) + + repo.getMediaFile().let { result -> + Truth.assertThat(result.isFailure).isTrue() + result.exceptionOrNull()?.let { exception -> + Truth.assertThat(exception).apply { + isInstanceOf(IllegalStateException::class.java) + hasMessageThat().isEqualTo("Failed to move file to cache.") + } + } + } + } + + @Test + fun `cache hit - returns cached file successfully`() = runTest { + temporaryFolder.createCachedFile() + val fakeMediaLoader = FakeMediaLoader().apply { + shouldFail = true // so that if we hit the media loader it will crash + } + val repo = createDefaultVoiceMessageMediaRepo( + temporaryFolder = temporaryFolder, + matrixMediaLoader = fakeMediaLoader, + ) + + repo.getMediaFile().let { result -> + Truth.assertThat(result.isSuccess).isTrue() + result.getOrThrow().let { file -> + Truth.assertThat(file.path).isEqualTo(temporaryFolder.cachedFilePath) + Truth.assertThat(file.exists()).isTrue() + } + } + } +} + +private fun createDefaultVoiceMessageMediaRepo( + temporaryFolder: TemporaryFolder, + matrixMediaLoader: MatrixMediaLoader = FakeMediaLoader(), +) = DefaultVoiceMessageMediaRepo( + cacheDir = temporaryFolder.root, + matrixMediaLoader = matrixMediaLoader, + mediaSource = MediaSource( + url = MXC_URI, + json = null + ), + mimeType = "audio/ogg", + body = "someBody.ogg" +) + +private const val MXC_URI = "mxc://matrix.org/1234567890abcdefg" +private val TemporaryFolder.cachedFilePath get() = "${this.root.path}/temp/voice/matrix.org/1234567890abcdefg" +private fun TemporaryFolder.createCachedFile() = File(cachedFilePath).apply { + parentFile?.mkdirs() + createNewFile() +} + +private fun TemporaryFolder.createRustMediaFile() = File(this.root, "rustMediaFile.ogg").apply { createNewFile() } diff --git a/features/messages/impl/src/test/kotlin/io/element/android/features/messages/voicemessages/timeline/DefaultVoiceMessagePlayerTest.kt b/features/messages/impl/src/test/kotlin/io/element/android/features/messages/voicemessages/timeline/DefaultVoiceMessagePlayerTest.kt new file mode 100644 index 0000000000..397a4a0373 --- /dev/null +++ b/features/messages/impl/src/test/kotlin/io/element/android/features/messages/voicemessages/timeline/DefaultVoiceMessagePlayerTest.kt @@ -0,0 +1,149 @@ +/* + * Copyright (c) 2023 New Vector Ltd + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package io.element.android.features.messages.voicemessages.timeline + +import app.cash.turbine.test +import com.google.common.truth.Truth +import io.element.android.features.messages.impl.mediaplayer.MediaPlayer +import io.element.android.features.messages.impl.voicemessages.timeline.DefaultVoiceMessagePlayer +import io.element.android.features.messages.impl.voicemessages.timeline.VoiceMessageMediaRepo +import io.element.android.features.messages.mediaplayer.FakeMediaPlayer +import io.element.android.libraries.matrix.api.core.EventId +import io.element.android.libraries.matrix.api.media.MediaSource +import io.element.android.libraries.matrix.test.AN_EVENT_ID +import kotlinx.coroutines.test.runTest +import org.junit.Test + +class DefaultVoiceMessagePlayerTest { + + @Test + fun `initial state`() = runTest { + createDefaultVoiceMessagePlayer().state.test { + awaitItem().let { + Truth.assertThat(it.isPlaying).isEqualTo(false) + Truth.assertThat(it.isMyMedia).isEqualTo(false) + Truth.assertThat(it.currentPosition).isEqualTo(0) + } + } + } + + @Test + fun `downloading and play works`() = runTest { + val player = createDefaultVoiceMessagePlayer() + player.state.test { + skipItems(1) // skip initial state. + Truth.assertThat(player.play().isSuccess).isTrue() + awaitItem().let { + Truth.assertThat(it.isPlaying).isEqualTo(true) + Truth.assertThat(it.isMyMedia).isEqualTo(true) + Truth.assertThat(it.currentPosition).isEqualTo(1000) + } + } + } + + @Test + fun `downloading and play fails`() = runTest { + val player = createDefaultVoiceMessagePlayer( + voiceMessageMediaRepo = FakeVoiceMessageMediaRepo().apply { + shouldFail = true + }, + ) + player.state.test { + skipItems(1) // skip initial state. + Truth.assertThat(player.play().isFailure).isTrue() + } + } + + @Test + fun `play fails with no eventId`() = runTest { + val player = createDefaultVoiceMessagePlayer( + eventId = null + ) + player.state.test { + skipItems(1) // skip initial state. + Truth.assertThat(player.play().isFailure).isTrue() + } + } + + @Test + fun `pause playing works`() = runTest { + val player = createDefaultVoiceMessagePlayer() + player.state.test { + skipItems(1) // skip initial state. + Truth.assertThat(player.play().isSuccess).isTrue() + skipItems(1) // skip play state + player.pause() + awaitItem().let { + Truth.assertThat(it.isPlaying).isEqualTo(false) + Truth.assertThat(it.isMyMedia).isEqualTo(true) + Truth.assertThat(it.currentPosition).isEqualTo(1000) + } + } + } + + @Test + fun `play after pause works`() = runTest { + val player = createDefaultVoiceMessagePlayer() + player.state.test { + skipItems(1) // skip initial state. + Truth.assertThat(player.play().isSuccess).isTrue() + skipItems(1) // skip play state + player.pause() + skipItems(1) + player.play() + awaitItem().let { + Truth.assertThat(it.isPlaying).isEqualTo(true) + Truth.assertThat(it.isMyMedia).isEqualTo(true) + Truth.assertThat(it.currentPosition).isEqualTo(2000) + } + } + } + + @Test + fun `seek to works`() = runTest { + val player = createDefaultVoiceMessagePlayer() + player.state.test { + skipItems(1) // skip initial state. + Truth.assertThat(player.play().isSuccess).isTrue() + skipItems(1) // skip play state + player.seekTo(2000) + awaitItem().let { + Truth.assertThat(it.isPlaying).isEqualTo(true) + Truth.assertThat(it.isMyMedia).isEqualTo(true) + Truth.assertThat(it.currentPosition).isEqualTo(2000) + } + } + } +} + +private fun createDefaultVoiceMessagePlayer( + mediaPlayer: MediaPlayer = FakeMediaPlayer(), + voiceMessageMediaRepo: VoiceMessageMediaRepo = FakeVoiceMessageMediaRepo(), + eventId: EventId? = AN_EVENT_ID, +) = DefaultVoiceMessagePlayer( + mediaPlayer = mediaPlayer, + voiceMessageMediaRepoFactory = { _, _, _ -> voiceMessageMediaRepo }, + eventId = eventId, + mediaSource = MediaSource( + url = MXC_URI, + json = null + ), + mimeType = "audio/ogg", + body = "someBody.ogg" +) + +private const val MXC_URI = "mxc://matrix.org/1234567890abcdefg" diff --git a/features/messages/impl/src/test/kotlin/io/element/android/features/messages/voicemessages/timeline/FakeVoiceMessageCache.kt b/features/messages/impl/src/test/kotlin/io/element/android/features/messages/voicemessages/timeline/FakeVoiceMessageMediaRepo.kt similarity index 52% rename from features/messages/impl/src/test/kotlin/io/element/android/features/messages/voicemessages/timeline/FakeVoiceMessageCache.kt rename to features/messages/impl/src/test/kotlin/io/element/android/features/messages/voicemessages/timeline/FakeVoiceMessageMediaRepo.kt index ee61a690e6..198b65e445 100644 --- a/features/messages/impl/src/test/kotlin/io/element/android/features/messages/voicemessages/timeline/FakeVoiceMessageCache.kt +++ b/features/messages/impl/src/test/kotlin/io/element/android/features/messages/voicemessages/timeline/FakeVoiceMessageMediaRepo.kt @@ -16,34 +16,22 @@ package io.element.android.features.messages.voicemessages.timeline -import io.element.android.features.messages.impl.voicemessages.timeline.VoiceMessageCache +import io.element.android.features.messages.impl.voicemessages.timeline.VoiceMessageMediaRepo +import io.element.android.tests.testutils.simulateLongTask import java.io.File /** - * A fake implementation of [VoiceMessageCache] for testing purposes. + * A fake implementation of [VoiceMessageMediaRepo] for testing purposes. */ -class FakeVoiceMessageCache : VoiceMessageCache { +class FakeVoiceMessageMediaRepo : VoiceMessageMediaRepo { - private var _cachePath: String = "" - private var _isInCache: Boolean = false - private var _moveToCache: Boolean = false + var shouldFail = false - override val cachePath: String - get() = _cachePath - - override fun isInCache(): Boolean = _isInCache - - override fun moveToCache(file: File): Boolean = _moveToCache - - fun givenCachePath(cachePath: String) { - _cachePath = cachePath - } - - fun givenIsInCache(isInCache: Boolean) { - _isInCache = isInCache - } - - fun givenMoveToCache(moveToCache: Boolean) { - _moveToCache = moveToCache + override suspend fun getMediaFile(): Result = simulateLongTask { + if (shouldFail) { + Result.failure(IllegalStateException("Failed to get media file")) + } else { + Result.success(File("")) + } } } diff --git a/features/messages/impl/src/test/kotlin/io/element/android/features/messages/voicemessages/timeline/VoiceMessageCacheTest.kt b/features/messages/impl/src/test/kotlin/io/element/android/features/messages/voicemessages/timeline/VoiceMessageCacheTest.kt deleted file mode 100644 index 3c7a6468a0..0000000000 --- a/features/messages/impl/src/test/kotlin/io/element/android/features/messages/voicemessages/timeline/VoiceMessageCacheTest.kt +++ /dev/null @@ -1,90 +0,0 @@ -/* - * Copyright (c) 2023 New Vector Ltd - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package io.element.android.features.messages.voicemessages.timeline - -import com.google.common.truth.Truth -import io.element.android.features.messages.impl.voicemessages.timeline.VoiceMessageCacheImpl -import org.junit.Rule -import org.junit.Test -import org.junit.rules.TemporaryFolder -import java.io.File - -class VoiceMessageCacheTest { - - @get:Rule - val temporaryFolder = TemporaryFolder() - - @Test - fun `moveToVoiceCache() should move the file to the voice cache dir`() { - val rootPath = temporaryFolder.root.path - val file = File("$rootPath/myFile.txt").apply { createNewFile() } - val cacheDir = File("$rootPath/cacheDir").apply { if (!exists()) mkdirs() } - val mxcUri = "mxc://matrix.org/1234567890abcdefg" - val cache = VoiceMessageCacheImpl(cacheDir, mxcUri) - - Truth.assertThat(cache.moveToCache(file)) - .isTrue() - Truth.assertThat(File("$rootPath/cacheDir/temp/voice/matrix.org/1234567890abcdefg").exists()) - .isTrue() - } - - @Test - fun `voiceCachePath() should point to cacheDir-temp-voice-mxcUri2fileName`() { - val rootPath = temporaryFolder.root.path - val cacheDir = File("$rootPath/cacheDir") - val mxcUri = "mxc://matrix.org/1234567890abcdefg" - val cache = VoiceMessageCacheImpl(cacheDir, mxcUri) - - Truth.assertThat(cache.cachePath) - .isEqualTo("$rootPath/cacheDir/temp/voice/matrix.org/1234567890abcdefg") - } - - @Test - fun `isInVoiceCache() should return true if the file exists`() { - val rootPath = temporaryFolder.root.path - val cacheDir = File("$rootPath/cacheDir") - val mxcUri = "mxc://matrix.org/1234567890abcdefg" - val file = File("$rootPath/cacheDir/temp/voice/matrix.org/1234567890abcdefg").apply { - parentFile?.mkdirs() - createNewFile() - } - val cache = VoiceMessageCacheImpl(cacheDir, mxcUri) - - Truth.assertThat(cache.isInCache()) - .isTrue() - } - - @Test - fun `isInVoiceCache() should return false if the file does not exist`() { - val rootPath = temporaryFolder.root.path - val cacheDir = File("$rootPath/cacheDir") - val mxcUri = "mxc://matrix.org/1234567890abcdefg" - val cache = VoiceMessageCacheImpl(cacheDir, mxcUri) - - Truth.assertThat(cache.isInCache()) - .isFalse() - } - - @Test(expected = IllegalStateException::class) - fun `isInVoiceCache() throws IllegalStateException on bogus mxc uri`() { - val cacheDir = File("") - val mxcUri = "bogus" - val cache = VoiceMessageCacheImpl(cacheDir, mxcUri) - - cache.isInCache() - } -} diff --git a/features/messages/impl/src/test/kotlin/io/element/android/features/messages/voicemessages/timeline/VoiceMessagePresenterTest.kt b/features/messages/impl/src/test/kotlin/io/element/android/features/messages/voicemessages/timeline/VoiceMessagePresenterTest.kt index c748679421..0a6ca3b8c4 100644 --- a/features/messages/impl/src/test/kotlin/io/element/android/features/messages/voicemessages/timeline/VoiceMessagePresenterTest.kt +++ b/features/messages/impl/src/test/kotlin/io/element/android/features/messages/voicemessages/timeline/VoiceMessagePresenterTest.kt @@ -22,23 +22,19 @@ import app.cash.turbine.test import com.google.common.truth.Truth import io.element.android.features.messages.impl.timeline.model.event.TimelineItemVoiceContent import io.element.android.features.messages.impl.timeline.model.event.aTimelineItemVoiceContent -import io.element.android.features.messages.mediaplayer.FakeMediaPlayer +import io.element.android.features.messages.impl.voicemessages.timeline.DefaultVoiceMessagePlayer import io.element.android.features.messages.impl.voicemessages.timeline.VoiceMessageEvents -import io.element.android.features.messages.impl.voicemessages.timeline.VoiceMessagePlayerImpl +import io.element.android.features.messages.impl.voicemessages.timeline.VoiceMessageMediaRepo import io.element.android.features.messages.impl.voicemessages.timeline.VoiceMessagePresenter import io.element.android.features.messages.impl.voicemessages.timeline.VoiceMessageState -import io.element.android.libraries.matrix.test.media.FakeMediaLoader +import io.element.android.features.messages.mediaplayer.FakeMediaPlayer import kotlinx.coroutines.test.runTest import org.junit.Test class VoiceMessagePresenterTest { - - private val fakeMediaLoader = FakeMediaLoader() - private val fakeVoiceCache = FakeVoiceMessageCache() - @Test fun `initial state has proper default values`() = runTest { - val presenter = createVoiceMessagePresenter(fakeMediaLoader, fakeVoiceCache) + val presenter = createVoiceMessagePresenter() moleculeFlow(RecompositionMode.Immediate) { presenter.present() }.test { @@ -51,12 +47,10 @@ class VoiceMessagePresenterTest { } @Test - fun `pressing play with file in cache plays`() = runTest { - fakeVoiceCache.apply { - givenIsInCache(true) - } - val content = aTimelineItemVoiceContent(durationMs = 2_000) - val presenter = createVoiceMessagePresenter(fakeMediaLoader, fakeVoiceCache, content) + fun `pressing play downloads and plays`() = runTest { + val presenter = createVoiceMessagePresenter( + content = aTimelineItemVoiceContent(durationMs = 2_000), + ) moleculeFlow(RecompositionMode.Immediate) { presenter.present() }.test { @@ -68,6 +62,11 @@ class VoiceMessagePresenterTest { initialState.eventSink(VoiceMessageEvents.PlayPause) + awaitItem().also { + Truth.assertThat(it.button).isEqualTo(VoiceMessageState.Button.Downloading) + Truth.assertThat(it.progress).isEqualTo(0f) + Truth.assertThat(it.time).isEqualTo("0:02") + } awaitItem().also { Truth.assertThat(it.button).isEqualTo(VoiceMessageState.Button.Pause) Truth.assertThat(it.progress).isEqualTo(0.5f) @@ -77,22 +76,18 @@ class VoiceMessagePresenterTest { } @Test - fun `pressing play with file not in cache downloads it but fails`() = runTest { - fakeMediaLoader.apply { - shouldFail = true - } - fakeVoiceCache.apply { - givenIsInCache(false) - givenMoveToCache(true) - } - val presenter = createVoiceMessagePresenter(fakeMediaLoader, fakeVoiceCache) + fun `pressing play downloads and fails`() = runTest { + val presenter = createVoiceMessagePresenter( + voiceMessageMediaRepo = FakeVoiceMessageMediaRepo().apply { shouldFail = true }, + content = aTimelineItemVoiceContent(durationMs = 2_000), + ) moleculeFlow(RecompositionMode.Immediate) { presenter.present() }.test { val initialState = awaitItem().also { Truth.assertThat(it.button).isEqualTo(VoiceMessageState.Button.Play) Truth.assertThat(it.progress).isEqualTo(0f) - Truth.assertThat(it.time).isEqualTo("1:01") + Truth.assertThat(it.time).isEqualTo("0:02") } initialState.eventSink(VoiceMessageEvents.PlayPause) @@ -100,59 +95,21 @@ class VoiceMessagePresenterTest { awaitItem().also { Truth.assertThat(it.button).isEqualTo(VoiceMessageState.Button.Downloading) Truth.assertThat(it.progress).isEqualTo(0f) - Truth.assertThat(it.time).isEqualTo("1:01") + Truth.assertThat(it.time).isEqualTo("0:02") } - awaitItem().also { Truth.assertThat(it.button).isEqualTo(VoiceMessageState.Button.Retry) Truth.assertThat(it.progress).isEqualTo(0f) - Truth.assertThat(it.time).isEqualTo("1:01") + Truth.assertThat(it.time).isEqualTo("0:02") } } } @Test - fun `pressing play with file not in cache downloads it but then caching fails`() = runTest { - fakeMediaLoader.apply { - shouldFail = false - } - fakeVoiceCache.apply { - givenIsInCache(false) - givenMoveToCache(false) - } - val presenter = createVoiceMessagePresenter(fakeMediaLoader, fakeVoiceCache) - moleculeFlow(RecompositionMode.Immediate) { - presenter.present() - }.test { - val initialState = awaitItem().also { - Truth.assertThat(it.button).isEqualTo(VoiceMessageState.Button.Play) - Truth.assertThat(it.progress).isEqualTo(0f) - Truth.assertThat(it.time).isEqualTo("1:01") - } - - initialState.eventSink(VoiceMessageEvents.PlayPause) - - awaitItem().also { - Truth.assertThat(it.button).isEqualTo(VoiceMessageState.Button.Downloading) - Truth.assertThat(it.progress).isEqualTo(0f) - Truth.assertThat(it.time).isEqualTo("1:01") - } - - awaitItem().also { - Truth.assertThat(it.button).isEqualTo(VoiceMessageState.Button.Retry) - Truth.assertThat(it.progress).isEqualTo(0f) - Truth.assertThat(it.time).isEqualTo("1:01") - } - } - } - - @Test - fun `acquire control then play then play and pause while having control`() = runTest { - fakeVoiceCache.apply { - givenIsInCache(true) - } - val content = aTimelineItemVoiceContent(durationMs = 2_000) - val presenter = createVoiceMessagePresenter(fakeMediaLoader, fakeVoiceCache, content) + fun `pressing pause while playing pauses`() = runTest { + val presenter = createVoiceMessagePresenter( + content = aTimelineItemVoiceContent(durationMs = 2_000), + ) moleculeFlow(RecompositionMode.Immediate) { presenter.present() }.test { @@ -163,78 +120,28 @@ class VoiceMessagePresenterTest { } initialState.eventSink(VoiceMessageEvents.PlayPause) + skipItems(1) // skip downloading state - awaitItem().also { + val playingState = awaitItem().also { Truth.assertThat(it.button).isEqualTo(VoiceMessageState.Button.Pause) Truth.assertThat(it.progress).isEqualTo(0.5f) Truth.assertThat(it.time).isEqualTo("0:01") } - initialState.eventSink(VoiceMessageEvents.PlayPause) - + playingState.eventSink(VoiceMessageEvents.PlayPause) awaitItem().also { Truth.assertThat(it.button).isEqualTo(VoiceMessageState.Button.Play) Truth.assertThat(it.progress).isEqualTo(0.5f) Truth.assertThat(it.time).isEqualTo("0:01") } - - initialState.eventSink(VoiceMessageEvents.PlayPause) - - awaitItem().also { - Truth.assertThat(it.button).isEqualTo(VoiceMessageState.Button.Pause) - Truth.assertThat(it.progress).isEqualTo(1.0f) - Truth.assertThat(it.time).isEqualTo("0:02") - } - } - } - - @Test - fun `pressing play with file not in cache downloads it successfully`() = runTest { - fakeMediaLoader.apply { - shouldFail = false - } - fakeVoiceCache.apply { - givenIsInCache(false) - givenMoveToCache(true) - } - val content = aTimelineItemVoiceContent(durationMs = 2_000) - val presenter = createVoiceMessagePresenter(fakeMediaLoader, fakeVoiceCache, content) - moleculeFlow(RecompositionMode.Immediate) { - presenter.present() - }.test { - val initialState = awaitItem().also { - Truth.assertThat(it.button).isEqualTo(VoiceMessageState.Button.Play) - Truth.assertThat(it.progress).isEqualTo(0f) - Truth.assertThat(it.time).isEqualTo("0:02") - } - - initialState.eventSink(VoiceMessageEvents.PlayPause) - - awaitItem().also { - Truth.assertThat(it.button).isEqualTo(VoiceMessageState.Button.Downloading) - Truth.assertThat(it.progress).isEqualTo(0f) - Truth.assertThat(it.time).isEqualTo("0:02") - } - - awaitItem().also { - Truth.assertThat(it.button).isEqualTo(VoiceMessageState.Button.Pause) - Truth.assertThat(it.progress).isEqualTo(0.5f) - Truth.assertThat(it.time).isEqualTo("0:01") - } } } @Test fun `content with null eventId shows disabled button`() = runTest { - fakeMediaLoader.apply { - shouldFail = false - } - fakeVoiceCache.apply { - givenIsInCache(false) - givenMoveToCache(true) - } - val content = aTimelineItemVoiceContent(eventId = null) - val presenter = createVoiceMessagePresenter(fakeMediaLoader, fakeVoiceCache, content) + val presenter = createVoiceMessagePresenter( + content = aTimelineItemVoiceContent(eventId = null), + ) moleculeFlow(RecompositionMode.Immediate) { presenter.present() }.test { @@ -248,12 +155,9 @@ class VoiceMessagePresenterTest { @Test fun `seeking seeks`() = runTest { - fakeVoiceCache.apply { - givenIsInCache(true) - } - val content = aTimelineItemVoiceContent(durationMs = 10_000) - - val presenter = createVoiceMessagePresenter(fakeMediaLoader, fakeVoiceCache, content) + val presenter = createVoiceMessagePresenter( + content = aTimelineItemVoiceContent(durationMs = 10_000), + ) moleculeFlow(RecompositionMode.Immediate) { presenter.present() }.test { @@ -265,6 +169,8 @@ class VoiceMessagePresenterTest { initialState.eventSink(VoiceMessageEvents.PlayPause) + skipItems(1) // skip downloading state + awaitItem().also { Truth.assertThat(it.button).isEqualTo(VoiceMessageState.Button.Pause) Truth.assertThat(it.progress).isEqualTo(0.1f) @@ -283,12 +189,18 @@ class VoiceMessagePresenterTest { } fun createVoiceMessagePresenter( - fakeMediaLoader: FakeMediaLoader, - voiceCacheFake: FakeVoiceMessageCache, + voiceMessageMediaRepo: VoiceMessageMediaRepo = FakeVoiceMessageMediaRepo(), content: TimelineItemVoiceContent = aTimelineItemVoiceContent(), ) = VoiceMessagePresenter( - mediaLoader = fakeMediaLoader, - voiceMessagePlayerFactory = { eventId, mediaPath -> VoiceMessagePlayerImpl(FakeMediaPlayer(), eventId, mediaPath) }, - voiceMessageCacheFactory = { voiceCacheFake }, + voiceMessagePlayerFactory = { eventId, mediaSource, mimeType, body -> + DefaultVoiceMessagePlayer( + mediaPlayer = FakeMediaPlayer(), + voiceMessageMediaRepoFactory = { _, _, _ -> voiceMessageMediaRepo }, + eventId = eventId, + mediaSource = mediaSource, + mimeType = mimeType, + body = body + ) + }, content = content, ) diff --git a/libraries/matrix/test/src/main/kotlin/io/element/android/libraries/matrix/test/media/FakeMediaLoader.kt b/libraries/matrix/test/src/main/kotlin/io/element/android/libraries/matrix/test/media/FakeMediaLoader.kt index 9ef0413a3a..8ceb878adf 100644 --- a/libraries/matrix/test/src/main/kotlin/io/element/android/libraries/matrix/test/media/FakeMediaLoader.kt +++ b/libraries/matrix/test/src/main/kotlin/io/element/android/libraries/matrix/test/media/FakeMediaLoader.kt @@ -24,6 +24,7 @@ import io.element.android.tests.testutils.simulateLongTask class FakeMediaLoader : MatrixMediaLoader { var shouldFail = false + var path: String = "" override suspend fun loadMediaContent(source: MediaSource): Result = simulateLongTask { if (shouldFail) { @@ -45,7 +46,7 @@ class FakeMediaLoader : MatrixMediaLoader { if (shouldFail) { Result.failure(RuntimeException()) } else { - Result.success(FakeMediaFile("")) + Result.success(FakeMediaFile(path)) } } }