Merge pull request #25 from florianjacob/null-absent-empty

Properly handle CanonicalAliasEvent and NameEvent content being absent, null or empty

src/lib.rs

@@ -104,7 +104,7 @@ use std::fmt::{Debug, Display, Error as FmtError, Formatter, Result as FmtResult
 
 use ruma_identifiers::{EventId, RoomId, UserId};
 use serde::{
-    de::{Error as SerdeError, Visitor},
+    de::{Error as SerdeError, IntoDeserializer, Visitor},
     Deserialize, Deserializer, Serialize, Serializer,
 };
 use serde_json::Value;
@@ -346,6 +346,31 @@ impl<'de> Deserialize<'de> for EventType {
     }
 }
 
+/// Serde deserialization decorator to map empty Strings to None,
+/// and forward non-empty Strings to the Deserialize implementation for T.
+/// Useful for the typical
+/// "A room with an X event with an absent, null, or empty Y field
+/// should be treated the same as a room with no such event."
+/// formulation in the spec.
+///
+/// To be used like this:
+/// `#[serde(deserialize_with = "empty_string_as_none"]`
+/// Relevant serde issue: https://github.com/serde-rs/serde/issues/1425
+fn empty_string_as_none<'de, D, T>(de: D) -> Result<Option<T>, D::Error>
+where
+    D: serde::Deserializer<'de>,
+    T: serde::Deserialize<'de>,
+{
+    let opt = Option::<String>::deserialize(de)?;
+    let opt = opt.as_ref().map(String::as_str);
+    match opt {
+        None | Some("") => Ok(None),
+        // If T = String, like in m.room.name, the second deserialize is actually superfluous.
+        // TODO: optimize that somehow?
+        Some(s) => T::deserialize(s.into_deserializer()).map(Some),
+    }
+}
+
 #[cfg(test)]
 mod tests {
     use serde_json::{from_str, to_string};

src/room/canonical_alias.rs

@@ -1,5 +1,6 @@
 //! Types for the *m.room.canonical_alias* event.
 
+use crate::empty_string_as_none;
 use ruma_identifiers::RoomAliasId;
 use serde::{Deserialize, Serialize};
 
@@ -12,5 +13,63 @@ state_event! {
 #[derive(Clone, Debug, Deserialize, Serialize)]
 pub struct CanonicalAliasEventContent {
     /// The canonical alias.
-    pub alias: RoomAliasId,
+    /// Rooms with `alias: None` should be treated the same as a room with no canonical alias.
+    // The spec says “A room with an m.room.canonical_alias event with an absent, null, or empty alias field
+    // should be treated the same as a room with no m.room.canonical_alias event.”.
+    // Serde maps null fields to None by default, serde(default) maps an absent field to None,
+    // and empty_string_as_none does exactly that, preventing empty strings getting parsed as RoomAliasId.
+    #[serde(default)]
+    #[serde(deserialize_with = "empty_string_as_none")]
+    pub alias: Option<RoomAliasId>,
+}
+
+#[cfg(test)]
+mod tests {
+    use serde_json::from_str;
+
+    use super::CanonicalAliasEventContent;
+    use ruma_identifiers::RoomAliasId;
+    use std::convert::TryFrom;
+
+    #[test]
+    fn absent_field_as_none() {
+        assert_eq!(
+            from_str::<CanonicalAliasEventContent>(r#"{}"#)
+                .unwrap()
+                .alias,
+            None
+        );
+    }
+
+    #[test]
+    fn null_field_as_none() {
+        assert_eq!(
+            from_str::<CanonicalAliasEventContent>(r#"{"alias":null}"#)
+                .unwrap()
+                .alias,
+            None
+        );
+    }
+
+    #[test]
+    fn empty_field_as_none() {
+        assert_eq!(
+            from_str::<CanonicalAliasEventContent>(r#"{"alias":""}"#)
+                .unwrap()
+                .alias,
+            None
+        );
+    }
+
+    #[test]
+    fn nonempty_field_as_some() {
+        let alias = Some(RoomAliasId::try_from("#somewhere:localhost").unwrap());
+
+        assert_eq!(
+            from_str::<CanonicalAliasEventContent>(r##"{"alias":"#somewhere:localhost"}"##)
+                .unwrap()
+                .alias,
+            alias
+        );
+    }
 }

src/room/name.rs

@@ -1,5 +1,6 @@
 //! Types for the *m.room.name* event.
 
+use crate::empty_string_as_none;
 use serde::{Deserialize, Serialize};
 
 state_event! {
@@ -11,5 +12,53 @@ state_event! {
 #[derive(Clone, Debug, Deserialize, Serialize)]
 pub struct NameEventContent {
     /// The name of the room. This MUST NOT exceed 255 bytes.
-    pub name: String,
+    /// Rooms with `name: None` should be treated the same as a room with no name.
+    // The spec says “A room with an m.room.name event with an absent, null, or empty name field
+    // should be treated the same as a room with no m.room.name event.”.
+    // Serde maps null fields to None by default, serde(default) maps an absent field to None,
+    // and empty_string_as_none completes the handling.
+    #[serde(default)]
+    #[serde(deserialize_with = "empty_string_as_none")]
+    pub name: Option<String>,
+}
+
+#[cfg(test)]
+mod tests {
+    use super::NameEventContent;
+    use serde_json::from_str;
+
+    #[test]
+    fn absent_field_as_none() {
+        assert_eq!(from_str::<NameEventContent>(r#"{}"#).unwrap().name, None);
+    }
+
+    #[test]
+    fn null_field_as_none() {
+        assert_eq!(
+            from_str::<NameEventContent>(r#"{"name":null}"#)
+                .unwrap()
+                .name,
+            None
+        );
+    }
+
+    #[test]
+    fn empty_field_as_none() {
+        assert_eq!(
+            from_str::<NameEventContent>(r#"{"name":""}"#).unwrap().name,
+            None
+        );
+    }
+
+    #[test]
+    fn nonempty_field_as_some() {
+        let name = Some("The room name".to_string());
+
+        assert_eq!(
+            from_str::<NameEventContent>(r##"{"name":"The room name"}"##)
+                .unwrap()
+                .name,
+            name
+        );
+    }
 }

src/stripped.rs

@@ -333,7 +333,7 @@ mod tests {
 
         match from_str::<StrippedState>(name_event).unwrap() {
             StrippedState::RoomName(event) => {
-                assert_eq!(event.content.name, "Ruma");
+                assert_eq!(event.content.name, Some("Ruma".to_string()));
                 assert_eq!(event.event_type, EventType::RoomName);
                 assert_eq!(event.state_key, "");
                 assert_eq!(event.sender.to_string(), "@example:localhost");