Remove settings migration trait

5 files changed, 216 insertions, 238 deletions

diff --git a/mullvad-daemon/src/migrations/mod.rs b/mullvad-daemon/src/migrations/mod.rs
index e8f432a63f..f2dc225ab4 100644
--- a/mullvad-daemon/src/migrations/mod.rs
+++ b/mullvad-daemon/src/migrations/mod.rs
@@ -40,11 +40,6 @@ pub enum Error {
 pub type Result<T> = std::result::Result<T, Error>;
 
 
-trait SettingsMigration {
-    fn version_matches(&self, settings: &mut serde_json::Value) -> bool;
-    fn migrate(&self, settings: &mut serde_json::Value) -> Result<()>;
-}
-
 pub async fn migrate_all(cache_dir: &Path, settings_dir: &Path) -> Result<()> {
     #[cfg(windows)]
     windows::migrate_after_windows_update(settings_dir)
@@ -76,19 +71,10 @@ pub async fn migrate_all(cache_dir: &Path, settings_dir: &Path) -> Result<()> {
         }
     }
 
-    let migrations: Vec<Box<dyn SettingsMigration>> = vec![
-        Box::new(v1::Migration),
-        Box::new(v2::Migration),
-        Box::new(v3::Migration),
-        Box::new(v4::Migration),
-    ];
-
-    for migration in &migrations {
-        if !migration.version_matches(&mut settings) {
-            continue;
-        }
-        migration.migrate(&mut settings)?;
-    }
+    v1::migrate(&mut settings)?;
+    v2::migrate(&mut settings)?;
+    v3::migrate(&mut settings)?;
+    v4::migrate(&mut settings)?;
 
     let buffer = serde_json::to_string_pretty(&settings).map_err(Error::SerializeError)?;
 
diff --git a/mullvad-daemon/src/migrations/v1.rs b/mullvad-daemon/src/migrations/v1.rs
index a7c8e51738..d04812cd00 100644
--- a/mullvad-daemon/src/migrations/v1.rs
+++ b/mullvad-daemon/src/migrations/v1.rs
@@ -3,65 +3,65 @@ use mullvad_types::{relay_constraints::Constraint, settings::SettingsVersion};
 use talpid_types::net::TunnelType;
 
 
-pub(super) struct Migration;
-
-impl super::SettingsMigration for Migration {
-    fn version_matches(&self, settings: &mut serde_json::Value) -> bool {
-        settings.get("settings_version").is_none()
+pub fn migrate(settings: &mut serde_json::Value) -> Result<()> {
+    if !version_matches(settings) {
+        return Ok(());
     }
 
-    fn migrate(&self, settings: &mut serde_json::Value) -> Result<()> {
-        log::info!("Migrating settings format to V2");
+    log::info!("Migrating settings format to V2");
 
-        let openvpn_constraints = || -> Option<serde_json::Value> {
-            settings
-                .get("relay_settings")?
-                .get("normal")?
-                .get("tunnel")?
-                .get("only")?
-                .get("openvpn")
-                .cloned()
-        }();
-        let wireguard_constraints = || -> Option<serde_json::Value> {
-            settings
-                .get("relay_settings")?
-                .get("normal")?
-                .get("tunnel")?
-                .get("only")?
-                .get("wireguard")
-                .cloned()
-        }();
+    let openvpn_constraints = || -> Option<serde_json::Value> {
+        settings
+            .get("relay_settings")?
+            .get("normal")?
+            .get("tunnel")?
+            .get("only")?
+            .get("openvpn")
+            .cloned()
+    }();
+    let wireguard_constraints = || -> Option<serde_json::Value> {
+        settings
+            .get("relay_settings")?
+            .get("normal")?
+            .get("tunnel")?
+            .get("only")?
+            .get("wireguard")
+            .cloned()
+    }();
 
-        if let Some(relay_settings) = settings.get_mut("relay_settings") {
-            if let Some(normal_settings) = relay_settings.get_mut("normal") {
-                if let Some(openvpn_constraints) = openvpn_constraints {
-                    normal_settings["openvpn_constraints"] = openvpn_constraints;
-                    normal_settings["tunnel_protocol"] =
-                        serde_json::json!(Constraint::<TunnelType>::Any);
-                } else if let Some(wireguard_constraints) = wireguard_constraints {
-                    normal_settings["wireguard_constraints"] = wireguard_constraints;
-                    normal_settings["tunnel_protocol"] =
-                        serde_json::json!(Constraint::Only(TunnelType::Wireguard));
-                } else {
-                    normal_settings["tunnel_protocol"] =
-                        serde_json::json!(Constraint::<TunnelType>::Any);
-                }
-                if let Some(object) = normal_settings.as_object_mut() {
-                    object.remove("tunnel");
-                }
+    if let Some(relay_settings) = settings.get_mut("relay_settings") {
+        if let Some(normal_settings) = relay_settings.get_mut("normal") {
+            if let Some(openvpn_constraints) = openvpn_constraints {
+                normal_settings["openvpn_constraints"] = openvpn_constraints;
+                normal_settings["tunnel_protocol"] =
+                    serde_json::json!(Constraint::<TunnelType>::Any);
+            } else if let Some(wireguard_constraints) = wireguard_constraints {
+                normal_settings["wireguard_constraints"] = wireguard_constraints;
+                normal_settings["tunnel_protocol"] =
+                    serde_json::json!(Constraint::Only(TunnelType::Wireguard));
+            } else {
+                normal_settings["tunnel_protocol"] =
+                    serde_json::json!(Constraint::<TunnelType>::Any);
+            }
+            if let Some(object) = normal_settings.as_object_mut() {
+                object.remove("tunnel");
             }
         }
+    }
 
-        settings["show_beta_releases"] = serde_json::json!(false);
-        settings["settings_version"] = serde_json::json!(SettingsVersion::V2);
+    settings["show_beta_releases"] = serde_json::json!(false);
+    settings["settings_version"] = serde_json::json!(SettingsVersion::V2);
 
-        Ok(())
-    }
+    Ok(())
+}
+
+fn version_matches(settings: &mut serde_json::Value) -> bool {
+    settings.get("settings_version").is_none()
 }
 
 #[cfg(test)]
 mod test {
-    use super::{super::SettingsMigration, Migration};
+    use super::{migrate, version_matches};
     use serde_json;
 
     pub const V2_SETTINGS: &str = r#"
@@ -191,10 +191,9 @@ mod test {
     fn test_v1_migration() {
         let mut old_settings = serde_json::from_str(V1_SETTINGS).unwrap();
 
-        let migration = Migration;
-        assert!(migration.version_matches(&mut old_settings));
+        assert!(version_matches(&mut old_settings));
 
-        migration.migrate(&mut old_settings).unwrap();
+        migrate(&mut old_settings).unwrap();
         let new_settings: serde_json::Value = serde_json::from_str(V2_SETTINGS).unwrap();
 
         assert_eq!(&old_settings, &new_settings);
@@ -204,10 +203,9 @@ mod test {
     fn test_v1_2019v3_migration() {
         let mut old_settings = serde_json::from_str(V1_SETTINGS_2019V3).unwrap();
 
-        let migration = Migration;
-        assert!(migration.version_matches(&mut old_settings));
+        assert!(version_matches(&mut old_settings));
 
-        migration.migrate(&mut old_settings).unwrap();
+        migrate(&mut old_settings).unwrap();
         let new_settings: serde_json::Value = serde_json::from_str(V2_SETTINGS).unwrap();
 
         assert_eq!(&old_settings, &new_settings);
diff --git a/mullvad-daemon/src/migrations/v2.rs b/mullvad-daemon/src/migrations/v2.rs
index 1ccf247212..5b0900ca13 100644
--- a/mullvad-daemon/src/migrations/v2.rs
+++ b/mullvad-daemon/src/migrations/v2.rs
@@ -4,69 +4,68 @@ use mullvad_types::settings::SettingsVersion;
 use std::time::Duration;
 
 
-pub(super) struct Migration;
+pub fn migrate(settings: &mut serde_json::Value) -> Result<()> {
+    if !version_matches(settings) {
+        return Ok(());
+    }
+
+    log::info!("Migrating settings format to V3");
 
-impl super::SettingsMigration for Migration {
-    fn version_matches(&self, settings: &mut serde_json::Value) -> bool {
+    // `show_beta_releases` used to be nullable
+    if settings
+        .get_mut("show_beta_releases")
+        .map(|val| val.is_null())
+        .unwrap_or(false)
+    {
         settings
-            .get("settings_version")
-            .map(|version| version == SettingsVersion::V2 as u64)
-            .unwrap_or(false)
+            .as_object_mut()
+            .ok_or(Error::NoMatchingVersion)?
+            .remove("show_beta_releases");
     }
 
-    fn migrate(&self, settings: &mut serde_json::Value) -> Result<()> {
-        log::info!("Migrating settings format to V3");
-
-        // `show_beta_releases` used to be nullable
-        if settings
-            .get_mut("show_beta_releases")
-            .map(|val| val.is_null())
-            .unwrap_or(false)
-        {
-            settings
-                .as_object_mut()
-                .ok_or(Error::NoMatchingVersion)?
-                .remove("show_beta_releases");
-        }
+    let automatic_rotation = || -> Option<u64> {
+        settings
+            .get("tunnel_options")?
+            .get("wireguard")?
+            .get("automatic_rotation")
+            .map(|ivl| ivl.as_u64())?
+    }();
 
-        let automatic_rotation = || -> Option<u64> {
-            settings
-                .get("tunnel_options")?
-                .get("wireguard")?
-                .get("automatic_rotation")
-                .map(|ivl| ivl.as_u64())?
-        }();
+    if let Some(interval) = automatic_rotation {
+        let new_ivl = match Duration::from_secs(60 * 60 * interval) {
+            ivl if ivl < MIN_ROTATION_INTERVAL => {
+                log::warn!("Increasing key rotation interval since it is below minimum");
+                MIN_ROTATION_INTERVAL
+            }
+            ivl if ivl > MAX_ROTATION_INTERVAL => {
+                log::warn!("Decreasing key rotation interval since it is above maximum");
+                MAX_ROTATION_INTERVAL
+            }
+            ivl => ivl,
+        };
 
-        if let Some(interval) = automatic_rotation {
-            let new_ivl = match Duration::from_secs(60 * 60 * interval) {
-                ivl if ivl < MIN_ROTATION_INTERVAL => {
-                    log::warn!("Increasing key rotation interval since it is below minimum");
-                    MIN_ROTATION_INTERVAL
-                }
-                ivl if ivl > MAX_ROTATION_INTERVAL => {
-                    log::warn!("Decreasing key rotation interval since it is above maximum");
-                    MAX_ROTATION_INTERVAL
-                }
-                ivl => ivl,
-            };
+        settings["tunnel_options"]["wireguard"]["rotation_interval"] = serde_json::json!(new_ivl);
+        settings["tunnel_options"]["wireguard"]
+            .as_object_mut()
+            .ok_or(Error::NoMatchingVersion)?
+            .remove("automatic_rotation");
+    }
 
-            settings["tunnel_options"]["wireguard"]["rotation_interval"] =
-                serde_json::json!(new_ivl);
-            settings["tunnel_options"]["wireguard"]
-                .as_object_mut()
-                .ok_or(Error::NoMatchingVersion)?
-                .remove("automatic_rotation");
-        }
+    settings["settings_version"] = serde_json::json!(SettingsVersion::V3);
 
-        settings["settings_version"] = serde_json::json!(SettingsVersion::V3);
+    Ok(())
+}
 
-        Ok(())
-    }
+fn version_matches(settings: &mut serde_json::Value) -> bool {
+    settings
+        .get("settings_version")
+        .map(|version| version == SettingsVersion::V2 as u64)
+        .unwrap_or(false)
 }
 
 #[cfg(test)]
 mod test {
-    use super::{super::SettingsMigration, Migration};
+    use super::{migrate, version_matches};
     use serde_json;
 
     const V2_SETTINGS: &str = r#"
@@ -176,10 +175,9 @@ mod test {
     fn test_v2_migration() {
         let mut old_settings = serde_json::from_str(V2_SETTINGS).unwrap();
 
-        let migration = Migration;
-        assert!(migration.version_matches(&mut old_settings));
+        assert!(version_matches(&mut old_settings));
 
-        migration.migrate(&mut old_settings).unwrap();
+        migrate(&mut old_settings).unwrap();
         let new_settings: serde_json::Value = serde_json::from_str(V3_SETTINGS).unwrap();
 
         assert_eq!(&old_settings, &new_settings);
diff --git a/mullvad-daemon/src/migrations/v3.rs b/mullvad-daemon/src/migrations/v3.rs
index 042fd79dcd..76b3522a96 100644
--- a/mullvad-daemon/src/migrations/v3.rs
+++ b/mullvad-daemon/src/migrations/v3.rs
@@ -4,57 +4,56 @@ use mullvad_types::settings::{
 };
 
 
-pub(super) struct Migration;
-
-impl super::SettingsMigration for Migration {
-    fn version_matches(&self, settings: &mut serde_json::Value) -> bool {
-        settings
-            .get("settings_version")
-            .map(|version| version == SettingsVersion::V3 as u64)
-            .unwrap_or(false)
+pub fn migrate(settings: &mut serde_json::Value) -> Result<()> {
+    if !version_matches(settings) {
+        return Ok(());
     }
 
-    fn migrate(&self, settings: &mut serde_json::Value) -> Result<()> {
-        log::info!("Migrating settings format to V4");
+    log::info!("Migrating settings format to V4");
 
-        let dns_options = || -> Option<&serde_json::Value> {
-            settings.get("tunnel_options")?.get("dns_options")
-        }();
+    let dns_options =
+        || -> Option<&serde_json::Value> { settings.get("tunnel_options")?.get("dns_options") }();
 
-        if let Some(options) = dns_options {
-            if options.get("state").is_none() {
-                let new_state = if options
-                    .get("custom")
-                    .map(|custom| custom.as_bool().unwrap_or(false))
-                    .unwrap_or(false)
-                {
-                    DnsState::Custom
-                } else {
-                    DnsState::Default
-                };
-                let addresses = if let Some(addrs) = options.get("addresses") {
-                    serde_json::from_value(addrs.clone()).map_err(Error::ParseError)?
-                } else {
-                    vec![]
-                };
+    if let Some(options) = dns_options {
+        if options.get("state").is_none() {
+            let new_state = if options
+                .get("custom")
+                .map(|custom| custom.as_bool().unwrap_or(false))
+                .unwrap_or(false)
+            {
+                DnsState::Custom
+            } else {
+                DnsState::Default
+            };
+            let addresses = if let Some(addrs) = options.get("addresses") {
+                serde_json::from_value(addrs.clone()).map_err(Error::ParseError)?
+            } else {
+                vec![]
+            };
 
-                settings["tunnel_options"]["dns_options"] = serde_json::json!(DnsOptions {
-                    state: new_state,
-                    default_options: DefaultDnsOptions::default(),
-                    custom_options: CustomDnsOptions { addresses },
-                });
-            }
+            settings["tunnel_options"]["dns_options"] = serde_json::json!(DnsOptions {
+                state: new_state,
+                default_options: DefaultDnsOptions::default(),
+                custom_options: CustomDnsOptions { addresses },
+            });
         }
+    }
 
-        settings["settings_version"] = serde_json::json!(SettingsVersion::V4);
+    settings["settings_version"] = serde_json::json!(SettingsVersion::V4);
 
-        Ok(())
-    }
+    Ok(())
+}
+
+fn version_matches(settings: &mut serde_json::Value) -> bool {
+    settings
+        .get("settings_version")
+        .map(|version| version == SettingsVersion::V3 as u64)
+        .unwrap_or(false)
 }
 
 #[cfg(test)]
 mod test {
-    use super::{super::SettingsMigration, Migration};
+    use super::{migrate, version_matches};
     use serde_json;
 
     pub const V3_SETTINGS: &str = r#"
@@ -186,10 +185,9 @@ mod test {
     fn test_v3_migration() {
         let mut old_settings = serde_json::from_str(V3_SETTINGS).unwrap();
 
-        let migration = Migration;
-        assert!(migration.version_matches(&mut old_settings));
+        assert!(version_matches(&mut old_settings));
 
-        migration.migrate(&mut old_settings).unwrap();
+        migrate(&mut old_settings).unwrap();
         let new_settings: serde_json::Value = serde_json::from_str(V4_SETTINGS).unwrap();
 
         assert_eq!(&old_settings, &new_settings);
diff --git a/mullvad-daemon/src/migrations/v4.rs b/mullvad-daemon/src/migrations/v4.rs
index 7a459ce516..76b88ce987 100644
--- a/mullvad-daemon/src/migrations/v4.rs
+++ b/mullvad-daemon/src/migrations/v4.rs
@@ -10,30 +10,23 @@ const WIREGUARD_TCP_PORTS: [u16; 3] = [80, 443, 5001];
 const OPENVPN_TCP_PORTS: [u16; 2] = [80, 443];
 
 
-pub(super) struct Migration;
-
-impl super::SettingsMigration for Migration {
-    fn version_matches(&self, settings: &mut serde_json::Value) -> bool {
-        settings
-            .get("settings_version")
-            .map(|version| version == SettingsVersion::V4 as u64)
-            .unwrap_or(false)
+pub fn migrate(settings: &mut serde_json::Value) -> Result<()> {
+    if !version_matches(settings) {
+        return Ok(());
     }
 
-    fn migrate(&self, settings: &mut serde_json::Value) -> Result<()> {
-        log::info!("Migrating settings format to V5");
+    log::info!("Migrating settings format to V5");
 
-        let wireguard_constraints = || -> Option<&serde_json::Value> {
-            settings
-                .get("relay_settings")?
-                .get("normal")?
-                .get("wireguard_constraints")
-        }();
+    let wireguard_constraints = || -> Option<&serde_json::Value> {
+        settings
+            .get("relay_settings")?
+            .get("normal")?
+            .get("wireguard_constraints")
+    }();
 
-        if let Some(constraints) = wireguard_constraints {
-            let (port, protocol): (Constraint<u16>, TransportProtocol) = if let Some(port) =
-                constraints.get("port")
-            {
+    if let Some(constraints) = wireguard_constraints {
+        let (port, protocol): (Constraint<u16>, TransportProtocol) =
+            if let Some(port) = constraints.get("port") {
                 let port_constraint =
                     serde_json::from_value(port.clone()).map_err(Error::ParseError)?;
                 match port_constraint {
@@ -44,64 +37,70 @@ impl super::SettingsMigration for Migration {
                 (Constraint::Any, TransportProtocol::Udp)
             };
 
-            settings["relay_settings"]["normal"]["wireguard_constraints"]["port"] = match port {
-                Constraint::Any => {
-                    serde_json::json!(Constraint::<TransportPort>::Any)
-                }
-                Constraint::Only(_) => {
-                    serde_json::json!(Constraint::Only(TransportPort { protocol, port }))
-                }
-            };
+        settings["relay_settings"]["normal"]["wireguard_constraints"]["port"] = match port {
+            Constraint::Any => {
+                serde_json::json!(Constraint::<TransportPort>::Any)
+            }
+            Constraint::Only(_) => {
+                serde_json::json!(Constraint::Only(TransportPort { protocol, port }))
+            }
+        };
 
-            settings["relay_settings"]["normal"]["wireguard_constraints"]
-                .as_object_mut()
-                .ok_or(Error::NoMatchingVersion)?
-                .remove("protocol");
-        }
+        settings["relay_settings"]["normal"]["wireguard_constraints"]
+            .as_object_mut()
+            .ok_or(Error::NoMatchingVersion)?
+            .remove("protocol");
+    }
 
-        let openvpn_constraints = || -> Option<&serde_json::Value> {
-            settings
-                .get("relay_settings")?
-                .get("normal")?
-                .get("openvpn_constraints")
-        }();
+    let openvpn_constraints = || -> Option<&serde_json::Value> {
+        settings
+            .get("relay_settings")?
+            .get("normal")?
+            .get("openvpn_constraints")
+    }();
 
-        if let Some(constraints) = openvpn_constraints {
-            let port: Constraint<u16> = if let Some(port) = constraints.get("port") {
-                serde_json::from_value(port.clone()).map_err(Error::ParseError)?
+    if let Some(constraints) = openvpn_constraints {
+        let port: Constraint<u16> = if let Some(port) = constraints.get("port") {
+            serde_json::from_value(port.clone()).map_err(Error::ParseError)?
+        } else {
+            Constraint::Any
+        };
+        let transport_constraint: Constraint<TransportProtocol> =
+            if let Some(protocol) = constraints.get("protocol") {
+                serde_json::from_value(protocol.clone()).map_err(Error::ParseError)?
             } else {
                 Constraint::Any
             };
-            let transport_constraint: Constraint<TransportProtocol> =
-                if let Some(protocol) = constraints.get("protocol") {
-                    serde_json::from_value(protocol.clone()).map_err(Error::ParseError)?
-                } else {
-                    Constraint::Any
-                };
 
-            let port = match (port, transport_constraint) {
-                (Constraint::Only(port), Constraint::Any) => Constraint::Only(TransportPort {
-                    protocol: openvpn_protocol_from_port(port),
-                    port: Constraint::Only(port),
-                }),
-                (port, Constraint::Only(protocol)) => {
-                    Constraint::Only(TransportPort { protocol, port })
-                }
-                (Constraint::Any, Constraint::Any) => Constraint::Any,
-            };
+        let port = match (port, transport_constraint) {
+            (Constraint::Only(port), Constraint::Any) => Constraint::Only(TransportPort {
+                protocol: openvpn_protocol_from_port(port),
+                port: Constraint::Only(port),
+            }),
+            (port, Constraint::Only(protocol)) => {
+                Constraint::Only(TransportPort { protocol, port })
+            }
+            (Constraint::Any, Constraint::Any) => Constraint::Any,
+        };
 
-            settings["relay_settings"]["normal"]["openvpn_constraints"]["port"] =
-                serde_json::json!(port);
-            settings["relay_settings"]["normal"]["openvpn_constraints"]
-                .as_object_mut()
-                .ok_or(Error::NoMatchingVersion)?
-                .remove("protocol");
-        }
+        settings["relay_settings"]["normal"]["openvpn_constraints"]["port"] =
+            serde_json::json!(port);
+        settings["relay_settings"]["normal"]["openvpn_constraints"]
+            .as_object_mut()
+            .ok_or(Error::NoMatchingVersion)?
+            .remove("protocol");
+    }
 
-        settings["settings_version"] = serde_json::json!(SettingsVersion::V5);
+    settings["settings_version"] = serde_json::json!(SettingsVersion::V5);
 
-        Ok(())
-    }
+    Ok(())
+}
+
+fn version_matches(settings: &mut serde_json::Value) -> bool {
+    settings
+        .get("settings_version")
+        .map(|version| version == SettingsVersion::V4 as u64)
+        .unwrap_or(false)
 }
 
 fn openvpn_protocol_from_port(port: u16) -> TransportProtocol {
@@ -124,7 +123,7 @@ fn wg_protocol_from_port(port: u16) -> TransportProtocol {
 
 #[cfg(test)]
 mod test {
-    use super::{super::SettingsMigration, Migration};
+    use super::{migrate, version_matches};
     use serde_json;
 
     pub const V4_SETTINGS: &str = r#"
@@ -272,10 +271,9 @@ mod test {
     fn test_v4_migration() {
         let mut old_settings = serde_json::from_str(V4_SETTINGS).unwrap();
 
-        let migration = Migration;
-        assert!(migration.version_matches(&mut old_settings));
+        assert!(version_matches(&mut old_settings));
 
-        migration.migrate(&mut old_settings).unwrap();
+        migrate(&mut old_settings).unwrap();
         let new_settings: serde_json::Value = serde_json::from_str(V5_SETTINGS).unwrap();
 
         assert_eq!(&old_settings, &new_settings);