From 8673b8f90e2eabb4e1c50057fcfbd4bbb3eab807 Mon Sep 17 00:00:00 2001 From: Emmanuel Gil Peyrot Date: Sat, 6 May 2017 20:51:39 +0100 Subject: [PATCH] data_forms: Switch to Into/TryFrom. --- src/data_forms.rs | 114 ++++++++++++++++++++++--------------------- src/disco.rs | 6 ++- src/mam.rs | 5 +- src/media_element.rs | 4 +- 4 files changed, 66 insertions(+), 63 deletions(-) diff --git a/src/data_forms.rs b/src/data_forms.rs index 94dae1ca..a83ebb82 100644 --- a/src/data_forms.rs +++ b/src/data_forms.rs @@ -53,70 +53,72 @@ pub struct DataForm { pub fields: Vec, } -pub fn parse_data_form(root: &Element) -> Result { - if !root.is("x", ns::DATA_FORMS) { - return Err(Error::ParseError("This is not a data form element.")); - } +impl<'a> TryFrom<&'a Element> for DataForm { + type Error = Error; - let type_: DataFormType = match root.attr("type") { - Some(type_) => type_.parse()?, - None => return Err(Error::ParseError("Type attribute on data form is mandatory.")), - }; - let mut fields = vec!(); - let mut form_type = None; - for field in root.children() { - if field.is("field", ns::DATA_FORMS) { - let var = field.attr("var").ok_or(Error::ParseError("Field must have a 'var' attribute."))?; - let field_type = field.attr("type").unwrap_or("text-single"); - let label = field.attr("label").and_then(|label| label.parse().ok()); - let mut values = vec!(); - let mut media = vec!(); - for element in field.children() { - if element.is("value", ns::DATA_FORMS) { - values.push(element.text()); - } else if element.is("media", ns::MEDIA_ELEMENT) { - match MediaElement::try_from(element) { - Ok(media_element) => media.push(media_element), - Err(_) => (), // TODO: is it really nice to swallow this error? - } - } else { - return Err(Error::ParseError("Field child isn’t a value or media element.")); - } - } - if var == "FORM_TYPE" && field_type == "hidden" { - if form_type != None { - return Err(Error::ParseError("More than one FORM_TYPE in a data form.")); - } - if values.len() != 1 { - return Err(Error::ParseError("Wrong number of values in FORM_TYPE.")); - } - form_type = Some(values[0].clone()); - } - fields.push(Field { - var: var.to_owned(), - type_: field_type.to_owned(), - label: label, - values: values, - media: media, - }); - } else { - return Err(Error::ParseError("Unknown field type in data form.")); + fn try_from(elem: &'a Element) -> Result { + if !elem.is("x", ns::DATA_FORMS) { + return Err(Error::ParseError("This is not a data form element.")); } + + let type_: DataFormType = match elem.attr("type") { + Some(type_) => type_.parse()?, + None => return Err(Error::ParseError("Type attribute on data form is mandatory.")), + }; + let mut fields = vec!(); + let mut form_type = None; + for field in elem.children() { + if field.is("field", ns::DATA_FORMS) { + let var = field.attr("var").ok_or(Error::ParseError("Field must have a 'var' attribute."))?; + let field_type = field.attr("type").unwrap_or("text-single"); + let label = field.attr("label").and_then(|label| label.parse().ok()); + let mut values = vec!(); + let mut media = vec!(); + for element in field.children() { + if element.is("value", ns::DATA_FORMS) { + values.push(element.text()); + } else if element.is("media", ns::MEDIA_ELEMENT) { + match MediaElement::try_from(element) { + Ok(media_element) => media.push(media_element), + Err(_) => (), // TODO: is it really nice to swallow this error? + } + } else { + return Err(Error::ParseError("Field child isn’t a value or media element.")); + } + } + if var == "FORM_TYPE" && field_type == "hidden" { + if form_type != None { + return Err(Error::ParseError("More than one FORM_TYPE in a data form.")); + } + if values.len() != 1 { + return Err(Error::ParseError("Wrong number of values in FORM_TYPE.")); + } + form_type = Some(values[0].clone()); + } + fields.push(Field { + var: var.to_owned(), + type_: field_type.to_owned(), + label: label, + values: values, + media: media, + }); + } else { + return Err(Error::ParseError("Unknown field type in data form.")); + } + } + Ok(DataForm { type_: type_, form_type: form_type, fields: fields }) } - Ok(DataForm { type_: type_, form_type: form_type, fields: fields }) } #[cfg(test)] mod tests { - use minidom::Element; - use error::Error; - use data_forms; + use super::*; #[test] fn test_simple() { let elem: Element = "".parse().unwrap(); - let form = data_forms::parse_data_form(&elem).unwrap(); - assert_eq!(form.type_, data_forms::DataFormType::Result_); + let form = DataForm::try_from(&elem).unwrap(); + assert_eq!(form.type_, DataFormType::Result_); assert!(form.form_type.is_none()); assert!(form.fields.is_empty()); } @@ -124,7 +126,7 @@ mod tests { #[test] fn test_invalid() { let elem: Element = "".parse().unwrap(); - let error = data_forms::parse_data_form(&elem).unwrap_err(); + let error = DataForm::try_from(&elem).unwrap_err(); let message = match error { Error::ParseError(string) => string, _ => panic!(), @@ -132,7 +134,7 @@ mod tests { assert_eq!(message, "Type attribute on data form is mandatory."); let elem: Element = "".parse().unwrap(); - let error = data_forms::parse_data_form(&elem).unwrap_err(); + let error = DataForm::try_from(&elem).unwrap_err(); let message = match error { Error::ParseError(string) => string, _ => panic!(), @@ -143,7 +145,7 @@ mod tests { #[test] fn test_wrong_child() { let elem: Element = "".parse().unwrap(); - let error = data_forms::parse_data_form(&elem).unwrap_err(); + let error = DataForm::try_from(&elem).unwrap_err(); let message = match error { Error::ParseError(string) => string, _ => panic!(), diff --git a/src/disco.rs b/src/disco.rs index bc448091..9fb52906 100644 --- a/src/disco.rs +++ b/src/disco.rs @@ -4,12 +4,14 @@ // License, v. 2.0. If a copy of the MPL was not distributed with this // file, You can obtain one at http://mozilla.org/MPL/2.0/. +use std::convert::TryFrom; + use minidom::Element; use error::Error; use ns; -use data_forms::{DataForm, DataFormType, parse_data_form}; +use data_forms::{DataForm, DataFormType}; #[derive(Debug, Clone, PartialEq)] pub struct Feature { @@ -74,7 +76,7 @@ pub fn parse_disco(root: &Element) -> Result { name: name, }); } else if child.is("x", ns::DATA_FORMS) { - let data_form = parse_data_form(child)?; + 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.")), diff --git a/src/mam.rs b/src/mam.rs index 77a3b6cf..308cc1c0 100644 --- a/src/mam.rs +++ b/src/mam.rs @@ -11,7 +11,6 @@ use jid::Jid; use error::Error; -use data_forms; use data_forms::DataForm; use rsm::Set; use forwarding; @@ -62,7 +61,7 @@ pub fn parse_query(root: &Element) -> Result { let mut set = None; for child in root.children() { if child.is("x", ns::DATA_FORMS) { - form = Some(data_forms::parse_data_form(child)?); + form = Some(DataForm::try_from(child)?); } else if child.is("set", ns::RSM) { set = Some(Set::try_from(child)?); } else { @@ -177,7 +176,7 @@ pub fn serialise_query(query: &Query) -> Element { .attr("node", query.node.clone()) .build(); //if let Some(form) = query.form { - // elem.append_child(data_forms::serialise(&form)); + // elem.append_child((&form).into()); //} if let Some(ref set) = query.set { elem.append_child(set.into()); diff --git a/src/media_element.rs b/src/media_element.rs index be2adda1..d4d31985 100644 --- a/src/media_element.rs +++ b/src/media_element.rs @@ -55,7 +55,7 @@ impl<'a> TryFrom<&'a Element> for MediaElement { #[cfg(test)] mod tests { use super::*; - use data_forms; + use data_forms::DataForm; #[test] fn test_simple() { @@ -177,7 +177,7 @@ mod tests { [ ... ] "#.parse().unwrap(); - let form = data_forms::parse_data_form(&elem).unwrap(); + let form = DataForm::try_from(&elem).unwrap(); assert_eq!(form.fields.len(), 1); assert_eq!(form.fields[0].var, "ocr"); assert_eq!(form.fields[0].media[0].width, Some(290));