events: Rework Relation serde

Relation types now implement `Deserialize`, `Serialize`.

crates/ruma-events/CHANGELOG.md

@@ -3,6 +3,12 @@
 Improvements:
 
 * Add `From` implementations for event and event content enums
+* It's now an error for a `room::message::Relation` to be `Replaces` without
+  there being `new_content`
+  * Previously, this used to set the relation to `None`
+* Unsupported relations are now deserialized to `relates_to: Some(_)` instead of
+  `None`
+  * It's not possible to inspect the inner value though
 
 # 0.24.4
 

crates/ruma-events/src/room/encrypted.rs

@@ -33,7 +33,7 @@ pub struct EncryptedEventContent {
     /// Information about related messages for [rich replies].
     ///
     /// [rich replies]: https://matrix.org/docs/spec/client_server/r0.6.1#rich-replies
-    #[serde(flatten, with = "relation_serde", skip_serializing_if = "Option::is_none")]
+    #[serde(flatten, skip_serializing_if = "Option::is_none")]
     pub relates_to: Option<Relation>,
 }
 
@@ -91,6 +91,7 @@ pub enum EncryptedEventScheme {
 ///
 /// Outside of the encrypted payload to support server aggregation.
 #[derive(Clone, Debug)]
+#[allow(clippy::manual_non_exhaustive)]
 #[cfg_attr(not(feature = "unstable-exhaustive-types"), non_exhaustive)]
 pub enum Relation {
     /// An `m.in_reply_to` relation indicating that the event is a reply to another event.
@@ -113,6 +114,9 @@ pub enum Relation {
     #[cfg(feature = "unstable-pre-spec")]
     #[cfg_attr(docsrs, doc(cfg(feature = "unstable-pre-spec")))]
     Annotation(Annotation),
+
+    #[doc(hidden)]
+    _Custom,
 }
 
 /// The event this relation belongs to replaces another event.
@@ -263,6 +267,7 @@ impl From<message::Relation> for Relation {
             message::Relation::Replacement(re) => {
                 Self::Replacement(Replacement { event_id: re.event_id })
             }
+            message::Relation::_Custom => Self::_Custom,
         }
     }
 }

crates/ruma-events/src/room/encrypted/relation_serde.rs

@@ -1,77 +1,69 @@
-use serde::{ser::SerializeStruct as _, Deserialize, Deserializer, Serialize, Serializer};
+use serde::{Deserialize, Deserializer, Serialize, Serializer};
 
 #[cfg(feature = "unstable-pre-spec")]
 use super::{Annotation, Reference, Replacement};
 use super::{InReplyTo, Relation};
 
-pub fn deserialize<'de, D>(deserializer: D) -> Result<Option<Relation>, D::Error>
+impl<'de> Deserialize<'de> for Relation {
-where
+    fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
-    D: Deserializer<'de>,
+    where
-{
+        D: Deserializer<'de>,
-    fn convert_relation(ev: EventWithRelatesToJsonRepr) -> Option<Relation> {
+    {
-        if let Some(in_reply_to) = ev.relates_to.in_reply_to {
+        fn convert_relation(ev: EventWithRelatesToJsonRepr) -> Relation {
-            return Some(Relation::Reply { in_reply_to });
+            if let Some(in_reply_to) = ev.relates_to.in_reply_to {
+                return Relation::Reply { in_reply_to };
+            }
+
+            #[cfg(feature = "unstable-pre-spec")]
+            if let Some(relation) = ev.relates_to.relation {
+                return match relation {
+                    RelationJsonRepr::Annotation(a) => Relation::Annotation(a),
+                    RelationJsonRepr::Reference(r) => Relation::Reference(r),
+                    RelationJsonRepr::Replacement(Replacement { event_id }) => {
+                        Relation::Replacement(Replacement { event_id })
+                    }
+                    // FIXME: Maybe we should log this, though at this point we don't even have
+                    // access to the rel_type of the unknown relation.
+                    RelationJsonRepr::Unknown => Relation::_Custom,
+                };
+            }
+
+            Relation::_Custom
         }
 
-        #[cfg(feature = "unstable-pre-spec")]
+        EventWithRelatesToJsonRepr::deserialize(deserializer).map(convert_relation)
-        if let Some(relation) = ev.relates_to.relation {
-            let relation = match relation {
-                RelationJsonRepr::Annotation(a) => Relation::Annotation(a),
-                RelationJsonRepr::Reference(r) => Relation::Reference(r),
-                RelationJsonRepr::Replacement(Replacement { event_id }) => {
-                    Relation::Replacement(Replacement { event_id })
-                }
-                // FIXME: Maybe we should log this, though at this point we don't even have access
-                // to the rel_type of the unknown relation.
-                RelationJsonRepr::Unknown => return None,
-            };
-
-            return Some(relation);
-        }
-
-        None
     }
-
-    EventWithRelatesToJsonRepr::deserialize(deserializer).map(convert_relation)
 }
 
-pub fn serialize<S>(relation: &Option<Relation>, serializer: S) -> Result<S::Ok, S::Error>
+impl Serialize for Relation {
-where
+    fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
-    S: Serializer,
+    where
-{
+        S: Serializer,
-    let relation = match relation {
+    {
-        Some(rel) => rel,
+        let relates_to = match self {
-        // FIXME: If this crate ends up depending on tracing, emit a warning here.
+            #[cfg(feature = "unstable-pre-spec")]
-        // This code path should not be reachable due to the skip_serializing_if serde attribute
+            Relation::Annotation(r) => RelatesToJsonRepr {
-        // that should be applied together with `with = "relation_serde"`.
+                relation: Some(RelationJsonRepr::Annotation(r.clone())),
-        None => return serializer.serialize_struct("NoRelation", 0)?.end(),
+                ..Default::default()
-    };
+            },
-
+            #[cfg(feature = "unstable-pre-spec")]
-    let json_repr = match relation {
+            Relation::Reference(r) => RelatesToJsonRepr {
-        #[cfg(feature = "unstable-pre-spec")]
+                relation: Some(RelationJsonRepr::Reference(r.clone())),
-        Relation::Annotation(r) => EventWithRelatesToJsonRepr::new(RelatesToJsonRepr {
+                ..Default::default()
-            relation: Some(RelationJsonRepr::Annotation(r.clone())),
+            },
-            ..Default::default()
+            #[cfg(feature = "unstable-pre-spec")]
-        }),
+            Relation::Replacement(r) => RelatesToJsonRepr {
-        #[cfg(feature = "unstable-pre-spec")]
-        Relation::Reference(r) => EventWithRelatesToJsonRepr::new(RelatesToJsonRepr {
-            relation: Some(RelationJsonRepr::Reference(r.clone())),
-            ..Default::default()
-        }),
-        #[cfg(feature = "unstable-pre-spec")]
-        Relation::Replacement(r) => EventWithRelatesToJsonRepr {
-            relates_to: RelatesToJsonRepr {
                 relation: Some(RelationJsonRepr::Replacement(r.clone())),
                 ..Default::default()
             },
-        },
+            Relation::Reply { in_reply_to } => {
-        Relation::Reply { in_reply_to } => EventWithRelatesToJsonRepr::new(RelatesToJsonRepr {
+                RelatesToJsonRepr { in_reply_to: Some(in_reply_to.clone()), ..Default::default() }
-            in_reply_to: Some(in_reply_to.clone()),
+            }
-            ..Default::default()
+            Relation::_Custom => RelatesToJsonRepr::default(),
-        }),
+        };
-    };
 
-    json_repr.serialize(serializer)
+        EventWithRelatesToJsonRepr { relates_to }.serialize(serializer)
+    }
 }
 
 #[derive(Deserialize, Serialize)]
@@ -80,12 +72,6 @@ struct EventWithRelatesToJsonRepr {
     relates_to: RelatesToJsonRepr,
 }
 
-impl EventWithRelatesToJsonRepr {
-    fn new(relates_to: RelatesToJsonRepr) -> Self {
-        Self { relates_to }
-    }
-}
-
 /// Enum modeling the different ways relationships can be expressed in a `m.relates_to` field of an
 /// event.
 #[derive(Default, Deserialize, Serialize)]

crates/ruma-events/src/room/message.rs

@@ -41,7 +41,7 @@ pub struct MessageEventContent {
     /// Information about related messages for [rich replies].
     ///
     /// [rich replies]: https://matrix.org/docs/spec/client_server/r0.6.1#rich-replies
-    #[serde(flatten, with = "relation_serde", skip_serializing_if = "Option::is_none")]
+    #[serde(flatten, skip_serializing_if = "Option::is_none")]
     pub relates_to: Option<Relation>,
 }
 
@@ -265,6 +265,7 @@ impl From<MessageType> for MessageEventContent {
 ///
 /// Currently used for replies and editing (message replacement).
 #[derive(Clone, Debug)]
+#[allow(clippy::manual_non_exhaustive)]
 #[cfg_attr(not(feature = "unstable-exhaustive-types"), non_exhaustive)]
 pub enum Relation {
     /// An `m.in_reply_to` relation indicating that the event is a reply to another event.
@@ -277,6 +278,9 @@ pub enum Relation {
     #[cfg(feature = "unstable-pre-spec")]
     #[cfg_attr(docsrs, doc(cfg(feature = "unstable-pre-spec")))]
     Replacement(Replacement),
+
+    #[doc(hidden)]
+    _Custom,
 }
 
 /// Information about the event a "rich reply" is replying to.

crates/ruma-events/src/room/message/content_serde.rs

@@ -3,10 +3,8 @@
 use serde::{de, Deserialize};
 use serde_json::value::RawValue as RawJsonValue;
 
-use crate::{
+use super::{MessageEventContent, MessageType, Relation};
-    from_raw_json_value,
+use crate::from_raw_json_value;
-    room::message::{MessageEventContent, MessageType},
-};
 
 impl<'de> Deserialize<'de> for MessageEventContent {
     fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
@@ -15,10 +13,10 @@ impl<'de> Deserialize<'de> for MessageEventContent {
     {
         let json = Box::<RawJsonValue>::deserialize(deserializer)?;
         let mut deserializer = serde_json::Deserializer::from_str(json.get());
-        let relation =
+        let relates_to =
-            super::relation_serde::deserialize(&mut deserializer).map_err(de::Error::custom)?;
+            Option::<Relation>::deserialize(&mut deserializer).map_err(de::Error::custom)?;
 
-        Ok(Self { msgtype: from_raw_json_value(&json)?, relates_to: relation })
+        Ok(Self { msgtype: from_raw_json_value(&json)?, relates_to })
     }
 }
 

crates/ruma-events/src/room/message/relation_serde.rs

@@ -1,6 +1,6 @@
 #[cfg(feature = "unstable-pre-spec")]
 use ruma_identifiers::EventId;
-use serde::{ser::SerializeStruct as _, Deserialize, Deserializer, Serialize, Serializer};
+use serde::{Deserialize, Deserializer, Serialize, Serializer};
 
 #[cfg(feature = "unstable-pre-spec")]
 use super::Replacement;
@@ -8,71 +8,66 @@ use super::{InReplyTo, Relation};
 #[cfg(feature = "unstable-pre-spec")]
 use crate::room::message::MessageEventContent;
 
-pub fn deserialize<'de, D>(deserializer: D) -> Result<Option<Relation>, D::Error>
+impl<'de> Deserialize<'de> for Relation {
-where
+    fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
-    D: Deserializer<'de>,
+    where
-{
+        D: Deserializer<'de>,
-    fn convert_relation(ev: EventWithRelatesToJsonRepr) -> Option<Relation> {
+    {
+        let ev = EventWithRelatesToJsonRepr::deserialize(deserializer)?;
+
         if let Some(in_reply_to) = ev.relates_to.in_reply_to {
-            return Some(Relation::Reply { in_reply_to });
+            return Ok(Relation::Reply { in_reply_to });
         }
 
         #[cfg(feature = "unstable-pre-spec")]
         if let Some(relation) = ev.relates_to.relation {
-            let relation = match relation {
+            return Ok(match relation {
                 RelationJsonRepr::Replacement(ReplacementJsonRepr { event_id }) => {
-                    let new_content = ev.new_content?;
+                    let new_content = ev
+                        .new_content
+                        .ok_or_else(|| serde::de::Error::missing_field("m.new_content"))?;
                     Relation::Replacement(Replacement { event_id, new_content })
                 }
-                // FIXME: Maybe we should log this, though at this point we don't even have access
+                // FIXME: Maybe we should log this, though at this point we don't even have
-                // to the rel_type of the unknown relation.
+                // access to the rel_type of the unknown relation.
-                RelationJsonRepr::Unknown => return None,
+                RelationJsonRepr::Unknown => Relation::_Custom,
-            };
+            });
-
-            return Some(relation);
         }
 
-        None
+        Ok(Relation::_Custom)
     }
-
-    EventWithRelatesToJsonRepr::deserialize(deserializer).map(convert_relation)
 }
 
-pub fn serialize<S>(relation: &Option<Relation>, serializer: S) -> Result<S::Ok, S::Error>
+impl Serialize for Relation {
-where
+    fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
-    S: Serializer,
+    where
-{
+        S: Serializer,
-    let relation = match relation {
+    {
-        Some(rel) => rel,
+        let json_repr = match self {
-        // FIXME: If this crate ends up depending on tracing, emit a warning here.
+            Relation::Reply { in_reply_to } => EventWithRelatesToJsonRepr::new(RelatesToJsonRepr {
-        // This code path should not be reachable due to the skip_serializing_if serde attribute
+                in_reply_to: Some(in_reply_to.clone()),
-        // that should be applied together with `with = "relation_serde"`.
+                ..Default::default()
-        None => return serializer.serialize_struct("NoRelation", 0)?.end(),
+            }),
-    };
+            #[cfg(feature = "unstable-pre-spec")]
-
+            Relation::Replacement(Replacement { event_id, new_content }) => {
-    let json_repr = match relation {
+                EventWithRelatesToJsonRepr {
-        Relation::Reply { in_reply_to } => EventWithRelatesToJsonRepr::new(RelatesToJsonRepr {
+                    relates_to: RelatesToJsonRepr {
-            in_reply_to: Some(in_reply_to.clone()),
+                        relation: Some(RelationJsonRepr::Replacement(ReplacementJsonRepr {
-            ..Default::default()
+                            event_id: event_id.clone(),
-        }),
+                        })),
-        #[cfg(feature = "unstable-pre-spec")]
+                        ..Default::default()
-        Relation::Replacement(Replacement { event_id, new_content }) => {
+                    },
-            EventWithRelatesToJsonRepr {
+                    new_content: Some(new_content.clone()),
-                relates_to: RelatesToJsonRepr {
+                }
-                    relation: Some(RelationJsonRepr::Replacement(ReplacementJsonRepr {
-                        event_id: event_id.clone(),
-                    })),
-                    ..Default::default()
-                },
-                new_content: Some(new_content.clone()),
             }
-        }
+            Relation::_Custom => EventWithRelatesToJsonRepr::default(),
-    };
+        };
 
-    json_repr.serialize(serializer)
+        json_repr.serialize(serializer)
+    }
 }
 
-#[derive(Deserialize, Serialize)]
+#[derive(Default, Deserialize, Serialize)]
 struct EventWithRelatesToJsonRepr {
     #[serde(rename = "m.relates_to", default, skip_serializing_if = "RelatesToJsonRepr::is_empty")]
     relates_to: RelatesToJsonRepr,