feat(sdk): Clean up and test SlidingSyncRoom

feat(sdk): Clean up and test `SlidingSyncRoom`

Author: Hywan

bindings/matrix-sdk-ffi/src/sliding_sync.rs

@@ -159,7 +159,7 @@ pub struct SlidingSyncRoom {
 #[uniffi::export]
 impl SlidingSyncRoom {
     pub fn name(&self) -> Option<String> {
-        self.inner.name().map(ToOwned::to_owned)
+        self.inner.name()
     }
 
     pub fn room_id(&self) -> String {
@@ -174,16 +174,12 @@ impl SlidingSyncRoom {
         self.inner.is_initial_response()
     }
 
-    pub fn is_loading_more(&self) -> bool {
-        self.inner.is_loading_more()
-    }
-
     pub fn has_unread_notifications(&self) -> bool {
         self.inner.has_unread_notifications()
     }
 
     pub fn unread_notifications(&self) -> Arc<UnreadNotificationsCount> {
-        Arc::new(self.inner.unread_notifications().clone().into())
+        Arc::new(self.inner.unread_notifications().into())
     }
 
     pub fn full_room(&self) -> Option<Arc<Room>> {
@@ -217,7 +213,7 @@ impl SlidingSyncRoom {
         settings: Option<RoomSubscription>,
     ) -> Result<SlidingSyncSubscribeResult, ClientError> {
         let (items, mut stoppable_spawn) = self.add_timeline_listener_inner(listener)?;
-        let room_id = self.inner.room_id().clone();
+        let room_id = self.inner.room_id().to_owned();
 
         self.runner.subscribe(room_id.clone(), settings.map(Into::into))?;
 

crates/matrix-sdk/src/sliding_sync/list/frozen.rs

@@ -4,7 +4,10 @@ use imbl::Vector;
 use ruma::OwnedRoomId;
 use serde::{Deserialize, Serialize};
 
-use super::{FrozenSlidingSyncRoom, RoomListEntry, SlidingSyncList, SlidingSyncRoom};
+use super::{
+    super::{FrozenSlidingSyncRoom, SlidingSyncRoom},
+    RoomListEntry, SlidingSyncList,
+};
 
 #[derive(Debug, Serialize, Deserialize)]
 pub struct FrozenSlidingSyncList {
@@ -72,7 +75,6 @@ mod tests {
                         FrozenSlidingSyncRoom {
                             room_id: room_id!("!foo:bar.org").to_owned(),
                             inner: v4::SlidingSyncRoom::default(),
-                            prev_batch: Some("let it go!".to_owned()),
                             timeline_queue: vector![TimelineEvent::new(
                                 Raw::new(&json!({
                                     "content": RoomMessageEventContent::text_plain("let it gooo!"),
@@ -100,7 +102,6 @@ mod tests {
                     "!foo:bar.org": {
                         "room_id": "!foo:bar.org",
                         "inner": {},
-                        "prev_batch": "let it go!",
                         "timeline": [
                             {
                                 "event": {

crates/matrix-sdk/src/sliding_sync/list/mod.rs

@@ -25,7 +25,7 @@ use serde::{Deserialize, Serialize};
 use tokio::sync::mpsc::Sender;
 use tracing::{instrument, warn};
 
-use super::{Error, FrozenSlidingSyncRoom, SlidingSyncInternalMessage, SlidingSyncRoom};
+use super::{Error, SlidingSyncInternalMessage};
 use crate::Result;
 
 /// The type used to express natural bounds (including but not limited to:

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

@@ -309,68 +309,76 @@ impl SlidingSync {
         }
 
         let update_summary = {
-            let mut rooms_map = self.inner.rooms.write().unwrap();
-
             // Update the rooms.
-            let mut updated_rooms = Vec::with_capacity(sliding_sync_response.rooms.len());
-
-            for (room_id, mut room_data) in sliding_sync_response.rooms.into_iter() {
-                // `sync_response` contains the rooms with decrypted events if any, so look at
-                // the timeline events here first if the room exists.
-                // Otherwise, let's look at the timeline inside the `sliding_sync_response`.
-                let timeline = if let Some(joined_room) = sync_response.rooms.join.remove(&room_id)
-                {
-                    joined_room.timeline.events
-                } else {
-                    room_data.timeline.drain(..).map(Into::into).collect()
-                };
-
-                if let Some(mut room) = rooms_map.remove(&room_id) {
-                    // The room existed before, let's update it.
-
-                    room.update(room_data, timeline);
-                    rooms_map.insert(room_id.clone(), room);
-                } else {
-                    // First time we need this room, let's create it.
-
-                    rooms_map.insert(
-                        room_id.clone(),
-                        SlidingSyncRoom::new(
-                            self.inner.client.clone(),
-                            room_id.clone(),
-                            room_data,
-                            timeline,
-                        ),
-                    );
+            let updated_rooms = {
+                let mut rooms_map = self.inner.rooms.write().unwrap();
+
+                let mut updated_rooms = Vec::with_capacity(sliding_sync_response.rooms.len());
+
+                for (room_id, mut room_data) in sliding_sync_response.rooms.into_iter() {
+                    // `sync_response` contains the rooms with decrypted events if any, so look at
+                    // the timeline events here first if the room exists.
+                    // Otherwise, let's look at the timeline inside the `sliding_sync_response`.
+                    let timeline =
+                        if let Some(joined_room) = sync_response.rooms.join.remove(&room_id) {
+                            joined_room.timeline.events
+                        } else {
+                            room_data.timeline.drain(..).map(Into::into).collect()
+                        };
+
+                    match rooms_map.get_mut(&room_id) {
+                        // The room existed before, let's update it.
+                        Some(room) => {
+                            room.update(room_data, timeline);
+                        }
+
+                        // First time we need this room, let's create it.
+                        None => {
+                            rooms_map.insert(
+                                room_id.clone(),
+                                SlidingSyncRoom::new(
+                                    self.inner.client.clone(),
+                                    room_id.clone(),
+                                    room_data,
+                                    timeline,
+                                ),
+                            );
+                        }
+                    }
+
+                    updated_rooms.push(room_id);
                 }
 
-                updated_rooms.push(room_id);
-            }
+                updated_rooms
+            };
 
             // Update the lists.
-            let mut updated_lists = Vec::with_capacity(sliding_sync_response.lists.len());
+            let updated_lists = {
+                let mut updated_lists = Vec::with_capacity(sliding_sync_response.lists.len());
+                let mut lists = self.inner.lists.write().unwrap();
 
-            let mut lists = self.inner.lists.write().unwrap();
+                for (name, updates) in sliding_sync_response.lists {
+                    let Some(list) = lists.get_mut(&name) else {
+                        error!("Response for list `{name}` - unknown to us; skipping");
 
-            for (name, updates) in sliding_sync_response.lists {
-                let Some(list) = lists.get_mut(&name) else {
-                    error!("Response for list `{name}` - unknown to us; skipping");
+                        continue;
+                    };
 
-                    continue;
-                };
+                    let maximum_number_of_rooms: u32 =
+                        updates.count.try_into().expect("failed to convert `count` to `u32`");
 
-                let maximum_number_of_rooms: u32 =
-                    updates.count.try_into().expect("failed to convert `count` to `u32`");
+                    if list.update(maximum_number_of_rooms, &updates.ops, &updated_rooms)? {
+                        updated_lists.push(name.clone());
+                    }
+                }
 
-                if list.update(maximum_number_of_rooms, &updates.ops, &updated_rooms)? {
-                    updated_lists.push(name.clone());
+                // Update the `to-device` next-batch if any.
+                if let Some(to_device) = sliding_sync_response.extensions.to_device {
+                    self.update_to_device_since(to_device.next_batch);
                 }
-            }
 
-            // Update the `to-device` next-batch if any.
-            if let Some(to_device) = sliding_sync_response.extensions.to_device {
-                self.update_to_device_since(to_device.next_batch);
-            }
+                updated_lists
+            };
 
             UpdateSummary { lists: updated_lists, rooms: updated_rooms }
         };