From 94f69af7d3420f2eb77d75e6e616d789e5df7f4d Mon Sep 17 00:00:00 2001 From: Christian Schabesberger Date: Wed, 16 Sep 2020 16:20:54 +0200 Subject: [PATCH] detach request logic from operation --- .../status/GetRemoteStatusOperation.kt | 114 ++---------------- .../lib/resources/status/StatusRequestor.kt | 113 +++++++++++++++++ ...perationTest.kt => StatusRequestorTest.kt} | 16 +-- 3 files changed, 130 insertions(+), 113 deletions(-) create mode 100644 owncloudComLibrary/src/main/java/com/owncloud/android/lib/resources/status/StatusRequestor.kt rename owncloudComLibrary/src/test/java/com/owncloud/android/lib/{GetRemoteStatusOperationTest.kt => StatusRequestorTest.kt} (66%) diff --git a/owncloudComLibrary/src/main/java/com/owncloud/android/lib/resources/status/GetRemoteStatusOperation.kt b/owncloudComLibrary/src/main/java/com/owncloud/android/lib/resources/status/GetRemoteStatusOperation.kt index ab344524..e9c1c10b 100644 --- a/owncloudComLibrary/src/main/java/com/owncloud/android/lib/resources/status/GetRemoteStatusOperation.kt +++ b/owncloudComLibrary/src/main/java/com/owncloud/android/lib/resources/status/GetRemoteStatusOperation.kt @@ -25,16 +25,12 @@ package com.owncloud.android.lib.resources.status import android.net.Uri import com.owncloud.android.lib.common.OwnCloudClient -import com.owncloud.android.lib.common.http.HttpConstants -import com.owncloud.android.lib.common.http.methods.nonwebdav.GetMethod import com.owncloud.android.lib.common.operations.RemoteOperation import com.owncloud.android.lib.common.operations.RemoteOperationResult import com.owncloud.android.lib.common.operations.RemoteOperationResult.ResultCode import org.json.JSONException -import org.json.JSONObject import timber.log.Timber -import java.net.URL -import java.util.concurrent.TimeUnit + /** * Checks if the server is valid @@ -45,6 +41,11 @@ import java.util.concurrent.TimeUnit * @author Abel García de Prada */ class GetRemoteStatusOperation : RemoteOperation() { + companion object { + const val HTTPS_SCHEME = "https" + const val HTTP_SCHEME = "http" + } + override fun run(client: OwnCloudClient): RemoteOperationResult { if (client.baseUri.scheme.isNullOrEmpty()) client.baseUri = Uri.parse(HTTPS_SCHEME + "://" + client.baseUri.toString()) @@ -59,114 +60,17 @@ class GetRemoteStatusOperation : RemoteOperation() { return result } - fun updateLocationWithRedirectPath(oldLocation: String, redirectedLocation: String): String { - if (!redirectedLocation.startsWith("/")) - return redirectedLocation - val oldLocation = URL(oldLocation) - return URL(oldLocation.protocol, oldLocation.host, oldLocation.port, redirectedLocation).toString() - } - - private fun checkIfConnectionIsRedirectedToNoneSecure( - isConnectionSecure: Boolean, - baseUrl: String, - redirectedUrl: String - ): Boolean { - return isConnectionSecure || - (baseUrl.startsWith(HTTPS_SCHEME) && redirectedUrl.startsWith(HTTP_SCHEME)) - } - - private fun getGetMethod(url: String): GetMethod { - return GetMethod(URL(url + OwnCloudClient.STATUS_PATH)).apply { - setReadTimeout(TRY_CONNECTION_TIMEOUT, TimeUnit.SECONDS) - setConnectionTimeout(TRY_CONNECTION_TIMEOUT, TimeUnit.SECONDS) - } - } - - data class RequestResult( - val getMethod: GetMethod, - val status: Int, - val result: RemoteOperationResult, - val redirectedToUnsecureLocation: Boolean - ) - - fun requestAndFollowRedirects(baseLocation: String): RequestResult { - var currentLocation = baseLocation - var redirectedToUnsecureLocation = false - var status: Int - - while (true) { - val getMethod = getGetMethod(currentLocation) - - status = client.executeHttpMethod(getMethod) - val result = - if (isSuccess(status)) RemoteOperationResult(ResultCode.OK) - else RemoteOperationResult(getMethod) - - if (result.redirectedLocation.isNullOrEmpty() || result.isSuccess) { - return RequestResult(getMethod, status, result, redirectedToUnsecureLocation) - } else { - val nextLocation = updateLocationWithRedirectPath(currentLocation, result.redirectedLocation) - redirectedToUnsecureLocation = - checkIfConnectionIsRedirectedToNoneSecure( - redirectedToUnsecureLocation, - currentLocation, - nextLocation - ) - currentLocation = nextLocation - } - } - } - - private fun handleRequestResult( - requestResult: RequestResult, - baseUrl: String - ): RemoteOperationResult { - if (!isSuccess(requestResult.status)) - return RemoteOperationResult(requestResult.getMethod) - - val respJSON = JSONObject(requestResult.getMethod.getResponseBodyAsString()) - if (!respJSON.getBoolean(NODE_INSTALLED)) - return RemoteOperationResult(ResultCode.INSTANCE_NOT_CONFIGURED) - - val version = respJSON.getString(NODE_VERSION) - val ocVersion = OwnCloudVersion(version) - // 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 = - if (requestResult.redirectedToUnsecureLocation) { - RemoteOperationResult(ResultCode.OK_REDIRECT_TO_NON_SECURE_CONNECTION) - } else { - if (baseUrl.startsWith(HTTPS_SCHEME)) RemoteOperationResult(ResultCode.OK_SSL) - else RemoteOperationResult(ResultCode.OK_NO_SSL) - } - result.data = ocVersion - return result - } - private fun tryToConnect(client: OwnCloudClient): RemoteOperationResult { val baseUrl = client.baseUri.toString() client.setFollowRedirects(false) return try { - val requestResult = requestAndFollowRedirects(baseUrl) - handleRequestResult(requestResult, baseUrl) + val requestor = StatusRequestor() + val requestResult = requestor.requestAndFollowRedirects(baseUrl, client) + requestor.handleRequestResult(requestResult, baseUrl) } catch (e: JSONException) { RemoteOperationResult(ResultCode.INSTANCE_NOT_CONFIGURED) } catch (e: Exception) { RemoteOperationResult(e) } } - - private fun isSuccess(status: Int): Boolean = status == HttpConstants.HTTP_OK - - companion object { - /** - * Maximum time to wait for a response from the server when the connection is being tested, - * in MILLISECONDs. - */ - private const val TRY_CONNECTION_TIMEOUT: Long = 5000 - private const val NODE_INSTALLED = "installed" - private const val NODE_VERSION = "version" - private const val HTTPS_SCHEME = "https" - private const val HTTP_SCHEME = "http" - } } diff --git a/owncloudComLibrary/src/main/java/com/owncloud/android/lib/resources/status/StatusRequestor.kt b/owncloudComLibrary/src/main/java/com/owncloud/android/lib/resources/status/StatusRequestor.kt new file mode 100644 index 00000000..c36c3426 --- /dev/null +++ b/owncloudComLibrary/src/main/java/com/owncloud/android/lib/resources/status/StatusRequestor.kt @@ -0,0 +1,113 @@ +package com.owncloud.android.lib.resources.status + +import com.owncloud.android.lib.common.OwnCloudClient +import com.owncloud.android.lib.common.http.HttpConstants +import com.owncloud.android.lib.common.http.methods.nonwebdav.GetMethod +import com.owncloud.android.lib.common.operations.RemoteOperationResult +import org.json.JSONObject +import java.net.URL +import java.util.concurrent.TimeUnit + +internal class StatusRequestor { + + companion object { + /** + * Maximum time to wait for a response from the server when the connection is being tested, + * in MILLISECONDs. + */ + private const val TRY_CONNECTION_TIMEOUT: Long = 5000 + private const val NODE_INSTALLED = "installed" + private const val HTTPS_SCHEME = "https" + private const val HTTP_SCHEME = "http" + private const val NODE_VERSION = "version" + } + + private fun checkIfConnectionIsRedirectedToNoneSecure( + isConnectionSecure: Boolean, + baseUrl: String, + redirectedUrl: String + ): Boolean { + return isConnectionSecure || + (baseUrl.startsWith(HTTPS_SCHEME) && redirectedUrl.startsWith( + HTTP_SCHEME + )) + } + + fun updateLocationWithRedirectPath(oldLocation: String, redirectedLocation: String): String { + if (!redirectedLocation.startsWith("/")) + return redirectedLocation + val oldLocation = URL(oldLocation) + return URL(oldLocation.protocol, oldLocation.host, oldLocation.port, redirectedLocation).toString() + } + + private fun getGetMethod(url: String): GetMethod { + return GetMethod(URL(url + OwnCloudClient.STATUS_PATH)).apply { + setReadTimeout(TRY_CONNECTION_TIMEOUT, TimeUnit.SECONDS) + setConnectionTimeout(TRY_CONNECTION_TIMEOUT, TimeUnit.SECONDS) + } + } + + data class RequestResult( + val getMethod: GetMethod, + val status: Int, + val result: RemoteOperationResult, + val redirectedToUnsecureLocation: Boolean + ) + + fun requestAndFollowRedirects(baseLocation: String, client: OwnCloudClient): RequestResult { + var currentLocation = baseLocation + var redirectedToUnsecureLocation = false + var status: Int + + while (true) { + val getMethod = getGetMethod(currentLocation) + + status = client.executeHttpMethod(getMethod) + val result = + if (isSuccess(status)) RemoteOperationResult(RemoteOperationResult.ResultCode.OK) + else RemoteOperationResult(getMethod) + + if (result.redirectedLocation.isNullOrEmpty() || result.isSuccess) { + return RequestResult(getMethod, status, result, redirectedToUnsecureLocation) + } else { + val nextLocation = updateLocationWithRedirectPath(currentLocation, result.redirectedLocation) + redirectedToUnsecureLocation = + checkIfConnectionIsRedirectedToNoneSecure( + redirectedToUnsecureLocation, + currentLocation, + nextLocation + ) + currentLocation = nextLocation + } + } + } + + private fun isSuccess(status: Int): Boolean = status == HttpConstants.HTTP_OK + + fun handleRequestResult( + requestResult: RequestResult, + baseUrl: String + ): RemoteOperationResult { + if (!isSuccess(requestResult.status)) + return RemoteOperationResult(requestResult.getMethod) + + val respJSON = JSONObject(requestResult.getMethod.getResponseBodyAsString() ?: "") + if (!respJSON.getBoolean(NODE_INSTALLED)) + return RemoteOperationResult(RemoteOperationResult.ResultCode.INSTANCE_NOT_CONFIGURED) + + val ocVersion = OwnCloudVersion(respJSON.getString(NODE_VERSION)) + // 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 = + 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) + } + result.data = ocVersion + return result + } +} \ No newline at end of file diff --git a/owncloudComLibrary/src/test/java/com/owncloud/android/lib/GetRemoteStatusOperationTest.kt b/owncloudComLibrary/src/test/java/com/owncloud/android/lib/StatusRequestorTest.kt similarity index 66% rename from owncloudComLibrary/src/test/java/com/owncloud/android/lib/GetRemoteStatusOperationTest.kt rename to owncloudComLibrary/src/test/java/com/owncloud/android/lib/StatusRequestorTest.kt index 2f3b3d8b..f1ca501b 100644 --- a/owncloudComLibrary/src/test/java/com/owncloud/android/lib/GetRemoteStatusOperationTest.kt +++ b/owncloudComLibrary/src/test/java/com/owncloud/android/lib/StatusRequestorTest.kt @@ -1,24 +1,24 @@ package com.owncloud.android.lib -import com.owncloud.android.lib.resources.status.GetRemoteStatusOperation +import com.owncloud.android.lib.resources.status.StatusRequestor import org.junit.Assert.assertEquals import org.junit.Test -class GetRemoteStatusOperationTest { - private val remoteStatusOperation = GetRemoteStatusOperation() +class StatusRequestorTest { + private val requestor = StatusRequestor() @Test fun `update location with an absolute path`() { - val newLocation = remoteStatusOperation.updateLocationWithRedirectPath( + val newLocation = requestor.updateLocationWithRedirectPath( "https://cloud.somewhere.com", "https://cloud.somewhere.com/subdir" ) assertEquals("https://cloud.somewhere.com/subdir", newLocation) } @Test - fun `update location with a smaler aboslute path`() { - val newLocation = remoteStatusOperation.updateLocationWithRedirectPath( + fun `update location with a smaler aboslute path`() { + val newLocation = requestor.updateLocationWithRedirectPath( "https://cloud.somewhere.com/subdir", "https://cloud.somewhere.com/" ) assertEquals("https://cloud.somewhere.com/", newLocation) @@ -26,7 +26,7 @@ class GetRemoteStatusOperationTest { @Test fun `update location with a relative path`() { - val newLocation = remoteStatusOperation.updateLocationWithRedirectPath( + val newLocation = requestor.updateLocationWithRedirectPath( "https://cloud.somewhere.com", "/subdir" ) assertEquals("https://cloud.somewhere.com/subdir", newLocation) @@ -34,7 +34,7 @@ class GetRemoteStatusOperationTest { @Test fun `update location by replacing the relative path`() { - val newLocation = remoteStatusOperation.updateLocationWithRedirectPath( + val newLocation = requestor.updateLocationWithRedirectPath( "https://cloud.somewhere.com/some/other/subdir", "/subdir" ) assertEquals("https://cloud.somewhere.com/subdir", newLocation)