refactor(timeline): introduce TimelineUniqueId as an opaque type fo…

…r the unique identifier

We can now use this type instead of passing a string, which means
there's no way to confuse oneself in methods like
`toggle_reaction_local`.

Changelog: Introduced `TimelineUniqueId`, returned by
`TimelineItem::unique_id()` and serving as an opaque identifier to use
in other methods modifying the timeline item (e.g. `toggle_reaction`).

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

@@ -32,6 +32,7 @@ use matrix_sdk::{
 };
 use matrix_sdk_ui::timeline::{
     EventItemOrigin, LiveBackPaginationStatus, Profile, RepliedToEvent, TimelineDetails,
+    TimelineUniqueId as SdkTimelineUniqueId,
 };
 use mime::Mime;
 use ruma::{
@@ -541,8 +542,12 @@ impl Timeline {
     ///
     /// Ensures that only one reaction is sent at a time to avoid race
     /// conditions and spamming the homeserver with requests.
-    pub async fn toggle_reaction(&self, unique_id: String, key: String) -> Result<(), ClientError> {
-        self.inner.toggle_reaction(&unique_id, &key).await?;
+    pub async fn toggle_reaction(
+        &self,
+        unique_id: &TimelineUniqueId,
+        key: String,
+    ) -> Result<(), ClientError> {
+        self.inner.toggle_reaction(&unique_id.into(), &key).await?;
         Ok(())
     }
 
@@ -866,6 +871,23 @@ pub enum TimelineChange {
     Reset,
 }
 
+#[derive(Clone, uniffi::Record)]
+pub struct TimelineUniqueId {
+    id: String,
+}
+
+impl From<&SdkTimelineUniqueId> for TimelineUniqueId {
+    fn from(value: &SdkTimelineUniqueId) -> Self {
+        Self { id: value.0.clone() }
+    }
+}
+
+impl From<&TimelineUniqueId> for SdkTimelineUniqueId {
+    fn from(value: &TimelineUniqueId) -> Self {
+        Self(value.id.clone())
+    }
+}
+
 #[repr(transparent)]
 #[derive(Clone, uniffi::Object)]
 pub struct TimelineItem(pub(crate) matrix_sdk_ui::timeline::TimelineItem);
@@ -893,8 +915,9 @@ impl TimelineItem {
         }
     }
 
-    pub fn unique_id(&self) -> String {
-        self.0.unique_id().to_owned()
+    /// An opaque unique identifier for this timeline item.
+    pub fn unique_id(&self) -> TimelineUniqueId {
+        self.0.unique_id().into()
     }
 
     pub fn fmt_debug(&self) -> String {

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

@@ -58,6 +58,7 @@ pub(super) use self::state::{
 use super::{
     event_handler::TimelineEventKind,
     event_item::{ReactionStatus, RemoteEventOrigin},
+    item::TimelineUniqueId,
     traits::{Decryptor, RoomDataProvider},
     util::{rfind_event_by_id, rfind_event_item, RelativePosition},
     Error, EventSendState, EventTimelineItem, InReplyToDetails, Message, PaginationError, Profile,
@@ -484,7 +485,7 @@ impl<P: RoomDataProvider> TimelineController<P> {
     #[instrument(skip_all)]
     pub(super) async fn toggle_reaction_local(
         &self,
-        unique_id: &str,
+        unique_id: &TimelineUniqueId,
         key: &str,
     ) -> Result<bool, Error> {
         let mut state = self.state.write().await;
@@ -1541,7 +1542,7 @@ async fn fetch_replied_to_event(
     mut state: RwLockWriteGuard<'_, TimelineState>,
     index: usize,
     item: &EventTimelineItem,
-    internal_id: String,
+    internal_id: TimelineUniqueId,
     message: &Message,
     in_reply_to: &EventId,
     room: &Room,

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

@@ -54,6 +54,7 @@ use crate::{
             TimelineItemPosition,
         },
         event_item::{PollState, RemoteEventOrigin, ResponseData},
+        item::TimelineUniqueId,
         reactions::Reactions,
         read_receipts::ReadReceipts,
         traits::RoomDataProvider,
@@ -977,11 +978,11 @@ impl TimelineMetadata {
 
     /// Returns the next internal id for a timeline item (and increment our
     /// internal counter).
-    fn next_internal_id(&mut self) -> String {
+    fn next_internal_id(&mut self) -> TimelineUniqueId {
         let val = self.next_internal_id;
         self.next_internal_id = self.next_internal_id.wrapping_add(1);
         let prefix = self.internal_id_prefix.as_deref().unwrap_or("");
-        format!("{prefix}{val}")
+        TimelineUniqueId(format!("{prefix}{val}"))
     }
 
     /// Returns a new timeline item with a fresh internal id.

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

@@ -18,6 +18,26 @@ use as_variant::as_variant;
 
 use super::{EventTimelineItem, VirtualTimelineItem};
 
+/// Opaque unique identifier for a timeline item.
+///
+/// It is transferred whenever a timeline item is updated. This can be used as a
+/// stable identifier for UI purposes, as well as operations on the event
+/// represented by the item.
+#[derive(Clone, Debug, PartialEq, Eq, Hash)]
+pub struct TimelineUniqueId(pub String);
+
+impl PartialEq<TimelineUniqueId> for &TimelineUniqueId {
+    fn eq(&self, other: &TimelineUniqueId) -> bool {
+        self.0 == other.0
+    }
+}
+
+impl PartialEq<&str> for &TimelineUniqueId {
+    fn eq(&self, other: &&str) -> bool {
+        self.0 == *other
+    }
+}
+
 #[derive(Clone, Debug)]
 #[allow(clippy::large_enum_variant)]
 pub enum TimelineItemKind {
@@ -32,12 +52,15 @@ pub enum TimelineItemKind {
 #[derive(Clone, Debug)]
 pub struct TimelineItem {
     pub(crate) kind: TimelineItemKind,
-    pub(crate) internal_id: String,
+    pub(crate) internal_id: TimelineUniqueId,
 }
 
 impl TimelineItem {
     /// Create a new `TimelineItem` with the given kind and internal id.
-    pub(crate) fn new(kind: impl Into<TimelineItemKind>, internal_id: String) -> Arc<Self> {
+    pub(crate) fn new(
+        kind: impl Into<TimelineItemKind>,
+        internal_id: TimelineUniqueId,
+    ) -> Arc<Self> {
         Arc::new(TimelineItem { kind: kind.into(), internal_id })
     }
 
@@ -71,14 +94,14 @@ impl TimelineItem {
     /// dividers, identity isn't easy to define though and you might
     /// see a new ID getting generated for a day divider that you
     /// perceive to be "the same" as a previous one.
-    pub fn unique_id(&self) -> &str {
+    pub fn unique_id(&self) -> &TimelineUniqueId {
         &self.internal_id
     }
 
     pub(crate) fn read_marker() -> Arc<TimelineItem> {
         Arc::new(Self {
             kind: TimelineItemKind::Virtual(VirtualTimelineItem::ReadMarker),
-            internal_id: "__read_marker".to_owned(),
+            internal_id: TimelineUniqueId("__read_marker".to_owned()),
         })
     }
 

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

@@ -54,6 +54,7 @@ use ruma::{
 };
 use thiserror::Error;
 use tracing::{error, instrument, trace, warn};
+use util::rfind_event_by_uid;
 
 use crate::timeline::pinned_events_loader::PinnedEventsRoom;
 
@@ -89,7 +90,7 @@ pub use self::{
         Sticker, TimelineDetails, TimelineEventItemId, TimelineItemContent,
     },
     event_type_filter::TimelineEventTypeFilter,
-    item::{TimelineItem, TimelineItemKind},
+    item::{TimelineItem, TimelineItemKind, TimelineUniqueId},
     pagination::LiveBackPaginationStatus,
     traits::RoomExt,
     virtual_item::VirtualTimelineItem,
@@ -557,7 +558,11 @@ impl Timeline {
     ///
     /// Ensures that only one reaction is sent at a time to avoid race
     /// conditions and spamming the homeserver with requests.
-    pub async fn toggle_reaction(&self, unique_id: &str, reaction_key: &str) -> Result<(), Error> {
+    pub async fn toggle_reaction(
+        &self,
+        unique_id: &TimelineUniqueId,
+        reaction_key: &str,
+    ) -> Result<(), Error> {
         self.controller.toggle_reaction_local(unique_id, reaction_key).await?;
         Ok(())
     }

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

@@ -59,6 +59,7 @@ use super::{
     controller::{TimelineEnd, TimelineSettings},
     event_handler::TimelineEventKind,
     event_item::RemoteEventOrigin,
+    item::TimelineUniqueId,
     traits::RoomDataProvider,
     EventTimelineItem, Profile, TimelineController, TimelineFocus, TimelineItem,
 };
@@ -265,7 +266,11 @@ impl TestTimeline {
         self.controller.handle_read_receipts(ev_content).await;
     }
 
-    async fn toggle_reaction_local(&self, unique_id: &str, key: &str) -> Result<(), super::Error> {
+    async fn toggle_reaction_local(
+        &self,
+        unique_id: &TimelineUniqueId,
+        key: &str,
+    ) -> Result<(), super::Error> {
         if self.controller.toggle_reaction_local(unique_id, key).await? {
             // TODO(bnjbvr): hacky?
             if let Some(event_id) = self

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

@@ -28,8 +28,8 @@ use stream_assert::assert_next_matches;
 use tokio::time::timeout;
 
 use crate::timeline::{
-    controller::TimelineEnd, event_item::RemoteEventOrigin, tests::TestTimeline, ReactionStatus,
-    TimelineItem,
+    controller::TimelineEnd, event_item::RemoteEventOrigin, item::TimelineUniqueId,
+    tests::TestTimeline, ReactionStatus, TimelineItem,
 };
 
 const REACTION_KEY: &str = "👍";
@@ -83,7 +83,10 @@ async fn test_add_reaction_on_non_existent_event() {
     let timeline = TestTimeline::new();
     let mut stream = timeline.subscribe().await;
 
-    timeline.toggle_reaction_local("nonexisting_unique_id", REACTION_KEY).await.unwrap_err();
+    timeline
+        .toggle_reaction_local(&TimelineUniqueId("nonexisting_unique_id".to_owned()), REACTION_KEY)
+        .await
+        .unwrap_err();
 
     assert!(stream.next().now_or_never().is_none());
 }
@@ -216,7 +219,7 @@ async fn test_initial_reaction_timestamp_is_stored() {
 async fn send_first_message(
     timeline: &TestTimeline,
     stream: &mut (impl Stream<Item = VectorDiff<Arc<TimelineItem>>> + Unpin),
-) -> (String, OwnedEventId, usize) {
+) -> (TimelineUniqueId, OwnedEventId, usize) {
     timeline.handle_live_event(timeline.factory.text_msg("I want you to react").sender(&BOB)).await;
 
     let item = assert_next_matches!(*stream, VectorDiff::PushBack { value } => value);

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

@@ -21,26 +21,27 @@ use ruma::{EventId, MilliSecondsSinceUnixEpoch};
 #[cfg(doc)]
 use super::controller::TimelineMetadata;
 use super::{
-    event_item::EventTimelineItemKind, EventTimelineItem, ReactionsByKeyBySender, TimelineItem,
+    event_item::EventTimelineItemKind, item::TimelineUniqueId, EventTimelineItem,
+    ReactionsByKeyBySender, TimelineItem,
 };
 
 pub(super) struct EventTimelineItemWithId<'a> {
     pub inner: &'a EventTimelineItem,
     /// Internal identifier generated by [`TimelineMetadata`].
-    pub internal_id: &'a str,
+    pub internal_id: &'a TimelineUniqueId,
 }
 
 impl<'a> EventTimelineItemWithId<'a> {
     /// Create a clone of the underlying [`TimelineItem`] with the given kind.
     pub fn with_inner_kind(&self, kind: impl Into<EventTimelineItemKind>) -> Arc<TimelineItem> {
-        TimelineItem::new(self.inner.with_kind(kind), self.internal_id.to_owned())
+        TimelineItem::new(self.inner.with_kind(kind), self.internal_id.clone())
     }
 
     /// Create a clone of the underlying [`TimelineItem`] with the given
     /// reactions.
     pub fn with_reactions(&self, reactions: ReactionsByKeyBySender) -> Arc<TimelineItem> {
         let event_item = self.inner.with_reactions(reactions);
-        TimelineItem::new(event_item, self.internal_id.to_owned())
+        TimelineItem::new(event_item, self.internal_id.clone())
     }
 }
 
@@ -86,7 +87,7 @@ pub(super) fn rfind_event_item(
 /// [`rfind_event_item`].
 pub(super) fn rfind_event_by_uid<'a>(
     items: &'a Vector<Arc<TimelineItem>>,
-    internal_id: &'a str,
+    internal_id: &'a TimelineUniqueId,
 ) -> Option<(usize, EventTimelineItemWithId<'a>)> {
     rfind_event_item_internal(items, |item_with_id| item_with_id.internal_id == internal_id)
 }