Merge pull request #3551 from Hywan/fix-ui-roomlist-slidingsyncroom

feat(ui): `RoomListService::room` is no longer async!

Author: Hywan

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

@@ -125,11 +125,11 @@ impl RoomListService {
         })))
     }
 
-    async fn room(&self, room_id: String) -> Result<Arc<RoomListItem>, RoomListError> {
+    fn room(&self, room_id: String) -> Result<Arc<RoomListItem>, RoomListError> {
         let room_id = <&RoomId>::try_from(room_id.as_str()).map_err(RoomListError::from)?;
 
         Ok(Arc::new(RoomListItem {
-            inner: Arc::new(self.inner.room(room_id).await?),
+            inner: Arc::new(self.inner.room(room_id)?),
             utd_hook: self.utd_hook.clone(),
         }))
     }
@@ -172,7 +172,7 @@ pub struct RoomList {
     inner: Arc<matrix_sdk_ui::room_list_service::RoomList>,
 }
 
-#[uniffi::export(async_runtime = "tokio")]
+#[uniffi::export]
 impl RoomList {
     fn loading_state(
         &self,
@@ -233,8 +233,8 @@ impl RoomList {
         }
     }
 
-    async fn room(&self, room_id: String) -> Result<Arc<RoomListItem>, RoomListError> {
-        self.room_list_service.room(room_id).await
+    fn room(&self, room_id: String) -> Result<Arc<RoomListItem>, RoomListError> {
+        self.room_list_service.room(room_id)
     }
 }
 

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

@@ -66,7 +66,12 @@ mod room;
 mod room_list;
 mod state;
 
-use std::{future::ready, num::NonZeroUsize, sync::Arc, time::Duration};
+use std::{
+    future::ready,
+    num::NonZeroUsize,
+    sync::{Arc, Mutex as StdMutex},
+    time::Duration,
+};
 
 use async_stream::stream;
 use eyeball::{SharedObservable, Subscriber};
@@ -90,10 +95,7 @@ use ruma::{
 };
 pub use state::*;
 use thiserror::Error;
-use tokio::{
-    sync::{Mutex, RwLock},
-    time::timeout,
-};
+use tokio::{sync::Mutex, time::timeout};
 
 use crate::timeline;
 
@@ -112,7 +114,7 @@ pub struct RoomListService {
     state: SharedObservable<State>,
 
     /// Room cache, to avoid recreating `Room`s every time users fetch them.
-    rooms: Arc<RwLock<RingBuffer<Room>>>,
+    rooms: Arc<StdMutex<RingBuffer<Room>>>,
 
     /// The current viewport ranges.
     ///
@@ -202,7 +204,7 @@ impl RoomListService {
             client,
             sliding_sync,
             state: SharedObservable::new(State::Init),
-            rooms: Arc::new(RwLock::new(RingBuffer::new(Self::ROOM_OBJECT_CACHE_SIZE))),
+            rooms: Arc::new(StdMutex::new(RingBuffer::new(Self::ROOM_OBJECT_CACHE_SIZE))),
             viewport_ranges: Mutex::new(vec![VISIBLE_ROOMS_DEFAULT_RANGE]),
         })
     }
@@ -424,21 +426,17 @@ impl RoomListService {
     }
 
     /// Get a [`Room`] if it exists.
-    pub async fn room(&self, room_id: &RoomId) -> Result<Room, Error> {
-        {
-            let rooms = self.rooms.read().await;
+    pub fn room(&self, room_id: &RoomId) -> Result<Room, Error> {
+        let mut rooms = self.rooms.lock().unwrap();
 
-            if let Some(room) = rooms.iter().rfind(|room| room.id() == room_id) {
-                return Ok(room.clone());
-            }
+        if let Some(room) = rooms.iter().rfind(|room| room.id() == room_id) {
+            return Ok(room.clone());
         }
 
-        let room = match self.sliding_sync.get_room(room_id).await {
-            Some(room) => Room::new(self.sliding_sync.clone(), room)?,
-            None => return Err(Error::RoomNotFound(room_id.to_owned())),
-        };
+        let room = Room::new(&self.client, room_id, &self.sliding_sync)?;
 
-        self.rooms.write().await.push(room.clone());
+        // Save for later.
+        rooms.push(room.clone());
 
         Ok(room)
     }

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

@@ -17,7 +17,7 @@
 use std::{ops::Deref, sync::Arc};
 
 use async_once_cell::OnceCell as AsyncOnceCell;
-use matrix_sdk::{event_cache, SlidingSync, SlidingSyncRoom};
+use matrix_sdk::{Client, SlidingSync};
 use ruma::{api::client::sync::sync_events::v4::RoomSubscription, events::StateEventType, RoomId};
 
 use super::Error;
@@ -39,9 +39,6 @@ struct RoomInner {
     /// The Sliding Sync where everything comes from.
     sliding_sync: Arc<SlidingSync>,
 
-    /// The Sliding Sync room.
-    sliding_sync_room: SlidingSyncRoom,
-
     /// The underlying client room.
     room: matrix_sdk::Room,
 
@@ -60,18 +57,16 @@ impl Deref for Room {
 impl Room {
     /// Create a new `Room`.
     pub(super) fn new(
-        sliding_sync: Arc<SlidingSync>,
-        sliding_sync_room: SlidingSyncRoom,
+        client: &Client,
+        room_id: &RoomId,
+        sliding_sync: &Arc<SlidingSync>,
     ) -> Result<Self, Error> {
-        let room = sliding_sync_room
-            .client()
-            .get_room(sliding_sync_room.room_id())
-            .ok_or_else(|| Error::RoomNotFound(sliding_sync_room.room_id().to_owned()))?;
+        let room =
+            client.get_room(room_id).ok_or_else(|| Error::RoomNotFound(room_id.to_owned()))?;
 
         Ok(Self {
             inner: Arc::new(RoomInner {
-                sliding_sync,
-                sliding_sync_room,
+                sliding_sync: sliding_sync.clone(),
                 room,
                 timeline: AsyncOnceCell::new(),
             }),
@@ -185,18 +180,27 @@ impl Room {
     }
 
     /// Create a new [`TimelineBuilder`] with the default configuration.
-    pub async fn default_room_timeline_builder(&self) -> event_cache::Result<TimelineBuilder> {
+    pub async fn default_room_timeline_builder(&self) -> Result<TimelineBuilder, Error> {
         // TODO we can remove this once the event cache handles his own cache.
+
+        let sliding_sync_room =
+            self.inner
+                .sliding_sync
+                .get_room(self.inner.room.room_id())
+                .await
+                .ok_or_else(|| Error::RoomNotFound(self.inner.room.room_id().to_owned()))?;
+
         self.inner
             .room
             .client()
             .event_cache()
             .add_initial_events(
                 self.inner.room.room_id(),
-                self.inner.sliding_sync_room.timeline_queue().iter().cloned().collect(),
-                self.inner.sliding_sync_room.prev_batch(),
+                sliding_sync_room.timeline_queue().iter().cloned().collect(),
+                sliding_sync_room.prev_batch(),
             )
-            .await?;
+            .await
+            .map_err(Error::EventCache)?;
 
         Ok(Timeline::builder(&self.inner.room).track_read_marker_and_receipts())
     }

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

@@ -2072,15 +2072,15 @@ async fn test_room() -> Result<(), Error> {
         },
     };
 
-    let room0 = room_list.room(room_id_0).await?;
+    let room0 = room_list.room(room_id_0)?;
 
     // Room has received a name from sliding sync.
     assert_eq!(room0.cached_display_name(), Some("Room #0".to_owned()));
 
     // Room has received an avatar from sliding sync.
     assert_eq!(room0.avatar_url(), Some(mxc_uri!("mxc://homeserver/media").to_owned()));
 
-    let room1 = room_list.room(room_id_1).await?;
+    let room1 = room_list.room(room_id_1)?;
 
     // Room has not received a name from sliding sync, then it's calculated.
     assert_eq!(room1.cached_display_name(), Some("Empty Room".to_owned()));
@@ -2144,7 +2144,7 @@ async fn test_room_not_found() -> Result<(), Error> {
     let room_id = room_id!("!foo:bar.org");
 
     assert_matches!(
-        room_list.room(room_id).await,
+        room_list.room(room_id),
         Err(Error::RoomNotFound(error_room_id)) => {
             assert_eq!(error_room_id, room_id.to_owned());
         }
@@ -2208,7 +2208,7 @@ async fn test_room_subscription() -> Result<(), Error> {
         },
     };
 
-    let room1 = room_list.room(room_id_1).await.unwrap();
+    let room1 = room_list.room(room_id_1).unwrap();
 
     // Subscribe.
 
@@ -2253,7 +2253,7 @@ async fn test_room_subscription() -> Result<(), Error> {
     // Unsubscribe.
 
     room1.unsubscribe();
-    room_list.room(room_id_2).await?.unsubscribe(); // unsubscribe from a room that has no subscription.
+    room_list.room(room_id_2)?.unsubscribe(); // unsubscribe from a room that has no subscription.
 
     sync_then_assert_request_and_fake_response! {
         [server, room_list, sync]
@@ -2316,7 +2316,7 @@ async fn test_room_unread_notifications() -> Result<(), Error> {
         },
     };
 
-    let room = room_list.room(room_id).await.unwrap();
+    let room = room_list.room(room_id).unwrap();
 
     assert_matches!(
         room.unread_notification_counts(),
@@ -2391,7 +2391,7 @@ async fn test_room_timeline() -> Result<(), Error> {
         },
     };
 
-    let room = room_list.room(room_id).await?;
+    let room = room_list.room(room_id)?;
     room.init_timeline_with_builder(room.default_room_timeline_builder().await.unwrap()).await?;
     let timeline = room.timeline().unwrap();
 
@@ -2473,7 +2473,7 @@ async fn test_room_latest_event() -> Result<(), Error> {
         },
     };
 
-    let room = room_list.room(room_id).await?;
+    let room = room_list.room(room_id)?;
     room.init_timeline_with_builder(room.default_room_timeline_builder().await.unwrap()).await?;
 
     // The latest event does not exist.

labs/multiverse/src/main.rs

@@ -227,7 +227,7 @@ impl App {
                     all_rooms.into_iter().filter(|room_id| !previous_ui_rooms.contains_key(room_id))
                 {
                     // Retrieve the room list service's Room.
-                    let Ok(ui_room) = sync_service.room_list_service().room(&room_id).await else {
+                    let Ok(ui_room) = sync_service.room_list_service().room(&room_id) else {
                         error!("error when retrieving room after an update");
                         continue;
                     };