Media upload cancellation (#1058)

* Initial implementation of media upload cancellation

* Add tests

* Add changelog

* Update screenshots

* Add documentation

* Fix lint issues

* Fix review comments

---------

Co-authored-by: ElementBot <benoitm+elementbot@element.io>

changelog.d/769.feature

@@ -0,0 +1 @@
+Allow cancelling media upload

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<Attachment>) -> Unit
+    onPreviewAttachments: (ImmutableList<Attachment>) -> 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,
             )
         }
     }

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>(SendActionState.Idle)
         }
 
+        val ongoingSendAttachmentJob = remember { mutableStateOf<Job?>(null) }
+
         fun handleEvents(attachmentsPreviewEvents: AttachmentsPreviewEvents) {
             when (attachmentsPreviewEvents) {
-                AttachmentsPreviewEvents.SendAttachment -> coroutineScope.sendAttachment(attachment, sendActionState)
+                AttachmentsPreviewEvents.SendAttachment -> ongoingSendAttachmentJob.value = coroutineScope.sendAttachment(attachment, sendActionState)
-                AttachmentsPreviewEvents.ClearSendState -> sendActionState.value = SendActionState.Idle
+                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<SendActionState>,
-    ) {
+    ) = 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(
+        ).getOrThrow()
-            onSuccess = {
+    }.fold(
-                sendActionState.value = SendActionState.Done
+        onSuccess = {
-            },
+            sendActionState.value = SendActionState.Done
-            onFailure = {
+        },
-                sendActionState.value = SendActionState.Failure(it)
+        onFailure = { error ->
+            if (error is CancellationException) {
+                throw error
+            } else {
+                sendActionState.value = SendActionState.Failure(error)
             }
-        )
+        }
-    }
+    )
 }

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 -> {

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
 }

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<String> = rememberSaveable {
             mutableStateOf("")
         }
+        val ongoingSendAttachmentJob = remember { mutableStateOf<Job?>(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<AttachmentsState>,
-    ) = launch {
+    ) = when (attachment) {
-        when (attachment) {
+        is Attachment.Media -> {
-            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<AttachmentsState>,
-    ) {
+    ) = 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)
+        mediaSender.sendMedia(uri, mimeType, compressIfPossible = false, progressCallback).getOrThrow()
-            .onSuccess {
+    }
-                attachmentState.value = AttachmentsState.None
+    .onSuccess {
-            }
+        attachmentState.value = AttachmentsState.None
-            .onFailure {
+    }
-                val snackbarMessage = SnackbarMessage(sendAttachmentError(it))
+    .onFailure { cause ->
-                snackbarDispatcher.post(snackbarMessage)
+        attachmentState.value = AttachmentsState.None
-                attachmentState.value = AttachmentsState.None
+        if (cause is CancellationException) {
-            }
+            throw cause
+        } else {
+            val snackbarMessage = SnackbarMessage(sendAttachmentError(cause))
+            snackbarDispatcher.post(snackbarMessage)
+        }
     }
 }

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,

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<MessageComposerState>.backToNormalMode(state: MessageComposerState, skipCount: Int = 0) {
         state.eventSink.invoke(MessageComposerEvents.CloseSpecialMode)
         skipItems(skipCount)

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<Unit>
+
+    /** Cancel the upload process. */
+    fun cancel()
+}

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<Unit>
 
-    suspend fun sendImage(file: File, thumbnailFile: File, imageInfo: ImageInfo, progressCallback: ProgressCallback?): Result<Unit>
+    suspend fun sendImage(file: File, thumbnailFile: File, imageInfo: ImageInfo, progressCallback: ProgressCallback?): Result<MediaUploadHandler>
 
-    suspend fun sendVideo(file: File, thumbnailFile: File, videoInfo: VideoInfo, progressCallback: ProgressCallback?): Result<Unit>
+    suspend fun sendVideo(file: File, thumbnailFile: File, videoInfo: VideoInfo, progressCallback: ProgressCallback?): Result<MediaUploadHandler>
 
-    suspend fun sendAudio(file: File, audioInfo: AudioInfo, progressCallback: ProgressCallback?): Result<Unit>
+    suspend fun sendAudio(file: File, audioInfo: AudioInfo, progressCallback: ProgressCallback?): Result<MediaUploadHandler>
 
-    suspend fun sendFile(file: File, fileInfo: FileInfo, progressCallback: ProgressCallback?): Result<Unit>
+    suspend fun sendFile(file: File, fileInfo: FileInfo, progressCallback: ProgressCallback?): Result<MediaUploadHandler>
 
     suspend fun toggleReaction(emoji: String, eventId: EventId): Result<Unit>
 

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<File>,
+    private val sendAttachmentJoinHandle: SendAttachmentJoinHandle,
+) : MediaUploadHandler {
+    override suspend fun await(): Result<Unit> =
+        runCatching {
+            sendAttachmentJoinHandle.join()
+        }
+        .also { cleanUpFiles() }
+
+    override fun cancel() {
+        sendAttachmentJoinHandle.cancel()
+        cleanUpFiles()
+    }
+
+    private fun cleanUpFiles() {
+        filesToUpload.forEach { file -> file.safeDelete() }
+    }
+}

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<Unit> {
+    override suspend fun sendImage(file: File, thumbnailFile: File, imageInfo: ImageInfo, progressCallback: ProgressCallback?): Result<MediaUploadHandler> {
-        return sendAttachment {
+        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<Unit> {
+    override suspend fun sendVideo(file: File, thumbnailFile: File, videoInfo: VideoInfo, progressCallback: ProgressCallback?): Result<MediaUploadHandler> {
-        return sendAttachment {
+        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<Unit> {
+    override suspend fun sendAudio(file: File, audioInfo: AudioInfo, progressCallback: ProgressCallback?): Result<MediaUploadHandler> {
-        return sendAttachment {
+        return sendAttachment(listOf(file)) {
             innerRoom.sendAudio(file.path, audioInfo.map(), progressCallback?.toProgressWatcher())
         }
     }
 
-    override suspend fun sendFile(file: File, fileInfo: FileInfo, progressCallback: ProgressCallback?): Result<Unit> {
+    override suspend fun sendFile(file: File, fileInfo: FileInfo, progressCallback: ProgressCallback?): Result<MediaUploadHandler> {
-        return sendAttachment {
+        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(files: List<File>, handle: () -> SendAttachmentJoinHandle): Result<MediaUploadHandler> {
-    private suspend fun sendAttachment(handle: () -> SendAttachmentJoinHandle): Result<Unit> {
         return runCatching {
-            handle().use {
+            MediaUploadHandlerImpl(files, handle())
-                it.join()
-            }
         }
     }
 }
-
-

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<Unit> = Result.success(Unit),
+) : MediaUploadHandler {
+    override suspend fun await(): Result<Unit> = simulateLongTask { result }
+
+    override fun cancel() {
+        result = Result.failure(CancellationException())
+    }
+}

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<StateEventType, Result<Boolean>>()
     private val canSendEventResults = mutableMapOf<MessageEventType, Result<Boolean>>()
-    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<Unit> = fakeSendMedia(progressCallback)
+    ): Result<MediaUploadHandler> = fakeSendMedia(progressCallback)
 
-    override suspend fun sendVideo(file: File, thumbnailFile: File, videoInfo: VideoInfo, progressCallback: ProgressCallback?): Result<Unit> = fakeSendMedia(
+    override suspend fun sendVideo(
+        file: File,
+        thumbnailFile: File,
+        videoInfo: VideoInfo,
+        progressCallback: ProgressCallback?
+    ): Result<MediaUploadHandler> = fakeSendMedia(
         progressCallback
     )
 
-    override suspend fun sendAudio(file: File, audioInfo: AudioInfo, progressCallback: ProgressCallback?): Result<Unit> = fakeSendMedia(progressCallback)
+    override suspend fun sendAudio(
+        file: File,
+        audioInfo: AudioInfo,
+        progressCallback: ProgressCallback?
+    ): Result<MediaUploadHandler> = fakeSendMedia(progressCallback)
 
-    override suspend fun sendFile(file: File, fileInfo: FileInfo, progressCallback: ProgressCallback?): Result<Unit> = fakeSendMedia(progressCallback)
+    override suspend fun sendFile(
+        file: File,
+        fileInfo: FileInfo,
+        progressCallback: ProgressCallback?
+    ): Result<MediaUploadHandler> = fakeSendMedia(progressCallback)
 
     override suspend fun forwardEvent(eventId: EventId, roomIds: List<RoomId>): Result<Unit> = simulateLongTask {
         forwardEventResult
     }
 
-    private suspend fun fakeSendMedia(progressCallback: ProgressCallback?): Result<Unit> = simulateLongTask {
+    private suspend fun fakeSendMedia(progressCallback: ProgressCallback?): Result<MediaUploadHandler> = simulateLongTask {
         sendMediaResult.onSuccess {
             progressCallbackValues.forEach { (current, total) ->
                 progressCallback?.onProgress(current, total)
@@ -338,7 +353,7 @@ class FakeMatrixRoom(
         unignoreResult = result
     }
 
-    fun givenSendMediaResult(result: Result<Unit>) {
+    fun givenSendMediaResult(result: Result<FakeMediaUploadHandler>) {
         sendMediaResult = result
     }
 

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)
     }
 }

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<Job.Key, MediaUploadHandler>()
+    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<Unit> {
-        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()
+            }
     }
 }

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,
+    )
+}

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<MediaUploadInfo> = Result.success(
         MediaUploadInfo.AnyFile(
             File("test"),
@@ -43,6 +46,7 @@ class FakeMediaPreProcessor : MediaPreProcessor {
         deleteOriginal: Boolean,
         compressIfPossible: Boolean
     ): Result<MediaUploadInfo> = simulateLongTask {
+        processCallCount++
         result
     }
 

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
+oid sha256:bfe50fa79033ece5df4dc8c9c5b4fd0ab7f7baa03bbd07197694bfea65c2e12a
-size 185108
+size 68248