refactor: use version type rather than strings

Cargo.toml

@@ -14,6 +14,7 @@ flate2 = "1.0"
 lazy_static = "1.4.0"
 regex = "1.10.2"
 reqwest = { version = "0.11", default-features = false, features = ["json", "rustls-tls"] }
+semver = "1.0.22"
 serde_json = "1.0"
 tar = "0.4.40"
 thiserror = "1.0.49"

src/error.rs

@@ -21,8 +21,6 @@ pub enum Error {
     DateTimeParseError(#[from] chrono::ParseError),
     #[error("Could not convert API response header links to string")]
     HeaderLinksToStrError,
-    #[error("The version string is invalid: {0}")]
-    InvalidVersionFormat(String),
     #[error(transparent)]
     Io(#[from] std::io::Error),
     #[error(transparent)]
@@ -37,6 +35,8 @@ pub enum Error {
     ReqwestError(#[from] reqwest::Error),
     #[error("Release binary {0} was not found")]
     ReleaseBinaryNotFound(String),
+    #[error(transparent)]
+    SemVerError(#[from] semver::Error),
     #[error("Could not parse version from tag name")]
     TagNameVersionParsingFailed,
     #[error("The URL must point to a zip or gzipped tar archive")]

src/lib.rs

@@ -13,6 +13,7 @@ pub mod error;
 use async_trait::async_trait;
 use lazy_static::lazy_static;
 use reqwest::Client;
+use semver::Version;
 use serde_json::Value;
 use std::collections::HashMap;
 use std::env::consts::{ARCH, OS};
@@ -112,12 +113,12 @@ impl fmt::Display for ArchiveType {
 pub type ProgressCallback = dyn Fn(u64, u64) + Send + Sync;
 
 #[async_trait]
-pub trait SafeReleaseRepositoryInterface {
-    async fn get_latest_version(&self, release_type: &ReleaseType) -> Result<String>;
+pub trait SafeReleaseRepoActions {
+    async fn get_latest_version(&self, release_type: &ReleaseType) -> Result<Version>;
     async fn download_release_from_s3(
         &self,
         release_type: &ReleaseType,
-        version: &str,
+        version: &Version,
         platform: &Platform,
         archive_type: &ArchiveType,
         dest_path: &Path,
@@ -133,8 +134,8 @@ pub trait SafeReleaseRepositoryInterface {
         -> Result<PathBuf>;
 }
 
-impl dyn SafeReleaseRepositoryInterface {
-    pub fn default_config() -> Box<dyn SafeReleaseRepositoryInterface> {
+impl dyn SafeReleaseRepoActions {
+    pub fn default_config() -> Box<dyn SafeReleaseRepoActions> {
         Box::new(SafeReleaseRepository {
             github_api_base_url: GITHUB_API_URL.to_string(),
             faucet_base_url: FAUCET_S3_BASE_URL.to_string(),
@@ -200,7 +201,7 @@ impl SafeReleaseRepository {
 }
 
 #[async_trait]
-impl SafeReleaseRepositoryInterface for SafeReleaseRepository {
+impl SafeReleaseRepoActions for SafeReleaseRepository {
     /// Uses the crates.io API to obtain the latest version of a crate.
     ///
     /// # Arguments
@@ -217,7 +218,7 @@ impl SafeReleaseRepositoryInterface for SafeReleaseRepository {
     /// This function will return an error if:
     /// - The HTTP request to crates.io API fails
     /// - The received JSON data does not have a `crate.newest_version` value
-    async fn get_latest_version(&self, release_type: &ReleaseType) -> Result<String> {
+    async fn get_latest_version(&self, release_type: &ReleaseType) -> Result<Version> {
         let crate_name = *RELEASE_TYPE_CRATE_NAME_MAP.get(release_type).unwrap();
         let url = format!("https://crates.io/api/v1/crates/{}", crate_name);
 
@@ -235,7 +236,7 @@ impl SafeReleaseRepositoryInterface for SafeReleaseRepository {
         let json: Value = serde_json::from_str(&body)?;
 
         if let Some(version) = json["crate"]["newest_version"].as_str() {
-            return Ok(version.to_string());
+            return Ok(Version::parse(version)?);
         }
 
         Err(Error::LatestReleaseNotFound(release_type.to_string()))
@@ -259,19 +260,12 @@ impl SafeReleaseRepositoryInterface for SafeReleaseRepository {
     async fn download_release_from_s3(
         &self,
         release_type: &ReleaseType,
-        version: &str,
+        version: &Version,
         platform: &Platform,
         archive_type: &ArchiveType,
         dest_path: &Path,
         callback: &ProgressCallback,
     ) -> Result<PathBuf> {
-        // parse version str.
-        let version_pattern =
-            regex::Regex::new(r"^\d+\.\d+\.\d+$").map_err(|_| Error::RegexError)?;
-        if !version_pattern.is_match(version) {
-            return Err(Error::InvalidVersionFormat(version.to_string()));
-        }
-
         let archive_ext = archive_type.to_string();
         let url = format!(
             "{}/{}-{}-{}.{}",

tests/download_url.rs

@@ -8,7 +8,7 @@
 
 use assert_fs::prelude::*;
 use sn_releases::error::Error;
-use sn_releases::SafeReleaseRepositoryInterface;
+use sn_releases::SafeReleaseRepoActions;
 
 #[tokio::test]
 async fn should_download_a_custom_binary() {
@@ -20,7 +20,7 @@ async fn should_download_a_custom_binary() {
 
     let url = "https://sn-node.s3.eu-west-2.amazonaws.com/jacderida/file-upload-address/safenode-charlie-x86_64-unknown-linux-musl.tar.gz";
     let progress_callback = |_downloaded: u64, _total: u64| {};
-    let release_repo = <dyn SafeReleaseRepositoryInterface>::default_config();
+    let release_repo = <dyn SafeReleaseRepoActions>::default_config();
     release_repo
         .download_release(url, &download_dir, &progress_callback)
         .await
@@ -37,7 +37,7 @@ async fn should_fail_to_download_non_archive() {
 
     let url = "https://sn-node.s3.eu-west-2.amazonaws.com/jacderida/file-upload-address/safenode-charlie-x86_64-unknown-linux-musl.txt";
     let progress_callback = |_downloaded: u64, _total: u64| {};
-    let release_repo = <dyn SafeReleaseRepositoryInterface>::default_config();
+    let release_repo = <dyn SafeReleaseRepoActions>::default_config();
     let result = release_repo
         .download_release(url, &download_dir, &progress_callback)
         .await;

tests/test_download_from_s3.rs

@@ -8,8 +8,8 @@
 
 use assert_fs::prelude::*;
 use predicates::prelude::*;
-use sn_releases::error::Error;
-use sn_releases::{ArchiveType, Platform, ReleaseType, SafeReleaseRepositoryInterface};
+use semver::Version;
+use sn_releases::{ArchiveType, Platform, ReleaseType, SafeReleaseRepoActions};
 
 const FAUCET_VERSION: &str = "0.1.98";
 const SAFE_VERSION: &str = "0.83.51";
@@ -32,11 +32,11 @@ async fn download_and_extract(
 
     let progress_callback = |_downloaded: u64, _total: u64| {};
 
-    let release_repo = <dyn SafeReleaseRepositoryInterface>::default_config();
+    let release_repo = <dyn SafeReleaseRepoActions>::default_config();
     let archive_path = release_repo
         .download_release_from_s3(
             release_type,
-            version,
+            &Version::parse(version).unwrap(),
             platform,
             archive_type,
             &download_dir,
@@ -68,37 +68,6 @@ async fn download_and_extract(
     assert_eq!(binary_path.to_path_buf(), extracted_path);
 }
 
-#[tokio::test]
-async fn should_fail_when_trying_to_download_with_invalid_version() {
-    let dest_dir = assert_fs::TempDir::new().unwrap();
-    let download_dir = dest_dir.child("download_to");
-    download_dir.create_dir_all().unwrap();
-    let extract_dir = dest_dir.child("extract_to");
-    extract_dir.create_dir_all().unwrap();
-
-    let progress_callback = |_downloaded: u64, _total: u64| {};
-
-    let release_repo = <dyn SafeReleaseRepositoryInterface>::default_config();
-    let result = release_repo
-        .download_release_from_s3(
-            &ReleaseType::Safe,
-            "x.y.z",
-            &Platform::LinuxMusl,
-            &ArchiveType::TarGz,
-            &download_dir,
-            &progress_callback,
-        )
-        .await;
-
-    match result {
-        Ok(_) => panic!("This test should result in a failure"),
-        Err(e) => match e {
-            Error::InvalidVersionFormat(_) => {}
-            _ => panic!("The error type should be InvalidVersionFormat. Got {e:?}"),
-        },
-    }
-}
-
 ///
 /// Safe Tests
 ///