fix: use new Git refs module, fix bugs found

This is a huge change, but it's mostly a few kinds of similar changes
in a lot of places. Unfortunately, they were not easy to tease apart
into small, easily reviewed commits.

Use the Git ref types created by the new refs module. Fix problems
this exposes.

The CiEvent enum variants for branch events now use the BranchName
type for branch names. In other words, they have a plain branch name.
The CiEvent::from_node_event constructor also always sets the branch
name to a plain branch name, instead of a name spaced ref for updated
or deleted branches. The ci_event.rs module's unit tests have been
fixed to use the correct kind of Git ref in each test. (It used to be
a mess.)

The CI event filter "Branch" now contains a BranchName, previously a
RefString. This should not break anything, as a RefString will always
work as a BranchName.

The DefaultBranch filter now works, because it compares plain branch
names, where it previously would compare a plain and a name spaced
branch name, and those are never equal.

"cibtool trigger" and "cibtool event add" now add branch events with a plain
branch instead of a name spaced ref.

Signed-off-by: Lars Wirzenius <liw@liw.fi>

ci-broker.md

@@ -933,8 +933,8 @@ when I run cibtool --db ci-broker.db event add --repo xyzzy --ref oksa --commit
 when I run ./env.sh cib --config config.yaml queued
 
 when I run cibtool --db ci-broker.db run list --json
-then stdout contains "/refs/heads/main"
+then stdout contains ""main""
-then stdout doesn't contain "/refs/heads/oksa"
+then stdout doesn't contain "oksa"
 ~~~
 
 ~~~{#filter-branchcreated.yaml .file .json}

doc/userguide.md

@@ -121,7 +121,7 @@ created, but that is not certain.
 |:----------------|:--------------------|:------------|
 | `BranchCreated` | `from_node`         | `NodeId`    |
 |                 | `repo`              | `RepoId`    |
-|                 | `branch`            | `RefSting`  |
+|                 | `branch`            | `BranchName`|
 |                 | `tip`               | `Oid`       |
 
 ## `BranchUpdated`
@@ -132,7 +132,7 @@ A branch has been updated.
 |:----------------|:------------|:------------|
 | `BranchUpdated` | `from_node` | `NodeId`    |
 |                 | `repo`      | `RepoId`    |
-|                 | `branch`    | `RefSting`  |
+|                 | `branch`    | `BranchName`|
 |                 | `tip`       | `Oid`       |
 |                 | `old_tip`   | `Oid`       |
 
@@ -144,8 +144,8 @@ A branch has been deleted.
 |:----------------|:------------|:------------|
 | `BranchDeleted` | `from_node` | `NodeId`    |
 |                 | `repo`      | `RepoId`    |
-|                 | `branch`    | `RefString` |
+|                 | `branch`    | `BranchName`|
-|                 | `tip`       | ` Oid`      |
+|                 | `tip`       | `Oid`       |
 
 ## `PatchCreated`
 

src/bin/cibtool.rs

@@ -21,7 +21,7 @@ use clap::Parser;
 use serde::Serialize;
 
 use radicle::{
-    git::{Oid, RefString},
+    git::Oid,
     prelude::{NodeId, RepoId},
     storage::ReadStorage,
     Profile, Storage,
@@ -306,9 +306,6 @@ enum CibToolError {
     #[error(transparent)]
     Page(#[from] PageError),
 
-    #[error("failed to interpret as a Git reference: {0:?}")]
-    RefString(String, #[source] radicle::git::fmt::Error),
-
     #[error("failed to read event ID from file {0}")]
     ReadEventId(PathBuf, #[source] std::io::Error),
 
@@ -385,4 +382,7 @@ enum CibToolError {
 
     #[error(transparent)]
     Log(#[from] cibtoolcmd::LogError),
+
+    #[error(transparent)]
+    RefError(#[from] radicle_ci_broker::refs::RefError),
 }

src/bin/cibtoolcmd/event.rs

@@ -3,9 +3,11 @@ use std::io::Write;
 use clap::ValueEnum;
 
 use radicle::patch::PatchId;
+use radicle_ci_broker::refs::branch_ref;
 #[allow(unused_imports)] // FIXME
 use radicle_ci_broker::{
-    filter::EventFilter, node_event_source::NodeEventSource, util::read_file_as_objectid,
+    filter::EventFilter, node_event_source::NodeEventSource, refs::ref_string,
+    util::read_file_as_objectid,
 };
 
 use super::*;
@@ -180,16 +182,16 @@ impl Leaf for AddEvent {
             self.lookup_commit(rid, &self.commit)?
         };
 
-        let name = format!("refs/namespaces/{}/refs/heads/{}", nid, self.name.as_str());
+        let branch_name = branch_ref(&ref_string(&self.name).map_err(CibToolError::RefError)?)
-        let name =
+            .map_err(CibToolError::RefError)?;
-            RefString::try_from(name.clone()).map_err(|e| CibToolError::RefString(name, e))?;
 
         let event = match &self.kind {
             EventKind::BranchCreated => {
                 if self.base.is_some() {
                     return Err(CibToolError::NoBaseAllowed);
                 } else {
-                    CiEvent::branch_created(nid, rid, &name, oid).map_err(CibToolError::CiEvent)?
+                    CiEvent::branch_created(nid, rid, &branch_name, oid)
+                        .map_err(CibToolError::CiEvent)?
                 }
             }
             EventKind::BranchUpdated => {
@@ -199,15 +201,14 @@ impl Leaf for AddEvent {
                     } else {
                         self.lookup_commit(rid, base)?
                     };
-                    CiEvent::branch_updated(nid, rid, &name, oid, base)
+                    CiEvent::branch_updated(nid, rid, &branch_name, oid, base)
                         .map_err(CibToolError::CiEvent)?
                 } else {
                     return Err(CibToolError::BaseRequired);
                 }
             }
-            EventKind::BranchDeleted => {
+            EventKind::BranchDeleted => CiEvent::branch_deleted(nid, rid, &branch_name, oid)
-                CiEvent::branch_deleted(nid, rid, &name, oid).map_err(CibToolError::CiEvent)?
+                .map_err(CibToolError::CiEvent)?,
-            }
             EventKind::PatchCreated => {
                 if let Some(patch_id) = &self.patch_id {
                     CiEvent::patch_created(nid, rid, *patch_id, oid)

src/bin/cibtoolcmd/trigger.rs

@@ -1,3 +1,5 @@
+use radicle_ci_broker::refs::{branch_ref, ref_string};
+
 use super::*;
 
 /// Trigger a CI run.
@@ -47,11 +49,10 @@ impl Leaf for TriggerCmd {
         let oid = util::oid_from_cli_arg(&profile, rid, &self.commit)?;
 
         let base = util::lookup_commit(&profile, rid, &format!("{oid}^")).unwrap_or(oid);
-        let name = format!("refs/namespaces/{nid}/refs/heads/{}", self.name.as_str());
+        let branch_name = branch_ref(&ref_string(&self.name).map_err(CibToolError::RefError)?)
-        let name =
+            .map_err(CibToolError::RefError)?;
-            RefString::try_from(name.clone()).map_err(|e| CibToolError::RefString(name, e))?;
+        let event = CiEvent::branch_updated(nid, rid, &branch_name, oid, base)
-        let event =
+            .map_err(CibToolError::CiEvent)?;
-            CiEvent::branch_updated(nid, rid, &name, oid, base).map_err(CibToolError::CiEvent)?;
 
         if self.stdout {
             let json = event.to_pretty_json().map_err(CibToolError::EventToJson2)?;

src/ci_event.rs

@@ -6,14 +6,13 @@ use serde::{Deserialize, Serialize};
 use radicle::{
     cob::patch::PatchId,
     crypto::PublicKey,
-    git::Oid,
+    git::{BranchName, Namespaced, Oid, RefString},
-    git::RefString,
     node::{Event, NodeId},
     prelude::RepoId,
     storage::RefUpdate,
 };
 
-use crate::logger;
+use crate::refs::{branch_from_namespaced, namespaced_from_str};
 
 #[derive(Debug, Eq, PartialEq, Clone, Serialize, Deserialize)]
 #[non_exhaustive]
@@ -28,20 +27,20 @@ pub enum CiEventV1 {
     BranchCreated {
         from_node: NodeId,
         repo: RepoId,
-        branch: RefString,
+        branch: BranchName,
         tip: Oid,
     },
     BranchUpdated {
         from_node: NodeId,
         repo: RepoId,
-        branch: RefString,
+        branch: BranchName,
         tip: Oid,
         old_tip: Oid,
     },
     BranchDeleted {
         from_node: NodeId,
         repo: RepoId,
-        branch: RefString,
+        branch: BranchName,
         tip: Oid,
     },
     PatchCreated {
@@ -81,7 +80,7 @@ impl CiEvent {
         }
     }
 
-    pub fn branch(&self) -> Option<&RefString> {
+    pub fn branch(&self) -> Option<&BranchName> {
         match self {
             Self::V1(CiEventV1::Shutdown) => None,
             Self::V1(CiEventV1::BranchCreated { branch, .. }) => Some(branch),
@@ -117,13 +116,14 @@ impl CiEvent {
     pub fn branch_created(
         from_node: NodeId,
         repo: RepoId,
-        branch: &str,
+        branch: &BranchName,
         tip: Oid,
     ) -> Result<Self, CiEventError> {
+        assert!(!branch.starts_with("refs/"));
         Ok(Self::V1(CiEventV1::BranchCreated {
             from_node,
             repo,
-            branch: ref_string(branch)?,
+            branch: branch.clone(),
             tip,
         }))
     }
@@ -131,14 +131,15 @@ impl CiEvent {
     pub fn branch_updated(
         from_node: NodeId,
         repo: RepoId,
-        branch: &str,
+        branch: &BranchName,
         tip: Oid,
         old_tip: Oid,
     ) -> Result<Self, CiEventError> {
+        assert!(!branch.starts_with("refs/"));
         Ok(Self::V1(CiEventV1::BranchUpdated {
             from_node,
             repo,
-            branch: branch_from_namspeced_ref(branch)?,
+            branch: branch.clone(),
             tip,
             old_tip,
         }))
@@ -147,13 +148,14 @@ impl CiEvent {
     pub fn branch_deleted(
         from_node: NodeId,
         repo: RepoId,
-        branch: &str,
+        branch: &BranchName,
         tip: Oid,
     ) -> Result<Self, CiEventError> {
+        assert!(!branch.starts_with("refs/"));
         Ok(Self::V1(CiEventV1::BranchDeleted {
             from_node,
             repo,
-            branch: branch_from_namspeced_ref(branch)?,
+            branch: branch.clone(),
             tip,
         }))
     }
@@ -188,28 +190,36 @@ impl CiEvent {
                 for update in updated {
                     let e = match update {
                         RefUpdate::Created { name, oid } => {
-                            let origin = originator(name.to_ref_string())?;
+                            let origin = originator(name.to_namespaced().unwrap())?;
                             if let Ok(patch_id) = patch_id(name) {
                                 Self::patch_created(origin, *rid, patch_id, *oid)
-                            } else if let Ok(branch) = namespaced_branch(name) {
+                            } else if let Ok(branch) = namespaced_from_str(name) {
+                                let branch = branch_from_namespaced(&branch)
+                                    .map_err(|err| CiEventError::branch_name(&branch, err))?;
                                 Self::branch_created(origin, *rid, &branch, *oid)?
                             } else {
                                 continue;
                             }
                         }
                         RefUpdate::Updated { name, old, new } => {
-                            let origin = originator(name.to_ref_string())?;
+                            let origin = originator(name.to_namespaced().unwrap())?;
                             if let Ok(patch_id) = patch_id(name) {
                                 Self::patch_updated(origin, *rid, patch_id, *new)
-                            } else if namespaced_branch(name).is_ok() {
+                            } else if let Ok(branch) = namespaced_from_str(name) {
-                                Self::branch_updated(origin, *rid, name, *new, *old)?
+                                let branch = branch_from_namespaced(&branch)
+                                    .map_err(|err| CiEventError::branch_name(&branch, err))?;
+                                Self::branch_updated(origin, *rid, &branch, *new, *old)?
                             } else {
                                 continue;
                             }
                         }
                         RefUpdate::Deleted { name, oid } => {
-                            let origin = originator(name.to_ref_string())?;
+                            let origin = originator(name.to_namespaced().unwrap())?;
-                            Self::branch_deleted(origin, *rid, name, *oid)?
+                            let branch = namespaced_from_str(name)
+                                .map_err(|err| CiEventError::branch_name(name, err))?;
+                            let branch = branch_from_namespaced(&branch)
+                                .map_err(|err| CiEventError::branch_name(&branch, err))?;
+                            Self::branch_deleted(origin, *rid, &branch, *oid)?
                         }
                         RefUpdate::Skipped { .. } => continue,
                     };
@@ -235,22 +245,8 @@ impl CiEvent {
     }
 }
 
-fn originator(name: RefString) -> Result<PublicKey, CiEventError> {
+fn originator(name: Namespaced) -> Result<PublicKey, CiEventError> {
-    if let Some(qualified) = name.clone().into_qualified() {
+    PublicKey::from_namespaced(&name).map_err(|err| CiEventError::key_from_namespaced(&name, err))
-        if let Some(namespaced) = qualified.to_namespaced() {
-            return PublicKey::from_namespaced(&namespaced)
-                .map_err(|err| CiEventError::key_from_namespaced(&name, err));
-        }
-    }
-    Err(CiEventError::without_namespace2(name.as_str()))
-}
-
-fn ref_string(s: &str) -> Result<RefString, CiEventError> {
-    RefString::try_from(s).map_err(|e| CiEventError::ref_string(s, e))
-}
-
-fn branch_from_namspeced_ref(branch: &str) -> Result<RefString, CiEventError> {
-    ref_string(&namespaced_branch(branch).map_err(|_| CiEventError::without_namespace2(branch))?)
 }
 
 pub struct CiEvents {
@@ -277,8 +273,8 @@ pub enum CiEventError {
     #[error("updated ref name has no name space: {0:?})")]
     WithoutNamespace2(String),
 
-    #[error("failed to create a RefString from {0:?}")]
+    #[error("failed to create a branch name from {0:?}")]
-    RefString(String, radicle::git::fmt::Error),
+    BranchName(String, crate::refs::RefError),
 
     #[error("failed to read broker events file {0}")]
     ReadFile(PathBuf, #[source] std::io::Error),
@@ -294,15 +290,14 @@ pub enum CiEventError {
 
     #[error("failed to encode CI event as JSON")]
     ToJson(#[source] serde_json::Error),
+
+    #[error(transparent)]
+    Parwe(#[from] ParseError),
 }
 
 impl CiEventError {
-    fn without_namespace2(refname: &str) -> Self {
+    fn branch_name(name: &str, err: crate::refs::RefError) -> Self {
-        Self::WithoutNamespace2(refname.into())
+        Self::BranchName(name.into(), err)
-    }
-
-    fn ref_string(name: &str, err: radicle::git::fmt::Error) -> Self {
-        Self::RefString(name.into(), err)
     }
 
     fn read_file(filename: &Path, err: std::io::Error) -> Self {
@@ -317,7 +312,7 @@ impl CiEventError {
         Self::NotJson(filename.into(), err)
     }
 
-    fn key_from_namespaced(name: &RefString, err: radicle_crypto::PublicKeyError) -> Self {
+    fn key_from_namespaced(name: &Namespaced, err: radicle_crypto::PublicKeyError) -> Self {
         Self::KeyFromNamespaced(name.to_ref_string(), err)
     }
 }
@@ -329,9 +324,10 @@ mod test {
     use radicle::{prelude::NodeId, storage::RefUpdate};
     use std::str::FromStr;
 
+    use crate::refs::ref_string;
+
     const MAIN_BRANCH_REF_NAME: &str =
         "refs/namespaces/z6MkiB8T5cBEQHnrs2MgjMVqvpSVj42X81HjKfFi2XBoMbtr/refs/heads/main";
-    const MAIN_BRANCH_NAME: &str = "main";
 
     const PATCH_REF_NAME: &str =
         "refs/namespaces/z6MkiB8T5cBEQHnrs2MgjMVqvpSVj42X81HjKfFi2XBoMbtr/refs/heads/patches/f9fa90725474de9002be503ae3cda4670c9a174";
@@ -356,8 +352,16 @@ mod test {
         oid_from(OID)
     }
 
-    fn refstring(s: &str) -> RefString {
+    fn namespaced_main<'a>() -> Namespaced<'a> {
-        RefString::try_from(s).unwrap()
+        ref_string(MAIN_BRANCH_REF_NAME)
+            .unwrap()
+            .to_namespaced()
+            .unwrap()
+            .to_owned()
+    }
+
+    fn plain_main() -> BranchName {
+        branch_from_namespaced(&namespaced_main()).unwrap()
     }
 
     #[test]
@@ -378,7 +382,7 @@ mod test {
             remote: nid(),
             rid: rid(),
             updated: vec![RefUpdate::Skipped {
-                name: refstring(MAIN_BRANCH_REF_NAME),
+                name: ref_string(MAIN_BRANCH_REF_NAME).unwrap(),
                 oid: oid(),
             }],
         };
@@ -391,13 +395,12 @@ mod test {
     #[test]
     fn branch_created() {
         let rid = rid();
-        let main = refstring(MAIN_BRANCH_NAME);
         let oid = oid();
         let event = Event::RefsFetched {
             remote: nid(),
             rid,
             updated: vec![RefUpdate::Created {
-                name: refstring(MAIN_BRANCH_REF_NAME),
+                name: namespaced_main().to_ref_string(),
                 oid,
             }],
         };
@@ -413,7 +416,7 @@ mod test {
                             repo,
                             branch,
                             tip,
-                        }) if repo == rid && branch == main && tip == oid => {}
+                        }) if repo == rid && branch == plain_main() && tip == oid => {}
                         _ => panic!("should not succeed that way"),
                     }
                 }
@@ -425,13 +428,12 @@ mod test {
     #[test]
     fn branch_updated() {
         let rid = rid();
-        let main = refstring(MAIN_BRANCH_NAME);
         let oid = oid();
         let event = Event::RefsFetched {
             remote: nid(),
             rid,
             updated: vec![RefUpdate::Updated {
-                name: refstring(MAIN_BRANCH_REF_NAME),
+                name: namespaced_main().to_ref_string(),
                 old: oid,
                 new: oid,
             }],
@@ -449,7 +451,10 @@ mod test {
                             branch,
                             tip,
                             old_tip,
-                        }) if repo == rid && branch == main && tip == oid && old_tip == oid => {}
+                        }) if repo == rid
+                            && branch == plain_main()
+                            && tip == oid
+                            && old_tip == oid => {}
                         _ => panic!("should not succeed that way"),
                     }
                 }
@@ -461,13 +466,12 @@ mod test {
     #[test]
     fn branch_deleted() {
         let rid = rid();
-        let main = refstring(MAIN_BRANCH_NAME);
         let oid = oid();
         let event = Event::RefsFetched {
             remote: nid(),
             rid,
             updated: vec![RefUpdate::Deleted {
-                name: refstring(MAIN_BRANCH_REF_NAME),
+                name: namespaced_main().to_ref_string(),
                 oid,
             }],
         };
@@ -480,7 +484,7 @@ mod test {
                     match e {
                         CiEvent::V1(CiEventV1::BranchDeleted {
                             repo, branch, tip, ..
-                        }) if repo == rid && branch == main && tip == oid => {}
+                        }) if repo == rid && branch == plain_main() && tip == oid => {}
                         _ => panic!("should not succeed that way"),
                     }
                 }
@@ -498,7 +502,7 @@ mod test {
             remote: nid(),
             rid,
             updated: vec![RefUpdate::Created {
-                name: refstring(PATCH_REF_NAME),
+                name: ref_string(PATCH_REF_NAME).unwrap(),
                 oid,
             }],
         };
@@ -532,7 +536,7 @@ mod test {
             remote: nid(),
             rid,
             updated: vec![RefUpdate::Updated {
-                name: refstring(PATCH_REF_NAME),
+                name: ref_string(PATCH_REF_NAME).unwrap(),
                 old: oid,
                 new: oid,
             }],
@@ -559,20 +563,6 @@ mod test {
     }
 }
 
-pub fn namespaced_branch(refname: &str) -> Result<String, ParseError> {
-    logger::debug2(format!("namespaced_branch: refname={refname:?}"));
-    const PAT_BRANCH: &str = r"^refs/namespaces/[^/]+/refs/heads/(.+)$";
-    let push_re = Regex::new(PAT_BRANCH).map_err(|e| ParseError::Regex(PAT_BRANCH, e))?;
-    if let Some(push_captures) = push_re.captures(refname) {
-        if let Some(branch) = push_captures.get(1) {
-            let branch = branch.as_str().to_string();
-            logger::debug2(format!("namespaced_branch: result={branch:?}"));
-            return Ok(branch);
-        }
-    }
-    Err(ParseError::NotBranch(refname.into()))
-}
-
 fn patch_id(refname: &str) -> Result<PatchId, ParseError> {
     const PAT_PATCH: &str = r"^refs/namespaces/[^/]+/refs/heads/patches/([^/]+)$";
     let patch_re = Regex::new(PAT_PATCH).map_err(|e| ParseError::regex(PAT_PATCH, e))?;
@@ -600,6 +590,9 @@ pub enum ParseError {
 
     #[error("Git ref name includes unacceptable Git object id: {0:?}")]
     Oid(String, radicle::git::raw::Error),
+
+    #[error("failed to create a Git branch name from {0:?}")]
+    BranchName(String, #[source] crate::refs::RefError),
 }
 
 impl ParseError {
@@ -616,42 +609,6 @@ impl ParseError {
     }
 }
 
-#[cfg(test)]
-#[allow(clippy::unwrap_used)]
-mod test_namespaced_branch {
-    use super::{namespaced_branch, ParseError};
-
-    #[test]
-    fn empty() {
-        assert!(matches!(
-            namespaced_branch(""),
-            Err(ParseError::NotBranch(_))
-        ));
-    }
-
-    #[test]
-    fn lacks_namespace() {
-        assert!(matches!(
-            namespaced_branch(""),
-            Err(ParseError::NotBranch(_))
-        ));
-    }
-
-    #[test]
-    fn has_namespace() {
-        let x = namespaced_branch("refs/namespaces/DID/refs/heads/main");
-        assert!(x.is_ok());
-        assert_eq!(x.unwrap(), "main");
-    }
-
-    #[test]
-    fn has_namespace_with_path() {
-        let x = namespaced_branch("refs/namespaces/DID/refs/heads/liw/debug/this/path");
-        assert!(x.is_ok());
-        assert_eq!(x.unwrap(), "liw/debug/this/path");
-    }
-}
-
 #[cfg(test)]
 #[allow(clippy::unwrap_used)]
 mod test_patch_id {

src/filter.rs

@@ -4,15 +4,16 @@ use serde::{Deserialize, Serialize};
 
 use radicle::{
     cob::patch::PatchId,
-    git::{raw::ObjectType, Oid, RefString},
+    git::{raw::ObjectType, BranchName, Oid},
     node::NodeId,
     prelude::{Profile, RepoId},
     storage::git::Repository,
 };
 
 use crate::{
-    ci_event::{namespaced_branch, CiEvent, CiEventV1},
+    ci_event::{CiEvent, CiEventV1},
     config::TriggerConfig,
+    refs::ref_string,
 };
 
 #[cfg(test)]
@@ -51,7 +52,7 @@ pub enum EventFilter {
     Repository(RepoId),
 
     /// Event is for a specific branch.
-    Branch(RefString),
+    Branch(BranchName),
 
     /// Event if for the default branch for the repository.
     DefaultBranch,
@@ -209,7 +210,8 @@ impl EventFilter {
     }
 
     pub fn allows(&self, event: &CiEvent) -> bool {
-        self.decide(event).allowed
+        let dec = self.decide(event);
+        dec.allowed
     }
 
     pub fn from_file(filename: &Path) -> Result<Vec<Self>, FilterError> {
@@ -279,29 +281,21 @@ impl Decision {
 }
 
 fn is_default_branch(repo_id: &RepoId, wanted: &str) -> bool {
-    if let Ok(default) = get_default_branch(repo_id) {
+    if let Ok(wanted) = ref_string(wanted) {
-        if let Ok(default) = namespaced_branch(&default) {
+        if let Ok(default) = get_default_branch(repo_id) {
-            if let Ok(wanted) = namespaced_branch(wanted) {
+            return wanted == default;
-                return default == wanted;
-            }
         }
     }
+
     false
 }
 
-pub fn get_default_branch(repo_id: &RepoId) -> Result<String, Box<dyn std::error::Error>> {
+pub fn get_default_branch(repo_id: &RepoId) -> Result<BranchName, Box<dyn std::error::Error>> {
     let profile = Profile::load()?;
     let path = profile.storage.path().join(repo_id.canonical());
     let repo = Repository::open(path, *repo_id)?;
     let proj = repo.project()?;
-
+    Ok(proj.default_branch().clone())
-    let def = format!(
-        "{}/refs/heads/{}",
-        profile.id().to_namespace(),
-        proj.default_branch()
-    );
-
-    Ok(def)
 }
 
 fn has_file(repo_id: &RepoId, oid: &Oid, filename: &Path) -> bool {
@@ -354,6 +348,8 @@ mod test {
     use qcheck_macros::quickcheck;
     use radicle::prelude::{Did, RepoId};
 
+    use crate::refs::branch_from_str;
+
     use super::*;
 
     fn did() -> Did {
@@ -396,10 +392,6 @@ mod test {
         PatchId::from(other_oid())
     }
 
-    fn refstring(s: &str) -> RefString {
-        RefString::try_from(s).unwrap()
-    }
-
     fn shutdown() -> CiEvent {
         CiEvent::V1(CiEventV1::Shutdown)
     }
@@ -407,7 +399,7 @@ mod test {
     fn all_events(
         did: Did,
         repo: RepoId,
-        branch: RefString,
+        branch: BranchName,
         patch: PatchId,
         tip: Oid,
         old_tip: Oid,
@@ -458,7 +450,14 @@ mod test {
     #[test]
     fn allows_all_for_default_repository() {
         let filter = EventFilter::Repository(rid());
-        let events = all_events(did(), rid(), refstring("main"), patch_id(), oid(), oid());
+        let events = all_events(
+            did(),
+            rid(),
+            branch_from_str("main").unwrap(),
+            patch_id(),
+            oid(),
+            oid(),
+        );
         assert!(events.iter().all(|e| filter.allows(e)));
     }
 
@@ -468,7 +467,7 @@ mod test {
         let events = all_events(
             did(),
             other_rid(),
-            refstring("main"),
+            branch_from_str("main").unwrap(),
             patch_id(),
             oid(),
             oid(),
@@ -482,8 +481,15 @@ mod test {
 
     #[test]
     fn allows_all_for_main_branch() {
-        let filter = EventFilter::Branch(refstring("main"));
+        let filter = EventFilter::Branch(branch_from_str("main").unwrap());
-        let events = all_events(did(), rid(), refstring("main"), patch_id(), oid(), oid());
+        let events = all_events(
+            did(),
+            rid(),
+            branch_from_str("main").unwrap(),
+            patch_id(),
+            oid(),
+            oid(),
+        );
         eprintln!("filter: {filter:#?}");
         for e in events.iter().filter(|e| {
             matches!(
@@ -500,11 +506,11 @@ mod test {
 
     #[test]
     fn doesnt_allow_any_for_other_branch() {
-        let filter = EventFilter::Branch(refstring("main"));
+        let filter = EventFilter::Branch(branch_from_str("main").unwrap());
         let events = all_events(
             did(),
             other_rid(),
-            refstring("other"),
+            branch_from_str("other").unwrap(),
             patch_id(),
             oid(),
             oid(),
@@ -521,9 +527,16 @@ mod test {
         let filter = EventFilter::BranchCreated;
 
         eprintln!("filter: {filter:#?}");
-        for e in all_events(did(), rid(), refstring("main"), patch_id(), oid(), oid())
+        for e in all_events(
-            .iter()
+            did(),
-            .filter(|e| matches!(e, CiEvent::V1(CiEventV1::BranchCreated { .. })))
+            rid(),
+            branch_from_str("main").unwrap(),
+            patch_id(),
+            oid(),
+            oid(),
+        )
+        .iter()
+        .filter(|e| matches!(e, CiEvent::V1(CiEventV1::BranchCreated { .. })))
         {
             eprintln!("{:#?} → {}", e, filter.allows(e));
             assert!(filter.allows(e));
@@ -535,9 +548,16 @@ mod test {
         let filter = EventFilter::BranchCreated;
 
         eprintln!("filter: {filter:#?}");
-        for e in all_events(did(), rid(), refstring("main"), patch_id(), oid(), oid())
+        for e in all_events(
-            .iter()
+            did(),
-            .filter(|e| !matches!(e, CiEvent::V1(CiEventV1::BranchCreated { .. })))
+            rid(),
+            branch_from_str("main").unwrap(),
+            patch_id(),
+            oid(),
+            oid(),
+        )
+        .iter()
+        .filter(|e| !matches!(e, CiEvent::V1(CiEventV1::BranchCreated { .. })))
         {
             eprintln!("{:#?} → {}", e, filter.allows(e));
             assert!(!filter.allows(e));
@@ -549,9 +569,16 @@ mod test {
         let filter = EventFilter::BranchUpdated;
 
         eprintln!("filter: {filter:#?}");
-        for e in all_events(did(), rid(), refstring("main"), patch_id(), oid(), oid())
+        for e in all_events(
-            .iter()
+            did(),
-            .filter(|e| matches!(e, CiEvent::V1(CiEventV1::BranchUpdated { .. })))
+            rid(),
+            branch_from_str("main").unwrap(),
+            patch_id(),
+            oid(),
+            oid(),
+        )
+        .iter()
+        .filter(|e| matches!(e, CiEvent::V1(CiEventV1::BranchUpdated { .. })))
         {
             eprintln!("{:#?} → {}", e, filter.allows(e));
             assert!(filter.allows(e));
@@ -563,9 +590,16 @@ mod test {
         let filter = EventFilter::BranchUpdated;
 
         eprintln!("filter: {filter:#?}");
-        for e in all_events(did(), rid(), refstring("main"), patch_id(), oid(), oid())
+        for e in all_events(
-            .iter()
+            did(),
-            .filter(|e| !matches!(e, CiEvent::V1(CiEventV1::BranchUpdated { .. })))
+            rid(),
+            branch_from_str("main").unwrap(),
+            patch_id(),
+            oid(),
+            oid(),
+        )
+        .iter()
+        .filter(|e| !matches!(e, CiEvent::V1(CiEventV1::BranchUpdated { .. })))
         {
             eprintln!("{:#?} → {}", e, filter.allows(e));
             assert!(!filter.allows(e));
@@ -577,9 +611,16 @@ mod test {
         let filter = EventFilter::BranchDeleted;
 
         eprintln!("filter: {filter:#?}");
-        for e in all_events(did(), rid(), refstring("main"), patch_id(), oid(), oid())
+        for e in all_events(
-            .iter()
+            did(),
-            .filter(|e| matches!(e, CiEvent::V1(CiEventV1::BranchDeleted { .. })))
+            rid(),
+            branch_from_str("main").unwrap(),
+            patch_id(),
+            oid(),
+            oid(),
+        )
+        .iter()
+        .filter(|e| matches!(e, CiEvent::V1(CiEventV1::BranchDeleted { .. })))
         {
             eprintln!("{:#?} → {}", e, filter.allows(e));
             assert!(filter.allows(e));
@@ -591,9 +632,16 @@ mod test {
         let filter = EventFilter::BranchDeleted;
 
         eprintln!("filter: {filter:#?}");
-        for e in all_events(did(), rid(), refstring("main"), patch_id(), oid(), oid())
+        for e in all_events(
-            .iter()
+            did(),
-            .filter(|e| !matches!(e, CiEvent::V1(CiEventV1::BranchDeleted { .. })))
+            rid(),
+            branch_from_str("main").unwrap(),
+            patch_id(),
+            oid(),
+            oid(),
+        )
+        .iter()
+        .filter(|e| !matches!(e, CiEvent::V1(CiEventV1::BranchDeleted { .. })))
         {
             eprintln!("{:#?} → {}", e, filter.allows(e));
             assert!(!filter.allows(e));
@@ -603,7 +651,14 @@ mod test {
     #[test]
     fn allows_specific_patch() {
         let filter = EventFilter::Patch(oid());
-        let events = all_events(did(), rid(), refstring("main"), patch_id(), oid(), oid());
+        let events = all_events(
+            did(),
+            rid(),
+            branch_from_str("main").unwrap(),
+            patch_id(),
+            oid(),
+            oid(),
+        );
         eprintln!("filter: {filter:#?}");
         for e in events.iter().filter(|e| {
             matches!(
@@ -623,7 +678,7 @@ mod test {
         let events = all_events(
             did(),
             rid(),
-            refstring("main"),
+            branch_from_str("main").unwrap(),
             other_patch_id(),
             oid(),
             oid(),
@@ -646,9 +701,16 @@ mod test {
         let filter = EventFilter::PatchCreated;
 
         eprintln!("filter: {filter:#?}");
-        for e in all_events(did(), rid(), refstring("main"), patch_id(), oid(), oid())
+        for e in all_events(
-            .iter()
+            did(),
-            .filter(|e| matches!(e, CiEvent::V1(CiEventV1::PatchCreated { .. })))
+            rid(),
+            branch_from_str("main").unwrap(),
+            patch_id(),
+            oid(),
+            oid(),
+        )
+        .iter()
+        .filter(|e| matches!(e, CiEvent::V1(CiEventV1::PatchCreated { .. })))
         {
             eprintln!("{:#?} → {}", e, filter.allows(e));
             assert!(filter.allows(e));
@@ -660,9 +722,16 @@ mod test {
         let filter = EventFilter::PatchCreated;
 
         eprintln!("filter: {filter:#?}");
-        for e in all_events(did(), rid(), refstring("main"), patch_id(), oid(), oid())
+        for e in all_events(
-            .iter()
+            did(),
-            .filter(|e| !matches!(e, CiEvent::V1(CiEventV1::PatchCreated { .. })))
+            rid(),
+            branch_from_str("main").unwrap(),
+            patch_id(),
+            oid(),
+            oid(),
+        )
+        .iter()
+        .filter(|e| !matches!(e, CiEvent::V1(CiEventV1::PatchCreated { .. })))
         {
             eprintln!("{:#?} → {}", e, filter.allows(e));
             assert!(!filter.allows(e));
@@ -674,9 +743,16 @@ mod test {
         let filter = EventFilter::PatchUpdated;
 
         eprintln!("filter: {filter:#?}");
-        for e in all_events(did(), rid(), refstring("main"), patch_id(), oid(), oid())
+        for e in all_events(
-            .iter()
+            did(),
-            .filter(|e| matches!(e, CiEvent::V1(CiEventV1::PatchUpdated { .. })))
+            rid(),
+            branch_from_str("main").unwrap(),
+            patch_id(),
+            oid(),
+            oid(),
+        )
+        .iter()
+        .filter(|e| matches!(e, CiEvent::V1(CiEventV1::PatchUpdated { .. })))
         {
             eprintln!("{:#?} → {}", e, filter.allows(e));
             assert!(filter.allows(e));
@@ -688,9 +764,16 @@ mod test {
         let filter = EventFilter::PatchUpdated;
 
         eprintln!("filter: {filter:#?}");
-        for e in all_events(did(), rid(), refstring("main"), patch_id(), oid(), oid())
+        for e in all_events(
-            .iter()
+            did(),
-            .filter(|e| !matches!(e, CiEvent::V1(CiEventV1::PatchUpdated { .. })))
+            rid(),
+            branch_from_str("main").unwrap(),
+            patch_id(),
+            oid(),
+            oid(),
+        )
+        .iter()
+        .filter(|e| !matches!(e, CiEvent::V1(CiEventV1::PatchUpdated { .. })))
         {
             eprintln!("{:#?} → {}", e, filter.allows(e));
             assert!(!filter.allows(e));
@@ -700,14 +783,28 @@ mod test {
     #[test]
     fn allows_all_for_right_node() {
         let filter = EventFilter::Node(*did());
-        let events = all_events(did(), rid(), refstring("main"), patch_id(), oid(), oid());
+        let events = all_events(
+            did(),
+            rid(),
+            branch_from_str("main").unwrap(),
+            patch_id(),
+            oid(),
+            oid(),
+        );
         assert!(events.iter().all(|e| filter.allows(e)));
     }
 
     #[test]
     fn allows_none_for_wrong_node() {
         let filter = EventFilter::Node(*other_did());
-        let events = all_events(did(), rid(), refstring("main"), patch_id(), oid(), oid());
+        let events = all_events(
+            did(),
+            rid(),
+            branch_from_str("main").unwrap(),
+            patch_id(),
+            oid(),
+            oid(),
+        );
         assert!(!events.iter().any(|e| filter.allows(e)));
     }
 
@@ -716,7 +813,16 @@ mod test {
         let filter = EventFilter::Allow;
 
         eprintln!("filter: {filter:#?}");
-        for e in all_events(did(), rid(), refstring("main"), patch_id(), oid(), oid()).iter() {
+        for e in all_events(
+            did(),
+            rid(),
+            branch_from_str("main").unwrap(),
+            patch_id(),
+            oid(),
+            oid(),
+        )
+        .iter()
+        {
             eprintln!("{:#?} → {}", e, filter.allows(e));
             assert!(filter.allows(e));
         }
@@ -727,7 +833,16 @@ mod test {
         let filter = EventFilter::Deny;
 
         eprintln!("filter: {filter:#?}");
-        for e in all_events(did(), rid(), refstring("main"), patch_id(), oid(), oid()).iter() {
+        for e in all_events(
+            did(),
+            rid(),
+            branch_from_str("main").unwrap(),
+            patch_id(),
+            oid(),
+            oid(),
+        )
+        .iter()
+        {
             eprintln!("{:#?} → {}", e, filter.allows(e));
             assert!(!filter.allows(e));
         }
@@ -738,7 +853,16 @@ mod test {
         let filter = EventFilter::Not(vec![EventFilter::Deny]);
 
         eprintln!("filter: {filter:#?}");
-        for e in all_events(did(), rid(), refstring("main"), patch_id(), oid(), oid()).iter() {
+        for e in all_events(
+            did(),
+            rid(),
+            branch_from_str("main").unwrap(),
+            patch_id(),
+            oid(),
+            oid(),
+        )
+        .iter()
+        {
             eprintln!("{:#?} → {}", e, filter.allows(e));
             assert!(filter.allows(e));
         }
@@ -749,7 +873,16 @@ mod test {
         let filter = EventFilter::And(vec![EventFilter::Allow, EventFilter::Allow]);
 
         eprintln!("filter: {filter:#?}");
-        for e in all_events(did(), rid(), refstring("main"), patch_id(), oid(), oid()).iter() {
+        for e in all_events(
+            did(),
+            rid(),
+            branch_from_str("main").unwrap(),
+            patch_id(),
+            oid(),
+            oid(),
+        )
+        .iter()
+        {
             eprintln!("{:#?} → {}", e, filter.allows(e));
             assert!(filter.allows(e));
         }
@@ -760,7 +893,16 @@ mod test {
         let filter = EventFilter::Or(vec![EventFilter::Deny, EventFilter::Allow]);
 
         eprintln!("filter: {filter:#?}");
-        for e in all_events(did(), rid(), refstring("main"), patch_id(), oid(), oid()).iter() {
+        for e in all_events(
+            did(),
+            rid(),
+            branch_from_str("main").unwrap(),
+            patch_id(),
+            oid(),
+            oid(),
+        )
+        .iter()
+        {
             eprintln!("{:#?} → {}", e, filter.allows(e));
             assert!(filter.allows(e));
         }

src/logger.rs

@@ -524,11 +524,12 @@ pub fn queueadd_control_socket_close() {
     );
 }
 
-pub fn queueadd_push_event(event: &CiEvent) {
+pub fn queueadd_push_event(event: &CiEvent, id: &QueueId) {
     debug!(
         msg_id = ?Id::QueueAddEnqueueEvent,
         kind = %Kind::GotEvent,
         ?event,
+        ?id,
         "insert broker event into queue"
     );
 }

src/queueadd.rs

@@ -5,7 +5,7 @@ use radicle::Profile;
 use crate::{
     ci_event::CiEvent,
     ci_event_source::{CiEventSource, CiEventSourceError},
-    db::{Db, DbError},
+    db::{Db, DbError, QueueId},
     logger,
     notif::NotificationSender,
 };
@@ -67,8 +67,8 @@ impl QueueAdder {
                 }
                 Ok(Some(events)) => {
                     for e in events {
-                        logger::queueadd_push_event(&e);
+                        let id = self.push_event(e.clone())?;
-                        self.push_event(e.clone())?;
+                        logger::queueadd_push_event(&e, &id);
                     }
                 }
             }
@@ -78,10 +78,10 @@ impl QueueAdder {
         Ok(())
     }
 
-    fn push_event(&self, e: CiEvent) -> Result<(), AdderError> {
+    fn push_event(&self, e: CiEvent) -> Result<QueueId, AdderError> {
-        self.db.push_queued_ci_event(e)?;
+        let id = self.db.push_queued_ci_event(e)?;
         self.events_tx.notify().map_err(|_| AdderError::Send)?;
-        Ok(())
+        Ok(id)
     }
 }