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) <noreply@anthropic.com>
This commit is contained in:
ember33 2026-03-18 11:06:58 +01:00
parent 50604724ff
commit 898f4a470c
4 changed files with 115 additions and 132 deletions

View file

@ -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<String>,
power_level: Option<i64>,
) -> 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

View file

@ -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.

View file

@ -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<HashMap<OwnedRoomId, HashMap<OwnedRoomId, HashSet<String>>>>,
/// Child room ID -> parent space IDs (a room can be in multiple spaces)
pub room_to_space: RwLock<HashMap<OwnedRoomId, HashSet<OwnedRoomId>>>,
/// Space ID -> child room IDs (forward index for fast child lookups)
pub space_to_rooms: RwLock<HashMap<OwnedRoomId, HashSet<OwnedRoomId>>>,
/// 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<Self>) -> 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<OwnedRoomId, HashSet<String>> = 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<OwnedRoomId> {
.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<OwnedRoomId> {
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<OwnedUserId> = 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

View file

@ -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<crate::rooms::roles::Service> =
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::<ruma::events::room::member::RoomMemberEventContent>()
{
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<crate::rooms::roles::Service> =
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::<ruma::events::room::member::RoomMemberEventContent>()
&& 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<crate::rooms::roles::Service> =
Arc::clone(&*self.services.roles);
roles.handle_space_member_join(room_id.to_owned(), user_id.to_owned());
}
}