Merge pull request #3491 from element-hq/feature/fga/room_list_debounce_subscribe

Room list : debounce subscribe to visible rooms.

features/roomlist/impl/src/main/kotlin/io/element/android/features/roomlist/impl/RoomListPresenter.kt

@@ -61,7 +61,7 @@ import kotlinx.collections.immutable.toPersistentList
 import kotlinx.coroutines.CoroutineScope
 import kotlinx.coroutines.ExperimentalCoroutinesApi
 import kotlinx.coroutines.Job
-import kotlinx.coroutines.SupervisorJob
+import kotlinx.coroutines.delay
 import kotlinx.coroutines.flow.collect
 import kotlinx.coroutines.flow.distinctUntilChanged
 import kotlinx.coroutines.flow.first
@@ -73,6 +73,7 @@ import kotlinx.coroutines.launch
 import javax.inject.Inject
 
 private const val EXTENDED_RANGE_SIZE = 40
+private const val SUBSCRIBE_TO_VISIBLE_ROOMS_DEBOUNCE_IN_MILLIS = 300L
 
 class RoomListPresenter @Inject constructor(
     private val client: MatrixClient,
@@ -301,7 +302,10 @@ class RoomListPresenter @Inject constructor(
     private var currentUpdateVisibleRangeJob: Job? = null
     private fun CoroutineScope.updateVisibleRange(range: IntRange) {
         currentUpdateVisibleRangeJob?.cancel()
-        currentUpdateVisibleRangeJob = launch(SupervisorJob()) {
+        currentUpdateVisibleRangeJob = launch {
+            // Debounce the subscription to avoid subscribing to too many rooms
+            delay(SUBSCRIBE_TO_VISIBLE_ROOMS_DEBOUNCE_IN_MILLIS)
+
             if (range.isEmpty()) return@launch
             val currentRoomList = roomListDataSource.allRooms.first()
             // Use extended range to 'prefetch' the next rooms info

features/roomlist/impl/src/test/kotlin/io/element/android/features/roomlist/impl/RoomListPresenterTest.kt

@@ -85,13 +85,16 @@ import io.element.android.tests.testutils.lambda.value
 import io.element.android.tests.testutils.test
 import io.element.android.tests.testutils.testCoroutineDispatchers
 import kotlinx.coroutines.CoroutineScope
+import kotlinx.coroutines.ExperimentalCoroutinesApi
 import kotlinx.coroutines.SupervisorJob
 import kotlinx.coroutines.cancel
 import kotlinx.coroutines.flow.MutableStateFlow
 import kotlinx.coroutines.test.TestScope
+import kotlinx.coroutines.test.advanceTimeBy
 import kotlinx.coroutines.test.runTest
 import org.junit.Rule
 import org.junit.Test
+import kotlin.time.Duration.Companion.seconds
 
 class RoomListPresenterTest {
     @get:Rule
@@ -599,6 +602,38 @@ class RoomListPresenterTest {
         }
     }
 
+    @OptIn(ExperimentalCoroutinesApi::class)
+    @Test
+    fun `present - UpdateVisibleRange will cancel the previous subscription if called too soon`() = runTest {
+        val subscribeToVisibleRoomsLambda = lambdaRecorder { _: List<RoomId> -> }
+        val roomListService = FakeRoomListService(subscribeToVisibleRoomsLambda = subscribeToVisibleRoomsLambda)
+        val scope = CoroutineScope(coroutineContext + SupervisorJob())
+        val matrixClient = FakeMatrixClient(
+            roomListService = roomListService,
+        )
+        val roomSummary = aRoomSummary(
+            currentUserMembership = CurrentUserMembership.INVITED
+        )
+        roomListService.postAllRoomsLoadingState(RoomList.LoadingState.Loaded(1))
+        roomListService.postAllRooms(listOf(roomSummary))
+        val presenter = createRoomListPresenter(
+            coroutineScope = scope,
+            client = matrixClient,
+        )
+        presenter.test {
+            val state = consumeItemsUntilPredicate {
+                it.contentState is RoomListContentState.Rooms
+            }.last()
+
+            state.eventSink(RoomListEvents.UpdateVisibleRange(IntRange(0, 10)))
+            // If called again, it will cancel the current one, which should not result in a test failure
+            state.eventSink(RoomListEvents.UpdateVisibleRange(IntRange(0, 11)))
+            advanceTimeBy(1.seconds)
+            subscribeToVisibleRoomsLambda.assertions().isCalledOnce()
+        }
+    }
+
+    @OptIn(ExperimentalCoroutinesApi::class)
     @Test
     fun `present - UpdateVisibleRange subscribes to rooms in visible range`() = runTest {
         val subscribeToVisibleRoomsLambda = lambdaRecorder { _: List<RoomId> -> }
@@ -622,10 +657,12 @@ class RoomListPresenterTest {
             }.last()
 
             state.eventSink(RoomListEvents.UpdateVisibleRange(IntRange(0, 10)))
+            advanceTimeBy(1.seconds)
             subscribeToVisibleRoomsLambda.assertions().isCalledOnce()
 
-            // If called again, it will cancel the current one, which should not result in a test failure
+            // If called again, it will subscribe to the next items
             state.eventSink(RoomListEvents.UpdateVisibleRange(IntRange(0, 11)))
+            advanceTimeBy(1.seconds)
             subscribeToVisibleRoomsLambda.assertions().isCalledExactly(2)
         }
     }

libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/roomlist/RustRoomListService.kt

@@ -53,11 +53,7 @@ internal class RustRoomListService(
     }
 
     override suspend fun subscribeToVisibleRooms(roomIds: List<RoomId>) {
-        val toSubscribe = roomIds.filterNot { roomSyncSubscriber.isSubscribedTo(it) }
+        roomSyncSubscriber.batchSubscribe(roomIds)
-        if (toSubscribe.isNotEmpty()) {
-            Timber.d("Subscribe to ${toSubscribe.size} rooms: $toSubscribe")
-            roomSyncSubscriber.batchSubscribe(toSubscribe)
-        }
     }
 
     override val allRooms: DynamicRoomList = roomListFactory.createRoomList(