Add ServiceConnection scope. Fix SplitTunnelingViewmodel tests

8 files changed, 109 insertions, 25 deletions

diff --git a/android/src/main/kotlin/net/mullvad/mullvadvpn/di/AppModule.kt b/android/src/main/kotlin/net/mullvad/mullvadvpn/di/AppModule.kt
index a144ae6bc2..87aabc6d32 100644
--- a/android/src/main/kotlin/net/mullvad/mullvadvpn/di/AppModule.kt
+++ b/android/src/main/kotlin/net/mullvad/mullvadvpn/di/AppModule.kt
@@ -1,10 +1,16 @@
 package net.mullvad.mullvadvpn.di
 
 import android.content.pm.PackageManager
+import android.os.Messenger
 import kotlinx.coroutines.Dispatchers
 import net.mullvad.mullvadvpn.applist.ApplicationsIconManager
 import net.mullvad.mullvadvpn.applist.ApplicationsProvider
-import net.mullvad.mullvadvpn.service.SplitTunneling
+import net.mullvad.mullvadvpn.ipc.Event
+import net.mullvad.mullvadvpn.ipc.MessageDispatcher
+import net.mullvad.mullvadvpn.service.ServiceInstance
+import net.mullvad.mullvadvpn.ui.MainActivity
+import net.mullvad.mullvadvpn.ui.serviceconnection.ServiceConnection
+import net.mullvad.mullvadvpn.ui.serviceconnection.SplitTunneling
 import net.mullvad.mullvadvpn.viewmodel.SplitTunnelingViewModel
 import org.koin.android.ext.koin.androidContext
 import org.koin.androidx.viewmodel.dsl.viewModel
@@ -14,7 +20,6 @@ import org.koin.dsl.onClose
 
 val appModule = module {
 
-    single<SplitTunneling> { SplitTunneling(androidContext()) }
     single<PackageManager> { androidContext().packageManager }
     single<String> (named(SELF_PACKAGE_NAME)) { androidContext().packageName }
 
@@ -23,6 +28,15 @@ val appModule = module {
         scoped { ApplicationsIconManager(get()) } onClose { it?.dispose() }
         scoped { ApplicationsProvider(get(), get(named(SELF_PACKAGE_NAME))) }
     }
+
+    scope<ServiceConnection> {
+        scoped<ServiceConnection> { (service: ServiceInstance, mainActivity: MainActivity) ->
+            ServiceConnection(service, mainActivity)
+        } onClose { it?.onDestroy() }
+        scoped<SplitTunneling> { (messenger: Messenger, dispatcher: MessageDispatcher<Event>) ->
+            SplitTunneling(messenger, dispatcher)
+        }
+    }
 }
 const val APPS_SCOPE = "APPS_SCOPE"
 const val SELF_PACKAGE_NAME = "SELF_PACKAGE_NAME"
diff --git a/android/src/main/kotlin/net/mullvad/mullvadvpn/ipc/DispatchingHandler.kt b/android/src/main/kotlin/net/mullvad/mullvadvpn/ipc/DispatchingHandler.kt
index 0f3820ff48..93c79a1ab9 100644
--- a/android/src/main/kotlin/net/mullvad/mullvadvpn/ipc/DispatchingHandler.kt
+++ b/android/src/main/kotlin/net/mullvad/mullvadvpn/ipc/DispatchingHandler.kt
@@ -11,11 +11,11 @@ import kotlin.reflect.KClass
 class DispatchingHandler<T : Any>(
     looper: Looper,
     private val extractor: (Message) -> T?
-) : Handler(looper) {
+) : Handler(looper), MessageDispatcher<T> {
     private val handlers = HashMap<KClass<out T>, (T) -> Unit>()
     private val lock = ReentrantReadWriteLock()
 
-    fun <V : T> registerHandler(variant: KClass<V>, handler: (V) -> Unit) {
+    override fun <V : T> registerHandler(variant: KClass<V>, handler: (V) -> Unit) {
         lock.writeLock().withLock {
             handlers.put(variant) { instance ->
                 @Suppress("UNCHECKED_CAST")
diff --git a/android/src/main/kotlin/net/mullvad/mullvadvpn/ipc/MessageDispatcher.kt b/android/src/main/kotlin/net/mullvad/mullvadvpn/ipc/MessageDispatcher.kt
new file mode 100644
index 0000000000..8a681b2ce4
--- /dev/null
+++ b/android/src/main/kotlin/net/mullvad/mullvadvpn/ipc/MessageDispatcher.kt
@@ -0,0 +1,7 @@
+package net.mullvad.mullvadvpn.ipc
+
+import kotlin.reflect.KClass
+
+interface MessageDispatcher<T : Any> {
+    fun <V : T> registerHandler(variant: KClass<V>, handler: (V) -> Unit)
+}
diff --git a/android/src/main/kotlin/net/mullvad/mullvadvpn/ui/MainActivity.kt b/android/src/main/kotlin/net/mullvad/mullvadvpn/ui/MainActivity.kt
index 89df0bbedb..486e1d156c 100644
--- a/android/src/main/kotlin/net/mullvad/mullvadvpn/ui/MainActivity.kt
+++ b/android/src/main/kotlin/net/mullvad/mullvadvpn/ui/MainActivity.kt
@@ -23,6 +23,9 @@ import net.mullvad.mullvadvpn.ipc.Event
 import net.mullvad.mullvadvpn.service.MullvadVpnService
 import net.mullvad.mullvadvpn.ui.serviceconnection.ServiceConnection
 import net.mullvad.talpid.util.EventNotifier
+import org.koin.android.ext.android.getKoin
+import org.koin.core.parameter.parametersOf
+import org.koin.core.scope.Scope
 
 open class MainActivity : FragmentActivity() {
     companion object {
@@ -44,6 +47,7 @@ open class MainActivity : FragmentActivity() {
         uiModeManager.currentModeType == Configuration.UI_MODE_TYPE_TELEVISION
     }
 
+    private var serviceConnectionScope: Scope? = null
     private val serviceConnectionManager = object : android.content.ServiceConnection {
         override fun onServiceConnected(className: ComponentName, binder: IBinder) {
             android.util.Log.d("mullvad", "UI successfully connected to the service")
@@ -55,10 +59,13 @@ open class MainActivity : FragmentActivity() {
 
             localBinder.serviceNotifier.subscribe(this@MainActivity) { service ->
                 android.util.Log.d("mullvad", "UI connection to the service changed: $service")
-                serviceConnection?.onDestroy()
+                serviceConnectionScope?.close()
 
                 val newConnection = service?.let { safeService ->
-                    ServiceConnection(safeService, this@MainActivity)
+                    serviceConnectionScope = getKoin().createScope<ServiceConnection>()
+                    serviceConnectionScope?.get<ServiceConnection>(
+                        parameters = { parametersOf(safeService, this@MainActivity) }
+                    )
                 }
 
                 serviceConnection = newConnection
@@ -80,7 +87,7 @@ open class MainActivity : FragmentActivity() {
         override fun onServiceDisconnected(className: ComponentName) {
             android.util.Log.d("mullvad", "UI lost the connection to the service")
             service?.serviceNotifier?.unsubscribe(this@MainActivity)
-            serviceConnection?.onDestroy()
+            serviceConnectionScope?.close()
             service = null
             serviceConnection = null
             serviceNotifier.notify(null)
@@ -154,7 +161,7 @@ open class MainActivity : FragmentActivity() {
 
     override fun onDestroy() {
         serviceNotifier.unsubscribeAll()
-        serviceConnection?.onDestroy()
+        serviceConnectionScope?.close()
 
         super.onDestroy()
     }
diff --git a/android/src/main/kotlin/net/mullvad/mullvadvpn/ui/serviceconnection/ServiceConnection.kt b/android/src/main/kotlin/net/mullvad/mullvadvpn/ui/serviceconnection/ServiceConnection.kt
index c5c1716ea9..becd03b203 100644
--- a/android/src/main/kotlin/net/mullvad/mullvadvpn/ui/serviceconnection/ServiceConnection.kt
+++ b/android/src/main/kotlin/net/mullvad/mullvadvpn/ui/serviceconnection/ServiceConnection.kt
@@ -11,12 +11,18 @@ import net.mullvad.mullvadvpn.ipc.Event
 import net.mullvad.mullvadvpn.ipc.Request
 import net.mullvad.mullvadvpn.service.ServiceInstance
 import net.mullvad.mullvadvpn.ui.MainActivity
+import org.koin.core.component.KoinApiExtension
+import org.koin.core.component.KoinComponent
+import org.koin.core.component.get
+import org.koin.core.parameter.parametersOf
 
 // Container of classes that communicate with the service through an active connection
 //
 // The properties of this class can be used to send events to the service, to listen for events from
 // the service and to get values received from events.
-class ServiceConnection(private val service: ServiceInstance, val mainActivity: MainActivity) {
+@OptIn(KoinApiExtension::class)
+class ServiceConnection(private val service: ServiceInstance, mainActivity: MainActivity) :
+    KoinComponent {
     val dispatcher = DispatchingHandler(Looper.getMainLooper()) { message ->
         Event.fromMessage(message)
     }
@@ -28,7 +34,9 @@ class ServiceConnection(private val service: ServiceInstance, val mainActivity:
     val keyStatusListener = KeyStatusListener(service.messenger, dispatcher)
     val locationInfoCache = LocationInfoCache(dispatcher)
     val settingsListener = SettingsListener(dispatcher)
-    val splitTunneling = SplitTunneling(service.messenger, dispatcher)
+    val splitTunneling = get<SplitTunneling>(
+        parameters = { parametersOf(service.messenger, dispatcher) }
+    )
 
     val appVersionInfoCache = AppVersionInfoCache(mainActivity, daemon, settingsListener)
     var relayListListener = RelayListListener(daemon, settingsListener)
diff --git a/android/src/main/kotlin/net/mullvad/mullvadvpn/ui/serviceconnection/SplitTunneling.kt b/android/src/main/kotlin/net/mullvad/mullvadvpn/ui/serviceconnection/SplitTunneling.kt
index a98c55b5de..4bcc3e83a1 100644
--- a/android/src/main/kotlin/net/mullvad/mullvadvpn/ui/serviceconnection/SplitTunneling.kt
+++ b/android/src/main/kotlin/net/mullvad/mullvadvpn/ui/serviceconnection/SplitTunneling.kt
@@ -2,11 +2,14 @@ package net.mullvad.mullvadvpn.ui.serviceconnection
 
 import android.os.Messenger
 import kotlin.properties.Delegates.observable
-import net.mullvad.mullvadvpn.ipc.DispatchingHandler
 import net.mullvad.mullvadvpn.ipc.Event
+import net.mullvad.mullvadvpn.ipc.MessageDispatcher
 import net.mullvad.mullvadvpn.ipc.Request
 
-class SplitTunneling(val connection: Messenger, eventDispatcher: DispatchingHandler<Event>) {
+class SplitTunneling(
+    private val connection: Messenger,
+    eventDispatcher: MessageDispatcher<Event>
+) {
     private var excludedApps: Set<String> = emptySet()
 
     var enabled by observable(false) { _, wasEnabled, isEnabled ->
@@ -28,15 +31,11 @@ class SplitTunneling(val connection: Messenger, eventDispatcher: DispatchingHand
 
     fun isAppExcluded(appPackageName: String): Boolean = excludedApps.contains(appPackageName)
 
-    fun excludeApp(appPackageName: String) {
+    fun excludeApp(appPackageName: String) =
         connection.send(Request.ExcludeApp(appPackageName).message)
-    }
 
-    fun includeApp(appPackageName: String) {
+    fun includeApp(appPackageName: String) =
         connection.send(Request.IncludeApp(appPackageName).message)
-    }
 
-    fun persist() {
-        connection.send(Request.PersistExcludedApps.message)
-    }
+    fun persist() = connection.send(Request.PersistExcludedApps.message)
 }
diff --git a/android/src/test/kotlin/net/mullvad/mullvadvpn/di/AppModuleTest.kt b/android/src/test/kotlin/net/mullvad/mullvadvpn/di/AppModuleTest.kt
new file mode 100644
index 0000000000..6984b064c1
--- /dev/null
+++ b/android/src/test/kotlin/net/mullvad/mullvadvpn/di/AppModuleTest.kt
@@ -0,0 +1,47 @@
+package net.mullvad.mullvadvpn.di
+
+import android.os.Messenger
+import io.mockk.mockk
+import io.mockk.unmockkAll
+import kotlin.test.assertEquals
+import net.mullvad.mullvadvpn.ipc.Event
+import net.mullvad.mullvadvpn.ipc.MessageDispatcher
+import net.mullvad.mullvadvpn.ui.serviceconnection.ServiceConnection
+import net.mullvad.mullvadvpn.ui.serviceconnection.SplitTunneling
+import org.junit.After
+import org.junit.Rule
+import org.junit.Test
+import org.koin.core.parameter.parametersOf
+import org.koin.core.qualifier.named
+import org.koin.core.scope.Scope
+import org.koin.test.KoinTest
+import org.koin.test.KoinTestRule
+
+class AppModuleTest : KoinTest {
+
+    @get:Rule
+    val koinTestRule = KoinTestRule.create {
+        modules(appModule)
+    }
+
+    @After
+    fun tearDown() {
+        unmockkAll()
+    }
+
+    @Test
+    fun test_scope_linking() {
+        val appsScope: Scope = getKoin().createScope(APPS_SCOPE, named(APPS_SCOPE))
+        val serviceConnectionScope = getKoin().createScope<ServiceConnection>()
+
+        appsScope.linkTo(serviceConnectionScope)
+
+        val mockedMessenger = mockk<Messenger>()
+        val mockedEventMessageHandler = mockk<MessageDispatcher<Event>>(relaxed = true)
+        val serviceConnectionSplitTunneling = serviceConnectionScope.get<SplitTunneling>(
+            parameters = { parametersOf(mockedMessenger, mockedEventMessageHandler) }
+        )
+
+        assertEquals(appsScope.get<SplitTunneling>(), serviceConnectionSplitTunneling)
+    }
+}
diff --git a/android/src/test/kotlin/net/mullvad/mullvadvpn/viewmodel/SplitTunnelingViewModelTest.kt b/android/src/test/kotlin/net/mullvad/mullvadvpn/viewmodel/SplitTunnelingViewModelTest.kt
index 00e6174487..f2834082c5 100644
--- a/android/src/test/kotlin/net/mullvad/mullvadvpn/viewmodel/SplitTunnelingViewModelTest.kt
+++ b/android/src/test/kotlin/net/mullvad/mullvadvpn/viewmodel/SplitTunnelingViewModelTest.kt
@@ -80,7 +80,8 @@ class SplitTunnelingViewModelTest {
     fun test_apps_list_delivered() = runBlockingTest(testCoroutineRule.testDispatcher) {
         val appExcluded = AppData("test.excluded", 0, "testName1")
         val appNotExcluded = AppData("test.not.excluded", 0, "testName2")
-        every { mockedSplitTunneling.excludedAppList } returns listOf(appExcluded.packageName)
+        every { mockedSplitTunneling.isAppExcluded(appExcluded.packageName) } returns true
+        every { mockedSplitTunneling.isAppExcluded(appNotExcluded.packageName) } returns false
 
         initTestSubject(listOf(appExcluded, appNotExcluded))
         testSubject.processIntent(ViewIntent.ViewIsReady)
@@ -99,14 +100,15 @@ class SplitTunnelingViewModelTest {
         assertLists(expectedList, actualList)
         verifyAll {
             mockedSplitTunneling.enabled
-            mockedSplitTunneling.excludedAppList
+            mockedSplitTunneling.isAppExcluded(appExcluded.packageName)
+            mockedSplitTunneling.isAppExcluded(appNotExcluded.packageName)
         }
     }
 
     @Test
     fun test_remove_app_from_excluded() = runBlockingTest(testCoroutineRule.testDispatcher) {
         val app = AppData("test", 0, "testName")
-        every { mockedSplitTunneling.excludedAppList } returns listOf(app.packageName)
+        every { mockedSplitTunneling.isAppExcluded(app.packageName) } returns true
         every { mockedSplitTunneling.includeApp(app.packageName) } just Runs
 
         initTestSubject(listOf(app))
@@ -137,7 +139,7 @@ class SplitTunnelingViewModelTest {
 
         verifyAll {
             mockedSplitTunneling.enabled
-            mockedSplitTunneling.excludedAppList
+            mockedSplitTunneling.isAppExcluded(app.packageName)
             mockedSplitTunneling.includeApp(app.packageName)
         }
     }
@@ -145,7 +147,7 @@ class SplitTunnelingViewModelTest {
     @Test
     fun test_add_app_to_excluded() = runBlockingTest(testCoroutineRule.testDispatcher) {
         val app = AppData("test", 0, "testName")
-        every { mockedSplitTunneling.excludedAppList } returns emptyList()
+        every { mockedSplitTunneling.isAppExcluded(app.packageName) } returns false
         every { mockedSplitTunneling.excludeApp(app.packageName) } just Runs
         initTestSubject(listOf(app))
         testSubject.processIntent(ViewIntent.ViewIsReady)
@@ -175,7 +177,7 @@ class SplitTunnelingViewModelTest {
 
         verifyAll {
             mockedSplitTunneling.enabled
-            mockedSplitTunneling.excludedAppList
+            mockedSplitTunneling.isAppExcluded(app.packageName)
             mockedSplitTunneling.excludeApp(app.packageName)
         }
     }