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 0d3d1098ad..c15dffcb81 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/database/RecipientTable.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/database/RecipientTable.kt @@ -1977,8 +1977,13 @@ open class RecipientTable(context: Context, databaseHelper: SignalDatabase) : Da /** * Applies multiple profile fields in a single UPDATE statement. Calls [rotateStorageId] and * [notifyRecipientChanged] at most once. Designed for bulk profile fetches. + * + * Of these fields, only the profile name and username live on the storage service contact record, so + * the storage id is only rotated when one of those actually changes. */ fun applyProfileUpdate(id: RecipientId, update: ProfileUpdate) { + val clearsUsername = update.clearUsername && hasUsername(id) + val contentValues = ContentValues().apply { update.profileName?.let { put(PROFILE_GIVEN_NAME, it.givenName.nullIfBlank()) @@ -2009,7 +2014,7 @@ open class RecipientTable(context: Context, databaseHelper: SignalDatabase) : Da .build() put(EXPIRING_PROFILE_KEY_CREDENTIAL, Base64.encodeWithPadding(columnData.encode())) } - if (update.clearUsername) { + if (clearsUsername) { putNull(USERNAME) } } @@ -2019,7 +2024,7 @@ open class RecipientTable(context: Context, databaseHelper: SignalDatabase) : Da } if (update(id, contentValues)) { - val needsStorageRotation = update.profileName != null || update.clearUsername + val needsStorageRotation = update.profileName != null || clearsUsername if (needsStorageRotation) { rotateStorageId(id) } @@ -2027,6 +2032,13 @@ open class RecipientTable(context: Context, databaseHelper: SignalDatabase) : Da } } + private fun hasUsername(id: RecipientId): Boolean { + return readableDatabase + .exists(TABLE_NAME) + .where("$ID = ? AND $USERNAME NOT NULL", id.serialize()) + .run() + } + fun setProfileName(id: RecipientId, profileName: ProfileName) { val contentValues = ContentValues(1).apply { put(PROFILE_GIVEN_NAME, profileName.givenName.nullIfBlank()) diff --git a/app/src/main/java/org/thoughtcrime/securesms/jobs/RetrieveProfileJob.kt b/app/src/main/java/org/thoughtcrime/securesms/jobs/RetrieveProfileJob.kt index 2a6ed3cf9b..12983e88f0 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/jobs/RetrieveProfileJob.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/jobs/RetrieveProfileJob.kt @@ -313,7 +313,7 @@ class RetrieveProfileJob private constructor(parameters: Parameters, private val val profileNameResult = resolveProfileName(recipient, recipientProfileKey, profile.name) val aboutResult = resolveProfileAbout(recipientProfileKey, profile.about, profile.aboutEmoji) val phoneNumberSharing = resolvePhoneNumberSharing(recipient, recipientProfileKey, profile.phoneNumberSharing) - val clearUsername = recipient.hasNonUsernameDisplayName(context) || profileNameResult?.changed == true + val clearUsername = (recipient.username.isPresent && recipient.hasNonUsernameDisplayName(context)) || profileNameResult?.changed == true val update = RecipientTable.ProfileUpdate( profileName = if (profileNameResult?.changed == true) profileNameResult.remoteProfileName else null, 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 5e097a947d..643fd88c99 100644 --- a/app/src/test/java/org/thoughtcrime/securesms/database/RecipientTableTest.kt +++ b/app/src/test/java/org/thoughtcrime/securesms/database/RecipientTableTest.kt @@ -13,6 +13,8 @@ import org.junit.Assert.assertEquals import org.junit.Assert.assertFalse import org.junit.Assert.assertNotEquals import org.junit.Assert.assertNotNull +import org.junit.Assert.assertNull +import org.junit.Assert.assertTrue import org.junit.Before import org.junit.Rule import org.junit.Test @@ -231,6 +233,70 @@ class RecipientTableTest { assertThat(uncategorized).isEmpty() } + @Test + fun givenAContactWithNoUsername_whenAProfileUpdateOnlyChangesFieldsAbsentFromTheContactRecord_thenIExpectNoStorageIdRotation() { + SignalDatabase.recipients.setStorageIdIfNotSet(target) + val originalStorageId: ByteArray? = SignalDatabase.recipients.getRecord(target).storageId + assertNotNull("Precondition: contact should have a storage id", originalStorageId) + assertNull("Precondition: contact should have no username", SignalDatabase.recipients.getUsername(target)) + + // WHEN a profile fetch reports a new sealed sender mode, which the contact record does not carry + SignalDatabase.recipients.applyProfileUpdate( + target, + RecipientTable.ProfileUpdate( + sealedSenderAccessMode = RecipientTable.SealedSenderAccessMode.ENABLED, + clearUsername = true + ) + ) + + assertEquals(RecipientTable.SealedSenderAccessMode.ENABLED, SignalDatabase.recipients.getRecord(target).sealedSenderAccessMode) + assertTrue( + "Storage id must not rotate for fields absent from the contact record, otherwise we republish identical content under a fresh id", + originalStorageId!!.contentEquals(SignalDatabase.recipients.getRecord(target).storageId) + ) + } + + @Test + fun givenAContactWithAUsername_whenAProfileUpdateClearsIt_thenIExpectAStorageIdRotation() { + SignalDatabase.recipients.setUsername(target, "target.01") + SignalDatabase.recipients.setStorageIdIfNotSet(target) + + val originalStorageId: ByteArray? = SignalDatabase.recipients.getRecord(target).storageId + assertNotNull("Precondition: contact should have a storage id", originalStorageId) + + SignalDatabase.recipients.applyProfileUpdate( + target, + RecipientTable.ProfileUpdate( + sealedSenderAccessMode = RecipientTable.SealedSenderAccessMode.ENABLED, + clearUsername = true + ) + ) + + assertNull(SignalDatabase.recipients.getUsername(target)) + assertFalse( + "Storage id should rotate when the username is actually cleared", + originalStorageId!!.contentEquals(SignalDatabase.recipients.getRecord(target).storageId) + ) + } + + @Test + fun givenASyncedContact_whenAProfileUpdateChangesTheProfileName_thenIExpectAStorageIdRotation() { + SignalDatabase.recipients.setStorageIdIfNotSet(target) + val originalStorageId: ByteArray? = SignalDatabase.recipients.getRecord(target).storageId + assertNotNull("Precondition: contact should have a storage id", originalStorageId) + + // WHEN a profile fetch reports a new profile name, which the contact record does carry + SignalDatabase.recipients.applyProfileUpdate( + target, + RecipientTable.ProfileUpdate(profileName = ProfileName.fromParts("Renamed", "Person")) + ) + + assertFalse( + "Storage id should rotate when the profile name changes", + originalStorageId!!.contentEquals(SignalDatabase.recipients.getRecord(target).storageId) + ) + } + 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"))