From d2fecb6701a4a607fb3fa19fe19c54d2680cecf0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jorge=20Mart=C3=ADn?= Date: Thu, 28 Nov 2024 18:03:29 +0100 Subject: [PATCH] fix(room_preview): When requesting a room summary, use fallback server names If no server names are provided for the room summary request and the room's server name doesn't match the current user's server name, add the room alias/id server name as a fallback value. This seems to fix room preview through federation. Also, when getting a summary for a room list item, if it's an invite one, add the server name of the inviter's user id as another possible fallback. Changelog: Use the inviter's server name and the server name from the room alias as fallback values for the via parameter when requesting the room summary from the homeserver. --- bindings/matrix-sdk-ffi/src/room_list.rs | 13 +++- crates/matrix-sdk/src/room_preview.rs | 94 +++++++++++++++++++++++- 2 files changed, 105 insertions(+), 2 deletions(-) diff --git a/bindings/matrix-sdk-ffi/src/room_list.rs b/bindings/matrix-sdk-ffi/src/room_list.rs index 412ee88cc..1c193b0ac 100644 --- a/bindings/matrix-sdk-ffi/src/room_list.rs +++ b/bindings/matrix-sdk-ffi/src/room_list.rs @@ -616,7 +616,8 @@ impl RoomListItem { // Do the thing. let client = self.inner.client(); - let (room_or_alias_id, server_names) = if let Some(alias) = self.inner.canonical_alias() { + let (room_or_alias_id, mut server_names) = if let Some(alias) = self.inner.canonical_alias() + { let room_or_alias_id: OwnedRoomOrAliasId = alias.into(); (room_or_alias_id, Vec::new()) } else { @@ -624,6 +625,16 @@ impl RoomListItem { (room_or_alias_id, server_names) }; + // If no server names are provided and the room's membership is invited, + // add the server name from the sender's user id as a fallback value + if server_names.is_empty() { + if let Ok(invite_details) = self.inner.invite_details().await { + if let Some(inviter) = invite_details.inviter { + server_names.push(inviter.user_id().server_name().to_owned()); + } + } + } + let room_preview = client.get_room_preview(&room_or_alias_id, server_names).await?; Ok(Arc::new(RoomPreview::new(AsyncRuntimeDropped::new(client), room_preview))) diff --git a/crates/matrix-sdk/src/room_preview.rs b/crates/matrix-sdk/src/room_preview.rs index 58c9c68a5..a5385fde0 100644 --- a/crates/matrix-sdk/src/room_preview.rs +++ b/crates/matrix-sdk/src/room_preview.rs @@ -26,7 +26,7 @@ use ruma::{ events::room::{history_visibility::HistoryVisibility, join_rules::JoinRule}, room::RoomType, space::SpaceRoomJoinRule, - OwnedMxcUri, OwnedRoomAliasId, OwnedRoomId, OwnedServerName, RoomId, RoomOrAliasId, + OwnedMxcUri, OwnedRoomAliasId, OwnedRoomId, OwnedServerName, RoomId, RoomOrAliasId, ServerName, }; use tokio::try_join; use tracing::{instrument, warn}; @@ -233,6 +233,9 @@ impl RoomPreview { room_or_alias_id: &RoomOrAliasId, via: Vec, ) -> crate::Result { + let own_server_name = client.session_meta().map(|s| s.user_id.server_name()); + let via = ensure_server_names_is_not_empty(own_server_name, via, room_or_alias_id); + let request = ruma::api::client::room::get_summary::msc3266::Request::new( room_or_alias_id.to_owned(), via, @@ -372,3 +375,92 @@ async fn search_for_room_preview_in_room_directory( Ok(None) } + +// Make sure the server name of the room id/alias is +// included in the list of server names to send if no server names are provided +fn ensure_server_names_is_not_empty( + own_server_name: Option<&ServerName>, + server_names: Vec, + room_or_alias_id: &RoomOrAliasId, +) -> Vec { + let mut server_names = server_names; + + if let Some((own_server, alias_server)) = own_server_name.zip(room_or_alias_id.server_name()) { + if server_names.is_empty() && own_server != alias_server { + server_names.push(alias_server.to_owned()); + } + } + + server_names +} + +#[cfg(test)] +mod tests { + use ruma::{owned_server_name, room_alias_id, room_id, server_name, RoomOrAliasId, ServerName}; + + use crate::room_preview::ensure_server_names_is_not_empty; + + #[test] + fn test_ensure_server_names_is_not_empty_when_no_own_server_name_is_provided() { + let own_server_name: Option<&ServerName> = None; + let room_or_alias_id: &RoomOrAliasId = room_id!("!test:localhost").into(); + + let server_names = + ensure_server_names_is_not_empty(own_server_name, Vec::new(), room_or_alias_id); + + // There was no own server name to check against, so no additional server name + // was added + assert!(server_names.is_empty()); + } + + #[test] + fn test_ensure_server_names_is_not_empty_when_room_alias_or_id_has_no_server_name() { + let own_server_name: Option<&ServerName> = Some(server_name!("localhost")); + let room_or_alias_id: &RoomOrAliasId = room_id!("!test").into(); + + let server_names = + ensure_server_names_is_not_empty(own_server_name, Vec::new(), room_or_alias_id); + + // The room id has no server name, so nothing could be added + assert!(server_names.is_empty()); + } + + #[test] + fn test_ensure_server_names_is_not_empty_with_same_server_name() { + let own_server_name: Option<&ServerName> = Some(server_name!("localhost")); + let room_or_alias_id: &RoomOrAliasId = room_id!("!test:localhost").into(); + + let server_names = + ensure_server_names_is_not_empty(own_server_name, Vec::new(), room_or_alias_id); + + // The room id's server name was the same as our own server name, so there's no + // need to add it + assert!(server_names.is_empty()); + } + + #[test] + fn test_ensure_server_names_is_not_empty_with_different_room_id_server_name() { + let own_server_name: Option<&ServerName> = Some(server_name!("localhost")); + let room_or_alias_id: &RoomOrAliasId = room_id!("!test:matrix.org").into(); + + let server_names = + ensure_server_names_is_not_empty(own_server_name, Vec::new(), room_or_alias_id); + + // The server name in the room id was added + assert!(!server_names.is_empty()); + assert_eq!(server_names[0], owned_server_name!("matrix.org")); + } + + #[test] + fn test_ensure_server_names_is_not_empty_with_different_room_alias_server_name() { + let own_server_name: Option<&ServerName> = Some(server_name!("localhost")); + let room_or_alias_id: &RoomOrAliasId = room_alias_id!("#test:matrix.org").into(); + + let server_names = + ensure_server_names_is_not_empty(own_server_name, Vec::new(), room_or_alias_id); + + // The server name in the room alias was added + assert!(!server_names.is_empty()); + assert_eq!(server_names[0], owned_server_name!("matrix.org")); + } +}