From aa3c5b70d87965880329a8a70af37fff4d360d6c Mon Sep 17 00:00:00 2001 From: SpiritCroc Date: Sun, 1 Sep 2024 21:16:28 +0200 Subject: [PATCH] Don't recreate properly ordered list on navigate back to inbox Change-Id: I377c5b2566e9b8d00cde7f9f03393bb0c9f1bba7 --- .../roomlist/ScRoomSortOrderSource.kt | 22 ++-------- .../impl/datasource/RoomListDataSource.kt | 2 +- .../matrix/api/roomlist/RoomListService.kt | 3 +- .../impl/roomlist/RustRoomListService.kt | 44 ++++++++++++++----- .../test/roomlist/FakeRoomListService.kt | 3 +- 5 files changed, 39 insertions(+), 35 deletions(-) diff --git a/features/roomlist/impl/src/main/kotlin/chat/schildi/features/roomlist/ScRoomSortOrderSource.kt b/features/roomlist/impl/src/main/kotlin/chat/schildi/features/roomlist/ScRoomSortOrderSource.kt index 07c745f65e..5642932fba 100644 --- a/features/roomlist/impl/src/main/kotlin/chat/schildi/features/roomlist/ScRoomSortOrderSource.kt +++ b/features/roomlist/impl/src/main/kotlin/chat/schildi/features/roomlist/ScRoomSortOrderSource.kt @@ -1,20 +1,13 @@ package chat.schildi.features.roomlist -import androidx.lifecycle.AtomicReference import chat.schildi.lib.preferences.ScPreferencesStore import chat.schildi.lib.preferences.ScPrefs -import io.element.android.libraries.core.coroutine.childScope 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.ScRoomSortOrder -import io.element.android.libraries.matrix.api.roomlist.loadAllIncrementally -import kotlinx.coroutines.CoroutineScope -import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.ExperimentalCoroutinesApi -import kotlinx.coroutines.cancel -import kotlinx.coroutines.currentCoroutineContext import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.combine import kotlinx.coroutines.flow.flatMapLatest @@ -25,10 +18,8 @@ class ScRoomSortOrderSource @Inject constructor( private val scPreferencesStore: ScPreferencesStore, private val roomListService: RoomListService, ) { - private val lastCoroutineScope = AtomicReference(null) - @OptIn(ExperimentalCoroutinesApi::class) - fun filteredSummaries(coroutineScope: CoroutineScope): Flow> { + fun filteredSummaries(): Flow> { // Listen to preferences relevant to sort order, then apply these to the dynamic room list return combine( scPreferencesStore.settingFlow(ScPrefs.SORT_BY_UNREAD), @@ -45,18 +36,13 @@ class ScRoomSortOrderSource @Inject constructor( }.flatMapLatest { sortOrder -> // TODO: would be nice to teach the SDK to update sort order without recreating the whole list // Cancel jobs for previous list - val scope = coroutineScope.childScope(Dispatchers.Default, "sc-sorted-room-list") - lastCoroutineScope.getAndSet(scope)?.cancel() - Timber.d("Create new filtered and sorted room list for $sortOrder") - roomListService.scCreateRoomList( + Timber.d("Request sorted room list for $sortOrder") + roomListService.getOrReplaceRoomListWithSortOrder( pageSize = 30, initialFilter = RoomListFilter.all(), source = RoomList.Source.All, sortOrder = sortOrder, - coroutineScope = scope, - ).also { - it.loadAllIncrementally(scope) - }.filteredSummaries + ).filteredSummaries } } } diff --git a/features/roomlist/impl/src/main/kotlin/io/element/android/features/roomlist/impl/datasource/RoomListDataSource.kt b/features/roomlist/impl/src/main/kotlin/io/element/android/features/roomlist/impl/datasource/RoomListDataSource.kt index 4b04f66752..4c7dc63b8f 100644 --- a/features/roomlist/impl/src/main/kotlin/io/element/android/features/roomlist/impl/datasource/RoomListDataSource.kt +++ b/features/roomlist/impl/src/main/kotlin/io/element/android/features/roomlist/impl/datasource/RoomListDataSource.kt @@ -70,7 +70,7 @@ class RoomListDataSource @Inject constructor( .allRooms .filteredSummaries */ - scRoomSortOrderSource.filteredSummaries(coroutineScope) + scRoomSortOrderSource.filteredSummaries() .onEach { roomSummaries -> replaceWith(roomSummaries) } diff --git a/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/roomlist/RoomListService.kt b/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/roomlist/RoomListService.kt index e67d96d328..19da4fccc3 100644 --- a/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/roomlist/RoomListService.kt +++ b/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/roomlist/RoomListService.kt @@ -55,11 +55,10 @@ interface RoomListService { source: RoomList.Source, ): DynamicRoomList - fun scCreateRoomList( + fun getOrReplaceRoomListWithSortOrder( pageSize: Int, initialFilter: RoomListFilter, source: RoomList.Source, - coroutineScope: CoroutineScope, sortOrder: ScRoomSortOrder = ScRoomSortOrder(), ): DynamicRoomList diff --git a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/roomlist/RustRoomListService.kt b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/roomlist/RustRoomListService.kt index 60e9f45dfa..381aafc22b 100644 --- a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/roomlist/RustRoomListService.kt +++ b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/roomlist/RustRoomListService.kt @@ -16,6 +16,7 @@ package io.element.android.libraries.matrix.impl.roomlist +import io.element.android.libraries.core.coroutine.childScope import io.element.android.libraries.matrix.api.core.RoomId import io.element.android.libraries.matrix.api.roomlist.DynamicRoomList import io.element.android.libraries.matrix.api.roomlist.RoomList @@ -26,15 +27,19 @@ import io.element.android.libraries.matrix.api.roomlist.loadAllIncrementally import io.element.android.libraries.matrix.impl.room.RoomSyncSubscriber import kotlinx.coroutines.CoroutineDispatcher import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.cancel import kotlinx.coroutines.flow.SharingStarted import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.distinctUntilChanged import kotlinx.coroutines.flow.map import kotlinx.coroutines.flow.onEach import kotlinx.coroutines.flow.stateIn +import kotlinx.coroutines.isActive import org.matrix.rustcomponents.sdk.RoomListServiceState import org.matrix.rustcomponents.sdk.RoomListServiceSyncIndicator import timber.log.Timber +import java.util.concurrent.atomic.AtomicReference import org.matrix.rustcomponents.sdk.RoomListService as InnerRustRoomListService private const val DEFAULT_PAGE_SIZE = 20 @@ -46,6 +51,8 @@ internal class RustRoomListService( private val roomListFactory: RoomListFactory, private val roomSyncSubscriber: RoomSyncSubscriber, ) : RoomListService { + private val lastListWithSortOrder = AtomicReference?>(null) + override fun createRoomList( pageSize: Int, initialFilter: RoomListFilter, @@ -63,24 +70,37 @@ internal class RustRoomListService( } } - override fun scCreateRoomList( // SC: add coroutineScope and sortOrder - tricky to have coroutineScope with default fallback for child classes + override fun getOrReplaceRoomListWithSortOrder( pageSize: Int, initialFilter: RoomListFilter, source: RoomList.Source, - coroutineScope: CoroutineScope, sortOrder: ScRoomSortOrder ): DynamicRoomList { - return roomListFactory.createRoomList( - pageSize = pageSize, - initialFilter = initialFilter, - coroutineScope = coroutineScope, - sortOrder = sortOrder, - ) { - when (source) { - RoomList.Source.All -> innerRoomListService.allRooms() - RoomList.Source.SPACES -> innerRoomListService.allSpaces() + return lastListWithSortOrder.updateAndGet { previous -> + if (previous?.third == sortOrder && previous.second.isActive) { + previous + } else { + val scope = sessionCoroutineScope.childScope(Dispatchers.Default, "sc-room-list") + Triple( + roomListFactory.createRoomList( + pageSize = pageSize, + initialFilter = initialFilter, + coroutineScope = scope, + sortOrder = sortOrder, + ) { + when (source) { + RoomList.Source.All -> innerRoomListService.allRooms() + RoomList.Source.SPACES -> innerRoomListService.allSpaces() + } + }.also { roomList -> + previous?.second?.cancel("Sorted room list being replaced") + roomList.loadAllIncrementally(scope) + }, + scope, + sortOrder + ) } - } + }!!.first } override suspend fun subscribeToVisibleRooms(roomIds: List) { diff --git a/libraries/matrix/test/src/main/kotlin/io/element/android/libraries/matrix/test/roomlist/FakeRoomListService.kt b/libraries/matrix/test/src/main/kotlin/io/element/android/libraries/matrix/test/roomlist/FakeRoomListService.kt index 17d5a687cf..472173bf32 100644 --- a/libraries/matrix/test/src/main/kotlin/io/element/android/libraries/matrix/test/roomlist/FakeRoomListService.kt +++ b/libraries/matrix/test/src/main/kotlin/io/element/android/libraries/matrix/test/roomlist/FakeRoomListService.kt @@ -23,7 +23,6 @@ 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.ScRoomSortOrder -import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.StateFlow @@ -64,7 +63,7 @@ class FakeRoomListService( } } - override fun scCreateRoomList(pageSize: Int, initialFilter: RoomListFilter, source: RoomList.Source, coroutineScope: CoroutineScope, sortOrder: ScRoomSortOrder) + override fun getOrReplaceRoomListWithSortOrder(pageSize: Int, initialFilter: RoomListFilter, source: RoomList.Source, sortOrder: ScRoomSortOrder) = createRoomList(pageSize, initialFilter, source) override suspend fun subscribeToVisibleRooms(roomIds: List) {