minidom: Implement PartialEq manually for Node and Element

Maxime “pep” Buquet created 

Move the NamespaceAwareCompare implementation from xmpp-parsers as Node
and Element's PartialEq implementation. Thanks Astro!

It's a lot more useful in tests to use `assert_eq!` than `assert!`, so
we get both items compared (left and right) instead of a "it failed."
message.

This "breaks" comparison for these two structs in the sense that it is
not strict object comparison anymore but it ensures that namespaces are
all present in the compared objects.

Signed-off-by: Maxime “pep” Buquet <pep@bouah.net>

Change summary

minidom-rs/CHANGELOG.md                   |   4 
minidom-rs/src/element.rs                 |  27 +++++
minidom-rs/src/node.rs                    |  12 ++
xmpp-parsers/src/bookmarks.rs             |   3 
xmpp-parsers/src/bookmarks2.rs            |   3 
xmpp-parsers/src/disco.rs                 |   3 
xmpp-parsers/src/ibr.rs                   |   5 
xmpp-parsers/src/iq.rs                    |   7 
xmpp-parsers/src/jingle_s5b.rs            |   5 
xmpp-parsers/src/message.rs               |   7 
xmpp-parsers/src/muc/muc.rs               |   3 
xmpp-parsers/src/muc/user.rs              |   3 
xmpp-parsers/src/presence.rs              |   3 
xmpp-parsers/src/pubsub/event.rs          |   3 
xmpp-parsers/src/pubsub/pubsub.rs         |  11 +-
xmpp-parsers/src/roster.rs                |   3 
xmpp-parsers/src/rsm.rs                   |   3 
xmpp-parsers/src/util/compare_elements.rs | 125 -------------------------
xmpp-parsers/src/util/mod.rs              |   4 
xmpp-parsers/src/version.rs               |   3 
20 files changed, 65 insertions(+), 172 deletions(-)

Detailed changes

minidom-rs/CHANGELOG.md 🔗 

@@ -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

minidom-rs/src/element.rs 🔗 

@@ -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,

minidom-rs/src/node.rs 🔗 

@@ -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,
+        }
+    }
+}

xmpp-parsers/src/bookmarks.rs 🔗 

@@ -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]

xmpp-parsers/src/bookmarks2.rs 🔗 

@@ -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]

xmpp-parsers/src/disco.rs 🔗 

@@ -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]

xmpp-parsers/src/ibr.rs 🔗 

@@ -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);
     }
 }

xmpp-parsers/src/iq.rs 🔗 

@@ -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,
         });
     }

xmpp-parsers/src/jingle_s5b.rs 🔗 

@@ -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);
     }
 }

xmpp-parsers/src/message.rs 🔗 

@@ -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]

xmpp-parsers/src/muc/muc.rs 🔗 

@@ -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]

xmpp-parsers/src/muc/user.rs 🔗 

@@ -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"))]

xmpp-parsers/src/presence.rs 🔗 

@@ -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]

xmpp-parsers/src/pubsub/event.rs 🔗 

@@ -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);
     }
 }

xmpp-parsers/src/pubsub/pubsub.rs 🔗 

@@ -518,7 +518,6 @@ impl From<PubSub> for Element {
 #[cfg(test)]
 mod tests {
     use super::*;
-    use crate::util::compare_elements::NamespaceAwareCompare;
 
     #[test]
     fn create() {
@@ -536,7 +535,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,7 +552,7 @@ mod tests {
         }
 
         let elem2 = Element::from(pubsub);
-        assert!(elem1.compare_to(&elem2));
+        assert_eq!(elem1, elem2);
     }
 
     #[test]
@@ -573,7 +572,7 @@ mod tests {
         }
 
         let elem2 = Element::from(pubsub);
-        assert!(elem1.compare_to(&elem2));
+        assert_eq!(elem1, elem2);
     }
 
     #[test]
@@ -596,7 +595,7 @@ mod tests {
         }
 
         let elem2 = Element::from(pubsub);
-        assert!(elem1.compare_to(&elem2));
+        assert_eq!(elem1, elem2);
     }
 
     #[test]
@@ -616,7 +615,7 @@ mod tests {
         }
 
         let elem2 = Element::from(pubsub);
-        assert!(elem1.compare_to(&elem2));
+        assert_eq!(elem1, elem2);
     }
 
     #[test]

xmpp-parsers/src/roster.rs 🔗 

@@ -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]

xmpp-parsers/src/rsm.rs 🔗 

@@ -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);
     }
 }

xmpp-parsers/src/util/compare_elements.rs 🔗 

@@ -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));
-    }
-}

xmpp-parsers/src/util/mod.rs 🔗 

@@ -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;

xmpp-parsers/src/version.rs 🔗 

@@ -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);
     }
 }