From ea20afee12f8f7e760e5641125dbf12cc073d74c Mon Sep 17 00:00:00 2001 From: Paul Campbell Date: Wed, 19 Jun 2024 07:02:18 +0100 Subject: [PATCH] refactor: config: use newtype --- crates/config/src/branch_name.rs | 2 +- .../src/{forge_name.rs => forge_alias.rs} | 2 +- crates/config/src/git_dir.rs | 7 +- crates/config/src/host_name.rs | 2 +- crates/config/src/lib.rs | 4 +- crates/config/src/newtype.rs | 57 +++++++------ crates/config/src/repo_branches.rs | 12 ++- crates/config/src/repo_config.rs | 14 +++- crates/config/src/repo_config_source.rs | 4 +- crates/config/src/server.rs | 17 +--- crates/config/src/tests.rs | 83 +++++++------------ crates/config/src/webhook/auth.rs | 6 +- crates/config/src/webhook/id.rs | 6 +- crates/config/src/webhook/tests.rs | 4 +- crates/forge-forgejo/src/lib.rs | 2 +- crates/git/src/repository/open/tests.rs | 2 +- crates/repo-actor/src/load.rs | 2 +- crates/server/src/config/tests.rs | 2 +- 18 files changed, 105 insertions(+), 123 deletions(-) rename crates/config/src/{forge_name.rs => forge_alias.rs} (69%) diff --git a/crates/config/src/branch_name.rs b/crates/config/src/branch_name.rs index 136f460..1eb81cf 100644 --- a/crates/config/src/branch_name.rs +++ b/crates/config/src/branch_name.rs @@ -1,2 +1,2 @@ // The name of a Branch -crate::newtype!(BranchName is a String); +crate::newtype!(BranchName is a String, derive_more::Display, Default); diff --git a/crates/config/src/forge_name.rs b/crates/config/src/forge_alias.rs similarity index 69% rename from crates/config/src/forge_name.rs rename to crates/config/src/forge_alias.rs index 1cb9892..e53790e 100644 --- a/crates/config/src/forge_name.rs +++ b/crates/config/src/forge_alias.rs @@ -1,5 +1,5 @@ // The name of a Forge to connect to -crate::newtype!(ForgeAlias is a String); +crate::newtype!(ForgeAlias is a String, derive_more::Display, Default); impl From<&ForgeAlias> for std::path::PathBuf { fn from(value: &ForgeAlias) -> Self { Self::from(&value.0) diff --git a/crates/config/src/git_dir.rs b/crates/config/src/git_dir.rs index 9716c01..b89222e 100644 --- a/crates/config/src/git_dir.rs +++ b/crates/config/src/git_dir.rs @@ -1,7 +1,7 @@ // use std::path::PathBuf; -crate::newtype!(GitDir is a PathBuf, without Display); +crate::newtype!(GitDir is a PathBuf, Default); impl GitDir { pub const fn pathbuf(&self) -> &PathBuf { &self.0 @@ -22,8 +22,3 @@ impl From<&GitDir> for PathBuf { value.to_path_buf() } } -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 ca69be4..f38a0e5 100644 --- a/crates/config/src/host_name.rs +++ b/crates/config/src/host_name.rs @@ -1,2 +1,2 @@ // The hostname of a forge -crate::newtype!(Hostname is a String); +crate::newtype!(Hostname is a String, derive_more::Display, Default); diff --git a/crates/config/src/lib.rs b/crates/config/src/lib.rs index 260dee2..ab2cd4e 100644 --- a/crates/config/src/lib.rs +++ b/crates/config/src/lib.rs @@ -3,9 +3,9 @@ mod api_token; mod branch_name; pub mod common; +mod forge_alias; mod forge_config; mod forge_details; -mod forge_name; mod forge_type; pub mod git_dir; mod host_name; @@ -26,9 +26,9 @@ mod tests; pub use api_token::ApiToken; pub use branch_name::BranchName; +pub use forge_alias::ForgeAlias; pub use forge_config::ForgeConfig; pub use forge_details::ForgeDetails; -pub use forge_name::ForgeAlias; pub use forge_type::ForgeType; pub use git_dir::GitDir; pub use host_name::Hostname; diff --git a/crates/config/src/newtype.rs b/crates/config/src/newtype.rs index df2ef6c..9536d22 100644 --- a/crates/config/src/newtype.rs +++ b/crates/config/src/newtype.rs @@ -1,35 +1,10 @@ // #[macro_export] macro_rules! newtype { - ($name:ident is a $type:ty, without Display) => { - #[derive( - Clone, - Default, - Debug, - derive_more::From, - PartialEq, - Eq, - PartialOrd, - Ord, - Hash, - derive_more::AsRef, - derive_more::Deref, - serde::Deserialize, - serde::Serialize, - )] - pub struct $name($type); - impl $name { - pub fn new(value: impl Into<$type>) -> Self { - Self(value.into()) - } - pub fn unwrap(self) -> $type { - self.0 - } - } - }; - ($name:ident is a $type:ty) => { + ($name:ident) => { #[derive( Clone, + Copy, Default, Debug, derive_more::Display, @@ -40,18 +15,42 @@ macro_rules! newtype { Ord, Hash, derive_more::AsRef, + )] + pub struct $name; + impl $name { + pub fn new() -> Self { + Self + } + } + }; + ($name:ident is a $type:ty $(, $derive:ty)*) => { + #[derive( + Clone, + Debug, + derive_more::From, + PartialEq, + Eq, + PartialOrd, + Ord, + Hash, + derive_more::AsRef, derive_more::Deref, - serde::Deserialize, - serde::Serialize, + $($derive),* )] pub struct $name($type); impl $name { pub fn new(value: impl Into<$type>) -> Self { Self(value.into()) } + #[allow(clippy::missing_const_for_fn)] pub fn unwrap(self) -> $type { self.0 } } + impl From<$name> for $type { + fn from(value: $name) -> $type { + value.unwrap() + } + } }; } diff --git a/crates/config/src/repo_branches.rs b/crates/config/src/repo_branches.rs index 637a2dd..7851370 100644 --- a/crates/config/src/repo_branches.rs +++ b/crates/config/src/repo_branches.rs @@ -2,7 +2,17 @@ use crate::BranchName; /// Mapped from `.git-next.toml` file at `branches` #[derive( - Clone, Debug, PartialEq, Eq, serde::Deserialize, derive_more::Constructor, derive_more::Display, + Clone, + Hash, + Debug, + PartialEq, + Eq, + PartialOrd, + Ord, + serde::Deserialize, + serde::Serialize, + derive_more::Constructor, + derive_more::Display, )] #[display("{},{},{}", main, next, dev)] pub struct RepoBranches { diff --git a/crates/config/src/repo_config.rs b/crates/config/src/repo_config.rs index 6a649e7..d7661ca 100644 --- a/crates/config/src/repo_config.rs +++ b/crates/config/src/repo_config.rs @@ -5,7 +5,17 @@ use crate::RepoConfigSource; /// 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_more::Constructor, derive_more::Display, + Clone, + Hash, + Debug, + PartialEq, + Eq, + PartialOrd, + Ord, + serde::Deserialize, + serde::Serialize, + derive_more::Constructor, + derive_more::Display, )] #[display("{}", branches)] pub struct RepoConfig { @@ -13,7 +23,7 @@ pub struct RepoConfig { source: RepoConfigSource, } impl RepoConfig { - pub fn load(toml: &str) -> Result { + pub fn parse(toml: &str) -> Result { toml::from_str(format!("source = \"Repo\"\n{}", toml).as_str()) } diff --git a/crates/config/src/repo_config_source.rs b/crates/config/src/repo_config_source.rs index 67263d8..475539f 100644 --- a/crates/config/src/repo_config_source.rs +++ b/crates/config/src/repo_config_source.rs @@ -1,4 +1,6 @@ -#[derive(Copy, Clone, Debug, PartialEq, Eq, serde::Deserialize)] +#[derive( + Copy, Hash, Clone, Debug, PartialEq, Eq, serde::Deserialize, PartialOrd, Ord, serde::Serialize, +)] pub enum RepoConfigSource { Repo, Server, diff --git a/crates/config/src/server.rs b/crates/config/src/server.rs index 068a2c3..a523b2a 100644 --- a/crates/config/src/server.rs +++ b/crates/config/src/server.rs @@ -1,6 +1,5 @@ // use actix::prelude::*; -use derive_more::Constructor; use std::{ collections::HashMap, @@ -12,7 +11,7 @@ use std::{ use kxio::fs::FileSystem; use tracing::info; -use crate::{ForgeAlias, ForgeConfig, RepoAlias}; +use crate::{newtype, ForgeAlias, ForgeConfig, RepoAlias}; #[derive(Debug, thiserror::Error)] pub enum Error { @@ -96,18 +95,8 @@ impl Webhook { } } -/// The URL for the webhook where forges should send their updates -#[derive( - Clone, - Debug, - PartialEq, - Eq, - serde::Serialize, - serde::Deserialize, - derive_more::AsRef, - Constructor, -)] -pub struct WebhookUrl(String); +// The RL for the webhook where forges should send their updates +newtype!(WebhookUrl is a String, serde::Serialize); /// The directory to store server data, such as cloned repos #[derive(Clone, Debug, PartialEq, Eq, serde::Deserialize, derive_more::Constructor)] diff --git a/crates/config/src/tests.rs b/crates/config/src/tests.rs index 62e0ded..5035233 100644 --- a/crates/config/src/tests.rs +++ b/crates/config/src/tests.rs @@ -1,18 +1,22 @@ // +use super::*; + +use assert2::let_assert; +use secrecy::ExposeSecret; +use std::collections::BTreeMap; +use std::path::PathBuf; + +use crate::server::Http; +use crate::server::ServerConfig; +use crate::server::ServerStorage; +use crate::server::Webhook; +use crate::webhook::push::Branch; type Result = core::result::Result>; type TestResult = Result<()>; mod server_repo_config { - use std::path::PathBuf; - - use assert2::let_assert; - - use crate::{ - tests::given, BranchName, GitDir, RepoBranches, RepoConfig, RepoConfigSource, RepoPath, - }; - - use super::super::server_repo_config::*; + use super::*; #[test] fn should_not_return_repo_config_when_no_branches() { @@ -91,9 +95,6 @@ mod server_repo_config { } } mod repo_config { - use crate::{RepoBranches, RepoConfigSource}; - - use super::super::repo_config::*; use super::*; #[test] @@ -110,7 +111,7 @@ mod repo_config { "# ); - let rc = RepoConfig::load(toml.as_str())?; + let rc = RepoConfig::parse(toml.as_str())?; assert_eq!( rc, @@ -144,13 +145,7 @@ mod repo_config { } } mod forge_config { - use std::collections::BTreeMap; - - use secrecy::ExposeSecret; - - use crate::{ - tests::given, ForgeConfig, ForgeType, Hostname, RepoAlias, ServerRepoConfig, User, - }; + use super::*; #[test] fn should_return_repos() { @@ -246,14 +241,9 @@ mod forge_config { } } mod forge_details { - use std::collections::BTreeMap; - + use super::*; use secrecy::ExposeSecret; - use crate::{ - tests::given, ApiToken, ForgeAlias, ForgeConfig, ForgeDetails, ForgeType, Hostname, User, - }; - #[test] fn should_return_forge_alias() { let forge_type = ForgeType::MockForge; @@ -364,10 +354,9 @@ mod forge_details { } } mod forge_name { + use super::*; use std::path::PathBuf; - use crate::{tests::given, ForgeAlias}; - #[test] fn should_convert_to_pathbuf() { let name = given::a_name(); @@ -379,7 +368,7 @@ mod forge_name { } } mod forge_type { - use crate::ForgeType; + use super::*; #[test] fn should_display_as_string() { @@ -387,10 +376,9 @@ mod forge_type { } } mod gitdir { + use super::*; use std::path::PathBuf; - use crate::{tests::given, GitDir}; - #[test] fn should_return_pathbuf() { let pathbuf = PathBuf::default().join(given::a_name()); @@ -439,7 +427,7 @@ mod gitdir { } } mod repo_branches { - use crate::{tests::given, BranchName, RepoBranches}; + use super::*; #[test] fn should_return_main() { @@ -470,15 +458,10 @@ mod repo_branches { } } mod server { + use super::*; mod load { - use assert2::let_assert; - use pretty_assertions::assert_eq; - - use crate::{ - server::ServerConfig, - tests::{given, TestResult}, - }; + use super::*; #[test] fn load_should_parse_server_config() -> TestResult { @@ -559,7 +542,7 @@ token = "{forge_token}" {repos} "# ); - eprintln!("{file_contents}"); + println!("{file_contents}"); fs.file_write( &fs.base().join("git-next-server.toml"), file_contents.as_str(), @@ -569,8 +552,7 @@ token = "{forge_token}" } } mod registered_webhook { - - use crate::{tests::given, RegisteredWebhook, WebhookAuth}; + use super::*; #[test] fn should_return_id() { @@ -590,11 +572,11 @@ mod registered_webhook { } } mod webhook { + use super::*; mod message { + use super::*; use std::collections::HashMap; - use crate::{tests::given, WebhookMessage}; - #[test] fn should_return_forge_alias() { let forge_alias = given::a_forge_alias(); @@ -645,7 +627,8 @@ mod webhook { } } mod push { - use crate::{tests::given, webhook::push::Branch, BranchName}; + + use super::*; #[test] fn should_return_main_branch() { @@ -717,18 +700,14 @@ mod push { } } mod given { + + use super::*; + use rand::Rng as _; use std::{ collections::{BTreeMap, HashMap}, path::{Path, PathBuf}, }; - use rand::Rng as _; - - use crate::{ - server::{Http, ServerConfig, ServerStorage, Webhook}, - ForgeAlias, ForgeConfig, ForgeType, RepoAlias, RepoBranches, ServerRepoConfig, WebhookId, - }; - pub fn a_name() -> String { use rand::Rng; use std::iter; diff --git a/crates/config/src/webhook/auth.rs b/crates/config/src/webhook/auth.rs index 2e4e1ff..d61d593 100644 --- a/crates/config/src/webhook/auth.rs +++ b/crates/config/src/webhook/auth.rs @@ -1,7 +1,7 @@ -#[derive(Clone, Debug, PartialEq, Eq, derive_more::Deref, derive_more::Display)] -pub struct WebhookAuth(ulid::Ulid); +// +crate::newtype!(WebhookAuth is a ulid::Ulid, derive_more::Display); impl WebhookAuth { - pub fn new(authorisation: &str) -> Result { + pub fn try_new(authorisation: &str) -> Result { use std::str::FromStr as _; let id = ulid::Ulid::from_str(authorisation)?; tracing::info!("Parse auth token: {}", id); diff --git a/crates/config/src/webhook/id.rs b/crates/config/src/webhook/id.rs index ed7fb32..66fca94 100644 --- a/crates/config/src/webhook/id.rs +++ b/crates/config/src/webhook/id.rs @@ -1,4 +1,2 @@ -use derive_more::{Constructor, Deref, Display}; - -#[derive(Clone, Debug, PartialEq, Eq, Constructor, Deref, Display)] -pub struct WebhookId(String); +// +crate::newtype!(WebhookId is a String, derive_more::Display); diff --git a/crates/config/src/webhook/tests.rs b/crates/config/src/webhook/tests.rs index c7e1df4..0a597ee 100644 --- a/crates/config/src/webhook/tests.rs +++ b/crates/config/src/webhook/tests.rs @@ -5,7 +5,7 @@ mod auth { fn bytes() -> Result<(), Box> { let ulid = ulid::Ulid::new(); - let wa = WebhookAuth::new(ulid.to_string().as_str())?; + let wa = WebhookAuth::try_new(ulid.to_string().as_str())?; assert_eq!(ulid.to_bytes(), wa.to_bytes()); @@ -16,7 +16,7 @@ mod auth { fn string() -> Result<(), Box> { let ulid = ulid::Ulid::new(); - let wa = WebhookAuth::new(ulid.to_string().as_str())?; + let wa = WebhookAuth::try_new(ulid.to_string().as_str())?; assert_eq!(ulid.to_string(), wa.to_string()); diff --git a/crates/forge-forgejo/src/lib.rs b/crates/forge-forgejo/src/lib.rs index 9fa6a3e..29a6f73 100644 --- a/crates/forge-forgejo/src/lib.rs +++ b/crates/forge-forgejo/src/lib.rs @@ -36,7 +36,7 @@ impl git::ForgeLike for ForgeJo { tracing::info!(?authorization, %expected, "is message authorised?"); authorization .and_then(|header| header.strip_prefix("Basic ").map(|v| v.to_owned())) - .and_then(|value| config::WebhookAuth::new(value.as_str()).ok()) + .and_then(|value| config::WebhookAuth::try_new(value.as_str()).ok()) .map(|auth| &auth == expected) .unwrap_or(false) } diff --git a/crates/git/src/repository/open/tests.rs b/crates/git/src/repository/open/tests.rs index 65988b3..1afedd6 100644 --- a/crates/git/src/repository/open/tests.rs +++ b/crates/git/src/repository/open/tests.rs @@ -114,7 +114,7 @@ mod repo_config { "# ); - let rc = config::RepoConfig::load(toml.as_str())?; + let rc = config::RepoConfig::parse(toml.as_str())?; assert_eq!( rc, diff --git a/crates/repo-actor/src/load.rs b/crates/repo-actor/src/load.rs index 313e344..be389d1 100644 --- a/crates/repo-actor/src/load.rs +++ b/crates/repo-actor/src/load.rs @@ -34,7 +34,7 @@ async fn load( open_repository: &git::OpenRepository, ) -> Result { let contents = open_repository.read_file(&details.branch, &PathBuf::from(".git-next.toml"))?; - let config = config::RepoConfig::load(&contents)?; + let config = config::RepoConfig::parse(&contents)?; let config = validate(config, open_repository).await?; Ok(config) } diff --git a/crates/server/src/config/tests.rs b/crates/server/src/config/tests.rs index f0b0076..6f829c3 100644 --- a/crates/server/src/config/tests.rs +++ b/crates/server/src/config/tests.rs @@ -18,7 +18,7 @@ fn test_repo_config_load() -> Result<()> { [options] "#; - let config = RepoConfig::load(toml)?; + let config = RepoConfig::parse(toml)?; assert_eq!( config,