From bcf57bc728fd53f0abb9c4e94d9768fcce5e9dbe Mon Sep 17 00:00:00 2001 From: Paul Campbell Date: Fri, 19 Jul 2024 19:41:20 +0100 Subject: [PATCH] feat: dispatch NotifyUser messages to server for user (1/2) --- .../src/handlers/load_config_from_repo.rs | 8 ++++--- .../src/handlers/receive_ci_status.rs | 4 ++-- .../src/handlers/register_webhook.rs | 24 ++++++++++++++----- crates/repo-actor/src/lib.rs | 13 ++++++++++ crates/repo-actor/src/messages.rs | 3 +++ crates/repo-actor/src/tests/given.rs | 1 + .../src/tests/handlers/register_webhook.rs | 1 - crates/repo-actor/src/user_notification.rs | 9 +++++++ .../server-actor/src/handlers/file_updated.rs | 4 ++-- crates/server-actor/src/handlers/mod.rs | 1 + .../src/handlers/notify_user.rs | 6 +++-- .../src/handlers/receive_server_config.rs | 4 ++-- .../handlers/receive_valid_server_config.rs | 17 +++++++++---- crates/server-actor/src/handlers/shutdown.rs | 4 ++-- crates/server-actor/src/lib.rs | 24 ++++++++++++++----- .../src/tests/receive_server_config.rs | 4 ++-- crates/server/src/lib.rs | 4 ++-- crates/webhook-actor/src/handlers/mod.rs | 1 - crates/webhook-actor/src/messages.rs | 2 -- 19 files changed, 96 insertions(+), 38 deletions(-) create mode 100644 crates/repo-actor/src/user_notification.rs rename crates/{webhook-actor => server-actor}/src/handlers/notify_user.rs (53%) diff --git a/crates/repo-actor/src/handlers/load_config_from_repo.rs b/crates/repo-actor/src/handlers/load_config_from_repo.rs index 7213dad..4a64a45 100644 --- a/crates/repo-actor/src/handlers/load_config_from_repo.rs +++ b/crates/repo-actor/src/handlers/load_config_from_repo.rs @@ -2,7 +2,7 @@ use actix::prelude::*; use tracing::Instrument as _; -use crate as actor; +use crate::{self as actor}; impl Handler for actor::RepoActor { type Result = (); @@ -26,8 +26,10 @@ impl Handler for actor::RepoActor { actor::logger(&log, "send: LoadedConfig"); addr.do_send(actor::messages::ReceiveRepoConfig::new(repo_config)) } - Err(err) => tracing::warn!(?err, "Failed to load config"), - // TODO: (#95) should notify user + Err(err) => { + tracing::warn!(?err, "Failed to load config"); + // self.notify_user(UserNotification::RepoConfigLoadFailure(err)) + } } } .in_current_span() diff --git a/crates/repo-actor/src/handlers/receive_ci_status.rs b/crates/repo-actor/src/handlers/receive_ci_status.rs index 87418f8..b5fec18 100644 --- a/crates/repo-actor/src/handlers/receive_ci_status.rs +++ b/crates/repo-actor/src/handlers/receive_ci_status.rs @@ -1,5 +1,5 @@ // -use crate as actor; +use crate::{self as actor, UserNotification}; use actix::prelude::*; use git_next_git as git; @@ -33,7 +33,7 @@ impl Handler for actor::RepoActor { } git::forge::commit::Status::Fail => { tracing::warn!("Checks have failed"); - // TODO: (#95) test: repo with next ahead of main and failing CI should notify user + self.notify_user(UserNotification::CICheckFailed(next)); actor::delay_send( addr, sleep_duration, diff --git a/crates/repo-actor/src/handlers/register_webhook.rs b/crates/repo-actor/src/handlers/register_webhook.rs index fda0af8..59edca2 100644 --- a/crates/repo-actor/src/handlers/register_webhook.rs +++ b/crates/repo-actor/src/handlers/register_webhook.rs @@ -2,7 +2,7 @@ use actix::prelude::*; use tracing::Instrument as _; -use crate as actor; +use crate::{self as actor, messages::NotifyUser, UserNotification}; use actor::{messages::RegisterWebhook, RepoActor}; impl Handler for RepoActor { @@ -10,11 +10,12 @@ impl Handler for RepoActor { fn handle(&mut self, _msg: RegisterWebhook, ctx: &mut Self::Context) -> Self::Result { if self.webhook_id.is_none() { - let forge_alias = self.repo_details.forge.forge_alias(); - let repo_alias = &self.repo_details.repo_alias; - let webhook_url = self.webhook.url(forge_alias, repo_alias); + let forge_alias = self.repo_details.forge.forge_alias().clone(); + let repo_alias = self.repo_details.repo_alias.clone(); + let webhook_url = self.webhook.url(&forge_alias, &repo_alias); let forge = self.forge.duplicate(); let addr = ctx.address(); + let notify_user_recipient = self.notify_user_recipient.clone(); let log = self.log.clone(); tracing::debug!("registering webhook"); async move { @@ -27,13 +28,24 @@ impl Handler for RepoActor { &log, ); } - Err(err) => tracing::warn!(?err, "registering webhook"), + Err(err) => { + let msg = NotifyUser::from(UserNotification::WebhookRegistration( + forge_alias, + repo_alias, + err.to_string(), + )); + let log_message = format!("send: {:?}", msg); + tracing::debug!(log_message); + actor::logger(&log, log_message); + if let Some(notify_user_recipient) = notify_user_recipient { + notify_user_recipient.do_send(msg); + } + } } } .in_current_span() .into_actor(self) .wait(ctx); - tracing::debug!("registering webhook done"); } } } diff --git a/crates/repo-actor/src/lib.rs b/crates/repo-actor/src/lib.rs index 91b6f96..1d29adf 100644 --- a/crates/repo-actor/src/lib.rs +++ b/crates/repo-actor/src/lib.rs @@ -2,6 +2,7 @@ mod branch; pub mod handlers; mod load; pub mod messages; +mod user_notification; #[cfg(test)] mod tests; @@ -13,6 +14,8 @@ use actix::prelude::*; use derive_more::Deref; use git_next_config as config; use git_next_git as git; +use messages::NotifyUser; +pub use user_notification::UserNotification; use kxio::network::Network; use tracing::{info, warn, Instrument}; @@ -48,8 +51,10 @@ pub struct RepoActor { net: Network, forge: Box, log: Option, + notify_user_recipient: Option>, } impl RepoActor { + #[allow(clippy::too_many_arguments)] pub fn new( repo_details: git::RepoDetails, forge: Box, @@ -58,6 +63,7 @@ impl RepoActor { net: Network, repository_factory: Box, sleep_duration: std::time::Duration, + notify_user_recipient: Option>, ) -> Self { let message_token = messages::MessageToken::default(); Self { @@ -76,6 +82,13 @@ impl RepoActor { net, sleep_duration, log: None, + notify_user_recipient, + } + } + + fn notify_user(&mut self, user_notification: UserNotification) { + if let Some(recipient) = &self.notify_user_recipient { + recipient.do_send(NotifyUser::from(user_notification)); } } } diff --git a/crates/repo-actor/src/messages.rs b/crates/repo-actor/src/messages.rs index 06e79c5..7a61491 100644 --- a/crates/repo-actor/src/messages.rs +++ b/crates/repo-actor/src/messages.rs @@ -6,6 +6,8 @@ use git_next_actor_macros::message; use git_next_config as config; use git_next_git as git; +use crate::UserNotification; + message!(LoadConfigFromRepo: "Request to load the `git-next.toml` from the git repo."); message!(CloneRepo: "Request to clone (or open) the git repo."); message!(ReceiveRepoConfig: config::RepoConfig: r#"Notification that the `git-next.toml` file has been loaded from the repo and parsed. @@ -63,3 +65,4 @@ Contains a tuple of the commit that was checked (the tip of the `next` branch) a message!(AdvanceNext: (git::Commit, Vec): "Request to advance the `next` branch on to the next commit on the `dev branch."); // next commit and the dev commit history message!(AdvanceMain: git::Commit: "Request to advance the `main` branch on to same commit as the `next` branch."); // next commit message!(WebhookNotification: config::webhook::forge_notification::ForgeNotification: "Notification of a webhook message from the forge."); +message!(NotifyUser: UserNotification: "Request to send the message payload to the notification webhook"); diff --git a/crates/repo-actor/src/tests/given.rs b/crates/repo-actor/src/tests/given.rs index 4d56efa..44a8aa8 100644 --- a/crates/repo-actor/src/tests/given.rs +++ b/crates/repo-actor/src/tests/given.rs @@ -209,6 +209,7 @@ pub fn a_repo_actor( net, repository_factory, std::time::Duration::from_nanos(1), + None, ) .with_log(actors_log), log, diff --git a/crates/repo-actor/src/tests/handlers/register_webhook.rs b/crates/repo-actor/src/tests/handlers/register_webhook.rs index 677f5b3..fea8568 100644 --- a/crates/repo-actor/src/tests/handlers/register_webhook.rs +++ b/crates/repo-actor/src/tests/handlers/register_webhook.rs @@ -36,7 +36,6 @@ async fn when_registered_ok_should_send_webhook_registered() -> TestResult { } #[actix::test] -#[ignore] //TODO: (#95) should notify user async fn when_registered_error_should_send_notify_user() -> TestResult { //given let fs = given::a_filesystem(); diff --git a/crates/repo-actor/src/user_notification.rs b/crates/repo-actor/src/user_notification.rs new file mode 100644 index 0000000..f4a91e6 --- /dev/null +++ b/crates/repo-actor/src/user_notification.rs @@ -0,0 +1,9 @@ +use git_next_config::{ForgeAlias, RepoAlias}; +use git_next_git::Commit; + +#[derive(Clone, Debug, PartialEq, Eq)] +pub enum UserNotification { + CICheckFailed(Commit), + RepoConfigLoadFailure(String), + WebhookRegistration(ForgeAlias, RepoAlias, String), +} diff --git a/crates/server-actor/src/handlers/file_updated.rs b/crates/server-actor/src/handlers/file_updated.rs index fc96f8d..8eda627 100644 --- a/crates/server-actor/src/handlers/file_updated.rs +++ b/crates/server-actor/src/handlers/file_updated.rs @@ -3,9 +3,9 @@ use actix::prelude::*; use git_next_config::server::ServerConfig; use git_next_file_watcher_actor::FileUpdated; -use crate::{messages::ReceiveServerConfig, Server}; +use crate::{messages::ReceiveServerConfig, ServerActor}; -impl Handler for Server { +impl Handler for ServerActor { type Result = (); fn handle(&mut self, _msg: FileUpdated, ctx: &mut Self::Context) -> Self::Result { diff --git a/crates/server-actor/src/handlers/mod.rs b/crates/server-actor/src/handlers/mod.rs index c92aeeb..dcdcab6 100644 --- a/crates/server-actor/src/handlers/mod.rs +++ b/crates/server-actor/src/handlers/mod.rs @@ -1,4 +1,5 @@ mod file_updated; +mod notify_user; mod receive_server_config; mod receive_valid_server_config; mod shutdown; diff --git a/crates/webhook-actor/src/handlers/notify_user.rs b/crates/server-actor/src/handlers/notify_user.rs similarity index 53% rename from crates/webhook-actor/src/handlers/notify_user.rs rename to crates/server-actor/src/handlers/notify_user.rs index adf866a..23ee70f 100644 --- a/crates/webhook-actor/src/handlers/notify_user.rs +++ b/crates/server-actor/src/handlers/notify_user.rs @@ -1,12 +1,14 @@ // use actix::prelude::*; +use git_next_repo_actor::messages::NotifyUser; -use crate::{messages::NotifyUser, WebhookActor}; +use crate::ServerActor; -impl Handler for WebhookActor { +impl Handler for ServerActor { type Result = (); fn handle(&mut self, _msg: NotifyUser, _ctx: &mut Self::Context) -> Self::Result { // TODO: (#95) should notify user + // send post to notification webhook url } } diff --git a/crates/server-actor/src/handlers/receive_server_config.rs b/crates/server-actor/src/handlers/receive_server_config.rs index 028e80a..5465d8c 100644 --- a/crates/server-actor/src/handlers/receive_server_config.rs +++ b/crates/server-actor/src/handlers/receive_server_config.rs @@ -2,10 +2,10 @@ use actix::prelude::*; use crate::{ messages::{ReceiveServerConfig, ReceiveValidServerConfig, ValidServerConfig}, - Server, + ServerActor, }; -impl Handler for Server { +impl Handler for ServerActor { type Result = (); #[allow(clippy::cognitive_complexity)] diff --git a/crates/server-actor/src/handlers/receive_valid_server_config.rs b/crates/server-actor/src/handlers/receive_valid_server_config.rs index f74d656..2f3bf10 100644 --- a/crates/server-actor/src/handlers/receive_valid_server_config.rs +++ b/crates/server-actor/src/handlers/receive_valid_server_config.rs @@ -3,13 +3,13 @@ use git_next_webhook_actor::{AddWebhookRecipient, ShutdownWebhook, WebhookActor, use crate::{ messages::{ReceiveValidServerConfig, ValidServerConfig}, - Server, + ServerActor, }; -impl Handler for Server { +impl Handler for ServerActor { type Result = (); - fn handle(&mut self, msg: ReceiveValidServerConfig, _ctx: &mut Self::Context) -> Self::Result { + fn handle(&mut self, msg: ReceiveValidServerConfig, ctx: &mut Self::Context) -> Self::Result { let ValidServerConfig { server_config, socket_address, @@ -23,11 +23,17 @@ impl Handler for Server { // Webhook Server tracing::info!("Starting Webhook Server..."); let webhook_router = WebhookRouter::default().start(); - let webhook = server_config.inbound_webhook(); + let inbound_webhook = server_config.inbound_webhook(); // Forge Actors for (forge_alias, forge_config) in server_config.forges() { let repo_actors = self - .create_forge_repos(forge_config, forge_alias.clone(), &server_storage, webhook) + .create_forge_repos( + forge_config, + forge_alias.clone(), + &server_storage, + inbound_webhook, + ctx.address().recipient(), + ) .into_iter() .map(|a| self.start_actor(a)) .collect::>(); @@ -47,6 +53,7 @@ impl Handler for Server { }); } let webhook = WebhookActor::new(socket_address, webhook_router.recipient()).start(); + self.server_config.replace(server_config); self.webhook.replace(webhook); } } diff --git a/crates/server-actor/src/handlers/shutdown.rs b/crates/server-actor/src/handlers/shutdown.rs index 652a4fd..f046037 100644 --- a/crates/server-actor/src/handlers/shutdown.rs +++ b/crates/server-actor/src/handlers/shutdown.rs @@ -2,9 +2,9 @@ use actix::prelude::*; use git_next_webhook_actor::ShutdownWebhook; -use crate::{messages::Shutdown, Server}; +use crate::{messages::Shutdown, ServerActor}; -impl Handler for Server { +impl Handler for ServerActor { type Result = (); fn handle(&mut self, _msg: Shutdown, _ctx: &mut Self::Context) -> Self::Result { diff --git a/crates/server-actor/src/lib.rs b/crates/server-actor/src/lib.rs index 47bae97..7d743bd 100644 --- a/crates/server-actor/src/lib.rs +++ b/crates/server-actor/src/lib.rs @@ -10,6 +10,7 @@ use git_next_config as config; use git_next_config::server::{InboundWebhook, ServerConfig, ServerStorage}; use git_next_config::{ForgeAlias, ForgeConfig, GitDir, RepoAlias, ServerRepoConfig}; use git_next_git::{Generation, RepoDetails}; +use git_next_repo_actor::messages::NotifyUser; use git_next_repo_actor::{messages::CloneRepo, RepoActor}; use git_next_webhook_actor as webhook; use kxio::{fs::FileSystem, network::Network}; @@ -43,7 +44,8 @@ type Result = core::result::Result; #[derive(derive_with::With)] #[with(message_log)] -pub struct Server { +pub struct ServerActor { + server_config: Option, generation: Generation, webhook: Option>, fs: FileSystem, @@ -55,11 +57,11 @@ pub struct Server { // testing message_log: Option>>>, } -impl Actor for Server { +impl Actor for ServerActor { type Context = Context; } -impl Server { +impl ServerActor { pub fn new( fs: FileSystem, net: Network, @@ -68,6 +70,7 @@ impl Server { ) -> Self { let generation = Generation::default(); Self { + server_config: None, generation, webhook: None, fs, @@ -105,6 +108,7 @@ impl Server { forge_name: ForgeAlias, server_storage: &ServerStorage, webhook: &InboundWebhook, + notify_user_recipient: Recipient, ) -> Vec<(ForgeAlias, RepoAlias, RepoActor)> { let span = tracing::info_span!("create_forge_repos", name = %forge_name, config = %forge_config); @@ -114,7 +118,11 @@ impl Server { let mut repos = vec![]; let creator = self.create_actor(forge_name, forge_config.clone(), server_storage, webhook); for (repo_alias, server_repo_config) in forge_config.repos() { - let forge_repo = creator((repo_alias, server_repo_config)); + let forge_repo = creator(( + repo_alias, + server_repo_config, + notify_user_recipient.clone(), + )); info!( alias = %forge_repo.1, "Created Repo" @@ -130,14 +138,17 @@ impl Server { forge_config: ForgeConfig, server_storage: &ServerStorage, webhook: &InboundWebhook, - ) -> impl Fn((RepoAlias, &ServerRepoConfig)) -> (ForgeAlias, RepoAlias, RepoActor) { + ) -> impl Fn( + (RepoAlias, &ServerRepoConfig, Recipient), + ) -> (ForgeAlias, RepoAlias, RepoActor) { let server_storage = server_storage.clone(); let webhook = webhook.clone(); let net = self.net.clone(); let repository_factory = self.repository_factory.duplicate(); let generation = self.generation; let sleep_duration = self.sleep_duration; - move |(repo_alias, server_repo_config)| { + // let notify_user_recipient = server_addr.recipient(); + move |(repo_alias, server_repo_config, notify_user_recipient)| { let span = tracing::info_span!("create_actor", alias = %repo_alias, config = %server_repo_config); let _guard = span.enter(); info!("Creating Repo"); @@ -173,6 +184,7 @@ impl Server { net.clone(), repository_factory.duplicate(), sleep_duration, + Some(notify_user_recipient), ); (forge_name.clone(), repo_alias, actor) } diff --git a/crates/server-actor/src/tests/receive_server_config.rs b/crates/server-actor/src/tests/receive_server_config.rs index de3f483..c6164f0 100644 --- a/crates/server-actor/src/tests/receive_server_config.rs +++ b/crates/server-actor/src/tests/receive_server_config.rs @@ -1,5 +1,5 @@ // -use crate::{tests::given, ReceiveServerConfig, Server}; +use crate::{tests::given, ReceiveServerConfig, ServerActor}; use actix::prelude::*; use git_next_config::server::{Http, InboundWebhook, Notification, ServerConfig, ServerStorage}; use std::{ @@ -17,7 +17,7 @@ async fn when_webhook_url_has_trailing_slash_should_not_send() { let duration = std::time::Duration::from_millis(1); // sut - let server = Server::new(fs.clone(), net.into(), repo, duration); + let server = ServerActor::new(fs.clone(), net.into(), repo, duration); // collaborators let http = Http::new("0.0.0.0".to_string(), 80); diff --git a/crates/server/src/lib.rs b/crates/server/src/lib.rs index 2506ea4..ce967fe 100644 --- a/crates/server/src/lib.rs +++ b/crates/server/src/lib.rs @@ -1,7 +1,7 @@ // use actix::prelude::*; use git_next_file_watcher_actor::{FileUpdated, FileWatcher}; -use git_next_server_actor::Server; +use git_next_server_actor::ServerActor; use kxio::{fs::FileSystem, network::Network}; use std::path::PathBuf; use tracing::{error, info, level_filters::LevelFilter}; @@ -40,7 +40,7 @@ pub fn start( info!("Starting Server..."); let execution = async move { - let server = Server::new(fs.clone(), net.clone(), repo, sleep_duration).start(); + let server = ServerActor::new(fs.clone(), net.clone(), repo, sleep_duration).start(); server.do_send(FileUpdated); info!("Starting File Watcher..."); diff --git a/crates/webhook-actor/src/handlers/mod.rs b/crates/webhook-actor/src/handlers/mod.rs index 3fa9272..418e9f6 100644 --- a/crates/webhook-actor/src/handlers/mod.rs +++ b/crates/webhook-actor/src/handlers/mod.rs @@ -1,2 +1 @@ -mod notify_user; mod shutdown_webhook; diff --git a/crates/webhook-actor/src/messages.rs b/crates/webhook-actor/src/messages.rs index 82af9c1..97cf082 100644 --- a/crates/webhook-actor/src/messages.rs +++ b/crates/webhook-actor/src/messages.rs @@ -2,5 +2,3 @@ use git_next_actor_macros::message; message!(ShutdownWebhook: "Request to shutdown the Webhook actor"); - -message!(NotifyUser: String: "Request to send the message payload to the notification webhook");