Convert command execution to use results.

src/bin/dicebot-cmd.rs

@@ -1,4 +1,5 @@
 use chronicle_dicebot::commands;
+use chronicle_dicebot::commands::ResponseExtractor;
 use chronicle_dicebot::context::{Context, RoomContext};
 use chronicle_dicebot::db::Database;
 use chronicle_dicebot::error::BotError;
@@ -24,6 +25,9 @@ async fn main() -> Result<(), BotError> {
         message_body: &input,
     };
 
-    println!("{}", command.execute(&context).await.plain());
+    println!(
+        "{}",
+        command.execute(&context).await.message_plain("fakeuser")
+    );
     Ok(())
 }

src/bot.rs

@@ -139,13 +139,15 @@ impl DiceBot {
             results.push(cmd_result);
         }
 
+        use crate::commands::ResponseExtractor;
+
         if results.len() >= 1 {
             if results.len() == 1 {
                 let cmd_result = &results[0];
                 let response = AnyMessageEventContent::RoomMessage(MessageEventContent::Notice(
                     NoticeMessageEventContent::html(
-                        cmd_result.plain.clone(),
-                        cmd_result.html.clone(),
+                        cmd_result.message_plain(&sender_username),
+                        cmd_result.message_html(&sender_username),
                     ),
                 ));
 

src/commands.rs

@@ -1,4 +1,5 @@
 use crate::context::Context;
+use crate::error::BotError;
 use async_trait::async_trait;
 use thiserror::Error;
 
@@ -10,6 +11,8 @@ pub mod misc;
 pub mod parser;
 pub mod variables;
 
+/// A custom error type specifically related to parsing command text.
+/// Does not wrap an execution failure.
 #[derive(Error, Debug)]
 pub enum CommandError {
     #[error("invalid command: {0}")]
@@ -19,59 +22,105 @@ pub enum CommandError {
     IgnoredCommand,
 }
 
+/// A successfully executed command returns a message to be sent back
+/// to the user in both plain text and HTML, one of which will be
+/// displayed in the user's client depending on its capabilities.
+#[derive(Debug)]
 pub struct Execution {
     plain: String,
     html: String,
 }
 
 impl Execution {
-    pub fn plain(&self) -> &str {
-        &self.plain
+    pub fn new(plain: String, html: String) -> CommandResult {
+        Ok(Execution { plain, html })
     }
 
-    pub fn html(&self) -> &str {
-        &self.html
+    /// Response message in plain text.
+    pub fn plain(&self) -> String {
+        self.plain.clone()
+    }
+
+    /// Response message in HTML.
+    pub fn html(&self) -> String {
+        self.html.clone()
     }
 }
 
+/// Wraps a command execution failure. Provides plain-text and HTML
+/// formatting for any error message from the BotError type, similar
+/// to how Response provides formatting for successfully executed
+/// commands.
+#[derive(Error, Debug)]
+#[error("{0}")]
+pub struct ExecutionError(#[from] BotError);
+
+impl From<crate::db::errors::DataError> for ExecutionError {
+    fn from(error: crate::db::errors::DataError) -> Self {
+        Self(BotError::DataError(error))
+    }
+}
+
+impl ExecutionError {
+    /// Error message in plain text.
+    pub fn plain(&self) -> String {
+        format!("{}", self.0)
+    }
+
+    /// Error message in bolded HTML.
+    pub fn html(&self) -> String {
+        format!("<p><strong>{}</strong></p>", self.0)
+    }
+}
+
+/// Wraps either a successful command execution response, or an error
+/// that occurred.
+pub type CommandResult = Result<Execution, ExecutionError>;
+
+/// Extract response messages out of a type, whether it is success or
+/// failure.
+pub trait ResponseExtractor {
+    /// Plain-text representation of the message, directly mentioning
+    /// the username.
+    fn message_plain(&self, username: &str) -> String;
+
+    /// HTML representation of the message, directly mentioning the
+    /// username.
+    fn message_html(&self, username: &str) -> String;
+}
+
+impl ResponseExtractor for CommandResult {
+    /// Error message in plain text.
+    fn message_plain(&self, username: &str) -> String {
+        match self {
+            Ok(resp) => format!("{}\n{}", username, resp.plain()),
+            Err(e) => format!("{}\n{}", username, e.plain()),
+        }
+    }
+
+    /// Error message in bolded HTML.
+    fn message_html(&self, username: &str) -> String {
+        match self {
+            Ok(resp) => format!("<p>{}</p>\n{}", username, resp.html),
+            Err(e) => format!("<p>{}</p>\n{}", username, e.html()),
+        }
+    }
+}
+
+/// The trait that any command that can be executed must implement.
 #[async_trait]
 pub trait Command: Send + Sync {
-    async fn execute(&self, ctx: &Context<'_>) -> Execution;
+    async fn execute(&self, ctx: &Context<'_>) -> CommandResult;
     fn name(&self) -> &'static str;
 }
 
-#[derive(Debug)]
-pub struct CommandResult {
-    pub plain: String,
-    pub html: String,
-}
-
 /// Attempt to execute a command, and return the content that should
 /// 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<'_>) -> 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(e) => {
-            let message = format!("Error parsing command: {}", e);
-            let html_message = format!("<p><strong>{}</strong></p>", message);
-            (message, html_message)
-        }
-    };
-
-    let plain = format!("{}\n{}", ctx.username, plain);
-    let html = format!("<p>{}</p>\n{}", ctx.username, html);
-
-    CommandResult {
-        plain: plain,
-        html: html,
-    }
+    let cmd = parser::parse_command(&ctx.message_body)?;
+    cmd.execute(ctx).await
 }
 
 #[cfg(test)]
@@ -98,6 +147,6 @@ mod tests {
             message_body: "!notacommand",
         };
         let result = execute_command(&ctx).await;
-        assert!(result.plain.contains("Error"));
+        assert!(result.is_err());
     }
 }

src/commands/basic_rolling.rs

@@ -1,4 +1,4 @@
-use super::{Command, Execution};
+use super::{Command, CommandResult, Execution};
 use crate::basic::dice::ElementExpression;
 use crate::basic::roll::Roll;
 use crate::context::Context;
@@ -12,13 +12,14 @@ impl Command for RollCommand {
         "roll regular dice"
     }
 
-    async fn execute(&self, _ctx: &Context<'_>) -> Execution {
+    async fn execute(&self, _ctx: &Context<'_>) -> CommandResult {
         let roll = self.0.roll();
         let plain = format!("Dice: {}\nResult: {}", self.0, roll);
         let html = format!(
             "<p><strong>Dice:</strong> {}</p><p><strong>Result</strong>: {}</p>",
             self.0, roll
         );
-        Execution { plain, html }
+
+        Execution::new(plain, html)
     }
 }

src/commands/cofd.rs

@@ -1,4 +1,4 @@
-use super::{Command, Execution};
+use super::{Command, CommandResult, Execution};
 use crate::cofd::dice::{roll_pool, DicePool, DicePoolWithContext};
 use crate::context::Context;
 use async_trait::async_trait;
@@ -11,26 +11,16 @@ impl Command for PoolRollCommand {
         "roll dice pool"
     }
 
-    async fn execute(&self, ctx: &Context<'_>) -> Execution {
+    async fn execute(&self, ctx: &Context<'_>) -> CommandResult {
         let pool_with_ctx = DicePoolWithContext(&self.0, ctx);
-        let roll_result = roll_pool(&pool_with_ctx).await;
+        let rolled_pool = roll_pool(&pool_with_ctx).await?;
 
-        let (plain, html) = match roll_result {
-            Ok(rolled_pool) => {
-                let plain = format!("Pool: {}\nResult: {}", rolled_pool, rolled_pool.roll);
-                let html = format!(
-                    "<p><strong>Pool:</strong> {}</p><p><strong>Result</strong>: {}</p>",
-                    rolled_pool, rolled_pool.roll
-                );
-                (plain, html)
-            }
-            Err(e) => {
-                let plain = format!("Error: {}", e);
-                let html = format!("<p><strong>Error:</strong> {}</p>", e);
-                (plain, html)
-            }
-        };
+        let plain = format!("Pool: {}\nResult: {}", rolled_pool, rolled_pool.roll);
+        let html = format!(
+            "<p><strong>Pool:</strong> {}</p><p><strong>Result</strong>: {}</p>",
+            rolled_pool, rolled_pool.roll
+        );
 
-        Execution { plain, html }
+        Execution::new(plain, html)
     }
 }

src/commands/cthulhu.rs

@@ -1,4 +1,4 @@
-use super::{Command, Execution};
+use super::{Command, CommandResult, Execution};
 use crate::context::Context;
 use crate::cthulhu::dice::{regular_roll, AdvancementRoll, DiceRoll, DiceRollWithContext};
 use async_trait::async_trait;
@@ -11,27 +11,17 @@ impl Command for CthRoll {
         "roll percentile pool"
     }
 
-    async fn execute(&self, ctx: &Context<'_>) -> Execution {
+    async fn execute(&self, ctx: &Context<'_>) -> CommandResult {
         let roll_with_ctx = DiceRollWithContext(&self.0, ctx);
-        let roll = regular_roll(&roll_with_ctx).await;
+        let executed_roll = regular_roll(&roll_with_ctx).await?;
 
-        let (plain, html) = match roll {
-            Ok(executed_roll) => {
-                let plain = format!("Roll: {}\nResult: {}", executed_roll, executed_roll.roll);
-                let html = format!(
-                    "<p><strong>Roll:</strong> {}</p><p><strong>Result</strong>: {}</p>",
-                    executed_roll, executed_roll.roll
-                );
-                (plain, html)
-            }
-            Err(e) => {
-                let plain = format!("Error: {}", e);
-                let html = format!("<p><strong>Error:</strong> {}</p>", e);
-                (plain, html)
-            }
-        };
+        let plain = format!("Roll: {}\nResult: {}", executed_roll, executed_roll.roll);
+        let html = format!(
+            "<p><strong>Roll:</strong> {}</p><p><strong>Result</strong>: {}</p>",
+            executed_roll, executed_roll.roll
+        );
 
-        Execution { plain, html }
+        Execution::new(plain, html)
     }
 }
 
@@ -43,7 +33,7 @@ impl Command for CthAdvanceRoll {
         "roll percentile pool"
     }
 
-    async fn execute(&self, _ctx: &Context<'_>) -> Execution {
+    async fn execute(&self, _ctx: &Context<'_>) -> CommandResult {
         //TODO this will be converted to a result when supporting variables.
         let roll = self.0.roll();
         let plain = format!("Roll: {}\nResult: {}", self.0, roll);
@@ -52,6 +42,6 @@ impl Command for CthAdvanceRoll {
             self.0, roll
         );
 
-        Execution { plain, html }
+        Execution::new(plain, html)
     }
 }

src/commands/management.rs

@@ -1,46 +1,33 @@
-use super::{Command, Execution};
+use super::{Command, CommandResult, Execution};
 use crate::context::Context;
-use crate::db::errors::DataError;
 use crate::logic::record_room_information;
 use async_trait::async_trait;
 use matrix_sdk::identifiers::UserId;
 
 pub struct ResyncCommand;
 
-type ResyncResult = Result<(), DataError>;
-
 #[async_trait]
 impl Command for ResyncCommand {
     fn name(&self) -> &'static str {
         "resync room information"
     }
 
-    async fn execute(&self, ctx: &Context<'_>) -> Execution {
+    async fn execute(&self, ctx: &Context<'_>) -> CommandResult {
         let our_username: Option<UserId> = ctx.matrix_client.user_id().await;
         let our_username: &str = our_username.as_ref().map_or("", UserId::as_str);
 
-        let result: ResyncResult = record_room_information(
+        record_room_information(
             ctx.matrix_client,
             &ctx.db,
             ctx.room.id,
             &ctx.room.display_name,
             our_username,
         )
-        .await;
+        .await?;
 
-        let (plain, html) = match result {
-            Ok(()) => {
-                let plain = "Room information resynced".to_string();
-                let html = "<p>Room information resynced.</p>".to_string();
-                (plain, html)
-            }
-            Err(e) => {
-                let plain = format!("Error: {}", e);
-                let html = format!("<p><strong>Error:</strong> {}</p>", e);
-                (plain, html)
-            }
-        };
+        let plain = "Room information resynced".to_string();
+        let html = "<p>Room information resynced.</p>".to_string();
 
-        Execution { plain, html }
+        Execution::new(plain, html)
     }
 }

src/commands/misc.rs

@@ -1,4 +1,4 @@
-use super::{Command, Execution};
+use super::{Command, CommandResult, Execution};
 use crate::context::Context;
 use crate::help::HelpTopic;
 use async_trait::async_trait;
@@ -11,7 +11,7 @@ impl Command for HelpCommand {
         "help information"
     }
 
-    async fn execute(&self, _ctx: &Context<'_>) -> Execution {
+    async fn execute(&self, _ctx: &Context<'_>) -> CommandResult {
         let help = match &self.0 {
             Some(topic) => topic.message(),
             _ => "There is no help for this topic",
@@ -19,6 +19,6 @@ impl Command for HelpCommand {
 
         let plain = format!("Help: {}", help);
         let html = format!("<p><strong>Help:</strong> {}", help.replace("\n", "<br/>"));
-        Execution { plain, html }
+        Execution::new(plain, html)
     }
 }

src/commands/variables.rs

@@ -1,4 +1,4 @@
-use super::{Command, Execution};
+use super::{Command, CommandResult, Execution};
 use crate::context::Context;
 use crate::db::errors::DataError;
 use crate::db::variables::UserAndRoom;
@@ -12,29 +12,24 @@ impl Command for GetAllVariablesCommand {
         "get all variables"
     }
 
-    async fn execute(&self, ctx: &Context<'_>) -> Execution {
+    async fn execute(&self, ctx: &Context<'_>) -> CommandResult {
         let key = UserAndRoom(&ctx.username, &ctx.room.id.as_str());
-        let result = ctx.db.variables.get_user_variables(&key);
+        let variables = ctx.db.variables.get_user_variables(&key)?;
 
-        let value = match result {
-            Ok(variables) => {
-                let mut variable_list = variables
-                    .into_iter()
-                    .map(|(name, value)| format!(" - {} = {}", name, value))
-                    .collect::<Vec<_>>();
+        let mut variable_list = variables
+            .into_iter()
+            .map(|(name, value)| format!(" - {} = {}", name, value))
+            .collect::<Vec<_>>();
 
-                variable_list.sort();
-                variable_list.join("\n")
-            }
-            Err(e) => format!("error getting variables: {}", e),
-        };
+        variable_list.sort();
 
+        let value = variable_list.join("\n");
         let plain = format!("Variables:\n{}", value);
         let html = format!(
             "<p><strong>Variables:</strong><br/>{}",
             value.replace("\n", "<br/>")
         );
-        Execution { plain, html }
+        Execution::new(plain, html)
     }
 }
 
@@ -46,7 +41,7 @@ impl Command for GetVariableCommand {
         "retrieve variable value"
     }
 
-    async fn execute(&self, ctx: &Context<'_>) -> Execution {
+    async fn execute(&self, ctx: &Context<'_>) -> CommandResult {
         let name = &self.0;
         let key = UserAndRoom(&ctx.username, &ctx.room.id.as_str());
         let result = ctx.db.variables.get_user_variable(&key, name);
@@ -54,12 +49,12 @@ impl Command for GetVariableCommand {
         let value = match result {
             Ok(num) => format!("{} = {}", name, num),
             Err(DataError::KeyDoesNotExist(_)) => format!("{} is not set", name),
-            Err(e) => format!("error getting {}: {}", name, e),
+            Err(e) => return Err(e.into()),
         };
 
         let plain = format!("Variable: {}", value);
         let html = format!("<p><strong>Variable:</strong> {}", value);
-        Execution { plain, html }
+        Execution::new(plain, html)
     }
 }
 
@@ -71,20 +66,17 @@ impl Command for SetVariableCommand {
         "set variable value"
     }
 
-    async fn execute(&self, ctx: &Context<'_>) -> Execution {
+    async fn execute(&self, ctx: &Context<'_>) -> CommandResult {
         let name = &self.0;
         let value = self.1;
         let key = UserAndRoom(&ctx.username, ctx.room.id.as_str());
-        let result = ctx.db.variables.set_user_variable(&key, name, value);
 
-        let content = match result {
-            Ok(_) => format!("{} = {}", name, value),
-            Err(e) => format!("error setting {}: {}", name, e),
-        };
+        ctx.db.variables.set_user_variable(&key, name, value)?;
 
+        let content = format!("{} = {}", name, value);
         let plain = format!("Set Variable: {}", content);
         let html = format!("<p><strong>Set Variable:</strong> {}", content);
-        Execution { plain, html }
+        Execution::new(plain, html)
     }
 }
 
@@ -96,7 +88,7 @@ impl Command for DeleteVariableCommand {
         "delete variable"
     }
 
-    async fn execute(&self, ctx: &Context<'_>) -> Execution {
+    async fn execute(&self, ctx: &Context<'_>) -> CommandResult {
         let name = &self.0;
         let key = UserAndRoom(&ctx.username, ctx.room.id.as_str());
         let result = ctx.db.variables.delete_user_variable(&key, name);
@@ -104,11 +96,11 @@ impl Command for DeleteVariableCommand {
         let value = match result {
             Ok(()) => format!("{} now unset", name),
             Err(DataError::KeyDoesNotExist(_)) => format!("{} is not currently set", name),
-            Err(e) => format!("error deleting {}: {}", name, e),
+            Err(e) => return Err(e.into()),
         };
 
         let plain = format!("Remove Variable: {}", value);
         let html = format!("<p><strong>Remove Variable:</strong> {}", value);
-        Execution { plain, html }
+        Execution::new(plain, html)
     }
 }