diff options
23 files changed, 143 insertions, 67 deletions
diff --git a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/repository/CustomListsRepository.kt b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/repository/CustomListsRepository.kt index 32dd9a8e0e..6b8fd69ff8 100644 --- a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/repository/CustomListsRepository.kt +++ b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/repository/CustomListsRepository.kt @@ -28,7 +28,10 @@ class CustomListsRepository( .mapNotNull { it.customLists } .stateIn(CoroutineScope(dispatcher), SharingStarted.Eagerly, null) - suspend fun createCustomList(name: CustomListName) = managementService.createCustomList(name) + suspend fun createCustomList( + name: CustomListName, + locations: List<GeoLocationId> = emptyList(), + ) = managementService.createCustomList(name, locations) suspend fun deleteCustomList(id: CustomListId) = managementService.deleteCustomList(id) diff --git a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/usecase/customlists/CustomListActionUseCase.kt b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/usecase/customlists/CustomListActionUseCase.kt index b1326d57b9..da1b6fcb56 100644 --- a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/usecase/customlists/CustomListActionUseCase.kt +++ b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/usecase/customlists/CustomListActionUseCase.kt @@ -10,10 +10,8 @@ import net.mullvad.mullvadvpn.compose.communication.Deleted import net.mullvad.mullvadvpn.compose.communication.LocationsChanged import net.mullvad.mullvadvpn.compose.communication.Renamed import net.mullvad.mullvadvpn.lib.model.CreateCustomListError -import net.mullvad.mullvadvpn.lib.model.CustomList import net.mullvad.mullvadvpn.lib.model.DeleteCustomListError import net.mullvad.mullvadvpn.lib.model.GetCustomListError -import net.mullvad.mullvadvpn.lib.model.UpdateCustomListError import net.mullvad.mullvadvpn.lib.model.UpdateCustomListLocationsError import net.mullvad.mullvadvpn.lib.model.UpdateCustomListNameError import net.mullvad.mullvadvpn.relaylist.getRelayItemsByCodes @@ -54,23 +52,12 @@ class CustomListActionUseCase( ): Either<CreateWithLocationsError, Created> = either { val customListId = customListsRepository - .createCustomList(action.name) + .createCustomList(action.name, action.locations) .mapLeft(CreateWithLocationsError::Create) .bind() val locationNames = if (action.locations.isNotEmpty()) { - customListsRepository - .updateCustomList( - CustomList( - id = customListId, - name = action.name, - locations = action.locations, - ) - ) - .mapLeft(CreateWithLocationsError::UpdateCustomList) - .bind() - relayListRepository.relayList .firstOrNull() ?.getRelayItemsByCodes(action.locations) @@ -129,8 +116,6 @@ sealed interface CreateWithLocationsError : CustomListActionError { data class Create(val error: CreateCustomListError) : CreateWithLocationsError - data class UpdateCustomList(val error: UpdateCustomListError) : CreateWithLocationsError - data object UnableToFetchRelayList : CreateWithLocationsError } diff --git a/android/app/src/test/kotlin/net/mullvad/mullvadvpn/usecase/CustomListActionUseCaseTest.kt b/android/app/src/test/kotlin/net/mullvad/mullvadvpn/usecase/CustomListActionUseCaseTest.kt index 03d46cbde0..d9a687362a 100644 --- a/android/app/src/test/kotlin/net/mullvad/mullvadvpn/usecase/CustomListActionUseCaseTest.kt +++ b/android/app/src/test/kotlin/net/mullvad/mullvadvpn/usecase/CustomListActionUseCaseTest.kt @@ -62,12 +62,8 @@ class CustomListActionUseCaseTest { undo = action.not(createdId), ) .right() - coEvery { mockCustomListsRepository.createCustomList(name) } returns createdId.right() - coEvery { - mockCustomListsRepository.updateCustomList( - CustomList(createdId, name, listOf(locationId)) - ) - } returns Unit.right() + coEvery { mockCustomListsRepository.createCustomList(name, listOf(locationId)) } returns + createdId.right() relayListFlow.value = listOf( RelayItem.Location.Country( @@ -91,7 +87,7 @@ class CustomListActionUseCaseTest { val locationId = GeoLocationId.Country("AB") val action = CustomListAction.Create(name = name, locations = listOf(locationId)) val expectedError = CreateWithLocationsError.Create(CustomListAlreadyExists).left() - coEvery { mockCustomListsRepository.createCustomList(name) } returns + coEvery { mockCustomListsRepository.createCustomList(name, listOf(locationId)) } returns CustomListAlreadyExists.left() // Act diff --git a/android/lib/daemon-grpc/src/main/kotlin/net/mullvad/mullvadvpn/lib/daemon/grpc/ManagementService.kt b/android/lib/daemon-grpc/src/main/kotlin/net/mullvad/mullvadvpn/lib/daemon/grpc/ManagementService.kt index f306c9f833..cff4e2bd3e 100644 --- a/android/lib/daemon-grpc/src/main/kotlin/net/mullvad/mullvadvpn/lib/daemon/grpc/ManagementService.kt +++ b/android/lib/daemon-grpc/src/main/kotlin/net/mullvad/mullvadvpn/lib/daemon/grpc/ManagementService.kt @@ -76,6 +76,7 @@ import net.mullvad.mullvadvpn.lib.model.DnsOptions as ModelDnsOptions import net.mullvad.mullvadvpn.lib.model.DnsOptions import net.mullvad.mullvadvpn.lib.model.DnsState as ModelDnsState import net.mullvad.mullvadvpn.lib.model.DnsState +import net.mullvad.mullvadvpn.lib.model.GeoLocationId import net.mullvad.mullvadvpn.lib.model.GetAccountDataError import net.mullvad.mullvadvpn.lib.model.GetAccountHistoryError import net.mullvad.mullvadvpn.lib.model.GetDeviceListError @@ -576,9 +577,17 @@ class ManagementService( .mapEmpty() suspend fun createCustomList( - name: CustomListName + name: CustomListName, + locations: List<GeoLocationId> = emptyList(), ): Either<CreateCustomListError, CustomListId> = - Either.catch { grpc.createCustomList(StringValue.of(name.value)) } + Either.catch { + grpc.createCustomList( + ManagementInterface.NewCustomList.newBuilder() + .setName(name.value) + .addAllLocations(locations.map(GeoLocationId::fromDomain)) + .build() + ) + } .map { CustomListId(it.value) } .mapLeftStatus { when (it.status.code) { diff --git a/desktop/packages/mullvad-vpn/src/main/daemon-rpc.ts b/desktop/packages/mullvad-vpn/src/main/daemon-rpc.ts index 206070598c..35c1f775f4 100644 --- a/desktop/packages/mullvad-vpn/src/main/daemon-rpc.ts +++ b/desktop/packages/mullvad-vpn/src/main/daemon-rpc.ts @@ -24,6 +24,7 @@ import { IRelayListWithEndpointData, ISettings, NewAccessMethodSetting, + NewCustomList, ObfuscationSettings, ObfuscationType, RelaySettings, @@ -45,6 +46,7 @@ import { convertToCustomList, convertToCustomProxy, convertToNewApiAccessMethodSetting, + convertToNewCustomList, convertToNormalBridgeSettings, convertToRelayConstraints, ensureExists, @@ -573,9 +575,12 @@ export class DaemonRpc extends GrpcClient { await this.call<grpcTypes.DeviceRemoval, Empty>(this.client.removeDevice, grpcDeviceRemoval); } - public async createCustomList(name: string): Promise<void | CustomListError> { + public async createCustomList(newCustomList: NewCustomList): Promise<void | CustomListError> { try { - await this.callString<Empty>(this.client.createCustomList, name); + await this.call<grpcTypes.NewCustomList, StringValue>( + this.client.createCustomList, + convertToNewCustomList(newCustomList), + ); } catch (e) { const error = e as grpc.ServiceError; if (error.code === 6) { diff --git a/desktop/packages/mullvad-vpn/src/main/grpc-type-convertions.ts b/desktop/packages/mullvad-vpn/src/main/grpc-type-convertions.ts index 64c805c3b9..c5c6ccf4d2 100644 --- a/desktop/packages/mullvad-vpn/src/main/grpc-type-convertions.ts +++ b/desktop/packages/mullvad-vpn/src/main/grpc-type-convertions.ts @@ -47,6 +47,7 @@ import { LoggedInDeviceState, LoggedOutDeviceState, NewAccessMethodSetting, + NewCustomList, ObfuscationSettings, ObfuscationType, Ownership, @@ -1345,6 +1346,14 @@ function convertFromSocksAuth(auth: grpcTypes.SocksAuth): SocksAuth { }; } +export function convertToNewCustomList(customList: NewCustomList): grpcTypes.NewCustomList { + const newCustomList = new grpcTypes.NewCustomList(); + newCustomList.setName(customList.name); + const locations = customList.locations.map(convertToGeographicConstraint); + newCustomList.setLocationsList(locations); + return newCustomList; +} + export function ensureExists<T>(value: T | undefined, errorMessage: string): T { if (value) { return value; diff --git a/desktop/packages/mullvad-vpn/src/renderer/app.tsx b/desktop/packages/mullvad-vpn/src/renderer/app.tsx index 7ceab22c2f..8c39327f40 100644 --- a/desktop/packages/mullvad-vpn/src/renderer/app.tsx +++ b/desktop/packages/mullvad-vpn/src/renderer/app.tsx @@ -29,6 +29,7 @@ import { ISettings, liftConstraint, NewAccessMethodSetting, + NewCustomList, ObfuscationSettings, RelaySettings, TunnelState, @@ -408,8 +409,8 @@ export default class AppRenderer { public getPathBaseName = (path: string) => IpcRendererEventChannel.app.getPathBaseName(path); public showOpenDialog = (options: Electron.OpenDialogOptions) => IpcRendererEventChannel.app.showOpenDialog(options); - public createCustomList = (name: string) => - IpcRendererEventChannel.customLists.createCustomList(name); + public createCustomList = (newCustomList: NewCustomList) => + IpcRendererEventChannel.customLists.createCustomList(newCustomList); public deleteCustomList = (id: string) => IpcRendererEventChannel.customLists.deleteCustomList(id); public updateCustomList = (customList: ICustomList) => diff --git a/desktop/packages/mullvad-vpn/src/renderer/components/select-location/CustomLists.tsx b/desktop/packages/mullvad-vpn/src/renderer/components/select-location/CustomLists.tsx index c16badd158..cd535dd91e 100644 --- a/desktop/packages/mullvad-vpn/src/renderer/components/select-location/CustomLists.tsx +++ b/desktop/packages/mullvad-vpn/src/renderer/components/select-location/CustomLists.tsx @@ -81,7 +81,10 @@ export default function CustomLists(props: CustomListsProps) { const createList = useCallback( async (name: string): Promise<void | CustomListError> => { - const result = await createCustomList(name); + const result = await createCustomList({ + name, + locations: [], + }); // If an error is returned it should be passed as the return value. if (result) { return result; diff --git a/desktop/packages/mullvad-vpn/src/shared/daemon-rpc-types.ts b/desktop/packages/mullvad-vpn/src/shared/daemon-rpc-types.ts index b00fe4587a..45ed498237 100644 --- a/desktop/packages/mullvad-vpn/src/shared/daemon-rpc-types.ts +++ b/desktop/packages/mullvad-vpn/src/shared/daemon-rpc-types.ts @@ -474,6 +474,8 @@ export interface ICustomList { locations: Array<RelayLocationGeographical>; } +export type NewCustomList = Pick<ICustomList, 'name' | 'locations'>; + export type CustomListError = { type: 'name already exists' }; export interface ISettings { diff --git a/desktop/packages/mullvad-vpn/src/shared/ipc-schema.ts b/desktop/packages/mullvad-vpn/src/shared/ipc-schema.ts index a85f55d18a..031d1fc593 100644 --- a/desktop/packages/mullvad-vpn/src/shared/ipc-schema.ts +++ b/desktop/packages/mullvad-vpn/src/shared/ipc-schema.ts @@ -20,6 +20,7 @@ import { IRelayListWithEndpointData, ISettings, NewAccessMethodSetting, + NewCustomList, ObfuscationSettings, RelaySettings, TunnelState, @@ -147,7 +148,7 @@ export const ipcSchema = { '': notifyRenderer<IRelayListWithEndpointData>(), }, customLists: { - createCustomList: invoke<string, void | CustomListError>(), + createCustomList: invoke<NewCustomList, void | CustomListError>(), deleteCustomList: invoke<string, void>(), updateCustomList: invoke<ICustomList, void | CustomListError>(), }, diff --git a/mullvad-cli/src/cmds/bridge.rs b/mullvad-cli/src/cmds/bridge.rs index 48458c9c1e..b5c436a893 100644 --- a/mullvad-cli/src/cmds/bridge.rs +++ b/mullvad-cli/src/cmds/bridge.rs @@ -155,7 +155,7 @@ impl Bridge { let list = super::custom_list::find_list_by_name(&mut rpc, &custom_list_name).await?; let location = - Constraint::Only(LocationConstraint::CustomList { list_id: list.id }); + Constraint::Only(LocationConstraint::CustomList { list_id: list.id() }); Self::update_bridge_settings(&mut rpc, Some(location), None, None).await } SetCommands::Ownership { ownership } => { diff --git a/mullvad-cli/src/cmds/custom_list.rs b/mullvad-cli/src/cmds/custom_list.rs index edc6b4b0e2..06e177c634 100644 --- a/mullvad-cli/src/cmds/custom_list.rs +++ b/mullvad-cli/src/cmds/custom_list.rs @@ -160,7 +160,7 @@ impl CustomList { async fn delete_list(name: String) -> Result<()> { let mut rpc = MullvadProxyClient::new().await?; let list = find_list_by_name(&mut rpc, &name).await?; - rpc.delete_custom_list(list.id.to_string()).await?; + rpc.delete_custom_list(list.id()).await?; Ok(()) } diff --git a/mullvad-cli/src/cmds/relay.rs b/mullvad-cli/src/cmds/relay.rs index 43010862a9..30d041ea86 100644 --- a/mullvad-cli/src/cmds/relay.rs +++ b/mullvad-cli/src/cmds/relay.rs @@ -587,7 +587,7 @@ impl Relay { let mut rpc = MullvadProxyClient::new().await?; let list_id = super::custom_list::find_list_by_name(&mut rpc, &custom_list_name) .await? - .id; + .id(); Self::update_constraints(|constraints| { constraints.location = Constraint::Only(LocationConstraint::CustomList { list_id }); }) @@ -685,7 +685,7 @@ impl Relay { Some(EntryArgs::CustomList { custom_list_name }) => { let list_id = super::custom_list::find_list_by_name(&mut rpc, &custom_list_name) .await? - .id; + .id(); wireguard_constraints.entry_location = Constraint::Only(LocationConstraint::CustomList { list_id }); } diff --git a/mullvad-daemon/src/custom_list.rs b/mullvad-daemon/src/custom_list.rs index ba662a2dac..06f5926a2c 100644 --- a/mullvad-daemon/src/custom_list.rs +++ b/mullvad-daemon/src/custom_list.rs @@ -1,19 +1,27 @@ use crate::{Daemon, Error}; use mullvad_relay_selector::SelectorConfig; +use mullvad_types::relay_constraints::GeographicLocationConstraint; use mullvad_types::{ constraints::Constraint, custom_list::{CustomList, Id}, relay_constraints::{BridgeState, LocationConstraint, RelaySettings, ResolvedBridgeSettings}, }; +use std::collections::BTreeSet; use talpid_types::net::TunnelType; impl Daemon { /// Create a new custom list. /// /// Returns an error if the name is not unique. - pub async fn create_custom_list(&mut self, name: String) -> Result<Id, crate::Error> { - let new_list = CustomList::new(name).map_err(crate::Error::CustomListError)?; - let id = new_list.id; + pub async fn create_custom_list( + &mut self, + name: String, + locations: BTreeSet<GeographicLocationConstraint>, + ) -> Result<Id, crate::Error> { + let mut new_list = CustomList::new(name).map_err(crate::Error::CustomListError)?; + new_list.append(locations); + + let id = new_list.id(); self.settings .try_update(|settings| settings.custom_lists.add(new_list)) @@ -57,7 +65,7 @@ impl Daemon { /// - there is no existing list with the same ID, /// - or the existing list has a different name. pub async fn update_custom_list(&mut self, new_list: CustomList) -> Result<(), Error> { - let list_id = new_list.id; + let list_id = new_list.id(); let settings_changed = self .settings .try_update(|settings| settings.custom_lists.update(new_list)) diff --git a/mullvad-daemon/src/lib.rs b/mullvad-daemon/src/lib.rs index f73ab6927c..5550ef3a8a 100644 --- a/mullvad-daemon/src/lib.rs +++ b/mullvad-daemon/src/lib.rs @@ -45,6 +45,7 @@ use mullvad_encrypted_dns_proxy::state::EncryptedDnsProxyState; use mullvad_relay_selector::{RelaySelector, SelectorConfig}; #[cfg(target_os = "android")] use mullvad_types::account::{PlayPurchase, PlayPurchasePaymentToken}; +use mullvad_types::relay_constraints::GeographicLocationConstraint; #[cfg(any(windows, target_os = "android", target_os = "macos"))] use mullvad_types::settings::SplitApp; #[cfg(daita)] @@ -70,6 +71,7 @@ use mullvad_types::{ }; use relay_list::{RelayListUpdater, RelayListUpdaterHandle, RELAYS_FILENAME}; use settings::SettingsPersister; +use std::collections::BTreeSet; #[cfg(any(windows, target_os = "android", target_os = "macos"))] use std::collections::HashSet; #[cfg(target_os = "android")] @@ -310,7 +312,11 @@ pub enum DaemonCommand { /// Return a public key of the currently set wireguard private key, if there is one GetWireguardKey(ResponseTx<Option<PublicKey>, Error>), /// Create custom list - CreateCustomList(ResponseTx<mullvad_types::custom_list::Id, Error>, String), + CreateCustomList( + ResponseTx<mullvad_types::custom_list::Id, Error>, + String, + BTreeSet<GeographicLocationConstraint>, + ), /// Delete custom list DeleteCustomList(ResponseTx<(), Error>, mullvad_types::custom_list::Id), /// Update a custom list with a given id @@ -1442,7 +1448,9 @@ impl Daemon { ResetSettings(tx) => self.on_reset_settings(tx).await, RotateWireguardKey(tx) => self.on_rotate_wireguard_key(tx), GetWireguardKey(tx) => self.on_get_wireguard_key(tx).await, - CreateCustomList(tx, name) => self.on_create_custom_list(tx, name).await, + CreateCustomList(tx, name, locations) => { + self.on_create_custom_list(tx, name, locations).await + } DeleteCustomList(tx, id) => self.on_delete_custom_list(tx, id).await, UpdateCustomList(tx, update) => self.on_update_custom_list(tx, update).await, ClearCustomLists(tx) => self.on_clear_custom_lists(tx).await, @@ -2865,8 +2873,9 @@ impl Daemon { &mut self, tx: ResponseTx<mullvad_types::custom_list::Id, Error>, name: String, + locations: BTreeSet<GeographicLocationConstraint>, ) { - let result = self.create_custom_list(name).await; + let result = self.create_custom_list(name, locations).await; Self::oneshot_send(tx, result, "create_custom_list response"); } diff --git a/mullvad-daemon/src/management_interface.rs b/mullvad-daemon/src/management_interface.rs index 5695683fcf..aef1d17c45 100644 --- a/mullvad-daemon/src/management_interface.rs +++ b/mullvad-daemon/src/management_interface.rs @@ -4,10 +4,12 @@ use futures::{ StreamExt, }; use mullvad_api::{rest::Error as RestError, StatusCode}; +use mullvad_management_interface::types::FromProtobufTypeError; use mullvad_management_interface::{ types::{self, daemon_event, management_service_server::ManagementService}, Code, Request, Response, ServerJoinHandle, Status, }; +use mullvad_types::relay_constraints::GeographicLocationConstraint; use mullvad_types::{ account::AccountNumber, relay_constraints::{ @@ -20,6 +22,7 @@ use mullvad_types::{ version, wireguard::{RotationInterval, RotationIntervalError}, }; +use std::collections::BTreeSet; use std::{ path::Path, str::FromStr, @@ -663,13 +666,22 @@ impl ManagementService for ManagementServiceImpl { // Custom lists // - async fn create_custom_list(&self, request: Request<String>) -> ServiceResult<String> { + async fn create_custom_list( + &self, + request: Request<types::NewCustomList>, + ) -> ServiceResult<String> { log::debug!("create_custom_list"); + let request = request.into_inner(); + let locations = request + .locations + .into_iter() + .map(GeographicLocationConstraint::try_from) + .collect::<Result<BTreeSet<_>, FromProtobufTypeError>>()?; let (tx, rx) = oneshot::channel(); - self.send_command_to_daemon(DaemonCommand::CreateCustomList(tx, request.into_inner()))?; + self.send_command_to_daemon(DaemonCommand::CreateCustomList(tx, request.name, locations))?; self.wait_for_result(rx) .await? - .map(|response| Response::new(response.to_string())) + .map(|id| Response::new(id.to_string())) .map_err(map_daemon_error) } diff --git a/mullvad-management-interface/proto/management_interface.proto b/mullvad-management-interface/proto/management_interface.proto index ab9883c1ef..14a11fff4e 100644 --- a/mullvad-management-interface/proto/management_interface.proto +++ b/mullvad-management-interface/proto/management_interface.proto @@ -81,7 +81,7 @@ service ManagementService { rpc GetWireguardKey(google.protobuf.Empty) returns (PublicKey) {} // Custom lists - rpc CreateCustomList(google.protobuf.StringValue) returns (google.protobuf.StringValue) {} + rpc CreateCustomList(NewCustomList) returns (google.protobuf.StringValue) {} rpc DeleteCustomList(google.protobuf.StringValue) returns (google.protobuf.Empty) {} rpc UpdateCustomList(CustomList) returns (google.protobuf.Empty) {} rpc ClearCustomLists(google.protobuf.Empty) returns (google.protobuf.Empty) {} @@ -442,6 +442,11 @@ message CustomList { repeated GeographicLocationConstraint locations = 3; } +message NewCustomList { + string name = 1; + repeated GeographicLocationConstraint locations = 2; +} + message CustomListSettings { repeated CustomList custom_lists = 1; } message Socks5Local { diff --git a/mullvad-management-interface/src/client.rs b/mullvad-management-interface/src/client.rs index 73129d6b99..93a31a5042 100644 --- a/mullvad-management-interface/src/client.rs +++ b/mullvad-management-interface/src/client.rs @@ -571,18 +571,22 @@ impl MullvadProxyClient { } pub async fn create_custom_list(&mut self, name: String) -> Result<Id> { + let request = types::NewCustomList { + name, + locations: Vec::new(), + }; let id = self .0 - .create_custom_list(name) + .create_custom_list(request) .await .map_err(map_custom_list_error)? .into_inner(); Id::from_str(&id).map_err(|_| Error::CustomListListNotFound) } - pub async fn delete_custom_list(&mut self, id: String) -> Result<()> { + pub async fn delete_custom_list(&mut self, id: Id) -> Result<()> { self.0 - .delete_custom_list(id) + .delete_custom_list(id.to_string()) .await .map_err(map_custom_list_error)?; Ok(()) diff --git a/mullvad-management-interface/src/types/conversions/custom_list.rs b/mullvad-management-interface/src/types/conversions/custom_list.rs index 9d4e9cee78..11f2c4bd76 100644 --- a/mullvad-management-interface/src/types/conversions/custom_list.rs +++ b/mullvad-management-interface/src/types/conversions/custom_list.rs @@ -30,13 +30,14 @@ impl TryFrom<proto::CustomListSettings> for mullvad_types::custom_list::CustomLi impl From<mullvad_types::custom_list::CustomList> for proto::CustomList { fn from(custom_list: mullvad_types::custom_list::CustomList) -> Self { + let id = custom_list.id().to_string(); let locations = custom_list .locations .into_iter() .map(proto::GeographicLocationConstraint::from) .collect(); Self { - id: custom_list.id.to_string(), + id, name: custom_list.name, locations, } @@ -52,11 +53,14 @@ impl TryFrom<proto::CustomList> for mullvad_types::custom_list::CustomList { .into_iter() .map(GeographicLocationConstraint::try_from) .collect::<Result<BTreeSet<_>, Self::Error>>()?; - Ok(Self { - id: Id::from_str(&custom_list.id) - .map_err(|_| FromProtobufTypeError::InvalidArgument("Invalid list ID"))?, - name: custom_list.name, - locations, - }) + + let id = Id::from_str(&custom_list.id) + .map_err(|_| FromProtobufTypeError::InvalidArgument("Invalid list ID"))?; + + let mut inner = Self::with_id(id); + inner.name = custom_list.name; + inner.append(locations); + + Ok(inner) } } diff --git a/mullvad-relay-selector/src/relay_selector/matcher.rs b/mullvad-relay-selector/src/relay_selector/matcher.rs index 88126137b0..e14885f2c4 100644 --- a/mullvad-relay-selector/src/relay_selector/matcher.rs +++ b/mullvad-relay-selector/src/relay_selector/matcher.rs @@ -235,7 +235,7 @@ impl<'a> ResolvedLocationConstraint<'a> { } LocationConstraint::CustomList { list_id } => custom_lists .iter() - .find(|list| list.id == *list_id) + .find(|list| list.id() == *list_id) .map(|custom_list| { ResolvedLocationConstraint(custom_list.locations.iter().collect()) }) diff --git a/mullvad-types/src/custom_list.rs b/mullvad-types/src/custom_list.rs index 581c049be3..ed0176db50 100644 --- a/mullvad-types/src/custom_list.rs +++ b/mullvad-types/src/custom_list.rs @@ -20,7 +20,7 @@ pub enum Error { ListExists, } -#[derive(Default, Debug, Clone, Copy, Serialize, Deserialize, PartialEq, Eq, PartialOrd, Ord)] +#[derive(Debug, Clone, Copy, Serialize, Deserialize, PartialEq, Eq, PartialOrd, Ord)] pub struct Id(uuid::Uuid); impl Deref for Id { @@ -138,21 +138,41 @@ impl DerefMut for CustomListsSettings { #[derive(Debug, Clone, Serialize, Deserialize, PartialEq)] pub struct CustomList { - pub id: Id, + id: Id, pub name: String, pub locations: BTreeSet<GeographicLocationConstraint>, } impl CustomList { + /// Create a new [CustomList] with a given name. This function will check that the name + /// is appropriate (see implementation for details) and generate a new unique [Id]. pub fn new(name: String) -> Result<Self, Error> { if name.chars().count() > CUSTOM_LIST_NAME_MAX_SIZE { return Err(Error::NameTooLong); } - Ok(CustomList { - id: Id(uuid::Uuid::new_v4()), - name, - locations: BTreeSet::new(), - }) + let id = Id(uuid::Uuid::new_v4()); + let mut custom_list = Self::with_id(id); + custom_list.name = name; + + Ok(custom_list) + } + + /// Instantiate an empty custom list with a pre-existing [Id]. This is useful when + /// serializing/deserializing, and most likely you want to use [CustomList::new] instead. + pub fn with_id(id: Id) -> Self { + Self { + id, + name: Default::default(), + locations: Default::default(), + } + } + + pub fn id(&self) -> Id { + self.id + } + + pub fn append(&mut self, mut locations: BTreeSet<GeographicLocationConstraint>) { + self.locations.append(&mut locations); } } diff --git a/mullvad-types/src/relay_constraints.rs b/mullvad-types/src/relay_constraints.rs index c0b6e31926..192386ddec 100644 --- a/mullvad-types/src/relay_constraints.rs +++ b/mullvad-types/src/relay_constraints.rs @@ -109,7 +109,7 @@ impl fmt::Display for LocationConstraintFormatter<'_> { LocationConstraint::CustomList { list_id } => self .custom_lists .iter() - .find(|list| &list.id == list_id) + .find(|list| list.id() == *list_id) .map(|custom_list| write!(f, "{}", custom_list.name)) .unwrap_or_else(|| write!(f, "invalid custom list")), } diff --git a/test/test-manager/src/tests/mod.rs b/test/test-manager/src/tests/mod.rs index 0858ffd9fa..fcf02ac0f6 100644 --- a/test/test-manager/src/tests/mod.rs +++ b/test/test-manager/src/tests/mod.rs @@ -191,7 +191,7 @@ pub async fn set_test_location( let mut custom_list = find_custom_list(mullvad_client, &custom_list_name).await?; - assert_eq!(list_id, custom_list.id); + assert_eq!(list_id, custom_list.id()); for location in locations { custom_list.locations.insert(location); } |