From 898f4a470c48e394171fafba0ca64e606bb4fc34 Mon Sep 17 00:00:00 2001 From: ember33 Date: Wed, 18 Mar 2026 11:06:58 +0100 Subject: [PATCH] refactor(spaces): extract constants, add forward index, reduce duplication Extract event type string literals as constants in space_roles.rs and replace all occurrences across service and admin code. Add a forward index (space_to_rooms) for O(1) child room lookups instead of scanning the reverse index. Introduce resolve_space! macro to deduplicate the repeated enabled-check + alias-resolve + space-type-guard pattern in all 9 admin command handlers. Flatten deeply nested if-let chains in append.rs using let-chains syntax. Co-Authored-By: Claude Opus 4.6 (1M context) --- src/admin/space/roles.rs | 131 +++++++++------------------ src/core/matrix/space_roles.rs | 9 ++ src/service/rooms/roles/mod.rs | 57 ++++++++---- src/service/rooms/timeline/append.rs | 50 ++++------ 4 files changed, 115 insertions(+), 132 deletions(-) diff --git a/src/admin/space/roles.rs b/src/admin/space/roles.rs index f5df1f9d..7f0206ff 100644 --- a/src/admin/space/roles.rs +++ b/src/admin/space/roles.rs @@ -2,7 +2,8 @@ use clap::Subcommand; use conduwuit::{Err, Result}; use conduwuit_core::matrix::space_roles::{ RoleDefinition, SpaceRoleMemberEventContent, SpaceRoleRoomEventContent, - SpaceRolesEventContent, + SpaceRolesEventContent, SPACE_ROLES_EVENT_TYPE, SPACE_ROLE_MEMBER_EVENT_TYPE, + SPACE_ROLE_ROOM_EVENT_TYPE, }; use ruma::{OwnedRoomId, OwnedRoomOrAliasId, OwnedUserId, events::StateEventType}; use serde_json::value::to_raw_value; @@ -24,6 +25,27 @@ macro_rules! require_enabled { }; } +macro_rules! resolve_space { + ($self:expr, $space:expr) => {{ + require_enabled!($self); + let space_id = $self.services.rooms.alias.resolve(&$space).await?; + if !matches!( + $self + .services + .rooms + .state_accessor + .get_room_type(&space_id) + .await, + Ok(ruma::room::RoomType::Space) + ) { + return $self + .write_str("Error: The specified room is not a Space.") + .await; + } + space_id + }}; +} + macro_rules! custom_state_pdu { ($event_type:expr, $state_key:expr, $content:expr) => { PduBuilder { @@ -97,15 +119,8 @@ pub enum SpaceRolesCommand { #[admin_command] async fn list(&self, space: OwnedRoomOrAliasId) -> Result { - require_enabled!(self); - let space_id = self.services.rooms.alias.resolve(&space).await?; - if !matches!( - self.services.rooms.state_accessor.get_room_type(&space_id).await, - Ok(ruma::room::RoomType::Space) - ) { - return self.write_str("Error: The specified room is not a Space.").await; - } - let roles_event_type = StateEventType::from("com.continuwuity.space.roles".to_owned()); + let space_id = resolve_space!(self, space); + let roles_event_type = StateEventType::from(SPACE_ROLES_EVENT_TYPE.to_owned()); let content: SpaceRolesEventContent = self .services @@ -140,15 +155,8 @@ async fn add( description: Option, power_level: Option, ) -> Result { - require_enabled!(self); - let space_id = self.services.rooms.alias.resolve(&space).await?; - if !matches!( - self.services.rooms.state_accessor.get_room_type(&space_id).await, - Ok(ruma::room::RoomType::Space) - ) { - return self.write_str("Error: The specified room is not a Space.").await; - } - let roles_event_type = StateEventType::from("com.continuwuity.space.roles".to_owned()); + let space_id = resolve_space!(self, space); + let roles_event_type = StateEventType::from(SPACE_ROLES_EVENT_TYPE.to_owned()); let mut content: SpaceRolesEventContent = self .services @@ -174,7 +182,7 @@ async fn add( .rooms .timeline .build_and_append_pdu( - custom_state_pdu!("com.continuwuity.space.roles", "", &content), + custom_state_pdu!(SPACE_ROLES_EVENT_TYPE, "", &content), server_user, Some(&space_id), &state_lock, @@ -187,15 +195,8 @@ async fn add( #[admin_command] async fn remove(&self, space: OwnedRoomOrAliasId, role_name: String) -> Result { - require_enabled!(self); - let space_id = self.services.rooms.alias.resolve(&space).await?; - if !matches!( - self.services.rooms.state_accessor.get_room_type(&space_id).await, - Ok(ruma::room::RoomType::Space) - ) { - return self.write_str("Error: The specified room is not a Space.").await; - } - let roles_event_type = StateEventType::from("com.continuwuity.space.roles".to_owned()); + let space_id = resolve_space!(self, space); + let roles_event_type = StateEventType::from(SPACE_ROLES_EVENT_TYPE.to_owned()); let mut content: SpaceRolesEventContent = self .services @@ -216,7 +217,7 @@ async fn remove(&self, space: OwnedRoomOrAliasId, role_name: String) -> Result { .rooms .timeline .build_and_append_pdu( - custom_state_pdu!("com.continuwuity.space.roles", "", &content), + custom_state_pdu!(SPACE_ROLES_EVENT_TYPE, "", &content), server_user, Some(&space_id), &state_lock, @@ -234,15 +235,8 @@ async fn assign( user_id: OwnedUserId, role_name: String, ) -> Result { - require_enabled!(self); - let space_id = self.services.rooms.alias.resolve(&space).await?; - if !matches!( - self.services.rooms.state_accessor.get_room_type(&space_id).await, - Ok(ruma::room::RoomType::Space) - ) { - return self.write_str("Error: The specified room is not a Space.").await; - } - let member_event_type = StateEventType::from("com.continuwuity.space.role.member".to_owned()); + let space_id = resolve_space!(self, space); + let member_event_type = StateEventType::from(SPACE_ROLE_MEMBER_EVENT_TYPE.to_owned()); let mut content: SpaceRoleMemberEventContent = self .services @@ -265,7 +259,7 @@ async fn assign( .rooms .timeline .build_and_append_pdu( - custom_state_pdu!("com.continuwuity.space.role.member", user_id.as_str(), &content), + custom_state_pdu!(SPACE_ROLE_MEMBER_EVENT_TYPE, user_id.as_str(), &content), server_user, Some(&space_id), &state_lock, @@ -285,15 +279,8 @@ async fn revoke( user_id: OwnedUserId, role_name: String, ) -> Result { - require_enabled!(self); - let space_id = self.services.rooms.alias.resolve(&space).await?; - if !matches!( - self.services.rooms.state_accessor.get_room_type(&space_id).await, - Ok(ruma::room::RoomType::Space) - ) { - return self.write_str("Error: The specified room is not a Space.").await; - } - let member_event_type = StateEventType::from("com.continuwuity.space.role.member".to_owned()); + let space_id = resolve_space!(self, space); + let member_event_type = StateEventType::from(SPACE_ROLE_MEMBER_EVENT_TYPE.to_owned()); let mut content: SpaceRoleMemberEventContent = self .services @@ -317,7 +304,7 @@ async fn revoke( .rooms .timeline .build_and_append_pdu( - custom_state_pdu!("com.continuwuity.space.role.member", user_id.as_str(), &content), + custom_state_pdu!(SPACE_ROLE_MEMBER_EVENT_TYPE, user_id.as_str(), &content), server_user, Some(&space_id), &state_lock, @@ -337,15 +324,8 @@ async fn require( room_id: OwnedRoomId, role_name: String, ) -> Result { - require_enabled!(self); - let space_id = self.services.rooms.alias.resolve(&space).await?; - if !matches!( - self.services.rooms.state_accessor.get_room_type(&space_id).await, - Ok(ruma::room::RoomType::Space) - ) { - return self.write_str("Error: The specified room is not a Space.").await; - } - let room_event_type = StateEventType::from("com.continuwuity.space.role.room".to_owned()); + let space_id = resolve_space!(self, space); + let room_event_type = StateEventType::from(SPACE_ROLE_ROOM_EVENT_TYPE.to_owned()); let mut content: SpaceRoleRoomEventContent = self .services @@ -368,7 +348,7 @@ async fn require( .rooms .timeline .build_and_append_pdu( - custom_state_pdu!("com.continuwuity.space.role.room", room_id.as_str(), &content), + custom_state_pdu!(SPACE_ROLE_ROOM_EVENT_TYPE, room_id.as_str(), &content), server_user, Some(&space_id), &state_lock, @@ -388,15 +368,8 @@ async fn unrequire( room_id: OwnedRoomId, role_name: String, ) -> Result { - require_enabled!(self); - let space_id = self.services.rooms.alias.resolve(&space).await?; - if !matches!( - self.services.rooms.state_accessor.get_room_type(&space_id).await, - Ok(ruma::room::RoomType::Space) - ) { - return self.write_str("Error: The specified room is not a Space.").await; - } - let room_event_type = StateEventType::from("com.continuwuity.space.role.room".to_owned()); + let space_id = resolve_space!(self, space); + let room_event_type = StateEventType::from(SPACE_ROLE_ROOM_EVENT_TYPE.to_owned()); let mut content: SpaceRoleRoomEventContent = self .services @@ -420,7 +393,7 @@ async fn unrequire( .rooms .timeline .build_and_append_pdu( - custom_state_pdu!("com.continuwuity.space.role.room", room_id.as_str(), &content), + custom_state_pdu!(SPACE_ROLE_ROOM_EVENT_TYPE, room_id.as_str(), &content), server_user, Some(&space_id), &state_lock, @@ -435,14 +408,7 @@ async fn unrequire( #[admin_command] async fn user(&self, space: OwnedRoomOrAliasId, user_id: OwnedUserId) -> Result { - require_enabled!(self); - let space_id = self.services.rooms.alias.resolve(&space).await?; - if !matches!( - self.services.rooms.state_accessor.get_room_type(&space_id).await, - Ok(ruma::room::RoomType::Space) - ) { - return self.write_str("Error: The specified room is not a Space.").await; - } + let space_id = resolve_space!(self, space); let user_roles = self.services.rooms.roles.user_roles.read().await; let roles = user_roles @@ -471,14 +437,7 @@ async fn user(&self, space: OwnedRoomOrAliasId, user_id: OwnedUserId) -> Result #[admin_command] async fn room(&self, space: OwnedRoomOrAliasId, room_id: OwnedRoomId) -> Result { - require_enabled!(self); - let space_id = self.services.rooms.alias.resolve(&space).await?; - if !matches!( - self.services.rooms.state_accessor.get_room_type(&space_id).await, - Ok(ruma::room::RoomType::Space) - ) { - return self.write_str("Error: The specified room is not a Space.").await; - } + let space_id = resolve_space!(self, space); let room_reqs = self.services.rooms.roles.room_requirements.read().await; let requirements = room_reqs diff --git a/src/core/matrix/space_roles.rs b/src/core/matrix/space_roles.rs index 420f7cb9..2bcd571f 100644 --- a/src/core/matrix/space_roles.rs +++ b/src/core/matrix/space_roles.rs @@ -7,6 +7,15 @@ use std::collections::BTreeMap; use serde::{Deserialize, Serialize}; +/// Custom event type for space role definitions. +pub const SPACE_ROLES_EVENT_TYPE: &str = "com.continuwuity.space.roles"; + +/// Custom event type for per-user role assignments within a space. +pub const SPACE_ROLE_MEMBER_EVENT_TYPE: &str = "com.continuwuity.space.role.member"; + +/// Custom event type for per-room role requirements within a space. +pub const SPACE_ROLE_ROOM_EVENT_TYPE: &str = "com.continuwuity.space.role.room"; + /// Content for `com.continuwuity.space.roles` (state key: "") /// /// Defines available roles for a Space. diff --git a/src/service/rooms/roles/mod.rs b/src/service/rooms/roles/mod.rs index 2bff986a..f72558cf 100644 --- a/src/service/rooms/roles/mod.rs +++ b/src/service/rooms/roles/mod.rs @@ -21,7 +21,8 @@ use serde_json::value::to_raw_value; use conduwuit_core::{ matrix::space_roles::{ RoleDefinition, SpaceRoleMemberEventContent, SpaceRoleRoomEventContent, - SpaceRolesEventContent, + SpaceRolesEventContent, SPACE_ROLES_EVENT_TYPE, SPACE_ROLE_MEMBER_EVENT_TYPE, + SPACE_ROLE_ROOM_EVENT_TYPE, }, utils::{ future::TryExtExt, @@ -55,6 +56,8 @@ pub struct Service { pub room_requirements: RwLock>>>, /// Child room ID -> parent space IDs (a room can be in multiple spaces) pub room_to_space: RwLock>>, + /// Space ID -> child room IDs (forward index for fast child lookups) + pub space_to_rooms: RwLock>>, /// Semaphore to limit concurrent enforcement tasks pub enforcement_semaphore: tokio::sync::Semaphore, } @@ -89,6 +92,7 @@ impl crate::Service for Service { user_roles: RwLock::new(HashMap::new()), room_requirements: RwLock::new(HashMap::new()), room_to_space: RwLock::new(HashMap::new()), + space_to_rooms: RwLock::new(HashMap::new()), enforcement_semaphore: tokio::sync::Semaphore::new(4), })) } @@ -102,11 +106,13 @@ impl crate::Service for Service { let user_roles = self.user_roles.read().await.len(); let room_requirements = self.room_requirements.read().await.len(); let room_to_space = self.room_to_space.read().await.len(); + let space_to_rooms = self.space_to_rooms.read().await.len(); writeln!(out, "space_roles_definitions: {roles}")?; writeln!(out, "space_user_roles: {user_roles}")?; writeln!(out, "space_room_requirements: {room_requirements}")?; writeln!(out, "space_room_to_space_index: {room_to_space}")?; + writeln!(out, "space_space_to_rooms_index: {space_to_rooms}")?; Ok(()) } @@ -120,6 +126,7 @@ impl crate::Service for Service { self.user_roles.write().await.clear(); self.room_requirements.write().await.clear(); self.room_to_space.write().await.clear(); + self.space_to_rooms.write().await.clear(); } async fn worker(self: Arc) -> Result<()> { @@ -172,7 +179,7 @@ pub async fn ensure_default_roles(&self, space_id: &RoomId) -> Result { } // Check if com.continuwuity.space.roles already exists - let roles_event_type = StateEventType::from("com.continuwuity.space.roles".to_owned()); + let roles_event_type = StateEventType::from(SPACE_ROLES_EVENT_TYPE.to_owned()); if self .services .state_accessor @@ -206,7 +213,7 @@ pub async fn ensure_default_roles(&self, space_id: &RoomId) -> Result { let state_lock = self.services.state.mutex.lock(space_id).await; let pdu = PduBuilder { - event_type: ruma::events::TimelineEventType::from("com.continuwuity.space.roles".to_owned()), + event_type: ruma::events::TimelineEventType::from(SPACE_ROLES_EVENT_TYPE.to_owned()), content: to_raw_value(&content) .map_err(|e| conduwuit::Error::Err(format!("Failed to serialize SpaceRolesEventContent: {e}").into()))?, state_key: Some(String::new().into()), @@ -218,7 +225,7 @@ pub async fn ensure_default_roles(&self, space_id: &RoomId) -> Result { .build_and_append_pdu(pdu, sender, Some(space_id), &state_lock) .await?; - debug!("Sent default com.continuwuity.space.roles event for {space_id}"); + debug!("Sent default {SPACE_ROLES_EVENT_TYPE} event for {space_id}"); Ok(()) } @@ -234,7 +241,7 @@ pub async fn populate_space(&self, space_id: &RoomId) { } // 1. Read com.continuwuity.space.roles (state key: "") - let roles_event_type = StateEventType::from("com.continuwuity.space.roles".to_owned()); + let roles_event_type = StateEventType::from(SPACE_ROLES_EVENT_TYPE.to_owned()); if let Ok(content) = self .services .state_accessor @@ -248,7 +255,7 @@ pub async fn populate_space(&self, space_id: &RoomId) { } // 2. Read all com.continuwuity.space.role.member state events (state key: user ID) - let member_event_type = StateEventType::from("com.continuwuity.space.role.member".to_owned()); + let member_event_type = StateEventType::from(SPACE_ROLE_MEMBER_EVENT_TYPE.to_owned()); let shortstatehash = match self .services .state @@ -293,7 +300,7 @@ pub async fn populate_space(&self, space_id: &RoomId) { .insert(space_id.to_owned(), user_roles_map); // 3. Read all com.continuwuity.space.role.room state events (state key: room ID) - let room_event_type = StateEventType::from("com.continuwuity.space.role.room".to_owned()); + let room_event_type = StateEventType::from(SPACE_ROLE_ROOM_EVENT_TYPE.to_owned()); let mut room_reqs_map: HashMap> = HashMap::new(); self.services @@ -356,6 +363,12 @@ pub async fn populate_space(&self, space_id: &RoomId) { }) .await; + // Update forward index + { + let mut space_to_rooms = self.space_to_rooms.write().await; + space_to_rooms.insert(space_id.to_owned(), child_rooms.iter().cloned().collect()); + } + // Single write lock for all children — clear stale entries first { let mut room_to_space = self.room_to_space.write().await; @@ -446,16 +459,15 @@ pub async fn get_parent_spaces(&self, room_id: &RoomId) -> Vec { .unwrap_or_default() } -/// Get all child rooms of a Space from the reverse index. +/// Get all child rooms of a Space from the forward index. #[implement(Service)] pub async fn get_child_rooms(&self, space_id: &RoomId) -> Vec { - self.room_to_space + self.space_to_rooms .read() .await - .iter() - .filter(|(_, parents)| parents.contains(space_id)) - .map(|(child, _)| child.clone()) - .collect() + .get(space_id) + .map(|set| set.iter().cloned().collect()) + .unwrap_or_default() } /// Synchronize power levels in a child room based on Space roles. @@ -676,7 +688,7 @@ impl Service { this.populate_space(&space_id).await; match event_type.as_str() { - | "com.continuwuity.space.roles" => { + | SPACE_ROLES_EVENT_TYPE => { // Role definitions changed — sync PLs in all child rooms let child_rooms = this.get_child_rooms(&space_id).await; for child_room_id in &child_rooms { @@ -687,7 +699,7 @@ impl Service { } } }, - | "com.continuwuity.space.role.member" => { + | SPACE_ROLE_MEMBER_EVENT_TYPE => { // User's roles changed — auto-join/kick + PL sync if let Ok(user_id) = UserId::parse(state_key.as_str()) { if let Err(e) = @@ -717,7 +729,7 @@ impl Service { } } }, - | "com.continuwuity.space.role.room" => { + | SPACE_ROLE_ROOM_EVENT_TYPE => { // Room requirements changed — kick unqualified members if let Ok(target_room) = RoomId::parse(state_key.as_str()) { let members: Vec = this @@ -793,6 +805,11 @@ impl Service { room_to_space.remove(&child_room_id); } } + // Remove child from space_to_rooms forward index + let mut space_to_rooms = this.space_to_rooms.write().await; + if let Some(children) = space_to_rooms.get_mut(&space_id) { + children.remove(&child_room_id); + } return; } @@ -804,6 +821,14 @@ impl Service { .or_default() .insert(space_id.clone()); + // Add child to forward index + this.space_to_rooms + .write() + .await + .entry(space_id.clone()) + .or_default() + .insert(child_room_id.clone()); + // Check if server user is joined to the child room before enforcement let server_user = this.services.globals.server_user.as_ref(); if !this diff --git a/src/service/rooms/timeline/append.rs b/src/service/rooms/timeline/append.rs index c6add2ff..611353ed 100644 --- a/src/service/rooms/timeline/append.rs +++ b/src/service/rooms/timeline/append.rs @@ -9,6 +9,9 @@ use conduwuit_core::{ matrix::{ event::Event, pdu::{PduCount, PduEvent, PduId, RawPduId}, + space_roles::{ + SPACE_ROLES_EVENT_TYPE, SPACE_ROLE_MEMBER_EVENT_TYPE, SPACE_ROLE_ROOM_EVENT_TYPE, + }, }, utils::{self, ReadyExt}, }; @@ -364,9 +367,9 @@ where if let Some(state_key) = pdu.state_key() { let event_type_str = pdu.event_type().to_string(); match event_type_str.as_str() { - | "com.continuwuity.space.roles" - | "com.continuwuity.space.role.member" - | "com.continuwuity.space.role.room" => { + | SPACE_ROLES_EVENT_TYPE + | SPACE_ROLE_MEMBER_EVENT_TYPE + | SPACE_ROLE_ROOM_EVENT_TYPE => { let roles: Arc = Arc::clone(&*self.services.roles); roles.handle_state_event_change( @@ -392,33 +395,20 @@ where } } // Handle m.room.member join in a Space — auto-join child rooms - if *pdu.kind() == TimelineEventType::RoomMember { - if let Some(state_key) = pdu.state_key() { - if let Ok(content) = - pdu.get_content::() - { - if content.membership - == ruma::events::room::member::MembershipState::Join - { - if let Ok(user_id) = UserId::parse(state_key) { - if matches!( - self.services - .state_accessor - .get_room_type(room_id) - .await, - Ok(ruma::room::RoomType::Space) - ) { - let roles: Arc = - Arc::clone(&*self.services.roles); - roles.handle_space_member_join( - room_id.to_owned(), - user_id.to_owned(), - ); - } - } - } - } - } + if *pdu.kind() == TimelineEventType::RoomMember + && let Some(state_key) = pdu.state_key() + && let Ok(content) = + pdu.get_content::() + && content.membership == ruma::events::room::member::MembershipState::Join + && let Ok(user_id) = UserId::parse(state_key) + && matches!( + self.services.state_accessor.get_room_type(room_id).await, + Ok(ruma::room::RoomType::Space) + ) + { + let roles: Arc = + Arc::clone(&*self.services.roles); + roles.handle_space_member_join(room_id.to_owned(), user_id.to_owned()); } }