fix sas test

matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/internal/crypto/verification/SasVerificationTestHelper.kt

@@ -16,56 +16,71 @@
 
 package org.matrix.android.sdk.internal.crypto.verification
 
-import org.amshove.kluent.fail
+import kotlinx.coroutines.CompletableDeferred
+import kotlinx.coroutines.CoroutineScope
+import kotlinx.coroutines.Dispatchers
+import kotlinx.coroutines.cancel
+import kotlinx.coroutines.flow.cancellable
+import kotlinx.coroutines.launch
 import org.matrix.android.sdk.api.session.Session
 import org.matrix.android.sdk.api.session.crypto.verification.EVerificationState
+import org.matrix.android.sdk.api.session.crypto.verification.PendingVerificationRequest
 import org.matrix.android.sdk.api.session.crypto.verification.VerificationMethod
+import org.matrix.android.sdk.api.session.crypto.verification.getRequest
 import org.matrix.android.sdk.common.CommonTestHelper
 import org.matrix.android.sdk.common.CryptoTestData
 
 class SasVerificationTestHelper(private val testHelper: CommonTestHelper) {
-    suspend fun requestVerificationAndWaitForReadyState(cryptoTestData: CryptoTestData, supportedMethods: List<VerificationMethod>): String {
+    suspend fun requestVerificationAndWaitForReadyState(
+            scope: CoroutineScope,
+            cryptoTestData: CryptoTestData, supportedMethods: List<VerificationMethod>
+    ): String {
         val aliceSession = cryptoTestData.firstSession
         val bobSession = cryptoTestData.secondSession!!
 
         val aliceVerificationService = aliceSession.cryptoService().verificationService()
         val bobVerificationService = bobSession.cryptoService().verificationService()
 
+        val bobSeesVerification = CompletableDeferred<PendingVerificationRequest>()
+        scope.launch(Dispatchers.IO) {
+            bobVerificationService.requestEventFlow()
+                    .cancellable()
+                    .collect {
+                        val request = it.getRequest()
+                        if (request != null) {
+                            bobSeesVerification.complete(request)
+                            return@collect cancel()
+                        }
+                    }
+        }
+
         val bobUserId = bobSession.myUserId
         // Step 1: Alice starts a verification request
         val transactionId = aliceVerificationService.requestKeyVerificationInDMs(
                 supportedMethods, bobUserId, cryptoTestData.roomId
+        ).transactionId
+
+        val aliceReady = CompletableDeferred<PendingVerificationRequest>()
+        scope.launch(Dispatchers.IO) {
+            aliceVerificationService.requestEventFlow()
+                    .cancellable()
+                    .collect {
+                        val request = it.getRequest()
+                        if (request?.state == EVerificationState.Ready) {
+                            aliceReady.complete(request)
+                            return@collect cancel()
+                        }
+                    }
+        }
+
+        bobSeesVerification.await()
+        bobVerificationService.readyPendingVerification(
+                supportedMethods,
+                aliceSession.myUserId,
+                transactionId
         )
-                .transactionId
-
-        testHelper.retryWithBackoff(
-                onFail = {
-                    fail("bob should see an incoming verification request with id $transactionId")
-                }
-        ) {
-            val incomingRequest = bobVerificationService.getExistingVerificationRequest(aliceSession.myUserId, transactionId)
-            if (incomingRequest != null) {
-                bobVerificationService.readyPendingVerification(
-                        supportedMethods,
-                        aliceSession.myUserId,
-                        incomingRequest.transactionId
-                )
-                true
-            } else {
-                false
-            }
-        }
-
-        // wait for alice to see the ready
-        testHelper.retryWithBackoff(
-                onFail = {
-                    fail("Alice request whould be ready $transactionId")
-                }
-        ) {
-            val pendingRequest = aliceVerificationService.getExistingVerificationRequest(bobUserId, transactionId)
-            pendingRequest?.state == EVerificationState.Ready
-        }
 
+        aliceReady.await()
         return transactionId
     }
 

matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/internal/crypto/verification/VerificationTest.kt

@@ -17,6 +17,13 @@
 package org.matrix.android.sdk.internal.crypto.verification
 
 import androidx.test.ext.junit.runners.AndroidJUnit4
+import kotlinx.coroutines.CompletableDeferred
+import kotlinx.coroutines.CoroutineScope
+import kotlinx.coroutines.Dispatchers
+import kotlinx.coroutines.SupervisorJob
+import kotlinx.coroutines.cancel
+import kotlinx.coroutines.flow.cancellable
+import kotlinx.coroutines.launch
 import org.amshove.kluent.shouldBe
 import org.junit.FixMethodOrder
 import org.junit.Test
@@ -24,7 +31,9 @@ import org.junit.runner.RunWith
 import org.junit.runners.MethodSorters
 import org.matrix.android.sdk.InstrumentedTest
 import org.matrix.android.sdk.api.session.crypto.verification.EVerificationState
+import org.matrix.android.sdk.api.session.crypto.verification.PendingVerificationRequest
 import org.matrix.android.sdk.api.session.crypto.verification.VerificationMethod
+import org.matrix.android.sdk.api.session.crypto.verification.getRequest
 import org.matrix.android.sdk.common.CommonTestHelper.Companion.runCryptoTest
 
 @RunWith(AndroidJUnit4::class)
@@ -142,7 +151,7 @@ class VerificationTest : InstrumentedTest {
             bobSupportedMethods: List<VerificationMethod>,
             expectedResultForAlice: ExpectedResult,
             expectedResultForBob: ExpectedResult
-    ) = runCryptoTest(context()) { cryptoTestHelper, testHelper ->
+    ) = runCryptoTest(context()) { cryptoTestHelper, _ ->
         val cryptoTestData = cryptoTestHelper.doE2ETestWithAliceAndBobInARoom()
 
         val aliceSession = cryptoTestData.firstSession
@@ -151,49 +160,76 @@ class VerificationTest : InstrumentedTest {
         cryptoTestHelper.initializeCrossSigning(aliceSession)
         cryptoTestHelper.initializeCrossSigning(bobSession)
 
+        val scope = CoroutineScope(SupervisorJob())
+
         val aliceVerificationService = aliceSession.cryptoService().verificationService()
         val bobVerificationService = bobSession.cryptoService().verificationService()
 
-        val transactionId = aliceVerificationService.requestKeyVerificationInDMs(
+        val bobSeesVerification = CompletableDeferred<PendingVerificationRequest>()
-                aliceSupportedMethods, bobSession.myUserId, cryptoTestData.roomId
+        scope.launch(Dispatchers.IO) {
+            bobVerificationService.requestEventFlow()
+                    .cancellable()
+                    .collect {
+                        val request = it.getRequest()
+                        if (request != null) {
+                            bobSeesVerification.complete(request)
+                            return@collect cancel()
+                        }
+                    }
+        }
+
+        val aliceReady = CompletableDeferred<PendingVerificationRequest>()
+        scope.launch(Dispatchers.IO) {
+            aliceVerificationService.requestEventFlow()
+                    .cancellable()
+                    .collect {
+                        val request = it.getRequest()
+                        if (request?.state == EVerificationState.Ready) {
+                            aliceReady.complete(request)
+                            return@collect cancel()
+                        }
+                    }
+        }
+        val bobReady = CompletableDeferred<PendingVerificationRequest>()
+        scope.launch(Dispatchers.IO) {
+            bobVerificationService.requestEventFlow()
+                    .cancellable()
+                    .collect {
+                        val request = it.getRequest()
+                        if (request?.state == EVerificationState.Ready) {
+                            bobReady.complete(request)
+                            return@collect cancel()
+                        }
+                    }
+        }
+
+        val requestID = aliceVerificationService.requestKeyVerificationInDMs(
+                methods = aliceSupportedMethods,
+                otherUserId = bobSession.myUserId,
+                roomId = cryptoTestData.roomId
+        ).transactionId
+
+        bobSeesVerification.await()
+        bobVerificationService.readyPendingVerification(
+                bobSupportedMethods,
+                aliceSession.myUserId,
+                requestID
         )
-                .transactionId
+        val aliceRequest = aliceReady.await()
+        val bobRequest = bobReady.await()
 
-        testHelper.retryPeriodically {
+        aliceRequest.let { pr ->
-            val incomingRequest = bobVerificationService.getExistingVerificationRequest(aliceSession.myUserId, transactionId)
-            if (incomingRequest != null) {
-                bobVerificationService.readyPendingVerification(
-                        bobSupportedMethods,
-                        aliceSession.myUserId,
-                        incomingRequest.transactionId
-                )
-                true
-            } else {
-                false
-            }
-        }
-
-        // wait for alice to see the ready
-        testHelper.retryPeriodically {
-            val pendingRequest = aliceVerificationService.getExistingVerificationRequest(bobSession.myUserId, transactionId)
-            pendingRequest?.state == EVerificationState.Ready
-        }
-
-        val aliceReadyPendingVerificationRequest = aliceVerificationService.getExistingVerificationRequest(bobSession.myUserId, transactionId)!!
-        val bobReadyPendingVerificationRequest = bobVerificationService.getExistingVerificationRequest(aliceSession.myUserId, transactionId)!!
-
-        aliceReadyPendingVerificationRequest.let { pr ->
             pr.isSasSupported shouldBe expectedResultForAlice.sasIsSupported
             pr.weShouldShowScanOption shouldBe expectedResultForAlice.otherCanShowQrCode
             pr.weShouldDisplayQRCode shouldBe expectedResultForAlice.otherCanScanQrCode
         }
 
-        bobReadyPendingVerificationRequest.let { pr ->
+        bobRequest.let { pr ->
             pr.isSasSupported shouldBe expectedResultForBob.sasIsSupported
             pr.weShouldShowScanOption shouldBe expectedResultForBob.otherCanShowQrCode
             pr.weShouldDisplayQRCode shouldBe expectedResultForBob.otherCanScanQrCode
         }
 
-        cryptoTestData.cleanUp(testHelper)
+        scope.cancel()
     }
 }

matrix-sdk-android/src/androidTestKotlinCrypto/java/org/matrix/android/sdk/internal/crypto/verification/SASTest.kt

@@ -16,6 +16,7 @@
 
 package org.matrix.android.sdk.internal.crypto.verification
 
+import android.util.Log
 import androidx.test.ext.junit.runners.AndroidJUnit4
 import kotlinx.coroutines.CompletableDeferred
 import kotlinx.coroutines.CoroutineScope
@@ -35,10 +36,10 @@ import org.matrix.android.sdk.api.session.crypto.verification.CancelCode
 import org.matrix.android.sdk.api.session.crypto.verification.EVerificationState
 import org.matrix.android.sdk.api.session.crypto.verification.SasTransactionState
 import org.matrix.android.sdk.api.session.crypto.verification.SasVerificationTransaction
-import org.matrix.android.sdk.api.session.crypto.verification.VerificationEvent
 import org.matrix.android.sdk.api.session.crypto.verification.VerificationMethod
+import org.matrix.android.sdk.api.session.crypto.verification.dbgState
+import org.matrix.android.sdk.api.session.crypto.verification.getTransaction
 import org.matrix.android.sdk.common.CommonTestHelper.Companion.runCryptoTest
-import timber.log.Timber
 
 @RunWith(AndroidJUnit4::class)
 @FixMethodOrder(MethodSorters.NAME_ASCENDING)
@@ -49,9 +50,9 @@ class SASTest : InstrumentedTest {
     @Test
     fun test_aliceStartThenAliceCancel() = runCryptoTest(context()) { cryptoTestHelper, testHelper ->
 
-        Timber.v("verification: doE2ETestWithAliceAndBobInARoom")
+        Log.d("#E2E", "verification: doE2ETestWithAliceAndBobInARoom")
         val cryptoTestData = cryptoTestHelper.doE2ETestWithAliceAndBobInARoom()
-        Timber.v("verification: initializeCrossSigning")
+        Log.d("#E2E", "verification: initializeCrossSigning")
         cryptoTestData.initializeCrossSigning(cryptoTestHelper)
         val aliceSession = cryptoTestData.firstSession
         val bobSession = cryptoTestData.secondSession
@@ -59,19 +60,20 @@ class SASTest : InstrumentedTest {
         val aliceVerificationService = aliceSession.cryptoService().verificationService()
         val bobVerificationService = bobSession!!.cryptoService().verificationService()
 
-        Timber.v("verification: requestVerificationAndWaitForReadyState")
+        Log.d("#E2E", "verification: requestVerificationAndWaitForReadyState")
         val txId = SasVerificationTestHelper(testHelper)
-                .requestVerificationAndWaitForReadyState(cryptoTestData, listOf(VerificationMethod.SAS))
+                .requestVerificationAndWaitForReadyState(scope, cryptoTestData, listOf(VerificationMethod.SAS))
 
-        Timber.v("verification: startKeyVerification")
+        Log.d("#E2E", "verification: startKeyVerification")
         aliceVerificationService.startKeyVerification(
                 VerificationMethod.SAS,
                 bobSession.myUserId,
                 txId
         )
 
-        Timber.v("verification: ensure bob has received starete")
+        Log.d("#E2E", "verification: ensure bob has received start")
         testHelper.retryWithBackoff {
+            Log.d("#E2E", "verification: ${bobVerificationService.getExistingVerificationRequest(aliceSession.myUserId, txId)?.state}")
             bobVerificationService.getExistingVerificationRequest(aliceSession.myUserId, txId)?.state == EVerificationState.Started
         }
 
@@ -85,41 +87,35 @@ class SASTest : InstrumentedTest {
 
         assertEquals("Alice and Bob have same transaction id", aliceKeyTx!!.transactionId, bobKeyTx!!.transactionId)
 
-        val aliceCancelled = CompletableDeferred<Unit>()
+        val aliceCancelled = CompletableDeferred<SasTransactionState.Cancelled>()
         aliceVerificationService.requestEventFlow().onEach {
-            println("alice flow event $it")
+            Log.d("#E2E", "alice flow event $it | ${it.getTransaction()?.dbgState()}")
-            if (it is VerificationEvent.TransactionUpdated && it.transactionId == txId) {
+            val tx = it.getTransaction()
-                val sasVerificationTransaction = it.transaction as SasVerificationTransaction
+            if (tx?.transactionId == txId && tx is SasVerificationTransaction) {
-                if (sasVerificationTransaction.state() is SasTransactionState.Cancelled) {
+                if (tx.state() is SasTransactionState.Cancelled) {
-                    aliceCancelled.complete(Unit)
+                    aliceCancelled.complete(tx.state() as SasTransactionState.Cancelled)
                 }
             }
         }.launchIn(scope)
 
-        val bobCancelled = CompletableDeferred<Unit>()
+        val bobCancelled = CompletableDeferred<SasTransactionState.Cancelled>()
         bobVerificationService.requestEventFlow().onEach {
-            println("alice flow event $it")
+            Log.d("#E2E", "bob flow event $it | ${it.getTransaction()?.dbgState()}")
-            if (it is VerificationEvent.TransactionUpdated && it.transactionId == txId) {
+            val tx = it.getTransaction()
-                val sasVerificationTransaction = it.transaction as SasVerificationTransaction
+            if (tx?.transactionId == txId && tx is SasVerificationTransaction) {
-                if (sasVerificationTransaction.state() is SasTransactionState.Cancelled) {
+                if (tx.state() is SasTransactionState.Cancelled) {
-                    bobCancelled.complete(Unit)
+                    bobCancelled.complete(tx.state() as SasTransactionState.Cancelled)
                 }
             }
         }.launchIn(scope)
 
         aliceVerificationService.cancelVerificationRequest(bobSession.myUserId, txId)
 
-        aliceCancelled.await()
+        val cancelledAlice = aliceCancelled.await()
-        bobCancelled.await()
+        val cancelledBob = bobCancelled.await()
 
-        val cancelledAlice = aliceVerificationService.getExistingVerificationRequest(bobSession.myUserId, txId)!!
+        assertEquals("Should be User cancelled on alice side", CancelCode.User, cancelledAlice.cancelCode)
-        val cancelledBob = aliceVerificationService.getExistingVerificationRequest(aliceSession.myUserId, txId)!!
+        assertEquals("Should be User cancelled on bob side", CancelCode.User, cancelledBob.cancelCode)
-
-        assertEquals("Should be cancelled on alice side", cancelledAlice.state, EVerificationState.Cancelled)
-        assertEquals("Should be cancelled on alice side", cancelledBob.state, EVerificationState.Cancelled)
-
-        assertEquals("Should be User cancelled on alice side", CancelCode.User, cancelledAlice.cancelConclusion)
-        assertEquals("Should be User cancelled on bob side", CancelCode.User, cancelledBob.cancelConclusion)
 
         assertNull(bobVerificationService.getExistingTransaction(aliceSession.myUserId, txId))
         assertNull(aliceVerificationService.getExistingTransaction(bobSession.myUserId, txId))

matrix-sdk-android/src/kotlinCrypto/java/org/matrix/android/sdk/internal/crypto/verification/VerificationActor.kt

@@ -271,15 +271,15 @@ internal class VerificationActor @AssistedInject constructor(
             is VerificationIntent.GetExistingTransaction -> {
                 verificationRequestsStore
                         .getExistingTransaction(msg.fromUser, msg.transactionId)
-                        ?.let {
+                        .let {
                             msg.deferred.complete(it)
                         }
             }
             is VerificationIntent.GetExistingRequest -> {
                 verificationRequestsStore
                         .getExistingRequest(msg.otherUserId, msg.transactionId)
-                        ?.let {
+                        .let {
-                            msg.deferred.complete(it.toPendingVerificationRequest())
+                            msg.deferred.complete(it?.toPendingVerificationRequest())
                         }
             }
             is VerificationIntent.OnCancelReceived -> {