From 2c1d18db5cf91d4c1c9e40c89ba2e8f803c5d50a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=81lvaro=20Brey?= Date: Mon, 7 Nov 2022 09:32:02 +0100 Subject: Cache walled check for 10 minutes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Tentative cache time, accepting other proposals. My rationale for 10 minutes was that 6 checks an hour seems like not too much, while not locking the app for too long in case there's an error and the cache is not cleared on network change. Signed-off-by: Álvaro Brey --- .../client/network/ConnectivityServiceImplIT.kt | 5 +- .../client/network/ConnectivityServiceImpl.java | 57 +++++++++++------- .../nextcloud/client/network/NetworkModule.java | 5 +- .../nextcloud/client/network/WalledCheckCache.kt | 67 ++++++++++++++++++++++ .../main/java/com/owncloud/android/MainApp.java | 13 +++-- .../android/files/BootupBroadcastReceiver.java | 5 +- .../owncloud/android/utils/ReceiversHelper.java | 5 +- .../client/network/ConnectivityServiceTest.kt | 7 ++- 8 files changed, 131 insertions(+), 33 deletions(-) create mode 100644 app/src/main/java/com/nextcloud/client/network/WalledCheckCache.kt diff --git a/app/src/androidTest/java/com/nextcloud/client/network/ConnectivityServiceImplIT.kt b/app/src/androidTest/java/com/nextcloud/client/network/ConnectivityServiceImplIT.kt index 7aad5fa828..145b0e6731 100644 --- a/app/src/androidTest/java/com/nextcloud/client/network/ConnectivityServiceImplIT.kt +++ b/app/src/androidTest/java/com/nextcloud/client/network/ConnectivityServiceImplIT.kt @@ -24,8 +24,8 @@ 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.core.ClockImpl import com.nextcloud.client.network.ConnectivityServiceImpl.GetRequestBuilder import com.owncloud.android.AbstractOnServerIT import org.junit.Assert.assertFalse @@ -40,13 +40,14 @@ class ConnectivityServiceImplIT : AbstractOnServerIT() { val userAccountManager = UserAccountManagerImpl(targetContext, accountManager) val clientFactory = ClientFactoryImpl(targetContext) val requestBuilder = GetRequestBuilder() + val walledCheckCache = WalledCheckCache(ClockImpl()) val sut = ConnectivityServiceImpl( connectivityManager, userAccountManager, clientFactory, requestBuilder, - Build.VERSION.SDK_INT + walledCheckCache ) assertTrue(sut.connectivity.isConnected) diff --git a/app/src/main/java/com/nextcloud/client/network/ConnectivityServiceImpl.java b/app/src/main/java/com/nextcloud/client/network/ConnectivityServiceImpl.java index 4d186d3477..095308851e 100644 --- a/app/src/main/java/com/nextcloud/client/network/ConnectivityServiceImpl.java +++ b/app/src/main/java/com/nextcloud/client/network/ConnectivityServiceImpl.java @@ -39,11 +39,14 @@ import kotlin.jvm.functions.Function1; class ConnectivityServiceImpl implements ConnectivityService { private static final String TAG = "ConnectivityServiceImpl"; + private static final String CONNECTIVITY_CHECK_ROUTE = "/index.php/204"; + private final ConnectivityManager platformConnectivityManager; private final UserAccountManager accountManager; private final ClientFactory clientFactory; private final GetRequestBuilder requestBuilder; - private final int sdkVersion; + private final WalledCheckCache walledCheckCache; + static class GetRequestBuilder implements Function1 { @Override @@ -56,37 +59,49 @@ class ConnectivityServiceImpl implements ConnectivityService { UserAccountManager accountManager, ClientFactory clientFactory, GetRequestBuilder requestBuilder, - int sdkVersion) { + final WalledCheckCache walledCheckCache) { this.platformConnectivityManager = platformConnectivityManager; this.accountManager = accountManager; this.clientFactory = clientFactory; this.requestBuilder = requestBuilder; - this.sdkVersion = sdkVersion; + this.walledCheckCache = walledCheckCache; } @Override public boolean isInternetWalled() { - Connectivity c = getConnectivity(); - if (c.isConnected() && c.isWifi() && !c.isMetered()) { - - Server server = accountManager.getUser().getServer(); - String baseServerAddress = server.getUri().toString(); - if (baseServerAddress.isEmpty()) { - return true; + final Boolean cachedValue = walledCheckCache.getValue(); + if (cachedValue != null) { + return cachedValue; + } else { + boolean result; + Connectivity c = getConnectivity(); + if (c.isConnected() && c.isWifi() && !c.isMetered()) { + + Server server = accountManager.getUser().getServer(); + String baseServerAddress = server.getUri().toString(); + if (baseServerAddress.isEmpty()) { + result = true; + } else { + + GetMethod get = requestBuilder.invoke(baseServerAddress + CONNECTIVITY_CHECK_ROUTE); + PlainClient client = clientFactory.createPlainClient(); + + int status = get.execute(client); + + // Content-Length is not available when using chunked transfer encoding, so check for -1 as well + result = !(status == HttpStatus.SC_NO_CONTENT && get.getResponseContentLength() <= 0); + get.releaseConnection(); + if (result) { + Log_OC.w(TAG, "isInternetWalled(): Failed to GET " + CONNECTIVITY_CHECK_ROUTE + "," + + " assuming connectivity is impaired"); + } + } + } else { + result = !c.isConnected(); } - GetMethod get = requestBuilder.invoke(baseServerAddress + "/index.php/204"); - PlainClient client = clientFactory.createPlainClient(); - - int status = get.execute(client); - - // 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(); - + walledCheckCache.setValue(result); return result; - } else { - return !c.isConnected(); } } diff --git a/app/src/main/java/com/nextcloud/client/network/NetworkModule.java b/app/src/main/java/com/nextcloud/client/network/NetworkModule.java index 01b73d0377..cd29f844f2 100644 --- a/app/src/main/java/com/nextcloud/client/network/NetworkModule.java +++ b/app/src/main/java/com/nextcloud/client/network/NetworkModule.java @@ -37,12 +37,13 @@ public class NetworkModule { @Provides ConnectivityService connectivityService(ConnectivityManager connectivityManager, UserAccountManager accountManager, - ClientFactory clientFactory) { + ClientFactory clientFactory, + WalledCheckCache walledCheckCache) { return new ConnectivityServiceImpl(connectivityManager, accountManager, clientFactory, new ConnectivityServiceImpl.GetRequestBuilder(), - Build.VERSION.SDK_INT + walledCheckCache ); } diff --git a/app/src/main/java/com/nextcloud/client/network/WalledCheckCache.kt b/app/src/main/java/com/nextcloud/client/network/WalledCheckCache.kt new file mode 100644 index 0000000000..6e0d70fe08 --- /dev/null +++ b/app/src/main/java/com/nextcloud/client/network/WalledCheckCache.kt @@ -0,0 +1,67 @@ +/* + * Nextcloud Android client application + * + * @author Álvaro Brey + * Copyright (C) 2022 Álvaro Brey + * Copyright (C) 2022 Nextcloud GmbH + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU AFFERO GENERAL PUBLIC LICENSE + * License as published by the Free Software Foundation; either + * version 3 of the License, or any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU AFFERO GENERAL PUBLIC LICENSE for more details. + * + * You should have received a copy of the GNU Affero General Public + * License along with this program. If not, see . + * + */ + +package com.nextcloud.client.network + +import com.nextcloud.client.core.Clock +import javax.inject.Inject +import javax.inject.Singleton + +@Singleton +class WalledCheckCache @Inject constructor(private val clock: Clock) { + + private var value: Pair? = null + + @Synchronized + fun isExpired(): Boolean { + return when (val timestamp = value?.first) { + null -> true + else -> { + val diff = clock.millisSinceBoot - timestamp + diff >= CACHE_TIME_MS + } + } + } + + @Synchronized + fun setValue(value: Boolean) { + this.value = Pair(clock.millisSinceBoot, value) + } + + @Synchronized + fun getValue(): Boolean? { + return when (isExpired()) { + true -> null + else -> value?.second + } + } + + @Synchronized + fun clear() { + value = null + } + + companion object { + // 10 minutes + private const val CACHE_TIME_MS = 10 * 60 * 1000 + } +} diff --git a/app/src/main/java/com/owncloud/android/MainApp.java b/app/src/main/java/com/owncloud/android/MainApp.java index 170d9a0d56..f7453ae904 100644 --- a/app/src/main/java/com/owncloud/android/MainApp.java +++ b/app/src/main/java/com/owncloud/android/MainApp.java @@ -54,6 +54,7 @@ import com.nextcloud.client.logger.LegacyLoggerAdapter; import com.nextcloud.client.logger.Logger; import com.nextcloud.client.migrations.MigrationsManager; import com.nextcloud.client.network.ConnectivityService; +import com.nextcloud.client.network.WalledCheckCache; import com.nextcloud.client.onboarding.OnboardingService; import com.nextcloud.client.preferences.AppPreferences; import com.nextcloud.client.preferences.AppPreferencesImpl; @@ -178,6 +179,8 @@ public class MainApp extends MultiDexApplication implements HasAndroidInjector { @Inject PassCodeManager passCodeManager; + @Inject WalledCheckCache walledCheckCache; + // workaround because injection is initialized on onAttachBaseContext // and getApplicationContext is null at that point, which crashes when getting current user @Inject Provider viewThemeUtilsProvider; @@ -326,7 +329,8 @@ public class MainApp extends MultiDexApplication implements HasAndroidInjector { powerManagementService, backgroundJobManager, clock, - viewThemeUtils); + viewThemeUtils, + walledCheckCache); initContactsBackup(accountManager, backgroundJobManager); notificationChannels(); @@ -506,8 +510,8 @@ public class MainApp extends MultiDexApplication implements HasAndroidInjector { final PowerManagementService powerManagementService, final BackgroundJobManager backgroundJobManager, final Clock clock, - final ViewThemeUtils viewThemeUtils - ) { + final ViewThemeUtils viewThemeUtils, + final WalledCheckCache walledCheckCache) { updateToAutoUpload(); cleanOldEntries(clock); updateAutoUploadEntries(clock); @@ -537,7 +541,8 @@ public class MainApp extends MultiDexApplication implements HasAndroidInjector { ReceiversHelper.registerNetworkChangeReceiver(uploadsStorageManager, accountManager, connectivityService, - powerManagementService); + powerManagementService, + walledCheckCache); ReceiversHelper.registerPowerChangeReceiver(uploadsStorageManager, accountManager, diff --git a/app/src/main/java/com/owncloud/android/files/BootupBroadcastReceiver.java b/app/src/main/java/com/owncloud/android/files/BootupBroadcastReceiver.java index 06fa844e19..0ed4a72ab3 100644 --- a/app/src/main/java/com/owncloud/android/files/BootupBroadcastReceiver.java +++ b/app/src/main/java/com/owncloud/android/files/BootupBroadcastReceiver.java @@ -32,6 +32,7 @@ import com.nextcloud.client.core.Clock; import com.nextcloud.client.device.PowerManagementService; import com.nextcloud.client.jobs.BackgroundJobManager; import com.nextcloud.client.network.ConnectivityService; +import com.nextcloud.client.network.WalledCheckCache; import com.nextcloud.client.preferences.AppPreferences; import com.owncloud.android.MainApp; import com.owncloud.android.datamodel.UploadsStorageManager; @@ -59,6 +60,7 @@ public class BootupBroadcastReceiver extends BroadcastReceiver { @Inject BackgroundJobManager backgroundJobManager; @Inject Clock clock; @Inject ViewThemeUtils viewThemeUtils; + @Inject WalledCheckCache walledCheckCache; /** * Receives broadcast intent reporting that the system was just boot up. * @@ -78,7 +80,8 @@ public class BootupBroadcastReceiver extends BroadcastReceiver { powerManagementService, backgroundJobManager, clock, - viewThemeUtils); + viewThemeUtils, + walledCheckCache); MainApp.initContactsBackup(accountManager, backgroundJobManager); } else { Log_OC.d(TAG, "Getting wrong intent: " + intent.getAction()); diff --git a/app/src/main/java/com/owncloud/android/utils/ReceiversHelper.java b/app/src/main/java/com/owncloud/android/utils/ReceiversHelper.java index 06f1c1222f..b5f00b9203 100644 --- a/app/src/main/java/com/owncloud/android/utils/ReceiversHelper.java +++ b/app/src/main/java/com/owncloud/android/utils/ReceiversHelper.java @@ -30,6 +30,7 @@ import android.content.IntentFilter; import com.nextcloud.client.account.UserAccountManager; import com.nextcloud.client.device.PowerManagementService; import com.nextcloud.client.network.ConnectivityService; +import com.nextcloud.client.network.WalledCheckCache; import com.nextcloud.common.DNSCache; import com.owncloud.android.MainApp; import com.owncloud.android.datamodel.UploadsStorageManager; @@ -46,7 +47,8 @@ public final class ReceiversHelper { public static void registerNetworkChangeReceiver(final UploadsStorageManager uploadsStorageManager, final UserAccountManager accountManager, final ConnectivityService connectivityService, - final PowerManagementService powerManagementService) { + final PowerManagementService powerManagementService, + final WalledCheckCache walledCheckCache) { Context context = MainApp.getAppContext(); IntentFilter intentFilter = new IntentFilter(); @@ -57,6 +59,7 @@ public final class ReceiversHelper { @Override public void onReceive(Context context, Intent intent) { DNSCache.clear(); + walledCheckCache.clear(); if (connectivityService.getConnectivity().isConnected()) { FilesSyncHelper.restartJobsIfNeeded(uploadsStorageManager, accountManager, diff --git a/app/src/test/java/com/nextcloud/client/network/ConnectivityServiceTest.kt b/app/src/test/java/com/nextcloud/client/network/ConnectivityServiceTest.kt index 593f07a955..426690bdf8 100644 --- a/app/src/test/java/com/nextcloud/client/network/ConnectivityServiceTest.kt +++ b/app/src/test/java/com/nextcloud/client/network/ConnectivityServiceTest.kt @@ -23,7 +23,6 @@ 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 @@ -97,6 +96,9 @@ class ConnectivityServiceTest { @Mock lateinit var network: Network + @Mock + lateinit var walledCheckCache: WalledCheckCache + @Mock lateinit var networkCapabilities: NetworkCapabilities @@ -120,7 +122,7 @@ class ConnectivityServiceTest { accountManager, clientFactory, requestBuilder, - Build.VERSION_CODES.Q + walledCheckCache ) whenever(networkCapabilities.hasCapability(eq(NetworkCapabilities.NET_CAPABILITY_NOT_RESTRICTED))) @@ -133,6 +135,7 @@ class ConnectivityServiceTest { whenever(clientFactory.createPlainClient()).thenReturn(client) whenever(user.server).thenReturn(newServer) whenever(accountManager.user).thenReturn(user) + whenever(walledCheckCache.getValue()).thenReturn(null) } } -- cgit v1.2.3