From dcc3d6e755a555d1d8ae13b58a8f83cbe6a5533a Mon Sep 17 00:00:00 2001 From: Devin R Date: Fri, 3 Jul 2020 15:29:10 -0400 Subject: [PATCH] sdk_base: Remove room_id as argument from all Room methods Remove room_id paramater from some client methods. Make CreationContent two methods of RoomBuilder. Add docs for MessageWrapper. --- matrix_sdk/src/client.rs | 2 +- matrix_sdk/src/request_builder.rs | 44 +++++++++++++++++---------- matrix_sdk_base/src/client.rs | 20 +++++------- matrix_sdk_base/src/models/message.rs | 7 ++++- matrix_sdk_base/src/models/room.rs | 31 +++++++------------ matrix_sdk_crypto/src/machine.rs | 3 +- matrix_sdk_test/src/lib.rs | 7 ++--- 7 files changed, 57 insertions(+), 57 deletions(-) diff --git a/matrix_sdk/src/client.rs b/matrix_sdk/src/client.rs index 2b69f64b..9eeb645c 100644 --- a/matrix_sdk/src/client.rs +++ b/matrix_sdk/src/client.rs @@ -773,7 +773,7 @@ impl Client { /// /// # let homeserver = Url::parse("http://example.com").unwrap(); /// let mut builder = RoomBuilder::default(); - /// builder.creation_content(false, None) + /// builder.federate(false) /// .initial_state(vec![]) /// .visibility(Visibility::Public) /// .name("name") diff --git a/matrix_sdk/src/request_builder.rs b/matrix_sdk/src/request_builder.rs index a4191d2e..23148197 100644 --- a/matrix_sdk/src/request_builder.rs +++ b/matrix_sdk/src/request_builder.rs @@ -32,7 +32,7 @@ use matrix_sdk_common::{ /// # let mut rt = tokio::runtime::Runtime::new().unwrap(); /// # rt.block_on(async { /// let mut builder = RoomBuilder::default(); -/// builder.creation_content(false, None) +/// builder.federate(false) /// .initial_state(vec![]) /// .visibility(Visibility::Public) /// .name("name") @@ -43,7 +43,8 @@ use matrix_sdk_common::{ /// ``` #[derive(Clone, Debug, Default)] pub struct RoomBuilder { - creation_content: Option, + previous_room: Option, + federate: bool, initial_state: Vec, invite: Vec, invite_3pid: Vec, @@ -60,21 +61,27 @@ pub struct RoomBuilder { impl RoomBuilder { /// Returns an empty `RoomBuilder` for creating rooms. pub fn new() -> Self { - Self::default() + Self { + // we can't send false as a default as it will overwrite the true default. + federate: true, + ..Default::default() + } } - /// Set the `CreationContent`. + /// A reference to the room this room replaces, if the previous room was upgraded. /// - /// Weather users on other servers can join this room. - pub fn creation_content( - &mut self, - federate: bool, - predecessor: Option, - ) -> &mut Self { - self.creation_content = Some(CreationContent { - federate, - predecessor, - }); + /// Note: this is used to create the `CreationContent`. + pub fn previous_room(&mut self, previous_room: PreviousRoom) -> &mut Self { + self.previous_room = Some(previous_room); + self + } + + /// Whether users on other servers can join this room. + /// + /// Defaults to `true` if key does not exist. + /// Note: this is used to create the `CreationContent`. + pub fn federate(&mut self, federate: bool) -> &mut Self { + self.federate = federate; self } @@ -156,8 +163,12 @@ impl RoomBuilder { impl Into for RoomBuilder { fn into(self) -> create_room::Request { + let creation_content = Some(CreationContent { + federate: self.federate, + predecessor: self.previous_room, + }); create_room::Request { - creation_content: self.creation_content, + creation_content, initial_state: self.initial_state, invite: self.invite, invite_3pid: self.invite_3pid, @@ -270,6 +281,7 @@ impl Into for MessagesRequestBuilder { from: self.from, to: self.to, dir: self.direction.unwrap_or(Direction::Backward), + // Will our default overwrite the normal default limit: self.limit.map(UInt::from).unwrap_or_default(), filter: self.filter, } @@ -509,7 +521,7 @@ mod test { let mut builder = RoomBuilder::new(); builder - .creation_content(false, None) + .federate(false) .initial_state(vec![]) .visibility(Visibility::Public) .name("room_name") diff --git a/matrix_sdk_base/src/client.rs b/matrix_sdk_base/src/client.rs index 91c529f6..7a90c8b4 100644 --- a/matrix_sdk_base/src/client.rs +++ b/matrix_sdk_base/src/client.rs @@ -743,7 +743,7 @@ impl BaseClient { let mut room = room_lock.write().await; if let AnyRoomEventStub::State(AnyStateEventStub::RoomMember(mem_event)) = &mut e { - let changed = room.handle_membership(mem_event, room_id); + let changed = room.handle_membership(mem_event); // The memberlist of the room changed, invalidate the group session // of the room. @@ -754,7 +754,7 @@ impl BaseClient { Ok(changed) } else { - Ok(room.receive_timeline_event(&e, room_id)) + Ok(room.receive_timeline_event(&e)) } } _ => Ok(false), @@ -780,7 +780,7 @@ impl BaseClient { let mut room = room_lock.write().await; if let AnyStateEventStub::RoomMember(e) = event { - let changed = room.handle_membership(e, room_id); + let changed = room.handle_membership(e); // The memberlist of the room changed, invalidate the group session // of the room. @@ -791,7 +791,7 @@ impl BaseClient { Ok(changed) } else { - Ok(room.receive_state_event(event, room_id)) + Ok(room.receive_state_event(event)) } } @@ -834,7 +834,7 @@ impl BaseClient { Ok(e) => { let room_lock = self.get_or_create_left_room(room_id).await?; let mut room = room_lock.write().await; - Ok(room.receive_timeline_event(&e, room_id)) + Ok(room.receive_timeline_event(&e)) } _ => Ok(false), } @@ -857,7 +857,7 @@ impl BaseClient { ) -> Result { let room_lock = self.get_or_create_left_room(room_id).await?; let mut room = room_lock.write().await; - Ok(room.receive_state_event(event, room_id)) + Ok(room.receive_state_event(event)) } /// Receive a presence event from a sync response and updates the client state. @@ -906,11 +906,7 @@ impl BaseClient { /// * `room_id` - The unique id of the room the event belongs to. /// /// * `event` - The presence event for a specified room member. - pub async fn receive_ephemeral_event( - &self, - _room_id: &RoomId, - event: &AnyEphemeralRoomEventStub, - ) -> bool { + pub async fn receive_ephemeral_event(&self, event: &AnyEphemeralRoomEventStub) -> bool { match &event { AnyEphemeralRoomEventStub::FullyRead(_) => {} AnyEphemeralRoomEventStub::Receipt(_) => {} @@ -1093,7 +1089,7 @@ impl BaseClient { if let Ok(e) = ephemeral.deserialize() { // FIXME: receive_* and emit_* methods shouldn't be called in parallel. We // should only pass events to receive_* methods and then let *them* emit. - if self.receive_ephemeral_event(&room_id, &e).await { + if self.receive_ephemeral_event(&e).await { updated = true; } diff --git a/matrix_sdk_base/src/models/message.rs b/matrix_sdk_base/src/models/message.rs index cee4cd60..f7d2141b 100644 --- a/matrix_sdk_base/src/models/message.rs +++ b/matrix_sdk_base/src/models/message.rs @@ -13,12 +13,17 @@ use serde::{de, ser, Serialize}; const MESSAGE_QUEUE_CAP: usize = 35; -/// A queue that holds the 10 most recent messages received from the server. +/// A queue that holds the 35 most recent messages received from the server. #[derive(Clone, Debug, Default)] pub struct MessageQueue { msgs: Vec, } +/// A wrapper for `ruma_events::AnyMessageEventStub` that allows implementation of +/// Eq, Ord and the Partial versions of the traits. +/// +/// `MessageWrapper` also implements Deref and DerefMut so accessing the events contents +/// are simplified. #[derive(Clone, Debug, Serialize)] pub struct MessageWrapper(pub AnyMessageEventStub); diff --git a/matrix_sdk_base/src/models/room.rs b/matrix_sdk_base/src/models/room.rs index b4b85272..ab2218f2 100644 --- a/matrix_sdk_base/src/models/room.rs +++ b/matrix_sdk_base/src/models/room.rs @@ -360,8 +360,8 @@ impl Room { } } - fn add_member(&mut self, event: &StateEventStub, room_id: &RoomId) -> bool { - let new_member = RoomMember::new(event, room_id); + fn add_member(&mut self, event: &StateEventStub) -> bool { + let new_member = RoomMember::new(event, &self.room_id); if self.joined_members.contains_key(&new_member.user_id) || self.invited_members.contains_key(&new_member.user_id) @@ -396,12 +396,8 @@ impl Room { /// Process the member event of a leaving user. /// /// Returns true if this made a change to the room's state, false otherwise. - fn remove_member( - &mut self, - event: &StateEventStub, - room_id: &RoomId, - ) -> bool { - let leaving_member = RoomMember::new(event, room_id); + fn remove_member(&mut self, event: &StateEventStub) -> bool { + let leaving_member = RoomMember::new(event, &self.room_id); // Perform display name disambiguations, if necessary. let disambiguations = @@ -587,19 +583,15 @@ impl Room { /// Handle a room.member updating the room state if necessary. /// /// Returns true if the joined member list changed, false otherwise. - pub fn handle_membership( - &mut self, - event: &StateEventStub, - room_id: &RoomId, - ) -> bool { + pub fn handle_membership(&mut self, event: &StateEventStub) -> bool { use MembershipChange::*; // TODO: This would not be handled correctly as all the MemberEvents have the `prev_content` // inside of `unsigned` field. match event.membership_change() { - Invited | Joined => self.add_member(event, room_id), + Invited | Joined => self.add_member(event), Kicked | Banned | KickedAndBanned | InvitationRejected | Left => { - self.remove_member(event, room_id) + self.remove_member(event) } ProfileChanged { .. } => { let user_id = if let Ok(id) = UserId::try_from(event.state_key.as_str()) { @@ -640,7 +632,6 @@ impl Room { .iter_mut() .find(|msg| &event.redacts == msg.event_id()) { - // TODO make msg an enum or use AnyMessageEventStub enum to represent *msg = MessageWrapper(AnyMessageEventStub::RoomRedaction(event.clone())); true } else { @@ -734,11 +725,11 @@ impl Room { /// # Arguments /// /// * `event` - The event of the room. - pub fn receive_timeline_event(&mut self, event: &AnyRoomEventStub, room_id: &RoomId) -> bool { + pub fn receive_timeline_event(&mut self, event: &AnyRoomEventStub) -> bool { match &event { AnyRoomEventStub::State(event) => match &event { // update to the current members of the room - AnyStateEventStub::RoomMember(event) => self.handle_membership(&event, room_id), + AnyStateEventStub::RoomMember(event) => self.handle_membership(&event), // finds all events related to the name of the room for later use AnyStateEventStub::RoomName(event) => self.handle_room_name(&event), AnyStateEventStub::RoomCanonicalAlias(event) => self.handle_canonical(&event), @@ -768,10 +759,10 @@ impl Room { /// # Arguments /// /// * `event` - The event of the room. - pub fn receive_state_event(&mut self, event: &AnyStateEventStub, room_id: &RoomId) -> bool { + pub fn receive_state_event(&mut self, event: &AnyStateEventStub) -> bool { match event { // update to the current members of the room - AnyStateEventStub::RoomMember(member) => self.handle_membership(member, room_id), + AnyStateEventStub::RoomMember(member) => self.handle_membership(member), // finds all events related to the name of the room for later use AnyStateEventStub::RoomName(name) => self.handle_room_name(name), AnyStateEventStub::RoomCanonicalAlias(c_alias) => self.handle_canonical(c_alias), diff --git a/matrix_sdk_crypto/src/machine.rs b/matrix_sdk_crypto/src/machine.rs index 2a0a09a9..857382c0 100644 --- a/matrix_sdk_crypto/src/machine.rs +++ b/matrix_sdk_crypto/src/machine.rs @@ -938,7 +938,6 @@ impl OlmMachine { .ok_or(EventError::MissingSigningKey)?; Ok(( - // FIXME EventJson fails to deserialize still? EventJson::from(serde_json::from_value::(decrypted_json)?), signing_key.to_owned(), )) @@ -999,7 +998,7 @@ impl OlmMachine { .await? { // Some events may have sensitive data e.g. private keys, while we - // wan't to notify our users that a private key was received we + // want to notify our users that a private key was received we // don't want them to be able to do silly things with it. Handling // events modifies them and returns a modified one, so replace it // here if we get one. diff --git a/matrix_sdk_test/src/lib.rs b/matrix_sdk_test/src/lib.rs index 51f7e113..2538ce77 100644 --- a/matrix_sdk_test/src/lib.rs +++ b/matrix_sdk_test/src/lib.rs @@ -6,8 +6,8 @@ use http::Response; use matrix_sdk_common::api::r0::sync::sync_events::Response as SyncResponse; use matrix_sdk_common::events::{ - presence::PresenceEvent, AnyBasicEvent, AnyEphemeralRoomEventContent, AnyRoomEventStub, - AnyStateEventStub, EphemeralRoomEventStub, + presence::PresenceEvent, AnyBasicEvent, AnyEphemeralRoomEventStub, AnyRoomEventStub, + AnyStateEventStub, }; use matrix_sdk_common::identifiers::RoomId; use serde_json::Value as JsonValue; @@ -16,9 +16,6 @@ pub use matrix_sdk_test_macros::async_test; pub mod test_json; -/// Static `serde_json::Value`s -type AnyEphemeralRoomEventStub = EphemeralRoomEventStub; - /// Embedded event files #[derive(Debug)] pub enum EventsJson {