Make sure we clean up the pre-processed and uploaded media (#5039)

jmartinesp · web-flow · commit d53457ec6675 · 2025-07-23T08:55:20.000+02:00
* Add `MediaSender.cleanUp()` and `MediaPreProcessor.cleanUp()` methods: this will remove the temporary files created when pre-processing media before sending them.

* Make sure we clean up also the previous temporary media.

* Fix the condition for the custom back handler in the attachments preview screen

* Tests: check the clean up is performed when needed
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
@@ -131,12 +131,17 @@ class AttachmentsPreviewPresenter @AssistedInject constructor(
                                 dismissAfterSend = !useSendQueue,
                                 replyParameters = null,
                             )
+
+                            // Clean up the pre-processed media after it's been sent
+                            mediaSender.cleanUp()
                         }
                     }
                 }
                 AttachmentsPreviewEvents.CancelAndDismiss -> {
                     // Cancel media preprocessing and sending
                     preprocessMediaJob?.cancel()
+                    // If we couldn't send the pre-processed media, remove it
+                    mediaSender.cleanUp()
                     ongoingSendAttachmentJob.value?.cancel()
 
                     // Dismiss the screen
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
@@ -66,7 +66,7 @@ fun AttachmentsPreviewView(
         state.eventSink(AttachmentsPreviewEvents.CancelAndClearSendState)
     }
 
-    BackHandler(enabled = state.sendActionState !is SendActionState.Sending) {
+    BackHandler(enabled = state.sendActionState !is SendActionState.Sending.Uploading) {
         postCancel()
     }
 
diff --git a/features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/attachments/AttachmentsPreviewPresenterTest.kt b/features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/attachments/AttachmentsPreviewPresenterTest.kt
@@ -123,8 +123,10 @@ class AttachmentsPreviewPresenterTest {
             },
         )
         val onDoneListener = lambdaRecorder<Unit> { }
+        val mediaPreProcessor = FakeMediaPreProcessor()
         val presenter = createAttachmentsPreviewPresenter(
             room = room,
+            mediaPreProcessor = mediaPreProcessor,
             onDoneListener = { onDoneListener() },
         )
         moleculeFlow(RecompositionMode.Immediate) {
@@ -143,6 +145,7 @@ class AttachmentsPreviewPresenterTest {
             assertThat(awaitItem().sendActionState).isEqualTo(SendActionState.Done)
             sendFileResult.assertions().isCalledOnce()
             onDoneListener.assertions().isCalledOnce()
+            assertThat(mediaPreProcessor.cleanUpCallCount).isEqualTo(1)
         }
     }
 
@@ -159,11 +162,10 @@ class AttachmentsPreviewPresenterTest {
         )
         val onDoneListener = lambdaRecorder<Unit> { }
         val processLatch = CompletableDeferred<Unit>()
+        val mediaPreProcessor = FakeMediaPreProcessor(processLatch)
         val presenter = createAttachmentsPreviewPresenter(
             room = room,
-            mediaPreProcessor = FakeMediaPreProcessor(
-                processLatch = processLatch,
-            ),
+            mediaPreProcessor = mediaPreProcessor,
             onDoneListener = { onDoneListener() },
         )
         moleculeFlow(RecompositionMode.Immediate) {
@@ -181,6 +183,7 @@ class AttachmentsPreviewPresenterTest {
             assertThat(awaitItem().sendActionState).isEqualTo(SendActionState.Done)
             sendFileResult.assertions().isCalledOnce()
             onDoneListener.assertions().isCalledOnce()
+            assertThat(mediaPreProcessor.cleanUpCallCount).isEqualTo(1)
         }
     }
 
@@ -197,11 +200,10 @@ class AttachmentsPreviewPresenterTest {
         )
         val onDoneListener = lambdaRecorder<Unit> { }
         val processLatch = CompletableDeferred<Unit>()
+        val mediaPreProcessor = FakeMediaPreProcessor(processLatch)
         val presenter = createAttachmentsPreviewPresenter(
             room = room,
-            mediaPreProcessor = FakeMediaPreProcessor(
-                processLatch = processLatch,
-            ),
+            mediaPreProcessor = mediaPreProcessor,
             onDoneListener = { onDoneListener() },
         )
         moleculeFlow(RecompositionMode.Immediate) {
@@ -219,6 +221,7 @@ class AttachmentsPreviewPresenterTest {
             assertThat(awaitItem().sendActionState).isEqualTo(SendActionState.Done)
             sendFileResult.assertions().isCalledOnce()
             onDoneListener.assertions().isCalledOnce()
+            assertThat(mediaPreProcessor.cleanUpCallCount).isEqualTo(1)
         }
     }
 
@@ -279,7 +282,9 @@ class AttachmentsPreviewPresenterTest {
     fun `present - cancel scenario`() = runTest {
         val onDoneListener = lambdaRecorder<Unit> { }
         val deleteCallback = lambdaRecorder<Uri?, Unit> {}
+        val mediaPreProcessor = FakeMediaPreProcessor()
         val presenter = createAttachmentsPreviewPresenter(
+            mediaPreProcessor = mediaPreProcessor,
             temporaryUriDeleter = FakeTemporaryUriDeleter(deleteCallback),
             onDoneListener = { onDoneListener() },
         )
@@ -293,6 +298,7 @@ class AttachmentsPreviewPresenterTest {
             assertThat(awaitItem().sendActionState).isEqualTo(SendActionState.Done)
             deleteCallback.assertions().isCalledOnce()
             onDoneListener.assertions().isCalledOnce()
+            assertThat(mediaPreProcessor.cleanUpCallCount).isEqualTo(1)
         }
     }
 
diff --git a/libraries/mediaupload/api/src/main/kotlin/io/element/android/libraries/mediaupload/api/MediaPreProcessor.kt b/libraries/mediaupload/api/src/main/kotlin/io/element/android/libraries/mediaupload/api/MediaPreProcessor.kt
@@ -22,5 +22,10 @@ interface MediaPreProcessor {
         compressIfPossible: Boolean,
     ): Result<MediaUploadInfo>
 
+    /**
+     * Clean up any temporary files or resources used during the media processing.
+     */
+    fun cleanUp()
+
     data class Failure(override val cause: Throwable?) : Exception(cause)
 }
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
@@ -200,4 +200,9 @@ class MediaSender @Inject constructor(
                 uploadHandler.await()
             }
     }
+
+    /**
+     * Clean up any temporary files or resources used during the media processing.
+     */
+    fun cleanUp() = preProcessor.cleanUp()
 }
diff --git a/libraries/mediaupload/impl/src/main/kotlin/io/element/android/libraries/mediaupload/impl/AndroidMediaPreProcessor.kt b/libraries/mediaupload/impl/src/main/kotlin/io/element/android/libraries/mediaupload/impl/AndroidMediaPreProcessor.kt
@@ -41,6 +41,7 @@ import kotlinx.coroutines.withContext
 import timber.log.Timber
 import java.io.File
 import java.io.InputStream
+import java.util.UUID
 import javax.inject.Inject
 import kotlin.time.Duration
 import kotlin.time.Duration.Companion.milliseconds
@@ -68,6 +69,9 @@ class AndroidMediaPreProcessor @Inject constructor(
 
     private val contentResolver = context.contentResolver
 
+    private val cacheDir = context.cacheDir
+    private val baseTmpFileDir = File(cacheDir, "uploads")
+
     override suspend fun process(
         uri: Uri,
         mimeType: String,
@@ -100,6 +104,28 @@ class AndroidMediaPreProcessor @Inject constructor(
         }
     }.mapFailure { MediaPreProcessor.Failure(it) }
 
+    override fun cleanUp() {
+        // Clear temporary files created in older versions of the app
+        cacheDir.listFiles()?.onEach { file ->
+            if (file.isFile) {
+                val nameWithoutExtension = file.nameWithoutExtension
+                // UUIDs are 36 characters long, so we check if we can take those 36 characters
+                val nameWithoutExtensionAndRandom = if (nameWithoutExtension.length > 36) {
+                    nameWithoutExtension.substring(0, 36)
+                } else {
+                    // Not a temp file
+                    return@onEach
+                }
+                val isUUID = tryOrNull { UUID.fromString(nameWithoutExtensionAndRandom) } != null
+                if (isUUID && file.extension.isNotEmpty()) {
+                    file.delete()
+                }
+            }
+        }
+        // Clear temporary files created by this pre-processor in the separate uploads directory
+        baseTmpFileDir.listFiles()?.onEach { it.delete() }
+    }
+
     private suspend fun processFile(uri: Uri, mimeType: String): MediaUploadInfo {
         val file = copyToTmpFile(uri)
         val info = FileInfo(
@@ -280,7 +306,10 @@ class AndroidMediaPreProcessor @Inject constructor(
     private suspend fun createTmpFileWithInput(inputStream: InputStream): File? {
         return withContext(coroutineDispatchers.io) {
             tryOrNull {
-                val tmpFile = context.createTmpFile()
+                if (!baseTmpFileDir.exists()) {
+                    baseTmpFileDir.mkdirs()
+                }
+                val tmpFile = context.createTmpFile(baseTmpFileDir)
                 tmpFile.outputStream().use { inputStream.copyTo(it) }
                 tmpFile
             }
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
@@ -26,6 +26,9 @@ class FakeMediaPreProcessor(
     var processCallCount = 0
         private set
 
+    var cleanUpCallCount = 0
+        private set
+
     private var result: Result<MediaUploadInfo> = Result.success(
         MediaUploadInfo.AnyFile(
             File("test"),
@@ -108,4 +111,8 @@ class FakeMediaPreProcessor(
             )
         )
     }
+
+    override fun cleanUp() {
+        cleanUpCallCount += 1
+    }
 }

Original file line number	Diff line number	Diff line change
`@@ -131,12 +131,17 @@ class AttachmentsPreviewPresenter @AssistedInject constructor(`
`131`	`131`	`dismissAfterSend = !useSendQueue,`
`132`	`132`	`replyParameters = null,`
`133`	`133`	`)`
	`134`	`+`
	`135`	`+ // Clean up the pre-processed media after it's been sent`
	`136`	`+ mediaSender.cleanUp()`
`134`	`137`	`}`
`135`	`138`	`}`
`136`	`139`	`}`
`137`	`140`	`AttachmentsPreviewEvents.CancelAndDismiss -> {`
`138`	`141`	`// Cancel media preprocessing and sending`
`139`	`142`	`preprocessMediaJob?.cancel()`
	`143`	`+ // If we couldn't send the pre-processed media, remove it`
	`144`	`+ mediaSender.cleanUp()`
`140`	`145`	`ongoingSendAttachmentJob.value?.cancel()`
`141`	`146`
`142`	`147`	`// Dismiss the screen`
Original file line number	Diff line number	Diff line change
`@@ -66,7 +66,7 @@ fun AttachmentsPreviewView(`
`66`	`66`	`state.eventSink(AttachmentsPreviewEvents.CancelAndClearSendState)`
`67`	`67`	`}`
`68`	`68`
`69`		`- BackHandler(enabled = state.sendActionState !is SendActionState.Sending) {`
	`69`	`+ BackHandler(enabled = state.sendActionState !is SendActionState.Sending.Uploading) {`
`70`	`70`	`postCancel()`
`71`	`71`	`}`
`72`	`72`
Original file line number	Diff line number	Diff line change
`@@ -123,8 +123,10 @@ class AttachmentsPreviewPresenterTest {`
`123`	`123`	`},`
`124`	`124`	`)`
`125`	`125`	`val onDoneListener = lambdaRecorder<Unit> { }`
	`126`	`+ val mediaPreProcessor = FakeMediaPreProcessor()`
`126`	`127`	`val presenter = createAttachmentsPreviewPresenter(`
`127`	`128`	`room = room,`
	`129`	`+ mediaPreProcessor = mediaPreProcessor,`
`128`	`130`	`onDoneListener = { onDoneListener() },`
`129`	`131`	`)`
`130`	`132`	`moleculeFlow(RecompositionMode.Immediate) {`
`@@ -143,6 +145,7 @@ class AttachmentsPreviewPresenterTest {`
`143`	`145`	`assertThat(awaitItem().sendActionState).isEqualTo(SendActionState.Done)`
`144`	`146`	`sendFileResult.assertions().isCalledOnce()`
`145`	`147`	`onDoneListener.assertions().isCalledOnce()`
	`148`	`+ assertThat(mediaPreProcessor.cleanUpCallCount).isEqualTo(1)`
`146`	`149`	`}`
`147`	`150`	`}`
`148`	`151`
`@@ -159,11 +162,10 @@ class AttachmentsPreviewPresenterTest {`
`159`	`162`	`)`
`160`	`163`	`val onDoneListener = lambdaRecorder<Unit> { }`
`161`	`164`	`val processLatch = CompletableDeferred<Unit>()`
	`165`	`+ val mediaPreProcessor = FakeMediaPreProcessor(processLatch)`
`162`	`166`	`val presenter = createAttachmentsPreviewPresenter(`
`163`	`167`	`room = room,`
`164`		`- mediaPreProcessor = FakeMediaPreProcessor(`
`165`		`- processLatch = processLatch,`
`166`		`- ),`
	`168`	`+ mediaPreProcessor = mediaPreProcessor,`
`167`	`169`	`onDoneListener = { onDoneListener() },`
`168`	`170`	`)`
`169`	`171`	`moleculeFlow(RecompositionMode.Immediate) {`
`@@ -181,6 +183,7 @@ class AttachmentsPreviewPresenterTest {`
`181`	`183`	`assertThat(awaitItem().sendActionState).isEqualTo(SendActionState.Done)`
`182`	`184`	`sendFileResult.assertions().isCalledOnce()`
`183`	`185`	`onDoneListener.assertions().isCalledOnce()`
	`186`	`+ assertThat(mediaPreProcessor.cleanUpCallCount).isEqualTo(1)`
`184`	`187`	`}`
`185`	`188`	`}`
`186`	`189`
`@@ -197,11 +200,10 @@ class AttachmentsPreviewPresenterTest {`
`197`	`200`	`)`
`198`	`201`	`val onDoneListener = lambdaRecorder<Unit> { }`
`199`	`202`	`val processLatch = CompletableDeferred<Unit>()`
	`203`	`+ val mediaPreProcessor = FakeMediaPreProcessor(processLatch)`
`200`	`204`	`val presenter = createAttachmentsPreviewPresenter(`
`201`	`205`	`room = room,`
`202`		`- mediaPreProcessor = FakeMediaPreProcessor(`
`203`		`- processLatch = processLatch,`
`204`		`- ),`
	`206`	`+ mediaPreProcessor = mediaPreProcessor,`
`205`	`207`	`onDoneListener = { onDoneListener() },`
`206`	`208`	`)`
`207`	`209`	`moleculeFlow(RecompositionMode.Immediate) {`
`@@ -219,6 +221,7 @@ class AttachmentsPreviewPresenterTest {`
`219`	`221`	`assertThat(awaitItem().sendActionState).isEqualTo(SendActionState.Done)`
`220`	`222`	`sendFileResult.assertions().isCalledOnce()`
`221`	`223`	`onDoneListener.assertions().isCalledOnce()`
	`224`	`+ assertThat(mediaPreProcessor.cleanUpCallCount).isEqualTo(1)`
`222`	`225`	`}`
`223`	`226`	`}`
`224`	`227`
`@@ -279,7 +282,9 @@ class AttachmentsPreviewPresenterTest {`
`279`	`282`	fun `present - cancel scenario`() = runTest {
`280`	`283`	`val onDoneListener = lambdaRecorder<Unit> { }`
`281`	`284`	`val deleteCallback = lambdaRecorder<Uri?, Unit> {}`
	`285`	`+ val mediaPreProcessor = FakeMediaPreProcessor()`
`282`	`286`	`val presenter = createAttachmentsPreviewPresenter(`
	`287`	`+ mediaPreProcessor = mediaPreProcessor,`
`283`	`288`	`temporaryUriDeleter = FakeTemporaryUriDeleter(deleteCallback),`
`284`	`289`	`onDoneListener = { onDoneListener() },`
`285`	`290`	`)`
`@@ -293,6 +298,7 @@ class AttachmentsPreviewPresenterTest {`
`293`	`298`	`assertThat(awaitItem().sendActionState).isEqualTo(SendActionState.Done)`
`294`	`299`	`deleteCallback.assertions().isCalledOnce()`
`295`	`300`	`onDoneListener.assertions().isCalledOnce()`
	`301`	`+ assertThat(mediaPreProcessor.cleanUpCallCount).isEqualTo(1)`
`296`	`302`	`}`
`297`	`303`	`}`
`298`	`304`
Original file line number	Diff line number	Diff line change
`@@ -200,4 +200,9 @@ class MediaSender @Inject constructor(`
`200`	`200`	`uploadHandler.await()`
`201`	`201`	`}`
`202`	`202`	`}`
	`203`	`+`
	`204`	`+ /**`
	`205`	`+ * Clean up any temporary files or resources used during the media processing.`
	`206`	`+ */`
	`207`	`+ fun cleanUp() = preProcessor.cleanUp()`
`203`	`208`	`}`
Original file line number	Diff line number	Diff line change
`@@ -26,6 +26,9 @@ class FakeMediaPreProcessor(`
`26`	`26`	`var processCallCount = 0`
`27`	`27`	`private set`
`28`	`28`
	`29`	`+ var cleanUpCallCount = 0`
	`30`	`+ private set`
	`31`	`+`
`29`	`32`	`private var result: Result<MediaUploadInfo> = Result.success(`
`30`	`33`	`MediaUploadInfo.AnyFile(`
`31`	`34`	`File("test"),`
`@@ -108,4 +111,8 @@ class FakeMediaPreProcessor(`
`108`	`111`	`)`
`109`	`112`	`)`
`110`	`113`	`}`
	`114`	`+`
	`115`	`+ override fun cleanUp() {`
	`116`	`+ cleanUpCallCount += 1`
	`117`	`+ }`
`111`	`118`	`}`