Merge branch 'fix-serialization' into 'master'

parsers: Fix serialization

See merge request xmpp-rs/xmpp-rs!42
This commit is contained in:
Maxime Buquet 2019-12-01 02:53:41 +00:00
commit 473aaa03ef
24 changed files with 154 additions and 200 deletions

View file

@ -1,6 +1,8 @@
Version XXX, released YYY:
* Breaking
`Element.write_to` doesn't prepand xml prelude anymore. Use `write_to_decl`.
* `Element.write_to` doesn't prepand xml prelude anymore. Use `write_to_decl`.
* PartialEq implementation for Element and Node have been changed to
ensure namespaces match even if the objects are slightly different.
* Changes
* Update edition to 2018
* Add NSChoice enum to allow comparing NSs differently

View file

@ -68,7 +68,7 @@ pub fn escape(raw: &[u8]) -> Cow<[u8]> {
}
}
#[derive(Clone, PartialEq, Eq, Debug)]
#[derive(Clone, Eq, Debug)]
/// A struct representing a DOM Element.
pub struct Element {
prefix: Option<String>,
@ -95,6 +95,31 @@ impl FromStr for Element {
}
}
impl PartialEq for Element {
fn eq(&self, other: &Self) -> bool {
if self.name() == other.name() && self.ns() == other.ns() && self.attrs().eq(other.attrs())
{
let child_elems = self.children().count();
let text_is_whitespace = self
.texts()
.all(|text| text.chars().all(char::is_whitespace));
if child_elems > 0 && text_is_whitespace {
// Ignore all the whitespace text nodes
self.children()
.zip(other.children())
.all(|(node1, node2)| node1 == node2)
} else {
// Compare with text nodes
self.nodes()
.zip(other.nodes())
.all(|(node1, node2)| node1 == node2)
}
} else {
false
}
}
}
impl Element {
fn new<NS: Into<NamespaceSet>>(
name: String,

View file

@ -9,7 +9,7 @@ use quick_xml::events::{BytesText, Event};
use quick_xml::Writer as EventWriter;
/// A node in an element tree.
#[derive(Clone, Debug, PartialEq, Eq)]
#[derive(Clone, Debug, Eq)]
pub enum Node {
/// An `Element`.
Element(Element),
@ -208,3 +208,13 @@ impl From<ElementBuilder> for Node {
Node::Element(builder.build())
}
}
impl PartialEq for Node {
fn eq(&self, other: &Self) -> bool {
match (self, other) {
(&Node::Element(ref elem1), &Node::Element(ref elem2)) => elem1 == elem2,
(&Node::Text(ref text1), &Node::Text(ref text2)) => text1 == text2,
_ => false,
}
}
}

View file

@ -70,7 +70,6 @@ impl Storage {
#[cfg(test)]
mod tests {
use super::*;
use crate::util::compare_elements::NamespaceAwareCompare;
use crate::Element;
use std::convert::TryFrom;
@ -99,7 +98,7 @@ mod tests {
assert_eq!(storage.urls.len(), 0);
let elem2 = Element::from(Storage::new());
assert!(elem1.compare_to(&elem2));
assert_eq!(elem1, elem2);
}
#[test]

View file

@ -48,7 +48,6 @@ mod tests {
use crate::ns;
use crate::pubsub::event::PubSubEvent;
use crate::pubsub::pubsub::Item as PubSubItem;
use crate::util::compare_elements::NamespaceAwareCompare;
use crate::Element;
use std::convert::TryFrom;
@ -77,7 +76,7 @@ mod tests {
assert_eq!(conference.password, None);
let elem2 = Element::from(Conference::new());
assert!(elem1.compare_to(&elem2));
assert_eq!(elem1, elem2);
}
#[test]

View file

@ -232,7 +232,6 @@ impl IqResultPayload for DiscoItemsResult {}
#[cfg(test)]
mod tests {
use super::*;
use crate::util::compare_elements::NamespaceAwareCompare;
use std::str::FromStr;
#[cfg(target_pointer_width = "32")]
@ -301,7 +300,7 @@ mod tests {
assert_eq!(query.extensions[0].form_type, Some(String::from("example")));
let elem2 = query.into();
assert!(elem1.compare_to(&elem2));
assert_eq!(elem1, elem2);
}
#[test]

View file

@ -117,7 +117,6 @@ impl From<Query> for Element {
#[cfg(test)]
mod tests {
use super::*;
use crate::util::compare_elements::NamespaceAwareCompare;
#[cfg(target_pointer_width = "32")]
#[test]
@ -207,7 +206,7 @@ mod tests {
let form = query.form.clone().unwrap();
assert!(!form.instructions.unwrap().is_empty());
let elem2 = query.into();
assert!(elem1.compare_to(&elem2));
assert_eq!(elem1, elem2);
}
#[test]
@ -242,6 +241,6 @@ mod tests {
panic!();
}
let elem2 = query.into();
assert!(elem1.compare_to(&elem2));
assert_eq!(elem1, elem2);
}
}

View file

@ -220,7 +220,6 @@ mod tests {
use super::*;
use crate::disco::DiscoInfoQuery;
use crate::stanza_error::{DefinedCondition, ErrorType};
use crate::util::compare_elements::NamespaceAwareCompare;
#[cfg(target_pointer_width = "32")]
#[test]
@ -283,7 +282,7 @@ mod tests {
assert_eq!(iq.to, None);
assert_eq!(&iq.id, "foo");
assert!(match iq.payload {
IqType::Get(element) => element.compare_to(&query),
IqType::Get(element) => element == query,
_ => false,
});
}
@ -308,7 +307,7 @@ mod tests {
assert_eq!(iq.to, None);
assert_eq!(&iq.id, "vcard");
assert!(match iq.payload {
IqType::Set(element) => element.compare_to(&vcard),
IqType::Set(element) => element == vcard,
_ => false,
});
}
@ -355,7 +354,7 @@ mod tests {
assert_eq!(iq.to, None);
assert_eq!(&iq.id, "res");
assert!(match iq.payload {
IqType::Result(Some(element)) => element.compare_to(&query),
IqType::Result(Some(element)) => element == query,
_ => false,
});
}

View file

@ -858,4 +858,39 @@ mod tests {
};
assert_eq!(message, "Text element present twice for the same xml:lang.");
}
#[test]
fn test_serialize_jingle() {
let reference: Element = "<jingle xmlns='urn:xmpp:jingle:1' action='session-initiate' sid='a73sjjvkla37jfea'><content xmlns='urn:xmpp:jingle:1' creator='initiator' name='this-is-a-stub'><description xmlns='urn:xmpp:jingle:apps:stub:0'/><transport xmlns='urn:xmpp:jingle:transports:stub:0'/></content></jingle>"
.parse()
.unwrap();
let jingle = Jingle {
action: Action::SessionInitiate,
initiator: None,
responder: None,
sid: SessionId(String::from("a73sjjvkla37jfea")),
contents: vec![Content {
creator: Creator::Initiator,
disposition: Disposition::default(),
name: ContentId(String::from("this-is-a-stub")),
senders: Senders::default(),
description: Some(Description::Unknown(
Element::builder("description")
.ns("urn:xmpp:jingle:apps:stub:0")
.build(),
)),
transport: Some(Transport::Unknown(
Element::builder("transport")
.ns("urn:xmpp:jingle:transports:stub:0")
.build(),
)),
security: None,
}],
reason: None,
other: vec![],
};
let serialized: Element = jingle.into();
assert_eq!(serialized, reference);
}
}

View file

@ -275,7 +275,6 @@ impl From<Transport> for Element {
#[cfg(test)]
mod tests {
use super::*;
use crate::util::compare_elements::NamespaceAwareCompare;
use std::str::FromStr;
#[cfg(target_pointer_width = "32")]
@ -327,7 +326,7 @@ mod tests {
payload: TransportPayload::Activated(CandidateId(String::from("coucou"))),
};
let elem2: Element = transport.into();
assert!(elem.compare_to(&elem2));
assert_eq!(elem, elem2);
}
#[test]
@ -347,6 +346,6 @@ mod tests {
}]),
};
let elem2: Element = transport.into();
assert!(elem.compare_to(&elem2));
assert_eq!(elem, elem2);
}
}

View file

@ -391,21 +391,9 @@ mod tests {
assert_eq!(elem, elem2);
}
#[ignore]
#[test]
fn test_serialize_query_with() {
let elem: Element = r#"
<query xmlns='urn:xmpp:mam:2'>
<x xmlns='jabber:x:data' type='submit'>
<field var='FORM_TYPE' type='hidden'>
<value>urn:xmpp:mam:2</value>
</field>
<field var='with'>
<value>juliet@capulet.lit</value>
</field>
</x>
</query>
"#
let reference: Element = "<query xmlns='urn:xmpp:mam:2'><x xmlns='jabber:x:data' type='submit'><field xmlns='jabber:x:data' var='FORM_TYPE' type='hidden'><value xmlns='jabber:x:data'>urn:xmpp:mam:2</value></field><field xmlns='jabber:x:data' var='with'><value xmlns='jabber:x:data'>juliet@capulet.lit</value></field></x></query>"
.parse()
.unwrap();
@ -415,10 +403,10 @@ mod tests {
title: None,
instructions: None,
fields: vec![Field {
var: String::from("var"),
var: String::from("with"),
type_: FieldType::TextSingle,
label: None,
required: true,
required: false,
options: vec![],
values: vec![String::from("juliet@capulet.lit")],
media: vec![],
@ -430,7 +418,7 @@ mod tests {
set: None,
form: Some(form),
};
let elem2 = foo.into();
assert_eq!(elem, elem2);
let serialized: Element = foo.into();
assert_eq!(serialized, reference);
}
}

View file

@ -242,7 +242,6 @@ impl From<Message> for Element {
#[cfg(test)]
mod tests {
use super::*;
use crate::util::compare_elements::NamespaceAwareCompare;
use std::str::FromStr;
#[cfg(target_pointer_width = "32")]
@ -312,7 +311,7 @@ mod tests {
}
let elem2 = message.into();
assert!(elem1.compare_to(&elem2));
assert_eq!(elem1, elem2);
}
#[test]
@ -326,7 +325,7 @@ mod tests {
.bodies
.insert(String::from(""), Body::from_str("Hello world!").unwrap());
let elem2 = message.into();
assert!(elem.compare_to(&elem2));
assert_eq!(elem, elem2);
}
#[test]
@ -349,7 +348,7 @@ mod tests {
}
let elem2 = message.into();
assert!(elem1.compare_to(&elem2));
assert_eq!(elem1, elem2);
}
#[test]

View file

@ -94,7 +94,6 @@ impl Muc {
#[cfg(test)]
mod tests {
use super::*;
use crate::util::compare_elements::NamespaceAwareCompare;
use crate::util::error::Error;
use crate::Element;
use std::convert::TryFrom;
@ -161,7 +160,7 @@ mod tests {
assert_eq!(muc.password, Some("coucou".to_owned()));
let elem2 = Element::from(muc);
assert!(elem1.compare_to(&elem2));
assert_eq!(elem1, elem2);
}
#[test]

View file

@ -236,7 +236,6 @@ generate_element!(
#[cfg(test)]
mod tests {
use super::*;
use crate::util::compare_elements::NamespaceAwareCompare;
use std::error::Error as StdError;
#[test]
@ -298,7 +297,7 @@ mod tests {
items: vec![],
};
let elem2 = muc.into();
assert!(elem.compare_to(&elem2));
assert_eq!(elem, elem2);
}
#[cfg(not(feature = "disable-validation"))]

View file

@ -52,6 +52,8 @@ pub const PUBSUB_ERRORS: &str = "http://jabber.org/protocol/pubsub#errors";
pub const PUBSUB_EVENT: &str = "http://jabber.org/protocol/pubsub#event";
/// XEP-0060: Publish-Subscribe
pub const PUBSUB_OWNER: &str = "http://jabber.org/protocol/pubsub#owner";
/// XEP-0060: Publish-Subscribe node configuration
pub const PUBSUB_CONFIGURE: &str = "http://jabber.org/protocol/pubsub#node_config";
/// XEP-0071: XHTML-IM
pub const XHTML_IM: &str = "http://jabber.org/protocol/xhtml-im";

View file

@ -335,7 +335,6 @@ impl From<Presence> for Element {
#[cfg(test)]
mod tests {
use super::*;
use crate::util::compare_elements::NamespaceAwareCompare;
use jid::{BareJid, FullJid};
#[cfg(target_pointer_width = "32")]
@ -382,7 +381,7 @@ mod tests {
.unwrap();
let presence = Presence::new(Type::Unavailable);
let elem2 = presence.into();
assert!(elem.compare_to(&elem2));
assert_eq!(elem, elem2);
}
#[test]

View file

@ -246,7 +246,6 @@ impl From<PubSubEvent> for Element {
#[cfg(test)]
mod tests {
use super::*;
use crate::util::compare_elements::NamespaceAwareCompare;
use std::str::FromStr;
#[test]
@ -420,6 +419,6 @@ mod tests {
}
let elem2: Element = event.into();
assert!(elem.compare_to(&elem2));
assert_eq!(elem, elem2);
}
}

View file

@ -518,7 +518,7 @@ impl From<PubSub> for Element {
#[cfg(test)]
mod tests {
use super::*;
use crate::util::compare_elements::NamespaceAwareCompare;
use crate::data_forms::{DataForm, DataFormType, Field, FieldType};
#[test]
fn create() {
@ -536,7 +536,7 @@ mod tests {
}
let elem2 = Element::from(pubsub);
assert!(elem1.compare_to(&elem2));
assert_eq!(elem1, elem2);
let elem: Element =
"<pubsub xmlns='http://jabber.org/protocol/pubsub'><create node='coucou'/></pubsub>"
@ -553,11 +553,11 @@ mod tests {
}
let elem2 = Element::from(pubsub);
assert!(elem1.compare_to(&elem2));
assert_eq!(elem1, elem2);
}
#[test]
fn create_and_configure() {
fn create_and_configure_empty() {
let elem: Element =
"<pubsub xmlns='http://jabber.org/protocol/pubsub'><create/><configure/></pubsub>"
.parse()
@ -573,7 +573,42 @@ mod tests {
}
let elem2 = Element::from(pubsub);
assert!(elem1.compare_to(&elem2));
assert_eq!(elem1, elem2);
}
#[test]
fn create_and_configure_simple() {
// XXX: Do we want xmpp-parsers to always specify the field type in the output Element?
let elem: Element = "<pubsub xmlns='http://jabber.org/protocol/pubsub'><create node='foo'/><configure><x xmlns='jabber:x:data' type='submit'><field var='FORM_TYPE' type='hidden'><value>http://jabber.org/protocol/pubsub#node_config</value></field><field var='pubsub#access_model' type='list-single'><value>whitelist</value></field></x></configure></pubsub>"
.parse()
.unwrap();
let elem1 = elem.clone();
let pubsub = PubSub::Create {
create: Create {
node: Some(NodeName(String::from("foo"))),
},
configure: Some(Configure {
form: Some(DataForm {
type_: DataFormType::Submit,
form_type: Some(String::from(ns::PUBSUB_CONFIGURE)),
title: None,
instructions: None,
fields: vec![Field {
var: String::from("pubsub#access_model"),
type_: FieldType::ListSingle,
label: None,
required: false,
options: vec![],
values: vec![String::from("whitelist")],
media: vec![],
}],
}),
}),
};
let elem2 = Element::from(pubsub);
assert_eq!(elem1, elem2);
}
#[test]
@ -596,7 +631,7 @@ mod tests {
}
let elem2 = Element::from(pubsub);
assert!(elem1.compare_to(&elem2));
assert_eq!(elem1, elem2);
}
#[test]
@ -616,7 +651,7 @@ mod tests {
}
let elem2 = Element::from(pubsub);
assert!(elem1.compare_to(&elem2));
assert_eq!(elem1, elem2);
}
#[test]

View file

@ -92,7 +92,6 @@ impl IqResultPayload for Roster {}
#[cfg(test)]
mod tests {
use super::*;
use crate::util::compare_elements::NamespaceAwareCompare;
use crate::util::error::Error;
use crate::Element;
use std::convert::TryFrom;
@ -220,7 +219,7 @@ mod tests {
assert_eq!(roster.items[0].groups[0], Group::from_str("A").unwrap());
assert_eq!(roster.items[0].groups[1], Group::from_str("B").unwrap());
let elem2 = roster.into();
assert!(elem1.compare_to(&elem2));
assert_eq!(elem1, elem2);
}
#[test]

View file

@ -174,7 +174,6 @@ impl From<SetResult> for Element {
#[cfg(test)]
mod tests {
use super::*;
use crate::util::compare_elements::NamespaceAwareCompare;
#[cfg(target_pointer_width = "32")]
#[test]
@ -304,6 +303,6 @@ mod tests {
count: None,
};
let elem2 = set2.into();
assert!(elem1.compare_to(&elem2));
assert_eq!(elem1, elem2);
}
}

View file

@ -1,125 +0,0 @@
// Copyright (c) 2017 Astro <astro@spaceboyz.net>
//
// This Source Code Form is subject to the terms of the Mozilla Public
// 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 minidom::{Element, Node};
pub trait NamespaceAwareCompare {
/// Namespace-aware comparison for tests
fn compare_to(&self, other: &Self) -> bool;
}
impl NamespaceAwareCompare for Node {
fn compare_to(&self, other: &Self) -> bool {
match (self, other) {
(&Node::Element(ref elem1), &Node::Element(ref elem2)) => {
Element::compare_to(elem1, elem2)
}
(&Node::Text(ref text1), &Node::Text(ref text2)) => text1 == text2,
_ => false,
}
}
}
impl NamespaceAwareCompare for Element {
fn compare_to(&self, other: &Self) -> bool {
if self.name() == other.name() && self.ns() == other.ns() && self.attrs().eq(other.attrs())
{
let child_elems = self.children().count();
let text_is_whitespace = self
.texts()
.all(|text| text.chars().all(char::is_whitespace));
if child_elems > 0 && text_is_whitespace {
// Ignore all the whitespace text nodes
self.children()
.zip(other.children())
.all(|(node1, node2)| node1.compare_to(node2))
} else {
// Compare with text nodes
self.nodes()
.zip(other.nodes())
.all(|(node1, node2)| node1.compare_to(node2))
}
} else {
false
}
}
}
#[cfg(test)]
mod tests {
use super::*;
use crate::Element;
#[test]
fn simple() {
let elem1: Element = "<a a='b'>x <l/> 3</a>".parse().unwrap();
let elem2: Element = "<a a='b'>x <l/> 3</a>".parse().unwrap();
assert!(elem1.compare_to(&elem2));
}
#[test]
fn wrong_attr_name() {
let elem1: Element = "<a a='b'>x 3</a>".parse().unwrap();
let elem2: Element = "<a c='b'>x 3</a>".parse().unwrap();
assert!(!elem1.compare_to(&elem2));
}
#[test]
fn wrong_attr_value() {
let elem1: Element = "<a a='b'>x 3</a>".parse().unwrap();
let elem2: Element = "<a a='c'>x 3</a>".parse().unwrap();
assert!(!elem1.compare_to(&elem2));
}
#[test]
fn attr_order() {
let elem1: Element = "<e1 a='b' c='d'/>".parse().unwrap();
let elem2: Element = "<e1 c='d' a='b'/>".parse().unwrap();
assert!(elem1.compare_to(&elem2));
}
#[test]
fn wrong_texts() {
let elem1: Element = "<e1>foo</e1>".parse().unwrap();
let elem2: Element = "<e1>bar</e1>".parse().unwrap();
assert!(!elem1.compare_to(&elem2));
}
#[test]
fn children() {
let elem1: Element = "<e1><foo/><bar/></e1>".parse().unwrap();
let elem2: Element = "<e1><foo/><bar/></e1>".parse().unwrap();
assert!(elem1.compare_to(&elem2));
}
#[test]
fn wrong_children() {
let elem1: Element = "<e1><foo/></e1>".parse().unwrap();
let elem2: Element = "<e1><bar/></e1>".parse().unwrap();
assert!(!elem1.compare_to(&elem2));
}
#[test]
fn xmlns_wrong() {
let elem1: Element = "<e1 xmlns='ns1'><foo/></e1>".parse().unwrap();
let elem2: Element = "<e1 xmlns='ns2'><foo/></e1>".parse().unwrap();
assert!(!elem1.compare_to(&elem2));
}
#[test]
fn xmlns_other_prefix() {
let elem1: Element = "<e1 xmlns='ns1'><foo/></e1>".parse().unwrap();
let elem2: Element = "<x:e1 xmlns:x='ns1'><x:foo/></x:e1>".parse().unwrap();
assert!(elem1.compare_to(&elem2));
}
#[test]
fn xmlns_dup() {
let elem1: Element = "<e1 xmlns='ns1'><foo/></e1>".parse().unwrap();
let elem2: Element = "<e1 xmlns='ns1'><foo xmlns='ns1'/></e1>".parse().unwrap();
assert!(elem1.compare_to(&elem2));
}
}

View file

@ -590,18 +590,18 @@ macro_rules! generate_serialiser {
}))
};
($builder:ident, $parent:ident, $elem:ident, Option, $constructor:ident, ($name:tt, *)) => {
$builder.append_all($parent.$elem.map(|elem| {
crate::Element::builder($name)
.ns(elem.get_ns())
.append(::minidom::Node::Element(crate::Element::from(elem)))
}))
$builder.append_all(
$parent
.$elem
.map(|elem| ::minidom::Node::Element(crate::Element::from(elem))),
)
};
($builder:ident, $parent:ident, $elem:ident, Option, $constructor:ident, ($name:tt, $ns:ident)) => {
$builder.append_all($parent.$elem.map(|elem| {
crate::Element::builder($name)
.ns(crate::ns::$ns)
.append(::minidom::Node::Element(crate::Element::from(elem)))
}))
$builder.append_all(
$parent
.$elem
.map(|elem| ::minidom::Node::Element(crate::Element::from(elem))),
)
};
($builder:ident, $parent:ident, $elem:ident, Vec, $constructor:ident, ($name:tt, $ns:ident)) => {
$builder.append_all($parent.$elem.into_iter())

View file

@ -13,7 +13,3 @@ pub(crate) mod helpers;
/// Helper macros to parse and serialise more easily.
#[macro_use]
mod macros;
#[cfg(test)]
/// Namespace-aware comparison for tests
pub(crate) mod compare_elements;

View file

@ -41,7 +41,6 @@ impl IqResultPayload for VersionResult {}
#[cfg(test)]
mod tests {
use super::*;
use crate::util::compare_elements::NamespaceAwareCompare;
use crate::Element;
use std::convert::TryFrom;
@ -84,6 +83,6 @@ mod tests {
.parse()
.unwrap();
println!("{:?}", elem1);
assert!(elem1.compare_to(&elem2));
assert_eq!(elem1, elem2);
}
}