From abd508676ef8bf7b529b237baa6d036a4aa8d4f4 Mon Sep 17 00:00:00 2001 From: Jonas Platte Date: Wed, 19 Apr 2023 10:37:36 +0200 Subject: [PATCH] sdk: Remove {Local,Remote}EventTimelineItem from public API --- bindings/matrix-sdk-ffi/src/timeline.rs | 4 +- .../src/room/timeline/event_item/local.rs | 6 +-- .../src/room/timeline/event_item/mod.rs | 42 ++++++++++++++----- .../src/room/timeline/event_item/remote.rs | 16 +++---- crates/matrix-sdk/src/room/timeline/mod.rs | 6 +-- .../tests/integration/room/timeline/mod.rs | 16 +++---- .../room/timeline/read_receipts.rs | 14 +++---- .../sliding-sync-integration-test/src/lib.rs | 6 +-- 8 files changed, 63 insertions(+), 47 deletions(-) diff --git a/bindings/matrix-sdk-ffi/src/timeline.rs b/bindings/matrix-sdk-ffi/src/timeline.rs index 849e52463..ab836fe6f 100644 --- a/bindings/matrix-sdk-ffi/src/timeline.rs +++ b/bindings/matrix-sdk-ffi/src/timeline.rs @@ -243,11 +243,11 @@ pub struct EventTimelineItem(pub(crate) matrix_sdk::room::timeline::EventTimelin #[uniffi::export] impl EventTimelineItem { pub fn is_local(&self) -> bool { - self.0.as_local().is_some() + self.0.is_local_echo() } pub fn is_remote(&self) -> bool { - self.0.as_remote().is_some() + !self.0.is_local_echo() } pub fn unique_identifier(&self) -> String { diff --git a/crates/matrix-sdk/src/room/timeline/event_item/local.rs b/crates/matrix-sdk/src/room/timeline/event_item/local.rs index 5c5f5229f..313c05cc8 100644 --- a/crates/matrix-sdk/src/room/timeline/event_item/local.rs +++ b/crates/matrix-sdk/src/room/timeline/event_item/local.rs @@ -5,7 +5,7 @@ use super::EventSendState; /// An item for an event that was created locally and not yet echoed back by /// the homeserver. #[derive(Debug, Clone)] -pub struct LocalEventTimelineItem { +pub(in crate::room::timeline) struct LocalEventTimelineItem { /// The send state of this local event. send_state: EventSendState, /// The transaction ID. @@ -15,7 +15,7 @@ pub struct LocalEventTimelineItem { } impl LocalEventTimelineItem { - pub(in crate::room::timeline) fn new( + pub fn new( send_state: EventSendState, transaction_id: OwnedTransactionId, timestamp: MilliSecondsSinceUnixEpoch, @@ -50,7 +50,7 @@ impl LocalEventTimelineItem { } /// Clone the current event item, and update its `send_state`. - pub(in crate::room::timeline) fn with_send_state(&self, send_state: EventSendState) -> Self { + pub fn with_send_state(&self, send_state: EventSendState) -> Self { Self { send_state, ..self.clone() } } } diff --git a/crates/matrix-sdk/src/room/timeline/event_item/mod.rs b/crates/matrix-sdk/src/room/timeline/event_item/mod.rs index 36b146ea2..1f0d776dc 100644 --- a/crates/matrix-sdk/src/room/timeline/event_item/mod.rs +++ b/crates/matrix-sdk/src/room/timeline/event_item/mod.rs @@ -14,10 +14,11 @@ use std::sync::Arc; +use indexmap::IndexMap; use matrix_sdk_base::deserialized_responses::EncryptionInfo; use once_cell::sync::Lazy; use ruma::{ - events::{room::message::MessageType, AnySyncTimelineEvent}, + events::{receipt::Receipt, room::message::MessageType, AnySyncTimelineEvent}, serde::Raw, EventId, MilliSecondsSinceUnixEpoch, OwnedEventId, OwnedMxcUri, OwnedUserId, TransactionId, UserId, @@ -29,15 +30,12 @@ mod content; mod local; mod remote; -pub use self::{ - content::{ - AnyOtherFullStateEventContent, BundledReactions, EncryptedMessage, InReplyToDetails, - MemberProfileChange, MembershipChange, Message, OtherState, ReactionGroup, RepliedToEvent, - RoomMembershipChange, Sticker, TimelineItemContent, - }, - local::LocalEventTimelineItem, - remote::RemoteEventTimelineItem, +pub use self::content::{ + AnyOtherFullStateEventContent, BundledReactions, EncryptedMessage, InReplyToDetails, + MemberProfileChange, MembershipChange, Message, OtherState, ReactionGroup, RepliedToEvent, + RoomMembershipChange, Sticker, TimelineItemContent, }; +pub(super) use self::{local::LocalEventTimelineItem, remote::RemoteEventTimelineItem}; /// An item in the timeline that represents at least one event. /// @@ -74,8 +72,16 @@ impl EventTimelineItem { Self { sender, sender_profile, content, kind } } + /// Check whether this item is a local echo. + /// + /// This returns `true` for events created locally, until the server echoes + /// back the full event as part of a sync response. + pub fn is_local_echo(&self) -> bool { + matches!(self.kind, EventTimelineItemKind::Local(_)) + } + /// Get the `LocalEventTimelineItem` if `self` is `Local`. - pub fn as_local(&self) -> Option<&LocalEventTimelineItem> { + pub(super) fn as_local(&self) -> Option<&LocalEventTimelineItem> { match &self.kind { EventTimelineItemKind::Local(local_event_item) => Some(local_event_item), EventTimelineItemKind::Remote(_) => None, @@ -83,7 +89,7 @@ impl EventTimelineItem { } /// Get the `RemoteEventTimelineItem` if `self` is `Remote`. - pub fn as_remote(&self) -> Option<&RemoteEventTimelineItem> { + pub(super) fn as_remote(&self) -> Option<&RemoteEventTimelineItem> { match &self.kind { EventTimelineItemKind::Local(_) => None, EventTimelineItemKind::Remote(remote_event_item) => Some(remote_event_item), @@ -169,6 +175,20 @@ impl EventTimelineItem { } } + /// Get the read receipts of this item. + /// + /// The key is the ID of a room member and the value are details about the + /// read receipt. + /// + /// Note that currently this ignores threads. + pub fn read_receipts(&self) -> &IndexMap { + static EMPTY_RECEIPTS: Lazy> = Lazy::new(Default::default); + match &self.kind { + EventTimelineItemKind::Local(_) => &EMPTY_RECEIPTS, + EventTimelineItemKind::Remote(remote_event) => remote_event.read_receipts(), + } + } + /// Get the timestamp of this item. /// /// If this event hasn't been echoed back by the server yet, returns the diff --git a/crates/matrix-sdk/src/room/timeline/event_item/remote.rs b/crates/matrix-sdk/src/room/timeline/event_item/remote.rs index 2551fc873..3e5e1d929 100644 --- a/crates/matrix-sdk/src/room/timeline/event_item/remote.rs +++ b/crates/matrix-sdk/src/room/timeline/event_item/remote.rs @@ -12,7 +12,7 @@ use super::BundledReactions; /// An item for an event that was received from the homeserver. #[derive(Clone)] -pub struct RemoteEventTimelineItem { +pub(in crate::room::timeline) struct RemoteEventTimelineItem { /// The event ID. event_id: OwnedEventId, /// The timestamp of the event. @@ -43,7 +43,7 @@ pub struct RemoteEventTimelineItem { impl RemoteEventTimelineItem { #[allow(clippy::too_many_arguments)] // Would be nice to fix, but unclear how - pub(in crate::room::timeline) fn new( + pub fn new( event_id: OwnedEventId, timestamp: MilliSecondsSinceUnixEpoch, reactions: BundledReactions, @@ -118,29 +118,25 @@ impl RemoteEventTimelineItem { self.is_highlighted } - pub(in crate::room::timeline) fn add_read_receipt( - &mut self, - user_id: OwnedUserId, - receipt: Receipt, - ) { + pub fn add_read_receipt(&mut self, user_id: OwnedUserId, receipt: Receipt) { self.read_receipts.insert(user_id, receipt); } /// Remove the read receipt for the given user. /// /// Returns `true` if there was one, `false` if not. - pub(in crate::room::timeline) fn remove_read_receipt(&mut self, user_id: &UserId) -> bool { + pub fn remove_read_receipt(&mut self, user_id: &UserId) -> bool { self.read_receipts.remove(user_id).is_some() } /// Clone the current event item, and update its `reactions`. - pub(in crate::room::timeline) fn with_reactions(&self, reactions: BundledReactions) -> Self { + pub fn with_reactions(&self, reactions: BundledReactions) -> Self { Self { reactions, ..self.clone() } } /// Clone the current event item, change its `content` to /// [`TimelineItemContent::RedactedMessage`], and reset its `reactions`. - pub(in crate::room::timeline) fn to_redacted(&self) -> Self { + pub fn to_redacted(&self) -> Self { Self { reactions: BundledReactions::default(), ..self.clone() } } diff --git a/crates/matrix-sdk/src/room/timeline/mod.rs b/crates/matrix-sdk/src/room/timeline/mod.rs index 978dbc419..c23516f98 100644 --- a/crates/matrix-sdk/src/room/timeline/mod.rs +++ b/crates/matrix-sdk/src/room/timeline/mod.rs @@ -59,9 +59,9 @@ use self::inner::{TimelineInner, TimelineInnerState}; pub use self::{ event_item::{ AnyOtherFullStateEventContent, BundledReactions, EncryptedMessage, EventSendState, - EventTimelineItem, InReplyToDetails, LocalEventTimelineItem, MemberProfileChange, - MembershipChange, Message, OtherState, Profile, ReactionGroup, RemoteEventTimelineItem, - RepliedToEvent, RoomMembershipChange, Sticker, TimelineDetails, TimelineItemContent, + EventTimelineItem, InReplyToDetails, MemberProfileChange, MembershipChange, Message, + OtherState, Profile, ReactionGroup, RepliedToEvent, RoomMembershipChange, Sticker, + TimelineDetails, TimelineItemContent, }, pagination::{PaginationOptions, PaginationOutcome}, virtual_item::VirtualTimelineItem, diff --git a/crates/matrix-sdk/tests/integration/room/timeline/mod.rs b/crates/matrix-sdk/tests/integration/room/timeline/mod.rs index 0df8b278f..5bc8b515e 100644 --- a/crates/matrix-sdk/tests/integration/room/timeline/mod.rs +++ b/crates/matrix-sdk/tests/integration/room/timeline/mod.rs @@ -192,7 +192,7 @@ async fn echo() { let _day_divider = assert_matches!(timeline_stream.next().await, Some(VectorDiff::PushBack { value }) => value); let local_echo = assert_matches!(timeline_stream.next().await, Some(VectorDiff::PushBack { value }) => value); let item = local_echo.as_event().unwrap(); - assert_matches!(item.as_local().unwrap().send_state(), EventSendState::NotSentYet); + assert_matches!(item.send_state(), Some(EventSendState::NotSentYet)); let msg = assert_matches!(item.content(), TimelineItemContent::Message(msg) => msg); let text = assert_matches!(msg.msgtype(), MessageType::Text(text) => text); @@ -205,8 +205,8 @@ async fn echo() { timeline_stream.next().await, Some(VectorDiff::Set { index: 1, value }) => value ); - let item = sent_confirmation.as_event().unwrap().as_local().unwrap(); - assert_matches!(item.send_state(), EventSendState::Sent { .. }); + let item = sent_confirmation.as_event().unwrap(); + assert_matches!(item.send_state(), Some(EventSendState::Sent { .. })); ev_builder.add_joined_room(JoinedRoomBuilder::new(room_id).add_timeline_event( TimelineTestEvent::Custom(json!({ @@ -243,7 +243,7 @@ async fn echo() { timeline_stream.next().await, Some(VectorDiff::PushBack { value }) => value ); - let item = remote_echo.as_event().unwrap().as_remote().unwrap(); + let item = remote_echo.as_event().unwrap(); assert!(item.is_own()); assert_eq!(item.timestamp(), MilliSecondsSinceUnixEpoch(uint!(152038280))); } @@ -807,7 +807,7 @@ async fn sync_highlighted() { timeline_stream.next().await, Some(VectorDiff::PushBack { value }) => value ); - let remote_event = first.as_event().unwrap().as_remote().unwrap(); + let remote_event = first.as_event().unwrap(); // Own events don't trigger push rules. assert!(!remote_event.is_highlighted()); @@ -833,7 +833,7 @@ async fn sync_highlighted() { timeline_stream.next().await, Some(VectorDiff::PushBack { value }) => value ); - let remote_event = second.as_event().unwrap().as_remote().unwrap(); + let remote_event = second.as_event().unwrap(); // `m.room.tombstone` should be highlighted by default. assert!(remote_event.is_highlighted()); } @@ -918,7 +918,7 @@ async fn back_pagination_highlighted() { timeline_stream.next().await, Some(VectorDiff::Insert { index: 2, value }) => value ); - let remote_event = first.as_event().unwrap().as_remote().unwrap(); + let remote_event = first.as_event().unwrap(); // Own events don't trigger push rules. assert!(!remote_event.is_highlighted()); @@ -926,7 +926,7 @@ async fn back_pagination_highlighted() { timeline_stream.next().await, Some(VectorDiff::Insert { index: 2, value }) => value ); - let remote_event = second.as_event().unwrap().as_remote().unwrap(); + let remote_event = second.as_event().unwrap(); // `m.room.tombstone` should be highlighted by default. assert!(remote_event.is_highlighted()); diff --git a/crates/matrix-sdk/tests/integration/room/timeline/read_receipts.rs b/crates/matrix-sdk/tests/integration/room/timeline/read_receipts.rs index 9edc03a8e..54f13fd61 100644 --- a/crates/matrix-sdk/tests/integration/room/timeline/read_receipts.rs +++ b/crates/matrix-sdk/tests/integration/room/timeline/read_receipts.rs @@ -86,24 +86,24 @@ async fn read_receipts_updates() { // We don't list the read receipt of our own user on events. let first_item = assert_matches!(timeline_stream.next().await, Some(VectorDiff::PushBack { value }) => value); - let first_event = first_item.as_event().unwrap().as_remote().unwrap(); + let first_event = first_item.as_event().unwrap(); assert!(first_event.read_receipts().is_empty()); let (own_receipt_event_id, _) = timeline.latest_user_read_receipt(own_user_id).await.unwrap(); - assert_eq!(own_receipt_event_id, first_event.event_id()); + assert_eq!(own_receipt_event_id, first_event.event_id().unwrap()); // Implicit read receipt of @alice:localhost. let second_item = assert_matches!(timeline_stream.next().await, Some(VectorDiff::PushBack { value }) => value); - let second_event = second_item.as_event().unwrap().as_remote().unwrap(); + let second_event = second_item.as_event().unwrap(); assert_eq!(second_event.read_receipts().len(), 1); // Read receipt of @alice:localhost is moved to third event. let second_item = assert_matches!(timeline_stream.next().await, Some(VectorDiff::Set { index: 2, value }) => value); - let second_event = second_item.as_event().unwrap().as_remote().unwrap(); + let second_event = second_item.as_event().unwrap(); assert!(second_event.read_receipts().is_empty()); let third_item = assert_matches!(timeline_stream.next().await, Some(VectorDiff::PushBack { value }) => value); - let third_event = third_item.as_event().unwrap().as_remote().unwrap(); + let third_event = third_item.as_event().unwrap(); assert_eq!(third_event.read_receipts().len(), 1); let (alice_receipt_event_id, _) = timeline.latest_user_read_receipt(alice).await.unwrap(); @@ -130,7 +130,7 @@ async fn read_receipts_updates() { server.reset().await; let (alice_receipt_event_id, _) = timeline.latest_user_read_receipt(alice).await.unwrap(); - assert_eq!(alice_receipt_event_id, third_event.event_id()); + assert_eq!(alice_receipt_event_id, third_event.event_id().unwrap()); // Read receipt on older event is ignored. ev_builder.add_joined_room(JoinedRoomBuilder::new(room_id).add_ephemeral_event( @@ -191,7 +191,7 @@ async fn read_receipts_updates() { server.reset().await; let third_item = assert_matches!(timeline_stream.next().await, Some(VectorDiff::Set { index: 3, value }) => value); - let third_event = third_item.as_event().unwrap().as_remote().unwrap(); + let third_event = third_item.as_event().unwrap(); assert_eq!(third_event.read_receipts().len(), 2); let (bob_receipt_event_id, _) = timeline.latest_user_read_receipt(bob).await.unwrap(); diff --git a/testing/sliding-sync-integration-test/src/lib.rs b/testing/sliding-sync-integration-test/src/lib.rs index 7d7219bcb..239b651ab 100644 --- a/testing/sliding-sync-integration-test/src/lib.rs +++ b/testing/sliding-sync-integration-test/src/lib.rs @@ -223,14 +223,14 @@ async fn modifying_timeline_limit() -> anyhow::Result<()> { assert_matches!(timeline_items[0].as_virtual(), Some(_)); // Second timeline item. - let latest_remote_event = timeline_items[1].as_event().unwrap().as_remote().unwrap(); - all_event_ids.push(latest_remote_event.event_id().to_owned()); + let latest_remote_event = timeline_items[1].as_event().unwrap(); + all_event_ids.push(latest_remote_event.event_id().unwrap().to_owned()); // Test the room to see the last event. let latest_event = room.latest_event().await.unwrap(); assert_eq!( latest_event.event_id(), - Some(latest_remote_event.event_id()), + latest_remote_event.event_id(), "Unexpected latest event" ); assert_eq!(latest_event.content().as_message().unwrap().body(), "Message #19");