From d2b5f99951c2b9281fa934df71157ca2a62bd5b9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kalle=20Lindstr=C3=B6m?= Date: Wed, 23 Oct 2024 16:46:31 +0200 Subject: [PATCH 1/2] Improve DnsDialogViewModel nav arguments --- .../compose/screen/VpnSettingsScreenTest.kt | 4 ++-- .../mullvadvpn/compose/dialog/DnsDialog.kt | 2 +- .../compose/screen/VpnSettingsScreen.kt | 14 +++++------ .../viewmodel/DnsDialogViewModel.kt | 24 ++++++++++++------- 4 files changed, 25 insertions(+), 19 deletions(-) diff --git a/android/app/src/androidTest/kotlin/net/mullvad/mullvadvpn/compose/screen/VpnSettingsScreenTest.kt b/android/app/src/androidTest/kotlin/net/mullvad/mullvadvpn/compose/screen/VpnSettingsScreenTest.kt index 45edfcd2048e..8405ad249f19 100644 --- a/android/app/src/androidTest/kotlin/net/mullvad/mullvadvpn/compose/screen/VpnSettingsScreenTest.kt +++ b/android/app/src/androidTest/kotlin/net/mullvad/mullvadvpn/compose/screen/VpnSettingsScreenTest.kt @@ -378,7 +378,7 @@ class VpnSettingsScreenTest { fun testClickAddDns() = composeExtension.use { // Arrange - val mockedClickHandler: (Int?, String?) -> Unit = mockk(relaxed = true) + val mockedClickHandler: (Int?) -> Unit = mockk(relaxed = true) setContentWithTheme { VpnSettingsScreen( state = VpnSettingsUiState.createDefault(isCustomDnsEnabled = true), @@ -390,7 +390,7 @@ class VpnSettingsScreenTest { onNodeWithText("Add a server").performClick() // Assert - verify { mockedClickHandler.invoke(null, null) } + verify { mockedClickHandler.invoke(null) } } @Test diff --git a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/dialog/DnsDialog.kt b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/dialog/DnsDialog.kt index c2e94ded5bf0..542cd885d48d 100644 --- a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/dialog/DnsDialog.kt +++ b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/dialog/DnsDialog.kt @@ -42,7 +42,7 @@ private fun PreviewDnsDialogEditAllowLanDisabled() { AppTheme { DnsDialog(DnsDialogViewState("192.168.1.1", null, true, false, 0), {}, {}, {}, {}) } } -data class DnsDialogNavArgs(val index: Int? = null, val initialValue: String? = null) +data class DnsDialogNavArgs(val index: Int? = null) @Destination(style = DestinationStyle.Dialog::class, navArgs = DnsDialogNavArgs::class) @Composable diff --git a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/screen/VpnSettingsScreen.kt b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/screen/VpnSettingsScreen.kt index 24b19cfae6f3..a685a166fd5c 100644 --- a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/screen/VpnSettingsScreen.kt +++ b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/screen/VpnSettingsScreen.kt @@ -123,7 +123,7 @@ private fun PreviewVpnSettings( onToggleBlockGambling = {}, onToggleBlockSocialMedia = {}, navigateToMtuDialog = {}, - navigateToDns = { _, _ -> }, + navigateToDns = { _ -> }, onToggleDnsClick = {}, onBackClick = {}, onSelectObfuscationMode = {}, @@ -184,7 +184,7 @@ fun VpnSettings( is VpnSettingsSideEffect.ShowToast -> launch { snackbarHostState.showSnackbarImmediately(message = it.message(context)) } VpnSettingsSideEffect.NavigateToDnsDialog -> - navigator.navigate(DnsDestination(null, null)) { launchSingleTop = true } + navigator.navigate(DnsDestination(null)) { launchSingleTop = true } } } @@ -239,9 +239,7 @@ fun VpnSettings( navigateToMtuDialog = dropUnlessResumed { mtu: Mtu? -> navigator.navigate(MtuDestination(mtu)) }, navigateToDns = - dropUnlessResumed { index: Int?, address: String? -> - navigator.navigate(DnsDestination(index, address)) - }, + dropUnlessResumed { index: Int? -> navigator.navigate(DnsDestination(index)) }, navigateToWireguardPortDialog = dropUnlessResumed { navigator.navigate( @@ -293,7 +291,7 @@ fun VpnSettingsScreen( onToggleBlockGambling: (Boolean) -> Unit = {}, onToggleBlockSocialMedia: (Boolean) -> Unit = {}, navigateToMtuDialog: (mtu: Mtu?) -> Unit = {}, - navigateToDns: (index: Int?, address: String?) -> Unit = { _, _ -> }, + navigateToDns: (index: Int?) -> Unit = { _ -> }, onToggleDnsClick: (Boolean) -> Unit = {}, onBackClick: () -> Unit = {}, onSelectObfuscationMode: (obfuscationMode: ObfuscationMode) -> Unit = {}, @@ -468,14 +466,14 @@ fun VpnSettingsScreen( address = item.address, isUnreachableLocalDnsWarningVisible = item.isLocal && !state.isLocalNetworkSharingEnabled, - onClick = { navigateToDns(index, item.address) }, + onClick = { navigateToDns(index) }, modifier = Modifier.animateItem(), ) } itemWithDivider { BaseCell( - onCellClicked = { navigateToDns(null, null) }, + onCellClicked = { navigateToDns(null) }, headlineContent = { Text( text = stringResource(id = R.string.add_a_server), diff --git a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/viewmodel/DnsDialogViewModel.kt b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/viewmodel/DnsDialogViewModel.kt index a009210318de..efd8eb235c97 100644 --- a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/viewmodel/DnsDialogViewModel.kt +++ b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/viewmodel/DnsDialogViewModel.kt @@ -22,6 +22,7 @@ import kotlinx.coroutines.flow.stateIn import kotlinx.coroutines.launch import net.mullvad.mullvadvpn.lib.model.Settings import net.mullvad.mullvadvpn.repository.SettingsRepository +import net.mullvad.talpid.util.addressString import org.apache.commons.validator.routines.InetAddressValidator sealed interface DnsDialogSideEffect { @@ -57,20 +58,27 @@ class DnsDialogViewModel( private val navArgs = DnsDestination.argsFrom(savedStateHandle) private val settings = MutableStateFlow(null) - private val currentIndex = MutableStateFlow(navArgs.index) - private val _ipAddressInput = MutableStateFlow(navArgs.initialValue ?: EMPTY_STRING) + private val _ipAddressInput = MutableStateFlow(null) val uiState: StateFlow = - combine(_ipAddressInput, currentIndex, settings.filterNotNull()) { - input, - currentIndex, - settings -> - createViewState(settings.addresses(), currentIndex, settings.allowLan, input) + combine(_ipAddressInput, settings.filterNotNull()) { inputAddress, settings -> + val dnsList = settings.addresses() + val input = + inputAddress + ?: navArgs.index?.let { index -> dnsList[index].addressString() } + ?: EMPTY_STRING + + createViewState( + customDnsList = dnsList, + currentIndex = navArgs.index, + isAllowLanEnabled = settings.allowLan, + input = input, + ) } .stateIn( viewModelScope, SharingStarted.Lazily, - createViewState(emptyList(), null, false, _ipAddressInput.value), + createViewState(emptyList(), null, false, EMPTY_STRING), ) private val _uiSideEffect = Channel() From 4d4111d32b6c9ab42864950533c9fe54e682a5da Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kalle=20Lindstr=C3=B6m?= Date: Wed, 30 Oct 2024 14:39:59 +0100 Subject: [PATCH 2/2] Add Lce --- .../mullvadvpn/compose/dialog/DnsDialog.kt | 5 +- .../viewmodel/DnsDialogViewModel.kt | 89 +++++++++++-------- 2 files changed, 54 insertions(+), 40 deletions(-) diff --git a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/dialog/DnsDialog.kt b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/dialog/DnsDialog.kt index 542cd885d48d..4665e5ca0192 100644 --- a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/dialog/DnsDialog.kt +++ b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/dialog/DnsDialog.kt @@ -57,7 +57,8 @@ fun Dns(resultNavigator: ResultBackNavigator) { resultNavigator.navigateBack(result = DnsDialogResult.Error) } } - val state by viewModel.uiState.collectAsStateWithLifecycle() + val stateLc by viewModel.uiState.collectAsStateWithLifecycle() + val state = stateLc.content() ?: return DnsDialog( state, @@ -69,6 +70,8 @@ fun Dns(resultNavigator: ResultBackNavigator) { ) } +// 4490997557103082 + @Composable fun DnsDialog( state: DnsDialogViewState, diff --git a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/viewmodel/DnsDialogViewModel.kt b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/viewmodel/DnsDialogViewModel.kt index efd8eb235c97..ef485bc02a2a 100644 --- a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/viewmodel/DnsDialogViewModel.kt +++ b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/viewmodel/DnsDialogViewModel.kt @@ -16,13 +16,11 @@ import kotlinx.coroutines.flow.SharingStarted import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.combine import kotlinx.coroutines.flow.filterNotNull -import kotlinx.coroutines.flow.first import kotlinx.coroutines.flow.receiveAsFlow import kotlinx.coroutines.flow.stateIn import kotlinx.coroutines.launch import net.mullvad.mullvadvpn.lib.model.Settings import net.mullvad.mullvadvpn.repository.SettingsRepository -import net.mullvad.talpid.util.addressString import org.apache.commons.validator.routines.InetAddressValidator sealed interface DnsDialogSideEffect { @@ -31,6 +29,33 @@ sealed interface DnsDialogSideEffect { data object Error : DnsDialogSideEffect } +sealed interface Lce { + data object Loading : Lce + + data class Content(val value: T) : Lce + + data class Error(val error: E) : Lce +} + +fun T.toLce(): Lce = Lce.Content(this) + +sealed interface Lc { + data object Loading : Lc + + data object Empty : Lc + + data class Content(val value: T) : Lc + + fun content(): T? = + when (this) { + is Loading, + Empty -> null + is Content -> value + } +} + +fun T.toLc(): Lc = Lc.Content(this) + data class DnsDialogViewState( val input: String, val validationError: ValidationError?, @@ -57,50 +82,35 @@ class DnsDialogViewModel( ) : ViewModel() { private val navArgs = DnsDestination.argsFrom(savedStateHandle) - private val settings = MutableStateFlow(null) - private val _ipAddressInput = MutableStateFlow(null) - - val uiState: StateFlow = - combine(_ipAddressInput, settings.filterNotNull()) { inputAddress, settings -> - val dnsList = settings.addresses() - val input = - inputAddress - ?: navArgs.index?.let { index -> dnsList[index].addressString() } - ?: EMPTY_STRING - - createViewState( - customDnsList = dnsList, - currentIndex = navArgs.index, - isAllowLanEnabled = settings.allowLan, - input = input, - ) + private val currentIndex = MutableStateFlow(navArgs.index) + private val ipAddressInput = MutableStateFlow(EMPTY_STRING) + + val uiState: StateFlow> = + combine(currentIndex, ipAddressInput, repository.settingsUpdates.filterNotNull()) { + currentIndex, + input, + settings -> + createViewState(settings.addresses(), currentIndex, settings.allowLan, input) } - .stateIn( - viewModelScope, - SharingStarted.Lazily, - createViewState(emptyList(), null, false, EMPTY_STRING), - ) + .stateIn(viewModelScope, SharingStarted.Lazily, Lc.Loading) private val _uiSideEffect = Channel() val uiSideEffect = _uiSideEffect.receiveAsFlow() - init { - viewModelScope.launch { settings.emit(repository.settingsUpdates.filterNotNull().first()) } - } - private fun createViewState( customDnsList: List, currentIndex: Int?, isAllowLanEnabled: Boolean, input: String, - ): DnsDialogViewState = + ): Lc = DnsDialogViewState( - input, - input.validateDnsEntry(currentIndex, customDnsList).leftOrNull(), - input.isLocalAddress(), - isAllowLanEnabled = isAllowLanEnabled, - currentIndex, - ) + input, + input.validateDnsEntry(currentIndex, customDnsList).leftOrNull(), + input.isLocalAddress(), + isAllowLanEnabled = isAllowLanEnabled, + currentIndex, + ) + .toLc() private fun String.validateDnsEntry( index: Int?, @@ -116,16 +126,17 @@ class DnsDialogViewModel( } fun onDnsInputChange(ipAddress: String) { - _ipAddressInput.value = ipAddress + ipAddressInput.value = ipAddress } fun onSaveDnsClick() = viewModelScope.launch(dispatcher) { - if (!uiState.value.isValid()) return@launch + val state = uiState.value.content() + if (state == null || !state.isValid()) return@launch - val address = InetAddress.getByName(uiState.value.input) + val address = InetAddress.getByName(state.input) - val index = uiState.value.index + val index = state.index val result = if (index != null) { repository.setCustomDns(index = index, address = address)