Media: make existing tests passes on MessagesPresenters

features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/media/local/LocalMedia.kt

@@ -30,5 +30,7 @@ data class LocalMedia(
     /**
      * This tries to convert the uri to a file if applicable, otherwise keep it as uri.
      */
-    @IgnoredOnParcel val model: Any = UriToFileMapper.map(uri) ?: uri
+    @IgnoredOnParcel val model: Any by lazy {
+        UriToFileMapper.map(uri) ?: uri
+    }
 }

features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/messagecomposer/MessageComposerPresenter.kt

@@ -28,6 +28,7 @@ import androidx.compose.runtime.rememberCoroutineScope
 import androidx.compose.runtime.saveable.rememberSaveable
 import androidx.compose.runtime.setValue
 import androidx.media3.common.MimeTypes
+import androidx.media3.common.util.UnstableApi
 import io.element.android.features.messages.impl.attachments.Attachment
 import io.element.android.features.messages.impl.attachments.preview.error.sendAttachmentError
 import io.element.android.features.messages.impl.media.local.LocalMediaFactory
@@ -70,33 +71,18 @@ class MessageComposerPresenter @Inject constructor(
             mutableStateOf<AttachmentsState>(AttachmentsState.None)
         }
 
-        fun handlePickedMedia(uri: Uri?, mimeType: String? = null, compressIfPossible: Boolean = true) {
+        val galleryMediaPicker = mediaPickerProvider.registerGalleryPicker { uri, mimeType ->
-            val localMedia = localMediaFactory.createFromUri(uri, mimeType)
+            handlePickedMedia(attachmentsState, uri, mimeType)
-            attachmentsState.value = if (localMedia == null) {
+        }
-                AttachmentsState.None
+        val filesPicker = mediaPickerProvider.registerFilePicker(AnyMimeTypes) { uri ->
-            } else {
+            handlePickedMedia(attachmentsState, uri, compressIfPossible = false)
-                val mediaAttachment = Attachment.Media(localMedia, compressIfPossible)
+        }
-                val isPreviewable = when {
+        val cameraPhotoPicker = mediaPickerProvider.registerCameraPhotoPicker { uri ->
-                    MimeTypes.isImage(localMedia.mimeType) -> true
+            handlePickedMedia(attachmentsState, uri, MimeTypes.IMAGE_JPEG)
-                    MimeTypes.isVideo(localMedia.mimeType) -> true
+        }
-                    MimeTypes.isAudio(localMedia.mimeType) -> true
+        val cameraVideoPicker = mediaPickerProvider.registerCameraVideoPicker { uri ->
-                    else -> false
+            handlePickedMedia(attachmentsState, uri, MimeTypes.VIDEO_MP4)
-                }
-                if (isPreviewable) {
-                    AttachmentsState.Previewing(persistentListOf(mediaAttachment))
-                } else {
-                    AttachmentsState.Sending(persistentListOf(mediaAttachment))
-                }
-            }
         }
-
-        val galleryMediaPicker = mediaPickerProvider.registerGalleryPicker(onResult = { uri, mimeType ->
-            handlePickedMedia(uri, mimeType)
-        })
-        val filesPicker = mediaPickerProvider.registerFilePicker(AnyMimeTypes, onResult = { handlePickedMedia(it, compressIfPossible = false) })
-        val cameraPhotoPicker = mediaPickerProvider.registerCameraPhotoPicker(onResult = { handlePickedMedia(it, MimeTypes.IMAGE_JPEG) })
-        val cameraVideoPicker = mediaPickerProvider.registerCameraVideoPicker(onResult = { handlePickedMedia(it, MimeTypes.VIDEO_MP4) })
-
         val isFullScreen = rememberSaveable {
             mutableStateOf(false)
         }
@@ -107,7 +93,7 @@ class MessageComposerPresenter @Inject constructor(
             mutableStateOf(MessageComposerMode.Normal(""))
         }
 
-    var showAttachmentSourcePicker: Boolean by remember { mutableStateOf(false) }
+        var showAttachmentSourcePicker: Boolean by remember { mutableStateOf(false) }
 
         LaunchedEffect(composerMode.value) {
             when (val modeValue = composerMode.value) {
@@ -134,23 +120,23 @@ class MessageComposerPresenter @Inject constructor(
 
                 is MessageComposerEvents.SendMessage -> appCoroutineScope.sendMessage(event.message, composerMode, text)
                 is MessageComposerEvents.SetMode -> composerMode.value = event.composerMode
-                MessageComposerEvents.AddAttachment -> localCoroutineScope.ifMediaPickersEnabled {
+                MessageComposerEvents.AddAttachment -> localCoroutineScope.launchIfMediaPickerEnabled {
                     showAttachmentSourcePicker = true
                 }
                 MessageComposerEvents.DismissAttachmentMenu -> showAttachmentSourcePicker = false
-                MessageComposerEvents.PickAttachmentSource.FromGallery -> localCoroutineScope.ifMediaPickersEnabled {
+                MessageComposerEvents.PickAttachmentSource.FromGallery -> localCoroutineScope.launchIfMediaPickerEnabled {
                     showAttachmentSourcePicker = false
                     galleryMediaPicker.launch()
                 }
-                MessageComposerEvents.PickAttachmentSource.FromFiles -> localCoroutineScope.ifMediaPickersEnabled {
+                MessageComposerEvents.PickAttachmentSource.FromFiles -> localCoroutineScope.launchIfMediaPickerEnabled {
                     showAttachmentSourcePicker = false
                     filesPicker.launch()
                 }
-                MessageComposerEvents.PickAttachmentSource.PhotoFromCamera -> localCoroutineScope.ifMediaPickersEnabled {
+                MessageComposerEvents.PickAttachmentSource.PhotoFromCamera -> localCoroutineScope.launchIfMediaPickerEnabled {
                     showAttachmentSourcePicker = false
                     cameraPhotoPicker.launch()
                 }
-                MessageComposerEvents.PickAttachmentSource.VideoFromCamera -> localCoroutineScope.ifMediaPickersEnabled {
+                MessageComposerEvents.PickAttachmentSource.VideoFromCamera -> localCoroutineScope.launchIfMediaPickerEnabled {
                     showAttachmentSourcePicker = false
                     cameraVideoPicker.launch()
                 }
@@ -167,7 +153,7 @@ class MessageComposerPresenter @Inject constructor(
         )
     }
 
-    private fun CoroutineScope.ifMediaPickersEnabled(action: suspend () -> Unit) = launch {
+    private fun CoroutineScope.launchIfMediaPickerEnabled(action: suspend () -> Unit) = launch {
         if (featureFlagService.isFeatureEnabled(FeatureFlags.ShowMediaUploadingFlow)) {
             action()
         }
@@ -213,6 +199,32 @@ class MessageComposerPresenter @Inject constructor(
         }
     }
 
+    @UnstableApi
+    private fun handlePickedMedia(
+        attachmentsState: MutableState<AttachmentsState>,
+        uri: Uri?,
+        mimeType: String? = null,
+        compressIfPossible: Boolean = true,
+    ) {
+        val localMedia = localMediaFactory.createFromUri(uri, mimeType)
+        attachmentsState.value = if (localMedia == null) {
+            AttachmentsState.None
+        } else {
+            val mediaAttachment = Attachment.Media(localMedia, compressIfPossible)
+            val isPreviewable = when {
+                MimeTypes.isImage(localMedia.mimeType) -> true
+                MimeTypes.isVideo(localMedia.mimeType) -> true
+                MimeTypes.isAudio(localMedia.mimeType) -> true
+                else -> false
+            }
+            if (isPreviewable) {
+                AttachmentsState.Previewing(persistentListOf(mediaAttachment))
+            } else {
+                AttachmentsState.Sending(persistentListOf(mediaAttachment))
+            }
+        }
+    }
+
     private suspend fun sendMedia(
         uri: Uri,
         mimeType: String,

features/messages/impl/src/test/kotlin/io/element/android/features/messages/MessagesPresenterTest.kt

@@ -26,6 +26,7 @@ import io.element.android.features.messages.impl.MessagesEvents
 import io.element.android.features.messages.impl.MessagesPresenter
 import io.element.android.features.messages.impl.actionlist.ActionListPresenter
 import io.element.android.features.messages.impl.actionlist.model.TimelineItemAction
+import io.element.android.features.messages.impl.media.local.FakeLocalMediaFactory
 import io.element.android.features.messages.impl.messagecomposer.MessageComposerPresenter
 import io.element.android.features.messages.impl.timeline.TimelinePresenter
 import io.element.android.features.networkmonitor.test.FakeNetworkMonitor
@@ -36,6 +37,7 @@ import io.element.android.libraries.matrix.test.AN_EVENT_ID
 import io.element.android.libraries.matrix.test.A_ROOM_ID
 import io.element.android.libraries.matrix.test.room.FakeMatrixRoom
 import io.element.android.libraries.mediapickers.test.FakePickerProvider
+import io.element.android.libraries.mediaupload.api.MediaSender
 import io.element.android.libraries.mediaupload.test.FakeMediaPreProcessor
 import io.element.android.libraries.textcomposer.MessageComposerMode
 import kotlinx.coroutines.test.TestScope
@@ -133,7 +135,8 @@ class MessagesPresenterTest {
             room = matrixRoom,
             mediaPickerProvider = FakePickerProvider(),
             featureFlagService = FakeFeatureFlagService(),
-            mediaPreProcessor = FakeMediaPreProcessor(),
+            localMediaFactory = FakeLocalMediaFactory(),
+            mediaSender = MediaSender(FakeMediaPreProcessor(),matrixRoom),
             snackbarDispatcher = SnackbarDispatcher(),
         )
         val timelinePresenter = TimelinePresenter(

features/messages/impl/src/test/kotlin/io/element/android/features/messages/textcomposer/MessageComposerPresenterTest.kt

@@ -21,7 +21,8 @@ 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.messages.impl.messagecomposer.AttachmentSourcePicker
+import io.element.android.features.messages.impl.media.local.FakeLocalMediaFactory
+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.MessageComposerPresenter
 import io.element.android.features.messages.impl.messagecomposer.MessageComposerState
@@ -44,13 +45,13 @@ import io.element.android.libraries.matrix.test.room.FakeMatrixRoom
 import io.element.android.libraries.mediapickers.api.PickerProvider
 import io.element.android.libraries.mediapickers.test.FakePickerProvider
 import io.element.android.libraries.mediaupload.api.MediaPreProcessor
+import io.element.android.libraries.mediaupload.api.MediaSender
 import io.element.android.libraries.mediaupload.api.MediaUploadInfo
 import io.element.android.libraries.mediaupload.api.ThumbnailProcessingInfo
 import io.element.android.libraries.mediaupload.test.FakeMediaPreProcessor
 import io.element.android.libraries.textcomposer.MessageComposerMode
 import io.mockk.mockk
 import kotlinx.coroutines.CoroutineScope
-import kotlinx.coroutines.runBlocking
 import kotlinx.coroutines.test.runCurrent
 import kotlinx.coroutines.test.runTest
 import org.junit.Test
@@ -61,13 +62,12 @@ class MessageComposerPresenterTest {
     private val pickerProvider = FakePickerProvider().apply {
         givenResult(mockk()) // Uri is not available in JVM, so the only way to have a non-null Uri is using Mockk
     }
-    private val featureFlagService = FakeFeatureFlagService().apply {
+    private val featureFlagService = FakeFeatureFlagService(
-        runBlocking {
+        mapOf(FeatureFlags.ShowMediaUploadingFlow.key to true)
-            setFeatureEnabled(FeatureFlags.ShowMediaUploadingFlow, true)
+    )
-        }
-    }
     private val mediaPreProcessor = FakeMediaPreProcessor()
     private val snackbarDispatcher = SnackbarDispatcher()
+    private val localMediaFactory = FakeLocalMediaFactory()
 
     @Test
     fun `present - initial state`() = runTest {
@@ -79,6 +79,8 @@ class MessageComposerPresenterTest {
             assertThat(initialState.isFullScreen).isFalse()
             assertThat(initialState.text).isEqualTo(StableCharSequence(""))
             assertThat(initialState.mode).isEqualTo(MessageComposerMode.Normal(""))
+            assertThat(initialState.showAttachmentSourcePicker).isFalse()
+            assertThat(initialState.attachmentsState).isEqualTo(AttachmentsState.None)
             assertThat(initialState.isSendButtonVisible).isFalse()
         }
     }
@@ -256,22 +258,9 @@ class MessageComposerPresenterTest {
             presenter.present()
         }.test {
             val initialState = awaitItem()
+            assertThat(initialState.showAttachmentSourcePicker).isEqualTo(false)
             initialState.eventSink(MessageComposerEvents.AddAttachment)
-
+            assertThat(awaitItem().showAttachmentSourcePicker).isEqualTo(true)
-            assertThat(awaitItem().showAttachmentSourcePicker).isEqualTo(AttachmentSourcePicker.AllMedia)
-        }
-    }
-
-    @Test
-    fun `present - Open camera attachments menu`() = runTest {
-        val presenter = createPresenter(this)
-        moleculeFlow(RecompositionClock.Immediate) {
-            presenter.present()
-        }.test {
-            val initialState = awaitItem()
-            initialState.eventSink(MessageComposerEvents.PickAttachmentSource.FromCamera)
-
-            assertThat(awaitItem().showAttachmentSourcePicker).isEqualTo(AttachmentSourcePicker.Camera)
         }
     }
 
@@ -286,7 +275,7 @@ class MessageComposerPresenterTest {
             skipItems(1)
 
             initialState.eventSink(MessageComposerEvents.DismissAttachmentMenu)
-            assertThat(awaitItem().showAttachmentSourcePicker).isNull()
+            assertThat(awaitItem().showAttachmentSourcePicker).isFalse()
         }
     }
 
@@ -326,9 +315,9 @@ class MessageComposerPresenterTest {
         }.test {
             val initialState = awaitItem()
             initialState.eventSink(MessageComposerEvents.PickAttachmentSource.FromGallery)
-            // Wait for the launched upload coroutine to run
+            val previewingState = awaitItem()
-            runCurrent()
+            assertThat(previewingState.showAttachmentSourcePicker).isFalse()
-            assertThat(room.sendMediaCount).isEqualTo(1)
+            assertThat(previewingState.attachmentsState).isInstanceOf(AttachmentsState.Previewing::class.java)
         }
     }
 
@@ -369,22 +358,9 @@ class MessageComposerPresenterTest {
         }.test {
             val initialState = awaitItem()
             initialState.eventSink(MessageComposerEvents.PickAttachmentSource.FromGallery)
-            // Wait for the launched upload coroutine to run
+            val previewingState = awaitItem()
-            runCurrent()
+            assertThat(previewingState.showAttachmentSourcePicker).isFalse()
-            assertThat(room.sendMediaCount).isEqualTo(1)
+            assertThat(previewingState.attachmentsState).isInstanceOf(AttachmentsState.Previewing::class.java)
-        }
-    }
-
-    @Test
-    fun `present - Pick media from gallery fails if returned mimetype is not video or image`() = runTest {
-        val presenter = createPresenter(this)
-        pickerProvider.givenMimeType(MimeTypes.Audio)
-        moleculeFlow(RecompositionClock.Immediate) {
-            presenter.present()
-        }.test {
-            val initialState = awaitItem()
-            initialState.eventSink(MessageComposerEvents.PickAttachmentSource.FromGallery)
-            assertThat(awaitError()).isInstanceOf(IllegalStateException::class.java)
         }
     }
 
@@ -413,9 +389,10 @@ class MessageComposerPresenterTest {
         }.test {
             val initialState = awaitItem()
             initialState.eventSink(MessageComposerEvents.PickAttachmentSource.FromFiles)
-            // Wait for the launched upload coroutine to run
+            val sendingState = awaitItem()
-            runCurrent()
+            assertThat(sendingState.showAttachmentSourcePicker).isFalse()
-            assertThat(room.sendMediaCount).isEqualTo(1)
+            assertThat(sendingState.attachmentsState).isInstanceOf(AttachmentsState.Sending::class.java)
+            cancelAndIgnoreRemainingEvents()
         }
     }
 
@@ -427,10 +404,11 @@ class MessageComposerPresenterTest {
             presenter.present()
         }.test {
             val initialState = awaitItem()
-            initialState.eventSink(MessageComposerEvents.PickCameraAttachmentSource.Photo)
+            initialState.eventSink(MessageComposerEvents.PickAttachmentSource.PhotoFromCamera)
-            // Wait for the launched upload coroutine to run
+            val previewingState = awaitItem()
-            runCurrent()
+            assertThat(previewingState.showAttachmentSourcePicker).isFalse()
-            assertThat(room.sendMediaCount).isEqualTo(1)
+            assertThat(previewingState.attachmentsState).isInstanceOf(AttachmentsState.Previewing::class.java)
+            cancelAndIgnoreRemainingEvents()
         }
     }
 
@@ -442,10 +420,10 @@ class MessageComposerPresenterTest {
             presenter.present()
         }.test {
             val initialState = awaitItem()
-            initialState.eventSink(MessageComposerEvents.PickCameraAttachmentSource.Video)
+            initialState.eventSink(MessageComposerEvents.PickAttachmentSource.VideoFromCamera)
-            // Wait for the launched upload coroutine to run
+            val previewingState = awaitItem()
-            runCurrent()
+            assertThat(previewingState.showAttachmentSourcePicker).isFalse()
-            assertThat(room.sendMediaCount).isEqualTo(1)
+            assertThat(previewingState.attachmentsState).isInstanceOf(AttachmentsState.Previewing::class.java)
         }
     }
 
@@ -460,10 +438,11 @@ class MessageComposerPresenterTest {
         }.test {
             val initialState = awaitItem()
             initialState.eventSink(MessageComposerEvents.PickAttachmentSource.FromFiles)
-
+            val sendingState = awaitItem()
+            assertThat(sendingState.attachmentsState).isInstanceOf(AttachmentsState.Sending::class.java)
+            val finalState= awaitItem()
+            assertThat(finalState.attachmentsState).isInstanceOf(AttachmentsState.None::class.java)
             snackbarDispatcher.snackbarMessage.test {
-                // Initial value is always null
-                skipItems(1)
                 // Assert error message received
                 assertThat(awaitItem()).isNotNull()
             }
@@ -491,7 +470,8 @@ class MessageComposerPresenterTest {
         room,
         pickerProvider,
         featureFlagService,
-        mediaPreProcessor,
+        localMediaFactory,
+        MediaSender(mediaPreProcessor, room),
         snackbarDispatcher
     )
 }