From f7af6966b7fbfac3de141f7101bfe8b5e3904c85 Mon Sep 17 00:00:00 2001 From: Jason Volk Date: Thu, 3 Oct 2024 09:44:43 +0000 Subject: [PATCH] refactor to room_state_get_content() for serde_json::from_ elim Signed-off-by: Jason Volk --- src/api/client/membership.rs | 78 ++++++++++--------------- src/api/client/profile.rs | 57 ++++++++---------- src/api/client/room.rs | 32 ++++------ src/service/rooms/alias/mod.rs | 27 +++++---- src/service/rooms/spaces/mod.rs | 9 +-- src/service/rooms/state_accessor/mod.rs | 23 +++----- src/service/rooms/timeline/mod.rs | 7 +-- 7 files changed, 91 insertions(+), 142 deletions(-) diff --git a/src/api/client/membership.rs b/src/api/client/membership.rs index ae56094c..a260b8c5 100644 --- a/src/api/client/membership.rs +++ b/src/api/client/membership.rs @@ -389,17 +389,12 @@ pub(crate) async fn kick_user_route( let state_lock = services.rooms.state.mutex.lock(&body.room_id).await; - let mut event: RoomMemberEventContent = serde_json::from_str( - services - .rooms - .state_accessor - .room_state_get(&body.room_id, &StateEventType::RoomMember, body.user_id.as_ref()) - .await - .map_err(|_| err!(Request(BadState("Cannot kick member that's not in the room."))))? - .content - .get(), - ) - .map_err(|_| err!(Database("Invalid member event in database.")))?; + let mut event: RoomMemberEventContent = services + .rooms + .state_accessor + .room_state_get_content(&body.room_id, &StateEventType::RoomMember, body.user_id.as_ref()) + .await + .map_err(|_| err!(Request(BadState("Cannot kick member that's not in the room."))))?; event.membership = MembershipState::Leave; event.reason.clone_from(&body.reason); @@ -442,10 +437,10 @@ pub(crate) async fn ban_user_route( let event = services .rooms .state_accessor - .room_state_get(&body.room_id, &StateEventType::RoomMember, body.user_id.as_ref()) + .room_state_get_content(&body.room_id, &StateEventType::RoomMember, body.user_id.as_ref()) .await - .map_or( - Ok(RoomMemberEventContent { + .map_or_else( + |_| RoomMemberEventContent { membership: MembershipState::Ban, displayname: None, avatar_url: None, @@ -454,21 +449,17 @@ pub(crate) async fn ban_user_route( blurhash: blurhash.clone(), reason: body.reason.clone(), join_authorized_via_users_server: None, - }), - |event| { - serde_json::from_str(event.content.get()) - .map(|event: RoomMemberEventContent| RoomMemberEventContent { - membership: MembershipState::Ban, - displayname: None, - avatar_url: None, - blurhash: blurhash.clone(), - reason: body.reason.clone(), - join_authorized_via_users_server: None, - ..event - }) - .map_err(|e| err!(Database("Invalid member event in database: {e:?}"))) }, - )?; + |event| RoomMemberEventContent { + membership: MembershipState::Ban, + displayname: None, + avatar_url: None, + blurhash: blurhash.clone(), + reason: body.reason.clone(), + join_authorized_via_users_server: None, + ..event + }, + ); services .rooms @@ -503,17 +494,12 @@ pub(crate) async fn unban_user_route( let state_lock = services.rooms.state.mutex.lock(&body.room_id).await; - let mut event: RoomMemberEventContent = serde_json::from_str( - services - .rooms - .state_accessor - .room_state_get(&body.room_id, &StateEventType::RoomMember, body.user_id.as_ref()) - .await - .map_err(|_| err!(Request(BadState("Cannot unban a user who is not banned."))))? - .content - .get(), - ) - .map_err(|e| err!(Database("Invalid member event in database: {e:?}")))?; + let mut event: RoomMemberEventContent = services + .rooms + .state_accessor + .room_state_get_content(&body.room_id, &StateEventType::RoomMember, body.user_id.as_ref()) + .await + .map_err(|_| err!(Request(BadState("Cannot unban a user who is not banned."))))?; event.membership = MembershipState::Leave; event.reason.clone_from(&body.reason); @@ -1650,14 +1636,13 @@ pub async fn leave_room(services: &Services, user_id: &UserId, room_id: &RoomId, } else { let state_lock = services.rooms.state.mutex.lock(room_id).await; - let member_event = services + let Ok(mut event) = services .rooms .state_accessor - .room_state_get(room_id, &StateEventType::RoomMember, user_id.as_str()) - .await; - - // Fix for broken rooms - let Ok(member_event) = member_event else { + .room_state_get_content::(room_id, &StateEventType::RoomMember, user_id.as_str()) + .await + else { + // Fix for broken rooms error!("Trying to leave a room you are not a member of."); services @@ -1677,9 +1662,6 @@ pub async fn leave_room(services: &Services, user_id: &UserId, room_id: &RoomId, return Ok(()); }; - let mut event: RoomMemberEventContent = serde_json::from_str(member_event.content.get()) - .map_err(|e| err!(Database(error!("Invalid room member event in database: {e}"))))?; - event.membership = MembershipState::Leave; event.reason = reason; diff --git a/src/api/client/profile.rs b/src/api/client/profile.rs index 495bc8ec..cdc047f0 100644 --- a/src/api/client/profile.rs +++ b/src/api/client/profile.rs @@ -301,10 +301,10 @@ pub async fn update_displayname( // Send a new join membership event into all joined rooms let mut joined_rooms = Vec::new(); for room_id in all_joined_rooms { - let Ok(event) = services + let Ok(content) = services .rooms .state_accessor - .room_state_get(room_id, &StateEventType::RoomMember, user_id.as_str()) + .room_state_get_content(room_id, &StateEventType::RoomMember, user_id.as_str()) .await else { continue; @@ -315,7 +315,7 @@ pub async fn update_displayname( content: to_raw_value(&RoomMemberEventContent { displayname: displayname.clone(), join_authorized_via_users_server: None, - ..serde_json::from_str(event.content.get()).expect("Database contains invalid PDU.") + ..content }) .expect("event is valid, we just created it"), unsigned: None, @@ -354,35 +354,28 @@ pub async fn update_avatar_url( .iter() .try_stream() .and_then(|room_id: &OwnedRoomId| async move { - Ok(( - PduBuilder { - event_type: TimelineEventType::RoomMember, - content: to_raw_value(&RoomMemberEventContent { - avatar_url: avatar_url.clone(), - blurhash: blurhash.clone(), - join_authorized_via_users_server: None, - ..serde_json::from_str( - services - .rooms - .state_accessor - .room_state_get(room_id, &StateEventType::RoomMember, user_id.as_str()) - .await - .map_err(|_| { - Error::bad_database("Tried to send avatar URL update for user not in the room.") - })? - .content - .get(), - ) - .map_err(|_| Error::bad_database("Database contains invalid PDU."))? - }) - .expect("event is valid, we just created it"), - unsigned: None, - state_key: Some(user_id.to_string()), - redacts: None, - timestamp: None, - }, - room_id, - )) + let content = services + .rooms + .state_accessor + .room_state_get_content(room_id, &StateEventType::RoomMember, user_id.as_str()) + .await?; + + let pdu = PduBuilder { + event_type: TimelineEventType::RoomMember, + content: to_raw_value(&RoomMemberEventContent { + avatar_url: avatar_url.clone(), + blurhash: blurhash.clone(), + join_authorized_via_users_server: None, + ..content + }) + .expect("event is valid, we just created it"), + unsigned: None, + state_key: Some(user_id.to_string()), + redacts: None, + timestamp: None, + }; + + Ok((pdu, room_id)) }) .ignore_err() .collect() diff --git a/src/api/client/room.rs b/src/api/client/room.rs index 0d8e12a2..e22ad796 100644 --- a/src/api/client/room.rs +++ b/src/api/client/room.rs @@ -664,16 +664,12 @@ pub(crate) async fn upgrade_room_route( let state_lock = services.rooms.state.mutex.lock(&replacement_room).await; // Get the old room creation event - let mut create_event_content = serde_json::from_str::( - services - .rooms - .state_accessor - .room_state_get(&body.room_id, &StateEventType::RoomCreate, "") - .await - .map_err(|_| err!(Database("Found room without m.room.create event.")))? - .content - .get(), - )?; + let mut create_event_content: CanonicalJsonObject = services + .rooms + .state_accessor + .room_state_get_content(&body.room_id, &StateEventType::RoomCreate, "") + .await + .map_err(|_| err!(Database("Found room without m.room.create event.")))?; // Use the m.room.tombstone event as the predecessor let predecessor = Some(ruma::events::room::create::PreviousRoom::new( @@ -825,16 +821,12 @@ pub(crate) async fn upgrade_room_route( } // Get the old room power levels - let mut power_levels_event_content: RoomPowerLevelsEventContent = serde_json::from_str( - services - .rooms - .state_accessor - .room_state_get(&body.room_id, &StateEventType::RoomPowerLevels, "") - .await - .map_err(|_| err!(Database("Found room without m.room.create event.")))? - .content - .get(), - )?; + let mut power_levels_event_content: RoomPowerLevelsEventContent = services + .rooms + .state_accessor + .room_state_get_content(&body.room_id, &StateEventType::RoomPowerLevels, "") + .await + .map_err(|_| err!(Database("Found room without m.room.power_levels event.")))?; // Setting events_default and invite to the greater of 50 and users_default + 1 let new_level = max( diff --git a/src/service/rooms/alias/mod.rs b/src/service/rooms/alias/mod.rs index f50cc46c..7fac6be6 100644 --- a/src/service/rooms/alias/mod.rs +++ b/src/service/rooms/alias/mod.rs @@ -190,32 +190,31 @@ impl Service { // Always allow the server service account to remove the alias, since there may not be an admin room || server_user == user_id { - Ok(true) - // Checking whether the user is able to change canonical aliases of the - // room - } else if let Ok(event) = self + return Ok(true); + } + + // Checking whether the user is able to change canonical aliases of the room + if let Ok(content) = self .services .state_accessor - .room_state_get(&room_id, &StateEventType::RoomPowerLevels, "") + .room_state_get_content::(&room_id, &StateEventType::RoomPowerLevels, "") .await { - serde_json::from_str(event.content.get()) - .map_err(|_| Error::bad_database("Invalid event content for m.room.power_levels")) - .map(|content: RoomPowerLevelsEventContent| { - RoomPowerLevels::from(content).user_can_send_state(user_id, StateEventType::RoomCanonicalAlias) - }) + return Ok(RoomPowerLevels::from(content).user_can_send_state(user_id, StateEventType::RoomCanonicalAlias)); + } + // If there is no power levels event, only the room creator can change // canonical aliases - } else if let Ok(event) = self + if let Ok(event) = self .services .state_accessor .room_state_get(&room_id, &StateEventType::RoomCreate, "") .await { - Ok(event.sender == user_id) - } else { - Err(Error::bad_database("Room has no m.room.create event")) + return Ok(event.sender == user_id); } + + Err!(Database("Room has no m.room.create event")) } async fn who_created_alias(&self, alias: &RoomAliasId) -> Result { diff --git a/src/service/rooms/spaces/mod.rs b/src/service/rooms/spaces/mod.rs index 920424a4..a30c2cfc 100644 --- a/src/service/rooms/spaces/mod.rs +++ b/src/service/rooms/spaces/mod.rs @@ -380,14 +380,9 @@ impl Service { let join_rule = self .services .state_accessor - .room_state_get(room_id, &StateEventType::RoomJoinRules, "") + .room_state_get_content(room_id, &StateEventType::RoomJoinRules, "") .await - .map_or(JoinRule::Invite, |s| { - serde_json::from_str(s.content.get()) - .map(|c: RoomJoinRulesEventContent| c.join_rule) - .map_err(|e| err!(Database(error!("Invalid room join rule event in database: {e}")))) - .unwrap() - }); + .map_or(JoinRule::Invite, |c: RoomJoinRulesEventContent| c.join_rule); let allowed_room_ids = self .services diff --git a/src/service/rooms/state_accessor/mod.rs b/src/service/rooms/state_accessor/mod.rs index ece8679d..3b2c2931 100644 --- a/src/service/rooms/state_accessor/mod.rs +++ b/src/service/rooms/state_accessor/mod.rs @@ -338,14 +338,13 @@ impl Service { .map(|c: RoomNameEventContent| c.name) } - pub async fn get_avatar(&self, room_id: &RoomId) -> ruma::JsOption { - self.room_state_get(room_id, &StateEventType::RoomAvatar, "") + pub async fn get_avatar(&self, room_id: &RoomId) -> JsOption { + let content = self + .room_state_get_content(room_id, &StateEventType::RoomAvatar, "") .await - .map_or(ruma::JsOption::Undefined, |s| { - serde_json::from_str(s.content.get()) - .map_err(|_| Error::bad_database("Invalid room avatar event in database.")) - .unwrap() - }) + .ok(); + + JsOption::from_option(content) } pub async fn get_member(&self, room_id: &RoomId, user_id: &UserId) -> Result { @@ -416,16 +415,10 @@ impl Service { &self, redacts: &EventId, sender: &UserId, room_id: &RoomId, federation: bool, ) -> Result { if let Ok(event) = self - .room_state_get(room_id, &StateEventType::RoomPowerLevels, "") + .room_state_get_content::(room_id, &StateEventType::RoomPowerLevels, "") .await { - let Ok(event) = serde_json::from_str(event.content.get()) - .map(|content: RoomPowerLevelsEventContent| content.into()) - .map(|event: RoomPowerLevels| event) - else { - return Ok(false); - }; - + let event: RoomPowerLevels = event.into(); Ok(event.user_can_redact_event_of_other(sender) || event.user_can_redact_own_event(sender) && if let Ok(pdu) = self.services.timeline.get_pdu(redacts).await { diff --git a/src/service/rooms/timeline/mod.rs b/src/service/rooms/timeline/mod.rs index 7cf06522..cc5940e6 100644 --- a/src/service/rooms/timeline/mod.rs +++ b/src/service/rooms/timeline/mod.rs @@ -1061,13 +1061,8 @@ impl Service { let power_levels: RoomPowerLevelsEventContent = self .services .state_accessor - .room_state_get(room_id, &StateEventType::RoomPowerLevels, "") + .room_state_get_content(room_id, &StateEventType::RoomPowerLevels, "") .await - .map(|ev| { - serde_json::from_str(ev.content.get()) - .map_err(|_| Error::bad_database("invalid m.room.power_levels event")) - .unwrap() - }) .unwrap_or_default(); let room_mods = power_levels.users.iter().filter_map(|(user_id, level)| {