diff --git a/src-tauri/src/api_client.rs b/src-tauri/src/api_client.rs index 7c0f2f2..ed304b5 100644 --- a/src-tauri/src/api_client.rs +++ b/src-tauri/src/api_client.rs @@ -221,6 +221,20 @@ pub fn sort_versions(versions: &mut [String]) { }); } +// Helper function to compare two versions +pub fn compare_versions(version1: &str, version2: &str) -> std::cmp::Ordering { + let version_a = VersionComponent::parse(version1); + let version_b = VersionComponent::parse(version2); + version_a.cmp(&version_b) +} + +pub fn is_version_newer(version1: &str, version2: &str) -> bool { + // Use the proper VersionComponent comparison from api_client.rs + let version_a = VersionComponent::parse(version1); + let version_b = VersionComponent::parse(version2); + version_a > version_b +} + // Helper function to sort GitHub releases pub fn sort_github_releases(releases: &mut [GithubRelease]) { releases.sort_by(|a, b| { diff --git a/src-tauri/src/auto_updater.rs b/src-tauri/src/auto_updater.rs index 4924a84..264a146 100644 --- a/src-tauri/src/auto_updater.rs +++ b/src-tauri/src/auto_updater.rs @@ -411,17 +411,11 @@ impl AutoUpdater { } fn is_version_newer(&self, version1: &str, version2: &str) -> bool { - // Use the proper VersionComponent comparison from api_client.rs - let version_a = crate::api_client::VersionComponent::parse(version1); - let version_b = crate::api_client::VersionComponent::parse(version2); - version_a > version_b + crate::api_client::is_version_newer(version1, version2) } fn compare_versions(&self, version1: &str, version2: &str) -> std::cmp::Ordering { - // Use the proper VersionComponent comparison from api_client.rs - let version_a = crate::api_client::VersionComponent::parse(version1); - let version_b = crate::api_client::VersionComponent::parse(version2); - version_a.cmp(&version_b) + crate::api_client::compare_versions(version1, version2) } fn get_auto_update_state_file(&self) -> PathBuf { diff --git a/src-tauri/src/browser_runner.rs b/src-tauri/src/browser_runner.rs index 5e80397..5daaa47 100644 --- a/src-tauri/src/browser_runner.rs +++ b/src-tauri/src/browser_runner.rs @@ -1200,6 +1200,18 @@ impl BrowserRunner { "Camoufox process cleanup completed for profile: {} (ID: {})", profile.name, profile.id ); + + // Consolidate browser versions after stopping a browser + let registry = DownloadedBrowsersRegistry::instance(); + if let Ok(consolidated) = registry.consolidate_browser_versions(&app_handle, self) { + if !consolidated.is_empty() { + println!("Post-stop version consolidation results:"); + for action in &consolidated { + println!(" {action}"); + } + } + } + return Ok(()); } @@ -1413,6 +1425,17 @@ impl BrowserRunner { ); } + // Consolidate browser versions after stopping a browser + let registry = DownloadedBrowsersRegistry::instance(); + if let Ok(consolidated) = registry.consolidate_browser_versions(&app_handle, self) { + if !consolidated.is_empty() { + println!("Post-stop version consolidation results:"); + for action in &consolidated { + println!(" {action}"); + } + } + } + Ok(()) } @@ -1609,6 +1632,16 @@ impl BrowserRunner { } } + // Consolidate browser versions - keep only latest version per browser + if let Ok(consolidated) = registry.consolidate_browser_versions(app_handle, self) { + if !consolidated.is_empty() { + println!("Version consolidation results:"); + for action in &consolidated { + println!(" {action}"); + } + } + } + let missing_binaries = self.check_missing_binaries().await?; let mut downloaded = Vec::new(); @@ -1724,11 +1757,8 @@ impl BrowserRunner { create_dir_all(&browser_dir).map_err(|e| format!("Failed to create browser directory: {e}"))?; - // Mark download as started in registry + // Mark download as started (but don't add to registry yet) registry.mark_download_started(&browser_str, &version, browser_dir.clone()); - registry - .save() - .map_err(|e| format!("Failed to save registry: {e}"))?; // Use the download module let downloader = crate::download::Downloader::instance(); @@ -1873,8 +1903,8 @@ impl BrowserRunner { return Err(error_details.into()); } - // Mark completion in registry. If it fails (e.g., rare race during cleanup), log but continue. - if let Err(e) = registry.mark_download_completed(&browser_str, &version) { + // Mark completion in registry - only now add to registry after verification + if let Err(e) = registry.mark_download_completed(&browser_str, &version, browser_dir.clone()) { eprintln!("Warning: Could not mark {browser_str} {version} as completed in registry: {e}"); } registry diff --git a/src-tauri/src/downloaded_browsers.rs b/src-tauri/src/downloaded_browsers.rs index 74ed6f3..08fd5cf 100644 --- a/src-tauri/src/downloaded_browsers.rs +++ b/src-tauri/src/downloaded_browsers.rs @@ -107,26 +107,31 @@ impl DownloadedBrowsersRegistry { } pub fn mark_download_started(&self, browser: &str, version: &str, file_path: PathBuf) { + // Only mark download started, don't add to registry yet + // The browser will be added to registry only after verification succeeds + println!( + "Marking download started for {}:{} at {}", + browser, + version, + file_path.display() + ); + } + + pub fn mark_download_completed( + &self, + browser: &str, + version: &str, + file_path: PathBuf, + ) -> Result<(), String> { + // Only mark as completed after verification succeeds let info = DownloadedBrowserInfo { browser: browser.to_string(), version: version.to_string(), file_path, }; self.add_browser(info); - } - - pub fn mark_download_completed(&self, browser: &str, version: &str) -> Result<(), String> { - let data = self.data.lock().unwrap(); - if data - .browsers - .get(browser) - .and_then(|versions| versions.get(version)) - .is_some() - { - Ok(()) - } else { - Err(format!("Browser {browser}:{version} not found in registry")) - } + println!("Browser {browser}:{version} successfully added to registry after verification"); + Ok(()) } pub fn cleanup_failed_download( @@ -590,6 +595,118 @@ impl DownloadedBrowsersRegistry { Ok(cleaned_up) } + /// Consolidate browser versions - keep only the latest version per browser + pub fn consolidate_browser_versions( + &self, + app_handle: &tauri::AppHandle, + browser_runner: &crate::browser_runner::BrowserRunner, + ) -> Result, Box> { + println!("Starting browser version consolidation..."); + + let profiles = browser_runner + .list_profiles() + .map_err(|e| format!("Failed to list profiles: {e}"))?; + + let binaries_dir = browser_runner.get_binaries_dir(); + let mut consolidated = Vec::new(); + + // Group profiles by browser + let mut browser_profiles: std::collections::HashMap> = + std::collections::HashMap::new(); + for profile in &profiles { + browser_profiles + .entry(profile.browser.clone()) + .or_default() + .push(profile); + } + + for (browser_name, browser_profiles) in browser_profiles { + // Find the latest version among all profiles for this browser + let mut versions: Vec<&str> = browser_profiles + .iter() + .map(|p| p.version.as_str()) + .collect(); + versions.sort_by(|a, b| { + // Sort versions using semantic versioning logic + crate::api_client::compare_versions(b, a) + }); + + if let Some(&latest_version) = versions.first() { + println!("Latest version for {browser_name}: {latest_version}"); + + // Check which profiles need to be updated to the latest version + let mut profiles_to_update = Vec::new(); + let mut older_versions_to_remove = std::collections::HashSet::new(); + + for profile in &browser_profiles { + if profile.version != latest_version { + // Only update if profile is not currently running + if profile.process_id.is_none() { + profiles_to_update.push(profile); + older_versions_to_remove.insert(profile.version.clone()); + } else { + println!( + "Skipping version update for running profile: {} ({})", + profile.name, profile.version + ); + } + } + } + + // Update profiles to latest version + for profile in profiles_to_update { + match browser_runner.update_profile_version( + app_handle, + &profile.id.to_string(), + latest_version, + ) { + Ok(_) => { + consolidated.push(format!( + "Updated profile '{}' from {} to {}", + profile.name, profile.version, latest_version + )); + } + Err(e) => { + eprintln!("Failed to update profile '{}': {}", profile.name, e); + } + } + } + + // Remove older version binaries that are no longer needed + for old_version in older_versions_to_remove { + let old_version_dir = binaries_dir.join(&browser_name).join(&old_version); + if old_version_dir.exists() { + match std::fs::remove_dir_all(&old_version_dir) { + Ok(_) => { + consolidated.push(format!("Removed old version: {browser_name} {old_version}")); + // Also remove from registry + self.remove_browser(&browser_name, &old_version); + } + Err(e) => { + eprintln!( + "Failed to remove old version directory {}: {}", + old_version_dir.display(), + e + ); + } + } + } + } + } + } + + // Save registry after consolidation + self + .save() + .map_err(|e| format!("Failed to save registry after consolidation: {e}"))?; + + println!( + "Browser version consolidation completed: {} actions taken", + consolidated.len() + ); + Ok(consolidated) + } + /// Simplified version of verify_and_cleanup_stale_entries that doesn't need BrowserRunner pub fn verify_and_cleanup_stale_entries_simple( &self, @@ -702,21 +819,21 @@ mod tests { // Mark download started registry.mark_download_started("firefox", "139.0", PathBuf::from("/test/path")); - // Should be considered downloaded immediately + // Should NOT be considered downloaded until verification completes assert!( - registry.is_browser_downloaded("firefox", "139.0"), - "Browser should be considered downloaded after marking as started" + !registry.is_browser_downloaded("firefox", "139.0"), + "Browser should NOT be considered downloaded after marking as started (only after verification)" ); - // Mark as completed + // Mark as completed (after verification) registry - .mark_download_completed("firefox", "139.0") + .mark_download_completed("firefox", "139.0", PathBuf::from("/test/path")) .expect("Failed to mark download as completed"); - // Should still be considered downloaded + // Should now be considered downloaded assert!( registry.is_browser_downloaded("firefox", "139.0"), - "Browser should still be considered downloaded after completion" + "Browser should be considered downloaded after verification completes" ); } @@ -753,10 +870,20 @@ mod tests { // Mark twilight download started registry.mark_download_started("zen", "twilight", PathBuf::from("/test/zen-twilight")); - // Check that it's registered + // Should NOT be registered until verification completes + assert!( + !registry.is_browser_downloaded("zen", "twilight"), + "Zen twilight version should NOT be registered until verification completes" + ); + + // Mark as completed (after verification) + registry.mark_download_completed("zen", "twilight", PathBuf::from("/test/zen-twilight")) + .expect("Failed to mark twilight download as completed"); + + // Now it should be registered assert!( registry.is_browser_downloaded("zen", "twilight"), - "Zen twilight version should be registered as downloaded" + "Zen twilight version should be registered after verification completes" ); } }