Improve handling members

This commit is contained in:
ganfra
2023-04-20 18:21:47 +02:00
parent df4c9b3cf7
commit f02ee307cc
15 changed files with 107 additions and 139 deletions

View File

@@ -41,7 +41,6 @@ import io.element.android.libraries.di.SessionScope
import io.element.android.libraries.matrix.api.room.MatrixRoom
import io.element.android.libraries.matrix.api.room.RoomMembershipObserver
import io.element.android.services.appnavstate.api.AppNavigationStateService
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.flow.filter
import kotlinx.coroutines.flow.launchIn
import kotlinx.coroutines.flow.onEach
@@ -102,12 +101,18 @@ class RoomFlowNode @AssistedInject constructor(
private fun fetchRoomMembers() = lifecycleScope.launch {
val room = inputs.room
/*
room.fetchMembers()
.map {
room.updateMembers()
}
.onFailure {
Timber.e(it, "Fail to fetch members for room ${room.roomId}")
}.onSuccess {
Timber.v("Success fetching members for room ${room.roomId}")
}
*/
}
override fun resolve(navTarget: NavTarget, buildContext: BuildContext): Node {

View File

@@ -24,7 +24,6 @@ import com.bumble.appyx.core.node.node
import com.bumble.appyx.core.plugin.Plugin
import com.bumble.appyx.navmodel.backstack.activeElement
import com.bumble.appyx.testing.junit4.util.MainDispatcherRule
import com.bumble.appyx.testing.unit.common.helper.nodeTestHelper
import com.bumble.appyx.testing.unit.common.helper.parentNodeTestHelper
import com.google.common.truth.Truth
import io.element.android.features.messages.api.MessagesEntryPoint
@@ -81,19 +80,6 @@ class RoomFlowNodeTest {
roomMembershipObserver = RoomMembershipObserver()
)
@Test
fun `given a room flow node when initialized then it fetches room members`() {
// GIVEN
val room = FakeMatrixRoom()
val inputs = RoomFlowNode.Inputs(room)
val roomFlowNode = aRoomFlowNode(listOf(inputs))
Truth.assertThat(room.areMembersFetched).isFalse()
// WHEN
roomFlowNode.nodeTestHelper()
// THEN
Truth.assertThat(room.areMembersFetched).isTrue()
}
@Test
fun `given a room flow node when initialized then it loads messages entry point`() {
// GIVEN

View File

@@ -102,7 +102,7 @@ class CreateRoomRootPresenterTests {
}.test {
val initialState = awaitItem()
val matrixUser = MatrixUser(UserId("@name:domain"))
val fakeDmResult = FakeMatrixRoom(RoomId("!fakeDmResult:domain"))
val fakeDmResult = FakeMatrixRoom(roomId = RoomId("!fakeDmResult:domain"))
fakeMatrixClient.givenFindDmResult(fakeDmResult)

View File

@@ -25,6 +25,7 @@ import io.element.android.libraries.designsystem.components.avatar.AvatarSize
import io.element.android.libraries.matrix.api.timeline.MatrixTimelineItem
import io.element.android.libraries.matrix.api.timeline.item.event.ProfileTimelineDetails
import kotlinx.collections.immutable.toImmutableList
import timber.log.Timber
import javax.inject.Inject
class TimelineItemEventFactory @Inject constructor(
@@ -42,6 +43,7 @@ class TimelineItemEventFactory @Inject constructor(
val senderDisplayName: String?
val senderAvatarUrl: String?
Timber.v("SenderProfile($currentSender) = ${currentTimelineItem.event.senderProfile}")
when (val senderProfile = currentTimelineItem.event.senderProfile) {
ProfileTimelineDetails.Unavailable,
ProfileTimelineDetails.Pending,

View File

@@ -18,6 +18,7 @@ package io.element.android.features.roomdetails.impl
import androidx.compose.runtime.Composable
import androidx.compose.runtime.LaunchedEffect
import androidx.compose.runtime.MutableState
import androidx.compose.runtime.getValue
import androidx.compose.runtime.mutableStateOf
import androidx.compose.runtime.remember
@@ -25,17 +26,16 @@ import androidx.compose.runtime.rememberCoroutineScope
import androidx.compose.runtime.setValue
import io.element.android.libraries.architecture.Async
import io.element.android.libraries.architecture.Presenter
import io.element.android.libraries.matrix.api.MatrixClient
import io.element.android.libraries.matrix.api.core.SessionId
import io.element.android.libraries.architecture.executeResult
import io.element.android.libraries.matrix.api.room.MatrixRoom
import io.element.android.libraries.matrix.api.room.RoomMembershipObserver
import io.element.android.libraries.matrix.api.room.getDmMember
import io.element.android.libraries.matrix.api.room.memberCount
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.launch
import kotlinx.coroutines.withContext
import javax.inject.Inject
class RoomDetailsPresenter @Inject constructor(
private val sessionId: SessionId,
private val room: MatrixRoom,
private val roomMembershipObserver: RoomMembershipObserver,
) : Presenter<RoomDetailsState> {
@@ -50,15 +50,14 @@ class RoomDetailsPresenter @Inject constructor(
mutableStateOf<RoomDetailsError?>(null)
}
var memberCount: Async<Int> by remember { mutableStateOf(Async.Loading()) }
val memberCount: MutableState<Async<Int>> = remember {
mutableStateOf(Async.Uninitialized)
}
LaunchedEffect(Unit) {
withContext(Dispatchers.IO) {
memberCount = runCatching { room.memberCount() }
.fold(
onSuccess = { Async.Success(it) },
onFailure = { Async.Failure(it) }
)
}
suspend {
room.updateMembers()
.map { room.memberCount() }
}.executeResult(memberCount)
}
val dmMember = room.getDmMember()
@@ -72,7 +71,7 @@ class RoomDetailsPresenter @Inject constructor(
when (event) {
is RoomDetailsEvent.LeaveRoom -> {
if (event.needsConfirmation) {
leaveRoomWarning = LeaveRoomWarning.computeLeaveRoomWarning(room.isPublic, memberCount)
leaveRoomWarning = LeaveRoomWarning.computeLeaveRoomWarning(room.isPublic, memberCount.value)
} else {
coroutineScope.launch(Dispatchers.IO) {
room.leave()
@@ -96,7 +95,7 @@ class RoomDetailsPresenter @Inject constructor(
roomAlias = room.alias,
roomAvatarUrl = room.avatarUrl,
roomTopic = room.topic,
memberCount = memberCount,
memberCount = memberCount.value,
isEncrypted = room.isEncrypted,
displayLeaveRoomWarning = leaveRoomWarning,
error = error,

View File

@@ -44,15 +44,6 @@ interface RoomMemberBindsModule {
@ContributesTo(RoomScope::class)
object RoomMemberProvidesModule {
@Provides
fun provideRoomDetailsPresenter(
matrixClient: MatrixClient,
room: MatrixRoom,
roomMembershipObserver: RoomMembershipObserver,
): RoomDetailsPresenter {
return RoomDetailsPresenter(matrixClient.sessionId, room, roomMembershipObserver)
}
@Provides
fun provideRoomMemberDetailsPresenterFactory(
room: MatrixRoom,

View File

@@ -28,6 +28,7 @@ import io.element.android.anvilannotations.ContributesNode
import io.element.android.libraries.di.RoomScope
import io.element.android.libraries.matrix.api.room.MatrixRoom
import io.element.android.libraries.matrix.api.room.RoomMember
import io.element.android.libraries.matrix.api.room.getMember
import io.element.android.libraries.matrix.ui.model.MatrixUser
import timber.log.Timber

View File

@@ -30,7 +30,7 @@ class RoomUserListDataSource @Inject constructor(
) : UserListDataSource {
override suspend fun search(query: String): List<MatrixUser> {
return room.members().filter { member ->
return room.membersFlow.value.filter { member ->
if (query.isBlank()) {
true
} else {

View File

@@ -32,7 +32,6 @@ import io.element.android.libraries.matrix.api.room.RoomMembershipState
import io.element.android.libraries.matrix.api.timeline.item.event.MembershipChange
import io.element.android.libraries.matrix.test.A_ROOM_ID
import io.element.android.libraries.matrix.test.A_ROOM_NAME
import io.element.android.libraries.matrix.test.A_SESSION_ID
import io.element.android.libraries.matrix.test.A_USER_ID
import io.element.android.libraries.matrix.test.room.FakeMatrixRoom
import kotlinx.coroutines.ExperimentalCoroutinesApi
@@ -50,7 +49,7 @@ class RoomDetailsPresenterTests {
@Test
fun `present - initial state is created from room info`() = runTest {
val room = aMatrixRoom()
val presenter = RoomDetailsPresenter(A_SESSION_ID, room, roomMembershipObserver)
val presenter = RoomDetailsPresenter(room, roomMembershipObserver)
moleculeFlow(RecompositionClock.Immediate) {
presenter.present()
}.test {
@@ -59,7 +58,7 @@ class RoomDetailsPresenterTests {
Truth.assertThat(initialState.roomName).isEqualTo(room.name)
Truth.assertThat(initialState.roomAvatarUrl).isEqualTo(room.avatarUrl)
Truth.assertThat(initialState.roomTopic).isEqualTo(room.topic)
Truth.assertThat(initialState.memberCount).isEqualTo(Async.Loading(null))
Truth.assertThat(initialState.memberCount).isEqualTo(Async.Uninitialized)
Truth.assertThat(initialState.isEncrypted).isEqualTo(room.isEncrypted)
cancelAndIgnoreRemainingEvents()
@@ -69,12 +68,12 @@ class RoomDetailsPresenterTests {
@Test
fun `present - room member count is calculated asynchronously`() = runTest {
val room = aMatrixRoom()
val presenter = RoomDetailsPresenter(A_SESSION_ID, room, roomMembershipObserver)
val presenter = RoomDetailsPresenter(room, roomMembershipObserver)
moleculeFlow(RecompositionClock.Immediate) {
presenter.present()
}.test {
val initialState = awaitItem()
Truth.assertThat(initialState.memberCount).isEqualTo(Async.Loading(null))
Truth.assertThat(initialState.memberCount).isEqualTo(Async.Uninitialized)
val finalState = awaitItem()
Truth.assertThat(finalState.memberCount).isEqualTo(Async.Success(0))
@@ -84,7 +83,7 @@ class RoomDetailsPresenterTests {
@Test
fun `present - initial state with no room name`() = runTest {
val room = aMatrixRoom(name = null)
val presenter = RoomDetailsPresenter(A_SESSION_ID, room, roomMembershipObserver)
val presenter = RoomDetailsPresenter(room, roomMembershipObserver)
moleculeFlow(RecompositionClock.Immediate) {
presenter.present()
}.test {
@@ -100,7 +99,7 @@ class RoomDetailsPresenterTests {
val room = aMatrixRoom(name = null).apply {
givenFetchMemberResult(Result.failure(Throwable()))
}
val presenter = RoomDetailsPresenter(A_SESSION_ID, room, roomMembershipObserver)
val presenter = RoomDetailsPresenter(room, roomMembershipObserver)
moleculeFlow(RecompositionClock.Immediate) {
presenter.present()
}.test {
@@ -114,7 +113,7 @@ class RoomDetailsPresenterTests {
@Test
fun `present - Leave with confirmation on private room shows a specific warning`() = runTest {
val room = aMatrixRoom(isPublic = false)
val presenter = RoomDetailsPresenter(A_SESSION_ID, room, roomMembershipObserver)
val presenter = RoomDetailsPresenter(room, roomMembershipObserver)
moleculeFlow(RecompositionClock.Immediate) {
presenter.present()
}.test {
@@ -131,7 +130,7 @@ class RoomDetailsPresenterTests {
@Test
fun `present - Leave with confirmation on empty room shows a specific warning`() = runTest {
val room = aMatrixRoom(members = listOf(aRoomMember()))
val presenter = RoomDetailsPresenter(A_SESSION_ID, room, roomMembershipObserver)
val presenter = RoomDetailsPresenter(room, roomMembershipObserver)
moleculeFlow(RecompositionClock.Immediate) {
presenter.present()
}.test {
@@ -148,7 +147,7 @@ class RoomDetailsPresenterTests {
@Test
fun `present - Leave with confirmation shows a generic warning`() = runTest {
val room = aMatrixRoom()
val presenter = RoomDetailsPresenter(A_SESSION_ID, room, roomMembershipObserver)
val presenter = RoomDetailsPresenter(room, roomMembershipObserver)
moleculeFlow(RecompositionClock.Immediate) {
presenter.present()
}.test {
@@ -165,7 +164,7 @@ class RoomDetailsPresenterTests {
@Test
fun `present - Leave without confirmation leaves the room`() = runTest {
val room = aMatrixRoom()
val presenter = RoomDetailsPresenter(A_SESSION_ID, room, roomMembershipObserver)
val presenter = RoomDetailsPresenter(room, roomMembershipObserver)
moleculeFlow(RecompositionClock.Immediate) {
presenter.present()
}.test {
@@ -189,7 +188,7 @@ class RoomDetailsPresenterTests {
val room = aMatrixRoom().apply {
givenLeaveRoomError(Throwable())
}
val presenter = RoomDetailsPresenter(A_SESSION_ID, room, roomMembershipObserver)
val presenter = RoomDetailsPresenter(room, roomMembershipObserver)
moleculeFlow(RecompositionClock.Immediate) {
presenter.present()
}.test {
@@ -218,10 +217,10 @@ fun aMatrixRoom(
) = FakeMatrixRoom(
roomId = roomId,
name = name,
initialMembers = members,
displayName = displayName,
topic = topic,
avatarUrl = avatarUrl,
members = members,
isEncrypted = isEncrypted,
isPublic = isPublic,
)

View File

@@ -18,12 +18,15 @@ package io.element.android.libraries.matrix.api.room
import io.element.android.libraries.matrix.api.core.EventId
import io.element.android.libraries.matrix.api.core.RoomId
import io.element.android.libraries.matrix.api.core.SessionId
import io.element.android.libraries.matrix.api.core.UserId
import io.element.android.libraries.matrix.api.timeline.MatrixTimeline
import kotlinx.coroutines.flow.Flow
import kotlinx.coroutines.flow.StateFlow
import java.io.Closeable
interface MatrixRoom: Closeable {
interface MatrixRoom : Closeable {
val sessionId: SessionId
val roomId: RoomId
val name: String?
val bestName: String
@@ -36,20 +39,22 @@ interface MatrixRoom: Closeable {
val isDirect: Boolean
val isPublic: Boolean
suspend fun members() : List<RoomMember>
/**
* The current loaded members as a StateFlow.
* Initial value is an emptyList.
* To update them you should call [updateMembers].
*/
val membersFlow: StateFlow<List<RoomMember>>
suspend fun memberCount(): Int
fun getMember(userId: UserId): RoomMember?
fun getDmMember(): RoomMember?
/**
* Try to load the room members and update the membersFlow.
*/
suspend fun updateMembers(): Result<Unit>
fun syncUpdateFlow(): Flow<Long>
fun timeline(): MatrixTimeline
suspend fun fetchMembers(): Result<Unit>
suspend fun userDisplayName(userId: UserId): Result<String?>
suspend fun userAvatarUrl(userId: UserId): Result<String?>
@@ -64,3 +69,19 @@ interface MatrixRoom: Closeable {
suspend fun leave(): Result<Unit>
}
fun MatrixRoom.getMember(userId: UserId): RoomMember? {
return membersFlow.value.find { it.userId == userId }
}
fun MatrixRoom.getDmMember(): RoomMember? {
return if (membersFlow.value.size == 2 && isDirect && isEncrypted) {
membersFlow.value.find { it.userId != this.sessionId }
} else {
null
}
}
fun MatrixRoom.memberCount(): Int {
return membersFlow.value.size
}

View File

@@ -198,7 +198,7 @@ class RustMatrixClient constructor(
val slidingSyncRoom = slidingSync.getRoom(roomId.value) ?: return null
val fullRoom = slidingSyncRoom.fullRoom() ?: return null
return RustMatrixRoom(
currentUserId = sessionId,
sessionId = sessionId,
slidingSyncUpdateFlow = slidingSyncObserverProxy.updateSummaryFlow,
slidingSyncRoom = slidingSyncRoom,
innerRoom = fullRoom,

View File

@@ -17,21 +17,21 @@
package io.element.android.libraries.matrix.impl.room
import io.element.android.libraries.core.coroutine.CoroutineDispatchers
import io.element.android.libraries.core.data.tryOrNull
import io.element.android.libraries.matrix.api.core.EventId
import io.element.android.libraries.matrix.api.core.RoomId
import io.element.android.libraries.matrix.api.core.SessionId
import io.element.android.libraries.matrix.api.core.UserId
import io.element.android.libraries.matrix.api.room.MatrixRoom
import io.element.android.libraries.matrix.api.room.RoomMember
import io.element.android.libraries.matrix.api.timeline.MatrixTimeline
import io.element.android.libraries.matrix.impl.timeline.RustMatrixTimeline
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.Job
import kotlinx.coroutines.flow.Flow
import kotlinx.coroutines.flow.MutableStateFlow
import kotlinx.coroutines.flow.StateFlow
import kotlinx.coroutines.flow.filter
import kotlinx.coroutines.flow.map
import kotlinx.coroutines.flow.onStart
import kotlinx.coroutines.launch
import kotlinx.coroutines.withContext
import org.matrix.rustcomponents.sdk.Room
import org.matrix.rustcomponents.sdk.SlidingSyncRoom
@@ -40,7 +40,7 @@ import org.matrix.rustcomponents.sdk.genTransactionId
import org.matrix.rustcomponents.sdk.messageEventContentFromMarkdown
class RustMatrixRoom(
private val currentUserId: UserId,
override val sessionId: SessionId,
private val slidingSyncUpdateFlow: Flow<UpdateSummary>,
private val slidingSyncRoom: SlidingSyncRoom,
private val innerRoom: Room,
@@ -48,39 +48,10 @@ class RustMatrixRoom(
private val coroutineDispatchers: CoroutineDispatchers,
) : MatrixRoom {
private var loadMembersJob: Job? = null
private var cachedMembers: List<RoomMember> = emptyList()
override val membersFlow: StateFlow<List<RoomMember>>
get() = cachedMembers
override suspend fun members(): List<RoomMember> {
return cachedMembers.ifEmpty {
if (loadMembersJob == null) {
loadMembersJob = coroutineScope.launch(coroutineDispatchers.io) {
cachedMembers = tryOrNull {
innerRoom.members().map(RoomMemberMapper::map)
} ?: emptyList()
}
}
loadMembersJob?.join()
loadMembersJob = null
cachedMembers
}
}
override suspend fun memberCount(): Int {
return members().size
}
override fun getMember(userId: UserId): RoomMember? {
return cachedMembers.find { it.userId == userId }
}
override fun getDmMember(): RoomMember? {
return if (cachedMembers.size == 2 && isDirect && isEncrypted) {
cachedMembers.find { it.userId != currentUserId }
} else {
null
}
}
private var cachedMembers = MutableStateFlow<List<RoomMember>>(emptyList())
override fun syncUpdateFlow(): Flow<Long> {
return slidingSyncUpdateFlow
@@ -150,9 +121,9 @@ class RustMatrixRoom(
override val isDirect: Boolean
get() = innerRoom.isDirect()
override suspend fun fetchMembers(): Result<Unit> = withContext(coroutineDispatchers.io) {
override suspend fun updateMembers(): Result<Unit> = withContext(coroutineDispatchers.io) {
runCatching {
innerRoom.fetchMembers()
cachedMembers.value = innerRoom.members().map(RoomMemberMapper::map)
}
}

View File

@@ -139,10 +139,24 @@ class RustMatrixTimeline(
private suspend fun addListener(timelineListener: TimelineListener): Result<List<TimelineItem>> = withContext(coroutineDispatchers.io) {
runCatching {
val settings = RoomSubscription(requiredState = listOf(RequiredState(key = "m.room.canonical_alias", value = "")), timelineLimit = null)
val settings = RoomSubscription(
requiredState = listOf(
RequiredState(key = "m.room.canonical_alias", value = ""),
RequiredState(key = "m.room.topic", value = ""),
RequiredState(key = "m.room.join_rule", value = ""),
),
timelineLimit = 20.toUInt()
)
val result = slidingSyncRoom.subscribeAndAddTimelineListener(timelineListener, settings)
fetchMembers()
listenerTokens += result.taskHandle
result.items
}
}
private suspend fun fetchMembers() = withContext(coroutineDispatchers.io) {
runCatching {
innerRoom.fetchMembers()
}
}
}

View File

@@ -54,7 +54,7 @@ class FakeMatrixClient(
private var logoutFailure: Throwable? = null
override fun getRoom(roomId: RoomId): MatrixRoom? {
return FakeMatrixRoom(roomId)
return FakeMatrixRoom(sessionId = sessionId, roomId = roomId)
}
override fun findDM(userId: UserId): MatrixRoom? {

View File

@@ -18,17 +18,21 @@ package io.element.android.libraries.matrix.test.room
import io.element.android.libraries.matrix.api.core.EventId
import io.element.android.libraries.matrix.api.core.RoomId
import io.element.android.libraries.matrix.api.core.SessionId
import io.element.android.libraries.matrix.api.core.UserId
import io.element.android.libraries.matrix.api.room.MatrixRoom
import io.element.android.libraries.matrix.api.room.RoomMember
import io.element.android.libraries.matrix.test.A_ROOM_ID
import io.element.android.libraries.matrix.test.timeline.FakeMatrixTimeline
import io.element.android.libraries.matrix.api.timeline.MatrixTimeline
import io.element.android.libraries.matrix.test.A_ROOM_ID
import io.element.android.libraries.matrix.test.A_SESSION_ID
import io.element.android.libraries.matrix.test.timeline.FakeMatrixTimeline
import kotlinx.coroutines.delay
import kotlinx.coroutines.flow.Flow
import kotlinx.coroutines.flow.MutableStateFlow
import kotlinx.coroutines.flow.emptyFlow
class FakeMatrixRoom(
override val sessionId: SessionId = A_SESSION_ID,
override val roomId: RoomId = A_ROOM_ID,
override val name: String? = null,
override val bestName: String = "",
@@ -40,20 +44,23 @@ class FakeMatrixRoom(
override val alternativeAliases: List<String> = emptyList(),
override val isPublic: Boolean = true,
override val isDirect: Boolean = false,
private val members: List<RoomMember> = emptyList(),
initialMembers: List<RoomMember> = emptyList(),
private val matrixTimeline: MatrixTimeline = FakeMatrixTimeline(),
) : MatrixRoom {
private var userDisplayNameResult = Result.success<String?>(null)
private var userAvatarUrlResult = Result.success<String?>(null)
private var dmMember: RoomMember? = null
private var fetchMemberResult: Result<Unit> = Result.success(Unit)
var areMembersFetched: Boolean = false
private set
private var updateMembersResult: Result<Unit> = Result.success(Unit)
private var leaveRoomError: Throwable? = null
override val membersFlow: MutableStateFlow<List<RoomMember>> = MutableStateFlow(initialMembers)
override suspend fun updateMembers(): Result<Unit> {
return updateMembersResult
}
override fun syncUpdateFlow(): Flow<Long> {
return emptyFlow()
}
@@ -62,18 +69,6 @@ class FakeMatrixRoom(
return matrixTimeline
}
override suspend fun fetchMembers(): Result<Unit> {
return fetchMemberResult.also { result ->
if (result.isSuccess) {
areMembersFetched = true
}
}
}
override fun getDmMember(): RoomMember? {
return dmMember
}
override suspend fun userDisplayName(userId: UserId): Result<String?> {
return userDisplayNameResult
}
@@ -82,22 +77,6 @@ class FakeMatrixRoom(
return userAvatarUrlResult
}
override suspend fun members(): List<RoomMember> {
return members
}
override suspend fun memberCount(): Int {
if (fetchMemberResult.isSuccess) {
return members.count()
} else {
throw fetchMemberResult.exceptionOrNull()!!
}
}
override fun getMember(userId: UserId): RoomMember? {
return members.firstOrNull { it.userId == userId }
}
override suspend fun sendMessage(message: String): Result<Unit> {
delay(100)
return Result.success(Unit)
@@ -139,7 +118,7 @@ class FakeMatrixRoom(
}
fun givenFetchMemberResult(result: Result<Unit>) {
fetchMemberResult = result
updateMembersResult = result
}
fun givenDmMember(roomMember: RoomMember) {