1
0
mirror of https://github.com/owncloud/android-library.git synced 2025-06-07 16:06:08 +00:00

fix according to review

This commit is contained in:
Christian Schabesberger 2020-09-17 12:23:45 +02:00
parent baf7327782
commit 467776d2b5
4 changed files with 119 additions and 45 deletions

View File

@ -28,10 +28,11 @@ import com.owncloud.android.lib.common.OwnCloudClient
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 com.owncloud.android.lib.resources.status.HttpScheme.HTTPS_SCHEME
import com.owncloud.android.lib.resources.status.HttpScheme.HTTP_SCHEME
import org.json.JSONException
import timber.log.Timber
/**
* Checks if the server is valid
*
@ -41,14 +42,10 @@ import timber.log.Timber
* @author Abel García de Prada
*/
class GetRemoteStatusOperation : RemoteOperation<OwnCloudVersion>() {
companion object {
const val HTTPS_SCHEME = "https"
const val HTTP_SCHEME = "http"
}
override fun run(client: OwnCloudClient): RemoteOperationResult<OwnCloudVersion> {
if (client.baseUri.scheme.isNullOrEmpty())
client.baseUri = Uri.parse(HTTPS_SCHEME + "://" + client.baseUri.toString())
client.baseUri = Uri.parse("$HTTPS_SCHEME://${client.baseUri}")
var result = tryToConnect(client)
if (result.code != ResultCode.OK_SSL && !result.isSslRecoverableException) {
@ -64,9 +61,9 @@ class GetRemoteStatusOperation : RemoteOperation<OwnCloudVersion>() {
val baseUrl = client.baseUri.toString()
client.setFollowRedirects(false)
return try {
val requestor = StatusRequestor()
val requestResult = requestor.requestAndFollowRedirects(baseUrl, client)
requestor.handleRequestResult(requestResult, baseUrl)
val requester = StatusRequester()
val requestResult = requester.requestAndFollowRedirects(baseUrl, client)
requester.handleRequestResult(requestResult, baseUrl)
} catch (e: JSONException) {
RemoteOperationResult(ResultCode.INSTANCE_NOT_CONFIGURED)
} catch (e: Exception) {

View File

@ -0,0 +1,30 @@
/* ownCloud Android Library is available under MIT license
* Copyright (C) 2020 ownCloud GmbH.
*
* Permission is hereby granted, free of charge, to any person obtaining a copy
* of this software and associated documentation files (the "Software"), to deal
* in the Software without restriction, including without limitation the rights
* to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
* copies of the Software, and to permit persons to whom the Software is
* furnished to do so, subject to the following conditions:
*
* The above copyright notice and this permission notice shall be included in
* all copies or substantial portions of the Software.
*
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
* EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
* MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
* NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS
* BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN
* ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
* CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
* THE SOFTWARE.
*
*/
package com.owncloud.android.lib.resources.status
object HttpScheme {
const val HTTP_SCHEME = "http"
const val HTTPS_SCHEME = "https"
}

View File

@ -1,26 +1,40 @@
/* ownCloud Android Library is available under MIT license
* Copyright (C) 2020 ownCloud GmbH.
*
* Permission is hereby granted, free of charge, to any person obtaining a copy
* of this software and associated documentation files (the "Software"), to deal
* in the Software without restriction, including without limitation the rights
* to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
* copies of the Software, and to permit persons to whom the Software is
* furnished to do so, subject to the following conditions:
*
* The above copyright notice and this permission notice shall be included in
* all copies or substantial portions of the Software.
*
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
* EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
* MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
* NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS
* BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN
* ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
* CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
* THE SOFTWARE.
*
*/
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 com.owncloud.android.lib.resources.status.HttpScheme.HTTPS_SCHEME
import com.owncloud.android.lib.resources.status.HttpScheme.HTTP_SCHEME
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"
}
internal class StatusRequester {
private fun checkIfConnectionIsRedirectedToNoneSecure(
isConnectionSecure: Boolean,
@ -28,9 +42,7 @@ internal class StatusRequestor {
redirectedUrl: String
): Boolean {
return isConnectionSecure ||
(baseUrl.startsWith(HTTPS_SCHEME) && redirectedUrl.startsWith(
HTTP_SCHEME
))
(baseUrl.startsWith(HTTPS_SCHEME) && redirectedUrl.startsWith(HTTP_SCHEME))
}
fun updateLocationWithRedirectPath(oldLocation: String, redirectedLocation: String): String {
@ -64,7 +76,7 @@ internal class StatusRequestor {
status = client.executeHttpMethod(getMethod)
val result =
if (isSuccess(status)) RemoteOperationResult<OwnCloudVersion>(RemoteOperationResult.ResultCode.OK)
if (status.isSuccess()) RemoteOperationResult<OwnCloudVersion>(RemoteOperationResult.ResultCode.OK)
else RemoteOperationResult(getMethod)
if (result.redirectedLocation.isNullOrEmpty() || result.isSuccess) {
@ -82,13 +94,13 @@ internal class StatusRequestor {
}
}
private fun isSuccess(status: Int): Boolean = status == HttpConstants.HTTP_OK
private fun Int.isSuccess() = this == HttpConstants.HTTP_OK
fun handleRequestResult(
requestResult: RequestResult,
baseUrl: String
): RemoteOperationResult<OwnCloudVersion> {
if (!isSuccess(requestResult.status))
if (!requestResult.status.isSuccess())
return RemoteOperationResult(requestResult.getMethod)
val respJSON = JSONObject(requestResult.getMethod.getResponseBodyAsString() ?: "")
@ -110,4 +122,14 @@ internal class StatusRequestor {
result.data = ocVersion
return result
}
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"
}
}

View File

@ -1,42 +1,67 @@
/* ownCloud Android Library is available under MIT license
* Copyright (C) 2020 ownCloud GmbH.
*
* Permission is hereby granted, free of charge, to any person obtaining a copy
* of this software and associated documentation files (the "Software"), to deal
* in the Software without restriction, including without limitation the rights
* to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
* copies of the Software, and to permit persons to whom the Software is
* furnished to do so, subject to the following conditions:
*
* The above copyright notice and this permission notice shall be included in
* all copies or substantial portions of the Software.
*
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
* EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
* MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
* NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS
* BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN
* ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
* CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
* THE SOFTWARE.
*
*/
package com.owncloud.android.lib
import com.owncloud.android.lib.resources.status.StatusRequestor
import com.owncloud.android.lib.resources.status.StatusRequester
import org.junit.Assert.assertEquals
import org.junit.Test
class StatusRequestorTest {
private val requestor = StatusRequestor()
private val requestor = StatusRequester()
@Test
fun `update location with an absolute path`() {
val newLocation = requestor.updateLocationWithRedirectPath(
"https://cloud.somewhere.com", "https://cloud.somewhere.com/subdir"
)
assertEquals("https://cloud.somewhere.com/subdir", newLocation)
val newLocation = requestor.updateLocationWithRedirectPath(TEST_DOMAIN, "$TEST_DOMAIN/subdir")
assertEquals("$TEST_DOMAIN$SUB_PATH", newLocation)
}
@Test
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)
fun `update location with a smaller absolute path`() {
val newLocation = requestor.updateLocationWithRedirectPath("$TEST_DOMAIN$SUB_PATH", TEST_DOMAIN)
assertEquals(TEST_DOMAIN, newLocation)
}
@Test
fun `update location with a relative path`() {
val newLocation = requestor.updateLocationWithRedirectPath(
"https://cloud.somewhere.com", "/subdir"
TEST_DOMAIN, SUB_PATH
)
assertEquals("https://cloud.somewhere.com/subdir", newLocation)
assertEquals("$TEST_DOMAIN$SUB_PATH", newLocation)
}
@Test
fun `update location by replacing the relative path`() {
val newLocation = requestor.updateLocationWithRedirectPath(
"https://cloud.somewhere.com/some/other/subdir", "/subdir"
"$TEST_DOMAIN/some/other/subdir", SUB_PATH
)
assertEquals("https://cloud.somewhere.com/subdir", newLocation)
assertEquals("$TEST_DOMAIN$SUB_PATH", newLocation)
}
companion object {
const val TEST_DOMAIN = "https://cloud.somewhere.com"
const val SUB_PATH = "/subdir"
}
}