From 8cf4ed7b7178691a493f33302ea21db6ac4fa588 Mon Sep 17 00:00:00 2001 From: Chris Narkiewicz Date: Mon, 25 Oct 2021 03:33:00 +0100 Subject: [PATCH 1/2] Suppress walled network check when on metered wifi Signed-off-by: Chris Narkiewicz --- .../network/ConnectivityServiceImpl.java | 42 ++++++---- .../client/network/NetworkModule.java | 4 +- .../client/network/ConnectivityServiceTest.kt | 81 ++++++++++++++++--- 3 files changed, 100 insertions(+), 27 deletions(-) diff --git a/src/main/java/com/nextcloud/client/network/ConnectivityServiceImpl.java b/src/main/java/com/nextcloud/client/network/ConnectivityServiceImpl.java index 908778a66f..b67f0da4a8 100644 --- a/src/main/java/com/nextcloud/client/network/ConnectivityServiceImpl.java +++ b/src/main/java/com/nextcloud/client/network/ConnectivityServiceImpl.java @@ -2,7 +2,7 @@ * Nextcloud Android client application * * @author Chris Narkiewicz - * Copyright (C) 2019 Chris Narkiewicz + * Copyright (C) 2021 Chris Narkiewicz * * This program is free software: you can redistribute it and/or modify * it under the terms of the GNU Affero General Public License as published by @@ -20,7 +20,9 @@ package com.nextcloud.client.network; +import android.annotation.SuppressLint; import android.net.ConnectivityManager; +import android.net.Network; import android.net.NetworkCapabilities; import android.net.NetworkInfo; @@ -32,6 +34,7 @@ import com.nextcloud.operations.GetMethod; import org.apache.commons.httpclient.HttpStatus; import androidx.core.net.ConnectivityManagerCompat; +import kotlin.jvm.functions.Function0; import kotlin.jvm.functions.Function1; class ConnectivityServiceImpl implements ConnectivityService { @@ -40,6 +43,7 @@ class ConnectivityServiceImpl implements ConnectivityService { private final UserAccountManager accountManager; private final ClientFactory clientFactory; private final GetRequestBuilder requestBuilder; + private final int sdkVersion; static class GetRequestBuilder implements Function1 { @Override @@ -51,17 +55,19 @@ class ConnectivityServiceImpl implements ConnectivityService { ConnectivityServiceImpl(ConnectivityManager platformConnectivityManager, UserAccountManager accountManager, ClientFactory clientFactory, - GetRequestBuilder requestBuilder) { + GetRequestBuilder requestBuilder, + int sdkVersion) { this.platformConnectivityManager = platformConnectivityManager; this.accountManager = accountManager; this.clientFactory = clientFactory; this.requestBuilder = requestBuilder; + this.sdkVersion = sdkVersion; } @Override public boolean isInternetWalled() { Connectivity c = getConnectivity(); - if (c.isConnected() && c.isWifi()) { + if (c.isConnected() && c.isWifi() && !c.isMetered()) { Server server = accountManager.getUser().getServer(); String baseServerAddress = server.getUri().toString(); @@ -76,7 +82,6 @@ class ConnectivityServiceImpl implements ConnectivityService { // Content-Length is not available when using chunked transfer encoding, so check for -1 as well boolean result = !(status == HttpStatus.SC_NO_CONTENT && get.getResponseContentLength() <= 0); - get.releaseConnection(); return result; @@ -96,21 +101,9 @@ class ConnectivityServiceImpl implements ConnectivityService { if (networkInfo != null) { boolean isConnected = networkInfo.isConnectedOrConnecting(); - // more detailed check boolean isMetered; - if (android.os.Build.VERSION.SDK_INT >= android.os.Build.VERSION_CODES.M) { - NetworkCapabilities networkCapabilities = platformConnectivityManager.getNetworkCapabilities( - platformConnectivityManager.getActiveNetwork()); - - if (networkCapabilities != null) { - isMetered = !networkCapabilities.hasCapability(NetworkCapabilities.NET_CAPABILITY_NOT_RESTRICTED); - } else { - isMetered = ConnectivityManagerCompat.isActiveNetworkMetered(platformConnectivityManager); - } - } else { - isMetered = ConnectivityManagerCompat.isActiveNetworkMetered(platformConnectivityManager); - } + isMetered = isNetworkMetered(); boolean isWifi = networkInfo.getType() == ConnectivityManager.TYPE_WIFI || hasNonCellularConnectivity(); return new Connectivity(isConnected, isMetered, isWifi, null); } else { @@ -118,6 +111,21 @@ class ConnectivityServiceImpl implements ConnectivityService { } } + @SuppressLint("NewApi") // false positive due to mocking + private boolean isNetworkMetered() { + if (sdkVersion >= android.os.Build.VERSION_CODES.M) { + final Network network = platformConnectivityManager.getActiveNetwork(); + NetworkCapabilities networkCapabilities = platformConnectivityManager.getNetworkCapabilities(network); + if (networkCapabilities != null) { + return !networkCapabilities.hasCapability(NetworkCapabilities.NET_CAPABILITY_NOT_RESTRICTED); + } else { + return ConnectivityManagerCompat.isActiveNetworkMetered(platformConnectivityManager); + } + } else { + return ConnectivityManagerCompat.isActiveNetworkMetered(platformConnectivityManager); + } + } + private boolean hasNonCellularConnectivity() { for (NetworkInfo networkInfo : platformConnectivityManager.getAllNetworkInfo()) { if (networkInfo.isConnectedOrConnecting() && (networkInfo.getType() == ConnectivityManager.TYPE_WIFI || diff --git a/src/main/java/com/nextcloud/client/network/NetworkModule.java b/src/main/java/com/nextcloud/client/network/NetworkModule.java index af1d9ccfd6..01b73d0377 100644 --- a/src/main/java/com/nextcloud/client/network/NetworkModule.java +++ b/src/main/java/com/nextcloud/client/network/NetworkModule.java @@ -22,6 +22,7 @@ package com.nextcloud.client.network; import android.content.Context; import android.net.ConnectivityManager; +import android.os.Build; import com.nextcloud.client.account.UserAccountManager; @@ -40,7 +41,8 @@ public class NetworkModule { return new ConnectivityServiceImpl(connectivityManager, accountManager, clientFactory, - new ConnectivityServiceImpl.GetRequestBuilder() + new ConnectivityServiceImpl.GetRequestBuilder(), + Build.VERSION.SDK_INT ); } diff --git a/src/test/java/com/nextcloud/client/network/ConnectivityServiceTest.kt b/src/test/java/com/nextcloud/client/network/ConnectivityServiceTest.kt index 30e0121cad..48d532b8aa 100644 --- a/src/test/java/com/nextcloud/client/network/ConnectivityServiceTest.kt +++ b/src/test/java/com/nextcloud/client/network/ConnectivityServiceTest.kt @@ -2,7 +2,7 @@ * Nextcloud Android client application * * @author Chris Narkiewicz - * Copyright (C) 2020 Chris Narkiewicz + * Copyright (C) 2021 Chris Narkiewicz * * This program is free software: you can redistribute it and/or modify * it under the terms of the GNU Affero General Public License as published by @@ -20,7 +20,10 @@ package com.nextcloud.client.network import android.net.ConnectivityManager +import android.net.Network +import android.net.NetworkCapabilities import android.net.NetworkInfo +import android.os.Build import com.nextcloud.client.account.Server import com.nextcloud.client.account.User import com.nextcloud.client.account.UserAccountManager @@ -28,8 +31,10 @@ import com.nextcloud.client.logger.Logger import com.nextcloud.common.PlainClient import com.nextcloud.operations.GetMethod import com.nhaarman.mockitokotlin2.any +import com.nhaarman.mockitokotlin2.eq import com.nhaarman.mockitokotlin2.mock import com.nhaarman.mockitokotlin2.never +import com.nhaarman.mockitokotlin2.times import com.nhaarman.mockitokotlin2.verify import com.nhaarman.mockitokotlin2.whenever import com.owncloud.android.lib.resources.status.OwnCloudVersion @@ -89,6 +94,12 @@ class ConnectivityServiceTest { @Mock lateinit var requestBuilder: ConnectivityServiceImpl.GetRequestBuilder + @Mock + lateinit var network: Network + + @Mock + lateinit var networkCapabilities: NetworkCapabilities + @Mock lateinit var logger: Logger @@ -108,11 +119,16 @@ class ConnectivityServiceTest { platformConnectivityManager, accountManager, clientFactory, - requestBuilder + requestBuilder, + Build.VERSION_CODES.M ) + whenever(networkCapabilities.hasCapability(eq(NetworkCapabilities.NET_CAPABILITY_NOT_RESTRICTED))) + .thenReturn(true) + whenever(platformConnectivityManager.activeNetwork).thenReturn(network) whenever(platformConnectivityManager.activeNetworkInfo).thenReturn(networkInfo) whenever(platformConnectivityManager.allNetworkInfo).thenReturn(arrayOf(networkInfo)) + whenever(platformConnectivityManager.getNetworkCapabilities(any())).thenReturn(networkCapabilities) whenever(requestBuilder.invoke(any())).thenReturn(getRequest) whenever(clientFactory.createPlainClient()).thenReturn(client) whenever(user.server).thenReturn(newServer) @@ -235,12 +251,55 @@ class ConnectivityServiceTest { whenever(networkInfo.isConnectedOrConnecting).thenReturn(true) whenever(networkInfo.type).thenReturn(ConnectivityManager.TYPE_WIFI) whenever(accountManager.getServerVersion(any())).thenReturn(OwnCloudVersion.nextcloud_20) - assertTrue( - "Precondition failed", - connectivityService.connectivity.let { - it.isConnected && it.isWifi - } - ) + connectivityService.connectivity.let { + assertTrue(it.isConnected) + assertTrue(it.isWifi) + assertFalse(it.isMetered) + } + } + + @Test + fun `request not sent when not connected`() { + // GIVEN + // network is not connected + whenever(networkInfo.isConnectedOrConnecting).thenReturn(false) + whenever(networkInfo.isConnected).thenReturn(false) + + // WHEN + // connectivity is checked + val result = connectivityService.isInternetWalled + + // THEN + // connection is walled + // request is not sent + assertTrue("Server should not be accessible", result) + verify(requestBuilder, never()).invoke(any()) + verify(client, never()).execute(any()) + } + + @Test + fun `request not sent when wifi is metered`() { + // GIVEN + // network is connected to wifi + // wifi is metered + whenever(networkCapabilities.hasCapability(any())).thenReturn(false) // this test is mocked for API M + whenever(platformConnectivityManager.isActiveNetworkMetered).thenReturn(true) + connectivityService.connectivity.let { + assertTrue("should be connected", it.isConnected) + assertTrue("should be connected to wifi", it.isWifi) + assertTrue("check mocking, this check is complicated and depends on SDK version", it.isMetered) + } + + // WHEN + // connectivity is checked + val result = connectivityService.isInternetWalled + + // THEN + // assume internet is not walled + // request is not sent + assertFalse("Server should not be accessible", result) + verify(requestBuilder, never()).invoke(any()) + verify(getRequest, never()).execute(any()) } @Test @@ -260,7 +319,7 @@ class ConnectivityServiceTest { // request is not sent assertTrue("Server should not be accessible", result) verify(requestBuilder, never()).invoke(any()) - verify(client, never()).execute(any()) + verify(getRequest, never()).execute(any()) } fun mockResponse(contentLength: Long = 0, status: Int = HttpStatus.SC_OK) { @@ -274,18 +333,21 @@ class ConnectivityServiceTest { fun `status 204 means internet is not walled`() { mockResponse(contentLength = 0, status = HttpStatus.SC_NO_CONTENT) assertFalse(connectivityService.isInternetWalled) + verify(getRequest, times(1)).execute(client) } @Test fun `status 204 and no content length means internet is not walled`() { mockResponse(contentLength = -1, status = HttpStatus.SC_NO_CONTENT) assertFalse(connectivityService.isInternetWalled) + verify(getRequest, times(1)).execute(client) } @Test fun `other status than 204 means internet is walled`() { mockResponse(contentLength = 0, status = HttpStatus.SC_GONE) assertTrue(connectivityService.isInternetWalled) + verify(getRequest, times(1)).execute(client) } @Test @@ -295,6 +357,7 @@ class ConnectivityServiceTest { val urlCaptor = ArgumentCaptor.forClass(String::class.java) verify(requestBuilder).invoke(urlCaptor.capture()) assertTrue("Invalid URL used to check status", urlCaptor.value.endsWith("/index.php/204")) + verify(getRequest, times(1)).execute(client) } } } From d0643520111838d4c4302329494bbbf231405864 Mon Sep 17 00:00:00 2001 From: tobiasKaminsky Date: Mon, 25 Oct 2021 09:10:21 +0200 Subject: [PATCH 2/2] Fix test Signed-off-by: tobiasKaminsky --- .../com/nextcloud/client/network/ConnectivityServiceImplIT.kt | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/androidTest/java/com/nextcloud/client/network/ConnectivityServiceImplIT.kt b/src/androidTest/java/com/nextcloud/client/network/ConnectivityServiceImplIT.kt index 651534914a..7aad5fa828 100644 --- a/src/androidTest/java/com/nextcloud/client/network/ConnectivityServiceImplIT.kt +++ b/src/androidTest/java/com/nextcloud/client/network/ConnectivityServiceImplIT.kt @@ -24,6 +24,7 @@ package com.nextcloud.client.network import android.accounts.AccountManager import android.content.Context import android.net.ConnectivityManager +import android.os.Build import com.nextcloud.client.account.UserAccountManagerImpl import com.nextcloud.client.network.ConnectivityServiceImpl.GetRequestBuilder import com.owncloud.android.AbstractOnServerIT @@ -44,7 +45,8 @@ class ConnectivityServiceImplIT : AbstractOnServerIT() { connectivityManager, userAccountManager, clientFactory, - requestBuilder + requestBuilder, + Build.VERSION.SDK_INT ) assertTrue(sut.connectivity.isConnected)