Merge pull request #5443 from vector-im/task/eric/stable-hierarchy-endpoint

Changes room hierarchy endpoint to stable

changelog.d/5443.misc

@@ -0,0 +1 @@
+Adds stable room hierarchy endpoint with a fallback to the unstable one

matrix-sdk-android/build.gradle

@@ -174,7 +174,7 @@ dependencies {
     // Note: version sticks to 1.9.2 due to https://github.com/mockk/mockk/issues/281
     testImplementation libs.mockk.mockk
     testImplementation libs.tests.kluent
-    implementation libs.jetbrains.coroutinesAndroid
+    testImplementation libs.jetbrains.coroutinesTest
     // Plant Timber tree for test
     testImplementation 'net.lachlanmckee:timber-junit-rule:1.0.1'
     // Transitively required for mocking realm as monarchy doesn't expose Rx

matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/network/NetworkConstants.kt

@@ -21,6 +21,7 @@ internal object NetworkConstants {
     private const val URI_API_PREFIX_PATH = "_matrix/client"
     const val URI_API_PREFIX_PATH_ = "$URI_API_PREFIX_PATH/"
     const val URI_API_PREFIX_PATH_R0 = "$URI_API_PREFIX_PATH/r0/"
+    const val URI_API_PREFIX_PATH_V1 = "$URI_API_PREFIX_PATH/v1/"
     const val URI_API_PREFIX_PATH_UNSTABLE = "$URI_API_PREFIX_PATH/unstable/"
 
     // Media

matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/space/DefaultSpaceService.kt

@@ -113,71 +113,108 @@ internal class DefaultSpaceService @Inject constructor(
         return peekSpaceTask.execute(PeekSpaceTask.Params(spaceId))
     }
 
-    override suspend fun querySpaceChildren(spaceId: String,
+    override suspend fun querySpaceChildren(
-                                            suggestedOnly: Boolean?,
+            spaceId: String,
-                                            limit: Int?,
+            suggestedOnly: Boolean?,
-                                            from: String?,
+            limit: Int?,
-                                            knownStateList: List<Event>?): SpaceHierarchyData {
+            from: String?,
-        return resolveSpaceInfoTask.execute(
+            knownStateList: List<Event>?
-                ResolveSpaceInfoTask.Params(
+    ): SpaceHierarchyData {
-                        spaceId = spaceId, limit = limit, maxDepth = 1, from = from, suggestedOnly = suggestedOnly
+        val spacesResponse = getSpacesResponse(spaceId, suggestedOnly, limit, from)
-                )
+        val spaceRootResponse = spacesResponse.getRoot(spaceId)
-        ).let { response ->
+        val spaceRoot = spaceRootResponse?.toRoomSummary() ?: createBlankRoomSummary(spaceId)
-            val spaceDesc = response.rooms?.firstOrNull { it.roomId == spaceId }
+        val spaceChildren = spacesResponse.rooms.mapSpaceChildren(spaceId, spaceRootResponse, knownStateList)
-            val root = RoomSummary(
+
-                    roomId = spaceDesc?.roomId ?: spaceId,
+        return SpaceHierarchyData(
-                    roomType = spaceDesc?.roomType,
+                rootSummary = spaceRoot,
-                    name = spaceDesc?.name ?: "",
+                children = spaceChildren,
-                    displayName = spaceDesc?.name ?: "",
+                childrenState = spaceRootResponse?.childrenState.orEmpty(),
-                    topic = spaceDesc?.topic ?: "",
+                nextToken = spacesResponse.nextBatch
-                    joinedMembersCount = spaceDesc?.numJoinedMembers,
+        )
-                    avatarUrl = spaceDesc?.avatarUrl ?: "",
-                    encryptionEventTs = null,
-                    typingUsers = emptyList(),
-                    isEncrypted = false,
-                    flattenParentIds = emptyList(),
-                    canonicalAlias = spaceDesc?.canonicalAlias,
-                    joinRules = RoomJoinRules.PUBLIC.takeIf { spaceDesc?.worldReadable == true }
-            )
-            val children = response.rooms
-                    ?.filter { it.roomId != spaceId }
-                    ?.flatMap { childSummary ->
-                        (spaceDesc?.childrenState ?: knownStateList)
-                                ?.filter { it.stateKey == childSummary.roomId && it.type == EventType.STATE_SPACE_CHILD }
-                                ?.mapNotNull { childStateEv ->
-                                    // create a child entry for everytime this room is the child of a space
-                                    // beware that a room could appear then twice in this list
-                                    childStateEv.content.toModel<SpaceChildContent>()?.let { childStateEvContent ->
-                                        SpaceChildInfo(
-                                                childRoomId = childSummary.roomId,
-                                                isKnown = true,
-                                                roomType = childSummary.roomType,
-                                                name = childSummary.name,
-                                                topic = childSummary.topic,
-                                                avatarUrl = childSummary.avatarUrl,
-                                                order = childStateEvContent.order,
-//                                                        autoJoin = childStateEvContent.autoJoin ?: false,
-                                                viaServers = childStateEvContent.via.orEmpty(),
-                                                activeMemberCount = childSummary.numJoinedMembers,
-                                                parentRoomId = childStateEv.roomId,
-                                                suggested = childStateEvContent.suggested,
-                                                canonicalAlias = childSummary.canonicalAlias,
-                                                aliases = childSummary.aliases,
-                                                worldReadable = childSummary.worldReadable
-                                        )
-                                    }
-                                }.orEmpty()
-                    }
-                    .orEmpty()
-            SpaceHierarchyData(
-                    rootSummary = root,
-                    children = children,
-                    childrenState = spaceDesc?.childrenState.orEmpty(),
-                    nextToken = response.nextBatch
-            )
-        }
     }
 
+    private suspend fun getSpacesResponse(spaceId: String, suggestedOnly: Boolean?, limit: Int?, from: String?) =
+            resolveSpaceInfoTask.execute(
+                    ResolveSpaceInfoTask.Params(spaceId = spaceId, limit = limit, maxDepth = 1, from = from, suggestedOnly = suggestedOnly)
+            )
+
+    private fun SpacesResponse.getRoot(spaceId: String) = rooms?.firstOrNull { it.roomId == spaceId }
+
+    private fun SpaceChildSummaryResponse.toRoomSummary() = RoomSummary(
+            roomId = roomId,
+            roomType = roomType,
+            name = name ?: "",
+            displayName = name ?: "",
+            topic = topic ?: "",
+            joinedMembersCount = numJoinedMembers,
+            avatarUrl = avatarUrl ?: "",
+            encryptionEventTs = null,
+            typingUsers = emptyList(),
+            isEncrypted = false,
+            flattenParentIds = emptyList(),
+            canonicalAlias = canonicalAlias,
+            joinRules = RoomJoinRules.PUBLIC.takeIf { isWorldReadable }
+    )
+
+    private fun createBlankRoomSummary(spaceId: String) = RoomSummary(
+            roomId = spaceId,
+            joinedMembersCount = null,
+            encryptionEventTs = null,
+            typingUsers = emptyList(),
+            isEncrypted = false,
+            flattenParentIds = emptyList(),
+            canonicalAlias = null,
+            joinRules = null
+    )
+
+    private fun List<SpaceChildSummaryResponse>?.mapSpaceChildren(
+            spaceId: String,
+            spaceRootResponse: SpaceChildSummaryResponse?,
+            knownStateList: List<Event>?,
+    ) = this?.filterIdIsNot(spaceId)
+            ?.toSpaceChildInfoList(spaceId, spaceRootResponse, knownStateList)
+            .orEmpty()
+
+    private fun List<SpaceChildSummaryResponse>.filterIdIsNot(spaceId: String) = filter { it.roomId != spaceId }
+
+    private fun List<SpaceChildSummaryResponse>.toSpaceChildInfoList(
+            spaceId: String,
+            rootRoomResponse: SpaceChildSummaryResponse?,
+            knownStateList: List<Event>?,
+    ) = flatMap { spaceChildSummary ->
+        (rootRoomResponse?.childrenState ?: knownStateList)
+                ?.filter { it.isChildOf(spaceChildSummary) }
+                ?.mapNotNull { childStateEvent -> childStateEvent.toSpaceChildInfo(spaceId, spaceChildSummary) }
+                .orEmpty()
+    }
+
+    private fun Event.isChildOf(space: SpaceChildSummaryResponse) = stateKey == space.roomId && type == EventType.STATE_SPACE_CHILD
+
+    private fun Event.toSpaceChildInfo(spaceId: String, summary: SpaceChildSummaryResponse) = content.toModel<SpaceChildContent>()?.let { content ->
+        createSpaceChildInfo(spaceId, summary, content)
+    }
+
+    private fun createSpaceChildInfo(
+            spaceId: String,
+            summary: SpaceChildSummaryResponse,
+            content: SpaceChildContent
+    ) = SpaceChildInfo(
+            childRoomId = summary.roomId,
+            isKnown = true,
+            roomType = summary.roomType,
+            name = summary.name,
+            topic = summary.topic,
+            avatarUrl = summary.avatarUrl,
+            order = content.order,
+            viaServers = content.via.orEmpty(),
+            activeMemberCount = summary.numJoinedMembers,
+            parentRoomId = spaceId,
+            suggested = content.suggested,
+            canonicalAlias = summary.canonicalAlias,
+            aliases = summary.aliases,
+            worldReadable = summary.isWorldReadable
+    )
+
     override suspend fun joinSpace(spaceIdOrAlias: String,
                                    reason: String?,
                                    viaServers: List<String>): JoinSpaceResult {
@@ -192,10 +229,6 @@ internal class DefaultSpaceService @Inject constructor(
         leaveRoomTask.execute(LeaveRoomTask.Params(spaceId, reason))
     }
 
-//    override fun getSpaceParentsOfRoom(roomId: String): List<SpaceSummary> {
-//        return spaceSummaryDataSource.getParentsOfRoom(roomId)
-//    }
-
     override suspend fun setSpaceParent(childRoomId: String, parentSpaceId: String, canonical: Boolean, viaServers: List<String>) {
         // Should we perform some validation here?,
         // and if client want to bypass, it could use sendStateEvent directly?

matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/space/ResolveSpaceInfoTask.kt

@@ -19,6 +19,7 @@ package org.matrix.android.sdk.internal.session.space
 import org.matrix.android.sdk.internal.network.GlobalErrorReceiver
 import org.matrix.android.sdk.internal.network.executeRequest
 import org.matrix.android.sdk.internal.task.Task
+import retrofit2.HttpException
 import javax.inject.Inject
 
 internal interface ResolveSpaceInfoTask : Task<ResolveSpaceInfoTask.Params, SpacesResponse> {
@@ -28,7 +29,6 @@ internal interface ResolveSpaceInfoTask : Task<ResolveSpaceInfoTask.Params, Spac
             val maxDepth: Int?,
             val from: String?,
             val suggestedOnly: Boolean?
-//            val autoJoinOnly: Boolean?
     )
 }
 
@@ -36,14 +36,30 @@ internal class DefaultResolveSpaceInfoTask @Inject constructor(
         private val spaceApi: SpaceApi,
         private val globalErrorReceiver: GlobalErrorReceiver
 ) : ResolveSpaceInfoTask {
-    override suspend fun execute(params: ResolveSpaceInfoTask.Params): SpacesResponse {
+
-        return executeRequest(globalErrorReceiver) {
+    override suspend fun execute(params: ResolveSpaceInfoTask.Params) = executeRequest(globalErrorReceiver) {
+        try {
+            getSpaceHierarchy(params)
+        } catch (e: HttpException) {
+            getUnstableSpaceHierarchy(params)
+        }
+    }
+
+    private suspend fun getSpaceHierarchy(params: ResolveSpaceInfoTask.Params) =
             spaceApi.getSpaceHierarchy(
                     spaceId = params.spaceId,
                     suggestedOnly = params.suggestedOnly,
                     limit = params.limit,
                     maxDepth = params.maxDepth,
-                    from = params.from)
+                    from = params.from,
-        }
+            )
-    }
+
+    private suspend fun getUnstableSpaceHierarchy(params: ResolveSpaceInfoTask.Params) =
+            spaceApi.getSpaceHierarchyUnstable(
+                    spaceId = params.spaceId,
+                    suggestedOnly = params.suggestedOnly,
+                    limit = params.limit,
+                    maxDepth = params.maxDepth,
+                    from = params.from,
+            )
 }

matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/space/SpaceApi.kt

@@ -31,11 +31,22 @@ internal interface SpaceApi {
      * @param from: Optional. Pagination token given to retrieve the next set of rooms.
      * Note that if a pagination token is provided, then the parameters given for suggested_only and max_depth must be the same.
      */
-    @GET(NetworkConstants.URI_API_PREFIX_PATH_UNSTABLE + "org.matrix.msc2946/rooms/{roomId}/hierarchy")
+    @GET(NetworkConstants.URI_API_PREFIX_PATH_V1 + "rooms/{roomId}/hierarchy")
     suspend fun getSpaceHierarchy(
             @Path("roomId") spaceId: String,
             @Query("suggested_only") suggestedOnly: Boolean?,
             @Query("limit") limit: Int?,
             @Query("max_depth") maxDepth: Int?,
             @Query("from") from: String?): SpacesResponse
+
+    /**
+     * Unstable version of [getSpaceHierarchy]
+     */
+    @GET(NetworkConstants.URI_API_PREFIX_PATH_UNSTABLE + "org.matrix.msc2946/rooms/{roomId}/hierarchy")
+    suspend fun getSpaceHierarchyUnstable(
+            @Path("roomId") spaceId: String,
+            @Query("suggested_only") suggestedOnly: Boolean?,
+            @Query("limit") limit: Int?,
+            @Query("max_depth") maxDepth: Int?,
+            @Query("from") from: String?): SpacesResponse
 }

matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/space/SpaceChildSummaryResponse.kt

@@ -81,7 +81,7 @@ internal data class SpaceChildSummaryResponse(
          * Required. Whether the room may be viewed by guest users without joining.
          */
         @Json(name = "world_readable")
-        val worldReadable: Boolean = false,
+        val isWorldReadable: Boolean = false,
 
         /**
          * Required. Whether guest users may join the room and participate in it. If they can,

matrix-sdk-android/src/test/java/org/matrix/android/sdk/internal/session/space/DefaultResolveSpaceInfoTaskTest.kt

@@ -0,0 +1,60 @@
+/*
+ * Copyright 2021 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.session.space
+
+import kotlinx.coroutines.ExperimentalCoroutinesApi
+import kotlinx.coroutines.test.runBlockingTest
+import okhttp3.ResponseBody.Companion.toResponseBody
+import org.amshove.kluent.shouldBeEqualTo
+import org.junit.Test
+import org.matrix.android.sdk.test.fakes.FakeGlobalErrorReceiver
+import org.matrix.android.sdk.test.fakes.FakeSpaceApi
+import org.matrix.android.sdk.test.fixtures.SpacesResponseFixture.aSpacesResponse
+import retrofit2.HttpException
+import retrofit2.Response
+
+@ExperimentalCoroutinesApi
+internal class DefaultResolveSpaceInfoTaskTest {
+
+    private val spaceApi = FakeSpaceApi()
+    private val globalErrorReceiver = FakeGlobalErrorReceiver()
+    private val resolveSpaceInfoTask = DefaultResolveSpaceInfoTask(spaceApi.instance, globalErrorReceiver)
+
+    @Test
+    fun `given stable endpoint works, when execute, then return stable api data`() = runBlockingTest {
+        spaceApi.givenStableEndpointReturns(response)
+
+        val result = resolveSpaceInfoTask.execute(spaceApi.params)
+
+        result shouldBeEqualTo response
+    }
+
+    @Test
+    fun `given stable endpoint fails, when execute, then fallback to unstable endpoint`() = runBlockingTest {
+        spaceApi.givenStableEndpointThrows(httpException)
+        spaceApi.givenUnstableEndpointReturns(response)
+
+        val result = resolveSpaceInfoTask.execute(spaceApi.params)
+
+        result shouldBeEqualTo response
+    }
+
+    companion object {
+        private val response = aSpacesResponse()
+        private val httpException = HttpException(Response.error<SpacesResponse>(500, "".toResponseBody()))
+    }
+}

matrix-sdk-android/src/test/java/org/matrix/android/sdk/test/fakes/FakeSpaceApi.kt

@@ -0,0 +1,41 @@
+/*
+ * Copyright 2021 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.test.fakes
+
+import io.mockk.coEvery
+import io.mockk.mockk
+import org.matrix.android.sdk.internal.session.space.SpaceApi
+import org.matrix.android.sdk.internal.session.space.SpacesResponse
+import org.matrix.android.sdk.test.fixtures.ResolveSpaceInfoTaskParamsFixture
+
+internal class FakeSpaceApi {
+
+    val instance: SpaceApi = mockk()
+    val params = ResolveSpaceInfoTaskParamsFixture.aResolveSpaceInfoTaskParams()
+
+    fun givenStableEndpointReturns(response: SpacesResponse) {
+        coEvery { instance.getSpaceHierarchy(params.spaceId, params.suggestedOnly, params.limit, params.maxDepth, params.from) } returns response
+    }
+
+    fun givenStableEndpointThrows(throwable: Throwable) {
+        coEvery { instance.getSpaceHierarchy(params.spaceId, params.suggestedOnly, params.limit, params.maxDepth, params.from) } throws throwable
+    }
+
+    fun givenUnstableEndpointReturns(response: SpacesResponse) {
+        coEvery { instance.getSpaceHierarchyUnstable(params.spaceId, params.suggestedOnly, params.limit, params.maxDepth, params.from) } returns response
+    }
+}

matrix-sdk-android/src/test/java/org/matrix/android/sdk/test/fixtures/ResolveSpaceInfoTaskParamsFixture.kt

@@ -0,0 +1,35 @@
+/*
+ * Copyright 2021 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.test.fixtures
+
+import org.matrix.android.sdk.internal.session.space.ResolveSpaceInfoTask
+
+internal object ResolveSpaceInfoTaskParamsFixture {
+    fun aResolveSpaceInfoTaskParams(
+            spaceId: String = "",
+            limit: Int? = null,
+            maxDepth: Int? = null,
+            from: String? = null,
+            suggestedOnly: Boolean? = null,
+    ) = ResolveSpaceInfoTask.Params(
+            spaceId,
+            limit,
+            maxDepth,
+            from,
+            suggestedOnly,
+    )
+}

matrix-sdk-android/src/test/java/org/matrix/android/sdk/test/fixtures/SpacesResponseFixture.kt

@@ -0,0 +1,30 @@
+/*
+ * Copyright 2021 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.test.fixtures
+
+import org.matrix.android.sdk.internal.session.space.SpaceChildSummaryResponse
+import org.matrix.android.sdk.internal.session.space.SpacesResponse
+
+internal object SpacesResponseFixture {
+    fun aSpacesResponse(
+            nextBatch: String? = null,
+            rooms: List<SpaceChildSummaryResponse>? = null,
+    ) = SpacesResponse(
+            nextBatch,
+            rooms,
+    )
+}