From 99ff097fc300be3688bf7c5a111c823ac119f838 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damir=20Jeli=C4=87?= Date: Thu, 22 Jul 2021 12:10:39 +0200 Subject: [PATCH] crypto: Move the update dispatching logic into a separate class --- .../sdk/internal/crypto/QrCodeVerification.kt | 18 ++---- .../sdk/internal/crypto/SasVerification.kt | 18 ++---- .../verification/RustVerificationService.kt | 61 ++++++++++++------- 3 files changed, 46 insertions(+), 51 deletions(-) diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/QrCodeVerification.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/QrCodeVerification.kt index 14bd4e3870..7bde4b4b23 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/QrCodeVerification.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/QrCodeVerification.kt @@ -16,8 +16,6 @@ package org.matrix.android.sdk.internal.crypto -import android.os.Handler -import android.os.Looper import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.runBlocking import kotlinx.coroutines.withContext @@ -27,7 +25,7 @@ import org.matrix.android.sdk.api.session.crypto.verification.VerificationServic import org.matrix.android.sdk.api.session.crypto.verification.VerificationTxState import org.matrix.android.sdk.api.session.crypto.verification.safeValueOf import org.matrix.android.sdk.internal.crypto.crosssigning.fromBase64 -import timber.log.Timber +import org.matrix.android.sdk.internal.crypto.verification.UpdateDispatcher import uniffi.olm.CryptoStoreErrorException import uniffi.olm.OlmMachine import uniffi.olm.OutgoingVerificationRequest @@ -39,20 +37,12 @@ internal class QrCodeVerification( private var request: VerificationRequest, private var inner: QrCode?, private val sender: RequestSender, - private val listeners: ArrayList, + listeners: ArrayList, ) : QrCodeVerificationTransaction { - private val uiHandler = Handler(Looper.getMainLooper()) + private val dispatcher = UpdateDispatcher(listeners) private fun dispatchTxUpdated() { - uiHandler.post { - listeners.forEach { - try { - it.transactionUpdated(this) - } catch (e: Throwable) { - Timber.e(e, "## Error while notifying listeners") - } - } - } + this.dispatcher.dispatchTxUpdated(this) } /** Generate, if possible, data that should be encoded as a QR code for QR code verification. diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/SasVerification.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/SasVerification.kt index 251f56eaae..cd1a7fc188 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/SasVerification.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/SasVerification.kt @@ -16,8 +16,6 @@ package org.matrix.android.sdk.internal.crypto -import android.os.Handler -import android.os.Looper import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.runBlocking import kotlinx.coroutines.withContext @@ -27,8 +25,8 @@ import org.matrix.android.sdk.api.session.crypto.verification.SasVerificationTra import org.matrix.android.sdk.api.session.crypto.verification.VerificationService import org.matrix.android.sdk.api.session.crypto.verification.VerificationTxState import org.matrix.android.sdk.api.session.crypto.verification.safeValueOf +import org.matrix.android.sdk.internal.crypto.verification.UpdateDispatcher import org.matrix.android.sdk.internal.crypto.verification.getEmojiForCode -import timber.log.Timber import uniffi.olm.CryptoStoreErrorException import uniffi.olm.OlmMachine import uniffi.olm.OutgoingVerificationRequest @@ -39,21 +37,13 @@ internal class SasVerification( private val machine: OlmMachine, private var inner: Sas, private val sender: RequestSender, - private val listeners: ArrayList, + listeners: ArrayList, ) : SasVerificationTransaction { - private val uiHandler = Handler(Looper.getMainLooper()) + private val dispatcher = UpdateDispatcher(listeners) private fun dispatchTxUpdated() { - uiHandler.post { - listeners.forEach { - try { - it.transactionUpdated(this) - } catch (e: Throwable) { - Timber.e(e, "## Error while notifying listeners") - } - } - } + this.dispatcher.dispatchTxUpdated(this) } /** The user ID of the other user that is participating in this verification flow */ diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/verification/RustVerificationService.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/verification/RustVerificationService.kt index c3fdc3a314..cd0c296825 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/verification/RustVerificationService.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/verification/RustVerificationService.kt @@ -70,27 +70,24 @@ internal fun prepareMethods(methods: List): List { return stringMethods } -internal class RustVerificationService( - private val olmMachine: OlmMachine, - private val requestSender: RequestSender, -) : VerificationService { +internal class UpdateDispatcher(private val listeners: ArrayList) { private val uiHandler = Handler(Looper.getMainLooper()) - override fun addListener(listener: VerificationService.Listener) { + internal fun addListener(listener: VerificationService.Listener) { uiHandler.post { - if (!this.olmMachine.verificationListeners.contains(listener)) { - this.olmMachine.verificationListeners.add(listener) + if (!this.listeners.contains(listener)) { + this.listeners.add(listener) } } } - override fun removeListener(listener: VerificationService.Listener) { - uiHandler.post { this.olmMachine.verificationListeners.remove(listener) } + internal fun removeListener(listener: VerificationService.Listener) { + uiHandler.post { this.listeners.remove(listener) } } - private fun dispatchTxAdded(tx: VerificationTransaction) { + internal fun dispatchTxAdded(tx: VerificationTransaction) { uiHandler.post { - this.olmMachine.verificationListeners.forEach { + this.listeners.forEach { try { it.transactionCreated(tx) } catch (e: Throwable) { @@ -100,9 +97,9 @@ internal class RustVerificationService( } } - private fun dispatchTxUpdated(tx: VerificationTransaction) { + internal fun dispatchTxUpdated(tx: VerificationTransaction) { uiHandler.post { - this.olmMachine.verificationListeners.forEach { + this.listeners.forEach { try { it.transactionUpdated(tx) } catch (e: Throwable) { @@ -112,10 +109,10 @@ internal class RustVerificationService( } } - private fun dispatchRequestAdded(tx: PendingVerificationRequest) { + internal fun dispatchRequestAdded(tx: PendingVerificationRequest) { Timber.v("## SAS dispatchRequestAdded txId:${tx.transactionId} $tx") uiHandler.post { - this.olmMachine.verificationListeners.forEach { + this.listeners.forEach { try { it.verificationRequestCreated(tx) } catch (e: Throwable) { @@ -124,8 +121,18 @@ internal class RustVerificationService( } } } +} + +internal class RustVerificationService( + private val olmMachine: OlmMachine, + private val requestSender: RequestSender, +) : VerificationService { + private val dispatcher = UpdateDispatcher(this.olmMachine.verificationListeners) internal suspend fun onEvent(event: Event) = when (event.getClearType()) { + // I'm not entirely sure why getClearType() returns a msgtype in one case + // and a event type in the other case, but this is how the old verification + // service did things and it does seem to work. MessageType.MSGTYPE_VERIFICATION_REQUEST -> onRequest(event) EventType.KEY_VERIFICATION_START -> onStart(event) EventType.KEY_VERIFICATION_READY, @@ -144,7 +151,7 @@ internal class RustVerificationService( this.olmMachine.getVerificationRequest(sender, flowId)?.dispatchRequestUpdated() val verification = this.getExistingTransaction(sender, flowId) ?: return - dispatchTxUpdated(verification) + this.dispatcher.dispatchTxUpdated(verification) } private suspend fun onStart(event: Event) { @@ -163,15 +170,15 @@ internal class RustVerificationService( Timber.d("## Verification: Auto accepting SAS verification with $sender") verification.accept() } else { - dispatchTxUpdated(verification) + this.dispatcher.dispatchTxUpdated(verification) } } else { // This didn't originate from a request, so tell our listeners that // this is a new verification. - dispatchTxAdded(verification) + this.dispatcher.dispatchTxAdded(verification) // The IncomingVerificationRequestHandler seems to only listen to updates // so let's trigger an update after the addition as well. - dispatchTxUpdated(verification) + this.dispatcher.dispatchTxUpdated(verification) } } @@ -181,7 +188,15 @@ internal class RustVerificationService( val request = this.getExistingVerificationRequest(sender, flowId) ?: return - dispatchRequestAdded(request) + this.dispatcher.dispatchRequestAdded(request) + } + + override fun addListener(listener: VerificationService.Listener) { + this.dispatcher.addListener(listener) + } + + override fun removeListener(listener: VerificationService.Listener) { + this.dispatcher.removeListener(listener) } override fun markedLocallyAsManuallyVerified(userId: String, deviceID: String) { @@ -303,7 +318,7 @@ internal class RustVerificationService( val qrcode = request.startQrVerification() if (qrcode != null) { - dispatchTxAdded(qrcode) + this.dispatcher.dispatchTxAdded(qrcode) } true @@ -338,7 +353,7 @@ internal class RustVerificationService( val sas = request?.startSasVerification() if (sas != null) { - dispatchTxAdded(sas) + dispatcher.dispatchTxAdded(sas) sas.transactionId } else { null @@ -353,7 +368,7 @@ internal class RustVerificationService( runBlocking { val verification = olmMachine.getDevice(otherUserId, otherDeviceId)?.startVerification() if (verification != null) { - dispatchTxAdded(verification) + dispatcher.dispatchTxAdded(verification) verification.transactionId } else { null