Merge branch 'feature/display-name'

matrix_sdk/examples/login.rs

@@ -24,12 +24,8 @@ impl EventEmitter for EventCallback {
                     // any reads should be held for the shortest time possible to
                     // avoid dead locks
                     let room = room.read().await;
-                    let member = room.members.get(&sender).unwrap();
+                    let member = room.joined_members.get(&sender).unwrap();
-                    member
+                    member.name()
-                        .display_name
-                        .as_ref()
-                        .map(ToString::to_string)
-                        .unwrap_or(sender.to_string())
                 };
                 println!("{}: {}", name, msg_body);
             }

matrix_sdk/src/client.rs

@@ -872,8 +872,11 @@ impl Client {
                 let missing_sessions = {
                     let room = self.base_client.get_joined_room(room_id).await;
                     let room = room.as_ref().unwrap().read().await;
-                    let users = room.members.keys();
+                    let members = room
-                    self.base_client.get_missing_sessions(users).await?
+                        .joined_members
+                        .keys()
+                        .chain(room.invited_members.keys());
+                    self.base_client.get_missing_sessions(members).await?
                 };
 
                 if !missing_sessions.is_empty() {
@@ -1398,7 +1401,6 @@ mod test {
     };
     use super::{Client, ClientConfig, Session, SyncSettings, Url};
     use crate::events::collections::all::RoomEvent;
-    use crate::events::room::member::MembershipState;
     use crate::events::room::message::TextMessageEventContent;
     use crate::identifiers::{EventId, RoomId, RoomIdOrAliasId, UserId};
     use crate::RegistrationBuilder;
@@ -2079,11 +2081,7 @@ mod test {
             .read()
             .await;
 
-        assert_eq!(2, room.members.len());
+        assert_eq!(1, room.joined_members.len());
-        for member in room.members.values() {
-            assert_eq!(MembershipState::Join, member.membership);
-        }
-
         assert!(room.power_levels.is_some())
     }
 
@@ -2116,7 +2114,7 @@ mod test {
             room_names.push(room.read().await.display_name())
         }
 
-        assert_eq!(vec!["example, example2"], room_names);
+        assert_eq!(vec!["example2"], room_names);
     }
 
     #[tokio::test]

matrix_sdk_base/src/client.rs

@@ -82,8 +82,15 @@ pub struct AdditionalUnsignedData {
     pub prev_content: Option<EventJson<MemberEventContent>>,
 }
 
-/// If a `prev_content` field is found inside of `unsigned` we move it up to the events `prev_content` field.
+/// Transform room event by hoisting `prev_content` field from `unsigned` to the top level.
-fn deserialize_prev_content(event: &EventJson<RoomEvent>) -> Option<EventJson<RoomEvent>> {
+///
+/// Due to a [bug in synapse][synapse-bug], `prev_content` often ends up in `unsigned` contrary to
+/// the C2S spec. Some more discussion can be found [here][discussion]. Until this is fixed in
+/// synapse or handled in Ruma, we use this to hoist up `prev_content` to the top level.
+///
+/// [synapse-bug]: <https://github.com/matrix-org/matrix-doc/issues/684#issuecomment-641182668>
+/// [discussion]: <https://github.com/matrix-org/matrix-doc/issues/684#issuecomment-641182668>
+fn hoist_room_event_prev_content(event: &mut EventJson<RoomEvent>) -> Option<EventJson<RoomEvent>> {
     let prev_content = serde_json::from_str::<AdditionalEventData>(event.json().get())
         .map(|more_unsigned| more_unsigned.unsigned)
         .map(|additional| additional.prev_content)
@@ -93,7 +100,33 @@ fn deserialize_prev_content(event: &EventJson<RoomEvent>) -> Option<EventJson<Ro
     let mut ev = event.deserialize().ok()?;
     match &mut ev {
         RoomEvent::RoomMember(ref mut member) if member.prev_content.is_none() => {
-            member.prev_content = prev_content.deserialize().ok();
+            if let Ok(prev) = prev_content.deserialize() {
+                member.prev_content = Some(prev)
+            }
+
+            Some(EventJson::from(ev))
+        }
+        _ => None,
+    }
+}
+
+/// Transform state event by hoisting `prev_content` field from `unsigned` to the top level.
+///
+/// See comment of `hoist_room_event_prev_content`.
+fn hoist_state_event_prev_content(event: &EventJson<StateEvent>) -> Option<EventJson<StateEvent>> {
+    let prev_content = serde_json::from_str::<AdditionalEventData>(event.json().get())
+        .map(|more_unsigned| more_unsigned.unsigned)
+        .map(|additional| additional.prev_content)
+        .ok()
+        .flatten()?;
+
+    let mut ev = event.deserialize().ok()?;
+    match &mut ev {
+        StateEvent::RoomMember(ref mut member) if member.prev_content.is_none() => {
+            if let Ok(prev) = prev_content.deserialize() {
+                member.prev_content = Some(prev)
+            }
+
             Some(EventJson::from(ev))
         }
         _ => None,
@@ -675,13 +708,6 @@ impl BaseClient {
         room_id: &RoomId,
         event: &mut EventJson<RoomEvent>,
     ) -> Result<(Option<EventJson<RoomEvent>>, bool)> {
-        // if the event is a m.room.member event the server will sometimes
-        // send the `prev_content` field as part of the unsigned field this extracts and
-        // places it where everything else expects it.
-        if let Some(e) = deserialize_prev_content(event) {
-            *event = e;
-        }
-
         match event.deserialize() {
             #[allow(unused_mut)]
             Ok(mut e) => {
@@ -941,7 +967,13 @@ impl BaseClient {
         let mut updated = false;
         for (room_id, joined_room) in &mut response.rooms.join {
             let matrix_room = {
-                for event in &joined_room.state.events {
+                for event in &mut joined_room.state.events {
+                    // XXX: Related to `prev_content` and `unsigned`; see the doc comment of
+                    // `hoist_room_event_prev_content`
+                    if let Some(e) = hoist_state_event_prev_content(event) {
+                        *event = e;
+                    }
+
                     if let Ok(e) = event.deserialize() {
                         if self.receive_joined_state_event(&room_id, &e).await? {
                             updated = true;
@@ -963,18 +995,19 @@ impl BaseClient {
 
                     // If the room is encrypted, update the tracked users.
                     if room.is_encrypted() {
-                        o.update_tracked_users(room.members.keys()).await;
+                        o.update_tracked_users(room.joined_members.keys()).await;
+                        o.update_tracked_users(room.invited_members.keys()).await;
                     }
                 }
             }
 
-            // RoomSummary contains information for calculating room name
+            // RoomSummary contains information for calculating room name.
             matrix_room
                 .write()
                 .await
                 .set_room_summary(&joined_room.summary);
 
-            // set unread notification count
+            // Set unread notification count.
             matrix_room
                 .write()
                 .await
@@ -995,7 +1028,9 @@ impl BaseClient {
                     *event = e;
                 }
 
-                if let Some(e) = deserialize_prev_content(&event) {
+                // XXX: Related to `prev_content` and `unsigned`; see the doc comment of
+                // `hoist_room_event_prev_content`
+                if let Some(e) = hoist_room_event_prev_content(event) {
                     *event = e;
                 }
 
@@ -1070,7 +1105,13 @@ impl BaseClient {
         let mut updated = false;
         for (room_id, left_room) in &mut response.rooms.leave {
             let matrix_room = {
-                for event in &left_room.state.events {
+                for event in &mut left_room.state.events {
+                    // XXX: Related to `prev_content` and `unsigned`; see the doc comment of
+                    // `hoist_room_event_prev_content`
+                    if let Some(e) = hoist_state_event_prev_content(event) {
+                        *event = e;
+                    }
+
                     if let Ok(e) = event.deserialize() {
                         if self.receive_left_state_event(&room_id, &e).await? {
                             updated = true;
@@ -1089,7 +1130,9 @@ impl BaseClient {
             }
 
             for event in &mut left_room.timeline.events {
-                if let Some(e) = deserialize_prev_content(&event) {
+                // XXX: Related to `prev_content` and `unsigned`; see the doc comment of
+                // `hoist_room_event_prev_content`
+                if let Some(e) = hoist_room_event_prev_content(event) {
                     *event = e;
                 }
 
@@ -1241,8 +1284,13 @@ impl BaseClient {
         match &mut *olm {
             Some(o) => {
                 let room = room.write().await;
-                let members = room.members.keys();
+
-                Ok(o.share_group_session(room_id, members).await?)
+                // XXX: We construct members in a slightly roundabout way instead of chaining the
+                // iterators directly because of https://github.com/rust-lang/rust/issues/64552
+                let joined_members = room.joined_members.keys();
+                let invited_members = room.joined_members.keys();
+                let members: Vec<&UserId> = joined_members.chain(invited_members).collect();
+                Ok(o.share_group_session(room_id, members.into_iter()).await?)
             }
             None => panic!("Olm machine wasn't started"),
         }

matrix_sdk_base/src/event_emitter/mod.rs

@@ -95,7 +95,7 @@ pub enum CustomOrRawEvent<'c> {
 ///             {
 ///                 let name = {
 ///                    let room = room.read().await;
-///                    let member = room.members.get(&sender).unwrap();
+///                    let member = room.joined_members.get(&sender).unwrap();
 ///                    member
 ///                        .display_name
 ///                        .as_ref()

matrix_sdk_base/src/models/message.rs

@@ -178,6 +178,7 @@ mod test {
             serde_json::json!({
                 "!roomid:example.com": {
                     "room_id": "!roomid:example.com",
+                    "disambiguated_display_names": {},
                     "room_name": {
                         "name": null,
                         "canonical_alias": null,
@@ -188,7 +189,8 @@ mod test {
                     },
                     "own_user_id": "@example:example.com",
                     "creator": null,
-                    "members": {},
+                    "joined_members": {},
+                    "invited_members": {},
                     "messages": [ message ],
                     "typing_users": [],
                     "power_levels": null,
@@ -227,6 +229,7 @@ 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,
@@ -237,7 +240,8 @@ mod test {
                 },
                 "own_user_id": "@example:example.com",
                 "creator": null,
-                "members": {},
+                "joined_members": {},
+                "invited_members": {},
                 "messages": [ message ],
                 "typing_users": [],
                 "power_levels": null,

matrix_sdk_base/src/models/room.rs

@@ -13,6 +13,7 @@
 // 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::convert::TryFrom;
 
@@ -143,6 +144,12 @@ pub struct Tombstone {
     replacement: RoomId,
 }
 
+#[derive(Debug, PartialEq, Eq)]
+enum MemberDirection {
+    Entering,
+    Exiting,
+}
+
 #[derive(Debug, PartialEq, Serialize, Deserialize, Clone)]
 /// A Matrix room.
 pub struct Room {
@@ -154,8 +161,11 @@ pub struct Room {
     pub own_user_id: UserId,
     /// The mxid of the room creator.
     pub creator: Option<UserId>,
-    /// The map of room members.
+    // TODO: Track banned members, e.g. for /unban support?
-    pub members: HashMap<UserId, RoomMember>,
+    /// The map of invited room members.
+    pub invited_members: HashMap<UserId, RoomMember>,
+    /// The map of joined room members.
+    pub joined_members: HashMap<UserId, RoomMember>,
     /// 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
@@ -176,6 +186,8 @@ pub struct Room {
     pub unread_notifications: Option<UInt>,
     /// The tombstone state of this room.
     pub tombstone: Option<Tombstone>,
+    /// The map of disambiguated display names for users who have the same display name
+    disambiguated_display_names: HashMap<UserId, String>,
 }
 
 impl RoomName {
@@ -194,9 +206,19 @@ impl RoomName {
         true
     }
 
-    pub fn calculate_name(&self, members: &HashMap<UserId, RoomMember>) -> String {
+    /// Calculate the canonical display name of a room, taking into account its name, aliases and
-        // https://matrix.org/docs/spec/client_server/latest#calculating-the-display-name-for-a-room.
+    /// members.
-        // the order in which we check for a name ^^
+    ///
+    /// The display name is calculated according to [this algorithm][spec].
+    ///
+    /// [spec]:
+    /// <https://matrix.org/docs/spec/client_server/latest#calculating-the-display-name-for-a-room>
+    pub fn calculate_name(
+        &self,
+        own_user_id: &UserId,
+        invited_members: &HashMap<UserId, RoomMember>,
+        joined_members: &HashMap<UserId, RoomMember>,
+    ) -> String {
         if let Some(name) = &self.name {
             let name = name.trim();
             name.to_string()
@@ -217,10 +239,12 @@ impl RoomName {
                 invited + joined - one
             };
 
-            // TODO this should use `self.heroes but it is always empty??
+            let members = joined_members.values().chain(invited_members.values());
+
+            // TODO: This should use `self.heroes` but it is always empty??
             if heroes >= invited_joined {
                 let mut names = members
-                    .values()
+                    .filter(|m| m.user_id != *own_user_id)
                     .take(3)
                     .map(|mem| {
                         mem.display_name
@@ -233,7 +257,7 @@ impl RoomName {
                 names.join(", ")
             } else if heroes < invited_joined && invited + joined > one {
                 let mut names = members
-                    .values()
+                    .filter(|m| m.user_id != *own_user_id)
                     .take(3)
                     .map(|mem| {
                         mem.display_name
@@ -242,10 +266,11 @@ impl RoomName {
                     })
                     .collect::<Vec<String>>();
                 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 {
-                format!("Empty Room (was {} others)", members.len())
+                "Empty room".to_string()
             }
         }
     }
@@ -265,7 +290,8 @@ impl Room {
             room_name: RoomName::default(),
             own_user_id: own_user_id.clone(),
             creator: None,
-            members: HashMap::new(),
+            invited_members: HashMap::new(),
+            joined_members: HashMap::new(),
             #[cfg(feature = "messages")]
             messages: MessageQueue::new(),
             typing_users: Vec::new(),
@@ -274,12 +300,17 @@ impl Room {
             unread_highlight: None,
             unread_notifications: None,
             tombstone: None,
+            disambiguated_display_names: HashMap::new(),
         }
     }
 
     /// Return the display name of the room.
     pub fn display_name(&self) -> String {
-        self.room_name.calculate_name(&self.members)
+        self.room_name.calculate_name(
+            &self.own_user_id,
+            &self.invited_members,
+            &self.joined_members,
+        )
     }
 
     /// Is the room a encrypted room.
@@ -294,22 +325,199 @@ impl Room {
         self.encrypted.as_ref()
     }
 
+    /// Get the disambiguated display name for a member of this room.
+    ///
+    /// 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.
+    ///
+    /// 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_ref().into()
+        }
+    }
+
+    /// Process the member event of an entering user.
+    ///
+    /// Returns true if this made a change to the room's state, false otherwise.
     fn add_member(&mut self, event: &MemberEvent) -> bool {
-        if self
+        let new_member = RoomMember::new(event);
-            .members
+
-            .contains_key(&UserId::try_from(event.state_key.as_str()).unwrap())
+        if self.joined_members.contains_key(&new_member.user_id)
+            || self.invited_members.contains_key(&new_member.user_id)
         {
             return false;
         }
 
-        let member = RoomMember::new(event);
+        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.")
+            }
+        };
 
-        self.members
+        // Perform display name disambiguations, if necessary.
-            .insert(UserId::try_from(event.state_key.as_str()).unwrap(), member);
+        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: &MemberEvent) -> bool {
+        let leaving_member = RoomMember::new(event);
+
+        // 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),
+            };
+        }
+
+        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
+        }
+    }
+
+    /// 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<UserId> {
+        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.
+        members
+            .filter(|(_, existing_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)
+                    })
+                    .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()
+            .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.
+    ///
+    /// The `inclusive` parameter controls whether or not the member for which we are
+    /// disambiguating should be considered a current member of the room.
+    ///
+    /// Returns a map from MXID to disambiguated name.
+    fn member_disambiguations(
+        &self,
+        member: &RoomMember,
+        inclusive: bool,
+    ) -> HashMap<UserId, String> {
+        let users_with_same_name = self.shares_displayname_with(member, inclusive);
+        let disambiguate_with = |members: Vec<UserId>, 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::<HashMap<UserId, String>>()
+        };
+
+        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.
+    fn disambiguation_updates(
+        &self,
+        member: &RoomMember,
+        when: MemberDirection,
+    ) -> HashMap<UserId, Option<String>> {
+        let before;
+        let after;
+
+        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 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))
+                }
+            })
+            .collect()
+    }
+
     /// Add to the list of `RoomAliasId`s.
     fn push_room_alias(&mut self, alias: &RoomAliasId) -> bool {
         self.room_name.push_alias(alias.clone());
@@ -376,22 +584,31 @@ impl Room {
     ///
     /// Returns true if the joined member list changed, false otherwise.
     pub fn handle_membership(&mut self, event: &MemberEvent) -> bool {
-        // TODO this would not be handled correctly as all the MemberEvents have the `prev_content`
+        use MembershipChange::*;
-        // inside of `unsigned` field
+
+        // TODO: This would not be handled correctly as all the MemberEvents have the `prev_content`
+        // inside of `unsigned` field.
         match event.membership_change() {
-            MembershipChange::Invited | MembershipChange::Joined => self.add_member(event),
+            Invited | Joined => self.add_member(event),
-            _ => {
+            Kicked | Banned | KickedAndBanned | InvitationRejected | Left => {
-                let user = if let Ok(id) = UserId::try_from(event.state_key.as_str()) {
+                self.remove_member(event)
+            }
+            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.members.get_mut(&user) {
+
-                    member.update_member(event)
+                if let Some(member) = self.joined_members.get_mut(&user_id) {
+                    member.update_profile(event)
                 } else {
                     false
                 }
             }
+
+            // Not interested in other events.
+            _ => false,
         }
     }
 
@@ -458,7 +675,7 @@ impl Room {
         }
 
         for user in event.content.users.keys() {
-            if let Some(member) = self.members.get_mut(user) {
+            if let Some(member) = self.joined_members.get_mut(user) {
                 if member.update_power(event, max_power) {
                     updated = true;
                 }
@@ -553,7 +770,7 @@ impl Room {
     ///
     /// * `event` - The presence event for a specified room member.
     pub fn receive_presence_event(&mut self, event: &PresenceEvent) -> bool {
-        if let Some(member) = self.members.get_mut(&event.sender) {
+        if let Some(member) = self.joined_members.get_mut(&event.sender) {
             if member.did_update_presence(event) {
                 false
             } else {
@@ -571,10 +788,7 @@ impl Room {
 #[cfg(test)]
 mod test {
     use super::*;
-    use crate::events::{
+    use crate::events::{room::encryption::EncryptionEventContent, UnsignedData};
-        room::{encryption::EncryptionEventContent, member::MembershipState},
-        UnsignedData,
-    };
     use crate::identifiers::{EventId, UserId};
     use crate::{BaseClient, Session};
     use matrix_sdk_test::{async_test, sync_response, EventBuilder, EventsJson, SyncResponseFile};
@@ -618,12 +832,190 @@ mod test {
             .read()
             .await;
 
-        assert_eq!(2, room.members.len());
+        assert_eq!(1, room.joined_members.len());
-        for member in room.members.values() {
+        assert!(room.deref().power_levels.is_some())
-            assert_eq!(MembershipState::Join, member.membership);
+    }
+
+    #[async_test]
+    async fn test_member_display_name() {
+        // Initialize
+
+        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 user_id3 = UserId::try_from("@example3:localhost").unwrap();
+
+        let member2_join_event = serde_json::json!({
+            "content": {
+                "avatar_url": null,
+                "displayname": "example",
+                "membership": "join"
+            },
+            "event_id": "$16345217l517tabbz:localhost",
+            "membership": "join",
+            "origin_server_ts": 1455123234,
+            "sender": format!("{}", user_id2),
+            "state_key": format!("{}", user_id2),
+            "type": "m.room.member",
+            "prev_content": {
+                "avatar_url": null,
+                "displayname": "example",
+                "membership": "invite"
+            },
+            "unsigned": {
+                "age": 1989321234,
+                "replaces_state": "$1622a2311315tkjoA:localhost"
+            }
+        });
+
+        let member2_leave_event = serde_json::json!({
+            "content": {
+                "avatar_url": null,
+                "displayname": "example",
+                "membership": "leave"
+            },
+            "event_id": "$263452333l22bggbz:localhost",
+            "membership": "leave",
+            "origin_server_ts": 1455123228,
+            "sender": format!("{}", user_id2),
+            "state_key": format!("{}", user_id2),
+            "type": "m.room.member",
+            "prev_content": {
+                "avatar_url": null,
+                "displayname": "example",
+                "membership": "join"
+            },
+            "unsigned": {
+                "age": 1989321221,
+                "replaces_state": "$16345217l517tabbz:localhost"
+            }
+        });
+
+        let member3_join_event = serde_json::json!({
+            "content": {
+                "avatar_url": null,
+                "displayname": "example",
+                "membership": "join"
+            },
+            "event_id": "$16845287981ktggba:localhost",
+            "membership": "join",
+            "origin_server_ts": 1455123244,
+            "sender": format!("{}", user_id3),
+            "state_key": format!("{}", user_id3),
+            "type": "m.room.member",
+            "prev_content": {
+                "avatar_url": null,
+                "displayname": "example",
+                "membership": "invite"
+            },
+            "unsigned": {
+                "age": 1989321254,
+                "replaces_state": "$1622l2323445kabrA:localhost"
+            }
+        });
+
+        let member3_leave_event = serde_json::json!({
+            "content": {
+                "avatar_url": null,
+                "displayname": "example",
+                "membership": "leave"
+            },
+            "event_id": "$11121987981abfgr:localhost",
+            "membership": "leave",
+            "origin_server_ts": 1455123230,
+            "sender": format!("{}", user_id3),
+            "state_key": format!("{}", user_id3),
+            "type": "m.room.member",
+            "prev_content": {
+                "avatar_url": null,
+                "displayname": "example",
+                "membership": "join"
+            },
+            "unsigned": {
+                "age": 1989321244,
+                "replaces_state": "$16845287981ktggba:localhost"
+            }
+        });
+
+        let mut event_builder = EventBuilder::new();
+
+        let mut member1_join_sync_response = event_builder
+            .add_room_event(EventsJson::Member, RoomEvent::RoomMember)
+            .build_sync_response();
+
+        let mut member2_join_sync_response = event_builder
+            .add_custom_joined_event(&room_id, member2_join_event, RoomEvent::RoomMember)
+            .build_sync_response();
+
+        let mut member3_join_sync_response = event_builder
+            .add_custom_joined_event(&room_id, member3_join_event, RoomEvent::RoomMember)
+            .build_sync_response();
+
+        let mut member2_leave_sync_response = event_builder
+            .add_custom_joined_event(&room_id, member2_leave_event, RoomEvent::RoomMember)
+            .build_sync_response();
+
+        let mut member3_leave_sync_response = event_builder
+            .add_custom_joined_event(&room_id, member3_leave_event, RoomEvent::RoomMember)
+            .build_sync_response();
+
+        // First member with display name "example" joins
+        client
+            .receive_sync_response(&mut member1_join_sync_response)
+            .await
+            .unwrap();
+
+        // First member's disambiguated display name is "example"
+        {
+            let room = client.get_joined_room(&room_id).await.unwrap();
+            let room = room.read().await;
+            let display_name1 = room.member_display_name(&user_id1);
+
+            assert_eq!("example", display_name1);
         }
 
-        assert!(room.deref().power_levels.is_some())
+        // Second and third member with display name "example" join
+        client
+            .receive_sync_response(&mut member2_join_sync_response)
+            .await
+            .unwrap();
+        client
+            .receive_sync_response(&mut member3_join_sync_response)
+            .await
+            .unwrap();
+
+        // All of their display names are now disambiguated with MXIDs
+        {
+            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);
+
+            assert_eq!(format!("example ({})", user_id1), display_name1);
+            assert_eq!(format!("example ({})", user_id2), display_name2);
+            assert_eq!(format!("example ({})", user_id3), display_name3);
+        }
+
+        // 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)
+            .await
+            .unwrap();
+
+        {
+            let room = client.get_joined_room(&room_id).await.unwrap();
+            let room = room.read().await;
+
+            let display_name1 = room.member_display_name(&user_id1);
+
+            assert_eq!("example", display_name1);
+        }
     }
 
     #[async_test]
@@ -642,13 +1034,13 @@ mod test {
         let room = client.get_joined_room(&room_id).await.unwrap();
         let room = room.read().await;
 
-        assert_eq!(room.members.len(), 1);
+        assert_eq!(room.joined_members.len(), 1);
         assert!(room.power_levels.is_some());
         assert_eq!(
             room.power_levels.as_ref().unwrap().kick,
             crate::js_int::Int::new(50).unwrap()
         );
-        let admin = room.members.get(&user_id).unwrap();
+        let admin = room.joined_members.get(&user_id).unwrap();
         assert_eq!(
             admin.power_level.unwrap(),
             crate::js_int::Int::new(100).unwrap()
@@ -727,7 +1119,7 @@ mod test {
             room_names.push(room.read().await.display_name())
         }
 
-        assert_eq!(vec!["example, example2"], room_names);
+        assert_eq!(vec!["example2"], room_names);
     }
 
     #[async_test]

matrix_sdk_base/src/models/room_member.rs

@@ -18,7 +18,7 @@ use std::convert::TryFrom;
 use crate::events::collections::all::Event;
 use crate::events::presence::{PresenceEvent, PresenceEventContent, PresenceState};
 use crate::events::room::{
-    member::{MemberEvent, MembershipChange, MembershipState},
+    member::{MemberEvent, MembershipChange},
     power_levels::PowerLevelsEvent,
 };
 use crate::identifiers::UserId;
@@ -31,7 +31,7 @@ use serde::{Deserialize, Serialize};
 /// A Matrix room member.
 ///
 pub struct RoomMember {
-    /// The unique mxid of the user.
+    /// The unique MXID of the user.
     pub user_id: UserId,
     /// The human readable name of the user.
     pub display_name: Option<String>,
@@ -53,8 +53,6 @@ pub struct RoomMember {
     pub power_level: Option<Int>,
     /// The normalized power level of this `RoomMember` (0-100).
     pub power_level_norm: Option<Int>,
-    /// The `MembershipState` of this `RoomMember`.
-    pub membership: MembershipState,
     /// The human readable name of this room member.
     pub name: String,
     /// The events that created the state of this room member.
@@ -74,7 +72,6 @@ impl PartialEq for RoomMember {
             && self.display_name == other.display_name
             && self.avatar_url == other.avatar_url
             && self.last_active_ago == other.last_active_ago
-            && self.membership == other.membership
     }
 }
 
@@ -93,13 +90,33 @@ impl RoomMember {
             typing: None,
             power_level: None,
             power_level_norm: None,
-            membership: event.content.membership,
             presence_events: Vec::default(),
             events: vec![Event::RoomMember(event.clone())],
         }
     }
 
-    pub fn update_member(&mut self, event: &MemberEvent) -> bool {
+    /// Returns the most ergonomic name available for the member.
+    ///
+    /// This is the member's display name if it is set, otherwise their MXID.
+    pub fn name(&self) -> String {
+        self.display_name
+            .clone()
+            .unwrap_or_else(|| format!("{}", self.user_id))
+    }
+
+    /// Returns a name for the member which is guaranteed to be unique.
+    ///
+    /// This is either of the format "DISPLAY_NAME (MXID)" if the display name is set for the
+    /// member, or simply "MXID" if not.
+    pub fn unique_name(&self) -> String {
+        self.display_name
+            .clone()
+            .map(|d| format!("{} ({})", d, self.user_id))
+            .unwrap_or_else(|| format!("{}", self.user_id))
+    }
+
+    /// Handle profile updates.
+    pub(crate) fn update_profile(&mut self, event: &MemberEvent) -> bool {
         use MembershipChange::*;
 
         match event.membership_change() {
@@ -108,15 +125,9 @@ impl RoomMember {
                 self.avatar_url = event.content.avatar_url.clone();
                 true
             }
-            Banned | Kicked | KickedAndBanned | InvitationRejected | InvitationRevoked | Left
+
-            | Unbanned | Joined | Invited => {
+            // We're only interested in profile changes here.
-                self.membership = event.content.membership;
+            _ => false,
-                true
-            }
-            NotImplemented => false,
-            None => false,
-            // we ignore the error here as only a buggy or malicious server would send this
-            Error => false,
         }
     }
 
@@ -201,7 +212,6 @@ mod test {
     use matrix_sdk_test::{async_test, EventBuilder, EventsJson};
 
     use crate::events::collections::all::RoomEvent;
-    use crate::events::room::member::MembershipState;
     use crate::identifiers::{RoomId, UserId};
     use crate::{BaseClient, Session};
 
@@ -244,10 +254,9 @@ mod test {
         let room = room.read().await;
 
         let member = room
-            .members
+            .joined_members
             .get(&UserId::try_from("@example:localhost").unwrap())
             .unwrap();
-        assert_eq!(member.membership, MembershipState::Join);
         assert_eq!(member.power_level, Int::new(100));
     }
 
@@ -269,11 +278,10 @@ mod test {
         let room = room.read().await;
 
         let member = room
-            .members
+            .joined_members
             .get(&UserId::try_from("@example:localhost").unwrap())
             .unwrap();
 
-        assert_eq!(member.membership, MembershipState::Join);
         assert_eq!(member.power_level, Int::new(100));
 
         assert!(member.avatar_url.is_none());

matrix_sdk_base/src/state/mod.rs

@@ -144,57 +144,61 @@ mod test {
 
         #[cfg(not(feature = "messages"))]
         assert_eq!(
-            r#"{
+            serde_json::json!({
-  "!roomid:example.com": {
+                "!roomid:example.com": {
-    "room_id": "!roomid:example.com",
+                    "room_id": "!roomid:example.com",
-    "room_name": {
+                    "room_name": {
-      "name": null,
+                        "name": null,
-      "canonical_alias": null,
+                        "canonical_alias": null,
-      "aliases": [],
+                        "aliases": [],
-      "heroes": [],
+                        "heroes": [],
-      "joined_member_count": null,
+                        "joined_member_count": null,
-      "invited_member_count": null
+                        "invited_member_count": null
-    },
+                    },
-    "own_user_id": "@example:example.com",
+                    "own_user_id": "@example:example.com",
-    "creator": null,
+                    "creator": null,
-    "members": {},
+                    "joined_members": {},
-    "typing_users": [],
+                    "invited_members": {},
-    "power_levels": null,
+                    "disambiguated_display_names": {},
-    "encrypted": null,
+                    "typing_users": [],
-    "unread_highlight": null,
+                    "power_levels": null,
-    "unread_notifications": null,
+                    "encrypted": null,
-    "tombstone": null
+                    "unread_highlight": null,
-  }
+                    "unread_notifications": null,
-}"#,
+                    "tombstone": null
-            serde_json::to_string_pretty(&joined_rooms).unwrap()
+                }
+            }),
+            serde_json::to_value(&joined_rooms).unwrap()
         );
 
         #[cfg(feature = "messages")]
         assert_eq!(
-            r#"{
+            serde_json::json!({
-  "!roomid:example.com": {
+                "!roomid:example.com": {
-    "room_id": "!roomid:example.com",
+                    "room_id": "!roomid:example.com",
-    "room_name": {
+                    "disambiguated_display_names": {},
-      "name": null,
+                    "room_name": {
-      "canonical_alias": null,
+                        "name": null,
-      "aliases": [],
+                        "canonical_alias": null,
-      "heroes": [],
+                        "aliases": [],
-      "joined_member_count": null,
+                        "heroes": [],
-      "invited_member_count": null
+                        "joined_member_count": null,
-    },
+                        "invited_member_count": null
-    "own_user_id": "@example:example.com",
+                    },
-    "creator": null,
+                    "own_user_id": "@example:example.com",
-    "members": {},
+                    "creator": null,
-    "messages": [],
+                    "joined_members": {},
-    "typing_users": [],
+                    "invited_members": {},
-    "power_levels": null,
+                    "messages": [],
-    "encrypted": null,
+                    "typing_users": [],
-    "unread_highlight": null,
+                    "power_levels": null,
-    "unread_notifications": null,
+                    "encrypted": null,
-    "tombstone": null
+                    "unread_highlight": null,
-  }
+                    "unread_notifications": null,
-}"#,
+                    "tombstone": null
-            serde_json::to_string_pretty(&joined_rooms).unwrap()
+                }
+            }),
+            serde_json::to_value(&joined_rooms).unwrap()
         );
     }
 

matrix_sdk_test/src/lib.rs

@@ -64,15 +64,22 @@ pub struct EventBuilder {
     ephemeral: Vec<Event>,
     /// The account data events that determine the state of a `Room`.
     account_data: Vec<Event>,
+    /// Internal counter to enable the `prev_batch` and `next_batch` of each sync response to vary.
+    batch_counter: i64,
 }
 
 impl EventBuilder {
+    pub fn new() -> Self {
+        let builder: EventBuilder = Default::default();
+        builder
+    }
+
     /// Add an event to the room events `Vec`.
     pub fn add_ephemeral<Ev: TryFromRaw>(
-        mut self,
+        &mut self,
         json: EventsJson,
         variant: fn(Ev) -> Event,
-    ) -> Self {
+    ) -> &mut Self {
         let val: &JsonValue = match json {
             EventsJson::Typing => &test_json::TYPING,
             _ => panic!("unknown ephemeral event {:?}", json),
@@ -89,10 +96,10 @@ impl EventBuilder {
     /// Add an event to the room events `Vec`.
     #[allow(clippy::match_single_binding, unused)]
     pub fn add_account<Ev: TryFromRaw>(
-        mut self,
+        &mut self,
         json: EventsJson,
         variant: fn(Ev) -> Event,
-    ) -> Self {
+    ) -> &mut Self {
         let val: &JsonValue = match json {
             _ => panic!("unknown account event {:?}", json),
         };
@@ -107,10 +114,10 @@ impl EventBuilder {
 
     /// Add an event to the room events `Vec`.
     pub fn add_room_event<Ev: TryFromRaw>(
-        mut self,
+        &mut self,
         json: EventsJson,
         variant: fn(Ev) -> RoomEvent,
-    ) -> Self {
+    ) -> &mut Self {
         let val: &JsonValue = match json {
             EventsJson::Member => &test_json::MEMBER,
             EventsJson::PowerLevels => &test_json::POWER_LEVELS,
@@ -129,11 +136,11 @@ impl EventBuilder {
     }
 
     pub fn add_custom_joined_event<Ev: TryFromRaw>(
-        mut self,
+        &mut self,
         room_id: &RoomId,
         event: serde_json::Value,
         variant: fn(Ev) -> RoomEvent,
-    ) -> Self {
+    ) -> &mut Self {
         let event = serde_json::from_value::<EventJson<Ev>>(event)
             .unwrap()
             .deserialize()
@@ -150,11 +157,11 @@ impl EventBuilder {
     }
 
     pub fn add_custom_invited_event<Ev: TryFromRaw>(
-        mut self,
+        &mut self,
         room_id: &RoomId,
         event: serde_json::Value,
         variant: fn(Ev) -> AnyStrippedStateEvent,
-    ) -> Self {
+    ) -> &mut Self {
         let event = serde_json::from_value::<EventJson<Ev>>(event)
             .unwrap()
             .deserialize()
@@ -167,11 +174,11 @@ impl EventBuilder {
     }
 
     pub fn add_custom_left_event<Ev: TryFromRaw>(
-        mut self,
+        &mut self,
         room_id: &RoomId,
         event: serde_json::Value,
         variant: fn(Ev) -> RoomEvent,
-    ) -> Self {
+    ) -> &mut Self {
         let event = serde_json::from_value::<EventJson<Ev>>(event)
             .unwrap()
             .deserialize()
@@ -185,10 +192,10 @@ impl EventBuilder {
 
     /// Add a state event to the state events `Vec`.
     pub fn add_state_event<Ev: TryFromRaw>(
-        mut self,
+        &mut self,
         json: EventsJson,
         variant: fn(Ev) -> StateEvent,
-    ) -> Self {
+    ) -> &mut Self {
         let val: &JsonValue = match json {
             EventsJson::Alias => &test_json::ALIAS,
             EventsJson::Aliases => &test_json::ALIASES,
@@ -205,7 +212,7 @@ impl EventBuilder {
     }
 
     /// Add an presence event to the presence events `Vec`.
-    pub fn add_presence_event(mut self, json: EventsJson) -> Self {
+    pub fn add_presence_event(&mut self, json: EventsJson) -> &mut Self {
         let val: &JsonValue = match json {
             EventsJson::Presence => &test_json::PRESENCE,
             _ => panic!("unknown presence event {:?}", json),
@@ -219,10 +226,15 @@ impl EventBuilder {
         self
     }
 
-    /// Consumes `ResponseBuilder and returns SyncResponse.
+    /// Consumes `ResponseBuilder` and returns `SyncResponse`.
-    pub fn build_sync_response(mut self) -> SyncResponse {
+    pub fn build_sync_response(&mut self) -> SyncResponse {
         let main_room_id = RoomId::try_from("!SVkFJHzfwvuaIEawgC:localhost").unwrap();
 
+        // First time building a sync response, so initialize the `prev_batch` to a default one.
+        let prev_batch = self.generate_sync_token();
+        self.batch_counter += 1;
+        let next_batch = self.generate_sync_token();
+
         // TODO generalize this.
         let joined_room = serde_json::json!({
             "summary": {},
@@ -238,7 +250,7 @@ impl EventBuilder {
             "timeline": {
                 "events": self.joined_room_events.remove(&main_room_id).unwrap_or_default(),
                 "limited": true,
-                "prev_batch": "t392-516_47314_0_7_1_1_1_11444_1"
+                "prev_batch": prev_batch
             },
             "unread_notifications": {
                 "highlight_count": 0,
@@ -265,7 +277,7 @@ impl EventBuilder {
                 "timeline": {
                     "events": events,
                     "limited": true,
-                    "prev_batch": "t392-516_47314_0_7_1_1_1_11444_1"
+                    "prev_batch": prev_batch
                 },
                 "unread_notifications": {
                     "highlight_count": 0,
@@ -285,7 +297,7 @@ impl EventBuilder {
                 "timeline": {
                     "events": events,
                     "limited": false,
-                    "prev_batch": "t392-516_47314_0_7_1_1_1_11444_1"
+                    "prev_batch": prev_batch
                 },
             });
             left_rooms.insert(room_id, room);
@@ -305,7 +317,7 @@ impl EventBuilder {
         let body = serde_json::json! {
             {
                 "device_one_time_keys_count": {},
-                "next_batch": "s526_47314_0_7_1_1_1_11444_1",
+                "next_batch": next_batch,
                 "device_lists": {
                     "changed": [],
                     "left": []
@@ -328,6 +340,10 @@ impl EventBuilder {
             .unwrap();
         SyncResponse::try_from(response).unwrap()
     }
+
+    fn generate_sync_token(&self) -> String {
+        format!("t392-516_47314_0_7_1_1_1_11444_{}", self.batch_counter)
+    }
 }
 
 /// Embedded sync reponse files