Merge pull request #7207 from nextcloud/newGet

Use new GetMethod, drop apache one

scripts/analysis/findbugs-results.txt

@@ -1 +1 @@
-310
+309

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

@@ -0,0 +1,53 @@
+/*
+ *
+ * Nextcloud Android client application
+ *
+ * @author Tobias Kaminsky
+ * Copyright (C) 2020 Tobias Kaminsky
+ * Copyright (C) 2020 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 as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) 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 <https://www.gnu.org/licenses/>.
+ */
+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.network.ConnectivityServiceImpl.GetRequestBuilder
+import com.owncloud.android.AbstractOnServerIT
+import org.junit.Assert.assertFalse
+import org.junit.Assert.assertTrue
+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 sut = ConnectivityServiceImpl(
+            connectivityManager,
+            userAccountManager,
+            clientFactory,
+            requestBuilder
+        )
+
+        assertTrue(sut.connectivity.isConnected)
+        assertFalse(sut.isInternetWalled)
+    }
+}

src/main/java/com/nextcloud/client/network/ClientFactory.java

@@ -28,21 +28,19 @@ import android.net.Uri;
 
 import com.nextcloud.client.account.User;
 import com.nextcloud.common.NextcloudClient;
+import com.nextcloud.common.PlainClient;
 import com.owncloud.android.lib.common.OwnCloudClient;
 import com.owncloud.android.lib.common.accounts.AccountUtils;
 
-import org.apache.commons.httpclient.HttpClient;
-
 import java.io.IOException;
 
 public interface ClientFactory {
 
     /**
-     * This exception wraps all possible errors thrown by trigger-happy
-     * OwnCloudClient constructor, making try-catch blocks manageable.
-     *
-     * This is a temporary refactoring measure, until a better
-     * error handling method can be procured.
+     * This exception wraps all possible errors thrown by trigger-happy OwnCloudClient constructor, making try-catch
+     * blocks manageable.
+     * <p>
+     * This is a temporary refactoring measure, until a better error handling method can be procured.
      */
     @Deprecated
     class CreationException extends Exception {
@@ -74,5 +72,5 @@ public interface ClientFactory {
 
     OwnCloudClient create(Uri uri, boolean followRedirects);
 
-    HttpClient createPlainClient();
+    PlainClient createPlainClient();
 }

src/main/java/com/nextcloud/client/network/ClientFactoryImpl.java

@@ -29,6 +29,7 @@ import android.net.Uri;
 
 import com.nextcloud.client.account.User;
 import com.nextcloud.common.NextcloudClient;
+import com.nextcloud.common.PlainClient;
 import com.owncloud.android.lib.common.OwnCloudClient;
 import com.owncloud.android.lib.common.OwnCloudClientFactory;
 import com.owncloud.android.lib.common.OwnCloudClientManagerFactory;
@@ -89,7 +90,7 @@ class ClientFactoryImpl implements ClientFactory {
     }
 
     @Override
-    public PlainHttpClient createPlainClient() {
-        return new PlainHttpClient();
+    public PlainClient createPlainClient() {
+        return new PlainClient(context);
     }
 }

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

@@ -26,44 +26,36 @@ import android.net.NetworkInfo;
 
 import com.nextcloud.client.account.Server;
 import com.nextcloud.client.account.UserAccountManager;
-import com.nextcloud.client.logger.Logger;
+import com.nextcloud.common.PlainClient;
+import com.nextcloud.operations.GetMethod;
 
-import org.apache.commons.httpclient.HttpClient;
 import org.apache.commons.httpclient.HttpStatus;
-import org.apache.commons.httpclient.methods.GetMethod;
-
-import java.io.IOException;
 
 import androidx.core.net.ConnectivityManagerCompat;
 import kotlin.jvm.functions.Function1;
 
 class ConnectivityServiceImpl implements ConnectivityService {
 
-    private final static String TAG = ConnectivityServiceImpl.class.getName();
-
     private final ConnectivityManager platformConnectivityManager;
     private final UserAccountManager accountManager;
     private final ClientFactory clientFactory;
     private final GetRequestBuilder requestBuilder;
-    private final Logger logger;
 
     static class GetRequestBuilder implements Function1<String, GetMethod> {
         @Override
         public GetMethod invoke(String url) {
-            return new GetMethod(url);
+            return new GetMethod(url, false);
         }
     }
 
     ConnectivityServiceImpl(ConnectivityManager platformConnectivityManager,
                             UserAccountManager accountManager,
                             ClientFactory clientFactory,
-                            GetRequestBuilder requestBuilder,
-                            Logger logger) {
+                            GetRequestBuilder requestBuilder) {
         this.platformConnectivityManager = platformConnectivityManager;
         this.accountManager = accountManager;
         this.clientFactory = clientFactory;
         this.requestBuilder = requestBuilder;
-        this.logger = logger;
     }
 
     @Override
@@ -71,33 +63,25 @@ class ConnectivityServiceImpl implements ConnectivityService {
         Connectivity c = getConnectivity();
         if (c.isConnected() && c.isWifi()) {
 
-            GetMethod get = null;
-            try {
-                Server server = accountManager.getUser().getServer();
-                String baseServerAddress = server.getUri().toString();
-                if (baseServerAddress.isEmpty()) {
-                    return true;
-                }
+            Server server = accountManager.getUser().getServer();
+            String baseServerAddress = server.getUri().toString();
+            if (baseServerAddress.isEmpty()) {
+                return true;
+            }
 
-                get = requestBuilder.invoke(baseServerAddress + "/index.php/204");
-                HttpClient client = clientFactory.createPlainClient();
+            GetMethod get = requestBuilder.invoke(baseServerAddress + "/index.php/204");
+            PlainClient client = clientFactory.createPlainClient();
 
-                int status = client.executeMethod(get);
+            int status = get.execute(client);
 
-                return !(status == HttpStatus.SC_NO_CONTENT &&
-                    (get.getResponseContentLength() == -1 || get.getResponseContentLength() == 0));
-            } catch (IOException e) {
-                logger.e(TAG, "Error checking internet connection", e);
-            } finally {
-                if (get != null) {
-                    get.releaseConnection();
-                }
-            }
+            boolean result = !(status == HttpStatus.SC_NO_CONTENT && get.getResponseContentLength() == 0);
+
+            get.releaseConnection();
+
+            return result;
         } else {
             return !getConnectivity().isConnected();
         }
-
-        return true;
     }
 
     @Override

src/main/java/com/nextcloud/client/network/NetworkModule.java

@@ -24,7 +24,6 @@ import android.content.Context;
 import android.net.ConnectivityManager;
 
 import com.nextcloud.client.account.UserAccountManager;
-import com.nextcloud.client.logger.Logger;
 
 import javax.inject.Singleton;
 
@@ -37,13 +36,12 @@ public class NetworkModule {
     @Provides
     ConnectivityService connectivityService(ConnectivityManager connectivityManager,
                                             UserAccountManager accountManager,
-                                            ClientFactory clientFactory,
-                                            Logger logger) {
+                                            ClientFactory clientFactory) {
         return new ConnectivityServiceImpl(connectivityManager,
                                            accountManager,
                                            clientFactory,
-                                           new ConnectivityServiceImpl.GetRequestBuilder(),
-                                           logger);
+                                           new ConnectivityServiceImpl.GetRequestBuilder()
+        );
     }
 
     @Provides

src/main/java/com/nextcloud/client/network/PlainHttpClient.java

@@ -1,42 +0,0 @@
-/*
- *
- *  * Nextcloud Android client application
- *  *
- *  * @author Tobias Kaminsky
- *  * Copyright (C) 2019 Tobias Kaminsky
- *  * Copyright (C) 2019 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 as published by
- *  * the Free Software Foundation, either version 3 of the License, or
- *  * (at your option) 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 <https://www.gnu.org/licenses/>.
- *
- */
-
-package com.nextcloud.client.network;
-
-import com.owncloud.android.lib.common.OwnCloudClientManagerFactory;
-
-import org.apache.commons.httpclient.HttpClient;
-import org.apache.commons.httpclient.HttpMethod;
-import org.apache.commons.httpclient.params.HttpMethodParams;
-
-import java.io.IOException;
-
-public class PlainHttpClient extends HttpClient {
-
-    @Override
-    public int executeMethod(HttpMethod method) throws IOException {
-        method.getParams().setParameter(HttpMethodParams.USER_AGENT, OwnCloudClientManagerFactory.getUserAgent());
-
-        return super.executeMethod(method);
-    }
-}

src/test/java/com/nextcloud/client/network/ConnectivityServiceTest.kt

@@ -25,15 +25,15 @@ import com.nextcloud.client.account.Server
 import com.nextcloud.client.account.User
 import com.nextcloud.client.account.UserAccountManager
 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.mock
 import com.nhaarman.mockitokotlin2.never
 import com.nhaarman.mockitokotlin2.verify
 import com.nhaarman.mockitokotlin2.whenever
 import com.owncloud.android.lib.resources.status.OwnCloudVersion
-import org.apache.commons.httpclient.HttpClient
 import org.apache.commons.httpclient.HttpStatus
-import org.apache.commons.httpclient.methods.GetMethod
 import org.junit.Assert.assertFalse
 import org.junit.Assert.assertSame
 import org.junit.Assert.assertTrue
@@ -81,7 +81,7 @@ class ConnectivityServiceTest {
         lateinit var clientFactory: ClientFactory
 
         @Mock
-        lateinit var client: HttpClient
+        lateinit var client: PlainClient
 
         @Mock
         lateinit var getRequest: GetMethod
@@ -108,8 +108,7 @@ class ConnectivityServiceTest {
                 platformConnectivityManager,
                 accountManager,
                 clientFactory,
-                requestBuilder,
-                logger
+                requestBuilder
             )
 
             whenever(platformConnectivityManager.activeNetworkInfo).thenReturn(networkInfo)
@@ -200,11 +199,11 @@ class ConnectivityServiceTest {
         }
 
         fun mockResponse(maintenance: Boolean = true, httpStatus: Int = HttpStatus.SC_OK) {
-            whenever(client.executeMethod(getRequest)).thenReturn(httpStatus)
+            whenever(client.execute(getRequest)).thenReturn(httpStatus)
             val body =
                 """{"maintenance":$maintenance}"""
-            whenever(getRequest.responseContentLength).thenReturn(body.length.toLong())
-            whenever(getRequest.responseBodyAsString).thenReturn(body)
+            whenever(getRequest.getResponseContentLength()).thenReturn(body.length.toLong())
+            whenever(getRequest.getResponseBodyAsString()).thenReturn(body)
         }
 
         @Test
@@ -261,15 +260,14 @@ class ConnectivityServiceTest {
             //      request is not sent
             assertTrue("Server should not be accessible", result)
             verify(requestBuilder, never()).invoke(any())
-            verify(client, never()).executeMethod(any())
-            verify(client, never()).executeMethod(any(), any())
-            verify(client, never()).executeMethod(any(), any(), any())
+            verify(client, never()).execute(any())
         }
 
         fun mockResponse(contentLength: Long = 0, status: Int = HttpStatus.SC_OK) {
-            whenever(client.executeMethod(any())).thenReturn(status)
-            whenever(getRequest.statusCode).thenReturn(status)
-            whenever(getRequest.responseContentLength).thenReturn(contentLength)
+            whenever(client.execute(any())).thenReturn(status)
+            whenever(getRequest.getStatusCode()).thenReturn(status)
+            whenever(getRequest.getResponseContentLength()).thenReturn(contentLength)
+            whenever(getRequest.execute(client)).thenReturn(status)
         }
 
         @Test