remote-helper: refactor pushing action

Reduce the nesting of if statements by using a `PushAction` enum that is
constructed by checking the `dst` reference and parsing the correct action, one
of:

- `OpenPatch`
- `UpdatePatch`
- `PushRef`

This makes the intent of the code a little bit more clear and reduces the tabbing
of big code blocks. It also captures the error specifically for handling the
parsing of this `dst` reference.
This commit is contained in:
Fintan Halpenny 2025-07-10 08:55:08 +01:00
parent a9f75d47ec
commit 14444a43ea
2 changed files with 74 additions and 28 deletions

View File

@ -1,4 +1,7 @@
#![allow(clippy::too_many_arguments)] #![allow(clippy::too_many_arguments)]
mod error;
use std::collections::HashMap; use std::collections::HashMap;
use std::io::IsTerminal; use std::io::IsTerminal;
use std::path::{Path, PathBuf}; use std::path::{Path, PathBuf};
@ -66,9 +69,6 @@ pub enum Error {
/// Invalid reference name. /// Invalid reference name.
#[error("invalid ref: {0}")] #[error("invalid ref: {0}")]
InvalidRef(#[from] radicle::git::fmt::Error), InvalidRef(#[from] radicle::git::fmt::Error),
/// Invalid reference name.
#[error("invalid qualified ref: {0}")]
InvalidQualifiedRef(git::RefString),
/// Git error. /// Git error.
#[error("git: {0}")] #[error("git: {0}")]
Git(#[from] git::raw::Error), Git(#[from] git::raw::Error),
@ -119,6 +119,8 @@ pub enum Error {
Quorum(#[from] radicle::git::canonical::QuorumError), Quorum(#[from] radicle::git::canonical::QuorumError),
#[error(transparent)] #[error(transparent)]
CanonicalRefs(#[from] radicle::identity::doc::CanonicalRefsError), CanonicalRefs(#[from] radicle::identity::doc::CanonicalRefsError),
#[error(transparent)]
PushAction(#[from] error::PushAction),
} }
/// Push command. /// Push command.
@ -196,6 +198,43 @@ impl Command {
} }
} }
enum PushAction {
OpenPatch,
UpdatePatch {
dst: git::Qualified<'static>,
patch: patch::PatchId,
},
PushRef {
dst: git::Qualified<'static>,
},
}
impl PushAction {
fn new(dst: &git::RefString) -> Result<Self, error::PushAction> {
if dst == &*rad::PATCHES_REFNAME {
Ok(Self::OpenPatch)
} else {
let dst = git::Qualified::from_refstr(dst)
.ok_or_else(|| error::PushAction::InvalidRef {
refname: dst.clone(),
})?
.to_owned();
if let Some(oid) = dst.strip_prefix(git::refname!("refs/heads/patches")) {
let patch = git::Oid::from_str(oid)
.map_err(|err| error::PushAction::InvalidPatchId {
suffix: oid.to_string(),
err,
})
.map(patch::PatchId::from)?;
Ok(Self::UpdatePatch { dst, patch })
} else {
Ok(Self::PushRef { dst })
}
}
}
}
/// Run a git push command. /// Run a git push command.
pub fn run( pub fn run(
mut specs: Vec<String>, mut specs: Vec<String>,
@ -268,9 +307,10 @@ pub fn run(
} }
Command::Push(git::Refspec { src, dst, force }) => { Command::Push(git::Refspec { src, dst, force }) => {
let patches = crate::patches_mut(profile, stored)?; let patches = crate::patches_mut(profile, stored)?;
let action = PushAction::new(dst)?;
if dst == &*rad::PATCHES_REFNAME { match action {
patch_open( PushAction::OpenPatch => patch_open(
src, src,
&remote, &remote,
&nid, &nid,
@ -280,27 +320,20 @@ pub fn run(
&signer, &signer,
profile, profile,
opts.clone(), opts.clone(),
) ),
} else { PushAction::UpdatePatch { dst, patch } => patch_update(
let dst = git::Qualified::from_refstr(dst) src,
.ok_or_else(|| Error::InvalidQualifiedRef(dst.clone()))?; &dst,
*force,
if let Some(oid) = dst.strip_prefix(git::refname!("refs/heads/patches")) { patch,
let oid = git::Oid::from_str(oid)?; &nid,
&working,
patch_update( stored,
src, patches,
&dst, &signer,
*force, opts.clone(),
&oid, ),
&nid, PushAction::PushRef { dst } => {
&working,
stored,
patches,
&signer,
opts.clone(),
)
} else {
let identity = stored.identity()?; let identity = stored.identity()?;
let crefs = identity.canonical_refs_or_default(|| { let crefs = identity.canonical_refs_or_default(|| {
let rule = identity.doc().default_branch_rule()?; let rule = identity.doc().default_branch_rule()?;
@ -573,7 +606,7 @@ fn patch_update<G>(
src: &git::Oid, src: &git::Oid,
dst: &git::Qualified, dst: &git::Qualified,
force: bool, force: bool,
oid: &git::Oid, patch_id: patch::PatchId,
nid: &NodeId, nid: &NodeId,
working: &git::raw::Repository, working: &git::raw::Repository,
stored: &storage::git::Repository, stored: &storage::git::Repository,
@ -588,7 +621,6 @@ where
G: crypto::signature::Signer<crypto::Signature>, G: crypto::signature::Signer<crypto::Signature>,
{ {
let commit = *src; let commit = *src;
let patch_id = radicle::cob::ObjectId::from(oid);
let dst = dst.with_namespace(nid.into()); let dst = dst.with_namespace(nid.into());
push_ref(src, &dst, force, working, stored.raw())?; push_ref(src, &dst, force, working, stored.raw())?;

View File

@ -0,0 +1,14 @@
use radicle::storage::git;
use thiserror::Error;
#[derive(Debug, Error)]
pub enum PushAction {
#[error("invalid reference {refname}, expected qualified reference starting with `refs/`")]
InvalidRef { refname: git::RefString },
#[error("found refs/heads/patches/{suffix} where {suffix} was an invalid Patch ID")]
InvalidPatchId {
suffix: String,
#[source]
err: git::raw::Error,
},
}