From 12707418d30cec3f05498cf8c23ce9ffd1cfdcb0 Mon Sep 17 00:00:00 2001 From: Titouan Rigoudy Date: Wed, 30 Nov 2022 21:36:04 +0000 Subject: [PATCH] Move start_joining() from RoomMap to RoomEntry. --- .../src/handlers/room_join_request_handler.rs | 22 ++--- client/src/room/map.rs | 81 ++++++++++++------- client/src/room/state.rs | 14 +++- 3 files changed, 72 insertions(+), 45 deletions(-) diff --git a/client/src/handlers/room_join_request_handler.rs b/client/src/handlers/room_join_request_handler.rs index e6b0bc4..7c9df86 100644 --- a/client/src/handlers/room_join_request_handler.rs +++ b/client/src/handlers/room_join_request_handler.rs @@ -6,26 +6,14 @@ use solstice_proto::ServerRequest; use crate::context::Context; use crate::control; use crate::message_handler::MessageHandler; -use crate::room::RoomError; #[derive(Debug, Default)] pub struct RoomJoinRequestHandler; -fn start_joining( - context: &mut Context, - room_name: &str, -) -> Result<(), RoomError> { - let result = context.state.rooms.start_joining(room_name); - - if let Err(RoomError::MembershipChangeInvalid(_, _)) = result { - // This `expect()` should never fail since the error was not - // `RoomNotFound` but `MembershipChangeInvalid`. - let room = context - .state - .rooms - .get_strict(room_name) - .expect("querying room state"); +fn start_joining(context: &mut Context, room_name: &str) -> anyhow::Result<()> { + let room = context.state.rooms.get_mut_strict(room_name)?; + room.start_joining().map_err(|err| { let response = control::Response::RoomJoinResponse(control::RoomJoinResponse { room_name: room_name.to_string(), @@ -38,9 +26,9 @@ fn start_joining( room_name, err ); } - } - result + err.into() + }) } impl MessageHandler for RoomJoinRequestHandler { diff --git a/client/src/room/map.rs b/client/src/room/map.rs index 5b37b0b..45536fd 100644 --- a/client/src/room/map.rs +++ b/client/src/room/map.rs @@ -13,10 +13,14 @@ pub enum RoomError { #[error(transparent)] RoomNotFound(#[from] RoomNotFoundError), - #[error("cannot change membership from {0:?} to {1:?}")] - MembershipChangeInvalid(RoomMembership, RoomMembership), + #[error(transparent)] + MembershipChangeInvalid(#[from] RoomMembershipChangeError), } +#[derive(Debug, Error)] +#[error("cannot change membership from {0:?} to {1:?}")] +pub struct RoomMembershipChangeError(RoomMembership, RoomMembership); + #[derive(Debug, Error)] #[error("room {0} not found")] pub struct RoomNotFoundError(String); @@ -28,6 +32,11 @@ pub struct RoomEntry { } impl RoomEntry { + /// Creates a new entry with the given state. + pub fn new(state: RoomState) -> Self { + Self { state } + } + /// Returns a copy of the room state contained in this entry. pub fn clone_state(&self) -> RoomState { self.state.clone() @@ -37,6 +46,23 @@ impl RoomEntry { pub fn insert_message(&mut self, message: RoomMessage) { self.state.messages.insert(message) } + + /// Records that we are now trying to join the given room. + /// If the room is not found, or if its membership is not `NonMember`, + /// returns an error. + pub fn start_joining(&mut self) -> Result<(), RoomMembershipChangeError> { + match self.state.membership { + RoomMembership::NonMember => { + self.state.membership = RoomMembership::Joining; + Ok(()) + } + + membership => Err(RoomMembershipChangeError( + membership, + RoomMembership::Joining, + )), + } + } } /// Contains the mapping from room names to room data and provides a clean @@ -58,7 +84,7 @@ impl RoomMap { pub fn insert(&mut self, name: String, room: RoomState) -> Option { self .map - .insert(name, RoomEntry { state: room }) + .insert(name, RoomEntry::new(room)) .map(|entry| entry.state) } @@ -157,25 +183,6 @@ impl RoomMap { rooms } - /// Records that we are now trying to join the given room. - /// If the room is not found, or if its membership is not `NonMember`, - /// returns an error. - pub fn start_joining(&mut self, room_name: &str) -> Result<(), RoomError> { - let room = self.get_mut_strict(room_name)?; - - match room.state.membership { - RoomMembership::NonMember => { - room.state.membership = RoomMembership::Joining; - Ok(()) - } - - membership => Err(RoomError::MembershipChangeInvalid( - membership, - RoomMembership::Joining, - )), - } - } - /// Records that we are now a member of the given room and updates the room /// information. pub fn join( @@ -230,8 +237,7 @@ impl RoomMap { } membership => Err(RoomError::MembershipChangeInvalid( - membership, - RoomMembership::Leaving, + RoomMembershipChangeError(membership, RoomMembership::Leaving), )), } } @@ -296,17 +302,38 @@ impl RoomMap { mod tests { use solstice_proto::server::RoomListResponse; - use crate::room::{RoomState, RoomVisibility}; + use crate::room::{RoomMembership, RoomState, RoomVisibility}; use super::*; #[test] - fn new_is_empty() { + fn entry_start_joining_error() { + let mut room = RoomEntry::new(RoomState { + membership: RoomMembership::Member, + ..RoomState::default() + }); + + room.start_joining().unwrap_err(); + + assert_eq!(room.clone_state().membership, RoomMembership::Member); + } + + #[test] + fn entry_start_joining_success() { + let mut room = RoomEntry::new(RoomState::default()); + + room.start_joining().unwrap(); + + assert_eq!(room.clone_state().membership, RoomMembership::Joining); + } + + #[test] + fn map_new_is_empty() { assert_eq!(RoomMap::new().get_room_list(), vec![]); } #[test] - fn get_strict() { + fn map_get_strict() { let mut rooms = RoomMap::new(); rooms.set_room_list(RoomListResponse { rooms: vec![("room a".to_string(), 42), ("room b".to_string(), 1337)], diff --git a/client/src/room/state.rs b/client/src/room/state.rs index 5c34a93..a0599f9 100644 --- a/client/src/room/state.rs +++ b/client/src/room/state.rs @@ -18,6 +18,12 @@ pub enum RoomMembership { Leaving, } +impl Default for RoomMembership { + fn default() -> Self { + Self::NonMember + } +} + /// This enumeration is the list of visibility types for rooms that the user is /// a member of. #[derive(Clone, Copy, Debug, Eq, PartialEq, Serialize, Deserialize)] @@ -30,6 +36,12 @@ pub enum RoomVisibility { PrivateOther, } +impl Default for RoomVisibility { + fn default() -> Self { + Self::Public + } +} + /// A message sent to a chat room. #[derive( Clone, Debug, Eq, PartialEq, Ord, PartialOrd, Serialize, Deserialize, @@ -105,7 +117,7 @@ impl RoomMessageHistory { /// This structure contains the last known information about a chat room. /// It does not store the name, as that is stored implicitly as the key in the /// room hash table. -#[derive(Clone, Debug, Eq, PartialEq, Serialize, Deserialize)] +#[derive(Clone, Debug, Default, Eq, PartialEq, Serialize, Deserialize)] pub struct RoomState { /// The membership state of the user for the room. pub membership: RoomMembership,