diff --git a/crates/repo-actor/src/handlers/webhook_notification.rs b/crates/repo-actor/src/handlers/webhook_notification.rs index a74e8c5d..d9011373 100644 --- a/crates/repo-actor/src/handlers/webhook_notification.rs +++ b/crates/repo-actor/src/handlers/webhook_notification.rs @@ -2,9 +2,12 @@ use actix::prelude::*; use crate::{self as actor, messages::WebhookNotification, RepoActorLog}; -use git_next_config::webhook::push::Branch; use git_next_config::WebhookAuth; -use git_next_git::{self as git, ForgeLike}; +use git_next_config::{ + webhook::{push::Branch, Push}, + BranchName, +}; +use git_next_git::{self as git, Commit, ForgeLike}; use tracing::{info, warn}; @@ -38,46 +41,40 @@ impl Handler for actor::RepoActor { return; } Some(Branch::Main) => { - actor::logger(&self.log, "message is for main branch"); - let commit = git::Commit::from(push); - if self.last_main_commit.as_ref() == Some(&commit) { - actor::logger(&self.log, "not a new commit on main"); - info!( - branch = %config.branches().main(), - %commit, - "Ignoring - already aware of branch at commit", - ); + if handle_push( + push, + config.branches().main(), + &mut self.last_main_commit, + &self.log, + ) + .is_err() + { return; - } - self.last_main_commit.replace(commit); + }; } Some(Branch::Next) => { - actor::logger(&self.log, "message is for next branch"); - let commit = git::Commit::from(push); - if self.last_next_commit.as_ref() == Some(&commit) { - actor::logger(&self.log, "not a new commit on next"); - info!( - branch = %config.branches().next(), - %commit, - "Ignoring - already aware of branch at commit", - ); + if handle_push( + push, + config.branches().next(), + &mut self.last_next_commit, + &self.log, + ) + .is_err() + { return; - } - self.last_next_commit.replace(commit); + }; } Some(Branch::Dev) => { - actor::logger(&self.log, "message is for dev branch"); - let commit = git::Commit::from(push); - if self.last_dev_commit.as_ref() == Some(&commit) { - actor::logger(&self.log, "not a new commit on dev"); - info!( - branch = %config.branches().dev(), - %commit, - "Ignoring - already aware of branch at commit", - ); + if handle_push( + push, + config.branches().dev(), + &mut self.last_dev_commit, + &self.log, + ) + .is_err() + { return; - } - self.last_dev_commit.replace(commit); + }; } }, } @@ -120,3 +117,24 @@ fn validate_notification( } Ok(()) } + +fn handle_push( + push: Push, + branch: BranchName, + last_commit: &mut Option, + log: &Option, +) -> Result<(), ()> { + actor::logger(log, "message is for dev branch"); + let commit = git::Commit::from(push); + if last_commit.as_ref() == Some(&commit) { + actor::logger(log, format!("not a new commit on {branch}")); + info!( + %branch , + %commit, + "Ignoring - already aware of branch at commit", + ); + return Err(()); + } + last_commit.replace(commit); + Ok(()) +} diff --git a/crates/repo-actor/src/tests/handlers/webhook_notification.rs b/crates/repo-actor/src/tests/handlers/webhook_notification.rs index f20024a9..c2db1100 100644 --- a/crates/repo-actor/src/tests/handlers/webhook_notification.rs +++ b/crates/repo-actor/src/tests/handlers/webhook_notification.rs @@ -252,8 +252,9 @@ async fn when_message_is_push_already_seen_commit_to_main() -> TestResult { let forge_notification = ForgeNotification::new(forge_alias, repo_alias, headers, body); let repository_factory = MockRepositoryFactory::new(); let commit = given::a_commit(); + let main = repo_config.branches().main(); let push = given::a_push() - .with_branch(repo_config.branches().main()) + .with_branch(main.clone()) .with_sha(commit.sha().to_string()) .with_message(commit.message().to_string()); let mut forge = given::a_forge(); @@ -283,7 +284,7 @@ async fn when_message_is_push_already_seen_commit_to_main() -> TestResult { //then log.no_message_contains("send")?; - log.require_message_containing("not a new commit on main")?; + log.require_message_containing(format!("not a new commit on {main}"))?; Ok(()) } @@ -300,8 +301,9 @@ async fn when_message_is_push_already_seen_commit_to_next() -> TestResult { let forge_notification = ForgeNotification::new(forge_alias, repo_alias, headers, body); let repository_factory = MockRepositoryFactory::new(); let commit = given::a_commit(); + let next = repo_config.branches().next(); let push = given::a_push() - .with_branch(repo_config.branches().next()) + .with_branch(next.clone()) .with_sha(commit.sha().to_string()) .with_message(commit.message().to_string()); let mut forge = given::a_forge(); @@ -331,7 +333,7 @@ async fn when_message_is_push_already_seen_commit_to_next() -> TestResult { //then log.no_message_contains("send")?; - log.require_message_containing("not a new commit on next")?; + log.require_message_containing(format!("not a new commit on {next}"))?; Ok(()) } @@ -348,8 +350,9 @@ async fn when_message_is_push_already_seen_commit_to_dev() -> TestResult { let forge_notification = ForgeNotification::new(forge_alias, repo_alias, headers, body); let repository_factory = MockRepositoryFactory::new(); let commit = given::a_commit(); + let dev = repo_config.branches().dev(); let push = given::a_push() - .with_branch(repo_config.branches().dev()) + .with_branch(dev.clone()) .with_sha(commit.sha().to_string()) .with_message(commit.message().to_string()); let mut forge = given::a_forge(); @@ -379,7 +382,7 @@ async fn when_message_is_push_already_seen_commit_to_dev() -> TestResult { //then log.no_message_contains("send")?; - log.require_message_containing("not a new commit on dev")?; + log.require_message_containing(format!("not a new commit on {dev}"))?; Ok(()) }