refactor: repo-actor: RepoActorLog: replace Mutex with RwLock

crates/repo-actor/src/lib.rs

@@ -16,9 +16,9 @@ use kxio::network::Network;
 use tracing::{info, warn, Instrument};
 
 #[derive(Clone, Debug, Default)]
-pub struct RepoActorLog(std::sync::Arc<std::sync::Mutex<Vec<String>>>);
+pub struct RepoActorLog(std::sync::Arc<std::sync::RwLock<Vec<String>>>);
 impl Deref for RepoActorLog {
-    type Target = std::sync::Arc<std::sync::Mutex<Vec<String>>>;
+    type Target = std::sync::Arc<std::sync::RwLock<Vec<String>>>;
 
     fn deref(&self) -> &Self::Target {
         &self.0
@@ -135,6 +135,6 @@ pub fn logger(log: &Option<crate::RepoActorLog>, message: impl Into<String>) {
     if let Some(log) = log {
         let message: String = message.into();
         tracing::debug!(message);
-        let _ = log.lock().map(|mut l| l.push(message));
+        let _ = log.write().map(|mut l| l.push(message));
     }
 }

crates/repo-actor/src/tests/handlers/advance_main.rs

@@ -38,7 +38,7 @@ async fn when_repo_config_should_fetch_then_push_then_revalidate() -> TestResult
 
     //then
     tracing::debug!(?log, "");
-    log.lock().map_err(|e| e.to_string()).map(|l| {
+    log.read().map_err(|e| e.to_string()).map(|l| {
         assert!(l
             .iter()
             .any(|message| message.contains("send: LoadConfigFromRepo")))
@@ -83,7 +83,7 @@ async fn when_server_config_should_fetch_then_push_then_revalidate() -> TestResu
 
     //then
     tracing::debug!(?log, "");
-    log.lock().map_err(|e| e.to_string()).map(|l| {
+    log.read().map_err(|e| e.to_string()).map(|l| {
         assert!(l
             .iter()
             .any(|message| message.contains("send: ValidateRepo")))

crates/repo-actor/src/tests/handlers/advance_next.rs

@@ -40,7 +40,7 @@ async fn should_fetch_then_push_then_revalidate() -> TestResult {
 
     //then
     tracing::debug!(?log, "");
-    log.lock().map_err(|e| e.to_string()).map(|l| {
+    log.read().map_err(|e| e.to_string()).map(|l| {
         assert!(l
             .iter()
             .any(|message| message.contains("send: ValidateRepo")))

crates/repo-actor/src/tests/handlers/check_ci_status.rs

@@ -26,7 +26,7 @@ async fn should_passthrough_to_receive_ci_status() -> TestResult {
 
     //then
     tracing::debug!(?log, "");
-    log.lock().map_err(|e| e.to_string()).map(|l| {
+    log.read().map_err(|e| e.to_string()).map(|l| {
         assert!(l
             .iter()
             .any(|message| message.contains("send: ReceiveCIStatus")))

crates/repo-actor/src/tests/handlers/clone_repo.rs

@@ -231,7 +231,7 @@ async fn valid_repo_with_default_remotes_should_assess_branch_positions() -> Tes
     System::current().stop();
 
     //then
-    log.lock()
+    log.read()
         .map_err(|e| e.to_string())
         .map(|l| assert!(l.iter().any(|l| l.contains("send: ValidateRepo"))))?;
 

crates/repo-actor/src/tests/handlers/load_config_from_repo.rs

@@ -45,7 +45,7 @@ async fn when_read_file_ok_should_send_config_loaded() -> TestResult {
 
     //then
     tracing::debug!(?log, "");
-    log.lock().map_err(|e| e.to_string()).map(|l| {
+    log.read().map_err(|e| e.to_string()).map(|l| {
         assert!(l
             .iter()
             .any(|message| message.contains("send: LoadedConfig")))
@@ -80,7 +80,7 @@ async fn when_read_file_err_should_notify_user() -> TestResult {
 
     //then
     tracing::debug!(?log, "");
-    log.lock().map_err(|e| e.to_string()).map(|l| {
+    log.read().map_err(|e| e.to_string()).map(|l| {
         assert!(l.iter().any(|message| message.contains("send: NotifyUser")));
         assert!(
             !l.iter()

crates/repo-actor/src/tests/handlers/loaded_config.rs

@@ -54,7 +54,7 @@ async fn should_validate_repo() -> TestResult {
 
     //then
     tracing::debug!(?log, "");
-    log.lock().map_err(|e| e.to_string()).map(|l| {
+    log.read().map_err(|e| e.to_string()).map(|l| {
         assert!(l
             .iter()
             .any(|message| message.contains("send: ValidateRepo")))
@@ -84,7 +84,7 @@ async fn should_register_webhook() -> TestResult {
 
     //then
     tracing::debug!(?log, "");
-    log.lock().map_err(|e| e.to_string()).map(|l| {
+    log.read().map_err(|e| e.to_string()).map(|l| {
         assert!(l
             .iter()
             .any(|message| message.contains("send: RegisterWebhook")))

crates/repo-actor/src/tests/handlers/receive_ci_status.rs

@@ -23,7 +23,7 @@ async fn when_pass_should_advance_main_to_next() -> TestResult {
 
     //then
     tracing::debug!(?log, "");
-    log.lock().map_err(|e| e.to_string()).map(|l| {
+    log.read().map_err(|e| e.to_string()).map(|l| {
         let expected = format!("send: AdvanceMain({:?})", next_commit);
         tracing::debug!(%expected,"");
         assert!(l.iter().any(|message| message.contains(&expected)))
@@ -53,7 +53,7 @@ async fn when_pending_should_recheck_ci_status() -> TestResult {
 
     //then
     tracing::debug!(?log, "");
-    log.lock().map_err(|e| e.to_string()).map(|l| {
+    log.read().map_err(|e| e.to_string()).map(|l| {
         assert!(l
             .iter()
             .any(|message| message.contains("send: ValidateRepo")))
@@ -84,7 +84,7 @@ async fn when_fail_should_notify_user() -> TestResult {
 
     //then
     tracing::debug!(?log, "");
-    log.lock()
+    log.read()
         .map_err(|e| e.to_string())
         .map(|l| assert!(l.iter().any(|message| message.contains("send: NotifyUser"))))?;
     Ok(())

crates/repo-actor/src/tests/handlers/register_webhook.rs

@@ -27,7 +27,7 @@ async fn when_registered_ok_should_send_webhook_registered() -> TestResult {
 
     //then
     tracing::debug!(?log, "");
-    log.lock().map_err(|e| e.to_string()).map(|l| {
+    log.read().map_err(|e| e.to_string()).map(|l| {
         assert!(l
             .iter()
             .any(|message| message.contains("send: WebhookRegistered")))
@@ -63,7 +63,7 @@ async fn when_registered_error_should_send_notify_user() -> TestResult {
 
     //then
     tracing::debug!(?log, "");
-    log.lock()
+    log.read()
         .map_err(|e| e.to_string())
         .map(|l| assert!(l.iter().any(|message| message.contains("send: NotifyUser"))))?;
     Ok(())

crates/repo-actor/src/tests/handlers/validate_repo.rs

@@ -44,7 +44,7 @@ async fn repo_with_next_not_an_ancestor_of_dev_should_be_reset() -> TestResult {
 
     //then
     tracing::debug!(?log, "");
-    log.lock().map_err(|e| e.to_string()).map(|l| {
+    log.read().map_err(|e| e.to_string()).map(|l| {
         assert!(l
             .iter()
             .any(|message| message.contains("NextBranchResetRequired")))
@@ -95,7 +95,7 @@ async fn repo_with_next_not_on_or_near_main_should_be_reset() -> TestResult {
 
     //then
     tracing::debug!(?log, "");
-    log.lock().map_err(|e| e.to_string()).map(|l| {
+    log.read().map_err(|e| e.to_string()).map(|l| {
         assert!(l
             .iter()
             .any(|message| message.contains("NextBranchResetRequired")))
@@ -146,7 +146,7 @@ async fn repo_with_next_not_based_on_main_should_be_reset() -> TestResult {
 
     //then
     tracing::debug!(?log, "");
-    log.lock().map_err(|e| e.to_string()).map(|l| {
+    log.read().map_err(|e| e.to_string()).map(|l| {
         assert!(l
             .iter()
             .any(|message| message.contains("NextBranchResetRequired")))
@@ -196,7 +196,7 @@ async fn repo_with_next_ahead_of_main_should_check_ci_status() -> TestResult {
 
     //then
     tracing::debug!(?log, "");
-    log.lock().map_err(|e| e.to_string()).map(|l| {
+    log.read().map_err(|e| e.to_string()).map(|l| {
         let expected = format!("send: CheckCIStatus({next_commit:?})");
         assert!(l.iter().any(|message| message.contains(&expected)))
     })?;
@@ -246,7 +246,7 @@ async fn repo_with_dev_and_next_on_main_should_do_nothing() -> TestResult {
 
     //then
     tracing::debug!(?log, "");
-    log.lock()
+    log.read()
         .map_err(|e| e.to_string())
         .map(|l| assert!(!l.iter().any(|message| message.contains("send:"))))?;
     Ok(())
@@ -296,7 +296,7 @@ async fn repo_with_dev_ahead_of_next_should_advance_next() -> TestResult {
 
     //then
     tracing::debug!(?log, "");
-    log.lock().map_err(|e| e.to_string()).map(|l| {
+    log.read().map_err(|e| e.to_string()).map(|l| {
         let expected = format!("send: AdvanceNext(({next_commit:?}, {dev_branch_log:?}))");
         assert!(l.iter().any(|message| message.contains(&expected)))
     })?;
@@ -326,7 +326,7 @@ async fn should_accept_message_with_current_token() -> TestResult {
 
     //then
     tracing::debug!(?log, "");
-    log.lock().map_err(|e| e.to_string()).map(|l| {
+    log.read().map_err(|e| e.to_string()).map(|l| {
         assert!(l
             .iter()
             .any(|message| message.contains("accepted token: 2")))
@@ -355,7 +355,7 @@ async fn should_accept_message_with_new_token() -> TestResult {
 
     //then
     tracing::debug!(?log, "");
-    log.lock().map_err(|e| e.to_string()).map(|l| {
+    log.read().map_err(|e| e.to_string()).map(|l| {
         assert!(l
             .iter()
             .any(|message| message.contains("accepted token: 3")))
@@ -384,7 +384,7 @@ async fn should_reject_message_with_expired_token() -> TestResult {
 
     //then
     tracing::debug!(?log, "");
-    log.lock()
+    log.read()
         .map_err(|e| e.to_string())
         .map(|l| assert!(!l.iter().any(|message| message.contains("accepted token"))))?;
     Ok(())

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

@@ -59,7 +59,7 @@ impl RepoActorLog {
         needle: impl AsRef<str>,
     ) -> Result<bool, Box<dyn std::error::Error>> {
         let found = self
-            .lock()
+            .read()
             .map_err(|e| e.to_string())?
             .iter()
             .any(|message| message.contains(needle.as_ref()));