From f352c90b6b997f74052855c2f6aa114b5981646b Mon Sep 17 00:00:00 2001 From: projectmoon Date: Thu, 12 Nov 2020 21:05:14 +0000 Subject: [PATCH] Return error on unrecognized commands. --- src/bin/dicebot-cmd.rs | 2 +- src/bot.rs | 5 ++-- src/commands.rs | 67 +++++++----------------------------------- src/commands/parser.rs | 41 ++++++++++++++++++++++++-- 4 files changed, 52 insertions(+), 63 deletions(-) diff --git a/src/bin/dicebot-cmd.rs b/src/bin/dicebot-cmd.rs index a57622a..6add6ab 100644 --- a/src/bin/dicebot-cmd.rs +++ b/src/bin/dicebot-cmd.rs @@ -7,7 +7,7 @@ use chronicle_dicebot::error::BotError; async fn main() -> Result<(), BotError> { let db = Database::new_temp()?; let input = std::env::args().skip(1).collect::>().join(" "); - let command = match commands::parse(&input) { + let command = match commands::parser::parse_command(&input) { Ok(command) => command, Err(e) => return Err(e), }; diff --git a/src/bot.rs b/src/bot.rs index 3e29162..5eb9b42 100644 --- a/src/bot.rs +++ b/src/bot.rs @@ -128,9 +128,8 @@ impl DiceBot { for command in commands { let ctx = Context::new(&self.db, &room_id.as_str(), &sender_username, &command); - if let Some(cmd_result) = execute_command(&ctx).await { - results.push(cmd_result); - } + let cmd_result = execute_command(&ctx).await; + results.push(cmd_result); } if results.len() >= 1 { diff --git a/src/commands.rs b/src/commands.rs index 991991e..0a30fbf 100644 --- a/src/commands.rs +++ b/src/commands.rs @@ -1,7 +1,5 @@ use crate::context::Context; -use crate::error::{BotError, BotError::CommandParsingError}; use async_trait::async_trait; -use parser::CommandParsingError::UnrecognizedCommand; use thiserror::Error; pub mod basic_rolling; @@ -41,20 +39,6 @@ pub trait Command: Send + Sync { fn name(&self) -> &'static str; } -/// Parse a command string into a dynamic command execution trait -/// object. Returns an error if a command was recognized but not -/// parsed correctly. Returns IgnoredCommand error if no command was -/// recognized. -pub fn parse(s: &str) -> Result, BotError> { - match parser::parse_command(s) { - Ok(command) => Ok(command), - Err(CommandParsingError(UnrecognizedCommand(_))) => { - Err(CommandError::IgnoredCommand.into()) - } - Err(e) => Err(e), - } -} - #[derive(Debug)] pub struct CommandResult { pub plain: String, @@ -65,15 +49,14 @@ pub struct CommandResult { /// go back to Matrix, if the command was executed (successfully or /// not). If a command is determined to be ignored, this function will /// return None, signifying that we should not send a response. -pub async fn execute_command(ctx: &Context<'_>) -> Option { - let res = parse(&ctx.message_body); +pub async fn execute_command(ctx: &Context<'_>) -> CommandResult { + let res = parser::parse_command(&ctx.message_body); let (plain, html) = match res { Ok(cmd) => { let execution = cmd.execute(ctx).await; (execution.plain().into(), execution.html().into()) } - Err(BotError::CommandError(CommandError::IgnoredCommand)) => return None, Err(e) => { let message = format!("Error parsing command: {}", e); let html_message = format!("

{}

", message); @@ -84,51 +67,21 @@ pub async fn execute_command(ctx: &Context<'_>) -> Option { let plain = format!("{}\n{}", ctx.username, plain); let html = format!("

{}

\n{}", ctx.username, html); - Some(CommandResult { + CommandResult { plain: plain, html: html, - }) + } } #[cfg(test)] mod tests { use super::*; - #[test] - fn chance_die_is_not_malformed() { - assert!(parse("!chance").is_ok()); - } - - #[test] - fn roll_malformed_expression_test() { - assert!(parse("!roll 1d20asdlfkj").is_err()); - assert!(parse("!roll 1d20asdlfkj ").is_err()); - } - - #[test] - fn roll_dice_pool_malformed_expression_test() { - assert!(parse("!pool 8abc").is_err()); - assert!(parse("!pool 8abc ").is_err()); - } - - #[test] - fn pool_whitespace_test() { - parse("!pool ns3:8 ").expect("was error"); - parse(" !pool ns3:8").expect("was error"); - parse(" !pool ns3:8 ").expect("was error"); - } - - #[test] - fn help_whitespace_test() { - parse("!help stuff ").expect("was error"); - parse(" !help stuff").expect("was error"); - parse(" !help stuff ").expect("was error"); - } - - #[test] - fn roll_whitespace_test() { - parse("!roll 1d4 + 5d6 -3 ").expect("was error"); - parse("!roll 1d4 + 5d6 -3 ").expect("was error"); - parse(" !roll 1d4 + 5d6 -3 ").expect("was error"); + #[tokio::test] + async fn unrecognized_command() { + let db = crate::db::Database::new_temp().unwrap(); + let ctx = Context::new(&db, "myroomid", "@testuser:example.com", "!notacommand"); + let result = execute_command(&ctx).await; + assert!(result.plain.contains("Error")); } } diff --git a/src/commands/parser.rs b/src/commands/parser.rs index 92b538e..9bd4f6a 100644 --- a/src/commands/parser.rs +++ b/src/commands/parser.rs @@ -116,7 +116,7 @@ fn split_command(input: &str) -> Result<(String, String), CommandParsingError> { /// Potentially parse a command expression. If we recognize the /// command, an error should be raised if the command is misparsed. If -/// we don't recognize the command, ignore it and return None. +/// we don't recognize the command, return an error. pub fn parse_command(input: &str) -> Result, BotError> { match split_command(input) { Ok((cmd, cmd_input)) => match cmd.as_ref() { @@ -130,7 +130,6 @@ pub fn parse_command(input: &str) -> Result, BotError> { "cthadv" | "cthARoll" => parse_cth_advancement_roll(&cmd_input), "chance" => chance_die(), "help" => help(&cmd_input), - // No recognized command, ignore this. _ => Err(CommandParsingError::UnrecognizedCommand(cmd).into()), }, //All other errors passed up. @@ -240,4 +239,42 @@ mod tests { assert!(split_command("roll 1d4").is_err()); assert!(split_command("roll").is_err()); } + + #[test] + fn chance_die_is_not_malformed() { + assert!(parse_command("!chance").is_ok()); + } + + #[test] + fn roll_malformed_expression_test() { + assert!(parse_command("!roll 1d20asdlfkj").is_err()); + assert!(parse_command("!roll 1d20asdlfkj ").is_err()); + } + + #[test] + fn roll_dice_pool_malformed_expression_test() { + assert!(parse_command("!pool 8abc").is_err()); + assert!(parse_command("!pool 8abc ").is_err()); + } + + #[test] + fn pool_whitespace_test() { + parse_command("!pool ns3:8 ").expect("was error"); + parse_command(" !pool ns3:8").expect("was error"); + parse_command(" !pool ns3:8 ").expect("was error"); + } + + #[test] + fn help_whitespace_test() { + parse_command("!help stuff ").expect("was error"); + parse_command(" !help stuff").expect("was error"); + parse_command(" !help stuff ").expect("was error"); + } + + #[test] + fn roll_whitespace_test() { + parse_command("!roll 1d4 + 5d6 -3 ").expect("was error"); + parse_command("!roll 1d4 + 5d6 -3 ").expect("was error"); + parse_command(" !roll 1d4 + 5d6 -3 ").expect("was error"); + } }