Subscribe to RoomListItems in the visible range (#3169)

* Subscribe to `RoomListItems` in the visible range

This ensures the room list items always have updated info.
This commit is contained in:
Jorge Martin Espinosa 2024-07-11 10:54:56 +02:00 committed by GitHub
parent 0be7058416
commit 5944f112fb
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
16 changed files with 204 additions and 17 deletions

View file

@ -20,6 +20,7 @@ import io.element.android.features.roomlist.impl.model.RoomListRoomSummary
import io.element.android.libraries.matrix.api.core.RoomId
sealed interface RoomListEvents {
data class UpdateVisibleRange(val range: IntRange) : RoomListEvents
data object DismissRequestVerificationPrompt : RoomListEvents
data object DismissRecoveryKeyPrompt : RoomListEvents
data object ToggleSearchResults : RoomListEvents

View file

@ -68,6 +68,8 @@ import io.element.android.services.analyticsproviders.api.trackers.captureIntera
import kotlinx.collections.immutable.toPersistentList
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.ExperimentalCoroutinesApi
import kotlinx.coroutines.Job
import kotlinx.coroutines.SupervisorJob
import kotlinx.coroutines.flow.collect
import kotlinx.coroutines.flow.distinctUntilChanged
import kotlinx.coroutines.flow.first
@ -78,6 +80,8 @@ import kotlinx.coroutines.flow.takeWhile
import kotlinx.coroutines.launch
import javax.inject.Inject
private const val EXTENDED_RANGE_SIZE = 40
class RoomListPresenter @Inject constructor(
private val client: MatrixClient,
private val networkMonitor: NetworkMonitor,
@ -122,6 +126,9 @@ class RoomListPresenter @Inject constructor(
fun handleEvents(event: RoomListEvents) {
when (event) {
is RoomListEvents.UpdateVisibleRange -> coroutineScope.launch {
updateVisibleRange(event.range)
}
RoomListEvents.DismissRequestVerificationPrompt -> securityBannerDismissed = true
RoomListEvents.DismissRecoveryKeyPrompt -> securityBannerDismissed = true
RoomListEvents.ToggleSearchResults -> searchState.eventSink(RoomListSearchEvents.ToggleSearchVisibility)
@ -283,6 +290,22 @@ class RoomListPresenter @Inject constructor(
}
}
}
private var currentUpdateVisibleRangeJob: Job? = null
private fun CoroutineScope.updateVisibleRange(range: IntRange) {
currentUpdateVisibleRangeJob?.cancel()
currentUpdateVisibleRangeJob = launch(SupervisorJob()) {
if (range.isEmpty()) return@launch
val currentRoomList = roomListDataSource.allRooms.first()
// Use extended range to 'prefetch' the next rooms info
val midExtendedRangeSize = EXTENDED_RANGE_SIZE / 2
val extendedRange = range.first until range.last + midExtendedRangeSize
val roomIds = extendedRange.mapNotNull { index ->
currentRoomList.getOrNull(index)?.roomId
}
roomListDataSource.subscribeToVisibleRooms(roomIds)
}
}
}
@VisibleForTesting

View file

@ -30,6 +30,11 @@ import androidx.compose.foundation.lazy.LazyColumn
import androidx.compose.foundation.lazy.itemsIndexed
import androidx.compose.foundation.lazy.rememberLazyListState
import androidx.compose.runtime.Composable
import androidx.compose.runtime.LaunchedEffect
import androidx.compose.runtime.derivedStateOf
import androidx.compose.runtime.getValue
import androidx.compose.runtime.remember
import androidx.compose.runtime.rememberUpdatedState
import androidx.compose.ui.Alignment
import androidx.compose.ui.Modifier
import androidx.compose.ui.res.stringResource
@ -165,6 +170,18 @@ private fun RoomsViewList(
modifier: Modifier = Modifier,
) {
val lazyListState = rememberLazyListState()
val visibleRange by remember {
derivedStateOf {
val layoutInfo = lazyListState.layoutInfo
val firstItemIndex = layoutInfo.visibleItemsInfo.firstOrNull()?.index ?: 0
val size = layoutInfo.visibleItemsInfo.size
firstItemIndex until firstItemIndex + size
}
}
val updatedEventSink by rememberUpdatedState(newValue = eventSink)
LaunchedEffect(visibleRange) {
updatedEventSink(RoomListEvents.UpdateVisibleRange(visibleRange))
}
LazyColumn(
state = lazyListState,
modifier = modifier,
@ -177,7 +194,7 @@ private fun RoomsViewList(
item {
ConfirmRecoveryKeyBanner(
onContinueClick = onConfirmRecoveryKeyClick,
onDismissClick = { eventSink(RoomListEvents.DismissRecoveryKeyPrompt) }
onDismissClick = { updatedEventSink(RoomListEvents.DismissRecoveryKeyPrompt) }
)
}
}

View file

@ -20,6 +20,7 @@ import io.element.android.features.roomlist.impl.model.RoomListRoomSummary
import io.element.android.libraries.androidutils.diff.DiffCacheUpdater
import io.element.android.libraries.androidutils.diff.MutableListDiffCache
import io.element.android.libraries.core.coroutine.CoroutineDispatchers
import io.element.android.libraries.matrix.api.core.RoomId
import io.element.android.libraries.matrix.api.notificationsettings.NotificationSettingsService
import io.element.android.libraries.matrix.api.roomlist.RoomListService
import io.element.android.libraries.matrix.api.roomlist.RoomSummary
@ -57,6 +58,10 @@ class RoomListDataSource @Inject constructor(
old?.roomId == new?.roomId
}
val allRooms: Flow<ImmutableList<RoomListRoomSummary>> = _allRooms
val loadingState = roomListService.allRooms.loadingState
fun launchIn(coroutineScope: CoroutineScope) {
roomListService
.allRooms
@ -67,9 +72,9 @@ class RoomListDataSource @Inject constructor(
.launchIn(coroutineScope)
}
val allRooms: Flow<ImmutableList<RoomListRoomSummary>> = _allRooms
val loadingState = roomListService.allRooms.loadingState
suspend fun subscribeToVisibleRooms(roomIds: List<RoomId>) {
roomListService.subscribeToVisibleRooms(roomIds)
}
@OptIn(FlowPreview::class)
private fun observeNotificationSettings() {

View file

@ -50,6 +50,7 @@ import io.element.android.libraries.featureflag.test.FakeFeatureFlagService
import io.element.android.libraries.fullscreenintent.test.FakeFullScreenIntentPermissionsPresenter
import io.element.android.libraries.indicator.impl.DefaultIndicatorService
import io.element.android.libraries.matrix.api.MatrixClient
import io.element.android.libraries.matrix.api.core.RoomId
import io.element.android.libraries.matrix.api.encryption.BackupState
import io.element.android.libraries.matrix.api.encryption.RecoveryState
import io.element.android.libraries.matrix.api.room.CurrentUserMembership
@ -584,6 +585,37 @@ class RoomListPresenterTest {
}
}
@Test
fun `present - UpdateVisibleRange subscribes to rooms in visible range`() = 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)))
subscribeToVisibleRoomsLambda.assertions().isCalledOnce()
// If called again, it will cancel the current one, which should not result in a test failure
state.eventSink(RoomListEvents.UpdateVisibleRange(IntRange(0, 11)))
subscribeToVisibleRoomsLambda.assertions().isCalledExactly(2)
}
}
private fun TestScope.createRoomListPresenter(
client: MatrixClient = FakeMatrixClient(),
networkMonitor: NetworkMonitor = FakeNetworkMonitor(),

View file

@ -44,6 +44,24 @@ import org.junit.runner.RunWith
class RoomListViewTest {
@get:Rule val rule = createAndroidComposeRule<ComponentActivity>()
@Test
fun `displaying the view automatically sends a couple of UpdateVisibleRangeEvents`() {
val eventsRecorder = EventsRecorder<RoomListEvents>()
rule.setRoomListView(
state = aRoomListState(
contentState = aRoomsContentState(securityBannerState = SecurityBannerState.RecoveryKeyConfirmation),
eventSink = eventsRecorder,
)
)
eventsRecorder.assertList(
listOf(
RoomListEvents.UpdateVisibleRange(IntRange.EMPTY),
RoomListEvents.UpdateVisibleRange(0 until 2),
)
)
}
@Test
fun `clicking on close recovery key banner emits the expected Event`() {
val eventsRecorder = EventsRecorder<RoomListEvents>()
@ -53,6 +71,10 @@ class RoomListViewTest {
eventSink = eventsRecorder,
)
)
// Remove automatic initial events
eventsRecorder.clear()
val close = rule.activity.getString(CommonStrings.action_close)
rule.onNodeWithContentDescription(close).performClick()
eventsRecorder.assertSingle(RoomListEvents.DismissRecoveryKeyPrompt)
@ -60,7 +82,7 @@ class RoomListViewTest {
@Test
fun `clicking on continue recovery key banner invokes the expected callback`() {
val eventsRecorder = EventsRecorder<RoomListEvents>(expectEvents = false)
val eventsRecorder = EventsRecorder<RoomListEvents>()
ensureCalledOnce { callback ->
rule.setRoomListView(
state = aRoomListState(
@ -69,7 +91,13 @@ class RoomListViewTest {
),
onConfirmRecoveryKeyClick = callback,
)
// Remove automatic initial events
eventsRecorder.clear()
rule.clickOn(CommonStrings.action_continue)
eventsRecorder.assertEmpty()
}
}
@ -90,7 +118,7 @@ class RoomListViewTest {
@Test
fun `clicking on a room invokes the expected callback`() {
val eventsRecorder = EventsRecorder<RoomListEvents>(expectEvents = false)
val eventsRecorder = EventsRecorder<RoomListEvents>()
val state = aRoomListState(
eventSink = eventsRecorder,
)
@ -102,8 +130,14 @@ class RoomListViewTest {
state = state,
onRoomClick = callback,
)
// Remove automatic initial events
eventsRecorder.clear()
rule.onNodeWithText(room0.lastMessage!!.toString()).performClick()
}
eventsRecorder.assertEmpty()
}
@Test
@ -118,6 +152,9 @@ class RoomListViewTest {
rule.setRoomListView(
state = state,
)
// Remove automatic initial events
eventsRecorder.clear()
rule.onNodeWithText(room0.lastMessage!!.toString()).performTouchInput { longClick() }
eventsRecorder.assertSingle(RoomListEvents.ShowContextMenu(room0))
}
@ -135,8 +172,13 @@ class RoomListViewTest {
state = state,
onRoomSettingsClick = callback,
)
// Remove automatic initial events
eventsRecorder.clear()
rule.clickOn(CommonStrings.common_settings)
}
eventsRecorder.assertSingle(RoomListEvents.HideContextMenu)
}
@ -150,6 +192,10 @@ class RoomListViewTest {
it.displayType == RoomSummaryDisplayType.INVITE
}
rule.setRoomListView(state = state)
// Remove automatic initial events
eventsRecorder.clear()
rule.clickOn(CommonStrings.action_accept)
rule.clickOn(CommonStrings.action_decline)
eventsRecorder.assertList(

View file

@ -17,6 +17,7 @@
package io.element.android.libraries.matrix.api.roomlist
import androidx.compose.runtime.Immutable
import io.element.android.libraries.matrix.api.core.RoomId
import kotlinx.coroutines.flow.Flow
import kotlinx.coroutines.flow.StateFlow
import kotlinx.coroutines.flow.filterIsInstance
@ -53,6 +54,12 @@ interface RoomListService {
source: RoomList.Source,
): DynamicRoomList
/**
* Subscribes to sync requests for the visible rooms.
* @param roomIds the list of visible room ids to subscribe to.
*/
suspend fun subscribeToVisibleRooms(roomIds: List<RoomId>)
/**
* Returns a [DynamicRoomList] object of all rooms we want to display.
* If you want to get a filtered room list, consider using [createRoomList].

View file

@ -55,6 +55,7 @@ import io.element.android.libraries.matrix.impl.notificationsettings.RustNotific
import io.element.android.libraries.matrix.impl.oidc.toRustAction
import io.element.android.libraries.matrix.impl.pushers.RustPushersService
import io.element.android.libraries.matrix.impl.room.RoomContentForwarder
import io.element.android.libraries.matrix.impl.room.RoomSyncSubscriber
import io.element.android.libraries.matrix.impl.room.RustRoomFactory
import io.element.android.libraries.matrix.impl.room.preview.RoomPreviewMapper
import io.element.android.libraries.matrix.impl.roomdirectory.RustRoomDirectoryService
@ -213,6 +214,8 @@ class RustMatrixClient(
}
}
private val roomSyncSubscriber: RoomSyncSubscriber = RoomSyncSubscriber(innerRoomListService, dispatchers)
override val roomListService: RoomListService = RustRoomListService(
innerRoomListService = innerRoomListService,
sessionCoroutineScope = sessionCoroutineScope,
@ -221,6 +224,7 @@ class RustMatrixClient(
innerRoomListService = innerRoomListService,
sessionCoroutineScope = sessionCoroutineScope,
),
roomSyncSubscriber = roomSyncSubscriber,
)
private val verificationService = RustSessionVerificationService(
@ -238,6 +242,7 @@ class RustMatrixClient(
dispatchers = dispatchers,
systemClock = clock,
roomContentForwarder = RoomContentForwarder(innerRoomListService),
roomSyncSubscriber = roomSyncSubscriber,
isKeyBackupEnabled = { client.encryption().use { it.backupState() == BackupState.ENABLED } },
getSessionData = { sessionStore.getSession(sessionId.value)!! },
)

View file

@ -19,18 +19,19 @@ package io.element.android.libraries.matrix.impl.room
import io.element.android.libraries.core.coroutine.CoroutineDispatchers
import io.element.android.libraries.matrix.api.core.RoomId
import io.element.android.libraries.matrix.api.timeline.item.event.EventType
import kotlinx.coroutines.CancellationException
import kotlinx.coroutines.sync.Mutex
import kotlinx.coroutines.sync.withLock
import kotlinx.coroutines.withContext
import org.matrix.rustcomponents.sdk.RequiredState
import org.matrix.rustcomponents.sdk.RoomListService
import org.matrix.rustcomponents.sdk.RoomListServiceInterface
import org.matrix.rustcomponents.sdk.RoomSubscription
import timber.log.Timber
private const val DEFAULT_TIMELINE_LIMIT = 20u
class RoomSyncSubscriber(
private val roomListService: RoomListService,
private val roomListService: RoomListServiceInterface,
private val dispatchers: CoroutineDispatchers,
) {
private val subscriptionCounts = HashMap<RoomId, Int>()
@ -38,8 +39,10 @@ class RoomSyncSubscriber(
private val settings = RoomSubscription(
requiredState = listOf(
RequiredState(key = EventType.STATE_ROOM_CANONICAL_ALIAS, value = ""),
RequiredState(key = EventType.STATE_ROOM_NAME, value = ""),
RequiredState(key = EventType.STATE_ROOM_TOPIC, value = ""),
RequiredState(key = EventType.STATE_ROOM_AVATAR, value = ""),
RequiredState(key = EventType.STATE_ROOM_CANONICAL_ALIAS, value = ""),
RequiredState(key = EventType.STATE_ROOM_JOIN_RULES, value = ""),
RequiredState(key = EventType.STATE_ROOM_POWER_LEVELS, value = ""),
),
@ -65,6 +68,27 @@ class RoomSyncSubscriber(
}
}
suspend fun batchSubscribe(roomIds: List<RoomId>) = mutex.withLock {
withContext(dispatchers.io) {
for (roomId in roomIds) {
try {
val currentSubscription = subscriptionCounts.getOrElse(roomId) { 0 }
if (currentSubscription == 0) {
Timber.d("Subscribing to room $roomId}")
roomListService.room(roomId.value).use { roomListItem ->
roomListItem.subscribe(settings)
}
}
subscriptionCounts[roomId] = currentSubscription + 1
} catch (cancellationException: CancellationException) {
throw cancellationException
} catch (exception: Exception) {
Timber.e("Failed to subscribe to room $roomId")
}
}
}
}
suspend fun unsubscribe(roomId: RoomId) = mutex.withLock {
withContext(dispatchers.io) {
try {
@ -84,4 +108,9 @@ class RoomSyncSubscriber(
}
}
}
fun isSubscribedTo(roomId: RoomId): Boolean {
val subscriptionCount = subscriptionCounts[roomId] ?: return false
return subscriptionCount > 0
}
}

View file

@ -50,6 +50,7 @@ class RustRoomFactory(
private val roomContentForwarder: RoomContentForwarder,
private val roomListService: RoomListService,
private val innerRoomListService: InnerRoomListService,
private val roomSyncSubscriber: RoomSyncSubscriber,
private val isKeyBackupEnabled: suspend () -> Boolean,
private val getSessionData: suspend () -> SessionData,
) {
@ -59,8 +60,6 @@ class RustRoomFactory(
private val matrixRoomInfoMapper = MatrixRoomInfoMapper()
private val roomSyncSubscriber: RoomSyncSubscriber = RoomSyncSubscriber(innerRoomListService, dispatchers)
private val eventFilters = TimelineConfig.excludedEvents
.takeIf { it.isNotEmpty() }
?.let { listStateEventType ->

View file

@ -31,8 +31,8 @@ import org.matrix.rustcomponents.sdk.use
class RoomSummaryDetailsFactory(private val roomMessageFactory: RoomMessageFactory = RoomMessageFactory()) {
suspend fun create(roomListItem: RoomListItem): RoomSummary {
val roomInfo = roomListItem.roomInfo()
val latestRoomMessage = roomListItem.latestEvent()?.use {
roomMessageFactory.create(it)
val latestRoomMessage = roomListItem.latestEvent().use { event ->
roomMessageFactory.create(event)
}
return RoomSummary(
roomId = RoomId(roomInfo.id),

View file

@ -113,9 +113,7 @@ class RoomSummaryListProcessor(
val builtRoomSummary = roomListService.roomOrNull(identifier)?.use { roomListItem ->
buildAndCacheRoomSummaryForRoomListItem(roomListItem)
}
if (builtRoomSummary != null) {
roomSummariesByIdentifier[identifier] = builtRoomSummary
} else {
if (builtRoomSummary == null) {
roomSummariesByIdentifier.remove(identifier)
}
return builtRoomSummary

View file

@ -16,11 +16,13 @@
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.DynamicRoomList
import io.element.android.libraries.matrix.api.roomlist.RoomList
import io.element.android.libraries.matrix.api.roomlist.RoomListFilter
import io.element.android.libraries.matrix.api.roomlist.RoomListService
import io.element.android.libraries.matrix.api.roomlist.loadAllIncrementally
import io.element.android.libraries.matrix.impl.room.RoomSyncSubscriber
import kotlinx.coroutines.CoroutineDispatcher
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.flow.SharingStarted
@ -41,6 +43,7 @@ internal class RustRoomListService(
private val sessionCoroutineScope: CoroutineScope,
private val sessionDispatcher: CoroutineDispatcher,
private val roomListFactory: RoomListFactory,
private val roomSyncSubscriber: RoomSyncSubscriber,
) : RoomListService {
override fun createRoomList(
pageSize: Int,
@ -58,6 +61,14 @@ internal class RustRoomListService(
}
}
override suspend fun subscribeToVisibleRooms(roomIds: List<RoomId>) {
val toSubscribe = roomIds.filterNot { roomSyncSubscriber.isSubscribedTo(it) }
if (toSubscribe.isNotEmpty()) {
Timber.d("Subscribe to ${toSubscribe.size} rooms: $toSubscribe")
roomSyncSubscriber.batchSubscribe(toSubscribe)
}
}
override val allRooms: DynamicRoomList = roomListFactory.createRoomList(
pageSize = DEFAULT_PAGE_SIZE,
coroutineContext = sessionDispatcher,

View file

@ -43,6 +43,7 @@ import org.matrix.rustcomponents.sdk.RoomListServiceStateListener
import org.matrix.rustcomponents.sdk.RoomListServiceSyncIndicatorListener
import org.matrix.rustcomponents.sdk.RoomMember
import org.matrix.rustcomponents.sdk.RoomNotificationMode
import org.matrix.rustcomponents.sdk.RoomSubscription
import org.matrix.rustcomponents.sdk.TaskHandle
// NOTE: this class is using a fake implementation of a Rust SDK interface which returns actual Rust objects with pointers.
@ -267,4 +268,6 @@ class FakeRoomListItem(
override suspend fun latestEvent(): EventTimelineItem? {
return latestEvent
}
override fun subscribe(settings: RoomSubscription?) = Unit
}

View file

@ -16,6 +16,7 @@
package io.element.android.libraries.matrix.test.roomlist
import io.element.android.libraries.matrix.api.core.RoomId
import io.element.android.libraries.matrix.api.roomlist.DynamicRoomList
import io.element.android.libraries.matrix.api.roomlist.RoomList
import io.element.android.libraries.matrix.api.roomlist.RoomListFilter
@ -24,7 +25,9 @@ import io.element.android.libraries.matrix.api.roomlist.RoomSummary
import kotlinx.coroutines.flow.MutableStateFlow
import kotlinx.coroutines.flow.StateFlow
class FakeRoomListService : RoomListService {
class FakeRoomListService(
var subscribeToVisibleRoomsLambda: (List<RoomId>) -> Unit = {},
) : RoomListService {
private val allRoomSummariesFlow = MutableStateFlow<List<RoomSummary>>(emptyList())
private val allRoomsLoadingStateFlow = MutableStateFlow<RoomList.LoadingState>(RoomList.LoadingState.NotLoaded)
private val roomListStateFlow = MutableStateFlow<RoomListService.State>(RoomListService.State.Idle)
@ -56,6 +59,10 @@ class FakeRoomListService : RoomListService {
}
}
override suspend fun subscribeToVisibleRooms(roomIds: List<RoomId>) {
subscribeToVisibleRoomsLambda(roomIds)
}
override val allRooms = SimplePagedRoomList(
allRoomSummariesFlow,
allRoomsLoadingStateFlow,

View file

@ -50,4 +50,8 @@ class EventsRecorder<T>(
fun assertTrue(index: Int, predicate: (T) -> Boolean) {
assertThat(predicate(events[index])).isTrue()
}
fun clear() {
events.clear()
}
}