feat(event cache): don't react specifically to limited timelines, when storage's enabled

Author: bnjbvr

crates/matrix-sdk/src/event_cache/mod.rs

@@ -313,7 +313,7 @@ impl EventCache {
         prev_batch: Option<String>,
     ) -> Result<()> {
         // If the event cache's storage has been enabled, do nothing.
-        if self.inner.store.get().is_some() {
+        if self.inner.has_storage() {
             return Ok(());
         }
 
@@ -387,6 +387,11 @@ impl EventCacheInner {
         self.client.get().ok_or(EventCacheError::ClientDropped)
     }
 
+    /// Has persistent storage been enabled for the event cache?
+    fn has_storage(&self) -> bool {
+        self.store.get().is_some()
+    }
+
     /// Clears all the room's data.
     async fn clear_all_rooms(&self) -> Result<()> {
         // Note: one must NOT clear the `by_room` map, because if something subscribed
@@ -419,7 +424,9 @@ impl EventCacheInner {
         for (room_id, left_room_update) in updates.leave {
             let room = self.for_room(&room_id).await?;
 
-            if let Err(err) = room.inner.handle_left_room_update(left_room_update).await {
+            if let Err(err) =
+                room.inner.handle_left_room_update(self.has_storage(), left_room_update).await
+            {
                 // Non-fatal error, try to continue to the next room.
                 error!("handling left room update: {err}");
             }
@@ -429,7 +436,9 @@ impl EventCacheInner {
         for (room_id, joined_room_update) in updates.join {
             let room = self.for_room(&room_id).await?;
 
-            if let Err(err) = room.inner.handle_joined_room_update(joined_room_update).await {
+            if let Err(err) =
+                room.inner.handle_joined_room_update(self.has_storage(), joined_room_update).await
+            {
                 // Non-fatal error, try to continue to the next room.
                 error!("handling joined room update: {err}");
             }
@@ -604,7 +613,10 @@ mod tests {
 
         room_event_cache
             .inner
-            .handle_joined_room_update(JoinedRoomUpdate { account_data, ..Default::default() })
+            .handle_joined_room_update(
+                event_cache.inner.has_storage(),
+                JoinedRoomUpdate { account_data, ..Default::default() },
+            )
             .await
             .unwrap();
 

crates/matrix-sdk/src/event_cache/room/mod.rs

@@ -286,8 +286,13 @@ impl RoomEventCacheInner {
         }
     }
 
-    pub(super) async fn handle_joined_room_update(&self, updates: JoinedRoomUpdate) -> Result<()> {
+    pub(super) async fn handle_joined_room_update(
+        &self,
+        has_storage: bool,
+        updates: JoinedRoomUpdate,
+    ) -> Result<()> {
         self.handle_timeline(
+            has_storage,
             updates.timeline,
             updates.ephemeral.clone(),
             updates.ambiguity_changes,
@@ -301,11 +306,12 @@ impl RoomEventCacheInner {
 
     async fn handle_timeline(
         &self,
+        has_storage: bool,
         timeline: Timeline,
         ephemeral_events: Vec<Raw<AnySyncEphemeralRoomEvent>>,
         ambiguity_changes: BTreeMap<OwnedEventId, AmbiguityChange>,
     ) -> Result<()> {
-        if timeline.limited {
+        if !has_storage && timeline.limited {
             // Ideally we'd try to reconcile existing events against those received in the
             // timeline, but we're not there yet. In the meanwhile, clear the
             // items from the room. TODO: implement Smart Matching™.
@@ -334,8 +340,13 @@ impl RoomEventCacheInner {
         Ok(())
     }
 
-    pub(super) async fn handle_left_room_update(&self, updates: LeftRoomUpdate) -> Result<()> {
-        self.handle_timeline(updates.timeline, Vec::new(), updates.ambiguity_changes).await?;
+    pub(super) async fn handle_left_room_update(
+        &self,
+        has_storage: bool,
+        updates: LeftRoomUpdate,
+    ) -> Result<()> {
+        self.handle_timeline(has_storage, updates.timeline, Vec::new(), updates.ambiguity_changes)
+            .await?;
         Ok(())
     }
 
@@ -990,7 +1001,7 @@ mod tests {
 
         room_event_cache
             .inner
-            .handle_joined_room_update(JoinedRoomUpdate { timeline, ..Default::default() })
+            .handle_joined_room_update(true, JoinedRoomUpdate { timeline, ..Default::default() })
             .await
             .unwrap();
 
@@ -1060,7 +1071,7 @@ mod tests {
 
         room_event_cache
             .inner
-            .handle_joined_room_update(JoinedRoomUpdate { timeline, ..Default::default() })
+            .handle_joined_room_update(true, JoinedRoomUpdate { timeline, ..Default::default() })
             .await
             .unwrap();
 
@@ -1188,7 +1199,7 @@ mod tests {
         let timeline = Timeline { limited: false, prev_batch: None, events: vec![ev1] };
         room_event_cache
             .inner
-            .handle_joined_room_update(JoinedRoomUpdate { timeline, ..Default::default() })
+            .handle_joined_room_update(true, JoinedRoomUpdate { timeline, ..Default::default() })
             .await
             .unwrap();
 

crates/matrix-sdk/tests/integration/event_cache.rs

@@ -1,10 +1,11 @@
 use std::{future::ready, ops::ControlFlow, time::Duration};
 
-use assert_matches2::{assert_let, assert_matches};
+use assert_matches::assert_matches;
+use assert_matches2::assert_let;
 use matrix_sdk::{
     event_cache::{
-        paginator::PaginatorState, BackPaginationOutcome, EventCacheError, RoomEventCacheUpdate,
-        TimelineHasBeenResetWhilePaginating,
+        paginator::PaginatorState, BackPaginationOutcome, EventCacheError, EventsOrigin,
+        RoomEventCacheUpdate, TimelineHasBeenResetWhilePaginating,
     },
     test_utils::{assert_event_matches_msg, logged_in_client_with_server, mocks::MatrixMockServer},
 };
@@ -852,3 +853,69 @@ async fn test_limited_timeline_resets_pagination() {
 
     assert!(room_stream.is_empty());
 }
+
+#[async_test]
+async fn test_limited_timeline_with_storage() {
+    let server = MatrixMockServer::new().await;
+    let client = server.client_builder().build().await;
+
+    let event_cache = client.event_cache();
+
+    // Don't forget to subscribe and like^W enable storage!
+    event_cache.subscribe().unwrap();
+    event_cache.enable_storage().unwrap();
+
+    let room_id = room_id!("!galette:saucisse.bzh");
+    let room = server.sync_joined_room(&client, room_id).await;
+
+    let (room_event_cache, _drop_handles) = room.event_cache().await.unwrap();
+
+    let f = EventFactory::new().room(room_id).sender(user_id!("@ben:saucisse.bzh"));
+
+    // First sync: get a message.
+    server
+        .sync_room(
+            &client,
+            JoinedRoomBuilder::new(room_id).add_timeline_event(f.text_msg("hey yo")),
+        )
+        .await;
+
+    let (initial_events, mut subscriber) = room_event_cache.subscribe().await.unwrap();
+
+    // This is racy: either the sync has been handled, or it hasn't yet.
+    if initial_events.is_empty() {
+        assert_let_timeout!(
+            Ok(RoomEventCacheUpdate::AddTimelineEvents { events, .. }) = subscriber.recv()
+        );
+        assert_eq!(events.len(), 1);
+        assert_event_matches_msg(&events[0], "hey yo");
+    } else {
+        assert_eq!(initial_events.len(), 1);
+        assert_event_matches_msg(&initial_events[0], "hey yo");
+    }
+
+    assert!(subscriber.is_empty());
+
+    // Second update: get a message but from a limited timeline.
+    server
+        .sync_room(
+            &client,
+            JoinedRoomBuilder::new(room_id)
+                .add_timeline_event(f.text_msg("gappy!"))
+                .set_timeline_limited(),
+        )
+        .await;
+
+    let update = timeout(Duration::from_secs(2), subscriber.recv())
+        .await
+        .expect("timeout after receiving a sync update")
+        .expect("should've received a room event cache update");
+
+    assert_matches!(update, RoomEventCacheUpdate::AddTimelineEvents { events, origin: EventsOrigin::Sync } => {
+        assert_eq!(events.len(), 1);
+        assert_event_matches_msg(&events[0], "gappy!");
+    });
+
+    // That's all, folks!
+    assert!(subscriber.is_empty());
+}