Fix instrumentation tests and group member regression.

signalapp · Mar 4, 2023 · e303e80 · e303e80
1 parent 018f6ac
commit e303e80
Show file tree

Hide file tree

Showing 6 changed files with 61 additions and 12 deletions.
diff --git a/.../thoughtcrime/securesms/components/settings/app/changenumber/ChangeNumberViewModelTest.kt b/.../thoughtcrime/securesms/components/settings/app/changenumber/ChangeNumberViewModelTest.kt
@@ -22,6 +22,7 @@ import org.thoughtcrime.securesms.registration.VerifyAccountRepository
 import org.thoughtcrime.securesms.registration.VerifyResponseProcessor
 import org.thoughtcrime.securesms.testing.Get
 import org.thoughtcrime.securesms.testing.MockProvider
+import org.thoughtcrime.securesms.testing.Post
 import org.thoughtcrime.securesms.testing.Put
 import org.thoughtcrime.securesms.testing.SignalActivityRule
 import org.thoughtcrime.securesms.testing.assertIs
@@ -82,8 +83,10 @@ class ChangeNumberViewModelTest {
     lateinit var setPreKeysRequest: PreKeyState
 
     InstrumentationApplicationDependencyProvider.addMockWebRequestHandlers(
+      Post("/v1/verification/session") { MockResponse().success(MockProvider.sessionMetadataJson.copy(verified = false)) },
+      Put("/v1/verification/session/${MockProvider.sessionMetadataJson.id}/code") { MockResponse().success(MockProvider.sessionMetadataJson) },
       Get("/v1/devices") { MockResponse().success(MockProvider.primaryOnlyDeviceList) },
-      Put("/v1/accounts/number") { r ->
+      Put("/v2/accounts/number") { r ->
         changeNumberRequest = r.parsedRequestBody()
         MockResponse().success(MockProvider.createVerifyAccountResponse(aci, newPni))
       },
@@ -95,6 +98,7 @@ class ChangeNumberViewModelTest {
     )
 
     // WHEN
+    viewModel.requestVerificationCode(VerifyAccountRepository.Mode.SMS_WITHOUT_LISTENER, null, null).blockingGet().resultOrThrow
     viewModel.verifyCodeWithoutRegistrationLock("123456").blockingGet().resultOrThrow
 
     // THEN
@@ -112,11 +116,14 @@ class ChangeNumberViewModelTest {
     val oldE164 = Recipient.self().requireE164()
 
     InstrumentationApplicationDependencyProvider.addMockWebRequestHandlers(
+      Post("/v1/verification/session") { MockResponse().success(MockProvider.sessionMetadataJson.copy(verified = false)) },
+      Put("/v1/verification/session/${MockProvider.sessionMetadataJson.id}/code") { MockResponse().success(MockProvider.sessionMetadataJson) },
       Get("/v1/devices") { MockResponse().success(MockProvider.primaryOnlyDeviceList) },
-      Put("/v1/accounts/number") { MockResponse().failure(500) }
+      Put("/v2/accounts/number") { MockResponse().failure(500) }
     )
 
     // WHEN
+    viewModel.requestVerificationCode(VerifyAccountRepository.Mode.SMS_WITHOUT_LISTENER, null, null).blockingGet().resultOrThrow
     val processor: VerifyResponseProcessor = viewModel.verifyCodeWithoutRegistrationLock("123456").blockingGet()
 
     // THEN
@@ -142,12 +149,15 @@ class ChangeNumberViewModelTest {
     val oldE164 = Recipient.self().requireE164()
 
     InstrumentationApplicationDependencyProvider.addMockWebRequestHandlers(
+      Post("/v1/verification/session") { MockResponse().success(MockProvider.sessionMetadataJson.copy(verified = false)) },
+      Put("/v1/verification/session/${MockProvider.sessionMetadataJson.id}/code") { MockResponse().success(MockProvider.sessionMetadataJson) },
       Get("/v1/devices") { MockResponse().success(MockProvider.primaryOnlyDeviceList) },
-      Put("/v1/accounts/number") { MockResponse().connectionFailure() },
+      Put("/v2/accounts/number") { MockResponse().connectionFailure() },
       Get("/v1/accounts/whoami") { MockResponse().success(MockProvider.createWhoAmIResponse(aci, oldPni, oldE164)) }
     )
 
     // WHEN
+    viewModel.requestVerificationCode(VerifyAccountRepository.Mode.SMS_WITHOUT_LISTENER, null, null).blockingGet().resultOrThrow
     val processor: VerifyResponseProcessor = viewModel.verifyCodeWithoutRegistrationLock("123456").blockingGet()
 
     // THEN
@@ -181,8 +191,10 @@ class ChangeNumberViewModelTest {
     lateinit var setPreKeysRequest: PreKeyState
 
     InstrumentationApplicationDependencyProvider.addMockWebRequestHandlers(
+      Post("/v1/verification/session") { MockResponse().success(MockProvider.sessionMetadataJson.copy(verified = false)) },
+      Put("/v1/verification/session/${MockProvider.sessionMetadataJson.id}/code") { MockResponse().success(MockProvider.sessionMetadataJson) },
       Get("/v1/devices") { MockResponse().success(MockProvider.primaryOnlyDeviceList) },
-      Put("/v1/accounts/number") { r ->
+      Put("/v2/accounts/number") { r ->
         changeNumberRequest = r.parsedRequestBody()
         MockResponse().timeout()
       },
@@ -195,6 +207,7 @@ class ChangeNumberViewModelTest {
     )
 
     // WHEN
+    viewModel.requestVerificationCode(VerifyAccountRepository.Mode.SMS_WITHOUT_LISTENER, null, null).blockingGet().resultOrThrow
     val processor: VerifyResponseProcessor = viewModel.verifyCodeWithoutRegistrationLock("123456").blockingGet()
 
     // THEN
@@ -225,8 +238,10 @@ class ChangeNumberViewModelTest {
     MockProvider.mockGetRegistrationLockStringFlow(kbsRepository)
 
     InstrumentationApplicationDependencyProvider.addMockWebRequestHandlers(
+      Post("/v1/verification/session") { MockResponse().success(MockProvider.sessionMetadataJson.copy(verified = false)) },
+      Put("/v1/verification/session/${MockProvider.sessionMetadataJson.id}/code") { MockResponse().success(MockProvider.sessionMetadataJson) },
       Get("/v1/devices") { MockResponse().success(MockProvider.primaryOnlyDeviceList) },
-      Put("/v1/accounts/number") { r ->
+      Put("/v2/accounts/number") { r ->
         changeNumberRequest = r.parsedRequestBody()
         if (changeNumberRequest.registrationLock.isNullOrEmpty()) {
           MockResponse().failure(423, MockProvider.lockedFailure)
@@ -242,6 +257,7 @@ class ChangeNumberViewModelTest {
     )
 
     // WHEN
+    viewModel.requestVerificationCode(VerifyAccountRepository.Mode.SMS_WITHOUT_LISTENER, null, null).blockingGet().resultOrThrow
     viewModel.verifyCodeWithoutRegistrationLock("123456").blockingGet().also { processor ->
       processor.registrationLock() assertIs true
       Recipient.self().requirePni() assertIsNot newPni
@@ -263,8 +279,10 @@ class ChangeNumberViewModelTest {
     lateinit var setPreKeysRequest: PreKeyState
 
     InstrumentationApplicationDependencyProvider.addMockWebRequestHandlers(
+      Post("/v1/verification/session") { MockResponse().success(MockProvider.sessionMetadataJson.copy(verified = false)) },
+      Put("/v1/verification/session/${MockProvider.sessionMetadataJson.id}/code") { MockResponse().success(MockProvider.sessionMetadataJson) },
       Get("/v1/devices") { MockResponse().success(MockProvider.primaryOnlyDeviceList) },
-      Put("/v1/accounts/number") { r ->
+      Put("/v2/accounts/number") { r ->
         changeNumberRequest = r.parsedRequestBody()
         if (changeNumberRequest.deviceMessages.isEmpty()) {
           MockResponse().failure(
@@ -289,6 +307,7 @@ class ChangeNumberViewModelTest {
     )
 
     // WHEN
+    viewModel.requestVerificationCode(VerifyAccountRepository.Mode.SMS_WITHOUT_LISTENER, null, null).blockingGet().resultOrThrow
     viewModel.verifyCodeWithoutRegistrationLock("123456").blockingGet().resultOrThrow
 
     // THEN
@@ -307,7 +326,9 @@ class ChangeNumberViewModelTest {
     MockProvider.mockGetRegistrationLockStringFlow(kbsRepository)
 
     InstrumentationApplicationDependencyProvider.addMockWebRequestHandlers(
-      Put("/v1/accounts/number") { r ->
+      Post("/v1/verification/session") { MockResponse().success(MockProvider.sessionMetadataJson.copy(verified = false)) },
+      Put("/v1/verification/session/${MockProvider.sessionMetadataJson.id}/code") { MockResponse().success(MockProvider.sessionMetadataJson) },
+      Put("/v2/accounts/number") { r ->
         changeNumberRequest = r.parsedRequestBody()
         if (changeNumberRequest.registrationLock.isNullOrEmpty()) {
           MockResponse().failure(423, MockProvider.lockedFailure)
@@ -345,6 +366,7 @@ class ChangeNumberViewModelTest {
     )
 
     // WHEN
+    viewModel.requestVerificationCode(VerifyAccountRepository.Mode.SMS_WITHOUT_LISTENER, null, null).blockingGet().resultOrThrow
     viewModel.verifyCodeWithoutRegistrationLock("123456").blockingGet().also { processor ->
       processor.registrationLock() assertIs true
       Recipient.self().requirePni() assertIsNot newPni

diff --git a/app/src/androidTest/java/org/thoughtcrime/securesms/database/GroupTableTest.kt b/app/src/androidTest/java/org/thoughtcrime/securesms/database/GroupTableTest.kt
@@ -119,7 +119,7 @@ class GroupTableTest {
     }
 
     val groupRecord = groupTable.getGroup(v2Group).get()
-    assertEquals(groupRecord.members.toSet(), setOf(harness.self.id, harness.others[1]))
+    assertEquals(setOf(harness.self.id, harness.others[1]), groupRecord.members.toSet())
   }
 
   @Test

diff --git a/app/src/androidTest/java/org/thoughtcrime/securesms/testing/MockProvider.kt b/app/src/androidTest/java/org/thoughtcrime/securesms/testing/MockProvider.kt
@@ -32,6 +32,7 @@ import org.whispersystems.signalservice.internal.push.PreKeyEntity
 import org.whispersystems.signalservice.internal.push.PreKeyResponse
 import org.whispersystems.signalservice.internal.push.PreKeyResponseItem
 import org.whispersystems.signalservice.internal.push.PushServiceSocket
+import org.whispersystems.signalservice.internal.push.RegistrationSessionMetadataJson
 import org.whispersystems.signalservice.internal.push.SenderCertificate
 import org.whispersystems.signalservice.internal.push.VerifyAccountResponse
 import org.whispersystems.signalservice.internal.push.WhoAmIResponse
@@ -56,6 +57,16 @@ object MockProvider {
     )
   }
 
+  val sessionMetadataJson = RegistrationSessionMetadataJson(
+    id = "asdfasdfasdfasdf",
+    nextCall = null,
+    nextSms = null,
+    nextVerificationAttempt = null,
+    allowedToRequestCode = true,
+    requestedInformation = emptyList(),
+    verified = true
+  )
+
   fun createVerifyAccountResponse(aci: ServiceId, newPni: ServiceId): VerifyAccountResponse {
     return VerifyAccountResponse().apply {
       uuid = aci.toString()

diff --git a/app/src/androidTest/java/org/thoughtcrime/securesms/testing/ResponseMocking.kt b/app/src/androidTest/java/org/thoughtcrime/securesms/testing/ResponseMocking.kt
@@ -17,6 +17,8 @@ class Get(path: String, responseFactory: ResponseFactory) : Verb("GET", path, re
 
 class Put(path: String, responseFactory: ResponseFactory) : Verb("PUT", path, responseFactory)
 
+class Post(path: String, responseFactory: ResponseFactory) : Verb("POST", path, responseFactory)
+
 fun MockResponse.success(response: Any? = null): MockResponse {
   return setResponseCode(200).apply {
     if (response != null) {

diff --git a/app/src/androidTest/java/org/thoughtcrime/securesms/testing/SignalActivityRule.kt b/app/src/androidTest/java/org/thoughtcrime/securesms/testing/SignalActivityRule.kt
@@ -83,15 +83,17 @@ class SignalActivityRule(private val othersCount: Int = 4) : ExternalResource()
         registrationId = registrationRepository.registrationId,
         profileKey = registrationRepository.getProfileKey("+15555550101"),
         fcmToken = null,
-        pniRegistrationId = registrationRepository.pniRegistrationId
+        pniRegistrationId = registrationRepository.pniRegistrationId,
+        recoveryPassword = "asdfasdfasdfasdf"
       ),
-      VerifyResponse(VerifyAccountResponse(UUID.randomUUID().toString(), UUID.randomUUID().toString(), false), null, null)
+      VerifyResponse(VerifyAccountResponse(UUID.randomUUID().toString(), UUID.randomUUID().toString(), false), null, null),
+      false
     ).blockingGet()
 
     ServiceResponseProcessor.DefaultProcessor(response).resultOrThrow
 
     SignalStore.kbsValues().optOut()
-    RegistrationUtil.maybeMarkRegistrationComplete(application)
+    RegistrationUtil.maybeMarkRegistrationComplete()
     SignalDatabase.recipients.setProfileName(Recipient.self().id, ProfileName.fromParts("Tester", "McTesterson"))
 
     return Recipient.self()

diff --git a/app/src/main/java/org/thoughtcrime/securesms/database/GroupTable.kt b/app/src/main/java/org/thoughtcrime/securesms/database/GroupTable.kt
@@ -213,6 +213,7 @@ class GroupTable(context: Context?, databaseHelper: SignalDatabase?) : DatabaseT
       .query(select, query.whereArgs)
       .use { cursor ->
         return if (cursor.moveToFirst()) {
+          var refreshCursor = false
           val groupRecord = getGroup(cursor)
           if (groupRecord.isPresent && RemappedRecords.getInstance().areAnyRemapped(groupRecord.get().members)) {
             val groupId = groupRecord.get().id
@@ -237,12 +238,23 @@ class GroupTable(context: Context?, databaseHelper: SignalDatabase?) : DatabaseT
 
             if (updateCount > 0) {
               Log.i(TAG, "Successfully updated $updateCount rows. GroupId: $groupId, Remaps: $remaps", true)
+              refreshCursor = true
             } else {
               Log.w(TAG, "Failed to update any rows. GroupId: $groupId, Remaps: $remaps", true)
             }
           }
 
-          getGroup(cursor)
+          if (refreshCursor) {
+            readableDatabase.query(select, query.whereArgs).use { refreshedCursor ->
+              if (refreshedCursor.moveToFirst()) {
+                getGroup(refreshedCursor)
+              } else {
+                Optional.empty()
+              }
+            }
+          } else {
+            getGroup(cursor)
+          }
         } else {
           Optional.empty()
         }