Skip to content

Commit a2cf872

Browse files
ericdecaniniericdecanini
authored
Merge pull request #6314 from vector-im/task/eric/replace_flatten_with_direct_parent
Replace flattenParents with directParentName
2 parents c6fd9f4 + 3f637ea commit a2cf872

File tree

14 files changed

+143
-36
lines changed

14 files changed

+143
-36
lines changed

‎changelog.d/6314.misc

+1
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
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

+78
Original file line numberDiff line numberDiff line change
@@ -610,4 +610,82 @@ class SpaceHierarchyTest : InstrumentedTest {
610610
}
611611
}
612612
}
613+
614+
@Test
615+
fun testDirectParentNames() = runSessionTest(context()) { commonTestHelper ->
616+
val aliceSession = commonTestHelper.createAccount("Alice", SessionTestParams(true))
617+
618+
val spaceAInfo = createPublicSpace(
619+
commonTestHelper,
620+
aliceSession, "SpaceA",
621+
listOf(
622+
Triple("A1", true /*auto-join*/, true/*canonical*/),
623+
Triple("A2", true, true)
624+
)
625+
)
626+
627+
val spaceBInfo = createPublicSpace(
628+
commonTestHelper,
629+
aliceSession, "SpaceB",
630+
listOf(
631+
Triple("B1", true /*auto-join*/, true/*canonical*/),
632+
Triple("B2", true, true),
633+
Triple("B3", true, true)
634+
)
635+
)
636+
637+
// also add B1 in space A
638+
639+
val B1roomId = spaceBInfo.roomIds.first()
640+
val viaServers = listOf(aliceSession.sessionParams.homeServerHost ?: "")
641+
642+
val spaceA = aliceSession.spaceService().getSpace(spaceAInfo.spaceId)
643+
val spaceB = aliceSession.spaceService().getSpace(spaceBInfo.spaceId)
644+
commonTestHelper.runBlockingTest {
645+
spaceA!!.addChildren(B1roomId, viaServers, null, true)
646+
}
647+
648+
commonTestHelper.waitWithLatch { latch ->
649+
commonTestHelper.retryPeriodicallyWithLatch(latch) {
650+
val roomSummary = aliceSession.getRoomSummary(B1roomId)
651+
roomSummary != null &&
652+
roomSummary.directParentNames.size == 2 &&
653+
roomSummary.directParentNames.contains(spaceA!!.spaceSummary()!!.name) &&
654+
roomSummary.directParentNames.contains(spaceB!!.spaceSummary()!!.name)
655+
}
656+
}
657+
658+
commonTestHelper.waitWithLatch { latch ->
659+
commonTestHelper.retryPeriodicallyWithLatch(latch) {
660+
val roomSummary = aliceSession.getRoomSummary(spaceAInfo.roomIds.first())
661+
roomSummary != null &&
662+
roomSummary.directParentNames.size == 1 &&
663+
roomSummary.directParentNames.contains(spaceA!!.spaceSummary()!!.name)
664+
}
665+
}
666+
667+
val newAName = "FooBar"
668+
commonTestHelper.runBlockingTest {
669+
spaceA!!.asRoom().stateService().updateName(newAName)
670+
}
671+
672+
commonTestHelper.waitWithLatch { latch ->
673+
commonTestHelper.retryPeriodicallyWithLatch(latch) {
674+
val roomSummary = aliceSession.getRoomSummary(B1roomId)
675+
roomSummary != null &&
676+
roomSummary.directParentNames.size == 2 &&
677+
roomSummary.directParentNames.contains(newAName) &&
678+
roomSummary.directParentNames.contains(spaceB!!.spaceSummary()!!.name)
679+
}
680+
}
681+
682+
commonTestHelper.waitWithLatch { latch ->
683+
commonTestHelper.retryPeriodicallyWithLatch(latch) {
684+
val roomSummary = aliceSession.getRoomSummary(spaceAInfo.roomIds.first())
685+
roomSummary != null &&
686+
roomSummary.directParentNames.size == 1 &&
687+
roomSummary.directParentNames.contains(newAName)
688+
}
689+
}
690+
}
613691
}

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

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

256253
/**

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

+2-2
Original file line numberDiff line numberDiff line change
@@ -164,9 +164,9 @@ data class RoomSummary(
164164
*/
165165
val spaceChildren: List<SpaceChildInfo>? = null,
166166
/**
167-
* List of all the space parents. Will be empty by default, you have to explicitly request it.
167+
* The names of the room's direct space parents if any.
168168
*/
169-
val flattenParents: List<RoomSummary> = emptyList(),
169+
val directParentNames: List<String> = emptyList(),
170170
/**
171171
* List of all the space parent Ids.
172172
*/

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

+3-1
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@ import org.matrix.android.sdk.internal.database.migration.MigrateSessionTo031
5151
import org.matrix.android.sdk.internal.database.migration.MigrateSessionTo032
5252
import org.matrix.android.sdk.internal.database.migration.MigrateSessionTo033
5353
import org.matrix.android.sdk.internal.database.migration.MigrateSessionTo034
54+
import org.matrix.android.sdk.internal.database.migration.MigrateSessionTo035
5455
import org.matrix.android.sdk.internal.util.Normalizer
5556
import org.matrix.android.sdk.internal.util.database.MatrixRealmMigration
5657
import javax.inject.Inject
@@ -59,7 +60,7 @@ internal class RealmSessionStoreMigration @Inject constructor(
5960
private val normalizer: Normalizer
6061
) : MatrixRealmMigration(
6162
dbName = "Session",
62-
schemaVersion = 34L,
63+
schemaVersion = 35L,
6364
) {
6465
/**
6566
* Forces all RealmSessionStoreMigration instances to be equal.
@@ -103,5 +104,6 @@ internal class RealmSessionStoreMigration @Inject constructor(
103104
if (oldVersion < 32) MigrateSessionTo032(realm).perform()
104105
if (oldVersion < 33) MigrateSessionTo033(realm).perform()
105106
if (oldVersion < 34) MigrateSessionTo034(realm).perform()
107+
if (oldVersion < 35) MigrateSessionTo035(realm).perform()
106108
}
107109
}

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

+1
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,7 @@ internal class RoomSummaryMapper @Inject constructor(
106106
worldReadable = it.childSummaryEntity?.joinRules == RoomJoinRules.PUBLIC
107107
)
108108
},
109+
directParentNames = roomSummaryEntity.directParentNames.toList(),
109110
flattenParentIds = roomSummaryEntity.flattenParentIds?.split("|") ?: emptyList(),
110111
roomEncryptionAlgorithm = when (val alg = roomSummaryEntity.e2eAlgorithm) {
111112
// 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

+31
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
/*
2+
* Copyright (c) 2022 The Matrix.org Foundation C.I.C.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package org.matrix.android.sdk.internal.database.migration
18+
19+
import io.realm.DynamicRealm
20+
import io.realm.RealmList
21+
import org.matrix.android.sdk.internal.database.model.RoomSummaryEntityFields
22+
import org.matrix.android.sdk.internal.util.database.RealmMigrator
23+
24+
internal class MigrateSessionTo035(realm: DynamicRealm) : RealmMigrator(realm, 35) {
25+
26+
override fun doMigrate(realm: DynamicRealm) {
27+
realm.schema.get("RoomSummaryEntity")
28+
?.addRealmListField(RoomSummaryEntityFields.DIRECT_PARENT_NAMES.`$`, String::class.java)
29+
?.transform { it.setList(RoomSummaryEntityFields.DIRECT_PARENT_NAMES.`$`, RealmList("")) }
30+
}
31+
}

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

+2-1
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,8 @@ internal open class RoomSummaryEntity(
3434
@PrimaryKey var roomId: String = "",
3535
var roomType: String? = null,
3636
var parents: RealmList<SpaceParentSummaryEntity> = RealmList(),
37-
var children: RealmList<SpaceChildSummaryEntity> = RealmList()
37+
var children: RealmList<SpaceChildSummaryEntity> = RealmList(),
38+
var directParentNames: RealmList<String> = RealmList(),
3839
) : RealmObject() {
3940

4041
private var displayName: String? = ""

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

+1-2
Original file line numberDiff line numberDiff line change
@@ -152,9 +152,8 @@ internal class DefaultRoomService @Inject constructor(
152152
queryParams: RoomSummaryQueryParams,
153153
pagedListConfig: PagedList.Config,
154154
sortOrder: RoomSortOrder,
155-
getFlattenParents: Boolean
156155
): UpdatableLivePageResult {
157-
return roomSummaryDataSource.getUpdatablePagedRoomSummariesLive(queryParams, pagedListConfig, sortOrder, getFlattenParents)
156+
return roomSummaryDataSource.getUpdatablePagedRoomSummariesLive(queryParams, pagedListConfig, sortOrder)
158157
}
159158

160159
override fun getRoomCountLive(queryParams: RoomSummaryQueryParams): LiveData<Int> {

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

+1-9
Original file line numberDiff line numberDiff line change
@@ -200,14 +200,13 @@ internal class RoomSummaryDataSource @Inject constructor(
200200
queryParams: RoomSummaryQueryParams,
201201
pagedListConfig: PagedList.Config,
202202
sortOrder: RoomSortOrder,
203-
getFlattenedParents: Boolean = false
204203
): UpdatableLivePageResult {
205204
val realmDataSourceFactory = monarchy.createDataSourceFactory { realm ->
206205
roomSummariesQuery(realm, queryParams).process(sortOrder)
207206
}
208207
val dataSourceFactory = realmDataSourceFactory.map {
209208
roomSummaryMapper.map(it)
210-
}.map { if (getFlattenedParents) it.getWithParents() else it }
209+
}
211210

212211
val boundaries = MutableLiveData(ResultBoundaries())
213212

@@ -246,13 +245,6 @@ internal class RoomSummaryDataSource @Inject constructor(
246245
}
247246
}
248247

249-
private fun RoomSummary.getWithParents(): RoomSummary {
250-
val parents = flattenParentIds.mapNotNull { parentId ->
251-
getRoomSummary(parentId)
252-
}
253-
return copy(flattenParents = parents)
254-
}
255-
256248
fun getCountLive(queryParams: RoomSummaryQueryParams): LiveData<Int> {
257249
val liveRooms = monarchy.findAllManagedWithChanges {
258250
roomSummariesQuery(it, queryParams)

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

+6-15
Original file line numberDiff line numberDiff line change
@@ -223,6 +223,7 @@ internal class RoomSummaryUpdater @Inject constructor(
223223
.sort(RoomSummaryEntityFields.ROOM_ID)
224224
.findAll().map {
225225
it.flattenParentIds = null
226+
it.directParentNames.clear()
226227
it to emptyList<RoomSummaryEntity>().toMutableSet()
227228
}
228229
.toMap()
@@ -350,39 +351,29 @@ internal class RoomSummaryUpdater @Inject constructor(
350351
}
351352

352353
val acyclicGraph = graph.withoutEdges(backEdges)
353-
// Timber.v("## SPACES: acyclicGraph $acyclicGraph")
354354
val flattenSpaceParents = acyclicGraph.flattenDestination().map {
355355
it.key.name to it.value.map { it.name }
356356
}.toMap()
357-
// Timber.v("## SPACES: flattenSpaceParents ${flattenSpaceParents.map { it.key.name to it.value.map { it.name } }.joinToString("\n") {
358-
// it.first + ": [" + it.second.joinToString(",") + "]"
359-
// }}")
360-
361-
// Timber.v("## SPACES: lookup map ${lookupMap.map { it.key.name to it.value.map { it.name } }.toMap()}")
362357

363358
lookupMap.entries
364359
.filter { it.key.roomType == RoomType.SPACE && it.key.membership == Membership.JOIN }
365360
.forEach { entry ->
366361
val parent = RoomSummaryEntity.where(realm, entry.key.roomId).findFirst()
367362
if (parent != null) {
368-
// Timber.v("## SPACES: check hierarchy of ${parent.name} id ${parent.roomId}")
369-
// Timber.v("## SPACES: flat known parents of ${parent.name} are ${flattenSpaceParents[parent.roomId]}")
370363
val flattenParentsIds = (flattenSpaceParents[parent.roomId] ?: emptyList()) + listOf(parent.roomId)
371-
// Timber.v("## SPACES: flatten known parents of children of ${parent.name} are ${flattenParentsIds}")
364+
372365
entry.value.forEach { child ->
373366
RoomSummaryEntity.where(realm, child.roomId).findFirst()?.let { childSum ->
367+
childSum.directParentNames.add(parent.displayName())
374368

375-
// Timber.w("## SPACES: ${childSum.name} is ${childSum.roomId} fc: ${childSum.flattenParentIds}")
376-
// var allParents = childSum.flattenParentIds ?: ""
377-
if (childSum.flattenParentIds == null) childSum.flattenParentIds = ""
369+
if (childSum.flattenParentIds == null) {
370+
childSum.flattenParentIds = ""
371+
}
378372
flattenParentsIds.forEach {
379373
if (childSum.flattenParentIds?.contains(it) != true) {
380374
childSum.flattenParentIds += "|$it"
381375
}
382376
}
383-
// childSum.flattenParentIds = "$allParents|"
384-
385-
// Timber.v("## SPACES: flatten of ${childSum.name} is ${childSum.flattenParentIds}")
386377
}
387378
}
388379
}

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

+1-1
Original file line numberDiff line numberDiff line change
@@ -331,7 +331,7 @@ class RoomListSectionBuilder(
331331
},
332332
{ queryParams ->
333333
val name = stringProvider.getString(R.string.bottom_action_rooms)
334-
val updatableFilterLivePageResult = session.roomService().getFilteredPagedRoomSummariesLive(queryParams, getFlattenParents = true)
334+
val updatableFilterLivePageResult = session.roomService().getFilteredPagedRoomSummariesLive(queryParams)
335335
onUpdatable(updatableFilterLivePageResult)
336336

337337
val itemCountFlow = updatableFilterLivePageResult.livePagedList.asFlow()

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

+11-2
Original file line numberDiff line numberDiff line change
@@ -207,9 +207,18 @@ class RoomSummaryItemFactory @Inject constructor(
207207

208208
private fun getSearchResultSubtitle(roomSummary: RoomSummary): String {
209209
val userId = roomSummary.directUserId
210-
val spaceName = roomSummary.flattenParents.lastOrNull()?.name
210+
val directParent = joinParentNames(roomSummary)
211211
val canonicalAlias = roomSummary.canonicalAlias
212212

213-
return (userId ?: spaceName ?: canonicalAlias).orEmpty()
213+
return (userId ?: directParent ?: canonicalAlias).orEmpty()
214+
}
215+
216+
private fun joinParentNames(roomSummary: RoomSummary) = with(roomSummary) {
217+
when (val size = directParentNames.size) {
218+
0 -> null
219+
1 -> directParentNames.first()
220+
2 -> stringProvider.getString(R.string.search_space_two_parents, directParentNames[0], directParentNames[1])
221+
else -> stringProvider.getQuantityString(R.plurals.search_space_multiple_parents, size - 1, directParentNames[0], size - 1)
222+
}
214223
}
215224
}

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

+5
Original file line numberDiff line numberDiff line change
@@ -764,6 +764,11 @@
764764
<string name="search_members_hint">Filter room members</string>
765765
<string name="search_banned_user_hint">Filter banned users</string>
766766
<string name="search_no_results">No results</string>
767+
<string name="search_space_two_parents">%1$s and %2$s</string>
768+
<plurals name="search_space_multiple_parents">
769+
<item quantity="one">%1$s and %2$d other</item>
770+
<item quantity="other">%1$s and %2$d others</item>
771+
</plurals>
767772

768773
<!-- home room settings -->
769774
<string name="room_settings_all_messages">All messages</string>

0 commit comments

Comments
 (0)