Move CLI parsing to separate module

fix potential cli parsing crash

Author: lovasoa

src/app_config.rs

@@ -1,7 +1,7 @@
 use crate::webserver::content_security_policy::ContentSecurityPolicyTemplate;
 use crate::webserver::routing::RoutingConfig;
 use anyhow::Context;
-use clap::Parser;
+use crate::cli::arguments::{Cli, parse_cli};
 use config::Config;
 use openidconnect::IssuerUrl;
 use percent_encoding::AsciiSet;
@@ -10,34 +10,6 @@ use serde::{Deserialize, Deserializer, Serialize};
 use std::net::{SocketAddr, ToSocketAddrs};
 use std::path::{Path, PathBuf};
 
-#[derive(Parser)]
-#[clap(author, version, about, long_about = None)]
-pub struct Cli {
-    /// The directory where the .sql files are located.
-    #[clap(short, long)]
-    pub web_root: Option<PathBuf>,
-    /// The directory where the sqlpage.json configuration, the templates, and the migrations are located.
-    #[clap(short = 'd', long)]
-    pub config_dir: Option<PathBuf>,
-    /// The path to the configuration file.
-    #[clap(short = 'c', long)]
-    pub config_file: Option<PathBuf>,
-
-    /// Subcommands for additional functionality.
-    #[clap(subcommand)]
-    pub command: Option<Commands>,
-}
-
-/// Enum for subcommands.
-#[derive(Parser)]
-pub enum Commands {
-    /// Create a new migration file.
-    CreateMigration {
-        /// Name of the migration.
-        migration_name: String,
-    },
-}
-
 #[cfg(not(feature = "lambda-web"))]
 const DEFAULT_DATABASE_FILE: &str = "sqlpage.db";
 
@@ -154,8 +126,8 @@ impl AppConfig {
     }
 }
 
-pub fn load_from_cli() -> anyhow::Result<AppConfig> {
-    let cli = Cli::parse();
+pub fn load_config() -> anyhow::Result<AppConfig> {
+    let cli = parse_cli()?;
     AppConfig::from_cli(&cli)
 }
 
@@ -654,23 +626,6 @@ mod test {
         );
     }
 
-    #[test]
-    fn test_cli_argument_parsing() {
-        let cli = Cli::parse_from([
-            "sqlpage",
-            "--web-root",
-            "/path/to/web",
-            "--config-dir",
-            "/path/to/config",
-            "--config-file",
-            "/path/to/config.json",
-        ]);
-
-        assert_eq!(cli.web_root, Some(PathBuf::from("/path/to/web")));
-        assert_eq!(cli.config_dir, Some(PathBuf::from("/path/to/config")));
-        assert_eq!(cli.config_file, Some(PathBuf::from("/path/to/config.json")));
-    }
-
     #[test]
     fn test_sqlpage_prefixed_env_variable_parsing() {
         let _lock = ENV_LOCK

src/cli/arguments.rs

@@ -0,0 +1,43 @@
+use clap::Parser;
+use std::path::PathBuf;
+use super::commands::SubCommand;
+
+#[derive(Parser)]
+#[clap(author, version, about, long_about = None)]
+pub struct Cli {
+    /// The directory where the .sql files are located.
+    #[clap(short, long)]
+    pub web_root: Option<PathBuf>,
+    /// The directory where the sqlpage.json configuration, the templates, and the migrations are located.
+    #[clap(short = 'd', long)]
+    pub config_dir: Option<PathBuf>,
+    /// The path to the configuration file.
+    #[clap(short = 'c', long)]
+    pub config_file: Option<PathBuf>,
+
+    /// Subcommands for additional functionality.
+    #[clap(subcommand)]
+    pub command: Option<SubCommand>,
+}
+
+pub fn parse_cli() -> anyhow::Result<Cli> {
+    let cli = Cli::parse();
+    Ok(cli)
+}
+
+#[test]
+fn test_cli_argument_parsing() {
+    let cli = Cli::parse_from([
+        "sqlpage",
+        "--web-root",
+        "/path/to/web",
+        "--config-dir",
+        "/path/to/config",
+        "--config-file",
+        "/path/to/config.json",
+    ]);
+
+    assert_eq!(cli.web_root, Some(PathBuf::from("/path/to/web")));
+    assert_eq!(cli.config_dir, Some(PathBuf::from("/path/to/config")));
+    assert_eq!(cli.config_file, Some(PathBuf::from("/path/to/config.json")));
+}

src/cli/commands.rs

@@ -0,0 +1,77 @@
+use clap::Parser;
+use std::path::Path;
+use chrono::Utc;
+
+use crate::app_config::AppConfig;
+
+/// Sub-commands for the sqlpage CLI.
+/// Each subcommand can be executed using the `sqlpage <subcommand name>` from the command line.
+#[derive(Parser)]
+pub enum SubCommand {
+    /// Create a new migration file.
+    CreateMigration {
+        /// Name of the migration.
+        migration_name: String,
+    },
+}
+
+impl SubCommand {
+    /// Execute the subcommand.
+    pub async fn execute(&self, app_config: AppConfig) -> anyhow::Result<()> {
+        match self {
+            SubCommand::CreateMigration { migration_name } => {
+                // Pass configuration_directory from app_config
+                create_migration_file(
+                    migration_name,
+                    &app_config.configuration_directory,
+                ).await?;
+                Ok(())
+            }
+        }
+    }
+}
+
+async fn create_migration_file(
+    migration_name: &str,
+    configuration_directory: &Path,
+) -> anyhow::Result<()> {
+    let timestamp = Utc::now().format("%Y%m%d%H%M%S").to_string();
+    let snake_case_name = migration_name
+        .replace(|c: char| !c.is_alphanumeric(), "_")
+        .to_lowercase();
+    let file_name = format!("{timestamp}_{snake_case_name}.sql");
+    let migrations_dir = Path::new(configuration_directory).join("migrations");
+
+    if !migrations_dir.exists() {
+        tokio::fs::create_dir_all(&migrations_dir).await?;
+    }
+
+    let mut unique_file_name = file_name.clone();
+    let mut counter = 1;
+
+    while migrations_dir.join(&unique_file_name).exists() {
+        unique_file_name = format!("{timestamp}_{snake_case_name}_{counter}.sql");
+        counter += 1;
+    }
+
+    let file_path = migrations_dir.join(unique_file_name);
+    tokio::fs::write(&file_path, "-- Write your migration here\n").await?;
+
+    // the following code cleans up the display path to show where the migration was created
+    // relative to the current working directory, and then outputs the path to the migration
+    let file_path_canon = file_path.canonicalize().unwrap_or(file_path.clone());
+    let cwd_canon = std::env::current_dir()?
+        .canonicalize()
+        .unwrap_or(std::env::current_dir()?);
+    let rel_path = match file_path_canon.strip_prefix(&cwd_canon) {
+        Ok(p) => p,
+        Err(_) => file_path_canon.as_path(),
+    };
+    let mut display_path_str = rel_path.display().to_string();
+    if display_path_str.starts_with("\\\\?\\") {
+        display_path_str = display_path_str.trim_start_matches("\\\\?\\").to_string();
+    }
+    display_path_str = display_path_str.replace('\\', "/");
+    println!("Migration file created: {display_path_str}");
+    Ok(())
+}

src/cli/mod.rs

@@ -0,0 +1,2 @@
+pub mod arguments;
+pub mod commands;

src/lib.rs

@@ -79,6 +79,7 @@ pub mod template_helpers;
 pub mod templates;
 pub mod utils;
 pub mod webserver;
+pub mod cli;
 
 use crate::app_config::AppConfig;
 use crate::filesystem::FileSystem;

src/main.rs

@@ -1,6 +1,6 @@
-use clap::Parser;
 use sqlpage::{
-    app_config,
+    app_config::AppConfig,
+    cli,
     webserver::{self, Database},
     AppState,
 };
@@ -15,20 +15,11 @@ async fn main() {
 }
 
 async fn start() -> anyhow::Result<()> {
-    let app_config = app_config::load_from_cli()?;
-    let cli = app_config::Cli::parse();
+    let cli = cli::arguments::parse_cli()?;
+    let app_config = AppConfig::from_cli(&cli)?;
 
     if let Some(command) = cli.command {
-        match command {
-            app_config::Commands::CreateMigration { migration_name } => {
-                // Pass configuration_directory from app_config
-                create_migration_file(
-                    &migration_name,
-                    app_config.configuration_directory.to_str().unwrap(),
-                )?;
-                return Ok(());
-            }
-        }
+        return command.execute(app_config).await;
     }
 
     let db = Database::init(&app_config).await?;
@@ -58,52 +49,3 @@ fn init_logging() {
         Err(e) => log::error!("Error loading .env file: {e}"),
     }
 }
-
-fn create_migration_file(
-    migration_name: &str,
-    configuration_directory: &str,
-) -> anyhow::Result<()> {
-    use chrono::Utc;
-    use std::fs;
-    use std::path::Path;
-
-    let timestamp = Utc::now().format("%Y%m%d%H%M%S").to_string();
-    let snake_case_name = migration_name
-        .replace(|c: char| !c.is_alphanumeric(), "_")
-        .to_lowercase();
-    let file_name = format!("{timestamp}_{snake_case_name}.sql");
-    let migrations_dir = Path::new(configuration_directory).join("migrations");
-
-    if !migrations_dir.exists() {
-        fs::create_dir_all(&migrations_dir)?;
-    }
-
-    let mut unique_file_name = file_name.clone();
-    let mut counter = 1;
-
-    while migrations_dir.join(&unique_file_name).exists() {
-        unique_file_name = format!("{timestamp}_{snake_case_name}_{counter}.sql");
-        counter += 1;
-    }
-
-    let file_path = migrations_dir.join(unique_file_name);
-    fs::write(&file_path, "-- Write your migration here\n")?;
-
-    // the following code cleans up the display path to show where the migration was created
-    // relative to the current working directory, and then outputs the path to the migration
-    let file_path_canon = file_path.canonicalize().unwrap_or(file_path.clone());
-    let cwd_canon = std::env::current_dir()?
-        .canonicalize()
-        .unwrap_or(std::env::current_dir()?);
-    let rel_path = match file_path_canon.strip_prefix(&cwd_canon) {
-        Ok(p) => p,
-        Err(_) => file_path_canon.as_path(),
-    };
-    let mut display_path_str = rel_path.display().to_string();
-    if display_path_str.starts_with("\\\\?\\") {
-        display_path_str = display_path_str.trim_start_matches("\\\\?\\").to_string();
-    }
-    display_path_str = display_path_str.replace('\\', "/");
-    println!("Migration file created: {display_path_str}");
-    Ok(())
-}