Timeline: avoid pagination when timeline is not ready

This commit is contained in:
ganfra 2023-07-12 22:43:35 +02:00
parent 1619b85685
commit 635e9b9edd
9 changed files with 55 additions and 33 deletions

View file

@ -31,14 +31,12 @@ import io.element.android.libraries.architecture.Presenter
import io.element.android.libraries.matrix.api.core.EventId import io.element.android.libraries.matrix.api.core.EventId
import io.element.android.libraries.matrix.api.room.MatrixRoom import io.element.android.libraries.matrix.api.room.MatrixRoom
import io.element.android.libraries.matrix.api.room.MessageEventType import io.element.android.libraries.matrix.api.room.MessageEventType
import io.element.android.libraries.matrix.api.timeline.MatrixTimeline
import io.element.android.libraries.matrix.ui.room.canSendEventAsState import io.element.android.libraries.matrix.ui.room.canSendEventAsState
import kotlinx.collections.immutable.ImmutableList import kotlinx.collections.immutable.ImmutableList
import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.flow.launchIn import kotlinx.coroutines.flow.launchIn
import kotlinx.coroutines.flow.onEach import kotlinx.coroutines.flow.onEach
import kotlinx.coroutines.launch import kotlinx.coroutines.launch
import timber.log.Timber
import javax.inject.Inject import javax.inject.Inject
private const val backPaginationEventLimit = 20 private const val backPaginationEventLimit = 20
@ -69,7 +67,7 @@ class TimelinePresenter @Inject constructor(
fun handleEvents(event: TimelineEvents) { fun handleEvents(event: TimelineEvents) {
when (event) { when (event) {
TimelineEvents.LoadMore -> localCoroutineScope.loadMore(paginationState) TimelineEvents.LoadMore -> localCoroutineScope.paginateBackwards()
is TimelineEvents.SetHighlightedEvent -> highlightedEventId.value = event.eventId is TimelineEvents.SetHighlightedEvent -> highlightedEventId.value = event.eventId
is TimelineEvents.OnScrollFinished -> { is TimelineEvents.OnScrollFinished -> {
// Get last valid EventId seen by the user, as the first index might refer to a Virtual item // Get last valid EventId seen by the user, as the first index might refer to a Virtual item
@ -87,11 +85,6 @@ class TimelinePresenter @Inject constructor(
timeline timeline
.timelineItems .timelineItems
.onEach(timelineItemsFactory::replaceWith) .onEach(timelineItemsFactory::replaceWith)
.onEach { timelineItems ->
if (timelineItems.isEmpty()) {
loadMore(paginationState)
}
}
.launchIn(this) .launchIn(this)
} }
@ -113,12 +106,8 @@ class TimelinePresenter @Inject constructor(
return null return null
} }
private fun CoroutineScope.loadMore(paginationState: MatrixTimeline.PaginationState) = launch { private fun CoroutineScope.paginateBackwards() = launch {
if (paginationState.canBackPaginate && !paginationState.isBackPaginating) { timeline.paginateBackwards(backPaginationEventLimit, backPaginationPageSize)
timeline.paginateBackwards(backPaginationEventLimit, backPaginationPageSize)
} else {
Timber.v("Can't back paginate as paginationState = $paginationState")
}
} }
private fun CoroutineScope.sendReadReceipt(eventId: EventId) = launch { private fun CoroutineScope.sendReadReceipt(eventId: EventId) = launch {

View file

@ -40,7 +40,7 @@ import kotlin.random.Random
fun aTimelineState(timelineItems: ImmutableList<TimelineItem> = persistentListOf()) = TimelineState( fun aTimelineState(timelineItems: ImmutableList<TimelineItem> = persistentListOf()) = TimelineState(
timelineItems = timelineItems, timelineItems = timelineItems,
paginationState = MatrixTimeline.PaginationState(isBackPaginating = false, canBackPaginate = true), paginationState = MatrixTimeline.PaginationState(isBackPaginating = false, hasMoreToLoadBackwards = true),
highlightedEventId = null, highlightedEventId = null,
canReply = true, canReply = true,
eventSink = {} eventSink = {}

View file

@ -134,7 +134,7 @@ fun TimelineView(
onSwipeToReply = onSwipeToReply, onSwipeToReply = onSwipeToReply,
) )
} }
if (state.paginationState.canBackPaginate) { if (state.paginationState.hasMoreToLoadBackwards) {
// Do not use key parameter to avoid wrong positioning // Do not use key parameter to avoid wrong positioning
item(contentType = "TimelineLoadingMoreIndicator") { item(contentType = "TimelineLoadingMoreIndicator") {
TimelineLoadingMoreIndicator() TimelineLoadingMoreIndicator()

View file

@ -59,14 +59,14 @@ class TimelinePresenterTest {
presenter.present() presenter.present()
}.test { }.test {
val initialState = awaitItem() val initialState = awaitItem()
assertThat(initialState.paginationState.canBackPaginate).isTrue() assertThat(initialState.paginationState.hasMoreToLoadBackwards).isTrue()
assertThat(initialState.paginationState.isBackPaginating).isFalse() assertThat(initialState.paginationState.isBackPaginating).isFalse()
initialState.eventSink.invoke(TimelineEvents.LoadMore) initialState.eventSink.invoke(TimelineEvents.LoadMore)
val inPaginationState = awaitItem() val inPaginationState = awaitItem()
assertThat(inPaginationState.paginationState.isBackPaginating).isTrue() assertThat(inPaginationState.paginationState.isBackPaginating).isTrue()
assertThat(inPaginationState.paginationState.canBackPaginate).isTrue() assertThat(inPaginationState.paginationState.hasMoreToLoadBackwards).isTrue()
val postPaginationState = awaitItem() val postPaginationState = awaitItem()
assertThat(postPaginationState.paginationState.canBackPaginate).isTrue() assertThat(postPaginationState.paginationState.hasMoreToLoadBackwards).isTrue()
assertThat(postPaginationState.paginationState.isBackPaginating).isFalse() assertThat(postPaginationState.paginationState.isBackPaginating).isFalse()
} }
} }

View file

@ -0,0 +1,21 @@
/*
* Copyright (c) 2023 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.api.timeline
sealed class TimelineException: Exception() {
object CannotPaginate: TimelineException()
}

View file

@ -24,8 +24,10 @@ interface MatrixTimeline {
data class PaginationState( data class PaginationState(
val isBackPaginating: Boolean, val isBackPaginating: Boolean,
val canBackPaginate: Boolean val hasMoreToLoadBackwards: Boolean
) ) {
val canBackPaginate = !isBackPaginating && hasMoreToLoadBackwards
}
val paginationState: StateFlow<PaginationState> val paginationState: StateFlow<PaginationState>
val timelineItems: Flow<List<MatrixTimelineItem>> val timelineItems: Flow<List<MatrixTimelineItem>>

View file

@ -17,7 +17,6 @@
package io.element.android.libraries.matrix.impl.timeline package io.element.android.libraries.matrix.impl.timeline
import io.element.android.libraries.matrix.api.timeline.MatrixTimelineItem import io.element.android.libraries.matrix.api.timeline.MatrixTimelineItem
import kotlinx.coroutines.CompletableDeferred
import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.MutableStateFlow
import kotlinx.coroutines.sync.Mutex import kotlinx.coroutines.sync.Mutex
import kotlinx.coroutines.sync.withLock import kotlinx.coroutines.sync.withLock
@ -30,20 +29,16 @@ internal class MatrixTimelineDiffProcessor(
private val timelineItemFactory: MatrixTimelineItemMapper, private val timelineItemFactory: MatrixTimelineItemMapper,
) { ) {
private val initLatch = CompletableDeferred<Unit>()
private val mutex = Mutex() private val mutex = Mutex()
suspend fun postItems(items: List<TimelineItem>) { suspend fun postItems(items: List<TimelineItem>) {
updateTimelineItems { updateTimelineItems {
val mappedItems = items.map { it.asMatrixTimelineItem() } val mappedItems = items.map { it.asMatrixTimelineItem() }
addAll(mappedItems) addAll(0, mappedItems)
} }
initLatch.complete(Unit)
} }
suspend fun postDiff(diff: TimelineDiff) { suspend fun postDiff(diff: TimelineDiff) {
// Makes sure to process first items before diff.
initLatch.await()
updateTimelineItems { updateTimelineItems {
applyDiff(diff) applyDiff(diff)
} }

View file

@ -20,10 +20,12 @@ import io.element.android.libraries.matrix.api.core.EventId
import io.element.android.libraries.matrix.api.room.MatrixRoom import io.element.android.libraries.matrix.api.room.MatrixRoom
import io.element.android.libraries.matrix.api.timeline.MatrixTimeline import io.element.android.libraries.matrix.api.timeline.MatrixTimeline
import io.element.android.libraries.matrix.api.timeline.MatrixTimelineItem import io.element.android.libraries.matrix.api.timeline.MatrixTimelineItem
import io.element.android.libraries.matrix.api.timeline.TimelineException
import io.element.android.libraries.matrix.impl.timeline.item.event.EventMessageMapper import io.element.android.libraries.matrix.impl.timeline.item.event.EventMessageMapper
import io.element.android.libraries.matrix.impl.timeline.item.event.EventTimelineItemMapper import io.element.android.libraries.matrix.impl.timeline.item.event.EventTimelineItemMapper
import io.element.android.libraries.matrix.impl.timeline.item.event.TimelineEventContentMapper import io.element.android.libraries.matrix.impl.timeline.item.event.TimelineEventContentMapper
import io.element.android.libraries.matrix.impl.timeline.item.virtual.VirtualTimelineItemMapper import io.element.android.libraries.matrix.impl.timeline.item.virtual.VirtualTimelineItemMapper
import kotlinx.coroutines.CompletableDeferred
import kotlinx.coroutines.CoroutineDispatcher import kotlinx.coroutines.CoroutineDispatcher
import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.FlowPreview import kotlinx.coroutines.FlowPreview
@ -40,6 +42,8 @@ import org.matrix.rustcomponents.sdk.Room
import org.matrix.rustcomponents.sdk.TimelineDiff import org.matrix.rustcomponents.sdk.TimelineDiff
import org.matrix.rustcomponents.sdk.TimelineItem import org.matrix.rustcomponents.sdk.TimelineItem
import timber.log.Timber import timber.log.Timber
import java.util.concurrent.atomic.AtomicBoolean
class RustMatrixTimeline( class RustMatrixTimeline(
roomCoroutineScope: CoroutineScope, roomCoroutineScope: CoroutineScope,
@ -48,11 +52,14 @@ class RustMatrixTimeline(
private val dispatcher: CoroutineDispatcher, private val dispatcher: CoroutineDispatcher,
) : MatrixTimeline { ) : MatrixTimeline {
private val initLatch = CompletableDeferred<Unit>()
private val isInit = AtomicBoolean(false)
private val _timelineItems: MutableStateFlow<List<MatrixTimelineItem>> = private val _timelineItems: MutableStateFlow<List<MatrixTimelineItem>> =
MutableStateFlow(emptyList()) MutableStateFlow(emptyList())
private val _paginationState = MutableStateFlow( private val _paginationState = MutableStateFlow(
MatrixTimeline.PaginationState(canBackPaginate = true, isBackPaginating = false) MatrixTimeline.PaginationState(hasMoreToLoadBackwards = true, isBackPaginating = false)
) )
private val timelineItemFactory = MatrixTimelineItemMapper( private val timelineItemFactory = MatrixTimelineItemMapper(
@ -78,9 +85,12 @@ class RustMatrixTimeline(
internal suspend fun postItems(items: List<TimelineItem>) { internal suspend fun postItems(items: List<TimelineItem>) {
timelineDiffProcessor.postItems(items) timelineDiffProcessor.postItems(items)
isInit.set(true)
initLatch.complete(Unit)
} }
internal suspend fun postDiff(timelineDiff: TimelineDiff) { internal suspend fun postDiff(timelineDiff: TimelineDiff) {
initLatch.await()
timelineDiffProcessor.postDiff(timelineDiff) timelineDiffProcessor.postDiff(timelineDiff)
} }
@ -90,19 +100,19 @@ class RustMatrixTimeline(
BackPaginationStatus.IDLE -> { BackPaginationStatus.IDLE -> {
currentPaginationState.copy( currentPaginationState.copy(
isBackPaginating = false, isBackPaginating = false,
canBackPaginate = true hasMoreToLoadBackwards = true
) )
} }
BackPaginationStatus.PAGINATING -> { BackPaginationStatus.PAGINATING -> {
currentPaginationState.copy( currentPaginationState.copy(
isBackPaginating = true, isBackPaginating = true,
canBackPaginate = true hasMoreToLoadBackwards = true
) )
} }
BackPaginationStatus.TIMELINE_START_REACHED -> { BackPaginationStatus.TIMELINE_START_REACHED -> {
currentPaginationState.copy( currentPaginationState.copy(
isBackPaginating = false, isBackPaginating = false,
canBackPaginate = false hasMoreToLoadBackwards = false
) )
} }
} }
@ -117,6 +127,7 @@ class RustMatrixTimeline(
override suspend fun paginateBackwards(requestSize: Int, untilNumberOfItems: Int): Result<Unit> = withContext(dispatcher) { override suspend fun paginateBackwards(requestSize: Int, untilNumberOfItems: Int): Result<Unit> = withContext(dispatcher) {
runCatching { runCatching {
if (!canBackPaginate()) throw TimelineException.CannotPaginate
Timber.v("Start back paginating for room ${matrixRoom.roomId} ") Timber.v("Start back paginating for room ${matrixRoom.roomId} ")
val paginationOptions = PaginationOptions.UntilNumItems( val paginationOptions = PaginationOptions.UntilNumItems(
eventLimit = requestSize.toUShort(), eventLimit = requestSize.toUShort(),
@ -125,12 +136,16 @@ class RustMatrixTimeline(
) )
innerRoom.paginateBackwards(paginationOptions) innerRoom.paginateBackwards(paginationOptions)
}.onFailure { }.onFailure {
Timber.e(it, "Fail to paginate for room ${matrixRoom.roomId}") Timber.d(it, "Fail to paginate for room ${matrixRoom.roomId}")
}.onSuccess { }.onSuccess {
Timber.v("Success back paginating for room ${matrixRoom.roomId}") Timber.v("Success back paginating for room ${matrixRoom.roomId}")
} }
} }
private fun canBackPaginate(): Boolean {
return isInit.get() && paginationState.value.canBackPaginate
}
override suspend fun sendReadReceipt(eventId: EventId) = withContext(dispatcher) { override suspend fun sendReadReceipt(eventId: EventId) = withContext(dispatcher) {
runCatching { runCatching {
innerRoom.sendReadReceipt(eventId = eventId.value) innerRoom.sendReadReceipt(eventId = eventId.value)

View file

@ -27,7 +27,7 @@ import kotlinx.coroutines.flow.getAndUpdate
class FakeMatrixTimeline( class FakeMatrixTimeline(
initialTimelineItems: List<MatrixTimelineItem> = emptyList(), initialTimelineItems: List<MatrixTimelineItem> = emptyList(),
initialPaginationState: MatrixTimeline.PaginationState = MatrixTimeline.PaginationState(canBackPaginate = true, isBackPaginating = false) initialPaginationState: MatrixTimeline.PaginationState = MatrixTimeline.PaginationState(hasMoreToLoadBackwards = true, isBackPaginating = false)
) : MatrixTimeline { ) : MatrixTimeline {
private val _paginationState: MutableStateFlow<MatrixTimeline.PaginationState> = MutableStateFlow(initialPaginationState) private val _paginationState: MutableStateFlow<MatrixTimeline.PaginationState> = MutableStateFlow(initialPaginationState)