diff --git a/app/src/main/java/org/thoughtcrime/securesms/dependencies/ConfigFactory.kt b/app/src/main/java/org/thoughtcrime/securesms/dependencies/ConfigFactory.kt index 23a45531fa..764e071e36 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/dependencies/ConfigFactory.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/dependencies/ConfigFactory.kt @@ -34,10 +34,12 @@ import org.session.libsession.utilities.getGroup import org.session.libsession.utilities.withGroupConfigs import org.session.libsession.utilities.withMutableUserConfigs import org.session.libsignal.utilities.AccountId +import org.session.libsignal.utilities.Log import org.thoughtcrime.securesms.auth.LoginStateRepository import org.thoughtcrime.securesms.configs.ConfigToDatabaseSync import org.thoughtcrime.securesms.database.ConfigDatabase import org.thoughtcrime.securesms.database.ConfigVariant +import org.thoughtcrime.securesms.groups.isErasedGroupStub import java.util.EnumSet import java.util.concurrent.locks.ReentrantReadWriteLock import javax.inject.Inject @@ -55,6 +57,8 @@ class ConfigFactory @Inject constructor( @param:ManagerScope private val coroutineScope: CoroutineScope ) : ConfigFactoryProtocol { companion object { + private const val TAG = "ConfigFactory" + // This is a buffer period within which we will process messages which would result in a // config change, any message which would normally result in a config change which was sent // before `lastConfigMessage.timestamp - configChangeBufferPeriod` will not actually have @@ -276,6 +280,8 @@ class ConfigFactory @Inject constructor( return } + var erasedGroupStubs = emptyList() + val result = doWithMutableUserConfigs(fromMerge = true) { configs -> val config = when (userConfigType) { UserConfigType.CONTACTS -> configs.contacts @@ -291,11 +297,30 @@ class ConfigFactory @Inject constructor( .mapNotNull { hash -> messages.firstOrNull { it.hash == hash } } .maxOfOrNull { it.timestamp } + // Finish the erase the merge undid before anything reads the entry back as a group + if (userConfigType == UserConfigType.USER_GROUPS) { + erasedGroupStubs = configs.userGroups.allClosedGroupInfo() + .filter { it.isErasedGroupStub() } + .map { AccountId(it.groupAccountId) } + + erasedGroupStubs.forEach { groupId -> + Log.w(TAG, "Erasing a group the merge recreated after another device erased it") + configs.userGroups.eraseClosedGroup(groupId.hexString) + configs.convoInfoVolatile.eraseClosedGroup(groupId.hexString) + } + } + + val changed = EnumSet.of(userConfigType) + if (erasedGroupStubs.isNotEmpty()) changed.add(UserConfigType.CONVO_INFO_VOLATILE) + maxTimestamp?.let { - (config.dump() to it) to EnumSet.of(userConfigType) - } ?: (null to emptySet()) + (config.dump() to it) to changed + } ?: (null to if (erasedGroupStubs.isEmpty()) emptySet() else changed) } + // A merge write is neither pushed nor fully dumped; removeGroup's write is both + erasedGroupStubs.forEach(::removeGroup) + // Dump now regardless so we can save the timestamp to the database if (result != null) { val (dump, timestamp) = result diff --git a/app/src/main/java/org/thoughtcrime/securesms/groups/ErasedGroupStub.kt b/app/src/main/java/org/thoughtcrime/securesms/groups/ErasedGroupStub.kt new file mode 100644 index 0000000000..12b54c50f4 --- /dev/null +++ b/app/src/main/java/org/thoughtcrime/securesms/groups/ErasedGroupStub.kt @@ -0,0 +1,20 @@ +package org.thoughtcrime.securesms.groups + +import network.loki.messenger.libsession_util.util.GroupInfo + +/** + * Whether this UserGroups entry is what a config merge leaves of a group another of our devices + * erased, rather than a group we are (or were) in. + * + * libsession applies each device's diff in turn, so when one device erases a group while another + * changes one of its fields (marking it destroyed after seeing the group's info, say), the merge + * recreates the erased entry holding only the changed fields: no name and no keys. Every client + * keeps the name on its kicked and destroyed entries (libsession's mark_kicked/mark_destroyed clear + * only the keys), so a removed entry without one can only be this. libsession's header describes + * the name as invite-only, though: if any client starts clearing it after joining, this rule has to + * change. + * + * The other clients apply the same rule; keep them in step. + */ +fun GroupInfo.ClosedGroupInfo.isErasedGroupStub(): Boolean = + (destroyed || kicked) && name.isEmpty() && adminKey == null && authData == null diff --git a/app/src/test/java/org/thoughtcrime/securesms/groups/ErasedGroupStubTest.kt b/app/src/test/java/org/thoughtcrime/securesms/groups/ErasedGroupStubTest.kt new file mode 100644 index 0000000000..07e98a8bf2 --- /dev/null +++ b/app/src/test/java/org/thoughtcrime/securesms/groups/ErasedGroupStubTest.kt @@ -0,0 +1,43 @@ +package org.thoughtcrime.securesms.groups + +import com.google.common.truth.Truth.assertThat +import network.loki.messenger.libsession_util.PRIORITY_VISIBLE +import network.loki.messenger.libsession_util.util.Bytes +import network.loki.messenger.libsession_util.util.GroupInfo +import org.junit.Test + +class ErasedGroupStubTest { + private val stub = GroupInfo.ClosedGroupInfo( + groupAccountId = "03" + "ab".repeat(32), + adminKey = null, + authData = null, + priority = PRIORITY_VISIBLE, + invited = false, + name = "", + destroyed = true, + joinedAtSecs = 0L, + kicked = false, + ) + + @Test + fun `a removed entry with no name and no keys is an erased-group stub`() { + assertThat(stub.isErasedGroupStub()).isTrue() + assertThat(stub.copy(destroyed = false, kicked = true).isErasedGroupStub()).isTrue() + } + + @Test + fun `a destroyed group we were in keeps its name and is not a stub`() { + assertThat(stub.copy(name = "Book club").isErasedGroupStub()).isFalse() + } + + @Test + fun `an entry with an admin key or auth data is not a stub`() { + assertThat(stub.copy(adminKey = Bytes(ByteArray(64) { 1 })).isErasedGroupStub()).isFalse() + assertThat(stub.copy(authData = Bytes(ByteArray(100) { 7 })).isErasedGroupStub()).isFalse() + } + + @Test + fun `a group still in use is never a stub, even without a name`() { + assertThat(stub.copy(destroyed = false).isErasedGroupStub()).isFalse() + } +}