hashes, presence, message, iq, disco: Use get_attr!.

Emmanuel Gil Peyrot created

Change summary

src/disco.rs    | 36 +++++++++++++++---------------------
src/hashes.rs   |  5 +----
src/iq.rs       | 16 +++++-----------
src/message.rs  | 18 ++++++------------
src/presence.rs | 16 +++++-----------
5 files changed, 32 insertions(+), 59 deletions(-)

Detailed changes

src/disco.rs 🔗

@@ -46,43 +46,37 @@ impl<'a> TryFrom<&'a Element> for Disco {
         let mut features: Vec<Feature> = vec!();
         let mut extensions: Vec<DataForm> = vec!();
 
-        let node = elem.attr("node")
-                       .and_then(|node| node.parse().ok());
+        let node = get_attr!(elem, "node", optional);
 
         for child in elem.children() {
             if child.is("feature", ns::DISCO_INFO) {
-                let feature = child.attr("var")
-                                   .ok_or(Error::ParseError("Feature must have a 'var' attribute."))?;
+                let feature = get_attr!(child, "var", required);
                 features.push(Feature {
-                    var: feature.to_owned(),
+                    var: feature,
                 });
             } else if child.is("identity", ns::DISCO_INFO) {
-                let category = child.attr("category")
-                                    .ok_or(Error::ParseError("Identity must have a 'category' attribute."))?;
+                let category = get_attr!(child, "category", required);
                 if category == "" {
                     return Err(Error::ParseError("Identity must have a non-empty 'category' attribute."))
                 }
 
-                let type_ = child.attr("type")
-                                 .ok_or(Error::ParseError("Identity must have a 'type' attribute."))?;
+                let type_ = get_attr!(child, "type", required);
                 if type_ == "" {
                     return Err(Error::ParseError("Identity must have a non-empty 'type' attribute."))
                 }
 
-                let xml_lang = child.attr("xml:lang").unwrap_or("");
-                let name = child.attr("name")
-                                .and_then(|name| name.parse().ok());
+                let lang = get_attr!(child, "xml:lang", default);
+                let name = get_attr!(child, "name", optional);
                 identities.push(Identity {
-                    category: category.to_owned(),
-                    type_: type_.to_owned(),
-                    xml_lang: xml_lang.to_owned(),
+                    category: category,
+                    type_: type_,
+                    xml_lang: lang,
                     name: name,
                 });
             } else if child.is("x", ns::DATA_FORMS) {
                 let data_form = DataForm::try_from(child)?;
-                match data_form.type_ {
-                    DataFormType::Result_ => (),
-                    _ => return Err(Error::ParseError("Data form must have a 'result' type in disco#info.")),
+                if data_form.type_ != DataFormType::Result_ {
+                    return Err(Error::ParseError("Data form must have a 'result' type in disco#info."));
                 }
                 match data_form.form_type {
                     Some(_) => extensions.push(data_form),
@@ -178,7 +172,7 @@ mod tests {
             Error::ParseError(string) => string,
             _ => panic!(),
         };
-        assert_eq!(message, "Identity must have a 'category' attribute.");
+        assert_eq!(message, "Required attribute 'category' missing.");
 
         let elem: Element = "<query xmlns='http://jabber.org/protocol/disco#info'><identity category=''/></query>".parse().unwrap();
         let error = Disco::try_from(&elem).unwrap_err();
@@ -194,7 +188,7 @@ mod tests {
             Error::ParseError(string) => string,
             _ => panic!(),
         };
-        assert_eq!(message, "Identity must have a 'type' attribute.");
+        assert_eq!(message, "Required attribute 'type' missing.");
 
         let elem: Element = "<query xmlns='http://jabber.org/protocol/disco#info'><identity category='coucou' type=''/></query>".parse().unwrap();
         let error = Disco::try_from(&elem).unwrap_err();
@@ -213,7 +207,7 @@ mod tests {
             Error::ParseError(string) => string,
             _ => panic!(),
         };
-        assert_eq!(message, "Feature must have a 'var' attribute.");
+        assert_eq!(message, "Required attribute 'var' missing.");
     }
 
     #[test]

src/hashes.rs 🔗

@@ -76,10 +76,7 @@ impl<'a> TryFrom<&'a Element> for Hash {
         for _ in elem.children() {
             return Err(Error::ParseError("Unknown child in hash element."));
         }
-        let algo = match elem.attr("algo") {
-            None => Err(Error::ParseError("Mandatory argument 'algo' not present in hash element.")),
-            Some(text) => Algo::from_str(text),
-        }?;
+        let algo = get_attr!(elem, "algo", required);
         let hash = match elem.text().as_ref() {
             "" => return Err(Error::ParseError("Hash element shouldn’t be empty.")),
             text => text.to_owned(),

src/iq.rs 🔗

@@ -100,16 +100,10 @@ impl<'a> TryFrom<&'a Element> for Iq {
         if !root.is("iq", ns::JABBER_CLIENT) {
             return Err(Error::ParseError("This is not an iq element."));
         }
-        let from = root.attr("from")
-            .and_then(|value| value.parse().ok());
-        let to = root.attr("to")
-            .and_then(|value| value.parse().ok());
-        let id = root.attr("id")
-            .and_then(|value| value.parse().ok());
-        let type_ = match root.attr("type") {
-            Some(type_) => type_,
-            None => return Err(Error::ParseError("Iq element requires a 'type' attribute.")),
-        };
+        let from = get_attr!(root, "from", optional);
+        let to = get_attr!(root, "to", optional);
+        let id = get_attr!(root, "id", optional);
+        let type_: String = get_attr!(root, "type", required);
 
         let mut payload = None;
         let mut error_payload = None;
@@ -218,7 +212,7 @@ mod tests {
             Error::ParseError(string) => string,
             _ => panic!(),
         };
-        assert_eq!(message, "Iq element requires a 'type' attribute.");
+        assert_eq!(message, "Required attribute 'type' missing.");
     }
 
     #[test]

src/message.rs 🔗

@@ -169,16 +169,10 @@ impl<'a> TryFrom<&'a Element> for Message {
         if !root.is("message", ns::JABBER_CLIENT) {
             return Err(Error::ParseError("This is not a message element."));
         }
-        let from = root.attr("from")
-            .and_then(|value| value.parse().ok());
-        let to = root.attr("to")
-            .and_then(|value| value.parse().ok());
-        let id = root.attr("id")
-            .and_then(|value| value.parse().ok());
-        let type_ = match root.attr("type") {
-            Some(type_) => type_.parse()?,
-            None => Default::default(),
-        };
+        let from = get_attr!(root, "from", optional);
+        let to = get_attr!(root, "to", optional);
+        let id = get_attr!(root, "id", optional);
+        let type_ = get_attr!(root, "type", default);
         let mut bodies = BTreeMap::new();
         let mut subjects = BTreeMap::new();
         let mut thread = None;
@@ -188,7 +182,7 @@ impl<'a> TryFrom<&'a Element> for Message {
                 for _ in elem.children() {
                     return Err(Error::ParseError("Unknown child in body element."));
                 }
-                let lang = elem.attr("xml:lang").unwrap_or("").to_owned();
+                let lang = get_attr!(root, "xml:lang", default);
                 if bodies.insert(lang, elem.text()).is_some() {
                     return Err(Error::ParseError("Body element present twice for the same xml:lang."));
                 }
@@ -196,7 +190,7 @@ impl<'a> TryFrom<&'a Element> for Message {
                 for _ in elem.children() {
                     return Err(Error::ParseError("Unknown child in subject element."));
                 }
-                let lang = elem.attr("xml:lang").unwrap_or("").to_owned();
+                let lang = get_attr!(root, "xml:lang", default);
                 if subjects.insert(lang, elem.text()).is_some() {
                     return Err(Error::ParseError("Subject element present twice for the same xml:lang."));
                 }

src/presence.rs 🔗

@@ -164,16 +164,10 @@ impl<'a> TryFrom<&'a Element> for Presence {
         if !root.is("presence", ns::JABBER_CLIENT) {
             return Err(Error::ParseError("This is not a presence element."));
         }
-        let from = root.attr("from")
-            .and_then(|value| value.parse().ok());
-        let to = root.attr("to")
-            .and_then(|value| value.parse().ok());
-        let id = root.attr("id")
-            .and_then(|value| value.parse().ok());
-        let type_ = match root.attr("type") {
-            Some(type_) => type_.parse()?,
-            None => Default::default(),
-        };
+        let from = get_attr!(root, "from", optional);
+        let to = get_attr!(root, "to", optional);
+        let id = get_attr!(root, "id", optional);
+        let type_ = get_attr!(root, "type", default);
         let mut show = None;
         let mut statuses = BTreeMap::new();
         let mut priority = None;
@@ -206,7 +200,7 @@ impl<'a> TryFrom<&'a Element> for Presence {
                         return Err(Error::ParseError("Unknown attribute in status element."));
                     }
                 }
-                let lang = elem.attr("xml:lang").unwrap_or("").to_owned();
+                let lang = get_attr!(elem, "xml:lang", default);
                 if statuses.insert(lang, elem.text()).is_some() {
                     return Err(Error::ParseError("Status element present twice for the same xml:lang."));
                 }