From 7751605e3726a68a1901edba8e7d980955b17725 Mon Sep 17 00:00:00 2001 From: Denis Kasak Date: Wed, 10 Jun 2020 16:36:51 +0200 Subject: [PATCH] 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. --- matrix_sdk/src/client.rs | 4 ---- matrix_sdk_base/src/models/room.rs | 23 +++++++++++----------- matrix_sdk_base/src/models/room_member.rs | 24 ++++++----------------- 3 files changed, 17 insertions(+), 34 deletions(-) diff --git a/matrix_sdk/src/client.rs b/matrix_sdk/src/client.rs index c88e097f..00fbf580 100644 --- a/matrix_sdk/src/client.rs +++ b/matrix_sdk/src/client.rs @@ -1918,10 +1918,6 @@ mod test { .await; assert_eq!(2, room.members.len()); - for member in room.members.values() { - assert_eq!(MembershipState::Join, member.membership); - } - assert!(room.power_levels.is_some()) } diff --git a/matrix_sdk_base/src/models/room.rs b/matrix_sdk_base/src/models/room.rs index cfefc66a..5730087b 100644 --- a/matrix_sdk_base/src/models/room.rs +++ b/matrix_sdk_base/src/models/room.rs @@ -360,9 +360,7 @@ impl Room { existing_member .display_name .as_ref() - .and_then(|existing_member_name| { - Some(new_member_name == existing_member_name) - }) + .map(|existing_member_name| new_member_name == existing_member_name) }) .unwrap_or(false) }) @@ -475,23 +473,28 @@ impl Room { /// /// Returns true if the joined member list changed, false otherwise. 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` // inside of `unsigned` field. match event.membership_change() { - MembershipChange::Invited | MembershipChange::Joined => self.add_member(event), - _ => { - let user = if let Ok(id) = UserId::try_from(event.state_key.as_str()) { + Invited | Joined => self.add_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.members.get_mut(&user_id) { + member.update_profile(event) } else { false } } + + // Not interested in other events. + _ => false, } } @@ -719,10 +722,6 @@ mod test { .await; 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()) } diff --git a/matrix_sdk_base/src/models/room_member.rs b/matrix_sdk_base/src/models/room_member.rs index f3bc7245..29d43aa9 100644 --- a/matrix_sdk_base/src/models/room_member.rs +++ b/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; @@ -54,8 +54,6 @@ 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, /// 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.avatar_url == other.avatar_url && self.last_active_ago == other.last_active_ago - && self.membership == other.membership } } @@ -94,7 +91,6 @@ 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())], } @@ -120,7 +116,8 @@ impl RoomMember { .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::*; match event.membership_change() { @@ -129,15 +126,9 @@ impl RoomMember { self.avatar_url = event.content.avatar_url.clone(); true } - Banned | Kicked | KickedAndBanned | InvitationRejected | InvitationRevoked | Left - | Unbanned | Joined | Invited => { - self.membership = event.content.membership; - true - } - NotImplemented => false, - None => false, - // we ignore the error here as only a buggy or malicious server would send this - Error => false, + + // We're only interested in profile changes here. + _ => false, } } @@ -222,7 +213,6 @@ mod test { use matrix_sdk_test::{async_test, EventBuilder, EventsFile}; use crate::events::collections::all::RoomEvent; - use crate::events::room::member::MembershipState; use crate::identifiers::{RoomId, UserId}; use crate::{BaseClient, Session}; @@ -268,7 +258,6 @@ mod test { .members .get(&UserId::try_from("@example:localhost").unwrap()) .unwrap(); - assert_eq!(member.membership, MembershipState::Join); assert_eq!(member.power_level, Int::new(100)); } @@ -294,7 +283,6 @@ mod test { .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());