Migrate away from placeholder revision to support exporting pending approval groups.

This commit is contained in:
Cody Henthorne
2025-07-03 10:11:12 -04:00
committed by Alex Hart
parent dc8e93a9d3
commit 5ce5326721
16 changed files with 102 additions and 36 deletions

View File

@@ -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 }

View File

@@ -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
)
}

View File

@@ -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) {

View File

@@ -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<Pair<Long, ByteArray>>()
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)
}
}
}

View File

@@ -1021,15 +1021,16 @@ final class GroupManagerV2 {
/**
* Creates a local group from what we know before joining.
* <p>
* 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();

View File

@@ -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<DecryptedGroupChangeLog> 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);

View File

@@ -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.

View File

@@ -45,6 +45,7 @@ private fun DecryptedGroup.formatAsHtml(): String {
Pending: $pending
Requesting: $requesting
Banned: $banned
Placeholder: $isPlaceholderGroup
""".trimIndent().replace("\n", "<br>")
}

View File

@@ -121,9 +121,10 @@ class GroupStateTestData(private val masterKey: GroupMasterKey, private val grou
pendingMembers: List<DecryptedPendingMember> = emptyList(),
requestingMembers: List<DecryptedRequestingMember> = 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<DecryptedPendingMember> = emptyList(),
requestingMembers: List<DecryptedRequestingMember> = 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
)
}

View File

@@ -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))

View File

@@ -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) {

View File

@@ -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

View File

@@ -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);

View File

@@ -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);

View File

@@ -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);

View File

@@ -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<Integer> IGNORED_DECRYPTED_GROUP_TAGS = Collections.singleton(64);
/**
* Finds the largest declared field number in the supplied protobuf class.
*/
static int getMaxDeclaredFieldNumber(Class<? extends Message<?, ?>> protobufClass) {
return getMaxDeclaredFieldNumber(protobufClass, Collections.emptySet());
}
/**
* Finds the largest declared field number in the supplied protobuf class.
*/
static int getMaxDeclaredFieldNumber(Class<? extends Message<?, ?>> protobufClass, Set<Integer> 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);
}