From 1daa5fbb60bab135418595f3ac26b87321252047 Mon Sep 17 00:00:00 2001 From: Filip Tibell Date: Thu, 28 Mar 2024 16:24:28 +0100 Subject: [PATCH] Implement parsing of provider in tool ids, dont require provider arg for artifact source --- lib/sources/artifact.rs | 5 ++- lib/sources/source.rs | 16 +++------ lib/tool/id.rs | 74 ++++++++++++++++++++++++++++++++++++++--- lib/tool/spec.rs | 12 +++++-- lib/tool/util.rs | 6 +++- src/cli/add.rs | 10 ++---- src/cli/init.rs | 2 +- src/cli/install.rs | 10 ++---- src/cli/list.rs | 2 +- src/cli/self_update.rs | 12 ++----- src/cli/update.rs | 19 ++++------- 11 files changed, 109 insertions(+), 59 deletions(-) diff --git a/lib/sources/artifact.rs b/lib/sources/artifact.rs index 4fa1c93..346bc55 100644 --- a/lib/sources/artifact.rs +++ b/lib/sources/artifact.rs @@ -17,9 +17,12 @@ use super::{ /** An artifact provider supported by Rokit. + + The default provider is [`ArtifactProvider::GitHub`]. */ -#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] +#[derive(Debug, Default, Clone, Copy, PartialEq, Eq, Hash)] pub enum ArtifactProvider { + #[default] GitHub, } diff --git a/lib/sources/source.rs b/lib/sources/source.rs index c3797da..3e036a5 100644 --- a/lib/sources/source.rs +++ b/lib/sources/source.rs @@ -57,12 +57,8 @@ impl ArtifactSource { - If the latest release could not be fetched. */ - pub async fn get_latest_release( - &self, - provider: ArtifactProvider, - id: &ToolId, - ) -> RokitResult> { - Ok(match provider { + pub async fn get_latest_release(&self, id: &ToolId) -> RokitResult> { + Ok(match id.provider() { ArtifactProvider::GitHub => self.github.get_latest_release(id).await?, }) } @@ -74,12 +70,8 @@ impl ArtifactSource { - If the specific release could not be fetched. */ - pub async fn get_specific_release( - &self, - provider: ArtifactProvider, - spec: &ToolSpec, - ) -> RokitResult> { - Ok(match provider { + pub async fn get_specific_release(&self, spec: &ToolSpec) -> RokitResult> { + Ok(match spec.provider() { ArtifactProvider::GitHub => self.github.get_specific_release(spec).await?, }) } diff --git a/lib/tool/id.rs b/lib/tool/id.rs index b125bc6..60e79a8 100644 --- a/lib/tool/id.rs +++ b/lib/tool/id.rs @@ -4,6 +4,8 @@ use semver::Version; use serde_with::{DeserializeFromStr, SerializeDisplay}; use thiserror::Error; +use crate::sources::ArtifactProvider; + use super::{util::is_invalid_identifier, ToolAlias, ToolSpec}; /** @@ -15,6 +17,8 @@ pub enum ToolIdParseError { Empty, #[error("missing '/' separator")] MissingSeparator, + #[error("artifact provider '{0}' is invalid")] + InvalidProvider(String), #[error("author '{0}' is empty or invalid")] InvalidAuthor(String), #[error("name '{0}' is empty or invalid")] @@ -24,17 +28,23 @@ pub enum ToolIdParseError { /** A tool identifier, which includes the author and name of a tool. + Also includes the provider of the artifact, which by default is `GitHub`. + Used to uniquely identify a tool, but not its version. */ -#[derive( - Debug, Clone, PartialEq, Eq, PartialOrd, Ord, Hash, DeserializeFromStr, SerializeDisplay, -)] +#[derive(Debug, Clone, PartialEq, Eq, Hash, DeserializeFromStr, SerializeDisplay)] pub struct ToolId { + pub(super) provider: ArtifactProvider, pub(super) author: String, pub(super) name: String, } impl ToolId { + #[must_use] + pub fn provider(&self) -> ArtifactProvider { + self.provider + } + #[must_use] pub fn author(&self) -> &str { &self.author @@ -56,6 +66,20 @@ impl ToolId { } } +impl Ord for ToolId { + fn cmp(&self, other: &Self) -> std::cmp::Ordering { + self.author + .cmp(&other.author) + .then_with(|| self.name.cmp(&other.name)) + } +} + +impl PartialOrd for ToolId { + fn partial_cmp(&self, other: &Self) -> Option { + Some(self.cmp(other)) + } +} + impl FromStr for ToolId { type Err = ToolIdParseError; fn from_str(s: &str) -> Result { @@ -63,7 +87,16 @@ impl FromStr for ToolId { return Err(ToolIdParseError::Empty); } - let Some((before, after)) = s.split_once('/') else { + let (provider, after_provider) = match s.split_once(':') { + None => (ArtifactProvider::default(), s), + Some((left, right)) => { + let provider = ArtifactProvider::from_str(left) + .map_err(|e| ToolIdParseError::InvalidProvider(e.to_string()))?; + (provider, right) + } + }; + + let Some((before, after)) = after_provider.split_once('/') else { return Err(ToolIdParseError::MissingSeparator); }; @@ -78,6 +111,7 @@ impl FromStr for ToolId { } Ok(Self { + provider, author: before.to_string(), name: after.to_string(), }) @@ -96,6 +130,15 @@ mod tests { fn new_id(author: &str, name: &str) -> ToolId { ToolId { + provider: ArtifactProvider::default(), + author: author.to_string(), + name: name.to_string(), + } + } + + fn new_id_with_provider(provider: ArtifactProvider, author: &str, name: &str) -> ToolId { + ToolId { + provider, author: author.to_string(), name: name.to_string(), } @@ -136,6 +179,17 @@ mod tests { assert_eq!("a/b ".parse::().unwrap(), id); } + #[test] + fn parse_valid_provider() { + // Known provider strings should parse ok + assert!("github:a/b".parse::().is_ok()); + // The parsed ToolId should match the input + assert_eq!( + "github:a/b".parse::().unwrap(), + new_id_with_provider(ArtifactProvider::GitHub, "a", "b") + ); + } + #[test] fn parse_invalid_missing() { // Empty strings or parts should not be allowed @@ -151,4 +205,16 @@ mod tests { assert!("a/b/".parse::().is_err()); assert!("a/b/c".parse::().is_err()); } + + #[test] + fn parse_invalid_provider() { + // Empty provider should not be allowed + assert!(":a/b".parse::().is_err()); + assert!(":a/b".parse::().is_err()); + assert!(":a/b".parse::().is_err()); + // Unrecognized provider should not be allowed + assert!("unknown:a/b".parse::().is_err()); + assert!("hubgit:a/b".parse::().is_err()); + assert!("bitbab:a/b".parse::().is_err()); + } } diff --git a/lib/tool/spec.rs b/lib/tool/spec.rs index f681928..feed9bc 100644 --- a/lib/tool/spec.rs +++ b/lib/tool/spec.rs @@ -4,6 +4,8 @@ use semver::Version; use serde_with::{DeserializeFromStr, SerializeDisplay}; use thiserror::Error; +use crate::sources::ArtifactProvider; + use super::{util::is_invalid_identifier, ToolId, ToolIdParseError}; /** @@ -38,14 +40,19 @@ pub struct ToolSpec { } impl ToolSpec { + #[must_use] + pub fn provider(&self) -> ArtifactProvider { + self.id.provider() + } + #[must_use] pub fn author(&self) -> &str { - &self.id.author + self.id.author() } #[must_use] pub fn name(&self) -> &str { - &self.id.name + self.id.name() } #[must_use] @@ -115,6 +122,7 @@ mod tests { fn new_spec(author: &str, name: &str, version: &str) -> ToolSpec { ToolSpec { id: ToolId { + provider: ArtifactProvider::default(), author: author.to_string(), name: name.to_string(), }, diff --git a/lib/tool/util.rs b/lib/tool/util.rs index fa52fea..c846851 100644 --- a/lib/tool/util.rs +++ b/lib/tool/util.rs @@ -1,5 +1,9 @@ pub fn is_invalid_identifier(s: &str) -> bool { s.is_empty() // Must not be empty || s.chars().all(char::is_whitespace) // Must contain some information - || s.chars().any(|c| c == '/') // Must not contain the separator character + || s.chars().any(|c| + c == ':' // Must not contain the provider separator character + || c == '/' // Must not contain the author/name separator character + || c == '@' // Must not contain the version separator character + ) } diff --git a/src/cli/add.rs b/src/cli/add.rs index 6af0117..3935bcc 100644 --- a/src/cli/add.rs +++ b/src/cli/add.rs @@ -5,7 +5,7 @@ use console::style; use rokit::{ discovery::discover_all_manifests, manifests::RokitManifest, - sources::{Artifact, ArtifactProvider}, + sources::Artifact, storage::Home, tool::{ToolAlias, ToolId}, }; @@ -84,9 +84,7 @@ impl AddSubcommand { let pb = new_progress_bar("Fetching", 3, 1); let (spec, artifact) = match self.tool.clone() { ToolIdOrSpec::Spec(spec) => { - let artifacts = source - .get_specific_release(ArtifactProvider::GitHub, &spec) - .await?; + let artifacts = source.get_specific_release(&spec).await?; let artifact = Artifact::sort_by_system_compatibility(&artifacts) .first() .cloned() @@ -95,9 +93,7 @@ impl AddSubcommand { (spec, artifact) } ToolIdOrSpec::Id(id) => { - let artifacts = source - .get_latest_release(ArtifactProvider::GitHub, &id) - .await?; + let artifacts = source.get_latest_release(&id).await?; let artifact = Artifact::sort_by_system_compatibility(&artifacts) .first() .cloned() diff --git a/src/cli/init.rs b/src/cli/init.rs index cb4d78a..9148bc7 100644 --- a/src/cli/init.rs +++ b/src/cli/init.rs @@ -1,7 +1,7 @@ use anyhow::{bail, Context, Result}; use clap::Parser; - use console::style; + use rokit::{manifests::RokitManifest, storage::Home, system::current_dir}; use crate::util::{finish_progress_bar, new_progress_bar}; diff --git a/src/cli/install.rs b/src/cli/install.rs index eddf3be..95cf8a4 100644 --- a/src/cli/install.rs +++ b/src/cli/install.rs @@ -5,11 +5,7 @@ use clap::Parser; use console::style; use futures::{stream::FuturesUnordered, TryStreamExt}; -use rokit::{ - discovery::discover_all_manifests, - sources::{Artifact, ArtifactProvider}, - storage::Home, -}; +use rokit::{discovery::discover_all_manifests, sources::Artifact, storage::Home}; use crate::util::{finish_progress_bar, new_progress_bar, prompt_for_trust_specs}; @@ -84,9 +80,7 @@ impl InstallSubcommand { return anyhow::Ok(tool_spec); } - let artifacts = source - .get_specific_release(ArtifactProvider::GitHub, &tool_spec) - .await?; + let artifacts = source.get_specific_release(&tool_spec).await?; pb.inc(1); let artifact = Artifact::sort_by_system_compatibility(&artifacts) diff --git a/src/cli/list.rs b/src/cli/list.rs index ae6b996..723eda5 100644 --- a/src/cli/list.rs +++ b/src/cli/list.rs @@ -1,7 +1,7 @@ use anyhow::Result; use clap::Parser; - use console::style; + use rokit::{storage::Home, tool::ToolId}; /// Lists all existing tools managed by Rokit. diff --git a/src/cli/self_update.rs b/src/cli/self_update.rs index 360f171..85349f7 100644 --- a/src/cli/self_update.rs +++ b/src/cli/self_update.rs @@ -1,14 +1,10 @@ use anyhow::{bail, Context, Result}; use clap::Parser; - use console::style; -use rokit::{ - sources::{Artifact, ArtifactProvider}, - storage::Home, - tool::ToolId, -}; use semver::Version; +use rokit::{sources::Artifact, storage::Home, tool::ToolId}; + use crate::util::{finish_progress_bar, new_progress_bar}; /// Updates Rokit to the latest version. @@ -38,9 +34,7 @@ impl SelfUpdateSubcommand { pb.inc(1); pb.set_message("Fetching"); - let artifacts = source - .get_latest_release(ArtifactProvider::GitHub, &tool_id) - .await?; + let artifacts = 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::().unwrap(); diff --git a/src/cli/update.rs b/src/cli/update.rs index 813af2d..6cd27ec 100644 --- a/src/cli/update.rs +++ b/src/cli/update.rs @@ -1,13 +1,10 @@ use anyhow::{bail, Context, Result}; use clap::Parser; - use console::style; use futures::{stream::FuturesUnordered, TryStreamExt}; + use rokit::{ - discovery::discover_all_manifests, - manifests::RokitManifest, - sources::{Artifact, ArtifactProvider}, - storage::Home, + discovery::discover_all_manifests, manifests::RokitManifest, sources::Artifact, storage::Home, }; use crate::util::{finish_progress_bar, new_progress_bar, ToolAliasOrIdOrSpec, ToolIdOrSpec}; @@ -123,10 +120,8 @@ impl UpdateSubcommand { .map(|(alias, tool)| async { let (alias, id, artifacts) = match tool { ToolIdOrSpec::Spec(spec) => { - let artifacts = source - .get_specific_release(ArtifactProvider::GitHub, &spec) - .await - .with_context(|| { + let artifacts = + source.get_specific_release(&spec).await.with_context(|| { format!( "Failed to fetch release for '{spec}'!\ \nMake sure the given tool version exists." @@ -135,10 +130,8 @@ impl UpdateSubcommand { (alias, spec.id().clone(), artifacts) } ToolIdOrSpec::Id(id) => { - let artifacts = source - .get_latest_release(ArtifactProvider::GitHub, &id) - .await - .with_context(|| { + let artifacts = + source.get_latest_release(&id).await.with_context(|| { format!( "Failed to fetch latest release for '{id}'!\ \nMake sure the given tool identifier exists."