Offer {Resource,Node,Domain}Ref on Jid API

Jonas Schäfer created 2 years ago

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.

Change summary

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(-)

Detailed changes

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

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..])
         })
     }
 }

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<NodePart> {
-        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<ResourcePart> {
-        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<NodePart> {
-        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<NodePart> {
-        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<FullJid, Error> {
         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]

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) }

tokio-xmpp/src/client/bind.rs

@@ -15,8 +15,8 @@ pub async fn bind<S: AsyncRead + AsyncWrite + Unpin>(
     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?;
 

tokio-xmpp/src/client/connect.rs

@@ -13,7 +13,7 @@ pub async fn client_login<C: ServerConnector>(
     jid: Jid,
     password: String,
 ) -> Result<XMPPStream<C::Stream>, 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?;

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<S: AsyncRead + AsyncWrite + Unpin>(
 async fn get_tls_stream<S: AsyncRead + AsyncWrite + Unpin>(
     xmpp_stream: XMPPStream<S>,
 ) -> Result<TlsStream<S>, 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();

tokio-xmpp/src/stream_start.rs

@@ -15,7 +15,7 @@ pub async fn start<S: AsyncRead + AsyncWrite + Unpin>(
     ns: String,
 ) -> Result<XMPPStream<S>, 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()),

xmpp/src/message/receive/chat.rs

@@ -39,7 +39,7 @@ pub async fn handle_message_chat<C: ServerConnector>(
                     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(),
                     ),

xmpp/src/message/receive/group_chat.rs

@@ -21,7 +21,7 @@ pub async fn handle_message_group_chat<C: ServerConnector>(
     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<C: ServerConnector>(
             Jid::Full(full) => Event::RoomMessage(
                 message.id.clone(),
                 from.to_bare(),
-                full.resource_str().to_owned(),
+                full.resource().to_string(),
                 body.clone(),
                 time_info,
             ),