From 31fb8c89fefd3167b8b928cd75ddc008da60b434 Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Sun, 7 Feb 2021 12:13:19 +0100 Subject: Remove `Artifact` type almost entirely This patch removes the `Artifact` type almost entirely. The type parsed its path to extract name and version. This information was _never_ needed, only resulting in the restriction that the path must be parsable (which technically is not required at all). This patch boils down the `Artifact` type to the absolute minimum, as a baseline for its complete removal. Signed-off-by: Matthias Beyer Tested-by: Matthias Beyer --- src/filestore/artifact.rs | 154 ++-------------------------------------------- src/filestore/path.rs | 24 -------- 2 files changed, 4 insertions(+), 174 deletions(-) (limited to 'src/filestore') diff --git a/src/filestore/artifact.rs b/src/filestore/artifact.rs index 1dc80a5..2f3477a 100644 --- a/src/filestore/artifact.rs +++ b/src/filestore/artifact.rs @@ -8,169 +8,23 @@ // SPDX-License-Identifier: EPL-2.0 // -use std::cmp::Ordering; - -use anyhow::anyhow; -use anyhow::Context; -use anyhow::Error; use anyhow::Result; use getset::Getters; -use pom::parser::Parser as PomParser; -use crate::filestore::path::*; -use crate::package::PackageName; -use crate::package::PackageVersion; +use crate::filestore::path::ArtifactPath; +use crate::filestore::path::StoreRoot; -#[derive(Clone, PartialEq, Eq, Debug, Getters)] +#[derive(Clone, PartialEq, Eq, PartialOrd, Ord, Debug, Getters)] pub struct Artifact { #[getset(get = "pub")] path: ArtifactPath, - - #[getset(get = "pub")] - name: PackageName, - - #[getset(get = "pub")] - version: PackageVersion, -} - -impl PartialOrd for Artifact { - fn partial_cmp(&self, other: &Artifact) -> Option { - self.version.partial_cmp(&other.version) - } -} - -impl Ord for Artifact { - fn cmp(&self, other: &Self) -> Ordering { - self.version.cmp(&other.version) - } } impl Artifact { - pub fn load(root: &StoreRoot, path: ArtifactPath) -> Result { - let joined_fullpath = root.join(&path)?; - let (name, version) = Self::parse_path(&joined_fullpath) - .with_context(|| anyhow!("Pathing artifact path: '{}'", joined_fullpath.display()))?; - + pub fn load(_root: &StoreRoot, path: ArtifactPath) -> Result { Ok(Artifact { path, - name, - version, }) } - - fn parse_path(path: &FullArtifactPath) -> Result<(PackageName, PackageVersion)> { - path.file_stem() - .ok_or_else(|| anyhow!("Cannot get filename from {}", path.display()))? - .to_owned() - .into_string() - .map_err(|_| anyhow!("Internal conversion of '{}' to UTF-8", path.display())) - .and_then(|s| Self::parser().parse(s.as_bytes()).map_err(Error::from)) - } - - /// Construct a parser that parses a Vec into (PackageName, PackageVersion) - fn parser<'a>() -> PomParser<'a, u8, (PackageName, PackageVersion)> { - (PackageName::parser() + crate::util::parser::dash() + PackageVersion::parser()) - .map(|((name, _), vers)| (name, vers)) - } } -#[cfg(test)] -mod tests { - use super::*; - use crate::filestore::path::ArtifactPath; - use crate::filestore::path::StoreRoot; - use crate::package::tests::pname; - use crate::package::tests::pversion; - use std::path::PathBuf; - - #[test] - fn test_parser_one_letter_name() { - let p = ArtifactPath::new_unchecked(PathBuf::from("a-1.ext")); - let root = StoreRoot::new_unchecked(PathBuf::from("/")); - let r = Artifact::parse_path(&root.join_unchecked(&p)); - - assert!(r.is_ok(), "Expected to be Ok(_): {:?}", r); - let (name, version) = r.unwrap(); - - assert_eq!(name, pname("a")); - assert_eq!(version, pversion("1")); - } - - #[test] - fn test_parser_multi_letter_name() { - let p = ArtifactPath::new_unchecked(PathBuf::from("foo-1.ext")); - let root = StoreRoot::new_unchecked(PathBuf::from("/")); - let r = Artifact::parse_path(&root.join_unchecked(&p)); - - assert!(r.is_ok(), "Expected to be Ok(_): {:?}", r); - let (name, version) = r.unwrap(); - - assert_eq!(name, pname("foo")); - assert_eq!(version, pversion("1")); - } - - #[test] - fn test_parser_multi_char_version() { - let p = ArtifactPath::new_unchecked(PathBuf::from("foo-1123.ext")); - let root = StoreRoot::new_unchecked(PathBuf::from("/")); - let r = Artifact::parse_path(&root.join_unchecked(&p)); - - assert!(r.is_ok(), "Expected to be Ok(_): {:?}", r); - let (name, version) = r.unwrap(); - - assert_eq!(name, pname("foo")); - assert_eq!(version, pversion("1123")); - } - - #[test] - fn test_parser_multi_char_version_dashed() { - let p = ArtifactPath::new_unchecked(PathBuf::from("foo-1-1-2-3.ext")); - let root = StoreRoot::new_unchecked(PathBuf::from("/")); - let r = Artifact::parse_path(&root.join_unchecked(&p)); - - assert!(r.is_ok(), "Expected to be Ok(_): {:?}", r); - let (name, version) = r.unwrap(); - - assert_eq!(name, pname("foo")); - assert_eq!(version, pversion("1-1-2-3")); - } - - #[test] - fn test_parser_multi_char_version_dashed_and_dotted() { - let p = ArtifactPath::new_unchecked(PathBuf::from("foo-1-1.2-3.ext")); - let root = StoreRoot::new_unchecked(PathBuf::from("/")); - let r = Artifact::parse_path(&root.join_unchecked(&p)); - - assert!(r.is_ok(), "Expected to be Ok(_): {:?}", r); - let (name, version) = r.unwrap(); - - assert_eq!(name, pname("foo")); - assert_eq!(version, pversion("1-1.2-3")); - } - - #[test] - fn test_parser_alnum_version() { - let p = ArtifactPath::new_unchecked(PathBuf::from("foo-1-1.2a3.ext")); - let root = StoreRoot::new_unchecked(PathBuf::from("/")); - let r = Artifact::parse_path(&root.join_unchecked(&p)); - - assert!(r.is_ok(), "Expected to be Ok(_): {:?}", r); - let (name, version) = r.unwrap(); - - assert_eq!(name, pname("foo")); - assert_eq!(version, pversion("1-1.2a3")); - } - - #[test] - fn test_parser_package_name_with_number() { - let p = ArtifactPath::new_unchecked(PathBuf::from("foo2-1-1.2a3.ext")); - let root = StoreRoot::new_unchecked(PathBuf::from("/")); - let r = Artifact::parse_path(&root.join_unchecked(&p)); - - assert!(r.is_ok(), "Expected to be Ok(_): {:?}", r); - let (name, version) = r.unwrap(); - - assert_eq!(name, pname("foo2")); - assert_eq!(version, pversion("1-1.2a3")); - } -} diff --git a/src/filestore/path.rs b/src/filestore/path.rs index cfa999f..c38c114 100644 --- a/src/filestore/path.rs +++ b/src/filestore/path.rs @@ -42,16 +42,6 @@ impl StoreRoot { } } - /// Unchecked variant of StoreRoot::new() - /// - /// Because StoreRoot::new() accesses the filesystem, this method is necessary to construct an - /// object of StoreRoot for a non-existing path, so that we can test its implementation without - /// the need to create objects on the filesystem. - #[cfg(test)] - pub fn new_unchecked(root: PathBuf) -> Self { - StoreRoot(root) - } - pub fn join<'a>(&'a self, ap: &'a ArtifactPath) -> Result> { let join = self.0.join(&ap.0); @@ -64,16 +54,6 @@ impl StoreRoot { } } - /// Unchecked variant of StoreRoot::join() - /// - /// This function is needed (like StoreRoot::new_unchecked()) to perform a construction of - /// FullArtifactPath like StoreRoot::join() in without its side effects (accessing the - /// filesystem) for testing purposes - #[cfg(test)] - pub fn join_unchecked<'a>(&'a self, ap: &'a ArtifactPath) -> FullArtifactPath<'a> { - FullArtifactPath(self, ap) - } - pub(in crate::filestore) fn is_dir(&self, subpath: &Path) -> bool { self.0.join(subpath).is_dir() } @@ -171,10 +151,6 @@ impl<'a> FullArtifactPath<'a> { FullArtifactPathDisplay(self.0, self.1) } - pub(in crate::filestore) fn file_stem(&self) -> Option<&OsStr> { - self.1 .0.file_stem() - } - pub async fn read(self) -> Result> { tokio::fs::read(self.joined()) .await -- cgit v1.2.3