Media timeline: improve pagination logic.

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

@@ -56,6 +56,7 @@ import kotlinx.coroutines.flow.MutableStateFlow
 import kotlinx.coroutines.flow.StateFlow
 import kotlinx.coroutines.flow.combine
 import kotlinx.coroutines.flow.distinctUntilChanged
+import kotlinx.coroutines.flow.filter
 import kotlinx.coroutines.flow.first
 import kotlinx.coroutines.flow.getAndUpdate
 import kotlinx.coroutines.flow.launchIn
@@ -64,8 +65,6 @@ import kotlinx.coroutines.flow.onCompletion
 import kotlinx.coroutines.flow.onEach
 import kotlinx.coroutines.flow.onStart
 import kotlinx.coroutines.launch
-import kotlinx.coroutines.sync.Mutex
-import kotlinx.coroutines.sync.withLock
 import kotlinx.coroutines.withContext
 import org.matrix.rustcomponents.sdk.EditedContent
 import org.matrix.rustcomponents.sdk.FormattedBody
@@ -172,36 +171,26 @@ class RustTimeline(
         }
     }
 
-    private val backwardsPaginationMutex = Mutex()
-    private val forwardsPaginationMutex = Mutex()
-
-    private fun getPaginationMutex(direction: Timeline.PaginationDirection) = when (direction) {
-        Timeline.PaginationDirection.BACKWARDS -> backwardsPaginationMutex
-        Timeline.PaginationDirection.FORWARDS -> forwardsPaginationMutex
-    }
-
     // Use NonCancellable to avoid breaking the timeline when the coroutine is cancelled.
     override suspend fun paginate(direction: Timeline.PaginationDirection): Result<Boolean> = withContext(NonCancellable) {
         withContext(dispatcher) {
             initLatch.await()
-            getPaginationMutex(direction).withLock {
+            runCatching {
-                runCatching {
+                if (!canPaginate(direction)) throw TimelineException.CannotPaginate
-                    if (!canPaginate(direction)) throw TimelineException.CannotPaginate
+                updatePaginationStatus(direction) { it.copy(isPaginating = true) }
-                    updatePaginationStatus(direction) { it.copy(isPaginating = true) }
+                when (direction) {
-                    when (direction) {
+                    Timeline.PaginationDirection.BACKWARDS -> inner.paginateBackwards(PAGINATION_SIZE.toUShort())
-                        Timeline.PaginationDirection.BACKWARDS -> inner.paginateBackwards(PAGINATION_SIZE.toUShort())
+                    Timeline.PaginationDirection.FORWARDS -> inner.focusedPaginateForwards(PAGINATION_SIZE.toUShort())
-                        Timeline.PaginationDirection.FORWARDS -> inner.focusedPaginateForwards(PAGINATION_SIZE.toUShort())
-                    }
-                }.onFailure { error ->
-                    updatePaginationStatus(direction) { it.copy(isPaginating = false) }
-                    if (error is TimelineException.CannotPaginate) {
-                        Timber.d("Can't paginate $direction on room ${matrixRoom.roomId} with paginationStatus: ${backPaginationStatus.value}")
-                    } else {
-                        Timber.e(error, "Error paginating $direction on room ${matrixRoom.roomId}")
-                    }
-                }.onSuccess { hasReachedEnd ->
-                    updatePaginationStatus(direction) { it.copy(isPaginating = false, hasMoreToLoad = !hasReachedEnd) }
                 }
+            }.onFailure { error ->
+                if (error is TimelineException.CannotPaginate) {
+                    Timber.d("Can't paginate $direction on room ${matrixRoom.roomId} with paginationStatus: ${backPaginationStatus.value}")
+                } else {
+                    updatePaginationStatus(direction) { it.copy(isPaginating = false) }
+                    Timber.e(error, "Error paginating $direction on room ${matrixRoom.roomId}")
+                }
+            }.onSuccess { hasReachedEnd ->
+                updatePaginationStatus(direction) { it.copy(isPaginating = false, hasMoreToLoad = !hasReachedEnd) }
             }
         }
     }
@@ -223,13 +212,13 @@ class RustTimeline(
 
     override val timelineItems: Flow<List<MatrixTimelineItem>> = combine(
         _timelineItems,
-        backPaginationStatus.map { it.hasMoreToLoad }.distinctUntilChanged(),
+        backPaginationStatus.filter { !it.isPaginating }.distinctUntilChanged(),
-        forwardPaginationStatus.map { it.hasMoreToLoad }.distinctUntilChanged(),
+        forwardPaginationStatus.filter { !it.isPaginating }.distinctUntilChanged(),
         matrixRoom.roomInfoFlow.map { it.creator },
         isTimelineInitialized,
     ) { timelineItems,
-        hasMoreToLoadBackward,
+        backwardPaginationStatus,
-        hasMoreToLoadForward,
+        forwardPaginationStatus,
         roomCreator,
         isTimelineInitialized ->
         withContext(dispatcher) {
@@ -239,15 +228,15 @@ class RustTimeline(
                         items = items,
                         isDm = matrixRoom.isDm,
                         roomCreator = roomCreator,
-                        hasMoreToLoadBackwards = hasMoreToLoadBackward,
+                        hasMoreToLoadBackwards = backwardPaginationStatus.hasMoreToLoad,
                     )
                 }
                 .let { items ->
                     loadingIndicatorsPostProcessor.process(
                         items = items,
                         isTimelineInitialized = isTimelineInitialized,
-                        hasMoreToLoadBackward = hasMoreToLoadBackward,
+                        hasMoreToLoadBackward = backwardPaginationStatus.hasMoreToLoad,
-                        hasMoreToLoadForward = hasMoreToLoadForward
+                        hasMoreToLoadForward = forwardPaginationStatus.hasMoreToLoad,
                     )
                 }
                 .let { items ->

libraries/mediaviewer/impl/src/main/kotlin/io/element/android/libraries/mediaviewer/impl/gallery/MediaGalleryPresenter.kt

@@ -183,14 +183,6 @@ class MediaGalleryPresenter @AssistedInject constructor(
                         }
                         .launchIn(this)
 
-                    timeline.data.paginationStatus(Timeline.PaginationDirection.BACKWARDS)
-                        .onEach { backwardPaginationStatus ->
-                            if (backwardPaginationStatus.canPaginate) {
-                                timelineMediaItemsFactory.onCanPaginate()
-                            }
-                        }
-                        .launchIn(this)
-
                     timelineMediaItemsFactory.timelineItems.map { timelineItems ->
                         AsyncData.Success(timelineItems)
                     }

libraries/mediaviewer/impl/src/main/kotlin/io/element/android/libraries/mediaviewer/impl/gallery/TimelineMediaItemsFactory.kt

@@ -14,7 +14,6 @@ import io.element.android.libraries.androidutils.diff.MutableListDiffCache
 import io.element.android.libraries.core.coroutine.CoroutineDispatchers
 import io.element.android.libraries.di.RoomScope
 import io.element.android.libraries.matrix.api.timeline.MatrixTimelineItem
-import io.element.android.services.toolbox.api.systemclock.SystemClock
 import kotlinx.collections.immutable.ImmutableList
 import kotlinx.collections.immutable.toPersistentList
 import kotlinx.coroutines.flow.Flow
@@ -23,14 +22,11 @@ import kotlinx.coroutines.flow.distinctUntilChanged
 import kotlinx.coroutines.sync.Mutex
 import kotlinx.coroutines.sync.withLock
 import kotlinx.coroutines.withContext
-import timber.log.Timber
 import javax.inject.Inject
 
 interface TimelineMediaItemsFactory {
     val timelineItems: Flow<ImmutableList<MediaItem>>
-
     suspend fun replaceWith(timelineItems: List<MatrixTimelineItem>)
-    suspend fun onCanPaginate()
 }
 
 @ContributesBinding(RoomScope::class)
@@ -38,7 +34,6 @@ class DefaultTimelineMediaItemsFactory @Inject constructor(
     private val dispatchers: CoroutineDispatchers,
     private val virtualItemFactory: VirtualItemFactory,
     private val eventItemFactory: EventItemFactory,
-    private val systemClock: SystemClock,
 ) : TimelineMediaItemsFactory {
     private val _timelineItems = MutableSharedFlow<ImmutableList<MediaItem>>(replay = 1)
     private val lock = Mutex()
@@ -66,26 +61,6 @@ class DefaultTimelineMediaItemsFactory @Inject constructor(
         }
     }
 
-    /**
-     * Update the timestamp of the loading indicator, so that it may trigger a new pagination request.
-     */
-    override suspend fun onCanPaginate() {
-        lock.withLock {
-            val values = _timelineItems.replayCache.firstOrNull() ?: return@withLock
-            val lastItem = values.lastOrNull()
-            if (lastItem is MediaItem.LoadingIndicator) {
-                val newList = values.toMutableList().apply {
-                    removeAt(size - 1)
-                    val newTs = systemClock.epochMillis()
-                    add(lastItem.copy(timestamp = newTs))
-                }
-                _timelineItems.emit(newList.toPersistentList())
-            } else {
-                Timber.w("onCanPaginate called but last item is not a loading indicator")
-            }
-        }
-    }
-
     private suspend fun buildAndEmitTimelineItemStates(
         timelineItems: List<MatrixTimelineItem>,
     ) {

libraries/mediaviewer/impl/src/test/kotlin/io/element/android/libraries/mediaviewer/impl/gallery/FakeTimelineMediaItemsFactory.kt

@@ -16,7 +16,6 @@ import kotlinx.coroutines.flow.flowOf
 
 class FakeTimelineMediaItemsFactory(
     private val replaceWithLambda: (List<MatrixTimelineItem>) -> Unit = { lambdaError() },
-    private val onCanPaginateLambda: () -> Unit = { lambdaError() }
 ) : TimelineMediaItemsFactory {
     override val timelineItems: Flow<ImmutableList<MediaItem>>
         get() = flowOf(emptyList<MediaItem>().toImmutableList())
@@ -24,8 +23,4 @@ class FakeTimelineMediaItemsFactory(
     override suspend fun replaceWith(timelineItems: List<MatrixTimelineItem>) {
         replaceWithLambda(timelineItems)
     }
-
-    override suspend fun onCanPaginate() {
-        onCanPaginateLambda()
-    }
 }

libraries/mediaviewer/impl/src/test/kotlin/io/element/android/libraries/mediaviewer/impl/gallery/MediaGalleryPresenterTest.kt

@@ -247,7 +247,6 @@ class MediaGalleryPresenterTest {
             room = room,
             timelineMediaItemsFactory = FakeTimelineMediaItemsFactory(
                 replaceWithLambda = lambdaRecorder<List<MatrixTimelineItem>, Unit> { _ -> },
-                onCanPaginateLambda = lambdaRecorder<Unit> { },
             ),
             localMediaFactory = localMediaFactory,
             mediaLoader = matrixMediaLoader,