Skip to content

refactor(timeline): use TimelineAction::from_event to create a RepliedToEvent struct #5057

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
May 21, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 8 additions & 5 deletions bindings/matrix-sdk-ffi/src/timeline/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -687,14 +687,12 @@ 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)),
};

match replied_to {
Ok(replied_to) => Ok(Arc::new(InReplyToDetails::new(
Ok(Some(replied_to)) => Ok(Arc::new(InReplyToDetails::new(
event_id_str,
RepliedToEventDetails::Ready {
content: replied_to.content().clone().into(),
Expand All @@ -703,6 +701,11 @@ impl Timeline {
},
))),

Ok(None) => Ok(Arc::new(InReplyToDetails::new(
event_id_str,
RepliedToEventDetails::Error { message: "unsupported event".to_owned() },
))),

Err(e) => Ok(Arc::new(InReplyToDetails::new(
event_id_str,
RepliedToEventDetails::Error { message: e.to_string() },
Expand Down
64 changes: 53 additions & 11 deletions crates/matrix-sdk-ui/src/timeline/controller/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1319,6 +1319,28 @@ impl<P: RoomDataProvider, D: Decryptor> TimelineController<P, D> {
);
txn.commit();
}

/// Create a [`RepliedToEvent`] from an arbitrary event, be it in the
/// timeline or not.
///
/// Can be `None` if the event cannot be represented as a standalone item,
/// because it's an aggregation.
pub(super) async fn make_replied_to(
&self,
event: TimelineEvent,
) -> Result<Option<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 {
Expand All @@ -1330,8 +1352,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()
Expand All @@ -1358,7 +1380,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,
Expand Down Expand Up @@ -1511,16 +1534,18 @@ impl<P: RoomDataProvider> TimelineController<P, (OlmMachine, OwnedRoomId)> {
}
}

#[allow(clippy::too_many_arguments)]
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);
Expand All @@ -1536,17 +1561,34 @@ 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;

let replied_to_item = RepliedToEvent::try_from_timeline_event(
timeline_event,
room,
&state.items,
&mut state.meta,
)
.await?;

if let Some(item) = replied_to_item {
TimelineDetails::Ready(Box::new(item))
} else {
// The replied-to item is an aggregation, not a standalone item.
return Err(Error::UnsupportedEvent);
}
}

Err(e) => TimelineDetails::Error(Arc::new(e)),
};

Ok(res)
}
156 changes: 34 additions & 122 deletions crates/matrix-sdk-ui/src/timeline/event_item/content/reply.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::{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.
Expand Down Expand Up @@ -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,
) -> 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);
}
timeline_items: &Vector<Arc<TimelineItem>>,
meta: &mut TimelineMetadata,
) -> Result<Option<Self>, TimelineError> {
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);
Expand All @@ -132,98 +116,26 @@ 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(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Super nice 👌 .. although TimelineAction might no longer be the right name here.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On another note: I wonder if populating those previously empty relations will create any issues 🤔

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you have in mind any particular issue it could cause?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I initially thought of reply fallbacks showing up in reply items if it's a reply to a reply but honestly I don't have enough context around this to know if that's even a possibility. Figured it's better to just ask.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's nothing that comes to mind, but we can watch out on this part during the next few releases 👍

event,
&raw_event,
room_data_provider,
unable_to_decrypt_info,
timeline_items,
meta,
)
.await;

let Some(TimelineAction::AddItem { content }) = action else {
// The event can't be represented as a standalone timeline item.
return Ok(None);
};

let sender = event.sender().to_owned();
let sender_profile = TimelineDetails::from_initial_value(
room_data_provider.profile_from_user_id(&sender).await,
);

Ok(Self { content, sender, sender_profile })
Ok(Some(Self { content, sender, sender_profile }))
}
}
13 changes: 13 additions & 0 deletions crates/matrix-sdk-ui/src/timeline/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -667,6 +668,18 @@ impl Timeline {
Ok(false)
}
}

/// Create a [`RepliedToEvent`] from an arbitrary event, be it in the
/// timeline or not.
///
/// Can be `None` if the event cannot be represented as a standalone item,
/// because it's an aggregation.
pub async fn make_replied_to(
&self,
event: TimelineEvent,
) -> Result<Option<RepliedToEvent>, Error> {
self.controller.make_replied_to(event).await
}
}

/// Test helpers, likely not very useful in production.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(_),
..
})
);
}
}

Expand Down
Loading