send queue: allow sending reactions to local echoes

bindings/matrix-sdk-ffi/src/timeline/mod.rs

@@ -45,7 +45,6 @@ use ruma::{
             },
         },
         receipt::ReceiptThread,
-        relation::Annotation,
         room::message::{
             ForwardThread, LocationMessageEventContent, MessageType,
             RoomMessageEventContentWithoutRelation,
@@ -538,9 +537,21 @@ impl Timeline {
         let _ = self.send(Arc::new(room_message_event_content)).await;
     }
 
-    pub async fn toggle_reaction(&self, event_id: String, key: String) -> Result<(), ClientError> {
-        let event_id = EventId::parse(event_id)?;
-        self.inner.toggle_reaction(&Annotation::new(event_id, key)).await?;
+    /// Toggle a reaction on an event.
+    ///
+    /// The `unique_id` parameter is a string returned by
+    /// the `TimelineItem::unique_id()` method. As such, this method works both
+    /// on local echoes and remote items.
+    ///
+    /// Adds or redacts a reaction based on the state of the reaction at the
+    /// time it is called.
+    ///
+    /// When redacting a previous reaction, the redaction reason is not set.
+    ///
+    /// Ensures that only one reaction is sent at a time to avoid race
+    /// conditions and spamming the homeserver with requests.
+    pub async fn toggle_reaction(&self, unique_id: String, key: String) -> Result<(), ClientError> {
+        self.inner.toggle_reaction(&unique_id, &key).await?;
         Ok(())
     }
 

crates/matrix-sdk-base/Cargo.toml

@@ -27,6 +27,11 @@ experimental-sliding-sync = [
 ]
 uniffi = ["dep:uniffi", "matrix-sdk-crypto?/uniffi", "matrix-sdk-common/uniffi"]
 
+# Private feature, see
+# https://github.com/matrix-org/matrix-rust-sdk/pull/3749#issuecomment-2312939823 for the gory
+# details.
+test-send-sync = []
+
 # "message-ids" feature doesn't do anything and is deprecated.
 message-ids = []
 

crates/matrix-sdk-base/src/rooms/normal.rs

@@ -992,6 +992,23 @@ impl Room {
     }
 }
 
+// See https://github.com/matrix-org/matrix-rust-sdk/pull/3749#issuecomment-2312939823.
+#[cfg(not(feature = "test-send-sync"))]
+unsafe impl Send for Room {}
+
+// See https://github.com/matrix-org/matrix-rust-sdk/pull/3749#issuecomment-2312939823.
+#[cfg(not(feature = "test-send-sync"))]
+unsafe impl Sync for Room {}
+
+#[cfg(feature = "test-send-sync")]
+#[test]
+// See https://github.com/matrix-org/matrix-rust-sdk/pull/3749#issuecomment-2312939823.
+fn test_send_sync_for_room() {
+    fn assert_send_sync<T: Send + Sync>() {}
+
+    assert_send_sync::<Room>();
+}
+
 /// The underlying pure data structure for joined and left rooms.
 ///
 /// Holds all the info needed to persist a room into the state store.

crates/matrix-sdk-base/src/store/traits.rs

@@ -1201,6 +1201,12 @@ pub enum DependentQueuedEventKind {
 
     /// The event should be redacted/aborted/removed.
     Redact,
+
+    /// The event should be reacted to, with the given key.
+    React {
+        /// Key used for the reaction.
+        key: String,
+    },
 }
 
 /// A transaction id identifying a [`DependentQueuedEvent`] rather than its

crates/matrix-sdk-ui/src/timeline/day_dividers.rs

@@ -623,7 +623,6 @@ mod tests {
         let event_kind = EventTimelineItemKind::Remote(RemoteEventTimelineItem {
             event_id: owned_event_id!("$1"),
             transaction_id: None,
-            reactions: Default::default(),
             read_receipts: Default::default(),
             is_own: false,
             is_highlighted: false,
@@ -638,6 +637,7 @@ mod tests {
             timestamp,
             TimelineItemContent::RedactedMessage,
             event_kind,
+            Default::default(),
             false,
         )
     }

crates/matrix-sdk-ui/src/timeline/event_handler.rs

@@ -572,11 +572,6 @@ impl<'a, 'o> TimelineEventHandler<'a, 'o> {
         };
 
         if let Some((idx, event_item)) = rfind_event_by_id(self.items, reacted_to_event_id) {
-            let Some(remote_event_item) = event_item.as_remote() else {
-                error!("received reaction to a local echo");
-                return;
-            };
-
             // Ignore reactions on redacted events.
             if let TimelineItemContent::RedactedMessage = event_item.content() {
                 debug!("Ignoring reaction on redacted event");
@@ -586,7 +581,7 @@ impl<'a, 'o> TimelineEventHandler<'a, 'o> {
             trace!("Added reaction");
 
             // Add the reaction to the event item's bundled reactions.
-            let mut reactions = remote_event_item.reactions.clone();
+            let mut reactions = event_item.reactions.clone();
 
             reactions.entry(c.relates_to.key.clone()).or_default().insert(
                 self.ctx.sender.clone(),
@@ -839,16 +834,10 @@ impl<'a, 'o> TimelineEventHandler<'a, 'o> {
                 return false;
             };
 
-            let Some(remote_event_item) = item.as_remote() else {
-                error!("inconsistent state: redaction received on a non-remote event item");
-                return false;
-            };
-
-            let mut reactions = remote_event_item.reactions.clone();
+            let mut reactions = item.reactions.clone();
             if reactions.remove_reaction(&sender, &key).is_some() {
                 trace!("Removing reaction");
-                let new_item = item.with_kind(remote_event_item.with_reactions(reactions));
-                self.items.set(item_pos, TimelineItem::new(new_item, item.internal_id.to_owned()));
+                self.items.set(item_pos, item.with_reactions(reactions));
                 self.result.items_updated += 1;
                 return true;
             }
@@ -894,7 +883,6 @@ impl<'a, 'o> TimelineEventHandler<'a, 'o> {
                 RemoteEventTimelineItem {
                     event_id: event_id.clone(),
                     transaction_id: txn_id.clone(),
-                    reactions,
                     read_receipts: self.ctx.read_receipts.clone(),
                     is_own: self.ctx.is_own_event,
                     is_highlighted: self.ctx.is_highlighted,
@@ -915,6 +903,7 @@ impl<'a, 'o> TimelineEventHandler<'a, 'o> {
             timestamp,
             content,
             kind,
+            reactions,
             is_room_encrypted,
         );
 
@@ -967,10 +956,7 @@ impl<'a, 'o> TimelineEventHandler<'a, 'o> {
                         if old_item.content.is_redacted() && !item.content.is_redacted() {
                             warn!("Got original form of an event that was previously redacted");
                             item.content = item.content.redact(&self.meta.room_version);
-                            item.as_remote_mut()
-                                .expect("Can't have a local item when flow == Remote")
-                                .reactions
-                                .clear();
+                            item.reactions.clear();
                         }
                     }
 

crates/matrix-sdk-ui/src/timeline/event_item/mod.rs

@@ -21,7 +21,7 @@ use as_variant::as_variant;
 use indexmap::IndexMap;
 use matrix_sdk::{
     deserialized_responses::{EncryptionInfo, ShieldState},
-    send_queue::SendHandle,
+    send_queue::{SendHandle, SendReactionHandle},
     Client, Error,
 };
 use matrix_sdk_base::{
@@ -66,6 +66,8 @@ pub struct EventTimelineItem {
     pub(super) sender: OwnedUserId,
     /// The sender's profile of the event.
     pub(super) sender_profile: TimelineDetails<Profile>,
+    /// All bundled reactions about the event.
+    pub(super) reactions: ReactionsByKeyBySender,
     /// The timestamp of the event.
     pub(super) timestamp: MilliSecondsSinceUnixEpoch,
     /// The content of the event.
@@ -114,10 +116,11 @@ impl EventTimelineItem {
         timestamp: MilliSecondsSinceUnixEpoch,
         content: TimelineItemContent,
         kind: EventTimelineItemKind,
+        reactions: ReactionsByKeyBySender,
         is_room_encrypted: bool,
     ) -> Self {
         let is_room_encrypted = Some(is_room_encrypted);
-        Self { sender, sender_profile, timestamp, content, kind, is_room_encrypted }
+        Self { sender, sender_profile, timestamp, content, reactions, kind, is_room_encrypted }
     }
 
     /// If the supplied low-level `SyncTimelineEvent` is suitable for use as the
@@ -175,7 +178,6 @@ impl EventTimelineItem {
         let kind = RemoteEventTimelineItem {
             event_id,
             transaction_id: None,
-            reactions,
             read_receipts,
             is_own,
             is_highlighted,
@@ -199,9 +201,16 @@ impl EventTimelineItem {
         } else {
             TimelineDetails::Unavailable
         };
-        let is_room_encrypted = None;
 
-        Some(Self { sender, sender_profile, timestamp, content, kind, is_room_encrypted })
+        Some(Self {
+            sender,
+            sender_profile,
+            timestamp,
+            content,
+            kind,
+            reactions,
+            is_room_encrypted: None,
+        })
     }
 
     /// Check whether this item is a local echo.
@@ -291,12 +300,7 @@ impl EventTimelineItem {
 
     /// Get the reactions of this item.
     pub fn reactions(&self) -> &ReactionsByKeyBySender {
-        // There's not much of a point in allowing reactions to local echoes.
-        static EMPTY_REACTIONS: Lazy<ReactionsByKeyBySender> = Lazy::new(Default::default);
-        match &self.kind {
-            EventTimelineItemKind::Local(_) => &EMPTY_REACTIONS,
-            EventTimelineItemKind::Remote(remote_event) => &remote_event.reactions,
-        }
+        &self.reactions
     }
 
     /// Get the read receipts of this item.
@@ -453,6 +457,11 @@ impl EventTimelineItem {
         Self { kind: kind.into(), ..self.clone() }
     }
 
+    /// Clone the current event item, and update its `reactions`.
+    pub fn with_reactions(&self, reactions: ReactionsByKeyBySender) -> Self {
+        Self { reactions, ..self.clone() }
+    }
+
     /// Clone the current event item, and update its content.
     ///
     /// Optionally update `latest_edit_json` if the update is an edit received
@@ -490,6 +499,7 @@ impl EventTimelineItem {
             content,
             kind,
             is_room_encrypted: self.is_room_encrypted,
+            reactions: ReactionsByKeyBySender::default(),
         }
     }
 
@@ -615,6 +625,10 @@ pub enum EventItemOrigin {
 /// What's the status of a reaction?
 #[derive(Clone, Debug)]
 pub enum ReactionStatus {
+    /// It's a local reaction to a local echo.
+    ///
+    /// The handle is missing only in testing contexts.
+    LocalToLocal(Option<SendReactionHandle>),
     /// It's a local reaction to a remote event.
     ///
     /// The handle is missing only in testing contexts.

crates/matrix-sdk-ui/src/timeline/event_item/remote.rs

@@ -22,8 +22,6 @@ use ruma::{
     OwnedEventId, OwnedTransactionId, OwnedUserId,
 };
 
-use super::ReactionsByKeyBySender;
-
 /// An item for an event that was received from the homeserver.
 #[derive(Clone)]
 pub(in crate::timeline) struct RemoteEventTimelineItem {
@@ -33,9 +31,6 @@ pub(in crate::timeline) struct RemoteEventTimelineItem {
     /// If available, the transaction id we've used to send this event.
     pub transaction_id: Option<OwnedTransactionId>,
 
-    /// All bundled reactions about the event.
-    pub reactions: ReactionsByKeyBySender,
-
     /// All read receipts for the event.
     ///
     /// The key is the ID of a room member and the value are details about the
@@ -78,20 +73,9 @@ impl RemoteEventTimelineItem {
         Self { encryption_info, ..self.clone() }
     }
 
-    /// Clone the current event item, and update its `reactions`.
-    pub fn with_reactions(&self, reactions: ReactionsByKeyBySender) -> Self {
-        Self { reactions, ..self.clone() }
-    }
-
-    /// Clone the current event item, and clear its `reactions` as well as the
-    /// JSON representation fields.
+    /// Clone the current event item, and redacts its fields.
     pub fn redact(&self) -> Self {
-        Self {
-            reactions: ReactionsByKeyBySender::default(),
-            original_json: None,
-            latest_edit_json: None,
-            ..self.clone()
-        }
+        Self { original_json: None, latest_edit_json: None, ..self.clone() }
     }
 }
 
@@ -116,7 +100,6 @@ impl fmt::Debug for RemoteEventTimelineItem {
         let Self {
             event_id,
             transaction_id,
-            reactions,
             read_receipts,
             is_own,
             encryption_info,
@@ -129,7 +112,6 @@ impl fmt::Debug for RemoteEventTimelineItem {
         f.debug_struct("RemoteEventTimelineItem")
             .field("event_id", event_id)
             .field("transaction_id", transaction_id)
-            .field("reactions", reactions)
             .field("read_receipts", read_receipts)
             .field("is_own", is_own)
             .field("is_highlighted", is_highlighted)