From 084e00b0aced96c95cbfb34bd668824b357dc179 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jorge=20Mart=C3=ADn?= Date: Fri, 18 Oct 2024 13:11:14 +0200 Subject: [PATCH 1/4] chore(ffi): add `RoomListItem::room_without_timeline` that can return both an invited or a knocked room. This deprecates `RoomListItem::invited_room`. --- bindings/matrix-sdk-ffi/src/lib.rs | 2 ++ bindings/matrix-sdk-ffi/src/room_list.rs | 30 ++++++++++++++++++------ 2 files changed, 25 insertions(+), 7 deletions(-) diff --git a/bindings/matrix-sdk-ffi/src/lib.rs b/bindings/matrix-sdk-ffi/src/lib.rs index 0e404a848ff..8916ee52282 100644 --- a/bindings/matrix-sdk-ffi/src/lib.rs +++ b/bindings/matrix-sdk-ffi/src/lib.rs @@ -18,6 +18,8 @@ mod platform; mod room; mod room_directory_search; mod room_info; + +#[allow(deprecated)] mod room_list; mod room_member; mod room_preview; diff --git a/bindings/matrix-sdk-ffi/src/room_list.rs b/bindings/matrix-sdk-ffi/src/room_list.rs index 5eae181a6c7..e8525dd4876 100644 --- a/bindings/matrix-sdk-ffi/src/room_list.rs +++ b/bindings/matrix-sdk-ffi/src/room_list.rs @@ -48,7 +48,7 @@ pub enum RoomListError { #[error("Event cache ran into an error: {error}")] EventCache { error: String }, #[error("The requested room doesn't match the membership requirements {expected:?}, observed {actual:?}")] - IncorrectRoomMembership { expected: Membership, actual: Membership }, + IncorrectRoomMembership { expected: Vec, actual: Membership }, } impl From for RoomListError { @@ -574,16 +574,32 @@ impl RoomListItem { } /// Builds a `Room` FFI from an invited room without initializing its - /// internal timeline + /// internal timeline. /// - /// An error will be returned if the room is a state different than invited + /// An error will be returned if the room is a state different than invited. /// /// ⚠️ Holding on to this room instance after it has been joined is not - /// safe. Use `full_room` instead + /// safe. Use `full_room` instead. + #[deprecated(note = "Please use `room_without_timeline` instead.")] fn invited_room(&self) -> Result, RoomListError> { - if !matches!(self.membership(), Membership::Invited) { + self.room_without_timeline() + } + + /// Builds a `Room` FFI from a room without initializing its internal + /// timeline. + /// + /// An error will be returned if the room is a state other than invited + /// or knocked, the 2 states that would match this use case. + /// + /// ⚠️ Holding on to this room instance after it has been joined is not + /// safe. Use `full_room` instead. + fn room_without_timeline(&self) -> Result, RoomListError> { + let membership = self.membership(); + if !matches!(membership, Membership::Invited) + && !matches!(self.membership(), Membership::Knocked) + { return Err(RoomListError::IncorrectRoomMembership { - expected: Membership::Invited, + expected: vec![Membership::Invited, Membership::Knocked], actual: self.membership(), }); } @@ -598,7 +614,7 @@ impl RoomListItem { fn full_room(&self) -> Result, RoomListError> { if !matches!(self.membership(), Membership::Joined) { return Err(RoomListError::IncorrectRoomMembership { - expected: Membership::Joined, + expected: vec![Membership::Joined], actual: self.membership(), }); } From 736f4d49f07e783e96c32694bd51fb661c3a9ce2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jorge=20Mart=C3=ADn?= Date: Tue, 22 Oct 2024 17:26:13 +0200 Subject: [PATCH 2/4] chore(room_preview): add `RoomList::preview_room`. This allows us to get `RoomPreview` from a room list item when the membership is either invited or knocked. Also, modify `RoomPreview` so it contains both the room info needed for the preview as well as 2 methods, for joining the room or leaving it (rejecting the invite/rescinding the knock). --- bindings/matrix-sdk-ffi/src/client.rs | 7 +- bindings/matrix-sdk-ffi/src/error.rs | 15 ++ bindings/matrix-sdk-ffi/src/room_list.rs | 50 +++++-- bindings/matrix-sdk-ffi/src/room_preview.rs | 119 ++++++++++++---- crates/matrix-sdk-base/src/rooms/normal.rs | 2 + crates/matrix-sdk/src/client/mod.rs | 2 +- crates/matrix-sdk/src/room/mod.rs | 4 +- crates/matrix-sdk/src/room_preview.rs | 148 ++++++++++++++++++-- 8 files changed, 291 insertions(+), 56 deletions(-) diff --git a/bindings/matrix-sdk-ffi/src/client.rs b/bindings/matrix-sdk-ffi/src/client.rs index f5f693bae21..3947dc7e882 100644 --- a/bindings/matrix-sdk-ffi/src/client.rs +++ b/bindings/matrix-sdk-ffi/src/client.rs @@ -2,6 +2,7 @@ use std::{ collections::HashMap, fmt::Debug, mem::ManuallyDrop, + ops::Deref, path::Path, sync::{Arc, RwLock}, }; @@ -1041,8 +1042,9 @@ impl Client { let room_id: &RoomId = &room_id; let sdk_room_preview = self.inner.get_room_preview(room_id.into(), via_servers).await?; + let client = (*self.inner.deref()).clone(); - Ok(RoomPreview::from_sdk(sdk_room_preview)) + RoomPreview::try_from_sdk(sdk_room_preview, client) } /// Given a room alias, get the preview of a room, to interact with it. @@ -1058,8 +1060,9 @@ impl Client { let room_alias: &RoomAliasId = &room_alias; let sdk_room_preview = self.inner.get_room_preview(room_alias.into(), Vec::new()).await?; + let client = (*self.inner.deref()).clone(); - Ok(RoomPreview::from_sdk(sdk_room_preview)) + RoomPreview::try_from_sdk(sdk_room_preview, client) } /// Waits until an at least partially synced room is received, and returns diff --git a/bindings/matrix-sdk-ffi/src/error.rs b/bindings/matrix-sdk-ffi/src/error.rs index 6262b765dc4..9c1f19fbb7f 100644 --- a/bindings/matrix-sdk-ffi/src/error.rs +++ b/bindings/matrix-sdk-ffi/src/error.rs @@ -8,6 +8,9 @@ use matrix_sdk::{ }; use matrix_sdk_ui::{encryption_sync_service, notification_client, sync_service, timeline}; use uniffi::UnexpectedUniFFICallbackError; +use matrix_sdk::room_preview::RoomPreviewError; +use crate::room_list::RoomListError; + #[derive(Debug, thiserror::Error)] pub enum ClientError { #[error("client error: {msg}")] @@ -128,6 +131,12 @@ impl From for ClientError { } } +impl From for ClientError { + fn from(e: RoomListError) -> Self { + Self::new(e) + } +} + impl From for ClientError { fn from(e: EventCacheError) -> Self { Self::new(e) @@ -146,6 +155,12 @@ impl From for ClientError { } } +impl From for ClientError { + fn from(e: RoomSendQueueError) -> Self { + Self::new(e) + } +} + /// Bindings version of the sdk type replacing OwnedUserId/DeviceIds with simple /// String. /// diff --git a/bindings/matrix-sdk-ffi/src/room_list.rs b/bindings/matrix-sdk-ffi/src/room_list.rs index e8525dd4876..60abac54b61 100644 --- a/bindings/matrix-sdk-ffi/src/room_list.rs +++ b/bindings/matrix-sdk-ffi/src/room_list.rs @@ -16,12 +16,16 @@ use matrix_sdk_ui::{ timeline::default_event_filter, unable_to_decrypt_hook::UtdHookManager, }; +use ruma::{ + IdParseError, OwnedRoomOrAliasId, OwnedServerName, RoomAliasId, RoomOrAliasId, ServerName, +}; use tokio::sync::RwLock; use crate::{ error::ClientError, room::{Membership, Room}, room_info::RoomInfo, + room_preview::RoomPreview, timeline::{EventTimelineItem, Timeline}, timeline_event_filter::TimelineEventTypeFilter, TaskHandle, RUNTIME, @@ -580,31 +584,47 @@ impl RoomListItem { /// /// ⚠️ Holding on to this room instance after it has been joined is not /// safe. Use `full_room` instead. - #[deprecated(note = "Please use `room_without_timeline` instead.")] + #[deprecated(note = "Please use `preview_room` instead.")] fn invited_room(&self) -> Result, RoomListError> { - self.room_without_timeline() + if !matches!(self.membership(), Membership::Invited) { + return Err(RoomListError::IncorrectRoomMembership { + expected: vec![Membership::Invited], + actual: self.membership(), + }); + } + Ok(Arc::new(Room::new(self.inner.inner_room().clone()))) } - /// Builds a `Room` FFI from a room without initializing its internal - /// timeline. + /// Builds a `RoomPreview` from a room list item. This is intended for rooms + /// with [`Membership::Invite`] or [`Membership::Knocked`], /// /// An error will be returned if the room is a state other than invited - /// or knocked, the 2 states that would match this use case. - /// - /// ⚠️ Holding on to this room instance after it has been joined is not - /// safe. Use `full_room` instead. - fn room_without_timeline(&self) -> Result, RoomListError> { + /// or knocked. + async fn preview_room(&self, via: Vec) -> Result, ClientError> { let membership = self.membership(); - if !matches!(membership, Membership::Invited) - && !matches!(self.membership(), Membership::Knocked) - { + if !matches!(membership, Membership::Invited | Membership::Knocked) { return Err(RoomListError::IncorrectRoomMembership { expected: vec![Membership::Invited, Membership::Knocked], - actual: self.membership(), - }); + actual: membership, + } + .into()); } - Ok(Arc::new(Room::new(self.inner.inner_room().clone()))) + 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: OwnedRoomOrAliasId = alias.into(); + (room_or_alias_id, Vec::new()) + } else { + let room_or_alias_id: OwnedRoomOrAliasId = self.inner.id().to_owned().into(); + let server_names: Vec = via + .into_iter() + .map(|server| ServerName::parse(server).map_err(ClientError::from)) + .collect::>()?; + (room_or_alias_id, server_names) + }; + + let room_preview = client.get_room_preview(&room_or_alias_id, server_names).await?; + Ok(Arc::new(RoomPreview::try_from_sdk(room_preview, client))) } /// Build a full `Room` FFI object, filling its associated timeline. diff --git a/bindings/matrix-sdk-ffi/src/room_preview.rs b/bindings/matrix-sdk-ffi/src/room_preview.rs index 68214bb11ec..2c37af788c2 100644 --- a/bindings/matrix-sdk-ffi/src/room_preview.rs +++ b/bindings/matrix-sdk-ffi/src/room_preview.rs @@ -1,9 +1,87 @@ -use matrix_sdk::{room_preview::RoomPreview as SdkRoomPreview, RoomState}; +use matrix_sdk::{ + room_preview::{ + JoinRoomPreviewAction as SdkJoinRoomPreviewAction, LeaveRoomPreviewAction as SdkLeaveRoomPreviewAction, RoomPreview as SdkRoomPreview, + RoomPreviewActions, + }, + Client, RoomState, +}; use ruma::space::SpaceRoomJoinRule; -/// The preview of a room, be it invited/joined/left, or not. +use crate::{client::JoinRule, error::ClientError, room::Membership}; + #[derive(uniffi::Record)] pub struct RoomPreview { + pub info: RoomPreviewInfo, + pub room_preview_actions: RoomPreviewActions, + pub(crate) sdk_info: matrix_sdk::room_preview::RoomPreview, +} + +#[derive(uniffi::Enum)] +pub enum RoomPreviewAction { + Invited { join: JoinRoomPreviewAction, leave: LeaveRoomPreviewAction }, + Knocked { leave: LeaveRoomPreviewAction }, +} + +#[derive(uniffi::Object)] +pub struct JoinRoomPreviewAction { + inner: SdkJoinRoomPreviewAction, +} + +#[matrix_sdk_ffi_macros::export] +impl JoinRoomPreviewAction { + async fn run(&self) { + self.inner.run().unwrap() + } +} + +impl From for RoomPreviewAction { + fn from(actions: RoomPreviewActions) -> Self { + match actions { + RoomPreviewActions::Invited { join, leave } => Self::Invited { join, leave }, + RoomPreviewActions::Knocked { leave } => Self::Knocked { leave }, + } + } +} + +struct + +impl RoomPreview { + pub(crate) fn try_from_sdk(info: SdkRoomPreview, client: Client) -> Result { + let info = RoomPreviewInfo { + room_id: info.room_id.to_string(), + canonical_alias: info.canonical_alias.map(|alias| alias.to_string()), + name: info.name, + topic: info.topic, + avatar_url: info.avatar_url.map(|url| url.to_string()), + num_joined_members: info.num_joined_members, + room_type: info.room_type.map(|room_type| room_type.to_string()), + is_history_world_readable: info.is_world_readable, + membership: info.state.map(|state| state.into()).unwrap_or_else(|| Membership::Left), + join_rule: info.join_rule.into(), + }; + + let room_preview_actions = match info.membership { + Membership::Invited => RoomPreviewActions::Invited { + join: JoinRoomPreviewAction::new(client.clone()), + leave: LeaveRoomPreviewAction::new(client.clone()), + }, + Membership::Knocked => { + RoomPreviewActions::Knocked { leave: LeaveRoomPreviewAction::new(client.clone()) } + } + _ => { + return Err(ClientError::new(format!( + "The room preview had membership {:?} instead of Invited or Knocked.", + info.membership + ))) + } + }; + Ok(Self { info, room_preview_actions }) + } +} + +/// The preview of a room, be it invited/joined/left, or not. +#[derive(uniffi::Record)] +pub struct RoomPreviewInfo { /// The room id for this room. pub room_id: String, /// The canonical alias for the room. @@ -21,33 +99,22 @@ pub struct RoomPreview { /// Is the history world-readable for this room? pub is_history_world_readable: bool, /// Is the room joined by the current user? - pub is_joined: bool, - /// Is the current user invited to this room? - pub is_invited: bool, + pub membership: Membership, /// is the join rule public for this room? - pub is_public: bool, - /// Can we knock (or restricted-knock) to this room? - pub can_knock: bool, + pub join_rule: JoinRule, } -impl RoomPreview { - pub(crate) fn from_sdk(preview: SdkRoomPreview) -> Self { - Self { - room_id: preview.room_id.to_string(), - canonical_alias: preview.canonical_alias.map(|alias| alias.to_string()), - name: preview.name, - topic: preview.topic, - avatar_url: preview.avatar_url.map(|url| url.to_string()), - num_joined_members: preview.num_joined_members, - room_type: preview.room_type.map(|room_type| room_type.to_string()), - is_history_world_readable: preview.is_world_readable, - is_joined: preview.state.map_or(false, |state| state == RoomState::Joined), - is_invited: preview.state.map_or(false, |state| state == RoomState::Invited), - is_public: preview.join_rule == SpaceRoomJoinRule::Public, - can_knock: matches!( - preview.join_rule, - SpaceRoomJoinRule::KnockRestricted | SpaceRoomJoinRule::Knock - ), +impl From for JoinRule { + fn from(join_rule: SpaceRoomJoinRule) -> Self { + match join_rule { + SpaceRoomJoinRule::Public => JoinRule::Public, + SpaceRoomJoinRule::Private => JoinRule::Private, + SpaceRoomJoinRule::Invite => JoinRule::Invite, + SpaceRoomJoinRule::Knock => JoinRule::Knock, + SpaceRoomJoinRule::KnockRestricted => JoinRule::KnockRestricted { rules: Vec::new() }, + SpaceRoomJoinRule::Restricted => JoinRule::Restricted { rules: Vec::new() }, + // For the `_Custom` case, assume it's private + _ => JoinRule::Private, } } } diff --git a/crates/matrix-sdk-base/src/rooms/normal.rs b/crates/matrix-sdk-base/src/rooms/normal.rs index 3d426261f2b..cb0ee349279 100644 --- a/crates/matrix-sdk-base/src/rooms/normal.rs +++ b/crates/matrix-sdk-base/src/rooms/normal.rs @@ -186,6 +186,8 @@ impl RoomSummary { /// Enum keeping track in which state the room is, e.g. if our own user is /// joined, RoomState::Invited, or has left the room. #[derive(Clone, Copy, Debug, Eq, PartialEq, Serialize, Deserialize)] +#[cfg(feature = "uniffi")] +#[derive(uniffi::Enum)] pub enum RoomState { /// The room is in a joined state. Joined, diff --git a/crates/matrix-sdk/src/client/mod.rs b/crates/matrix-sdk/src/client/mod.rs index e2304998034..7066fcba545 100644 --- a/crates/matrix-sdk/src/client/mod.rs +++ b/crates/matrix-sdk/src/client/mod.rs @@ -1009,7 +1009,7 @@ impl Client { }; if let Some(room) = self.get_room(&room_id) { - return Ok(RoomPreview::from_known(&room)); + return Ok(RoomPreview::from_known(&room).await); } RoomPreview::from_unknown(self, room_id, room_or_alias_id, via).await diff --git a/crates/matrix-sdk/src/room/mod.rs b/crates/matrix-sdk/src/room/mod.rs index 1fa314d473c..b3a0e01ed01 100644 --- a/crates/matrix-sdk/src/room/mod.rs +++ b/crates/matrix-sdk/src/room/mod.rs @@ -195,9 +195,7 @@ impl Room { false }); - let request = join_room_by_id::v3::Request::new(self.inner.room_id().to_owned()); - let response = self.client.send(request, None).await?; - self.client.base_client().room_joined(&response.room_id).await?; + self.client.join_room_by_id(self.room_id()).await?; if mark_as_direct { self.set_is_direct(true).await?; diff --git a/crates/matrix-sdk/src/room_preview.rs b/crates/matrix-sdk/src/room_preview.rs index 9b8f1da8469..1ebdd7dd982 100644 --- a/crates/matrix-sdk/src/room_preview.rs +++ b/crates/matrix-sdk/src/room_preview.rs @@ -18,9 +18,14 @@ //! This offers a few capabilities for previewing the content of the room as //! well. +use anyhow::anyhow; +use mas_oidc_client::types::errors::ClientError; use matrix_sdk_base::{RoomInfo, RoomState}; use ruma::{ - api::client::{membership::joined_members, state::get_state_events}, + api::client::{ + membership::{joined_members, leave_room}, + state::get_state_events, + }, events::room::{history_visibility::HistoryVisibility, join_rules::JoinRule}, room::RoomType, space::SpaceRoomJoinRule, @@ -29,10 +34,13 @@ use ruma::{ use tokio::try_join; use tracing::{instrument, warn}; -use crate::{Client, Room}; +use crate::{ + error::WrongRoomState, + Client, Error, Room, +}; /// The preview of a room, be it invited/joined/left, or not. -#[derive(Debug)] +#[derive(Debug, Clone)] pub struct RoomPreview { /// The actual room id for this room. /// @@ -69,6 +77,9 @@ pub struct RoomPreview { /// /// Set to `None` if the room is unknown to the user. pub state: Option, + + /// The `m.room.direct` state of the room, if known. + pub is_direct: Option, } impl RoomPreview { @@ -76,11 +87,17 @@ impl RoomPreview { /// /// Note: not using the room info's state/count of joined members, because /// we can do better than that. - fn from_room_info( + async fn from_room_info( + client: &Client, room_info: RoomInfo, num_joined_members: u64, state: Option, ) -> Self { + let is_direct = if let Some(room) = client.get_room(room_info.room_id()) { + room.is_direct().await.ok() + } else { + None + }; RoomPreview { room_id: room_info.room_id().to_owned(), canonical_alias: room_info.canonical_alias().map(ToOwned::to_owned), @@ -102,16 +119,22 @@ impl RoomPreview { } }, is_world_readable: *room_info.history_visibility() == HistoryVisibility::WorldReadable, - num_joined_members, state, + is_direct, } } /// 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) fn from_known(room: &Room) -> Self { - Self::from_room_info(room.clone_info(), room.joined_members_count(), Some(room.state())) + pub(crate) async fn from_known(room: &Room) -> Self { + Self::from_room_info( + &room.client, + room.clone_info(), + room.joined_members_count(), + Some(room.state()), + ) + .await } #[instrument(skip(client))] @@ -160,12 +183,19 @@ impl RoomPreview { // The server returns a `Left` room state for rooms the user has not joined. Be // more precise than that, and set it to `None` if we haven't joined // that room. - let state = if client.get_room(&room_id).is_none() { + let cached_room = client.get_room(&room_id); + let state = if cached_room.is_none() { None } else { response.membership.map(|membership| RoomState::from(&membership)) }; + let is_direct = if let Some(cached_room) = cached_room { + cached_room.is_direct().await.ok() + } else { + None + }; + Ok(RoomPreview { room_id, canonical_alias: response.canonical_alias, @@ -177,6 +207,7 @@ impl RoomPreview { join_rule: response.join_rule, is_world_readable: response.world_readable, state, + is_direct, }) } @@ -219,6 +250,105 @@ impl RoomPreview { let state = client.get_room(room_id).map(|room| room.state()); - Ok(Self::from_room_info(room_info, num_joined_members, state)) + Ok(Self::from_room_info(client, room_info, num_joined_members, state).await) } } + +pub enum RoomPreviewActions { + Invited { join: JoinRoomPreviewAction, leave: LeaveRoomPreviewAction }, + Knocked { leave: LeaveRoomPreviewAction }, +} + +pub struct JoinRoomPreviewAction { + client: Client, +} + +impl JoinRoomPreviewAction { + pub fn new(client: Client) -> Self { + Self { client } + } + + pub async fn run(&self, room_preview: &RoomPreview) -> Result<(), RoomPreviewError> { + if room_preview.state.is_none() || matches!(room_preview.state, Some(RoomState::Invited)) { + match self.client.join_room_by_id(&room_preview.room_id).await { + Ok(room) => { + if room_preview.is_direct.unwrap_or(false) { + if let Some(e) = room.set_is_direct(true).await.err() { + warn!("error when setting room as direct: {e}"); + } + } + Ok(()) + } + Err(e) => Err(e.into()), + } + } else { + Err(RoomPreviewError::WrongRoomPreviewState(WrongRoomPreviewState::new( + "None or Invited", + room_preview.state, + ))) + } + } +} + +pub struct LeaveRoomPreviewAction { + client: Client, +} + +impl LeaveRoomPreviewAction { + pub fn new(client: Client) -> Self { + Self { client } + } + + pub async fn run(&self, room_preview: &RoomPreview) -> Result<(), RoomPreviewError> { + let Some(state) = room_preview.state else { + return Err(RoomPreviewError::WrongRoomPreviewState(WrongRoomPreviewState::new( + "Known room state (not None).", + None, + ))); + }; + + match state { + RoomState::Left => Err(RoomPreviewError::WrongRoomPreviewState(WrongRoomPreviewState::new( + "Joined, Invited or Knocked", + Some(state), + ))), + _ => { + let request = leave_room::v3::Request::new(room_preview.room_id.to_owned()); + self.client.send(request, None).await.map_err(Into::into)?; + self.client.base_client().room_left(&room_preview.room_id).await.map_err(Into::into)?; + Ok(()) + } + } + } +} + +#[derive(thiserror::Error, Debug)] +pub enum RoomPreviewError { + /// Attempted to call a method on a room preview that requires the user to + /// have a specific membership state, but the membership state is + /// different. + #[error("wrong room state: {0}")] + WrongRoomPreviewState(WrongRoomPreviewState), + + #[error(transparent)] + Generic(#[from] Box) +} + +#[derive(Debug, thiserror::Error)] +#[error("expected: {expected}, got: {got:?}")] +pub struct WrongRoomPreviewState { + expected: &'static str, + got: Option, +} + +impl WrongRoomPreviewState { + pub(crate) fn new(expected: &'static str, got: Option) -> Self { + Self { expected, got } + } +} + +impl From for RoomPreviewError { + fn from(e: Error) -> Self { + Self::Generic(Box::new(e)) + } +} \ No newline at end of file From d0048e792bb1848645f6f9cb6f56edb8eb275556 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jorge=20Mart=C3=ADn?= Date: Wed, 23 Oct 2024 13:44:49 +0200 Subject: [PATCH 3/4] chore(room_preview): Add knocking action to the `RoomPreviewActions` too --- bindings/matrix-sdk-ffi/src/client.rs | 15 +- bindings/matrix-sdk-ffi/src/error.rs | 10 +- bindings/matrix-sdk-ffi/src/room.rs | 2 +- bindings/matrix-sdk-ffi/src/room_list.rs | 12 +- bindings/matrix-sdk-ffi/src/room_preview.rs | 116 ++++++--------- crates/matrix-sdk-base/src/rooms/normal.rs | 2 - crates/matrix-sdk-base/src/sync.rs | 6 +- crates/matrix-sdk/src/client/mod.rs | 9 +- crates/matrix-sdk/src/error.rs | 10 +- crates/matrix-sdk/src/room/mod.rs | 2 +- crates/matrix-sdk/src/room_preview.rs | 121 +++------------ crates/matrix-sdk/tests/integration/main.rs | 1 + .../tests/integration/room_preview.rs | 139 ++++++++++++++++++ testing/matrix-sdk-test/src/lib.rs | 4 +- .../src/sync_builder/knocked_room.rs | 43 ++++++ .../matrix-sdk-test/src/sync_builder/mod.rs | 23 ++- 16 files changed, 313 insertions(+), 202 deletions(-) create mode 100644 crates/matrix-sdk/tests/integration/room_preview.rs create mode 100644 testing/matrix-sdk-test/src/sync_builder/knocked_room.rs diff --git a/bindings/matrix-sdk-ffi/src/client.rs b/bindings/matrix-sdk-ffi/src/client.rs index 3947dc7e882..371196013f5 100644 --- a/bindings/matrix-sdk-ffi/src/client.rs +++ b/bindings/matrix-sdk-ffi/src/client.rs @@ -2,7 +2,6 @@ use std::{ collections::HashMap, fmt::Debug, mem::ManuallyDrop, - ops::Deref, path::Path, sync::{Arc, RwLock}, }; @@ -1028,7 +1027,7 @@ impl Client { &self, room_id: String, via_servers: Vec, - ) -> Result { + ) -> Result, ClientError> { let room_id = RoomId::parse(&room_id).context("room_id is not a valid room id")?; let via_servers = via_servers @@ -1042,16 +1041,15 @@ impl Client { let room_id: &RoomId = &room_id; let sdk_room_preview = self.inner.get_room_preview(room_id.into(), via_servers).await?; - let client = (*self.inner.deref()).clone(); - RoomPreview::try_from_sdk(sdk_room_preview, client) + Ok(Arc::new(RoomPreview::from_sdk(sdk_room_preview))) } /// Given a room alias, get the preview of a room, to interact with it. pub async fn get_room_preview_from_room_alias( &self, room_alias: String, - ) -> Result { + ) -> Result, ClientError> { let room_alias = RoomAliasId::parse(&room_alias).context("room_alias is not a valid room alias")?; @@ -1060,9 +1058,8 @@ impl Client { let room_alias: &RoomAliasId = &room_alias; let sdk_room_preview = self.inner.get_room_preview(room_alias.into(), Vec::new()).await?; - let client = (*self.inner.deref()).clone(); - RoomPreview::try_from_sdk(sdk_room_preview, client) + Ok(Arc::new(RoomPreview::from_sdk(sdk_room_preview))) } /// Waits until an at least partially synced room is received, and returns @@ -1807,7 +1804,7 @@ impl From for SdkOidcPrompt { } /// The rule used for users wishing to join this room. -#[derive(uniffi::Enum)] +#[derive(Debug, Clone, uniffi::Enum)] pub enum JoinRule { /// Anyone can join the room without any prior action. Public, @@ -1836,7 +1833,7 @@ pub enum JoinRule { } /// An allow rule which defines a condition that allows joining a room. -#[derive(uniffi::Enum)] +#[derive(Debug, Clone, uniffi::Enum)] pub enum AllowRule { /// Only a member of the `room_id` Room can join the one this rule is used /// in. diff --git a/bindings/matrix-sdk-ffi/src/error.rs b/bindings/matrix-sdk-ffi/src/error.rs index 9c1f19fbb7f..6e4437b91b3 100644 --- a/bindings/matrix-sdk-ffi/src/error.rs +++ b/bindings/matrix-sdk-ffi/src/error.rs @@ -2,13 +2,13 @@ use std::{collections::HashMap, fmt, fmt::Display}; use matrix_sdk::{ encryption::CryptoStoreError, event_cache::EventCacheError, oidc::OidcError, reqwest, - room::edit::EditError, send_queue::RoomSendQueueError, HttpError, IdParseError, - NotificationSettingsError as SdkNotificationSettingsError, + room::edit::EditError, room_preview::WrongRoomPreviewState, send_queue::RoomSendQueueError, + HttpError, IdParseError, NotificationSettingsError as SdkNotificationSettingsError, QueueWedgeError as SdkQueueWedgeError, StoreError, }; use matrix_sdk_ui::{encryption_sync_service, notification_client, sync_service, timeline}; use uniffi::UnexpectedUniFFICallbackError; -use matrix_sdk::room_preview::RoomPreviewError; + use crate::room_list::RoomListError; #[derive(Debug, thiserror::Error)] @@ -155,8 +155,8 @@ impl From for ClientError { } } -impl From for ClientError { - fn from(e: RoomSendQueueError) -> Self { +impl From for ClientError { + fn from(e: WrongRoomPreviewState) -> Self { Self::new(e) } } diff --git a/bindings/matrix-sdk-ffi/src/room.rs b/bindings/matrix-sdk-ffi/src/room.rs index d10aea0e4ba..9f11366a53c 100644 --- a/bindings/matrix-sdk-ffi/src/room.rs +++ b/bindings/matrix-sdk-ffi/src/room.rs @@ -45,7 +45,7 @@ use crate::{ TaskHandle, }; -#[derive(Debug, uniffi::Enum)] +#[derive(Debug, Clone, uniffi::Enum)] pub enum Membership { Invited, Joined, diff --git a/bindings/matrix-sdk-ffi/src/room_list.rs b/bindings/matrix-sdk-ffi/src/room_list.rs index 60abac54b61..dff2eb5eb40 100644 --- a/bindings/matrix-sdk-ffi/src/room_list.rs +++ b/bindings/matrix-sdk-ffi/src/room_list.rs @@ -16,9 +16,7 @@ use matrix_sdk_ui::{ timeline::default_event_filter, unable_to_decrypt_hook::UtdHookManager, }; -use ruma::{ - IdParseError, OwnedRoomOrAliasId, OwnedServerName, RoomAliasId, RoomOrAliasId, ServerName, -}; +use ruma::{OwnedRoomOrAliasId, OwnedServerName, ServerName}; use tokio::sync::RwLock; use crate::{ @@ -595,10 +593,10 @@ impl RoomListItem { Ok(Arc::new(Room::new(self.inner.inner_room().clone()))) } - /// Builds a `RoomPreview` from a room list item. This is intended for rooms - /// with [`Membership::Invite`] or [`Membership::Knocked`], + /// Builds a `RoomPreview` from a room list item. This is intended for + /// invited or knocked rooms. /// - /// An error will be returned if the room is a state other than invited + /// An error will be returned if the room is in a state other than invited /// or knocked. async fn preview_room(&self, via: Vec) -> Result, ClientError> { let membership = self.membership(); @@ -624,7 +622,7 @@ impl RoomListItem { }; let room_preview = client.get_room_preview(&room_or_alias_id, server_names).await?; - Ok(Arc::new(RoomPreview::try_from_sdk(room_preview, client))) + Ok(Arc::new(RoomPreview::from_sdk(room_preview))) } /// Build a full `Room` FFI object, filling its associated timeline. diff --git a/bindings/matrix-sdk-ffi/src/room_preview.rs b/bindings/matrix-sdk-ffi/src/room_preview.rs index 2c37af788c2..77a110b4f76 100644 --- a/bindings/matrix-sdk-ffi/src/room_preview.rs +++ b/bindings/matrix-sdk-ffi/src/room_preview.rs @@ -1,81 +1,48 @@ -use matrix_sdk::{ - room_preview::{ - JoinRoomPreviewAction as SdkJoinRoomPreviewAction, LeaveRoomPreviewAction as SdkLeaveRoomPreviewAction, RoomPreview as SdkRoomPreview, - RoomPreviewActions, - }, - Client, RoomState, -}; +use std::sync::Arc; + +use matrix_sdk::room_preview::RoomPreview as SdkRoomPreview; use ruma::space::SpaceRoomJoinRule; use crate::{client::JoinRule, error::ClientError, room::Membership}; -#[derive(uniffi::Record)] -pub struct RoomPreview { - pub info: RoomPreviewInfo, - pub room_preview_actions: RoomPreviewActions, - pub(crate) sdk_info: matrix_sdk::room_preview::RoomPreview, -} - -#[derive(uniffi::Enum)] -pub enum RoomPreviewAction { - Invited { join: JoinRoomPreviewAction, leave: LeaveRoomPreviewAction }, - Knocked { leave: LeaveRoomPreviewAction }, -} - +/// A room preview for a room. It's intended to be used to represent rooms that +/// aren't joined yet. #[derive(uniffi::Object)] -pub struct JoinRoomPreviewAction { - inner: SdkJoinRoomPreviewAction, +pub struct RoomPreview { + inner: Arc, } #[matrix_sdk_ffi_macros::export] -impl JoinRoomPreviewAction { - async fn run(&self) { - self.inner.run().unwrap() +impl RoomPreview { + /// Returns the room info the preview contains. + pub fn info(&self) -> RoomPreviewInfo { + let info = &self.inner; + RoomPreviewInfo { + room_id: info.room_id.to_string(), + canonical_alias: info.canonical_alias.as_ref().map(|alias| alias.to_string()), + name: info.name.clone(), + topic: info.topic.clone(), + avatar_url: info.avatar_url.as_ref().map(|url| url.to_string()), + num_joined_members: info.num_joined_members, + room_type: info.room_type.as_ref().map(|room_type| room_type.to_string()), + is_history_world_readable: info.is_world_readable, + membership: info.state.map(|state| state.into()), + join_rule: info.join_rule.clone().into(), + } } -} -impl From for RoomPreviewAction { - fn from(actions: RoomPreviewActions) -> Self { - match actions { - RoomPreviewActions::Invited { join, leave } => Self::Invited { join, leave }, - RoomPreviewActions::Knocked { leave } => Self::Knocked { leave }, - } + /// Leave the room if the room preview state is either joined, invited or + /// knocked. + /// + /// Will return an error otherwise. + pub async fn leave(&self) -> Result<(), ClientError> { + self.inner.leave().await.map_err(Into::into) } } -struct - impl RoomPreview { - pub(crate) fn try_from_sdk(info: SdkRoomPreview, client: Client) -> Result { - let info = RoomPreviewInfo { - room_id: info.room_id.to_string(), - canonical_alias: info.canonical_alias.map(|alias| alias.to_string()), - name: info.name, - topic: info.topic, - avatar_url: info.avatar_url.map(|url| url.to_string()), - num_joined_members: info.num_joined_members, - room_type: info.room_type.map(|room_type| room_type.to_string()), - is_history_world_readable: info.is_world_readable, - membership: info.state.map(|state| state.into()).unwrap_or_else(|| Membership::Left), - join_rule: info.join_rule.into(), - }; - - let room_preview_actions = match info.membership { - Membership::Invited => RoomPreviewActions::Invited { - join: JoinRoomPreviewAction::new(client.clone()), - leave: LeaveRoomPreviewAction::new(client.clone()), - }, - Membership::Knocked => { - RoomPreviewActions::Knocked { leave: LeaveRoomPreviewAction::new(client.clone()) } - } - _ => { - return Err(ClientError::new(format!( - "The room preview had membership {:?} instead of Invited or Knocked.", - info.membership - ))) - } - }; - Ok(Self { info, room_preview_actions }) + pub(crate) fn from_sdk(room_preview: SdkRoomPreview) -> Self { + Self { inner: Arc::new(room_preview) } } } @@ -98,23 +65,28 @@ pub struct RoomPreviewInfo { pub room_type: Option, /// Is the history world-readable for this room? pub is_history_world_readable: bool, - /// Is the room joined by the current user? - pub membership: Membership, - /// is the join rule public for this room? + /// The membership state for the current user, if known. + pub membership: Option, + /// The join rule for this room (private, public, knock, etc.). pub join_rule: JoinRule, } impl From for JoinRule { fn from(join_rule: SpaceRoomJoinRule) -> Self { match join_rule { - SpaceRoomJoinRule::Public => JoinRule::Public, - SpaceRoomJoinRule::Private => JoinRule::Private, SpaceRoomJoinRule::Invite => JoinRule::Invite, SpaceRoomJoinRule::Knock => JoinRule::Knock, - SpaceRoomJoinRule::KnockRestricted => JoinRule::KnockRestricted { rules: Vec::new() }, + SpaceRoomJoinRule::Private => JoinRule::Private, SpaceRoomJoinRule::Restricted => JoinRule::Restricted { rules: Vec::new() }, - // For the `_Custom` case, assume it's private - _ => JoinRule::Private, + SpaceRoomJoinRule::KnockRestricted => JoinRule::KnockRestricted { rules: Vec::new() }, + SpaceRoomJoinRule::Public => JoinRule::Public, + _ => { + // Since we have no way to get the _Custom contents, assume it's private. + // Warning! If new join rules are introduced we may be mapping wrong values + // here, but there's no way to match + // `SpaceRoomJoinRule::_Custom(_)` and have an exhaustive match. + JoinRule::Private + } } } } diff --git a/crates/matrix-sdk-base/src/rooms/normal.rs b/crates/matrix-sdk-base/src/rooms/normal.rs index cb0ee349279..3d426261f2b 100644 --- a/crates/matrix-sdk-base/src/rooms/normal.rs +++ b/crates/matrix-sdk-base/src/rooms/normal.rs @@ -186,8 +186,6 @@ impl RoomSummary { /// Enum keeping track in which state the room is, e.g. if our own user is /// joined, RoomState::Invited, or has left the room. #[derive(Clone, Copy, Debug, Eq, PartialEq, Serialize, Deserialize)] -#[cfg(feature = "uniffi")] -#[derive(uniffi::Enum)] pub enum RoomState { /// The room is in a joined state. Joined, diff --git a/crates/matrix-sdk-base/src/sync.rs b/crates/matrix-sdk-base/src/sync.rs index 4d47cd904ce..36ee7421ef7 100644 --- a/crates/matrix-sdk-base/src/sync.rs +++ b/crates/matrix-sdk-base/src/sync.rs @@ -19,7 +19,7 @@ use std::{collections::BTreeMap, fmt}; use matrix_sdk_common::{debug::DebugRawEvent, deserialized_responses::SyncTimelineEvent}; use ruma::{ api::client::sync::sync_events::{ - v3::{InvitedRoom as InvitedRoomUpdate, KnockedRoom}, + v3::{InvitedRoom as InvitedRoomUpdate, KnockedRoom as KnockedRoomUpdate}, UnreadNotificationsCount as RumaUnreadNotificationsCount, }, events::{ @@ -78,7 +78,7 @@ pub struct RoomUpdates { /// The rooms that the user has been invited to. pub invite: BTreeMap, /// The rooms that the user has knocked on. - pub knocked: BTreeMap, + pub knocked: BTreeMap, } impl RoomUpdates { @@ -254,7 +254,7 @@ impl<'a> fmt::Debug for DebugInvitedRoomUpdates<'a> { } } -struct DebugKnockedRoomUpdates<'a>(&'a BTreeMap); +struct DebugKnockedRoomUpdates<'a>(&'a BTreeMap); #[cfg(not(tarpaulin_include))] impl<'a> fmt::Debug for DebugKnockedRoomUpdates<'a> { diff --git a/crates/matrix-sdk/src/client/mod.rs b/crates/matrix-sdk/src/client/mod.rs index 7066fcba545..ce8afc85ba0 100644 --- a/crates/matrix-sdk/src/client/mod.rs +++ b/crates/matrix-sdk/src/client/mod.rs @@ -54,7 +54,7 @@ use ruma::{ }, filter::{create_filter::v3::Request as FilterUploadRequest, FilterDefinition}, knock::knock_room, - membership::{join_room_by_id, join_room_by_id_or_alias}, + membership::{join_room_by_id, join_room_by_id_or_alias, leave_room}, room::create_room, session::login::v3::DiscoveryInfo, sync::sync_events, @@ -1208,6 +1208,13 @@ impl Client { Ok(Room::new(self.clone(), base_room)) } + /// Leave a room given its `RoomId`. + pub async fn leave_room(&self, room_id: &RoomId) -> Result<()> { + let request = leave_room::v3::Request::new(room_id.to_owned()); + self.send(request, None).await?; + self.base_client().room_left(room_id).await.map_err(Into::into) + } + /// Search the homeserver's directory of public rooms. /// /// Sends a request to "_matrix/client/r0/publicRooms", returns diff --git a/crates/matrix-sdk/src/error.rs b/crates/matrix-sdk/src/error.rs index 86c31496f99..c65e2cd132d 100644 --- a/crates/matrix-sdk/src/error.rs +++ b/crates/matrix-sdk/src/error.rs @@ -45,7 +45,9 @@ use serde_json::Error as JsonError; use thiserror::Error; use url::ParseError as UrlParseError; -use crate::{event_cache::EventCacheError, store_locks::LockStoreError}; +use crate::{ + event_cache::EventCacheError, room_preview::WrongRoomPreviewState, store_locks::LockStoreError, +}; /// Result type of the matrix-sdk. pub type Result = std::result::Result; @@ -348,6 +350,12 @@ pub enum Error { #[error("wrong room state: {0}")] WrongRoomState(WrongRoomState), + /// Attempted to call a method on a room preview that requires the user to + /// have a specific membership state in the room, but the membership + /// state is different or none. + #[error("wrong room state: {0}")] + WrongRoomPreviewState(WrongRoomPreviewState), + /// Session callbacks have been set multiple times. #[error("session callbacks have been set multiple times")] MultipleSessionCallbacks, diff --git a/crates/matrix-sdk/src/room/mod.rs b/crates/matrix-sdk/src/room/mod.rs index b3a0e01ed01..e68d82004a0 100644 --- a/crates/matrix-sdk/src/room/mod.rs +++ b/crates/matrix-sdk/src/room/mod.rs @@ -47,7 +47,7 @@ use ruma::{ membership::{ ban_user, forget_room, get_member_events, invite_user::{self, v3::InvitationRecipient}, - join_room_by_id, kick_user, leave_room, unban_user, Invite3pid, + kick_user, leave_room, unban_user, Invite3pid, }, message::send_message_event, read_marker::set_read_marker, diff --git a/crates/matrix-sdk/src/room_preview.rs b/crates/matrix-sdk/src/room_preview.rs index 1ebdd7dd982..ca6db08a1f8 100644 --- a/crates/matrix-sdk/src/room_preview.rs +++ b/crates/matrix-sdk/src/room_preview.rs @@ -18,14 +18,9 @@ //! This offers a few capabilities for previewing the content of the room as //! well. -use anyhow::anyhow; -use mas_oidc_client::types::errors::ClientError; use matrix_sdk_base::{RoomInfo, RoomState}; use ruma::{ - api::client::{ - membership::{joined_members, leave_room}, - state::get_state_events, - }, + api::client::{membership::joined_members, state::get_state_events}, events::room::{history_visibility::HistoryVisibility, join_rules::JoinRule}, room::RoomType, space::SpaceRoomJoinRule, @@ -34,10 +29,7 @@ use ruma::{ use tokio::try_join; use tracing::{instrument, warn}; -use crate::{ - error::WrongRoomState, - Client, Error, Room, -}; +use crate::{Client, Error, Room}; /// The preview of a room, be it invited/joined/left, or not. #[derive(Debug, Clone)] @@ -80,9 +72,26 @@ pub struct RoomPreview { /// The `m.room.direct` state of the room, if known. pub is_direct: Option, + + /// The [`Client`] that can be used to perform actions given the room + /// preview. + pub(crate) client: Client, } impl RoomPreview { + /// Leave a room using its [`RoomPreview`]. + pub async fn leave(&self) -> Result<(), Error> { + match self.state { + Some(RoomState::Left) | None => { + Err(Error::WrongRoomPreviewState(WrongRoomPreviewState { + expected: "Invited, Knocked or Joined", + got: self.state, + })) + } + Some(_) => self.client.leave_room(&self.room_id).await.map_err(Into::into), + } + } + /// Constructs a [`RoomPreview`] from the associated room info. /// /// Note: not using the room info's state/count of joined members, because @@ -99,6 +108,7 @@ impl RoomPreview { None }; RoomPreview { + client: client.clone(), room_id: room_info.room_id().to_owned(), canonical_alias: room_info.canonical_alias().map(ToOwned::to_owned), name: room_info.name().map(ToOwned::to_owned), @@ -197,6 +207,7 @@ impl RoomPreview { }; Ok(RoomPreview { + client: client.clone(), room_id, canonical_alias: response.canonical_alias, name: response.name, @@ -254,86 +265,7 @@ impl RoomPreview { } } -pub enum RoomPreviewActions { - Invited { join: JoinRoomPreviewAction, leave: LeaveRoomPreviewAction }, - Knocked { leave: LeaveRoomPreviewAction }, -} - -pub struct JoinRoomPreviewAction { - client: Client, -} - -impl JoinRoomPreviewAction { - pub fn new(client: Client) -> Self { - Self { client } - } - - pub async fn run(&self, room_preview: &RoomPreview) -> Result<(), RoomPreviewError> { - if room_preview.state.is_none() || matches!(room_preview.state, Some(RoomState::Invited)) { - match self.client.join_room_by_id(&room_preview.room_id).await { - Ok(room) => { - if room_preview.is_direct.unwrap_or(false) { - if let Some(e) = room.set_is_direct(true).await.err() { - warn!("error when setting room as direct: {e}"); - } - } - Ok(()) - } - Err(e) => Err(e.into()), - } - } else { - Err(RoomPreviewError::WrongRoomPreviewState(WrongRoomPreviewState::new( - "None or Invited", - room_preview.state, - ))) - } - } -} - -pub struct LeaveRoomPreviewAction { - client: Client, -} - -impl LeaveRoomPreviewAction { - pub fn new(client: Client) -> Self { - Self { client } - } - - pub async fn run(&self, room_preview: &RoomPreview) -> Result<(), RoomPreviewError> { - let Some(state) = room_preview.state else { - return Err(RoomPreviewError::WrongRoomPreviewState(WrongRoomPreviewState::new( - "Known room state (not None).", - None, - ))); - }; - - match state { - RoomState::Left => Err(RoomPreviewError::WrongRoomPreviewState(WrongRoomPreviewState::new( - "Joined, Invited or Knocked", - Some(state), - ))), - _ => { - let request = leave_room::v3::Request::new(room_preview.room_id.to_owned()); - self.client.send(request, None).await.map_err(Into::into)?; - self.client.base_client().room_left(&room_preview.room_id).await.map_err(Into::into)?; - Ok(()) - } - } - } -} - -#[derive(thiserror::Error, Debug)] -pub enum RoomPreviewError { - /// Attempted to call a method on a room preview that requires the user to - /// have a specific membership state, but the membership state is - /// different. - #[error("wrong room state: {0}")] - WrongRoomPreviewState(WrongRoomPreviewState), - - #[error(transparent)] - Generic(#[from] Box) -} - +/// An unexpected room preview state was found. #[derive(Debug, thiserror::Error)] #[error("expected: {expected}, got: {got:?}")] pub struct WrongRoomPreviewState { @@ -342,13 +274,8 @@ pub struct WrongRoomPreviewState { } impl WrongRoomPreviewState { - pub(crate) fn new(expected: &'static str, got: Option) -> Self { + /// Instantiate a new + pub fn new(expected: &'static str, got: Option) -> Self { Self { expected, got } } } - -impl From for RoomPreviewError { - fn from(e: Error) -> Self { - Self::Generic(Box::new(e)) - } -} \ No newline at end of file diff --git a/crates/matrix-sdk/tests/integration/main.rs b/crates/matrix-sdk/tests/integration/main.rs index 9192d0233f9..13cebccde96 100644 --- a/crates/matrix-sdk/tests/integration/main.rs +++ b/crates/matrix-sdk/tests/integration/main.rs @@ -20,6 +20,7 @@ mod media; mod notification; mod refresh_token; mod room; +mod room_preview; mod send_queue; #[cfg(feature = "experimental-widgets")] mod widget; diff --git a/crates/matrix-sdk/tests/integration/room_preview.rs b/crates/matrix-sdk/tests/integration/room_preview.rs new file mode 100644 index 00000000000..853640691d2 --- /dev/null +++ b/crates/matrix-sdk/tests/integration/room_preview.rs @@ -0,0 +1,139 @@ +use assert_matches2::assert_matches; +use matrix_sdk::{config::SyncSettings, test_utils::logged_in_client_with_server, Error}; +use matrix_sdk_base::RoomState; +use matrix_sdk_test::{ + async_test, InvitedRoomBuilder, JoinedRoomBuilder, KnockedRoomBuilder, SyncResponseBuilder, +}; +use ruma::{room_id, space::SpaceRoomJoinRule, RoomId}; +use serde_json::json; +use wiremock::{ + matchers::{header, method, path_regex}, + Mock, MockServer, ResponseTemplate, +}; + +use crate::mock_sync; + +#[async_test] +async fn test_room_preview_leave_invited() { + let (client, server) = logged_in_client_with_server().await; + let room_id = room_id!("!room:localhost"); + + let mut sync_builder = SyncResponseBuilder::new(); + sync_builder.add_invited_room(InvitedRoomBuilder::new(room_id)); + + mock_sync(&server, sync_builder.build_json_sync_response(), None).await; + client.sync_once(SyncSettings::default()).await.unwrap(); + server.reset().await; + + mock_leave(room_id, &server).await; + + let room_preview = client.get_room_preview(room_id.into(), Vec::new()).await.unwrap(); + assert_eq!(room_preview.state.unwrap(), RoomState::Invited); + + room_preview.leave().await.unwrap(); + + assert_eq!(client.get_room(room_id).unwrap().state(), RoomState::Left); +} + +#[async_test] +async fn test_room_preview_leave_knocked() { + let (client, server) = logged_in_client_with_server().await; + let room_id = room_id!("!room:localhost"); + + let mut sync_builder = SyncResponseBuilder::new(); + sync_builder.add_knocked_room(KnockedRoomBuilder::new(room_id)); + + mock_sync(&server, sync_builder.build_json_sync_response(), None).await; + client.sync_once(SyncSettings::default()).await.unwrap(); + server.reset().await; + + mock_leave(room_id, &server).await; + + let room_preview = client.get_room_preview(room_id.into(), Vec::new()).await.unwrap(); + assert_eq!(room_preview.state.unwrap(), RoomState::Knocked); + + room_preview.leave().await.unwrap(); + + assert_eq!(client.get_room(room_id).unwrap().state(), RoomState::Left); +} + +#[async_test] +async fn test_room_preview_leave_joined() { + let (client, server) = logged_in_client_with_server().await; + let room_id = room_id!("!room:localhost"); + + let mut sync_builder = SyncResponseBuilder::new(); + sync_builder.add_joined_room(JoinedRoomBuilder::new(room_id)); + + mock_sync(&server, sync_builder.build_json_sync_response(), None).await; + client.sync_once(SyncSettings::default()).await.unwrap(); + server.reset().await; + + mock_leave(room_id, &server).await; + + let room_preview = client.get_room_preview(room_id.into(), Vec::new()).await.unwrap(); + assert_eq!(room_preview.state.unwrap(), RoomState::Joined); + + room_preview.leave().await.unwrap(); + + assert_eq!(client.get_room(room_id).unwrap().state(), RoomState::Left); +} + +#[async_test] +async fn test_room_preview_leave_unknown_room_fails() { + let (client, server) = logged_in_client_with_server().await; + let room_id = room_id!("!room:localhost"); + + mock_unknown_summary(room_id, None, SpaceRoomJoinRule::Knock, &server).await; + mock_leave(room_id, &server).await; + + let room_preview = client.get_room_preview(room_id.into(), Vec::new()).await.unwrap(); + assert!(room_preview.state.is_none()); + + let error = room_preview.leave().await.err(); + + assert_matches!(error, Some(Error::WrongRoomPreviewState(_))); +} + +async fn mock_leave(room_id: &RoomId, server: &MockServer) { + Mock::given(method("POST")) + .and(path_regex(r"^/_matrix/client/r0/rooms/.*/leave")) + .and(header("authorization", "Bearer 1234")) + .respond_with(ResponseTemplate::new(200).set_body_json(json!({ + "room_id": room_id, + }))) + .mount(server) + .await +} + +async fn mock_unknown_summary( + room_id: &RoomId, + alias: Option, + join_rule: SpaceRoomJoinRule, + server: &MockServer, +) { + let body = if let Some(alias) = alias { + json!({ + "room_id": room_id, + "canonical_alias": alias, + "guest_can_join": true, + "num_joined_members": 1, + "world_readable": true, + "join_rule": join_rule, + }) + } else { + json!({ + "room_id": room_id, + "guest_can_join": true, + "num_joined_members": 1, + "world_readable": true, + "join_rule": join_rule, + }) + }; + Mock::given(method("GET")) + .and(path_regex(r"^/_matrix/client/unstable/im.nheko.summary/rooms/.*/summary")) + .and(header("authorization", "Bearer 1234")) + .respond_with(ResponseTemplate::new(200).set_body_json(body)) + .mount(server) + .await +} diff --git a/testing/matrix-sdk-test/src/lib.rs b/testing/matrix-sdk-test/src/lib.rs index 0a7c99e2384..1b06e142d56 100644 --- a/testing/matrix-sdk-test/src/lib.rs +++ b/testing/matrix-sdk-test/src/lib.rs @@ -122,8 +122,8 @@ pub use self::{ event_builder::EventBuilder, sync_builder::{ bulk_room_members, EphemeralTestEvent, GlobalAccountDataTestEvent, InvitedRoomBuilder, - JoinedRoomBuilder, LeftRoomBuilder, PresenceTestEvent, RoomAccountDataTestEvent, - StateTestEvent, StrippedStateTestEvent, SyncResponseBuilder, + JoinedRoomBuilder, KnockedRoomBuilder, LeftRoomBuilder, PresenceTestEvent, + RoomAccountDataTestEvent, StateTestEvent, StrippedStateTestEvent, SyncResponseBuilder, }, }; diff --git a/testing/matrix-sdk-test/src/sync_builder/knocked_room.rs b/testing/matrix-sdk-test/src/sync_builder/knocked_room.rs new file mode 100644 index 00000000000..0df9a4e73a6 --- /dev/null +++ b/testing/matrix-sdk-test/src/sync_builder/knocked_room.rs @@ -0,0 +1,43 @@ +use ruma::{ + api::client::sync::sync_events::v3::KnockedRoom, events::AnyStrippedStateEvent, serde::Raw, + OwnedRoomId, RoomId, +}; + +use super::StrippedStateTestEvent; +use crate::DEFAULT_TEST_ROOM_ID; + +pub struct KnockedRoomBuilder { + pub(super) room_id: OwnedRoomId, + pub(super) inner: KnockedRoom, +} + +impl KnockedRoomBuilder { + /// Create a new `KnockedRoomBuilder` for the given room ID. + /// + /// If the room ID is [`DEFAULT_TEST_ROOM_ID`], + /// [`KnockedRoomBuilder::default()`] can be used instead. + pub fn new(room_id: &RoomId) -> Self { + Self { room_id: room_id.to_owned(), inner: Default::default() } + } + + /// Add an event to the state. + pub fn add_state_event(mut self, event: StrippedStateTestEvent) -> Self { + self.inner.knock_state.events.push(event.into_raw_event()); + self + } + + /// Add events to the state in bulk. + pub fn add_state_bulk(mut self, events: I) -> Self + where + I: IntoIterator>, + { + self.inner.knock_state.events.extend(events); + self + } +} + +impl Default for KnockedRoomBuilder { + fn default() -> Self { + Self::new(&DEFAULT_TEST_ROOM_ID) + } +} diff --git a/testing/matrix-sdk-test/src/sync_builder/mod.rs b/testing/matrix-sdk-test/src/sync_builder/mod.rs index ea6d05bd8cb..919fe6356d6 100644 --- a/testing/matrix-sdk-test/src/sync_builder/mod.rs +++ b/testing/matrix-sdk-test/src/sync_builder/mod.rs @@ -4,7 +4,7 @@ use http::Response; use ruma::{ api::{ client::sync::sync_events::v3::{ - InvitedRoom, JoinedRoom, LeftRoom, Response as SyncResponse, + InvitedRoom, JoinedRoom, KnockedRoom, LeftRoom, Response as SyncResponse, }, IncomingResponse, }, @@ -19,12 +19,14 @@ use super::test_json; mod bulk; mod invited_room; mod joined_room; +mod knocked_room; mod left_room; mod test_event; pub use bulk::bulk_room_members; pub use invited_room::InvitedRoomBuilder; pub use joined_room::JoinedRoomBuilder; +pub use knocked_room::KnockedRoomBuilder; pub use left_room::LeftRoomBuilder; pub use test_event::{ EphemeralTestEvent, GlobalAccountDataTestEvent, PresenceTestEvent, RoomAccountDataTestEvent, @@ -45,6 +47,8 @@ pub struct SyncResponseBuilder { invited_rooms: HashMap, /// Updates to left `Room`s. left_rooms: HashMap, + /// Updates to knocked `Room`s. + knocked_rooms: HashMap, /// Events that determine the presence state of a user. presence: Vec>, /// Global account data events. @@ -68,6 +72,7 @@ impl SyncResponseBuilder { pub fn add_joined_room(&mut self, room: JoinedRoomBuilder) -> &mut Self { self.invited_rooms.remove(&room.room_id); self.left_rooms.remove(&room.room_id); + self.knocked_rooms.remove(&room.room_id); self.joined_rooms.insert(room.room_id, room.inner); self } @@ -79,6 +84,7 @@ impl SyncResponseBuilder { pub fn add_invited_room(&mut self, room: InvitedRoomBuilder) -> &mut Self { self.joined_rooms.remove(&room.room_id); self.left_rooms.remove(&room.room_id); + self.knocked_rooms.remove(&room.room_id); self.invited_rooms.insert(room.room_id, room.inner); self } @@ -90,10 +96,23 @@ impl SyncResponseBuilder { pub fn add_left_room(&mut self, room: LeftRoomBuilder) -> &mut Self { self.joined_rooms.remove(&room.room_id); self.invited_rooms.remove(&room.room_id); + self.knocked_rooms.remove(&room.room_id); self.left_rooms.insert(room.room_id, room.inner); self } + /// Add a knocked room to the next sync response. + /// + /// If a room with the same room ID already exists, it is replaced by this + /// one. + pub fn add_knocked_room(&mut self, room: KnockedRoomBuilder) -> &mut Self { + self.joined_rooms.remove(&room.room_id); + self.invited_rooms.remove(&room.room_id); + self.left_rooms.remove(&room.room_id); + self.knocked_rooms.insert(room.room_id, room.inner); + self + } + /// Add a presence event. pub fn add_presence_event(&mut self, event: PresenceTestEvent) -> &mut Self { let val = match event { @@ -169,6 +188,7 @@ impl SyncResponseBuilder { "invite": self.invited_rooms, "join": self.joined_rooms, "leave": self.left_rooms, + "knock": self.knocked_rooms, }, "to_device": { "events": [] @@ -215,6 +235,7 @@ impl SyncResponseBuilder { self.invited_rooms.clear(); self.joined_rooms.clear(); self.left_rooms.clear(); + self.knocked_rooms.clear(); self.presence.clear(); } } From c7f5638877f113b4b374a452a2f5561cf11f1f69 Mon Sep 17 00:00:00 2001 From: Benjamin Bouvier Date: Fri, 25 Oct 2024 13:33:14 +0200 Subject: [PATCH 4/4] benjamin's review; let's squash this MR --- bindings/matrix-sdk-ffi/src/client.rs | 15 +++-- bindings/matrix-sdk-ffi/src/error.rs | 10 +-- bindings/matrix-sdk-ffi/src/lib.rs | 2 - bindings/matrix-sdk-ffi/src/room_list.rs | 25 ++++++-- bindings/matrix-sdk-ffi/src/room_preview.rs | 62 +++++++++++++------ crates/matrix-sdk/src/client/mod.rs | 9 +-- crates/matrix-sdk/src/error.rs | 10 +-- crates/matrix-sdk/src/room_preview.rs | 56 +++-------------- .../tests/integration/room_preview.rs | 18 +++--- 9 files changed, 95 insertions(+), 112 deletions(-) diff --git a/bindings/matrix-sdk-ffi/src/client.rs b/bindings/matrix-sdk-ffi/src/client.rs index 371196013f5..f44134fa0ba 100644 --- a/bindings/matrix-sdk-ffi/src/client.rs +++ b/bindings/matrix-sdk-ffi/src/client.rs @@ -1040,9 +1040,9 @@ impl Client { // rustc win that one fight. let room_id: &RoomId = &room_id; - let sdk_room_preview = self.inner.get_room_preview(room_id.into(), via_servers).await?; + let room_preview = self.inner.get_room_preview(room_id.into(), via_servers).await?; - Ok(Arc::new(RoomPreview::from_sdk(sdk_room_preview))) + Ok(Arc::new(RoomPreview::new(self.inner.clone(), room_preview))) } /// Given a room alias, get the preview of a room, to interact with it. @@ -1057,9 +1057,9 @@ impl Client { // rustc win that one fight. let room_alias: &RoomAliasId = &room_alias; - let sdk_room_preview = self.inner.get_room_preview(room_alias.into(), Vec::new()).await?; + let room_preview = self.inner.get_room_preview(room_alias.into(), Vec::new()).await?; - Ok(Arc::new(RoomPreview::from_sdk(sdk_room_preview))) + Ok(Arc::new(RoomPreview::new(self.inner.clone(), room_preview))) } /// Waits until an at least partially synced room is received, and returns @@ -1830,6 +1830,12 @@ pub enum JoinRule { /// conditions described in a set of [`AllowRule`]s, or they can request /// an invite to the room. KnockRestricted { rules: Vec }, + + /// A custom join rule, up for interpretation by the consumer. + Custom { + /// The string representation for this custom rule. + repr: String, + }, } /// An allow rule which defines a condition that allows joining a room. @@ -1857,6 +1863,7 @@ impl TryFrom for ruma::events::room::join_rules::JoinRule { let rules = allow_rules_from(rules)?; Ok(Self::KnockRestricted(ruma::events::room::join_rules::Restricted::new(rules))) } + JoinRule::Custom { repr } => Ok(serde_json::from_str(&repr)?), } } } diff --git a/bindings/matrix-sdk-ffi/src/error.rs b/bindings/matrix-sdk-ffi/src/error.rs index 6e4437b91b3..e644d7974d8 100644 --- a/bindings/matrix-sdk-ffi/src/error.rs +++ b/bindings/matrix-sdk-ffi/src/error.rs @@ -2,8 +2,8 @@ use std::{collections::HashMap, fmt, fmt::Display}; use matrix_sdk::{ encryption::CryptoStoreError, event_cache::EventCacheError, oidc::OidcError, reqwest, - room::edit::EditError, room_preview::WrongRoomPreviewState, send_queue::RoomSendQueueError, - HttpError, IdParseError, NotificationSettingsError as SdkNotificationSettingsError, + room::edit::EditError, send_queue::RoomSendQueueError, HttpError, IdParseError, + NotificationSettingsError as SdkNotificationSettingsError, QueueWedgeError as SdkQueueWedgeError, StoreError, }; use matrix_sdk_ui::{encryption_sync_service, notification_client, sync_service, timeline}; @@ -155,12 +155,6 @@ impl From for ClientError { } } -impl From for ClientError { - fn from(e: WrongRoomPreviewState) -> Self { - Self::new(e) - } -} - /// Bindings version of the sdk type replacing OwnedUserId/DeviceIds with simple /// String. /// diff --git a/bindings/matrix-sdk-ffi/src/lib.rs b/bindings/matrix-sdk-ffi/src/lib.rs index 8916ee52282..0e404a848ff 100644 --- a/bindings/matrix-sdk-ffi/src/lib.rs +++ b/bindings/matrix-sdk-ffi/src/lib.rs @@ -18,8 +18,6 @@ mod platform; mod room; mod room_directory_search; mod room_info; - -#[allow(deprecated)] mod room_list; mod room_member; mod room_preview; diff --git a/bindings/matrix-sdk-ffi/src/room_list.rs b/bindings/matrix-sdk-ffi/src/room_list.rs index dff2eb5eb40..27bc20dcc34 100644 --- a/bindings/matrix-sdk-ffi/src/room_list.rs +++ b/bindings/matrix-sdk-ffi/src/room_list.rs @@ -1,4 +1,12 @@ -use std::{fmt::Debug, mem::MaybeUninit, ptr::addr_of_mut, sync::Arc, time::Duration}; +#![allow(deprecated)] + +use std::{ + fmt::Debug, + mem::{ManuallyDrop, MaybeUninit}, + ptr::addr_of_mut, + sync::Arc, + time::Duration, +}; use eyeball_im::VectorDiff; use futures_util::{pin_mut, StreamExt, TryFutureExt}; @@ -599,6 +607,13 @@ impl RoomListItem { /// An error will be returned if the room is in a state other than invited /// or knocked. async fn preview_room(&self, via: Vec) -> Result, ClientError> { + // Validate parameters first. + let server_names: Vec = via + .into_iter() + .map(|server| ServerName::parse(server).map_err(ClientError::from)) + .collect::>()?; + + // Validate internal room state. let membership = self.membership(); if !matches!(membership, Membership::Invited | Membership::Knocked) { return Err(RoomListError::IncorrectRoomMembership { @@ -608,21 +623,19 @@ impl RoomListItem { .into()); } + // 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: OwnedRoomOrAliasId = alias.into(); (room_or_alias_id, Vec::new()) } else { let room_or_alias_id: OwnedRoomOrAliasId = self.inner.id().to_owned().into(); - let server_names: Vec = via - .into_iter() - .map(|server| ServerName::parse(server).map_err(ClientError::from)) - .collect::>()?; (room_or_alias_id, server_names) }; let room_preview = client.get_room_preview(&room_or_alias_id, server_names).await?; - Ok(Arc::new(RoomPreview::from_sdk(room_preview))) + + Ok(Arc::new(RoomPreview::new(ManuallyDrop::new(client), room_preview))) } /// Build a full `Room` FFI object, filling its associated timeline. diff --git a/bindings/matrix-sdk-ffi/src/room_preview.rs b/bindings/matrix-sdk-ffi/src/room_preview.rs index 77a110b4f76..08d3b626573 100644 --- a/bindings/matrix-sdk-ffi/src/room_preview.rs +++ b/bindings/matrix-sdk-ffi/src/room_preview.rs @@ -1,7 +1,10 @@ -use std::sync::Arc; +use std::mem::ManuallyDrop; -use matrix_sdk::room_preview::RoomPreview as SdkRoomPreview; +use anyhow::Context as _; +use async_compat::TOKIO1 as RUNTIME; +use matrix_sdk::{room_preview::RoomPreview as SdkRoomPreview, Client}; use ruma::space::SpaceRoomJoinRule; +use tracing::warn; use crate::{client::JoinRule, error::ClientError, room::Membership}; @@ -9,15 +12,30 @@ use crate::{client::JoinRule, error::ClientError, room::Membership}; /// aren't joined yet. #[derive(uniffi::Object)] pub struct RoomPreview { - inner: Arc, + inner: SdkRoomPreview, + client: ManuallyDrop, +} + +impl Drop for RoomPreview { + fn drop(&mut self) { + // Dropping the inner OlmMachine must happen within a tokio context + // because deadpool drops sqlite connections in the DB pool on tokio's + // blocking threadpool to avoid blocking async worker threads. + let _guard = RUNTIME.enter(); + // SAFETY: self.client is never used again, which is the only requirement + // for ManuallyDrop::drop to be used safely. + unsafe { + ManuallyDrop::drop(&mut self.client); + } + } } #[matrix_sdk_ffi_macros::export] impl RoomPreview { /// Returns the room info the preview contains. - pub fn info(&self) -> RoomPreviewInfo { + pub fn info(&self) -> Result { let info = &self.inner; - RoomPreviewInfo { + Ok(RoomPreviewInfo { room_id: info.room_id.to_string(), canonical_alias: info.canonical_alias.as_ref().map(|alias| alias.to_string()), name: info.name.clone(), @@ -27,8 +45,12 @@ impl RoomPreview { room_type: info.room_type.as_ref().map(|room_type| room_type.to_string()), is_history_world_readable: info.is_world_readable, membership: info.state.map(|state| state.into()), - join_rule: info.join_rule.clone().into(), - } + join_rule: info + .join_rule + .clone() + .try_into() + .map_err(|_| anyhow::anyhow!("unhandled SpaceRoomJoinRule kind"))?, + }) } /// Leave the room if the room preview state is either joined, invited or @@ -36,13 +58,15 @@ impl RoomPreview { /// /// Will return an error otherwise. pub async fn leave(&self) -> Result<(), ClientError> { - self.inner.leave().await.map_err(Into::into) + let room = + self.client.get_room(&self.inner.room_id).context("missing room for a room preview")?; + room.leave().await.map_err(Into::into) } } impl RoomPreview { - pub(crate) fn from_sdk(room_preview: SdkRoomPreview) -> Self { - Self { inner: Arc::new(room_preview) } + pub(crate) fn new(client: ManuallyDrop, inner: SdkRoomPreview) -> Self { + Self { client, inner } } } @@ -71,22 +95,22 @@ pub struct RoomPreviewInfo { pub join_rule: JoinRule, } -impl From for JoinRule { - fn from(join_rule: SpaceRoomJoinRule) -> Self { - match join_rule { +impl TryFrom for JoinRule { + type Error = (); + + fn try_from(join_rule: SpaceRoomJoinRule) -> Result { + Ok(match join_rule { SpaceRoomJoinRule::Invite => JoinRule::Invite, SpaceRoomJoinRule::Knock => JoinRule::Knock, SpaceRoomJoinRule::Private => JoinRule::Private, SpaceRoomJoinRule::Restricted => JoinRule::Restricted { rules: Vec::new() }, SpaceRoomJoinRule::KnockRestricted => JoinRule::KnockRestricted { rules: Vec::new() }, SpaceRoomJoinRule::Public => JoinRule::Public, + SpaceRoomJoinRule::_Custom(_) => JoinRule::Custom { repr: join_rule.to_string() }, _ => { - // Since we have no way to get the _Custom contents, assume it's private. - // Warning! If new join rules are introduced we may be mapping wrong values - // here, but there's no way to match - // `SpaceRoomJoinRule::_Custom(_)` and have an exhaustive match. - JoinRule::Private + warn!("unhandled SpaceRoomJoinRule: {join_rule}"); + return Err(()); } - } + }) } } diff --git a/crates/matrix-sdk/src/client/mod.rs b/crates/matrix-sdk/src/client/mod.rs index ce8afc85ba0..7066fcba545 100644 --- a/crates/matrix-sdk/src/client/mod.rs +++ b/crates/matrix-sdk/src/client/mod.rs @@ -54,7 +54,7 @@ use ruma::{ }, filter::{create_filter::v3::Request as FilterUploadRequest, FilterDefinition}, knock::knock_room, - membership::{join_room_by_id, join_room_by_id_or_alias, leave_room}, + membership::{join_room_by_id, join_room_by_id_or_alias}, room::create_room, session::login::v3::DiscoveryInfo, sync::sync_events, @@ -1208,13 +1208,6 @@ impl Client { Ok(Room::new(self.clone(), base_room)) } - /// Leave a room given its `RoomId`. - pub async fn leave_room(&self, room_id: &RoomId) -> Result<()> { - let request = leave_room::v3::Request::new(room_id.to_owned()); - self.send(request, None).await?; - self.base_client().room_left(room_id).await.map_err(Into::into) - } - /// Search the homeserver's directory of public rooms. /// /// Sends a request to "_matrix/client/r0/publicRooms", returns diff --git a/crates/matrix-sdk/src/error.rs b/crates/matrix-sdk/src/error.rs index c65e2cd132d..86c31496f99 100644 --- a/crates/matrix-sdk/src/error.rs +++ b/crates/matrix-sdk/src/error.rs @@ -45,9 +45,7 @@ use serde_json::Error as JsonError; use thiserror::Error; use url::ParseError as UrlParseError; -use crate::{ - event_cache::EventCacheError, room_preview::WrongRoomPreviewState, store_locks::LockStoreError, -}; +use crate::{event_cache::EventCacheError, store_locks::LockStoreError}; /// Result type of the matrix-sdk. pub type Result = std::result::Result; @@ -350,12 +348,6 @@ pub enum Error { #[error("wrong room state: {0}")] WrongRoomState(WrongRoomState), - /// Attempted to call a method on a room preview that requires the user to - /// have a specific membership state in the room, but the membership - /// state is different or none. - #[error("wrong room state: {0}")] - WrongRoomPreviewState(WrongRoomPreviewState), - /// Session callbacks have been set multiple times. #[error("session callbacks have been set multiple times")] MultipleSessionCallbacks, diff --git a/crates/matrix-sdk/src/room_preview.rs b/crates/matrix-sdk/src/room_preview.rs index ca6db08a1f8..c102b6225ff 100644 --- a/crates/matrix-sdk/src/room_preview.rs +++ b/crates/matrix-sdk/src/room_preview.rs @@ -29,7 +29,7 @@ use ruma::{ use tokio::try_join; use tracing::{instrument, warn}; -use crate::{Client, Error, Room}; +use crate::{Client, Room}; /// The preview of a room, be it invited/joined/left, or not. #[derive(Debug, Clone)] @@ -72,43 +72,20 @@ pub struct RoomPreview { /// The `m.room.direct` state of the room, if known. pub is_direct: Option, - - /// The [`Client`] that can be used to perform actions given the room - /// preview. - pub(crate) client: Client, } impl RoomPreview { - /// Leave a room using its [`RoomPreview`]. - pub async fn leave(&self) -> Result<(), Error> { - match self.state { - Some(RoomState::Left) | None => { - Err(Error::WrongRoomPreviewState(WrongRoomPreviewState { - expected: "Invited, Knocked or Joined", - got: self.state, - })) - } - Some(_) => self.client.leave_room(&self.room_id).await.map_err(Into::into), - } - } - /// Constructs a [`RoomPreview`] from the associated room info. /// /// Note: not using the room info's state/count of joined members, because /// we can do better than that. - async fn from_room_info( - client: &Client, + fn from_room_info( room_info: RoomInfo, + is_direct: Option, num_joined_members: u64, state: Option, ) -> Self { - let is_direct = if let Some(room) = client.get_room(room_info.room_id()) { - room.is_direct().await.ok() - } else { - None - }; RoomPreview { - client: client.clone(), room_id: room_info.room_id().to_owned(), canonical_alias: room_info.canonical_alias().map(ToOwned::to_owned), name: room_info.name().map(ToOwned::to_owned), @@ -138,13 +115,14 @@ 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 { + let is_direct = room.is_direct().await.ok(); + Self::from_room_info( - &room.client, room.clone_info(), + is_direct, room.joined_members_count(), Some(room.state()), ) - .await } #[instrument(skip(client))] @@ -207,7 +185,6 @@ impl RoomPreview { }; Ok(RoomPreview { - client: client.clone(), room_id, canonical_alias: response.canonical_alias, name: response.name, @@ -259,23 +236,10 @@ impl RoomPreview { room_info.handle_state_event(&ev.into()); } - let state = client.get_room(room_id).map(|room| room.state()); - - Ok(Self::from_room_info(client, room_info, num_joined_members, state).await) - } -} - -/// An unexpected room preview state was found. -#[derive(Debug, thiserror::Error)] -#[error("expected: {expected}, got: {got:?}")] -pub struct WrongRoomPreviewState { - expected: &'static str, - got: Option, -} + let room = client.get_room(room_id); + let state = room.as_ref().map(|room| room.state()); + let is_direct = if let Some(room) = room { room.is_direct().await.ok() } else { None }; -impl WrongRoomPreviewState { - /// Instantiate a new - pub fn new(expected: &'static str, got: Option) -> Self { - Self { expected, got } + Ok(Self::from_room_info(room_info, is_direct, num_joined_members, state)) } } diff --git a/crates/matrix-sdk/tests/integration/room_preview.rs b/crates/matrix-sdk/tests/integration/room_preview.rs index 853640691d2..142df0b83ef 100644 --- a/crates/matrix-sdk/tests/integration/room_preview.rs +++ b/crates/matrix-sdk/tests/integration/room_preview.rs @@ -1,5 +1,4 @@ -use assert_matches2::assert_matches; -use matrix_sdk::{config::SyncSettings, test_utils::logged_in_client_with_server, Error}; +use matrix_sdk::{config::SyncSettings, test_utils::logged_in_client_with_server}; use matrix_sdk_base::RoomState; use matrix_sdk_test::{ async_test, InvitedRoomBuilder, JoinedRoomBuilder, KnockedRoomBuilder, SyncResponseBuilder, @@ -30,7 +29,7 @@ async fn test_room_preview_leave_invited() { let room_preview = client.get_room_preview(room_id.into(), Vec::new()).await.unwrap(); assert_eq!(room_preview.state.unwrap(), RoomState::Invited); - room_preview.leave().await.unwrap(); + client.get_room(room_id).unwrap().leave().await.unwrap(); assert_eq!(client.get_room(room_id).unwrap().state(), RoomState::Left); } @@ -52,7 +51,8 @@ async fn test_room_preview_leave_knocked() { let room_preview = client.get_room_preview(room_id.into(), Vec::new()).await.unwrap(); assert_eq!(room_preview.state.unwrap(), RoomState::Knocked); - room_preview.leave().await.unwrap(); + let room = client.get_room(room_id).unwrap(); + room.leave().await.unwrap(); assert_eq!(client.get_room(room_id).unwrap().state(), RoomState::Left); } @@ -74,9 +74,10 @@ async fn test_room_preview_leave_joined() { let room_preview = client.get_room_preview(room_id.into(), Vec::new()).await.unwrap(); assert_eq!(room_preview.state.unwrap(), RoomState::Joined); - room_preview.leave().await.unwrap(); + let room = client.get_room(room_id).unwrap(); + room.leave().await.unwrap(); - assert_eq!(client.get_room(room_id).unwrap().state(), RoomState::Left); + assert_eq!(room.state(), RoomState::Left); } #[async_test] @@ -85,14 +86,11 @@ async fn test_room_preview_leave_unknown_room_fails() { let room_id = room_id!("!room:localhost"); mock_unknown_summary(room_id, None, SpaceRoomJoinRule::Knock, &server).await; - mock_leave(room_id, &server).await; let room_preview = client.get_room_preview(room_id.into(), Vec::new()).await.unwrap(); assert!(room_preview.state.is_none()); - let error = room_preview.leave().await.err(); - - assert_matches!(error, Some(Error::WrongRoomPreviewState(_))); + assert!(client.get_room(room_id).is_none()); } async fn mock_leave(room_id: &RoomId, server: &MockServer) {