fix(event cache): don't fill initial items if the room already had ev…

…ents (#4381)

The test requires subtle conditions to trigger:

- initialize a timeline from a room-list-service's room
- start a backpagination with that timeline (so the room event cache's
paginator is busy)
- try to initialize another timeline with the same room-list-service's
room (e.g. because the first room has been closed, and the app using it
doesn't have a room cache)

This would fail, because initializing a timeline calls
`EventCache::add_initial_events()` all the time, which tries to reset
the paginator's state, which assumes the paginator's not paginating at
this point. In a soon future, we'll get rid of the
`add_initial_events()` function because the event cache will handle its
own persistent storage; in the meantime, a correct fix is to skip
`add_initial_events()` if there was already something in the linked
chunk. After all, we're likely to fill the initial events with the same
events all the time, or a subset of more recent events. By doing that,
we're likely keeping *more* events in the linked chunk, instead.

Thanks to @stefanceriu for reporting the issue and confirming the fix
works!

crates/matrix-sdk-ui/tests/integration/room_list_service.rs

@@ -8,11 +8,16 @@ use eyeball_im::VectorDiff;
 use futures_util::{pin_mut, FutureExt, StreamExt};
 use matrix_sdk::{
     config::RequestConfig,
-    test_utils::{logged_in_client_with_server, set_client_session, test_client_builder},
+    test_utils::{
+        logged_in_client_with_server, mocks::MatrixMockServer, set_client_session,
+        test_client_builder,
+    },
     Client,
 };
 use matrix_sdk_base::sync::UnreadNotificationsCount;
-use matrix_sdk_test::{async_test, mocks::mock_encryption_state};
+use matrix_sdk_test::{
+    async_test, event_factory::EventFactory, mocks::mock_encryption_state, ALICE,
+};
 use matrix_sdk_ui::{
     room_list_service::{
         filters::{new_filter_fuzzy_match_room_name, new_filter_non_left, new_filter_none},
@@ -28,7 +33,7 @@ use ruma::{
 use serde_json::json;
 use stream_assert::{assert_next_matches, assert_pending};
 use tempfile::TempDir;
-use tokio::{spawn, sync::mpsc::channel, task::yield_now};
+use tokio::{spawn, sync::mpsc::channel, task::yield_now, time::sleep};
 use wiremock::{
     matchers::{header, method, path},
     Mock, MockServer, ResponseTemplate,
@@ -2795,3 +2800,81 @@ async fn test_sync_indicator() -> Result<(), Error> {
 
     Ok(())
 }
+
+#[async_test]
+async fn test_multiple_timeline_init() {
+    let server = MatrixMockServer::new().await;
+    let client = server.client_builder().build().await;
+    let room_list = RoomListService::new(client.clone()).await.unwrap();
+
+    let sync = room_list.sync();
+    pin_mut!(sync);
+
+    let room_id = room_id!("!r0:bar.org");
+
+    let mock_server = server.server();
+    sync_then_assert_request_and_fake_response! {
+        [mock_server, room_list, sync]
+        assert request >= {},
+        respond with = {
+            "pos": "0",
+            "lists": {
+                ALL_ROOMS: {
+                    "count": 2,
+                },
+            },
+            "rooms": {
+                room_id: {
+                    "initial": true,
+                    "timeline": [
+                        timeline_event!("$x0:bar.org" at 0 sec),
+                    ],
+                    "prev_batch": "prev-batch-token"
+                },
+            },
+        },
+    };
+
+    server.mock_room_state_encryption().plain().mount().await;
+
+    let f = EventFactory::new().room(room_id).sender(*ALICE);
+
+    // Send back-pagination responses with a small delay.
+    server
+        .mock_room_messages()
+        .respond_with(
+            ResponseTemplate::new(200)
+                .set_body_json(json!({
+                    "start": "unused-start",
+                    "end": null,
+                    "chunk": vec![f.text_msg("hello").into_raw_timeline()],
+                    "state": [],
+                }))
+                .set_delay(Duration::from_millis(500)),
+        )
+        .mount()
+        .await;
+
+    let task = {
+        // Get a RoomListService::Room, initialize the timeline, start a pagination.
+        let room = room_list.room(room_id).unwrap();
+
+        let builder = room.default_room_timeline_builder().await.unwrap();
+        room.init_timeline_with_builder(builder).await.unwrap();
+
+        let timeline = room.timeline().unwrap();
+
+        spawn(async move { timeline.paginate_backwards(20).await })
+    };
+
+    // Rinse and repeat.
+    let room = room_list.room(room_id).unwrap();
+
+    // Let the pagination start in the other timeline, and quickly abort it.
+    sleep(Duration::from_millis(200)).await;
+    task.abort();
+
+    // A new timeline for the same room can still be constructed.
+    let builder = room.default_room_timeline_builder().await.unwrap();
+    room.init_timeline_with_builder(builder).await.unwrap();
+}

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

@@ -319,6 +319,12 @@ impl EventCache {
 
         let room_cache = self.inner.for_room(room_id).await?;
 
+        // If the linked chunked already has at least one chunk (gap or events), ignore
+        // this request, as it should happen at most once per room.
+        if room_cache.inner.state.read().await.events().chunks().next().is_some() {
+            return Ok(());
+        }
+
         // We could have received events during a previous sync; remove them all, since
         // we can't know where to insert the "initial events" with respect to
         // them.