From 341dc97a51792b89fea6f60e9d3dbcf255e1d2cc Mon Sep 17 00:00:00 2001 From: Paul Campbell Date: Sun, 19 May 2024 20:02:06 +0100 Subject: [PATCH] refactor(git): add mock repository and tests Repository is now behind an enum to allow selection of a mock Repsitory for use in tests. --- Cargo.toml | 1 + crates/config/Cargo.toml | 5 +- crates/config/src/api_token.rs | 5 + crates/config/src/branch_name.rs | 2 +- crates/config/src/forge_details.rs | 7 +- crates/config/src/forge_name.rs | 2 +- crates/config/src/forge_type.rs | 3 +- crates/config/src/git_dir.rs | 10 +- crates/config/src/host_name.rs | 2 +- crates/config/src/repo_alias.rs | 2 +- crates/config/src/repo_path.rs | 2 +- crates/config/src/user.rs | 2 +- crates/git/Cargo.toml | 13 +- crates/git/src/fetch.rs | 11 ++ crates/git/src/repo_details.rs | 2 +- crates/git/src/repository/mock.rs | 159 ++++++++++++++- crates/git/src/repository/mod.rs | 56 +++--- crates/git/src/repository/open/mod.rs | 15 +- crates/git/src/repository/open/oreal.rs | 43 ++-- crates/git/src/repository/open/tests.rs | 38 ++++ crates/git/src/repository/real.rs | 4 +- crates/git/src/repository/tests.rs | 207 ++++++++++++++++++++ crates/git/src/validate.rs | 2 +- crates/server/src/actors/server.rs | 11 +- crates/server/src/gitforge/tests/forgejo.rs | 4 +- 25 files changed, 526 insertions(+), 82 deletions(-) create mode 100644 crates/git/src/repository/open/tests.rs create mode 100644 crates/git/src/repository/tests.rs diff --git a/Cargo.toml b/Cargo.toml index ee1bcbce..7f991a6f 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -62,6 +62,7 @@ derive_more = { version = "1.0.0-beta.6", features = [ "deref", "from", ] } +derive-with = "0.5" # file watcher inotify = "0.10" diff --git a/crates/config/Cargo.toml b/crates/config/Cargo.toml index 17b41f21..53f74635 100644 --- a/crates/config/Cargo.toml +++ b/crates/config/Cargo.toml @@ -40,9 +40,10 @@ secrecy = { workspace = true } # bytes = { workspace = true } # ulid = { workspace = true } # warp = { workspace = true } -# -# # error handling + +# boilerplate derive_more = { workspace = true } +derive-with = { workspace = true } # # # file watcher # inotify = { workspace = true } diff --git a/crates/config/src/api_token.rs b/crates/config/src/api_token.rs index e07d1afc..42dc0497 100644 --- a/crates/config/src/api_token.rs +++ b/crates/config/src/api_token.rs @@ -9,3 +9,8 @@ impl secrecy::ExposeSecret for ApiToken { self.0.expose_secret() } } +impl Default for ApiToken { + fn default() -> Self { + Self("".to_string().into()) + } +} diff --git a/crates/config/src/branch_name.rs b/crates/config/src/branch_name.rs index b2f360fd..70df7c62 100644 --- a/crates/config/src/branch_name.rs +++ b/crates/config/src/branch_name.rs @@ -1,5 +1,5 @@ /// The name of a Branch -#[derive(Clone, Debug, Hash, PartialEq, Eq, derive_more::Display)] +#[derive(Clone, Default, Debug, Hash, PartialEq, Eq, derive_more::Display)] pub struct BranchName(String); impl BranchName { pub fn new(str: impl Into) -> Self { diff --git a/crates/config/src/forge_details.rs b/crates/config/src/forge_details.rs index 6754ca67..22d0e5b7 100644 --- a/crates/config/src/forge_details.rs +++ b/crates/config/src/forge_details.rs @@ -1,7 +1,7 @@ use crate::{ApiToken, ForgeConfig, ForgeName, ForgeType, Hostname, User}; /// The derived information about a Forge, used to create interactions with it -#[derive(Clone, Debug, derive_more::Constructor)] +#[derive(Clone, Default, Debug, derive_more::Constructor, derive_with::With)] pub struct ForgeDetails { forge_name: ForgeName, forge_type: ForgeType, @@ -27,11 +27,6 @@ impl ForgeDetails { pub const fn token(&self) -> &ApiToken { &self.token } - pub fn with_hostname(self, hostname: Hostname) -> Self { - let mut me = self; - me.hostname = hostname; - me - } } impl From<(&ForgeName, &ForgeConfig)> for ForgeDetails { fn from(forge: (&ForgeName, &ForgeConfig)) -> Self { diff --git a/crates/config/src/forge_name.rs b/crates/config/src/forge_name.rs index bea4284d..b74406c6 100644 --- a/crates/config/src/forge_name.rs +++ b/crates/config/src/forge_name.rs @@ -1,7 +1,7 @@ use std::path::PathBuf; /// The name of a Forge to connect to -#[derive(Clone, Debug, PartialEq, Eq, derive_more::Constructor, derive_more::Display)] +#[derive(Clone, Default, Debug, PartialEq, Eq, derive_more::Constructor, derive_more::Display)] pub struct ForgeName(String); impl From<&ForgeName> for PathBuf { fn from(value: &ForgeName) -> Self { diff --git a/crates/config/src/forge_type.rs b/crates/config/src/forge_type.rs index 49225955..a92c477c 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, Copy, Debug, PartialEq, Eq, serde::Deserialize)] +#[derive(Clone, Default, Copy, Debug, PartialEq, Eq, serde::Deserialize)] pub enum ForgeType { #[cfg(feature = "forgejo")] ForgeJo, @@ -7,6 +7,7 @@ pub enum ForgeType { // GitHub, // GitLab, // BitBucket, + #[default] MockForge, } impl std::fmt::Display for ForgeType { diff --git a/crates/config/src/git_dir.rs b/crates/config/src/git_dir.rs index fb5b23a6..f27596dc 100644 --- a/crates/config/src/git_dir.rs +++ b/crates/config/src/git_dir.rs @@ -1,7 +1,15 @@ use std::path::PathBuf; #[derive( - Debug, Clone, PartialEq, Eq, serde::Deserialize, derive_more::Deref, derive_more::From, + Clone, + Default, + Debug, + Hash, + PartialEq, + Eq, + serde::Deserialize, + derive_more::Deref, + derive_more::From, )] pub struct GitDir(PathBuf); impl GitDir { diff --git a/crates/config/src/host_name.rs b/crates/config/src/host_name.rs index a1b7b15e..cb2967a8 100644 --- a/crates/config/src/host_name.rs +++ b/crates/config/src/host_name.rs @@ -1,5 +1,5 @@ /// The hostname of a forge -#[derive(Clone, Debug, PartialEq, Eq, derive_more::Display)] +#[derive(Clone, Default, Debug, PartialEq, Eq, derive_more::Display)] pub struct Hostname(String); impl Hostname { pub fn new(str: impl Into) -> Self { diff --git a/crates/config/src/repo_alias.rs b/crates/config/src/repo_alias.rs index 38091fdc..18e83dad 100644 --- a/crates/config/src/repo_alias.rs +++ b/crates/config/src/repo_alias.rs @@ -1,6 +1,6 @@ /// The alias of a repo /// This is the alias for the repo within `git-next-server.toml` -#[derive(Clone, Debug, PartialEq, Eq, Hash, derive_more::Display)] +#[derive(Clone, Default, Debug, PartialEq, Eq, Hash, derive_more::Display)] pub struct RepoAlias(String); impl RepoAlias { pub fn new(str: impl Into) -> Self { diff --git a/crates/config/src/repo_path.rs b/crates/config/src/repo_path.rs index 832f9be3..f3079af4 100644 --- a/crates/config/src/repo_path.rs +++ b/crates/config/src/repo_path.rs @@ -1,5 +1,5 @@ /// 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_more::Constructor, derive_more::Display)] +#[derive(Clone, Default, Debug, PartialEq, Eq, derive_more::Constructor, derive_more::Display)] pub struct RepoPath(String); diff --git a/crates/config/src/user.rs b/crates/config/src/user.rs index 48eea03d..d3b94378 100644 --- a/crates/config/src/user.rs +++ b/crates/config/src/user.rs @@ -1,3 +1,3 @@ /// The user within the forge to connect as -#[derive(Clone, Debug, PartialEq, Eq, derive_more::Constructor, derive_more::Display)] +#[derive(Clone, Default, Debug, PartialEq, Eq, derive_more::Constructor, derive_more::Display)] pub struct User(String); diff --git a/crates/git/Cargo.toml b/crates/git/Cargo.toml index 65cbb657..21d61283 100644 --- a/crates/git/Cargo.toml +++ b/crates/git/Cargo.toml @@ -23,9 +23,9 @@ tracing = { workspace = true } # # gix = { workspace = true } gix = { workspace = true } # async-trait = { workspace = true } -# -# # fs/network -# kxio = { workspace = true } + +# fs/network +kxio = { workspace = true } # # TOML parsing # serde = { workspace = true } @@ -45,6 +45,7 @@ secrecy = { workspace = true } # error handling derive_more = { workspace = true } +derive-with = { workspace = true } # # file watcher # inotify = { workspace = true } @@ -54,9 +55,9 @@ derive_more = { workspace = true } # actix-rt = { workspace = true } # tokio = { workspace = true } # -# [dev-dependencies] -# # Testing -# assert2 = { workspace = true } +[dev-dependencies] +# Testing +assert2 = { workspace = true } [lints.clippy] nursery = { level = "warn", priority = -1 } diff --git a/crates/git/src/fetch.rs b/crates/git/src/fetch.rs index ec969321..29de9e09 100644 --- a/crates/git/src/fetch.rs +++ b/crates/git/src/fetch.rs @@ -7,3 +7,14 @@ pub enum Error { Lock, } impl std::error::Error for Error {} + +mod gix_error { + #![cfg(not(tarpaulin_include))] // third-party library errors + use super::Error; + + impl From for Error { + fn from(gix_error: gix::remote::connect::Error) -> Self { + Self::Connect(gix_error.to_string()) + } + } +} diff --git a/crates/git/src/repo_details.rs b/crates/git/src/repo_details.rs index 93e83cc2..f771fb9d 100644 --- a/crates/git/src/repo_details.rs +++ b/crates/git/src/repo_details.rs @@ -6,7 +6,7 @@ use git_next_config::{ use super::{Generation, GitRemote}; /// The derived information about a repo, used to interact with it -#[derive(Clone, Debug, derive_more::Display)] +#[derive(Clone, Default, Debug, derive_more::Display, derive_with::With)] #[display("gen-{}:{}:{}/{}:{}@{}/{}@{}", generation, forge.forge_type(), forge.forge_name(), repo_alias, forge.user(), forge.hostname(), repo_path, branch)] diff --git a/crates/git/src/repository/mock.rs b/crates/git/src/repository/mock.rs index c074826a..6975b3e6 100644 --- a/crates/git/src/repository/mock.rs +++ b/crates/git/src/repository/mock.rs @@ -1,17 +1,158 @@ +// +#![cfg(not(tarpaulin_include))] // don't test mocks + +use std::{ + collections::HashMap, + sync::{Arc, Mutex}, +}; + use git_next_config::GitDir; use crate::{ - repository::{open::OpenRepository, Error, RepositoryLike}, - RepoDetails, + repository::{ + open::{OpenRepository, OpenRepositoryLike}, + Direction, RepositoryLike, Result, + }, + GitRemote, RepoDetails, }; -pub struct MockRepository; -impl RepositoryLike for MockRepository { - fn open(&self, _gitdir: &GitDir) -> Result { - Ok(OpenRepository::Mock) - } +use super::Error; - fn git_clone(&self, _repo_details: &RepoDetails) -> Result { - Ok(OpenRepository::Mock) +#[derive(Debug, Default, Clone)] +pub struct MockRepository(Arc>); +impl MockRepository { + pub fn new() -> Self { + Self(Arc::new(Mutex::new(Reality::default()))) + } + pub fn can_open_repo(&mut self, gitdir: &GitDir) -> Result { + self.0 + .lock() + .map_err(|_| Error::MockLock) + .map(|mut r| r.can_open_repo(gitdir)) + } + fn open_repository( + &self, + gitdir: &GitDir, + ) -> std::result::Result { + self.0.lock().map_err(|_| Error::MockLock).and_then(|r| { + r.open_repository(gitdir) + .ok_or_else(|| Error::Open(format!("mock - could not open: {}", gitdir))) + }) + } + fn clone_repository( + &self, + ) -> std::result::Result { + todo!() + } +} +impl RepositoryLike for MockRepository { + fn open( + &self, + gitdir: &GitDir, + ) -> std::result::Result { + Ok(OpenRepository::Mock(self.open_repository(gitdir)?)) + } + + fn git_clone( + &self, + _repo_details: &RepoDetails, + ) -> std::result::Result { + Ok(OpenRepository::Mock(self.clone_repository()?)) + } +} + +#[derive(Debug, Default)] +pub struct Reality { + openable_repos: HashMap, +} +impl Reality { + pub fn can_open_repo(&mut self, gitdir: &GitDir) -> MockOpenRepository { + let mor = self.openable_repos.get(gitdir); + match mor { + Some(mor) => mor.clone(), + None => { + let mor = MockOpenRepository::default(); + self.openable_repos.insert(gitdir.clone(), mor.clone()); + mor + } + } + } + pub fn open_repository(&self, gitdir: &GitDir) -> Option { + self.openable_repos.get(gitdir).cloned() + } +} +#[derive(Clone, Debug, Default)] +pub struct InnerMockOpenRepository { + default_push_remote: Option, + default_fetch_remote: Option, +} + +#[derive(Clone, Debug, Default)] +pub struct MockOpenRepository { + inner: Arc>, +} +impl std::ops::Deref for MockOpenRepository { + type Target = Arc>; + + fn deref(&self) -> &Self::Target { + &self.inner + } +} + +impl InnerMockOpenRepository { + pub fn has_default_remote(&mut self, direction: Direction, git_remote: GitRemote) { + match direction { + Direction::Push => self.default_push_remote.replace(git_remote), + Direction::Fetch => self.default_fetch_remote.replace(git_remote), + }; + } +} + +#[allow(clippy::unwrap_used)] +impl OpenRepositoryLike for MockOpenRepository { + fn find_default_remote(&self, direction: Direction) -> Option { + self.inner + .lock() + .map(|inner| inner.find_default_remote(direction)) + .unwrap() + } + + fn fetch(&self) -> std::prelude::v1::Result<(), crate::fetch::Error> { + self.inner.lock().map(|inner| inner.fetch()).unwrap() + } + + fn push( + &self, + repo_details: &RepoDetails, + branch_name: git_next_config::BranchName, + to_commit: crate::GitRef, + force: crate::push::Force, + ) -> std::prelude::v1::Result<(), crate::push::Error> { + self.inner + .lock() + .map(|inner| inner.push(repo_details, branch_name, to_commit, force)) + .unwrap() + } +} +impl OpenRepositoryLike for InnerMockOpenRepository { + fn find_default_remote(&self, direction: Direction) -> Option { + match direction { + Direction::Push => self.default_push_remote.clone(), + Direction::Fetch => self.default_fetch_remote.clone(), + } + } + + fn fetch(&self) -> std::prelude::v1::Result<(), crate::fetch::Error> { + todo!() + } + + fn push( + &self, + _repo_details: &RepoDetails, + _branch_name: git_next_config::BranchName, + _to_commit: crate::GitRef, + _force: crate::push::Force, + ) -> std::prelude::v1::Result<(), crate::push::Error> { + todo!() } } diff --git a/crates/git/src/repository/mod.rs b/crates/git/src/repository/mod.rs index 5e858ebb..c58677ca 100644 --- a/crates/git/src/repository/mod.rs +++ b/crates/git/src/repository/mod.rs @@ -1,31 +1,35 @@ // -#![cfg(not(tarpaulin_include))] - mod mock; mod open; mod real; +#[cfg(test)] +mod tests; + use git_next_config::GitDir; pub use open::OpenRepository; +use crate::repository::mock::MockRepository; + use super::RepoDetails; -#[derive(Clone, Copy, Debug)] +#[derive(Clone, Debug)] pub enum Repository { Real, - Mock, + Mock(MockRepository), } pub const fn new() -> Repository { Repository::Real } -pub const fn mock() -> Repository { - Repository::Mock +pub fn mock() -> (Repository, MockRepository) { + let mock_repository = MockRepository::new(); + (Repository::Mock(mock_repository.clone()), mock_repository) } pub trait RepositoryLike { - fn open(&self, gitdir: &GitDir) -> Result; - fn git_clone(&self, repo_details: &RepoDetails) -> Result; + fn open(&self, gitdir: &GitDir) -> Result; + fn git_clone(&self, repo_details: &RepoDetails) -> Result; } impl std::ops::Deref for Repository { type Target = dyn RepositoryLike; @@ -33,7 +37,7 @@ impl std::ops::Deref for Repository { fn deref(&self) -> &Self::Target { match self { Self::Real => &real::RealRepository, - Self::Mock => &mock::MockRepository, + Self::Mock(mock_repository) => mock_repository, } } } @@ -54,6 +58,8 @@ impl From for gix::remote::Direction { } } +pub type Result = core::result::Result; + #[derive(Debug, derive_more::Display)] pub enum Error { InvalidGitDir(git_next_config::GitDir), @@ -64,20 +70,26 @@ pub enum Error { Clone(String), Open(String), Fetch(String), + MockLock, } impl std::error::Error for Error {} -impl From for Error { - fn from(value: gix::clone::Error) -> Self { - Self::Clone(value.to_string()) - } -} -impl From for Error { - fn from(value: gix::open::Error) -> Self { - Self::Open(value.to_string()) - } -} -impl From for Error { - fn from(value: gix::clone::fetch::Error) -> Self { - Self::Fetch(value.to_string()) + +mod gix_errors { + #![cfg(not(tarpaulin_include))] // third-party library errors + use super::Error; + impl From for Error { + fn from(value: gix::clone::Error) -> Self { + Self::Clone(value.to_string()) + } + } + impl From for Error { + fn from(value: gix::open::Error) -> Self { + Self::Open(value.to_string()) + } + } + impl From for Error { + fn from(value: gix::clone::fetch::Error) -> Self { + Self::Fetch(value.to_string()) + } } } diff --git a/crates/git/src/repository/open/mod.rs b/crates/git/src/repository/open/mod.rs index 2922f9b1..2a86c43b 100644 --- a/crates/git/src/repository/open/mod.rs +++ b/crates/git/src/repository/open/mod.rs @@ -1,4 +1,7 @@ // +#[cfg(test)] +mod tests; + pub mod oreal; use std::sync::{Arc, Mutex}; @@ -7,19 +10,23 @@ use git_next_config::BranchName; use crate::{ fetch, push, - repository::{open::oreal::RealOpenRepository, Direction}, + repository::{mock::MockOpenRepository, open::oreal::RealOpenRepository, Direction}, GitRef, GitRemote, RepoDetails, }; #[derive(Clone, Debug)] pub enum OpenRepository { Real(oreal::RealOpenRepository), - Mock, // TODO: (#38) contain a mock model of a repo + Mock(MockOpenRepository), // TODO: (#38) contain a mock model of a repo } impl OpenRepository { - pub fn new(gix_repo: gix::Repository) -> Self { + pub fn real(gix_repo: gix::Repository) -> Self { Self::Real(RealOpenRepository::new(Arc::new(Mutex::new(gix_repo)))) } + #[cfg(not(tarpaulin_include))] // don't test mocks + pub const fn mock(mock: MockOpenRepository) -> Self { + Self::Mock(mock) + } } pub trait OpenRepositoryLike { fn find_default_remote(&self, direction: Direction) -> Option; @@ -38,7 +45,7 @@ impl std::ops::Deref for OpenRepository { fn deref(&self) -> &Self::Target { match self { Self::Real(real) => real, - Self::Mock => todo!(), + Self::Mock(mock) => mock, } } } diff --git a/crates/git/src/repository/open/oreal.rs b/crates/git/src/repository/open/oreal.rs index f103557b..b03f4b87 100644 --- a/crates/git/src/repository/open/oreal.rs +++ b/crates/git/src/repository/open/oreal.rs @@ -11,41 +11,42 @@ pub struct RealOpenRepository(Arc>); impl super::OpenRepositoryLike for RealOpenRepository { fn find_default_remote(&self, direction: Direction) -> Option { let Ok(repository) = self.0.lock() else { + #[cfg(not(tarpaulin_include))] // don't test mutex lock failure return None; }; let Some(Ok(remote)) = repository.find_default_remote(direction.into()) else { + #[cfg(not(tarpaulin_include))] // test is on local repo - should always have remotes return None; }; - let url = remote.url(direction.into())?; - let host = url.host()?; - let path = url.path.to_string(); - let path = path.strip_prefix('/').map_or(path.as_str(), |path| path); - let path = path.strip_suffix(".git").map_or(path, |path| path); - Some(GitRemote::new( - Hostname::new(host), - RepoPath::new(path.to_string()), - )) + remote.url(direction.into()).map(Into::into) } fn fetch(&self) -> Result<(), fetch::Error> { let Ok(repository) = self.0.lock() else { + #[cfg(not(tarpaulin_include))] // don't test mutex lock failure return Err(fetch::Error::Lock); }; let Some(Ok(remote)) = repository.find_default_remote(Direction::Fetch.into()) else { + #[cfg(not(tarpaulin_include))] // test is on local repo - should always have remotes return Err(fetch::Error::NoFetchRemoteFound); }; - remote - .connect(gix::remote::Direction::Fetch) - .map_err(|e| fetch::Error::Connect(e.to_string()))? - .prepare_fetch(gix::progress::Discard, Default::default()) - .map_err(|e| fetch::Error::Fetch(e.to_string()))? - .receive(gix::progress::Discard, &Default::default()) - .map_err(|e| fetch::Error::Fetch(e.to_string()))?; + let prepared_fetch = remote + .connect(gix::remote::Direction::Fetch)? + .prepare_fetch(gix::progress::Discard, Default::default()); + match prepared_fetch { + Ok(fetch) => { + fetch + .receive(gix::progress::Discard, &Default::default()) + .map_err(|e| fetch::Error::Fetch(e.to_string()))?; + } + Err(e) => Err(fetch::Error::Fetch(e.to_string()))?, + } Ok(()) } // TODO: (#72) reimplement using `gix` + #[cfg(not(tarpaulin_include))] // would require writing to external service fn push( &self, repo_details: &RepoDetails, @@ -100,3 +101,13 @@ impl super::OpenRepositoryLike for RealOpenRepository { } } } + +impl From<&gix::Url> for GitRemote { + fn from(url: &gix::Url) -> Self { + let host = url.host().unwrap_or_default(); + let path = url.path.to_string(); + let path = path.strip_prefix('/').map_or(path.as_str(), |path| path); + let path = path.strip_suffix(".git").map_or(path, |path| path); + Self::new(Hostname::new(host), RepoPath::new(path.to_string())) + } +} diff --git a/crates/git/src/repository/open/tests.rs b/crates/git/src/repository/open/tests.rs new file mode 100644 index 00000000..b7333fa8 --- /dev/null +++ b/crates/git/src/repository/open/tests.rs @@ -0,0 +1,38 @@ +// + +mod find_default_remote { + use assert2::let_assert; + use git_next_config::{GitDir, Hostname, RepoPath}; + + use crate::GitRemote; + #[test] + fn should_find_default_push_remote() { + // uses the current repo + let_assert!(Ok(cwd) = std::env::current_dir()); + let gitdir = GitDir::from(cwd.join("../..")); // from ./crate/git directory to the project rook + let_assert!(Ok(repo) = crate::repository::new().open(&gitdir)); + let_assert!(Some(remote) = repo.find_default_remote(crate::repository::Direction::Push)); + assert_eq!( + remote, + GitRemote::new( + Hostname::new("git.kemitix.net"), + RepoPath::new("kemitix/git-next".to_string()) + ) + ) + } +} + +mod fetch { + use assert2::let_assert; + use git_next_config::GitDir; + + #[test] + #[ignore] // requires authentication to the server + fn should_fetch_from_repo() { + // uses the current repo and fetches from the remote server + let_assert!(Ok(cwd) = std::env::current_dir()); + let gitdir = GitDir::from(cwd.join("../..")); + let_assert!(Ok(repo) = crate::repository::new().open(&gitdir)); + let_assert!(Ok(_) = repo.fetch()); + } +} diff --git a/crates/git/src/repository/real.rs b/crates/git/src/repository/real.rs index 4d04fd7b..b693c7ad 100644 --- a/crates/git/src/repository/real.rs +++ b/crates/git/src/repository/real.rs @@ -12,7 +12,7 @@ pub struct RealRepository; impl RepositoryLike for RealRepository { fn open(&self, gitdir: &GitDir) -> Result { let gix_repo = gix::ThreadSafeRepository::open(gitdir.to_path_buf())?.to_thread_local(); - Ok(OpenRepository::new(gix_repo)) + Ok(OpenRepository::real(gix_repo)) } fn git_clone(&self, repo_details: &RepoDetails) -> Result { @@ -23,6 +23,6 @@ impl RepositoryLike for RealRepository { )? .fetch_only(gix::progress::Discard, &AtomicBool::new(false))?; - Ok(OpenRepository::new(gix_repo)) + Ok(OpenRepository::real(gix_repo)) } } diff --git a/crates/git/src/repository/tests.rs b/crates/git/src/repository/tests.rs new file mode 100644 index 00000000..702d6b42 --- /dev/null +++ b/crates/git/src/repository/tests.rs @@ -0,0 +1,207 @@ +mod validate { + use assert2::let_assert; + use git_next_config::{ForgeDetails, GitDir, Hostname, RepoPath}; + + use crate::{ + repository::{self, Direction}, + validate, GitRemote, RepoDetails, + }; + + #[test] + fn should_ok_a_valid_repo() { + let repo_details = RepoDetails::default() + .with_forge( + ForgeDetails::default().with_hostname(Hostname::new("localhost".to_string())), + ) + .with_repo_path(RepoPath::new("kemitix/test".to_string())); + let gitdir = GitDir::from("foo"); + let remote = GitRemote::new( + Hostname::new("localhost"), + RepoPath::new("kemitix/test".to_string()), + ); + + let (repository, mut reality) = repository::mock(); + let_assert!( + Ok(_) = reality.can_open_repo(&gitdir).map( + #[allow(clippy::significant_drop_tightening)] + |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); + }) + ); + } + ) + ); + + let_assert!(Ok(open_repository) = repository.open(&gitdir)); + + let_assert!(Ok(_) = validate(&open_repository, &repo_details)); + } + + #[test] + fn should_fail_where_no_default_push_remote() { + let repo_details = RepoDetails::default() + .with_forge( + ForgeDetails::default().with_hostname(Hostname::new("localhost".to_string())), + ) + .with_repo_path(RepoPath::new("kemitix/test".to_string())); + let gitdir = GitDir::from("foo"); + let remote = GitRemote::new( + Hostname::new("localhost"), + RepoPath::new("kemitix/test".to_string()), + ); + + let (repository, mut reality) = repository::mock(); + let_assert!( + Ok(_) = reality.can_open_repo(&gitdir).map( + #[allow(clippy::significant_drop_tightening)] + |open_repo| { + 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); + }) + ); + } + ) + ); + + let_assert!(Ok(open_repository) = repository.open(&gitdir)); + + let_assert!(Err(_) = validate(&open_repository, &repo_details)); + } + #[test] + fn should_fail_where_no_default_fetch_remote() { + let repo_details = RepoDetails::default() + .with_forge( + ForgeDetails::default().with_hostname(Hostname::new("localhost".to_string())), + ) + .with_repo_path(RepoPath::new("kemitix/test".to_string())); + let gitdir = GitDir::from("foo"); + let remote = GitRemote::new( + Hostname::new("localhost"), + RepoPath::new("kemitix/test".to_string()), + ); + + let (repository, mut reality) = repository::mock(); + let_assert!( + Ok(_) = reality.can_open_repo(&gitdir).map( + #[allow(clippy::significant_drop_tightening)] + |open_repo| { + let_assert!( + Ok(_) = open_repo.lock().map(|mut open_repo| { + open_repo.has_default_remote(Direction::Push, remote); + // INFO: open_repo.has_default_remote(Direction::Fetch, remote); + }) + ); + } + ) + ); + + let_assert!(Ok(open_repository) = repository.open(&gitdir)); + + let_assert!(Err(_) = validate(&open_repository, &repo_details)); + } + #[test] + fn should_fail_where_invalid_default_push_remote() { + let repo_details = RepoDetails::default() + .with_forge( + ForgeDetails::default().with_hostname(Hostname::new("localhost".to_string())), + ) + .with_repo_path(RepoPath::new("kemitix/test".to_string())); + let gitdir = GitDir::from("foo"); + let remote = GitRemote::new( + Hostname::new("localhost"), + RepoPath::new("kemitix/test".to_string()), + ); + let other_remote = GitRemote::new( + Hostname::new("localhost"), + RepoPath::new("kemitix/other".to_string()), + ); + + let (repository, mut reality) = repository::mock(); + let_assert!( + Ok(_) = reality.can_open_repo(&gitdir).map( + #[allow(clippy::significant_drop_tightening)] + |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); + }) + ); + } + ) + ); + + let_assert!(Ok(open_repository) = repository.open(&gitdir)); + + let_assert!(Err(_) = validate(&open_repository, &repo_details)); + } + #[test] + fn should_fail_where_invalid_default_fetch_remote() { + let repo_details = RepoDetails::default() + .with_forge( + ForgeDetails::default().with_hostname(Hostname::new("localhost".to_string())), + ) + .with_repo_path(RepoPath::new("kemitix/test".to_string())); + let gitdir = GitDir::from("foo"); + let remote = GitRemote::new( + Hostname::new("localhost"), + RepoPath::new("kemitix/test".to_string()), + ); + let other_remote = GitRemote::new( + Hostname::new("localhost"), + RepoPath::new("kemitix/other".to_string()), + ); + + let (repository, mut reality) = repository::mock(); + let_assert!( + Ok(_) = reality.can_open_repo(&gitdir).map( + #[allow(clippy::significant_drop_tightening)] + |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); + }) + ); + } + ) + ); + + let_assert!(Ok(open_repository) = repository.open(&gitdir)); + + let_assert!(Err(_) = validate(&open_repository, &repo_details)); + } +} + +mod git_clone { + use assert2::let_assert; + use git_next_config::{ForgeDetails, GitDir, Hostname, RepoPath}; + + use crate::{repository::Direction, GitRemote, RepoDetails}; + + #[test] + fn should_clone_repo() { + let_assert!(Ok(fs) = kxio::fs::temp()); + let r = crate::repository::new(); + let repo_details = RepoDetails::default() + .with_forge( + ForgeDetails::default().with_hostname(Hostname::new("git.kemitix.net".to_string())), + ) + .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)); + assert_eq!( + remote, + GitRemote::new( + Hostname::new("git.kemitix.net"), + RepoPath::new("kemitix/git-next".to_string()) + ) + ); + } +} diff --git a/crates/git/src/validate.rs b/crates/git/src/validate.rs index 31e306dd..6505ee59 100644 --- a/crates/git/src/validate.rs +++ b/crates/git/src/validate.rs @@ -6,13 +6,13 @@ use super::{GitRemote, RepoDetails}; #[tracing::instrument(skip_all)] pub fn validate(repository: &OpenRepository, repo_details: &RepoDetails) -> Result<()> { - let git_remote = repo_details.git_remote(); let Some(push_remote) = repository.find_default_remote(Direction::Push) else { return Err(Error::NoDefaultPushRemote); }; let Some(fetch_remote) = repository.find_default_remote(Direction::Fetch) else { return Err(Error::NoDefaultFetchRemote); }; + let git_remote = repo_details.git_remote(); info!(config = %git_remote, push = %push_remote, fetch = %fetch_remote, "Check remotes match"); if git_remote != push_remote { return Err(Error::MismatchDefaultPushRemote { diff --git a/crates/server/src/actors/server.rs b/crates/server/src/actors/server.rs index c111bf80..eb6f9263 100644 --- a/crates/server/src/actors/server.rs +++ b/crates/server/src/actors/server.rs @@ -177,7 +177,7 @@ impl Server { let server_storage = server_storage.clone(); let webhook = webhook.clone(); let net = self.net.clone(); - let repo = self.repo; + let repo = self.repo.clone(); let generation = self.generation; move |(repo_alias, server_repo_config)| { let span = tracing::info_span!("create_actor", alias = %repo_alias, config = %server_repo_config); @@ -205,8 +205,13 @@ impl Server { gitdir, ); info!("Starting Repo Actor"); - let actor = - RepoActor::new(repo_details, webhook.clone(), generation, net.clone(), repo); + let actor = RepoActor::new( + repo_details, + webhook.clone(), + generation, + net.clone(), + repo.clone(), + ); (forge_name.clone(), repo_alias, actor) } } diff --git a/crates/server/src/gitforge/tests/forgejo.rs b/crates/server/src/gitforge/tests/forgejo.rs index ebd77caf..3edf231e 100644 --- a/crates/server/src/gitforge/tests/forgejo.rs +++ b/crates/server/src/gitforge/tests/forgejo.rs @@ -13,7 +13,7 @@ fn test_name() { panic!("fs") }; let net = Network::new_mock(); - let repo = git::repository::mock(); + let (repo, _reality) = git::repository::mock(); let repo_details = common::repo_details( 1, Generation::new(), @@ -40,7 +40,7 @@ async fn test_branches_get() { let body = include_str!("./data-forgejo-branches-get.json"); net.add_get_response(&url, StatusCode::OK, body); let net = Network::from(net); - let repo = git::repository::mock(); + let (repo, _reality) = git::repository::mock(); let repo_details = common::repo_details( 1,