Rename and change to tuple struct

Serock3 · Serock3 · commit 630bc17ae3b6 · 2025-03-20T10:23:53.000+01:00
diff --git a/mullvad-daemon/src/tunnel.rs b/mullvad-daemon/src/tunnel.rs
@@ -8,7 +8,7 @@ use std::{
 
 use tokio::sync::Mutex;
 
-use mullvad_relay_selector::{GetRelay, RelaySelector, RuntimeParameters, WireguardConfig};
+use mullvad_relay_selector::{GetRelay, RelaySelector, RuntimeIpAvailability, WireguardConfig};
 use mullvad_types::{
     endpoint::MullvadWireguardEndpoint, location::GeoIpLocation, relay_list::Relay,
     settings::TunnelOptions,
@@ -164,9 +164,10 @@ impl InnerParametersGenerator {
         ipv6: bool,
     ) -> Result<TunnelParameters, Error> {
         let data = self.device().await?;
-        let selected_relay = self
-            .relay_selector
-            .get_relay(retry_attempt as usize, RuntimeParameters::new(ipv4, ipv6))?;
+        let selected_relay = self.relay_selector.get_relay(
+            retry_attempt as usize,
+            RuntimeIpAvailability::new(ipv4, ipv6),
+        )?;
 
         match selected_relay {
             #[cfg(not(target_os = "android"))]
diff --git a/mullvad-relay-selector/src/lib.rs b/mullvad-relay-selector/src/lib.rs
@@ -11,6 +11,6 @@ pub use error::Error;
 pub use relay_selector::{
     detailer, matcher, matcher::filter_matching_relay_list, query, relays::WireguardConfig,
     AdditionalRelayConstraints, AdditionalWireguardConstraints, GetRelay, RelaySelector,
-    RuntimeParameters, SelectedBridge, SelectedObfuscator, SelectorConfig, OPENVPN_RETRY_ORDER,
+    RuntimeIpAvailability, SelectedBridge, SelectedObfuscator, SelectorConfig, OPENVPN_RETRY_ORDER,
     WIREGUARD_RETRY_ORDER,
 };
diff --git a/mullvad-relay-selector/src/relay_selector/mod.rs b/mullvad-relay-selector/src/relay_selector/mod.rs
@@ -52,8 +52,8 @@ use talpid_types::{
     ErrorExt,
 };
 
-/// [`WIREGUARD_RETRY_ORDER`] defines an ordered set of relay parameters which the relay selector should
-/// prioritize on successive connection attempts. Note that these will *never* override user
+/// [`WIREGUARD_RETRY_ORDER`] defines an ordered set of relay parameters which the relay selector
+/// should prioritize on successive connection attempts. Note that these will *never* override user
 /// preferences. See [the documentation on `RelayQuery`][RelayQuery] for further details.
 ///
 /// This list should be kept in sync with the expected behavior defined in `docs/relay-selector.md`
@@ -80,8 +80,8 @@ pub static WIREGUARD_RETRY_ORDER: LazyLock<Vec<RelayQuery>> = LazyLock::new(|| {
     ]
 });
 
-/// [`OPENVPN_RETRY_ORDER`] defines an ordered set of relay parameters which the relay selector should
-/// prioritize on successive connection attempts. Note that these will *never* override user
+/// [`OPENVPN_RETRY_ORDER`] defines an ordered set of relay parameters which the relay selector
+/// should prioritize on successive connection attempts. Note that these will *never* override user
 /// preferences. See [the documentation on `RelayQuery`][RelayQuery] for further details.
 ///
 /// This list should be kept in sync with the expected behavior defined in `docs/relay-selector.md`
@@ -178,31 +178,26 @@ pub struct AdditionalWireguardConstraints {
     pub quantum_resistant: QuantumResistantState,
 }
 
-/// Values which affect the choice of relay but are only known at runtime.
+/// Whether IPv4, IPv6 is available at runtime.
+///
+/// `None` means that no IP version is available, `Some(Any)` means that both are available,
 #[derive(Clone, Debug)]
-pub struct RuntimeParameters {
-    /// Whether IPv4, IPv6 or both is available
-    pub ip_availability: Option<Constraint<IpVersion>>,
-}
-
-impl RuntimeParameters {
-    pub fn new(ipv4: bool, ipv6: bool) -> RuntimeParameters {
-            RuntimeParameters {
-            ip_availability: match (ipv4, ipv6) {
-                (true, true) => Some(Constraint::Any),
-                (false, true) => Some(Constraint::Only(IpVersion::V6)),
-                (true, false) => Some(Constraint::Only(IpVersion::V4)),
-                (false, false) => None,
-            },
-        }
+pub struct RuntimeIpAvailability(pub Option<Constraint<IpVersion>>);
+
+impl RuntimeIpAvailability {
+    pub fn new(ipv4: bool, ipv6: bool) -> RuntimeIpAvailability {
+        RuntimeIpAvailability(match (ipv4, ipv6) {
+            (true, true) => Some(Constraint::Any),
+            (false, true) => Some(Constraint::Only(IpVersion::V6)),
+            (true, false) => Some(Constraint::Only(IpVersion::V4)),
+            (false, false) => None,
+        })
     }
 }
 
-impl Default for RuntimeParameters {
+impl Default for RuntimeIpAvailability {
     fn default() -> Self {
-        RuntimeParameters {
-            ip_availability: Some(Constraint::Only(IpVersion::V4)),
-        }
+        RuntimeIpAvailability(Some(Constraint::Only(IpVersion::V4)))
     }
 }
 
@@ -555,7 +550,7 @@ impl RelaySelector {
     pub fn get_relay(
         &self,
         retry_attempt: usize,
-        runtime_params: RuntimeParameters,
+        runtime_params: RuntimeIpAvailability,
     ) -> Result<GetRelay, Error> {
         let config_guard = self.config.lock().unwrap();
         let config = SpecializedSelectorConfig::from(&*config_guard);
@@ -589,7 +584,7 @@ impl RelaySelector {
         &self,
         retry_attempt: usize,
         retry_order: &[RelayQuery],
-        runtime_params: RuntimeParameters,
+        runtime_ip_availability: RuntimeIpAvailability,
     ) -> Result<GetRelay, Error> {
         let config_guard = self.config.lock().unwrap();
         let config = SpecializedSelectorConfig::from(&*config_guard);
@@ -606,7 +601,7 @@ impl RelaySelector {
                 let query = Self::pick_and_merge_query(
                     retry_attempt,
                     retry_order,
-                    runtime_params,
+                    runtime_ip_availability,
                     &normal_config,
                     &relay_list,
                 )?;
@@ -632,12 +627,12 @@ impl RelaySelector {
     fn pick_and_merge_query(
         retry_attempt: usize,
         retry_order: &[RelayQuery],
-        runtime_params: RuntimeParameters,
+        runtime_ip_availability: RuntimeIpAvailability,
         user_config: &NormalSelectorConfig<'_>,
         parsed_relays: &RelayList,
     ) -> Result<RelayQuery, Error> {
         let mut user_query = RelayQuery::try_from(user_config.clone())?;
-        apply_ip_availability(runtime_params, &mut user_query)?;
+        apply_ip_availability(runtime_ip_availability, &mut user_query)?;
         log::trace!("Merging user preferences {user_query:?} with default retry strategy");
         retry_order
             .iter()
@@ -683,10 +678,10 @@ impl RelaySelector {
         parsed_relays: &RelayList,
         custom_lists: &CustomListsSettings,
     ) -> Result<GetRelay, Error> {
-        // FIXME: A bit of defensive programming - calling `get_wireguard_relay_inner` with a query that
-        // doesn't specify Wireguard as the desired tunnel type is not valid and will lead
-        // to unwanted behavior. This should be seen as a workaround, and it would be nicer
-        // to lift this invariant to be checked by the type system instead.
+        // FIXME: A bit of defensive programming - calling `get_wireguard_relay_inner` with a query
+        // that doesn't specify Wireguard as the desired tunnel type is not valid and will
+        // lead to unwanted behavior. This should be seen as a workaround, and it would be
+        // nicer to lift this invariant to be checked by the type system instead.
         let mut query = query.clone();
         query.set_tunnel_protocol(TunnelType::Wireguard)?;
         Self::get_wireguard_relay_inner(&query, custom_lists, parsed_relays)
@@ -1174,15 +1169,15 @@ impl RelaySelector {
 }
 
 fn apply_ip_availability(
-    runtime_params: RuntimeParameters,
+    runtime_ip_availability: RuntimeIpAvailability,
     user_query: &mut RelayQuery,
 ) -> Result<(), Error> {
     let wireguard_constraints = user_query
         .wireguard_constraints()
         .to_owned()
         .intersection(WireguardRelayQuery {
-            ip_version: runtime_params
-                .ip_availability
+            ip_version: runtime_ip_availability
+                .0
                 .ok_or(Error::IpVersionUnavailable)?,
             ..Default::default()
         })
diff --git a/mullvad-relay-selector/tests/relay_selector.rs b/mullvad-relay-selector/tests/relay_selector.rs
@@ -15,7 +15,7 @@ use talpid_types::net::{
 
 use mullvad_relay_selector::{
     query::{builder::RelayQueryBuilder, BridgeQuery, ObfuscationQuery, OpenVpnRelayQuery},
-    Error, GetRelay, RelaySelector, RuntimeParameters, SelectedObfuscator, SelectorConfig,
+    Error, GetRelay, RelaySelector, RuntimeIpAvailability, SelectedObfuscator, SelectorConfig,
     WireguardConfig, OPENVPN_RETRY_ORDER, WIREGUARD_RETRY_ORDER,
 };
 use mullvad_types::{
@@ -379,7 +379,8 @@ fn assert_openvpn_retry_order() {
     );
 }
 
-/// Test whether the relay selector seems to respect the order as defined by [`WIREGUARD_RETRY_ORDER`].
+/// Test whether the relay selector seems to respect the order as defined by
+/// [`WIREGUARD_RETRY_ORDER`].
 #[test]
 fn test_wireguard_retry_order() {
     // In order to for the relay queries defined by `RETRY_ORDER` to always take precedence,
@@ -390,12 +391,7 @@ fn test_wireguard_retry_order() {
     let relay_selector = default_relay_selector();
     for (retry_attempt, query) in WIREGUARD_RETRY_ORDER.iter().enumerate() {
         let relay = relay_selector
-            .get_relay(
-                retry_attempt,
-                RuntimeParameters {
-                    ip_availability: Some(Constraint::Any),
-                },
-            )
+            .get_relay(retry_attempt, RuntimeIpAvailability(Some(Constraint::Any)))
             .unwrap_or_else(|_| panic!("Retry attempt {retry_attempt} did not yield any relay"));
         // For each relay, cross-check that the it has the expected tunnel protocol
         let tunnel_type = tunnel_type(&unwrap_relay(relay.clone()));
@@ -434,7 +430,8 @@ fn test_wireguard_retry_order() {
     }
 }
 
-/// Test whether the relay selector seems to respect the order as defined by [`OPENVPN_RETRY_ORDER`].
+/// Test whether the relay selector seems to respect the order as defined by
+/// [`OPENVPN_RETRY_ORDER`].
 #[test]
 fn test_openvpn_retry_order() {
     // In order to for the relay queries defined by `RETRY_ORDER` to always take precedence,
@@ -453,12 +450,7 @@ fn test_openvpn_retry_order() {
 
     for (retry_attempt, query) in OPENVPN_RETRY_ORDER.iter().enumerate() {
         let relay = relay_selector
-            .get_relay(
-                retry_attempt,
-                RuntimeParameters {
-                    ip_availability: Some(Constraint::Any),
-                },
-            )
+            .get_relay(retry_attempt, RuntimeIpAvailability(Some(Constraint::Any)))
             .unwrap_or_else(|_| panic!("Retry attempt {retry_attempt} did not yield any relay"));
         // For each relay, cross-check that the it has the expected tunnel protocol
         let tunnel_type = tunnel_type(&unwrap_relay(relay.clone()));
@@ -1164,7 +1156,11 @@ fn test_openvpn_auto_bridge() {
         .take(100 * retry_order.len())
     {
         let relay = relay_selector
-            .get_relay_with_custom_params(retry_attempt, &retry_order, RuntimeParameters::default())
+            .get_relay_with_custom_params(
+                retry_attempt,
+                &retry_order,
+                RuntimeIpAvailability::default(),
+            )
             .unwrap();
         match relay {
             GetRelay::OpenVpn { bridge, .. } => {
@@ -1273,7 +1269,7 @@ fn test_include_in_country() {
     // If include_in_country is false for all relays, a relay must be selected anyway.
     let relay_selector = RelaySelector::from_list(SelectorConfig::default(), relay_list.clone());
     assert!(relay_selector
-        .get_relay(0, RuntimeParameters::default())
+        .get_relay(0, RuntimeIpAvailability::default())
         .is_ok());
 
     // If include_in_country is true for some relay, it must always be selected.
@@ -1282,7 +1278,7 @@ fn test_include_in_country() {
     let relay_selector = RelaySelector::from_list(SelectorConfig::default(), relay_list);
     let relay = unwrap_relay(
         relay_selector
-            .get_relay(0, RuntimeParameters::default())
+            .get_relay(0, RuntimeIpAvailability::default())
             .expect("expected match"),
     );
 
@@ -1619,7 +1615,7 @@ fn valid_user_setting_should_yield_relay() {
     let user_result = relay_selector.get_relay_by_query(user_query.clone());
     for retry_attempt in 0..WIREGUARD_RETRY_ORDER.len() {
         let post_unification_result =
-            relay_selector.get_relay(retry_attempt, RuntimeParameters::default());
+            relay_selector.get_relay(retry_attempt, RuntimeIpAvailability::default());
         if user_result.is_ok() {
             assert!(post_unification_result.is_ok(), "Expected Post-unification query to be valid because original query {:#?} yielded a connection configuration", user_query)
         }
@@ -1642,9 +1638,7 @@ fn test_shadowsocks_runtime_ipv4_unavailable() {
         ..SelectorConfig::default()
     };
     let relay_selector = RelaySelector::from_list(config, RELAYS.clone());
-    let runtime_parameters = RuntimeParameters {
-        ip_availability: Some(Constraint::Only(IpVersion::V6)),
-    };
+    let runtime_parameters = RuntimeIpAvailability(Some(Constraint::Only(IpVersion::V6)));
     let user_result = relay_selector.get_relay(0, runtime_parameters).unwrap();
     assert!(
         matches!(user_result, GetRelay::Wireguard {
@@ -1672,9 +1666,7 @@ fn test_runtime_ipv4_unavailable() {
         ..SelectorConfig::default()
     };
     let relay_selector = RelaySelector::from_list(config, RELAYS.clone());
-    let runtime_parameters = RuntimeParameters {
-        ip_availability: Some(Constraint::Only(IpVersion::V6)),
-    };
+    let runtime_parameters = RuntimeIpAvailability(Some(Constraint::Only(IpVersion::V6)));
     let relay = relay_selector.get_relay(0, runtime_parameters).unwrap();
     match relay {
         GetRelay::Wireguard { endpoint, .. } => {
