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

fix(rc): persist proper team members user types [WPB-5343] #2211

Merged
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 @@ -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))
}
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,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 All @@ -71,7 +72,6 @@ import com.wire.kalium.persistence.dao.client.ClientDAO
import com.wire.kalium.util.DateTimeUtil
import io.ktor.util.collections.ConcurrentMap
import kotlinx.coroutines.ExperimentalCoroutinesApi
import kotlinx.coroutines.FlowPreview
import kotlinx.coroutines.flow.Flow
import kotlinx.coroutines.flow.filterNotNull
import kotlinx.coroutines.flow.firstOrNull
Expand Down Expand Up @@ -141,6 +141,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 @@ -217,7 +218,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 @@ -263,38 +267,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 @@ -304,7 +328,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 Expand Up @@ -339,7 +369,7 @@ internal class UserDataSource internal constructor(
}
}

@OptIn(FlowPreview::class)
@OptIn(ExperimentalCoroutinesApi::class)
override suspend fun observeSelfUserWithTeam(): Flow<Pair<SelfUser, Team?>> {
return metadataDAO.valueByKeyFlow(SELF_USER_ID_KEY).filterNotNull().flatMapMerge { encodedValue ->
val selfUserID: QualifiedIDEntity = Json.decodeFromString(encodedValue)
Expand Down
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
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -639,6 +639,7 @@ class UserSessionScope internal constructor(
userStorage.database.clientDAO,
authenticatedNetworkContainer.selfApi,
authenticatedNetworkContainer.userDetailsApi,
authenticatedNetworkContainer.teamsApi,
globalScope.sessionRepository,
userId,
qualifiedIdMapper,
Expand Down Expand Up @@ -696,6 +697,7 @@ class UserSessionScope internal constructor(
userStorage.database.userDAO,
userStorage.database.metadataDAO,
authenticatedNetworkContainer.userDetailsApi,
authenticatedNetworkContainer.teamsApi,
userSearchApiWrapper
)

Expand Down
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
Loading
Loading