Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: handle federation context for MLS add participants to a conversation (WPB-2229) #2275

Merged
merged 44 commits into from
Dec 6, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
44 commits
Select commit Hold shift + click to select a range
5f8d091
feat: preparing adding members for retry
yamilmedina Nov 15, 2023
56e3c78
chore: move forward with error handling on convo repo layer
yamilmedina Nov 17, 2023
e6a2299
chore: move forward with error handling on convo repo layer
yamilmedina Nov 17, 2023
da22b77
chore: move forward with error handling on convo repo layer
yamilmedina Nov 17, 2023
ecaa03a
chore: move forward with error handling on convo repo layer
yamilmedina Nov 17, 2023
37a8472
feat: add fold accumulation and bubble up failed users
yamilmedina Nov 20, 2023
bfc9c62
Merge branch 'develop' into feat/mls-addparticipants-federation
yamilmedina Nov 20, 2023
a98b0f3
feat: wrap result into dto to avoid calling api twice
yamilmedina Nov 20, 2023
35f3a5c
feat: wrap result into dto to avoid calling api twice
yamilmedina Nov 20, 2023
7c251b1
feat: renaming var
yamilmedina Nov 20, 2023
ee86ccf
feat: renaming var, comments
yamilmedina Nov 20, 2023
a9b382e
Merge branch 'develop' into feat/mls-addparticipants-federation
yamilmedina Nov 21, 2023
0398627
feat: preparing adding members for retry
yamilmedina Nov 22, 2023
aae7dfd
Merge branch 'develop' into feat/mls-addparticipants-federation
yamilmedina Nov 22, 2023
a64e2f3
feat: preparing adding members for retry
yamilmedina Nov 22, 2023
69b3c35
feat: add member logic receive preclaimed packages
yamilmedina Nov 22, 2023
4e7dc75
chore: add base for simpler version of failure, no retain preclaimed
yamilmedina Nov 23, 2023
9ed0328
chore: filtering failed users
yamilmedina Nov 23, 2023
ea2bf6d
chore: filtering failed users
yamilmedina Nov 23, 2023
69b61ef
Merge branch 'develop' into feat/mls-addparticipants-federation-v2
yamilmedina Nov 27, 2023
7839c8b
Merge branch 'develop' into feat/mls-addparticipants-federation-v2
yamilmedina Nov 28, 2023
f407aca
feat: persist system messages for failed and added, and handle retry
yamilmedina Nov 28, 2023
adf20c7
Merge branch 'develop' into feat/mls-addparticipants-federation-v2
yamilmedina Nov 28, 2023
0936e48
feat: persist system messages for failed and added, and handle retry
yamilmedina Nov 28, 2023
c2439e4
feat: persist system messages for failed and added, and handle retry
yamilmedina Nov 28, 2023
e1e075f
feat: persist system messages for failed and added, and handle retry
yamilmedina Nov 28, 2023
3638298
Merge branch 'develop' into feat/mls-addparticipants-federation-v2
yamilmedina Nov 28, 2023
b6be6cc
Merge branch 'develop' into feat/mls-addparticipants-federation-v2
yamilmedina Nov 29, 2023
af27f04
chore: refactor cleanuop
yamilmedina Nov 29, 2023
c1d93e3
feat: fixing tests
yamilmedina Nov 30, 2023
5d6060f
Merge branch 'develop' into feat/mls-addparticipants-federation-v2
yamilmedina Nov 30, 2023
3da38d1
feat: add test coverage for retry mec
yamilmedina Nov 30, 2023
775ed92
feat: add test coverage for retry bundle
yamilmedina Nov 30, 2023
ca9fd4b
chore: detekt
yamilmedina Nov 30, 2023
7302b9a
Merge branch 'develop' into feat/mls-addparticipants-federation-v2
yamilmedina Nov 30, 2023
1949985
chore: test cov
yamilmedina Dec 1, 2023
5a3a2f6
fix: testst
yamilmedina Dec 1, 2023
f48364d
fix: python dependency
yamilmedina Dec 1, 2023
79326fe
fix: python dependency
yamilmedina Dec 1, 2023
48aa010
Merge branch 'develop' into feat/mls-addparticipants-federation-v2
yamilmedina Dec 1, 2023
8c85adc
Merge branch 'develop' into feat/mls-addparticipants-federation-v2
yamilmedina Dec 1, 2023
71b467c
Merge branch 'develop' into feat/mls-addparticipants-federation-v2
yamilmedina Dec 4, 2023
00a1425
Merge branch 'develop' into feat/mls-addparticipants-federation-v2
yamilmedina Dec 5, 2023
5c45a19
feat: address pr comments
yamilmedina Dec 5, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -54,10 +54,10 @@ sealed interface CoreFailure {
data object MissingClientRegistration : CoreFailure

/**
* A user 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.
*/
data class NoKeyPackagesAvailable(val userId: UserId) : CoreFailure
data class NoKeyPackagesAvailable(val failedUserIds: Set<UserId>) : CoreFailure

/**
* It's not allowed to run the application with development API enabled when
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -185,15 +185,95 @@ internal class ConversationGroupRepositoryImpl(
is ConversationEntity.ProtocolInfo.Mixed ->
tryAddMembersToCloudAndStorage(userIdList, conversationId)
.flatMap {
// best effort approach for migrated conversations, no retries
mlsConversationRepository.addMemberToMLSGroup(GroupID(protocol.groupId), userIdList)
}

is ConversationEntity.ProtocolInfo.MLS -> {
mlsConversationRepository.addMemberToMLSGroup(GroupID(protocol.groupId), userIdList)
tryAddMembersToMLSGroup(conversationId, protocol.groupId, userIdList)
}
}
}

/**
* 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(
conversationId: ConversationId,
groupId: String,
userIdList: List<UserId>,
failedUsersList: Set<UserId> = emptySet(),
remainingAttempts: Int = 2
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion: can we extract this default value to the top of the class for example?

): Either<CoreFailure, Unit> {
return when (val addingMemberResult = mlsConversationRepository.addMemberToMLSGroup(GroupID(groupId), userIdList)) {
is Either.Right -> handleMLSMembersAdded(conversationId, userIdList, failedUsersList)
is Either.Left -> {
addingMemberResult.value.handleMLSMembersFailed(
conversationId = conversationId,
groupId = groupId,
userIdList = userIdList,
failedUsersList = failedUsersList,
remainingAttempts = remainingAttempts,
)
}
}
}

private suspend fun CoreFailure.handleMLSMembersFailed(
conversationId: ConversationId,
groupId: String,
userIdList: List<UserId>,
failedUsersList: Set<UserId>,
remainingAttempts: Int,
): Either<CoreFailure, Unit> {
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
)
}

// 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)
}
}
}
}

private suspend fun handleMLSMembersAdded(
conversationId: ConversationId,
userIdList: List<UserId>,
failedUsersList: Set<UserId>
): Either<CoreFailure, Unit> {
return newGroupConversationSystemMessagesCreator.value.conversationResolvedMembersAddedAndFailed(
conversationId.toDao(), userIdList, failedUsersList.toList()
).flatMap {
Either.Right(Unit)
}
}

override suspend fun addService(serviceId: ServiceId, conversationId: ConversationId): Either<CoreFailure, Unit> =
wrapStorageRequest { conversationDAO.getConversationProtocolInfo(conversationId.toDao()) }
.flatMap { protocol ->
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -59,7 +60,7 @@ internal class NewConversationMembersRepositoryImpl(
}.flatMap {
newGroupConversationSystemMessagesCreator.value.conversationResolvedMembersAddedAndFailed(
conversationId,
conversationResponse,
conversationResponse.members.otherMembers.map { it.id.toModel() },
failedUsersList
)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -47,7 +46,7 @@ internal interface NewGroupConversationSystemMessagesCreator {
suspend fun conversationReadReceiptStatus(conversation: ConversationResponse): Either<CoreFailure, Unit>
suspend fun conversationResolvedMembersAddedAndFailed(
conversationId: ConversationIDEntity,
conversationResponse: ConversationResponse,
validUsers: List<UserId>,
failedUsersList: List<UserId> = emptyList()
): Either<CoreFailure, Unit>

Expand All @@ -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 {
Expand Down Expand Up @@ -143,16 +141,14 @@ internal class NewGroupConversationSystemMessagesCreatorImpl(

override suspend fun conversationResolvedMembersAddedAndFailed(
conversationId: ConversationIDEntity,
conversationResponse: ConversationResponse,
validUsers: List<UserId>,
failedUsersList: List<UserId>
): Either<CoreFailure, Unit> = 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,
Expand All @@ -169,18 +165,22 @@ internal class NewGroupConversationSystemMessagesCreatorImpl(
override suspend fun conversationFailedToAddMembers(
conversationId: ConversationId,
userIdList: Set<UserId>
): Either<CoreFailure, Unit> {
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<CoreFailure, Unit> = run {
if (userIdList.isNotEmpty()) {
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<UserId>) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,14 +22,12 @@ 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.IdMapper
import com.wire.kalium.logic.data.id.CurrentClientIdProvider
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
Expand Down Expand Up @@ -58,25 +56,28 @@ 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<UserId>): Either<CoreFailure, List<KeyPackageDTO>> =
currentClientIdProvider().flatMap { selfClientId ->
userIds.map { userId ->
val failedUsers = mutableSetOf<UserId>()
val claimedKeyPackages = mutableListOf<KeyPackageDTO>()
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))
failedUsers.add(userId)
} else {
Either.Right(it.keyPackages)
claimedKeyPackages.addAll(it.keyPackages)
}
}
}.foldToEitherWhileRight(emptyList()) { item, acc ->
item.flatMap { Either.Right(acc + it) }
}

if (failedUsers.isNotEmpty()) {
Either.Left(CoreFailure.NoKeyPackagesAvailable(failedUsers))
} else {
Either.Right(claimedKeyPackages)
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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() }
)
newGroupConversationSystemMessagesCreator.conversationReadReceiptStatus(event.conversation)
newGroupConversationSystemMessagesCreator.conversationStartedUnverifiedWarning(event.conversation.id.toModel())
Expand Down
Loading
Loading