apply changes according to review

owncloudComLibrary/build.gradle

@@ -7,8 +7,6 @@ dependencies {
     implementation "org.jetbrains.kotlin:kotlin-stdlib-jdk8:$kotlinVersion"
     api 'com.gitlab.ownclouders:dav4android:oc_support_2.1.5'
     api 'com.github.AppDevNext.Logcat:LogcatCore:2.2.2'
-    debugImplementation 'com.facebook.stetho:stetho:1.5.1'
-    debugImplementation 'com.facebook.stetho:stetho-okhttp3:1.5.1'
 
     // Moshi
     implementation("com.squareup.moshi:moshi-kotlin:$moshiVersion") {

owncloudComLibrary/src/main/java/com/owncloud/android/lib/common/ConnectionValidator.kt

@@ -19,7 +19,6 @@ class ConnectionValidator(
     val context: Context,
     val clearCookiesOnValidation: Boolean
 ) {
-
     fun validate(baseClient: OwnCloudClient, singleSessionManager: SingleSessionManager): Boolean {
         try {
             var validationRetryCount = 0
@@ -182,6 +181,6 @@ class ConnectionValidator(
     }
 
     companion object {
-        val VALIDATION_RETRY_COUNT = 3
+        private val VALIDATION_RETRY_COUNT = 3
     }
 }

owncloudComLibrary/src/main/java/com/owncloud/android/lib/common/OwnCloudClient.java

@@ -27,7 +27,6 @@ package com.owncloud.android.lib.common;
 
 import android.net.Uri;
 
-import at.bitfire.dav4jvm.exception.HttpException;
 import com.owncloud.android.lib.common.accounts.AccountUtils;
 import com.owncloud.android.lib.common.authentication.OwnCloudCredentials;
 import com.owncloud.android.lib.common.authentication.OwnCloudCredentialsFactory;
@@ -35,12 +34,10 @@ import com.owncloud.android.lib.common.authentication.OwnCloudCredentialsFactory
 import com.owncloud.android.lib.common.http.HttpClient;
 import com.owncloud.android.lib.common.http.HttpConstants;
 import com.owncloud.android.lib.common.http.methods.HttpBaseMethod;
-import com.owncloud.android.lib.common.network.RedirectionPath;
 import com.owncloud.android.lib.common.utils.RandomUtils;
 import com.owncloud.android.lib.resources.status.OwnCloudVersion;
 import okhttp3.Cookie;
 import okhttp3.HttpUrl;
-import org.apache.commons.lang3.exception.ExceptionUtils;
 import timber.log.Timber;
 
 import java.io.IOException;
@@ -49,7 +46,6 @@ import java.util.List;
 
 import static com.owncloud.android.lib.common.http.HttpConstants.AUTHORIZATION_HEADER;
 import static com.owncloud.android.lib.common.http.HttpConstants.HTTP_MOVED_PERMANENTLY;
-import static com.owncloud.android.lib.common.http.HttpConstants.OC_X_REQUEST_ID;
 
 public class OwnCloudClient extends HttpClient {
 
@@ -137,12 +133,7 @@ public class OwnCloudClient extends HttpClient {
 
             status = method.execute();
 
-            if (!mFollowRedirects &&
+            if (shouldConnectionValidatorBeCalled(method, status)) {
-                    !method.getFollowRedirects() &&
-                    mConnectionValidator != null &&
-                    (status == HttpConstants.HTTP_MOVED_TEMPORARILY ||
-                            (!(mCredentials instanceof OwnCloudAnonymousCredentials) &&
-                                    status == HttpConstants.HTTP_UNAUTHORIZED))) {
                 retry = mConnectionValidator.validate(this, mSingleSessionManager); // retry on success fail on no success
             } else if(method.getFollowPermanentRedirects() && status == HTTP_MOVED_PERMANENTLY) {
                 retry = true;
@@ -154,6 +145,15 @@ public class OwnCloudClient extends HttpClient {
         return status;
     }
 
+    private boolean shouldConnectionValidatorBeCalled(HttpBaseMethod method, int status) {
+        return !mFollowRedirects &&
+                !method.getFollowRedirects() &&
+                mConnectionValidator != null &&
+                (status == HttpConstants.HTTP_MOVED_TEMPORARILY ||
+                        (!(mCredentials instanceof OwnCloudAnonymousCredentials) &&
+                                status == HttpConstants.HTTP_UNAUTHORIZED));
+    }
+
     /**
      * Exhausts a not interesting HTTP response. Encouraged by HttpClient documentation.
      *
@@ -250,10 +250,6 @@ public class OwnCloudClient extends HttpClient {
         this.mAccount = account;
     }
 
-    public boolean getFollowRedirects() {
-        return mFollowRedirects;
-    }
-
     public void setFollowRedirects(boolean followRedirects) {
         this.mFollowRedirects = followRedirects;
     }

owncloudComLibrary/src/main/java/com/owncloud/android/lib/common/http/methods/HttpBaseMethod.kt

@@ -166,7 +166,7 @@ abstract class HttpBaseMethod constructor(url: URL) {
 
     open fun getFollowRedirects() = okHttpClient.followRedirects
 
-    open fun setFollPermanentRedirects(followRedirects: Boolean) {
+    open fun setFollowPermanentRedirects(followRedirects: Boolean) {
         _followPermanentRedirects = followRedirects
     }
 

owncloudComLibrary/src/main/java/com/owncloud/android/lib/common/operations/RemoteOperationResult.java

@@ -60,6 +60,8 @@ public class RemoteOperationResult<T>
      * Generated - should be refreshed every time the class changes!!
      */
     private static final long serialVersionUID = 4968939884332372230L;
+    private static final String LOCATION = "location";
+    private static final String WWW_AUTHENTICATE = "www-authenticate";
 
     private boolean mSuccess = false;
     private int mHttpCode = -1;
@@ -257,11 +259,11 @@ public class RemoteOperationResult<T>
         this(httpCode, httpPhrase);
         if (headers != null) {
             for (Map.Entry<String, List<String>> header : headers.toMultimap().entrySet()) {
-                if ("location".equalsIgnoreCase(header.getKey())) {
+                if (LOCATION.equalsIgnoreCase(header.getKey())) {
                     mRedirectedLocation = header.getValue().get(0);
                     continue;
                 }
-                if ("www-authenticate".equalsIgnoreCase(header.getKey())) {
+                if (WWW_AUTHENTICATE.equalsIgnoreCase(header.getKey())) {
                     for (String value: header.getValue()) {
                         mAuthenticate.add(value.toLowerCase());
                     }

owncloudComLibrary/src/main/java/com/owncloud/android/lib/resources/files/GetBaseUrlRemoteOperation.kt

@@ -45,16 +45,21 @@ class GetBaseUrlRemoteOperation : RemoteOperation<String?>() {
             val stringUrl = client.baseFilesWebDavUri.toString()
 
             val propFindMethod = PropfindMethod(URL(stringUrl), 0, DavUtils.allPropset).apply {
-                setReadTimeout(TIMEOUT.toLong(), TimeUnit.SECONDS)
+                setReadTimeout(TIMEOUT, TimeUnit.SECONDS)
-                setConnectionTimeout(TIMEOUT.toLong(), TimeUnit.SECONDS)
+                setConnectionTimeout(TIMEOUT, TimeUnit.SECONDS)
             }
 
             val status = client.executeHttpMethod(propFindMethod)
 
-            if (isSuccess(status)) RemoteOperationResult<String?>(RemoteOperationResult.ResultCode.OK).apply {
+            if (isSuccess(status)) {
+                RemoteOperationResult<String?>(RemoteOperationResult.ResultCode.OK).apply {
                     data = propFindMethod.getFinalUrl().toString()
                 }
-            else RemoteOperationResult<String?>(propFindMethod).apply { data = null }
+            } else {
+                RemoteOperationResult<String?>(propFindMethod).apply {
+                    data = null
+                }
+            }
         } catch (e: Exception) {
             Timber.e(e, "Could not get actuall (or redirected) base URL from base url (/).")
             RemoteOperationResult<String?>(e)
@@ -67,6 +72,6 @@ class GetBaseUrlRemoteOperation : RemoteOperation<String?>() {
         /**
          * Maximum time to wait for a response from the server in milliseconds.
          */
-        private const val TIMEOUT = 10000
+        private const val TIMEOUT = 10_000L
     }
 }

owncloudComLibrary/src/main/java/com/owncloud/android/lib/resources/status/StatusRequester.kt

@@ -73,20 +73,18 @@ internal class StatusRequester {
     data class RequestResult(
         val getMethod: GetMethod,
         val status: Int,
-        val redirectedToUnsecureLocation: Boolean,
         val lastLocation: String
     )
 
     fun request(baseLocation: String, client: OwnCloudClient): RequestResult {
-        var currentLocation = baseLocation + OwnCloudClient.STATUS_PATH
+        val currentLocation = baseLocation + OwnCloudClient.STATUS_PATH
-        var redirectedToUnsecureLocation = false
         var status: Int
 
         val getMethod = getGetMethod(currentLocation)
-        getMethod.setFollPermanentRedirects(true)
+        getMethod.setFollowPermanentRedirects(true)
         status = client.executeHttpMethod(getMethod)
 
-        return RequestResult(getMethod, status, redirectedToUnsecureLocation, getMethod.getFinalUrl().toString())
+        return RequestResult(getMethod, status, getMethod.getFinalUrl().toString())
     }
 
     private fun Int.isSuccess() = this == HttpConstants.HTTP_OK
@@ -106,12 +104,8 @@ internal class StatusRequester {
         // the version object will be returned even if the version is invalid, no error code;
         // every app will decide how to act if (ocVersion.isVersionValid() == false)
         val result: RemoteOperationResult<RemoteServerInfo> =
-            if (requestResult.redirectedToUnsecureLocation) {
-                RemoteOperationResult(RemoteOperationResult.ResultCode.OK_REDIRECT_TO_NON_SECURE_CONNECTION)
-            } else {
             if (baseUrl.startsWith(HTTPS_SCHEME)) RemoteOperationResult(RemoteOperationResult.ResultCode.OK_SSL)
             else RemoteOperationResult(RemoteOperationResult.ResultCode.OK_NO_SSL)
-            }
         val finalUrl = URL(requestResult.lastLocation)
         val finalBaseUrl = URL(
             finalUrl.protocol,