From 4ec9f1aa035027a31ece175b908b56b4c40102b0 Mon Sep 17 00:00:00 2001 From: Jonas Platte Date: Fri, 17 Sep 2021 19:08:57 +0200 Subject: [PATCH] events: Require MessageType to always contain a body --- crates/ruma-events/CHANGELOG.md | 6 ++ crates/ruma-events/src/room/message.rs | 70 +++++++++++++++++------- crates/ruma-events/tests/room_message.rs | 6 +- 3 files changed, 60 insertions(+), 22 deletions(-) diff --git a/crates/ruma-events/CHANGELOG.md b/crates/ruma-events/CHANGELOG.md index 8bbf36fd..6ce3ec0a 100644 --- a/crates/ruma-events/CHANGELOG.md +++ b/crates/ruma-events/CHANGELOG.md @@ -5,6 +5,12 @@ Breaking changes: * Remove `RedactedStrippedStateEvent` * It was not used anywhere since stripped state events are never actually redacted. * Use `Box` instead of `JsonValue` for PDU `content` field +* Require `room::message::MessageType` to always contain a body + * The `new` constructor now also has a body parameter + +Improvements: + +* Add `room::message::MessageType::body` accessor method # 0.24.5 diff --git a/crates/ruma-events/src/room/message.rs b/crates/ruma-events/src/room/message.rs index 81f42813..c9b01362 100644 --- a/crates/ruma-events/src/room/message.rs +++ b/crates/ruma-events/src/room/message.rs @@ -187,24 +187,35 @@ impl MessageType { /// Prefer to use the public variants of `MessageType` where possible; this constructor is meant /// be used for unsupported message types only and does not allow setting arbitrary data for /// supported ones. - pub fn new(msgtype: &str, data: JsonObject) -> serde_json::Result { - fn from_json_object(obj: JsonObject) -> serde_json::Result { + /// + /// # Errors + /// + /// Returns an error if the `msgtype` is known and serialization of `data` to the corresponding + /// `MessageType` variant fails. + pub fn new(msgtype: &str, body: String, data: JsonObject) -> serde_json::Result { + fn deserialize_variant( + body: String, + mut obj: JsonObject, + ) -> serde_json::Result { + obj.insert("body".into(), body.into()); serde_json::from_value(JsonValue::Object(obj)) } Ok(match msgtype { - "m.audio" => Self::Audio(from_json_object(data)?), - "m.emote" => Self::Emote(from_json_object(data)?), - "m.file" => Self::File(from_json_object(data)?), - "m.image" => Self::Image(from_json_object(data)?), - "m.location" => Self::Location(from_json_object(data)?), - "m.notice" => Self::Notice(from_json_object(data)?), - "m.server_notice" => Self::ServerNotice(from_json_object(data)?), - "m.text" => Self::Text(from_json_object(data)?), - "m.video" => Self::Video(from_json_object(data)?), + "m.audio" => Self::Audio(deserialize_variant(body, data)?), + "m.emote" => Self::Emote(deserialize_variant(body, data)?), + "m.file" => Self::File(deserialize_variant(body, data)?), + "m.image" => Self::Image(deserialize_variant(body, data)?), + "m.location" => Self::Location(deserialize_variant(body, data)?), + "m.notice" => Self::Notice(deserialize_variant(body, data)?), + "m.server_notice" => Self::ServerNotice(deserialize_variant(body, data)?), + "m.text" => Self::Text(deserialize_variant(body, data)?), + "m.video" => Self::Video(deserialize_variant(body, data)?), #[cfg(feature = "unstable-pre-spec")] - "m.key.verification.request" => Self::VerificationRequest(from_json_object(data)?), - _ => Self::_Custom(CustomEventContent { msgtype: msgtype.to_owned(), data }), + "m.key.verification.request" => { + Self::VerificationRequest(deserialize_variant(body, data)?) + } + _ => Self::_Custom(CustomEventContent { msgtype: msgtype.to_owned(), body, data }), }) } @@ -226,6 +237,24 @@ impl MessageType { } } + /// Return a reference to the message body. + pub fn body(&self) -> &str { + match self { + MessageType::Audio(m) => &m.body, + MessageType::Emote(m) => &m.body, + MessageType::File(m) => &m.body, + MessageType::Image(m) => &m.body, + MessageType::Location(m) => &m.body, + MessageType::Notice(m) => &m.body, + MessageType::ServerNotice(m) => &m.body, + MessageType::Text(m) => &m.body, + MessageType::Video(m) => &m.body, + #[cfg(feature = "unstable-pre-spec")] + MessageType::VerificationRequest(m) => &m.body, + MessageType::_Custom(m) => &m.body, + } + } + /// Returns the associated data. /// /// Prefer to use the public variants of `MessageType` where possible; this method is meant to @@ -929,10 +958,13 @@ impl KeyVerificationRequestEventContent { #[doc(hidden)] #[derive(Clone, Debug, Deserialize, Serialize)] pub struct CustomEventContent { - /// A custom msgtype + /// A custom msgtype. msgtype: String, - /// Remaining event content + /// The message body. + body: String, + + /// Remaining event content. #[serde(flatten)] data: JsonObject, } @@ -967,11 +999,7 @@ fn get_plain_quote_fallback(original_message: &MessageEvent) -> String { format!("> <{:?}> sent a video.", original_message.sender) } MessageType::_Custom(content) => { - format!( - "> <{:?}> {}", - original_message.sender, - content.data["body"].as_str().unwrap_or(""), - ) + format!("> <{:?}> {}", original_message.sender, content.body) } #[cfg(feature = "unstable-pre-spec")] MessageType::VerificationRequest(content) => { @@ -1156,7 +1184,7 @@ fn get_html_quote_fallback(original_message: &MessageEvent) -> String { room_id = original_message.room_id, event_id = original_message.event_id, sender = original_message.sender, - body = content.data["body"].as_str().unwrap_or(""), + body = content.body, ) } #[cfg(feature = "unstable-pre-spec")] diff --git a/crates/ruma-events/tests/room_message.rs b/crates/ruma-events/tests/room_message.rs index bfd35309..d97b045f 100644 --- a/crates/ruma-events/tests/room_message.rs +++ b/crates/ruma-events/tests/room_message.rs @@ -90,12 +90,14 @@ fn custom_msgtype_serialization() { "custom_field".into() => json!("baba"), "another_one".into() => json!("abab"), }; - let custom_msgtype = MessageType::new("my_custom_msgtype", json_data).unwrap(); + let custom_msgtype = + MessageType::new("my_custom_msgtype", "my message body".into(), json_data).unwrap(); assert_eq!( to_json_value(&custom_msgtype).unwrap(), json!({ "msgtype": "my_custom_msgtype", + "body": "my message body", "custom_field": "baba", "another_one": "abab", }) @@ -106,6 +108,7 @@ fn custom_msgtype_serialization() { fn custom_content_deserialization() { let json_data = json!({ "msgtype": "my_custom_msgtype", + "body": "my custom message", "custom_field": "baba", "another_one": "abab", }); @@ -119,6 +122,7 @@ fn custom_content_deserialization() { from_json_value::>(json_data).unwrap().deserialize().unwrap(); assert_eq!(custom_event.msgtype(), "my_custom_msgtype"); + assert_eq!(custom_event.body(), "my custom message"); assert_eq!(custom_event.data(), Cow::Owned(expected_json_data)); }