From 31a7d3f3bb248bfdcf0c9943cb09fc3a890573b7 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Thu, 23 Jan 2025 17:44:31 +0100 Subject: [PATCH] Fix pagination restart issue and cover by unit test. --- .../matrix/impl/timeline/RustTimeline.kt | 6 +- .../impl/fixtures/fakes/FakeRustTimeline.kt | 14 ++ .../matrix/impl/timeline/RustTimelineTest.kt | 124 ++++++++++++++++++ .../test/systemclock/FakeSystemClock.kt | 8 +- 4 files changed, 144 insertions(+), 8 deletions(-) create mode 100644 libraries/matrix/impl/src/test/kotlin/io/element/android/libraries/matrix/impl/timeline/RustTimelineTest.kt diff --git a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/timeline/RustTimeline.kt b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/timeline/RustTimeline.kt index 3e10ce1a5b..4908d73d2e 100644 --- a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/timeline/RustTimeline.kt +++ b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/timeline/RustTimeline.kt @@ -55,8 +55,6 @@ import kotlinx.coroutines.flow.MutableSharedFlow 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 @@ -213,8 +211,8 @@ class RustTimeline( override val timelineItems: Flow> = combine( _timelineItems, - backPaginationStatus.filter { !it.isPaginating }.distinctUntilChanged(), - forwardPaginationStatus.filter { !it.isPaginating }.distinctUntilChanged(), + backPaginationStatus, + forwardPaginationStatus, matrixRoom.roomInfoFlow.map { it.creator }, isTimelineInitialized, ) { timelineItems, diff --git a/libraries/matrix/impl/src/test/kotlin/io/element/android/libraries/matrix/impl/fixtures/fakes/FakeRustTimeline.kt b/libraries/matrix/impl/src/test/kotlin/io/element/android/libraries/matrix/impl/fixtures/fakes/FakeRustTimeline.kt index e269a30b1b..042d4c98a1 100644 --- a/libraries/matrix/impl/src/test/kotlin/io/element/android/libraries/matrix/impl/fixtures/fakes/FakeRustTimeline.kt +++ b/libraries/matrix/impl/src/test/kotlin/io/element/android/libraries/matrix/impl/fixtures/fakes/FakeRustTimeline.kt @@ -8,10 +8,12 @@ package io.element.android.libraries.matrix.impl.fixtures.fakes import org.matrix.rustcomponents.sdk.NoPointer +import org.matrix.rustcomponents.sdk.PaginationStatusListener import org.matrix.rustcomponents.sdk.TaskHandle import org.matrix.rustcomponents.sdk.Timeline import org.matrix.rustcomponents.sdk.TimelineDiff import org.matrix.rustcomponents.sdk.TimelineListener +import uniffi.matrix_sdk_ui.LiveBackPaginationStatus class FakeRustTimeline : Timeline(NoPointer) { private var listener: TimelineListener? = null @@ -23,4 +25,16 @@ class FakeRustTimeline : Timeline(NoPointer) { fun emitDiff(diff: List) { listener!!.onUpdate(diff) } + + private var paginationStatusListener: PaginationStatusListener? = null + override suspend fun subscribeToBackPaginationStatus(listener: PaginationStatusListener): TaskHandle { + this.paginationStatusListener = listener + return FakeRustTaskHandle() + } + + fun emitPaginationStatus(status: LiveBackPaginationStatus) { + paginationStatusListener!!.onUpdate(status) + } + + override suspend fun fetchMembers() = Unit } diff --git a/libraries/matrix/impl/src/test/kotlin/io/element/android/libraries/matrix/impl/timeline/RustTimelineTest.kt b/libraries/matrix/impl/src/test/kotlin/io/element/android/libraries/matrix/impl/timeline/RustTimelineTest.kt new file mode 100644 index 0000000000..8e9120a839 --- /dev/null +++ b/libraries/matrix/impl/src/test/kotlin/io/element/android/libraries/matrix/impl/timeline/RustTimelineTest.kt @@ -0,0 +1,124 @@ +/* + * Copyright 2025 New Vector Ltd. + * + * SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Element-Commercial + * Please see LICENSE files in the repository root for full details. + */ +@file:OptIn(ExperimentalCoroutinesApi::class) + +package io.element.android.libraries.matrix.impl.timeline + +import app.cash.turbine.test +import com.google.common.truth.Truth.assertThat +import io.element.android.libraries.featureflag.api.FeatureFlagService +import io.element.android.libraries.featureflag.test.FakeFeatureFlagService +import io.element.android.libraries.matrix.api.room.MatrixRoom +import io.element.android.libraries.matrix.api.timeline.MatrixTimelineItem +import io.element.android.libraries.matrix.api.timeline.Timeline +import io.element.android.libraries.matrix.api.timeline.item.virtual.VirtualTimelineItem +import io.element.android.libraries.matrix.impl.fixtures.fakes.FakeRustRoomListService +import io.element.android.libraries.matrix.impl.fixtures.fakes.FakeRustTimeline +import io.element.android.libraries.matrix.impl.fixtures.fakes.FakeRustTimelineDiff +import io.element.android.libraries.matrix.impl.room.RoomContentForwarder +import io.element.android.libraries.matrix.test.room.FakeMatrixRoom +import io.element.android.libraries.matrix.test.room.aRoomInfo +import io.element.android.services.toolbox.api.systemclock.SystemClock +import io.element.android.services.toolbox.test.systemclock.A_FAKE_TIMESTAMP +import io.element.android.services.toolbox.test.systemclock.FakeSystemClock +import io.element.android.tests.testutils.testCoroutineDispatchers +import kotlinx.coroutines.CoroutineDispatcher +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.test.TestScope +import kotlinx.coroutines.test.runCurrent +import kotlinx.coroutines.test.runTest +import org.junit.Test +import org.matrix.rustcomponents.sdk.TimelineChange +import uniffi.matrix_sdk_ui.LiveBackPaginationStatus +import org.matrix.rustcomponents.sdk.Timeline as InnerTimeline + +class RustTimelineTest { + @Test + fun `ensure that the timeline emits new loading item when pagination does not bring new events`() = runTest { + val inner = FakeRustTimeline() + val systemClock = FakeSystemClock() + val sut = createRustTimeline( + inner = inner, + systemClock = systemClock, + ) + sut.timelineItems.test { + // Give time for the listener to be set + runCurrent() + inner.emitDiff( + listOf( + FakeRustTimelineDiff( + item = null, + change = TimelineChange.RESET, + ) + ) + ) + with(awaitItem()) { + assertThat(size).isEqualTo(1) + // Typing notification + assertThat((get(0) as MatrixTimelineItem.Virtual).virtual).isEqualTo(VirtualTimelineItem.TypingNotification) + } + with(awaitItem()) { + assertThat(size).isEqualTo(2) + // The loading + assertThat((get(0) as MatrixTimelineItem.Virtual).virtual).isEqualTo( + VirtualTimelineItem.LoadingIndicator( + direction = Timeline.PaginationDirection.BACKWARDS, + timestamp = A_FAKE_TIMESTAMP, + ) + ) + // Typing notification + assertThat((get(1) as MatrixTimelineItem.Virtual).virtual).isEqualTo(VirtualTimelineItem.TypingNotification) + } + systemClock.epochMillisResult = A_FAKE_TIMESTAMP + 1 + // Start pagination + sut.paginate(Timeline.PaginationDirection.BACKWARDS) + // Simulate SDK starting pagination + inner.emitPaginationStatus(LiveBackPaginationStatus.Paginating) + // No new events received + // Simulate SDK stopping pagination, more event to load + inner.emitPaginationStatus(LiveBackPaginationStatus.Idle(hitStartOfTimeline = false)) + // expect an item to be emitted, with an updated timestamp + with(awaitItem()) { + assertThat(size).isEqualTo(2) + // The loading + assertThat((get(0) as MatrixTimelineItem.Virtual).virtual).isEqualTo( + VirtualTimelineItem.LoadingIndicator( + direction = Timeline.PaginationDirection.BACKWARDS, + timestamp = A_FAKE_TIMESTAMP + 1, + ) + ) + // Typing notification + assertThat((get(1) as MatrixTimelineItem.Virtual).virtual).isEqualTo(VirtualTimelineItem.TypingNotification) + } + } + } +} + +private fun TestScope.createRustTimeline( + inner: InnerTimeline, + mode: Timeline.Mode = Timeline.Mode.LIVE, + systemClock: SystemClock = FakeSystemClock(), + matrixRoom: MatrixRoom = FakeMatrixRoom().apply { givenRoomInfo(aRoomInfo()) }, + coroutineScope: CoroutineScope = backgroundScope, + dispatcher: CoroutineDispatcher = testCoroutineDispatchers().io, + roomContentForwarder: RoomContentForwarder = RoomContentForwarder(FakeRustRoomListService()), + featureFlagsService: FeatureFlagService = FakeFeatureFlagService(), + onNewSyncedEvent: () -> Unit = {}, +): RustTimeline { + return RustTimeline( + inner = inner, + mode = mode, + systemClock = systemClock, + matrixRoom = matrixRoom, + coroutineScope = coroutineScope, + dispatcher = dispatcher, + roomContentForwarder = roomContentForwarder, + featureFlagsService = featureFlagsService, + onNewSyncedEvent = onNewSyncedEvent, + ) +} diff --git a/services/toolbox/test/src/main/kotlin/io/element/android/services/toolbox/test/systemclock/FakeSystemClock.kt b/services/toolbox/test/src/main/kotlin/io/element/android/services/toolbox/test/systemclock/FakeSystemClock.kt index 3835b163ac..444502aea8 100644 --- a/services/toolbox/test/src/main/kotlin/io/element/android/services/toolbox/test/systemclock/FakeSystemClock.kt +++ b/services/toolbox/test/src/main/kotlin/io/element/android/services/toolbox/test/systemclock/FakeSystemClock.kt @@ -11,8 +11,8 @@ import io.element.android.services.toolbox.api.systemclock.SystemClock const val A_FAKE_TIMESTAMP = 123L -class FakeSystemClock : SystemClock { - override fun epochMillis(): Long { - return A_FAKE_TIMESTAMP - } +class FakeSystemClock( + var epochMillisResult: Long = A_FAKE_TIMESTAMP +) : SystemClock { + override fun epochMillis() = epochMillisResult }