From 395753e8a9a737484913bbe9b1b0959f9c438d9a Mon Sep 17 00:00:00 2001 From: projectmoon Date: Mon, 24 May 2021 21:32:00 +0000 Subject: [PATCH 1/3] Remove room state mgmt; let matrix SDK do it on-demand instead. Fixes #71. Fixes #20. --- src/bot/event_handlers.rs | 72 ++------------------------------------ src/commands/management.rs | 33 +---------------- src/commands/parser.rs | 7 +--- src/logic.rs | 47 +------------------------ src/matrix.rs | 29 +++++++++++++-- 5 files changed, 32 insertions(+), 156 deletions(-) diff --git a/src/bot/event_handlers.rs b/src/bot/event_handlers.rs index b9a3015..d4f1ab5 100644 --- a/src/bot/event_handlers.rs +++ b/src/bot/event_handlers.rs @@ -7,15 +7,14 @@ use super::DiceBot; use crate::db::sqlite::Database; use crate::db::Rooms; use crate::error::BotError; -use crate::logic::record_room_information; use async_trait::async_trait; use log::{debug, error, info, warn}; use matrix_sdk::{ self, events::{ - room::member::{MemberEventContent, MembershipChange}, + room::member::MemberEventContent, room::message::{MessageEventContent, MessageType, TextMessageEventContent}, - StrippedStateEvent, SyncMessageEvent, SyncStateEvent, + StrippedStateEvent, SyncMessageEvent, }, room::Room, EventHandler, @@ -111,73 +110,6 @@ async fn should_process_event(db: &Database, room_id: &str, event_id: &str) -> b /// Originally adapted from the matrix-rust-sdk examples. #[async_trait] impl EventHandler for DiceBot { - async fn on_room_member(&self, room: Room, event: &SyncStateEvent) { - //let room = Common::new(self.client.clone(), room); - let (room_id, room_display_name) = match room.display_name().await { - Ok(display_name) => (room.room_id(), display_name), - _ => return, - }; - - let room_id_str = room_id.as_str(); - let username = &event.state_key; - - if !should_process_event(&self.db, room_id_str, event.event_id.as_str()).await { - return; - } - - let event_affects_us = if let Some(our_user_id) = self.client.user_id().await { - event.state_key == our_user_id - } else { - false - }; - - // user_joing is true if a user is joining this room, and - // false if they have left for some reason. This user may be - // us, or another user in the room. - use MembershipChange::*; - let user_joining = match event.membership_change() { - Joined => true, - Banned | Left | Kicked | KickedAndBanned => false, - _ => return, - }; - - let result = if event_affects_us && !user_joining { - info!("Clearing all information for room ID {}", room_id); - self.db.clear_info(room_id_str).await.map_err(|e| e.into()) - } else if event_affects_us && user_joining { - info!("Joined room {}; recording room information", room_id); - record_room_information( - &self.client, - &self.db, - &room_id, - &room_display_name, - &event.state_key, - ) - .await - } else if !event_affects_us && user_joining { - info!("Adding user {} to room ID {}", username, room_id); - self.db - .add_user_to_room(username, room_id_str) - .await - .map_err(|e| e.into()) - } else if !event_affects_us && !user_joining { - info!("Removing user {} from room ID {}", username, room_id); - self.db - .remove_user_from_room(username, room_id_str) - .await - .map_err(|e| e.into()) - } else { - debug!("Ignoring a room member event: {:#?}", event); - Ok(()) - }; - - if let Err(e) = result { - error!("Could not update room information: {}", e.to_string()); - } else { - debug!("Successfully processed room member update."); - } - } - async fn on_stripped_state_member( &self, room: Room, diff --git a/src/commands/management.rs b/src/commands/management.rs index 7e8d31b..a29b040 100644 --- a/src/commands/management.rs +++ b/src/commands/management.rs @@ -2,40 +2,9 @@ use super::{Command, Execution, ExecutionResult}; use crate::context::Context; use crate::db::Users; use crate::error::BotError::{AccountDoesNotExist, AuthenticationError, PasswordCreationError}; -use crate::logic::{hash_password, record_room_information}; +use crate::logic::hash_password; use crate::models::User; use async_trait::async_trait; -use matrix_sdk::identifiers::UserId; - -pub struct ResyncCommand; - -#[async_trait] -impl Command for ResyncCommand { - fn name(&self) -> &'static str { - "resync room information" - } - - fn is_secure(&self) -> bool { - false - } - - async fn execute(&self, ctx: &Context<'_>) -> ExecutionResult { - let our_username: Option = ctx.matrix_client.user_id().await; - let our_username: &str = our_username.as_ref().map_or("", UserId::as_str); - - record_room_information( - ctx.matrix_client, - &ctx.db, - ctx.room_id(), - &ctx.room.display_name, - our_username, - ) - .await?; - - let message = "Room information resynced.".to_string(); - Execution::success(message) - } -} pub struct RegisterCommand(pub String); diff --git a/src/commands/parser.rs b/src/commands/parser.rs index 327ca6b..0f8567a 100644 --- a/src/commands/parser.rs +++ b/src/commands/parser.rs @@ -9,7 +9,7 @@ use crate::commands::{ basic_rolling::RollCommand, cofd::PoolRollCommand, cthulhu::{CthAdvanceRoll, CthRoll}, - management::{CheckCommand, RegisterCommand, ResyncCommand, UnregisterCommand}, + management::{CheckCommand, RegisterCommand, UnregisterCommand}, misc::HelpCommand, variables::{ DeleteVariableCommand, GetAllVariablesCommand, GetVariableCommand, SetVariableCommand, @@ -96,10 +96,6 @@ fn get_all_variables() -> Result, BotError> { Ok(Box::new(GetAllVariablesCommand)) } -fn parse_resync() -> Result, BotError> { - Ok(Box::new(ResyncCommand)) -} - fn help(topic: &str) -> Result, BotError> { let topic = parse_help_topic(topic); Ok(Box::new(HelpCommand(topic))) @@ -146,7 +142,6 @@ pub fn parse_command(input: &str) -> Result, BotError> { "get" => parse_get_variable_command(&cmd_input), "set" => parse_set_variable_command(&cmd_input), "del" => parse_delete_variable_command(&cmd_input), - "resync" => parse_resync(), "r" | "roll" => parse_roll(&cmd_input), "rp" | "pool" => parse_pool_roll(&cmd_input), "cthroll" => parse_cth_roll(&cmd_input), diff --git a/src/logic.rs b/src/logic.rs index 8ce0156..a0386ca 100644 --- a/src/logic.rs +++ b/src/logic.rs @@ -1,57 +1,12 @@ use crate::context::Context; -use crate::db::{Rooms, Variables}; +use crate::db::Variables; use crate::error::{BotError, DiceRollingError}; -use crate::matrix; -use crate::models::RoomInfo; use crate::parser::dice::{Amount, Element}; use argon2::{self, Config, Error as ArgonError}; use futures::stream::{self, StreamExt, TryStreamExt}; -use matrix_sdk::{self, identifiers::RoomId, Client}; use rand::Rng; use std::slice; -/// Record the information about a room, including users in it. -pub async fn record_room_information( - client: &Client, - db: &crate::db::sqlite::Database, - room_id: &RoomId, - room_display_name: &str, - our_username: &str, -) -> Result<(), BotError> { - //Clear out any old room info first. - db.clear_info(room_id.as_str()).await?; - - let room_id_str = room_id.as_str(); - let usernames = matrix::get_users_in_room(&client, &room_id).await?; - - let info = RoomInfo { - room_id: room_id_str.to_owned(), - room_name: room_display_name.to_owned(), - }; - - // TODO this and the username adding should be one whole - // transaction in the db. - db.insert_room_info(&info).await?; - - let filtered_usernames = usernames - .into_iter() - .filter(|username| username != our_username); - - // Async collect into vec of results, then use into_iter of result - // to go to from Result> to just Result<()>. Easier than - // attempting to async-collect our way to a single Result<()>. - stream::iter(filtered_usernames) - .then(|username| async move { - db.add_user_to_room(&username, &room_id_str) - .await - .map_err(|e| e.into()) - }) - .collect::>>() - .await - .into_iter() - .collect() -} - /// Calculate the amount of dice to roll by consulting the database /// and replacing variables with corresponding the amount. Errors out /// if it cannot find a variable defined, or if the database errors. diff --git a/src/matrix.rs b/src/matrix.rs index 6c8e7d8..58d2e53 100644 --- a/src/matrix.rs +++ b/src/matrix.rs @@ -1,5 +1,6 @@ +use futures::stream::{self, StreamExt, TryStreamExt}; use log::error; -use matrix_sdk::events::room::message::NoticeMessageEventContent; +use matrix_sdk::{events::room::message::NoticeMessageEventContent, room::Joined, StoreError}; use matrix_sdk::{ events::room::message::{InReplyTo, Relation}, events::room::message::{MessageEventContent, MessageType}, @@ -7,7 +8,7 @@ use matrix_sdk::{ identifiers::EventId, Error as MatrixError, }; -use matrix_sdk::{identifiers::RoomId, Client}; +use matrix_sdk::{identifiers::RoomId, identifiers::UserId, Client}; /// Extracts more detailed error messages out of a matrix SDK error. fn extract_error_message(error: MatrixError) -> String { @@ -36,6 +37,30 @@ pub async fn get_users_in_room( } } +pub async fn get_rooms_for_user( + client: &Client, + user: &UserId, +) -> Result, MatrixError> { + // Carries errors through, in case we cannot load joined user IDs + // from the room for some reason. + let user_is_in_room = |room: Joined| async move { + match room.joined_user_ids().await { + Ok(users) => match users.contains(user) { + true => Some(Ok(room)), + false => None, + }, + Err(e) => Some(Err(e)), + } + }; + + let rooms_for_user: Vec = stream::iter(client.joined_rooms()) + .filter_map(user_is_in_room) + .try_collect() + .await?; + + Ok(rooms_for_user) +} + pub async fn send_message( client: &Client, room_id: &RoomId, -- 2.40.1 From 97be5d5ccb712a519e583d073393900eb0815955 Mon Sep 17 00:00:00 2001 From: projectmoon Date: Mon, 24 May 2021 22:10:41 +0000 Subject: [PATCH 2/3] Add migration to remove room state management tables. --- .../migrator/migrations/V7__no_more_room_state.rs | 10 ++++++++++ src/matrix.rs | 2 +- 2 files changed, 11 insertions(+), 1 deletion(-) create mode 100644 src/db/sqlite/migrator/migrations/V7__no_more_room_state.rs diff --git a/src/db/sqlite/migrator/migrations/V7__no_more_room_state.rs b/src/db/sqlite/migrator/migrations/V7__no_more_room_state.rs new file mode 100644 index 0000000..e29c52e --- /dev/null +++ b/src/db/sqlite/migrator/migrations/V7__no_more_room_state.rs @@ -0,0 +1,10 @@ +use barrel::backend::Sqlite; +use barrel::{types, Migration}; + +pub fn migration() -> String { + let mut m = Migration::new(); + + m.drop_table_if_exists("room_info"); + m.drop_table_if_exists("room_users"); + m.make::() +} diff --git a/src/matrix.rs b/src/matrix.rs index 58d2e53..5c9f6a8 100644 --- a/src/matrix.rs +++ b/src/matrix.rs @@ -1,6 +1,6 @@ use futures::stream::{self, StreamExt, TryStreamExt}; use log::error; -use matrix_sdk::{events::room::message::NoticeMessageEventContent, room::Joined, StoreError}; +use matrix_sdk::{events::room::message::NoticeMessageEventContent, room::Joined}; use matrix_sdk::{ events::room::message::{InReplyTo, Relation}, events::room::message::{MessageEventContent, MessageType}, -- 2.40.1 From 849a1b6a14d5f68570f42a951f806f82c9aea8e3 Mon Sep 17 00:00:00 2001 From: projectmoon Date: Mon, 24 May 2021 22:25:20 +0000 Subject: [PATCH 3/3] Remove most of Room DB API --- src/db/mod.rs | 18 +- .../migrations/V7__no_more_room_state.rs | 2 +- src/db/sqlite/rooms.rs | 294 ------------------ 3 files changed, 2 insertions(+), 312 deletions(-) diff --git a/src/db/mod.rs b/src/db/mod.rs index e3aac77..f725a56 100644 --- a/src/db/mod.rs +++ b/src/db/mod.rs @@ -2,9 +2,7 @@ use crate::error::BotError; use crate::models::User; use async_trait::async_trait; use errors::DataError; -use std::collections::{HashMap, HashSet}; - -use crate::models::RoomInfo; +use std::collections::HashMap; pub mod errors; pub mod sqlite; @@ -34,20 +32,6 @@ pub(crate) trait Users { #[async_trait] pub(crate) trait Rooms { async fn should_process(&self, room_id: &str, event_id: &str) -> Result; - - async fn insert_room_info(&self, info: &RoomInfo) -> Result<(), DataError>; - - async fn get_room_info(&self, room_id: &str) -> Result, DataError>; - - async fn get_rooms_for_user(&self, user_id: &str) -> Result, DataError>; - - async fn get_users_in_room(&self, room_id: &str) -> Result, DataError>; - - async fn add_user_to_room(&self, username: &str, room_id: &str) -> Result<(), DataError>; - - async fn remove_user_from_room(&self, username: &str, room_id: &str) -> Result<(), DataError>; - - async fn clear_info(&self, room_id: &str) -> Result<(), DataError>; } // TODO move this up to the top once we delete sled. Traits will be the diff --git a/src/db/sqlite/migrator/migrations/V7__no_more_room_state.rs b/src/db/sqlite/migrator/migrations/V7__no_more_room_state.rs index e29c52e..65787eb 100644 --- a/src/db/sqlite/migrator/migrations/V7__no_more_room_state.rs +++ b/src/db/sqlite/migrator/migrations/V7__no_more_room_state.rs @@ -1,5 +1,5 @@ use barrel::backend::Sqlite; -use barrel::{types, Migration}; +use barrel::Migration; pub fn migration() -> String { let mut m = Migration::new(); diff --git a/src/db/sqlite/rooms.rs b/src/db/sqlite/rooms.rs index 937567e..2b09a76 100644 --- a/src/db/sqlite/rooms.rs +++ b/src/db/sqlite/rooms.rs @@ -1,9 +1,7 @@ use super::Database; use crate::db::{errors::DataError, Rooms}; -use crate::models::RoomInfo; use async_trait::async_trait; use sqlx::SqlitePool; -use std::collections::HashSet; use std::time::{SystemTime, UNIX_EPOCH}; async fn record_event(conn: &SqlitePool, room_id: &str, event_id: &str) -> Result<(), DataError> { @@ -49,106 +47,10 @@ impl Rooms for Database { } } } - - async fn insert_room_info(&self, info: &RoomInfo) -> Result<(), DataError> { - sqlx::query( - r#"INSERT INTO room_info (room_id, room_name) VALUES (?, ?) - ON CONFLICT(room_id) DO UPDATE SET room_name = ?"#, - ) - .bind(&info.room_id) - .bind(&info.room_name) - .bind(&info.room_name) - .execute(&self.conn) - .await?; - - Ok(()) - } - - async fn get_room_info(&self, room_id: &str) -> Result, DataError> { - let info = sqlx::query!( - r#"SELECT room_id, room_name FROM room_info - WHERE room_id = ?"#, - room_id - ) - .fetch_optional(&self.conn) - .await?; - - Ok(info.map(|i| RoomInfo { - room_id: i.room_id, - room_name: i.room_name, - })) - } - - async fn get_rooms_for_user(&self, user_id: &str) -> Result, DataError> { - let room_ids = sqlx::query!( - r#"SELECT room_id FROM room_users - WHERE username = ?"#, - user_id - ) - .fetch_all(&self.conn) - .await?; - - Ok(room_ids.into_iter().map(|row| row.room_id).collect()) - } - - async fn get_users_in_room(&self, room_id: &str) -> Result, DataError> { - let usernames = sqlx::query!( - r#"SELECT username FROM room_users - WHERE room_id = ?"#, - room_id - ) - .fetch_all(&self.conn) - .await?; - - Ok(usernames.into_iter().map(|row| row.username).collect()) - } - - async fn add_user_to_room(&self, username: &str, room_id: &str) -> Result<(), DataError> { - sqlx::query( - "INSERT INTO room_users (room_id, username) VALUES (?, ?) - ON CONFLICT DO NOTHING", - ) - .bind(room_id) - .bind(username) - .execute(&self.conn) - .await?; - - Ok(()) - } - - async fn remove_user_from_room(&self, username: &str, room_id: &str) -> Result<(), DataError> { - sqlx::query("DELETE FROM room_users where username = ? AND room_id = ?") - .bind(username) - .bind(room_id) - .execute(&self.conn) - .await?; - - Ok(()) - } - - async fn clear_info(&self, room_id: &str) -> Result<(), DataError> { - // We do not clear event history here, because if we rejoin a - // room, we would re-process events we've already seen. - let mut tx = self.conn.begin().await?; - - sqlx::query("DELETE FROM room_info where room_id = ?") - .bind(room_id) - .execute(&mut tx) - .await?; - - sqlx::query("DELETE FROM room_users where room_id = ?") - .bind(room_id) - .execute(&mut tx) - .await?; - - tx.commit().await?; - Ok(()) - } } #[cfg(test)] mod tests { - use super::*; use crate::db::sqlite::Database; use crate::db::Rooms; @@ -181,200 +83,4 @@ mod tests { assert_eq!(second_check, false); } - - #[tokio::test(flavor = "multi_thread", worker_threads = 1)] - async fn insert_and_get_room_info_test() { - let db = create_db().await; - - let info = RoomInfo { - room_id: "myroomid".to_string(), - room_name: "myroomname".to_string(), - }; - - db.insert_room_info(&info) - .await - .expect("Could not insert room info."); - - let retrieved_info = db - .get_room_info("myroomid") - .await - .expect("Could not retrieve room info."); - - assert!(retrieved_info.is_some()); - assert_eq!(info, retrieved_info.unwrap()); - } - - #[tokio::test(flavor = "multi_thread", worker_threads = 1)] - async fn insert_room_info_updates_existing() { - let db = create_db().await; - - let info1 = RoomInfo { - room_id: "myroomid".to_string(), - room_name: "myroomname".to_string(), - }; - - db.insert_room_info(&info1) - .await - .expect("Could not insert room info1."); - - let info2 = RoomInfo { - room_id: "myroomid".to_string(), - room_name: "myroomname2".to_string(), - }; - - db.insert_room_info(&info2) - .await - .expect("Could not update room info after first insert"); - - let retrieved_info = db - .get_room_info("myroomid") - .await - .expect("Could not get room info"); - - assert!(retrieved_info.is_some()); - let retrieved_info = retrieved_info.unwrap(); - - assert_eq!(retrieved_info.room_id, "myroomid"); - assert_eq!(retrieved_info.room_name, "myroomname2"); - } - - #[tokio::test(flavor = "multi_thread", worker_threads = 1)] - async fn add_user_to_room_test() { - let db = create_db().await; - - db.add_user_to_room("myuser", "myroom") - .await - .expect("Could not add user to room."); - - let users_in_room = db - .get_users_in_room("myroom") - .await - .expect("Could not get users in room."); - - assert_eq!(users_in_room.len(), 1); - assert!(users_in_room.contains("myuser")); - - let rooms_for_user = db - .get_rooms_for_user("myuser") - .await - .expect("Could not get rooms for user"); - - assert_eq!(rooms_for_user.len(), 1); - assert!(rooms_for_user.contains("myroom")); - } - - #[tokio::test(flavor = "multi_thread", worker_threads = 1)] - async fn add_user_to_room_does_not_have_constraint_violation() { - let db = create_db().await; - - db.add_user_to_room("myuser", "myroom") - .await - .expect("Could not add user to room."); - - let second_attempt = db.add_user_to_room("myuser", "myroom").await; - - assert!(second_attempt.is_ok()); - - let users_in_room = db - .get_users_in_room("myroom") - .await - .expect("Could not get users in room."); - - assert_eq!(users_in_room.len(), 1); - assert!(users_in_room.contains("myuser")); - } - - #[tokio::test(flavor = "multi_thread", worker_threads = 1)] - async fn remove_user_from_room_test() { - let db = create_db().await; - - db.add_user_to_room("myuser", "myroom") - .await - .expect("Could not add user to room."); - - let remove_attempt = db.remove_user_from_room("myuser", "myroom").await; - - assert!(remove_attempt.is_ok()); - - let users_in_room = db - .get_users_in_room("myroom") - .await - .expect("Could not get users in room."); - - assert_eq!(users_in_room.len(), 0); - } - - #[tokio::test(flavor = "multi_thread", worker_threads = 1)] - async fn clear_info_does_not_delete_other_rooms() { - let db = create_db().await; - - let info1 = RoomInfo { - room_id: "myroomid".to_string(), - room_name: "myroomname".to_string(), - }; - - let info2 = RoomInfo { - room_id: "myroomid2".to_string(), - room_name: "myroomname2".to_string(), - }; - - db.insert_room_info(&info1) - .await - .expect("Could not insert room info1."); - - db.insert_room_info(&info2) - .await - .expect("Could not insert room info2."); - - db.add_user_to_room("myuser", &info1.room_id) - .await - .expect("Could not add user to room."); - - db.clear_info(&info1.room_id) - .await - .expect("Could not clear room info1"); - - let room_info2 = db - .get_room_info(&info2.room_id) - .await - .expect("Could not get room info2."); - - assert!(room_info2.is_some()); - } - - #[tokio::test(flavor = "multi_thread", worker_threads = 1)] - async fn clear_info_test() { - let db = create_db().await; - - let info = RoomInfo { - room_id: "myroomid".to_string(), - room_name: "myroomname".to_string(), - }; - - db.insert_room_info(&info) - .await - .expect("Could not insert room info."); - - db.add_user_to_room("myuser", &info.room_id) - .await - .expect("Could not add user to room."); - - db.clear_info(&info.room_id) - .await - .expect("Could not clear room info"); - - let users_in_room = db - .get_users_in_room(&info.room_id) - .await - .expect("Could not get users in room."); - - assert_eq!(users_in_room.len(), 0); - - let room_info = db - .get_room_info(&info.room_id) - .await - .expect("Could not get room info."); - - assert!(room_info.is_none()); - } } -- 2.40.1