Return cached room members before fetching new ones, do it in batches (#2274)

* Use cached users from the Rust SDK. Also lazy load received users by batches. * Create `RoomMemberListFetcher` to wrap all the room member loading logic * Ensure we clear `RoomMember` Rust references if the fetching coroutine is canceled
2024-01-23 18:23:20 +01:00 · 2024-01-23 18:23:20 +01:00 · da4825aa44
commit da4825aa44
parent 196d8a2db6
17 changed files with 579 additions and 84 deletions
--- 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()
--- 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<RoomMember> = 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 {
--- 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(),
--- 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 {
--- 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()))