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 d19882481..efcbda760 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 @@ -217,7 +217,7 @@ public final class GroupsV2StateProcessor { if (inputGroupState == null) { try { - return updateLocalGroupFromServerPaged(revision, localState, timestamp); + return updateLocalGroupFromServerPaged(revision, localState, timestamp, false); } catch (GroupNotAMemberException e) { if (localState != null && signedGroupChange != null) { try { @@ -296,7 +296,7 @@ public final class GroupsV2StateProcessor { /** * Using network, attempt to bring the local copy of the group up to the revision specified via paging. */ - private GroupUpdateResult updateLocalGroupFromServerPaged(int revision, DecryptedGroup localState, long timestamp) throws IOException, GroupNotAMemberException { + private GroupUpdateResult updateLocalGroupFromServerPaged(int revision, DecryptedGroup localState, long timestamp, boolean forceIncludeFirst) throws IOException, GroupNotAMemberException { boolean latestRevisionOnly = revision == LATEST && (localState == null || localState.getRevision() == GroupsV2StateProcessor.RESTORE_PLACEHOLDER_REVISION); ACI selfAci = this.selfAci; @@ -324,9 +324,16 @@ public final class GroupsV2StateProcessor { } else { int revisionWeWereAdded = GroupProtoUtil.findRevisionWeWereAdded(latestServerGroup, selfAci.uuid()); int logsNeededFrom = localState != null ? Math.max(localState.getRevision(), revisionWeWereAdded) : revisionWeWereAdded; - boolean includeFirstState = localState == null || localState.getRevision() < 0 || (revision == LATEST && localState.getRevision() + 1 < latestServerGroup.getRevision()); + boolean includeFirstState = forceIncludeFirst || + localState == null || + localState.getRevision() < 0 || + (revision == LATEST && localState.getRevision() + 1 < latestServerGroup.getRevision()); - Log.i(TAG, "Requesting from server currentRevision: " + (localState != null ? localState.getRevision() : "null") + " logsNeededFrom: " + logsNeededFrom); + Log.i(TAG, + "Requesting from server currentRevision: " + (localState != null ? localState.getRevision() : "null") + + " logsNeededFrom: " + logsNeededFrom + + " includeFirstState: " + includeFirstState + + " forceIncludeFirst: " + forceIncludeFirst); inputGroupState = getFullMemberHistoryPage(localState, selfAci, logsNeededFrom, includeFirstState); } @@ -341,6 +348,15 @@ public final class GroupsV2StateProcessor { DecryptedGroup newLocalState = advanceGroupStateResult.getNewGlobalGroupState().getLocalState(); Log.i(TAG, "Advanced group to revision: " + (newLocalState != null ? newLocalState.getRevision() : "null")); + if (newLocalState != null && !inputGroupState.hasMore() && !forceIncludeFirst) { + int newLocalRevision = newLocalState.getRevision(); + int requestRevision = (revision == LATEST) ? latestServerGroup.getRevision() : revision; + if (newLocalRevision < requestRevision) { + Log.w(TAG, "Paging again with force first snapshot enabled due to error processing changes. New local revision [" + newLocalRevision + "] hasn't reached our desired level [" + requestRevision + "]"); + return updateLocalGroupFromServerPaged(revision, localState, timestamp, true); + } + } + if (newLocalState == null || newLocalState == inputGroupState.getLocalState()) { return new GroupUpdateResult(GroupState.GROUP_CONSISTENT_OR_AHEAD, null); } 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 61abe9599..273751d40 100644 --- a/app/src/test/java/org/thoughtcrime/securesms/database/GroupTestUtil.kt +++ b/app/src/test/java/org/thoughtcrime/securesms/database/GroupTestUtil.kt @@ -19,6 +19,7 @@ import org.whispersystems.signalservice.api.groupsv2.DecryptedGroupHistoryEntry import org.whispersystems.signalservice.api.groupsv2.GroupHistoryPage import org.whispersystems.signalservice.api.push.ACI import org.whispersystems.signalservice.api.push.DistributionId +import java.util.UUID fun DecryptedGroupChange.Builder.setNewDescription(description: String) { newDescription = DecryptedString.newBuilder().setValue(description).build() @@ -189,6 +190,10 @@ fun decryptedGroup( return builder.build() } +fun member(aci: UUID, role: Member.Role = Member.Role.DEFAULT, joinedAt: Int = 0): DecryptedMember { + return member(ACI.from(aci), role, joinedAt) +} + fun member(aci: ACI, role: Member.Role = Member.Role.DEFAULT, joinedAt: Int = 0): DecryptedMember { return DecryptedMember.newBuilder() .setRole(role) 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 8c881595e..c98ec241c 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 @@ -18,7 +18,9 @@ import org.mockito.Mockito.reset import org.mockito.Mockito.verify import org.robolectric.RobolectricTestRunner import org.robolectric.annotation.Config +import org.signal.core.util.logging.Log import org.signal.storageservice.protos.groups.local.DecryptedGroupChange +import org.signal.storageservice.protos.groups.local.DecryptedMember import org.signal.storageservice.protos.groups.local.DecryptedTimer import org.signal.zkgroup.groups.GroupMasterKey import org.thoughtcrime.securesms.SignalStoreRule @@ -33,7 +35,10 @@ import org.thoughtcrime.securesms.dependencies.ApplicationDependencies import org.thoughtcrime.securesms.groups.GroupId import org.thoughtcrime.securesms.groups.GroupsV2Authorization import org.thoughtcrime.securesms.jobs.RequestGroupV2InfoJob +import org.thoughtcrime.securesms.logging.CustomSignalProtocolLogger +import org.thoughtcrime.securesms.testutil.SystemOutLogger import org.thoughtcrime.securesms.util.Hex.fromStringCondensed +import org.whispersystems.libsignal.logging.SignalProtocolLoggerProvider import org.whispersystems.signalservice.api.groupsv2.GroupsV2Api import org.whispersystems.signalservice.api.push.ACI import java.util.UUID @@ -44,10 +49,10 @@ class GroupsV2StateProcessorTest { companion object { val masterKey = GroupMasterKey(fromStringCondensed("0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef")) - val selfAci = ACI.from(UUID.randomUUID()) - val otherAci = ACI.from(UUID.randomUUID()) - val selfAndOthers = listOf(member(selfAci), member(otherAci)) - val others = listOf(member(otherAci)) + val selfAci: ACI = ACI.from(UUID.randomUUID()) + val otherAci: ACI = ACI.from(UUID.randomUUID()) + val selfAndOthers: List = listOf(member(selfAci), member(otherAci)) + val others: List = listOf(member(otherAci)) } private lateinit var groupDatabase: GroupDatabase @@ -63,6 +68,9 @@ class GroupsV2StateProcessorTest { @Before fun setUp() { + Log.initialize(SystemOutLogger()) + SignalProtocolLoggerProvider.setProvider(CustomSignalProtocolLogger()) + groupDatabase = mock(GroupDatabase::class.java) recipientDatabase = mock(RecipientDatabase::class.java) groupsV2API = mock(GroupsV2Api::class.java) @@ -372,4 +380,61 @@ class GroupsV2StateProcessorTest { assertThat("title matches revision approved at", result.latestServer!!.title, `is`("Beam me up")) verify(ApplicationDependencies.getJobManager()).add(isA(RequestGroupV2InfoJob::class.java)) } + + @Test + fun `when failing to update fully to desired revision, then try again forcing inclusion of full group state, and then successfully update from server to latest revision`() { + val randomMembers = listOf(member(UUID.randomUUID()), member(UUID.randomUUID())) + given { + localState( + revision = 100, + title = "Title", + members = others + ) + serverState( + extendGroup = localState, + revision = 101, + members = listOf(others[0], randomMembers[0], member(selfAci, joinedAt = 100)) + ) + changeSet { + changeLog(100) { + change { + addNewMembers(member(selfAci, joinedAt = 100)) + } + } + changeLog(101) { + change { + addDeleteMembers(randomMembers[1].uuid) + addModifiedProfileKeys(randomMembers[0]) + } + } + } + apiCallParameters(100, false) + } + + val secondApiCallChangeSet = GroupStateTestData(masterKey).apply { + changeSet { + changeLog(100) { + fullSnapshot( + extendGroup = localState, + members = selfAndOthers + randomMembers[0] + randomMembers[1] + ) + change { + addNewMembers(member(selfAci, joinedAt = 100)) + } + } + changeLog(101) { + change { + addDeleteMembers(randomMembers[1].uuid) + addModifiedProfileKeys(randomMembers[0]) + } + } + } + } + doReturn(secondApiCallChangeSet.changeSet!!.toApiResponse()).`when`(groupsV2API).getGroupHistoryPage(any(), eq(100), any(), eq(true)) + + val result = processor.updateLocalGroupToRevision(GroupsV2StateProcessor.LATEST, 0, null) + + assertThat("local should update to server", result.groupState, `is`(GroupsV2StateProcessor.GroupState.GROUP_UPDATED)) + assertThat("revision matches latest revision on server", result.latestServer!!.revision, `is`(101)) + } }