Split joined/invited users and handle removing users.

master
Denis Kasak 2020-06-10 18:12:27 +02:00
parent 7751605e37
commit 331cb02266
7 changed files with 159 additions and 85 deletions

View File

@ -23,12 +23,8 @@ impl EventEmitter for EventCallback {
// any reads should be held for the shortest time possible to // any reads should be held for the shortest time possible to
// avoid dead locks // avoid dead locks
let room = room.read().await; 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); println!("{}: {}", name, msg_body);
} }

View File

@ -833,8 +833,11 @@ impl Client {
let missing_sessions = { let missing_sessions = {
let room = self.base_client.get_joined_room(room_id).await; let room = self.base_client.get_joined_room(room_id).await;
let room = room.as_ref().unwrap().read().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() { if !missing_sessions.is_empty() {
@ -1276,7 +1279,6 @@ mod test {
}; };
use super::{Client, ClientConfig, Session, SyncSettings, Url}; use super::{Client, ClientConfig, Session, SyncSettings, Url};
use crate::events::collections::all::RoomEvent; use crate::events::collections::all::RoomEvent;
use crate::events::room::member::MembershipState;
use crate::events::room::message::TextMessageEventContent; use crate::events::room::message::TextMessageEventContent;
use crate::identifiers::{EventId, RoomId, RoomIdOrAliasId, UserId}; use crate::identifiers::{EventId, RoomId, RoomIdOrAliasId, UserId};
@ -1917,7 +1919,7 @@ mod test {
.read() .read()
.await; .await;
assert_eq!(2, room.members.len()); assert_eq!(2, room.joined_members.len());
assert!(room.power_levels.is_some()) assert!(room.power_levels.is_some())
} }

View File

@ -963,7 +963,8 @@ impl BaseClient {
// If the room is encrypted, update the tracked users. // If the room is encrypted, update the tracked users.
if room.is_encrypted() { 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;
} }
} }
} }
@ -1241,7 +1242,10 @@ impl BaseClient {
match &mut *olm { match &mut *olm {
Some(o) => { Some(o) => {
let room = room.write().await; let room = room.write().await;
let members = room.members.keys(); let members = room
.joined_members
.keys()
.chain(room.invited_members.keys());
Ok(o.share_group_session(room_id, members).await?) Ok(o.share_group_session(room_id, members).await?)
} }
None => panic!("Olm machine wasn't started"), None => panic!("Olm machine wasn't started"),

View File

@ -187,7 +187,8 @@ mod test {
}, },
"own_user_id": "@example:example.com", "own_user_id": "@example:example.com",
"creator": null, "creator": null,
"members": {}, "joined_members": {},
"invited_members": {},
"messages": [ message ], "messages": [ message ],
"typing_users": [], "typing_users": [],
"power_levels": null, "power_levels": null,
@ -237,7 +238,8 @@ mod test {
}, },
"own_user_id": "@example:example.com", "own_user_id": "@example:example.com",
"creator": null, "creator": null,
"members": {}, "joined_members": {},
"invited_members": {},
"messages": [ message ], "messages": [ message ],
"typing_users": [], "typing_users": [],
"power_levels": null, "power_levels": null,

View File

@ -159,8 +159,11 @@ pub struct Room {
pub own_user_id: UserId, pub own_user_id: UserId,
/// The mxid of the room creator. /// The mxid of the room creator.
pub creator: Option<UserId>, 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. /// 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 /// This is helpful when using a `StateStore` to avoid multiple requests
@ -208,7 +211,11 @@ impl RoomName {
/// ///
/// [spec]: /// [spec]:
/// <https://matrix.org/docs/spec/client_server/latest#calculating-the-display-name-for-a-room> /// <https://matrix.org/docs/spec/client_server/latest#calculating-the-display-name-for-a-room>
pub fn calculate_name(&self, members: &HashMap<UserId, RoomMember>) -> String { pub fn calculate_name(
&self,
invited_members: &HashMap<UserId, RoomMember>,
joined_members: &HashMap<UserId, RoomMember>,
) -> String {
if let Some(name) = &self.name { if let Some(name) = &self.name {
let name = name.trim(); let name = name.trim();
name.to_string() name.to_string()
@ -229,10 +236,11 @@ impl RoomName {
invited + joined - one invited + joined - one
}; };
let members = joined_members.values().chain(invited_members.values());
// TODO: This should use `self.heroes` but it is always empty?? // TODO: This should use `self.heroes` but it is always empty??
if heroes >= invited_joined { if heroes >= invited_joined {
let mut names = members let mut names = members
.values()
.take(3) .take(3)
.map(|mem| { .map(|mem| {
mem.display_name mem.display_name
@ -245,7 +253,6 @@ impl RoomName {
names.join(", ") names.join(", ")
} else if heroes < invited_joined && invited + joined > one { } else if heroes < invited_joined && invited + joined > one {
let mut names = members let mut names = members
.values()
.take(3) .take(3)
.map(|mem| { .map(|mem| {
mem.display_name mem.display_name
@ -258,7 +265,7 @@ impl RoomName {
// 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)) format!("{}, and {} others", names.join(", "), (joined + invited))
} else { } else {
format!("Empty room (was {} others)", members.len()) "Empty room".to_string()
} }
} }
} }
@ -278,7 +285,8 @@ impl Room {
room_name: RoomName::default(), room_name: RoomName::default(),
own_user_id: own_user_id.clone(), own_user_id: own_user_id.clone(),
creator: None, creator: None,
members: HashMap::new(), invited_members: HashMap::new(),
joined_members: HashMap::new(),
#[cfg(feature = "messages")] #[cfg(feature = "messages")]
messages: MessageQueue::new(), messages: MessageQueue::new(),
typing_users: Vec::new(), typing_users: Vec::new(),
@ -293,7 +301,8 @@ impl Room {
/// Return the display name of the room. /// Return the display name of the room.
pub fn display_name(&self) -> String { pub fn display_name(&self) -> String {
self.room_name.calculate_name(&self.members) self.room_name
.calculate_name(&self.invited_members, &self.joined_members)
} }
/// Is the room a encrypted room. /// Is the room a encrypted room.
@ -326,34 +335,92 @@ impl Room {
if let Some(name) = disambiguated_name { if let Some(name) = disambiguated_name {
// The display name of the member is non-unique so we return a disambiguated version. // The display name of the member is non-unique so we return a disambiguated version.
name name
} else if let Some(member) = self.members.get(id) { } 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. // The display name of the member is unique so we can return it directly if it is set.
// If not, we return his MXID. // If not, we return his MXID.
member member.name().into()
.display_name
.as_ref()
.map(|s| s.to_string())
.unwrap_or_else(|| format!("{}", member.user_id))
.into()
} else { } else {
// There is no member with the requested MXID in the room. We still return the MXID. // There is no member with the requested MXID in the room. We still return the MXID.
id.as_ref().into() 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 { fn add_member(&mut self, event: &MemberEvent) -> bool {
let new_member = RoomMember::new(event); let new_member = RoomMember::new(event);
if self.members.contains_key(&new_member.user_id) { if self.joined_members.contains_key(&new_member.user_id)
|| self.invited_members.contains_key(&new_member.user_id)
{
return false; return false;
} }
// Find all users that share the same display name as the joining user. match event.membership_change() {
let users_with_same_name: Vec<UserId> = self MembershipChange::Joined => self
.members .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.
if let Some(disambiguations) = self.disambiguate_member(&new_member, true) {
for (id, name) in disambiguations.into_iter() {
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);
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, generate a map of member display name disambiguations required in
/// order to make everyone's display name unique.
///
/// The `inclusive` parameter controls whether the member which we are disambiguating should
/// be considered a member of the room or not.
///
/// Returns either the map of disambiguations or None if no disambiguations are necessary.
fn disambiguate_member(
&self,
member: &RoomMember,
inclusive: bool,
) -> Option<HashMap<UserId, String>> {
let members = self
.invited_members
.iter() .iter()
.chain(self.joined_members.iter());
// Find all other users that share the same display name as the joining user.
let users_with_same_name: Vec<UserId> = members
.filter(|(_, existing_member)| { .filter(|(_, existing_member)| {
new_member member
.display_name .display_name
.as_ref() .as_ref()
.and_then(|new_member_name| { .and_then(|new_member_name| {
@ -364,47 +431,49 @@ impl Room {
}) })
.unwrap_or(false) .unwrap_or(false)
}) })
.map(|(k, _)| k) // 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() .cloned()
.collect(); .collect();
// If there is another user with the same display name, use "DISPLAY_NAME (MXID)" instead // There is at least one other user with the same display name.
// to disambiguate.
if !users_with_same_name.is_empty() { if !users_with_same_name.is_empty() {
let users_with_same_name = users_with_same_name if users_with_same_name.len() == 1 {
.into_iter() // The ambiguous set is now of size 1, so we can revert to simply using the display
.filter_map(|id| { // name for this user.
self.members Some(
.get(&id) users_with_same_name
.map(|m| { .into_iter()
m.display_name .filter_map(|id| {
.as_ref() self.joined_members
.map(|d| format!("{} ({})", d, m.user_id)) .get(&id)
.unwrap_or_else(|| format!("{}", m.user_id)) .or_else(|| self.invited_members.get(&id))
.map(|m| m.name())
.map(|m| (id, m))
}) })
.map(|m| (id, m)) .collect::<HashMap<UserId, String>>(),
}) )
.collect::<Vec<(UserId, String)>>(); } else {
// Disambiguate everyone in the list by using "DISPLAY_NAME (MXID)" as their
// Update all existing users with same name. // display name.
for (id, member) in users_with_same_name { Some(
self.disambiguated_display_names.insert(id, member); users_with_same_name
.into_iter()
.filter_map(|id| {
self.joined_members
.get(&id)
.or_else(|| self.invited_members.get(&id))
.map(|m| m.unique_name())
.map(|m| (id, m))
})
.collect::<HashMap<UserId, String>>(),
)
} }
} else {
// Insert new member's display name. // Nothing to disambiguate.
self.disambiguated_display_names.insert( None
new_member.user_id.clone(),
new_member
.display_name
.as_ref()
.map(|n| format!("{} ({})", n, new_member.user_id))
.unwrap_or_else(|| format!("{}", new_member.user_id)),
);
} }
self.members.insert(new_member.user_id.clone(), new_member);
true
} }
/// Add to the list of `RoomAliasId`s. /// Add to the list of `RoomAliasId`s.
@ -479,6 +548,9 @@ impl Room {
// inside of `unsigned` field. // inside of `unsigned` field.
match event.membership_change() { match event.membership_change() {
Invited | Joined => self.add_member(event), Invited | Joined => self.add_member(event),
Kicked | Banned | KickedAndBanned | InvitationRejected | Left => {
self.remove_member(event)
}
ProfileChanged => { ProfileChanged => {
let user_id = if let Ok(id) = UserId::try_from(event.state_key.as_str()) { let user_id = if let Ok(id) = UserId::try_from(event.state_key.as_str()) {
id id
@ -486,7 +558,7 @@ impl Room {
return false; return false;
}; };
if let Some(member) = self.members.get_mut(&user_id) { if let Some(member) = self.joined_members.get_mut(&user_id) {
member.update_profile(event) member.update_profile(event)
} else { } else {
false false
@ -561,7 +633,7 @@ impl Room {
} }
for user in event.content.users.keys() { 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) { if member.update_power(event, max_power) {
updated = true; updated = true;
} }
@ -656,7 +728,7 @@ impl Room {
/// ///
/// * `event` - The presence event for a specified room member. /// * `event` - The presence event for a specified room member.
pub fn receive_presence_event(&mut self, event: &PresenceEvent) -> bool { 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) { if member.did_update_presence(event) {
false false
} else { } else {
@ -674,10 +746,7 @@ impl Room {
#[cfg(test)] #[cfg(test)]
mod test { mod test {
use super::*; 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::identifiers::{EventId, UserId};
use crate::{BaseClient, Session}; use crate::{BaseClient, Session};
use matrix_sdk_test::{async_test, sync_response, EventBuilder, EventsFile, SyncResponseFile}; use matrix_sdk_test::{async_test, sync_response, EventBuilder, EventsFile, SyncResponseFile};
@ -721,7 +790,7 @@ mod test {
.read() .read()
.await; .await;
assert_eq!(2, room.members.len()); assert_eq!(2, room.joined_members.len());
assert!(room.deref().power_levels.is_some()) assert!(room.deref().power_levels.is_some())
} }
@ -764,7 +833,7 @@ mod test {
.build_sync_response(); .build_sync_response();
let mut member2_join_sync_response = event_builder let mut member2_join_sync_response = event_builder
.add_custom_joined_event(&room_id, member2_join_event, |ev| RoomEvent::RoomMember(ev)) .add_custom_joined_event(&room_id, member2_join_event, RoomEvent::RoomMember)
.build_sync_response(); .build_sync_response();
// First member with display name "example" joins // First member with display name "example" joins
@ -816,13 +885,13 @@ mod test {
let room = client.get_joined_room(&room_id).await.unwrap(); let room = client.get_joined_room(&room_id).await.unwrap();
let room = room.read().await; 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!(room.power_levels.is_some());
assert_eq!( assert_eq!(
room.power_levels.as_ref().unwrap().kick, room.power_levels.as_ref().unwrap().kick,
crate::js_int::Int::new(50).unwrap() 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!( assert_eq!(
admin.power_level.unwrap(), admin.power_level.unwrap(),
crate::js_int::Int::new(100).unwrap() crate::js_int::Int::new(100).unwrap()

View File

@ -27,8 +27,7 @@ use crate::js_int::{Int, UInt};
use serde::{Deserialize, Serialize}; 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. // 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)] #[derive(Clone, Debug, Serialize, Deserialize)]
#[cfg_attr(test, derive(Clone))]
/// A Matrix room member. /// A Matrix room member.
/// ///
pub struct RoomMember { pub struct RoomMember {
@ -255,7 +254,7 @@ mod test {
let room = room.read().await; let room = room.read().await;
let member = room let member = room
.members .joined_members
.get(&UserId::try_from("@example:localhost").unwrap()) .get(&UserId::try_from("@example:localhost").unwrap())
.unwrap(); .unwrap();
assert_eq!(member.power_level, Int::new(100)); assert_eq!(member.power_level, Int::new(100));
@ -279,7 +278,7 @@ mod test {
let room = room.read().await; let room = room.read().await;
let member = room let member = room
.members .joined_members
.get(&UserId::try_from("@example:localhost").unwrap()) .get(&UserId::try_from("@example:localhost").unwrap())
.unwrap(); .unwrap();

View File

@ -153,7 +153,8 @@ mod test {
}, },
"own_user_id": "@example:example.com", "own_user_id": "@example:example.com",
"creator": null, "creator": null,
"members": {}, "joined_members": {},
"invited_members": {},
"disambiguated_display_names": {}, "disambiguated_display_names": {},
"typing_users": [], "typing_users": [],
"power_levels": null, "power_levels": null,
@ -182,7 +183,8 @@ mod test {
}, },
"own_user_id": "@example:example.com", "own_user_id": "@example:example.com",
"creator": null, "creator": null,
"members": {}, "joined_members": {},
"invited_members": {},
"messages": [], "messages": [],
"typing_users": [], "typing_users": [],
"power_levels": null, "power_levels": null,