diff --git a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/TimelinePresenter.kt b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/TimelinePresenter.kt index 765becfd11..913a878b12 100644 --- a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/TimelinePresenter.kt +++ b/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 @@ -87,11 +85,6 @@ class TimelinePresenter @Inject constructor( timeline .timelineItems .onEach(timelineItemsFactory::replaceWith) - .onEach { timelineItems -> - if (timelineItems.isEmpty()) { - loadMore(paginationState) - } - } .launchIn(this) } @@ -113,12 +106,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 { diff --git a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/TimelineStateProvider.kt b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/TimelineStateProvider.kt index e96d7ac0f3..e17852ee0e 100644 --- a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/TimelineStateProvider.kt +++ b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/TimelineStateProvider.kt @@ -40,7 +40,7 @@ import kotlin.random.Random fun aTimelineState(timelineItems: ImmutableList = persistentListOf()) = TimelineState( timelineItems = timelineItems, - paginationState = MatrixTimeline.PaginationState(isBackPaginating = false, canBackPaginate = true), + paginationState = MatrixTimeline.PaginationState(isBackPaginating = false, hasMoreToLoadBackwards = true), highlightedEventId = null, canReply = true, eventSink = {} diff --git a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/TimelineView.kt b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/TimelineView.kt index bb33981a64..ee7a57c70e 100644 --- a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/TimelineView.kt +++ b/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() diff --git a/features/messages/impl/src/test/kotlin/io/element/android/features/messages/timeline/TimelinePresenterTest.kt b/features/messages/impl/src/test/kotlin/io/element/android/features/messages/timeline/TimelinePresenterTest.kt index 9151376d4d..dc3f28d527 100644 --- a/features/messages/impl/src/test/kotlin/io/element/android/features/messages/timeline/TimelinePresenterTest.kt +++ b/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() } } diff --git a/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/timeline/CannotPaginateException.kt b/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/timeline/CannotPaginateException.kt new file mode 100644 index 0000000000..82a87631c3 --- /dev/null +++ b/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/timeline/CannotPaginateException.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() +} diff --git a/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/timeline/MatrixTimeline.kt b/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/timeline/MatrixTimeline.kt index b74a34bd01..af411216e5 100644 --- a/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/timeline/MatrixTimeline.kt +++ b/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 val timelineItems: Flow> diff --git a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/timeline/MatrixTimelineDiffProcessor.kt b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/timeline/MatrixTimelineDiffProcessor.kt index dd0198cf18..b14d459697 100644 --- a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/timeline/MatrixTimelineDiffProcessor.kt +++ b/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() private val mutex = Mutex() suspend fun postItems(items: List) { 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) } diff --git a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/timeline/RustMatrixTimeline.kt b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/timeline/RustMatrixTimeline.kt index 2c3a579b2f..de97d48a8a 100644 --- a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/timeline/RustMatrixTimeline.kt +++ b/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,8 @@ 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 + class RustMatrixTimeline( roomCoroutineScope: CoroutineScope, @@ -48,11 +52,14 @@ class RustMatrixTimeline( private val dispatcher: CoroutineDispatcher, ) : MatrixTimeline { + private val initLatch = CompletableDeferred() + private val isInit = AtomicBoolean(false) + private val _timelineItems: MutableStateFlow> = MutableStateFlow(emptyList()) private val _paginationState = MutableStateFlow( - MatrixTimeline.PaginationState(canBackPaginate = true, isBackPaginating = false) + MatrixTimeline.PaginationState(hasMoreToLoadBackwards = true, isBackPaginating = false) ) private val timelineItemFactory = MatrixTimelineItemMapper( @@ -78,9 +85,12 @@ class RustMatrixTimeline( internal suspend fun postItems(items: List) { timelineDiffProcessor.postItems(items) + isInit.set(true) + initLatch.complete(Unit) } internal suspend fun postDiff(timelineDiff: TimelineDiff) { + initLatch.await() timelineDiffProcessor.postDiff(timelineDiff) } @@ -90,19 +100,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 +127,7 @@ class RustMatrixTimeline( override suspend fun paginateBackwards(requestSize: Int, untilNumberOfItems: Int): Result = 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 +136,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) diff --git a/libraries/matrix/test/src/main/kotlin/io/element/android/libraries/matrix/test/timeline/FakeMatrixTimeline.kt b/libraries/matrix/test/src/main/kotlin/io/element/android/libraries/matrix/test/timeline/FakeMatrixTimeline.kt index db31fbd8f0..b49a5490ce 100644 --- a/libraries/matrix/test/src/main/kotlin/io/element/android/libraries/matrix/test/timeline/FakeMatrixTimeline.kt +++ b/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 = emptyList(), - initialPaginationState: MatrixTimeline.PaginationState = MatrixTimeline.PaginationState(canBackPaginate = true, isBackPaginating = false) + initialPaginationState: MatrixTimeline.PaginationState = MatrixTimeline.PaginationState(hasMoreToLoadBackwards = true, isBackPaginating = false) ) : MatrixTimeline { private val _paginationState: MutableStateFlow = MutableStateFlow(initialPaginationState)