Skip to content
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

Better room preview, use room Summary API if available #3986

Merged
merged 2 commits into from
Sep 15, 2021
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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/3946.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Restricted Room previews aren't working
Original file line number Diff line number Diff line change
@@ -0,0 +1,111 @@
/*
* 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.api.session.room.model

import com.squareup.moshi.Json
import com.squareup.moshi.JsonClass

/**
* These are the same fields as those returned by /publicRooms, with a few additions: room_type, membership and is_encrypted.
*/
@JsonClass(generateAdapter = true)
data class RoomStrippedState(
/**
* Aliases of the room. May be empty.
*/
@Json(name = "aliases")
val aliases: List<String>? = null,

/**
* The canonical alias of the room, if any.
*/
@Json(name = "canonical_alias")
val canonicalAlias: String? = null,

/**
* The name of the room, if any.
*/
@Json(name = "name")
val name: String? = null,

/**
* Required. The number of members joined to the room.
*/
@Json(name = "num_joined_members")
val numJoinedMembers: Int = 0,

/**
* Required. The ID of the room.
*/
@Json(name = "room_id")
val roomId: String,

/**
* The topic of the room, if any.
*/
@Json(name = "topic")
val topic: String? = null,

/**
* Required. Whether the room may be viewed by guest users without joining.
*/
@Json(name = "world_readable")
val worldReadable: Boolean = false,

/**
* Required. Whether guest users may join the room and participate in it. If they can,
* they will be subject to ordinary power level rules like any other user.
*/
@Json(name = "guest_can_join")
val guestCanJoin: Boolean = false,

/**
* The URL for the room's avatar, if one is set.
*/
@Json(name = "avatar_url")
val avatarUrl: String? = null,

/**
* Undocumented item
*/
@Json(name = "m.federate")
val isFederated: Boolean = false,

/**
* Optional. If the room is encrypted. This is already accessible as stripped state.
*/
@Json(name = "is_encrypted")
val isEncrypted: Boolean?,

/**
* Optional. Type of the room, if any, i.e. m.space
*/
@Json(name = "room_type")
val roomType: String?,

/**
* The current membership of this user in the room. Usually leave if the room is fetched over federation.
*/
@Json(name = "membership")
val membership: String?
) {
/**
* Return the canonical alias, or the first alias from the list of aliases, or null
*/
fun getPrimaryAlias(): String? {
return canonicalAlias ?: aliases?.firstOrNull()
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
/*
* 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.room

import org.matrix.android.sdk.api.session.room.model.RoomStrippedState
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 javax.inject.Inject

internal interface GetRoomSummaryTask : Task<GetRoomSummaryTask.Params, RoomStrippedState> {
data class Params(
val roomId: String,
val viaServers: List<String>?
)
}

internal class DefaultGetRoomSummaryTask @Inject constructor(
private val roomAPI: RoomAPI,
private val globalErrorReceiver: GlobalErrorReceiver
) : GetRoomSummaryTask {

override suspend fun execute(params: GetRoomSummaryTask.Params): RoomStrippedState {
return executeRequest(globalErrorReceiver) {
roomAPI.getRoomSummary(params.roomId, params.viaServers)
Copy link
Member

Choose a reason for hiding this comment

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

Is there any fallback if this unstable API returns a 404? This can occur a lot.

Copy link
Member Author

Choose a reason for hiding this comment

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

It's done in PeekRoom task. It will try first the summary API, then check the public repository, and fallback to raw peek state if no results

}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ package org.matrix.android.sdk.internal.session.room
import org.matrix.android.sdk.api.session.events.model.Content
import org.matrix.android.sdk.api.session.events.model.Event
import org.matrix.android.sdk.api.session.room.model.Membership
import org.matrix.android.sdk.api.session.room.model.RoomStrippedState
import org.matrix.android.sdk.api.session.room.model.roomdirectory.PublicRoomsParams
import org.matrix.android.sdk.api.session.room.model.roomdirectory.PublicRoomsResponse
import org.matrix.android.sdk.api.util.JsonDict
Expand Down Expand Up @@ -254,7 +255,7 @@ internal interface RoomAPI {
@POST(NetworkConstants.URI_API_PREFIX_PATH_R0 + "join/{roomIdOrAlias}")
suspend fun join(@Path("roomIdOrAlias") roomIdOrAlias: String,
@Query("server_name") viaServers: List<String>,
@Body params: JsonDict): JoinRoomResponse
@Body params: JsonDict): JoinRoomResponse

/**
* Leave the given room.
Expand Down Expand Up @@ -381,4 +382,14 @@ internal interface RoomAPI {
@POST(NetworkConstants.URI_API_PREFIX_PATH_R0 + "rooms/{roomId}/upgrade")
suspend fun upgradeRoom(@Path("roomId") roomId: String,
@Body body: RoomUpgradeBody): RoomUpgradeResponse

/**
* The API returns the summary of the specified room, if the room could be found and the client should be able to view
* its contents according to the join_rules, history visibility, space membership and similar rules outlined in MSC3173
* as well as if the user is already a member of that room.
* https://github.com/deepbluev7/matrix-doc/blob/room-summaries/proposals/3266-room-summary.md
*/
@GET(NetworkConstants.URI_API_PREFIX_PATH_UNSTABLE + "im.nheko.summary/rooms/{roomidOrAlias}/summary")
Copy link
Member

Choose a reason for hiding this comment

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

I know it is like that in the MSC, I've made a comment about it, but here, can you write roomIdOrAlias, with a capital I please?

suspend fun getRoomSummary(@Path("roomidOrAlias") roomidOrAlias: String,
@Query("via") viaServers: List<String>?): RoomStrippedState
}
Original file line number Diff line number Diff line change
Expand Up @@ -253,4 +253,7 @@ internal abstract class RoomModule {

@Binds
abstract fun bindSign3pidInvitationTask(task: DefaultSign3pidInvitationTask): Sign3pidInvitationTask

@Binds
abstract fun bindSGetRoomSummaryTask(task: DefaultGetRoomSummaryTask): GetRoomSummaryTask
Copy link
Member

Choose a reason for hiding this comment

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

Remove the extra "S"

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

}
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import org.matrix.android.sdk.api.session.room.model.roomdirectory.PublicRoomsFi
import org.matrix.android.sdk.api.session.room.model.roomdirectory.PublicRoomsParams
import org.matrix.android.sdk.api.session.room.peeking.PeekResult
import org.matrix.android.sdk.api.util.MatrixItem
import org.matrix.android.sdk.internal.session.room.GetRoomSummaryTask
import org.matrix.android.sdk.internal.session.room.alias.GetRoomIdByAliasTask
import org.matrix.android.sdk.internal.session.room.directory.GetPublicRoomTask
import org.matrix.android.sdk.internal.session.room.directory.GetRoomDirectoryVisibilityTask
Expand All @@ -49,6 +50,7 @@ internal class DefaultPeekRoomTask @Inject constructor(
private val getRoomIdByAliasTask: GetRoomIdByAliasTask,
private val getRoomDirectoryVisibilityTask: GetRoomDirectoryVisibilityTask,
private val getPublicRoomTask: GetPublicRoomTask,
private val getRoomSummaryTask: GetRoomSummaryTask,
private val resolveRoomStateTask: ResolveRoomStateTask
) : PeekRoomTask {

Expand All @@ -70,6 +72,25 @@ internal class DefaultPeekRoomTask @Inject constructor(
serverList = emptyList()
}

// If the room summary API is available on the Home Server we should try it first
val strippedState = tryOrNull("Failed to get room stripped state roomId:$roomId") {
getRoomSummaryTask.execute(GetRoomSummaryTask.Params(roomId, serverList))
}
if (strippedState != null) {
BillCarsonFr marked this conversation as resolved.
Show resolved Hide resolved
return PeekResult.Success(
roomId = strippedState.roomId,
alias = strippedState.getPrimaryAlias() ?: params.roomIdOrAlias.takeIf { isAlias },
avatarUrl = strippedState.avatarUrl,
name = strippedState.name,
topic = strippedState.topic,
numJoinedMembers = strippedState.numJoinedMembers,
viaServers = serverList,
roomType = strippedState.roomType,
someMembers = null,
isPublic = strippedState.worldReadable
)
}

// Is it a public room?
val visibilityRes = tryOrNull("## PEEK: failed to get visibility") {
getRoomDirectoryVisibilityTask.execute(GetRoomDirectoryVisibilityTask.Params(roomId))
Expand Down