Skip to content

Commit

Permalink
refactor: use version type rather than strings
Browse files Browse the repository at this point in the history
BREAKING CHANGE: the `semver::Version` type is used rather strings for working with versions.

The need for the version parsing test and tests for retrieving the latest version was removed. The
latter of those was only testing that returned string was a valid semantic version, which wouldn't
be possible with a `Version` type.
  • Loading branch information
jacderida committed Mar 19, 2024
1 parent b7731e6 commit bb64146
Show file tree
Hide file tree
Showing 5 changed files with 11 additions and 128 deletions.
1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
4 changes: 2 additions & 2 deletions src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand All @@ -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")]
Expand Down
18 changes: 6 additions & 12 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -113,11 +114,11 @@ pub type ProgressCallback = dyn Fn(u64, u64) + Send + Sync;

#[async_trait]
pub trait SafeReleaseRepoActions {
async fn get_latest_version(&self, release_type: &ReleaseType) -> Result<String>;
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,
Expand Down Expand Up @@ -217,7 +218,7 @@ impl SafeReleaseRepoActions 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);

Expand All @@ -235,7 +236,7 @@ impl SafeReleaseRepoActions 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()))
Expand All @@ -259,19 +260,12 @@ impl SafeReleaseRepoActions 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!(
"{}/{}-{}-{}.{}",
Expand Down
35 changes: 2 additions & 33 deletions tests/test_download_from_s3.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

use assert_fs::prelude::*;
use predicates::prelude::*;
use sn_releases::error::Error;
use semver::Version;
use sn_releases::{ArchiveType, Platform, ReleaseType, SafeReleaseRepoActions};

const FAUCET_VERSION: &str = "0.1.98";
Expand Down Expand Up @@ -36,7 +36,7 @@ async fn download_and_extract(
let archive_path = release_repo
.download_release_from_s3(
release_type,
version,
&Version::parse(version).unwrap(),
platform,
archive_type,
&download_dir,
Expand Down Expand Up @@ -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 SafeReleaseRepoActions>::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
///
Expand Down
81 changes: 0 additions & 81 deletions tests/test_get_latest_version.rs

This file was deleted.

0 comments on commit bb64146

Please sign in to comment.