diff --git a/appnav/src/main/kotlin/io/element/android/appnav/room/RoomLoadedFlowNode.kt b/appnav/src/main/kotlin/io/element/android/appnav/room/RoomLoadedFlowNode.kt index d6ea4fc533..21e412ef63 100644 --- a/appnav/src/main/kotlin/io/element/android/appnav/room/RoomLoadedFlowNode.kt +++ b/appnav/src/main/kotlin/io/element/android/appnav/room/RoomLoadedFlowNode.kt @@ -118,14 +118,7 @@ class RoomLoadedFlowNode @AssistedInject constructor( } private fun fetchRoomMembers() = lifecycleScope.launch { - val room = inputs.room - room.updateMembers() - .onFailure { - Timber.e(it, "Fail to fetch members for room ${room.roomId}") - } - .onSuccess { - Timber.v("Success fetching members for room ${room.roomId}") - } + inputs.room.updateMembers() } private fun createRoomDetailsNode(buildContext: BuildContext, initialTarget: RoomDetailsEntryPoint.InitialTarget): Node { diff --git a/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/RoomDetailsPresenter.kt b/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/RoomDetailsPresenter.kt index c832cab666..a232d4e87e 100644 --- a/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/RoomDetailsPresenter.kt +++ b/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/RoomDetailsPresenter.kt @@ -26,11 +26,13 @@ import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.produceState import androidx.compose.runtime.remember import androidx.compose.runtime.rememberCoroutineScope +import androidx.lifecycle.Lifecycle import io.element.android.features.leaveroom.api.LeaveRoomEvent import io.element.android.features.leaveroom.api.LeaveRoomPresenter import io.element.android.features.roomdetails.impl.members.details.RoomMemberDetailsPresenter import io.element.android.libraries.architecture.Presenter import io.element.android.libraries.core.coroutine.CoroutineDispatchers +import io.element.android.libraries.designsystem.utils.OnLifecycleEvent import io.element.android.libraries.featureflag.api.FeatureFlagService import io.element.android.libraries.featureflag.api.FeatureFlags import io.element.android.libraries.matrix.api.MatrixClient @@ -76,7 +78,13 @@ class RoomDetailsPresenter @Inject constructor( room.updateRoomNotificationSettings() observeNotificationSettings() } - room.updateMembers() + } + + // Update room members only when first presenting the node + OnLifecycleEvent { _, event -> + if (event == Lifecycle.Event.ON_CREATE) { + scope.launch { room.updateMembers() } + } } val membersState by room.membersStateFlow.collectAsState() diff --git a/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/members/RoomMemberListDataSource.kt b/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/members/RoomMemberListDataSource.kt index 867ecd991f..faf1f9fc78 100644 --- a/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/members/RoomMemberListDataSource.kt +++ b/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/members/RoomMemberListDataSource.kt @@ -19,11 +19,8 @@ package io.element.android.features.roomdetails.impl.members import io.element.android.libraries.core.bool.orFalse import io.element.android.libraries.core.coroutine.CoroutineDispatchers import io.element.android.libraries.matrix.api.room.MatrixRoom -import io.element.android.libraries.matrix.api.room.MatrixRoomMembersState import io.element.android.libraries.matrix.api.room.RoomMember import io.element.android.libraries.matrix.api.room.roomMembers -import kotlinx.coroutines.flow.dropWhile -import kotlinx.coroutines.flow.first import kotlinx.coroutines.withContext import javax.inject.Inject @@ -32,11 +29,8 @@ class RoomMemberListDataSource @Inject constructor( private val coroutineDispatchers: CoroutineDispatchers, ) { suspend fun search(query: String): List = withContext(coroutineDispatchers.io) { - val roomMembers = room.membersStateFlow - .dropWhile { it !is MatrixRoomMembersState.Ready } - .first() - .roomMembers() - .orEmpty() + val roomMembersState = room.membersStateFlow.value + val roomMembers = roomMembersState.roomMembers().orEmpty() val filteredMembers = if (query.isBlank()) { roomMembers } else { diff --git a/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/members/RoomMemberListPresenter.kt b/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/members/RoomMemberListPresenter.kt index 6fc5b6ece7..399a2ad1e1 100644 --- a/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/members/RoomMemberListPresenter.kt +++ b/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/members/RoomMemberListPresenter.kt @@ -30,8 +30,10 @@ import io.element.android.libraries.architecture.Presenter import io.element.android.libraries.core.coroutine.CoroutineDispatchers import io.element.android.libraries.designsystem.theme.components.SearchBarResultState import io.element.android.libraries.matrix.api.room.MatrixRoom +import io.element.android.libraries.matrix.api.room.MatrixRoomMembersState import io.element.android.libraries.matrix.api.room.RoomMembershipState import io.element.android.libraries.matrix.api.room.powerlevels.canInvite +import io.element.android.libraries.matrix.api.room.roomMembers import kotlinx.collections.immutable.toImmutableList import kotlinx.coroutines.withContext import javax.inject.Inject @@ -55,9 +57,12 @@ class RoomMemberListPresenter @Inject constructor( value = room.canInvite().getOrElse { false } } - LaunchedEffect(Unit) { + LaunchedEffect(membersState) { + if (membersState is MatrixRoomMembersState.Unknown) { + return@LaunchedEffect + } withContext(coroutineDispatchers.io) { - val members = roomMemberListDataSource.search("").groupBy { it.membership } + val members = membersState.roomMembers().orEmpty().groupBy { it.membership } roomMembers = AsyncData.Success( RoomMembers( invited = members.getOrDefault(RoomMembershipState.INVITE, emptyList()).toImmutableList(), diff --git a/features/roomdetails/impl/src/test/kotlin/io/element/android/features/roomdetails/RoomDetailsPresenterTests.kt b/features/roomdetails/impl/src/test/kotlin/io/element/android/features/roomdetails/RoomDetailsPresenterTests.kt index 95deffad31..299a8120a5 100644 --- a/features/roomdetails/impl/src/test/kotlin/io/element/android/features/roomdetails/RoomDetailsPresenterTests.kt +++ b/features/roomdetails/impl/src/test/kotlin/io/element/android/features/roomdetails/RoomDetailsPresenterTests.kt @@ -16,6 +16,7 @@ package io.element.android.features.roomdetails +import androidx.lifecycle.Lifecycle import app.cash.molecule.RecompositionMode import app.cash.molecule.moleculeFlow import app.cash.turbine.test @@ -47,9 +48,11 @@ import io.element.android.libraries.matrix.test.FakeMatrixClient import io.element.android.libraries.matrix.test.notificationsettings.FakeNotificationSettingsService import io.element.android.libraries.matrix.test.room.FakeMatrixRoom import io.element.android.libraries.matrix.test.room.aRoomInfo +import io.element.android.tests.testutils.FakeLifecycleOwner import io.element.android.tests.testutils.WarmUpRule import io.element.android.tests.testutils.consumeItemsUntilPredicate import io.element.android.tests.testutils.testCoroutineDispatchers +import io.element.android.tests.testutils.withFakeLifecycleOwner import kotlinx.collections.immutable.persistentListOf import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.test.TestScope @@ -63,6 +66,10 @@ class RoomDetailsPresenterTests { @get:Rule val warmUpRule = WarmUpRule() + private val fakeLifecycleOwner = FakeLifecycleOwner().apply { + givenState(Lifecycle.State.RESUMED) + } + private fun TestScope.createRoomDetailsPresenter( room: MatrixRoom, leaveRoomPresenter: LeaveRoomPresenter = FakeLeaveRoomPresenter(), @@ -94,7 +101,9 @@ class RoomDetailsPresenterTests { val room = aMatrixRoom() val presenter = createRoomDetailsPresenter(room) moleculeFlow(RecompositionMode.Immediate) { - presenter.present() + withFakeLifecycleOwner(fakeLifecycleOwner) { + presenter.present() + } }.test { val initialState = awaitItem() assertThat(initialState.roomId).isEqualTo(room.roomId.value) @@ -116,7 +125,9 @@ class RoomDetailsPresenterTests { } val presenter = createRoomDetailsPresenter(room) moleculeFlow(RecompositionMode.Immediate) { - presenter.present() + withFakeLifecycleOwner(fakeLifecycleOwner) { + presenter.present() + } }.test { skipItems(1) val updatedState = awaitItem() @@ -133,7 +144,9 @@ class RoomDetailsPresenterTests { val room = aMatrixRoom(name = null) val presenter = createRoomDetailsPresenter(room) moleculeFlow(RecompositionMode.Immediate) { - presenter.present() + withFakeLifecycleOwner(fakeLifecycleOwner) { + presenter.present() + } }.test { val initialState = awaitItem() assertThat(initialState.roomName).isEqualTo(room.displayName) @@ -155,7 +168,9 @@ class RoomDetailsPresenterTests { } val presenter = createRoomDetailsPresenter(room) moleculeFlow(RecompositionMode.Immediate) { - presenter.present() + withFakeLifecycleOwner(fakeLifecycleOwner) { + presenter.present() + } }.test { val initialState = awaitItem() assertThat(initialState.roomType).isEqualTo(RoomDetailsType.Dm(otherRoomMember)) @@ -171,7 +186,9 @@ class RoomDetailsPresenterTests { } val presenter = createRoomDetailsPresenter(room, dispatchers = testCoroutineDispatchers()) moleculeFlow(RecompositionMode.Immediate) { - presenter.present() + withFakeLifecycleOwner(fakeLifecycleOwner) { + presenter.present() + } }.test { // Initially false assertThat(awaitItem().canInvite).isFalse() @@ -189,7 +206,9 @@ class RoomDetailsPresenterTests { } val presenter = createRoomDetailsPresenter(room) moleculeFlow(RecompositionMode.Immediate) { - presenter.present() + withFakeLifecycleOwner(fakeLifecycleOwner) { + presenter.present() + } }.test { assertThat(awaitItem().canInvite).isFalse() @@ -204,7 +223,9 @@ class RoomDetailsPresenterTests { } val presenter = createRoomDetailsPresenter(room) moleculeFlow(RecompositionMode.Immediate) { - presenter.present() + withFakeLifecycleOwner(fakeLifecycleOwner) { + presenter.present() + } }.test { assertThat(awaitItem().canInvite).isFalse() @@ -222,7 +243,9 @@ class RoomDetailsPresenterTests { } val presenter = createRoomDetailsPresenter(room) moleculeFlow(RecompositionMode.Immediate) { - presenter.present() + withFakeLifecycleOwner(fakeLifecycleOwner) { + presenter.present() + } }.test { // Initially false assertThat(awaitItem().canEdit).isFalse() @@ -251,7 +274,9 @@ class RoomDetailsPresenterTests { } val presenter = createRoomDetailsPresenter(room) moleculeFlow(RecompositionMode.Immediate) { - presenter.present() + withFakeLifecycleOwner(fakeLifecycleOwner) { + presenter.present() + } }.test { // Initially false assertThat(awaitItem().canEdit).isFalse() @@ -281,7 +306,9 @@ class RoomDetailsPresenterTests { } val presenter = createRoomDetailsPresenter(room) moleculeFlow(RecompositionMode.Immediate) { - presenter.present() + withFakeLifecycleOwner(fakeLifecycleOwner) { + presenter.present() + } }.test { skipItems(1) @@ -302,7 +329,9 @@ class RoomDetailsPresenterTests { } val presenter = createRoomDetailsPresenter(room) moleculeFlow(RecompositionMode.Immediate) { - presenter.present() + withFakeLifecycleOwner(fakeLifecycleOwner) { + presenter.present() + } }.test { // Initially false assertThat(awaitItem().canEdit).isFalse() @@ -323,7 +352,9 @@ class RoomDetailsPresenterTests { } val presenter = createRoomDetailsPresenter(room) moleculeFlow(RecompositionMode.Immediate) { - presenter.present() + withFakeLifecycleOwner(fakeLifecycleOwner) { + presenter.present() + } }.test { // Initially false, and no further events assertThat(awaitItem().canEdit).isFalse() @@ -341,7 +372,9 @@ class RoomDetailsPresenterTests { val presenter = createRoomDetailsPresenter(room) moleculeFlow(RecompositionMode.Immediate) { - presenter.present() + withFakeLifecycleOwner(fakeLifecycleOwner) { + presenter.present() + } }.test { // The initial state is "hidden" and no further state changes happen assertThat(awaitItem().roomTopic).isEqualTo(RoomTopicState.Hidden) @@ -360,7 +393,9 @@ class RoomDetailsPresenterTests { val presenter = createRoomDetailsPresenter(room) moleculeFlow(RecompositionMode.Immediate) { - presenter.present() + withFakeLifecycleOwner(fakeLifecycleOwner) { + presenter.present() + } }.test { // Ignore the initial state skipItems(1) @@ -382,7 +417,9 @@ class RoomDetailsPresenterTests { dispatchers = testCoroutineDispatchers() ) moleculeFlow(RecompositionMode.Immediate) { - presenter.present() + withFakeLifecycleOwner(fakeLifecycleOwner) { + presenter.present() + } }.test { awaitItem().eventSink(RoomDetailsEvent.LeaveRoom) @@ -403,7 +440,9 @@ class RoomDetailsPresenterTests { notificationSettingsService = notificationSettingsService, ) moleculeFlow(RecompositionMode.Immediate) { - presenter.present() + withFakeLifecycleOwner(fakeLifecycleOwner) { + presenter.present() + } }.test { notificationSettingsService.setRoomNotificationMode(room.roomId, RoomNotificationMode.MENTIONS_AND_KEYWORDS_ONLY) val updatedState = consumeItemsUntilPredicate { @@ -420,7 +459,9 @@ class RoomDetailsPresenterTests { val room = aMatrixRoom(notificationSettingsService = notificationSettingsService) val presenter = createRoomDetailsPresenter(room = room, notificationSettingsService = notificationSettingsService) moleculeFlow(RecompositionMode.Immediate) { - presenter.present() + withFakeLifecycleOwner(fakeLifecycleOwner) { + presenter.present() + } }.test { awaitItem().eventSink(RoomDetailsEvent.MuteNotification) val updatedState = consumeItemsUntilPredicate(timeout = 250.milliseconds) { @@ -440,7 +481,9 @@ class RoomDetailsPresenterTests { val room = aMatrixRoom(notificationSettingsService = notificationSettingsService) val presenter = createRoomDetailsPresenter(room = room, notificationSettingsService = notificationSettingsService) moleculeFlow(RecompositionMode.Immediate) { - presenter.present() + withFakeLifecycleOwner(fakeLifecycleOwner) { + presenter.present() + } }.test { awaitItem().eventSink(RoomDetailsEvent.UnmuteNotification) val updatedState = consumeItemsUntilPredicate { diff --git a/features/roomdetails/impl/src/test/kotlin/io/element/android/features/roomdetails/members/RoomMemberListPresenterTests.kt b/features/roomdetails/impl/src/test/kotlin/io/element/android/features/roomdetails/members/RoomMemberListPresenterTests.kt index a869bb5e04..88e9c7df86 100644 --- a/features/roomdetails/impl/src/test/kotlin/io/element/android/features/roomdetails/members/RoomMemberListPresenterTests.kt +++ b/features/roomdetails/impl/src/test/kotlin/io/element/android/features/roomdetails/members/RoomMemberListPresenterTests.kt @@ -47,15 +47,20 @@ class RoomMemberListPresenterTests { @Test fun `search is done automatically on start, but is async`() = runTest { - val presenter = createPresenter() + val room = FakeMatrixRoom() + val presenter = createPresenter(matrixRoom = room) moleculeFlow(RecompositionMode.Immediate) { presenter.present() }.test { + skipItems(1) val initialState = awaitItem() assertThat(initialState.roomMembers).isInstanceOf(AsyncData.Loading::class.java) assertThat(initialState.searchQuery).isEmpty() assertThat(initialState.searchResults).isInstanceOf(SearchBarResultState.Initial::class.java) assertThat(initialState.isSearchActive).isFalse() + room.givenRoomMembersState(MatrixRoomMembersState.Ready(aRoomMemberList())) + // Skip item while the new members state is processed + skipItems(1) val loadedState = awaitItem() assertThat(loadedState.roomMembers).isInstanceOf(AsyncData.Success::class.java) assertThat((loadedState.roomMembers as AsyncData.Success).data.invited).isEqualTo(listOf(aVictor(), aWalter())) diff --git a/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/room/MatrixRoom.kt b/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/room/MatrixRoom.kt index af6cd81fb2..41043d8ba5 100644 --- a/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/room/MatrixRoom.kt +++ b/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/room/MatrixRoom.kt @@ -75,7 +75,7 @@ interface MatrixRoom : Closeable { /** * Try to load the room members and update the membersFlow. */ - suspend fun updateMembers(): Result + suspend fun updateMembers() suspend fun updateRoomNotificationSettings(): Result diff --git a/libraries/matrix/impl/build.gradle.kts b/libraries/matrix/impl/build.gradle.kts index 41c84a0384..8815214dd6 100644 --- a/libraries/matrix/impl/build.gradle.kts +++ b/libraries/matrix/impl/build.gradle.kts @@ -53,4 +53,5 @@ dependencies { testImplementation(libs.test.truth) testImplementation(projects.libraries.matrix.test) testImplementation(libs.coroutines.test) + testImplementation(libs.test.turbine) } diff --git a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/notification/TimelineEventToNotificationContentMapper.kt b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/notification/TimelineEventToNotificationContentMapper.kt index cc14fad815..6526120328 100644 --- a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/notification/TimelineEventToNotificationContentMapper.kt +++ b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/notification/TimelineEventToNotificationContentMapper.kt @@ -18,7 +18,7 @@ package io.element.android.libraries.matrix.impl.notification import io.element.android.libraries.matrix.api.core.UserId import io.element.android.libraries.matrix.api.notification.NotificationContent -import io.element.android.libraries.matrix.impl.room.RoomMemberMapper +import io.element.android.libraries.matrix.impl.room.member.RoomMemberMapper import io.element.android.libraries.matrix.impl.timeline.item.event.EventMessageMapper import org.matrix.rustcomponents.sdk.MessageLikeEventContent import org.matrix.rustcomponents.sdk.StateEventContent diff --git a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/room/MatrixRoomInfoMapper.kt b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/room/MatrixRoomInfoMapper.kt index b2a13fbe4a..909649441f 100644 --- a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/room/MatrixRoomInfoMapper.kt +++ b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/room/MatrixRoomInfoMapper.kt @@ -19,6 +19,7 @@ package io.element.android.libraries.matrix.impl.room import io.element.android.libraries.matrix.api.room.CurrentUserMembership import io.element.android.libraries.matrix.api.room.MatrixRoomInfo import io.element.android.libraries.matrix.api.room.RoomNotificationMode +import io.element.android.libraries.matrix.impl.room.member.RoomMemberMapper import io.element.android.libraries.matrix.impl.timeline.item.event.EventTimelineItemMapper import kotlinx.collections.immutable.toImmutableList import org.matrix.rustcomponents.sdk.use diff --git a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/room/RustMatrixRoom.kt b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/room/RustMatrixRoom.kt index c9a89e558e..647586431a 100644 --- a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/room/RustMatrixRoom.kt +++ b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/room/RustMatrixRoom.kt @@ -18,7 +18,6 @@ package io.element.android.libraries.matrix.impl.room import io.element.android.libraries.core.coroutine.CoroutineDispatchers import io.element.android.libraries.core.coroutine.childScope -import io.element.android.libraries.core.coroutine.parallelMap import io.element.android.libraries.matrix.api.core.EventId import io.element.android.libraries.matrix.api.core.ProgressCallback import io.element.android.libraries.matrix.api.core.RoomId @@ -39,7 +38,6 @@ import io.element.android.libraries.matrix.api.room.Mention import io.element.android.libraries.matrix.api.room.MessageEventType import io.element.android.libraries.matrix.api.room.StateEventType import io.element.android.libraries.matrix.api.room.location.AssetType -import io.element.android.libraries.matrix.api.room.roomMembers import io.element.android.libraries.matrix.api.room.roomNotificationSettings import io.element.android.libraries.matrix.api.timeline.MatrixTimeline import io.element.android.libraries.matrix.api.widget.MatrixWidgetDriver @@ -51,20 +49,17 @@ import io.element.android.libraries.matrix.impl.media.toMSC3246range import io.element.android.libraries.matrix.impl.notificationsettings.RustNotificationSettingsService import io.element.android.libraries.matrix.impl.poll.toInner import io.element.android.libraries.matrix.impl.room.location.toInner +import io.element.android.libraries.matrix.impl.room.member.RoomMemberListFetcher import io.element.android.libraries.matrix.impl.timeline.AsyncMatrixTimeline import io.element.android.libraries.matrix.impl.timeline.RustMatrixTimeline -import io.element.android.libraries.matrix.impl.util.destroyAll import io.element.android.libraries.matrix.impl.util.mxCallbackFlow import io.element.android.libraries.matrix.impl.widget.RustWidgetDriver import io.element.android.libraries.matrix.impl.widget.generateWidgetWebViewUrl import io.element.android.libraries.sessionstorage.api.SessionData import io.element.android.services.toolbox.api.systemclock.SystemClock -import kotlinx.collections.immutable.toImmutableList -import kotlinx.coroutines.CancellationException import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.cancel -import kotlinx.coroutines.ensureActive import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.StateFlow @@ -75,7 +70,6 @@ import org.matrix.rustcomponents.sdk.EventTimelineItem import org.matrix.rustcomponents.sdk.RoomInfo import org.matrix.rustcomponents.sdk.RoomInfoListener import org.matrix.rustcomponents.sdk.RoomListItem -import org.matrix.rustcomponents.sdk.RoomMember import org.matrix.rustcomponents.sdk.RoomMessageEventContentWithoutRelation import org.matrix.rustcomponents.sdk.SendAttachmentJoinHandle import org.matrix.rustcomponents.sdk.WidgetCapabilities @@ -125,8 +119,8 @@ class RustMatrixRoom( private val roomMembersDispatcher = coroutineDispatchers.io.limitedParallelism(8) private val roomCoroutineScope = sessionCoroutineScope.childScope(coroutineDispatchers.main, "RoomScope-$roomId") - private val _membersStateFlow = MutableStateFlow(MatrixRoomMembersState.Unknown) private val _syncUpdateFlow = MutableStateFlow(0L) + private val roomMemberListFetcher = RoomMemberListFetcher(innerRoom, roomMembersDispatcher) private val _roomNotificationSettingsStateFlow = MutableStateFlow(MatrixRoomNotificationSettingsState.Unknown) override val roomNotificationSettingsStateFlow: StateFlow = _roomNotificationSettingsStateFlow @@ -135,7 +129,7 @@ class RustMatrixRoom( _syncUpdateFlow.value = systemClock.epochMillis() } - override val membersStateFlow: StateFlow = _membersStateFlow.asStateFlow() + override val membersStateFlow: StateFlow = roomMemberListFetcher.membersFlow override val syncUpdateFlow: StateFlow = _syncUpdateFlow.asStateFlow() @@ -192,35 +186,7 @@ class RustMatrixRoom( override val activeMemberCount: Long get() = innerRoom.activeMembersCount().toLong() - override suspend fun updateMembers(): Result = withContext(roomMembersDispatcher) { - val currentState = _membersStateFlow.value - val currentMembers = currentState.roomMembers()?.toImmutableList() - _membersStateFlow.value = MatrixRoomMembersState.Pending(prevRoomMembers = currentMembers) - var rustMembers: List? = null - try { - rustMembers = innerRoom.members().use { membersIterator -> - buildList { - while (true) { - // Loading the whole membersIterator as a stop-gap measure. - // We should probably implement some sort of paging in the future. - ensureActive() - addAll(membersIterator.nextChunk(1000u) ?: break) - } - } - } - val mappedMembers = rustMembers.parallelMap(RoomMemberMapper::map) - _membersStateFlow.value = MatrixRoomMembersState.Ready(mappedMembers.toImmutableList()) - Result.success(Unit) - } catch (exception: CancellationException) { - _membersStateFlow.value = MatrixRoomMembersState.Error(prevRoomMembers = currentMembers, failure = exception) - throw exception - } catch (exception: Exception) { - _membersStateFlow.value = MatrixRoomMembersState.Error(prevRoomMembers = currentMembers, failure = exception) - Result.failure(exception) - } finally { - rustMembers?.destroyAll() - } - } + override suspend fun updateMembers() = roomMemberListFetcher.fetchRoomMembers() override suspend fun userDisplayName(userId: UserId): Result = withContext(roomDispatcher) { runCatching { diff --git a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/room/member/RoomMemberListFetcher.kt b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/room/member/RoomMemberListFetcher.kt new file mode 100644 index 0000000000..83850f574b --- /dev/null +++ b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/room/member/RoomMemberListFetcher.kt @@ -0,0 +1,129 @@ +/* + * Copyright (c) 2024 New Vector Ltd + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package io.element.android.libraries.matrix.impl.room.member + +import io.element.android.libraries.core.coroutine.parallelMap +import io.element.android.libraries.matrix.api.room.MatrixRoomMembersState +import io.element.android.libraries.matrix.api.room.roomMembers +import io.element.android.libraries.matrix.impl.util.destroyAll +import kotlinx.collections.immutable.toImmutableList +import kotlinx.coroutines.CoroutineDispatcher +import kotlinx.coroutines.ensureActive +import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.flow.StateFlow +import kotlinx.coroutines.sync.Mutex +import kotlinx.coroutines.sync.withLock +import kotlinx.coroutines.withContext +import org.matrix.rustcomponents.sdk.RoomInterface +import org.matrix.rustcomponents.sdk.RoomMembersIterator +import org.matrix.rustcomponents.sdk.use +import timber.log.Timber +import kotlin.coroutines.cancellation.CancellationException +import kotlin.coroutines.coroutineContext + +/** + * This class fetches the room members for a given room in a 'paginated' way, and taking into account previous cached values. + */ +internal class RoomMemberListFetcher( + private val room: RoomInterface, + private val dispatcher: CoroutineDispatcher, + private val pageSize: Int = 1000, +) { + private val updatedRoomMemberMutex = Mutex() + private val roomId = room.id() + + private val _membersFlow = MutableStateFlow(MatrixRoomMembersState.Unknown) + val membersFlow: StateFlow = _membersFlow + + /** + * Fetches the room members for the given room. + * It will emit the cached members first, and then the updated members in batches of [pageSize] items, through [membersFlow]. + * @param withCache Whether to load the cached members first. Defaults to true. + */ + suspend fun fetchRoomMembers(withCache: Boolean = true) { + if (updatedRoomMemberMutex.isLocked) { + Timber.i("Room members are already being updated for room $roomId") + return + } + updatedRoomMemberMutex.withLock { + withContext(dispatcher) { + // Load cached members as fallback and to get faster results + if (withCache) { + if (_membersFlow.value !is MatrixRoomMembersState.Ready) { + fetchCachedRoomMembers() + } else { + Timber.i("No need to load cached members found for room $roomId") + } + } + + val prevRoomMembers = (_membersFlow.value as? MatrixRoomMembersState.Ready)?.roomMembers?.toImmutableList() + _membersFlow.value = MatrixRoomMembersState.Pending(prevRoomMembers = prevRoomMembers) + + try { + // Start loading new members + parseAndEmitMembers(room.members()) + } catch (exception: CancellationException) { + Timber.d("Cancelled loading updated members for room $roomId") + throw exception + } catch (exception: Exception) { + Timber.e(exception, "Failed to load updated members for room $roomId") + _membersFlow.value = MatrixRoomMembersState.Error(exception, prevRoomMembers) + } + } + } + } + + internal suspend fun fetchCachedRoomMembers() = withContext(dispatcher) { + Timber.i("Loading cached members for room $roomId") + try { + val iterator = room.membersNoSync() + parseAndEmitMembers(iterator) + } catch (exception: CancellationException) { + Timber.d("Cancelled loading cached members for room $roomId") + throw exception + } catch (exception: Exception) { + Timber.e(exception, "Failed to load cached members for room $roomId") + _membersFlow.value = MatrixRoomMembersState.Error(exception, _membersFlow.value.roomMembers()?.toImmutableList()) + } + } + + private suspend fun parseAndEmitMembers(roomMembersIterator: RoomMembersIterator) { + roomMembersIterator.use { iterator -> + val results = buildList { + while (true) { + // Loading the whole membersIterator as a stop-gap measure. + // We should probably implement some sort of paging in the future. + coroutineContext.ensureActive() + val chunk = iterator.nextChunk(pageSize.toUInt()) + val members = try { + // Load next chunk. If null (no more items), exit the loop + chunk?.parallelMap(RoomMemberMapper::map) ?: break + } finally { + // Make sure we clear all member references + chunk?.destroyAll() + } + addAll(members) + Timber.i("Emitting first $size members for room $roomId") + _membersFlow.value = MatrixRoomMembersState.Ready(toImmutableList()) + } + } + if (results.isEmpty()) { + _membersFlow.value = MatrixRoomMembersState.Ready(results.toImmutableList()) + } + } + } +} diff --git a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/room/RoomMemberMapper.kt b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/room/member/RoomMemberMapper.kt similarity index 94% rename from libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/room/RoomMemberMapper.kt rename to libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/room/member/RoomMemberMapper.kt index 3354abbe16..e3d5b37cd4 100644 --- a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/room/RoomMemberMapper.kt +++ b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/room/member/RoomMemberMapper.kt @@ -1,5 +1,5 @@ /* - * Copyright (c) 2023 New Vector Ltd + * Copyright (c) 2024 New Vector Ltd * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -14,7 +14,7 @@ * limitations under the License. */ -package io.element.android.libraries.matrix.impl.room +package io.element.android.libraries.matrix.impl.room.member import io.element.android.libraries.matrix.api.core.UserId import io.element.android.libraries.matrix.api.room.RoomMember diff --git a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/roomlist/RoomSummaryDetailsFactory.kt b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/roomlist/RoomSummaryDetailsFactory.kt index d308f2b1d3..59be0ba4ce 100644 --- a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/roomlist/RoomSummaryDetailsFactory.kt +++ b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/roomlist/RoomSummaryDetailsFactory.kt @@ -19,7 +19,7 @@ package io.element.android.libraries.matrix.impl.roomlist import io.element.android.libraries.matrix.api.core.RoomId import io.element.android.libraries.matrix.api.roomlist.RoomSummaryDetails import io.element.android.libraries.matrix.impl.notificationsettings.RoomNotificationSettingsMapper -import io.element.android.libraries.matrix.impl.room.RoomMemberMapper +import io.element.android.libraries.matrix.impl.room.member.RoomMemberMapper import io.element.android.libraries.matrix.impl.room.message.RoomMessageFactory import org.matrix.rustcomponents.sdk.RoomInfo import org.matrix.rustcomponents.sdk.use diff --git a/libraries/matrix/impl/src/test/kotlin/io/element/android/libraries/matrix/impl/room/member/RoomMemberListFetcherTest.kt b/libraries/matrix/impl/src/test/kotlin/io/element/android/libraries/matrix/impl/room/member/RoomMemberListFetcherTest.kt new file mode 100644 index 0000000000..1c153435db --- /dev/null +++ b/libraries/matrix/impl/src/test/kotlin/io/element/android/libraries/matrix/impl/room/member/RoomMemberListFetcherTest.kt @@ -0,0 +1,303 @@ +/* + * Copyright (c) 2024 New Vector Ltd + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package io.element.android.libraries.matrix.impl.room.member + +import app.cash.turbine.test +import com.google.common.truth.Truth.assertThat +import io.element.android.libraries.matrix.api.core.UserId +import io.element.android.libraries.matrix.api.room.MatrixRoomMembersState +import io.element.android.libraries.matrix.api.room.roomMembers +import io.element.android.libraries.matrix.test.A_ROOM_ID +import io.element.android.libraries.matrix.test.A_USER_ID +import io.element.android.libraries.matrix.test.A_USER_ID_2 +import io.element.android.libraries.matrix.test.A_USER_ID_3 +import io.element.android.libraries.matrix.test.A_USER_ID_4 +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.test.runTest +import org.junit.Test +import org.matrix.rustcomponents.sdk.MembershipState +import org.matrix.rustcomponents.sdk.NoPointer +import org.matrix.rustcomponents.sdk.Room +import org.matrix.rustcomponents.sdk.RoomMember +import org.matrix.rustcomponents.sdk.RoomMembersIterator + +class RoomMemberListFetcherTest { + @Test + fun `fetchCachedRoomMembers - emits cached members, if any`() = runTest { + val room = FakeRustRoom(getMembersNoSync = { + FakeRoomMembersIterator( + listOf( + FakeRustRoomMember(A_USER_ID), + FakeRustRoomMember(A_USER_ID_2), + FakeRustRoomMember(A_USER_ID_3), + ) + ) + }) + + val fetcher = RoomMemberListFetcher(room, Dispatchers.Default) + fetcher.membersFlow.test { + assertThat(awaitItem()).isInstanceOf(MatrixRoomMembersState.Unknown::class.java) + + fetcher.fetchCachedRoomMembers() + + val readyItem = awaitItem() + assertThat(readyItem).isInstanceOf(MatrixRoomMembersState.Ready::class.java) + assertThat((readyItem as? MatrixRoomMembersState.Ready)?.roomMembers?.size).isEqualTo(3) + } + } + + @Test + fun `fetchCachedRoomMembers - emits empty list, if no members exist`() = runTest { + val room = FakeRustRoom(getMembersNoSync = { + FakeRoomMembersIterator(emptyList()) + }) + + val fetcher = RoomMemberListFetcher(room, Dispatchers.Default) + fetcher.membersFlow.test { + fetcher.fetchCachedRoomMembers() + assertThat(awaitItem()).isInstanceOf(MatrixRoomMembersState.Unknown::class.java) + assertThat(awaitItem().roomMembers()).isEmpty() + } + } + + @Test + fun `fetchCachedRoomMembers - emits Error on error found`() = runTest { + val room = FakeRustRoom(getMembersNoSync = { + error("Some unexpected issue") + }) + + val fetcher = RoomMemberListFetcher(room, Dispatchers.Default) + fetcher.membersFlow.test { + fetcher.fetchCachedRoomMembers() + assertThat(awaitItem()).isInstanceOf(MatrixRoomMembersState.Unknown::class.java) + assertThat(awaitItem()).isInstanceOf(MatrixRoomMembersState.Error::class.java) + } + } + + @Test + fun `fetchCachedRoomMembers - emits items using page size`() = runTest { + val room = FakeRustRoom(getMembersNoSync = { + FakeRoomMembersIterator( + listOf( + FakeRustRoomMember(A_USER_ID), + FakeRustRoomMember(A_USER_ID_2), + FakeRustRoomMember(A_USER_ID_3), + ) + ) + }) + + val fetcher = RoomMemberListFetcher(room, Dispatchers.Default, pageSize = 2) + fetcher.membersFlow.test { + fetcher.fetchCachedRoomMembers() + + assertThat(awaitItem()).isInstanceOf(MatrixRoomMembersState.Unknown::class.java) + assertThat((awaitItem() as? MatrixRoomMembersState.Ready)?.roomMembers?.size).isEqualTo(2) + assertThat((awaitItem() as? MatrixRoomMembersState.Ready)?.roomMembers?.size).isEqualTo(3) + + ensureAllEventsConsumed() + } + } + + @Test + fun `fetchRoomMembers - with 'withCache' set to false emits only new members, if any`() = runTest { + val room = FakeRustRoom(getMembers = { + FakeRoomMembersIterator( + listOf( + FakeRustRoomMember(A_USER_ID), + FakeRustRoomMember(A_USER_ID_2), + FakeRustRoomMember(A_USER_ID_3), + ) + ) + }) + + val fetcher = RoomMemberListFetcher(room, Dispatchers.Default) + fetcher.membersFlow.test { + fetcher.fetchRoomMembers(withCache = false) + + assertThat(awaitItem()).isInstanceOf(MatrixRoomMembersState.Unknown::class.java) + assertThat(awaitItem()).isInstanceOf(MatrixRoomMembersState.Pending::class.java) + assertThat((awaitItem() as? MatrixRoomMembersState.Ready)?.roomMembers?.size).isEqualTo(3) + } + } + + @Test + fun `fetchRoomMembers - on error it emits an Error item`() = runTest { + val room = FakeRustRoom(getMembers = { error("An unexpected error") }) + + val fetcher = RoomMemberListFetcher(room, Dispatchers.Default) + fetcher.membersFlow.test { + fetcher.fetchRoomMembers(withCache = false) + + assertThat(awaitItem()).isInstanceOf(MatrixRoomMembersState.Unknown::class.java) + assertThat(awaitItem()).isInstanceOf(MatrixRoomMembersState.Pending::class.java) + assertThat(awaitItem()).isInstanceOf(MatrixRoomMembersState.Error::class.java) + } + } + + @Test + fun `fetchRoomMembers - with 'withCache' returns cached items first, then new ones`() = runTest { + val room = FakeRustRoom( + getMembersNoSync = { + FakeRoomMembersIterator(listOf(FakeRustRoomMember(A_USER_ID_4))) + }, + getMembers = { + FakeRoomMembersIterator( + listOf( + FakeRustRoomMember(A_USER_ID), + FakeRustRoomMember(A_USER_ID_2), + FakeRustRoomMember(A_USER_ID_3), + ) + ) + } + ) + + val fetcher = RoomMemberListFetcher(room, Dispatchers.Default) + fetcher.membersFlow.test { + fetcher.fetchRoomMembers(withCache = true) + // Initial + assertThat(awaitItem()).isInstanceOf(MatrixRoomMembersState.Unknown::class.java) + // Loaded cached + awaitItem().let { cached -> + assertThat(cached).isInstanceOf(MatrixRoomMembersState.Ready::class.java) + assertThat(cached.roomMembers()).hasSize(1) + } + // Start loading new + assertThat(awaitItem()).isInstanceOf(MatrixRoomMembersState.Pending::class.java) + awaitItem().let { ready -> + assertThat(ready).isInstanceOf(MatrixRoomMembersState.Ready::class.java) + assertThat(ready.roomMembers()).hasSize(3) + } + } + } + + @Test + fun `fetchRoomMembers - with 'withCache' skips cache if there is already a ready state`() = runTest { + val room = FakeRustRoom( + getMembersNoSync = { + FakeRoomMembersIterator(listOf(FakeRustRoomMember(A_USER_ID_4))) + }, + getMembers = { + FakeRoomMembersIterator( + listOf( + FakeRustRoomMember(A_USER_ID), + FakeRustRoomMember(A_USER_ID_2), + FakeRustRoomMember(A_USER_ID_3), + ) + ) + } + ) + + val fetcher = RoomMemberListFetcher(room, Dispatchers.Default) + // Set a ready state + fetcher.fetchRoomMembers(withCache = false) + + fetcher.membersFlow.test { + // Start loading new members + fetcher.fetchRoomMembers(withCache = true) + // Previous ready state + assertThat(awaitItem()).isInstanceOf(MatrixRoomMembersState.Ready::class.java) + // New pending state + assertThat(awaitItem()).isInstanceOf(MatrixRoomMembersState.Pending::class.java) + // New ready state + awaitItem().let { ready -> + assertThat(ready).isInstanceOf(MatrixRoomMembersState.Ready::class.java) + assertThat(ready.roomMembers()).hasSize(3) + } + } + } +} + +class FakeRustRoom( + private val getMembers: () -> RoomMembersIterator = { FakeRoomMembersIterator() }, + private val getMembersNoSync: () -> RoomMembersIterator = { FakeRoomMembersIterator() }, +) : Room(NoPointer) { + override fun id(): String { + return A_ROOM_ID.value + } + + override suspend fun members(): RoomMembersIterator { + return getMembers() + } + + override suspend fun membersNoSync(): RoomMembersIterator { + return getMembersNoSync() + } + + override fun close() { + // No-op + } +} + +class FakeRoomMembersIterator( + private var members: List? = null +) : RoomMembersIterator(NoPointer) { + override fun len(): UInt { + return members?.size?.toUInt() ?: 0u + } + + override fun nextChunk(chunkSize: UInt): List? { + if (members?.isEmpty() == true) { + return null + } + return members?.let { + val result = it.take(chunkSize.toInt()) + members = it.subList(result.size, it.size) + result + } + } +} + +class FakeRustRoomMember( + private val userId: UserId, + private val displayName: String? = null, + private val avatarUrl: String? = null, + private val membership: MembershipState = MembershipState.JOIN, + private val isNameAmbiguous: Boolean = false, + private val powerLevel: Long = 0L, +) : RoomMember(NoPointer) { + override fun userId(): String { + return userId.value + } + + override fun displayName(): String? { + return displayName + } + + override fun avatarUrl(): String? { + return avatarUrl + } + + override fun membership(): MembershipState { + return membership + } + + override fun isNameAmbiguous(): Boolean { + return isNameAmbiguous + } + + override fun powerLevel(): Long { + return powerLevel + } + + override fun normalizedPowerLevel(): Long { + return powerLevel + } + + override fun isIgnored(): Boolean { + return false + } +} diff --git a/libraries/matrix/test/src/main/kotlin/io/element/android/libraries/matrix/test/room/FakeMatrixRoom.kt b/libraries/matrix/test/src/main/kotlin/io/element/android/libraries/matrix/test/room/FakeMatrixRoom.kt index fbddf28bbe..19b5132016 100644 --- a/libraries/matrix/test/src/main/kotlin/io/element/android/libraries/matrix/test/room/FakeMatrixRoom.kt +++ b/libraries/matrix/test/src/main/kotlin/io/element/android/libraries/matrix/test/room/FakeMatrixRoom.kt @@ -171,9 +171,7 @@ class FakeMatrixRoom( override val roomNotificationSettingsStateFlow: MutableStateFlow = MutableStateFlow(MatrixRoomNotificationSettingsState.Unknown) - override suspend fun updateMembers(): Result = simulateLongTask { - updateMembersResult - } + override suspend fun updateMembers() = Unit override suspend fun updateRoomNotificationSettings(): Result = simulateLongTask { val notificationSettings = notificationSettingsService.getRoomNotificationSettings(roomId, isEncrypted, isOneToOne).getOrThrow() diff --git a/tests/testutils/src/main/kotlin/io/element/android/tests/testutils/WithFakeLifecycleOwner.kt b/tests/testutils/src/main/kotlin/io/element/android/tests/testutils/WithFakeLifecycleOwner.kt new file mode 100644 index 0000000000..e49dc0317b --- /dev/null +++ b/tests/testutils/src/main/kotlin/io/element/android/tests/testutils/WithFakeLifecycleOwner.kt @@ -0,0 +1,49 @@ +/* + * Copyright (c) 2024 New Vector Ltd + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package io.element.android.tests.testutils + +import android.annotation.SuppressLint +import androidx.compose.runtime.Composable +import androidx.compose.runtime.CompositionLocalProvider +import androidx.compose.runtime.Stable +import androidx.compose.runtime.getValue +import androidx.compose.runtime.mutableStateOf +import androidx.compose.runtime.remember +import androidx.compose.runtime.setValue +import androidx.compose.ui.platform.LocalLifecycleOwner +import androidx.lifecycle.Lifecycle +import androidx.lifecycle.LifecycleOwner +import androidx.lifecycle.LifecycleRegistry + +@Stable +@Composable +fun withFakeLifecycleOwner(lifecycleOwner: FakeLifecycleOwner = FakeLifecycleOwner(), block: @Composable () -> T): T { + var state: T? by remember { mutableStateOf(null) } + CompositionLocalProvider(LocalLifecycleOwner provides lifecycleOwner) { + state = block() + } + return state!! +} + +@SuppressLint("VisibleForTests") +class FakeLifecycleOwner : LifecycleOwner { + override val lifecycle: Lifecycle = LifecycleRegistry.createUnsafe(this) + + fun givenState(state: Lifecycle.State) { + (lifecycle as LifecycleRegistry).currentState = state + } +}