Don't recreate properly ordered list on navigate back to inbox

Change-Id: I377c5b2566e9b8d00cde7f9f03393bb0c9f1bba7
SchildiChat · Sep 1, 2024 · aa3c5b7 · aa3c5b7
1 parent 5c310cd
commit aa3c5b7
Show file tree

Hide file tree

Showing 5 changed files with 39 additions and 35 deletions.
diff --git a/...res/roomlist/impl/src/main/kotlin/chat/schildi/features/roomlist/ScRoomSortOrderSource.kt b/...res/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<CoroutineScope?>(null)
-
     @OptIn(ExperimentalCoroutinesApi::class)
-    fun filteredSummaries(coroutineScope: CoroutineScope): Flow<List<RoomSummary>> {
+    fun filteredSummaries(): Flow<List<RoomSummary>> {
         // 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/...rc/main/kotlin/io/element/android/features/roomlist/impl/datasource/RoomListDataSource.kt b/...rc/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/...x/api/src/main/kotlin/io/element/android/libraries/matrix/api/roomlist/RoomListService.kt b/...x/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/.../src/main/kotlin/io/element/android/libraries/matrix/impl/roomlist/RustRoomListService.kt b/.../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<Triple<DynamicRoomList, CoroutineScope, ScRoomSortOrder>?>(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<RoomId>) {

diff --git a/.../src/main/kotlin/io/element/android/libraries/matrix/test/roomlist/FakeRoomListService.kt b/.../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<RoomId>) {