From ff7c96539b8679d22ed3449f80903e879c23c371 Mon Sep 17 00:00:00 2001 From: Chris O'Neil Date: Wed, 20 Mar 2024 22:27:27 +0000 Subject: [PATCH] refactor: use version type rather than strings Upgrade to `sn-releases` version 0.2.0, which forces the use of the `semver::Version` type rather than `String` for dealing with versions. It made sense to do the same thing with string-based version fields in this crate. All of the fields on the `Settings` type were also turned into an `Option`, which makes sense because it's possible that we don't have one of the asset types installed. I'm not sure why this wasn't done in the first place. It made the code a bit simpler. BREAKING CHANGE: settings files from previous versions will not deserialize properly and will have to be deleted. --- Cargo.toml | 2 +- src/cmd.rs | 63 +++++----- src/install.rs | 310 +++++++++++++++++++++++++++++-------------------- src/update.rs | 228 ++++++++++++++++++------------------ 4 files changed, 326 insertions(+), 277 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index ada37e7..f477289 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -25,7 +25,7 @@ semver = "1.0.4" serde = "1.0" serde_derive = "1.0" serde_json = "1.0" -sn-releases = "0.1.7" +sn-releases = "0.2.0" tempfile = "3.8.1" textwrap = "0.16.0" tokio = { version = "1.26", features = ["full"] } diff --git a/src/cmd.rs b/src/cmd.rs index 3284e7c..2d080fc 100644 --- a/src/cmd.rs +++ b/src/cmd.rs @@ -11,7 +11,8 @@ use crate::update::{perform_update_assessment, UpdateAssessmentResult}; use color_eyre::{eyre::eyre, Result}; use lazy_static::lazy_static; use prettytable::{Cell, Row, Table}; -use sn_releases::SafeReleaseRepositoryInterface; +use semver::Version; +use sn_releases::SafeReleaseRepoActions; use std::collections::HashMap; use std::env::consts::{ARCH, OS}; use std::path::PathBuf; @@ -50,6 +51,11 @@ pub(crate) async fn process_install_cmd( get_default_install_path()? }; + let version = if let Some(version) = version { + Some(Version::parse(&version)?) + } else { + None + }; do_install_binary(&asset_type, dest_dir_path.clone(), version).await?; if !no_modify_shell_profile { @@ -68,7 +74,7 @@ pub(crate) async fn process_update_cmd() -> Result<()> { let safe_config_dir_path = get_safe_config_dir_path()?; let settings_file_path = safe_config_dir_path.join("safeup.json"); let settings = Settings::read(&settings_file_path)?; - let release_repo = ::default_config(); + let release_repo = ::default_config(); for asset_type in AssetType::variants() { println!("Retrieving latest version for {asset_type}..."); @@ -76,18 +82,11 @@ pub(crate) async fn process_update_cmd() -> Result<()> { .get_latest_version(&asset_type.get_release_type()) .await?; println!("Latest version of {asset_type} is {latest_version}"); - if settings.is_installed(&asset_type) { - println!( - "Current version of {asset_type} is {}", - settings.get_installed_version(&asset_type) - ); - } let decision = perform_update_assessment(&asset_type, &latest_version, &settings)?; match decision { - UpdateAssessmentResult::PerformUpdate => { + UpdateAssessmentResult::PerformUpdate(installed_path) => { println!("Updating {asset_type} to {latest_version}..."); - let installed_path = settings.get_install_path(&asset_type).clone(); let installed_dir_path = installed_path .parent() .ok_or_else(|| eyre!("could not retrieve parent directory"))?; @@ -126,19 +125,21 @@ pub(crate) fn process_ls_command() -> Result<()> { Cell::new("Path"), ])); for asset_type in AssetType::variants() { - let installed_path = settings.get_install_path(&asset_type); - let wrapped_install_path = textwrap::wrap( - installed_path - .to_str() - .ok_or_else(|| eyre!("could not obtain install path"))?, - WRAP_LENGTH, - ) - .join("\n"); - table.add_row(Row::new(vec![ - Cell::new(&asset_type.to_string()), - Cell::new(&settings.get_installed_version(&asset_type)), - Cell::new(&wrapped_install_path), - ])); + if let Some((installed_path, installed_version)) = settings.get_install_details(&asset_type) + { + let wrapped_install_path = textwrap::wrap( + installed_path + .to_str() + .ok_or_else(|| eyre!("could not obtain install path"))?, + WRAP_LENGTH, + ) + .join("\n"); + table.add_row(Row::new(vec![ + Cell::new(&asset_type.to_string()), + Cell::new(&installed_version.to_string()), + Cell::new(&wrapped_install_path), + ])); + } } table.printstd(); Ok(()) @@ -147,10 +148,10 @@ pub(crate) fn process_ls_command() -> Result<()> { async fn do_install_binary( asset_type: &AssetType, dest_dir_path: PathBuf, - version: Option, + version: Option, ) -> Result<()> { let platform = get_platform()?; - let release_repo = ::default_config(); + let release_repo = ::default_config(); let (installed_version, bin_path) = crate::install::install_bin( asset_type.clone(), release_repo, @@ -165,16 +166,16 @@ async fn do_install_binary( let mut settings = Settings::read(&settings_file_path)?; match asset_type { AssetType::Client => { - settings.safe_path = bin_path; - settings.safe_version = installed_version; + settings.safe_path = Some(bin_path); + settings.safe_version = Some(installed_version); } AssetType::Node => { - settings.safenode_path = bin_path; - settings.safenode_version = installed_version; + settings.safenode_path = Some(bin_path); + settings.safenode_version = Some(installed_version); } AssetType::NodeManager => { - settings.safenode_manager_path = bin_path; - settings.safenode_manager_version = installed_version; + settings.safenode_manager_path = Some(bin_path); + settings.safenode_manager_version = Some(installed_version); } } settings.save(&settings_file_path)?; diff --git a/src/install.rs b/src/install.rs index 91653ee..dba97d1 100644 --- a/src/install.rs +++ b/src/install.rs @@ -13,9 +13,13 @@ use color_eyre::{eyre::eyre, Result}; use indicatif::{ProgressBar, ProgressStyle}; #[cfg(unix)] use indoc::indoc; +use semver::Version; +use serde::de::Visitor; +use serde::{Deserializer, Serializer}; use serde_derive::{Deserialize, Serialize}; -use sn_releases::{get_running_platform, ArchiveType, ReleaseType, SafeReleaseRepositoryInterface}; +use sn_releases::{get_running_platform, ArchiveType, ReleaseType, SafeReleaseRepoActions}; use std::env::consts::OS; +use std::fmt; use std::fs::{File, OpenOptions}; #[cfg(unix)] use std::io::prelude::*; @@ -78,12 +82,82 @@ impl std::fmt::Display for AssetType { #[derive(Clone, Debug, Serialize, Deserialize)] pub struct Settings { - pub safe_path: PathBuf, - pub safe_version: String, - pub safenode_path: PathBuf, - pub safenode_version: String, - pub safenode_manager_path: PathBuf, - pub safenode_manager_version: String, + pub safe_path: Option, + #[serde( + serialize_with = "serialize_version", + deserialize_with = "deserialize_version" + )] + pub safe_version: Option, + pub safenode_path: Option, + #[serde( + serialize_with = "serialize_version", + deserialize_with = "deserialize_version" + )] + pub safenode_version: Option, + pub safenode_manager_path: Option, + #[serde( + serialize_with = "serialize_version", + deserialize_with = "deserialize_version" + )] + pub safenode_manager_version: Option, +} + +fn serialize_version(version: &Option, serializer: S) -> Result +where + S: Serializer, +{ + match version { + Some(v) => serializer.serialize_some(&v.to_string()), + None => serializer.serialize_none(), + } +} + +fn deserialize_version<'de, D>(deserializer: D) -> Result, D::Error> +where + D: Deserializer<'de>, +{ + deserializer.deserialize_option(VersionOptionVisitor) +} + +struct VersionOptionVisitor; + +impl<'de> Visitor<'de> for VersionOptionVisitor { + type Value = Option; + + fn expecting(&self, formatter: &mut fmt::Formatter) -> fmt::Result { + formatter.write_str("an option of a string representing a semver version") + } + + fn visit_none(self) -> Result + where + E: serde::de::Error, + { + Ok(None) + } + + fn visit_some(self, deserializer: D) -> Result + where + D: Deserializer<'de>, + { + deserializer.deserialize_str(VersionVisitor).map(Some) + } +} + +struct VersionVisitor; + +impl<'de> Visitor<'de> for VersionVisitor { + type Value = Version; + + fn expecting(&self, formatter: &mut fmt::Formatter) -> fmt::Result { + formatter.write_str("a string representing a semver version") + } + + fn visit_str(self, value: &str) -> Result + where + E: serde::de::Error, + { + value.parse().map_err(serde::de::Error::custom) + } } impl Settings { @@ -92,47 +166,54 @@ impl Settings { let mut contents = String::new(); file.read_to_string(&mut contents)?; serde_json::from_str(&contents).unwrap_or_else(|_| Settings { - safe_path: PathBuf::new(), - safe_version: String::new(), - safenode_path: PathBuf::new(), - safenode_version: String::new(), - safenode_manager_path: PathBuf::new(), - safenode_manager_version: String::new(), + safe_path: None, + safe_version: None, + safenode_path: None, + safenode_version: None, + safenode_manager_path: None, + safenode_manager_version: None, }) } else { Settings { - safe_path: PathBuf::new(), - safe_version: String::new(), - safenode_path: PathBuf::new(), - safenode_version: String::new(), - safenode_manager_path: PathBuf::new(), - safenode_manager_version: String::new(), + safe_path: None, + safe_version: None, + safenode_path: None, + safenode_version: None, + safenode_manager_path: None, + safenode_manager_version: None, } }; Ok(settings) } - pub fn get_installed_version(&self, asset_type: &AssetType) -> String { - match asset_type { - AssetType::Client => self.safe_version.clone(), - AssetType::Node => self.safenode_version.clone(), - AssetType::NodeManager => self.safenode_manager_version.clone(), - } - } - - pub fn is_installed(&self, asset_type: &AssetType) -> bool { - match asset_type { - AssetType::Client => !self.safe_version.is_empty(), - AssetType::Node => !self.safenode_version.is_empty(), - AssetType::NodeManager => !self.safenode_manager_version.is_empty(), - } - } - - pub fn get_install_path(&self, asset_type: &AssetType) -> PathBuf { + pub fn get_install_details(&self, asset_type: &AssetType) -> Option<(PathBuf, Version)> { + // In each of these cases, if either the path or the version is set, both of them have to + // be set, so the unwrap seems justified. If only one of them is set, that's a bug. match asset_type { - AssetType::Client => self.safe_path.clone(), - AssetType::Node => self.safenode_path.clone(), - AssetType::NodeManager => self.safenode_manager_path.clone(), + AssetType::Client => { + if self.safe_path.is_some() { + let path = self.safe_path.as_ref().unwrap(); + let version = self.safe_version.as_ref().unwrap(); + return Some((path.clone(), version.clone())); + } + None + } + AssetType::Node => { + if self.safenode_path.is_some() { + let path = self.safenode_path.as_ref().unwrap(); + let version = self.safenode_version.as_ref().unwrap(); + return Some((path.clone(), version.clone())); + } + None + } + AssetType::NodeManager => { + if self.safenode_manager_path.is_some() { + let path = self.safenode_manager_path.as_ref().unwrap(); + let version = self.safenode_manager_version.as_ref().unwrap(); + return Some((path.clone(), version.clone())); + } + None + } } } @@ -204,11 +285,11 @@ pub fn check_prerequisites() -> Result<()> { /// A tuple of the version number and full path of the installed binary. pub async fn install_bin( asset_type: AssetType, - release_repo: Box, + release_repo: Box, platform: &str, dest_dir_path: PathBuf, - version: Option, -) -> Result<(String, PathBuf)> { + version: Option, +) -> Result<(Version, PathBuf)> { let bin_name = get_bin_name(&asset_type); println!( "Installing {bin_name} for {platform} at {}...", @@ -368,9 +449,10 @@ mod test { use mockall::mock; use mockall::predicate::*; use mockall::Sequence; + use semver::Version; use sn_releases::{ ArchiveType, Platform, ProgressCallback, ReleaseType, Result as SnReleaseResult, - SafeReleaseRepositoryInterface, + SafeReleaseRepoActions, }; #[cfg(unix)] use std::os::unix::fs::PermissionsExt; @@ -383,10 +465,6 @@ mod test { #[cfg(windows)] const SAFE_BIN_NAME: &str = "safe.exe"; #[cfg(unix)] - const SAFENODE_BIN_NAME: &str = "safenode"; - #[cfg(windows)] - const SAFENODE_BIN_NAME: &str = "safenode.exe"; - #[cfg(unix)] const PLATFORM: &str = "x86_64-unknown-linux-musl"; #[cfg(windows)] const PLATFORM: &str = "x86_64-pc-windows-msvc"; @@ -394,12 +472,12 @@ mod test { mock! { pub SafeReleaseRepository {} #[async_trait] - impl SafeReleaseRepositoryInterface for SafeReleaseRepository { - async fn get_latest_version(&self, release_type: &ReleaseType) -> SnReleaseResult; + impl SafeReleaseRepoActions for SafeReleaseRepository { + async fn get_latest_version(&self, release_type: &ReleaseType) -> SnReleaseResult; async fn download_release_from_s3( &self, release_type: &ReleaseType, - version: &str, + version: &Version, platform: &Platform, archive_type: &ArchiveType, download_dir: &Path, @@ -417,7 +495,9 @@ mod test { #[tokio::test] async fn install_bin_should_install_the_latest_version() -> Result<()> { - let latest_version = "0.86.55"; + let latest_version = Version::new(0, 86, 55); + let latest_version_clone1 = latest_version.clone(); + let latest_version_clone2 = latest_version.clone(); let temp_dir = assert_fs::TempDir::new()?; let install_dir = temp_dir.child("install"); @@ -431,14 +511,14 @@ mod test { mock_release_repo .expect_get_latest_version() .times(1) - .returning(|_| Ok(latest_version.to_string())) + .returning(move |_| Ok(latest_version_clone1.clone())) .in_sequence(&mut seq); mock_release_repo .expect_download_release_from_s3() .with( eq(&ReleaseType::Safe), - eq(latest_version), + eq(latest_version.clone()), always(), // Varies per platform eq(&ArchiveType::TarGz), always(), // Temporary directory which doesn't really matter @@ -448,7 +528,7 @@ mod test { .returning(move |_, _, _, _, _, _| { Ok(PathBuf::from(format!( "/tmp/safe-{}-x86_64-unknown-linux-musl.tar.gz", - latest_version + latest_version_clone2 ))) }) .in_sequence(&mut seq); @@ -460,7 +540,7 @@ mod test { .with( eq(PathBuf::from(format!( "/tmp/safe-{}-x86_64-unknown-linux-musl.tar.gz", - latest_version + latest_version.clone() ))), always(), // We will extract to a temporary directory ) @@ -477,7 +557,7 @@ mod test { ) .await?; - assert_eq!(version, "0.86.55"); + assert_eq!(version, Version::new(0, 86, 55)); assert_eq!(bin_path, installed_safe.to_path_buf()); #[cfg(unix)] @@ -495,7 +575,9 @@ mod test { #[tokio::test] async fn install_bin_when_parent_dirs_in_dest_path_do_not_exist_should_install_the_latest_version( ) -> Result<()> { - let latest_version = "0.86.55"; + let latest_version = Version::new(0, 86, 55); + let latest_version_clone1 = latest_version.clone(); + let latest_version_clone2 = latest_version.clone(); let temp_dir = assert_fs::TempDir::new()?; let install_dir = temp_dir.child("install/using/many/paths"); @@ -509,14 +591,14 @@ mod test { mock_release_repo .expect_get_latest_version() .times(1) - .returning(|_| Ok(latest_version.to_string())) + .returning(move |_| Ok(latest_version_clone1.clone())) .in_sequence(&mut seq); mock_release_repo .expect_download_release_from_s3() .with( eq(&ReleaseType::Safe), - eq(latest_version), + eq(latest_version.clone()), always(), // Varies per platform eq(&ArchiveType::TarGz), always(), // Temporary directory which doesn't really matter @@ -526,7 +608,7 @@ mod test { .returning(move |_, _, _, _, _, _| { Ok(PathBuf::from(format!( "/tmp/safe-{}-x86_64-unknown-linux-musl.tar.gz", - latest_version + latest_version_clone2 ))) }) .in_sequence(&mut seq); @@ -555,7 +637,7 @@ mod test { ) .await?; - assert_eq!(version, "0.86.55"); + assert_eq!(version, Version::new(0, 86, 55)); assert_eq!(bin_path, installed_safe.to_path_buf()); Ok(()) @@ -563,7 +645,8 @@ mod test { #[tokio::test] async fn install_bin_should_install_a_specific_version() -> Result<()> { - let specific_version = "0.85.0"; + let specific_version = Version::new(0, 85, 0); + let specific_version_clone = specific_version.clone(); let temp_dir = assert_fs::TempDir::new()?; let install_dir = temp_dir.child("install"); @@ -578,7 +661,7 @@ mod test { .expect_download_release_from_s3() .with( eq(&ReleaseType::Safe), - eq(specific_version), + eq(specific_version.clone()), always(), // Varies per platform eq(&ArchiveType::TarGz), always(), // Temporary directory which doesn't really matter @@ -588,7 +671,7 @@ mod test { .returning(move |_, _, _, _, _, _| { Ok(PathBuf::from(format!( "/tmp/safe-{}-x86_64-unknown-linux-musl.tar.gz", - specific_version + specific_version_clone ))) }) .in_sequence(&mut seq); @@ -600,7 +683,7 @@ mod test { .with( eq(PathBuf::from(format!( "/tmp/safe-{}-x86_64-unknown-linux-musl.tar.gz", - specific_version + specific_version.clone() ))), always(), // We will extract to a temporary directory ) @@ -613,7 +696,7 @@ mod test { Box::new(mock_release_repo), PLATFORM, install_dir.path().to_path_buf(), - Some(specific_version.to_string()), + Some(specific_version.clone()), ) .await?; @@ -709,27 +792,33 @@ mod test { testnet_bin_file.write_binary(b"fake testnet code")?; let settings = Settings { - safe_path: safe_bin_file.to_path_buf(), - safe_version: "v0.75.1".to_string(), - safenode_path: safenode_bin_file.to_path_buf(), - safenode_version: "v0.75.2".to_string(), - safenode_manager_path: safenode_manager_bin_file.to_path_buf(), - safenode_manager_version: "v0.1.8".to_string(), + safe_path: Some(safe_bin_file.to_path_buf()), + safe_version: Some(Version::new(0, 75, 1)), + safenode_path: Some(safenode_bin_file.to_path_buf()), + safenode_version: Some(Version::new(0, 75, 2)), + safenode_manager_path: Some(safenode_manager_bin_file.to_path_buf()), + safenode_manager_version: Some(Version::new(0, 1, 8)), }; settings.save(&settings_file.to_path_buf())?; settings_file.assert(predicates::path::is_file()); let settings = Settings::read(&settings_file.to_path_buf())?; - assert_eq!(settings.safe_path, safe_bin_file.to_path_buf()); - assert_eq!(settings.safe_version, "v0.75.1"); - assert_eq!(settings.safenode_path, safenode_bin_file.to_path_buf()); - assert_eq!(settings.safenode_version, "v0.75.2"); + assert_eq!(settings.safe_path, Some(safe_bin_file.to_path_buf())); + assert_eq!(settings.safe_version, Some(Version::new(0, 75, 1))); + assert_eq!( + settings.safenode_path, + Some(safenode_bin_file.to_path_buf()) + ); + assert_eq!(settings.safenode_version, Some(Version::new(0, 75, 2))); assert_eq!( settings.safenode_manager_path, - safenode_manager_bin_file.to_path_buf() + Some(safenode_manager_bin_file.to_path_buf()) + ); + assert_eq!( + settings.safenode_manager_version, + Some(Version::new(0, 1, 8)) ); - assert_eq!(settings.safenode_manager_version, "v0.1.8"); Ok(()) } @@ -752,66 +841,33 @@ mod test { testnet_bin_file.write_binary(b"fake testnet code")?; let settings = Settings { - safe_path: safe_bin_file.to_path_buf(), - safe_version: "v0.75.1".to_string(), - safenode_path: safenode_bin_file.to_path_buf(), - safenode_version: "v0.75.2".to_string(), - safenode_manager_path: safenode_manager_bin_file.to_path_buf(), - safenode_manager_version: "v0.1.8".to_string(), + safe_path: Some(safe_bin_file.to_path_buf()), + safe_version: Some(Version::new(0, 75, 1)), + safenode_path: Some(safenode_bin_file.to_path_buf()), + safenode_version: Some(Version::new(0, 75, 2)), + safenode_manager_path: Some(safenode_manager_bin_file.to_path_buf()), + safenode_manager_version: Some(Version::new(0, 1, 8)), }; settings.save(&settings_file.to_path_buf())?; settings_file.assert(predicates::path::is_file()); let settings = Settings::read(&settings_file.to_path_buf())?; - assert_eq!(settings.safe_path, safe_bin_file.to_path_buf()); - assert_eq!(settings.safe_version, "v0.75.1"); - assert_eq!(settings.safenode_path, safenode_bin_file.to_path_buf()); - assert_eq!(settings.safenode_version, "v0.75.2"); + assert_eq!(settings.safe_path, Some(safe_bin_file.to_path_buf())); + assert_eq!(settings.safe_version, Some(Version::new(0, 75, 1))); assert_eq!( - settings.safenode_manager_path, - safenode_manager_bin_file.to_path_buf() + settings.safenode_path, + Some(safenode_bin_file.to_path_buf()) ); - assert_eq!(settings.safenode_manager_version, "v0.1.8"); - Ok(()) - } - - #[tokio::test] - async fn save_should_write_updated_settings() -> Result<()> { - let tmp_data_path = assert_fs::TempDir::new()?; - let settings_file = tmp_data_path.child("safeup.json"); - settings_file.write_str( - r#" - { - "safe_path": "/home/chris/.local/safe", - "safe_version": "v0.75.1", - "safenode_path": "/home/chris/.local/bin/safenode", - "safenode_version": "v0.75.2", - "safenode_manager_path": "/home/chris/.local/bin/safenode-manager", - "safenode_manager_version": "v0.1.8" - } - "#, - )?; - - let safenode_bin_file = tmp_data_path.child(SAFENODE_BIN_NAME); - safenode_bin_file.write_binary(b"fake safenode code")?; - - let mut settings = Settings::read(&settings_file.to_path_buf())?; - settings.safenode_path = safenode_bin_file.to_path_buf(); - - settings.save(&settings_file.to_path_buf())?; - - settings_file.assert(predicates::path::is_file()); - let settings = Settings::read(&settings_file.to_path_buf())?; - assert_eq!(settings.safe_path, PathBuf::from("/home/chris/.local/safe")); - assert_eq!(settings.safe_version, "v0.75.1"); - assert_eq!(settings.safenode_path, safenode_bin_file.to_path_buf()); - assert_eq!(settings.safenode_version, "v0.75.2"); + assert_eq!(settings.safenode_version, Some(Version::new(0, 75, 2))); assert_eq!( settings.safenode_manager_path, - PathBuf::from("/home/chris/.local/bin/safenode-manager") + Some(safenode_manager_bin_file.to_path_buf()) + ); + assert_eq!( + settings.safenode_manager_version, + Some(Version::new(0, 1, 8)) ); - assert_eq!(settings.safenode_manager_version, "v0.1.8"); Ok(()) } } diff --git a/src/update.rs b/src/update.rs index 17ba692..2e133f8 100644 --- a/src/update.rs +++ b/src/update.rs @@ -1,12 +1,13 @@ use crate::install::{AssetType, Settings}; use semver::Version; use std::cmp::Ordering; +use std::path::PathBuf; use color_eyre::{eyre::eyre, Help, Result}; #[derive(Clone, Debug)] pub enum UpdateAssessmentResult { - PerformUpdate, + PerformUpdate(PathBuf), NoPreviousInstallation, AtLatestVersion, } @@ -20,26 +21,23 @@ pub enum UpdateAssessmentResult { /// * Otherwise, if there is a newer version available, we'll perform the upgrade. pub fn perform_update_assessment( asset_type: &AssetType, - latest_version: &str, + latest_version: &Version, settings: &Settings, ) -> Result { - if !settings.is_installed(asset_type) { - return Ok(UpdateAssessmentResult::NoPreviousInstallation); - } - match compare_versions(latest_version, &settings.get_installed_version(asset_type))? { - Ordering::Equal => Ok(UpdateAssessmentResult::AtLatestVersion), - Ordering::Less => Err(eyre!( - "The latest version is less than the current version of your binary." - ) - .suggestion("You may want to remove your safeup.conf and install safeup again.")), - Ordering::Greater => Ok(UpdateAssessmentResult::PerformUpdate), + if let Some((installed_path, installed_version)) = settings.get_install_details(asset_type) { + println!("Current version of {asset_type} is {installed_version}"); + match latest_version.cmp(&installed_version) { + Ordering::Equal => return Ok(UpdateAssessmentResult::AtLatestVersion), + Ordering::Less => { + return Err(eyre!( + "The latest version is less than the current version of your binary." + ) + .suggestion("You may want to remove your safeup.conf and install safeup again.")) + } + Ordering::Greater => return Ok(UpdateAssessmentResult::PerformUpdate(installed_path)), + } } -} - -fn compare_versions(version_a: &str, version_b: &str) -> Result { - let v1 = Version::parse(version_a.strip_prefix('v').unwrap_or(version_a))?; - let v2 = Version::parse(version_b.strip_prefix('v').unwrap_or(version_b))?; - Ok(v1.cmp(&v2)) + Ok(UpdateAssessmentResult::NoPreviousInstallation) } #[cfg(test)] @@ -47,41 +45,45 @@ mod test { use super::{perform_update_assessment, UpdateAssessmentResult}; use crate::install::{AssetType, Settings}; use color_eyre::{eyre::eyre, Result}; + use semver::Version; use std::path::PathBuf; #[test] fn perform_upgrade_assessment_should_indicate_no_previous_installation() -> Result<()> { let settings = Settings { - safe_path: PathBuf::new(), - safe_version: String::new(), - safenode_path: PathBuf::from("/home/chris/.local/bin/safenode"), - safenode_version: "v0.83.13".to_string(), - safenode_manager_path: PathBuf::from("/home/chris/.local/bin/safenode-manager"), - safenode_manager_version: "v0.1.8".to_string(), + safe_path: None, + safe_version: None, + safenode_path: Some(PathBuf::from("/home/chris/.local/bin/safenode")), + safenode_version: Some(Version::new(0, 83, 13)), + safenode_manager_path: Some(PathBuf::from("/home/chris/.local/bin/safenode-manager")), + safenode_manager_version: Some(Version::new(0, 1, 8)), }; - let decision = perform_update_assessment(&AssetType::Client, "v0.78.26", &settings)?; + let decision = + perform_update_assessment(&AssetType::Client, &Version::new(0, 78, 26), &settings)?; assert_matches!(decision, UpdateAssessmentResult::NoPreviousInstallation); let settings = Settings { - safe_path: PathBuf::from("/home/chris/.local/safe"), - safe_version: "v0.78.26".to_string(), - safenode_path: PathBuf::new(), - safenode_version: String::new(), - safenode_manager_path: PathBuf::from("/home/chris/.local/bin/safenode-manager"), - safenode_manager_version: "v0.1.8".to_string(), + safe_path: Some(PathBuf::from("/home/chris/.local/safe")), + safe_version: Some(Version::new(0, 78, 26)), + safenode_path: None, + safenode_version: None, + safenode_manager_path: Some(PathBuf::from("/home/chris/.local/bin/safenode-manager")), + safenode_manager_version: Some(Version::new(0, 1, 8)), }; - let decision = perform_update_assessment(&AssetType::Node, "v0.83.13", &settings)?; + let decision = + perform_update_assessment(&AssetType::Node, &Version::new(0, 83, 13), &settings)?; assert_matches!(decision, UpdateAssessmentResult::NoPreviousInstallation); let settings = Settings { - safe_path: PathBuf::from("/home/chris/.local/safe"), - safe_version: "v0.78.26".to_string(), - safenode_path: PathBuf::from("/home/chris/.local/bin/safenode"), - safenode_version: "v0.83.13".to_string(), - safenode_manager_path: PathBuf::new(), - safenode_manager_version: String::new(), + safe_path: Some(PathBuf::from("/home/chris/.local/safe")), + safe_version: Some(Version::new(0, 78, 26)), + safenode_path: Some(PathBuf::from("/home/chris/.local/bin/safenode")), + safenode_version: Some(Version::new(0, 83, 13)), + safenode_manager_path: None, + safenode_manager_version: None, }; - let decision = perform_update_assessment(&AssetType::NodeManager, "v0.1.8", &settings)?; + let decision = + perform_update_assessment(&AssetType::NodeManager, &Version::new(0, 1, 8), &settings)?; assert_matches!(decision, UpdateAssessmentResult::NoPreviousInstallation); Ok(()) @@ -90,21 +92,24 @@ mod test { #[test] fn perform_upgrade_assessment_should_indicate_we_are_at_latest_version() -> Result<()> { let settings = Settings { - safe_path: PathBuf::from("/home/chris/.local/safe"), - safe_version: "v0.78.26".to_string(), - safenode_path: PathBuf::from("/home/chris/.local/bin/safenode"), - safenode_version: "v0.83.13".to_string(), - safenode_manager_path: PathBuf::from("/home/chris/.local/bin/safenode-manager"), - safenode_manager_version: "v0.1.8".to_string(), + safe_path: Some(PathBuf::from("/home/chris/.local/safe")), + safe_version: Some(Version::new(0, 78, 26)), + safenode_path: Some(PathBuf::from("/home/chris/.local/bin/safenode")), + safenode_version: Some(Version::new(0, 83, 13)), + safenode_manager_path: Some(PathBuf::from("/home/chris/.local/bin/safenode-manager")), + safenode_manager_version: Some(Version::new(0, 1, 8)), }; - let decision = perform_update_assessment(&AssetType::Client, "v0.78.26", &settings)?; + let decision = + perform_update_assessment(&AssetType::Client, &Version::new(0, 78, 26), &settings)?; assert_matches!(decision, UpdateAssessmentResult::AtLatestVersion); - let decision = perform_update_assessment(&AssetType::Node, "v0.83.13", &settings)?; + let decision = + perform_update_assessment(&AssetType::Node, &Version::new(0, 83, 13), &settings)?; assert_matches!(decision, UpdateAssessmentResult::AtLatestVersion); - let decision = perform_update_assessment(&AssetType::NodeManager, "v0.1.8", &settings)?; + let decision = + perform_update_assessment(&AssetType::NodeManager, &Version::new(0, 1, 8), &settings)?; assert_matches!(decision, UpdateAssessmentResult::AtLatestVersion); Ok(()) @@ -114,15 +119,16 @@ mod test { fn perform_upgrade_assessment_latest_version_is_less_than_current_should_return_error( ) -> Result<()> { let settings = Settings { - safe_path: PathBuf::from("/home/chris/.local/safe"), - safe_version: "v0.78.26".to_string(), - safenode_path: PathBuf::from("/home/chris/.local/bin/safenode"), - safenode_version: "v0.83.13".to_string(), - safenode_manager_path: PathBuf::from("/home/chris/.local/bin/safenode-manager"), - safenode_manager_version: "v0.1.8".to_string(), + safe_path: Some(PathBuf::from("/home/chris/.local/safe")), + safe_version: Some(Version::new(0, 78, 26)), + safenode_path: Some(PathBuf::from("/home/chris/.local/bin/safenode")), + safenode_version: Some(Version::new(0, 83, 13)), + safenode_manager_path: Some(PathBuf::from("/home/chris/.local/bin/safenode-manager")), + safenode_manager_version: Some(Version::new(0, 1, 8)), }; - let result = perform_update_assessment(&AssetType::Client, "v0.76.0", &settings); + let result = + perform_update_assessment(&AssetType::Client, &Version::new(0, 76, 0), &settings); match result { Ok(_) => return Err(eyre!("this test should return an error")), Err(e) => assert_eq!( @@ -131,7 +137,8 @@ mod test { ), } - let result = perform_update_assessment(&AssetType::Node, "v0.82.0", &settings); + let result = + perform_update_assessment(&AssetType::Node, &Version::new(0, 81, 0), &settings); match result { Ok(_) => return Err(eyre!("this test should return an error")), Err(e) => assert_eq!( @@ -140,7 +147,8 @@ mod test { ), } - let result = perform_update_assessment(&AssetType::NodeManager, "v0.1.7", &settings); + let result = + perform_update_assessment(&AssetType::NodeManager, &Version::new(0, 1, 7), &settings); match result { Ok(_) => return Err(eyre!("this test should return an error")), Err(e) => assert_eq!( @@ -149,14 +157,6 @@ mod test { ), } - let result = perform_update_assessment(&AssetType::Node, "v0.2.0", &settings); - match result { - Ok(_) => return Err(eyre!("this test should return an error")), - Err(e) => assert_eq!( - "The latest version is less than the current version of your binary.", - e.to_string() - ), - } Ok(()) } @@ -164,22 +164,25 @@ mod test { fn perform_upgrade_assessment_should_perform_update_when_latest_patch_version_is_greater( ) -> Result<()> { let settings = Settings { - safe_path: PathBuf::from("/home/chris/.local/safe"), - safe_version: "v0.78.26".to_string(), - safenode_path: PathBuf::from("/home/chris/.local/bin/safenode"), - safenode_version: "v0.83.13".to_string(), - safenode_manager_path: PathBuf::from("/home/chris/.local/bin/safenode-manager"), - safenode_manager_version: "v0.1.7".to_string(), + safe_path: Some(PathBuf::from("/home/chris/.local/safe")), + safe_version: Some(Version::new(0, 78, 26)), + safenode_path: Some(PathBuf::from("/home/chris/.local/bin/safenode")), + safenode_version: Some(Version::new(0, 83, 13)), + safenode_manager_path: Some(PathBuf::from("/home/chris/.local/bin/safenode-manager")), + safenode_manager_version: Some(Version::new(0, 1, 7)), }; - let decision = perform_update_assessment(&AssetType::Client, "v0.78.27", &settings)?; - assert_matches!(decision, UpdateAssessmentResult::PerformUpdate); + let decision = + perform_update_assessment(&AssetType::Client, &Version::new(0, 78, 27), &settings)?; + assert_matches!(decision, UpdateAssessmentResult::PerformUpdate(_)); - let decision = perform_update_assessment(&AssetType::Node, "v0.83.14", &settings)?; - assert_matches!(decision, UpdateAssessmentResult::PerformUpdate); + let decision = + perform_update_assessment(&AssetType::Node, &Version::new(0, 83, 14), &settings)?; + assert_matches!(decision, UpdateAssessmentResult::PerformUpdate(_)); - let decision = perform_update_assessment(&AssetType::NodeManager, "v0.1.8", &settings)?; - assert_matches!(decision, UpdateAssessmentResult::PerformUpdate); + let decision = + perform_update_assessment(&AssetType::NodeManager, &Version::new(0, 1, 8), &settings)?; + assert_matches!(decision, UpdateAssessmentResult::PerformUpdate(_)); Ok(()) } @@ -188,22 +191,25 @@ mod test { fn perform_upgrade_assessment_should_perform_update_when_latest_minor_version_is_greater( ) -> Result<()> { let settings = Settings { - safe_path: PathBuf::from("/home/chris/.local/safe"), - safe_version: "v0.78.26".to_string(), - safenode_path: PathBuf::from("/home/chris/.local/bin/safenode"), - safenode_version: "v0.83.13".to_string(), - safenode_manager_path: PathBuf::from("/home/chris/.local/bin/safenode-manager"), - safenode_manager_version: "v0.1.7".to_string(), + safe_path: Some(PathBuf::from("/home/chris/.local/safe")), + safe_version: Some(Version::new(0, 78, 26)), + safenode_path: Some(PathBuf::from("/home/chris/.local/bin/safenode")), + safenode_version: Some(Version::new(0, 83, 13)), + safenode_manager_path: Some(PathBuf::from("/home/chris/.local/bin/safenode-manager")), + safenode_manager_version: Some(Version::new(0, 1, 7)), }; - let decision = perform_update_assessment(&AssetType::Client, "v0.79.0", &settings)?; - assert_matches!(decision, UpdateAssessmentResult::PerformUpdate); + let decision = + perform_update_assessment(&AssetType::Client, &Version::new(0, 79, 0), &settings)?; + assert_matches!(decision, UpdateAssessmentResult::PerformUpdate(_)); - let decision = perform_update_assessment(&AssetType::Node, "v0.84.0", &settings)?; - assert_matches!(decision, UpdateAssessmentResult::PerformUpdate); + let decision = + perform_update_assessment(&AssetType::Node, &Version::new(0, 84, 0), &settings)?; + assert_matches!(decision, UpdateAssessmentResult::PerformUpdate(_)); - let decision = perform_update_assessment(&AssetType::NodeManager, "v0.2.0", &settings)?; - assert_matches!(decision, UpdateAssessmentResult::PerformUpdate); + let decision = + perform_update_assessment(&AssetType::NodeManager, &Version::new(0, 2, 0), &settings)?; + assert_matches!(decision, UpdateAssessmentResult::PerformUpdate(_)); Ok(()) } @@ -212,39 +218,25 @@ mod test { fn perform_upgrade_assessment_should_perform_update_when_latest_major_version_is_greater( ) -> Result<()> { let settings = Settings { - safe_path: PathBuf::from("/home/chris/.local/safe"), - safe_version: "v0.78.26".to_string(), - safenode_path: PathBuf::from("/home/chris/.local/bin/safenode"), - safenode_version: "v0.83.13".to_string(), - safenode_manager_path: PathBuf::from("/home/chris/.local/bin/safenode-manager"), - safenode_manager_version: "v0.1.7".to_string(), + safe_path: Some(PathBuf::from("/home/chris/.local/safe")), + safe_version: Some(Version::new(0, 78, 26)), + safenode_path: Some(PathBuf::from("/home/chris/.local/bin/safenode")), + safenode_version: Some(Version::new(0, 83, 13)), + safenode_manager_path: Some(PathBuf::from("/home/chris/.local/bin/safenode-manager")), + safenode_manager_version: Some(Version::new(0, 1, 7)), }; - let decision = perform_update_assessment(&AssetType::Client, "v1.0.0", &settings)?; - assert_matches!(decision, UpdateAssessmentResult::PerformUpdate); - - let decision = perform_update_assessment(&AssetType::Node, "v1.0.0", &settings)?; - assert_matches!(decision, UpdateAssessmentResult::PerformUpdate); - - let decision = perform_update_assessment(&AssetType::NodeManager, "v1.0.0", &settings)?; - assert_matches!(decision, UpdateAssessmentResult::PerformUpdate); + let decision = + perform_update_assessment(&AssetType::Client, &Version::new(1, 0, 0), &settings)?; + assert_matches!(decision, UpdateAssessmentResult::PerformUpdate(_)); - Ok(()) - } - - #[test] - fn perform_upgrade_assessment_should_not_error_when_versions_have_no_leading_v() -> Result<()> { - let settings = Settings { - safe_path: PathBuf::from("/home/chris/.local/safe"), - safe_version: "0.78.26".to_string(), - safenode_path: PathBuf::from("/home/chris/.local/bin/safenode"), - safenode_version: "0.83.13".to_string(), - safenode_manager_path: PathBuf::from("/home/chris/.local/bin/safenode-manager"), - safenode_manager_version: "v0.1.7".to_string(), - }; + let decision = + perform_update_assessment(&AssetType::Node, &Version::new(1, 0, 0), &settings)?; + assert_matches!(decision, UpdateAssessmentResult::PerformUpdate(_)); - let decision = perform_update_assessment(&AssetType::Client, "0.78.27", &settings)?; - assert_matches!(decision, UpdateAssessmentResult::PerformUpdate); + let decision = + perform_update_assessment(&AssetType::NodeManager, &Version::new(1, 0, 0), &settings)?; + assert_matches!(decision, UpdateAssessmentResult::PerformUpdate(_)); Ok(()) }