Merge branch 'improve-relay-sorting'

3 files changed, 133 insertions, 1 deletions

diff --git a/android/src/main/kotlin/net/mullvad/mullvadvpn/relaylist/RelayList.kt b/android/src/main/kotlin/net/mullvad/mullvadvpn/relaylist/RelayList.kt
index 781155df51..aed15f9508 100644
--- a/android/src/main/kotlin/net/mullvad/mullvadvpn/relaylist/RelayList.kt
+++ b/android/src/main/kotlin/net/mullvad/mullvadvpn/relaylist/RelayList.kt
@@ -21,7 +21,7 @@ class RelayList {
                     for (relay in validCityRelays) {
                         relays.add(Relay(relayCity, relay.hostname, relay.active))
                     }
-                    relays.sortBy({ it.name })
+                    relays.sortWith(RelayNameComparator)
 
                     if (relays.isNotEmpty()) {
                         cities.add(relayCity)
diff --git a/android/src/main/kotlin/net/mullvad/mullvadvpn/relaylist/RelayNameComparator.kt b/android/src/main/kotlin/net/mullvad/mullvadvpn/relaylist/RelayNameComparator.kt
new file mode 100644
index 0000000000..32f473b194
--- /dev/null
+++ b/android/src/main/kotlin/net/mullvad/mullvadvpn/relaylist/RelayNameComparator.kt
@@ -0,0 +1,35 @@
+package net.mullvad.mullvadvpn.relaylist
+
+internal object RelayNameComparator : Comparator<Relay> {
+    override fun compare(o1: Relay, o2: Relay): Int {
+        val partitions1 = o1.name.split(regex)
+        val partitions2 = o2.name.split(regex)
+        return if (partitions1.size > partitions2.size)
+            partitions1 compareWith partitions2
+        else
+            -(partitions2 compareWith partitions1)
+    }
+
+    private infix fun List<String>.compareWith(other: List<String>): Int {
+        this.forEachIndexed { index, s ->
+            if (other.size <= index)
+                return 1
+            val partsCompareResult = compareStringOrInt(other[index], s)
+            if (partsCompareResult != 0)
+                return partsCompareResult
+        }
+        return 0
+    }
+
+    private fun compareStringOrInt(s1: String, s2: String): Int {
+        val int1 = s1.toIntOrNull()
+        val int2 = s2.toIntOrNull()
+        return if (int1 == null || int2 == null || int1 == int2) {
+            s2.compareTo(s1)
+        } else {
+            int2.compareTo(int1)
+        }
+    }
+
+    private val regex = "(?<=\\d)(?=\\D)|(?<=\\D)(?=\\d)".toRegex()
+}
diff --git a/android/src/test/kotlin/net/mullvad/mullvadvpn/relaylist/RelayNameComparatorTest.kt b/android/src/test/kotlin/net/mullvad/mullvadvpn/relaylist/RelayNameComparatorTest.kt
new file mode 100644
index 0000000000..a3c96349d9
--- /dev/null
+++ b/android/src/test/kotlin/net/mullvad/mullvadvpn/relaylist/RelayNameComparatorTest.kt
@@ -0,0 +1,97 @@
+package net.mullvad.mullvadvpn.relaylist
+
+import io.mockk.mockk
+import io.mockk.unmockkAll
+import org.junit.After
+import org.junit.Assert.assertTrue
+import org.junit.Test
+
+class RelayNameComparatorTest {
+
+    private val mockedCity = mockk<RelayCity>(relaxed = true)
+
+    @After
+    fun tearDown() {
+        unmockkAll()
+    }
+
+    @Test
+    fun test_compare_respect_numbers_in_name() {
+        val relay9 = Relay(mockedCity, "se9-wireguard", false)
+        val relay10 = Relay(mockedCity, "se10-wireguard", false)
+
+        relay9 assertOrderBothDirection relay10
+    }
+
+    @Test
+    fun test_compare_same_name() {
+        val relay9a = Relay(mockedCity, "se9-wireguard", false)
+        val relay9b = Relay(mockedCity, "se9-wireguard", false)
+
+        assertTrue(RelayNameComparator.compare(relay9a, relay9b) == 0)
+        assertTrue(RelayNameComparator.compare(relay9b, relay9a) == 0)
+    }
+
+    @Test
+    fun test_compare_only_numbers_in_name() {
+        val relay001 = Relay(mockedCity, "001", false)
+        val relay1 = Relay(mockedCity, "1", false)
+        val relay3 = Relay(mockedCity, "3", false)
+        val relay100 = Relay(mockedCity, "100", false)
+
+        relay001 assertOrderBothDirection relay1
+        relay001 assertOrderBothDirection relay3
+        relay1 assertOrderBothDirection relay3
+        relay3 assertOrderBothDirection relay100
+    }
+
+    @Test
+    fun test_compare_without_numbers_in_name() {
+        val relay9a = Relay(mockedCity, "se-wireguard", false)
+        val relay9b = Relay(mockedCity, "se-wireguard", false)
+
+        assertTrue(RelayNameComparator.compare(relay9a, relay9b) == 0)
+        assertTrue(RelayNameComparator.compare(relay9b, relay9a) == 0)
+    }
+
+    @Test
+    fun test_compare_with_trailing_zeros_in_name() {
+        val relay001 = Relay(mockedCity, "se001-wireguard", false)
+        val relay005 = Relay(mockedCity, "se005-wireguard", false)
+
+        relay001 assertOrderBothDirection relay005
+    }
+
+    @Test
+    fun test_compare_prefix_and_numbers() {
+        val relayAr2 = Relay(mockedCity, "ar2-wireguard", false)
+        val relayAr8 = Relay(mockedCity, "ar8-wireguard", false)
+        val relaySe5 = Relay(mockedCity, "se5-wireguard", false)
+        val relaySe10 = Relay(mockedCity, "se10-wireguard", false)
+
+        relayAr2 assertOrderBothDirection relayAr8
+        relayAr8 assertOrderBothDirection relaySe5
+        relaySe5 assertOrderBothDirection relaySe10
+    }
+
+    @Test
+    fun test_compare_suffix_and_numbers() {
+        val relay2c = Relay(mockedCity, "se2-cloud", false)
+        val relay2w = Relay(mockedCity, "se2-wireguard", false)
+
+        relay2c assertOrderBothDirection relay2w
+    }
+
+    @Test
+    fun test_compare_different_length() {
+        val relay22a = Relay(mockedCity, "se22", false)
+        val relay22b = Relay(mockedCity, "se22-wireguard", false)
+
+        relay22a assertOrderBothDirection relay22b
+    }
+
+    private infix fun Relay.assertOrderBothDirection(other: Relay) {
+        assertTrue(RelayNameComparator.compare(this, other) < 0)
+        assertTrue(RelayNameComparator.compare(other, this) > 0)
+    }
+}