Radish alpha
h
Radicle Heartwood Protocol & Stack
Radicle
Git (anonymous pull)
Log in to clone via SSH
node: Use stronger types for refs
Alexis Sellier committed 3 years ago
commit e4e4d9dcaa17e0deb6f6f73df0a6abaca32be59a
parent a4d2232578e263d5c79befc49a5a7ae69a78e283
6 files changed +41 -33
modified node/src/git.rs
@@ -9,6 +9,7 @@ use crate::storage::refs::Refs;
use crate::storage::RemoteId;

pub use git_ext::Oid;
+
pub use git_ref_format::{refname, RefStr, RefString};
pub use git_url::Url;

/// Default port of the `git` transport protocol.
@@ -43,7 +44,7 @@ pub fn remote_refs(url: &Url) -> Result<HashMap<RemoteId, Refs>, ListRefsError>
        let (id, refname) = parse_ref::<UserId>(r.name())?;
        let entry = remotes.entry(id).or_insert_with(Refs::default);

-
        entry.insert(refname.to_string(), r.oid().into());
+
        entry.insert(refname, r.oid().into());
    }

    Ok(remotes)
modified node/src/storage.rs
@@ -15,8 +15,8 @@ pub use radicle_git_ext::Oid;

use crate::collections::HashMap;
use crate::crypto::{self, Unverified, Verified};
-
use crate::git::RefError;
use crate::git::Url;
+
use crate::git::{RefError, RefStr};
use crate::identity;
use crate::identity::{ProjId, ProjIdError, UserId};
use crate::storage::refs::Refs;
@@ -48,7 +48,6 @@ pub enum Error {
}

pub type RemoteId = UserId;
-
pub type RefName = String;

/// Project remotes. Tracks the git state of a project.
#[derive(Debug, Clone, PartialEq, Eq)]
@@ -186,9 +185,9 @@ pub trait ReadRepository {
    fn reference(
        &self,
        user: &UserId,
-
        reference: &str,
+
        reference: &RefStr,
    ) -> Result<Option<git2::Reference>, git2::Error>;
-
    fn reference_oid(&self, user: &UserId, reference: &str) -> Result<Option<Oid>, git2::Error>;
+
    fn reference_oid(&self, user: &UserId, reference: &RefStr) -> Result<Option<Oid>, git2::Error>;
    fn references(&self, user: &UserId) -> Result<Refs, Error>;
    fn remote(&self, user: &UserId) -> Result<Remote<Verified>, refs::Error>;
    fn remotes(&self) -> Result<Remotes<Verified>, refs::Error>;
modified node/src/storage/git.rs
@@ -214,7 +214,7 @@ impl ReadRepository for Repository {
    fn reference(
        &self,
        remote: &RemoteId,
-
        name: &str,
+
        name: &git::RefStr,
    ) -> Result<Option<git2::Reference>, git2::Error> {
        let name = format!("refs/remotes/{remote}/{name}");
        self.backend.find_reference(&name).map(Some).or_else(|e| {
@@ -226,7 +226,11 @@ impl ReadRepository for Repository {
        })
    }

-
    fn reference_oid(&self, user: &RemoteId, reference: &str) -> Result<Option<Oid>, git2::Error> {
+
    fn reference_oid(
+
        &self,
+
        user: &RemoteId,
+
        reference: &git::RefStr,
+
    ) -> Result<Option<Oid>, git2::Error> {
        let reference = self.reference(user, reference)?;
        Ok(reference.and_then(|r| r.target().map(|o| o.into())))
    }
@@ -249,7 +253,7 @@ impl ReadRepository for Repository {
            let (_, refname) = git::parse_ref::<RemoteId>(name)?;
            let oid = e.target().ok_or(Error::InvalidRef)?;

-
            refs.insert(refname.to_string(), oid.into());
+
            refs.insert(refname, oid.into());
        }
        Ok(refs.into())
    }
@@ -338,7 +342,7 @@ mod tests {

        // Strip the remote refs of sigrefs so we can compare them.
        for remote in refs.values_mut() {
-
            remote.remove(SIGNATURE_REF).unwrap();
+
            remote.remove(&*SIGNATURE_REF).unwrap();
        }
        assert_eq!(refs, remotes.into());
    }
@@ -351,7 +355,7 @@ mod tests {
        let inventory = alice.inventory().unwrap();
        let proj = inventory.first().unwrap();
        let remotes = alice.repository(proj).unwrap().remotes().unwrap();
-
        let refname = "heads/master";
+
        let refname = git::refname!("heads/master");

        // Have Bob fetch Alice's refs.
        bob.repository(proj)
@@ -366,10 +370,10 @@ mod tests {

        for (id, _) in remotes.into_iter() {
            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().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().unwrap();

            assert_eq!(alice_oid.target(), bob_oid.target());
        }
@@ -403,7 +407,7 @@ mod tests {
        let mut unsigned = project.references(&alice).unwrap();

        // The signed refs doesn't contain the signature ref itself.
-
        unsigned.remove(SIGNATURE_REF).unwrap();
+
        unsigned.remove(&*SIGNATURE_REF).unwrap();

        assert_eq!(remote.refs, signed);
        assert_eq!(*remote.refs, unsigned);
modified node/src/storage/refs.rs
@@ -7,6 +7,7 @@ use std::ops::{Deref, DerefMut};
use std::path::Path;
use std::str::FromStr;

+
use once_cell::sync::Lazy;
use radicle_git_ext as git_ext;
use serde::{Deserialize, Serialize};
use thiserror::Error;
@@ -18,7 +19,7 @@ use crate::git::Oid;
use crate::storage;
use crate::storage::{ReadRepository, RemoteId, WriteRepository};

-
pub const SIGNATURE_REF: &str = "radicle/signature";
+
pub static SIGNATURE_REF: Lazy<git::RefString> = Lazy::new(|| git::refname!("radicle/signature"));
pub const REFS_BLOB_PATH: &str = "refs";
pub const SIGNATURE_BLOB_PATH: &str = "signature";

@@ -51,7 +52,7 @@ pub enum Error {

/// The published state of a local repository.
#[derive(Default, Clone, Debug, PartialEq, Eq, Serialize, Deserialize)]
-
pub struct Refs(BTreeMap<String, Oid>);
+
pub struct Refs(BTreeMap<git::RefString, Oid>);

impl Refs {
    /// Verify the given signature on these refs, and return [`SignedRefs`] on success.
@@ -100,7 +101,7 @@ impl Refs {
                .split_once(' ')
                .ok_or(canonical::Error::InvalidFormat)?;

-
            let name = name.to_owned();
+
            let name = git::RefString::try_from(name)?;
            let oid = Oid::from_str(oid)?;

            refs.insert(name, oid);
@@ -110,7 +111,7 @@ impl Refs {

    pub fn canonical(&self) -> Vec<u8> {
        let mut buf = String::new();
-
        let refs = self.iter().filter(|(name, _)| *name != SIGNATURE_REF);
+
        let refs = self.iter().filter(|(name, _)| *name != &*SIGNATURE_REF);

        for (name, oid) in refs {
            buf.push_str(&oid.to_string());
@@ -122,7 +123,7 @@ impl Refs {
    }
}

-
impl From<Refs> for BTreeMap<String, Oid> {
+
impl From<Refs> for BTreeMap<git::RefString, Oid> {
    fn from(refs: Refs) -> Self {
        refs.0
    }
@@ -134,14 +135,14 @@ impl<V> From<SignedRefs<V>> for Refs {
    }
}

-
impl From<BTreeMap<String, Oid>> for Refs {
-
    fn from(refs: BTreeMap<String, Oid>) -> Self {
+
impl From<BTreeMap<git::RefString, Oid>> for Refs {
+
    fn from(refs: BTreeMap<git::RefString, Oid>) -> Self {
        Self(refs)
    }
}

impl Deref for Refs {
-
    type Target = BTreeMap<String, Oid>;
+
    type Target = BTreeMap<git::RefString, Oid>;

    fn deref(&self) -> &Self::Target {
        &self.0
@@ -196,7 +197,7 @@ impl SignedRefs<Verified> {
    where
        S: ReadRepository,
    {
-
        if let Some(oid) = repo.reference_oid(remote, SIGNATURE_REF)? {
+
        if let Some(oid) = repo.reference_oid(remote, &SIGNATURE_REF)? {
            Self::load_at(oid, remote, repo)
        } else {
            Err(Error::NotFound)
@@ -233,8 +234,9 @@ impl SignedRefs<Verified> {
        remote: &RemoteId,
        repo: &S,
    ) -> Result<Updated, Error> {
+
        let sigref = &*SIGNATURE_REF;
        let parent: Option<git2::Commit> = repo
-
            .reference(remote, SIGNATURE_REF)?
+
            .reference(remote, sigref)?
            .map(|r| r.peel_to_commit())
            .transpose()?;

@@ -260,13 +262,13 @@ impl SignedRefs<Verified> {
            }
        }

-
        let sigref = format!("refs/remotes/{remote}/{SIGNATURE_REF}");
+
        let sigref = format!("refs/remotes/{remote}/{sigref}");
        let author = repo.raw().signature()?;
        let commit = repo.raw().commit(
            Some(&sigref),
            &author,
            &author,
-
            &format!("Update {} for {}", SIGNATURE_REF, remote),
+
            &format!("Update {} for {}", sigref, remote),
            &tree,
            &parent.iter().collect::<Vec<&git2::Commit>>(),
        );
@@ -306,6 +308,8 @@ pub mod canonical {

    #[derive(Debug, thiserror::Error)]
    pub enum Error {
+
        #[error(transparent)]
+
        InvalidRef(#[from] git_ref_format::Error),
        #[error("invalid canonical format")]
        InvalidFormat,
        #[error(transparent)]
@@ -325,10 +329,6 @@ mod tests {
        let encoded = refs.canonical();
        let decoded = Refs::from_canonical(&encoded).unwrap();

-
        println!("{:?}", refs);
-

        assert_eq!(refs, decoded);
    }
}
-

-
// TODO: Test canonical/from_canonical
modified node/src/test/arbitrary.rs
@@ -7,6 +7,7 @@ use quickcheck::Arbitrary;
use crate::collections::HashMap;
use crate::crypto::{self, Signer};
use crate::crypto::{PublicKey, SecretKey};
+
use crate::git;
use crate::hash;
use crate::identity::ProjId;
use crate::storage;
@@ -50,7 +51,7 @@ impl Arbitrary for MockStorage {

impl Arbitrary for Refs {
    fn arbitrary(g: &mut quickcheck::Gen) -> Self {
-
        let mut refs: BTreeMap<storage::RefName, storage::Oid> = BTreeMap::new();
+
        let mut refs: BTreeMap<git::RefString, storage::Oid> = BTreeMap::new();
        let mut bytes: [u8; 20] = [0; 20];
        let names = &[
            "heads/master",
@@ -69,7 +70,9 @@ impl Arbitrary for Refs {
                    *byte = u8::arbitrary(g);
                }
                let oid = storage::Oid::try_from(&bytes[..]).unwrap();
-
                refs.insert(name.to_string(), oid);
+
                let name = git::RefString::try_from(*name).unwrap();
+

+
                refs.insert(name, oid);
            }
        }
        Self::from(refs)
modified node/src/test/storage.rs
@@ -1,6 +1,7 @@
use git_url::Url;

use crate::crypto::Verified;
+
use crate::git;
use crate::identity::{ProjId, UserId};
use crate::storage::refs;
use crate::storage::{
@@ -96,7 +97,7 @@ impl ReadRepository for MockRepository {
    fn reference(
        &self,
        _user: &UserId,
-
        _reference: &str,
+
        _reference: &git::RefStr,
    ) -> Result<Option<git2::Reference>, git2::Error> {
        todo!()
    }
@@ -104,7 +105,7 @@ impl ReadRepository for MockRepository {
    fn reference_oid(
        &self,
        _user: &UserId,
-
        _reference: &str,
+
        _reference: &git::RefStr,
    ) -> Result<Option<radicle_git_ext::Oid>, git2::Error> {
        todo!()
    }