Update both RelayListeners and add use cases

[Pururun]

• Update app RelayListener to use events and state flows instead of the old system.
• Update service RelayListener to use parsed messages instead.
• Add use cases for relay list and port ranges.
• Add filter to relay list based on ownership and selected providers.
• Add the ability to set ownership and providers and send it to the daemon.
• Get the selected ownership and providers from the daemon.
• Get a list of all providers from the relay list and if they are Mullvad owned or not.

---

This change is 

[linear]

DROID-466 Filter select location relay list on ownership and selected providers

[Rawa]

Reviewable status: 0 of 17 files reviewed, 1 unresolved discussion (waiting on @Pururun)

---

android/app/src/main/kotlin/net/mullvad/mullvadvpn/relaylist/RelayListExtensions.kt line 197 at r1 (raw file):

        }
        valid
    }

I feel that this filter is not very intuitive, we sort of mash it into one big filter. Alternative would be:

private fun List<net.mullvad.mullvadvpn.model.Relay>.filterValidRelays2(
    ownership: Constraint<Ownership>,
    providers: Constraint<Providers>
): List<net.mullvad.mullvadvpn.model.Relay> =
    filter { it.isWireguardRelay }
        .filter {
            when(ownership) {
                is Constraint.Any -> true
                is Constraint.Only -> when(ownership.value) {
                    Ownership.MullvadOwned -> it.owned
                    Ownership.Rented -> !it.owned
                }
            }
        }
        .filter { relay ->
            when(providers) {
                is Constraint.Any -> true
                is Constraint.Only -> providers.value.providers.contains(relay.provider)
            }
        }

A bit more exotic solution would be the following, but I'm not sure I think it is more readable and it is almost a bit of a overkill since we only have two Constraints right now:

private fun List<net.mullvad.mullvadvpn.model.Relay>.filterValidRelays3(
    ownership: Constraint<Ownership>,
    providers: Constraint<Providers>
): List<net.mullvad.mullvadvpn.model.Relay> =
    filter { it.isWireguardRelay }
        .filterConstraint(ownership) { relay, requirement ->
            when (requirement) {
                Ownership.MullvadOwned -> relay.owned
                Ownership.Rented -> !relay.owned
            }
        }
        .filterConstraint(providers) { relay, requirement ->
            requirement.providers.contains(relay.provider)
        }

private inline fun <E, T> List<E>.filterConstraint(
    constraint: Constraint<T>,
    predicate: (E, onlyValue: T) -> Boolean
): List<E> {
    return when (constraint) {
        is Constraint.Any -> this
        is Constraint.Only -> filter { predicate(it, constraint.value) }
    }
}

[Pururun]

Reviewable status: 0 of 17 files reviewed, 1 unresolved discussion (waiting on @Rawa)

---

android/app/src/main/kotlin/net/mullvad/mullvadvpn/relaylist/RelayListExtensions.kt line 197 at r1 (raw file):

Previously, Rawa (David Göransson) wrote…

I feel that this filter is not very intuitive, we sort of mash it into one big filter. Alternative would be:

private fun List<net.mullvad.mullvadvpn.model.Relay>.filterValidRelays2(
    ownership: Constraint<Ownership>,
    providers: Constraint<Providers>
): List<net.mullvad.mullvadvpn.model.Relay> =
    filter { it.isWireguardRelay }
        .filter {
            when(ownership) {
                is Constraint.Any -> true
                is Constraint.Only -> when(ownership.value) {
                    Ownership.MullvadOwned -> it.owned
                    Ownership.Rented -> !it.owned
                }
            }
        }
        .filter { relay ->
            when(providers) {
                is Constraint.Any -> true
                is Constraint.Only -> providers.value.providers.contains(relay.provider)
            }
        }

A bit more exotic solution would be the following, but I'm not sure I think it is more readable and it is almost a bit of a overkill since we only have two Constraints right now:

private fun List<net.mullvad.mullvadvpn.model.Relay>.filterValidRelays3(
    ownership: Constraint<Ownership>,
    providers: Constraint<Providers>
): List<net.mullvad.mullvadvpn.model.Relay> =
    filter { it.isWireguardRelay }
        .filterConstraint(ownership) { relay, requirement ->
            when (requirement) {
                Ownership.MullvadOwned -> relay.owned
                Ownership.Rented -> !relay.owned
            }
        }
        .filterConstraint(providers) { relay, requirement ->
            requirement.providers.contains(relay.provider)
        }

private inline fun <E, T> List<E>.filterConstraint(
    constraint: Constraint<T>,
    predicate: (E, onlyValue: T) -> Boolean
): List<E> {
    return when (constraint) {
        is Constraint.Any -> this
        is Constraint.Only -> filter { predicate(it, constraint.value) }
    }
}

I will switch to suggestion filterValidRelays2

[Pururun]

Reviewable status: 0 of 17 files reviewed, 1 unresolved discussion (waiting on @Rawa)

---

android/app/src/main/kotlin/net/mullvad/mullvadvpn/relaylist/RelayListExtensions.kt line 197 at r1 (raw file):

Previously, Pururun (Jonatan Rhodin) wrote…

I will switch to suggestion filterValidRelays2

Done

[Rawa]

Reviewed 17 of 17 files at r1, 2 of 2 files at r2, 4 of 4 files at r3, all commit messages.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @Pururun)

---

android/app/src/main/kotlin/net/mullvad/mullvadvpn/usecase/RelayListFilterUseCase.kt line 35 at r1 (raw file):

        }

    fun availableProviders(): Flow<List<Pair<String, Boolean>>> =

We should make a domain type to represent this, I helped the interns a bit with it and made a UiProvider but I think it would make sense to add something to our domain called "Provider" that we can use to represent a provider.

---

android/app/src/main/kotlin/net/mullvad/mullvadvpn/usecase/RelayListUseCase.kt line 47 at r1 (raw file):

                    relayCountries = relayCountries
                )
            Pair(relayCountries, selectedItem)

Imo it would be more clear with a data class than a Pair.

I'm also a bit torn whether maybe ViewModel should combine these or if we need it at this level. (E.g exposing RelayList and it's selection seperately but I think that part probably is fine for now.

---

android/app/src/main/kotlin/net/mullvad/mullvadvpn/usecase/RelayListUseCase.kt line 52 at r1 (raw file):

    private fun findSelectedRelayItem(
        relaySettings: RelaySettings?,
        relayCountries: List<RelayCountry>?

Does it make sense that everything can be null? Feels like we allow for nullability a bit too much in this case. Maybe we should have an extension function on List<RelayCountry> instead?

E.g relayCountries?.findItemFittingSettings(relaySettings) or something along that line.

---

android/app/src/main/kotlin/net/mullvad/mullvadvpn/viewmodel/SelectLocationViewModel.kt line 31 at r1 (raw file):

    val uiState =
        combine(relayListUseCase.relayListWithSelection(), _searchTerm) {

way nicer 👍

[Rawa]

Good job! nice improvement, breaking it out to UseCases makes it may more simple to understand imo.

Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @Pururun)

[Pururun]

Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @Rawa)

---

android/app/src/main/kotlin/net/mullvad/mullvadvpn/usecase/RelayListFilterUseCase.kt line 35 at r1 (raw file):

Previously, Rawa (David Göransson) wrote…

We should make a domain type to represent this, I helped the interns a bit with it and made a UiProvider but I think it would make sense to add something to our domain called "Provider" that we can use to represent a provider.

Yeah sounds good

---

android/app/src/main/kotlin/net/mullvad/mullvadvpn/usecase/RelayListUseCase.kt line 47 at r1 (raw file):

Previously, Rawa (David Göransson) wrote…

Imo it would be more clear with a data class than a Pair.

I'm also a bit torn whether maybe ViewModel should combine these or if we need it at this level. (E.g exposing RelayList and it's selection seperately but I think that part probably is fine for now.

It is a pair because that is how it worked before. I think we can improve it a lot if we put some thought into it. :) So let's do that.

[Pururun]

Reviewable status: 13 of 20 files reviewed, 4 unresolved discussions (waiting on @Rawa)

---

android/app/src/main/kotlin/net/mullvad/mullvadvpn/usecase/RelayListFilterUseCase.kt line 35 at r1 (raw file):

Previously, Pururun (Jonatan Rhodin) wrote…

Yeah sounds good

Done

---

android/app/src/main/kotlin/net/mullvad/mullvadvpn/usecase/RelayListUseCase.kt line 52 at r1 (raw file):

Previously, Rawa (David Göransson) wrote…

Does it make sense that everything can be null? Feels like we allow for nullability a bit too much in this case. Maybe we should have an extension function on List<RelayCountry> instead?

E.g relayCountries?.findItemFittingSettings(relaySettings) or something along that line.

So I changed so that we get some kind of default value from the relay list listener instead of null. In general we should not get a relay list that is null from the daemon unless something is horribly wrong. I did keep null it for the location constraint since we are able to get null settings from the repository and I thought it might be a bit out of scope here to change that part as well. Made it into an extension function.

[Rawa]

Nice clean up on the tests! 🙏

Reviewed 7 of 7 files at r4, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @Pururun)

---

android/app/src/main/kotlin/net/mullvad/mullvadvpn/relaylist/Provider.kt line 3 at r4 (raw file):

package net.mullvad.mullvadvpn.relaylist

data class Provider(val name: String, val mullvadOwned: Boolean)

Nice 🥇

---

android/app/src/main/kotlin/net/mullvad/mullvadvpn/usecase/RelayListUseCase.kt line 52 at r1 (raw file):

Previously, Pururun (Jonatan Rhodin) wrote…

So I changed so that we get some kind of default value from the relay list listener instead of null. In general we should not get a relay list that is null from the daemon unless something is horribly wrong. I did keep null it for the location constraint since we are able to get null settings from the repository and I thought it might be a bit out of scope here to change that part as well. Made it into an extension function.

Yeah, makes sense from this perspective that findSelectRelayItem only is relevant if List<RelayCountry>isn't null. If relayCountries as null makes more sense i think it is fine. Now we can always call relayCountries?.findSelectedRelayItem() which would be more clear.

[Rawa]

Reviewed 7 of 7 files at r5, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @Pururun)

[Rawa]

Reviewed 5 of 5 files at r6, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @Pururun)

[Rawa]

Reviewed 6 of 6 files at r7, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @Pururun)