Make `Id` to string conversion more explicit

Two types of conversion: `canonical`, which is just the base58 encoding,
and `urn` which is like canonical, but includes `rad:`.

radicle-cli/src/commands/init.rs

@@ -232,7 +232,7 @@ pub fn init(options: Options, profile: &profile::Profile) -> anyhow::Result<()>
             term::blank();
             term::info!(
                 "Your project id is {}. You can show it any time by running:",
-                term::format::highlight(id)
+                term::format::highlight(id.urn())
             );
             term::indented(term::format::secondary("rad ."));
 

radicle-cli/src/commands/inspect.rs

@@ -130,7 +130,7 @@ pub fn run(options: Options, ctx: impl term::Context) -> anyhow::Result<()> {
         let path = profile
             .paths()
             .storage()
-            .join(id.to_human())
+            .join(id.urn())
             .join("refs")
             .join("namespaces");
 
@@ -195,7 +195,7 @@ pub fn run(options: Options, ctx: impl term::Context) -> anyhow::Result<()> {
             println!();
         }
     } else if options.id_only {
-        term::info!("{}", term::format::highlight(id.to_human()));
+        term::info!("{}", term::format::highlight(id.urn()));
     } else {
         term::info!("{}", term::format::highlight(id));
     }

radicle-cli/src/commands/ls.rs

@@ -53,7 +53,7 @@ pub fn run(_options: Options, ctx: impl term::Context) -> anyhow::Result<()> {
         let head = term::format::oid(head);
         table.push([
             term::format::bold(proj.name()),
-            term::format::tertiary(id),
+            term::format::tertiary(id.urn()),
             term::format::secondary(head),
             term::format::italic(proj.description()),
         ]);

radicle-cli/src/commands/rm.rs

@@ -81,12 +81,12 @@ pub fn run(options: Options, ctx: impl term::Context) -> anyhow::Result<()> {
     let id = options.id;
 
     if let Ok(Some(_)) = storage.get(signer.public_key(), id.to_owned()) {
-        let namespace = profile.paths().storage().join(id.to_human());
+        let namespace = profile.paths().storage().join(id.urn());
 
         if !options.confirm
             || term::confirm(format!(
                 "Are you sure you would like to delete {}?",
-                term::format::dim(id.to_human())
+                term::format::dim(id.urn())
             ))
         {
             rad_untrack::untrack(id.to_owned(), &profile)?;

radicle-cli/src/commands/untrack.rs

@@ -42,7 +42,7 @@ impl Args for Options {
                 Value(val) if id.is_none() => {
                     let val = val.to_string_lossy();
 
-                    if let Ok(val) = Id::from_human(&val) {
+                    if let Ok(val) = Id::from_urn(&val) {
                         id = Some(val);
                     } else {
                         return Err(anyhow!("invalid ID '{}'", val));
@@ -74,13 +74,13 @@ pub fn run(options: Options, ctx: impl term::Context) -> anyhow::Result<()> {
         term::success!(
             "Tracking relationships for {} ({}) removed",
             term::format::highlight(project.name()),
-            &id.to_human()
+            &id.urn()
         );
     } else {
         term::info!(
             "Tracking relationships for {} ({}) doesn't exist",
             term::format::highlight(project.name()),
-            &id.to_human()
+            &id.urn()
         );
     }
 

radicle-tools/src/rad-clone.rs

@@ -1,6 +1,5 @@
 use std::env;
 use std::path::Path;
-use std::str::FromStr;
 
 use radicle::identity::Id;
 
@@ -10,7 +9,7 @@ fn main() -> anyhow::Result<()> {
     let signer = profile.signer()?;
 
     if let Some(id) = env::args().nth(1) {
-        let id = Id::from_str(&id)?;
+        let id = Id::from_urn(&id)?;
         let mut node = radicle::node::connect(profile.socket())?;
         let repo = radicle::rad::clone(id, &cwd, &signer, &profile.storage, &mut node)?;
 

radicle/src/identity.rs

@@ -167,7 +167,7 @@ mod test {
         assert_eq!(format!("\"{}\"", pk), json);
 
         let json = serde_json::to_string(&proj).unwrap();
-        assert_eq!(format!("\"{}\"", proj), json);
+        assert_eq!(format!("\"{}\"", proj.urn()), json);
 
         let json = serde_json::to_string(&did).unwrap();
         assert_eq!(format!("\"{}\"", did), json);

radicle/src/identity/doc.rs

@@ -394,10 +394,7 @@ mod test {
             (*id).to_string(),
             "d96f425412c9f8ad5d9a9a05c9831d0728e2338d"
         );
-        assert_eq!(
+        assert_eq!(id.urn(), String::from("rad:z42hL2jL4XNk6K8oHQaSWfMgCL7ji"));
-            id.to_human(),
-            String::from("rad:z42hL2jL4XNk6K8oHQaSWfMgCL7ji")
-        );
     }
 
     #[test]

radicle/src/identity/doc/id.rs

@@ -27,7 +27,7 @@ pub struct Id(git::Oid);
 
 impl fmt::Display for Id {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        f.write_str(&self.to_human())
+        f.write_str(self.urn().as_str())
     }
 }
 
@@ -38,23 +38,34 @@ impl fmt::Debug for Id {
 }
 
 impl Id {
-    /// Format the identifier in a human-readable way.
+    /// Format the identifier as a human-readable URN.
     ///
     /// Eg. `rad:z3XncAdkZjeK9mQS5Sdc4qhw98BUX`.
     ///
-    pub fn to_human(&self) -> String {
+    pub fn urn(&self) -> String {
-        format!(
+        format!("{RAD_PREFIX}{}", self.canonical())
-            "{RAD_PREFIX}{}",
-            multibase::encode(multibase::Base::Base58Btc, self.0.as_bytes())
-        )
     }
 
-    /// Parse an identifier from the human-readable format.
+    /// Parse an identifier from the human-readable URN format.
     /// Accepts strings without the radicle prefix as well,
     /// for convenience.
-    pub fn from_human(s: &str) -> Result<Self, IdError> {
+    pub fn from_urn(s: &str) -> Result<Self, IdError> {
         let s = s.strip_prefix(RAD_PREFIX).unwrap_or(s);
-        let (_, bytes) = multibase::decode(s)?;
+        let id = Self::from_canonical(s)?;
+
+        Ok(id)
+    }
+
+    /// Format the identifier as a multibase string.
+    ///
+    /// Eg. `z3XncAdkZjeK9mQS5Sdc4qhw98BUX`.
+    ///
+    pub fn canonical(&self) -> String {
+        multibase::encode(multibase::Base::Base58Btc, self.0.as_bytes())
+    }
+
+    pub fn from_canonical(input: &str) -> Result<Self, IdError> {
+        let (_, bytes) = multibase::decode(input)?;
         let array: git::Oid = bytes.as_slice().try_into()?;
 
         Ok(Self(array))
@@ -65,7 +76,7 @@ impl FromStr for Id {
     type Err = IdError;
 
     fn from_str(s: &str) -> Result<Self, Self::Err> {
-        Self::from_human(s)
+        Self::from_urn(s)
     }
 }
 
@@ -74,7 +85,7 @@ impl TryFrom<OsString> for Id {
 
     fn try_from(value: OsString) -> Result<Self, Self::Error> {
         let string = value.to_string_lossy();
-        Self::from_str(&string)
+        Self::from_canonical(&string)
     }
 }
 
@@ -103,7 +114,7 @@ impl serde::Serialize for Id {
     where
         S: serde::Serializer,
     {
-        serde_ext::string::serialize(self, serializer)
+        serde_ext::string::serialize(&self.urn(), serializer)
     }
 }
 

radicle/src/node.rs

@@ -138,7 +138,7 @@ impl Handle for Node {
     }
 
     fn fetch(&mut self, id: Id) -> Result<(), Error> {
-        for line in self.call("fetch", &[id])? {
+        for line in self.call("fetch", &[id.urn()])? {
             let line = line?;
             log::debug!("node: {}", line);
         }
@@ -169,7 +169,7 @@ impl Handle for Node {
     }
 
     fn track_repo(&mut self, id: Id) -> Result<bool, Error> {
-        let mut line = self.call("track-repo", &[id])?;
+        let mut line = self.call("track-repo", &[id.urn()])?;
         let line = line
             .next()
             .ok_or(Error::EmptyResponse { cmd: "track-repo" })??;
@@ -205,7 +205,7 @@ impl Handle for Node {
     }
 
     fn untrack_repo(&mut self, id: Id) -> Result<bool, Error> {
-        let mut line = self.call("untrack-repo", &[id])?;
+        let mut line = self.call("untrack-repo", &[id.urn()])?;
         let line = line.next().ok_or(Error::EmptyResponse {
             cmd: "untrack-repo",
         })??;
@@ -223,7 +223,7 @@ impl Handle for Node {
     }
 
     fn announce_refs(&mut self, id: Id) -> Result<(), Error> {
-        for line in self.call("announce-refs", &[id])? {
+        for line in self.call("announce-refs", &[id.urn()])? {
             let line = line?;
             log::debug!("node: {}", line);
         }

radicle/src/sql.rs

@@ -13,7 +13,7 @@ impl TryFrom<&Value> for Id {
 
     fn try_from(value: &Value) -> Result<Self, Self::Error> {
         match value {
-            Value::String(id) => Id::from_str(id).map_err(|e| sql::Error {
+            Value::String(id) => Id::from_urn(id).map_err(|e| sql::Error {
                 code: None,
                 message: Some(e.to_string()),
             }),
@@ -27,7 +27,7 @@ impl TryFrom<&Value> for Id {
 
 impl sqlite::BindableWithIndex for &Id {
     fn bind<I: sql::ParameterIndex>(self, stmt: &mut sql::Statement<'_>, i: I) -> sql::Result<()> {
-        self.to_human().as_str().bind(stmt, i)
+        self.urn().as_str().bind(stmt, i)
     }
 }
 

radicle/src/storage/git.rs

@@ -134,7 +134,7 @@ impl Storage {
                 let name = r.name().ok_or(Error::InvalidRef)?;
                 let oid = r.target().ok_or(Error::InvalidRef)?;
 
-                println!("{} {} {}", proj, oid, name);
+                println!("{} {} {}", proj.urn(), oid, name);
             }
         }
         Ok(())
@@ -726,21 +726,17 @@ pub mod paths {
     use super::ReadStorage;
 
     pub fn repository<S: ReadStorage>(storage: &S, proj: &Id) -> PathBuf {
-        storage.path().join(proj.to_string())
+        storage.path().join(proj.canonical())
     }
 }
 
 #[cfg(test)]
 mod tests {
-    use std::io::{Read, Write};
-    use std::{io, net, process, thread};
-
     use crypto::test::signer::MockSigner;
 
     use super::*;
     use crate::assert_matches;
     use crate::git;
-    use crate::rad;
     use crate::storage::refs::SIGREFS_BRANCH;
     use crate::storage::{ReadRepository, ReadStorage, RefUpdate, WriteRepository};
     use crate::test::arbitrary;
@@ -903,115 +899,6 @@ mod tests {
         assert_eq!(refs, vec!["refs/heads/master", "refs/rad/id"]);
     }
 
-    #[test]
-    #[ignore]
-    // Test the remote transport using `git-upload-pack` and TCP streams.
-    // Must be run on its own, since it tries to register the remote transport, which
-    // will fail if the mock transport was already registered.
-    fn test_upload_pack() {
-        let tmp = tempfile::tempdir().unwrap();
-        let signer = MockSigner::default();
-        let remote = *signer.public_key();
-        let storage = Storage::open(tmp.path().join("storage")).unwrap();
-        let socket = net::TcpListener::bind(net::SocketAddr::from(([0, 0, 0, 0], 0))).unwrap();
-        let addr = socket.local_addr().unwrap();
-        let source_path = tmp.path().join("source");
-        let target_path = tmp.path().join("target");
-        let (source, _) = fixtures::repository(source_path);
-
-        transport::local::register(storage.clone());
-
-        let (proj, _, _) = rad::init(
-            &source,
-            "radicle",
-            "radicle",
-            git::refname!("master"),
-            &signer,
-            &storage,
-        )
-        .unwrap();
-
-        let t = thread::spawn(move || {
-            let (stream, _) = socket.accept().unwrap();
-            let repo = storage.repository(proj).unwrap();
-            // NOTE: `GIT_PROTOCOL=version=2` doesn't work.
-            let mut child = process::Command::new("git")
-                .current_dir(repo.path())
-                .arg("upload-pack")
-                .arg("--strict") // The path to the git repo must be exact.
-                .arg(".")
-                .stdout(process::Stdio::piped())
-                .stdin(process::Stdio::piped())
-                .spawn()
-                .unwrap();
-
-            let mut stdin = child.stdin.take().unwrap();
-            let mut stdout = child.stdout.take().unwrap();
-
-            let mut stream_r = stream.try_clone().unwrap();
-            let mut stream_w = stream;
-
-            let t = thread::spawn(move || {
-                let mut buf = [0u8; 1024];
-
-                while let Ok(n) = stream_r.read(&mut buf) {
-                    if n == 0 {
-                        break;
-                    }
-                    if stdin.write_all(&buf[..n]).is_err() {
-                        break;
-                    }
-                }
-            });
-            io::copy(&mut stdout, &mut stream_w).unwrap();
-
-            t.join().unwrap();
-            child.wait().unwrap();
-        });
-
-        let mut updates = Vec::new();
-        {
-            let mut callbacks = git2::RemoteCallbacks::new();
-            let mut opts = git2::FetchOptions::default();
-
-            callbacks.update_tips(|name, _, _| {
-                updates.push(name.to_owned());
-                true
-            });
-            opts.remote_callbacks(callbacks);
-
-            let target = git2::Repository::init_bare(target_path).unwrap();
-            let stream = net::TcpStream::connect(addr).unwrap();
-
-            // Register the `heartwood://` transport for this stream.
-            transport::remote::register(remote, stream.try_clone().unwrap());
-
-            // Fetch with the `heartwood://` transport.
-            target
-                .remote_anonymous(&format!("heartwood://{remote}/{proj}"))
-                .unwrap()
-                .fetch(
-                    &["refs/namespaces/*:refs/namespaces/*"],
-                    Some(&mut opts),
-                    None,
-                )
-                .unwrap();
-
-            stream.shutdown(net::Shutdown::Both).unwrap();
-
-            t.join().unwrap();
-        }
-
-        assert_eq!(
-            updates,
-            vec![
-                format!("refs/namespaces/{remote}/refs/heads/master"),
-                format!("refs/namespaces/{remote}/refs/rad/id"),
-                format!("refs/namespaces/{remote}/refs/rad/sigrefs")
-            ]
-        );
-    }
-
     #[test]
     fn test_sign_refs() {
         let tmp = tempfile::tempdir().unwrap();

radicle/src/storage/git/transport/local/url.rs

@@ -66,9 +66,9 @@ impl From<Id> for Url {
 impl fmt::Display for Url {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         if let Some(ns) = self.namespace {
-            write!(f, "{}://{}/{}", Self::SCHEME, self.repo, ns)
+            write!(f, "{}://{}/{}", Self::SCHEME, self.repo.canonical(), ns)
         } else {
-            write!(f, "{}://{}", Self::SCHEME, self.repo)
+            write!(f, "{}://{}", Self::SCHEME, self.repo.canonical())
         }
     }
 }
@@ -88,7 +88,7 @@ impl FromStr for Url {
             _ => return Err(UrlError::InvalidFormat),
         };
 
-        let resource = Id::from_str(resource).map_err(UrlError::InvalidRepository)?;
+        let resource = Id::from_canonical(resource).map_err(UrlError::InvalidRepository)?;
         let namespace = namespace
             .map(|pk| Namespace::from_str(pk).map_err(UrlError::InvalidNamespace))
             .transpose()?;
@@ -106,33 +106,37 @@ mod test {
 
     #[test]
     fn test_url_parse() {
-        let repo = Id::from_str("z2w8RArM3gaBXZxXhQUswE3hhLcss").unwrap();
+        let repo = Id::from_canonical("z2w8RArM3gaBXZxXhQUswE3hhLcss").unwrap();
         let namespace =
             Namespace::from_str("z6Mkifeb5NPS6j7JP72kEQEeuqMTpCAVcHsJi1C86jGTzHRi").unwrap();
 
-        let url = format!("rad://{repo}");
+        let url = format!("rad://{}", repo.canonical());
         let url = Url::from_str(&url).unwrap();
 
         assert_eq!(url.repo, repo);
         assert_eq!(url.namespace, None);
 
-        let url = format!("rad://{repo}/{namespace}");
+        let url = format!("rad://{}/{namespace}", repo.canonical());
         let url = Url::from_str(&url).unwrap();
 
         assert_eq!(url.repo, repo);
         assert_eq!(url.namespace, Some(namespace));
 
-        assert!(format!("heartwood://{repo}").parse::<Url>().is_err());
+        assert!(format!("heartwood://{}", repo.canonical())
-        assert!(format!("git://{repo}").parse::<Url>().is_err());
+            .parse::<Url>()
+            .is_err());
+        assert!(format!("git://{}", repo.canonical())
+            .parse::<Url>()
+            .is_err());
         assert!(format!("rad://{namespace}").parse::<Url>().is_err());
-        assert!(format!("rad://{repo}/{namespace}/fnord")
+        assert!(format!("rad://{}/{namespace}/fnord", repo.canonical())
             .parse::<Url>()
             .is_err());
     }
 
     #[test]
     fn test_url_to_string() {
-        let repo = Id::from_str("z2w8RArM3gaBXZxXhQUswE3hhLcss").unwrap();
+        let repo = Id::from_canonical("z2w8RArM3gaBXZxXhQUswE3hhLcss").unwrap();
         let namespace =
             Namespace::from_str("z6Mkifeb5NPS6j7JP72kEQEeuqMTpCAVcHsJi1C86jGTzHRi").unwrap();
 
@@ -140,12 +144,15 @@ mod test {
             repo,
             namespace: None,
         };
-        assert_eq!(url.to_string(), format!("rad://{repo}"));
+        assert_eq!(url.to_string(), format!("rad://{}", repo.canonical()));
 
         let url = Url {
             repo,
             namespace: Some(namespace),
         };
-        assert_eq!(url.to_string(), format!("rad://{repo}/{namespace}"));
+        assert_eq!(
+            url.to_string(),
+            format!("rad://{}/{namespace}", repo.canonical())
+        );
     }
 }

radicle/src/storage/git/transport/remote/mock.rs

@@ -42,7 +42,7 @@ impl git2::transport::SmartSubtransport for MockTransport {
                 url.node
             )));
         };
-        let git_dir = storage.join(url.repo.to_human());
+        let git_dir = storage.join(url.repo.canonical());
         let mut cmd = process::Command::new("git");
         let mut child = cmd
             .arg("upload-pack")

radicle/src/storage/git/transport/remote/url.rs

@@ -53,9 +53,22 @@ impl Url {
 impl fmt::Display for Url {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         if let Some(ns) = self.namespace {
-            write!(f, "{}://{}/{}/{}", Self::SCHEME, self.node, self.repo, ns)
+            write!(
+                f,
+                "{}://{}/{}/{}",
+                Self::SCHEME,
+                self.node,
+                self.repo.canonical(),
+                ns
+            )
         } else {
-            write!(f, "{}://{}/{}", Self::SCHEME, self.node, self.repo)
+            write!(
+                f,
+                "{}://{}/{}",
+                Self::SCHEME,
+                self.node,
+                self.repo.canonical()
+            )
         }
     }
 }
@@ -76,7 +89,7 @@ impl FromStr for Url {
         };
 
         let node = NodeId::from_str(node).map_err(UrlError::InvalidNode)?;
-        let resource = Id::from_str(resource).map_err(UrlError::InvalidRepository)?;
+        let resource = Id::from_canonical(resource).map_err(UrlError::InvalidRepository)?;
         let namespace = namespace
             .map(|pk| Namespace::from_str(pk).map_err(UrlError::InvalidNamespace))
             .transpose()?;
@@ -96,18 +109,18 @@ mod test {
     #[test]
     fn test_url_parse() {
         let node = NodeId::from_str("z6MkhaXgBZDvotDkL5257faiztiGiC2QtKLGpbnnEGta2doK").unwrap();
-        let repo = Id::from_str("z2w8RArM3gaBXZxXhQUswE3hhLcss").unwrap();
+        let repo = Id::from_canonical("z2w8RArM3gaBXZxXhQUswE3hhLcss").unwrap();
         let namespace =
             Namespace::from_str("z6Mkifeb5NPS6j7JP72kEQEeuqMTpCAVcHsJi1C86jGTzHRi").unwrap();
 
-        let url = format!("heartwood://{node}/{repo}");
+        let url = format!("heartwood://{node}/{}", repo.canonical());
         let url = Url::from_str(&url).unwrap();
 
         assert_eq!(url.node, node);
         assert_eq!(url.repo, repo);
         assert_eq!(url.namespace, None);
 
-        let url = format!("heartwood://{node}/{repo}/{namespace}");
+        let url = format!("heartwood://{node}/{}/{namespace}", repo.canonical());
         let url = Url::from_str(&url).unwrap();
 
         assert_eq!(url.node, node);
@@ -119,8 +132,10 @@ mod test {
         assert!(format!("heartwood://{node}/{namespace}")
             .parse::<Url>()
             .is_err());
-        assert!(format!("heartwood://{node}/{repo}/{namespace}/fnord")
+        assert!(
-            .parse::<Url>()
+            format!("heartwood://{node}/{}/{namespace}/fnord", repo.canonical())
-            .is_err());
+                .parse::<Url>()
+                .is_err()
+        );
     }
 }