Merge pull request #3449 from element-hq/feature/bma/dmRoomBeginning

Fix events blinking at the beginning of DM

Author: bmarty

gradle/libs.versions.toml

@@ -162,7 +162,7 @@ jsoup = "org.jsoup:jsoup:1.18.1"
 appyx_core = { module = "com.bumble.appyx:core", version.ref = "appyx" }
 molecule-runtime = "app.cash.molecule:molecule-runtime:2.0.0"
 timber = "com.jakewharton.timber:timber:5.0.1"
-matrix_sdk = "org.matrix.rustcomponents:sdk-android:0.2.43"
+matrix_sdk = "org.matrix.rustcomponents:sdk-android:0.2.44"
 matrix_richtexteditor = { module = "io.element.android:wysiwyg", version.ref = "wysiwyg" }
 matrix_richtexteditor_compose = { module = "io.element.android:wysiwyg-compose", version.ref = "wysiwyg" }
 sqldelight-driver-android = { module = "app.cash.sqldelight:android-driver", version.ref = "sqldelight" }

libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/room/MatrixRoomInfo.kt

@@ -44,5 +44,6 @@ data class MatrixRoomInfo(
     val hasRoomCall: Boolean,
     val activeRoomCallParticipants: ImmutableList<String>,
     val heroes: ImmutableList<MatrixUser>,
-    val pinnedEventIds: ImmutableList<EventId>
+    val pinnedEventIds: ImmutableList<EventId>,
+    val creator: UserId?,
 )

libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/room/MatrixRoomInfoMapper.kt

@@ -28,6 +28,7 @@ class MatrixRoomInfoMapper {
     fun map(rustRoomInfo: RustRoomInfo): MatrixRoomInfo = rustRoomInfo.let {
         return MatrixRoomInfo(
             id = RoomId(it.id),
+            creator = it.creator?.let(::UserId),
             name = it.displayName,
             rawName = it.rawName,
             topic = it.topic,

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

@@ -86,7 +86,7 @@ class RustTimeline(
     onNewSyncedEvent: () -> Unit,
 ) : Timeline {
     private val initLatch = CompletableDeferred<Unit>()
-    private val isInit = MutableStateFlow(false)
+    private val isTimelineInitialized = MutableStateFlow(false)
 
     private val _timelineItems: MutableStateFlow<List<MatrixTimelineItem>> =
         MutableStateFlow(emptyList())
@@ -110,7 +110,7 @@ class RustTimeline(
         timelineCoroutineScope = coroutineScope,
         timelineDiffProcessor = timelineDiffProcessor,
         initLatch = initLatch,
-        isInit = isInit,
+        isTimelineInitialized = isTimelineInitialized,
         dispatcher = dispatcher,
         onNewSyncedEvent = onNewSyncedEvent,
     )
@@ -189,7 +189,7 @@ class RustTimeline(
     }
 
     private fun canPaginate(direction: Timeline.PaginationDirection): Boolean {
-        if (!isInit.value) return false
+        if (!isTimelineInitialized.value) return false
         return when (direction) {
             Timeline.PaginationDirection.BACKWARDS -> backPaginationStatus.value.canPaginate
             Timeline.PaginationDirection.FORWARDS -> forwardPaginationStatus.value.canPaginate
@@ -207,23 +207,37 @@ class RustTimeline(
         _timelineItems,
         backPaginationStatus.map { it.hasMoreToLoad }.distinctUntilChanged(),
         forwardPaginationStatus.map { it.hasMoreToLoad }.distinctUntilChanged(),
-        isInit,
-    ) { timelineItems, hasMoreToLoadBackward, hasMoreToLoadForward, isInit ->
+        matrixRoom.roomInfoFlow.map { it.creator },
+        isTimelineInitialized,
+    ) { timelineItems,
+        hasMoreToLoadBackward,
+        hasMoreToLoadForward,
+        roomCreator,
+        isTimelineInitialized ->
         withContext(dispatcher) {
             timelineItems
-                .process { items ->
+                .let { items ->
                     roomBeginningPostProcessor.process(
                         items = items,
                         isDm = matrixRoom.isDm,
-                        hasMoreToLoadBackwards = hasMoreToLoadBackward
+                        roomCreator = roomCreator,
+                        hasMoreToLoadBackwards = hasMoreToLoadBackward,
                     )
                 }
-                .process(predicate = isInit) { items ->
-                    loadingIndicatorsPostProcessor.process(items, hasMoreToLoadBackward, hasMoreToLoadForward)
+                .let { items ->
+                    loadingIndicatorsPostProcessor.process(
+                        items = items,
+                        isTimelineInitialized = isTimelineInitialized,
+                        hasMoreToLoadBackward = hasMoreToLoadBackward,
+                        hasMoreToLoadForward = hasMoreToLoadForward
+                    )
                 }
                 // Keep lastForwardIndicatorsPostProcessor last
-                .process(predicate = isInit) { items ->
-                    lastForwardIndicatorsPostProcessor.process(items)
+                .let { items ->
+                    lastForwardIndicatorsPostProcessor.process(
+                        items = items,
+                        isTimelineInitialized = isTimelineInitialized,
+                    )
                 }
         }
     }.onStart {
@@ -542,14 +556,3 @@ class RustTimeline(
         }
     }
 }
-
-private suspend fun List<MatrixTimelineItem>.process(
-    predicate: Boolean = true,
-    processor: suspend (List<MatrixTimelineItem>) -> List<MatrixTimelineItem>
-): List<MatrixTimelineItem> {
-    return if (predicate) {
-        processor(this)
-    } else {
-        this
-    }
-}

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

@@ -38,7 +38,7 @@ internal class TimelineItemsSubscriber(
     private val timeline: Timeline,
     private val timelineDiffProcessor: MatrixTimelineDiffProcessor,
     private val initLatch: CompletableDeferred<Unit>,
-    private val isInit: MutableStateFlow<Boolean>,
+    private val isTimelineInitialized: MutableStateFlow<Boolean>,
     private val onNewSyncedEvent: () -> Unit,
 ) {
     private var subscriptionCount = 0
@@ -85,13 +85,13 @@ internal class TimelineItemsSubscriber(
             ensureActive()
             timelineDiffProcessor.postItems(it)
         }
-        isInit.value = true
+        isTimelineInitialized.value = true
         initLatch.complete(Unit)
     }
 
     private suspend fun postDiffs(diffs: List<TimelineDiff>) {
         val diffsToProcess = diffs.toMutableList()
-        if (!isInit.value) {
+        if (!isTimelineInitialized.value) {
             val resetDiff = diffsToProcess.firstOrNull { it.change() == TimelineChange.RESET }
             if (resetDiff != null) {
                 // Keep using the postItems logic so we can post the timelineItems asap.

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

@@ -22,7 +22,9 @@ class LastForwardIndicatorsPostProcessor(
 
     fun process(
         items: List<MatrixTimelineItem>,
+        isTimelineInitialized: Boolean,
     ): List<MatrixTimelineItem> {
+        if (!isTimelineInitialized) return items
         // We don't need to add the last forward indicator if we are not in the FOCUSED_ON_EVENT mode
         if (mode != Timeline.Mode.FOCUSED_ON_EVENT) {
             return items

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

@@ -16,9 +16,11 @@ import io.element.android.services.toolbox.api.systemclock.SystemClock
 class LoadingIndicatorsPostProcessor(private val systemClock: SystemClock) {
     fun process(
         items: List<MatrixTimelineItem>,
+        isTimelineInitialized: Boolean,
         hasMoreToLoadBackward: Boolean,
         hasMoreToLoadForward: Boolean,
     ): List<MatrixTimelineItem> {
+        if (!isTimelineInitialized) return items
         val shouldAddForwardLoadingIndicator = hasMoreToLoadForward && items.isNotEmpty()
         val currentTimestamp = systemClock.epochMillis()
         return buildList {

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

@@ -9,6 +9,7 @@ package io.element.android.libraries.matrix.impl.timeline.postprocessor
 
 import androidx.annotation.VisibleForTesting
 import io.element.android.libraries.matrix.api.core.UniqueId
+import io.element.android.libraries.matrix.api.core.UserId
 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.event.MembershipChange
@@ -25,12 +26,14 @@ class RoomBeginningPostProcessor(private val mode: Timeline.Mode) {
     fun process(
         items: List<MatrixTimelineItem>,
         isDm: Boolean,
-        hasMoreToLoadBackwards: Boolean
+        roomCreator: UserId?,
+        hasMoreToLoadBackwards: Boolean,
     ): List<MatrixTimelineItem> {
         return when {
+            items.isEmpty() -> items
             mode == Timeline.Mode.PINNED_EVENTS -> items
+            isDm -> processForDM(items, roomCreator)
             hasMoreToLoadBackwards -> items
-            isDm -> processForDM(items)
             else -> processForRoom(items)
         }
     }
@@ -40,22 +43,28 @@ class RoomBeginningPostProcessor(private val mode: Timeline.Mode) {
         return listOf(roomBeginningItem) + items
     }
 
-    private fun processForDM(items: List<MatrixTimelineItem>): List<MatrixTimelineItem> {
-        // Find room creation event. This is usually index 0
+    private fun processForDM(items: List<MatrixTimelineItem>, roomCreator: UserId?): List<MatrixTimelineItem> {
+        // Find room creation event.
+        // This is usually the first MatrixTimelineItem.Event (so index 1, index 0 is a date)
         val roomCreationEventIndex = items.indexOfFirst {
             val stateEventContent = (it as? MatrixTimelineItem.Event)?.event?.content as? StateContent
             stateEventContent?.content is OtherState.RoomCreate
         }
 
-        // Find self-join event for room creator. This is usually index 1
-        val roomCreatorUserId = (items.getOrNull(roomCreationEventIndex) as? MatrixTimelineItem.Event)?.event?.sender
+        // If the parameter roomCreator is null, the creator is the sender of the RoomCreate Event.
+        val roomCreatorUserId = roomCreator ?: (items.getOrNull(roomCreationEventIndex) as? MatrixTimelineItem.Event)?.event?.sender
+        // Find self-join event for the room creator.
+        // This is usually the second MatrixTimelineItem.Event (so index 2)
         val selfUserJoinedEventIndex = roomCreatorUserId?.let { creatorUserId ->
             items.indexOfFirst {
                 val stateEventContent = (it as? MatrixTimelineItem.Event)?.event?.content as? RoomMembershipContent
                 stateEventContent?.change == MembershipChange.JOINED && stateEventContent.userId == creatorUserId
             }
         } ?: -1
 
+        if (roomCreationEventIndex == -1 && selfUserJoinedEventIndex == -1) {
+            return items
+        }
         // Remove items at the indices we found
         val newItems = items.toMutableList()
         if (selfUserJoinedEventIndex in newItems.indices) {

libraries/matrix/impl/src/test/kotlin/io/element/android/libraries/matrix/impl/roomlist/RoomSummaryListProcessorTest.kt

@@ -10,6 +10,7 @@ package io.element.android.libraries.matrix.impl.roomlist
 import com.google.common.truth.Truth.assertThat
 import com.sun.jna.Pointer
 import io.element.android.libraries.matrix.api.core.RoomId
+import io.element.android.libraries.matrix.api.core.UserId
 import io.element.android.libraries.matrix.api.roomlist.RoomSummary
 import io.element.android.libraries.matrix.test.A_ROOM_ID
 import io.element.android.libraries.matrix.test.A_ROOM_ID_2
@@ -215,6 +216,7 @@ private fun aRustRoomInfo(
     numUnreadNotifications: ULong = 0uL,
     numUnreadMentions: ULong = 0uL,
     pinnedEventIds: List<String> = listOf(),
+    roomCreator: UserId? = null,
 ) = RoomInfo(
     id = id,
     displayName = displayName,
@@ -245,6 +247,7 @@ private fun aRustRoomInfo(
     numUnreadNotifications = numUnreadNotifications,
     numUnreadMentions = numUnreadMentions,
     pinnedEventIds = pinnedEventIds,
+    creator = roomCreator?.value,
 )
 
 class FakeRoomListItem(