From 1c5551a917febf7b97b0bd7dc470d7235d4bd10b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maxime=20=E2=80=9Cpep=E2=80=9D=20Buquet?= Date: Fri, 29 Nov 2019 14:33:17 +0100 Subject: [PATCH] minidom: Implement PartialEq manually for Node and Element MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- 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(-) delete mode 100644 xmpp-parsers/src/util/compare_elements.rs diff --git a/minidom-rs/CHANGELOG.md b/minidom-rs/CHANGELOG.md index 9cef50c4dc2621f427d6b02f8014c2740f5bff21..6564977527ad5243d4d0fb83f91d86cafbceacd7 100644 --- a/minidom-rs/CHANGELOG.md +++ b/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 diff --git a/minidom-rs/src/element.rs b/minidom-rs/src/element.rs index 9907bc55d0f21921f852aa5c7b3208674c300241..9d0406568aa8bd09c0cd17402e9c8f00a923ef47 100644 --- a/minidom-rs/src/element.rs +++ b/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, @@ -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>( name: String, diff --git a/minidom-rs/src/node.rs b/minidom-rs/src/node.rs index 33f3ad03020f01dbd34c66ba867d8eaa2c0fcc3d..5f675026d3df461d17aa18db65f14bf9e3c5d8e7 100644 --- a/minidom-rs/src/node.rs +++ b/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 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, + } + } +} diff --git a/xmpp-parsers/src/bookmarks.rs b/xmpp-parsers/src/bookmarks.rs index 900a3a911656feccdd6bce3c556ac9a892eea570..7346cd8fb1a36a1a45487e263882ccb552d36d79 100644 --- a/xmpp-parsers/src/bookmarks.rs +++ b/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] diff --git a/xmpp-parsers/src/bookmarks2.rs b/xmpp-parsers/src/bookmarks2.rs index cf3eaf614c43f1da3609d08c38e6e315e7479455..ff84be9b1776078cef67c49ec535e57ddcc40d62 100644 --- a/xmpp-parsers/src/bookmarks2.rs +++ b/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] diff --git a/xmpp-parsers/src/disco.rs b/xmpp-parsers/src/disco.rs index 2041819e56e84b5faa31894a444916c9d7ad2532..6621f8b250e3b7edfd0b6b6e62ff424f435c95ad 100644 --- a/xmpp-parsers/src/disco.rs +++ b/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] diff --git a/xmpp-parsers/src/ibr.rs b/xmpp-parsers/src/ibr.rs index 72f582ffebd1eb45f3b8e74d26d6668a43364427..31d0e3325ffa08e64cc4cd380e69e2d0dd8457f1 100644 --- a/xmpp-parsers/src/ibr.rs +++ b/xmpp-parsers/src/ibr.rs @@ -117,7 +117,6 @@ impl From 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); } } diff --git a/xmpp-parsers/src/iq.rs b/xmpp-parsers/src/iq.rs index 05535283280e544f50c7ab35593f4ee0a25d755f..0f6165dde934054313bd9ee7baaa00b71ca12603 100644 --- a/xmpp-parsers/src/iq.rs +++ b/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, }); } diff --git a/xmpp-parsers/src/jingle_s5b.rs b/xmpp-parsers/src/jingle_s5b.rs index 2d7208046017d841dfafda8b8c061b0b9cb3d559..816da52692314817102cc8b3de5a117f682e6ae8 100644 --- a/xmpp-parsers/src/jingle_s5b.rs +++ b/xmpp-parsers/src/jingle_s5b.rs @@ -275,7 +275,6 @@ impl From 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); } } diff --git a/xmpp-parsers/src/message.rs b/xmpp-parsers/src/message.rs index 4c4ce2607759d7a2550f594a6731f7c3731ed71c..4e6cb3bb6dfbe600c626460e19afdc87f567cae6 100644 --- a/xmpp-parsers/src/message.rs +++ b/xmpp-parsers/src/message.rs @@ -242,7 +242,6 @@ impl From 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] diff --git a/xmpp-parsers/src/muc/muc.rs b/xmpp-parsers/src/muc/muc.rs index 935edd5f8213aa6a7fa01dc8598bbed71a1e0b7e..ec0954c24c2d2bc3d31235dfc945ba6f0b256799 100644 --- a/xmpp-parsers/src/muc/muc.rs +++ b/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] diff --git a/xmpp-parsers/src/muc/user.rs b/xmpp-parsers/src/muc/user.rs index b90298e05c91cab4fc796ceef41ddee4c7e7b33d..bcb997e258e5a37b4bbb491632e157447050bcbc 100644 --- a/xmpp-parsers/src/muc/user.rs +++ b/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"))] diff --git a/xmpp-parsers/src/presence.rs b/xmpp-parsers/src/presence.rs index 082eefbb6821bbd8ef4dc0915f4f373ebbd4a73a..8a67dde178e1ac0010d469101142da6e10b6a827 100644 --- a/xmpp-parsers/src/presence.rs +++ b/xmpp-parsers/src/presence.rs @@ -335,7 +335,6 @@ impl From 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] diff --git a/xmpp-parsers/src/pubsub/event.rs b/xmpp-parsers/src/pubsub/event.rs index 322a79f5092b5e6efdc0bef4500a6d7088e0ba74..d46b750844b6555ef6cb6ccbc224d03261aa5c24 100644 --- a/xmpp-parsers/src/pubsub/event.rs +++ b/xmpp-parsers/src/pubsub/event.rs @@ -246,7 +246,6 @@ impl From 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); } } diff --git a/xmpp-parsers/src/pubsub/pubsub.rs b/xmpp-parsers/src/pubsub/pubsub.rs index 24ad3c480777a98d4a068f9521f89c236c3012fa..42e9bdaa0eb86a153b62d7b47155297cea74a53d 100644 --- a/xmpp-parsers/src/pubsub/pubsub.rs +++ b/xmpp-parsers/src/pubsub/pubsub.rs @@ -518,7 +518,6 @@ impl From 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 = "" @@ -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] diff --git a/xmpp-parsers/src/roster.rs b/xmpp-parsers/src/roster.rs index 3153e4662a2a5c1413d4d47af7b94c1654e85304..ea3399586d6eaaa5aadb078769a67b1eb17834d1 100644 --- a/xmpp-parsers/src/roster.rs +++ b/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] diff --git a/xmpp-parsers/src/rsm.rs b/xmpp-parsers/src/rsm.rs index f59235bffe1ae082e99291e1b02e252e07b14491..4e31b2710ccb948ca68f9630d237a948f7a62472 100644 --- a/xmpp-parsers/src/rsm.rs +++ b/xmpp-parsers/src/rsm.rs @@ -174,7 +174,6 @@ impl From 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); } } diff --git a/xmpp-parsers/src/util/compare_elements.rs b/xmpp-parsers/src/util/compare_elements.rs deleted file mode 100644 index 7494be7bf2fd14a724bb0e683227b143840c8328..0000000000000000000000000000000000000000 --- a/xmpp-parsers/src/util/compare_elements.rs +++ /dev/null @@ -1,125 +0,0 @@ -// Copyright (c) 2017 Astro -// -// 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 = "x 3".parse().unwrap(); - let elem2: Element = "x 3".parse().unwrap(); - assert!(elem1.compare_to(&elem2)); - } - - #[test] - fn wrong_attr_name() { - let elem1: Element = "x 3".parse().unwrap(); - let elem2: Element = "x 3".parse().unwrap(); - assert!(!elem1.compare_to(&elem2)); - } - - #[test] - fn wrong_attr_value() { - let elem1: Element = "x 3".parse().unwrap(); - let elem2: Element = "x 3".parse().unwrap(); - assert!(!elem1.compare_to(&elem2)); - } - - #[test] - fn attr_order() { - let elem1: Element = "".parse().unwrap(); - let elem2: Element = "".parse().unwrap(); - assert!(elem1.compare_to(&elem2)); - } - - #[test] - fn wrong_texts() { - let elem1: Element = "foo".parse().unwrap(); - let elem2: Element = "bar".parse().unwrap(); - assert!(!elem1.compare_to(&elem2)); - } - - #[test] - fn children() { - let elem1: Element = "".parse().unwrap(); - let elem2: Element = "".parse().unwrap(); - assert!(elem1.compare_to(&elem2)); - } - - #[test] - fn wrong_children() { - let elem1: Element = "".parse().unwrap(); - let elem2: Element = "".parse().unwrap(); - assert!(!elem1.compare_to(&elem2)); - } - - #[test] - fn xmlns_wrong() { - let elem1: Element = "".parse().unwrap(); - let elem2: Element = "".parse().unwrap(); - assert!(!elem1.compare_to(&elem2)); - } - - #[test] - fn xmlns_other_prefix() { - let elem1: Element = "".parse().unwrap(); - let elem2: Element = "".parse().unwrap(); - assert!(elem1.compare_to(&elem2)); - } - - #[test] - fn xmlns_dup() { - let elem1: Element = "".parse().unwrap(); - let elem2: Element = "".parse().unwrap(); - assert!(elem1.compare_to(&elem2)); - } -} diff --git a/xmpp-parsers/src/util/mod.rs b/xmpp-parsers/src/util/mod.rs index 2a595f98dcf136ad651002cee79194cd64c48733..1d3ee644991bc4fe684ba8da76f70855c8d66fe1 100644 --- a/xmpp-parsers/src/util/mod.rs +++ b/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; diff --git a/xmpp-parsers/src/version.rs b/xmpp-parsers/src/version.rs index 028ce968a2bc70e7a5bf168366958d04109b0061..2bd894b872c16133d4da599ce79ed94446bd176c 100644 --- a/xmpp-parsers/src/version.rs +++ b/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); } }