RoomListFilters : use kotlin filtering as rust one is slower and has more chance to bust the room list cache.

This commit is contained in:
ganfra
2024-02-27 16:24:54 +01:00
parent f5c6aaf94e
commit 26607761a1
13 changed files with 278 additions and 112 deletions

View File

@@ -83,7 +83,7 @@ class RoomListFiltersPresenter @Inject constructor(
RoomListFilter.Unread -> MatrixRoomListFilter.Unread
RoomListFilter.Favourites -> MatrixRoomListFilter.Favorite
}
}.plus(MatrixRoomListFilter.NonLeft)
}
)
roomListService.allRooms.updateFilter(allRoomsFilter)
}

View File

@@ -0,0 +1,73 @@
/*
* Copyright (c) 2024 New Vector Ltd
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package io.element.android.features.roomlist.impl.search
import io.element.android.features.roomlist.impl.datasource.RoomListRoomSummaryFactory
import io.element.android.features.roomlist.impl.model.RoomListRoomSummary
import io.element.android.libraries.core.coroutine.CoroutineDispatchers
import io.element.android.libraries.matrix.api.roomlist.RoomList
import io.element.android.libraries.matrix.api.roomlist.RoomListFilter
import io.element.android.libraries.matrix.api.roomlist.RoomListService
import io.element.android.libraries.matrix.api.roomlist.RoomSummary
import io.element.android.libraries.matrix.api.roomlist.loadAllIncrementally
import kotlinx.collections.immutable.PersistentList
import kotlinx.collections.immutable.toPersistentList
import kotlinx.coroutines.coroutineScope
import kotlinx.coroutines.flow.Flow
import kotlinx.coroutines.flow.flowOn
import kotlinx.coroutines.flow.map
import javax.inject.Inject
private const val PAGE_SIZE = 30
class RoomListSearchDataSource @Inject constructor(
roomListService: RoomListService,
coroutineDispatchers: CoroutineDispatchers,
private val roomSummaryFactory: RoomListRoomSummaryFactory,
) {
private val roomList = roomListService.createRoomList(
pageSize = PAGE_SIZE,
initialFilter = RoomListFilter.None,
source = RoomList.Source.All,
)
val roomSummaries: Flow<PersistentList<RoomListRoomSummary>> = roomList.summaries
.map { roomSummaries ->
roomSummaries
.filterIsInstance<RoomSummary.Filled>()
.map(roomSummaryFactory::create)
.toPersistentList()
}
.flowOn(coroutineDispatchers.computation)
suspend fun setIsActive(isActive: Boolean) = coroutineScope {
if (isActive) {
roomList.loadAllIncrementally(this)
} else {
roomList.reset()
}
}
suspend fun setSearchQuery(searchQuery: String) = coroutineScope {
val filter = if (searchQuery.isBlank()) {
RoomListFilter.None
} else {
RoomListFilter.NormalizedMatchRoomName(searchQuery)
}
roomList.updateFilter(filter)
}
}

View File

@@ -21,30 +21,14 @@ import androidx.compose.runtime.LaunchedEffect
import androidx.compose.runtime.collectAsState
import androidx.compose.runtime.getValue
import androidx.compose.runtime.mutableStateOf
import androidx.compose.runtime.remember
import androidx.compose.runtime.rememberCoroutineScope
import androidx.compose.runtime.saveable.rememberSaveable
import androidx.compose.runtime.setValue
import io.element.android.features.roomlist.impl.datasource.RoomListRoomSummaryFactory
import io.element.android.libraries.architecture.Presenter
import io.element.android.libraries.core.coroutine.CoroutineDispatchers
import io.element.android.libraries.matrix.api.roomlist.RoomList
import io.element.android.libraries.matrix.api.roomlist.RoomListFilter
import io.element.android.libraries.matrix.api.roomlist.RoomListService
import io.element.android.libraries.matrix.api.roomlist.RoomSummary
import io.element.android.libraries.matrix.api.roomlist.loadAllIncrementally
import kotlinx.collections.immutable.persistentListOf
import kotlinx.collections.immutable.toPersistentList
import kotlinx.coroutines.flow.flowOn
import kotlinx.coroutines.flow.map
import javax.inject.Inject
private const val PAGE_SIZE = 50
class RoomListSearchPresenter @Inject constructor(
private val roomListService: RoomListService,
private val roomSummaryFactory: RoomListRoomSummaryFactory,
private val coroutineDispatchers: CoroutineDispatchers,
private val dataSource: RoomListSearchDataSource,
) : Presenter<RoomListSearchState> {
@Composable
override fun present(): RoomListSearchState {
@@ -54,27 +38,13 @@ class RoomListSearchPresenter @Inject constructor(
var searchQuery by rememberSaveable {
mutableStateOf("")
}
val coroutineScope = rememberCoroutineScope()
val roomList = remember {
roomListService.createRoomList(
coroutineScope = coroutineScope,
pageSize = PAGE_SIZE,
initialFilter = RoomListFilter.all(RoomListFilter.None),
source = RoomList.Source.All,
)
LaunchedEffect(isSearchActive) {
dataSource.setIsActive(isSearchActive)
}
LaunchedEffect(Unit) {
roomList.loadAllIncrementally(this)
}
LaunchedEffect(key1 = searchQuery) {
val filter = if (searchQuery.isBlank()) {
RoomListFilter.all(RoomListFilter.None)
} else {
RoomListFilter.all(RoomListFilter.NonLeft, RoomListFilter.NormalizedMatchRoomName(searchQuery))
}
roomList.updateFilter(filter)
LaunchedEffect(searchQuery) {
dataSource.setSearchQuery(searchQuery)
}
fun handleEvents(event: RoomListSearchEvents) {
@@ -92,9 +62,7 @@ class RoomListSearchPresenter @Inject constructor(
}
}
val searchResults by roomList
.rememberMappedSummaries()
.collectAsState(initial = persistentListOf())
val searchResults by dataSource.roomSummaries.collectAsState(initial = persistentListOf())
return RoomListSearchState(
isSearchActive = isSearchActive,
@@ -103,16 +71,4 @@ class RoomListSearchPresenter @Inject constructor(
eventSink = ::handleEvents
)
}
@Composable
private fun RoomList.rememberMappedSummaries() = remember {
summaries
.map { roomSummaries ->
roomSummaries
.filterIsInstance<RoomSummary.Filled>()
.map(roomSummaryFactory::create)
.toPersistentList()
}
.flowOn(coroutineDispatchers.computation)
}
}

View File

@@ -69,7 +69,6 @@ class RoomListFiltersPresenterTests {
)
val roomListCurrentFilter = roomListService.allRooms.currentFilter.value as MatrixRoomListFilter.All
assertThat(roomListCurrentFilter.filters).containsExactly(
MatrixRoomListFilter.NonLeft,
MatrixRoomListFilter.Category.Group,
)
@@ -86,9 +85,7 @@ class RoomListFiltersPresenterTests {
RoomListFilter.Favourites,
)
val roomListCurrentFilter = roomListService.allRooms.currentFilter.value as MatrixRoomListFilter.All
assertThat(roomListCurrentFilter.filters).containsExactly(
MatrixRoomListFilter.NonLeft,
)
assertThat(roomListCurrentFilter.filters).isEmpty()
}
}
}

View File

@@ -79,9 +79,7 @@ class RoomListSearchPresenterTests {
assertThat(
roomListService.allRooms.currentFilter.value
).isEqualTo(
RoomListFilter.all(
RoomListFilter.None,
)
RoomListFilter.None,
)
state.eventSink(RoomListSearchEvents.QueryChanged("Search"))
}
@@ -90,10 +88,7 @@ class RoomListSearchPresenterTests {
assertThat(
roomListService.allRooms.currentFilter.value
).isEqualTo(
RoomListFilter.all(
RoomListFilter.NonLeft,
RoomListFilter.NormalizedMatchRoomName("Search")
)
RoomListFilter.NormalizedMatchRoomName("Search")
)
state.eventSink(RoomListSearchEvents.ClearQuery)
}
@@ -102,9 +97,7 @@ class RoomListSearchPresenterTests {
assertThat(
roomListService.allRooms.currentFilter.value
).isEqualTo(
RoomListFilter.all(
RoomListFilter.None,
)
RoomListFilter.None,
)
}
}
@@ -141,11 +134,13 @@ fun TestScope.createRoomListSearchPresenter(
roomListService: RoomListService = FakeRoomListService(),
): RoomListSearchPresenter {
return RoomListSearchPresenter(
roomListService = roomListService,
roomSummaryFactory = RoomListRoomSummaryFactory(
lastMessageTimestampFormatter = FakeLastMessageTimestampFormatter(),
roomLastMessageFormatter = FakeRoomLastMessageFormatter(),
dataSource = RoomListSearchDataSource(
roomListService = roomListService,
roomSummaryFactory = RoomListRoomSummaryFactory(
lastMessageTimestampFormatter = FakeLastMessageTimestampFormatter(),
roomLastMessageFormatter = FakeRoomLastMessageFormatter(),
),
coroutineDispatchers = testCoroutineDispatchers(),
coroutineDispatchers = testCoroutineDispatchers(),
)
)
}

View File

@@ -47,11 +47,6 @@ sealed interface RoomListFilter {
val filters: List<RoomListFilter>
) : RoomListFilter
/**
* A filter that matches rooms that are not left.
*/
data object NonLeft : RoomListFilter
/**
* A filter that matches rooms that are unread.
*/
@@ -81,11 +76,4 @@ sealed interface RoomListFilter {
data class NormalizedMatchRoomName(
val pattern: String
) : RoomListFilter
/**
* A filter that matches rooms with a name using a fuzzy match.
*/
data class FuzzyMatchRoomName(
val pattern: String
) : RoomListFilter
}

View File

@@ -17,7 +17,6 @@
package io.element.android.libraries.matrix.api.roomlist
import androidx.compose.runtime.Immutable
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.flow.StateFlow
/**
@@ -42,13 +41,11 @@ interface RoomListService {
/**
* Creates a room list that can be used to load more rooms and filter them dynamically.
* @param coroutineScope the scope to use for the room list. When the scope will be closed, the room list will be closed too.
* @param pageSize the number of rooms to load at once.
* @param initialFilter the initial filter to apply to the rooms.
* @param source the source of the rooms, either all rooms or invites.
*/
fun createRoomList(
coroutineScope: CoroutineScope,
pageSize: Int,
initialFilter: RoomListFilter,
source: RoomList.Source,

View File

@@ -23,11 +23,13 @@ import io.element.android.libraries.matrix.api.roomlist.RoomSummary
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.flow.MutableSharedFlow
import kotlinx.coroutines.flow.MutableStateFlow
import kotlinx.coroutines.flow.combine
import kotlinx.coroutines.flow.getAndUpdate
import kotlinx.coroutines.flow.launchIn
import kotlinx.coroutines.flow.map
import kotlinx.coroutines.flow.onEach
import kotlinx.coroutines.launch
import org.matrix.rustcomponents.sdk.RoomListEntriesDynamicFilterKind
import org.matrix.rustcomponents.sdk.RoomListLoadingState
import org.matrix.rustcomponents.sdk.RoomListService
import kotlin.coroutines.CoroutineContext
@@ -50,6 +52,7 @@ internal class RoomListFactory(
innerProvider: suspend () -> InnerRoomList
): DynamicRoomList {
val loadingStateFlow: MutableStateFlow<RoomList.LoadingState> = MutableStateFlow(RoomList.LoadingState.NotLoaded)
val filteredSummariesFlow = MutableSharedFlow<List<RoomSummary>>(replay = 1, extraBufferCapacity = 1)
val summariesFlow = MutableSharedFlow<List<RoomSummary>>(replay = 1, extraBufferCapacity = 1)
val processor = RoomSummaryListProcessor(summariesFlow, innerRoomListService, coroutineContext, roomSummaryDetailsFactory)
// Makes sure we don't miss any events
@@ -63,8 +66,8 @@ internal class RoomListFactory(
innerRoomList?.let { innerRoomList ->
innerRoomList.entriesFlow(
pageSize = pageSize,
initialFilterKind = initialFilter.toRustFilter(),
roomListDynamicEvents = dynamicEvents
roomListDynamicEvents = dynamicEvents,
initialFilterKind = RoomListEntriesDynamicFilterKind.NonLeft
).onEach { update ->
processor.postUpdate(update)
}.launchIn(this)
@@ -75,12 +78,21 @@ internal class RoomListFactory(
loadingStateFlow.value = it
}
.launchIn(this)
combine(
currentFilter,
summariesFlow
) { filter, summaries ->
summaries.filter(filter)
}.onEach {
filteredSummariesFlow.emit(it)
}.launchIn(this)
}
}.invokeOnCompletion {
innerRoomList?.destroy()
}
return RustDynamicRoomList(
summaries = summariesFlow,
summaries = filteredSummariesFlow,
loadingState = loadingStateFlow,
currentFilter = currentFilter,
loadedPages = loadedPages,
@@ -106,8 +118,6 @@ private class RustDynamicRoomList(
override suspend fun updateFilter(filter: RoomListFilter) {
currentFilter.emit(filter)
val filterEvent = RoomListDynamicEvents.SetFilter(filter.toRustFilter())
dynamicEvents.emit(filterEvent)
}
override suspend fun loadMore() {

View File

@@ -17,20 +17,41 @@
package io.element.android.libraries.matrix.impl.roomlist
import io.element.android.libraries.matrix.api.roomlist.RoomListFilter
import org.matrix.rustcomponents.sdk.RoomListEntriesDynamicFilterKind
import org.matrix.rustcomponents.sdk.RoomListFilterCategory
import io.element.android.libraries.matrix.api.roomlist.RoomSummary
fun RoomListFilter.toRustFilter(): RoomListEntriesDynamicFilterKind {
return when (this) {
is RoomListFilter.All -> RoomListEntriesDynamicFilterKind.All(filters.map { it.toRustFilter() })
is RoomListFilter.Any -> RoomListEntriesDynamicFilterKind.Any(filters.map { it.toRustFilter() })
RoomListFilter.Category.Group -> RoomListEntriesDynamicFilterKind.Category(RoomListFilterCategory.GROUP)
RoomListFilter.Category.People -> RoomListEntriesDynamicFilterKind.Category(RoomListFilterCategory.PEOPLE)
is RoomListFilter.FuzzyMatchRoomName -> RoomListEntriesDynamicFilterKind.FuzzyMatchRoomName(pattern)
RoomListFilter.NonLeft -> RoomListEntriesDynamicFilterKind.NonLeft
RoomListFilter.None -> RoomListEntriesDynamicFilterKind.None
is RoomListFilter.NormalizedMatchRoomName -> RoomListEntriesDynamicFilterKind.NormalizedMatchRoomName(pattern)
RoomListFilter.Unread -> RoomListEntriesDynamicFilterKind.Unread
RoomListFilter.Favorite -> RoomListEntriesDynamicFilterKind.Favourite
val RoomListFilter.predicate
get() = when (this) {
is RoomListFilter.All -> { _: RoomSummary -> true }
is RoomListFilter.Any -> { _: RoomSummary -> true }
RoomListFilter.None -> { _: RoomSummary -> false }
RoomListFilter.Category.Group -> { roomSummary: RoomSummary ->
roomSummary is RoomSummary.Filled && !roomSummary.details.isDirect
}
RoomListFilter.Category.People -> { roomSummary: RoomSummary ->
roomSummary is RoomSummary.Filled && roomSummary.details.isDirect
}
RoomListFilter.Favorite -> { roomSummary: RoomSummary ->
roomSummary is RoomSummary.Filled && roomSummary.details.isFavorite
}
RoomListFilter.Unread -> { roomSummary: RoomSummary ->
roomSummary is RoomSummary.Filled &&
(roomSummary.details.numUnreadNotifications > 0 || roomSummary.details.isMarkedUnread)
}
is RoomListFilter.NormalizedMatchRoomName -> { roomSummary: RoomSummary ->
roomSummary is RoomSummary.Filled && roomSummary.details.name.contains(pattern, ignoreCase = true)
}
}
fun List<RoomSummary>.filter(filter: RoomListFilter): List<RoomSummary> {
return when (filter) {
is RoomListFilter.All -> {
val predicates = filter.filters.map { it.predicate }
filter { roomSummary -> predicates.all { it(roomSummary) } }
}
is RoomListFilter.Any -> {
val predicates = filter.filters.map { it.predicate }
filter { roomSummary -> predicates.any { it(roomSummary) } }
}
else -> filter(filter.predicate)
}
}

View File

@@ -47,7 +47,6 @@ internal class RustRoomListService(
private val roomListFactory: RoomListFactory,
) : RoomListService {
override fun createRoomList(
coroutineScope: CoroutineScope,
pageSize: Int,
initialFilter: RoomListFilter,
source: RoomList.Source
@@ -55,7 +54,6 @@ internal class RustRoomListService(
return roomListFactory.createRoomList(
pageSize = pageSize,
initialFilter = initialFilter,
coroutineScope = coroutineScope,
coroutineContext = sessionDispatcher,
) {
when (source) {
@@ -68,7 +66,6 @@ internal class RustRoomListService(
override val allRooms: DynamicRoomList = roomListFactory.createRoomList(
pageSize = DEFAULT_PAGE_SIZE,
coroutineContext = sessionDispatcher,
initialFilter = RoomListFilter.all(RoomListFilter.NonLeft),
) {
innerRoomListService.allRooms()
}

View File

@@ -0,0 +1,126 @@
/*
* Copyright (c) 2024 New Vector Ltd
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package io.element.android.libraries.matrix.impl.roomlist
import com.google.common.truth.Truth.assertThat
import io.element.android.libraries.matrix.api.roomlist.RoomListFilter
import io.element.android.libraries.matrix.test.room.aRoomSummaryDetails
import io.element.android.libraries.matrix.test.room.aRoomSummaryFilled
import kotlinx.coroutines.test.runTest
import org.junit.Test
class RoomListFilterTests {
private val regularRoom = aRoomSummaryFilled(
aRoomSummaryDetails(
isDirect = false
)
)
private val directRoom = aRoomSummaryFilled(
aRoomSummaryDetails(
isDirect = true
)
)
private val favoriteRoom = aRoomSummaryFilled(
aRoomSummaryDetails(
isFavorite = true
)
)
private val markedAsUnreadRoom = aRoomSummaryFilled(
aRoomSummaryDetails(
isMarkedUnread = true
)
)
private val unreadNotificationRoom = aRoomSummaryFilled(
aRoomSummaryDetails(
numUnreadNotifications = 1
)
)
private val roomToSearch = aRoomSummaryFilled(
aRoomSummaryDetails(
name = "Room to search"
)
)
private val roomSummaries = listOf(
regularRoom,
directRoom,
favoriteRoom,
markedAsUnreadRoom,
unreadNotificationRoom,
roomToSearch
)
@Test
fun `Room list filter all empty`() = runTest {
val filter = RoomListFilter.all()
assertThat(roomSummaries.filter(filter)).isEqualTo(roomSummaries)
}
@Test
fun `Room list filter none`() = runTest {
val filter = RoomListFilter.None
assertThat(roomSummaries.filter(filter)).isEmpty()
}
@Test
fun `Room list filter people`() = runTest {
val filter = RoomListFilter.Category.People
assertThat(roomSummaries.filter(filter)).containsExactly(directRoom)
}
@Test
fun `Room list filter group`() = runTest {
val filter = RoomListFilter.Category.Group
assertThat(roomSummaries.filter(filter)).containsExactly(regularRoom, favoriteRoom, markedAsUnreadRoom, unreadNotificationRoom, roomToSearch)
}
@Test
fun `Room list filter favorite`() = runTest {
val filter = RoomListFilter.Favorite
assertThat(roomSummaries.filter(filter)).containsExactly(favoriteRoom)
}
@Test
fun `Room list filter unread`() = runTest {
val filter = RoomListFilter.Unread
assertThat(roomSummaries.filter(filter)).containsExactly(markedAsUnreadRoom, unreadNotificationRoom)
}
@Test
fun `Room list filter normalized match room name`() = runTest {
val filter = RoomListFilter.NormalizedMatchRoomName("search")
assertThat(roomSummaries.filter(filter)).containsExactly(roomToSearch)
}
@Test
fun `Room list filter all with one match`() = runTest {
val filter = RoomListFilter.all(
RoomListFilter.Category.Group,
RoomListFilter.Favorite
)
assertThat(roomSummaries.filter(filter)).containsExactly(favoriteRoom)
}
@Test
fun `Room list filter all with no match`() = runTest {
val filter = RoomListFilter.all(
RoomListFilter.Category.People,
RoomListFilter.Favorite
)
assertThat(roomSummaries.filter(filter)).isEmpty()
}
}

View File

@@ -21,7 +21,6 @@ import io.element.android.libraries.matrix.api.roomlist.RoomList
import io.element.android.libraries.matrix.api.roomlist.RoomListFilter
import io.element.android.libraries.matrix.api.roomlist.RoomListService
import io.element.android.libraries.matrix.api.roomlist.RoomSummary
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.flow.MutableStateFlow
import kotlinx.coroutines.flow.StateFlow
@@ -60,7 +59,11 @@ class FakeRoomListService : RoomListService {
var latestSlidingSyncRange: IntRange? = null
private set
override fun createRoomList(coroutineScope: CoroutineScope, pageSize: Int, initialFilter: RoomListFilter, source: RoomList.Source): DynamicRoomList {
override fun createRoomList(
pageSize: Int,
initialFilter: RoomListFilter,
source: RoomList.Source
): DynamicRoomList {
return when (source) {
RoomList.Source.All -> allRooms
RoomList.Source.Invites -> invites

View File

@@ -31,6 +31,7 @@ import io.element.android.features.roomlist.impl.datasource.RoomListRoomSummaryF
import io.element.android.features.roomlist.impl.filters.RoomListFiltersPresenter
import io.element.android.features.roomlist.impl.migration.MigrationScreenPresenter
import io.element.android.features.roomlist.impl.migration.SharedPrefsMigrationScreenStore
import io.element.android.features.roomlist.impl.search.RoomListSearchDataSource
import io.element.android.features.roomlist.impl.search.RoomListSearchPresenter
import io.element.android.libraries.core.coroutine.CoroutineDispatchers
import io.element.android.libraries.dateformatter.impl.DateFormatters
@@ -112,9 +113,11 @@ class RoomListScreen(
migrationScreenStore = SharedPrefsMigrationScreenStore(context.getSharedPreferences("migration", Context.MODE_PRIVATE))
),
searchPresenter = RoomListSearchPresenter(
roomListService = matrixClient.roomListService,
roomSummaryFactory = roomListRoomSummaryFactory,
coroutineDispatchers = coroutineDispatchers,
RoomListSearchDataSource(
roomListService = matrixClient.roomListService,
roomSummaryFactory = roomListRoomSummaryFactory,
coroutineDispatchers = coroutineDispatchers,
)
),
sessionPreferencesStore = DefaultSessionPreferencesStore(
context = context,