From 99e78488b6946095870da36d27efeb2ecda18142 Mon Sep 17 00:00:00 2001 From: Stefan Ceriu Date: Fri, 1 Aug 2025 14:15:28 +0300 Subject: [PATCH 01/35] chore(spaces): setup a simple space service and some unit tests --- bindings/matrix-sdk-ffi/src/client.rs | 7 ++ bindings/matrix-sdk-ffi/src/lib.rs | 1 + bindings/matrix-sdk-ffi/src/spaces.rs | 48 ++++++++ crates/matrix-sdk-ui/src/lib.rs | 1 + crates/matrix-sdk-ui/src/spaces/mod.rs | 159 +++++++++++++++++++++++++ 5 files changed, 216 insertions(+) create mode 100644 bindings/matrix-sdk-ffi/src/spaces.rs create mode 100644 crates/matrix-sdk-ui/src/spaces/mod.rs diff --git a/bindings/matrix-sdk-ffi/src/client.rs b/bindings/matrix-sdk-ffi/src/client.rs index ef8724be79d..43bfc9dfcaa 100644 --- a/bindings/matrix-sdk-ffi/src/client.rs +++ b/bindings/matrix-sdk-ffi/src/client.rs @@ -48,6 +48,7 @@ use matrix_sdk_ui::{ NotificationClient as MatrixNotificationClient, NotificationProcessSetup as MatrixNotificationProcessSetup, }, + spaces::SpaceService as UISpaceService, unable_to_decrypt_hook::UtdHookManager, }; use mime::Mime; @@ -111,6 +112,7 @@ use crate::{ MediaPreviews, MediaSource, RoomAccountDataEvent, RoomAccountDataEventType, }, runtime::get_runtime_handle, + spaces::SpaceService, sync_service::{SyncService, SyncServiceBuilder}, task_handle::TaskHandle, utd::{UnableToDecryptDelegate, UtdHook}, @@ -1257,6 +1259,11 @@ impl Client { SyncServiceBuilder::new((*self.inner).clone(), self.utd_hook_manager.get().cloned()) } + pub fn spaces_service(&self) -> Arc { + let inner = UISpaceService::new((*self.inner).clone()); + Arc::new(SpaceService::new(inner)) + } + pub async fn get_notification_settings(&self) -> Arc { let inner = self.inner.notification_settings().await; diff --git a/bindings/matrix-sdk-ffi/src/lib.rs b/bindings/matrix-sdk-ffi/src/lib.rs index 07077841f53..09fd4a3657e 100644 --- a/bindings/matrix-sdk-ffi/src/lib.rs +++ b/bindings/matrix-sdk-ffi/src/lib.rs @@ -25,6 +25,7 @@ mod room_preview; mod ruma; mod runtime; mod session_verification; +mod spaces; mod sync_service; mod task_handle; mod timeline; diff --git a/bindings/matrix-sdk-ffi/src/spaces.rs b/bindings/matrix-sdk-ffi/src/spaces.rs new file mode 100644 index 00000000000..b6b44aafa8c --- /dev/null +++ b/bindings/matrix-sdk-ffi/src/spaces.rs @@ -0,0 +1,48 @@ +// Copyright 2025 The Matrix.org Foundation C.I.C. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +use matrix_sdk_ui::spaces::SpaceService as UISpaceService; +use ruma::RoomId; + +use crate::error::ClientError; + +#[derive(uniffi::Object)] +pub struct SpaceService { + inner: UISpaceService, +} + +impl SpaceService { + /// Create a new instance of `SpaceService`. + pub fn new(inner: UISpaceService) -> Self { + Self { inner } + } +} + +#[matrix_sdk_ffi_macros::export] +impl SpaceService { + /// Get the list of joined spaces. + pub fn joined_spaces(&self) -> Vec { + self.inner.joined_spaces().into_iter().map(|id| id.to_string()).collect() + } + + /// Get the top-level children for a given space. + pub async fn top_level_children_for( + &self, + space_id: String, + ) -> Result, ClientError> { + let space_id = RoomId::parse(space_id)?; + let children = self.inner.top_level_children_for(space_id).await?; + Ok(children.into_iter().map(|id| id.to_string()).collect()) + } +} diff --git a/crates/matrix-sdk-ui/src/lib.rs b/crates/matrix-sdk-ui/src/lib.rs index 1c7a2a97ab1..05ad2ffa5f4 100644 --- a/crates/matrix-sdk-ui/src/lib.rs +++ b/crates/matrix-sdk-ui/src/lib.rs @@ -21,6 +21,7 @@ use ruma::html::HtmlSanitizerMode; pub mod encryption_sync_service; pub mod notification_client; pub mod room_list_service; +pub mod spaces; pub mod sync_service; pub mod timeline; pub mod unable_to_decrypt_hook; diff --git a/crates/matrix-sdk-ui/src/spaces/mod.rs b/crates/matrix-sdk-ui/src/spaces/mod.rs new file mode 100644 index 00000000000..d207eb7ae40 --- /dev/null +++ b/crates/matrix-sdk-ui/src/spaces/mod.rs @@ -0,0 +1,159 @@ +// Copyright 2025 The Matrix.org Foundation C.I.C. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for that specific language governing permissions and +// limitations under the License. + +//! High level interfaces for working with Spaces +//! +//! See [`SpaceDiscoveryService`] for details. + +use matrix_sdk::{Client, Error}; +use ruma::OwnedRoomId; +use ruma::api::client::space::get_hierarchy; + +pub struct SpaceService { + client: Client, +} + +impl SpaceService { + pub fn new(client: Client) -> Self { + Self { client } + } + + pub fn joined_spaces(&self) -> Vec { + self.client + .joined_rooms() + .into_iter() + .filter_map(|room| if room.is_space() { Some(room.room_id().to_owned()) } else { None }) + .collect() + } + + pub async fn top_level_children_for( + &self, + space_id: OwnedRoomId, + ) -> Result, Error> { + let request = get_hierarchy::v1::Request::new(space_id.clone()); + + let result = self.client.send(request).await?; + + println!("Top level children for space {}: {:?}", space_id, result.rooms); + + Ok(vec![]) + } +} + +#[cfg(test)] +mod tests { + use super::*; + use assert_matches2::assert_let; + use matrix_sdk::{room::ParentSpace, test_utils::mocks::MatrixMockServer}; + use matrix_sdk_test::{JoinedRoomBuilder, async_test, event_factory::EventFactory}; + use ruma::{RoomVersionId, room_id}; + use tokio_stream::StreamExt; + + #[async_test] + async fn test_spaces_hierarchy() { + let server = MatrixMockServer::new().await; + let client = server.client_builder().build().await; + let user_id = client.user_id().unwrap(); + let space_service = SpaceService::new(client.clone()); + let factory = EventFactory::new(); + + server.mock_room_state_encryption().plain().mount().await; + + // Given one parent space with 2 children spaces + + let parent_space_id = room_id!("!3:example.org"); + let child_space_id_1 = room_id!("!1:example.org"); + let child_space_id_2 = room_id!("!2:example.org"); + + server + .sync_room( + &client, + JoinedRoomBuilder::new(child_space_id_1) + .add_state_event(factory.create(user_id, RoomVersionId::V1).with_space_type()) + .add_state_event( + factory + .space_parent(parent_space_id.to_owned(), child_space_id_1.to_owned()) + .sender(user_id), + ), + ) + .await; + + server + .sync_room( + &client, + JoinedRoomBuilder::new(child_space_id_2) + .add_state_event(factory.create(user_id, RoomVersionId::V1).with_space_type()) + .add_state_event( + factory + .space_parent(parent_space_id.to_owned(), child_space_id_2.to_owned()) + .sender(user_id), + ), + ) + .await; + server + .sync_room( + &client, + JoinedRoomBuilder::new(parent_space_id) + .add_state_event(factory.create(user_id, RoomVersionId::V1).with_space_type()) + .add_state_event( + factory + .space_child(parent_space_id.to_owned(), child_space_id_1.to_owned()) + .sender(user_id), + ) + .add_state_event( + factory + .space_child(parent_space_id.to_owned(), child_space_id_2.to_owned()) + .sender(user_id), + ), + ) + .await; + + // All joined + assert_eq!( + space_service.joined_spaces(), + vec![child_space_id_1, child_space_id_2, parent_space_id] + ); + + let parent_space = client.get_room(parent_space_id).unwrap(); + assert!(parent_space.is_space()); + + // Then the parent space and the two child spaces are linked + + let spaces: Vec = client + .get_room(child_space_id_1) + .unwrap() + .parent_spaces() + .await + .unwrap() + .map(Result::unwrap) + .collect() + .await; + + assert_let!(ParentSpace::Reciprocal(parent) = spaces.first().unwrap()); + assert_eq!(parent.room_id(), parent_space.room_id()); + + let spaces: Vec = client + .get_room(child_space_id_2) + .unwrap() + .parent_spaces() + .await + .unwrap() + .map(Result::unwrap) + .collect() + .await; + + assert_let!(ParentSpace::Reciprocal(parent) = spaces.last().unwrap()); + assert_eq!(parent.room_id(), parent_space.room_id()); + } +} From b4606fe44d95706cc0f6ea512393f435cd4cc861 Mon Sep 17 00:00:00 2001 From: Stefan Ceriu Date: Mon, 4 Aug 2025 17:07:30 +0300 Subject: [PATCH 02/35] chore(spaces): introduce a `SpaceServiceRoom` --- bindings/matrix-sdk-ffi/src/room_preview.rs | 12 ++-- bindings/matrix-sdk-ffi/src/spaces.rs | 55 +++++++++++++-- crates/matrix-sdk-ui/src/spaces/mod.rs | 24 ++++--- crates/matrix-sdk-ui/src/spaces/room.rs | 76 +++++++++++++++++++++ 4 files changed, 147 insertions(+), 20 deletions(-) create mode 100644 crates/matrix-sdk-ui/src/spaces/room.rs diff --git a/bindings/matrix-sdk-ffi/src/room_preview.rs b/bindings/matrix-sdk-ffi/src/room_preview.rs index f14d738c6c1..8b04d708ec1 100644 --- a/bindings/matrix-sdk-ffi/src/room_preview.rs +++ b/bindings/matrix-sdk-ffi/src/room_preview.rs @@ -31,10 +31,10 @@ impl RoomPreview { avatar_url: info.avatar_url.as_ref().map(|url| url.to_string()), num_joined_members: info.num_joined_members, num_active_members: info.num_active_members, - room_type: info.room_type.as_ref().into(), + room_type: info.room_type.clone().into(), is_history_world_readable: info.is_world_readable, membership: info.state.map(|state| state.into()), - join_rule: info.join_rule.as_ref().map(Into::into), + join_rule: info.join_rule.clone().map(Into::into), is_direct: info.is_direct, heroes: info .heroes @@ -116,8 +116,8 @@ pub struct RoomPreviewInfo { pub heroes: Option>, } -impl From<&JoinRuleSummary> for JoinRule { - fn from(join_rule: &JoinRuleSummary) -> Self { +impl From for JoinRule { + fn from(join_rule: JoinRuleSummary) -> Self { match join_rule { JoinRuleSummary::Invite => JoinRule::Invite, JoinRuleSummary::Knock => JoinRule::Knock, @@ -153,8 +153,8 @@ pub enum RoomType { Custom { value: String }, } -impl From> for RoomType { - fn from(value: Option<&RumaRoomType>) -> Self { +impl From> for RoomType { + fn from(value: Option) -> Self { match value { Some(RumaRoomType::Space) => RoomType::Space, Some(RumaRoomType::_Custom(_)) => RoomType::Custom { diff --git a/bindings/matrix-sdk-ffi/src/spaces.rs b/bindings/matrix-sdk-ffi/src/spaces.rs index b6b44aafa8c..d6dfff24209 100644 --- a/bindings/matrix-sdk-ffi/src/spaces.rs +++ b/bindings/matrix-sdk-ffi/src/spaces.rs @@ -12,10 +12,17 @@ // See the License for the specific language governing permissions and // limitations under the License. -use matrix_sdk_ui::spaces::SpaceService as UISpaceService; +use matrix_sdk_ui::spaces::{ + SpaceService as UISpaceService, SpaceServiceRoom as UISpaceServiceRoom, +}; use ruma::RoomId; -use crate::error::ClientError; +use crate::{ + client::JoinRule, + error::ClientError, + room::{Membership, RoomHero}, + room_preview::RoomType, +}; #[derive(uniffi::Object)] pub struct SpaceService { @@ -32,17 +39,53 @@ impl SpaceService { #[matrix_sdk_ffi_macros::export] impl SpaceService { /// Get the list of joined spaces. - pub fn joined_spaces(&self) -> Vec { - self.inner.joined_spaces().into_iter().map(|id| id.to_string()).collect() + pub fn joined_spaces(&self) -> Vec { + self.inner.joined_spaces().into_iter().map(Into::into).collect() } /// Get the top-level children for a given space. pub async fn top_level_children_for( &self, space_id: String, - ) -> Result, ClientError> { + ) -> Result, ClientError> { let space_id = RoomId::parse(space_id)?; let children = self.inner.top_level_children_for(space_id).await?; - Ok(children.into_iter().map(|id| id.to_string()).collect()) + Ok(children.into_iter().map(Into::into).collect()) + } +} + +#[derive(uniffi::Record)] +pub struct SpaceServiceRoom { + pub room_id: String, + pub canonical_alias: Option, + pub name: Option, + pub topic: Option, + pub avatar_url: Option, + pub room_type: RoomType, + pub num_joined_members: u64, + pub join_rule: Option, + pub world_readable: Option, + pub guest_can_join: bool, + + pub state: Option, + pub heroes: Option>, +} + +impl From for SpaceServiceRoom { + fn from(room: UISpaceServiceRoom) -> Self { + Self { + room_id: room.room_id.into(), + canonical_alias: room.canonical_alias.map(|alias| alias.into()), + name: room.name, + topic: room.topic, + avatar_url: room.avatar_url.map(|url| url.into()), + room_type: room.room_type.into(), + num_joined_members: room.num_joined_members, + join_rule: room.join_rule.map(Into::into), + world_readable: room.world_readable, + guest_can_join: room.guest_can_join, + state: room.state.map(Into::into), + heroes: room.heroes.map(|heroes| heroes.into_iter().map(Into::into).collect()), + } } } diff --git a/crates/matrix-sdk-ui/src/spaces/mod.rs b/crates/matrix-sdk-ui/src/spaces/mod.rs index d207eb7ae40..fb217c8c4f3 100644 --- a/crates/matrix-sdk-ui/src/spaces/mod.rs +++ b/crates/matrix-sdk-ui/src/spaces/mod.rs @@ -20,6 +20,10 @@ use matrix_sdk::{Client, Error}; use ruma::OwnedRoomId; use ruma::api::client::space::get_hierarchy; +pub use crate::spaces::room::SpaceServiceRoom; + +pub mod room; + pub struct SpaceService { client: Client, } @@ -29,25 +33,29 @@ impl SpaceService { Self { client } } - pub fn joined_spaces(&self) -> Vec { + pub fn joined_spaces(&self) -> Vec { self.client .joined_rooms() .into_iter() - .filter_map(|room| if room.is_space() { Some(room.room_id().to_owned()) } else { None }) - .collect() + .filter_map(|room| if room.is_space() { Some(room) } else { None }) + .map(|room| SpaceServiceRoom::new_from_known(room)) + .collect::>() } pub async fn top_level_children_for( &self, space_id: OwnedRoomId, - ) -> Result, Error> { + ) -> Result, Error> { let request = get_hierarchy::v1::Request::new(space_id.clone()); let result = self.client.send(request).await?; - println!("Top level children for space {}: {:?}", space_id, result.rooms); - - Ok(vec![]) + Ok(result + .rooms + .iter() + .map(|room| (&room.summary, self.client.get_room(&room.summary.room_id))) + .map(|(summary, room)| SpaceServiceRoom::new_from_summary(summary, room)) + .collect::>()) } } @@ -121,7 +129,7 @@ mod tests { // All joined assert_eq!( - space_service.joined_spaces(), + space_service.joined_spaces().iter().map(|s| s.room_id.to_owned()).collect::>(), vec![child_space_id_1, child_space_id_2, parent_space_id] ); diff --git a/crates/matrix-sdk-ui/src/spaces/room.rs b/crates/matrix-sdk-ui/src/spaces/room.rs new file mode 100644 index 00000000000..2df6b61614e --- /dev/null +++ b/crates/matrix-sdk-ui/src/spaces/room.rs @@ -0,0 +1,76 @@ +// Copyright 2025 The Matrix.org Foundation C.I.C. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for that specific language governing permissions and +// limitations under the License. + +use matrix_sdk::{Room, RoomHero, RoomState}; +use ruma::events::room::guest_access::GuestAccess; +use ruma::events::room::history_visibility::HistoryVisibility; +use ruma::room::{JoinRuleSummary, RoomSummary, RoomType}; +use ruma::{OwnedMxcUri, OwnedRoomAliasId, OwnedRoomId}; + +#[derive(Debug)] +pub struct SpaceServiceRoom { + pub room_id: OwnedRoomId, + pub canonical_alias: Option, + pub name: Option, + pub topic: Option, + pub avatar_url: Option, + pub room_type: Option, + pub num_joined_members: u64, + pub join_rule: Option, + pub world_readable: Option, + pub guest_can_join: bool, + + pub state: Option, + pub heroes: Option>, +} + +impl SpaceServiceRoom { + pub fn new_from_summary(summary: &RoomSummary, known_room: Option) -> Self { + Self { + room_id: summary.room_id.clone(), + canonical_alias: summary.canonical_alias.clone(), + name: summary.name.clone(), + topic: summary.topic.clone(), + avatar_url: summary.avatar_url.clone(), + room_type: summary.room_type.clone(), + num_joined_members: summary.num_joined_members.into(), + join_rule: Some(summary.join_rule.clone()), + world_readable: Some(summary.world_readable), + guest_can_join: summary.guest_can_join, + state: known_room.as_ref().map(|r| r.state()), + heroes: known_room.map(|r| r.heroes()), + } + } + + pub fn new_from_known(known_room: Room) -> Self { + let room_info = known_room.clone_info(); + + Self { + room_id: room_info.room_id().to_owned(), + canonical_alias: room_info.canonical_alias().map(ToOwned::to_owned), + name: room_info.name().map(ToOwned::to_owned), + topic: room_info.topic().map(ToOwned::to_owned), + avatar_url: room_info.avatar_url().map(ToOwned::to_owned), + room_type: room_info.room_type().cloned(), + num_joined_members: known_room.joined_members_count(), + join_rule: room_info.join_rule().cloned().map(Into::into), + world_readable: room_info + .history_visibility() + .map(|vis| *vis == HistoryVisibility::WorldReadable), + guest_can_join: known_room.guest_access() == GuestAccess::CanJoin, + state: Some(known_room.state()), + heroes: Some(room_info.heroes().to_vec()), + } + } +} From 3cb58744156216405e886a7d0faa235609d3f3d3 Mon Sep 17 00:00:00 2001 From: Stefan Ceriu Date: Wed, 6 Aug 2025 11:41:17 +0300 Subject: [PATCH 03/35] feat(spaces): introduce a `SpaceRoomList` that allows pagination and provides reactive interfaces to its rooms and pagination state --- bindings/matrix-sdk-ffi/src/spaces.rs | 98 +++++++- crates/matrix-sdk-ui/src/spaces/mod.rs | 35 +-- crates/matrix-sdk-ui/src/spaces/room.rs | 11 +- crates/matrix-sdk-ui/src/spaces/room_list.rs | 223 ++++++++++++++++++ crates/matrix-sdk/src/test_utils/mocks/mod.rs | 40 ++++ 5 files changed, 372 insertions(+), 35 deletions(-) create mode 100644 crates/matrix-sdk-ui/src/spaces/room_list.rs diff --git a/bindings/matrix-sdk-ffi/src/spaces.rs b/bindings/matrix-sdk-ffi/src/spaces.rs index d6dfff24209..661077a6ccc 100644 --- a/bindings/matrix-sdk-ffi/src/spaces.rs +++ b/bindings/matrix-sdk-ffi/src/spaces.rs @@ -12,8 +12,14 @@ // See the License for the specific language governing permissions and // limitations under the License. +use std::{fmt::Debug, sync::Arc}; + +use futures_util::{pin_mut, StreamExt}; +use matrix_sdk_common::{SendOutsideWasm, SyncOutsideWasm}; use matrix_sdk_ui::spaces::{ + room_list::SpaceServiceRoomListPaginationState as UISpaceServiceRoomListPaginationState, SpaceService as UISpaceService, SpaceServiceRoom as UISpaceServiceRoom, + SpaceServiceRoomList as UISpaceServiceRoomList, }; use ruma::RoomId; @@ -22,6 +28,8 @@ use crate::{ error::ClientError, room::{Membership, RoomHero}, room_preview::RoomType, + runtime::get_runtime_handle, + TaskHandle, }; #[derive(uniffi::Object)] @@ -30,7 +38,6 @@ pub struct SpaceService { } impl SpaceService { - /// Create a new instance of `SpaceService`. pub fn new(inner: UISpaceService) -> Self { Self { inner } } @@ -38,20 +45,97 @@ impl SpaceService { #[matrix_sdk_ffi_macros::export] impl SpaceService { - /// Get the list of joined spaces. pub fn joined_spaces(&self) -> Vec { self.inner.joined_spaces().into_iter().map(Into::into).collect() } - /// Get the top-level children for a given space. - pub async fn top_level_children_for( + pub fn top_level_children_for( &self, space_id: String, - ) -> Result, ClientError> { + ) -> Result { let space_id = RoomId::parse(space_id)?; - let children = self.inner.top_level_children_for(space_id).await?; - Ok(children.into_iter().map(Into::into).collect()) + Ok(SpaceServiceRoomList::new(self.inner.space_room_list(space_id))) + } +} + +#[derive(uniffi::Object)] +pub struct SpaceServiceRoomList { + inner: UISpaceServiceRoomList, +} + +impl SpaceServiceRoomList { + pub fn new(inner: UISpaceServiceRoomList) -> Self { + Self { inner } + } +} + +#[matrix_sdk_ffi_macros::export] +impl SpaceServiceRoomList { + pub fn pagination_state(&self) -> SpaceServiceRoomListPaginationState { + self.inner.pagination_state().into() + } + + pub fn subscribe_to_pagination_state_updates( + &self, + listener: Box, + ) { + let pagination_state = self.inner.subscribe_to_pagination_state_updates(); + + Arc::new(TaskHandle::new(get_runtime_handle().spawn(async move { + pin_mut!(pagination_state); + + while let Some(state) = pagination_state.next().await { + listener.on_update(state.into()); + } + }))); } + + pub fn rooms(&self) -> Vec { + self.inner.rooms().into_iter().map(Into::into).collect() + } + + pub fn subscribe_to_room_update(&self, listener: Box) { + let entries_stream = self.inner.subscribe_to_room_updates(); + + Arc::new(TaskHandle::new(get_runtime_handle().spawn(async move { + pin_mut!(entries_stream); + + while let Some(rooms) = entries_stream.next().await { + listener.on_update(rooms.into_iter().map(Into::into).collect()); + } + }))); + } +} + +#[derive(uniffi::Enum)] +pub enum SpaceServiceRoomListPaginationState { + Idle { end_reached: bool }, + Loading, +} + +impl From for SpaceServiceRoomListPaginationState { + fn from(state: UISpaceServiceRoomListPaginationState) -> Self { + match state { + UISpaceServiceRoomListPaginationState::Idle { end_reached } => { + SpaceServiceRoomListPaginationState::Idle { end_reached } + } + UISpaceServiceRoomListPaginationState::Loading => { + SpaceServiceRoomListPaginationState::Loading + } + } + } +} + +#[matrix_sdk_ffi_macros::export(callback_interface)] +pub trait SpaceServiceRoomListPaginationStateListener: + SendOutsideWasm + SyncOutsideWasm + Debug +{ + fn on_update(&self, pagination_state: SpaceServiceRoomListPaginationState); +} + +#[matrix_sdk_ffi_macros::export(callback_interface)] +pub trait SpaceServiceRoomListEntriesListener: SendOutsideWasm + SyncOutsideWasm + Debug { + fn on_update(&self, rooms: Vec); } #[derive(uniffi::Record)] diff --git a/crates/matrix-sdk-ui/src/spaces/mod.rs b/crates/matrix-sdk-ui/src/spaces/mod.rs index fb217c8c4f3..59abce07416 100644 --- a/crates/matrix-sdk-ui/src/spaces/mod.rs +++ b/crates/matrix-sdk-ui/src/spaces/mod.rs @@ -14,15 +14,15 @@ //! High level interfaces for working with Spaces //! -//! See [`SpaceDiscoveryService`] for details. +//! See [`SpaceService`] for details. -use matrix_sdk::{Client, Error}; +use matrix_sdk::Client; use ruma::OwnedRoomId; -use ruma::api::client::space::get_hierarchy; -pub use crate::spaces::room::SpaceServiceRoom; +pub use crate::spaces::{room::SpaceServiceRoom, room_list::SpaceServiceRoomList}; pub mod room; +pub mod room_list; pub struct SpaceService { client: Client, @@ -38,36 +38,25 @@ impl SpaceService { .joined_rooms() .into_iter() .filter_map(|room| if room.is_space() { Some(room) } else { None }) - .map(|room| SpaceServiceRoom::new_from_known(room)) + .map(SpaceServiceRoom::new_from_known) .collect::>() } - pub async fn top_level_children_for( - &self, - space_id: OwnedRoomId, - ) -> Result, Error> { - let request = get_hierarchy::v1::Request::new(space_id.clone()); - - let result = self.client.send(request).await?; - - Ok(result - .rooms - .iter() - .map(|room| (&room.summary, self.client.get_room(&room.summary.room_id))) - .map(|(summary, room)| SpaceServiceRoom::new_from_summary(summary, room)) - .collect::>()) + pub fn space_room_list(&self, space_id: OwnedRoomId) -> SpaceServiceRoomList { + SpaceServiceRoomList::new(self.client.clone(), space_id) } } #[cfg(test)] mod tests { - use super::*; use assert_matches2::assert_let; use matrix_sdk::{room::ParentSpace, test_utils::mocks::MatrixMockServer}; use matrix_sdk_test::{JoinedRoomBuilder, async_test, event_factory::EventFactory}; use ruma::{RoomVersionId, room_id}; use tokio_stream::StreamExt; + use super::*; + #[async_test] async fn test_spaces_hierarchy() { let server = MatrixMockServer::new().await; @@ -80,9 +69,9 @@ mod tests { // Given one parent space with 2 children spaces - let parent_space_id = room_id!("!3:example.org"); - let child_space_id_1 = room_id!("!1:example.org"); - let child_space_id_2 = room_id!("!2:example.org"); + let parent_space_id = room_id!("!parent_space:example.org"); + let child_space_id_1 = room_id!("!child_space_1:example.org"); + let child_space_id_2 = room_id!("!child_space_2:example.org"); server .sync_room( diff --git a/crates/matrix-sdk-ui/src/spaces/room.rs b/crates/matrix-sdk-ui/src/spaces/room.rs index 2df6b61614e..8bc778ca5c9 100644 --- a/crates/matrix-sdk-ui/src/spaces/room.rs +++ b/crates/matrix-sdk-ui/src/spaces/room.rs @@ -13,12 +13,13 @@ // limitations under the License. use matrix_sdk::{Room, RoomHero, RoomState}; -use ruma::events::room::guest_access::GuestAccess; -use ruma::events::room::history_visibility::HistoryVisibility; -use ruma::room::{JoinRuleSummary, RoomSummary, RoomType}; -use ruma::{OwnedMxcUri, OwnedRoomAliasId, OwnedRoomId}; +use ruma::{ + OwnedMxcUri, OwnedRoomAliasId, OwnedRoomId, + events::room::{guest_access::GuestAccess, history_visibility::HistoryVisibility}, + room::{JoinRuleSummary, RoomSummary, RoomType}, +}; -#[derive(Debug)] +#[derive(Debug, Clone, PartialEq)] pub struct SpaceServiceRoom { pub room_id: OwnedRoomId, pub canonical_alias: Option, diff --git a/crates/matrix-sdk-ui/src/spaces/room_list.rs b/crates/matrix-sdk-ui/src/spaces/room_list.rs new file mode 100644 index 00000000000..0fbdc9305ba --- /dev/null +++ b/crates/matrix-sdk-ui/src/spaces/room_list.rs @@ -0,0 +1,223 @@ +// Copyright 2025 The Matrix.org Foundation C.I.C. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for that specific language governing permissions and +// limitations under the License. + +use std::sync::Mutex; + +use eyeball::{SharedObservable, Subscriber}; +use matrix_sdk::{Client, Error, paginators::PaginationToken}; +use ruma::{OwnedRoomId, api::client::space::get_hierarchy}; + +use crate::spaces::SpaceServiceRoom; + +#[derive(Clone, Debug, Eq, PartialEq)] +pub enum SpaceServiceRoomListPaginationState { + Idle { end_reached: bool }, + Loading, +} + +pub struct SpaceServiceRoomList { + client: Client, + + parent_space_id: OwnedRoomId, + + token: Mutex, + + pagination_state: SharedObservable, + + rooms: SharedObservable>, +} + +impl SpaceServiceRoomList { + pub fn new(client: Client, parent_space_id: OwnedRoomId) -> Self { + Self { + client, + parent_space_id, + token: Mutex::new(None.into()), + pagination_state: SharedObservable::new(SpaceServiceRoomListPaginationState::Idle { + end_reached: false, + }), + rooms: SharedObservable::new(Vec::new()), + } + } + + pub fn pagination_state(&self) -> SpaceServiceRoomListPaginationState { + self.pagination_state.get() + } + + pub fn subscribe_to_pagination_state_updates( + &self, + ) -> Subscriber { + self.pagination_state.subscribe() + } + + pub fn rooms(&self) -> Vec { + self.rooms.get() + } + + pub fn subscribe_to_room_updates(&self) -> Subscriber> { + self.rooms.subscribe() + } + + pub async fn paginate(&self) -> Result<(), Error> { + match *self.pagination_state.read() { + SpaceServiceRoomListPaginationState::Idle { end_reached } if end_reached => { + return Ok(()); + } + SpaceServiceRoomListPaginationState::Loading => { + return Ok(()); + } + _ => {} + } + + self.pagination_state.set(SpaceServiceRoomListPaginationState::Loading); + + let mut request = get_hierarchy::v1::Request::new(self.parent_space_id.clone()); + + if let PaginationToken::HasMore(ref token) = *self.token.lock().unwrap() { + request.from = Some(token.clone()); + } + + match self.client.send(request).await { + Ok(result) => { + let mut token = self.token.lock().unwrap(); + *token = match &result.next_batch { + Some(val) => PaginationToken::HasMore(val.clone()), + None => PaginationToken::HitEnd, + }; + + let mut current_rooms = Vec::new(); + + (self.rooms.read()).iter().for_each(|room| { + current_rooms.push(room.clone()); + }); + + current_rooms.extend( + result + .rooms + .iter() + .map(|room| (&room.summary, self.client.get_room(&room.summary.room_id))) + .map(|(summary, room)| SpaceServiceRoom::new_from_summary(summary, room)) + .collect::>(), + ); + + self.rooms.set(current_rooms.clone()); + + self.pagination_state.set(SpaceServiceRoomListPaginationState::Idle { + end_reached: result.next_batch.is_none(), + }); + + Ok(()) + } + Err(err) => { + self.pagination_state + .set(SpaceServiceRoomListPaginationState::Idle { end_reached: false }); + Err(err.into()) + } + } + } +} + +#[cfg(test)] +mod tests { + use assert_matches2::assert_matches; + use futures_util::pin_mut; + use matrix_sdk::test_utils::mocks::MatrixMockServer; + use matrix_sdk_test::async_test; + use ruma::{ + room::{JoinRuleSummary, RoomSummary}, + room_id, uint, + }; + use stream_assert::{assert_next_eq, assert_next_matches, assert_pending}; + + use crate::spaces::{ + SpaceService, SpaceServiceRoom, room_list::SpaceServiceRoomListPaginationState, + }; + + #[async_test] + async fn test_room_list_pagination() { + let server = MatrixMockServer::new().await; + let client = server.client_builder().build().await; + let space_service = SpaceService::new(client.clone()); + + server.mock_room_state_encryption().plain().mount().await; + + let parent_space_id = room_id!("!parent_space:example.org"); + + let room_list = space_service.space_room_list(parent_space_id.to_owned()); + + // Start off idle + assert_matches!( + room_list.pagination_state(), + SpaceServiceRoomListPaginationState::Idle { end_reached: false } + ); + + // without any rooms + assert_eq!(room_list.rooms(), vec![]); + + // and with pending subscribers + + let pagination_state_subscriber = room_list.subscribe_to_pagination_state_updates(); + pin_mut!(pagination_state_subscriber); + assert_pending!(pagination_state_subscriber); + + let rooms_subscriber = room_list.subscribe_to_room_updates(); + pin_mut!(rooms_subscriber); + assert_pending!(rooms_subscriber); + + let child_space_id_1 = room_id!("!1:example.org"); + let child_space_id_2 = room_id!("!2:example.org"); + + // Paginating the room list + server + .mock_get_hierarchy() + .ok_with_room_ids(vec![child_space_id_1, child_space_id_2]) + .mount() + .await; + + room_list.paginate().await.unwrap(); + + // informs that the pagination reached the end + assert_next_matches!( + pagination_state_subscriber, + SpaceServiceRoomListPaginationState::Idle { end_reached: true } + ); + + // yields results + assert_next_eq!( + rooms_subscriber, + vec![ + SpaceServiceRoom::new_from_summary( + &RoomSummary::new( + child_space_id_1.to_owned(), + JoinRuleSummary::Public, + false, + uint!(1), + false, + ), + None, + ), + SpaceServiceRoom::new_from_summary( + &RoomSummary::new( + child_space_id_2.to_owned(), + JoinRuleSummary::Public, + false, + uint!(1), + false, + ), + None, + ), + ] + ); + } +} diff --git a/crates/matrix-sdk/src/test_utils/mocks/mod.rs b/crates/matrix-sdk/src/test_utils/mocks/mod.rs index e8e3cf82104..1c7c89fa1f0 100644 --- a/crates/matrix-sdk/src/test_utils/mocks/mod.rs +++ b/crates/matrix-sdk/src/test_utils/mocks/mod.rs @@ -1448,6 +1448,13 @@ impl MatrixMockServer { self.mock_endpoint(mock, GetThreadSubscriptionsEndpoint::default()) .expect_default_access_token() } + + /// Create a prebuilt mock for the endpoint used to retrieve a space tree + pub fn mock_get_hierarchy(&self) -> MockEndpoint<'_, GetHierarchyEndpoint> { + let mock = + Mock::given(method("GET")).and(path_regex(r"^/_matrix/client/v1/rooms/.*/hierarchy")); + self.mock_endpoint(mock, GetHierarchyEndpoint).expect_default_access_token() + } } /// A specification for a push rule ID. @@ -4210,3 +4217,36 @@ impl<'a> MockEndpoint<'a, GetThreadSubscriptionsEndpoint> { self.respond_with(ResponseTemplate::new(200).set_body_json(response_body)) } } + +/// A prebuilt mock for `GET /client/*/rooms/{roomId}/hierarchy` +#[derive(Default)] +pub struct GetHierarchyEndpoint; + +impl<'a> MockEndpoint<'a, GetHierarchyEndpoint> { + /// Returns a successful response containing the given room IDs. + pub fn ok_with_room_ids(self, room_ids: Vec<&RoomId>) -> MatrixMock<'a> { + let rooms = room_ids + .iter() + .map(|id| { + json!({ + "room_id": id, + "num_joined_members": 1, + "world_readable": false, + "guest_can_join": false, + "children_state": [] + }) + }) + .collect::>(); + + self.respond_with(ResponseTemplate::new(200).set_body_json(json!({ + "rooms": rooms, + }))) + } + + /// Returns a successful response with an empty list of rooms. + pub fn ok(self) -> MatrixMock<'a> { + self.respond_with(ResponseTemplate::new(200).set_body_json(json!({ + "rooms": [] + }))) + } +} From c6678331d736d7231f2633e63424504c5404070c Mon Sep 17 00:00:00 2001 From: Stefan Ceriu Date: Thu, 7 Aug 2025 13:57:52 +0300 Subject: [PATCH 04/35] feat(spaces): add a reactive version of the `joined_spaces` method --- bindings/matrix-sdk-ffi/src/client.rs | 6 +- crates/matrix-sdk-ui/src/spaces/mod.rs | 158 +++++++++++++++++++++++-- 2 files changed, 155 insertions(+), 9 deletions(-) diff --git a/bindings/matrix-sdk-ffi/src/client.rs b/bindings/matrix-sdk-ffi/src/client.rs index 43bfc9dfcaa..9411e114426 100644 --- a/bindings/matrix-sdk-ffi/src/client.rs +++ b/bindings/matrix-sdk-ffi/src/client.rs @@ -1259,7 +1259,11 @@ impl Client { SyncServiceBuilder::new((*self.inner).clone(), self.utd_hook_manager.get().cloned()) } - pub fn spaces_service(&self) -> Arc { + pub async fn spaces_service(&self) -> Arc { + // This method doesn't need to be async but if its not the FFI layer panics + // with "there is no no reactor running, must be called from the context + // of a Tokio 1.x runtime" error because the undelying constructor spawns + // an async task. let inner = UISpaceService::new((*self.inner).clone()); Arc::new(SpaceService::new(inner)) } diff --git a/crates/matrix-sdk-ui/src/spaces/mod.rs b/crates/matrix-sdk-ui/src/spaces/mod.rs index 59abce07416..b91956208fa 100644 --- a/crates/matrix-sdk-ui/src/spaces/mod.rs +++ b/crates/matrix-sdk-ui/src/spaces/mod.rs @@ -16,8 +16,12 @@ //! //! See [`SpaceService`] for details. +use eyeball::{SharedObservable, Subscriber}; +use futures_util::pin_mut; use matrix_sdk::Client; use ruma::OwnedRoomId; +use tokio::task::JoinHandle; +use tracing::error; pub use crate::spaces::{room::SpaceServiceRoom, room_list::SpaceServiceRoomList}; @@ -26,37 +30,87 @@ pub mod room_list; pub struct SpaceService { client: Client, + + joined_spaces: SharedObservable>, + + room_update_handle: JoinHandle<()>, +} + +impl Drop for SpaceService { + fn drop(&mut self) { + self.room_update_handle.abort(); + } } impl SpaceService { pub fn new(client: Client) -> Self { - Self { client } + let joined_spaces = SharedObservable::new(Vec::new()); + + joined_spaces.set(Self::joined_spaces_for(&client)); + + let client_clone = client.clone(); + let joined_spaces_clone = joined_spaces.clone(); + let all_room_updates_receiver = client.subscribe_to_all_room_updates(); + + let handle = tokio::spawn(async move { + pin_mut!(all_room_updates_receiver); + + loop { + match all_room_updates_receiver.recv().await { + Ok(_) => { + let new_spaces = Self::joined_spaces_for(&client_clone); + if new_spaces != joined_spaces_clone.get() { + joined_spaces_clone.set(new_spaces); + } + } + Err(err) => { + error!("error when listening to room updates: {err}"); + } + } + } + }); + + Self { client, joined_spaces, room_update_handle: handle } + } + + pub fn subscribe_to_joined_spaces(&self) -> Subscriber> { + self.joined_spaces.subscribe() } pub fn joined_spaces(&self) -> Vec { - self.client + self.joined_spaces.get() + } + + pub fn space_room_list(&self, space_id: OwnedRoomId) -> SpaceServiceRoomList { + SpaceServiceRoomList::new(self.client.clone(), space_id) + } + + fn joined_spaces_for(client: &Client) -> Vec { + client .joined_rooms() .into_iter() .filter_map(|room| if room.is_space() { Some(room) } else { None }) .map(SpaceServiceRoom::new_from_known) .collect::>() } - - pub fn space_room_list(&self, space_id: OwnedRoomId) -> SpaceServiceRoomList { - SpaceServiceRoomList::new(self.client.clone(), space_id) - } } #[cfg(test)] mod tests { use assert_matches2::assert_let; use matrix_sdk::{room::ParentSpace, test_utils::mocks::MatrixMockServer}; - use matrix_sdk_test::{JoinedRoomBuilder, async_test, event_factory::EventFactory}; + use matrix_sdk_test::{ + JoinedRoomBuilder, LeftRoomBuilder, async_test, event_factory::EventFactory, + }; use ruma::{RoomVersionId, room_id}; use tokio_stream::StreamExt; use super::*; + use futures_util::pin_mut; + + use stream_assert::{assert_next_eq, assert_pending}; + #[async_test] async fn test_spaces_hierarchy() { let server = MatrixMockServer::new().await; @@ -125,7 +179,7 @@ mod tests { let parent_space = client.get_room(parent_space_id).unwrap(); assert!(parent_space.is_space()); - // Then the parent space and the two child spaces are linked + // And the parent space and the two child spaces are linked let spaces: Vec = client .get_room(child_space_id_1) @@ -153,4 +207,92 @@ mod tests { assert_let!(ParentSpace::Reciprocal(parent) = spaces.last().unwrap()); assert_eq!(parent.room_id(), parent_space.room_id()); } + + #[async_test] + async fn test_joined_spaces_updates() { + let server = MatrixMockServer::new().await; + let client = server.client_builder().build().await; + let user_id = client.user_id().unwrap(); + let factory = EventFactory::new(); + + server.mock_room_state_encryption().plain().mount().await; + + let first_space_id = room_id!("!first_space:example.org"); + let second_space_id = room_id!("!second_space:example.org"); + + // Join the first space + server + .sync_room( + &client, + JoinedRoomBuilder::new(first_space_id) + .add_state_event(factory.create(user_id, RoomVersionId::V1).with_space_type()), + ) + .await; + + // Build the `SpaceService` and expect the room to show up with no updates + // pending + + let space_service = SpaceService::new(client.clone()); + + let joined_spaces_subscriber = space_service.subscribe_to_joined_spaces(); + pin_mut!(joined_spaces_subscriber); + assert_pending!(joined_spaces_subscriber); + + assert_eq!( + space_service.joined_spaces(), + vec![SpaceServiceRoom::new_from_known(client.get_room(first_space_id).unwrap())] + ); + + // Join the second space + + server + .sync_room( + &client, + JoinedRoomBuilder::new(second_space_id) + .add_state_event(factory.create(user_id, RoomVersionId::V1).with_space_type()), + ) + .await; + + // And expect the list to update + assert_eq!( + space_service.joined_spaces(), + vec![ + SpaceServiceRoom::new_from_known(client.get_room(first_space_id).unwrap()), + SpaceServiceRoom::new_from_known(client.get_room(second_space_id).unwrap()) + ] + ); + + // The subscriber yields new results when a space is joined + assert_next_eq!( + joined_spaces_subscriber, + vec![ + SpaceServiceRoom::new_from_known(client.get_room(first_space_id).unwrap()), + SpaceServiceRoom::new_from_known(client.get_room(second_space_id).unwrap()) + ] + ); + + server.sync_room(&client, LeftRoomBuilder::new(second_space_id)).await; + + // and when one is left + assert_next_eq!( + joined_spaces_subscriber, + vec![SpaceServiceRoom::new_from_known(client.get_room(first_space_id).unwrap())] + ); + + // but it doesn't when a non-space room gets joined + server + .sync_room( + &client, + JoinedRoomBuilder::new(room_id!("!room:example.org")) + .add_state_event(factory.create(user_id, RoomVersionId::V1)), + ) + .await; + + // and the subscriber doesn't yield any updates + assert_pending!(joined_spaces_subscriber); + assert_eq!( + space_service.joined_spaces(), + vec![SpaceServiceRoom::new_from_known(client.get_room(first_space_id).unwrap())] + ); + } } From 245906111e63ae3a90139bdc220c4fa6179fa0b9 Mon Sep 17 00:00:00 2001 From: Stefan Ceriu Date: Thu, 7 Aug 2025 18:16:49 +0300 Subject: [PATCH 05/35] change(spaces): return only top level joined rooms from `SpaceService::joined_spaces` and its reactive counterpart --- bindings/matrix-sdk-ffi/src/client.rs | 6 +-- crates/matrix-sdk-ui/src/spaces/mod.rs | 39 ++++++++++++++------ crates/matrix-sdk-ui/src/spaces/room_list.rs | 2 +- 3 files changed, 30 insertions(+), 17 deletions(-) diff --git a/bindings/matrix-sdk-ffi/src/client.rs b/bindings/matrix-sdk-ffi/src/client.rs index 9411e114426..f4c27d239fc 100644 --- a/bindings/matrix-sdk-ffi/src/client.rs +++ b/bindings/matrix-sdk-ffi/src/client.rs @@ -1260,11 +1260,7 @@ impl Client { } pub async fn spaces_service(&self) -> Arc { - // This method doesn't need to be async but if its not the FFI layer panics - // with "there is no no reactor running, must be called from the context - // of a Tokio 1.x runtime" error because the undelying constructor spawns - // an async task. - let inner = UISpaceService::new((*self.inner).clone()); + let inner = UISpaceService::new((*self.inner).clone()).await; Arc::new(SpaceService::new(inner)) } diff --git a/crates/matrix-sdk-ui/src/spaces/mod.rs b/crates/matrix-sdk-ui/src/spaces/mod.rs index b91956208fa..deb76d05b51 100644 --- a/crates/matrix-sdk-ui/src/spaces/mod.rs +++ b/crates/matrix-sdk-ui/src/spaces/mod.rs @@ -21,6 +21,7 @@ use futures_util::pin_mut; use matrix_sdk::Client; use ruma::OwnedRoomId; use tokio::task::JoinHandle; +use tokio_stream::StreamExt; use tracing::error; pub use crate::spaces::{room::SpaceServiceRoom, room_list::SpaceServiceRoomList}; @@ -43,10 +44,10 @@ impl Drop for SpaceService { } impl SpaceService { - pub fn new(client: Client) -> Self { + pub async fn new(client: Client) -> Self { let joined_spaces = SharedObservable::new(Vec::new()); - joined_spaces.set(Self::joined_spaces_for(&client)); + joined_spaces.set(Self::joined_spaces_for(&client).await); let client_clone = client.clone(); let joined_spaces_clone = joined_spaces.clone(); @@ -58,7 +59,7 @@ impl SpaceService { loop { match all_room_updates_receiver.recv().await { Ok(_) => { - let new_spaces = Self::joined_spaces_for(&client_clone); + let new_spaces = Self::joined_spaces_for(&client_clone).await; if new_spaces != joined_spaces_clone.get() { joined_spaces_clone.set(new_spaces); } @@ -85,13 +86,29 @@ impl SpaceService { SpaceServiceRoomList::new(self.client.clone(), space_id) } - fn joined_spaces_for(client: &Client) -> Vec { - client + async fn joined_spaces_for(client: &Client) -> Vec { + let joined_spaces = client .joined_rooms() .into_iter() .filter_map(|room| if room.is_space() { Some(room) } else { None }) - .map(SpaceServiceRoom::new_from_known) - .collect::>() + .collect::>(); + + let top_level_spaces: Vec = tokio_stream::iter(joined_spaces) + .then(|room| async move { + let ok = if let Ok(parents) = room.parent_spaces().await { + pin_mut!(parents); + parents.any(|p| p.is_ok()).await == false + } else { + false + }; + (room, ok) + }) + .filter(|(_, ok)| *ok) + .map(|(x, _)| SpaceServiceRoom::new_from_known(x)) + .collect() + .await; + + top_level_spaces } } @@ -116,7 +133,7 @@ mod tests { let server = MatrixMockServer::new().await; let client = server.client_builder().build().await; let user_id = client.user_id().unwrap(); - let space_service = SpaceService::new(client.clone()); + let space_service = SpaceService::new(client.clone()).await; let factory = EventFactory::new(); server.mock_room_state_encryption().plain().mount().await; @@ -170,10 +187,10 @@ mod tests { ) .await; - // All joined + // Only the parent space is returned assert_eq!( space_service.joined_spaces().iter().map(|s| s.room_id.to_owned()).collect::>(), - vec![child_space_id_1, child_space_id_2, parent_space_id] + vec![parent_space_id] ); let parent_space = client.get_room(parent_space_id).unwrap(); @@ -232,7 +249,7 @@ mod tests { // Build the `SpaceService` and expect the room to show up with no updates // pending - let space_service = SpaceService::new(client.clone()); + let space_service = SpaceService::new(client.clone()).await; let joined_spaces_subscriber = space_service.subscribe_to_joined_spaces(); pin_mut!(joined_spaces_subscriber); diff --git a/crates/matrix-sdk-ui/src/spaces/room_list.rs b/crates/matrix-sdk-ui/src/spaces/room_list.rs index 0fbdc9305ba..c6887a9584f 100644 --- a/crates/matrix-sdk-ui/src/spaces/room_list.rs +++ b/crates/matrix-sdk-ui/src/spaces/room_list.rs @@ -148,7 +148,7 @@ mod tests { async fn test_room_list_pagination() { let server = MatrixMockServer::new().await; let client = server.client_builder().build().await; - let space_service = SpaceService::new(client.clone()); + let space_service = SpaceService::new(client.clone()).await; server.mock_room_state_encryption().plain().mount().await; From 3de2d8d7659923ac88833153ef353cf5248ad163 Mon Sep 17 00:00:00 2001 From: Stefan Ceriu Date: Fri, 8 Aug 2025 14:19:44 +0300 Subject: [PATCH 06/35] feat(spaces): have the `SpaceRoomList` publish updates as known room states change i.e. they get joined or left. --- bindings/matrix-sdk-ffi/src/spaces.rs | 7 +- crates/matrix-sdk-base/src/sync.rs | 2 +- crates/matrix-sdk-ui/src/spaces/mod.rs | 9 +- crates/matrix-sdk-ui/src/spaces/room_list.rs | 107 ++++++++++++++++++- 4 files changed, 113 insertions(+), 12 deletions(-) diff --git a/bindings/matrix-sdk-ffi/src/spaces.rs b/bindings/matrix-sdk-ffi/src/spaces.rs index 661077a6ccc..e6e1315fdad 100644 --- a/bindings/matrix-sdk-ffi/src/spaces.rs +++ b/bindings/matrix-sdk-ffi/src/spaces.rs @@ -49,7 +49,12 @@ impl SpaceService { self.inner.joined_spaces().into_iter().map(Into::into).collect() } - pub fn top_level_children_for( + #[allow(clippy::unused_async)] + // This method doesn't need to be async but if its not the FFI layer panics + // with "there is no no reactor running, must be called from the context + // of a Tokio 1.x runtime" error because the underlying constructor spawns + // an async task. + pub async fn space_room_list( &self, space_id: String, ) -> Result { diff --git a/crates/matrix-sdk-base/src/sync.rs b/crates/matrix-sdk-base/src/sync.rs index ead5e587d27..c1431405351 100644 --- a/crates/matrix-sdk-base/src/sync.rs +++ b/crates/matrix-sdk-base/src/sync.rs @@ -90,7 +90,7 @@ impl RoomUpdates { /// Iterate over all room IDs, from [`RoomUpdates::left`], /// [`RoomUpdates::joined`], [`RoomUpdates::invited`] and /// [`RoomUpdates::knocked`]. - pub(crate) fn iter_all_room_ids(&self) -> impl Iterator { + pub fn iter_all_room_ids(&self) -> impl Iterator { self.left .keys() .chain(self.joined.keys()) diff --git a/crates/matrix-sdk-ui/src/spaces/mod.rs b/crates/matrix-sdk-ui/src/spaces/mod.rs index deb76d05b51..161bbe5f6d3 100644 --- a/crates/matrix-sdk-ui/src/spaces/mod.rs +++ b/crates/matrix-sdk-ui/src/spaces/mod.rs @@ -97,7 +97,7 @@ impl SpaceService { .then(|room| async move { let ok = if let Ok(parents) = room.parent_spaces().await { pin_mut!(parents); - parents.any(|p| p.is_ok()).await == false + !parents.any(|p| p.is_ok()).await } else { false }; @@ -115,19 +115,16 @@ impl SpaceService { #[cfg(test)] mod tests { use assert_matches2::assert_let; + use futures_util::{StreamExt, pin_mut}; use matrix_sdk::{room::ParentSpace, test_utils::mocks::MatrixMockServer}; use matrix_sdk_test::{ JoinedRoomBuilder, LeftRoomBuilder, async_test, event_factory::EventFactory, }; use ruma::{RoomVersionId, room_id}; - use tokio_stream::StreamExt; + use stream_assert::{assert_next_eq, assert_pending}; use super::*; - use futures_util::pin_mut; - - use stream_assert::{assert_next_eq, assert_pending}; - #[async_test] async fn test_spaces_hierarchy() { let server = MatrixMockServer::new().await; diff --git a/crates/matrix-sdk-ui/src/spaces/room_list.rs b/crates/matrix-sdk-ui/src/spaces/room_list.rs index c6887a9584f..559d627427d 100644 --- a/crates/matrix-sdk-ui/src/spaces/room_list.rs +++ b/crates/matrix-sdk-ui/src/spaces/room_list.rs @@ -15,8 +15,11 @@ use std::sync::Mutex; use eyeball::{SharedObservable, Subscriber}; +use futures_util::pin_mut; use matrix_sdk::{Client, Error, paginators::PaginationToken}; use ruma::{OwnedRoomId, api::client::space::get_hierarchy}; +use tokio::task::JoinHandle; +use tracing::error; use crate::spaces::SpaceServiceRoom; @@ -36,10 +39,51 @@ pub struct SpaceServiceRoomList { pagination_state: SharedObservable, rooms: SharedObservable>, + + room_update_handle: JoinHandle<()>, } +impl Drop for SpaceServiceRoomList { + fn drop(&mut self) { + self.room_update_handle.abort(); + } +} impl SpaceServiceRoomList { pub fn new(client: Client, parent_space_id: OwnedRoomId) -> Self { + let rooms = SharedObservable::new(Vec::::new()); + + let client_clone = client.clone(); + let rooms_clone = rooms.clone(); + let all_room_updates_receiver = client.subscribe_to_all_room_updates(); + + let handle = tokio::spawn(async move { + pin_mut!(all_room_updates_receiver); + + loop { + match all_room_updates_receiver.recv().await { + Ok(updates) => { + let mut new_rooms = rooms_clone.get(); + + updates.iter_all_room_ids().for_each(|updated_room_id| { + if let Some(room) = + new_rooms.iter_mut().find(|room| &room.room_id == updated_room_id) + && let Some(update_room) = client_clone.get_room(updated_room_id) + { + *room = SpaceServiceRoom::new_from_known(update_room); + } + }); + + if new_rooms != rooms_clone.get() { + rooms_clone.set(new_rooms); + } + } + Err(err) => { + error!("error when listening to room updates: {err}"); + } + } + } + }); + Self { client, parent_space_id, @@ -47,7 +91,8 @@ impl SpaceServiceRoomList { pagination_state: SharedObservable::new(SpaceServiceRoomListPaginationState::Idle { end_reached: false, }), - rooms: SharedObservable::new(Vec::new()), + rooms, + room_update_handle: handle, } } @@ -132,13 +177,13 @@ impl SpaceServiceRoomList { mod tests { use assert_matches2::assert_matches; use futures_util::pin_mut; - use matrix_sdk::test_utils::mocks::MatrixMockServer; - use matrix_sdk_test::async_test; + use matrix_sdk::{RoomState, test_utils::mocks::MatrixMockServer}; + use matrix_sdk_test::{JoinedRoomBuilder, LeftRoomBuilder, async_test}; use ruma::{ room::{JoinRuleSummary, RoomSummary}, room_id, uint, }; - use stream_assert::{assert_next_eq, assert_next_matches, assert_pending}; + use stream_assert::{assert_next_eq, assert_next_matches, assert_pending, assert_ready}; use crate::spaces::{ SpaceService, SpaceServiceRoom, room_list::SpaceServiceRoomListPaginationState, @@ -220,4 +265,58 @@ mod tests { ] ); } + + #[async_test] + async fn test_room_state_updates() { + let server = MatrixMockServer::new().await; + let client = server.client_builder().build().await; + let space_service = SpaceService::new(client.clone()).await; + + let parent_space_id = room_id!("!parent_space:example.org"); + let child_room_id_1 = room_id!("!1:example.org"); + let child_room_id_2 = room_id!("!2:example.org"); + + server + .mock_get_hierarchy() + .ok_with_room_ids(vec![child_room_id_1, child_room_id_2]) + .mount() + .await; + + let room_list = space_service.space_room_list(parent_space_id.to_owned()); + + room_list.paginate().await.unwrap(); + + // This space contains 2 rooms + assert_eq!(room_list.rooms().first().unwrap().room_id, child_room_id_1); + assert_eq!(room_list.rooms().last().unwrap().room_id, child_room_id_2); + + // and we don't know about either of them + assert_eq!(room_list.rooms().first().unwrap().state, None); + assert_eq!(room_list.rooms().last().unwrap().state, None); + + let rooms_subscriber = room_list.subscribe_to_room_updates(); + pin_mut!(rooms_subscriber); + assert_pending!(rooms_subscriber); + + // Joining one of them though + server.sync_room(&client, JoinedRoomBuilder::new(child_room_id_1)).await; + + // Results in an update being pushed through + assert_ready!(rooms_subscriber); + assert_eq!(room_list.rooms().first().unwrap().state, Some(RoomState::Joined)); + assert_eq!(room_list.rooms().last().unwrap().state, None); + + // Same for the second one + server.sync_room(&client, JoinedRoomBuilder::new(child_room_id_2)).await; + assert_ready!(rooms_subscriber); + assert_eq!(room_list.rooms().first().unwrap().state, Some(RoomState::Joined)); + assert_eq!(room_list.rooms().last().unwrap().state, Some(RoomState::Joined)); + + // And when leaving them + server.sync_room(&client, LeftRoomBuilder::new(child_room_id_1)).await; + server.sync_room(&client, LeftRoomBuilder::new(child_room_id_2)).await; + assert_ready!(rooms_subscriber); + assert_eq!(room_list.rooms().first().unwrap().state, Some(RoomState::Left)); + assert_eq!(room_list.rooms().last().unwrap().state, Some(RoomState::Left)); + } } From bafcd7d4713270940a66f6f3298ec95d624b59c1 Mon Sep 17 00:00:00 2001 From: Stefan Ceriu Date: Fri, 8 Aug 2025 15:30:02 +0300 Subject: [PATCH 07/35] fix(spaces): fix complement-crypto failures because of using an outdated uniffi version - automatic Arc inference was introduced in 0.27 and complement is using 0.25 --- bindings/matrix-sdk-ffi/src/spaces.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/bindings/matrix-sdk-ffi/src/spaces.rs b/bindings/matrix-sdk-ffi/src/spaces.rs index e6e1315fdad..25cdfd3c198 100644 --- a/bindings/matrix-sdk-ffi/src/spaces.rs +++ b/bindings/matrix-sdk-ffi/src/spaces.rs @@ -57,9 +57,9 @@ impl SpaceService { pub async fn space_room_list( &self, space_id: String, - ) -> Result { + ) -> Result, ClientError> { let space_id = RoomId::parse(space_id)?; - Ok(SpaceServiceRoomList::new(self.inner.space_room_list(space_id))) + Ok(Arc::new(SpaceServiceRoomList::new(self.inner.space_room_list(space_id)))) } } From 4cd3e1e8f8a2aa2eefc20d2b387df03babb17692 Mon Sep 17 00:00:00 2001 From: Stefan Ceriu Date: Fri, 8 Aug 2025 15:30:37 +0300 Subject: [PATCH 08/35] fix(spaces): use wasm compatible executor `spawn` and `JoinHandle` --- crates/matrix-sdk-ui/src/spaces/mod.rs | 4 ++-- crates/matrix-sdk-ui/src/spaces/room_list.rs | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/crates/matrix-sdk-ui/src/spaces/mod.rs b/crates/matrix-sdk-ui/src/spaces/mod.rs index 161bbe5f6d3..84e36eded31 100644 --- a/crates/matrix-sdk-ui/src/spaces/mod.rs +++ b/crates/matrix-sdk-ui/src/spaces/mod.rs @@ -19,8 +19,8 @@ use eyeball::{SharedObservable, Subscriber}; use futures_util::pin_mut; use matrix_sdk::Client; +use matrix_sdk_common::executor::{JoinHandle, spawn}; use ruma::OwnedRoomId; -use tokio::task::JoinHandle; use tokio_stream::StreamExt; use tracing::error; @@ -53,7 +53,7 @@ impl SpaceService { let joined_spaces_clone = joined_spaces.clone(); let all_room_updates_receiver = client.subscribe_to_all_room_updates(); - let handle = tokio::spawn(async move { + let handle = spawn(async move { pin_mut!(all_room_updates_receiver); loop { diff --git a/crates/matrix-sdk-ui/src/spaces/room_list.rs b/crates/matrix-sdk-ui/src/spaces/room_list.rs index 559d627427d..81b7125983a 100644 --- a/crates/matrix-sdk-ui/src/spaces/room_list.rs +++ b/crates/matrix-sdk-ui/src/spaces/room_list.rs @@ -17,8 +17,8 @@ use std::sync::Mutex; use eyeball::{SharedObservable, Subscriber}; use futures_util::pin_mut; use matrix_sdk::{Client, Error, paginators::PaginationToken}; +use matrix_sdk_common::executor::{JoinHandle, spawn}; use ruma::{OwnedRoomId, api::client::space::get_hierarchy}; -use tokio::task::JoinHandle; use tracing::error; use crate::spaces::SpaceServiceRoom; @@ -56,7 +56,7 @@ impl SpaceServiceRoomList { let rooms_clone = rooms.clone(); let all_room_updates_receiver = client.subscribe_to_all_room_updates(); - let handle = tokio::spawn(async move { + let handle = spawn(async move { pin_mut!(all_room_updates_receiver); loop { From dd728d9c067d21b1aa5348661a2fb4ffd68c3467 Mon Sep 17 00:00:00 2001 From: Stefan Ceriu Date: Mon, 11 Aug 2025 12:12:00 +0300 Subject: [PATCH 09/35] change(room_list): request both `m.space.parent` and `m.space.child` state events in the sliding sync required state as they're both required to build a full view of the space room hierarchy --- crates/matrix-sdk-ui/src/room_list_service/mod.rs | 2 ++ crates/matrix-sdk-ui/tests/integration/room_list_service.rs | 6 ++++++ 2 files changed, 8 insertions(+) diff --git a/crates/matrix-sdk-ui/src/room_list_service/mod.rs b/crates/matrix-sdk-ui/src/room_list_service/mod.rs index 3e12f905af4..38a2a6c62e9 100644 --- a/crates/matrix-sdk-ui/src/room_list_service/mod.rs +++ b/crates/matrix-sdk-ui/src/room_list_service/mod.rs @@ -96,6 +96,8 @@ const DEFAULT_REQUIRED_STATE: &[(StateEventType, &str)] = &[ (StateEventType::RoomHistoryVisibility, ""), // Required to correctly calculate the room display name. (StateEventType::MemberHints, ""), + (StateEventType::SpaceParent, "*"), + (StateEventType::SpaceChild, "*"), ]; /// The default `required_state` constant value for sliding sync room diff --git a/crates/matrix-sdk-ui/tests/integration/room_list_service.rs b/crates/matrix-sdk-ui/tests/integration/room_list_service.rs index 9d496df6048..0ceb2d8d373 100644 --- a/crates/matrix-sdk-ui/tests/integration/room_list_service.rs +++ b/crates/matrix-sdk-ui/tests/integration/room_list_service.rs @@ -369,6 +369,8 @@ async fn test_sync_all_states() -> Result<(), Error> { ["m.room.create", ""], ["m.room.history_visibility", ""], ["io.element.functional_members", ""], + ["m.space.parent", "*"], + ["m.space.child", "*"], ], "filters": {}, "timeline_limit": 1, @@ -2273,6 +2275,8 @@ async fn test_room_subscription() -> Result<(), Error> { ["m.room.create", ""], ["m.room.history_visibility", ""], ["io.element.functional_members", ""], + ["m.space.parent", "*"], + ["m.space.child", "*"], ["m.room.pinned_events", ""], ], "timeline_limit": 20, @@ -2316,6 +2320,8 @@ async fn test_room_subscription() -> Result<(), Error> { ["m.room.create", ""], ["m.room.history_visibility", ""], ["io.element.functional_members", ""], + ["m.space.parent", "*"], + ["m.space.child", "*"], ["m.room.pinned_events", ""], ], "timeline_limit": 20, From 8fddceef5df5b342a60aeb38bc84a66da99da4dc Mon Sep 17 00:00:00 2001 From: Stefan Ceriu Date: Mon, 11 Aug 2025 12:15:48 +0300 Subject: [PATCH 10/35] chore(spaces): build a graph from joined spaces parent and child relations to correctly detect all the edges and be able to remove any cycles. - cycle removing is done through DFS and keeping a list of visited nodes --- crates/matrix-sdk-ui/src/spaces/graph.rs | 200 +++++++++++++++++++++++ crates/matrix-sdk-ui/src/spaces/mod.rs | 79 +++++++-- 2 files changed, 262 insertions(+), 17 deletions(-) create mode 100644 crates/matrix-sdk-ui/src/spaces/graph.rs diff --git a/crates/matrix-sdk-ui/src/spaces/graph.rs b/crates/matrix-sdk-ui/src/spaces/graph.rs new file mode 100644 index 00000000000..1d703c42a16 --- /dev/null +++ b/crates/matrix-sdk-ui/src/spaces/graph.rs @@ -0,0 +1,200 @@ +// Copyright 2025 The Matrix.org Foundation C.I.C. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for that specific language governing permissions and +// limitations under the License. + +use std::collections::{BTreeMap, BTreeSet}; + +use ruma::OwnedRoomId; + +#[derive(Debug)] +struct SpaceServiceGraphNode { + id: OwnedRoomId, + parents: BTreeSet, + children: BTreeSet, +} + +impl SpaceServiceGraphNode { + fn new(id: OwnedRoomId) -> Self { + Self { id, parents: BTreeSet::new(), children: BTreeSet::new() } + } +} + +#[derive(Debug)] +pub struct SpaceServiceGraph { + nodes: BTreeMap, +} + +impl Default for SpaceServiceGraph { + fn default() -> Self { + Self::new() + } +} + +impl SpaceServiceGraph { + pub fn new() -> Self { + Self { nodes: BTreeMap::new() } + } + + pub fn root_nodes(&self) -> Vec<&OwnedRoomId> { + self.nodes.values().filter(|node| node.parents.is_empty()).map(|node| &node.id).collect() + } + + pub fn add_node(&mut self, node_id: OwnedRoomId) { + self.nodes.entry(node_id.clone()).or_insert(SpaceServiceGraphNode::new(node_id)); + } + + pub fn add_edge(&mut self, parent_id: OwnedRoomId, child_id: OwnedRoomId) { + self.nodes + .entry(parent_id.clone()) + .or_insert(SpaceServiceGraphNode::new(parent_id.clone())); + + self.nodes.entry(child_id.clone()).or_insert(SpaceServiceGraphNode::new(child_id.clone())); + + self.nodes.get_mut(&parent_id).unwrap().children.insert(child_id.clone()); + self.nodes.get_mut(&child_id).unwrap().parents.insert(parent_id); + } + + pub fn remove_cycles(&mut self) { + let mut visited = BTreeSet::new(); + let mut stack = BTreeSet::new(); + + let mut edges_to_remove = Vec::new(); + + for node_id in self.nodes.keys() { + self.dfs_remove_cycles(node_id, &mut visited, &mut stack, &mut edges_to_remove); + } + + for (parent, child) in edges_to_remove { + if let Some(node) = self.nodes.get_mut(&parent) { + node.children.remove(&child); + } + if let Some(node) = self.nodes.get_mut(&child) { + node.parents.remove(&parent); + } + } + } + + fn dfs_remove_cycles( + &self, + node_id: &OwnedRoomId, + visited: &mut BTreeSet, + stack: &mut BTreeSet, + edges_to_remove: &mut Vec<(OwnedRoomId, OwnedRoomId)>, + ) { + if !visited.insert(node_id.clone()) { + return; + } + + stack.insert(node_id.clone()); + + if let Some(node) = self.nodes.get(node_id) { + for child in &node.children { + if stack.contains(child) { + // Found a cycle → mark this edge for removal + edges_to_remove.push((node_id.clone(), child.clone())); + } else { + self.dfs_remove_cycles(child, visited, stack, edges_to_remove); + } + } + } + + stack.remove(node_id); + } +} + +#[cfg(test)] +mod tests { + use ruma::room_id; + + use super::*; + + #[test] + fn test_add_edge_and_root_nodes() { + let mut graph = SpaceServiceGraph::new(); + + let a = room_id!("!a:example.org").to_owned(); + let b = room_id!("!b:example.org").to_owned(); + let c = room_id!("!c:example.org").to_owned(); + + graph.add_edge(a.clone(), b.clone()); + graph.add_edge(a.clone(), c.clone()); + + assert_eq!(graph.root_nodes(), vec![&a]); + + assert!(graph.nodes[&b].parents.contains(&a)); + assert!(graph.nodes[&c].parents.contains(&a)); + } + + #[test] + fn test_remove_cycles() { + let mut graph = SpaceServiceGraph::new(); + + let a = room_id!("!a:example.org").to_owned(); + let b = room_id!("!b:example.org").to_owned(); + let c = room_id!("!c:example.org").to_owned(); + + graph.add_edge(a.clone(), b.clone()); + graph.add_edge(b, c.clone()); + graph.add_edge(c.clone(), a.clone()); // creates a cycle + + assert!(graph.nodes[&c].children.contains(&a)); + + graph.remove_cycles(); + + assert!(!graph.nodes[&c].children.contains(&a)); + assert!(!graph.nodes[&a].parents.contains(&c)); + } + + #[test] + fn test_disconnected_graph_roots() { + let mut graph = SpaceServiceGraph::new(); + + let a = room_id!("!a:example.org").to_owned(); + let b = room_id!("!b:example.org").to_owned(); + graph.add_edge(a.clone(), b); + + let x = room_id!("!x:example.org").to_owned(); + let y = room_id!("!y:example.org").to_owned(); + graph.add_edge(x.clone(), y); + + let mut roots = graph.root_nodes(); + roots.sort_by_key(|key| key.to_string()); + + let expected: Vec<&OwnedRoomId> = vec![&a, &x]; + assert_eq!(roots, expected); + } + + #[test] + fn test_multiple_parents() { + let mut graph = SpaceServiceGraph::new(); + + let a = room_id!("!a:example.org").to_owned(); + let b = room_id!("!b:example.org").to_owned(); + let c = room_id!("!c:example.org").to_owned(); + let d = room_id!("!d:example.org").to_owned(); + + graph.add_edge(a.clone(), c.clone()); + graph.add_edge(b.clone(), c.clone()); + graph.add_edge(c.clone(), d); + + let mut roots = graph.root_nodes(); + roots.sort_by_key(|key| key.to_string()); + + let expected = vec![&a, &b]; + assert_eq!(roots, expected); + + let c_parents = &graph.nodes[&c].parents; + assert!(c_parents.contains(&a)); + assert!(c_parents.contains(&b)); + } +} diff --git a/crates/matrix-sdk-ui/src/spaces/mod.rs b/crates/matrix-sdk-ui/src/spaces/mod.rs index 84e36eded31..a20af9498a0 100644 --- a/crates/matrix-sdk-ui/src/spaces/mod.rs +++ b/crates/matrix-sdk-ui/src/spaces/mod.rs @@ -18,14 +18,21 @@ use eyeball::{SharedObservable, Subscriber}; use futures_util::pin_mut; -use matrix_sdk::Client; +use matrix_sdk::{Client, deserialized_responses::SyncOrStrippedState}; use matrix_sdk_common::executor::{JoinHandle, spawn}; -use ruma::OwnedRoomId; -use tokio_stream::StreamExt; +use ruma::{ + OwnedRoomId, + events::{ + SyncStateEvent, + space::{child::SpaceChildEventContent, parent::SpaceParentEventContent}, + }, +}; use tracing::error; +use crate::spaces::graph::SpaceServiceGraph; pub use crate::spaces::{room::SpaceServiceRoom, room_list::SpaceServiceRoomList}; +pub mod graph; pub mod room; pub mod room_list; @@ -90,25 +97,63 @@ impl SpaceService { let joined_spaces = client .joined_rooms() .into_iter() - .filter_map(|room| if room.is_space() { Some(room) } else { None }) + .filter_map(|room| room.is_space().then_some(room)) .collect::>(); - let top_level_spaces: Vec = tokio_stream::iter(joined_spaces) - .then(|room| async move { - let ok = if let Ok(parents) = room.parent_spaces().await { - pin_mut!(parents); - !parents.any(|p| p.is_ok()).await + let mut graph = SpaceServiceGraph::new(); + + for space in joined_spaces.iter() { + graph.add_node(space.room_id().to_owned()); + + if let Ok(parents) = space.get_state_events_static::().await { + parents.into_iter() + .flat_map(|parent_event| match parent_event.deserialize() { + Ok(SyncOrStrippedState::Sync(SyncStateEvent::Original(e))) => { + Some(e.state_key) + } + Ok(SyncOrStrippedState::Sync(SyncStateEvent::Redacted(_))) => None, + Ok(SyncOrStrippedState::Stripped(e)) => Some(e.state_key), + Err(e) => { + error!(room_id = ?space.room_id(), "Could not deserialize m.space.parent: {e}"); + None + } + }).for_each(|parent| graph.add_edge(parent, space.room_id().to_owned())); + } else { + error!(room_id = ?space.room_id(), "Could not get m.space.parent events"); + } + + if let Ok(children) = space.get_state_events_static::().await { + children.into_iter() + .flat_map(|child_event| match child_event.deserialize() { + Ok(SyncOrStrippedState::Sync(SyncStateEvent::Original(e))) => { + Some(e.state_key) + } + Ok(SyncOrStrippedState::Sync(SyncStateEvent::Redacted(_))) => None, + Ok(SyncOrStrippedState::Stripped(e)) => Some(e.state_key), + Err(e) => { + error!(room_id = ?space.room_id(), "Could not deserialize m.space.child: {e}"); + None + } + }).for_each(|child| graph.add_edge(space.room_id().to_owned(), child)); + } else { + error!(room_id = ?space.room_id(), "Could not get m.space.child events"); + } + } + + graph.remove_cycles(); + + let root_notes = graph.root_nodes(); + + joined_spaces + .iter() + .flat_map(|room| { + if root_notes.contains(&&room.room_id().to_owned()) { + Some(SpaceServiceRoom::new_from_known(room.clone())) } else { - false - }; - (room, ok) + None + } }) - .filter(|(_, ok)| *ok) - .map(|(x, _)| SpaceServiceRoom::new_from_known(x)) .collect() - .await; - - top_level_spaces } } From 13aad5c8d7eaf4a3c0c2b18167b75730ac5b83b0 Mon Sep 17 00:00:00 2001 From: Stefan Ceriu Date: Mon, 11 Aug 2025 12:21:33 +0300 Subject: [PATCH 11/35] feat(ffi): expose `SpaceService::subscribe_to_joined_spaces` and `SpaceServiceRoomList::paginate` --- bindings/matrix-sdk-ffi/src/spaces.rs | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/bindings/matrix-sdk-ffi/src/spaces.rs b/bindings/matrix-sdk-ffi/src/spaces.rs index 25cdfd3c198..3e1a4ff5a00 100644 --- a/bindings/matrix-sdk-ffi/src/spaces.rs +++ b/bindings/matrix-sdk-ffi/src/spaces.rs @@ -49,6 +49,18 @@ impl SpaceService { self.inner.joined_spaces().into_iter().map(Into::into).collect() } + pub fn subscribe_to_joined_spaces(&self, listener: Box) { + let entries_stream = self.inner.subscribe_to_joined_spaces(); + + Arc::new(TaskHandle::new(get_runtime_handle().spawn(async move { + pin_mut!(entries_stream); + + while let Some(rooms) = entries_stream.next().await { + listener.on_update(rooms.into_iter().map(Into::into).collect()); + } + }))); + } + #[allow(clippy::unused_async)] // This method doesn't need to be async but if its not the FFI layer panics // with "there is no no reactor running, must be called from the context @@ -110,6 +122,10 @@ impl SpaceServiceRoomList { } }))); } + + pub async fn paginate(&self) -> Result<(), ClientError> { + self.inner.paginate().await.map_err(ClientError::from) + } } #[derive(uniffi::Enum)] @@ -143,6 +159,11 @@ pub trait SpaceServiceRoomListEntriesListener: SendOutsideWasm + SyncOutsideWasm fn on_update(&self, rooms: Vec); } +#[matrix_sdk_ffi_macros::export(callback_interface)] +pub trait SpaceServiceJoinedSpacesListener: SendOutsideWasm + SyncOutsideWasm + Debug { + fn on_update(&self, rooms: Vec); +} + #[derive(uniffi::Record)] pub struct SpaceServiceRoom { pub room_id: String, From d6d1799b5c83bc7665427c9bb9d2a3172b779a91 Mon Sep 17 00:00:00 2001 From: Stefan Ceriu Date: Mon, 11 Aug 2025 13:47:59 +0300 Subject: [PATCH 12/35] fix(spaces): return the TaskHandle from the FFI space service subscription methods so it can be retained on the client side --- bindings/matrix-sdk-ffi/src/spaces.rs | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/bindings/matrix-sdk-ffi/src/spaces.rs b/bindings/matrix-sdk-ffi/src/spaces.rs index 3e1a4ff5a00..3751c38b538 100644 --- a/bindings/matrix-sdk-ffi/src/spaces.rs +++ b/bindings/matrix-sdk-ffi/src/spaces.rs @@ -49,7 +49,10 @@ impl SpaceService { self.inner.joined_spaces().into_iter().map(Into::into).collect() } - pub fn subscribe_to_joined_spaces(&self, listener: Box) { + pub fn subscribe_to_joined_spaces( + &self, + listener: Box, + ) -> Arc { let entries_stream = self.inner.subscribe_to_joined_spaces(); Arc::new(TaskHandle::new(get_runtime_handle().spawn(async move { @@ -58,7 +61,7 @@ impl SpaceService { while let Some(rooms) = entries_stream.next().await { listener.on_update(rooms.into_iter().map(Into::into).collect()); } - }))); + }))) } #[allow(clippy::unused_async)] @@ -95,7 +98,7 @@ impl SpaceServiceRoomList { pub fn subscribe_to_pagination_state_updates( &self, listener: Box, - ) { + ) -> Arc { let pagination_state = self.inner.subscribe_to_pagination_state_updates(); Arc::new(TaskHandle::new(get_runtime_handle().spawn(async move { @@ -104,14 +107,17 @@ impl SpaceServiceRoomList { while let Some(state) = pagination_state.next().await { listener.on_update(state.into()); } - }))); + }))) } pub fn rooms(&self) -> Vec { self.inner.rooms().into_iter().map(Into::into).collect() } - pub fn subscribe_to_room_update(&self, listener: Box) { + pub fn subscribe_to_room_update( + &self, + listener: Box, + ) -> Arc { let entries_stream = self.inner.subscribe_to_room_updates(); Arc::new(TaskHandle::new(get_runtime_handle().spawn(async move { @@ -120,7 +126,7 @@ impl SpaceServiceRoomList { while let Some(rooms) = entries_stream.next().await { listener.on_update(rooms.into_iter().map(Into::into).collect()); } - }))); + }))) } pub async fn paginate(&self) -> Result<(), ClientError> { From 9246f0bc01083b4d96a5736f9879a2b57f290655 Mon Sep 17 00:00:00 2001 From: Stefan Ceriu Date: Mon, 11 Aug 2025 14:21:57 +0300 Subject: [PATCH 13/35] feat(spaces): expose the number of children each space room has --- bindings/matrix-sdk-ffi/src/spaces.rs | 2 + crates/matrix-sdk-ui/src/spaces/graph.rs | 6 +++ crates/matrix-sdk-ui/src/spaces/mod.rs | 41 ++++++++++++++----- crates/matrix-sdk-ui/src/spaces/room.rs | 11 ++++- crates/matrix-sdk-ui/src/spaces/room_list.rs | 24 +++++++++-- crates/matrix-sdk/src/test_utils/mocks/mod.rs | 30 ++++++++++++++ 6 files changed, 97 insertions(+), 17 deletions(-) diff --git a/bindings/matrix-sdk-ffi/src/spaces.rs b/bindings/matrix-sdk-ffi/src/spaces.rs index 3751c38b538..2b237696ab8 100644 --- a/bindings/matrix-sdk-ffi/src/spaces.rs +++ b/bindings/matrix-sdk-ffi/src/spaces.rs @@ -183,6 +183,7 @@ pub struct SpaceServiceRoom { pub world_readable: Option, pub guest_can_join: bool, + pub children_count: u64, pub state: Option, pub heroes: Option>, } @@ -200,6 +201,7 @@ impl From for SpaceServiceRoom { join_rule: room.join_rule.map(Into::into), world_readable: room.world_readable, guest_can_join: room.guest_can_join, + children_count: room.children_count, state: room.state.map(Into::into), heroes: room.heroes.map(|heroes| heroes.into_iter().map(Into::into).collect()), } diff --git a/crates/matrix-sdk-ui/src/spaces/graph.rs b/crates/matrix-sdk-ui/src/spaces/graph.rs index 1d703c42a16..202f5783424 100644 --- a/crates/matrix-sdk-ui/src/spaces/graph.rs +++ b/crates/matrix-sdk-ui/src/spaces/graph.rs @@ -49,6 +49,10 @@ impl SpaceServiceGraph { self.nodes.values().filter(|node| node.parents.is_empty()).map(|node| &node.id).collect() } + pub fn children_of(&self, node_id: &OwnedRoomId) -> Vec<&OwnedRoomId> { + self.nodes.get(node_id).map_or(vec![], |node| node.children.iter().collect()) + } + pub fn add_node(&mut self, node_id: OwnedRoomId) { self.nodes.entry(node_id.clone()).or_insert(SpaceServiceGraphNode::new(node_id)); } @@ -133,6 +137,8 @@ mod tests { assert!(graph.nodes[&b].parents.contains(&a)); assert!(graph.nodes[&c].parents.contains(&a)); + + assert_eq!(graph.children_of(&a), vec![&b, &c]); } #[test] diff --git a/crates/matrix-sdk-ui/src/spaces/mod.rs b/crates/matrix-sdk-ui/src/spaces/mod.rs index a20af9498a0..43b374a5082 100644 --- a/crates/matrix-sdk-ui/src/spaces/mod.rs +++ b/crates/matrix-sdk-ui/src/spaces/mod.rs @@ -147,8 +147,13 @@ impl SpaceService { joined_spaces .iter() .flat_map(|room| { - if root_notes.contains(&&room.room_id().to_owned()) { - Some(SpaceServiceRoom::new_from_known(room.clone())) + let room_id = room.room_id().to_owned(); + + if root_notes.contains(&&room_id) { + Some(SpaceServiceRoom::new_from_known( + room.clone(), + graph.children_of(&room_id).len() as u64, + )) } else { None } @@ -165,7 +170,7 @@ mod tests { use matrix_sdk_test::{ JoinedRoomBuilder, LeftRoomBuilder, async_test, event_factory::EventFactory, }; - use ruma::{RoomVersionId, room_id}; + use ruma::{RoomVersionId, owned_room_id, room_id}; use stream_assert::{assert_next_eq, assert_pending}; use super::*; @@ -235,6 +240,12 @@ mod tests { vec![parent_space_id] ); + // and it has 2 children + assert_eq!( + space_service.joined_spaces().iter().map(|s| s.children_count).collect::>(), + vec![2] + ); + let parent_space = client.get_room(parent_space_id).unwrap(); assert!(parent_space.is_space()); @@ -299,7 +310,7 @@ mod tests { assert_eq!( space_service.joined_spaces(), - vec![SpaceServiceRoom::new_from_known(client.get_room(first_space_id).unwrap())] + vec![SpaceServiceRoom::new_from_known(client.get_room(first_space_id).unwrap(), 0)] ); // Join the second space @@ -308,7 +319,15 @@ mod tests { .sync_room( &client, JoinedRoomBuilder::new(second_space_id) - .add_state_event(factory.create(user_id, RoomVersionId::V1).with_space_type()), + .add_state_event(factory.create(user_id, RoomVersionId::V1).with_space_type()) + .add_state_event( + factory + .space_child( + second_space_id.to_owned(), + owned_room_id!("!child:example.org"), + ) + .sender(user_id), + ), ) .await; @@ -316,8 +335,8 @@ mod tests { assert_eq!( space_service.joined_spaces(), vec![ - SpaceServiceRoom::new_from_known(client.get_room(first_space_id).unwrap()), - SpaceServiceRoom::new_from_known(client.get_room(second_space_id).unwrap()) + SpaceServiceRoom::new_from_known(client.get_room(first_space_id).unwrap(), 0), + SpaceServiceRoom::new_from_known(client.get_room(second_space_id).unwrap(), 1) ] ); @@ -325,8 +344,8 @@ mod tests { assert_next_eq!( joined_spaces_subscriber, vec![ - SpaceServiceRoom::new_from_known(client.get_room(first_space_id).unwrap()), - SpaceServiceRoom::new_from_known(client.get_room(second_space_id).unwrap()) + SpaceServiceRoom::new_from_known(client.get_room(first_space_id).unwrap(), 0), + SpaceServiceRoom::new_from_known(client.get_room(second_space_id).unwrap(), 1) ] ); @@ -335,7 +354,7 @@ mod tests { // and when one is left assert_next_eq!( joined_spaces_subscriber, - vec![SpaceServiceRoom::new_from_known(client.get_room(first_space_id).unwrap())] + vec![SpaceServiceRoom::new_from_known(client.get_room(first_space_id).unwrap(), 0)] ); // but it doesn't when a non-space room gets joined @@ -351,7 +370,7 @@ mod tests { assert_pending!(joined_spaces_subscriber); assert_eq!( space_service.joined_spaces(), - vec![SpaceServiceRoom::new_from_known(client.get_room(first_space_id).unwrap())] + vec![SpaceServiceRoom::new_from_known(client.get_room(first_space_id).unwrap(), 0)] ); } } diff --git a/crates/matrix-sdk-ui/src/spaces/room.rs b/crates/matrix-sdk-ui/src/spaces/room.rs index 8bc778ca5c9..11d623273e7 100644 --- a/crates/matrix-sdk-ui/src/spaces/room.rs +++ b/crates/matrix-sdk-ui/src/spaces/room.rs @@ -32,12 +32,17 @@ pub struct SpaceServiceRoom { pub world_readable: Option, pub guest_can_join: bool, + pub children_count: u64, pub state: Option, pub heroes: Option>, } impl SpaceServiceRoom { - pub fn new_from_summary(summary: &RoomSummary, known_room: Option) -> Self { + pub fn new_from_summary( + summary: &RoomSummary, + known_room: Option, + children_count: u64, + ) -> Self { Self { room_id: summary.room_id.clone(), canonical_alias: summary.canonical_alias.clone(), @@ -49,12 +54,13 @@ impl SpaceServiceRoom { join_rule: Some(summary.join_rule.clone()), world_readable: Some(summary.world_readable), guest_can_join: summary.guest_can_join, + children_count, state: known_room.as_ref().map(|r| r.state()), heroes: known_room.map(|r| r.heroes()), } } - pub fn new_from_known(known_room: Room) -> Self { + pub fn new_from_known(known_room: Room, children_count: u64) -> Self { let room_info = known_room.clone_info(); Self { @@ -70,6 +76,7 @@ impl SpaceServiceRoom { .history_visibility() .map(|vis| *vis == HistoryVisibility::WorldReadable), guest_can_join: known_room.guest_access() == GuestAccess::CanJoin, + children_count, state: Some(known_room.state()), heroes: Some(room_info.heroes().to_vec()), } diff --git a/crates/matrix-sdk-ui/src/spaces/room_list.rs b/crates/matrix-sdk-ui/src/spaces/room_list.rs index 81b7125983a..5b40f45c425 100644 --- a/crates/matrix-sdk-ui/src/spaces/room_list.rs +++ b/crates/matrix-sdk-ui/src/spaces/room_list.rs @@ -69,7 +69,10 @@ impl SpaceServiceRoomList { new_rooms.iter_mut().find(|room| &room.room_id == updated_room_id) && let Some(update_room) = client_clone.get_room(updated_room_id) { - *room = SpaceServiceRoom::new_from_known(update_room); + *room = SpaceServiceRoom::new_from_known( + update_room, + room.children_count, + ); } }); @@ -151,8 +154,16 @@ impl SpaceServiceRoomList { result .rooms .iter() - .map(|room| (&room.summary, self.client.get_room(&room.summary.room_id))) - .map(|(summary, room)| SpaceServiceRoom::new_from_summary(summary, room)) + .map(|room| { + ( + &room.summary, + self.client.get_room(&room.summary.room_id), + room.children_state.len(), + ) + }) + .map(|(summary, room, children_count)| { + SpaceServiceRoom::new_from_summary(summary, room, children_count as u64) + }) .collect::>(), ); @@ -226,7 +237,10 @@ mod tests { // Paginating the room list server .mock_get_hierarchy() - .ok_with_room_ids(vec![child_space_id_1, child_space_id_2]) + .ok_with_room_ids_and_children_state( + vec![child_space_id_1, child_space_id_2], + vec![room_id!("!child:example.org")], + ) .mount() .await; @@ -251,6 +265,7 @@ mod tests { false, ), None, + 1 ), SpaceServiceRoom::new_from_summary( &RoomSummary::new( @@ -261,6 +276,7 @@ mod tests { false, ), None, + 1 ), ] ); diff --git a/crates/matrix-sdk/src/test_utils/mocks/mod.rs b/crates/matrix-sdk/src/test_utils/mocks/mod.rs index 1c7c89fa1f0..b72a28dcf3d 100644 --- a/crates/matrix-sdk/src/test_utils/mocks/mod.rs +++ b/crates/matrix-sdk/src/test_utils/mocks/mod.rs @@ -4243,6 +4243,36 @@ impl<'a> MockEndpoint<'a, GetHierarchyEndpoint> { }))) } + /// Returns a successful response containing the given room IDs and children + /// states + pub fn ok_with_room_ids_and_children_state( + self, + room_ids: Vec<&RoomId>, + children_state: Vec<&RoomId>, + ) -> MatrixMock<'a> { + let children_state = children_state + .into_iter() + .map(|id| json!({ "type": "m.space.child", "state_key": id })) + .collect::>(); + + let rooms = room_ids + .iter() + .map(|id| { + json!({ + "room_id": id, + "num_joined_members": 1, + "world_readable": false, + "guest_can_join": false, + "children_state": children_state + }) + }) + .collect::>(); + + self.respond_with(ResponseTemplate::new(200).set_body_json(json!({ + "rooms": rooms, + }))) + } + /// Returns a successful response with an empty list of rooms. pub fn ok(self) -> MatrixMock<'a> { self.respond_with(ResponseTemplate::new(200).set_body_json(json!({ From c71065c87c350e08bac95104655c85f574aa48d9 Mon Sep 17 00:00:00 2001 From: Stefan Ceriu Date: Mon, 11 Aug 2025 14:42:21 +0300 Subject: [PATCH 14/35] change(spaces): put a limit of 1 on the max depth of the /hierarchy calls so only the direct children are fetche --- crates/matrix-sdk-ui/src/spaces/room_list.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/crates/matrix-sdk-ui/src/spaces/room_list.rs b/crates/matrix-sdk-ui/src/spaces/room_list.rs index 5b40f45c425..2156622afdc 100644 --- a/crates/matrix-sdk-ui/src/spaces/room_list.rs +++ b/crates/matrix-sdk-ui/src/spaces/room_list.rs @@ -18,7 +18,7 @@ use eyeball::{SharedObservable, Subscriber}; use futures_util::pin_mut; use matrix_sdk::{Client, Error, paginators::PaginationToken}; use matrix_sdk_common::executor::{JoinHandle, spawn}; -use ruma::{OwnedRoomId, api::client::space::get_hierarchy}; +use ruma::{OwnedRoomId, api::client::space::get_hierarchy, uint}; use tracing::error; use crate::spaces::SpaceServiceRoom; @@ -131,6 +131,7 @@ impl SpaceServiceRoomList { self.pagination_state.set(SpaceServiceRoomListPaginationState::Loading); let mut request = get_hierarchy::v1::Request::new(self.parent_space_id.clone()); + request.max_depth = Some(uint!(1)); // We only want the immediate children of the space if let PaginationToken::HasMore(ref token) = *self.token.lock().unwrap() { request.from = Some(token.clone()); From 87558cd33ecc62695bad363d3ca042778f8c5c38 Mon Sep 17 00:00:00 2001 From: Stefan Ceriu Date: Mon, 11 Aug 2025 16:04:14 +0300 Subject: [PATCH 15/35] fix(spaces): filter out the current parent space from the `/hierarchy` responses and `SpaceServiceRoomList` instances --- crates/matrix-sdk-ui/src/spaces/room_list.rs | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/crates/matrix-sdk-ui/src/spaces/room_list.rs b/crates/matrix-sdk-ui/src/spaces/room_list.rs index 2156622afdc..5345cb62a05 100644 --- a/crates/matrix-sdk-ui/src/spaces/room_list.rs +++ b/crates/matrix-sdk-ui/src/spaces/room_list.rs @@ -155,15 +155,16 @@ impl SpaceServiceRoomList { result .rooms .iter() - .map(|room| { - ( - &room.summary, - self.client.get_room(&room.summary.room_id), - room.children_state.len(), - ) - }) - .map(|(summary, room, children_count)| { - SpaceServiceRoom::new_from_summary(summary, room, children_count as u64) + .flat_map(|room| { + if room.summary.room_id == self.parent_space_id { + None + } else { + Some(SpaceServiceRoom::new_from_summary( + &room.summary, + self.client.get_room(&room.summary.room_id), + room.children_state.len() as u64, + )) + } }) .collect::>(), ); From 136e068501052d12a925eb9ee0d446fc2866fd39 Mon Sep 17 00:00:00 2001 From: Stefan Ceriu Date: Mon, 11 Aug 2025 17:56:47 +0300 Subject: [PATCH 16/35] change(spaces): make the `SpaceService` constructor non-async and instead automatically setup a client subscription when requesting the joined services subscription --- bindings/matrix-sdk-ffi/src/client.rs | 4 +- bindings/matrix-sdk-ffi/src/spaces.rs | 11 ++- crates/matrix-sdk-ui/src/spaces/mod.rs | 96 ++++++++++++-------- crates/matrix-sdk-ui/src/spaces/room_list.rs | 4 +- 4 files changed, 70 insertions(+), 45 deletions(-) diff --git a/bindings/matrix-sdk-ffi/src/client.rs b/bindings/matrix-sdk-ffi/src/client.rs index f4c27d239fc..43bfc9dfcaa 100644 --- a/bindings/matrix-sdk-ffi/src/client.rs +++ b/bindings/matrix-sdk-ffi/src/client.rs @@ -1259,8 +1259,8 @@ impl Client { SyncServiceBuilder::new((*self.inner).clone(), self.utd_hook_manager.get().cloned()) } - pub async fn spaces_service(&self) -> Arc { - let inner = UISpaceService::new((*self.inner).clone()).await; + pub fn spaces_service(&self) -> Arc { + let inner = UISpaceService::new((*self.inner).clone()); Arc::new(SpaceService::new(inner)) } diff --git a/bindings/matrix-sdk-ffi/src/spaces.rs b/bindings/matrix-sdk-ffi/src/spaces.rs index 2b237696ab8..fc952ead7fb 100644 --- a/bindings/matrix-sdk-ffi/src/spaces.rs +++ b/bindings/matrix-sdk-ffi/src/spaces.rs @@ -45,11 +45,16 @@ impl SpaceService { #[matrix_sdk_ffi_macros::export] impl SpaceService { - pub fn joined_spaces(&self) -> Vec { - self.inner.joined_spaces().into_iter().map(Into::into).collect() + pub async fn joined_spaces(&self) -> Vec { + self.inner.joined_spaces().await.into_iter().map(Into::into).collect() } - pub fn subscribe_to_joined_spaces( + #[allow(clippy::unused_async)] + // This method doesn't need to be async but if its not the FFI layer panics + // with "there is no no reactor running, must be called from the context + // of a Tokio 1.x runtime" error because the underlying method spawns an + // async task. + pub async fn subscribe_to_joined_spaces( &self, listener: Box, ) -> Arc { diff --git a/crates/matrix-sdk-ui/src/spaces/mod.rs b/crates/matrix-sdk-ui/src/spaces/mod.rs index 43b374a5082..786277c03cb 100644 --- a/crates/matrix-sdk-ui/src/spaces/mod.rs +++ b/crates/matrix-sdk-ui/src/spaces/mod.rs @@ -18,7 +18,7 @@ use eyeball::{SharedObservable, Subscriber}; use futures_util::pin_mut; -use matrix_sdk::{Client, deserialized_responses::SyncOrStrippedState}; +use matrix_sdk::{Client, deserialized_responses::SyncOrStrippedState, locks::Mutex}; use matrix_sdk_common::executor::{JoinHandle, spawn}; use ruma::{ OwnedRoomId, @@ -41,52 +41,62 @@ pub struct SpaceService { joined_spaces: SharedObservable>, - room_update_handle: JoinHandle<()>, + room_update_handle: Mutex>>, } impl Drop for SpaceService { fn drop(&mut self) { - self.room_update_handle.abort(); + if let Some(handle) = &*self.room_update_handle.lock() { + handle.abort(); + } } } impl SpaceService { - pub async fn new(client: Client) -> Self { - let joined_spaces = SharedObservable::new(Vec::new()); - - joined_spaces.set(Self::joined_spaces_for(&client).await); - - let client_clone = client.clone(); - let joined_spaces_clone = joined_spaces.clone(); - let all_room_updates_receiver = client.subscribe_to_all_room_updates(); - - let handle = spawn(async move { - pin_mut!(all_room_updates_receiver); + pub fn new(client: Client) -> Self { + Self { + client, + joined_spaces: SharedObservable::new(Vec::new()), + room_update_handle: Mutex::new(None), + } + } - loop { - match all_room_updates_receiver.recv().await { - Ok(_) => { - let new_spaces = Self::joined_spaces_for(&client_clone).await; - if new_spaces != joined_spaces_clone.get() { - joined_spaces_clone.set(new_spaces); + pub fn subscribe_to_joined_spaces(&self) -> Subscriber> { + if self.room_update_handle.lock().is_none() { + let client_clone = self.client.clone(); + let joined_spaces_clone = self.joined_spaces.clone(); + let all_room_updates_receiver = self.client.subscribe_to_all_room_updates(); + + *self.room_update_handle.lock() = Some(spawn(async move { + pin_mut!(all_room_updates_receiver); + + loop { + match all_room_updates_receiver.recv().await { + Ok(_) => { + let new_spaces = Self::joined_spaces_for(&client_clone).await; + if new_spaces != joined_spaces_clone.get() { + joined_spaces_clone.set(new_spaces); + } + } + Err(err) => { + error!("error when listening to room updates: {err}"); } - } - Err(err) => { - error!("error when listening to room updates: {err}"); } } - } - }); - - Self { client, joined_spaces, room_update_handle: handle } - } + })); + } - pub fn subscribe_to_joined_spaces(&self) -> Subscriber> { self.joined_spaces.subscribe() } - pub fn joined_spaces(&self) -> Vec { - self.joined_spaces.get() + pub async fn joined_spaces(&self) -> Vec { + let spaces = Self::joined_spaces_for(&self.client).await; + + if spaces != self.joined_spaces.get() { + self.joined_spaces.set(spaces.clone()); + } + + spaces } pub fn space_room_list(&self, space_id: OwnedRoomId) -> SpaceServiceRoomList { @@ -180,7 +190,7 @@ mod tests { let server = MatrixMockServer::new().await; let client = server.client_builder().build().await; let user_id = client.user_id().unwrap(); - let space_service = SpaceService::new(client.clone()).await; + let space_service = SpaceService::new(client.clone()); let factory = EventFactory::new(); server.mock_room_state_encryption().plain().mount().await; @@ -236,13 +246,23 @@ mod tests { // Only the parent space is returned assert_eq!( - space_service.joined_spaces().iter().map(|s| s.room_id.to_owned()).collect::>(), + space_service + .joined_spaces() + .await + .iter() + .map(|s| s.room_id.to_owned()) + .collect::>(), vec![parent_space_id] ); // and it has 2 children assert_eq!( - space_service.joined_spaces().iter().map(|s| s.children_count).collect::>(), + space_service + .joined_spaces() + .await + .iter() + .map(|s| s.children_count) + .collect::>(), vec![2] ); @@ -302,14 +322,14 @@ mod tests { // Build the `SpaceService` and expect the room to show up with no updates // pending - let space_service = SpaceService::new(client.clone()).await; + let space_service = SpaceService::new(client.clone()); let joined_spaces_subscriber = space_service.subscribe_to_joined_spaces(); pin_mut!(joined_spaces_subscriber); assert_pending!(joined_spaces_subscriber); assert_eq!( - space_service.joined_spaces(), + space_service.joined_spaces().await, vec![SpaceServiceRoom::new_from_known(client.get_room(first_space_id).unwrap(), 0)] ); @@ -333,7 +353,7 @@ mod tests { // And expect the list to update assert_eq!( - space_service.joined_spaces(), + space_service.joined_spaces().await, vec![ SpaceServiceRoom::new_from_known(client.get_room(first_space_id).unwrap(), 0), SpaceServiceRoom::new_from_known(client.get_room(second_space_id).unwrap(), 1) @@ -369,7 +389,7 @@ mod tests { // and the subscriber doesn't yield any updates assert_pending!(joined_spaces_subscriber); assert_eq!( - space_service.joined_spaces(), + space_service.joined_spaces().await, vec![SpaceServiceRoom::new_from_known(client.get_room(first_space_id).unwrap(), 0)] ); } diff --git a/crates/matrix-sdk-ui/src/spaces/room_list.rs b/crates/matrix-sdk-ui/src/spaces/room_list.rs index 5345cb62a05..293b815167f 100644 --- a/crates/matrix-sdk-ui/src/spaces/room_list.rs +++ b/crates/matrix-sdk-ui/src/spaces/room_list.rs @@ -206,7 +206,7 @@ mod tests { async fn test_room_list_pagination() { let server = MatrixMockServer::new().await; let client = server.client_builder().build().await; - let space_service = SpaceService::new(client.clone()).await; + let space_service = SpaceService::new(client.clone()); server.mock_room_state_encryption().plain().mount().await; @@ -288,7 +288,7 @@ mod tests { async fn test_room_state_updates() { let server = MatrixMockServer::new().await; let client = server.client_builder().build().await; - let space_service = SpaceService::new(client.clone()).await; + let space_service = SpaceService::new(client.clone()); let parent_space_id = room_id!("!parent_space:example.org"); let child_room_id_1 = room_id!("!1:example.org"); From 0993802c2527e4e53bfea68a969bab3a6cb6b9dc Mon Sep 17 00:00:00 2001 From: Stefan Ceriu Date: Tue, 12 Aug 2025 14:25:30 +0300 Subject: [PATCH 17/35] chore(spaces): converge on single naming scheme for all spaces related components on both the UI and the FFI crates --- bindings/matrix-sdk-ffi/src/client.rs | 2 +- bindings/matrix-sdk-ffi/src/spaces.rs | 72 +++++++------------- crates/matrix-sdk-ui/src/spaces/graph.rs | 28 ++++---- crates/matrix-sdk-ui/src/spaces/mod.rs | 34 ++++----- crates/matrix-sdk-ui/src/spaces/room.rs | 4 +- crates/matrix-sdk-ui/src/spaces/room_list.rs | 56 +++++++-------- 6 files changed, 84 insertions(+), 112 deletions(-) diff --git a/bindings/matrix-sdk-ffi/src/client.rs b/bindings/matrix-sdk-ffi/src/client.rs index 43bfc9dfcaa..e4c7306bf07 100644 --- a/bindings/matrix-sdk-ffi/src/client.rs +++ b/bindings/matrix-sdk-ffi/src/client.rs @@ -1259,7 +1259,7 @@ impl Client { SyncServiceBuilder::new((*self.inner).clone(), self.utd_hook_manager.get().cloned()) } - pub fn spaces_service(&self) -> Arc { + pub fn space_service(&self) -> Arc { let inner = UISpaceService::new((*self.inner).clone()); Arc::new(SpaceService::new(inner)) } diff --git a/bindings/matrix-sdk-ffi/src/spaces.rs b/bindings/matrix-sdk-ffi/src/spaces.rs index fc952ead7fb..5453b6b8f57 100644 --- a/bindings/matrix-sdk-ffi/src/spaces.rs +++ b/bindings/matrix-sdk-ffi/src/spaces.rs @@ -17,9 +17,8 @@ use std::{fmt::Debug, sync::Arc}; use futures_util::{pin_mut, StreamExt}; use matrix_sdk_common::{SendOutsideWasm, SyncOutsideWasm}; use matrix_sdk_ui::spaces::{ - room_list::SpaceServiceRoomListPaginationState as UISpaceServiceRoomListPaginationState, - SpaceService as UISpaceService, SpaceServiceRoom as UISpaceServiceRoom, - SpaceServiceRoomList as UISpaceServiceRoomList, + room_list::SpaceRoomListPaginationState, SpaceRoom as UISpaceRoom, + SpaceRoomList as UISpaceRoomList, SpaceService as UISpaceService, }; use ruma::RoomId; @@ -38,14 +37,14 @@ pub struct SpaceService { } impl SpaceService { - pub fn new(inner: UISpaceService) -> Self { + pub(crate) fn new(inner: UISpaceService) -> Self { Self { inner } } } #[matrix_sdk_ffi_macros::export] impl SpaceService { - pub async fn joined_spaces(&self) -> Vec { + pub async fn joined_spaces(&self) -> Vec { self.inner.joined_spaces().await.into_iter().map(Into::into).collect() } @@ -77,32 +76,32 @@ impl SpaceService { pub async fn space_room_list( &self, space_id: String, - ) -> Result, ClientError> { + ) -> Result, ClientError> { let space_id = RoomId::parse(space_id)?; - Ok(Arc::new(SpaceServiceRoomList::new(self.inner.space_room_list(space_id)))) + Ok(Arc::new(SpaceRoomList::new(self.inner.space_room_list(space_id)))) } } #[derive(uniffi::Object)] -pub struct SpaceServiceRoomList { - inner: UISpaceServiceRoomList, +pub struct SpaceRoomList { + inner: UISpaceRoomList, } -impl SpaceServiceRoomList { - pub fn new(inner: UISpaceServiceRoomList) -> Self { +impl SpaceRoomList { + fn new(inner: UISpaceRoomList) -> Self { Self { inner } } } #[matrix_sdk_ffi_macros::export] -impl SpaceServiceRoomList { - pub fn pagination_state(&self) -> SpaceServiceRoomListPaginationState { - self.inner.pagination_state().into() +impl SpaceRoomList { + pub fn pagination_state(&self) -> SpaceRoomListPaginationState { + self.inner.pagination_state() } pub fn subscribe_to_pagination_state_updates( &self, - listener: Box, + listener: Box, ) -> Arc { let pagination_state = self.inner.subscribe_to_pagination_state_updates(); @@ -110,18 +109,18 @@ impl SpaceServiceRoomList { pin_mut!(pagination_state); while let Some(state) = pagination_state.next().await { - listener.on_update(state.into()); + listener.on_update(state); } }))) } - pub fn rooms(&self) -> Vec { + pub fn rooms(&self) -> Vec { self.inner.rooms().into_iter().map(Into::into).collect() } pub fn subscribe_to_room_update( &self, - listener: Box, + listener: Box, ) -> Arc { let entries_stream = self.inner.subscribe_to_room_updates(); @@ -139,44 +138,23 @@ impl SpaceServiceRoomList { } } -#[derive(uniffi::Enum)] -pub enum SpaceServiceRoomListPaginationState { - Idle { end_reached: bool }, - Loading, -} - -impl From for SpaceServiceRoomListPaginationState { - fn from(state: UISpaceServiceRoomListPaginationState) -> Self { - match state { - UISpaceServiceRoomListPaginationState::Idle { end_reached } => { - SpaceServiceRoomListPaginationState::Idle { end_reached } - } - UISpaceServiceRoomListPaginationState::Loading => { - SpaceServiceRoomListPaginationState::Loading - } - } - } -} - #[matrix_sdk_ffi_macros::export(callback_interface)] -pub trait SpaceServiceRoomListPaginationStateListener: - SendOutsideWasm + SyncOutsideWasm + Debug -{ - fn on_update(&self, pagination_state: SpaceServiceRoomListPaginationState); +pub trait SpaceRoomListPaginationStateListener: SendOutsideWasm + SyncOutsideWasm + Debug { + fn on_update(&self, pagination_state: SpaceRoomListPaginationState); } #[matrix_sdk_ffi_macros::export(callback_interface)] -pub trait SpaceServiceRoomListEntriesListener: SendOutsideWasm + SyncOutsideWasm + Debug { - fn on_update(&self, rooms: Vec); +pub trait SpaceRoomListEntriesListener: SendOutsideWasm + SyncOutsideWasm + Debug { + fn on_update(&self, rooms: Vec); } #[matrix_sdk_ffi_macros::export(callback_interface)] pub trait SpaceServiceJoinedSpacesListener: SendOutsideWasm + SyncOutsideWasm + Debug { - fn on_update(&self, rooms: Vec); + fn on_update(&self, rooms: Vec); } #[derive(uniffi::Record)] -pub struct SpaceServiceRoom { +pub struct SpaceRoom { pub room_id: String, pub canonical_alias: Option, pub name: Option, @@ -193,8 +171,8 @@ pub struct SpaceServiceRoom { pub heroes: Option>, } -impl From for SpaceServiceRoom { - fn from(room: UISpaceServiceRoom) -> Self { +impl From for SpaceRoom { + fn from(room: UISpaceRoom) -> Self { Self { room_id: room.room_id.into(), canonical_alias: room.canonical_alias.map(|alias| alias.into()), diff --git a/crates/matrix-sdk-ui/src/spaces/graph.rs b/crates/matrix-sdk-ui/src/spaces/graph.rs index 202f5783424..a173ead23b7 100644 --- a/crates/matrix-sdk-ui/src/spaces/graph.rs +++ b/crates/matrix-sdk-ui/src/spaces/graph.rs @@ -17,30 +17,30 @@ use std::collections::{BTreeMap, BTreeSet}; use ruma::OwnedRoomId; #[derive(Debug)] -struct SpaceServiceGraphNode { +struct SpaceGraphNode { id: OwnedRoomId, parents: BTreeSet, children: BTreeSet, } -impl SpaceServiceGraphNode { +impl SpaceGraphNode { fn new(id: OwnedRoomId) -> Self { Self { id, parents: BTreeSet::new(), children: BTreeSet::new() } } } #[derive(Debug)] -pub struct SpaceServiceGraph { - nodes: BTreeMap, +pub struct SpaceGraph { + nodes: BTreeMap, } -impl Default for SpaceServiceGraph { +impl Default for SpaceGraph { fn default() -> Self { Self::new() } } -impl SpaceServiceGraph { +impl SpaceGraph { pub fn new() -> Self { Self { nodes: BTreeMap::new() } } @@ -54,15 +54,13 @@ impl SpaceServiceGraph { } pub fn add_node(&mut self, node_id: OwnedRoomId) { - self.nodes.entry(node_id.clone()).or_insert(SpaceServiceGraphNode::new(node_id)); + self.nodes.entry(node_id.clone()).or_insert(SpaceGraphNode::new(node_id)); } pub fn add_edge(&mut self, parent_id: OwnedRoomId, child_id: OwnedRoomId) { - self.nodes - .entry(parent_id.clone()) - .or_insert(SpaceServiceGraphNode::new(parent_id.clone())); + self.nodes.entry(parent_id.clone()).or_insert(SpaceGraphNode::new(parent_id.clone())); - self.nodes.entry(child_id.clone()).or_insert(SpaceServiceGraphNode::new(child_id.clone())); + self.nodes.entry(child_id.clone()).or_insert(SpaceGraphNode::new(child_id.clone())); self.nodes.get_mut(&parent_id).unwrap().children.insert(child_id.clone()); self.nodes.get_mut(&child_id).unwrap().parents.insert(parent_id); @@ -124,7 +122,7 @@ mod tests { #[test] fn test_add_edge_and_root_nodes() { - let mut graph = SpaceServiceGraph::new(); + let mut graph = SpaceGraph::new(); let a = room_id!("!a:example.org").to_owned(); let b = room_id!("!b:example.org").to_owned(); @@ -143,7 +141,7 @@ mod tests { #[test] fn test_remove_cycles() { - let mut graph = SpaceServiceGraph::new(); + let mut graph = SpaceGraph::new(); let a = room_id!("!a:example.org").to_owned(); let b = room_id!("!b:example.org").to_owned(); @@ -163,7 +161,7 @@ mod tests { #[test] fn test_disconnected_graph_roots() { - let mut graph = SpaceServiceGraph::new(); + let mut graph = SpaceGraph::new(); let a = room_id!("!a:example.org").to_owned(); let b = room_id!("!b:example.org").to_owned(); @@ -182,7 +180,7 @@ mod tests { #[test] fn test_multiple_parents() { - let mut graph = SpaceServiceGraph::new(); + let mut graph = SpaceGraph::new(); let a = room_id!("!a:example.org").to_owned(); let b = room_id!("!b:example.org").to_owned(); diff --git a/crates/matrix-sdk-ui/src/spaces/mod.rs b/crates/matrix-sdk-ui/src/spaces/mod.rs index 786277c03cb..a08b9371871 100644 --- a/crates/matrix-sdk-ui/src/spaces/mod.rs +++ b/crates/matrix-sdk-ui/src/spaces/mod.rs @@ -29,8 +29,8 @@ use ruma::{ }; use tracing::error; -use crate::spaces::graph::SpaceServiceGraph; -pub use crate::spaces::{room::SpaceServiceRoom, room_list::SpaceServiceRoomList}; +use crate::spaces::graph::SpaceGraph; +pub use crate::spaces::{room::SpaceRoom, room_list::SpaceRoomList}; pub mod graph; pub mod room; @@ -39,7 +39,7 @@ pub mod room_list; pub struct SpaceService { client: Client, - joined_spaces: SharedObservable>, + joined_spaces: SharedObservable>, room_update_handle: Mutex>>, } @@ -61,7 +61,7 @@ impl SpaceService { } } - pub fn subscribe_to_joined_spaces(&self) -> Subscriber> { + pub fn subscribe_to_joined_spaces(&self) -> Subscriber> { if self.room_update_handle.lock().is_none() { let client_clone = self.client.clone(); let joined_spaces_clone = self.joined_spaces.clone(); @@ -89,7 +89,7 @@ impl SpaceService { self.joined_spaces.subscribe() } - pub async fn joined_spaces(&self) -> Vec { + pub async fn joined_spaces(&self) -> Vec { let spaces = Self::joined_spaces_for(&self.client).await; if spaces != self.joined_spaces.get() { @@ -99,18 +99,18 @@ impl SpaceService { spaces } - pub fn space_room_list(&self, space_id: OwnedRoomId) -> SpaceServiceRoomList { - SpaceServiceRoomList::new(self.client.clone(), space_id) + pub fn space_room_list(&self, space_id: OwnedRoomId) -> SpaceRoomList { + SpaceRoomList::new(self.client.clone(), space_id) } - async fn joined_spaces_for(client: &Client) -> Vec { + async fn joined_spaces_for(client: &Client) -> Vec { let joined_spaces = client .joined_rooms() .into_iter() .filter_map(|room| room.is_space().then_some(room)) .collect::>(); - let mut graph = SpaceServiceGraph::new(); + let mut graph = SpaceGraph::new(); for space in joined_spaces.iter() { graph.add_node(space.room_id().to_owned()); @@ -160,7 +160,7 @@ impl SpaceService { let room_id = room.room_id().to_owned(); if root_notes.contains(&&room_id) { - Some(SpaceServiceRoom::new_from_known( + Some(SpaceRoom::new_from_known( room.clone(), graph.children_of(&room_id).len() as u64, )) @@ -330,7 +330,7 @@ mod tests { assert_eq!( space_service.joined_spaces().await, - vec![SpaceServiceRoom::new_from_known(client.get_room(first_space_id).unwrap(), 0)] + vec![SpaceRoom::new_from_known(client.get_room(first_space_id).unwrap(), 0)] ); // Join the second space @@ -355,8 +355,8 @@ mod tests { assert_eq!( space_service.joined_spaces().await, vec![ - SpaceServiceRoom::new_from_known(client.get_room(first_space_id).unwrap(), 0), - SpaceServiceRoom::new_from_known(client.get_room(second_space_id).unwrap(), 1) + SpaceRoom::new_from_known(client.get_room(first_space_id).unwrap(), 0), + SpaceRoom::new_from_known(client.get_room(second_space_id).unwrap(), 1) ] ); @@ -364,8 +364,8 @@ mod tests { assert_next_eq!( joined_spaces_subscriber, vec![ - SpaceServiceRoom::new_from_known(client.get_room(first_space_id).unwrap(), 0), - SpaceServiceRoom::new_from_known(client.get_room(second_space_id).unwrap(), 1) + SpaceRoom::new_from_known(client.get_room(first_space_id).unwrap(), 0), + SpaceRoom::new_from_known(client.get_room(second_space_id).unwrap(), 1) ] ); @@ -374,7 +374,7 @@ mod tests { // and when one is left assert_next_eq!( joined_spaces_subscriber, - vec![SpaceServiceRoom::new_from_known(client.get_room(first_space_id).unwrap(), 0)] + vec![SpaceRoom::new_from_known(client.get_room(first_space_id).unwrap(), 0)] ); // but it doesn't when a non-space room gets joined @@ -390,7 +390,7 @@ mod tests { assert_pending!(joined_spaces_subscriber); assert_eq!( space_service.joined_spaces().await, - vec![SpaceServiceRoom::new_from_known(client.get_room(first_space_id).unwrap(), 0)] + vec![SpaceRoom::new_from_known(client.get_room(first_space_id).unwrap(), 0)] ); } } diff --git a/crates/matrix-sdk-ui/src/spaces/room.rs b/crates/matrix-sdk-ui/src/spaces/room.rs index 11d623273e7..592d9d0a050 100644 --- a/crates/matrix-sdk-ui/src/spaces/room.rs +++ b/crates/matrix-sdk-ui/src/spaces/room.rs @@ -20,7 +20,7 @@ use ruma::{ }; #[derive(Debug, Clone, PartialEq)] -pub struct SpaceServiceRoom { +pub struct SpaceRoom { pub room_id: OwnedRoomId, pub canonical_alias: Option, pub name: Option, @@ -37,7 +37,7 @@ pub struct SpaceServiceRoom { pub heroes: Option>, } -impl SpaceServiceRoom { +impl SpaceRoom { pub fn new_from_summary( summary: &RoomSummary, known_room: Option, diff --git a/crates/matrix-sdk-ui/src/spaces/room_list.rs b/crates/matrix-sdk-ui/src/spaces/room_list.rs index 293b815167f..c15dc11e43b 100644 --- a/crates/matrix-sdk-ui/src/spaces/room_list.rs +++ b/crates/matrix-sdk-ui/src/spaces/room_list.rs @@ -21,36 +21,37 @@ use matrix_sdk_common::executor::{JoinHandle, spawn}; use ruma::{OwnedRoomId, api::client::space::get_hierarchy, uint}; use tracing::error; -use crate::spaces::SpaceServiceRoom; +use crate::spaces::SpaceRoom; +#[cfg_attr(feature = "uniffi", derive(uniffi::Enum))] #[derive(Clone, Debug, Eq, PartialEq)] -pub enum SpaceServiceRoomListPaginationState { +pub enum SpaceRoomListPaginationState { Idle { end_reached: bool }, Loading, } -pub struct SpaceServiceRoomList { +pub struct SpaceRoomList { client: Client, parent_space_id: OwnedRoomId, token: Mutex, - pagination_state: SharedObservable, + pagination_state: SharedObservable, - rooms: SharedObservable>, + rooms: SharedObservable>, room_update_handle: JoinHandle<()>, } -impl Drop for SpaceServiceRoomList { +impl Drop for SpaceRoomList { fn drop(&mut self) { self.room_update_handle.abort(); } } -impl SpaceServiceRoomList { +impl SpaceRoomList { pub fn new(client: Client, parent_space_id: OwnedRoomId) -> Self { - let rooms = SharedObservable::new(Vec::::new()); + let rooms = SharedObservable::new(Vec::::new()); let client_clone = client.clone(); let rooms_clone = rooms.clone(); @@ -69,10 +70,7 @@ impl SpaceServiceRoomList { new_rooms.iter_mut().find(|room| &room.room_id == updated_room_id) && let Some(update_room) = client_clone.get_room(updated_room_id) { - *room = SpaceServiceRoom::new_from_known( - update_room, - room.children_count, - ); + *room = SpaceRoom::new_from_known(update_room, room.children_count); } }); @@ -91,7 +89,7 @@ impl SpaceServiceRoomList { client, parent_space_id, token: Mutex::new(None.into()), - pagination_state: SharedObservable::new(SpaceServiceRoomListPaginationState::Idle { + pagination_state: SharedObservable::new(SpaceRoomListPaginationState::Idle { end_reached: false, }), rooms, @@ -99,36 +97,36 @@ impl SpaceServiceRoomList { } } - pub fn pagination_state(&self) -> SpaceServiceRoomListPaginationState { + pub fn pagination_state(&self) -> SpaceRoomListPaginationState { self.pagination_state.get() } pub fn subscribe_to_pagination_state_updates( &self, - ) -> Subscriber { + ) -> Subscriber { self.pagination_state.subscribe() } - pub fn rooms(&self) -> Vec { + pub fn rooms(&self) -> Vec { self.rooms.get() } - pub fn subscribe_to_room_updates(&self) -> Subscriber> { + pub fn subscribe_to_room_updates(&self) -> Subscriber> { self.rooms.subscribe() } pub async fn paginate(&self) -> Result<(), Error> { match *self.pagination_state.read() { - SpaceServiceRoomListPaginationState::Idle { end_reached } if end_reached => { + SpaceRoomListPaginationState::Idle { end_reached } if end_reached => { return Ok(()); } - SpaceServiceRoomListPaginationState::Loading => { + SpaceRoomListPaginationState::Loading => { return Ok(()); } _ => {} } - self.pagination_state.set(SpaceServiceRoomListPaginationState::Loading); + self.pagination_state.set(SpaceRoomListPaginationState::Loading); let mut request = get_hierarchy::v1::Request::new(self.parent_space_id.clone()); request.max_depth = Some(uint!(1)); // We only want the immediate children of the space @@ -159,7 +157,7 @@ impl SpaceServiceRoomList { if room.summary.room_id == self.parent_space_id { None } else { - Some(SpaceServiceRoom::new_from_summary( + Some(SpaceRoom::new_from_summary( &room.summary, self.client.get_room(&room.summary.room_id), room.children_state.len() as u64, @@ -171,7 +169,7 @@ impl SpaceServiceRoomList { self.rooms.set(current_rooms.clone()); - self.pagination_state.set(SpaceServiceRoomListPaginationState::Idle { + self.pagination_state.set(SpaceRoomListPaginationState::Idle { end_reached: result.next_batch.is_none(), }); @@ -179,7 +177,7 @@ impl SpaceServiceRoomList { } Err(err) => { self.pagination_state - .set(SpaceServiceRoomListPaginationState::Idle { end_reached: false }); + .set(SpaceRoomListPaginationState::Idle { end_reached: false }); Err(err.into()) } } @@ -198,9 +196,7 @@ mod tests { }; use stream_assert::{assert_next_eq, assert_next_matches, assert_pending, assert_ready}; - use crate::spaces::{ - SpaceService, SpaceServiceRoom, room_list::SpaceServiceRoomListPaginationState, - }; + use crate::spaces::{SpaceRoom, SpaceService, room_list::SpaceRoomListPaginationState}; #[async_test] async fn test_room_list_pagination() { @@ -217,7 +213,7 @@ mod tests { // Start off idle assert_matches!( room_list.pagination_state(), - SpaceServiceRoomListPaginationState::Idle { end_reached: false } + SpaceRoomListPaginationState::Idle { end_reached: false } ); // without any rooms @@ -251,14 +247,14 @@ mod tests { // informs that the pagination reached the end assert_next_matches!( pagination_state_subscriber, - SpaceServiceRoomListPaginationState::Idle { end_reached: true } + SpaceRoomListPaginationState::Idle { end_reached: true } ); // yields results assert_next_eq!( rooms_subscriber, vec![ - SpaceServiceRoom::new_from_summary( + SpaceRoom::new_from_summary( &RoomSummary::new( child_space_id_1.to_owned(), JoinRuleSummary::Public, @@ -269,7 +265,7 @@ mod tests { None, 1 ), - SpaceServiceRoom::new_from_summary( + SpaceRoom::new_from_summary( &RoomSummary::new( child_space_id_2.to_owned(), JoinRuleSummary::Public, From fb106b7c58287a3c7a2cc768d95656c45a23ac91 Mon Sep 17 00:00:00 2001 From: Stefan Ceriu Date: Wed, 13 Aug 2025 10:38:56 +0300 Subject: [PATCH 18/35] chore(spaces): add changelog --- crates/matrix-sdk-ui/CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/crates/matrix-sdk-ui/CHANGELOG.md b/crates/matrix-sdk-ui/CHANGELOG.md index 8ffcb3ab8bb..942805a1f15 100644 --- a/crates/matrix-sdk-ui/CHANGELOG.md +++ b/crates/matrix-sdk-ui/CHANGELOG.md @@ -7,6 +7,9 @@ All notable changes to this project will be documented in this file. ## [Unreleased] - ReleaseDate ### Features +- Add a new [`SpaceService`] that provides high level reactive interfaces for listing + the user's joined top level spaces as long as their children. + ([#5509](https://github.com/matrix-org/matrix-rust-sdk/pull/5509)) - Add `new_filter_low_priority` and `new_filter_non_low_priority` filters to the room list filtering system, allowing clients to filter rooms based on their low priority status. The filters use the `Room::is_low_priority()` method which checks for the `m.lowpriority` room tag. From 6ecc27040741702b938381b5d835c0641bf6e165 Mon Sep 17 00:00:00 2001 From: Stefan Ceriu Date: Thu, 14 Aug 2025 14:32:40 +0300 Subject: [PATCH 19/35] change(spaces): publish VectorDiffs instead of a full vectors for joined spaces and space room list subscriptions --- bindings/matrix-sdk-ffi/src/spaces.rs | 71 ++++++++-- crates/matrix-sdk-ui/src/spaces/mod.rs | 77 ++++++++--- crates/matrix-sdk-ui/src/spaces/room_list.rs | 136 ++++++++++--------- 3 files changed, 185 insertions(+), 99 deletions(-) diff --git a/bindings/matrix-sdk-ffi/src/spaces.rs b/bindings/matrix-sdk-ffi/src/spaces.rs index 5453b6b8f57..2d1d5460060 100644 --- a/bindings/matrix-sdk-ffi/src/spaces.rs +++ b/bindings/matrix-sdk-ffi/src/spaces.rs @@ -14,6 +14,7 @@ use std::{fmt::Debug, sync::Arc}; +use eyeball_im::VectorDiff; use futures_util::{pin_mut, StreamExt}; use matrix_sdk_common::{SendOutsideWasm, SyncOutsideWasm}; use matrix_sdk_ui::spaces::{ @@ -57,17 +58,18 @@ impl SpaceService { &self, listener: Box, ) -> Arc { - let entries_stream = self.inner.subscribe_to_joined_spaces(); + let (initial_values, mut stream) = self.inner.subscribe_to_joined_spaces(); - Arc::new(TaskHandle::new(get_runtime_handle().spawn(async move { - pin_mut!(entries_stream); + listener.on_update(vec![SpaceListUpdate::Reset { + values: initial_values.into_iter().map(Into::into).collect(), + }]); - while let Some(rooms) = entries_stream.next().await { - listener.on_update(rooms.into_iter().map(Into::into).collect()); + Arc::new(TaskHandle::new(get_runtime_handle().spawn(async move { + while let Some(diffs) = stream.next().await { + listener.on_update(diffs.into_iter().map(Into::into).collect()); } }))) } - #[allow(clippy::unused_async)] // This method doesn't need to be async but if its not the FFI layer panics // with "there is no no reactor running, must be called from the context @@ -122,13 +124,15 @@ impl SpaceRoomList { &self, listener: Box, ) -> Arc { - let entries_stream = self.inner.subscribe_to_room_updates(); + let (initial_values, mut stream) = self.inner.subscribe_to_room_updates(); - Arc::new(TaskHandle::new(get_runtime_handle().spawn(async move { - pin_mut!(entries_stream); + listener.on_update(vec![SpaceListUpdate::Reset { + values: initial_values.into_iter().map(Into::into).collect(), + }]); - while let Some(rooms) = entries_stream.next().await { - listener.on_update(rooms.into_iter().map(Into::into).collect()); + Arc::new(TaskHandle::new(get_runtime_handle().spawn(async move { + while let Some(diffs) = stream.next().await { + listener.on_update(diffs.into_iter().map(Into::into).collect()); } }))) } @@ -145,12 +149,12 @@ pub trait SpaceRoomListPaginationStateListener: SendOutsideWasm + SyncOutsideWas #[matrix_sdk_ffi_macros::export(callback_interface)] pub trait SpaceRoomListEntriesListener: SendOutsideWasm + SyncOutsideWasm + Debug { - fn on_update(&self, rooms: Vec); + fn on_update(&self, rooms: Vec); } #[matrix_sdk_ffi_macros::export(callback_interface)] pub trait SpaceServiceJoinedSpacesListener: SendOutsideWasm + SyncOutsideWasm + Debug { - fn on_update(&self, rooms: Vec); + fn on_update(&self, room_updates: Vec); } #[derive(uniffi::Record)] @@ -190,3 +194,44 @@ impl From for SpaceRoom { } } } + +#[derive(uniffi::Enum)] +pub enum SpaceListUpdate { + Append { values: Vec }, + Clear, + PushFront { value: SpaceRoom }, + PushBack { value: SpaceRoom }, + PopFront, + PopBack, + Insert { index: u32, value: SpaceRoom }, + Set { index: u32, value: SpaceRoom }, + Remove { index: u32 }, + Truncate { length: u32 }, + Reset { values: Vec }, +} + +impl From> for SpaceListUpdate { + fn from(diff: VectorDiff) -> Self { + match diff { + VectorDiff::Append { values } => { + Self::Append { values: values.into_iter().map(|v| v.into()).collect() } + } + VectorDiff::Clear => Self::Clear, + VectorDiff::PushFront { value } => Self::PushFront { value: value.into() }, + VectorDiff::PushBack { value } => Self::PushBack { value: value.into() }, + VectorDiff::PopFront => Self::PopFront, + VectorDiff::PopBack => Self::PopBack, + VectorDiff::Insert { index, value } => { + Self::Insert { index: index as u32, value: value.into() } + } + VectorDiff::Set { index, value } => { + Self::Set { index: index as u32, value: value.into() } + } + VectorDiff::Remove { index } => Self::Remove { index: index as u32 }, + VectorDiff::Truncate { length } => Self::Truncate { length: length as u32 }, + VectorDiff::Reset { values } => { + Self::Reset { values: values.into_iter().map(|v| v.into()).collect() } + } + } + } +} diff --git a/crates/matrix-sdk-ui/src/spaces/mod.rs b/crates/matrix-sdk-ui/src/spaces/mod.rs index a08b9371871..e039b92830e 100644 --- a/crates/matrix-sdk-ui/src/spaces/mod.rs +++ b/crates/matrix-sdk-ui/src/spaces/mod.rs @@ -16,8 +16,11 @@ //! //! See [`SpaceService`] for details. -use eyeball::{SharedObservable, Subscriber}; +use std::sync::Arc; + +use eyeball_im::{ObservableVector, VectorSubscriberBatchedStream}; use futures_util::pin_mut; +use imbl::Vector; use matrix_sdk::{Client, deserialized_responses::SyncOrStrippedState, locks::Mutex}; use matrix_sdk_common::executor::{JoinHandle, spawn}; use ruma::{ @@ -39,7 +42,7 @@ pub mod room_list; pub struct SpaceService { client: Client, - joined_spaces: SharedObservable>, + joined_spaces: Arc>>, room_update_handle: Mutex>>, } @@ -56,15 +59,17 @@ impl SpaceService { pub fn new(client: Client) -> Self { Self { client, - joined_spaces: SharedObservable::new(Vec::new()), + joined_spaces: Arc::new(Mutex::new(ObservableVector::new())), room_update_handle: Mutex::new(None), } } - pub fn subscribe_to_joined_spaces(&self) -> Subscriber> { + pub fn subscribe_to_joined_spaces( + &self, + ) -> (Vector, VectorSubscriberBatchedStream) { if self.room_update_handle.lock().is_none() { - let client_clone = self.client.clone(); - let joined_spaces_clone = self.joined_spaces.clone(); + let client = self.client.clone(); + let joined_spaces = Arc::clone(&self.joined_spaces); let all_room_updates_receiver = self.client.subscribe_to_all_room_updates(); *self.room_update_handle.lock() = Some(spawn(async move { @@ -73,10 +78,8 @@ impl SpaceService { loop { match all_room_updates_receiver.recv().await { Ok(_) => { - let new_spaces = Self::joined_spaces_for(&client_clone).await; - if new_spaces != joined_spaces_clone.get() { - joined_spaces_clone.set(new_spaces); - } + let new_spaces = Vector::from(Self::joined_spaces_for(&client).await); + Self::update_joined_spaces_if_needed(new_spaces, &joined_spaces); } Err(err) => { error!("error when listening to room updates: {err}"); @@ -86,15 +89,13 @@ impl SpaceService { })); } - self.joined_spaces.subscribe() + self.joined_spaces.lock().subscribe().into_values_and_batched_stream() } pub async fn joined_spaces(&self) -> Vec { let spaces = Self::joined_spaces_for(&self.client).await; - if spaces != self.joined_spaces.get() { - self.joined_spaces.set(spaces.clone()); - } + Self::update_joined_spaces_if_needed(Vector::from(spaces.clone()), &self.joined_spaces); spaces } @@ -103,6 +104,18 @@ impl SpaceService { SpaceRoomList::new(self.client.clone(), space_id) } + fn update_joined_spaces_if_needed( + new_spaces: Vector, + joined_spaces: &Arc>>, + ) { + let old_spaces = joined_spaces.lock().clone(); + + if new_spaces != old_spaces { + joined_spaces.lock().clear(); + joined_spaces.lock().append(new_spaces); + } + } + async fn joined_spaces_for(client: &Client) -> Vec { let joined_spaces = client .joined_rooms() @@ -175,6 +188,7 @@ impl SpaceService { #[cfg(test)] mod tests { use assert_matches2::assert_let; + use eyeball_im::VectorDiff; use futures_util::{StreamExt, pin_mut}; use matrix_sdk::{room::ParentSpace, test_utils::mocks::MatrixMockServer}; use matrix_sdk_test::{ @@ -324,7 +338,7 @@ mod tests { let space_service = SpaceService::new(client.clone()); - let joined_spaces_subscriber = space_service.subscribe_to_joined_spaces(); + let (_, joined_spaces_subscriber) = space_service.subscribe_to_joined_spaces(); pin_mut!(joined_spaces_subscriber); assert_pending!(joined_spaces_subscriber); @@ -333,6 +347,17 @@ mod tests { vec![SpaceRoom::new_from_known(client.get_room(first_space_id).unwrap(), 0)] ); + assert_next_eq!( + joined_spaces_subscriber, + vec![VectorDiff::Append { + values: vec![SpaceRoom::new_from_known( + client.get_room(first_space_id).unwrap(), + 0 + )] + .into() + }] + ); + // Join the second space server @@ -360,12 +385,17 @@ mod tests { ] ); - // The subscriber yields new results when a space is joined assert_next_eq!( joined_spaces_subscriber, vec![ - SpaceRoom::new_from_known(client.get_room(first_space_id).unwrap(), 0), - SpaceRoom::new_from_known(client.get_room(second_space_id).unwrap(), 1) + VectorDiff::Clear, + VectorDiff::Append { + values: vec![ + SpaceRoom::new_from_known(client.get_room(first_space_id).unwrap(), 0), + SpaceRoom::new_from_known(client.get_room(second_space_id).unwrap(), 1) + ] + .into() + }, ] ); @@ -374,7 +404,16 @@ mod tests { // and when one is left assert_next_eq!( joined_spaces_subscriber, - vec![SpaceRoom::new_from_known(client.get_room(first_space_id).unwrap(), 0)] + vec![ + VectorDiff::Clear, + VectorDiff::Append { + values: vec![SpaceRoom::new_from_known( + client.get_room(first_space_id).unwrap(), + 0 + )] + .into() + }, + ] ); // but it doesn't when a non-space room gets joined diff --git a/crates/matrix-sdk-ui/src/spaces/room_list.rs b/crates/matrix-sdk-ui/src/spaces/room_list.rs index c15dc11e43b..25098467c47 100644 --- a/crates/matrix-sdk-ui/src/spaces/room_list.rs +++ b/crates/matrix-sdk-ui/src/spaces/room_list.rs @@ -12,11 +12,14 @@ // See the License for that specific language governing permissions and // limitations under the License. -use std::sync::Mutex; +use std::sync::Arc; use eyeball::{SharedObservable, Subscriber}; +use eyeball_im::{ObservableVector, VectorSubscriberBatchedStream}; use futures_util::pin_mut; -use matrix_sdk::{Client, Error, paginators::PaginationToken}; +use imbl::Vector; +use itertools::Itertools; +use matrix_sdk::{Client, Error, locks::Mutex, paginators::PaginationToken}; use matrix_sdk_common::executor::{JoinHandle, spawn}; use ruma::{OwnedRoomId, api::client::space::get_hierarchy, uint}; use tracing::error; @@ -39,7 +42,7 @@ pub struct SpaceRoomList { pagination_state: SharedObservable, - rooms: SharedObservable>, + rooms: Arc>>, room_update_handle: JoinHandle<()>, } @@ -51,7 +54,7 @@ impl Drop for SpaceRoomList { } impl SpaceRoomList { pub fn new(client: Client, parent_space_id: OwnedRoomId) -> Self { - let rooms = SharedObservable::new(Vec::::new()); + let rooms = Arc::new(Mutex::new(ObservableVector::::new())); let client_clone = client.clone(); let rooms_clone = rooms.clone(); @@ -63,20 +66,21 @@ impl SpaceRoomList { loop { match all_room_updates_receiver.recv().await { Ok(updates) => { - let mut new_rooms = rooms_clone.get(); + let mut mutable_rooms = rooms_clone.lock(); updates.iter_all_room_ids().for_each(|updated_room_id| { - if let Some(room) = - new_rooms.iter_mut().find(|room| &room.room_id == updated_room_id) + if let Some((position, room)) = mutable_rooms + .clone() + .iter() + .find_position(|room| &room.room_id == updated_room_id) && let Some(update_room) = client_clone.get_room(updated_room_id) { - *room = SpaceRoom::new_from_known(update_room, room.children_count); + mutable_rooms.set( + position, + SpaceRoom::new_from_known(update_room, room.children_count), + ); } - }); - - if new_rooms != rooms_clone.get() { - rooms_clone.set(new_rooms); - } + }) } Err(err) => { error!("error when listening to room updates: {err}"); @@ -108,11 +112,13 @@ impl SpaceRoomList { } pub fn rooms(&self) -> Vec { - self.rooms.get() + self.rooms.lock().iter().cloned().collect_vec() } - pub fn subscribe_to_room_updates(&self) -> Subscriber> { - self.rooms.subscribe() + pub fn subscribe_to_room_updates( + &self, + ) -> (Vector, VectorSubscriberBatchedStream) { + self.rooms.lock().subscribe().into_values_and_batched_stream() } pub async fn paginate(&self) -> Result<(), Error> { @@ -131,43 +137,34 @@ impl SpaceRoomList { let mut request = get_hierarchy::v1::Request::new(self.parent_space_id.clone()); request.max_depth = Some(uint!(1)); // We only want the immediate children of the space - if let PaginationToken::HasMore(ref token) = *self.token.lock().unwrap() { + if let PaginationToken::HasMore(ref token) = *self.token.lock() { request.from = Some(token.clone()); } match self.client.send(request).await { Ok(result) => { - let mut token = self.token.lock().unwrap(); + let mut token = self.token.lock(); *token = match &result.next_batch { Some(val) => PaginationToken::HasMore(val.clone()), None => PaginationToken::HitEnd, }; - let mut current_rooms = Vec::new(); - - (self.rooms.read()).iter().for_each(|room| { - current_rooms.push(room.clone()); - }); - - current_rooms.extend( - result - .rooms - .iter() - .flat_map(|room| { - if room.summary.room_id == self.parent_space_id { - None - } else { - Some(SpaceRoom::new_from_summary( - &room.summary, - self.client.get_room(&room.summary.room_id), - room.children_state.len() as u64, - )) - } - }) - .collect::>(), - ); - - self.rooms.set(current_rooms.clone()); + let mut rooms = self.rooms.lock(); + result + .rooms + .iter() + .flat_map(|room| { + if room.summary.room_id == self.parent_space_id { + None + } else { + Some(SpaceRoom::new_from_summary( + &room.summary, + self.client.get_room(&room.summary.room_id), + room.children_state.len() as u64, + )) + } + }) + .for_each(|room| rooms.push_back(room)); self.pagination_state.set(SpaceRoomListPaginationState::Idle { end_reached: result.next_batch.is_none(), @@ -187,6 +184,7 @@ impl SpaceRoomList { #[cfg(test)] mod tests { use assert_matches2::assert_matches; + use eyeball_im::VectorDiff; use futures_util::pin_mut; use matrix_sdk::{RoomState, test_utils::mocks::MatrixMockServer}; use matrix_sdk_test::{JoinedRoomBuilder, LeftRoomBuilder, async_test}; @@ -225,7 +223,7 @@ mod tests { pin_mut!(pagination_state_subscriber); assert_pending!(pagination_state_subscriber); - let rooms_subscriber = room_list.subscribe_to_room_updates(); + let (_, rooms_subscriber) = room_list.subscribe_to_room_updates(); pin_mut!(rooms_subscriber); assert_pending!(rooms_subscriber); @@ -250,32 +248,36 @@ mod tests { SpaceRoomListPaginationState::Idle { end_reached: true } ); - // yields results + // and yields results assert_next_eq!( rooms_subscriber, vec![ - SpaceRoom::new_from_summary( - &RoomSummary::new( - child_space_id_1.to_owned(), - JoinRuleSummary::Public, - false, - uint!(1), - false, - ), - None, - 1 - ), - SpaceRoom::new_from_summary( - &RoomSummary::new( - child_space_id_2.to_owned(), - JoinRuleSummary::Public, - false, - uint!(1), - false, + VectorDiff::PushBack { + value: SpaceRoom::new_from_summary( + &RoomSummary::new( + child_space_id_1.to_owned(), + JoinRuleSummary::Public, + false, + uint!(1), + false, + ), + None, + 1 + ) + }, + VectorDiff::PushBack { + value: SpaceRoom::new_from_summary( + &RoomSummary::new( + child_space_id_2.to_owned(), + JoinRuleSummary::Public, + false, + uint!(1), + false, + ), + None, + 1 ), - None, - 1 - ), + } ] ); } @@ -308,7 +310,7 @@ mod tests { assert_eq!(room_list.rooms().first().unwrap().state, None); assert_eq!(room_list.rooms().last().unwrap().state, None); - let rooms_subscriber = room_list.subscribe_to_room_updates(); + let (_, rooms_subscriber) = room_list.subscribe_to_room_updates(); pin_mut!(rooms_subscriber); assert_pending!(rooms_subscriber); From 9e2e46c567d35784af14e9bc78a41dd0a2625359 Mon Sep 17 00:00:00 2001 From: Stefan Ceriu Date: Mon, 18 Aug 2025 11:39:35 +0300 Subject: [PATCH 20/35] chore(spaces): ignore room list change updates if empty --- crates/matrix-sdk-ui/src/spaces/mod.rs | 6 +++++- crates/matrix-sdk-ui/src/spaces/room_list.rs | 4 ++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/crates/matrix-sdk-ui/src/spaces/mod.rs b/crates/matrix-sdk-ui/src/spaces/mod.rs index e039b92830e..47f7e34c0b5 100644 --- a/crates/matrix-sdk-ui/src/spaces/mod.rs +++ b/crates/matrix-sdk-ui/src/spaces/mod.rs @@ -77,7 +77,11 @@ impl SpaceService { loop { match all_room_updates_receiver.recv().await { - Ok(_) => { + Ok(updates) => { + if updates.is_empty() { + continue; + } + let new_spaces = Vector::from(Self::joined_spaces_for(&client).await); Self::update_joined_spaces_if_needed(new_spaces, &joined_spaces); } diff --git a/crates/matrix-sdk-ui/src/spaces/room_list.rs b/crates/matrix-sdk-ui/src/spaces/room_list.rs index 25098467c47..32aad7d1b72 100644 --- a/crates/matrix-sdk-ui/src/spaces/room_list.rs +++ b/crates/matrix-sdk-ui/src/spaces/room_list.rs @@ -66,6 +66,10 @@ impl SpaceRoomList { loop { match all_room_updates_receiver.recv().await { Ok(updates) => { + if updates.is_empty() { + continue; + } + let mut mutable_rooms = rooms_clone.lock(); updates.iter_all_room_ids().for_each(|updated_room_id| { From c00b82660ca51a788250205954778b52b717b3aa Mon Sep 17 00:00:00 2001 From: Stefan Ceriu Date: Tue, 19 Aug 2025 09:59:59 +0300 Subject: [PATCH 21/35] docs(spaces): add documentation, comments and examples --- bindings/matrix-sdk-ffi/src/spaces.rs | 30 +++++++++ crates/matrix-sdk-ui/src/spaces/graph.rs | 13 ++++ crates/matrix-sdk-ui/src/spaces/mod.rs | 60 ++++++++++++++++- crates/matrix-sdk-ui/src/spaces/room.rs | 2 + crates/matrix-sdk-ui/src/spaces/room_list.rs | 69 ++++++++++++++++++++ 5 files changed, 173 insertions(+), 1 deletion(-) diff --git a/bindings/matrix-sdk-ffi/src/spaces.rs b/bindings/matrix-sdk-ffi/src/spaces.rs index 2d1d5460060..41712ee11e4 100644 --- a/bindings/matrix-sdk-ffi/src/spaces.rs +++ b/bindings/matrix-sdk-ffi/src/spaces.rs @@ -32,6 +32,11 @@ use crate::{ TaskHandle, }; +/// The main entry point into the Spaces facilities. +/// +/// The spaces service is responsible for retrieving one's joined rooms, +/// building a graph out of their `m.space.parent` and `m.space.child` state +/// events, and providing access to the top-level spaces and their children. #[derive(uniffi::Object)] pub struct SpaceService { inner: UISpaceService, @@ -45,10 +50,15 @@ impl SpaceService { #[matrix_sdk_ffi_macros::export] impl SpaceService { + /// Returns a list of all the top-level joined spaces. It will eagerly + /// compute the latest version and also notify subscribers if there were + /// any changes. pub async fn joined_spaces(&self) -> Vec { self.inner.joined_spaces().await.into_iter().map(Into::into).collect() } + /// Subscribes to updates on the joined spaces list. If space rooms are + /// joined or left, the stream will yield diffs that reflect the changes. #[allow(clippy::unused_async)] // This method doesn't need to be async but if its not the FFI layer panics // with "there is no no reactor running, must be called from the context @@ -70,6 +80,8 @@ impl SpaceService { } }))) } + + /// Returns a `SpaceRoomList` for the given space ID. #[allow(clippy::unused_async)] // This method doesn't need to be async but if its not the FFI layer panics // with "there is no no reactor running, must be called from the context @@ -84,12 +96,22 @@ impl SpaceService { } } +/// The `SpaceRoomList`represents a paginated list of direct rooms +/// that belong to a particular space. +/// +/// It can be used to paginate through the list (and have live updates on the +/// pagination state) as well as subscribe to changes as rooms are joined or +/// left. +/// +/// The `SpaceRoomList` also automatically subscribes to client room changes +/// and updates the list accordingly as rooms are joined or left. #[derive(uniffi::Object)] pub struct SpaceRoomList { inner: UISpaceRoomList, } impl SpaceRoomList { + /// Creates a new `SpaceRoomList` for the underlying UI crate room list. fn new(inner: UISpaceRoomList) -> Self { Self { inner } } @@ -97,10 +119,12 @@ impl SpaceRoomList { #[matrix_sdk_ffi_macros::export] impl SpaceRoomList { + /// Returns if the room list is currently paginating or not. pub fn pagination_state(&self) -> SpaceRoomListPaginationState { self.inner.pagination_state() } + /// Subscribe to pagination updates. pub fn subscribe_to_pagination_state_updates( &self, listener: Box, @@ -116,10 +140,12 @@ impl SpaceRoomList { }))) } + /// Return the current list of rooms. pub fn rooms(&self) -> Vec { self.inner.rooms().into_iter().map(Into::into).collect() } + /// Subscribes to room list updates. pub fn subscribe_to_room_update( &self, listener: Box, @@ -137,6 +163,8 @@ impl SpaceRoomList { }))) } + /// Ask the list to retrieve the next page if the end hasn't been reached + /// yet. Otherwise it no-ops. pub async fn paginate(&self) -> Result<(), ClientError> { self.inner.paginate().await.map_err(ClientError::from) } @@ -157,6 +185,8 @@ pub trait SpaceServiceJoinedSpacesListener: SendOutsideWasm + SyncOutsideWasm + fn on_update(&self, room_updates: Vec); } +/// Structure representing a room in a space and aggregated information +/// relevant to the UI layer. #[derive(uniffi::Record)] pub struct SpaceRoom { pub room_id: String, diff --git a/crates/matrix-sdk-ui/src/spaces/graph.rs b/crates/matrix-sdk-ui/src/spaces/graph.rs index a173ead23b7..dd739d24cf3 100644 --- a/crates/matrix-sdk-ui/src/spaces/graph.rs +++ b/crates/matrix-sdk-ui/src/spaces/graph.rs @@ -30,6 +30,9 @@ impl SpaceGraphNode { } #[derive(Debug)] +/// A graph structure representing a space hierarchy. Contains functionality +/// for mapping parent-child relationships between rooms, removing cycles and +/// retrieving top-level parents/roots. pub struct SpaceGraph { nodes: BTreeMap, } @@ -45,18 +48,25 @@ impl SpaceGraph { Self { nodes: BTreeMap::new() } } + /// Returns the root nodes of the graph, which are nodes without any + /// parents. pub fn root_nodes(&self) -> Vec<&OwnedRoomId> { self.nodes.values().filter(|node| node.parents.is_empty()).map(|node| &node.id).collect() } + /// Returns the children of a given node. If the node does not exist, it + /// returns an empty vector. pub fn children_of(&self, node_id: &OwnedRoomId) -> Vec<&OwnedRoomId> { self.nodes.get(node_id).map_or(vec![], |node| node.children.iter().collect()) } + /// Adds a node to the graph. If the node already exists, it does nothing. pub fn add_node(&mut self, node_id: OwnedRoomId) { self.nodes.entry(node_id.clone()).or_insert(SpaceGraphNode::new(node_id)); } + /// Adds a directed edge from `parent_id` to `child_id`, creating nodes if + /// they do not already exist in the graph. pub fn add_edge(&mut self, parent_id: OwnedRoomId, child_id: OwnedRoomId) { self.nodes.entry(parent_id.clone()).or_insert(SpaceGraphNode::new(parent_id.clone())); @@ -66,6 +76,9 @@ impl SpaceGraph { self.nodes.get_mut(&child_id).unwrap().parents.insert(parent_id); } + /// Removes cycles in the graph by performing a depth-first search (DFS) and + /// remembering the visited nodes. If a node is revisited while still in the + /// current path (i.e. it's on the stack), it indicates a cycle. pub fn remove_cycles(&mut self) { let mut visited = BTreeSet::new(); let mut stack = BTreeSet::new(); diff --git a/crates/matrix-sdk-ui/src/spaces/mod.rs b/crates/matrix-sdk-ui/src/spaces/mod.rs index 47f7e34c0b5..4c203c84a5a 100644 --- a/crates/matrix-sdk-ui/src/spaces/mod.rs +++ b/crates/matrix-sdk-ui/src/spaces/mod.rs @@ -14,7 +14,17 @@ //! High level interfaces for working with Spaces //! -//! See [`SpaceService`] for details. +//! The `SpaceService` is an UI oriented, high-level interface for working with +//! Matrix Spaces. It provides methods to retrieve joined spaces, subscribe +//! to updates, and navigate space hierarchies. +//! +//! It consists of 3 main components: +//! - `SpaceService`: The main service for managing spaces. It +//! - `SpaceGraph`: An utility that maps the `m.space.parent` and +//! `m.space.child` fields into a graph structure, removing cycles and +//! providing access to top level parents. +//! - `SpaceRoomList`: A component for retrieving a space's children rooms and +//! their details. use std::sync::Arc; @@ -39,6 +49,43 @@ pub mod graph; pub mod room; pub mod room_list; +/// The main entry point into the Spaces facilities. +/// +/// The spaces service is responsible for retrieving one's joined rooms, +/// building a graph out of their `m.space.parent` and `m.space.child` state +/// events, and providing access to the top-level spaces and their children. +/// +/// # Examples +/// +/// ```no_run +/// use futures_util::StreamExt; +/// use matrix_sdk::Client; +/// use matrix_sdk_ui::spaces::SpaceService; +/// use ruma::owned_room_id; +/// +/// # async { +/// let client: Client = todo!(); +/// let space_service = SpaceService::new(client.clone()); +/// +/// // Get a list of all the joined spaces +/// let joined_spaces = space_service.joined_spaces().await; +/// +/// // And subscribe to changes on them +/// // `initial_values` is equal to `joined_spaces` if nothing changed meanwhile +/// let (initial_values, stream) = space_service.subscribe_to_joined_spaces(); +/// +/// while let Some(diffs) = stream.next().await { +/// println!("Received joined spaces updates: {diffs:?}"); +/// } +/// +/// // Get a list of all the rooms in a particular space +/// let room_list = space_service +/// .space_room_list(owned_room_id!("!some_space:example.org")); +/// +/// // Which can be used to retrieve information about the children rooms +/// let children = room_list.rooms(); +/// # anyhow::Ok(()) }; +/// ``` pub struct SpaceService { client: Client, @@ -64,6 +111,8 @@ impl SpaceService { } } + /// Subscribes to updates on the joined spaces list. If space rooms are + /// joined or left, the stream will yield diffs that reflect the changes. pub fn subscribe_to_joined_spaces( &self, ) -> (Vector, VectorSubscriberBatchedStream) { @@ -96,6 +145,9 @@ impl SpaceService { self.joined_spaces.lock().subscribe().into_values_and_batched_stream() } + /// Returns a list of all the top-level joined spaces. It will eagerly + /// compute the latest version and also notify subscribers if there were + /// any changes. pub async fn joined_spaces(&self) -> Vec { let spaces = Self::joined_spaces_for(&self.client).await; @@ -104,6 +156,7 @@ impl SpaceService { spaces } + /// Returns a `SpaceRoomList` for the given space ID. pub fn space_room_list(&self, space_id: OwnedRoomId) -> SpaceRoomList { SpaceRoomList::new(self.client.clone(), space_id) } @@ -127,8 +180,11 @@ impl SpaceService { .filter_map(|room| room.is_space().then_some(room)) .collect::>(); + // Build a graph to hold the parent-child relations let mut graph = SpaceGraph::new(); + // Iterate over all joined spaces and populate the graph with edges based + // on `m.space.parent` and `m.space.child` state events. for space in joined_spaces.iter() { graph.add_node(space.room_id().to_owned()); @@ -167,6 +223,8 @@ impl SpaceService { } } + // Remove cycles from the graph. This is important because they are not + // enforced backend side. graph.remove_cycles(); let root_notes = graph.root_nodes(); diff --git a/crates/matrix-sdk-ui/src/spaces/room.rs b/crates/matrix-sdk-ui/src/spaces/room.rs index 592d9d0a050..d154581f0dd 100644 --- a/crates/matrix-sdk-ui/src/spaces/room.rs +++ b/crates/matrix-sdk-ui/src/spaces/room.rs @@ -19,6 +19,8 @@ use ruma::{ room::{JoinRuleSummary, RoomSummary, RoomType}, }; +/// Structure representing a room in a space and aggregated information +/// relevant to the UI layer. #[derive(Debug, Clone, PartialEq)] pub struct SpaceRoom { pub room_id: OwnedRoomId, diff --git a/crates/matrix-sdk-ui/src/spaces/room_list.rs b/crates/matrix-sdk-ui/src/spaces/room_list.rs index 32aad7d1b72..9a2479f0bad 100644 --- a/crates/matrix-sdk-ui/src/spaces/room_list.rs +++ b/crates/matrix-sdk-ui/src/spaces/room_list.rs @@ -33,6 +33,68 @@ pub enum SpaceRoomListPaginationState { Loading, } +/// The `SpaceRoomList`represents a paginated list of direct rooms +/// that belong to a particular space. +/// +/// It can be used to paginate through the list (and have live updates on the +/// pagination state) as well as subscribe to changes as rooms are joined or +/// left. +/// +/// The `SpaceRoomList` also automatically subscribes to client room changes +/// and updates the list accordingly as rooms are joined or left. +/// +/// # Examples +/// +/// ```no_run +/// use futures_util::StreamExt; +/// use matrix_sdk::Client; +/// use matrix_sdk_ui::spaces::{ +/// SpaceService, room_list::SpaceRoomListPaginationState, +/// }; +/// use ruma::owned_room_id; +/// +/// # async { +/// let client: Client = todo!(); +/// let space_service = SpaceService::new(client.clone()); +/// +/// // Get a list of all the rooms in a particular space +/// let room_list = space_service +/// .space_room_list(owned_room_id!("!some_space:example.org")); +/// +/// // Start off with an empty and idle list +/// room_list.rooms().is_empty(); +/// +/// assert_eq!( +/// room_list.pagination_state(), +/// SpaceRoomListPaginationState::Idle { end_reached: false } +/// ); +/// +/// // Subscribe to pagination state updates +/// let pagination_state_stream = +/// room_list.subscribe_to_pagination_state_updates(); +/// +/// // And to room list updates +/// let (_, room_stream) = room_list.subscribe_to_room_updates(); +/// +/// // spawn { +/// while let Some(pagination_state) = pagination_state_stream.next().await { +/// println!("Received pagination state update: {pagination_state:?}"); +/// } +/// // } +/// +/// // spawn { +/// while let Some(diffs) = room_stream.next().await { +/// println!("Received room list update: {diffs:?}"); +/// } +/// // } +/// +/// // Ask the room to load the next page +/// room_list.paginate().await.unwrap(); +/// +/// // And, if successful, rooms are available +/// let rooms = room_list.rooms(); +/// # anyhow::Ok(()) }; +/// ``` pub struct SpaceRoomList { client: Client, @@ -53,6 +115,7 @@ impl Drop for SpaceRoomList { } } impl SpaceRoomList { + /// Creates a new `SpaceRoomList` for the given space identifier. pub fn new(client: Client, parent_space_id: OwnedRoomId) -> Self { let rooms = Arc::new(Mutex::new(ObservableVector::::new())); @@ -105,26 +168,32 @@ impl SpaceRoomList { } } + /// Returns if the room list is currently paginating or not. pub fn pagination_state(&self) -> SpaceRoomListPaginationState { self.pagination_state.get() } + /// Subscribe to pagination updates. pub fn subscribe_to_pagination_state_updates( &self, ) -> Subscriber { self.pagination_state.subscribe() } + /// Return the current list of rooms. pub fn rooms(&self) -> Vec { self.rooms.lock().iter().cloned().collect_vec() } + /// Subscribes to room list updates. pub fn subscribe_to_room_updates( &self, ) -> (Vector, VectorSubscriberBatchedStream) { self.rooms.lock().subscribe().into_values_and_batched_stream() } + /// Ask the list to retrieve the next page if the end hasn't been reached + /// yet. Otherwise it no-ops. pub async fn paginate(&self) -> Result<(), Error> { match *self.pagination_state.read() { SpaceRoomListPaginationState::Idle { end_reached } if end_reached => { From b0d783157bf9b3657d6bab480576ba7f61d45932 Mon Sep 17 00:00:00 2001 From: Stefan Ceriu Date: Wed, 20 Aug 2025 14:13:02 +0300 Subject: [PATCH 22/35] chore(spaces): switch from manually dropping `JoinHandle`s to `AbortOnDrop` --- crates/matrix-sdk-ui/src/spaces/mod.rs | 20 +++++++------------- crates/matrix-sdk-ui/src/spaces/room_list.rs | 13 ++++--------- 2 files changed, 11 insertions(+), 22 deletions(-) diff --git a/crates/matrix-sdk-ui/src/spaces/mod.rs b/crates/matrix-sdk-ui/src/spaces/mod.rs index 4c203c84a5a..9f0d6a2c7fe 100644 --- a/crates/matrix-sdk-ui/src/spaces/mod.rs +++ b/crates/matrix-sdk-ui/src/spaces/mod.rs @@ -31,8 +31,10 @@ use std::sync::Arc; use eyeball_im::{ObservableVector, VectorSubscriberBatchedStream}; use futures_util::pin_mut; use imbl::Vector; -use matrix_sdk::{Client, deserialized_responses::SyncOrStrippedState, locks::Mutex}; -use matrix_sdk_common::executor::{JoinHandle, spawn}; +use matrix_sdk::{ + Client, deserialized_responses::SyncOrStrippedState, executor::AbortOnDrop, locks::Mutex, +}; +use matrix_sdk_common::executor::spawn; use ruma::{ OwnedRoomId, events::{ @@ -91,15 +93,7 @@ pub struct SpaceService { joined_spaces: Arc>>, - room_update_handle: Mutex>>, -} - -impl Drop for SpaceService { - fn drop(&mut self) { - if let Some(handle) = &*self.room_update_handle.lock() { - handle.abort(); - } - } + room_update_handle: Mutex>>, } impl SpaceService { @@ -121,7 +115,7 @@ impl SpaceService { let joined_spaces = Arc::clone(&self.joined_spaces); let all_room_updates_receiver = self.client.subscribe_to_all_room_updates(); - *self.room_update_handle.lock() = Some(spawn(async move { + *self.room_update_handle.lock() = Some(AbortOnDrop::new(spawn(async move { pin_mut!(all_room_updates_receiver); loop { @@ -139,7 +133,7 @@ impl SpaceService { } } } - })); + }))); } self.joined_spaces.lock().subscribe().into_values_and_batched_stream() diff --git a/crates/matrix-sdk-ui/src/spaces/room_list.rs b/crates/matrix-sdk-ui/src/spaces/room_list.rs index 9a2479f0bad..692898e583d 100644 --- a/crates/matrix-sdk-ui/src/spaces/room_list.rs +++ b/crates/matrix-sdk-ui/src/spaces/room_list.rs @@ -19,8 +19,8 @@ use eyeball_im::{ObservableVector, VectorSubscriberBatchedStream}; use futures_util::pin_mut; use imbl::Vector; use itertools::Itertools; -use matrix_sdk::{Client, Error, locks::Mutex, paginators::PaginationToken}; -use matrix_sdk_common::executor::{JoinHandle, spawn}; +use matrix_sdk::{Client, Error, executor::AbortOnDrop, locks::Mutex, paginators::PaginationToken}; +use matrix_sdk_common::executor::spawn; use ruma::{OwnedRoomId, api::client::space::get_hierarchy, uint}; use tracing::error; @@ -106,14 +106,9 @@ pub struct SpaceRoomList { rooms: Arc>>, - room_update_handle: JoinHandle<()>, + _room_update_handle: AbortOnDrop<()>, } -impl Drop for SpaceRoomList { - fn drop(&mut self) { - self.room_update_handle.abort(); - } -} impl SpaceRoomList { /// Creates a new `SpaceRoomList` for the given space identifier. pub fn new(client: Client, parent_space_id: OwnedRoomId) -> Self { @@ -164,7 +159,7 @@ impl SpaceRoomList { end_reached: false, }), rooms, - room_update_handle: handle, + _room_update_handle: AbortOnDrop::new(handle), } } From b6b4611e7d69cd668b7ea54bfc343056911d9432 Mon Sep 17 00:00:00 2001 From: Stefan Ceriu Date: Wed, 20 Aug 2025 15:13:49 +0300 Subject: [PATCH 23/35] fix(spaces): compute the initial `joined_spaces` value when first setting up a subscription - fixes values being reported only after the first sync update --- bindings/matrix-sdk-ffi/src/spaces.rs | 7 +----- crates/matrix-sdk-ui/src/spaces/mod.rs | 30 +++++++++++++++----------- 2 files changed, 18 insertions(+), 19 deletions(-) diff --git a/bindings/matrix-sdk-ffi/src/spaces.rs b/bindings/matrix-sdk-ffi/src/spaces.rs index 41712ee11e4..18e962094f0 100644 --- a/bindings/matrix-sdk-ffi/src/spaces.rs +++ b/bindings/matrix-sdk-ffi/src/spaces.rs @@ -59,16 +59,11 @@ impl SpaceService { /// Subscribes to updates on the joined spaces list. If space rooms are /// joined or left, the stream will yield diffs that reflect the changes. - #[allow(clippy::unused_async)] - // This method doesn't need to be async but if its not the FFI layer panics - // with "there is no no reactor running, must be called from the context - // of a Tokio 1.x runtime" error because the underlying method spawns an - // async task. pub async fn subscribe_to_joined_spaces( &self, listener: Box, ) -> Arc { - let (initial_values, mut stream) = self.inner.subscribe_to_joined_spaces(); + let (initial_values, mut stream) = self.inner.subscribe_to_joined_spaces().await; listener.on_update(vec![SpaceListUpdate::Reset { values: initial_values.into_iter().map(Into::into).collect(), diff --git a/crates/matrix-sdk-ui/src/spaces/mod.rs b/crates/matrix-sdk-ui/src/spaces/mod.rs index 9f0d6a2c7fe..fb2c212e67a 100644 --- a/crates/matrix-sdk-ui/src/spaces/mod.rs +++ b/crates/matrix-sdk-ui/src/spaces/mod.rs @@ -74,7 +74,8 @@ pub mod room_list; /// /// // And subscribe to changes on them /// // `initial_values` is equal to `joined_spaces` if nothing changed meanwhile -/// let (initial_values, stream) = space_service.subscribe_to_joined_spaces(); +/// let (initial_values, stream) = +/// space_service.subscribe_to_joined_spaces().await; /// /// while let Some(diffs) = stream.next().await { /// println!("Received joined spaces updates: {diffs:?}"); @@ -107,7 +108,7 @@ impl SpaceService { /// Subscribes to updates on the joined spaces list. If space rooms are /// joined or left, the stream will yield diffs that reflect the changes. - pub fn subscribe_to_joined_spaces( + pub async fn subscribe_to_joined_spaces( &self, ) -> (Vector, VectorSubscriberBatchedStream) { if self.room_update_handle.lock().is_none() { @@ -134,6 +135,10 @@ impl SpaceService { } } }))); + + // Make sure to also update the currently joined spaces for the initial values. + let spaces = Self::joined_spaces_for(&self.client).await; + Self::update_joined_spaces_if_needed(Vector::from(spaces), &self.joined_spaces); } self.joined_spaces.lock().subscribe().into_values_and_batched_stream() @@ -394,25 +399,24 @@ mod tests { let space_service = SpaceService::new(client.clone()); - let (_, joined_spaces_subscriber) = space_service.subscribe_to_joined_spaces(); + let (initial_values, joined_spaces_subscriber) = + space_service.subscribe_to_joined_spaces().await; pin_mut!(joined_spaces_subscriber); assert_pending!(joined_spaces_subscriber); + assert_eq!( + initial_values, + vec![SpaceRoom::new_from_known(client.get_room(first_space_id).unwrap(), 0)].into() + ); + assert_eq!( space_service.joined_spaces().await, vec![SpaceRoom::new_from_known(client.get_room(first_space_id).unwrap(), 0)] ); - assert_next_eq!( - joined_spaces_subscriber, - vec![VectorDiff::Append { - values: vec![SpaceRoom::new_from_known( - client.get_room(first_space_id).unwrap(), - 0 - )] - .into() - }] - ); + // And the stream is still pending as the initial values were + // already set. + assert_pending!(joined_spaces_subscriber); // Join the second space From a6be4ae38cce6dea598388b1d3c56167c179b52d Mon Sep 17 00:00:00 2001 From: Stefan Ceriu Date: Thu, 21 Aug 2025 10:09:17 +0300 Subject: [PATCH 24/35] chore(spaces): use `Client::joined_space_rooms` within the space service to reduce the number of iterations required --- crates/matrix-sdk-ui/src/spaces/mod.rs | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/crates/matrix-sdk-ui/src/spaces/mod.rs b/crates/matrix-sdk-ui/src/spaces/mod.rs index fb2c212e67a..7d5710f4abb 100644 --- a/crates/matrix-sdk-ui/src/spaces/mod.rs +++ b/crates/matrix-sdk-ui/src/spaces/mod.rs @@ -173,11 +173,7 @@ impl SpaceService { } async fn joined_spaces_for(client: &Client) -> Vec { - let joined_spaces = client - .joined_rooms() - .into_iter() - .filter_map(|room| room.is_space().then_some(room)) - .collect::>(); + let joined_spaces = client.joined_space_rooms(); // Build a graph to hold the parent-child relations let mut graph = SpaceGraph::new(); From 46826686532b9c2037999d7d15a7248a2ae1f9f4 Mon Sep 17 00:00:00 2001 From: Stefan Ceriu Date: Fri, 29 Aug 2025 13:32:00 +0300 Subject: [PATCH 25/35] chore(spaces): various documentation fixes --- bindings/matrix-sdk-ffi/src/spaces.rs | 14 ++++++++++++++ crates/matrix-sdk-ui/src/spaces/graph.rs | 5 +++-- crates/matrix-sdk-ui/src/spaces/mod.rs | 4 +++- crates/matrix-sdk-ui/src/spaces/room.rs | 18 +++++++++++++++++- crates/matrix-sdk-ui/src/spaces/room_list.rs | 8 +++----- 5 files changed, 40 insertions(+), 9 deletions(-) diff --git a/bindings/matrix-sdk-ffi/src/spaces.rs b/bindings/matrix-sdk-ffi/src/spaces.rs index 18e962094f0..e03e6851760 100644 --- a/bindings/matrix-sdk-ffi/src/spaces.rs +++ b/bindings/matrix-sdk-ffi/src/spaces.rs @@ -43,6 +43,7 @@ pub struct SpaceService { } impl SpaceService { + /// Creates a new `SpaceService` instance. pub(crate) fn new(inner: UISpaceService) -> Self { Self { inner } } @@ -184,19 +185,32 @@ pub trait SpaceServiceJoinedSpacesListener: SendOutsideWasm + SyncOutsideWasm + /// relevant to the UI layer. #[derive(uniffi::Record)] pub struct SpaceRoom { + /// The ID of the room. pub room_id: String, + /// The canonical alias of the room, if any. pub canonical_alias: Option, + /// The name of the room, if any. pub name: Option, + /// The topic of the room, if any. pub topic: Option, + /// The URL for the room's avatar, if one is set. pub avatar_url: Option, + /// The type of room from `m.room.create`, if any. pub room_type: RoomType, + /// The number of members joined to the room. pub num_joined_members: u64, + /// The join rule of the room. pub join_rule: Option, + /// Whether the room may be viewed by users without joining. pub world_readable: Option, + /// Whether guest users may join the room and participate in it. pub guest_can_join: bool, + /// The number of children room this has, if a space. pub children_count: u64, + /// Whether this room is joined, left etc. pub state: Option, + /// A list of room members considered to be heroes. pub heroes: Option>, } diff --git a/crates/matrix-sdk-ui/src/spaces/graph.rs b/crates/matrix-sdk-ui/src/spaces/graph.rs index dd739d24cf3..bb724005620 100644 --- a/crates/matrix-sdk-ui/src/spaces/graph.rs +++ b/crates/matrix-sdk-ui/src/spaces/graph.rs @@ -29,11 +29,11 @@ impl SpaceGraphNode { } } -#[derive(Debug)] /// A graph structure representing a space hierarchy. Contains functionality /// for mapping parent-child relationships between rooms, removing cycles and /// retrieving top-level parents/roots. -pub struct SpaceGraph { +#[derive(Debug)] +pub(super) struct SpaceGraph { nodes: BTreeMap, } @@ -44,6 +44,7 @@ impl Default for SpaceGraph { } impl SpaceGraph { + /// Creates a new empty space graph containing no nodes or edges. pub fn new() -> Self { Self { nodes: BTreeMap::new() } } diff --git a/crates/matrix-sdk-ui/src/spaces/mod.rs b/crates/matrix-sdk-ui/src/spaces/mod.rs index 7d5710f4abb..3a1ad789234 100644 --- a/crates/matrix-sdk-ui/src/spaces/mod.rs +++ b/crates/matrix-sdk-ui/src/spaces/mod.rs @@ -15,7 +15,8 @@ //! High level interfaces for working with Spaces //! //! The `SpaceService` is an UI oriented, high-level interface for working with -//! Matrix Spaces. It provides methods to retrieve joined spaces, subscribe +//! [Matrix Spaces](https://spec.matrix.org/latest/client-server-api/#spaces). +//! It provides methods to retrieve joined spaces, subscribe //! to updates, and navigate space hierarchies. //! //! It consists of 3 main components: @@ -98,6 +99,7 @@ pub struct SpaceService { } impl SpaceService { + /// Creates a new `SpaceService` instance. pub fn new(client: Client) -> Self { Self { client, diff --git a/crates/matrix-sdk-ui/src/spaces/room.rs b/crates/matrix-sdk-ui/src/spaces/room.rs index d154581f0dd..774b0d3ddbd 100644 --- a/crates/matrix-sdk-ui/src/spaces/room.rs +++ b/crates/matrix-sdk-ui/src/spaces/room.rs @@ -23,24 +23,39 @@ use ruma::{ /// relevant to the UI layer. #[derive(Debug, Clone, PartialEq)] pub struct SpaceRoom { + /// The ID of the room. pub room_id: OwnedRoomId, + /// The canonical alias of the room, if any. pub canonical_alias: Option, + /// The name of the room, if any. pub name: Option, + /// The topic of the room, if any. pub topic: Option, + /// The URL for the room's avatar, if one is set. pub avatar_url: Option, + /// The type of room from `m.room.create`, if any. pub room_type: Option, + /// The number of members joined to the room. pub num_joined_members: u64, + /// The join rule of the room. pub join_rule: Option, + /// Whether the room may be viewed by users without joining. pub world_readable: Option, + /// Whether guest users may join the room and participate in it. pub guest_can_join: bool, + /// The number of children room this has, if a space. pub children_count: u64, + /// Whether this room is joined, left etc. pub state: Option, + /// A list of room members considered to be heroes. pub heroes: Option>, } impl SpaceRoom { - pub fn new_from_summary( + /// Build a `SpaceRoom` from a `RoomSummary` received from the /hierarchy + /// endpoint. + pub(crate) fn new_from_summary( summary: &RoomSummary, known_room: Option, children_count: u64, @@ -62,6 +77,7 @@ impl SpaceRoom { } } + /// Build a `SpaceRoom` from a room already known to this client. pub fn new_from_known(known_room: Room, children_count: u64) -> Self { let room_info = known_room.clone_info(); diff --git a/crates/matrix-sdk-ui/src/spaces/room_list.rs b/crates/matrix-sdk-ui/src/spaces/room_list.rs index 692898e583d..53bebd2368d 100644 --- a/crates/matrix-sdk-ui/src/spaces/room_list.rs +++ b/crates/matrix-sdk-ui/src/spaces/room_list.rs @@ -54,7 +54,7 @@ pub enum SpaceRoomListPaginationState { /// use ruma::owned_room_id; /// /// # async { -/// let client: Client = todo!(); +/// # let client: Client = todo!(); /// let space_service = SpaceService::new(client.clone()); /// /// // Get a list of all the rooms in a particular space @@ -76,17 +76,15 @@ pub enum SpaceRoomListPaginationState { /// // And to room list updates /// let (_, room_stream) = room_list.subscribe_to_room_updates(); /// -/// // spawn { +/// // Run this in a background task so it doesn't block /// while let Some(pagination_state) = pagination_state_stream.next().await { /// println!("Received pagination state update: {pagination_state:?}"); /// } -/// // } /// -/// // spawn { +/// // Run this in a background task so it doesn't block /// while let Some(diffs) = room_stream.next().await { /// println!("Received room list update: {diffs:?}"); /// } -/// // } /// /// // Ask the room to load the next page /// room_list.paginate().await.unwrap(); From 90f323d8af99162956409b76a21b67345c9fb8ca Mon Sep 17 00:00:00 2001 From: Stefan Ceriu Date: Fri, 29 Aug 2025 13:33:12 +0300 Subject: [PATCH 26/35] chore(spaces): simplify the `SpaceGraph` interfaces --- crates/matrix-sdk-ui/src/spaces/graph.rs | 12 ++++++------ crates/matrix-sdk-ui/src/spaces/room.rs | 2 +- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/crates/matrix-sdk-ui/src/spaces/graph.rs b/crates/matrix-sdk-ui/src/spaces/graph.rs index bb724005620..b8c0a1b22a6 100644 --- a/crates/matrix-sdk-ui/src/spaces/graph.rs +++ b/crates/matrix-sdk-ui/src/spaces/graph.rs @@ -45,30 +45,30 @@ impl Default for SpaceGraph { impl SpaceGraph { /// Creates a new empty space graph containing no nodes or edges. - pub fn new() -> Self { + pub(super) fn new() -> Self { Self { nodes: BTreeMap::new() } } /// Returns the root nodes of the graph, which are nodes without any /// parents. - pub fn root_nodes(&self) -> Vec<&OwnedRoomId> { + pub(super) fn root_nodes(&self) -> Vec<&OwnedRoomId> { self.nodes.values().filter(|node| node.parents.is_empty()).map(|node| &node.id).collect() } /// Returns the children of a given node. If the node does not exist, it /// returns an empty vector. - pub fn children_of(&self, node_id: &OwnedRoomId) -> Vec<&OwnedRoomId> { + pub(super) fn children_of(&self, node_id: &OwnedRoomId) -> Vec<&OwnedRoomId> { self.nodes.get(node_id).map_or(vec![], |node| node.children.iter().collect()) } /// Adds a node to the graph. If the node already exists, it does nothing. - pub fn add_node(&mut self, node_id: OwnedRoomId) { + pub(super) fn add_node(&mut self, node_id: OwnedRoomId) { self.nodes.entry(node_id.clone()).or_insert(SpaceGraphNode::new(node_id)); } /// Adds a directed edge from `parent_id` to `child_id`, creating nodes if /// they do not already exist in the graph. - pub fn add_edge(&mut self, parent_id: OwnedRoomId, child_id: OwnedRoomId) { + pub(super) fn add_edge(&mut self, parent_id: OwnedRoomId, child_id: OwnedRoomId) { self.nodes.entry(parent_id.clone()).or_insert(SpaceGraphNode::new(parent_id.clone())); self.nodes.entry(child_id.clone()).or_insert(SpaceGraphNode::new(child_id.clone())); @@ -80,7 +80,7 @@ impl SpaceGraph { /// Removes cycles in the graph by performing a depth-first search (DFS) and /// remembering the visited nodes. If a node is revisited while still in the /// current path (i.e. it's on the stack), it indicates a cycle. - pub fn remove_cycles(&mut self) { + pub(super) fn remove_cycles(&mut self) { let mut visited = BTreeSet::new(); let mut stack = BTreeSet::new(); diff --git a/crates/matrix-sdk-ui/src/spaces/room.rs b/crates/matrix-sdk-ui/src/spaces/room.rs index 774b0d3ddbd..d44063535a1 100644 --- a/crates/matrix-sdk-ui/src/spaces/room.rs +++ b/crates/matrix-sdk-ui/src/spaces/room.rs @@ -78,7 +78,7 @@ impl SpaceRoom { } /// Build a `SpaceRoom` from a room already known to this client. - pub fn new_from_known(known_room: Room, children_count: u64) -> Self { + pub(crate) fn new_from_known(known_room: Room, children_count: u64) -> Self { let room_info = known_room.clone_info(); Self { From 22e23a276dea1189c9de7204ab7d8683c7f9139f Mon Sep 17 00:00:00 2001 From: Stefan Ceriu Date: Fri, 29 Aug 2025 13:45:11 +0300 Subject: [PATCH 27/35] chore(spaces): switch some `flat_map`s to `filter_map` --- crates/matrix-sdk-ui/src/spaces/mod.rs | 6 +++--- crates/matrix-sdk-ui/src/spaces/room_list.rs | 2 +- crates/matrix-sdk/src/room/mod.rs | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/crates/matrix-sdk-ui/src/spaces/mod.rs b/crates/matrix-sdk-ui/src/spaces/mod.rs index 3a1ad789234..d6b23240d6e 100644 --- a/crates/matrix-sdk-ui/src/spaces/mod.rs +++ b/crates/matrix-sdk-ui/src/spaces/mod.rs @@ -67,7 +67,7 @@ pub mod room_list; /// use ruma::owned_room_id; /// /// # async { -/// let client: Client = todo!(); +/// # let client: Client = todo!(); /// let space_service = SpaceService::new(client.clone()); /// /// // Get a list of all the joined spaces @@ -204,7 +204,7 @@ impl SpaceService { if let Ok(children) = space.get_state_events_static::().await { children.into_iter() - .flat_map(|child_event| match child_event.deserialize() { + .filter_map(|child_event| match child_event.deserialize() { Ok(SyncOrStrippedState::Sync(SyncStateEvent::Original(e))) => { Some(e.state_key) } @@ -228,7 +228,7 @@ impl SpaceService { joined_spaces .iter() - .flat_map(|room| { + .filter_map(|room| { let room_id = room.room_id().to_owned(); if root_notes.contains(&&room_id) { diff --git a/crates/matrix-sdk-ui/src/spaces/room_list.rs b/crates/matrix-sdk-ui/src/spaces/room_list.rs index 53bebd2368d..6c6cb0d8b6f 100644 --- a/crates/matrix-sdk-ui/src/spaces/room_list.rs +++ b/crates/matrix-sdk-ui/src/spaces/room_list.rs @@ -219,7 +219,7 @@ impl SpaceRoomList { result .rooms .iter() - .flat_map(|room| { + .filter_map(|room| { if room.summary.room_id == self.parent_space_id { None } else { diff --git a/crates/matrix-sdk/src/room/mod.rs b/crates/matrix-sdk/src/room/mod.rs index 4b3f1946efd..354275aac7e 100644 --- a/crates/matrix-sdk/src/room/mod.rs +++ b/crates/matrix-sdk/src/room/mod.rs @@ -1281,7 +1281,7 @@ impl Room { .await? .into_iter() // Extract state key (ie. the parent's id) and sender - .flat_map(|parent_event| match parent_event.deserialize() { + .filter_map(|parent_event| match parent_event.deserialize() { Ok(SyncOrStrippedState::Sync(SyncStateEvent::Original(e))) => { Some((e.state_key.to_owned(), e.sender)) } From af195ea70cffc00502494e6dc004bdbd8a960eac Mon Sep 17 00:00:00 2001 From: Stefan Ceriu Date: Fri, 29 Aug 2025 13:52:33 +0300 Subject: [PATCH 28/35] chore(spaces): fix typo --- crates/matrix-sdk-ui/src/spaces/mod.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/matrix-sdk-ui/src/spaces/mod.rs b/crates/matrix-sdk-ui/src/spaces/mod.rs index d6b23240d6e..d30055e56eb 100644 --- a/crates/matrix-sdk-ui/src/spaces/mod.rs +++ b/crates/matrix-sdk-ui/src/spaces/mod.rs @@ -224,14 +224,14 @@ impl SpaceService { // enforced backend side. graph.remove_cycles(); - let root_notes = graph.root_nodes(); + let root_nodes = graph.root_nodes(); joined_spaces .iter() .filter_map(|room| { let room_id = room.room_id().to_owned(); - if root_notes.contains(&&room_id) { + if root_nodes.contains(&&room_id) { Some(SpaceRoom::new_from_known( room.clone(), graph.children_of(&room_id).len() as u64, From aa27a6422898670db5b0b9a50f2a39e0afc1f9f3 Mon Sep 17 00:00:00 2001 From: Stefan Ceriu Date: Fri, 29 Aug 2025 13:57:14 +0300 Subject: [PATCH 29/35] chore(spaces): improve how space graph edge additions work --- crates/matrix-sdk-ui/src/spaces/graph.rs | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/crates/matrix-sdk-ui/src/spaces/graph.rs b/crates/matrix-sdk-ui/src/spaces/graph.rs index b8c0a1b22a6..1b788dd8b50 100644 --- a/crates/matrix-sdk-ui/src/spaces/graph.rs +++ b/crates/matrix-sdk-ui/src/spaces/graph.rs @@ -69,12 +69,13 @@ impl SpaceGraph { /// Adds a directed edge from `parent_id` to `child_id`, creating nodes if /// they do not already exist in the graph. pub(super) fn add_edge(&mut self, parent_id: OwnedRoomId, child_id: OwnedRoomId) { - self.nodes.entry(parent_id.clone()).or_insert(SpaceGraphNode::new(parent_id.clone())); + let parent_entry = + self.nodes.entry(parent_id.clone()).or_insert(SpaceGraphNode::new(parent_id.clone())); + parent_entry.children.insert(child_id.clone()); - self.nodes.entry(child_id.clone()).or_insert(SpaceGraphNode::new(child_id.clone())); - - self.nodes.get_mut(&parent_id).unwrap().children.insert(child_id.clone()); - self.nodes.get_mut(&child_id).unwrap().parents.insert(parent_id); + let child_entry = + self.nodes.entry(child_id.clone()).or_insert(SpaceGraphNode::new(child_id)); + child_entry.parents.insert(parent_id); } /// Removes cycles in the graph by performing a depth-first search (DFS) and From 7a11db448dfeb1df9eed6367794c1842fd619416 Mon Sep 17 00:00:00 2001 From: Stefan Ceriu Date: Fri, 29 Aug 2025 14:06:50 +0300 Subject: [PATCH 30/35] chore(spaces): simplify the `SpaceGraph` interfaces Fix graph reference crap --- crates/matrix-sdk-ui/src/spaces/graph.rs | 16 +++++++++++----- crates/matrix-sdk-ui/src/spaces/mod.rs | 6 +++--- 2 files changed, 14 insertions(+), 8 deletions(-) diff --git a/crates/matrix-sdk-ui/src/spaces/graph.rs b/crates/matrix-sdk-ui/src/spaces/graph.rs index 1b788dd8b50..04025bb80dd 100644 --- a/crates/matrix-sdk-ui/src/spaces/graph.rs +++ b/crates/matrix-sdk-ui/src/spaces/graph.rs @@ -14,7 +14,7 @@ use std::collections::{BTreeMap, BTreeSet}; -use ruma::OwnedRoomId; +use ruma::{OwnedRoomId, RoomId}; #[derive(Debug)] struct SpaceGraphNode { @@ -51,14 +51,20 @@ impl SpaceGraph { /// Returns the root nodes of the graph, which are nodes without any /// parents. - pub(super) fn root_nodes(&self) -> Vec<&OwnedRoomId> { - self.nodes.values().filter(|node| node.parents.is_empty()).map(|node| &node.id).collect() + pub(super) fn root_nodes(&self) -> Vec<&RoomId> { + self.nodes + .values() + .filter(|node| node.parents.is_empty()) + .map(|node| node.id.as_ref()) + .collect() } /// Returns the children of a given node. If the node does not exist, it /// returns an empty vector. - pub(super) fn children_of(&self, node_id: &OwnedRoomId) -> Vec<&OwnedRoomId> { - self.nodes.get(node_id).map_or(vec![], |node| node.children.iter().collect()) + pub(super) fn children_of(&self, node_id: &RoomId) -> Vec<&RoomId> { + self.nodes + .get(node_id) + .map_or(vec![], |node| node.children.iter().map(|id| id.as_ref()).collect()) } /// Adds a node to the graph. If the node already exists, it does nothing. diff --git a/crates/matrix-sdk-ui/src/spaces/mod.rs b/crates/matrix-sdk-ui/src/spaces/mod.rs index d30055e56eb..8ee8723797a 100644 --- a/crates/matrix-sdk-ui/src/spaces/mod.rs +++ b/crates/matrix-sdk-ui/src/spaces/mod.rs @@ -229,12 +229,12 @@ impl SpaceService { joined_spaces .iter() .filter_map(|room| { - let room_id = room.room_id().to_owned(); + let room_id = room.room_id(); - if root_nodes.contains(&&room_id) { + if root_nodes.contains(&room_id) { Some(SpaceRoom::new_from_known( room.clone(), - graph.children_of(&room_id).len() as u64, + graph.children_of(room_id).len() as u64, )) } else { None From 3c45e4b22bac8198c0cc9a44f2727319bd31658f Mon Sep 17 00:00:00 2001 From: Stefan Ceriu Date: Fri, 29 Aug 2025 14:14:57 +0300 Subject: [PATCH 31/35] chore(spaces): cleanup clone names in room updates listener --- crates/matrix-sdk-ui/src/spaces/room_list.rs | 57 ++++++++++---------- 1 file changed, 30 insertions(+), 27 deletions(-) diff --git a/crates/matrix-sdk-ui/src/spaces/room_list.rs b/crates/matrix-sdk-ui/src/spaces/room_list.rs index 6c6cb0d8b6f..de4c23504f4 100644 --- a/crates/matrix-sdk-ui/src/spaces/room_list.rs +++ b/crates/matrix-sdk-ui/src/spaces/room_list.rs @@ -112,38 +112,41 @@ impl SpaceRoomList { pub fn new(client: Client, parent_space_id: OwnedRoomId) -> Self { let rooms = Arc::new(Mutex::new(ObservableVector::::new())); - let client_clone = client.clone(); - let rooms_clone = rooms.clone(); let all_room_updates_receiver = client.subscribe_to_all_room_updates(); - let handle = spawn(async move { - pin_mut!(all_room_updates_receiver); + let handle = spawn({ + let client = client.clone(); + let rooms = rooms.clone(); - loop { - match all_room_updates_receiver.recv().await { - Ok(updates) => { - if updates.is_empty() { - continue; - } + async move { + pin_mut!(all_room_updates_receiver); - let mut mutable_rooms = rooms_clone.lock(); - - updates.iter_all_room_ids().for_each(|updated_room_id| { - if let Some((position, room)) = mutable_rooms - .clone() - .iter() - .find_position(|room| &room.room_id == updated_room_id) - && let Some(update_room) = client_clone.get_room(updated_room_id) - { - mutable_rooms.set( - position, - SpaceRoom::new_from_known(update_room, room.children_count), - ); + loop { + match all_room_updates_receiver.recv().await { + Ok(updates) => { + if updates.is_empty() { + continue; } - }) - } - Err(err) => { - error!("error when listening to room updates: {err}"); + + let mut mutable_rooms = rooms.lock(); + + updates.iter_all_room_ids().for_each(|updated_room_id| { + if let Some((position, room)) = mutable_rooms + .clone() + .iter() + .find_position(|room| &room.room_id == updated_room_id) + && let Some(update_room) = client.get_room(updated_room_id) + { + mutable_rooms.set( + position, + SpaceRoom::new_from_known(update_room, room.children_count), + ); + } + }) + } + Err(err) => { + error!("error when listening to room updates: {err}"); + } } } } From 6515acecc7bfbca73129d321491df0daaff395b6 Mon Sep 17 00:00:00 2001 From: Stefan Ceriu Date: Fri, 29 Aug 2025 14:24:04 +0300 Subject: [PATCH 32/35] fix(spaces): mitigate eventual race conditions when updating the pagination state --- crates/matrix-sdk-ui/src/spaces/room_list.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/matrix-sdk-ui/src/spaces/room_list.rs b/crates/matrix-sdk-ui/src/spaces/room_list.rs index de4c23504f4..1b7cea04d44 100644 --- a/crates/matrix-sdk-ui/src/spaces/room_list.rs +++ b/crates/matrix-sdk-ui/src/spaces/room_list.rs @@ -201,7 +201,7 @@ impl SpaceRoomList { _ => {} } - self.pagination_state.set(SpaceRoomListPaginationState::Loading); + self.pagination_state.set_if_not_eq(SpaceRoomListPaginationState::Loading); let mut request = get_hierarchy::v1::Request::new(self.parent_space_id.clone()); request.max_depth = Some(uint!(1)); // We only want the immediate children of the space From 243e51a978c3b7f2f97a9a70368327ab9d9a3430 Mon Sep 17 00:00:00 2001 From: Stefan Ceriu Date: Fri, 29 Aug 2025 14:35:53 +0300 Subject: [PATCH 33/35] chore(spaces): acquire and retain an async lock on the pagination token for the duration of the request to futher prevent inconsistencies --- crates/matrix-sdk-ui/src/spaces/room_list.rs | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/crates/matrix-sdk-ui/src/spaces/room_list.rs b/crates/matrix-sdk-ui/src/spaces/room_list.rs index 1b7cea04d44..112cba36832 100644 --- a/crates/matrix-sdk-ui/src/spaces/room_list.rs +++ b/crates/matrix-sdk-ui/src/spaces/room_list.rs @@ -22,6 +22,7 @@ use itertools::Itertools; use matrix_sdk::{Client, Error, executor::AbortOnDrop, locks::Mutex, paginators::PaginationToken}; use matrix_sdk_common::executor::spawn; use ruma::{OwnedRoomId, api::client::space::get_hierarchy, uint}; +use tokio::sync::Mutex as AsyncMutex; use tracing::error; use crate::spaces::SpaceRoom; @@ -98,7 +99,7 @@ pub struct SpaceRoomList { parent_space_id: OwnedRoomId, - token: Mutex, + token: AsyncMutex, pagination_state: SharedObservable, @@ -155,7 +156,7 @@ impl SpaceRoomList { Self { client, parent_space_id, - token: Mutex::new(None.into()), + token: AsyncMutex::new(None.into()), pagination_state: SharedObservable::new(SpaceRoomListPaginationState::Idle { end_reached: false, }), @@ -206,14 +207,15 @@ impl SpaceRoomList { let mut request = get_hierarchy::v1::Request::new(self.parent_space_id.clone()); request.max_depth = Some(uint!(1)); // We only want the immediate children of the space - if let PaginationToken::HasMore(ref token) = *self.token.lock() { + let mut pagination_token = self.token.lock().await; + + if let PaginationToken::HasMore(ref token) = *pagination_token { request.from = Some(token.clone()); } match self.client.send(request).await { Ok(result) => { - let mut token = self.token.lock(); - *token = match &result.next_batch { + *pagination_token = match &result.next_batch { Some(val) => PaginationToken::HasMore(val.clone()), None => PaginationToken::HitEnd, }; From 74fd7544c42ba82fd1d48448f56f7f01d8c30269 Mon Sep 17 00:00:00 2001 From: Stefan Ceriu Date: Fri, 29 Aug 2025 15:08:22 +0300 Subject: [PATCH 34/35] fix(spaces): address potential race when setting up the room updates listener --- crates/matrix-sdk-ui/src/spaces/mod.rs | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/crates/matrix-sdk-ui/src/spaces/mod.rs b/crates/matrix-sdk-ui/src/spaces/mod.rs index 8ee8723797a..7f84363b2e1 100644 --- a/crates/matrix-sdk-ui/src/spaces/mod.rs +++ b/crates/matrix-sdk-ui/src/spaces/mod.rs @@ -43,6 +43,7 @@ use ruma::{ space::{child::SpaceChildEventContent, parent::SpaceParentEventContent}, }, }; +use tokio::sync::Mutex as AsyncMutex; use tracing::error; use crate::spaces::graph::SpaceGraph; @@ -95,7 +96,7 @@ pub struct SpaceService { joined_spaces: Arc>>, - room_update_handle: Mutex>>, + room_update_handle: AsyncMutex>>, } impl SpaceService { @@ -104,7 +105,7 @@ impl SpaceService { Self { client, joined_spaces: Arc::new(Mutex::new(ObservableVector::new())), - room_update_handle: Mutex::new(None), + room_update_handle: AsyncMutex::new(None), } } @@ -113,12 +114,14 @@ impl SpaceService { pub async fn subscribe_to_joined_spaces( &self, ) -> (Vector, VectorSubscriberBatchedStream) { - if self.room_update_handle.lock().is_none() { + let mut room_update_handle = self.room_update_handle.lock().await; + + if room_update_handle.is_none() { let client = self.client.clone(); let joined_spaces = Arc::clone(&self.joined_spaces); let all_room_updates_receiver = self.client.subscribe_to_all_room_updates(); - *self.room_update_handle.lock() = Some(AbortOnDrop::new(spawn(async move { + *room_update_handle = Some(AbortOnDrop::new(spawn(async move { pin_mut!(all_room_updates_receiver); loop { From 89f2483878a38b204b2a3ac957ccda658bf7d700 Mon Sep 17 00:00:00 2001 From: Stefan Ceriu Date: Fri, 29 Aug 2025 17:16:47 +0300 Subject: [PATCH 35/35] fix(spaces): mitigate eventual race conditions when updating the pagination state (part #2) --- crates/matrix-sdk-ui/src/spaces/room_list.rs | 24 ++++++++++++-------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/crates/matrix-sdk-ui/src/spaces/room_list.rs b/crates/matrix-sdk-ui/src/spaces/room_list.rs index 112cba36832..b5cf6d2d07b 100644 --- a/crates/matrix-sdk-ui/src/spaces/room_list.rs +++ b/crates/matrix-sdk-ui/src/spaces/room_list.rs @@ -14,7 +14,7 @@ use std::sync::Arc; -use eyeball::{SharedObservable, Subscriber}; +use eyeball::{ObservableWriteGuard, SharedObservable, Subscriber}; use eyeball_im::{ObservableVector, VectorSubscriberBatchedStream}; use futures_util::pin_mut; use imbl::Vector; @@ -192,17 +192,21 @@ impl SpaceRoomList { /// Ask the list to retrieve the next page if the end hasn't been reached /// yet. Otherwise it no-ops. pub async fn paginate(&self) -> Result<(), Error> { - match *self.pagination_state.read() { - SpaceRoomListPaginationState::Idle { end_reached } if end_reached => { - return Ok(()); - } - SpaceRoomListPaginationState::Loading => { - return Ok(()); + { + let mut pagination_state = self.pagination_state.write(); + + match *pagination_state { + SpaceRoomListPaginationState::Idle { end_reached } if end_reached => { + return Ok(()); + } + SpaceRoomListPaginationState::Loading => { + return Ok(()); + } + _ => {} } - _ => {} - } - self.pagination_state.set_if_not_eq(SpaceRoomListPaginationState::Loading); + ObservableWriteGuard::set(&mut pagination_state, SpaceRoomListPaginationState::Loading); + } let mut request = get_hierarchy::v1::Request::new(self.parent_space_id.clone()); request.max_depth = Some(uint!(1)); // We only want the immediate children of the space