Skip to content

Replace flattenParents with directParentName #6314

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 32 commits into from
Jul 20, 2022
Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
385720b
Replaces flatten parents with direct parent name in RoomSummary
ericdecanini Jun 15, 2022
d5c5c57
Merge remote-tracking branch 'origin/develop' into task/eric/replace_…
ericdecanini Jun 15, 2022
d40c2f8
Fixes migration error
ericdecanini Jun 15, 2022
7f3325e
Fixes wrong user id in search room summary item
ericdecanini Jun 15, 2022
b5fc0b5
Adds changelog file
ericdecanini Jun 15, 2022
9766b62
Fixes lint error
ericdecanini Jun 15, 2022
9350053
Merge remote-tracking branch 'origin/develop' into task/eric/replace_…
ericdecanini Jun 29, 2022
cb400b6
Fixes post merge error
ericdecanini Jun 29, 2022
ed3a201
Changes directParentName to a list
ericdecanini Jul 1, 2022
b437996
Merge remote-tracking branch 'origin/develop' into task/eric/replace_…
ericdecanini Jul 1, 2022
e003bc9
Adds clearing to directParentNames
ericdecanini Jul 4, 2022
98d195c
Changes migration to be realm list field
ericdecanini Jul 5, 2022
c6728dd
Changes string format of space parents to maintain parity with web
ericdecanini Jul 6, 2022
051f925
Adds MigrateSessionTo032Test
ericdecanini Jul 7, 2022
13b9b5b
Changes string into plural
ericdecanini Jul 18, 2022
1ae5965
Fixes import order
ericdecanini Jul 18, 2022
67dd250
Merge remote-tracking branch 'origin/develop' into task/eric/replace_…
ericdecanini Jul 18, 2022
c73c36e
Merge remote-tracking branch 'origin/develop' into task/eric/replace_…
ericdecanini Jul 18, 2022
7d29fbc
Fixes post merge errors
ericdecanini Jul 18, 2022
7bb7a08
Fixes copyright lint error
ericdecanini Jul 18, 2022
e6475a3
Fixes copyright lint error
ericdecanini Jul 18, 2022
bc5c53b
Fixes copyright lint error
ericdecanini Jul 18, 2022
e926871
Merge remote-tracking branch 'origin/develop' into task/eric/replace_…
ericdecanini Jul 19, 2022
6f9b5c0
Deletes migration test
ericdecanini Jul 19, 2022
f9580e6
Fixes error in RealmSessionStoreMigration
ericdecanini Jul 19, 2022
ee941cf
Merge remote-tracking branch 'origin/develop' into task/eric/replace_…
ericdecanini Jul 20, 2022
5f6f16d
Adds migrate session to 034
ericdecanini Jul 20, 2022
10b1443
add quick test for direct parent name
BillCarsonFr Jul 20, 2022
0ad2051
Improves plural used in strings
ericdecanini Jul 20, 2022
9e1235d
Merge remote-tracking branch 'origin/develop' into task/eric/replace_…
ericdecanini Jul 20, 2022
b89df0b
Adds MigrateSessionTo035
ericdecanini Jul 20, 2022
3f637ea
Fixes migration crash
ericdecanini Jul 20, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.d/6314.misc
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Improves performance on search screen by replacing flattenParents with directParentName in RoomSummary
Original file line number Diff line number Diff line change
Expand Up @@ -231,14 +231,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

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ import org.matrix.android.sdk.internal.database.migration.MigrateSessionTo028
import org.matrix.android.sdk.internal.database.migration.MigrateSessionTo029
import org.matrix.android.sdk.internal.database.migration.MigrateSessionTo030
import org.matrix.android.sdk.internal.database.migration.MigrateSessionTo031
import org.matrix.android.sdk.internal.database.migration.MigrateSessionTo032
import org.matrix.android.sdk.internal.util.Normalizer
import timber.log.Timber
import javax.inject.Inject
Expand All @@ -63,7 +64,7 @@ internal class RealmSessionStoreMigration @Inject constructor(
override fun equals(other: Any?) = other is RealmSessionStoreMigration
override fun hashCode() = 1000

val schemaVersion = 31L
val schemaVersion = 32L

override fun migrate(realm: DynamicRealm, oldVersion: Long, newVersion: Long) {
Timber.d("Migrating Realm Session from $oldVersion to $newVersion")
Expand Down Expand Up @@ -99,5 +100,6 @@ internal class RealmSessionStoreMigration @Inject constructor(
if (oldVersion < 29) MigrateSessionTo029(realm).perform()
if (oldVersion < 30) MigrateSessionTo030(realm).perform()
if (oldVersion < 31) MigrateSessionTo031(realm).perform()
if (oldVersion < 32) MigrateSessionTo032(realm).perform()
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
/*
* Copyright (c) 2022 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 org.matrix.android.sdk.internal.database.migration

import io.realm.DynamicRealm
import org.matrix.android.sdk.internal.database.model.RoomSummaryEntityFields
import org.matrix.android.sdk.internal.util.database.RealmMigrator

internal class MigrateSessionTo032(realm: DynamicRealm) : RealmMigrator(realm, 32) {

override fun doMigrate(realm: DynamicRealm) {
realm.schema.get("RoomSummaryEntity")
?.addRealmListField(RoomSummaryEntityFields.DIRECT_PARENT_NAMES.`$`, String::class.java)
?.transform { it.setString(RoomSummaryEntityFields.DIRECT_PARENT_NAMES.`$`, "") }
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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? = ""
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -140,9 +140,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> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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())

Expand Down Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,7 @@ internal class RoomSummaryUpdater @Inject constructor(
.sort(RoomSummaryEntityFields.ROOM_ID)
.findAll().map {
it.flattenParentIds = null
it.directParentNames.clear()
it to emptyList<RoomSummaryEntity>().toMutableSet()
}
.toMap()
Expand Down Expand Up @@ -351,39 +352,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}")
// var allParents = childSum.flattenParentIds ?: ""
if (childSum.flattenParentIds == null) 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}")
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ class RoomListSectionBuilderGroup(
},
{ qpm ->
val name = stringProvider.getString(R.string.bottom_action_rooms)
val updatableFilterLivePageResult = session.roomService().getFilteredPagedRoomSummariesLive(qpm, getFlattenParents = true)
val updatableFilterLivePageResult = session.roomService().getFilteredPagedRoomSummariesLive(qpm)
onUpdatable(updatableFilterLivePageResult)

val itemCountFlow = updatableFilterLivePageResult.livePagedList.asFlow()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -332,7 +332,7 @@ class RoomListSectionBuilderSpace(
},
{ 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()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -207,9 +207,9 @@ class RoomSummaryItemFactory @Inject constructor(

private fun getSearchResultSubtitle(roomSummary: RoomSummary): String {
val userId = roomSummary.directUserId
val spaceName = roomSummary.flattenParents.lastOrNull()?.name
val directParent = roomSummary.directParentNames.takeIf { it.isNotEmpty() }?.joinToString()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

val canonicalAlias = roomSummary.canonicalAlias

return (userId ?: spaceName ?: canonicalAlias).orEmpty()
return (userId ?: directParent ?: canonicalAlias).orEmpty()
}
}