diff --git a/app/src/main/java/org/thoughtcrime/securesms/backup/v2/exporters/GroupArchiveExporter.kt b/app/src/main/java/org/thoughtcrime/securesms/backup/v2/exporters/GroupArchiveExporter.kt index d7afab0c2e..08095eaa5b 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/backup/v2/exporters/GroupArchiveExporter.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/backup/v2/exporters/GroupArchiveExporter.kt @@ -29,7 +29,6 @@ import org.thoughtcrime.securesms.conversation.colors.AvatarColor import org.thoughtcrime.securesms.database.GroupTable import org.thoughtcrime.securesms.database.RecipientTable import org.thoughtcrime.securesms.database.RecipientTableCursorUtil -import org.thoughtcrime.securesms.groups.v2.processing.GroupsV2StateProcessor import org.whispersystems.signalservice.api.push.ServiceId import java.io.Closeable @@ -82,10 +81,6 @@ private fun GroupTable.ShowAsStoryState.toRemote(): Group.StorySendMode { } private fun DecryptedGroup.toRemote(isActive: Boolean, selfAci: ServiceId.ACI): Group.GroupSnapshot? { - if (this.revision == GroupsV2StateProcessor.RESTORE_PLACEHOLDER_REVISION || this.revision == GroupsV2StateProcessor.PLACEHOLDER_REVISION) { - return null - } - val selfAciBytes = selfAci.toByteString() val memberFilter = { m: DecryptedMember -> isActive || m.aciBytes != selfAciBytes } diff --git a/app/src/main/java/org/thoughtcrime/securesms/backup/v2/importer/GroupArchiveImporter.kt b/app/src/main/java/org/thoughtcrime/securesms/backup/v2/importer/GroupArchiveImporter.kt index 74b3fdf6bb..8ec3e95867 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/backup/v2/importer/GroupArchiveImporter.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/backup/v2/importer/GroupArchiveImporter.kt @@ -30,6 +30,7 @@ import org.thoughtcrime.securesms.database.model.databaseprotos.RecipientExtras import org.thoughtcrime.securesms.dependencies.AppDependencies import org.thoughtcrime.securesms.groups.GroupId import org.thoughtcrime.securesms.groups.v2.processing.GroupsV2StateProcessor +import org.thoughtcrime.securesms.keyvalue.SignalStore import org.thoughtcrime.securesms.recipients.RecipientId import org.thoughtcrime.securesms.storage.StorageSyncHelper import org.whispersystems.signalservice.api.groupsv2.GroupsV2Operations @@ -133,6 +134,10 @@ private fun Group.MemberBanned.toLocal(): DecryptedBannedMember { } private fun Group.GroupSnapshot.toLocal(operations: GroupsV2Operations.GroupOperations): DecryptedGroup { + val selfAciBytes = SignalStore.account.aci?.toByteString() + val requestingMembers = this.membersPendingAdminApproval.map { requesting -> requesting.toLocal() } + val isPlaceholder = requestingMembers.any { it.aciBytes == selfAciBytes } + return DecryptedGroup( title = this.title?.title ?: "", avatar = this.avatarUrl, @@ -141,10 +146,11 @@ private fun Group.GroupSnapshot.toLocal(operations: GroupsV2Operations.GroupOper revision = this.version, members = this.members.map { member -> member.toLocal() }, pendingMembers = this.membersPendingProfileKey.map { pending -> pending.toLocal(operations) }, - requestingMembers = this.membersPendingAdminApproval.map { requesting -> requesting.toLocal() }, + requestingMembers = requestingMembers, inviteLinkPassword = this.inviteLinkPassword, description = this.description?.descriptionText ?: "", isAnnouncementGroup = if (this.announcements_only) EnabledState.ENABLED else EnabledState.DISABLED, - bannedMembers = this.members_banned.map { it.toLocal() } + bannedMembers = this.members_banned.map { it.toLocal() }, + isPlaceholderGroup = isPlaceholder ) } diff --git a/app/src/main/java/org/thoughtcrime/securesms/database/helpers/SignalDatabaseMigrations.kt b/app/src/main/java/org/thoughtcrime/securesms/database/helpers/SignalDatabaseMigrations.kt index a35ab14276..2c8c6cb8b3 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/database/helpers/SignalDatabaseMigrations.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/database/helpers/SignalDatabaseMigrations.kt @@ -138,6 +138,7 @@ import org.thoughtcrime.securesms.database.helpers.migration.V280_RemoveAttachme import org.thoughtcrime.securesms.database.helpers.migration.V281_RemoveArchiveTransferFile import org.thoughtcrime.securesms.database.helpers.migration.V282_AddSnippetMessageIdColumnToThreadTable import org.thoughtcrime.securesms.database.helpers.migration.V283_ViewOnceRemoteDataCleanup +import org.thoughtcrime.securesms.database.helpers.migration.V284_SetPlaceholderGroupFlag import org.thoughtcrime.securesms.database.SQLiteDatabase as SignalSqliteDatabase /** @@ -281,10 +282,11 @@ object SignalDatabaseMigrations { 280 to V280_RemoveAttachmentIv, 281 to V281_RemoveArchiveTransferFile, 282 to V282_AddSnippetMessageIdColumnToThreadTable, - 283 to V283_ViewOnceRemoteDataCleanup + 283 to V283_ViewOnceRemoteDataCleanup, + 284 to V284_SetPlaceholderGroupFlag ) - const val DATABASE_VERSION = 283 + const val DATABASE_VERSION = 284 @JvmStatic fun migrate(context: Application, db: SignalSqliteDatabase, oldVersion: Int, newVersion: Int) { diff --git a/app/src/main/java/org/thoughtcrime/securesms/database/helpers/migration/V284_SetPlaceholderGroupFlag.kt b/app/src/main/java/org/thoughtcrime/securesms/database/helpers/migration/V284_SetPlaceholderGroupFlag.kt new file mode 100644 index 0000000000..bf2f40f5b7 --- /dev/null +++ b/app/src/main/java/org/thoughtcrime/securesms/database/helpers/migration/V284_SetPlaceholderGroupFlag.kt @@ -0,0 +1,45 @@ +/* + * Copyright 2025 Signal Messenger, LLC + * SPDX-License-Identifier: AGPL-3.0-only + */ + +package org.thoughtcrime.securesms.database.helpers.migration + +import android.app.Application +import androidx.core.content.contentValuesOf +import org.signal.core.util.logging.Log +import org.signal.core.util.requireLong +import org.signal.core.util.requireNonNullBlob +import org.signal.storageservice.protos.groups.local.DecryptedGroup +import org.thoughtcrime.securesms.database.SQLiteDatabase + +/** + * For all of time, we used the revision of -1 to indicate a placeholder group (i.e., pending invite approval). With + * backups we want to be able to export those groups which require a non-negative revision. Migrates groups with a + * revision of -1 to a group dummy revision of 0 but with the placeholder group state flag set. + */ +object V284_SetPlaceholderGroupFlag : SignalDatabaseMigration { + private val TAG = Log.tag(V284_SetPlaceholderGroupFlag::class) + + override fun migrate(context: Application, db: SQLiteDatabase, oldVersion: Int, newVersion: Int) { + val updates = mutableListOf>() + + db.query("groups", arrayOf("_id", "decrypted_group"), "revision = -1 AND decrypted_group IS NOT NULL", null, null, null, null).use { cursor -> + while (cursor.moveToNext()) { + val decryptedGroup = try { + DecryptedGroup.ADAPTER.decode(cursor.requireNonNullBlob("decrypted_group")) + } catch (e: Exception) { + Log.w(TAG, "Unable to parse group state", e) + continue + } + + updates += cursor.requireLong("_id") to decryptedGroup.newBuilder().revision(0).isPlaceholderGroup(true).build().encode() + } + } + + updates.forEach { (id, groupState) -> + val values = contentValuesOf("decrypted_group" to groupState, "revision" to 0) + db.update("groups", values, "_id = $id", null) + } + } +} diff --git a/app/src/main/java/org/thoughtcrime/securesms/groups/GroupManagerV2.java b/app/src/main/java/org/thoughtcrime/securesms/groups/GroupManagerV2.java index f60be887b5..537e171fc1 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/groups/GroupManagerV2.java +++ b/app/src/main/java/org/thoughtcrime/securesms/groups/GroupManagerV2.java @@ -1021,15 +1021,16 @@ final class GroupManagerV2 { /** * Creates a local group from what we know before joining. *

- * Creates as a {@link GroupsV2StateProcessor#PLACEHOLDER_REVISION} so that we know not do do a - * full diff against this group once we learn more about this group as that would create a large - * update message. + * Creates as a placeholder group so that we know not do do a full diff against this group once we learn more about this + * group as that would create a large update message. */ private DecryptedGroup createPlaceholderGroup(@NonNull DecryptedGroupJoinInfo joinInfo, boolean requestToJoin) { DecryptedGroup.Builder group = new DecryptedGroup.Builder() .title(joinInfo.title) .avatar(joinInfo.avatar) - .revision(GroupsV2StateProcessor.PLACEHOLDER_REVISION); + .description(joinInfo.description) + .revision(joinInfo.revision) + .isPlaceholderGroup(true); Recipient self = Recipient.self(); ByteString selfAciBytes = selfAci.toByteString(); diff --git a/app/src/main/java/org/thoughtcrime/securesms/groups/v2/processing/GroupStatePatcher.java b/app/src/main/java/org/thoughtcrime/securesms/groups/v2/processing/GroupStatePatcher.java index c586e96604..4c143eaf98 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/groups/v2/processing/GroupStatePatcher.java +++ b/app/src/main/java/org/thoughtcrime/securesms/groups/v2/processing/GroupStatePatcher.java @@ -23,7 +23,6 @@ final class GroupStatePatcher { private static final String TAG = Log.tag(GroupStatePatcher.class); static final int LATEST = Integer.MAX_VALUE; - static final int PLACEHOLDER_REVISION = -1; static final int RESTORE_PLACEHOLDER_REVISION = -2; private static final Comparator BY_REVISION = (o1, o2) -> Integer.compare(o1.getRevision(), o2.getRevision()); @@ -71,7 +70,7 @@ final class GroupStatePatcher { final int from = Math.max(0, inputState.getEarliestRevisionNumber()); final int to = Math.min(inputState.getLatestRevisionNumber(), maximumRevisionToApply); - if (current != null && current.revision == PLACEHOLDER_REVISION) { + if (current != null && current.isPlaceholderGroup) { Log.i(TAG, "Ignoring place holder group state"); } else { stateChain.push(current, null); @@ -84,10 +83,11 @@ final class GroupStatePatcher { continue; } - if (stateChain.getLatestState() == null && entry.getGroup() != null && current != null && current.revision == PLACEHOLDER_REVISION) { + if (stateChain.getLatestState() == null && entry.getGroup() != null && current != null && current.isPlaceholderGroup) { DecryptedGroup previousState = entry.getGroup().newBuilder() .title(current.title) .avatar(current.avatar) + .description(current.description) .build(); stateChain.push(previousState, null); diff --git a/app/src/main/java/org/thoughtcrime/securesms/groups/v2/processing/GroupsV2StateProcessor.kt b/app/src/main/java/org/thoughtcrime/securesms/groups/v2/processing/GroupsV2StateProcessor.kt index 6b83990d9c..1fc513435b 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/groups/v2/processing/GroupsV2StateProcessor.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/groups/v2/processing/GroupsV2StateProcessor.kt @@ -70,12 +70,6 @@ class GroupsV2StateProcessor private constructor( const val LATEST = GroupStatePatcher.LATEST - /** - * Used to mark a group state as a placeholder when there is partial knowledge (title and avater) - * gathered from a group join link. - */ - const val PLACEHOLDER_REVISION = GroupStatePatcher.PLACEHOLDER_REVISION - /** * Used to mark a group state as a placeholder when you have no knowledge at all of the group * e.g. from a group master key from a storage service restore. diff --git a/app/src/spinner/java/org/thoughtcrime/securesms/database/GV2Transformer.kt b/app/src/spinner/java/org/thoughtcrime/securesms/database/GV2Transformer.kt index bdada89108..1d684227a6 100644 --- a/app/src/spinner/java/org/thoughtcrime/securesms/database/GV2Transformer.kt +++ b/app/src/spinner/java/org/thoughtcrime/securesms/database/GV2Transformer.kt @@ -45,6 +45,7 @@ private fun DecryptedGroup.formatAsHtml(): String { Pending: $pending Requesting: $requesting Banned: $banned + Placeholder: $isPlaceholderGroup """.trimIndent().replace("\n", "
") } diff --git a/app/src/test/java/org/thoughtcrime/securesms/database/GroupTestUtil.kt b/app/src/test/java/org/thoughtcrime/securesms/database/GroupTestUtil.kt index e69d878c80..b63995837c 100644 --- a/app/src/test/java/org/thoughtcrime/securesms/database/GroupTestUtil.kt +++ b/app/src/test/java/org/thoughtcrime/securesms/database/GroupTestUtil.kt @@ -121,9 +121,10 @@ class GroupStateTestData(private val masterKey: GroupMasterKey, private val grou pendingMembers: List = emptyList(), requestingMembers: List = emptyList(), inviteLinkPassword: ByteArray = ByteArray(0), - disappearingMessageTimer: DecryptedTimer = DecryptedTimer() + disappearingMessageTimer: DecryptedTimer = DecryptedTimer(), + isPlaceholderGroup: Boolean = false ) { - localState = decryptedGroup(revision, title, avatar, description, accessControl, members, pendingMembers, requestingMembers, inviteLinkPassword, disappearingMessageTimer) + localState = decryptedGroup(revision, title, avatar, description, accessControl, members, pendingMembers, requestingMembers, inviteLinkPassword, disappearingMessageTimer, isPlaceholderGroup) groupRecord = groupRecord(masterKey, localState!!, active = active) } @@ -209,7 +210,8 @@ fun decryptedGroup( pendingMembers: List = emptyList(), requestingMembers: List = emptyList(), inviteLinkPassword: ByteArray = ByteArray(0), - disappearingMessageTimer: DecryptedTimer = DecryptedTimer() + disappearingMessageTimer: DecryptedTimer = DecryptedTimer(), + isPlaceholderGroup: Boolean = false ): DecryptedGroup { return DecryptedGroup( accessControl = accessControl, @@ -222,6 +224,7 @@ fun decryptedGroup( revision = revision, members = members, pendingMembers = pendingMembers, - requestingMembers = requestingMembers + requestingMembers = requestingMembers, + isPlaceholderGroup = isPlaceholderGroup ) } diff --git a/app/src/test/java/org/thoughtcrime/securesms/groups/v2/processing/GroupStatePatcherTest.kt b/app/src/test/java/org/thoughtcrime/securesms/groups/v2/processing/GroupStatePatcherTest.kt index c0b46662cc..600324668d 100644 --- a/app/src/test/java/org/thoughtcrime/securesms/groups/v2/processing/GroupStatePatcherTest.kt +++ b/app/src/test/java/org/thoughtcrime/securesms/groups/v2/processing/GroupStatePatcherTest.kt @@ -543,7 +543,8 @@ class GroupStatePatcherTest { @Test fun no_repair_change_is_posted_if_the_local_state_is_a_placeholder() { val currentState = DecryptedGroup.Builder() - .revision(GroupStatePatcher.PLACEHOLDER_REVISION) + .revision(0) + .isPlaceholderGroup(true) .title("Incorrect group title, Revision " + 6) .build() val log6 = serverLogEntry(6) @@ -574,7 +575,8 @@ class GroupStatePatcherTest { .aciBytes(ServiceId.ACI.from(UUID.randomUUID()).toByteString()) .build() val currentState = DecryptedGroup.Builder() - .revision(GroupStatePatcher.PLACEHOLDER_REVISION) + .revision(0) + .isPlaceholderGroup(true) .title("Group Revision " + 8) .members(listOf(newMember)) .build() @@ -684,7 +686,8 @@ class GroupStatePatcherTest { .aciBytes(ServiceId.ACI.from(UUID.randomUUID()).toByteString()) .build() val currentState = DecryptedGroup.Builder() - .revision(GroupStatePatcher.PLACEHOLDER_REVISION) + .revision(0) + .isPlaceholderGroup(true) .title("Incorrect group title") .avatar("Incorrect group avatar") .members(listOf(newMember)) diff --git a/app/src/test/java/org/thoughtcrime/securesms/groups/v2/processing/GroupsV2StateProcessorTest.kt b/app/src/test/java/org/thoughtcrime/securesms/groups/v2/processing/GroupsV2StateProcessorTest.kt index 1f288b3c89..00dd2f42e1 100644 --- a/app/src/test/java/org/thoughtcrime/securesms/groups/v2/processing/GroupsV2StateProcessorTest.kt +++ b/app/src/test/java/org/thoughtcrime/securesms/groups/v2/processing/GroupsV2StateProcessorTest.kt @@ -662,9 +662,10 @@ class GroupsV2StateProcessorTest { fun `when request to join group is approved, with no group changes after approved, then update from server to revision we were added`() { given { localState( - revision = GroupsV2StateProcessor.PLACEHOLDER_REVISION, + revision = 0, title = "Beam me up", - requestingMembers = listOf(requestingMember(selfAci)) + requestingMembers = listOf(requestingMember(selfAci)), + isPlaceholderGroup = true ) changeSet { @@ -703,9 +704,10 @@ class GroupsV2StateProcessorTest { fun `when request to join group is approved, with group changes occurring after approved, then update from server to revision we were added, and then schedule pulling additional changes later`() { given { localState( - revision = GroupsV2StateProcessor.PLACEHOLDER_REVISION, + revision = 0, title = "Beam me up", - requestingMembers = listOf(requestingMember(selfAci)) + requestingMembers = listOf(requestingMember(selfAci)), + isPlaceholderGroup = true ) changeSet { changeLog(3) { diff --git a/libsignal-service/src/main/protowire/DecryptedGroups.proto b/libsignal-service/src/main/protowire/DecryptedGroups.proto index 86b0595438..cf2329a996 100644 --- a/libsignal-service/src/main/protowire/DecryptedGroups.proto +++ b/libsignal-service/src/main/protowire/DecryptedGroups.proto @@ -69,6 +69,7 @@ message DecryptedGroup { string description = 11; EnabledState isAnnouncementGroup = 12; repeated DecryptedBannedMember bannedMembers = 13; + bool isPlaceholderGroup = 64; } // Decrypted version of message GroupChange.Actions diff --git a/libsignal-service/src/test/java/org/whispersystems/signalservice/api/groupsv2/GroupChangeReconstructTest.java b/libsignal-service/src/test/java/org/whispersystems/signalservice/api/groupsv2/GroupChangeReconstructTest.java index a625bb299b..db65fefa90 100644 --- a/libsignal-service/src/test/java/org/whispersystems/signalservice/api/groupsv2/GroupChangeReconstructTest.java +++ b/libsignal-service/src/test/java/org/whispersystems/signalservice/api/groupsv2/GroupChangeReconstructTest.java @@ -41,7 +41,7 @@ public final class GroupChangeReconstructTest { */ @Test public void ensure_GroupChangeReconstruct_knows_about_all_fields_of_DecryptedGroup() { - int maxFieldFound = getMaxDeclaredFieldNumber(DecryptedGroup.class); + int maxFieldFound = getMaxDeclaredFieldNumber(DecryptedGroup.class, ProtobufTestUtils.IGNORED_DECRYPTED_GROUP_TAGS); assertEquals("GroupChangeReconstruct and its tests need updating to account for new fields on " + DecryptedGroup.class.getName(), 13, maxFieldFound); diff --git a/libsignal-service/src/test/java/org/whispersystems/signalservice/api/groupsv2/GroupChangeUtil_resolveConflict_Test.java b/libsignal-service/src/test/java/org/whispersystems/signalservice/api/groupsv2/GroupChangeUtil_resolveConflict_Test.java index e2c4035261..aa5d0b51c9 100644 --- a/libsignal-service/src/test/java/org/whispersystems/signalservice/api/groupsv2/GroupChangeUtil_resolveConflict_Test.java +++ b/libsignal-service/src/test/java/org/whispersystems/signalservice/api/groupsv2/GroupChangeUtil_resolveConflict_Test.java @@ -75,7 +75,7 @@ public final class GroupChangeUtil_resolveConflict_Test { */ @Test public void ensure_resolveConflict_knows_about_all_fields_of_DecryptedGroup() { - int maxFieldFound = getMaxDeclaredFieldNumber(DecryptedGroup.class); + int maxFieldFound = getMaxDeclaredFieldNumber(DecryptedGroup.class, ProtobufTestUtils.IGNORED_DECRYPTED_GROUP_TAGS); assertEquals("GroupChangeUtil#resolveConflict and its tests need updating to account for new fields on " + DecryptedGroup.class.getName(), 13, maxFieldFound); diff --git a/libsignal-service/src/test/java/org/whispersystems/signalservice/api/groupsv2/GroupChangeUtil_resolveConflict_decryptedOnly_Test.java b/libsignal-service/src/test/java/org/whispersystems/signalservice/api/groupsv2/GroupChangeUtil_resolveConflict_decryptedOnly_Test.java index 9f4fbdb680..cb93004931 100644 --- a/libsignal-service/src/test/java/org/whispersystems/signalservice/api/groupsv2/GroupChangeUtil_resolveConflict_decryptedOnly_Test.java +++ b/libsignal-service/src/test/java/org/whispersystems/signalservice/api/groupsv2/GroupChangeUtil_resolveConflict_decryptedOnly_Test.java @@ -54,7 +54,7 @@ public final class GroupChangeUtil_resolveConflict_decryptedOnly_Test { */ @Test public void ensure_resolveConflict_knows_about_all_fields_of_DecryptedGroup() { - int maxFieldFound = getMaxDeclaredFieldNumber(DecryptedGroup.class); + int maxFieldFound = getMaxDeclaredFieldNumber(DecryptedGroup.class, ProtobufTestUtils.IGNORED_DECRYPTED_GROUP_TAGS); assertEquals("GroupChangeUtil#resolveConflict and its tests need updating to account for new fields on " + DecryptedGroup.class.getName(), 13, maxFieldFound); diff --git a/libsignal-service/src/test/java/org/whispersystems/signalservice/api/groupsv2/ProtobufTestUtils.java b/libsignal-service/src/test/java/org/whispersystems/signalservice/api/groupsv2/ProtobufTestUtils.java index e3d41a4eae..46a03c38ab 100644 --- a/libsignal-service/src/test/java/org/whispersystems/signalservice/api/groupsv2/ProtobufTestUtils.java +++ b/libsignal-service/src/test/java/org/whispersystems/signalservice/api/groupsv2/ProtobufTestUtils.java @@ -3,18 +3,31 @@ package org.whispersystems.signalservice.api.groupsv2; import com.squareup.wire.Message; import com.squareup.wire.WireField; +import java.util.Collections; +import java.util.Set; import java.util.stream.Stream; final class ProtobufTestUtils { + /** Tags that should be ignored and not count as part of 'needs support' in the various group decryption tests. */ + static final Set IGNORED_DECRYPTED_GROUP_TAGS = Collections.singleton(64); + /** * Finds the largest declared field number in the supplied protobuf class. */ static int getMaxDeclaredFieldNumber(Class> protobufClass) { + return getMaxDeclaredFieldNumber(protobufClass, Collections.emptySet()); + } + + /** + * Finds the largest declared field number in the supplied protobuf class. + */ + static int getMaxDeclaredFieldNumber(Class> protobufClass, Set excludeTags) { return Stream.of(protobufClass.getFields()) .map(f -> f.getAnnotationsByType(WireField.class)) .filter(a -> a.length == 1) .map(a -> a[0].tag()) + .filter(t -> !excludeTags.contains(t)) .max(Integer::compareTo) .orElse(0); }