From 2b667ac8d016a0e5db4bb593200ef6638910ba81 Mon Sep 17 00:00:00 2001 From: SpiritCroc Date: Mon, 22 Jan 2024 14:38:43 +0100 Subject: [PATCH] Respect child space order Change-Id: Ibf350b03ff01b9ba31898d25cc7e4dab33bcde07 --- .../impl/datasource/SpaceListDataSource.kt | 31 ++++++++++--------- .../datasource/TopLevelSpaceComparator.kt | 4 ++- gradle/libs.versions.toml | 2 +- .../libraries/matrix/api/room/MatrixRoom.kt | 2 +- .../matrix/api/room/MatrixSpaceChildInfo.kt | 10 ++++++ .../impl/room/MatrixSpaceChildInfoMapper.kt | 14 +++++++++ .../matrix/impl/room/RustMatrixRoom.kt | 5 +-- .../matrix/test/room/FakeMatrixRoom.kt | 3 +- 8 files changed, 51 insertions(+), 20 deletions(-) create mode 100644 libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/room/MatrixSpaceChildInfo.kt create mode 100644 libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/room/MatrixSpaceChildInfoMapper.kt diff --git a/features/roomlist/impl/src/main/kotlin/io/element/android/features/roomlist/impl/datasource/SpaceListDataSource.kt b/features/roomlist/impl/src/main/kotlin/io/element/android/features/roomlist/impl/datasource/SpaceListDataSource.kt index ef1c2920e6..3a90edac0c 100644 --- a/features/roomlist/impl/src/main/kotlin/io/element/android/features/roomlist/impl/datasource/SpaceListDataSource.kt +++ b/features/roomlist/impl/src/main/kotlin/io/element/android/features/roomlist/impl/datasource/SpaceListDataSource.kt @@ -30,6 +30,7 @@ import io.element.android.libraries.designsystem.components.avatar.AvatarSize import io.element.android.libraries.eventformatter.api.RoomLastMessageFormatter import io.element.android.libraries.matrix.api.MatrixClient import io.element.android.libraries.matrix.api.core.RoomId +import io.element.android.libraries.matrix.api.room.MatrixSpaceChildInfo import io.element.android.libraries.matrix.api.roomlist.RoomListService import io.element.android.libraries.matrix.api.roomlist.RoomSummary import kotlinx.collections.immutable.ImmutableList @@ -113,22 +114,24 @@ class SpaceListDataSource @Inject constructor( // TODO what can we cache something here? private suspend fun buildSpaceHierarchy(spaceSummaries: List): ImmutableList { // Map spaceId -> list of child spaces - val spaceHierarchyMap = HashMap>() + val spaceHierarchyMap = HashMap>>() // Map spaceId -> list of regular child rooms - val regularChildren = HashMap>() + val regularChildren = HashMap>() val rootSpaces = HashSet(spaceSummaries) spaceSummaries.forEach { parentSpace -> val spaceInfo = client.getRoom(parentSpace.roomId) val spaceChildren = spaceInfo?.spaceChildren - spaceChildren?.forEach childLoop@{ childId -> - val child = spaceSummaries.find { it.roomId.value == childId } + spaceChildren?.forEach childLoop@{ spaceChild -> + val child = spaceSummaries.find { it.roomId.value == spaceChild.roomId } if (child == null) { // Treat as regular child, since it doesn't appear to be a space (at least none known to us at this point) - regularChildren[parentSpace.roomId.value] = regularChildren[parentSpace.roomId.value]?.apply { add(childId) } ?: mutableListOf(childId) + regularChildren[parentSpace.roomId.value] = regularChildren[parentSpace.roomId.value]?.apply { add(spaceChild) } ?: mutableListOf(spaceChild) return@childLoop } - rootSpaces.removeAll { it.roomId.value == childId } - spaceHierarchyMap[parentSpace.roomId.value] = spaceHierarchyMap[parentSpace.roomId.value]?.apply { add(child) } ?: mutableListOf(child) + rootSpaces.removeAll { it.roomId.value == spaceChild.roomId } + spaceHierarchyMap[parentSpace.roomId.value] = spaceHierarchyMap[parentSpace.roomId.value]?.apply { + add(Pair(spaceChild, child)) + } ?: mutableListOf(Pair(spaceChild, child)) } } @@ -142,16 +145,16 @@ class SpaceListDataSource @Inject constructor( private fun createSpaceHierarchyItem( spaceSummary: RoomListRoomSummary, order: String?, - hierarchy: HashMap>, - regularChildren: HashMap>, + hierarchy: HashMap>>, + regularChildren: HashMap>, forbiddenChildren: List = emptyList(), ): SpaceHierarchyItem { - val children = hierarchy[spaceSummary.id]?.mapNotNull { - if (it.roomId.value in forbiddenChildren) { - Timber.w("Detected space loop: ${spaceSummary.id} -> ${it.roomId.value}") + val children = hierarchy[spaceSummary.id]?.mapNotNull { (spaceChildInfo, child) -> + if (child.roomId.value in forbiddenChildren) { + Timber.w("Detected space loop: ${spaceSummary.id} -> ${child.roomId.value}") null } else { - createSpaceHierarchyItem(it, null, hierarchy, regularChildren, forbiddenChildren + listOf(spaceSummary.roomId.value)) + createSpaceHierarchyItem(child, spaceChildInfo.order, hierarchy, regularChildren, forbiddenChildren + listOf(spaceSummary.roomId.value)) } }?.sortedWith(SpaceComparator)?.toImmutableList() ?: persistentListOf() return SpaceHierarchyItem( @@ -160,7 +163,7 @@ class SpaceListDataSource @Inject constructor( spaces = children, flattenedRooms = ( // All direct children rooms - regularChildren[spaceSummary.id].orEmpty() + regularChildren[spaceSummary.id].orEmpty().map { it.roomId } // All indirect children rooms + children.flatMap { it.flattenedRooms } ).toImmutableList(), diff --git a/features/roomlist/impl/src/main/kotlin/io/element/android/features/roomlist/impl/datasource/TopLevelSpaceComparator.kt b/features/roomlist/impl/src/main/kotlin/io/element/android/features/roomlist/impl/datasource/TopLevelSpaceComparator.kt index 63c9c5b2e2..906027daff 100644 --- a/features/roomlist/impl/src/main/kotlin/io/element/android/features/roomlist/impl/datasource/TopLevelSpaceComparator.kt +++ b/features/roomlist/impl/src/main/kotlin/io/element/android/features/roomlist/impl/datasource/TopLevelSpaceComparator.kt @@ -27,7 +27,9 @@ object SpaceComparator : Comparator { } else { if (leftOrder == null) { if (rightOrder == null) { - compareValues(left?.info?.name, right?.info?.name) + // Spec says to fallback to roomId, but we at SchildiChat find lowercase names more suitable + //compareValues(left?.info?.roomId?.value, right?.info?.roomId?.value) + compareValues(left?.info?.name?.lowercase(), right?.info?.name?.lowercase()) } else { 1 } diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index 889d94bd53..461c491de5 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -152,7 +152,7 @@ jsoup = "org.jsoup:jsoup:1.17.2" appyx_core = { module = "com.bumble.appyx:core", version.ref = "appyx" } molecule-runtime = "app.cash.molecule:molecule-runtime:1.3.2" timber = "com.jakewharton.timber:timber:5.0.1" -matrix_sdk = "chat.schildi.rustcomponents:sdk-android:0.2.1" +matrix_sdk = "chat.schildi.rustcomponents:sdk-android:0.2.2" matrix_richtexteditor = { module = "io.element.android:wysiwyg", version.ref = "wysiwyg" } matrix_richtexteditor_compose = { module = "io.element.android:wysiwyg-compose", version.ref = "wysiwyg" } sqldelight-driver-android = { module = "app.cash.sqldelight:android-driver", version.ref = "sqldelight" } diff --git a/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/room/MatrixRoom.kt b/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/room/MatrixRoom.kt index 9d05fe85f2..295e103804 100644 --- a/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/room/MatrixRoom.kt +++ b/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/room/MatrixRoom.kt @@ -49,7 +49,7 @@ interface MatrixRoom : Closeable { val isEncrypted: Boolean val isDirect: Boolean val isPublic: Boolean - val spaceChildren: List + val spaceChildren: List val rootSpaceOrder: String? val activeMemberCount: Long val joinedMemberCount: Long diff --git a/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/room/MatrixSpaceChildInfo.kt b/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/room/MatrixSpaceChildInfo.kt new file mode 100644 index 0000000000..c6ff2daec2 --- /dev/null +++ b/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/room/MatrixSpaceChildInfo.kt @@ -0,0 +1,10 @@ +package io.element.android.libraries.matrix.api.room + +import androidx.compose.runtime.Immutable + +@Immutable +data class MatrixSpaceChildInfo( + val roomId: String, + val order: String?, + val suggested: Boolean, +) diff --git a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/room/MatrixSpaceChildInfoMapper.kt b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/room/MatrixSpaceChildInfoMapper.kt new file mode 100644 index 0000000000..32a0bc95ec --- /dev/null +++ b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/room/MatrixSpaceChildInfoMapper.kt @@ -0,0 +1,14 @@ +package io.element.android.libraries.matrix.impl.room + +import io.element.android.libraries.matrix.api.room.MatrixSpaceChildInfo +import org.matrix.rustcomponents.sdk.SpaceChildInfo + +object MatrixSpaceChildInfoMapper { + fun map(spaceChildInfo: SpaceChildInfo): MatrixSpaceChildInfo = spaceChildInfo.let { + return MatrixSpaceChildInfo( + roomId = it.roomId, + order = it.order, + suggested = it.suggested, + ) + } +} diff --git a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/room/RustMatrixRoom.kt b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/room/RustMatrixRoom.kt index 3005fd32e0..6f7507b1c0 100644 --- a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/room/RustMatrixRoom.kt +++ b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/room/RustMatrixRoom.kt @@ -37,6 +37,7 @@ import io.element.android.libraries.matrix.api.room.MatrixRoom import io.element.android.libraries.matrix.api.room.MatrixRoomInfo import io.element.android.libraries.matrix.api.room.MatrixRoomMembersState import io.element.android.libraries.matrix.api.room.MatrixRoomNotificationSettingsState +import io.element.android.libraries.matrix.api.room.MatrixSpaceChildInfo import io.element.android.libraries.matrix.api.room.Mention import io.element.android.libraries.matrix.api.room.MessageEventType import io.element.android.libraries.matrix.api.room.StateEventType @@ -185,8 +186,8 @@ class RustMatrixRoom( override val isPublic: Boolean get() = innerRoom.isPublic() - override val spaceChildren: List - get() = innerRoom.spaceChildren() + override val spaceChildren: List + get() = innerRoom.spaceChildren().map(MatrixSpaceChildInfoMapper::map) override val isDirect: Boolean get() = innerRoom.isDirect() diff --git a/libraries/matrix/test/src/main/kotlin/io/element/android/libraries/matrix/test/room/FakeMatrixRoom.kt b/libraries/matrix/test/src/main/kotlin/io/element/android/libraries/matrix/test/room/FakeMatrixRoom.kt index b7039a6ee4..885243038a 100644 --- a/libraries/matrix/test/src/main/kotlin/io/element/android/libraries/matrix/test/room/FakeMatrixRoom.kt +++ b/libraries/matrix/test/src/main/kotlin/io/element/android/libraries/matrix/test/room/FakeMatrixRoom.kt @@ -34,6 +34,7 @@ import io.element.android.libraries.matrix.api.room.MatrixRoom import io.element.android.libraries.matrix.api.room.MatrixRoomInfo import io.element.android.libraries.matrix.api.room.MatrixRoomMembersState import io.element.android.libraries.matrix.api.room.MatrixRoomNotificationSettingsState +import io.element.android.libraries.matrix.api.room.MatrixSpaceChildInfo import io.element.android.libraries.matrix.api.room.Mention import io.element.android.libraries.matrix.api.room.MessageEventType import io.element.android.libraries.matrix.api.room.RoomMember @@ -72,7 +73,7 @@ class FakeMatrixRoom( override val alias: String? = null, override val alternativeAliases: List = emptyList(), override val isPublic: Boolean = true, - override val spaceChildren: List = emptyList(), + override val spaceChildren: List = emptyList(), override val rootSpaceOrder: String? = null, override val isDirect: Boolean = false, override val isOneToOne: Boolean = false,