From 33212862427094fb1a0c08e0bdfb322ee404ba0a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?K=C3=A9vin=20Commaille?= Date: Sun, 1 Dec 2024 13:00:50 +0100 Subject: [PATCH] events: Do not generate reply fallbacks anymore According to MSC2781. The helper methods for the relevant relations have been simplified to only take the necessary data. --- crates/ruma-events/CHANGELOG.md | 21 + crates/ruma-events/src/doc/rich_reply.md | 18 - crates/ruma-events/src/room/message.rs | 185 +++----- crates/ruma-events/src/room/message/reply.rs | 179 ------- .../src/room/message/without_relation.rs | 207 ++------- crates/ruma-events/tests/it/room_message.rs | 437 +----------------- 6 files changed, 139 insertions(+), 908 deletions(-) delete mode 100644 crates/ruma-events/src/doc/rich_reply.md delete mode 100644 crates/ruma-events/src/room/message/reply.rs diff --git a/crates/ruma-events/CHANGELOG.md b/crates/ruma-events/CHANGELOG.md index f3fc8533..405df9c2 100644 --- a/crates/ruma-events/CHANGELOG.md +++ b/crates/ruma-events/CHANGELOG.md @@ -1,5 +1,26 @@ # [unreleased] + +- Reply fallbacks are not generated anymore, according to MSC2781 / Matrix 1.13. + As a result, the following methods of `RoomMessageEventContent(WithoutRelation)` + were simplified: + - `make_reply_to` and `make_reply_to_raw` have been merged into + `make_reply_to`. It takes a `ReplyMetadata`, that can be virtually + constructed from any event and includes built-in conversions for room + message events. + - `make_for_thread` also takes a `ReplyMetadata` instead of a room message + event. + - `make_replacement` does not take the replied-to message anymore. + +# 0.30.0 + +Breaking changes: + +- Take newly introduced `DirectUserIdentifier(str)` as a key for `DirectEventContent`. + This change allows to have an email or MSISDN phone number as a key for example, + which can be used when issuing invites through third-party systems. + `DirectUserIdentifier` can easily be converted to an `UserId`. + Improvements: - Add unstable support for MSC4171 for the m.member_hints state event. diff --git a/crates/ruma-events/src/doc/rich_reply.md b/crates/ruma-events/src/doc/rich_reply.md deleted file mode 100644 index 25111cc5..00000000 --- a/crates/ruma-events/src/doc/rich_reply.md +++ /dev/null @@ -1,18 +0,0 @@ - -This function requires an [`OriginalRoomMessageEvent`] since it creates a permalink to -the previous message, for which the room ID is required. If you want to reply to an -[`OriginalSyncRoomMessageEvent`], you have to convert it first by calling [`.into_full_event()`]. - -If the message was edited, the previous message should be the original message that was edited, -with the content of its replacement, to allow the fallback to be accurate at the time it is added. - -It is recommended to enable the `html` feature when using this method as this will -clean up nested [rich reply fallbacks] in chains of replies. This uses [`sanitize_html()`] -internally, with [`RemoveReplyFallback::Yes`]. - -[`OriginalRoomMessageEvent`]: crate::room::message::OriginalRoomMessageEvent -[`OriginalSyncRoomMessageEvent`]: crate::room::message::OriginalSyncRoomMessageEvent -[`.into_full_event()`]: crate::OriginalSyncMessageLikeEvent::into_full_event -[rich reply fallbacks]: https://spec.matrix.org/latest/client-server-api/#fallbacks-for-rich-replies -[`sanitize_html()`]: ruma_html::sanitize_html -[`RemoveReplyFallback::Yes`]: ruma_html::RemoveReplyFallback::Yes diff --git a/crates/ruma-events/src/room/message.rs b/crates/ruma-events/src/room/message.rs index 776d4062..432ad57d 100644 --- a/crates/ruma-events/src/room/message.rs +++ b/crates/ruma-events/src/room/message.rs @@ -4,9 +4,10 @@ use std::borrow::Cow; +use as_variant::as_variant; use ruma_common::{ - serde::{JsonObject, Raw, StringEnum}, - OwnedEventId, RoomId, + serde::{JsonObject, StringEnum}, + EventId, OwnedEventId, UserId, }; #[cfg(feature = "html")] use ruma_html::{sanitize_html, HtmlSanitizerMode, RemoveReplyFallback}; @@ -15,12 +16,11 @@ use serde::{de::DeserializeOwned, Deserialize, Serialize}; use serde_json::Value as JsonValue; use tracing::warn; -use self::reply::OriginalEventData; #[cfg(feature = "html")] use self::sanitize::remove_plain_reply_fallback; use crate::{ relation::{InReplyTo, Replacement, Thread}, - AnySyncTimelineEvent, Mentions, PrivOwnedStr, + Mentions, PrivOwnedStr, }; mod audio; @@ -34,7 +34,6 @@ mod media_caption; mod notice; mod relation; pub(crate) mod relation_serde; -mod reply; pub mod sanitize; mod server_notice; mod text; @@ -151,84 +150,45 @@ impl RoomMessageEventContent { Self::new(MessageType::emote_markdown(body)) } - /// Turns `self` into a reply to the given message. + /// Turns `self` into a [rich reply] to the message using the given metadata. /// - /// Takes the `body` / `formatted_body` (if any) in `self` for the main text and prepends a - /// quoted version of `original_message`. Also sets the `in_reply_to` field inside `relates_to`, - /// and optionally the `rel_type` to `m.thread` if the `original_message is in a thread and - /// thread forwarding is enabled. - #[doc = include_str!(concat!(env!("CARGO_MANIFEST_DIR"), "/src/doc/rich_reply.md"))] + /// Sets the `in_reply_to` field inside `relates_to`, and optionally the `rel_type` to + /// `m.thread` if the metadata has a `thread` and `ForwardThread::Yes` is used. /// - /// # Panics + /// If `AddMentions::Yes` is used, the `sender` in the metadata is added as a user mention. /// - /// Panics if `self` has a `formatted_body` with a format other than HTML. + /// [rich reply]: https://spec.matrix.org/latest/client-server-api/#rich-replies #[track_caller] - pub fn make_reply_to( + pub fn make_reply_to<'a>( self, - original_message: &OriginalRoomMessageEvent, + metadata: impl Into>, forward_thread: ForwardThread, add_mentions: AddMentions, ) -> Self { - self.without_relation().make_reply_to(original_message, forward_thread, add_mentions) + self.without_relation().make_reply_to(metadata, forward_thread, add_mentions) } - /// Turns `self` into a reply to the given raw event. + /// Turns `self` into a new message for a [thread], that is optionally a reply. /// - /// Takes the `body` / `formatted_body` (if any) in `self` for the main text and prepends a - /// quoted version of the `body` of `original_event` (if any). Also sets the `in_reply_to` field - /// inside `relates_to`, and optionally the `rel_type` to `m.thread` if the - /// `original_message is in a thread and thread forwarding is enabled. + /// Looks for the `thread` in the given metadata. If it exists, this message will be in the same + /// thread. If it doesn't, a new thread is created with the `event_id` in the metadata as the + /// root. /// - /// It is recommended to use [`Self::make_reply_to()`] for replies to `m.room.message` events, - /// as the generated fallback is better for some `msgtype`s. + /// It also sets the `in_reply_to` field inside `relates_to` to point the `event_id` + /// in the metadata. If `ReplyWithinThread::Yes` is used, the metadata should be constructed + /// from the event to make a reply to, otherwise it should be constructed from the latest + /// event in the thread. /// - /// Note that except for the panic below, this is infallible. Which means that if a field is - /// missing when deserializing the data, the changes that require it will not be applied. It - /// will still at least apply the `m.in_reply_to` relation to this content. + /// If `AddMentions::Yes` is used, the `sender` in the metadata is added as a user mention. /// - /// # Panics - /// - /// Panics if `self` has a `formatted_body` with a format other than HTML. - #[track_caller] - pub fn make_reply_to_raw( + /// [thread]: https://spec.matrix.org/latest/client-server-api/#threading + pub fn make_for_thread<'a>( self, - original_event: &Raw, - original_event_id: OwnedEventId, - room_id: &RoomId, - forward_thread: ForwardThread, - add_mentions: AddMentions, - ) -> Self { - self.without_relation().make_reply_to_raw( - original_event, - original_event_id, - room_id, - forward_thread, - add_mentions, - ) - } - - /// Turns `self` into a new message for a thread, that is optionally a reply. - /// - /// Looks for a [`Relation::Thread`] in `previous_message`. If it exists, this message will be - /// in the same thread. If it doesn't, a new thread with `previous_message` as the root is - /// created. - /// - /// If this is a reply within the thread, takes the `body` / `formatted_body` (if any) in `self` - /// for the main text and prepends a quoted version of `previous_message`. Also sets the - /// `in_reply_to` field inside `relates_to`. - #[doc = include_str!(concat!(env!("CARGO_MANIFEST_DIR"), "/src/doc/rich_reply.md"))] - /// - /// # Panics - /// - /// Panics if this is a reply within the thread and `self` has a `formatted_body` with a format - /// other than HTML. - pub fn make_for_thread( - self, - previous_message: &OriginalRoomMessageEvent, + metadata: impl Into>, is_reply: ReplyWithinThread, add_mentions: AddMentions, ) -> Self { - self.without_relation().make_for_thread(previous_message, is_reply, add_mentions) + self.without_relation().make_for_thread(metadata, is_reply, add_mentions) } /// Turns `self` into a [replacement] (or edit) for a given message. @@ -240,12 +200,6 @@ impl RoomMessageEventContent { /// This takes the content and sets it in `m.new_content`, and modifies the `content` to include /// a fallback. /// - /// If the message that is replaced is a reply to another message, the latter should also be - /// provided to be able to generate a rich reply fallback that takes the `body` / - /// `formatted_body` (if any) in `self` for the main text and prepends a quoted version of - /// `original_message`. - #[doc = include_str!(concat!(env!("CARGO_MANIFEST_DIR"), "/src/doc/rich_reply.md"))] - /// /// If this message contains [`Mentions`], they are copied into `m.new_content` to keep the same /// mentions, but the ones in `content` are filtered with the ones in the /// [`ReplacementMetadata`] so only new mentions will trigger a notification. @@ -256,12 +210,8 @@ impl RoomMessageEventContent { /// /// [replacement]: https://spec.matrix.org/latest/client-server-api/#event-replacements #[track_caller] - pub fn make_replacement( - self, - metadata: impl Into, - replied_to_message: Option<&OriginalRoomMessageEvent>, - ) -> Self { - self.without_relation().make_replacement(metadata, replied_to_message) + pub fn make_replacement(self, metadata: impl Into) -> Self { + self.without_relation().make_replacement(metadata) } /// Set the [mentions] of this event. @@ -375,6 +325,11 @@ impl RoomMessageEventContent { self.into() } + + /// Get the thread relation from this content, if any. + fn thread(&self) -> Option<&Thread> { + self.relates_to.as_ref().and_then(|relates_to| as_variant!(relates_to, Relation::Thread)) + } } /// Whether or not to forward a [`Relation::Thread`] when sending a reply. @@ -661,49 +616,6 @@ impl MessageType { } } - #[track_caller] - fn add_reply_fallback(&mut self, original_event: OriginalEventData<'_>) { - let empty_formatted_body = || FormattedBody::html(String::new()); - - let (body, formatted) = { - match self { - MessageType::Emote(m) => { - (&mut m.body, Some(m.formatted.get_or_insert_with(empty_formatted_body))) - } - MessageType::Notice(m) => { - (&mut m.body, Some(m.formatted.get_or_insert_with(empty_formatted_body))) - } - MessageType::Text(m) => { - (&mut m.body, Some(m.formatted.get_or_insert_with(empty_formatted_body))) - } - MessageType::Audio(m) => (&mut m.body, None), - MessageType::File(m) => (&mut m.body, None), - MessageType::Image(m) => (&mut m.body, None), - MessageType::Location(m) => (&mut m.body, None), - MessageType::ServerNotice(m) => (&mut m.body, None), - MessageType::Video(m) => (&mut m.body, None), - MessageType::VerificationRequest(m) => (&mut m.body, None), - MessageType::_Custom(m) => (&mut m.body, None), - } - }; - - if let Some(f) = formatted { - assert_eq!( - f.format, - MessageFormat::Html, - "can't add reply fallback to non-HTML formatted messages" - ); - - let formatted_body = &mut f.body; - - (*body, *formatted_body) = reply::plain_and_formatted_reply_body( - body.as_str(), - (!formatted_body.is_empty()).then_some(formatted_body.as_str()), - original_event, - ); - } - } - fn make_replacement_body(&mut self) { let empty_formatted_body = || FormattedBody::html(String::new()); @@ -784,6 +696,39 @@ impl From<&OriginalSyncRoomMessageEvent> for ReplacementMetadata { } } +/// Metadata about an event to reply to or to add to a thread. +/// +/// To be used with [`RoomMessageEventContent::make_reply_to`] or +/// [`RoomMessageEventContent::make_for_thread`]. +#[derive(Clone, Copy, Debug)] +pub struct ReplyMetadata<'a> { + /// The event ID of the event to reply to. + event_id: &'a EventId, + /// The sender of the event to reply to. + sender: &'a UserId, + /// The `m.thread` relation of the event to reply to, if any. + thread: Option<&'a Thread>, +} + +impl<'a> ReplyMetadata<'a> { + /// Creates a new `ReplyMetadata` with the given event ID, sender and thread relation. + pub fn new(event_id: &'a EventId, sender: &'a UserId, thread: Option<&'a Thread>) -> Self { + Self { event_id, sender, thread } + } +} + +impl<'a> From<&'a OriginalRoomMessageEvent> for ReplyMetadata<'a> { + fn from(value: &'a OriginalRoomMessageEvent) -> Self { + ReplyMetadata::new(&value.event_id, &value.sender, value.content.thread()) + } +} + +impl<'a> From<&'a OriginalSyncRoomMessageEvent> for ReplyMetadata<'a> { + fn from(value: &'a OriginalSyncRoomMessageEvent) -> Self { + ReplyMetadata::new(&value.event_id, &value.sender, value.content.thread()) + } +} + /// The format for the formatted representation of a message body. #[doc = include_str!(concat!(env!("CARGO_MANIFEST_DIR"), "/src/doc/string_enum.md"))] #[derive(Clone, PartialEq, Eq, StringEnum)] diff --git a/crates/ruma-events/src/room/message/reply.rs b/crates/ruma-events/src/room/message/reply.rs deleted file mode 100644 index e9ec0dcc..00000000 --- a/crates/ruma-events/src/room/message/reply.rs +++ /dev/null @@ -1,179 +0,0 @@ -use std::fmt::{self, Write}; - -use ruma_common::{EventId, RoomId, UserId}; -#[cfg(feature = "html")] -use ruma_html::Html; - -use super::{ - sanitize::remove_plain_reply_fallback, FormattedBody, MessageType, OriginalRoomMessageEvent, - Relation, -}; - -pub(super) struct OriginalEventData<'a> { - pub(super) body: &'a str, - pub(super) formatted: Option<&'a FormattedBody>, - pub(super) is_emote: bool, - pub(super) is_reply: bool, - pub(super) room_id: &'a RoomId, - pub(super) event_id: &'a EventId, - pub(super) sender: &'a UserId, -} - -impl<'a> From<&'a OriginalRoomMessageEvent> for OriginalEventData<'a> { - fn from(message: &'a OriginalRoomMessageEvent) -> Self { - let OriginalRoomMessageEvent { room_id, event_id, sender, content, .. } = message; - let is_reply = matches!(content.relates_to, Some(Relation::Reply { .. })); - - let (body, formatted, is_emote) = match &content.msgtype { - MessageType::Audio(_) => ("sent an audio file.", None, false), - MessageType::Emote(c) => (&*c.body, c.formatted.as_ref(), true), - MessageType::File(_) => ("sent a file.", None, false), - MessageType::Image(_) => ("sent an image.", None, false), - MessageType::Location(_) => ("sent a location.", None, false), - MessageType::Notice(c) => (&*c.body, c.formatted.as_ref(), false), - MessageType::ServerNotice(c) => (&*c.body, None, false), - MessageType::Text(c) => (&*c.body, c.formatted.as_ref(), false), - MessageType::Video(_) => ("sent a video.", None, false), - MessageType::VerificationRequest(c) => (&*c.body, None, false), - MessageType::_Custom(c) => (&*c.body, None, false), - }; - - Self { body, formatted, is_emote, is_reply, room_id, event_id, sender } - } -} - -fn get_message_quote_fallbacks(original_event: OriginalEventData<'_>) -> (String, String) { - let OriginalEventData { body, formatted, is_emote, is_reply, room_id, event_id, sender } = - original_event; - let emote_sign = is_emote.then_some("* ").unwrap_or_default(); - let body = is_reply.then(|| remove_plain_reply_fallback(body)).unwrap_or(body); - #[cfg(feature = "html")] - let html_body = FormattedOrPlainBody { formatted, body, is_reply }; - #[cfg(not(feature = "html"))] - let html_body = FormattedOrPlainBody { formatted, body }; - - ( - format!("> {emote_sign}<{sender}> {body}").replace('\n', "\n> "), - format!( - "\ -
\ - In reply to \ - {emote_sign}{sender}\ -
\ - {html_body}\ -
\ - " - ), - ) -} - -struct EscapeHtmlEntities<'a>(&'a str); - -impl fmt::Display for EscapeHtmlEntities<'_> { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - for c in self.0.chars() { - // Escape reserved HTML entities and new lines. - // - match c { - '&' => f.write_str("&")?, - '<' => f.write_str("<")?, - '>' => f.write_str(">")?, - '"' => f.write_str(""")?, - '\n' => f.write_str("
")?, - _ => f.write_char(c)?, - } - } - - Ok(()) - } -} - -struct FormattedOrPlainBody<'a> { - formatted: Option<&'a FormattedBody>, - body: &'a str, - #[cfg(feature = "html")] - is_reply: bool, -} - -impl fmt::Display for FormattedOrPlainBody<'_> { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - if let Some(formatted_body) = self.formatted { - #[cfg(feature = "html")] - if self.is_reply { - let html = Html::parse(&formatted_body.body); - html.sanitize(); - - write!(f, "{html}") - } else { - f.write_str(&formatted_body.body) - } - - #[cfg(not(feature = "html"))] - f.write_str(&formatted_body.body) - } else { - write!(f, "{}", EscapeHtmlEntities(self.body)) - } - } -} - -/// Get the plain and formatted body for a rich reply. -/// -/// Returns a `(plain, html)` tuple. -/// -/// With the `sanitize` feature, [HTML tags and attributes] that are not allowed in the Matrix -/// spec and previous [rich reply fallbacks] are removed from the previous message in the new rich -/// reply fallback. -/// -/// [HTML tags and attributes]: https://spec.matrix.org/latest/client-server-api/#mroommessage-msgtypes -/// [rich reply fallbacks]: https://spec.matrix.org/latest/client-server-api/#fallbacks-for-rich-replies -pub(super) fn plain_and_formatted_reply_body( - body: &str, - formatted: Option, - original_event: OriginalEventData<'_>, -) -> (String, String) { - let (quoted, quoted_html) = get_message_quote_fallbacks(original_event); - - let plain = format!("{quoted}\n\n{body}"); - let html = match formatted { - Some(formatted) => format!("{quoted_html}{formatted}"), - None => format!("{quoted_html}{}", EscapeHtmlEntities(body)), - }; - - (plain, html) -} - -#[cfg(test)] -mod tests { - use ruma_common::{owned_event_id, owned_room_id, owned_user_id, MilliSecondsSinceUnixEpoch}; - - use super::OriginalRoomMessageEvent; - use crate::{room::message::RoomMessageEventContent, MessageLikeUnsigned}; - - #[test] - fn fallback_multiline() { - let (plain_quote, html_quote) = super::get_message_quote_fallbacks( - (&OriginalRoomMessageEvent { - content: RoomMessageEventContent::text_plain("multi\nline"), - event_id: owned_event_id!("$1598361704261elfgc:localhost"), - sender: owned_user_id!("@alice:example.com"), - origin_server_ts: MilliSecondsSinceUnixEpoch::now(), - room_id: owned_room_id!("!n8f893n9:example.com"), - unsigned: MessageLikeUnsigned::new(), - }) - .into(), - ); - - assert_eq!(plain_quote, "> <@alice:example.com> multi\n> line"); - assert_eq!( - html_quote, - "\ -
\ - In reply to \ - @alice:example.com\ -
\ - multi
line\ -
\ - ", - ); - } -} diff --git a/crates/ruma-events/src/room/message/without_relation.rs b/crates/ruma-events/src/room/message/without_relation.rs index 3111c353..042679c1 100644 --- a/crates/ruma-events/src/room/message/without_relation.rs +++ b/crates/ruma-events/src/room/message/without_relation.rs @@ -1,15 +1,12 @@ -use as_variant::as_variant; -use ruma_common::{serde::Raw, OwnedEventId, OwnedUserId, RoomId, UserId}; -use serde::{Deserialize, Serialize}; +use serde::Serialize; use super::{ - AddMentions, ForwardThread, MessageType, OriginalRoomMessageEvent, Relation, - ReplacementMetadata, ReplyWithinThread, RoomMessageEventContent, + AddMentions, ForwardThread, MessageType, Relation, ReplacementMetadata, ReplyMetadata, + ReplyWithinThread, RoomMessageEventContent, }; use crate::{ relation::{InReplyTo, Replacement, Thread}, - room::message::{reply::OriginalEventData, FormattedBody}, - AnySyncTimelineEvent, Mentions, + Mentions, }; /// Form of [`RoomMessageEventContent`] without relation. @@ -92,161 +89,81 @@ impl RoomMessageEventContentWithoutRelation { RoomMessageEventContent { msgtype, relates_to, mentions } } - /// Turns `self` into a reply to the given message. + /// Turns `self` into a [rich reply] to the message using the given metadata. /// - /// Takes the `body` / `formatted_body` (if any) in `self` for the main text and prepends a - /// quoted version of `original_message`. Also sets the `in_reply_to` field inside `relates_to`, - /// and optionally the `rel_type` to `m.thread` if the `original_message is in a thread and - /// thread forwarding is enabled. - #[doc = include_str!(concat!(env!("CARGO_MANIFEST_DIR"), "/src/doc/rich_reply.md"))] + /// Sets the `in_reply_to` field inside `relates_to`, and optionally the `rel_type` to + /// `m.thread` if the metadata has a `thread` and `ForwardThread::Yes` is used. /// - /// # Panics + /// If `AddMentions::Yes` is used, the `sender` in the metadata is added as a user mention. /// - /// Panics if `self` has a `formatted_body` with a format other than HTML. + /// [rich reply]: https://spec.matrix.org/latest/client-server-api/#rich-replies #[track_caller] - pub fn make_reply_to( + pub fn make_reply_to<'a>( mut self, - original_message: &OriginalRoomMessageEvent, + metadata: impl Into>, forward_thread: ForwardThread, add_mentions: AddMentions, ) -> RoomMessageEventContent { - self.msgtype.add_reply_fallback(original_message.into()); - let original_event_id = original_message.event_id.clone(); + let metadata = metadata.into(); + let original_event_id = metadata.event_id.to_owned(); - let original_thread_id = if forward_thread == ForwardThread::Yes { - original_message - .content - .relates_to - .as_ref() - .and_then(as_variant!(Relation::Thread)) - .map(|thread| thread.event_id.clone()) + let original_thread_id = metadata + .thread + .filter(|_| forward_thread == ForwardThread::Yes) + .map(|thread| thread.event_id.clone()); + let relates_to = if let Some(event_id) = original_thread_id { + Relation::Thread(Thread::plain(event_id.to_owned(), original_event_id.to_owned())) } else { - None + Relation::Reply { in_reply_to: InReplyTo { event_id: original_event_id.to_owned() } } }; - let sender_for_mentions = - (add_mentions == AddMentions::Yes).then_some(&*original_message.sender); - - self.make_reply_tweaks(original_event_id, original_thread_id, sender_for_mentions) - } - - /// Turns `self` into a reply to the given raw event. - /// - /// Takes the `body` / `formatted_body` (if any) in `self` for the main text and prepends a - /// quoted version of the `body` of `original_event` (if any). Also sets the `in_reply_to` field - /// inside `relates_to`, and optionally the `rel_type` to `m.thread` if the - /// `original_message is in a thread and thread forwarding is enabled. - /// - /// It is recommended to use [`Self::make_reply_to()`] for replies to `m.room.message` events, - /// as the generated fallback is better for some `msgtype`s. - /// - /// Note that except for the panic below, this is infallible. Which means that if a field is - /// missing when deserializing the data, the changes that require it will not be applied. It - /// will still at least apply the `m.in_reply_to` relation to this content. - /// - /// # Panics - /// - /// Panics if `self` has a `formatted_body` with a format other than HTML. - #[track_caller] - pub fn make_reply_to_raw( - mut self, - original_event: &Raw, - original_event_id: OwnedEventId, - room_id: &RoomId, - forward_thread: ForwardThread, - add_mentions: AddMentions, - ) -> RoomMessageEventContent { - #[derive(Deserialize)] - struct ContentDeHelper { - body: Option, - #[serde(flatten)] - formatted: Option, - #[cfg(feature = "unstable-msc1767")] - #[serde(rename = "org.matrix.msc1767.text")] - text: Option, - #[serde(rename = "m.relates_to")] - relates_to: Option, + if add_mentions == AddMentions::Yes { + self.mentions + .get_or_insert_with(Mentions::new) + .user_ids + .insert(metadata.sender.to_owned()); } - let sender = original_event.get_field::("sender").ok().flatten(); - let content = original_event.get_field::("content").ok().flatten(); - let relates_to = content.as_ref().and_then(|c| c.relates_to.as_ref()); - - let content_body = content.as_ref().and_then(|c| { - let body = c.body.as_deref(); - #[cfg(feature = "unstable-msc1767")] - let body = body.or(c.text.as_deref()); - - Some((c, body?)) - }); - - // Only apply fallback if we managed to deserialize raw event. - if let (Some(sender), Some((content, body))) = (&sender, content_body) { - let is_reply = - matches!(content.relates_to, Some(crate::room::encrypted::Relation::Reply { .. })); - let data = OriginalEventData { - body, - formatted: content.formatted.as_ref(), - is_emote: false, - is_reply, - room_id, - event_id: &original_event_id, - sender, - }; - - self.msgtype.add_reply_fallback(data); - } - - let original_thread_id = if forward_thread == ForwardThread::Yes { - relates_to - .and_then(as_variant!(crate::room::encrypted::Relation::Thread)) - .map(|thread| thread.event_id.clone()) - } else { - None - }; - - let sender_for_mentions = sender.as_deref().filter(|_| add_mentions == AddMentions::Yes); - self.make_reply_tweaks(original_event_id, original_thread_id, sender_for_mentions) + self.with_relation(Some(relates_to)) } - /// Turns `self` into a new message for a thread, that is optionally a reply. + /// Turns `self` into a new message for a [thread], that is optionally a reply. /// - /// Looks for a [`Relation::Thread`] in `previous_message`. If it exists, this message will be - /// in the same thread. If it doesn't, a new thread with `previous_message` as the root is - /// created. + /// Looks for the `thread` in the given metadata. If it exists, this message will be in the same + /// thread. If it doesn't, a new thread is created with the `event_id` in the metadata as the + /// root. /// - /// If this is a reply within the thread, takes the `body` / `formatted_body` (if any) in `self` - /// for the main text and prepends a quoted version of `previous_message`. Also sets the - /// `in_reply_to` field inside `relates_to`. - #[doc = include_str!(concat!(env!("CARGO_MANIFEST_DIR"), "/src/doc/rich_reply.md"))] + /// It also sets the `in_reply_to` field inside `relates_to` to point the `event_id` + /// in the metadata. If `ReplyWithinThread::Yes` is used, the metadata should be constructed + /// from the event to make a reply to, otherwise it should be constructed from the latest + /// event in the thread. /// - /// # Panics + /// If `AddMentions::Yes` is used, the `sender` in the metadata is added as a user mention. /// - /// Panics if this is a reply within the thread and `self` has a `formatted_body` with a format - /// other than HTML. - pub fn make_for_thread( + /// [thread]: https://spec.matrix.org/latest/client-server-api/#threading + pub fn make_for_thread<'a>( self, - previous_message: &OriginalRoomMessageEvent, + metadata: impl Into>, is_reply: ReplyWithinThread, add_mentions: AddMentions, ) -> RoomMessageEventContent { + let metadata = metadata.into(); + let mut content = if is_reply == ReplyWithinThread::Yes { - self.make_reply_to(previous_message, ForwardThread::No, add_mentions) + self.make_reply_to(metadata, ForwardThread::No, add_mentions) } else { self.into() }; - let thread_root = if let Some(Relation::Thread(Thread { event_id, .. })) = - &previous_message.content.relates_to - { - event_id.clone() + let thread_root = if let Some(Thread { event_id, .. }) = &metadata.thread { + event_id.to_owned() } else { - previous_message.event_id.clone() + metadata.event_id.to_owned() }; content.relates_to = Some(Relation::Thread(Thread { event_id: thread_root, - in_reply_to: Some(InReplyTo { event_id: previous_message.event_id.clone() }), + in_reply_to: Some(InReplyTo { event_id: metadata.event_id.to_owned() }), is_falling_back: is_reply == ReplyWithinThread::No, })); @@ -262,12 +179,6 @@ impl RoomMessageEventContentWithoutRelation { /// This takes the content and sets it in `m.new_content`, and modifies the `content` to include /// a fallback. /// - /// If the message that is replaced is a reply to another message, the latter should also be - /// provided to be able to generate a rich reply fallback that takes the `body` / - /// `formatted_body` (if any) in `self` for the main text and prepends a quoted version of - /// `original_message`. - #[doc = include_str!(concat!(env!("CARGO_MANIFEST_DIR"), "/src/doc/rich_reply.md"))] - /// /// If this message contains [`Mentions`], they are copied into `m.new_content` to keep the same /// mentions, but the ones in `content` are filtered with the ones in the /// [`ReplacementMetadata`] so only new mentions will trigger a notification. @@ -281,7 +192,6 @@ impl RoomMessageEventContentWithoutRelation { pub fn make_replacement( mut self, metadata: impl Into, - replied_to_message: Option<&OriginalRoomMessageEvent>, ) -> RoomMessageEventContent { let metadata = metadata.into(); @@ -318,13 +228,7 @@ impl RoomMessageEventContentWithoutRelation { self.msgtype.make_replacement_body(); - // Add reply fallback if needed. - let mut content = if let Some(original_message) = replied_to_message { - self.make_reply_to(original_message, ForwardThread::No, AddMentions::No) - } else { - self.into() - }; - + let mut content = RoomMessageEventContent::from(self); content.relates_to = Some(relates_to); content @@ -341,25 +245,6 @@ impl RoomMessageEventContentWithoutRelation { self.mentions.get_or_insert_with(Mentions::new).add(mentions); self } - - fn make_reply_tweaks( - mut self, - original_event_id: OwnedEventId, - original_thread_id: Option, - sender_for_mentions: Option<&UserId>, - ) -> RoomMessageEventContent { - let relates_to = if let Some(event_id) = original_thread_id { - Relation::Thread(Thread::plain(event_id.to_owned(), original_event_id.to_owned())) - } else { - Relation::Reply { in_reply_to: InReplyTo { event_id: original_event_id.to_owned() } } - }; - - if let Some(sender) = sender_for_mentions { - self.mentions.get_or_insert_with(Mentions::new).user_ids.insert(sender.to_owned()); - } - - self.with_relation(Some(relates_to)) - } } impl From for RoomMessageEventContentWithoutRelation { diff --git a/crates/ruma-events/tests/it/room_message.rs b/crates/ruma-events/tests/it/room_message.rs index 37404c70..34b278cd 100644 --- a/crates/ruma-events/tests/it/room_message.rs +++ b/crates/ruma-events/tests/it/room_message.rs @@ -1,9 +1,9 @@ -use std::{borrow::Cow, collections::BTreeSet}; +use std::borrow::Cow; use assert_matches2::assert_matches; use js_int::uint; use ruma_common::{ - mxc_uri, owned_event_id, owned_room_id, owned_user_id, room_id, + mxc_uri, owned_event_id, owned_room_id, owned_user_id, serde::{Base64, Raw}, user_id, MilliSecondsSinceUnixEpoch, OwnedDeviceId, }; @@ -19,7 +19,7 @@ use ruma_events::{ }, EncryptedFileInit, JsonWebKeyInit, MediaSource, }, - AnySyncTimelineEvent, EventContent, Mentions, MessageLikeUnsigned, RawExt, + EventContent, Mentions, MessageLikeUnsigned, RawExt, }; use serde_json::{ from_value as from_json_value, json, to_value as to_json_value, Value as JsonValue, @@ -302,155 +302,6 @@ fn content_deserialization_failure() { assert_matches!(from_json_value::(json_data), Err(_)); } -#[test] -fn escape_tags_in_plain_reply_body() { - let first_message = OriginalRoomMessageEvent { - content: RoomMessageEventContent::text_plain("Usage: cp "), - event_id: owned_event_id!("$143273582443PhrSn:example.org"), - origin_server_ts: MilliSecondsSinceUnixEpoch(uint!(10_000)), - room_id: owned_room_id!("!testroomid:example.org"), - sender: owned_user_id!("@user:example.org"), - unsigned: MessageLikeUnsigned::default(), - }; - let second_message = RoomMessageEventContent::text_plain("Usage: rm ").make_reply_to( - &first_message, - ForwardThread::Yes, - AddMentions::No, - ); - assert_matches!(second_message.mentions, None); - - assert_matches!( - first_message.content.msgtype, - MessageType::Text(TextMessageEventContent { body, formatted: None, .. }) - ); - assert_eq!(body, "Usage: cp "); - - assert_matches!( - second_message.msgtype, - MessageType::Text(TextMessageEventContent { body, formatted, .. }) - ); - assert_eq!( - body, - "\ - > <@user:example.org> Usage: cp \n\ - \n\ - Usage: rm \ - " - ); - let formatted = formatted.unwrap(); - assert_eq!( - formatted.body, - "\ - \ -
\ - In reply to \ - @user:example.org\ -
\ - Usage: cp <source> <destination>\ -
\ - \ - Usage: rm <path>\ - " - ); -} - -#[test] -#[cfg(feature = "html")] -fn reply_sanitize() { - let first_message = OriginalRoomMessageEvent { - content: RoomMessageEventContent::text_html( - "# This is the first message", - "

This is the first message

", - ), - event_id: owned_event_id!("$143273582443PhrSn:example.org"), - origin_server_ts: MilliSecondsSinceUnixEpoch(uint!(10_000)), - room_id: owned_room_id!("!testroomid:example.org"), - sender: owned_user_id!("@user:example.org"), - unsigned: MessageLikeUnsigned::default(), - }; - let second_message = OriginalRoomMessageEvent { - content: RoomMessageEventContent::text_html( - "This is the _second_ message", - "This is the second message", - ) - .make_reply_to(&first_message, ForwardThread::Yes, AddMentions::No), - event_id: owned_event_id!("$143273582443PhrSn:example.org"), - origin_server_ts: MilliSecondsSinceUnixEpoch(uint!(10_000)), - room_id: owned_room_id!("!testroomid:example.org"), - sender: owned_user_id!("@user:example.org"), - unsigned: MessageLikeUnsigned::default(), - }; - let final_reply = RoomMessageEventContent::text_html( - "This is **my** reply", - "This is my reply", - ) - .make_reply_to(&second_message, ForwardThread::Yes, AddMentions::No); - - assert_matches!( - first_message.content.msgtype, - MessageType::Text(TextMessageEventContent { body, formatted, .. }) - ); - assert_eq!(body, "# This is the first message"); - let formatted = formatted.unwrap(); - assert_eq!(formatted.body, "

This is the first message

"); - - assert_matches!( - second_message.content.msgtype, - MessageType::Text(TextMessageEventContent { body, formatted, .. }) - ); - assert_eq!( - body, - "\ - > <@user:example.org> # This is the first message\n\ - \n\ - This is the _second_ message\ - " - ); - let formatted = formatted.unwrap(); - assert_eq!( - formatted.body, - "\ - \ -
\ - In reply to \ - @user:example.org\ -
\ -

This is the first message

\ -
\ - \ - This is the second message\ - " - ); - - assert_matches!( - final_reply.msgtype, - MessageType::Text(TextMessageEventContent { body, formatted, .. }) - ); - assert_eq!( - body, - "\ - > <@user:example.org> This is the _second_ message\n\ - \n\ - This is **my** reply\ - " - ); - let formatted = formatted.unwrap(); - assert_eq!( - formatted.body, - "\ - \ -
\ - In reply to \ - @user:example.org\ -
\ - This is the second message\ -
\ - \ - This is my reply\ - " - ); -} - #[test] fn reply_thread_fallback() { let thread_root = OriginalRoomMessageEvent { @@ -569,216 +420,7 @@ fn reply_add_mentions() { } #[test] -fn reply_to_raw() { - let room_id = room_id!("!roomid:notareal.hs"); - let event_id = owned_event_id!("$143273582443PhrSn"); - - let original_message: Raw = from_json_value(json!({ - "content": { - "body": "Hello, World!", - "msgtype": "m.text", - }, - "event_id": event_id, - "origin_server_ts": 134_829_848, - "sender": "@user:notareal.hs", - "type": "m.room.message", - })) - .unwrap(); - - let reply = RoomMessageEventContent::text_html( - "This is **my** reply", - "This is my reply", - ) - .make_reply_to_raw( - &original_message, - event_id.clone(), - room_id, - ForwardThread::Yes, - AddMentions::No, - ); - - assert_matches!(reply.relates_to, Some(Relation::Reply { in_reply_to })); - assert_eq!(in_reply_to.event_id, event_id); - - assert_matches!(reply.msgtype, MessageType::Text(text_msg)); - assert_eq!( - text_msg.body, - "> <@user:notareal.hs> Hello, World!\n\ - \n\ - This is **my** reply" - ); - assert_eq!( - text_msg.formatted.unwrap().body, - "\ -
\ - In reply to \ - @user:notareal.hs\ -
\ - Hello, World!\ -
\ - \ - This is my reply" - ); -} - -#[test] -fn reply_to_raw_no_body() { - let room_id = room_id!("!roomid:notareal.hs"); - let event_id = owned_event_id!("$143273582443PhrSn"); - - let original_message: Raw = from_json_value(json!({ - "content": {}, - "event_id": event_id, - "origin_server_ts": 134_829_848, - "sender": "@user:notareal.hs", - "type": "m.room.message", - })) - .unwrap(); - - let reply = RoomMessageEventContent::text_html( - "This is **my** reply", - "This is my reply", - ) - .make_reply_to_raw( - &original_message, - event_id.clone(), - room_id, - ForwardThread::Yes, - AddMentions::No, - ); - - assert_matches!(reply.relates_to, Some(Relation::Reply { in_reply_to })); - assert_eq!(in_reply_to.event_id, event_id); - - assert_matches!(reply.msgtype, MessageType::Text(text_msg)); - assert_eq!(text_msg.body, "This is **my** reply"); - assert_eq!(text_msg.formatted.unwrap().body, "This is my reply"); -} - -#[test] -fn reply_to_raw_no_sender() { - let room_id = room_id!("!roomid:notareal.hs"); - let event_id = owned_event_id!("$143273582443PhrSn"); - - let original_message: Raw = from_json_value(json!({ - "content": { - "body": "Hello, World!", - "msgtype": "m.text", - }, - "event_id": event_id, - "origin_server_ts": 134_829_848, - "type": "m.room.message", - })) - .unwrap(); - - let reply = RoomMessageEventContent::text_html( - "This is **my** reply", - "This is my reply", - ) - .make_reply_to_raw( - &original_message, - event_id.clone(), - room_id, - ForwardThread::Yes, - AddMentions::No, - ); - - assert_matches!(reply.relates_to, Some(Relation::Reply { in_reply_to })); - assert_eq!(in_reply_to.event_id, event_id); - - assert_matches!(reply.msgtype, MessageType::Text(text_msg)); - assert_eq!(text_msg.body, "This is **my** reply"); - assert_eq!(text_msg.formatted.unwrap().body, "This is my reply"); -} - -#[test] -fn reply_to_raw_forward_thread() { - let room_id = room_id!("!roomid:notareal.hs"); - let event_id = owned_event_id!("$143273582443PhrSn"); - - let original_message: Raw = from_json_value(json!({ - "content": { - "m.relates_to": { - "rel_type": "m.thread", - "event_id": "$threadroot", - "m.in_reply_to": { - "event_id": "$repliedto", - }, - }, - }, - "event_id": event_id, - "origin_server_ts": 134_829_848, - "sender": "@user:notareal.hs", - "type": "m.room.message", - })) - .unwrap(); - - let reply = RoomMessageEventContent::text_html( - "This is **my** reply", - "This is my reply", - ) - .make_reply_to_raw( - &original_message, - event_id.clone(), - room_id, - ForwardThread::Yes, - AddMentions::No, - ); - - assert_matches!(reply.relates_to, Some(Relation::Thread(thread))); - assert_eq!(thread.event_id, "$threadroot"); - assert_eq!(thread.in_reply_to.unwrap().event_id, event_id); - - assert_matches!(reply.msgtype, MessageType::Text(text_msg)); - assert_eq!(text_msg.body, "This is **my** reply"); - assert_eq!(text_msg.formatted.unwrap().body, "This is my reply"); -} - -#[test] -fn reply_to_raw_add_mentions() { - let room_id = room_id!("!roomid:notareal.hs"); - let event_id = owned_event_id!("$143273582443PhrSn"); - - let user_id = owned_user_id!("@user:notareal.hs"); - let other_user_id = owned_user_id!("@other_user:notareal.hs"); - - let original_message: Raw = from_json_value(json!({ - "content": { - "m.mentions": { - "user_ids": [other_user_id], - }, - }, - "event_id": event_id, - "origin_server_ts": 134_829_848, - "sender": user_id, - "type": "m.room.message", - })) - .unwrap(); - - let reply = RoomMessageEventContent::text_html( - "This is **my** reply", - "This is my reply", - ) - .make_reply_to_raw( - &original_message, - event_id.clone(), - room_id, - ForwardThread::Yes, - AddMentions::Yes, - ); - - assert_matches!(reply.relates_to, Some(Relation::Reply { in_reply_to })); - assert_eq!(in_reply_to.event_id, event_id); - - assert_matches!(reply.msgtype, MessageType::Text(text_msg)); - assert_eq!(text_msg.body, "This is **my** reply"); - assert_eq!(text_msg.formatted.unwrap().body, "This is my reply"); - - assert_eq!(reply.mentions.unwrap().user_ids, BTreeSet::from([user_id])); -} - -#[test] -fn make_replacement_no_reply() { +fn make_replacement() { let content = RoomMessageEventContent::text_html( "This is _an edited_ message.", "This is an edited message.", @@ -798,7 +440,7 @@ fn make_replacement_no_reply() { let original_message: OriginalSyncRoomMessageEvent = from_json_value(original_message_json).unwrap(); - let content = content.make_replacement(&original_message, None); + let content = content.make_replacement(&original_message); assert_matches!( content.msgtype, @@ -810,71 +452,6 @@ fn make_replacement_no_reply() { assert_matches!(content.mentions, None); } -#[test] -fn make_replacement_with_reply() { - let replied_to_message = OriginalRoomMessageEvent { - content: RoomMessageEventContent::text_html( - "# This is the first message", - "

This is the first message

", - ), - event_id: owned_event_id!("$143273582443PhrSn:example.org"), - origin_server_ts: MilliSecondsSinceUnixEpoch(uint!(10_000)), - room_id: owned_room_id!("!testroomid:example.org"), - sender: owned_user_id!("@user:example.org"), - unsigned: MessageLikeUnsigned::default(), - }; - - let content = RoomMessageEventContent::text_html( - "This is _an edited_ reply.", - "This is an edited reply.", - ); - - let original_message_json = json!({ - "content": { - "body": "Hello, World!", - "msgtype": "m.text", - }, - "event_id": "$143273582443PhrSn", - "origin_server_ts": 134_829_848, - "room_id": "!roomid:notareal.hs", - "sender": "@user:notareal.hs", - "type": "m.room.message", - }); - let original_message: OriginalSyncRoomMessageEvent = - from_json_value(original_message_json).unwrap(); - - let content = content.make_replacement(&original_message, Some(&replied_to_message)); - - assert_matches!( - content.msgtype, - MessageType::Text(TextMessageEventContent { body, formatted, .. }) - ); - assert_eq!( - body, - "\ - > <@user:example.org> # This is the first message\n\ - \n\ - * This is _an edited_ reply.\ - " - ); - let formatted = formatted.unwrap(); - assert_eq!( - formatted.body, - "\ - \ -
\ - In reply to \ - @user:example.org\ -
\ -

This is the first message

\ -
\ - \ - * This is an edited reply.\ - " - ); - assert_matches!(content.mentions, None); -} - #[test] fn audio_msgtype_serialization() { let message_event_content = @@ -1253,7 +830,7 @@ fn add_mentions_then_make_replacement() { "This is an edited message.", ); content = content.add_mentions(Mentions::with_user_ids(vec![alice.clone(), bob.clone()])); - content = content.make_replacement(&original_message, None); + content = content.make_replacement(&original_message); let mentions = content.mentions.unwrap(); assert_eq!(mentions.user_ids, [bob.clone()].into()); @@ -1287,7 +864,7 @@ fn make_replacement_then_add_mentions() { "This is _an edited_ message.", "This is an edited message.", ); - content = content.make_replacement(&original_message, None); + content = content.make_replacement(&original_message); content = content.add_mentions(Mentions::with_user_ids(vec![alice.clone(), bob.clone()])); let mentions = content.mentions.unwrap();