diff --git a/app/src/main/java/org/thoughtcrime/securesms/groups/v2/processing/GroupsV2StateProcessor.java b/app/src/main/java/org/thoughtcrime/securesms/groups/v2/processing/GroupsV2StateProcessor.java index 39575adc6a..1bada6f464 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/groups/v2/processing/GroupsV2StateProcessor.java +++ b/app/src/main/java/org/thoughtcrime/securesms/groups/v2/processing/GroupsV2StateProcessor.java @@ -272,7 +272,11 @@ public final class GroupsV2StateProcessor { private void insertUpdateMessages(long timestamp, Collection processedLogEntries) { for (GroupLogEntry entry : processedLogEntries) { - storeMessage(GroupProtoUtil.createDecryptedGroupV2Context(masterKey, entry.getGroup(), entry.getChange(), null), timestamp); + if (entry.getChange() != null && DecryptedGroupUtil.changeIsEmptyExceptForProfileKeyChanges(entry.getChange())) { + Log.d(TAG, "Skipping profile key changes only update message"); + } else { + storeMessage(GroupProtoUtil.createDecryptedGroupV2Context(masterKey, entry.getGroup(), entry.getChange(), null), timestamp); + } } } diff --git a/libsignal/service/src/main/java/org/whispersystems/signalservice/api/groupsv2/DecryptedGroupUtil.java b/libsignal/service/src/main/java/org/whispersystems/signalservice/api/groupsv2/DecryptedGroupUtil.java index 669aa34228..e2f5f9919d 100644 --- a/libsignal/service/src/main/java/org/whispersystems/signalservice/api/groupsv2/DecryptedGroupUtil.java +++ b/libsignal/service/src/main/java/org/whispersystems/signalservice/api/groupsv2/DecryptedGroupUtil.java @@ -23,6 +23,8 @@ import java.util.UUID; public final class DecryptedGroupUtil { + static final int MAX_CHANGE_FIELD = 14; + public static Set toUuidSet(Collection membersList) { HashSet uuids = new HashSet<>(membersList.size()); @@ -282,6 +284,29 @@ public final class DecryptedGroupUtil { .orNull()); } + public static boolean changeIsEmpty(DecryptedGroupChange change) { + return change.getModifiedProfileKeysCount() == 0 && // field 6 + changeIsEmptyExceptForProfileKeyChanges(change); + } + + public static boolean changeIsEmptyExceptForProfileKeyChanges(DecryptedGroupChange change) { + return change.getNewMembersCount() == 0 && // field 3 + change.getDeleteMembersCount() == 0 && // field 4 + change.getModifyMemberRolesCount() == 0 && // field 5 + change.getNewPendingMembersCount() == 0 && // field 7 + change.getDeletePendingMembersCount() == 0 && // field 8 + change.getPromotePendingMembersCount() == 0 && // field 9 + !change.hasNewTitle() && // field 10 + !change.hasNewAvatar() && // field 11 + !change.hasNewTimer() && // field 12 + isSet(change.getNewAttributeAccess()) && // field 13 + isSet(change.getNewMemberAccess()); // field 14 + } + + static boolean isSet(AccessControl.AccessRequired newAttributeAccess) { + return newAttributeAccess == AccessControl.AccessRequired.UNKNOWN; + } + public static class NotAbleToApplyChangeException extends Throwable { } } diff --git a/libsignal/service/src/test/java/org/whispersystems/signalservice/api/groupsv2/DecryptedGroupUtil_empty_Test.java b/libsignal/service/src/test/java/org/whispersystems/signalservice/api/groupsv2/DecryptedGroupUtil_empty_Test.java new file mode 100644 index 0000000000..28cc6fcbd1 --- /dev/null +++ b/libsignal/service/src/test/java/org/whispersystems/signalservice/api/groupsv2/DecryptedGroupUtil_empty_Test.java @@ -0,0 +1,161 @@ +package org.whispersystems.signalservice.api.groupsv2; + +import org.junit.Test; +import org.signal.storageservice.protos.groups.AccessControl; +import org.signal.storageservice.protos.groups.local.DecryptedGroupChange; +import org.signal.storageservice.protos.groups.local.DecryptedString; +import org.signal.storageservice.protos.groups.local.DecryptedTimer; +import org.whispersystems.signalservice.api.util.UuidUtil; + +import java.util.UUID; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; +import static org.whispersystems.signalservice.api.groupsv2.ProtoTestUtils.member; +import static org.whispersystems.signalservice.api.groupsv2.ProtoTestUtils.pendingMember; +import static org.whispersystems.signalservice.api.groupsv2.ProtoTestUtils.pendingMemberRemoval; +import static org.whispersystems.signalservice.api.groupsv2.ProtoTestUtils.promoteAdmin; +import static org.whispersystems.signalservice.api.groupsv2.ProtoTestUtils.randomProfileKey; +import static org.whispersystems.signalservice.api.groupsv2.ProtobufTestUtils.getMaxDeclaredFieldNumber; + +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. + *

+ * If we didn't, newly added fields would easily affect {@link DecryptedGroupUtil}'s ability to detect non-empty change states. + */ + @Test + public void ensure_DecryptedGroupUtil_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(), + DecryptedGroupUtil.MAX_CHANGE_FIELD, maxFieldFound); + } + + @Test + public void empty_change_set() { + assertTrue(DecryptedGroupUtil.changeIsEmpty(DecryptedGroupChange.newBuilder().build())); + } + + @Test + public void not_empty_with_add_member_field_3() { + DecryptedGroupChange change = DecryptedGroupChange.newBuilder() + .addNewMembers(member(UUID.randomUUID())) + .build(); + + assertFalse(DecryptedGroupUtil.changeIsEmpty(change)); + assertFalse(DecryptedGroupUtil.changeIsEmptyExceptForProfileKeyChanges(change)); + } + + @Test + public void not_empty_with_delete_member_field_4() { + DecryptedGroupChange change = DecryptedGroupChange.newBuilder() + .addDeleteMembers(UuidUtil.toByteString(UUID.randomUUID())) + .build(); + + assertFalse(DecryptedGroupUtil.changeIsEmpty(change)); + assertFalse(DecryptedGroupUtil.changeIsEmptyExceptForProfileKeyChanges(change)); + } + + @Test + public void not_empty_with_modify_member_roles_field_5() { + DecryptedGroupChange change = DecryptedGroupChange.newBuilder() + .addModifyMemberRoles(promoteAdmin(UUID.randomUUID())) + .build(); + + assertFalse(DecryptedGroupUtil.changeIsEmpty(change)); + assertFalse(DecryptedGroupUtil.changeIsEmptyExceptForProfileKeyChanges(change)); + } + + @Test + public void not_empty_with_modify_profile_keys_field_6() { + DecryptedGroupChange change = DecryptedGroupChange.newBuilder() + .addModifiedProfileKeys(member(UUID.randomUUID(), randomProfileKey())) + .build(); + + assertFalse(DecryptedGroupUtil.changeIsEmpty(change)); + assertTrue(DecryptedGroupUtil.changeIsEmptyExceptForProfileKeyChanges(change)); + } + + @Test + public void not_empty_with_add_pending_members_field_7() { + DecryptedGroupChange change = DecryptedGroupChange.newBuilder() + .addNewPendingMembers(pendingMember(UUID.randomUUID())) + .build(); + + assertFalse(DecryptedGroupUtil.changeIsEmpty(change)); + assertFalse(DecryptedGroupUtil.changeIsEmptyExceptForProfileKeyChanges(change)); + } + + @Test + public void not_empty_with_delete_pending_members_field_8() { + DecryptedGroupChange change = DecryptedGroupChange.newBuilder() + .addDeletePendingMembers(pendingMemberRemoval(UUID.randomUUID())) + .build(); + + assertFalse(DecryptedGroupUtil.changeIsEmpty(change)); + assertFalse(DecryptedGroupUtil.changeIsEmptyExceptForProfileKeyChanges(change)); + } + + @Test + public void not_empty_with_promote_delete_pending_members_field_9() { + DecryptedGroupChange change = DecryptedGroupChange.newBuilder() + .addPromotePendingMembers(member(UUID.randomUUID())) + .build(); + + assertFalse(DecryptedGroupUtil.changeIsEmpty(change)); + assertFalse(DecryptedGroupUtil.changeIsEmptyExceptForProfileKeyChanges(change)); + } + + @Test + public void not_empty_with_modify_title_field_10() { + DecryptedGroupChange change = DecryptedGroupChange.newBuilder() + .setNewTitle(DecryptedString.newBuilder().setValue("New title")) + .build(); + + assertFalse(DecryptedGroupUtil.changeIsEmpty(change)); + assertFalse(DecryptedGroupUtil.changeIsEmptyExceptForProfileKeyChanges(change)); + } + + @Test + public void not_empty_with_modify_avatar_field_11() { + DecryptedGroupChange change = DecryptedGroupChange.newBuilder() + .setNewAvatar(DecryptedString.newBuilder().setValue("New Avatar")) + .build(); + + assertFalse(DecryptedGroupUtil.changeIsEmpty(change)); + assertFalse(DecryptedGroupUtil.changeIsEmptyExceptForProfileKeyChanges(change)); + } + + @Test + public void not_empty_with_modify_disappearing_message_timer_field_12() { + DecryptedGroupChange change = DecryptedGroupChange.newBuilder() + .setNewTimer(DecryptedTimer.newBuilder().setDuration(60)) + .build(); + + assertFalse(DecryptedGroupUtil.changeIsEmpty(change)); + assertFalse(DecryptedGroupUtil.changeIsEmptyExceptForProfileKeyChanges(change)); + } + + @Test + public void not_empty_with_modify_attributes_field_13() { + DecryptedGroupChange change = DecryptedGroupChange.newBuilder() + .setNewAttributeAccess(AccessControl.AccessRequired.ADMINISTRATOR) + .build(); + + assertFalse(DecryptedGroupUtil.changeIsEmpty(change)); + assertFalse(DecryptedGroupUtil.changeIsEmptyExceptForProfileKeyChanges(change)); + } + + @Test + public void not_empty_with_modify_member_access_field_14() { + DecryptedGroupChange change = DecryptedGroupChange.newBuilder() + .setNewMemberAccess(AccessControl.AccessRequired.MEMBER) + .build(); + + assertFalse(DecryptedGroupUtil.changeIsEmpty(change)); + assertFalse(DecryptedGroupUtil.changeIsEmptyExceptForProfileKeyChanges(change)); + } +} diff --git a/libsignal/service/src/test/java/org/whispersystems/signalservice/api/groupsv2/GroupChangeUtilTest.java b/libsignal/service/src/test/java/org/whispersystems/signalservice/api/groupsv2/GroupChangeUtilTest.java index f4300d13eb..9c41f4b6ce 100644 --- a/libsignal/service/src/test/java/org/whispersystems/signalservice/api/groupsv2/GroupChangeUtilTest.java +++ b/libsignal/service/src/test/java/org/whispersystems/signalservice/api/groupsv2/GroupChangeUtilTest.java @@ -3,11 +3,10 @@ package org.whispersystems.signalservice.api.groupsv2; import org.junit.Test; import org.signal.storageservice.protos.groups.GroupChange; -import java.util.stream.Stream; - import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; +import static org.whispersystems.signalservice.api.groupsv2.ProtobufTestUtils.getMaxDeclaredFieldNumber; public final class GroupChangeUtilTest { @@ -18,17 +17,7 @@ public final class GroupChangeUtilTest { */ @Test public void ensure_GroupChangeUtil_knows_about_all_fields_of_GroupChange_Actions() { - int maxFieldFound = Stream.of(GroupChange.Actions.class.getFields()) - .filter(f -> f.getType() == int.class) - .mapToInt(f -> { - try { - return (int) f.get(null); - } catch (IllegalAccessException e) { - throw new AssertionError(e); - } - }) - .max() - .orElse(0); + int maxFieldFound = getMaxDeclaredFieldNumber(GroupChange.Actions.class); assertEquals("GroupChangeUtil and its tests need updating to account for new fields on " + GroupChange.Actions.class.getName(), GroupChangeUtil.CHANGE_ACTION_MAX_FIELD, 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 new file mode 100644 index 0000000000..f9dc671299 --- /dev/null +++ b/libsignal/service/src/test/java/org/whispersystems/signalservice/api/groupsv2/ProtobufTestUtils.java @@ -0,0 +1,25 @@ +package org.whispersystems.signalservice.api.groupsv2; + +import com.google.protobuf.MessageLite; + +import java.util.stream.Stream; + +final class ProtobufTestUtils { + + /** + * Finds the largest declared field number in the supplied protobuf class. + */ + static int getMaxDeclaredFieldNumber(Class protobufClass) { + return Stream.of(protobufClass.getFields()) + .filter(f -> f.getType() == int.class) + .mapToInt(f -> { + try { + return (int) f.get(null); + } catch (IllegalAccessException e) { + throw new AssertionError(e); + } + }) + .max() + .orElse(0); + } +}