Skip to content

Commit

Permalink
fix(room_preview): When requesting a room summary, use fallback serve…
Browse files Browse the repository at this point in the history
…r 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.
  • Loading branch information
jmartinesp committed Dec 2, 2024
1 parent 685386d commit d2fecb6
Show file tree
Hide file tree
Showing 2 changed files with 105 additions and 2 deletions.
13 changes: 12 additions & 1 deletion bindings/matrix-sdk-ffi/src/room_list.rs
Original file line number Diff line number Diff line change
Expand Up @@ -616,14 +616,25 @@ 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 {
let room_or_alias_id: OwnedRoomOrAliasId = self.inner.id().to_owned().into();
(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)))
Expand Down
94 changes: 93 additions & 1 deletion crates/matrix-sdk/src/room_preview.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -233,6 +233,9 @@ impl RoomPreview {
room_or_alias_id: &RoomOrAliasId,
via: Vec<OwnedServerName>,
) -> crate::Result<Self> {
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,
Expand Down Expand Up @@ -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<OwnedServerName>,
room_or_alias_id: &RoomOrAliasId,
) -> Vec<OwnedServerName> {
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"));
}
}

0 comments on commit d2fecb6

Please sign in to comment.