From f87422a0224ef5d86247ff221a586f2e5ec00277 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Wed, 11 Sep 2024 15:50:55 +0200 Subject: [PATCH] Fix filtering of Event at the beginning of DM. --- .../matrix/api/room/MatrixRoomInfo.kt | 3 +- .../matrix/impl/room/MatrixRoomInfoMapper.kt | 1 + .../matrix/impl/timeline/RustTimeline.kt | 6 +- .../RoomBeginningPostProcessor.kt | 21 ++- .../roomlist/RoomSummaryListProcessorTest.kt | 3 + .../RoomBeginningPostProcessorTest.kt | 157 ++++++++++++++---- .../matrix/test/room/FakeMatrixRoom.kt | 2 + 7 files changed, 150 insertions(+), 43 deletions(-) diff --git a/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/room/MatrixRoomInfo.kt b/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/room/MatrixRoomInfo.kt index 32eddf7b00..b466a28da6 100644 --- a/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/room/MatrixRoomInfo.kt +++ b/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, val heroes: ImmutableList, - val pinnedEventIds: ImmutableList + val pinnedEventIds: ImmutableList, + val creator: UserId?, ) diff --git a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/room/MatrixRoomInfoMapper.kt b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/room/MatrixRoomInfoMapper.kt index d5a0b86083..7fcef163b6 100644 --- a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/room/MatrixRoomInfoMapper.kt +++ b/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, 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 25d7c52c7b..bb8947b2ba 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 @@ -207,15 +207,17 @@ class RustTimeline( _timelineItems, backPaginationStatus.map { it.hasMoreToLoad }.distinctUntilChanged(), forwardPaginationStatus.map { it.hasMoreToLoad }.distinctUntilChanged(), + matrixRoom.roomInfoFlow.map { it.creator }, isInit, - ) { timelineItems, hasMoreToLoadBackward, hasMoreToLoadForward, isInit -> + ) { timelineItems, hasMoreToLoadBackward, hasMoreToLoadForward, roomCreator, isInit -> withContext(dispatcher) { timelineItems .process { items -> roomBeginningPostProcessor.process( items = items, isDm = matrixRoom.isDm, - hasMoreToLoadBackwards = hasMoreToLoadBackward + roomCreator = roomCreator, + hasMoreToLoadBackwards = hasMoreToLoadBackward, ) } .process(predicate = isInit) { items -> diff --git a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/timeline/postprocessor/RoomBeginningPostProcessor.kt b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/timeline/postprocessor/RoomBeginningPostProcessor.kt index 2fb41a745c..7ed9de2fe3 100644 --- a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/timeline/postprocessor/RoomBeginningPostProcessor.kt +++ b/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, isDm: Boolean, - hasMoreToLoadBackwards: Boolean + roomCreator: UserId?, + hasMoreToLoadBackwards: Boolean, ): List { 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): List { - // Find room creation event. This is usually index 0 + private fun processForDM(items: List, roomCreator: UserId?): List { + // 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) { diff --git a/libraries/matrix/impl/src/test/kotlin/io/element/android/libraries/matrix/impl/roomlist/RoomSummaryListProcessorTest.kt b/libraries/matrix/impl/src/test/kotlin/io/element/android/libraries/matrix/impl/roomlist/RoomSummaryListProcessorTest.kt index f4b7f8a1d0..faa8a1102e 100644 --- a/libraries/matrix/impl/src/test/kotlin/io/element/android/libraries/matrix/impl/roomlist/RoomSummaryListProcessorTest.kt +++ b/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 = 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( diff --git a/libraries/matrix/impl/src/test/kotlin/io/element/android/libraries/matrix/impl/timeline/postprocessor/RoomBeginningPostProcessorTest.kt b/libraries/matrix/impl/src/test/kotlin/io/element/android/libraries/matrix/impl/timeline/postprocessor/RoomBeginningPostProcessorTest.kt index db93b86142..fadc174cd1 100644 --- a/libraries/matrix/impl/src/test/kotlin/io/element/android/libraries/matrix/impl/timeline/postprocessor/RoomBeginningPostProcessorTest.kt +++ b/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)) } } diff --git a/libraries/matrix/test/src/main/kotlin/io/element/android/libraries/matrix/test/room/FakeMatrixRoom.kt b/libraries/matrix/test/src/main/kotlin/io/element/android/libraries/matrix/test/room/FakeMatrixRoom.kt index 8418d0c047..4f35c37d4d 100644 --- a/libraries/matrix/test/src/main/kotlin/io/element/android/libraries/matrix/test/room/FakeMatrixRoom.kt +++ b/libraries/matrix/test/src/main/kotlin/io/element/android/libraries/matrix/test/room/FakeMatrixRoom.kt @@ -523,6 +523,7 @@ fun aRoomInfo( activeRoomCallParticipants: List = emptyList(), heroes: List = emptyList(), pinnedEventIds: List = 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(