From 8e0dba641d010375d760ca96c4c69d903dd5abaf Mon Sep 17 00:00:00 2001 From: Doug Date: Fri, 14 Nov 2025 15:53:25 +0000 Subject: [PATCH] spaces: Add methods to add/remove space children. --- bindings/matrix-sdk-ffi/src/spaces.rs | 22 ++ crates/matrix-sdk-ui/src/spaces/mod.rs | 237 +++++++++++++++++- crates/matrix-sdk/src/test_utils/mocks/mod.rs | 48 ++++ 3 files changed, 304 insertions(+), 3 deletions(-) diff --git a/bindings/matrix-sdk-ffi/src/spaces.rs b/bindings/matrix-sdk-ffi/src/spaces.rs index 391da7ab9..a383649b6 100644 --- a/bindings/matrix-sdk-ffi/src/spaces.rs +++ b/bindings/matrix-sdk-ffi/src/spaces.rs @@ -87,6 +87,28 @@ impl SpaceService { Ok(Arc::new(SpaceRoomList::new(self.inner.space_room_list(space_id).await))) } + pub async fn add_child_to_space( + &self, + child_id: String, + space_id: String, + ) -> Result<(), ClientError> { + let space_id = RoomId::parse(space_id)?; + let child_id = RoomId::parse(child_id)?; + + self.inner.add_child_to_space(child_id, space_id).await.map_err(ClientError::from) + } + + pub async fn remove_child_from_space( + &self, + child_id: String, + space_id: String, + ) -> Result<(), ClientError> { + let space_id = RoomId::parse(space_id)?; + let child_id = RoomId::parse(child_id)?; + + self.inner.remove_child_from_space(child_id, space_id).await.map_err(ClientError::from) + } + /// Start a space leave process returning a [`LeaveSpaceHandle`] from which /// rooms can be retrieved in reversed BFS order starting from the requested /// `space_id` graph node. If the room is unknown then an error will be diff --git a/crates/matrix-sdk-ui/src/spaces/mod.rs b/crates/matrix-sdk-ui/src/spaces/mod.rs index b3b67aec1..b23b5df43 100644 --- a/crates/matrix-sdk-ui/src/spaces/mod.rs +++ b/crates/matrix-sdk-ui/src/spaces/mod.rs @@ -40,13 +40,13 @@ use matrix_sdk_common::executor::spawn; use ruma::{ OwnedRoomId, RoomId, events::{ - self, SyncStateEvent, + self, StateEventType, SyncStateEvent, space::{child::SpaceChildEventContent, parent::SpaceParentEventContent}, }, }; use thiserror::Error; use tokio::sync::Mutex as AsyncMutex; -use tracing::error; +use tracing::{error, warn}; use crate::spaces::{graph::SpaceGraph, leave::LeaveSpaceHandle}; pub use crate::spaces::{room::SpaceRoom, room_list::SpaceRoomList}; @@ -59,10 +59,22 @@ pub mod room_list; /// Possible [`SpaceService`] errors. #[derive(Debug, Error)] pub enum Error { + /// The user ID was not available from the client. + #[error("User ID not available from client")] + UserIdNotFound, + /// The requested room was not found. #[error("Room `{0}` not found")] RoomNotFound(OwnedRoomId), + /// The space parent/child state was missing. + #[error("Missing `{0}` for `{1}`")] + MissingState(StateEventType, OwnedRoomId), + + /// Failed to set the expected m.space.parent or m.space.child state events. + #[error("Failed updating space parent/child relationship")] + UpdateRelationship(SDKError), + /// Failed to leave a space. #[error("Failed to leave space")] LeaveSpace(SDKError), @@ -204,6 +216,88 @@ impl SpaceService { SpaceRoomList::new(self.client.clone(), space_id).await } + pub async fn add_child_to_space( + &self, + child_id: OwnedRoomId, + space_id: OwnedRoomId, + ) -> Result<(), Error> { + let user_id = self.client.user_id().ok_or(Error::UserIdNotFound)?; + let space_room = + self.client.get_room(&space_id).ok_or(Error::RoomNotFound(space_id.to_owned()))?; + let child_room = + self.client.get_room(&child_id).ok_or(Error::RoomNotFound(child_id.to_owned()))?; + let child_power_levels = child_room + .power_levels() + .await + .map_err(|error| Error::UpdateRelationship(matrix_sdk::Error::from(error)))?; + + // Add the child to the space. + let child_route = child_room.route().await.map_err(Error::UpdateRelationship)?; + space_room + .send_state_event_for_key(&child_id, SpaceChildEventContent::new(child_route)) + .await + .map_err(Error::UpdateRelationship)?; + + // Add the space as parent of the child if allowed. + if child_power_levels.user_can_send_state(user_id, StateEventType::SpaceParent) { + let parent_route = space_room.route().await.map_err(Error::UpdateRelationship)?; + child_room + .send_state_event_for_key(&space_id, SpaceParentEventContent::new(parent_route)) + .await + .map_err(Error::UpdateRelationship)?; + } else { + warn!("The current user doesn't have permission to set the child's parent."); + } + + Ok(()) + } + + pub async fn remove_child_from_space( + &self, + child_id: OwnedRoomId, + space_id: OwnedRoomId, + ) -> Result<(), Error> { + let space_room = + self.client.get_room(&space_id).ok_or(Error::RoomNotFound(space_id.to_owned()))?; + let child_room = + self.client.get_room(&child_id).ok_or(Error::RoomNotFound(child_id.to_owned()))?; + + if space_room + .get_state_event_static_for_key::(&child_id) + .await + .is_err() + { + warn!("A space child event wasn't found on the parent, ignoring."); + return Ok(()); + } + // Redacting state is a "weird" thing to do, so send {} instead. + // https://github.com/matrix-org/matrix-spec/issues/2252 + // + // Specifically, "The redaction of the state doesn't participate in state + // resolution so behaves quite differently from e.g. sending an empty form of + // that state events". + space_room + .send_state_event_raw("m.space.child", child_id.as_str(), serde_json::json!({})) + .await + .map_err(Error::UpdateRelationship)?; + + if child_room + .get_state_event_static_for_key::(&space_id) + .await + .is_err() + { + warn!("A space parent event wasn't found on the child, ignoring."); + return Ok(()); + } + // Same as the comment above. + child_room + .send_state_event_raw("m.space.parent", space_id.as_str(), serde_json::json!({})) + .await + .map_err(Error::UpdateRelationship)?; + + Ok(()) + } + /// Start a space leave process returning a [`LeaveSpaceHandle`] from which /// rooms can be retrieved in reversed BFS order starting from the requested /// `space_id` graph node. If the room is unknown then an error will be @@ -337,6 +431,8 @@ impl SpaceService { #[cfg(test)] mod tests { + use std::collections::BTreeMap; + use assert_matches2::assert_let; use eyeball_im::VectorDiff; use futures_util::{StreamExt, pin_mut}; @@ -345,7 +441,7 @@ mod tests { JoinedRoomBuilder, LeftRoomBuilder, RoomAccountDataTestEvent, async_test, event_factory::EventFactory, }; - use ruma::{RoomVersionId, UserId, owned_room_id, room_id}; + use ruma::{RoomVersionId, UserId, event_id, owned_room_id, room_id, user_id}; use serde_json::json; use stream_assert::{assert_next_eq, assert_pending}; @@ -620,6 +716,118 @@ mod tests { ); } + #[async_test] + async fn test_add_child_to_space() { + // Given a space and child room where the user is admin of both. + 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 space_child_event_id = event_id!("$1"); + let space_parent_event_id = event_id!("$2"); + server.mock_set_space_child().ok(space_child_event_id.to_owned()).expect(1).mount().await; + server.mock_set_space_parent().ok(space_parent_event_id.to_owned()).expect(1).mount().await; + + let space_id = room_id!("!my_space:example.org"); + let child_id = room_id!("!my_child:example.org"); + + add_rooms_with_power_level( + vec![(space_id, 100), (child_id, 100)], + &client, + &server, + &factory, + user_id, + ) + .await; + + let space_service = SpaceService::new(client.clone()); + + // When adding the child to the space. + let result = + space_service.add_child_to_space(child_id.to_owned(), space_id.to_owned()).await; + + // Then both space child and parent events are set successfully. + assert!(result.is_ok()); + } + + #[async_test] + async fn test_add_child_to_space_without_space_admin() { + // Given a space and child room where the user is a regular member of both. + 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; + + server.mock_set_space_child().unauthorized().expect(1).mount().await; + server.mock_set_space_parent().unauthorized().expect(0).mount().await; + + let space_id = room_id!("!my_space:example.org"); + let child_id = room_id!("!my_child:example.org"); + + add_rooms_with_power_level( + vec![(space_id, 0), (child_id, 0)], + &client, + &server, + &factory, + user_id, + ) + .await; + + let space_service = SpaceService::new(client.clone()); + + // When adding the child to the space. + let result = + space_service.add_child_to_space(child_id.to_owned(), space_id.to_owned()).await; + + // Then the operation fails when trying to set the space child event and the + // parent event is not attempted. + assert!(result.is_err()); + } + + #[async_test] + async fn test_add_child_to_space_without_child_admin() { + // Given a space and child room where the user is admin of the space but not of + // the child. + 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 space_child_event_id = event_id!("$1"); + server.mock_set_space_child().ok(space_child_event_id.to_owned()).expect(1).mount().await; + server.mock_set_space_parent().unauthorized().expect(0).mount().await; + + let space_id = room_id!("!my_space:example.org"); + let child_id = room_id!("!my_child:example.org"); + + add_rooms_with_power_level( + vec![(space_id, 100), (child_id, 0)], + &client, + &server, + &factory, + user_id, + ) + .await; + + let space_service = SpaceService::new(client.clone()); + + // When adding the child to the space. + let result = + space_service.add_child_to_space(child_id.to_owned(), space_id.to_owned()).await; + + error!("result: {:?}", result); + // Then the operation succeeds in setting the space child event and the parent + // event is not attempted. + assert!(result.is_ok()); + } + async fn add_space_rooms_with( rooms: Vec<(&RoomId, Option<&str>)>, client: &Client, @@ -643,4 +851,27 @@ mod tests { server.sync_room(client, builder).await; } } + + async fn add_rooms_with_power_level( + rooms: Vec<(&RoomId, i32)>, + client: &Client, + server: &MatrixMockServer, + factory: &EventFactory, + user_id: &UserId, + ) { + for (room_id, power_level) in rooms { + let mut builder = JoinedRoomBuilder::new(room_id); + let mut power_levels = BTreeMap::from([(user_id.to_owned(), power_level.into())]); + + builder = builder + .add_state_event( + factory.create(user_id!("@creator:example.com"), RoomVersionId::V1), + ) + .add_state_event( + factory.power_levels(&mut power_levels).state_key("").sender(user_id), + ); + + server.sync_room(client, builder).await; + } + } } diff --git a/crates/matrix-sdk/src/test_utils/mocks/mod.rs b/crates/matrix-sdk/src/test_utils/mocks/mod.rs index b8065b28e..aa42fb9c3 100644 --- a/crates/matrix-sdk/src/test_utils/mocks/mod.rs +++ b/crates/matrix-sdk/src/test_utils/mocks/mod.rs @@ -1630,6 +1630,20 @@ impl MatrixMockServer { self.mock_endpoint(mock, GetHierarchyEndpoint).expect_default_access_token() } + /// Create a prebuilt mock for the endpoint used to set a space child. + pub fn mock_set_space_child(&self) -> MockEndpoint<'_, SetSpaceChildEndpoint> { + let mock = Mock::given(method("PUT")) + .and(path_regex(r"^/_matrix/client/v3/rooms/.*/state/m.space.child/.*?")); + self.mock_endpoint(mock, SetSpaceChildEndpoint).expect_default_access_token() + } + + /// Create a prebuilt mock for the endpoint used to set a space parent. + pub fn mock_set_space_parent(&self) -> MockEndpoint<'_, SetSpaceParentEndpoint> { + let mock = Mock::given(method("PUT")) + .and(path_regex(r"^/_matrix/client/v3/rooms/.*/state/m.space.parent")); + self.mock_endpoint(mock, SetSpaceParentEndpoint).expect_default_access_token() + } + /// Create a prebuilt mock for the endpoint used to get a profile field. pub fn mock_get_profile_field( &self, @@ -4680,6 +4694,40 @@ impl<'a> MockEndpoint<'a, GetHierarchyEndpoint> { } } +/// A prebuilt mock for `PUT +/// /_matrix/client/v3/rooms/{roomId}/state/m.space.child/{stateKey}` +pub struct SetSpaceChildEndpoint; + +impl<'a> MockEndpoint<'a, SetSpaceChildEndpoint> { + /// Returns a successful response with a given event id. + pub fn ok(self, event_id: OwnedEventId) -> MatrixMock<'a> { + self.ok_with_event_id(event_id) + } + + /// Returns an error response with a generic error code indicating the + /// client is not authorized to set space children. + pub fn unauthorized(self) -> MatrixMock<'a> { + self.respond_with(ResponseTemplate::new(400)) + } +} + +/// A prebuilt mock for `PUT +/// /_matrix/client/v3/rooms/{roomId}/state/m.space.parent/{stateKey}` +pub struct SetSpaceParentEndpoint; + +impl<'a> MockEndpoint<'a, SetSpaceParentEndpoint> { + /// Returns a successful response with a given event id. + pub fn ok(self, event_id: OwnedEventId) -> MatrixMock<'a> { + self.ok_with_event_id(event_id) + } + + /// Returns an error response with a generic error code indicating the + /// client is not authorized to set space parents. + pub fn unauthorized(self) -> MatrixMock<'a> { + self.respond_with(ResponseTemplate::new(400)) + } +} + /// A prebuilt mock for running simplified sliding sync. pub struct SlidingSyncEndpoint;