Room::change_nickname: avoid duplicating presence broadcast

Signed-off-by: Maxime “pep” Buquet <pep@bouah.net>

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(),
+			status: {
-			items: vec![MucItem::new(
+				#[allow(clippy::redundant_pattern_matching)]
-				Affiliation::Owner,
+				if let Some(_) = with_nick {
-				if leave { Role::None } else { Role::Moderator },
+					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)
+		self.broadcast_presence(
-			.with_from(Jid::Full(joined_session.participant().clone()));
+			component,
-
+			self.get_occupant(&joined_session)?,
-		let presence_leave_to_others = presence_leave.clone().with_payloads(vec![MucUser {
+			joined_session.presence.clone(),
-			status: vec![MucStatus::NewNick],
+			BroadcastPresence::Leave,
-			items: vec![MucItem::new(Affiliation::Owner, Role::None).with_nick(newnick.clone())],
+			Some(&**newnick),
-		}
+		)
-		.into()]);
+		.await?;
-
-		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?;
-					}
-				}
-			}
-		}
 
 		// 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();