Fixes and improvements to alias handling

* Return the correct alias from a node announcement, stripping '\0'
  bytes.
* Accept `AliasStore` types as references and as owned.
* Store a `null` in the database if the alias received is empty.

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

@@ -457,7 +457,7 @@ async fn issues_handler(
     let aliases = &ctx.profile.aliases()?;
     let issues = issues
         .into_iter()
-        .map(|(id, issue, _)| api::json::issue(id, issue, &aliases))
+        .map(|(id, issue, _)| api::json::issue(id, issue, aliases))
         .skip(page * per_page)
         .take(per_page)
         .collect::<Vec<_>>();
@@ -568,7 +568,7 @@ async fn issue_handler(
     let issue = issue::Issues::open(&repo)?
         .get(&issue_id.into())?
         .ok_or(Error::NotFound)?;
-    let aliases = &ctx.profile.aliases()?;
+    let aliases = ctx.profile.aliases()?;
 
     Ok::<_, Error>(Json(api::json::issue(issue_id.into(), issue, &aliases)))
 }
@@ -720,7 +720,7 @@ async fn patches_handler(
         })
         .collect::<Vec<_>>();
     patches.sort_by(|(_, a, _), (_, b, _)| b.timestamp().cmp(&a.timestamp()));
-    let aliases = &ctx.profile.aliases()?;
+    let aliases = ctx.profile.aliases()?;
     let patches = patches
         .into_iter()
         .map(|(id, patch, _)| api::json::patch(id, patch, &repo, &aliases))
@@ -742,7 +742,7 @@ async fn patch_handler(
     let patch = patch::Patches::open(&repo)?
         .get(&patch_id.into())?
         .ok_or(Error::NotFound)?;
-    let aliases = &ctx.profile.aliases()?;
+    let aliases = ctx.profile.aliases()?;
 
     Ok::<_, Error>(Json(api::json::patch(
         patch_id.into(),

radicle-node/src/service/config.rs

@@ -131,3 +131,17 @@ impl Config {
         }
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn test_node_announcement() {
+        let cfg = Config {
+            alias: Some(String::from("cloudhead")),
+            ..Config::default()
+        };
+        assert_eq!("cloudhead", cfg.node(0).alias().unwrap());
+    }
+}

radicle-node/src/service/message.rs

@@ -121,7 +121,12 @@ impl NodeAnnouncement {
 
     /// Get the alias as a UTF-8 string.
     pub fn alias(&self) -> Result<&str, std::str::Utf8Error> {
-        str::from_utf8(&self.alias)
+        let length = self
+            .alias
+            .iter()
+            .position(|&b| b == 0)
+            .unwrap_or(self.alias.len());
+        str::from_utf8(&self.alias[..length])
     }
 }
 

radicle/src/node.rs

@@ -5,7 +5,7 @@ pub mod events;
 pub mod routing;
 pub mod tracking;
 
-use std::collections::{BTreeSet, HashSet};
+use std::collections::{BTreeSet, HashMap, HashSet};
 use std::io::{BufRead, BufReader};
 use std::ops::Deref;
 use std::os::unix::net::UnixStream;
@@ -707,6 +707,24 @@ pub trait AliasStore {
     fn alias(&self, nid: &NodeId) -> Option<String>;
 }
 
+impl<T: AliasStore + ?Sized> AliasStore for &T {
+    fn alias(&self, nid: &NodeId) -> Option<String> {
+        dbg!((*self).alias(nid))
+    }
+}
+
+impl<T: AliasStore + ?Sized> AliasStore for Box<T> {
+    fn alias(&self, nid: &NodeId) -> Option<String> {
+        self.deref().alias(nid)
+    }
+}
+
+impl AliasStore for HashMap<NodeId, String> {
+    fn alias(&self, nid: &NodeId) -> Option<String> {
+        dbg!(self.get(nid).map(ToOwned::to_owned))
+    }
+}
+
 #[cfg(test)]
 mod test {
     use super::*;

radicle/src/node/address/store.rs

@@ -77,7 +77,7 @@ impl Store for Book {
 
         if let Some(Ok(row)) = stmt.into_iter().next() {
             let features = row.read::<node::Features, _>("features");
-            let alias = row.read::<&str, _>("alias").to_owned();
+            let alias = row.read::<Option<&str>, _>("alias").map(ToOwned::to_owned);
             let timestamp = row.read::<i64, _>("timestamp") as Timestamp;
             let pow = row.read::<i64, _>("pow") as u32;
             let mut addrs = Vec::new();
@@ -143,6 +143,11 @@ impl Store for Book {
                  SET features = ?2, alias = ?3, pow = ?4, timestamp = ?5
                  WHERE timestamp < ?5",
             )?;
+            let alias = if alias.is_empty() {
+                sql::Value::Null
+            } else {
+                sql::Value::String(alias.to_owned())
+            };
 
             stmt.bind((1, node))?;
             stmt.bind((2, features))?;
@@ -260,7 +265,7 @@ impl AliasStore for Book {
     /// Calls `Self::get` under the hood.
     fn alias(&self, nid: &NodeId) -> Option<String> {
         self.get(nid)
-            .map(|node| node.map(|n| n.alias))
+            .map(|node| node.and_then(|n| n.alias))
             .unwrap_or(None)
     }
 }
@@ -396,6 +401,26 @@ mod test {
         assert!(!removed);
     }
 
+    #[test]
+    fn test_alias() {
+        let alice = arbitrary::gen::<NodeId>(1);
+        let mut cache = Book::memory().unwrap();
+        let features = node::Features::SEED;
+        let timestamp = LocalTime::now().as_millis();
+
+        cache
+            .insert(&alice, features, "alice", 16, timestamp, [])
+            .unwrap();
+        let node = cache.get(&alice).unwrap().unwrap();
+        assert_eq!(node.alias.as_deref(), Some("alice"));
+
+        cache
+            .insert(&alice, features, "", 16, timestamp + 1, [])
+            .unwrap();
+        let node = cache.get(&alice).unwrap().unwrap();
+        assert_eq!(node.alias.as_deref(), None);
+    }
+
     #[test]
     fn test_insert_and_get() {
         let alice = arbitrary::gen::<NodeId>(1);
@@ -419,7 +444,7 @@ mod test {
         assert_eq!(node.features, features);
         assert_eq!(node.pow, 16);
         assert_eq!(node.timestamp, timestamp);
-        assert_eq!(node.alias.as_str(), "alice");
+        assert_eq!(node.alias.as_deref(), Some("alice"));
         assert_eq!(node.addrs, vec![ka]);
     }
 
@@ -478,7 +503,7 @@ mod test {
         assert!(!updated, "Can't update using a smaller timestamp");
 
         let node = cache.get(&alice).unwrap().unwrap();
-        assert_eq!(node.alias, "alice");
+        assert_eq!(node.alias.as_deref(), Some("alice"));
         assert_eq!(node.timestamp, timestamp);
         assert_eq!(node.pow, 0);
 
@@ -501,7 +526,7 @@ mod test {
 
         let node = cache.get(&alice).unwrap().unwrap();
         assert_eq!(node.features, node::Features::NONE);
-        assert_eq!(node.alias, "~alice~");
+        assert_eq!(node.alias.as_deref(), Some("~alice~"));
         assert_eq!(node.timestamp, timestamp + 2);
         assert_eq!(node.pow, 1);
         assert_eq!(node.addrs, vec![ka]);

radicle/src/node/address/types.rs

@@ -78,7 +78,7 @@ impl<K, V> DerefMut for AddressBook<K, V> {
 #[derive(Debug, Clone, PartialEq, Eq)]
 pub struct Node {
     /// Advertized alias.
-    pub alias: String,
+    pub alias: Option<String>,
     /// Advertized features.
     pub features: node::Features,
     /// Advertized addresses

radicle/src/node/tracking/store.rs

@@ -257,7 +257,7 @@ impl Config {
     }
 }
 
-impl AliasStore for &Config {
+impl AliasStore for Config {
     /// Retrieve `alias` of given node.
     /// Calls `Self::node_policy` under the hood.
     fn alias(&self, nid: &NodeId) -> Option<String> {

radicle/src/profile.rs

@@ -191,13 +191,13 @@ pub struct Aliases {
     addresses: address::Book,
 }
 
-impl AliasStore for &Aliases {
+impl AliasStore for Aliases {
     /// Retrieve `alias` of given node.
-    /// First looks in `tracking.db` and then `address.db`,
+    /// First looks in `tracking.db` and then `address.db`.
     fn alias(&self, nid: &NodeId) -> Option<String> {
-        (&&self.tracking as &dyn AliasStore)
+        self.tracking
             .alias(nid)
-            .or_else(|| (&self.addresses as &dyn AliasStore).alias(nid))
+            .or_else(|| self.addresses.alias(nid))
     }
 }