From 7539dacb2f772fc2ff7cf59f353860ca3ea793ce Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 22 Oct 2025 16:57:47 +0000 Subject: [PATCH 01/14] Initial plan From d844f627f73ad4752d3c1838997a71b9fd547d55 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 22 Oct 2025 17:09:29 +0000 Subject: [PATCH 02/14] feat: add VerbosityLevel enum and UserOutput struct with dual writers Implement user-facing output functionality separate from internal logging with dual-channel strategy following Unix conventions: - stdout: final results and data for piping/redirection - stderr: progress updates, status messages, warnings, errors Add comprehensive unit tests with buffer writers for output capture and assertion. Co-authored-by: josecelano <58816+josecelano@users.noreply.github.com> --- src/shared/mod.rs | 2 + src/shared/user_output.rs | 488 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 490 insertions(+) create mode 100644 src/shared/user_output.rs diff --git a/src/shared/mod.rs b/src/shared/mod.rs index 03c61696..dc8bb52f 100644 --- a/src/shared/mod.rs +++ b/src/shared/mod.rs @@ -9,6 +9,7 @@ pub mod command; pub mod error; pub mod port_checker; pub mod port_usage_checker; +pub mod user_output; pub mod username; // Re-export commonly used types for convenience @@ -17,4 +18,5 @@ pub use command::{CommandError, CommandExecutor, CommandResult}; pub use error::{ErrorKind, Traceable}; pub use port_checker::{PortChecker, PortCheckerError}; pub use port_usage_checker::{PortUsageChecker, PortUsageError}; +pub use user_output::{UserOutput, VerbosityLevel}; pub use username::{Username, UsernameError}; diff --git a/src/shared/user_output.rs b/src/shared/user_output.rs new file mode 100644 index 00000000..b4a638fb --- /dev/null +++ b/src/shared/user_output.rs @@ -0,0 +1,488 @@ +//! User-facing output handling +//! +//! This module provides user-facing output functionality separate from internal logging. +//! It implements a dual-channel strategy following Unix conventions and modern CLI best practices +//! (similar to cargo, docker, npm): +//! +//! - **stdout (Results Channel)**: Final results, structured data, output for piping/redirection +//! - **stderr (Progress/Operational Channel)**: Progress updates, status messages, warnings, errors +//! +//! This separation enables: +//! - Clean piping: `torrust-tracker-deployer destroy env | jq .status` works correctly +//! - Automation friendly: Scripts can redirect progress to /dev/null while capturing results +//! - Unix convention compliance: Follows established patterns from modern CLI tools +//! - Better UX: Progress feedback doesn't interfere with result data +//! +//! ## Example Usage +//! +//! ```rust +//! use torrust_tracker_deployer_lib::shared::user_output::{UserOutput, VerbosityLevel}; +//! +//! let mut output = UserOutput::new(VerbosityLevel::Normal); +//! +//! // Progress messages go to stderr +//! output.progress("Destroying environment..."); +//! +//! // Success status goes to stderr +//! output.success("Environment destroyed successfully"); +//! +//! // Results go to stdout for piping +//! output.result(r#"{"status": "destroyed"}"#); +//! ``` +//! +//! ## Channel Strategy +//! +//! Based on research from [`docs/research/UX/console-app-output-patterns.md`](../../docs/research/UX/console-app-output-patterns.md): +//! +//! - **stdout**: Deployment results, configuration summaries, structured data (JSON) +//! - **stderr**: Step progress, status updates, warnings, error messages with actionable guidance +//! +//! See also: [`docs/research/UX/user-output-vs-logging-separation.md`](../../docs/research/UX/user-output-vs-logging-separation.md) + +use std::io::Write; + +/// Verbosity levels for user output +/// +/// Controls the amount of detail shown to users. Higher verbosity levels include +/// all output from lower levels. +/// +/// # Examples +/// +/// ```rust +/// use torrust_tracker_deployer_lib::shared::user_output::VerbosityLevel; +/// +/// let level = VerbosityLevel::Normal; +/// assert!(level >= VerbosityLevel::Quiet); +/// assert!(level < VerbosityLevel::Verbose); +/// ``` +#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord)] +pub enum VerbosityLevel { + /// Minimal output - only errors and final results + Quiet, + /// Default level - essential progress and results + Normal, + /// Detailed progress including intermediate steps + Verbose, + /// Very detailed including decisions and retries + VeryVerbose, + /// Maximum detail for troubleshooting + Debug, +} + +impl Default for VerbosityLevel { + fn default() -> Self { + Self::Normal + } +} + +/// Handles user-facing output separate from internal logging +/// +/// Uses dual channels following Unix conventions and modern CLI best practices: +/// - **stdout**: Final results and data for piping/redirection +/// - **stderr**: Progress updates, status messages, operational info, errors +/// +/// This separation allows scripts to cleanly capture results while seeing progress: +/// +/// ```bash +/// # Suppress progress, capture results only +/// torrust-tracker-deployer destroy env 2>/dev/null > result.json +/// +/// # Suppress results, see progress only +/// torrust-tracker-deployer destroy env > /dev/null +/// ``` +/// +/// # Examples +/// +/// ```rust +/// use torrust_tracker_deployer_lib::shared::user_output::{UserOutput, VerbosityLevel}; +/// +/// let mut output = UserOutput::new(VerbosityLevel::Normal); +/// +/// // Progress to stderr (visible during execution, doesn't interfere with piping) +/// output.progress("Processing data..."); +/// +/// // Results to stdout (can be piped to other commands) +/// output.result("Processing complete"); +/// ``` +pub struct UserOutput { + verbosity: VerbosityLevel, + stdout_writer: Box, + stderr_writer: Box, +} + +impl UserOutput { + /// Create new `UserOutput` with default stdout/stderr channels + /// + /// # Examples + /// + /// ```rust + /// use torrust_tracker_deployer_lib::shared::user_output::{UserOutput, VerbosityLevel}; + /// + /// let output = UserOutput::new(VerbosityLevel::Normal); + /// ``` + #[must_use] + pub fn new(verbosity: VerbosityLevel) -> Self { + Self { + verbosity, + stdout_writer: Box::new(std::io::stdout()), + stderr_writer: Box::new(std::io::stderr()), + } + } + + /// Create `UserOutput` for testing with custom writers + /// + /// This constructor allows injecting custom writers for testing, + /// enabling output capture and assertion. + /// + /// # Examples + /// + /// ```rust + /// use torrust_tracker_deployer_lib::shared::user_output::{UserOutput, VerbosityLevel}; + /// use std::io::Cursor; + /// + /// let stdout_buf = Vec::new(); + /// let stderr_buf = Vec::new(); + /// + /// let output = UserOutput::with_writers( + /// VerbosityLevel::Normal, + /// Box::new(Cursor::new(stdout_buf)), + /// Box::new(Cursor::new(stderr_buf)), + /// ); + /// ``` + #[must_use] + pub fn with_writers( + verbosity: VerbosityLevel, + stdout_writer: Box, + stderr_writer: Box, + ) -> Self { + Self { + verbosity, + stdout_writer, + stderr_writer, + } + } + + /// Display progress message to stderr (Normal level and above) + /// + /// Progress messages go to stderr following cargo/docker patterns. + /// This keeps stdout clean for result data that may be piped. + /// + /// # Examples + /// + /// ```rust + /// use torrust_tracker_deployer_lib::shared::user_output::{UserOutput, VerbosityLevel}; + /// + /// let mut output = UserOutput::new(VerbosityLevel::Normal); + /// output.progress("Destroying environment..."); + /// // Output to stderr: ⏳ Destroying environment... + /// ``` + pub fn progress(&mut self, message: &str) { + if self.verbosity >= VerbosityLevel::Normal { + writeln!(self.stderr_writer, "⏳ {message}").ok(); + } + } + + /// Display success message to stderr (Normal level and above) + /// + /// Success status goes to stderr to allow clean result piping. + /// + /// # Examples + /// + /// ```rust + /// use torrust_tracker_deployer_lib::shared::user_output::{UserOutput, VerbosityLevel}; + /// + /// let mut output = UserOutput::new(VerbosityLevel::Normal); + /// output.success("Environment destroyed successfully"); + /// // Output to stderr: ✅ Environment destroyed successfully + /// ``` + pub fn success(&mut self, message: &str) { + if self.verbosity >= VerbosityLevel::Normal { + writeln!(self.stderr_writer, "✅ {message}").ok(); + } + } + + /// Display warning message to stderr (Normal level and above) + /// + /// # Examples + /// + /// ```rust + /// use torrust_tracker_deployer_lib::shared::user_output::{UserOutput, VerbosityLevel}; + /// + /// let mut output = UserOutput::new(VerbosityLevel::Normal); + /// output.warn("Infrastructure may already be destroyed"); + /// // Output to stderr: ⚠️ Infrastructure may already be destroyed + /// ``` + pub fn warn(&mut self, message: &str) { + if self.verbosity >= VerbosityLevel::Normal { + writeln!(self.stderr_writer, "⚠️ {message}").ok(); + } + } + + /// Display error message to stderr (all levels) + /// + /// Errors are always shown regardless of verbosity level. + /// + /// # Examples + /// + /// ```rust + /// use torrust_tracker_deployer_lib::shared::user_output::{UserOutput, VerbosityLevel}; + /// + /// let mut output = UserOutput::new(VerbosityLevel::Quiet); + /// output.error("Failed to destroy environment"); + /// // Output to stderr: ❌ Failed to destroy environment + /// ``` + pub fn error(&mut self, message: &str) { + writeln!(self.stderr_writer, "❌ {message}").ok(); + } + + /// Output final results to stdout for piping/redirection + /// + /// This is where deployment results, configuration summaries, etc. go. + /// Since this goes to stdout, it can be cleanly piped to other commands. + /// + /// # Examples + /// + /// ```rust + /// use torrust_tracker_deployer_lib::shared::user_output::{UserOutput, VerbosityLevel}; + /// + /// let mut output = UserOutput::new(VerbosityLevel::Normal); + /// output.result("Deployment complete"); + /// // Output to stdout: Deployment complete + /// ``` + pub fn result(&mut self, message: &str) { + writeln!(self.stdout_writer, "{message}").ok(); + } + + /// Output structured data to stdout (JSON, etc.) + /// + /// For machine-readable output that should be piped or processed. + /// + /// # Examples + /// + /// ```rust + /// use torrust_tracker_deployer_lib::shared::user_output::{UserOutput, VerbosityLevel}; + /// + /// let mut output = UserOutput::new(VerbosityLevel::Normal); + /// output.data(r#"{"status": "destroyed", "environment": "test"}"#); + /// // Output to stdout: {"status": "destroyed", "environment": "test"} + /// ``` + pub fn data(&mut self, data: &str) { + writeln!(self.stdout_writer, "{data}").ok(); + } +} + +#[cfg(test)] +mod tests { + use super::*; + + /// Helper to create test `UserOutput` with buffer writers + /// + /// Returns: (`UserOutput`, Arc to stdout buffer, Arc to stderr buffer) + fn create_test_user_output( + verbosity: VerbosityLevel, + ) -> ( + UserOutput, + std::sync::Arc>>, + std::sync::Arc>>, + ) { + use std::sync::{Arc, Mutex}; + + let stdout_buffer = Arc::new(Mutex::new(Vec::new())); + let stderr_buffer = Arc::new(Mutex::new(Vec::new())); + + let stdout_clone = Arc::clone(&stdout_buffer); + let stderr_clone = Arc::clone(&stderr_buffer); + + // Create thread-safe writers that share the buffer + let stdout_writer = Box::new(SharedWriter(Arc::clone(&stdout_buffer))); + let stderr_writer = Box::new(SharedWriter(Arc::clone(&stderr_buffer))); + + let output = UserOutput::with_writers(verbosity, stdout_writer, stderr_writer); + + (output, stdout_clone, stderr_clone) + } + + /// A writer that shares a buffer through an Arc>> + struct SharedWriter(std::sync::Arc>>); + + impl Write for SharedWriter { + fn write(&mut self, buf: &[u8]) -> std::io::Result { + self.0 + .lock() + .unwrap() + .write(buf) + } + + fn flush(&mut self) -> std::io::Result<()> { + self.0.lock().unwrap().flush() + } + } + + #[test] + fn it_should_write_progress_messages_to_stderr() { + let (mut output, stdout_buf, stderr_buf) = + create_test_user_output(VerbosityLevel::Normal); + + output.progress("Testing progress message"); + + // Verify message went to stderr + let stderr_content = String::from_utf8(stderr_buf.lock().unwrap().clone()).unwrap(); + assert_eq!(stderr_content, "⏳ Testing progress message\n"); + + // Verify stdout is empty + let stdout_content = String::from_utf8(stdout_buf.lock().unwrap().clone()).unwrap(); + assert_eq!(stdout_content, ""); + } + + #[test] + fn it_should_write_success_messages_to_stderr() { + let (mut output, stdout_buf, stderr_buf) = + create_test_user_output(VerbosityLevel::Normal); + + output.success("Testing success message"); + + // Verify message went to stderr + let stderr_content = String::from_utf8(stderr_buf.lock().unwrap().clone()).unwrap(); + assert_eq!(stderr_content, "✅ Testing success message\n"); + + // Verify stdout is empty + let stdout_content = String::from_utf8(stdout_buf.lock().unwrap().clone()).unwrap(); + assert_eq!(stdout_content, ""); + } + + #[test] + fn it_should_write_warning_messages_to_stderr() { + let (mut output, stdout_buf, stderr_buf) = + create_test_user_output(VerbosityLevel::Normal); + + output.warn("Testing warning message"); + + // Verify message went to stderr + let stderr_content = String::from_utf8(stderr_buf.lock().unwrap().clone()).unwrap(); + assert_eq!(stderr_content, "⚠️ Testing warning message\n"); + + // Verify stdout is empty + let stdout_content = String::from_utf8(stdout_buf.lock().unwrap().clone()).unwrap(); + assert_eq!(stdout_content, ""); + } + + #[test] + fn it_should_write_error_messages_to_stderr() { + let (mut output, stdout_buf, stderr_buf) = + create_test_user_output(VerbosityLevel::Normal); + + output.error("Testing error message"); + + // Verify message went to stderr + let stderr_content = String::from_utf8(stderr_buf.lock().unwrap().clone()).unwrap(); + assert_eq!(stderr_content, "❌ Testing error message\n"); + + // Verify stdout is empty + let stdout_content = String::from_utf8(stdout_buf.lock().unwrap().clone()).unwrap(); + assert_eq!(stdout_content, ""); + } + + #[test] + fn it_should_write_results_to_stdout() { + let (mut output, stdout_buf, stderr_buf) = + create_test_user_output(VerbosityLevel::Normal); + + output.result("Test result data"); + + // Verify message went to stdout + let stdout_content = String::from_utf8(stdout_buf.lock().unwrap().clone()).unwrap(); + assert_eq!(stdout_content, "Test result data\n"); + + // Verify stderr is empty + let stderr_content = String::from_utf8(stderr_buf.lock().unwrap().clone()).unwrap(); + assert_eq!(stderr_content, ""); + } + + #[test] + fn it_should_write_data_to_stdout() { + let (mut output, stdout_buf, stderr_buf) = + create_test_user_output(VerbosityLevel::Normal); + + output.data(r#"{"status": "destroyed"}"#); + + // Verify message went to stdout + let stdout_content = String::from_utf8(stdout_buf.lock().unwrap().clone()).unwrap(); + assert_eq!(stdout_content, "{\"status\": \"destroyed\"}\n"); + + // Verify stderr is empty + let stderr_content = String::from_utf8(stderr_buf.lock().unwrap().clone()).unwrap(); + assert_eq!(stderr_content, ""); + } + + #[test] + fn it_should_respect_verbosity_levels_for_progress() { + let (mut output, _stdout_buf, stderr_buf) = create_test_user_output(VerbosityLevel::Quiet); + + output.progress("This should not appear"); + + // Verify no output at Quiet level + let stderr_content = String::from_utf8(stderr_buf.lock().unwrap().clone()).unwrap(); + assert_eq!(stderr_content, ""); + } + + #[test] + fn it_should_respect_verbosity_levels_for_success() { + let (mut output, _stdout_buf, stderr_buf) = create_test_user_output(VerbosityLevel::Quiet); + + output.success("This should not appear"); + + // Verify no output at Quiet level + let stderr_content = String::from_utf8(stderr_buf.lock().unwrap().clone()).unwrap(); + assert_eq!(stderr_content, ""); + } + + #[test] + fn it_should_respect_verbosity_levels_for_warn() { + let (mut output, _stdout_buf, stderr_buf) = create_test_user_output(VerbosityLevel::Quiet); + + output.warn("This should not appear"); + + // Verify no output at Quiet level + let stderr_content = String::from_utf8(stderr_buf.lock().unwrap().clone()).unwrap(); + assert_eq!(stderr_content, ""); + } + + #[test] + fn it_should_always_show_errors_regardless_of_verbosity() { + let (mut output, _stdout_buf, stderr_buf) = create_test_user_output(VerbosityLevel::Quiet); + + output.error("Critical error message"); + + // Verify error appears even at Quiet level + let stderr_content = String::from_utf8(stderr_buf.lock().unwrap().clone()).unwrap(); + assert_eq!(stderr_content, "❌ Critical error message\n"); + } + + #[test] + fn it_should_use_normal_as_default_verbosity() { + let default = VerbosityLevel::default(); + assert_eq!(default, VerbosityLevel::Normal); + } + + #[test] + fn it_should_order_verbosity_levels_correctly() { + assert!(VerbosityLevel::Quiet < VerbosityLevel::Normal); + assert!(VerbosityLevel::Normal < VerbosityLevel::Verbose); + assert!(VerbosityLevel::Verbose < VerbosityLevel::VeryVerbose); + assert!(VerbosityLevel::VeryVerbose < VerbosityLevel::Debug); + } + + #[test] + fn it_should_support_equality_comparison() { + assert_eq!(VerbosityLevel::Normal, VerbosityLevel::Normal); + assert_ne!(VerbosityLevel::Normal, VerbosityLevel::Verbose); + } + + #[test] + fn it_should_support_ordering_comparison() { + let normal = VerbosityLevel::Normal; + assert!(normal >= VerbosityLevel::Quiet); + assert!(normal >= VerbosityLevel::Normal); + assert!(normal < VerbosityLevel::Verbose); + } +} From 24d453dabec89d925c42b8ac0ab1341990d68082 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 22 Oct 2025 17:17:51 +0000 Subject: [PATCH 03/14] feat: add destroy subcommand to CLI with command handler integration Add Commands enum with Destroy variant accepting environment parameter. Wire up destroy subcommand to call DestroyCommandHandler from application layer. Implement comprehensive CLI argument parsing tests. Integrate UserOutput for user-friendly progress messages and error handling. Co-authored-by: josecelano <58816+josecelano@users.noreply.github.com> --- src/app.rs | 292 ++++++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 287 insertions(+), 5 deletions(-) diff --git a/src/app.rs b/src/app.rs index 98f0f02c..5be04654 100644 --- a/src/app.rs +++ b/src/app.rs @@ -3,14 +3,14 @@ //! This module contains the CLI structure and main application logic. //! It initializes logging and handles the application lifecycle. -use clap::Parser; +use clap::{Parser, Subcommand}; use std::path::PathBuf; use tracing::info; use torrust_tracker_deployer_lib::logging::{LogFormat, LogOutput, LoggingBuilder}; /// Command-line interface for Torrust Tracker Deployer -#[derive(Parser)] +#[derive(Parser, Debug)] #[command(name = "torrust-tracker-deployer")] #[command(about = "Automated deployment infrastructure for Torrust Tracker")] #[command(version)] @@ -56,12 +56,26 @@ pub struct Cli { /// observability and the application cannot function without it. #[arg(long, default_value = "./data/logs", global = true)] pub log_dir: PathBuf, + + /// Subcommand to execute + #[command(subcommand)] + pub command: Option, +} + +/// Available CLI commands +#[derive(Subcommand, Debug)] +pub enum Commands { + /// Destroy an existing deployment environment + Destroy { + /// Name of the environment to destroy + environment: String, + }, } /// Main application entry point /// /// This function initializes logging, displays information to the user, -/// and prepares the application for future command processing. +/// and executes the requested command if provided. /// /// # Panics /// @@ -97,7 +111,24 @@ pub fn run() { "Application started" ); - // Display info to user (keep existing behavior for now) + // Execute command if provided, otherwise display help + match cli.command { + Some(Commands::Destroy { environment }) => { + if let Err(e) = handle_destroy_command(environment) { + eprintln!("Error: {e}"); + std::process::exit(1); + } + } + None => { + display_help_info(); + } + } + + info!("Application finished"); +} + +/// Display helpful information to the user when no command is provided +fn display_help_info() { println!("🏗️ Torrust Tracker Deployer"); println!("========================="); println!(); @@ -116,6 +147,257 @@ pub fn run() { println!(" cargo e2e-provision && cargo e2e-config"); println!(); println!("📖 For detailed instructions, see: README.md"); + println!(); + println!("💡 To see available commands, run: torrust-tracker-deployer --help"); +} - info!("Application finished"); +/// Handle the destroy command +/// +/// This function orchestrates the environment destruction workflow by: +/// 1. Validating the environment name +/// 2. Loading the environment from persistent storage +/// 3. Executing the destroy command handler +/// 4. Providing user-friendly progress updates +/// +/// # Arguments +/// +/// * `environment_name` - The name of the environment to destroy +/// +/// # Returns +/// +/// Returns `Ok(())` on success, or an error if: +/// - Environment name is invalid +/// - Environment cannot be loaded +/// - Destruction fails +/// +/// # Errors +/// +/// This function will return an error if the environment name is invalid, +/// the environment cannot be loaded, or the destruction process fails. +fn handle_destroy_command(environment_name: String) -> Result<(), Box> { + use std::sync::Arc; + use std::time::Duration; + use torrust_tracker_deployer_lib::adapters::tofu::client::OpenTofuClient; + use torrust_tracker_deployer_lib::application::command_handlers::DestroyCommandHandler; + use torrust_tracker_deployer_lib::domain::environment::name::EnvironmentName; + use torrust_tracker_deployer_lib::domain::environment::state::AnyEnvironmentState; + use torrust_tracker_deployer_lib::infrastructure::persistence::repository_factory::RepositoryFactory; + use torrust_tracker_deployer_lib::shared::user_output::{UserOutput, VerbosityLevel}; + + // Create user output with default stdout/stderr channels + let mut output = UserOutput::new(VerbosityLevel::Normal); + + // Display initial progress (to stderr) + output.progress(&format!("Destroying environment '{environment_name}'...")); + + // Validate environment name + let env_name = EnvironmentName::new(environment_name.clone()).map_err(|e| { + output.error(&format!( + "Invalid environment name '{environment_name}': {e}" + )); + format!("Invalid environment name: {e}") + })?; + + // Build path for the environment + let build_dir = PathBuf::from("build").join(env_name.as_str()); + + // Create OpenTofu client + let opentofu_client = Arc::new(OpenTofuClient::new(build_dir.join("opentofu"))); + + // Create repository for loading environment state + let repository_factory = RepositoryFactory::new(Duration::from_secs(30)); + let repository = repository_factory.create(PathBuf::from("data")); + + // Load the environment from storage + let environment = repository.load(&env_name).map_err(|e| { + output.error(&format!( + "Failed to load environment '{environment_name}': {e}" + )); + format!("Failed to load environment: {e}") + })?; + + // Check if environment exists + let environment = environment.ok_or_else(|| { + output.error(&format!( + "Environment '{environment_name}' not found. Has it been provisioned?" + )); + format!("Environment '{environment_name}' not found") + })?; + + // Create and execute destroy command handler + output.progress("Tearing down infrastructure..."); + + let command_handler = DestroyCommandHandler::new(opentofu_client, repository); + + // Execute destroy based on the environment's current state + let _destroyed_env = match environment { + AnyEnvironmentState::Destroyed(env) => { + output.warn("Environment is already destroyed"); + Ok(env) + } + AnyEnvironmentState::Created(env) => command_handler.execute(env), + AnyEnvironmentState::Provisioning(env) => command_handler.execute(env), + AnyEnvironmentState::Provisioned(env) => command_handler.execute(env), + AnyEnvironmentState::Configuring(env) => command_handler.execute(env), + AnyEnvironmentState::Configured(env) => command_handler.execute(env), + AnyEnvironmentState::Releasing(env) => command_handler.execute(env), + AnyEnvironmentState::Released(env) => command_handler.execute(env), + AnyEnvironmentState::Running(env) => command_handler.execute(env), + AnyEnvironmentState::ProvisionFailed(env) => command_handler.execute(env), + AnyEnvironmentState::ConfigureFailed(env) => command_handler.execute(env), + AnyEnvironmentState::ReleaseFailed(env) => command_handler.execute(env), + AnyEnvironmentState::RunFailed(env) => command_handler.execute(env), + } + .map_err(|e| { + output.error(&format!( + "Failed to destroy environment '{environment_name}': {e}" + )); + format!("Destroy command failed: {e}") + })?; + + output.progress("Cleaning up resources..."); + output.success(&format!( + "Environment '{environment_name}' destroyed successfully" + )); + + Ok(()) } + +#[cfg(test)] +mod tests { + use super::*; + use clap::Parser; + + #[test] + fn it_should_parse_destroy_subcommand() { + let args = vec!["torrust-tracker-deployer", "destroy", "test-env"]; + let cli = Cli::try_parse_from(args).unwrap(); + + assert!(cli.command.is_some()); + match cli.command.unwrap() { + Commands::Destroy { environment } => { + assert_eq!(environment, "test-env"); + } + } + } + + #[test] + fn it_should_parse_destroy_with_different_environment_names() { + let test_cases = vec![ + "e2e-provision", + "production", + "test-123", + "dev-environment", + ]; + + for env_name in test_cases { + let args = vec!["torrust-tracker-deployer", "destroy", env_name]; + let cli = Cli::try_parse_from(args).unwrap(); + + match cli.command.unwrap() { + Commands::Destroy { environment } => { + assert_eq!(environment, env_name); + } + } + } + } + + #[test] + fn it_should_require_environment_parameter() { + let args = vec!["torrust-tracker-deployer", "destroy"]; + let result = Cli::try_parse_from(args); + + assert!(result.is_err()); + let error = result.unwrap_err(); + let error_message = error.to_string(); + assert!( + error_message.contains("required") || error_message.contains("argument"), + "Error message should indicate missing required argument: {error_message}" + ); + } + + #[test] + fn it_should_parse_global_log_options_with_destroy_command() { + let args = vec![ + "torrust-tracker-deployer", + "--log-file-format", + "json", + "--log-stderr-format", + "compact", + "--log-output", + "file-and-stderr", + "--log-dir", + "/tmp/logs", + "destroy", + "test-env", + ]; + let cli = Cli::try_parse_from(args).unwrap(); + + // Verify the destroy command was parsed correctly + match cli.command.unwrap() { + Commands::Destroy { environment } => { + assert_eq!(environment, "test-env"); + } + } + + // Log options are set but we don't compare them as they don't implement PartialEq + assert_eq!(cli.log_dir, PathBuf::from("/tmp/logs")); + } + + #[test] + fn it_should_use_default_log_dir_when_not_specified() { + let args = vec!["torrust-tracker-deployer", "destroy", "test-env"]; + let cli = Cli::try_parse_from(args).unwrap(); + + assert_eq!(cli.log_dir, PathBuf::from("./data/logs")); + } + + #[test] + fn it_should_handle_no_command() { + let args = vec!["torrust-tracker-deployer"]; + let cli = Cli::try_parse_from(args).unwrap(); + + assert!(cli.command.is_none()); + } + + #[test] + fn it_should_show_help_with_help_flag() { + let args = vec!["torrust-tracker-deployer", "--help"]; + let result = Cli::try_parse_from(args); + + // Help flag causes a "display help" error + assert!(result.is_err()); + let error = result.unwrap_err(); + assert_eq!(error.kind(), clap::error::ErrorKind::DisplayHelp); + } + + #[test] + fn it_should_show_version_with_version_flag() { + let args = vec!["torrust-tracker-deployer", "--version"]; + let result = Cli::try_parse_from(args); + + // Version flag causes a "display version" error + assert!(result.is_err()); + let error = result.unwrap_err(); + assert_eq!(error.kind(), clap::error::ErrorKind::DisplayVersion); + } + + #[test] + fn it_should_show_destroy_help() { + let args = vec!["torrust-tracker-deployer", "destroy", "--help"]; + let result = Cli::try_parse_from(args); + + // Help flag causes a "display help" error + assert!(result.is_err()); + let error = result.unwrap_err(); + assert_eq!(error.kind(), clap::error::ErrorKind::DisplayHelp); + + // Verify the help text mentions the environment parameter + let help_text = error.to_string(); + assert!( + help_text.contains("environment") || help_text.contains(""), + "Help text should mention environment parameter" + ); + } +} + From 52c139703fa5b493bb1b22b0f13e5eeb6f9f0c6a Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 22 Oct 2025 17:26:42 +0000 Subject: [PATCH 04/14] fix: address clippy warnings and format code Fix needless_pass_by_value clippy warning by using &str instead of String. Add allow attribute for type_complexity in test helper. Run cargo fmt to ensure consistent code formatting. Co-authored-by: josecelano <58816+josecelano@users.noreply.github.com> --- src/app.rs | 14 ++++---------- src/shared/user_output.rs | 24 ++++++++---------------- 2 files changed, 12 insertions(+), 26 deletions(-) diff --git a/src/app.rs b/src/app.rs index 5be04654..4d20f011 100644 --- a/src/app.rs +++ b/src/app.rs @@ -114,7 +114,7 @@ pub fn run() { // Execute command if provided, otherwise display help match cli.command { Some(Commands::Destroy { environment }) => { - if let Err(e) = handle_destroy_command(environment) { + if let Err(e) = handle_destroy_command(&environment) { eprintln!("Error: {e}"); std::process::exit(1); } @@ -174,7 +174,7 @@ fn display_help_info() { /// /// This function will return an error if the environment name is invalid, /// the environment cannot be loaded, or the destruction process fails. -fn handle_destroy_command(environment_name: String) -> Result<(), Box> { +fn handle_destroy_command(environment_name: &str) -> Result<(), Box> { use std::sync::Arc; use std::time::Duration; use torrust_tracker_deployer_lib::adapters::tofu::client::OpenTofuClient; @@ -191,7 +191,7 @@ fn handle_destroy_command(environment_name: String) -> Result<(), Box ( @@ -307,10 +308,7 @@ mod tests { impl Write for SharedWriter { fn write(&mut self, buf: &[u8]) -> std::io::Result { - self.0 - .lock() - .unwrap() - .write(buf) + self.0.lock().unwrap().write(buf) } fn flush(&mut self) -> std::io::Result<()> { @@ -320,8 +318,7 @@ mod tests { #[test] fn it_should_write_progress_messages_to_stderr() { - let (mut output, stdout_buf, stderr_buf) = - create_test_user_output(VerbosityLevel::Normal); + let (mut output, stdout_buf, stderr_buf) = create_test_user_output(VerbosityLevel::Normal); output.progress("Testing progress message"); @@ -336,8 +333,7 @@ mod tests { #[test] fn it_should_write_success_messages_to_stderr() { - let (mut output, stdout_buf, stderr_buf) = - create_test_user_output(VerbosityLevel::Normal); + let (mut output, stdout_buf, stderr_buf) = create_test_user_output(VerbosityLevel::Normal); output.success("Testing success message"); @@ -352,8 +348,7 @@ mod tests { #[test] fn it_should_write_warning_messages_to_stderr() { - let (mut output, stdout_buf, stderr_buf) = - create_test_user_output(VerbosityLevel::Normal); + let (mut output, stdout_buf, stderr_buf) = create_test_user_output(VerbosityLevel::Normal); output.warn("Testing warning message"); @@ -368,8 +363,7 @@ mod tests { #[test] fn it_should_write_error_messages_to_stderr() { - let (mut output, stdout_buf, stderr_buf) = - create_test_user_output(VerbosityLevel::Normal); + let (mut output, stdout_buf, stderr_buf) = create_test_user_output(VerbosityLevel::Normal); output.error("Testing error message"); @@ -384,8 +378,7 @@ mod tests { #[test] fn it_should_write_results_to_stdout() { - let (mut output, stdout_buf, stderr_buf) = - create_test_user_output(VerbosityLevel::Normal); + let (mut output, stdout_buf, stderr_buf) = create_test_user_output(VerbosityLevel::Normal); output.result("Test result data"); @@ -400,8 +393,7 @@ mod tests { #[test] fn it_should_write_data_to_stdout() { - let (mut output, stdout_buf, stderr_buf) = - create_test_user_output(VerbosityLevel::Normal); + let (mut output, stdout_buf, stderr_buf) = create_test_user_output(VerbosityLevel::Normal); output.data(r#"{"status": "destroyed"}"#); From 399cc23e0e44b49d4a7a16797d861c35bc2698ad Mon Sep 17 00:00:00 2001 From: Jose Celano Date: Thu, 23 Oct 2025 11:38:23 +0100 Subject: [PATCH 05/14] style: [#23] use derive Default for VerbosityLevel enum Replace manual impl Default with derive attribute and #[default] annotation as suggested by clippy for better maintainability. --- src/shared/user_output.rs | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/src/shared/user_output.rs b/src/shared/user_output.rs index 5259f75d..b34ef6d6 100644 --- a/src/shared/user_output.rs +++ b/src/shared/user_output.rs @@ -55,11 +55,12 @@ use std::io::Write; /// assert!(level >= VerbosityLevel::Quiet); /// assert!(level < VerbosityLevel::Verbose); /// ``` -#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord)] +#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Default)] pub enum VerbosityLevel { /// Minimal output - only errors and final results Quiet, /// Default level - essential progress and results + #[default] Normal, /// Detailed progress including intermediate steps Verbose, @@ -69,12 +70,6 @@ pub enum VerbosityLevel { Debug, } -impl Default for VerbosityLevel { - fn default() -> Self { - Self::Normal - } -} - /// Handles user-facing output separate from internal logging /// /// Uses dual channels following Unix conventions and modern CLI best practices: From 94535bd525efc7a99edb6f97e46d78b55e1db52f Mon Sep 17 00:00:00 2001 From: Jose Celano Date: Thu, 23 Oct 2025 11:45:42 +0100 Subject: [PATCH 06/14] refactor: [#23] move business logic from CLI to DestroyCommandHandler Improve separation of concerns by moving environment loading and state handling logic from presentation layer (CLI) to application layer: - DestroyCommandHandler now loads environment internally from EnvironmentName - CLI simplified to focus only on user interface concerns - Proper build directory path resolution using env.tofu_build_dir() - Eliminates circular dependency between CLI and command handler - Better testability and maintainability of business logic Breaking changes: - DestroyCommandHandler.execute() now takes EnvironmentName instead of Environment - Constructor no longer requires OpenTofuClient (created internally) This follows clean architecture principles with proper layer separation. --- src/app.rs | 51 +------- src/application/command_handlers/destroy.rs | 119 +++++++++++++----- .../virtual_machine/run_destroy_command.rs | 41 ++---- 3 files changed, 101 insertions(+), 110 deletions(-) diff --git a/src/app.rs b/src/app.rs index 4d20f011..af0ac674 100644 --- a/src/app.rs +++ b/src/app.rs @@ -175,12 +175,9 @@ fn display_help_info() { /// This function will return an error if the environment name is invalid, /// the environment cannot be loaded, or the destruction process fails. fn handle_destroy_command(environment_name: &str) -> Result<(), Box> { - use std::sync::Arc; use std::time::Duration; - use torrust_tracker_deployer_lib::adapters::tofu::client::OpenTofuClient; use torrust_tracker_deployer_lib::application::command_handlers::DestroyCommandHandler; use torrust_tracker_deployer_lib::domain::environment::name::EnvironmentName; - use torrust_tracker_deployer_lib::domain::environment::state::AnyEnvironmentState; use torrust_tracker_deployer_lib::infrastructure::persistence::repository_factory::RepositoryFactory; use torrust_tracker_deployer_lib::shared::user_output::{UserOutput, VerbosityLevel}; @@ -198,57 +195,17 @@ fn handle_destroy_command(environment_name: &str) -> Result<(), Box { - output.warn("Environment is already destroyed"); - Ok(env) - } - AnyEnvironmentState::Created(env) => command_handler.execute(env), - AnyEnvironmentState::Provisioning(env) => command_handler.execute(env), - AnyEnvironmentState::Provisioned(env) => command_handler.execute(env), - AnyEnvironmentState::Configuring(env) => command_handler.execute(env), - AnyEnvironmentState::Configured(env) => command_handler.execute(env), - AnyEnvironmentState::Releasing(env) => command_handler.execute(env), - AnyEnvironmentState::Released(env) => command_handler.execute(env), - AnyEnvironmentState::Running(env) => command_handler.execute(env), - AnyEnvironmentState::ProvisionFailed(env) => command_handler.execute(env), - AnyEnvironmentState::ConfigureFailed(env) => command_handler.execute(env), - AnyEnvironmentState::ReleaseFailed(env) => command_handler.execute(env), - AnyEnvironmentState::RunFailed(env) => command_handler.execute(env), - } - .map_err(|e| { + // Execute destroy - the handler will load the environment and handle all states internally + let _destroyed_env = command_handler.execute(&env_name).map_err(|e| { output.error(&format!( "Failed to destroy environment '{environment_name}': {e}" )); diff --git a/src/application/command_handlers/destroy.rs b/src/application/command_handlers/destroy.rs index 34ffa8c2..4e8c90cf 100644 --- a/src/application/command_handlers/destroy.rs +++ b/src/application/command_handlers/destroy.rs @@ -104,28 +104,21 @@ impl crate::shared::Traceable for DestroyCommandHandlerError { /// - Report appropriate status to the user /// - Not fail due to missing resources pub struct DestroyCommandHandler { - opentofu_client: Arc, repository: Arc, } impl DestroyCommandHandler { /// Create a new `DestroyCommandHandler` #[must_use] - pub fn new( - opentofu_client: Arc, - repository: Arc, - ) -> Self { - Self { - opentofu_client, - repository, - } + pub fn new(repository: Arc) -> Self { + Self { repository } } /// Execute the complete destruction workflow /// /// # Arguments /// - /// * `environment` - The environment to destroy (can be in any state) + /// * `env_name` - The name of the environment to destroy /// /// # Returns /// @@ -134,6 +127,8 @@ impl DestroyCommandHandler { /// # Errors /// /// Returns an error if any step in the destruction workflow fails: + /// * Environment not found or cannot be loaded + /// * Environment is in an invalid state for destruction /// * `OpenTofu` destroy fails /// * Unable to persist the destroyed state /// @@ -144,30 +139,92 @@ impl DestroyCommandHandler { skip_all, fields( command_type = "destroy", - environment = %environment.name() + environment = %env_name ) )] - pub fn execute( + pub fn execute( &self, - environment: Environment, - ) -> Result, DestroyCommandHandlerError> { + env_name: &crate::domain::environment::name::EnvironmentName, + ) -> Result, DestroyCommandHandlerError> + { + use crate::domain::environment::state::AnyEnvironmentState; + info!( command = "destroy", - environment = %environment.name(), + environment = %env_name, "Starting complete infrastructure destruction workflow" ); - // Execute infrastructure destruction - // OpenTofu destroy is idempotent - it will succeed even if infrastructure doesn't exist - self.destroy_infrastructure()?; + // 1. Load the environment from storage + let environment = self + .repository + .load(env_name) + .map_err(DestroyCommandHandlerError::StatePersistence)?; + + // 2. Check if environment exists + let environment = environment.ok_or_else(|| { + DestroyCommandHandlerError::StatePersistence( + crate::domain::environment::repository::RepositoryError::NotFound, + ) + })?; + + // 3. Check if environment is already destroyed + if let AnyEnvironmentState::Destroyed(env) = environment { + info!( + command = "destroy", + environment = %env_name, + "Environment is already destroyed" + ); + return Ok(env); + } + + // 4. Get the build directory from the environment context + let opentofu_build_dir = match &environment { + AnyEnvironmentState::Created(env) => env.tofu_build_dir().join("lxd"), + AnyEnvironmentState::Provisioning(env) => env.tofu_build_dir().join("lxd"), + AnyEnvironmentState::Provisioned(env) => env.tofu_build_dir().join("lxd"), + AnyEnvironmentState::Configuring(env) => env.tofu_build_dir().join("lxd"), + AnyEnvironmentState::Configured(env) => env.tofu_build_dir().join("lxd"), + AnyEnvironmentState::Releasing(env) => env.tofu_build_dir().join("lxd"), + AnyEnvironmentState::Released(env) => env.tofu_build_dir().join("lxd"), + AnyEnvironmentState::Running(env) => env.tofu_build_dir().join("lxd"), + AnyEnvironmentState::ProvisionFailed(env) => env.tofu_build_dir().join("lxd"), + AnyEnvironmentState::ConfigureFailed(env) => env.tofu_build_dir().join("lxd"), + AnyEnvironmentState::ReleaseFailed(env) => env.tofu_build_dir().join("lxd"), + AnyEnvironmentState::RunFailed(env) => env.tofu_build_dir().join("lxd"), + AnyEnvironmentState::Destroyed(_) => unreachable!("Already handled above"), + }; + + // 5. Create OpenTofu client with correct build directory + let opentofu_client = Arc::new(crate::adapters::tofu::client::OpenTofuClient::new( + opentofu_build_dir, + )); - // Transition to Destroyed state - let destroyed = environment.destroy(); + // 6. Execute infrastructure destruction + // OpenTofu destroy is idempotent - it will succeed even if infrastructure doesn't exist + Self::destroy_infrastructure(&opentofu_client)?; + + // 7. Transition to Destroyed state based on current state + let destroyed = match environment { + AnyEnvironmentState::Created(env) => env.destroy(), + AnyEnvironmentState::Provisioning(env) => env.destroy(), + AnyEnvironmentState::Provisioned(env) => env.destroy(), + AnyEnvironmentState::Configuring(env) => env.destroy(), + AnyEnvironmentState::Configured(env) => env.destroy(), + AnyEnvironmentState::Releasing(env) => env.destroy(), + AnyEnvironmentState::Released(env) => env.destroy(), + AnyEnvironmentState::Running(env) => env.destroy(), + AnyEnvironmentState::ProvisionFailed(env) => env.destroy(), + AnyEnvironmentState::ConfigureFailed(env) => env.destroy(), + AnyEnvironmentState::ReleaseFailed(env) => env.destroy(), + AnyEnvironmentState::RunFailed(env) => env.destroy(), + AnyEnvironmentState::Destroyed(_) => unreachable!("Already handled above"), + }; - // Clean up state files only after successful infrastructure destruction + // 8. Clean up state files only after successful infrastructure destruction Self::cleanup_state_files(&destroyed)?; - // Persist final state + // 9. Persist final state self.repository.save(&destroyed.clone().into_any())?; info!( @@ -185,11 +242,17 @@ impl DestroyCommandHandler { /// /// Executes the `OpenTofu` destroy workflow to remove all managed infrastructure. /// + /// # Arguments + /// + /// * `opentofu_client` - The `OpenTofu` client configured with the correct build directory + /// /// # Errors /// /// Returns an error if `OpenTofu` destroy fails - fn destroy_infrastructure(&self) -> Result<(), DestroyCommandHandlerError> { - DestroyInfrastructureStep::new(Arc::clone(&self.opentofu_client)).execute()?; + fn destroy_infrastructure( + opentofu_client: &Arc, + ) -> Result<(), DestroyCommandHandlerError> { + DestroyInfrastructureStep::new(Arc::clone(opentofu_client)).execute()?; Ok(()) } @@ -271,17 +334,13 @@ mod tests { /// Returns: (`command_handler`, `temp_dir`) /// The `temp_dir` must be kept alive for the duration of the test. pub fn build(self) -> (DestroyCommandHandler, TempDir) { - let opentofu_client = Arc::new(crate::adapters::tofu::client::OpenTofuClient::new( - self.temp_dir.path(), - )); - let repository_factory = crate::infrastructure::persistence::repository_factory::RepositoryFactory::new( std::time::Duration::from_secs(30), ); let repository = repository_factory.create(self.temp_dir.path().to_path_buf()); - let command_handler = DestroyCommandHandler::new(opentofu_client, repository); + let command_handler = DestroyCommandHandler::new(repository); (command_handler, self.temp_dir) } @@ -293,7 +352,7 @@ mod tests { // Verify the command handler was created (basic structure test) // This test just verifies that the command handler can be created with the dependencies - assert_eq!(Arc::strong_count(&command_handler.opentofu_client), 1); + assert_eq!(Arc::strong_count(&command_handler.repository), 1); } #[test] diff --git a/src/testing/e2e/tasks/virtual_machine/run_destroy_command.rs b/src/testing/e2e/tasks/virtual_machine/run_destroy_command.rs index e6cd5ed8..c4db65a3 100644 --- a/src/testing/e2e/tasks/virtual_machine/run_destroy_command.rs +++ b/src/testing/e2e/tasks/virtual_machine/run_destroy_command.rs @@ -16,7 +16,6 @@ //! This task is typically the final step in E2E testing workflows, cleaning up //! all provisioned infrastructure after tests complete. -use std::sync::Arc; use thiserror::Error; use tracing::info; @@ -105,8 +104,6 @@ For more information, see docs/e2e-testing.md and docs/vm-providers.md." /// - Infrastructure destruction fails /// - `OpenTofu` destroy operations fail pub fn run_destroy_command(test_context: &mut TestContext) -> Result<(), DestroyTaskError> { - use crate::domain::environment::state::AnyEnvironmentState; - // If keep_env is set, skip destruction and preserve the environment if test_context.keep_env { let instance_name = &test_context.environment.instance_name(); @@ -126,36 +123,14 @@ pub fn run_destroy_command(test_context: &mut TestContext) -> Result<(), Destroy let repository = test_context.create_repository(); // Use the new DestroyCommandHandler to handle all infrastructure destruction steps - let destroy_command_handler = DestroyCommandHandler::new( - Arc::clone(&test_context.services.opentofu_client), - repository, - ); - - // Execute destruction with environment (can be in any state) - // The DestroyCommandHandler accepts Environment generically, so we need to extract - // the environment from AnyEnvironmentState. Since destroy works on any state, - // we handle the special case of already-destroyed environments. - - let destroyed_env = match test_context.environment.clone() { - AnyEnvironmentState::Destroyed(env) => { - // Already destroyed, just return it - info!("Environment is already in Destroyed state"); - Ok(env) - } - AnyEnvironmentState::Created(env) => destroy_command_handler.execute(env), - AnyEnvironmentState::Provisioning(env) => destroy_command_handler.execute(env), - AnyEnvironmentState::Provisioned(env) => destroy_command_handler.execute(env), - AnyEnvironmentState::Configuring(env) => destroy_command_handler.execute(env), - AnyEnvironmentState::Configured(env) => destroy_command_handler.execute(env), - AnyEnvironmentState::Releasing(env) => destroy_command_handler.execute(env), - AnyEnvironmentState::Released(env) => destroy_command_handler.execute(env), - AnyEnvironmentState::Running(env) => destroy_command_handler.execute(env), - AnyEnvironmentState::ProvisionFailed(env) => destroy_command_handler.execute(env), - AnyEnvironmentState::ConfigureFailed(env) => destroy_command_handler.execute(env), - AnyEnvironmentState::ReleaseFailed(env) => destroy_command_handler.execute(env), - AnyEnvironmentState::RunFailed(env) => destroy_command_handler.execute(env), - } - .map_err(|source| DestroyTaskError::DestructionFailed { source })?; + let destroy_command_handler = DestroyCommandHandler::new(repository); + + // Execute destruction using environment name + // The DestroyCommandHandler now loads the environment internally and handles all states + let env_name = test_context.environment.name(); + let destroyed_env = destroy_command_handler + .execute(env_name) + .map_err(|source| DestroyTaskError::DestructionFailed { source })?; info!( status = "complete", From af408fbf8d1a0f28560e6c822d9dcaf88101a18c Mon Sep 17 00:00:00 2001 From: Jose Celano Date: Thu, 23 Oct 2025 13:33:59 +0100 Subject: [PATCH 07/14] refactor: [#23] consolidate repetitive match patterns in AnyEnvironmentState - Add destroy() method to AnyEnvironmentState for unified state destruction - Add tofu_build_dir() method to AnyEnvironmentState for unified directory access - Update DestroyCommandHandler to use new methods with Result error handling - Add StateTypeError integration with Clone derive for error propagation - Remove 24 lines of repetitive match patterns across two locations - Add comprehensive test coverage for new methods and error cases --- src/application/command_handlers/destroy.rs | 44 ++----- src/domain/environment/state/mod.rs | 127 +++++++++++++++++++- 2 files changed, 139 insertions(+), 32 deletions(-) diff --git a/src/application/command_handlers/destroy.rs b/src/application/command_handlers/destroy.rs index 4e8c90cf..5457bb6b 100644 --- a/src/application/command_handlers/destroy.rs +++ b/src/application/command_handlers/destroy.rs @@ -17,6 +17,7 @@ use tracing::{info, instrument}; use crate::adapters::tofu::client::OpenTofuError; use crate::application::steps::DestroyInfrastructureStep; use crate::domain::environment::repository::EnvironmentRepository; +use crate::domain::environment::state::StateTypeError; use crate::domain::environment::{Destroyed, Environment}; use crate::shared::command::CommandError; @@ -32,6 +33,9 @@ pub enum DestroyCommandHandlerError { #[error("Failed to persist environment state: {0}")] StatePersistence(#[from] crate::domain::environment::repository::RepositoryError), + #[error("Invalid state transition: {0}")] + StateTransition(#[from] StateTypeError), + #[error("Failed to clean up state files at '{path}': {source}")] StateCleanupFailed { path: PathBuf, @@ -52,6 +56,9 @@ impl crate::shared::Traceable for DestroyCommandHandlerError { Self::StatePersistence(e) => { format!("DestroyCommandHandlerError: Failed to persist environment state - {e}") } + Self::StateTransition(e) => { + format!("DestroyCommandHandlerError: Invalid state transition - {e}") + } Self::StateCleanupFailed { path, source } => { format!( "DestroyCommandHandlerError: Failed to clean up state files at '{}' - {source}", @@ -65,7 +72,9 @@ impl crate::shared::Traceable for DestroyCommandHandlerError { match self { Self::OpenTofu(e) => Some(e), Self::Command(e) => Some(e), - Self::StatePersistence(_) | Self::StateCleanupFailed { .. } => None, + Self::StatePersistence(_) + | Self::StateTransition(_) + | Self::StateCleanupFailed { .. } => None, } } @@ -73,6 +82,7 @@ impl crate::shared::Traceable for DestroyCommandHandlerError { match self { Self::OpenTofu(_) => crate::shared::ErrorKind::InfrastructureOperation, Self::Command(_) => crate::shared::ErrorKind::CommandExecution, + Self::StateTransition(_) => crate::shared::ErrorKind::Configuration, Self::StatePersistence(_) | Self::StateCleanupFailed { .. } => { crate::shared::ErrorKind::StatePersistence } @@ -179,21 +189,7 @@ impl DestroyCommandHandler { } // 4. Get the build directory from the environment context - let opentofu_build_dir = match &environment { - AnyEnvironmentState::Created(env) => env.tofu_build_dir().join("lxd"), - AnyEnvironmentState::Provisioning(env) => env.tofu_build_dir().join("lxd"), - AnyEnvironmentState::Provisioned(env) => env.tofu_build_dir().join("lxd"), - AnyEnvironmentState::Configuring(env) => env.tofu_build_dir().join("lxd"), - AnyEnvironmentState::Configured(env) => env.tofu_build_dir().join("lxd"), - AnyEnvironmentState::Releasing(env) => env.tofu_build_dir().join("lxd"), - AnyEnvironmentState::Released(env) => env.tofu_build_dir().join("lxd"), - AnyEnvironmentState::Running(env) => env.tofu_build_dir().join("lxd"), - AnyEnvironmentState::ProvisionFailed(env) => env.tofu_build_dir().join("lxd"), - AnyEnvironmentState::ConfigureFailed(env) => env.tofu_build_dir().join("lxd"), - AnyEnvironmentState::ReleaseFailed(env) => env.tofu_build_dir().join("lxd"), - AnyEnvironmentState::RunFailed(env) => env.tofu_build_dir().join("lxd"), - AnyEnvironmentState::Destroyed(_) => unreachable!("Already handled above"), - }; + let opentofu_build_dir = environment.tofu_build_dir()?; // 5. Create OpenTofu client with correct build directory let opentofu_client = Arc::new(crate::adapters::tofu::client::OpenTofuClient::new( @@ -205,21 +201,7 @@ impl DestroyCommandHandler { Self::destroy_infrastructure(&opentofu_client)?; // 7. Transition to Destroyed state based on current state - let destroyed = match environment { - AnyEnvironmentState::Created(env) => env.destroy(), - AnyEnvironmentState::Provisioning(env) => env.destroy(), - AnyEnvironmentState::Provisioned(env) => env.destroy(), - AnyEnvironmentState::Configuring(env) => env.destroy(), - AnyEnvironmentState::Configured(env) => env.destroy(), - AnyEnvironmentState::Releasing(env) => env.destroy(), - AnyEnvironmentState::Released(env) => env.destroy(), - AnyEnvironmentState::Running(env) => env.destroy(), - AnyEnvironmentState::ProvisionFailed(env) => env.destroy(), - AnyEnvironmentState::ConfigureFailed(env) => env.destroy(), - AnyEnvironmentState::ReleaseFailed(env) => env.destroy(), - AnyEnvironmentState::RunFailed(env) => env.destroy(), - AnyEnvironmentState::Destroyed(_) => unreachable!("Already handled above"), - }; + let destroyed = environment.destroy()?; // 8. Clean up state files only after successful infrastructure destruction Self::cleanup_state_files(&destroyed)?; diff --git a/src/domain/environment/state/mod.rs b/src/domain/environment/state/mod.rs index 8bdfc3a2..f01688f5 100644 --- a/src/domain/environment/state/mod.rs +++ b/src/domain/environment/state/mod.rs @@ -90,7 +90,7 @@ pub use running::Running; /// // let result = any_env.try_into_created(); /// // assert!(result.is_err()); /// ``` -#[derive(Debug, Error)] +#[derive(Debug, Clone, Error)] pub enum StateTypeError { /// The environment is in a different state than expected #[error("Expected state '{expected}', but found '{actual}'")] @@ -410,6 +410,79 @@ impl AnyEnvironmentState { pub fn instance_ip(&self) -> Option { self.context().runtime_outputs.instance_ip } + + /// Destroy the environment, transitioning it to the Destroyed state + /// + /// This method provides a unified interface to destroy an environment + /// regardless of its current state. It encapsulates the repetitive match + /// pattern that would otherwise be needed in calling code. + /// + /// # Returns + /// + /// - `Ok(Environment)` if the environment was successfully destroyed + /// - `Err(StateTypeError)` if the environment is already in the `Destroyed` state + /// + /// # Errors + /// + /// Returns `StateTypeError::UnexpectedState` if called on an environment + /// already in the `Destroyed` state. + pub fn destroy(self) -> Result, StateTypeError> { + match self { + Self::Created(env) => Ok(env.destroy()), + Self::Provisioning(env) => Ok(env.destroy()), + Self::Provisioned(env) => Ok(env.destroy()), + Self::Configuring(env) => Ok(env.destroy()), + Self::Configured(env) => Ok(env.destroy()), + Self::Releasing(env) => Ok(env.destroy()), + Self::Released(env) => Ok(env.destroy()), + Self::Running(env) => Ok(env.destroy()), + Self::ProvisionFailed(env) => Ok(env.destroy()), + Self::ConfigureFailed(env) => Ok(env.destroy()), + Self::ReleaseFailed(env) => Ok(env.destroy()), + Self::RunFailed(env) => Ok(env.destroy()), + Self::Destroyed(_) => Err(StateTypeError::UnexpectedState { + expected: "any state except destroyed", + actual: "destroyed".to_string(), + }), + } + } + + /// Get the `OpenTofu` build directory path regardless of current state + /// + /// This method provides a unified interface to access the build directory + /// for `OpenTofu` operations. It encapsulates the repetitive match pattern + /// that would otherwise be needed in calling code. + /// + /// # Returns + /// + /// - `Ok(PathBuf)` containing the path to the `OpenTofu` build directory (with "lxd" subdirectory) + /// - `Err(StateTypeError)` if called on an environment in the `Destroyed` state + /// + /// # Errors + /// + /// Returns `StateTypeError::UnexpectedState` if called on an environment + /// in the `Destroyed` state since destroyed environments don't have build directories. + #[must_use = "build directory path should be used for OpenTofu operations"] + pub fn tofu_build_dir(&self) -> Result { + match self { + Self::Created(env) => Ok(env.tofu_build_dir().join("lxd")), + Self::Provisioning(env) => Ok(env.tofu_build_dir().join("lxd")), + Self::Provisioned(env) => Ok(env.tofu_build_dir().join("lxd")), + Self::Configuring(env) => Ok(env.tofu_build_dir().join("lxd")), + Self::Configured(env) => Ok(env.tofu_build_dir().join("lxd")), + Self::Releasing(env) => Ok(env.tofu_build_dir().join("lxd")), + Self::Released(env) => Ok(env.tofu_build_dir().join("lxd")), + Self::Running(env) => Ok(env.tofu_build_dir().join("lxd")), + Self::ProvisionFailed(env) => Ok(env.tofu_build_dir().join("lxd")), + Self::ConfigureFailed(env) => Ok(env.tofu_build_dir().join("lxd")), + Self::ReleaseFailed(env) => Ok(env.tofu_build_dir().join("lxd")), + Self::RunFailed(env) => Ok(env.tofu_build_dir().join("lxd")), + Self::Destroyed(_) => Err(StateTypeError::UnexpectedState { + expected: "any state except destroyed", + actual: "destroyed".to_string(), + }), + } + } } /// Display implementation for user-friendly state representation @@ -1028,6 +1101,58 @@ mod tests { error_message ); } + + // Tests for new utility methods + + #[test] + fn it_should_destroy_environment_from_any_state() { + let env = super::create_test_environment_created(); + let any_env = AnyEnvironmentState::Created(env); + + let destroyed = any_env.destroy().unwrap(); + // Convert back to any state to check state name + let destroyed_any = destroyed.into_any(); + assert_eq!(destroyed_any.state_name(), "destroyed"); + } + + #[test] + fn it_should_get_tofu_build_dir_from_any_state() { + let env = super::create_test_environment_created(); + let any_env = AnyEnvironmentState::Created(env); + + let build_dir = any_env.tofu_build_dir().unwrap(); + assert!(build_dir.ends_with("lxd")); + } + + #[test] + fn it_should_error_when_destroying_already_destroyed_environment() { + let env = super::create_test_environment_created().destroy(); + let any_env = AnyEnvironmentState::Destroyed(env); + + // This should return an error + let result = any_env.destroy(); + assert!(result.is_err()); + let error = result.unwrap_err(); + assert_eq!( + error.to_string(), + "Expected state 'any state except destroyed', but found 'destroyed'" + ); + } + + #[test] + fn it_should_error_when_accessing_build_dir_of_destroyed_environment() { + let env = super::create_test_environment_created().destroy(); + let any_env = AnyEnvironmentState::Destroyed(env); + + // This should return an error + let result = any_env.tofu_build_dir(); + assert!(result.is_err()); + let error = result.unwrap_err(); + assert_eq!( + error.to_string(), + "Expected state 'any state except destroyed', but found 'destroyed'" + ); + } } mod introspection_tests { From 1e50d4dba01de7a7101f3e1ccf8c555c56f696a7 Mon Sep 17 00:00:00 2001 From: Jose Celano Date: Thu, 23 Oct 2025 13:38:52 +0100 Subject: [PATCH 08/14] refactor: [#23] simplify tofu_build_dir to always return path consistently - Remove Result wrapper from tofu_build_dir method - now returns PathBuf directly - Include Destroyed state in pattern matching like all other states - Simplify caller code in DestroyCommandHandler (no more error handling needed) - Update tests to reflect the simplified interface - Improve consistency: getter methods should return same result regardless of state - Let caller decide how to use the path based on their specific needs --- src/application/command_handlers/destroy.rs | 2 +- src/domain/environment/state/mod.rs | 58 +++++++++------------ 2 files changed, 25 insertions(+), 35 deletions(-) diff --git a/src/application/command_handlers/destroy.rs b/src/application/command_handlers/destroy.rs index 5457bb6b..f1bdd628 100644 --- a/src/application/command_handlers/destroy.rs +++ b/src/application/command_handlers/destroy.rs @@ -189,7 +189,7 @@ impl DestroyCommandHandler { } // 4. Get the build directory from the environment context - let opentofu_build_dir = environment.tofu_build_dir()?; + let opentofu_build_dir = environment.tofu_build_dir(); // 5. Create OpenTofu client with correct build directory let opentofu_client = Arc::new(crate::adapters::tofu::client::OpenTofuClient::new( diff --git a/src/domain/environment/state/mod.rs b/src/domain/environment/state/mod.rs index f01688f5..c8bac8d2 100644 --- a/src/domain/environment/state/mod.rs +++ b/src/domain/environment/state/mod.rs @@ -453,34 +453,29 @@ impl AnyEnvironmentState { /// for `OpenTofu` operations. It encapsulates the repetitive match pattern /// that would otherwise be needed in calling code. /// - /// # Returns - /// - /// - `Ok(PathBuf)` containing the path to the `OpenTofu` build directory (with "lxd" subdirectory) - /// - `Err(StateTypeError)` if called on an environment in the `Destroyed` state + /// The path is returned consistently regardless of the environment's state. + /// The caller is responsible for determining how to use the path based on + /// their specific needs and the environment's current state. /// - /// # Errors + /// # Returns /// - /// Returns `StateTypeError::UnexpectedState` if called on an environment - /// in the `Destroyed` state since destroyed environments don't have build directories. + /// The path to the `OpenTofu` build directory (with "lxd" subdirectory). #[must_use = "build directory path should be used for OpenTofu operations"] - pub fn tofu_build_dir(&self) -> Result { + pub fn tofu_build_dir(&self) -> std::path::PathBuf { match self { - Self::Created(env) => Ok(env.tofu_build_dir().join("lxd")), - Self::Provisioning(env) => Ok(env.tofu_build_dir().join("lxd")), - Self::Provisioned(env) => Ok(env.tofu_build_dir().join("lxd")), - Self::Configuring(env) => Ok(env.tofu_build_dir().join("lxd")), - Self::Configured(env) => Ok(env.tofu_build_dir().join("lxd")), - Self::Releasing(env) => Ok(env.tofu_build_dir().join("lxd")), - Self::Released(env) => Ok(env.tofu_build_dir().join("lxd")), - Self::Running(env) => Ok(env.tofu_build_dir().join("lxd")), - Self::ProvisionFailed(env) => Ok(env.tofu_build_dir().join("lxd")), - Self::ConfigureFailed(env) => Ok(env.tofu_build_dir().join("lxd")), - Self::ReleaseFailed(env) => Ok(env.tofu_build_dir().join("lxd")), - Self::RunFailed(env) => Ok(env.tofu_build_dir().join("lxd")), - Self::Destroyed(_) => Err(StateTypeError::UnexpectedState { - expected: "any state except destroyed", - actual: "destroyed".to_string(), - }), + Self::Created(env) => env.tofu_build_dir().join("lxd"), + Self::Provisioning(env) => env.tofu_build_dir().join("lxd"), + Self::Provisioned(env) => env.tofu_build_dir().join("lxd"), + Self::Configuring(env) => env.tofu_build_dir().join("lxd"), + Self::Configured(env) => env.tofu_build_dir().join("lxd"), + Self::Releasing(env) => env.tofu_build_dir().join("lxd"), + Self::Released(env) => env.tofu_build_dir().join("lxd"), + Self::Running(env) => env.tofu_build_dir().join("lxd"), + Self::ProvisionFailed(env) => env.tofu_build_dir().join("lxd"), + Self::ConfigureFailed(env) => env.tofu_build_dir().join("lxd"), + Self::ReleaseFailed(env) => env.tofu_build_dir().join("lxd"), + Self::RunFailed(env) => env.tofu_build_dir().join("lxd"), + Self::Destroyed(env) => env.tofu_build_dir().join("lxd"), } } } @@ -1120,7 +1115,7 @@ mod tests { let env = super::create_test_environment_created(); let any_env = AnyEnvironmentState::Created(env); - let build_dir = any_env.tofu_build_dir().unwrap(); + let build_dir = any_env.tofu_build_dir(); assert!(build_dir.ends_with("lxd")); } @@ -1140,18 +1135,13 @@ mod tests { } #[test] - fn it_should_error_when_accessing_build_dir_of_destroyed_environment() { + fn it_should_get_tofu_build_dir_from_destroyed_environment() { let env = super::create_test_environment_created().destroy(); let any_env = AnyEnvironmentState::Destroyed(env); - // This should return an error - let result = any_env.tofu_build_dir(); - assert!(result.is_err()); - let error = result.unwrap_err(); - assert_eq!( - error.to_string(), - "Expected state 'any state except destroyed', but found 'destroyed'" - ); + // This should now always return the path, even for destroyed environments + let build_dir = any_env.tofu_build_dir(); + assert!(build_dir.ends_with("lxd")); } } From 349894e66f48d4214695499efb473f8d1f6fbdcc Mon Sep 17 00:00:00 2001 From: Jose Celano Date: Thu, 23 Oct 2025 14:05:50 +0100 Subject: [PATCH 09/14] refactor: [#23] use LXD_PROVIDER_NAME constant instead of hardcoded string - Add LXD_PROVIDER_NAME constant to domain::environment module - Replace hardcoded 'lxd' string in tofu_build_dir() with constant reference - Improves maintainability and follows existing pattern with other constants - All 847 tests pass with comprehensive E2E validation --- src/domain/environment/internal_config.rs | 8 +++++--- src/domain/environment/mod.rs | 7 +++++-- src/domain/environment/state/mod.rs | 24 +++++------------------ 3 files changed, 15 insertions(+), 24 deletions(-) diff --git a/src/domain/environment/internal_config.rs b/src/domain/environment/internal_config.rs index 98c8f292..5db05d9c 100644 --- a/src/domain/environment/internal_config.rs +++ b/src/domain/environment/internal_config.rs @@ -111,12 +111,14 @@ impl InternalConfig { self.build_dir.join(super::ANSIBLE_DIR_NAME) } - /// Returns the tofu build directory + /// Returns the `OpenTofu` build directory for the LXD provider /// - /// Path: `build/{env_name}/tofu` + /// Path: `build/{env_name}/tofu/lxd` #[must_use] pub fn tofu_build_dir(&self) -> PathBuf { - self.build_dir.join(super::TOFU_DIR_NAME) + self.build_dir + .join(super::TOFU_DIR_NAME) + .join(super::LXD_PROVIDER_NAME) } /// Returns the ansible templates directory diff --git a/src/domain/environment/mod.rs b/src/domain/environment/mod.rs index 9cad504d..51f36056 100644 --- a/src/domain/environment/mod.rs +++ b/src/domain/environment/mod.rs @@ -139,6 +139,9 @@ pub const ANSIBLE_DIR_NAME: &str = "ansible"; /// Directory name for OpenTofu-related files pub const TOFU_DIR_NAME: &str = "tofu"; +/// Provider name for LXD infrastructure +pub const LXD_PROVIDER_NAME: &str = "lxd"; + /// Environment configuration encapsulating all environment-specific settings /// /// This entity represents a complete environment configuration including naming, @@ -602,7 +605,7 @@ impl Environment { /// /// assert_eq!( /// environment.tofu_build_dir(), - /// PathBuf::from("build/test/tofu") + /// PathBuf::from("build/test/tofu/lxd") /// ); /// /// # Ok::<(), Box>(()) @@ -978,7 +981,7 @@ mod tests { // Assert assert_eq!(ansible_dir, build_dir.join("ansible")); - assert_eq!(tofu_dir, build_dir.join("tofu")); + assert_eq!(tofu_dir, build_dir.join("tofu").join("lxd")); } #[test] diff --git a/src/domain/environment/state/mod.rs b/src/domain/environment/state/mod.rs index c8bac8d2..b7789986 100644 --- a/src/domain/environment/state/mod.rs +++ b/src/domain/environment/state/mod.rs @@ -450,8 +450,8 @@ impl AnyEnvironmentState { /// Get the `OpenTofu` build directory path regardless of current state /// /// This method provides a unified interface to access the build directory - /// for `OpenTofu` operations. It encapsulates the repetitive match pattern - /// that would otherwise be needed in calling code. + /// for `OpenTofu` operations without needing to pattern match on the + /// specific state variant. /// /// The path is returned consistently regardless of the environment's state. /// The caller is responsible for determining how to use the path based on @@ -459,24 +459,10 @@ impl AnyEnvironmentState { /// /// # Returns /// - /// The path to the `OpenTofu` build directory (with "lxd" subdirectory). - #[must_use = "build directory path should be used for OpenTofu operations"] + /// The path to the `OpenTofu` build directory for the LXD provider. + #[must_use] pub fn tofu_build_dir(&self) -> std::path::PathBuf { - match self { - Self::Created(env) => env.tofu_build_dir().join("lxd"), - Self::Provisioning(env) => env.tofu_build_dir().join("lxd"), - Self::Provisioned(env) => env.tofu_build_dir().join("lxd"), - Self::Configuring(env) => env.tofu_build_dir().join("lxd"), - Self::Configured(env) => env.tofu_build_dir().join("lxd"), - Self::Releasing(env) => env.tofu_build_dir().join("lxd"), - Self::Released(env) => env.tofu_build_dir().join("lxd"), - Self::Running(env) => env.tofu_build_dir().join("lxd"), - Self::ProvisionFailed(env) => env.tofu_build_dir().join("lxd"), - Self::ConfigureFailed(env) => env.tofu_build_dir().join("lxd"), - Self::ReleaseFailed(env) => env.tofu_build_dir().join("lxd"), - Self::RunFailed(env) => env.tofu_build_dir().join("lxd"), - Self::Destroyed(env) => env.tofu_build_dir().join("lxd"), - } + self.context().tofu_build_dir() } } From cca9a9e96117d62ca8d6fa3756662d1c13274ec9 Mon Sep 17 00:00:00 2001 From: Jose Celano Date: Thu, 23 Oct 2025 14:12:31 +0100 Subject: [PATCH 10/14] refactor: [#23] move user_output to presentation layer following DDD architecture - Create new src/presentation/ layer for presentation concerns - Move user_output.rs from shared/ to presentation/ layer - Update all import paths from shared::user_output to presentation::user_output - Add proper presentation layer module documentation - Update lib.rs to include presentation module in DDD architecture - Update documentation references to reflect new file location - Maintains proper DDD separation: presentation handles user-facing output The user_output module represents presentation layer logic in DDD and belongs with other presentation concerns rather than shared utilities. --- .../23-add-clap-subcommand-configuration.md | 18 +++++++------- src/app.rs | 2 +- src/lib.rs | 2 ++ src/presentation/mod.rs | 24 +++++++++++++++++++ src/{shared => presentation}/user_output.rs | 22 ++++++++--------- src/shared/mod.rs | 2 -- 6 files changed, 47 insertions(+), 23 deletions(-) create mode 100644 src/presentation/mod.rs rename src/{shared => presentation}/user_output.rs (93%) diff --git a/docs/issues/23-add-clap-subcommand-configuration.md b/docs/issues/23-add-clap-subcommand-configuration.md index 41fe200d..7c33e2f7 100644 --- a/docs/issues/23-add-clap-subcommand-configuration.md +++ b/docs/issues/23-add-clap-subcommand-configuration.md @@ -57,7 +57,7 @@ pub enum Commands { ### VerbosityLevel Implementation -Create `src/shared/user_output.rs`: +Create `src/presentation/user_output.rs`: ```rust /// Verbosity levels for user output @@ -303,7 +303,7 @@ Update command execution in `src/app.rs`: ```rust async fn handle_destroy_command(environment: String) -> anyhow::Result<()> { use crate::application::command_handlers::destroy::DestroyCommandHandler; - use crate::shared::user_output::{UserOutput, VerbosityLevel}; + use crate::presentation::user_output::{UserOutput, VerbosityLevel}; // Create UserOutput with default stdout/stderr channels let mut output = UserOutput::new(VerbosityLevel::Normal); @@ -405,13 +405,13 @@ output.error("Failed to destroy environment 'my-env': OpenTofu destroy operation ### Subtask 1: Implement VerbosityLevel and UserOutput (1.5 hours) -- [ ] Create `src/shared/user_output.rs` -- [ ] Implement `VerbosityLevel` enum with 5 levels -- [ ] Implement `UserOutput` struct with dual writers (stdout/stderr) -- [ ] Add methods: `progress`, `success`, `warn`, `error` (stderr), `result`, `data` (stdout) -- [ ] Add constructor for default channels and testing with custom writers -- [ ] Document channel usage strategy based on console patterns research -- [ ] Update `src/shared/mod.rs` to export new module +- [x] Create `src/presentation/user_output.rs` (moved from shared to presentation layer) +- [x] Implement `VerbosityLevel` enum with 5 levels +- [x] Implement `UserOutput` struct with dual writers (stdout/stderr) +- [x] Add methods: `progress`, `success`, `warn`, `error` (stderr), `result`, `data` (stdout) +- [x] Add constructor for default channels and testing with custom writers +- [x] Document channel usage strategy based on console patterns research +- [x] Update `src/presentation/mod.rs` to export new module ### Subtask 2: Add Destroy Subcommand to Clap (45 minutes) diff --git a/src/app.rs b/src/app.rs index af0ac674..e044a9f4 100644 --- a/src/app.rs +++ b/src/app.rs @@ -179,7 +179,7 @@ fn handle_destroy_command(environment_name: &str) -> Result<(), Box= VerbosityLevel::Quiet); @@ -89,7 +89,7 @@ pub enum VerbosityLevel { /// # Examples /// /// ```rust -/// use torrust_tracker_deployer_lib::shared::user_output::{UserOutput, VerbosityLevel}; +/// use torrust_tracker_deployer_lib::presentation::user_output::{UserOutput, VerbosityLevel}; /// /// let mut output = UserOutput::new(VerbosityLevel::Normal); /// @@ -111,7 +111,7 @@ impl UserOutput { /// # Examples /// /// ```rust - /// use torrust_tracker_deployer_lib::shared::user_output::{UserOutput, VerbosityLevel}; + /// use torrust_tracker_deployer_lib::presentation::user_output::{UserOutput, VerbosityLevel}; /// /// let output = UserOutput::new(VerbosityLevel::Normal); /// ``` @@ -132,7 +132,7 @@ impl UserOutput { /// # Examples /// /// ```rust - /// use torrust_tracker_deployer_lib::shared::user_output::{UserOutput, VerbosityLevel}; + /// use torrust_tracker_deployer_lib::presentation::user_output::{UserOutput, VerbosityLevel}; /// use std::io::Cursor; /// /// let stdout_buf = Vec::new(); @@ -165,7 +165,7 @@ impl UserOutput { /// # Examples /// /// ```rust - /// use torrust_tracker_deployer_lib::shared::user_output::{UserOutput, VerbosityLevel}; + /// use torrust_tracker_deployer_lib::presentation::user_output::{UserOutput, VerbosityLevel}; /// /// let mut output = UserOutput::new(VerbosityLevel::Normal); /// output.progress("Destroying environment..."); @@ -184,7 +184,7 @@ impl UserOutput { /// # Examples /// /// ```rust - /// use torrust_tracker_deployer_lib::shared::user_output::{UserOutput, VerbosityLevel}; + /// use torrust_tracker_deployer_lib::presentation::user_output::{UserOutput, VerbosityLevel}; /// /// let mut output = UserOutput::new(VerbosityLevel::Normal); /// output.success("Environment destroyed successfully"); @@ -201,7 +201,7 @@ impl UserOutput { /// # Examples /// /// ```rust - /// use torrust_tracker_deployer_lib::shared::user_output::{UserOutput, VerbosityLevel}; + /// use torrust_tracker_deployer_lib::presentation::user_output::{UserOutput, VerbosityLevel}; /// /// let mut output = UserOutput::new(VerbosityLevel::Normal); /// output.warn("Infrastructure may already be destroyed"); @@ -220,7 +220,7 @@ impl UserOutput { /// # Examples /// /// ```rust - /// use torrust_tracker_deployer_lib::shared::user_output::{UserOutput, VerbosityLevel}; + /// use torrust_tracker_deployer_lib::presentation::user_output::{UserOutput, VerbosityLevel}; /// /// let mut output = UserOutput::new(VerbosityLevel::Quiet); /// output.error("Failed to destroy environment"); @@ -238,7 +238,7 @@ impl UserOutput { /// # Examples /// /// ```rust - /// use torrust_tracker_deployer_lib::shared::user_output::{UserOutput, VerbosityLevel}; + /// use torrust_tracker_deployer_lib::presentation::user_output::{UserOutput, VerbosityLevel}; /// /// let mut output = UserOutput::new(VerbosityLevel::Normal); /// output.result("Deployment complete"); @@ -255,7 +255,7 @@ impl UserOutput { /// # Examples /// /// ```rust - /// use torrust_tracker_deployer_lib::shared::user_output::{UserOutput, VerbosityLevel}; + /// use torrust_tracker_deployer_lib::presentation::user_output::{UserOutput, VerbosityLevel}; /// /// let mut output = UserOutput::new(VerbosityLevel::Normal); /// output.data(r#"{"status": "destroyed", "environment": "test"}"#); diff --git a/src/shared/mod.rs b/src/shared/mod.rs index dc8bb52f..03c61696 100644 --- a/src/shared/mod.rs +++ b/src/shared/mod.rs @@ -9,7 +9,6 @@ pub mod command; pub mod error; pub mod port_checker; pub mod port_usage_checker; -pub mod user_output; pub mod username; // Re-export commonly used types for convenience @@ -18,5 +17,4 @@ pub use command::{CommandError, CommandExecutor, CommandResult}; pub use error::{ErrorKind, Traceable}; pub use port_checker::{PortChecker, PortCheckerError}; pub use port_usage_checker::{PortUsageChecker, PortUsageError}; -pub use user_output::{UserOutput, VerbosityLevel}; pub use username::{Username, UsernameError}; From ace705fad66f96c4ad423e7d1f0ce4226005eac4 Mon Sep 17 00:00:00 2001 From: Jose Celano Date: Thu, 23 Oct 2025 15:54:25 +0100 Subject: [PATCH 11/14] feat: [#23] implement structured error handling for destroy command - Replace Box with specific DestroyCommandError enum using thiserror - Add comprehensive error variants for different failure scenarios - Implement tiered help system with brief tips + detailed .help() method - Include actionable troubleshooting guidance following error handling conventions - Preserve error chains with #[source] attributes for full traceability - Provide clear, user-friendly error messages with specific instructions Features: - InvalidEnvironmentName: Validates environment name format with examples - DestroyOperationFailed: Handles destroy command execution failures - Environment context: Includes environment name and data directory paths - Detailed help: Platform-specific commands and recovery procedures Error handling follows docs/contributing/error-handling.md conventions: - Clarity: Unambiguous error messages with specific context - Traceability: Full error chains preserved for debugging - Actionability: Clear instructions for resolution --- src/app.rs | 205 ++++++++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 194 insertions(+), 11 deletions(-) diff --git a/src/app.rs b/src/app.rs index e044a9f4..1db46f65 100644 --- a/src/app.rs +++ b/src/app.rs @@ -5,6 +5,7 @@ use clap::{Parser, Subcommand}; use std::path::PathBuf; +use thiserror::Error; use tracing::info; use torrust_tracker_deployer_lib::logging::{LogFormat, LogOutput, LoggingBuilder}; @@ -116,6 +117,8 @@ pub fn run() { Some(Commands::Destroy { environment }) => { if let Err(e) = handle_destroy_command(&environment) { eprintln!("Error: {e}"); + eprintln!("\nFor detailed troubleshooting, run with --help or see:"); + eprintln!("{}", e.help()); std::process::exit(1); } } @@ -151,6 +154,181 @@ fn display_help_info() { println!("💡 To see available commands, run: torrust-tracker-deployer --help"); } +/// Errors that can occur during the destroy command execution +#[derive(Debug, Error)] +#[allow(dead_code)] // Some variants may be used in future implementations +pub enum DestroyCommandError { + /// Environment name validation failed + /// + /// The provided environment name doesn't meet the validation requirements. + /// Use `.help()` for detailed troubleshooting steps. + #[error("Invalid environment name '{name}': {source} +Tip: Environment names must be 1-63 characters, start with letter/digit, contain only letters/digits/hyphens")] + InvalidEnvironmentName { + name: String, + #[source] + source: torrust_tracker_deployer_lib::domain::environment::name::EnvironmentNameError, + }, + + /// Environment not found or inaccessible + /// + /// The environment couldn't be loaded from persistent storage. + /// Use `.help()` for detailed troubleshooting steps. + #[error( + "Environment '{name}' not found or inaccessible from data directory '{data_dir}' +Tip: Check if environment exists: ls -la {data_dir}/" + )] + EnvironmentNotAccessible { name: String, data_dir: String }, + + /// Destroy operation failed + /// + /// The destruction process encountered an error during execution. + /// Use `.help()` for detailed troubleshooting steps. + #[error( + "Failed to destroy environment '{name}': {source} +Tip: Check logs and try running with --log-output file-and-stderr for more details" + )] + DestroyOperationFailed { + name: String, + #[source] + source: torrust_tracker_deployer_lib::application::command_handlers::destroy::DestroyCommandHandlerError, + }, + + /// Repository operation failed + /// + /// Failed to create or access the environment repository. + /// Use `.help()` for detailed troubleshooting steps. + #[error( + "Failed to access environment repository at '{data_dir}': {reason} +Tip: Check directory permissions and disk space" + )] + RepositoryAccessFailed { data_dir: String, reason: String }, +} + +impl DestroyCommandError { + /// Get detailed troubleshooting guidance for this error + /// + /// This method provides comprehensive troubleshooting steps that can be + /// displayed to users when they need more help resolving the error. + /// + /// # Example + /// + /// ```rust + /// if let Err(e) = handle_destroy_command("test-env") { + /// eprintln!("Error: {e}"); + /// eprintln!("\nTroubleshooting:\n{}", e.help()); + /// } + /// ``` + pub fn help(&self) -> &'static str { + match self { + Self::InvalidEnvironmentName { .. } => { + "Invalid Environment Name - Detailed Troubleshooting: + +1. Check environment name format: + - Length: Must be 1-63 characters + - Start: Must begin with letter (a-z, A-Z) or digit (0-9) + - Characters: Only letters, digits, and hyphens allowed + - End: Must not end with a hyphen + +2. Common valid examples: + - 'production' + - 'test-env' + - 'e2e-provision' + - 'dev123' + +3. Common invalid examples: + - 'test_env' (underscores not allowed) + - '-test' (starts with hyphen) + - 'test-' (ends with hyphen) + - '' (empty string) + +For more information, see the environment naming conventions in the documentation." + } + + Self::EnvironmentNotAccessible { .. } => { + "Environment Not Accessible - Detailed Troubleshooting: + +1. Check if environment exists: + - List environments: ls -la data/ + - Look for directory with your environment name + +2. Verify file permissions: + - Check directory permissions: ls -ld data/ + - Ensure read/write access: chmod 755 data/ + +3. Check if environment was provisioned: + - Look for environment.json file: ls -la data/{env_name}/ + - Verify it's a valid deployment environment + +4. Common causes: + - Environment was never created (run provision first) + - Wrong data directory path + - Permission issues + - Corrupted environment state + +If the environment should exist, check the logs for more details." + } + + Self::DestroyOperationFailed { .. } => { + "Destroy Operation Failed - Detailed Troubleshooting: + +1. Check system resources: + - Ensure sufficient disk space + - Check network connectivity + - Verify system permissions + +2. Review the operation logs: + - Run with verbose logging: --log-output file-and-stderr + - Check log files in data/logs/ + - Look for specific error details + +3. Check infrastructure state: + - Verify LXD/OpenTofu are accessible + - Check if VMs/containers are running + - Ensure cleanup tools are available + +4. Manual intervention may be needed: + - Some resources might need manual cleanup + - Check provider-specific tools (lxc list, tofu state list) + - Remove stale infrastructure manually if needed + +5. Recovery options: + - Retry the destroy operation + - Force cleanup with provider tools + - Contact administrator if permissions are needed + +For persistent issues, check the infrastructure documentation." + } + + Self::RepositoryAccessFailed { .. } => { + "Repository Access Failed - Detailed Troubleshooting: + +1. Check directory permissions: + - Verify data directory exists and is accessible + - Ensure write permissions: chmod 755 data/ + - Check parent directory permissions + +2. Verify disk space: + - Check available space: df -h . + - Ensure sufficient space for operations + - Clean up if disk is full + +3. Check file system issues: + - Test file creation: touch data/test.tmp && rm data/test.tmp + - Look for file system errors in system logs + - Check if directory is on a read-only mount + +4. Common solutions: + - Create data directory: mkdir -p data + - Fix permissions: sudo chown -R $USER:$USER data/ + - Move to directory with sufficient space + +If the problem persists, check system logs and contact administrator." + } + } + } +} + /// Handle the destroy command /// /// This function orchestrates the environment destruction workflow by: @@ -174,7 +352,8 @@ fn display_help_info() { /// /// This function will return an error if the environment name is invalid, /// the environment cannot be loaded, or the destruction process fails. -fn handle_destroy_command(environment_name: &str) -> Result<(), Box> { +#[allow(clippy::result_large_err)] // Error contains detailed context for user guidance +fn handle_destroy_command(environment_name: &str) -> Result<(), DestroyCommandError> { use std::time::Duration; use torrust_tracker_deployer_lib::application::command_handlers::DestroyCommandHandler; use torrust_tracker_deployer_lib::domain::environment::name::EnvironmentName; @@ -188,11 +367,13 @@ fn handle_destroy_command(environment_name: &str) -> Result<(), Box Result<(), Box Date: Fri, 24 Oct 2025 16:11:08 +0100 Subject: [PATCH 12/14] refactor: [#23] implement DDD presentation layer with unified CLI and error handling MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Separate CLI parsing from business logic in presentation/cli/ - Create modular command execution system in presentation/commands/ - Implement unified CommandError with tiered help system - Add LoggingConfig abstraction decoupled from CLI parsing - Reduce app.rs to thin bootstrap (452 → 38 lines) - Create centralized help system in src/help.rs - Follow module organization guidelines with proper separation of concerns This completes the DDD architectural refactoring for clean separation between presentation, application, and domain layers. --- src/app.rs | 537 ++------------------------- src/help.rs | 130 +++++++ src/lib.rs | 2 + src/logging.rs | 117 +++++- src/presentation/cli/args.rs | 97 +++++ src/presentation/cli/commands.rs | 51 +++ src/presentation/cli/mod.rs | 166 +++++++++ src/presentation/commands/destroy.rs | 284 ++++++++++++++ src/presentation/commands/mod.rs | 115 ++++++ src/presentation/errors.rs | 84 +++++ src/presentation/mod.rs | 27 ++ 11 files changed, 1091 insertions(+), 519 deletions(-) create mode 100644 src/help.rs create mode 100644 src/presentation/cli/args.rs create mode 100644 src/presentation/cli/commands.rs create mode 100644 src/presentation/cli/mod.rs create mode 100644 src/presentation/commands/destroy.rs create mode 100644 src/presentation/commands/mod.rs create mode 100644 src/presentation/errors.rs diff --git a/src/app.rs b/src/app.rs index 1db46f65..5edffee1 100644 --- a/src/app.rs +++ b/src/app.rs @@ -1,82 +1,34 @@ -//! Main application module for the Torrust Tracker Deployer CLI +//! Main Application Bootstrap //! -//! This module contains the CLI structure and main application logic. -//! It initializes logging and handles the application lifecycle. +//! This module provides a thin bootstrap layer for the Torrust Tracker Deployer CLI. +//! It handles application initialization, logging setup, and command dispatch while +//! delegating all CLI parsing and business logic to the presentation layer. +//! +//! ## Responsibilities +//! +//! - **Application Lifecycle**: Initialize and shutdown the application +//! - **Logging Setup**: Configure logging based on CLI arguments +//! - **Command Dispatch**: Route commands to the presentation layer for execution +//! - **Exit Handling**: Manage application exit codes and cleanup +//! +//! ## Design Principles +//! +//! - **Thin Layer**: Minimal logic, maximum delegation to appropriate layers +//! - **Single Responsibility**: Focus only on application bootstrap concerns +//! - **Clean Separation**: No CLI parsing or business logic in this module -use clap::{Parser, Subcommand}; -use std::path::PathBuf; -use thiserror::Error; +use clap::Parser; use tracing::info; -use torrust_tracker_deployer_lib::logging::{LogFormat, LogOutput, LoggingBuilder}; - -/// Command-line interface for Torrust Tracker Deployer -#[derive(Parser, Debug)] -#[command(name = "torrust-tracker-deployer")] -#[command(about = "Automated deployment infrastructure for Torrust Tracker")] -#[command(version)] -#[allow(clippy::struct_field_names)] // CLI arguments intentionally share 'log_' prefix for clarity -pub struct Cli { - /// Format for file logging (default: compact, without ANSI codes) - /// - /// - pretty: Pretty-printed output for development (no ANSI in files) - /// - json: JSON output for production environments (no ANSI) - /// - compact: Compact output for minimal verbosity (no ANSI in files) - /// - /// Note: ANSI color codes are automatically disabled for file output - /// to ensure logs are easily parsed with standard text tools (grep, awk, sed). - #[arg(long, value_enum, default_value = "compact", global = true)] - pub log_file_format: LogFormat, - - /// Format for stderr logging (default: pretty, with ANSI codes) - /// - /// - pretty: Pretty-printed output with colors for development - /// - json: JSON output for machine processing - /// - compact: Compact output with colors for minimal verbosity - /// - /// Note: ANSI color codes are automatically enabled for stderr output - /// to provide colored terminal output for better readability. - #[arg(long, value_enum, default_value = "pretty", global = true)] - pub log_stderr_format: LogFormat, - - /// Log output mode (default: file-only for production) - /// - /// - file-only: Write logs to file only (production mode) - /// - file-and-stderr: Write logs to both file and stderr (development/testing mode) - #[arg(long, value_enum, default_value = "file-only", global = true)] - pub log_output: LogOutput, - - /// Log directory (default: ./data/logs) - /// - /// Directory where log files will be written. The log file will be - /// named 'log.txt' inside this directory. Parent directories will be - /// created automatically if they don't exist. - /// - /// Note: If the directory cannot be created due to filesystem permissions, - /// the application will exit with an error. Logging is critical for - /// observability and the application cannot function without it. - #[arg(long, default_value = "./data/logs", global = true)] - pub log_dir: PathBuf, - - /// Subcommand to execute - #[command(subcommand)] - pub command: Option, -} - -/// Available CLI commands -#[derive(Subcommand, Debug)] -pub enum Commands { - /// Destroy an existing deployment environment - Destroy { - /// Name of the environment to destroy - environment: String, - }, -} +use torrust_tracker_deployer_lib::{help, logging, presentation}; /// Main application entry point /// -/// This function initializes logging, displays information to the user, -/// and executes the requested command if provided. +/// This function serves as the application bootstrap, handling: +/// 1. CLI argument parsing (delegated to presentation layer) +/// 2. Logging initialization using `LoggingConfig` +/// 3. Command execution (delegated to presentation layer) +/// 4. Error handling and exit code management /// /// # Panics /// @@ -86,452 +38,33 @@ pub enum Commands { /// /// Both panics are intentional as logging is critical for observability. pub fn run() { - let cli = Cli::parse(); + let cli = presentation::Cli::parse(); - // Clone values for logging before moving them - let log_file_format = cli.log_file_format.clone(); - let log_stderr_format = cli.log_stderr_format.clone(); - let log_output = cli.log_output; - let log_dir = cli.log_dir.clone(); + let logging_config = cli.global.logging_config(); - // Initialize logging FIRST before any other logic - LoggingBuilder::new(&cli.log_dir) - .with_file_format(cli.log_file_format) - .with_stderr_format(cli.log_stderr_format) - .with_output(cli.log_output) - .init(); + logging::init_subscriber(logging_config); - // Log startup event with configuration details info!( app = "torrust-tracker-deployer", version = env!("CARGO_PKG_VERSION"), - log_dir = %log_dir.display(), - log_file_format = ?log_file_format, - log_stderr_format = ?log_stderr_format, - log_output = ?log_output, + log_dir = %cli.global.log_dir.display(), + log_file_format = ?cli.global.log_file_format, + log_stderr_format = ?cli.global.log_stderr_format, + log_output = ?cli.global.log_output, "Application started" ); - // Execute command if provided, otherwise display help match cli.command { - Some(Commands::Destroy { environment }) => { - if let Err(e) = handle_destroy_command(&environment) { - eprintln!("Error: {e}"); - eprintln!("\nFor detailed troubleshooting, run with --help or see:"); - eprintln!("{}", e.help()); + Some(command) => { + if let Err(e) = presentation::execute(command) { + presentation::handle_error(&e); std::process::exit(1); } } None => { - display_help_info(); + help::display_getting_started(); } } info!("Application finished"); } - -/// Display helpful information to the user when no command is provided -fn display_help_info() { - println!("🏗️ Torrust Tracker Deployer"); - println!("========================="); - println!(); - println!("This repository provides automated deployment infrastructure for Torrust tracker projects."); - println!("The infrastructure includes VM provisioning with OpenTofu and configuration"); - println!("management with Ansible playbooks."); - println!(); - println!("📋 Getting Started:"); - println!(" Please follow the instructions in the README.md file to:"); - println!(" 1. Set up the required dependencies (OpenTofu, Ansible, LXD)"); - println!(" 2. Provision the deployment infrastructure"); - println!(" 3. Deploy and configure the services"); - println!(); - println!("🧪 Running E2E Tests:"); - println!(" Use the e2e tests binaries to run end-to-end tests:"); - println!(" cargo e2e-provision && cargo e2e-config"); - println!(); - println!("📖 For detailed instructions, see: README.md"); - println!(); - println!("💡 To see available commands, run: torrust-tracker-deployer --help"); -} - -/// Errors that can occur during the destroy command execution -#[derive(Debug, Error)] -#[allow(dead_code)] // Some variants may be used in future implementations -pub enum DestroyCommandError { - /// Environment name validation failed - /// - /// The provided environment name doesn't meet the validation requirements. - /// Use `.help()` for detailed troubleshooting steps. - #[error("Invalid environment name '{name}': {source} -Tip: Environment names must be 1-63 characters, start with letter/digit, contain only letters/digits/hyphens")] - InvalidEnvironmentName { - name: String, - #[source] - source: torrust_tracker_deployer_lib::domain::environment::name::EnvironmentNameError, - }, - - /// Environment not found or inaccessible - /// - /// The environment couldn't be loaded from persistent storage. - /// Use `.help()` for detailed troubleshooting steps. - #[error( - "Environment '{name}' not found or inaccessible from data directory '{data_dir}' -Tip: Check if environment exists: ls -la {data_dir}/" - )] - EnvironmentNotAccessible { name: String, data_dir: String }, - - /// Destroy operation failed - /// - /// The destruction process encountered an error during execution. - /// Use `.help()` for detailed troubleshooting steps. - #[error( - "Failed to destroy environment '{name}': {source} -Tip: Check logs and try running with --log-output file-and-stderr for more details" - )] - DestroyOperationFailed { - name: String, - #[source] - source: torrust_tracker_deployer_lib::application::command_handlers::destroy::DestroyCommandHandlerError, - }, - - /// Repository operation failed - /// - /// Failed to create or access the environment repository. - /// Use `.help()` for detailed troubleshooting steps. - #[error( - "Failed to access environment repository at '{data_dir}': {reason} -Tip: Check directory permissions and disk space" - )] - RepositoryAccessFailed { data_dir: String, reason: String }, -} - -impl DestroyCommandError { - /// Get detailed troubleshooting guidance for this error - /// - /// This method provides comprehensive troubleshooting steps that can be - /// displayed to users when they need more help resolving the error. - /// - /// # Example - /// - /// ```rust - /// if let Err(e) = handle_destroy_command("test-env") { - /// eprintln!("Error: {e}"); - /// eprintln!("\nTroubleshooting:\n{}", e.help()); - /// } - /// ``` - pub fn help(&self) -> &'static str { - match self { - Self::InvalidEnvironmentName { .. } => { - "Invalid Environment Name - Detailed Troubleshooting: - -1. Check environment name format: - - Length: Must be 1-63 characters - - Start: Must begin with letter (a-z, A-Z) or digit (0-9) - - Characters: Only letters, digits, and hyphens allowed - - End: Must not end with a hyphen - -2. Common valid examples: - - 'production' - - 'test-env' - - 'e2e-provision' - - 'dev123' - -3. Common invalid examples: - - 'test_env' (underscores not allowed) - - '-test' (starts with hyphen) - - 'test-' (ends with hyphen) - - '' (empty string) - -For more information, see the environment naming conventions in the documentation." - } - - Self::EnvironmentNotAccessible { .. } => { - "Environment Not Accessible - Detailed Troubleshooting: - -1. Check if environment exists: - - List environments: ls -la data/ - - Look for directory with your environment name - -2. Verify file permissions: - - Check directory permissions: ls -ld data/ - - Ensure read/write access: chmod 755 data/ - -3. Check if environment was provisioned: - - Look for environment.json file: ls -la data/{env_name}/ - - Verify it's a valid deployment environment - -4. Common causes: - - Environment was never created (run provision first) - - Wrong data directory path - - Permission issues - - Corrupted environment state - -If the environment should exist, check the logs for more details." - } - - Self::DestroyOperationFailed { .. } => { - "Destroy Operation Failed - Detailed Troubleshooting: - -1. Check system resources: - - Ensure sufficient disk space - - Check network connectivity - - Verify system permissions - -2. Review the operation logs: - - Run with verbose logging: --log-output file-and-stderr - - Check log files in data/logs/ - - Look for specific error details - -3. Check infrastructure state: - - Verify LXD/OpenTofu are accessible - - Check if VMs/containers are running - - Ensure cleanup tools are available - -4. Manual intervention may be needed: - - Some resources might need manual cleanup - - Check provider-specific tools (lxc list, tofu state list) - - Remove stale infrastructure manually if needed - -5. Recovery options: - - Retry the destroy operation - - Force cleanup with provider tools - - Contact administrator if permissions are needed - -For persistent issues, check the infrastructure documentation." - } - - Self::RepositoryAccessFailed { .. } => { - "Repository Access Failed - Detailed Troubleshooting: - -1. Check directory permissions: - - Verify data directory exists and is accessible - - Ensure write permissions: chmod 755 data/ - - Check parent directory permissions - -2. Verify disk space: - - Check available space: df -h . - - Ensure sufficient space for operations - - Clean up if disk is full - -3. Check file system issues: - - Test file creation: touch data/test.tmp && rm data/test.tmp - - Look for file system errors in system logs - - Check if directory is on a read-only mount - -4. Common solutions: - - Create data directory: mkdir -p data - - Fix permissions: sudo chown -R $USER:$USER data/ - - Move to directory with sufficient space - -If the problem persists, check system logs and contact administrator." - } - } - } -} - -/// Handle the destroy command -/// -/// This function orchestrates the environment destruction workflow by: -/// 1. Validating the environment name -/// 2. Loading the environment from persistent storage -/// 3. Executing the destroy command handler -/// 4. Providing user-friendly progress updates -/// -/// # Arguments -/// -/// * `environment_name` - The name of the environment to destroy -/// -/// # Returns -/// -/// Returns `Ok(())` on success, or an error if: -/// - Environment name is invalid -/// - Environment cannot be loaded -/// - Destruction fails -/// -/// # Errors -/// -/// This function will return an error if the environment name is invalid, -/// the environment cannot be loaded, or the destruction process fails. -#[allow(clippy::result_large_err)] // Error contains detailed context for user guidance -fn handle_destroy_command(environment_name: &str) -> Result<(), DestroyCommandError> { - use std::time::Duration; - use torrust_tracker_deployer_lib::application::command_handlers::DestroyCommandHandler; - use torrust_tracker_deployer_lib::domain::environment::name::EnvironmentName; - use torrust_tracker_deployer_lib::infrastructure::persistence::repository_factory::RepositoryFactory; - use torrust_tracker_deployer_lib::presentation::user_output::{UserOutput, VerbosityLevel}; - - // Create user output with default stdout/stderr channels - let mut output = UserOutput::new(VerbosityLevel::Normal); - - // Display initial progress (to stderr) - output.progress(&format!("Destroying environment '{environment_name}'...")); - - // Validate environment name - let env_name = EnvironmentName::new(environment_name.to_string()).map_err(|source| { - let error = DestroyCommandError::InvalidEnvironmentName { - name: environment_name.to_string(), - source, - }; - output.error(&error.to_string()); - error - })?; - - // Create repository for loading environment state - let repository_factory = RepositoryFactory::new(Duration::from_secs(30)); - let repository = repository_factory.create(std::path::PathBuf::from("data")); - - // Create and execute destroy command handler - output.progress("Tearing down infrastructure..."); - - let command_handler = DestroyCommandHandler::new(repository); - - // Execute destroy - the handler will load the environment and handle all states internally - let _destroyed_env = command_handler.execute(&env_name).map_err(|source| { - let error = DestroyCommandError::DestroyOperationFailed { - name: environment_name.to_string(), - source, - }; - output.error(&error.to_string()); - error - })?; - - output.progress("Cleaning up resources..."); - output.success(&format!( - "Environment '{environment_name}' destroyed successfully" - )); - - Ok(()) -} - -#[cfg(test)] -mod tests { - use super::*; - use clap::Parser; - - #[test] - fn it_should_parse_destroy_subcommand() { - let args = vec!["torrust-tracker-deployer", "destroy", "test-env"]; - let cli = Cli::try_parse_from(args).unwrap(); - - assert!(cli.command.is_some()); - match cli.command.unwrap() { - Commands::Destroy { environment } => { - assert_eq!(environment, "test-env"); - } - } - } - - #[test] - fn it_should_parse_destroy_with_different_environment_names() { - let test_cases = vec!["e2e-provision", "production", "test-123", "dev-environment"]; - - for env_name in test_cases { - let args = vec!["torrust-tracker-deployer", "destroy", env_name]; - let cli = Cli::try_parse_from(args).unwrap(); - - match cli.command.unwrap() { - Commands::Destroy { environment } => { - assert_eq!(environment, env_name); - } - } - } - } - - #[test] - fn it_should_require_environment_parameter() { - let args = vec!["torrust-tracker-deployer", "destroy"]; - let result = Cli::try_parse_from(args); - - assert!(result.is_err()); - let error = result.unwrap_err(); - let error_message = error.to_string(); - assert!( - error_message.contains("required") || error_message.contains("argument"), - "Error message should indicate missing required argument: {error_message}" - ); - } - - #[test] - fn it_should_parse_global_log_options_with_destroy_command() { - let args = vec![ - "torrust-tracker-deployer", - "--log-file-format", - "json", - "--log-stderr-format", - "compact", - "--log-output", - "file-and-stderr", - "--log-dir", - "/tmp/logs", - "destroy", - "test-env", - ]; - let cli = Cli::try_parse_from(args).unwrap(); - - // Verify the destroy command was parsed correctly - match cli.command.unwrap() { - Commands::Destroy { environment } => { - assert_eq!(environment, "test-env"); - } - } - - // Log options are set but we don't compare them as they don't implement PartialEq - assert_eq!(cli.log_dir, PathBuf::from("/tmp/logs")); - } - - #[test] - fn it_should_use_default_log_dir_when_not_specified() { - let args = vec!["torrust-tracker-deployer", "destroy", "test-env"]; - let cli = Cli::try_parse_from(args).unwrap(); - - assert_eq!(cli.log_dir, PathBuf::from("./data/logs")); - } - - #[test] - fn it_should_handle_no_command() { - let args = vec!["torrust-tracker-deployer"]; - let cli = Cli::try_parse_from(args).unwrap(); - - assert!(cli.command.is_none()); - } - - #[test] - fn it_should_show_help_with_help_flag() { - let args = vec!["torrust-tracker-deployer", "--help"]; - let result = Cli::try_parse_from(args); - - // Help flag causes a "display help" error - assert!(result.is_err()); - let error = result.unwrap_err(); - assert_eq!(error.kind(), clap::error::ErrorKind::DisplayHelp); - } - - #[test] - fn it_should_show_version_with_version_flag() { - let args = vec!["torrust-tracker-deployer", "--version"]; - let result = Cli::try_parse_from(args); - - // Version flag causes a "display version" error - assert!(result.is_err()); - let error = result.unwrap_err(); - assert_eq!(error.kind(), clap::error::ErrorKind::DisplayVersion); - } - - #[test] - fn it_should_show_destroy_help() { - let args = vec!["torrust-tracker-deployer", "destroy", "--help"]; - let result = Cli::try_parse_from(args); - - // Help flag causes a "display help" error - assert!(result.is_err()); - let error = result.unwrap_err(); - assert_eq!(error.kind(), clap::error::ErrorKind::DisplayHelp); - - // Verify the help text mentions the environment parameter - let help_text = error.to_string(); - assert!( - help_text.contains("environment") || help_text.contains(""), - "Help text should mention environment parameter" - ); - } -} diff --git a/src/help.rs b/src/help.rs new file mode 100644 index 00000000..29627242 --- /dev/null +++ b/src/help.rs @@ -0,0 +1,130 @@ +//! Help and Usage Information Module +//! +//! This module provides help and usage information for the Torrust Tracker Deployer +//! application. It contains functions to display helpful information to users +//! when they need guidance on how to use the application. +//! +//! ## Design Principles +//! +//! - **Independent**: No dependencies on presentation layer or CLI structures +//! - **User-Focused**: Clear, actionable guidance for users +//! - **Comprehensive**: Covers getting started, examples, and next steps + +/// Display helpful information to the user when no command is provided +/// +/// This function shows getting started information, usage examples, and +/// helpful links when the user runs the application without any subcommands. +/// It provides a friendly introduction to the application and guides users +/// toward productive next steps. +/// +/// # Output +/// +/// Prints directly to stdout with formatted, user-friendly content including: +/// - Application overview and purpose +/// - Getting started instructions +/// - Testing guidance +/// - Documentation references +/// - Next steps for users +/// +/// # Example +/// +/// ```rust +/// use torrust_tracker_deployer_lib::help; +/// +/// // Display help when user runs app without arguments +/// help::display_getting_started(); +/// ``` +pub fn display_getting_started() { + println!("🏗️ Torrust Tracker Deployer"); + println!("========================="); + println!(); + println!("This repository provides automated deployment infrastructure for Torrust tracker projects."); + println!("The infrastructure includes VM provisioning with OpenTofu and configuration"); + println!("management with Ansible playbooks."); + println!(); + println!("📋 Getting Started:"); + println!(" Please follow the instructions in the README.md file to:"); + println!(" 1. Set up the required dependencies (OpenTofu, Ansible, LXD)"); + println!(" 2. Provision the deployment infrastructure"); + println!(" 3. Deploy and configure the services"); + println!(); + println!("🧪 Running E2E Tests:"); + println!(" Use the e2e tests binaries to run end-to-end tests:"); + println!(" cargo e2e-provision && cargo e2e-config"); + println!(); + println!("📖 For detailed instructions, see: README.md"); + println!(); + println!("💡 To see available commands, run: torrust-tracker-deployer --help"); +} + +/// Display troubleshooting information for common issues +/// +/// This function provides guidance for common problems users might encounter +/// when setting up or using the Torrust Tracker Deployer. +/// +/// # Output +/// +/// Prints troubleshooting guidance to stdout including: +/// - Common setup issues and solutions +/// - Dependency verification steps +/// - Configuration validation tips +/// - Where to get additional help +/// +/// # Example +/// +/// ```rust +/// use torrust_tracker_deployer_lib::help; +/// +/// // Display troubleshooting info when user encounters issues +/// help::display_troubleshooting(); +/// ``` +pub fn display_troubleshooting() { + println!("🔧 Troubleshooting Guide"); + println!("======================="); + println!(); + println!("Common issues and solutions:"); + println!(); + println!("1. Dependencies not found:"); + println!(" - Ensure OpenTofu is installed and in PATH"); + println!(" - Verify Ansible is installed and accessible"); + println!(" - Check that LXD is properly configured"); + println!(); + println!("2. Permission errors:"); + println!(" - Add your user to the lxd group: sudo usermod -aG lxd $USER"); + println!(" - Log out and log back in to apply group changes"); + println!(" - Verify permissions with: groups"); + println!(); + println!("3. Network connectivity issues:"); + println!(" - Check internet connectivity for image downloads"); + println!(" - Verify LXD daemon is running: lxd --version"); + println!(" - Test basic LXD functionality: lxc list"); + println!(); + println!("4. Configuration problems:"); + println!(" - Validate YAML/JSON syntax in configuration files"); + println!(" - Check that environment names follow naming conventions"); + println!(" - Ensure SSH keys are properly configured"); + println!(); + println!("📖 For more help:"); + println!(" - Check the docs/ directory for detailed guides"); + println!(" - Review the README.md for setup instructions"); + println!(" - Open an issue on GitHub for additional support"); +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn it_should_display_getting_started_without_panicking() { + // Test that the function runs without panicking + // We can't easily test stdout content in unit tests, + // but we can ensure the function doesn't crash + display_getting_started(); + } + + #[test] + fn it_should_display_troubleshooting_without_panicking() { + // Test that the function runs without panicking + display_troubleshooting(); + } +} diff --git a/src/lib.rs b/src/lib.rs index 6c2e136e..0b992b1b 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -22,6 +22,7 @@ //! - `adapters` - External tool adapters (thin CLI wrappers) //! - `config` - Configuration management for deployment environments //! - `container` - Service container for dependency injection +//! - `help` - Help and usage information display functions //! - `logging` - Logging configuration and utilities //! - `shared` - Shared modules used across different layers //! - `testing` - Testing utilities (unit, integration, and end-to-end) @@ -31,6 +32,7 @@ pub mod application; pub mod config; pub mod container; pub mod domain; +pub mod help; pub mod infrastructure; pub mod logging; pub mod presentation; diff --git a/src/logging.rs b/src/logging.rs index 97b6bd14..0970db06 100644 --- a/src/logging.rs +++ b/src/logging.rs @@ -79,6 +79,61 @@ pub enum LogFormat { Compact, } +// ============================================================================ +// LOGGING CONFIGURATION - Domain Type +// ============================================================================ + +/// Configuration for logging system initialization +/// +/// This struct represents the domain-specific logging configuration that is +/// independent of CLI parsing concerns. It can be constructed from CLI arguments +/// or other configuration sources without creating circular dependencies. +/// +/// # Design Principles +/// +/// - **Independence**: No dependency on presentation layer types +/// - **Reusability**: Can be constructed from various sources (CLI, config files, tests) +/// - **Clarity**: Clear field names and comprehensive documentation +#[derive(Debug, Clone)] +pub struct LoggingConfig { + /// Directory where log files will be written + pub log_dir: std::path::PathBuf, + + /// Format for file logging output + pub file_format: LogFormat, + + /// Format for stderr logging output + pub stderr_format: LogFormat, + + /// Output target (file-only vs file-and-stderr) + pub output: LogOutput, +} + +impl LoggingConfig { + /// Create a new logging configuration + /// + /// # Arguments + /// + /// * `log_dir` - Directory for log files + /// * `file_format` - Format for file output + /// * `stderr_format` - Format for stderr output + /// * `output` - Output target configuration + #[must_use] + pub fn new( + log_dir: std::path::PathBuf, + file_format: LogFormat, + stderr_format: LogFormat, + output: LogOutput, + ) -> Self { + Self { + log_dir, + file_format, + stderr_format, + output, + } + } +} + // ============================================================================ // BUILDER PATTERN - Core Implementation // ============================================================================ @@ -210,23 +265,28 @@ impl LoggingBuilder { /// /// Both panics are intentional as logging is critical for observability. pub fn init(self) { - init_subscriber( - &self.log_dir, + let config = LoggingConfig::new( + self.log_dir, + self.file_format, + self.stderr_format, self.output, - &self.file_format, - &self.stderr_format, ); + init_subscriber(config); } } // ============================================================================ -// INTERNAL INITIALIZATION - Single Source of Truth +// PUBLIC INITIALIZATION FUNCTIONS // ============================================================================ -/// Internal initialization function that handles all subscriber setup +/// Initialize logging with the provided configuration +/// +/// This function takes a `LoggingConfig` and sets up the global logging infrastructure. +/// After calling this, all logging macros (`tracing::info!`, etc.) will use +/// this configuration. /// /// This is the single source of truth for subscriber initialization. -/// All public init functions delegate to this to eliminate duplication. +/// All other init functions delegate to this to eliminate duplication. /// /// Automatically configures ANSI codes: /// - File output: ANSI codes disabled (clean text for parsing) @@ -237,20 +297,43 @@ impl LoggingBuilder { /// concrete type, and Rust's type system requires all match arms to return /// the same type. Type erasure with boxed layers would work but adds runtime /// overhead for a one-time initialization cost. +/// +/// # Arguments +/// +/// * `config` - The logging configuration containing all settings +/// +/// # Panics +/// +/// Panics if: +/// - Log directory cannot be created (filesystem permissions issue) +/// - Subscriber initialization fails (usually means it was already initialized) +/// +/// Both panics are intentional as logging is critical for observability. +/// +/// # Example +/// +/// ```rust,no_run +/// use std::path::PathBuf; +/// use torrust_tracker_deployer_lib::logging::{LogFormat, LogOutput, LoggingConfig, init_subscriber}; +/// +/// let config = LoggingConfig::new( +/// PathBuf::from("./data/logs"), +/// LogFormat::Compact, +/// LogFormat::Pretty, +/// LogOutput::FileAndStderr, +/// ); +/// +/// init_subscriber(config); +/// ``` #[allow(clippy::too_many_lines)] -fn init_subscriber( - log_dir: &Path, - output: LogOutput, - file_format: &LogFormat, - stderr_format: &LogFormat, -) { - let file_appender = create_log_file_appender(log_dir); +pub fn init_subscriber(config: LoggingConfig) { + let file_appender = create_log_file_appender(&config.log_dir); let env_filter = create_env_filter(); - match output { + match config.output { LogOutput::FileOnly => { // File-only mode: single layer with ANSI disabled - match file_format { + match config.file_format { LogFormat::Pretty => { tracing_subscriber::registry() .with( @@ -288,7 +371,7 @@ fn init_subscriber( } LogOutput::FileAndStderr => { // Dual output mode: file layer (no ANSI) + stderr layer (with ANSI) - match (file_format, stderr_format) { + match (config.file_format, config.stderr_format) { // Pretty file format combinations (LogFormat::Pretty, LogFormat::Pretty) => { tracing_subscriber::registry() diff --git a/src/presentation/cli/args.rs b/src/presentation/cli/args.rs new file mode 100644 index 00000000..c7043e52 --- /dev/null +++ b/src/presentation/cli/args.rs @@ -0,0 +1,97 @@ +//! CLI Argument Definitions +//! +//! This module contains the global CLI arguments that are shared across all commands, +//! primarily logging configuration options. These arguments follow clap conventions +//! and provide comprehensive documentation for users. + +use std::path::PathBuf; + +use crate::logging::{LogFormat, LogOutput, LoggingConfig}; + +/// Global CLI arguments for logging configuration +/// +/// These arguments are available for all commands and control how logging +/// is handled throughout the application. They provide fine-grained control +/// over log output, formatting, and destinations. +#[derive(clap::Args, Debug)] +pub struct GlobalArgs { + /// Format for file logging (default: compact, without ANSI codes) + /// + /// - pretty: Pretty-printed output for development (no ANSI in files) + /// - json: JSON output for production environments (no ANSI) + /// - compact: Compact output for minimal verbosity (no ANSI in files) + /// + /// Note: ANSI color codes are automatically disabled for file output + /// to ensure logs are easily parsed with standard text tools (grep, awk, sed). + #[arg(long, value_enum, default_value = "compact", global = true)] + pub log_file_format: LogFormat, + + /// Format for stderr logging (default: pretty, with ANSI codes) + /// + /// - pretty: Pretty-printed output with colors for development + /// - json: JSON output for machine processing + /// - compact: Compact output with colors for minimal verbosity + /// + /// Note: ANSI color codes are automatically enabled for stderr output + /// to provide colored terminal output for better readability. + #[arg(long, value_enum, default_value = "pretty", global = true)] + pub log_stderr_format: LogFormat, + + /// Log output mode (default: file-only for production) + /// + /// - file-only: Write logs to file only (production mode) + /// - file-and-stderr: Write logs to both file and stderr (development/testing mode) + #[arg(long, value_enum, default_value = "file-only", global = true)] + pub log_output: LogOutput, + + /// Log directory (default: ./data/logs) + /// + /// Directory where log files will be written. The log file will be + /// named 'log.txt' inside this directory. Parent directories will be + /// created automatically if they don't exist. + /// + /// Note: If the directory cannot be created due to filesystem permissions, + /// the application will exit with an error. Logging is critical for + /// observability and the application cannot function without it. + #[arg(long, default_value = "./data/logs", global = true)] + pub log_dir: PathBuf, +} + +impl GlobalArgs { + /// Create a logging configuration from these global arguments + /// + /// This method extracts the logging-specific configuration from CLI arguments + /// and creates a domain-appropriate `LoggingConfig` struct. This encapsulates + /// the conversion logic and avoids spreading logging configuration details + /// throughout the application bootstrap code. + /// + /// # Returns + /// + /// A `LoggingConfig` that can be used to initialize the logging system + /// + /// # Example + /// + /// ```rust + /// # use torrust_tracker_deployer_lib::presentation::cli::args::GlobalArgs; + /// # use torrust_tracker_deployer_lib::logging::{LogFormat, LogOutput, LoggingConfig}; + /// # use std::path::PathBuf; + /// // Create args with log configuration + /// let args = GlobalArgs { + /// log_file_format: LogFormat::Compact, + /// log_stderr_format: LogFormat::Pretty, + /// log_output: LogOutput::FileAndStderr, + /// log_dir: PathBuf::from("/tmp/logs"), + /// }; + /// let config = args.logging_config(); + /// // config will have specified log formats and directory + /// ``` + #[must_use] + pub fn logging_config(&self) -> LoggingConfig { + LoggingConfig::new( + self.log_dir.clone(), + self.log_file_format.clone(), + self.log_stderr_format.clone(), + self.log_output, + ) + } +} diff --git a/src/presentation/cli/commands.rs b/src/presentation/cli/commands.rs new file mode 100644 index 00000000..8a6eb5dc --- /dev/null +++ b/src/presentation/cli/commands.rs @@ -0,0 +1,51 @@ +//! CLI Command Definitions +//! +//! This module defines the command-line interface structure and available commands +//! for the Torrust Tracker Deployer CLI application. + +use clap::Subcommand; + +/// Available CLI commands +/// +/// This enum defines all the subcommands available in the CLI application. +/// Each variant represents a specific operation that can be performed. +#[derive(Subcommand, Debug)] +pub enum Commands { + /// Destroy an existing deployment environment + /// + /// This command will tear down all infrastructure associated with the + /// specified environment, including virtual machines, networks, and + /// persistent data. This operation is irreversible. + Destroy { + /// Name of the environment to destroy + /// + /// The environment name must be a valid identifier that was previously + /// created through the provision command. Use 'list' command to see + /// available environments. + environment: String, + }, + // Future commands will be added here: + // + // /// Provision a new deployment environment + // Provision { + // /// Name of the environment to create + // environment: String, + // /// Infrastructure provider to use (lxd, multipass, etc.) + // #[arg(long, default_value = "lxd")] + // provider: String, + // }, + // + // /// Configure an existing deployment environment + // Configure { + // /// Name of the environment to configure + // environment: String, + // }, + // + // /// Create a new release of the deployed application + // Release { + // /// Name of the environment for the release + // environment: String, + // /// Version tag for the release + // version: String, + // }, +} diff --git a/src/presentation/cli/mod.rs b/src/presentation/cli/mod.rs new file mode 100644 index 00000000..2e634f07 --- /dev/null +++ b/src/presentation/cli/mod.rs @@ -0,0 +1,166 @@ +//! CLI Module +//! +//! This module provides the command-line interface structure and functionality +//! for the Torrust Tracker Deployer application. It handles CLI argument parsing +//! and provides the CLI data structures. + +use clap::Parser; + +// Re-export submodules for convenient access +pub mod args; +pub mod commands; + +pub use args::GlobalArgs; +pub use commands::Commands; + +/// Command-line interface for Torrust Tracker Deployer +/// +/// This struct defines the top-level CLI structure including global arguments +/// and available subcommands. It uses clap for argument parsing and provides +/// comprehensive help documentation. +#[derive(Parser, Debug)] +#[command(name = "torrust-tracker-deployer")] +#[command(about = "Automated deployment infrastructure for Torrust Tracker")] +#[command(version)] +#[allow(clippy::struct_field_names)] // CLI arguments intentionally share 'log_' prefix for clarity +pub struct Cli { + /// Global arguments (logging configuration) + #[command(flatten)] + pub global: GlobalArgs, + + /// Subcommand to execute + #[command(subcommand)] + pub command: Option, +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn it_should_parse_destroy_subcommand() { + let args = vec!["torrust-tracker-deployer", "destroy", "test-env"]; + let cli = Cli::try_parse_from(args).unwrap(); + + assert!(cli.command.is_some()); + match cli.command.unwrap() { + Commands::Destroy { environment } => { + assert_eq!(environment, "test-env"); + } + } + } + + #[test] + fn it_should_parse_destroy_with_different_environment_names() { + let test_cases = vec!["e2e-provision", "production", "test-123", "dev-environment"]; + + for env_name in test_cases { + let args = vec!["torrust-tracker-deployer", "destroy", env_name]; + let cli = Cli::try_parse_from(args).unwrap(); + + match cli.command.unwrap() { + Commands::Destroy { environment } => { + assert_eq!(environment, env_name); + } + } + } + } + + #[test] + fn it_should_require_environment_parameter() { + let args = vec!["torrust-tracker-deployer", "destroy"]; + let result = Cli::try_parse_from(args); + + assert!(result.is_err()); + let error = result.unwrap_err(); + let error_message = error.to_string(); + assert!( + error_message.contains("required") || error_message.contains("argument"), + "Error message should indicate missing required argument: {error_message}" + ); + } + + #[test] + fn it_should_parse_global_log_options_with_destroy_command() { + let args = vec![ + "torrust-tracker-deployer", + "--log-file-format", + "json", + "--log-stderr-format", + "compact", + "--log-output", + "file-and-stderr", + "--log-dir", + "/tmp/logs", + "destroy", + "test-env", + ]; + let cli = Cli::try_parse_from(args).unwrap(); + + // Verify the destroy command was parsed correctly + match cli.command.unwrap() { + Commands::Destroy { environment } => { + assert_eq!(environment, "test-env"); + } + } + + // Log options are set but we don't compare them as they don't implement PartialEq + assert_eq!(cli.global.log_dir, std::path::PathBuf::from("/tmp/logs")); + } + + #[test] + fn it_should_use_default_log_dir_when_not_specified() { + let args = vec!["torrust-tracker-deployer", "destroy", "test-env"]; + let cli = Cli::try_parse_from(args).unwrap(); + + assert_eq!(cli.global.log_dir, std::path::PathBuf::from("./data/logs")); + } + + #[test] + fn it_should_handle_no_command() { + let args = vec!["torrust-tracker-deployer"]; + let cli = Cli::try_parse_from(args).unwrap(); + + assert!(cli.command.is_none()); + } + + #[test] + fn it_should_show_help_with_help_flag() { + let args = vec!["torrust-tracker-deployer", "--help"]; + let result = Cli::try_parse_from(args); + + // Help flag causes a "display help" error + assert!(result.is_err()); + let error = result.unwrap_err(); + assert_eq!(error.kind(), clap::error::ErrorKind::DisplayHelp); + } + + #[test] + fn it_should_show_version_with_version_flag() { + let args = vec!["torrust-tracker-deployer", "--version"]; + let result = Cli::try_parse_from(args); + + // Version flag causes a "display version" error + assert!(result.is_err()); + let error = result.unwrap_err(); + assert_eq!(error.kind(), clap::error::ErrorKind::DisplayVersion); + } + + #[test] + fn it_should_show_destroy_help() { + let args = vec!["torrust-tracker-deployer", "destroy", "--help"]; + let result = Cli::try_parse_from(args); + + // Help flag causes a "display help" error + assert!(result.is_err()); + let error = result.unwrap_err(); + assert_eq!(error.kind(), clap::error::ErrorKind::DisplayHelp); + + // Verify the help text mentions the environment parameter + let help_text = error.to_string(); + assert!( + help_text.contains("environment") || help_text.contains(""), + "Help text should mention environment parameter" + ); + } +} diff --git a/src/presentation/commands/destroy.rs b/src/presentation/commands/destroy.rs new file mode 100644 index 00000000..55c705f8 --- /dev/null +++ b/src/presentation/commands/destroy.rs @@ -0,0 +1,284 @@ +//! Destroy Command Handler +//! +//! This module handles the destroy command execution, including environment validation, +//! repository access, and infrastructure destruction. It provides user-friendly +//! progress updates and comprehensive error handling. + +use std::time::Duration; + +use thiserror::Error; + +use crate::application::command_handlers::{ + destroy::DestroyCommandHandlerError, DestroyCommandHandler, +}; +use crate::domain::environment::name::{EnvironmentName, EnvironmentNameError}; +use crate::infrastructure::persistence::repository_factory::RepositoryFactory; +use crate::presentation::user_output::{UserOutput, VerbosityLevel}; + +/// Handle the destroy command +/// +/// This function orchestrates the environment destruction workflow by: +/// 1. Validating the environment name +/// 2. Loading the environment from persistent storage +/// 3. Executing the destroy command handler +/// 4. Providing user-friendly progress updates +/// +/// # Arguments +/// +/// * `environment_name` - The name of the environment to destroy +/// +/// # Returns +/// +/// Returns `Ok(())` on success, or a `DestroyError` if: +/// - Environment name is invalid +/// - Environment cannot be loaded +/// - Destruction fails +/// +/// # Errors +/// +/// This function will return an error if the environment name is invalid, +/// the environment cannot be loaded, or the destruction process fails. +/// All errors include detailed context and actionable troubleshooting guidance. +/// +/// # Example +/// +/// ```rust +/// use torrust_tracker_deployer_lib::presentation::commands::destroy; +/// +/// if let Err(e) = destroy::handle("test-env") { +/// eprintln!("Destroy failed: {e}"); +/// eprintln!("Help: {}", e.help()); +/// } +/// ``` +#[allow(clippy::result_large_err)] // Error contains detailed context for user guidance +pub fn handle(environment_name: &str) -> Result<(), DestroyError> { + // Create user output with default stdout/stderr channels + let mut output = UserOutput::new(VerbosityLevel::Normal); + + // Display initial progress (to stderr) + output.progress(&format!("Destroying environment '{environment_name}'...")); + + // Validate environment name + let env_name = EnvironmentName::new(environment_name.to_string()).map_err(|source| { + let error = DestroyError::InvalidEnvironmentName { + name: environment_name.to_string(), + source, + }; + output.error(&error.to_string()); + error + })?; + + // Create repository for loading environment state + let repository_factory = RepositoryFactory::new(Duration::from_secs(30)); + let repository = repository_factory.create(std::path::PathBuf::from("data")); + + // Create and execute destroy command handler + output.progress("Tearing down infrastructure..."); + + let command_handler = DestroyCommandHandler::new(repository); + + // Execute destroy - the handler will load the environment and handle all states internally + let _destroyed_env = command_handler.execute(&env_name).map_err(|source| { + let error = DestroyError::DestroyOperationFailed { + name: environment_name.to_string(), + source, + }; + output.error(&error.to_string()); + error + })?; + + output.progress("Cleaning up resources..."); + output.success(&format!( + "Environment '{environment_name}' destroyed successfully" + )); + + Ok(()) +} + +// ============================================================================ +// ERROR TYPES - Secondary Concerns +// ============================================================================ + +/// Destroy command specific errors +/// +/// This enum contains all error variants specific to the destroy command, +/// including environment validation, repository access, and destruction failures. +/// Each variant includes relevant context and actionable error messages. +#[derive(Debug, Error)] +pub enum DestroyError { + /// Environment name validation failed + /// + /// The provided environment name doesn't meet the validation requirements. + /// Use `.help()` for detailed troubleshooting steps. + #[error("Invalid environment name '{name}': {source} +Tip: Environment names must be 1-63 characters, start with letter/digit, contain only letters/digits/hyphens")] + InvalidEnvironmentName { + name: String, + #[source] + source: EnvironmentNameError, + }, + + /// Environment not found or inaccessible + /// + /// The environment couldn't be loaded from persistent storage. + /// Use `.help()` for detailed troubleshooting steps. + #[error( + "Environment '{name}' not found or inaccessible from data directory '{data_dir}' +Tip: Check if environment exists: ls -la {data_dir}/" + )] + EnvironmentNotAccessible { name: String, data_dir: String }, + + /// Destroy operation failed + /// + /// The destruction process encountered an error during execution. + /// Use `.help()` for detailed troubleshooting steps. + #[error( + "Failed to destroy environment '{name}': {source} +Tip: Check logs and try running with --log-output file-and-stderr for more details" + )] + DestroyOperationFailed { + name: String, + #[source] + source: DestroyCommandHandlerError, + }, + + /// Repository operation failed + /// + /// Failed to create or access the environment repository. + /// Use `.help()` for detailed troubleshooting steps. + #[error( + "Failed to access environment repository at '{data_dir}': {reason} +Tip: Check directory permissions and disk space" + )] + RepositoryAccessFailed { data_dir: String, reason: String }, +} + +impl DestroyError { + /// Get detailed troubleshooting guidance for this error + /// + /// This method provides comprehensive troubleshooting steps that can be + /// displayed to users when they need more help resolving the error. + /// + /// # Example + /// + /// ```rust + /// use torrust_tracker_deployer_lib::presentation::commands::destroy; + /// + /// # fn main() -> Result<(), Box> { + /// if let Err(e) = destroy::handle("test-env") { + /// eprintln!("Error: {e}"); + /// eprintln!("\nTroubleshooting:\n{}", e.help()); + /// } + /// # Ok(()) + /// # } + /// ``` + #[must_use] + pub fn help(&self) -> &'static str { + match self { + Self::InvalidEnvironmentName { .. } => { + "Invalid Environment Name - Detailed Troubleshooting: + +1. Check environment name format: + - Length: Must be 1-63 characters + - Start: Must begin with letter (a-z, A-Z) or digit (0-9) + - Characters: Only letters, digits, and hyphens allowed + - End: Must not end with a hyphen + +2. Common valid examples: + - 'production' + - 'test-env' + - 'e2e-provision' + - 'dev123' + +3. Common invalid examples: + - 'test_env' (underscores not allowed) + - '-test' (starts with hyphen) + - 'test-' (ends with hyphen) + - '' (empty string) + +For more information, see the environment naming conventions in the documentation." + } + + Self::EnvironmentNotAccessible { .. } => { + "Environment Not Accessible - Detailed Troubleshooting: + +1. Check if environment exists: + - List environments: ls -la data/ + - Look for directory with your environment name + +2. Verify file permissions: + - Check directory permissions: ls -ld data/ + - Ensure read/write access: chmod 755 data/ + +3. Check if environment was provisioned: + - Look for environment.json file: ls -la data/{env_name}/ + - Verify it's a valid deployment environment + +4. Common causes: + - Environment was never created (run provision first) + - Wrong data directory path + - Permission issues + - Corrupted environment state + +If the environment should exist, check the logs for more details." + } + + Self::DestroyOperationFailed { .. } => { + "Destroy Operation Failed - Detailed Troubleshooting: + +1. Check system resources: + - Ensure sufficient disk space + - Check network connectivity + - Verify system permissions + +2. Review the operation logs: + - Run with verbose logging: --log-output file-and-stderr + - Check log files in data/logs/ + - Look for specific error details + +3. Check infrastructure state: + - Verify LXD/OpenTofu are accessible + - Check if VMs/containers are running + - Ensure cleanup tools are available + +4. Manual intervention may be needed: + - Some resources might need manual cleanup + - Check provider-specific tools (lxc list, tofu state list) + - Remove stale infrastructure manually if needed + +5. Recovery options: + - Retry the destroy operation + - Force cleanup with provider tools + - Contact administrator if permissions are needed + +For persistent issues, check the infrastructure documentation." + } + + Self::RepositoryAccessFailed { .. } => { + "Repository Access Failed - Detailed Troubleshooting: + +1. Check directory permissions: + - Verify data directory exists and is accessible + - Ensure write permissions: chmod 755 data/ + - Check parent directory permissions + +2. Verify disk space: + - Check available space: df -h . + - Ensure sufficient space for operations + - Clean up if disk is full + +3. Check file system issues: + - Test file creation: touch data/test.tmp && rm data/test.tmp + - Look for file system errors in system logs + - Check if directory is on a read-only mount + +4. Common solutions: + - Create data directory: mkdir -p data + - Fix permissions: sudo chown -R $USER:$USER data/ + - Move to directory with sufficient space + +If the problem persists, check system logs and contact administrator." + } + } + } +} diff --git a/src/presentation/commands/mod.rs b/src/presentation/commands/mod.rs new file mode 100644 index 00000000..74fdad26 --- /dev/null +++ b/src/presentation/commands/mod.rs @@ -0,0 +1,115 @@ +//! Command Handlers Module +//! +//! This module provides unified command execution and error handling for all CLI commands. +//! It serves as the central dispatch point for command execution and provides consistent +//! error handling across all commands. + +use crate::presentation::cli::Commands; +use crate::presentation::errors::CommandError; + +// Re-export command modules +pub mod destroy; + +// Future command modules will be added here: +// pub mod provision; +// pub mod configure; +// pub mod release; + +/// Execute the given command +/// +/// This function serves as the central dispatcher for all CLI commands. +/// It matches the command type and delegates execution to the appropriate +/// command handler module. +/// +/// # Arguments +/// +/// * `command` - The parsed CLI command to execute +/// +/// # Returns +/// +/// Returns `Ok(())` on successful execution, or a `CommandError` if execution fails. +/// The error contains detailed context and actionable troubleshooting information. +/// +/// # Errors +/// +/// Returns an error if command execution fails. +/// +/// # Example +/// +/// ```rust +/// use clap::Parser; +/// use torrust_tracker_deployer_lib::presentation::{cli, commands}; +/// +/// let cli = cli::Cli::parse(); +/// if let Some(command) = cli.command { +/// let result = commands::execute(command); +/// match result { +/// Ok(_) => println!("Command executed successfully"), +/// Err(e) => commands::handle_error(&e), +/// } +/// } +/// ``` +pub fn execute(command: Commands) -> Result<(), CommandError> { + match command { + Commands::Destroy { environment } => { + destroy::handle(&environment)?; + Ok(()) + } // Future commands will be added here: + // + // Commands::Provision { environment, provider } => { + // provision::handle(&environment, &provider)?; + // Ok(()) + // } + // + // Commands::Configure { environment } => { + // configure::handle(&environment)?; + // Ok(()) + // } + // + // Commands::Release { environment, version } => { + // release::handle(&environment, &version)?; + // Ok(()) + // } + } +} + +/// Handle command errors with consistent user output +/// +/// This function provides standardized error output for all command failures. +/// It displays the error message and detailed troubleshooting information +/// to help users resolve issues. +/// +/// # Arguments +/// +/// * `error` - The command error to handle and display +/// +/// # Example +/// +/// ```rust +/// use clap::Parser; +/// use torrust_tracker_deployer_lib::presentation::{commands, cli, errors}; +/// use torrust_tracker_deployer_lib::presentation::commands::destroy::DestroyError; +/// use torrust_tracker_deployer_lib::domain::environment::name::EnvironmentNameError; +/// +/// # fn main() -> Result<(), Box> { +/// // Example of handling a command error (simulated for testing) +/// let name_error = EnvironmentNameError::InvalidFormat { +/// attempted_name: "invalid_name".to_string(), +/// reason: "contains invalid characters: _".to_string(), +/// valid_examples: vec!["dev".to_string(), "staging".to_string()], +/// }; +/// let sample_error = errors::CommandError::Destroy( +/// Box::new(DestroyError::InvalidEnvironmentName { +/// name: "invalid_name".to_string(), +/// source: name_error, +/// }) +/// ); +/// commands::handle_error(&sample_error); +/// # Ok(()) +/// # } +/// ``` +pub fn handle_error(error: &CommandError) { + eprintln!("Error: {error}"); + eprintln!("\nFor detailed troubleshooting:"); + eprintln!("{}", error.help()); +} diff --git a/src/presentation/errors.rs b/src/presentation/errors.rs new file mode 100644 index 00000000..e66d747e --- /dev/null +++ b/src/presentation/errors.rs @@ -0,0 +1,84 @@ +//! Presentation Layer Error Types +//! +//! This module defines unified error handling for CLI commands following the error +//! handling conventions documented in docs/contributing/error-handling.md. +//! +//! ## Design Principles +//! +//! - **Clarity**: Unambiguous error messages with specific context +//! - **Traceability**: Full error chains preserved for debugging +//! - **Actionability**: Clear instructions for resolution +//! - **Unified Structure**: Single `CommandError` enum containing all command-specific errors +//! +//! ## Error Hierarchy +//! +//! ```text +//! CommandError +//! └── Destroy(DestroyError) # Destroy command errors +//! ``` + +use thiserror::Error; + +use crate::presentation::commands::destroy::DestroyError; + +/// Errors that can occur during CLI command execution +/// +/// This enum provides a unified interface for all command-specific errors, +/// following the project's error handling conventions with structured error +/// types, source preservation, and tiered help system support. +#[derive(Debug, Error)] +pub enum CommandError { + /// Destroy command specific errors + /// + /// Encapsulates all errors that can occur during environment destruction. + /// Use `.help()` for detailed troubleshooting steps. + #[error("Destroy command failed: {0}")] + Destroy(Box), +} + +impl From for CommandError { + fn from(error: DestroyError) -> Self { + Self::Destroy(Box::new(error)) + } +} + +impl CommandError { + /// Get detailed troubleshooting guidance for this error + /// + /// This method provides comprehensive troubleshooting steps that can be + /// displayed to users when they need more help resolving the error. + /// It delegates to the specific command error's help method. + /// + /// # Example + /// + /// ```rust + /// use clap::Parser; + /// use torrust_tracker_deployer_lib::presentation::{cli, errors}; + /// use torrust_tracker_deployer_lib::presentation::commands::destroy::DestroyError; + /// use torrust_tracker_deployer_lib::application::command_handlers::destroy::DestroyCommandHandlerError; + /// use std::path::PathBuf; + /// + /// # fn main() -> Result<(), Box> { + /// // Create error for demonstration + /// let destroy_error = DestroyError::DestroyOperationFailed { + /// name: "test-env".to_string(), + /// source: DestroyCommandHandlerError::StateCleanupFailed { + /// path: PathBuf::from("/tmp/test"), + /// source: std::io::Error::new(std::io::ErrorKind::PermissionDenied, "access denied"), + /// }, + /// }; + /// let error = errors::CommandError::Destroy(Box::new(destroy_error)); + /// + /// // Get help text + /// let help_text = error.help(); + /// println!("{}", help_text); + /// # Ok(()) + /// # } + /// ``` + #[must_use] + pub fn help(&self) -> &'static str { + match self { + Self::Destroy(e) => e.help(), + } + } +} diff --git a/src/presentation/mod.rs b/src/presentation/mod.rs index b8d21739..c943760e 100644 --- a/src/presentation/mod.rs +++ b/src/presentation/mod.rs @@ -7,6 +7,8 @@ //! ## Responsibilities //! //! - **User Output**: Managing user-facing messages, progress updates, and result presentation +//! - **CLI Interface**: Command-line argument parsing and subcommand definitions +//! - **Command Execution**: Coordinating command handlers and providing unified error handling //! - **Output Channels**: Implementing proper stdout/stderr separation for CLI applications //! - **Verbosity Control**: Handling different levels of output detail based on user preferences //! - **Output Formatting**: Structuring output for both human consumption and automation/piping @@ -17,8 +19,33 @@ //! - **Automation Friendly**: Supporting clean piping and redirection for scripting //! - **User Experience**: Providing clear, actionable feedback without interfering with result data //! - **Verbosity Levels**: Respecting user preferences for output detail +//! - **Error Conventions**: Following project error handling guidelines with structured errors and tiered help +//! +//! ## Module Structure +//! +//! ```text +//! presentation/ +//! ├── cli/ # CLI argument parsing and structure +//! │ ├── args.rs # Global CLI arguments (logging config) +//! │ ├── commands.rs # Subcommand definitions +//! │ └── mod.rs # Main Cli struct and parsing logic +//! ├── commands/ # Command execution handlers +//! │ ├── destroy.rs # Destroy command handler +//! │ └── mod.rs # Unified command dispatch and error handling +//! ├── errors.rs # Unified error types for all commands +//! ├── user_output.rs # User-facing output management +//! └── mod.rs # This file - layer exports and documentation +//! ``` +// Core presentation modules +pub mod cli; +pub mod commands; +pub mod errors; pub mod user_output; // Re-export commonly used presentation types for convenience +pub use cli::{Cli, Commands, GlobalArgs}; +pub use commands::destroy::DestroyError; +pub use commands::{execute, handle_error}; +pub use errors::CommandError; pub use user_output::{UserOutput, VerbosityLevel}; From 60668341bd24036129318200a6ce75536ec594e8 Mon Sep 17 00:00:00 2001 From: Jose Celano Date: Fri, 24 Oct 2025 16:40:16 +0100 Subject: [PATCH 13/14] docs: [#23] improve architectural guidance and rename confusing templates - Enhanced codebase-architecture.md with presentation layer documentation - Added architecture requirements to issue templates - Renamed TEMPLATE.md to SPECIFICATION-TEMPLATE.md for clarity - Renamed TASK-TEMPLATE.md to GITHUB-ISSUE-TEMPLATE.md for clarity - Updated template references in roadmap-issues.md These improvements address architectural gaps that caused AI assistants to implement CLI functionality in wrong DDD layers. --- docs/codebase-architecture.md | 91 ++++++++++++++++++- docs/contributing/roadmap-issues.md | 20 +++- docs/issues/GITHUB-ISSUE-TEMPLATE.md | 57 ++++++++++++ ...{TEMPLATE.md => SPECIFICATION-TEMPLATE.md} | 24 +++++ docs/issues/TASK-TEMPLATE.md | 33 ------- 5 files changed, 183 insertions(+), 42 deletions(-) create mode 100644 docs/issues/GITHUB-ISSUE-TEMPLATE.md rename docs/issues/{TEMPLATE.md => SPECIFICATION-TEMPLATE.md} (51%) delete mode 100644 docs/issues/TASK-TEMPLATE.md diff --git a/docs/codebase-architecture.md b/docs/codebase-architecture.md index aa6efc72..df075de1 100644 --- a/docs/codebase-architecture.md +++ b/docs/codebase-architecture.md @@ -10,6 +10,14 @@ The project follows **Domain-Driven Design** principles with a layered architect ```text ┌─────────────────────────────────────┐ +│ Presentation Layer │ +│ (CLI, User Output, Command │ +│ Dispatch, Error Display) │ +│ src/presentation/ │ +└────────────┬────────────────────────┘ + │ depends on + ↓ +┌─────────────────────────────────────┐ │ Application Layer │ │ (Commands, Use Cases, Steps) │ │ src/application/ │ @@ -35,6 +43,18 @@ The project follows **Domain-Driven Design** principles with a layered architect ### Layer Responsibilities +**Presentation Layer** (`src/presentation/`): + +- **Purpose**: User interface and interaction handling +- **Contains**: CLI parsing, command dispatch, user output, error presentation, help systems +- **Rules**: Depends on application layer, handles all user-facing concerns +- **Example**: CLI subcommands calling application command handlers with user-friendly error messages +- **Module Structure**: + - `cli/` - Command-line argument parsing and validation + - `commands/` - Command execution handlers and dispatch logic + - `errors.rs` - Unified error types with tiered help system + - `user_output.rs` - User-facing output management and verbosity control + **Domain Layer** (`src/domain/`): - **Purpose**: Core business logic and domain entities @@ -58,14 +78,16 @@ The project follows **Domain-Driven Design** principles with a layered architect ### Dependency Rule -The fundamental rule is that **dependencies flow inward**: +The fundamental rule is that **dependencies flow inward toward the domain**: +- Presentation → Application → Domain (✅ Correct) - Infrastructure → Domain (✅ Correct) -- Application → Domain (✅ Correct) -- Domain → Infrastructure (❌ Forbidden) - Domain → Application (❌ Forbidden) +- Domain → Infrastructure (❌ Forbidden) +- Domain → Presentation (❌ Forbidden) +- Application → Presentation (❌ Forbidden) -This ensures the domain layer remains pure business logic, free from technical implementation details. +This ensures the domain layer remains pure business logic, free from technical implementation details and user interface concerns. The presentation layer orchestrates the application layer but never contains business logic. ## 🏗️ Three-Level Architecture Pattern @@ -181,6 +203,21 @@ All modules include comprehensive `//!` documentation with: - ✅ `src/bin/e2e-provision-and-destroy-tests.rs` - E2E provisioning and destruction tests - ✅ `src/bin/e2e-tests-full.rs` - Full E2E test suite +### Presentation Layer + +**CLI Interface and User Interaction:** + +- ✅ `src/presentation/mod.rs` - Presentation layer root module with exports +- ✅ `src/presentation/cli/` - Command-line interface parsing and structure + - `cli/mod.rs` - Main Cli struct and global argument definitions + - `cli/args.rs` - Global CLI arguments (logging configuration) + - `cli/commands.rs` - Subcommand definitions (destroy, future commands) +- ✅ `src/presentation/commands/` - Command execution handlers + - `commands/mod.rs` - Unified command dispatch and error handling + - `commands/destroy.rs` - Destroy command handler with error management +- ✅ `src/presentation/errors.rs` - Unified error types with tiered help system +- ✅ `src/presentation/user_output.rs` - User-facing output management and verbosity control + ### Domain Layer **Core Domain Entities:** @@ -381,11 +418,55 @@ The typical deployment flow follows this pattern: ## 📊 Module Statistics - **Total Modules**: ~100+ Rust files -- **Architecture Layers**: 3 (Domain, Application, Infrastructure) + Shared +- **Architecture Layers**: 4 (Presentation, Application, Domain, Infrastructure) + Shared - **External Tool Integrations**: 3 (OpenTofu, Ansible, LXD) - **Step Categories**: 7 (Infrastructure, System, Software, Validation, Connectivity, Application, Rendering) - **State Types**: 13+ environment states with type-state pattern +## 🏗️ Architectural Guidance for Development + +When working on this codebase, follow these guidelines to maintain architectural integrity: + +### Layer Selection Guide + +**When creating new functionality, choose the appropriate layer:** + +- **Presentation Layer** (`src/presentation/`): CLI commands, user output, error display, input validation +- **Application Layer** (`src/application/`): Use cases, command orchestration, workflow coordination +- **Domain Layer** (`src/domain/`): Business entities, value objects, domain rules, state machines +- **Infrastructure Layer** (`src/infrastructure/`): External tool integration, file operations, remote actions + +### Module Placement Rules + +1. **CLI and User Interface**: Always in `src/presentation/` +2. **Business Logic**: Always in `src/domain/` +3. **Use Case Orchestration**: Always in `src/application/` +4. **External Integration**: Always in `src/infrastructure/` +5. **Cross-Layer Utilities**: Only in `src/shared/` (use sparingly) + +### Dependency Guidelines + +- ✅ **Allowed**: Presentation → Application → Domain ← Infrastructure +- ❌ **Forbidden**: Domain depending on any other layer +- ❌ **Forbidden**: Application depending on Presentation or Infrastructure +- ❌ **Forbidden**: Circular dependencies between any layers + +### Implementation Patterns + +- **Error Handling**: Use structured enums with `thiserror`, implement tiered help systems +- **Module Organization**: Follow [docs/contributing/module-organization.md](../docs/contributing/module-organization.md) +- **Testing**: Layer-appropriate testing (unit tests per layer, integration tests across layers) + +### Quality Assurance + +Before implementing new features: + +1. **Identify the correct layer** based on the functionality's purpose +2. **Check architectural constraints** - ensure no forbidden dependencies +3. **Follow module organization** - public before private, important before secondary +4. **Implement proper error handling** - structured errors with actionable messages +5. **Add comprehensive tests** - appropriate for the layer and functionality + ## 💡 Key Design Principles - **Domain-Driven Design**: Pure domain logic independent of infrastructure diff --git a/docs/contributing/roadmap-issues.md b/docs/contributing/roadmap-issues.md index dfcd4c39..2374fc23 100644 --- a/docs/contributing/roadmap-issues.md +++ b/docs/contributing/roadmap-issues.md @@ -45,7 +45,7 @@ Create a detailed specification in `docs/issues/` with temporary name: ```bash # Copy the template to create your specification document -cp docs/issues/TEMPLATE.md docs/issues/setup-logging-for-production-cli.md +cp docs/issues/SPECIFICATION-TEMPLATE.md docs/issues/setup-logging-for-production-cli.md # Edit the document to fill in all sections vim docs/issues/setup-logging-for-production-cli.md @@ -53,7 +53,7 @@ vim docs/issues/setup-logging-for-production-cli.md #### Document Structure -Use the template at [`docs/issues/TEMPLATE.md`](../issues/TEMPLATE.md) as your starting point. The template includes: +Use the template at [`docs/issues/SPECIFICATION-TEMPLATE.md`](../issues/SPECIFICATION-TEMPLATE.md) as your starting point. The template includes: - **Header**: Issue number, parent epic, and related links - **Overview**: Clear description of what the task accomplishes @@ -71,6 +71,18 @@ Use the template at [`docs/issues/TEMPLATE.md`](../issues/TEMPLATE.md) as your s - **Provide Context**: Link to related documentation, ADRs, and examples - **Estimate Time**: Help others understand scope - **Define Success**: Clear acceptance criteria +- **Specify Architecture**: Include DDD layer, module path, and architectural constraints (see template guidance below) + +#### Architectural Requirements + +When creating issues that involve code changes, always specify: + +- **DDD Layer**: Which layer the change belongs to (Presentation, Application, Domain, Infrastructure) +- **Module Path**: Exact location in the codebase (`src/{layer}/{module}/`) +- **Architectural Constraints**: Dependencies, patterns, and anti-patterns to follow +- **Related Documentation**: Link to [docs/codebase-architecture.md](../docs/codebase-architecture.md) and other relevant architectural guidance + +This ensures AI assistants and contributors understand not just **what** to implement, but **where** and **how** to implement it within the project's architectural patterns. ### Step 2: Create GitHub Epic Issue (if needed) @@ -100,7 +112,7 @@ If this is the first task in a roadmap section, create an epic issue first: 2. **Click "New Issue"** -3. **Fill in Task Details**: Use the template at [`docs/issues/TASK-TEMPLATE.md`](../issues/TASK-TEMPLATE.md) and fill in: +3. **Fill in Task Details**: Use the template at [`docs/issues/GITHUB-ISSUE-TEMPLATE.md`](../issues/GITHUB-ISSUE-TEMPLATE.md) and fill in: - Title with the task name from roadmap - Overview with brief description @@ -223,7 +235,7 @@ Implementing roadmap task **1.1 Setup logging** ```bash # 1. Create initial specification document from template -cp docs/issues/TEMPLATE.md docs/issues/setup-logging-for-production-cli.md +cp docs/issues/SPECIFICATION-TEMPLATE.md docs/issues/setup-logging-for-production-cli.md vim docs/issues/setup-logging-for-production-cli.md # ... fill in all sections: overview, goals, specs, implementation plan ... diff --git a/docs/issues/GITHUB-ISSUE-TEMPLATE.md b/docs/issues/GITHUB-ISSUE-TEMPLATE.md new file mode 100644 index 00000000..a52fcd30 --- /dev/null +++ b/docs/issues/GITHUB-ISSUE-TEMPLATE.md @@ -0,0 +1,57 @@ +Title: [Task Name from Roadmap] + +## Overview + +[Brief description of what this task accomplishes] + +## Specification + +See detailed specification: [docs/issues/{number}-{name}.md](../docs/issues/{number}-{name}.md) + +(Link will be updated after file rename) + +## 🏗️ Architecture Requirements + +**DDD Layer**: [Domain | Application | Infrastructure | Presentation] +**Module Path**: `src/{layer}/{module}/` +**Pattern**: [Command | Step | Action | CLI Subcommand | Entity | Value Object | Repository] + +### Module Structure Requirements + +- [ ] Follow DDD layer separation (see [docs/codebase-architecture.md](../docs/codebase-architecture.md)) +- [ ] Respect dependency flow rules (dependencies flow toward domain) +- [ ] Use appropriate module organization (see [docs/contributing/module-organization.md](../docs/contributing/module-organization.md)) + +### Architectural Constraints + +- [ ] No business logic in presentation layer +- [ ] Error handling follows project conventions (see [docs/contributing/error-handling.md](../docs/contributing/error-handling.md)) +- [ ] Testing strategy aligns with layer responsibilities + +### Anti-Patterns to Avoid + +- ❌ Mixing concerns across layers +- ❌ Domain layer depending on infrastructure +- ❌ Monolithic modules with multiple responsibilities + +## Implementation Plan + +### Phase 1: [Phase Name] + +- [ ] Task 1.1 +- [ ] Task 1.2 + +### Phase 2: [Phase Name] + +- [ ] Task 2.1 + +## Acceptance Criteria + +- [ ] Criterion 1 +- [ ] Criterion 2 + +## Related + +- Parent: #X (Epic: [Epic Name]) +- Roadmap: #1 (Project Roadmap) +- Specification: docs/issues/{number}-{name}.md diff --git a/docs/issues/TEMPLATE.md b/docs/issues/SPECIFICATION-TEMPLATE.md similarity index 51% rename from docs/issues/TEMPLATE.md rename to docs/issues/SPECIFICATION-TEMPLATE.md index 6bb069ab..e737c54c 100644 --- a/docs/issues/TEMPLATE.md +++ b/docs/issues/SPECIFICATION-TEMPLATE.md @@ -14,6 +14,30 @@ - [ ] Goal 2 - [ ] Goal 3 +## 🏗️ Architecture Requirements + +**DDD Layer**: [Domain | Application | Infrastructure | Presentation] +**Module Path**: `src/{layer}/{module}/` +**Pattern**: [Command | Step | Action | CLI Subcommand | Entity | Value Object | Repository] + +### Module Structure Requirements + +- [ ] Follow DDD layer separation (see [docs/codebase-architecture.md](../docs/codebase-architecture.md)) +- [ ] Respect dependency flow rules (dependencies flow toward domain) +- [ ] Use appropriate module organization (see [docs/contributing/module-organization.md](../docs/contributing/module-organization.md)) + +### Architectural Constraints + +- [ ] No business logic in presentation layer +- [ ] Error handling follows project conventions (see [docs/contributing/error-handling.md](../docs/contributing/error-handling.md)) +- [ ] Testing strategy aligns with layer responsibilities + +### Anti-Patterns to Avoid + +- ❌ Mixing concerns across layers +- ❌ Domain layer depending on infrastructure +- ❌ Monolithic modules with multiple responsibilities + ## Specifications ### [Specification Section 1] diff --git a/docs/issues/TASK-TEMPLATE.md b/docs/issues/TASK-TEMPLATE.md deleted file mode 100644 index 0865c4bb..00000000 --- a/docs/issues/TASK-TEMPLATE.md +++ /dev/null @@ -1,33 +0,0 @@ -Title: [Task Name from Roadmap] - -## Overview - -[Brief description of what this task accomplishes] - -## Specification - -See detailed specification: [docs/issues/{number}-{name}.md](../docs/issues/{number}-{name}.md) - -(Link will be updated after file rename) - -## Implementation Plan - -### Phase 1: [Phase Name] - -- [ ] Task 1.1 -- [ ] Task 1.2 - -### Phase 2: [Phase Name] - -- [ ] Task 2.1 - -## Acceptance Criteria - -- [ ] Criterion 1 -- [ ] Criterion 2 - -## Related - -- Parent: #X (Epic: [Epic Name]) -- Roadmap: #1 (Project Roadmap) -- Specification: docs/issues/{number}-{name}.md From 87ac5f6a752b0d0601bbf4ddcdf643941354db84 Mon Sep 17 00:00:00 2001 From: Jose Celano Date: Fri, 24 Oct 2025 16:51:30 +0100 Subject: [PATCH 14/14] fix: [#23] add reprovisioning to project dictionary Fixes CI linting failure by adding 'reprovisioning' to project-words.txt. This word appears in docs/features/hybrid-command-architecture/README.md from main branch when PR is merged for CI testing. --- project-words.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/project-words.txt b/project-words.txt index af33c73b..e8dc1bbe 100644 --- a/project-words.txt +++ b/project-words.txt @@ -106,6 +106,7 @@ Pulumi pytest RAII Repomix +reprovisioning reqwest resolv rgba