From 6effbc46664b741f702adc0168053f46fa44dd41 Mon Sep 17 00:00:00 2001 From: Jonatan Rhodin Date: Wed, 7 Feb 2024 23:20:47 +0100 Subject: [PATCH 01/11] Add support for custom lists in jni --- mullvad-jni/src/classes.rs | 2 + mullvad-jni/src/daemon_interface.rs | 31 +++++++++++++ mullvad-jni/src/lib.rs | 70 +++++++++++++++++++++++++++++ mullvad-types/src/custom_list.rs | 47 +++++++++++++++++++ mullvad-types/src/settings/mod.rs | 1 - 5 files changed, 150 insertions(+), 1 deletion(-) diff --git a/mullvad-jni/src/classes.rs b/mullvad-jni/src/classes.rs index 454e0e6c6281..562fe786f057 100644 --- a/mullvad-jni/src/classes.rs +++ b/mullvad-jni/src/classes.rs @@ -9,6 +9,8 @@ pub const CLASSES: &[&str] = &[ "net/mullvad/mullvadvpn/model/AppVersionInfo", "net/mullvad/mullvadvpn/model/Constraint$Any", "net/mullvad/mullvadvpn/model/Constraint$Only", + "net/mullvad/mullvadvpn/model/CustomList", + "net/mullvad/mullvadvpn/model/CustomListsSettings", "net/mullvad/mullvadvpn/model/DnsState", "net/mullvad/mullvadvpn/model/DnsOptions", "net/mullvad/mullvadvpn/model/CustomDnsOptions", diff --git a/mullvad-jni/src/daemon_interface.rs b/mullvad-jni/src/daemon_interface.rs index 7ecc91585a68..7abd35cf5b5e 100644 --- a/mullvad-jni/src/daemon_interface.rs +++ b/mullvad-jni/src/daemon_interface.rs @@ -2,6 +2,7 @@ use futures::{channel::oneshot, executor::block_on}; use mullvad_daemon::{device, DaemonCommand, DaemonCommandSender}; use mullvad_types::{ account::{AccountData, AccountToken, PlayPurchase, VoucherSubmission}, + custom_list::CustomList, device::{Device, DeviceState}, relay_constraints::{ObfuscationSettings, RelaySettings}, relay_list::RelayList, @@ -354,6 +355,36 @@ impl DaemonInterface { .map_err(|_| Error::UpdateSettings) } + pub fn create_custom_list(&self, name: String) -> Result { + let (tx, rx) = oneshot::channel(); + + self.send_command(DaemonCommand::CreateCustomList(tx, name))?; + + block_on(rx) + .map_err(|_| Error::NoResponse)? + .map_err(Error::from) + } + + pub fn delete_custom_list(&self, id: mullvad_types::custom_list::Id) -> Result<()> { + let (tx, rx) = oneshot::channel(); + + self.send_command(DaemonCommand::DeleteCustomList(tx, id))?; + + block_on(rx) + .map_err(|_| Error::NoResponse)? + .map_err(Error::from) + } + + pub fn update_custom_list(&self, custom_list: CustomList) -> Result<()> { + let (tx, rx) = oneshot::channel(); + + self.send_command(DaemonCommand::UpdateCustomList(tx, custom_list))?; + + block_on(rx) + .map_err(|_| Error::NoResponse)? + .map_err(Error::from) + } + fn send_command(&self, command: DaemonCommand) -> Result<()> { self.command_sender.send(command).map_err(Error::NoDaemon) } diff --git a/mullvad-jni/src/lib.rs b/mullvad-jni/src/lib.rs index a41b8d664310..1416fd70b505 100644 --- a/mullvad-jni/src/lib.rs +++ b/mullvad-jni/src/lib.rs @@ -24,6 +24,7 @@ use mullvad_daemon::{ }; use mullvad_types::{ account::{AccountData, PlayPurchase, VoucherSubmission}, + custom_list::CustomList, settings::DnsOptions, }; use std::{ @@ -1338,6 +1339,75 @@ pub extern "system" fn Java_net_mullvad_mullvadvpn_service_MullvadDaemon_setQuan } } +#[no_mangle] +#[allow(non_snake_case)] +pub extern "system" fn Java_net_mullvad_mullvadvpn_service_MullvadDaemon_createCustomList<'env>( + env: JNIEnv<'env>, + _: JObject<'_>, + daemon_interface_address: jlong, + name: JString<'_>, +) -> JObject<'env> { + let env = JnixEnv::from(env); + + // SAFETY: The address points to an instance valid for the duration of this function call + if let Some(daemon_interface) = unsafe { get_daemon_interface(daemon_interface_address) } { + let name = String::from_java(&env, name); + match daemon_interface.create_custom_list(name) { + Ok(id) => id.into_java(&env).forget(), + Err(error) => { + log_request_error("create custom list", &error); + JObject::null() + } + } + } else { + JObject::null() + } +} + +#[no_mangle] +#[allow(non_snake_case)] +pub extern "system" fn Java_net_mullvad_mullvadvpn_service_MullvadDaemon_deleteCustomList( + env: JNIEnv<'_>, + _: JObject<'_>, + daemon_interface_address: jlong, + id: JString<'_>, +) { + let env = JnixEnv::from(env); + + // SAFETY: The address points to an instance valid for the duration of this function call + if let Some(daemon_interface) = unsafe { get_daemon_interface(daemon_interface_address) } { + let id = mullvad_types::custom_list::Id::from_java(&env, id); + if let Err(error) = daemon_interface.delete_custom_list(id) { + log::error!( + "{}", + error.display_chain_with_msg("Failed to delete custom list") + ); + } + } +} + +#[no_mangle] +#[allow(non_snake_case)] +pub extern "system" fn Java_net_mullvad_mullvadvpn_service_MullvadDaemon_updateCustomList( + env: JNIEnv<'_>, + _: JObject<'_>, + daemon_interface_address: jlong, + customList: JObject<'_>, +) { + let env = JnixEnv::from(env); + + // SAFETY: The address points to an instance valid for the duration of this function call + if let Some(daemon_interface) = unsafe { get_daemon_interface(daemon_interface_address) } { + let list = CustomList::from_java(&env, customList); + if let Err(error) = daemon_interface.update_custom_list(list) { + log::error!( + "{}", + error.display_chain_with_msg("Failed to update custom list") + ); + } + } +} + fn log_request_error(request: &str, error: &daemon_interface::Error) { match error { daemon_interface::Error::Api(RestError::Aborted) => { diff --git a/mullvad-types/src/custom_list.rs b/mullvad-types/src/custom_list.rs index 58fd046a9e50..b92f128a3bd3 100644 --- a/mullvad-types/src/custom_list.rs +++ b/mullvad-types/src/custom_list.rs @@ -77,6 +77,8 @@ impl<'borrow, 'env: 'borrow> IntoJava<'borrow, 'env> for Id { } } +#[cfg_attr(target_os = "android", derive(IntoJava, FromJava))] +#[cfg_attr(target_os = "android", jnix(package = "net.mullvad.mullvadvpn.model"))] #[derive(Default, Debug, Clone, Serialize, Deserialize, PartialEq)] pub struct CustomListsSettings { custom_lists: Vec, @@ -122,9 +124,17 @@ impl DerefMut for CustomListsSettings { } #[derive(Debug, Clone, Serialize, Deserialize, PartialEq)] +#[cfg_attr(target_os = "android", derive(IntoJava))] +#[cfg_attr(target_os = "android", jnix(package = "net.mullvad.mullvadvpn.model"))] pub struct CustomList { pub id: Id, pub name: String, + #[cfg_attr( + target_os = "android", + jnix( + map = "|locations| locations.into_iter().collect::>()" + ) + )] pub locations: BTreeSet, } @@ -137,3 +147,40 @@ impl CustomList { } } } + +#[cfg(target_os = "android")] +impl<'env, 'sub_env> FromJava<'env, JObject<'sub_env>> for CustomList +where + 'env: 'sub_env, +{ + const JNI_SIGNATURE: &'static str = "Lnet/mullvad/mullvadvpn/model/CustomList;"; + + fn from_java(env: &JnixEnv<'env>, source: JObject<'sub_env>) -> Self { + let object_id = env + .call_method(source, "component1", "()Ljava/lang/String;", &[]) + .expect("missing CustomList.id") + .l() + .expect("CustomList.id did not return an object"); + let id = Id::from_str(&String::from_java(env, object_id)).expect("invalid ID"); + + let object_name = env + .call_method(source, "component2", "()Ljava/lang/String;", &[]) + .expect("missing CustomList.name") + .l() + .expect("CustomList.name did not return an object"); + let name = String::from_java(env, object_name); + + let object_locations = env + .call_method(source, "component3", "()Ljava/util/ArrayList;", &[]) + .expect("missing CustomList.locations") + .l() + .expect("CustomList.locations did not return an object"); + let locations = BTreeSet::from_iter(Vec::from_java(env, object_locations)); + + CustomList { + id, + name, + locations, + } + } +} diff --git a/mullvad-types/src/settings/mod.rs b/mullvad-types/src/settings/mod.rs index 3adde14b5d6b..607e1d9539da 100644 --- a/mullvad-types/src/settings/mod.rs +++ b/mullvad-types/src/settings/mod.rs @@ -77,7 +77,6 @@ pub struct Settings { #[cfg_attr(target_os = "android", jnix(skip))] pub bridge_state: BridgeState, /// All of the custom relay lists - #[cfg_attr(target_os = "android", jnix(skip))] pub custom_lists: CustomListsSettings, /// API access methods #[cfg_attr(target_os = "android", jnix(skip))] From 066742e2ba964b9b651c6b88e2ca840bdf47d89a Mon Sep 17 00:00:00 2001 From: Jonatan Rhodin Date: Wed, 7 Feb 2024 23:22:42 +0100 Subject: [PATCH 02/11] Add custom list model --- .../kotlin/net/mullvad/mullvadvpn/model/CustomList.kt | 11 +++++++++++ .../mullvad/mullvadvpn/model/CustomListsSettings.kt | 6 ++++++ .../kotlin/net/mullvad/mullvadvpn/model/Settings.kt | 1 + 3 files changed, 18 insertions(+) create mode 100644 android/lib/model/src/main/kotlin/net/mullvad/mullvadvpn/model/CustomList.kt create mode 100644 android/lib/model/src/main/kotlin/net/mullvad/mullvadvpn/model/CustomListsSettings.kt diff --git a/android/lib/model/src/main/kotlin/net/mullvad/mullvadvpn/model/CustomList.kt b/android/lib/model/src/main/kotlin/net/mullvad/mullvadvpn/model/CustomList.kt new file mode 100644 index 000000000000..cdfa1b96878f --- /dev/null +++ b/android/lib/model/src/main/kotlin/net/mullvad/mullvadvpn/model/CustomList.kt @@ -0,0 +1,11 @@ +package net.mullvad.mullvadvpn.model + +import android.os.Parcelable +import kotlinx.parcelize.Parcelize + +@Parcelize +data class CustomList( + val id: String, + val name: String, + val locations: ArrayList +) : Parcelable diff --git a/android/lib/model/src/main/kotlin/net/mullvad/mullvadvpn/model/CustomListsSettings.kt b/android/lib/model/src/main/kotlin/net/mullvad/mullvadvpn/model/CustomListsSettings.kt new file mode 100644 index 000000000000..8a8c03ef054b --- /dev/null +++ b/android/lib/model/src/main/kotlin/net/mullvad/mullvadvpn/model/CustomListsSettings.kt @@ -0,0 +1,6 @@ +package net.mullvad.mullvadvpn.model + +import android.os.Parcelable +import kotlinx.parcelize.Parcelize + +@Parcelize data class CustomListsSettings(val customLists: ArrayList) : Parcelable diff --git a/android/lib/model/src/main/kotlin/net/mullvad/mullvadvpn/model/Settings.kt b/android/lib/model/src/main/kotlin/net/mullvad/mullvadvpn/model/Settings.kt index 0d45b38179dc..304edc404a2a 100644 --- a/android/lib/model/src/main/kotlin/net/mullvad/mullvadvpn/model/Settings.kt +++ b/android/lib/model/src/main/kotlin/net/mullvad/mullvadvpn/model/Settings.kt @@ -7,6 +7,7 @@ import kotlinx.parcelize.Parcelize data class Settings( val relaySettings: RelaySettings, val obfuscationSettings: ObfuscationSettings, + val customLists: CustomListsSettings, val allowLan: Boolean, val autoConnect: Boolean, val tunnelOptions: TunnelOptions, From a969af0ed6c820d47f15d66009ec38926159288b Mon Sep 17 00:00:00 2001 From: Jonatan Rhodin Date: Wed, 7 Feb 2024 23:23:41 +0100 Subject: [PATCH 03/11] Add custom lists to the service --- .../mullvadvpn/service/MullvadDaemon.kt | 20 ++++++++ .../service/endpoint/CustomLists.kt | 49 +++++++++++++++++++ .../service/endpoint/RelayListListener.kt | 8 +-- .../service/endpoint/ServiceEndpoint.kt | 2 + 4 files changed, 72 insertions(+), 7 deletions(-) create mode 100644 android/service/src/main/kotlin/net/mullvad/mullvadvpn/service/endpoint/CustomLists.kt diff --git a/android/service/src/main/kotlin/net/mullvad/mullvadvpn/service/MullvadDaemon.kt b/android/service/src/main/kotlin/net/mullvad/mullvadvpn/service/MullvadDaemon.kt index 52e662c77194..d09287fbf234 100644 --- a/android/service/src/main/kotlin/net/mullvad/mullvadvpn/service/MullvadDaemon.kt +++ b/android/service/src/main/kotlin/net/mullvad/mullvadvpn/service/MullvadDaemon.kt @@ -5,6 +5,7 @@ import kotlinx.coroutines.flow.asSharedFlow import net.mullvad.mullvadvpn.lib.endpoint.ApiEndpoint import net.mullvad.mullvadvpn.lib.endpoint.ApiEndpointConfiguration import net.mullvad.mullvadvpn.model.AppVersionInfo +import net.mullvad.mullvadvpn.model.CustomList import net.mullvad.mullvadvpn.model.Device import net.mullvad.mullvadvpn.model.DeviceEvent import net.mullvad.mullvadvpn.model.DeviceListEvent @@ -189,6 +190,18 @@ class MullvadDaemon( setQuantumResistantTunnel(daemonInterfaceAddress, quantumResistant) } + fun createCustomList(name: String): String { + return createCustomList(daemonInterfaceAddress, name) + } + + fun deleteCustomList(id: String) { + deleteCustomList(daemonInterfaceAddress, id) + } + + fun updateCustomList(customList: CustomList) { + updateCustomList(daemonInterfaceAddress, customList) + } + fun onDestroy() { onSettingsChange.unsubscribeAll() onTunnelStateChange.unsubscribeAll() @@ -297,6 +310,13 @@ class MullvadDaemon( ) // Used by JNI + + private external fun createCustomList(daemonInterfaceAddress: Long, name: String): String + + private external fun deleteCustomList(daemonInterfaceAddress: Long, id: String) + + private external fun updateCustomList(daemonInterfaceAddress: Long, customList: CustomList) + @Suppress("unused") private fun notifyAppVersionInfoEvent(appVersionInfo: AppVersionInfo) { onAppVersionInfoChange?.invoke(appVersionInfo) diff --git a/android/service/src/main/kotlin/net/mullvad/mullvadvpn/service/endpoint/CustomLists.kt b/android/service/src/main/kotlin/net/mullvad/mullvadvpn/service/endpoint/CustomLists.kt new file mode 100644 index 000000000000..d80bcf04ff46 --- /dev/null +++ b/android/service/src/main/kotlin/net/mullvad/mullvadvpn/service/endpoint/CustomLists.kt @@ -0,0 +1,49 @@ +package net.mullvad.mullvadvpn.service.endpoint + +import kotlinx.coroutines.CoroutineDispatcher +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.SupervisorJob +import kotlinx.coroutines.cancel +import kotlinx.coroutines.flow.filterIsInstance +import kotlinx.coroutines.launch +import net.mullvad.mullvadvpn.lib.ipc.Event +import net.mullvad.mullvadvpn.lib.ipc.Request + +class CustomLists( + private val endpoint: ServiceEndpoint, + dispatcher: CoroutineDispatcher = Dispatchers.IO +) { + private val scope: CoroutineScope = CoroutineScope(SupervisorJob() + dispatcher) + private val daemon + get() = endpoint.intermittentDaemon + + init { + scope.launch { + endpoint.dispatcher.parsedMessages + .filterIsInstance() + .collect { createCustomList(it.name) } + } + + scope.launch { + endpoint.dispatcher.parsedMessages + .filterIsInstance() + .collect { daemon.await().deleteCustomList(it.id) } + } + + scope.launch { + endpoint.dispatcher.parsedMessages + .filterIsInstance() + .collect { daemon.await().updateCustomList(it.customList) } + } + } + + private suspend fun createCustomList(name: String) { + val result = daemon.await().createCustomList(name) + endpoint.sendEvent(Event.CreateCustomListResult(result)) + } + + fun onDestroy() { + scope.cancel() + } +} diff --git a/android/service/src/main/kotlin/net/mullvad/mullvadvpn/service/endpoint/RelayListListener.kt b/android/service/src/main/kotlin/net/mullvad/mullvadvpn/service/endpoint/RelayListListener.kt index 2f18c090640a..8ba6234cf664 100644 --- a/android/service/src/main/kotlin/net/mullvad/mullvadvpn/service/endpoint/RelayListListener.kt +++ b/android/service/src/main/kotlin/net/mullvad/mullvadvpn/service/endpoint/RelayListListener.kt @@ -11,7 +11,6 @@ import kotlinx.coroutines.launch import net.mullvad.mullvadvpn.lib.ipc.Event import net.mullvad.mullvadvpn.lib.ipc.Request import net.mullvad.mullvadvpn.model.Constraint -import net.mullvad.mullvadvpn.model.LocationConstraint import net.mullvad.mullvadvpn.model.RelayConstraints import net.mullvad.mullvadvpn.model.RelayList import net.mullvad.mullvadvpn.model.RelaySettings @@ -45,12 +44,7 @@ class RelayListListener( .collect { request -> val update = getCurrentRelayConstraints() - .copy( - location = - Constraint.Only( - LocationConstraint.Location(request.relayLocation) - ) - ) + .copy(location = Constraint.Only(request.locationConstraint)) daemon.await().setRelaySettings(RelaySettings.Normal(update)) } } diff --git a/android/service/src/main/kotlin/net/mullvad/mullvadvpn/service/endpoint/ServiceEndpoint.kt b/android/service/src/main/kotlin/net/mullvad/mullvadvpn/service/endpoint/ServiceEndpoint.kt index 09fc73e5d3cd..5485c528b079 100644 --- a/android/service/src/main/kotlin/net/mullvad/mullvadvpn/service/endpoint/ServiceEndpoint.kt +++ b/android/service/src/main/kotlin/net/mullvad/mullvadvpn/service/endpoint/ServiceEndpoint.kt @@ -51,6 +51,7 @@ class ServiceEndpoint( val voucherRedeemer = VoucherRedeemer(this, accountCache) private val playPurchaseHandler = PlayPurchaseHandler(this) + private val customLists = CustomLists(this) private val deviceRepositoryBackend = DaemonDeviceDataSource(this) @@ -81,6 +82,7 @@ class ServiceEndpoint( splitTunneling.onDestroy() voucherRedeemer.onDestroy() playPurchaseHandler.onDestroy() + customLists.onDestroy() } internal fun sendEvent(event: Event) { From 4f12d12eab7eb86254e2a0efeb9ef7b97c913afc Mon Sep 17 00:00:00 2001 From: Jonatan Rhodin Date: Wed, 7 Feb 2024 23:24:51 +0100 Subject: [PATCH 04/11] Add custom lists Events and Requests --- .../kotlin/net/mullvad/mullvadvpn/lib/ipc/Event.kt | 2 ++ .../kotlin/net/mullvad/mullvadvpn/lib/ipc/Request.kt | 12 +++++++++--- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/android/lib/ipc/src/main/kotlin/net/mullvad/mullvadvpn/lib/ipc/Event.kt b/android/lib/ipc/src/main/kotlin/net/mullvad/mullvadvpn/lib/ipc/Event.kt index 69c28bb379da..1136ae8c55d7 100644 --- a/android/lib/ipc/src/main/kotlin/net/mullvad/mullvadvpn/lib/ipc/Event.kt +++ b/android/lib/ipc/src/main/kotlin/net/mullvad/mullvadvpn/lib/ipc/Event.kt @@ -65,6 +65,8 @@ sealed class Event : Message.EventMessage() { @Parcelize object VpnPermissionRequest : Event() + @Parcelize data class CreateCustomListResult(val listId: String) : Event() + companion object { private const val MESSAGE_KEY = "event" diff --git a/android/lib/ipc/src/main/kotlin/net/mullvad/mullvadvpn/lib/ipc/Request.kt b/android/lib/ipc/src/main/kotlin/net/mullvad/mullvadvpn/lib/ipc/Request.kt index 267f1f2619a8..fe9d3b46d9e5 100644 --- a/android/lib/ipc/src/main/kotlin/net/mullvad/mullvadvpn/lib/ipc/Request.kt +++ b/android/lib/ipc/src/main/kotlin/net/mullvad/mullvadvpn/lib/ipc/Request.kt @@ -5,8 +5,9 @@ import android.os.Messenger import java.net.InetAddress import kotlinx.parcelize.Parcelize import net.mullvad.mullvadvpn.model.Constraint +import net.mullvad.mullvadvpn.model.CustomList import net.mullvad.mullvadvpn.model.DnsOptions -import net.mullvad.mullvadvpn.model.GeographicLocationConstraint +import net.mullvad.mullvadvpn.model.LocationConstraint import net.mullvad.mullvadvpn.model.ObfuscationSettings import net.mullvad.mullvadvpn.model.Ownership import net.mullvad.mullvadvpn.model.PlayPurchase @@ -77,8 +78,7 @@ sealed class Request : Message.RequestMessage() { @Parcelize data class SetEnableSplitTunneling(val enable: Boolean) : Request() - @Parcelize - data class SetRelayLocation(val relayLocation: GeographicLocationConstraint) : Request() + @Parcelize data class SetRelayLocation(val locationConstraint: LocationConstraint) : Request() @Parcelize data class SetWireGuardMtu(val mtu: Int?) : Request() @@ -111,6 +111,12 @@ sealed class Request : Message.RequestMessage() { val providers: Constraint ) : Request() + @Parcelize data class CreateCustomList(val name: String) : Request() + + @Parcelize data class DeleteCustomList(val id: String) : Request() + + @Parcelize data class UpdateCustomList(val customList: CustomList) : Request() + companion object { private const val MESSAGE_KEY = "request" From 25054733a42638a5375ef801025a025d0a1ef605 Mon Sep 17 00:00:00 2001 From: Jonatan Rhodin Date: Wed, 7 Feb 2024 23:26:26 +0100 Subject: [PATCH 05/11] Add CustomListsRepository with add, delete and update --- .../repository/CustomListsRepository.kt | 28 +++++++++++++++++++ 1 file changed, 28 insertions(+) create mode 100644 android/app/src/main/kotlin/net/mullvad/mullvadvpn/repository/CustomListsRepository.kt 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 new file mode 100644 index 000000000000..966098168842 --- /dev/null +++ b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/repository/CustomListsRepository.kt @@ -0,0 +1,28 @@ +package net.mullvad.mullvadvpn.repository + +import kotlinx.coroutines.flow.first +import net.mullvad.mullvadvpn.lib.ipc.Event +import net.mullvad.mullvadvpn.lib.ipc.MessageHandler +import net.mullvad.mullvadvpn.lib.ipc.Request +import net.mullvad.mullvadvpn.lib.ipc.events +import net.mullvad.mullvadvpn.model.CustomList + +class CustomListsRepository(private val messageHandler: MessageHandler) { + suspend fun createCustomList(name: String): String? { + val result = messageHandler.trySendRequest(Request.CreateCustomList(name)) + + return if (result) { + messageHandler.events().first().listId + } else { + null + } + } + + fun deleteCustomList(id: String) { + messageHandler.trySendRequest(Request.DeleteCustomList(id)) + } + + fun updateCustomList(customList: CustomList) { + messageHandler.trySendRequest(Request.UpdateCustomList(customList)) + } +} From 52a9cf9686756b366c5e1d8359f5a2b37c18df8a Mon Sep 17 00:00:00 2001 From: Jonatan Rhodin Date: Wed, 7 Feb 2024 23:27:20 +0100 Subject: [PATCH 06/11] Update the relaylist model Use sealed interface for RelayItem Add CustomList as a type of RelayItem --- .../relaylist/CustomListExtensions.kt | 19 +++++ .../net/mullvad/mullvadvpn/relaylist/Relay.kt | 16 ---- .../mullvad/mullvadvpn/relaylist/RelayCity.kt | 19 ----- .../mullvadvpn/relaylist/RelayCountry.kt | 19 ----- .../mullvad/mullvadvpn/relaylist/RelayItem.kt | 68 +++++++++++++++- .../relaylist/RelayItemExtensions.kt | 12 +++ .../mullvad/mullvadvpn/relaylist/RelayList.kt | 6 +- .../relaylist/RelayListExtensions.kt | 78 +++++++++---------- .../relaylist/RelayNameComparator.kt | 4 +- .../ui/serviceconnection/RelayListListener.kt | 4 +- .../util/LocationConstraintExtensions.kt | 23 ------ 11 files changed, 142 insertions(+), 126 deletions(-) create mode 100644 android/app/src/main/kotlin/net/mullvad/mullvadvpn/relaylist/CustomListExtensions.kt delete mode 100644 android/app/src/main/kotlin/net/mullvad/mullvadvpn/relaylist/Relay.kt delete mode 100644 android/app/src/main/kotlin/net/mullvad/mullvadvpn/relaylist/RelayCity.kt delete mode 100644 android/app/src/main/kotlin/net/mullvad/mullvadvpn/relaylist/RelayCountry.kt create mode 100644 android/app/src/main/kotlin/net/mullvad/mullvadvpn/relaylist/RelayItemExtensions.kt delete mode 100644 android/lib/common/src/main/kotlin/net/mullvad/mullvadvpn/lib/common/util/LocationConstraintExtensions.kt diff --git a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/relaylist/CustomListExtensions.kt b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/relaylist/CustomListExtensions.kt new file mode 100644 index 000000000000..9ad0c220e9d4 --- /dev/null +++ b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/relaylist/CustomListExtensions.kt @@ -0,0 +1,19 @@ +package net.mullvad.mullvadvpn.relaylist + +import net.mullvad.mullvadvpn.model.CustomList + +fun CustomList.toRelayItemCustomList( + relayCountries: List +): RelayItem.CustomList = + RelayItem.CustomList( + code = this.id, + id = this.id, + name = this.name, + expanded = false, + locations = + this.locations.mapNotNull { relayCountries.findItemForGeographicLocationConstraint(it) } + ) + +fun List.toRelayItemLists( + relayCountries: List +): List = this.map { it.toRelayItemCustomList(relayCountries) } diff --git a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/relaylist/Relay.kt b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/relaylist/Relay.kt deleted file mode 100644 index 9bdb59168b1a..000000000000 --- a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/relaylist/Relay.kt +++ /dev/null @@ -1,16 +0,0 @@ -package net.mullvad.mullvadvpn.relaylist - -import net.mullvad.mullvadvpn.model.GeographicLocationConstraint - -data class Relay( - override val name: String, - override val location: GeographicLocationConstraint, - override val locationName: String, - override val active: Boolean -) : RelayItem { - override val code = name - override val type = RelayItemType.Relay - override val hasChildren = false - - override val expanded = false -} diff --git a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/relaylist/RelayCity.kt b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/relaylist/RelayCity.kt deleted file mode 100644 index e3693963f81f..000000000000 --- a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/relaylist/RelayCity.kt +++ /dev/null @@ -1,19 +0,0 @@ -package net.mullvad.mullvadvpn.relaylist - -import net.mullvad.mullvadvpn.model.GeographicLocationConstraint - -data class RelayCity( - override val name: String, - override val code: String, - override val location: GeographicLocationConstraint, - override val expanded: Boolean, - val relays: List -) : RelayItem { - override val type = RelayItemType.City - - override val active - get() = relays.any { relay -> relay.active } - - override val hasChildren - get() = relays.isNotEmpty() -} diff --git a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/relaylist/RelayCountry.kt b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/relaylist/RelayCountry.kt deleted file mode 100644 index 3ad1d5962fcc..000000000000 --- a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/relaylist/RelayCountry.kt +++ /dev/null @@ -1,19 +0,0 @@ -package net.mullvad.mullvadvpn.relaylist - -import net.mullvad.mullvadvpn.model.GeographicLocationConstraint - -data class RelayCountry( - override val name: String, - override val code: String, - override val expanded: Boolean, - val cities: List -) : RelayItem { - override val type = RelayItemType.Country - override val location = GeographicLocationConstraint.Country(code) - - override val active - get() = cities.any { city -> city.active } - - override val hasChildren - get() = cities.isNotEmpty() -} diff --git a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/relaylist/RelayItem.kt b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/relaylist/RelayItem.kt index 3d38d6b0685d..03ba7a02f3db 100644 --- a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/relaylist/RelayItem.kt +++ b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/relaylist/RelayItem.kt @@ -1,12 +1,11 @@ package net.mullvad.mullvadvpn.relaylist +import net.mullvad.mullvadvpn.model.GeoIpLocation import net.mullvad.mullvadvpn.model.GeographicLocationConstraint -interface RelayItem { - val type: RelayItemType +sealed interface RelayItem { val name: String val code: String - val location: GeographicLocationConstraint val active: Boolean val hasChildren: Boolean @@ -14,4 +13,67 @@ interface RelayItem { get() = name val expanded: Boolean + + data class CustomList( + override val name: String, + override val code: String, + override val expanded: Boolean, + val id: String, + val locations: List, + ) : RelayItem { + override val active + get() = locations.any { location -> location.active } + + override val hasChildren + get() = locations.isNotEmpty() + } + + data class Country( + override val name: String, + override val code: String, + override val expanded: Boolean, + val cities: List + ) : RelayItem { + val location = GeographicLocationConstraint.Country(code) + override val active + get() = cities.any { city -> city.active } + + override val hasChildren + get() = cities.isNotEmpty() + } + + data class City( + override val name: String, + override val code: String, + override val expanded: Boolean, + val location: GeographicLocationConstraint.City, + val relays: List + ) : RelayItem { + + override val active + get() = relays.any { relay -> relay.active } + + override val hasChildren + get() = relays.isNotEmpty() + } + + data class Relay( + override val name: String, + override val locationName: String, + override val active: Boolean, + val location: GeographicLocationConstraint.Hostname, + ) : RelayItem { + override val code = name + override val hasChildren = false + override val expanded = false + } + + fun location(): GeoIpLocation? { + return when (this) { + is CustomList -> null + is Country -> location.location + is City -> location.location + is Relay -> location.location + } + } } diff --git a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/relaylist/RelayItemExtensions.kt b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/relaylist/RelayItemExtensions.kt new file mode 100644 index 000000000000..6808d707b209 --- /dev/null +++ b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/relaylist/RelayItemExtensions.kt @@ -0,0 +1,12 @@ +package net.mullvad.mullvadvpn.relaylist + +import net.mullvad.mullvadvpn.model.LocationConstraint + +fun RelayItem.toLocationConstraint(): LocationConstraint { + return when (this) { + is RelayItem.Country -> LocationConstraint.Location(location) + is RelayItem.City -> LocationConstraint.Location(location) + is RelayItem.Relay -> LocationConstraint.Location(location) + is RelayItem.CustomList -> LocationConstraint.CustomList(id) + } +} diff --git a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/relaylist/RelayList.kt b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/relaylist/RelayList.kt index c74c74ba437a..30e4146245ac 100644 --- a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/relaylist/RelayList.kt +++ b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/relaylist/RelayList.kt @@ -1,3 +1,7 @@ package net.mullvad.mullvadvpn.relaylist -data class RelayList(val country: List, val selectedItem: RelayItem?) +data class RelayList( + val customLists: List, + val country: List, + val selectedItem: RelayItem?, +) diff --git a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/relaylist/RelayListExtensions.kt b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/relaylist/RelayListExtensions.kt index 6462ed9f4151..06e00a022aa5 100644 --- a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/relaylist/RelayListExtensions.kt +++ b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/relaylist/RelayListExtensions.kt @@ -15,17 +15,17 @@ import net.mullvad.mullvadvpn.model.RelayList fun RelayList.toRelayCountries( ownership: Constraint, providers: Constraint -): List { +): List { val relayCountries = this.countries .map { country -> - val cities = mutableListOf() - val relayCountry = RelayCountry(country.name, country.code, false, cities) + val cities = mutableListOf() + val relayCountry = RelayItem.Country(country.name, country.code, false, cities) for (city in country.cities) { - val relays = mutableListOf() + val relays = mutableListOf() val relayCity = - RelayCity( + RelayItem.City( name = city.name, code = city.code, location = GeographicLocationConstraint.City(country.code, city.code), @@ -38,7 +38,7 @@ fun RelayList.toRelayCountries( for (relay in validCityRelays) { relays.add( - Relay( + RelayItem.Relay( name = relay.hostname, location = GeographicLocationConstraint.Hostname( @@ -69,32 +69,26 @@ fun RelayList.toRelayCountries( return relayCountries.toList() } -fun List.findItemForLocation( - constraint: Constraint -): RelayItem? { - return when (constraint) { - is Constraint.Any -> null - is Constraint.Only -> { - when (val location = constraint.value) { - is GeographicLocationConstraint.Country -> { - this.find { country -> country.code == location.countryCode } - } - is GeographicLocationConstraint.City -> { - val country = this.find { country -> country.code == location.countryCode } +fun List.findItemForGeographicLocationConstraint( + constraint: GeographicLocationConstraint +) = + when (constraint) { + is GeographicLocationConstraint.Country -> { + this.find { country -> country.code == constraint.countryCode } + } + is GeographicLocationConstraint.City -> { + val country = this.find { country -> country.code == constraint.countryCode } - country?.cities?.find { city -> city.code == location.cityCode } - } - is GeographicLocationConstraint.Hostname -> { - val country = this.find { country -> country.code == location.countryCode } + country?.cities?.find { city -> city.code == constraint.cityCode } + } + is GeographicLocationConstraint.Hostname -> { + val country = this.find { country -> country.code == constraint.countryCode } - val city = country?.cities?.find { city -> city.code == location.cityCode } + val city = country?.cities?.find { city -> city.code == constraint.cityCode } - city?.relays?.find { relay -> relay.name == location.hostname } - } - } + city?.relays?.find { relay -> relay.name == constraint.hostname } } } -} /** * Filter and expand the list based on search terms If a country is matched, that country and all @@ -102,14 +96,15 @@ fun List.findItemForLocation( * parent country is added and expanded if needed and its children are added, but the city is not * expanded If a relay is matched, its parents are added and expanded and itself is also added. */ -fun List.filterOnSearchTerm( +@Suppress("NestedBlockDepth") +fun List.filterOnSearchTerm( searchTerm: String, selectedItem: RelayItem? -): List { +): List { return if (searchTerm.length >= MIN_SEARCH_LENGTH) { - val filteredCountries = mutableMapOf() + val filteredCountries = mutableMapOf() this.forEach { relayCountry -> - val cities = mutableListOf() + val cities = mutableListOf() // Try to match the search term with a country // If we match a country, add that country and all cities and relays in that country @@ -122,7 +117,7 @@ fun List.filterOnSearchTerm( // Go through and try to match the search term with every city relayCountry.cities.forEach { relayCity -> - val relays = mutableListOf() + val relays = mutableListOf() // If we match and we already added the country to the filtered list just expand the // country. // If the country is not currently in the filtered list, add it and expand it. @@ -200,31 +195,31 @@ private fun List.filterValidRelays( } /** Expand the parent(s), if any, for the current selected item */ -private fun List.expandItemForSelection( +private fun List.expandItemForSelection( selectedItem: RelayItem? -): List { +): List { return selectedItem?.let { - when (val location = selectedItem.location) { - is GeographicLocationConstraint.Country -> { + when (selectedItem) { + is RelayItem.Country -> { this } - is GeographicLocationConstraint.City -> { + is RelayItem.City -> { this.map { country -> - if (country.code == location.countryCode) { + if (country.code == selectedItem.location.countryCode) { country.copy(expanded = true) } else { country } } } - is GeographicLocationConstraint.Hostname -> { + is RelayItem.Relay -> { this.map { country -> - if (country.code == location.countryCode) { + if (country.code == selectedItem.location.countryCode) { country.copy( expanded = true, cities = country.cities.map { city -> - if (city.code == location.cityCode) { + if (city.code == selectedItem.location.cityCode) { city.copy(expanded = true) } else { city @@ -236,6 +231,7 @@ private fun List.expandItemForSelection( } } } + is RelayItem.CustomList -> this } } ?: this } diff --git a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/relaylist/RelayNameComparator.kt b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/relaylist/RelayNameComparator.kt index 92a3c9c1d611..c062fd1466f7 100644 --- a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/relaylist/RelayNameComparator.kt +++ b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/relaylist/RelayNameComparator.kt @@ -1,7 +1,7 @@ package net.mullvad.mullvadvpn.relaylist -internal object RelayNameComparator : Comparator { - override fun compare(o1: Relay, o2: Relay): Int { +internal object RelayNameComparator : Comparator { + override fun compare(o1: RelayItem.Relay, o2: RelayItem.Relay): Int { val partitions1 = o1.name.split(regex) val partitions2 = o2.name.split(regex) return if (partitions1.size > partitions2.size) partitions1 compareWith partitions2 diff --git a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/ui/serviceconnection/RelayListListener.kt b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/ui/serviceconnection/RelayListListener.kt index 2d6ce332f693..841c9e0c59d2 100644 --- a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/ui/serviceconnection/RelayListListener.kt +++ b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/ui/serviceconnection/RelayListListener.kt @@ -13,7 +13,7 @@ import net.mullvad.mullvadvpn.lib.ipc.MessageHandler import net.mullvad.mullvadvpn.lib.ipc.Request import net.mullvad.mullvadvpn.lib.ipc.events import net.mullvad.mullvadvpn.model.Constraint -import net.mullvad.mullvadvpn.model.GeographicLocationConstraint +import net.mullvad.mullvadvpn.model.LocationConstraint import net.mullvad.mullvadvpn.model.Ownership import net.mullvad.mullvadvpn.model.Providers import net.mullvad.mullvadvpn.model.RelayList @@ -38,7 +38,7 @@ class RelayListListener( defaultRelayList() ) - fun updateSelectedRelayLocation(value: GeographicLocationConstraint) { + fun updateSelectedRelayLocation(value: LocationConstraint) { messageHandler.trySendRequest(Request.SetRelayLocation(value)) } diff --git a/android/lib/common/src/main/kotlin/net/mullvad/mullvadvpn/lib/common/util/LocationConstraintExtensions.kt b/android/lib/common/src/main/kotlin/net/mullvad/mullvadvpn/lib/common/util/LocationConstraintExtensions.kt deleted file mode 100644 index d845e3aba909..000000000000 --- a/android/lib/common/src/main/kotlin/net/mullvad/mullvadvpn/lib/common/util/LocationConstraintExtensions.kt +++ /dev/null @@ -1,23 +0,0 @@ -package net.mullvad.mullvadvpn.lib.common.util - -import net.mullvad.mullvadvpn.model.Constraint -import net.mullvad.mullvadvpn.model.GeographicLocationConstraint -import net.mullvad.mullvadvpn.model.LocationConstraint - -fun LocationConstraint.toGeographicLocationConstraint(): GeographicLocationConstraint? = - when (this) { - is LocationConstraint.Location -> this.location - is LocationConstraint.CustomList -> null - } - -fun Constraint.toGeographicLocationConstraint(): - Constraint = - when (this) { - is Constraint.Only -> - when (value) { - is LocationConstraint.Location -> - Constraint.Only((value as LocationConstraint.Location).location) - is LocationConstraint.CustomList -> Constraint.Any() - } - is Constraint.Any -> Constraint.Any() - } From a63afba8c2f3912185fbc5ed17c351e0a3701dd3 Mon Sep 17 00:00:00 2001 From: Jonatan Rhodin Date: Wed, 7 Feb 2024 23:30:19 +0100 Subject: [PATCH 07/11] Add customlists support for RelayListUseCase --- .../mullvadvpn/usecase/RelayListUseCase.kt | 36 +++++++++++++------ 1 file changed, 26 insertions(+), 10 deletions(-) diff --git a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/usecase/RelayListUseCase.kt b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/usecase/RelayListUseCase.kt index 8db90b939050..ab3d93e06e6b 100644 --- a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/usecase/RelayListUseCase.kt +++ b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/usecase/RelayListUseCase.kt @@ -3,16 +3,15 @@ package net.mullvad.mullvadvpn.usecase import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.combine import kotlinx.coroutines.flow.map -import net.mullvad.mullvadvpn.lib.common.util.toGeographicLocationConstraint import net.mullvad.mullvadvpn.model.Constraint -import net.mullvad.mullvadvpn.model.GeographicLocationConstraint +import net.mullvad.mullvadvpn.model.LocationConstraint import net.mullvad.mullvadvpn.model.RelaySettings import net.mullvad.mullvadvpn.model.WireguardConstraints -import net.mullvad.mullvadvpn.relaylist.RelayCountry import net.mullvad.mullvadvpn.relaylist.RelayItem import net.mullvad.mullvadvpn.relaylist.RelayList -import net.mullvad.mullvadvpn.relaylist.findItemForLocation +import net.mullvad.mullvadvpn.relaylist.findItemForGeographicLocationConstraint import net.mullvad.mullvadvpn.relaylist.toRelayCountries +import net.mullvad.mullvadvpn.relaylist.toRelayItemLists import net.mullvad.mullvadvpn.repository.SettingsRepository import net.mullvad.mullvadvpn.ui.serviceconnection.RelayListListener @@ -21,7 +20,7 @@ class RelayListUseCase( private val settingsRepository: SettingsRepository ) { - fun updateSelectedRelayLocation(value: GeographicLocationConstraint) { + fun updateSelectedRelayLocation(value: LocationConstraint) { relayListListener.updateSelectedRelayLocation(value) } @@ -39,11 +38,15 @@ class RelayListUseCase( settings?.relaySettings?.relayConstraints()?.providers ?: Constraint.Any() val relayCountries = relayList.toRelayCountries(ownership = ownership, providers = providers) + val customLists = + settings?.customLists?.customLists?.toRelayItemLists(relayCountries) ?: emptyList() val selectedItem = - relayCountries.findSelectedRelayItem( + findSelectedRelayItem( relaySettings = settings?.relaySettings, + relayCountries = relayCountries, + customLists = customLists ) - RelayList(relayCountries, selectedItem) + RelayList(customLists, relayCountries, selectedItem) } fun selectedRelayItem(): Flow = relayListWithSelection().map { it.selectedItem } @@ -52,10 +55,23 @@ class RelayListUseCase( relayListListener.fetchRelayList() } - private fun List.findSelectedRelayItem( + private fun findSelectedRelayItem( relaySettings: RelaySettings?, + relayCountries: List, + customLists: List ): RelayItem? { - val location = relaySettings?.relayConstraints()?.location - return location?.let { this.findItemForLocation(location.toGeographicLocationConstraint()) } + val locationConstraint = relaySettings?.relayConstraints()?.location + return if (locationConstraint is Constraint.Only) { + when (val location = locationConstraint.value) { + is LocationConstraint.CustomList -> { + customLists.firstOrNull { it.id == location.listId } + } + is LocationConstraint.Location -> { + relayCountries.findItemForGeographicLocationConstraint(location.location) + } + } + } else { + null + } } } From 8baf83c9ca8ffab6f7b103c001042dae1d701c9e Mon Sep 17 00:00:00 2001 From: Jonatan Rhodin Date: Wed, 7 Feb 2024 23:32:14 +0100 Subject: [PATCH 08/11] Support new relay list item in SelectLocationScreen --- .../compose/cell/RelayLocationCell.kt | 43 +++++++++---------- .../compose/screen/SelectLocationScreen.kt | 30 ++++++++----- .../compose/state/SelectLocationUiState.kt | 3 +- .../net/mullvad/mullvadvpn/di/UiModule.kt | 2 + .../viewmodel/SelectLocationViewModel.kt | 10 +++-- 5 files changed, 49 insertions(+), 39 deletions(-) diff --git a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/cell/RelayLocationCell.kt b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/cell/RelayLocationCell.kt index acd963fa61b8..d6adf33b8347 100644 --- a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/cell/RelayLocationCell.kt +++ b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/cell/RelayLocationCell.kt @@ -38,11 +38,7 @@ import net.mullvad.mullvadvpn.lib.theme.color.AlphaInvisible import net.mullvad.mullvadvpn.lib.theme.color.AlphaVisible import net.mullvad.mullvadvpn.lib.theme.color.selected import net.mullvad.mullvadvpn.model.GeographicLocationConstraint -import net.mullvad.mullvadvpn.relaylist.Relay -import net.mullvad.mullvadvpn.relaylist.RelayCity -import net.mullvad.mullvadvpn.relaylist.RelayCountry import net.mullvad.mullvadvpn.relaylist.RelayItem -import net.mullvad.mullvadvpn.relaylist.RelayItemType @Composable @Preview @@ -50,20 +46,20 @@ private fun PreviewRelayLocationCell() { AppTheme { Column(Modifier.background(color = MaterialTheme.colorScheme.background)) { val countryActive = - RelayCountry( + RelayItem.Country( name = "Relay country Active", code = "RC1", expanded = false, cities = listOf( - RelayCity( + RelayItem.City( name = "Relay city 1", code = "RI1", expanded = false, location = GeographicLocationConstraint.City("RC1", "RI1"), relays = listOf( - Relay( + RelayItem.Relay( name = "Relay 1", active = true, locationName = "", @@ -76,14 +72,14 @@ private fun PreviewRelayLocationCell() { ) ) ), - RelayCity( + RelayItem.City( name = "Relay city 2", code = "RI2", expanded = true, location = GeographicLocationConstraint.City("RC1", "RI2"), relays = listOf( - Relay( + RelayItem.Relay( name = "Relay 2", active = true, locationName = "", @@ -94,7 +90,7 @@ private fun PreviewRelayLocationCell() { "NER" ) ), - Relay( + RelayItem.Relay( name = "Relay 3", active = true, locationName = "", @@ -110,20 +106,20 @@ private fun PreviewRelayLocationCell() { ) ) val countryNotActive = - RelayCountry( + RelayItem.Country( name = "Not Enabled Relay country", code = "RC3", expanded = true, cities = listOf( - RelayCity( + RelayItem.City( name = "Not Enabled city", code = "RI3", expanded = true, location = GeographicLocationConstraint.City("RC3", "RI3"), relays = listOf( - Relay( + RelayItem.Relay( name = "Not Enabled Relay", active = false, locationName = "", @@ -158,10 +154,11 @@ fun RelayLocationCell( onSelectRelay: (item: RelayItem) -> Unit = {} ) { val startPadding = - when (relay.type) { - RelayItemType.Country -> Dimens.countryRowPadding - RelayItemType.City -> Dimens.cityRowPadding - RelayItemType.Relay -> Dimens.relayRowPadding + when (relay) { + is RelayItem.Country, + is RelayItem.CustomList -> Dimens.countryRowPadding + is RelayItem.City -> Dimens.cityRowPadding + is RelayItem.Relay -> Dimens.relayRowPadding } val selected = selectedItem?.code == relay.code val expanded = @@ -169,12 +166,12 @@ fun RelayLocationCell( val backgroundColor = when { selected -> MaterialTheme.colorScheme.inversePrimary - relay.type == RelayItemType.Country -> MaterialTheme.colorScheme.primary - relay.type == RelayItemType.City -> + relay is RelayItem.Country -> MaterialTheme.colorScheme.primary + relay is RelayItem.City -> MaterialTheme.colorScheme.primary .copy(alpha = Alpha40) .compositeOver(MaterialTheme.colorScheme.background) - relay.type == RelayItemType.Relay -> MaterialTheme.colorScheme.secondaryContainer + relay is RelayItem.Relay -> MaterialTheme.colorScheme.secondaryContainer else -> MaterialTheme.colorScheme.primary } Column( @@ -273,7 +270,7 @@ fun RelayLocationCell( } if (expanded.value) { when (relay) { - is RelayCountry -> { + is RelayItem.Country -> { relay.cities.forEach { relayCity -> RelayLocationCell( relay = relayCity, @@ -283,7 +280,7 @@ fun RelayLocationCell( ) } } - is RelayCity -> { + is RelayItem.City -> { relay.relays.forEach { relay -> RelayLocationCell( relay = relay, @@ -293,6 +290,8 @@ fun RelayLocationCell( ) } } + is RelayItem.Relay, + is RelayItem.CustomList -> {} } } } diff --git a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/screen/SelectLocationScreen.kt b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/screen/SelectLocationScreen.kt index cf5d4f02c7f5..e024ae313294 100644 --- a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/screen/SelectLocationScreen.kt +++ b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/screen/SelectLocationScreen.kt @@ -53,7 +53,6 @@ import net.mullvad.mullvadvpn.compose.transitions.SelectLocationTransition import net.mullvad.mullvadvpn.lib.theme.AppTheme import net.mullvad.mullvadvpn.lib.theme.Dimens import net.mullvad.mullvadvpn.lib.theme.color.AlphaScrollbar -import net.mullvad.mullvadvpn.relaylist.RelayCountry import net.mullvad.mullvadvpn.relaylist.RelayItem import net.mullvad.mullvadvpn.viewmodel.SelectLocationSideEffect import net.mullvad.mullvadvpn.viewmodel.SelectLocationViewModel @@ -69,8 +68,9 @@ private fun PreviewSelectLocationScreen() { selectedProvidersCount = 0, relayListState = RelayListState.RelayList( - countries = listOf(RelayCountry("Country 1", "Code 1", false, emptyList())), - selectedRelay = null, + countries = + listOf(RelayItem.Country("Country 1", "Code 1", false, emptyList())), + selectedItem = null, ) ) AppTheme { @@ -172,14 +172,10 @@ fun SelectLocationScreen( if ( uiState is SelectLocationUiState.Data && uiState.relayListState is RelayListState.RelayList && - uiState.relayListState.selectedRelay != null + uiState.relayListState.selectedItem != null ) { - LaunchedEffect(uiState.relayListState.selectedRelay) { - val index = - uiState.relayListState.countries.indexOfFirst { relayCountry -> - relayCountry.location.location.country == - uiState.relayListState.selectedRelay.location.location.country - } + LaunchedEffect(uiState.relayListState.selectedItem) { + val index = uiState.relayListState.indexOfSelectedRelayItem() if (index >= 0) { lazyListState.scrollToItem(index) @@ -235,7 +231,7 @@ private fun LazyListScope.relayList( val country = relayListState.countries[index] RelayLocationCell( relay = country, - selectedItem = relayListState.selectedRelay, + selectedItem = relayListState.selectedItem, onSelectRelay = onSelectRelay, modifier = Modifier.animateContentSize() ) @@ -279,6 +275,18 @@ private fun LazyListScope.relayList( } } +private fun RelayListState.RelayList.indexOfSelectedRelayItem(): Int = + countries.indexOfFirst { relayCountry -> + relayCountry.location.location.country == + when (selectedItem) { + is RelayItem.Country -> selectedItem.code + is RelayItem.City -> selectedItem.location.countryCode + is RelayItem.Relay -> selectedItem.location.countryCode + is RelayItem.CustomList, + null -> null + } + } + suspend fun LazyListState.animateScrollAndCentralizeItem(index: Int) { val itemInfo = this.layoutInfo.visibleItemsInfo.firstOrNull { it.index == index } if (itemInfo != null) { diff --git a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/state/SelectLocationUiState.kt b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/state/SelectLocationUiState.kt index 3152ed1a348d..fd775fa1bb6e 100644 --- a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/state/SelectLocationUiState.kt +++ b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/state/SelectLocationUiState.kt @@ -1,7 +1,6 @@ package net.mullvad.mullvadvpn.compose.state import net.mullvad.mullvadvpn.model.Ownership -import net.mullvad.mullvadvpn.relaylist.RelayCountry import net.mullvad.mullvadvpn.relaylist.RelayItem sealed interface SelectLocationUiState { @@ -21,6 +20,6 @@ sealed interface SelectLocationUiState { sealed interface RelayListState { data object Empty : RelayListState - data class RelayList(val countries: List, val selectedRelay: RelayItem?) : + data class RelayList(val countries: List, val selectedItem: RelayItem?) : RelayListState } diff --git a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/di/UiModule.kt b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/di/UiModule.kt index 5e51cc99d4b7..c62cf0385145 100644 --- a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/di/UiModule.kt +++ b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/di/UiModule.kt @@ -15,6 +15,7 @@ import net.mullvad.mullvadvpn.lib.ipc.MessageHandler import net.mullvad.mullvadvpn.lib.payment.PaymentProvider import net.mullvad.mullvadvpn.repository.AccountRepository import net.mullvad.mullvadvpn.repository.ChangelogRepository +import net.mullvad.mullvadvpn.repository.CustomListsRepository import net.mullvad.mullvadvpn.repository.DeviceRepository import net.mullvad.mullvadvpn.repository.InAppNotificationController import net.mullvad.mullvadvpn.repository.PrivacyDisclaimerRepository @@ -98,6 +99,7 @@ val uiModule = module { } single { SettingsRepository(get()) } single { MullvadProblemReport(get()) } + single { CustomListsRepository(get()) } single { AccountExpiryNotificationUseCase(get()) } single { TunnelStateNotificationUseCase(get()) } diff --git a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/viewmodel/SelectLocationViewModel.kt b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/viewmodel/SelectLocationViewModel.kt index d3c5977e27bd..fc46441de18d 100644 --- a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/viewmodel/SelectLocationViewModel.kt +++ b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/viewmodel/SelectLocationViewModel.kt @@ -20,6 +20,7 @@ import net.mullvad.mullvadvpn.model.Ownership import net.mullvad.mullvadvpn.relaylist.Provider import net.mullvad.mullvadvpn.relaylist.RelayItem import net.mullvad.mullvadvpn.relaylist.filterOnSearchTerm +import net.mullvad.mullvadvpn.relaylist.toLocationConstraint import net.mullvad.mullvadvpn.ui.serviceconnection.ServiceConnectionManager import net.mullvad.mullvadvpn.ui.serviceconnection.connectionProxy import net.mullvad.mullvadvpn.usecase.RelayListFilterUseCase @@ -40,7 +41,7 @@ class SelectLocationViewModel( relayListFilterUseCase.availableProviders(), relayListFilterUseCase.selectedProviders() ) { - (relayCountries, relayItem), + (customList, relayCountries, selectedItem), searchTerm, selectedOwnership, allProviders, @@ -54,7 +55,7 @@ class SelectLocationViewModel( ?.size val filteredRelayCountries = - relayCountries.filterOnSearchTerm(searchTerm, relayItem) + relayCountries.filterOnSearchTerm(searchTerm, selectedItem) SelectLocationUiState.Data( searchTerm = searchTerm, @@ -64,7 +65,7 @@ class SelectLocationViewModel( if (filteredRelayCountries.isNotEmpty()) { RelayListState.RelayList( countries = filteredRelayCountries, - selectedRelay = relayItem + selectedItem = selectedItem ) } else { RelayListState.Empty @@ -85,7 +86,8 @@ class SelectLocationViewModel( } fun selectRelay(relayItem: RelayItem) { - relayListUseCase.updateSelectedRelayLocation(relayItem.location) + val locationConstraint = relayItem.toLocationConstraint() + relayListUseCase.updateSelectedRelayLocation(locationConstraint) serviceConnectionManager.connectionProxy()?.connect() viewModelScope.launch { _uiSideEffect.send(SelectLocationSideEffect.CloseScreen) } } From 1884feac95427a342e3e85f1ddfaf90efd654f5e Mon Sep 17 00:00:00 2001 From: Jonatan Rhodin Date: Wed, 7 Feb 2024 23:34:07 +0100 Subject: [PATCH 09/11] Support new relay item in connect screen --- .../net/mullvad/mullvadvpn/compose/screen/ConnectScreen.kt | 4 ++-- .../net/mullvad/mullvadvpn/compose/state/ConnectUiState.kt | 4 ++-- .../net/mullvad/mullvadvpn/viewmodel/ConnectViewModel.kt | 6 +++--- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/screen/ConnectScreen.kt b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/screen/ConnectScreen.kt index f626191b4cbb..84d2a0418eb6 100644 --- a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/screen/ConnectScreen.kt +++ b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/screen/ConnectScreen.kt @@ -264,8 +264,8 @@ fun ConnectScreen( onClick = onSwitchLocationClick, showChevron = uiState.showLocation, text = - if (uiState.showLocation) { - uiState.relayLocation?.locationName ?: "" + if (uiState.showLocation && uiState.selectedRelayItem != null) { + uiState.selectedRelayItem.locationName } else { stringResource(id = R.string.switch_location) } diff --git a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/state/ConnectUiState.kt b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/state/ConnectUiState.kt index dc26e24741df..988ae914e9c3 100644 --- a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/state/ConnectUiState.kt +++ b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/state/ConnectUiState.kt @@ -8,7 +8,7 @@ import net.mullvad.talpid.net.TransportProtocol data class ConnectUiState( val location: GeoIpLocation?, - val relayLocation: RelayItem?, + val selectedRelayItem: RelayItem?, val tunnelUiState: TunnelState, val tunnelRealState: TunnelState, val inAddress: Triple?, @@ -23,7 +23,7 @@ data class ConnectUiState( val INITIAL = ConnectUiState( location = null, - relayLocation = null, + selectedRelayItem = null, tunnelUiState = TunnelState.Disconnected(), tunnelRealState = TunnelState.Disconnected(), inAddress = null, diff --git a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/viewmodel/ConnectViewModel.kt b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/viewmodel/ConnectViewModel.kt index 9b5a6c1e0013..bee5b1ad0012 100644 --- a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/viewmodel/ConnectViewModel.kt +++ b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/viewmodel/ConnectViewModel.kt @@ -84,7 +84,7 @@ class ConnectViewModel( accountRepository.accountExpiryState, deviceRepository.deviceState.map { it.deviceName() } ) { - relayLocation, + selectedRelayItem, notifications, tunnelUiState, tunnelRealState, @@ -97,12 +97,12 @@ class ConnectViewModel( is TunnelState.Disconnected -> tunnelRealState.location() ?: lastKnownDisconnectedLocation is TunnelState.Connecting -> - tunnelRealState.location ?: relayLocation?.location?.location + tunnelRealState.location ?: selectedRelayItem?.location() is TunnelState.Connected -> tunnelRealState.location is TunnelState.Disconnecting -> lastKnownDisconnectedLocation is TunnelState.Error -> null }, - relayLocation = relayLocation, + selectedRelayItem = selectedRelayItem, tunnelUiState = tunnelUiState, tunnelRealState = tunnelRealState, inAddress = From e423fece7abb4bd870d2dd5f70b1caf90fe47c77 Mon Sep 17 00:00:00 2001 From: Jonatan Rhodin Date: Wed, 7 Feb 2024 23:36:15 +0100 Subject: [PATCH 10/11] Update unit tests for new relay items --- .../relaylist/RelayNameComparatorTest.kt | 99 +++++++++++++++---- .../viewmodel/ConnectViewModelTest.kt | 15 ++- .../viewmodel/SelectLocationViewModelTest.kt | 70 +++++++------ 3 files changed, 126 insertions(+), 58 deletions(-) diff --git a/android/app/src/test/kotlin/net/mullvad/mullvadvpn/relaylist/RelayNameComparatorTest.kt b/android/app/src/test/kotlin/net/mullvad/mullvadvpn/relaylist/RelayNameComparatorTest.kt index dc9c141f28ab..6fe71ae44316 100644 --- a/android/app/src/test/kotlin/net/mullvad/mullvadvpn/relaylist/RelayNameComparatorTest.kt +++ b/android/app/src/test/kotlin/net/mullvad/mullvadvpn/relaylist/RelayNameComparatorTest.kt @@ -16,9 +16,14 @@ class RelayNameComparatorTest { @Test fun test_compare_respect_numbers_in_name() { val relay9 = - Relay(name = "se9-wireguard", location = mockk(), locationName = "mock", active = false) + RelayItem.Relay( + name = "se9-wireguard", + location = mockk(), + locationName = "mock", + active = false + ) val relay10 = - Relay( + RelayItem.Relay( name = "se10-wireguard", location = mockk(), locationName = "mock", @@ -31,9 +36,19 @@ class RelayNameComparatorTest { @Test fun test_compare_same_name() { val relay9a = - Relay(name = "se9-wireguard", location = mockk(), locationName = "mock", active = false) + RelayItem.Relay( + name = "se9-wireguard", + location = mockk(), + locationName = "mock", + active = false + ) val relay9b = - Relay(name = "se9-wireguard", location = mockk(), locationName = "mock", active = false) + RelayItem.Relay( + name = "se9-wireguard", + location = mockk(), + locationName = "mock", + active = false + ) assertTrue(RelayNameComparator.compare(relay9a, relay9b) == 0) assertTrue(RelayNameComparator.compare(relay9b, relay9a) == 0) @@ -42,11 +57,13 @@ class RelayNameComparatorTest { @Test fun test_compare_only_numbers_in_name() { val relay001 = - Relay(name = "001", location = mockk(), locationName = "mock", active = false) - val relay1 = Relay(name = "1", location = mockk(), locationName = "mock", active = false) - val relay3 = Relay(name = "3", location = mockk(), locationName = "mock", active = false) + RelayItem.Relay(name = "001", location = mockk(), locationName = "mock", active = false) + val relay1 = + RelayItem.Relay(name = "1", location = mockk(), locationName = "mock", active = false) + val relay3 = + RelayItem.Relay(name = "3", location = mockk(), locationName = "mock", active = false) val relay100 = - Relay(name = "100", location = mockk(), locationName = "mock", active = false) + RelayItem.Relay(name = "100", location = mockk(), locationName = "mock", active = false) relay001 assertOrderBothDirection relay1 relay001 assertOrderBothDirection relay3 @@ -57,9 +74,19 @@ class RelayNameComparatorTest { @Test fun test_compare_without_numbers_in_name() { val relay9a = - Relay(name = "se-wireguard", location = mockk(), locationName = "mock", active = false) + RelayItem.Relay( + name = "se-wireguard", + location = mockk(), + locationName = "mock", + active = false + ) val relay9b = - Relay(name = "se-wireguard", location = mockk(), locationName = "mock", active = false) + RelayItem.Relay( + name = "se-wireguard", + location = mockk(), + locationName = "mock", + active = false + ) assertTrue(RelayNameComparator.compare(relay9a, relay9b) == 0) assertTrue(RelayNameComparator.compare(relay9b, relay9a) == 0) @@ -68,14 +95,14 @@ class RelayNameComparatorTest { @Test fun test_compare_with_trailing_zeros_in_name() { val relay001 = - Relay( + RelayItem.Relay( name = "se001-wireguard", location = mockk(), locationName = "mock", active = false ) val relay005 = - Relay( + RelayItem.Relay( name = "se005-wireguard", location = mockk(), locationName = "mock", @@ -88,13 +115,28 @@ class RelayNameComparatorTest { @Test fun test_compare_prefix_and_numbers() { val relayAr2 = - Relay(name = "ar2-wireguard", location = mockk(), locationName = "mock", active = false) + RelayItem.Relay( + name = "ar2-wireguard", + location = mockk(), + locationName = "mock", + active = false + ) val relayAr8 = - Relay(name = "ar8-wireguard", location = mockk(), locationName = "mock", active = false) + RelayItem.Relay( + name = "ar8-wireguard", + location = mockk(), + locationName = "mock", + active = false + ) val relaySe5 = - Relay(name = "se5-wireguard", location = mockk(), locationName = "mock", active = false) + RelayItem.Relay( + name = "se5-wireguard", + location = mockk(), + locationName = "mock", + active = false + ) val relaySe10 = - Relay( + RelayItem.Relay( name = "se10-wireguard", location = mockk(), locationName = "mock", @@ -109,9 +151,19 @@ class RelayNameComparatorTest { @Test fun test_compare_suffix_and_numbers() { val relay2c = - Relay(name = "se2-cloud", location = mockk(), locationName = "mock", active = false) + RelayItem.Relay( + name = "se2-cloud", + location = mockk(), + locationName = "mock", + active = false + ) val relay2w = - Relay(name = "se2-wireguard", location = mockk(), locationName = "mock", active = false) + RelayItem.Relay( + name = "se2-wireguard", + location = mockk(), + locationName = "mock", + active = false + ) relay2c assertOrderBothDirection relay2w } @@ -119,9 +171,14 @@ class RelayNameComparatorTest { @Test fun test_compare_different_length() { val relay22a = - Relay(name = "se22", location = mockk(), locationName = "mock", active = false) + RelayItem.Relay( + name = "se22", + location = mockk(), + locationName = "mock", + active = false + ) val relay22b = - Relay( + RelayItem.Relay( name = "se22-wireguard", location = mockk(), locationName = "mock", @@ -131,7 +188,7 @@ class RelayNameComparatorTest { relay22a assertOrderBothDirection relay22b } - private infix fun Relay.assertOrderBothDirection(other: Relay) { + private infix fun RelayItem.Relay.assertOrderBothDirection(other: RelayItem.Relay) { assertTrue(RelayNameComparator.compare(this, other) < 0) assertTrue(RelayNameComparator.compare(other, this) > 0) } diff --git a/android/app/src/test/kotlin/net/mullvad/mullvadvpn/viewmodel/ConnectViewModelTest.kt b/android/app/src/test/kotlin/net/mullvad/mullvadvpn/viewmodel/ConnectViewModelTest.kt index a1e3ce57e57b..57eeef5ca432 100644 --- a/android/app/src/test/kotlin/net/mullvad/mullvadvpn/viewmodel/ConnectViewModelTest.kt +++ b/android/app/src/test/kotlin/net/mullvad/mullvadvpn/viewmodel/ConnectViewModelTest.kt @@ -22,7 +22,6 @@ import net.mullvad.mullvadvpn.model.AccountExpiry import net.mullvad.mullvadvpn.model.DeviceState import net.mullvad.mullvadvpn.model.GeoIpLocation import net.mullvad.mullvadvpn.model.TunnelState -import net.mullvad.mullvadvpn.relaylist.RelayCountry import net.mullvad.mullvadvpn.relaylist.RelayItem import net.mullvad.mullvadvpn.repository.AccountRepository import net.mullvad.mullvadvpn.repository.DeviceRepository @@ -96,7 +95,7 @@ class ConnectViewModelTest { EventNotifier(TunnelState.Disconnected()) // Flows - private val selectedRelayFlow = MutableStateFlow(null) + private val selectedRelayItemFlow = MutableStateFlow(null) // Out Of Time Use Case private val outOfTimeUseCase: OutOfTimeUseCase = mockk() @@ -131,7 +130,7 @@ class ConnectViewModelTest { every { mockAppVersionInfoCache.onUpdate = any() } answers {} // Flows - every { mockRelayListUseCase.selectedRelayItem() } returns selectedRelayFlow + every { mockRelayListUseCase.selectedRelayItem() } returns selectedRelayItemFlow every { outOfTimeUseCase.isOutOfTime() } returns outOfTimeViewFlow viewModel = @@ -188,17 +187,17 @@ class ConnectViewModelTest { } @Test - fun testRelayItemUpdate() = runTest { - val relayTestItem = - RelayCountry(name = "Name", code = "Code", expanded = false, cities = emptyList()) - selectedRelayFlow.value = relayTestItem + fun testSelectedLocationUpdate() = runTest { + val selectedRelayItem = + RelayItem.Country(name = "Name", code = "Code", expanded = false, cities = emptyList()) + selectedRelayItemFlow.value = selectedRelayItem viewModel.uiState.test { assertEquals(ConnectUiState.INITIAL, awaitItem()) serviceConnectionState.value = ServiceConnectionState.ConnectedReady(mockServiceConnectionContainer) val result = awaitItem() - assertEquals(relayTestItem, result.relayLocation) + assertEquals(selectedRelayItem, result.selectedRelayItem) } } diff --git a/android/app/src/test/kotlin/net/mullvad/mullvadvpn/viewmodel/SelectLocationViewModelTest.kt b/android/app/src/test/kotlin/net/mullvad/mullvadvpn/viewmodel/SelectLocationViewModelTest.kt index fc6408d8ab7e..4224563d3b62 100644 --- a/android/app/src/test/kotlin/net/mullvad/mullvadvpn/viewmodel/SelectLocationViewModelTest.kt +++ b/android/app/src/test/kotlin/net/mullvad/mullvadvpn/viewmodel/SelectLocationViewModelTest.kt @@ -20,13 +20,14 @@ import net.mullvad.mullvadvpn.lib.common.test.TestCoroutineRule import net.mullvad.mullvadvpn.lib.common.test.assertLists import net.mullvad.mullvadvpn.model.Constraint import net.mullvad.mullvadvpn.model.GeographicLocationConstraint +import net.mullvad.mullvadvpn.model.LocationConstraint import net.mullvad.mullvadvpn.model.Ownership import net.mullvad.mullvadvpn.model.Providers import net.mullvad.mullvadvpn.relaylist.Provider -import net.mullvad.mullvadvpn.relaylist.RelayCountry import net.mullvad.mullvadvpn.relaylist.RelayItem import net.mullvad.mullvadvpn.relaylist.RelayList import net.mullvad.mullvadvpn.relaylist.filterOnSearchTerm +import net.mullvad.mullvadvpn.relaylist.toLocationConstraint import net.mullvad.mullvadvpn.ui.serviceconnection.ConnectionProxy import net.mullvad.mullvadvpn.ui.serviceconnection.ServiceConnectionManager import net.mullvad.mullvadvpn.ui.serviceconnection.connectionProxy @@ -43,7 +44,8 @@ class SelectLocationViewModelTest { private val mockRelayListFilterUseCase: RelayListFilterUseCase = mockk(relaxed = true) private val mockServiceConnectionManager: ServiceConnectionManager = mockk() private lateinit var viewModel: SelectLocationViewModel - private val relayListWithSelectionFlow = MutableStateFlow(RelayList(emptyList(), null)) + private val relayListWithSelectionFlow = + MutableStateFlow(RelayList(emptyList(), emptyList(), null)) private val mockRelayListUseCase: RelayListUseCase = mockk() private val selectedOwnership = MutableStateFlow>(Constraint.Any()) private val selectedProvider = MutableStateFlow>(Constraint.Any()) @@ -60,6 +62,7 @@ class SelectLocationViewModelTest { mockkStatic(SERVICE_CONNECTION_MANAGER_EXTENSIONS) mockkStatic(RELAY_LIST_EXTENSIONS) + mockkStatic(RELAY_ITEM_EXTENSIONS) viewModel = SelectLocationViewModel( mockServiceConnectionManager, @@ -82,10 +85,11 @@ class SelectLocationViewModelTest { @Test fun testUpdateLocations() = runTest { // Arrange - val mockCountries = listOf(mockk(), mockk()) - val selectedRelay: RelayItem = mockk() - every { mockCountries.filterOnSearchTerm(any(), selectedRelay) } returns mockCountries - relayListWithSelectionFlow.value = RelayList(mockCountries, selectedRelay) + val mockCountries = listOf(mockk(), mockk()) + val mockCustomList = listOf(mockk()) + val selectedItem: RelayItem = mockk() + every { mockCountries.filterOnSearchTerm(any(), selectedItem) } returns mockCountries + relayListWithSelectionFlow.value = RelayList(mockCustomList, mockCountries, selectedItem) // Act, Assert viewModel.uiState.test { @@ -97,8 +101,8 @@ class SelectLocationViewModelTest { (actualState.relayListState as RelayListState.RelayList).countries ) assertEquals( - selectedRelay, - (actualState.relayListState as RelayListState.RelayList).selectedRelay + selectedItem, + (actualState.relayListState as RelayListState.RelayList).selectedItem ) } } @@ -106,10 +110,11 @@ class SelectLocationViewModelTest { @Test fun testUpdateLocationsNoSelectedRelay() = runTest { // Arrange - val mockCountries = listOf(mockk(), mockk()) - val selectedRelay: RelayItem? = null - every { mockCountries.filterOnSearchTerm(any(), selectedRelay) } returns mockCountries - relayListWithSelectionFlow.value = RelayList(mockCountries, selectedRelay) + val mockCustomList = listOf(mockk()) + val mockCountries = listOf(mockk(), mockk()) + val selectedItem: RelayItem? = null + every { mockCountries.filterOnSearchTerm(any(), selectedItem) } returns mockCountries + relayListWithSelectionFlow.value = RelayList(mockCustomList, mockCountries, selectedItem) // Act, Assert viewModel.uiState.test { @@ -121,8 +126,8 @@ class SelectLocationViewModelTest { (actualState.relayListState as RelayListState.RelayList).countries ) assertEquals( - selectedRelay, - (actualState.relayListState as RelayListState.RelayList).selectedRelay + selectedItem, + (actualState.relayListState as RelayListState.RelayList).selectedItem ) } } @@ -130,12 +135,15 @@ class SelectLocationViewModelTest { @Test fun testSelectRelayAndClose() = runTest { // Arrange - val mockRelayItem: RelayItem = mockk() + val mockRelayItem: RelayItem.Country = mockk() val mockLocation: GeographicLocationConstraint.Country = mockk(relaxed = true) + val mockLocationConstraint: LocationConstraint = mockk() val connectionProxyMock: ConnectionProxy = mockk(relaxUnitFun = true) every { mockRelayItem.location } returns mockLocation every { mockServiceConnectionManager.connectionProxy() } returns connectionProxyMock - every { mockRelayListUseCase.updateSelectedRelayLocation(mockLocation) } returns Unit + every { mockRelayListUseCase.updateSelectedRelayLocation(mockLocationConstraint) } returns + Unit + every { mockRelayItem.toLocationConstraint() } returns mockLocationConstraint // Act, Assert viewModel.uiSideEffect.test { @@ -144,7 +152,7 @@ class SelectLocationViewModelTest { assertEquals(SelectLocationSideEffect.CloseScreen, awaitItem()) verify { connectionProxyMock.connect() - mockRelayListUseCase.updateSelectedRelayLocation(mockLocation) + mockRelayListUseCase.updateSelectedRelayLocation(mockLocationConstraint) } } } @@ -152,13 +160,14 @@ class SelectLocationViewModelTest { @Test fun testFilterRelay() = runTest { // Arrange - val mockCountries = listOf(mockk(), mockk()) - val selectedRelay: RelayItem? = null - val mockRelayList: List = mockk(relaxed = true) + val mockCustomList = listOf(mockk()) + val mockCountries = listOf(mockk(), mockk()) + val selectedItem: RelayItem? = null + val mockRelayList: List = mockk(relaxed = true) val mockSearchString = "SEARCH" - every { mockRelayList.filterOnSearchTerm(mockSearchString, selectedRelay) } returns + every { mockRelayList.filterOnSearchTerm(mockSearchString, selectedItem) } returns mockCountries - relayListWithSelectionFlow.value = RelayList(mockRelayList, selectedRelay) + relayListWithSelectionFlow.value = RelayList(mockCustomList, mockRelayList, selectedItem) // Act, Assert viewModel.uiState.test { @@ -177,8 +186,8 @@ class SelectLocationViewModelTest { (actualState.relayListState as RelayListState.RelayList).countries ) assertEquals( - selectedRelay, - (actualState.relayListState as RelayListState.RelayList).selectedRelay + selectedItem, + (actualState.relayListState as RelayListState.RelayList).selectedItem ) } } @@ -186,13 +195,14 @@ class SelectLocationViewModelTest { @Test fun testFilterNotFound() = runTest { // Arrange - val mockCountries = emptyList() - val selectedRelay: RelayItem? = null - val mockRelayList: List = mockk(relaxed = true) + val mockCustomList = listOf(mockk()) + val mockCountries = emptyList() + val selectedItem: RelayItem? = null + val mockRelayList: List = mockk(relaxed = true) val mockSearchString = "SEARCH" - every { mockRelayList.filterOnSearchTerm(mockSearchString, selectedRelay) } returns + every { mockRelayList.filterOnSearchTerm(mockSearchString, selectedItem) } returns mockCountries - relayListWithSelectionFlow.value = RelayList(mockRelayList, selectedRelay) + relayListWithSelectionFlow.value = RelayList(mockCustomList, mockRelayList, selectedItem) // Act, Assert viewModel.uiState.test { @@ -250,5 +260,7 @@ class SelectLocationViewModelTest { "net.mullvad.mullvadvpn.ui.serviceconnection.ServiceConnectionManagerExtensionsKt" private const val RELAY_LIST_EXTENSIONS = "net.mullvad.mullvadvpn.relaylist.RelayListExtensionsKt" + private const val RELAY_ITEM_EXTENSIONS = + "net.mullvad.mullvadvpn.relaylist.RelayItemExtensionsKt" } } From af3df963648d4cc535b552f75cf3abdfe936b395 Mon Sep 17 00:00:00 2001 From: Jonatan Rhodin Date: Wed, 7 Feb 2024 23:36:54 +0100 Subject: [PATCH 11/11] Update screen tests for new relay item --- .../compose/screen/ConnectScreenTest.kt | 66 +++++++++---------- .../screen/SelectLocationScreenTest.kt | 6 +- 2 files changed, 36 insertions(+), 36 deletions(-) diff --git a/android/app/src/androidTest/kotlin/net/mullvad/mullvadvpn/compose/screen/ConnectScreenTest.kt b/android/app/src/androidTest/kotlin/net/mullvad/mullvadvpn/compose/screen/ConnectScreenTest.kt index 39e11a3c142a..2e568676e832 100644 --- a/android/app/src/androidTest/kotlin/net/mullvad/mullvadvpn/compose/screen/ConnectScreenTest.kt +++ b/android/app/src/androidTest/kotlin/net/mullvad/mullvadvpn/compose/screen/ConnectScreenTest.kt @@ -78,7 +78,7 @@ class ConnectScreenTest { uiState = ConnectUiState( location = null, - relayLocation = null, + selectedRelayItem = null, tunnelUiState = TunnelState.Connecting(null, null), tunnelRealState = TunnelState.Connecting(null, null), inAddress = null, @@ -112,7 +112,7 @@ class ConnectScreenTest { uiState = ConnectUiState( location = null, - relayLocation = null, + selectedRelayItem = null, tunnelUiState = TunnelState.Connecting(endpoint = mockTunnelEndpoint, null), tunnelRealState = @@ -147,7 +147,7 @@ class ConnectScreenTest { uiState = ConnectUiState( location = null, - relayLocation = null, + selectedRelayItem = null, tunnelUiState = TunnelState.Connected(mockTunnelEndpoint, null), tunnelRealState = TunnelState.Connected(mockTunnelEndpoint, null), inAddress = null, @@ -179,7 +179,7 @@ class ConnectScreenTest { uiState = ConnectUiState( location = null, - relayLocation = null, + selectedRelayItem = null, tunnelUiState = TunnelState.Connected(mockTunnelEndpoint, null), tunnelRealState = TunnelState.Connected(mockTunnelEndpoint, null), inAddress = null, @@ -204,15 +204,15 @@ class ConnectScreenTest { fun testDisconnectingState() { composeExtension.use { // Arrange - val mockRelayLocation: RelayItem = mockk(relaxed = true) + val mockSelectedLocation: RelayItem = mockk(relaxed = true) val mockLocationName = "Home" - every { mockRelayLocation.locationName } returns mockLocationName + every { mockSelectedLocation.locationName } returns mockLocationName setContentWithTheme { ConnectScreen( uiState = ConnectUiState( location = null, - relayLocation = mockRelayLocation, + selectedRelayItem = mockSelectedLocation, tunnelUiState = TunnelState.Disconnecting(ActionAfterDisconnect.Nothing), tunnelRealState = @@ -239,15 +239,15 @@ class ConnectScreenTest { fun testDisconnectedState() { composeExtension.use { // Arrange - val mockRelayLocation: RelayItem = mockk(relaxed = true) + val mockSelectedLocation: RelayItem = mockk(relaxed = true) val mockLocationName = "Home" - every { mockRelayLocation.locationName } returns mockLocationName + every { mockSelectedLocation.locationName } returns mockLocationName setContentWithTheme { ConnectScreen( uiState = ConnectUiState( location = null, - relayLocation = mockRelayLocation, + selectedRelayItem = mockSelectedLocation, tunnelUiState = TunnelState.Disconnected(), tunnelRealState = TunnelState.Disconnected(), inAddress = null, @@ -272,15 +272,15 @@ class ConnectScreenTest { fun testErrorStateBlocked() { composeExtension.use { // Arrange - val mockRelayLocation: RelayItem = mockk(relaxed = true) + val mockSelectedLocation: RelayItem = mockk(relaxed = true) val mockLocationName = "Home" - every { mockRelayLocation.locationName } returns mockLocationName + every { mockSelectedLocation.locationName } returns mockLocationName setContentWithTheme { ConnectScreen( uiState = ConnectUiState( location = null, - relayLocation = mockRelayLocation, + selectedRelayItem = mockSelectedLocation, tunnelUiState = TunnelState.Error( ErrorState(ErrorStateCause.StartTunnelError, true) @@ -315,15 +315,15 @@ class ConnectScreenTest { fun testErrorStateNotBlocked() { composeExtension.use { // Arrange - val mockRelayLocation: RelayItem = mockk(relaxed = true) + val mockSelectedLocation: RelayItem = mockk(relaxed = true) val mockLocationName = "Home" - every { mockRelayLocation.locationName } returns mockLocationName + every { mockSelectedLocation.locationName } returns mockLocationName setContentWithTheme { ConnectScreen( uiState = ConnectUiState( location = null, - relayLocation = mockRelayLocation, + selectedRelayItem = mockSelectedLocation, tunnelUiState = TunnelState.Error( ErrorState(ErrorStateCause.StartTunnelError, false) @@ -364,7 +364,7 @@ class ConnectScreenTest { uiState = ConnectUiState( location = null, - relayLocation = null, + selectedRelayItem = null, tunnelUiState = TunnelState.Disconnecting(ActionAfterDisconnect.Reconnect), tunnelRealState = @@ -393,15 +393,15 @@ class ConnectScreenTest { fun testDisconnectingBlockState() { composeExtension.use { // Arrange - val mockRelayLocation: RelayItem = mockk(relaxed = true) + val mockSelectedLocation: RelayItem = mockk(relaxed = true) val mockLocationName = "Home" - every { mockRelayLocation.locationName } returns mockLocationName + every { mockSelectedLocation.locationName } returns mockLocationName setContentWithTheme { ConnectScreen( uiState = ConnectUiState( location = null, - relayLocation = mockRelayLocation, + selectedRelayItem = mockSelectedLocation, tunnelUiState = TunnelState.Disconnecting(ActionAfterDisconnect.Block), tunnelRealState = TunnelState.Disconnecting(ActionAfterDisconnect.Block), @@ -428,16 +428,16 @@ class ConnectScreenTest { fun testClickSelectLocationButton() { composeExtension.use { // Arrange - val mockRelayLocation: RelayItem = mockk(relaxed = true) + val mockSelectedLocation: RelayItem = mockk(relaxed = true) val mockLocationName = "Home" - every { mockRelayLocation.locationName } returns mockLocationName + every { mockSelectedLocation.name } returns mockLocationName val mockedClickHandler: () -> Unit = mockk(relaxed = true) setContentWithTheme { ConnectScreen( uiState = ConnectUiState( location = null, - relayLocation = mockRelayLocation, + selectedRelayItem = mockSelectedLocation, tunnelUiState = TunnelState.Disconnected(), tunnelRealState = TunnelState.Disconnected(), inAddress = null, @@ -471,7 +471,7 @@ class ConnectScreenTest { uiState = ConnectUiState( location = null, - relayLocation = null, + selectedRelayItem = null, tunnelUiState = TunnelState.Connected(mockTunnelEndpoint, null), tunnelRealState = TunnelState.Connected(mockTunnelEndpoint, null), inAddress = null, @@ -505,7 +505,7 @@ class ConnectScreenTest { uiState = ConnectUiState( location = null, - relayLocation = null, + selectedRelayItem = null, tunnelUiState = TunnelState.Connected(mockTunnelEndpoint, null), tunnelRealState = TunnelState.Connected(mockTunnelEndpoint, null), inAddress = null, @@ -538,7 +538,7 @@ class ConnectScreenTest { uiState = ConnectUiState( location = null, - relayLocation = null, + selectedRelayItem = null, tunnelUiState = TunnelState.Disconnected(), tunnelRealState = TunnelState.Disconnected(), inAddress = null, @@ -571,7 +571,7 @@ class ConnectScreenTest { uiState = ConnectUiState( location = null, - relayLocation = null, + selectedRelayItem = null, tunnelUiState = TunnelState.Connecting(null, null), tunnelRealState = TunnelState.Connecting(null, null), inAddress = null, @@ -612,7 +612,7 @@ class ConnectScreenTest { uiState = ConnectUiState( location = mockLocation, - relayLocation = null, + selectedRelayItem = null, tunnelUiState = TunnelState.Connected(mockTunnelEndpoint, null), tunnelRealState = TunnelState.Connected(mockTunnelEndpoint, null), inAddress = mockInAddress, @@ -653,7 +653,7 @@ class ConnectScreenTest { uiState = ConnectUiState( location = null, - relayLocation = null, + selectedRelayItem = null, tunnelUiState = TunnelState.Connecting(null, null), tunnelRealState = TunnelState.Connecting(null, null), inAddress = null, @@ -689,7 +689,7 @@ class ConnectScreenTest { uiState = ConnectUiState( location = null, - relayLocation = null, + selectedRelayItem = null, tunnelUiState = TunnelState.Connecting(null, null), tunnelRealState = TunnelState.Connecting(null, null), inAddress = null, @@ -722,7 +722,7 @@ class ConnectScreenTest { uiState = ConnectUiState( location = null, - relayLocation = null, + selectedRelayItem = null, tunnelUiState = TunnelState.Connecting(null, null), tunnelRealState = TunnelState.Connecting(null, null), inAddress = null, @@ -760,7 +760,7 @@ class ConnectScreenTest { uiState = ConnectUiState( location = null, - relayLocation = null, + selectedRelayItem = null, tunnelUiState = TunnelState.Connecting(null, null), tunnelRealState = TunnelState.Connecting(null, null), inAddress = null, @@ -794,7 +794,7 @@ class ConnectScreenTest { uiState = ConnectUiState( location = null, - relayLocation = null, + selectedRelayItem = null, tunnelUiState = TunnelState.Connecting(null, null), tunnelRealState = TunnelState.Connecting(null, null), inAddress = null, diff --git a/android/app/src/androidTest/kotlin/net/mullvad/mullvadvpn/compose/screen/SelectLocationScreenTest.kt b/android/app/src/androidTest/kotlin/net/mullvad/mullvadvpn/compose/screen/SelectLocationScreenTest.kt index bb64adc8a68f..12f4b1032388 100644 --- a/android/app/src/androidTest/kotlin/net/mullvad/mullvadvpn/compose/screen/SelectLocationScreenTest.kt +++ b/android/app/src/androidTest/kotlin/net/mullvad/mullvadvpn/compose/screen/SelectLocationScreenTest.kt @@ -59,7 +59,7 @@ class SelectLocationScreenTest { relayListState = RelayListState.RelayList( countries = DUMMY_RELAY_COUNTRIES, - selectedRelay = null + selectedItem = null ), selectedOwnership = null, selectedProvidersCount = 0, @@ -99,7 +99,7 @@ class SelectLocationScreenTest { relayListState = RelayListState.RelayList( countries = updatedDummyList, - selectedRelay = updatedDummyList[0].cities[0].relays[0] + selectedItem = updatedDummyList[0].cities[0].relays[0] ), selectedOwnership = null, selectedProvidersCount = 0, @@ -129,7 +129,7 @@ class SelectLocationScreenTest { relayListState = RelayListState.RelayList( countries = emptyList(), - selectedRelay = null + selectedItem = null ), selectedOwnership = null, selectedProvidersCount = 0,