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

* 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
This commit is contained in:
Jorge Martin Espinosa 2025-07-23 08:55:20 +02:00 committed by GitHub
parent 746699b468
commit b219c05d06
7 changed files with 65 additions and 8 deletions

View file

@ -131,12 +131,17 @@ class AttachmentsPreviewPresenter @AssistedInject constructor(
dismissAfterSend = !useSendQueue, dismissAfterSend = !useSendQueue,
replyParameters = null, replyParameters = null,
) )
// Clean up the pre-processed media after it's been sent
mediaSender.cleanUp()
} }
} }
} }
AttachmentsPreviewEvents.CancelAndDismiss -> { AttachmentsPreviewEvents.CancelAndDismiss -> {
// Cancel media preprocessing and sending // Cancel media preprocessing and sending
preprocessMediaJob?.cancel() preprocessMediaJob?.cancel()
// If we couldn't send the pre-processed media, remove it
mediaSender.cleanUp()
ongoingSendAttachmentJob.value?.cancel() ongoingSendAttachmentJob.value?.cancel()
// Dismiss the screen // Dismiss the screen

View file

@ -66,7 +66,7 @@ fun AttachmentsPreviewView(
state.eventSink(AttachmentsPreviewEvents.CancelAndClearSendState) state.eventSink(AttachmentsPreviewEvents.CancelAndClearSendState)
} }
BackHandler(enabled = state.sendActionState !is SendActionState.Sending) { BackHandler(enabled = state.sendActionState !is SendActionState.Sending.Uploading) {
postCancel() postCancel()
} }

View file

@ -123,8 +123,10 @@ class AttachmentsPreviewPresenterTest {
}, },
) )
val onDoneListener = lambdaRecorder<Unit> { } val onDoneListener = lambdaRecorder<Unit> { }
val mediaPreProcessor = FakeMediaPreProcessor()
val presenter = createAttachmentsPreviewPresenter( val presenter = createAttachmentsPreviewPresenter(
room = room, room = room,
mediaPreProcessor = mediaPreProcessor,
onDoneListener = { onDoneListener() }, onDoneListener = { onDoneListener() },
) )
moleculeFlow(RecompositionMode.Immediate) { moleculeFlow(RecompositionMode.Immediate) {
@ -143,6 +145,7 @@ class AttachmentsPreviewPresenterTest {
assertThat(awaitItem().sendActionState).isEqualTo(SendActionState.Done) assertThat(awaitItem().sendActionState).isEqualTo(SendActionState.Done)
sendFileResult.assertions().isCalledOnce() sendFileResult.assertions().isCalledOnce()
onDoneListener.assertions().isCalledOnce() onDoneListener.assertions().isCalledOnce()
assertThat(mediaPreProcessor.cleanUpCallCount).isEqualTo(1)
} }
} }
@ -159,11 +162,10 @@ class AttachmentsPreviewPresenterTest {
) )
val onDoneListener = lambdaRecorder<Unit> { } val onDoneListener = lambdaRecorder<Unit> { }
val processLatch = CompletableDeferred<Unit>() val processLatch = CompletableDeferred<Unit>()
val mediaPreProcessor = FakeMediaPreProcessor(processLatch)
val presenter = createAttachmentsPreviewPresenter( val presenter = createAttachmentsPreviewPresenter(
room = room, room = room,
mediaPreProcessor = FakeMediaPreProcessor( mediaPreProcessor = mediaPreProcessor,
processLatch = processLatch,
),
onDoneListener = { onDoneListener() }, onDoneListener = { onDoneListener() },
) )
moleculeFlow(RecompositionMode.Immediate) { moleculeFlow(RecompositionMode.Immediate) {
@ -181,6 +183,7 @@ class AttachmentsPreviewPresenterTest {
assertThat(awaitItem().sendActionState).isEqualTo(SendActionState.Done) assertThat(awaitItem().sendActionState).isEqualTo(SendActionState.Done)
sendFileResult.assertions().isCalledOnce() sendFileResult.assertions().isCalledOnce()
onDoneListener.assertions().isCalledOnce() onDoneListener.assertions().isCalledOnce()
assertThat(mediaPreProcessor.cleanUpCallCount).isEqualTo(1)
} }
} }
@ -197,11 +200,10 @@ class AttachmentsPreviewPresenterTest {
) )
val onDoneListener = lambdaRecorder<Unit> { } val onDoneListener = lambdaRecorder<Unit> { }
val processLatch = CompletableDeferred<Unit>() val processLatch = CompletableDeferred<Unit>()
val mediaPreProcessor = FakeMediaPreProcessor(processLatch)
val presenter = createAttachmentsPreviewPresenter( val presenter = createAttachmentsPreviewPresenter(
room = room, room = room,
mediaPreProcessor = FakeMediaPreProcessor( mediaPreProcessor = mediaPreProcessor,
processLatch = processLatch,
),
onDoneListener = { onDoneListener() }, onDoneListener = { onDoneListener() },
) )
moleculeFlow(RecompositionMode.Immediate) { moleculeFlow(RecompositionMode.Immediate) {
@ -219,6 +221,7 @@ class AttachmentsPreviewPresenterTest {
assertThat(awaitItem().sendActionState).isEqualTo(SendActionState.Done) assertThat(awaitItem().sendActionState).isEqualTo(SendActionState.Done)
sendFileResult.assertions().isCalledOnce() sendFileResult.assertions().isCalledOnce()
onDoneListener.assertions().isCalledOnce() onDoneListener.assertions().isCalledOnce()
assertThat(mediaPreProcessor.cleanUpCallCount).isEqualTo(1)
} }
} }
@ -279,7 +282,9 @@ class AttachmentsPreviewPresenterTest {
fun `present - cancel scenario`() = runTest { fun `present - cancel scenario`() = runTest {
val onDoneListener = lambdaRecorder<Unit> { } val onDoneListener = lambdaRecorder<Unit> { }
val deleteCallback = lambdaRecorder<Uri?, Unit> {} val deleteCallback = lambdaRecorder<Uri?, Unit> {}
val mediaPreProcessor = FakeMediaPreProcessor()
val presenter = createAttachmentsPreviewPresenter( val presenter = createAttachmentsPreviewPresenter(
mediaPreProcessor = mediaPreProcessor,
temporaryUriDeleter = FakeTemporaryUriDeleter(deleteCallback), temporaryUriDeleter = FakeTemporaryUriDeleter(deleteCallback),
onDoneListener = { onDoneListener() }, onDoneListener = { onDoneListener() },
) )
@ -293,6 +298,7 @@ class AttachmentsPreviewPresenterTest {
assertThat(awaitItem().sendActionState).isEqualTo(SendActionState.Done) assertThat(awaitItem().sendActionState).isEqualTo(SendActionState.Done)
deleteCallback.assertions().isCalledOnce() deleteCallback.assertions().isCalledOnce()
onDoneListener.assertions().isCalledOnce() onDoneListener.assertions().isCalledOnce()
assertThat(mediaPreProcessor.cleanUpCallCount).isEqualTo(1)
} }
} }

View file

@ -22,5 +22,10 @@ interface MediaPreProcessor {
compressIfPossible: Boolean, compressIfPossible: Boolean,
): Result<MediaUploadInfo> ): 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) data class Failure(override val cause: Throwable?) : Exception(cause)
} }

View file

@ -200,4 +200,9 @@ class MediaSender @Inject constructor(
uploadHandler.await() uploadHandler.await()
} }
} }
/**
* Clean up any temporary files or resources used during the media processing.
*/
fun cleanUp() = preProcessor.cleanUp()
} }

View file

@ -41,6 +41,7 @@ import kotlinx.coroutines.withContext
import timber.log.Timber import timber.log.Timber
import java.io.File import java.io.File
import java.io.InputStream import java.io.InputStream
import java.util.UUID
import javax.inject.Inject import javax.inject.Inject
import kotlin.time.Duration import kotlin.time.Duration
import kotlin.time.Duration.Companion.milliseconds import kotlin.time.Duration.Companion.milliseconds
@ -68,6 +69,9 @@ class AndroidMediaPreProcessor @Inject constructor(
private val contentResolver = context.contentResolver private val contentResolver = context.contentResolver
private val cacheDir = context.cacheDir
private val baseTmpFileDir = File(cacheDir, "uploads")
override suspend fun process( override suspend fun process(
uri: Uri, uri: Uri,
mimeType: String, mimeType: String,
@ -100,6 +104,28 @@ class AndroidMediaPreProcessor @Inject constructor(
} }
}.mapFailure { MediaPreProcessor.Failure(it) } }.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 { private suspend fun processFile(uri: Uri, mimeType: String): MediaUploadInfo {
val file = copyToTmpFile(uri) val file = copyToTmpFile(uri)
val info = FileInfo( val info = FileInfo(
@ -280,7 +306,10 @@ class AndroidMediaPreProcessor @Inject constructor(
private suspend fun createTmpFileWithInput(inputStream: InputStream): File? { private suspend fun createTmpFileWithInput(inputStream: InputStream): File? {
return withContext(coroutineDispatchers.io) { return withContext(coroutineDispatchers.io) {
tryOrNull { tryOrNull {
val tmpFile = context.createTmpFile() if (!baseTmpFileDir.exists()) {
baseTmpFileDir.mkdirs()
}
val tmpFile = context.createTmpFile(baseTmpFileDir)
tmpFile.outputStream().use { inputStream.copyTo(it) } tmpFile.outputStream().use { inputStream.copyTo(it) }
tmpFile tmpFile
} }

View file

@ -26,6 +26,9 @@ class FakeMediaPreProcessor(
var processCallCount = 0 var processCallCount = 0
private set private set
var cleanUpCallCount = 0
private set
private var result: Result<MediaUploadInfo> = Result.success( private var result: Result<MediaUploadInfo> = Result.success(
MediaUploadInfo.AnyFile( MediaUploadInfo.AnyFile(
File("test"), File("test"),
@ -108,4 +111,8 @@ class FakeMediaPreProcessor(
) )
) )
} }
override fun cleanUp() {
cleanUpCallCount += 1
}
} }