Skip to content

Commit

Permalink
Fix unnecessary clone and some naming
Browse files Browse the repository at this point in the history
  • Loading branch information
filiptibell committed Aug 18, 2024
1 parent 1086e3a commit 6f9ad43
Show file tree
Hide file tree
Showing 6 changed files with 36 additions and 35 deletions.
14 changes: 12 additions & 2 deletions lib/sources/artifact/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use crate::{
use super::{
decompression::decompress_gzip,
extraction::{extract_tar_file, extract_zip_file},
github::models::Asset,
github::models::GithubAsset,
ExtractError,
};

Expand All @@ -25,6 +25,16 @@ use self::util::split_filename_and_extensions;
pub use self::format::ArtifactFormat;
pub use self::provider::ArtifactProvider;

/**
A release found by Rokit, containing a list
of artifacts, and optionally a changelog.
*/
#[derive(Debug, Clone)]
pub struct Release {
pub changelog: Option<String>,
pub artifacts: Vec<Artifact>,
}

/**
An artifact found by Rokit, to be downloaded and installed.
*/
Expand All @@ -39,7 +49,7 @@ pub struct Artifact {
}

impl Artifact {
pub(crate) fn from_github_release_asset(asset: &Asset, spec: &ToolSpec) -> Self {
pub(crate) fn from_github_release_asset(asset: &GithubAsset, spec: &ToolSpec) -> Self {
let (name, extensions) = split_filename_and_extensions(&asset.name);
let format = ArtifactFormat::from_extensions(extensions);
Self {
Expand Down
29 changes: 13 additions & 16 deletions lib/sources/github/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,14 @@ use reqwest::{

use crate::tool::{ToolId, ToolSpec};

use super::{client::create_client, source::ReleaseArtifact, Artifact, ArtifactProvider};
use super::{client::create_client, Artifact, ArtifactProvider, Release};

const BASE_URL: &str = "https://api.github.com";

pub mod models;
mod result;

use self::models::Release;
use self::models::GithubRelease;

pub use self::result::{GithubError, GithubResult};

Expand Down Expand Up @@ -127,7 +127,7 @@ impl GithubProvider {
Fetches the latest release for a given tool.
*/
#[instrument(skip(self), fields(%tool_id), level = "debug")]
pub async fn get_latest_release(&self, tool_id: &ToolId) -> GithubResult<ReleaseArtifact> {
pub async fn get_latest_release(&self, tool_id: &ToolId) -> GithubResult<Release> {
debug!(id = %tool_id, "fetching latest release for tool");

let url = format!(
Expand All @@ -136,7 +136,7 @@ impl GithubProvider {
repo = tool_id.name(),
);

let release: Release = match self.get_json(&url).await {
let release: GithubRelease = match self.get_json(&url).await {
Err(e) if is_404(&e) => {
return Err(GithubError::LatestReleaseNotFound(tool_id.clone().into()));
}
Expand All @@ -151,20 +151,17 @@ impl GithubProvider {
.map_err(|e| GithubError::Other(e.to_string()))?;

let tool_spec: ToolSpec = (tool_id.clone(), version).into();
Ok(ReleaseArtifact {
artifacts: artifacts_from_release(&release.clone(), &tool_spec),
changelog: release.changelog,
Ok(Release {
changelog: release.changelog.clone(),
artifacts: artifacts_from_release(&release, &tool_spec),
})
}

/**
Fetches a specific release for a given tool.
*/
#[instrument(skip(self), fields(%tool_spec), level = "debug")]
pub async fn get_specific_release(
&self,
tool_spec: &ToolSpec,
) -> GithubResult<ReleaseArtifact> {
pub async fn get_specific_release(&self, tool_spec: &ToolSpec) -> GithubResult<Release> {
debug!(spec = %tool_spec, "fetching release for tool");

let url_with_prefix = format!(
Expand All @@ -180,7 +177,7 @@ impl GithubProvider {
tag = tool_spec.version(),
);

let release: Release = match self.get_json(&url_with_prefix).await {
let release: GithubRelease = match self.get_json(&url_with_prefix).await {
Err(e) if is_404(&e) => match self.get_json(&url_without_prefix).await {
Err(e) if is_404(&e) => {
return Err(GithubError::ReleaseNotFound(tool_spec.clone().into()));
Expand All @@ -192,9 +189,9 @@ impl GithubProvider {
Ok(r) => r,
};

Ok(ReleaseArtifact {
artifacts: artifacts_from_release(&release.clone(), tool_spec),
changelog: release.changelog,
Ok(Release {
changelog: release.changelog.clone(),
artifacts: artifacts_from_release(&release, tool_spec),
})
}

Expand Down Expand Up @@ -241,7 +238,7 @@ fn is_unauthenticated(err: &GithubError) -> bool {
false
}

fn artifacts_from_release(release: &Release, spec: &ToolSpec) -> Vec<Artifact> {
fn artifacts_from_release(release: &GithubRelease, spec: &ToolSpec) -> Vec<Artifact> {
release
.assets
.iter()
Expand Down
6 changes: 3 additions & 3 deletions lib/sources/github/models.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,16 @@ use serde::Deserialize;
use url::Url;

#[derive(Debug, Clone, Deserialize)]
pub struct Release {
pub assets: Vec<Asset>,
pub struct GithubRelease {
pub assets: Vec<GithubAsset>,
pub tag_name: String,
pub prerelease: bool,
#[serde(rename = "body")]
pub changelog: Option<String>,
}

#[derive(Debug, Clone, Deserialize)]
pub struct Asset {
pub struct GithubAsset {
pub id: u64,
pub url: Url,
pub name: String,
Expand Down
2 changes: 1 addition & 1 deletion lib/sources/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,6 @@ mod source;

pub mod github;

pub use self::artifact::{Artifact, ArtifactFormat, ArtifactProvider};
pub use self::artifact::{Artifact, ArtifactFormat, ArtifactProvider, Release};
pub use self::extraction::ExtractError;
pub use self::source::ArtifactSource;
12 changes: 3 additions & 9 deletions lib/sources/source.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use crate::{
tool::{ToolId, ToolSpec},
};

use super::{github::GithubProvider, Artifact, ArtifactProvider};
use super::{github::GithubProvider, Artifact, ArtifactProvider, Release};

/**
A source for artifacts.
Expand All @@ -17,12 +17,6 @@ pub struct ArtifactSource {
github: GithubProvider,
}

#[derive(Debug, Clone)]
pub struct ReleaseArtifact {
pub changelog: Option<String>,
pub artifacts: Vec<Artifact>,
}

impl ArtifactSource {
/**
Creates a new artifact source.
Expand Down Expand Up @@ -63,7 +57,7 @@ impl ArtifactSource {
- If the latest release could not be fetched.
*/
pub async fn get_latest_release(&self, id: &ToolId) -> RokitResult<ReleaseArtifact> {
pub async fn get_latest_release(&self, id: &ToolId) -> RokitResult<Release> {
Ok(match id.provider() {
ArtifactProvider::GitHub => self.github.get_latest_release(id).await?,
})
Expand All @@ -76,7 +70,7 @@ impl ArtifactSource {
- If the specific release could not be fetched.
*/
pub async fn get_specific_release(&self, spec: &ToolSpec) -> RokitResult<ReleaseArtifact> {
pub async fn get_specific_release(&self, spec: &ToolSpec) -> RokitResult<Release> {
Ok(match spec.provider() {
ArtifactProvider::GitHub => self.github.get_specific_release(spec).await?,
})
Expand Down
8 changes: 4 additions & 4 deletions src/cli/self_update.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,11 +43,11 @@ impl SelfUpdateSubcommand {
pt.task_completed();
pt.update_message("Fetching");

let release_artifact = source.get_latest_release(&tool_id).await?;
let release = source.get_latest_release(&tool_id).await?;

// Skip updating if we are already on the latest version
let version_current = env!("CARGO_PKG_VERSION").parse::<Version>().unwrap();
let version_latest = release_artifact
let version_latest = release
.artifacts
.first()
.unwrap()
Expand All @@ -70,7 +70,7 @@ impl SelfUpdateSubcommand {
pt.task_completed();
pt.update_message("Downloading");

let artifact = find_most_compatible_artifact(&release_artifact.artifacts, &tool_id)
let artifact = find_most_compatible_artifact(&release.artifacts, &tool_id)
.context("No compatible Rokit artifact was found (WAT???)")?;
let artifact_contents = source
.download_artifact_contents(&artifact)
Expand Down Expand Up @@ -107,7 +107,7 @@ impl SelfUpdateSubcommand {
pt.finish_with_message(msg);

// If there is a changelog, and the user wants to see it, show it
if let Some(changelog) = release_artifact.changelog {
if let Some(changelog) = release.changelog {
let to_show_changelog = Confirm::with_theme(&ColorfulTheme {
active_item_prefix: style("📋 ".to_string()),
prompt_style: Style::new(),
Expand Down

0 comments on commit 6f9ad43

Please sign in to comment.