chore(sdk): Remove RoomListEntry and ops in Sliding Sync.

This patch removes everything related to the computation of `ops`
from a sliding sync response. With the recent `RoomList`'s client-side
sorting project, we no longer need to handle these `ops`. Moreover, the
simplified sliding sync specification that is coming removes the `ops`.

A `SlidingSyncList` was containing a `rooms` field. It's removed by
this patch. Consequently, all the `SlidingSyncList::room_list` and
`::room_list_stream` methods are also removed.

A `FrozenSlidingSyncList` was containing the `FrozenSlidingSyncRoom`.
This patch moves the `FrozenSlidingSyncRoom`s inside
`FrozenSlidingSync`. Why is it still correct? We only want to keep the
`SlidingSyncRoom::timeline_queue` in the cache for the moment (until
the `EventCache` has a persistent storage). Since a `SlidingSyncList`
no longer holds any information about the rooms, and since `SlidingSync`
itself has all the `SlidingSyncRoom`, this move is natural and still
valid.

Bye bye all this code :'-).

Author: Hywan

crates/matrix-sdk/src/lib.rs

@@ -86,7 +86,7 @@ pub use room::Room;
 pub use ruma::{IdParseError, OwnedServerName, ServerName};
 #[cfg(feature = "experimental-sliding-sync")]
 pub use sliding_sync::{
-    RoomListEntry, SlidingSync, SlidingSyncBuilder, SlidingSyncList, SlidingSyncListBuilder,
+    SlidingSync, SlidingSyncBuilder, SlidingSyncList, SlidingSyncListBuilder,
     SlidingSyncListLoadingState, SlidingSyncMode, SlidingSyncRoom, UpdateSummary,
 };
 

crates/matrix-sdk/src/sliding_sync/README.md

@@ -95,15 +95,11 @@ list-object. Once a list has been added to [`SlidingSync`], a cloned shared
 copy can be retrieved by calling `SlidingSync::list()`, providing the name
 of the list. Next to the configuration settings (like name and
 `timeline_limit`), the list provides the stateful
-[`maximum_number_of_rooms`](SlidingSyncList::maximum_number_of_rooms),
-[`room_list`](SlidingSyncList::room_list) and
+[`maximum_number_of_rooms`](SlidingSyncList::maximum_number_of_rooms) and
 [`state`](SlidingSyncList::state):
 
  - `maximum_number_of_rooms` is the number of rooms _total_ there were found
    matching the filters given.
- - `room_list` is a vector of `maximum_number_of_rooms` [`RoomListEntry`]'s
-   at the current state. `RoomListEntry`'s only hold `the room_id` if given,
-   the [Rooms API](#rooms) holds the actual information about each room
  - `state` is a [`SlidingSyncMode`] signalling meta information about the
    list and its stateful data — whether this is the state loaded from local
    cache, whether the [full sync](#helper-lists) is in progress or whether
@@ -149,32 +145,6 @@ visible in any list. The most common case for using this API is when the user
 enters a room - as we want to receive the incoming new messages regardless of
 whether the room is pushed out of the lists room list.
 
-### Room List Entries
-
-As the room list of each list is a vec of the `maximum_number_of_rooms` len
-but a room may only know of a subset of entries for sure at any given time,
-these entries are wrapped in [`RoomListEntry`][]. This type, in close
-proximity to the [specification][MSC], can be either `Empty`, `Filled` or
-`Invalidated`, signaling the state of each entry position.
-- `Empty` we don't know what sits here at this position in the list.
-- `Filled`: there is this `room_id` at this position.
-- `Invalidated` in that sense means that we _knew_ what was here before, but
-  can't be sure anymore this is still accurate. This occurs when we move the
-  sliding window (by changing the ranges) or when a room might drop out of
-  the window we are looking at. For the sake of displaying, this is probably
-  still fine to display to be at this position, but we can't be sure
-  anymore.
-
-Because `Invalidated` occurs whenever a room we knew about before drops out
-of focus, we aren't updated about its changes anymore either, there could be
-duplicates rooms within invalidated rooms as well as in the union of
-invalidated and filled rooms. Keep that in mind, as most UI frameworks don't
-like it when their list entries aren't unique.
-
-When [restoring from cold cache][#caching] the room list also only
-propagated with `Invalidated` rooms. So if you want to be able to display
-data quickly, ensure you are able to render `Invalidated` entries.
-
 ### Unsubscribe
 
 Don't forget to [unsubscribe](`SlidingSync::unsubscribe_from_room`) when the
@@ -345,14 +315,12 @@ subscribing to updates via [`eyeball`].
 
 ## Caching
 
-All room data, for filled but also _invalidated_ rooms, including the entire
-timeline events as well as all list `room_lists` and
-`maximum_number_of_rooms` are held in memory (unless one `pop`s the list
-out).
+All room data, including the entire timeline events as well as all lists and
+`maximum_number_of_rooms` are held in memory (unless one `pop`s the list out).
 
-Sliding Sync instances are also cached on disk, and restored from disk at creation. This ensures
-that we keep track of important position markers, like the `since` tokens used in the sync
-requests.
+Sliding Sync instances are also cached on disk, and restored from disk at
+creation. This ensures that we keep track of important position markers, like
+the `since` tokens used in the sync requests.
 
 Caching for lists can be enabled independently, using the
 [`add_cached_list`][`SlidingSyncBuilder::add_cached_list`] method, assuming
@@ -370,10 +338,6 @@ are stored to storage, but only retrieved upon `build()` of the
 the data from storage (to avoid inconsistencies) and might require more data to
 be sent in their first request than if they were loaded from a cold cache.
 
-When loading from storage `room_list` entries found are set to
-`Invalidated` — the initial setting here is communicated as a single
-`VecDiff::Replace` event through the [reactive API](#reactive-api).
-
 Only the latest 10 timeline items of each room are cached and they are reset
 whenever a new set of timeline items is received by the server.
 
@@ -446,30 +410,15 @@ let sliding_sync = sliding_sync_builder
     .build()
     .await?;
 
-// subscribe to the list APIs for updates
-
-let ((_, list_state_stream), list_count_stream, (_, list_stream)) = sliding_sync.on_list(&active_list_name, |list| {
-    ready((list.state_stream(), list.maximum_number_of_rooms_stream(), list.room_list_stream()))
-}).await.unwrap();
-
-tokio::spawn(async move {
-    pin_mut!(list_state_stream);
-    while let Some(new_state) = list_state_stream.next().await {
-        info!("active-list switched state to {new_state:?}");
-    }
-});
 
 tokio::spawn(async move {
-    pin_mut!(list_count_stream);
-    while let Some(new_count) = list_count_stream.next().await {
-        info!("active-list new count: {new_count:?}");
-    }
-});
+    // subscribe to rooms updates
+    let (_rooms, rooms_stream) = client.rooms_stream();
+    // do something with `_rooms`
 
-tokio::spawn(async move {
-    pin_mut!(list_stream);
-    while let Some(v_diff) = list_stream.next().await {
-        info!("active-list room list diff update: {v_diff:?}");
+    pin_mut!(rooms_stream);
+    while let Some(_room) = rooms_stream.next().await {
+        info!("a room has been updated");
     }
 });
 

crates/matrix-sdk/src/sliding_sync/builder.rs

@@ -95,16 +95,10 @@ impl SlidingSyncBuilder {
     /// cache.
     ///
     /// Replace any list with the same name.
-    pub async fn add_cached_list(mut self, mut list: SlidingSyncListBuilder) -> Result<Self> {
+    pub async fn add_cached_list(self, mut list: SlidingSyncListBuilder) -> Result<Self> {
         let _timer = timer!(format!("restoring (loading+processing) list {}", list.name));
 
-        let reloaded_rooms = list.set_cached_and_reload(&self.client, &self.storage_key).await?;
-
-        for (key, frozen) in reloaded_rooms {
-            self.rooms
-                .entry(key)
-                .or_insert_with(|| SlidingSyncRoom::from_frozen(frozen, self.client.clone()));
-        }
+        list.set_cached_and_reload(&self.client, &self.storage_key).await?;
 
         Ok(self.add_list(list))
     }

crates/matrix-sdk/src/sliding_sync/cache.rs

@@ -8,12 +8,12 @@ use std::collections::BTreeMap;
 
 use matrix_sdk_base::{StateStore, StoreError};
 use matrix_sdk_common::timer;
-use ruma::UserId;
+use ruma::{OwnedRoomId, UserId};
 use tracing::{trace, warn};
 
 use super::{
     FrozenSlidingSync, FrozenSlidingSyncList, SlidingSync, SlidingSyncList,
-    SlidingSyncPositionMarkers,
+    SlidingSyncPositionMarkers, SlidingSyncRoom,
 };
 #[cfg(feature = "e2e-encryption")]
 use crate::sliding_sync::FrozenSlidingSyncPos;
@@ -88,7 +88,10 @@ pub(super) async fn store_sliding_sync_state(
     storage
         .set_custom_value(
             instance_storage_key.as_bytes(),
-            serde_json::to_vec(&FrozenSlidingSync::new(position))?,
+            serde_json::to_vec(&FrozenSlidingSync::new(
+                position,
+                &*sliding_sync.inner.rooms.read().await,
+            ))?,
         )
         .await?;
 
@@ -106,8 +109,6 @@ pub(super) async fn store_sliding_sync_state(
 
     // Write every `SlidingSyncList` that's configured for caching into the store.
     let frozen_lists = {
-        let rooms_lock = sliding_sync.inner.rooms.read().await;
-
         sliding_sync
             .inner
             .lists
@@ -118,7 +119,7 @@ pub(super) async fn store_sliding_sync_state(
             .map(|(list_name, list)| {
                 Ok((
                     format_storage_key_for_sliding_sync_list(storage_key, list_name),
-                    serde_json::to_vec(&FrozenSlidingSyncList::freeze(list, &rooms_lock))?,
+                    serde_json::to_vec(&FrozenSlidingSyncList::freeze(list))?,
                 ))
             })
             .collect::<Result<Vec<_>, crate::Error>>()?
@@ -163,9 +164,9 @@ pub(super) async fn restore_sliding_sync_list(
             // error, we remove the entry from the cache and keep the list in its initial
             // state.
             warn!(
-                    list_name,
-                    "failed to deserialize the list from the cache, it is obsolete; removing the cache entry!"
-                );
+                list_name,
+                "failed to deserialize the list from the cache, it is obsolete; removing the cache entry!"
+            );
             // Let's clear the list and stop here.
             invalidate_cached_list(storage, storage_key, list_name).await;
         }
@@ -186,6 +187,7 @@ pub(super) struct RestoredFields {
     pub delta_token: Option<String>,
     pub to_device_token: Option<String>,
     pub pos: Option<String>,
+    pub rooms: BTreeMap<OwnedRoomId, SlidingSyncRoom>,
 }
 
 /// Restore the `SlidingSync`'s state from what is stored in the storage.
@@ -221,7 +223,11 @@ pub(super) async fn restore_sliding_sync_state(
         .map(|custom_value| serde_json::from_slice::<FrozenSlidingSync>(&custom_value))
     {
         // `SlidingSync` has been found and successfully deserialized.
-        Some(Ok(FrozenSlidingSync { to_device_since, delta_token: frozen_delta_token })) => {
+        Some(Ok(FrozenSlidingSync {
+            to_device_since,
+            delta_token: frozen_delta_token,
+            rooms: frozen_rooms,
+        })) => {
             trace!("Successfully read the `SlidingSync` from the cache");
             // Only update the to-device token if we failed to read it from the crypto store
             // above.
@@ -246,6 +252,16 @@ pub(super) async fn restore_sliding_sync_state(
                     }
                 }
             }
+
+            restored_fields.rooms = frozen_rooms
+                .into_iter()
+                .map(|frozen_room| {
+                    (
+                        frozen_room.room_id.clone(),
+                        SlidingSyncRoom::from_frozen(frozen_room, client.clone()),
+                    )
+                })
+                .collect();
         }
 
         // `SlidingSync` has been found, but it wasn't possible to deserialize it. It's
@@ -281,11 +297,11 @@ mod tests {
     use matrix_sdk_test::async_test;
 
     use super::{
-        clean_storage, format_storage_key_for_sliding_sync,
+        super::FrozenSlidingSyncRoom, clean_storage, format_storage_key_for_sliding_sync,
         format_storage_key_for_sliding_sync_list, format_storage_key_prefix,
-        restore_sliding_sync_state, store_sliding_sync_state,
+        restore_sliding_sync_state, store_sliding_sync_state, SlidingSyncList,
     };
-    use crate::{test_utils::logged_in_client, Result, SlidingSyncList};
+    use crate::{test_utils::logged_in_client, Result};
 
     #[allow(clippy::await_holding_lock)]
     #[async_test]
@@ -440,6 +456,9 @@ mod tests {
     #[cfg(feature = "e2e-encryption")]
     #[async_test]
     async fn test_sliding_sync_high_level_cache_and_restore() -> Result<()> {
+        use imbl::Vector;
+        use ruma::owned_room_id;
+
         use crate::sliding_sync::FrozenSlidingSync;
 
         let client = logged_in_client(Some("https://foo.bar".to_owned())).await;
@@ -513,6 +532,11 @@ mod tests {
                 serde_json::to_vec(&FrozenSlidingSync {
                     to_device_since: Some(to_device_token.clone()),
                     delta_token: Some(delta_token.clone()),
+                    rooms: vec![FrozenSlidingSyncRoom {
+                        room_id: owned_room_id!("!r0:matrix.org"),
+                        prev_batch: Some("t0ken".to_owned()),
+                        timeline_queue: Vector::new(),
+                    }],
                 })?,
             )
             .await?;
@@ -521,11 +545,12 @@ mod tests {
             .await?
             .expect("must have restored fields");
 
-        // After restoring, the delta token, the to-device since token, and stream
-        // position could be read from the state store.
+        // After restoring, the delta token, the to-device since token, stream
+        // position and rooms could be read from the state store.
         assert_eq!(restored_fields.delta_token.unwrap(), delta_token);
         assert_eq!(restored_fields.to_device_token.unwrap(), to_device_token);
         assert_eq!(restored_fields.pos.unwrap(), pos);
+        assert_eq!(restored_fields.rooms.len(), 1);
 
         Ok(())
     }