From bfc5609ab6704a82028ff4d51ca524bdbcf1ca82 Mon Sep 17 00:00:00 2001 From: projectmoon Date: Sun, 16 May 2021 21:39:19 +0000 Subject: [PATCH] Add proper constraints to db tables. Report errors when listing users. --- src/bot.rs | 18 ++++++---- src/bot/event_handlers.rs | 12 +++++-- src/error.rs | 4 +-- src/logic.rs | 18 +++++++--- src/matrix.rs | 13 +++++--- src/migrator/migrations/V1__variables.rs | 1 - src/migrator/migrations/V2__rooms.rs | 26 ++++++++------- src/migrator/migrations/V4__room_events.rs | 38 ++++++++++++++++++++++ 8 files changed, 97 insertions(+), 33 deletions(-) create mode 100644 src/migrator/migrations/V4__room_events.rs diff --git a/src/bot.rs b/src/bot.rs index b941789..926a76a 100644 --- a/src/bot.rs +++ b/src/bot.rs @@ -161,19 +161,25 @@ impl DiceBot { let client = self.client.clone(); self.login(&client).await?; + // Initial sync without event handler prevents responding to + // messages received while bot was offline. TODO: selectively + // respond to old messages? e.g. comands missed while offline. + //info!("Performing intial sync (no commands will be responded to)"); + // self.client.sync_once(SyncSettings::default()).await?; + client.set_event_handler(Box::new(self)).await; info!("Listening for commands"); - let token = client - .sync_token() - .await - .ok_or(BotError::SyncTokenRequired)?; + // let token = client + // .sync_token() + // .await + // .ok_or(BotError::SyncTokenRequired)?; - let settings = SyncSettings::default().token(token); + // let settings = SyncSettings::default().token(token); // TODO replace with sync_with_callback for cleaner shutdown // process. - client.sync(settings).await; + client.sync(SyncSettings::default()).await; Ok(()) } diff --git a/src/bot/event_handlers.rs b/src/bot/event_handlers.rs index 7ceb1b5..24ac3c1 100644 --- a/src/bot/event_handlers.rs +++ b/src/bot/event_handlers.rs @@ -136,7 +136,7 @@ impl EventHandler for DiceBot { let result = if event_affects_us && !adding_user { info!("Clearing all information for room ID {}", room_id); - self.db.clear_info(room_id_str).await + self.db.clear_info(room_id_str).await.map_err(|e| e.into()) } else if event_affects_us && adding_user { info!("Joined room {}; recording room information", room_id); record_room_information( @@ -149,10 +149,16 @@ impl EventHandler for DiceBot { .await } else if !event_affects_us && adding_user { info!("Adding user {} to room ID {}", username, room_id); - self.db.add_user_to_room(username, room_id_str).await + self.db + .add_user_to_room(username, room_id_str) + .await + .map_err(|e| e.into()) } else if !event_affects_us && !adding_user { info!("Removing user {} from room ID {}", username, room_id); - self.db.remove_user_from_room(username, room_id_str).await + 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(()) diff --git a/src/error.rs b/src/error.rs index 3efd786..f091d98 100644 --- a/src/error.rs +++ b/src/error.rs @@ -36,10 +36,10 @@ pub enum BotError { #[error("error in matrix state store: {0}")] MatrixStateStoreError(#[from] matrix_sdk::StoreError), - #[error("uncategorized matrix SDK error")] + #[error("uncategorized matrix SDK error: {0}")] MatrixError(#[from] matrix_sdk::Error), - #[error("uncategorized matrix SDK base error")] + #[error("uncategorized matrix SDK base error: {0}")] MatrixBaseError(#[from] matrix_sdk::BaseError), #[error("future canceled")] diff --git a/src/logic.rs b/src/logic.rs index 8375754..5be59a6 100644 --- a/src/logic.rs +++ b/src/logic.rs @@ -1,5 +1,6 @@ use crate::db::sqlite::errors::DataError; use crate::db::sqlite::Rooms; +use crate::error::BotError; use crate::matrix; use crate::models::RoomInfo; use futures::stream::{self, StreamExt, TryStreamExt}; @@ -12,9 +13,12 @@ pub async fn record_room_information( room_id: &RoomId, room_display_name: &str, our_username: &str, -) -> Result<(), DataError> { +) -> 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 usernames = matrix::get_users_in_room(&client, &room_id).await?; let info = RoomInfo { room_id: room_id_str.to_owned(), @@ -29,12 +33,18 @@ pub async fn record_room_information( .into_iter() .filter(|username| username != our_username); + println!("Users to add to room: {:?}", filtered_usernames); + // 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 }) - .collect::>>() + .then(|username| async move { + db.add_user_to_room(&username, &room_id_str) + .await + .map_err(|e| e.into()) + }) + .collect::>>() .await .into_iter() .collect() diff --git a/src/matrix.rs b/src/matrix.rs index 173a2ae..7ff73c4 100644 --- a/src/matrix.rs +++ b/src/matrix.rs @@ -20,16 +20,19 @@ fn extract_error_message(error: MatrixError) -> String { } /// Retrieve a list of users in a given room. -pub async fn get_users_in_room(client: &Client, room_id: &RoomId) -> Vec { +pub async fn get_users_in_room( + client: &Client, + room_id: &RoomId, +) -> Result, MatrixError> { if let Some(joined_room) = client.get_joined_room(room_id) { - let members = joined_room.joined_members().await.ok().unwrap_or_default(); + let members = joined_room.joined_members().await?; - members + Ok(members .into_iter() .map(|member| member.user_id().to_string()) - .collect() + .collect()) } else { - vec![] + Ok(vec![]) } } diff --git a/src/migrator/migrations/V1__variables.rs b/src/migrator/migrations/V1__variables.rs index 298233a..ec8f063 100644 --- a/src/migrator/migrations/V1__variables.rs +++ b/src/migrator/migrations/V1__variables.rs @@ -7,7 +7,6 @@ pub fn migration() -> String { info!("Applying migration: {}", file!()); m.create_table("user_variables", |t| { - t.add_column("id", types::primary()); t.add_column("room_id", types::text()); t.add_column("user_id", types::text()); t.add_column("key", types::text()); diff --git a/src/migrator/migrations/V2__rooms.rs b/src/migrator/migrations/V2__rooms.rs index 14f463b..5568a01 100644 --- a/src/migrator/migrations/V2__rooms.rs +++ b/src/migrator/migrations/V2__rooms.rs @@ -1,31 +1,33 @@ use barrel::backend::Sqlite; -use barrel::{types, Migration}; +use barrel::{types, types::Type, Migration}; use log::info; +fn primary_uuid() -> Type { + types::text().unique(true).primary(true).nullable(false) +} + +fn autoincrement_int() -> Type { + types::integer() + .increments(true) + .unique(true) + .primary(false) +} + pub fn migration() -> String { let mut m = Migration::new(); info!("Applying migration: {}", file!()); //Table for basic room information: room ID, room name m.create_table("room_info", move |t| { - t.add_column("id", types::primary()); - t.add_column("room_id", types::text()); + t.add_column("room_id", primary_uuid()); t.add_column("room_name", types::text()); }); //Table of users in rooms. m.create_table("room_users", move |t| { - t.add_column("id", types::primary()); - t.add_column("room_id", types::text()); + t.add_column("room_id", autoincrement_int()); t.add_column("username", types::text()); }); - //Table of room ID, event ID, event timestamp - m.create_table("room_events", move |t| { - t.add_column("id", types::primary()); - t.add_column("room_id", types::text()); - t.add_column("event_id", types::text()); - t.add_column("event_timestamp", types::integer()); - }); m.make::() } diff --git a/src/migrator/migrations/V4__room_events.rs b/src/migrator/migrations/V4__room_events.rs new file mode 100644 index 0000000..d9d487b --- /dev/null +++ b/src/migrator/migrations/V4__room_events.rs @@ -0,0 +1,38 @@ +use barrel::backend::Sqlite; +use barrel::{types, types::Type, Migration}; +use log::info; + +fn primary_uuid() -> Type { + types::text().unique(true).nullable(false) +} + +fn autoincrement_int() -> Type { + types::integer() + .increments(true) + .unique(true) + .primary(false) +} + +pub fn migration() -> String { + let mut m = Migration::new(); + info!("Applying migration: {}", file!()); + + //Table of room ID, event ID, event timestamp + m.create_table("room_events", move |t| { + t.add_column("room_id", types::text().nullable(false)); + t.add_column("event_id", types::text().nullable(false)); + t.add_column("event_timestamp", types::integer()); + }); + + let mut res = m.make::(); + + //This is a hack that gives us a composite primary key. + if res.ends_with(");") { + res.pop(); + res.pop(); + } + + let x = format!("{}, PRIMARY KEY (room_id, event_id));", res); + println!("{}", x); + x +}