From 6cb3879cafdcdd827120783c30c7c2dba7144a9c Mon Sep 17 00:00:00 2001 From: Suraj Shah Date: Tue, 17 Dec 2019 06:03:11 +0000 Subject: [PATCH] For #7048: Network info migration (#7091) * Fixes #7048. Adds extension function to check if online or not based on capabilities Modified `buildDeviceList` Modifies tests * Fixing lint error for max length Fixing test in AppRequestInterceptorTest.kt * Adds suppression for deprecation Moving away from using anko for ConnectivityManager instance * Creates ConnectivityManager extension component * Fixes #7180 Refactors test cases to fix static mocks --- .../mozilla/fenix/AppRequestInterceptor.kt | 9 +++--- .../mozilla/fenix/ext/ConnectivityManager.kt | 29 +++++++++++++++++++ .../org/mozilla/fenix/share/ShareViewModel.kt | 14 ++++----- .../fenix/AppRequestInterceptorTest.kt | 10 +++++++ .../mozilla/fenix/share/ShareViewModelTest.kt | 13 +++++---- 5 files changed, 58 insertions(+), 17 deletions(-) create mode 100644 app/src/main/java/org/mozilla/fenix/ext/ConnectivityManager.kt diff --git a/app/src/main/java/org/mozilla/fenix/AppRequestInterceptor.kt b/app/src/main/java/org/mozilla/fenix/AppRequestInterceptor.kt index 92a8b07b5d..7c167b47d0 100644 --- a/app/src/main/java/org/mozilla/fenix/AppRequestInterceptor.kt +++ b/app/src/main/java/org/mozilla/fenix/AppRequestInterceptor.kt @@ -6,8 +6,8 @@ package org.mozilla.fenix import android.content.Context import android.net.ConnectivityManager -import android.net.NetworkInfo import androidx.annotation.RawRes +import androidx.core.content.getSystemService import mozilla.components.browser.errorpages.ErrorPages import mozilla.components.browser.errorpages.ErrorType import mozilla.components.concept.engine.EngineSession @@ -15,6 +15,7 @@ import mozilla.components.concept.engine.request.RequestInterceptor import org.mozilla.fenix.components.metrics.Event import org.mozilla.fenix.ext.components import org.mozilla.fenix.ext.settings +import org.mozilla.fenix.ext.isOnline class AppRequestInterceptor(private val context: Context) : RequestInterceptor { override fun onLoadRequest( @@ -71,10 +72,8 @@ class AppRequestInterceptor(private val context: Context) : RequestInterceptor { private fun improveErrorType(errorType: ErrorType): ErrorType { // This is not an ideal solution. For context, see: // https://github.com/mozilla-mobile/android-components/pull/5068#issuecomment-558415367 - val cm = context.getSystemService(Context.CONNECTIVITY_SERVICE) as ConnectivityManager - val activeNetwork: NetworkInfo? = cm.activeNetworkInfo - @Suppress("DEPRECATION") // NetworkCallback is not appropriate for this use case - val isConnected: Boolean = activeNetwork?.isConnectedOrConnecting == true + + val isConnected: Boolean = context.getSystemService()!!.isOnline() return when { errorType == ErrorType.ERROR_UNKNOWN_HOST && !isConnected -> ErrorType.ERROR_NO_INTERNET diff --git a/app/src/main/java/org/mozilla/fenix/ext/ConnectivityManager.kt b/app/src/main/java/org/mozilla/fenix/ext/ConnectivityManager.kt new file mode 100644 index 0000000000..465090c354 --- /dev/null +++ b/app/src/main/java/org/mozilla/fenix/ext/ConnectivityManager.kt @@ -0,0 +1,29 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +package org.mozilla.fenix.ext + +import android.net.ConnectivityManager +import android.net.Network +import android.net.NetworkCapabilities +import android.os.Build + +/** +* Checks for availability of network. +* +* For devices above [Build.VERSION_CODES.M] it even checks if there's internet flowing through it or not. +* */ +fun ConnectivityManager.isOnline(network: Network? = null): Boolean { + return if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.M) { + getNetworkCapabilities(network ?: activeNetwork)?.let { + it.hasCapability(NetworkCapabilities.NET_CAPABILITY_INTERNET) && + it.hasCapability(NetworkCapabilities.NET_CAPABILITY_VALIDATED) + } ?: false + } else { + // for devices below android M, there's no better way to get this. + // active network info can be null if there are no active networks. + @Suppress("Deprecation") + activeNetworkInfo?.isConnected ?: false + } +} diff --git a/app/src/main/java/org/mozilla/fenix/share/ShareViewModel.kt b/app/src/main/java/org/mozilla/fenix/share/ShareViewModel.kt index 397f537c8b..0cbbf4b660 100644 --- a/app/src/main/java/org/mozilla/fenix/share/ShareViewModel.kt +++ b/app/src/main/java/org/mozilla/fenix/share/ShareViewModel.kt @@ -23,6 +23,7 @@ import kotlinx.coroutines.launch import mozilla.components.concept.sync.DeviceCapability import mozilla.components.service.fxa.manager.FxaAccountManager import org.mozilla.fenix.ext.components +import org.mozilla.fenix.ext.isOnline import org.mozilla.fenix.share.listadapters.AppShareOption import org.mozilla.fenix.share.listadapters.SyncShareOption @@ -36,17 +37,17 @@ class ShareViewModel(application: Application) : AndroidViewModel(application) { @VisibleForTesting internal val networkCallback = object : ConnectivityManager.NetworkCallback() { - override fun onLost(network: Network?) = reloadDevices() - override fun onAvailable(network: Network?) = reloadDevices() + override fun onLost(network: Network?) = reloadDevices(network) + override fun onAvailable(network: Network?) = reloadDevices(network) - private fun reloadDevices() { + private fun reloadDevices(network: Network?) { viewModelScope.launch(IO) { fxaAccountManager.authenticatedAccount() ?.deviceConstellation() ?.refreshDevicesAsync() ?.await() - val devicesShareOptions = buildDeviceList(fxaAccountManager) + val devicesShareOptions = buildDeviceList(fxaAccountManager, network) devicesListLiveData.postValue(devicesShareOptions) } } @@ -128,13 +129,12 @@ class ShareViewModel(application: Application) : AndroidViewModel(application) { */ @VisibleForTesting @WorkerThread - internal fun buildDeviceList(accountManager: FxaAccountManager): List { - val activeNetwork = connectivityManager?.activeNetworkInfo + internal fun buildDeviceList(accountManager: FxaAccountManager, network: Network? = null): List { val account = accountManager.authenticatedAccount() return when { // No network - activeNetwork?.isConnected != true -> listOf(SyncShareOption.Offline) + connectivityManager?.isOnline(network) != true -> listOf(SyncShareOption.Offline) // No account signed in account == null -> listOf(SyncShareOption.SignIn) // Account needs to be re-authenticated diff --git a/app/src/test/java/org/mozilla/fenix/AppRequestInterceptorTest.kt b/app/src/test/java/org/mozilla/fenix/AppRequestInterceptorTest.kt index 9351972a72..08ebeb68ed 100644 --- a/app/src/test/java/org/mozilla/fenix/AppRequestInterceptorTest.kt +++ b/app/src/test/java/org/mozilla/fenix/AppRequestInterceptorTest.kt @@ -4,10 +4,14 @@ package org.mozilla.fenix +import android.net.ConnectivityManager import androidx.annotation.RawRes +import androidx.core.content.getSystemService import assertk.assertThat import assertk.assertions.isEqualTo +import io.mockk.every import io.mockk.mockk +import io.mockk.mockkStatic import kotlinx.coroutines.ObsoleteCoroutinesApi import mozilla.components.browser.errorpages.ErrorPages import mozilla.components.browser.errorpages.ErrorType @@ -16,6 +20,7 @@ import mozilla.components.support.test.robolectric.testContext import org.junit.Before import org.junit.Test import org.junit.runner.RunWith +import org.mozilla.fenix.ext.isOnline import org.robolectric.RobolectricTestRunner import org.robolectric.annotation.Config @@ -28,6 +33,11 @@ class AppRequestInterceptorTest { @Before fun setUp() { + + mockkStatic("org.mozilla.fenix.ext.ConnectivityManagerKt") + + every { testContext.getSystemService()!!.isOnline() } returns true + interceptor = AppRequestInterceptor(testContext) } diff --git a/app/src/test/java/org/mozilla/fenix/share/ShareViewModelTest.kt b/app/src/test/java/org/mozilla/fenix/share/ShareViewModelTest.kt index 72a9168680..d1244b7cea 100644 --- a/app/src/test/java/org/mozilla/fenix/share/ShareViewModelTest.kt +++ b/app/src/test/java/org/mozilla/fenix/share/ShareViewModelTest.kt @@ -15,6 +15,7 @@ import io.mockk.every import io.mockk.mockk import io.mockk.spyk import io.mockk.verify +import io.mockk.mockkStatic import kotlinx.coroutines.runBlocking import mozilla.components.service.fxa.manager.FxaAccountManager import mozilla.components.support.test.robolectric.testContext @@ -25,6 +26,7 @@ import org.junit.runner.RunWith import org.mozilla.fenix.TestApplication import org.mozilla.fenix.ext.application import org.mozilla.fenix.ext.components +import org.mozilla.fenix.ext.isOnline import org.mozilla.fenix.share.listadapters.AppShareOption import org.mozilla.fenix.share.listadapters.SyncShareOption import org.robolectric.RobolectricTestRunner @@ -48,11 +50,12 @@ class ShareViewModelTest { connectivityManager = mockk(relaxed = true) fxaAccountManager = mockk(relaxed = true) + mockkStatic("org.mozilla.fenix.ext.ConnectivityManagerKt") + every { application.packageName } returns packageName every { application.packageManager } returns packageManager every { application.getSystemService() } returns connectivityManager every { application.components.backgroundServices.accountManager } returns fxaAccountManager - every { connectivityManager.activeNetworkInfo } returns mockk(relaxed = true) viewModel = ShareViewModel(application) } @@ -91,16 +94,16 @@ class ShareViewModelTest { @Test fun `buildDevicesList returns offline option`() { - every { connectivityManager.activeNetworkInfo.isConnected } returns false + every { connectivityManager.isOnline() } returns false assertEquals(listOf(SyncShareOption.Offline), viewModel.buildDeviceList(fxaAccountManager)) - every { connectivityManager.activeNetworkInfo } returns null + every { connectivityManager.getNetworkCapabilities(connectivityManager.activeNetwork) } returns null assertEquals(listOf(SyncShareOption.Offline), viewModel.buildDeviceList(fxaAccountManager)) } @Test fun `buildDevicesList returns sign-in option`() { - every { connectivityManager.activeNetworkInfo.isConnected } returns true + every { connectivityManager.isOnline() } returns true every { fxaAccountManager.authenticatedAccount() } returns null assertEquals(listOf(SyncShareOption.SignIn), viewModel.buildDeviceList(fxaAccountManager)) @@ -108,7 +111,7 @@ class ShareViewModelTest { @Test fun `buildDevicesList returns reconnect option`() { - every { connectivityManager.activeNetworkInfo.isConnected } returns true + every { connectivityManager.isOnline() } returns true every { fxaAccountManager.authenticatedAccount() } returns mockk() every { fxaAccountManager.accountNeedsReauth() } returns true