Skip to content
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

ui/ffi: add timestamp to reaction group senders #2153

Merged
merged 11 commits into from
Jul 10, 2023
18 changes: 12 additions & 6 deletions bindings/matrix-sdk-ffi/src/timeline.rs
Original file line number Diff line number Diff line change
Expand Up @@ -324,7 +324,13 @@ impl EventTimelineItem {
.map(|(k, v)| Reaction {
key: k.to_owned(),
count: v.len().try_into().unwrap(),
senders: v.senders().map(ToString::to_string).collect(),
senders: v
.senders()
.map(|v| ReactionSenderData {
sender_id: v.sender_id.to_string(),
timestamp: v.timestamp.0.into(),
})
.collect(),
})
.collect()
}
Expand Down Expand Up @@ -1082,13 +1088,13 @@ impl EncryptedMessage {
pub struct Reaction {
pub key: String,
pub count: u64,
pub senders: Vec<String>,
pub senders: Vec<ReactionSenderData>,
}

#[derive(Clone)]
pub struct ReactionDetails {
pub id: String,
pub sender: String,
#[derive(Clone, uniffi::Record)]
pub struct ReactionSenderData {
pub sender_id: String,
pub timestamp: u64,
}

/// A [`TimelineItem`](super::TimelineItem) that doesn't correspond to an event.
Expand Down
63 changes: 39 additions & 24 deletions crates/matrix-sdk-ui/src/timeline/event_handler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,16 +42,17 @@ use tracing::{debug, error, field::debug, info, instrument, trace, warn};

use super::{
event_item::{
AnyOtherFullStateEventContent, BundledReactions, EventSendState, EventTimelineItemKind,
LocalEventTimelineItem, Profile, RemoteEventOrigin, RemoteEventTimelineItem,
AnyOtherFullStateEventContent, BundledReactions, EventItemIdentifier, EventSendState,
EventTimelineItemKind, LocalEventTimelineItem, Profile, RemoteEventOrigin,
RemoteEventTimelineItem,
},
find_read_marker,
read_receipts::maybe_add_implicit_read_receipt,
rfind_event_by_id, rfind_event_item, EventTimelineItem, Message, OtherState, ReactionGroup,
Sticker, TimelineDetails, TimelineInnerState, TimelineItem, TimelineItemContent,
VirtualTimelineItem, DEFAULT_SANITIZER_MODE,
};
use crate::events::SyncTimelineEventWithoutContent;
use crate::{events::SyncTimelineEventWithoutContent, timeline::event_item::ReactionSenderData};

#[derive(Clone)]
pub(super) enum Flow {
Expand Down Expand Up @@ -208,11 +209,7 @@ pub(super) struct TimelineEventHandler<'a> {
meta: TimelineEventMetadata,
flow: Flow,
items: &'a mut ObservableVector<Arc<TimelineItem>>,
#[allow(clippy::type_complexity)]
reaction_map: &'a mut HashMap<
(Option<OwnedTransactionId>, Option<OwnedEventId>),
(OwnedUserId, Annotation),
>,
reaction_map: &'a mut HashMap<EventItemIdentifier, (ReactionSenderData, Annotation)>,
pending_reactions: &'a mut HashMap<OwnedEventId, IndexSet<OwnedEventId>>,
fully_read_event: &'a mut Option<OwnedEventId>,
event_should_update_fully_read_marker: &'a mut bool,
Expand Down Expand Up @@ -433,9 +430,11 @@ impl<'a> TimelineEventHandler<'a> {
fn handle_reaction(&mut self, c: ReactionEventContent) {
let event_id: &EventId = &c.relates_to.event_id;
let (reaction_id, old_txn_id) = match &self.flow {
Flow::Local { txn_id, .. } => ((Some(txn_id.clone()), None), None),
Flow::Local { txn_id, .. } => {
(EventItemIdentifier::TransactionId(txn_id.clone()), None)
}
Flow::Remote { event_id, txn_id, .. } => {
((None, Some(event_id.clone())), txn_id.as_ref())
(EventItemIdentifier::EventId(event_id.clone()), txn_id.as_ref())
}
};

Expand All @@ -455,15 +454,22 @@ impl<'a> TimelineEventHandler<'a> {
let reaction_group = reactions.entry(c.relates_to.key.clone()).or_default();

if let Some(txn_id) = old_txn_id {
let id = EventItemIdentifier::TransactionId(txn_id.clone());
// Remove the local echo from the related event.
if reaction_group.0.remove(&(Some(txn_id.clone()), None)).is_none() {
if reaction_group.0.remove(&id).is_none() {
warn!(
"Received reaction with transaction ID, but didn't \
find matching reaction in the related event's reactions"
);
}
}
reaction_group.0.insert(reaction_id.clone(), self.meta.sender.clone());
reaction_group.0.insert(
reaction_id.clone(),
ReactionSenderData {
sender_id: self.meta.sender.clone(),
timestamp: self.meta.timestamp,
},
);

trace!("Adding reaction");
self.items.set(
Expand All @@ -476,26 +482,31 @@ impl<'a> TimelineEventHandler<'a> {
}
} else {
trace!("Timeline item not found, adding reaction to the pending list");
let (None, Some(reaction_event_id)) = &reaction_id else {
let EventItemIdentifier::EventId(reaction_event_id) = reaction_id.clone() else {
error!("Adding local reaction echo to event absent from the timeline");
return;
};

let pending = self.pending_reactions.entry(event_id.to_owned()).or_default();

pending.insert(reaction_event_id.clone());
pending.insert(reaction_event_id);
}

if let Flow::Remote { txn_id: Some(txn_id), .. } = &self.flow {
let id = EventItemIdentifier::TransactionId(txn_id.clone());
// Remove the local echo from the reaction map.
if self.reaction_map.remove(&(Some(txn_id.clone()), None)).is_none() {
if self.reaction_map.remove(&id).is_none() {
warn!(
"Received reaction with transaction ID, but didn't \
find matching reaction in reaction_map"
);
}
}
self.reaction_map.insert(reaction_id, (self.meta.sender.clone(), c.relates_to));
let reaction_sender_data = ReactionSenderData {
sender_id: self.meta.sender.clone(),
timestamp: self.meta.timestamp,
};
self.reaction_map.insert(reaction_id, (reaction_sender_data, c.relates_to));
}

#[instrument(skip_all)]
Expand All @@ -507,7 +518,8 @@ impl<'a> TimelineEventHandler<'a> {
// Redacted redactions are no-ops (unfortunately)
#[instrument(skip_all, fields(redacts_event_id = ?redacts))]
fn handle_redaction(&mut self, redacts: OwnedEventId, _content: RoomRedactionEventContent) {
if let Some((_, rel)) = self.reaction_map.remove(&(None, Some(redacts.clone()))) {
let id = EventItemIdentifier::EventId(redacts.clone());
if let Some((_, rel)) = self.reaction_map.remove(&id) {
update_timeline_item!(self, &rel.event_id, "redaction", |event_item| {
let Some(remote_event_item) = event_item.as_remote() else {
error!("inconsistent state: redaction received on a non-remote event item");
Expand All @@ -522,7 +534,7 @@ impl<'a> TimelineEventHandler<'a> {
};
let group = group_entry.get_mut();

if group.0.remove(&(None, Some(redacts.clone()))).is_none() {
if group.0.remove(&id).is_none() {
error!(
"inconsistent state: reaction from reaction_map not in reaction list \
of timeline item"
Expand All @@ -543,7 +555,7 @@ impl<'a> TimelineEventHandler<'a> {

if self.result.items_updated == 0 {
if let Some(reactions) = self.pending_reactions.get_mut(&rel.event_id) {
if !reactions.remove(&redacts) {
if !reactions.remove(&redacts.clone()) {
error!(
"inconsistent state: reaction from reaction_map not in reaction list \
of pending_reactions"
Expand Down Expand Up @@ -589,7 +601,8 @@ impl<'a> TimelineEventHandler<'a> {
redacts: OwnedTransactionId,
_content: RoomRedactionEventContent,
) {
if let Some((_, rel)) = self.reaction_map.remove(&(Some(redacts.clone()), None)) {
let id = EventItemIdentifier::TransactionId(redacts);
if let Some((_, rel)) = self.reaction_map.remove(&id) {
update_timeline_item!(self, &rel.event_id, "redaction", |event_item| {
let Some(remote_event_item) = event_item.as_remote() else {
error!("inconsistent state: redaction received on a non-remote event item");
Expand All @@ -602,7 +615,7 @@ impl<'a> TimelineEventHandler<'a> {
};
let group = group_entry.get_mut();

if group.0.remove(&(Some(redacts.clone()), None)).is_none() {
if group.0.remove(&id).is_none() {
error!(
"inconsistent state: reaction from reaction_map not in reaction list \
of timeline item"
Expand Down Expand Up @@ -941,8 +954,10 @@ impl<'a> TimelineEventHandler<'a> {
let mut bundled = IndexMap::new();

for reaction_event_id in reactions {
let reaction_id = (None, Some(reaction_event_id));
let Some((sender, annotation)) = self.reaction_map.get(&reaction_id) else {
let reaction_id = EventItemIdentifier::EventId(reaction_event_id);
let Some((reaction_sender_data, annotation)) =
self.reaction_map.get(&reaction_id)
else {
error!(
"inconsistent state: reaction from pending_reactions not in reaction_map"
);
Expand All @@ -951,7 +966,7 @@ impl<'a> TimelineEventHandler<'a> {

let group: &mut ReactionGroup =
bundled.entry(annotation.key.clone()).or_default();
group.0.insert(reaction_id, sender.clone());
group.0.insert(reaction_id, reaction_sender_data.clone());
}

Some(bundled)
Expand Down
23 changes: 11 additions & 12 deletions crates/matrix-sdk-ui/src/timeline/event_item/content.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ use ruma::{
};
use tracing::{debug, error};

use super::{EventTimelineItem, Profile, TimelineDetails};
use super::{EventItemIdentifier, EventTimelineItem, Profile, ReactionSenderData, TimelineDetails};
use crate::timeline::{
traits::RoomDataProvider, Error as TimelineError, TimelineItem, DEFAULT_SANITIZER_MODE,
};
Expand Down Expand Up @@ -475,22 +475,17 @@ impl From<RoomEncryptedEventContent> for EncryptedMessage {
/// Key: The reaction, usually an emoji.\
/// Value: The group of reactions.
pub type BundledReactions = IndexMap<String, ReactionGroup>;

// The long type after a long visibility specified trips up rustfmt currently.
// This works around. Report: https://github.com/rust-lang/rustfmt/issues/5703
type ReactionGroupInner = IndexMap<(Option<OwnedTransactionId>, Option<OwnedEventId>), OwnedUserId>;

/// A group of reaction events on the same event with the same key.
///
/// This is a map of the event ID or transaction ID of the reactions to the ID
/// of the sender of the reaction.
#[derive(Clone, Debug, Default)]
pub struct ReactionGroup(pub(in crate::timeline) ReactionGroupInner);
pub struct ReactionGroup(pub(in crate::timeline) IndexMap<EventItemIdentifier, ReactionSenderData>);

impl ReactionGroup {
/// The (deduplicated) senders of the reactions in this group.
pub fn senders(&self) -> impl Iterator<Item = &UserId> {
self.values().unique().map(AsRef::as_ref)
pub fn senders(&self) -> impl Iterator<Item = &ReactionSenderData> {
self.values().unique_by(|v| &v.sender_id)
}

/// All reactions within this reaction group that were sent by the given
Expand All @@ -502,13 +497,17 @@ impl ReactionGroup {
&'a self,
user_id: &'a UserId,
) -> impl Iterator<Item = (Option<&OwnedTransactionId>, Option<&OwnedEventId>)> + 'a {
self.iter()
.filter_map(move |(k, v)| (*v == user_id).then_some((k.0.as_ref(), k.1.as_ref())))
self.iter().filter_map(move |(k, v)| {
(v.sender_id == user_id).then_some(match k {
EventItemIdentifier::TransactionId(txn_id) => (Some(txn_id), None),
EventItemIdentifier::EventId(event_id) => (None, Some(event_id)),
})
})
}
}

impl Deref for ReactionGroup {
type Target = IndexMap<(Option<OwnedTransactionId>, Option<OwnedEventId>), OwnedUserId>;
type Target = IndexMap<EventItemIdentifier, ReactionSenderData>;

fn deref(&self) -> &Self::Target {
&self.0
Expand Down
21 changes: 20 additions & 1 deletion crates/matrix-sdk-ui/src/timeline/event_item/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,8 @@ use once_cell::sync::Lazy;
use ruma::{
events::{receipt::Receipt, room::message::MessageType, AnySyncTimelineEvent},
serde::Raw,
EventId, MilliSecondsSinceUnixEpoch, OwnedMxcUri, OwnedUserId, TransactionId, UserId,
EventId, MilliSecondsSinceUnixEpoch, OwnedEventId, OwnedMxcUri, OwnedTransactionId,
OwnedUserId, TransactionId, UserId,
};

mod content;
Expand Down Expand Up @@ -67,6 +68,24 @@ pub(super) enum EventTimelineItemKind {
Remote(RemoteEventTimelineItem),
}

/// A wrapper that can contain either a transaction id, or an event id.
#[derive(Clone, Debug, Eq, Hash, PartialEq)]
pub enum EventItemIdentifier {
aringenbach marked this conversation as resolved.
Show resolved Hide resolved
TransactionId(OwnedTransactionId),
EventId(OwnedEventId),
}

/// Data associated with a reaction sender. It can be used to display
/// a details UI component for a reaction with both sender
/// names and the date at which they sent a reaction.
#[derive(Clone, Debug, Eq, PartialEq)]
pub struct ReactionSenderData {
/// Sender identifier.
pub sender_id: OwnedUserId,
/// Date at which the sender reacted.
pub timestamp: MilliSecondsSinceUnixEpoch,
}

impl EventTimelineItem {
pub(super) fn new(
sender: OwnedUserId,
Expand Down
Loading