Skip to content

Commit

Permalink
fix: get identities only for newly emitted members [WPB-8753]
Browse files Browse the repository at this point in the history
  • Loading branch information
@saleniuk
saleniuk committed Apr 26, 2024
1 parent 413a82b commit a123580
Show file tree
Hide file tree
Showing 2 changed files with 114 additions and 4 deletions.
20 changes: 17 additions & 3 deletions ...e/conversations/details/participants/usecase/ObserveParticipantsForConversationUseCase.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,16 @@ import com.wire.kalium.logic.data.conversation.Conversation.Member
import com.wire.kalium.logic.data.id.ConversationId
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.type.UserType
import com.wire.kalium.logic.feature.conversation.ObserveConversationMembersUseCase
import com.wire.kalium.logic.feature.e2ei.CertificateStatus
import com.wire.kalium.logic.feature.e2ei.usecase.GetMembersE2EICertificateStatusesUseCase
import kotlinx.coroutines.flow.Flow
import kotlinx.coroutines.flow.drop
import kotlinx.coroutines.flow.flowOn
import kotlinx.coroutines.flow.map
import kotlinx.coroutines.flow.scan
import javax.inject.Inject

class ObserveParticipantsForConversationUseCase @Inject constructor(
Expand All @@ -50,7 +54,9 @@ class ObserveParticipantsForConversationUseCase @Inject constructor(
isAdmin && !isService
}
}
.map { sortedMemberList ->
.scan(
ConversationParticipantsData() to emptyMap<UserId, CertificateStatus?>()
) { (_, previousMlsVerificationMap), sortedMemberList ->
val allAdminsWithoutServices = sortedMemberList.getOrDefault(true, listOf())
val visibleAdminsWithoutServices = allAdminsWithoutServices.limit(limit)
val allParticipants = sortedMemberList.getOrDefault(false, listOf())
Expand All @@ -59,7 +65,13 @@ class ObserveParticipantsForConversationUseCase @Inject constructor(
val visibleUserIds = visibleParticipants.map { it.userId }
.plus(visibleAdminsWithoutServices.map { it.userId })

val mlsVerificationMap = getMembersE2EICertificateStatuses(conversationId, visibleUserIds)
// only fetch certificate statuses for newly emitted users and get the rest from previous iterations
val newlyEmittedVisibleUserIds = visibleUserIds - previousMlsVerificationMap.keys
val mlsVerificationMap = previousMlsVerificationMap.plus(
if (newlyEmittedVisibleUserIds.isEmpty()) emptyMap()
else getMembersE2EICertificateStatuses(conversationId, newlyEmittedVisibleUserIds)
)

ConversationParticipantsData(
admins = visibleAdminsWithoutServices
.map { uiParticipantMapper.toUIParticipant(it.user, mlsVerificationMap[it.user.id]) },
Expand All @@ -68,8 +80,10 @@ class ObserveParticipantsForConversationUseCase @Inject constructor(
allAdminsCount = allAdminsWithoutServices.size,
allParticipantsCount = allParticipants.size,
isSelfAnAdmin = allAdminsWithoutServices.any { it.user is SelfUser }
)
) to mlsVerificationMap
}
.drop(1) // ignore the initial value from scan
.map { (data, _) -> data }
.flowOn(dispatchers.io())

private fun <T> List<T>.limit(limit: Int = -1) = when {
Expand Down
98 changes: 97 additions & 1 deletion ...nversations/details/participants/usecase/ObserveParticipantsForConversationUseCaseTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,16 +27,19 @@ import com.wire.android.util.ui.WireSessionImageLoader
import com.wire.kalium.logic.data.conversation.Conversation.Member
import com.wire.kalium.logic.data.conversation.MemberDetails
import com.wire.kalium.logic.data.id.ConversationId
import com.wire.kalium.logic.data.user.UserId
import com.wire.kalium.logic.data.user.type.UserType
import com.wire.kalium.logic.feature.conversation.ObserveConversationMembersUseCase
import com.wire.kalium.logic.feature.e2ei.usecase.GetMembersE2EICertificateStatusesUseCase
import io.mockk.MockKAnnotations
import io.mockk.coEvery
import io.mockk.coVerify
import io.mockk.impl.annotations.MockK
import kotlinx.coroutines.ExperimentalCoroutinesApi
import kotlinx.coroutines.channels.Channel
import kotlinx.coroutines.flow.consumeAsFlow
import kotlinx.coroutines.flow.flowOf
import kotlinx.coroutines.test.advanceUntilIdle
import kotlinx.coroutines.test.runTest
import org.amshove.kluent.internal.assertEquals
import org.junit.jupiter.api.Test
Expand Down Expand Up @@ -82,6 +85,99 @@ class ObserveParticipantsForConversationUseCaseTest {
assert(data.allParticipantsCount == members.size)
}
}

@Test
fun givenGroup_whenVisibleMembersListChanges_thenGetE2EICertificateOnlyForNewOnes() = runTest {
// Given
val members: List<MemberDetails> = buildList {
for (i in 1..6) {
add(MemberDetails(testOtherUser(i).copy(userType = UserType.INTERNAL), Member.Role.Member))
}
}
val members1 = members.subList(0, 4)
val members2 = members.subList(4, 6)
val (arrangement, useCase) = ObserveParticipantsForConversationUseCaseArrangement()
.withConversationParticipantsUpdate(members1)
.arrange()
// When - Then
useCase(ConversationId("", "")).test {
val result1 = awaitItem()
assertEquals(result1.participants.size, members1.size)
assertEquals(result1.allParticipantsCount, members1.size)
// emit updated members list with new members that should also be visible because there is no limit set
arrangement.withConversationParticipantsUpdate(members1 + members2)
advanceUntilIdle()
val result2 = awaitItem()
assertEquals(result2.participants.size, members1.size + members2.size)
assertEquals(result2.allParticipantsCount, members1.size + members2.size)

// certificate statuses are fetched once for the initial visible members list
coVerify(exactly = 1) { arrangement.getMembersE2EICertificateStatuses(any(), eq(members1.map { it.user.id })) }
// and then second time only for newly emitted visible members
coVerify(exactly = 1) { arrangement.getMembersE2EICertificateStatuses(any(), eq(members2.map { it.user.id })) }
}
}

@Test
fun givenGroup_whenVisibleMembersDoesNotChange_thenDoNotGetE2EICertificateForNonVisibleMembers() = runTest {
// Given
val members: List<MemberDetails> = buildList {
for (i in 1..6) {
add(MemberDetails(testOtherUser(i).copy(userType = UserType.INTERNAL), Member.Role.Member))
}
}
val limit = 4
val members1 = members.subList(0, limit)
val members2 = members.subList(limit, limit + 2)
val (arrangement, useCase) = ObserveParticipantsForConversationUseCaseArrangement()
.withConversationParticipantsUpdate(members1)
.arrange()
// When - Then
useCase(ConversationId("", ""), limit = limit).test {
val result1 = awaitItem()
assertEquals(result1.participants.size, members1.size)
assertEquals(result1.allParticipantsCount, members1.size)
// emit updated members list with new members that should not be visible because there is no limit set
arrangement.withConversationParticipantsUpdate(members1 + members2)
advanceUntilIdle()
val result2 = awaitItem()
assertEquals(result2.participants.size, members1.size)
assertEquals(result2.allParticipantsCount, members1.size + members2.size)

// certificate statuses are fetched once for the initial visible members list
coVerify(exactly = 1) { arrangement.getMembersE2EICertificateStatuses(any(), eq(members1.map { it.user.id })) }
// and then newly emitted members are non-visible (because of the limit) and should not be fetched
coVerify(exactly = 0) { arrangement.getMembersE2EICertificateStatuses(any(), eq(members2.map { it.user.id })) }
}
}

@Test
fun givenGroup_whenVisibleMembersDoesNotChange_thenDoNotGetE2EICertificateAgainForTheSameList() = runTest {
// Given
val members: List<MemberDetails> = buildList {
for (i in 1..6) {
add(MemberDetails(testOtherUser(i).copy(userType = UserType.INTERNAL), Member.Role.Member))
}
}
val (arrangement, useCase) = ObserveParticipantsForConversationUseCaseArrangement()
.withConversationParticipantsUpdate(members)
.arrange()
// When - Then
useCase(ConversationId("", "")).test {
val result1 = awaitItem()
assertEquals(result1.participants.size, members.size)
assertEquals(result1.allParticipantsCount, members.size)
// emit the same members list again
arrangement.withConversationParticipantsUpdate(members)
advanceUntilIdle()
val result2 = awaitItem()
assertEquals(result2.participants.size, members.size)
assertEquals(result2.allParticipantsCount, members.size)

// executed only once for visible members list even if the same list was emitted twice
coVerify(exactly = 1) { arrangement.getMembersE2EICertificateStatuses(any(), eq(members.map { it.user.id })) }
}
}
}

internal class ObserveParticipantsForConversationUseCaseArrangement {
Expand Down Expand Up @@ -110,7 +206,7 @@ internal class ObserveParticipantsForConversationUseCaseArrangement {
MockKAnnotations.init(this, relaxUnitFun = true)
// Default empty values
coEvery { observeConversationMembersUseCase(any()) } returns flowOf()
coEvery { getMembersE2EICertificateStatuses(any(), any()) } returns mapOf()
coEvery { getMembersE2EICertificateStatuses(any(), any()) } answers { secondArg<List<UserId>>().associateWith { null } }
}

suspend fun withConversationParticipantsUpdate(members: List<MemberDetails>): ObserveParticipantsForConversationUseCaseArrangement {
Expand Down

0 comments on commit a123580

Please sign in to comment.