Merge pull request #6314 from vector-im/task/eric/replace_flatten_with_direct_parent

Replace flattenParents with directParentName

changelog.d/6314.misc

@@ -0,0 +1 @@
+Improves performance on search screen by replacing flattenParents with directParentName in RoomSummary

matrix-sdk-android/src/androidTest/java/org/matrix/android/sdk/session/space/SpaceHierarchyTest.kt

@@ -610,4 +610,82 @@ class SpaceHierarchyTest : InstrumentedTest {
             }
         }
     }
+
+    @Test
+    fun testDirectParentNames() = runSessionTest(context()) { commonTestHelper ->
+        val aliceSession = commonTestHelper.createAccount("Alice", SessionTestParams(true))
+
+        val spaceAInfo = createPublicSpace(
+                commonTestHelper,
+                aliceSession, "SpaceA",
+                listOf(
+                        Triple("A1", true /*auto-join*/, true/*canonical*/),
+                        Triple("A2", true, true)
+                )
+        )
+
+        val spaceBInfo = createPublicSpace(
+                commonTestHelper,
+                aliceSession, "SpaceB",
+                listOf(
+                        Triple("B1", true /*auto-join*/, true/*canonical*/),
+                        Triple("B2", true, true),
+                        Triple("B3", true, true)
+                )
+        )
+
+        // also add B1 in space A
+
+        val B1roomId = spaceBInfo.roomIds.first()
+        val viaServers = listOf(aliceSession.sessionParams.homeServerHost ?: "")
+
+        val spaceA = aliceSession.spaceService().getSpace(spaceAInfo.spaceId)
+        val spaceB = aliceSession.spaceService().getSpace(spaceBInfo.spaceId)
+        commonTestHelper.runBlockingTest {
+            spaceA!!.addChildren(B1roomId, viaServers, null, true)
+        }
+
+        commonTestHelper.waitWithLatch { latch ->
+            commonTestHelper.retryPeriodicallyWithLatch(latch) {
+                val roomSummary = aliceSession.getRoomSummary(B1roomId)
+                roomSummary != null &&
+                        roomSummary.directParentNames.size == 2 &&
+                        roomSummary.directParentNames.contains(spaceA!!.spaceSummary()!!.name) &&
+                        roomSummary.directParentNames.contains(spaceB!!.spaceSummary()!!.name)
+            }
+        }
+
+        commonTestHelper.waitWithLatch { latch ->
+            commonTestHelper.retryPeriodicallyWithLatch(latch) {
+                val roomSummary = aliceSession.getRoomSummary(spaceAInfo.roomIds.first())
+                roomSummary != null &&
+                        roomSummary.directParentNames.size == 1 &&
+                        roomSummary.directParentNames.contains(spaceA!!.spaceSummary()!!.name)
+            }
+        }
+
+        val newAName = "FooBar"
+        commonTestHelper.runBlockingTest {
+            spaceA!!.asRoom().stateService().updateName(newAName)
+        }
+
+        commonTestHelper.waitWithLatch { latch ->
+            commonTestHelper.retryPeriodicallyWithLatch(latch) {
+                val roomSummary = aliceSession.getRoomSummary(B1roomId)
+                roomSummary != null &&
+                        roomSummary.directParentNames.size == 2 &&
+                        roomSummary.directParentNames.contains(newAName) &&
+                        roomSummary.directParentNames.contains(spaceB!!.spaceSummary()!!.name)
+            }
+        }
+
+        commonTestHelper.waitWithLatch { latch ->
+            commonTestHelper.retryPeriodicallyWithLatch(latch) {
+                val roomSummary = aliceSession.getRoomSummary(spaceAInfo.roomIds.first())
+                roomSummary != null &&
+                        roomSummary.directParentNames.size == 1 &&
+                        roomSummary.directParentNames.contains(newAName)
+            }
+        }
+    }
 }

matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/room/RoomService.kt

@@ -243,14 +243,11 @@ interface RoomService {
      * @param queryParams The filter to use
      * @param pagedListConfig The paged list configuration (page size, initial load, prefetch distance...)
      * @param sortOrder defines how to sort the results
-     * @param getFlattenParents When true, the list of known parents and grand parents summaries will be resolved.
-     * This can have significant impact on performance, better be used only on manageable list (filtered by displayName, ..).
      */
     fun getFilteredPagedRoomSummariesLive(
             queryParams: RoomSummaryQueryParams,
             pagedListConfig: PagedList.Config = defaultPagedListConfig,
             sortOrder: RoomSortOrder = RoomSortOrder.ACTIVITY,
-            getFlattenParents: Boolean = false,
     ): UpdatableLivePageResult
 
     /**

matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/room/model/RoomSummary.kt

@@ -164,9 +164,9 @@ data class RoomSummary(
          */
         val spaceChildren: List<SpaceChildInfo>? = null,
         /**
-         * List of all the space parents. Will be empty by default, you have to explicitly request it.
+         * The names of the room's direct space parents if any.
          */
-        val flattenParents: List<RoomSummary> = emptyList(),
+        val directParentNames: List<String> = emptyList(),
         /**
          * List of all the space parent Ids.
          */

matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/database/RealmSessionStoreMigration.kt

@@ -51,6 +51,7 @@ import org.matrix.android.sdk.internal.database.migration.MigrateSessionTo031
 import org.matrix.android.sdk.internal.database.migration.MigrateSessionTo032
 import org.matrix.android.sdk.internal.database.migration.MigrateSessionTo033
 import org.matrix.android.sdk.internal.database.migration.MigrateSessionTo034
+import org.matrix.android.sdk.internal.database.migration.MigrateSessionTo035
 import org.matrix.android.sdk.internal.util.Normalizer
 import org.matrix.android.sdk.internal.util.database.MatrixRealmMigration
 import javax.inject.Inject
@@ -59,7 +60,7 @@ internal class RealmSessionStoreMigration @Inject constructor(
         private val normalizer: Normalizer
 ) : MatrixRealmMigration(
         dbName = "Session",
-        schemaVersion = 34L,
+        schemaVersion = 35L,
 ) {
     /**
      * Forces all RealmSessionStoreMigration instances to be equal.
@@ -103,5 +104,6 @@ internal class RealmSessionStoreMigration @Inject constructor(
         if (oldVersion < 32) MigrateSessionTo032(realm).perform()
         if (oldVersion < 33) MigrateSessionTo033(realm).perform()
         if (oldVersion < 34) MigrateSessionTo034(realm).perform()
+        if (oldVersion < 35) MigrateSessionTo035(realm).perform()
     }
 }

matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/database/mapper/RoomSummaryMapper.kt

@@ -106,6 +106,7 @@ internal class RoomSummaryMapper @Inject constructor(
                             worldReadable = it.childSummaryEntity?.joinRules == RoomJoinRules.PUBLIC
                     )
                 },
+                directParentNames = roomSummaryEntity.directParentNames.toList(),
                 flattenParentIds = roomSummaryEntity.flattenParentIds?.split("|") ?: emptyList(),
                 roomEncryptionAlgorithm = when (val alg = roomSummaryEntity.e2eAlgorithm) {
                     // I should probably use #hasEncryptorClassForAlgorithm but it says it supports

matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/database/migration/MigrateSessionTo035.kt

@@ -0,0 +1,31 @@
+/*
+ * Copyright (c) 2022 The Matrix.org Foundation C.I.C.
+ *
+ * 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 org.matrix.android.sdk.internal.database.migration
+
+import io.realm.DynamicRealm
+import io.realm.RealmList
+import org.matrix.android.sdk.internal.database.model.RoomSummaryEntityFields
+import org.matrix.android.sdk.internal.util.database.RealmMigrator
+
+internal class MigrateSessionTo035(realm: DynamicRealm) : RealmMigrator(realm, 35) {
+
+    override fun doMigrate(realm: DynamicRealm) {
+        realm.schema.get("RoomSummaryEntity")
+                ?.addRealmListField(RoomSummaryEntityFields.DIRECT_PARENT_NAMES.`$`, String::class.java)
+                ?.transform { it.setList(RoomSummaryEntityFields.DIRECT_PARENT_NAMES.`$`, RealmList("")) }
+    }
+}

matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/database/model/RoomSummaryEntity.kt

@@ -34,7 +34,8 @@ internal open class RoomSummaryEntity(
         @PrimaryKey var roomId: String = "",
         var roomType: String? = null,
         var parents: RealmList<SpaceParentSummaryEntity> = RealmList(),
-        var children: RealmList<SpaceChildSummaryEntity> = RealmList()
+        var children: RealmList<SpaceChildSummaryEntity> = RealmList(),
+        var directParentNames: RealmList<String> = RealmList(),
 ) : RealmObject() {
 
     private var displayName: String? = ""

matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/DefaultRoomService.kt

@@ -152,9 +152,8 @@ internal class DefaultRoomService @Inject constructor(
             queryParams: RoomSummaryQueryParams,
             pagedListConfig: PagedList.Config,
             sortOrder: RoomSortOrder,
-            getFlattenParents: Boolean
     ): UpdatableLivePageResult {
-        return roomSummaryDataSource.getUpdatablePagedRoomSummariesLive(queryParams, pagedListConfig, sortOrder, getFlattenParents)
+        return roomSummaryDataSource.getUpdatablePagedRoomSummariesLive(queryParams, pagedListConfig, sortOrder)
     }
 
     override fun getRoomCountLive(queryParams: RoomSummaryQueryParams): LiveData<Int> {

matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/summary/RoomSummaryDataSource.kt

@@ -200,14 +200,13 @@ internal class RoomSummaryDataSource @Inject constructor(
             queryParams: RoomSummaryQueryParams,
             pagedListConfig: PagedList.Config,
             sortOrder: RoomSortOrder,
-            getFlattenedParents: Boolean = false
     ): UpdatableLivePageResult {
         val realmDataSourceFactory = monarchy.createDataSourceFactory { realm ->
             roomSummariesQuery(realm, queryParams).process(sortOrder)
         }
         val dataSourceFactory = realmDataSourceFactory.map {
             roomSummaryMapper.map(it)
-        }.map { if (getFlattenedParents) it.getWithParents() else it }
+        }
 
         val boundaries = MutableLiveData(ResultBoundaries())
 
@@ -246,13 +245,6 @@ internal class RoomSummaryDataSource @Inject constructor(
         }
     }
 
-    private fun RoomSummary.getWithParents(): RoomSummary {
-        val parents = flattenParentIds.mapNotNull { parentId ->
-            getRoomSummary(parentId)
-        }
-        return copy(flattenParents = parents)
-    }
-
     fun getCountLive(queryParams: RoomSummaryQueryParams): LiveData<Int> {
         val liveRooms = monarchy.findAllManagedWithChanges {
             roomSummariesQuery(it, queryParams)

matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/summary/RoomSummaryUpdater.kt

@@ -223,6 +223,7 @@ internal class RoomSummaryUpdater @Inject constructor(
                     .sort(RoomSummaryEntityFields.ROOM_ID)
                     .findAll().map {
                         it.flattenParentIds = null
+                        it.directParentNames.clear()
                         it to emptyList<RoomSummaryEntity>().toMutableSet()
                     }
                     .toMap()
@@ -350,39 +351,29 @@ internal class RoomSummaryUpdater @Inject constructor(
             }
 
             val acyclicGraph = graph.withoutEdges(backEdges)
-//            Timber.v("## SPACES: acyclicGraph $acyclicGraph")
             val flattenSpaceParents = acyclicGraph.flattenDestination().map {
                 it.key.name to it.value.map { it.name }
             }.toMap()
-//            Timber.v("## SPACES: flattenSpaceParents ${flattenSpaceParents.map { it.key.name to it.value.map { it.name } }.joinToString("\n") {
-//                it.first + ": [" + it.second.joinToString(",") + "]"
-//            }}")
-
-//            Timber.v("## SPACES: lookup map ${lookupMap.map { it.key.name to it.value.map { it.name } }.toMap()}")
 
             lookupMap.entries
                     .filter { it.key.roomType == RoomType.SPACE && it.key.membership == Membership.JOIN }
                     .forEach { entry ->
                         val parent = RoomSummaryEntity.where(realm, entry.key.roomId).findFirst()
                         if (parent != null) {
-//                            Timber.v("## SPACES: check hierarchy of ${parent.name} id ${parent.roomId}")
-//                            Timber.v("## SPACES: flat known parents of ${parent.name} are ${flattenSpaceParents[parent.roomId]}")
                             val flattenParentsIds = (flattenSpaceParents[parent.roomId] ?: emptyList()) + listOf(parent.roomId)
-//                            Timber.v("## SPACES: flatten known parents of children of ${parent.name} are ${flattenParentsIds}")
+
                             entry.value.forEach { child ->
                                 RoomSummaryEntity.where(realm, child.roomId).findFirst()?.let { childSum ->
+                                    childSum.directParentNames.add(parent.displayName())
 
-//                                    Timber.w("## SPACES: ${childSum.name} is ${childSum.roomId} fc: ${childSum.flattenParentIds}")
+                                    if (childSum.flattenParentIds == null) {
-//                                    var allParents = childSum.flattenParentIds ?: ""
+                                        childSum.flattenParentIds = ""
-                                    if (childSum.flattenParentIds == null) childSum.flattenParentIds = ""
+                                    }
                                     flattenParentsIds.forEach {
                                         if (childSum.flattenParentIds?.contains(it) != true) {
                                             childSum.flattenParentIds += "|$it"
                                         }
                                     }
-//                                    childSum.flattenParentIds = "$allParents|"
-
-//                                    Timber.v("## SPACES: flatten of ${childSum.name} is ${childSum.flattenParentIds}")
                                 }
                             }
                         }

vector/src/main/java/im/vector/app/features/home/room/list/RoomListSectionBuilder.kt

@@ -331,7 +331,7 @@ class RoomListSectionBuilder(
                 },
                 { queryParams ->
                     val name = stringProvider.getString(R.string.bottom_action_rooms)
-                    val updatableFilterLivePageResult = session.roomService().getFilteredPagedRoomSummariesLive(queryParams, getFlattenParents = true)
+                    val updatableFilterLivePageResult = session.roomService().getFilteredPagedRoomSummariesLive(queryParams)
                     onUpdatable(updatableFilterLivePageResult)
 
                     val itemCountFlow = updatableFilterLivePageResult.livePagedList.asFlow()

vector/src/main/java/im/vector/app/features/home/room/list/RoomSummaryItemFactory.kt

@@ -207,9 +207,18 @@ class RoomSummaryItemFactory @Inject constructor(
 
     private fun getSearchResultSubtitle(roomSummary: RoomSummary): String {
         val userId = roomSummary.directUserId
-        val spaceName = roomSummary.flattenParents.lastOrNull()?.name
+        val directParent = joinParentNames(roomSummary)
         val canonicalAlias = roomSummary.canonicalAlias
 
-        return (userId ?: spaceName ?: canonicalAlias).orEmpty()
+        return (userId ?: directParent ?: canonicalAlias).orEmpty()
+    }
+
+    private fun joinParentNames(roomSummary: RoomSummary) = with(roomSummary) {
+        when (val size = directParentNames.size) {
+            0 -> null
+            1 -> directParentNames.first()
+            2 -> stringProvider.getString(R.string.search_space_two_parents, directParentNames[0], directParentNames[1])
+            else -> stringProvider.getQuantityString(R.plurals.search_space_multiple_parents, size - 1, directParentNames[0], size - 1)
+        }
     }
 }

vector/src/main/res/values/strings.xml

@@ -764,6 +764,11 @@
     <string name="search_members_hint">Filter room members</string>
     <string name="search_banned_user_hint">Filter banned users</string>
     <string name="search_no_results">No results</string>
+    <string name="search_space_two_parents">%1$s and %2$s</string>
+    <plurals name="search_space_multiple_parents">
+        <item quantity="one">%1$s and %2$d other</item>
+        <item quantity="other">%1$s and %2$d others</item>
+    </plurals>
 
     <!-- home room settings -->
     <string name="room_settings_all_messages">All messages</string>