From 74c2032974a65a1fc18103ca5b429f232b32bfdd Mon Sep 17 00:00:00 2001 From: Stefan Ceriu Date: Mon, 11 Aug 2025 12:15:48 +0300 Subject: [PATCH] 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 | 83 +++++++--- 2 files changed, 264 insertions(+), 19 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 000000000..1d703c42a --- /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 84e36eded..a20af9498 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 - } else { - false - }; - (room, ok) - }) - .filter(|(_, ok)| *ok) - .map(|(x, _)| SpaceServiceRoom::new_from_known(x)) - .collect() - .await; + let mut graph = SpaceServiceGraph::new(); - top_level_spaces + 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 { + None + } + }) + .collect() } }