chore: reduce redundancy (#8)

Author: QuakeWang

src/config.rs

@@ -20,12 +20,7 @@ const ENV_NO_PROGRESS: &str = "CLOUD_CLI_NO_PROGRESS";
 
 impl Default for Config {
     fn default() -> Self {
-        Self {
-            jdk_path: PathBuf::from("/opt/jdk"),
-            output_dir: PathBuf::from("/opt/selectdb/information"),
-            timeout_seconds: 60,
-            no_progress_animation: false,
-        }
+        config_loader::to_app_config(config_loader::DorisConfig::default())
     }
 }
 
@@ -74,16 +69,6 @@ impl Config {
         self
     }
 
-    pub fn with_timeout(mut self, seconds: u64) -> Self {
-        self.timeout_seconds = seconds;
-        self
-    }
-
-    pub fn with_progress_animation(mut self, enable: bool) -> Self {
-        self.no_progress_animation = !enable;
-        self
-    }
-
     pub fn validate(&self) -> Result<()> {
         self.validate_jdk_path()?;
         self.validate_output_dir()?;

src/config_loader/config_parser.rs

@@ -102,12 +102,6 @@ impl<'a> ConfigParser for StringConfigParser<'a> {
     }
 }
 
-/// Parse configuration for the specified environment
-pub fn parse_config(env: Environment) -> Result<DorisConfig> {
-    let (install_dir, jdk_path) = process_detector::get_paths(env)?;
-    parse_config_internal(env, &install_dir, &jdk_path)
-}
-
 /// Parse configuration from specified path
 pub fn parse_config_from_path(env: Environment, install_dir: &Path) -> Result<DorisConfig> {
     let jdk_path = PathBuf::from("/opt/jdk");

src/config_loader/config_persister.rs

@@ -203,19 +203,6 @@ impl ConfigConverter<ProcessInfo> for DorisConfig {
     }
 }
 
-impl ConfigConverter<PersistentConfig> for DorisConfig {
-    fn convert_to(&self) -> PersistentConfig {
-        PersistentConfig {
-            metadata: self.convert_to(),
-            paths: self.convert_to(),
-            ports: self.convert_to(),
-            network: self.convert_to(),
-            settings: self.convert_to(),
-            process: self.convert_to(),
-        }
-    }
-}
-
 impl ConfigConverter<DorisConfig> for PersistentConfig {
     fn convert_to(&self) -> DorisConfig {
         let environment = match self.metadata.environment.as_str() {
@@ -380,14 +367,6 @@ impl PersistResult {
     pub fn is_success(&self) -> bool {
         !matches!(self, PersistResult::AllFailed(_))
     }
-
-    pub fn success_path(&self) -> Option<&PathBuf> {
-        match self {
-            PersistResult::Success(path) => Some(path),
-            PersistResult::PartialSuccess(path, _) => Some(path),
-            PersistResult::AllFailed(_) => None,
-        }
-    }
 }
 
 /// Persist configuration to file
@@ -751,53 +730,3 @@ fn to_organized_config(config: &DorisConfig) -> OrganizedConfig {
         process: config.convert_to(),
     }
 }
-
-/// Save configuration in organized format
-pub fn save_organized_config(config: &DorisConfig) -> Result<PersistResult> {
-    let config_paths = get_config_file_paths()?;
-    let organized_config = to_organized_config(config);
-    let toml_str = toml::to_string_pretty(&organized_config)?;
-
-    let mut errors = Vec::new();
-
-    for config_path in &config_paths {
-        if let Err(e) = ensure_dir_exists(config_path) {
-            errors.push((
-                config_path.clone(),
-                format!("Failed to create directory: {e}"),
-            ));
-            continue;
-        }
-
-        if !is_path_writable(config_path.parent().unwrap_or(config_path)) {
-            errors.push((config_path.clone(), "No write permission".to_string()));
-            continue;
-        }
-
-        match fs::File::create(config_path) {
-            Ok(mut file) => match file.write_all(toml_str.as_bytes()) {
-                Ok(_) => {
-                    if errors.is_empty() {
-                        return Ok(PersistResult::Success(config_path.clone()));
-                    } else {
-                        return Ok(PersistResult::PartialSuccess(config_path.clone(), errors));
-                    }
-                }
-                Err(e) => {
-                    errors.push((config_path.clone(), format!("Write error: {e}")));
-                }
-            },
-            Err(e) => {
-                errors.push((config_path.clone(), format!("Create file error: {e}")));
-            }
-        }
-    }
-
-    if !errors.is_empty() {
-        Ok(PersistResult::AllFailed(errors))
-    } else {
-        Err(CliError::ConfigError(
-            "No valid paths to persist config".to_string(),
-        ))
-    }
-}

src/config_loader/mod.rs

@@ -25,6 +25,7 @@ impl std::fmt::Display for Environment {
     }
 }
 
+/// Doris configuration model with all system settings
 #[derive(Debug, Clone)]
 pub struct DorisConfig {
     pub environment: Environment,
@@ -159,8 +160,16 @@ fn clean_process_info(config: &mut DorisConfig) {
     config.be_install_dir = None;
 }
 
-fn update_mixed_deployment(config: &mut DorisConfig) -> Result<()> {
+/// Update mixed deployment detection and environment setting
+fn update_mixed_environment(config: &mut DorisConfig) -> Result<()> {
+    // Detect if both FE and BE processes are running
     process_detector::detect_mixed_deployment(config)?;
+
+    // Update environment to Mixed if both FE and BE processes are detected
+    if config.fe_process_pid.is_some() && config.be_process_pid.is_some() {
+        config.environment = Environment::Mixed;
+    }
+
     Ok(())
 }
 
@@ -170,13 +179,48 @@ fn persist_configuration(config: &DorisConfig) {
     }
 }
 
-/// Update environment to Mixed if both FE and BE processes are detected
-fn update_environment_for_mixed_deployment(config: &mut DorisConfig) {
-    if config.fe_process_pid.is_some() && config.be_process_pid.is_some() {
-        config.environment = Environment::Mixed;
+/// Apply environment-specific port configurations
+fn apply_environment_specific_ports(
+    config: &mut DorisConfig,
+    parsed_config: &DorisConfig,
+    env: Environment,
+) {
+    match env {
+        Environment::BE => {
+            apply_be_ports(config, parsed_config);
+        }
+        Environment::FE => {
+            apply_fe_ports(config, parsed_config);
+        }
+        Environment::Mixed => {
+            // For Mixed environment, apply both BE and FE configurations
+            apply_be_ports(config, parsed_config);
+            apply_fe_ports(config, parsed_config);
+        }
+        Environment::Unknown => {
+            // No specific ports to apply for unknown environment
+        }
     }
 }
 
+/// Apply BE-specific port configurations
+fn apply_be_ports(config: &mut DorisConfig, parsed_config: &DorisConfig) {
+    config.be_port = parsed_config.be_port;
+    config.brpc_port = parsed_config.brpc_port;
+    config.webserver_port = parsed_config.webserver_port;
+    config.heartbeat_service_port = parsed_config.heartbeat_service_port;
+}
+
+/// Apply FE-specific port configurations
+fn apply_fe_ports(config: &mut DorisConfig, parsed_config: &DorisConfig) {
+    config.http_port = parsed_config.http_port;
+    config.rpc_port = parsed_config.rpc_port;
+    config.query_port = parsed_config.query_port;
+    config.edit_log_port = parsed_config.edit_log_port;
+    config.cloud_http_port = parsed_config.cloud_http_port;
+    config.meta_dir = parsed_config.meta_dir.clone();
+}
+
 /// Load configuration, first from persisted file, then detect environment and generate if needed
 pub fn load_config() -> Result<DorisConfig> {
     let mut config = config_persister::load_persisted_config().unwrap_or_default();
@@ -185,14 +229,10 @@ pub fn load_config() -> Result<DorisConfig> {
         Ok(current_process) => {
             if needs_config_update(&config, &current_process) {
                 config = update_config_from_process(config, current_process)?;
-
-                let _ = update_mixed_deployment(&mut config);
-                update_environment_for_mixed_deployment(&mut config);
-
+                let _ = update_mixed_environment(&mut config);
                 persist_configuration(&config);
             } else {
-                let _ = update_mixed_deployment(&mut config);
-                update_environment_for_mixed_deployment(&mut config);
+                let _ = update_mixed_environment(&mut config);
             }
         }
         Err(_) => {
@@ -215,7 +255,7 @@ fn parse_env_specific_config(env: Environment) -> DorisConfig {
     let result = match env {
         Environment::BE => config_parser::parse_be_config(),
         Environment::FE => config_parser::parse_fe_config(),
-        Environment::Mixed => config_parser::parse_be_config(), // For Mixed, we'll start with BE config and add FE later
+        Environment::Mixed => config_parser::parse_be_config(),
         Environment::Unknown => return DorisConfig::default(),
     };
     result.unwrap_or_else(|_| DorisConfig::default())
@@ -235,8 +275,7 @@ fn fallback_load_config() -> Result<DorisConfig> {
 
     // If we detect both FE and BE processes, update to Mixed environment
     if env != Environment::Unknown {
-        let _ = update_mixed_deployment(&mut config);
-        update_environment_for_mixed_deployment(&mut config);
+        let _ = update_mixed_environment(&mut config);
     }
 
     persist_configuration(&config);
@@ -253,11 +292,6 @@ pub fn to_app_config(doris_config: DorisConfig) -> crate::config::Config {
     }
 }
 
-/// Get the current Doris configuration
-pub fn get_current_config() -> Result<DorisConfig> {
-    load_config()
-}
-
 /// Get current process PID from configuration (convenience function)
 pub fn get_current_pid() -> Option<u32> {
     load_config().ok()?.get_valid_pid()
@@ -299,38 +333,8 @@ fn update_config_from_process(
     if let Ok(parsed_config) =
         config_parser::parse_config_from_path(process.environment, &process.doris_home)
     {
-        match process.environment {
-            Environment::BE => {
-                config.be_port = parsed_config.be_port;
-                config.brpc_port = parsed_config.brpc_port;
-                config.webserver_port = parsed_config.webserver_port;
-                config.heartbeat_service_port = parsed_config.heartbeat_service_port;
-            }
-            Environment::FE => {
-                config.http_port = parsed_config.http_port;
-                config.rpc_port = parsed_config.rpc_port;
-                config.query_port = parsed_config.query_port;
-                config.edit_log_port = parsed_config.edit_log_port;
-                config.cloud_http_port = parsed_config.cloud_http_port;
-                config.meta_dir = parsed_config.meta_dir;
-            }
-            Environment::Mixed => {
-                // For Mixed environment, we'll update both BE and FE ports
-                config.be_port = parsed_config.be_port;
-                config.brpc_port = parsed_config.brpc_port;
-                config.webserver_port = parsed_config.webserver_port;
-                config.heartbeat_service_port = parsed_config.heartbeat_service_port;
-                config.http_port = parsed_config.http_port;
-                config.rpc_port = parsed_config.rpc_port;
-                config.query_port = parsed_config.query_port;
-                config.edit_log_port = parsed_config.edit_log_port;
-                config.cloud_http_port = parsed_config.cloud_http_port;
-                config.meta_dir = parsed_config.meta_dir;
-            }
-            Environment::Unknown => {
-                // This shouldn't happen as detect_current_process only returns FE/BE
-            }
-        }
+        // Apply port configurations based on environment
+        apply_environment_specific_ports(&mut config, &parsed_config, process.environment);
     }
 
     Ok(config)

src/config_loader/process_detector.rs

@@ -207,15 +207,6 @@ fn get_paths_by_pid(pid: u32) -> (PathBuf, PathBuf) {
     (PathBuf::from("/opt/selectdb"), PathBuf::from("/opt/jdk"))
 }
 
-/// Check if a process with given PID is still running
-pub fn is_process_alive(pid: u32) -> bool {
-    Command::new("kill")
-        .args(["-0", &pid.to_string()])
-        .output()
-        .map(|output| output.status.success())
-        .unwrap_or(false)
-}
-
 /// Verify that a config file exists
 pub fn verify_config_file(path: &Path) -> Result<()> {
     if !path.exists() {
@@ -286,31 +277,3 @@ pub fn detect_mixed_deployment(config: &mut crate::config_loader::DorisConfig) -
 
     Ok(is_mixed)
 }
-
-/// Get config path for the specified environment
-pub fn get_config_path(env: Environment) -> Result<PathBuf> {
-    let (install_path, _) = get_paths(env)?;
-
-    let config_file = match env {
-        Environment::BE => "be.conf",
-        Environment::FE => "fe.conf",
-        Environment::Mixed => {
-            return Err(CliError::ConfigError(
-                "Cannot get config path for Mixed environment".to_string(),
-            ));
-        }
-        _ => return Err(CliError::ConfigError("Invalid environment".to_string())),
-    };
-
-    Ok(install_path.join("conf").join(config_file))
-}
-
-/// Get BE config path
-pub fn get_be_config_path() -> Result<PathBuf> {
-    get_config_path(Environment::BE)
-}
-
-/// Get FE config path
-pub fn get_fe_config_path() -> Result<PathBuf> {
-    get_config_path(Environment::FE)
-}

src/lib.rs

@@ -218,7 +218,7 @@ fn handle_tool_execution_error(config: &Config, error: &error::CliError) -> Resu
 
 /// Persist updated configuration to disk
 fn persist_updated_config(config: &Config) -> Result<()> {
-    let mut doris_config = config_loader::get_current_config()?;
+    let mut doris_config = config_loader::load_config()?;
     doris_config = doris_config.with_app_config(config);
     match config_loader::config_persister::persist_config(&doris_config) {
         Ok(_) => Ok(()),

src/main.rs

@@ -5,5 +5,10 @@ fn main() -> Result<()> {
     ui::print_header();
     let doris_config = config_loader::load_config()?;
     let config = config_loader::to_app_config(doris_config);
+
+    if let Err(e) = config.validate() {
+        ui::print_error(&format!("Config warning: {e}"));
+    }
+
     run_cli(config)
 }