From a4694d48f68c0caf4df8ff59912bec0d52b7fd25 Mon Sep 17 00:00:00 2001 From: Paul Campbell Date: Sun, 12 May 2024 22:27:20 +0100 Subject: [PATCH] refactor(config): replace boilerplate with derive_more --- crates/config/Cargo.toml | 2 +- crates/config/src/branch_name.rs | 19 +++++------------ crates/config/src/forge_config.rs | 21 ++++--------------- crates/config/src/forge_details.rs | 2 +- crates/config/src/forge_name.rs | 7 +------ crates/config/src/forge_type.rs | 2 +- crates/config/src/git_dir.rs | 7 +------ crates/config/src/host_name.rs | 9 +------- crates/config/src/repo_alias.rs | 7 +------ crates/config/src/repo_branches.rs | 21 +++++-------------- crates/config/src/repo_config.rs | 12 ++--------- crates/config/src/repo_path.rs | 9 +------- crates/config/src/server_repo_config.rs | 17 +++------------ crates/config/src/user.rs | 7 +------ crates/git/src/git_ref.rs | 9 ++------ crates/git/src/repo_details.rs | 4 ++-- crates/server/src/actors/repo/webhook.rs | 11 +++++----- .../src/gitforge/forgejo/branch/get_all.rs | 2 +- crates/server/src/gitforge/tests/common.rs | 12 +++++++---- crates/server/src/gitforge/tests/forgejo.rs | 2 +- 20 files changed, 48 insertions(+), 134 deletions(-) diff --git a/crates/config/Cargo.toml b/crates/config/Cargo.toml index f0fd3cb7..9e4800ff 100644 --- a/crates/config/Cargo.toml +++ b/crates/config/Cargo.toml @@ -45,7 +45,7 @@ secrecy = { workspace = true } # warp = { workspace = true } # # # error handling -# derive_more = { workspace = true } +derive_more = { workspace = true } # terrors = { workspace = true } # # # file watcher diff --git a/crates/config/src/branch_name.rs b/crates/config/src/branch_name.rs index 49e6017c..b2f360fd 100644 --- a/crates/config/src/branch_name.rs +++ b/crates/config/src/branch_name.rs @@ -1,17 +1,8 @@ -use std::fmt::Formatter; - /// The name of a Branch -#[derive(Clone, Debug, Hash, PartialEq, Eq)] -pub struct BranchName(pub String); -impl std::fmt::Display for BranchName { - fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { - write!(f, "{}", self.0) - } -} -impl std::ops::Deref for BranchName { - type Target = String; - - fn deref(&self) -> &Self::Target { - &self.0 +#[derive(Clone, Debug, Hash, PartialEq, Eq, derive_more::Display)] +pub struct BranchName(String); +impl BranchName { + pub fn new(str: impl Into) -> Self { + Self(str.into()) } } diff --git a/crates/config/src/forge_config.rs b/crates/config/src/forge_config.rs index b352dc5d..1d2fe194 100644 --- a/crates/config/src/forge_config.rs +++ b/crates/config/src/forge_config.rs @@ -1,12 +1,11 @@ use std::collections::HashMap; -use serde::Deserialize; - use crate::{ApiToken, ForgeType, Hostname, RepoAlias, ServerRepoConfig, User}; /// Defines a Forge to connect to /// Maps from `git-next-server.toml` at `forge.{forge}` -#[derive(Clone, Debug, PartialEq, Eq, Deserialize)] +#[derive(Clone, Debug, PartialEq, Eq, serde::Deserialize, derive_more::Display)] +#[display("{}:{}@{}", forge_type.to_string().to_lowercase(), user, hostname)] pub struct ForgeConfig { pub forge_type: ForgeType, pub hostname: String, @@ -15,9 +14,8 @@ pub struct ForgeConfig { pub repos: HashMap, } impl ForgeConfig { - #[allow(dead_code)] - pub const fn forge_type(&self) -> &ForgeType { - &self.forge_type + pub const fn forge_type(&self) -> ForgeType { + self.forge_type } pub fn hostname(&self) -> Hostname { @@ -38,14 +36,3 @@ impl ForgeConfig { .map(|(name, repo)| (RepoAlias(name.clone()), repo)) } } -impl std::fmt::Display for ForgeConfig { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - write!( - f, - "{}:{}@{}", - self.forge_type.to_string().to_lowercase(), - self.user, - self.hostname - ) - } -} diff --git a/crates/config/src/forge_details.rs b/crates/config/src/forge_details.rs index 96368938..1ea26d28 100644 --- a/crates/config/src/forge_details.rs +++ b/crates/config/src/forge_details.rs @@ -15,7 +15,7 @@ impl From<(&ForgeName, &ForgeConfig)> for ForgeDetails { fn from(forge: (&ForgeName, &ForgeConfig)) -> Self { Self { forge_name: forge.0.clone(), - forge_type: forge.1.forge_type.clone(), + forge_type: forge.1.forge_type(), hostname: forge.1.hostname(), user: forge.1.user(), token: forge.1.token(), diff --git a/crates/config/src/forge_name.rs b/crates/config/src/forge_name.rs index b55bd770..16fd16ca 100644 --- a/crates/config/src/forge_name.rs +++ b/crates/config/src/forge_name.rs @@ -1,13 +1,8 @@ use std::path::PathBuf; /// The name of a Forge to connect to -#[derive(Clone, Debug, PartialEq, Eq)] +#[derive(Clone, Debug, PartialEq, Eq, derive_more::Display)] pub struct ForgeName(pub String); -impl std::fmt::Display for ForgeName { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - write!(f, "{}", self.0) - } -} impl From<&ForgeName> for PathBuf { fn from(value: &ForgeName) -> Self { Self::from(&value.0) diff --git a/crates/config/src/forge_type.rs b/crates/config/src/forge_type.rs index 74d119d3..49225955 100644 --- a/crates/config/src/forge_type.rs +++ b/crates/config/src/forge_type.rs @@ -1,5 +1,5 @@ /// Identifier for the type of Forge -#[derive(Clone, Debug, PartialEq, Eq, serde::Deserialize)] +#[derive(Clone, Copy, Debug, PartialEq, Eq, serde::Deserialize)] pub enum ForgeType { #[cfg(feature = "forgejo")] ForgeJo, diff --git a/crates/config/src/git_dir.rs b/crates/config/src/git_dir.rs index ebbd4ee9..424c4300 100644 --- a/crates/config/src/git_dir.rs +++ b/crates/config/src/git_dir.rs @@ -1,6 +1,6 @@ use std::{ops::Deref, path::PathBuf}; -#[derive(Debug, Clone, PartialEq, Eq, serde::Deserialize)] +#[derive(Debug, Clone, PartialEq, Eq, serde::Deserialize, derive_more::From)] pub struct GitDir(PathBuf); impl GitDir { pub fn new(pathbuf: &std::path::Path) -> Self { @@ -33,11 +33,6 @@ impl From<&GitDir> for PathBuf { value.to_path_buf() } } -impl From for GitDir { - fn from(value: PathBuf) -> Self { - Self(value) - } -} impl From for PathBuf { fn from(value: GitDir) -> Self { value.0 diff --git a/crates/config/src/host_name.rs b/crates/config/src/host_name.rs index b8943a12..6fb93163 100644 --- a/crates/config/src/host_name.rs +++ b/crates/config/src/host_name.rs @@ -1,10 +1,3 @@ -use std::fmt::{Display, Formatter}; - /// The hostname of a forge -#[derive(Clone, Debug, PartialEq, Eq)] +#[derive(Clone, Debug, PartialEq, Eq, derive_more::Display)] pub struct Hostname(pub String); -impl Display for Hostname { - fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { - write!(f, "{}", self.0) - } -} diff --git a/crates/config/src/repo_alias.rs b/crates/config/src/repo_alias.rs index 08ebb27a..1e130ad3 100644 --- a/crates/config/src/repo_alias.rs +++ b/crates/config/src/repo_alias.rs @@ -1,9 +1,4 @@ /// The alias of a repo /// This is the alias for the repo within `git-next-server.toml` -#[derive(Clone, Debug, PartialEq, Eq, Hash)] +#[derive(Clone, Debug, PartialEq, Eq, Hash, derive_more::Display)] pub struct RepoAlias(pub String); -impl std::fmt::Display for RepoAlias { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - write!(f, "{}", self.0) - } -} diff --git a/crates/config/src/repo_branches.rs b/crates/config/src/repo_branches.rs index 6f87c579..2685d6d0 100644 --- a/crates/config/src/repo_branches.rs +++ b/crates/config/src/repo_branches.rs @@ -1,34 +1,23 @@ use crate::BranchName; /// Mapped from `.git-next.toml` file at `branches` -#[derive(Clone, Debug, PartialEq, Eq, serde::Deserialize)] +#[derive(Clone, Debug, PartialEq, Eq, serde::Deserialize, derive_more::Display)] +#[display("{},{},{}", main, next, dev)] pub struct RepoBranches { pub main: String, pub next: String, pub dev: String, } impl RepoBranches { - pub fn new(main: impl Into, next: impl Into, dev: impl Into) -> Self { - Self { - main: main.into(), - next: next.into(), - dev: dev.into(), - } - } pub fn main(&self) -> BranchName { - BranchName(self.main.clone()) + BranchName::new(&self.main) } pub fn next(&self) -> BranchName { - BranchName(self.next.clone()) + BranchName::new(&self.next) } pub fn dev(&self) -> BranchName { - BranchName(self.dev.clone()) - } -} -impl std::fmt::Display for RepoBranches { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - write!(f, "{},{},{}", self.main, self.next, self.dev) + BranchName::new(&self.dev) } } diff --git a/crates/config/src/repo_config.rs b/crates/config/src/repo_config.rs index 4d0e795f..d1ccf06e 100644 --- a/crates/config/src/repo_config.rs +++ b/crates/config/src/repo_config.rs @@ -4,16 +4,13 @@ use crate::RepoConfigSource; /// Mapped from `.git-next.toml` file in target repo /// Is also derived from the optional parameters in `git-next-server.toml` at /// `forge.{forge}.repos.{repo}.(main|next|dev)` -#[derive(Clone, Debug, PartialEq, Eq, serde::Deserialize)] +#[derive(Clone, Debug, PartialEq, Eq, serde::Deserialize, derive_more::Display)] +#[display("{}", branches)] pub struct RepoConfig { pub branches: RepoBranches, pub source: RepoConfigSource, } impl RepoConfig { - pub const fn new(branches: RepoBranches, source: RepoConfigSource) -> Self { - Self { branches, source } - } - pub fn load(toml: &str) -> Result { toml::from_str(format!("source = \"Repo\"\n{}", toml).as_str()) } @@ -26,8 +23,3 @@ impl RepoConfig { self.source } } -impl std::fmt::Display for RepoConfig { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - write!(f, "{}", self.branches) - } -} diff --git a/crates/config/src/repo_path.rs b/crates/config/src/repo_path.rs index 8c7ff16c..16329029 100644 --- a/crates/config/src/repo_path.rs +++ b/crates/config/src/repo_path.rs @@ -1,12 +1,5 @@ -use std::fmt::{Display, Formatter}; - /// The path for the repo within the forge. /// Typically this is composed of the user or organisation and the name of the repo /// e.g. `{user}/{repo}` -#[derive(Clone, Debug, PartialEq, Eq)] +#[derive(Clone, Debug, PartialEq, Eq, derive_more::Display)] pub struct RepoPath(pub String); -impl Display for RepoPath { - fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { - write!(f, "{}", self.0) - } -} diff --git a/crates/config/src/server_repo_config.rs b/crates/config/src/server_repo_config.rs index 38ca7570..13725adc 100644 --- a/crates/config/src/server_repo_config.rs +++ b/crates/config/src/server_repo_config.rs @@ -4,7 +4,8 @@ use crate::{BranchName, GitDir, RepoBranches, RepoConfig, RepoConfigSource, Repo /// Defines a Repo within a ForgeConfig to be monitored by the server /// Maps from `git-next-server.toml` at `forge.{forge}.repos.{name}` -#[derive(Clone, Debug, PartialEq, Eq, serde::Deserialize)] +#[derive(Clone, Debug, PartialEq, Eq, serde::Deserialize, derive_more::Display)] +#[display("{}@{}", repo, branch)] pub struct ServerRepoConfig { pub repo: String, pub branch: String, @@ -18,9 +19,8 @@ impl ServerRepoConfig { RepoPath(self.repo.clone()) } - #[allow(dead_code)] pub fn branch(&self) -> BranchName { - BranchName(self.branch.clone()) + BranchName::new(&self.branch) } pub fn gitdir(&self) -> Option { @@ -42,14 +42,3 @@ impl ServerRepoConfig { } } } -#[cfg(test)] -impl AsRef for ServerRepoConfig { - fn as_ref(&self) -> &Self { - self - } -} -impl std::fmt::Display for ServerRepoConfig { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - write!(f, "{}@{}", self.repo, self.branch) - } -} diff --git a/crates/config/src/user.rs b/crates/config/src/user.rs index e65bc711..d27e2dc2 100644 --- a/crates/config/src/user.rs +++ b/crates/config/src/user.rs @@ -1,8 +1,3 @@ /// The user within the forge to connect as -#[derive(Clone, Debug, PartialEq, Eq)] +#[derive(Clone, Debug, PartialEq, Eq, derive_more::Display)] pub struct User(pub String); -impl std::fmt::Display for User { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - write!(f, "{}", self.0) - } -} diff --git a/crates/git/src/git_ref.rs b/crates/git/src/git_ref.rs index 520860e3..261382c0 100644 --- a/crates/git/src/git_ref.rs +++ b/crates/git/src/git_ref.rs @@ -2,7 +2,7 @@ use git_next_config::BranchName; use crate::Commit; -#[derive(Clone, Debug, Hash, PartialEq, Eq)] +#[derive(Clone, Debug, Hash, PartialEq, Eq, derive_more::Display)] pub struct GitRef(pub String); impl From for GitRef { fn from(value: Commit) -> Self { @@ -11,11 +11,6 @@ impl From for GitRef { } impl From for GitRef { fn from(value: BranchName) -> Self { - Self(value.0) - } -} -impl std::fmt::Display for GitRef { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - write!(f, "{}", self.0) + Self(value.to_string()) } } diff --git a/crates/git/src/repo_details.rs b/crates/git/src/repo_details.rs index bb0aaffe..7afd2f73 100644 --- a/crates/git/src/repo_details.rs +++ b/crates/git/src/repo_details.rs @@ -30,11 +30,11 @@ impl RepoDetails { repo_alias: repo_alias.clone(), repo_path: RepoPath(server_repo_config.repo.clone()), repo_config: server_repo_config.repo_config(), - branch: BranchName(server_repo_config.branch.clone()), + branch: BranchName::new(&server_repo_config.branch), gitdir, forge: ForgeDetails { forge_name: forge_name.clone(), - forge_type: forge_config.forge_type.clone(), + forge_type: forge_config.forge_type(), hostname: forge_config.hostname(), user: forge_config.user(), token: forge_config.token(), diff --git a/crates/server/src/actors/repo/webhook.rs b/crates/server/src/actors/repo/webhook.rs index bdf20fa6..199a298b 100644 --- a/crates/server/src/actors/repo/webhook.rs +++ b/crates/server/src/actors/repo/webhook.rs @@ -1,5 +1,5 @@ use actix::prelude::*; -use git_next_config::RepoBranches; +use git_next_config::{BranchName, RepoBranches}; use git_next_git::{self as git, RepoDetails}; use kxio::network::{self, json}; use tracing::{debug, info, warn}; @@ -286,16 +286,17 @@ impl Push { return None; } let (_, branch) = self.reference.split_at(11); - if branch == *repo_branches.main() { + let branch = BranchName::new(branch); + if branch == repo_branches.main() { return Some(Branch::Main); } - if branch == *repo_branches.next() { + if branch == repo_branches.next() { return Some(Branch::Next); } - if branch == *repo_branches.dev() { + if branch == repo_branches.dev() { return Some(Branch::Dev); } - warn!(branch, "Unexpected branch"); + warn!(%branch, "Unexpected branch"); None } pub fn commit(&self) -> git::Commit { diff --git a/crates/server/src/gitforge/forgejo/branch/get_all.rs b/crates/server/src/gitforge/forgejo/branch/get_all.rs index 648c6cb1..d3d88e09 100644 --- a/crates/server/src/gitforge/forgejo/branch/get_all.rs +++ b/crates/server/src/gitforge/forgejo/branch/get_all.rs @@ -47,6 +47,6 @@ struct Branch { } impl From for BranchName { fn from(value: Branch) -> Self { - Self(value.name) + Self::new(value.name) } } diff --git a/crates/server/src/gitforge/tests/common.rs b/crates/server/src/gitforge/tests/common.rs index b55b5d2c..b359a97d 100644 --- a/crates/server/src/gitforge/tests/common.rs +++ b/crates/server/src/gitforge/tests/common.rs @@ -49,7 +49,7 @@ pub fn repo_details( } pub fn branch_name(n: u32) -> BranchName { - BranchName(format!("branch-name-{}", n)) + BranchName::new(format!("branch-name-{}", n)) } pub fn repo_path(n: u32) -> RepoPath { @@ -61,8 +61,12 @@ pub fn repo_alias(n: u32) -> RepoAlias { } pub fn repo_config(n: u32, source: RepoConfigSource) -> RepoConfig { - RepoConfig::new( - RepoBranches::new(format!("main-{n}"), format!("next-{n}"), format!("dev-{n}")), + RepoConfig { + branches: RepoBranches { + main: format!("main-{n}"), + next: format!("next-{n}"), + dev: format!("dev-{n}"), + }, source, - ) + } } diff --git a/crates/server/src/gitforge/tests/forgejo.rs b/crates/server/src/gitforge/tests/forgejo.rs index b3315448..fe2b628f 100644 --- a/crates/server/src/gitforge/tests/forgejo.rs +++ b/crates/server/src/gitforge/tests/forgejo.rs @@ -55,5 +55,5 @@ async fn test_branches_get() { let_assert!(Some(requests) = net.mocked_requests()); assert_eq!(requests.len(), 1); - assert_eq!(branches, vec![BranchName("string".into())]); + assert_eq!(branches, vec![BranchName::new("string")]); }