Merge pull request #863 from vector-im/feature/fga/timeline_pagination

Feature/fga/timeline pagination

Author: ganfra

features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/TimelinePresenter.kt

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

features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/TimelineStateProvider.kt

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

features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/TimelineView.kt

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

features/messages/impl/src/test/kotlin/io/element/android/features/messages/timeline/TimelinePresenterTest.kt

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

libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/timeline/MatrixTimeline.kt

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

libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/timeline/TimelineException.kt

@@ -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()
+}

libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/timeline/MatrixTimelineDiffProcessor.kt

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

libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/timeline/RustMatrixTimeline.kt

@@ -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.timeline.MatrixTimeline
 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.EventTimelineItemMapper
 import io.element.android.libraries.matrix.impl.timeline.item.event.TimelineEventContentMapper
 import io.element.android.libraries.matrix.impl.timeline.item.virtual.VirtualTimelineItemMapper
+import kotlinx.coroutines.CompletableDeferred
 import kotlinx.coroutines.CoroutineDispatcher
 import kotlinx.coroutines.CoroutineScope
 import kotlinx.coroutines.FlowPreview
@@ -40,6 +42,9 @@ import org.matrix.rustcomponents.sdk.Room
 import org.matrix.rustcomponents.sdk.TimelineDiff
 import org.matrix.rustcomponents.sdk.TimelineItem
 import timber.log.Timber
+import java.util.concurrent.atomic.AtomicBoolean
+
+private const val INITIAL_MAX_SIZE = 50
 
 class RustMatrixTimeline(
     roomCoroutineScope: CoroutineScope,
@@ -48,11 +53,14 @@ class RustMatrixTimeline(
     private val dispatcher: CoroutineDispatcher,
 ) : MatrixTimeline {
 
+    private val initLatch = CompletableDeferred<Unit>()
+    private val isInit = AtomicBoolean(false)
+
     private val _timelineItems: MutableStateFlow<List<MatrixTimelineItem>> =
         MutableStateFlow(emptyList())
 
     private val _paginationState = MutableStateFlow(
-        MatrixTimeline.PaginationState(canBackPaginate = true, isBackPaginating = false)
+        MatrixTimeline.PaginationState(hasMoreToLoadBackwards = true, isBackPaginating = false)
     )
 
     private val timelineItemFactory = MatrixTimelineItemMapper(
@@ -77,10 +85,16 @@ class RustMatrixTimeline(
     override val timelineItems: Flow<List<MatrixTimelineItem>> = _timelineItems.sample(50)
 
     internal suspend fun postItems(items: List<TimelineItem>) {
-        timelineDiffProcessor.postItems(items)
+        // Split the initial items in multiple list as there is no pagination in the cached data, so we can post timelineItems asap.
+        items.chunked(INITIAL_MAX_SIZE).reversed().forEach {
+            timelineDiffProcessor.postItems(it)
+        }
+        isInit.set(true)
+        initLatch.complete(Unit)
     }
 
     internal suspend fun postDiff(timelineDiff: TimelineDiff) {
+        initLatch.await()
         timelineDiffProcessor.postDiff(timelineDiff)
     }
 
@@ -90,19 +104,19 @@ class RustMatrixTimeline(
                 BackPaginationStatus.IDLE -> {
                     currentPaginationState.copy(
                         isBackPaginating = false,
-                        canBackPaginate = true
+                        hasMoreToLoadBackwards = true
                     )
                 }
                 BackPaginationStatus.PAGINATING -> {
                     currentPaginationState.copy(
                         isBackPaginating = true,
-                        canBackPaginate = true
+                        hasMoreToLoadBackwards = true
                     )
                 }
                 BackPaginationStatus.TIMELINE_START_REACHED -> {
                     currentPaginationState.copy(
                         isBackPaginating = false,
-                        canBackPaginate = false
+                        hasMoreToLoadBackwards = false
                     )
                 }
             }
@@ -117,6 +131,7 @@ class RustMatrixTimeline(
 
     override suspend fun paginateBackwards(requestSize: Int, untilNumberOfItems: Int): Result<Unit> = withContext(dispatcher) {
         runCatching {
+            if (!canBackPaginate()) throw TimelineException.CannotPaginate
             Timber.v("Start back paginating for room ${matrixRoom.roomId} ")
             val paginationOptions = PaginationOptions.UntilNumItems(
                 eventLimit = requestSize.toUShort(),
@@ -125,12 +140,16 @@ class RustMatrixTimeline(
             )
             innerRoom.paginateBackwards(paginationOptions)
         }.onFailure {
-            Timber.e(it, "Fail to paginate for room ${matrixRoom.roomId}")
+            Timber.d(it, "Fail to paginate for room ${matrixRoom.roomId}")
         }.onSuccess {
             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) {
         runCatching {
             innerRoom.sendReadReceipt(eventId = eventId.value)

libraries/matrix/test/src/main/kotlin/io/element/android/libraries/matrix/test/timeline/FakeMatrixTimeline.kt

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