From f14abb0bcde1bb99274f4dbfd115f56681bbf7df Mon Sep 17 00:00:00 2001
From: Fernando Sanz <fsanz@izertis.com>
Date: Thu, 2 Sep 2021 10:55:02 +0200
Subject: [PATCH] Code revision suggestions have been implemented.

---
 .../lib/common/http/HttpConstants.java        | 14 +++++++++++++-
 .../shares/CreateRemoteShareOperation.kt      | 19 +++++++++----------
 .../shares/GetRemoteShareOperation.kt         | 10 ++--------
 .../shares/GetRemoteShareesOperation.kt       | 10 +++++-----
 .../shares/GetRemoteSharesForFileOperation.kt | 13 +++++--------
 .../shares/RemoveRemoteShareOperation.kt      | 16 +++++-----------
 .../shares/UpdateRemoteShareOperation.kt      | 10 +++++-----
 7 files changed, 44 insertions(+), 48 deletions(-)

diff --git a/owncloudComLibrary/src/main/java/com/owncloud/android/lib/common/http/HttpConstants.java b/owncloudComLibrary/src/main/java/com/owncloud/android/lib/common/http/HttpConstants.java
index 12a2c3f6..091643ab 100644
--- a/owncloudComLibrary/src/main/java/com/owncloud/android/lib/common/http/HttpConstants.java
+++ b/owncloudComLibrary/src/main/java/com/owncloud/android/lib/common/http/HttpConstants.java
@@ -66,6 +66,18 @@ public class HttpConstants {
     public static final String CONTENT_TYPE_JSON = "application/json";
     public static final String CONTENT_TYPE_WWW_FORM = "application/x-www-form-urlencoded";
 
+    /***********************************************************************************************************
+     ************************************************ ARGUMENTS NAMES ********************************************
+     ***********************************************************************************************************/
+
+    public static final String PARAM_FORMAT = "format";
+
+    /***********************************************************************************************************
+     ************************************************ ARGUMENTS VALUES ********************************************
+     ***********************************************************************************************************/
+
+    public static final String VALUE_FORMAT = "json";
+
     /***********************************************************************************************************
      ************************************************ STATUS CODES *********************************************
      ***********************************************************************************************************/
@@ -205,4 +217,4 @@ public class HttpConstants {
      * Default timeout for establishing a connection
      */
     public static final int DEFAULT_CONNECTION_TIMEOUT = 60000;
-}
\ No newline at end of file
+}
diff --git a/owncloudComLibrary/src/main/java/com/owncloud/android/lib/resources/shares/CreateRemoteShareOperation.kt b/owncloudComLibrary/src/main/java/com/owncloud/android/lib/resources/shares/CreateRemoteShareOperation.kt
index 5ab4732c..0f946bcd 100644
--- a/owncloudComLibrary/src/main/java/com/owncloud/android/lib/resources/shares/CreateRemoteShareOperation.kt
+++ b/owncloudComLibrary/src/main/java/com/owncloud/android/lib/resources/shares/CreateRemoteShareOperation.kt
@@ -31,6 +31,8 @@ package com.owncloud.android.lib.resources.shares
 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.HttpConstants.PARAM_FORMAT
+import com.owncloud.android.lib.common.http.HttpConstants.VALUE_FORMAT
 import com.owncloud.android.lib.common.http.methods.nonwebdav.PostMethod
 import com.owncloud.android.lib.common.operations.RemoteOperation
 import com.owncloud.android.lib.common.operations.RemoteOperationResult
@@ -127,7 +129,7 @@ class CreateRemoteShareOperation(
         return result
     }
 
-    private fun createFormBodyBuilder(): FormBody.Builder {
+    private fun createFormBodyBuilder(): FormBody {
 
         val formBodyBuilder = FormBody.Builder()
             .add(PARAM_PATH, remoteFilePath)
@@ -156,15 +158,13 @@ class CreateRemoteShareOperation(
             formBodyBuilder.add(PARAM_PERMISSIONS, permissions.toString())
         }
 
-        return formBodyBuilder
+        return formBodyBuilder.build()
     }
 
     override fun run(client: OwnCloudClient): RemoteOperationResult<ShareResponse> {
         val requestUri = buildRequestUri(client.baseUri)
 
-        val formBodyBuilder = createFormBodyBuilder()
-
-        val postMethod = PostMethod(URL(requestUri.toString()), formBodyBuilder.build()).apply {
+        val postMethod = PostMethod(URL(requestUri.toString()), createFormBodyBuilder()).apply {
             setRequestHeader(HttpConstants.CONTENT_TYPE_HEADER, HttpConstants.CONTENT_TYPE_URLENCODED_UTF8)
             addRequestHeader(OCS_API_HEADER, OCS_API_HEADER_VALUE)
         }
@@ -173,10 +173,10 @@ class CreateRemoteShareOperation(
             val status = client.executeHttpMethod(postMethod)
             val response = postMethod.getResponseBodyAsString()
 
-            if (!isSuccess(status)) {
-                onResultUnsuccessful(postMethod, response, status)
-            } else {
+            if (isSuccess(status)) {
                 onRequestSuccessful(response)
+            } else {
+                onResultUnsuccessful(postMethod, response, status)
             }
 
         } catch (e: Exception) {
@@ -193,7 +193,7 @@ class CreateRemoteShareOperation(
         private const val OCS_ROUTE = "ocs/v2.php/apps/files_sharing/api/v1/shares"
 
         //Arguments - names
-        private const val PARAM_FORMAT = "format"
+
         private const val PARAM_NAME = "name"
         private const val PARAM_EXPIRATION_DATE = "expireDate"
         private const val PARAM_PATH = "path"
@@ -204,7 +204,6 @@ class CreateRemoteShareOperation(
         private const val PARAM_PERMISSIONS = "permissions"
 
         //Arguments - constant values
-        private const val VALUE_FORMAT = "json"
         private const val FORMAT_EXPIRATION_DATE = "yyyy-MM-dd"
     }
 }
diff --git a/owncloudComLibrary/src/main/java/com/owncloud/android/lib/resources/shares/GetRemoteShareOperation.kt b/owncloudComLibrary/src/main/java/com/owncloud/android/lib/resources/shares/GetRemoteShareOperation.kt
index cd95e6db..0f14168e 100644
--- a/owncloudComLibrary/src/main/java/com/owncloud/android/lib/resources/shares/GetRemoteShareOperation.kt
+++ b/owncloudComLibrary/src/main/java/com/owncloud/android/lib/resources/shares/GetRemoteShareOperation.kt
@@ -28,6 +28,8 @@ package com.owncloud.android.lib.resources.shares
 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.HttpConstants.PARAM_FORMAT
+import com.owncloud.android.lib.common.http.HttpConstants.VALUE_FORMAT
 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
@@ -108,15 +110,7 @@ class GetRemoteShareOperation(private val remoteId: String) : RemoteOperation<Sh
     private fun isSuccess(status: Int) = status == HttpConstants.HTTP_OK
 
     companion object {
-
         //OCS Route
         private const val OCS_ROUTE = "ocs/v2.php/apps/files_sharing/api/v1/shares"
-
-        //Arguments - names
-        private const val PARAM_FORMAT = "format"
-
-        //Arguments - constant values
-        private const val VALUE_FORMAT = "json"
-
     }
 }
diff --git a/owncloudComLibrary/src/main/java/com/owncloud/android/lib/resources/shares/GetRemoteShareesOperation.kt b/owncloudComLibrary/src/main/java/com/owncloud/android/lib/resources/shares/GetRemoteShareesOperation.kt
index 546bf05b..10154820 100644
--- a/owncloudComLibrary/src/main/java/com/owncloud/android/lib/resources/shares/GetRemoteShareesOperation.kt
+++ b/owncloudComLibrary/src/main/java/com/owncloud/android/lib/resources/shares/GetRemoteShareesOperation.kt
@@ -32,6 +32,8 @@ package com.owncloud.android.lib.resources.shares
 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.HttpConstants.PARAM_FORMAT
+import com.owncloud.android.lib.common.http.HttpConstants.VALUE_FORMAT
 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
@@ -136,10 +138,10 @@ class GetRemoteShareesOperation
             val status = client.executeHttpMethod(getMethod)
             val response = getMethod.getResponseBodyAsString()
 
-            if (!isSuccess(status)) {
-                onResultUnsuccessful(getMethod, response, status)
-            } else {
+            if (isSuccess(status)) {
                 onRequestSuccessful(response)
+            } else {
+                onResultUnsuccessful(getMethod, response, status)
             }
         } catch (e: Exception) {
             Timber.e(e, "Exception while getting users/groups")
@@ -155,14 +157,12 @@ class GetRemoteShareesOperation
         private const val OCS_ROUTE = "ocs/v2.php/apps/files_sharing/api/v1/sharees"    // from OC 8.2
 
         // Arguments - names
-        private const val PARAM_FORMAT = "format"
         private const val PARAM_ITEM_TYPE = "itemType"
         private const val PARAM_SEARCH = "search"
         private const val PARAM_PAGE = "page"                //  default = 1
         private const val PARAM_PER_PAGE = "perPage"         //  default = 200
 
         // Arguments - constant values
-        private const val VALUE_FORMAT = "json"
         private const val VALUE_ITEM_TYPE = "file"         //  to get the server search for users / groups
     }
 }
diff --git a/owncloudComLibrary/src/main/java/com/owncloud/android/lib/resources/shares/GetRemoteSharesForFileOperation.kt b/owncloudComLibrary/src/main/java/com/owncloud/android/lib/resources/shares/GetRemoteSharesForFileOperation.kt
index 6301e8f7..adb4374e 100644
--- a/owncloudComLibrary/src/main/java/com/owncloud/android/lib/resources/shares/GetRemoteSharesForFileOperation.kt
+++ b/owncloudComLibrary/src/main/java/com/owncloud/android/lib/resources/shares/GetRemoteSharesForFileOperation.kt
@@ -31,6 +31,8 @@ package com.owncloud.android.lib.resources.shares
 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.HttpConstants.PARAM_FORMAT
+import com.owncloud.android.lib.common.http.HttpConstants.VALUE_FORMAT
 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
@@ -125,10 +127,10 @@ class GetRemoteSharesForFileOperation(
             val status = client.executeHttpMethod(getMethod)
             val response = getMethod.getResponseBodyAsString()
 
-            if (!isSuccess(status)) {
-                onResultUnsuccessful(getMethod, response, status)
-            } else {
+            if (isSuccess(status)) {
                 onRequestSuccessful(response)
+            } else {
+                onResultUnsuccessful(getMethod, response, status)
             }
         } catch (e: Exception) {
             Timber.e(e, "Exception while getting remote shares for file operation")
@@ -144,13 +146,8 @@ class GetRemoteSharesForFileOperation(
         private const val OCS_ROUTE = "ocs/v2.php/apps/files_sharing/api/v1/shares"
 
         //Arguments - names
-        private const val PARAM_FORMAT = "format"
         private const val PARAM_PATH = "path"
         private const val PARAM_RESHARES = "reshares"
         private const val PARAM_SUBFILES = "subfiles"
-
-        //Arguments - constant values
-        private const val VALUE_FORMAT = "json"
-
     }
 }
diff --git a/owncloudComLibrary/src/main/java/com/owncloud/android/lib/resources/shares/RemoveRemoteShareOperation.kt b/owncloudComLibrary/src/main/java/com/owncloud/android/lib/resources/shares/RemoveRemoteShareOperation.kt
index 15e5ce58..2ce20a2c 100644
--- a/owncloudComLibrary/src/main/java/com/owncloud/android/lib/resources/shares/RemoveRemoteShareOperation.kt
+++ b/owncloudComLibrary/src/main/java/com/owncloud/android/lib/resources/shares/RemoveRemoteShareOperation.kt
@@ -31,6 +31,8 @@ package com.owncloud.android.lib.resources.shares
 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.HttpConstants.PARAM_FORMAT
+import com.owncloud.android.lib.common.http.HttpConstants.VALUE_FORMAT
 import com.owncloud.android.lib.common.http.methods.nonwebdav.DeleteMethod
 import com.owncloud.android.lib.common.operations.RemoteOperation
 import com.owncloud.android.lib.common.operations.RemoteOperationResult
@@ -112,10 +114,10 @@ class RemoveRemoteShareOperation(private val remoteShareId: String) : RemoteOper
             val status = client.executeHttpMethod(deleteMethod)
             val response = deleteMethod.getResponseBodyAsString()
 
-            if (!isSuccess(status)) {
-                onResultUnsuccessful(deleteMethod, response, status)
-            } else {
+            if (isSuccess(status)) {
                 onRequestSuccessful(response)
+            } else {
+                onResultUnsuccessful(deleteMethod, response, status)
             }
         } catch (e: Exception) {
             Timber.e(e, "Exception while unshare link")
@@ -126,15 +128,7 @@ class RemoveRemoteShareOperation(private val remoteShareId: String) : RemoteOper
     private fun isSuccess(status: Int): Boolean = status == HttpConstants.HTTP_OK
 
     companion object {
-
         //OCS Route
         private const val OCS_ROUTE = "ocs/v2.php/apps/files_sharing/api/v1/shares"
-
-        //Arguments - names
-        private const val PARAM_FORMAT = "format"
-
-        //Arguments - constant values
-        private const val VALUE_FORMAT = "json"
-
     }
 }
diff --git a/owncloudComLibrary/src/main/java/com/owncloud/android/lib/resources/shares/UpdateRemoteShareOperation.kt b/owncloudComLibrary/src/main/java/com/owncloud/android/lib/resources/shares/UpdateRemoteShareOperation.kt
index 12143227..f18a3be3 100644
--- a/owncloudComLibrary/src/main/java/com/owncloud/android/lib/resources/shares/UpdateRemoteShareOperation.kt
+++ b/owncloudComLibrary/src/main/java/com/owncloud/android/lib/resources/shares/UpdateRemoteShareOperation.kt
@@ -31,6 +31,8 @@ package com.owncloud.android.lib.resources.shares
 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.HttpConstants.PARAM_FORMAT
+import com.owncloud.android.lib.common.http.HttpConstants.VALUE_FORMAT
 import com.owncloud.android.lib.common.http.methods.nonwebdav.PutMethod
 import com.owncloud.android.lib.common.operations.RemoteOperation
 import com.owncloud.android.lib.common.operations.RemoteOperationResult
@@ -208,10 +210,10 @@ class UpdateRemoteShareOperation
             val status = client.executeHttpMethod(putMethod)
             val response = putMethod.getResponseBodyAsString()
 
-            if (!isSuccess(status)) {
-                onResultUnsuccessful(putMethod, response, status)
-            } else {
+            if (isSuccess(status)) {
                 onRequestSuccessful(response)
+            } else {
+                onResultUnsuccessful(putMethod, response, status)
             }
         } catch (e: Exception) {
             Timber.e(e, "Exception while updating remote share")
@@ -227,7 +229,6 @@ class UpdateRemoteShareOperation
         private const val OCS_ROUTE = "ocs/v2.php/apps/files_sharing/api/v1/shares"
 
         //Arguments - names
-        private const val PARAM_FORMAT = "format"
         private const val PARAM_NAME = "name"
         private const val PARAM_PASSWORD = "password"
         private const val PARAM_EXPIRATION_DATE = "expireDate"
@@ -235,7 +236,6 @@ class UpdateRemoteShareOperation
         private const val PARAM_PUBLIC_UPLOAD = "publicUpload"
 
         //Arguments - constant values
-        private const val VALUE_FORMAT = "json"
         private const val FORMAT_EXPIRATION_DATE = "yyyy-MM-dd"
         private const val INITIAL_EXPIRATION_DATE_IN_MILLIS: Long = 0
     }