feat(server): use cached Repository from RepoActor

This commit is contained in:
Paul Campbell 2024-05-09 21:53:50 +01:00
parent 992821d563
commit 62bee38c85
11 changed files with 93 additions and 69 deletions

View file

@ -17,6 +17,7 @@ pub async fn advance_next(
dev_commit_history: Vec<gitforge::Commit>, dev_commit_history: Vec<gitforge::Commit>,
repo_config: config::RepoConfig, repo_config: config::RepoConfig,
forge: gitforge::Forge, forge: gitforge::Forge,
repository: gitforge::Repository,
addr: Addr<super::RepoActor>, addr: Addr<super::RepoActor>,
message_token: super::MessageToken, message_token: super::MessageToken,
) { ) {
@ -31,6 +32,7 @@ pub async fn advance_next(
} }
info!("Advancing next to commit '{}'", commit); info!("Advancing next to commit '{}'", commit);
if let Err(err) = forge.branch_reset( if let Err(err) = forge.branch_reset(
&repository,
repo_config.branches().next(), repo_config.branches().next(),
commit.into(), commit.into(),
gitforge::Force::No, gitforge::Force::No,
@ -79,11 +81,13 @@ pub async fn advance_main(
next: gitforge::Commit, next: gitforge::Commit,
repo_config: config::RepoConfig, repo_config: config::RepoConfig,
forge: gitforge::Forge, forge: gitforge::Forge,
repository: gitforge::Repository,
addr: Addr<RepoActor>, addr: Addr<RepoActor>,
message_token: super::MessageToken, message_token: super::MessageToken,
) { ) {
info!("Advancing main to next"); info!("Advancing main to next");
if let Err(err) = forge.branch_reset( if let Err(err) = forge.branch_reset(
&repository,
repo_config.branches().main(), repo_config.branches().main(),
next.into(), next.into(),
gitforge::Force::No, gitforge::Force::No,

View file

@ -191,13 +191,15 @@ impl Handler<ValidateRepo> for RepoActor {
.into_actor(self) .into_actor(self)
.wait(ctx); .wait(ctx);
} }
if let Some(repo_config) = self.details.repo_config.clone() { if let (Some(repository), Some(repo_config)) =
(self.repository.clone(), self.details.repo_config.clone())
{
let forge = self.forge.clone(); let forge = self.forge.clone();
let addr = ctx.address(); let addr = ctx.address();
let message_token = self.message_token; let message_token = self.message_token;
async move { async move {
forge forge
.branches_validate_positions(repo_config, addr, message_token) .branches_validate_positions(repository, repo_config, addr, message_token)
.await .await
} }
.in_current_span() .in_current_span()
@ -238,11 +240,13 @@ impl Handler<StartMonitoring> for RepoActor {
.into_actor(self) .into_actor(self)
.wait(ctx); .wait(ctx);
} else if dev_ahead_of_next { } else if dev_ahead_of_next {
if let Some(repository) = self.repository.clone() {
branch::advance_next( branch::advance_next(
msg.next, msg.next,
msg.dev_commit_history, msg.dev_commit_history,
repo_config, repo_config,
forge, forge,
repository,
addr, addr,
self.message_token, self.message_token,
) )
@ -251,6 +255,7 @@ impl Handler<StartMonitoring> for RepoActor {
.wait(ctx); .wait(ctx);
} }
} }
}
} }
#[derive(Message)] #[derive(Message)]
@ -278,9 +283,20 @@ impl Handler<AdvanceMainTo> for RepoActor {
warn!("No config loaded"); warn!("No config loaded");
return; return;
}; };
let Some(repository) = self.repository.clone() else {
warn!("No repository opened");
return;
};
let forge = self.forge.clone(); let forge = self.forge.clone();
let addr = ctx.address(); let addr = ctx.address();
branch::advance_main(msg.0, repo_config, forge, addr, self.message_token) branch::advance_main(
msg.0,
repo_config,
forge,
repository,
addr,
self.message_token,
)
.in_current_span() .in_current_span()
.into_actor(self) .into_actor(self)
.wait(ctx); .wait(ctx);

View file

@ -14,7 +14,7 @@ use serde::Deserialize;
use kxio::fs::FileSystem; use kxio::fs::FileSystem;
use tracing::info; use tracing::info;
use crate::server::types::ServerGeneration; use crate::server::{gitforge::Repository, types::ServerGeneration};
#[derive(Debug, derive_more::From, derive_more::Display)] #[derive(Debug, derive_more::From, derive_more::Display)]
pub enum Error { pub enum Error {
@ -418,11 +418,6 @@ impl RepoDetails {
origin.into() origin.into()
} }
#[allow(dead_code)]
pub fn validate_repo(&self) -> ValidationResult<()> {
self.gitdir.validate(self)
}
pub fn git_remote(&self) -> GitRemote { pub fn git_remote(&self) -> GitRemote {
GitRemote::new(self.forge.hostname.clone(), self.repo_path.clone()) GitRemote::new(self.forge.hostname.clone(), self.repo_path.clone())
} }
@ -516,11 +511,15 @@ impl GitDir {
} }
#[tracing::instrument(skip_all)] #[tracing::instrument(skip_all)]
pub fn validate(&self, repo_details: &RepoDetails) -> ValidationResult<()> { pub fn validate(
&self,
repository: &Repository,
repo_details: &RepoDetails,
) -> ValidationResult<()> {
let git_remote = repo_details.git_remote(); let git_remote = repo_details.git_remote();
use gix::remote::Direction; use gix::remote::Direction;
let push_remote = self.find_default_remote(Direction::Push)?; let push_remote = self.find_default_remote(repository, Direction::Push)?;
let fetch_remote = self.find_default_remote(Direction::Fetch)?; let fetch_remote = self.find_default_remote(repository, Direction::Fetch)?;
info!(config = %git_remote, push = %push_remote, fetch = %fetch_remote, "Check remotes match"); info!(config = %git_remote, push = %push_remote, fetch = %fetch_remote, "Check remotes match");
if git_remote != push_remote { if git_remote != push_remote {
return Err(RepoValidationError::MismatchDefaultPushRemote { return Err(RepoValidationError::MismatchDefaultPushRemote {
@ -540,11 +539,10 @@ impl GitDir {
#[tracing::instrument(skip_all, fields(?direction))] #[tracing::instrument(skip_all, fields(?direction))]
fn find_default_remote( fn find_default_remote(
&self, &self,
repository: &Repository,
direction: gix::remote::Direction, direction: gix::remote::Direction,
) -> ValidationResult<GitRemote> { ) -> ValidationResult<GitRemote> {
let repository = gix::ThreadSafeRepository::open(self.deref()) let repository = repository.deref().to_thread_local();
.map_err(|e| RepoValidationError::UnableToOpenRepo(e.to_string()))?
.to_thread_local();
let Some(Ok(remote)) = repository.find_default_remote(gix::remote::Direction::Push) else { let Some(Ok(remote)) = repository.find_default_remote(gix::remote::Direction::Push) else {
return Err(RepoValidationError::NoDefaultPushRemote); return Err(RepoValidationError::NoDefaultPushRemote);
}; };

View file

@ -166,7 +166,8 @@ fn repo_details_find_default_push_remote_finds_correct_remote() -> Result<()> {
repo_details.forge.hostname = Hostname("git.kemitix.net".to_string()); repo_details.forge.hostname = Hostname("git.kemitix.net".to_string());
repo_details.repo_path = RepoPath("kemitix/git-next".to_string()); repo_details.repo_path = RepoPath("kemitix/git-next".to_string());
let gitdir = &repo_details.gitdir; let gitdir = &repo_details.gitdir;
let found_git_remote = gitdir.find_default_remote(Direction::Push)?; let repository = gix::ThreadSafeRepository::open(gitdir.to_path_buf())?;
let found_git_remote = gitdir.find_default_remote(&repository.into(), Direction::Push)?;
let config_git_remote = repo_details.git_remote(); let config_git_remote = repo_details.git_remote();
assert_eq!( assert_eq!(
@ -189,29 +190,15 @@ fn gitdir_validate_should_pass_a_valid_git_repo() -> Result<()> {
); );
repo_details.forge.hostname = Hostname("git.kemitix.net".to_string()); repo_details.forge.hostname = Hostname("git.kemitix.net".to_string());
repo_details.repo_path = RepoPath("kemitix/git-next".to_string()); repo_details.repo_path = RepoPath("kemitix/git-next".to_string());
let git_dir = &repo_details.gitdir; let gitdir = &repo_details.gitdir;
git_dir.validate(&repo_details)?; let repository = gix::ThreadSafeRepository::open(gitdir.to_path_buf())?;
gitdir.validate(&repository.into(), &repo_details)?;
Ok(()) Ok(())
} }
#[test] #[test]
fn gitdir_validate_should_fail_a_non_git_dir() { fn gitdir_validate_should_fail_a_git_repo_with_wrong_remote() -> Result<()> {
let_assert!(Ok(fs) = kxio::fs::temp());
let cwd = fs.base();
let repo_details = common::repo_details(
1,
ServerGeneration::new(),
common::forge_details(1, ForgeType::MockForge),
None,
GitDir::new(cwd), // Server GitDir - should be ignored
);
let git_dir = &repo_details.gitdir;
let_assert!(Err(_) = git_dir.validate(&repo_details));
}
#[test]
fn gitdir_validate_should_fail_a_git_repo_with_wrong_remote() {
let_assert!(Ok(cwd) = std::env::current_dir().map_err(RepoValidationError::Io)); let_assert!(Ok(cwd) = std::env::current_dir().map_err(RepoValidationError::Io));
let mut repo_details = common::repo_details( let mut repo_details = common::repo_details(
1, 1,
@ -222,8 +209,11 @@ fn gitdir_validate_should_fail_a_git_repo_with_wrong_remote() {
); );
repo_details.forge.hostname = Hostname("localhost".to_string()); repo_details.forge.hostname = Hostname("localhost".to_string());
repo_details.repo_path = RepoPath("hello/world".to_string()); repo_details.repo_path = RepoPath("hello/world".to_string());
let git_dir = &repo_details.gitdir; let gitdir = &repo_details.gitdir;
let_assert!(Err(_) = git_dir.validate(&repo_details)); let repository = gix::ThreadSafeRepository::open(gitdir.to_path_buf())?;
let_assert!(Err(_) = gitdir.validate(&repository.into(), &repo_details));
Ok(())
} }
#[test] #[test]

View file

@ -5,22 +5,19 @@ use tracing::{info, warn};
use crate::server::{ use crate::server::{
config::{BranchName, RepoDetails}, config::{BranchName, RepoDetails},
gitforge::{BranchResetError, BranchResetResult, Force}, gitforge::{BranchResetError, BranchResetResult, Force, Repository},
types::GitRef, types::GitRef,
}; };
// TODO: (#72) reimplement using `gix` // TODO: (#72) reimplement using `gix`
#[tracing::instrument(skip_all, fields(branch = %branch_name, to = %to_commit, force = %force))] #[tracing::instrument(skip_all, fields(branch = %branch_name, to = %to_commit, force = %force))]
pub fn reset( pub fn reset(
repository: &Repository,
repo_details: &RepoDetails, repo_details: &RepoDetails,
branch_name: BranchName, branch_name: BranchName,
to_commit: GitRef, to_commit: GitRef,
force: Force, force: Force,
) -> BranchResetResult { ) -> BranchResetResult {
let repository = gix::ThreadSafeRepository::open(repo_details.gitdir.deref())
.map_err(Box::new)?
.to_thread_local();
let gitdir = repository.git_dir();
let origin = repo_details.origin(); let origin = repo_details.origin();
let force = match force { let force = match force {
Force::No => "".to_string(), Force::No => "".to_string(),
@ -33,7 +30,7 @@ pub fn reset(
) )
.into(); .into();
let ctx = gix::diff::command::Context { let ctx = gix::diff::command::Context {
git_dir: Some(gitdir.to_path_buf()), git_dir: Some(repository.deref().git_dir().to_path_buf()),
..Default::default() ..Default::default()
}; };
match gix::command::prepare(command.expose_secret()) match gix::command::prepare(command.expose_secret())

View file

@ -2,7 +2,7 @@ use std::ops::Deref;
use tracing::{debug, info}; use tracing::{debug, info};
use crate::server::config::RepoDetails; use crate::server::{config::RepoDetails, gitforge::Repository};
#[derive(Debug, derive_more::From, derive_more::Display)] #[derive(Debug, derive_more::From, derive_more::Display)]
pub enum Error { pub enum Error {
@ -14,12 +14,8 @@ pub enum Error {
impl std::error::Error for Error {} impl std::error::Error for Error {}
#[tracing::instrument(skip_all, fields(repo = %repo_details))] #[tracing::instrument(skip_all, fields(repo = %repo_details))]
pub fn fetch(repo_details: &RepoDetails) -> Result<(), Error> { pub fn fetch(repository: &Repository, repo_details: &RepoDetails) -> Result<(), Error> {
// INFO: gitdir validate tests that the default fetch remote matches the configured remote let repository = repository.deref().to_thread_local();
let repository = gix::ThreadSafeRepository::open(repo_details.gitdir.deref())
.map_err(Box::new)?
.to_thread_local();
debug!(?repository, "opened repo");
let Some(remote) = repository.find_default_remote(gix::remote::Direction::Fetch) else { let Some(remote) = repository.find_default_remote(gix::remote::Direction::Fetch) else {
return Err(Error::NoFetchRemoteFound); return Err(Error::NoFetchRemoteFound);
}; };

View file

@ -30,6 +30,7 @@ pub struct ValidatedPositions {
pub async fn validate_positions( pub async fn validate_positions(
forge: &gitforge::forgejo::ForgeJoEnv, forge: &gitforge::forgejo::ForgeJoEnv,
repository: &gitforge::Repository,
repo_config: RepoConfig, repo_config: RepoConfig,
) -> Result<ValidatedPositions, Error> { ) -> Result<ValidatedPositions, Error> {
let repo_details = &forge.repo_details; let repo_details = &forge.repo_details;
@ -73,7 +74,9 @@ pub async fn validate_positions(
let next_is_ancestor_of_dev = commit_histories.dev.iter().any(|dev| dev == &next); let next_is_ancestor_of_dev = commit_histories.dev.iter().any(|dev| dev == &next);
if !next_is_ancestor_of_dev { if !next_is_ancestor_of_dev {
info!("Next is not an ancestor of dev - resetting next to main"); info!("Next is not an ancestor of dev - resetting next to main");
if let Err(err) = forge.branch_reset( if let Err(err) = forge.branch_reset(
repository,
repo_config.branches().next(), repo_config.branches().next(),
main.into(), main.into(),
gitforge::Force::From(next.clone().into()), gitforge::Force::From(next.clone().into()),
@ -99,6 +102,7 @@ pub async fn validate_positions(
repo_config.branches().next() repo_config.branches().next()
); );
if let Err(err) = forge.branch_reset( if let Err(err) = forge.branch_reset(
repository,
repo_config.branches().next(), repo_config.branches().next(),
main.into(), main.into(),
gitforge::Force::From(next.clone().into()), gitforge::Force::From(next.clone().into()),

View file

@ -48,11 +48,12 @@ impl super::ForgeLike for ForgeJoEnv {
async fn branches_validate_positions( async fn branches_validate_positions(
&self, &self,
repository: Repository,
repo_config: RepoConfig, repo_config: RepoConfig,
addr: Addr<RepoActor>, addr: Addr<RepoActor>,
message_token: MessageToken, message_token: MessageToken,
) { ) {
match branch::validate_positions(self, repo_config).await { match branch::validate_positions(self, &repository, repo_config).await {
Ok(ValidatedPositions { Ok(ValidatedPositions {
main, main,
next, next,
@ -76,12 +77,19 @@ impl super::ForgeLike for ForgeJoEnv {
fn branch_reset( fn branch_reset(
&self, &self,
repository: &Repository,
branch_name: BranchName, branch_name: BranchName,
to_commit: GitRef, to_commit: GitRef,
force: gitforge::Force, force: gitforge::Force,
) -> gitforge::BranchResetResult { ) -> gitforge::BranchResetResult {
branch::fetch(&self.repo_details)?; branch::fetch(repository, &self.repo_details)?;
git::reset(&self.repo_details, branch_name, to_commit, force) git::reset(
repository,
&self.repo_details,
branch_name,
to_commit,
force,
)
} }
async fn commit_status(&self, commit: &gitforge::Commit) -> gitforge::CommitStatus { async fn commit_status(&self, commit: &gitforge::Commit) -> gitforge::CommitStatus {
@ -129,7 +137,7 @@ impl super::ForgeLike for ForgeJoEnv {
} }
fn repo_clone(&self, gitdir: GitDir) -> Result<Repository, RepoCloneError> { fn repo_clone(&self, gitdir: GitDir) -> Result<Repository, RepoCloneError> {
let repo = if !gitdir.exists() { let repository = if !gitdir.exists() {
info!("Local copy not found - cloning..."); info!("Local copy not found - cloning...");
repo::clone(&self.repo_details, gitdir.clone())? repo::clone(&self.repo_details, gitdir.clone())?
} else { } else {
@ -137,10 +145,10 @@ impl super::ForgeLike for ForgeJoEnv {
}; };
info!("Validating..."); info!("Validating...");
gitdir gitdir
.validate(&self.repo_details) .validate(&repository, &self.repo_details)
.map_err(|e| RepoCloneError::Validation(e.to_string())) .map_err(|e| RepoCloneError::Validation(e.to_string()))
.inspect(|_| info!("Validation - OK"))?; .inspect(|_| info!("Validation - OK"))?;
Ok(repo) Ok(repository)
} }
} }

View file

@ -33,6 +33,7 @@ impl super::ForgeLike for MockForgeEnv {
async fn branches_validate_positions( async fn branches_validate_positions(
&self, &self,
_repository: Repository,
_repo_config: RepoConfig, _repo_config: RepoConfig,
_addr: actix::prelude::Addr<RepoActor>, _addr: actix::prelude::Addr<RepoActor>,
_message_token: MessageToken, _message_token: MessageToken,
@ -42,6 +43,7 @@ impl super::ForgeLike for MockForgeEnv {
fn branch_reset( fn branch_reset(
&self, &self,
_repository: &Repository,
_branch_name: BranchName, _branch_name: BranchName,
_to_commit: GitRef, _to_commit: GitRef,
_force: gitforge::Force, _force: gitforge::Force,

View file

@ -39,6 +39,7 @@ pub trait ForgeLike {
/// positions as needed. /// positions as needed.
async fn branches_validate_positions( async fn branches_validate_positions(
&self, &self,
repository: Repository,
repo_config: RepoConfig, repo_config: RepoConfig,
addr: actix::prelude::Addr<super::actors::repo::RepoActor>, addr: actix::prelude::Addr<super::actors::repo::RepoActor>,
message_token: MessageToken, message_token: MessageToken,
@ -47,6 +48,7 @@ pub trait ForgeLike {
/// Moves a branch to a new commit. /// Moves a branch to a new commit.
fn branch_reset( fn branch_reset(
&self, &self,
repository: &Repository,
branch_name: BranchName, branch_name: BranchName,
to_commit: GitRef, to_commit: GitRef,
force: Force, force: Force,

View file

@ -99,3 +99,10 @@ impl std::fmt::Display for Message {
#[derive(Debug, Clone, derive_more::From)] #[derive(Debug, Clone, derive_more::From)]
pub struct Repository(gix::ThreadSafeRepository); pub struct Repository(gix::ThreadSafeRepository);
impl std::ops::Deref for Repository {
type Target = gix::ThreadSafeRepository;
fn deref(&self) -> &Self::Target {
&self.0
}
}