diff --git a/app/src/androidTest/java/org/thoughtcrime/securesms/database/RecipientTableTest_applyStorageSyncContactUpdate.kt b/app/src/androidTest/java/org/thoughtcrime/securesms/database/RecipientTableTest_applyStorageSyncContactUpdate.kt index 9acfd64b5a..542e63864b 100644 --- a/app/src/androidTest/java/org/thoughtcrime/securesms/database/RecipientTableTest_applyStorageSyncContactUpdate.kt +++ b/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) + } } diff --git a/app/src/main/java/org/thoughtcrime/securesms/database/RecipientTable.kt b/app/src/main/java/org/thoughtcrime/securesms/database/RecipientTable.kt index 97dc98b642..e9b3b0aa44 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/database/RecipientTable.kt +++ b/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 -> diff --git a/app/src/spinner/java/org/thoughtcrime/securesms/StorageServicePlugin.kt b/app/src/spinner/java/org/thoughtcrime/securesms/StorageServicePlugin.kt index b967bbf0d5..081b0f6ce4 100644 --- a/app/src/spinner/java/org/thoughtcrime/securesms/StorageServicePlugin.kt +++ b/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 += "" diff --git a/app/src/test/java/org/thoughtcrime/securesms/database/RecipientTableTest.kt b/app/src/test/java/org/thoughtcrime/securesms/database/RecipientTableTest.kt index 8f41960eea..38589fe7a6 100644 --- a/app/src/test/java/org/thoughtcrime/securesms/database/RecipientTableTest.kt +++ b/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"))