Merge pull request #2235 from vector-im/feature/bca/perf_better_send_…

…time Feature/bca/perf better send time
element-hq · Oct 20, 2020 · c88c2a1 · c88c2a1
2 parents c05147a + dc8a6cc
commit c88c2a1
Show file tree

Hide file tree

Showing 34 changed files with 995 additions and 528 deletions.
diff --git a/CHANGES.md b/CHANGES.md
@@ -5,7 +5,7 @@ Features ✨:
  -
 
 Improvements 🙌:
- -
+ - Rework sending Event management (#154)
 
 Bugfix 🐛:
  -

diff --git a/...roid/src/main/java/org/matrix/android/sdk/api/session/room/model/relation/ReactionInfo.kt b/...roid/src/main/java/org/matrix/android/sdk/api/session/room/model/relation/ReactionInfo.kt
@@ -23,7 +23,7 @@ import com.squareup.moshi.JsonClass
 data class ReactionInfo(
         @Json(name = "rel_type") override val type: String?,
         @Json(name = "event_id") override val eventId: String,
-        val key: String,
+        @Json(name = "key") val key: String,
         // always null for reaction
         @Json(name = "m.in_reply_to") override val inReplyTo: ReplyToContent? = null,
         @Json(name = "option") override val option: Int? = null

diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/room/send/SendService.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/room/send/SendService.kt
@@ -123,11 +123,6 @@ interface SendService {
      */
     fun deleteFailedEcho(localEcho: TimelineEvent)
 
-    /**
-     * Delete all the events in one of the sending states
-     */
-    fun clearSendingQueue()
-
     /**
      * Cancel sending a specific event. It has to be in one of the sending states
      */

diff --git a/.../main/java/org/matrix/android/sdk/internal/crypto/algorithms/megolm/MXMegolmEncryption.kt b/.../main/java/org/matrix/android/sdk/internal/crypto/algorithms/megolm/MXMegolmEncryption.kt
@@ -420,7 +420,7 @@ internal class MXMegolmEncryption(
             sendToDeviceTask.execute(sendToDeviceParams)
             true
         } catch (failure: Throwable) {
-            Timber.v("## CRYPTO | CRYPTO | reshareKey() : fail to send <$sessionId> to $userId:$deviceId")
+            Timber.e(failure, "## CRYPTO | CRYPTO | reshareKey() : fail to send <$sessionId> to $userId:$deviceId")
             false
         }
     }

diff --git a/...dk-android/src/main/java/org/matrix/android/sdk/internal/crypto/tasks/EncryptEventTask.kt b/...dk-android/src/main/java/org/matrix/android/sdk/internal/crypto/tasks/EncryptEventTask.kt
@@ -17,8 +17,13 @@ package org.matrix.android.sdk.internal.crypto.tasks
 
 import org.matrix.android.sdk.api.session.crypto.CryptoService
 import org.matrix.android.sdk.api.session.events.model.Event
+import org.matrix.android.sdk.api.session.events.model.EventType
+import org.matrix.android.sdk.api.session.events.model.toContent
 import org.matrix.android.sdk.api.session.room.send.SendState
+import org.matrix.android.sdk.internal.crypto.MXCRYPTO_ALGORITHM_MEGOLM
+import org.matrix.android.sdk.internal.crypto.MXEventDecryptionResult
 import org.matrix.android.sdk.internal.crypto.model.MXEncryptEventContentResult
+import org.matrix.android.sdk.internal.database.mapper.ContentMapper
 import org.matrix.android.sdk.internal.session.room.send.LocalEchoRepository
 import org.matrix.android.sdk.internal.task.Task
 import org.matrix.android.sdk.internal.util.awaitCallback
@@ -38,20 +43,22 @@ internal class DefaultEncryptEventTask @Inject constructor(
         private val localEchoRepository: LocalEchoRepository
 ) : EncryptEventTask {
     override suspend fun execute(params: EncryptEventTask.Params): Event {
-        if (!params.crypto.isRoomEncrypted(params.roomId)) return params.event
+        // don't want to wait for any query
+        // if (!params.crypto.isRoomEncrypted(params.roomId)) return params.event
         val localEvent = params.event
         if (localEvent.eventId == null) {
             throw IllegalArgumentException()
         }
 
-        localEchoRepository.updateSendState(localEvent.eventId, SendState.ENCRYPTING)
+        localEchoRepository.updateSendState(localEvent.eventId, localEvent.roomId, SendState.ENCRYPTING)
 
         val localMutableContent = localEvent.content?.toMutableMap() ?: mutableMapOf()
         params.keepKeys?.forEach {
             localMutableContent.remove(it)
         }
 
 //        try {
+        // let it throws
         awaitCallback<MXEncryptEventContentResult> {
             params.crypto.encryptEventContent(localMutableContent, localEvent.type, params.roomId, it)
         }.let { result ->
@@ -63,18 +70,34 @@ internal class DefaultEncryptEventTask @Inject constructor(
                 }
             }
             val safeResult = result.copy(eventContent = modifiedContent)
+            // Better handling of local echo, to avoid decrypting transition on remote echo
+            // Should I only do it for text messages?
+            val decryptionLocalEcho = if (result.eventContent["algorithm"] == MXCRYPTO_ALGORITHM_MEGOLM) {
+                MXEventDecryptionResult(
+                        clearEvent = Event(
+                                type = localEvent.type,
+                                content = localEvent.content,
+                                roomId = localEvent.roomId
+                        ).toContent(),
+                        forwardingCurve25519KeyChain = emptyList(),
+                        senderCurve25519Key = result.eventContent["sender_key"] as? String,
+                        claimedEd25519Key = params.crypto.getMyDevice().fingerprint()
+                )
+            } else {
+                null
+            }
+
+            localEchoRepository.updateEcho(localEvent.eventId) { _, localEcho ->
+                localEcho.type = EventType.ENCRYPTED
+                localEcho.content = ContentMapper.map(modifiedContent)
+                decryptionLocalEcho?.also {
+                    localEcho.setDecryptionResult(it)
+                }
+            }
             return localEvent.copy(
                     type = safeResult.eventType,
                     content = safeResult.eventContent
             )
         }
-//        } catch (throwable: Throwable) {
-//            val sendState = when (throwable) {
-//                is Failure.CryptoError -> SendState.FAILED_UNKNOWN_DEVICES
-//                else                   -> SendState.UNDELIVERED
-//            }
-//            localEchoUpdater.updateSendState(localEvent.eventId, sendState)
-//            throw throwable
-//        }
     }
 }
diff --git a/...sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/tasks/RedactEventTask.kt b/...sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/tasks/RedactEventTask.kt
@@ -0,0 +1,49 @@
+/*
+ * Copyright 2020 The Matrix.org Foundation C.I.C.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.matrix.android.sdk.internal.crypto.tasks
+
+import org.greenrobot.eventbus.EventBus
+import org.matrix.android.sdk.internal.network.executeRequest
+import org.matrix.android.sdk.internal.session.room.RoomAPI
+import org.matrix.android.sdk.internal.session.room.send.SendResponse
+import org.matrix.android.sdk.internal.task.Task
+import javax.inject.Inject
+
+internal interface RedactEventTask : Task<RedactEventTask.Params, String> {
+    data class Params(
+            val txID: String,
+            val roomId: String,
+            val eventId: String,
+            val reason: String?
+    )
+}
+
+internal class DefaultRedactEventTask @Inject constructor(
+        private val roomAPI: RoomAPI,
+        private val eventBus: EventBus) : RedactEventTask {
+
+    override suspend fun execute(params: RedactEventTask.Params): String {
+        val executeRequest = executeRequest<SendResponse>(eventBus) {
+            apiCall = roomAPI.redactEvent(
+                    txId = params.txID,
+                    roomId = params.roomId,
+                    eventId = params.eventId,
+                    reason = if (params.reason == null) emptyMap() else mapOf("reason" to params.reason)
+            )
+        }
+        return executeRequest.eventId
+    }
+}
diff --git a/...x-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/tasks/SendEventTask.kt b/...x-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/tasks/SendEventTask.kt
@@ -15,6 +15,7 @@
  */
 package org.matrix.android.sdk.internal.crypto.tasks
 
+import org.greenrobot.eventbus.EventBus
 import org.matrix.android.sdk.api.session.crypto.CryptoService
 import org.matrix.android.sdk.api.session.events.model.Event
 import org.matrix.android.sdk.api.session.room.send.SendState
@@ -23,12 +24,12 @@ import org.matrix.android.sdk.internal.session.room.RoomAPI
 import org.matrix.android.sdk.internal.session.room.send.LocalEchoRepository
 import org.matrix.android.sdk.internal.session.room.send.SendResponse
 import org.matrix.android.sdk.internal.task.Task
-import org.greenrobot.eventbus.EventBus
 import javax.inject.Inject
 
 internal interface SendEventTask : Task<SendEventTask.Params, String> {
     data class Params(
             val event: Event,
+            val encrypt: Boolean,
             val cryptoService: CryptoService?
     )
 }
@@ -40,11 +41,11 @@ internal class DefaultSendEventTask @Inject constructor(
         private val eventBus: EventBus) : SendEventTask {
 
     override suspend fun execute(params: SendEventTask.Params): String {
-        val event = handleEncryption(params)
-        val localId = event.eventId!!
-
         try {
-            localEchoRepository.updateSendState(localId, SendState.SENDING)
+            val event = handleEncryption(params)
+            val localId = event.eventId!!
+
+            localEchoRepository.updateSendState(localId, params.event.roomId, SendState.SENDING)
             val executeRequest = executeRequest<SendResponse>(eventBus) {
                 apiCall = roomAPI.send(
                         localId,
@@ -53,26 +54,23 @@ internal class DefaultSendEventTask @Inject constructor(
                         eventType = event.type
                 )
             }
-            localEchoRepository.updateSendState(localId, SendState.SENT)
+            localEchoRepository.updateSendState(localId, params.event.roomId, SendState.SENT)
             return executeRequest.eventId
         } catch (e: Throwable) {
-            localEchoRepository.updateSendState(localId, SendState.UNDELIVERED)
+//            localEchoRepository.updateSendState(params.event.eventId!!, SendState.UNDELIVERED)
             throw e
         }
     }
 
+    @Throws
     private suspend fun handleEncryption(params: SendEventTask.Params): Event {
-        if (params.cryptoService?.isRoomEncrypted(params.event.roomId ?: "") == true) {
-            try {
-                return encryptEventTask.execute(EncryptEventTask.Params(
-                        params.event.roomId ?: "",
-                        params.event,
-                        listOf("m.relates_to"),
-                        params.cryptoService
-                ))
-            } catch (throwable: Throwable) {
-                // We said it's ok to send verification request in clear
-            }
+        if (params.encrypt && !params.event.isEncrypted()) {
+            return encryptEventTask.execute(EncryptEventTask.Params(
+                    params.event.roomId ?: "",
+                    params.event,
+                    listOf("m.relates_to"),
+                    params.cryptoService!!
+            ))
         }
         return params.event
     }

diff --git a/...src/main/java/org/matrix/android/sdk/internal/crypto/tasks/SendVerificationMessageTask.kt b/...src/main/java/org/matrix/android/sdk/internal/crypto/tasks/SendVerificationMessageTask.kt
@@ -44,7 +44,7 @@ internal class DefaultSendVerificationMessageTask @Inject constructor(
         val localId = event.eventId!!
 
         try {
-            localEchoRepository.updateSendState(localId, SendState.SENDING)
+            localEchoRepository.updateSendState(localId, event.roomId, SendState.SENDING)
             val executeRequest = executeRequest<SendResponse>(eventBus) {
                 apiCall = roomAPI.send(
                         localId,
@@ -53,10 +53,10 @@ internal class DefaultSendVerificationMessageTask @Inject constructor(
                         eventType = event.type
                 )
             }
-            localEchoRepository.updateSendState(localId, SendState.SENT)
+            localEchoRepository.updateSendState(localId, event.roomId, SendState.SENT)
             return executeRequest.eventId
         } catch (e: Throwable) {
-            localEchoRepository.updateSendState(localId, SendState.UNDELIVERED)
+            localEchoRepository.updateSendState(localId, event.roomId, SendState.UNDELIVERED)
             throw e
         }
     }

diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/DefaultSession.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/DefaultSession.kt
@@ -64,6 +64,7 @@ import org.matrix.android.sdk.internal.di.SessionId
 import org.matrix.android.sdk.internal.di.UnauthenticatedWithCertificate
 import org.matrix.android.sdk.internal.di.WorkManagerProvider
 import org.matrix.android.sdk.internal.session.identity.DefaultIdentityService
+import org.matrix.android.sdk.internal.session.room.send.queue.EventSenderProcessor
 import org.matrix.android.sdk.internal.session.room.timeline.TimelineEventDecryptor
 import org.matrix.android.sdk.internal.session.sync.SyncTokenStore
 import org.matrix.android.sdk.internal.session.sync.job.SyncThread
@@ -121,7 +122,8 @@ internal class DefaultSession @Inject constructor(
         private val taskExecutor: TaskExecutor,
         private val callSignalingService: Lazy<CallSignalingService>,
         @UnauthenticatedWithCertificate
-        private val unauthenticatedWithCertificateOkHttpClient: Lazy<OkHttpClient>
+        private val unauthenticatedWithCertificateOkHttpClient: Lazy<OkHttpClient>,
+        private val eventSenderProcessor: EventSenderProcessor
 ) : Session,
         RoomService by roomService.get(),
         RoomDirectoryService by roomDirectoryService.get(),
@@ -161,6 +163,7 @@ internal class DefaultSession @Inject constructor(
         }
         eventBus.register(this)
         timelineEventDecryptor.start()
+        eventSenderProcessor.start()
     }
 
     override fun requireBackgroundSync() {
@@ -204,6 +207,7 @@ internal class DefaultSession @Inject constructor(
         cryptoService.get().close()
         isOpen = false
         eventBus.unregister(this)
+        eventSenderProcessor.interrupt()
     }
 
     override fun getSyncStateLive() = getSyncThread().liveState()

diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/SessionComponent.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/SessionComponent.kt
@@ -45,7 +45,6 @@ import org.matrix.android.sdk.internal.session.pushers.AddHttpPusherWorker
 import org.matrix.android.sdk.internal.session.pushers.PushersModule
 import org.matrix.android.sdk.internal.session.room.RoomModule
 import org.matrix.android.sdk.internal.session.room.relation.SendRelationWorker
-import org.matrix.android.sdk.internal.session.room.send.EncryptEventWorker
 import org.matrix.android.sdk.internal.session.room.send.MultipleEventSendingDispatcherWorker
 import org.matrix.android.sdk.internal.session.room.send.RedactEventWorker
 import org.matrix.android.sdk.internal.session.room.send.SendEventWorker
@@ -109,8 +108,6 @@ internal interface SessionComponent {
 
     fun inject(worker: SendRelationWorker)
 
-    fun inject(worker: EncryptEventWorker)
-
     fun inject(worker: MultipleEventSendingDispatcherWorker)
 
     fun inject(worker: RedactEventWorker)

diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/SessionModule.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/SessionModule.kt
@@ -43,6 +43,8 @@ import org.matrix.android.sdk.api.session.securestorage.SharedSecretStorageServi
 import org.matrix.android.sdk.api.session.typing.TypingUsersTracker
 import org.matrix.android.sdk.internal.crypto.crosssigning.ShieldTrustUpdater
 import org.matrix.android.sdk.internal.crypto.secrets.DefaultSharedSecretStorageService
+import org.matrix.android.sdk.internal.crypto.tasks.DefaultRedactEventTask
+import org.matrix.android.sdk.internal.crypto.tasks.RedactEventTask
 import org.matrix.android.sdk.internal.crypto.verification.VerificationMessageProcessor
 import org.matrix.android.sdk.internal.database.DatabaseCleaner
 import org.matrix.android.sdk.internal.database.EventInsertLiveObserver
@@ -367,4 +369,7 @@ internal abstract class SessionModule {
 
     @Binds
     abstract fun bindTypingUsersTracker(tracker: DefaultTypingUsersTracker): TypingUsersTracker
+
+    @Binds
+    abstract fun bindRedactEventTask(task: DefaultRedactEventTask): RedactEventTask
 }
diff --git a/...src/main/java/org/matrix/android/sdk/internal/session/call/DefaultCallSignalingService.kt b/...src/main/java/org/matrix/android/sdk/internal/session/call/DefaultCallSignalingService.kt
@@ -36,8 +36,8 @@ import org.matrix.android.sdk.api.util.NoOpCancellable
 import org.matrix.android.sdk.internal.di.UserId
 import org.matrix.android.sdk.internal.session.SessionScope
 import org.matrix.android.sdk.internal.session.call.model.MxCallImpl
+import org.matrix.android.sdk.internal.session.room.send.queue.EventSenderProcessor
 import org.matrix.android.sdk.internal.session.room.send.LocalEchoEventFactory
-import org.matrix.android.sdk.internal.session.room.send.RoomEventSender
 import org.matrix.android.sdk.internal.task.TaskExecutor
 import org.matrix.android.sdk.internal.task.configureWith
 import timber.log.Timber
@@ -50,7 +50,7 @@ internal class DefaultCallSignalingService @Inject constructor(
         private val userId: String,
         private val activeCallHandler: ActiveCallHandler,
         private val localEchoEventFactory: LocalEchoEventFactory,
-        private val roomEventSender: RoomEventSender,
+        private val eventSenderProcessor: EventSenderProcessor,
         private val taskExecutor: TaskExecutor,
         private val turnServerTask: GetTurnServerTask
 ) : CallSignalingService {
@@ -103,7 +103,7 @@ internal class DefaultCallSignalingService @Inject constructor(
                 otherUserId = otherUserId,
                 isVideoCall = isVideoCall,
                 localEchoEventFactory = localEchoEventFactory,
-                roomEventSender = roomEventSender
+                eventSenderProcessor = eventSenderProcessor
         )
         activeCallHandler.addCall(call).also {
             return call
@@ -165,7 +165,7 @@ internal class DefaultCallSignalingService @Inject constructor(
                             otherUserId = event.senderId ?: return@let,
                             isVideoCall = content.isVideo(),
                             localEchoEventFactory = localEchoEventFactory,
-                            roomEventSender = roomEventSender
+                            eventSenderProcessor = eventSenderProcessor
                     )
                     activeCallHandler.addCall(incomingCall)
                     onCallInvite(incomingCall, content)
-Original file line number
+Diff line change
@@ Expand Up / @@ -5,7 +5,7 @@ Features ✨: @@
      -
     Improvements 🙌:
-     -
+     - Rework sending Event management (#154)
     Bugfix 🐛:
      -
@@ Expand Down @@