From 5f8d09183217c91cbbcf033a093600ff7e960896 Mon Sep 17 00:00:00 2001 From: yamilmedina Date: Wed, 15 Nov 2023 17:59:43 +0100 Subject: [PATCH 01/30] feat: preparing adding members for retry --- .../data/conversation/ConversationGroupRepository.kt | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/conversation/ConversationGroupRepository.kt b/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/conversation/ConversationGroupRepository.kt index 4760884d7b4..5ba1455e1d0 100644 --- a/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/conversation/ConversationGroupRepository.kt +++ b/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/conversation/ConversationGroupRepository.kt @@ -24,13 +24,13 @@ import com.wire.kalium.logic.NetworkFailure import com.wire.kalium.logic.data.event.EventMapper import com.wire.kalium.logic.data.id.ConversationId import com.wire.kalium.logic.data.id.GroupID +import com.wire.kalium.logic.data.id.SelfTeamIdProvider import com.wire.kalium.logic.data.id.toApi import com.wire.kalium.logic.data.id.toDao import com.wire.kalium.logic.data.id.toModel import com.wire.kalium.logic.data.service.ServiceId import com.wire.kalium.logic.data.user.UserId import com.wire.kalium.logic.di.MapperProvider -import com.wire.kalium.logic.data.id.SelfTeamIdProvider import com.wire.kalium.logic.functional.Either import com.wire.kalium.logic.functional.flatMap import com.wire.kalium.logic.functional.map @@ -185,15 +185,19 @@ internal class ConversationGroupRepositoryImpl( is ConversationEntity.ProtocolInfo.Mixed -> tryAddMembersToCloudAndStorage(userIdList, conversationId) .flatMap { - mlsConversationRepository.addMemberToMLSGroup(GroupID(protocol.groupId), userIdList) + tryAddMembersToMLSGroup(protocol.groupId, userIdList) } is ConversationEntity.ProtocolInfo.MLS -> { - mlsConversationRepository.addMemberToMLSGroup(GroupID(protocol.groupId), userIdList) + tryAddMembersToMLSGroup(protocol.groupId, userIdList) } } } + private suspend fun tryAddMembersToMLSGroup(groupId: String, userIdList: List): Either = + mlsConversationRepository.addMemberToMLSGroup(GroupID(groupId), userIdList) + + override suspend fun addService(serviceId: ServiceId, conversationId: ConversationId): Either = wrapStorageRequest { conversationDAO.getConversationProtocolInfo(conversationId.toDao()) } .flatMap { protocol -> From 56e3c7864e69f1c224e90370a9b30fe01b086c56 Mon Sep 17 00:00:00 2001 From: yamilmedina Date: Fri, 17 Nov 2023 12:24:01 +0100 Subject: [PATCH 02/30] chore: move forward with error handling on convo repo layer --- .../logic/data/conversation/ConversationGroupRepository.kt | 4 ++++ .../wire/kalium/logic/data/keypackage/KeyPackageRepository.kt | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/conversation/ConversationGroupRepository.kt b/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/conversation/ConversationGroupRepository.kt index 5ba1455e1d0..f0f901aae0d 100644 --- a/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/conversation/ConversationGroupRepository.kt +++ b/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/conversation/ConversationGroupRepository.kt @@ -194,6 +194,10 @@ internal class ConversationGroupRepositoryImpl( } } + /** + * Handle the error cases and retry for claimPackages offline and out of packages. + * Handle error case and retry for sendingCommit unreachable. + */ private suspend fun tryAddMembersToMLSGroup(groupId: String, userIdList: List): Either = mlsConversationRepository.addMemberToMLSGroup(GroupID(groupId), userIdList) diff --git a/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/keypackage/KeyPackageRepository.kt b/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/keypackage/KeyPackageRepository.kt index a993a238bf2..46e703845ce 100644 --- a/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/keypackage/KeyPackageRepository.kt +++ b/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/keypackage/KeyPackageRepository.kt @@ -70,7 +70,7 @@ class KeyPackageDataSource( ) }.flatMap { if (it.keyPackages.isEmpty() && userId != selfUserId) { - Either.Left(CoreFailure.NoKeyPackagesAvailable(userId)) + Either.Left(CoreFailure.NoKeyPackagesAvailable(userId)) // todo, handle similar to unreachable with list of users } else { Either.Right(it.keyPackages) } From e6a22996910be79be864d59cd52bf7c7221041a8 Mon Sep 17 00:00:00 2001 From: yamilmedina Date: Fri, 17 Nov 2023 12:26:07 +0100 Subject: [PATCH 03/30] chore: move forward with error handling on convo repo layer --- .../logic/data/conversation/ConversationGroupRepository.kt | 1 - 1 file changed, 1 deletion(-) diff --git a/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/conversation/ConversationGroupRepository.kt b/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/conversation/ConversationGroupRepository.kt index f0f901aae0d..5474d086a6c 100644 --- a/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/conversation/ConversationGroupRepository.kt +++ b/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/conversation/ConversationGroupRepository.kt @@ -201,7 +201,6 @@ internal class ConversationGroupRepositoryImpl( private suspend fun tryAddMembersToMLSGroup(groupId: String, userIdList: List): Either = mlsConversationRepository.addMemberToMLSGroup(GroupID(groupId), userIdList) - override suspend fun addService(serviceId: ServiceId, conversationId: ConversationId): Either = wrapStorageRequest { conversationDAO.getConversationProtocolInfo(conversationId.toDao()) } .flatMap { protocol -> From da22b77c008ac10d838db8f29995dc31a6fc1e7a Mon Sep 17 00:00:00 2001 From: yamilmedina Date: Fri, 17 Nov 2023 12:45:18 +0100 Subject: [PATCH 04/30] chore: move forward with error handling on convo repo layer --- .../data/conversation/ConversationGroupRepository.kt | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/conversation/ConversationGroupRepository.kt b/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/conversation/ConversationGroupRepository.kt index 5474d086a6c..a96072a677d 100644 --- a/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/conversation/ConversationGroupRepository.kt +++ b/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/conversation/ConversationGroupRepository.kt @@ -198,8 +198,13 @@ internal class ConversationGroupRepositoryImpl( * Handle the error cases and retry for claimPackages offline and out of packages. * Handle error case and retry for sendingCommit unreachable. */ - private suspend fun tryAddMembersToMLSGroup(groupId: String, userIdList: List): Either = - mlsConversationRepository.addMemberToMLSGroup(GroupID(groupId), userIdList) + private suspend fun tryAddMembersToMLSGroup(groupId: String, userIdList: List): Either { + val operationResult = mlsConversationRepository.addMemberToMLSGroup(GroupID(groupId), userIdList) + when (operationResult) { + is Either.Left -> TODO("retry for claimPackages and offline sendCommit cases") + is Either.Right -> TODO("=)") + } + } override suspend fun addService(serviceId: ServiceId, conversationId: ConversationId): Either = wrapStorageRequest { conversationDAO.getConversationProtocolInfo(conversationId.toDao()) } From ecaa03a44c03ae09e9642652798524d50938d237 Mon Sep 17 00:00:00 2001 From: yamilmedina Date: Fri, 17 Nov 2023 17:36:47 +0100 Subject: [PATCH 05/30] chore: move forward with error handling on convo repo layer --- .../conversation/ConversationGroupRepository.kt | 16 +++++++++++----- .../data/keypackage/KeyPackageRepository.kt | 1 + .../api/v5/authenticated/KeyPackageApiV5.kt | 6 +++++- 3 files changed, 17 insertions(+), 6 deletions(-) diff --git a/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/conversation/ConversationGroupRepository.kt b/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/conversation/ConversationGroupRepository.kt index a96072a677d..3c42cc62ab1 100644 --- a/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/conversation/ConversationGroupRepository.kt +++ b/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/conversation/ConversationGroupRepository.kt @@ -198,11 +198,17 @@ internal class ConversationGroupRepositoryImpl( * Handle the error cases and retry for claimPackages offline and out of packages. * Handle error case and retry for sendingCommit unreachable. */ - private suspend fun tryAddMembersToMLSGroup(groupId: String, userIdList: List): Either { - val operationResult = mlsConversationRepository.addMemberToMLSGroup(GroupID(groupId), userIdList) - when (operationResult) { - is Either.Left -> TODO("retry for claimPackages and offline sendCommit cases") - is Either.Right -> TODO("=)") + private suspend fun tryAddMembersToMLSGroup( + groupId: String, + userIdList: List, + failedUsersList: Set = emptySet() + ): Either { + return when (val addingMemberResult = mlsConversationRepository.addMemberToMLSGroup(GroupID(groupId), userIdList)) { + is Either.Right -> addingMemberResult + is Either.Left -> { + + TODO("retry for claimPackages and offline sendCommit cases") + } } } diff --git a/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/keypackage/KeyPackageRepository.kt b/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/keypackage/KeyPackageRepository.kt index 46e703845ce..0242623e559 100644 --- a/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/keypackage/KeyPackageRepository.kt +++ b/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/keypackage/KeyPackageRepository.kt @@ -61,6 +61,7 @@ class KeyPackageDataSource( private val idMapper: IdMapper = MapperProvider.idMapper(), ) : KeyPackageRepository { + // todo(ym) make it federation aware and make it to return list of failed users, aka accumulate responses. override suspend fun claimKeyPackages(userIds: List): Either> = currentClientIdProvider().flatMap { selfClientId -> userIds.map { userId -> diff --git a/network/src/commonMain/kotlin/com/wire/kalium/network/api/v5/authenticated/KeyPackageApiV5.kt b/network/src/commonMain/kotlin/com/wire/kalium/network/api/v5/authenticated/KeyPackageApiV5.kt index 58d37bc988a..b6b1d0eb7f1 100644 --- a/network/src/commonMain/kotlin/com/wire/kalium/network/api/v5/authenticated/KeyPackageApiV5.kt +++ b/network/src/commonMain/kotlin/com/wire/kalium/network/api/v5/authenticated/KeyPackageApiV5.kt @@ -27,6 +27,8 @@ import com.wire.kalium.network.api.base.authenticated.keypackage.KeyPackageList import com.wire.kalium.network.api.v4.authenticated.KeyPackageApiV4 import com.wire.kalium.network.kaliumLogger import com.wire.kalium.network.utils.NetworkResponse +import com.wire.kalium.network.utils.handleUnsuccessfulResponse +import com.wire.kalium.network.utils.wrapFederationResponse import com.wire.kalium.network.utils.wrapKaliumResponse import io.ktor.client.request.delete import io.ktor.client.request.get @@ -41,7 +43,9 @@ internal open class KeyPackageApiV5 internal constructor( override suspend fun claimKeyPackages( param: KeyPackageApi.Param - ): NetworkResponse = wrapKaliumResponse { + ): NetworkResponse = wrapKaliumResponse(unsuccessfulResponseOverride = { response -> + wrapFederationResponse(response) { handleUnsuccessfulResponse(response) } + }) { httpClient.post("$PATH_KEY_PACKAGES/$PATH_CLAIM/${param.user.domain}/${param.user.value}") { if (param is KeyPackageApi.Param.SkipOwnClient) { parameter(QUERY_SKIP_OWN, param.selfClientId) From 37a8472ae616362d7bba61dd042c30a6bf3db332 Mon Sep 17 00:00:00 2001 From: yamilmedina Date: Mon, 20 Nov 2023 12:08:38 +0100 Subject: [PATCH 06/30] feat: add fold accumulation and bubble up failed users --- .../com/wire/kalium/logic/CoreFailure.kt | 3 ++- .../data/keypackage/KeyPackageRepository.kt | 27 ++++++++++--------- .../keypackage/KeyPackageRepositoryTest.kt | 2 +- 3 files changed, 18 insertions(+), 14 deletions(-) diff --git a/logic/src/commonMain/kotlin/com/wire/kalium/logic/CoreFailure.kt b/logic/src/commonMain/kotlin/com/wire/kalium/logic/CoreFailure.kt index 3bafa321f4f..c09abf5f672 100644 --- a/logic/src/commonMain/kotlin/com/wire/kalium/logic/CoreFailure.kt +++ b/logic/src/commonMain/kotlin/com/wire/kalium/logic/CoreFailure.kt @@ -56,7 +56,7 @@ sealed interface CoreFailure { * A user has no key packages available which prevents him/her from being added * to an existing or new conversation. */ - data class NoKeyPackagesAvailable(val userId: UserId) : CoreFailure + data class NoKeyPackagesAvailable(val userId: Set) : CoreFailure /** * It's not allowed to run the application with development API enabled when @@ -107,6 +107,7 @@ sealed interface CoreFailure { * the event is skipped and the sync continues */ data object FeatureNotImplemented : FeatureFailure() + /** * No common Protocol found in order to establish a conversation between parties. * Could be, for example, that the desired user only supports Proteus, but we only support MLS. diff --git a/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/keypackage/KeyPackageRepository.kt b/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/keypackage/KeyPackageRepository.kt index 0242623e559..a4c7ea654dc 100644 --- a/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/keypackage/KeyPackageRepository.kt +++ b/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/keypackage/KeyPackageRepository.kt @@ -22,14 +22,14 @@ import com.wire.kalium.logic.CoreFailure import com.wire.kalium.logic.NetworkFailure import com.wire.kalium.logic.data.client.MLSClientProvider import com.wire.kalium.logic.data.conversation.ClientId +import com.wire.kalium.logic.data.id.CurrentClientIdProvider import com.wire.kalium.logic.data.id.IdMapper import com.wire.kalium.logic.data.id.toApi import com.wire.kalium.logic.data.user.UserId import com.wire.kalium.logic.di.MapperProvider -import com.wire.kalium.logic.data.id.CurrentClientIdProvider import com.wire.kalium.logic.functional.Either import com.wire.kalium.logic.functional.flatMap -import com.wire.kalium.logic.functional.foldToEitherWhileRight +import com.wire.kalium.logic.functional.fold import com.wire.kalium.logic.wrapApiRequest import com.wire.kalium.logic.wrapMLSRequest import com.wire.kalium.network.api.base.authenticated.keypackage.KeyPackageApi @@ -61,23 +61,26 @@ class KeyPackageDataSource( private val idMapper: IdMapper = MapperProvider.idMapper(), ) : KeyPackageRepository { - // todo(ym) make it federation aware and make it to return list of failed users, aka accumulate responses. override suspend fun claimKeyPackages(userIds: List): Either> = currentClientIdProvider().flatMap { selfClientId -> - userIds.map { userId -> + val failedUsers = mutableSetOf() + val keyPackages = mutableListOf() + userIds.forEach { userId -> wrapApiRequest { - keyPackageApi.claimKeyPackages( - KeyPackageApi.Param.SkipOwnClient(userId.toApi(), selfClientId.value) - ) - }.flatMap { + keyPackageApi.claimKeyPackages(KeyPackageApi.Param.SkipOwnClient(userId.toApi(), selfClientId.value)) + }.fold({ failedUsers.add(userId) }) { if (it.keyPackages.isEmpty() && userId != selfUserId) { - Either.Left(CoreFailure.NoKeyPackagesAvailable(userId)) // todo, handle similar to unreachable with list of users + failedUsers.add(userId) } else { - Either.Right(it.keyPackages) + keyPackages.addAll(it.keyPackages) } } - }.foldToEitherWhileRight(emptyList()) { item, acc -> - item.flatMap { Either.Right(acc + it) } + } + + if (failedUsers.isNotEmpty()) { + Either.Left(CoreFailure.NoKeyPackagesAvailable(failedUsers)) + } else { + Either.Right(keyPackages) } } diff --git a/logic/src/commonTest/kotlin/com/wire/kalium/logic/data/keypackage/KeyPackageRepositoryTest.kt b/logic/src/commonTest/kotlin/com/wire/kalium/logic/data/keypackage/KeyPackageRepositoryTest.kt index 0e408de9ddb..c01e172c448 100644 --- a/logic/src/commonTest/kotlin/com/wire/kalium/logic/data/keypackage/KeyPackageRepositoryTest.kt +++ b/logic/src/commonTest/kotlin/com/wire/kalium/logic/data/keypackage/KeyPackageRepositoryTest.kt @@ -110,7 +110,7 @@ class KeyPackageRepositoryTest { val result = keyPackageRepository.claimKeyPackages(listOf(Arrangement.USER_ID)) result.shouldFail { failure -> - assertEquals(CoreFailure.NoKeyPackagesAvailable(Arrangement.USER_ID), failure) + assertEquals(CoreFailure.NoKeyPackagesAvailable(setOf(Arrangement.USER_ID)), failure) } } From a98b0f3b967059b882b6c7eb9d1fea983a2605ff Mon Sep 17 00:00:00 2001 From: yamilmedina Date: Mon, 20 Nov 2023 15:30:51 +0100 Subject: [PATCH 07/30] feat: wrap result into dto to avoid calling api twice --- .../conversation/MLSConversationRepository.kt | 2 +- .../data/keypackage/ClaimedKeyPackages.kt | 26 +++++++++++++++++++ .../data/keypackage/KeyPackageRepository.kt | 11 +++----- .../MLSConversationRepositoryTest.kt | 3 ++- .../keypackage/KeyPackageRepositoryTest.kt | 8 +++--- 5 files changed, 37 insertions(+), 13 deletions(-) create mode 100644 logic/src/commonMain/kotlin/com/wire/kalium/logic/data/keypackage/ClaimedKeyPackages.kt diff --git a/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/conversation/MLSConversationRepository.kt b/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/conversation/MLSConversationRepository.kt index fadda7f9584..1e732d055f6 100644 --- a/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/conversation/MLSConversationRepository.kt +++ b/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/conversation/MLSConversationRepository.kt @@ -401,7 +401,7 @@ internal class MLSConversationDataSource( retryOnCommitFailure(groupID, retryOnStaleMessage = retryOnStaleMessage) { keyPackageRepository.claimKeyPackages(userIdList).flatMap { keyPackages -> mlsClientProvider.getMLSClient().flatMap { mlsClient -> - val clientKeyPackageList = keyPackages + val clientKeyPackageList = keyPackages.keyPackages .map { Pair( CryptoQualifiedClientId(it.clientID, CryptoQualifiedID(it.userId, it.domain)), diff --git a/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/keypackage/ClaimedKeyPackages.kt b/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/keypackage/ClaimedKeyPackages.kt new file mode 100644 index 00000000000..05675517e95 --- /dev/null +++ b/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/keypackage/ClaimedKeyPackages.kt @@ -0,0 +1,26 @@ +/* + * Wire + * Copyright (C) 2023 Wire Swiss GmbH + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see http://www.gnu.org/licenses/. + */ +package com.wire.kalium.logic.data.keypackage + +import com.wire.kalium.logic.data.user.UserId +import com.wire.kalium.network.api.base.authenticated.keypackage.KeyPackageDTO + +data class ClaimedKeyPackages( + val keyPackages: List, + val failedUsers: Set +) diff --git a/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/keypackage/KeyPackageRepository.kt b/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/keypackage/KeyPackageRepository.kt index a4c7ea654dc..92f93550534 100644 --- a/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/keypackage/KeyPackageRepository.kt +++ b/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/keypackage/KeyPackageRepository.kt @@ -23,10 +23,8 @@ import com.wire.kalium.logic.NetworkFailure import com.wire.kalium.logic.data.client.MLSClientProvider import com.wire.kalium.logic.data.conversation.ClientId import com.wire.kalium.logic.data.id.CurrentClientIdProvider -import com.wire.kalium.logic.data.id.IdMapper import com.wire.kalium.logic.data.id.toApi import com.wire.kalium.logic.data.user.UserId -import com.wire.kalium.logic.di.MapperProvider import com.wire.kalium.logic.functional.Either import com.wire.kalium.logic.functional.flatMap import com.wire.kalium.logic.functional.fold @@ -39,7 +37,7 @@ import io.ktor.util.encodeBase64 interface KeyPackageRepository { - suspend fun claimKeyPackages(userIds: List): Either> + suspend fun claimKeyPackages(userIds: List): Either suspend fun uploadNewKeyPackages(clientId: ClientId, amount: Int = 100): Either @@ -58,10 +56,9 @@ class KeyPackageDataSource( private val keyPackageApi: KeyPackageApi, private val mlsClientProvider: MLSClientProvider, private val selfUserId: UserId, - private val idMapper: IdMapper = MapperProvider.idMapper(), ) : KeyPackageRepository { - override suspend fun claimKeyPackages(userIds: List): Either> = + override suspend fun claimKeyPackages(userIds: List): Either = currentClientIdProvider().flatMap { selfClientId -> val failedUsers = mutableSetOf() val keyPackages = mutableListOf() @@ -77,10 +74,10 @@ class KeyPackageDataSource( } } - if (failedUsers.isNotEmpty()) { + if (keyPackages.isEmpty()) { Either.Left(CoreFailure.NoKeyPackagesAvailable(failedUsers)) } else { - Either.Right(keyPackages) + Either.Right(ClaimedKeyPackages(keyPackages, failedUsers)) } } diff --git a/logic/src/commonTest/kotlin/com/wire/kalium/logic/data/conversation/MLSConversationRepositoryTest.kt b/logic/src/commonTest/kotlin/com/wire/kalium/logic/data/conversation/MLSConversationRepositoryTest.kt index ffaab33a469..a1c05f94a11 100644 --- a/logic/src/commonTest/kotlin/com/wire/kalium/logic/data/conversation/MLSConversationRepositoryTest.kt +++ b/logic/src/commonTest/kotlin/com/wire/kalium/logic/data/conversation/MLSConversationRepositoryTest.kt @@ -36,6 +36,7 @@ import com.wire.kalium.logic.data.conversation.MLSConversationRepositoryTest.Arr import com.wire.kalium.logic.data.event.Event import com.wire.kalium.logic.data.id.GroupID import com.wire.kalium.logic.data.id.QualifiedClientID +import com.wire.kalium.logic.data.keypackage.ClaimedKeyPackages import com.wire.kalium.logic.data.keypackage.KeyPackageRepository import com.wire.kalium.logic.data.mlspublickeys.Ed25519Key import com.wire.kalium.logic.data.mlspublickeys.KeyType @@ -1388,7 +1389,7 @@ class MLSConversationRepositoryTest { given(keyPackageRepository) .suspendFunction(keyPackageRepository::claimKeyPackages) .whenInvokedWith(anything()) - .then { Either.Right(keyPackages) } + .then { Either.Right(ClaimedKeyPackages(keyPackages, emptySet())) } } fun withUploadKeyPackagesReturning(result: Either) = apply { diff --git a/logic/src/commonTest/kotlin/com/wire/kalium/logic/data/keypackage/KeyPackageRepositoryTest.kt b/logic/src/commonTest/kotlin/com/wire/kalium/logic/data/keypackage/KeyPackageRepositoryTest.kt index c01e172c448..8afa7cc8206 100644 --- a/logic/src/commonTest/kotlin/com/wire/kalium/logic/data/keypackage/KeyPackageRepositoryTest.kt +++ b/logic/src/commonTest/kotlin/com/wire/kalium/logic/data/keypackage/KeyPackageRepositoryTest.kt @@ -94,8 +94,8 @@ class KeyPackageRepositoryTest { val result = keyPackageRepository.claimKeyPackages(listOf(Arrangement.USER_ID)) - result.shouldSucceed { keyPackages -> - assertEquals(listOf(Arrangement.CLAIMED_KEY_PACKAGES.keyPackages[0]), keyPackages) + result.shouldSucceed { claimedKeyPackages -> + assertEquals(listOf(Arrangement.CLAIMED_KEY_PACKAGES.keyPackages[0]), claimedKeyPackages.keyPackages) } } @@ -124,8 +124,8 @@ class KeyPackageRepositoryTest { val result = keyPackageRepository.claimKeyPackages(listOf(Arrangement.SELF_USER_ID)) - result.shouldSucceed() { keyPackages -> - assertEquals(emptyList(), keyPackages) + result.shouldSucceed { claimedKeyPackages -> + assertEquals(emptyList(), claimedKeyPackages.keyPackages) } } From 35f3a5c665094c97737f66e8b8cc6c03668ca015 Mon Sep 17 00:00:00 2001 From: yamilmedina Date: Mon, 20 Nov 2023 15:36:10 +0100 Subject: [PATCH 08/30] feat: wrap result into dto to avoid calling api twice --- .../wire/kalium/logic/data/keypackage/KeyPackageRepository.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/keypackage/KeyPackageRepository.kt b/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/keypackage/KeyPackageRepository.kt index 92f93550534..f3147ccdffc 100644 --- a/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/keypackage/KeyPackageRepository.kt +++ b/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/keypackage/KeyPackageRepository.kt @@ -74,7 +74,7 @@ class KeyPackageDataSource( } } - if (keyPackages.isEmpty()) { + if (keyPackages.isEmpty() && failedUsers.isNotEmpty()) { Either.Left(CoreFailure.NoKeyPackagesAvailable(failedUsers)) } else { Either.Right(ClaimedKeyPackages(keyPackages, failedUsers)) From 7c251b184ff4931dfd733b4c3d30b584ee6b41d1 Mon Sep 17 00:00:00 2001 From: yamilmedina Date: Mon, 20 Nov 2023 16:47:15 +0100 Subject: [PATCH 09/30] feat: renaming var --- .../logic/data/conversation/MLSConversationRepository.kt | 6 ++++-- .../kalium/logic/data/keypackage/KeyPackageRepository.kt | 3 ++- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/conversation/MLSConversationRepository.kt b/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/conversation/MLSConversationRepository.kt index 1e732d055f6..3ee380e03f4 100644 --- a/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/conversation/MLSConversationRepository.kt +++ b/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/conversation/MLSConversationRepository.kt @@ -399,9 +399,11 @@ internal class MLSConversationDataSource( ): Either = withContext(serialDispatcher) { commitPendingProposals(groupID).flatMap { retryOnCommitFailure(groupID, retryOnStaleMessage = retryOnStaleMessage) { - keyPackageRepository.claimKeyPackages(userIdList).flatMap { keyPackages -> + keyPackageRepository.claimKeyPackages(userIdList).flatMap { claimedKeyPackages -> + // if (claimedKeyPackages.failedUsers.isNotEmpty()) + mlsClientProvider.getMLSClient().flatMap { mlsClient -> - val clientKeyPackageList = keyPackages.keyPackages + val clientKeyPackageList = claimedKeyPackages.keyPackages .map { Pair( CryptoQualifiedClientId(it.clientID, CryptoQualifiedID(it.userId, it.domain)), diff --git a/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/keypackage/KeyPackageRepository.kt b/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/keypackage/KeyPackageRepository.kt index f3147ccdffc..32ba1629f5f 100644 --- a/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/keypackage/KeyPackageRepository.kt +++ b/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/keypackage/KeyPackageRepository.kt @@ -74,7 +74,8 @@ class KeyPackageDataSource( } } - if (keyPackages.isEmpty() && failedUsers.isNotEmpty()) { + val noKeyPackagesForOtherUsers = keyPackages.isEmpty() && failedUsers.isNotEmpty() + if (noKeyPackagesForOtherUsers) { Either.Left(CoreFailure.NoKeyPackagesAvailable(failedUsers)) } else { Either.Right(ClaimedKeyPackages(keyPackages, failedUsers)) From ee86ccf0df771be642c1044a858085862defc337 Mon Sep 17 00:00:00 2001 From: yamilmedina Date: Mon, 20 Nov 2023 16:54:32 +0100 Subject: [PATCH 10/30] feat: renaming var, comments --- .../logic/data/conversation/MLSConversationRepository.kt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/conversation/MLSConversationRepository.kt b/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/conversation/MLSConversationRepository.kt index 3ee380e03f4..4e88baea20f 100644 --- a/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/conversation/MLSConversationRepository.kt +++ b/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/conversation/MLSConversationRepository.kt @@ -400,7 +400,8 @@ internal class MLSConversationDataSource( commitPendingProposals(groupID).flatMap { retryOnCommitFailure(groupID, retryOnStaleMessage = retryOnStaleMessage) { keyPackageRepository.claimKeyPackages(userIdList).flatMap { claimedKeyPackages -> - // if (claimedKeyPackages.failedUsers.isNotEmpty()) + // todo. acc and bubble up the failed users if they are. + val failedUsers = claimedKeyPackages.failedUsers mlsClientProvider.getMLSClient().flatMap { mlsClient -> val clientKeyPackageList = claimedKeyPackages.keyPackages From 039862745ba0ae654e94b4f43c2c001e561a09cb Mon Sep 17 00:00:00 2001 From: yamilmedina Date: Wed, 22 Nov 2023 13:05:47 +0100 Subject: [PATCH 11/30] feat: preparing adding members for retry --- .../kalium/network/api/v5/authenticated/KeyPackageApiV5.kt | 2 +- .../kalium/network/api/v5/authenticated/MLSMessageApiV5.kt | 6 +++++- .../kotlin/com/wire/kalium/network/utils/NetworkUtils.kt | 2 ++ 3 files changed, 8 insertions(+), 2 deletions(-) diff --git a/network/src/commonMain/kotlin/com/wire/kalium/network/api/v5/authenticated/KeyPackageApiV5.kt b/network/src/commonMain/kotlin/com/wire/kalium/network/api/v5/authenticated/KeyPackageApiV5.kt index b6b1d0eb7f1..c3d2d6be931 100644 --- a/network/src/commonMain/kotlin/com/wire/kalium/network/api/v5/authenticated/KeyPackageApiV5.kt +++ b/network/src/commonMain/kotlin/com/wire/kalium/network/api/v5/authenticated/KeyPackageApiV5.kt @@ -44,7 +44,7 @@ internal open class KeyPackageApiV5 internal constructor( override suspend fun claimKeyPackages( param: KeyPackageApi.Param ): NetworkResponse = wrapKaliumResponse(unsuccessfulResponseOverride = { response -> - wrapFederationResponse(response) { handleUnsuccessfulResponse(response) } + wrapFederationResponse(response, delegatedHandler = { handleUnsuccessfulResponse(response) }) }) { httpClient.post("$PATH_KEY_PACKAGES/$PATH_CLAIM/${param.user.domain}/${param.user.value}") { if (param is KeyPackageApi.Param.SkipOwnClient) { diff --git a/network/src/commonMain/kotlin/com/wire/kalium/network/api/v5/authenticated/MLSMessageApiV5.kt b/network/src/commonMain/kotlin/com/wire/kalium/network/api/v5/authenticated/MLSMessageApiV5.kt index 9ca9a119a3e..e7293351628 100644 --- a/network/src/commonMain/kotlin/com/wire/kalium/network/api/v5/authenticated/MLSMessageApiV5.kt +++ b/network/src/commonMain/kotlin/com/wire/kalium/network/api/v5/authenticated/MLSMessageApiV5.kt @@ -24,6 +24,8 @@ import com.wire.kalium.network.api.base.authenticated.message.SendMLSMessageResp import com.wire.kalium.network.api.v4.authenticated.MLSMessageApiV4 import com.wire.kalium.network.serialization.Mls import com.wire.kalium.network.utils.NetworkResponse +import com.wire.kalium.network.utils.handleUnsuccessfulResponse +import com.wire.kalium.network.utils.wrapFederationResponse import com.wire.kalium.network.utils.wrapKaliumResponse import io.ktor.client.request.post import io.ktor.client.request.setBody @@ -44,7 +46,9 @@ internal open class MLSMessageApiV5 internal constructor( } override suspend fun sendCommitBundle(bundle: MLSMessageApi.CommitBundle): NetworkResponse = - wrapKaliumResponse { + wrapKaliumResponse(unsuccessfulResponseOverride = { response -> + wrapFederationResponse(response, delegatedHandler = { handleUnsuccessfulResponse(response) }) + }) { httpClient.post(PATH_COMMIT_BUNDLES) { setBody(bundle.value) contentType(ContentType.Message.Mls) diff --git a/network/src/commonMain/kotlin/com/wire/kalium/network/utils/NetworkUtils.kt b/network/src/commonMain/kotlin/com/wire/kalium/network/utils/NetworkUtils.kt index 27f2e802912..a91ed94a815 100644 --- a/network/src/commonMain/kotlin/com/wire/kalium/network/utils/NetworkUtils.kt +++ b/network/src/commonMain/kotlin/com/wire/kalium/network/utils/NetworkUtils.kt @@ -238,6 +238,8 @@ private fun toStatusCodeBasedKaliumException( /** * Wrap and handles federation aware endpoints that can send errors responses * And raise proper federated exceptions + * + * @delegatedHandler the fallback handler when the response is not a federation error */ suspend fun wrapFederationResponse( response: HttpResponse, From a64e2f32290f24dbb7a933c9fdc582c9dc252bb7 Mon Sep 17 00:00:00 2001 From: yamilmedina Date: Wed, 22 Nov 2023 13:37:59 +0100 Subject: [PATCH 12/30] feat: preparing adding members for retry --- .../com/wire/kalium/logic/CoreFailure.kt | 13 ++++++++-- .../ConversationGroupRepository.kt | 5 ++-- .../conversation/MLSConversationRepository.kt | 7 ++--- .../data/keypackage/ClaimedKeyPackages.kt | 26 ------------------- .../data/keypackage/KeyPackageRepository.kt | 25 +++++++++++------- .../MLSConversationRepositoryTest.kt | 3 +-- .../keypackage/KeyPackageRepositoryTest.kt | 8 +++--- 7 files changed, 37 insertions(+), 50 deletions(-) delete mode 100644 logic/src/commonMain/kotlin/com/wire/kalium/logic/data/keypackage/ClaimedKeyPackages.kt diff --git a/logic/src/commonMain/kotlin/com/wire/kalium/logic/CoreFailure.kt b/logic/src/commonMain/kotlin/com/wire/kalium/logic/CoreFailure.kt index ad173b1795d..dacf80866d0 100644 --- a/logic/src/commonMain/kotlin/com/wire/kalium/logic/CoreFailure.kt +++ b/logic/src/commonMain/kotlin/com/wire/kalium/logic/CoreFailure.kt @@ -22,6 +22,7 @@ import com.wire.kalium.cryptography.exceptions.ProteusException import com.wire.kalium.logic.data.user.UserId import com.wire.kalium.logic.feature.e2ei.usecase.E2EIEnrollmentResult import com.wire.kalium.logic.functional.Either +import com.wire.kalium.network.api.base.authenticated.keypackage.KeyPackageDTO import com.wire.kalium.network.exceptions.APINotSupported import com.wire.kalium.network.exceptions.KaliumException import com.wire.kalium.network.exceptions.isFederationDenied @@ -54,10 +55,18 @@ sealed interface CoreFailure { data object MissingClientRegistration : CoreFailure /** - * A user has no key packages available which prevents him/her from being added + * One or All users has no key packages available which prevents him/her from being added * to an existing or new conversation. */ - data class NoKeyPackagesAvailable(val userId: Set) : CoreFailure + interface NoKeyPackagesAvailable : CoreFailure { + val failedUserIds: Set + } + + data class CompleteKeyPackagesUnAvailable(override val failedUserIds: Set) : NoKeyPackagesAvailable + data class PartialKeyPackagesUnAvailable( + override val failedUserIds: Set, + val claimedKeyPackages: List + ) : NoKeyPackagesAvailable /** * It's not allowed to run the application with development API enabled when diff --git a/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/conversation/ConversationGroupRepository.kt b/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/conversation/ConversationGroupRepository.kt index 3c42cc62ab1..d243a17f8fb 100644 --- a/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/conversation/ConversationGroupRepository.kt +++ b/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/conversation/ConversationGroupRepository.kt @@ -206,8 +206,9 @@ internal class ConversationGroupRepositoryImpl( return when (val addingMemberResult = mlsConversationRepository.addMemberToMLSGroup(GroupID(groupId), userIdList)) { is Either.Right -> addingMemberResult is Either.Left -> { - - TODO("retry for claimPackages and offline sendCommit cases") + if (addingMemberResult.value.isRetryable && addingMemberResult.value.hasUnreachableDomainsError) + (addingMemberResult.value as NetworkFailure.FederatedBackendFailure.RetryableFailure).domains + TODO() // do retry stuff } } } diff --git a/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/conversation/MLSConversationRepository.kt b/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/conversation/MLSConversationRepository.kt index 4e88baea20f..fadda7f9584 100644 --- a/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/conversation/MLSConversationRepository.kt +++ b/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/conversation/MLSConversationRepository.kt @@ -399,12 +399,9 @@ internal class MLSConversationDataSource( ): Either = withContext(serialDispatcher) { commitPendingProposals(groupID).flatMap { retryOnCommitFailure(groupID, retryOnStaleMessage = retryOnStaleMessage) { - keyPackageRepository.claimKeyPackages(userIdList).flatMap { claimedKeyPackages -> - // todo. acc and bubble up the failed users if they are. - val failedUsers = claimedKeyPackages.failedUsers - + keyPackageRepository.claimKeyPackages(userIdList).flatMap { keyPackages -> mlsClientProvider.getMLSClient().flatMap { mlsClient -> - val clientKeyPackageList = claimedKeyPackages.keyPackages + val clientKeyPackageList = keyPackages .map { Pair( CryptoQualifiedClientId(it.clientID, CryptoQualifiedID(it.userId, it.domain)), diff --git a/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/keypackage/ClaimedKeyPackages.kt b/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/keypackage/ClaimedKeyPackages.kt deleted file mode 100644 index 05675517e95..00000000000 --- a/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/keypackage/ClaimedKeyPackages.kt +++ /dev/null @@ -1,26 +0,0 @@ -/* - * Wire - * Copyright (C) 2023 Wire Swiss GmbH - * - * This program is free software: you can redistribute it and/or modify - * it under the terms of the GNU General Public License as published by - * the Free Software Foundation, either version 3 of the License, or - * (at your option) any later version. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU General Public License for more details. - * - * You should have received a copy of the GNU General Public License - * along with this program. If not, see http://www.gnu.org/licenses/. - */ -package com.wire.kalium.logic.data.keypackage - -import com.wire.kalium.logic.data.user.UserId -import com.wire.kalium.network.api.base.authenticated.keypackage.KeyPackageDTO - -data class ClaimedKeyPackages( - val keyPackages: List, - val failedUsers: Set -) diff --git a/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/keypackage/KeyPackageRepository.kt b/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/keypackage/KeyPackageRepository.kt index 32ba1629f5f..84d83ae0cb6 100644 --- a/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/keypackage/KeyPackageRepository.kt +++ b/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/keypackage/KeyPackageRepository.kt @@ -37,7 +37,7 @@ import io.ktor.util.encodeBase64 interface KeyPackageRepository { - suspend fun claimKeyPackages(userIds: List): Either + suspend fun claimKeyPackages(userIds: List): Either> suspend fun uploadNewKeyPackages(clientId: ClientId, amount: Int = 100): Either @@ -58,10 +58,10 @@ class KeyPackageDataSource( private val selfUserId: UserId, ) : KeyPackageRepository { - override suspend fun claimKeyPackages(userIds: List): Either = + override suspend fun claimKeyPackages(userIds: List): Either> = currentClientIdProvider().flatMap { selfClientId -> val failedUsers = mutableSetOf() - val keyPackages = mutableListOf() + val claimedKeyPackages = mutableListOf() userIds.forEach { userId -> wrapApiRequest { keyPackageApi.claimKeyPackages(KeyPackageApi.Param.SkipOwnClient(userId.toApi(), selfClientId.value)) @@ -69,16 +69,23 @@ class KeyPackageDataSource( if (it.keyPackages.isEmpty() && userId != selfUserId) { failedUsers.add(userId) } else { - keyPackages.addAll(it.keyPackages) + claimedKeyPackages.addAll(it.keyPackages) } } } - val noKeyPackagesForOtherUsers = keyPackages.isEmpty() && failedUsers.isNotEmpty() - if (noKeyPackagesForOtherUsers) { - Either.Left(CoreFailure.NoKeyPackagesAvailable(failedUsers)) - } else { - Either.Right(ClaimedKeyPackages(keyPackages, failedUsers)) + when { + claimedKeyPackages.isEmpty() && failedUsers.isNotEmpty() -> { + Either.Left(CoreFailure.CompleteKeyPackagesUnAvailable(failedUsers)) + } + + claimedKeyPackages.isNotEmpty() && failedUsers.isNotEmpty() -> { + Either.Left(CoreFailure.PartialKeyPackagesUnAvailable(failedUsers, claimedKeyPackages)) + } + + else -> { + Either.Right(claimedKeyPackages) + } } } diff --git a/logic/src/commonTest/kotlin/com/wire/kalium/logic/data/conversation/MLSConversationRepositoryTest.kt b/logic/src/commonTest/kotlin/com/wire/kalium/logic/data/conversation/MLSConversationRepositoryTest.kt index a1c05f94a11..ffaab33a469 100644 --- a/logic/src/commonTest/kotlin/com/wire/kalium/logic/data/conversation/MLSConversationRepositoryTest.kt +++ b/logic/src/commonTest/kotlin/com/wire/kalium/logic/data/conversation/MLSConversationRepositoryTest.kt @@ -36,7 +36,6 @@ import com.wire.kalium.logic.data.conversation.MLSConversationRepositoryTest.Arr import com.wire.kalium.logic.data.event.Event import com.wire.kalium.logic.data.id.GroupID import com.wire.kalium.logic.data.id.QualifiedClientID -import com.wire.kalium.logic.data.keypackage.ClaimedKeyPackages import com.wire.kalium.logic.data.keypackage.KeyPackageRepository import com.wire.kalium.logic.data.mlspublickeys.Ed25519Key import com.wire.kalium.logic.data.mlspublickeys.KeyType @@ -1389,7 +1388,7 @@ class MLSConversationRepositoryTest { given(keyPackageRepository) .suspendFunction(keyPackageRepository::claimKeyPackages) .whenInvokedWith(anything()) - .then { Either.Right(ClaimedKeyPackages(keyPackages, emptySet())) } + .then { Either.Right(keyPackages) } } fun withUploadKeyPackagesReturning(result: Either) = apply { diff --git a/logic/src/commonTest/kotlin/com/wire/kalium/logic/data/keypackage/KeyPackageRepositoryTest.kt b/logic/src/commonTest/kotlin/com/wire/kalium/logic/data/keypackage/KeyPackageRepositoryTest.kt index 8afa7cc8206..c5620d7c2ee 100644 --- a/logic/src/commonTest/kotlin/com/wire/kalium/logic/data/keypackage/KeyPackageRepositoryTest.kt +++ b/logic/src/commonTest/kotlin/com/wire/kalium/logic/data/keypackage/KeyPackageRepositoryTest.kt @@ -22,10 +22,10 @@ import com.wire.kalium.cryptography.MLSClient import com.wire.kalium.logic.CoreFailure import com.wire.kalium.logic.data.client.MLSClientProvider import com.wire.kalium.logic.data.conversation.ClientId +import com.wire.kalium.logic.data.id.CurrentClientIdProvider import com.wire.kalium.logic.data.id.PlainId import com.wire.kalium.logic.data.id.toApi import com.wire.kalium.logic.data.user.UserId -import com.wire.kalium.logic.data.id.CurrentClientIdProvider import com.wire.kalium.logic.functional.Either import com.wire.kalium.logic.util.shouldFail import com.wire.kalium.logic.util.shouldSucceed @@ -95,7 +95,7 @@ class KeyPackageRepositoryTest { val result = keyPackageRepository.claimKeyPackages(listOf(Arrangement.USER_ID)) result.shouldSucceed { claimedKeyPackages -> - assertEquals(listOf(Arrangement.CLAIMED_KEY_PACKAGES.keyPackages[0]), claimedKeyPackages.keyPackages) + assertEquals(listOf(Arrangement.CLAIMED_KEY_PACKAGES.keyPackages[0]), claimedKeyPackages) } } @@ -110,7 +110,7 @@ class KeyPackageRepositoryTest { val result = keyPackageRepository.claimKeyPackages(listOf(Arrangement.USER_ID)) result.shouldFail { failure -> - assertEquals(CoreFailure.NoKeyPackagesAvailable(setOf(Arrangement.USER_ID)), failure) + assertEquals(CoreFailure.CompleteKeyPackagesUnAvailable(setOf(Arrangement.USER_ID)), failure) } } @@ -125,7 +125,7 @@ class KeyPackageRepositoryTest { val result = keyPackageRepository.claimKeyPackages(listOf(Arrangement.SELF_USER_ID)) result.shouldSucceed { claimedKeyPackages -> - assertEquals(emptyList(), claimedKeyPackages.keyPackages) + assertEquals(emptyList(), claimedKeyPackages) } } From 69b3c35b0e537d66dc08669e4ef7056bb0e20d06 Mon Sep 17 00:00:00 2001 From: yamilmedina Date: Wed, 22 Nov 2023 17:48:55 +0100 Subject: [PATCH 13/30] feat: add member logic receive preclaimed packages --- .../com/wire/kalium/logic/CoreFailure.kt | 13 ++++++++++-- .../ConversationGroupRepository.kt | 17 ++++++++++----- .../conversation/MLSConversationRepository.kt | 21 ++++++++++++++----- 3 files changed, 39 insertions(+), 12 deletions(-) diff --git a/logic/src/commonMain/kotlin/com/wire/kalium/logic/CoreFailure.kt b/logic/src/commonMain/kotlin/com/wire/kalium/logic/CoreFailure.kt index dacf80866d0..66b80ec568a 100644 --- a/logic/src/commonMain/kotlin/com/wire/kalium/logic/CoreFailure.kt +++ b/logic/src/commonMain/kotlin/com/wire/kalium/logic/CoreFailure.kt @@ -55,17 +55,26 @@ sealed interface CoreFailure { data object MissingClientRegistration : CoreFailure /** - * One or All users has no key packages available which prevents him/her from being added + * Key packages requested not available which prevents them from being added * to an existing or new conversation. */ interface NoKeyPackagesAvailable : CoreFailure { val failedUserIds: Set } + /** + * All users have no key packages available which prevents him/her from being added + * to an existing or new conversation. + */ data class CompleteKeyPackagesUnAvailable(override val failedUserIds: Set) : NoKeyPackagesAvailable + + /** + * Some users have no key packages available which prevents him/her from being added. + * Claimed key packages are + */ data class PartialKeyPackagesUnAvailable( override val failedUserIds: Set, - val claimedKeyPackages: List + val claimedKeyPackages: List // todo(enhancement): this be a list KeyPackage domain model. ) : NoKeyPackagesAvailable /** diff --git a/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/conversation/ConversationGroupRepository.kt b/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/conversation/ConversationGroupRepository.kt index d243a17f8fb..50af7795e0c 100644 --- a/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/conversation/ConversationGroupRepository.kt +++ b/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/conversation/ConversationGroupRepository.kt @@ -200,15 +200,22 @@ internal class ConversationGroupRepositoryImpl( */ private suspend fun tryAddMembersToMLSGroup( groupId: String, - userIdList: List, - failedUsersList: Set = emptySet() + userIdList: List ): Either { return when (val addingMemberResult = mlsConversationRepository.addMemberToMLSGroup(GroupID(groupId), userIdList)) { is Either.Right -> addingMemberResult is Either.Left -> { - if (addingMemberResult.value.isRetryable && addingMemberResult.value.hasUnreachableDomainsError) - (addingMemberResult.value as NetworkFailure.FederatedBackendFailure.RetryableFailure).domains - TODO() // do retry stuff + when (val failure = addingMemberResult.value) { + is NetworkFailure.FederatedBackendFailure.RetryableFailure -> { + mlsConversationRepository.addMemberToMLSGroup(GroupID(groupId), userIdList) + } + + is CoreFailure.PartialKeyPackagesUnAvailable -> { + mlsConversationRepository.addMemberToMLSGroup(GroupID(groupId), userIdList, failure.claimedKeyPackages) + } + + else -> addingMemberResult + } } } } diff --git a/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/conversation/MLSConversationRepository.kt b/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/conversation/MLSConversationRepository.kt index fadda7f9584..4a334bb30f4 100644 --- a/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/conversation/MLSConversationRepository.kt +++ b/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/conversation/MLSConversationRepository.kt @@ -57,6 +57,7 @@ import com.wire.kalium.logic.wrapApiRequest import com.wire.kalium.logic.wrapMLSRequest import com.wire.kalium.logic.wrapStorageRequest import com.wire.kalium.network.api.base.authenticated.client.ClientApi +import com.wire.kalium.network.api.base.authenticated.keypackage.KeyPackageDTO import com.wire.kalium.network.api.base.authenticated.message.MLSMessageApi import com.wire.kalium.network.api.base.authenticated.notification.EventContentDTO import com.wire.kalium.network.exceptions.KaliumException @@ -99,7 +100,12 @@ interface MLSConversationRepository { suspend fun establishMLSGroup(groupID: GroupID, members: List): Either suspend fun establishMLSGroupFromWelcome(welcomeEvent: MLSWelcome): Either suspend fun hasEstablishedMLSGroup(groupID: GroupID): Either - suspend fun addMemberToMLSGroup(groupID: GroupID, userIdList: List): Either + suspend fun addMemberToMLSGroup( + groupID: GroupID, + userIdList: List, + preClaimedKeysList: List = emptyList() + ): Either + suspend fun removeMembersFromMLSGroup(groupID: GroupID, userIdList: List): Either suspend fun removeClientsFromMLSGroup(groupID: GroupID, clientIdList: List): Either suspend fun leaveGroup(groupID: GroupID): Either @@ -389,19 +395,24 @@ internal class MLSConversationDataSource( return epochsFlow } - override suspend fun addMemberToMLSGroup(groupID: GroupID, userIdList: List): Either = - internalAddMemberToMLSGroup(groupID, userIdList, retryOnStaleMessage = true) + override suspend fun addMemberToMLSGroup( + groupID: GroupID, + userIdList: List, + preClaimedKeysList: List + ): Either = + internalAddMemberToMLSGroup(groupID, userIdList, preClaimedKeysList, retryOnStaleMessage = true) private suspend fun internalAddMemberToMLSGroup( groupID: GroupID, userIdList: List, + preClaimedKeysList: List, retryOnStaleMessage: Boolean ): Either = withContext(serialDispatcher) { commitPendingProposals(groupID).flatMap { retryOnCommitFailure(groupID, retryOnStaleMessage = retryOnStaleMessage) { keyPackageRepository.claimKeyPackages(userIdList).flatMap { keyPackages -> mlsClientProvider.getMLSClient().flatMap { mlsClient -> - val clientKeyPackageList = keyPackages + val clientKeyPackageList = (keyPackages + preClaimedKeysList).toSet() .map { Pair( CryptoQualifiedClientId(it.clientID, CryptoQualifiedID(it.userId, it.domain)), @@ -505,7 +516,7 @@ internal class MLSConversationDataSource( } } }.flatMap { - internalAddMemberToMLSGroup(groupID, members, retryOnStaleMessage = false).onFailure { + internalAddMemberToMLSGroup(groupID, members, emptyList(), retryOnStaleMessage = false).onFailure { wrapMLSRequest { mlsClient.wipeConversation(groupID.toCrypto()) } From 4e7dc756f97dc378d05ca24c25c2b43654d233a4 Mon Sep 17 00:00:00 2001 From: yamilmedina Date: Thu, 23 Nov 2023 11:44:01 +0100 Subject: [PATCH 14/30] chore: add base for simpler version of failure, no retain preclaimed --- .../com/wire/kalium/logic/CoreFailure.kt | 20 +------------------ .../ConversationGroupRepository.kt | 6 ++++-- .../conversation/MLSConversationRepository.kt | 20 +++++-------------- .../data/keypackage/KeyPackageRepository.kt | 16 ++++----------- .../keypackage/KeyPackageRepositoryTest.kt | 2 +- 5 files changed, 15 insertions(+), 49 deletions(-) diff --git a/logic/src/commonMain/kotlin/com/wire/kalium/logic/CoreFailure.kt b/logic/src/commonMain/kotlin/com/wire/kalium/logic/CoreFailure.kt index 66b80ec568a..cc5838bc60f 100644 --- a/logic/src/commonMain/kotlin/com/wire/kalium/logic/CoreFailure.kt +++ b/logic/src/commonMain/kotlin/com/wire/kalium/logic/CoreFailure.kt @@ -22,7 +22,6 @@ import com.wire.kalium.cryptography.exceptions.ProteusException import com.wire.kalium.logic.data.user.UserId import com.wire.kalium.logic.feature.e2ei.usecase.E2EIEnrollmentResult import com.wire.kalium.logic.functional.Either -import com.wire.kalium.network.api.base.authenticated.keypackage.KeyPackageDTO import com.wire.kalium.network.exceptions.APINotSupported import com.wire.kalium.network.exceptions.KaliumException import com.wire.kalium.network.exceptions.isFederationDenied @@ -58,24 +57,7 @@ sealed interface CoreFailure { * Key packages requested not available which prevents them from being added * to an existing or new conversation. */ - interface NoKeyPackagesAvailable : CoreFailure { - val failedUserIds: Set - } - - /** - * All users have no key packages available which prevents him/her from being added - * to an existing or new conversation. - */ - data class CompleteKeyPackagesUnAvailable(override val failedUserIds: Set) : NoKeyPackagesAvailable - - /** - * Some users have no key packages available which prevents him/her from being added. - * Claimed key packages are - */ - data class PartialKeyPackagesUnAvailable( - override val failedUserIds: Set, - val claimedKeyPackages: List // todo(enhancement): this be a list KeyPackage domain model. - ) : NoKeyPackagesAvailable + data class NoKeyPackagesAvailable(val failedUserIds: Set) : CoreFailure /** * It's not allowed to run the application with development API enabled when diff --git a/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/conversation/ConversationGroupRepository.kt b/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/conversation/ConversationGroupRepository.kt index 50af7795e0c..51a7d4327c0 100644 --- a/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/conversation/ConversationGroupRepository.kt +++ b/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/conversation/ConversationGroupRepository.kt @@ -210,8 +210,10 @@ internal class ConversationGroupRepositoryImpl( mlsConversationRepository.addMemberToMLSGroup(GroupID(groupId), userIdList) } - is CoreFailure.PartialKeyPackagesUnAvailable -> { - mlsConversationRepository.addMemberToMLSGroup(GroupID(groupId), userIdList, failure.claimedKeyPackages) + is CoreFailure.NoKeyPackagesAvailable -> { + mlsConversationRepository.addMemberToMLSGroup( + GroupID(groupId), + userIdList.filterNot { failure.failedUserIds.contains(it) }) } else -> addingMemberResult diff --git a/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/conversation/MLSConversationRepository.kt b/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/conversation/MLSConversationRepository.kt index 4a334bb30f4..c174f339125 100644 --- a/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/conversation/MLSConversationRepository.kt +++ b/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/conversation/MLSConversationRepository.kt @@ -57,7 +57,6 @@ import com.wire.kalium.logic.wrapApiRequest import com.wire.kalium.logic.wrapMLSRequest import com.wire.kalium.logic.wrapStorageRequest import com.wire.kalium.network.api.base.authenticated.client.ClientApi -import com.wire.kalium.network.api.base.authenticated.keypackage.KeyPackageDTO import com.wire.kalium.network.api.base.authenticated.message.MLSMessageApi import com.wire.kalium.network.api.base.authenticated.notification.EventContentDTO import com.wire.kalium.network.exceptions.KaliumException @@ -100,11 +99,7 @@ interface MLSConversationRepository { suspend fun establishMLSGroup(groupID: GroupID, members: List): Either suspend fun establishMLSGroupFromWelcome(welcomeEvent: MLSWelcome): Either suspend fun hasEstablishedMLSGroup(groupID: GroupID): Either - suspend fun addMemberToMLSGroup( - groupID: GroupID, - userIdList: List, - preClaimedKeysList: List = emptyList() - ): Either + suspend fun addMemberToMLSGroup(groupID: GroupID, userIdList: List): Either suspend fun removeMembersFromMLSGroup(groupID: GroupID, userIdList: List): Either suspend fun removeClientsFromMLSGroup(groupID: GroupID, clientIdList: List): Either @@ -395,24 +390,19 @@ internal class MLSConversationDataSource( return epochsFlow } - override suspend fun addMemberToMLSGroup( - groupID: GroupID, - userIdList: List, - preClaimedKeysList: List - ): Either = - internalAddMemberToMLSGroup(groupID, userIdList, preClaimedKeysList, retryOnStaleMessage = true) + override suspend fun addMemberToMLSGroup(groupID: GroupID, userIdList: List): Either = + internalAddMemberToMLSGroup(groupID, userIdList, retryOnStaleMessage = true) private suspend fun internalAddMemberToMLSGroup( groupID: GroupID, userIdList: List, - preClaimedKeysList: List, retryOnStaleMessage: Boolean ): Either = withContext(serialDispatcher) { commitPendingProposals(groupID).flatMap { retryOnCommitFailure(groupID, retryOnStaleMessage = retryOnStaleMessage) { keyPackageRepository.claimKeyPackages(userIdList).flatMap { keyPackages -> mlsClientProvider.getMLSClient().flatMap { mlsClient -> - val clientKeyPackageList = (keyPackages + preClaimedKeysList).toSet() + val clientKeyPackageList = keyPackages .map { Pair( CryptoQualifiedClientId(it.clientID, CryptoQualifiedID(it.userId, it.domain)), @@ -516,7 +506,7 @@ internal class MLSConversationDataSource( } } }.flatMap { - internalAddMemberToMLSGroup(groupID, members, emptyList(), retryOnStaleMessage = false).onFailure { + internalAddMemberToMLSGroup(groupID, members, retryOnStaleMessage = false).onFailure { wrapMLSRequest { mlsClient.wipeConversation(groupID.toCrypto()) } diff --git a/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/keypackage/KeyPackageRepository.kt b/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/keypackage/KeyPackageRepository.kt index 84d83ae0cb6..8d43c97c818 100644 --- a/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/keypackage/KeyPackageRepository.kt +++ b/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/keypackage/KeyPackageRepository.kt @@ -74,18 +74,10 @@ class KeyPackageDataSource( } } - when { - claimedKeyPackages.isEmpty() && failedUsers.isNotEmpty() -> { - Either.Left(CoreFailure.CompleteKeyPackagesUnAvailable(failedUsers)) - } - - claimedKeyPackages.isNotEmpty() && failedUsers.isNotEmpty() -> { - Either.Left(CoreFailure.PartialKeyPackagesUnAvailable(failedUsers, claimedKeyPackages)) - } - - else -> { - Either.Right(claimedKeyPackages) - } + if (failedUsers.isNotEmpty()) { + Either.Left(CoreFailure.NoKeyPackagesAvailable(failedUsers)) + } else { + Either.Right(claimedKeyPackages) } } diff --git a/logic/src/commonTest/kotlin/com/wire/kalium/logic/data/keypackage/KeyPackageRepositoryTest.kt b/logic/src/commonTest/kotlin/com/wire/kalium/logic/data/keypackage/KeyPackageRepositoryTest.kt index c5620d7c2ee..12b45bf179b 100644 --- a/logic/src/commonTest/kotlin/com/wire/kalium/logic/data/keypackage/KeyPackageRepositoryTest.kt +++ b/logic/src/commonTest/kotlin/com/wire/kalium/logic/data/keypackage/KeyPackageRepositoryTest.kt @@ -110,7 +110,7 @@ class KeyPackageRepositoryTest { val result = keyPackageRepository.claimKeyPackages(listOf(Arrangement.USER_ID)) result.shouldFail { failure -> - assertEquals(CoreFailure.CompleteKeyPackagesUnAvailable(setOf(Arrangement.USER_ID)), failure) + assertEquals(CoreFailure.NoKeyPackagesAvailable(setOf(Arrangement.USER_ID)), failure) } } From 9ed032869d82874eaa13b82aabb2e14dd4c8a9df Mon Sep 17 00:00:00 2001 From: yamilmedina Date: Thu, 23 Nov 2023 11:54:14 +0100 Subject: [PATCH 15/30] chore: filtering failed users --- .../data/conversation/ConversationGroupRepository.kt | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/conversation/ConversationGroupRepository.kt b/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/conversation/ConversationGroupRepository.kt index 51a7d4327c0..7937253e509 100644 --- a/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/conversation/ConversationGroupRepository.kt +++ b/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/conversation/ConversationGroupRepository.kt @@ -206,16 +206,20 @@ internal class ConversationGroupRepositoryImpl( is Either.Right -> addingMemberResult is Either.Left -> { when (val failure = addingMemberResult.value) { - is NetworkFailure.FederatedBackendFailure.RetryableFailure -> { - mlsConversationRepository.addMemberToMLSGroup(GroupID(groupId), userIdList) - } - + // claiming key packages offline or out of packages is CoreFailure.NoKeyPackagesAvailable -> { mlsConversationRepository.addMemberToMLSGroup( GroupID(groupId), userIdList.filterNot { failure.failedUserIds.contains(it) }) } + // sending commit unreachable + is NetworkFailure.FederatedBackendFailure.RetryableFailure -> { + mlsConversationRepository.addMemberToMLSGroup(GroupID(groupId), userIdList.filterNot { + !failure.domains.contains(it.domain) + }) + } + else -> addingMemberResult } } From ea2bf6d30f7fbabeb718d052cb89d478de295bfe Mon Sep 17 00:00:00 2001 From: yamilmedina Date: Thu, 23 Nov 2023 11:55:45 +0100 Subject: [PATCH 16/30] chore: filtering failed users --- .../logic/data/conversation/MLSConversationRepository.kt | 1 - .../logic/data/keypackage/KeyPackageRepositoryTest.kt | 8 ++++---- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/conversation/MLSConversationRepository.kt b/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/conversation/MLSConversationRepository.kt index c174f339125..fadda7f9584 100644 --- a/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/conversation/MLSConversationRepository.kt +++ b/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/conversation/MLSConversationRepository.kt @@ -100,7 +100,6 @@ interface MLSConversationRepository { suspend fun establishMLSGroupFromWelcome(welcomeEvent: MLSWelcome): Either suspend fun hasEstablishedMLSGroup(groupID: GroupID): Either suspend fun addMemberToMLSGroup(groupID: GroupID, userIdList: List): Either - suspend fun removeMembersFromMLSGroup(groupID: GroupID, userIdList: List): Either suspend fun removeClientsFromMLSGroup(groupID: GroupID, clientIdList: List): Either suspend fun leaveGroup(groupID: GroupID): Either diff --git a/logic/src/commonTest/kotlin/com/wire/kalium/logic/data/keypackage/KeyPackageRepositoryTest.kt b/logic/src/commonTest/kotlin/com/wire/kalium/logic/data/keypackage/KeyPackageRepositoryTest.kt index 12b45bf179b..5c44ef51717 100644 --- a/logic/src/commonTest/kotlin/com/wire/kalium/logic/data/keypackage/KeyPackageRepositoryTest.kt +++ b/logic/src/commonTest/kotlin/com/wire/kalium/logic/data/keypackage/KeyPackageRepositoryTest.kt @@ -94,8 +94,8 @@ class KeyPackageRepositoryTest { val result = keyPackageRepository.claimKeyPackages(listOf(Arrangement.USER_ID)) - result.shouldSucceed { claimedKeyPackages -> - assertEquals(listOf(Arrangement.CLAIMED_KEY_PACKAGES.keyPackages[0]), claimedKeyPackages) + result.shouldSucceed { keyPackages -> + assertEquals(listOf(Arrangement.CLAIMED_KEY_PACKAGES.keyPackages[0]), keyPackages) } } @@ -124,8 +124,8 @@ class KeyPackageRepositoryTest { val result = keyPackageRepository.claimKeyPackages(listOf(Arrangement.SELF_USER_ID)) - result.shouldSucceed { claimedKeyPackages -> - assertEquals(emptyList(), claimedKeyPackages) + result.shouldSucceed { keyPackages -> + assertEquals(emptyList(), keyPackages) } } From f407acae7731a82f7779eb9c2ee0a1625ae1b905 Mon Sep 17 00:00:00 2001 From: yamilmedina Date: Tue, 28 Nov 2023 18:08:31 +0100 Subject: [PATCH 17/30] feat: persist system messages for failed and added, and handle retry --- .../ConversationGroupRepository.kt | 73 ++++++++++++++----- .../NewConversationMembersRepository.kt | 5 +- ...wGroupConversationSystemMessagesCreator.kt | 48 ++++++------ .../NewConversationEventHandler.kt | 2 +- ...upConversationSystemMessagesCreatorTest.kt | 17 +++-- .../NewConversationEventHandlerTest.kt | 2 +- 6 files changed, 95 insertions(+), 52 deletions(-) diff --git a/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/conversation/ConversationGroupRepository.kt b/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/conversation/ConversationGroupRepository.kt index 7937253e509..43b2b1c2d47 100644 --- a/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/conversation/ConversationGroupRepository.kt +++ b/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/conversation/ConversationGroupRepository.kt @@ -185,11 +185,11 @@ internal class ConversationGroupRepositoryImpl( is ConversationEntity.ProtocolInfo.Mixed -> tryAddMembersToCloudAndStorage(userIdList, conversationId) .flatMap { - tryAddMembersToMLSGroup(protocol.groupId, userIdList) + tryAddMembersToMLSGroup(conversationId, protocol.groupId, userIdList) } is ConversationEntity.ProtocolInfo.MLS -> { - tryAddMembersToMLSGroup(protocol.groupId, userIdList) + tryAddMembersToMLSGroup(conversationId, protocol.groupId, userIdList) } } } @@ -199,33 +199,70 @@ internal class ConversationGroupRepositoryImpl( * Handle error case and retry for sendingCommit unreachable. */ private suspend fun tryAddMembersToMLSGroup( + conversationId: ConversationId, groupId: String, - userIdList: List + userIdList: List, + failedUsersList: Set = emptySet(), + remainingAttempts: Int = 2 ): Either { return when (val addingMemberResult = mlsConversationRepository.addMemberToMLSGroup(GroupID(groupId), userIdList)) { - is Either.Right -> addingMemberResult + is Either.Right -> handleMLSMembersAdded(conversationId, userIdList, failedUsersList) is Either.Left -> { - when (val failure = addingMemberResult.value) { - // claiming key packages offline or out of packages - is CoreFailure.NoKeyPackagesAvailable -> { - mlsConversationRepository.addMemberToMLSGroup( - GroupID(groupId), - userIdList.filterNot { failure.failedUserIds.contains(it) }) - } + if (remainingAttempts >= 0) { + when (val failure = addingMemberResult.value) { + // claiming key packages offline or out of packages + is CoreFailure.NoKeyPackagesAvailable -> { + val (validUsers, failedUsers) = userIdList.partition { !failure.failedUserIds.contains(it) } + tryAddMembersToMLSGroup( + conversationId = conversationId, + groupId = groupId, + userIdList = validUsers, + failedUsersList = (failedUsersList + failedUsers).toSet(), + remainingAttempts = remainingAttempts - 1 + ) + } - // sending commit unreachable - is NetworkFailure.FederatedBackendFailure.RetryableFailure -> { - mlsConversationRepository.addMemberToMLSGroup(GroupID(groupId), userIdList.filterNot { - !failure.domains.contains(it.domain) - }) - } + // sending commit unreachable + is NetworkFailure.FederatedBackendFailure.RetryableFailure -> { + val (validUsers, failedUsers) = extractValidUsersForRetryableFederationError(userIdList, failure) + tryAddMembersToMLSGroup( + conversationId = conversationId, + groupId = groupId, + userIdList = validUsers, + failedUsersList = (failedUsersList + failedUsers).toSet(), + remainingAttempts = remainingAttempts - 1 + ) + } - else -> addingMemberResult + else -> { + newGroupConversationSystemMessagesCreator.value.conversationFailedToAddMembers( + conversationId, (failedUsersList + userIdList) + ).flatMap { + Either.Left(addingMemberResult.value) + } + } + } + } else { + newGroupConversationSystemMessagesCreator.value.conversationFailedToAddMembers( + conversationId, (failedUsersList + userIdList) + ).flatMap { + Either.Left(addingMemberResult.value) + } } } } } + private suspend fun handleMLSMembersAdded( + conversationId: ConversationId, userIdList: List, failedUsersList: Set + ): Either { + return newGroupConversationSystemMessagesCreator.value.conversationResolvedMembersAddedAndFailed( + conversationId.toDao(), userIdList.toSet(), failedUsersList.toSet() + ).flatMap { + Either.Right(Unit) + } + } + override suspend fun addService(serviceId: ServiceId, conversationId: ConversationId): Either = wrapStorageRequest { conversationDAO.getConversationProtocolInfo(conversationId.toDao()) } .flatMap { protocol -> diff --git a/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/conversation/NewConversationMembersRepository.kt b/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/conversation/NewConversationMembersRepository.kt index f174bb54a41..8e25eeb9574 100644 --- a/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/conversation/NewConversationMembersRepository.kt +++ b/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/conversation/NewConversationMembersRepository.kt @@ -19,6 +19,7 @@ package com.wire.kalium.logic.data.conversation import com.wire.kalium.logic.CoreFailure import com.wire.kalium.logic.data.id.IdMapper +import com.wire.kalium.logic.data.id.toModel import com.wire.kalium.logic.data.user.UserId import com.wire.kalium.logic.di.MapperProvider import com.wire.kalium.logic.functional.Either @@ -59,8 +60,8 @@ internal class NewConversationMembersRepositoryImpl( }.flatMap { newGroupConversationSystemMessagesCreator.value.conversationResolvedMembersAddedAndFailed( conversationId, - conversationResponse, - failedUsersList + conversationResponse.members.otherMembers.map { it.id.toModel() }.toSet(), + failedUsersList.toSet() ) } diff --git a/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/conversation/NewGroupConversationSystemMessagesCreator.kt b/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/conversation/NewGroupConversationSystemMessagesCreator.kt index 046fc23e0e6..8cb2ebab6b6 100644 --- a/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/conversation/NewGroupConversationSystemMessagesCreator.kt +++ b/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/conversation/NewGroupConversationSystemMessagesCreator.kt @@ -27,7 +27,6 @@ import com.wire.kalium.logic.data.message.Message import com.wire.kalium.logic.data.message.MessageContent import com.wire.kalium.logic.data.message.PersistMessageUseCase import com.wire.kalium.logic.data.user.UserId -import com.wire.kalium.logic.di.MapperProvider import com.wire.kalium.logic.functional.Either import com.wire.kalium.logic.functional.fold import com.wire.kalium.network.api.base.authenticated.conversation.ConversationResponse @@ -47,8 +46,8 @@ internal interface NewGroupConversationSystemMessagesCreator { suspend fun conversationReadReceiptStatus(conversation: ConversationResponse): Either suspend fun conversationResolvedMembersAddedAndFailed( conversationId: ConversationIDEntity, - conversationResponse: ConversationResponse, - failedUsersList: List = emptyList() + validUsers: Set, + failedUsersList: Set = emptySet() ): Either suspend fun conversationFailedToAddMembers( @@ -64,7 +63,6 @@ internal class NewGroupConversationSystemMessagesCreatorImpl( private val selfTeamIdProvider: SelfTeamIdProvider, private val qualifiedIdMapper: QualifiedIdMapper, private val selfUserId: UserId, - private val memberMapper: MemberMapper = MapperProvider.memberMapper() ) : NewGroupConversationSystemMessagesCreator { override suspend fun conversationStarted(conversation: ConversationEntity) = run { @@ -143,16 +141,14 @@ internal class NewGroupConversationSystemMessagesCreatorImpl( override suspend fun conversationResolvedMembersAddedAndFailed( conversationId: ConversationIDEntity, - conversationResponse: ConversationResponse, - failedUsersList: List + validUsers: Set, + failedUsersList: Set ): Either = run { - if (conversationResponse.members.otherMembers.isNotEmpty()) { + if (validUsers.isNotEmpty()) { persistMessage( Message.System( id = uuid4().toString(), - content = MessageContent.MemberChange.CreationAdded( - memberMapper.fromApiModel(conversationResponse.members).otherMembers.map { it.id } - ), + content = MessageContent.MemberChange.CreationAdded(validUsers.toList()), conversationId = conversationId.toModel(), date = DateTimeUtil.currentIsoDateTimeString(), senderUserId = selfUserId, @@ -169,25 +165,29 @@ internal class NewGroupConversationSystemMessagesCreatorImpl( override suspend fun conversationFailedToAddMembers( conversationId: ConversationId, userIdList: Set - ): Either { - val messageFailedToAddMembers = Message.System( - uuid4().toString(), - MessageContent.MemberChange.FailedToAdd(userIdList.toList()), - conversationId, - DateTimeUtil.currentIsoDateTimeString(), - selfUserId, - Message.Status.Sent, - Message.Visibility.VISIBLE, - expirationData = null - ) - return persistMessage(messageFailedToAddMembers) + ): Either = run { + if (userIdList.isNotEmpty()) { + return persistMessage( + Message.System( + uuid4().toString(), + MessageContent.MemberChange.FailedToAdd(userIdList.toList()), + conversationId, + DateTimeUtil.currentIsoDateTimeString(), + selfUserId, + Message.Status.Sent, + Message.Visibility.VISIBLE, + expirationData = null + ) + ) + } + Either.Right(Unit) } - private suspend fun createFailedToAddSystemMessage(conversationId: ConversationIDEntity, failedUsersList: List) { + private suspend fun createFailedToAddSystemMessage(conversationId: ConversationIDEntity, failedUsersList: Set) { if (failedUsersList.isNotEmpty()) { val messageStartedWithFailedMembers = Message.System( uuid4().toString(), - MessageContent.MemberChange.FailedToAdd(failedUsersList), + MessageContent.MemberChange.FailedToAdd(failedUsersList.toList()), conversationId.toModel(), DateTimeUtil.currentIsoDateTimeString(), selfUserId, diff --git a/logic/src/commonMain/kotlin/com/wire/kalium/logic/sync/receiver/conversation/NewConversationEventHandler.kt b/logic/src/commonMain/kotlin/com/wire/kalium/logic/sync/receiver/conversation/NewConversationEventHandler.kt index 46d857d301b..78effec683d 100644 --- a/logic/src/commonMain/kotlin/com/wire/kalium/logic/sync/receiver/conversation/NewConversationEventHandler.kt +++ b/logic/src/commonMain/kotlin/com/wire/kalium/logic/sync/receiver/conversation/NewConversationEventHandler.kt @@ -94,7 +94,7 @@ internal class NewConversationEventHandlerImpl( newGroupConversationSystemMessagesCreator.conversationStarted(event.senderUserId, event.conversation) newGroupConversationSystemMessagesCreator.conversationResolvedMembersAddedAndFailed( event.conversationId.toDao(), - event.conversation + event.conversation.members.otherMembers.map { it.id.toModel() }.toSet() ) newGroupConversationSystemMessagesCreator.conversationReadReceiptStatus(event.conversation) newGroupConversationSystemMessagesCreator.conversationStartedUnverifiedWarning(event.conversation.id.toModel()) diff --git a/logic/src/commonTest/kotlin/com/wire/kalium/logic/data/conversation/NewGroupConversationSystemMessagesCreatorTest.kt b/logic/src/commonTest/kotlin/com/wire/kalium/logic/data/conversation/NewGroupConversationSystemMessagesCreatorTest.kt index 1944bddcc35..a205a965ef7 100644 --- a/logic/src/commonTest/kotlin/com/wire/kalium/logic/data/conversation/NewGroupConversationSystemMessagesCreatorTest.kt +++ b/logic/src/commonTest/kotlin/com/wire/kalium/logic/data/conversation/NewGroupConversationSystemMessagesCreatorTest.kt @@ -17,6 +17,7 @@ */ package com.wire.kalium.logic.data.conversation +import com.wire.kalium.logic.data.conversation.NewGroupConversationSystemMessagesCreatorTest.Arrangement.Companion.otherMembersIds import com.wire.kalium.logic.data.id.QualifiedIdMapper import com.wire.kalium.logic.data.id.SelfTeamIdProvider import com.wire.kalium.logic.data.id.toApi @@ -236,7 +237,7 @@ class NewGroupConversationSystemMessagesCreatorTest { val result = sysMessageCreator.conversationResolvedMembersAddedAndFailed( TestConversation.CONVERSATION_RESPONSE.id.toDao(), - TestConversation.CONVERSATION_RESPONSE + otherMembersIds ) result.shouldSucceed() @@ -266,8 +267,8 @@ class NewGroupConversationSystemMessagesCreatorTest { val result = sysMessageCreator.conversationResolvedMembersAddedAndFailed( TestConversation.CONVERSATION_RESPONSE.id.toDao(), - TestConversation.CONVERSATION_RESPONSE, - listOf(TestUser.USER_ID) + otherMembersIds, + setOf(TestUser.USER_ID) ) result.shouldSucceed() @@ -302,10 +303,10 @@ class NewGroupConversationSystemMessagesCreatorTest { ConversationMemberDTO.Self( TestUser.SELF.id.toApi(), "wire_admin" - ), listOf() + ), emptyList() ) - ), - listOf(TestUser.USER_ID) + ).members.otherMembers.map { it.id.toModel() }.toSet(), + setOf(TestUser.USER_ID) ) result.shouldSucceed() @@ -400,6 +401,10 @@ class NewGroupConversationSystemMessagesCreatorTest { qualifiedIdMapper, TestUser.SELF.id, ) + + companion object { + val otherMembersIds = TestConversation.CONVERSATION_RESPONSE.members.otherMembers.map { it.id.toModel() }.toSet() + } } } diff --git a/logic/src/commonTest/kotlin/com/wire/kalium/logic/sync/receiver/conversation/NewConversationEventHandlerTest.kt b/logic/src/commonTest/kotlin/com/wire/kalium/logic/sync/receiver/conversation/NewConversationEventHandlerTest.kt index dcd48ca4eba..82f385445d1 100644 --- a/logic/src/commonTest/kotlin/com/wire/kalium/logic/sync/receiver/conversation/NewConversationEventHandlerTest.kt +++ b/logic/src/commonTest/kotlin/com/wire/kalium/logic/sync/receiver/conversation/NewConversationEventHandlerTest.kt @@ -166,7 +166,7 @@ class NewConversationEventHandlerTest { verify(arrangement.newGroupConversationSystemMessagesCreator) .suspendFunction(arrangement.newGroupConversationSystemMessagesCreator::conversationResolvedMembersAddedAndFailed) - .with(eq(event.conversationId.toDao()), eq(event.conversation)) + .with(eq(event.conversationId.toDao()), eq(event.conversation.members.otherMembers.map { it.id.toModel() }.toSet()), any()) .wasInvoked(exactly = once) verify(arrangement.newGroupConversationSystemMessagesCreator) From 0936e4833807864d98e0c081598538c0a07c0070 Mon Sep 17 00:00:00 2001 From: yamilmedina Date: Tue, 28 Nov 2023 18:23:15 +0100 Subject: [PATCH 18/30] feat: persist system messages for failed and added, and handle retry --- .../conversation/ConversationGroupRepository.kt | 2 +- .../NewConversationMembersRepository.kt | 4 ++-- .../NewGroupConversationSystemMessagesCreator.kt | 14 +++++++------- .../conversation/NewConversationEventHandler.kt | 2 +- ...ewGroupConversationSystemMessagesCreatorTest.kt | 8 ++++---- 5 files changed, 15 insertions(+), 15 deletions(-) diff --git a/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/conversation/ConversationGroupRepository.kt b/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/conversation/ConversationGroupRepository.kt index 43b2b1c2d47..76dc0bb6931 100644 --- a/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/conversation/ConversationGroupRepository.kt +++ b/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/conversation/ConversationGroupRepository.kt @@ -257,7 +257,7 @@ internal class ConversationGroupRepositoryImpl( conversationId: ConversationId, userIdList: List, failedUsersList: Set ): Either { return newGroupConversationSystemMessagesCreator.value.conversationResolvedMembersAddedAndFailed( - conversationId.toDao(), userIdList.toSet(), failedUsersList.toSet() + conversationId.toDao(), userIdList, failedUsersList.toList() ).flatMap { Either.Right(Unit) } diff --git a/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/conversation/NewConversationMembersRepository.kt b/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/conversation/NewConversationMembersRepository.kt index 8e25eeb9574..500ffa58de6 100644 --- a/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/conversation/NewConversationMembersRepository.kt +++ b/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/conversation/NewConversationMembersRepository.kt @@ -60,8 +60,8 @@ internal class NewConversationMembersRepositoryImpl( }.flatMap { newGroupConversationSystemMessagesCreator.value.conversationResolvedMembersAddedAndFailed( conversationId, - conversationResponse.members.otherMembers.map { it.id.toModel() }.toSet(), - failedUsersList.toSet() + conversationResponse.members.otherMembers.map { it.id.toModel() }, + failedUsersList ) } diff --git a/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/conversation/NewGroupConversationSystemMessagesCreator.kt b/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/conversation/NewGroupConversationSystemMessagesCreator.kt index 8cb2ebab6b6..79c38fb5959 100644 --- a/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/conversation/NewGroupConversationSystemMessagesCreator.kt +++ b/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/conversation/NewGroupConversationSystemMessagesCreator.kt @@ -46,8 +46,8 @@ internal interface NewGroupConversationSystemMessagesCreator { suspend fun conversationReadReceiptStatus(conversation: ConversationResponse): Either suspend fun conversationResolvedMembersAddedAndFailed( conversationId: ConversationIDEntity, - validUsers: Set, - failedUsersList: Set = emptySet() + validUsers: List, + failedUsersList: List = emptyList() ): Either suspend fun conversationFailedToAddMembers( @@ -141,8 +141,8 @@ internal class NewGroupConversationSystemMessagesCreatorImpl( override suspend fun conversationResolvedMembersAddedAndFailed( conversationId: ConversationIDEntity, - validUsers: Set, - failedUsersList: Set + validUsers: List, + failedUsersList: List ): Either = run { if (validUsers.isNotEmpty()) { persistMessage( @@ -167,7 +167,7 @@ internal class NewGroupConversationSystemMessagesCreatorImpl( userIdList: Set ): Either = run { if (userIdList.isNotEmpty()) { - return persistMessage( + persistMessage( Message.System( uuid4().toString(), MessageContent.MemberChange.FailedToAdd(userIdList.toList()), @@ -183,11 +183,11 @@ internal class NewGroupConversationSystemMessagesCreatorImpl( Either.Right(Unit) } - private suspend fun createFailedToAddSystemMessage(conversationId: ConversationIDEntity, failedUsersList: Set) { + private suspend fun createFailedToAddSystemMessage(conversationId: ConversationIDEntity, failedUsersList: List) { if (failedUsersList.isNotEmpty()) { val messageStartedWithFailedMembers = Message.System( uuid4().toString(), - MessageContent.MemberChange.FailedToAdd(failedUsersList.toList()), + MessageContent.MemberChange.FailedToAdd(failedUsersList), conversationId.toModel(), DateTimeUtil.currentIsoDateTimeString(), selfUserId, diff --git a/logic/src/commonMain/kotlin/com/wire/kalium/logic/sync/receiver/conversation/NewConversationEventHandler.kt b/logic/src/commonMain/kotlin/com/wire/kalium/logic/sync/receiver/conversation/NewConversationEventHandler.kt index 78effec683d..518cf85e161 100644 --- a/logic/src/commonMain/kotlin/com/wire/kalium/logic/sync/receiver/conversation/NewConversationEventHandler.kt +++ b/logic/src/commonMain/kotlin/com/wire/kalium/logic/sync/receiver/conversation/NewConversationEventHandler.kt @@ -94,7 +94,7 @@ internal class NewConversationEventHandlerImpl( newGroupConversationSystemMessagesCreator.conversationStarted(event.senderUserId, event.conversation) newGroupConversationSystemMessagesCreator.conversationResolvedMembersAddedAndFailed( event.conversationId.toDao(), - event.conversation.members.otherMembers.map { it.id.toModel() }.toSet() + event.conversation.members.otherMembers.map { it.id.toModel() } ) newGroupConversationSystemMessagesCreator.conversationReadReceiptStatus(event.conversation) newGroupConversationSystemMessagesCreator.conversationStartedUnverifiedWarning(event.conversation.id.toModel()) diff --git a/logic/src/commonTest/kotlin/com/wire/kalium/logic/data/conversation/NewGroupConversationSystemMessagesCreatorTest.kt b/logic/src/commonTest/kotlin/com/wire/kalium/logic/data/conversation/NewGroupConversationSystemMessagesCreatorTest.kt index a205a965ef7..a2a86d7c20e 100644 --- a/logic/src/commonTest/kotlin/com/wire/kalium/logic/data/conversation/NewGroupConversationSystemMessagesCreatorTest.kt +++ b/logic/src/commonTest/kotlin/com/wire/kalium/logic/data/conversation/NewGroupConversationSystemMessagesCreatorTest.kt @@ -268,7 +268,7 @@ class NewGroupConversationSystemMessagesCreatorTest { val result = sysMessageCreator.conversationResolvedMembersAddedAndFailed( TestConversation.CONVERSATION_RESPONSE.id.toDao(), otherMembersIds, - setOf(TestUser.USER_ID) + listOf(TestUser.USER_ID) ) result.shouldSucceed() @@ -305,8 +305,8 @@ class NewGroupConversationSystemMessagesCreatorTest { "wire_admin" ), emptyList() ) - ).members.otherMembers.map { it.id.toModel() }.toSet(), - setOf(TestUser.USER_ID) + ).members.otherMembers.map { it.id.toModel() }, + listOf(TestUser.USER_ID) ) result.shouldSucceed() @@ -403,7 +403,7 @@ class NewGroupConversationSystemMessagesCreatorTest { ) companion object { - val otherMembersIds = TestConversation.CONVERSATION_RESPONSE.members.otherMembers.map { it.id.toModel() }.toSet() + val otherMembersIds = TestConversation.CONVERSATION_RESPONSE.members.otherMembers.map { it.id.toModel() } } } From c2439e4ddaa679a1a48a6a181418d632a7741288 Mon Sep 17 00:00:00 2001 From: yamilmedina Date: Tue, 28 Nov 2023 18:25:02 +0100 Subject: [PATCH 19/30] feat: persist system messages for failed and added, and handle retry --- .../logic/data/conversation/ConversationGroupRepository.kt | 1 + .../receiver/conversation/NewConversationEventHandlerTest.kt | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/conversation/ConversationGroupRepository.kt b/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/conversation/ConversationGroupRepository.kt index 76dc0bb6931..0aa64ccf7dc 100644 --- a/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/conversation/ConversationGroupRepository.kt +++ b/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/conversation/ConversationGroupRepository.kt @@ -208,6 +208,7 @@ internal class ConversationGroupRepositoryImpl( return when (val addingMemberResult = mlsConversationRepository.addMemberToMLSGroup(GroupID(groupId), userIdList)) { is Either.Right -> handleMLSMembersAdded(conversationId, userIdList, failedUsersList) is Either.Left -> { + // todo, cleanup, extract to a function and try to reuse if (remainingAttempts >= 0) { when (val failure = addingMemberResult.value) { // claiming key packages offline or out of packages diff --git a/logic/src/commonTest/kotlin/com/wire/kalium/logic/sync/receiver/conversation/NewConversationEventHandlerTest.kt b/logic/src/commonTest/kotlin/com/wire/kalium/logic/sync/receiver/conversation/NewConversationEventHandlerTest.kt index 82f385445d1..b04f98f8953 100644 --- a/logic/src/commonTest/kotlin/com/wire/kalium/logic/sync/receiver/conversation/NewConversationEventHandlerTest.kt +++ b/logic/src/commonTest/kotlin/com/wire/kalium/logic/sync/receiver/conversation/NewConversationEventHandlerTest.kt @@ -166,7 +166,7 @@ class NewConversationEventHandlerTest { verify(arrangement.newGroupConversationSystemMessagesCreator) .suspendFunction(arrangement.newGroupConversationSystemMessagesCreator::conversationResolvedMembersAddedAndFailed) - .with(eq(event.conversationId.toDao()), eq(event.conversation.members.otherMembers.map { it.id.toModel() }.toSet()), any()) + .with(eq(event.conversationId.toDao()), eq(event.conversation.members.otherMembers.map { it.id.toModel() }), any()) .wasInvoked(exactly = once) verify(arrangement.newGroupConversationSystemMessagesCreator) From e1e075f5534ffe31f61cf8362cc52e24762f1ae3 Mon Sep 17 00:00:00 2001 From: yamilmedina Date: Tue, 28 Nov 2023 18:30:27 +0100 Subject: [PATCH 20/30] feat: persist system messages for failed and added, and handle retry --- .../logic/data/conversation/ConversationGroupRepository.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/conversation/ConversationGroupRepository.kt b/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/conversation/ConversationGroupRepository.kt index 0aa64ccf7dc..0f6a3099688 100644 --- a/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/conversation/ConversationGroupRepository.kt +++ b/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/conversation/ConversationGroupRepository.kt @@ -209,7 +209,7 @@ internal class ConversationGroupRepositoryImpl( is Either.Right -> handleMLSMembersAdded(conversationId, userIdList, failedUsersList) is Either.Left -> { // todo, cleanup, extract to a function and try to reuse - if (remainingAttempts >= 0) { + if (remainingAttempts > 0) { when (val failure = addingMemberResult.value) { // claiming key packages offline or out of packages is CoreFailure.NoKeyPackagesAvailable -> { From af27f04d19386bd2a86bf3120818b6632c55512e Mon Sep 17 00:00:00 2001 From: yamilmedina Date: Wed, 29 Nov 2023 23:42:38 +0100 Subject: [PATCH 21/30] chore: refactor cleanuop --- .../ConversationGroupRepository.kt | 85 ++++++++++--------- 1 file changed, 46 insertions(+), 39 deletions(-) diff --git a/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/conversation/ConversationGroupRepository.kt b/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/conversation/ConversationGroupRepository.kt index 0f6a3099688..ce5175d423c 100644 --- a/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/conversation/ConversationGroupRepository.kt +++ b/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/conversation/ConversationGroupRepository.kt @@ -208,47 +208,54 @@ internal class ConversationGroupRepositoryImpl( return when (val addingMemberResult = mlsConversationRepository.addMemberToMLSGroup(GroupID(groupId), userIdList)) { is Either.Right -> handleMLSMembersAdded(conversationId, userIdList, failedUsersList) is Either.Left -> { - // todo, cleanup, extract to a function and try to reuse - if (remainingAttempts > 0) { - when (val failure = addingMemberResult.value) { - // claiming key packages offline or out of packages - is CoreFailure.NoKeyPackagesAvailable -> { - val (validUsers, failedUsers) = userIdList.partition { !failure.failedUserIds.contains(it) } - tryAddMembersToMLSGroup( - conversationId = conversationId, - groupId = groupId, - userIdList = validUsers, - failedUsersList = (failedUsersList + failedUsers).toSet(), - remainingAttempts = remainingAttempts - 1 - ) - } + addingMemberResult.value.handleMLSMembersFailed( + conversationId = conversationId, + groupId = groupId, + userIdList = userIdList, + failedUsersList = failedUsersList, + remainingAttempts = remainingAttempts, + ) + } + } + } - // sending commit unreachable - is NetworkFailure.FederatedBackendFailure.RetryableFailure -> { - val (validUsers, failedUsers) = extractValidUsersForRetryableFederationError(userIdList, failure) - tryAddMembersToMLSGroup( - conversationId = conversationId, - groupId = groupId, - userIdList = validUsers, - failedUsersList = (failedUsersList + failedUsers).toSet(), - remainingAttempts = remainingAttempts - 1 - ) - } + private suspend fun CoreFailure.handleMLSMembersFailed( + conversationId: ConversationId, + groupId: String, + userIdList: List, + failedUsersList: Set, + remainingAttempts: Int, + ): Either { + return when { + // claiming key packages offline or out of packages + this is CoreFailure.NoKeyPackagesAvailable && remainingAttempts > 0 -> { + val (validUsers, failedUsers) = userIdList.partition { !this.failedUserIds.contains(it) } + tryAddMembersToMLSGroup( + conversationId = conversationId, + groupId = groupId, + userIdList = validUsers, + failedUsersList = (failedUsersList + failedUsers).toSet(), + remainingAttempts = remainingAttempts - 1 + ) + } - else -> { - newGroupConversationSystemMessagesCreator.value.conversationFailedToAddMembers( - conversationId, (failedUsersList + userIdList) - ).flatMap { - Either.Left(addingMemberResult.value) - } - } - } - } else { - newGroupConversationSystemMessagesCreator.value.conversationFailedToAddMembers( - conversationId, (failedUsersList + userIdList) - ).flatMap { - Either.Left(addingMemberResult.value) - } + // sending commit unreachable + this is NetworkFailure.FederatedBackendFailure.RetryableFailure && remainingAttempts > 0 -> { + val (validUsers, failedUsers) = extractValidUsersForRetryableFederationError(userIdList, this) + tryAddMembersToMLSGroup( + conversationId = conversationId, + groupId = groupId, + userIdList = validUsers, + failedUsersList = (failedUsersList + failedUsers).toSet(), + remainingAttempts = remainingAttempts - 1 + ) + } + + else -> { + newGroupConversationSystemMessagesCreator.value.conversationFailedToAddMembers( + conversationId, (failedUsersList + userIdList) + ).flatMap { + Either.Left(this) } } } From c1d93e355c2e9991a5377c7296d1aa86dcba5607 Mon Sep 17 00:00:00 2001 From: yamilmedina Date: Thu, 30 Nov 2023 11:20:59 +0100 Subject: [PATCH 22/30] feat: fixing tests --- .../conversation/ConversationGroupRepositoryTest.kt | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/logic/src/commonTest/kotlin/com/wire/kalium/logic/data/conversation/ConversationGroupRepositoryTest.kt b/logic/src/commonTest/kotlin/com/wire/kalium/logic/data/conversation/ConversationGroupRepositoryTest.kt index ad1ec0543d9..6b3cbd621eb 100644 --- a/logic/src/commonTest/kotlin/com/wire/kalium/logic/data/conversation/ConversationGroupRepositoryTest.kt +++ b/logic/src/commonTest/kotlin/com/wire/kalium/logic/data/conversation/ConversationGroupRepositoryTest.kt @@ -22,12 +22,12 @@ import com.wire.kalium.logic.MLSFailure import com.wire.kalium.logic.StorageFailure import com.wire.kalium.logic.data.id.ConversationId import com.wire.kalium.logic.data.id.GroupID +import com.wire.kalium.logic.data.id.SelfTeamIdProvider import com.wire.kalium.logic.data.id.TeamId import com.wire.kalium.logic.data.id.toApi import com.wire.kalium.logic.data.id.toModel import com.wire.kalium.logic.data.service.ServiceId import com.wire.kalium.logic.data.user.UserRepository -import com.wire.kalium.logic.data.id.SelfTeamIdProvider import com.wire.kalium.logic.framework.TestConversation import com.wire.kalium.logic.framework.TestConversation.ADD_MEMBER_TO_CONVERSATION_SUCCESSFUL_RESPONSE import com.wire.kalium.logic.framework.TestUser @@ -450,6 +450,7 @@ class ConversationGroupRepositoryTest { .withProtocolInfoById(MLS_PROTOCOL_INFO) .withAddMemberAPISucceedChanged() .withSuccessfulAddMemberToMLSGroup() + .withInsertAddedAndFailedSystemMessageSuccess() .arrange() conversationGroupRepository.addMembers(listOf(TestConversation.USER_1), TestConversation.ID) @@ -475,6 +476,7 @@ class ConversationGroupRepositoryTest { .withAddMemberAPISucceedChanged() .withSuccessfulAddMemberToMLSGroup() .withSuccessfulHandleMemberJoinEvent() + .withInsertAddedAndFailedSystemMessageSuccess() .arrange() conversationGroupRepository.addMembers(listOf(TestConversation.USER_1), TestConversation.ID) @@ -1568,6 +1570,13 @@ class ConversationGroupRepositoryTest { .thenReturn(Either.Right(Unit)) } + fun withInsertAddedAndFailedSystemMessageSuccess(): Arrangement = apply { + given(newGroupConversationSystemMessagesCreator) + .suspendFunction(newGroupConversationSystemMessagesCreator::conversationResolvedMembersAddedAndFailed) + .whenInvokedWith(anything(), anything(), anything()) + .thenReturn(Either.Right(Unit)) + } + fun withAddMemberAPIFailsFirstWithUnreachableThenSucceed(networkResponses: Array>) = apply { given(conversationApi) From 3da38d10e4c23a5b9b7d3fc49874bfa0d60d8ebf Mon Sep 17 00:00:00 2001 From: yamilmedina Date: Thu, 30 Nov 2023 12:08:20 +0100 Subject: [PATCH 23/30] feat: add test coverage for retry mec --- .../ConversationGroupRepositoryTest.kt | 51 +++++++++++++++++++ 1 file changed, 51 insertions(+) diff --git a/logic/src/commonTest/kotlin/com/wire/kalium/logic/data/conversation/ConversationGroupRepositoryTest.kt b/logic/src/commonTest/kotlin/com/wire/kalium/logic/data/conversation/ConversationGroupRepositoryTest.kt index 6b3cbd621eb..a91f497aca8 100644 --- a/logic/src/commonTest/kotlin/com/wire/kalium/logic/data/conversation/ConversationGroupRepositoryTest.kt +++ b/logic/src/commonTest/kotlin/com/wire/kalium/logic/data/conversation/ConversationGroupRepositoryTest.kt @@ -18,6 +18,7 @@ package com.wire.kalium.logic.data.conversation +import com.wire.kalium.logic.CoreFailure import com.wire.kalium.logic.MLSFailure import com.wire.kalium.logic.StorageFailure import com.wire.kalium.logic.data.id.ConversationId @@ -1186,6 +1187,46 @@ class ConversationGroupRepositoryTest { } } + @Test + fun givenNoPackagesAvailable_whenAddingMembers_thenCreateRetryOnceWithValidUsersAndPersistSystemMessage() = runTest { + // given + val expectedInitialUsers = listOf(TestConversation.USER_1, TestUser.OTHER_FEDERATED_USER_ID) + val (arrangement, conversationGroupRepository) = Arrangement() + .withConversationDetailsById(TestConversation.MLS_CONVERSATION) + .withProtocolInfoById(MLS_PROTOCOL_INFO) + .withAddMemberAPISucceedChanged() + .withSuccessfulAddMemberToMLSGroup() + .withAddingMemberToMlsGroupResults( + Either.Left(CoreFailure.NoKeyPackagesAvailable(setOf(expectedInitialUsers.last()))), + Either.Right(Unit) + ) + .withInsertAddedAndFailedSystemMessageSuccess() + .arrange() + + // when + conversationGroupRepository.addMembers(expectedInitialUsers, TestConversation.ID).shouldSucceed() + + // then + val expectedFullUserIdsForRequestCount = 2 + val expectedValidUsersWithKeyPackagesCount = 1 + verify(arrangement.mlsConversationRepository) + .suspendFunction(arrangement.mlsConversationRepository::addMemberToMLSGroup) + .with(anything(), matching { + it.size == expectedFullUserIdsForRequestCount + }).wasInvoked(exactly = once) + + verify(arrangement.mlsConversationRepository) + .suspendFunction(arrangement.mlsConversationRepository::addMemberToMLSGroup) + .with(anything(), matching { + it.size == expectedValidUsersWithKeyPackagesCount && it.first() == TestConversation.USER_1 + }).wasInvoked(exactly = once) + + verify(arrangement.newGroupConversationSystemMessagesCreator) + .suspendFunction(arrangement.newGroupConversationSystemMessagesCreator::conversationResolvedMembersAddedAndFailed) + .with(anything(), matching { it.size == 1 }, matching { it.size == 1 }) + .wasInvoked(exactly = once) + } + private class Arrangement : MemberDAOArrangement by MemberDAOArrangementImpl() { @@ -1445,6 +1486,16 @@ class ConversationGroupRepositoryTest { .thenReturn(Either.Right(Unit)) } + /** + * Mocks sequentially responses from [MLSConversationRepository] with [result]. + */ + fun withAddingMemberToMlsGroupResults(vararg results: Either) = apply { + given(mlsConversationRepository) + .suspendFunction(mlsConversationRepository::addMemberToMLSGroup) + .whenInvokedWith(any(), any()) + .thenReturnSequentially(*results) + } + fun withSuccessfulRemoveMemberFromMLSGroup() = apply { given(mlsConversationRepository) .suspendFunction(mlsConversationRepository::removeMembersFromMLSGroup) From 775ed922604eba48a1ed0799f0723c6f053fb23f Mon Sep 17 00:00:00 2001 From: yamilmedina Date: Thu, 30 Nov 2023 14:17:53 +0100 Subject: [PATCH 24/30] feat: add test coverage for retry bundle --- .../ConversationGroupRepositoryTest.kt | 48 ++++++++++++++++++- 1 file changed, 46 insertions(+), 2 deletions(-) diff --git a/logic/src/commonTest/kotlin/com/wire/kalium/logic/data/conversation/ConversationGroupRepositoryTest.kt b/logic/src/commonTest/kotlin/com/wire/kalium/logic/data/conversation/ConversationGroupRepositoryTest.kt index a91f497aca8..2f13ce5f484 100644 --- a/logic/src/commonTest/kotlin/com/wire/kalium/logic/data/conversation/ConversationGroupRepositoryTest.kt +++ b/logic/src/commonTest/kotlin/com/wire/kalium/logic/data/conversation/ConversationGroupRepositoryTest.kt @@ -20,6 +20,7 @@ package com.wire.kalium.logic.data.conversation import com.wire.kalium.logic.CoreFailure import com.wire.kalium.logic.MLSFailure +import com.wire.kalium.logic.NetworkFailure import com.wire.kalium.logic.StorageFailure import com.wire.kalium.logic.data.id.ConversationId import com.wire.kalium.logic.data.id.GroupID @@ -1188,7 +1189,7 @@ class ConversationGroupRepositoryTest { } @Test - fun givenNoPackagesAvailable_whenAddingMembers_thenCreateRetryOnceWithValidUsersAndPersistSystemMessage() = runTest { + fun givenNoPackagesAvailable_whenAddingMembers_thenRetryOnceWithValidUsersAndPersistSystemMessage() = runTest { // given val expectedInitialUsers = listOf(TestConversation.USER_1, TestUser.OTHER_FEDERATED_USER_ID) val (arrangement, conversationGroupRepository) = Arrangement() @@ -1197,7 +1198,47 @@ class ConversationGroupRepositoryTest { .withAddMemberAPISucceedChanged() .withSuccessfulAddMemberToMLSGroup() .withAddingMemberToMlsGroupResults( - Either.Left(CoreFailure.NoKeyPackagesAvailable(setOf(expectedInitialUsers.last()))), + KEY_PACKAGES_NOT_AVAILABLE_FAILURE, + Either.Right(Unit) + ) + .withInsertAddedAndFailedSystemMessageSuccess() + .arrange() + + // when + conversationGroupRepository.addMembers(expectedInitialUsers, TestConversation.ID).shouldSucceed() + + // then + val expectedFullUserIdsForRequestCount = 2 + val expectedValidUsersWithKeyPackagesCount = 1 + verify(arrangement.mlsConversationRepository) + .suspendFunction(arrangement.mlsConversationRepository::addMemberToMLSGroup) + .with(anything(), matching { + it.size == expectedFullUserIdsForRequestCount + }).wasInvoked(exactly = once) + + verify(arrangement.mlsConversationRepository) + .suspendFunction(arrangement.mlsConversationRepository::addMemberToMLSGroup) + .with(anything(), matching { + it.size == expectedValidUsersWithKeyPackagesCount && it.first() == TestConversation.USER_1 + }).wasInvoked(exactly = once) + + verify(arrangement.newGroupConversationSystemMessagesCreator) + .suspendFunction(arrangement.newGroupConversationSystemMessagesCreator::conversationResolvedMembersAddedAndFailed) + .with(anything(), matching { it.size == 1 }, matching { it.size == 1 }) + .wasInvoked(exactly = once) + } + + @Test + fun givenSendCommitFederatedFailure_whenAddingMembers_thenRetryOnceWithValidUsersAndPersistSystemMessage() = runTest { + // given + val expectedInitialUsers = listOf(TestConversation.USER_1, TestUser.OTHER_FEDERATED_USER_ID) + val (arrangement, conversationGroupRepository) = Arrangement() + .withConversationDetailsById(TestConversation.MLS_CONVERSATION) + .withProtocolInfoById(MLS_PROTOCOL_INFO) + .withAddMemberAPISucceedChanged() + .withSuccessfulAddMemberToMLSGroup() + .withAddingMemberToMlsGroupResults( + COMMIT_BUNDLE_FEDERATED_FAILURE, Either.Right(Unit) ) .withInsertAddedAndFailedSystemMessageSuccess() @@ -1711,5 +1752,8 @@ class ConversationGroupRepositoryTest { mapOf(), HttpStatusCode.OK.value ) + + val COMMIT_BUNDLE_FEDERATED_FAILURE = Either.Left(NetworkFailure.FederatedBackendFailure.FailedDomains(listOf("otherDomain"))) + val KEY_PACKAGES_NOT_AVAILABLE_FAILURE = Either.Left(CoreFailure.NoKeyPackagesAvailable(setOf(TestUser.OTHER_FEDERATED_USER_ID))) } } From ca9fd4bc07dc656addb2f23bc18bea921c4a864c Mon Sep 17 00:00:00 2001 From: yamilmedina Date: Thu, 30 Nov 2023 14:38:57 +0100 Subject: [PATCH 25/30] chore: detekt --- .../logic/data/conversation/ConversationGroupRepository.kt | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/conversation/ConversationGroupRepository.kt b/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/conversation/ConversationGroupRepository.kt index ce5175d423c..42e4f390f80 100644 --- a/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/conversation/ConversationGroupRepository.kt +++ b/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/conversation/ConversationGroupRepository.kt @@ -262,7 +262,9 @@ internal class ConversationGroupRepositoryImpl( } private suspend fun handleMLSMembersAdded( - conversationId: ConversationId, userIdList: List, failedUsersList: Set + conversationId: ConversationId, + userIdList: List, + failedUsersList: Set ): Either { return newGroupConversationSystemMessagesCreator.value.conversationResolvedMembersAddedAndFailed( conversationId.toDao(), userIdList, failedUsersList.toList() From 194998528a1516cc71f67776de74c4cc4611f9fe Mon Sep 17 00:00:00 2001 From: yamilmedina Date: Fri, 1 Dec 2023 11:55:04 +0100 Subject: [PATCH 26/30] chore: test cov --- .../ConversationGroupRepositoryTest.kt | 57 +++++++++++++++++-- 1 file changed, 52 insertions(+), 5 deletions(-) diff --git a/logic/src/commonTest/kotlin/com/wire/kalium/logic/data/conversation/ConversationGroupRepositoryTest.kt b/logic/src/commonTest/kotlin/com/wire/kalium/logic/data/conversation/ConversationGroupRepositoryTest.kt index 2f13ce5f484..d9f3985dcb9 100644 --- a/logic/src/commonTest/kotlin/com/wire/kalium/logic/data/conversation/ConversationGroupRepositoryTest.kt +++ b/logic/src/commonTest/kotlin/com/wire/kalium/logic/data/conversation/ConversationGroupRepositoryTest.kt @@ -1195,12 +1195,12 @@ class ConversationGroupRepositoryTest { val (arrangement, conversationGroupRepository) = Arrangement() .withConversationDetailsById(TestConversation.MLS_CONVERSATION) .withProtocolInfoById(MLS_PROTOCOL_INFO) - .withAddMemberAPISucceedChanged() - .withSuccessfulAddMemberToMLSGroup() .withAddingMemberToMlsGroupResults( KEY_PACKAGES_NOT_AVAILABLE_FAILURE, Either.Right(Unit) ) + .withAddMemberAPISucceedChanged() + .withSuccessfulAddMemberToMLSGroup() .withInsertAddedAndFailedSystemMessageSuccess() .arrange() @@ -1236,11 +1236,11 @@ class ConversationGroupRepositoryTest { .withConversationDetailsById(TestConversation.MLS_CONVERSATION) .withProtocolInfoById(MLS_PROTOCOL_INFO) .withAddMemberAPISucceedChanged() - .withSuccessfulAddMemberToMLSGroup() .withAddingMemberToMlsGroupResults( - COMMIT_BUNDLE_FEDERATED_FAILURE, + buildCommitBundleFederatedFailure("otherDomain"), Either.Right(Unit) ) + .withSuccessfulAddMemberToMLSGroup() .withInsertAddedAndFailedSystemMessageSuccess() .arrange() @@ -1268,6 +1268,50 @@ class ConversationGroupRepositoryTest { .wasInvoked(exactly = once) } + @Test + fun givenMoreThan2Retries_whenAddingMembers_thenTheOperationsShouldFailAndPersistFailedToAddAllMembers() = runTest { + // given + val expectedInitialUsers = listOf(TestConversation.USER_1, TestUser.OTHER_FEDERATED_USER_ID_2, TestUser.OTHER_FEDERATED_USER_ID) + val (arrangement, conversationGroupRepository) = Arrangement() + .withConversationDetailsById(TestConversation.MLS_CONVERSATION) + .withProtocolInfoById(MLS_PROTOCOL_INFO) + .withAddingMemberToMlsGroupResults( + KEY_PACKAGES_NOT_AVAILABLE_FAILURE, + buildCommitBundleFederatedFailure("otherDomain2"), + buildCommitBundleFederatedFailure("domainMember"), + ) + .withInsertFailedToAddSystemMessageSuccess() + .arrange() + + // when + conversationGroupRepository.addMembers(expectedInitialUsers, TestConversation.ID).shouldFail() + + // then + val initialCountUsers = expectedInitialUsers.size + verify(arrangement.mlsConversationRepository) + .suspendFunction(arrangement.mlsConversationRepository::addMemberToMLSGroup) + .with(anything(), matching { + it.size == initialCountUsers + }).wasInvoked(exactly = once) + + verify(arrangement.mlsConversationRepository) + .suspendFunction(arrangement.mlsConversationRepository::addMemberToMLSGroup) + .with(anything(), matching { + it.size == initialCountUsers - 1 // removed 1 failed users with key packages + }).wasInvoked(exactly = once) + + verify(arrangement.mlsConversationRepository) + .suspendFunction(arrangement.mlsConversationRepository::addMemberToMLSGroup) + .with(anything(), matching { + it.size == initialCountUsers - 1 // removed 1 failed user with commit bundle federated error + }).wasInvoked(exactly = once) + + verify(arrangement.newGroupConversationSystemMessagesCreator) + .suspendFunction(arrangement.newGroupConversationSystemMessagesCreator::conversationFailedToAddMembers) + .with(anything(), matching { it.size == 3 }) + .wasInvoked(exactly = once) + } + private class Arrangement : MemberDAOArrangement by MemberDAOArrangementImpl() { @@ -1753,7 +1797,10 @@ class ConversationGroupRepositoryTest { HttpStatusCode.OK.value ) - val COMMIT_BUNDLE_FEDERATED_FAILURE = Either.Left(NetworkFailure.FederatedBackendFailure.FailedDomains(listOf("otherDomain"))) + private fun buildCommitBundleFederatedFailure(vararg domains: String = arrayOf("otherDomain")) = Either.Left( + NetworkFailure.FederatedBackendFailure.FailedDomains(domains.toList()) + ) + val KEY_PACKAGES_NOT_AVAILABLE_FAILURE = Either.Left(CoreFailure.NoKeyPackagesAvailable(setOf(TestUser.OTHER_FEDERATED_USER_ID))) } } From 5a3a2f64804aaf19fe304981486a041f841b3098 Mon Sep 17 00:00:00 2001 From: yamilmedina Date: Fri, 1 Dec 2023 14:33:36 +0100 Subject: [PATCH 27/30] fix: testst --- .../data/conversation/ConversationGroupRepositoryTest.kt | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/logic/src/commonTest/kotlin/com/wire/kalium/logic/data/conversation/ConversationGroupRepositoryTest.kt b/logic/src/commonTest/kotlin/com/wire/kalium/logic/data/conversation/ConversationGroupRepositoryTest.kt index d9f3985dcb9..934e3185b2f 100644 --- a/logic/src/commonTest/kotlin/com/wire/kalium/logic/data/conversation/ConversationGroupRepositoryTest.kt +++ b/logic/src/commonTest/kotlin/com/wire/kalium/logic/data/conversation/ConversationGroupRepositoryTest.kt @@ -1195,12 +1195,12 @@ class ConversationGroupRepositoryTest { val (arrangement, conversationGroupRepository) = Arrangement() .withConversationDetailsById(TestConversation.MLS_CONVERSATION) .withProtocolInfoById(MLS_PROTOCOL_INFO) + .withAddMemberAPISucceedChanged() + .withSuccessfulAddMemberToMLSGroup() .withAddingMemberToMlsGroupResults( KEY_PACKAGES_NOT_AVAILABLE_FAILURE, Either.Right(Unit) ) - .withAddMemberAPISucceedChanged() - .withSuccessfulAddMemberToMLSGroup() .withInsertAddedAndFailedSystemMessageSuccess() .arrange() @@ -1236,11 +1236,11 @@ class ConversationGroupRepositoryTest { .withConversationDetailsById(TestConversation.MLS_CONVERSATION) .withProtocolInfoById(MLS_PROTOCOL_INFO) .withAddMemberAPISucceedChanged() + .withSuccessfulAddMemberToMLSGroup() .withAddingMemberToMlsGroupResults( buildCommitBundleFederatedFailure("otherDomain"), Either.Right(Unit) ) - .withSuccessfulAddMemberToMLSGroup() .withInsertAddedAndFailedSystemMessageSuccess() .arrange() @@ -1303,7 +1303,7 @@ class ConversationGroupRepositoryTest { verify(arrangement.mlsConversationRepository) .suspendFunction(arrangement.mlsConversationRepository::addMemberToMLSGroup) .with(anything(), matching { - it.size == initialCountUsers - 1 // removed 1 failed user with commit bundle federated error + it.size == initialCountUsers - 2 // removed 1 failed user with commit bundle federated error }).wasInvoked(exactly = once) verify(arrangement.newGroupConversationSystemMessagesCreator) From f48364d1a0979c7c8f6937732c916461879d7250 Mon Sep 17 00:00:00 2001 From: yamilmedina Date: Fri, 1 Dec 2023 15:18:57 +0100 Subject: [PATCH 28/30] fix: python dependency --- .github/workflows/gradle-ios-tests.yml | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/.github/workflows/gradle-ios-tests.yml b/.github/workflows/gradle-ios-tests.yml index 0e1ba96b857..2ea72196c36 100644 --- a/.github/workflows/gradle-ios-tests.yml +++ b/.github/workflows/gradle-ios-tests.yml @@ -29,6 +29,11 @@ jobs: distribution: 'temurin' # cache: gradle # Cache disabled as we use Gradle Setup below + - name: Set up Python 3.11 + - uses: actions/setup-python@v4 + with: + python-version: '3.11' + - name: Gradle Setup uses: gradle/gradle-build-action@v2 From 79326fec0e93afb476af0e69614bee10546574ca Mon Sep 17 00:00:00 2001 From: yamilmedina Date: Fri, 1 Dec 2023 15:31:21 +0100 Subject: [PATCH 29/30] fix: python dependency --- .github/workflows/gradle-ios-tests.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/gradle-ios-tests.yml b/.github/workflows/gradle-ios-tests.yml index 2ea72196c36..38a08015ba5 100644 --- a/.github/workflows/gradle-ios-tests.yml +++ b/.github/workflows/gradle-ios-tests.yml @@ -30,7 +30,7 @@ jobs: # cache: gradle # Cache disabled as we use Gradle Setup below - name: Set up Python 3.11 - - uses: actions/setup-python@v4 + uses: actions/setup-python@v4 with: python-version: '3.11' From 5c45a197724161a64578c489d9ce3a9e99d617a3 Mon Sep 17 00:00:00 2001 From: yamilmedina Date: Tue, 5 Dec 2023 15:37:08 +0100 Subject: [PATCH 30/30] feat: address pr comments --- .../logic/data/conversation/ConversationGroupRepository.kt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/conversation/ConversationGroupRepository.kt b/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/conversation/ConversationGroupRepository.kt index 42e4f390f80..82cfc94d214 100644 --- a/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/conversation/ConversationGroupRepository.kt +++ b/logic/src/commonMain/kotlin/com/wire/kalium/logic/data/conversation/ConversationGroupRepository.kt @@ -185,7 +185,8 @@ internal class ConversationGroupRepositoryImpl( is ConversationEntity.ProtocolInfo.Mixed -> tryAddMembersToCloudAndStorage(userIdList, conversationId) .flatMap { - tryAddMembersToMLSGroup(conversationId, protocol.groupId, userIdList) + // best effort approach for migrated conversations, no retries + mlsConversationRepository.addMemberToMLSGroup(GroupID(protocol.groupId), userIdList) } is ConversationEntity.ProtocolInfo.MLS -> {