Merge branch 'bnjbvr/oopsy-state-deadlock' into bnjbvr/permalink-mvp

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

@@ -332,47 +332,6 @@ impl<P: RoomDataProvider> TimelineInner<P> {
         self
     }
 
-    /// Load the current fully-read event in this inner timeline from storage.
-    pub(super) async fn load_fully_read_event(&self) {
-        if let Some(fully_read) = self.room_data_provider.load_fully_read_event().await {
-            self.set_fully_read_event(fully_read.content.event_id).await;
-        }
-    }
-
-    /// Replaces the content of the current timeline with initial events.
-    ///
-    /// Also sets up read receipts and the read marker for a live timeline of a
-    /// room.
-    ///
-    /// This is all done with a single lock guard, since we don't want the state
-    /// to be modified between the clear and re-insertion of new events.
-    pub(super) async fn replace_with_initial_events(&self, events: Vec<SyncTimelineEvent>) {
-        let mut state = self.state.write().await;
-
-        state.clear();
-
-        let track_read_markers = self.settings.track_read_receipts;
-        if track_read_markers {
-            self.populate_initial_user_receipt(ReceiptType::Read).await;
-            self.populate_initial_user_receipt(ReceiptType::ReadPrivate).await;
-        }
-
-        if !events.is_empty() {
-            state
-                .add_events_at(
-                    events,
-                    TimelineEnd::Back { origin: RemoteEventOrigin::Cache },
-                    &self.room_data_provider,
-                    &self.settings,
-                )
-                .await;
-        }
-
-        if track_read_markers {
-            self.load_fully_read_event().await;
-        }
-    }
-
     /// Switches the focus to the live mode.
     pub(super) async fn focus_live(
         &self,
@@ -705,6 +664,44 @@ impl<P: RoomDataProvider> TimelineInner<P> {
         self.state.write().await.clear();
     }
 
+    /// Replaces the content of the current timeline with initial events.
+    ///
+    /// Also sets up read receipts and the read marker for a live timeline of a
+    /// room.
+    ///
+    /// This is all done with a single lock guard, since we don't want the state
+    /// to be modified between the clear and re-insertion of new events.
+    pub(super) async fn replace_with_initial_events(&self, events: Vec<SyncTimelineEvent>) {
+        let mut state = self.state.write().await;
+
+        state.clear();
+
+        let track_read_markers = self.settings.track_read_receipts;
+        if track_read_markers {
+            self.populate_initial_user_receipt(ReceiptType::Read).await;
+            self.populate_initial_user_receipt(ReceiptType::ReadPrivate).await;
+        }
+
+        if !events.is_empty() {
+            state
+                .add_events_at(
+                    events,
+                    TimelineEnd::Back { origin: RemoteEventOrigin::Cache },
+                    &self.room_data_provider,
+                    &self.settings,
+                )
+                .await;
+        }
+
+        if track_read_markers {
+            if let Some(fully_read_event_id) =
+                self.room_data_provider.load_fully_read_marker().await
+            {
+                state.set_fully_read_event(fully_read_event_id);
+            }
+        }
+    }
+
     pub(super) async fn handle_fully_read_marker(&self, fully_read_event_id: OwnedEventId) {
         self.state.write().await.handle_fully_read_marker(fully_read_event_id);
     }
@@ -946,6 +943,7 @@ impl<P: RoomDataProvider> TimelineInner<P> {
         }
     }
 
+    #[cfg(any(test, feature = "testing"))]
     pub(super) async fn set_fully_read_event(&self, fully_read_event_id: OwnedEventId) {
         self.state.write().await.set_fully_read_event(fully_read_event_id);
     }
@@ -1336,11 +1334,10 @@ impl TimelineInner {
                 }
             }
             SendReceiptType::FullyRead => {
-                if let Some(old_fully_read) = self.room_data_provider.load_fully_read_event().await
+                if let Some(prev_event_id) = self.room_data_provider.load_fully_read_marker().await
                 {
-                    if let Some(relative_pos) = state
-                        .meta
-                        .compare_events_positions(&old_fully_read.content.event_id, event_id)
+                    if let Some(relative_pos) =
+                        state.meta.compare_events_positions(&prev_event_id, event_id)
                     {
                         return relative_pos == RelativePosition::After;
                     }

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

@@ -15,6 +15,7 @@
 use assert_matches::assert_matches;
 use assert_matches2::assert_let;
 use eyeball_im::VectorDiff;
+use matrix_sdk::test_utils::events::EventFactory;
 use matrix_sdk_base::deserialized_responses::SyncTimelineEvent;
 use matrix_sdk_test::{async_test, sync_timeline_event, ALICE, BOB, CAROL};
 use ruma::{
@@ -29,13 +30,15 @@ use ruma::{
         },
         FullStateEventContent,
     },
+    owned_event_id,
 };
 use stream_assert::assert_next_matches;
 
 use super::TestTimeline;
 use crate::timeline::{
     event_item::{AnyOtherFullStateEventContent, RemoteEventOrigin},
-    inner::TimelineEnd,
+    inner::{TimelineEnd, TimelineInnerSettings},
+    tests::TestRoomDataProvider,
     MembershipChange, TimelineDetails, TimelineItemContent, TimelineItemKind, VirtualTimelineItem,
 };
 
@@ -73,6 +76,36 @@ async fn test_initial_events() {
     assert_matches!(&item.kind, TimelineItemKind::Virtual(VirtualTimelineItem::DayDivider(_)));
 }
 
+#[async_test]
+async fn test_replace_with_initial_events_and_read_marker() {
+    let event_id = owned_event_id!("$1");
+    let timeline = TestTimeline::with_room_data_provider(
+        TestRoomDataProvider::default().with_fully_read_marker(event_id),
+    )
+    .with_settings(TimelineInnerSettings { track_read_receipts: true, ..Default::default() });
+
+    let factory = EventFactory::new();
+    let ev = factory.text_msg("hey").sender(*ALICE).into_sync();
+
+    timeline
+        .inner
+        .add_events_at(vec![ev], TimelineEnd::Back { origin: RemoteEventOrigin::Cache })
+        .await;
+
+    let items = timeline.inner.items().await;
+    assert_eq!(items.len(), 2);
+    assert!(items[0].is_day_divider());
+    assert_eq!(items[1].as_event().unwrap().content().as_message().unwrap().body(), "hey");
+
+    let ev = factory.text_msg("yo").sender(*BOB).into_sync();
+    timeline.inner.replace_with_initial_events(vec![ev]).await;
+
+    let items = timeline.inner.items().await;
+    assert_eq!(items.len(), 2);
+    assert!(items[0].is_day_divider());
+    assert_eq!(items[1].as_event().unwrap().content().as_message().unwrap().body(), "yo");
+}
+
 #[async_test]
 async fn test_sticker() {
     let timeline = TestTimeline::new();

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

@@ -35,7 +35,6 @@ use matrix_sdk_test::{EventBuilder, ALICE, BOB};
 use ruma::{
     event_id,
     events::{
-        fully_read::FullyReadEvent,
         receipt::{Receipt, ReceiptThread, ReceiptType},
         relation::Annotation,
         AnyMessageLikeEventContent, AnySyncTimelineEvent, AnyTimelineEvent, EmptyStateKey,
@@ -291,11 +290,17 @@ type ReadReceiptMap =
 #[derive(Clone, Default)]
 struct TestRoomDataProvider {
     initial_user_receipts: ReadReceiptMap,
+    fully_read_marker: Option<OwnedEventId>,
 }
 
 impl TestRoomDataProvider {
-    fn with_initial_user_receipts(initial_user_receipts: ReadReceiptMap) -> Self {
-        Self { initial_user_receipts }
+    fn with_initial_user_receipts(mut self, initial_user_receipts: ReadReceiptMap) -> Self {
+        self.initial_user_receipts = initial_user_receipts;
+        self
+    }
+    fn with_fully_read_marker(mut self, event_id: OwnedEventId) -> Self {
+        self.fully_read_marker = Some(event_id);
+        self
     }
 }
 
@@ -373,8 +378,8 @@ impl RoomDataProvider for TestRoomDataProvider {
         Some((push_rules, push_context))
     }
 
-    async fn load_fully_read_event(&self) -> Option<FullyReadEvent> {
-        None
+    async fn load_fully_read_marker(&self) -> Option<OwnedEventId> {
+        self.fully_read_marker.clone()
     }
 }
 

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

@@ -490,7 +490,7 @@ async fn test_initial_public_unthreaded_receipt() {
         );
 
     let timeline = TestTimeline::with_room_data_provider(
-        TestRoomDataProvider::with_initial_user_receipts(initial_user_receipts),
+        TestRoomDataProvider::default().with_initial_user_receipts(initial_user_receipts),
     )
     .with_settings(TimelineInnerSettings { track_read_receipts: true, ..Default::default() });
 
@@ -515,7 +515,7 @@ async fn test_initial_public_main_thread_receipt() {
         );
 
     let timeline = TestTimeline::with_room_data_provider(
-        TestRoomDataProvider::with_initial_user_receipts(initial_user_receipts),
+        TestRoomDataProvider::default().with_initial_user_receipts(initial_user_receipts),
     )
     .with_settings(TimelineInnerSettings { track_read_receipts: true, ..Default::default() });
 
@@ -540,7 +540,7 @@ async fn test_initial_private_unthreaded_receipt() {
         );
 
     let timeline = TestTimeline::with_room_data_provider(
-        TestRoomDataProvider::with_initial_user_receipts(initial_user_receipts),
+        TestRoomDataProvider::default().with_initial_user_receipts(initial_user_receipts),
     )
     .with_settings(TimelineInnerSettings { track_read_receipts: true, ..Default::default() });
 
@@ -565,7 +565,7 @@ async fn test_initial_private_main_thread_receipt() {
         );
 
     let timeline = TestTimeline::with_room_data_provider(
-        TestRoomDataProvider::with_initial_user_receipts(initial_user_receipts),
+        TestRoomDataProvider::default().with_initial_user_receipts(initial_user_receipts),
     )
     .with_settings(TimelineInnerSettings { track_read_receipts: true, ..Default::default() });
 

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

@@ -22,7 +22,7 @@ use matrix_sdk_base::latest_event::LatestEvent;
 use ruma::{events::AnySyncTimelineEvent, serde::Raw};
 use ruma::{
     events::{
-        fully_read::FullyReadEvent,
+        fully_read::FullyReadEventContent,
         receipt::{Receipt, ReceiptThread, ReceiptType},
     },
     push::{PushConditionRoomCtx, Ruleset},
@@ -84,8 +84,8 @@ pub(super) trait RoomDataProvider: Clone + Send + Sync + 'static + PaginableRoom
     /// Loads read receipts for an event from the storage backend.
     async fn load_event_receipts(&self, event_id: &EventId) -> IndexMap<OwnedUserId, Receipt>;
 
-    /// Get the current fully-read event, from storage.
-    async fn load_fully_read_event(&self) -> Option<FullyReadEvent>;
+    /// Load the current fully-read event id, from storage.
+    async fn load_fully_read_marker(&self) -> Option<OwnedEventId>;
 
     async fn push_rules_and_context(&self) -> Option<(Ruleset, PushConditionRoomCtx)>;
 }
@@ -195,20 +195,20 @@ impl RoomDataProvider for Room {
         }
     }
 
-    async fn load_fully_read_event(&self) -> Option<FullyReadEvent> {
-        match self.account_data_static().await {
+    async fn load_fully_read_marker(&self) -> Option<OwnedEventId> {
+        match self.account_data_static::<FullyReadEventContent>().await {
             Ok(Some(fully_read)) => match fully_read.deserialize() {
-                Ok(fully_read) => Some(fully_read),
+                Ok(fully_read) => Some(fully_read.content.event_id),
                 Err(e) => {
                     error!("Failed to deserialize fully-read account data: {e}");
                     None
                 }
             },
-            Ok(None) => None,
             Err(e) => {
                 error!("Failed to get fully-read account data from the store: {e}");
                 None
             }
+            _ => None,
         }
     }
 }