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

refactor: update the list of users ID when adding a new members fails with 403 #2267

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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 @@ -248,7 +248,6 @@ internal class UserDataSource internal constructor(
.flatMap { persistUsers(listOf(userProfileDTO), it) }
}

@Suppress("MagicNumber")
override suspend fun fetchUsersByIds(qualifiedUserIdList: Set<UserId>): Either<CoreFailure, Unit> =
if (qualifiedUserIdList.isEmpty()) {
Either.Right(Unit)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1549,7 +1549,6 @@ class UserSessionScope internal constructor(
messages.sendConfirmation,
renamedConversationHandler,
qualifiedIdMapper,
team.isSelfATeamMember,
globalScope.serverConfigRepository,
userStorage,
userPropertyRepository,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,15 @@
package com.wire.kalium.logic.feature.conversation

import com.wire.kalium.logic.CoreFailure
import com.wire.kalium.logic.NetworkFailure
import com.wire.kalium.logic.data.conversation.ConversationGroupRepository
import com.wire.kalium.logic.data.id.ConversationId
import com.wire.kalium.logic.data.user.UserId
import com.wire.kalium.logic.data.user.UserRepository
import com.wire.kalium.logic.functional.fold
import com.wire.kalium.logic.functional.onFailure
import com.wire.kalium.network.exceptions.KaliumException
import io.ktor.http.HttpStatusCode

/**
* This use case will add a member(s) to a given conversation.
Expand All @@ -42,13 +47,31 @@ interface AddMemberToConversationUseCase {
}

internal class AddMemberToConversationUseCaseImpl(
private val conversationGroupRepository: ConversationGroupRepository
private val conversationGroupRepository: ConversationGroupRepository,
private val userRepository: UserRepository
) : AddMemberToConversationUseCase {
override suspend fun invoke(conversationId: ConversationId, userIdList: List<UserId>): AddMemberToConversationUseCase.Result {
return conversationGroupRepository.addMembers(userIdList, conversationId).fold({
AddMemberToConversationUseCase.Result.Failure(it)
}, {
AddMemberToConversationUseCase.Result.Success
})
return conversationGroupRepository.addMembers(userIdList, conversationId)
.onFailure {
when (it) {
is NetworkFailure.ServerMiscommunication -> {
if (it.kaliumException is KaliumException.InvalidRequestError &&
it.kaliumException.errorResponse.code == HttpStatusCode.Forbidden.value) {
handle403Error(userIdList)
}
}

else -> { /* do nothing */ }
}
}
.fold({
AddMemberToConversationUseCase.Result.Failure(it)
}, {
AddMemberToConversationUseCase.Result.Success
})
}

private suspend fun handle403Error(userIdList: List<UserId>) {
userRepository.fetchUsersByIds(userIdList.toSet())
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,6 @@ import com.wire.kalium.logic.feature.team.DeleteTeamConversationUseCase
import com.wire.kalium.logic.feature.team.DeleteTeamConversationUseCaseImpl
import com.wire.kalium.logic.feature.team.GetSelfTeamUseCase
import com.wire.kalium.logic.feature.team.GetSelfTeamUseCaseImpl
import com.wire.kalium.logic.feature.user.IsSelfATeamMemberUseCase
import com.wire.kalium.logic.sync.SyncManager
import com.wire.kalium.logic.sync.receiver.conversation.RenamedConversationEventHandler
import com.wire.kalium.logic.sync.receiver.handler.CodeUpdateHandlerImpl
Expand All @@ -89,10 +88,9 @@ class ConversationScope internal constructor(
private val sendConfirmation: SendConfirmationUseCase,
private val renamedConversationHandler: RenamedConversationEventHandler,
private val qualifiedIdMapper: QualifiedIdMapper,
private val isSelfATeamMember: IsSelfATeamMemberUseCase,
private val serverConfigRepository: ServerConfigRepository,
private val userStorage: UserStorage,
private val userPropertyRepository: UserPropertyRepository,
userPropertyRepository: UserPropertyRepository,
private val oneOnOneResolver: OneOnOneResolver,
private val scope: CoroutineScope
) {
Expand Down Expand Up @@ -154,7 +152,7 @@ class ConversationScope internal constructor(
)

val addMemberToConversationUseCase: AddMemberToConversationUseCase
get() = AddMemberToConversationUseCaseImpl(conversationGroupRepository)
get() = AddMemberToConversationUseCaseImpl(conversationGroupRepository, userRepository)

val addServiceToConversationUseCase: AddServiceToConversationUseCase
get() = AddServiceToConversationUseCase(groupRepository = conversationGroupRepository)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,15 @@
package com.wire.kalium.logic.feature.conversation

import com.wire.kalium.logic.CoreFailure
import com.wire.kalium.logic.NetworkFailure
import com.wire.kalium.logic.StorageFailure
import com.wire.kalium.logic.data.conversation.ConversationGroupRepository
import com.wire.kalium.logic.framework.TestConversation
import com.wire.kalium.logic.functional.Either
import com.wire.kalium.logic.util.arrangement.UserRepositoryArrangement
import com.wire.kalium.logic.util.arrangement.UserRepositoryArrangementImpl
import com.wire.kalium.network.api.base.model.ErrorResponse
import com.wire.kalium.network.exceptions.KaliumException
import io.mockative.Mock
import io.mockative.any
import io.mockative.classOf
Expand Down Expand Up @@ -70,12 +75,45 @@ class AddMemberToConversationUseCaseTest {
.wasInvoked(exactly = once)
}

private class Arrangement {
@Test
fun givenServerResponseWith403_whenAddingToGroupConversionFail_thenUpdateUsersInfo() = runTest {

val error = NetworkFailure.ServerMiscommunication(
KaliumException.InvalidRequestError(
errorResponse = ErrorResponse(
code = 403,
message = "Forbidden",
label = "Forbidden"
)
)
)
val (arrangement, addMemberUseCase) = Arrangement()
.arrange {
withAddMembers(Either.Left(error))
withFetchUsersByIdReturning(Either.Right(Unit))
}

val result = addMemberUseCase(TestConversation.ID, listOf(TestConversation.USER_1))
assertIs<AddMemberToConversationUseCase.Result.Failure>(result)

verify(arrangement.conversationGroupRepository)
.suspendFunction(arrangement.conversationGroupRepository::addMembers)
.with(eq(listOf(TestConversation.USER_1)), eq(TestConversation.ID))
.wasInvoked(exactly = once)

verify(arrangement.userRepository)
.suspendFunction(arrangement.userRepository::fetchUsersByIds)
.with(eq(setOf(TestConversation.USER_1)))
.wasInvoked(exactly = once)
}

private class Arrangement : UserRepositoryArrangement by UserRepositoryArrangementImpl() {
@Mock
val conversationGroupRepository = mock(classOf<ConversationGroupRepository>())

private val addMemberUseCase = AddMemberToConversationUseCaseImpl(
conversationGroupRepository
conversationGroupRepository,
userRepository
)

fun withAddMembers(either: Either<CoreFailure, Unit>) = apply {
Expand All @@ -85,6 +123,6 @@ class AddMemberToConversationUseCaseTest {
.thenReturn(either)
}

fun arrange() = this to addMemberUseCase
fun arrange(block: Arrangement.() -> Unit = { }) = apply(block).let { this to addMemberUseCase }
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,13 @@ package com.wire.kalium.logic.util.arrangement
import com.wire.kalium.logic.CoreFailure
import com.wire.kalium.logic.data.user.OtherUser
import com.wire.kalium.logic.data.user.SelfUser
import com.wire.kalium.logic.data.user.UserId
import com.wire.kalium.logic.data.user.UserRepository
import com.wire.kalium.logic.functional.Either
import io.mockative.Mock
import io.mockative.any
import io.mockative.given
import io.mockative.matchers.Matcher
import io.mockative.mock
import kotlinx.coroutines.flow.Flow

Expand All @@ -50,6 +52,11 @@ internal interface UserRepositoryArrangement {
fun withFetchAllOtherUsersReturning(result: Either<CoreFailure, Unit>)

fun withFetchUserInfoReturning(result: Either<CoreFailure, Unit>)

fun withFetchUsersByIdReturning(
result: Either<CoreFailure, Unit>,
userIdList: Matcher<Set<UserId>> = any()
)
}

internal class UserRepositoryArrangementImpl: UserRepositoryArrangement {
Expand Down Expand Up @@ -121,4 +128,15 @@ internal class UserRepositoryArrangementImpl: UserRepositoryArrangement {
.thenReturn(result)
}

override fun withFetchUsersByIdReturning(
result: Either<CoreFailure, Unit>,
userIdList: Matcher<Set<UserId>>
) {
given(userRepository)
.suspendFunction(userRepository::fetchUsersByIds)
.whenInvokedWith(userIdList)
.thenReturn(result)
}


}
Loading