Don't nest options inside results

Signed-off-by: Alexis Sellier <alexis@radicle.xyz>

radicle/src/git.rs

@@ -12,7 +12,9 @@ use crate::storage;
 use crate::storage::refs::Refs;
 use crate::storage::RemoteId;
 
+pub use ext::is_not_found_err;
 pub use ext::Error;
+pub use ext::NotFound;
 pub use ext::Oid;
 pub use git2 as raw;
 pub use git_ref_format as fmt;

radicle/src/identity/project.rs

@@ -60,6 +60,7 @@ impl DocError {
         match self {
             Self::NotFound(_) => true,
             Self::Git(git::Error::NotFound(_)) => true,
+            Self::Git(git::Error::Git(e)) if git::is_not_found_err(e) => true,
             _ => false,
         }
     }
@@ -358,8 +359,7 @@ impl Doc<Unverified> {
 impl<V> Doc<V> {
     pub fn head<R: ReadRepository>(remote: &RemoteId, repo: &R) -> Result<Oid, DocError> {
         let head = &git::refname!("heads").join(&*git::refs::IDENTITY_BRANCH);
-        repo.reference_oid(remote, head)?
+        repo.reference_oid(remote, head).map_err(DocError::from)
-            .ok_or_else(|| DocError::NotFound(head.to_owned()))
     }
 }
 
@@ -483,12 +483,29 @@ mod test {
     use crate::rad;
     use crate::storage::git::Storage;
     use crate::storage::{ReadStorage, WriteStorage};
+    use crate::test::arbitrary;
     use crate::test::fixtures;
     use crate::test::signer::MockSigner;
 
     use super::*;
     use quickcheck_macros::quickcheck;
 
+    #[test]
+    fn test_not_found() {
+        let tempdir = tempfile::tempdir().unwrap();
+        let storage = Storage::open(tempdir.path().join("storage")).unwrap();
+        let remote = arbitrary::gen::<RemoteId>(1);
+        let proj = arbitrary::gen::<Id>(1);
+        let repo = storage.repository(proj).unwrap();
+        let oid = git2::Oid::from_str("2d52a53ce5e4f141148a5f770cfd3ead2d6a45b8").unwrap();
+
+        let err = Doc::<Unverified>::head(&remote, &repo).unwrap_err();
+        assert!(dbg!(err).is_not_found());
+
+        let err = Doc::load_at(oid.into(), &repo).unwrap_err();
+        assert!(err.is_not_found());
+    }
+
     #[test]
     fn test_valid_identity() {
         let tempdir = tempfile::tempdir().unwrap();

radicle/src/rad.rs

@@ -89,8 +89,12 @@ pub fn init<G: Signer, S: storage::WriteStorage>(
 
 #[derive(Error, Debug)]
 pub enum ForkError {
+    #[error("ref string: {0}")]
+    RefString(#[from] git::fmt::Error),
     #[error("git: {0}")]
-    Git(#[from] git2::Error),
+    GitRaw(#[from] git2::Error),
+    #[error("git: {0}")]
+    Git(#[from] git::Error),
     #[error("storage: {0}")]
     Storage(#[from] storage::Error),
     #[error("project `{0}` was not found in storage")]
@@ -168,12 +172,8 @@ pub fn fork<G: Signer, S: storage::WriteStorage>(
         let mut heads = Vec::new();
         for delegate in project.delegates.iter() {
             let name = format!("heads/{}", &project.default_branch);
-            let refname = git::RefString::try_from(name.as_str()).unwrap();
+            let refname = git::RefString::try_from(name.as_str())?;
-            let r = repository
+            let r = repository.reference_oid(&delegate.id, &refname)?.into();
-                .reference(&delegate.id, &refname)?
-                .unwrap()
-                .target()
-                .unwrap();
 
             heads.push(r);
         }

radicle/src/storage.rs

@@ -246,14 +246,10 @@ pub trait ReadRepository {
         &self,
         remote: &RemoteId,
         reference: &RefStr,
-    ) -> Result<Option<git2::Reference>, git2::Error>;
+    ) -> Result<git2::Reference, git_ext::Error>;
     fn commit(&self, oid: Oid) -> Result<Option<git2::Commit>, git2::Error>;
     fn revwalk(&self, head: Oid) -> Result<git2::Revwalk, git2::Error>;
-    fn reference_oid(
+    fn reference_oid(&self, remote: &RemoteId, reference: &RefStr) -> Result<Oid, git_ext::Error>;
-        &self,
-        remote: &RemoteId,
-        reference: &RefStr,
-    ) -> Result<Option<Oid>, git2::Error>;
     fn references(&self, remote: &RemoteId) -> Result<Refs, Error>;
     fn remote(&self, remote: &RemoteId) -> Result<Remote<Verified>, refs::Error>;
     fn remotes(&self) -> Result<Remotes<Verified>, refs::Error>;

radicle/src/storage/git.rs

@@ -417,16 +417,11 @@ impl ReadRepository for Repository {
         &self,
         remote: &RemoteId,
         name: &git::RefStr,
-    ) -> Result<Option<git2::Reference>, git2::Error> {
+    ) -> Result<git2::Reference, git::Error> {
         let name = name.strip_prefix(git::refname!("refs")).unwrap_or(name);
         let name = format!("refs/remotes/{remote}/{name}");
-        self.backend.find_reference(&name).map(Some).or_else(|e| {
+
-            if git::ext::is_not_found_err(&e) {
+        self.backend.find_reference(&name).map_err(git::Error::from)
-                Ok(None)
-            } else {
-                Err(e)
-            }
-        })
     }
 
     fn commit(&self, oid: Oid) -> Result<Option<git2::Commit>, git2::Error> {
@@ -446,13 +441,13 @@ impl ReadRepository for Repository {
         Ok(revwalk)
     }
 
-    fn reference_oid(
+    fn reference_oid(&self, remote: &RemoteId, reference: &git::RefStr) -> Result<Oid, git::Error> {
-        &self,
+        let oid = self
-        remote: &RemoteId,
+            .reference(remote, reference)?
-        reference: &git::RefStr,
+            .target()
-    ) -> Result<Option<Oid>, git2::Error> {
+            .ok_or(git::Error::NotFound(git::NotFound::NoRefTarget))?;
-        let reference = self.reference(remote, reference)?;
+
-        Ok(reference.and_then(|r| r.target().map(|o| o.into())))
+        Ok(oid.into())
     }
 
     fn remote(&self, remote: &RemoteId) -> Result<Remote<Verified>, refs::Error> {
@@ -752,10 +747,10 @@ mod tests {
         for remote in remotes {
             let (id, _) = remote.unwrap();
             let alice_repo = alice.repository(proj).unwrap();
-            let alice_oid = alice_repo.reference(&id, &refname).unwrap().unwrap();
+            let alice_oid = alice_repo.reference(&id, &refname).unwrap();
 
             let bob_repo = bob.repository(proj).unwrap();
-            let bob_oid = bob_repo.reference(&id, &refname).unwrap().unwrap();
+            let bob_oid = bob_repo.reference(&id, &refname).unwrap();
 
             assert_eq!(alice_oid.target(), bob_oid.target());
         }
@@ -805,7 +800,7 @@ mod tests {
 
         // Bob's storage is updated.
         let bob_repo = bob.repository(proj_id).unwrap();
-        let bob_master = bob_repo.reference(alice_id, &refname).unwrap().unwrap();
+        let bob_master = bob_repo.reference(alice_id, &refname).unwrap();
 
         assert_eq!(bob_master.target().unwrap(), alice_head);
     }

radicle/src/storage/refs.rs

@@ -45,8 +45,17 @@ pub enum Error {
     Git(#[from] git2::Error),
     #[error(transparent)]
     GitExt(#[from] git_ext::Error),
-    #[error("refs were not found")]
+}
-    NotFound,
+
+impl Error {
+    /// Whether this error is caused by a reference not being found.
+    pub fn is_not_found(&self) -> bool {
+        match self {
+            Self::GitExt(git::Error::NotFound(_)) => true,
+            Self::GitExt(git::Error::Git(e)) if git::is_not_found_err(e) => true,
+            _ => false,
+        }
+    }
 }
 
 /// The published state of a local repository.
@@ -217,11 +226,9 @@ impl SignedRefs<Verified> {
     where
         S: ReadRepository,
     {
-        if let Some(oid) = repo.reference_oid(remote, &SIGNATURE_REF)? {
+        let oid = repo.reference_oid(remote, &SIGNATURE_REF)?;
-            Self::load_at(oid, remote, repo)
+
-        } else {
+        SignedRefs::load_at(oid, remote, repo)
-            Err(Error::NotFound)
-        }
     }
 
     pub fn load_at<S>(oid: Oid, remote: &RemoteId, repo: &S) -> Result<Self, Error>
@@ -255,10 +262,12 @@ impl SignedRefs<Verified> {
         repo: &S,
     ) -> Result<Updated, Error> {
         let sigref = &*SIGNATURE_REF;
-        let parent: Option<git2::Commit> = repo
+        let parent = match repo.reference(remote, sigref).map_err(|e| dbg!(e)) {
-            .reference(remote, sigref)?
+            Ok(r) => Some(r.peel_to_commit()?),
-            .map(|r| r.peel_to_commit())
+            Err(git_ext::Error::Git(e)) if git_ext::is_not_found_err(&e) => None,
-            .transpose()?;
+            Err(git_ext::Error::NotFound(_)) => None,
+            Err(e) => return Err(e.into()),
+        };
 
         let tree = {
             let raw = repo.raw();

radicle/src/test/storage.rs

@@ -2,6 +2,7 @@ use std::collections::HashMap;
 use std::path::{Path, PathBuf};
 
 use git_url::Url;
+use radicle_git_ext as git_ext;
 
 use crate::crypto::{Signer, Verified};
 use crate::identity::project::Doc;
@@ -106,9 +107,9 @@ impl ReadRepository for MockRepository {
 
     fn blob_at<'a>(
         &'a self,
-        _oid: radicle_git_ext::Oid,
+        _oid: git_ext::Oid,
         _path: &'a std::path::Path,
-    ) -> Result<git2::Blob<'a>, radicle_git_ext::Error> {
+    ) -> Result<git2::Blob<'a>, git_ext::Error> {
         todo!()
     }
 
@@ -116,7 +117,7 @@ impl ReadRepository for MockRepository {
         &self,
         _remote: &RemoteId,
         _reference: &git::RefStr,
-    ) -> Result<Option<git2::Reference>, git2::Error> {
+    ) -> Result<git2::Reference, git_ext::Error> {
         todo!()
     }
 
@@ -124,7 +125,7 @@ impl ReadRepository for MockRepository {
         &self,
         _remote: &RemoteId,
         _reference: &git::RefStr,
-    ) -> Result<Option<radicle_git_ext::Oid>, git2::Error> {
+    ) -> Result<git_ext::Oid, git_ext::Error> {
         todo!()
     }