Skip to content

Commit

Permalink
Fix registering virtual users multiple times
Browse files Browse the repository at this point in the history
  • Loading branch information
exul committed Apr 15, 2017
1 parent fe21df9 commit 89e388e
Show file tree
Hide file tree
Showing 4 changed files with 105 additions and 20 deletions.
3 changes: 1 addition & 2 deletions src/matrix-rocketchat/handlers/events/command_handler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -401,10 +401,9 @@ impl<'a> CommandHandler<'a> {
//TODO: Check if a max number of users per channel has to be defined to avoid problems when
//there are several thousand users in a channel.
for username in channel.usernames.iter() {
debug!(self.logger, username);
let rocketchat_user = rocketchat_api.users_info(&username)?;
let user_on_rocketchat_server =
virtual_user_handler.register(rocketchat_server_id, rocketchat_user.id, username.to_string())?;
virtual_user_handler.find_or_register(rocketchat_server_id, rocketchat_user.id, username.to_string())?;
virtual_user_handler.add_to_room(user_on_rocketchat_server.matrix_user_id, matrix_room_id.clone())?;
}

Expand Down
17 changes: 4 additions & 13 deletions src/matrix-rocketchat/handlers/rocketchat/forwarder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,21 +35,12 @@ impl<'a> Forwarder<'a> {
matrix_api: self.matrix_api,
};

let mut user_on_rocketchat_server = match UserOnRocketchatServer::find_by_rocketchat_user_id(self.connection,
rocketchat_server.id,
message.user_id
.clone(),
true)? {
Some(user_on_rocketchat_server) => user_on_rocketchat_server,
None => {
self.connection
.transaction(|| {
virtual_user_handler.register(rocketchat_server.id,
let mut user_on_rocketchat_server = self.connection
.transaction(|| {
virtual_user_handler.find_or_register(rocketchat_server.id,
message.user_id.clone(),
message.user_name.clone())
})?
}
};
})?;

if !self.is_sendable_message(&user_on_rocketchat_server)? {
debug!(self.logger,
Expand Down
18 changes: 13 additions & 5 deletions src/matrix-rocketchat/handlers/rocketchat/virtual_user_handler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,15 +31,23 @@ impl<'a> VirtualUserHandler<'a> {
}

/// Register a virtual user on the Matrix server and assign it to a Rocket.Chat server.
pub fn register(&self,
rocketchat_server_id: i32,
rocketchat_user_id: String,
rocketchat_user_name: String)
-> Result<UserOnRocketchatServer> {
pub fn find_or_register(&self,
rocketchat_server_id: i32,
rocketchat_user_id: String,
rocketchat_user_name: String)
-> Result<UserOnRocketchatServer> {
let user_id_local_part = format!("{}_{}_{}", self.config.sender_localpart, &rocketchat_user_id, rocketchat_server_id);
let user_id = format!("@{}:{}", user_id_local_part, self.config.hs_domain);
let matrix_user_id = UserId::try_from(&user_id).chain_err(|| ErrorKind::InvalidUserId(user_id))?;

if let Some(user_on_rocketchat_server) =
UserOnRocketchatServer::find_by_rocketchat_user_id(self.connection,
rocketchat_server_id.clone(),
rocketchat_user_id.clone(),
true)? {
return Ok(user_on_rocketchat_server);
}

let new_user = NewUser {
language: DEFAULT_LANGUAGE,
matrix_user_id: matrix_user_id.clone(),
Expand Down
87 changes: 87 additions & 0 deletions tests/admin_commands_bridge.rs
Original file line number Diff line number Diff line change
Expand Up @@ -300,6 +300,93 @@ fn susccessfully_bridge_a_rocketchat_room_that_was_unbridged_before() {
assert!(users_in_room.iter().any(|u| u.matrix_user_id == UserId::try_from("@spec_user:localhost").unwrap()));
}

#[test]
fn successfully_bridge_two_different_rocketchat_rooms() {
let (message_forwarder, receiver) = MessageForwarder::new();
let mut matrix_router = Router::new();
matrix_router.put(SendMessageEventEndpoint::router_path(), message_forwarder, "send_message_event");
matrix_router.post(CreateRoomEndpoint::router_path(), handlers::MatrixCreateRoom {}, "create_room");
let mut channels = HashMap::new();
channels.insert("first_channel", vec!["spec_user", "other_user"]);
channels.insert("second_channel", vec!["spec_user", "other_user"]);

let test = Test::new()
.with_matrix_routes(matrix_router)
.with_rocketchat_mock()
.with_connected_admin_room()
.with_logged_in_user()
.with_custom_channel_list(channels)
.run();

// discard welcome message
receiver.recv_timeout(default_timeout()).unwrap();
// discard connect message
receiver.recv_timeout(default_timeout()).unwrap();
// discard login message
receiver.recv_timeout(default_timeout()).unwrap();

helpers::send_room_message_from_matrix(&test.config.as_url,
RoomId::try_from("!admin:localhost").unwrap(),
UserId::try_from("@spec_user:localhost").unwrap(),
"bridge first_channel".to_string());

helpers::send_room_message_from_matrix(&test.config.as_url,
RoomId::try_from("!admin:localhost").unwrap(),
UserId::try_from("@spec_user:localhost").unwrap(),
"bridge second_channel".to_string());

let first_message_received_by_matrix = receiver.recv_timeout(default_timeout()).unwrap();
assert!(first_message_received_by_matrix.contains("first_channel is now bridged."));

let second_message_received_by_matrix = receiver.recv_timeout(default_timeout()).unwrap();
assert!(second_message_received_by_matrix.contains("second_channel is now bridged."));

// users accepts invite from bot user
helpers::join(&test.config.as_url,
RoomId::try_from("!first_channel_id:localhost").unwrap(),
UserId::try_from("@spec_user:localhost").unwrap());

helpers::join(&test.config.as_url,
RoomId::try_from("!first_channel_id:localhost").unwrap(),
UserId::try_from("@rocketchat_spec_user_id_1:localhost").unwrap());

helpers::join(&test.config.as_url,
RoomId::try_from("!first_channel_id:localhost").unwrap(),
UserId::try_from("@rocketchat_other_user_id_1:localhost").unwrap());

helpers::join(&test.config.as_url,
RoomId::try_from("!second_channel_id:localhost").unwrap(),
UserId::try_from("@spec_user:localhost").unwrap());

helpers::join(&test.config.as_url,
RoomId::try_from("!second_channel_id:localhost").unwrap(),
UserId::try_from("@rocketchat_spec_user_id_1:localhost").unwrap());

helpers::join(&test.config.as_url,
RoomId::try_from("!second_channel_id:localhost").unwrap(),
UserId::try_from("@rocketchat_other_user_id_1:localhost").unwrap());


let connection = test.connection_pool.get().unwrap();
let first_room = Room::find(&connection, &RoomId::try_from("!first_channel_id:localhost").unwrap()).unwrap();
assert_eq!(first_room.display_name, "first_channel");

let first_users = first_room.users(&connection).unwrap();
assert!(first_users.iter().any(|u| u.matrix_user_id == UserId::try_from("@rocketchat:localhost").unwrap()));
assert!(first_users.iter().any(|u| u.matrix_user_id == UserId::try_from("@spec_user:localhost").unwrap()));
assert!(first_users.iter().any(|u| u.matrix_user_id == UserId::try_from("@rocketchat_spec_user_id_1:localhost").unwrap()));
assert!(first_users.iter().any(|u| u.matrix_user_id == UserId::try_from("@rocketchat_other_user_id_1:localhost").unwrap()));

let sec_room = Room::find(&connection, &RoomId::try_from("!second_channel_id:localhost").unwrap()).unwrap();
assert_eq!(sec_room.display_name, "second_channel");

let sec_users = sec_room.users(&connection).unwrap();
assert!(sec_users.iter().any(|u| u.matrix_user_id == UserId::try_from("@rocketchat:localhost").unwrap()));
assert!(sec_users.iter().any(|u| u.matrix_user_id == UserId::try_from("@spec_user:localhost").unwrap()));
assert!(sec_users.iter().any(|u| u.matrix_user_id == UserId::try_from("@rocketchat_spec_user_id_1:localhost").unwrap()));
assert!(sec_users.iter().any(|u| u.matrix_user_id == UserId::try_from("@rocketchat_other_user_id_1:localhost").unwrap()));
}

#[test]
fn do_not_allow_to_bridge_channels_that_the_user_has_not_joined_on_the_rocketchat_server() {
let (message_forwarder, receiver) = MessageForwarder::new();
Expand Down

0 comments on commit 89e388e

Please sign in to comment.