Merge branch 'improve-relay-sorting'

6 files changed, 159 insertions, 2 deletions

diff --git a/.github/workflows/android-app.yml b/.github/workflows/android-app.yml
index 7119167f76..f87b518774 100644
--- a/.github/workflows/android-app.yml
+++ b/.github/workflows/android-app.yml
@@ -75,3 +75,4 @@ jobs:
                 cargo build --target aarch64-linux-android --verbose --package mullvad-jni
                 cd android
                 ./gradlew --console plain assembleDebug
+                ./gradlew testDebugUnitTest
diff --git a/.github/workflows/android-xml-tidy.yml b/.github/workflows/android-xml-tidy.yml
index 3b533c0d69..e733df4378 100644
--- a/.github/workflows/android-xml-tidy.yml
+++ b/.github/workflows/android-xml-tidy.yml
@@ -4,7 +4,7 @@ on:
     push:
         paths:
             - .github/workflows/android-xml-tidy.yml
-            - android/**/*.xml
+            - android/src/main/**/*.xml
     # Run verifier if requested manually from the Actions tab
     workflow_dispatch:
 jobs:
diff --git a/android/build.gradle b/android/build.gradle
index f6d91922c5..91f1ff8945 100644
--- a/android/build.gradle
+++ b/android/build.gradle
@@ -65,6 +65,12 @@ android {
                 srcDirs += 'src/main/kotlin/'
             }
         }
+
+        test {
+            java {
+                srcDirs += 'src/test/kotlin/'
+            }
+        }
     }
     compileOptions {
         sourceCompatibility JavaVersion.VERSION_1_8
@@ -76,6 +82,17 @@ android {
             dependsOn copyExtraAssets
         }
     }
+
+    testOptions {
+        unitTests.all {
+            testLogging {
+                outputs.upToDateWhen { false }
+                events "passed", "skipped", "failed", "standardOut", "standardError"
+                showCauses true
+                showExceptions true
+            }
+        }
+    }
 }
 
 tasks.withType(org.jetbrains.kotlin.gradle.tasks.KotlinCompile).all {
@@ -109,9 +126,16 @@ dependencies {
     implementation 'joda-time:joda-time:2.10.2'
     implementation 'org.jetbrains.kotlin:kotlin-stdlib:1.4.10'
     implementation 'org.jetbrains.kotlinx:kotlinx-coroutines-android:1.4.2'
+
+    /* Test dependencies */
+    testImplementation "io.mockk:mockk:$mockkVersion"
+    testImplementation 'junit:junit:4.12'
 }
 
 buildscript {
+    ext {
+        mockkVersion = '1.10.6'
+    }
     repositories {
         jcenter()
         google()
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)
+    }
+}