diff --git a/changelog.d/769.feature b/changelog.d/769.feature new file mode 100644 index 0000000000..8df765c27c --- /dev/null +++ b/changelog.d/769.feature @@ -0,0 +1 @@ +Allow cancelling media upload diff --git a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/MessagesView.kt b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/MessagesView.kt index b007d59e36..145813fe54 100644 --- a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/MessagesView.kt +++ b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/MessagesView.kt @@ -52,6 +52,7 @@ import io.element.android.features.messages.impl.actionlist.ActionListView import io.element.android.features.messages.impl.actionlist.model.TimelineItemAction import io.element.android.features.messages.impl.attachments.Attachment import io.element.android.features.messages.impl.messagecomposer.AttachmentsState +import io.element.android.features.messages.impl.messagecomposer.MessageComposerEvents import io.element.android.features.messages.impl.messagecomposer.MessageComposerView import io.element.android.features.messages.impl.timeline.TimelineView import io.element.android.features.messages.impl.timeline.components.customreaction.CustomReactionBottomSheet @@ -100,7 +101,11 @@ fun MessagesView( ) { LogCompositions(tag = "MessagesScreen", msg = "Root") - AttachmentStateView(state.composerState.attachmentsState, onPreviewAttachments) + AttachmentStateView( + state = state.composerState.attachmentsState, + onPreviewAttachments = onPreviewAttachments, + onCancel = { state.composerState.eventSink(MessageComposerEvents.CancelSendAttachment) }, + ) val snackbarHostState = rememberSnackbarHostState(snackbarMessage = state.snackbarMessage) @@ -229,7 +234,8 @@ private fun ReinviteDialog(state: MessagesState) { @Composable private fun AttachmentStateView( state: AttachmentsState, - onPreviewAttachments: (ImmutableList) -> Unit + onPreviewAttachments: (ImmutableList) -> Unit, + onCancel: () -> Unit, ) { when (state) { AttachmentsState.None -> Unit @@ -242,7 +248,9 @@ private fun AttachmentStateView( is AttachmentsState.Sending.Uploading -> ProgressDialogType.Determinate(state.progress) is AttachmentsState.Sending.Processing -> ProgressDialogType.Indeterminate }, - text = stringResource(id = CommonStrings.common_sending) + text = stringResource(id = CommonStrings.common_sending), + isCancellable = true, + onDismissRequest = onCancel, ) } } diff --git a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/attachments/preview/AttachmentsPreviewPresenter.kt b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/attachments/preview/AttachmentsPreviewPresenter.kt index 3ee87c0bc8..983d016854 100644 --- a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/attachments/preview/AttachmentsPreviewPresenter.kt +++ b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/attachments/preview/AttachmentsPreviewPresenter.kt @@ -28,8 +28,12 @@ import io.element.android.features.messages.impl.attachments.Attachment import io.element.android.libraries.architecture.Presenter import io.element.android.libraries.matrix.api.core.ProgressCallback import io.element.android.libraries.mediaupload.api.MediaSender +import kotlinx.coroutines.CancellationException import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.Job +import kotlinx.coroutines.isActive import kotlinx.coroutines.launch +import kotlin.coroutines.coroutineContext class AttachmentsPreviewPresenter @AssistedInject constructor( @Assisted private val attachment: Attachment, @@ -50,10 +54,18 @@ class AttachmentsPreviewPresenter @AssistedInject constructor( mutableStateOf(SendActionState.Idle) } + val ongoingSendAttachmentJob = remember { mutableStateOf(null) } + fun handleEvents(attachmentsPreviewEvents: AttachmentsPreviewEvents) { when (attachmentsPreviewEvents) { - AttachmentsPreviewEvents.SendAttachment -> coroutineScope.sendAttachment(attachment, sendActionState) - AttachmentsPreviewEvents.ClearSendState -> sendActionState.value = SendActionState.Idle + AttachmentsPreviewEvents.SendAttachment -> ongoingSendAttachmentJob.value = coroutineScope.sendAttachment(attachment, sendActionState) + AttachmentsPreviewEvents.ClearSendState -> { + ongoingSendAttachmentJob.value?.let { + it.cancel() + ongoingSendAttachmentJob.value = null + } + sendActionState.value = SendActionState.Idle + } } } @@ -72,7 +84,7 @@ class AttachmentsPreviewPresenter @AssistedInject constructor( is Attachment.Media -> { sendMedia( mediaAttachment = attachment, - sendActionState = sendActionState + sendActionState = sendActionState, ) } } @@ -81,10 +93,13 @@ class AttachmentsPreviewPresenter @AssistedInject constructor( private suspend fun sendMedia( mediaAttachment: Attachment.Media, sendActionState: MutableState, - ) { + ) = runCatching { + val context = coroutineContext val progressCallback = object : ProgressCallback { override fun onProgress(current: Long, total: Long) { - sendActionState.value = SendActionState.Sending.Uploading(current.toFloat() / total.toFloat()) + if (context.isActive) { + sendActionState.value = SendActionState.Sending.Uploading(current.toFloat() / total.toFloat()) + } } } sendActionState.value = SendActionState.Sending.Processing @@ -93,13 +108,17 @@ class AttachmentsPreviewPresenter @AssistedInject constructor( mimeType = mediaAttachment.localMedia.info.mimeType, compressIfPossible = mediaAttachment.compressIfPossible, progressCallback = progressCallback - ).fold( - onSuccess = { - sendActionState.value = SendActionState.Done - }, - onFailure = { - sendActionState.value = SendActionState.Failure(it) + ).getOrThrow() + }.fold( + onSuccess = { + sendActionState.value = SendActionState.Done + }, + onFailure = { error -> + if (error is CancellationException) { + throw error + } else { + sendActionState.value = SendActionState.Failure(error) } - ) - } + } + ) } diff --git a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/attachments/preview/AttachmentsPreviewView.kt b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/attachments/preview/AttachmentsPreviewView.kt index c696098fed..eed7606ceb 100644 --- a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/attachments/preview/AttachmentsPreviewView.kt +++ b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/attachments/preview/AttachmentsPreviewView.kt @@ -96,7 +96,9 @@ private fun AttachmentSendStateView( is SendActionState.Sending.Uploading -> ProgressDialogType.Determinate(sendActionState.progress) SendActionState.Sending.Processing -> ProgressDialogType.Indeterminate }, - text = stringResource(id = CommonStrings.common_sending) + text = stringResource(id = CommonStrings.common_sending), + isCancellable = true, + onDismissRequest = onDismissClicked, ) } is SendActionState.Failure -> { diff --git a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/messagecomposer/MessageComposerEvents.kt b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/messagecomposer/MessageComposerEvents.kt index 46e57e92de..d040c503b1 100644 --- a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/messagecomposer/MessageComposerEvents.kt +++ b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/messagecomposer/MessageComposerEvents.kt @@ -36,4 +36,5 @@ sealed interface MessageComposerEvents { object VideoFromCamera : PickAttachmentSource object Location : PickAttachmentSource } + object CancelSendAttachment : MessageComposerEvents } diff --git a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/messagecomposer/MessageComposerPresenter.kt b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/messagecomposer/MessageComposerPresenter.kt index 934a67f2e4..5477b10c63 100644 --- a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/messagecomposer/MessageComposerPresenter.kt +++ b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/messagecomposer/MessageComposerPresenter.kt @@ -47,9 +47,13 @@ import io.element.android.libraries.mediaupload.api.MediaSender import io.element.android.libraries.textcomposer.MessageComposerMode import io.element.android.services.analytics.api.AnalyticsService import kotlinx.collections.immutable.persistentListOf +import kotlinx.coroutines.CancellationException import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.Job +import kotlinx.coroutines.isActive import kotlinx.coroutines.launch import javax.inject.Inject +import kotlin.coroutines.coroutineContext import io.element.android.libraries.core.mimetype.MimeTypes.Any as AnyMimeTypes @SingleIn(RoomScope::class) @@ -100,6 +104,7 @@ class MessageComposerPresenter @Inject constructor( val text: MutableState = rememberSaveable { mutableStateOf("") } + val ongoingSendAttachmentJob = remember { mutableStateOf(null) } var showAttachmentSourcePicker: Boolean by remember { mutableStateOf(false) } @@ -112,7 +117,12 @@ class MessageComposerPresenter @Inject constructor( LaunchedEffect(attachmentsState.value) { when (val attachmentStateValue = attachmentsState.value) { - is AttachmentsState.Sending.Processing -> localCoroutineScope.sendAttachment(attachmentStateValue.attachments.first(), attachmentsState) + is AttachmentsState.Sending.Processing -> { + ongoingSendAttachmentJob.value = localCoroutineScope.sendAttachment( + attachmentStateValue.attachments.first(), + attachmentsState, + ) + } else -> Unit } } @@ -169,6 +179,12 @@ class MessageComposerPresenter @Inject constructor( showAttachmentSourcePicker = false // Navigation to the location picker screen is done at the view layer } + is MessageComposerEvents.CancelSendAttachment -> { + ongoingSendAttachmentJob.value?.let { + it.cancel() + ongoingSendAttachmentJob.value == null + } + } } } @@ -212,13 +228,13 @@ class MessageComposerPresenter @Inject constructor( private fun CoroutineScope.sendAttachment( attachment: Attachment, attachmentState: MutableState, - ) = launch { - when (attachment) { - is Attachment.Media -> { + ) = when (attachment) { + is Attachment.Media -> { + launch { sendMedia( uri = attachment.localMedia.uri, mimeType = attachment.localMedia.info.mimeType, - attachmentState = attachmentState + attachmentState = attachmentState, ) } } @@ -259,20 +275,27 @@ class MessageComposerPresenter @Inject constructor( uri: Uri, mimeType: String, attachmentState: MutableState, - ) { + ) = runCatching { + val context = coroutineContext val progressCallback = object : ProgressCallback { override fun onProgress(current: Long, total: Long) { - attachmentState.value = AttachmentsState.Sending.Uploading(current.toFloat() / total.toFloat()) + if (context.isActive) { + attachmentState.value = AttachmentsState.Sending.Uploading(current.toFloat() / total.toFloat()) + } } } - mediaSender.sendMedia(uri, mimeType, compressIfPossible = false, progressCallback) - .onSuccess { - attachmentState.value = AttachmentsState.None - } - .onFailure { - val snackbarMessage = SnackbarMessage(sendAttachmentError(it)) - snackbarDispatcher.post(snackbarMessage) - attachmentState.value = AttachmentsState.None - } + mediaSender.sendMedia(uri, mimeType, compressIfPossible = false, progressCallback).getOrThrow() + } + .onSuccess { + attachmentState.value = AttachmentsState.None + } + .onFailure { cause -> + attachmentState.value = AttachmentsState.None + if (cause is CancellationException) { + throw cause + } else { + val snackbarMessage = SnackbarMessage(sendAttachmentError(cause)) + snackbarDispatcher.post(snackbarMessage) + } } } diff --git a/features/messages/impl/src/test/kotlin/io/element/android/features/messages/attachments/AttachmentsPreviewPresenterTest.kt b/features/messages/impl/src/test/kotlin/io/element/android/features/messages/attachments/AttachmentsPreviewPresenterTest.kt index fb8b3b1948..3608d9e80e 100644 --- a/features/messages/impl/src/test/kotlin/io/element/android/features/messages/attachments/AttachmentsPreviewPresenterTest.kt +++ b/features/messages/impl/src/test/kotlin/io/element/android/features/messages/attachments/AttachmentsPreviewPresenterTest.kt @@ -94,6 +94,21 @@ class AttachmentsPreviewPresenterTest { } } + @Test + fun `present - dismissing the progress dialog stops media upload`() = runTest { + val presenter = anAttachmentsPreviewPresenter() + moleculeFlow(RecompositionMode.Immediate) { + presenter.present() + }.test { + val initialState = awaitItem() + assertThat(initialState.sendActionState).isEqualTo(SendActionState.Idle) + initialState.eventSink(AttachmentsPreviewEvents.SendAttachment) + assertThat(awaitItem().sendActionState).isEqualTo(SendActionState.Sending.Processing) + initialState.eventSink(AttachmentsPreviewEvents.ClearSendState) + assertThat(awaitItem().sendActionState).isEqualTo(SendActionState.Idle) + } + } + private fun anAttachmentsPreviewPresenter( localMedia: LocalMedia = aLocalMedia( uri = mockMediaUrl, diff --git a/features/messages/impl/src/test/kotlin/io/element/android/features/messages/textcomposer/MessageComposerPresenterTest.kt b/features/messages/impl/src/test/kotlin/io/element/android/features/messages/textcomposer/MessageComposerPresenterTest.kt index 5c75c963e5..25b506eb7f 100644 --- a/features/messages/impl/src/test/kotlin/io/element/android/features/messages/textcomposer/MessageComposerPresenterTest.kt +++ b/features/messages/impl/src/test/kotlin/io/element/android/features/messages/textcomposer/MessageComposerPresenterTest.kt @@ -500,6 +500,23 @@ class MessageComposerPresenterTest { } } + @Test + fun `present - CancelSendAttachment stops media upload`() = runTest { + val presenter = createPresenter(this) + moleculeFlow(RecompositionMode.Immediate) { + presenter.present() + }.test { + skipItems(1) + val initialState = awaitItem() + initialState.eventSink(MessageComposerEvents.PickAttachmentSource.FromFiles) + val sendingState = awaitItem() + assertThat(sendingState.showAttachmentSourcePicker).isFalse() + assertThat(sendingState.attachmentsState).isInstanceOf(AttachmentsState.Sending.Processing::class.java) + sendingState.eventSink(MessageComposerEvents.CancelSendAttachment) + assertThat(awaitItem().attachmentsState).isEqualTo(AttachmentsState.None) + } + } + private suspend fun ReceiveTurbine.backToNormalMode(state: MessageComposerState, skipCount: Int = 0) { state.eventSink.invoke(MessageComposerEvents.CloseSpecialMode) skipItems(skipCount) diff --git a/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/media/MediaUploadHandler.kt b/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/media/MediaUploadHandler.kt new file mode 100644 index 0000000000..17d204715e --- /dev/null +++ b/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/media/MediaUploadHandler.kt @@ -0,0 +1,28 @@ +/* + * 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.libraries.matrix.api.media + +/** + * This is an abstraction over the Rust SDK's `SendAttachmentJoinHandle` which allows us to either [await] the upload process or [cancel] it. + */ +interface MediaUploadHandler { + /** Await the upload process to finish. */ + suspend fun await(): Result + + /** Cancel the upload process. */ + fun cancel() +} diff --git a/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/room/MatrixRoom.kt b/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/room/MatrixRoom.kt index f8b9e6c2c3..f2c51c357e 100644 --- a/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/room/MatrixRoom.kt +++ b/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/room/MatrixRoom.kt @@ -25,6 +25,7 @@ import io.element.android.libraries.matrix.api.core.UserId import io.element.android.libraries.matrix.api.media.AudioInfo import io.element.android.libraries.matrix.api.media.FileInfo import io.element.android.libraries.matrix.api.media.ImageInfo +import io.element.android.libraries.matrix.api.media.MediaUploadHandler import io.element.android.libraries.matrix.api.media.VideoInfo import io.element.android.libraries.matrix.api.room.location.AssetType import io.element.android.libraries.matrix.api.timeline.MatrixTimeline @@ -81,13 +82,13 @@ interface MatrixRoom : Closeable { suspend fun redactEvent(eventId: EventId, reason: String? = null): Result - suspend fun sendImage(file: File, thumbnailFile: File, imageInfo: ImageInfo, progressCallback: ProgressCallback?): Result + suspend fun sendImage(file: File, thumbnailFile: File, imageInfo: ImageInfo, progressCallback: ProgressCallback?): Result - suspend fun sendVideo(file: File, thumbnailFile: File, videoInfo: VideoInfo, progressCallback: ProgressCallback?): Result + suspend fun sendVideo(file: File, thumbnailFile: File, videoInfo: VideoInfo, progressCallback: ProgressCallback?): Result - suspend fun sendAudio(file: File, audioInfo: AudioInfo, progressCallback: ProgressCallback?): Result + suspend fun sendAudio(file: File, audioInfo: AudioInfo, progressCallback: ProgressCallback?): Result - suspend fun sendFile(file: File, fileInfo: FileInfo, progressCallback: ProgressCallback?): Result + suspend fun sendFile(file: File, fileInfo: FileInfo, progressCallback: ProgressCallback?): Result suspend fun toggleReaction(emoji: String, eventId: EventId): Result diff --git a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/media/MediaUploadHandlerImpl.kt b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/media/MediaUploadHandlerImpl.kt new file mode 100644 index 0000000000..639f9149c4 --- /dev/null +++ b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/media/MediaUploadHandlerImpl.kt @@ -0,0 +1,42 @@ +/* + * 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.libraries.matrix.impl.media + +import io.element.android.libraries.androidutils.file.safeDelete +import io.element.android.libraries.matrix.api.media.MediaUploadHandler +import org.matrix.rustcomponents.sdk.SendAttachmentJoinHandle +import java.io.File + +class MediaUploadHandlerImpl( + private val filesToUpload: List, + private val sendAttachmentJoinHandle: SendAttachmentJoinHandle, +) : MediaUploadHandler { + override suspend fun await(): Result = + runCatching { + sendAttachmentJoinHandle.join() + } + .also { cleanUpFiles() } + + override fun cancel() { + sendAttachmentJoinHandle.cancel() + cleanUpFiles() + } + + private fun cleanUpFiles() { + filesToUpload.forEach { file -> file.safeDelete() } + } +} diff --git a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/room/RustMatrixRoom.kt b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/room/RustMatrixRoom.kt index dc74a273f4..373880568f 100644 --- a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/room/RustMatrixRoom.kt +++ b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/room/RustMatrixRoom.kt @@ -28,6 +28,7 @@ import io.element.android.libraries.matrix.api.core.UserId import io.element.android.libraries.matrix.api.media.AudioInfo import io.element.android.libraries.matrix.api.media.FileInfo import io.element.android.libraries.matrix.api.media.ImageInfo +import io.element.android.libraries.matrix.api.media.MediaUploadHandler import io.element.android.libraries.matrix.api.media.VideoInfo import io.element.android.libraries.matrix.api.room.MatrixRoom import io.element.android.libraries.matrix.api.room.MatrixRoomMembersState @@ -38,6 +39,7 @@ import io.element.android.libraries.matrix.api.room.roomMembers import io.element.android.libraries.matrix.api.timeline.MatrixTimeline import io.element.android.libraries.matrix.api.timeline.item.event.EventType import io.element.android.libraries.matrix.impl.core.toProgressWatcher +import io.element.android.libraries.matrix.impl.media.MediaUploadHandlerImpl import io.element.android.libraries.matrix.impl.media.map import io.element.android.libraries.matrix.impl.room.location.toInner import io.element.android.libraries.matrix.impl.timeline.RustMatrixTimeline @@ -268,26 +270,26 @@ class RustMatrixRoom( } } - override suspend fun sendImage(file: File, thumbnailFile: File, imageInfo: ImageInfo, progressCallback: ProgressCallback?): Result { - return sendAttachment { + override suspend fun sendImage(file: File, thumbnailFile: File, imageInfo: ImageInfo, progressCallback: ProgressCallback?): Result { + return sendAttachment(listOf(file, thumbnailFile)) { innerRoom.sendImage(file.path, thumbnailFile.path, imageInfo.map(), progressCallback?.toProgressWatcher()) } } - override suspend fun sendVideo(file: File, thumbnailFile: File, videoInfo: VideoInfo, progressCallback: ProgressCallback?): Result { - return sendAttachment { + override suspend fun sendVideo(file: File, thumbnailFile: File, videoInfo: VideoInfo, progressCallback: ProgressCallback?): Result { + return sendAttachment(listOf(file, thumbnailFile)) { innerRoom.sendVideo(file.path, thumbnailFile.path, videoInfo.map(), progressCallback?.toProgressWatcher()) } } - override suspend fun sendAudio(file: File, audioInfo: AudioInfo, progressCallback: ProgressCallback?): Result { - return sendAttachment { + override suspend fun sendAudio(file: File, audioInfo: AudioInfo, progressCallback: ProgressCallback?): Result { + return sendAttachment(listOf(file)) { innerRoom.sendAudio(file.path, audioInfo.map(), progressCallback?.toProgressWatcher()) } } - override suspend fun sendFile(file: File, fileInfo: FileInfo, progressCallback: ProgressCallback?): Result { - return sendAttachment { + override suspend fun sendFile(file: File, fileInfo: FileInfo, progressCallback: ProgressCallback?): Result { + return sendAttachment(listOf(file)) { innerRoom.sendFile(file.path, fileInfo.map(), progressCallback?.toProgressWatcher()) } } @@ -371,14 +373,9 @@ class RustMatrixRoom( } } - //TODO handle cancellation, need refactoring of how we are catching errors - private suspend fun sendAttachment(handle: () -> SendAttachmentJoinHandle): Result { + private suspend fun sendAttachment(files: List, handle: () -> SendAttachmentJoinHandle): Result { return runCatching { - handle().use { - it.join() - } + MediaUploadHandlerImpl(files, handle()) } } } - - diff --git a/libraries/matrix/test/src/main/kotlin/io/element/android/libraries/matrix/test/media/FakeMediaUploadHandler.kt b/libraries/matrix/test/src/main/kotlin/io/element/android/libraries/matrix/test/media/FakeMediaUploadHandler.kt new file mode 100644 index 0000000000..100dbd5f66 --- /dev/null +++ b/libraries/matrix/test/src/main/kotlin/io/element/android/libraries/matrix/test/media/FakeMediaUploadHandler.kt @@ -0,0 +1,31 @@ +/* + * 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.libraries.matrix.test.media + +import io.element.android.libraries.matrix.api.media.MediaUploadHandler +import io.element.android.tests.testutils.simulateLongTask +import kotlin.coroutines.cancellation.CancellationException + +class FakeMediaUploadHandler( + private var result: Result = Result.success(Unit), +) : MediaUploadHandler { + override suspend fun await(): Result = simulateLongTask { result } + + override fun cancel() { + result = Result.failure(CancellationException()) + } +} diff --git a/libraries/matrix/test/src/main/kotlin/io/element/android/libraries/matrix/test/room/FakeMatrixRoom.kt b/libraries/matrix/test/src/main/kotlin/io/element/android/libraries/matrix/test/room/FakeMatrixRoom.kt index ee735ae81b..a660c56a99 100644 --- a/libraries/matrix/test/src/main/kotlin/io/element/android/libraries/matrix/test/room/FakeMatrixRoom.kt +++ b/libraries/matrix/test/src/main/kotlin/io/element/android/libraries/matrix/test/room/FakeMatrixRoom.kt @@ -25,6 +25,7 @@ import io.element.android.libraries.matrix.api.core.UserId import io.element.android.libraries.matrix.api.media.AudioInfo import io.element.android.libraries.matrix.api.media.FileInfo import io.element.android.libraries.matrix.api.media.ImageInfo +import io.element.android.libraries.matrix.api.media.MediaUploadHandler import io.element.android.libraries.matrix.api.media.VideoInfo import io.element.android.libraries.matrix.api.room.MatrixRoom import io.element.android.libraries.matrix.api.room.MatrixRoomMembersState @@ -34,6 +35,7 @@ import io.element.android.libraries.matrix.api.room.location.AssetType import io.element.android.libraries.matrix.api.timeline.MatrixTimeline import io.element.android.libraries.matrix.test.A_ROOM_ID import io.element.android.libraries.matrix.test.A_SESSION_ID +import io.element.android.libraries.matrix.test.media.FakeMediaUploadHandler import io.element.android.libraries.matrix.test.timeline.FakeMatrixTimeline import io.element.android.tests.testutils.simulateLongTask import kotlinx.coroutines.delay @@ -70,7 +72,7 @@ class FakeMatrixRoom( private var canRedactResult = Result.success(canRedact) private val canSendStateResults = mutableMapOf>() private val canSendEventResults = mutableMapOf>() - private var sendMediaResult = Result.success(Unit) + private var sendMediaResult = Result.success(FakeMediaUploadHandler()) private var setNameResult = Result.success(Unit) private var setTopicResult = Result.success(Unit) private var updateAvatarResult = Result.success(Unit) @@ -226,21 +228,34 @@ class FakeMatrixRoom( thumbnailFile: File, imageInfo: ImageInfo, progressCallback: ProgressCallback? - ): Result = fakeSendMedia(progressCallback) + ): Result = fakeSendMedia(progressCallback) - override suspend fun sendVideo(file: File, thumbnailFile: File, videoInfo: VideoInfo, progressCallback: ProgressCallback?): Result = fakeSendMedia( + override suspend fun sendVideo( + file: File, + thumbnailFile: File, + videoInfo: VideoInfo, + progressCallback: ProgressCallback? + ): Result = fakeSendMedia( progressCallback ) - override suspend fun sendAudio(file: File, audioInfo: AudioInfo, progressCallback: ProgressCallback?): Result = fakeSendMedia(progressCallback) + override suspend fun sendAudio( + file: File, + audioInfo: AudioInfo, + progressCallback: ProgressCallback? + ): Result = fakeSendMedia(progressCallback) - override suspend fun sendFile(file: File, fileInfo: FileInfo, progressCallback: ProgressCallback?): Result = fakeSendMedia(progressCallback) + override suspend fun sendFile( + file: File, + fileInfo: FileInfo, + progressCallback: ProgressCallback? + ): Result = fakeSendMedia(progressCallback) override suspend fun forwardEvent(eventId: EventId, roomIds: List): Result = simulateLongTask { forwardEventResult } - private suspend fun fakeSendMedia(progressCallback: ProgressCallback?): Result = simulateLongTask { + private suspend fun fakeSendMedia(progressCallback: ProgressCallback?): Result = simulateLongTask { sendMediaResult.onSuccess { progressCallbackValues.forEach { (current, total) -> progressCallback?.onProgress(current, total) @@ -338,7 +353,7 @@ class FakeMatrixRoom( unignoreResult = result } - fun givenSendMediaResult(result: Result) { + fun givenSendMediaResult(result: Result) { sendMediaResult = result } diff --git a/libraries/mediaupload/api/build.gradle.kts b/libraries/mediaupload/api/build.gradle.kts index 111abc2bcc..c1e501d02a 100644 --- a/libraries/mediaupload/api/build.gradle.kts +++ b/libraries/mediaupload/api/build.gradle.kts @@ -38,5 +38,12 @@ android { api(projects.libraries.matrix.api) implementation(libs.inject) implementation(libs.coroutines.core) + + testImplementation(projects.libraries.matrix.test) + testImplementation(projects.libraries.mediaupload.test) + testImplementation(libs.test.junit) + testImplementation(libs.test.truth) + testImplementation(libs.coroutines.test) + testImplementation(libs.test.robolectric) } } diff --git a/libraries/mediaupload/api/src/main/kotlin/io/element/android/libraries/mediaupload/api/MediaSender.kt b/libraries/mediaupload/api/src/main/kotlin/io/element/android/libraries/mediaupload/api/MediaSender.kt index 1622ab2eef..899e92efc5 100644 --- a/libraries/mediaupload/api/src/main/kotlin/io/element/android/libraries/mediaupload/api/MediaSender.kt +++ b/libraries/mediaupload/api/src/main/kotlin/io/element/android/libraries/mediaupload/api/MediaSender.kt @@ -17,9 +17,13 @@ package io.element.android.libraries.mediaupload.api import android.net.Uri -import io.element.android.libraries.core.extensions.flatMap +import io.element.android.libraries.core.extensions.flatMapCatching import io.element.android.libraries.matrix.api.core.ProgressCallback +import io.element.android.libraries.matrix.api.media.MediaUploadHandler import io.element.android.libraries.matrix.api.room.MatrixRoom +import kotlinx.coroutines.CancellationException +import kotlinx.coroutines.Job +import java.util.concurrent.ConcurrentHashMap import javax.inject.Inject class MediaSender @Inject constructor( @@ -27,6 +31,9 @@ class MediaSender @Inject constructor( private val room: MatrixRoom, ) { + private val ongoingUploadJobs = ConcurrentHashMap() + val hasOngoingMediaUploads get() = ongoingUploadJobs.isNotEmpty() + suspend fun sendMedia( uri: Uri, mimeType: String, @@ -40,16 +47,25 @@ class MediaSender @Inject constructor( deleteOriginal = true, compressIfPossible = compressIfPossible ) - .flatMap { info -> + .flatMapCatching { info -> room.sendMedia(info, progressCallback) } + .onFailure { error -> + val job = ongoingUploadJobs.remove(Job) + if (error !is CancellationException) { + job?.cancel() + } + } + .onSuccess { + ongoingUploadJobs.remove(Job) + } } private suspend fun MatrixRoom.sendMedia( uploadInfo: MediaUploadInfo, - progressCallback: ProgressCallback? + progressCallback: ProgressCallback?, ): Result { - return when (uploadInfo) { + val handler = when (uploadInfo) { is MediaUploadInfo.Image -> { sendImage( file = uploadInfo.file, @@ -83,5 +99,11 @@ class MediaSender @Inject constructor( ) } } + + return handler + .flatMapCatching { uploadHandler -> + ongoingUploadJobs[Job] = uploadHandler + uploadHandler.await() + } } } diff --git a/libraries/mediaupload/api/src/test/kotlin/io/element/android/libraries/mediaupload/api/MediaSenderTests.kt b/libraries/mediaupload/api/src/test/kotlin/io/element/android/libraries/mediaupload/api/MediaSenderTests.kt new file mode 100644 index 0000000000..480cf8065f --- /dev/null +++ b/libraries/mediaupload/api/src/test/kotlin/io/element/android/libraries/mediaupload/api/MediaSenderTests.kt @@ -0,0 +1,116 @@ +/* + * 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.libraries.mediaupload.api + +import android.net.Uri +import com.google.common.truth.Truth.assertThat +import io.element.android.libraries.matrix.api.room.MatrixRoom +import io.element.android.libraries.matrix.test.room.FakeMatrixRoom +import io.element.android.libraries.mediaupload.test.FakeMediaPreProcessor +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.launch +import kotlinx.coroutines.test.StandardTestDispatcher +import kotlinx.coroutines.test.advanceTimeBy +import kotlinx.coroutines.test.runTest +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner + +@RunWith(RobolectricTestRunner::class) +class MediaSenderTests { + + @Test + fun `given an attachment when sending it the preprocessor always runs`() = runTest { + val preProcessor = FakeMediaPreProcessor() + val sender = aMediaSender(preProcessor) + + val uri = Uri.parse("content://image.jpg") + sender.sendMedia(uri = uri, mimeType = "image/jpeg", compressIfPossible = true) + + assertThat(preProcessor.processCallCount).isEqualTo(1) + } + + @Test + fun `given an attachment when sending it the MatrixRoom will call sendMedia`() = runTest { + val room = FakeMatrixRoom() + val sender = aMediaSender(room = room) + + val uri = Uri.parse("content://image.jpg") + sender.sendMedia(uri = uri, mimeType = "image/jpeg", compressIfPossible = true) + + assertThat(room.sendMediaCount).isEqualTo(1) + } + + @Test + fun `given a failure in the preprocessor when sending the whole process fails`() = runTest { + val preProcessor = FakeMediaPreProcessor().apply { + givenResult(Result.failure(Exception())) + } + val sender = aMediaSender(preProcessor) + + val uri = Uri.parse("content://image.jpg") + val result = sender.sendMedia(uri = uri, mimeType = "image/jpeg", compressIfPossible = true) + + assertThat(result.exceptionOrNull()).isNotNull() + } + + @Test + fun `given a failure in the media upload when sending the whole process fails`() = runTest { + val room = FakeMatrixRoom().apply { + givenSendMediaResult(Result.failure(Exception())) + } + val sender = aMediaSender(room = room) + + val uri = Uri.parse("content://image.jpg") + val result = sender.sendMedia(uri = uri, mimeType = "image/jpeg", compressIfPossible = true) + + assertThat(result.exceptionOrNull()).isNotNull() + } + + @OptIn(ExperimentalCoroutinesApi::class) + @Test + fun `given a cancellation in the media upload when sending the job is cancelled`() = runTest(StandardTestDispatcher()) { + val room = FakeMatrixRoom() + val sender = aMediaSender(room = room) + val sendJob = launch { + val uri = Uri.parse("content://image.jpg") + sender.sendMedia(uri = uri, mimeType = "image/jpeg", compressIfPossible = true) + } + // Wait until several internal tasks run and the file is being uploaded + advanceTimeBy(3L) + + // Assert the file is being uploaded + assertThat(sender.hasOngoingMediaUploads).isTrue() + + // Cancel the coroutine + sendJob.cancel() + + // Wait for the coroutine cleanup to happen + advanceTimeBy(1L) + + // Assert the file is not being uploaded anymore + assertThat(sender.hasOngoingMediaUploads).isFalse() + } + + private fun aMediaSender( + preProcessor: MediaPreProcessor = FakeMediaPreProcessor(), + room: MatrixRoom = FakeMatrixRoom(), + ) = MediaSender( + preProcessor, + room, + ) +} diff --git a/libraries/mediaupload/test/src/main/kotlin/io/element/android/libraries/mediaupload/test/FakeMediaPreProcessor.kt b/libraries/mediaupload/test/src/main/kotlin/io/element/android/libraries/mediaupload/test/FakeMediaPreProcessor.kt index 0cc7803578..d94414d2d7 100644 --- a/libraries/mediaupload/test/src/main/kotlin/io/element/android/libraries/mediaupload/test/FakeMediaPreProcessor.kt +++ b/libraries/mediaupload/test/src/main/kotlin/io/element/android/libraries/mediaupload/test/FakeMediaPreProcessor.kt @@ -25,6 +25,9 @@ import java.io.File class FakeMediaPreProcessor : MediaPreProcessor { + var processCallCount = 0 + private set + private var result: Result = Result.success( MediaUploadInfo.AnyFile( File("test"), @@ -43,6 +46,7 @@ class FakeMediaPreProcessor : MediaPreProcessor { deleteOriginal: Boolean, compressIfPossible: Boolean ): Result = simulateLongTask { + processCallCount++ result } diff --git a/tests/uitests/src/test/snapshots/images/io.element.android.tests.uitests_ScreenshotTest_preview_tests[io.element.android.features.messages.impl.attachments.preview_null_DefaultGroup_AttachmentsPreviewViewDarkPreview_0_null_2,NEXUS_5,1.0,en].png b/tests/uitests/src/test/snapshots/images/io.element.android.tests.uitests_ScreenshotTest_preview_tests[io.element.android.features.messages.impl.attachments.preview_null_DefaultGroup_AttachmentsPreviewViewDarkPreview_0_null_2,NEXUS_5,1.0,en].png index 62b6486890..eae4aa7665 100644 --- a/tests/uitests/src/test/snapshots/images/io.element.android.tests.uitests_ScreenshotTest_preview_tests[io.element.android.features.messages.impl.attachments.preview_null_DefaultGroup_AttachmentsPreviewViewDarkPreview_0_null_2,NEXUS_5,1.0,en].png +++ b/tests/uitests/src/test/snapshots/images/io.element.android.tests.uitests_ScreenshotTest_preview_tests[io.element.android.features.messages.impl.attachments.preview_null_DefaultGroup_AttachmentsPreviewViewDarkPreview_0_null_2,NEXUS_5,1.0,en].png @@ -1,3 +1,3 @@ version https://git-lfs.github.com/spec/v1 -oid sha256:3ce8ff927a9c9e414e2a37da8296b5a880a043d2e162f7169d58161c209adcae -size 185108 +oid sha256:bfe50fa79033ece5df4dc8c9c5b4fd0ab7f7baa03bbd07197694bfea65c2e12a +size 68248