Save data after changes in `SettingsPersister`

2 files changed, 73 insertions, 112 deletions

diff --git a/mullvad-daemon/src/settings.rs b/mullvad-daemon/src/settings.rs
index 51482de537..78ec0016dc 100644
--- a/mullvad-daemon/src/settings.rs
+++ b/mullvad-daemon/src/settings.rs
@@ -49,7 +49,9 @@ impl SettingsPersister {
 
         // Force IPv6 to be enabled on Android
         if cfg!(target_os = "android") {
-            let _ = settings.set_enable_ipv6(true);
+            if Self::update_field(&mut settings.tunnel_options.generic.enable_ipv6, true) {
+                let _ = settings.save();
+            }
         }
 
         SettingsPersister { settings }
@@ -161,61 +163,104 @@ impl SettingsPersister {
     /// Changes account number to the one given. Also saves the new settings to disk.
     /// The boolean in the Result indicates if the account token changed or not
     pub fn set_account_token(&mut self, account_token: Option<String>) -> Result<bool, Error> {
-        Ok(self.settings.set_account_token(account_token)?)
+        let should_save = self.settings.set_account_token(account_token);
+        self.update(should_save)
     }
 
     pub fn update_relay_settings(&mut self, update: RelaySettingsUpdate) -> Result<bool, Error> {
-        Ok(self.settings.update_relay_settings(update)?)
+        let should_save = self.settings.update_relay_settings(update);
+        self.update(should_save)
     }
 
     pub fn set_allow_lan(&mut self, allow_lan: bool) -> Result<bool, Error> {
-        Ok(self.settings.set_allow_lan(allow_lan)?)
+        let should_save = Self::update_field(&mut self.settings.allow_lan, allow_lan);
+        self.update(should_save)
     }
 
     pub fn set_block_when_disconnected(
         &mut self,
         block_when_disconnected: bool,
     ) -> Result<bool, Error> {
-        Ok(self
-            .settings
-            .set_block_when_disconnected(block_when_disconnected)?)
+        let should_save = Self::update_field(
+            &mut self.settings.block_when_disconnected,
+            block_when_disconnected,
+        );
+        self.update(should_save)
     }
 
     pub fn set_auto_connect(&mut self, auto_connect: bool) -> Result<bool, Error> {
-        Ok(self.settings.set_auto_connect(auto_connect)?)
+        let should_save = Self::update_field(&mut self.settings.auto_connect, auto_connect);
+        self.update(should_save)
     }
 
     pub fn set_openvpn_mssfix(&mut self, openvpn_mssfix: Option<u16>) -> Result<bool, Error> {
-        Ok(self.settings.set_openvpn_mssfix(openvpn_mssfix)?)
+        let should_save = Self::update_field(
+            &mut self.settings.tunnel_options.openvpn.mssfix,
+            openvpn_mssfix,
+        );
+        self.update(should_save)
     }
 
     pub fn set_enable_ipv6(&mut self, enable_ipv6: bool) -> Result<bool, Error> {
-        Ok(self.settings.set_enable_ipv6(enable_ipv6)?)
+        let should_save = Self::update_field(
+            &mut self.settings.tunnel_options.generic.enable_ipv6,
+            enable_ipv6,
+        );
+        self.update(should_save)
     }
 
     pub fn set_wireguard_mtu(&mut self, mtu: Option<u16>) -> Result<bool, Error> {
-        Ok(self.settings.set_wireguard_mtu(mtu)?)
+        let should_save = Self::update_field(&mut self.settings.tunnel_options.wireguard.mtu, mtu);
+        self.update(should_save)
     }
 
     pub fn set_wireguard_rotation_interval(
         &mut self,
         automatic_rotation: Option<u32>,
     ) -> Result<bool, Error> {
-        Ok(self
-            .settings
-            .set_wireguard_rotation_interval(automatic_rotation)?)
+        let should_save = Self::update_field(
+            &mut self.settings.tunnel_options.wireguard.automatic_rotation,
+            automatic_rotation,
+        );
+        self.update(should_save)
     }
 
-    pub fn set_show_beta_releases(&mut self, enabled: bool) -> Result<bool, Error> {
-        Ok(self.settings.set_show_beta_releases(enabled)?)
+    pub fn set_show_beta_releases(&mut self, show_beta_releases: bool) -> Result<bool, Error> {
+        let should_save = Self::update_field(
+            &mut self.settings.show_beta_releases,
+            Some(show_beta_releases),
+        );
+        self.update(should_save)
     }
 
     pub fn set_bridge_settings(&mut self, bridge_settings: BridgeSettings) -> Result<bool, Error> {
-        Ok(self.settings.set_bridge_settings(bridge_settings)?)
+        let should_save = Self::update_field(&mut self.settings.bridge_settings, bridge_settings);
+        self.update(should_save)
     }
 
     pub fn set_bridge_state(&mut self, bridge_state: BridgeState) -> Result<bool, Error> {
-        Ok(self.settings.set_bridge_state(bridge_state)?)
+        let should_save = self.settings.set_bridge_state(bridge_state);
+        self.update(should_save)
+    }
+
+    fn update_field<T: Eq>(field: &mut T, new_value: T) -> bool {
+        if *field != new_value {
+            *field = new_value;
+            true
+        } else {
+            false
+        }
+    }
+
+    fn update(&mut self, should_save: bool) -> Result<bool, Error> {
+        if should_save {
+            self.settings
+                .save()
+                .map(|_| true)
+                .map_err(Error::SettingsError)
+        } else {
+            Ok(false)
+        }
     }
 }
 
diff --git a/mullvad-types/src/settings/mod.rs b/mullvad-types/src/settings/mod.rs
index 6b5ae102cc..ff3b93e39b 100644
--- a/mullvad-types/src/settings/mod.rs
+++ b/mullvad-types/src/settings/mod.rs
@@ -98,7 +98,7 @@ impl Settings {
     }
 
     /// Serializes the settings and saves them to the file it was loaded from.
-    fn save(&self) -> Result<()> {
+    pub fn save(&self) -> Result<()> {
         let path = Self::get_settings_path()?;
 
         debug!("Writing settings to {}", path.display());
@@ -121,7 +121,7 @@ impl Settings {
 
     /// Changes account number to the one given. Also saves the new settings to disk.
     /// The boolean in the Result indicates if the account token changed or not
-    pub fn set_account_token(&mut self, mut account_token: Option<String>) -> Result<bool> {
+    pub fn set_account_token(&mut self, mut account_token: Option<String>) -> bool {
         if account_token.as_ref().map(String::len) == Some(0) {
             debug!("Setting empty account token is treated as unsetting it");
             account_token = None;
@@ -135,9 +135,9 @@ impl Settings {
                 info!("Changing account token")
             }
             self.account_token = account_token;
-            self.save().map(|_| true)
+            true
         } else {
-            Ok(false)
+            false
         }
     }
 
@@ -145,7 +145,7 @@ impl Settings {
         self.relay_settings.clone()
     }
 
-    pub fn update_relay_settings(&mut self, update: RelaySettingsUpdate) -> Result<bool> {
+    pub fn update_relay_settings(&mut self, update: RelaySettingsUpdate) -> bool {
         let update_supports_bridge = update.supports_bridge();
         let new_settings = self.relay_settings.merge(update);
         if self.relay_settings != new_settings {
@@ -158,93 +158,9 @@ impl Settings {
             );
 
             self.relay_settings = new_settings;
-            self.save().map(|_| true)
+            true
         } else {
-            Ok(false)
-        }
-    }
-
-    pub fn set_allow_lan(&mut self, allow_lan: bool) -> Result<bool> {
-        if allow_lan != self.allow_lan {
-            self.allow_lan = allow_lan;
-            self.save().map(|_| true)
-        } else {
-            Ok(false)
-        }
-    }
-
-    pub fn set_block_when_disconnected(&mut self, block_when_disconnected: bool) -> Result<bool> {
-        if block_when_disconnected != self.block_when_disconnected {
-            self.block_when_disconnected = block_when_disconnected;
-            self.save().map(|_| true)
-        } else {
-            Ok(false)
-        }
-    }
-
-    pub fn set_auto_connect(&mut self, auto_connect: bool) -> Result<bool> {
-        if auto_connect != self.auto_connect {
-            self.auto_connect = auto_connect;
-            self.save().map(|_| true)
-        } else {
-            Ok(false)
-        }
-    }
-
-    pub fn set_openvpn_mssfix(&mut self, openvpn_mssfix: Option<u16>) -> Result<bool> {
-        if self.tunnel_options.openvpn.mssfix != openvpn_mssfix {
-            self.tunnel_options.openvpn.mssfix = openvpn_mssfix;
-            self.save().map(|_| true)
-        } else {
-            Ok(false)
-        }
-    }
-
-    pub fn set_enable_ipv6(&mut self, enable_ipv6: bool) -> Result<bool> {
-        if self.tunnel_options.generic.enable_ipv6 != enable_ipv6 {
-            self.tunnel_options.generic.enable_ipv6 = enable_ipv6;
-            self.save().map(|_| true)
-        } else {
-            Ok(false)
-        }
-    }
-
-    pub fn set_wireguard_mtu(&mut self, mtu: Option<u16>) -> Result<bool> {
-        if self.tunnel_options.wireguard.mtu != mtu {
-            self.tunnel_options.wireguard.mtu = mtu;
-            self.save().map(|_| true)
-        } else {
-            Ok(false)
-        }
-    }
-
-    pub fn set_wireguard_rotation_interval(
-        &mut self,
-        automatic_rotation: Option<u32>,
-    ) -> Result<bool> {
-        if self.tunnel_options.wireguard.automatic_rotation != automatic_rotation {
-            self.tunnel_options.wireguard.automatic_rotation = automatic_rotation;
-            self.save().map(|_| true)
-        } else {
-            Ok(false)
-        }
-    }
-
-    pub fn set_show_beta_releases(&mut self, enabled: bool) -> Result<bool> {
-        if Some(enabled) != self.show_beta_releases {
-            self.show_beta_releases = Some(enabled);
-            self.save().map(|_| true)
-        } else {
-            Ok(false)
-        }
-    }
-
-    pub fn set_bridge_settings(&mut self, bridge_settings: BridgeSettings) -> Result<bool> {
-        if self.bridge_settings != bridge_settings {
-            self.bridge_settings = bridge_settings;
-            self.save().map(|_| true)
-        } else {
-            Ok(false)
+            false
         }
     }
 
@@ -252,15 +168,15 @@ impl Settings {
         &self.bridge_state
     }
 
-    pub fn set_bridge_state(&mut self, bridge_state: BridgeState) -> Result<bool> {
+    pub fn set_bridge_state(&mut self, bridge_state: BridgeState) -> bool {
         if self.bridge_state != bridge_state {
             self.bridge_state = bridge_state;
             if self.bridge_state == BridgeState::On {
                 self.relay_settings.ensure_bridge_compatibility();
             }
-            self.save().map(|_| true)
+            true
         } else {
-            Ok(false)
+            false
         }
     }
 }