diff --git a/features/createroom/impl/src/main/kotlin/io/element/android/features/createroom/impl/configureroom/ConfigureRoomPresenter.kt b/features/createroom/impl/src/main/kotlin/io/element/android/features/createroom/impl/configureroom/ConfigureRoomPresenter.kt index ad19d2cb9b7..bbefc987ef3 100644 --- a/features/createroom/impl/src/main/kotlin/io/element/android/features/createroom/impl/configureroom/ConfigureRoomPresenter.kt +++ b/features/createroom/impl/src/main/kotlin/io/element/android/features/createroom/impl/configureroom/ConfigureRoomPresenter.kt @@ -175,7 +175,12 @@ class ConfigureRoomPresenter @Inject constructor( } private suspend fun uploadAvatar(avatarUri: Uri): String { - val preprocessed = mediaPreProcessor.process(avatarUri, MimeTypes.Jpeg, compressIfPossible = false).getOrThrow() + val preprocessed = mediaPreProcessor.process( + uri = avatarUri, + mimeType = MimeTypes.Jpeg, + deleteOriginal = false, + compressIfPossible = false, + ).getOrThrow() val byteArray = preprocessed.file.readBytes() return matrixClient.uploadMedia(MimeTypes.Jpeg, byteArray, null).getOrThrow() } diff --git a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/attachments/preview/AttachmentsPreviewEvents.kt b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/attachments/preview/AttachmentsPreviewEvents.kt index 28cc95eaf06..742e78e8e03 100644 --- a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/attachments/preview/AttachmentsPreviewEvents.kt +++ b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/attachments/preview/AttachmentsPreviewEvents.kt @@ -12,5 +12,6 @@ import androidx.compose.runtime.Immutable @Immutable sealed interface AttachmentsPreviewEvents { data object SendAttachment : AttachmentsPreviewEvents + data object Cancel : AttachmentsPreviewEvents data object ClearSendState : AttachmentsPreviewEvents } diff --git a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/attachments/preview/AttachmentsPreviewNode.kt b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/attachments/preview/AttachmentsPreviewNode.kt index a435821197b..2417d8346ea 100644 --- a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/attachments/preview/AttachmentsPreviewNode.kt +++ b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/attachments/preview/AttachmentsPreviewNode.kt @@ -31,7 +31,14 @@ class AttachmentsPreviewNode @AssistedInject constructor( private val inputs: Inputs = inputs() - private val presenter = presenterFactory.create(inputs.attachment) + private val onDoneListener = OnDoneListener { + navigateUp() + } + + private val presenter = presenterFactory.create( + attachment = inputs.attachment, + onDoneListener = onDoneListener, + ) @Composable override fun View(modifier: Modifier) { @@ -39,7 +46,6 @@ class AttachmentsPreviewNode @AssistedInject constructor( val state = presenter.present() AttachmentsPreviewView( state = state, - onDismiss = this::navigateUp, modifier = modifier ) } 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 57e2ee3ad85..bc3e121070d 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 @@ -18,6 +18,7 @@ import dagger.assisted.Assisted import dagger.assisted.AssistedFactory import dagger.assisted.AssistedInject import io.element.android.features.messages.impl.attachments.Attachment +import io.element.android.libraries.androidutils.file.TemporaryUriDeleter import io.element.android.libraries.architecture.Presenter import io.element.android.libraries.matrix.api.core.ProgressCallback import io.element.android.libraries.matrix.api.permalink.PermalinkBuilder @@ -34,12 +35,17 @@ import kotlin.coroutines.coroutineContext class AttachmentsPreviewPresenter @AssistedInject constructor( @Assisted private val attachment: Attachment, + @Assisted private val onDoneListener: OnDoneListener, private val mediaSender: MediaSender, private val permalinkBuilder: PermalinkBuilder, + private val temporaryUriDeleter: TemporaryUriDeleter, ) : Presenter { @AssistedFactory interface Factory { - fun create(attachment: Attachment): AttachmentsPreviewPresenter + fun create( + attachment: Attachment, + onDoneListener: OnDoneListener, + ): AttachmentsPreviewPresenter } @Composable @@ -68,6 +74,9 @@ class AttachmentsPreviewPresenter @AssistedInject constructor( sendActionState = sendActionState, ) } + AttachmentsPreviewEvents.Cancel -> { + coroutineScope.cancel(attachment) + } AttachmentsPreviewEvents.ClearSendState -> { ongoingSendAttachmentJob.value?.let { it.cancel() @@ -102,6 +111,18 @@ class AttachmentsPreviewPresenter @AssistedInject constructor( } } + private fun CoroutineScope.cancel( + attachment: Attachment, + ) = launch { + // Delete the temporary file + when (attachment) { + is Attachment.Media -> { + temporaryUriDeleter.delete(attachment.localMedia.uri) + } + } + onDoneListener() + } + private suspend fun sendMedia( mediaAttachment: Attachment.Media, caption: String?, @@ -124,7 +145,7 @@ class AttachmentsPreviewPresenter @AssistedInject constructor( ).getOrThrow() }.fold( onSuccess = { - sendActionState.value = SendActionState.Done + onDoneListener() }, onFailure = { error -> Timber.e(error, "Failed to send attachment") diff --git a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/attachments/preview/AttachmentsPreviewState.kt b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/attachments/preview/AttachmentsPreviewState.kt index 72ea0a20989..5ffe9364ff8 100644 --- a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/attachments/preview/AttachmentsPreviewState.kt +++ b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/attachments/preview/AttachmentsPreviewState.kt @@ -36,5 +36,4 @@ sealed interface SendActionState { } data class Failure(val error: Throwable) : SendActionState - data object Done : SendActionState } 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 2906f26b3ca..1380add3298 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 @@ -7,6 +7,7 @@ package io.element.android.features.messages.impl.attachments.preview +import androidx.activity.compose.BackHandler import androidx.compose.foundation.background import androidx.compose.foundation.layout.Box import androidx.compose.foundation.layout.IntrinsicSize @@ -17,9 +18,6 @@ import androidx.compose.foundation.layout.imePadding import androidx.compose.foundation.layout.navigationBarsPadding import androidx.compose.material3.ExperimentalMaterial3Api import androidx.compose.runtime.Composable -import androidx.compose.runtime.LaunchedEffect -import androidx.compose.runtime.getValue -import androidx.compose.runtime.rememberUpdatedState import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier import androidx.compose.ui.res.stringResource @@ -50,22 +48,22 @@ import me.saket.telephoto.zoomable.rememberZoomableState @Composable fun AttachmentsPreviewView( state: AttachmentsPreviewState, - onDismiss: () -> Unit, modifier: Modifier = Modifier, ) { fun postSendAttachment() { state.eventSink(AttachmentsPreviewEvents.SendAttachment) } + fun postCancel() { + state.eventSink(AttachmentsPreviewEvents.Cancel) + } + fun postClearSendState() { state.eventSink(AttachmentsPreviewEvents.ClearSendState) } - if (state.sendActionState is SendActionState.Done) { - val latestOnDismiss by rememberUpdatedState(onDismiss) - LaunchedEffect(state.sendActionState) { - latestOnDismiss() - } + BackHandler(enabled = state.sendActionState !is SendActionState.Sending) { + postCancel() } Scaffold( @@ -75,7 +73,7 @@ fun AttachmentsPreviewView( navigationIcon = { BackButton( imageVector = CompoundIcons.Close(), - onClick = onDismiss, + onClick = ::postCancel, ) }, title = {}, @@ -202,6 +200,5 @@ private fun AttachmentsPreviewBottomActions( internal fun AttachmentsPreviewViewPreview(@PreviewParameter(AttachmentsPreviewStateProvider::class) state: AttachmentsPreviewState) = ElementPreviewDark { AttachmentsPreviewView( state = state, - onDismiss = {}, ) } diff --git a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/attachments/preview/OnDoneListener.kt b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/attachments/preview/OnDoneListener.kt new file mode 100644 index 00000000000..2e53ab2c500 --- /dev/null +++ b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/attachments/preview/OnDoneListener.kt @@ -0,0 +1,12 @@ +/* + * Copyright 2024 New Vector Ltd. + * + * SPDX-License-Identifier: AGPL-3.0-only + * Please see LICENSE in the repository root for full details. + */ + +package io.element.android.features.messages.impl.attachments.preview + +fun interface OnDoneListener { + operator fun invoke() +} 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 index 51318454ebb..ac753f6cdb1 100644 --- 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 @@ -16,8 +16,10 @@ import app.cash.turbine.test import com.google.common.truth.Truth.assertThat import io.element.android.features.messages.impl.attachments.preview.AttachmentsPreviewEvents import io.element.android.features.messages.impl.attachments.preview.AttachmentsPreviewPresenter +import io.element.android.features.messages.impl.attachments.preview.OnDoneListener import io.element.android.features.messages.impl.attachments.preview.SendActionState import io.element.android.features.messages.impl.fixtures.aMediaAttachment +import io.element.android.libraries.androidutils.file.TemporaryUriDeleter import io.element.android.libraries.matrix.api.core.ProgressCallback import io.element.android.libraries.matrix.api.media.FileInfo import io.element.android.libraries.matrix.api.media.ImageInfo @@ -35,11 +37,13 @@ import io.element.android.libraries.mediaviewer.api.local.LocalMedia import io.element.android.libraries.mediaviewer.test.viewer.aLocalMedia import io.element.android.libraries.preferences.test.InMemorySessionPreferencesStore import io.element.android.tests.testutils.WarmUpRule +import io.element.android.tests.testutils.fake.FakeTemporaryUriDeleter import io.element.android.tests.testutils.lambda.any import io.element.android.tests.testutils.lambda.lambdaRecorder import io.element.android.tests.testutils.lambda.value import io.mockk.mockk import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.test.advanceUntilIdle import kotlinx.coroutines.test.runTest import org.junit.Rule import org.junit.Test @@ -67,7 +71,11 @@ class AttachmentsPreviewPresenterTest { ), sendFileResult = sendFileResult, ) - val presenter = createAttachmentsPreviewPresenter(room = room) + val onDoneListener = lambdaRecorder { } + val presenter = createAttachmentsPreviewPresenter( + room = room, + onDoneListener = { onDoneListener() }, + ) moleculeFlow(RecompositionMode.Immediate) { presenter.present() }.test { @@ -78,9 +86,28 @@ class AttachmentsPreviewPresenterTest { assertThat(awaitItem().sendActionState).isEqualTo(SendActionState.Sending.Uploading(0f)) assertThat(awaitItem().sendActionState).isEqualTo(SendActionState.Sending.Uploading(0.5f)) assertThat(awaitItem().sendActionState).isEqualTo(SendActionState.Sending.Uploading(1f)) - val successState = awaitItem() - assertThat(successState.sendActionState).isEqualTo(SendActionState.Done) + advanceUntilIdle() sendFileResult.assertions().isCalledOnce() + onDoneListener.assertions().isCalledOnce() + } + } + + @Test + fun `present - cancel scenario`() = runTest { + val onDoneListener = lambdaRecorder { } + val deleteCallback = lambdaRecorder {} + val presenter = createAttachmentsPreviewPresenter( + temporaryUriDeleter = FakeTemporaryUriDeleter(deleteCallback), + onDoneListener = { onDoneListener() }, + ) + moleculeFlow(RecompositionMode.Immediate) { + presenter.present() + }.test { + val initialState = awaitItem() + assertThat(initialState.sendActionState).isEqualTo(SendActionState.Idle) + initialState.eventSink(AttachmentsPreviewEvents.Cancel) + deleteCallback.assertions().isCalledOnce() + onDoneListener.assertions().isCalledOnce() } } @@ -96,9 +123,11 @@ class AttachmentsPreviewPresenterTest { val room = FakeMatrixRoom( sendImageResult = sendImageResult, ) + val onDoneListener = lambdaRecorder { } val presenter = createAttachmentsPreviewPresenter( room = room, mediaPreProcessor = mediaPreProcessor, + onDoneListener = { onDoneListener() }, ) moleculeFlow(RecompositionMode.Immediate) { presenter.present() @@ -108,8 +137,7 @@ class AttachmentsPreviewPresenterTest { initialState.textEditorState.setMarkdown(A_CAPTION) initialState.eventSink(AttachmentsPreviewEvents.SendAttachment) assertThat(awaitItem().sendActionState).isEqualTo(SendActionState.Sending.Processing) - val successState = awaitItem() - assertThat(successState.sendActionState).isEqualTo(SendActionState.Done) + advanceUntilIdle() sendImageResult.assertions().isCalledOnce().with( any(), any(), @@ -118,6 +146,7 @@ class AttachmentsPreviewPresenterTest { any(), any(), ) + onDoneListener.assertions().isCalledOnce() } } @@ -133,9 +162,11 @@ class AttachmentsPreviewPresenterTest { val room = FakeMatrixRoom( sendVideoResult = sendVideoResult, ) + val onDoneListener = lambdaRecorder { } val presenter = createAttachmentsPreviewPresenter( room = room, mediaPreProcessor = mediaPreProcessor, + onDoneListener = { onDoneListener() }, ) moleculeFlow(RecompositionMode.Immediate) { presenter.present() @@ -145,8 +176,7 @@ class AttachmentsPreviewPresenterTest { initialState.textEditorState.setMarkdown(A_CAPTION) initialState.eventSink(AttachmentsPreviewEvents.SendAttachment) assertThat(awaitItem().sendActionState).isEqualTo(SendActionState.Sending.Processing) - val successState = awaitItem() - assertThat(successState.sendActionState).isEqualTo(SendActionState.Done) + advanceUntilIdle() sendVideoResult.assertions().isCalledOnce().with( any(), any(), @@ -155,6 +185,7 @@ class AttachmentsPreviewPresenterTest { any(), any(), ) + onDoneListener.assertions().isCalledOnce() } } @@ -207,11 +238,15 @@ class AttachmentsPreviewPresenterTest { room: MatrixRoom = FakeMatrixRoom(), permalinkBuilder: PermalinkBuilder = FakePermalinkBuilder(), mediaPreProcessor: MediaPreProcessor = FakeMediaPreProcessor(), + temporaryUriDeleter: TemporaryUriDeleter = FakeTemporaryUriDeleter(), + onDoneListener: OnDoneListener = OnDoneListener {}, ): AttachmentsPreviewPresenter { return AttachmentsPreviewPresenter( attachment = aMediaAttachment(localMedia), + onDoneListener = onDoneListener, mediaSender = MediaSender(mediaPreProcessor, room, InMemorySessionPreferencesStore()), permalinkBuilder = permalinkBuilder, + temporaryUriDeleter = temporaryUriDeleter, ) } } diff --git a/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/user/editprofile/EditUserProfilePresenter.kt b/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/user/editprofile/EditUserProfilePresenter.kt index 14d0bd7dcd2..0e5a05be561 100644 --- a/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/user/editprofile/EditUserProfilePresenter.kt +++ b/features/preferences/impl/src/main/kotlin/io/element/android/features/preferences/impl/user/editprofile/EditUserProfilePresenter.kt @@ -22,6 +22,7 @@ import androidx.core.net.toUri import dagger.assisted.Assisted import dagger.assisted.AssistedFactory import dagger.assisted.AssistedInject +import io.element.android.libraries.androidutils.file.TemporaryUriDeleter import io.element.android.libraries.architecture.AsyncAction import io.element.android.libraries.architecture.Presenter import io.element.android.libraries.architecture.runCatchingUpdatingState @@ -43,6 +44,7 @@ class EditUserProfilePresenter @AssistedInject constructor( private val matrixClient: MatrixClient, private val mediaPickerProvider: PickerProvider, private val mediaPreProcessor: MediaPreProcessor, + private val temporaryUriDeleter: TemporaryUriDeleter, permissionsPresenterFactory: PermissionsPresenter.Factory, ) : Presenter { private val cameraPermissionPresenter: PermissionsPresenter = permissionsPresenterFactory.create(android.Manifest.permission.CAMERA) @@ -59,10 +61,20 @@ class EditUserProfilePresenter @AssistedInject constructor( var userAvatarUri by rememberSaveable { mutableStateOf(matrixUser.avatarUrl?.let { Uri.parse(it) }) } var userDisplayName by rememberSaveable { mutableStateOf(matrixUser.displayName) } val cameraPhotoPicker = mediaPickerProvider.registerCameraPhotoPicker( - onResult = { uri -> if (uri != null) userAvatarUri = uri } + onResult = { uri -> + if (uri != null) { + temporaryUriDeleter.delete(userAvatarUri) + userAvatarUri = uri + } + } ) val galleryImagePicker = mediaPickerProvider.registerGalleryImagePicker( - onResult = { uri -> if (uri != null) userAvatarUri = uri } + onResult = { uri -> + if (uri != null) { + temporaryUriDeleter.delete(userAvatarUri) + userAvatarUri = uri + } + } ) val avatarActions by remember(userAvatarUri) { @@ -96,7 +108,10 @@ class EditUserProfilePresenter @AssistedInject constructor( pendingPermissionRequest = true cameraPermissionState.eventSink(PermissionsEvents.RequestPermissions) } - AvatarAction.Remove -> userAvatarUri = null + AvatarAction.Remove -> { + temporaryUriDeleter.delete(userAvatarUri) + userAvatarUri = null + } } } @@ -155,7 +170,12 @@ class EditUserProfilePresenter @AssistedInject constructor( private suspend fun updateAvatar(avatarUri: Uri?): Result { return runCatching { if (avatarUri != null) { - val preprocessed = mediaPreProcessor.process(avatarUri, MimeTypes.Jpeg, compressIfPossible = false).getOrThrow() + val preprocessed = mediaPreProcessor.process( + uri = avatarUri, + mimeType = MimeTypes.Jpeg, + deleteOriginal = false, + compressIfPossible = false, + ).getOrThrow() matrixClient.uploadAvatar(MimeTypes.Jpeg, preprocessed.file.readBytes()).getOrThrow() } else { matrixClient.removeAvatar().getOrThrow() diff --git a/features/preferences/impl/src/test/kotlin/io/element/android/features/preferences/impl/user/editprofile/EditUserProfilePresenterTest.kt b/features/preferences/impl/src/test/kotlin/io/element/android/features/preferences/impl/user/editprofile/EditUserProfilePresenterTest.kt index 5fae451e0fd..0f39ab491ce 100644 --- a/features/preferences/impl/src/test/kotlin/io/element/android/features/preferences/impl/user/editprofile/EditUserProfilePresenterTest.kt +++ b/features/preferences/impl/src/test/kotlin/io/element/android/features/preferences/impl/user/editprofile/EditUserProfilePresenterTest.kt @@ -12,6 +12,7 @@ import app.cash.molecule.RecompositionMode import app.cash.molecule.moleculeFlow import app.cash.turbine.test import com.google.common.truth.Truth.assertThat +import io.element.android.libraries.androidutils.file.TemporaryUriDeleter import io.element.android.libraries.architecture.AsyncAction import io.element.android.libraries.matrix.api.MatrixClient import io.element.android.libraries.matrix.api.user.MatrixUser @@ -29,6 +30,9 @@ import io.element.android.libraries.permissions.test.FakePermissionsPresenterFac import io.element.android.tests.testutils.WarmUpRule import io.element.android.tests.testutils.consumeItemsUntilPredicate import io.element.android.tests.testutils.consumeItemsUntilTimeout +import io.element.android.tests.testutils.fake.FakeTemporaryUriDeleter +import io.element.android.tests.testutils.lambda.lambdaRecorder +import io.element.android.tests.testutils.lambda.value import io.mockk.every import io.mockk.mockk import io.mockk.mockkStatic @@ -73,12 +77,14 @@ class EditUserProfilePresenterTest { matrixClient: MatrixClient = FakeMatrixClient(), matrixUser: MatrixUser = aMatrixUser(), permissionsPresenter: PermissionsPresenter = FakePermissionsPresenter(), + temporaryUriDeleter: TemporaryUriDeleter = FakeTemporaryUriDeleter(), ): EditUserProfilePresenter { return EditUserProfilePresenter( matrixClient = matrixClient, matrixUser = matrixUser, mediaPickerProvider = fakePickerProvider, mediaPreProcessor = fakeMediaPreProcessor, + temporaryUriDeleter = temporaryUriDeleter, permissionsPresenterFactory = FakePermissionsPresenterFactory(permissionsPresenter), ) } @@ -107,7 +113,12 @@ class EditUserProfilePresenterTest { @Test fun `present - updates state in response to changes`() = runTest { val user = aMatrixUser(id = A_USER_ID.value, displayName = "Name", avatarUrl = AN_AVATAR_URL) - val presenter = createEditUserProfilePresenter(matrixUser = user) + val presenter = createEditUserProfilePresenter( + matrixUser = user, + temporaryUriDeleter = FakeTemporaryUriDeleter( + deleteLambda = { assertThat(it).isEqualTo(userAvatarUri) } + ), + ) moleculeFlow(RecompositionMode.Immediate) { presenter.present() }.test { @@ -136,7 +147,12 @@ class EditUserProfilePresenterTest { fun `present - obtains avatar uris from gallery`() = runTest { val user = aMatrixUser(id = A_USER_ID.value, displayName = "Name", avatarUrl = AN_AVATAR_URL) fakePickerProvider.givenResult(anotherAvatarUri) - val presenter = createEditUserProfilePresenter(matrixUser = user) + val presenter = createEditUserProfilePresenter( + matrixUser = user, + temporaryUriDeleter = FakeTemporaryUriDeleter( + deleteLambda = { assertThat(it).isEqualTo(userAvatarUri) } + ), + ) moleculeFlow(RecompositionMode.Immediate) { presenter.present() }.test { @@ -154,9 +170,13 @@ class EditUserProfilePresenterTest { val user = aMatrixUser(id = A_USER_ID.value, displayName = "Name", avatarUrl = AN_AVATAR_URL) fakePickerProvider.givenResult(anotherAvatarUri) val fakePermissionsPresenter = FakePermissionsPresenter() + val deleteCallback = lambdaRecorder {} val presenter = createEditUserProfilePresenter( matrixUser = user, permissionsPresenter = fakePermissionsPresenter, + temporaryUriDeleter = FakeTemporaryUriDeleter( + deleteLambda = deleteCallback, + ), ) moleculeFlow(RecompositionMode.Immediate) { presenter.present() @@ -177,6 +197,10 @@ class EditUserProfilePresenterTest { stateWithNewAvatar.eventSink(EditUserProfileEvents.HandleAvatarAction(AvatarAction.TakePhoto)) val stateWithNewAvatar2 = awaitItem() assertThat(stateWithNewAvatar2.userAvatarUrl).isEqualTo(userAvatarUri) + deleteCallback.assertions().isCalledExactly(2).withSequence( + listOf(value(userAvatarUri)), + listOf(value(anotherAvatarUri)), + ) } } @@ -184,7 +208,13 @@ class EditUserProfilePresenterTest { fun `present - updates save button state`() = runTest { val user = aMatrixUser(id = A_USER_ID.value, displayName = "Name", avatarUrl = AN_AVATAR_URL) fakePickerProvider.givenResult(userAvatarUri) - val presenter = createEditUserProfilePresenter(matrixUser = user) + val deleteCallback = lambdaRecorder {} + val presenter = createEditUserProfilePresenter( + matrixUser = user, + temporaryUriDeleter = FakeTemporaryUriDeleter( + deleteLambda = deleteCallback + ), + ) moleculeFlow(RecompositionMode.Immediate) { presenter.present() }.test { @@ -210,6 +240,10 @@ class EditUserProfilePresenterTest { awaitItem().apply { assertThat(saveButtonEnabled).isFalse() } + deleteCallback.assertions().isCalledExactly(2).withSequence( + listOf(value(userAvatarUri)), + listOf(value(null)), + ) } } @@ -217,7 +251,13 @@ class EditUserProfilePresenterTest { fun `present - updates save button state when initial values are null`() = runTest { val user = aMatrixUser(id = A_USER_ID.value, displayName = "Name", avatarUrl = null) fakePickerProvider.givenResult(userAvatarUri) - val presenter = createEditUserProfilePresenter(matrixUser = user) + val deleteCallback = lambdaRecorder {} + val presenter = createEditUserProfilePresenter( + matrixUser = user, + temporaryUriDeleter = FakeTemporaryUriDeleter( + deleteLambda = deleteCallback + ), + ) moleculeFlow(RecompositionMode.Immediate) { presenter.present() }.test { @@ -243,6 +283,10 @@ class EditUserProfilePresenterTest { awaitItem().apply { assertThat(saveButtonEnabled).isFalse() } + deleteCallback.assertions().isCalledExactly(2).withSequence( + listOf(value(null)), + listOf(value(userAvatarUri)), + ) } } @@ -252,7 +296,10 @@ class EditUserProfilePresenterTest { val user = aMatrixUser(id = A_USER_ID.value, displayName = "Name", avatarUrl = AN_AVATAR_URL) val presenter = createEditUserProfilePresenter( matrixClient = matrixClient, - matrixUser = user + matrixUser = user, + temporaryUriDeleter = FakeTemporaryUriDeleter( + deleteLambda = { assertThat(it).isEqualTo(userAvatarUri) } + ), ) moleculeFlow(RecompositionMode.Immediate) { presenter.present() @@ -318,7 +365,10 @@ class EditUserProfilePresenterTest { givenPickerReturnsFile() val presenter = createEditUserProfilePresenter( matrixClient = matrixClient, - matrixUser = user + matrixUser = user, + temporaryUriDeleter = FakeTemporaryUriDeleter( + deleteLambda = { assertThat(it).isEqualTo(userAvatarUri) } + ), ) moleculeFlow(RecompositionMode.Immediate) { presenter.present() @@ -337,7 +387,10 @@ class EditUserProfilePresenterTest { val user = aMatrixUser(id = A_USER_ID.value, displayName = "Name", avatarUrl = AN_AVATAR_URL) val presenter = createEditUserProfilePresenter( matrixClient = matrixClient, - matrixUser = user + matrixUser = user, + temporaryUriDeleter = FakeTemporaryUriDeleter( + deleteLambda = { assertThat(it).isEqualTo(userAvatarUri) } + ), ) fakePickerProvider.givenResult(anotherAvatarUri) fakeMediaPreProcessor.givenResult(Result.failure(Throwable("Oh no"))) @@ -403,7 +456,13 @@ class EditUserProfilePresenterTest { } private suspend fun saveAndAssertFailure(matrixUser: MatrixUser, matrixClient: MatrixClient, event: EditUserProfileEvents) { - val presenter = createEditUserProfilePresenter(matrixUser = matrixUser, matrixClient = matrixClient) + val presenter = createEditUserProfilePresenter( + matrixUser = matrixUser, + matrixClient = matrixClient, + temporaryUriDeleter = FakeTemporaryUriDeleter( + deleteLambda = { assertThat(it).isEqualTo(userAvatarUri) } + ), + ) moleculeFlow(RecompositionMode.Immediate) { presenter.present() }.test { diff --git a/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/edit/RoomDetailsEditPresenter.kt b/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/edit/RoomDetailsEditPresenter.kt index 8ad7eed8f4d..df220f0d501 100644 --- a/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/edit/RoomDetailsEditPresenter.kt +++ b/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/edit/RoomDetailsEditPresenter.kt @@ -20,6 +20,7 @@ import androidx.compose.runtime.rememberCoroutineScope import androidx.compose.runtime.saveable.rememberSaveable import androidx.compose.runtime.setValue import androidx.core.net.toUri +import io.element.android.libraries.androidutils.file.TemporaryUriDeleter import io.element.android.libraries.architecture.AsyncAction import io.element.android.libraries.architecture.Presenter import io.element.android.libraries.architecture.runCatchingUpdatingState @@ -45,6 +46,7 @@ class RoomDetailsEditPresenter @Inject constructor( private val room: MatrixRoom, private val mediaPickerProvider: PickerProvider, private val mediaPreProcessor: MediaPreProcessor, + private val temporaryUriDeleter: TemporaryUriDeleter, permissionsPresenterFactory: PermissionsPresenter.Factory, ) : Presenter { private val cameraPermissionPresenter = permissionsPresenterFactory.create(android.Manifest.permission.CAMERA) @@ -59,6 +61,7 @@ class RoomDetailsEditPresenter @Inject constructor( var roomAvatarUriEdited by rememberSaveable { mutableStateOf(null) } LaunchedEffect(roomAvatarUri) { // Every time the roomAvatar change (from sync), we can set the new avatar. + temporaryUriDeleter.delete(roomAvatarUriEdited) roomAvatarUriEdited = roomAvatarUri } @@ -98,10 +101,20 @@ class RoomDetailsEditPresenter @Inject constructor( } val cameraPhotoPicker = mediaPickerProvider.registerCameraPhotoPicker( - onResult = { uri -> if (uri != null) roomAvatarUriEdited = uri } + onResult = { uri -> + if (uri != null) { + temporaryUriDeleter.delete(roomAvatarUriEdited) + roomAvatarUriEdited = uri + } + } ) val galleryImagePicker = mediaPickerProvider.registerGalleryImagePicker( - onResult = { uri -> if (uri != null) roomAvatarUriEdited = uri } + onResult = { uri -> + if (uri != null) { + temporaryUriDeleter.delete(roomAvatarUriEdited) + roomAvatarUriEdited = uri + } + } ) LaunchedEffect(cameraPermissionState.permissionGranted) { @@ -143,7 +156,10 @@ class RoomDetailsEditPresenter @Inject constructor( pendingPermissionRequest = true cameraPermissionState.eventSink(PermissionsEvents.RequestPermissions) } - AvatarAction.Remove -> roomAvatarUriEdited = null + AvatarAction.Remove -> { + temporaryUriDeleter.delete(roomAvatarUriEdited) + roomAvatarUriEdited = null + } } } @@ -202,7 +218,12 @@ class RoomDetailsEditPresenter @Inject constructor( private suspend fun updateAvatar(avatarUri: Uri?): Result { return runCatching { if (avatarUri != null) { - val preprocessed = mediaPreProcessor.process(avatarUri, MimeTypes.Jpeg, compressIfPossible = false).getOrThrow() + val preprocessed = mediaPreProcessor.process( + uri = avatarUri, + mimeType = MimeTypes.Jpeg, + deleteOriginal = false, + compressIfPossible = false, + ).getOrThrow() room.updateAvatar(MimeTypes.Jpeg, preprocessed.file.readBytes()).getOrThrow() } else { room.removeAvatar().getOrThrow() diff --git a/features/roomdetails/impl/src/test/kotlin/io/element/android/features/roomdetails/edit/RoomDetailsEditPresenterTest.kt b/features/roomdetails/impl/src/test/kotlin/io/element/android/features/roomdetails/edit/RoomDetailsEditPresenterTest.kt index 3b40e3bf3cf..b1edabbbff3 100644 --- a/features/roomdetails/impl/src/test/kotlin/io/element/android/features/roomdetails/edit/RoomDetailsEditPresenterTest.kt +++ b/features/roomdetails/impl/src/test/kotlin/io/element/android/features/roomdetails/edit/RoomDetailsEditPresenterTest.kt @@ -8,14 +8,12 @@ package io.element.android.features.roomdetails.edit import android.net.Uri -import app.cash.molecule.RecompositionMode -import app.cash.molecule.moleculeFlow import app.cash.turbine.ReceiveTurbine -import app.cash.turbine.test import com.google.common.truth.Truth.assertThat import io.element.android.features.roomdetails.aMatrixRoom import io.element.android.features.roomdetails.impl.edit.RoomDetailsEditEvents import io.element.android.features.roomdetails.impl.edit.RoomDetailsEditPresenter +import io.element.android.libraries.androidutils.file.TemporaryUriDeleter import io.element.android.libraries.architecture.AsyncAction import io.element.android.libraries.core.mimetype.MimeTypes import io.element.android.libraries.matrix.api.room.MatrixRoom @@ -31,9 +29,11 @@ import io.element.android.libraries.permissions.api.PermissionsPresenter import io.element.android.libraries.permissions.test.FakePermissionsPresenter import io.element.android.libraries.permissions.test.FakePermissionsPresenterFactory import io.element.android.tests.testutils.WarmUpRule +import io.element.android.tests.testutils.fake.FakeTemporaryUriDeleter import io.element.android.tests.testutils.lambda.lambdaError import io.element.android.tests.testutils.lambda.lambdaRecorder import io.element.android.tests.testutils.lambda.value +import io.element.android.tests.testutils.test import io.mockk.every import io.mockk.mockk import io.mockk.mockkStatic @@ -46,6 +46,7 @@ import org.junit.Rule import org.junit.Test import java.io.File +@Suppress("LargeClass") @ExperimentalCoroutinesApi class RoomDetailsEditPresenterTest { @get:Rule @@ -77,12 +78,14 @@ class RoomDetailsEditPresenterTest { private fun createRoomDetailsEditPresenter( room: MatrixRoom, permissionsPresenter: PermissionsPresenter = FakePermissionsPresenter(), + temporaryUriDeleter: TemporaryUriDeleter = FakeTemporaryUriDeleter(), ): RoomDetailsEditPresenter { return RoomDetailsEditPresenter( room = room, mediaPickerProvider = fakePickerProvider, mediaPreProcessor = fakeMediaPreProcessor, permissionsPresenterFactory = FakePermissionsPresenterFactory(permissionsPresenter), + temporaryUriDeleter = temporaryUriDeleter, ) } @@ -95,10 +98,12 @@ class RoomDetailsEditPresenterTest { emitRoomInfo = true, canSendStateResult = { _, _ -> Result.success(true) } ) - val presenter = createRoomDetailsEditPresenter(room) - moleculeFlow(RecompositionMode.Immediate) { - presenter.present() - }.test { + val deleteCallback = lambdaRecorder {} + val presenter = createRoomDetailsEditPresenter( + room = room, + temporaryUriDeleter = FakeTemporaryUriDeleter(deleteCallback), + ) + presenter.test { val initialState = awaitFirstItem() assertThat(initialState.roomId).isEqualTo(room.roomId) assertThat(initialState.roomRawName).isEqualTo(A_ROOM_RAW_NAME) @@ -127,10 +132,12 @@ class RoomDetailsEditPresenterTest { } }, ) - val presenter = createRoomDetailsEditPresenter(room) - moleculeFlow(RecompositionMode.Immediate) { - presenter.present() - }.test { + val deleteCallback = lambdaRecorder {} + val presenter = createRoomDetailsEditPresenter( + room = room, + temporaryUriDeleter = FakeTemporaryUriDeleter(deleteCallback), + ) + presenter.test { // Initially false val initialState = awaitItem() assertThat(initialState.canChangeName).isFalse() @@ -141,6 +148,7 @@ class RoomDetailsEditPresenterTest { assertThat(settledState.canChangeName).isTrue() assertThat(settledState.canChangeAvatar).isFalse() assertThat(settledState.canChangeTopic).isFalse() + deleteCallback.assertions().isCalledOnce().with(value(null)) } } @@ -157,10 +165,12 @@ class RoomDetailsEditPresenterTest { } } ) - val presenter = createRoomDetailsEditPresenter(room) - moleculeFlow(RecompositionMode.Immediate) { - presenter.present() - }.test { + val deleteCallback = lambdaRecorder {} + val presenter = createRoomDetailsEditPresenter( + room = room, + temporaryUriDeleter = FakeTemporaryUriDeleter(deleteCallback), + ) + presenter.test { // Initially false val initialState = awaitItem() assertThat(initialState.canChangeName).isFalse() @@ -187,10 +197,12 @@ class RoomDetailsEditPresenterTest { } } ) - val presenter = createRoomDetailsEditPresenter(room) - moleculeFlow(RecompositionMode.Immediate) { - presenter.present() - }.test { + val deleteCallback = lambdaRecorder {} + val presenter = createRoomDetailsEditPresenter( + room = room, + temporaryUriDeleter = FakeTemporaryUriDeleter(deleteCallback), + ) + presenter.test { // Initially false val initialState = awaitItem() assertThat(initialState.canChangeName).isFalse() @@ -213,10 +225,12 @@ class RoomDetailsEditPresenterTest { emitRoomInfo = true, canSendStateResult = { _, _ -> Result.success(true) } ) - val presenter = createRoomDetailsEditPresenter(room) - moleculeFlow(RecompositionMode.Immediate) { - presenter.present() - }.test { + val deleteCallback = lambdaRecorder {} + val presenter = createRoomDetailsEditPresenter( + room = room, + temporaryUriDeleter = FakeTemporaryUriDeleter(deleteCallback), + ) + presenter.test { val initialState = awaitFirstItem() assertThat(initialState.roomTopic).isEqualTo("My topic") assertThat(initialState.roomRawName).isEqualTo("Name") @@ -258,10 +272,12 @@ class RoomDetailsEditPresenterTest { canSendStateResult = { _, _ -> Result.success(true) } ) fakePickerProvider.givenResult(anotherAvatarUri) - val presenter = createRoomDetailsEditPresenter(room) - moleculeFlow(RecompositionMode.Immediate) { - presenter.present() - }.test { + val deleteCallback = lambdaRecorder {} + val presenter = createRoomDetailsEditPresenter( + room = room, + temporaryUriDeleter = FakeTemporaryUriDeleter(deleteCallback), + ) + presenter.test { val initialState = awaitFirstItem() assertThat(initialState.roomAvatarUrl).isEqualTo(roomAvatarUri) initialState.eventSink(RoomDetailsEditEvents.HandleAvatarAction(AvatarAction.ChoosePhoto)) @@ -282,13 +298,13 @@ class RoomDetailsEditPresenterTest { ) fakePickerProvider.givenResult(anotherAvatarUri) val fakePermissionsPresenter = FakePermissionsPresenter() + val deleteCallback = lambdaRecorder {} val presenter = createRoomDetailsEditPresenter( room = room, permissionsPresenter = fakePermissionsPresenter, + temporaryUriDeleter = FakeTemporaryUriDeleter(deleteCallback), ) - moleculeFlow(RecompositionMode.Immediate) { - presenter.present() - }.test { + presenter.test { val initialState = awaitFirstItem() assertThat(initialState.roomAvatarUrl).isEqualTo(roomAvatarUri) assertThat(initialState.cameraPermissionState.permissionGranted).isFalse() @@ -305,6 +321,12 @@ class RoomDetailsEditPresenterTest { stateWithNewAvatar.eventSink(RoomDetailsEditEvents.HandleAvatarAction(AvatarAction.TakePhoto)) val stateWithNewAvatar2 = awaitItem() assertThat(stateWithNewAvatar2.roomAvatarUrl).isEqualTo(roomAvatarUri) + deleteCallback.assertions().isCalledExactly(4).withSequence( + listOf(value(null)), + listOf(value(null)), + listOf(value(roomAvatarUri)), + listOf(value(anotherAvatarUri)), + ) } } @@ -318,10 +340,12 @@ class RoomDetailsEditPresenterTest { canSendStateResult = { _, _ -> Result.success(true) } ) fakePickerProvider.givenResult(roomAvatarUri) - val presenter = createRoomDetailsEditPresenter(room) - moleculeFlow(RecompositionMode.Immediate) { - presenter.present() - }.test { + val deleteCallback = lambdaRecorder {} + val presenter = createRoomDetailsEditPresenter( + room = room, + temporaryUriDeleter = FakeTemporaryUriDeleter(deleteCallback), + ) + presenter.test { val initialState = awaitFirstItem() assertThat(initialState.saveButtonEnabled).isFalse() // Once a change is made, the save button is enabled @@ -367,10 +391,12 @@ class RoomDetailsEditPresenterTest { canSendStateResult = { _, _ -> Result.success(true) } ) fakePickerProvider.givenResult(roomAvatarUri) - val presenter = createRoomDetailsEditPresenter(room) - moleculeFlow(RecompositionMode.Immediate) { - presenter.present() - }.test { + val deleteCallback = lambdaRecorder {} + val presenter = createRoomDetailsEditPresenter( + room = room, + temporaryUriDeleter = FakeTemporaryUriDeleter(deleteCallback), + ) + presenter.test { val initialState = awaitFirstItem() assertThat(initialState.saveButtonEnabled).isFalse() // Once a change is made, the save button is enabled @@ -421,10 +447,12 @@ class RoomDetailsEditPresenterTest { removeAvatarResult = removeAvatarResult, canSendStateResult = { _, _ -> Result.success(true) } ) - val presenter = createRoomDetailsEditPresenter(room) - moleculeFlow(RecompositionMode.Immediate) { - presenter.present() - }.test { + val deleteCallback = lambdaRecorder {} + val presenter = createRoomDetailsEditPresenter( + room = room, + temporaryUriDeleter = FakeTemporaryUriDeleter(deleteCallback), + ) + presenter.test { val initialState = awaitFirstItem() initialState.eventSink(RoomDetailsEditEvents.UpdateRoomName("New name")) initialState.eventSink(RoomDetailsEditEvents.UpdateRoomTopic("New topic")) @@ -445,10 +473,12 @@ class RoomDetailsEditPresenterTest { avatarUrl = AN_AVATAR_URL, canSendStateResult = { _, _ -> Result.success(true) } ) - val presenter = createRoomDetailsEditPresenter(room) - moleculeFlow(RecompositionMode.Immediate) { - presenter.present() - }.test { + val deleteCallback = lambdaRecorder {} + val presenter = createRoomDetailsEditPresenter( + room = room, + temporaryUriDeleter = FakeTemporaryUriDeleter(deleteCallback), + ) + presenter.test { val initialState = awaitItem() initialState.eventSink(RoomDetailsEditEvents.UpdateRoomName(" Name ")) initialState.eventSink(RoomDetailsEditEvents.UpdateRoomTopic(" My topic ")) @@ -465,14 +495,17 @@ class RoomDetailsEditPresenterTest { avatarUrl = AN_AVATAR_URL, canSendStateResult = { _, _ -> Result.success(true) } ) - val presenter = createRoomDetailsEditPresenter(room) - moleculeFlow(RecompositionMode.Immediate) { - presenter.present() - }.test { + val deleteCallback = lambdaRecorder {} + val presenter = createRoomDetailsEditPresenter( + room = room, + temporaryUriDeleter = FakeTemporaryUriDeleter(deleteCallback), + ) + presenter.test { val initialState = awaitItem() initialState.eventSink(RoomDetailsEditEvents.UpdateRoomTopic("")) initialState.eventSink(RoomDetailsEditEvents.Save) cancelAndIgnoreRemainingEvents() + deleteCallback.assertions().isCalledOnce().with(value(null)) } } @@ -484,14 +517,17 @@ class RoomDetailsEditPresenterTest { avatarUrl = AN_AVATAR_URL, canSendStateResult = { _, _ -> Result.success(true) } ) - val presenter = createRoomDetailsEditPresenter(room) - moleculeFlow(RecompositionMode.Immediate) { - presenter.present() - }.test { + val deleteCallback = lambdaRecorder {} + val presenter = createRoomDetailsEditPresenter( + room = room, + temporaryUriDeleter = FakeTemporaryUriDeleter(deleteCallback), + ) + presenter.test { val initialState = awaitItem() initialState.eventSink(RoomDetailsEditEvents.UpdateRoomName("")) initialState.eventSink(RoomDetailsEditEvents.Save) cancelAndIgnoreRemainingEvents() + deleteCallback.assertions().isCalledOnce().with(value(null)) } } @@ -506,15 +542,21 @@ class RoomDetailsEditPresenterTest { canSendStateResult = { _, _ -> Result.success(true) } ) givenPickerReturnsFile() - val presenter = createRoomDetailsEditPresenter(room) - moleculeFlow(RecompositionMode.Immediate) { - presenter.present() - }.test { + val deleteCallback = lambdaRecorder {} + val presenter = createRoomDetailsEditPresenter( + room = room, + temporaryUriDeleter = FakeTemporaryUriDeleter(deleteCallback), + ) + presenter.test { val initialState = awaitItem() initialState.eventSink(RoomDetailsEditEvents.HandleAvatarAction(AvatarAction.ChoosePhoto)) initialState.eventSink(RoomDetailsEditEvents.Save) skipItems(4) updateAvatarResult.assertions().isCalledOnce().with(value(MimeTypes.Jpeg), value(fakeFileContents)) + deleteCallback.assertions().isCalledExactly(2).withSequence( + listOf(value(null)), + listOf(value(null)), + ) } } @@ -528,10 +570,12 @@ class RoomDetailsEditPresenterTest { ) fakePickerProvider.givenResult(anotherAvatarUri) fakeMediaPreProcessor.givenResult(Result.failure(Throwable("Oh no"))) - val presenter = createRoomDetailsEditPresenter(room) - moleculeFlow(RecompositionMode.Immediate) { - presenter.present() - }.test { + val deleteCallback = lambdaRecorder {} + val presenter = createRoomDetailsEditPresenter( + room = room, + temporaryUriDeleter = FakeTemporaryUriDeleter(deleteCallback), + ) + presenter.test { val initialState = awaitItem() initialState.eventSink(RoomDetailsEditEvents.HandleAvatarAction(AvatarAction.ChoosePhoto)) initialState.eventSink(RoomDetailsEditEvents.Save) @@ -576,7 +620,7 @@ class RoomDetailsEditPresenterTest { removeAvatarResult = { Result.failure(Throwable("!")) }, canSendStateResult = { _, _ -> Result.success(true) } ) - saveAndAssertFailure(room, RoomDetailsEditEvents.HandleAvatarAction(AvatarAction.Remove)) + saveAndAssertFailure(room, RoomDetailsEditEvents.HandleAvatarAction(AvatarAction.Remove), deleteCallbackNumberOfInvocation = 3) } @Test @@ -590,7 +634,7 @@ class RoomDetailsEditPresenterTest { updateAvatarResult = { _, _ -> Result.failure(Throwable("!")) }, canSendStateResult = { _, _ -> Result.success(true) } ) - saveAndAssertFailure(room, RoomDetailsEditEvents.HandleAvatarAction(AvatarAction.ChoosePhoto)) + saveAndAssertFailure(room, RoomDetailsEditEvents.HandleAvatarAction(AvatarAction.ChoosePhoto), deleteCallbackNumberOfInvocation = 3) } @Test @@ -603,10 +647,12 @@ class RoomDetailsEditPresenterTest { setTopicResult = { Result.failure(Throwable("!")) }, canSendStateResult = { _, _ -> Result.success(true) } ) - val presenter = createRoomDetailsEditPresenter(room) - moleculeFlow(RecompositionMode.Immediate) { - presenter.present() - }.test { + val deleteCallback = lambdaRecorder {} + val presenter = createRoomDetailsEditPresenter( + room = room, + temporaryUriDeleter = FakeTemporaryUriDeleter(deleteCallback), + ) + presenter.test { val initialState = awaitItem() initialState.eventSink(RoomDetailsEditEvents.UpdateRoomTopic("foo")) initialState.eventSink(RoomDetailsEditEvents.Save) @@ -617,17 +663,24 @@ class RoomDetailsEditPresenterTest { } } - private suspend fun saveAndAssertFailure(room: MatrixRoom, event: RoomDetailsEditEvents) { - val presenter = createRoomDetailsEditPresenter(room) - moleculeFlow(RecompositionMode.Immediate) { - presenter.present() - }.test { + private suspend fun saveAndAssertFailure( + room: MatrixRoom, + event: RoomDetailsEditEvents, + deleteCallbackNumberOfInvocation: Int = 2, + ) { + val deleteCallback = lambdaRecorder {} + val presenter = createRoomDetailsEditPresenter( + room = room, + temporaryUriDeleter = FakeTemporaryUriDeleter(deleteCallback), + ) + presenter.test { val initialState = awaitFirstItem() initialState.eventSink(event) initialState.eventSink(RoomDetailsEditEvents.Save) skipItems(1) assertThat(awaitItem().saveAction).isInstanceOf(AsyncAction.Loading::class.java) assertThat(awaitItem().saveAction).isInstanceOf(AsyncAction.Failure::class.java) + deleteCallback.assertions().isCalledExactly(deleteCallbackNumberOfInvocation) } } diff --git a/libraries/androidutils/src/main/kotlin/io/element/android/libraries/androidutils/file/TemporaryUriDeleter.kt b/libraries/androidutils/src/main/kotlin/io/element/android/libraries/androidutils/file/TemporaryUriDeleter.kt new file mode 100644 index 00000000000..4eabda8972c --- /dev/null +++ b/libraries/androidutils/src/main/kotlin/io/element/android/libraries/androidutils/file/TemporaryUriDeleter.kt @@ -0,0 +1,39 @@ +/* + * Copyright 2024 New Vector Ltd. + * + * SPDX-License-Identifier: AGPL-3.0-only + * Please see LICENSE in the repository root for full details. + */ + +package io.element.android.libraries.androidutils.file + +import android.content.Context +import android.net.Uri +import com.squareup.anvil.annotations.ContributesBinding +import io.element.android.libraries.di.AppScope +import io.element.android.libraries.di.ApplicationContext +import timber.log.Timber +import javax.inject.Inject + +interface TemporaryUriDeleter { + /** + * Delete the Uri only if it is a temporary one. + */ + fun delete(uri: Uri?) +} + +@ContributesBinding(AppScope::class) +class DefaultTemporaryUriDeleter @Inject constructor( + @ApplicationContext private val context: Context, +) : TemporaryUriDeleter { + private val baseCacheUri = "content://${context.packageName}.fileprovider/cache" + + override fun delete(uri: Uri?) { + uri ?: return + if (uri.toString().startsWith(baseCacheUri)) { + context.contentResolver.delete(uri, null, null) + } else { + Timber.d("Do not delete the uri") + } + } +} 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 index 19b0a0700d1..75a4be1cf3d 100644 --- 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 @@ -18,8 +18,8 @@ interface MediaPreProcessor { suspend fun process( uri: Uri, mimeType: String, - deleteOriginal: Boolean = false, - compressIfPossible: Boolean + deleteOriginal: Boolean, + compressIfPossible: Boolean, ): Result 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 index 54f886d3028..09b8de4b41b 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 @@ -39,7 +39,7 @@ class MediaSender @Inject constructor( .process( uri = uri, mimeType = mimeType, - deleteOriginal = true, + deleteOriginal = false, compressIfPossible = compressIfPossible, ) .flatMapCatching { info -> 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 index 288ba988eff..54b880369b3 100644 --- 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 @@ -13,6 +13,7 @@ import android.media.MediaMetadataRetriever import android.net.Uri import androidx.exifinterface.media.ExifInterface import com.squareup.anvil.annotations.ContributesBinding +import io.element.android.libraries.androidutils.file.TemporaryUriDeleter import io.element.android.libraries.androidutils.file.createTmpFile import io.element.android.libraries.androidutils.file.getFileName import io.element.android.libraries.androidutils.file.safeRenameTo @@ -36,6 +37,7 @@ import kotlinx.coroutines.flow.filterIsInstance import kotlinx.coroutines.flow.first import kotlinx.coroutines.flow.onEach import kotlinx.coroutines.withContext +import timber.log.Timber import java.io.File import java.io.InputStream import javax.inject.Inject @@ -49,6 +51,7 @@ class AndroidMediaPreProcessor @Inject constructor( private val imageCompressor: ImageCompressor, private val videoCompressor: VideoCompressor, private val coroutineDispatchers: CoroutineDispatchers, + private val temporaryUriDeleter: TemporaryUriDeleter, ) : MediaPreProcessor { companion object { /** @@ -82,8 +85,11 @@ class AndroidMediaPreProcessor @Inject constructor( } if (deleteOriginal) { tryOrNull { + Timber.w("Deleting original uri $uri") contentResolver.delete(uri, null, null) } + } else { + temporaryUriDeleter.delete(uri) } result.postProcess(uri) } diff --git a/libraries/mediaupload/impl/src/test/kotlin/io/element/android/libraries/mediaupload/impl/AndroidMediaPreProcessorTest.kt b/libraries/mediaupload/impl/src/test/kotlin/io/element/android/libraries/mediaupload/impl/AndroidMediaPreProcessorTest.kt index 09e316bd2f2..a9e9c3f7c2e 100644 --- a/libraries/mediaupload/impl/src/test/kotlin/io/element/android/libraries/mediaupload/impl/AndroidMediaPreProcessorTest.kt +++ b/libraries/mediaupload/impl/src/test/kotlin/io/element/android/libraries/mediaupload/impl/AndroidMediaPreProcessorTest.kt @@ -8,10 +8,12 @@ package io.element.android.libraries.mediaupload.impl import android.content.Context +import android.net.Uri import android.os.Build import androidx.core.net.toUri import androidx.test.platform.app.InstrumentationRegistry import com.google.common.truth.Truth.assertThat +import io.element.android.libraries.androidutils.file.TemporaryUriDeleter import io.element.android.libraries.core.mimetype.MimeTypes import io.element.android.libraries.matrix.api.media.AudioInfo import io.element.android.libraries.matrix.api.media.FileInfo @@ -21,6 +23,8 @@ import io.element.android.libraries.matrix.api.media.VideoInfo import io.element.android.libraries.mediaupload.api.MediaPreProcessor import io.element.android.libraries.mediaupload.api.MediaUploadInfo import io.element.android.services.toolbox.test.sdk.FakeBuildVersionSdkIntProvider +import io.element.android.tests.testutils.fake.FakeTemporaryUriDeleter +import io.element.android.tests.testutils.lambda.lambdaRecorder import io.element.android.tests.testutils.testCoroutineDispatchers import kotlinx.coroutines.test.TestScope import kotlinx.coroutines.test.runTest @@ -42,7 +46,12 @@ class AndroidMediaPreProcessorTest { deleteOriginal: Boolean = false, ): MediaUploadInfo { val context = InstrumentationRegistry.getInstrumentation().context - val sut = createAndroidMediaPreProcessor(context, sdkIntVersion) + val deleteCallback = lambdaRecorder {} + val sut = createAndroidMediaPreProcessor( + context = context, + sdkIntVersion = sdkIntVersion, + temporaryUriDeleter = FakeTemporaryUriDeleter(deleteCallback), + ) val file = getFileFromAssets(context, asset.filename) val result = sut.process( uri = file.toUri(), @@ -52,6 +61,7 @@ class AndroidMediaPreProcessorTest { ) val data = result.getOrThrow() assertThat(data.file.path).endsWith(asset.filename) + deleteCallback.assertions().isCalledExactly(if (deleteOriginal) 0 else 1) return data } @@ -356,13 +366,15 @@ class AndroidMediaPreProcessorTest { private fun TestScope.createAndroidMediaPreProcessor( context: Context, - sdkIntVersion: Int = Build.VERSION_CODES.P + sdkIntVersion: Int = Build.VERSION_CODES.P, + temporaryUriDeleter: TemporaryUriDeleter = FakeTemporaryUriDeleter(), ) = AndroidMediaPreProcessor( context = context, thumbnailFactory = ThumbnailFactory(context, FakeBuildVersionSdkIntProvider(sdkIntVersion)), imageCompressor = ImageCompressor(context, testCoroutineDispatchers()), videoCompressor = VideoCompressor(context), coroutineDispatchers = testCoroutineDispatchers(), + temporaryUriDeleter = temporaryUriDeleter, ) @Throws(IOException::class) diff --git a/tests/testutils/src/main/kotlin/io/element/android/tests/testutils/fake/FakeTemporaryUriDeleter.kt b/tests/testutils/src/main/kotlin/io/element/android/tests/testutils/fake/FakeTemporaryUriDeleter.kt new file mode 100644 index 00000000000..bb14d2783d7 --- /dev/null +++ b/tests/testutils/src/main/kotlin/io/element/android/tests/testutils/fake/FakeTemporaryUriDeleter.kt @@ -0,0 +1,20 @@ +/* + * Copyright 2024 New Vector Ltd. + * + * SPDX-License-Identifier: AGPL-3.0-only + * Please see LICENSE in the repository root for full details. + */ + +package io.element.android.tests.testutils.fake + +import android.net.Uri +import io.element.android.libraries.androidutils.file.TemporaryUriDeleter +import io.element.android.tests.testutils.lambda.lambdaError + +class FakeTemporaryUriDeleter( + val deleteLambda: (uri: Uri?) -> Unit = { lambdaError() } +) : TemporaryUriDeleter { + override fun delete(uri: Uri?) { + deleteLambda(uri) + } +}