Check if media processing finished before sending status (#2458)

* make MastodonApi.createStatus suspending

* check if media processing has finished before sending status

* add backoff for retrying processed media check

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

@@ -251,13 +251,15 @@ class ComposeViewModel @Inject constructor(
         val sendObservable = media
             .filter { items -> items.all { it.uploadPercent == -1 } }
             .map {
-                val mediaIds = ArrayList<String>()
-                val mediaUris = ArrayList<Uri>()
-                val mediaDescriptions = ArrayList<String>()
+                val mediaIds: MutableList<String> = mutableListOf()
+                val mediaUris: MutableList<Uri> = mutableListOf()
+                val mediaDescriptions: MutableList<String> = mutableListOf()
+                val mediaProcessed: MutableList<Boolean> = mutableListOf()
                 for (item in media.value!!) {
                     mediaIds.add(item.id!!)
                     mediaUris.add(item.uri)
                     mediaDescriptions.add(item.description ?: "")
+                    mediaProcessed.add(false)
                 }
 
                 val tootToSend = StatusToSend(
@@ -276,7 +278,8 @@ class ComposeViewModel @Inject constructor(
                     accountId = accountManager.activeAccount!!.id,
                     draftId = draftId,
                     idempotencyKey = randomAlphanumericString(16),
-                    retries = 0
+                    retries = 0,
+                    mediaProcessed = mediaProcessed
                 )
 
                 serviceClient.sendToot(tootToSend)

app/src/main/java/com/keylesspalace/tusky/network/MastodonApi.kt

@@ -156,13 +156,18 @@ interface MastodonApi {
         @Field("description") description: String
     ): Result<Attachment>
 
+    @GET("api/v1/media/{mediaId}")
+    suspend fun getMedia(
+        @Path("mediaId") mediaId: String
+    ): Response<MediaUploadResult>
+
     @POST("api/v1/statuses")
-    fun createStatus(
+    suspend fun createStatus(
         @Header("Authorization") auth: String,
         @Header(DOMAIN_HEADER) domain: String,
         @Header("Idempotency-Key") idempotencyKey: String,
         @Body status: NewStatus
-    ): Call<Status>
+    ): Result<Status>
 
     @GET("api/v1/statuses/{id}")
     fun status(

app/src/main/java/com/keylesspalace/tusky/receiver/SendStatusBroadcastReceiver.kt

@@ -100,7 +100,8 @@ class SendStatusBroadcastReceiver : BroadcastReceiver() {
                         accountId = account.id,
                         draftId = -1,
                         idempotencyKey = randomAlphanumericString(16),
-                        retries = 0
+                        retries = 0,
+                        mediaProcessed = mutableListOf()
                     )
                 )
 

app/src/main/java/com/keylesspalace/tusky/service/SendStatusService.kt

@@ -11,6 +11,7 @@ import android.content.Intent
 import android.os.Build
 import android.os.IBinder
 import android.os.Parcelable
+import android.util.Log
 import androidx.core.app.NotificationCompat
 import androidx.core.app.ServiceCompat
 import androidx.core.content.ContextCompat
@@ -29,13 +30,12 @@ import com.keylesspalace.tusky.network.MastodonApi
 import dagger.android.AndroidInjection
 import kotlinx.coroutines.CoroutineScope
 import kotlinx.coroutines.Dispatchers
+import kotlinx.coroutines.Job
 import kotlinx.coroutines.SupervisorJob
 import kotlinx.coroutines.delay
 import kotlinx.coroutines.launch
 import kotlinx.parcelize.Parcelize
-import retrofit2.Call
-import retrofit2.Callback
-import retrofit2.Response
+import retrofit2.HttpException
 import java.util.concurrent.ConcurrentHashMap
 import java.util.concurrent.TimeUnit
 import javax.inject.Inject
@@ -55,7 +55,7 @@ class SendStatusService : Service(), Injectable {
     private val serviceScope = CoroutineScope(Dispatchers.Main + supervisorJob)
 
     private val statusesToSend = ConcurrentHashMap<Int, StatusToSend>()
-    private val sendCalls = ConcurrentHashMap<Int, Call<Status>>()
+    private val sendJobs = ConcurrentHashMap<Int, Job>()
 
     private val notificationManager by lazy { getSystemService(Context.NOTIFICATION_SERVICE) as NotificationManager }
 
@@ -64,12 +64,9 @@ class SendStatusService : Service(), Injectable {
         super.onCreate()
     }
 
-    override fun onBind(intent: Intent): IBinder? {
-        return null
-    }
+    override fun onBind(intent: Intent): IBinder? = null
 
     override fun onStartCommand(intent: Intent, flags: Int, startId: Int): Int {
-
         if (intent.hasExtra(KEY_STATUS)) {
             val statusToSend = intent.getParcelableExtra<StatusToSend>(KEY_STATUS)
                 ?: throw IllegalStateException("SendStatusService started without $KEY_STATUS extra")
@@ -129,6 +126,25 @@ class SendStatusService : Service(), Injectable {
 
         statusToSend.retries++
 
+        sendJobs[statusId] = serviceScope.launch {
+            try {
+                var mediaCheckRetries = 0
+                while (statusToSend.mediaProcessed.any { !it }) {
+                    delay(1000L * mediaCheckRetries)
+                    statusToSend.mediaProcessed.forEachIndexed { index, processed ->
+                        if (!processed) {
+                            // Mastodon returns 206 if the media was not yet processed
+                            statusToSend.mediaProcessed[index] = mastodonApi.getMedia(statusToSend.mediaIds[index]).code() == 200
+                        }
+                    }
+                    mediaCheckRetries ++
+                }
+            } catch (e: Exception) {
+                Log.w(TAG, "failed getting media status", e)
+                retrySending(statusId)
+                return@launch
+            }
+
             val newStatus = NewStatus(
                 statusToSend.text,
                 statusToSend.warningText,
@@ -140,37 +156,32 @@ class SendStatusService : Service(), Injectable {
                 statusToSend.poll
             )
 
-        val sendCall = mastodonApi.createStatus(
+            mastodonApi.createStatus(
                 "Bearer " + account.accessToken,
                 account.domain,
                 statusToSend.idempotencyKey,
                 newStatus
-        )
-
-        sendCalls[statusId] = sendCall
-
-        val callback = object : Callback<Status> {
-            override fun onResponse(call: Call<Status>, response: Response<Status>) {
-                serviceScope.launch {
-
-                    val scheduled = !statusToSend.scheduledAt.isNullOrEmpty()
+            ).fold({ sentStatus ->
                 statusesToSend.remove(statusId)
-
-                    if (response.isSuccessful) {
                 // If the status was loaded from a draft, delete the draft and associated media files.
                 if (statusToSend.draftId != 0) {
                     draftHelper.deleteDraftAndAttachments(statusToSend.draftId)
                 }
 
+                val scheduled = !statusToSend.scheduledAt.isNullOrEmpty()
+
                 if (scheduled) {
-                            response.body()?.let(::StatusScheduledEvent)?.let(eventHub::dispatch)
+                    eventHub.dispatch(StatusScheduledEvent(sentStatus))
                 } else {
-                            response.body()?.let(::StatusComposedEvent)?.let(eventHub::dispatch)
+                    eventHub.dispatch(StatusComposedEvent(sentStatus))
                 }
 
                 notificationManager.cancel(statusId)
-                    } else {
+            }, { throwable ->
+                Log.w(TAG, "failed sending status", throwable)
+                if (throwable is HttpException) {
                     // the server refused to accept the status, save status & show error message
+                    statusesToSend.remove(statusId)
                     saveStatusToDrafts(statusToSend)
 
                     val builder = NotificationCompat.Builder(this@SendStatusService, CHANNEL_ID)
@@ -186,26 +197,24 @@ class SendStatusService : Service(), Injectable {
 
                     notificationManager.cancel(statusId)
                     notificationManager.notify(errorNotificationId--, builder.build())
+                } else {
+                    // a network problem occurred, let's retry sending the status
+                    retrySending(statusId)
                 }
+            })
             stopSelfWhenDone()
         }
     }
 
-            override fun onFailure(call: Call<Status>, t: Throwable) {
-                serviceScope.launch {
-                    var backoff = TimeUnit.SECONDS.toMillis(statusToSend.retries.toLong())
-                    if (backoff > MAX_RETRY_INTERVAL) {
-                        backoff = MAX_RETRY_INTERVAL
-                    }
+    private suspend fun retrySending(statusId: Int) {
+        // when statusToSend == null, sending has been canceled
+        val statusToSend = statusesToSend[statusId] ?: return
+
+        val backoff = TimeUnit.SECONDS.toMillis(statusToSend.retries.toLong()).coerceAtMost(MAX_RETRY_INTERVAL)
 
         delay(backoff)
         sendStatus(statusId)
     }
-            }
-        }
-
-        sendCall.enqueue(callback)
-    }
 
     private fun stopSelfWhenDone() {
 
@@ -218,8 +227,8 @@ class SendStatusService : Service(), Injectable {
     private fun cancelSending(statusId: Int) = serviceScope.launch {
         val statusToCancel = statusesToSend.remove(statusId)
         if (statusToCancel != null) {
-            val sendCall = sendCalls.remove(statusId)
-            sendCall?.cancel()
+            val sendJob = sendJobs.remove(statusId)
+            sendJob?.cancel()
 
             saveStatusToDrafts(statusToCancel)
 
@@ -263,6 +272,7 @@ class SendStatusService : Service(), Injectable {
     }
 
     companion object {
+        private const val TAG = "SendStatusService"
 
         private const val KEY_STATUS = "status"
         private const val KEY_CANCEL = "cancel_id"
@@ -319,5 +329,6 @@ data class StatusToSend(
     val accountId: Long,
     val draftId: Int,
     val idempotencyKey: String,
-    var retries: Int
+    var retries: Int,
+    val mediaProcessed: MutableList<Boolean>
 ) : Parcelable