From 99bd27986089de8dd05b839d2b8361cae8aefcda Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20G=C3=B6ransson?= Date: Fri, 20 Dec 2024 07:04:56 +0100 Subject: [PATCH] Remove one-to-one relationship between provider and ownership --- android/CHANGELOG.md | 3 + .../compose/data/DummyRelayItems.kt | 9 +- .../compose/screen/FilterScreenTest.kt | 94 ++++++------ .../mullvadvpn/compose/cell/CheckboxCell.kt | 3 +- .../FilterUiStatePreviewParameterProvider.kt | 8 +- .../compose/preview/RelayItemPreviewData.kt | 4 +- ...ocationsUiStatePreviewParameterProvider.kt | 2 - .../mullvadvpn/compose/screen/FilterScreen.kt | 44 ++++-- .../state/FilterConstrainExtensions.kt | 13 +- .../compose/state/RelayFilterUiState.kt | 35 +++-- .../net/mullvad/mullvadvpn/di/UiModule.kt | 4 +- .../relaylist/RelayItemExtensions.kt | 5 +- .../mullvadvpn/usecase/FilterChipUseCase.kt | 40 ++--- ...Case.kt => ProviderToOwnershipsUseCase.kt} | 12 +- .../mullvadvpn/viewmodel/FilterViewModel.kt | 55 ++++--- .../usecase/FilterChipUseCaseTest.kt | 48 ++++-- .../CustomListLocationsViewModelTest.kt | 11 +- .../viewmodel/FilterViewModelTest.kt | 85 ++++++----- .../lib/daemon/grpc/mapper/ToDomain.kt | 8 +- .../daemon/grpc/RelayNameComparatorTest.kt | 141 +++++------------- .../mullvad/mullvadvpn/lib/model/Provider.kt | 3 - .../mullvadvpn/lib/model/ProviderId.kt | 6 +- .../mullvad/mullvadvpn/lib/model/RelayItem.kt | 3 +- .../resource/src/main/res/values/strings.xml | 1 + .../packages/mullvad-vpn/locales/messages.pot | 3 + 25 files changed, 322 insertions(+), 318 deletions(-) rename android/app/src/main/kotlin/net/mullvad/mullvadvpn/usecase/{AvailableProvidersUseCase.kt => ProviderToOwnershipsUseCase.kt} (52%) delete mode 100644 android/lib/model/src/main/kotlin/net/mullvad/mullvadvpn/lib/model/Provider.kt diff --git a/android/CHANGELOG.md b/android/CHANGELOG.md index f6b4d8288cf5..09b28934f194 100644 --- a/android/CHANGELOG.md +++ b/android/CHANGELOG.md @@ -33,6 +33,9 @@ Line wrap the file at 100 chars. Th - Update to DAITA v2. The main difference is that many different machines are provided by relays instead of a bundled list. +### Fixed +- Fix a crash that would occur because a Provider would be listed twice in the filter screen. + ## [android/2024.9] - 2024-12-09 ### Changed diff --git a/android/app/src/androidTest/kotlin/net/mullvad/mullvadvpn/compose/data/DummyRelayItems.kt b/android/app/src/androidTest/kotlin/net/mullvad/mullvadvpn/compose/data/DummyRelayItems.kt index fa24c504af05..8e4024a4d550 100644 --- a/android/app/src/androidTest/kotlin/net/mullvad/mullvadvpn/compose/data/DummyRelayItems.kt +++ b/android/app/src/androidTest/kotlin/net/mullvad/mullvadvpn/compose/data/DummyRelayItems.kt @@ -6,7 +6,6 @@ import net.mullvad.mullvadvpn.lib.model.CustomListName import net.mullvad.mullvadvpn.lib.model.GeoLocationId import net.mullvad.mullvadvpn.lib.model.Ownership import net.mullvad.mullvadvpn.lib.model.PortRange -import net.mullvad.mullvadvpn.lib.model.Provider import net.mullvad.mullvadvpn.lib.model.ProviderId import net.mullvad.mullvadvpn.lib.model.RelayItem import net.mullvad.mullvadvpn.lib.model.RelayList @@ -20,8 +19,8 @@ private val DUMMY_RELAY_1 = "Relay host 1", ), active = true, - provider = - Provider(providerId = ProviderId("PROVIDER RENTED"), ownership = Ownership.Rented), + provider = ProviderId("PROVIDER RENTED"), + ownership = Ownership.Rented, daita = false, ) private val DUMMY_RELAY_2 = @@ -32,8 +31,8 @@ private val DUMMY_RELAY_2 = "Relay host 2", ), active = true, - provider = - Provider(providerId = ProviderId("PROVIDER OWNED"), ownership = Ownership.MullvadOwned), + provider = ProviderId("PROVIDER OWNED"), + ownership = Ownership.MullvadOwned, daita = false, ) private val DUMMY_RELAY_CITY_1 = diff --git a/android/app/src/androidTest/kotlin/net/mullvad/mullvadvpn/compose/screen/FilterScreenTest.kt b/android/app/src/androidTest/kotlin/net/mullvad/mullvadvpn/compose/screen/FilterScreenTest.kt index 2f16b27c237a..4b80ea0e3c2b 100644 --- a/android/app/src/androidTest/kotlin/net/mullvad/mullvadvpn/compose/screen/FilterScreenTest.kt +++ b/android/app/src/androidTest/kotlin/net/mullvad/mullvadvpn/compose/screen/FilterScreenTest.kt @@ -11,7 +11,6 @@ import net.mullvad.mullvadvpn.compose.createEdgeToEdgeComposeExtension import net.mullvad.mullvadvpn.compose.setContentWithTheme import net.mullvad.mullvadvpn.compose.state.RelayFilterUiState import net.mullvad.mullvadvpn.lib.model.Ownership -import net.mullvad.mullvadvpn.lib.model.Provider import net.mullvad.mullvadvpn.lib.model.ProviderId import org.junit.jupiter.api.Test import org.junit.jupiter.api.extension.RegisterExtension @@ -30,7 +29,7 @@ class FilterScreenTest { onApplyClick: () -> Unit = {}, onSelectedOwnership: (ownership: Ownership?) -> Unit = {}, onAllProviderCheckChange: (isChecked: Boolean) -> Unit = {}, - onSelectedProvider: (checked: Boolean, provider: Provider) -> Unit = { _, _ -> }, + onSelectedProvider: (checked: Boolean, provider: ProviderId) -> Unit = { _, _ -> }, ) { setContentWithTheme { FilterScreen( @@ -50,7 +49,7 @@ class FilterScreenTest { initScreen( state = RelayFilterUiState( - allProviders = DUMMY_RELAY_ALL_PROVIDERS, + providerToOwnerships = DUMMY_RELAY_ALL_PROVIDERS, selectedOwnership = null, selectedProviders = DUMMY_SELECTED_PROVIDERS, ) @@ -65,7 +64,7 @@ class FilterScreenTest { initScreen( state = RelayFilterUiState( - allProviders = DUMMY_RELAY_ALL_PROVIDERS, + providerToOwnerships = DUMMY_RELAY_ALL_PROVIDERS, selectedOwnership = null, selectedProviders = DUMMY_SELECTED_PROVIDERS, ) @@ -80,7 +79,7 @@ class FilterScreenTest { initScreen( state = RelayFilterUiState( - allProviders = DUMMY_RELAY_ALL_PROVIDERS, + providerToOwnerships = DUMMY_RELAY_ALL_PROVIDERS, selectedOwnership = Ownership.MullvadOwned, selectedProviders = DUMMY_SELECTED_PROVIDERS, ) @@ -95,7 +94,7 @@ class FilterScreenTest { initScreen( state = RelayFilterUiState( - allProviders = DUMMY_RELAY_ALL_PROVIDERS, + providerToOwnerships = DUMMY_RELAY_ALL_PROVIDERS, selectedOwnership = Ownership.Rented, selectedProviders = DUMMY_SELECTED_PROVIDERS, ) @@ -110,7 +109,7 @@ class FilterScreenTest { initScreen( state = RelayFilterUiState( - allProviders = DUMMY_RELAY_ALL_PROVIDERS, + providerToOwnerships = DUMMY_RELAY_ALL_PROVIDERS, selectedOwnership = null, selectedProviders = DUMMY_SELECTED_PROVIDERS, ) @@ -128,10 +127,9 @@ class FilterScreenTest { initScreen( state = RelayFilterUiState( - allProviders = listOf(), + providerToOwnerships = DUMMY_RELAY_ALL_PROVIDERS, selectedOwnership = null, - selectedProviders = - listOf(Provider(ProviderId("31173"), Ownership.MullvadOwned)), + selectedProviders = listOf(ProviderId("31173")), ), onApplyClick = mockClickListener, ) @@ -139,47 +137,47 @@ class FilterScreenTest { verify { mockClickListener() } } + @Test + fun ensureSelectedProviderIsShowEvenThoughItIsNotInAllProviders() = + composeExtension.use { + // Arrange + initScreen( + state = + RelayFilterUiState( + providerToOwnerships = DUMMY_RELAY_ALL_PROVIDERS, + selectedOwnership = null, + selectedProviders = listOf(ProviderId("1RemovedProvider")), + ) + ) + + // Act + onNodeWithText("Providers").performClick() + // Asset + onNodeWithText("1RemovedProvider (removed)").assertExists() + } + companion object { private val DUMMY_RELAY_ALL_PROVIDERS = - listOf( - Provider(ProviderId("31173"), Ownership.MullvadOwned), - Provider(ProviderId("100TB"), Ownership.Rented), - Provider(ProviderId("Blix"), Ownership.MullvadOwned), - Provider(ProviderId("Creanova"), Ownership.MullvadOwned), - Provider(ProviderId("DataPacket"), Ownership.Rented), - Provider(ProviderId("HostRoyale"), Ownership.Rented), - Provider(ProviderId("hostuniversal"), Ownership.Rented), - Provider(ProviderId("iRegister"), Ownership.Rented), - Provider(ProviderId("M247"), Ownership.Rented), - Provider(ProviderId("Makonix"), Ownership.Rented), - Provider(ProviderId("PrivateLayer"), Ownership.Rented), - Provider(ProviderId("ptisp"), Ownership.Rented), - Provider(ProviderId("Qnax"), Ownership.Rented), - Provider(ProviderId("Quadranet"), Ownership.Rented), - Provider(ProviderId("techfutures"), Ownership.Rented), - Provider(ProviderId("Tzulo"), Ownership.Rented), - Provider(ProviderId("xtom"), Ownership.Rented), + mapOf( + ProviderId("31173") to setOf(Ownership.MullvadOwned), + ProviderId("100TB") to setOf(Ownership.Rented), + ProviderId("Blix") to setOf(Ownership.MullvadOwned), + ProviderId("Creanova") to setOf(Ownership.MullvadOwned), + ProviderId("DataPacket") to setOf(Ownership.Rented), + ProviderId("HostRoyale") to setOf(Ownership.Rented), + ProviderId("hostuniversal") to setOf(Ownership.Rented), + ProviderId("iRegister") to setOf(Ownership.Rented), + ProviderId("M247") to setOf(Ownership.Rented), + ProviderId("Makonix") to setOf(Ownership.Rented), + ProviderId("PrivateLayer") to setOf(Ownership.Rented), + ProviderId("ptisp") to setOf(Ownership.Rented), + ProviderId("Qnax") to setOf(Ownership.Rented), + ProviderId("Quadranet") to setOf(Ownership.Rented), + ProviderId("techfutures") to setOf(Ownership.Rented), + ProviderId("Tzulo") to setOf(Ownership.Rented), + ProviderId("xtom") to setOf(Ownership.Rented), ) - private val DUMMY_SELECTED_PROVIDERS = - listOf( - Provider(ProviderId("31173"), Ownership.MullvadOwned), - Provider(ProviderId("100TB"), Ownership.Rented), - Provider(ProviderId("Blix"), Ownership.MullvadOwned), - Provider(ProviderId("Creanova"), Ownership.MullvadOwned), - Provider(ProviderId("DataPacket"), Ownership.Rented), - Provider(ProviderId("HostRoyale"), Ownership.Rented), - Provider(ProviderId("hostuniversal"), Ownership.Rented), - Provider(ProviderId("iRegister"), Ownership.Rented), - Provider(ProviderId("M247"), Ownership.Rented), - Provider(ProviderId("Makonix"), Ownership.Rented), - Provider(ProviderId("PrivateLayer"), Ownership.Rented), - Provider(ProviderId("ptisp"), Ownership.Rented), - Provider(ProviderId("Qnax"), Ownership.Rented), - Provider(ProviderId("Quadranet"), Ownership.Rented), - Provider(ProviderId("techfutures"), Ownership.Rented), - Provider(ProviderId("Tzulo"), Ownership.Rented), - Provider(ProviderId("xtom"), Ownership.Rented), - ) + private val DUMMY_SELECTED_PROVIDERS = DUMMY_RELAY_ALL_PROVIDERS.keys.toList() } } diff --git a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/cell/CheckboxCell.kt b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/cell/CheckboxCell.kt index 13c237d6f482..e1b0c1a5d5a2 100644 --- a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/cell/CheckboxCell.kt +++ b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/cell/CheckboxCell.kt @@ -31,6 +31,7 @@ internal fun CheckboxCell( modifier: Modifier = Modifier, title: String, checked: Boolean, + enabled: Boolean = true, onCheckedChange: (Boolean) -> Unit, background: Color = MaterialTheme.colorScheme.surfaceContainerLow, startPadding: Dp = Dimens.mediumPadding, @@ -41,7 +42,7 @@ internal fun CheckboxCell( verticalAlignment = Alignment.CenterVertically, modifier = modifier - .clickable { onCheckedChange(!checked) } + .clickable(enabled) { onCheckedChange(!checked) } .defaultMinSize(minHeight = minHeight) .fillMaxWidth() .background(background) diff --git a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/preview/FilterUiStatePreviewParameterProvider.kt b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/preview/FilterUiStatePreviewParameterProvider.kt index 872814306771..0aa3be815831 100644 --- a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/preview/FilterUiStatePreviewParameterProvider.kt +++ b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/preview/FilterUiStatePreviewParameterProvider.kt @@ -3,19 +3,17 @@ package net.mullvad.mullvadvpn.compose.preview import androidx.compose.ui.tooling.preview.PreviewParameterProvider import net.mullvad.mullvadvpn.compose.state.RelayFilterUiState import net.mullvad.mullvadvpn.lib.model.Ownership -import net.mullvad.mullvadvpn.lib.model.Provider import net.mullvad.mullvadvpn.lib.model.ProviderId -private val PROVIDER = - Provider(providerId = ProviderId("provider1"), ownership = Ownership.MullvadOwned) +private val PROVIDER_TO_OWNERSHIPS = mapOf(ProviderId("provider1") to setOf(Ownership.MullvadOwned)) class FilterUiStatePreviewParameterProvider : PreviewParameterProvider { override val values = sequenceOf( RelayFilterUiState( + providerToOwnerships = PROVIDER_TO_OWNERSHIPS, selectedOwnership = Ownership.MullvadOwned, - allProviders = listOf(PROVIDER), - selectedProviders = listOf(PROVIDER), + selectedProviders = PROVIDER_TO_OWNERSHIPS.keys.toList(), ) ) } diff --git a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/preview/RelayItemPreviewData.kt b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/preview/RelayItemPreviewData.kt index 0af4199f40ed..20f69e3bcd90 100644 --- a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/preview/RelayItemPreviewData.kt +++ b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/preview/RelayItemPreviewData.kt @@ -2,7 +2,6 @@ package net.mullvad.mullvadvpn.compose.preview import net.mullvad.mullvadvpn.lib.model.GeoLocationId import net.mullvad.mullvadvpn.lib.model.Ownership -import net.mullvad.mullvadvpn.lib.model.Provider import net.mullvad.mullvadvpn.lib.model.ProviderId import net.mullvad.mullvadvpn.lib.model.RelayItem @@ -56,7 +55,8 @@ private fun generateRelayItemRelay( RelayItem.Location.Relay( id = GeoLocationId.Hostname(city = cityCode, code = hostName), active = active, - provider = Provider(ProviderId("Provider"), Ownership.MullvadOwned), + provider = ProviderId("Provider"), + ownership = Ownership.MullvadOwned, daita = daita, ) diff --git a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/preview/SelectLocationsUiStatePreviewParameterProvider.kt b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/preview/SelectLocationsUiStatePreviewParameterProvider.kt index 6893397f9ffc..965999c7f1e0 100644 --- a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/preview/SelectLocationsUiStatePreviewParameterProvider.kt +++ b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/preview/SelectLocationsUiStatePreviewParameterProvider.kt @@ -3,8 +3,6 @@ package net.mullvad.mullvadvpn.compose.preview import androidx.compose.ui.tooling.preview.PreviewParameterProvider import net.mullvad.mullvadvpn.compose.state.RelayListType import net.mullvad.mullvadvpn.compose.state.SelectLocationUiState -import net.mullvad.mullvadvpn.lib.model.Ownership -import net.mullvad.mullvadvpn.lib.model.Provider import net.mullvad.mullvadvpn.usecase.FilterChip import net.mullvad.mullvadvpn.usecase.ModelOwnership diff --git a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/screen/FilterScreen.kt b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/screen/FilterScreen.kt index 60788bffd593..3d3f937c7368 100644 --- a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/screen/FilterScreen.kt +++ b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/screen/FilterScreen.kt @@ -46,7 +46,7 @@ import net.mullvad.mullvadvpn.compose.state.RelayFilterUiState import net.mullvad.mullvadvpn.compose.transitions.SlideInFromRightTransition import net.mullvad.mullvadvpn.compose.util.CollectSideEffectWithLifecycle import net.mullvad.mullvadvpn.lib.model.Ownership -import net.mullvad.mullvadvpn.lib.model.Provider +import net.mullvad.mullvadvpn.lib.model.ProviderId import net.mullvad.mullvadvpn.lib.theme.AppTheme import net.mullvad.mullvadvpn.lib.theme.Dimens import net.mullvad.mullvadvpn.viewmodel.FilterScreenSideEffect @@ -98,7 +98,7 @@ fun FilterScreen( onApplyClick: () -> Unit, onSelectedOwnership: (ownership: Ownership?) -> Unit, onAllProviderCheckChange: (isChecked: Boolean) -> Unit, - onSelectedProvider: (checked: Boolean, provider: Provider) -> Unit, + onSelectedProvider: (checked: Boolean, provider: ProviderId) -> Unit, ) { var providerExpanded by rememberSaveable { mutableStateOf(false) } var ownershipExpanded by rememberSaveable { mutableStateOf(false) } @@ -126,7 +126,7 @@ fun FilterScreen( itemsWithDivider( key = { it.name }, contentType = { ContentType.ITEM }, - items = state.filteredOwnershipByProviders, + items = state.selectableOwnerships, ) { ownership -> Ownership(ownership, state, onSelectedOwnership) } @@ -139,9 +139,17 @@ fun FilterScreen( AllProviders(state, onAllProviderCheckChange) } itemsWithDivider( - key = { it.providerId.value }, + key = { it.value }, contentType = { ContentType.ITEM }, - items = state.filteredProvidersByOwnership, + items = state.removedProviders, + ) { provider -> + RemovedProvider(provider, state, onSelectedProvider) + } + + itemsWithDivider( + key = { it.value }, + contentType = { ContentType.ITEM }, + items = state.selectableProviders, ) { provider -> Provider(provider, state, onSelectedProvider) } @@ -216,14 +224,30 @@ private fun LazyItemScope.AllProviders( @Composable private fun LazyItemScope.Provider( - provider: Provider, + providerId: ProviderId, + state: RelayFilterUiState, + onSelectedProvider: (checked: Boolean, providerId: ProviderId) -> Unit, +) { + CheckboxCell( + title = providerId.value, + checked = providerId in state.selectedProviders, + onCheckedChange = { checked -> onSelectedProvider(checked, providerId) }, + modifier = Modifier.animateItem(), + ) +} + +@Composable +private fun LazyItemScope.RemovedProvider( + providerId: ProviderId, state: RelayFilterUiState, - onSelectedProvider: (checked: Boolean, provider: Provider) -> Unit, + onSelectedProvider: (checked: Boolean, providerId: ProviderId) -> Unit, ) { + val checked = providerId in state.selectedProviders CheckboxCell( - title = provider.providerId.value, - checked = provider in state.selectedProviders, - onCheckedChange = { checked -> onSelectedProvider(checked, provider) }, + title = stringResource(R.string.removed_provider, providerId.value), + checked = checked, + enabled = checked, + onCheckedChange = { checked -> onSelectedProvider(checked, providerId) }, modifier = Modifier.animateItem(), ) } diff --git a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/state/FilterConstrainExtensions.kt b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/state/FilterConstrainExtensions.kt index 01fe84f76c76..2ddf35fad52c 100644 --- a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/state/FilterConstrainExtensions.kt +++ b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/state/FilterConstrainExtensions.kt @@ -2,7 +2,7 @@ package net.mullvad.mullvadvpn.compose.state import net.mullvad.mullvadvpn.lib.model.Constraint import net.mullvad.mullvadvpn.lib.model.Ownership -import net.mullvad.mullvadvpn.lib.model.Provider +import net.mullvad.mullvadvpn.lib.model.ProviderId import net.mullvad.mullvadvpn.lib.model.Providers fun Ownership?.toOwnershipConstraint(): Constraint = @@ -11,18 +11,15 @@ fun Ownership?.toOwnershipConstraint(): Constraint = else -> Constraint.Only(this) } -fun Constraint.toSelectedProviders(allProviders: List): List = +fun Constraint.toSelectedProviders(allProviders: List): List = when (this) { Constraint.Any -> allProviders - is Constraint.Only -> - value.providers.toList().mapNotNull { provider -> - allProviders.firstOrNull { it.providerId == provider } - } + is Constraint.Only -> value.providers.toList() } -fun List.toConstraintProviders(allProviders: List): Constraint = +fun List.toConstraintProviders(allProviders: List): Constraint = if (size == allProviders.size) { Constraint.Any } else { - Constraint.Only(Providers(map { provider -> provider.providerId }.toHashSet())) + Constraint.Only(Providers(toHashSet())) } diff --git a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/state/RelayFilterUiState.kt b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/state/RelayFilterUiState.kt index ffc26b0bb71b..0c81ac0aa5d9 100644 --- a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/state/RelayFilterUiState.kt +++ b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/compose/state/RelayFilterUiState.kt @@ -1,26 +1,37 @@ package net.mullvad.mullvadvpn.compose.state import net.mullvad.mullvadvpn.lib.model.Ownership -import net.mullvad.mullvadvpn.lib.model.Provider +import net.mullvad.mullvadvpn.lib.model.ProviderId data class RelayFilterUiState( + private val providerToOwnerships: Map> = emptyMap(), val selectedOwnership: Ownership? = null, - val allProviders: List = emptyList(), - val selectedProviders: List = allProviders, + val selectedProviders: List = emptyList(), ) { - val isApplyButtonEnabled = selectedProviders.isNotEmpty() + val allProviders: List = providerToOwnerships.keys.toList().sorted() - val filteredOwnershipByProviders = + val selectableOwnerships: List = if (selectedProviders.isEmpty()) { - Ownership.entries - } else { - Ownership.entries.filter { ownership -> - selectedProviders.any { provider -> provider.ownership == ownership } + Ownership.entries + } else { + providerToOwnerships + .filterKeys { it in selectedProviders } + .values + .flatten() + .distinct() } + .sorted() + + val selectableProviders: List = + if (selectedOwnership != null) { + providerToOwnerships.filterValues { selectedOwnership in it }.keys.toList().sorted() + } else { + allProviders } - val filteredProvidersByOwnership = - if (selectedOwnership == null) allProviders - else allProviders.filter { provider -> provider.ownership == selectedOwnership } + + val removedProviders: List = selectedProviders - allProviders + + val isApplyButtonEnabled = selectedProviders.isNotEmpty() val isAllProvidersChecked = allProviders.size == selectedProviders.size } 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 650ee67eaa1b..4acf52c7b05b 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 @@ -37,7 +37,6 @@ import net.mullvad.mullvadvpn.ui.MainActivity import net.mullvad.mullvadvpn.ui.serviceconnection.AppVersionInfoRepository import net.mullvad.mullvadvpn.ui.serviceconnection.ServiceConnectionManager import net.mullvad.mullvadvpn.usecase.AccountExpiryInAppNotificationUseCase -import net.mullvad.mullvadvpn.usecase.AvailableProvidersUseCase import net.mullvad.mullvadvpn.usecase.EmptyPaymentUseCase import net.mullvad.mullvadvpn.usecase.FilterChipUseCase import net.mullvad.mullvadvpn.usecase.FilteredRelayListUseCase @@ -47,6 +46,7 @@ import net.mullvad.mullvadvpn.usecase.NewDeviceNotificationUseCase import net.mullvad.mullvadvpn.usecase.OutOfTimeUseCase import net.mullvad.mullvadvpn.usecase.PaymentUseCase import net.mullvad.mullvadvpn.usecase.PlayPaymentUseCase +import net.mullvad.mullvadvpn.usecase.ProviderToOwnershipsUseCase import net.mullvad.mullvadvpn.usecase.SelectedLocationTitleUseCase import net.mullvad.mullvadvpn.usecase.SelectedLocationUseCase import net.mullvad.mullvadvpn.usecase.SystemVpnSettingsAvailableUseCase @@ -156,7 +156,7 @@ val uiModule = module { single { SystemVpnSettingsAvailableUseCase(androidContext()) } single { CustomListActionUseCase(get(), get()) } single { SelectedLocationTitleUseCase(get(), get()) } - single { AvailableProvidersUseCase(get()) } + single { ProviderToOwnershipsUseCase(get()) } single { FilterCustomListsRelayItemUseCase(get(), get(), get(), get()) } single { CustomListsRelayItemUseCase(get(), get()) } single { CustomListRelayItemsUseCase(get(), get()) } 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 index 5584d8e99133..21d3294de408 100644 --- a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/relaylist/RelayItemExtensions.kt +++ b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/relaylist/RelayItemExtensions.kt @@ -35,7 +35,7 @@ private fun RelayItem.Location.hasOwnership(ownershipConstraint: Constraint cities.any { it.hasOwnership(ownershipConstraint) } is RelayItem.Location.City -> relays.any { it.hasOwnership(ownershipConstraint) } - is RelayItem.Location.Relay -> this.provider.ownership == ownershipConstraint.value + is RelayItem.Location.Relay -> ownershipConstraint.value == ownership } } else { true @@ -46,8 +46,7 @@ private fun RelayItem.Location.hasProvider(providersConstraint: Constraint cities.any { it.hasProvider(providersConstraint) } is RelayItem.Location.City -> relays.any { it.hasProvider(providersConstraint) } - is RelayItem.Location.Relay -> - providersConstraint.value.providers.contains(provider.providerId) + is RelayItem.Location.Relay -> provider in providersConstraint.value.providers } } else { true diff --git a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/usecase/FilterChipUseCase.kt b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/usecase/FilterChipUseCase.kt index 37e5f71eccc9..361e29944e11 100644 --- a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/usecase/FilterChipUseCase.kt +++ b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/usecase/FilterChipUseCase.kt @@ -3,10 +3,9 @@ package net.mullvad.mullvadvpn.usecase import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.combine import net.mullvad.mullvadvpn.compose.state.RelayListType -import net.mullvad.mullvadvpn.compose.state.toSelectedProviders import net.mullvad.mullvadvpn.lib.model.Constraint import net.mullvad.mullvadvpn.lib.model.Ownership -import net.mullvad.mullvadvpn.lib.model.Provider +import net.mullvad.mullvadvpn.lib.model.ProviderId import net.mullvad.mullvadvpn.lib.model.Providers import net.mullvad.mullvadvpn.lib.model.Settings import net.mullvad.mullvadvpn.repository.RelayListFilterRepository @@ -18,7 +17,7 @@ typealias ModelOwnership = Ownership class FilterChipUseCase( private val relayListFilterRepository: RelayListFilterRepository, - private val availableProvidersUseCase: AvailableProvidersUseCase, + private val providerToOwnershipsUseCase: ProviderToOwnershipsUseCase, private val settingsRepository: SettingsRepository, private val wireguardConstraintsRepository: WireguardConstraintsRepository, ) { @@ -26,19 +25,19 @@ class FilterChipUseCase( combine( relayListFilterRepository.selectedOwnership, relayListFilterRepository.selectedProviders, - availableProvidersUseCase(), + providerToOwnershipsUseCase(), settingsRepository.settingsUpdates, wireguardConstraintsRepository.wireguardConstraints, ) { selectedOwnership, selectedConstraintProviders, - allProviders, + providerOwnership, settings, wireguardConstraints -> filterChips( selectedOwnership = selectedOwnership, selectedConstraintProviders = selectedConstraintProviders, - allProviders = allProviders, + providerToOwnerships = providerOwnership, daitaDirectOnly = settings?.daitaAndDirectOnly() == true, isMultihopEnabled = wireguardConstraints?.isMultihopEnabled == true, relayListType = relayListType, @@ -48,7 +47,7 @@ class FilterChipUseCase( private fun filterChips( selectedOwnership: Constraint, selectedConstraintProviders: Constraint, - allProviders: List, + providerToOwnerships: Map>, daitaDirectOnly: Boolean, isMultihopEnabled: Boolean, relayListType: RelayListType, @@ -58,10 +57,22 @@ class FilterChipUseCase( when (selectedConstraintProviders) { is Constraint.Any -> null is Constraint.Only -> - filterSelectedProvidersByOwnership( - selectedConstraintProviders.toSelectedProviders(allProviders), - ownershipFilter, - ) + selectedConstraintProviders.value.providers + .filter { providerId -> + if (ownershipFilter == null) { + true + } else { + val providerOwnerships = providerToOwnerships[providerId] + // If the provider has been removed from the relay list we add it + // so it is visible for the user, because we won't know what + // ownerships it had. + if (providerOwnerships == null) { + true + } else { + providerOwnerships.contains(ownershipFilter) + } + } + } .size } return buildList { @@ -83,13 +94,6 @@ class FilterChipUseCase( } } - private fun filterSelectedProvidersByOwnership( - selectedProviders: List, - selectedOwnership: Ownership?, - ): List = - if (selectedOwnership == null) selectedProviders - else selectedProviders.filter { it.ownership == selectedOwnership } - private fun Settings.daitaAndDirectOnly() = tunnelOptions.wireguard.daitaSettings.enabled && tunnelOptions.wireguard.daitaSettings.directOnly diff --git a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/usecase/AvailableProvidersUseCase.kt b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/usecase/ProviderToOwnershipsUseCase.kt similarity index 52% rename from android/app/src/main/kotlin/net/mullvad/mullvadvpn/usecase/AvailableProvidersUseCase.kt rename to android/app/src/main/kotlin/net/mullvad/mullvadvpn/usecase/ProviderToOwnershipsUseCase.kt index 14aa3824cfce..42f5a94e2959 100644 --- a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/usecase/AvailableProvidersUseCase.kt +++ b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/usecase/ProviderToOwnershipsUseCase.kt @@ -2,18 +2,18 @@ package net.mullvad.mullvadvpn.usecase import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.map -import net.mullvad.mullvadvpn.lib.model.Provider +import net.mullvad.mullvadvpn.lib.model.Ownership +import net.mullvad.mullvadvpn.lib.model.ProviderId import net.mullvad.mullvadvpn.lib.model.RelayItem import net.mullvad.mullvadvpn.repository.RelayListRepository -class AvailableProvidersUseCase(private val relayListRepository: RelayListRepository) { - - operator fun invoke(): Flow> = +class ProviderToOwnershipsUseCase(private val relayListRepository: RelayListRepository) { + operator fun invoke(): Flow>> = relayListRepository.relayList.map { relayList -> relayList .flatMap(RelayItem.Location.Country::cities) .flatMap(RelayItem.Location.City::relays) - .map(RelayItem.Location.Relay::provider) - .distinct() + .groupBy({ it.provider }, { it.ownership }) + .mapValues { (_, ownerships) -> ownerships.toSet() } } } diff --git a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/viewmodel/FilterViewModel.kt b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/viewmodel/FilterViewModel.kt index 548d6b70a723..c05515126e31 100644 --- a/android/app/src/main/kotlin/net/mullvad/mullvadvpn/viewmodel/FilterViewModel.kt +++ b/android/app/src/main/kotlin/net/mullvad/mullvadvpn/viewmodel/FilterViewModel.kt @@ -16,27 +16,30 @@ import net.mullvad.mullvadvpn.compose.state.toConstraintProviders import net.mullvad.mullvadvpn.compose.state.toOwnershipConstraint import net.mullvad.mullvadvpn.compose.state.toSelectedProviders import net.mullvad.mullvadvpn.lib.model.Ownership -import net.mullvad.mullvadvpn.lib.model.Provider +import net.mullvad.mullvadvpn.lib.model.ProviderId import net.mullvad.mullvadvpn.repository.RelayListFilterRepository -import net.mullvad.mullvadvpn.usecase.AvailableProvidersUseCase +import net.mullvad.mullvadvpn.usecase.ProviderToOwnershipsUseCase class FilterViewModel( - private val availableProvidersUseCase: AvailableProvidersUseCase, + private val providerToOwnershipsUseCase: ProviderToOwnershipsUseCase, private val relayListFilterRepository: RelayListFilterRepository, ) : ViewModel() { private val _uiSideEffect = Channel() val uiSideEffect = _uiSideEffect.receiveAsFlow() private val selectedOwnership = MutableStateFlow(null) - private val selectedProviders = MutableStateFlow>(emptyList()) + private val selectedProviders = MutableStateFlow>(emptyList()) init { viewModelScope.launch { selectedProviders.value = - combine(availableProvidersUseCase(), relayListFilterRepository.selectedProviders) { - allProviders, - selectedConstraintProviders -> - selectedConstraintProviders.toSelectedProviders(allProviders) + combine( + providerToOwnershipsUseCase(), + relayListFilterRepository.selectedProviders, + ) { providerToOwnerships, selectedConstraintProviders -> + selectedConstraintProviders.toSelectedProviders( + providerToOwnerships.keys.toList() + ) } .first() @@ -46,31 +49,25 @@ class FilterViewModel( } val uiState: StateFlow = - combine(selectedOwnership, availableProvidersUseCase(), selectedProviders) { - selectedOwnership, - allProviders, - selectedProviders -> - RelayFilterUiState( - selectedOwnership = selectedOwnership, - allProviders = allProviders, - selectedProviders = selectedProviders, - ) - } - .stateIn( - viewModelScope, - SharingStarted.WhileSubscribed(), - RelayFilterUiState( - allProviders = emptyList(), - selectedOwnership = null, - selectedProviders = emptyList(), - ), - ) + combine(providerToOwnershipsUseCase(), selectedOwnership, selectedProviders, ::createState) + .stateIn(viewModelScope, SharingStarted.WhileSubscribed(), RelayFilterUiState()) + + private fun createState( + providerToOwnerships: Map>, + selectedOwnership: Ownership?, + selectedProviders: List, + ): RelayFilterUiState = + RelayFilterUiState( + providerToOwnerships = providerToOwnerships, + selectedOwnership = selectedOwnership, + selectedProviders = selectedProviders, + ) fun setSelectedOwnership(ownership: Ownership?) { selectedOwnership.value = ownership } - fun setSelectedProvider(checked: Boolean, provider: Provider) { + fun setSelectedProvider(checked: Boolean, provider: ProviderId) { selectedProviders.value = if (checked) { selectedProviders.value + provider @@ -83,7 +80,7 @@ class FilterViewModel( viewModelScope.launch { selectedProviders.value = if (isChecked) { - availableProvidersUseCase().first() + providerToOwnershipsUseCase().first().keys.toList() } else { emptyList() } diff --git a/android/app/src/test/kotlin/net/mullvad/mullvadvpn/usecase/FilterChipUseCaseTest.kt b/android/app/src/test/kotlin/net/mullvad/mullvadvpn/usecase/FilterChipUseCaseTest.kt index 221d89cf4006..bd1a759b77e1 100644 --- a/android/app/src/test/kotlin/net/mullvad/mullvadvpn/usecase/FilterChipUseCaseTest.kt +++ b/android/app/src/test/kotlin/net/mullvad/mullvadvpn/usecase/FilterChipUseCaseTest.kt @@ -9,7 +9,6 @@ import net.mullvad.mullvadvpn.compose.state.RelayListType import net.mullvad.mullvadvpn.lib.common.test.assertLists import net.mullvad.mullvadvpn.lib.model.Constraint import net.mullvad.mullvadvpn.lib.model.Ownership -import net.mullvad.mullvadvpn.lib.model.Provider import net.mullvad.mullvadvpn.lib.model.ProviderId import net.mullvad.mullvadvpn.lib.model.Providers import net.mullvad.mullvadvpn.lib.model.Settings @@ -23,13 +22,13 @@ import org.junit.jupiter.api.Test class FilterChipUseCaseTest { private val mockRelayListFilterRepository: RelayListFilterRepository = mockk() - private val mockAvailableProvidersUseCase: AvailableProvidersUseCase = mockk() + private val mockProviderToOwnershipsUseCase: ProviderToOwnershipsUseCase = mockk() private val mockSettingRepository: SettingsRepository = mockk() private val mockWireguardConstraintsRepository: WireguardConstraintsRepository = mockk() private val selectedOwnership = MutableStateFlow>(Constraint.Any) private val selectedProviders = MutableStateFlow>(Constraint.Any) - private val availableProviders = MutableStateFlow>(emptyList()) + private val providerToOwnerships = MutableStateFlow>>(emptyMap()) private val settings = MutableStateFlow(mockk(relaxed = true)) private val wireguardConstraints = MutableStateFlow(mockk(relaxed = true)) @@ -39,7 +38,7 @@ class FilterChipUseCaseTest { fun setUp() { every { mockRelayListFilterRepository.selectedOwnership } returns selectedOwnership every { mockRelayListFilterRepository.selectedProviders } returns selectedProviders - every { mockAvailableProvidersUseCase() } returns availableProviders + every { mockProviderToOwnershipsUseCase() } returns providerToOwnerships every { mockSettingRepository.settingsUpdates } returns settings every { mockWireguardConstraintsRepository.wireguardConstraints } returns wireguardConstraints @@ -47,7 +46,7 @@ class FilterChipUseCaseTest { filterChipUseCase = FilterChipUseCase( relayListFilterRepository = mockRelayListFilterRepository, - availableProvidersUseCase = mockAvailableProvidersUseCase, + providerToOwnershipsUseCase = mockProviderToOwnershipsUseCase, settingsRepository = mockSettingRepository, wireguardConstraintsRepository = mockWireguardConstraintsRepository, ) @@ -74,10 +73,10 @@ class FilterChipUseCaseTest { // Arrange val expectedProviders = Providers(providers = setOf(ProviderId("1"), ProviderId("2"))) selectedProviders.value = Constraint.Only(expectedProviders) - availableProviders.value = - listOf( - Provider(ProviderId("1"), Ownership.MullvadOwned), - Provider(ProviderId("2"), Ownership.Rented), + providerToOwnerships.value = + mapOf( + ProviderId("1") to setOf(Ownership.MullvadOwned), + ProviderId("2") to setOf(Ownership.Rented), ) filterChipUseCase(RelayListType.EXIT).test { @@ -93,10 +92,10 @@ class FilterChipUseCaseTest { val expectedOwnership = Ownership.MullvadOwned selectedProviders.value = Constraint.Only(expectedProviders) selectedOwnership.value = Constraint.Only(expectedOwnership) - availableProviders.value = - listOf( - Provider(ProviderId("1"), Ownership.MullvadOwned), - Provider(ProviderId("2"), Ownership.Rented), + providerToOwnerships.value = + mapOf( + ProviderId("1") to setOf(Ownership.MullvadOwned), + ProviderId("2") to setOf(Ownership.Rented), ) filterChipUseCase(RelayListType.EXIT).test { @@ -185,4 +184,27 @@ class FilterChipUseCaseTest { filterChipUseCase(RelayListType.EXIT).test { assertLists(emptyList(), awaitItem()) } } + + @Test + fun `ensure that a selected provider that is not in the provider list is still counted`() = + runTest { + // Arrange + val expectedProviders = Providers(providers = setOf(ProviderId("1"))) + val expectedOwnership = Ownership.MullvadOwned + selectedProviders.value = Constraint.Only(expectedProviders) + selectedOwnership.value = Constraint.Only(expectedOwnership) + providerToOwnerships.value = + mapOf( + ProviderId("2") to setOf(Ownership.MullvadOwned), + ProviderId("3") to setOf(Ownership.Rented), + ) + + // Act, Assert + filterChipUseCase(RelayListType.EXIT).test { + assertLists( + listOf(FilterChip.Ownership(expectedOwnership), FilterChip.Provider(1)), + awaitItem(), + ) + } + } } diff --git a/android/app/src/test/kotlin/net/mullvad/mullvadvpn/viewmodel/CustomListLocationsViewModelTest.kt b/android/app/src/test/kotlin/net/mullvad/mullvadvpn/viewmodel/CustomListLocationsViewModelTest.kt index 75b8deca8fab..865b4ce4712f 100644 --- a/android/app/src/test/kotlin/net/mullvad/mullvadvpn/viewmodel/CustomListLocationsViewModelTest.kt +++ b/android/app/src/test/kotlin/net/mullvad/mullvadvpn/viewmodel/CustomListLocationsViewModelTest.kt @@ -22,7 +22,6 @@ import net.mullvad.mullvadvpn.lib.model.CustomListId import net.mullvad.mullvadvpn.lib.model.CustomListName import net.mullvad.mullvadvpn.lib.model.GeoLocationId import net.mullvad.mullvadvpn.lib.model.Ownership -import net.mullvad.mullvadvpn.lib.model.Provider import net.mullvad.mullvadvpn.lib.model.ProviderId import net.mullvad.mullvadvpn.lib.model.RelayItem import net.mullvad.mullvadvpn.relaylist.descendants @@ -357,11 +356,8 @@ class CustomListLocationsViewModelTest { "gbg-1", ), active = true, - provider = - Provider( - ProviderId("Provider"), - ownership = Ownership.MullvadOwned, - ), + provider = ProviderId("Provider"), + ownership = Ownership.MullvadOwned, daita = false, ) ), @@ -377,7 +373,8 @@ class CustomListLocationsViewModelTest { "cph-1", ), active = true, - provider = Provider(ProviderId("Provider"), ownership = Ownership.MullvadOwned), + provider = ProviderId("Provider"), + ownership = Ownership.MullvadOwned, daita = false, ) } diff --git a/android/app/src/test/kotlin/net/mullvad/mullvadvpn/viewmodel/FilterViewModelTest.kt b/android/app/src/test/kotlin/net/mullvad/mullvadvpn/viewmodel/FilterViewModelTest.kt index 4453f08ee424..49b9b4baffa8 100644 --- a/android/app/src/test/kotlin/net/mullvad/mullvadvpn/viewmodel/FilterViewModelTest.kt +++ b/android/app/src/test/kotlin/net/mullvad/mullvadvpn/viewmodel/FilterViewModelTest.kt @@ -19,11 +19,10 @@ import net.mullvad.mullvadvpn.lib.common.test.TestCoroutineRule import net.mullvad.mullvadvpn.lib.common.test.assertLists import net.mullvad.mullvadvpn.lib.model.Constraint import net.mullvad.mullvadvpn.lib.model.Ownership -import net.mullvad.mullvadvpn.lib.model.Provider import net.mullvad.mullvadvpn.lib.model.ProviderId import net.mullvad.mullvadvpn.lib.model.Providers import net.mullvad.mullvadvpn.repository.RelayListFilterRepository -import net.mullvad.mullvadvpn.usecase.AvailableProvidersUseCase +import net.mullvad.mullvadvpn.usecase.ProviderToOwnershipsUseCase import org.junit.jupiter.api.AfterEach import org.junit.jupiter.api.BeforeEach import org.junit.jupiter.api.Test @@ -31,49 +30,43 @@ import org.junit.jupiter.api.extension.ExtendWith @ExtendWith(TestCoroutineRule::class) class FilterViewModelTest { - private val mockAvailableProvidersUseCase: AvailableProvidersUseCase = mockk(relaxed = true) + private val mockProvidersOwnershipUseCase: ProviderToOwnershipsUseCase = mockk(relaxed = true) private val mockRelayListFilterRepository: RelayListFilterRepository = mockk() private lateinit var viewModel: FilterViewModel private val selectedOwnership = MutableStateFlow>(Constraint.Only(Ownership.MullvadOwned)) private val dummyListOfAllProviders = - listOf( - Provider(ProviderId("31173"), Ownership.MullvadOwned), - Provider(ProviderId("100TB"), Ownership.Rented), - Provider(ProviderId("Blix"), Ownership.MullvadOwned), - Provider(ProviderId("Creanova"), Ownership.MullvadOwned), - Provider(ProviderId("DataPacket"), Ownership.Rented), - Provider(ProviderId("HostRoyale"), Ownership.Rented), - Provider(ProviderId("hostuniversal"), Ownership.Rented), - Provider(ProviderId("iRegister"), Ownership.Rented), - Provider(ProviderId("M247"), Ownership.Rented), - Provider(ProviderId("Makonix"), Ownership.Rented), - Provider(ProviderId("PrivateLayer"), Ownership.Rented), - Provider(ProviderId("ptisp"), Ownership.Rented), - Provider(ProviderId("Qnax"), Ownership.Rented), - Provider(ProviderId("Quadranet"), Ownership.Rented), - Provider(ProviderId("techfutures"), Ownership.Rented), - Provider(ProviderId("Tzulo"), Ownership.Rented), - Provider(ProviderId("xtom"), Ownership.Rented), - ) - private val mockSelectedProviders: List = - listOf( - Provider(ProviderId("31173"), Ownership.MullvadOwned), - Provider(ProviderId("Blix"), Ownership.MullvadOwned), - Provider(ProviderId("Creanova"), Ownership.MullvadOwned), + mapOf( + ProviderId("31173") to setOf(Ownership.MullvadOwned), + ProviderId("100TB") to setOf(Ownership.Rented), + ProviderId("Blix") to setOf(Ownership.MullvadOwned), + ProviderId("Creanova") to setOf(Ownership.MullvadOwned), + ProviderId("DataPacket") to setOf(Ownership.Rented, Ownership.MullvadOwned), + ProviderId("HostRoyale") to setOf(Ownership.Rented), + ProviderId("hostuniversal") to setOf(Ownership.Rented), + ProviderId("iRegister") to setOf(Ownership.Rented), + ProviderId("M247") to setOf(Ownership.Rented), + ProviderId("Makonix") to setOf(Ownership.Rented), + ProviderId("PrivateLayer") to setOf(Ownership.Rented), + ProviderId("ptisp") to setOf(Ownership.Rented), + ProviderId("Qnax") to setOf(Ownership.Rented), + ProviderId("Quadranet") to setOf(Ownership.Rented), + ProviderId("techfutures") to setOf(Ownership.Rented), + ProviderId("Tzulo") to setOf(Ownership.Rented), + ProviderId("xtom") to setOf(Ownership.Rented), ) + private val mockSelectedProviders: List = + listOf(ProviderId("31173"), ProviderId("Blix"), ProviderId("Creanova")) @BeforeEach fun setup() { every { mockRelayListFilterRepository.selectedOwnership } returns selectedOwnership - every { mockAvailableProvidersUseCase() } returns flowOf(dummyListOfAllProviders) + every { mockProvidersOwnershipUseCase() } returns flowOf(dummyListOfAllProviders) every { mockRelayListFilterRepository.selectedProviders } returns - MutableStateFlow( - Constraint.Only(Providers(mockSelectedProviders.map { it.providerId }.toSet())) - ) + MutableStateFlow(Constraint.Only(Providers(mockSelectedProviders.toHashSet()))) viewModel = FilterViewModel( - availableProvidersUseCase = mockAvailableProvidersUseCase, + providerToOwnershipsUseCase = mockProvidersOwnershipUseCase, relayListFilterRepository = mockRelayListFilterRepository, ) } @@ -101,7 +94,7 @@ class FilterViewModelTest { fun `setSelectionProvider should emit uiState where selectedProviders include the selected provider`() = runTest { // Arrange - val mockSelectedProvidersList = Provider(ProviderId("ptisp"), Ownership.Rented) + val mockSelectedProvidersList = ProviderId("ptisp") // Assert viewModel.uiState.test { assertLists(awaitItem().selectedProviders, mockSelectedProviders) @@ -117,7 +110,7 @@ class FilterViewModelTest { fun `setAllProvider with true should emit uiState with selectedProviders includes all providers`() = runTest { // Arrange - val mockProvidersList = dummyListOfAllProviders + val mockProvidersList = dummyListOfAllProviders.keys.toList() // Act viewModel.setAllProviders(true) // Assert @@ -133,7 +126,7 @@ class FilterViewModelTest { // Arrange val mockOwnership = Ownership.MullvadOwned.toOwnershipConstraint() val mockSelectedProviders = - mockSelectedProviders.toConstraintProviders(dummyListOfAllProviders) + mockSelectedProviders.toConstraintProviders(dummyListOfAllProviders.keys.toList()) coEvery { mockRelayListFilterRepository.updateSelectedOwnershipAndProviderFilter( mockOwnership, @@ -152,4 +145,26 @@ class FilterViewModelTest { ) } } + + @Test + fun `ensure that providers with multiple ownership are only returned once`() = runTest { + // Arrange + val expectedProviderList = dummyListOfAllProviders.keys.toList() + + // Assert + viewModel.uiState.test { + val state = awaitItem() + assertLists(expectedProviderList, state.allProviders) + } + } + + @Test + fun `ensure that providers are sorted by name`() = runTest { + // Assert + viewModel.uiState.test { + val state = awaitItem() + assertEquals(state.allProviders.sorted(), state.allProviders) + assertEquals(state.selectableProviders.sorted(), state.selectableProviders) + } + } } diff --git a/android/lib/daemon-grpc/src/main/kotlin/net/mullvad/mullvadvpn/lib/daemon/grpc/mapper/ToDomain.kt b/android/lib/daemon-grpc/src/main/kotlin/net/mullvad/mullvadvpn/lib/daemon/grpc/mapper/ToDomain.kt index c7f47b0c291c..91399b48b614 100644 --- a/android/lib/daemon-grpc/src/main/kotlin/net/mullvad/mullvadvpn/lib/daemon/grpc/mapper/ToDomain.kt +++ b/android/lib/daemon-grpc/src/main/kotlin/net/mullvad/mullvadvpn/lib/daemon/grpc/mapper/ToDomain.kt @@ -52,7 +52,6 @@ import net.mullvad.mullvadvpn.lib.model.ParameterGenerationError import net.mullvad.mullvadvpn.lib.model.PlayPurchasePaymentToken import net.mullvad.mullvadvpn.lib.model.Port import net.mullvad.mullvadvpn.lib.model.PortRange -import net.mullvad.mullvadvpn.lib.model.Provider import net.mullvad.mullvadvpn.lib.model.ProviderId import net.mullvad.mullvadvpn.lib.model.Providers import net.mullvad.mullvadvpn.lib.model.QuantumResistantState @@ -565,11 +564,8 @@ internal fun ManagementInterface.Relay.toDomain( RelayItem.Location.Relay( id = GeoLocationId.Hostname(cityCode, hostname), active = active, - provider = - Provider( - ProviderId(provider), - ownership = if (owned) Ownership.MullvadOwned else Ownership.Rented, - ), + provider = ProviderId(provider), + ownership = if (owned) Ownership.MullvadOwned else Ownership.Rented, daita = if ( hasEndpointData() && endpointType == ManagementInterface.Relay.RelayType.WIREGUARD diff --git a/android/lib/daemon-grpc/src/test/kotlin/net/mullvad/mullvadvpn/lib/daemon/grpc/RelayNameComparatorTest.kt b/android/lib/daemon-grpc/src/test/kotlin/net/mullvad/mullvadvpn/lib/daemon/grpc/RelayNameComparatorTest.kt index e496c6d5aa74..df710649c53f 100644 --- a/android/lib/daemon-grpc/src/test/kotlin/net/mullvad/mullvadvpn/lib/daemon/grpc/RelayNameComparatorTest.kt +++ b/android/lib/daemon-grpc/src/test/kotlin/net/mullvad/mullvadvpn/lib/daemon/grpc/RelayNameComparatorTest.kt @@ -4,7 +4,6 @@ import io.mockk.mockk import io.mockk.unmockkAll import net.mullvad.mullvadvpn.lib.model.GeoLocationId import net.mullvad.mullvadvpn.lib.model.Ownership -import net.mullvad.mullvadvpn.lib.model.Provider import net.mullvad.mullvadvpn.lib.model.ProviderId import net.mullvad.mullvadvpn.lib.model.RelayItem import org.junit.jupiter.api.AfterEach @@ -24,22 +23,16 @@ class RelayNameComparatorTest { RelayItem.Location.Relay( id = GeoLocationId.Hostname(city = mockk(), "se9-wireguard"), active = false, - provider = - Provider( - providerId = ProviderId("Provider"), - ownership = Ownership.MullvadOwned, - ), + provider = ProviderId("Provider"), + ownership = Ownership.MullvadOwned, daita = false, ) val relay10 = RelayItem.Location.Relay( id = GeoLocationId.Hostname(city = mockk(), "se10-wireguard"), active = false, - provider = - Provider( - providerId = ProviderId("Provider"), - ownership = Ownership.MullvadOwned, - ), + provider = ProviderId("Provider"), + ownership = Ownership.MullvadOwned, daita = false, ) @@ -52,22 +45,16 @@ class RelayNameComparatorTest { RelayItem.Location.Relay( id = GeoLocationId.Hostname(city = mockk(), "se9-wireguard"), active = false, - provider = - Provider( - providerId = ProviderId("Provider"), - ownership = Ownership.MullvadOwned, - ), + provider = ProviderId("Provider"), + ownership = Ownership.MullvadOwned, daita = false, ) val relay9b = RelayItem.Location.Relay( id = GeoLocationId.Hostname(city = mockk(), "se9-wireguard"), active = false, - provider = - Provider( - providerId = ProviderId("Provider"), - ownership = Ownership.MullvadOwned, - ), + provider = ProviderId("Provider"), + ownership = Ownership.MullvadOwned, daita = false, ) @@ -81,44 +68,32 @@ class RelayNameComparatorTest { RelayItem.Location.Relay( id = GeoLocationId.Hostname(city = mockk(), "001"), active = false, - provider = - Provider( - providerId = ProviderId("Provider"), - ownership = Ownership.MullvadOwned, - ), + provider = ProviderId("Provider"), + ownership = Ownership.MullvadOwned, daita = false, ) val relay1 = RelayItem.Location.Relay( id = GeoLocationId.Hostname(city = mockk(), "1"), active = false, - provider = - Provider( - providerId = ProviderId("Provider"), - ownership = Ownership.MullvadOwned, - ), + provider = ProviderId("Provider"), + ownership = Ownership.MullvadOwned, daita = false, ) val relay3 = RelayItem.Location.Relay( id = GeoLocationId.Hostname(city = mockk(), "3"), active = false, - provider = - Provider( - providerId = ProviderId("Provider"), - ownership = Ownership.MullvadOwned, - ), + provider = ProviderId("Provider"), + ownership = Ownership.MullvadOwned, daita = false, ) val relay100 = RelayItem.Location.Relay( id = GeoLocationId.Hostname(city = mockk(), "100"), active = false, - provider = - Provider( - providerId = ProviderId("Provider"), - ownership = Ownership.MullvadOwned, - ), + provider = ProviderId("Provider"), + ownership = Ownership.MullvadOwned, daita = false, ) @@ -134,22 +109,16 @@ class RelayNameComparatorTest { RelayItem.Location.Relay( id = GeoLocationId.Hostname(city = mockk(), "se-wireguard"), active = false, - provider = - Provider( - providerId = ProviderId("Provider"), - ownership = Ownership.MullvadOwned, - ), + provider = ProviderId("Provider"), + ownership = Ownership.MullvadOwned, daita = false, ) val relay9b = RelayItem.Location.Relay( id = GeoLocationId.Hostname(city = mockk(), "se-wireguard"), active = false, - provider = - Provider( - providerId = ProviderId("Provider"), - ownership = Ownership.MullvadOwned, - ), + provider = ProviderId("Provider"), + ownership = Ownership.MullvadOwned, daita = false, ) @@ -163,22 +132,16 @@ class RelayNameComparatorTest { RelayItem.Location.Relay( id = GeoLocationId.Hostname(city = mockk(), "se001-wireguard"), active = false, - provider = - Provider( - providerId = ProviderId("Provider"), - ownership = Ownership.MullvadOwned, - ), + provider = ProviderId("Provider"), + ownership = Ownership.MullvadOwned, daita = false, ) val relay005 = RelayItem.Location.Relay( id = GeoLocationId.Hostname(city = mockk(), "se005-wireguard"), active = false, - provider = - Provider( - providerId = ProviderId("Provider"), - ownership = Ownership.MullvadOwned, - ), + provider = ProviderId("Provider"), + ownership = Ownership.MullvadOwned, daita = false, ) @@ -191,44 +154,32 @@ class RelayNameComparatorTest { RelayItem.Location.Relay( id = GeoLocationId.Hostname(city = mockk(), "ar2-wireguard"), active = false, - provider = - Provider( - providerId = ProviderId("Provider"), - ownership = Ownership.MullvadOwned, - ), + provider = ProviderId("Provider"), + ownership = Ownership.MullvadOwned, daita = false, ) val relayAr8 = RelayItem.Location.Relay( id = GeoLocationId.Hostname(city = mockk(), "ar8-wireguard"), active = false, - provider = - Provider( - providerId = ProviderId("Provider"), - ownership = Ownership.MullvadOwned, - ), + provider = ProviderId("Provider"), + ownership = Ownership.MullvadOwned, daita = false, ) val relaySe5 = RelayItem.Location.Relay( id = GeoLocationId.Hostname(city = mockk(), "se5-wireguard"), active = false, - provider = - Provider( - providerId = ProviderId("Provider"), - ownership = Ownership.MullvadOwned, - ), + provider = ProviderId("Provider"), + ownership = Ownership.MullvadOwned, daita = false, ) val relaySe10 = RelayItem.Location.Relay( id = GeoLocationId.Hostname(city = mockk(), "se10-wireguard"), active = false, - provider = - Provider( - providerId = ProviderId("Provider"), - ownership = Ownership.MullvadOwned, - ), + provider = ProviderId("Provider"), + ownership = Ownership.MullvadOwned, daita = false, ) @@ -243,22 +194,16 @@ class RelayNameComparatorTest { RelayItem.Location.Relay( id = GeoLocationId.Hostname(city = mockk(), "se2-cloud"), active = false, - provider = - Provider( - providerId = ProviderId("Provider"), - ownership = Ownership.MullvadOwned, - ), + provider = ProviderId("Provider"), + ownership = Ownership.MullvadOwned, daita = false, ) val relay2w = RelayItem.Location.Relay( id = GeoLocationId.Hostname(city = mockk(), "se2-wireguard"), active = false, - provider = - Provider( - providerId = ProviderId("Provider"), - ownership = Ownership.MullvadOwned, - ), + provider = ProviderId("Provider"), + ownership = Ownership.MullvadOwned, daita = false, ) @@ -271,22 +216,16 @@ class RelayNameComparatorTest { RelayItem.Location.Relay( id = GeoLocationId.Hostname(city = mockk(), "se22"), active = false, - provider = - Provider( - providerId = ProviderId("Provider"), - ownership = Ownership.MullvadOwned, - ), + provider = ProviderId("Provider"), + ownership = Ownership.MullvadOwned, daita = false, ) val relay22b = RelayItem.Location.Relay( id = GeoLocationId.Hostname(city = mockk(), "se22-wireguard"), active = false, - provider = - Provider( - providerId = ProviderId("Provider"), - ownership = Ownership.MullvadOwned, - ), + provider = ProviderId("Provider"), + ownership = Ownership.MullvadOwned, daita = false, ) diff --git a/android/lib/model/src/main/kotlin/net/mullvad/mullvadvpn/lib/model/Provider.kt b/android/lib/model/src/main/kotlin/net/mullvad/mullvadvpn/lib/model/Provider.kt deleted file mode 100644 index e704e9554d79..000000000000 --- a/android/lib/model/src/main/kotlin/net/mullvad/mullvadvpn/lib/model/Provider.kt +++ /dev/null @@ -1,3 +0,0 @@ -package net.mullvad.mullvadvpn.lib.model - -data class Provider(val providerId: ProviderId, val ownership: Ownership) diff --git a/android/lib/model/src/main/kotlin/net/mullvad/mullvadvpn/lib/model/ProviderId.kt b/android/lib/model/src/main/kotlin/net/mullvad/mullvadvpn/lib/model/ProviderId.kt index cc23c3e9b65f..1682e2018d83 100644 --- a/android/lib/model/src/main/kotlin/net/mullvad/mullvadvpn/lib/model/ProviderId.kt +++ b/android/lib/model/src/main/kotlin/net/mullvad/mullvadvpn/lib/model/ProviderId.kt @@ -1,3 +1,7 @@ package net.mullvad.mullvadvpn.lib.model -@JvmInline value class ProviderId(val value: String) +@JvmInline +value class ProviderId(val value: String) : Comparable { + override fun compareTo(other: ProviderId): Int = + value.uppercase().compareTo(other.value.uppercase()) +} diff --git a/android/lib/model/src/main/kotlin/net/mullvad/mullvadvpn/lib/model/RelayItem.kt b/android/lib/model/src/main/kotlin/net/mullvad/mullvadvpn/lib/model/RelayItem.kt index af96c4d94d76..3ff078877625 100644 --- a/android/lib/model/src/main/kotlin/net/mullvad/mullvadvpn/lib/model/RelayItem.kt +++ b/android/lib/model/src/main/kotlin/net/mullvad/mullvadvpn/lib/model/RelayItem.kt @@ -56,7 +56,8 @@ sealed interface RelayItem { @optics data class Relay( override val id: GeoLocationId.Hostname, - val provider: Provider, + val provider: ProviderId, + val ownership: Ownership, override val active: Boolean, val daita: Boolean, ) : Location { diff --git a/android/lib/resource/src/main/res/values/strings.xml b/android/lib/resource/src/main/res/values/strings.xml index bc3512501c98..c19faafd54d9 100644 --- a/android/lib/resource/src/main/res/values/strings.xml +++ b/android/lib/resource/src/main/res/values/strings.xml @@ -402,6 +402,7 @@ %s (Entry) %s (Exit) Filters: + %s (removed) Type at least 2 characters to start searching. %1$s (%2$s) hides patterns in your encrypted VPN traffic. By using sophisticated AI it’s possible to analyze the traffic of data packets going in and out of your device (even if the traffic is encrypted). diff --git a/desktop/packages/mullvad-vpn/locales/messages.pot b/desktop/packages/mullvad-vpn/locales/messages.pot index 1b5f981cc5b2..4c3cffa6e524 100644 --- a/desktop/packages/mullvad-vpn/locales/messages.pot +++ b/desktop/packages/mullvad-vpn/locales/messages.pot @@ -2266,6 +2266,9 @@ msgstr "" msgid "%s (added)" msgstr "" +msgid "%s (removed)" +msgstr "" + msgid "%s custom port" msgstr ""