From 7fce1146e005eea428687923ce3b63cd9eab2b16 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jonas=20Sch=C3=A4fer?= Date: Sun, 3 Mar 2024 16:15:04 +0100 Subject: [PATCH] Offer {Resource,Node,Domain}Ref on Jid API This provides a non-copying API, which is generally favourable. The other accessors were removed, because the intent was to provide this "most sensible" API via the "default" (i.e. shortest, most concisely named) functions. --- jid/CHANGELOG.md | 13 ++ jid/src/inner.rs | 20 +-- jid/src/lib.rs | 162 ++++++++----------------- jid/src/parts.rs | 6 +- tokio-xmpp/src/client/bind.rs | 4 +- tokio-xmpp/src/client/connect.rs | 2 +- tokio-xmpp/src/starttls/mod.rs | 4 +- tokio-xmpp/src/stream_start.rs | 2 +- xmpp/src/message/receive/chat.rs | 2 +- xmpp/src/message/receive/group_chat.rs | 4 +- 10 files changed, 86 insertions(+), 133 deletions(-) diff --git a/jid/CHANGELOG.md b/jid/CHANGELOG.md index da9714b8..9306df2f 100644 --- a/jid/CHANGELOG.md +++ b/jid/CHANGELOG.md @@ -1,7 +1,20 @@ Version xxx, release xxx: + * Breaking: + - With the addition of `str`-like types for `DomainPart`, `NodePart` and + `ResourcePart`, the functions on `Jid`, `BareJid` and `FullJid` which + return the respective types have been changed to return references + instead. Use `ToOwned::to_owned` to obtain a copy or `.as_str()` to + obtain a plain `str` reference. + - The `node_str`, `domain_str` and `resource_str` functions returning str + references have been removed from the JID types. Use `.as_str()` or + `.map(|x| x.as_str())` on the corresponding `node`/`domain`/`resource` + functions instead. * Additions: - Add optional quote support. Implement quote::ToTokens for Jid, FullJid and BareJid. + - `str`-like reference types have been added for `DomainPart`, `NodePart` + and `ResourcePart`, called `DomainRef`, `NodeRef` and `ResourceRef` + respectively. Version 0.10.0, release 2023-08-17: * Breaking diff --git a/jid/src/inner.rs b/jid/src/inner.rs index ec7c5a6f..1ea83adc 100644 --- a/jid/src/inner.rs +++ b/jid/src/inner.rs @@ -17,6 +17,8 @@ use std::borrow::Cow; use std::str::FromStr; use stringprep::{nameprep, nodeprep, resourceprep}; +use crate::parts::{DomainRef, NodeRef, ResourceRef}; + fn length_check(len: usize, error_empty: Error, error_too_long: Error) -> Result<(), Error> { if len == 0 { Err(error_empty) @@ -107,36 +109,36 @@ impl InnerJid { }) } - pub(crate) fn node(&self) -> Option<&str> { + pub(crate) fn node(&self) -> Option<&NodeRef> { self.at.map(|at| { let at = u16::from(at) as usize; - &self.normalized[..at] + NodeRef::from_str_unchecked(&self.normalized[..at]) }) } - pub(crate) fn domain(&self) -> &str { + pub(crate) fn domain(&self) -> &DomainRef { match (self.at, self.slash) { (Some(at), Some(slash)) => { let at = u16::from(at) as usize; let slash = u16::from(slash) as usize; - &self.normalized[at + 1..slash] + DomainRef::from_str_unchecked(&self.normalized[at + 1..slash]) } (Some(at), None) => { let at = u16::from(at) as usize; - &self.normalized[at + 1..] + DomainRef::from_str_unchecked(&self.normalized[at + 1..]) } (None, Some(slash)) => { let slash = u16::from(slash) as usize; - &self.normalized[..slash] + DomainRef::from_str_unchecked(&self.normalized[..slash]) } - (None, None) => &self.normalized, + (None, None) => DomainRef::from_str_unchecked(&self.normalized), } } - pub(crate) fn resource(&self) -> Option<&str> { + pub(crate) fn resource(&self) -> Option<&ResourceRef> { self.slash.map(|slash| { let slash = u16::from(slash) as usize; - &self.normalized[slash + 1..] + ResourceRef::from_str_unchecked(&self.normalized[slash + 1..]) }) } } diff --git a/jid/src/lib.rs b/jid/src/lib.rs index 0de4d362..b10c3d64 100644 --- a/jid/src/lib.rs +++ b/jid/src/lib.rs @@ -106,9 +106,9 @@ impl Jid { /// # fn main() -> Result<(), Error> { /// let jid = Jid::new("node@domain/resource")?; /// - /// assert_eq!(jid.node_str(), Some("node")); - /// assert_eq!(jid.domain_str(), "domain"); - /// assert_eq!(jid.resource_str(), Some("resource")); + /// assert_eq!(jid.node().map(|x| x.as_str()), Some("node")); + /// assert_eq!(jid.domain().as_str(), "domain"); + /// assert_eq!(jid.resource().map(|x| x.as_str()), Some("resource")); /// # Ok(()) /// # } /// ``` @@ -132,9 +132,9 @@ impl Jid { /// already been parsed and stringprepped into [`NodePart`], [`DomainPart`], and [`ResourcePart`]. /// This method allocates and does not consume the typed parts. pub fn from_parts( - node: Option<&NodePart>, - domain: &DomainPart, - resource: Option<&ResourcePart>, + node: Option<&NodeRef>, + domain: &DomainRef, + resource: Option<&ResourceRef>, ) -> Jid { if let Some(resource) = resource { Jid::Full(FullJid::from_parts(node, domain, resource)) @@ -143,51 +143,23 @@ impl Jid { } } - /// The optional node part of the JID, as a [`NodePart`] - pub fn node(&self) -> Option { - match self { - Jid::Bare(BareJid { inner }) | Jid::Full(FullJid { inner }) => { - inner.node().map(NodePart::new_unchecked) - } - } - } - - /// The optional node part of the JID, as a stringy reference - pub fn node_str(&self) -> Option<&str> { + /// The optional node part of the JID as reference. + pub fn node(&self) -> Option<&NodeRef> { match self { Jid::Bare(BareJid { inner }) | Jid::Full(FullJid { inner }) => inner.node(), } } - /// The domain part of the JID, as a [`DomainPart`] - pub fn domain(&self) -> DomainPart { - match self { - Jid::Bare(BareJid { inner }) | Jid::Full(FullJid { inner }) => { - DomainPart::new_unchecked(inner.domain()) - } - } - } - - /// The domain part of the JID, as a stringy reference - pub fn domain_str(&self) -> &str { + /// The domain part of the JID as reference + pub fn domain(&self) -> &DomainRef { match self { Jid::Bare(BareJid { inner }) | Jid::Full(FullJid { inner }) => inner.domain(), } } - /// The optional resource part of the JID, as a [`ResourcePart`]. It is guaranteed to be present - /// when the JID is a Full variant, which you can check with [`Jid::is_full`]. - pub fn resource(&self) -> Option { - match self { - Jid::Bare(BareJid { inner }) | Jid::Full(FullJid { inner }) => { - inner.resource().map(ResourcePart::new_unchecked) - } - } - } - /// The optional resource of the Jabber ID. It is guaranteed to be present when the JID is /// a Full variant, which you can check with [`Jid::is_full`]. - pub fn resource_str(&self) -> Option<&str> { + pub fn resource(&self) -> Option<&ResourceRef> { match self { Jid::Bare(BareJid { inner }) | Jid::Full(FullJid { inner }) => inner.resource(), } @@ -415,9 +387,9 @@ impl FullJid { /// # fn main() -> Result<(), Error> { /// let jid = FullJid::new("node@domain/resource")?; /// - /// assert_eq!(jid.node_str(), Some("node")); - /// assert_eq!(jid.domain_str(), "domain"); - /// assert_eq!(jid.resource_str(), "resource"); + /// assert_eq!(jid.node().map(|x| x.as_str()), Some("node")); + /// assert_eq!(jid.domain().as_str(), "domain"); + /// assert_eq!(jid.resource().as_str(), "resource"); /// # Ok(()) /// # } /// ``` @@ -439,22 +411,27 @@ impl FullJid { /// already been parsed and stringprepped into [`NodePart`], [`DomainPart`], and [`ResourcePart`]. /// This method allocates and does not consume the typed parts. pub fn from_parts( - node: Option<&NodePart>, - domain: &DomainPart, - resource: &ResourcePart, + node: Option<&NodeRef>, + domain: &DomainRef, + resource: &ResourceRef, ) -> FullJid { let (at, slash, normalized) = if let Some(node) = node { // Parts are never empty so len > 0 for NonZeroU16::new is always Some ( - NonZeroU16::new(node.0.len() as u16), - NonZeroU16::new((node.0.len() + 1 + domain.0.len()) as u16), - format!("{}@{}/{}", node.0, domain.0, resource.0), + NonZeroU16::new(node.len() as u16), + NonZeroU16::new((node.len() + 1 + domain.len()) as u16), + format!( + "{}@{}/{}", + node.as_str(), + domain.as_str(), + resource.as_str() + ), ) } else { ( None, - NonZeroU16::new(domain.0.len() as u16), - format!("{}/{}", domain.0, resource.0), + NonZeroU16::new(domain.len() as u16), + format!("{}/{}", domain.as_str(), resource.as_str()), ) }; @@ -467,34 +444,18 @@ impl FullJid { FullJid { inner } } - /// The optional node part of the JID, as a [`NodePart`] - pub fn node(&self) -> Option { - self.node_str().map(NodePart::new_unchecked) - } - - /// The optional node part of the JID, as a stringy reference - pub fn node_str(&self) -> Option<&str> { + /// The optional node part of the JID as reference. + pub fn node(&self) -> Option<&NodeRef> { self.inner.node() } - /// The domain part of the JID, as a [`DomainPart`] - pub fn domain(&self) -> DomainPart { - DomainPart::new_unchecked(self.domain_str()) - } - - /// The domain part of the JID, as a stringy reference - pub fn domain_str(&self) -> &str { + /// The domain part of the JID as reference + pub fn domain(&self) -> &DomainRef { self.inner.domain() } - /// The optional resource part of the JID, as a [`ResourcePart`]. Since this is a full JID it - /// is always present. - pub fn resource(&self) -> ResourcePart { - ResourcePart::new_unchecked(self.resource_str()) - } - /// The optional resource of the Jabber ID. Since this is a full JID it is always present. - pub fn resource_str(&self) -> &str { + pub fn resource(&self) -> &ResourceRef { self.inner.resource().unwrap() } @@ -542,8 +503,8 @@ impl BareJid { /// # fn main() -> Result<(), Error> { /// let jid = BareJid::new("node@domain")?; /// - /// assert_eq!(jid.node_str(), Some("node")); - /// assert_eq!(jid.domain_str(), "domain"); + /// assert_eq!(jid.node().map(|x| x.as_str()), Some("node")); + /// assert_eq!(jid.domain().as_str(), "domain"); /// # Ok(()) /// # } /// ``` @@ -564,15 +525,15 @@ impl BareJid { /// Build a [`BareJid`] from typed parts. This method cannot fail because it uses parts that have /// already been parsed and stringprepped into [`NodePart`] and [`DomainPart`]. This method allocates /// and does not consume the typed parts. - pub fn from_parts(node: Option<&NodePart>, domain: &DomainPart) -> BareJid { + pub fn from_parts(node: Option<&NodeRef>, domain: &DomainRef) -> BareJid { let (at, normalized) = if let Some(node) = node { // Parts are never empty so len > 0 for NonZeroU16::new is always Some ( - NonZeroU16::new(node.0.len() as u16), - format!("{}@{}", node.0, domain.0), + NonZeroU16::new(node.len() as u16), + format!("{}@{}", node.as_str(), domain.as_str()), ) } else { - (None, domain.0.clone()) + (None, domain.to_string()) }; let inner = InnerJid { @@ -584,23 +545,13 @@ impl BareJid { BareJid { inner } } - /// The optional node part of the JID, as a [`NodePart`] - pub fn node(&self) -> Option { - self.node_str().map(NodePart::new_unchecked) - } - - /// The optional node part of the JID, as a stringy reference - pub fn node_str(&self) -> Option<&str> { + /// The optional node part of the JID as reference. + pub fn node(&self) -> Option<&NodeRef> { self.inner.node() } - /// The domain part of the JID, as a [`DomainPart`] - pub fn domain(&self) -> DomainPart { - DomainPart::new_unchecked(self.domain_str()) - } - - /// The domain part of the JID, as a stringy reference - pub fn domain_str(&self) -> &str { + /// The domain part of the JID as reference + pub fn domain(&self) -> &DomainRef { self.inner.domain() } @@ -616,11 +567,11 @@ impl BareJid { /// let bare = BareJid::new("node@domain").unwrap(); /// let full = bare.with_resource(&resource); /// - /// assert_eq!(full.node_str(), Some("node")); - /// assert_eq!(full.domain_str(), "domain"); - /// assert_eq!(full.resource_str(), "resource"); + /// assert_eq!(full.node().map(|x| x.as_str()), Some("node")); + /// assert_eq!(full.domain().as_str(), "domain"); + /// assert_eq!(full.resource().as_str(), "resource"); /// ``` - pub fn with_resource(&self, resource: &ResourcePart) -> FullJid { + pub fn with_resource(&self, resource: &ResourceRef) -> FullJid { let slash = NonZeroU16::new(self.inner.normalized.len() as u16); let normalized = format!("{}/{resource}", self.inner.normalized); let inner = InnerJid { @@ -643,9 +594,9 @@ impl BareJid { /// let bare = BareJid::new("node@domain").unwrap(); /// let full = bare.with_resource_str("resource").unwrap(); /// - /// assert_eq!(full.node_str(), Some("node")); - /// assert_eq!(full.domain_str(), "domain"); - /// assert_eq!(full.resource_str(), "resource"); + /// assert_eq!(full.node().map(|x| x.as_str()), Some("node")); + /// assert_eq!(full.domain().as_str(), "domain"); + /// assert_eq!(full.resource().as_str(), "resource"); /// ``` pub fn with_resource_str(&self, resource: &str) -> Result { let resource = ResourcePart::new(resource)?; @@ -796,30 +747,21 @@ mod tests { fn node_from_jid() { let jid = Jid::new("a@b.c/d").unwrap(); - assert_eq!(jid.node_str(), Some("a"),); - - assert_eq!(jid.node(), Some(NodePart::new("a").unwrap().into_owned())); + assert_eq!(jid.node().map(|x| x.as_str()), Some("a"),); } #[test] fn domain_from_jid() { let jid = Jid::new("a@b.c").unwrap(); - assert_eq!(jid.domain_str(), "b.c"); - - assert_eq!(jid.domain(), DomainPart::new("b.c").unwrap().into_owned()); + assert_eq!(jid.domain().as_str(), "b.c"); } #[test] fn resource_from_jid() { let jid = Jid::new("a@b.c/d").unwrap(); - assert_eq!(jid.resource_str(), Some("d"),); - - assert_eq!( - jid.resource(), - Some(ResourcePart::new("d").unwrap().into_owned()) - ); + assert_eq!(jid.resource().map(|x| x.as_str()), Some("d"),); } #[test] diff --git a/jid/src/parts.rs b/jid/src/parts.rs index cd01be0f..2ed44cd9 100644 --- a/jid/src/parts.rs +++ b/jid/src/parts.rs @@ -77,10 +77,6 @@ macro_rules! def_part_types { pub fn into_inner(self) -> String { self.0 } - - pub(crate) fn new_unchecked(s: &str) -> Self { - $name(s.to_string()) - } } impl FromStr for $name { @@ -163,7 +159,7 @@ macro_rules! def_part_types { pub struct $borrowed(pub(crate) str); impl $borrowed { - fn from_str_unchecked(s: &str) -> &Self { + pub(crate) fn from_str_unchecked(s: &str) -> &Self { // SAFETY: repr(transparent) thing can be transmuted to/from // its inner. unsafe { std::mem::transmute(s) } diff --git a/tokio-xmpp/src/client/bind.rs b/tokio-xmpp/src/client/bind.rs index b3dfcc8e..ca79ff63 100644 --- a/tokio-xmpp/src/client/bind.rs +++ b/tokio-xmpp/src/client/bind.rs @@ -15,8 +15,8 @@ pub async fn bind( if stream.stream_features.can_bind() { let resource = stream .jid - .resource_str() - .and_then(|resource| Some(resource.to_owned())); + .resource() + .and_then(|resource| Some(resource.to_string())); let iq = Iq::from_set(BIND_REQ_ID, BindQuery::new(resource)); stream.send_stanza(iq).await?; diff --git a/tokio-xmpp/src/client/connect.rs b/tokio-xmpp/src/client/connect.rs index d34929ce..29d71024 100644 --- a/tokio-xmpp/src/client/connect.rs +++ b/tokio-xmpp/src/client/connect.rs @@ -13,7 +13,7 @@ pub async fn client_login( jid: Jid, password: String, ) -> Result, Error> { - let username = jid.node_str().unwrap(); + let username = jid.node().unwrap().as_str(); let password = password; let xmpp_stream = server.connect(&jid, ns::JABBER_CLIENT).await?; diff --git a/tokio-xmpp/src/starttls/mod.rs b/tokio-xmpp/src/starttls/mod.rs index 4e6387a2..bde220cd 100644 --- a/tokio-xmpp/src/starttls/mod.rs +++ b/tokio-xmpp/src/starttls/mod.rs @@ -64,7 +64,7 @@ impl ServerConnector for ServerConfig { // TCP connection let tcp_stream = match self { ServerConfig::UseSrv => { - connect_with_srv(jid.domain_str(), "_xmpp-client._tcp", 5222).await? + connect_with_srv(jid.domain().as_str(), "_xmpp-client._tcp", 5222).await? } ServerConfig::Manual { host, port } => connect_to_host(host.as_str(), *port).await?, }; @@ -126,7 +126,7 @@ async fn get_tls_stream( async fn get_tls_stream( xmpp_stream: XMPPStream, ) -> Result, Error> { - let domain = xmpp_stream.jid.domain_str().to_owned(); + let domain = xmpp_stream.jid.domain().to_string(); let domain = ServerName::try_from(domain.as_str())?; let stream = xmpp_stream.into_inner(); let mut root_store = RootCertStore::empty(); diff --git a/tokio-xmpp/src/stream_start.rs b/tokio-xmpp/src/stream_start.rs index 7c7d4d26..4bd3fd28 100644 --- a/tokio-xmpp/src/stream_start.rs +++ b/tokio-xmpp/src/stream_start.rs @@ -15,7 +15,7 @@ pub async fn start( ns: String, ) -> Result, Error> { let attrs = [ - ("to".to_owned(), jid.domain_str().to_owned()), + ("to".to_owned(), jid.domain().to_string()), ("version".to_owned(), "1.0".to_owned()), ("xmlns".to_owned(), ns.clone()), ("xmlns:stream".to_owned(), ns::STREAM.to_owned()), diff --git a/xmpp/src/message/receive/chat.rs b/xmpp/src/message/receive/chat.rs index 198ad6ac..bca97c86 100644 --- a/xmpp/src/message/receive/chat.rs +++ b/xmpp/src/message/receive/chat.rs @@ -39,7 +39,7 @@ pub async fn handle_message_chat( Jid::Full(full) => Event::RoomPrivateMessage( message.id.clone(), full.to_bare(), - full.resource_str().to_owned(), + full.resource().to_string(), body.clone(), time_info.clone(), ), diff --git a/xmpp/src/message/receive/group_chat.rs b/xmpp/src/message/receive/group_chat.rs index 9fa17cd1..118039e6 100644 --- a/xmpp/src/message/receive/group_chat.rs +++ b/xmpp/src/message/receive/group_chat.rs @@ -21,7 +21,7 @@ pub async fn handle_message_group_chat( if let Some((_lang, subject)) = message.get_best_subject(langs.clone()) { events.push(Event::RoomSubject( from.to_bare(), - from.resource_str().map(String::from), + from.resource().map(|x| x.to_string()), subject.0.clone(), time_info.clone(), )); @@ -32,7 +32,7 @@ pub async fn handle_message_group_chat( Jid::Full(full) => Event::RoomMessage( message.id.clone(), from.to_bare(), - full.resource_str().to_owned(), + full.resource().to_string(), body.clone(), time_info, ),