diff --git a/matrix_sdk/src/client.rs b/matrix_sdk/src/client.rs index 74e1caf2..a560d4cf 100644 --- a/matrix_sdk/src/client.rs +++ b/matrix_sdk/src/client.rs @@ -30,8 +30,7 @@ use matrix_sdk_common::uuid::Uuid; use futures_timer::Delay as sleep; use std::future::Future; #[cfg(feature = "encryption")] -use tracing::{debug, warn}; -use tracing::{info, instrument, trace}; +use tracing::{debug, error, info, instrument, trace, warn}; use http::Method as HttpMethod; use http::Response as HttpResponse; @@ -1311,12 +1310,13 @@ impl Client { loop { let response = self.sync(sync_settings.clone()).await; - let response = if let Ok(r) = response { - r - } else { - sleep::new(Duration::from_secs(1)).await; - - continue; + let response = match response { + Ok(r) => r, + Err(e) => { + error!("Received an invalid response: {}", e); + sleep::new(Duration::from_secs(1)).await; + continue; + } }; // TODO send out to-device messages here diff --git a/matrix_sdk_base/Cargo.toml b/matrix_sdk_base/Cargo.toml index d7f4d0dc..bbb9a264 100644 --- a/matrix_sdk_base/Cargo.toml +++ b/matrix_sdk_base/Cargo.toml @@ -21,6 +21,7 @@ async-trait = "0.1.36" serde = "1.0.114" serde_json = "1.0.56" zeroize = "1.1.0" +tracing = "0.1.14" matrix-sdk-common-macros = { version = "0.1.0", path = "../matrix_sdk_common_macros" } matrix-sdk-common = { version = "0.1.0", path = "../matrix_sdk_common" } diff --git a/matrix_sdk_base/src/client.rs b/matrix_sdk_base/src/client.rs index 75a422bf..1c38b90c 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); + let (changed, _) = room.handle_membership(mem_event, false); // The memberlist of the room changed, invalidate the group session // of the room. @@ -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); + let (changed, _) = room.handle_membership(e, true); // The memberlist of the room changed, invalidate the group session // of the room. @@ -2157,8 +2157,8 @@ mod test { let member = room.joined_members.get(&user_id).unwrap(); assert_eq!(*member.display_name.as_ref().unwrap(), "changed"); - // The second part tests that the event is emitted correctly. If `prev_content` was - // missing, this bool is reset to false. + // The second part tests that the event is emitted correctly. If `prev_content` were + // missing, this bool would had been flipped. assert!(passed.load(Ordering::SeqCst)) } @@ -2411,7 +2411,8 @@ mod test { } } - // `receive_joined_timeline_event` does not save the state to the store so we must + // `receive_joined_timeline_event` does not save the state to the store + // so we must do it ourselves client.store_room_state(&room_id).await.unwrap(); // we load state from the store only diff --git a/matrix_sdk_base/src/models/message.rs b/matrix_sdk_base/src/models/message.rs index d1431e6c..4bb0aae2 100644 --- a/matrix_sdk_base/src/models/message.rs +++ b/matrix_sdk_base/src/models/message.rs @@ -216,7 +216,6 @@ mod test { serde_json::json!({ "!roomid:example.com": { "room_id": "!roomid:example.com", - "disambiguated_display_names": {}, "room_name": { "name": null, "canonical_alias": null, @@ -262,7 +261,6 @@ mod test { let json = serde_json::json!({ "!roomid:example.com": { "room_id": "!roomid:example.com", - "disambiguated_display_names": {}, "room_name": { "name": null, "canonical_alias": null, diff --git a/matrix_sdk_base/src/models/room.rs b/matrix_sdk_base/src/models/room.rs index 54a99ed2..c0c97cd7 100644 --- a/matrix_sdk_base/src/models/room.rs +++ b/matrix_sdk_base/src/models/room.rs @@ -13,21 +13,23 @@ // See the License for the specific language governing permissions and // limitations under the License. -use std::borrow::Cow; -use std::collections::{BTreeMap, HashMap}; +use std::collections::{BTreeMap, HashMap, HashSet}; use std::convert::TryFrom; +use serde::{Deserialize, Serialize}; +use tracing::{debug, error, trace}; + #[cfg(feature = "messages")] use super::message::{MessageQueue, MessageWrapper}; use super::RoomMember; use crate::api::r0::sync::sync_events::{RoomSummary, UnreadNotificationsCount}; -use crate::events::presence::PresenceEvent; +use crate::events::presence::{PresenceEvent, PresenceEventContent}; use crate::events::room::{ aliases::AliasesEventContent, canonical_alias::CanonicalAliasEventContent, encryption::EncryptionEventContent, - member::{MemberEventContent, MembershipChange}, + member::{MemberEventContent, MembershipChange, MembershipState}, name::NameEventContent, power_levels::{NotificationPowerLevels, PowerLevelsEventContent}, tombstone::TombstoneEventContent, @@ -46,8 +48,7 @@ use crate::events::{ use crate::identifiers::{RoomAliasId, RoomId, UserId}; -use crate::js_int::{uint, Int, UInt}; -use serde::{Deserialize, Serialize}; +use crate::js_int::{int, uint, Int, UInt}; #[cfg(feature = "messages")] fn redaction_event_from_redaction_stub( @@ -70,20 +71,20 @@ fn redaction_event_from_redaction_stub( pub struct RoomName { /// The displayed name of the room. name: Option, - /// The canonical alias of the room ex. `#room-name:example.com` and port number. + /// The canonical alias of the room ex. `#room-name:example.com` and port + /// number. canonical_alias: Option, /// List of `RoomAliasId`s the room has been given. aliases: Vec, - /// Users which can be used to generate a room name if the room does not have - /// one. Required if room name or canonical aliases are not set or empty. + /// Users which can be used to generate a room name if the room does not + /// have one. Required if room name or canonical aliases are not set or + /// empty. pub heroes: Vec, - /// Number of users whose membership status is `join`. - /// Required if field has changed since last sync; otherwise, it may be - /// omitted. + /// Number of users whose membership status is `join`. Required if field + /// has changed since last sync; otherwise, it may be omitted. pub joined_member_count: Option, - /// Number of users whose membership status is `invite`. - /// Required if field has changed since last sync; otherwise, it may be - /// omitted. + /// Number of users whose membership status is `invite`. Required if field + /// has changed since last sync; otherwise, it may be omitted. pub invited_member_count: Option, } @@ -166,12 +167,6 @@ pub struct Tombstone { replacement: RoomId, } -#[derive(Debug, PartialEq, Eq)] -enum MemberDirection { - Entering, - Exiting, -} - #[derive(Clone, Debug, PartialEq, Serialize, Deserialize)] /// A Matrix room. pub struct Room { @@ -190,8 +185,8 @@ pub struct Room { pub joined_members: HashMap, /// A queue of messages, holds no more than 10 of the most recent messages. /// - /// This is helpful when using a `StateStore` to avoid multiple requests - /// to the server for messages. + /// This is helpful when using a `StateStore` to avoid multiple requests to + /// the server for messages. #[cfg(feature = "messages")] #[cfg_attr(docsrs, doc(cfg(feature = "messages")))] #[serde(with = "super::message::ser_deser")] @@ -208,8 +203,6 @@ pub struct Room { pub unread_notifications: Option, /// The tombstone state of this room. pub tombstone: Option, - /// The map of disambiguated display names for users who have the same display name - disambiguated_display_names: HashMap, } impl RoomName { @@ -228,8 +221,8 @@ impl RoomName { true } - /// Calculate the canonical display name of a room, taking into account its name, aliases and - /// members. + /// Calculate the canonical display name of the room, taking into account + /// its name, aliases and members. /// /// The display name is calculated according to [this algorithm][spec]. /// @@ -283,7 +276,8 @@ impl RoomName { .collect::>(); names.sort(); - // TODO: What length does the spec want us to use here and in the `else`? + // TODO: What length does the spec want us to use here and in + // the `else`? format!("{}, and {} others", names.join(", "), (joined + invited)) } else { "Empty room".to_string() @@ -316,7 +310,6 @@ impl Room { unread_highlight: None, unread_notifications: None, tombstone: None, - disambiguated_display_names: HashMap::new(), } } @@ -341,193 +334,207 @@ impl Room { self.encrypted.as_ref() } - /// Get the disambiguated display name for a member of this room. + /// Process the join or invite event for a new room member. /// - /// If a member has no display name set, returns the MXID as a fallback. Additionally, we - /// return the MXID even if there is no such member in the room. + /// This method should only be called on events which add new members, not + /// those related to existing ones. /// - /// When displaying a room member's display name, clients *must* use this method to obtain the - /// name instead of displaying the `RoomMember::display_name` directly. This is because - /// multiple members can share the same display name in which case the display name has to be - /// disambiguated. - pub fn member_display_name<'a>(&'a self, id: &'a UserId) -> Cow<'a, str> { - let disambiguated_name = self - .disambiguated_display_names - .get(id) - .map(|s| s.as_str().into()); - - if let Some(name) = disambiguated_name { - // The display name of the member is non-unique so we return a disambiguated version. - name - } else if let Some(member) = self - .joined_members - .get(id) - .or_else(|| self.invited_members.get(id)) - { - // The display name of the member is unique so we can return it directly if it is set. - // If not, we return his MXID. - member.name().into() - } else { - // There is no member with the requested MXID in the room. We still return the MXID. - id.as_str().into() - } - } - - fn add_member(&mut self, event: &StateEventStub) -> bool { + /// Returns a tuple of: + /// + /// 1. True if the event made changes to the room's state, false otherwise. + /// 2. A map of display name ambiguity status changes (see + /// `disambiguation_updates`). + /// + /// # Arguments + /// + /// * `target_member` - The ID of the member to add. + /// * `event` - The join or invite event for the specified room member. + fn add_member( + &mut self, + target_member: &UserId, + event: &StateEventStub, + ) -> (bool, HashMap) { 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) - { - return false; + if self.joined_members.contains_key(&new_member.user_id) { + error!("add_member called on event of an already joined user"); + return (false, HashMap::new()); } - match event.membership_change() { - MembershipChange::Joined => self - .joined_members - .insert(new_member.user_id.clone(), new_member.clone()), - MembershipChange::Invited => self - .invited_members - .insert(new_member.user_id.clone(), new_member.clone()), - _ => { - panic!("Room::add_member called on an event that is neither a join nor an invite.") - } - }; - - // Perform display name disambiguations, if necessary. - let disambiguations = self.disambiguation_updates(&new_member, MemberDirection::Entering); - for (id, name) in disambiguations.into_iter() { - match name { - None => self.disambiguated_display_names.remove(&id), - Some(name) => self.disambiguated_display_names.insert(id, name), - }; - } - - true - } - - /// 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) -> bool { - let leaving_member = RoomMember::new(event, &self.room_id); - // Perform display name disambiguations, if necessary. let disambiguations = - self.disambiguation_updates(&leaving_member, MemberDirection::Exiting); - for (id, name) in disambiguations.into_iter() { - match name { - None => self.disambiguated_display_names.remove(&id), - Some(name) => self.disambiguated_display_names.insert(id, name), - }; + self.disambiguation_updates(target_member, None, new_member.display_name.clone()); + + debug!("add_member: disambiguations: {:#?}", disambiguations); + + match event.content.membership { + MembershipState::Join => { + // Since the member is now joined, he shouldn't be tracked as an invited member any + // longer if he was previously tracked as such. + self.invited_members.remove(target_member); + + self.joined_members + .insert(target_member.clone(), new_member.clone()) + } + + MembershipState::Invite => self + .invited_members + .insert(target_member.clone(), new_member.clone()), + + _ => panic!("Room::add_member called on event that is neither `join` nor `invite`."), + }; + + for (id, is_ambiguous) in disambiguations.iter() { + self.get_member_mut(id).unwrap().display_name_ambiguous = *is_ambiguous; } - if self.joined_members.contains_key(&leaving_member.user_id) { - self.joined_members.remove(&leaving_member.user_id); - true - } else if self.invited_members.contains_key(&leaving_member.user_id) { - self.invited_members.remove(&leaving_member.user_id); - true - } else { - false + (true, disambiguations) + } + + /// Process the leaving event for a room member. + /// + /// Returns a tuple of: + /// + /// 1. True if the event made changes to the room's state, false otherwise. + /// 2. A map of display name ambiguity status changes (see + /// `disambiguation_updates`). + /// + /// # Arguments + /// + /// * `target_member` - The ID of the member to remove. + /// * `event` - The leaving event for the specified room member. + fn remove_member( + &mut self, + target_member: &UserId, + event: &StateEventStub, + ) -> (bool, HashMap) { + let leaving_member = RoomMember::new(event, &self.room_id); + + if self.get_member(target_member).is_none() { + return (false, HashMap::new()); + } + + // Perform display name disambiguations, if necessary. + let disambiguations = + self.disambiguation_updates(target_member, leaving_member.display_name.clone(), None); + + debug!("remove_member: disambiguations: {:#?}", disambiguations); + + for (id, is_ambiguous) in disambiguations.iter() { + self.get_member_mut(id).unwrap().display_name_ambiguous = *is_ambiguous; + } + + // TODO: factor this out to a method called `remove_member` and rename this method + // to something like `process_member_leaving_event`. + self.joined_members + .remove(target_member) + .or_else(|| self.invited_members.remove(target_member)); + + (true, disambiguations) + } + + /// Check whether the user with the MXID `user_id` is joined or invited to + /// the room. + /// + /// Returns true if so, false otherwise. + pub fn member_is_tracked(&self, user_id: &UserId) -> bool { + self.invited_members.contains_key(&user_id) || self.joined_members.contains_key(&user_id) + } + + /// Get a room member by user ID. + /// + /// If there is no such member, returns `None`. + pub fn get_member(&self, user_id: &UserId) -> Option<&RoomMember> { + self.joined_members + .get(user_id) + .or_else(|| self.invited_members.get(user_id)) + } + + /// Get a room member by user ID. + /// + /// If there is no such member, returns `None`. + pub fn get_member_mut(&mut self, user_id: &UserId) -> Option<&mut RoomMember> { + match self.joined_members.get_mut(user_id) { + None => self.invited_members.get_mut(user_id), + Some(m) => Some(m), } } - /// Given a room `member`, return the list of members which have the same display name. - /// - /// The `inclusive` parameter controls whether the passed member should be included in the - /// list or not. - fn shares_displayname_with(&self, member: &RoomMember, inclusive: bool) -> Vec { + /// Given a display name, return the set of members which share it. + fn display_name_equivalence_set(&self, name: &str) -> HashSet { let members = self .invited_members .iter() .chain(self.joined_members.iter()); - // Find all other users that share the same display name as the joining user. + // Find all other users that share the display name with the joining user. members - .filter(|(_, existing_member)| { + .filter(|(_, member)| { member .display_name .as_ref() - .and_then(|new_member_name| { - existing_member - .display_name - .as_ref() - .map(|existing_member_name| new_member_name == existing_member_name) - }) + .map(|other_name| other_name == name) .unwrap_or(false) }) - // If not an inclusive search, do not consider the member for which we are disambiguating. - .filter(|(id, _)| inclusive || **id != member.user_id) - .map(|(id, _)| id) - .cloned() + .map(|(_, member)| member.user_id.clone()) .collect() } - /// Given a room member, generate a map of all display name disambiguations which are necessary - /// in order to make that member's display name unique. + /// Answers the question "If `member` changed their display name from + /// `old_name` to `new_name`, which members' display names would become + /// ambiguous and which would no longer be ambiguous?". /// - /// The `inclusive` parameter controls whether or not the member for which we are - /// disambiguating should be considered a current member of the room. + /// Returns the map of ambiguity status changes for those members which + /// would be affected by the change. /// - /// Returns a map from MXID to disambiguated name. - fn member_disambiguations( - &self, - member: &RoomMember, - inclusive: bool, - ) -> HashMap { - let users_with_same_name = self.shares_displayname_with(member, inclusive); - let disambiguate_with = |members: Vec, f: fn(&RoomMember) -> String| { - members - .into_iter() - .filter_map(|id| { - self.joined_members - .get(&id) - .or_else(|| self.invited_members.get(&id)) - .map(f) - .map(|m| (id, m)) - }) - .collect::>() - }; - - match users_with_same_name.len() { - 0 => HashMap::new(), - 1 => disambiguate_with(users_with_same_name, |m: &RoomMember| m.name()), - _ => disambiguate_with(users_with_same_name, |m: &RoomMember| m.unique_name()), - } - } - - /// Calculate disambiguation updates needed when a room member either enters or exits. + /// It is important that this method be called *before* any changes are made + /// to the model, i.e. before any actual display name changes. + /// + /// # Arguments + /// + /// - `member`: The MXID of the member who is changing their display name. + /// - `old_name`: The old display name of `member`. May be `None` if + /// `member` had no display name in the room before (because he had not + /// set it or he is just entering the room). + /// - `new_name`: The new display name of `member`. May be `None` if + /// `member` will no longer have a display name in the room after the + /// change (because he is removing it or exiting the room). fn disambiguation_updates( &self, - member: &RoomMember, - when: MemberDirection, - ) -> HashMap> { - let before; - let after; + member: &UserId, + old_name: Option, + new_name: Option, + ) -> HashMap { + let old_name_eq_set = match old_name { + None => HashSet::new(), + Some(name) => self.display_name_equivalence_set(&name), + }; - match when { - MemberDirection::Entering => { - before = self.member_disambiguations(member, false); - after = self.member_disambiguations(member, true); - } - MemberDirection::Exiting => { - before = self.member_disambiguations(member, true); - after = self.member_disambiguations(member, false); - } - } + let disambiguate_old = match old_name_eq_set.len().saturating_sub(1) { + n if n > 1 => vec![(member.clone(), false)].into_iter().collect(), + 1 => old_name_eq_set.into_iter().map(|m| (m, false)).collect(), + 0 => HashMap::new(), + _ => panic!("impossible"), + }; - let mut res = before; - res.extend(after.clone()); + // - res.into_iter() - .map(|(user_id, name)| { - if !after.contains_key(&user_id) { - (user_id, None) - } else { - (user_id, Some(name)) - } - }) + let mut new_name_eq_set = match new_name { + None => HashSet::new(), + Some(name) => self.display_name_equivalence_set(&name), + }; + + new_name_eq_set.insert(member.clone()); + + let disambiguate_new = match new_name_eq_set.len() { + 1 => HashMap::new(), + 2 => new_name_eq_set.into_iter().map(|m| (m, true)).collect(), + _ => vec![(member.clone(), true)].into_iter().collect(), + }; + + disambiguate_old + .into_iter() + .chain(disambiguate_new.into_iter()) .collect() } @@ -595,33 +602,69 @@ 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) -> bool { + /// Returns a tuple of: + /// + /// 1. True if the joined member list changed, false otherwise. + /// 2. A map of display name ambiguity status changes (see + /// `disambiguation_updates`). + pub fn handle_membership( + &mut self, + event: &StateEventStub, + state_event: bool, + ) -> (bool, HashMap) { use MembershipChange::*; + use MembershipState::*; - // 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), - Kicked | Banned | KickedAndBanned | InvitationRejected | Left => { - self.remove_member(event) + trace!( + "Received {} event: {}", + if state_event { "state" } else { "timeline" }, + event.event_id + ); + + let target_user = match UserId::try_from(event.state_key.clone()) { + Ok(id) => id, + Err(e) => { + error!("Received a member event with invalid state_key: {}", e); + return (false, HashMap::new()); } - ProfileChanged { .. } => { - let user_id = if let Ok(id) = UserId::try_from(event.state_key.as_str()) { - id - } else { - return false; - }; + }; - if let Some(member) = self.joined_members.get_mut(&user_id) { - member.update_profile(event) - } else { - false + if state_event && !self.member_is_tracked(&target_user) { + debug!( + "handle_membership: User {user_id} is {state} the room {room_id} ({room_name})", + user_id = target_user, + state = event.content.membership.describe(), + room_id = self.room_id, + room_name = self.display_name(), + ); + + match event.content.membership { + Join | Invite => self.add_member(&target_user, event), + + // We are not interested in tracking past members for now + _ => (false, HashMap::new()), + } + } else { + let change = event.membership_change(); + + debug!( + "handle_membership: User {user_id} {action} the room {room_id} ({room_name})", + user_id = target_user, + action = change.describe(), + room_id = self.room_id, + room_name = self.display_name(), + ); + + match change { + Invited | Joined => self.add_member(&target_user, event), + Kicked | Banned | KickedAndBanned | InvitationRejected | Left => { + self.remove_member(&target_user, event) } - } + ProfileChanged { .. } => self.update_member_profile(&target_user, event, change), - // Not interested in other events. - _ => false, + // Not interested in other events. + _ => (false, HashMap::new()), + } } } @@ -638,8 +681,9 @@ impl Room { /// Handle a room.redaction event and update the `MessageQueue`. /// /// Returns true if `MessageQueue` was updated. The effected message event - /// has it's contents replaced with the `RedactionEventContents` and the whole - /// redaction event is added to the `Unsigned` `redacted_because` field. + /// has it's contents replaced with the `RedactionEventContents` and the + /// whole redaction event is added to the `Unsigned` `redacted_because` + /// field. #[cfg(feature = "messages")] #[cfg_attr(docsrs, doc(cfg(feature = "messages")))] pub fn handle_redaction(&mut self, event: &RedactionEventStub) -> bool { @@ -670,7 +714,8 @@ impl Room { } } - /// Handle a room.canonical_alias event, updating the room state if necessary. + /// Handle a room.canonical_alias event, updating the room state if + /// necessary. /// /// Returns true if the room name changed, false otherwise. pub fn handle_canonical(&mut self, event: &StateEventStub) -> bool { @@ -717,11 +762,12 @@ impl Room { for user in event.content.users.keys() { if let Some(member) = self.joined_members.get_mut(user) { - if member.update_power(event, max_power) { + if Room::update_member_power(member, event, max_power) { updated = true; } } } + updated } @@ -749,7 +795,7 @@ impl Room { match event { AnyRoomEventStub::State(event) => match event { // update to the current members of the room - AnyStateEventStub::RoomMember(event) => self.handle_membership(event), + AnyStateEventStub::RoomMember(event) => self.handle_membership(event, false).0, // 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), @@ -782,7 +828,7 @@ impl Room { 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), + AnyStateEventStub::RoomMember(member) => self.handle_membership(member, true).0, // 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), @@ -801,8 +847,8 @@ impl Room { /// /// # Arguments /// - /// * `event` - The `AnyStrippedStateEvent` sent by the server for invited but not - /// joined rooms. + /// * `event` - The `AnyStrippedStateEvent` sent by the server for invited + /// but not joined rooms. pub fn receive_stripped_state_event(&mut self, event: &AnyStrippedStateEventStub) -> bool { match event { AnyStrippedStateEventStub::RoomName(event) => self.handle_stripped_room_name(event), @@ -810,29 +856,233 @@ impl Room { } } - /// Receive a presence event from an `IncomingResponse` and updates the client state. + /// Receive a presence event for a member of the current room. /// - /// This will only update the user if found in the current room looped through - /// by `Client::sync`. - /// Returns true if the specific users presence has changed, false otherwise. + /// Returns true if the event causes a change to the member's presence, + /// false otherwise. /// /// # Arguments /// - /// * `event` - The presence event for a specified room member. + /// * `event` - The presence event to receive and process. pub fn receive_presence_event(&mut self, event: &PresenceEvent) -> bool { + let PresenceEvent { + content: + PresenceEventContent { + avatar_url, + currently_active, + displayname, + last_active_ago, + presence, + status_msg, + }, + .. + } = event; + if let Some(member) = self.joined_members.get_mut(&event.sender) { - if member.did_update_presence(event) { + if member.display_name == *displayname + && member.avatar_url == *avatar_url + && member.presence.as_ref() == Some(presence) + && member.status_msg == *status_msg + && member.last_active_ago == *last_active_ago + && member.currently_active == *currently_active + { + // Everything is the same, nothing to do. false } else { - member.update_presence(event); + // Something changed, do the update. + + member.presence_events.push(event.clone()); + member.avatar_url = avatar_url.clone(); + member.currently_active = *currently_active; + member.display_name = displayname.clone(); + member.last_active_ago = *last_active_ago; + member.presence = Some(*presence); + member.status_msg = status_msg.clone(); + true } } else { - // this is probably an error as we have a `PresenceEvent` for a user - // we don't know about + // This is probably an error as we have a `PresenceEvent` for a user + // we don't know about. false } } + + /// Process an update of a member's profile. + /// + /// Returns a tuple of: + /// + /// 1. True if the event made changes to the room's state, false otherwise. + /// 2. A map of display name ambiguity status changes (see + /// `disambiguation_updates`). + /// + /// # Arguments + /// + /// * `target_member` - The ID of the member to update. + /// * `event` - The profile update event for the specified room member. + pub fn update_member_profile( + &mut self, + target_member: &UserId, + event: &StateEventStub, + change: MembershipChange, + ) -> (bool, HashMap) { + let member = self.get_member(target_member); + let member = match member { + Some(member) => member, + + None => { + debug!("update_member_profile [{}]: Got a profile update for user {} but he's not a room member", + self.room_id, target_member); + return (false, HashMap::new()); + } + }; + + let old_name = member.display_name.clone(); + let new_name = event.content.displayname.clone(); + + match change { + MembershipChange::ProfileChanged { + displayname_changed, + avatar_url_changed, + } => { + if displayname_changed { + debug!( + "update_member_profile [{}]: {} changed display name from {:#?} to {:#?}", + self.room_id, target_member, old_name, &new_name + ); + } + + if avatar_url_changed { + debug!( + "update_member_profile [{}]: {} changed avatar URL from {:#?} to {:#?}", + self.room_id, target_member, &member.avatar_url, &new_name + ); + } + } + + _ => { + error!( + "update_member_profile [{}]: got a ProfileChanged but nothing changed", + self.room_id + ); + return (false, HashMap::new()); + } + } + + let disambiguations = + self.disambiguation_updates(target_member, old_name.clone(), new_name.clone()); + for (id, is_ambiguous) in disambiguations.iter() { + if self.get_member_mut(id).is_none() { + debug!("update_member_profile [{}]: Tried disambiguating display name for {} but he's not there", + self.room_id, + id); + } else { + self.get_member_mut(id).unwrap().display_name_ambiguous = *is_ambiguous; + } + } + + debug!( + "update_member_profile [{}]: disambiguations: {:#?}", + self.room_id, &disambiguations + ); + + let changed = match self.get_member_mut(target_member) { + Some(member) => { + member.display_name = new_name; + member.avatar_url = event.content.avatar_url.clone(); + true + } + None => { + error!( + "update_member_profile [{}]: user {} does not exist", + self.room_id, target_member + ); + + false + } + }; + + (changed, disambiguations) + } + + /// Process an update of a member's power level. + /// + /// # Arguments + /// + /// * `event` - The power level event to process. + /// * `max_power` - Maximum power level allowed. + pub fn update_member_power( + member: &mut RoomMember, + event: &StateEventStub, + max_power: Int, + ) -> bool { + let changed; + + if let Some(user_power) = event.content.users.get(&member.user_id) { + changed = member.power_level != Some(*user_power); + member.power_level = Some(*user_power); + } else { + changed = member.power_level != Some(event.content.users_default); + member.power_level = Some(event.content.users_default); + } + + if max_power > int!(0) { + member.power_level_norm = Some((member.power_level.unwrap() * int!(100)) / max_power); + } + + changed + } +} + +trait Describe { + fn describe(&self) -> String; +} + +impl Describe for MembershipState { + fn describe(&self) -> String { + match self { + Self::Ban => "is banned in", + Self::Invite => "is invited to", + Self::Join => "is a member of", + Self::Knock => "is requesting access", + Self::Leave => "left", + _ => "unhandled case of MembershipState", + } + .to_string() + } +} + +impl Describe for MembershipChange { + fn describe(&self) -> String { + match self { + Self::Invited => "got invited to", + Self::Joined => "joined", + Self::Kicked => "got kicked from", + Self::Banned => "got banned from", + Self::Unbanned => "got unbanned from", + Self::KickedAndBanned => "got kicked and banned from", + Self::InvitationRejected => "rejected the invitation to", + Self::InvitationRevoked => "got their invitation revoked from", + Self::Left => "left", + Self::ProfileChanged { + displayname_changed, + avatar_url_changed, + } => match (*displayname_changed, *avatar_url_changed) { + (true, true) => "changed their displayname and avatar", + (true, false) => "changed their displayname", + (false, true) => "changed their avatar", + _ => { + error!("Got ProfileChanged but nothing changed"); + "impossible: changed nothing in their profile" + } + }, + Self::None => "did nothing in", + Self::NotImplemented => "NOT IMPLEMENTED", + Self::Error => "ERROR", + _ => "unhandled case of MembershipChange", + } + .to_string() + } } #[cfg(test)] @@ -886,6 +1136,113 @@ mod test { assert!(room.deref().power_levels.is_some()) } + #[async_test] + async fn member_is_not_both_invited_and_joined() { + let client = get_client().await; + let room_id = get_room_id(); + let user_id1 = UserId::try_from("@example:localhost").unwrap(); + let user_id2 = UserId::try_from("@example2:localhost").unwrap(); + + let member2_invite_event = serde_json::json!({ + "content": { + "avatar_url": null, + "displayname": "example2", + "membership": "invite" + }, + "event_id": "$16345217l517tabbz:localhost", + "membership": "join", + "origin_server_ts": 1455123234, + "sender": format!("{}", user_id1), + "state_key": format!("{}", user_id2), + "type": "m.room.member", + "unsigned": { + "age": 1989321234, + "replaces_state": "$1622a2311315tkjoA:localhost" + } + }); + + let member2_join_event = serde_json::json!({ + "content": { + "avatar_url": null, + "displayname": "example2", + "membership": "join" + }, + "event_id": "$163409224327jkbba:localhost", + "membership": "join", + "origin_server_ts": 1455123238, + "sender": format!("{}", user_id2), + "state_key": format!("{}", user_id2), + "type": "m.room.member", + "prev_content": { + "avatar_url": null, + "displayname": "example2", + "membership": "invite" + }, + "unsigned": { + "age": 1989321214, + "replaces_state": "$16345217l517tabbz:localhost" + } + }); + + let mut event_builder = EventBuilder::new(); + + let mut member1_join_sync_response = event_builder + .add_room_event(EventsJson::Member) + .build_sync_response(); + + let mut member2_invite_sync_response = event_builder + .add_custom_joined_event(&room_id, member2_invite_event) + .build_sync_response(); + + let mut member2_join_sync_response = event_builder + .add_custom_joined_event(&room_id, member2_join_event) + .build_sync_response(); + + // Test that `user` is either joined or invited to `room` but not both. + async fn invited_or_joined_but_not_both(client: &BaseClient, room: &RoomId, user: &UserId) { + let room = client.get_joined_room(&room).await.unwrap(); + let room = room.read().await; + + assert!( + room.invited_members.get(&user).is_none() + || room.joined_members.get(&user).is_none() + ); + assert!( + room.invited_members.get(&user).is_some() + || room.joined_members.get(&user).is_some() + ); + }; + + // First member joins. + client + .receive_sync_response(&mut member1_join_sync_response) + .await + .unwrap(); + + // The first member is not *both* invited and joined but it *is* one of those. + invited_or_joined_but_not_both(&client, &room_id, &user_id1).await; + + // First member invites second member. + client + .receive_sync_response(&mut member2_invite_sync_response) + .await + .unwrap(); + + // Neither member is *both* invited and joined, but they are both *at least one* of those. + invited_or_joined_but_not_both(&client, &room_id, &user_id1).await; + invited_or_joined_but_not_both(&client, &room_id, &user_id2).await; + + // Second member joins. + client + .receive_sync_response(&mut member2_join_sync_response) + .await + .unwrap(); + + // Repeat the previous test. + invited_or_joined_but_not_both(&client, &room_id, &user_id1).await; + invited_or_joined_but_not_both(&client, &room_id, &user_id2).await; + } + #[async_test] async fn test_member_display_name() { // Initialize @@ -919,6 +1276,47 @@ mod test { } }); + let member1_invites_member2_event = serde_json::json!({ + "content": { + "avatar_url": null, + "displayname": "example", + "membership": "invite" + }, + "event_id": "$16345217l517tabbz:localhost", + "membership": "invite", + "origin_server_ts": 1455123238, + "sender": format!("{}", user_id1), + "state_key": format!("{}", user_id2), + "type": "m.room.member", + "unsigned": { + "age": 1989321238, + "replaces_state": "$1622a2311315tkjoA:localhost" + } + }); + + let member2_name_change_event = serde_json::json!({ + "content": { + "avatar_url": null, + "displayname": "changed", + "membership": "join" + }, + "event_id": "$16345217l517tabbz:localhost", + "membership": "join", + "origin_server_ts": 1455123238, + "sender": format!("{}", user_id2), + "state_key": format!("{}", user_id2), + "type": "m.room.member", + "prev_content": { + "avatar_url": null, + "displayname": "example", + "membership": "join" + }, + "unsigned": { + "age": 1989321238, + "replaces_state": "$1622a2311315tkjoA:localhost" + } + }); + let member2_leave_event = serde_json::json!({ "content": { "avatar_url": null, @@ -995,19 +1393,29 @@ mod test { .build_sync_response(); let mut member2_join_sync_response = event_builder - .add_custom_joined_event(&room_id, member2_join_event) + .add_custom_joined_event(&room_id, member2_join_event.clone()) .build_sync_response(); let mut member3_join_sync_response = event_builder .add_custom_joined_event(&room_id, member3_join_event) .build_sync_response(); - let mut member2_leave_sync_response = event_builder + let mut member2_and_member3_leave_sync_response = event_builder .add_custom_joined_event(&room_id, member2_leave_event) + .add_custom_joined_event(&room_id, member3_leave_event) .build_sync_response(); - let mut member3_leave_sync_response = event_builder - .add_custom_joined_event(&room_id, member3_leave_event) + let mut member2_rejoins_when_invited_sync_response = event_builder + .add_custom_joined_event(&room_id, member1_invites_member2_event) + .add_custom_joined_event(&room_id, member2_join_event) + .build_sync_response(); + + let mut member1_name_change_sync_response = event_builder + .add_room_event(EventsJson::MemberNameChange) + .build_sync_response(); + + let mut member2_name_change_sync_response = event_builder + .add_custom_joined_event(&room_id, member2_name_change_event) .build_sync_response(); // First member with display name "example" joins @@ -1020,7 +1428,7 @@ mod test { { let room = client.get_joined_room(&room_id).await.unwrap(); let room = room.read().await; - let display_name1 = room.member_display_name(&user_id1); + let display_name1 = room.get_member(&user_id1).unwrap().disambiguated_name(); assert_eq!("example", display_name1); } @@ -1039,9 +1447,9 @@ mod test { { let room = client.get_joined_room(&room_id).await.unwrap(); let room = room.read().await; - let display_name1 = room.member_display_name(&user_id1); - let display_name2 = room.member_display_name(&user_id2); - let display_name3 = room.member_display_name(&user_id3); + let display_name1 = room.get_member(&user_id1).unwrap().disambiguated_name(); + let display_name2 = room.get_member(&user_id2).unwrap().disambiguated_name(); + let display_name3 = room.get_member(&user_id3).unwrap().disambiguated_name(); assert_eq!(format!("example ({})", user_id1), display_name1); assert_eq!(format!("example ({})", user_id2), display_name2); @@ -1050,11 +1458,7 @@ mod test { // Second and third member leave. The first's display name is now just "example" again. client - .receive_sync_response(&mut member2_leave_sync_response) - .await - .unwrap(); - client - .receive_sync_response(&mut member3_leave_sync_response) + .receive_sync_response(&mut member2_and_member3_leave_sync_response) .await .unwrap(); @@ -1062,10 +1466,64 @@ mod test { let room = client.get_joined_room(&room_id).await.unwrap(); let room = room.read().await; - let display_name1 = room.member_display_name(&user_id1); + let display_name1 = room.get_member(&user_id1).unwrap().disambiguated_name(); assert_eq!("example", display_name1); } + + // Second member rejoins after being invited by first member. Both of their names are + // disambiguated. + client + .receive_sync_response(&mut member2_rejoins_when_invited_sync_response) + .await + .unwrap(); + + { + let room = client.get_joined_room(&room_id).await.unwrap(); + let room = room.read().await; + + let display_name1 = room.get_member(&user_id1).unwrap().disambiguated_name(); + let display_name2 = room.get_member(&user_id2).unwrap().disambiguated_name(); + + assert_eq!(format!("example ({})", user_id1), display_name1); + assert_eq!(format!("example ({})", user_id2), display_name2); + } + + // First member changes his display name to "changed". None of the display names are + // disambiguated. + client + .receive_sync_response(&mut member1_name_change_sync_response) + .await + .unwrap(); + + { + let room = client.get_joined_room(&room_id).await.unwrap(); + let room = room.read().await; + + let display_name1 = room.get_member(&user_id1).unwrap().disambiguated_name(); + let display_name2 = room.get_member(&user_id2).unwrap().disambiguated_name(); + + assert_eq!("changed", display_name1); + assert_eq!("example", display_name2); + } + + // Second member *also* changes his display name to "changed". Again, both display name are + // disambiguated. + client + .receive_sync_response(&mut member2_name_change_sync_response) + .await + .unwrap(); + + { + let room = client.get_joined_room(&room_id).await.unwrap(); + let room = room.read().await; + + let display_name1 = room.get_member(&user_id1).unwrap().disambiguated_name(); + let display_name2 = room.get_member(&user_id2).unwrap().disambiguated_name(); + + assert_eq!(format!("changed ({})", user_id1), display_name1); + assert_eq!(format!("changed ({})", user_id2), display_name2); + } } #[async_test] diff --git a/matrix_sdk_base/src/models/room_member.rs b/matrix_sdk_base/src/models/room_member.rs index 6b56d49c..de0a50f9 100644 --- a/matrix_sdk_base/src/models/room_member.rs +++ b/matrix_sdk_base/src/models/room_member.rs @@ -15,16 +15,14 @@ use std::convert::TryFrom; -use crate::events::presence::{PresenceEvent, PresenceEventContent, PresenceState}; -use crate::events::room::{ - member::{MemberEventContent, MembershipChange, MembershipState}, - power_levels::PowerLevelsEventContent, -}; +use crate::events::presence::{PresenceEvent, PresenceState}; +use crate::events::room::member::MemberEventContent; use crate::events::StateEventStub; use crate::identifiers::{RoomId, UserId}; -use crate::js_int::{int, Int, UInt}; +use crate::js_int::{Int, UInt}; use serde::{Deserialize, Serialize}; + // Notes: if Alice invites Bob into a room we will get an event with the sender as Alice and the state key as Bob. #[derive(Debug, Serialize, Deserialize, Clone)] @@ -34,6 +32,9 @@ pub struct RoomMember { pub user_id: UserId, /// The human readable name of the user. pub display_name: Option, + /// Whether the member's display name is ambiguous due to being shared with + /// other members. + pub display_name_ambiguous: bool, /// The matrix url of the users avatar. pub avatar_url: Option, /// The time, in ms, since the user interacted with the server. @@ -52,10 +53,18 @@ pub struct RoomMember { pub power_level: Option, /// The normalized power level of this `RoomMember` (0-100). pub power_level_norm: Option, - /// The `MembershipState` of this `RoomMember`. - pub membership: MembershipState, /// The human readable name of this room member. pub name: String, + // FIXME: The docstring below is currently a lie since we only store the initial event that + // creates the member (the one we pass to RoomMember::new). + // + // The intent of this field is to keep the last (or last few?) state events related to the room + // member cached so we can quickly go back to the previous one in case some of them get + // redacted. Keeping all state for each room member is probably too much. + // + // Needs design. + /// The events that created the state of this room member. + pub events: Vec>, /// The `PresenceEvent`s connected to this user. pub presence_events: Vec, } @@ -67,6 +76,7 @@ impl PartialEq for RoomMember { && self.user_id == other.user_id && self.name == other.name && self.display_name == other.display_name + && self.display_name_ambiguous == other.display_name_ambiguous && self.avatar_url == other.avatar_url && self.last_active_ago == other.last_active_ago } @@ -79,6 +89,7 @@ impl RoomMember { room_id: room_id.clone(), user_id: UserId::try_from(event.state_key.as_str()).unwrap(), display_name: event.content.displayname.clone(), + display_name_ambiguous: false, avatar_url: event.content.avatar_url.clone(), presence: None, status_msg: None, @@ -87,12 +98,13 @@ impl RoomMember { typing: None, power_level: None, power_level_norm: None, - membership: event.content.membership, - presence_events: vec![], + presence_events: Vec::default(), + events: vec![event.clone()], } } - /// Returns the most ergonomic name available for the member. + /// Returns the most ergonomic (but potentially ambiguous/non-unique) name + /// available for the member. /// /// This is the member's display name if it is set, otherwise their MXID. pub fn name(&self) -> String { @@ -101,10 +113,11 @@ impl RoomMember { .unwrap_or_else(|| format!("{}", self.user_id)) } - /// Returns a name for the member which is guaranteed to be unique. + /// Returns a name for the member which is guaranteed to be unique, but not + /// necessarily the most ergonomic. /// - /// This is either of the format "DISPLAY_NAME (MXID)" if the display name is set for the - /// member, or simply "MXID" if not. + /// This is either a name in the format "DISPLAY_NAME (MXID)" if the + /// member's display name is set, or simply "MXID" if not. pub fn unique_name(&self) -> String { self.display_name .clone() @@ -112,100 +125,28 @@ impl RoomMember { .unwrap_or_else(|| format!("{}", self.user_id)) } - /// Handle profile updates. - pub(crate) fn update_profile(&mut self, event: &StateEventStub) -> bool { - use MembershipChange::*; - - match event.membership_change() { - // we assume that the profile has changed - ProfileChanged { .. } => { - self.display_name = event.content.displayname.clone(); - self.avatar_url = event.content.avatar_url.clone(); - true - } - - // We're only interested in profile changes here. - _ => false, - } - } - - pub fn update_power( - &mut self, - event: &StateEventStub, - max_power: Int, - ) -> bool { - let changed; - if let Some(user_power) = event.content.users.get(&self.user_id) { - changed = self.power_level != Some(*user_power); - self.power_level = Some(*user_power); + /// Get the disambiguated display name for the member which is as ergonomic + /// as possible while still guaranteeing it is unique. + /// + /// If the member's display name is currently ambiguous (i.e. shared by + /// other room members), this method will return the same result as + /// `RoomMember::unique_name`. Otherwise, this method will return the same + /// result as `RoomMember::name`. + /// + /// This is usually the name you want when showing room messages from the + /// member or when showing the member in the member list. + /// + /// **Warning**: When displaying a room member's display name, clients + /// *must* use a disambiguated name, so they *must not* use + /// `RoomMember::display_name` directly. Clients *should* use this method to + /// obtain the name, but an acceptable alternative is to use + /// `RoomMember::unique_name` in certain situations. + pub fn disambiguated_name(&self) -> String { + if self.display_name_ambiguous { + self.unique_name().into() } else { - changed = self.power_level != Some(event.content.users_default); - self.power_level = Some(event.content.users_default); + self.name().into() } - - if max_power > int!(0) { - self.power_level_norm = Some((self.power_level.unwrap() * int!(100)) / max_power); - } - - changed - } - - /// If the current `PresenceEvent` updated the state of this `User`. - /// - /// Returns true if the specific users presence has changed, false otherwise. - /// - /// # Arguments - /// - /// * `presence` - The presence event for a this room member. - pub fn did_update_presence(&self, presence: &PresenceEvent) -> bool { - let PresenceEvent { - content: - PresenceEventContent { - avatar_url, - currently_active, - displayname, - last_active_ago, - presence, - status_msg, - }, - .. - } = presence; - self.display_name == *displayname - && self.avatar_url == *avatar_url - && self.presence.as_ref() == Some(presence) - && self.status_msg == *status_msg - && self.last_active_ago == *last_active_ago - && self.currently_active == *currently_active - } - - /// Updates the `User`s presence. - /// - /// This should only be used if `did_update_presence` was true. - /// - /// # Arguments - /// - /// * `presence` - The presence event for a this room member. - pub fn update_presence(&mut self, presence_ev: &PresenceEvent) { - let PresenceEvent { - content: - PresenceEventContent { - avatar_url, - currently_active, - displayname, - last_active_ago, - presence, - status_msg, - }, - .. - } = presence_ev; - - self.presence_events.push(presence_ev.clone()); - self.avatar_url = avatar_url.clone(); - self.currently_active = *currently_active; - self.display_name = displayname.clone(); - self.last_active_ago = *last_active_ago; - self.presence = Some(*presence); - self.status_msg = status_msg.clone(); } } @@ -234,7 +175,9 @@ mod test { client } - fn get_room_id() -> RoomId { + // TODO: Move this to EventBuilder since it's a magic room ID used in EventBuilder's example + // events. + fn test_room_id() -> RoomId { RoomId::try_from("!SVkFJHzfwvuaIEawgC:localhost").unwrap() } @@ -242,11 +185,11 @@ mod test { async fn room_member_events() { let client = get_client().await; - let room_id = get_room_id(); + let room_id = test_room_id(); let mut response = EventBuilder::default() - .add_state_event(EventsJson::Member) - .add_state_event(EventsJson::PowerLevels) + .add_room_event(EventsJson::Member) + .add_room_event(EventsJson::PowerLevels) .build_sync_response(); client.receive_sync_response(&mut response).await.unwrap(); @@ -261,15 +204,65 @@ mod test { assert_eq!(member.power_level, Some(int!(100))); } + #[async_test] + async fn room_member_display_name_change() { + let client = get_client().await; + let room_id = test_room_id(); + + let mut builder = EventBuilder::default(); + let mut initial_response = builder + .add_room_event(EventsJson::Member) + .build_sync_response(); + let mut name_change_response = builder + .add_room_event(EventsJson::MemberNameChange) + .build_sync_response(); + + client + .receive_sync_response(&mut initial_response) + .await + .unwrap(); + + let room = client.get_joined_room(&room_id).await.unwrap(); + + // Initially, the display name is "example". + { + let room = room.read().await; + + let member = room + .joined_members + .get(&UserId::try_from("@example:localhost").unwrap()) + .unwrap(); + + assert_eq!(member.display_name.as_ref().unwrap(), "example"); + } + + client + .receive_sync_response(&mut name_change_response) + .await + .unwrap(); + + // Afterwards, the display name is "changed". + { + let room = room.read().await; + + let member = room + .joined_members + .get(&UserId::try_from("@example:localhost").unwrap()) + .unwrap(); + + assert_eq!(member.display_name.as_ref().unwrap(), "changed"); + } + } + #[async_test] async fn member_presence_events() { let client = get_client().await; - let room_id = get_room_id(); + let room_id = test_room_id(); let mut response = EventBuilder::default() - .add_state_event(EventsJson::Member) - .add_state_event(EventsJson::PowerLevels) + .add_room_event(EventsJson::Member) + .add_room_event(EventsJson::PowerLevels) .add_presence_event(EventsJson::Presence) .build_sync_response(); diff --git a/matrix_sdk_base/src/state/mod.rs b/matrix_sdk_base/src/state/mod.rs index c26f13bb..3b5d0203 100644 --- a/matrix_sdk_base/src/state/mod.rs +++ b/matrix_sdk_base/src/state/mod.rs @@ -159,7 +159,6 @@ mod test { "creator": null, "joined_members": {}, "invited_members": {}, - "disambiguated_display_names": {}, "typing_users": [], "power_levels": null, "encrypted": null, @@ -176,7 +175,6 @@ mod test { serde_json::json!({ "!roomid:example.com": { "room_id": "!roomid:example.com", - "disambiguated_display_names": {}, "room_name": { "name": null, "canonical_alias": null, diff --git a/matrix_sdk_test/src/lib.rs b/matrix_sdk_test/src/lib.rs index 2538ce77..3755e5d7 100644 --- a/matrix_sdk_test/src/lib.rs +++ b/matrix_sdk_test/src/lib.rs @@ -26,6 +26,7 @@ pub enum EventsJson { HistoryVisibility, JoinRules, Member, + MemberNameChange, MessageEmote, MessageNotice, MessageText, @@ -42,7 +43,40 @@ pub enum EventsJson { Typing, } -/// Easily create events to stream into either a Client or a `Room` for testing. +/// The `EventBuilder` struct can be used to easily generate valid sync responses for testing. +/// These can be then fed into either `Client` or `Room`. +/// +/// It supports generated a number of canned events, such as a member entering a room, his power +/// level and display name changing and similar. It also supports insertion of custom events in the +/// form of `EventsJson` values. +/// +/// **Important** You *must* use the *same* builder when sending multiple sync responses to +/// a single client. Otherwise, the subsequent responses will be *ignored* by the client because +/// the `next_batch` sync token will not be rotated properly. +/// +/// # Example usage +/// +/// ```rust +/// use matrix_sdk_test::{EventBuilder, EventsJson}; +/// +/// let mut builder = EventBuilder::new(); +/// +/// // response1 now contains events that add an example member to the room and change their power +/// // level +/// let response1 = builder +/// .add_room_event(EventsJson::Member) +/// .add_room_event(EventsJson::PowerLevels) +/// .build_sync_response(); +/// +/// // response2 is now empty (nothing changed) +/// let response2 = builder.build_sync_response(); +/// +/// // response3 contains a display name change for member example +/// let response3 = builder +/// .add_room_event(EventsJson::MemberNameChange) +/// .build_sync_response(); +/// ``` + #[derive(Default)] pub struct EventBuilder { /// The events that determine the state of a `Room`. @@ -97,6 +131,7 @@ impl EventBuilder { pub fn add_room_event(&mut self, json: EventsJson) -> &mut Self { let val: &JsonValue = match json { EventsJson::Member => &test_json::MEMBER, + EventsJson::MemberNameChange => &test_json::MEMBER_NAME_CHANGE, EventsJson::PowerLevels => &test_json::POWER_LEVELS, _ => panic!("unknown room event json {:?}", json), }; @@ -181,7 +216,8 @@ impl EventBuilder { self } - /// Consumes `ResponseBuilder` and returns `SyncResponse`. + /// Builds a `SyncResponse` containing the events we queued so far. The next response returned + /// by `build_sync_response` will then be empty if no further events were queued. pub fn build_sync_response(&mut self) -> SyncResponse { let main_room_id = RoomId::try_from("!SVkFJHzfwvuaIEawgC:localhost").unwrap(); @@ -293,12 +329,26 @@ impl EventBuilder { let response = Response::builder() .body(serde_json::to_vec(&body).unwrap()) .unwrap(); + + // Clear state so that the next sync response will be empty if nothing was added. + self.clear(); + SyncResponse::try_from(response).unwrap() } fn generate_sync_token(&self) -> String { format!("t392-516_47314_0_7_1_1_1_11444_{}", self.batch_counter) } + + pub fn clear(&mut self) { + self.account_data.clear(); + self.ephemeral.clear(); + self.invited_room_events.clear(); + self.joined_room_events.clear(); + self.left_room_events.clear(); + self.presence_events.clear(); + self.state_events.clear(); + } } /// Embedded sync reponse files diff --git a/matrix_sdk_test/src/test_json/events.rs b/matrix_sdk_test/src/test_json/events.rs index a4ba0ced..da3bc5a4 100644 --- a/matrix_sdk_test/src/test_json/events.rs +++ b/matrix_sdk_test/src/test_json/events.rs @@ -208,6 +208,7 @@ lazy_static! { }); } +// TODO: Move `prev_content` into `unsigned` once ruma supports it lazy_static! { pub static ref MEMBER: JsonValue = json!({ "content": { @@ -221,14 +222,40 @@ lazy_static! { "sender": "@example:localhost", "state_key": "@example:localhost", "type": "m.room.member", + "prev_content": { + "avatar_url": null, + "displayname": "example", + "membership": "invite" + }, "unsigned": { "age": 297036, - "replaces_state": "$151800111315tsynI:localhost", - "prev_content": { - "avatar_url": null, - "displayname": "example", - "membership": "invite" - } + "replaces_state": "$151800111315tsynI:localhost" + } + }); +} + +// TODO: Move `prev_content` into `unsigned` once ruma supports it +lazy_static! { + pub static ref MEMBER_NAME_CHANGE: JsonValue = json!({ + "content": { + "avatar_url": null, + "displayname": "changed", + "membership": "join" + }, + "event_id": "$151800234427abgho:localhost", + "membership": "join", + "origin_server_ts": 151800152, + "sender": "@example:localhost", + "state_key": "@example:localhost", + "type": "m.room.member", + "prev_content": { + "avatar_url": null, + "displayname": "example", + "membership": "join" + }, + "unsigned": { + "age": 297032, + "replaces_state": "$151800140517rfvjc:localhost" } }); } @@ -552,6 +579,7 @@ lazy_static! { }); } +// TODO: Move `prev_content` into `unsigned` once ruma supports it lazy_static! { pub static ref TOPIC: JsonValue = json!({ "content": { @@ -562,11 +590,11 @@ lazy_static! { "sender": "@example:localhost", "state_key": "", "type": "m.room.topic", + "prev_content": { + "topic": "test" + }, "unsigned": { "age": 1392989, - "prev_content": { - "topic": "test" - }, "prev_sender": "@example:localhost", "replaces_state": "$151957069225EVYKm:localhost" } diff --git a/matrix_sdk_test/src/test_json/mod.rs b/matrix_sdk_test/src/test_json/mod.rs index 474a397f..b85e0bad 100644 --- a/matrix_sdk_test/src/test_json/mod.rs +++ b/matrix_sdk_test/src/test_json/mod.rs @@ -9,8 +9,8 @@ pub mod sync; pub use events::{ ALIAS, ALIASES, EVENT_ID, KEYS_QUERY, KEYS_UPLOAD, LOGIN, LOGIN_RESPONSE_ERR, LOGOUT, MEMBER, - MESSAGE_EDIT, MESSAGE_TEXT, NAME, POWER_LEVELS, PRESENCE, PUBLIC_ROOMS, REACTION, REDACTED, - REDACTED_INVALID, REDACTED_STATE, REDACTION, REGISTRATION_RESPONSE_ERR, ROOM_ID, ROOM_MESSAGES, - TYPING, + MEMBER_NAME_CHANGE, MESSAGE_EDIT, MESSAGE_TEXT, NAME, POWER_LEVELS, PRESENCE, PUBLIC_ROOMS, + REACTION, REDACTED, REDACTED_INVALID, REDACTED_STATE, REDACTION, REGISTRATION_RESPONSE_ERR, + ROOM_ID, ROOM_MESSAGES, TYPING, }; pub use sync::{DEFAULT_SYNC_SUMMARY, INVITE_SYNC, LEAVE_SYNC, LEAVE_SYNC_EVENT, MORE_SYNC, SYNC};