From b275546a8ac57cf7722c032e55a22225775f0210 Mon Sep 17 00:00:00 2001
From: Valere <valeref@matrix.org>
Date: Wed, 5 Jan 2022 16:42:45 +0100
Subject: [PATCH] Code review + reduce grace period
---
.../algorithms/megolm/MXMegolmDecryption.kt | 4 +--
.../analytics/DecryptionFailureTracker.kt | 27 +++++++++++--------
.../timeline/factory/TimelineItemFactory.kt | 2 +-
3 files changed, 19 insertions(+), 14 deletions(-)
diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/algorithms/megolm/MXMegolmDecryption.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/algorithms/megolm/MXMegolmDecryption.kt
index 441b501360..ceceedc802 100644
--- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/algorithms/megolm/MXMegolmDecryption.kt
+++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/algorithms/megolm/MXMegolmDecryption.kt
@@ -136,8 +136,8 @@ internal class MXMegolmDecryption(private val userId: String,
throw MXCryptoError.Base(
MXCryptoError.ErrorType.UNKNOWN_MESSAGE_INDEX,
- throwable.olmException.message ?: "",
- throwable.olmException.message)
+ "UNKNOWN_MESSAGE_INDEX",
+ null)
}
val reason = String.format(MXCryptoError.OLM_REASON, throwable.olmException.message)
diff --git a/vector/src/main/java/im/vector/app/features/analytics/DecryptionFailureTracker.kt b/vector/src/main/java/im/vector/app/features/analytics/DecryptionFailureTracker.kt
index c74a65c8ed..cd98356445 100644
--- a/vector/src/main/java/im/vector/app/features/analytics/DecryptionFailureTracker.kt
+++ b/vector/src/main/java/im/vector/app/features/analytics/DecryptionFailureTracker.kt
@@ -17,6 +17,7 @@
package im.vector.app.features.analytics
import im.vector.app.core.flow.tickerFlow
+import im.vector.app.core.time.Clock
import im.vector.app.features.analytics.plan.Error
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.Dispatchers
@@ -26,17 +27,19 @@ import kotlinx.coroutines.flow.launchIn
import kotlinx.coroutines.flow.onEach
import kotlinx.coroutines.launch
import org.matrix.android.sdk.api.session.crypto.MXCryptoError
+import org.matrix.android.sdk.api.session.room.timeline.TimelineEvent
import javax.inject.Inject
import javax.inject.Singleton
-data class DecryptionFailure(
- val timeStamp: Long = System.currentTimeMillis(),
+private data class DecryptionFailure(
+ val timeStamp: Long,
val roomId: String,
val failedEventId: String,
val error: MXCryptoError.ErrorType
)
-private const val GRACE_PERIOD_MILLIS = 20_000
+private const val GRACE_PERIOD_MILLIS = 4_000
+private const val CHECK_INTERVAL = 2_000L
/**
* Tracks decryption errors that are visible to the user.
@@ -45,7 +48,8 @@ private const val GRACE_PERIOD_MILLIS = 20_000
*/
@Singleton
class DecryptionFailureTracker @Inject constructor(
- private val vectorAnalytics: VectorAnalytics
+ private val vectorAnalytics: VectorAnalytics,
+ private val clock: Clock
) {
private val scope: CoroutineScope = CoroutineScope(SupervisorJob())
@@ -57,7 +61,7 @@ class DecryptionFailureTracker @Inject constructor(
}
fun start() {
- tickerFlow(scope, 5_000)
+ tickerFlow(scope, CHECK_INTERVAL)
.onEach {
checkFailures()
}.launchIn(scope)
@@ -67,12 +71,13 @@ class DecryptionFailureTracker @Inject constructor(
scope.cancel()
}
- fun e2eEventDisplayedInTimeline(roomId: String, eventId: String, error: MXCryptoError.ErrorType?) {
+ fun e2eEventDisplayedInTimeline(event: TimelineEvent) {
scope.launch(Dispatchers.Default) {
- if (error != null) {
- addDecryptionFailure(DecryptionFailure(roomId = roomId, failedEventId = eventId, error = error))
+ val mCryptoError = event.root.mCryptoError
+ if (mCryptoError != null) {
+ addDecryptionFailure(DecryptionFailure(clock.epochMillis(), event.roomId, event.eventId, mCryptoError))
} else {
- removeFailureForEventId(eventId)
+ removeFailureForEventId(event.eventId)
}
}
}
@@ -92,7 +97,7 @@ class DecryptionFailureTracker @Inject constructor(
private fun addDecryptionFailure(failure: DecryptionFailure) {
// de duplicate
synchronized(failures) {
- if (failures.indexOfFirst { it.failedEventId == failure.failedEventId } == -1) {
+ if (failures.none { it.failedEventId == failure.failedEventId }) {
failures.add(failure)
}
}
@@ -105,7 +110,7 @@ class DecryptionFailureTracker @Inject constructor(
}
private fun checkFailures() {
- val now = System.currentTimeMillis()
+ val now = clock.epochMillis()
val aggregatedErrors: Map<Error.Name, List<String>>
synchronized(failures) {
val toReport = mutableListOf<DecryptionFailure>()
diff --git a/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/factory/TimelineItemFactory.kt b/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/factory/TimelineItemFactory.kt
index a8c578a58a..dfe1cc1d9b 100644
--- a/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/factory/TimelineItemFactory.kt
+++ b/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/factory/TimelineItemFactory.kt
@@ -126,7 +126,7 @@ class TimelineItemFactory @Inject constructor(private val messageItemFactory: Me
}
}.also {
if (it != null && event.isEncrypted()) {
- decryptionFailureTracker.e2eEventDisplayedInTimeline(event.roomId, event.eventId, event.root.mCryptoError)
+ decryptionFailureTracker.e2eEventDisplayedInTimeline(event)
}
}
}