feat(room_list): make RoomListService updates subscriptions non-destructively

Signed-off-by: Johannes Marbach <n0-0ne+github@mailbox.org>

Author: Johennes

crates/matrix-sdk-ui/changelog.d/6652.changed.md

@@ -0,0 +1 @@
+`RoomListService::subscribe_to_rooms` now updates subscriptions non-destructively.

crates/matrix-sdk-ui/src/room_list_service/mod.rs

@@ -474,7 +474,8 @@ impl RoomListService {
     /// room in `room_ids`, so that the [`LatestEventValue`] will automatically
     /// be calculated and updated for these rooms, for free.
     ///
-    /// All previous room subscriptions will be forgotten.
+    /// Previous room subscriptions that are not contained in the specified room
+    /// IDs will be forgotten.
     ///
     /// [listen_to_room]: matrix_sdk::latest_events::LatestEvents::listen_to_room
     /// [`LatestEventValue`]: matrix_sdk::latest_events::LatestEventValue
@@ -516,11 +517,7 @@ impl RoomListService {
         }
 
         // Subscribe to the rooms.
-        self.sliding_sync.clear_and_subscribe_to_rooms(
-            room_ids,
-            Some(settings),
-            cancel_in_flight_request,
-        )
+        self.sliding_sync.resubscribe_to_rooms(room_ids, Some(settings), cancel_in_flight_request)
     }
 
     #[cfg(test)]

crates/matrix-sdk/changelog.d/6652.added.md

@@ -0,0 +1 @@
+Add `SlidingSync::resubscribe_to_rooms` for updating room subscriptions non-destructively.

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

@@ -175,6 +175,42 @@ impl SlidingSync {
         }
     }
 
+    /// Add subscriptions to the specified rooms if they don't already exist and
+    /// remove any existing subscriptions to other rooms.
+    pub fn resubscribe_to_rooms(
+        &self,
+        room_ids: &[&RoomId],
+        settings: Option<http::request::RoomSubscription>,
+        cancel_in_flight_request: bool,
+    ) {
+        let mut room_subscriptions = self.inner.room_subscriptions.write().unwrap();
+        let mut skip_over_current_sync_loop_iteration = false;
+
+        // Remove rooms that are no longer subscribed.
+        let to_remove: Vec<_> = room_subscriptions
+            .keys()
+            .filter(|room_id| !room_ids.contains(&room_id.as_ref())).cloned()
+            .collect();
+        for room_id in to_remove {
+            println!("unsub {:?}", room_id);
+            if room_subscriptions.remove(&room_id).is_some() {
+                skip_over_current_sync_loop_iteration = true;
+            }
+        }
+
+        if subscribe_to_rooms(
+            room_subscriptions,
+            &self.inner.client,
+            room_ids,
+            settings,
+            cancel_in_flight_request && skip_over_current_sync_loop_iteration,
+        ) {
+            self.inner.internal_channel_send_if_possible(
+                SlidingSyncInternalMessage::SyncLoopSkipOverCurrentIteration,
+            );
+        }
+    }
+
     /// Replace all subscriptions to rooms by other ones.
     ///
     /// If the associated `Room`s exist, they will be marked as members are
@@ -186,15 +222,27 @@ impl SlidingSync {
         cancel_in_flight_request: bool,
     ) {
         let mut room_subscriptions = self.inner.room_subscriptions.write().unwrap();
-        room_subscriptions.clear();
+
+        // Remove rooms that are no longer subscribed.
+        let to_remove: Vec<_> = room_ids
+            .iter()
+            .filter(|room_id| room_subscriptions.contains_key(*room_id.to_owned()))
+            .collect();
+        let mut any_removed = false;
+        for room_id in to_remove {
+            if room_subscriptions.remove(*room_id).is_some() {
+                any_removed = true;
+            }
+        }
 
         if subscribe_to_rooms(
             room_subscriptions,
             &self.inner.client,
             room_ids,
             settings,
             cancel_in_flight_request,
-        ) {
+        ) || any_removed
+        {
             self.inner.internal_channel_send_if_possible(
                 SlidingSyncInternalMessage::SyncLoopSkipOverCurrentIteration,
             );
@@ -1205,6 +1253,136 @@ mod tests {
         Ok(())
     }
 
+    #[async_test]
+    async fn test_resubscribe_to_rooms() -> Result<()> {
+        let (server, sliding_sync) = new_sliding_sync(vec![
+            SlidingSyncList::builder("foo")
+                .sync_mode(SlidingSyncMode::new_selective().add_range(0..=10)),
+        ])
+        .await?;
+
+        let stream = sliding_sync.sync();
+        pin_mut!(stream);
+
+        let room_id_0 = room_id!("!r0:bar.org");
+        let room_id_1 = room_id!("!r1:bar.org");
+        let room_id_2 = room_id!("!r2:bar.org");
+
+        {
+            let _mock_guard = Mock::given(SlidingSyncMatcher)
+                .respond_with(ResponseTemplate::new(200).set_body_json(json!({
+                    "pos": "1",
+                    "lists": {},
+                    "rooms": {
+                        room_id_0: {
+                            "name": "Room #0",
+                            "initial": true,
+                        },
+                        room_id_1: {
+                            "name": "Room #1",
+                            "initial": true,
+                        },
+                        room_id_2: {
+                            "name": "Room #2",
+                            "initial": true,
+                        },
+                    }
+                })))
+                .mount_as_scoped(&server)
+                .await;
+
+            let _ = stream.next().await.unwrap()?;
+        }
+
+        let room0 = sliding_sync.inner.client.get_room(room_id_0).unwrap();
+
+        // Members aren't synced.
+        // We need to make them synced, so that we can test that subscribing to a room
+        // make members not synced. That's a desired feature.
+        assert!(room0.are_members_synced().not());
+
+        {
+            struct MemberMatcher(OwnedRoomId);
+
+            impl Match for MemberMatcher {
+                fn matches(&self, request: &Request) -> bool {
+                    request.url.path()
+                        == format!("/_matrix/client/r0/rooms/{room_id}/members", room_id = self.0)
+                        && request.method == Method::GET
+                }
+            }
+
+            let _mock_guard = Mock::given(MemberMatcher(room_id_0.to_owned()))
+                .respond_with(ResponseTemplate::new(200).set_body_json(json!({
+                    "chunk": [],
+                })))
+                .mount_as_scoped(&server)
+                .await;
+
+            assert_matches!(room0.request_members().await, Ok(()));
+        }
+
+        // Members are now synced! We can start subscribing and see how it goes.
+        assert!(room0.are_members_synced());
+
+        sliding_sync.resubscribe_to_rooms(&[room_id_0, room_id_1], None, true);
+
+        // OK, we have subscribed to some rooms. Let's check on `room0` if members are
+        // now marked as not synced.
+        assert!(room0.are_members_synced().not());
+
+        // Both resubscribed rooms are subscribed.
+        {
+            let room_subscriptions = sliding_sync.inner.room_subscriptions.read().unwrap();
+
+            assert!(room_subscriptions.contains_key(room_id_0));
+            assert!(room_subscriptions.contains_key(room_id_1));
+            assert!(!room_subscriptions.contains_key(room_id_2));
+        }
+
+        // Subscribing to the same room doesn't reset the member sync state.
+
+        {
+            struct MemberMatcher(OwnedRoomId);
+
+            impl Match for MemberMatcher {
+                fn matches(&self, request: &Request) -> bool {
+                    request.url.path()
+                        == format!("/_matrix/client/r0/rooms/{room_id}/members", room_id = self.0)
+                        && request.method == Method::GET
+                }
+            }
+
+            let _mock_guard = Mock::given(MemberMatcher(room_id_0.to_owned()))
+                .respond_with(ResponseTemplate::new(200).set_body_json(json!({
+                    "chunk": [],
+                })))
+                .mount_as_scoped(&server)
+                .await;
+
+            assert_matches!(room0.request_members().await, Ok(()));
+        }
+
+        // Members are synced, good, good.
+        assert!(room0.are_members_synced());
+
+        sliding_sync.resubscribe_to_rooms(&[room_id_0], None, false);
+
+        // Members are still synced: because we have already subscribed to the
+        // room, the members aren't marked as unsynced.
+        assert!(room0.are_members_synced());
+
+        // Only the resubscribed room is subscribed.
+        {
+            let room_subscriptions = sliding_sync.inner.room_subscriptions.read().unwrap();
+
+            assert!(room_subscriptions.contains_key(room_id_0));
+            assert!(!room_subscriptions.contains_key(room_id_1));
+            assert!(!room_subscriptions.contains_key(room_id_2));
+        }
+        Ok(())
+    }
+
     #[async_test]
     async fn test_room_subscriptions_are_reset_when_session_expires() -> Result<()> {
         let (_server, sliding_sync) = new_sliding_sync(vec![