fixup: Use optional primitives for relay constraint

gui/src/main/daemon-rpc.ts

@@ -1277,11 +1277,11 @@ function convertFromLocationConstraint(
     return { customList: location.getCustomList() };
   } else {
     const innerLocation = location.getLocation()?.toObject();
-    return innerLocation && convertFromRelayLocation(innerLocation);
+    return innerLocation && convertFromGeographicConstraint(innerLocation);
   }
 }
 
-function convertFromRelayLocation(location: grpcTypes.RelayLocation.AsObject): RelayLocation {
+function convertFromGeographicConstraint(location: grpcTypes.GeographicLocationConstraint.AsObject): RelayLocation {
   if (location.hostname) {
     return location;
   } else if (location.city) {
@@ -1520,15 +1520,15 @@ function convertToLocation(
   if (constraint && 'customList' in constraint && constraint.customList) {
     locationConstraint.setCustomList(constraint.customList);
   } else {
-    const location = constraint && convertToRelayLocation(constraint);
+    const location = constraint && convertToGeographicConstraint(constraint);
     locationConstraint.setLocation(location);
   }
 
   return locationConstraint;
 }
 
-function convertToRelayLocation(location: RelayLocation): grpcTypes.RelayLocation {
-  const relayLocation = new grpcTypes.RelayLocation();
+function convertToGeographicConstraint(location: RelayLocation): grpcTypes.GeographicLocationConstraint {
+  const relayLocation = new grpcTypes.GeographicLocationConstraint();
   if ('hostname' in location) {
     relayLocation.setCountry(location.country);
     relayLocation.setCity(location.city);
@@ -1678,7 +1678,7 @@ function convertFromCustomLists(customLists: Array<grpcTypes.CustomList>): Custo
     locations: list
       .getLocationsList()
       .map((location) =>
-        convertFromRelayLocation(location.toObject()),
+        convertFromGeographicConstraint(location.toObject()),
       ) as Array<RelayLocationGeographical>,
   }));
 }
@@ -1688,7 +1688,7 @@ function convertToCustomList(customList: ICustomList): grpcTypes.CustomList {
   grpcCustomList.setId(customList.id);
   grpcCustomList.setName(customList.name);
 
-  const locations = customList.locations.map(convertToRelayLocation);
+  const locations = customList.locations.map(convertToGeographicConstraint);
   grpcCustomList.setLocationsList(locations);
 
   return grpcCustomList;

mullvad-management-interface/proto/management_interface.proto

@@ -297,14 +297,14 @@ message BridgeSettings {
 message LocationConstraint {
   oneof type {
     string custom_list = 1;
-    RelayLocation location = 2;
+    GeographicLocationConstraint location = 2;
   }
 }
 
-message RelayLocation {
+message GeographicLocationConstraint {
   string country = 1;
-  string city = 2;
-  string hostname = 3;
+  optional string city = 2;
+  optional string hostname = 3;
 }
 
 message BridgeState {
@@ -331,7 +331,7 @@ message ObfuscationSettings {
 message CustomList {
   string id = 1;
   string name = 2;
-  repeated RelayLocation locations = 3;
+  repeated GeographicLocationConstraint locations = 3;
 }
 
 message CustomListSettings { repeated CustomList custom_lists = 1; }

mullvad-management-interface/src/types/conversions/custom_list.rs

@@ -33,7 +33,7 @@ impl From<mullvad_types::custom_list::CustomList> for proto::CustomList {
         let locations = custom_list
             .locations
             .into_iter()
-            .map(proto::RelayLocation::from)
+            .map(proto::GeographicLocationConstraint::from)
             .collect();
         Self {
             id: custom_list.id.to_string(),
@@ -60,39 +60,3 @@ impl TryFrom<proto::CustomList> for mullvad_types::custom_list::CustomList {
         })
     }
 }
-
-impl TryFrom<proto::RelayLocation> for GeographicLocationConstraint {
-    type Error = FromProtobufTypeError;
-
-    fn try_from(relay_location: proto::RelayLocation) -> Result<Self, Self::Error> {
-        match (
-            relay_location.country.as_ref(),
-            relay_location.city.as_ref(),
-            relay_location.hostname.as_ref(),
-        ) {
-            ("", ..) => Err(FromProtobufTypeError::InvalidArgument(
-                "Invalid geographic relay location",
-            )),
-            (_country, "", "") => Ok(GeographicLocationConstraint::Country(
-                relay_location.country,
-            )),
-            (_country, _city, "") => Ok(GeographicLocationConstraint::City(
-                relay_location.country,
-                relay_location.city,
-            )),
-            (_country, city, _hostname) => {
-                if city.is_empty() {
-                    Err(FromProtobufTypeError::InvalidArgument(
-                        "Relay location must contain a city if hostname is included",
-                    ))
-                } else {
-                    Ok(GeographicLocationConstraint::Hostname(
-                        relay_location.country,
-                        relay_location.city,
-                        relay_location.hostname,
-                    ))
-                }
-            }
-        }
-    }
-}

mullvad-management-interface/src/types/conversions/mod.rs

@@ -45,14 +45,6 @@ fn bytes_to_wg_key<'a>(
     <&[u8; 32]>::try_from(bytes).map_err(|_| FromProtobufTypeError::InvalidArgument(error_msg))
 }
 
-/// Returns `Option<String>`, where an empty string represents `None`.
-fn option_from_proto_string(s: String) -> Option<String> {
-    match s {
-        s if s.is_empty() => None,
-        s => Some(s),
-    }
-}
-
 fn arg_from_str<T: FromStr<Err = E>, E>(
     s: &str,
     invalid_arg_msg: &'static str,

mullvad-management-interface/src/types/conversions/relay_constraints.rs

@@ -1,10 +1,7 @@
-use crate::types::{
-    conversions::{net::try_tunnel_type_from_i32, option_from_proto_string},
-    proto, FromProtobufTypeError,
-};
+use crate::types::{conversions::net::try_tunnel_type_from_i32, proto, FromProtobufTypeError};
 use mullvad_types::{
     custom_list::Id,
-    relay_constraints::{Constraint, RelaySettingsUpdate},
+    relay_constraints::{Constraint, GeographicLocationConstraint, RelaySettingsUpdate},
 };
 use std::str::FromStr;
 use talpid_types::net::TunnelType;
@@ -479,7 +476,7 @@ impl From<mullvad_types::relay_constraints::LocationConstraint> for proto::Locat
         match location {
             LocationConstraint::Location(location) => Self {
                 r#type: Some(proto::location_constraint::Type::Location(
-                    proto::RelayLocation::from(location),
+                    proto::GeographicLocationConstraint::from(location),
                 )),
             },
             LocationConstraint::CustomList { list_id } => Self {
@@ -499,17 +496,9 @@ impl TryFrom<proto::LocationConstraint>
     fn try_from(location: proto::LocationConstraint) -> Result<Self, Self::Error> {
         use mullvad_types::relay_constraints::LocationConstraint;
         match location.r#type {
-            Some(proto::location_constraint::Type::Location(location)) => {
-                let location = Constraint::<
-                    mullvad_types::relay_constraints::GeographicLocationConstraint,
-                >::from(location);
-                match location {
-                    Constraint::Any => Ok(Constraint::Any),
-                    Constraint::Only(location) => {
-                        Ok(Constraint::Only(LocationConstraint::Location(location)))
-                    }
-                }
-            }
+            Some(proto::location_constraint::Type::Location(location)) => Ok(Constraint::Only(
+                LocationConstraint::Location(GeographicLocationConstraint::try_from(location)?),
+            )),
             Some(proto::location_constraint::Type::CustomList(list_id)) => {
                 let location = LocationConstraint::CustomList {
                     list_id: Id::from_str(&list_id).map_err(|_| {
@@ -523,47 +512,44 @@ impl TryFrom<proto::LocationConstraint>
     }
 }
 
-impl From<mullvad_types::relay_constraints::GeographicLocationConstraint> for proto::RelayLocation {
+impl From<GeographicLocationConstraint> for proto::GeographicLocationConstraint {
     fn from(location: mullvad_types::relay_constraints::GeographicLocationConstraint) -> Self {
-        use mullvad_types::relay_constraints::GeographicLocationConstraint;
-
         match location {
             GeographicLocationConstraint::Country(country) => Self {
                 country,
                 ..Default::default()
             },
             GeographicLocationConstraint::City(country, city) => Self {
                 country,
-                city,
-                ..Default::default()
+                city: Some(city),
+                hostname: None,
             },
             GeographicLocationConstraint::Hostname(country, city, hostname) => Self {
                 country,
-                city,
-                hostname,
+                city: Some(city),
+                hostname: Some(hostname),
             },
         }
     }
 }
 
-impl From<proto::RelayLocation>
-    for Constraint<mullvad_types::relay_constraints::GeographicLocationConstraint>
-{
-    fn from(location: proto::RelayLocation) -> Self {
-        use mullvad_types::relay_constraints::GeographicLocationConstraint;
-
-        if let Some(hostname) = option_from_proto_string(location.hostname) {
-            Constraint::Only(GeographicLocationConstraint::Hostname(
-                location.country,
-                location.city,
-                hostname,
-            ))
-        } else if let Some(city) = option_from_proto_string(location.city) {
-            Constraint::Only(GeographicLocationConstraint::City(location.country, city))
-        } else if let Some(country) = option_from_proto_string(location.country) {
-            Constraint::Only(GeographicLocationConstraint::Country(country))
-        } else {
-            Constraint::Any
+impl TryFrom<proto::GeographicLocationConstraint> for GeographicLocationConstraint {
+    type Error = FromProtobufTypeError;
+
+    fn try_from(relay_location: proto::GeographicLocationConstraint) -> Result<Self, Self::Error> {
+        match (
+            relay_location.country,
+            relay_location.city,
+            relay_location.hostname,
+        ) {
+            (country, None, None) => Ok(GeographicLocationConstraint::Country(country)),
+            (country, Some(city), None) => Ok(GeographicLocationConstraint::City(country, city)),
+            (country, Some(city), Some(hostname)) => Ok(GeographicLocationConstraint::Hostname(
+                country, city, hostname,
+            )),
+            (_country, None, Some(_hostname)) => Err(FromProtobufTypeError::InvalidArgument(
+                "Relay location contains hostname but no city",
+            )),
         }
     }
 }