From d7ddf3710f6aff40d0595430f5f49255c89fdca1 Mon Sep 17 00:00:00 2001 From: "Begonia, HE" <163421589+BegoniaHe@users.noreply.github.com> Date: Sun, 25 Jan 2026 04:52:35 +0100 Subject: refactor(java): modularize Java detection and management system - Split monolithic java.rs (1089 lines) into focused modules - detection: Java installation discovery - validation: Version validation and requirements checking - priority: Installation selection priority logic - provider: Java download provider trait - providers: Provider implementations (Adoptium) - persistence: Cache and catalog management - Add java_path_override field to Instance struct for per-instance Java configuration - Export JavaInstallation at core module level This refactoring improves maintainability and prepares for multi-vendor Java provider support. Reviewed-by: Claude Sonnet 4.5 --- src-tauri/src/core/java/persistence.rs | 82 ++++++++++++++++++++++++++++++++++ 1 file changed, 82 insertions(+) create mode 100644 src-tauri/src/core/java/persistence.rs (limited to 'src-tauri/src/core/java/persistence.rs') diff --git a/src-tauri/src/core/java/persistence.rs b/src-tauri/src/core/java/persistence.rs new file mode 100644 index 0000000..0932f2e --- /dev/null +++ b/src-tauri/src/core/java/persistence.rs @@ -0,0 +1,82 @@ +use serde::{Deserialize, Serialize}; +use std::path::PathBuf; +use tauri::{AppHandle, Manager}; + +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct JavaConfig { + pub user_defined_paths: Vec, + pub preferred_java_path: Option, + pub last_detection_time: u64, +} + +impl Default for JavaConfig { + fn default() -> Self { + Self { + user_defined_paths: Vec::new(), + preferred_java_path: None, + last_detection_time: 0, + } + } +} + +fn get_java_config_path(app_handle: &AppHandle) -> PathBuf { + app_handle + .path() + .app_data_dir() + .unwrap() + .join("java_config.json") +} + +pub fn load_java_config(app_handle: &AppHandle) -> JavaConfig { + let config_path = get_java_config_path(app_handle); + if !config_path.exists() { + return JavaConfig::default(); + } + + match std::fs::read_to_string(&config_path) { + Ok(content) => serde_json::from_str(&content).unwrap_or_default(), + Err(_) => JavaConfig::default(), + } +} + +pub fn save_java_config(app_handle: &AppHandle, config: &JavaConfig) -> Result<(), String> { + let config_path = get_java_config_path(app_handle); + let content = serde_json::to_string_pretty(config).map_err(|e| e.to_string())?; + std::fs::create_dir_all(config_path.parent().unwrap()).map_err(|e| e.to_string())?; + std::fs::write(&config_path, content).map_err(|e| e.to_string())?; + Ok(()) +} + +pub fn add_user_defined_path(app_handle: &AppHandle, path: String) -> Result<(), String> { + let mut config = load_java_config(app_handle); + if !config.user_defined_paths.contains(&path) { + config.user_defined_paths.push(path); + } + save_java_config(app_handle, &config) +} + +pub fn remove_user_defined_path(app_handle: &AppHandle, path: &str) -> Result<(), String> { + let mut config = load_java_config(app_handle); + config.user_defined_paths.retain(|p| p != path); + save_java_config(app_handle, &config) +} + +pub fn set_preferred_java_path(app_handle: &AppHandle, path: Option) -> Result<(), String> { + let mut config = load_java_config(app_handle); + config.preferred_java_path = path; + save_java_config(app_handle, &config) +} + +pub fn get_preferred_java_path(app_handle: &AppHandle) -> Option { + let config = load_java_config(app_handle); + config.preferred_java_path +} + +pub fn update_last_detection_time(app_handle: &AppHandle) -> Result<(), String> { + let mut config = load_java_config(app_handle); + config.last_detection_time = std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .unwrap() + .as_secs(); + save_java_config(app_handle, &config) +} -- cgit v1.2.3-70-g09d2 From aba94d55f00c4241c12f5d7ccd6e87c5955a3fd5 Mon Sep 17 00:00:00 2001 From: "Begonia, HE" <163421589+BegoniaHe@users.noreply.github.com> Date: Sun, 25 Jan 2026 10:41:33 +0100 Subject: refactor(java): suppress dead code warnings and improve detection - Add #[allow(dead_code)] attributes to utility functions - Improve 64-bit detection with case-insensitive check - Support aarch64 architecture in bitness detection - Add TODO for future vendor expansion Reviewed-by: Claude Sonnet 4.5 --- src-tauri/src/core/java/persistence.rs | 5 +++++ src-tauri/src/core/java/priority.rs | 2 ++ src-tauri/src/core/java/provider.rs | 9 +-------- src-tauri/src/core/java/validation.rs | 19 +++++++++++-------- 4 files changed, 19 insertions(+), 16 deletions(-) (limited to 'src-tauri/src/core/java/persistence.rs') diff --git a/src-tauri/src/core/java/persistence.rs b/src-tauri/src/core/java/persistence.rs index 0932f2e..5e263fb 100644 --- a/src-tauri/src/core/java/persistence.rs +++ b/src-tauri/src/core/java/persistence.rs @@ -47,6 +47,7 @@ pub fn save_java_config(app_handle: &AppHandle, config: &JavaConfig) -> Result<( Ok(()) } +#[allow(dead_code)] pub fn add_user_defined_path(app_handle: &AppHandle, path: String) -> Result<(), String> { let mut config = load_java_config(app_handle); if !config.user_defined_paths.contains(&path) { @@ -55,23 +56,27 @@ pub fn add_user_defined_path(app_handle: &AppHandle, path: String) -> Result<(), save_java_config(app_handle, &config) } +#[allow(dead_code)] pub fn remove_user_defined_path(app_handle: &AppHandle, path: &str) -> Result<(), String> { let mut config = load_java_config(app_handle); config.user_defined_paths.retain(|p| p != path); save_java_config(app_handle, &config) } +#[allow(dead_code)] pub fn set_preferred_java_path(app_handle: &AppHandle, path: Option) -> Result<(), String> { let mut config = load_java_config(app_handle); config.preferred_java_path = path; save_java_config(app_handle, &config) } +#[allow(dead_code)] pub fn get_preferred_java_path(app_handle: &AppHandle) -> Option { let config = load_java_config(app_handle); config.preferred_java_path } +#[allow(dead_code)] pub fn update_last_detection_time(app_handle: &AppHandle) -> Result<(), String> { let mut config = load_java_config(app_handle); config.last_detection_time = std::time::SystemTime::now() diff --git a/src-tauri/src/core/java/priority.rs b/src-tauri/src/core/java/priority.rs index cf39fdd..98f8b0e 100644 --- a/src-tauri/src/core/java/priority.rs +++ b/src-tauri/src/core/java/priority.rs @@ -4,6 +4,7 @@ use super::JavaInstallation; use crate::core::java::persistence; use crate::core::java::validation; +#[allow(dead_code)] pub async fn resolve_java_for_launch( app_handle: &AppHandle, instance_java_override: Option<&str>, @@ -49,6 +50,7 @@ pub async fn resolve_java_for_launch( .find(|java| is_version_compatible(java, required_major_version, max_major_version)) } +#[allow(dead_code)] fn is_version_compatible( java: &JavaInstallation, required_major_version: Option, diff --git a/src-tauri/src/core/java/provider.rs b/src-tauri/src/core/java/provider.rs index 0f9d78a..1b79681 100644 --- a/src-tauri/src/core/java/provider.rs +++ b/src-tauri/src/core/java/provider.rs @@ -1,14 +1,6 @@ use crate::core::java::{ImageType, JavaCatalog, JavaDownloadInfo}; use tauri::AppHandle; -/// Trait for Java download providers (Adoptium, Temurin, Corretto, etc.) -/// -/// Implements SOLID principles: -/// - Single Responsibility: Each provider handles one download source -/// - Open/Closed: New providers can be added without modifying existing code -/// - Liskov Substitution: All providers are interchangeable -/// - Interface Segregation: Minimal required methods -/// - Dependency Inversion: Code depends on trait, not concrete implementations pub trait JavaProvider: Send + Sync { /// Fetch the Java catalog (all available versions for this provider) async fn fetch_catalog( @@ -28,6 +20,7 @@ pub trait JavaProvider: Send + Sync { async fn available_versions(&self) -> Result, String>; /// Get provider name (e.g., "adoptium", "corretto") + #[allow(dead_code)] fn provider_name(&self) -> &'static str; /// Get OS name for this provider's API diff --git a/src-tauri/src/core/java/validation.rs b/src-tauri/src/core/java/validation.rs index 8eca58a..cfe6f14 100644 --- a/src-tauri/src/core/java/validation.rs +++ b/src-tauri/src/core/java/validation.rs @@ -16,19 +16,21 @@ pub async fn check_java_installation(path: &PathBuf) -> Option } fn check_java_installation_blocking(path: &PathBuf) -> Option { - let mut cmd = Command::new(path); - cmd.arg("-version"); - #[cfg(target_os = "windows")] - cmd.creation_flags(0x08000000); + let mut cmd = Command::new(path); + cmd.arg("-version"); + + // Hide console window + #[cfg(target_os = "windows")] + cmd.creation_flags(0x08000000); let output = cmd.output().ok()?; let version_output = String::from_utf8_lossy(&output.stderr); - let version = parse_version_string(&version_output)?; - let arch = extract_architecture(&version_output); - let vendor = extract_vendor(&version_output); - let is_64bit = version_output.contains("64-Bit"); + let version = parse_version_string(&version_output)?; + let arch = extract_architecture(&version_output); + let vendor = extract_vendor(&version_output); + let is_64bit = version_output.to_lowercase().contains("64-bit") || arch == "aarch64"; Some(JavaInstallation { path: path.to_string_lossy().to_string(), @@ -84,6 +86,7 @@ pub fn extract_architecture(version_output: &str) -> String { pub fn extract_vendor(version_output: &str) -> String { let lower = version_output.to_lowercase(); + // TODO: Expand with more vendors as needed if lower.contains("temurin") || lower.contains("adoptium") { "Eclipse Adoptium".to_string() } else if lower.contains("openjdk") { -- cgit v1.2.3-70-g09d2 From 2c90c392114a8948190e4253f0cae9379f3a614d Mon Sep 17 00:00:00 2001 From: "Begonia, HE" <163421589+BegoniaHe@users.noreply.github.com> Date: Sun, 25 Jan 2026 10:46:30 +0100 Subject: refactor(java): replace unwrap with expect for better error handling Replace potentially panicking unwrap() call with expect() that includes a descriptive error message to aid debugging if the edge case occurs. Reviewed-by: Claude Sonnet 4.5 --- src-tauri/src/core/java/persistence.rs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) (limited to 'src-tauri/src/core/java/persistence.rs') diff --git a/src-tauri/src/core/java/persistence.rs b/src-tauri/src/core/java/persistence.rs index 5e263fb..d1e999e 100644 --- a/src-tauri/src/core/java/persistence.rs +++ b/src-tauri/src/core/java/persistence.rs @@ -42,7 +42,12 @@ pub fn load_java_config(app_handle: &AppHandle) -> JavaConfig { pub fn save_java_config(app_handle: &AppHandle, config: &JavaConfig) -> Result<(), String> { let config_path = get_java_config_path(app_handle); let content = serde_json::to_string_pretty(config).map_err(|e| e.to_string())?; - std::fs::create_dir_all(config_path.parent().unwrap()).map_err(|e| e.to_string())?; + std::fs::create_dir_all( + config_path + .parent() + .expect("Java config path should have a parent directory"), + ) + .map_err(|e| e.to_string())?; std::fs::write(&config_path, content).map_err(|e| e.to_string())?; Ok(()) } -- cgit v1.2.3-70-g09d2 From c46d6c51b8bec6a52ca66087ef9b8edc48d809a3 Mon Sep 17 00:00:00 2001 From: "Begonia, HE" <163421589+BegoniaHe@users.noreply.github.com> Date: Thu, 29 Jan 2026 02:34:16 +0100 Subject: refactor(java): improve error handling and logging - Extract JavaError to dedicated error.rs module - Add serde defaults for JavaInstallation optional fields - Replace unwrap() with proper error propagation - Add detailed logging for Java resolution priority chain - Improve error mapping in validation (NotFound vs VerificationFailed) Reviewed-by: Claude Sonnet 4.5 --- src-tauri/src/core/java/error.rs | 95 +++++++++++++++++++++++++++ src-tauri/src/core/java/persistence.rs | 54 ++++++++++----- src-tauri/src/core/java/providers/adoptium.rs | 8 ++- src-tauri/src/core/mod.rs | 2 - src-tauri/src/main.rs | 1 + 5 files changed, 140 insertions(+), 20 deletions(-) create mode 100644 src-tauri/src/core/java/error.rs (limited to 'src-tauri/src/core/java/persistence.rs') diff --git a/src-tauri/src/core/java/error.rs b/src-tauri/src/core/java/error.rs new file mode 100644 index 0000000..bf78d3b --- /dev/null +++ b/src-tauri/src/core/java/error.rs @@ -0,0 +1,95 @@ +use std::fmt; + +/// Unified error type for Java component operations +/// +/// This enum represents all possible errors that can occur in the Java component, +/// providing a consistent error handling interface across all modules. +#[derive(Debug, Clone)] +pub enum JavaError { + // Java installation not found at the specified path + NotFound, + // Invalid Java version format or unable to parse version + InvalidVersion(String), + // Java installation verification failed (e.g., -version command failed) + VerificationFailed(String), + // Network error during API calls or downloads + NetworkError(String), + // File I/O error (reading, writing, or accessing files) + IoError(String), + // Timeout occurred during operation + Timeout(String), + // Serialization/deserialization error + SerializationError(String), + // Invalid configuration or parameters + InvalidConfig(String), + // Download or installation failed + DownloadFailed(String), + // Extraction or decompression failed + ExtractionFailed(String), + // Checksum verification failed + ChecksumMismatch(String), + // Other unspecified errors + Other(String), +} + +impl fmt::Display for JavaError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + JavaError::NotFound => write!(f, "Java installation not found"), + JavaError::InvalidVersion(msg) => write!(f, "Invalid Java version: {}", msg), + JavaError::VerificationFailed(msg) => write!(f, "Java verification failed: {}", msg), + JavaError::NetworkError(msg) => write!(f, "Network error: {}", msg), + JavaError::IoError(msg) => write!(f, "I/O error: {}", msg), + JavaError::Timeout(msg) => write!(f, "Operation timeout: {}", msg), + JavaError::SerializationError(msg) => write!(f, "Serialization error: {}", msg), + JavaError::InvalidConfig(msg) => write!(f, "Invalid configuration: {}", msg), + JavaError::DownloadFailed(msg) => write!(f, "Download failed: {}", msg), + JavaError::ExtractionFailed(msg) => write!(f, "Extraction failed: {}", msg), + JavaError::ChecksumMismatch(msg) => write!(f, "Checksum mismatch: {}", msg), + JavaError::Other(msg) => write!(f, "{}", msg), + } + } +} + +impl std::error::Error for JavaError {} + +/// Convert JavaError to String for Tauri command results +impl From for String { + fn from(err: JavaError) -> Self { + err.to_string() + } +} + +/// Convert std::io::Error to JavaError +impl From for JavaError { + fn from(err: std::io::Error) -> Self { + JavaError::IoError(err.to_string()) + } +} + +/// Convert serde_json::Error to JavaError +impl From for JavaError { + fn from(err: serde_json::Error) -> Self { + JavaError::SerializationError(err.to_string()) + } +} + +/// Convert reqwest::Error to JavaError +impl From for JavaError { + fn from(err: reqwest::Error) -> Self { + if err.is_timeout() { + JavaError::Timeout(err.to_string()) + } else if err.is_connect() || err.is_request() { + JavaError::NetworkError(err.to_string()) + } else { + JavaError::NetworkError(err.to_string()) + } + } +} + +/// Convert String to JavaError +impl From for JavaError { + fn from(err: String) -> Self { + JavaError::Other(err) + } +} diff --git a/src-tauri/src/core/java/persistence.rs b/src-tauri/src/core/java/persistence.rs index d1e999e..fd81394 100644 --- a/src-tauri/src/core/java/persistence.rs +++ b/src-tauri/src/core/java/persistence.rs @@ -2,6 +2,8 @@ use serde::{Deserialize, Serialize}; use std::path::PathBuf; use tauri::{AppHandle, Manager}; +use super::error::JavaError; + #[derive(Debug, Clone, Serialize, Deserialize)] pub struct JavaConfig { pub user_defined_paths: Vec, @@ -34,26 +36,43 @@ pub fn load_java_config(app_handle: &AppHandle) -> JavaConfig { } match std::fs::read_to_string(&config_path) { - Ok(content) => serde_json::from_str(&content).unwrap_or_default(), - Err(_) => JavaConfig::default(), + Ok(content) => match serde_json::from_str(&content) { + Ok(config) => config, + Err(err) => { + // Log the error but don't panic - return default config + log::warn!( + "Failed to parse Java config at {}: {}. Using default configuration.", + config_path.display(), + err + ); + JavaConfig::default() + } + }, + Err(err) => { + log::warn!( + "Failed to read Java config at {}: {}. Using default configuration.", + config_path.display(), + err + ); + JavaConfig::default() + } } } -pub fn save_java_config(app_handle: &AppHandle, config: &JavaConfig) -> Result<(), String> { +pub fn save_java_config(app_handle: &AppHandle, config: &JavaConfig) -> Result<(), JavaError> { let config_path = get_java_config_path(app_handle); - let content = serde_json::to_string_pretty(config).map_err(|e| e.to_string())?; - std::fs::create_dir_all( - config_path - .parent() - .expect("Java config path should have a parent directory"), - ) - .map_err(|e| e.to_string())?; - std::fs::write(&config_path, content).map_err(|e| e.to_string())?; + let content = serde_json::to_string_pretty(config)?; + + std::fs::create_dir_all(config_path.parent().ok_or_else(|| { + JavaError::InvalidConfig("Java config path has no parent directory".to_string()) + })?)?; + + std::fs::write(&config_path, content)?; Ok(()) } #[allow(dead_code)] -pub fn add_user_defined_path(app_handle: &AppHandle, path: String) -> Result<(), String> { +pub fn add_user_defined_path(app_handle: &AppHandle, path: String) -> Result<(), JavaError> { let mut config = load_java_config(app_handle); if !config.user_defined_paths.contains(&path) { config.user_defined_paths.push(path); @@ -62,14 +81,17 @@ pub fn add_user_defined_path(app_handle: &AppHandle, path: String) -> Result<(), } #[allow(dead_code)] -pub fn remove_user_defined_path(app_handle: &AppHandle, path: &str) -> Result<(), String> { +pub fn remove_user_defined_path(app_handle: &AppHandle, path: &str) -> Result<(), JavaError> { let mut config = load_java_config(app_handle); config.user_defined_paths.retain(|p| p != path); save_java_config(app_handle, &config) } #[allow(dead_code)] -pub fn set_preferred_java_path(app_handle: &AppHandle, path: Option) -> Result<(), String> { +pub fn set_preferred_java_path( + app_handle: &AppHandle, + path: Option, +) -> Result<(), JavaError> { let mut config = load_java_config(app_handle); config.preferred_java_path = path; save_java_config(app_handle, &config) @@ -82,11 +104,11 @@ pub fn get_preferred_java_path(app_handle: &AppHandle) -> Option { } #[allow(dead_code)] -pub fn update_last_detection_time(app_handle: &AppHandle) -> Result<(), String> { +pub fn update_last_detection_time(app_handle: &AppHandle) -> Result<(), JavaError> { let mut config = load_java_config(app_handle); config.last_detection_time = std::time::SystemTime::now() .duration_since(std::time::UNIX_EPOCH) - .unwrap() + .map_err(|e| JavaError::Other(format!("System time error: {}", e)))? .as_secs(); save_java_config(app_handle, &config) } diff --git a/src-tauri/src/core/java/providers/adoptium.rs b/src-tauri/src/core/java/providers/adoptium.rs index 4b06721..40e1757 100644 --- a/src-tauri/src/core/java/providers/adoptium.rs +++ b/src-tauri/src/core/java/providers/adoptium.rs @@ -1,5 +1,6 @@ +use crate::core::java::error::JavaError; use crate::core::java::provider::JavaProvider; -use crate::core::java::{ImageType, JavaCatalog, JavaDownloadInfo, JavaError, JavaReleaseInfo}; +use crate::core::java::{ImageType, JavaCatalog, JavaDownloadInfo, JavaReleaseInfo}; use serde::Deserialize; use tauri::AppHandle; @@ -183,7 +184,10 @@ impl JavaProvider for AdoptiumProvider { // Task completed but returned None, should not happen in current implementation } Err(e) => { - eprintln!("AdoptiumProvider::fetch_catalog task join error: {:?}", e); + return Err(JavaError::NetworkError(format!( + "Failed to join Adoptium catalog fetch task: {}", + e + ))); } } } diff --git a/src-tauri/src/core/mod.rs b/src-tauri/src/core/mod.rs index 12dff7c..dcbd47a 100644 --- a/src-tauri/src/core/mod.rs +++ b/src-tauri/src/core/mod.rs @@ -12,5 +12,3 @@ pub mod manifest; pub mod maven; pub mod rules; pub mod version_merge; - -pub use java::JavaInstallation; diff --git a/src-tauri/src/main.rs b/src-tauri/src/main.rs index b74c746..7984ea8 100644 --- a/src-tauri/src/main.rs +++ b/src-tauri/src/main.rs @@ -208,6 +208,7 @@ async fn start_game( let java_installation = core::java::priority::resolve_java_for_launch( app_handle, + &window, instance.java_path_override.as_deref(), Some(&config.java_path), required_java_major, -- cgit v1.2.3-70-g09d2