Refactor: replace client-side event filtering for public rooms with SDK (#7025)
* Refactor: replace client-side event filtering for public rooms with an SDK-based one --------- Co-authored-by: Benoit Marty <benoitm@element.io>
This commit is contained in:
committed by
GitHub
parent
576fc3d7f3
commit
19ee0ae0cd
+8
-1
@@ -8,6 +8,7 @@
|
||||
|
||||
package io.element.android.libraries.matrix.impl.room
|
||||
|
||||
import io.element.android.appconfig.TimelineConfig
|
||||
import io.element.android.libraries.core.coroutine.CoroutineDispatchers
|
||||
import io.element.android.libraries.core.coroutine.childScope
|
||||
import io.element.android.libraries.core.extensions.mapFailure
|
||||
@@ -222,7 +223,13 @@ class JoinedRustRoom(
|
||||
)
|
||||
is CreateTimelineParams.Focused,
|
||||
CreateTimelineParams.PinnedOnly,
|
||||
is CreateTimelineParams.Threaded -> TimelineFilter.All
|
||||
is CreateTimelineParams.Threaded -> {
|
||||
RustTimelineEventFilterFactory().create(
|
||||
joinRule = roomInfoFlow.value.joinRule,
|
||||
isEncrypted = roomInfoFlow.value.isEncrypted,
|
||||
excludedStateTypes = TimelineConfig.excludedEvents,
|
||||
)?.let(TimelineFilter::EventFilter) ?: TimelineFilter.All
|
||||
}
|
||||
}
|
||||
|
||||
val internalIdPrefix = when (createTimelineParams) {
|
||||
|
||||
+13
-7
@@ -21,6 +21,7 @@ import io.element.android.libraries.matrix.api.room.JoinedRoom
|
||||
import io.element.android.libraries.matrix.api.room.RoomMembershipObserver
|
||||
import io.element.android.libraries.matrix.api.roomlist.RoomListService
|
||||
import io.element.android.libraries.matrix.api.roomlist.awaitLoaded
|
||||
import io.element.android.libraries.matrix.impl.room.join.map
|
||||
import io.element.android.libraries.matrix.impl.room.preview.RoomPreviewInfoMapper
|
||||
import io.element.android.libraries.matrix.impl.roomlist.roomOrNull
|
||||
import io.element.android.services.analytics.api.AnalyticsLongRunningTransaction
|
||||
@@ -41,6 +42,7 @@ import org.matrix.rustcomponents.sdk.TimelineConfiguration
|
||||
import org.matrix.rustcomponents.sdk.TimelineFilter
|
||||
import org.matrix.rustcomponents.sdk.TimelineFocus
|
||||
import timber.log.Timber
|
||||
import uniffi.matrix_sdk_base.EncryptionState
|
||||
import uniffi.matrix_sdk_ui.TimelineReadReceiptTracking
|
||||
import java.util.concurrent.atomic.AtomicBoolean
|
||||
import org.matrix.rustcomponents.sdk.RoomListService as InnerRoomListService
|
||||
@@ -66,12 +68,6 @@ class RustRoomFactory(
|
||||
private val mutex = Mutex()
|
||||
private val isDestroyed: AtomicBoolean = AtomicBoolean(false)
|
||||
|
||||
private val eventFilters = TimelineConfig.excludedEvents
|
||||
.takeIf { it.isNotEmpty() }
|
||||
?.let { listStateEventType ->
|
||||
timelineEventFilterFactory.create(listStateEventType)
|
||||
}
|
||||
|
||||
suspend fun destroy() {
|
||||
withContext(NonCancellable + dispatcher) {
|
||||
mutex.withLock {
|
||||
@@ -128,10 +124,20 @@ class RustRoomFactory(
|
||||
operation = "sdkRoom.timelineWithConfiguration",
|
||||
description = "Get timeline from the SDK",
|
||||
) {
|
||||
val isEncrypted = when (roomInfo.encryptionState) {
|
||||
EncryptionState.ENCRYPTED -> true
|
||||
EncryptionState.NOT_ENCRYPTED -> false
|
||||
EncryptionState.UNKNOWN -> null
|
||||
}
|
||||
val timelineFilter = timelineEventFilterFactory.create(
|
||||
joinRule = roomInfo.joinRule?.map(),
|
||||
isEncrypted = isEncrypted,
|
||||
excludedStateTypes = TimelineConfig.excludedEvents,
|
||||
)
|
||||
sdkRoom.timelineWithConfiguration(
|
||||
TimelineConfiguration(
|
||||
focus = TimelineFocus.Live(hideThreadedEvents = hideThreadedEvents),
|
||||
filter = eventFilters?.let(TimelineFilter::EventFilter) ?: TimelineFilter.All,
|
||||
filter = timelineFilter?.let(TimelineFilter::EventFilter) ?: TimelineFilter.All,
|
||||
internalIdPrefix = "live",
|
||||
dateDividerMode = DateDividerMode.DAILY,
|
||||
trackReadReceipts = TimelineReadReceiptTracking.ALL_EVENTS,
|
||||
|
||||
+32
-7
@@ -11,20 +11,45 @@ package io.element.android.libraries.matrix.impl.room
|
||||
import dev.zacsweers.metro.AppScope
|
||||
import dev.zacsweers.metro.ContributesBinding
|
||||
import io.element.android.libraries.matrix.api.room.StateEventType
|
||||
import io.element.android.libraries.matrix.api.room.join.JoinRule
|
||||
import org.matrix.rustcomponents.sdk.FilterTimelineEventCondition
|
||||
import org.matrix.rustcomponents.sdk.FilterTimelineEventType
|
||||
import org.matrix.rustcomponents.sdk.TimelineEventFilter
|
||||
import uniffi.matrix_sdk_ui.MembershipChangeFilter
|
||||
|
||||
interface TimelineEventFilterFactory {
|
||||
fun create(listStateEventType: List<StateEventType>): TimelineEventFilter
|
||||
fun create(
|
||||
joinRule: JoinRule?,
|
||||
isEncrypted: Boolean?,
|
||||
excludedStateTypes: List<StateEventType>
|
||||
): TimelineEventFilter?
|
||||
}
|
||||
|
||||
@ContributesBinding(AppScope::class)
|
||||
class RustTimelineEventFilterFactory : TimelineEventFilterFactory {
|
||||
override fun create(listStateEventType: List<StateEventType>): TimelineEventFilter {
|
||||
return TimelineEventFilter.excludeEventTypes(
|
||||
listStateEventType.map { stateEventType ->
|
||||
FilterTimelineEventType.State(stateEventType.map())
|
||||
}
|
||||
)
|
||||
override fun create(
|
||||
joinRule: JoinRule?,
|
||||
isEncrypted: Boolean?,
|
||||
excludedStateTypes: List<StateEventType>
|
||||
): TimelineEventFilter? {
|
||||
val excludedEventTypes = excludedStateTypes.map {
|
||||
FilterTimelineEventCondition.EventType(FilterTimelineEventType.State(it.map()))
|
||||
}
|
||||
// If the room is publicly joinable and not encrypted, we also want to exclude membership changes and profile changes,
|
||||
// as they will pollute the timelines since they're quite common and not add much value.
|
||||
val excludedMembershipChanges = if (joinRule !is JoinRule.Invite && isEncrypted == false) {
|
||||
listOf(
|
||||
FilterTimelineEventCondition.MembershipChange(MembershipChangeFilter.JOIN),
|
||||
FilterTimelineEventCondition.MembershipChange(MembershipChangeFilter.LEAVE),
|
||||
FilterTimelineEventCondition.ProfileChange,
|
||||
)
|
||||
} else {
|
||||
emptyList()
|
||||
}
|
||||
return if (excludedEventTypes.isNotEmpty() || excludedMembershipChanges.isNotEmpty()) {
|
||||
TimelineEventFilter.exclude(excludedEventTypes + excludedMembershipChanges)
|
||||
} else {
|
||||
null
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
-16
@@ -38,8 +38,6 @@ import io.element.android.libraries.matrix.impl.room.location.into
|
||||
import io.element.android.libraries.matrix.impl.timeline.item.event.EventTimelineItemMapper
|
||||
import io.element.android.libraries.matrix.impl.timeline.item.event.TimelineEventContentMapper
|
||||
import io.element.android.libraries.matrix.impl.timeline.item.virtual.VirtualTimelineItemMapper
|
||||
import io.element.android.libraries.matrix.impl.timeline.postprocessor.FilterEmptyDayPostProcessor
|
||||
import io.element.android.libraries.matrix.impl.timeline.postprocessor.FilterPublicMembershipChangesPostProcessor
|
||||
import io.element.android.libraries.matrix.impl.timeline.postprocessor.LastForwardIndicatorsPostProcessor
|
||||
import io.element.android.libraries.matrix.impl.timeline.postprocessor.LoadingIndicatorsPostProcessor
|
||||
import io.element.android.libraries.matrix.impl.timeline.postprocessor.RoomBeginningPostProcessor
|
||||
@@ -125,8 +123,6 @@ class RustTimeline(
|
||||
private val loadingIndicatorsPostProcessor = LoadingIndicatorsPostProcessor(systemClock)
|
||||
private val lastForwardIndicatorsPostProcessor = LastForwardIndicatorsPostProcessor(mode)
|
||||
private val typingNotificationPostProcessor = TypingNotificationPostProcessor(mode)
|
||||
private val publicMembershipChangesPostProcessor = FilterPublicMembershipChangesPostProcessor()
|
||||
private val emptyDayPostProcessor = FilterEmptyDayPostProcessor()
|
||||
|
||||
private data class RoomTimelineInfo(
|
||||
val roomCreators: ImmutableList<UserId>,
|
||||
@@ -252,18 +248,6 @@ class RustTimeline(
|
||||
hasMoreToLoadBackwards = backwardPaginationStatus.hasMoreToLoad,
|
||||
)
|
||||
}
|
||||
// This should be the first post processor after room beginning.
|
||||
.let { items ->
|
||||
publicMembershipChangesPostProcessor.process(
|
||||
items = items,
|
||||
joinRule = joinRule,
|
||||
isEncrypted = isEncrypted,
|
||||
)
|
||||
}
|
||||
// After removing public membership changes, we might end up with empty days, so we need to filter them out.
|
||||
.let { items ->
|
||||
emptyDayPostProcessor.process(items)
|
||||
}
|
||||
.let { items ->
|
||||
loadingIndicatorsPostProcessor.process(
|
||||
items = items,
|
||||
|
||||
-40
@@ -1,40 +0,0 @@
|
||||
/*
|
||||
* Copyright (c) 2026 Element Creations Ltd.
|
||||
*
|
||||
* SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Element-Commercial.
|
||||
* Please see LICENSE files in the repository root for full details.
|
||||
*/
|
||||
|
||||
package io.element.android.libraries.matrix.impl.timeline.postprocessor
|
||||
|
||||
import io.element.android.libraries.matrix.api.timeline.MatrixTimelineItem
|
||||
import io.element.android.libraries.matrix.api.timeline.item.virtual.VirtualTimelineItem
|
||||
|
||||
/**
|
||||
* Post-processor to filter out day separators for days that don't contain any events.
|
||||
*/
|
||||
class FilterEmptyDayPostProcessor {
|
||||
/**
|
||||
* Filters out day separators from [items] for days that don't contain any events.
|
||||
*/
|
||||
fun process(items: List<MatrixTimelineItem>): List<MatrixTimelineItem> = buildList {
|
||||
// The timeline is ordered by ascending timestamp, so events that happened during a day appear after the day separator for that day.
|
||||
// We can use this to determine if a day separator should be kept or not by traversing the list in reverse.
|
||||
var hasEvent = false
|
||||
for (item in items.asReversed()) {
|
||||
if (item is MatrixTimelineItem.Event) {
|
||||
hasEvent = true
|
||||
add(item)
|
||||
} else if (item is MatrixTimelineItem.Virtual && item.virtual is VirtualTimelineItem.DayDivider) {
|
||||
if (hasEvent) {
|
||||
add(item)
|
||||
hasEvent = false
|
||||
}
|
||||
} else {
|
||||
add(item)
|
||||
}
|
||||
}
|
||||
}
|
||||
// Then reverse the result to restore the original order, minus the empty day separators.
|
||||
.asReversed()
|
||||
}
|
||||
-43
@@ -1,43 +0,0 @@
|
||||
/*
|
||||
* Copyright (c) 2026 Element Creations Ltd.
|
||||
*
|
||||
* SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Element-Commercial.
|
||||
* Please see LICENSE files in the repository root for full details.
|
||||
*/
|
||||
|
||||
package io.element.android.libraries.matrix.impl.timeline.postprocessor
|
||||
|
||||
import io.element.android.libraries.matrix.api.room.join.JoinRule
|
||||
import io.element.android.libraries.matrix.api.timeline.MatrixTimelineItem
|
||||
import io.element.android.libraries.matrix.api.timeline.item.event.MembershipChange
|
||||
import io.element.android.libraries.matrix.api.timeline.item.event.ProfileChangeContent
|
||||
import io.element.android.libraries.matrix.api.timeline.item.event.RoomMembershipContent
|
||||
|
||||
/**
|
||||
* Post-processor to filter out public membership changes for non-encrypted, publicly joinable rooms.
|
||||
*/
|
||||
class FilterPublicMembershipChangesPostProcessor {
|
||||
/**
|
||||
* Filters out public membership changes from [items] if the room is publicly joinable and not encrypted.
|
||||
*/
|
||||
fun process(
|
||||
items: List<MatrixTimelineItem>,
|
||||
joinRule: JoinRule?,
|
||||
isEncrypted: Boolean?,
|
||||
): List<MatrixTimelineItem> {
|
||||
return if (joinRule !is JoinRule.Invite && isEncrypted == false) {
|
||||
filterMembershipEvents(items)
|
||||
} else {
|
||||
items
|
||||
}
|
||||
}
|
||||
|
||||
private fun filterMembershipEvents(items: List<MatrixTimelineItem>): List<MatrixTimelineItem> = items.filter { item ->
|
||||
val eventContent = (item as? MatrixTimelineItem.Event)?.event?.content ?: return@filter true
|
||||
when (eventContent) {
|
||||
is RoomMembershipContent -> eventContent.change != null && eventContent.change !in listOf(MembershipChange.JOINED, MembershipChange.LEFT)
|
||||
is ProfileChangeContent -> false
|
||||
else -> true
|
||||
}
|
||||
}
|
||||
}
|
||||
+2
-1
@@ -9,11 +9,12 @@
|
||||
package io.element.android.libraries.matrix.impl.room
|
||||
|
||||
import io.element.android.libraries.matrix.api.room.StateEventType
|
||||
import io.element.android.libraries.matrix.api.room.join.JoinRule
|
||||
import io.element.android.libraries.matrix.impl.fixtures.fakes.FakeFfiTimelineEventFilter
|
||||
import org.matrix.rustcomponents.sdk.TimelineEventFilter
|
||||
|
||||
class FakeTimelineEventFilterFactory : TimelineEventFilterFactory {
|
||||
override fun create(listStateEventType: List<StateEventType>): TimelineEventFilter {
|
||||
override fun create(joinRule: JoinRule?, isEncrypted: Boolean?, excludedStateTypes: List<StateEventType>): TimelineEventFilter {
|
||||
return FakeFfiTimelineEventFilter()
|
||||
}
|
||||
}
|
||||
|
||||
-118
@@ -1,118 +0,0 @@
|
||||
/*
|
||||
* Copyright (c) 2026 Element Creations Ltd.
|
||||
*
|
||||
* SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Element-Commercial.
|
||||
* Please see LICENSE files in the repository root for full details.
|
||||
*/
|
||||
|
||||
package io.element.android.libraries.matrix.impl.timeline.postprocessor
|
||||
|
||||
import com.google.common.truth.Truth.assertThat
|
||||
import io.element.android.libraries.matrix.api.core.UniqueId
|
||||
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 org.junit.Test
|
||||
|
||||
private const val TODAY = 1_779_779_967_000
|
||||
private const val YESTERDAY = TODAY - 24 * 60 * 60 * 1000
|
||||
private const val DAY_BEFORE_YESTERDAY = YESTERDAY - 24 * 60 * 60 * 1000
|
||||
|
||||
class FilterEmptyDayPostProcessorTest {
|
||||
private val anEvent = MatrixTimelineItem.Event(
|
||||
uniqueId = UniqueId("event"),
|
||||
event = anEventTimelineItem(),
|
||||
)
|
||||
|
||||
private fun aDaySeparator(timestmap: Long) = MatrixTimelineItem.Virtual(
|
||||
uniqueId = UniqueId("day_$timestmap"),
|
||||
virtual = VirtualTimelineItem.DayDivider(timestmap)
|
||||
)
|
||||
|
||||
@Test
|
||||
fun `filterEmptyDaySeparators keeps day separator with events after it`() {
|
||||
val items = listOf(
|
||||
aDaySeparator(TODAY),
|
||||
anEvent,
|
||||
)
|
||||
val result = FilterEmptyDayPostProcessor().process(items)
|
||||
assertThat(result).hasSize(2)
|
||||
assertThat(result[0]).isEqualTo(aDaySeparator(TODAY))
|
||||
assertThat(result[1]).isEqualTo(anEvent)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `filterEmptyDaySeparators removes day separator with no events after it`() {
|
||||
val items = listOf(
|
||||
aDaySeparator(YESTERDAY),
|
||||
aDaySeparator(TODAY),
|
||||
)
|
||||
val result = FilterEmptyDayPostProcessor().process(items)
|
||||
assertThat(result).isEmpty()
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `filterEmptyDaySeparators removes second day separator and keeps first when only first has events`() {
|
||||
val items = listOf(
|
||||
aDaySeparator(YESTERDAY),
|
||||
anEvent,
|
||||
aDaySeparator(TODAY),
|
||||
)
|
||||
val result = FilterEmptyDayPostProcessor().process(items)
|
||||
assertThat(result).hasSize(2)
|
||||
assertThat(result[0]).isEqualTo(aDaySeparator(YESTERDAY))
|
||||
assertThat(result[1]).isEqualTo(anEvent)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `filterEmptyDaySeparators handles multiple day separators in a row with no events`() {
|
||||
val items = listOf(
|
||||
aDaySeparator(TODAY),
|
||||
aDaySeparator(YESTERDAY),
|
||||
aDaySeparator(DAY_BEFORE_YESTERDAY),
|
||||
)
|
||||
val result = FilterEmptyDayPostProcessor().process(items)
|
||||
assertThat(result).isEmpty()
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `filterEmptyDaySeparators keeps all items when no day separators`() {
|
||||
val items = listOf(
|
||||
anEvent.copy(uniqueId = UniqueId("event2")),
|
||||
anEvent,
|
||||
)
|
||||
val result = FilterEmptyDayPostProcessor().process(items)
|
||||
assertThat(result).hasSize(2)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `filterEmptyDaySeparators removes day separator preceded by non-event virtual item`() {
|
||||
val readMarker = MatrixTimelineItem.Virtual(
|
||||
uniqueId = UniqueId("readMarker"),
|
||||
virtual = VirtualTimelineItem.ReadMarker
|
||||
)
|
||||
val items = listOf(
|
||||
aDaySeparator(TODAY),
|
||||
readMarker,
|
||||
)
|
||||
val result = FilterEmptyDayPostProcessor().process(items)
|
||||
assertThat(result).hasSize(1)
|
||||
assertThat(result[0]).isEqualTo(readMarker)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `filterEmptyDaySeparators keeps day separator when non-event virtual items are between separator and event`() {
|
||||
val readMarker = MatrixTimelineItem.Virtual(
|
||||
uniqueId = UniqueId("readMarker"),
|
||||
virtual = VirtualTimelineItem.ReadMarker
|
||||
)
|
||||
val items = listOf(
|
||||
aDaySeparator(TODAY),
|
||||
readMarker,
|
||||
anEvent,
|
||||
)
|
||||
val result = FilterEmptyDayPostProcessor().process(items)
|
||||
assertThat(result).hasSize(3)
|
||||
assertThat(result[0]).isEqualTo(aDaySeparator(TODAY))
|
||||
}
|
||||
}
|
||||
-75
@@ -1,75 +0,0 @@
|
||||
/*
|
||||
* Copyright (c) 2026 Element Creations Ltd.
|
||||
*
|
||||
* SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Element-Commercial.
|
||||
* Please see LICENSE files in the repository root for full details.
|
||||
*/
|
||||
|
||||
package io.element.android.libraries.matrix.impl.timeline.postprocessor
|
||||
|
||||
import com.google.common.truth.Truth.assertThat
|
||||
import io.element.android.libraries.matrix.api.room.join.JoinRule
|
||||
import org.junit.Test
|
||||
|
||||
class FilterPublicMembershipChangesPostProcessorTest {
|
||||
@Test
|
||||
fun `processor removes join, leave, and profile events in unencrypted public rooms`() {
|
||||
val timelineItems = listOf(
|
||||
roomCreateEvent,
|
||||
roomCreatorJoinEvent,
|
||||
otherMemberJoinEvent,
|
||||
messageEvent,
|
||||
otherMemberLeaveEvent,
|
||||
profileChangeEvent,
|
||||
)
|
||||
val expected = listOf(
|
||||
roomCreateEvent,
|
||||
messageEvent,
|
||||
)
|
||||
val processor = FilterPublicMembershipChangesPostProcessor()
|
||||
val processedItems = processor.process(
|
||||
timelineItems,
|
||||
joinRule = JoinRule.Public,
|
||||
isEncrypted = false,
|
||||
)
|
||||
assertThat(processedItems).isEqualTo(expected)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `processor keeps all events in encrypted public rooms`() {
|
||||
val timelineItems = listOf(
|
||||
roomCreateEvent,
|
||||
roomCreatorJoinEvent,
|
||||
otherMemberJoinEvent,
|
||||
messageEvent,
|
||||
otherMemberLeaveEvent,
|
||||
profileChangeEvent,
|
||||
)
|
||||
val processor = FilterPublicMembershipChangesPostProcessor()
|
||||
val processedItems = processor.process(
|
||||
timelineItems,
|
||||
joinRule = JoinRule.Public,
|
||||
isEncrypted = true,
|
||||
)
|
||||
assertThat(processedItems).isEqualTo(timelineItems)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `processor keeps membership events in invite-only rooms`() {
|
||||
val timelineItems = listOf(
|
||||
roomCreateEvent,
|
||||
roomCreatorJoinEvent,
|
||||
otherMemberJoinEvent,
|
||||
messageEvent,
|
||||
otherMemberLeaveEvent,
|
||||
profileChangeEvent,
|
||||
)
|
||||
val processor = FilterPublicMembershipChangesPostProcessor()
|
||||
val processedItems = processor.process(
|
||||
timelineItems,
|
||||
joinRule = JoinRule.Invite,
|
||||
isEncrypted = null,
|
||||
)
|
||||
assertThat(processedItems).isEqualTo(timelineItems)
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user