keep ordering when picking multiple media files at once (#4841)

This restructures the code so that all picked media will be added to the
upload queue in the correct order and also does some other code cleanup.

closes #4754

app/src/main/java/com/keylesspalace/tusky/components/compose/ComposeActivity.kt

@@ -73,6 +73,7 @@ import com.keylesspalace.tusky.adapter.EmojiAdapter
 import com.keylesspalace.tusky.adapter.LocaleAdapter
 import com.keylesspalace.tusky.adapter.OnEmojiSelectedListener
 import com.keylesspalace.tusky.components.compose.ComposeViewModel.ConfirmationKind
+import com.keylesspalace.tusky.components.compose.ComposeViewModel.QueuedMedia
 import com.keylesspalace.tusky.components.compose.dialog.CaptionDialog
 import com.keylesspalace.tusky.components.compose.dialog.makeFocusDialog
 import com.keylesspalace.tusky.components.compose.dialog.showAddPollDialog
@@ -102,6 +103,7 @@ import com.keylesspalace.tusky.util.getSerializableCompat
 import com.keylesspalace.tusky.util.hide
 import com.keylesspalace.tusky.util.highlightSpans
 import com.keylesspalace.tusky.util.loadAvatar
+import com.keylesspalace.tusky.util.map
 import com.keylesspalace.tusky.util.modernLanguageCode
 import com.keylesspalace.tusky.util.setDrawableTint
 import com.keylesspalace.tusky.util.show
@@ -162,7 +164,7 @@ class ComposeActivity :
     private val takePictureLauncher =
         registerForActivityResult(ActivityResultContracts.TakePicture()) { success ->
             if (success) {
-                pickMedia(photoUploadUri!!)
+                viewModel.pickMedia(photoUploadUri!!)
             }
         }
     private val pickMediaFilePermissionLauncher =
@@ -194,9 +196,11 @@ class ComposeActivity :
                 Toast.LENGTH_SHORT
             ).show()
         } else {
-            uris.forEach { uri ->
-                pickMedia(uri)
-            }
+            viewModel.pickMedia(
+                uris.map { uri ->
+                    ComposeViewModel.MediaData(uri)
+                }
+            )
         }
     }
 
@@ -207,17 +211,15 @@ class ComposeActivity :
             viewModel.cropImageItemOld?.let { itemOld ->
                 val size = getMediaSize(contentResolver, uriNew)
 
-                lifecycleScope.launch {
-                    viewModel.addMediaToQueue(
-                        itemOld.type,
-                        uriNew,
-                        size,
-                        itemOld.description,
-                        // Intentionally reset focus when cropping
-                        null,
-                        itemOld
-                    )
-                }
+                viewModel.addMediaToQueue(
+                    type = itemOld.type,
+                    uri = uriNew,
+                    mediaSize = size,
+                    description = itemOld.description,
+                    // Intentionally reset focus when cropping
+                    focus = null,
+                    replaceItem = itemOld
+                )
             }
         } else if (result == CropImage.CancelledResult) {
             Log.w(TAG, "Edit image cancelled by user")
@@ -308,7 +310,7 @@ class ComposeActivity :
         }
 
         if (!composeOptions?.scheduledAt.isNullOrEmpty()) {
-            binding.composeScheduleView.setDateTime(composeOptions?.scheduledAt)
+            binding.composeScheduleView.setDateTime(composeOptions.scheduledAt)
         }
 
         setupLanguageSpinner(getInitialLanguages(composeOptions?.language, activeAccount))
@@ -347,14 +349,14 @@ class ComposeActivity :
                     when (intent.action) {
                         Intent.ACTION_SEND -> {
                             intent.getParcelableExtraCompat<Uri>(Intent.EXTRA_STREAM)?.let { uri ->
-                                pickMedia(uri)
+                                viewModel.pickMedia(uri)
                             }
                         }
                         Intent.ACTION_SEND_MULTIPLE -> {
                             intent.getParcelableArrayListExtraCompat<Uri>(Intent.EXTRA_STREAM)
-                                ?.forEach { uri ->
-                                    pickMedia(uri)
-                                }
+                                ?.map { uri ->
+                                    ComposeViewModel.MediaData(uri)
+                                }?.let(viewModel::pickMedia)
                         }
                     }
                 }
@@ -557,16 +559,25 @@ class ComposeActivity :
 
         lifecycleScope.launch {
             viewModel.uploadError.collect { throwable ->
-                if (throwable is UploadServerError) {
-                    displayTransientMessage(throwable.errorMessage)
-                } else {
-                    displayTransientMessage(
-                        getString(
-                            R.string.error_media_upload_sending_fmt,
-                            throwable.message
-                        )
+                val errorString = when (throwable) {
+                    is UploadServerError -> throwable.errorMessage
+                    is FileSizeException -> {
+                        val decimalFormat = DecimalFormat("0.##")
+                        val allowedSizeInMb = throwable.allowedSizeInBytes.toDouble() / (1024 * 1024)
+                        val formattedSize = decimalFormat.format(allowedSizeInMb)
+                        getString(R.string.error_multimedia_size_limit, formattedSize)
+                    }
+                    is VideoOrImageException -> getString(
+                        R.string.error_media_upload_image_or_video
+                    )
+                    is CouldNotOpenFileException -> getString(R.string.error_media_upload_opening)
+                    is MediaTypeException -> getString(R.string.error_media_upload_opening)
+                    else -> getString(
+                        R.string.error_media_upload_sending_fmt,
+                        throwable.message
                     )
                 }
+                displayTransientMessage(errorString)
             }
         }
 
@@ -1090,12 +1101,27 @@ class ComposeActivity :
         if (contentInfo.clip.description.hasMimeType("image/*")) {
             val split = contentInfo.partition { item: ClipData.Item -> item.uri != null }
             split.first?.let { content ->
-                for (i in 0 until content.clip.itemCount) {
-                    pickMedia(
-                        content.clip.getItemAt(i).uri,
-                        contentInfo.clip.description.label as String?
-                    )
+                val description = (contentInfo.clip.description.label as String?)?.let {
+                    // The Gboard android keyboard attaches this text whenever the user
+                    // pastes something from the keyboard's suggestion bar.
+                    // Due to different end user locales, the exact text may vary, but at
+                    // least in version 13.4.08, all of the translations contained the
+                    // string "Gboard".
+                    if ("Gboard" in it) {
+                        null
+                    } else {
+                        it
+                    }
                 }
+
+                viewModel.pickMedia(
+                    content.clip.map { clipItem ->
+                        ComposeViewModel.MediaData(
+                            uri = clipItem.uri,
+                            description = description
+                        )
+                    }
+                )
             }
             return split.second
         }
@@ -1199,45 +1225,6 @@ class ComposeActivity :
         viewModel.removeMediaFromQueue(item)
     }
 
-    private fun sanitizePickMediaDescription(description: String?): String? {
-        if (description == null) {
-            return null
-        }
-
-        // The Gboard android keyboard attaches this text whenever the user
-        // pastes something from the keyboard's suggestion bar.
-        // Due to different end user locales, the exact text may vary, but at
-        // least in version 13.4.08, all of the translations contained the
-        // string "Gboard".
-        if ("Gboard" in description) {
-            return null
-        }
-
-        return description
-    }
-
-    private fun pickMedia(uri: Uri, description: String? = null) {
-        val sanitizedDescription = sanitizePickMediaDescription(description)
-
-        lifecycleScope.launch {
-            viewModel.pickMedia(uri, sanitizedDescription).onFailure { throwable ->
-                val errorString = when (throwable) {
-                    is FileSizeException -> {
-                        val decimalFormat = DecimalFormat("0.##")
-                        val allowedSizeInMb = throwable.allowedSizeInBytes.toDouble() / (1024 * 1024)
-                        val formattedSize = decimalFormat.format(allowedSizeInMb)
-                        getString(R.string.error_multimedia_size_limit, formattedSize)
-                    }
-                    is VideoOrImageException -> getString(
-                        R.string.error_media_upload_image_or_video
-                    )
-                    else -> getString(R.string.error_media_upload_opening)
-                }
-                displayTransientMessage(errorString)
-            }
-        }
-    }
-
     private fun showContentWarning(show: Boolean) {
         TransitionManager.beginDelayedTransition(
             binding.composeContentWarningBar.parent as ViewGroup
@@ -1420,30 +1407,6 @@ class ComposeActivity :
         }
     }
 
-    data class QueuedMedia(
-        val localId: Int,
-        val uri: Uri,
-        val type: Type,
-        val mediaSize: Long,
-        val uploadPercent: Int = 0,
-        val id: String? = null,
-        val description: String? = null,
-        val focus: Attachment.Focus? = null,
-        val state: State
-    ) {
-        enum class Type {
-            IMAGE,
-            VIDEO,
-            AUDIO
-        }
-        enum class State {
-            UPLOADING,
-            UNPROCESSED,
-            PROCESSED,
-            PUBLISHED
-        }
-    }
-
     override fun onTimeSet(time: String?) {
         viewModel.updateScheduledAt(time)
         if (verifyScheduledTime()) {

app/src/main/java/com/keylesspalace/tusky/components/compose/ComposeViewModel.kt

@@ -22,7 +22,6 @@ import androidx.lifecycle.ViewModel
 import androidx.lifecycle.viewModelScope
 import at.connyduck.calladapter.networkresult.fold
 import com.keylesspalace.tusky.components.compose.ComposeActivity.ComposeKind
-import com.keylesspalace.tusky.components.compose.ComposeActivity.QueuedMedia
 import com.keylesspalace.tusky.components.compose.ComposeAutoCompleteAdapter.AutocompleteResult
 import com.keylesspalace.tusky.components.drafts.DraftHelper
 import com.keylesspalace.tusky.components.instanceinfo.InstanceInfo
@@ -41,6 +40,7 @@ import com.keylesspalace.tusky.util.randomAlphanumericString
 import dagger.hilt.android.lifecycle.HiltViewModel
 import javax.inject.Inject
 import kotlinx.coroutines.Dispatchers
+import kotlinx.coroutines.async
 import kotlinx.coroutines.channels.BufferOverflow
 import kotlinx.coroutines.flow.MutableSharedFlow
 import kotlinx.coroutines.flow.MutableStateFlow
@@ -55,7 +55,6 @@ import kotlinx.coroutines.flow.shareIn
 import kotlinx.coroutines.flow.update
 import kotlinx.coroutines.launch
 import kotlinx.coroutines.runBlocking
-import kotlinx.coroutines.withContext
 
 @HiltViewModel
 class ComposeViewModel @Inject constructor(
@@ -92,7 +91,7 @@ class ComposeViewModel @Inject constructor(
         .shareIn(viewModelScope, SharingStarted.Eagerly, replay = 1)
 
     private val _markMediaAsSensitive =
-        MutableStateFlow(accountManager.activeAccount?.defaultMediaSensitivity ?: false)
+        MutableStateFlow(accountManager.activeAccount?.defaultMediaSensitivity == true)
     val markMediaAsSensitive: StateFlow<Boolean> = _markMediaAsSensitive.asStateFlow()
 
     private val _statusVisibility = MutableStateFlow(Status.Visibility.UNKNOWN)
@@ -127,31 +126,31 @@ class ComposeViewModel @Inject constructor(
 
     private var setupComplete = false
 
-    suspend fun pickMedia(
-        mediaUri: Uri,
-        description: String? = null,
-        focus: Attachment.Focus? = null
-    ): Result<QueuedMedia> = withContext(
-        Dispatchers.IO
-    ) {
-        try {
-            val (type, uri, size) = mediaUploader.prepareMedia(mediaUri, instanceInfo.first())
-            val mediaItems = _media.value
-            if (type != QueuedMedia.Type.IMAGE &&
-                mediaItems.isNotEmpty() &&
-                mediaItems[0].type == QueuedMedia.Type.IMAGE
-            ) {
-                Result.failure(VideoOrImageException())
-            } else {
-                val queuedMedia = addMediaToQueue(type, uri, size, description, focus)
-                Result.success(queuedMedia)
-            }
-        } catch (e: Exception) {
-            Result.failure(e)
+    fun pickMedia(uri: Uri) {
+        pickMedia(listOf(MediaData(uri)))
+    }
+
+    fun pickMedia(mediaList: List<MediaData>) = viewModelScope.launch(Dispatchers.IO) {
+        val instanceInfo = instanceInfo.first()
+        mediaList.map { m ->
+            async { mediaUploader.prepareMedia(m.uri, instanceInfo) }
+        }.forEachIndexed { index, preparedMedia ->
+            preparedMedia.await().fold({ (type, uri, size) ->
+                if (type != QueuedMedia.Type.IMAGE &&
+                    _media.value.firstOrNull()?.type == QueuedMedia.Type.IMAGE
+                ) {
+                    _uploadError.emit(VideoOrImageException())
+                } else {
+                    val pickedMedia = mediaList[index]
+                    addMediaToQueue(type, uri, size, pickedMedia.description, pickedMedia.focus)
+                }
+            }, { error ->
+                _uploadError.emit(error)
+            })
         }
     }
 
-    suspend fun addMediaToQueue(
+    fun addMediaToQueue(
         type: QueuedMedia.Type,
         uri: Uri,
         mediaSize: Long,
@@ -159,20 +158,17 @@ class ComposeViewModel @Inject constructor(
         focus: Attachment.Focus? = null,
         replaceItem: QueuedMedia? = null
     ): QueuedMedia {
-        var stashMediaItem: QueuedMedia? = null
+        val mediaItem = QueuedMedia(
+            localId = mediaUploader.getNewLocalMediaId(),
+            uri = uri,
+            type = type,
+            mediaSize = mediaSize,
+            description = description,
+            focus = focus,
+            state = QueuedMedia.State.UPLOADING
+        )
 
         _media.update { mediaList ->
-            val mediaItem = QueuedMedia(
-                localId = mediaUploader.getNewLocalMediaId(),
-                uri = uri,
-                type = type,
-                mediaSize = mediaSize,
-                description = description,
-                focus = focus,
-                state = QueuedMedia.State.UPLOADING
-            )
-            stashMediaItem = mediaItem
-
             if (replaceItem != null) {
                 mediaUploader.cancelUploadScope(replaceItem.localId)
                 mediaList.map {
@@ -182,8 +178,6 @@ class ComposeViewModel @Inject constructor(
                 mediaList + mediaItem
             }
         }
-        val mediaItem =
-            stashMediaItem!! // stashMediaItem is always non-null and uncaptured at this point, but Kotlin doesn't know that
 
         viewModelScope.launch {
             mediaUploader
@@ -505,11 +499,9 @@ class ComposeViewModel @Inject constructor(
         val draftAttachments = composeOptions?.draftAttachments
         if (draftAttachments != null) {
             // when coming from DraftActivity
-            viewModelScope.launch {
-                draftAttachments.forEach { attachment ->
-                    pickMedia(attachment.uri, attachment.description, attachment.focus)
-                }
-            }
+            draftAttachments.map { attachment ->
+                MediaData(attachment.uri, attachment.description, attachment.focus)
+            }.let(::pickMedia)
         } else {
             composeOptions?.mediaAttachments?.forEach { a ->
                 // when coming from redraft or ScheduledTootActivity
@@ -588,6 +580,36 @@ class ComposeViewModel @Inject constructor(
         CONTINUE_EDITING_OR_DISCARD_CHANGES, // editing post
         CONTINUE_EDITING_OR_DISCARD_DRAFT // edit draft
     }
+
+    data class QueuedMedia(
+        val localId: Int,
+        val uri: Uri,
+        val type: Type,
+        val mediaSize: Long,
+        val uploadPercent: Int = 0,
+        val id: String? = null,
+        val description: String? = null,
+        val focus: Attachment.Focus? = null,
+        val state: State
+    ) {
+        enum class Type {
+            IMAGE,
+            VIDEO,
+            AUDIO
+        }
+        enum class State {
+            UPLOADING,
+            UNPROCESSED,
+            PROCESSED,
+            PUBLISHED
+        }
+    }
+
+    data class MediaData(
+        val uri: Uri,
+        val description: String? = null,
+        val focus: Attachment.Focus? = null
+    )
 }
 
 /**

app/src/main/java/com/keylesspalace/tusky/components/compose/MediaPreviewAdapter.kt

@@ -31,25 +31,25 @@ import com.keylesspalace.tusky.components.compose.view.ProgressImageView
 
 class MediaPreviewAdapter(
     context: Context,
-    private val onAddCaption: (ComposeActivity.QueuedMedia) -> Unit,
-    private val onAddFocus: (ComposeActivity.QueuedMedia) -> Unit,
-    private val onEditImage: (ComposeActivity.QueuedMedia) -> Unit,
-    private val onRemove: (ComposeActivity.QueuedMedia) -> Unit
-) : ListAdapter<ComposeActivity.QueuedMedia, MediaPreviewAdapter.PreviewViewHolder>(
-    object : DiffUtil.ItemCallback<ComposeActivity.QueuedMedia>() {
+    private val onAddCaption: (ComposeViewModel.QueuedMedia) -> Unit,
+    private val onAddFocus: (ComposeViewModel.QueuedMedia) -> Unit,
+    private val onEditImage: (ComposeViewModel.QueuedMedia) -> Unit,
+    private val onRemove: (ComposeViewModel.QueuedMedia) -> Unit
+) : ListAdapter<ComposeViewModel.QueuedMedia, MediaPreviewAdapter.PreviewViewHolder>(
+    object : DiffUtil.ItemCallback<ComposeViewModel.QueuedMedia>() {
         override fun areItemsTheSame(
-            oldItem: ComposeActivity.QueuedMedia,
-            newItem: ComposeActivity.QueuedMedia
+            oldItem: ComposeViewModel.QueuedMedia,
+            newItem: ComposeViewModel.QueuedMedia
         ) = oldItem.localId == newItem.localId
 
         override fun areContentsTheSame(
-            oldItem: ComposeActivity.QueuedMedia,
-            newItem: ComposeActivity.QueuedMedia
+            oldItem: ComposeViewModel.QueuedMedia,
+            newItem: ComposeViewModel.QueuedMedia
         ) = oldItem == newItem
     }
 ) {
 
-    private fun onMediaClick(item: ComposeActivity.QueuedMedia, view: View) {
+    private fun onMediaClick(item: ComposeViewModel.QueuedMedia, view: View) {
         val popup = PopupMenu(view.context, view)
         val addCaptionId = 1
         val addFocusId = 2
@@ -57,9 +57,9 @@ class MediaPreviewAdapter(
         val removeId = 4
 
         popup.menu.add(0, addCaptionId, 0, R.string.action_set_caption)
-        if (item.type == ComposeActivity.QueuedMedia.Type.IMAGE) {
+        if (item.type == ComposeViewModel.QueuedMedia.Type.IMAGE) {
             popup.menu.add(0, addFocusId, 0, R.string.action_set_focus)
-            if (item.state != ComposeActivity.QueuedMedia.State.PUBLISHED) {
+            if (item.state != ComposeViewModel.QueuedMedia.State.PUBLISHED) {
                 // Already-published items can't be edited
                 popup.menu.add(0, editImageId, 0, R.string.action_edit_image)
             }
@@ -88,7 +88,7 @@ class MediaPreviewAdapter(
         val item = getItem(position)
         holder.progressImageView.setChecked(!item.description.isNullOrEmpty())
         holder.progressImageView.setProgress(item.uploadPercent)
-        if (item.type == ComposeActivity.QueuedMedia.Type.AUDIO) {
+        if (item.type == ComposeViewModel.QueuedMedia.Type.AUDIO) {
             // TODO: Fancy waveform display?
             holder.progressImageView.setImageResource(R.drawable.ic_music_box_preview_24dp)
         } else {

app/src/main/java/com/keylesspalace/tusky/components/compose/MediaUploader.kt

@@ -27,7 +27,7 @@ import androidx.core.content.FileProvider
 import androidx.core.net.toUri
 import com.keylesspalace.tusky.BuildConfig
 import com.keylesspalace.tusky.R
-import com.keylesspalace.tusky.components.compose.ComposeActivity.QueuedMedia
+import com.keylesspalace.tusky.components.compose.ComposeViewModel.QueuedMedia
 import com.keylesspalace.tusky.components.instanceinfo.InstanceInfo
 import com.keylesspalace.tusky.network.MediaUploadApi
 import com.keylesspalace.tusky.network.asRequestBody
@@ -151,11 +151,13 @@ class MediaUploader @Inject constructor(
         }
     }
 
-    fun prepareMedia(inUri: Uri, instanceInfo: InstanceInfo): PreparedMedia {
+    fun prepareMedia(inUri: Uri, instanceInfo: InstanceInfo): Result<PreparedMedia> = runCatching {
         var mediaSize = MEDIA_SIZE_UNKNOWN
         var uri = inUri
         val mimeType: String?
 
+        println("preparing media on thread ${Thread.currentThread().name}")
+
         try {
             when (inUri.scheme) {
                 ContentResolver.SCHEME_CONTENT -> {
@@ -217,9 +219,8 @@ class MediaUploader @Inject constructor(
             Log.w(TAG, "Could not determine file size of upload")
             throw MediaTypeException()
         }
-
         if (mimeType != null) {
-            return when (mimeType.substring(0, mimeType.indexOf('/'))) {
+            when (mimeType.substring(0, mimeType.indexOf('/'))) {
                 "video" -> {
                     if (mediaSize > instanceInfo.videoSizeLimit) {
                         throw FileSizeException(instanceInfo.videoSizeLimit)
@@ -247,7 +248,7 @@ class MediaUploader @Inject constructor(
 
     private val contentResolver = context.contentResolver
 
-    private suspend fun upload(media: QueuedMedia): Flow<UploadEvent> {
+    private fun upload(media: QueuedMedia): Flow<UploadEvent> {
         return callbackFlow {
             var mimeType = contentResolver.getType(media.uri)
 

app/src/main/java/com/keylesspalace/tusky/util/PickMediaFiles.kt

@@ -16,6 +16,7 @@
 package com.keylesspalace.tusky.util
 
 import android.app.Activity
+import android.content.ClipData
 import android.content.Context
 import android.content.Intent
 import android.net.Uri
@@ -40,13 +41,17 @@ class PickMediaFiles : ActivityResultContract<Boolean, List<Uri>>() {
                 // Single media, upload it and done.
                 return listOf(intentData)
             } else if (clipData != null) {
-                val result: MutableList<Uri> = mutableListOf()
-                for (i in 0 until clipData.itemCount) {
-                    result.add(clipData.getItemAt(i).uri)
-                }
-                return result
+                return clipData.map { clipItem -> clipItem.uri }
             }
         }
         return emptyList()
     }
 }
+
+fun <T> ClipData.map(transform: (ClipData.Item) -> T): List<T> {
+    val destination = ArrayList<T>(this.itemCount)
+    for (i in 0 until this.itemCount) {
+        destination.add(transform(getItemAt(i)))
+    }
+    return destination
+}