radicle: separate repository validation

Make the validation logic customisable by extracting it out into a
separate trait that can be defined by other crates.

The Repository validation logic stays the same.

Signed-off-by: Fintan Halpenny <fintan.halpenny@gmail.com>
X-Clacks-Overhead: GNU Terry Pratchett

radicle-cli/src/commands/clone.rs

@@ -18,6 +18,7 @@ use radicle::prelude::*;
 use radicle::rad;
 use radicle::storage;
 use radicle::storage::git::Storage;
+use radicle::storage::RemoteRepository as _;
 use radicle::storage::RepositoryError;
 
 use crate::commands::rad_checkout as checkout;

radicle-cli/src/commands/patch/list.rs

@@ -3,6 +3,7 @@ use radicle::cob::patch::{Patch, PatchId, Patches, Verdict};
 use radicle::prelude::*;
 use radicle::profile::Profile;
 use radicle::storage::git::Repository;
+use radicle::storage::RemoteRepository as _;
 
 use crate::terminal as term;
 use term::format::Author;

radicle-cli/src/commands/publish.rs

@@ -5,7 +5,7 @@ use anyhow::{anyhow, Context as _};
 use radicle::identity::{Identity, Visibility};
 use radicle::node::Handle as _;
 use radicle::prelude::Id;
-use radicle::storage::{ReadRepository, SignRepository, WriteRepository, WriteStorage};
+use radicle::storage::{SignRepository, ValidateRepository, WriteRepository, WriteStorage};
 
 use crate::terminal as term;
 use crate::terminal::args::{Args, Error, Help};

radicle-cli/tests/commands.rs

@@ -10,7 +10,7 @@ use radicle::node::Handle as _;
 use radicle::node::{Alias, DEFAULT_TIMEOUT};
 use radicle::prelude::Id;
 use radicle::profile::Home;
-use radicle::storage::{ReadRepository, ReadStorage};
+use radicle::storage::{ReadStorage, RemoteRepository};
 use radicle::test::fixtures;
 
 use radicle_cli_test::TestFormula;

radicle-httpd/src/api/json.rs

@@ -17,7 +17,7 @@ use radicle::cob::{ActorId, Author, Embed, Reaction, Timestamp, Uri};
 use radicle::git::RefString;
 use radicle::node::{Alias, AliasStore};
 use radicle::prelude::NodeId;
-use radicle::storage::{git, refs, ReadRepository};
+use radicle::storage::{git, refs, RemoteRepository};
 use radicle_surf::blob::Blob;
 use radicle_surf::tree::Tree;
 use radicle_surf::{Commit, Oid, Stats};

radicle-httpd/src/api/v1/projects.rs

@@ -19,7 +19,7 @@ use radicle::node::routing::Store;
 use radicle::node::AliasStore;
 use radicle::node::NodeId;
 use radicle::storage::git::paths;
-use radicle::storage::{ReadRepository, ReadStorage, WriteRepository};
+use radicle::storage::{ReadRepository, ReadStorage, RemoteRepository, WriteRepository};
 use radicle_surf::{diff, Glob, Oid, Repository};
 
 use crate::api::error::Error;

radicle-node/src/service.rs

@@ -39,7 +39,7 @@ use crate::service::message::{NodeAnnouncement, RefsAnnouncement};
 use crate::service::tracking::{store::Write, Scope};
 use crate::storage;
 use crate::storage::{Namespaces, ReadStorage};
-use crate::storage::{ReadRepository, RefUpdate};
+use crate::storage::{ReadRepository, RefUpdate, RemoteRepository as _};
 use crate::worker::FetchError;
 use crate::Link;
 

radicle-node/src/service/message.rs

@@ -10,7 +10,7 @@ use crate::service::filter::Filter;
 use crate::service::{Link, NodeId, Timestamp};
 use crate::storage;
 use crate::storage::refs::SignedRefs;
-use crate::storage::{ReadRepository, ReadStorage};
+use crate::storage::{ReadStorage, RemoteRepository as _};
 use crate::wire;
 
 /// Maximum number of addresses which can be announced to other nodes.

radicle-node/src/test/environment.rs

@@ -29,7 +29,7 @@ use radicle::profile;
 use radicle::profile::Home;
 use radicle::profile::Profile;
 use radicle::rad;
-use radicle::storage::{ReadRepository, ReadStorage as _, SignRepository as _};
+use radicle::storage::{ReadStorage as _, RemoteRepository as _, SignRepository as _};
 use radicle::test::fixtures;
 use radicle::Storage;
 

radicle-node/src/test/peer.rs

@@ -10,7 +10,7 @@ use radicle::identity::Visibility;
 use radicle::node::address::Store;
 use radicle::node::{address, Alias, ConnectOptions};
 use radicle::rad;
-use radicle::storage::ReadRepository;
+use radicle::storage::{ReadRepository, RemoteRepository};
 use radicle::Storage;
 
 use crate::crypto::test::signer::MockSigner;

radicle-node/src/tests.rs

@@ -11,7 +11,7 @@ use netservices::Direction as Link;
 use radicle::identity::Visibility;
 use radicle::node::routing::Store as _;
 use radicle::node::{ConnectOptions, DEFAULT_TIMEOUT};
-use radicle::storage::ReadRepository;
+use radicle::storage::RemoteRepository as _;
 
 use crate::collections::{RandomMap, RandomSet};
 use crate::crypto::test::signer::MockSigner;

radicle-node/src/tests/e2e.rs

@@ -3,7 +3,10 @@ use std::{collections::HashSet, thread, time};
 use radicle::crypto::{test::signer::MockSigner, Signer};
 use radicle::git;
 use radicle::node::{Alias, FetchResult, Handle as _, DEFAULT_TIMEOUT};
-use radicle::storage::{ReadRepository, ReadStorage, WriteRepository, WriteStorage};
+use radicle::storage::{
+    ReadRepository, ReadStorage, RemoteRepository, ValidateRepository, WriteRepository,
+    WriteStorage,
+};
 use radicle::test::fixtures;
 use radicle::{assert_matches, rad};
 

radicle-node/src/worker/fetch.rs

@@ -13,7 +13,10 @@ use radicle::prelude::{Doc, Id, NodeId};
 use radicle::storage::git::Repository;
 use radicle::storage::refs::IDENTITY_BRANCH;
 use radicle::storage::{Namespaces, RefUpdate, Remote, RemoteId, Validation, Validations};
-use radicle::storage::{ReadRepository, ReadStorage, WriteRepository, WriteStorage};
+use radicle::storage::{
+    ReadRepository, ReadStorage, RemoteRepository, ValidateRepository, WriteRepository,
+    WriteStorage,
+};
 use radicle::{git, Storage};
 
 pub type Refspec = refspec::Refspec<git::PatternString, git::PatternString>;

radicle/src/rad.rs

@@ -351,7 +351,7 @@ mod tests {
     use crate::identity::Did;
     use crate::storage::git::transport;
     use crate::storage::git::Storage;
-    use crate::storage::ReadStorage;
+    use crate::storage::{ReadStorage, RemoteRepository as _};
     use crate::test::fixtures;
 
     use super::*;

radicle/src/storage.rs

@@ -345,7 +345,7 @@ pub trait WriteStorage: ReadStorage {
 }
 
 /// Allows read-only access to a repository.
-pub trait ReadRepository: Sized {
+pub trait ReadRepository: Sized + ValidateRepository {
     /// Return the repository id.
     fn id(&self) -> Id;
 
@@ -361,21 +361,6 @@ pub trait ReadRepository: Sized {
     /// Get a blob in this repository, given its id.
     fn blob(&self, oid: Oid) -> Result<git2::Blob, git_ext::Error>;
 
-    /// Validate all remotes with [`ReadRepository::validate_remote`].
-    fn validate(&self) -> Result<Validations, Error> {
-        let mut failures = Validations::default();
-        for (_, remote) in self.remotes()? {
-            failures.append(&mut self.validate_remote(&remote)?);
-        }
-        Ok(failures)
-    }
-
-    /// Validates a remote's signed refs and identity.
-    ///
-    /// Returns any ref found under that remote that isn't signed.
-    /// If a signed ref is missing from the repository, an error is returned.
-    fn validate_remote(&self, remote: &Remote<Verified>) -> Result<Validations, Error>;
-
     /// Get the head of this repository.
     ///
     /// Returns the reference pointed to by `HEAD` if it is set. Otherwise, computes the canonical
@@ -467,12 +452,6 @@ pub trait ReadRepository: Sized {
         pattern: &git::PatternStr,
     ) -> Result<Vec<(Qualified, Oid)>, git::ext::Error>;
 
-    /// Get the given remote.
-    fn remote(&self, remote: &RemoteId) -> Result<Remote<Verified>, refs::Error>;
-
-    /// Get all remotes.
-    fn remotes(&self) -> Result<Remotes<Verified>, refs::Error>;
-
     /// Get repository delegates.
     fn delegates(&self) -> Result<NonEmpty<Did>, RepositoryError> {
         let doc: Doc<_> = self.identity_doc()?.into();
@@ -495,6 +474,35 @@ pub trait ReadRepository: Sized {
     fn merge_base(&self, left: &Oid, right: &Oid) -> Result<Oid, git::ext::Error>;
 }
 
+/// Access the remotes of a repository.
+pub trait RemoteRepository {
+    /// Get the given remote.
+    fn remote(&self, remote: &RemoteId) -> Result<Remote<Verified>, refs::Error>;
+
+    /// Get all remotes.
+    fn remotes(&self) -> Result<Remotes<Verified>, refs::Error>;
+}
+
+pub trait ValidateRepository
+where
+    Self: RemoteRepository,
+{
+    /// Validate all remotes with [`ValidateRepository::validate_remote`].
+    fn validate(&self) -> Result<Validations, Error> {
+        let mut failures = Validations::default();
+        for (_, remote) in self.remotes()? {
+            failures.append(&mut self.validate_remote(&remote)?);
+        }
+        Ok(failures)
+    }
+
+    /// Validates a remote's signed refs and identity.
+    ///
+    /// Returns any ref found under that remote that isn't signed.
+    /// If a signed ref is missing from the repository, an error is returned.
+    fn validate_remote(&self, remote: &Remote<Verified>) -> Result<Validations, Error>;
+}
+
 /// Allows read-write access to a repository.
 pub trait WriteRepository: ReadRepository + SignRepository {
     /// Set the repository head to the canonical branch.

radicle/src/storage/git.rs

@@ -25,7 +25,7 @@ use crate::storage::{
 pub use crate::git::*;
 pub use crate::storage::Error;
 
-use super::RemoteId;
+use super::{RemoteId, RemoteRepository, ValidateRepository};
 
 pub static NAMESPACES_GLOB: Lazy<git::refspec::PatternString> =
     Lazy::new(|| git::refspec::pattern!("refs/namespaces/*"));
@@ -394,37 +394,22 @@ impl Repository {
     }
 }
 
-impl ReadRepository for Repository {
+impl RemoteRepository for Repository {
-    fn id(&self) -> Id {
+    fn remotes(&self) -> Result<Remotes<Verified>, refs::Error> {
-        self.id
+        let mut remotes = Vec::new();
+        for remote in Repository::remotes(self)? {
+            remotes.push(remote?);
+        }
+        Ok(Remotes::from_iter(remotes))
     }
 
-    fn is_empty(&self) -> Result<bool, git2::Error> {
+    fn remote(&self, remote: &RemoteId) -> Result<Remote<Verified>, refs::Error> {
-        Ok(self.remotes()?.next().is_none())
+        let refs = SignedRefs::load(*remote, self)?;
-    }
+        Ok(Remote::<Verified>::new(refs))
-
+    }
-    fn path(&self) -> &Path {
-        self.backend.path()
-    }
-
-    fn blob_at<P: AsRef<Path>>(&self, commit: Oid, path: P) -> Result<git2::Blob, git::Error> {
-        let commit = self.backend.find_commit(*commit)?;
-        let tree = commit.tree()?;
-        let entry = tree.get_path(path.as_ref())?;
-        let obj = entry.to_object(&self.backend)?;
-        let blob = obj.into_blob().map_err(|_| {
-            git::Error::NotFound(git::NotFound::NoSuchBlob(
-                path.as_ref().display().to_string(),
-            ))
-        })?;
-
-        Ok(blob)
-    }
-
-    fn blob(&self, oid: Oid) -> Result<git2::Blob, git::Error> {
-        self.backend.find_blob(oid.into()).map_err(git::Error::from)
 }
 
+impl ValidateRepository for Repository {
     fn validate_remote(&self, remote: &Remote<Verified>) -> Result<Validations, Error> {
         // Contains a copy of the signed refs of this remote.
         let mut signed = BTreeMap::from((*remote.refs).clone());
@@ -469,6 +454,38 @@ impl ReadRepository for Repository {
 
         Ok(failures)
     }
+}
+
+impl ReadRepository for Repository {
+    fn id(&self) -> Id {
+        self.id
+    }
+
+    fn is_empty(&self) -> Result<bool, git2::Error> {
+        Ok(self.remotes()?.next().is_none())
+    }
+
+    fn path(&self) -> &Path {
+        self.backend.path()
+    }
+
+    fn blob_at<P: AsRef<Path>>(&self, commit: Oid, path: P) -> Result<git2::Blob, git::Error> {
+        let commit = self.backend.find_commit(*commit)?;
+        let tree = commit.tree()?;
+        let entry = tree.get_path(path.as_ref())?;
+        let obj = entry.to_object(&self.backend)?;
+        let blob = obj.into_blob().map_err(|_| {
+            git::Error::NotFound(git::NotFound::NoSuchBlob(
+                path.as_ref().display().to_string(),
+            ))
+        })?;
+
+        Ok(blob)
+    }
+
+    fn blob(&self, oid: Oid) -> Result<git2::Blob, git::Error> {
+        self.backend.find_blob(oid.into()).map_err(git::Error::from)
+    }
 
     fn reference(
         &self,
@@ -509,11 +526,6 @@ impl ReadRepository for Repository {
             .map_err(git::Error::from)
     }
 
-    fn remote(&self, remote: &RemoteId) -> Result<Remote<Verified>, refs::Error> {
-        let refs = SignedRefs::load(*remote, self)?;
-        Ok(Remote::<Verified>::new(refs))
-    }
-
     fn references_of(&self, remote: &RemoteId) -> Result<Refs, Error> {
         let entries = self
             .backend
@@ -563,14 +575,6 @@ impl ReadRepository for Repository {
         Ok(refs)
     }
 
-    fn remotes(&self) -> Result<Remotes<Verified>, refs::Error> {
-        let mut remotes = Vec::new();
-        for remote in Repository::remotes(self)? {
-            remotes.push(remote?);
-        }
-        Ok(Remotes::from_iter(remotes))
-    }
-
     fn identity_doc_at(&self, head: Oid) -> Result<DocAt, DocError> {
         Doc::<Verified>::load_at(head, self)
     }

radicle/src/storage/git/cob.rs

@@ -5,8 +5,10 @@ use std::path::Path;
 use cob::object::Objects;
 use radicle_cob as cob;
 use radicle_cob::change;
+use storage::RemoteRepository;
 use storage::RepositoryError;
 use storage::SignRepository;
+use storage::ValidateRepository;
 
 use crate::git::*;
 use crate::storage;
@@ -231,6 +233,22 @@ impl<'a, R: storage::ReadRepository> SignRepository for DraftStore<'a, R> {
     }
 }
 
+impl<'a, R: storage::RemoteRepository> RemoteRepository for DraftStore<'a, R> {
+    fn remote(&self, id: &RemoteId) -> Result<Remote<Verified>, storage::refs::Error> {
+        self.repo.remote(id)
+    }
+
+    fn remotes(&self) -> Result<Remotes<Verified>, storage::refs::Error> {
+        RemoteRepository::remotes(self.repo)
+    }
+}
+
+impl<'a, R: storage::ValidateRepository> ValidateRepository for DraftStore<'a, R> {
+    fn validate_remote(&self, remote: &Remote<Verified>) -> Result<Validations, Error> {
+        self.repo.validate_remote(remote)
+    }
+}
+
 impl<'a, R: storage::ReadRepository> ReadRepository for DraftStore<'a, R> {
     fn id(&self) -> identity::Id {
         self.repo.id()
@@ -248,22 +266,10 @@ impl<'a, R: storage::ReadRepository> ReadRepository for DraftStore<'a, R> {
         self.repo.canonical_head()
     }
 
-    fn validate_remote(&self, remote: &Remote<Verified>) -> Result<Validations, Error> {
-        self.repo.validate_remote(remote)
-    }
-
     fn path(&self) -> &std::path::Path {
         self.repo.path()
     }
 
-    fn remote(&self, id: &RemoteId) -> Result<Remote<Verified>, storage::refs::Error> {
-        self.repo.remote(id)
-    }
-
-    fn remotes(&self) -> Result<Remotes<Verified>, storage::refs::Error> {
-        ReadRepository::remotes(self.repo)
-    }
-
     fn commit(&self, oid: Oid) -> Result<git2::Commit, git_ext::Error> {
         self.repo.commit(oid)
     }

radicle/src/test/storage.rs

@@ -124,6 +124,29 @@ impl MockRepository {
     }
 }
 
+impl RemoteRepository for MockRepository {
+    fn remote(&self, id: &RemoteId) -> Result<Remote<Verified>, refs::Error> {
+        self.remotes
+            .get(id)
+            .map(|refs| Remote { refs: refs.clone() })
+            .ok_or(refs::Error::InvalidRef)
+    }
+
+    fn remotes(&self) -> Result<Remotes<Verified>, refs::Error> {
+        Ok(self
+            .remotes
+            .iter()
+            .map(|(id, refs)| (*id, Remote { refs: refs.clone() }))
+            .collect())
+    }
+}
+
+impl ValidateRepository for MockRepository {
+    fn validate_remote(&self, _remote: &Remote<Verified>) -> Result<Validations, Error> {
+        Ok(Validations::default())
+    }
+}
+
 impl ReadRepository for MockRepository {
     fn id(&self) -> Id {
         self.id
@@ -141,29 +164,10 @@ impl ReadRepository for MockRepository {
         todo!()
     }
 
-    fn validate_remote(&self, _remote: &Remote<Verified>) -> Result<Validations, Error> {
-        Ok(Validations::default())
-    }
-
     fn path(&self) -> &std::path::Path {
         todo!()
     }
 
-    fn remote(&self, id: &RemoteId) -> Result<Remote<Verified>, refs::Error> {
-        self.remotes
-            .get(id)
-            .map(|refs| Remote { refs: refs.clone() })
-            .ok_or(refs::Error::InvalidRef)
-    }
-
-    fn remotes(&self) -> Result<Remotes<Verified>, refs::Error> {
-        Ok(self
-            .remotes
-            .iter()
-            .map(|(id, refs)| (*id, Remote { refs: refs.clone() }))
-            .collect())
-    }
-
     fn commit(&self, _oid: Oid) -> Result<git2::Commit, git_ext::Error> {
         todo!()
     }