xmpp-parsers: Simplify PubSub Item using xso

Now that we can have a single arbitrary payload, we can derive FromXml
and AsXml on the Item.

parsers/ChangeLog

@@ -36,6 +36,7 @@ XXXX-YY-ZZ RELEASER <admin@example.com>
         fast::Tls0Rtt, legacy_omemo::IsPreKey, mam::Complete, sm::ResumeAttr (!476)
       - bookmarks::Conference and bookmarks2::Conference use ResourcePart to store
         the optional nickname instead of a String (!485)
+      - pubsub Item and pubsub#event Item are now distinct structs (!503)
       - message::Message id field is now Option<message::Id> instead of
         Option<String> (!504)
       - message_correction::Replace id field is now Id instead of String (!504)

parsers/src/openpgp.rs

@@ -64,8 +64,8 @@ mod tests {
     use super::*;
     use crate::ns;
     use crate::pubsub::{
-        pubsub::{Item as PubSubItem, Publish},
+        pubsub::{Item, Publish},
-        Item, NodeName,
+        NodeName,
     };
     use core::str::FromStr;
     use minidom::Element;
@@ -82,7 +82,7 @@ mod tests {
 
         let pubsub = Publish {
             node: NodeName(format!("{}:{}", ns::OX_PUBKEYS, "some-fingerprint")),
-            items: vec![PubSubItem(Item::new(None, None, Some(pubkey)))],
+            items: vec![Item::new(None, None, Some(pubkey))],
         };
         println!("Foo2: {:?}", pubsub);
     }
@@ -99,7 +99,7 @@ mod tests {
 
         let pubsub = Publish {
             node: NodeName("foo".to_owned()),
-            items: vec![PubSubItem(Item::new(None, None, Some(pubkeymeta)))],
+            items: vec![Item::new(None, None, Some(pubkeymeta))],
         };
         println!("Foo2: {:?}", pubsub);
     }

parsers/src/pubsub/event.rs

@@ -4,20 +4,33 @@
 // 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 xso::{AsXml, FromXml};
+
 use crate::data_forms::DataForm;
 use crate::date::DateTime;
 use crate::message::MessagePayload;
 use crate::ns;
-use crate::pubsub::{Item as PubSubItem, ItemId, NodeName, Subscription, SubscriptionId};
+use crate::pubsub::{ItemId, NodeName, Subscription, SubscriptionId};
 use jid::Jid;
 use minidom::Element;
 use xso::error::{Error, FromElementError};
 
-/// Event wrapper for a PubSub `<item/>`.
+/// An event item from a PubSub node.
-#[derive(Debug, Clone, PartialEq)]
+#[derive(FromXml, AsXml, Debug, Clone, PartialEq)]
-pub struct Item(pub PubSubItem);
+#[xml(namespace = ns::PUBSUB_EVENT, name = "item")]
+pub struct Item {
+    /// The identifier for this item, unique per node.
+    #[xml(attribute(default))]
+    pub id: Option<ItemId>,
 
-impl_pubsub_item!(Item, PUBSUB_EVENT);
+    /// The JID of the entity who published this item.
+    #[xml(attribute(default))]
+    pub publisher: Option<Jid>,
+
+    /// The payload of this item, in an arbitrary namespace.
+    #[xml(element(default))]
+    pub payload: Option<Element>,
+}
 
 /// Represents an event happening to a PubSub node.
 #[derive(Debug, Clone, PartialEq)]

parsers/src/pubsub/mod.rs

@@ -18,7 +18,6 @@ pub use self::event::PubSubEvent;
 pub use self::owner::PubSubOwner;
 pub use self::pubsub::PubSub;
 
-use jid::Jid;
 use minidom::Element;
 
 generate_id!(
@@ -77,33 +76,5 @@ generate_attribute!(
     }
 );
 
-/// An item from a PubSub node.
-#[derive(Debug, Clone, PartialEq)]
-pub struct Item {
-    /// The identifier for this item, unique per node.
-    pub id: Option<ItemId>,
-
-    /// The JID of the entity who published this item.
-    pub publisher: Option<Jid>,
-
-    /// The payload of this item, in an arbitrary namespace.
-    pub payload: Option<Element>,
-}
-
-impl Item {
-    /// Create a new item, accepting only payloads implementing `PubSubPayload`.
-    pub fn new<P: PubSubPayload>(
-        id: Option<ItemId>,
-        publisher: Option<Jid>,
-        payload: Option<P>,
-    ) -> Item {
-        Item {
-            id,
-            publisher,
-            payload: payload.map(Into::into),
-        }
-    }
-}
-
 /// This trait should be implemented on any element which can be included as a PubSub payload.
 pub trait PubSubPayload: TryFrom<Element> + Into<Element> {}

parsers/src/pubsub/pubsub.rs

@@ -14,7 +14,7 @@ use crate::data_forms::DataForm;
 use crate::iq::{IqGetPayload, IqResultPayload, IqSetPayload};
 use crate::ns;
 use crate::pubsub::{
-    AffiliationAttribute, Item as PubSubItem, NodeName, Subscription, SubscriptionId,
+    AffiliationAttribute, ItemId, NodeName, PubSubPayload, Subscription, SubscriptionId,
 };
 use jid::Jid;
 use minidom::Element;
@@ -111,11 +111,37 @@ impl Items {
     }
 }
 
-/// Response wrapper for a PubSub `<item/>`.
+/// A requested item from a PubSub node.
-#[derive(Debug, Clone, PartialEq)]
+#[derive(FromXml, AsXml, Debug, Clone, PartialEq)]
-pub struct Item(pub PubSubItem);
+#[xml(namespace = ns::PUBSUB, name = "item")]
+pub struct Item {
+    /// The identifier for this item, unique per node.
+    #[xml(attribute(default))]
+    pub id: Option<ItemId>,
 
-impl_pubsub_item!(Item, PUBSUB);
+    /// The JID of the entity who published this item.
+    #[xml(attribute(default))]
+    pub publisher: Option<Jid>,
+
+    /// The payload of this item, in an arbitrary namespace.
+    #[xml(element(default))]
+    pub payload: Option<Element>,
+}
+
+impl Item {
+    /// Create a new item, accepting only payloads implementing `PubSubPayload`.
+    pub fn new<P: PubSubPayload>(
+        id: Option<ItemId>,
+        publisher: Option<Jid>,
+        payload: Option<P>,
+    ) -> Item {
+        Item {
+            id,
+            publisher,
+            payload: payload.map(Into::into),
+        }
+    }
+}
 
 /// The options associated to a subscription request.
 #[derive(FromXml, AsXml, Debug, PartialEq, Clone)]

parsers/src/util/macros.rs

@@ -408,73 +408,3 @@ macro_rules! assert_size (
         assert_eq!(::core::mem::size_of::<$t>(), $sz);
     );
 );
-
-// TODO: move that to src/pubsub/mod.rs, once we figure out how to use macros from there.
-macro_rules! impl_pubsub_item {
-    ($item:ident, $ns:ident) => {
-        impl ::core::convert::TryFrom<minidom::Element> for $item {
-            type Error = FromElementError;
-
-            fn try_from(mut elem: minidom::Element) -> Result<$item, FromElementError> {
-                check_self!(elem, "item", $ns);
-                check_no_unknown_attributes!(elem, "item", ["id", "publisher"]);
-                let mut payloads = elem.take_contents_as_children().collect::<Vec<_>>();
-                let payload = payloads.pop();
-                if !payloads.is_empty() {
-                    return Err(Error::Other("More than a single payload in item element.").into());
-                }
-                Ok($item(crate::pubsub::Item {
-                    id: get_attr!(elem, "id", Option),
-                    publisher: get_attr!(elem, "publisher", Option),
-                    payload,
-                }))
-            }
-        }
-
-        impl ::xso::FromXml for $item {
-            type Builder = ::xso::minidom_compat::FromEventsViaElement<$item>;
-
-            fn from_events(
-                qname: ::xso::exports::rxml::QName,
-                attrs: ::xso::exports::rxml::AttrMap,
-            ) -> Result<Self::Builder, ::xso::error::FromEventsError> {
-                if qname.0 != crate::ns::$ns || qname.1 != "item" {
-                    return Err(::xso::error::FromEventsError::Mismatch { name: qname, attrs });
-                }
-                Self::Builder::new(qname, attrs)
-            }
-        }
-
-        impl From<$item> for minidom::Element {
-            fn from(item: $item) -> minidom::Element {
-                minidom::Element::builder("item", ns::$ns)
-                    .attr("id", item.0.id)
-                    .attr("publisher", item.0.publisher)
-                    .append_all(item.0.payload)
-                    .build()
-            }
-        }
-
-        impl ::xso::AsXml for $item {
-            type ItemIter<'x> = ::xso::minidom_compat::AsItemsViaElement<'x>;
-
-            fn as_xml_iter(&self) -> Result<Self::ItemIter<'_>, ::xso::error::Error> {
-                ::xso::minidom_compat::AsItemsViaElement::new(self.clone())
-            }
-        }
-
-        impl ::core::ops::Deref for $item {
-            type Target = crate::pubsub::Item;
-
-            fn deref(&self) -> &Self::Target {
-                &self.0
-            }
-        }
-
-        impl ::core::ops::DerefMut for $item {
-            fn deref_mut(&mut self) -> &mut Self::Target {
-                &mut self.0
-            }
-        }
-    };
-}

xmpp/src/pubsub/mod.rs

@@ -123,7 +123,6 @@ pub(crate) async fn handle_iq_result(
                 let mut new_room_list: Vec<BareJid> = Vec::new();
 
                 for item in items.items {
-                    let item = item.0;
                     let jid = BareJid::from_str(&item.id.clone().unwrap().0).unwrap();
                     let payload = item.payload.clone().unwrap();
                     match bookmarks2::Conference::try_from(payload) {