mirror/matrix-rust-sdk
1
0
Fork 0
mirror of https://github.com/matrix-org/matrix-rust-sdk.git synced 2026-05-16 12:43:01 -04:00
Code Issues Packages Projects Releases Wiki Activity

timeline: store reaction by key by sender, instead of by key by local or remote id

Browse Source 
This makes it impossible to represent states like "there's a local *and*
a remote echo for the same sender for a given reaction", or multiple
reactions from the same sender to the same event, and so on.
This commit is contained in:
Benjamin Bouvier
2024-07-24 22:51:01 +02:00
committed by Benjamin Bouvier
parent aa4f606171
commit f0015bb10d
13 changed files with 154 additions and 386 deletions
Download Patch File Download Diff File Expand all files Collapse all files

8
bindings/matrix-sdk-ffi/src/timeline/mod.rs
View File

@@ -979,10 +979,10 @@ impl EventTimelineItem {
.map(|(k, v)| Reaction {
key: k.to_owned(),
senders: v
.senders()
.map(|v| ReactionSenderData {
sender_id: v.sender_id.to_string(),
timestamp: v.timestamp.0.into(),
.iter()
.map(|(sender_id, info)| ReactionSenderData {
sender_id: sender_id.to_string(),
timestamp: info.timestamp.0.into(),
})
.collect(),
})

167
crates/matrix-sdk-ui/src/timeline/event_handler.rs
View File

@@ -16,7 +16,7 @@ use std::sync::Arc;
use as_variant::as_variant;
use eyeball_im::{ObservableVectorTransaction, ObservableVectorTransactionEntry};
use indexmap::{map::Entry, IndexMap};
use indexmap::IndexMap;
use matrix_sdk::{
crypto::types::events::UtdCause, deserialized_responses::EncryptionInfo, send_queue::SendHandle,
};
@@ -51,17 +51,20 @@ use tracing::{debug, error, field::debug, info, instrument, trace, warn};
use super::{
day_dividers::DayDividerAdjuster,
event_item::{
AnyOtherFullStateEventContent, BundledReactions, EventSendState, EventTimelineItemKind,
LocalEventTimelineItem, Profile, RemoteEventOrigin, RemoteEventTimelineItem,
TimelineEventItemId,
AnyOtherFullStateEventContent, EventSendState, EventTimelineItemKind,
LocalEventTimelineItem, Profile, ReactionsByKeyBySender, RemoteEventOrigin,
RemoteEventTimelineItem, TimelineEventItemId,
},
inner::{TimelineInnerMetadata, TimelineInnerStateTransaction},
polls::PollState,
util::{rfind_event_by_id, rfind_event_item},
EventTimelineItem, InReplyToDetails, Message, OtherState, ReactionGroup, ReactionSenderData,
Sticker, TimelineDetails, TimelineItem, TimelineItemContent,
EventTimelineItem, InReplyToDetails, Message, OtherState, ReactionSenderData, Sticker,
TimelineDetails, TimelineItem, TimelineItemContent,
};
use crate::{
events::SyncTimelineEventWithoutContent, timeline::event_item::ReactionInfo,
DEFAULT_SANITIZER_MODE,
};
use crate::{events::SyncTimelineEventWithoutContent, DEFAULT_SANITIZER_MODE};
/// When adding an event, useful information related to the source of the event.
#[derive(Clone)]
@@ -424,7 +427,7 @@ impl<'a, 'o> TimelineEventHandler<'a, 'o> {
self.handle_redaction(redacts);
}
TimelineEventKind::LocalRedaction { redacts } => {
self.handle_local_redaction(redacts);
self.handle_reaction_redaction(TimelineEventItemId::TransactionId(redacts));
}
TimelineEventKind::RoomMember { user_id, content, sender } => {
@@ -559,12 +562,9 @@ impl<'a, 'o> TimelineEventHandler<'a, 'o> {
// Add the reaction to event item's bundled reactions.
let mut reactions = remote_event_item.reactions.clone();
reactions.entry(c.relates_to.key.clone()).or_default().0.insert(
reaction_id.clone(),
ReactionSenderData {
sender_id: self.ctx.sender.clone(),
timestamp: self.ctx.timestamp,
},
reactions.entry(c.relates_to.key.clone()).or_default().insert(
self.ctx.sender.clone(),
ReactionInfo { timestamp: self.ctx.timestamp, id: reaction_id.clone() },
);
trace!("Adding reaction");
@@ -710,77 +710,24 @@ impl<'a, 'o> TimelineEventHandler<'a, 'o> {
/// redacts it.
///
/// This only applies to *remote* events; for local items being redacted,
/// use [`Self::handle_local_redaction`].
/// use [`Self::handle_reaction_redaction`].
///
/// This assumes the redacted event was present in the timeline in the first
/// place; it will warn if the redacted event has not been found.
#[instrument(skip_all, fields(redacts_event_id = ?redacts))]
fn handle_redaction(&mut self, redacts: OwnedEventId) {
#[instrument(skip_all, fields(redacts_event_id = ?redacted))]
fn handle_redaction(&mut self, redacted: OwnedEventId) {
// TODO: Apply local redaction of PollResponse and PollEnd events.
// https://github.com/matrix-org/matrix-rust-sdk/pull/2381#issuecomment-1689647825
let id = TimelineEventItemId::EventId(redacts.clone());
// If it's a reaction that's being redacted, handle it here.
if let Some((_, rel)) = self.meta.reactions.map.remove(&id) {
let found_reacted_to = self.update_timeline_item(&rel.event_id, |_this, event_item| {
let Some(remote_event_item) = event_item.as_remote() else {
error!("inconsistent state: redaction received on a non-remote event item");
return None;
};
let mut reactions = remote_event_item.reactions.clone();
let count = {
let Entry::Occupied(mut group_entry) = reactions.entry(rel.key.clone()) else {
return None;
};
let group = group_entry.get_mut();
if group.0.swap_remove(&id).is_none() {
error!(
"inconsistent state: reaction from reaction_map not in reaction list \
of timeline item"
);
return None;
}
group.len()
};
if count == 0 {
reactions.swap_remove(&rel.key);
}
trace!("Removing reaction");
Some(event_item.with_kind(remote_event_item.with_reactions(reactions)))
});
if !found_reacted_to {
debug!("Timeline item not found, discarding reaction redaction");
}
if self.result.items_updated == 0 {
if let Some(reactions) = self.meta.reactions.pending.get_mut(&rel.event_id) {
if !reactions.swap_remove(&redacts.clone()) {
error!(
"inconsistent state: reaction from reaction_map not in reaction list \
of pending_reactions"
);
}
} else {
warn!("reaction_map out of sync with timeline items");
}
}
// Even if the event being redacted is a reaction (found in
// `reaction_map`), it can still be present in the timeline items
// directly with the raw event timeline feature (not yet
// implemented) => no early return here.
if self.handle_reaction_redaction(TimelineEventItemId::EventId(redacted.clone())) {
// When we have raw timeline items, we should not return here anymore, as we
// might need to redact the raw item as well.
return;
}
// General path: redact another kind of (non-reaction) event.
let found_redacted_event = self.update_timeline_item(&redacts, |this, event_item| {
let found_redacted_event = self.update_timeline_item(&redacted, |this, event_item| {
if event_item.as_remote().is_none() {
error!("inconsistent state: redaction received on a non-remote event item");
return None;
@@ -810,7 +757,7 @@ impl<'a, 'o> TimelineEventHandler<'a, 'o> {
let Some(message) = event_item.content.as_message() else { return };
let Some(in_reply_to) = message.in_reply_to() else { return };
let TimelineDetails::Ready(replied_to_event) = &in_reply_to.event else { return };
if redacts == in_reply_to.event_id {
if redacted == in_reply_to.event_id {
let replied_to_event = replied_to_event.redact(&self.meta.room_version);
let in_reply_to = InReplyToDetails {
event_id: in_reply_to.event_id.clone(),
@@ -824,50 +771,57 @@ impl<'a, 'o> TimelineEventHandler<'a, 'o> {
});
}
// Redacted redactions are no-ops (unfortunately)
#[instrument(skip_all, fields(redacts_event_id = ?redacts))]
fn handle_local_redaction(&mut self, redacts: OwnedTransactionId) {
let id = TimelineEventItemId::TransactionId(redacts);
// Redact the reaction, if any.
if let Some((_, rel)) = self.meta.reactions.map.remove(&id) {
let found = self.update_timeline_item(&rel.event_id, |_this, event_item| {
/// Attempts to redact a reaction, local or remote.
///
/// Returns true if it's succeeded.
#[instrument(skip_all, fields(redacts = ?reaction_id))]
fn handle_reaction_redaction(&mut self, reaction_id: TimelineEventItemId) -> bool {
if let Some((_, rel)) = self.meta.reactions.map.remove(&reaction_id) {
let updated_event = self.update_timeline_item(&rel.event_id, |this, event_item| {
let Some(remote_event_item) = event_item.as_remote() else {
error!("inconsistent state: redaction received on a non-remote event item");
return None;
};
let mut reactions = remote_event_item.reactions.clone();
let Entry::Occupied(mut group_entry) = reactions.entry(rel.key.clone()) else {
return None;
};
let group = group_entry.get_mut();
if group.0.swap_remove(&id).is_none() {
error!(
"inconsistent state: reaction from reaction_map not in reaction list \
of timeline item"
);
return None;
}
if group.len() == 0 {
group_entry.swap_remove();
if let Some(by_sender) = reactions.get_mut(&rel.key) {
if let Some(reaction) = by_sender.get(&this.ctx.sender) {
if reaction.id == reaction_id {
by_sender.swap_remove(&this.ctx.sender);
// Remove the reaction group if this was the last reaction.
if by_sender.is_empty() {
reactions.swap_remove(&rel.key);
}
} else {
warn!("Tried to remove a reaction which didn't match the known one.");
}
}
}
trace!("Removing reaction");
Some(event_item.with_kind(remote_event_item.with_reactions(reactions)))
});
if !found {
debug!("Timeline item not found, discarding local redaction");
if !updated_event {
if let TimelineEventItemId::EventId(event_id) = reaction_id {
// If the remote event wasn't in the timeline, remove any possibly pending
// reactions to that event, as this redaction would affect them.
if let Some(reactions) = self.meta.reactions.pending.get_mut(&rel.event_id) {
reactions.swap_remove(&event_id);
}
}
}
return updated_event;
}
if self.result.items_updated == 0 {
// We will want to know this when debugging redaction issues.
error!("redaction affected no event");
}
false
}
/// Add a new event item in the timeline.
@@ -1046,7 +1000,10 @@ impl<'a, 'o> TimelineEventHandler<'a, 'o> {
}
}
fn pending_reactions(&mut self, content: &TimelineItemContent) -> Option<BundledReactions> {
fn pending_reactions(
&mut self,
content: &TimelineItemContent,
) -> Option<ReactionsByKeyBySender> {
// Drop pending reactions if the message is redacted.
if let TimelineItemContent::RedactedMessage = content {
return None;
@@ -1069,9 +1026,13 @@ impl<'a, 'o> TimelineEventHandler<'a, 'o> {
continue;
};
let group: &mut ReactionGroup =
let group: &mut IndexMap<OwnedUserId, ReactionInfo> =
bundled.entry(annotation.key.clone()).or_default();
group.0.insert(reaction_id, reaction_sender_data.clone());
group.insert(
reaction_sender_data.sender_id.clone(),
ReactionInfo { timestamp: reaction_sender_data.timestamp, id: reaction_id },
);
}
Some(bundled)

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

@@ -33,7 +33,6 @@ use tracing::warn;
mod content;
mod local;
mod reactions;
mod remote;
pub use self::{
@@ -43,7 +42,6 @@ pub use self::{
TimelineItemContent,
},
local::EventSendState,
reactions::{BundledReactions, ReactionGroup},
};
pub(super) use self::{
local::LocalEventTimelineItem,
@@ -269,9 +267,9 @@ impl EventTimelineItem {
}
/// Get the reactions of this item.
pub fn reactions(&self) -> &BundledReactions {
pub fn reactions(&self) -> &ReactionsByKeyBySender {
// There's not much of a point in allowing reactions to local echoes.
static EMPTY_REACTIONS: Lazy<BundledReactions> = Lazy::new(Default::default);
static EMPTY_REACTIONS: Lazy<ReactionsByKeyBySender> = Lazy::new(Default::default);
match &self.kind {
EventTimelineItemKind::Local(_) => &EMPTY_REACTIONS,
EventTimelineItemKind::Remote(remote_event) => &remote_event.reactions,
@@ -580,6 +578,19 @@ pub enum EventItemOrigin {
Pagination,
}
/// Information about a single reaction stored in [`ReactionsByKeyBySender`].
#[derive(Clone, Debug)]
pub struct ReactionInfo {
pub timestamp: MilliSecondsSinceUnixEpoch,
pub id: TimelineEventItemId,
}
/// Reactions grouped by key first, then by sender.
///
/// This representation makes sure that a given sender has sent at most one
/// reaction for an event.
pub type ReactionsByKeyBySender = IndexMap<String, IndexMap<OwnedUserId, ReactionInfo>>;
#[cfg(test)]
mod tests {
use assert_matches::assert_matches;

62
crates/matrix-sdk-ui/src/timeline/event_item/reactions.rs
View File

@@ -1,62 +0,0 @@
// Copyright 2023 The Matrix.org Foundation C.I.C.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
use indexmap::IndexMap;
use itertools::Itertools as _;
use ruma::{OwnedEventId, OwnedTransactionId, UserId};
use super::TimelineEventItemId;
use crate::timeline::ReactionSenderData;
/// The reactions grouped by key.
///
/// Key: The reaction, usually an emoji.
/// Value: The group of reactions.
pub type BundledReactions = IndexMap<String, ReactionGroup>;
/// 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) IndexMap<TimelineEventItemId, ReactionSenderData>);
impl ReactionGroup {
/// The (deduplicated) senders of the reactions in this group.
pub fn senders(&self) -> impl Iterator<Item = &ReactionSenderData> {
self.0.values().unique_by(|v| &v.sender_id)
}
/// Returns the number of reactions in this group.
pub fn len(&self) -> usize {
self.0.len()
}
/// All reactions within this reaction group that were sent by the given
/// user.
///
/// Note that it is possible for multiple reactions by the same user to
/// have arrived over federation.
pub fn by_sender<'a>(
&'a self,
user_id: &'a UserId,
) -> impl Iterator<Item = (Option<&OwnedTransactionId>, Option<&OwnedEventId>)> + 'a {
self.0.iter().filter_map(move |(k, v)| {
(v.sender_id == user_id).then_some(match k {
TimelineEventItemId::TransactionId(txn_id) => (Some(txn_id), None),
TimelineEventItemId::EventId(event_id) => (None, Some(event_id)),
})
})
}
}

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

@@ -22,7 +22,7 @@ use ruma::{
OwnedEventId, OwnedTransactionId, OwnedUserId,
};
use super::BundledReactions;
use super::ReactionsByKeyBySender;
/// An item for an event that was received from the homeserver.
#[derive(Clone)]
@@ -34,7 +34,7 @@ pub(in crate::timeline) struct RemoteEventTimelineItem {
pub transaction_id: Option<OwnedTransactionId>,
/// All bundled reactions about the event.
pub reactions: BundledReactions,
pub reactions: ReactionsByKeyBySender,
/// All read receipts for the event.
///
@@ -74,7 +74,7 @@ pub(in crate::timeline) struct RemoteEventTimelineItem {
impl RemoteEventTimelineItem {
/// Clone the current event item, and update its `reactions`.
pub fn with_reactions(&self, reactions: BundledReactions) -> Self {
pub fn with_reactions(&self, reactions: ReactionsByKeyBySender) -> Self {
Self { reactions, ..self.clone() }
}
@@ -82,7 +82,7 @@ impl RemoteEventTimelineItem {
/// JSON representation fields.
pub fn redact(&self) -> Self {
Self {
reactions: BundledReactions::default(),
reactions: ReactionsByKeyBySender::default(),
original_json: None,
latest_edit_json: None,
..self.clone()

23
crates/matrix-sdk-ui/src/timeline/inner/mod.rs
View File

@@ -21,7 +21,6 @@ use eyeball_im::{ObservableVectorEntry, VectorDiff};
use eyeball_im_util::vector::VectorObserverExt;
use futures_core::Stream;
use imbl::Vector;
use itertools::Itertools;
#[cfg(all(test, feature = "e2e-encryption"))]
use matrix_sdk::crypto::OlmMachine;
use matrix_sdk::{
@@ -62,8 +61,8 @@ use super::{
traits::RoomDataProvider,
util::{rfind_event_by_id, rfind_event_item, RelativePosition},
Error, EventSendState, EventTimelineItem, InReplyToDetails, Message, PaginationError, Profile,
RepliedToEvent, TimelineDetails, TimelineFocus, TimelineItem, TimelineItemContent,
TimelineItemKind,
RepliedToEvent, TimelineDetails, TimelineEventItemId, TimelineFocus, TimelineItem,
TimelineItemContent, TimelineItemKind,
};
use crate::{
timeline::{day_dividers::DayDividerAdjuster, TimelineEventFilterFn},
@@ -409,17 +408,13 @@ impl<P: RoomDataProvider> TimelineInner<P> {
};
let (local_echo_txn_id, remote_echo_event_id) = {
let reactions = related_event.reactions();
let user_reactions =
reactions.get(&annotation.key).map(|group| group.by_sender(user_id));
user_reactions
.map(|reactions| {
let reactions = reactions.collect_vec();
let local = reactions.iter().find_map(|(txid, _event_id)| *txid);
let remote = reactions.iter().find_map(|(_txid, event_id)| *event_id);
(local, remote)
related_event
.reactions()
.get(&annotation.key)
.and_then(|group| group.get(user_id))
.map(|reaction_info| match &reaction_info.id {
TimelineEventItemId::TransactionId(txn_id) => (Some(txn_id), None),
TimelineEventItemId::EventId(event_id) => (None, Some(event_id)),
})
.unwrap_or((None, None))
};

48
crates/matrix-sdk-ui/src/timeline/inner/state.rs
View File

@@ -37,7 +37,7 @@ use crate::{
Flow, HandleEventResult, TimelineEventContext, TimelineEventHandler, TimelineEventKind,
TimelineItemPosition,
},
event_item::{RemoteEventOrigin, TimelineEventItemId},
event_item::{ReactionInfo, RemoteEventOrigin, TimelineEventItemId},
polls::PollPendingEvents,
reactions::{ReactionToggleResult, Reactions},
read_receipts::ReadReceipts,
@@ -277,42 +277,36 @@ impl TimelineInnerState {
error!("inconsistent state: reaction received on a non-remote event item");
return Err(TimelineError::FailedToToggleReaction);
};
// Note: remote event is not synced yet, so we're adding an item
// with the local timestamp.
let reaction_sender_data = ReactionSenderData {
sender_id: own_user_id.to_owned(),
timestamp: MilliSecondsSinceUnixEpoch::now(),
};
let now = MilliSecondsSinceUnixEpoch::now();
let reaction_sender_data =
ReactionSenderData { sender_id: own_user_id.to_owned(), timestamp: now };
let new_reactions = {
let mut reactions = remote_related.reactions.clone();
let reaction_group = reactions.entry(annotation.key.clone()).or_default();
let reaction_by_sender = reactions.entry(annotation.key.clone()).or_default();
// Remove the local echo from the related event.
if let Some(txn_id) = local_echo_to_remove {
let id = TimelineEventItemId::TransactionId(txn_id.clone());
if reaction_group.0.swap_remove(&id).is_none() {
warn!(
"Tried to remove reaction by transaction ID, but didn't \
find matching reaction in the related event's reactions"
);
}
}
// Add the remote echo to the related event
if let Some(event_id) = remote_echo_to_add {
reaction_group.0.insert(
TimelineEventItemId::EventId(event_id.clone()),
reaction_sender_data.clone(),
reaction_by_sender.insert(
own_user_id.to_owned(),
ReactionInfo {
// Note: remote event is not synced yet, so we're adding an item
// with the local timestamp.
timestamp: now,
id: TimelineEventItemId::EventId(event_id.clone()),
},
);
};
if reaction_group.0.is_empty() {
reactions.swap_remove(&annotation.key);
} else if local_echo_to_remove.is_some() {
reaction_by_sender.swap_remove(own_user_id);
// Remove the group if we were the last reaction.
if reaction_by_sender.is_empty() {
reactions.swap_remove(&annotation.key);
}
}
reactions
};
let new_related = related.with_kind(remote_related.with_reactions(new_reactions));
// Update the reactions stored in the timeline state

8
crates/matrix-sdk-ui/src/timeline/mod.rs
View File

@@ -87,10 +87,10 @@ pub use self::{
builder::TimelineBuilder,
error::*,
event_item::{
AnyOtherFullStateEventContent, BundledReactions, EncryptedMessage, EventItemOrigin,
EventSendState, EventTimelineItem, InReplyToDetails, MemberProfileChange, MembershipChange,
Message, OtherState, Profile, ReactionGroup, RepliedToEvent, RoomMembershipChange, Sticker,
TimelineDetails, TimelineEventItemId, TimelineItemContent,
AnyOtherFullStateEventContent, EncryptedMessage, EventItemOrigin, EventSendState,
EventTimelineItem, InReplyToDetails, MemberProfileChange, MembershipChange, Message,
OtherState, Profile, RepliedToEvent, RoomMembershipChange, Sticker, TimelineDetails,
TimelineEventItemId, TimelineItemContent,
},
event_type_filter::TimelineEventTypeFilter,
inner::default_event_filter,

1
crates/matrix-sdk-ui/src/timeline/tests/mod.rs
View File

@@ -68,7 +68,6 @@ mod encryption;
mod event_filter;
mod invalid;
mod polls;
mod reaction_group;
mod reactions;
mod read_receipts;
mod redaction;

131
crates/matrix-sdk-ui/src/timeline/tests/reaction_group.rs
View File

@@ -1,131 +0,0 @@
// Copyright 2023 The Matrix.org Foundation C.I.C.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
use assert_matches2::assert_let;
use itertools::Itertools;
use matrix_sdk_test::{ALICE, BOB};
use ruma::{server_name, uint, user_id, EventId, MilliSecondsSinceUnixEpoch, OwnedUserId, UserId};
use crate::timeline::{event_item::TimelineEventItemId, ReactionGroup, ReactionSenderData};
#[test]
fn test_by_sender() {
let alice = ALICE.to_owned();
let bob = BOB.to_owned();
let reaction_1 = new_reaction();
let reaction_2 = new_reaction();
let mut reaction_group = ReactionGroup::default();
reaction_group.0.insert(reaction_1.clone(), new_sender_data(alice.clone()));
reaction_group.0.insert(reaction_2, new_sender_data(bob));
let alice_reactions = reaction_group.by_sender(&alice).collect::<Vec<_>>();
let reaction = alice_reactions[0];
assert_let!(TimelineEventItemId::EventId(event_id) = reaction_1);
assert_eq!(reaction.1.unwrap(), &event_id);
}
#[test]
fn test_by_sender_with_empty_group() {
let reaction_group = ReactionGroup::default();
let reactions = reaction_group.by_sender(&ALICE).collect::<Vec<_>>();
assert!(reactions.is_empty());
}
#[test]
fn test_by_sender_with_multiple_users() {
let alice = ALICE.to_owned();
let bob = BOB.to_owned();
let carol = user_id!("@carol:other.server");
let reaction_1 = new_reaction();
let reaction_2 = new_reaction();
let reaction_3 = new_reaction();
let mut reaction_group = ReactionGroup::default();
reaction_group.0.insert(reaction_1, new_sender_data(alice.clone()));
reaction_group.0.insert(reaction_2, new_sender_data(alice.clone()));
reaction_group.0.insert(reaction_3, new_sender_data(bob.clone()));
let alice_reactions = reaction_group.by_sender(&alice).collect::<Vec<_>>();
let bob_reactions = reaction_group.by_sender(&bob).collect::<Vec<_>>();
let carol_reactions = reaction_group.by_sender(carol).collect::<Vec<_>>();
assert_eq!(alice_reactions.len(), 2);
assert_eq!(bob_reactions.len(), 1);
assert!(carol_reactions.is_empty());
}
/// The Matrix spec does not allow duplicate annotations to be created but it
/// is still possible for duplicates to be received over federation. And in
/// that case, clients are expected to treat duplicates as a single annotation.
#[test]
fn test_senders_are_deduplicated() {
let group = {
let mut group = ReactionGroup::default();
insert(&mut group, &ALICE, 3);
insert(&mut group, &BOB, 2);
group
};
let senders = group.senders().map(|v| &v.sender_id).collect::<Vec<_>>();
assert_eq!(senders, vec![&ALICE.to_owned(), &BOB.to_owned()]);
}
#[test]
fn test_timestamps_are_stored() {
let reaction = new_reaction();
let reaction_2 = new_reaction();
let timestamp = MilliSecondsSinceUnixEpoch(uint!(0));
let timestamp_2 = MilliSecondsSinceUnixEpoch::now();
let mut reaction_group = ReactionGroup::default();
reaction_group
.0
.insert(reaction, ReactionSenderData { sender_id: ALICE.to_owned(), timestamp });
reaction_group.0.insert(
reaction_2,
ReactionSenderData { sender_id: BOB.to_owned(), timestamp: timestamp_2 },
);
assert_eq!(
reaction_group.senders().map(|v| v.timestamp).collect_vec(),
vec![timestamp, timestamp_2]
);
}
fn insert(group: &mut ReactionGroup, sender: &UserId, count: u64) {
for _ in 0..count {
group.0.insert(
new_reaction(),
ReactionSenderData {
sender_id: sender.to_owned(),
timestamp: MilliSecondsSinceUnixEpoch::now(),
},
);
}
}
fn new_reaction() -> TimelineEventItemId {
let event_id = EventId::new(server_name!("example.org"));
TimelineEventItemId::EventId(event_id)
}
fn new_sender_data(sender: OwnedUserId) -> ReactionSenderData {
ReactionSenderData { sender_id: sender, timestamp: MilliSecondsSinceUnixEpoch::now() }
}

41
crates/matrix-sdk-ui/src/timeline/tests/reactions.rs
View File

@@ -19,12 +19,11 @@ use assert_matches2::assert_let;
use eyeball_im::VectorDiff;
use futures_core::Stream;
use matrix_sdk::test_utils::events::EventFactory;
use matrix_sdk_base::deserialized_responses::SyncTimelineEvent;
use matrix_sdk_test::{async_test, ALICE, BOB};
use ruma::{
event_id,
events::{relation::Annotation, room::message::RoomMessageEventContent},
server_name, uint, EventId, MilliSecondsSinceUnixEpoch, OwnedEventId, TransactionId,
server_name, uint, EventId, MilliSecondsSinceUnixEpoch, OwnedEventId, TransactionId, UserId,
};
use stream_assert::assert_next_matches;
@@ -33,7 +32,7 @@ use crate::timeline::{
inner::TimelineEnd,
reactions::{ReactionAction, ReactionToggleResult},
tests::{assert_event_is_updated, assert_no_more_updates, TestTimeline},
TimelineItem,
TimelineEventItemId, TimelineItem,
};
const REACTION_KEY: &str = "👍";
@@ -230,7 +229,7 @@ async fn test_reactions_store_timestamp() {
let _ = timeline.toggle_reaction_local(&reaction).await.unwrap();
let event = assert_event_is_updated(&mut stream, &msg_id, msg_pos).await;
let reactions = event.reactions().get(&REACTION_KEY.to_owned()).unwrap();
let timestamp = reactions.senders().next().unwrap().timestamp;
let timestamp = reactions.values().next().unwrap().timestamp;
assert!(timestamp_range_until_now_from(timestamp_before).contains(&timestamp));
// Failing a redaction.
@@ -247,7 +246,7 @@ async fn test_reactions_store_timestamp() {
// Restores an event with a valid timestamp.
let event = assert_event_is_updated(&mut stream, &msg_id, msg_pos).await;
let reactions = event.reactions().get(&REACTION_KEY.to_owned()).unwrap();
let new_timestamp = reactions.senders().next().unwrap().timestamp;
let new_timestamp = reactions.values().next().unwrap().timestamp;
assert!(timestamp_range_until_now_from(timestamp_before).contains(&new_timestamp));
}
@@ -255,6 +254,7 @@ async fn test_reactions_store_timestamp() {
async fn test_initial_reaction_timestamp_is_stored() {
let timeline = TestTimeline::new();
let f = EventFactory::new().sender(*ALICE);
let message_event_id = EventId::new(server_name!("dummy.server"));
let reaction_timestamp = MilliSecondsSinceUnixEpoch(uint!(39845));
@@ -262,16 +262,12 @@ async fn test_initial_reaction_timestamp_is_stored() {
.inner
.add_events_at(
vec![
SyncTimelineEvent::new(timeline.event_builder.make_sync_reaction(
*ALICE,
&Annotation::new(message_event_id.clone(), REACTION_KEY.to_owned()),
reaction_timestamp,
)),
SyncTimelineEvent::new(timeline.event_builder.make_sync_message_event_with_id(
*ALICE,
&message_event_id,
RoomMessageEventContent::text_plain("A"),
)),
// Reaction comes first.
f.reaction(&message_event_id, REACTION_KEY.to_owned())
.server_ts(reaction_timestamp)
.into_sync(),
// Event comes next.
f.text_msg("A").event_id(&message_event_id).into_sync(),
],
TimelineEnd::Back,
RemoteEventOrigin::Sync,
@@ -282,7 +278,7 @@ async fn test_initial_reaction_timestamp_is_stored() {
let reactions = items.last().unwrap().as_event().unwrap().reactions();
let entry = reactions.get(&REACTION_KEY.to_owned()).unwrap();
assert_eq!(reaction_timestamp, entry.senders().next().unwrap().timestamp);
assert_eq!(reaction_timestamp, entry.values().next().unwrap().timestamp);
}
fn create_reaction(related_message_id: &EventId) -> Annotation {
@@ -317,12 +313,15 @@ async fn assert_reaction_is_updated(
expected_event_id: Option<&EventId>,
expected_txn_id: Option<&TransactionId>,
) {
let own_user_id = &ALICE;
let own_user_id: &UserId = &ALICE;
let event = assert_event_is_updated(stream, related_to, message_position).await;
let (reaction_tx_id, reaction_event_id) = {
let reactions = event.reactions().get(&REACTION_KEY.to_owned()).unwrap();
let reaction = reactions.by_sender(own_user_id).next().unwrap();
reaction.to_owned()
let reaction = reactions.get(own_user_id).unwrap();
match &reaction.id {
TimelineEventItemId::TransactionId(txn_id) => (Some(txn_id), None),
TimelineEventItemId::EventId(event_id) => (None, Some(event_id)),
}
};
assert_eq!(reaction_tx_id, expected_txn_id.map(|it| it.to_owned()).as_ref());
assert_eq!(reaction_event_id, expected_event_id.map(|it| it.to_owned()).as_ref());
@@ -333,10 +332,10 @@ async fn assert_reaction_is_added(
related_to: &EventId,
message_position: usize,
) {
let own_user_id = &ALICE;
let own_user_id: &UserId = &ALICE;
let event = assert_event_is_updated(stream, related_to, message_position).await;
let reactions = event.reactions().get(&REACTION_KEY.to_owned()).unwrap();
assert!(reactions.by_sender(own_user_id).next().is_some());
assert!(reactions.get(own_user_id).is_some());
}
async fn assert_reactions_are_removed(

2
crates/matrix-sdk-ui/tests/integration/timeline/mod.rs
View File

@@ -127,7 +127,7 @@ async fn test_reaction() {
assert_eq!(event_item.reactions().len(), 1);
let group = &event_item.reactions()["👍"];
assert_eq!(group.len(), 1);
let senders: Vec<_> = group.senders().map(|v| &v.sender_id).collect();
let senders: Vec<_> = group.keys().collect();
assert_eq!(senders.as_slice(), [user_id!("@bob:example.org")]);
// The day divider.

22
testing/matrix-sdk-integration-testing/src/tests/timeline.rs
View File

@@ -26,7 +26,9 @@ use matrix_sdk::ruma::{
events::{relation::Annotation, room::message::RoomMessageEventContent},
EventId, MilliSecondsSinceUnixEpoch, UserId,
};
use matrix_sdk_ui::timeline::{EventSendState, EventTimelineItem, RoomExt, TimelineItem};
use matrix_sdk_ui::timeline::{
EventSendState, EventTimelineItem, RoomExt, TimelineEventItemId, TimelineItem,
};
use tokio::{
spawn,
task::JoinHandle,
@@ -195,8 +197,11 @@ async fn assert_local_added(
let (reaction_tx_id, reaction_event_id) = {
let reactions = event.reactions().get(&reaction.key).unwrap();
let reaction = reactions.by_sender(user_id).next().unwrap();
reaction.to_owned()
let reaction = reactions.get(user_id).unwrap();
match &reaction.id {
TimelineEventItemId::TransactionId(txn_id) => (Some(txn_id), None),
TimelineEventItemId::EventId(event_id) => (None, Some(event_id)),
}
};
assert_matches!(reaction_tx_id, Some(_));
// Event ID hasn't been received from homeserver yet
@@ -222,18 +227,15 @@ async fn assert_remote_added(
let event = assert_event_is_updated(stream, event_id, message_position).await;
let reactions = event.reactions().get(&reaction.key).unwrap();
assert_eq!(reactions.senders().count(), 1);
assert_eq!(reactions.keys().count(), 1);
let reaction = reactions.by_sender(user_id).next().unwrap();
let (reaction_tx_id, reaction_event_id) = reaction;
assert_matches!(reaction_tx_id, None);
assert_matches!(reaction_event_id, Some(value) => value);
let reaction = reactions.get(user_id).unwrap();
assert_matches!(reaction.id, TimelineEventItemId::EventId(..));
// Remote event should have a timestamp <= than now.
// Note: this can actually be equal because if the timestamp from
// server is not available, it might be created with a local call to `now()`
let reaction = reactions.senders().next();
assert!(reaction.unwrap().timestamp <= MilliSecondsSinceUnixEpoch::now());
assert!(reaction.timestamp <= MilliSecondsSinceUnixEpoch::now());
}
async fn assert_event_is_updated(