From 7fc0ac1ceddda72a5a62feac7427ad0b148a461a Mon Sep 17 00:00:00 2001 From: ganfra Date: Fri, 15 Dec 2023 14:48:49 +0100 Subject: [PATCH 1/3] Timeline : uniqueId exposed as String directly from matrix module. --- .../event/TimelineItemEventFactory.kt | 2 +- .../virtual/TimelineItemVirtualFactory.kt | 7 +---- .../impl/timeline/TimelinePresenterTest.kt | 16 ++++++------ .../RedactedVoiceMessageManagerTest.kt | 2 +- .../features/poll/impl/PollFixtures.kt | 2 +- .../matrix/api/timeline/MatrixTimelineItem.kt | 4 +-- .../impl/timeline/MatrixTimelineItemMapper.kt | 2 +- .../TimelineEncryptedHistoryPostProcessor.kt | 2 +- ...melineEncryptedHistoryPostProcessorTest.kt | 26 +++++++++---------- 9 files changed, 29 insertions(+), 34 deletions(-) diff --git a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/factories/event/TimelineItemEventFactory.kt b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/factories/event/TimelineItemEventFactory.kt index 5e2d97e6a1..51520c66dc 100644 --- a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/factories/event/TimelineItemEventFactory.kt +++ b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/factories/event/TimelineItemEventFactory.kt @@ -80,7 +80,7 @@ class TimelineItemEventFactory @Inject constructor( ) currentTimelineItem.event return TimelineItem.Event( - id = currentTimelineItem.uniqueId.toString(), + id = currentTimelineItem.uniqueId, eventId = currentTimelineItem.eventId, transactionId = currentTimelineItem.transactionId, senderId = currentSender, diff --git a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/factories/virtual/TimelineItemVirtualFactory.kt b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/factories/virtual/TimelineItemVirtualFactory.kt index 6178b1dee7..cc513705e3 100644 --- a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/factories/virtual/TimelineItemVirtualFactory.kt +++ b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/factories/virtual/TimelineItemVirtualFactory.kt @@ -31,13 +31,8 @@ class TimelineItemVirtualFactory @Inject constructor( fun create( virtualTimelineItem: MatrixTimelineItem.Virtual, ): TimelineItem.Virtual { - val id = if (virtualTimelineItem.virtual is VirtualTimelineItem.EncryptedHistoryBanner) { - "encrypted_history_banner" - } else { - virtualTimelineItem.uniqueId.toString() - } return TimelineItem.Virtual( - id = id, + id = virtualTimelineItem.uniqueId, model = virtualTimelineItem.computeModel() ) } diff --git a/features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/timeline/TimelinePresenterTest.kt b/features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/timeline/TimelinePresenterTest.kt index 4316750a6a..640849c5f4 100644 --- a/features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/timeline/TimelinePresenterTest.kt +++ b/features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/timeline/TimelinePresenterTest.kt @@ -121,7 +121,7 @@ class TimelinePresenterTest { fun `present - on scroll finished send read receipt if an event is before the index`() = runTest { val timeline = FakeMatrixTimeline( initialTimelineItems = listOf( - MatrixTimelineItem.Event(0, anEventTimelineItem()) + MatrixTimelineItem.Event("0", anEventTimelineItem()) ) ) val presenter = createTimelinePresenter(timeline) @@ -145,7 +145,7 @@ class TimelinePresenterTest { fun `present - on scroll finished will not send read receipt if no event is before the index`() = runTest { val timeline = FakeMatrixTimeline( initialTimelineItems = listOf( - MatrixTimelineItem.Event(0, anEventTimelineItem()) + MatrixTimelineItem.Event("0", anEventTimelineItem()) ) ) val presenter = createTimelinePresenter(timeline) @@ -169,7 +169,7 @@ class TimelinePresenterTest { fun `present - on scroll finished will not send read receipt only virtual events exist before the index`() = runTest { val timeline = FakeMatrixTimeline( initialTimelineItems = listOf( - MatrixTimelineItem.Virtual(0, VirtualTimelineItem.ReadMarker) + MatrixTimelineItem.Virtual("0", VirtualTimelineItem.ReadMarker) ) ) val presenter = createTimelinePresenter(timeline) @@ -200,13 +200,13 @@ class TimelinePresenterTest { assertThat(initialState.newEventState).isEqualTo(NewEventState.None) assertThat(initialState.timelineItems.size).isEqualTo(0) timeline.updateTimelineItems { - listOf(MatrixTimelineItem.Event(0, anEventTimelineItem(content = aMessageContent()))) + listOf(MatrixTimelineItem.Event("0", anEventTimelineItem(content = aMessageContent()))) } consumeItemsUntilPredicate { it.timelineItems.size == 1 } // Mimics sending a message, and assert newEventState is FromMe timeline.updateTimelineItems { items -> val event = anEventTimelineItem(content = aMessageContent(), localSendState = LocalEventSendState.Sent(AN_EVENT_ID)) - items + listOf(MatrixTimelineItem.Event(1, event)) + items + listOf(MatrixTimelineItem.Event("1", event)) } consumeItemsUntilPredicate { it.timelineItems.size == 2 } awaitLastSequentialItem().also { state -> @@ -215,7 +215,7 @@ class TimelinePresenterTest { // Mimics receiving a message without clearing the previous FromMe timeline.updateTimelineItems { items -> val event = anEventTimelineItem(content = aMessageContent()) - items + listOf(MatrixTimelineItem.Event(2, event)) + items + listOf(MatrixTimelineItem.Event("2", event)) } consumeItemsUntilPredicate { it.timelineItems.size == 3 } @@ -227,7 +227,7 @@ class TimelinePresenterTest { // Mimics receiving a message and assert newEventState is FromOther timeline.updateTimelineItems { items -> val event = anEventTimelineItem(content = aMessageContent()) - items + listOf(MatrixTimelineItem.Event(3, event)) + items + listOf(MatrixTimelineItem.Event("3", event)) } consumeItemsUntilPredicate { it.timelineItems.size == 4 } awaitLastSequentialItem().also { state -> @@ -268,7 +268,7 @@ class TimelinePresenterTest { ), ) timeline.updateTimelineItems { - listOf(MatrixTimelineItem.Event(0, anEventTimelineItem(reactions = oneReaction))) + listOf(MatrixTimelineItem.Event("0", anEventTimelineItem(reactions = oneReaction))) } skipItems(1) val item = awaitItem().timelineItems.first() diff --git a/features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/voicemessages/timeline/RedactedVoiceMessageManagerTest.kt b/features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/voicemessages/timeline/RedactedVoiceMessageManagerTest.kt index 00eb7141c3..4cabc6450a 100644 --- a/features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/voicemessages/timeline/RedactedVoiceMessageManagerTest.kt +++ b/features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/voicemessages/timeline/RedactedVoiceMessageManagerTest.kt @@ -80,7 +80,7 @@ fun TestScope.aDefaultRedactedVoiceMessageManager( fun aRedactedMatrixTimeline(eventId: EventId) = listOf( MatrixTimelineItem.Event( - uniqueId = 0, + uniqueId = "0", event = EventTimelineItem( eventId = eventId, transactionId = null, diff --git a/features/poll/impl/src/test/kotlin/io/element/android/features/poll/impl/PollFixtures.kt b/features/poll/impl/src/test/kotlin/io/element/android/features/poll/impl/PollFixtures.kt index bfe925a993..b54b18b8c0 100644 --- a/features/poll/impl/src/test/kotlin/io/element/android/features/poll/impl/PollFixtures.kt +++ b/features/poll/impl/src/test/kotlin/io/element/android/features/poll/impl/PollFixtures.kt @@ -31,7 +31,7 @@ fun aPollTimeline( return FakeMatrixTimeline( initialTimelineItems = polls.map { entry -> MatrixTimelineItem.Event( - entry.key.hashCode().toLong(), + entry.key.value, anEventTimelineItem( eventId = entry.key, content = entry.value, diff --git a/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/timeline/MatrixTimelineItem.kt b/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/timeline/MatrixTimelineItem.kt index fe328a57d2..ae4e07a8f4 100644 --- a/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/timeline/MatrixTimelineItem.kt +++ b/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/timeline/MatrixTimelineItem.kt @@ -22,12 +22,12 @@ import io.element.android.libraries.matrix.api.timeline.item.event.EventTimeline import io.element.android.libraries.matrix.api.timeline.item.virtual.VirtualTimelineItem sealed interface MatrixTimelineItem { - data class Event(val uniqueId: Long, val event: EventTimelineItem) : MatrixTimelineItem { + data class Event(val uniqueId: String, val event: EventTimelineItem) : MatrixTimelineItem { val eventId: EventId? = event.eventId val transactionId: TransactionId? = event.transactionId } - data class Virtual(val uniqueId: Long, val virtual: VirtualTimelineItem) : MatrixTimelineItem + data class Virtual(val uniqueId: String, val virtual: VirtualTimelineItem) : MatrixTimelineItem data object Other : MatrixTimelineItem } diff --git a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/timeline/MatrixTimelineItemMapper.kt b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/timeline/MatrixTimelineItemMapper.kt index 2e86aa0706..beca5f17fb 100644 --- a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/timeline/MatrixTimelineItemMapper.kt +++ b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/timeline/MatrixTimelineItemMapper.kt @@ -32,7 +32,7 @@ class MatrixTimelineItemMapper( ) { fun map(timelineItem: TimelineItem): MatrixTimelineItem = timelineItem.use { - val uniqueId = timelineItem.uniqueId().toLong() + val uniqueId = timelineItem.uniqueId().toString() val asEvent = it.asEvent() if (asEvent != null) { val eventTimelineItem = eventTimelineItemMapper.map(asEvent) diff --git a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/timeline/postprocessor/TimelineEncryptedHistoryPostProcessor.kt b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/timeline/postprocessor/TimelineEncryptedHistoryPostProcessor.kt index 25241ff3e1..1a6b27203e 100644 --- a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/timeline/postprocessor/TimelineEncryptedHistoryPostProcessor.kt +++ b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/timeline/postprocessor/TimelineEncryptedHistoryPostProcessor.kt @@ -62,7 +62,7 @@ class TimelineEncryptedHistoryPostProcessor( } return if (lastEncryptedHistoryBannerIndex >= 0) { val sublist = list.drop(lastEncryptedHistoryBannerIndex + 1).toMutableList() - sublist.add(0, MatrixTimelineItem.Virtual(0L, VirtualTimelineItem.EncryptedHistoryBanner)) + sublist.add(0, MatrixTimelineItem.Virtual(VirtualTimelineItem.EncryptedHistoryBanner.toString(), VirtualTimelineItem.EncryptedHistoryBanner)) sublist } else { list diff --git a/libraries/matrix/impl/src/test/kotlin/io/element/android/libraries/matrix/impl/timeline/postprocessor/TimelineEncryptedHistoryPostProcessorTest.kt b/libraries/matrix/impl/src/test/kotlin/io/element/android/libraries/matrix/impl/timeline/postprocessor/TimelineEncryptedHistoryPostProcessorTest.kt index 029ae2336a..ccfddba40a 100644 --- a/libraries/matrix/impl/src/test/kotlin/io/element/android/libraries/matrix/impl/timeline/postprocessor/TimelineEncryptedHistoryPostProcessorTest.kt +++ b/libraries/matrix/impl/src/test/kotlin/io/element/android/libraries/matrix/impl/timeline/postprocessor/TimelineEncryptedHistoryPostProcessorTest.kt @@ -36,7 +36,7 @@ class TimelineEncryptedHistoryPostProcessorTest { fun `given an unencrypted room, nothing is done`() = runTest { val processor = createPostProcessor(isRoomEncrypted = false) val items = listOf( - MatrixTimelineItem.Event(0L, anEventTimelineItem()) + MatrixTimelineItem.Event("0L", anEventTimelineItem()) ) assertThat(processor.process(items)).isSameInstanceAs(items) } @@ -45,7 +45,7 @@ class TimelineEncryptedHistoryPostProcessorTest { fun `given an encrypted room, and key backup enabled, nothing is done`() = runTest { val processor = createPostProcessor(isKeyBackupEnabled = true) val items = listOf( - MatrixTimelineItem.Event(0L, anEventTimelineItem()) + MatrixTimelineItem.Event("0L", anEventTimelineItem()) ) assertThat(processor.process(items)).isSameInstanceAs(items) } @@ -54,7 +54,7 @@ class TimelineEncryptedHistoryPostProcessorTest { fun `given a null lastLoginTimestamp, nothing is done`() = runTest { val processor = createPostProcessor(lastLoginTimestamp = null) val items = listOf( - MatrixTimelineItem.Event(0L, anEventTimelineItem()) + MatrixTimelineItem.Event("0L", anEventTimelineItem()) ) assertThat(processor.process(items)).isSameInstanceAs(items) } @@ -70,7 +70,7 @@ class TimelineEncryptedHistoryPostProcessorTest { fun `given a list with no items before lastLoginTimestamp, nothing is done`() = runTest { val processor = createPostProcessor() val items = listOf( - MatrixTimelineItem.Event(0L, anEventTimelineItem(timestamp = defaultLastLoginTimestamp.time + 1)) + MatrixTimelineItem.Event("0L", anEventTimelineItem(timestamp = defaultLastLoginTimestamp.time + 1)) ) assertThat(processor.process(items)).isSameInstanceAs(items) } @@ -79,20 +79,20 @@ class TimelineEncryptedHistoryPostProcessorTest { fun `given a list with an item with equal timestamp as lastLoginTimestamp, it's replaced`() = runTest { val processor = createPostProcessor() val items = listOf( - MatrixTimelineItem.Event(0L, anEventTimelineItem(timestamp = defaultLastLoginTimestamp.time)) + MatrixTimelineItem.Event("0L", anEventTimelineItem(timestamp = defaultLastLoginTimestamp.time)) ) assertThat(processor.process(items)) - .isEqualTo(listOf(MatrixTimelineItem.Virtual(0L, VirtualTimelineItem.EncryptedHistoryBanner))) + .isEqualTo(listOf(MatrixTimelineItem.Virtual(VirtualTimelineItem.EncryptedHistoryBanner.toString(), VirtualTimelineItem.EncryptedHistoryBanner))) } @Test fun `given a list with an item with a lower timestamp than lastLoginTimestamp, it's replaced`() = runTest { val processor = createPostProcessor() val items = listOf( - MatrixTimelineItem.Event(0L, anEventTimelineItem(timestamp = defaultLastLoginTimestamp.time - 1)) + MatrixTimelineItem.Event("0", anEventTimelineItem(timestamp = defaultLastLoginTimestamp.time - 1)) ) assertThat(processor.process(items)).isEqualTo( - listOf(MatrixTimelineItem.Virtual(0L, VirtualTimelineItem.EncryptedHistoryBanner)) + listOf(MatrixTimelineItem.Virtual(VirtualTimelineItem.EncryptedHistoryBanner.toString(), VirtualTimelineItem.EncryptedHistoryBanner)) ) } @@ -107,14 +107,14 @@ class TimelineEncryptedHistoryPostProcessorTest { ) val processor = createPostProcessor(paginationStateFlow = paginationStateFlow) val items = listOf( - MatrixTimelineItem.Event(0L, anEventTimelineItem(timestamp = defaultLastLoginTimestamp.time - 1)), - MatrixTimelineItem.Event(0L, anEventTimelineItem(timestamp = defaultLastLoginTimestamp.time)), - MatrixTimelineItem.Event(0L, anEventTimelineItem(timestamp = defaultLastLoginTimestamp.time + 1)), + MatrixTimelineItem.Event("0L", anEventTimelineItem(timestamp = defaultLastLoginTimestamp.time - 1)), + MatrixTimelineItem.Event("0L", anEventTimelineItem(timestamp = defaultLastLoginTimestamp.time)), + MatrixTimelineItem.Event("0L", anEventTimelineItem(timestamp = defaultLastLoginTimestamp.time + 1)), ) assertThat(processor.process(items)).isEqualTo( listOf( - MatrixTimelineItem.Virtual(0L, VirtualTimelineItem.EncryptedHistoryBanner), - MatrixTimelineItem.Event(0L, anEventTimelineItem(timestamp = defaultLastLoginTimestamp.time + 1)) + MatrixTimelineItem.Virtual(VirtualTimelineItem.EncryptedHistoryBanner.toString(), VirtualTimelineItem.EncryptedHistoryBanner), + MatrixTimelineItem.Event("0L", anEventTimelineItem(timestamp = defaultLastLoginTimestamp.time + 1)) ) ) assertThat(paginationStateFlow.value).isEqualTo( From d837aa060cd82e091a27fd36114d06ceeee7bd79 Mon Sep 17 00:00:00 2001 From: ganfra Date: Fri, 15 Dec 2023 15:02:46 +0100 Subject: [PATCH 2/3] Timeline : makes sure pagination state is computed correctly and only in one place. --- .../impl/timeline/RustMatrixTimeline.kt | 82 ++++++++++--------- .../TimelineEncryptedHistoryPostProcessor.kt | 19 +---- ...melineEncryptedHistoryPostProcessorTest.kt | 29 +------ 3 files changed, 46 insertions(+), 84 deletions(-) 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 a7c0bef7c3..53ec9efccc 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 @@ -38,7 +38,7 @@ import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.asStateFlow -import kotlinx.coroutines.flow.getAndUpdate +import kotlinx.coroutines.flow.combine import kotlinx.coroutines.flow.launchIn import kotlinx.coroutines.flow.mapLatest import kotlinx.coroutines.flow.onEach @@ -80,7 +80,6 @@ class RustMatrixTimeline( lastLoginTimestamp = lastLoginTimestamp, isRoomEncrypted = matrixRoom.isEncrypted, isKeyBackupEnabled = isKeyBackupEnabled, - paginationStateFlow = _paginationState, dispatcher = dispatcher, ) @@ -115,9 +114,9 @@ class RustMatrixTimeline( postDiffs(diffs) }.launchIn(this) - innerTimeline.backPaginationStatusFlow() + paginationStateFlow() .onEach { - postPaginationStatus(it) + _paginationState.value = it } .launchIn(this) @@ -125,6 +124,44 @@ class RustMatrixTimeline( } } + private fun paginationStateFlow(): Flow { + return combine( + innerTimeline.backPaginationStatusFlow(), + timelineItems, + ) { paginationStatus, filteredItems -> + if (filteredItems.hasEncryptionHistoryBanner()) { + return@combine MatrixTimeline.PaginationState( + isBackPaginating = false, + hasMoreToLoadBackwards = false, + beginningOfRoomReached = false, + ) + } + when (paginationStatus) { + BackPaginationStatus.IDLE -> { + MatrixTimeline.PaginationState( + isBackPaginating = false, + hasMoreToLoadBackwards = true, + beginningOfRoomReached = false, + ) + } + BackPaginationStatus.PAGINATING -> { + MatrixTimeline.PaginationState( + isBackPaginating = true, + hasMoreToLoadBackwards = true, + beginningOfRoomReached = false, + ) + } + BackPaginationStatus.TIMELINE_START_REACHED -> { + MatrixTimeline.PaginationState( + isBackPaginating = false, + hasMoreToLoadBackwards = false, + beginningOfRoomReached = true, + ) + } + } + } + } + private suspend fun fetchMembers() = withContext(dispatcher) { initLatch.await() try { @@ -154,39 +191,6 @@ class RustMatrixTimeline( timelineDiffProcessor.postDiffs(diffs) } - private fun postPaginationStatus(status: BackPaginationStatus) { - _paginationState.getAndUpdate { currentPaginationState -> - if (hasEncryptionHistoryBanner()) { - return@getAndUpdate currentPaginationState.copy( - isBackPaginating = false, - hasMoreToLoadBackwards = false, - beginningOfRoomReached = false, - ) - } - when (status) { - BackPaginationStatus.IDLE -> { - currentPaginationState.copy( - isBackPaginating = false, - hasMoreToLoadBackwards = true - ) - } - BackPaginationStatus.PAGINATING -> { - currentPaginationState.copy( - isBackPaginating = true, - hasMoreToLoadBackwards = true - ) - } - BackPaginationStatus.TIMELINE_START_REACHED -> { - currentPaginationState.copy( - isBackPaginating = false, - hasMoreToLoadBackwards = false, - beginningOfRoomReached = true, - ) - } - } - } - } - override suspend fun fetchDetailsForEvent(eventId: EventId): Result = withContext(dispatcher) { runCatching { innerTimeline.fetchDetailsForEvent(eventId.value) @@ -251,8 +255,8 @@ class RustMatrixTimeline( return _timelineItems.value.firstOrNull { (it as? MatrixTimelineItem.Event)?.eventId == eventId } as? MatrixTimelineItem.Event } - private fun hasEncryptionHistoryBanner(): Boolean { - val firstItem = _timelineItems.value.firstOrNull() + private fun List.hasEncryptionHistoryBanner(): Boolean { + val firstItem = firstOrNull() return firstItem is MatrixTimelineItem.Virtual && firstItem.virtual is VirtualTimelineItem.EncryptedHistoryBanner } diff --git a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/timeline/postprocessor/TimelineEncryptedHistoryPostProcessor.kt b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/timeline/postprocessor/TimelineEncryptedHistoryPostProcessor.kt index 1a6b27203e..9c0fbfd115 100644 --- a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/timeline/postprocessor/TimelineEncryptedHistoryPostProcessor.kt +++ b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/timeline/postprocessor/TimelineEncryptedHistoryPostProcessor.kt @@ -16,12 +16,9 @@ package io.element.android.libraries.matrix.impl.timeline.postprocessor -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.item.virtual.VirtualTimelineItem import kotlinx.coroutines.CoroutineDispatcher -import kotlinx.coroutines.flow.MutableStateFlow -import kotlinx.coroutines.flow.getAndUpdate import kotlinx.coroutines.withContext import timber.log.Timber import java.util.Date @@ -31,26 +28,12 @@ class TimelineEncryptedHistoryPostProcessor( private val lastLoginTimestamp: Date?, private val isRoomEncrypted: Boolean, private val isKeyBackupEnabled: Boolean, - private val paginationStateFlow: MutableStateFlow, ) { suspend fun process(items: List): List = withContext(dispatcher) { Timber.d("Process on Thread=${Thread.currentThread()}") if (!isRoomEncrypted || isKeyBackupEnabled || lastLoginTimestamp == null) return@withContext items - - val filteredItems = replaceWithEncryptionHistoryBannerIfNeeded(items) - // Disable back pagination - val wasFiltered = filteredItems !== items - if (wasFiltered) { - paginationStateFlow.getAndUpdate { - it.copy( - isBackPaginating = false, - hasMoreToLoadBackwards = false, - beginningOfRoomReached = false, - ) - } - } - filteredItems + replaceWithEncryptionHistoryBannerIfNeeded(items) } private fun replaceWithEncryptionHistoryBannerIfNeeded(list: List): List { diff --git a/libraries/matrix/impl/src/test/kotlin/io/element/android/libraries/matrix/impl/timeline/postprocessor/TimelineEncryptedHistoryPostProcessorTest.kt b/libraries/matrix/impl/src/test/kotlin/io/element/android/libraries/matrix/impl/timeline/postprocessor/TimelineEncryptedHistoryPostProcessorTest.kt index ccfddba40a..29c8eca051 100644 --- a/libraries/matrix/impl/src/test/kotlin/io/element/android/libraries/matrix/impl/timeline/postprocessor/TimelineEncryptedHistoryPostProcessorTest.kt +++ b/libraries/matrix/impl/src/test/kotlin/io/element/android/libraries/matrix/impl/timeline/postprocessor/TimelineEncryptedHistoryPostProcessorTest.kt @@ -17,11 +17,9 @@ package io.element.android.libraries.matrix.impl.timeline.postprocessor import com.google.common.truth.Truth.assertThat -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.item.virtual.VirtualTimelineItem import io.element.android.libraries.matrix.test.timeline.anEventTimelineItem -import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.test.StandardTestDispatcher import kotlinx.coroutines.test.TestScope import kotlinx.coroutines.test.runTest @@ -97,15 +95,8 @@ class TimelineEncryptedHistoryPostProcessorTest { } @Test - fun `given a list with several with lower or equal timestamps than lastLoginTimestamp, they're replaced and the user can't back paginate`() = runTest { - val paginationStateFlow = MutableStateFlow( - MatrixTimeline.PaginationState( - hasMoreToLoadBackwards = true, - isBackPaginating = false, - beginningOfRoomReached = false, - ) - ) - val processor = createPostProcessor(paginationStateFlow = paginationStateFlow) + fun `given a list with several with lower or equal timestamps than lastLoginTimestamp, then they're replaced`() = runTest { + val processor = createPostProcessor() val items = listOf( MatrixTimelineItem.Event("0L", anEventTimelineItem(timestamp = defaultLastLoginTimestamp.time - 1)), MatrixTimelineItem.Event("0L", anEventTimelineItem(timestamp = defaultLastLoginTimestamp.time)), @@ -117,32 +108,16 @@ class TimelineEncryptedHistoryPostProcessorTest { MatrixTimelineItem.Event("0L", anEventTimelineItem(timestamp = defaultLastLoginTimestamp.time + 1)) ) ) - assertThat(paginationStateFlow.value).isEqualTo( - MatrixTimeline.PaginationState( - hasMoreToLoadBackwards = false, - isBackPaginating = false, - beginningOfRoomReached = false, - ) - ) } private fun TestScope.createPostProcessor( lastLoginTimestamp: Date? = defaultLastLoginTimestamp, isRoomEncrypted: Boolean = true, isKeyBackupEnabled: Boolean = false, - paginationStateFlow: MutableStateFlow = - MutableStateFlow( - MatrixTimeline.PaginationState( - hasMoreToLoadBackwards = true, - isBackPaginating = false, - beginningOfRoomReached = false, - ) - ) ) = TimelineEncryptedHistoryPostProcessor( lastLoginTimestamp = lastLoginTimestamp, isRoomEncrypted = isRoomEncrypted, isKeyBackupEnabled = isKeyBackupEnabled, - paginationStateFlow = paginationStateFlow, dispatcher = StandardTestDispatcher(testScheduler) ) } From a47e6546017dcc3a3aaf1dbac81eba7f9d049fb3 Mon Sep 17 00:00:00 2001 From: ganfra Date: Mon, 18 Dec 2023 18:19:32 +0100 Subject: [PATCH 3/3] Tests: replace "0" by FAKE_UNIQUE_ID when its ok. --- .../impl/timeline/TimelinePresenterTest.kt | 10 +++++---- .../impl/timeline/RustMatrixTimeline.kt | 10 ++++----- ...melineEncryptedHistoryPostProcessorTest.kt | 22 ++++++++++--------- 3 files changed, 23 insertions(+), 19 deletions(-) diff --git a/features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/timeline/TimelinePresenterTest.kt b/features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/timeline/TimelinePresenterTest.kt index e85e9fa2d7..90609b2230 100644 --- a/features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/timeline/TimelinePresenterTest.kt +++ b/features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/timeline/TimelinePresenterTest.kt @@ -60,6 +60,8 @@ import org.junit.Rule import org.junit.Test import java.util.Date +private const val FAKE_UNIQUE_ID = "FAKE_UNIQUE_ID" + class TimelinePresenterTest { @get:Rule @@ -120,7 +122,7 @@ class TimelinePresenterTest { fun `present - on scroll finished send read receipt if an event is before the index`() = runTest { val timeline = FakeMatrixTimeline( initialTimelineItems = listOf( - MatrixTimelineItem.Event("0", anEventTimelineItem()) + MatrixTimelineItem.Event(FAKE_UNIQUE_ID, anEventTimelineItem()) ) ) val presenter = createTimelinePresenter(timeline) @@ -144,7 +146,7 @@ class TimelinePresenterTest { fun `present - on scroll finished will not send read receipt if no event is before the index`() = runTest { val timeline = FakeMatrixTimeline( initialTimelineItems = listOf( - MatrixTimelineItem.Event("0", anEventTimelineItem()) + MatrixTimelineItem.Event(FAKE_UNIQUE_ID, anEventTimelineItem()) ) ) val presenter = createTimelinePresenter(timeline) @@ -168,7 +170,7 @@ class TimelinePresenterTest { fun `present - on scroll finished will not send read receipt only virtual events exist before the index`() = runTest { val timeline = FakeMatrixTimeline( initialTimelineItems = listOf( - MatrixTimelineItem.Virtual("0", VirtualTimelineItem.ReadMarker) + MatrixTimelineItem.Virtual(FAKE_UNIQUE_ID, VirtualTimelineItem.ReadMarker) ) ) val presenter = createTimelinePresenter(timeline) @@ -267,7 +269,7 @@ class TimelinePresenterTest { ), ) timeline.updateTimelineItems { - listOf(MatrixTimelineItem.Event("0", anEventTimelineItem(reactions = oneReaction))) + listOf(MatrixTimelineItem.Event(FAKE_UNIQUE_ID, anEventTimelineItem(reactions = oneReaction))) } skipItems(1) val item = awaitItem().timelineItems.first() 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 53ec9efccc..547ccf1377 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 @@ -101,6 +101,11 @@ class RustMatrixTimeline( override val paginationState: StateFlow = _paginationState.asStateFlow() + @OptIn(ExperimentalCoroutinesApi::class) + override val timelineItems: Flow> = _timelineItems.mapLatest { items -> + encryptedHistoryPostProcessor.process(items) + } + init { Timber.d("Initialize timeline for room ${matrixRoom.roomId}") @@ -171,11 +176,6 @@ class RustMatrixTimeline( } } - @OptIn(ExperimentalCoroutinesApi::class) - override val timelineItems: Flow> = _timelineItems.mapLatest { items -> - encryptedHistoryPostProcessor.process(items) - } - private suspend fun postItems(items: List) = coroutineScope { // 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 { diff --git a/libraries/matrix/impl/src/test/kotlin/io/element/android/libraries/matrix/impl/timeline/postprocessor/TimelineEncryptedHistoryPostProcessorTest.kt b/libraries/matrix/impl/src/test/kotlin/io/element/android/libraries/matrix/impl/timeline/postprocessor/TimelineEncryptedHistoryPostProcessorTest.kt index 29c8eca051..8d36c45bb6 100644 --- a/libraries/matrix/impl/src/test/kotlin/io/element/android/libraries/matrix/impl/timeline/postprocessor/TimelineEncryptedHistoryPostProcessorTest.kt +++ b/libraries/matrix/impl/src/test/kotlin/io/element/android/libraries/matrix/impl/timeline/postprocessor/TimelineEncryptedHistoryPostProcessorTest.kt @@ -26,6 +26,8 @@ import kotlinx.coroutines.test.runTest import org.junit.Test import java.util.Date +private const val FAKE_UNIQUE_ID = "FAKE_UNIQUE_ID" + class TimelineEncryptedHistoryPostProcessorTest { private val defaultLastLoginTimestamp = Date(1_689_061_264L) @@ -34,7 +36,7 @@ class TimelineEncryptedHistoryPostProcessorTest { fun `given an unencrypted room, nothing is done`() = runTest { val processor = createPostProcessor(isRoomEncrypted = false) val items = listOf( - MatrixTimelineItem.Event("0L", anEventTimelineItem()) + MatrixTimelineItem.Event(FAKE_UNIQUE_ID, anEventTimelineItem()) ) assertThat(processor.process(items)).isSameInstanceAs(items) } @@ -43,7 +45,7 @@ class TimelineEncryptedHistoryPostProcessorTest { fun `given an encrypted room, and key backup enabled, nothing is done`() = runTest { val processor = createPostProcessor(isKeyBackupEnabled = true) val items = listOf( - MatrixTimelineItem.Event("0L", anEventTimelineItem()) + MatrixTimelineItem.Event(FAKE_UNIQUE_ID, anEventTimelineItem()) ) assertThat(processor.process(items)).isSameInstanceAs(items) } @@ -52,7 +54,7 @@ class TimelineEncryptedHistoryPostProcessorTest { fun `given a null lastLoginTimestamp, nothing is done`() = runTest { val processor = createPostProcessor(lastLoginTimestamp = null) val items = listOf( - MatrixTimelineItem.Event("0L", anEventTimelineItem()) + MatrixTimelineItem.Event(FAKE_UNIQUE_ID, anEventTimelineItem()) ) assertThat(processor.process(items)).isSameInstanceAs(items) } @@ -68,7 +70,7 @@ class TimelineEncryptedHistoryPostProcessorTest { fun `given a list with no items before lastLoginTimestamp, nothing is done`() = runTest { val processor = createPostProcessor() val items = listOf( - MatrixTimelineItem.Event("0L", anEventTimelineItem(timestamp = defaultLastLoginTimestamp.time + 1)) + MatrixTimelineItem.Event(FAKE_UNIQUE_ID, anEventTimelineItem(timestamp = defaultLastLoginTimestamp.time + 1)) ) assertThat(processor.process(items)).isSameInstanceAs(items) } @@ -77,7 +79,7 @@ class TimelineEncryptedHistoryPostProcessorTest { fun `given a list with an item with equal timestamp as lastLoginTimestamp, it's replaced`() = runTest { val processor = createPostProcessor() val items = listOf( - MatrixTimelineItem.Event("0L", anEventTimelineItem(timestamp = defaultLastLoginTimestamp.time)) + MatrixTimelineItem.Event(FAKE_UNIQUE_ID, anEventTimelineItem(timestamp = defaultLastLoginTimestamp.time)) ) assertThat(processor.process(items)) .isEqualTo(listOf(MatrixTimelineItem.Virtual(VirtualTimelineItem.EncryptedHistoryBanner.toString(), VirtualTimelineItem.EncryptedHistoryBanner))) @@ -87,7 +89,7 @@ class TimelineEncryptedHistoryPostProcessorTest { fun `given a list with an item with a lower timestamp than lastLoginTimestamp, it's replaced`() = runTest { val processor = createPostProcessor() val items = listOf( - MatrixTimelineItem.Event("0", anEventTimelineItem(timestamp = defaultLastLoginTimestamp.time - 1)) + MatrixTimelineItem.Event(FAKE_UNIQUE_ID, anEventTimelineItem(timestamp = defaultLastLoginTimestamp.time - 1)) ) assertThat(processor.process(items)).isEqualTo( listOf(MatrixTimelineItem.Virtual(VirtualTimelineItem.EncryptedHistoryBanner.toString(), VirtualTimelineItem.EncryptedHistoryBanner)) @@ -98,14 +100,14 @@ class TimelineEncryptedHistoryPostProcessorTest { fun `given a list with several with lower or equal timestamps than lastLoginTimestamp, then they're replaced`() = runTest { val processor = createPostProcessor() val items = listOf( - MatrixTimelineItem.Event("0L", anEventTimelineItem(timestamp = defaultLastLoginTimestamp.time - 1)), - MatrixTimelineItem.Event("0L", anEventTimelineItem(timestamp = defaultLastLoginTimestamp.time)), - MatrixTimelineItem.Event("0L", anEventTimelineItem(timestamp = defaultLastLoginTimestamp.time + 1)), + MatrixTimelineItem.Event(FAKE_UNIQUE_ID, anEventTimelineItem(timestamp = defaultLastLoginTimestamp.time - 1)), + MatrixTimelineItem.Event(FAKE_UNIQUE_ID, anEventTimelineItem(timestamp = defaultLastLoginTimestamp.time)), + MatrixTimelineItem.Event(FAKE_UNIQUE_ID, anEventTimelineItem(timestamp = defaultLastLoginTimestamp.time + 1)), ) assertThat(processor.process(items)).isEqualTo( listOf( MatrixTimelineItem.Virtual(VirtualTimelineItem.EncryptedHistoryBanner.toString(), VirtualTimelineItem.EncryptedHistoryBanner), - MatrixTimelineItem.Event("0L", anEventTimelineItem(timestamp = defaultLastLoginTimestamp.time + 1)) + MatrixTimelineItem.Event(FAKE_UNIQUE_ID, anEventTimelineItem(timestamp = defaultLastLoginTimestamp.time + 1)) ) ) }