de418e8 return Result<> instead of Result<Option<>> for find_entry — radicle-git

radicle-git

rad:z6cFWeWpnZNHh9rUW8phgA3b5yGt

Git libraries for Radicle

return Result<> instead of Result<Option<>> for find_entry

Han Xu committed 3 years ago

commit de418e811e72014d91e9d17c346b45f60db35cef
parent 016ece6

3 files changed +63 -76

modified radicle-surf/src/fs.rs

@@ -29,7 +29,6 @@ use std::{

 use git2::Blob;
 use radicle_git_ext::{is_not_found_err, Oid};
 use radicle_std_ext::result::ResultExt as _;
 use crate::{Repository, Revision};

@@ -43,17 +42,15 @@ pub mod error {

         #[error(transparent)]
         Git(#[from] git2::Error),
         #[error(transparent)]
         Entry(#[from] Entry),
         #[error(transparent)]
         File(#[from] File),
         #[error("the path {0} is not valid")]
         InvalidPath(PathBuf),
     }
     #[derive(Debug, Error, PartialEq, Eq)]
     pub enum Entry {
         #[error("{0}: {1}")]
         InvalidType(PathBuf, &'static str),
         #[error("the entry name was not valid UTF-8")]
         Utf8Error,
         #[error("the path {0} not found")]
         PathNotFound(PathBuf),
     }
     #[derive(Debug, Error, PartialEq)]

@@ -258,15 +255,18 @@ impl Entry {

     pub(crate) fn from_entry(
         entry: &git2::TreeEntry,
         path: PathBuf,
     ) -> Result<Option<Self>, error::Entry> {
         let name = entry.name().ok_or(error::Entry::Utf8Error)?.to_string();
     ) -> Result<Self, error::Directory> {
         let name = entry.name().ok_or(error::Directory::Utf8Error)?.to_string();
         let id = entry.id().into();
         Ok(entry.kind().and_then(|kind| match kind {
             git2::ObjectType::Tree => Some(Self::Directory(Directory::new(name, path, id))),
             git2::ObjectType::Blob => Some(Self::File(File::new(name, path, id))),
             _ => None,
         }))
         match entry.kind() {
             Some(git2::ObjectType::Tree) => Ok(Self::Directory(Directory::new(name, path, id))),
             Some(git2::ObjectType::Blob) => Ok(Self::File(File::new(name, path, id))),
             _ => Err(error::Directory::InvalidType(
                 path,
                 "Entry must be a tree or a blob",
             )),
         }
     }
 }

@@ -361,7 +361,7 @@ impl Directory {

         // empty for the first level.
         tree.walk(git2::TreeWalkMode::PreOrder, |_entry_path, entry| {
             match Entry::from_entry(entry, path.clone()) {
                 Ok(Some(entry)) => match entry {
                 Ok(entry) => match entry {
                     Entry::File(_) => {
                         entries.insert(entry.name().clone(), entry);
                         git2::TreeWalkResult::Ok

@@ -372,7 +372,6 @@ impl Directory {

                         git2::TreeWalkResult::Skip
                     },
                 },
                 Ok(None) => git2::TreeWalkResult::Skip,
                 Err(err) => {
                     error = Some(err);
                     git2::TreeWalkResult::Abort

@@ -381,72 +380,66 @@ impl Directory {

         })?;
         match error {
             Some(err) => Err(err.into()),
             Some(err) => Err(err),
             None => Ok(Entries { listing: entries }),
         }
     }
     /// Find the [`Entry`] found at `path`, if it exists.
     pub fn find_entry<P>(
         &self,
         path: &P,
         repo: &Repository,
     ) -> Result<Option<Entry>, error::Directory>
     /// Find the [`Entry`] found at a non-empty `path`, if it exists.
     pub fn find_entry<P>(&self, path: &P, repo: &Repository) -> Result<Entry, error::Directory>
     where
         P: AsRef<Path>,
     {
         // Search the path in git2 tree.
         let path = path.as_ref();
         let git2_tree = repo.find_tree(self.id)?;
         let entry = git2_tree
             .get_path(path)
             .map(Some)
             .or_matches::<git2::Error, _, _>(is_not_found_err, || Ok(None))?;
         let entry = git2_tree.get_path(path).map_err(|e| {
             if is_not_found_err(&e) {
                 error::Directory::PathNotFound(path.to_path_buf())
             } else {
                 error::Directory::Git(e)
             }
         })?;
         let parent = path
             .parent()
             .ok_or_else(|| error::Directory::InvalidPath(path.to_path_buf()))?;
         let root_path = self.path().join(parent);
         Ok(entry
             .and_then(|entry| Entry::from_entry(&entry, root_path.to_path_buf()).transpose())
             .transpose()
             .unwrap())
         Entry::from_entry(&entry, root_path)
     }
     /// Find the `Oid`, for a [`File`], found at `path`, if it exists.
     pub fn find_file<P>(
         &self,
         path: &P,
         repo: &Repository,
     ) -> Result<Option<File>, error::Directory>
     pub fn find_file<P>(&self, path: &P, repo: &Repository) -> Result<File, error::Directory>
     where
         P: AsRef<Path>,
     {
         Ok(match self.find_entry(path, repo)? {
             Some(Entry::File(file)) => Some(file),
             _ => None,
         })
         match self.find_entry(path, repo)? {
             Entry::File(file) => Ok(file),
             _ => Err(error::Directory::InvalidType(
                 path.as_ref().to_path_buf(),
                 "Entry must be a file",
             )),
         }
     }
     /// Find the `Directory` found at `path`, if it exists.
     ///
     /// If `path` is `ROOT_DIR` (i.e. an empty path), returns self.
     pub fn find_directory<P>(
         &self,
         path: &P,
         repo: &Repository,
     ) -> Result<Option<Self>, error::Directory>
     pub fn find_directory<P>(&self, path: &P, repo: &Repository) -> Result<Self, error::Directory>
     where
         P: AsRef<Path>,
     {
         if path.as_ref() == Path::new(ROOT_DIR) {
             return Ok(Some(self.clone()));
             return Ok(self.clone());
         }
         Ok(match self.find_entry(path, repo)? {
             Some(Entry::Directory(d)) => Some(d),
             _ => None,
         })
         match self.find_entry(path, repo)? {
             Entry::Directory(d) => Ok(d),
             _ => Err(error::Directory::InvalidType(
                 path.as_ref().to_path_buf(),
                 "Entry must be a directory",
             )),
         }
     }
     // TODO(fintan): This is going to be a bit trickier so going to leave it out for

modified radicle-surf/src/repo.rs

@@ -258,15 +258,13 @@ impl Repository {

         path: &P,
     ) -> Result<Directory, Error> {
         let root = self.root_dir(commit)?;
         root.find_directory(path, self)?
             .ok_or_else(|| Error::PathNotFound(path.as_ref().to_path_buf()))
         Ok(root.find_directory(path, self)?)
     }
     /// Returns a [`File`] for `path` in `commit`.
     pub fn file<C: ToCommit, P: AsRef<Path>>(&self, commit: C, path: &P) -> Result<File, Error> {
         let root = self.root_dir(commit)?;
         root.find_file(path, self)?
             .ok_or_else(|| Error::PathNotFound(path.as_ref().to_path_buf()))
         Ok(root.find_file(path, self)?)
     }
     /// Returns a [`Tree`] for `path` in `commit`.

modified radicle-surf/t/src/file_system.rs

@@ -22,32 +22,37 @@ mod directory {

         // find_entry for a file.
         let path = Path::new("src/memory.rs");
         let entry = root.find_entry(&path, &repo).unwrap();
         assert!(matches!(entry, Some(fs::Entry::File(_))));
         assert!(matches!(entry, fs::Entry::File(_)));
         // find_entry for a directory.
         let path = Path::new("this/is/a/really/deeply/nested/directory/tree");
         let entry = root.find_entry(&path, &repo).unwrap();
         assert!(matches!(entry, Some(fs::Entry::Directory(_))));
         assert!(matches!(entry, fs::Entry::Directory(_)));
         // find_entry for a non-leaf directory and its relative path.
         let path = Path::new("text");
         let entry = root.find_entry(&path, &repo).unwrap();
         assert!(matches!(entry, Some(fs::Entry::Directory(_))));
         if let Some(fs::Entry::Directory(sub_dir)) = entry {
         assert!(matches!(entry, fs::Entry::Directory(_)));
         if let fs::Entry::Directory(sub_dir) = entry {
             let inner_path = Path::new("garden.txt");
             let inner_entry = sub_dir.find_entry(&inner_path, &repo).unwrap();
             assert!(matches!(inner_entry, Some(fs::Entry::File(_))));
             assert!(matches!(inner_entry, fs::Entry::File(_)));
         }
         // find_entry for non-existing file
         let path = Path::new("this/is/a/really/missing_file");
         let result = root.find_entry(&path, &repo).unwrap();
         assert_eq!(result, None);
         let result = root.find_entry(&path, &repo);
         assert!(matches!(result, Err(fs::error::Directory::PathNotFound(_))));
         // find_entry for absolute path: fail.
         let path = Path::new("/src/memory.rs");
         let result = root.find_entry(&path, &repo);
         assert!(result.is_err());
         // find entry for an empty path
         let path = Path::new("");
         let result = root.find_entry(&path, &repo);
         assert!(result.is_err());
     }
     #[test]

@@ -59,10 +64,7 @@ mod directory {

             .unwrap();
         // Assert that we can find the memory.rs file!
         assert!(root
             .find_file(&Path::new("src/memory.rs"), &repo)
             .unwrap()
             .is_some());
         assert!(root.find_file(&Path::new("src/memory.rs"), &repo).is_ok());
         let root_contents: Vec<Entry> = root.entries(&repo).unwrap().collect();
         assert_eq!(root_contents.len(), 7);

@@ -82,10 +84,7 @@ mod directory {

         assert_eq!(root_contents[5].name(), "text");
         assert_eq!(root_contents[6].name(), "this");
         let src = root
             .find_directory(&Path::new("src"), &repo)
             .unwrap()
             .unwrap();
         let src = root.find_directory(&Path::new("src"), &repo).unwrap();
         assert_eq!(src.path(), Path::new("src").to_path_buf());
         let src_contents: Vec<Entry> = src.entries(&repo).unwrap().collect();
         assert_eq!(src_contents.len(), 3);

@@ -109,8 +108,8 @@ mod directory {

         let path = Path::new("src");
         let entry = root.find_entry(&path, &repo).unwrap();
         assert!(matches!(entry, Some(fs::Entry::Directory(_))));
         if let Some(fs::Entry::Directory(d)) = entry {
         assert!(matches!(entry, fs::Entry::Directory(_)));
         if let fs::Entry::Directory(d) = entry {
             assert_eq!(16297, d.size(&repo).unwrap());
         }
     }

@@ -120,7 +119,7 @@ mod directory {

         let repo = Repository::open(GIT_PLATINUM).unwrap();
         let branch = Branch::local(refname!("dev"));
         let root = repo.root_dir(&branch).unwrap();
         let dir = root.find_directory(&"this/is", &repo).unwrap().unwrap();
         let dir = root.find_directory(&"this/is", &repo).unwrap();
         let last_commit = repo.last_commit(&dir.path(), &branch).unwrap().unwrap();
         assert_eq!(
             last_commit.id.to_string(),

@@ -135,10 +134,7 @@ mod directory {

         let root = repo.root_dir(&branch).unwrap();
         // Find a file with "\" in its name.
         let f = root
             .find_file(&"special/faux\\path", &repo)
             .unwrap()
             .unwrap();
         let f = root.find_file(&"special/faux\\path", &repo).unwrap();
         let last_commit = repo.last_commit(&f.path(), &branch).unwrap().unwrap();
         assert_eq!(
             last_commit.id.to_string(),