Skip to content

Commit

Permalink
feat: fetch full list of team members when kalium consumers needs to (#…
Browse files Browse the repository at this point in the history
…2284)

* Revert "refactor: remove the fetch of full team members during slow sync (#2253)"

This reverts commit 1bdd10c.

* add flag to fetchAllTeamMembersEagerly

* add flag to fetchAllTeamMembersEagerly

* fetch all team members pages

* fetch all team members pages

* MayBeConst

* fix tests
  • Loading branch information
MohamadJaara authored Dec 5, 2023
1 parent b564ec6 commit 1ad916c
Show file tree
Hide file tree
Showing 7 changed files with 216 additions and 43 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ 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.functional.onFailure
import com.wire.kalium.logic.functional.onSuccess
import com.wire.kalium.logic.sync.receiver.handler.legalhold.LegalHoldHandler
import com.wire.kalium.logic.sync.receiver.handler.legalhold.LegalHoldRequestHandler
Expand All @@ -50,6 +51,7 @@ import kotlinx.coroutines.flow.map

interface TeamRepository {
suspend fun fetchTeamById(teamId: TeamId): Either<CoreFailure, Team>
suspend fun fetchMembersByTeamId(teamId: TeamId, userDomain: String): Either<CoreFailure, Unit>
suspend fun getTeam(teamId: TeamId): Flow<Team?>
suspend fun deleteConversation(conversationId: ConversationId, teamId: TeamId): Either<CoreFailure, Unit>
suspend fun updateMemberRole(teamId: String, userId: String, permissionCode: Int?): Either<CoreFailure, Unit>
Expand Down Expand Up @@ -81,11 +83,45 @@ internal class TeamDataSource(
}.map { teamDTO ->
teamMapper.fromDtoToEntity(teamDTO)
}.flatMap { teamEntity ->
wrapStorageRequest { teamDAO.insertTeam(team = teamEntity) }.map {
wrapStorageRequest {
teamDAO.insertTeam(teamEntity)
}.map {
teamMapper.fromDaoModelToTeam(teamEntity)
}
}

override suspend fun fetchMembersByTeamId(teamId: TeamId, userDomain: String): Either<CoreFailure, Unit> {
var hasMore = true
var error: CoreFailure? = null
while (hasMore && error == null) {
wrapApiRequest {
teamsApi.getTeamMembers(
teamId = teamId.value,
limitTo = FETCH_TEAM_MEMBER_PAGE_SIZE
)
}.onSuccess {
hasMore = it.hasMore
}.map {
it.members.map { teamMember ->
val userId = QualifiedIDEntity(teamMember.nonQualifiedUserId, userDomain)
val userType = userTypeEntityTypeMapper.teamRoleCodeToUserType(teamMember.permissions?.own)
userId to userType
}
}.flatMap { teamMembers ->
wrapStorageRequest {
userDAO.upsertTeamMemberUserTypes(teamMembers.toMap())
}
}.onFailure {
error = it
}
}
return if (error != null) {
Either.Left(error!!)
} else {
Either.Right(Unit)
}
}

override suspend fun getTeam(teamId: TeamId): Flow<Team?> =
teamDAO.getTeamById(teamId.value)
.map {
Expand Down Expand Up @@ -147,6 +183,7 @@ internal class TeamDataSource(
eventContentDTO = EventContentDTO.User.LegalHoldEnabledDTO(id = selfUserId.toString())
)
)

LegalHoldStatusDTO.DISABLED -> legalHoldHandler.handleDisable(
eventMapper.legalHoldDisabled(
id = LocalId.generate(),
Expand All @@ -155,6 +192,7 @@ internal class TeamDataSource(
eventContentDTO = EventContentDTO.User.LegalHoldDisabledDTO(id = selfUserId.toString())
)
)

LegalHoldStatusDTO.PENDING ->
legalHoldRequestHandler.handle(
eventMapper.legalHoldRequest(
Expand All @@ -168,7 +206,12 @@ internal class TeamDataSource(
)
)
)

LegalHoldStatusDTO.NO_CONSENT -> Either.Right(Unit)
}.map { legalHoldStatusMapper.fromApiModel(response.legalHoldStatusDTO) }
}

private companion object {
const val FETCH_TEAM_MEMBER_PAGE_SIZE = 200
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -875,7 +875,9 @@ class UserSessionScope internal constructor(

private val syncSelfTeamUseCase: SyncSelfTeamUseCase
get() = SyncSelfTeamUseCaseImpl(
userRepository = userRepository, teamRepository = teamRepository
userRepository = userRepository,
teamRepository = teamRepository,
fetchAllTeamMembersEagerly = kaliumConfigs.fetchAllTeamMembersEagerly
)

private val joinExistingMLSConversationUseCase: JoinExistingMLSConversationUseCase
Expand Down Expand Up @@ -1088,22 +1090,23 @@ class UserSessionScope internal constructor(
)
})

internal val mlsMigrationWorker get() =
MLSMigrationWorkerImpl(
userConfigRepository,
featureConfigRepository,
mlsConfigHandler,
mlsMigrationConfigHandler,
mlsMigrator,
)
internal val mlsMigrationWorker
get() =
MLSMigrationWorkerImpl(
userConfigRepository,
featureConfigRepository,
mlsConfigHandler,
mlsMigrationConfigHandler,
mlsMigrator,
)

internal val mlsMigrationManager: MLSMigrationManager = MLSMigrationManagerImpl(
kaliumConfigs,
featureSupport,
incrementalSyncRepository,
lazy { clientRepository },
lazy { users.timestampKeyRepository },
lazy { mlsMigrationWorker }
kaliumConfigs,
featureSupport,
incrementalSyncRepository,
lazy { clientRepository },
lazy { users.timestampKeyRepository },
lazy { mlsMigrationWorker }
)

private val mlsPublicKeysRepository: MLSPublicKeysRepository
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import com.wire.kalium.logic.CoreFailure
import com.wire.kalium.logic.data.team.TeamRepository
import com.wire.kalium.logic.data.user.UserRepository
import com.wire.kalium.logic.functional.Either
import com.wire.kalium.logic.functional.map
import com.wire.kalium.logic.functional.flatMap
import com.wire.kalium.logic.functional.onSuccess
import com.wire.kalium.logic.kaliumLogger
import kotlinx.coroutines.flow.first
Expand All @@ -34,18 +34,27 @@ internal interface SyncSelfTeamUseCase {

internal class SyncSelfTeamUseCaseImpl(
private val userRepository: UserRepository,
private val teamRepository: TeamRepository
private val teamRepository: TeamRepository,
private val fetchAllTeamMembersEagerly: Boolean
) : SyncSelfTeamUseCase {

override suspend fun invoke(): Either<CoreFailure, Unit> {
val user = userRepository.observeSelfUser().first()

return user.teamId?.let { teamId ->
teamRepository.fetchTeamById(teamId = teamId)
.map { }
.onSuccess {
teamRepository.syncServices(teamId = teamId)
teamRepository.fetchTeamById(teamId = teamId).flatMap {
if (fetchAllTeamMembersEagerly) {
kaliumLogger.withFeatureId(SYNC).i("Fetching all team members eagerly")
teamRepository.fetchMembersByTeamId(
teamId = teamId,
userDomain = user.id.domain
)
} else {
Either.Right(Unit)
}
}.onSuccess {
teamRepository.syncServices(teamId = teamId)
}
} ?: run {
kaliumLogger.withFeatureId(SYNC).i("Skipping team sync because user doesn't belong to a team")
Either.Right(Unit)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,8 @@ data class KaliumConfigs(
val kaliumMockEngine: KaliumMockEngine? = null,
val mockNetworkStateObserver: NetworkStateObserver? = null,
// Interval between attempts to advance the proteus to MLS migration
val mlsMigrationInterval: Duration = 24.hours
val mlsMigrationInterval: Duration = 24.hours,
val fetchAllTeamMembersEagerly: Boolean = false,
)

sealed interface BuildFileRestrictionState {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ import com.wire.kalium.persistence.dao.UserDAO
import com.wire.kalium.persistence.dao.unread.UserConfigDAO
import io.mockative.Mock
import io.mockative.any
import io.mockative.anything
import io.mockative.classOf
import io.mockative.configure
import io.mockative.eq
Expand All @@ -58,11 +59,13 @@ import io.mockative.mock
import io.mockative.once
import io.mockative.oneOf
import io.mockative.verify
import kotlinx.coroutines.ExperimentalCoroutinesApi
import kotlinx.coroutines.flow.flowOf
import kotlinx.coroutines.test.runTest
import kotlin.test.Test
import kotlin.test.assertEquals

@OptIn(ExperimentalCoroutinesApi::class)
class TeamRepositoryTest {
@Test
fun givenSelfUserExists_whenFetchingTeamInfo_thenTeamInfoShouldBeSuccessful() = runTest {
Expand Down Expand Up @@ -103,6 +106,52 @@ class TeamRepositoryTest {
}
}

@Test
fun givenTeamIdAndUserDomain_whenFetchingTeamMembers_thenTeamMembersShouldBeSuccessful() = runTest {
val teamMember = TestTeam.memberDTO(
nonQualifiedUserId = "teamMember1"
)

val teamMembersList = TeamsApi.TeamMemberList(
hasMore = false,
members = listOf(
teamMember
)
)

val (arrangement, teamRepository) = Arrangement()
.withGetTeamMembers(NetworkResponse.Success(teamMembersList, mapOf(), 200))
.arrange()

val result = teamRepository.fetchMembersByTeamId(teamId = TeamId("teamId"), userDomain = "userDomain")

// Verifies that userDAO insertUsers was called with the correct mapped values
verify(arrangement.userDAO)
.suspendFunction(arrangement.userDAO::upsertTeamMemberUserTypes)
.with(any())
.wasInvoked(exactly = once)

// Verifies that when fetching members by team id, it succeeded
result.shouldSucceed()
}

@Test
fun givenTeamApiFails_whenFetchingTeamMembers_thenTheFailureIsPropagated() = runTest {
val (arrangement, teamRepository) = Arrangement()
.arrange()

given(arrangement.teamsApi)
.suspendFunction(arrangement.teamsApi::getTeamMembers)
.whenInvokedWith(any(), anything())
.thenReturn(NetworkResponse.Error(KaliumException.ServerError(ErrorResponse(500, "error_message", "error_label"))))

val result = teamRepository.fetchMembersByTeamId(teamId = TeamId("teamId"), userDomain = "userDomain")

result.shouldFail {
assertEquals(it::class, NetworkFailure.ServerMiscommunication::class)
}
}

@Test
fun givenSelfUserExists_whenGettingTeamById_thenTeamDataShouldBePassed() = runTest {
val teamEntity = TeamEntity(id = "teamId", name = "teamName", icon = "icon")
Expand Down Expand Up @@ -364,13 +413,6 @@ class TeamRepositoryTest {
.then { NetworkResponse.Success(value = teamDTO, headers = mapOf(), httpCode = 200) }
}

fun withApiGetTeamMemberSuccess(teamMemberDTO: TeamsApi.TeamMemberDTO) = apply {
given(teamsApi)
.suspendFunction(teamsApi::getTeamMember)
.whenInvokedWith(any(), any())
.thenReturn(NetworkResponse.Success(value = teamMemberDTO, headers = mapOf(), httpCode = 200))
}

fun withFetchWhiteListedServicesSuccess() = apply {
given(teamsApi)
.suspendFunction(teamsApi::whiteListedServices)
Expand Down Expand Up @@ -407,6 +449,14 @@ class TeamRepositoryTest {
.thenReturn(Either.Right(Unit))
}

fun withGetTeamMembers(result: NetworkResponse<TeamsApi.TeamMemberList>) = apply {
given(teamsApi)
.suspendFunction(teamsApi::getTeamMembers)
.whenInvokedWith(any(), any())
.thenReturn(result)

}

fun arrange() = this to teamRepository

companion object {
Expand Down
Loading

0 comments on commit 1ad916c

Please sign in to comment.