Skip to content

Commit

Permalink
fix: persist proper team members user types [WPB-5343]
Browse files Browse the repository at this point in the history
  • Loading branch information
@saleniuk
saleniuk committed Nov 8, 2023
1 parent d77445c commit f7b7a81
Show file tree
Hide file tree
Showing 11 changed files with 225 additions and 62 deletions.
74 changes: 45 additions & 29 deletions logic/src/commonMain/kotlin/com/wire/kalium/logic/data/publicuser/SearchUserRepository.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,11 +27,13 @@ import com.wire.kalium.logic.data.user.SelfUser
import com.wire.kalium.logic.data.user.UserDataSource
import com.wire.kalium.logic.data.user.UserMapper
import com.wire.kalium.logic.data.user.type.DomainUserTypeMapper
import com.wire.kalium.logic.data.user.type.UserEntityTypeMapper
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.map
import com.wire.kalium.logic.wrapApiRequest
import com.wire.kalium.network.api.base.authenticated.TeamsApi
import com.wire.kalium.network.api.base.authenticated.userDetails.ListUserRequest
import com.wire.kalium.network.api.base.authenticated.userDetails.UserDetailsApi
import com.wire.kalium.network.api.base.authenticated.userDetails.qualifiedIds
Expand All @@ -41,7 +43,6 @@ import com.wire.kalium.persistence.dao.MetadataDAO
import com.wire.kalium.persistence.dao.QualifiedIDEntity
import com.wire.kalium.persistence.dao.UserDAO
import com.wire.kalium.persistence.dao.UserEntity
import com.wire.kalium.persistence.dao.UserTypeEntity
import kotlinx.coroutines.ExperimentalCoroutinesApi
import kotlinx.coroutines.flow.Flow
import kotlinx.coroutines.flow.filterNotNull
Expand Down Expand Up @@ -93,10 +94,12 @@ internal class SearchUserRepositoryImpl(
private val userDAO: UserDAO,
private val metadataDAO: MetadataDAO,
private val userDetailsApi: UserDetailsApi,
private val teamsApi: TeamsApi,
private val userSearchAPiWrapper: UserSearchApiWrapper,
private val publicUserMapper: PublicUserMapper = MapperProvider.publicUserMapper(),
private val userMapper: UserMapper = MapperProvider.userMapper(),
private val userTypeMapper: DomainUserTypeMapper = MapperProvider.userTypeMapper()
private val userTypeMapper: DomainUserTypeMapper = MapperProvider.userTypeMapper(),
private val userEntityTypeMapper: UserEntityTypeMapper = MapperProvider.userTypeEntityMapper(),
) : SearchUserRepository {

override suspend fun searchKnownUsersByNameOrHandleOrEmail(
Expand Down Expand Up @@ -152,42 +155,56 @@ internal class SearchUserRepositoryImpl(
searchUsersOptions
).flatMap {
val qualifiedIdList = it.documents.map { it.qualifiedID }
val response =
val usersResponse =
if (qualifiedIdList.isEmpty()) Either.Right(listOf())
else wrapApiRequest {
userDetailsApi.getMultipleUsers(ListUserRequest.qualifiedIds(qualifiedIdList))
}.map { listUsersDTO -> listUsersDTO.usersFound }
response.map { userProfileDTOList ->
val otherUserList = if (userProfileDTOList.isEmpty()) emptyList() else {
val selfUser = getSelfUser()
val (teamMembers, otherUsers) = userProfileDTOList
.partition { it.isTeamMember(selfUser.teamId?.value, selfUser.id.domain) }

// We need to store all found team members locally and not return them as they will be "known" users from now on.
userDAO.upsertTeamMembers(
teamMembers.map { userProfileDTO ->
userMapper.fromUserProfileDtoToUserEntity(
userProfile = userProfileDTO,
connectionState = ConnectionEntity.State.ACCEPTED,
userTypeEntity = UserTypeEntity.STANDARD
)
}
)
usersResponse.flatMap { userProfileDTOList ->
if (userProfileDTOList.isEmpty())
return Either.Right(UserSearchResult(emptyList()))

val selfUser = getSelfUser()
val (teamMembers, otherUsers) = userProfileDTOList
.partition { it.isTeamMember(selfUser.teamId?.value, selfUser.id.domain) }
val teamMembersResponse =
if (selfUser.teamId == null || teamMembers.isEmpty()) Either.Right(emptyMap())
else wrapApiRequest {
teamsApi.getTeamMembersByIds(selfUser.teamId.value, TeamsApi.TeamMemberIdList(teamMembers.map { it.id.value }))
}.map { teamMemberList -> teamMemberList.members.associateBy { it.nonQualifiedUserId } }

otherUsers.map { userProfileDTO ->
publicUserMapper.fromUserProfileDtoToOtherUser(
userDetailResponse = userProfileDTO,
userType = userTypeMapper.fromTeamAndDomain(
otherUserDomain = userProfileDTO.id.domain,
selfUserTeamId = selfUser.teamId?.value,
otherUserTeamId = userProfileDTO.teamId,
selfUserDomain = selfUser.id.domain,
isService = userProfileDTO.service != null,
teamMembersResponse.map { teamMemberMap ->
// We need to store all found team members locally and not return them as they will be "known" users from now on.
teamMembers.map { userProfileDTO ->
userMapper.fromUserProfileDtoToUserEntity(
userProfile = userProfileDTO,
connectionState = ConnectionEntity.State.ACCEPTED,
userTypeEntity = userEntityTypeMapper.teamRoleCodeToUserType(
permissionCode = teamMemberMap[userProfileDTO.id.value]?.permissions?.own,
isService = userProfileDTO.service != null
)
)
}.let {
userDAO.upsertTeamMembers(it)
userDAO.upsertTeamMembersTypes(it)
}

UserSearchResult(
otherUsers.map { userProfileDTO ->
publicUserMapper.fromUserProfileDtoToOtherUser(
userDetailResponse = userProfileDTO,
userType = userTypeMapper.fromTeamAndDomain(
otherUserDomain = userProfileDTO.id.domain,
selfUserTeamId = selfUser.teamId?.value,
otherUserTeamId = userProfileDTO.teamId,
selfUserDomain = selfUser.id.domain,
isService = userProfileDTO.service != null,
)
)
}
)
}
UserSearchResult(otherUserList)
}
}

Expand Down Expand Up @@ -221,5 +238,4 @@ internal class SearchUserRepositoryImpl(
UserSearchResult(it.map(publicUserMapper::fromUserEntityToOtherUser))
}
}

}
63 changes: 47 additions & 16 deletions logic/src/commonMain/kotlin/com/wire/kalium/logic/data/user/UserRepository.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ import com.wire.kalium.logic.functional.mapRight
import com.wire.kalium.logic.kaliumLogger
import com.wire.kalium.logic.wrapApiRequest
import com.wire.kalium.logic.wrapStorageRequest
import com.wire.kalium.network.api.base.authenticated.TeamsApi
import com.wire.kalium.network.api.base.authenticated.self.SelfApi
import com.wire.kalium.network.api.base.authenticated.userDetails.ListUserRequest
import com.wire.kalium.network.api.base.authenticated.userDetails.ListUsersDTO
Expand Down Expand Up @@ -138,6 +139,7 @@ internal class UserDataSource internal constructor(
private val clientDAO: ClientDAO,
private val selfApi: SelfApi,
private val userDetailsApi: UserDetailsApi,
private val teamsApi: TeamsApi,
private val sessionRepository: SessionRepository,
private val selfUserId: UserId,
private val qualifiedIdMapper: QualifiedIdMapper,
Expand Down Expand Up @@ -209,7 +211,10 @@ internal class UserDataSource internal constructor(

override suspend fun fetchUserInfo(userId: UserId) =
wrapApiRequest { userDetailsApi.getUserInfo(userId.toApi()) }
.flatMap { userProfileDTO -> persistUsers(listOf(userProfileDTO)) }
.flatMap { userProfileDTO ->
fetchTeamMembersByIds(listOf(userProfileDTO))
.flatMap { persistUsers(listOf(userProfileDTO), it) }
}

@Suppress("MagicNumber")
override suspend fun fetchUsersByIds(qualifiedUserIdList: Set<UserId>): Either<CoreFailure, Unit> =
Expand Down Expand Up @@ -237,38 +242,58 @@ internal class UserDataSource internal constructor(
kaliumLogger.d("Handling ${listUserProfileDTO.usersFailed.size} failed users")
persistIncompleteUsers(listUserProfileDTO.usersFailed)
}
persistUsers(listUserProfileDTO.usersFound)
fetchTeamMembersByIds(listUserProfileDTO.usersFound)
.flatMap { persistUsers(listUserProfileDTO.usersFound, it) }
}
}

private suspend fun fetchTeamMembersByIds(userProfileList: List<UserProfileDTO>): Either<CoreFailure, List<TeamsApi.TeamMemberDTO>> {
val selfUserDomain = selfUserId.domain
val selfUserTeamId = selfTeamIdProvider().getOrNull()
val teamMemberIds = userProfileList.filter { it.isTeamMember(selfUserTeamId?.value, selfUserDomain) }.map { it.id.value }
return if (selfUserTeamId == null || teamMemberIds.isEmpty()) Either.Right(emptyList())
else teamMemberIds
.chunked(BATCH_SIZE)
.foldToEitherWhileRight(emptyList()) { chunk, acc ->
wrapApiRequest {
kaliumLogger.d("Fetching ${chunk.size} team members")
teamsApi.getTeamMembersByIds(selfUserTeamId.value, TeamsApi.TeamMemberIdList(chunk))
}.map {
kaliumLogger.d("Found ${it.members.size} team members")
(acc + it.members).distinct()
}
}
}

private suspend fun persistIncompleteUsers(usersFailed: List<NetworkQualifiedId>) = wrapStorageRequest {
usersFailed.map { userMapper.fromFailedUserToEntity(it) }.forEach {
userDAO.insertUser(it)
}
}

private suspend fun persistUsers(listUserProfileDTO: List<UserProfileDTO>) = wrapStorageRequest {
val selfUserDomain = selfUserId.domain
private suspend fun persistUsers(
listUserProfileDTO: List<UserProfileDTO>,
listTeamMemberDTO: List<TeamsApi.TeamMemberDTO>
) = wrapStorageRequest {
val mapTeamMemberDTO = listTeamMemberDTO.associateBy { it.nonQualifiedUserId }
val selfUserTeamId = selfTeamIdProvider().getOrNull()?.value
val teamMembers = listUserProfileDTO
.filter { userProfileDTO -> userProfileDTO.isTeamMember(selfUserTeamId, selfUserDomain) }
val otherUsers = listUserProfileDTO
.filter { userProfileDTO -> !userProfileDTO.isTeamMember(selfUserTeamId, selfUserDomain) }
userDAO.upsertTeamMembers(
teamMembers.map { userProfileDTO ->
.filter { userProfileDTO -> mapTeamMemberDTO.containsKey(userProfileDTO.id.value) }
.map { userProfileDTO ->
userMapper.fromUserProfileDtoToUserEntity(
userProfile = userProfileDTO,
connectionState = ConnectionEntity.State.ACCEPTED,
userTypeEntity = UserTypeEntity.STANDARD
userTypeEntity =
if (userProfileDTO.service != null) UserTypeEntity.SERVICE
else userTypeEntityMapper.teamRoleCodeToUserType(mapTeamMemberDTO[userProfileDTO.id.value]?.permissions?.own)
)
}
)

userDAO.upsertUsers(
otherUsers.map { userProfileDTO ->
val otherUsers = listUserProfileDTO
.filter { userProfileDTO -> !mapTeamMemberDTO.containsKey(userProfileDTO.id.value) }
.map { userProfileDTO ->
userMapper.fromUserProfileDtoToUserEntity(
userProfile = userProfileDTO,
connectionState = ConnectionEntity.State.NOT_CONNECTED,
connectionState = ConnectionEntity.State.NOT_CONNECTED, // this won't be updated, just to avoid a null value
userTypeEntity = userTypeEntityMapper.fromTeamAndDomain(
otherUserDomain = userProfileDTO.id.domain,
selfUserTeamId = selfUserTeamId,
Expand All @@ -278,7 +303,13 @@ internal class UserDataSource internal constructor(
)
)
}
)
if (teamMembers.isNotEmpty()) {
userDAO.upsertTeamMembers(teamMembers)
userDAO.upsertTeamMembersTypes(teamMembers)
}
if (otherUsers.isNotEmpty()) {
userDAO.upsertUsers(otherUsers)
}
}

override suspend fun fetchUsersIfUnknownByIds(ids: Set<UserId>): Either<CoreFailure, Unit> = wrapStorageRequest {
Expand Down
19 changes: 10 additions & 9 deletions logic/src/commonMain/kotlin/com/wire/kalium/logic/data/user/type/UserTypeMapper.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -129,13 +129,14 @@ interface UserTypeMapper<T> {

private fun selfUserIsTeamMember(selfUserTeamId: String?) = selfUserTeamId != null

fun teamRoleCodeToUserType(permissionCode: Int?): T = when (permissionCode) {
TeamRole.ExternalPartner.value -> external
TeamRole.Member.value -> standard
TeamRole.Admin.value -> admin
TeamRole.Owner.value -> owner
null -> standard
else -> guest
}

fun teamRoleCodeToUserType(permissionCode: Int?, isService: Boolean = false): T =
if (isService) service
else when (permissionCode) {
TeamRole.ExternalPartner.value -> external
TeamRole.Member.value -> standard
TeamRole.Admin.value -> admin
TeamRole.Owner.value -> owner
null -> standard
else -> guest
}
}
2 changes: 2 additions & 0 deletions logic/src/commonMain/kotlin/com/wire/kalium/logic/feature/UserSessionScope.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -637,6 +637,7 @@ class UserSessionScope internal constructor(
userStorage.database.clientDAO,
authenticatedNetworkContainer.selfApi,
authenticatedNetworkContainer.userDetailsApi,
authenticatedNetworkContainer.teamsApi,
globalScope.sessionRepository,
userId,
qualifiedIdMapper,
Expand Down Expand Up @@ -694,6 +695,7 @@ class UserSessionScope internal constructor(
userStorage.database.userDAO,
userStorage.database.metadataDAO,
authenticatedNetworkContainer.userDetailsApi,
authenticatedNetworkContainer.teamsApi,
userSearchApiWrapper
)

Expand Down
31 changes: 29 additions & 2 deletions ...c/src/commonTest/kotlin/com/wire/kalium/logic/data/publicuser/SearchUserRepositoryTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ package com.wire.kalium.logic.data.publicuser

import com.wire.kalium.logic.NetworkFailure
import com.wire.kalium.logic.data.id.ConversationId
import com.wire.kalium.logic.data.id.IdMapper
import com.wire.kalium.logic.data.id.QualifiedID
import com.wire.kalium.logic.data.id.TeamId
import com.wire.kalium.logic.data.publicuser.model.UserSearchResult
Expand All @@ -33,6 +32,7 @@ import com.wire.kalium.logic.data.user.type.DomainUserTypeMapper
import com.wire.kalium.logic.data.user.type.UserType
import com.wire.kalium.logic.functional.Either
import com.wire.kalium.logic.test_util.TestNetworkResponseError
import com.wire.kalium.network.api.base.authenticated.TeamsApi
import com.wire.kalium.network.api.base.authenticated.search.ContactDTO
import com.wire.kalium.network.api.base.authenticated.search.SearchPolicyDTO
import com.wire.kalium.network.api.base.authenticated.search.UserSearchResponse
Expand Down Expand Up @@ -69,7 +69,6 @@ import kotlin.test.assertIs
import kotlin.test.assertTrue
import com.wire.kalium.network.api.base.model.UserId as UserIdDTO

// TODO: refactor to arrangement pattern
@OptIn(ExperimentalCoroutinesApi::class)
class SearchUserRepositoryTest {

Expand Down Expand Up @@ -337,9 +336,14 @@ class SearchUserRepositoryTest {
usersFailed = emptyList(),
usersFound = listOf(USER_PROFILE_DTO.copy(id = UserId("teamUser", SELF_USER.id.domain), teamId = SELF_USER.teamId?.value),)
)
val teamMembersResponse = TeamsApi.TeamMemberList(
hasMore = false,
members = listOf(TEAM_MEMBER_DTO.copy(nonQualifiedUserId = "teamUser"))
)
val (arrangement, searchUserRepository) = Arrangement()
.withSearchResult(Either.Right(CONTACT_SEARCH_RESPONSE))
.withGetMultipleUsersResult(NetworkResponse.Success(userListResponse, mapOf(), 200))
.withGetTeamMembersByIdsResult(NetworkResponse.Success(teamMembersResponse, mapOf(), 200))
.withValueByKeyFlowResult(flowOf(JSON_QUALIFIED_ID))
.withGetUserByQualifiedIdResult(flowOf(USER_ENTITY))
.withFromUserEntityToSelfUser(SELF_USER)
Expand All @@ -360,6 +364,10 @@ class SearchUserRepositoryTest {
.suspendFunction(arrangement.userDAO::upsertTeamMembers)
.with(eq(listOf(USER_ENTITY)))
.wasInvoked()
verify(arrangement.userDAO)
.suspendFunction(arrangement.userDAO::upsertTeamMembersTypes)
.with(eq(listOf(USER_ENTITY)))
.wasInvoked()
}

internal class Arrangement {
Expand All @@ -370,6 +378,9 @@ class SearchUserRepositoryTest {
@Mock
internal val userDetailsApi: UserDetailsApi = mock(classOf<UserDetailsApi>())

@Mock
internal val teamsApi: TeamsApi = mock(classOf<TeamsApi>())

@Mock
internal val userSearchApiWrapper: UserSearchApiWrapper = mock(classOf<UserSearchApiWrapper>())

Expand All @@ -390,6 +401,7 @@ class SearchUserRepositoryTest {
userDAO,
metadataDAO,
userDetailsApi,
teamsApi,
userSearchApiWrapper,
publicUserMapper,
userMapper,
Expand Down Expand Up @@ -420,6 +432,13 @@ class SearchUserRepositoryTest {
.thenReturn(result)
}

fun withGetTeamMembersByIdsResult(result: NetworkResponse<TeamsApi.TeamMemberList>) = apply {
given(teamsApi)
.suspendFunction(teamsApi::getTeamMembersByIds)
.whenInvokedWith(any())
.thenReturn(result)
}

fun withFromUserProfileDtoToOtherUserResult(result: OtherUser) = apply {
given(publicUserMapper)
.function(publicUserMapper::fromUserProfileDtoToOtherUser)
Expand Down Expand Up @@ -570,6 +589,14 @@ class SearchUserRepositoryTest {
service = null
)

val TEAM_MEMBER_DTO = TeamsApi.TeamMemberDTO(
nonQualifiedUserId = "value",
createdBy = null,
legalHoldStatus = null,
createdAt = null,
permissions = null,
)

val USER_RESPONSE = ListUsersDTO(usersFailed = emptyList(), usersFound = listOf(USER_PROFILE_DTO))

const val JSON_QUALIFIED_ID = """{"value":"test" , "domain":"test" }"""
Expand Down
8 changes: 8 additions & 0 deletions logic/src/commonTest/kotlin/com/wire/kalium/logic/data/user/DomainUserTypeMapperTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,14 @@ class DomainUserTypeMapperTest {
assertEquals(UserType.INTERNAL, result)
}

@Test
fun givenServiceTeamMember_whenMappingToConversationDetails_ThenConversationDetailsUserTypeIsService() {
// when
val result = userTypeMapper.teamRoleCodeToUserType(TeamRole.Member.value, true)
// then
assertEquals(UserType.SERVICE, result)
}

@Test
fun givenCommonNotWireDomainAndDifferentTeam_whenMappingToConversationDetails_ThenConversationDetailsUserTypeIsFederated() {
// given
Expand Down
Loading

0 comments on commit f7b7a81

Please sign in to comment.