From 74a01ef3c5433ef87af693fc5b027b254d2b8a01 Mon Sep 17 00:00:00 2001 From: Dan Tran Date: Sun, 7 Apr 2024 11:53:08 -0400 Subject: [PATCH 1/9] Add ConnectivityNetworkCallback class in ReachabilityMonitor --- .../syncengine/ReachabilityMonitor.kt | 136 +++++++++++++++--- 1 file changed, 116 insertions(+), 20 deletions(-) diff --git a/aws-datastore/src/main/java/com/amplifyframework/datastore/syncengine/ReachabilityMonitor.kt b/aws-datastore/src/main/java/com/amplifyframework/datastore/syncengine/ReachabilityMonitor.kt index 1ce2112b65..4002e06ed9 100644 --- a/aws-datastore/src/main/java/com/amplifyframework/datastore/syncengine/ReachabilityMonitor.kt +++ b/aws-datastore/src/main/java/com/amplifyframework/datastore/syncengine/ReachabilityMonitor.kt @@ -15,16 +15,23 @@ package com.amplifyframework.datastore.syncengine +import android.annotation.SuppressLint import android.content.Context import android.net.ConnectivityManager import android.net.ConnectivityManager.NetworkCallback +import android.net.LinkProperties import android.net.Network +import android.net.NetworkCapabilities +import android.os.Build import androidx.annotation.VisibleForTesting import com.amplifyframework.datastore.DataStoreException +import io.reactivex.rxjava3.core.Completable import io.reactivex.rxjava3.core.Observable +import io.reactivex.rxjava3.core.ObservableEmitter import io.reactivex.rxjava3.subjects.BehaviorSubject import java.util.concurrent.TimeUnit + /** * The ReachabilityMonitor is responsible for watching the network status as provided by the OS. * It returns an observable that publishes "true" when the network becomes available and "false" when @@ -35,7 +42,7 @@ import java.util.concurrent.TimeUnit * The network changes are debounced with a 250 ms delay to allow some time for one network to connect after another * network has disconnected (for example, wifi is lost, then cellular connects) to reduce thrashing. */ -public interface ReachabilityMonitor { +interface ReachabilityMonitor { fun configure(context: Context) @VisibleForTesting fun configure(context: Context, connectivityProvider: ConnectivityProvider) @@ -65,15 +72,7 @@ private class ReachabilityMonitorImpl constructor(val schedulerProvider: Schedul val observable = Observable.create { emitter -> connectivityProvider.registerDefaultNetworkCallback( context, - object : NetworkCallback() { - override fun onAvailable(network: Network) { - emitter.onNext(true) - } - - override fun onLost(network: Network) { - emitter.onNext(false) - } - } + ConnectivityNetworkCallback(emitter) ) emitter.onNext(connectivityProvider.hasActiveNetwork) } @@ -91,6 +90,61 @@ private class ReachabilityMonitorImpl constructor(val schedulerProvider: Schedul } return subject.subscribeOn(schedulerProvider.io()) } + + private inner class ConnectivityNetworkCallback(private val emitter: ObservableEmitter) : NetworkCallback() { + private var currentNetwork: Network? = null + private var currentCapabilities: NetworkCapabilities? = null + private val DELAY_MS: Long = 250 + + override fun onAvailable(network: Network) { + currentNetwork = network + asyncUpdateAndSend() + } + + override fun onLosing(network: Network, maxMsToLive: Int) { + currentNetwork = network + updateAndSend() + } + + override fun onLost(network: Network) { + currentNetwork = null + currentCapabilities = null + updateAndSend() + } + + override fun onUnavailable() { + currentNetwork = null + currentCapabilities = null + updateAndSend() + } + + override fun onCapabilitiesChanged(network: Network, networkCapabilities: NetworkCapabilities) { + currentNetwork = network + currentCapabilities = networkCapabilities + updateAndSend() + } + + override fun onLinkPropertiesChanged(network: Network, linkProperties: LinkProperties) { + if (currentNetwork != null) { + currentNetwork = network + } + asyncUpdateAndSend() + } + + private fun updateAndSend() { + emitter.onNext(DefaultConnectivityProvider.isInternetReachable(currentCapabilities)) + } + + @SuppressLint("CheckResult") + private fun asyncUpdateAndSend() { + Completable.timer(DELAY_MS, TimeUnit.MILLISECONDS, schedulerProvider.computation()).subscribe { + currentCapabilities = DefaultConnectivityProvider.getNetworkCapabilities(connectivityProvider?.connectivityNetworkManager) + if (currentCapabilities != null) { + updateAndSend() + } + } + } + } } /** @@ -98,27 +152,32 @@ private class ReachabilityMonitorImpl constructor(val schedulerProvider: Schedul * is a concrete class created within context.getSystemService() it can't be overridden with a test * implementation, so this interface works around that issue. */ -public interface ConnectivityProvider { +interface ConnectivityProvider { val hasActiveNetwork: Boolean + val connectivityNetworkManager: ConnectivityManager? fun registerDefaultNetworkCallback(context: Context, callback: NetworkCallback) } private class DefaultConnectivityProvider : ConnectivityProvider { - private var connectivityManager: ConnectivityManager? = null + override val connectivityNetworkManager: ConnectivityManager? + get() = connectivityManager + override val hasActiveNetwork: Boolean - get() = connectivityManager?.let { it.activeNetwork != null } - ?: run { - throw DataStoreException( - "ReachabilityMonitor has not been configured.", - "Call ReachabilityMonitor.configure() before calling ReachabilityMonitor.getObservable()" - ) - } + get() = connectivityManager?.let { + val networkCapabilities: NetworkCapabilities? = getNetworkCapabilities(it) + isInternetReachable(networkCapabilities) + } ?: run { + throw DataStoreException( + "ReachabilityMonitor has not been configured.", + "Call ReachabilityMonitor.configure() before calling ReachabilityMonitor.getObservable()" + ) + } override fun registerDefaultNetworkCallback(context: Context, callback: NetworkCallback) { connectivityManager = context.getSystemService(ConnectivityManager::class.java) - connectivityManager?.let { it.registerDefaultNetworkCallback(callback) } + connectivityManager?.registerDefaultNetworkCallback(callback) ?: run { throw DataStoreException( "ConnectivityManager not available", @@ -126,4 +185,41 @@ private class DefaultConnectivityProvider : ConnectivityProvider { ) } } + + companion object { + fun getNetworkCapabilities(connectivityManager: ConnectivityManager?): NetworkCapabilities? { + try { + return connectivityManager?.let { + it.getNetworkCapabilities(it.activeNetwork) + } + } catch (ignored: SecurityException) { + // Android 11 may throw a 'package does not belong' security exception here. + // Google fixed Android 14, 13 and 12 with the issue where Chaland Jean patched those versions. + // Android 11 is too old, so that's why we have to catch this exception here to be safe. + // https://android.googlesource.com/platform/frameworks/base/+/249be21013e389837f5b2beb7d36890b25ecfaaf%5E%21/ + // We need to catch this to prevent app crash. + } + return null + } + + fun isInternetReachable(capabilities: NetworkCapabilities?): Boolean { + var isInternetReachable = false + if (capabilities != null) { + // Check to see if the network is temporarily unavailable or if airplane mode is toggled on + val isInternetSuspended = if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.P) { + !capabilities.hasCapability(NetworkCapabilities.NET_CAPABILITY_NOT_SUSPENDED) + } else { + // TODO: for SDK < 28, add extra check to evaluate airplane mode + false + } + isInternetReachable = (capabilities.hasCapability(NetworkCapabilities.NET_CAPABILITY_INTERNET) + && capabilities.hasCapability(NetworkCapabilities.NET_CAPABILITY_VALIDATED) + && !isInternetSuspended) + if (capabilities.hasTransport(NetworkCapabilities.TRANSPORT_VPN)) { + isInternetReachable = isInternetReachable && capabilities.linkDownstreamBandwidthKbps != 0 + } + } + return isInternetReachable + } + } } From 116d12ad7869b861d109c0f74c4573d52ac80ea0 Mon Sep 17 00:00:00 2001 From: Dan Tran Date: Wed, 8 May 2024 10:05:36 -0400 Subject: [PATCH 2/9] Move NetworkCapabilitiesUtil to a new util class and add unit test --- .../syncengine/NetworkCapabilitiesUtil.kt | 46 +++++++++++ .../syncengine/ReachabilityMonitor.kt | 77 ++++--------------- .../syncengine/NetworkCapabilitiesUtilTest.kt | 61 +++++++++++++++ .../syncengine/ReachabilityMonitorTest.kt | 6 ++ 4 files changed, 126 insertions(+), 64 deletions(-) create mode 100644 aws-datastore/src/main/java/com/amplifyframework/datastore/syncengine/NetworkCapabilitiesUtil.kt create mode 100644 aws-datastore/src/test/java/com/amplifyframework/datastore/syncengine/NetworkCapabilitiesUtilTest.kt diff --git a/aws-datastore/src/main/java/com/amplifyframework/datastore/syncengine/NetworkCapabilitiesUtil.kt b/aws-datastore/src/main/java/com/amplifyframework/datastore/syncengine/NetworkCapabilitiesUtil.kt new file mode 100644 index 0000000000..e8009655f6 --- /dev/null +++ b/aws-datastore/src/main/java/com/amplifyframework/datastore/syncengine/NetworkCapabilitiesUtil.kt @@ -0,0 +1,46 @@ +package com.amplifyframework.datastore.syncengine + +import android.net.ConnectivityManager +import android.net.NetworkCapabilities + +/** + * The NetworkCapabilitiesUtil provides convenient methods to check network capabilities and connection status + */ +object NetworkCapabilitiesUtil { + fun getNetworkCapabilities(connectivityManager: ConnectivityManager?): NetworkCapabilities? { + try { + return connectivityManager?.let { + it.getNetworkCapabilities(it.activeNetwork) + } + } catch (ignored: SecurityException) { + // Android 11 may throw a 'package does not belong' security exception here. + // Google fixed Android 14, 13 and 12 with the issue where Chaland Jean patched those versions. + // Android 11 is too old, so that's why we have to catch this exception here to be safe. + // https://android.googlesource.com/platform/frameworks/base/+/249be21013e389837f5b2beb7d36890b25ecfaaf%5E%21/ + // We need to catch this to prevent app crash. + } + return null + } + + fun isInternetReachable(capabilities: NetworkCapabilities?): Boolean { + if (capabilities == null) { + return false + } + + return when { + capabilities.hasTransport(NetworkCapabilities.TRANSPORT_CELLULAR) -> { + true + } + capabilities.hasTransport(NetworkCapabilities.TRANSPORT_WIFI) -> { + true + } + capabilities.hasTransport(NetworkCapabilities.TRANSPORT_ETHERNET) -> { + true + } + capabilities.hasTransport(NetworkCapabilities.TRANSPORT_VPN) -> { + capabilities.linkDownstreamBandwidthKbps != 0 + } + else -> false + } + } +} \ No newline at end of file diff --git a/aws-datastore/src/main/java/com/amplifyframework/datastore/syncengine/ReachabilityMonitor.kt b/aws-datastore/src/main/java/com/amplifyframework/datastore/syncengine/ReachabilityMonitor.kt index 4002e06ed9..ef599dc428 100644 --- a/aws-datastore/src/main/java/com/amplifyframework/datastore/syncengine/ReachabilityMonitor.kt +++ b/aws-datastore/src/main/java/com/amplifyframework/datastore/syncengine/ReachabilityMonitor.kt @@ -15,7 +15,6 @@ package com.amplifyframework.datastore.syncengine -import android.annotation.SuppressLint import android.content.Context import android.net.ConnectivityManager import android.net.ConnectivityManager.NetworkCallback @@ -25,13 +24,11 @@ import android.net.NetworkCapabilities import android.os.Build import androidx.annotation.VisibleForTesting import com.amplifyframework.datastore.DataStoreException -import io.reactivex.rxjava3.core.Completable import io.reactivex.rxjava3.core.Observable import io.reactivex.rxjava3.core.ObservableEmitter import io.reactivex.rxjava3.subjects.BehaviorSubject import java.util.concurrent.TimeUnit - /** * The ReachabilityMonitor is responsible for watching the network status as provided by the OS. * It returns an observable that publishes "true" when the network becomes available and "false" when @@ -94,11 +91,10 @@ private class ReachabilityMonitorImpl constructor(val schedulerProvider: Schedul private inner class ConnectivityNetworkCallback(private val emitter: ObservableEmitter) : NetworkCallback() { private var currentNetwork: Network? = null private var currentCapabilities: NetworkCapabilities? = null - private val DELAY_MS: Long = 250 override fun onAvailable(network: Network) { currentNetwork = network - asyncUpdateAndSend() + updateAndSend() } override fun onLosing(network: Network, maxMsToLive: Int) { @@ -125,24 +121,12 @@ private class ReachabilityMonitorImpl constructor(val schedulerProvider: Schedul } override fun onLinkPropertiesChanged(network: Network, linkProperties: LinkProperties) { - if (currentNetwork != null) { - currentNetwork = network - } - asyncUpdateAndSend() + currentNetwork = network + updateAndSend() } private fun updateAndSend() { - emitter.onNext(DefaultConnectivityProvider.isInternetReachable(currentCapabilities)) - } - - @SuppressLint("CheckResult") - private fun asyncUpdateAndSend() { - Completable.timer(DELAY_MS, TimeUnit.MILLISECONDS, schedulerProvider.computation()).subscribe { - currentCapabilities = DefaultConnectivityProvider.getNetworkCapabilities(connectivityProvider?.connectivityNetworkManager) - if (currentCapabilities != null) { - updateAndSend() - } - } + emitter.onNext(NetworkCapabilitiesUtil.isInternetReachable(currentCapabilities)) } } } @@ -154,20 +138,22 @@ private class ReachabilityMonitorImpl constructor(val schedulerProvider: Schedul */ interface ConnectivityProvider { val hasActiveNetwork: Boolean - val connectivityNetworkManager: ConnectivityManager? fun registerDefaultNetworkCallback(context: Context, callback: NetworkCallback) } private class DefaultConnectivityProvider : ConnectivityProvider { private var connectivityManager: ConnectivityManager? = null - override val connectivityNetworkManager: ConnectivityManager? - get() = connectivityManager - override val hasActiveNetwork: Boolean - get() = connectivityManager?.let { - val networkCapabilities: NetworkCapabilities? = getNetworkCapabilities(it) - isInternetReachable(networkCapabilities) + get() = connectivityManager?.let { manager -> + // the same logic as https://github.com/aws-amplify/amplify-android/blob/main/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/transfer/worker/BaseTransferWorker.kt#L176 + if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.Q) { + val networkCapabilities: NetworkCapabilities? = NetworkCapabilitiesUtil.getNetworkCapabilities(manager) + NetworkCapabilitiesUtil.isInternetReachable(networkCapabilities) + } else { + val activeNetworkInfo = manager.activeNetworkInfo + activeNetworkInfo != null && activeNetworkInfo.isConnected + } } ?: run { throw DataStoreException( "ReachabilityMonitor has not been configured.", @@ -185,41 +171,4 @@ private class DefaultConnectivityProvider : ConnectivityProvider { ) } } - - companion object { - fun getNetworkCapabilities(connectivityManager: ConnectivityManager?): NetworkCapabilities? { - try { - return connectivityManager?.let { - it.getNetworkCapabilities(it.activeNetwork) - } - } catch (ignored: SecurityException) { - // Android 11 may throw a 'package does not belong' security exception here. - // Google fixed Android 14, 13 and 12 with the issue where Chaland Jean patched those versions. - // Android 11 is too old, so that's why we have to catch this exception here to be safe. - // https://android.googlesource.com/platform/frameworks/base/+/249be21013e389837f5b2beb7d36890b25ecfaaf%5E%21/ - // We need to catch this to prevent app crash. - } - return null - } - - fun isInternetReachable(capabilities: NetworkCapabilities?): Boolean { - var isInternetReachable = false - if (capabilities != null) { - // Check to see if the network is temporarily unavailable or if airplane mode is toggled on - val isInternetSuspended = if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.P) { - !capabilities.hasCapability(NetworkCapabilities.NET_CAPABILITY_NOT_SUSPENDED) - } else { - // TODO: for SDK < 28, add extra check to evaluate airplane mode - false - } - isInternetReachable = (capabilities.hasCapability(NetworkCapabilities.NET_CAPABILITY_INTERNET) - && capabilities.hasCapability(NetworkCapabilities.NET_CAPABILITY_VALIDATED) - && !isInternetSuspended) - if (capabilities.hasTransport(NetworkCapabilities.TRANSPORT_VPN)) { - isInternetReachable = isInternetReachable && capabilities.linkDownstreamBandwidthKbps != 0 - } - } - return isInternetReachable - } - } } diff --git a/aws-datastore/src/test/java/com/amplifyframework/datastore/syncengine/NetworkCapabilitiesUtilTest.kt b/aws-datastore/src/test/java/com/amplifyframework/datastore/syncengine/NetworkCapabilitiesUtilTest.kt new file mode 100644 index 0000000000..b252909fcd --- /dev/null +++ b/aws-datastore/src/test/java/com/amplifyframework/datastore/syncengine/NetworkCapabilitiesUtilTest.kt @@ -0,0 +1,61 @@ +package com.amplifyframework.datastore.syncengine + +import android.net.ConnectivityManager +import android.net.NetworkCapabilities +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNull +import org.junit.Assert.assertTrue +import org.junit.Assert.assertFalse +import org.junit.Test +import org.mockito.Mockito + +class NetworkCapabilitiesUtilTest { + + @Test + fun testGetNetworkCapabilitiesSecurityException() { + val mockConnectivityManager = Mockito.mock(ConnectivityManager::class.java) + Mockito.`when`(mockConnectivityManager.activeNetwork).thenThrow(SecurityException()) + + val networkCapabilities = NetworkCapabilitiesUtil.getNetworkCapabilities(mockConnectivityManager) + assertNull(networkCapabilities) + } + + @Test + fun testGetNetworkCapabilities() { + val mockConnectivityManager = Mockito.mock(ConnectivityManager::class.java) + val expectedNetworkCapabilities = Mockito.mock(NetworkCapabilities::class.java) + Mockito.`when`(mockConnectivityManager.getNetworkCapabilities(mockConnectivityManager.activeNetwork)) + .thenReturn(expectedNetworkCapabilities) + + val networkCapabilities = NetworkCapabilitiesUtil.getNetworkCapabilities(mockConnectivityManager) + + assertEquals(expectedNetworkCapabilities, networkCapabilities) + } + + @Test + fun testIsInternetReachable() { + val networkCapabilitiesWithCellular = Mockito.mock(NetworkCapabilities::class.java) + Mockito.`when`(networkCapabilitiesWithCellular.hasTransport(NetworkCapabilities.TRANSPORT_CELLULAR)).thenReturn(true) + + val networkCapabilitiesWithWifi = Mockito.mock(NetworkCapabilities::class.java) + Mockito.`when`(networkCapabilitiesWithWifi.hasTransport(NetworkCapabilities.TRANSPORT_WIFI)).thenReturn(true) + + val networkCapabilitiesWithEthernet = Mockito.mock(NetworkCapabilities::class.java) + Mockito.`when`(networkCapabilitiesWithEthernet.hasTransport(NetworkCapabilities.TRANSPORT_ETHERNET)).thenReturn(true) + + val networkCapabilitiesWithVpnAndBandwidth = Mockito.mock(NetworkCapabilities::class.java) + Mockito.`when`(networkCapabilitiesWithVpnAndBandwidth.hasTransport(NetworkCapabilities.TRANSPORT_VPN)).thenReturn(true) + Mockito.`when`(networkCapabilitiesWithVpnAndBandwidth.linkDownstreamBandwidthKbps).thenReturn(1000) + + val networkCapabilitiesWithVpnNoBandwidth = Mockito.mock(NetworkCapabilities::class.java) + Mockito.`when`(networkCapabilitiesWithVpnNoBandwidth.hasTransport(NetworkCapabilities.TRANSPORT_VPN)).thenReturn(true) + Mockito.`when`(networkCapabilitiesWithVpnNoBandwidth.linkDownstreamBandwidthKbps).thenReturn(0) + + assertTrue(NetworkCapabilitiesUtil.isInternetReachable(networkCapabilitiesWithCellular)) + assertTrue(NetworkCapabilitiesUtil.isInternetReachable(networkCapabilitiesWithWifi)) + assertTrue(NetworkCapabilitiesUtil.isInternetReachable(networkCapabilitiesWithEthernet)) + assertTrue(NetworkCapabilitiesUtil.isInternetReachable(networkCapabilitiesWithVpnAndBandwidth)) + assertFalse(NetworkCapabilitiesUtil.isInternetReachable(networkCapabilitiesWithVpnNoBandwidth)) + assertFalse(NetworkCapabilitiesUtil.isInternetReachable(null)) + } +} \ No newline at end of file diff --git a/aws-datastore/src/test/java/com/amplifyframework/datastore/syncengine/ReachabilityMonitorTest.kt b/aws-datastore/src/test/java/com/amplifyframework/datastore/syncengine/ReachabilityMonitorTest.kt index cd078f0dc7..e74fce2649 100644 --- a/aws-datastore/src/test/java/com/amplifyframework/datastore/syncengine/ReachabilityMonitorTest.kt +++ b/aws-datastore/src/test/java/com/amplifyframework/datastore/syncengine/ReachabilityMonitorTest.kt @@ -18,6 +18,7 @@ package com.amplifyframework.datastore.syncengine import android.content.Context import android.net.ConnectivityManager import android.net.Network +import android.net.NetworkCapabilities import com.amplifyframework.datastore.DataStoreException import io.reactivex.rxjava3.core.BackpressureStrategy import io.reactivex.rxjava3.schedulers.TestScheduler @@ -26,6 +27,7 @@ import java.util.concurrent.TimeUnit import org.junit.Assert.assertEquals import org.junit.Assert.assertTrue import org.junit.Test +import org.mockito.Mockito import org.mockito.Mockito.mock class ReachabilityMonitorTest { @@ -70,6 +72,10 @@ class ReachabilityMonitorTest { .subscribe(testSubscriber) val network = mock(Network::class.java) + val networkCapabilities = mock(NetworkCapabilities::class.java) + Mockito.`when`(networkCapabilities.hasCapability(NetworkCapabilities.TRANSPORT_WIFI)) + .thenReturn(true) + // Should provide initial network state (true) upon subscription (after debounce) testScheduler.advanceTimeBy(251, TimeUnit.MILLISECONDS) callback!!.onAvailable(network) From 49817afb91dcef505ff5bcaab820cea7bdc7ae8c Mon Sep 17 00:00:00 2001 From: Dan Tran Date: Wed, 8 May 2024 10:05:36 -0400 Subject: [PATCH 3/9] Move NetworkCapabilitiesUtil to a new util class and add unit test --- .../syncengine/ReachabilityMonitorTest.kt | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/aws-datastore/src/test/java/com/amplifyframework/datastore/syncengine/ReachabilityMonitorTest.kt b/aws-datastore/src/test/java/com/amplifyframework/datastore/syncengine/ReachabilityMonitorTest.kt index e74fce2649..f9d1141150 100644 --- a/aws-datastore/src/test/java/com/amplifyframework/datastore/syncengine/ReachabilityMonitorTest.kt +++ b/aws-datastore/src/test/java/com/amplifyframework/datastore/syncengine/ReachabilityMonitorTest.kt @@ -29,6 +29,7 @@ import org.junit.Assert.assertTrue import org.junit.Test import org.mockito.Mockito import org.mockito.Mockito.mock +import org.mockito.Mockito.`when` class ReachabilityMonitorTest { @@ -73,20 +74,28 @@ class ReachabilityMonitorTest { val network = mock(Network::class.java) val networkCapabilities = mock(NetworkCapabilities::class.java) +<<<<<<< HEAD Mockito.`when`(networkCapabilities.hasCapability(NetworkCapabilities.TRANSPORT_WIFI)) +======= + `when`(networkCapabilities.hasTransport(NetworkCapabilities.TRANSPORT_CELLULAR)) +>>>>>>> ad4c66a8 (Move NetworkCapabilitiesUtil to a new util class and add unit test) .thenReturn(true) // Should provide initial network state (true) upon subscription (after debounce) testScheduler.advanceTimeBy(251, TimeUnit.MILLISECONDS) callback!!.onAvailable(network) + callback!!.onCapabilitiesChanged(network, networkCapabilities) callback!!.onAvailable(network) + callback!!.onCapabilitiesChanged(network, networkCapabilities) callback!!.onLost(network) // Should provide false after debounce testScheduler.advanceTimeBy(251, TimeUnit.MILLISECONDS) callback!!.onAvailable(network) + callback!!.onCapabilitiesChanged(network, networkCapabilities) // Should provide true after debounce testScheduler.advanceTimeBy(251, TimeUnit.MILLISECONDS) callback!!.onAvailable(network) + callback!!.onCapabilitiesChanged(network, networkCapabilities) // Should provide true after debounce testScheduler.advanceTimeBy(251, TimeUnit.MILLISECONDS) @@ -128,9 +137,13 @@ class ReachabilityMonitorTest { .subscribe(testSubscriber) val network = mock(Network::class.java) + val networkCapabilities = mock(NetworkCapabilities::class.java) + `when`(networkCapabilities.hasTransport(NetworkCapabilities.TRANSPORT_CELLULAR)) + .thenReturn(true) // Assert that the first value is returned callback!!.onAvailable(network) + callback!!.onCapabilitiesChanged(network, networkCapabilities) testScheduler.advanceTimeBy(251, TimeUnit.MILLISECONDS) var result1: Boolean? = null val disposable1 = reachabilityMonitor.getObservable().subscribeOn(testScheduler).subscribe { result1 = it } @@ -152,10 +165,12 @@ class ReachabilityMonitorTest { // Assert that if debouncer keeps getting restarted, value doesn't change callback!!.onAvailable(network) + callback!!.onCapabilitiesChanged(network, networkCapabilities) testScheduler.advanceTimeBy(100, TimeUnit.MILLISECONDS) callback!!.onLost(network) testScheduler.advanceTimeBy(100, TimeUnit.MILLISECONDS) callback!!.onAvailable(network) + callback!!.onCapabilitiesChanged(network, networkCapabilities) testScheduler.advanceTimeBy(100, TimeUnit.MILLISECONDS) var result4: Boolean? = null From dbe6727b295f528e792597c19027b18ba5f57b57 Mon Sep 17 00:00:00 2001 From: Dan Tran Date: Wed, 19 Jun 2024 10:02:45 -0400 Subject: [PATCH 4/9] fix conflict --- .../datastore/syncengine/ReachabilityMonitor.kt | 10 ---------- .../datastore/syncengine/ReachabilityMonitorTest.kt | 5 ----- 2 files changed, 15 deletions(-) diff --git a/aws-datastore/src/main/java/com/amplifyframework/datastore/syncengine/ReachabilityMonitor.kt b/aws-datastore/src/main/java/com/amplifyframework/datastore/syncengine/ReachabilityMonitor.kt index ef599dc428..fa16a53ea7 100644 --- a/aws-datastore/src/main/java/com/amplifyframework/datastore/syncengine/ReachabilityMonitor.kt +++ b/aws-datastore/src/main/java/com/amplifyframework/datastore/syncengine/ReachabilityMonitor.kt @@ -97,11 +97,6 @@ private class ReachabilityMonitorImpl constructor(val schedulerProvider: Schedul updateAndSend() } - override fun onLosing(network: Network, maxMsToLive: Int) { - currentNetwork = network - updateAndSend() - } - override fun onLost(network: Network) { currentNetwork = null currentCapabilities = null @@ -120,11 +115,6 @@ private class ReachabilityMonitorImpl constructor(val schedulerProvider: Schedul updateAndSend() } - override fun onLinkPropertiesChanged(network: Network, linkProperties: LinkProperties) { - currentNetwork = network - updateAndSend() - } - private fun updateAndSend() { emitter.onNext(NetworkCapabilitiesUtil.isInternetReachable(currentCapabilities)) } diff --git a/aws-datastore/src/test/java/com/amplifyframework/datastore/syncengine/ReachabilityMonitorTest.kt b/aws-datastore/src/test/java/com/amplifyframework/datastore/syncengine/ReachabilityMonitorTest.kt index f9d1141150..fa9a6fd745 100644 --- a/aws-datastore/src/test/java/com/amplifyframework/datastore/syncengine/ReachabilityMonitorTest.kt +++ b/aws-datastore/src/test/java/com/amplifyframework/datastore/syncengine/ReachabilityMonitorTest.kt @@ -27,7 +27,6 @@ import java.util.concurrent.TimeUnit import org.junit.Assert.assertEquals import org.junit.Assert.assertTrue import org.junit.Test -import org.mockito.Mockito import org.mockito.Mockito.mock import org.mockito.Mockito.`when` @@ -74,11 +73,7 @@ class ReachabilityMonitorTest { val network = mock(Network::class.java) val networkCapabilities = mock(NetworkCapabilities::class.java) -<<<<<<< HEAD - Mockito.`when`(networkCapabilities.hasCapability(NetworkCapabilities.TRANSPORT_WIFI)) -======= `when`(networkCapabilities.hasTransport(NetworkCapabilities.TRANSPORT_CELLULAR)) ->>>>>>> ad4c66a8 (Move NetworkCapabilitiesUtil to a new util class and add unit test) .thenReturn(true) // Should provide initial network state (true) upon subscription (after debounce) From 1e153dddf4291c77eaf3c1184f1ae0d39b1eb8f3 Mon Sep 17 00:00:00 2001 From: Dan Date: Tue, 13 Aug 2024 07:48:39 -0700 Subject: [PATCH 5/9] Update followed comment aws-datastore/src/main/java/com/amplifyframework/datastore/syncengine/NetworkCapabilitiesUtil.kt Co-authored-by: Matt Creaser --- .../syncengine/NetworkCapabilitiesUtil.kt | 53 ++++++------------- 1 file changed, 17 insertions(+), 36 deletions(-) diff --git a/aws-datastore/src/main/java/com/amplifyframework/datastore/syncengine/NetworkCapabilitiesUtil.kt b/aws-datastore/src/main/java/com/amplifyframework/datastore/syncengine/NetworkCapabilitiesUtil.kt index e8009655f6..b12d1d1079 100644 --- a/aws-datastore/src/main/java/com/amplifyframework/datastore/syncengine/NetworkCapabilitiesUtil.kt +++ b/aws-datastore/src/main/java/com/amplifyframework/datastore/syncengine/NetworkCapabilitiesUtil.kt @@ -6,41 +6,22 @@ import android.net.NetworkCapabilities /** * The NetworkCapabilitiesUtil provides convenient methods to check network capabilities and connection status */ -object NetworkCapabilitiesUtil { - fun getNetworkCapabilities(connectivityManager: ConnectivityManager?): NetworkCapabilities? { - try { - return connectivityManager?.let { - it.getNetworkCapabilities(it.activeNetwork) - } - } catch (ignored: SecurityException) { - // Android 11 may throw a 'package does not belong' security exception here. - // Google fixed Android 14, 13 and 12 with the issue where Chaland Jean patched those versions. - // Android 11 is too old, so that's why we have to catch this exception here to be safe. - // https://android.googlesource.com/platform/frameworks/base/+/249be21013e389837f5b2beb7d36890b25ecfaaf%5E%21/ - // We need to catch this to prevent app crash. - } - return null - } +fun ConnectivityManager.networkCapabilitiesOrNull() = try { + getNetworkCapabilities(activeNetwork) +} catch (ignored: SecurityException) { + // Android 11 may throw a 'package does not belong' security exception here. + // Google fixed Android 14, 13 and 12 with the issue where Chaland Jean patched those versions. + // Android 11 is too old, so that's why we have to catch this exception here to be safe. + // https://android.googlesource.com/platform/frameworks/base/+/249be21013e389837f5b2beb7d36890b25ecfaaf%5E%21/ + // We need to catch this to prevent app crash. + null +} - fun isInternetReachable(capabilities: NetworkCapabilities?): Boolean { - if (capabilities == null) { - return false - } - - return when { - capabilities.hasTransport(NetworkCapabilities.TRANSPORT_CELLULAR) -> { - true - } - capabilities.hasTransport(NetworkCapabilities.TRANSPORT_WIFI) -> { - true - } - capabilities.hasTransport(NetworkCapabilities.TRANSPORT_ETHERNET) -> { - true - } - capabilities.hasTransport(NetworkCapabilities.TRANSPORT_VPN) -> { - capabilities.linkDownstreamBandwidthKbps != 0 - } - else -> false - } - } +fun NetworkCapabilities?.isInternetReachable() = when { + this == null -> false + hasTransport(NetworkCapabilities.TRANSPORT_CELLULAR) -> true + hasTransport(NetworkCapabilities.TRANSPORT_WIFI) -> true + hasTransport(NetworkCapabilities.TRANSPORT_ETHERNET) -> true + hasTransport(NetworkCapabilities.TRANSPORT_VPN) -> linkDownstreamBandwidthKbps != 0 + else -> false } \ No newline at end of file From e69a7d3590ea8e5705595b543509e530e470f5c6 Mon Sep 17 00:00:00 2001 From: Dan Tran Date: Tue, 13 Aug 2024 08:11:42 -0700 Subject: [PATCH 6/9] fix: add new NetworkExtensions follow suggestions --- .../datastore/extensions/NetworkExtensions.kt | 49 +++++++++++++++++++ .../syncengine/NetworkCapabilitiesUtil.kt | 27 ---------- .../syncengine/ReachabilityMonitor.kt | 35 +++++-------- 3 files changed, 60 insertions(+), 51 deletions(-) create mode 100644 aws-datastore/src/main/java/com/amplifyframework/datastore/extensions/NetworkExtensions.kt delete mode 100644 aws-datastore/src/main/java/com/amplifyframework/datastore/syncengine/NetworkCapabilitiesUtil.kt diff --git a/aws-datastore/src/main/java/com/amplifyframework/datastore/extensions/NetworkExtensions.kt b/aws-datastore/src/main/java/com/amplifyframework/datastore/extensions/NetworkExtensions.kt new file mode 100644 index 0000000000..147b7aa61a --- /dev/null +++ b/aws-datastore/src/main/java/com/amplifyframework/datastore/extensions/NetworkExtensions.kt @@ -0,0 +1,49 @@ +/* + * Copyright 2024 Amazon.com, Inc. or its affiliates. All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"). + * You may not use this file except in compliance with the License. + * A copy of the License is located at + * + * http://aws.amazon.com/apache2.0 + * + * or in the "license" file accompanying this file. This file is distributed + * on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either + * express or implied. See the License for the specific language governing + * permissions and limitations under the License. + */ +package com.amplifyframework.datastore.extensions + +import android.net.ConnectivityManager +import android.net.NetworkCapabilities +import android.os.Build + +// Return network capabilities based on Connectivity Manager +private fun ConnectivityManager.networkCapabilitiesOrNull() = try { + getNetworkCapabilities(activeNetwork) +} catch (ignored: SecurityException) { + // Android 11 may throw a 'package does not belong' security exception here. + // Google fixed Android 14, 13 and 12 with the issue where Chaland Jean patched those versions. + // Android 11 is too old, so that's why we have to catch this exception here to be safe. + // https://android.googlesource.com/platform/frameworks/base/+/249be21013e389837f5b2beb7d36890b25ecfaaf%5E%21/ + // We need to catch this to prevent app crash. + null +} + +// Return whether internet is reachable based on network capabilities. +fun NetworkCapabilities?.isInternetReachable() = when { + this == null -> false + hasTransport(NetworkCapabilities.TRANSPORT_CELLULAR) -> true + hasTransport(NetworkCapabilities.TRANSPORT_WIFI) -> true + hasTransport(NetworkCapabilities.TRANSPORT_ETHERNET) -> true + else -> false +} + +// Check whether network is available based on connectivity manager +// the same logic as https://github.com/aws-amplify/amplify-android/blob/main/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/transfer/worker/BaseTransferWorker.kt#L176 +fun ConnectivityManager.isNetworkAvailable(): Boolean = + if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.Q) { + networkCapabilitiesOrNull()?.isInternetReachable() ?: false + } else { + activeNetworkInfo?.isConnected ?: false + } diff --git a/aws-datastore/src/main/java/com/amplifyframework/datastore/syncengine/NetworkCapabilitiesUtil.kt b/aws-datastore/src/main/java/com/amplifyframework/datastore/syncengine/NetworkCapabilitiesUtil.kt deleted file mode 100644 index b12d1d1079..0000000000 --- a/aws-datastore/src/main/java/com/amplifyframework/datastore/syncengine/NetworkCapabilitiesUtil.kt +++ /dev/null @@ -1,27 +0,0 @@ -package com.amplifyframework.datastore.syncengine - -import android.net.ConnectivityManager -import android.net.NetworkCapabilities - -/** - * The NetworkCapabilitiesUtil provides convenient methods to check network capabilities and connection status - */ -fun ConnectivityManager.networkCapabilitiesOrNull() = try { - getNetworkCapabilities(activeNetwork) -} catch (ignored: SecurityException) { - // Android 11 may throw a 'package does not belong' security exception here. - // Google fixed Android 14, 13 and 12 with the issue where Chaland Jean patched those versions. - // Android 11 is too old, so that's why we have to catch this exception here to be safe. - // https://android.googlesource.com/platform/frameworks/base/+/249be21013e389837f5b2beb7d36890b25ecfaaf%5E%21/ - // We need to catch this to prevent app crash. - null -} - -fun NetworkCapabilities?.isInternetReachable() = when { - this == null -> false - hasTransport(NetworkCapabilities.TRANSPORT_CELLULAR) -> true - hasTransport(NetworkCapabilities.TRANSPORT_WIFI) -> true - hasTransport(NetworkCapabilities.TRANSPORT_ETHERNET) -> true - hasTransport(NetworkCapabilities.TRANSPORT_VPN) -> linkDownstreamBandwidthKbps != 0 - else -> false -} \ No newline at end of file diff --git a/aws-datastore/src/main/java/com/amplifyframework/datastore/syncengine/ReachabilityMonitor.kt b/aws-datastore/src/main/java/com/amplifyframework/datastore/syncengine/ReachabilityMonitor.kt index fa16a53ea7..68f10345db 100644 --- a/aws-datastore/src/main/java/com/amplifyframework/datastore/syncengine/ReachabilityMonitor.kt +++ b/aws-datastore/src/main/java/com/amplifyframework/datastore/syncengine/ReachabilityMonitor.kt @@ -18,12 +18,12 @@ package com.amplifyframework.datastore.syncengine import android.content.Context import android.net.ConnectivityManager import android.net.ConnectivityManager.NetworkCallback -import android.net.LinkProperties import android.net.Network import android.net.NetworkCapabilities -import android.os.Build import androidx.annotation.VisibleForTesting import com.amplifyframework.datastore.DataStoreException +import com.amplifyframework.datastore.extensions.isInternetReachable +import com.amplifyframework.datastore.extensions.isNetworkAvailable import io.reactivex.rxjava3.core.Observable import io.reactivex.rxjava3.core.ObservableEmitter import io.reactivex.rxjava3.subjects.BehaviorSubject @@ -116,7 +116,7 @@ private class ReachabilityMonitorImpl constructor(val schedulerProvider: Schedul } private fun updateAndSend() { - emitter.onNext(NetworkCapabilitiesUtil.isInternetReachable(currentCapabilities)) + emitter.onNext(currentCapabilities.isInternetReachable()) } } } @@ -135,30 +135,17 @@ private class DefaultConnectivityProvider : ConnectivityProvider { private var connectivityManager: ConnectivityManager? = null override val hasActiveNetwork: Boolean - get() = connectivityManager?.let { manager -> - // the same logic as https://github.com/aws-amplify/amplify-android/blob/main/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/transfer/worker/BaseTransferWorker.kt#L176 - if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.Q) { - val networkCapabilities: NetworkCapabilities? = NetworkCapabilitiesUtil.getNetworkCapabilities(manager) - NetworkCapabilitiesUtil.isInternetReachable(networkCapabilities) - } else { - val activeNetworkInfo = manager.activeNetworkInfo - activeNetworkInfo != null && activeNetworkInfo.isConnected - } - } ?: run { - throw DataStoreException( - "ReachabilityMonitor has not been configured.", - "Call ReachabilityMonitor.configure() before calling ReachabilityMonitor.getObservable()" - ) - } + get() = connectivityManager?.isNetworkAvailable() ?: throw DataStoreException( + "ReachabilityMonitor has not been configured.", + "Call ReachabilityMonitor.configure() before calling ReachabilityMonitor.getObservable()" + ) override fun registerDefaultNetworkCallback(context: Context, callback: NetworkCallback) { connectivityManager = context.getSystemService(ConnectivityManager::class.java) connectivityManager?.registerDefaultNetworkCallback(callback) - ?: run { - throw DataStoreException( - "ConnectivityManager not available", - "No recovery suggestion is available" - ) - } + ?: throw DataStoreException( + "ConnectivityManager not available", + "No recovery suggestion is available" + ) } } From ff110de16214ce3cf93441474eee7c3360cf24bb Mon Sep 17 00:00:00 2001 From: Dan Tran Date: Sun, 11 Aug 2024 14:37:46 -0400 Subject: [PATCH 7/9] Remove unused currentNetwork and change unit test to use mockk --- .../syncengine/ReachabilityMonitor.kt | 15 ++++++----- .../syncengine/ReachabilityMonitorTest.kt | 26 +++++++++---------- 2 files changed, 21 insertions(+), 20 deletions(-) diff --git a/aws-datastore/src/main/java/com/amplifyframework/datastore/syncengine/ReachabilityMonitor.kt b/aws-datastore/src/main/java/com/amplifyframework/datastore/syncengine/ReachabilityMonitor.kt index 68f10345db..3b3ae7d8ec 100644 --- a/aws-datastore/src/main/java/com/amplifyframework/datastore/syncengine/ReachabilityMonitor.kt +++ b/aws-datastore/src/main/java/com/amplifyframework/datastore/syncengine/ReachabilityMonitor.kt @@ -89,28 +89,31 @@ private class ReachabilityMonitorImpl constructor(val schedulerProvider: Schedul } private inner class ConnectivityNetworkCallback(private val emitter: ObservableEmitter) : NetworkCallback() { - private var currentNetwork: Network? = null private var currentCapabilities: NetworkCapabilities? = null override fun onAvailable(network: Network) { - currentNetwork = network - updateAndSend() + // The order of callbacks between onAvailable and onCapabilitiesChanged can vary depending on Android SDK level + // To ensure the network capabilities are not reported incorrectly when there is insufficient information: + // - On Android 5 (API 21) to Android 12 (API 31), onAvailable() is called before onCapabilitiesChanged() + // - On Android 13 and above, onCapabilitiesChanged() may be triggered first + // Therefore, we need to check if currentCapabilities is null to avoid sending false values when the + // necessary information is not yet available + if (currentCapabilities !== null) { + updateAndSend() + } } override fun onLost(network: Network) { - currentNetwork = null currentCapabilities = null updateAndSend() } override fun onUnavailable() { - currentNetwork = null currentCapabilities = null updateAndSend() } override fun onCapabilitiesChanged(network: Network, networkCapabilities: NetworkCapabilities) { - currentNetwork = network currentCapabilities = networkCapabilities updateAndSend() } diff --git a/aws-datastore/src/test/java/com/amplifyframework/datastore/syncengine/ReachabilityMonitorTest.kt b/aws-datastore/src/test/java/com/amplifyframework/datastore/syncengine/ReachabilityMonitorTest.kt index fa9a6fd745..003f273a8b 100644 --- a/aws-datastore/src/test/java/com/amplifyframework/datastore/syncengine/ReachabilityMonitorTest.kt +++ b/aws-datastore/src/test/java/com/amplifyframework/datastore/syncengine/ReachabilityMonitorTest.kt @@ -20,6 +20,8 @@ import android.net.ConnectivityManager import android.net.Network import android.net.NetworkCapabilities import com.amplifyframework.datastore.DataStoreException +import io.mockk.every +import io.mockk.mockk import io.reactivex.rxjava3.core.BackpressureStrategy import io.reactivex.rxjava3.schedulers.TestScheduler import io.reactivex.rxjava3.subscribers.TestSubscriber @@ -27,8 +29,6 @@ import java.util.concurrent.TimeUnit import org.junit.Assert.assertEquals import org.junit.Assert.assertTrue import org.junit.Test -import org.mockito.Mockito.mock -import org.mockito.Mockito.`when` class ReachabilityMonitorTest { @@ -58,7 +58,7 @@ class ReachabilityMonitorTest { } } - val mockContext = mock(Context::class.java) + val mockContext = mockk(relaxed = true) // TestScheduler allows the virtual time to be advanced by exact amounts, to allow for repeatable tests val testScheduler = TestScheduler() val reachabilityMonitor = ReachabilityMonitor.createForTesting(TestSchedulerProvider(testScheduler)) @@ -71,10 +71,9 @@ class ReachabilityMonitorTest { .toFlowable(BackpressureStrategy.BUFFER) .subscribe(testSubscriber) - val network = mock(Network::class.java) - val networkCapabilities = mock(NetworkCapabilities::class.java) - `when`(networkCapabilities.hasTransport(NetworkCapabilities.TRANSPORT_CELLULAR)) - .thenReturn(true) + val network = mockk(relaxed = true) + val networkCapabilities = mockk() + every { networkCapabilities.hasTransport(NetworkCapabilities.TRANSPORT_CELLULAR) } returns true // Should provide initial network state (true) upon subscription (after debounce) testScheduler.advanceTimeBy(251, TimeUnit.MILLISECONDS) @@ -118,7 +117,7 @@ class ReachabilityMonitorTest { } } - val mockContext = mock(Context::class.java) + val mockContext = mockk(relaxed = true) // TestScheduler allows the virtual time to be advanced by exact amounts, to allow for repeatable tests val testScheduler = TestScheduler() val reachabilityMonitor = ReachabilityMonitor.createForTesting(TestSchedulerProvider(testScheduler)) @@ -131,10 +130,9 @@ class ReachabilityMonitorTest { .toFlowable(BackpressureStrategy.BUFFER) .subscribe(testSubscriber) - val network = mock(Network::class.java) - val networkCapabilities = mock(NetworkCapabilities::class.java) - `when`(networkCapabilities.hasTransport(NetworkCapabilities.TRANSPORT_CELLULAR)) - .thenReturn(true) + val network = mockk(relaxed = true) + val networkCapabilities = mockk() + every { networkCapabilities.hasTransport(NetworkCapabilities.TRANSPORT_CELLULAR) } returns true // Assert that the first value is returned callback!!.onAvailable(network) @@ -213,11 +211,11 @@ class ReachabilityMonitorTest { // TestScheduler allows the virtual time to be advanced by exact amounts, to allow for repeatable tests val testScheduler = TestScheduler() val reachabilityMonitor = ReachabilityMonitor.createForTesting(TestSchedulerProvider(testScheduler)) - val mockContext = mock(Context::class.java) + val mockContext = mockk(relaxed = true) reachabilityMonitor.configure(mockContext, connectivityProvider) reachabilityMonitor.getObservable().subscribe() - val network = mock(Network::class.java) + val network = mockk(relaxed = true) // Should provide initial network state (true) upon subscription (after debounce) testScheduler.advanceTimeBy(251, TimeUnit.MILLISECONDS) networkCallback!!.onAvailable(network) From 4d7a27e9d156ef5ae9010ac7be50a58a07fb1641 Mon Sep 17 00:00:00 2001 From: Dan Tran Date: Tue, 13 Aug 2024 08:30:39 -0700 Subject: [PATCH 8/9] fix: update unit test with mockk --- .../datastore/extensions/NetworkExtensions.kt | 4 +- .../syncengine/ReachabilityMonitor.kt | 25 ++------ .../extensions/NetworkExtensionsTest.kt | 62 +++++++++++++++++++ .../syncengine/NetworkCapabilitiesUtilTest.kt | 61 ------------------ 4 files changed, 69 insertions(+), 83 deletions(-) create mode 100644 aws-datastore/src/test/java/com/amplifyframework/datastore/extensions/NetworkExtensionsTest.kt delete mode 100644 aws-datastore/src/test/java/com/amplifyframework/datastore/syncengine/NetworkCapabilitiesUtilTest.kt diff --git a/aws-datastore/src/main/java/com/amplifyframework/datastore/extensions/NetworkExtensions.kt b/aws-datastore/src/main/java/com/amplifyframework/datastore/extensions/NetworkExtensions.kt index 147b7aa61a..6bf3df53ae 100644 --- a/aws-datastore/src/main/java/com/amplifyframework/datastore/extensions/NetworkExtensions.kt +++ b/aws-datastore/src/main/java/com/amplifyframework/datastore/extensions/NetworkExtensions.kt @@ -17,9 +17,11 @@ package com.amplifyframework.datastore.extensions import android.net.ConnectivityManager import android.net.NetworkCapabilities import android.os.Build +import androidx.annotation.VisibleForTesting // Return network capabilities based on Connectivity Manager -private fun ConnectivityManager.networkCapabilitiesOrNull() = try { +@VisibleForTesting(otherwise = VisibleForTesting.PRIVATE) +fun ConnectivityManager.networkCapabilitiesOrNull() = try { getNetworkCapabilities(activeNetwork) } catch (ignored: SecurityException) { // Android 11 may throw a 'package does not belong' security exception here. diff --git a/aws-datastore/src/main/java/com/amplifyframework/datastore/syncengine/ReachabilityMonitor.kt b/aws-datastore/src/main/java/com/amplifyframework/datastore/syncengine/ReachabilityMonitor.kt index 3b3ae7d8ec..f4c82a8e78 100644 --- a/aws-datastore/src/main/java/com/amplifyframework/datastore/syncengine/ReachabilityMonitor.kt +++ b/aws-datastore/src/main/java/com/amplifyframework/datastore/syncengine/ReachabilityMonitor.kt @@ -89,37 +89,20 @@ private class ReachabilityMonitorImpl constructor(val schedulerProvider: Schedul } private inner class ConnectivityNetworkCallback(private val emitter: ObservableEmitter) : NetworkCallback() { - private var currentCapabilities: NetworkCapabilities? = null - override fun onAvailable(network: Network) { - // The order of callbacks between onAvailable and onCapabilitiesChanged can vary depending on Android SDK level - // To ensure the network capabilities are not reported incorrectly when there is insufficient information: - // - On Android 5 (API 21) to Android 12 (API 31), onAvailable() is called before onCapabilitiesChanged() - // - On Android 13 and above, onCapabilitiesChanged() may be triggered first - // Therefore, we need to check if currentCapabilities is null to avoid sending false values when the - // necessary information is not yet available - if (currentCapabilities !== null) { - updateAndSend() - } + connectivityProvider?.let { emitter.onNext(it.hasActiveNetwork) } } override fun onLost(network: Network) { - currentCapabilities = null - updateAndSend() + emitter.onNext(false) } override fun onUnavailable() { - currentCapabilities = null - updateAndSend() + emitter.onNext(false) } override fun onCapabilitiesChanged(network: Network, networkCapabilities: NetworkCapabilities) { - currentCapabilities = networkCapabilities - updateAndSend() - } - - private fun updateAndSend() { - emitter.onNext(currentCapabilities.isInternetReachable()) + emitter.onNext(networkCapabilities.isInternetReachable()) } } } diff --git a/aws-datastore/src/test/java/com/amplifyframework/datastore/extensions/NetworkExtensionsTest.kt b/aws-datastore/src/test/java/com/amplifyframework/datastore/extensions/NetworkExtensionsTest.kt new file mode 100644 index 0000000000..f68902f470 --- /dev/null +++ b/aws-datastore/src/test/java/com/amplifyframework/datastore/extensions/NetworkExtensionsTest.kt @@ -0,0 +1,62 @@ +package com.amplifyframework.datastore.extensions + +import android.net.ConnectivityManager +import android.net.NetworkCapabilities +import io.mockk.every +import io.mockk.mockk +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNull +import org.junit.Assert.assertTrue +import org.junit.Assert.assertFalse +import org.junit.Test + +class NetworkCapabilitiesUtilTest { + + @Test + fun testGetNetworkCapabilitiesSecurityException() { + val mockConnectivityManager = mockk { + every { activeNetwork } throws SecurityException() + } + + val networkCapabilities = mockConnectivityManager.networkCapabilitiesOrNull() + assertNull(networkCapabilities) + } + + @Test + fun testGetNetworkCapabilities() { + val expectedNetworkCapabilities = mockk() + val mockConnectivityManager = mockk { + every { getNetworkCapabilities(any()) } returns expectedNetworkCapabilities + } + + val networkCapabilities = mockConnectivityManager.networkCapabilitiesOrNull() + + assertEquals(expectedNetworkCapabilities, networkCapabilities) + } + + @Test + fun testIsInternetReachable() { + val networkCapabilitiesWithCellular = mockk { + every { hasTransport(NetworkCapabilities.TRANSPORT_WIFI) } returns false + every { hasTransport(NetworkCapabilities.TRANSPORT_CELLULAR) } returns true + every { hasTransport(NetworkCapabilities.TRANSPORT_ETHERNET) } returns false + } + + val networkCapabilitiesWithWifi = mockk { + every { hasTransport(NetworkCapabilities.TRANSPORT_WIFI) } returns true + every { hasTransport(NetworkCapabilities.TRANSPORT_CELLULAR) } returns false + every { hasTransport(NetworkCapabilities.TRANSPORT_ETHERNET) } returns false + } + + val networkCapabilitiesWithEthernet = mockk { + every { hasTransport(NetworkCapabilities.TRANSPORT_WIFI) } returns false + every { hasTransport(NetworkCapabilities.TRANSPORT_CELLULAR) } returns false + every { hasTransport(NetworkCapabilities.TRANSPORT_ETHERNET) } returns true + } + + assertTrue(networkCapabilitiesWithCellular.isInternetReachable()) + assertTrue(networkCapabilitiesWithWifi.isInternetReachable()) + assertTrue(networkCapabilitiesWithEthernet.isInternetReachable()) + assertFalse(null.isInternetReachable()) + } +} \ No newline at end of file diff --git a/aws-datastore/src/test/java/com/amplifyframework/datastore/syncengine/NetworkCapabilitiesUtilTest.kt b/aws-datastore/src/test/java/com/amplifyframework/datastore/syncengine/NetworkCapabilitiesUtilTest.kt deleted file mode 100644 index b252909fcd..0000000000 --- a/aws-datastore/src/test/java/com/amplifyframework/datastore/syncengine/NetworkCapabilitiesUtilTest.kt +++ /dev/null @@ -1,61 +0,0 @@ -package com.amplifyframework.datastore.syncengine - -import android.net.ConnectivityManager -import android.net.NetworkCapabilities -import org.junit.Assert.assertEquals -import org.junit.Assert.assertNull -import org.junit.Assert.assertTrue -import org.junit.Assert.assertFalse -import org.junit.Test -import org.mockito.Mockito - -class NetworkCapabilitiesUtilTest { - - @Test - fun testGetNetworkCapabilitiesSecurityException() { - val mockConnectivityManager = Mockito.mock(ConnectivityManager::class.java) - Mockito.`when`(mockConnectivityManager.activeNetwork).thenThrow(SecurityException()) - - val networkCapabilities = NetworkCapabilitiesUtil.getNetworkCapabilities(mockConnectivityManager) - assertNull(networkCapabilities) - } - - @Test - fun testGetNetworkCapabilities() { - val mockConnectivityManager = Mockito.mock(ConnectivityManager::class.java) - val expectedNetworkCapabilities = Mockito.mock(NetworkCapabilities::class.java) - Mockito.`when`(mockConnectivityManager.getNetworkCapabilities(mockConnectivityManager.activeNetwork)) - .thenReturn(expectedNetworkCapabilities) - - val networkCapabilities = NetworkCapabilitiesUtil.getNetworkCapabilities(mockConnectivityManager) - - assertEquals(expectedNetworkCapabilities, networkCapabilities) - } - - @Test - fun testIsInternetReachable() { - val networkCapabilitiesWithCellular = Mockito.mock(NetworkCapabilities::class.java) - Mockito.`when`(networkCapabilitiesWithCellular.hasTransport(NetworkCapabilities.TRANSPORT_CELLULAR)).thenReturn(true) - - val networkCapabilitiesWithWifi = Mockito.mock(NetworkCapabilities::class.java) - Mockito.`when`(networkCapabilitiesWithWifi.hasTransport(NetworkCapabilities.TRANSPORT_WIFI)).thenReturn(true) - - val networkCapabilitiesWithEthernet = Mockito.mock(NetworkCapabilities::class.java) - Mockito.`when`(networkCapabilitiesWithEthernet.hasTransport(NetworkCapabilities.TRANSPORT_ETHERNET)).thenReturn(true) - - val networkCapabilitiesWithVpnAndBandwidth = Mockito.mock(NetworkCapabilities::class.java) - Mockito.`when`(networkCapabilitiesWithVpnAndBandwidth.hasTransport(NetworkCapabilities.TRANSPORT_VPN)).thenReturn(true) - Mockito.`when`(networkCapabilitiesWithVpnAndBandwidth.linkDownstreamBandwidthKbps).thenReturn(1000) - - val networkCapabilitiesWithVpnNoBandwidth = Mockito.mock(NetworkCapabilities::class.java) - Mockito.`when`(networkCapabilitiesWithVpnNoBandwidth.hasTransport(NetworkCapabilities.TRANSPORT_VPN)).thenReturn(true) - Mockito.`when`(networkCapabilitiesWithVpnNoBandwidth.linkDownstreamBandwidthKbps).thenReturn(0) - - assertTrue(NetworkCapabilitiesUtil.isInternetReachable(networkCapabilitiesWithCellular)) - assertTrue(NetworkCapabilitiesUtil.isInternetReachable(networkCapabilitiesWithWifi)) - assertTrue(NetworkCapabilitiesUtil.isInternetReachable(networkCapabilitiesWithEthernet)) - assertTrue(NetworkCapabilitiesUtil.isInternetReachable(networkCapabilitiesWithVpnAndBandwidth)) - assertFalse(NetworkCapabilitiesUtil.isInternetReachable(networkCapabilitiesWithVpnNoBandwidth)) - assertFalse(NetworkCapabilitiesUtil.isInternetReachable(null)) - } -} \ No newline at end of file From e1e87226a56701d67ec239d38da1d4888418c98b Mon Sep 17 00:00:00 2001 From: Dan Tran Date: Wed, 28 Aug 2024 23:39:41 -0400 Subject: [PATCH 9/9] fix: refactor onAvailable method to use network capabilities if available --- .../datastore/syncengine/ReachabilityMonitor.kt | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/aws-datastore/src/main/java/com/amplifyframework/datastore/syncengine/ReachabilityMonitor.kt b/aws-datastore/src/main/java/com/amplifyframework/datastore/syncengine/ReachabilityMonitor.kt index f4c82a8e78..6ecb63db78 100644 --- a/aws-datastore/src/main/java/com/amplifyframework/datastore/syncengine/ReachabilityMonitor.kt +++ b/aws-datastore/src/main/java/com/amplifyframework/datastore/syncengine/ReachabilityMonitor.kt @@ -89,19 +89,32 @@ private class ReachabilityMonitorImpl constructor(val schedulerProvider: Schedul } private inner class ConnectivityNetworkCallback(private val emitter: ObservableEmitter) : NetworkCallback() { + private var currentNetworkCapabilities: NetworkCapabilities? = null + override fun onAvailable(network: Network) { - connectivityProvider?.let { emitter.onNext(it.hasActiveNetwork) } + // in recent Android SDK version, onCapabilitiesChanged is triggered before onAvailable + // we should make use of network capabilities if it was updated before + currentNetworkCapabilities?.let { + emitter.onNext(it.isInternetReachable()) + } ?: run { + connectivityProvider?.let { + emitter.onNext(it.hasActiveNetwork) + } + } } override fun onLost(network: Network) { + currentNetworkCapabilities = null emitter.onNext(false) } override fun onUnavailable() { + currentNetworkCapabilities = null emitter.onNext(false) } override fun onCapabilitiesChanged(network: Network, networkCapabilities: NetworkCapabilities) { + currentNetworkCapabilities = networkCapabilities emitter.onNext(networkCapabilities.isInternetReachable()) } }