diff --git a/app/src/androidTest/java/at/bitfire/davdroid/network/ConnectionUtilsTest.kt b/app/src/androidTest/java/at/bitfire/davdroid/network/ConnectionUtilsTest.kt new file mode 100644 index 00000000..e1311821 --- /dev/null +++ b/app/src/androidTest/java/at/bitfire/davdroid/network/ConnectionUtilsTest.kt @@ -0,0 +1,149 @@ +/* + * Copyright © All Contributors. See LICENSE and AUTHORS in the root directory for details. + */ + +package at.bitfire.davdroid.network + +import android.net.ConnectivityManager +import android.net.Network +import android.net.NetworkCapabilities +import android.net.NetworkCapabilities.NET_CAPABILITY_INTERNET +import android.net.NetworkCapabilities.NET_CAPABILITY_NOT_VPN +import android.net.NetworkCapabilities.NET_CAPABILITY_VALIDATED +import android.net.NetworkCapabilities.TRANSPORT_WIFI +import dagger.hilt.android.testing.HiltAndroidTest +import io.mockk.every +import io.mockk.mockk +import junit.framework.TestCase.assertFalse +import junit.framework.TestCase.assertTrue +import org.junit.Before +import org.junit.Test + +@HiltAndroidTest +class ConnectionUtilsTest { + + private val connectivityManager = mockk() + private val network1 = mockk() + private val network2 = mockk() + private val capabilities = mockk() + + @Before + fun setUp() { + every { connectivityManager.allNetworks } returns arrayOf(network1, network2) + every { connectivityManager.getNetworkInfo(network1) } returns mockk() + every { connectivityManager.getNetworkInfo(network2) } returns mockk() + every { connectivityManager.getNetworkCapabilities(network1) } returns capabilities + every { connectivityManager.getNetworkCapabilities(network2) } returns capabilities + } + + @Test + fun testWifiAvailable_capabilitiesNull() { + every { connectivityManager.getNetworkCapabilities(network1) } returns null + every { connectivityManager.getNetworkCapabilities(network2) } returns null + assertFalse(ConnectionUtils.wifiAvailable(connectivityManager)) + } + + @Test + fun testWifiAvailable() { + every { capabilities.hasTransport(TRANSPORT_WIFI) } returns false + every { capabilities.hasCapability(NET_CAPABILITY_VALIDATED) } returns false + assertFalse(ConnectionUtils.wifiAvailable(connectivityManager)) + } + + @Test + fun testWifiAvailable_wifi() { + every { capabilities.hasTransport(TRANSPORT_WIFI) } returns true + every { capabilities.hasCapability(NET_CAPABILITY_VALIDATED) } returns false + assertFalse(ConnectionUtils.wifiAvailable(connectivityManager)) + } + + @Test + fun testWifiAvailable_validated() { + every { capabilities.hasTransport(TRANSPORT_WIFI) } returns false + every { capabilities.hasCapability(NET_CAPABILITY_VALIDATED) } returns true + assertFalse(ConnectionUtils.wifiAvailable(connectivityManager)) + } + + @Test + fun testWifiAvailable_wifiValidated() { + every { capabilities.hasTransport(TRANSPORT_WIFI) } returns true + every { capabilities.hasCapability(NET_CAPABILITY_VALIDATED) } returns true + assertTrue(ConnectionUtils.wifiAvailable(connectivityManager)) + } + + + @Test + fun testInternetAvailable_capabilitiesNull() { + every { connectivityManager.getNetworkCapabilities(network1) } returns null + every { connectivityManager.getNetworkCapabilities(network2) } returns null + assertFalse(ConnectionUtils.internetAvailable(connectivityManager, false)) + } + + @Test + fun testInternetAvailable_Internet() { + every { capabilities.hasCapability(NET_CAPABILITY_INTERNET) } returns true + every { capabilities.hasCapability(NET_CAPABILITY_VALIDATED) } returns false + assertFalse(ConnectionUtils.internetAvailable(connectivityManager, false)) + } + + @Test + fun testInternetAvailable_Validated() { + every { capabilities.hasCapability(NET_CAPABILITY_INTERNET) } returns false + every { capabilities.hasCapability(NET_CAPABILITY_VALIDATED) } returns true + assertFalse(ConnectionUtils.internetAvailable(connectivityManager, false)) + } + + @Test + fun testInternetAvailable_InternetValidated() { + every { capabilities.hasCapability(NET_CAPABILITY_INTERNET) } returns true + every { capabilities.hasCapability(NET_CAPABILITY_VALIDATED) } returns true + assertTrue(ConnectionUtils.internetAvailable(connectivityManager, false)) + } + + @Test + fun testInternetAvailable_ignoreVpns() { + every { capabilities.hasCapability(NET_CAPABILITY_INTERNET) } returns true + every { capabilities.hasCapability(NET_CAPABILITY_VALIDATED) } returns true + every { capabilities.hasCapability(NET_CAPABILITY_NOT_VPN) } returns false + assertFalse(ConnectionUtils.internetAvailable(connectivityManager, true)) + } + + @Test + fun testInternetAvailable_ignoreVpns_Notvpn() { + every { capabilities.hasCapability(NET_CAPABILITY_INTERNET) } returns true + every { capabilities.hasCapability(NET_CAPABILITY_VALIDATED) } returns true + every { capabilities.hasCapability(NET_CAPABILITY_NOT_VPN) } returns true + assertTrue(ConnectionUtils.internetAvailable(connectivityManager, true)) + } + + @Test + fun testInternetAvailable_twoConnectionsFirstOneWithoutInternet() { + // The real case that failed in davx5-ose#395 is that the connection list contains (in this order) + // 1. a mobile network without INTERNET, but with VALIDATED + // 2. a WiFi network with INTERNET and VALIDATED + + // The "return false" of hasINTERNET will trigger at the first connection, the + // "andThen true" will trigger for the second connection + every { capabilities.hasCapability(NET_CAPABILITY_INTERNET) } returns false andThen true + every { capabilities.hasCapability(NET_CAPABILITY_VALIDATED) } returns true + + // There is an internet connection if any(!) connection has both INTERNET and VALIDATED. + assertTrue(ConnectionUtils.internetAvailable(connectivityManager, false)) + } + + @Test + fun testInternetAvailable_twoConnectionsFirstOneWithoutValidated() { + every { capabilities.hasCapability(NET_CAPABILITY_INTERNET) } returns true + every { capabilities.hasCapability(NET_CAPABILITY_VALIDATED) } returns false andThen true + assertTrue(ConnectionUtils.internetAvailable(connectivityManager, false)) + } + + @Test + fun testInternetAvailable_twoConnectionsFirstOneWithoutNotvpn() { + every { capabilities.hasCapability(NET_CAPABILITY_INTERNET) } returns true + every { capabilities.hasCapability(NET_CAPABILITY_VALIDATED) } returns true + every { capabilities.hasCapability(NET_CAPABILITY_NOT_VPN) } returns false andThen true + assertTrue(ConnectionUtils.internetAvailable(connectivityManager, true)) + } + +} \ No newline at end of file diff --git a/app/src/main/java/at/bitfire/davdroid/network/ConnectionUtils.kt b/app/src/main/java/at/bitfire/davdroid/network/ConnectionUtils.kt index f54ff2d4..36162a60 100644 --- a/app/src/main/java/at/bitfire/davdroid/network/ConnectionUtils.kt +++ b/app/src/main/java/at/bitfire/davdroid/network/ConnectionUtils.kt @@ -4,21 +4,18 @@ package at.bitfire.davdroid.network -import android.content.Context import android.net.ConnectivityManager import android.net.NetworkCapabilities import androidx.annotation.RequiresApi -import androidx.core.content.getSystemService import at.bitfire.davdroid.log.Logger import java.util.logging.Level object ConnectionUtils { /** - * Checks whether we are connected to working WiFi + * Checks whether we are connected to validated WiFi */ - internal fun wifiAvailable(context: Context): Boolean { - val connectivityManager = context.getSystemService()!! + internal fun wifiAvailable(connectivityManager: ConnectivityManager): Boolean { connectivityManager.allNetworks.forEach { network -> connectivityManager.getNetworkCapabilities(network)?.let { capabilities -> if (capabilities.hasTransport(NetworkCapabilities.TRANSPORT_WIFI) && @@ -33,7 +30,7 @@ object ConnectionUtils { * Checks whether we are connected to the Internet. * * On API 26+ devices, if a VPN is used, WorkManager might start the SyncWorker without an - * internet connection (because NET_CAPABILITY_VALIDATED is always set for VPN connections). + * Internet connection (because [NetworkCapabilities.NET_CAPABILITY_VALIDATED] is always set for VPN connections). * To prevent the start without internet access, we don't check for VPN connections by default * (by using [NetworkCapabilities.NET_CAPABILITY_NOT_VPN]). * @@ -44,8 +41,7 @@ object ConnectionUtils { * @return whether we are connected to the Internet */ @RequiresApi(23) - internal fun internetAvailable(context: Context, ignoreVpns: Boolean): Boolean { - val connectivityManager = context.getSystemService()!! + internal fun internetAvailable(connectivityManager: ConnectivityManager, ignoreVpns: Boolean): Boolean { return connectivityManager.allNetworks.any { network -> val capabilities = connectivityManager.getNetworkCapabilities(network) Logger.log.log(Level.FINE, "Looking for validated Internet over this connection.", diff --git a/app/src/main/java/at/bitfire/davdroid/syncadapter/SyncWorker.kt b/app/src/main/java/at/bitfire/davdroid/syncadapter/SyncWorker.kt index 9ca95ec5..e8788c2d 100644 --- a/app/src/main/java/at/bitfire/davdroid/syncadapter/SyncWorker.kt +++ b/app/src/main/java/at/bitfire/davdroid/syncadapter/SyncWorker.kt @@ -10,6 +10,7 @@ import android.content.ContentResolver import android.content.Context import android.content.Intent import android.content.SyncResult +import android.net.ConnectivityManager import android.net.wifi.WifiManager import android.os.Build import android.provider.CalendarContract @@ -254,7 +255,8 @@ class SyncWorker @AssistedInject constructor( return true // yes, continue // WiFi required, is it available? - if (!wifiAvailable(context)) { + val connectivityManager = context.getSystemService()!! + if (!wifiAvailable(connectivityManager)) { Logger.log.info("Not on connected WiFi, stopping") return false } @@ -282,7 +284,8 @@ class SyncWorker @AssistedInject constructor( // Check internet connection val ignoreVpns = AccountSettings(applicationContext, account).getIgnoreVpns() - if (Build.VERSION.SDK_INT >= 23 && !internetAvailable(applicationContext, ignoreVpns)) { + val connectivityManager = applicationContext.getSystemService()!! + if (Build.VERSION.SDK_INT >= 23 && !internetAvailable(connectivityManager, ignoreVpns)) { Logger.log.info("WorkManager started SyncWorker without Internet connection. Aborting.") return Result.failure() }