feat(event cache): shrink the linked chunk upon gappy syncs

Author: bnjbvr

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

@@ -435,7 +435,7 @@ impl RoomEventCacheInner {
         } else {
             // Add the previous back-pagination token (if present), followed by the timeline
             // events themselves.
-            let (_, sync_timeline_events_diffs) = state
+            let ((), mut sync_timeline_events_diffs) = state
                 .with_events_mut(|room_events| {
                     // If we only received duplicated events, we don't need to store the gap: if
                     // there was a gap, we'd have received an unknown event at the tail of
@@ -461,8 +461,21 @@ impl RoomEventCacheInner {
                 })
                 .await?;
 
+            if prev_batch.is_some() && !all_duplicates {
                 // If there was a previous batch token, and there's at least one non-duplicated
+                // new event, unload the chunks so it only contains the last
+                // one; otherwise, there might be a valid gap in between, and
+                // observers may not render it (yet). In this case, extend the
+                // updates with those from the unload; the new updates include a `clear` (as of
+                // 2025-02-24), so they will remove all the previous ones first.
+                //
                 // We must do this *after* the above call to `.with_events_mut`, so the new
+                // events and gaps are properly persisted to storage.
+                if let Some(new_events_diffs) = state.shrink_to_last_chunk().await? {
+                    sync_timeline_events_diffs.extend(new_events_diffs);
+                }
+            }
+
             {
                 // Fill the AllEventsCache.
                 let mut all_events = self.all_events.write().await;
@@ -1690,6 +1703,8 @@ mod tests {
     #[cfg(not(target_arch = "wasm32"))] // This uses the cross-process lock, so needs time support.
     #[async_test]
     async fn test_no_useless_gaps() {
+        use crate::event_cache::room::LoadMoreEventsBackwardsOutcome;
+
         let room_id = room_id!("!galette:saucisse.bzh");
 
         let client = MockClientBuilder::new("http://localhost".to_owned()).build().await;
@@ -1725,7 +1740,7 @@ mod tests {
             .unwrap();
 
         {
-            let state = room_event_cache.inner.state.read().await;
+            let mut state = room_event_cache.inner.state.write().await;
 
             let mut num_gaps = 0;
             let mut num_events = 0;
@@ -1737,6 +1752,26 @@ mod tests {
                 }
             }
 
+            // The limited sync unloads the chunk, so it will appear as if there are only
+            // the events.
+            assert_eq!(num_gaps, 0);
+            assert_eq!(num_events, 1);
+
+            // But if I manually reload more of the chunk, the gap will be present.
+            assert_matches!(
+                state.load_more_events_backwards().await.unwrap(),
+                LoadMoreEventsBackwardsOutcome::Gap
+            );
+
+            num_gaps = 0;
+            num_events = 0;
+            for c in state.events().chunks() {
+                match c.content() {
+                    ChunkContent::Items(items) => num_events += items.len(),
+                    ChunkContent::Gap(_) => num_gaps += 1,
+                }
+            }
+
             // The gap must have been stored.
             assert_eq!(num_gaps, 1);
             assert_eq!(num_events, 1);

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

@@ -1302,15 +1302,16 @@ async fn test_no_gap_stored_after_deduplicated_backpagination() {
     assert_let_timeout!(
         Ok(RoomEventCacheUpdate::UpdateTimelineEvents { diffs, .. }) = stream.recv()
     );
-    assert_eq!(diffs.len(), 2);
+    assert_eq!(diffs.len(), 4);
 
-    // The linked chunk is unloaded, because of the limited sync with a gap:
-    // It's first cleared…
-    assert_matches!(&diffs[0], VectorDiff::Clear);
+    // The first two diffs are for the gap and the new events, but they don't really
+    // matter in this test, because then, the linked chunk is unloaded, causing
+    // a clear:
+    assert_matches!(&diffs[2], VectorDiff::Clear);
 
     // Then the latest event chunk is reloaded.
     // `$ev1`, `$ev2` and `$ev3` are added.
-    assert_matches!(&diffs[1], VectorDiff::Append { values: events } => {
+    assert_matches!(&diffs[3], VectorDiff::Append { values: events } => {
         assert_eq!(events.len(), 3);
         assert_eq!(events[0].event_id().unwrap().as_str(), "$1");
         assert_eq!(events[1].event_id().unwrap().as_str(), "$2");