From f10dc25aeb0117af9db1d815624ad1a5bffb96ff Mon Sep 17 00:00:00 2001 From: Paul Campbell Date: Sun, 26 May 2024 08:56:01 +0100 Subject: [PATCH] refactor: merge git::validate module into git::validation --- crates/forge-forgejo/src/lib.rs | 3 +- crates/git/src/lib.rs | 3 -- crates/git/src/repository/tests.rs | 38 ++++++++++--------- crates/git/src/validation/mod.rs | 2 + .../positions.rs} | 24 ++++++------ .../src/{validate.rs => validation/repo.rs} | 22 ++++++----- crates/repo-actor/src/lib.rs | 5 ++- crates/server/src/config/tests.rs | 14 ++++--- 8 files changed, 60 insertions(+), 51 deletions(-) create mode 100644 crates/git/src/validation/mod.rs rename crates/git/src/{validation.rs => validation/positions.rs} (87%) rename crates/git/src/{validate.rs => validation/repo.rs} (72%) diff --git a/crates/forge-forgejo/src/lib.rs b/crates/forge-forgejo/src/lib.rs index 76c1187a..d00e5efc 100644 --- a/crates/forge-forgejo/src/lib.rs +++ b/crates/forge-forgejo/src/lib.rs @@ -4,6 +4,7 @@ mod file; #[cfg(test)] mod tests; +use git::validation::repo::validate_repo; use git_next_config as config; use git_next_git as git; @@ -98,7 +99,7 @@ impl git::ForgeLike for ForgeJo { self.repo.open(&gitdir)? }; info!("Validating..."); - git::validate(&repository, &self.repo_details) + validate_repo(&repository, &self.repo_details) .map_err(|e| git::repository::Error::Validation(e.to_string()))?; Ok(repository) } diff --git a/crates/git/src/lib.rs b/crates/git/src/lib.rs index 3e55ddc2..978256da 100644 --- a/crates/git/src/lib.rs +++ b/crates/git/src/lib.rs @@ -11,7 +11,6 @@ mod git_remote; pub mod push; mod repo_details; pub mod repository; -pub mod validate; pub mod validation; #[cfg(test)] @@ -25,5 +24,3 @@ pub use git_remote::GitRemote; pub use repo_details::RepoDetails; pub use repository::OpenRepository; pub use repository::Repository; -pub use validate::validate; -pub use validation::validate_positions; diff --git a/crates/git/src/repository/tests.rs b/crates/git/src/repository/tests.rs index 702d6b42..36e2c233 100644 --- a/crates/git/src/repository/tests.rs +++ b/crates/git/src/repository/tests.rs @@ -1,10 +1,12 @@ mod validate { + use assert2::let_assert; use git_next_config::{ForgeDetails, GitDir, Hostname, RepoPath}; use crate::{ - repository::{self, Direction}, - validate, GitRemote, RepoDetails, + repository, + validation::{self, repo::validate_repo}, + GitRemote, RepoDetails, }; #[test] @@ -27,8 +29,9 @@ mod validate { |open_repo| { let_assert!( Ok(_) = open_repo.lock().map(|mut open_repo| { - open_repo.has_default_remote(Direction::Push, remote.clone()); - open_repo.has_default_remote(Direction::Fetch, remote); + open_repo + .has_default_remote(repository::Direction::Push, remote.clone()); + open_repo.has_default_remote(repository::Direction::Fetch, remote); }) ); } @@ -37,7 +40,7 @@ mod validate { let_assert!(Ok(open_repository) = repository.open(&gitdir)); - let_assert!(Ok(_) = validate(&open_repository, &repo_details)); + let_assert!(Ok(_) = validation::repo::validate_repo(&open_repository, &repo_details)); } #[test] @@ -61,7 +64,7 @@ mod validate { let_assert!( Ok(_) = open_repo.lock().map(|mut open_repo| { // INFO: open_repo.has_default_remote(Direction::Push, remote.clone()); - open_repo.has_default_remote(Direction::Fetch, remote); + open_repo.has_default_remote(repository::Direction::Fetch, remote); }) ); } @@ -70,7 +73,7 @@ mod validate { let_assert!(Ok(open_repository) = repository.open(&gitdir)); - let_assert!(Err(_) = validate(&open_repository, &repo_details)); + let_assert!(Err(_) = validate_repo(&open_repository, &repo_details)); } #[test] fn should_fail_where_no_default_fetch_remote() { @@ -92,7 +95,7 @@ mod validate { |open_repo| { let_assert!( Ok(_) = open_repo.lock().map(|mut open_repo| { - open_repo.has_default_remote(Direction::Push, remote); + open_repo.has_default_remote(repository::Direction::Push, remote); // INFO: open_repo.has_default_remote(Direction::Fetch, remote); }) ); @@ -102,7 +105,7 @@ mod validate { let_assert!(Ok(open_repository) = repository.open(&gitdir)); - let_assert!(Err(_) = validate(&open_repository, &repo_details)); + let_assert!(Err(_) = validate_repo(&open_repository, &repo_details)); } #[test] fn should_fail_where_invalid_default_push_remote() { @@ -128,8 +131,8 @@ mod validate { |open_repo| { let_assert!( Ok(_) = open_repo.lock().map(|mut open_repo| { - open_repo.has_default_remote(Direction::Push, other_remote); - open_repo.has_default_remote(Direction::Fetch, remote); + open_repo.has_default_remote(repository::Direction::Push, other_remote); + open_repo.has_default_remote(repository::Direction::Fetch, remote); }) ); } @@ -138,7 +141,7 @@ mod validate { let_assert!(Ok(open_repository) = repository.open(&gitdir)); - let_assert!(Err(_) = validate(&open_repository, &repo_details)); + let_assert!(Err(_) = validate_repo(&open_repository, &repo_details)); } #[test] fn should_fail_where_invalid_default_fetch_remote() { @@ -164,8 +167,9 @@ mod validate { |open_repo| { let_assert!( Ok(_) = open_repo.lock().map(|mut open_repo| { - open_repo.has_default_remote(Direction::Push, remote); - open_repo.has_default_remote(Direction::Fetch, other_remote); + open_repo.has_default_remote(repository::Direction::Push, remote); + open_repo + .has_default_remote(repository::Direction::Fetch, other_remote); }) ); } @@ -174,7 +178,7 @@ mod validate { let_assert!(Ok(open_repository) = repository.open(&gitdir)); - let_assert!(Err(_) = validate(&open_repository, &repo_details)); + let_assert!(Err(_) = validate_repo(&open_repository, &repo_details)); } } @@ -182,7 +186,7 @@ mod git_clone { use assert2::let_assert; use git_next_config::{ForgeDetails, GitDir, Hostname, RepoPath}; - use crate::{repository::Direction, GitRemote, RepoDetails}; + use crate::{repository, GitRemote, RepoDetails}; #[test] fn should_clone_repo() { @@ -195,7 +199,7 @@ mod git_clone { .with_gitdir(GitDir::new(fs.base())) .with_repo_path(RepoPath::new("kemitix/git-next".to_string())); let_assert!(Ok(open_repo) = r.git_clone(&repo_details)); - let_assert!(Some(remote) = open_repo.find_default_remote(Direction::Fetch)); + let_assert!(Some(remote) = open_repo.find_default_remote(repository::Direction::Fetch)); assert_eq!( remote, GitRemote::new( diff --git a/crates/git/src/validation/mod.rs b/crates/git/src/validation/mod.rs new file mode 100644 index 00000000..6180e68c --- /dev/null +++ b/crates/git/src/validation/mod.rs @@ -0,0 +1,2 @@ +pub mod positions; +pub mod repo; diff --git a/crates/git/src/validation.rs b/crates/git/src/validation/positions.rs similarity index 87% rename from crates/git/src/validation.rs rename to crates/git/src/validation/positions.rs index c131c80e..5c4941b1 100644 --- a/crates/git/src/validation.rs +++ b/crates/git/src/validation/positions.rs @@ -26,7 +26,7 @@ pub fn validate_positions( Ok(commit_histories) => commit_histories, Err(err) => { error!(?err, "Failed to get commit histories"); - return Err(git::validation::Error::CommitLog(err)); + return Err(git::validation::positions::Error::CommitLog(err)); } }; // Validations @@ -35,7 +35,7 @@ pub fn validate_positions( "No commits on main branch '{}'", repo_config.branches().main() ); - return Err(git::validation::Error::BranchHasNoCommits( + return Err(git::validation::positions::Error::BranchHasNoCommits( repo_config.branches().main(), )); }; @@ -48,7 +48,7 @@ pub fn validate_positions( repo_config.branches().main(), repo_config.branches().main(), ); - return Err(git::validation::Error::DevBranchNotBasedOn( + return Err(git::validation::positions::Error::DevBranchNotBasedOn( repo_config.branches().main(), )); } @@ -58,7 +58,7 @@ pub fn validate_positions( "No commits on next branch '{}", repo_config.branches().next() ); - return Err(git::validation::Error::BranchHasNoCommits( + return Err(git::validation::positions::Error::BranchHasNoCommits( repo_config.branches().next(), )); }; @@ -73,12 +73,12 @@ pub fn validate_positions( git::push::Force::From(next.clone().into()), ) { warn!(?err, "Failed to reset next to main"); - return Err(git::validation::Error::FailedToResetBranch { + return Err(git::validation::positions::Error::FailedToResetBranch { branch: repo_config.branches().next(), commit: next, }); } - return Err(git::validation::Error::BranchReset( + return Err(git::validation::positions::Error::BranchReset( repo_config.branches().next(), )); } @@ -103,12 +103,12 @@ pub fn validate_positions( git::push::Force::From(next.clone().into()), ) { warn!(?err, "Failed to reset next to main"); - return Err(git::validation::Error::FailedToResetBranch { + return Err(git::validation::positions::Error::FailedToResetBranch { branch: repo_config.branches().next(), commit: next, }); } - return Err(git::validation::Error::BranchReset( + return Err(git::validation::positions::Error::BranchReset( repo_config.branches().next(), )); } @@ -117,7 +117,7 @@ pub fn validate_positions( "No commits on next branch '{}'", repo_config.branches().next() ); - return Err(git::validation::Error::BranchHasNoCommits( + return Err(git::validation::positions::Error::BranchHasNoCommits( repo_config.branches().next(), )); }; @@ -131,7 +131,7 @@ pub fn validate_positions( repo_config.branches().dev(), repo_config.branches().next() ); - return Err(git::validation::Error::DevBranchNotBasedOn( + return Err(git::validation::positions::Error::DevBranchNotBasedOn( repo_config.branches().next(), )); // dev is not based on next } @@ -140,11 +140,11 @@ pub fn validate_positions( "No commits on dev branch '{}'", repo_config.branches().dev() ); - return Err(git::validation::Error::BranchHasNoCommits( + return Err(git::validation::positions::Error::BranchHasNoCommits( repo_config.branches().dev(), )); }; - Ok(git::validation::Positions { + Ok(git::validation::positions::Positions { main, next, dev, diff --git a/crates/git/src/validate.rs b/crates/git/src/validation/repo.rs similarity index 72% rename from crates/git/src/validate.rs rename to crates/git/src/validation/repo.rs index 6505ee59..b70db450 100644 --- a/crates/git/src/validate.rs +++ b/crates/git/src/validation/repo.rs @@ -1,15 +1,17 @@ use tracing::info; -use crate::repository::{Direction, OpenRepository}; - -use super::{GitRemote, RepoDetails}; +use crate as git; #[tracing::instrument(skip_all)] -pub fn validate(repository: &OpenRepository, repo_details: &RepoDetails) -> Result<()> { - let Some(push_remote) = repository.find_default_remote(Direction::Push) else { +pub fn validate_repo( + repository: &git::OpenRepository, + repo_details: &git::RepoDetails, +) -> Result<()> { + let Some(push_remote) = repository.find_default_remote(git::repository::Direction::Push) else { return Err(Error::NoDefaultPushRemote); }; - let Some(fetch_remote) = repository.find_default_remote(Direction::Fetch) else { + let Some(fetch_remote) = repository.find_default_remote(git::repository::Direction::Fetch) + else { return Err(Error::NoDefaultFetchRemote); }; let git_remote = repo_details.git_remote(); @@ -40,13 +42,13 @@ pub enum Error { Io(std::io::Error), #[display("MismatchDefaultPushRemote(found: {found}, expected: {expected})")] MismatchDefaultPushRemote { - found: GitRemote, - expected: GitRemote, + found: git::GitRemote, + expected: git::GitRemote, }, #[display("MismatchDefaultFetchRemote(found: {found}, expected: {expected})")] MismatchDefaultFetchRemote { - found: GitRemote, - expected: GitRemote, + found: git::GitRemote, + expected: git::GitRemote, }, } impl std::error::Error for Error {} diff --git a/crates/repo-actor/src/lib.rs b/crates/repo-actor/src/lib.rs index 17fe0aa4..679cc007 100644 --- a/crates/repo-actor/src/lib.rs +++ b/crates/repo-actor/src/lib.rs @@ -9,6 +9,7 @@ mod tests; use std::time::Duration; use actix::prelude::*; +use git::validation::positions::{validate_positions, Positions}; use crate as repo_actor; use git_next_config as config; @@ -185,8 +186,8 @@ impl Handler for RepoActor { let addr = ctx.address(); let message_token = self.message_token; async move { - match git::validate_positions(&repository, &repo_details, repo_config) { - Ok(git::validation::Positions { + match validate_positions(&repository, &repo_details, repo_config) { + Ok(Positions { main, next, dev, diff --git a/crates/server/src/config/tests.rs b/crates/server/src/config/tests.rs index e2e6d0d4..f0b00769 100644 --- a/crates/server/src/config/tests.rs +++ b/crates/server/src/config/tests.rs @@ -1,6 +1,6 @@ // use assert2::let_assert; -use git::repository::Direction; +use git::{repository::Direction, validation::repo::validate_repo}; use git_next_config::{ self as config, ForgeType, GitDir, Hostname, RepoBranches, RepoConfig, RepoConfigSource, RepoPath, @@ -46,7 +46,7 @@ fn gitdir_should_display_as_pathbuf() { // git.kemitix.net:kemitix/git-next // If the default push remote is something else, then this test will fail fn repo_details_find_default_push_remote_finds_correct_remote() -> Result<()> { - let cli_crate_dir = std::env::current_dir().map_err(git::validate::Error::Io)?; + let cli_crate_dir = std::env::current_dir().map_err(git::validation::repo::Error::Io)?; let_assert!(Some(Some(root)) = cli_crate_dir.parent().map(|p| p.parent())); let mut repo_details = git::common::repo_details( 1, @@ -77,7 +77,7 @@ fn repo_details_find_default_push_remote_finds_correct_remote() -> Result<()> { #[test] fn gitdir_validate_should_pass_a_valid_git_repo() -> Result<()> { - let cli_crate_dir = std::env::current_dir().map_err(git::validate::Error::Io)?; + let cli_crate_dir = std::env::current_dir().map_err(git::validation::repo::Error::Io)?; let_assert!(Some(Some(root)) = cli_crate_dir.parent().map(|p| p.parent())); let mut repo_details = git::common::repo_details( 1, @@ -92,14 +92,16 @@ fn gitdir_validate_should_pass_a_valid_git_repo() -> Result<()> { repo_details.repo_path = RepoPath::new("kemitix/git-next".to_string()); let gitdir = &repo_details.gitdir; let repository = git::repository::new().open(gitdir)?; - git::validate(&repository, &repo_details)?; + validate_repo(&repository, &repo_details)?; Ok(()) } #[test] fn gitdir_validate_should_fail_a_git_repo_with_wrong_remote() -> Result<()> { - let_assert!(Ok(cli_crate_dir) = std::env::current_dir().map_err(git::validate::Error::Io)); + let_assert!( + Ok(cli_crate_dir) = std::env::current_dir().map_err(git::validation::repo::Error::Io) + ); let_assert!(Some(Some(root)) = cli_crate_dir.parent().map(|p| p.parent())); let mut repo_details = git::common::repo_details( 1, @@ -111,7 +113,7 @@ fn gitdir_validate_should_fail_a_git_repo_with_wrong_remote() -> Result<()> { repo_details.repo_path = RepoPath::new("hello/world".to_string()); let gitdir = &repo_details.gitdir; let repository = git::repository::new().open(gitdir)?; - let_assert!(Err(_) = git::validate(&repository, &repo_details)); + let_assert!(Err(_) = validate_repo(&repository, &repo_details)); Ok(()) }