Ensure unregistered contacts update storage service if previously registered.

app/src/androidTest/java/org/thoughtcrime/securesms/database/RecipientTableTest_applyStorageSyncContactUpdate.kt

@@ -8,10 +8,17 @@ package org.thoughtcrime.securesms.database
 import androidx.test.ext.junit.runners.AndroidJUnit4
 import assertk.assertThat
 import assertk.assertions.isEqualTo
+import assertk.assertions.isFalse
+import assertk.assertions.isNotEqualTo
+import assertk.assertions.isNotNull
+import assertk.assertions.isNull
 import assertk.assertions.isTrue
 import org.junit.Rule
 import org.junit.Test
 import org.junit.runner.RunWith
+import org.signal.core.models.ServiceId.ACI
+import org.signal.core.models.ServiceId.PNI
+import org.signal.core.util.nullIfBlank
 import org.thoughtcrime.securesms.dependencies.AppDependencies
 import org.thoughtcrime.securesms.recipients.Recipient
 import org.thoughtcrime.securesms.storage.StorageRecordUpdate
@@ -19,8 +26,11 @@ import org.thoughtcrime.securesms.storage.StorageSyncModels
 import org.thoughtcrime.securesms.testing.SignalActivityRule
 import org.thoughtcrime.securesms.util.MessageTableTestUtils
 import org.whispersystems.signalservice.api.storage.SignalContactRecord
+import org.whispersystems.signalservice.api.storage.signalAci
+import org.whispersystems.signalservice.api.storage.signalPni
 import org.whispersystems.signalservice.api.storage.toSignalContactRecord
 import org.whispersystems.signalservice.internal.storage.protos.ContactRecord
+import java.util.UUID
 
 @Suppress("ClassName")
 @RunWith(AndroidJUnit4::class)
@@ -60,4 +70,46 @@ class RecipientTableTest_applyStorageSyncContactUpdate {
     val messages = MessageTableTestUtils.getMessages(SignalDatabase.threads.getThreadIdFor(other.id)!!)
     assertThat(messages.first().isIdentityDefault).isTrue()
   }
+
+  @Test
+  fun givenAnAlreadySyncedContact_whenMarkedUnregistered_thenItSplitsAndPublishesTheSplit() {
+    // GIVEN a registered contact with aci+pni+e164 that is already in storage service (has a storageId)
+    val aci = ACI.from(UUID.randomUUID())
+    val pni = PNI.from(UUID.randomUUID())
+    val e164 = "+13334445555"
+
+    val id = SignalDatabase.recipients.getAndPossiblyMerge(aci, pni, e164)
+    SignalDatabase.recipients.markRegistered(id, aci)
+
+    val originalStorageId: ByteArray? = SignalDatabase.recipients.getRecord(id).storageId
+    assertThat(originalStorageId).isNotNull()
+
+    // Sanity: the record it currently publishes is whole + registered.
+    val before = StorageSyncModels.localToRemoteRecord(SignalDatabase.recipients.getRecordForSync(id)!!).proto.contact!!
+    assertThat(before.signalAci).isEqualTo(aci)
+    assertThat(before.signalPni).isEqualTo(pni)
+    assertThat(before.unregisteredAtTimestamp).isEqualTo(0L)
+
+    // WHEN it is marked unregistered (which strips its pni/e164 and splits it)
+    SignalDatabase.recipients.markUnregistered(id)
+
+    // THEN its storageId rotates
+    val updatedStorageId: ByteArray? = SignalDatabase.recipients.getRecord(id).storageId
+    assertThat(updatedStorageId).isNotNull()
+    assertThat(originalStorageId!!.contentEquals(updatedStorageId!!)).isFalse()
+
+    // THEN the published record is now ACI-only + unregistered
+    val after = StorageSyncModels.localToRemoteRecord(SignalDatabase.recipients.getRecordForSync(id)!!).proto.contact!!
+    assertThat(after.signalAci).isEqualTo(aci)
+    assertThat(after.signalPni).isNull()
+    assertThat(after.e164.nullIfBlank()).isNull()
+    assertThat(after.unregisteredAtTimestamp > 0L).isTrue()
+
+    // THEN the number now lives on a separate PNI-only recipient, so no whole aci+pni+e164 record remains.
+    val byPni = SignalDatabase.recipients.getByPni(pni).get()
+    assertThat(byPni).isNotEqualTo(id)
+    val pniRecord = StorageSyncModels.localToRemoteRecord(SignalDatabase.recipients.getRecordForSync(byPni)!!).proto.contact!!
+    assertThat(pniRecord.signalAci).isNull()
+    assertThat(pniRecord.signalPni).isEqualTo(pni)
+  }
 }

app/src/main/java/org/thoughtcrime/securesms/database/RecipientTable.kt

@@ -4087,7 +4087,7 @@ open class RecipientTable(context: Context, databaseHelper: SignalDatabase) : Da
   /**
    * Does not trigger any recipient refreshes -- it is assumed the caller handles this.
    * Will *not* give storageIds to those that shouldn't get them (e.g. MMS groups, unregistered
-   * users).
+   * users) but will rotate ids if one already exists regardless of state.
    */
   fun rotateStorageId(recipientId: RecipientId, logFailure: Boolean = false) {
     val selfId = Recipient.self().id
@@ -4101,7 +4101,7 @@ open class RecipientTable(context: Context, databaseHelper: SignalDatabase) : Da
       put(STORAGE_SERVICE_ID, Base64.encodeWithPadding(StorageSyncHelper.generateKey()))
     }
 
-    val query = "$ID = ? AND ($TYPE IN (?, ?, ?, ?) OR $REGISTERED = ? OR $ID = ?)"
+    val query = "$ID = ? AND ($TYPE IN (?, ?, ?, ?) OR $REGISTERED = ? OR $ID = ? OR $STORAGE_SERVICE_ID IS NOT NULL)"
     val args = SqlUtil.buildArgs(recipientId, RecipientType.GV1.id, RecipientType.GV2.id, RecipientType.DISTRIBUTION_LIST.id, RecipientType.CALL_LINK.id, RegisteredState.REGISTERED.id, selfId.toLong())
 
     writableDatabase.update(TABLE_NAME, values, query, args).also { updateCount ->

app/src/spinner/java/org/thoughtcrime/securesms/StorageServicePlugin.kt

@@ -1,11 +1,14 @@
 package org.thoughtcrime.securesms
 
 import org.signal.core.util.Base64
+import org.signal.core.util.nullIfBlank
 import org.signal.network.service.StorageServiceService
 import org.signal.spinner.Plugin
 import org.signal.spinner.PluginResult
 import org.thoughtcrime.securesms.keyvalue.SignalStore
 import org.thoughtcrime.securesms.net.SignalNetwork
+import org.whispersystems.signalservice.api.storage.signalAci
+import org.whispersystems.signalservice.api.storage.signalPni
 
 class StorageServicePlugin : Plugin {
   override val name: String = "Storage"
@@ -36,25 +39,30 @@ class StorageServicePlugin : Plugin {
         row += record.proto.account.toString().prettyPrintProto()
       } else if (record.proto.contact != null) {
         row += "Contact"
-        row += record.proto.toString()
+        val contact = record.proto.contact!!
+        val readable = contact.copy(
+          aci = contact.aci.nullIfBlank() ?: contact.signalAci?.toString()?.let { "**$it**" } ?: contact.aci,
+          pni = contact.pni.nullIfBlank() ?: contact.signalPni?.toString()?.let { "**$it**" } ?: contact.pni
+        )
+        row += readable.toString().prettyPrintProto()
       } else if (record.proto.groupV1 != null) {
         row += "GV1"
-        row += record.proto.toString()
+        row += record.proto.groupV1.toString().prettyPrintProto()
       } else if (record.proto.groupV2 != null) {
         row += "GV2"
-        row += record.proto.toString()
+        row += record.proto.groupV2.toString().prettyPrintProto()
       } else if (record.proto.storyDistributionList != null) {
         row += "Distribution List"
-        row += record.proto.toString()
+        row += record.proto.storyDistributionList.toString().prettyPrintProto()
       } else if (record.proto.callLink != null) {
         row += "Call Link"
-        row += record.proto.callLink.toString()
+        row += record.proto.callLink.toString().prettyPrintProto()
       } else if (record.proto.chatFolder != null) {
         row += "Chat Folder"
-        row += record.proto.chatFolder.toString()
+        row += record.proto.chatFolder.toString().prettyPrintProto()
       } else if (record.proto.notificationProfile != null) {
         row += "Notification Profile"
-        row += record.proto.notificationProfile.toString()
+        row += record.proto.notificationProfile.toString().prettyPrintProto()
       } else {
         row += "Unknown"
         row += ""

app/src/test/java/org/thoughtcrime/securesms/database/RecipientTableTest.kt

@@ -9,6 +9,7 @@ import android.app.Application
 import org.junit.Assert.assertEquals
 import org.junit.Assert.assertFalse
 import org.junit.Assert.assertNotEquals
+import org.junit.Assert.assertNotNull
 import org.junit.Before
 import org.junit.Rule
 import org.junit.Test
@@ -182,6 +183,27 @@ class RecipientTableTest {
     assertNotEquals(byAci, byE164)
   }
 
+  @Test
+  fun givenAnAlreadySyncedRecipient_whenIMarkItUnregistered_thenItsStorageIdRotatesSoTheChangePublishes() {
+    // GIVEN a registered contact that already has a storage service id
+    val mainId = SignalDatabase.recipients.getAndPossiblyMerge(ACI_A, PNI_A, E164_A)
+    SignalDatabase.recipients.markRegistered(mainId, ACI_A)
+
+    val originalStorageId: ByteArray? = SignalDatabase.recipients.getRecord(mainId).storageId
+    assertNotNull("Precondition: an already-synced contact should have a storage id", originalStorageId)
+
+    // WHEN it is marked unregistered
+    SignalDatabase.recipients.markUnregistered(mainId)
+
+    // THEN its storage id must rotate
+    val updatedStorageId: ByteArray? = SignalDatabase.recipients.getRecord(mainId).storageId
+    assertNotNull("Storage id should still be set after unregistering an already-synced contact", updatedStorageId)
+    assertFalse(
+      "Storage id should rotate when an already-synced contact is unregistered, so the change publishes to storage service",
+      originalStorageId!!.contentEquals(updatedStorageId!!)
+    )
+  }
+
   companion object {
     val ACI_A = ACI.from(UUID.fromString("aaaa0000-5a76-47fa-a98a-7e72c948a82e"))
     val PNI_A = PNI.from(UUID.fromString("aaaa1111-c960-4f6c-8385-671ad2ffb999"))