Merge pull request #68 from rust-lang/Cleanup

Refactor some code to be more maintainable

Author: technetos

src/api.rs

@@ -1,14 +1,9 @@
-use crate::{
-    commands::{Args, Result},
-    db::DB,
-    schema::roles,
-    CommandHistory,
-};
+use crate::{command_history::CommandHistory, commands::Args, db::DB, schema::roles, Error};
 use diesel::prelude::*;
 use serenity::{model::prelude::*, utils::parse_username};
 
 /// Send a reply to the channel the message was received on.  
-pub(crate) fn send_reply(args: &Args, message: &str) -> Result<()> {
+pub(crate) fn send_reply(args: &Args, message: &str) -> Result<(), Error> {
     if let Some(response_id) = response_exists(args) {
         info!("editing message: {:?}", response_id);
         args.msg
@@ -33,7 +28,7 @@ fn response_exists(args: &Args) -> Option<MessageId> {
 }
 
 /// Determine if a member sending a message has the `Role`.  
-pub(crate) fn has_role(args: &Args, role: &RoleId) -> Result<bool> {
+pub(crate) fn has_role(args: &Args, role: &RoleId) -> Result<bool, Error> {
     Ok(args
         .msg
         .member
@@ -43,7 +38,7 @@ pub(crate) fn has_role(args: &Args, role: &RoleId) -> Result<bool> {
         .contains(role))
 }
 
-fn check_permission(args: &Args, role: Option<String>) -> Result<bool> {
+fn check_permission(args: &Args, role: Option<String>) -> Result<bool, Error> {
     use std::str::FromStr;
     if let Some(role_id) = role {
         Ok(has_role(args, &RoleId::from(u64::from_str(&role_id)?))?)
@@ -53,7 +48,7 @@ fn check_permission(args: &Args, role: Option<String>) -> Result<bool> {
 }
 
 /// Return whether or not the user is a mod.  
-pub(crate) fn is_mod(args: &Args) -> Result<bool> {
+pub(crate) fn is_mod(args: &Args) -> Result<bool, Error> {
     let role = roles::table
         .filter(roles::name.eq("mod"))
         .first::<(i32, String, String)>(&DB.get()?)
@@ -62,7 +57,7 @@ pub(crate) fn is_mod(args: &Args) -> Result<bool> {
     check_permission(args, role.map(|(_, role_id, _)| role_id))
 }
 
-pub(crate) fn is_wg_and_teams(args: &Args) -> Result<bool> {
+pub(crate) fn is_wg_and_teams(args: &Args) -> Result<bool, Error> {
     let role = roles::table
         .filter(roles::name.eq("wg_and_teams"))
         .first::<(i32, String, String)>(&DB.get()?)
@@ -74,7 +69,7 @@ pub(crate) fn is_wg_and_teams(args: &Args) -> Result<bool> {
 /// Set slow mode for a channel.  
 ///
 /// A `seconds` value of 0 will disable slowmode
-pub(crate) fn slow_mode(args: Args) -> Result<()> {
+pub(crate) fn slow_mode(args: Args) -> Result<(), Error> {
     use std::str::FromStr;
 
     if is_mod(&args)? {
@@ -95,7 +90,7 @@ pub(crate) fn slow_mode(args: Args) -> Result<()> {
     Ok(())
 }
 
-pub(crate) fn slow_mode_help(args: Args) -> Result<()> {
+pub(crate) fn slow_mode_help(args: Args) -> Result<(), Error> {
     let help_string = "
 Set slowmode on a channel
 ```
@@ -119,7 +114,7 @@ will disable slowmode on the `#bot-usage` channel.";
 /// Kick a user from the guild.  
 ///
 /// Requires the kick members permission
-pub(crate) fn kick(args: Args) -> Result<()> {
+pub(crate) fn kick(args: Args) -> Result<(), Error> {
     if is_mod(&args)? {
         let user_id = parse_username(
             &args
@@ -137,7 +132,7 @@ pub(crate) fn kick(args: Args) -> Result<()> {
     Ok(())
 }
 
-pub(crate) fn kick_help(args: Args) -> Result<()> {
+pub(crate) fn kick_help(args: Args) -> Result<(), Error> {
     let help_string = "
 Kick a user from the guild
 ```

src/ban.rs

@@ -1,23 +1,11 @@
-use super::CommandHistory;
 use crate::{
-    api,
-    commands::{Args, Result},
-    db::DB,
-    schema::bans,
-    text::ban_message,
+    api, commands::Args, db::DB, schema::bans, text::ban_message, Error, SendSyncError, HOUR,
 };
 use diesel::prelude::*;
 use serenity::{model::prelude::*, prelude::*, utils::parse_username};
-use std::{
-    sync::atomic::{AtomicBool, Ordering},
-    thread::sleep,
-    time::{Duration, SystemTime},
-};
-
-const HOUR: u64 = 3600;
-static UNBAN_THREAD_INITIALIZED: AtomicBool = AtomicBool::new(false);
+use std::time::{Duration, SystemTime};
 
-pub(crate) fn save_ban(user_id: String, guild_id: String, hours: u64) -> Result<()> {
+pub(crate) fn save_ban(user_id: String, guild_id: String, hours: u64) -> Result<(), Error> {
     info!("Recording ban for user {}", &user_id);
     let conn = DB.get()?;
     diesel::insert_into(bans::table)
@@ -34,7 +22,7 @@ pub(crate) fn save_ban(user_id: String, guild_id: String, hours: u64) -> Result<
     Ok(())
 }
 
-pub(crate) fn save_unban(user_id: String, guild_id: String) -> Result<()> {
+pub(crate) fn save_unban(user_id: String, guild_id: String) -> Result<(), Error> {
     info!("Recording unban for user {}", &user_id);
     let conn = DB.get()?;
     diesel::update(bans::table)
@@ -49,49 +37,30 @@ pub(crate) fn save_unban(user_id: String, guild_id: String) -> Result<()> {
     Ok(())
 }
 
-pub(crate) fn start_cleanup_thread(cx: Context) {
+pub(crate) fn unban_users(cx: &Context) -> Result<(), SendSyncError> {
     use std::str::FromStr;
-    if !UNBAN_THREAD_INITIALIZED.load(Ordering::SeqCst) {
-        UNBAN_THREAD_INITIALIZED.store(true, Ordering::SeqCst);
-        type SendSyncError = Box<dyn std::error::Error + Send + Sync>;
-        std::thread::spawn(move || -> std::result::Result<(), SendSyncError> {
-            loop {
-                let conn = DB.get()?;
-                let to_unban = bans::table
-                    .filter(
-                        bans::unbanned
-                            .eq(false)
-                            .and(bans::end_time.le(SystemTime::now())),
-                    )
-                    .load::<(i32, String, String, bool, SystemTime, SystemTime)>(&conn)?;
-
-                for row in &to_unban {
-                    let guild_id = GuildId::from(u64::from_str(&row.2)?);
-                    info!("Unbanning user {}", &row.1);
-                    guild_id.unban(&cx, u64::from_str(&row.1)?)?;
-                }
-
-                let mut data = cx.data.write();
-                let history = data.get_mut::<CommandHistory>().unwrap();
-
-                // always keep the last command in history
-                if history.len() > 0 {
-                    info!("Clearing command history");
-                    history.drain(..history.len() - 1);
-                }
-
-                drop(data);
-
-                sleep(Duration::new(HOUR, 0));
-            }
-        });
+
+    let conn = DB.get()?;
+    let to_unban = bans::table
+        .filter(
+            bans::unbanned
+                .eq(false)
+                .and(bans::end_time.le(SystemTime::now())),
+        )
+        .load::<(i32, String, String, bool, SystemTime, SystemTime)>(&conn)?;
+
+    for row in &to_unban {
+        let guild_id = GuildId::from(u64::from_str(&row.2)?);
+        info!("Unbanning user {}", &row.1);
+        guild_id.unban(&cx, u64::from_str(&row.1)?)?;
     }
+    Ok(())
 }
 
 /// Temporarily ban an user from the guild.  
 ///
 /// Requires the ban members permission
-pub(crate) fn temp_ban(args: Args) -> Result<()> {
+pub(crate) fn temp_ban(args: Args) -> Result<(), Error> {
     let user_id = parse_username(
         &args
             .params
@@ -131,7 +100,7 @@ pub(crate) fn temp_ban(args: Args) -> Result<()> {
     Ok(())
 }
 
-pub(crate) fn help(args: Args) -> Result<()> {
+pub(crate) fn help(args: Args) -> Result<(), Error> {
     let hours = 24;
     let reason = "violating the code of conduct";
 

src/command_history.rs

@@ -0,0 +1,57 @@
+use crate::{
+    commands::{Commands, PREFIX},
+    Error, SendSyncError, HOUR,
+};
+use indexmap::IndexMap;
+use serenity::{model::prelude::*, prelude::*, utils::CustomMessage};
+use std::time::Duration;
+
+const MESSAGE_AGE_MAX: Duration = Duration::from_secs(HOUR);
+
+pub(crate) struct CommandHistory;
+
+impl TypeMapKey for CommandHistory {
+    type Value = IndexMap<MessageId, MessageId>;
+}
+
+pub(crate) fn replay_message(
+    cx: Context,
+    ev: MessageUpdateEvent,
+    cmds: &Commands,
+) -> Result<(), Error> {
+    let age = ev.timestamp.and_then(|create| {
+        ev.edited_timestamp
+            .and_then(|edit| edit.signed_duration_since(create).to_std().ok())
+    });
+
+    if age.is_some() && age.unwrap() < MESSAGE_AGE_MAX {
+        let mut msg = CustomMessage::new();
+        msg.id(ev.id)
+            .channel_id(ev.channel_id)
+            .content(ev.content.unwrap_or_else(|| String::new()));
+
+        let msg = msg.build();
+
+        if msg.content.starts_with(PREFIX) {
+            info!(
+                "sending edited message - {:?} {:?}",
+                msg.content, msg.author
+            );
+            cmds.execute(cx, &msg);
+        }
+    }
+
+    Ok(())
+}
+
+pub(crate) fn clear_command_history(cx: &Context) -> Result<(), SendSyncError> {
+    let mut data = cx.data.write();
+    let history = data.get_mut::<CommandHistory>().unwrap();
+
+    // always keep the last command in history
+    if history.len() > 0 {
+        info!("Clearing command history");
+        history.drain(..history.len() - 1);
+    }
+    Ok(())
+}

src/commands.rs

@@ -1,27 +1,27 @@
 use crate::{
     api,
     state_machine::{CharacterSet, StateMachine},
+    Error,
 };
 use indexmap::IndexMap;
 use reqwest::blocking::Client as HttpClient;
 use serenity::{model::channel::Message, prelude::Context};
 use std::{collections::HashMap, sync::Arc};
 
 pub(crate) const PREFIX: &'static str = "?";
-pub(crate) type Result<T> = std::result::Result<T, Box<dyn std::error::Error>>;
-pub(crate) type GuardFn = fn(&Args) -> Result<bool>;
+pub(crate) type GuardFn = fn(&Args) -> Result<bool, Error>;
 
 struct Command {
     guard: GuardFn,
-    ptr: Box<dyn for<'m> Fn(Args<'m>) -> Result<()> + Send + Sync>,
+    ptr: Box<dyn for<'m> Fn(Args<'m>) -> Result<(), Error> + Send + Sync>,
 }
 
 impl Command {
-    fn authorize(&self, args: &Args) -> Result<bool> {
+    fn authorize(&self, args: &Args) -> Result<bool, Error> {
         (self.guard)(&args)
     }
 
-    fn call(&self, args: Args) -> Result<()> {
+    fn call(&self, args: Args) -> Result<(), Error> {
         (self.ptr)(args)
     }
 }
@@ -51,15 +51,15 @@ impl Commands {
     pub(crate) fn add(
         &mut self,
         command: &'static str,
-        handler: impl Fn(Args) -> Result<()> + Send + Sync + 'static,
+        handler: impl Fn(Args) -> Result<(), Error> + Send + Sync + 'static,
     ) {
         self.add_protected(command, handler, |_| Ok(true));
     }
 
     pub(crate) fn add_protected(
         &mut self,
         command: &'static str,
-        handler: impl Fn(Args) -> Result<()> + Send + Sync + 'static,
+        handler: impl Fn(Args) -> Result<(), Error> + Send + Sync + 'static,
         guard: GuardFn,
     ) {
         info!("Adding command {}", &command);
@@ -130,7 +130,7 @@ impl Commands {
         &mut self,
         cmd: &'static str,
         desc: &'static str,
-        handler: impl Fn(Args) -> Result<()> + Send + Sync + 'static,
+        handler: impl Fn(Args) -> Result<(), Error> + Send + Sync + 'static,
     ) {
         self.help_protected(cmd, desc, handler, |_| Ok(true));
     }
@@ -139,7 +139,7 @@ impl Commands {
         &mut self,
         cmd: &'static str,
         desc: &'static str,
-        handler: impl Fn(Args) -> Result<()> + Send + Sync + 'static,
+        handler: impl Fn(Args) -> Result<(), Error> + Send + Sync + 'static,
         guard: GuardFn,
     ) {
         let base_cmd = &cmd[1..];

src/crates.rs

@@ -1,7 +1,4 @@
-use crate::{
-    api,
-    commands::{Args, Result},
-};
+use crate::{api, commands::Args, Error};
 
 use reqwest::header;
 use serde::Deserialize;
@@ -25,7 +22,7 @@ struct Crate {
     documentation: Option<String>,
 }
 
-fn get_crate(args: &Args) -> Result<Option<Crate>> {
+fn get_crate(args: &Args) -> Result<Option<Crate>, Error> {
     let query = args
         .params
         .get("query")
@@ -44,7 +41,7 @@ fn get_crate(args: &Args) -> Result<Option<Crate>> {
     Ok(crate_list.crates.into_iter().nth(0))
 }
 
-pub fn search(args: Args) -> Result<()> {
+pub fn search(args: Args) -> Result<(), Error> {
     if let Some(krate) = get_crate(&args)? {
         args.msg.channel_id.send_message(&args.cx, |m| {
             m.embed(|e| {
@@ -79,7 +76,7 @@ fn rustc_crate(crate_name: &str) -> Option<&str> {
     }
 }
 
-pub fn doc_search(args: Args) -> Result<()> {
+pub fn doc_search(args: Args) -> Result<(), Error> {
     let query = args
         .params
         .get("query")
@@ -114,7 +111,7 @@ pub fn doc_search(args: Args) -> Result<()> {
 }
 
 /// Print the help message
-pub fn help(args: Args) -> Result<()> {
+pub fn help(args: Args) -> Result<(), Error> {
     let help_string = "search for a crate on crates.io
 ```
 ?crate query...
@@ -124,7 +121,7 @@ pub fn help(args: Args) -> Result<()> {
 }
 
 /// Print the help message
-pub fn doc_help(args: Args) -> Result<()> {
+pub fn doc_help(args: Args) -> Result<(), Error> {
     let help_string = "retrieve documentation for a given crate
 ```
 ?docs crate_name...

src/db.rs

@@ -1,4 +1,4 @@
-use crate::commands::Result;
+use crate::Error;
 use diesel::prelude::*;
 use diesel::r2d2;
 use lazy_static::lazy_static;
@@ -12,7 +12,7 @@ lazy_static! {
     .expect("Unable to connect to database");
 }
 
-pub(crate) fn run_migrations() -> Result<()> {
+pub(crate) fn run_migrations() -> Result<(), Error> {
     let conn = PgConnection::establish(&std::env::var("DATABASE_URL")?)?;
 
     diesel_migrations::embed_migrations!();