Do retry update after a failure & fix tests (#94)

* Do retry update after a failure & fix tests

* Fix formatting

* Fix clippy errors

* Add codecov ignore for ip_source files

.clippy.toml

@@ -0,0 +1,3 @@
+# assert_eq!(..., true) or false is a lot clearer when testing functions that
+# return booleans.
+bool_assert_comparison = false

.codecov.yml

@@ -0,0 +1,4 @@
+ignore:
+  # These are tested, but the tests hit external services which is not
+  # necessarily smart to do in CI, so they get skipped.
+  - "src/ip_source"

Cargo.lock

@@ -663,6 +663,7 @@ dependencies = [
  "governor",
  "httpmock",
  "json",
+ "lazy_static",
  "regex",
  "reqwest",
  "serde",

Cargo.toml

@@ -38,6 +38,7 @@ thiserror = "1.0.38"
 [dev-dependencies]
 httpmock = "0.6"
 regex = "1.6"
+lazy_static = "1.4.0"
 
 [dev-dependencies.die-exit]
 version = "0.4"

src/ip_source/icanhazip.rs

@@ -2,7 +2,7 @@ use async_trait::async_trait;
 
 use crate::ClientError;
 
-use super::ip_source::IPSource;
+use super::common::IPSource;
 
 pub(crate) struct IPSourceIcanhazip;
 
@@ -34,7 +34,7 @@ impl IPSource for IPSourceIcanhazip {
 mod tests {
     use regex::Regex;
 
-    use crate::ip_source::ip_source::IPSource;
+    use crate::ip_source::common::IPSource;
 
     use super::IPSourceIcanhazip;
 

src/ip_source/ipify.rs

@@ -2,7 +2,7 @@ use async_trait::async_trait;
 
 use crate::ClientError;
 
-use super::ip_source::IPSource;
+use super::common::IPSource;
 
 pub(crate) struct IPSourceIpify;
 

src/ip_source/mod.rs

@@ -1,4 +1,4 @@
+pub(crate) mod common;
 pub(crate) mod icanhazip;
-pub(crate) mod ip_source;
 pub(crate) mod ipify;
 pub(crate) mod seeip;

src/ip_source/seeip.rs

@@ -2,7 +2,7 @@ use async_trait::async_trait;
 
 use crate::ClientError;
 
-use super::ip_source::IPSource;
+use super::common::IPSource;
 
 pub(crate) struct IPSourceSeeIP;
 

src/main.rs

@@ -1,6 +1,6 @@
 use crate::config::Config;
 use crate::gandi::GandiAPI;
-use crate::ip_source::{ip_source::IPSource, ipify::IPSourceIpify};
+use crate::ip_source::{common::IPSource, ipify::IPSourceIpify};
 use clap::Parser;
 use config::{ConfigError, IPSourceName};
 use ip_source::icanhazip::IPSourceIcanhazip;
@@ -57,7 +57,7 @@ pub enum ApiError {
 fn api_client(api_key: &str) -> Result<Client, ClientError> {
     let client_builder = ClientBuilder::new();
 
-    let key = format!("Apikey {}", api_key);
+    let key = format!("Apikey {api_key}");
     let mut auth_value = header::HeaderValue::from_str(&key)?;
     let mut headers = header::HeaderMap::new();
     auth_value.set_sensitive(true);
@@ -82,6 +82,9 @@ struct ResponseFeedback {
 }
 
 #[derive(Deserialize)]
+// Allowing dead code because this is the API response we get from Gandi.
+// We don't necessarily need all the fields, but we get them anyway.
+#[allow(dead_code)]
 struct ApiResponse {
     message: String,
     cause: Option<String>,
@@ -105,12 +108,12 @@ async fn run(
         let ipv6 = ipv6_result.as_ref();
         println!("Found these:");
         match ipv4 {
-            Ok(ip) => println!("\tIPv4: {}", ip),
-            Err(err) => eprintln!("\tIPv4 failed: {}", err),
+            Ok(ip) => println!("\tIPv4: {ip}"),
+            Err(err) => eprintln!("\tIPv4 failed: {err}"),
         }
         match ipv6 {
-            Ok(ip) => println!("\tIPv6: {}", ip),
-            Err(err) => eprintln!("\tIPv6 failed: {}", err),
+            Ok(ip) => println!("\tIPv6: {ip}"),
+            Err(err) => eprintln!("\tIPv6 failed: {err}"),
         }
 
         let ipv4_same = last_ipv4
@@ -122,9 +125,6 @@ async fn run(
             .map(|p| ipv6.map(|q| p == q).unwrap_or(false))
             .unwrap_or(false);
 
-        last_ipv4 = ipv4.ok().map(|v| v.to_string());
-        last_ipv6 = ipv6.ok().map(|v| v.to_string());
-
         if !ipv4_same || !ipv6_same || conf.always_update {
             let client = api_client(&conf.api_key)?;
             let mut tasks: Vec<JoinHandle<Result<ResponseFeedback, ClientError>>> = Vec::new();
@@ -229,11 +229,11 @@ async fn run(
                     .filter(|item| item.response.is_ok())
                     .count()
             );
-            for item in results {
+            for item in &results {
                 match item {
                     Ok(value) => println!(
                         "{}",
-                        match value.response {
+                        match &value.response {
                             Ok(val) => format!(
                                 "Record '{}' ({}): {}",
                                 value.entry_name, value.entry_type, val
@@ -244,9 +244,21 @@ async fn run(
                             ),
                         }
                     ),
-                    Err(err) => println!("{}", err),
+                    Err(err) => println!("{err}"),
                 }
             }
+            if results
+                .iter()
+                // all tasks finished OK, and all responses were OK as well
+                .all(|result| result.as_ref().map(|v| v.response.is_ok()).unwrap_or(false))
+            {
+                // Only then we update the last seen IP, because we want to
+                // retry updates in case the last update just happened to fail
+                last_ipv4 = ipv4.ok().map(|v| v.to_string());
+                last_ipv6 = ipv6.ok().map(|v| v.to_string());
+            } else if opts.repeat.is_some() {
+                println!("Some operations failed. They will be retried during the next repeat.")
+            }
         } else {
             println!("IP address has not changed since last update");
         }
@@ -280,11 +292,15 @@ async fn main() -> anyhow::Result<()> {
 
 #[cfg(test)]
 mod tests {
-    use std::{env::temp_dir, time::Duration};
-
-    use crate::{config, ip_source::ip_source::IPSource, opts::Opts, run, ClientError};
+    use crate::{config, ip_source::common::IPSource, opts::Opts, run, ClientError};
     use async_trait::async_trait;
     use httpmock::MockServer;
+    use lazy_static::lazy_static;
+    use std::{
+        env::temp_dir,
+        sync::atomic::{AtomicBool, Ordering::SeqCst},
+        time::Duration,
+    };
     use tokio::{fs, task::LocalSet, time::sleep};
 
     struct IPSourceMock;
@@ -322,7 +338,8 @@ mod tests {
                     "/v5/livedns/domains/{fqdn}/records/{rname}/{rtype}"
                 ))
                 .body_contains("192.168.0.0");
-            then.status(200);
+            then.status(201)
+                .body("{\"cause\":\"\", \"code\":201, \"message\":\"\", \"object\":\"\"}");
         });
 
         let opts = Opts {
@@ -369,7 +386,8 @@ mod tests {
                         "/v5/livedns/domains/{fqdn}/records/{rname}/{rtype}"
                     ))
                     .body_contains("192.168.0.0");
-                then.status(200);
+                then.status(201)
+                    .body("{\"cause\":\"\", \"code\":201, \"message\":\"\", \"object\":\"\"}");
             });
 
             let server_url = server.base_url();
@@ -394,6 +412,85 @@ mod tests {
         });
     }
 
+    #[test]
+    fn repeat_with_failure() {
+        let runtime = tokio::runtime::Builder::new_current_thread()
+            .enable_all()
+            .build()
+            .unwrap();
+
+        LocalSet::new().block_on(&runtime, async {
+            let mut temp = temp_dir().join("gandi-live-dns-test");
+            fs::create_dir_all(&temp)
+                .await
+                .expect("Failed to create test dir");
+            temp.push("test.toml");
+            fs::write(
+                &temp,
+                "fqdn = \"example.com\"\napi_key = \"xxx\"\nttl = 300\n[[entry]]\nname =\"@\"\n",
+            )
+            .await
+            .expect("Failed to write test config file");
+
+            let fqdn = "example.com";
+            let rname = "@";
+            let rtype = "A";
+            let server = MockServer::start();
+            let mock = server.mock(|when, then| {
+                when.method("PUT")
+                    .path(format!(
+                        "/v5/livedns/domains/{fqdn}/records/{rname}/{rtype}"
+                    ))
+                    .body_contains("192.168.0.0")
+                    .matches(|_| {
+                        // Don't match during the first call, but do during the second call
+                        lazy_static! {
+                            static ref FIRST_CALL: AtomicBool = AtomicBool::new(true);
+                        }
+                        if FIRST_CALL.load(SeqCst) {
+                            FIRST_CALL.store(false, SeqCst);
+                            return true;
+                        }
+                        false
+                    });
+                then.status(500)
+                    .body("{\"cause\":\"\", \"code\":500, \"message\":\"Something went wrong\", \"object\":\"\"}");
+            });
+            let mock_fail = server.mock(|when, then| {
+                when.method("PUT")
+                    .path(format!(
+                        "/v5/livedns/domains/{fqdn}/records/{rname}/{rtype}"
+                    ))
+                    .body_contains("192.168.0.0");
+                then.status(201)
+                    .body("{\"cause\":\"\", \"code\":201, \"message\":\"\", \"object\":\"\"}");
+            });
+
+            let server_url = server.base_url();
+            let handle = tokio::task::spawn_local(async move {
+                let opts = Opts {
+                    config: Some(temp.to_string_lossy().to_string()),
+                    repeat: Some(1),
+                    ..Opts::default()
+                };
+                let conf = config::load_config(&opts).expect("Failed to load config");
+                let ip_source: Box<dyn IPSource> = Box::new(IPSourceMock);
+                run(&server_url, &ip_source, &conf, &opts)
+                    .await
+                    .expect("Failed when running the update");
+            });
+
+            sleep(Duration::from_secs(4)).await;
+            handle.abort();
+
+            // The first call failed
+            mock_fail.assert();
+            // We then retried since the first call failed. The retry succeeds
+            // so we don't retry again.
+            mock.assert();
+        });
+    }
+
     #[test]
     fn repeat_always_update() {
         let runtime = tokio::runtime::Builder::new_current_thread()
@@ -424,7 +521,7 @@ mod tests {
                         "/v5/livedns/domains/{fqdn}/records/{rname}/{rtype}"
                     ))
                     .body_contains("192.168.0.0");
-                then.status(200);
+                then.status(201).body("{\"cause\":\"\", \"code\":201, \"message\":\"\", \"object\":\"\"}");
             });
 
             let server_url = server.base_url();