Skip to content
Merged
Show file tree
Hide file tree
Changes from 5 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 name of the room's direct space parent if any
*/
val flattenParents: List<RoomSummary> = emptyList(),
val directParentName: String? = null,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Will it be properly updated, if the parent name changes?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Confirmed this one, it properly updates

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@ericdecanini this should be a list, you can have several direct parents.

/**
* List of all the space parent Ids.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ import org.matrix.android.sdk.internal.database.migration.MigrateSessionTo026
import org.matrix.android.sdk.internal.database.migration.MigrateSessionTo027
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.util.Normalizer
import timber.log.Timber
import javax.inject.Inject
Expand All @@ -61,7 +62,7 @@ internal class RealmSessionStoreMigration @Inject constructor(
override fun equals(other: Any?) = other is RealmSessionStoreMigration
override fun hashCode() = 1000

val schemaVersion = 29L
val schemaVersion = 30L

override fun migrate(realm: DynamicRealm, oldVersion: Long, newVersion: Long) {
Timber.d("Migrating Realm Session from $oldVersion to $newVersion")
Expand Down Expand Up @@ -95,5 +96,6 @@ internal class RealmSessionStoreMigration @Inject constructor(
if (oldVersion < 27) MigrateSessionTo027(realm).perform()
if (oldVersion < 28) MigrateSessionTo028(realm).perform()
if (oldVersion < 29) MigrateSessionTo029(realm).perform()
if (oldVersion < 30) MigrateSessionTo030(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
)
},
directParentName = roomSummaryEntity.directParentName,
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 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 org.matrix.android.sdk.internal.database.model.RoomSummaryEntityFields
import org.matrix.android.sdk.internal.util.database.RealmMigrator

internal class MigrateSessionTo030(realm: DynamicRealm) : RealmMigrator(realm, 30) {

override fun doMigrate(realm: DynamicRealm) {
realm.schema.get("RoomSummaryEntity")
?.addField(RoomSummaryEntityFields.DIRECT_PARENT_NAME, String::class.java)
?.transform { it.setString(RoomSummaryEntityFields.DIRECT_PARENT_NAME, "") }

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Even if we migrate it as empty here, it seems like it automatically syncs on launch and updates the value with the corresponding space parent of each room

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

it's updated after each sync. Is there a need to set a default value.

}
}
Original file line number Diff line number Diff line change
Expand Up @@ -235,6 +235,11 @@ internal open class RoomSummaryEntity(
if (value != field) field = value
}

var directParentName: String? = null
set(value) {
if (value != field) field = value
}

var flattenParentIds: String? = null
set(value) {
if (value != field) field = value
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 @@ -366,24 +366,20 @@ internal class RoomSummaryUpdater @Inject constructor(
.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.directParentName = 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.directParentName
val canonicalAlias = roomSummary.canonicalAlias

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