Cache walled check for 10 minutes

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 <alvaro.brey@nextcloud.com>

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)

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<String, GetMethod> {
         @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();
         }
     }
 

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
         );
     }
 

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 <http://www.gnu.org/licenses/>.
+ *
+ */
+
+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<Long, Boolean>? = 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
+    }
+}

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<ViewThemeUtils> 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,

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());

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,

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)
         }
     }