From 7adb55aa59a5de37d6b083f9b2c54b14e51197d2 Mon Sep 17 00:00:00 2001 From: alperozturk Date: Thu, 8 May 2025 11:53:34 +0200 Subject: [PATCH 1/7] fix unnecessary calls Signed-off-by: alperozturk --- .../shares/UpdateShareRemoteOperation.java | 95 ++++++++----------- 1 file changed, 40 insertions(+), 55 deletions(-) diff --git a/library/src/main/java/com/owncloud/android/lib/resources/shares/UpdateShareRemoteOperation.java b/library/src/main/java/com/owncloud/android/lib/resources/shares/UpdateShareRemoteOperation.java index 826c772848..a5d95bb16c 100644 --- a/library/src/main/java/com/owncloud/android/lib/resources/shares/UpdateShareRemoteOperation.java +++ b/library/src/main/java/com/owncloud/android/lib/resources/shares/UpdateShareRemoteOperation.java @@ -33,7 +33,7 @@ /** * Updates parameters of an existing Share resource, known its remote ID. - * + *

* Allow updating several parameters, triggering a request to the server per parameter. */ public class UpdateShareRemoteOperation extends RemoteOperation { @@ -55,7 +55,7 @@ public class UpdateShareRemoteOperation extends RemoteOperation { /** * Identifier of the share to update */ - private long remoteId; + private final long remoteId; /** * Password to set for the public link @@ -149,8 +149,7 @@ public void setNote(String note) { @Override protected RemoteOperationResult> run(OwnCloudClient client) { - RemoteOperationResult> result = null; - int status; + RemoteOperationResult> result; /// prepare array of parameters to update List> parametersToUpdate = new ArrayList<>(); @@ -171,7 +170,6 @@ protected RemoteOperationResult> run(OwnCloudClient client) { } if (permissions > 0) { - // set permissions parametersToUpdate.add(new Pair<>(PARAM_PERMISSIONS, Integer.toString(permissions))); } @@ -179,74 +177,61 @@ protected RemoteOperationResult> run(OwnCloudClient client) { parametersToUpdate.add(new Pair<>(PARAM_HIDE_DOWNLOAD, Boolean.toString(hideFileDownload))); } - if (note != null) { - parametersToUpdate.add(new Pair<>(PARAM_NOTE, URLEncoder.encode(note))); - } - - if (label != null) { - parametersToUpdate.add(new Pair<>(PARAM_LABEL, URLEncoder.encode(label))); - } - - if (attributes != null) { - parametersToUpdate.add(new Pair<>(PARAM_ATTRIBUTES, URLEncoder.encode(attributes))); - } - - /// perform required PUT requests PutMethod put = null; - String uriString; - try { + if (note != null) { + parametersToUpdate.add(new Pair<>(PARAM_NOTE, URLEncoder.encode(note, ENTITY_CHARSET))); + } + + if (label != null) { + parametersToUpdate.add(new Pair<>(PARAM_LABEL, URLEncoder.encode(label, ENTITY_CHARSET))); + } + Uri requestUri = client.getBaseUri(); Uri.Builder uriBuilder = requestUri.buildUpon(); uriBuilder.appendEncodedPath(ShareUtils.SHARING_API_PATH.substring(1)); uriBuilder.appendEncodedPath(Long.toString(remoteId)); - uriString = uriBuilder.build().toString(); + String uriString = uriBuilder.build().toString(); - for (Pair parameter : parametersToUpdate) { - if (put != null) { - put.releaseConnection(); - } - put = new PutMethod(uriString); - put.addRequestHeader(OCS_API_HEADER, OCS_API_HEADER_VALUE); - put.setRequestEntity(new StringRequestEntity( - parameter.first + "=" + parameter.second, - ENTITY_CONTENT_TYPE, - ENTITY_CHARSET - )); - - status = client.executeMethod(put); - - if (status == HttpStatus.SC_OK || status == HttpStatus.SC_BAD_REQUEST) { - String response = put.getResponseBodyAsString(); - - // Parse xml response - ShareToRemoteOperationResultParser parser = new ShareToRemoteOperationResultParser( - new ShareXMLParser() - ); - parser.setServerBaseUri(client.getBaseUri()); - result = parser.parse(response); - - } else { - result = new RemoteOperationResult<>(false, put); - } - if (!result.isSuccess()) { - break; + StringBuilder bodyBuilder = new StringBuilder(); + for (int i = 0; i < parametersToUpdate.size(); i++) { + Pair param = parametersToUpdate.get(i); + bodyBuilder.append(param.first) + .append("=") + .append(param.second); + + if (i < parametersToUpdate.size() - 1) { + bodyBuilder.append("&"); } } + put = new PutMethod(uriString); + put.addRequestHeader(OCS_API_HEADER, OCS_API_HEADER_VALUE); + put.setRequestEntity(new StringRequestEntity( + bodyBuilder.toString(), + ENTITY_CONTENT_TYPE, + ENTITY_CHARSET + )); + + int status = client.executeMethod(put); + if (status == HttpStatus.SC_OK || status == HttpStatus.SC_BAD_REQUEST) { + String response = put.getResponseBodyAsString(); + final var shareXMLParser = new ShareXMLParser(); + final var parser = new ShareToRemoteOperationResultParser(shareXMLParser); + parser.setServerBaseUri(client.getBaseUri()); + result = parser.parse(response); + } else { + result = new RemoteOperationResult<>(false, put); + } } catch (Exception e) { result = new RemoteOperationResult<>(e); Log_OC.e(TAG, "Exception while updating remote share ", e); - if (put != null) { - put.releaseConnection(); - } - } finally { if (put != null) { put.releaseConnection(); } } + return result; } - } From dd6a36b36a7cfd44508ba689a62809d25b579db2 Mon Sep 17 00:00:00 2001 From: alperozturk Date: Thu, 8 May 2025 12:31:01 +0200 Subject: [PATCH 2/7] fix code analytics, get rid of List> use JsonObject Signed-off-by: alperozturk --- .../shares/UpdateShareRemoteOperation.java | 35 ++++++------------- 1 file changed, 11 insertions(+), 24 deletions(-) diff --git a/library/src/main/java/com/owncloud/android/lib/resources/shares/UpdateShareRemoteOperation.java b/library/src/main/java/com/owncloud/android/lib/resources/shares/UpdateShareRemoteOperation.java index a5d95bb16c..ff99519a6e 100644 --- a/library/src/main/java/com/owncloud/android/lib/resources/shares/UpdateShareRemoteOperation.java +++ b/library/src/main/java/com/owncloud/android/lib/resources/shares/UpdateShareRemoteOperation.java @@ -11,8 +11,8 @@ package com.owncloud.android.lib.resources.shares; import android.net.Uri; -import android.util.Pair; +import com.google.gson.JsonObject; import com.owncloud.android.lib.common.OwnCloudClient; import com.owncloud.android.lib.common.operations.RemoteOperation; import com.owncloud.android.lib.common.operations.RemoteOperationResult; @@ -25,7 +25,6 @@ import java.net.URLEncoder; import java.text.DateFormat; import java.text.SimpleDateFormat; -import java.util.ArrayList; import java.util.Calendar; import java.util.List; import java.util.Locale; @@ -47,7 +46,7 @@ public class UpdateShareRemoteOperation extends RemoteOperation { private static final String PARAM_HIDE_DOWNLOAD = "hideDownload"; private static final String PARAM_LABEL = "label"; private static final String FORMAT_EXPIRATION_DATE = "yyyy-MM-dd"; - private static final String ENTITY_CONTENT_TYPE = "application/x-www-form-urlencoded"; + private static final String ENTITY_CONTENT_TYPE = "application/json"; private static final String ENTITY_CHARSET = "UTF-8"; private static final String PARAM_ATTRIBUTES = "attributes"; @@ -152,39 +151,39 @@ protected RemoteOperationResult> run(OwnCloudClient client) { RemoteOperationResult> result; /// prepare array of parameters to update - List> parametersToUpdate = new ArrayList<>(); + JsonObject params = new JsonObject(); if (password != null) { - parametersToUpdate.add(new Pair<>(PARAM_PASSWORD, password)); + params.addProperty(PARAM_PASSWORD, password); } if (expirationDateInMillis < 0) { // clear expiration date - parametersToUpdate.add(new Pair<>(PARAM_EXPIRATION_DATE, "")); + params.addProperty(PARAM_EXPIRATION_DATE, ""); } else if (expirationDateInMillis > 0) { // set expiration date DateFormat dateFormat = new SimpleDateFormat(FORMAT_EXPIRATION_DATE, Locale.US); Calendar expirationDate = Calendar.getInstance(); expirationDate.setTimeInMillis(expirationDateInMillis); String formattedExpirationDate = dateFormat.format(expirationDate.getTime()); - parametersToUpdate.add(new Pair<>(PARAM_EXPIRATION_DATE, formattedExpirationDate)); + params.addProperty(PARAM_EXPIRATION_DATE, formattedExpirationDate); } if (permissions > 0) { - parametersToUpdate.add(new Pair<>(PARAM_PERMISSIONS, Integer.toString(permissions))); + params.addProperty(PARAM_PERMISSIONS, Integer.toString(permissions)); } if (hideFileDownload != null) { - parametersToUpdate.add(new Pair<>(PARAM_HIDE_DOWNLOAD, Boolean.toString(hideFileDownload))); + params.addProperty(PARAM_HIDE_DOWNLOAD, Boolean.toString(hideFileDownload)); } PutMethod put = null; try { if (note != null) { - parametersToUpdate.add(new Pair<>(PARAM_NOTE, URLEncoder.encode(note, ENTITY_CHARSET))); + params.addProperty(PARAM_NOTE, URLEncoder.encode(note, ENTITY_CHARSET)); } if (label != null) { - parametersToUpdate.add(new Pair<>(PARAM_LABEL, URLEncoder.encode(label, ENTITY_CHARSET))); + params.addProperty(PARAM_LABEL, URLEncoder.encode(label, ENTITY_CHARSET)); } Uri requestUri = client.getBaseUri(); @@ -193,22 +192,10 @@ protected RemoteOperationResult> run(OwnCloudClient client) { uriBuilder.appendEncodedPath(Long.toString(remoteId)); String uriString = uriBuilder.build().toString(); - StringBuilder bodyBuilder = new StringBuilder(); - for (int i = 0; i < parametersToUpdate.size(); i++) { - Pair param = parametersToUpdate.get(i); - bodyBuilder.append(param.first) - .append("=") - .append(param.second); - - if (i < parametersToUpdate.size() - 1) { - bodyBuilder.append("&"); - } - } - put = new PutMethod(uriString); put.addRequestHeader(OCS_API_HEADER, OCS_API_HEADER_VALUE); put.setRequestEntity(new StringRequestEntity( - bodyBuilder.toString(), + params.toString(), ENTITY_CONTENT_TYPE, ENTITY_CHARSET )); From 2b88297361d1a4383345e933921b7a0d9eabc081 Mon Sep 17 00:00:00 2001 From: alperozturk Date: Thu, 8 May 2025 14:15:51 +0200 Subject: [PATCH 3/7] remove URLEncode Signed-off-by: alperozturk --- .../shares/UpdateShareRemoteOperation.java | 39 +++++++++---------- 1 file changed, 19 insertions(+), 20 deletions(-) diff --git a/library/src/main/java/com/owncloud/android/lib/resources/shares/UpdateShareRemoteOperation.java b/library/src/main/java/com/owncloud/android/lib/resources/shares/UpdateShareRemoteOperation.java index ff99519a6e..4b05adc85f 100644 --- a/library/src/main/java/com/owncloud/android/lib/resources/shares/UpdateShareRemoteOperation.java +++ b/library/src/main/java/com/owncloud/android/lib/resources/shares/UpdateShareRemoteOperation.java @@ -22,7 +22,6 @@ import org.apache.commons.httpclient.methods.PutMethod; import org.apache.commons.httpclient.methods.StringRequestEntity; -import java.net.URLEncoder; import java.text.DateFormat; import java.text.SimpleDateFormat; import java.util.Calendar; @@ -146,11 +145,7 @@ public void setNote(String note) { this.note = note; } - @Override - protected RemoteOperationResult> run(OwnCloudClient client) { - RemoteOperationResult> result; - - /// prepare array of parameters to update + private String getRequestBody() { JsonObject params = new JsonObject(); if (password != null) { params.addProperty(PARAM_PASSWORD, password); @@ -167,7 +162,7 @@ protected RemoteOperationResult> run(OwnCloudClient client) { String formattedExpirationDate = dateFormat.format(expirationDate.getTime()); params.addProperty(PARAM_EXPIRATION_DATE, formattedExpirationDate); } - + if (permissions > 0) { params.addProperty(PARAM_PERMISSIONS, Integer.toString(permissions)); } @@ -176,16 +171,24 @@ protected RemoteOperationResult> run(OwnCloudClient client) { params.addProperty(PARAM_HIDE_DOWNLOAD, Boolean.toString(hideFileDownload)); } - PutMethod put = null; - try { - if (note != null) { - params.addProperty(PARAM_NOTE, URLEncoder.encode(note, ENTITY_CHARSET)); - } + if (note != null) { + params.addProperty(PARAM_NOTE, note); + } - if (label != null) { - params.addProperty(PARAM_LABEL, URLEncoder.encode(label, ENTITY_CHARSET)); - } + if (label != null) { + params.addProperty(PARAM_LABEL, label); + } + + return params.toString(); + } + @Override + protected RemoteOperationResult> run(OwnCloudClient client) { + RemoteOperationResult> result; + String requestBody = getRequestBody(); + + PutMethod put = null; + try { Uri requestUri = client.getBaseUri(); Uri.Builder uriBuilder = requestUri.buildUpon(); uriBuilder.appendEncodedPath(ShareUtils.SHARING_API_PATH.substring(1)); @@ -194,11 +197,7 @@ protected RemoteOperationResult> run(OwnCloudClient client) { put = new PutMethod(uriString); put.addRequestHeader(OCS_API_HEADER, OCS_API_HEADER_VALUE); - put.setRequestEntity(new StringRequestEntity( - params.toString(), - ENTITY_CONTENT_TYPE, - ENTITY_CHARSET - )); + put.setRequestEntity(new StringRequestEntity(requestBody, ENTITY_CONTENT_TYPE, ENTITY_CHARSET)); int status = client.executeMethod(put); if (status == HttpStatus.SC_OK || status == HttpStatus.SC_BAD_REQUEST) { From 927dd527e36d7dff9f191ac7730942dac0c1f9bc Mon Sep 17 00:00:00 2001 From: alperozturk Date: Tue, 27 May 2025 18:33:57 +0800 Subject: [PATCH 4/7] revert changes Signed-off-by: alperozturk --- .../android/lib/resources/shares/UpdateShareRemoteOperation.java | 1 - 1 file changed, 1 deletion(-) diff --git a/library/src/main/java/com/owncloud/android/lib/resources/shares/UpdateShareRemoteOperation.java b/library/src/main/java/com/owncloud/android/lib/resources/shares/UpdateShareRemoteOperation.java index 4b05adc85f..3c85027c57 100644 --- a/library/src/main/java/com/owncloud/android/lib/resources/shares/UpdateShareRemoteOperation.java +++ b/library/src/main/java/com/owncloud/android/lib/resources/shares/UpdateShareRemoteOperation.java @@ -49,7 +49,6 @@ public class UpdateShareRemoteOperation extends RemoteOperation { private static final String ENTITY_CHARSET = "UTF-8"; private static final String PARAM_ATTRIBUTES = "attributes"; - /** * Identifier of the share to update */ From 05eee4d6e34a4a3a3c85720c386dfb98e31c06b5 Mon Sep 17 00:00:00 2001 From: alperozturk Date: Mon, 2 Jun 2025 17:39:19 +0800 Subject: [PATCH 5/7] add updateMultipleParams tests Signed-off-by: alperozturk --- .../shares/UpdateShareRemoteOperationIT.kt | 42 +++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/library/src/androidTest/java/com/owncloud/android/lib/resources/shares/UpdateShareRemoteOperationIT.kt b/library/src/androidTest/java/com/owncloud/android/lib/resources/shares/UpdateShareRemoteOperationIT.kt index dae6aa80e2..d62d993514 100644 --- a/library/src/androidTest/java/com/owncloud/android/lib/resources/shares/UpdateShareRemoteOperationIT.kt +++ b/library/src/androidTest/java/com/owncloud/android/lib/resources/shares/UpdateShareRemoteOperationIT.kt @@ -81,6 +81,48 @@ class UpdateShareRemoteOperationIT : AbstractIT() { assertTrue(RemoveFileRemoteOperation("/note/").execute(client).isSuccess) } + @Test + fun updateMultipleParams() { + + assertTrue(CreateFolderRemoteOperation("/label/", true).execute(client).isSuccess) + + val createOperationResult = + CreateShareRemoteOperation( + "/label/", + ShareType.PUBLIC_LINK, + "", + true, + "", + OCShare.READ_PERMISSION_FLAG + ).execute(client) + + assertTrue(createOperationResult.isSuccess) + + val share = createOperationResult.resultData[0] + + val sut = UpdateShareRemoteOperation(share.remoteId) + val label = "test & test" + sut.setLabel(label) + + val note = "test note" + sut.setNote(note) + + val password = "test_pass_%_90" + sut.setPassword(password) + + assertTrue(sut.execute(client).isSuccess) + + val getShareOperationResult = GetShareRemoteOperation(share.remoteId).execute(client) + assertTrue(getShareOperationResult.isSuccess) + val updatedShare = getShareOperationResult.resultData[0] + + assertEquals(label, updatedShare.label) + assertEquals(true, updatedShare.isPasswordProtected) + assertEquals(note, updatedShare.note) + + assertTrue(RemoveFileRemoteOperation("/label/").execute(client).isSuccess) + } + @Test fun updateLabel() { val label = "test & test" From 6342d86a8a7e80fdc6e2d3a14b5f01c71c525c11 Mon Sep 17 00:00:00 2001 From: alperozturk Date: Mon, 2 Jun 2025 18:23:59 +0800 Subject: [PATCH 6/7] add updateMultipleParams tests Signed-off-by: alperozturk --- .../android/lib/resources/shares/UpdateShareRemoteOperationIT.kt | 1 - 1 file changed, 1 deletion(-) diff --git a/library/src/androidTest/java/com/owncloud/android/lib/resources/shares/UpdateShareRemoteOperationIT.kt b/library/src/androidTest/java/com/owncloud/android/lib/resources/shares/UpdateShareRemoteOperationIT.kt index d62d993514..359a71b5be 100644 --- a/library/src/androidTest/java/com/owncloud/android/lib/resources/shares/UpdateShareRemoteOperationIT.kt +++ b/library/src/androidTest/java/com/owncloud/android/lib/resources/shares/UpdateShareRemoteOperationIT.kt @@ -83,7 +83,6 @@ class UpdateShareRemoteOperationIT : AbstractIT() { @Test fun updateMultipleParams() { - assertTrue(CreateFolderRemoteOperation("/label/", true).execute(client).isSuccess) val createOperationResult = From 37d47095c2ca957ee9960987d67a4d64f444967c Mon Sep 17 00:00:00 2001 From: alperozturk Date: Tue, 10 Jun 2025 10:39:29 +0200 Subject: [PATCH 7/7] rebase Signed-off-by: alperozturk --- .../lib/resources/shares/UpdateShareRemoteOperation.java | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/library/src/main/java/com/owncloud/android/lib/resources/shares/UpdateShareRemoteOperation.java b/library/src/main/java/com/owncloud/android/lib/resources/shares/UpdateShareRemoteOperation.java index 3c85027c57..a466e9f367 100644 --- a/library/src/main/java/com/owncloud/android/lib/resources/shares/UpdateShareRemoteOperation.java +++ b/library/src/main/java/com/owncloud/android/lib/resources/shares/UpdateShareRemoteOperation.java @@ -2,6 +2,7 @@ * Nextcloud Android Library * * SPDX-FileCopyrightText: 2018-2024 Nextcloud GmbH and Nextcloud contributors + * SPDX-FileCopyrightText: 2025 Alper Ozturk * SPDX-FileCopyrightText: 2018-2021 Tobias Kaminsky * SPDX-FileCopyrightText: 2015 ownCloud Inc. * SPDX-FileCopyrightText: 2015 David A. Velasco @@ -178,6 +179,10 @@ private String getRequestBody() { params.addProperty(PARAM_LABEL, label); } + if (attributes != null) { + params.addProperty(PARAM_ATTRIBUTES, attributes); + } + return params.toString(); }