feat: log as an error when webhook url ends with a slash

Closes kemitix/git-next#87

crates/server-actor/Cargo.toml

@@ -20,6 +20,7 @@ kxio = { workspace = true }
 
 # boilerplate
 derive_more = { workspace = true }
+derive-with = { workspace = true }
 
 # Actors
 actix = { workspace = true }
@@ -27,6 +28,8 @@ actix = { workspace = true }
 [dev-dependencies]
 # Testing
 # assert2 = { workspace = true }
+test-log = { workspace = true }
+tokio = { workspace = true }
 
 [lints.clippy]
 nursery = { level = "warn", priority = -1 }

crates/server-actor/src/lib.rs

@@ -1,4 +1,7 @@
 //
+#[cfg(test)]
+mod tests;
+
 use actix::prelude::*;
 use derive_more::Constructor;
 use git_next_actor_macros::message;
@@ -10,7 +13,11 @@ use git_next_git::{Generation, RepoDetails};
 use git_next_repo_actor::{messages::CloneRepo, RepoActor};
 use git_next_webhook_actor as webhook;
 use kxio::{fs::FileSystem, network::Network};
-use std::{net::SocketAddr, path::PathBuf};
+use std::{
+    net::SocketAddr,
+    path::PathBuf,
+    sync::{Arc, RwLock},
+};
 use tracing::{error, info};
 use webhook::{AddWebhookRecipient, ShutdownWebhook, WebhookActor, WebhookRouter};
 
@@ -47,6 +54,8 @@ pub enum Error {
 }
 type Result<T> = core::result::Result<T, Error>;
 
+#[derive(derive_with::With)]
+#[with(message_log)]
 pub struct Server {
     generation: Generation,
     webhook: Option<Addr<WebhookActor>>,
@@ -54,6 +63,9 @@ pub struct Server {
     net: Network,
     repository_factory: Box<dyn RepositoryFactory>,
     sleep_duration: std::time::Duration,
+
+    // testing
+    message_log: Option<Arc<RwLock<Vec<String>>>>,
 }
 impl Actor for Server {
     type Context = Context<Self>;
@@ -69,13 +81,15 @@ impl Handler<FileUpdated> for Server {
                 return;
             }
         };
-        ctx.notify(ReceiveServerConfig::new(server_config));
+        self.do_send(ReceiveServerConfig::new(server_config), ctx);
     }
 }
 impl Handler<ReceiveServerConfig> for Server {
     type Result = ();
 
+    #[allow(clippy::cognitive_complexity)]
     fn handle(&mut self, msg: ReceiveServerConfig, ctx: &mut Self::Context) -> Self::Result {
+        tracing::info!("recieved server config");
         let Ok(socket_addr) = msg.http() else {
             error!("Unable to parse http.addr");
             return;
@@ -86,12 +100,19 @@ impl Handler<ReceiveServerConfig> for Server {
             return;
         };
 
-        ctx.address()
+        if msg.webhook().base_url().ends_with('/') {
-            .do_send(ReceiveValidServerConfig::new(ValidServerConfig::new(
+            error!("webhook.url must not end with a '/'");
+            return;
+        }
+
+        self.do_send(
+            ReceiveValidServerConfig::new(ValidServerConfig::new(
                 msg.0,
                 socket_addr,
                 server_storage,
-            )));
+            )),
+            ctx,
+        );
     }
 }
 impl Handler<ReceiveValidServerConfig> for Server {
@@ -140,6 +161,7 @@ impl Server {
             net,
             repository_factory: repo,
             sleep_duration,
+            message_log: None,
         }
     }
     fn create_forge_data_directories(
@@ -274,4 +296,23 @@ impl Server {
         }
         Some(server_storage)
     }
+
+    fn do_send<M>(&mut self, msg: M, _ctx: &mut <Self as actix::Actor>::Context)
+    where
+        M: actix::Message + Send + 'static + std::fmt::Debug,
+        Self: actix::Handler<M>,
+        <M as actix::Message>::Result: Send,
+    {
+        tracing::info!(?msg, "send");
+        if let Some(message_log) = &self.message_log {
+            let log_message = format!("send: {:?}", msg);
+            tracing::debug!(log_message);
+            if let Ok(mut log) = message_log.write() {
+                log.push(log_message);
+            }
+        }
+        #[cfg(not(test))]
+        _ctx.address().do_send(msg);
+        tracing::info!("sent");
+    }
 }

crates/server-actor/src/tests/given.rs

@@ -0,0 +1,7 @@
+pub fn a_filesystem() -> kxio::fs::FileSystem {
+    kxio::fs::temp().unwrap_or_else(|e| panic!("{}", e))
+}
+
+pub fn a_network() -> kxio::network::MockNetwork {
+    kxio::network::MockNetwork::new()
+}

crates/server-actor/src/tests/mod.rs

@@ -0,0 +1,3 @@
+mod receive_server_config;
+
+mod given;

crates/server-actor/src/tests/receive_server_config.rs

@@ -0,0 +1,49 @@
+//
+use crate::{tests::given, ReceiveServerConfig, Server};
+use actix::prelude::*;
+use git_next_config::server::{Http, ServerConfig, ServerStorage, Webhook};
+use std::{
+    collections::BTreeMap,
+    sync::{Arc, RwLock},
+};
+
+#[test_log::test(actix::test)]
+async fn when_webhook_url_has_trailing_slash_should_not_send() {
+    //given
+    // parameters
+    let fs = given::a_filesystem();
+    let net = given::a_network();
+    let repo = git_next_git::repository::factory::mock();
+    let duration = std::time::Duration::from_millis(1);
+
+    // sut
+    let server = Server::new(fs.clone(), net.into(), repo, duration);
+
+    // collaborators
+    let http = Http::new("0.0.0.0".to_string(), 80);
+    let webhook = Webhook::new("http://localhost/".to_string()); // With trailing slash
+    let server_storage = ServerStorage::new((fs.base()).to_path_buf());
+    let repos = BTreeMap::default();
+
+    // debugging
+    let message_log: Arc<RwLock<Vec<String>>> = Arc::new(RwLock::new(vec![]));
+    let server = server.with_message_log(Some(message_log.clone()));
+
+    //when
+    server
+        .start()
+        .do_send(ReceiveServerConfig::new(ServerConfig::new(
+            http,
+            webhook,
+            server_storage,
+            repos,
+        )));
+    tokio::time::sleep(std::time::Duration::from_millis(1)).await;
+
+    //then
+    // INFO: assert that ReceiveValidServerConfig is NOT sent
+    tracing::debug!(?message_log, "");
+    assert!(message_log.read().iter().any(|log| !log
+        .iter()
+        .any(|line| line != "send: ReceiveValidServerConfig")));
+}