fix: connection service impl

Signed-off-by: alperozturk <alper_ozturk@proton.me>

Author: alperozturk96

app/src/androidTest/java/com/nextcloud/client/network/ConnectivityServiceImplIT.kt

@@ -9,7 +9,6 @@ package com.nextcloud.client.network
 
 import android.accounts.AccountManager
 import android.content.Context
-import android.net.ConnectivityManager
 import com.nextcloud.client.account.UserAccountManagerImpl
 import com.nextcloud.client.core.ClockImpl
 import com.nextcloud.client.network.ConnectivityServiceImpl.GetRequestBuilder
@@ -21,15 +20,14 @@ import org.junit.Test
 class ConnectivityServiceImplIT : AbstractOnServerIT() {
     @Test
     fun testInternetWalled() {
-        val connectivityManager = targetContext.getSystemService(Context.CONNECTIVITY_SERVICE) as ConnectivityManager
         val accountManager = targetContext.getSystemService(Context.ACCOUNT_SERVICE) as AccountManager
         val userAccountManager = UserAccountManagerImpl(targetContext, accountManager)
         val clientFactory = ClientFactoryImpl(targetContext)
         val requestBuilder = GetRequestBuilder()
         val walledCheckCache = WalledCheckCache(ClockImpl())
 
         val sut = ConnectivityServiceImpl(
-            connectivityManager,
+            targetContext,
             userAccountManager,
             clientFactory,
             requestBuilder,

app/src/main/java/com/nextcloud/client/network/ConnectivityService.java

@@ -7,6 +7,12 @@
 package com.nextcloud.client.network;
 
 
+import android.net.ConnectivityManager;
+import android.net.Network;
+
+import com.nextcloud.client.account.Server;
+import com.nextcloud.client.account.UserAccountManager;
+
 import androidx.annotation.NonNull;
 
 /**
@@ -15,31 +21,81 @@
  */
 public interface ConnectivityService {
     /**
-     * Checks the availability of the server and the device's internet connection.
-     * <p>
-     * This method performs a network request to verify if the server is accessible and
-     * checks if the device has an active internet connection.
-     * </p>
+     * Asynchronously checks whether both the device's network connection
+     * and the Nextcloud server are available.
+     *
+     * <p>This method executes its logic on a background thread and posts the result
+     * back to the main thread through the provided {@link GenericCallback}.</p>
+     *
+     * <p>The check is based on {@link #isInternetWalled()} — if the Internet is not
+     * walled (i.e., the server is reachable and not restricted by a captive portal),
+     * this method reports {@code true}. Otherwise, it reports {@code false}.</p>
      *
-     * @param callback A callback to handle the result of the network and server availability check.
+     * @param callback a callback that receives {@code true} when the network and
+     *                 Nextcloud server are reachable, or {@code false} otherwise.
      */
     void isNetworkAndServerAvailable(@NonNull GenericCallback<Boolean> callback);
 
+    /**
+     * Checks whether the device currently has an active, validated Internet connection
+     * via a recognized transport type.
+     *
+     * <p>This method queries the Android {@link ConnectivityManager} to determine
+     * whether there is an active {@link Network} with Internet capability and an
+     * acceptable transport such as Wi-Fi, Cellular, Ethernet, VPN, or Bluetooth.</p>
+     *
+     * <p>For Android 12 (API 31) and newer, USB network transport is also considered valid.</p>
+     *
+     * <p>Note: This only confirms that the Android system has validated Internet access,
+     * not necessarily that the Nextcloud server itself is reachable.</p>
+     *
+     * @return {@code true} if the device is connected to the Internet through a supported
+     *         transport type; {@code false} otherwise.
+     */
     boolean isConnected();
 
     /**
-     * Check if server is accessible by issuing HTTP status check request.
-     * Since this call involves network traffic, it should not be called
-     * on a main thread.
+     * Determines whether the device's current Internet connection is "walled" — that is,
+     * restricted by a captive portal or other form of network access control that prevents
+     * full connectivity to the Nextcloud server.
+     *
+     * <p>This method does <strong>not</strong> test general Internet reachability (e.g. Google or DNS),
+     * but rather focuses on the ability to access the configured Nextcloud server directly.
+     * In other words, it checks whether the server can be reached without network interference
+     * such as a hotel's captive portal, Wi-Fi login page, or similar restrictions.</p>
+     *
+     * <p>The implementation performs the following steps:</p>
+     * <ul>
+     *     <li>Uses cached results from {@link WalledCheckCache} when available to avoid
+     *         redundant network calls.</li>
+     *     <li>Retrieves the active {@link Server} from {@link UserAccountManager}.</li>
+     *     <li>If connected issues a lightweight
+     *         HTTP {@code GET} request to the server’s <code>/index.php/204</code> endpoint
+     *         (which should respond with HTTP 204 No Content when connectivity is healthy).</li>
+     *     <li>If the response differs from the expected 204 No Content, the connection is
+     *         assumed to be behind a captive portal or otherwise restricted.</li>
+     *     <li>If no active network or server is detected, the method assumes the Internet
+     *         is walled.</li>
+     * </ul>
      *
-     * @return True if server is unreachable, false otherwise
+     * <p>Results are cached for subsequent checks to minimize unnecessary HTTP requests.</p>
+     *
+     * @return {@code true} if the Internet appears to be walled (e.g. captive portal or
+     *         restricted access); {@code false} if the Nextcloud server is reachable and
+     *         the network allows normal Internet access.
      */
     boolean isInternetWalled();
 
     /**
-     * Get current network connectivity status.
+     * Returns a {@link Connectivity} object that represents the current network state.
+     *
+     * <p>This includes whether the device is connected, whether the network is metered,
+     * and whether it uses Wi-Fi or Ethernet transport. It uses
+     * {@link #isConnected()} to verify active Internet capability</p>
+     *
+     * <p>If no active network is found, {@link Connectivity#DISCONNECTED} is returned.</p>
      *
-     * @return Network connectivity status in platform-agnostic format
+     * @return a {@link Connectivity} instance describing the current network connection.
      */
     Connectivity getConnectivity();
 

app/src/main/java/com/nextcloud/client/network/ConnectivityServiceImpl.java

@@ -6,15 +6,16 @@
  *
  * SPDX-License-Identifier: AGPL-3.0-or-later OR GPL-2.0-only
  */
-
 package com.nextcloud.client.network;
 
+import android.content.Context;
 import android.net.ConnectivityManager;
 import android.net.Network;
 import android.net.NetworkCapabilities;
 import android.os.Build;
 import android.os.Handler;
 import android.os.Looper;
+import android.text.TextUtils;
 
 import com.nextcloud.client.account.Server;
 import com.nextcloud.client.account.UserAccountManager;
@@ -28,154 +29,150 @@
 import java.util.concurrent.Executors;
 
 import androidx.annotation.NonNull;
-import kotlin.jvm.functions.Function1;
 
-class ConnectivityServiceImpl implements ConnectivityService {
+public 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 ConnectivityManager connectivityManager;
     private final UserAccountManager accountManager;
     private final ClientFactory clientFactory;
     private final GetRequestBuilder requestBuilder;
     private final WalledCheckCache walledCheckCache;
-    private final ExecutorService executor = Executors.newSingleThreadExecutor();
     private final Handler mainThreadHandler = new Handler(Looper.getMainLooper());
+    private final ExecutorService executor = Executors.newSingleThreadExecutor();
+    private Connectivity currentConnectivity = Connectivity.DISCONNECTED;
+
+    private final ConnectivityManager.NetworkCallback networkCallback = new ConnectivityManager.NetworkCallback() {
+        @Override
+        public void onAvailable(@NonNull Network network) {
+            updateConnectivity();
+        }
 
-    static class GetRequestBuilder implements Function1<String, GetMethod> {
+        @Override
+        public void onLost(@NonNull Network network) {
+            updateConnectivity();
+        }
+
+        @Override
+        public void onCapabilitiesChanged(@NonNull Network network, @NonNull NetworkCapabilities networkCapabilities) {
+            updateConnectivity();
+        }
+    };
+
+    static class GetRequestBuilder implements kotlin.jvm.functions.Function1<String, GetMethod> {
         @Override
         public GetMethod invoke(String url) {
             return new GetMethod(url, false);
         }
     }
 
-    ConnectivityServiceImpl(ConnectivityManager platformConnectivityManager,
-                            UserAccountManager accountManager,
-                            ClientFactory clientFactory,
-                            GetRequestBuilder requestBuilder,
-                            final WalledCheckCache walledCheckCache) {
-        this.platformConnectivityManager = platformConnectivityManager;
+    public ConnectivityServiceImpl(@NonNull Context context,
+                                   @NonNull UserAccountManager accountManager,
+                                   @NonNull ClientFactory clientFactory,
+                                   @NonNull GetRequestBuilder requestBuilder,
+                                   @NonNull WalledCheckCache walledCheckCache) {
+        this.connectivityManager = (ConnectivityManager) context.getSystemService(Context.CONNECTIVITY_SERVICE);
         this.accountManager = accountManager;
         this.clientFactory = clientFactory;
         this.requestBuilder = requestBuilder;
         this.walledCheckCache = walledCheckCache;
-    }
 
-    @Override
-    public void isNetworkAndServerAvailable(@NonNull GenericCallback<Boolean> callback) {
-        executor.submit(() -> {
-            boolean isAvailable = !isInternetWalled();
-            mainThreadHandler.post(() -> callback.onComplete(isAvailable));
-        });
+        // Register callback for real-time network updates
+        connectivityManager.registerDefaultNetworkCallback(networkCallback);
+        updateConnectivity();
     }
 
-    /**
-     * Checks whether the device is currently connected to a network
-     * that has verified Internet access.
-     *
-     * <p>This method performs multiple levels of validation:
-     * <ul>
-     *     <li>Ensures there is an active network connection.</li>
-     *     <li>Retrieves and checks network capabilities.</li>
-     *     <li>Verifies that the active network provides and has validated Internet access.</li>
-     *     <li>Confirms that the network uses a supported transport type
-     *         (Wi-Fi, Cellular, Ethernet, VPN, etc.).</li>
-     * </ul>
-     *
-     * @return {@code true} if the device is connected to the Internet via a valid transport type;
-     *         {@code false} otherwise.
-     */
-    @Override
-    public boolean isConnected() {
-        Network nw = platformConnectivityManager.getActiveNetwork();
-        if (nw == null) {
-            return false;
+    private void updateConnectivity() {
+        Network activeNetwork = connectivityManager.getActiveNetwork();
+        if (activeNetwork == null) {
+            currentConnectivity = Connectivity.DISCONNECTED;
+            return;
         }
 
-        NetworkCapabilities actNw = platformConnectivityManager.getNetworkCapabilities(nw);
-        if (actNw == null) {
-            return false;
+        NetworkCapabilities capabilities = connectivityManager.getNetworkCapabilities(activeNetwork);
+        if (capabilities == null) {
+            currentConnectivity = Connectivity.DISCONNECTED;
+            return;
         }
 
-        // Verify that the network both claims to provide Internet
-        // and has been validated (i.e., Internet is actually reachable).
-        if (actNw.hasCapability(NetworkCapabilities.NET_CAPABILITY_INTERNET) && actNw.hasCapability(NetworkCapabilities.NET_CAPABILITY_VALIDATED)) {
+        boolean isConnected = capabilities.hasCapability(NetworkCapabilities.NET_CAPABILITY_INTERNET) || isSupportedTransport(capabilities);
+        boolean isMetered = !capabilities.hasCapability(NetworkCapabilities.NET_CAPABILITY_NOT_RESTRICTED);
+        boolean isWifi = capabilities.hasTransport(NetworkCapabilities.TRANSPORT_WIFI)
+            || capabilities.hasTransport(NetworkCapabilities.TRANSPORT_ETHERNET);
 
-            // Check if the active network uses one of the recognized transport types.
-            if (actNw.hasTransport(NetworkCapabilities.TRANSPORT_WIFI) ||
-                actNw.hasTransport(NetworkCapabilities.TRANSPORT_CELLULAR) ||
-                actNw.hasTransport(NetworkCapabilities.TRANSPORT_ETHERNET) ||
-                actNw.hasTransport(NetworkCapabilities.TRANSPORT_VPN) ||
-                actNw.hasTransport(NetworkCapabilities.TRANSPORT_BLUETOOTH) ||
-                actNw.hasTransport(NetworkCapabilities.TRANSPORT_WIFI_AWARE)) {
+        currentConnectivity = new Connectivity(isConnected, isMetered, isWifi, null);
+    }
 
-                // Connected through a valid, verified network transport.
-                return true;
-            }
+    private boolean isSupportedTransport(@NonNull NetworkCapabilities capabilities) {
+        return capabilities.hasTransport(NetworkCapabilities.TRANSPORT_WIFI) ||
+            capabilities.hasTransport(NetworkCapabilities.TRANSPORT_CELLULAR) ||
+            capabilities.hasTransport(NetworkCapabilities.TRANSPORT_ETHERNET) ||
+            capabilities.hasTransport(NetworkCapabilities.TRANSPORT_VPN) ||
+            capabilities.hasTransport(NetworkCapabilities.TRANSPORT_BLUETOOTH) ||
+            capabilities.hasTransport(NetworkCapabilities.TRANSPORT_WIFI_AWARE) ||
+            (Build.VERSION.SDK_INT >= Build.VERSION_CODES.S &&
+                capabilities.hasTransport(NetworkCapabilities.TRANSPORT_USB));
+    }
 
-            // If still nothing matched check Android 12 (API 31, "S") and above via USB network transport.
-            return Build.VERSION.SDK_INT >= Build.VERSION_CODES.S &&
-                actNw.hasTransport(NetworkCapabilities.TRANSPORT_USB);
-        }
+    @Override
+    public void isNetworkAndServerAvailable(@NonNull GenericCallback<Boolean> callback) {
+        executor.execute(() -> {
+            boolean available = !isInternetWalled();
+            mainThreadHandler.post(() -> callback.onComplete(available));
+        });
+    }
 
-        return false;
+    @Override
+    public boolean isConnected() {
+        return currentConnectivity.isConnected();
     }
 
     @Override
     public boolean isInternetWalled() {
-        final Boolean cachedValue = walledCheckCache.getValue();
-        if (cachedValue != null) {
-            return cachedValue;
+        Boolean cached = walledCheckCache.getValue();
+        if (cached != null) {
+            return cached;
         }
 
-        final Server server = accountManager.getUser().getServer();
-        final String baseServerAddress = server.getUri().toString();
+        Server server = accountManager.getUser().getServer();
+        String baseServerAddress = server.getUri().toString();
 
-        if (!isConnected() || baseServerAddress.isEmpty()) {
+        if (!currentConnectivity.isConnected() || TextUtils.isEmpty(baseServerAddress)) {
             walledCheckCache.setValue(true);
             return true;
         }
 
-        final GetMethod get = requestBuilder.invoke(baseServerAddress + CONNECTIVITY_CHECK_ROUTE);
+        boolean isWalled;
+        GetMethod get = requestBuilder.invoke(baseServerAddress + CONNECTIVITY_CHECK_ROUTE);
+        PlainClient client = clientFactory.createPlainClient();
+
         try {
-            final PlainClient client = clientFactory.createPlainClient();
             int status = get.execute(client);
 
-            boolean isWalled = !(status == HttpStatus.SC_NO_CONTENT && get.getResponseContentLength() <= 0);
-
+            // Server is reachable and responds correctly = NOT walled
+            isWalled = !(status == HttpStatus.SC_NO_CONTENT && get.getResponseContentLength() <= 0);
             if (isWalled) {
-                Log_OC.w(TAG, "isInternetWalled(): Failed to GET " + CONNECTIVITY_CHECK_ROUTE +
-                    ", assuming connectivity is impaired");
+                Log_OC.w(TAG, "isInternetWalled(): Server returned unexpected response");
             }
-
-            // Cache and return result
-            walledCheckCache.setValue(isWalled);
-            return isWalled;
         } catch (Exception e) {
-            Log_OC.e(TAG, "Exception while checking internet walled state", e);
-            walledCheckCache.setValue(true);
-            return true;
+            Log_OC.e(TAG, "isInternetWalled(): Exception during server check", e);
+            isWalled = true;
         } finally {
             get.releaseConnection();
         }
+
+        walledCheckCache.setValue(isWalled);
+        return isWalled;
     }
 
     @Override
     public Connectivity getConnectivity() {
-        Network nw = platformConnectivityManager.getActiveNetwork();
-        if (nw == null) {
-            return Connectivity.DISCONNECTED;
-        }
-
-        NetworkCapabilities nc = platformConnectivityManager.getNetworkCapabilities(nw);
-        boolean isConnected = isConnected();
-        boolean isMetered = (nc != null) && !nc.hasCapability(NetworkCapabilities.NET_CAPABILITY_NOT_METERED);
-        boolean isWifi = (nc != null) &&
-            (nc.hasTransport(NetworkCapabilities.TRANSPORT_WIFI) ||
-                nc.hasTransport(NetworkCapabilities.TRANSPORT_ETHERNET) ||
-                nc.hasTransport(NetworkCapabilities.TRANSPORT_WIFI_AWARE));
+        return currentConnectivity;
+    }
 
-        return new Connectivity(isConnected, isMetered, isWifi, null);
+    public void unregisterCallback() {
+        connectivityManager.unregisterNetworkCallback(networkCallback);
     }
 }