Implement formatter for constraints

4 files changed, 129 insertions, 142 deletions

diff --git a/mullvad-cli/src/cmds/bridge.rs b/mullvad-cli/src/cmds/bridge.rs
index 3cc1a95b37..2ee7d9a4f0 100644
--- a/mullvad-cli/src/cmds/bridge.rs
+++ b/mullvad-cli/src/cmds/bridge.rs
@@ -3,8 +3,8 @@ use clap::Subcommand;
 use mullvad_management_interface::MullvadProxyClient;
 use mullvad_types::{
     relay_constraints::{
-        BridgeConstraints, BridgeSettings, BridgeState, Constraint, LocationConstraint, Ownership,
-        Provider, Providers,
+        BridgeConstraints, BridgeConstraintsFormatter, BridgeSettings, BridgeState, Constraint,
+        LocationConstraint, Ownership, Provider, Providers,
     },
     relay_list::RelayEndpointData,
 };
@@ -279,10 +279,14 @@ impl Bridge {
                     Self::print_shadowsocks_proxy(&shadowsocks_proxy)
                 }
             },
-            BridgeSettings::Normal(constraints) => {
-                let mut buf = String::new();
-                let _ = constraints.format(&mut buf, &settings.custom_lists);
-                println!("Bridge constraints: {buf}")
+            BridgeSettings::Normal(ref constraints) => {
+                println!(
+                    "Bridge constraints: {}",
+                    BridgeConstraintsFormatter {
+                        constraints,
+                        custom_lists: &settings.custom_lists
+                    }
+                )
             }
         };
         Ok(())
diff --git a/mullvad-relay-selector/src/lib.rs b/mullvad-relay-selector/src/lib.rs
index 843797c853..4baee72d04 100644
--- a/mullvad-relay-selector/src/lib.rs
+++ b/mullvad-relay-selector/src/lib.rs
@@ -10,8 +10,8 @@ use mullvad_types::{
     relay_constraints::{
         BridgeSettings, BridgeState, Constraint, InternalBridgeConstraints, LocationConstraint,
         Match, ObfuscationSettings, OpenVpnConstraints, Ownership, Providers, RelayConstraints,
-        RelaySettings, ResolvedLocationConstraint, SelectedObfuscation, Set, TransportPort,
-        Udp2TcpObfuscationSettings,
+        RelayConstraintsFormatter, RelaySettings, ResolvedLocationConstraint, SelectedObfuscation,
+        Set, TransportPort, Udp2TcpObfuscationSettings,
     },
     relay_list::{BridgeEndpointData, Relay, RelayEndpointData, RelayList},
     CustomTunnelEndpoint,
@@ -651,9 +651,13 @@ impl RelaySelector {
             );
             Ok(result)
         } else {
-            let mut relay_constraints_string = String::new();
-            let _ = relay_constraints.format(&mut relay_constraints_string, custom_lists);
-            log::warn!("No relays matching {}", &relay_constraints_string);
+            log::warn!(
+                "No relays matching constraints: {}",
+                RelayConstraintsFormatter {
+                    constraints: relay_constraints,
+                    custom_lists,
+                }
+            );
             Err(Error::NoRelay)
         }
     }
diff --git a/mullvad-types/src/relay_constraints.rs b/mullvad-types/src/relay_constraints.rs
index b1282e2349..dcce9fc950 100644
--- a/mullvad-types/src/relay_constraints.rs
+++ b/mullvad-types/src/relay_constraints.rs
@@ -10,7 +10,7 @@ use crate::{
 #[cfg(target_os = "android")]
 use jnix::{jni::objects::JObject, FromJava, IntoJava, JnixEnv};
 use serde::{Deserialize, Serialize};
-use std::{collections::HashSet, fmt, fmt::Write, str::FromStr};
+use std::{collections::HashSet, fmt, str::FromStr};
 use talpid_types::net::{openvpn::ProxySettings, IpVersion, TransportProtocol, TunnelType};
 
 pub trait Match<T> {
@@ -205,21 +205,6 @@ pub enum RelaySettings {
 }
 
 impl RelaySettings {
-    pub fn format(
-        &self,
-        s: &mut String,
-        custom_lists: &CustomListsSettings,
-    ) -> Result<(), fmt::Error> {
-        match self {
-            RelaySettings::CustomTunnelEndpoint(endpoint) => {
-                write!(s, "custom endpoint {endpoint}")
-            }
-            RelaySettings::Normal(constraints) => constraints.format(s, custom_lists),
-        }
-    }
-}
-
-impl RelaySettings {
     pub fn merge(&self, update: RelaySettingsUpdate) -> Self {
         match update {
             RelaySettingsUpdate::CustomTunnelEndpoint(relay) => {
@@ -235,6 +220,31 @@ impl RelaySettings {
     }
 }
 
+pub struct RelaySettingsFormatter<'a> {
+    pub settings: &'a RelaySettings,
+    pub custom_lists: &'a CustomListsSettings,
+}
+
+impl<'a> fmt::Display for RelaySettingsFormatter<'a> {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        match self.settings {
+            RelaySettings::CustomTunnelEndpoint(endpoint) => {
+                write!(f, "custom endpoint {endpoint}")
+            }
+            RelaySettings::Normal(constraints) => {
+                write!(
+                    f,
+                    "{}",
+                    RelayConstraintsFormatter {
+                        constraints,
+                        custom_lists: self.custom_lists
+                    }
+                )
+            }
+        }
+    }
+}
+
 #[derive(Debug, Clone, Eq, PartialEq, Serialize, Deserialize)]
 #[serde(rename_all = "snake_case")]
 #[cfg_attr(target_os = "android", derive(IntoJava, FromJava))]
@@ -326,30 +336,6 @@ impl Constraint<ResolvedLocationConstraint> {
     }
 }
 
-impl LocationConstraint {
-    fn format(&self, f: &mut String, custom_lists: &CustomListsSettings) -> Result<(), fmt::Error> {
-        match self {
-            Self::Location(location) => writeln!(f, "location - {location}"),
-            Self::CustomList { list_id } => match custom_lists
-                .custom_lists
-                .iter()
-                .find(|custom_list| &custom_list.id == list_id)
-            {
-                Some(list) => {
-                    writeln!(f, "custom list - {}", list.name)?;
-                    for location in &list.locations {
-                        writeln!(f, "\t{}", location)?;
-                    }
-                    Ok(())
-                }
-                None => {
-                    writeln!(f, "custom list - list not found")
-                }
-            },
-        }
-    }
-}
-
 pub struct LocationConstraintFormatter<'a> {
     pub constraint: &'a LocationConstraint,
     pub custom_lists: &'a CustomListsSettings,
@@ -420,51 +406,36 @@ impl RelayConstraints {
     }
 }
 
-impl RelayConstraints {
-    pub fn format(
-        &self,
-        f: &mut String,
-        custom_lists: &CustomListsSettings,
-    ) -> Result<(), fmt::Error> {
-        match self.tunnel_protocol {
-            Constraint::Any => {
-                writeln!(
-                    f,
-                    "Tunnel protocol: Any\nOpenVPN constraints: {}\nWireguard constraints: ",
-                    &self.openvpn_constraints,
-                )?;
-                self.wireguard_constraints.format(f, custom_lists)?;
-            }
-            Constraint::Only(ref tunnel_protocol) => {
-                writeln!(f, "Tunnel protocol: {}", tunnel_protocol)?;
-                match tunnel_protocol {
-                    TunnelType::Wireguard => {
-                        writeln!(f, "Wireguard constraints: ")?;
-                        self.wireguard_constraints.format(f, custom_lists)?;
-                    }
-                    TunnelType::OpenVpn => {
-                        writeln!(f, "OpenVPN constraints: {}", &self.openvpn_constraints)?;
-                    }
-                };
-            }
-        }
-        match self.location {
-            Constraint::Any => writeln!(f, "Location: Any")?,
-            Constraint::Only(ref location_constraint) => {
-                write!(f, "Location: ")?;
-                location_constraint.format(f, custom_lists)?;
-            }
-        }
-        match self.providers {
-            Constraint::Any => writeln!(f, "Provider: Any")?,
-            Constraint::Only(ref constraint) => writeln!(f, "Provider: {}", constraint)?,
-        }
-        match self.ownership {
-            Constraint::Any => Ok(()),
-            Constraint::Only(ref constraint) => {
-                write!(f, "Constraints: {constraint}")
-            }
-        }
+pub struct RelayConstraintsFormatter<'a> {
+    pub constraints: &'a RelayConstraints,
+    pub custom_lists: &'a CustomListsSettings,
+}
+
+impl<'a> fmt::Display for RelayConstraintsFormatter<'a> {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        writeln!(
+            f,
+            "Tunnel protocol: {}\nOpenVPN constraints: {}\nWireguard constraints: {}",
+            self.constraints.tunnel_protocol,
+            self.constraints.openvpn_constraints,
+            WireguardConstraintsFormatter {
+                constraints: &self.constraints.wireguard_constraints,
+                custom_lists: self.custom_lists,
+            },
+        )?;
+        writeln!(
+            f,
+            "Location: {}",
+            self.constraints
+                .location
+                .as_ref()
+                .map(|location| LocationConstraintFormatter {
+                    constraint: location,
+                    custom_lists: self.custom_lists,
+                })
+        )?;
+        writeln!(f, "Provider(s): {}", self.constraints.providers)?;
+        write!(f, "Ownership: {}", self.constraints.ownership)
     }
 }
 
@@ -741,6 +712,33 @@ pub struct WireguardConstraints {
     pub entry_location: Constraint<LocationConstraint>,
 }
 
+pub struct WireguardConstraintsFormatter<'a> {
+    pub constraints: &'a WireguardConstraints,
+    pub custom_lists: &'a CustomListsSettings,
+}
+
+impl<'a> fmt::Display for WireguardConstraintsFormatter<'a> {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        match self.constraints.port {
+            Constraint::Any => write!(f, "any port")?,
+            Constraint::Only(port) => write!(f, "port {}", port)?,
+        }
+        if let Constraint::Only(ip_version) = self.constraints.ip_version {
+            write!(f, ", {},", ip_version)?;
+        }
+        if self.constraints.use_multihop {
+            let location = self.constraints.entry_location.as_ref().map(|location| {
+                LocationConstraintFormatter {
+                    constraint: location,
+                    custom_lists: self.custom_lists,
+                }
+            });
+            write!(f, ", multihop entry {}", location)?;
+        }
+        Ok(())
+    }
+}
+
 #[cfg(target_os = "android")]
 impl<'env, 'sub_env> FromJava<'env, JObject<'sub_env>> for WireguardConstraints
 where
@@ -769,30 +767,6 @@ where
     }
 }
 
-impl WireguardConstraints {
-    fn format(&self, f: &mut String, custom_lists: &CustomListsSettings) -> Result<(), fmt::Error> {
-        match self.port {
-            Constraint::Any => writeln!(f, "Port: Any")?,
-            Constraint::Only(port) => writeln!(f, "port {port}")?,
-        }
-        match self.ip_version {
-            Constraint::Any => writeln!(f, "Protocol: IPv4 or IPv6")?,
-            Constraint::Only(protocol) => writeln!(f, "Protocol: {protocol}")?,
-        }
-        if self.use_multihop {
-            match &self.entry_location {
-                Constraint::Any => writeln!(f, "Entry location: Any"),
-                Constraint::Only(location) => {
-                    write!(f, "Wireguard entry ")?;
-                    location.format(f, custom_lists)
-                }
-            }
-        } else {
-            Ok(())
-        }
-    }
-}
-
 /// Used for jni conversion.
 #[cfg(target_os = "android")]
 #[derive(Debug, Default, Clone, Eq, PartialEq, FromJava, IntoJava)]
@@ -903,24 +877,30 @@ pub struct BridgeConstraints {
     pub ownership: Constraint<Ownership>,
 }
 
-impl BridgeConstraints {
-    pub fn format(
-        &self,
-        f: &mut String,
-        custom_lists: &CustomListsSettings,
-    ) -> Result<(), fmt::Error> {
-        match self.location {
+pub struct BridgeConstraintsFormatter<'a> {
+    pub constraints: &'a BridgeConstraints,
+    pub custom_lists: &'a CustomListsSettings,
+}
+
+impl<'a> fmt::Display for BridgeConstraintsFormatter<'a> {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        match self.constraints.location {
             Constraint::Any => write!(f, "any location")?,
-            Constraint::Only(ref location_constraint) => {
-                location_constraint.format(f, custom_lists)?
-            }
+            Constraint::Only(ref constraint) => write!(
+                f,
+                "{}",
+                LocationConstraintFormatter {
+                    constraint,
+                    custom_lists: self.custom_lists,
+                }
+            )?,
         }
         write!(f, " using ")?;
-        match self.providers {
+        match self.constraints.providers {
             Constraint::Any => write!(f, "any provider")?,
             Constraint::Only(ref constraint) => write!(f, "{}", constraint)?,
         }
-        match self.ownership {
+        match self.constraints.ownership {
             Constraint::Any => Ok(()),
             Constraint::Only(ref constraint) => {
                 write!(f, " and {constraint}")
diff --git a/mullvad-types/src/settings/mod.rs b/mullvad-types/src/settings/mod.rs
index 11a19f6552..3b3ca15014 100644
--- a/mullvad-types/src/settings/mod.rs
+++ b/mullvad-types/src/settings/mod.rs
@@ -3,7 +3,7 @@ use crate::{
     relay_constraints::{
         BridgeConstraints, BridgeSettings, BridgeState, Constraint, GeographicLocationConstraint,
         LocationConstraint, ObfuscationSettings, RelayConstraints, RelaySettings,
-        RelaySettingsUpdate, SelectedObfuscation, WireguardConstraints,
+        RelaySettingsFormatter, RelaySettingsUpdate, SelectedObfuscation, WireguardConstraints,
     },
     wireguard,
 };
@@ -153,17 +153,16 @@ impl Settings {
                 self.bridge_state = BridgeState::Auto;
             }
 
-            let mut old_settings_string = String::new();
-            let _ = self
-                .relay_settings
-                .format(&mut old_settings_string, &self.custom_lists);
-            let mut new_settings_string = String::new();
-            let _ = new_settings.format(&mut new_settings_string, &self.custom_lists);
-
             log::debug!(
                 "Changing relay settings:\n\tfrom: {}\n\tto: {}",
-                old_settings_string,
-                new_settings_string,
+                RelaySettingsFormatter {
+                    settings: &self.relay_settings,
+                    custom_lists: &self.custom_lists,
+                },
+                RelaySettingsFormatter {
+                    settings: &new_settings,
+                    custom_lists: &self.custom_lists,
+                },
             );
 
             self.relay_settings = new_settings;