refactor(timeline): deduplicate parsing of events when making a RepliedToEvent

Author: bnjbvr

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

@@ -32,7 +32,7 @@ use matrix_sdk::{
     },
 };
 use matrix_sdk_ui::timeline::{
-    self, AttachmentSource, EventItemOrigin, Profile, RepliedToEvent, TimelineDetails,
+    self, AttachmentSource, EventItemOrigin, Profile, TimelineDetails,
     TimelineUniqueId as SdkTimelineUniqueId,
 };
 use mime::Mime;
@@ -687,9 +687,7 @@ impl Timeline {
         let event_id = EventId::parse(&event_id_str)?;
 
         let replied_to = match self.inner.room().load_or_fetch_event(&event_id, None).await {
-            Ok(event) => RepliedToEvent::try_from_timeline_event_for_room(event, self.inner.room())
-                .await
-                .map_err(ClientError::from),
+            Ok(event) => self.inner.make_replied_to(event).await.map_err(ClientError::from),
             Err(e) => Err(ClientError::from(e)),
         };
 

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

@@ -1314,6 +1314,23 @@ impl<P: RoomDataProvider, D: Decryptor> TimelineController<P, D> {
         );
         txn.commit();
     }
+
+    pub(super) async fn make_replied_to(
+        &self,
+        event: TimelineEvent,
+    ) -> Result<RepliedToEvent, Error> {
+        // Reborrow, to avoid that the automatic deref borrows the entire guard (and we
+        // can't borrow both items and meta).
+        let state = &mut *self.state.write().await;
+
+        RepliedToEvent::try_from_timeline_event(
+            event,
+            &self.room_data_provider,
+            &state.items,
+            &mut state.meta,
+        )
+        .await
+    }
 }
 
 impl TimelineController {
@@ -1325,8 +1342,8 @@ impl TimelineController {
     /// replying to, if applicable.
     #[instrument(skip(self))]
     pub(super) async fn fetch_in_reply_to_details(&self, event_id: &EventId) -> Result<(), Error> {
-        let state = self.state.write().await;
-        let (index, item) = rfind_event_by_id(&state.items, event_id)
+        let state_guard = self.state.write().await;
+        let (index, item) = rfind_event_by_id(&state_guard.items, event_id)
             .ok_or(Error::EventNotInTimeline(TimelineEventItemId::EventId(event_id.to_owned())))?;
         let remote_item = item
             .as_remote()
@@ -1353,7 +1370,8 @@ impl TimelineController {
         let internal_id = item.internal_id.to_owned();
         let item = item.clone();
         let event = fetch_replied_to_event(
-            state,
+            state_guard,
+            &self.state,
             index,
             &item,
             internal_id,
@@ -1507,15 +1525,16 @@ impl<P: RoomDataProvider> TimelineController<P, (OlmMachine, OwnedRoomId)> {
 }
 
 async fn fetch_replied_to_event(
-    mut state: RwLockWriteGuard<'_, TimelineState>,
+    mut state_guard: RwLockWriteGuard<'_, TimelineState>,
+    state_lock: &RwLock<TimelineState>,
     index: usize,
     item: &EventTimelineItem,
     internal_id: TimelineUniqueId,
     msglike: &MsgLikeContent,
     in_reply_to: &EventId,
     room: &Room,
 ) -> Result<TimelineDetails<Box<RepliedToEvent>>, Error> {
-    if let Some((_, item)) = rfind_event_by_id(&state.items, in_reply_to) {
+    if let Some((_, item)) = rfind_event_by_id(&state_guard.items, in_reply_to) {
         let details = TimelineDetails::Ready(Box::new(RepliedToEvent::from_timeline_item(&item)));
         trace!("Found replied-to event locally");
         return Ok(details);
@@ -1531,16 +1550,25 @@ async fn fetch_replied_to_event(
         .with_content(TimelineItemContent::MsgLike(msglike.with_in_reply_to(in_reply_to_details)));
 
     let new_timeline_item = TimelineItem::new(event_item, internal_id);
-    state.items.replace(index, new_timeline_item);
+    state_guard.items.replace(index, new_timeline_item);
 
-    // Don't hold the state lock while the network request is made
-    drop(state);
+    // Don't hold the state lock while the network request is made.
+    drop(state_guard);
 
     trace!("Fetching replied-to event");
     let res = match room.load_or_fetch_event(in_reply_to, None).await {
-        Ok(timeline_event) => TimelineDetails::Ready(Box::new(
-            RepliedToEvent::try_from_timeline_event(timeline_event, room).await?,
-        )),
+        Ok(timeline_event) => {
+            let state = &mut *state_lock.write().await;
+            TimelineDetails::Ready(Box::new(
+                RepliedToEvent::try_from_timeline_event(
+                    timeline_event,
+                    room,
+                    &state.items,
+                    &mut state.meta,
+                )
+                .await?,
+            ))
+        }
         Err(e) => TimelineDetails::Error(Arc::new(e)),
     };
     Ok(res)

crates/matrix-sdk-ui/src/timeline/event_item/content/reply.rs

@@ -15,30 +15,17 @@
 use std::sync::Arc;
 
 use imbl::Vector;
-use matrix_sdk::{
-    crypto::types::events::UtdCause,
-    deserialized_responses::{TimelineEvent, TimelineEventKind},
-    Room,
-};
-use ruma::{
-    events::{
-        poll::unstable_start::UnstablePollStartEventContent, AnyMessageLikeEventContent,
-        AnySyncTimelineEvent,
-    },
-    html::RemoveReplyFallback,
-    OwnedEventId, OwnedUserId, UserId,
-};
+use matrix_sdk::deserialized_responses::TimelineEvent;
+use ruma::{OwnedEventId, OwnedUserId, UserId};
 use tracing::{debug, instrument, warn};
 
 use super::TimelineItemContent;
 use crate::timeline::{
-    event_item::{
-        content::{MsgLikeContent, MsgLikeKind},
-        extract_room_msg_edit_content, EventTimelineItem, Profile, TimelineDetails,
-    },
+    controller::TimelineMetadata,
+    event_handler::TimelineAction,
+    event_item::{content::MsgLikeContent, EventTimelineItem, Profile, TimelineDetails},
     traits::RoomDataProvider,
-    EncryptedMessage, Error as TimelineError, Message, PollState, ReactionsByKeyBySender, Sticker,
-    TimelineItem,
+    Error as TimelineError, TimelineItem,
 };
 
 /// Details about an event being replied to.
@@ -104,26 +91,23 @@ impl RepliedToEvent {
         }
     }
 
-    /// Try to create a `RepliedToEvent` from a `TimelineEvent` by providing the
-    /// room.
-    pub async fn try_from_timeline_event_for_room(
-        timeline_event: TimelineEvent,
-        room_data_provider: &Room,
-    ) -> Result<Self, TimelineError> {
-        Self::try_from_timeline_event(timeline_event, room_data_provider).await
-    }
-
     #[instrument(skip_all)]
     pub(in crate::timeline) async fn try_from_timeline_event<P: RoomDataProvider>(
         timeline_event: TimelineEvent,
         room_data_provider: &P,
+        timeline_items: &Vector<Arc<TimelineItem>>,
+        meta: &mut TimelineMetadata,
     ) -> Result<Self, TimelineError> {
-        let event = match timeline_event.raw().deserialize() {
-            Ok(AnySyncTimelineEvent::MessageLike(event)) => event,
-            Ok(_) => {
-                warn!("can't get details, event isn't a message-like event");
-                return Err(TimelineError::UnsupportedEvent);
-            }
+        let (raw_event, unable_to_decrypt_info) = match timeline_event.kind {
+            matrix_sdk::deserialized_responses::TimelineEventKind::UnableToDecrypt {
+                utd_info,
+                event,
+            } => (event, Some(utd_info)),
+            _ => (timeline_event.kind.into_raw(), None),
+        };
+
+        let event = match raw_event.deserialize() {
+            Ok(event) => event,
             Err(err) => {
                 warn!("can't get details, event couldn't be deserialized: {err}");
                 return Err(TimelineError::UnsupportedEvent);
@@ -132,94 +116,24 @@ impl RepliedToEvent {
 
         debug!(event_type = %event.event_type(), "got deserialized event");
 
-        let content = match event.original_content() {
-            Some(content) => match content {
-                AnyMessageLikeEventContent::RoomMessage(c) => {
-                    // Assume we're not interested in aggregations in this context:
-                    // this is information for an embedded (replied-to) event, that will usually not
-                    // include detailed information like reactions.
-                    let reactions = ReactionsByKeyBySender::default();
-                    let thread_root = None;
-                    let in_reply_to = None;
-                    let thread_summary = None;
-
-                    TimelineItemContent::MsgLike(MsgLikeContent {
-                        kind: MsgLikeKind::Message(Message::from_event(
-                            c.msgtype,
-                            c.mentions,
-                            extract_room_msg_edit_content(event.relations()),
-                            RemoveReplyFallback::Yes,
-                        )),
-                        reactions,
-                        thread_root,
-                        in_reply_to,
-                        thread_summary,
-                    })
-                }
-
-                AnyMessageLikeEventContent::Sticker(content) => {
-                    // Assume we're not interested in aggregations in this context.
-                    // (See above an explanation as to why that's the case.)
-                    let reactions = Default::default();
-                    let thread_root = None;
-                    let in_reply_to = None;
-                    let thread_summary = None;
-
-                    TimelineItemContent::MsgLike(MsgLikeContent {
-                        kind: MsgLikeKind::Sticker(Sticker { content }),
-                        reactions,
-                        thread_root,
-                        in_reply_to,
-                        thread_summary,
-                    })
-                }
-
-                AnyMessageLikeEventContent::RoomEncrypted(content) => {
-                    let utd_cause = match &timeline_event.kind {
-                        TimelineEventKind::UnableToDecrypt { utd_info, .. } => UtdCause::determine(
-                            timeline_event.raw(),
-                            room_data_provider.crypto_context_info().await,
-                            utd_info,
-                        ),
-                        _ => UtdCause::Unknown,
-                    };
-
-                    TimelineItemContent::MsgLike(MsgLikeContent::unable_to_decrypt(
-                        EncryptedMessage::from_content(content, utd_cause),
-                    ))
-                }
-
-                AnyMessageLikeEventContent::UnstablePollStart(
-                    UnstablePollStartEventContent::New(content),
-                ) => {
-                    // Assume we're not interested in aggregations in this context.
-                    // (See above an explanation as to why that's the case.)
-                    let reactions = Default::default();
-                    let thread_root = None;
-                    let in_reply_to = None;
-                    let thread_summary = None;
-
-                    // TODO: could we provide the bundled edit here?
-                    let poll_state = PollState::new(content);
-                    TimelineItemContent::MsgLike(MsgLikeContent {
-                        kind: MsgLikeKind::Poll(poll_state),
-                        reactions,
-                        thread_root,
-                        in_reply_to,
-                        thread_summary,
-                    })
-                }
-
-                _ => {
-                    warn!("unsupported event type");
-                    return Err(TimelineError::UnsupportedEvent);
-                }
-            },
-
-            None => TimelineItemContent::MsgLike(MsgLikeContent::redacted()),
+        let sender = event.sender().to_owned();
+        let action = TimelineAction::from_event(
+            event,
+            &raw_event,
+            room_data_provider,
+            unable_to_decrypt_info,
+            timeline_items,
+            meta,
+        )
+        .await;
+
+        let content = if let Some(TimelineAction::AddItem { content, .. }) = action {
+            content
+        } else {
+            // TODO: need some kind of "unsupported" event here: return an option
+            TimelineItemContent::MsgLike(MsgLikeContent::redacted())
         };
 
-        let sender = event.sender().to_owned();
         let sender_profile = TimelineDetails::from_initial_value(
             room_data_provider.profile_from_user_id(&sender).await,
         );

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

@@ -25,6 +25,7 @@ use futures_core::Stream;
 use imbl::Vector;
 use matrix_sdk::{
     attachment::AttachmentConfig,
+    deserialized_responses::TimelineEvent,
     event_cache::{EventCacheDropHandles, RoomEventCache},
     event_handler::EventHandlerHandle,
     executor::JoinHandle,
@@ -667,6 +668,12 @@ impl Timeline {
             Ok(false)
         }
     }
+
+    /// Create a [`RepliedToEvent`] from an arbitrary event, be it in the
+    /// timeline or not.
+    pub async fn make_replied_to(&self, event: TimelineEvent) -> Result<RepliedToEvent, Error> {
+        self.controller.make_replied_to(event).await
+    }
 }
 
 /// Test helpers, likely not very useful in production.

crates/matrix-sdk-ui/tests/integration/timeline/replies.rs

@@ -350,7 +350,13 @@ async fn test_fetch_details_utd() {
         let in_reply_to = in_reply_to.clone().unwrap();
         assert_let!(TimelineDetails::Ready(replied_to) = &in_reply_to.event);
         assert_eq!(replied_to.sender(), *ALICE);
-        assert!(replied_to.content().is_unable_to_decrypt());
+        assert_matches!(
+            replied_to.content(),
+            TimelineItemContent::MsgLike(MsgLikeContent {
+                kind: MsgLikeKind::UnableToDecrypt(_),
+                ..
+            })
+        );
     }
 }