From ff726ec4d2654b1b7939ac049d144c50cddd1230 Mon Sep 17 00:00:00 2001 From: jeffrey-signal Date: Tue, 3 Feb 2026 12:20:33 -0500 Subject: [PATCH] Don't send group update messages when member labels are changed. --- .../securesms/groups/GroupManagerV2.java | 13 +- .../v2/processing/GroupStatePatcher.java | 3 +- .../v2/processing/GroupsV2StateProcessor.kt | 21 ++- .../sms/GroupV2UpdateMessageUtil.java | 4 +- .../api/groupsv2/DecryptedGroupExtensions.kt | 80 +++++++++++ .../api/groupsv2/DecryptedGroupUtil.java | 72 +--------- .../DecryptedGroupUtil_empty_Test.java | 128 ++++++++++-------- ...il_resolveConflict_decryptedOnly_Test.java | 16 +-- 8 files changed, 189 insertions(+), 148 deletions(-) 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 e6a27849d9..7f8c1840a5 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/groups/GroupManagerV2.java +++ b/app/src/main/java/org/thoughtcrime/securesms/groups/GroupManagerV2.java @@ -7,6 +7,10 @@ import androidx.annotation.Nullable; import androidx.annotation.VisibleForTesting; import androidx.annotation.WorkerThread; +import org.signal.core.models.ServiceId; +import org.signal.core.models.ServiceId.ACI; +import org.signal.core.models.ServiceId.PNI; +import org.signal.core.util.UuidUtil; import org.signal.core.util.logging.Log; import org.signal.libsignal.zkgroup.InvalidInputException; import org.signal.libsignal.zkgroup.VerificationFailedException; @@ -17,9 +21,9 @@ import org.signal.libsignal.zkgroup.groups.UuidCiphertext; import org.signal.libsignal.zkgroup.profiles.ExpiringProfileKeyCredential; import org.signal.libsignal.zkgroup.profiles.ProfileKey; import org.signal.storageservice.storage.protos.groups.AccessControl; +import org.signal.storageservice.storage.protos.groups.ExternalGroupCredential; import org.signal.storageservice.storage.protos.groups.GroupChange; import org.signal.storageservice.storage.protos.groups.GroupChangeResponse; -import org.signal.storageservice.storage.protos.groups.ExternalGroupCredential; import org.signal.storageservice.storage.protos.groups.Member; import org.signal.storageservice.storage.protos.groups.local.DecryptedGroup; import org.signal.storageservice.storage.protos.groups.local.DecryptedGroupChange; @@ -51,6 +55,7 @@ import org.thoughtcrime.securesms.recipients.RecipientId; import org.thoughtcrime.securesms.sms.MessageSender; import org.thoughtcrime.securesms.util.ProfileUtil; import org.whispersystems.signalservice.api.groupsv2.DecryptChangeVerificationMode; +import org.whispersystems.signalservice.api.groupsv2.DecryptedGroupExtensions; import org.whispersystems.signalservice.api.groupsv2.DecryptedGroupResponse; import org.whispersystems.signalservice.api.groupsv2.DecryptedGroupUtil; import org.whispersystems.signalservice.api.groupsv2.GroupCandidate; @@ -62,13 +67,9 @@ import org.whispersystems.signalservice.api.groupsv2.GroupsV2Operations; import org.whispersystems.signalservice.api.groupsv2.InvalidGroupStateException; import org.whispersystems.signalservice.api.groupsv2.NotAbleToApplyGroupV2ChangeException; import org.whispersystems.signalservice.api.groupsv2.ReceivedGroupSendEndorsements; -import org.signal.core.models.ServiceId; -import org.signal.core.models.ServiceId.ACI; -import org.signal.core.models.ServiceId.PNI; import org.whispersystems.signalservice.api.push.ServiceIds; import org.whispersystems.signalservice.api.push.exceptions.AuthorizationFailedException; import org.whispersystems.signalservice.api.push.exceptions.ConflictException; -import org.signal.core.util.UuidUtil; import org.whispersystems.signalservice.internal.push.exceptions.GroupExistsException; import org.whispersystems.signalservice.internal.push.exceptions.GroupPatchNotAcceptedException; import org.whispersystems.signalservice.internal.push.exceptions.NotInGroupException; @@ -1296,7 +1297,7 @@ final class GroupManagerV2 { DecryptedGroupChange plainGroupChange = groupMutation.getGroupChange(); - if (plainGroupChange != null && DecryptedGroupUtil.changeIsSilent(plainGroupChange)) { + if (plainGroupChange != null && DecryptedGroupExtensions.isSilent(plainGroupChange)) { if (sendToMembers) { AppDependencies.getJobManager().add(PushGroupSilentUpdateSendJob.create(context, groupId, groupMutation.getNewGroupState(), outgoingMessage)); } 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 a131bcd884..1f5ce55831 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 @@ -7,6 +7,7 @@ import org.signal.core.util.logging.Log; import org.signal.storageservice.storage.protos.groups.local.DecryptedGroup; import org.signal.storageservice.storage.protos.groups.local.DecryptedGroupChange; import org.whispersystems.signalservice.api.groupsv2.DecryptedGroupChangeLog; +import org.whispersystems.signalservice.api.groupsv2.DecryptedGroupExtensions; import org.whispersystems.signalservice.api.groupsv2.DecryptedGroupUtil; import org.whispersystems.signalservice.api.groupsv2.GroupChangeReconstruct; import org.whispersystems.signalservice.api.groupsv2.GroupChangeUtil; @@ -144,7 +145,7 @@ final class GroupStatePatcher { } }, (groupB, groupA) -> GroupChangeReconstruct.reconstructGroupChange(groupA, groupB), - (groupA, groupB) -> groupA.revision == groupB.revision && DecryptedGroupUtil.changeIsEmpty(GroupChangeReconstruct.reconstructGroupChange(groupA, groupB)) + (groupA, groupB) -> groupA.revision == groupB.revision && DecryptedGroupExtensions.getChangedFields(GroupChangeReconstruct.reconstructGroupChange(groupA, groupB)).isEmpty() ); } } 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 e72665193b..dca8c81d0e 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 @@ -48,6 +48,8 @@ import org.whispersystems.signalservice.api.groupsv2.GroupHistoryPage import org.whispersystems.signalservice.api.groupsv2.InvalidGroupStateException import org.whispersystems.signalservice.api.groupsv2.NotAbleToApplyGroupV2ChangeException import org.whispersystems.signalservice.api.groupsv2.ReceivedGroupSendEndorsements +import org.whispersystems.signalservice.api.groupsv2.getChangedFields +import org.whispersystems.signalservice.api.groupsv2.isSilent import org.whispersystems.signalservice.api.push.ServiceIds import org.whispersystems.signalservice.internal.push.exceptions.GroupNotFoundException import org.whispersystems.signalservice.internal.push.exceptions.NotInGroupException @@ -633,14 +635,19 @@ class GroupsV2StateProcessor private constructor( var runningGroupState = previousGroupState for (entry in processedLogEntries) { - if (entry.change != null && DecryptedGroupUtil.changeIsEmptyExceptForProfileKeyChanges(entry.change) && !DecryptedGroupUtil.changeIsEmpty(entry.change)) { - Log.d(TAG, "Skipping profile key changes only update message") - } else if (entry.change != null && DecryptedGroupUtil.changeIsEmptyExceptForBanChangesAndOptionalProfileKeyChanges(entry.change)) { - Log.d(TAG, "Skipping ban changes only update message") - } else { - if (entry.change != null && DecryptedGroupUtil.changeIsEmpty(entry.change) && runningGroupState != null) { + val changedFields = entry.change?.getChangedFields().orEmpty() + val changeSilently = entry.change?.isSilent(changedFields) == true + + when { + entry.change != null && changeSilently && changedFields.isNotEmpty() -> { + Log.d(TAG, "Skipping silent changes: $changedFields") + } + + entry.change != null && changedFields.isEmpty() && runningGroupState != null -> { Log.w(TAG, "Empty group update message seen. Not inserting.") - } else { + } + + else -> { storeMessage(GroupProtoUtil.createDecryptedGroupV2Context(masterKey, GroupMutation(runningGroupState, entry.change, entry.group), null), runningTimestamp, serverGuid) runningTimestamp++ } diff --git a/app/src/main/java/org/thoughtcrime/securesms/sms/GroupV2UpdateMessageUtil.java b/app/src/main/java/org/thoughtcrime/securesms/sms/GroupV2UpdateMessageUtil.java index d08371420a..d763e778d8 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/sms/GroupV2UpdateMessageUtil.java +++ b/app/src/main/java/org/thoughtcrime/securesms/sms/GroupV2UpdateMessageUtil.java @@ -4,8 +4,8 @@ import androidx.annotation.NonNull; import org.signal.storageservice.storage.protos.groups.local.DecryptedGroupChange; import org.thoughtcrime.securesms.mms.MessageGroupContext; -import org.whispersystems.signalservice.api.groupsv2.DecryptedGroupUtil; import org.signal.core.models.ServiceId; +import org.whispersystems.signalservice.api.groupsv2.DecryptedGroupExtensions; import java.util.Collections; import java.util.Optional; @@ -44,7 +44,7 @@ public final class GroupV2UpdateMessageUtil { DecryptedGroupChange withoutDeletedMembers = decryptedGroupChange.newBuilder() .deleteMembers(Collections.emptyList()) .build(); - return DecryptedGroupUtil.changeIsEmpty(withoutDeletedMembers); + return DecryptedGroupExtensions.getChangedFields(withoutDeletedMembers).isEmpty(); } public static boolean isJoinRequestCancel(@NonNull MessageGroupContext groupContext) { diff --git a/lib/libsignal-service/src/main/java/org/whispersystems/signalservice/api/groupsv2/DecryptedGroupExtensions.kt b/lib/libsignal-service/src/main/java/org/whispersystems/signalservice/api/groupsv2/DecryptedGroupExtensions.kt index 5ab5355706..c9be5c56c0 100644 --- a/lib/libsignal-service/src/main/java/org/whispersystems/signalservice/api/groupsv2/DecryptedGroupExtensions.kt +++ b/lib/libsignal-service/src/main/java/org/whispersystems/signalservice/api/groupsv2/DecryptedGroupExtensions.kt @@ -3,15 +3,20 @@ * SPDX-License-Identifier: AGPL-3.0-only */ +@file:JvmName("DecryptedGroupExtensions") + package org.whispersystems.signalservice.api.groupsv2 import org.signal.core.models.ServiceId import org.signal.core.models.ServiceId.ACI +import org.signal.storageservice.storage.protos.groups.AccessControl import org.signal.storageservice.storage.protos.groups.local.DecryptedGroup +import org.signal.storageservice.storage.protos.groups.local.DecryptedGroupChange import org.signal.storageservice.storage.protos.groups.local.DecryptedMember import org.signal.storageservice.storage.protos.groups.local.DecryptedModifyMemberLabel import org.signal.storageservice.storage.protos.groups.local.DecryptedPendingMember import org.signal.storageservice.storage.protos.groups.local.DecryptedRequestingMember +import org.signal.storageservice.storage.protos.groups.local.EnabledState import java.util.Optional fun Collection.toAciListWithUnknowns(): List { @@ -54,3 +59,78 @@ fun DecryptedGroup.Builder.setModifyMemberLabelActions( members = updatedMembers } + +/** + * Returns the group change fields that contain actual changes (value is not empty or default). + */ +fun DecryptedGroupChange.getChangedFields(): Set { + return buildSet { + if (newIsAnnouncementGroup != EnabledState.UNKNOWN) add(GroupChangeField.ANNOUNCEMENT_GROUP) + if (newAttributeAccess != AccessControl.AccessRequired.UNKNOWN) add(GroupChangeField.ATTRIBUTE_ACCESS) + if (newAvatar != null) add(GroupChangeField.AVATAR) + if (deleteBannedMembers.isNotEmpty()) add(GroupChangeField.BANNED_MEMBER_REMOVALS) + if (newBannedMembers.isNotEmpty()) add(GroupChangeField.BANNED_MEMBERS) + if (newDescription != null) add(GroupChangeField.DESCRIPTION) + if (newInviteLinkAccess != AccessControl.AccessRequired.UNKNOWN) add(GroupChangeField.INVITE_LINK_ACCESS) + if (newInviteLinkPassword.size != 0) add(GroupChangeField.INVITE_LINK_PASSWORD) + if (newMemberAccess != AccessControl.AccessRequired.UNKNOWN) add(GroupChangeField.MEMBER_ACCESS) + if (modifyMemberLabels.isNotEmpty()) add(GroupChangeField.MEMBER_LABELS) + if (deleteMembers.isNotEmpty()) add(GroupChangeField.MEMBER_REMOVALS) + if (modifyMemberRoles.isNotEmpty()) add(GroupChangeField.MEMBER_ROLES) + if (newMembers.isNotEmpty()) add(GroupChangeField.MEMBERS) + if (promotePendingMembers.isNotEmpty()) add(GroupChangeField.PENDING_MEMBER_PROMOTIONS) + if (deletePendingMembers.isNotEmpty()) add(GroupChangeField.PENDING_MEMBER_REMOVALS) + if (newPendingMembers.isNotEmpty()) add(GroupChangeField.PENDING_MEMBERS) + if (promotePendingPniAciMembers.isNotEmpty()) add(GroupChangeField.PNI_ACI_PROMOTIONS) + if (modifiedProfileKeys.isNotEmpty()) add(GroupChangeField.PROFILE_KEYS) + if (promoteRequestingMembers.isNotEmpty()) add(GroupChangeField.REQUESTING_MEMBER_APPROVALS) + if (deleteRequestingMembers.isNotEmpty()) add(GroupChangeField.REQUESTING_MEMBER_REMOVALS) + if (newRequestingMembers.isNotEmpty()) add(GroupChangeField.REQUESTING_MEMBERS) + if (newTimer != null) add(GroupChangeField.TIMER) + if (newTitle != null) add(GroupChangeField.TITLE) + } +} + +/** + * Returns true if the group change should not be announced to the group members. + */ +@JvmOverloads +fun DecryptedGroupChange.isSilent( + changedFields: Set = getChangedFields() +): Boolean { + return GroupChangeField.silentChanges.containsAll(changedFields) +} + +/** + * Fields representing possible changes to a group state. + * To add a new field, update the enum and add corresponding checks in getChangedFields(). + */ +enum class GroupChangeField(val changeSilently: Boolean = false) { + ANNOUNCEMENT_GROUP, + ATTRIBUTE_ACCESS, + AVATAR, + BANNED_MEMBER_REMOVALS, + BANNED_MEMBERS(changeSilently = true), + DESCRIPTION, + INVITE_LINK_ACCESS, + INVITE_LINK_PASSWORD, + MEMBER_ACCESS, + MEMBER_LABELS(changeSilently = true), + MEMBER_REMOVALS, + MEMBER_ROLES, + MEMBERS, + PENDING_MEMBER_PROMOTIONS, + PENDING_MEMBER_REMOVALS, + PENDING_MEMBERS, + PNI_ACI_PROMOTIONS, + PROFILE_KEYS(changeSilently = true), + REQUESTING_MEMBER_APPROVALS, + REQUESTING_MEMBER_REMOVALS, + REQUESTING_MEMBERS, + TIMER, + TITLE; + + companion object { + val silentChanges = GroupChangeField.entries.filter { it.changeSilently } + } +} diff --git a/lib/libsignal-service/src/main/java/org/whispersystems/signalservice/api/groupsv2/DecryptedGroupUtil.java b/lib/libsignal-service/src/main/java/org/whispersystems/signalservice/api/groupsv2/DecryptedGroupUtil.java index 362ec0a43f..efdf050830 100644 --- a/lib/libsignal-service/src/main/java/org/whispersystems/signalservice/api/groupsv2/DecryptedGroupUtil.java +++ b/lib/libsignal-service/src/main/java/org/whispersystems/signalservice/api/groupsv2/DecryptedGroupUtil.java @@ -8,7 +8,6 @@ import org.signal.storageservice.storage.protos.groups.local.DecryptedBannedMemb import org.signal.storageservice.storage.protos.groups.local.DecryptedGroup; import org.signal.storageservice.storage.protos.groups.local.DecryptedGroupChange; import org.signal.storageservice.storage.protos.groups.local.DecryptedMember; -import org.signal.storageservice.storage.protos.groups.local.DecryptedModifyMemberLabel; import org.signal.storageservice.storage.protos.groups.local.DecryptedModifyMemberRole; import org.signal.storageservice.storage.protos.groups.local.DecryptedPendingMember; import org.signal.storageservice.storage.protos.groups.local.DecryptedPendingMemberRemoval; @@ -338,7 +337,7 @@ public final class DecryptedGroupUtil { applyPromotePendingPniAciMemberActions(builder, change.promotePendingPniAciMembers); - DecryptedGroupExtensionsKt.setModifyMemberLabelActions(builder, change.modifyMemberLabels); + DecryptedGroupExtensions.setModifyMemberLabelActions(builder, change.modifyMemberLabels); return builder.build(); } @@ -722,73 +721,4 @@ public final class DecryptedGroupUtil { } return -1; } - - public static boolean changeIsEmpty(DecryptedGroupChange change) { - return change.modifiedProfileKeys.size() == 0 && // field 6 - changeIsEmptyExceptForProfileKeyChanges(change); - } - - /* - * When updating this, update {@link #changeIsEmptyExceptForBanChangesAndOptionalProfileKeyChanges(DecryptedGroupChange)} - */ - public static boolean changeIsEmptyExceptForProfileKeyChanges(DecryptedGroupChange change) { - return change.newMembers.size() == 0 && // field 3 - change.deleteMembers.size() == 0 && // field 4 - change.modifyMemberRoles.size() == 0 && // field 5 - change.newPendingMembers.size() == 0 && // field 7 - change.deletePendingMembers.size() == 0 && // field 8 - change.promotePendingMembers.size() == 0 && // field 9 - change.newTitle == null && // field 10 - change.newAvatar == null && // field 11 - change.newTimer == null && // field 12 - isEmpty(change.newAttributeAccess) && // field 13 - isEmpty(change.newMemberAccess) && // field 14 - isEmpty(change.newInviteLinkAccess) && // field 15 - change.newRequestingMembers.size() == 0 && // field 16 - change.deleteRequestingMembers.size() == 0 && // field 17 - change.promoteRequestingMembers.size() == 0 && // field 18 - change.newInviteLinkPassword.size() == 0 && // field 19 - change.newDescription == null && // field 20 - isEmpty(change.newIsAnnouncementGroup) && // field 21 - change.newBannedMembers.size() == 0 && // field 22 - change.deleteBannedMembers.size() == 0 && // field 23 - change.promotePendingPniAciMembers.size() == 0 && // field 24 - change.modifyMemberLabels.isEmpty(); // field 26 - } - - public static boolean changeIsEmptyExceptForBanChangesAndOptionalProfileKeyChanges(DecryptedGroupChange change) { - return (change.newBannedMembers.size() != 0 || change.deleteBannedMembers.size() != 0) && - change.newMembers.size() == 0 && // field 3 - change.deleteMembers.size() == 0 && // field 4 - change.modifyMemberRoles.size() == 0 && // field 5 - change.newPendingMembers.size() == 0 && // field 7 - change.deletePendingMembers.size() == 0 && // field 8 - change.promotePendingMembers.size() == 0 && // field 9 - change.newTitle == null && // field 10 - change.newAvatar == null && // field 11 - change.newTimer == null && // field 12 - isEmpty(change.newAttributeAccess) && // field 13 - isEmpty(change.newMemberAccess) && // field 14 - isEmpty(change.newInviteLinkAccess) && // field 15 - change.newRequestingMembers.size() == 0 && // field 16 - change.deleteRequestingMembers.size() == 0 && // field 17 - change.promoteRequestingMembers.size() == 0 && // field 18 - change.newInviteLinkPassword.size() == 0 && // field 19 - change.newDescription == null && // field 20 - isEmpty(change.newIsAnnouncementGroup) && // field 21 - change.promotePendingPniAciMembers.size() == 0 && // field 24 - change.modifyMemberLabels.isEmpty(); // field 26 - } - - static boolean isEmpty(AccessControl.AccessRequired newAttributeAccess) { - return newAttributeAccess == AccessControl.AccessRequired.UNKNOWN; - } - - static boolean isEmpty(EnabledState enabledState) { - return enabledState == EnabledState.UNKNOWN; - } - - public static boolean changeIsSilent(DecryptedGroupChange plainGroupChange) { - return changeIsEmptyExceptForProfileKeyChanges(plainGroupChange) || changeIsEmptyExceptForBanChangesAndOptionalProfileKeyChanges(plainGroupChange); - } } diff --git a/lib/libsignal-service/src/test/java/org/whispersystems/signalservice/api/groupsv2/DecryptedGroupUtil_empty_Test.java b/lib/libsignal-service/src/test/java/org/whispersystems/signalservice/api/groupsv2/DecryptedGroupUtil_empty_Test.java index 7d14def7dd..6a7995bca1 100644 --- a/lib/libsignal-service/src/test/java/org/whispersystems/signalservice/api/groupsv2/DecryptedGroupUtil_empty_Test.java +++ b/lib/libsignal-service/src/test/java/org/whispersystems/signalservice/api/groupsv2/DecryptedGroupUtil_empty_Test.java @@ -31,21 +31,22 @@ import static org.whispersystems.signalservice.api.groupsv2.ProtobufTestUtils.ge @SuppressWarnings("NewClassNamingConvention") public final class DecryptedGroupUtil_empty_Test { /** - * Reflects over the generated protobuf class and ensures that no new fields have been added since we wrote this. + * Ensures that {@link GroupChangeField} enum covers all fields of {@link DecryptedGroupChange}. *

- * If we didn't, newly added fields would easily affect {@link DecryptedGroupUtil}'s ability to detect non-empty change states. + * If this test fails after a proto update, add the new field to {@link GroupChangeField} + * and update {{@link DecryptedGroupExtensions#getChangedFields(DecryptedGroupChange)}}. */ @Test - public void ensure_DecryptedGroupUtil_knows_about_all_fields_of_DecryptedGroupChange() { + public void ensure_GroupChangeField_knows_about_all_fields_of_DecryptedGroupChange() { int maxFieldFound = getMaxDeclaredFieldNumber(DecryptedGroupChange.class); - assertEquals("DecryptedGroupUtil and its tests need updating to account for new fields on " + DecryptedGroupChange.class.getName(), + assertEquals("GroupChangeField and getChangedFields() need updating to account for new fields on " + DecryptedGroupChange.class.getName(), 26, maxFieldFound); } @Test public void empty_change_set() { - assertTrue(DecryptedGroupUtil.changeIsEmpty(new DecryptedGroupChange.Builder().build())); + assertTrue(DecryptedGroupExtensions.getChangedFields(new DecryptedGroupChange.Builder().build()).isEmpty()); } @Test @@ -54,8 +55,8 @@ public final class DecryptedGroupUtil_empty_Test { .newMembers(List.of(member(UUID.randomUUID()))) .build(); - assertFalse(DecryptedGroupUtil.changeIsEmpty(change)); - assertFalse(DecryptedGroupUtil.changeIsEmptyExceptForProfileKeyChanges(change)); + assertFalse(DecryptedGroupExtensions.getChangedFields(change).isEmpty()); + assertFalse(DecryptedGroupExtensions.isSilent(change)); } @Test @@ -64,8 +65,8 @@ public final class DecryptedGroupUtil_empty_Test { .deleteMembers(List.of(UuidUtil.toByteString(UUID.randomUUID()))) .build(); - assertFalse(DecryptedGroupUtil.changeIsEmpty(change)); - assertFalse(DecryptedGroupUtil.changeIsEmptyExceptForProfileKeyChanges(change)); + assertFalse(DecryptedGroupExtensions.getChangedFields(change).isEmpty()); + assertFalse(DecryptedGroupExtensions.isSilent(change)); } @Test @@ -74,8 +75,8 @@ public final class DecryptedGroupUtil_empty_Test { .modifyMemberRoles(List.of(promoteAdmin(UUID.randomUUID()))) .build(); - assertFalse(DecryptedGroupUtil.changeIsEmpty(change)); - assertFalse(DecryptedGroupUtil.changeIsEmptyExceptForProfileKeyChanges(change)); + assertFalse(DecryptedGroupExtensions.getChangedFields(change).isEmpty()); + assertFalse(DecryptedGroupExtensions.isSilent(change)); } @Test @@ -84,8 +85,8 @@ public final class DecryptedGroupUtil_empty_Test { .modifiedProfileKeys(List.of(member(UUID.randomUUID(), randomProfileKey()))) .build(); - assertFalse(DecryptedGroupUtil.changeIsEmpty(change)); - assertTrue(DecryptedGroupUtil.changeIsEmptyExceptForProfileKeyChanges(change)); + assertFalse(DecryptedGroupExtensions.getChangedFields(change).isEmpty()); + assertTrue(DecryptedGroupExtensions.isSilent(change)); } @Test @@ -94,8 +95,8 @@ public final class DecryptedGroupUtil_empty_Test { .newPendingMembers(List.of(pendingMember(UUID.randomUUID()))) .build(); - assertFalse(DecryptedGroupUtil.changeIsEmpty(change)); - assertFalse(DecryptedGroupUtil.changeIsEmptyExceptForProfileKeyChanges(change)); + assertFalse(DecryptedGroupExtensions.getChangedFields(change).isEmpty()); + assertFalse(DecryptedGroupExtensions.isSilent(change)); } @Test @@ -104,8 +105,8 @@ public final class DecryptedGroupUtil_empty_Test { .deletePendingMembers(List.of(pendingMemberRemoval(UUID.randomUUID()))) .build(); - assertFalse(DecryptedGroupUtil.changeIsEmpty(change)); - assertFalse(DecryptedGroupUtil.changeIsEmptyExceptForProfileKeyChanges(change)); + assertFalse(DecryptedGroupExtensions.getChangedFields(change).isEmpty()); + assertFalse(DecryptedGroupExtensions.isSilent(change)); } @Test @@ -114,8 +115,8 @@ public final class DecryptedGroupUtil_empty_Test { .promotePendingMembers(List.of(member(UUID.randomUUID()))) .build(); - assertFalse(DecryptedGroupUtil.changeIsEmpty(change)); - assertFalse(DecryptedGroupUtil.changeIsEmptyExceptForProfileKeyChanges(change)); + assertFalse(DecryptedGroupExtensions.getChangedFields(change).isEmpty()); + assertFalse(DecryptedGroupExtensions.isSilent(change)); } @Test @@ -124,8 +125,8 @@ public final class DecryptedGroupUtil_empty_Test { .newTitle(new DecryptedString.Builder().value_("New title").build()) .build(); - assertFalse(DecryptedGroupUtil.changeIsEmpty(change)); - assertFalse(DecryptedGroupUtil.changeIsEmptyExceptForProfileKeyChanges(change)); + assertFalse(DecryptedGroupExtensions.getChangedFields(change).isEmpty()); + assertFalse(DecryptedGroupExtensions.isSilent(change)); } @Test @@ -134,8 +135,8 @@ public final class DecryptedGroupUtil_empty_Test { .newAvatar(new DecryptedString.Builder().value_("New Avatar").build()) .build(); - assertFalse(DecryptedGroupUtil.changeIsEmpty(change)); - assertFalse(DecryptedGroupUtil.changeIsEmptyExceptForProfileKeyChanges(change)); + assertFalse(DecryptedGroupExtensions.getChangedFields(change).isEmpty()); + assertFalse(DecryptedGroupExtensions.isSilent(change)); } @Test @@ -144,8 +145,8 @@ public final class DecryptedGroupUtil_empty_Test { .newTimer(new DecryptedTimer.Builder().duration(60).build()) .build(); - assertFalse(DecryptedGroupUtil.changeIsEmpty(change)); - assertFalse(DecryptedGroupUtil.changeIsEmptyExceptForProfileKeyChanges(change)); + assertFalse(DecryptedGroupExtensions.getChangedFields(change).isEmpty()); + assertFalse(DecryptedGroupExtensions.isSilent(change)); } @Test @@ -154,8 +155,8 @@ public final class DecryptedGroupUtil_empty_Test { .newAttributeAccess(AccessControl.AccessRequired.ADMINISTRATOR) .build(); - assertFalse(DecryptedGroupUtil.changeIsEmpty(change)); - assertFalse(DecryptedGroupUtil.changeIsEmptyExceptForProfileKeyChanges(change)); + assertFalse(DecryptedGroupExtensions.getChangedFields(change).isEmpty()); + assertFalse(DecryptedGroupExtensions.isSilent(change)); } @Test @@ -164,8 +165,8 @@ public final class DecryptedGroupUtil_empty_Test { .newMemberAccess(AccessControl.AccessRequired.MEMBER) .build(); - assertFalse(DecryptedGroupUtil.changeIsEmpty(change)); - assertFalse(DecryptedGroupUtil.changeIsEmptyExceptForProfileKeyChanges(change)); + assertFalse(DecryptedGroupExtensions.getChangedFields(change).isEmpty()); + assertFalse(DecryptedGroupExtensions.isSilent(change)); } @Test @@ -174,8 +175,8 @@ public final class DecryptedGroupUtil_empty_Test { .newInviteLinkAccess(AccessControl.AccessRequired.ADMINISTRATOR) .build(); - assertFalse(DecryptedGroupUtil.changeIsEmpty(change)); - assertFalse(DecryptedGroupUtil.changeIsEmptyExceptForProfileKeyChanges(change)); + assertFalse(DecryptedGroupExtensions.getChangedFields(change).isEmpty()); + assertFalse(DecryptedGroupExtensions.isSilent(change)); } @Test @@ -184,8 +185,8 @@ public final class DecryptedGroupUtil_empty_Test { .newRequestingMembers(List.of(new DecryptedRequestingMember())) .build(); - assertFalse(DecryptedGroupUtil.changeIsEmpty(change)); - assertFalse(DecryptedGroupUtil.changeIsEmptyExceptForProfileKeyChanges(change)); + assertFalse(DecryptedGroupExtensions.getChangedFields(change).isEmpty()); + assertFalse(DecryptedGroupExtensions.isSilent(change)); } @Test @@ -194,8 +195,8 @@ public final class DecryptedGroupUtil_empty_Test { .deleteRequestingMembers(List.of(ByteString.of(new byte[16]))) .build(); - assertFalse(DecryptedGroupUtil.changeIsEmpty(change)); - assertFalse(DecryptedGroupUtil.changeIsEmptyExceptForProfileKeyChanges(change)); + assertFalse(DecryptedGroupExtensions.getChangedFields(change).isEmpty()); + assertFalse(DecryptedGroupExtensions.isSilent(change)); } @Test @@ -204,8 +205,8 @@ public final class DecryptedGroupUtil_empty_Test { .promoteRequestingMembers(List.of(new DecryptedApproveMember())) .build(); - assertFalse(DecryptedGroupUtil.changeIsEmpty(change)); - assertFalse(DecryptedGroupUtil.changeIsEmptyExceptForProfileKeyChanges(change)); + assertFalse(DecryptedGroupExtensions.getChangedFields(change).isEmpty()); + assertFalse(DecryptedGroupExtensions.isSilent(change)); } @Test @@ -214,8 +215,8 @@ public final class DecryptedGroupUtil_empty_Test { .newInviteLinkPassword(ByteString.of(new byte[16])) .build(); - assertFalse(DecryptedGroupUtil.changeIsEmpty(change)); - assertFalse(DecryptedGroupUtil.changeIsEmptyExceptForProfileKeyChanges(change)); + assertFalse(DecryptedGroupExtensions.getChangedFields(change).isEmpty()); + assertFalse(DecryptedGroupExtensions.isSilent(change)); } @Test @@ -224,8 +225,8 @@ public final class DecryptedGroupUtil_empty_Test { .newDescription(new DecryptedString.Builder().value_("New description").build()) .build(); - assertFalse(DecryptedGroupUtil.changeIsEmpty(change)); - assertFalse(DecryptedGroupUtil.changeIsEmptyExceptForProfileKeyChanges(change)); + assertFalse(DecryptedGroupExtensions.getChangedFields(change).isEmpty()); + assertFalse(DecryptedGroupExtensions.isSilent(change)); } @Test @@ -234,8 +235,8 @@ public final class DecryptedGroupUtil_empty_Test { .newIsAnnouncementGroup(EnabledState.ENABLED) .build(); - assertFalse(DecryptedGroupUtil.changeIsEmpty(change)); - assertFalse(DecryptedGroupUtil.changeIsEmptyExceptForProfileKeyChanges(change)); + assertFalse(DecryptedGroupExtensions.getChangedFields(change).isEmpty()); + assertFalse(DecryptedGroupExtensions.isSilent(change)); } @Test @@ -244,8 +245,8 @@ public final class DecryptedGroupUtil_empty_Test { .newBannedMembers(List.of(new DecryptedBannedMember())) .build(); - assertFalse(DecryptedGroupUtil.changeIsEmpty(change)); - assertFalse(DecryptedGroupUtil.changeIsEmptyExceptForProfileKeyChanges(change)); + assertFalse(DecryptedGroupExtensions.getChangedFields(change).isEmpty()); + assertTrue(DecryptedGroupExtensions.isSilent(change)); } @Test @@ -254,8 +255,8 @@ public final class DecryptedGroupUtil_empty_Test { .deleteBannedMembers(List.of(new DecryptedBannedMember())) .build(); - assertFalse(DecryptedGroupUtil.changeIsEmpty(change)); - assertFalse(DecryptedGroupUtil.changeIsEmptyExceptForProfileKeyChanges(change)); + assertFalse(DecryptedGroupExtensions.getChangedFields(change).isEmpty()); + assertFalse(DecryptedGroupExtensions.isSilent(change)); } @Test @@ -264,12 +265,12 @@ public final class DecryptedGroupUtil_empty_Test { .promotePendingPniAciMembers(List.of(pendingPniAciMember(UUID.randomUUID(), UUID.randomUUID(), randomProfileKey()))) .build(); - assertFalse(DecryptedGroupUtil.changeIsEmpty(change)); - assertFalse(DecryptedGroupUtil.changeIsEmptyExceptForProfileKeyChanges(change)); + assertFalse(DecryptedGroupExtensions.getChangedFields(change).isEmpty()); + assertFalse(DecryptedGroupExtensions.isSilent(change)); } @Test - public void not_empty_with_modify_member_label_field_26() { + public void silent_with_modify_member_label_field_26() { DecryptedModifyMemberLabel modifyLabelAction = new DecryptedModifyMemberLabel.Builder() .aciBytes(UuidUtil.toByteString(UUID.randomUUID())) .labelEmoji("🔥") @@ -280,8 +281,29 @@ public final class DecryptedGroupUtil_empty_Test { .modifyMemberLabels(List.of(modifyLabelAction)) .build(); - assertFalse(DecryptedGroupUtil.changeIsEmpty(change)); - assertFalse(DecryptedGroupUtil.changeIsEmptyExceptForProfileKeyChanges(change)); - assertFalse(DecryptedGroupUtil.changeIsEmptyExceptForBanChangesAndOptionalProfileKeyChanges(change)); + assertFalse(DecryptedGroupExtensions.getChangedFields(change).isEmpty()); + assertTrue(DecryptedGroupExtensions.isSilent(change)); + } + + @Test + public void silent_with_profile_keys_and_banned_members() { + DecryptedGroupChange change = new DecryptedGroupChange.Builder() + .modifiedProfileKeys(List.of(member(UUID.randomUUID(), randomProfileKey()))) + .newBannedMembers(List.of(new DecryptedBannedMember())) + .build(); + + assertFalse(DecryptedGroupExtensions.getChangedFields(change).isEmpty()); + assertTrue(DecryptedGroupExtensions.isSilent(change)); + } + + @Test + public void not_silent_with_profile_keys_and_new_members() { + DecryptedGroupChange change = new DecryptedGroupChange.Builder() + .modifiedProfileKeys(List.of(member(UUID.randomUUID(), randomProfileKey()))) + .newMembers(List.of(member(UUID.randomUUID()))) + .build(); + + assertFalse(DecryptedGroupExtensions.getChangedFields(change).isEmpty()); + assertFalse(DecryptedGroupExtensions.isSilent(change)); } } diff --git a/lib/libsignal-service/src/test/java/org/whispersystems/signalservice/api/groupsv2/GroupChangeUtil_resolveConflict_decryptedOnly_Test.java b/lib/libsignal-service/src/test/java/org/whispersystems/signalservice/api/groupsv2/GroupChangeUtil_resolveConflict_decryptedOnly_Test.java index 952cac0ab3..6ba2073c12 100644 --- a/lib/libsignal-service/src/test/java/org/whispersystems/signalservice/api/groupsv2/GroupChangeUtil_resolveConflict_decryptedOnly_Test.java +++ b/lib/libsignal-service/src/test/java/org/whispersystems/signalservice/api/groupsv2/GroupChangeUtil_resolveConflict_decryptedOnly_Test.java @@ -300,7 +300,7 @@ public final class GroupChangeUtil_resolveConflict_decryptedOnly_Test { DecryptedGroupChange resolvedChanges = GroupChangeUtil.resolveConflict(groupState, decryptedChange).build(); - assertTrue(DecryptedGroupUtil.changeIsEmpty(resolvedChanges)); + assertTrue(DecryptedGroupExtensions.getChangedFields(resolvedChanges).isEmpty()); } @Test @@ -328,7 +328,7 @@ public final class GroupChangeUtil_resolveConflict_decryptedOnly_Test { DecryptedGroupChange resolvedChanges = GroupChangeUtil.resolveConflict(groupState, decryptedChange).build(); - assertTrue(DecryptedGroupUtil.changeIsEmpty(resolvedChanges)); + assertTrue(DecryptedGroupExtensions.getChangedFields(resolvedChanges).isEmpty()); } @Test @@ -356,7 +356,7 @@ public final class GroupChangeUtil_resolveConflict_decryptedOnly_Test { DecryptedGroupChange resolvedChanges = GroupChangeUtil.resolveConflict(groupState, decryptedChange).build(); - assertTrue(DecryptedGroupUtil.changeIsEmpty(resolvedChanges)); + assertTrue(DecryptedGroupExtensions.getChangedFields(resolvedChanges).isEmpty()); } @Test @@ -384,7 +384,7 @@ public final class GroupChangeUtil_resolveConflict_decryptedOnly_Test { DecryptedGroupChange resolvedChanges = GroupChangeUtil.resolveConflict(groupState, decryptedChange).build(); - assertTrue(DecryptedGroupUtil.changeIsEmpty(resolvedChanges)); + assertTrue(DecryptedGroupExtensions.getChangedFields(resolvedChanges).isEmpty()); } @Test @@ -412,7 +412,7 @@ public final class GroupChangeUtil_resolveConflict_decryptedOnly_Test { DecryptedGroupChange resolvedChanges = GroupChangeUtil.resolveConflict(groupState, decryptedChange).build(); - assertTrue(DecryptedGroupUtil.changeIsEmpty(resolvedChanges)); + assertTrue(DecryptedGroupExtensions.getChangedFields(resolvedChanges).isEmpty()); } @Test @@ -426,7 +426,7 @@ public final class GroupChangeUtil_resolveConflict_decryptedOnly_Test { DecryptedGroupChange resolvedChanges = GroupChangeUtil.resolveConflict(groupState, decryptedChange).build(); - assertTrue(DecryptedGroupUtil.changeIsEmpty(resolvedChanges)); + assertTrue(DecryptedGroupExtensions.getChangedFields(resolvedChanges).isEmpty()); } @Test @@ -572,7 +572,7 @@ public final class GroupChangeUtil_resolveConflict_decryptedOnly_Test { DecryptedGroupChange resolvedChanges = GroupChangeUtil.resolveConflict(groupState, decryptedChange).build(); - assertTrue(DecryptedGroupUtil.changeIsEmpty(resolvedChanges)); + assertTrue(DecryptedGroupExtensions.getChangedFields(resolvedChanges).isEmpty()); } @Test @@ -602,7 +602,7 @@ public final class GroupChangeUtil_resolveConflict_decryptedOnly_Test { DecryptedGroupChange resolvedChanges = GroupChangeUtil.resolveConflict(groupState, decryptedChange).build(); - assertTrue(DecryptedGroupUtil.changeIsEmpty(resolvedChanges)); + assertTrue(DecryptedGroupExtensions.getChangedFields(resolvedChanges).isEmpty()); } @Test