From 60fc4b5a43e68a4eeff62e33944dc5e82d295a99 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jorge=20Mart=C3=ADn?= Date: Fri, 15 Nov 2024 15:50:03 +0100 Subject: [PATCH] feat(room_preview): Use room directory search as another data source --- crates/matrix-sdk/src/room_preview.rs | 98 +++++++++++++++++-- crates/matrix-sdk/src/test_utils/mocks.rs | 76 +++++++++++++- .../src/tests/sliding_sync/room.rs | 94 +++++++++++++++++- 3 files changed, 255 insertions(+), 13 deletions(-) diff --git a/crates/matrix-sdk/src/room_preview.rs b/crates/matrix-sdk/src/room_preview.rs index 3cccf952ea3..ae59b660526 100644 --- a/crates/matrix-sdk/src/room_preview.rs +++ b/crates/matrix-sdk/src/room_preview.rs @@ -21,6 +21,7 @@ use matrix_sdk_base::{RoomInfo, RoomState}; use ruma::{ api::client::{membership::joined_members, state::get_state_events}, + directory::PublicRoomJoinRule, events::room::{history_visibility::HistoryVisibility, join_rules::JoinRule}, room::RoomType, space::SpaceRoomJoinRule, @@ -29,7 +30,10 @@ use ruma::{ use tokio::try_join; use tracing::{instrument, warn}; -use crate::{Client, Room}; +use crate::{ + room_directory_search::{RoomDescription, RoomDirectorySearch}, + Client, Room, +}; /// The preview of a room, be it invited/joined/left, or not. #[derive(Debug, Clone)] @@ -119,7 +123,7 @@ impl RoomPreview { /// Create a room preview from a known room (i.e. one we've been invited to, /// we've joined or we've left). - pub(crate) async fn from_known(room: &Room) -> Self { + pub async fn from_known(room: &Room) -> Self { let is_direct = room.is_direct().await.ok(); Self::from_room_info( @@ -140,21 +144,76 @@ impl RoomPreview { ) -> crate::Result { // Use the room summary endpoint, if available, as described in // https://github.com/deepbluev7/matrix-doc/blob/room-summaries/proposals/3266-room-summary.md - match Self::from_room_summary(client, room_id.clone(), room_or_alias_id, via).await { + match Self::from_room_summary(client, room_id.clone(), room_or_alias_id, via.clone()).await + { Ok(res) => return Ok(res), Err(err) => { warn!("error when previewing room from the room summary endpoint: {err}"); } } - // TODO: (optimization) Use the room search directory, if available: - // - if the room directory visibility is public, - // - then use a public room filter set to this room id + // Try room directory search next. + match Self::from_room_directory_search(client, &room_id, room_or_alias_id, via).await { + Ok(res) => return Ok(res), + Err(err) => { + warn!("Room '{room_or_alias_id}' not found in room directory search: {err}"); + } + } // Resort to using the room state endpoint, as well as the joined members one. Self::from_state_events(client, &room_id).await } + /// Get a [`RoomPreview`] by searching in the room directory for the + /// provided room alias or room id and transforming the [`RoomDescription`] + /// into a preview. + pub(crate) async fn from_room_directory_search( + client: &Client, + room_id: &RoomId, + room_or_alias_id: &RoomOrAliasId, + via: Vec, + ) -> crate::Result { + let mut directory_search = RoomDirectorySearch::new(client.clone()); + + // Get either the room alias or the room id without the leading identifier char + let search_term = if room_or_alias_id.is_room_alias_id() { + Some(room_or_alias_id.to_string().chars().skip(1).collect::()) + } else { + None + }; + + // If we have no alias, filtering using a room id is impossible, so just take + // the first 100 results and try to find the current room #YOLO + let batch_size = if search_term.is_some() { 20 } else { 100 }; + + if via.is_empty() { + // Just search in the current homeserver + let _ = directory_search.search(search_term, batch_size, None).await; + } else { + // Iterate all servers until one returns some results + for server in via { + if directory_search.loaded_pages() > 0 { + // If we previously got results don't search anymore + break; + } + + let _ = + directory_search.search(search_term.clone(), batch_size, Some(server)).await; + } + } + + let (values, _) = directory_search.results(); + while let Some(room_description) = values.iter().next() { + // Iterate until we find a room description with a matching room id + if room_description.room_id != room_id { + continue; + } + return Ok(RoomPreview::from_room_description(room_description.to_owned())); + } + + Err(crate::Error::InsufficientData) + } + /// Get a [`RoomPreview`] using MSC3266, if available on the remote server. /// /// Will fail with a 404 if the API is not available. @@ -258,4 +317,31 @@ impl RoomPreview { state, )) } + + pub(crate) fn from_room_description(room_description: RoomDescription) -> Self { + RoomPreview { + room_id: room_description.room_id, + canonical_alias: room_description.alias, + name: room_description.name, + topic: room_description.topic, + avatar_url: room_description.avatar_url, + num_joined_members: room_description.joined_members, + num_active_members: None, + // Assume it's a room + room_type: None, + join_rule: space_rule_from_public_rule(room_description.join_rule), + is_world_readable: room_description.is_world_readable, + state: None, + is_direct: None, + } + } +} + +fn space_rule_from_public_rule(join_rule: PublicRoomJoinRule) -> SpaceRoomJoinRule { + match join_rule { + PublicRoomJoinRule::Public => SpaceRoomJoinRule::Public, + PublicRoomJoinRule::Knock => SpaceRoomJoinRule::Knock, + PublicRoomJoinRule::_Custom(rule) => SpaceRoomJoinRule::_Custom(rule), + _ => panic!("Unexpected PublicRoomJoinRule {:?}", join_rule), + } } diff --git a/crates/matrix-sdk/src/test_utils/mocks.rs b/crates/matrix-sdk/src/test_utils/mocks.rs index 6f923eb73c2..0cd4929b37f 100644 --- a/crates/matrix-sdk/src/test_utils/mocks.rs +++ b/crates/matrix-sdk/src/test_utils/mocks.rs @@ -17,24 +17,31 @@ #![allow(missing_debug_implementations)] -use std::sync::{Arc, Mutex}; +use std::{ + collections::BTreeMap, + sync::{Arc, Mutex}, +}; use matrix_sdk_base::{deserialized_responses::TimelineEvent, store::StoreConfig, SessionMeta}; use matrix_sdk_test::{ test_json, InvitedRoomBuilder, JoinedRoomBuilder, KnockedRoomBuilder, LeftRoomBuilder, SyncResponseBuilder, }; -use ruma::{api::MatrixVersion, device_id, user_id, MxcUri, OwnedEventId, OwnedRoomId, RoomId}; +use ruma::{ + api::MatrixVersion, device_id, directory::PublicRoomsChunk, user_id, MxcUri, OwnedEventId, + OwnedRoomId, RoomId, ServerName, +}; +use serde::Deserialize; use serde_json::json; use wiremock::{ matchers::{body_partial_json, header, method, path, path_regex}, - Mock, MockBuilder, MockGuard, MockServer, Respond, ResponseTemplate, Times, + Mock, MockBuilder, MockGuard, MockServer, Request, Respond, ResponseTemplate, Times, }; use crate::{ config::RequestConfig, matrix_auth::{MatrixSession, MatrixSessionTokens}, - Client, ClientBuilder, Room, + Client, ClientBuilder, OwnedServerName, Room, }; /// A [`wiremock`] [`MockServer`] along with useful methods to help mocking @@ -452,6 +459,12 @@ impl MatrixMockServer { Mock::given(method("PUT")).and(path_regex(r"/_matrix/client/v3/directory/room/.*")); MockEndpoint { mock, server: &self.server, endpoint: CreateRoomAliasEndpoint } } + + /// Create a prebuilt mock for creating room aliases. + pub fn mock_public_rooms(&self) -> MockEndpoint<'_, PublicRoomsEndpoint> { + let mock = Mock::given(method("POST")).and(path_regex(r"/_matrix/client/v3/publicRooms")); + MockEndpoint { mock, server: &self.server, endpoint: PublicRoomsEndpoint } + } } /// Parameter to [`MatrixMockServer::sync_room`]. @@ -1033,6 +1046,61 @@ impl<'a> MockEndpoint<'a, CreateRoomAliasEndpoint> { } } +/// A prebuilt mock for paginating the public room list. +pub struct PublicRoomsEndpoint; + +impl<'a> MockEndpoint<'a, PublicRoomsEndpoint> { + /// Returns a data endpoint for paginating the public room list. + pub fn ok( + self, + chunk: Vec, + next_batch: Option, + prev_batch: Option, + total_room_count_estimate: Option, + ) -> MatrixMock<'a> { + let mock = self.mock.respond_with(ResponseTemplate::new(200).set_body_json(json!({ + "chunk": chunk, + "next_batch": next_batch, + "prev_batch": prev_batch, + "total_room_count_estimate": total_room_count_estimate, + }))); + MatrixMock { server: self.server, mock } + } + + /// Returns a data endpoint for paginating the public room list with several + /// `via` params. + /// + /// Each `via` param must be in the `server_map` parameter, otherwise it'll + /// fail. + pub fn with_via_params( + self, + server_map: BTreeMap>, + ) -> MatrixMock<'a> { + let mock = self.mock.respond_with(move |req: &Request| { + #[derive(Deserialize)] + struct PartialRequest { + server: Option, + } + + let (_, server) = req + .url + .query_pairs() + .into_iter() + .find(|(key, _)| key == "server") + .expect("Server param not found in request URL"); + let server = ServerName::parse(server).expect("Couldn't parse server name"); + let chunk = server_map + .get(&server) + .expect("Chunk for the server param not found"); + ResponseTemplate::new(200).set_body_json(json!({ + "chunk": chunk, + "total_room_count_estimate": chunk.len(), + })) + }); + MatrixMock { server: self.server, mock } + } +} + /// An augmented [`ClientBuilder`] that also allows for handling session login. pub struct MockClientBuilder { builder: ClientBuilder, diff --git a/testing/matrix-sdk-integration-testing/src/tests/sliding_sync/room.rs b/testing/matrix-sdk-integration-testing/src/tests/sliding_sync/room.rs index e8b41d9c528..104c1324124 100644 --- a/testing/matrix-sdk-integration-testing/src/tests/sliding_sync/room.rs +++ b/testing/matrix-sdk-integration-testing/src/tests/sliding_sync/room.rs @@ -2,6 +2,7 @@ #![allow(unused)] use std::{ + collections::BTreeMap, sync::{Arc, Mutex as StdMutex}, time::Duration, }; @@ -21,6 +22,7 @@ use matrix_sdk::{ room::create_room::v3::{Request as CreateRoomRequest, RoomPreset}, }, assign, + directory::PublicRoomsChunkInit, events::{ receipt::ReceiptThread, room::{ @@ -30,14 +32,16 @@ use matrix_sdk::{ }, AnySyncMessageLikeEvent, InitialStateEvent, Mentions, StateEventType, }, - mxc_uri, + mxc_uri, owned_server_name, room_id, space::SpaceRoomJoinRule, - RoomId, + uint, RoomId, }, sliding_sync::VersionBuilder, + test_utils::{logged_in_client_with_server, mocks::MatrixMockServer}, Client, RoomInfo, RoomMemberships, RoomState, SlidingSyncList, SlidingSyncMode, }; -use matrix_sdk_base::sliding_sync::http; +use matrix_sdk_base::{ruma::room_alias_id, sliding_sync::http}; +use matrix_sdk_test::async_test; use matrix_sdk_ui::{ room_list_service::filters::new_filter_all, sync_service::SyncService, timeline::RoomExt, RoomListService, @@ -1092,6 +1096,90 @@ async fn test_room_preview() -> Result<()> { Ok(()) } +#[async_test] +async fn test_room_preview_with_room_directory_search_and_room_alias_only() { + let server = MatrixMockServer::new().await; + let client = server.client_builder().build().await; + + let room_alias = room_alias_id!("#a-room:matrix.org"); + let expected_room_id = room_id!("!a-room:matrix.org"); + + // Allow retrieving the room id + server + .mock_room_directory_resolve_alias() + .ok(expected_room_id.as_ref(), Vec::new()) + .mock_once() + .mount() + .await; + + // Given a successful public room search + let chunks = vec![PublicRoomsChunkInit { + num_joined_members: uint!(0), + room_id: expected_room_id.to_owned(), + world_readable: true, + guest_can_join: true, + } + .into()]; + server.mock_public_rooms().ok(chunks, None, None, Some(1)).mock_once().mount().await; + + // The room preview is found + let preview = client + .get_room_preview(room_alias.into(), Vec::new()) + .await + .expect("room preview couldn't be retrieved"); + assert_eq!(preview.room_id, expected_room_id); +} + +#[async_test] +async fn test_room_preview_with_room_directory_search_and_room_alias_only_in_several_homeservers() { + let server = MatrixMockServer::new().await; + let client = server.client_builder().build().await; + + let room_alias = room_alias_id!("#a-room:matrix.org"); + let expected_room_id = room_id!("!a-room:matrix.org"); + + // Allow retrieving the room id + server + .mock_room_directory_resolve_alias() + .ok(expected_room_id.as_ref(), Vec::new()) + .mock_once() + .mount() + .await; + + let via_1 = owned_server_name!("server1.com"); + let via_2 = owned_server_name!("server2.com"); + + // Given a couple of successful public room search responses + let via_map = BTreeMap::from_iter(vec![ + ( + via_1.to_owned(), + vec![PublicRoomsChunkInit { + num_joined_members: uint!(0), + room_id: expected_room_id.to_owned(), + world_readable: true, + guest_can_join: true, + } + .into()], + ), + (via_2.to_owned(), Vec::new()), + ]); + server + .mock_public_rooms() + .with_via_params(via_map) + // Called only once as the first response returns the room, so no need to check other + // homeservers + .expect(1) + .mount() + .await; + + // The room preview is found in the first response + let preview = client + .get_room_preview(room_alias.into(), vec![via_1, via_2]) + .await + .expect("room preview couldn't be retrieved"); + assert_eq!(preview.room_id, expected_room_id); +} + fn assert_room_preview(preview: &RoomPreview, room_alias: &str) { assert_eq!(preview.canonical_alias.as_ref().unwrap().alias(), room_alias); assert_eq!(preview.name.as_ref().unwrap(), "Alice's Room");