From a4f50f2d43d0e9a2168b0d6bcb2b8eb51d84e525 Mon Sep 17 00:00:00 2001 From: Emmanuel Gil Peyrot Date: Sun, 21 May 2017 16:08:25 +0100 Subject: [PATCH] jingle_ibb, ibb, rsm: Simplify attribute parsing. --- src/ibb.rs | 12 ++++++------ src/jingle_ibb.rs | 36 ++++++++++++++++-------------------- src/lib.rs | 3 +++ src/rsm.rs | 2 +- 4 files changed, 26 insertions(+), 27 deletions(-) diff --git a/src/ibb.rs b/src/ibb.rs index 4d11ec11..ffdb907e 100644 --- a/src/ibb.rs +++ b/src/ibb.rs @@ -73,9 +73,9 @@ impl<'a> TryFrom<&'a Element> for IBB { for _ in elem.children() { return Err(Error::ParseError("Unknown child in open element.")); } - let block_size = get_attr!(elem, "block-size", required, block_size, block_size.parse()?); - let sid = get_attr!(elem, "sid", required, sid, sid.parse()?); - let stanza = get_attr!(elem, "stanza", default, stanza, stanza.parse()?); + let block_size = get_attr!(elem, "block-size", required); + let sid = get_attr!(elem, "sid", required); + let stanza = get_attr!(elem, "stanza", default); Ok(IBB::Open { block_size: block_size, sid: sid, @@ -85,8 +85,8 @@ impl<'a> TryFrom<&'a Element> for IBB { for _ in elem.children() { return Err(Error::ParseError("Unknown child in data element.")); } - let seq = get_attr!(elem, "seq", required, seq, seq.parse()?); - let sid = get_attr!(elem, "sid", required, sid, sid.parse()?); + let seq = get_attr!(elem, "seq", required); + let sid = get_attr!(elem, "sid", required); let data = base64::decode(&elem.text())?; Ok(IBB::Data { seq: seq, @@ -97,7 +97,7 @@ impl<'a> TryFrom<&'a Element> for IBB { for _ in elem.children() { return Err(Error::ParseError("Unknown child in close element.")); } - let sid = get_attr!(elem, "sid", required, sid, sid.parse()?); + let sid = get_attr!(elem, "sid", required); Ok(IBB::Close { sid: sid, }) diff --git a/src/jingle_ibb.rs b/src/jingle_ibb.rs index 4ac847cc..73544c26 100644 --- a/src/jingle_ibb.rs +++ b/src/jingle_ibb.rs @@ -5,7 +5,6 @@ // file, You can obtain one at http://mozilla.org/MPL/2.0/. use std::convert::TryFrom; -use std::str::FromStr; use minidom::Element; @@ -22,15 +21,6 @@ pub struct Transport { pub stanza: Stanza, } -fn optional_attr(root: &Element, attr: &str) -> Option { - root.attr(attr) - .and_then(|value| value.parse().ok()) -} - -fn required_attr(root: &Element, attr: &str, err: Error) -> Result { - optional_attr(root, attr).ok_or(err) -} - impl<'a> TryFrom<&'a Element> for Transport { type Error = Error; @@ -39,11 +29,9 @@ impl<'a> TryFrom<&'a Element> for Transport { for _ in elem.children() { return Err(Error::ParseError("Unknown child in JingleIBB element.")); } - let block_size = required_attr(elem, "block-size", Error::ParseError("Required attribute 'block-size' missing in JingleIBB element."))?; - let sid = required_attr(elem, "sid", Error::ParseError("Required attribute 'sid' missing in JingleIBB element."))?; - let stanza = elem.attr("stanza") - .unwrap_or("iq") - .parse()?; + let block_size = get_attr!(elem, "block-size", required); + let sid = get_attr!(elem, "sid", required); + let stanza = get_attr!(elem, "stanza", default); Ok(Transport { block_size: block_size, sid: sid, @@ -69,6 +57,7 @@ impl<'a> Into for &'a Transport { #[cfg(test)] mod tests { use super::*; + use std::error::Error as StdError; #[test] fn test_simple() { @@ -87,16 +76,23 @@ mod tests { Error::ParseError(string) => string, _ => panic!(), }; - assert_eq!(message, "Required attribute 'block-size' missing in JingleIBB element."); + assert_eq!(message, "Required attribute 'block-size' missing."); + + let elem: Element = "".parse().unwrap(); + let error = Transport::try_from(&elem).unwrap_err(); + let message = match error { + Error::ParseIntError(error) => error, + _ => panic!(), + }; + assert_eq!(message.description(), "number too large to fit in target type"); - // TODO: maybe make a better error message here. let elem: Element = "".parse().unwrap(); let error = Transport::try_from(&elem).unwrap_err(); let message = match error { - Error::ParseError(string) => string, + Error::ParseIntError(error) => error, _ => panic!(), }; - assert_eq!(message, "Required attribute 'block-size' missing in JingleIBB element."); + assert_eq!(message.description(), "invalid digit found in string"); let elem: Element = "".parse().unwrap(); let error = Transport::try_from(&elem).unwrap_err(); @@ -104,7 +100,7 @@ mod tests { Error::ParseError(string) => string, _ => panic!(), }; - assert_eq!(message, "Required attribute 'sid' missing in JingleIBB element."); + assert_eq!(message, "Required attribute 'sid' missing."); } #[test] diff --git a/src/lib.rs b/src/lib.rs index 24890252..db9b95aa 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -23,6 +23,9 @@ extern crate sha3; extern crate blake2; macro_rules! get_attr { + ($elem:ident, $attr:tt, $type:tt) => ( + get_attr!($elem, $attr, $type, value, value.parse()?) + ); ($elem:ident, $attr:tt, optional, $value:ident, $func:expr) => ( match $elem.attr($attr) { Some($value) => Some($func), diff --git a/src/rsm.rs b/src/rsm.rs index 99f18930..a697b370 100644 --- a/src/rsm.rs +++ b/src/rsm.rs @@ -61,7 +61,7 @@ impl<'a> TryFrom<&'a Element> for Set { if set.first.is_some() { return Err(Error::ParseError("Set can’t have more than one first.")); } - set.first_index = get_attr!(child, "index", optional, index, index.parse()?); + set.first_index = get_attr!(child, "index", optional); set.first = Some(child.text()); } else if child.is("index", ns::RSM) { if set.index.is_some() {