From a81880c68f08e52dd2e163c2e8cae7f68e211c89 Mon Sep 17 00:00:00 2001 From: Jonas Platte Date: Sat, 21 Aug 2021 15:20:15 +0200 Subject: [PATCH] events: Rework Relation serde Relation types now implement `Deserialize`, `Serialize`. --- crates/ruma-events/CHANGELOG.md | 6 + crates/ruma-events/src/room/encrypted.rs | 7 +- .../src/room/encrypted/relation_serde.rs | 118 ++++++++---------- crates/ruma-events/src/room/message.rs | 6 +- .../src/room/message/content_serde.rs | 12 +- .../src/room/message/relation_serde.rs | 93 +++++++------- 6 files changed, 118 insertions(+), 124 deletions(-) diff --git a/crates/ruma-events/CHANGELOG.md b/crates/ruma-events/CHANGELOG.md index 15297a93..29673464 100644 --- a/crates/ruma-events/CHANGELOG.md +++ b/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 diff --git a/crates/ruma-events/src/room/encrypted.rs b/crates/ruma-events/src/room/encrypted.rs index 0788ff75..320f3795 100644 --- a/crates/ruma-events/src/room/encrypted.rs +++ b/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, } @@ -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 for Relation { message::Relation::Replacement(re) => { Self::Replacement(Replacement { event_id: re.event_id }) } + message::Relation::_Custom => Self::_Custom, } } } diff --git a/crates/ruma-events/src/room/encrypted/relation_serde.rs b/crates/ruma-events/src/room/encrypted/relation_serde.rs index 77f7f7f3..18690a8b 100644 --- a/crates/ruma-events/src/room/encrypted/relation_serde.rs +++ b/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, D::Error> -where - D: Deserializer<'de>, -{ - fn convert_relation(ev: EventWithRelatesToJsonRepr) -> Option { - if let Some(in_reply_to) = ev.relates_to.in_reply_to { - return Some(Relation::Reply { in_reply_to }); +impl<'de> Deserialize<'de> for Relation { + fn deserialize(deserializer: D) -> Result + where + D: Deserializer<'de>, + { + fn convert_relation(ev: EventWithRelatesToJsonRepr) -> Relation { + 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")] - 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) } - - EventWithRelatesToJsonRepr::deserialize(deserializer).map(convert_relation) } -pub fn serialize(relation: &Option, serializer: S) -> Result -where - S: Serializer, -{ - let relation = match relation { - Some(rel) => rel, - // FIXME: If this crate ends up depending on tracing, emit a warning here. - // This code path should not be reachable due to the skip_serializing_if serde attribute - // that should be applied together with `with = "relation_serde"`. - None => return serializer.serialize_struct("NoRelation", 0)?.end(), - }; - - let json_repr = match relation { - #[cfg(feature = "unstable-pre-spec")] - Relation::Annotation(r) => EventWithRelatesToJsonRepr::new(RelatesToJsonRepr { - relation: Some(RelationJsonRepr::Annotation(r.clone())), - ..Default::default() - }), - #[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 { +impl Serialize for Relation { + fn serialize(&self, serializer: S) -> Result + where + S: Serializer, + { + let relates_to = match self { + #[cfg(feature = "unstable-pre-spec")] + Relation::Annotation(r) => RelatesToJsonRepr { + relation: Some(RelationJsonRepr::Annotation(r.clone())), + ..Default::default() + }, + #[cfg(feature = "unstable-pre-spec")] + Relation::Reference(r) => RelatesToJsonRepr { + relation: Some(RelationJsonRepr::Reference(r.clone())), + ..Default::default() + }, + #[cfg(feature = "unstable-pre-spec")] + Relation::Replacement(r) => RelatesToJsonRepr { relation: Some(RelationJsonRepr::Replacement(r.clone())), ..Default::default() }, - }, - Relation::Reply { in_reply_to } => EventWithRelatesToJsonRepr::new(RelatesToJsonRepr { - in_reply_to: Some(in_reply_to.clone()), - ..Default::default() - }), - }; + Relation::Reply { in_reply_to } => { + RelatesToJsonRepr { 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)] diff --git a/crates/ruma-events/src/room/message.rs b/crates/ruma-events/src/room/message.rs index 9604aa76..b6cdac06 100644 --- a/crates/ruma-events/src/room/message.rs +++ b/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, } @@ -265,6 +265,7 @@ impl From 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. diff --git a/crates/ruma-events/src/room/message/content_serde.rs b/crates/ruma-events/src/room/message/content_serde.rs index 6aa8d9f4..2e5d0b28 100644 --- a/crates/ruma-events/src/room/message/content_serde.rs +++ b/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::{ - from_raw_json_value, - room::message::{MessageEventContent, MessageType}, -}; +use super::{MessageEventContent, MessageType, Relation}; +use crate::from_raw_json_value; impl<'de> Deserialize<'de> for MessageEventContent { fn deserialize(deserializer: D) -> Result @@ -15,10 +13,10 @@ impl<'de> Deserialize<'de> for MessageEventContent { { let json = Box::::deserialize(deserializer)?; let mut deserializer = serde_json::Deserializer::from_str(json.get()); - let relation = - super::relation_serde::deserialize(&mut deserializer).map_err(de::Error::custom)?; + let relates_to = + Option::::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 }) } } diff --git a/crates/ruma-events/src/room/message/relation_serde.rs b/crates/ruma-events/src/room/message/relation_serde.rs index a918a848..3e05381d 100644 --- a/crates/ruma-events/src/room/message/relation_serde.rs +++ b/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, D::Error> -where - D: Deserializer<'de>, -{ - fn convert_relation(ev: EventWithRelatesToJsonRepr) -> Option { +impl<'de> Deserialize<'de> for Relation { + fn deserialize(deserializer: D) -> Result + where + D: Deserializer<'de>, + { + 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 - // to the rel_type of the unknown relation. - RelationJsonRepr::Unknown => return None, - }; - - return Some(relation); + // 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, + }); } - None + Ok(Relation::_Custom) } - - EventWithRelatesToJsonRepr::deserialize(deserializer).map(convert_relation) } -pub fn serialize(relation: &Option, serializer: S) -> Result -where - S: Serializer, -{ - let relation = match relation { - Some(rel) => rel, - // FIXME: If this crate ends up depending on tracing, emit a warning here. - // This code path should not be reachable due to the skip_serializing_if serde attribute - // that should be applied together with `with = "relation_serde"`. - None => return serializer.serialize_struct("NoRelation", 0)?.end(), - }; - - let json_repr = match relation { - Relation::Reply { in_reply_to } => EventWithRelatesToJsonRepr::new(RelatesToJsonRepr { - in_reply_to: Some(in_reply_to.clone()), - ..Default::default() - }), - #[cfg(feature = "unstable-pre-spec")] - Relation::Replacement(Replacement { event_id, new_content }) => { - EventWithRelatesToJsonRepr { - relates_to: RelatesToJsonRepr { - relation: Some(RelationJsonRepr::Replacement(ReplacementJsonRepr { - event_id: event_id.clone(), - })), - ..Default::default() - }, - new_content: Some(new_content.clone()), +impl Serialize for Relation { + fn serialize(&self, serializer: S) -> Result + where + S: Serializer, + { + let json_repr = match self { + Relation::Reply { in_reply_to } => EventWithRelatesToJsonRepr::new(RelatesToJsonRepr { + in_reply_to: Some(in_reply_to.clone()), + ..Default::default() + }), + #[cfg(feature = "unstable-pre-spec")] + Relation::Replacement(Replacement { event_id, new_content }) => { + EventWithRelatesToJsonRepr { + 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,