Nix RoomMember::update_member and tracking membership.

After discussing with poljar, we concluded we don't actually need to
tracking membership state, since we won't be tracking users that
left (banned, kicked, disinvited).

The only thing we need to keep track of is the difference between joined
and invited users which will be dealt with in a separate commit.
master
Denis Kasak 2020-06-10 16:36:51 +02:00
parent a0eaa9c364
commit 7751605e37
3 changed files with 17 additions and 34 deletions

View File

@ -1918,10 +1918,6 @@ mod test {
.await; .await;
assert_eq!(2, room.members.len()); assert_eq!(2, room.members.len());
for member in room.members.values() {
assert_eq!(MembershipState::Join, member.membership);
}
assert!(room.power_levels.is_some()) assert!(room.power_levels.is_some())
} }

View File

@ -360,9 +360,7 @@ impl Room {
existing_member existing_member
.display_name .display_name
.as_ref() .as_ref()
.and_then(|existing_member_name| { .map(|existing_member_name| new_member_name == existing_member_name)
Some(new_member_name == existing_member_name)
})
}) })
.unwrap_or(false) .unwrap_or(false)
}) })
@ -475,23 +473,28 @@ impl Room {
/// ///
/// Returns true if the joined member list changed, false otherwise. /// Returns true if the joined member list changed, false otherwise.
pub fn handle_membership(&mut self, event: &MemberEvent) -> bool { pub fn handle_membership(&mut self, event: &MemberEvent) -> bool {
use MembershipChange::*;
// TODO: This would not be handled correctly as all the MemberEvents have the `prev_content` // TODO: This would not be handled correctly as all the MemberEvents have the `prev_content`
// inside of `unsigned` field. // inside of `unsigned` field.
match event.membership_change() { match event.membership_change() {
MembershipChange::Invited | MembershipChange::Joined => self.add_member(event), Invited | Joined => self.add_member(event),
_ => { ProfileChanged => {
let user = 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
} else { } else {
return false; return false;
}; };
if let Some(member) = self.members.get_mut(&user) { if let Some(member) = self.members.get_mut(&user_id) {
member.update_member(event) member.update_profile(event)
} else { } else {
false false
} }
} }
// Not interested in other events.
_ => false,
} }
} }
@ -719,10 +722,6 @@ mod test {
.await; .await;
assert_eq!(2, room.members.len()); assert_eq!(2, room.members.len());
for member in room.members.values() {
assert_eq!(MembershipState::Join, member.membership);
}
assert!(room.deref().power_levels.is_some()) assert!(room.deref().power_levels.is_some())
} }

View File

@ -18,7 +18,7 @@ use std::convert::TryFrom;
use crate::events::collections::all::Event; use crate::events::collections::all::Event;
use crate::events::presence::{PresenceEvent, PresenceEventContent, PresenceState}; use crate::events::presence::{PresenceEvent, PresenceEventContent, PresenceState};
use crate::events::room::{ use crate::events::room::{
member::{MemberEvent, MembershipChange, MembershipState}, member::{MemberEvent, MembershipChange},
power_levels::PowerLevelsEvent, power_levels::PowerLevelsEvent,
}; };
use crate::identifiers::UserId; use crate::identifiers::UserId;
@ -54,8 +54,6 @@ pub struct RoomMember {
pub power_level: Option<Int>, pub power_level: Option<Int>,
/// The normalized power level of this `RoomMember` (0-100). /// The normalized power level of this `RoomMember` (0-100).
pub power_level_norm: Option<Int>, pub power_level_norm: Option<Int>,
/// The `MembershipState` of this `RoomMember`.
pub membership: MembershipState,
/// The human readable name of this room member. /// The human readable name of this room member.
pub name: String, pub name: String,
/// The events that created the state of this room member. /// The events that created the state of this room member.
@ -75,7 +73,6 @@ impl PartialEq for RoomMember {
&& self.display_name == other.display_name && self.display_name == other.display_name
&& self.avatar_url == other.avatar_url && self.avatar_url == other.avatar_url
&& self.last_active_ago == other.last_active_ago && self.last_active_ago == other.last_active_ago
&& self.membership == other.membership
} }
} }
@ -94,7 +91,6 @@ impl RoomMember {
typing: None, typing: None,
power_level: None, power_level: None,
power_level_norm: None, power_level_norm: None,
membership: event.content.membership,
presence_events: Vec::default(), presence_events: Vec::default(),
events: vec![Event::RoomMember(event.clone())], events: vec![Event::RoomMember(event.clone())],
} }
@ -120,7 +116,8 @@ impl RoomMember {
.unwrap_or_else(|| format!("{}", self.user_id)) .unwrap_or_else(|| format!("{}", self.user_id))
} }
pub fn update_member(&mut self, event: &MemberEvent) -> bool { /// Handle profile updates.
pub(crate) fn update_profile(&mut self, event: &MemberEvent) -> bool {
use MembershipChange::*; use MembershipChange::*;
match event.membership_change() { match event.membership_change() {
@ -129,15 +126,9 @@ impl RoomMember {
self.avatar_url = event.content.avatar_url.clone(); self.avatar_url = event.content.avatar_url.clone();
true 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,
} }
} }
@ -222,7 +213,6 @@ mod test {
use matrix_sdk_test::{async_test, EventBuilder, EventsFile}; use matrix_sdk_test::{async_test, EventBuilder, EventsFile};
use crate::events::collections::all::RoomEvent; use crate::events::collections::all::RoomEvent;
use crate::events::room::member::MembershipState;
use crate::identifiers::{RoomId, UserId}; use crate::identifiers::{RoomId, UserId};
use crate::{BaseClient, Session}; use crate::{BaseClient, Session};
@ -268,7 +258,6 @@ mod test {
.members .members
.get(&UserId::try_from("@example:localhost").unwrap()) .get(&UserId::try_from("@example:localhost").unwrap())
.unwrap(); .unwrap();
assert_eq!(member.membership, MembershipState::Join);
assert_eq!(member.power_level, Int::new(100)); assert_eq!(member.power_level, Int::new(100));
} }
@ -294,7 +283,6 @@ mod test {
.get(&UserId::try_from("@example:localhost").unwrap()) .get(&UserId::try_from("@example:localhost").unwrap())
.unwrap(); .unwrap();
assert_eq!(member.membership, MembershipState::Join);
assert_eq!(member.power_level, Int::new(100)); assert_eq!(member.power_level, Int::new(100));
assert!(member.avatar_url.is_none()); assert!(member.avatar_url.is_none());