xmpp-parsers: Simplify RTT implementation using more xso

Emmanuel Gil Peyrot created

Especially exhaustive enums are super useful for this kind of pattern.

Change summary

parsers/ChangeLog  |   6 +
parsers/src/rtt.rs | 278 ++++++++++++-----------------------------------
2 files changed, 76 insertions(+), 208 deletions(-)

Detailed changes

parsers/ChangeLog 🔗

@@ -9,6 +9,12 @@ XXXX-YY-ZZ RELEASER <admin@example.com>
         are now of type `BareJid` (instead of `String`), as they should always
         have been. This influences various other places, such as these
         struct's constructors.
+      - The rtt module has been changed to make use of xso on enums. This
+        allows us to simplify the `Action` type to include all three possible
+        variants directly in the enum, rather than as separate types which are
+        then included in `Action`.  Also `Action::Erase::num` is now a wrapper
+        type around u32, which lets us implement a custom Default on it.
+        (!416)
     * New parsers/serialisers:
         - Stream Features (RFC 6120) (!400)
 

parsers/src/rtt.rs 🔗

@@ -7,8 +7,6 @@
 use xso::{text::EmptyAsNone, AsXml, FromXml};
 
 use crate::ns;
-use minidom::Element;
-use xso::error::{Error, FromElementError};
 
 generate_attribute!(
     /// Events for real-time text.
@@ -30,228 +28,87 @@ generate_attribute!(
     }, Default = Edit
 );
 
-/// Supports the transmission of text, including key presses, and text block inserts.
-#[derive(FromXml, AsXml, PartialEq, Debug, Clone)]
-#[xml(namespace = ns::RTT, name = "t")]
-pub struct Insert {
-    /// Position in the message to start inserting from.  If None, this means to start from the
-    /// end of the message.
-    #[xml(attribute(default, name = "p"))]
-    pub pos: Option<u32>,
-
-    /// Text to insert.
-    #[xml(text = EmptyAsNone)]
-    pub text: Option<String>,
-}
-
-impl TryFrom<Action> for Insert {
-    type Error = Error;
-
-    fn try_from(action: Action) -> Result<Insert, Error> {
-        match action {
-            Action::Insert(insert) => Ok(insert),
-            _ => Err(Error::Other("This is not an insert action.")),
-        }
-    }
-}
-
-// TODO: add a way in the macro to set a default value.
-/*
-generate_element!(
-    Erase, "e", RTT,
-    attributes: [
-        pos: Option<u32> = "p",
-        num: Default<u32> = "n",
-    ]
+generate_attribute!(
+    /// The number of codepoints to erase.
+    Num, "n", u32, Default = 1
 );
-*/
-
-/// Supports the behavior of backspace key presses.  Text is removed towards beginning of the
-/// message.  This element is also used for all delete operations, including the backspace key, the
-/// delete key, and text block deletes.
-#[derive(Debug, Clone, PartialEq)]
-pub struct Erase {
-    /// Position in the message to start erasing from.  If None, this means to start from the end
-    /// of the message.
-    pub pos: Option<u32>,
-
-    /// Amount of characters to erase, to the left.
-    pub num: u32,
-}
-
-impl TryFrom<Element> for Erase {
-    type Error = FromElementError;
-    fn try_from(elem: Element) -> Result<Erase, FromElementError> {
-        check_self!(elem, "e", RTT);
-        check_no_unknown_attributes!(elem, "e", ["p", "n"]);
-        let pos = get_attr!(elem, "p", Option);
-        let num = get_attr!(elem, "n", Option).unwrap_or(1);
-        check_no_children!(elem, "e");
-        Ok(Erase { pos, num })
-    }
-}
-
-impl From<Erase> for Element {
-    fn from(elem: Erase) -> Element {
-        Element::builder("e", ns::RTT)
-            .attr("p", elem.pos)
-            .attr("n", elem.num)
-            .build()
-    }
-}
-
-impl TryFrom<Action> for Erase {
-    type Error = Error;
-
-    fn try_from(action: Action) -> Result<Erase, Error> {
-        match action {
-            Action::Erase(erase) => Ok(erase),
-            _ => Err(Error::Other("This is not an erase action.")),
-        }
-    }
-}
-
-/// Allow for the transmission of intervals, between real-time text actions, to recreate the
-/// pauses between key presses.
-#[derive(FromXml, AsXml, PartialEq, Debug, Clone)]
-#[xml(namespace = ns::RTT, name = "w")]
-pub struct Wait {
-    /// Amount of milliseconds to wait before the next action.
-    #[xml(attribute = "n")]
-    pub time: u32,
-}
-
-impl TryFrom<Action> for Wait {
-    type Error = Error;
-
-    fn try_from(action: Action) -> Result<Wait, Error> {
-        match action {
-            Action::Wait(wait) => Ok(wait),
-            _ => Err(Error::Other("This is not a wait action.")),
-        }
-    }
-}
 
 /// Choice between the three possible actions.
-#[derive(Debug, Clone, PartialEq)]
+#[derive(FromXml, AsXml, Debug, Clone, PartialEq)]
+#[xml(namespace = ns::RTT, exhaustive)]
 pub enum Action {
-    /// Insert text action.
-    Insert(Insert),
-
-    /// Erase text action.
-    Erase(Erase),
-
-    /// Wait action.
-    Wait(Wait),
+    /// Supports the transmission of text, including key presses, and text block inserts.
+    #[xml(name = "t")]
+    Insert {
+        /// Position in the message to start inserting from.  If None, this means to start from the
+        /// end of the message.
+        #[xml(attribute(default, name = "p"))]
+        pos: Option<u32>,
+
+        /// Text to insert.
+        #[xml(text = EmptyAsNone)]
+        text: Option<String>,
+    },
+
+    /// Supports the behavior of backspace key presses.  Text is removed towards beginning of the
+    /// message.  This element is also used for all delete operations, including the backspace key,
+    /// the delete key, and text block deletes.
+    #[xml(name = "e")]
+    Erase {
+        /// Position in the message to start erasing from.  If None, this means to start from the end
+        /// of the message.
+        #[xml(attribute(default))]
+        pos: Option<u32>,
+
+        /// Amount of characters to erase, to the left.
+        #[xml(attribute(default))]
+        num: Num,
+    },
+
+    /// Allow for the transmission of intervals, between real-time text actions, to recreate the
+    /// pauses between key presses.
+    #[xml(name = "w")]
+    Wait {
+        /// Amount of milliseconds to wait before the next action.
+        #[xml(attribute = "n")]
+        time: u32,
+    },
 }
 
-impl TryFrom<Element> for Action {
-    type Error = FromElementError;
-
-    fn try_from(elem: Element) -> Result<Action, FromElementError> {
-        match elem.name() {
-            "t" => Insert::try_from(elem).map(Action::Insert),
-            "e" => Erase::try_from(elem).map(Action::Erase),
-            "w" => Wait::try_from(elem).map(Action::Wait),
-            _ => Err(FromElementError::Mismatch(elem)),
-        }
-    }
-}
-
-impl From<Action> for Element {
-    fn from(action: Action) -> Element {
-        match action {
-            Action::Insert(insert) => Element::from(insert),
-            Action::Erase(erase) => Element::from(erase),
-            Action::Wait(wait) => Element::from(wait),
-        }
-    }
-}
-
-// TODO: Allow a wildcard name to the macro, to simplify the following code:
-/*
-generate_element!(
-    Rtt, "rtt", RTT,
-    attributes: [
-        seq: Required<u32> = "seq",
-        event: Default<Event> = "event",
-        id: Option<String> = "id",
-    ],
-    children: [
-        actions: Vec<Action> = (*, RTT) => Action,
-    ]
-);
-*/
-
 /// Element transmitted at regular interval by the sender client while a message is being composed.
-#[derive(Debug, Clone, PartialEq)]
+#[derive(FromXml, AsXml, Debug, Clone, PartialEq)]
+#[xml(namespace = ns::RTT, name = "rtt")]
 pub struct Rtt {
     /// Counter to maintain synchronisation of real-time text.  Senders MUST increment this value
     /// by 1 for each subsequent edit to the same real-time message, including when appending new
     /// text.  Receiving clients MUST monitor this 'seq' value as a lightweight verification on the
     /// synchronization of real-time text messages.  The bounds of 'seq' is 31-bits, the range of
     /// positive values for a signed 32-bit integer.
+    #[xml(attribute)]
     pub seq: u32,
 
     /// This attribute signals events for real-time text.
+    #[xml(attribute(default))]
     pub event: Event,
 
     /// When editing a message using XEP-0308, this references the id of the message being edited.
+    #[xml(attribute(default))]
     pub id: Option<String>,
 
     /// Vector of actions being transmitted by this element.
+    #[xml(child(n = ..))]
     pub actions: Vec<Action>,
 }
 
-impl TryFrom<Element> for Rtt {
-    type Error = FromElementError;
-    fn try_from(elem: Element) -> Result<Rtt, FromElementError> {
-        check_self!(elem, "rtt", RTT);
-
-        check_no_unknown_attributes!(elem, "rtt", ["seq", "event", "id"]);
-        let seq = get_attr!(elem, "seq", Required);
-        let event = get_attr!(elem, "event", Default);
-        let id = get_attr!(elem, "id", Option);
-
-        let mut actions = Vec::new();
-        for child in elem.children() {
-            if child.ns() != ns::RTT {
-                return Err(Error::Other("Unknown child in rtt element.").into());
-            }
-            actions.push(Action::try_from(child.clone())?);
-        }
-
-        Ok(Rtt {
-            seq,
-            event,
-            id,
-            actions,
-        })
-    }
-}
-
-impl From<Rtt> for Element {
-    fn from(elem: Rtt) -> Element {
-        Element::builder("rtt", ns::RTT)
-            .attr("seq", elem.seq)
-            .attr("event", elem.event)
-            .attr("id", elem.id)
-            .append_all(elem.actions)
-            .build()
-    }
-}
-
 #[cfg(test)]
 mod tests {
     use super::*;
+    use minidom::Element;
 
     #[cfg(target_pointer_width = "32")]
     #[test]
     fn test_size() {
         assert_size!(Event, 1);
-        assert_size!(Insert, 20);
-        assert_size!(Erase, 12);
-        assert_size!(Wait, 4);
         assert_size!(Action, 20);
         assert_size!(Rtt, 32);
     }
@@ -260,9 +117,6 @@ mod tests {
     #[test]
     fn test_size() {
         assert_size!(Event, 1);
-        assert_size!(Insert, 32);
-        assert_size!(Erase, 12);
-        assert_size!(Wait, 4);
         assert_size!(Action, 32);
         assert_size!(Rtt, 56);
     }
@@ -291,19 +145,27 @@ mod tests {
         let mut actions = rtt.actions.into_iter();
         assert_eq!(actions.len(), 4);
 
-        let t: Insert = actions.next().unwrap().try_into().unwrap();
-        assert_eq!(t.pos, None);
-        assert_eq!(t.text, Some(String::from("Hello,")));
-
-        let w: Wait = actions.next().unwrap().try_into().unwrap();
-        assert_eq!(w.time, 50);
-
-        let e: Erase = actions.next().unwrap().try_into().unwrap();
-        assert_eq!(e.pos, None);
-        assert_eq!(e.num, 1);
-
-        let t: Insert = actions.next().unwrap().try_into().unwrap();
-        assert_eq!(t.pos, None);
-        assert_eq!(t.text, Some(String::from("!")));
+        let Action::Insert { pos, text } = actions.next().unwrap() else {
+            panic!()
+        };
+        assert_eq!(pos, None);
+        assert_eq!(text.unwrap(), "Hello,");
+
+        let Action::Wait { time } = actions.next().unwrap() else {
+            panic!()
+        };
+        assert_eq!(time, 50);
+
+        let Action::Erase { pos, num } = actions.next().unwrap() else {
+            panic!()
+        };
+        assert_eq!(pos, None);
+        assert_eq!(num, Num(1));
+
+        let Action::Insert { pos, text } = actions.next().unwrap() else {
+            panic!()
+        };
+        assert_eq!(pos, None);
+        assert_eq!(text.unwrap(), "!");
     }
 }