From 41da68bd5e303eb0663b10a31964de46a88f68c1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maxime=20=E2=80=9Cpep=E2=80=9D=20Buquet?= Date: Sat, 7 Jan 2023 16:42:13 +0100 Subject: [PATCH] Room::change_nickname: avoid duplicating presence broadcast MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Maxime “pep” Buquet --- src/room.rs | 171 +++++++++++++++++++++++++++------------------------- 1 file changed, 88 insertions(+), 83 deletions(-) diff --git a/src/room.rs b/src/room.rs index 0625dfe..e94ac51 100644 --- a/src/room.rs +++ b/src/room.rs @@ -32,7 +32,7 @@ use xmpp_parsers::{ MucUser, }, presence::{Presence, Type as PresenceType}, - BareJid, Element, Jid, + BareJid, Jid, }; #[derive(Debug, PartialEq, Eq)] @@ -72,6 +72,7 @@ impl Room { own_occupant: &Occupant, presence: PresenceFull, mode: BroadcastPresence, + with_nick: Option<&str>, ) -> Result<(), Error> { let leave = matches!(mode, BroadcastPresence::Leave); let sync = matches!(mode, BroadcastPresence::Join | BroadcastPresence::Resync); @@ -101,11 +102,28 @@ impl Room { }) .with_from(Jid::Full(own_occupant.participant.clone())) .with_payloads(vec![MucUser { - status: Vec::new(), - items: vec![MucItem::new( - Affiliation::Owner, - if leave { Role::None } else { Role::Moderator }, - )], + status: { + #[allow(clippy::redundant_pattern_matching)] + if let Some(_) = with_nick { + vec![MucStatus::NewNick] + } else { + vec![] + } + }, + items: vec![{ + if let Some(newnick) = with_nick { + MucItem::new( + Affiliation::Owner, + if leave { Role::None } else { Role::Moderator }, + ) + .with_nick(newnick) + } else { + MucItem::new( + Affiliation::Owner, + if leave { Role::None } else { Role::Moderator }, + ) + } + }], } .into()]); @@ -140,7 +158,41 @@ impl Room { let presence = presence_to_old .clone() .with_to(Jid::Full(session.real().clone())); - component.send_stanza(presence).await?; + + if other.real == own_occupant.real { + let presence = presence_to_old + .clone() + .with_to(Jid::Full(session.real().clone())) + .with_payloads(vec![MucUser { + status: { + #[allow(clippy::redundant_pattern_matching)] + if let Some(_) = with_nick { + vec![MucStatus::NewNick] + } else { + vec![] + } + }, + items: vec![{ + if let Some(newnick) = with_nick { + MucItem::new( + Affiliation::Owner, + if leave { Role::None } else { Role::Moderator }, + ) + .with_nick(newnick) + .with_jid(own_session.real().clone()) + } else { + MucItem::new( + Affiliation::Owner, + if leave { Role::None } else { Role::Moderator }, + ) + } + }], + } + .into()]); + component.send_stanza(presence).await?; + } else { + component.send_stanza(presence).await?; + } } } } @@ -156,7 +208,11 @@ impl Room { Role::Moderator }, jid: Some(session.real().clone()), - nick: None, + nick: if leave { + with_nick.map(String::from) + } else { + None + }, actor: None, continue_: None, reason: None, @@ -199,7 +255,12 @@ impl Room { .with_to(own_session.real().clone()) .with_payloads(vec![MucUser { status: if leave { - vec![MucStatus::SelfPresence] + #[allow(clippy::redundant_pattern_matching)] + if let Some(_) = with_nick { + vec![MucStatus::SelfPresence, MucStatus::NewNick] + } else { + vec![MucStatus::SelfPresence] + } } else { vec![MucStatus::SelfPresence, MucStatus::AssignedNick] }, @@ -289,6 +350,7 @@ impl Room { occupant, new_session.presence, BroadcastPresence::Resync, + None, ) .await?; } @@ -300,6 +362,7 @@ impl Room { occupant, new_session.presence.clone(), BroadcastPresence::Join, + None, ) .await?; self.send_subject(component, new_session, occupant.clone()) @@ -330,6 +393,7 @@ impl Room { occupant, session.presence, BroadcastPresence::Update, + None, ) .await } @@ -361,78 +425,14 @@ impl Room { // Send unavailable for current session to everyone - let presence_leave = Presence::new(PresenceType::Unavailable) - .with_from(Jid::Full(joined_session.participant().clone())); - - let presence_leave_to_others = presence_leave.clone().with_payloads(vec![MucUser { - status: vec![MucStatus::NewNick], - items: vec![MucItem::new(Affiliation::Owner, Role::None).with_nick(newnick.clone())], - } - .into()]); - - for (nick, occupant) in self.occupants.iter() { - for session in occupant.iter() { - // Self occupant - if nick == oldnick { - let mucuser = MucUser { - status: vec![MucStatus::NewNick], - items: vec![MucItem::new(Affiliation::Owner, Role::None) - .with_nick(newnick.clone()) - .with_jid(session.real().clone())], - }; - - // Self session - if session.real() == joined_session.real() { - let mucuser = MucUser { - status: vec![MucStatus::SelfPresence, MucStatus::NewNick], - items: vec![MucItem::new(Affiliation::Owner, Role::None) - .with_nick(newnick.clone()) - .with_jid(joined_session.real().clone())], - }; - - let presence: Element = presence_leave - .clone() - .with_to(Jid::Full(session.real().clone())) - .with_payloads(vec![mucuser.into()]) - .into(); - component.send_stanza(presence).await?; - } else { - // Self occupant, other sessions - let presence: Element = presence_leave - .clone() - .with_to(Jid::Full(session.real().clone())) - .with_payloads(vec![mucuser.into()]) - .into(); - component.send_stanza(presence).await?; - } - } else { - // Other occupants' sessions - if occupant.real == bare { - // Same barejid - let mucuser = MucUser { - status: vec![MucStatus::NewNick], - items: vec![MucItem::new(Affiliation::Owner, Role::None) - .with_nick(newnick.clone()) - .with_jid(joined_session.real().clone())], - }; - - let presence = presence_leave - .clone() - .with_to(Jid::Full(session.real().clone())) - .with_payloads(vec![mucuser.into()]); - component.send_stanza(presence).await?; - } else { - component - .send_stanza( - presence_leave_to_others - .clone() - .with_to(Jid::Full(session.real().clone())), - ) - .await?; - } - } - } - } + self.broadcast_presence( + component, + self.get_occupant(&joined_session)?, + joined_session.presence.clone(), + BroadcastPresence::Leave, + Some(&**newnick), + ) + .await?; // Remove old session match self.get_mut_occupant(&joined_session) { @@ -454,7 +454,7 @@ impl Room { } else { // Nickname isn't present already in the room. Create new occupant. let occupant = Occupant::new(new_session.presence.clone())?; - self.occupants.insert(newnick.to_string(), occupant.clone()); + self.occupants.insert(newnick.to_string(), occupant); } // Send available for new session to everyone @@ -539,6 +539,7 @@ impl Room { &occupant, session.presence.clone(), BroadcastPresence::Leave, + None, ) .await?; @@ -561,7 +562,7 @@ impl Room { for (_nick, occupant) in self.occupants.iter() { #[allow(clippy::redundant_pattern_matching)] if let Ok(joined_session) = occupant.find_session(requested_session) { - return Ok(joined_session.clone()); + return Ok(joined_session); } } @@ -719,6 +720,7 @@ mod tests { &occupant3, presence3, BroadcastPresence::Resync, + None, ) .await .unwrap(); @@ -800,6 +802,7 @@ mod tests { &occupant3, presence3, BroadcastPresence::Update, + None, ) .await .unwrap(); @@ -920,6 +923,7 @@ mod tests { &occupant3, presence3, BroadcastPresence::Join, + None, ) .await .unwrap(); @@ -1013,6 +1017,7 @@ mod tests { &occupant3, presence_leave, BroadcastPresence::Leave, + None, ) .await .unwrap();