Merge pull request #10996 from nextcloud/cache-walled-check

Cache walled check for 10 minutes

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 cachedEntry: Pair<Long, Boolean>? = null
+
+    @Synchronized
+    fun isExpired(): Boolean {
+        return when (val timestamp = cachedEntry?.first) {
+            null -> true
+            else -> {
+                val diff = clock.currentTime - timestamp
+                diff >= CACHE_TIME_MS
+            }
+        }
+    }
+
+    @Synchronized
+    fun setValue(isWalled: Boolean) {
+        this.cachedEntry = Pair(clock.currentTime, isWalled)
+    }
+
+    @Synchronized
+    fun getValue(): Boolean? {
+        return when (isExpired()) {
+            true -> null
+            else -> cachedEntry?.second
+        }
+    }
+
+    @Synchronized
+    fun clear() {
+        cachedEntry = 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)
         }
     }