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

Fix events blinking at the beginning of DM

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,15 +43,18 @@ 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
@@ -56,6 +62,9 @@ class RoomBeginningPostProcessor(private val mode: Timeline.Mode) {
             }
         } ?: -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(

libraries/matrix/impl/src/test/kotlin/io/element/android/libraries/matrix/impl/timeline/postprocessor/RoomBeginningPostProcessorTest.kt

@@ -22,85 +22,174 @@ import io.element.android.libraries.matrix.test.timeline.anEventTimelineItem
 import org.junit.Test
 
 class RoomBeginningPostProcessorTest {
+    private val roomCreateEvent = MatrixTimelineItem.Event(
+        uniqueId = UniqueId("m.room.create"),
+        event = anEventTimelineItem(sender = A_USER_ID, content = StateContent("", OtherState.RoomCreate))
+    )
+    private val roomCreatorJoinEvent = MatrixTimelineItem.Event(
+        uniqueId = UniqueId("m.room.member"),
+        event = anEventTimelineItem(content = RoomMembershipContent(A_USER_ID, null, MembershipChange.JOINED))
+    )
+    private val otherMemberJoinEvent = MatrixTimelineItem.Event(
+        uniqueId = UniqueId("m.room.member_other"),
+        event = anEventTimelineItem(content = RoomMembershipContent(A_USER_ID_2, null, MembershipChange.JOINED))
+    )
+    private val messageEvent = MatrixTimelineItem.Event(
+        uniqueId = UniqueId("m.room.message"),
+        event = anEventTimelineItem(content = aMessageContent("hi"))
+    )
+
+    @Test
+    fun `processor returns empty list when empty list is provided`() {
+        val processor = RoomBeginningPostProcessor(Timeline.Mode.LIVE)
+        val processedItems = processor.process(
+            items = emptyList(),
+            isDm = true,
+            roomCreator = A_USER_ID,
+            hasMoreToLoadBackwards = false,
+        )
+        assertThat(processedItems).isEmpty()
+    }
+
+    @Test
+    fun `processor returns the provided list when it only contains a message`() {
+        val processor = RoomBeginningPostProcessor(Timeline.Mode.LIVE)
+        val processedItems = processor.process(
+            items = listOf(messageEvent),
+            isDm = true,
+            roomCreator = A_USER_ID,
+            hasMoreToLoadBackwards = false,
+        )
+        assertThat(processedItems).isEqualTo(listOf(messageEvent))
+    }
+
+    @Test
+    fun `processor returns the provided list when it only contains a message and the roomCreator is not provided`() {
+        val processor = RoomBeginningPostProcessor(Timeline.Mode.LIVE)
+        val processedItems = processor.process(
+            items = listOf(messageEvent),
+            isDm = true,
+            roomCreator = null,
+            hasMoreToLoadBackwards = false,
+        )
+        assertThat(processedItems).isEqualTo(listOf(messageEvent))
+    }
+
     @Test
     fun `processor removes room creation event and self-join event from DM timeline`() {
         val timelineItems = listOf(
-            MatrixTimelineItem.Event(UniqueId("m.room.create"), anEventTimelineItem(sender = A_USER_ID, content = StateContent("", OtherState.RoomCreate))),
-            MatrixTimelineItem.Event(UniqueId("m.room.member"), anEventTimelineItem(content = RoomMembershipContent(A_USER_ID, null, MembershipChange.JOINED))),
+            roomCreateEvent,
+            roomCreatorJoinEvent,
         )
         val processor = RoomBeginningPostProcessor(Timeline.Mode.LIVE)
-        val processedItems = processor.process(timelineItems, isDm = true, hasMoreToLoadBackwards = false)
+        val processedItems = processor.process(
+            items = timelineItems,
+            isDm = true,
+            roomCreator = A_USER_ID,
+            hasMoreToLoadBackwards = false,
+        )
         assertThat(processedItems).isEmpty()
     }
 
+    @Test
+    fun `processor does not remove anything with PINNED_EVENTS mode`() {
+        val timelineItems = listOf(
+            roomCreateEvent,
+            roomCreatorJoinEvent,
+        )
+        val processor = RoomBeginningPostProcessor(Timeline.Mode.PINNED_EVENTS)
+        val processedItems = processor.process(
+            items = timelineItems,
+            isDm = true,
+            roomCreator = A_USER_ID,
+            hasMoreToLoadBackwards = false,
+        )
+        assertThat(processedItems).isEqualTo(timelineItems)
+    }
+
     @Test
     fun `processor removes room creation event and self-join event from DM timeline even if they're not the first items`() {
         val timelineItems = listOf(
-            MatrixTimelineItem.Event(
-                UniqueId("m.room.member_other"),
-                anEventTimelineItem(content = RoomMembershipContent(A_USER_ID_2, null, MembershipChange.JOINED))
-            ),
-            MatrixTimelineItem.Event(UniqueId("m.room.create"), anEventTimelineItem(sender = A_USER_ID, content = StateContent("", OtherState.RoomCreate))),
-            MatrixTimelineItem.Event(UniqueId("m.room.message"), anEventTimelineItem(content = aMessageContent("hi"))),
-            MatrixTimelineItem.Event(UniqueId("m.room.member"), anEventTimelineItem(content = RoomMembershipContent(A_USER_ID, null, MembershipChange.JOINED))),
+            otherMemberJoinEvent,
+            roomCreateEvent,
+            messageEvent,
+            roomCreatorJoinEvent,
         )
         val expected = listOf(
-            MatrixTimelineItem.Event(
-                UniqueId("m.room.member_other"),
-                anEventTimelineItem(content = RoomMembershipContent(A_USER_ID_2, null, MembershipChange.JOINED))
-            ),
-            MatrixTimelineItem.Event(UniqueId("m.room.message"), anEventTimelineItem(content = aMessageContent("hi"))),
+            otherMemberJoinEvent,
+            messageEvent,
         )
         val processor = RoomBeginningPostProcessor(Timeline.Mode.LIVE)
-        val processedItems = processor.process(timelineItems, isDm = true, hasMoreToLoadBackwards = false)
+        val processedItems = processor.process(timelineItems, isDm = true, roomCreator = A_USER_ID, hasMoreToLoadBackwards = false)
         assertThat(processedItems).isEqualTo(expected)
     }
 
     @Test
     fun `processor will add beginning of room item if it's not a DM`() {
         val timelineItems = listOf(
-            MatrixTimelineItem.Event(UniqueId("m.room.create"), anEventTimelineItem(sender = A_USER_ID, content = StateContent("", OtherState.RoomCreate))),
-            MatrixTimelineItem.Event(UniqueId("m.room.member"), anEventTimelineItem(content = RoomMembershipContent(A_USER_ID, null, MembershipChange.JOINED))),
+            roomCreateEvent,
+            roomCreatorJoinEvent,
         )
         val processor = RoomBeginningPostProcessor(Timeline.Mode.LIVE)
-        val processedItems = processor.process(timelineItems, isDm = false, hasMoreToLoadBackwards = false)
+        val processedItems = processor.process(timelineItems, isDm = false, roomCreator = A_USER_ID, hasMoreToLoadBackwards = false)
         assertThat(processedItems).isEqualTo(
             listOf(processor.createRoomBeginningItem()) + timelineItems
         )
     }
 
     @Test
-    fun `processor won't remove items if it's not at the start of the timeline`() {
+    fun `processor will not add beginning of room item if it's not a DM but the room has more to load`() {
         val timelineItems = listOf(
-            MatrixTimelineItem.Event(UniqueId("m.room.create"), anEventTimelineItem(sender = A_USER_ID, content = StateContent("", OtherState.RoomCreate))),
-            MatrixTimelineItem.Event(UniqueId("m.room.member"), anEventTimelineItem(content = RoomMembershipContent(A_USER_ID, null, MembershipChange.JOINED))),
+            roomCreateEvent,
+            roomCreatorJoinEvent,
         )
         val processor = RoomBeginningPostProcessor(Timeline.Mode.LIVE)
-        val processedItems = processor.process(timelineItems, isDm = true, hasMoreToLoadBackwards = true)
+        val processedItems = processor.process(timelineItems, isDm = false, roomCreator = A_USER_ID, hasMoreToLoadBackwards = true)
         assertThat(processedItems).isEqualTo(timelineItems)
     }
 
     @Test
-    fun `processor won't remove the first member join event if it can't find the room creation event`() {
+    fun `processor will add beginning of room item if it's not a DM, when the parameter roomCreator is null`() {
         val timelineItems = listOf(
-            MatrixTimelineItem.Event(UniqueId("m.room.member"), anEventTimelineItem(content = RoomMembershipContent(A_USER_ID, null, MembershipChange.JOINED))),
+            roomCreateEvent,
+            roomCreatorJoinEvent,
         )
         val processor = RoomBeginningPostProcessor(Timeline.Mode.LIVE)
-        val processedItems = processor.process(timelineItems, isDm = true, hasMoreToLoadBackwards = true)
-        assertThat(processedItems).isEqualTo(timelineItems)
+        val processedItems = processor.process(timelineItems, isDm = false, roomCreator = null, hasMoreToLoadBackwards = false)
+        assertThat(processedItems).isEqualTo(
+            listOf(processor.createRoomBeginningItem()) + timelineItems
+        )
+    }
+
+    @Test
+    fun `processor removes items event it's not at the start of the timeline`() {
+        val timelineItems = listOf(
+            roomCreateEvent,
+            roomCreatorJoinEvent,
+        )
+        val processor = RoomBeginningPostProcessor(Timeline.Mode.LIVE)
+        val processedItems = processor.process(timelineItems, isDm = true, roomCreator = A_USER_ID, hasMoreToLoadBackwards = true)
+        assertThat(processedItems).isEmpty()
+    }
+
+    @Test
+    fun `processor removes the first member join event if it matches the roomCreator parameter`() {
+        val timelineItems = listOf(
+            roomCreatorJoinEvent,
+        )
+        val processor = RoomBeginningPostProcessor(Timeline.Mode.LIVE)
+        val processedItems = processor.process(timelineItems, isDm = true, roomCreator = A_USER_ID, hasMoreToLoadBackwards = true)
+        assertThat(processedItems).isEmpty()
     }
 
     @Test
     fun `processor won't remove the first member join event if it's not from the room creator`() {
         val timelineItems = listOf(
-            MatrixTimelineItem.Event(UniqueId("m.room.create"), anEventTimelineItem(sender = A_USER_ID, content = StateContent("", OtherState.RoomCreate))),
-            MatrixTimelineItem.Event(
-                UniqueId("m.room.member"),
-                anEventTimelineItem(content = RoomMembershipContent(A_USER_ID_2, null, MembershipChange.JOINED))
-            ),
+            roomCreateEvent,
+            otherMemberJoinEvent,
         )
         val processor = RoomBeginningPostProcessor(Timeline.Mode.LIVE)
-        val processedItems = processor.process(timelineItems, isDm = true, hasMoreToLoadBackwards = true)
-        assertThat(processedItems).isEqualTo(timelineItems)
+        val processedItems = processor.process(timelineItems, isDm = true, roomCreator = A_USER_ID, hasMoreToLoadBackwards = true)
+        assertThat(processedItems).isEqualTo(listOf(otherMemberJoinEvent))
     }
 }

libraries/matrix/test/src/main/kotlin/io/element/android/libraries/matrix/test/room/FakeMatrixRoom.kt

@@ -523,6 +523,7 @@ fun aRoomInfo(
     activeRoomCallParticipants: List<String> = emptyList(),
     heroes: List<MatrixUser> = emptyList(),
     pinnedEventIds: List<EventId> = emptyList(),
+    roomCreator: UserId? = null,
 ) = MatrixRoomInfo(
     id = id,
     name = name,
@@ -549,6 +550,7 @@ fun aRoomInfo(
     activeRoomCallParticipants = activeRoomCallParticipants.toImmutableList(),
     heroes = heroes.toImmutableList(),
     pinnedEventIds = pinnedEventIds.toImmutableList(),
+    creator = roomCreator,
 )
 
 fun defaultRoomPowerLevels() = MatrixRoomPowerLevels(