From 5ff9062ea8fefb1bf2b3952b9d64daa4f9922a39 Mon Sep 17 00:00:00 2001 From: "David A. Velasco" Date: Fri, 11 Nov 2016 09:10:26 +0100 Subject: [PATCH 1/4] Allow conditional read of list of files in a folder based on ETag --- .../operations/RemoteOperationResult.java | 34 +++++++------ .../files/ReadRemoteFolderOperation.java | 50 ++++++++++++------- 2 files changed, 52 insertions(+), 32 deletions(-) diff --git a/src/com/owncloud/android/lib/common/operations/RemoteOperationResult.java b/src/com/owncloud/android/lib/common/operations/RemoteOperationResult.java index cce359b6..c0227b8f 100644 --- a/src/com/owncloud/android/lib/common/operations/RemoteOperationResult.java +++ b/src/com/owncloud/android/lib/common/operations/RemoteOperationResult.java @@ -61,8 +61,8 @@ import javax.net.ssl.SSLException; */ public class RemoteOperationResult implements Serializable { - /** Generated - should be refreshed every time the class changes!! */; - private static final long serialVersionUID = 1129130415603799707L; + /** Generated - should be refreshed every time the class changes!! */; + private static final long serialVersionUID = -1909603208238358633L; private static final String TAG = RemoteOperationResult.class.getSimpleName(); @@ -100,19 +100,20 @@ public class RemoteOperationResult implements Serializable { ACCOUNT_NOT_THE_SAME, INVALID_CHARACTER_IN_NAME, SHARE_NOT_FOUND, - LOCAL_STORAGE_NOT_REMOVED, - FORBIDDEN, - SHARE_FORBIDDEN, - OK_REDIRECT_TO_NON_SECURE_CONNECTION, - INVALID_MOVE_INTO_DESCENDANT, + LOCAL_STORAGE_NOT_REMOVED, + FORBIDDEN, + SHARE_FORBIDDEN, + OK_REDIRECT_TO_NON_SECURE_CONNECTION, + INVALID_MOVE_INTO_DESCENDANT, INVALID_COPY_INTO_DESCENDANT, - PARTIAL_MOVE_DONE, + PARTIAL_MOVE_DONE, PARTIAL_COPY_DONE, SHARE_WRONG_PARAMETER, WRONG_SERVER_RESPONSE, INVALID_CHARACTER_DETECT_IN_SERVER, DELAYED_FOR_WIFI, - LOCAL_FILE_NOT_FOUND + LOCAL_FILE_NOT_FOUND, + NOT_MODIFIED } private boolean mSuccess = false; @@ -127,7 +128,7 @@ public class RemoteOperationResult implements Serializable { public RemoteOperationResult(ResultCode code) { mCode = code; - mSuccess = (code == ResultCode.OK || code == ResultCode.OK_SSL || + mSuccess = (code == ResultCode.OK || code == ResultCode.OK_SSL || code == ResultCode.OK_NO_SSL || code == ResultCode.OK_REDIRECT_TO_NON_SECURE_CONNECTION); mData = null; @@ -157,9 +158,11 @@ public class RemoteOperationResult implements Serializable { case HttpStatus.SC_INSUFFICIENT_STORAGE: mCode = ResultCode.QUOTA_EXCEEDED; break; - case HttpStatus.SC_FORBIDDEN: - mCode = ResultCode.FORBIDDEN; + case HttpStatus.SC_FORBIDDEN: + mCode = ResultCode.FORBIDDEN; break; + case HttpStatus.SC_NOT_MODIFIED: + mCode = ResultCode.NOT_MODIFIED; default: mCode = ResultCode.UNHANDLED_HTTP_CODE; Log_OC.d(TAG, "RemoteOperationResult has processed UNHANDLED_HTTP_CODE: " + @@ -406,10 +409,13 @@ public class RemoteOperationResult implements Serializable { return "The file name contains an forbidden character"; } else if (mCode == ResultCode.FILE_NOT_FOUND) { - return "Local file does not exist"; + return "Local file does not exist"; - } else if (mCode == ResultCode.SYNC_CONFLICT) { + } else if (mCode == ResultCode.SYNC_CONFLICT) { return "Synchronization conflict"; + + } else if (mCode == ResultCode.NOT_MODIFIED) { + return "Resource in server was not modified"; } return "Operation finished with HTTP status code " + mHttpCode + " (" + diff --git a/src/com/owncloud/android/lib/resources/files/ReadRemoteFolderOperation.java b/src/com/owncloud/android/lib/resources/files/ReadRemoteFolderOperation.java index ef2a06fa..de4e15f1 100644 --- a/src/com/owncloud/android/lib/resources/files/ReadRemoteFolderOperation.java +++ b/src/com/owncloud/android/lib/resources/files/ReadRemoteFolderOperation.java @@ -47,28 +47,42 @@ import com.owncloud.android.lib.common.utils.Log_OC; public class ReadRemoteFolderOperation extends RemoteOperation { - private static final String TAG = ReadRemoteFolderOperation.class.getSimpleName(); + private static final String TAG = ReadRemoteFolderOperation.class.getSimpleName(); - private String mRemotePath; - private ArrayList mFolderAndFiles; - - /** + private static final String IF_NONE_MATCH_HEADER = "If-None-Match"; + + private String mRemotePath; + private String mETagToNotMatch; + private ArrayList mFolderAndFiles; + + /** * Constructor * * @param remotePath Remote path of the file. */ - public ReadRemoteFolderOperation(String remotePath) { - mRemotePath = remotePath; - } + public ReadRemoteFolderOperation(String remotePath) { + mRemotePath = remotePath; + mETagToNotMatch = ""; + } - /** + /** + * Constructor + * + * @param remotePath Remote path of the file. + */ + public ReadRemoteFolderOperation(String remotePath, String eTagToNotMatch) { + mRemotePath = remotePath; + mETagToNotMatch = (eTagToNotMatch == null) ? "" : eTagToNotMatch; + } + + /** * Performs the read operation. * * @param client Client object to communicate with the remote ownCloud server. */ - @Override - protected RemoteOperationResult run(OwnCloudClient client) { - RemoteOperationResult result = null; + @Override + protected RemoteOperationResult run(OwnCloudClient client) { + RemoteOperationResult result = null; PropFindMethod query = null; try { @@ -76,13 +90,17 @@ public class ReadRemoteFolderOperation extends RemoteOperation { query = new PropFindMethod(client.getWebdavUri() + WebdavUtils.encodePath(mRemotePath), WebdavUtils.getAllPropSet(), // PropFind Properties DavConstants.DEPTH_1); + if (mETagToNotMatch.length() > 0) { + query.addRequestHeader(IF_NONE_MATCH_HEADER, "\"" + mETagToNotMatch + "\""); + } + int status = client.executeMethod(query); // check and process response boolean isSuccess = ( status == HttpStatus.SC_MULTI_STATUS || status == HttpStatus.SC_OK - ); + ); if (isSuccess) { // get data from remote folder MultiStatus dataInServer = query.getResponseBodyAsMultiStatus(); @@ -95,7 +113,7 @@ public class ReadRemoteFolderOperation extends RemoteOperation { result.setData(mFolderAndFiles); } } else { - // synchronization failed + // synchronization failed, or no change in folder (mETagToNotMatch matched) client.exhaustResponse(query.getResponseBodyAsStream()); result = new RemoteOperationResult(false, status, query.getResponseHeaders()); } @@ -120,10 +138,6 @@ public class ReadRemoteFolderOperation extends RemoteOperation { } return result; - } - - public boolean isMultiStatus(int status) { - return (status == HttpStatus.SC_MULTI_STATUS); } /** From b87d2c9e7c9c2fe7ca1628d339a0105e4826d4a2 Mon Sep 17 00:00:00 2001 From: "David A. Velasco" Date: Tue, 22 Nov 2016 15:32:43 +0100 Subject: [PATCH 2/4] Prevent conditional update of remote avatar to work around bug breaking refesh of root folder --- .../users/GetRemoteUserAvatarOperation.java | 37 +++++++++++++++++-- 1 file changed, 33 insertions(+), 4 deletions(-) diff --git a/src/com/owncloud/android/lib/resources/users/GetRemoteUserAvatarOperation.java b/src/com/owncloud/android/lib/resources/users/GetRemoteUserAvatarOperation.java index c051b0e9..c6e9fe93 100644 --- a/src/com/owncloud/android/lib/resources/users/GetRemoteUserAvatarOperation.java +++ b/src/com/owncloud/android/lib/resources/users/GetRemoteUserAvatarOperation.java @@ -28,6 +28,7 @@ package com.owncloud.android.lib.resources.users; import java.io.BufferedInputStream; import java.io.ByteArrayOutputStream; +import java.io.IOException; import java.io.InputStream; import java.util.ArrayList; @@ -73,6 +74,8 @@ public class GetRemoteUserAvatarOperation extends RemoteOperation { RemoteOperationResult result = null; GetMethod get = null; InputStream inputStream = null; + BufferedInputStream bis = null; + ByteArrayOutputStream bos = null; try { String uri = @@ -81,9 +84,19 @@ public class GetRemoteUserAvatarOperation extends RemoteOperation { ; Log_OC.d(TAG, "avatar URI: " + uri); get = new GetMethod(uri); + /* Conditioned call is corrupting the input stream of the connection. + Seems that response with 304 is also including the avatar in the response body, + though it's forbidden by HTTPS specification. Besides, HTTPClient library + assumes there is nothing in the response body, but the bytes are read + by the next request, resulting in an exception due to a corrupt status line + + Maybe when we have a real API we can enable this again. + if (mCurrentEtag != null && mCurrentEtag.length() > 0) { get.addRequestHeader(IF_NONE_MATCH_HEADER, "\"" + mCurrentEtag + "\""); } + */ + //get.addRequestHeader(OCS_API_HEADER, OCS_API_HEADER_VALUE); int status = client.executeMethod(get); if (isSuccess(status)) { @@ -111,8 +124,8 @@ public class GetRemoteUserAvatarOperation extends RemoteOperation { /// download will be performed to a buffer inputStream = get.getResponseBodyAsStream(); - BufferedInputStream bis = new BufferedInputStream(inputStream); - ByteArrayOutputStream bos = new ByteArrayOutputStream(totalToTransfer); + bis = new BufferedInputStream(inputStream); + bos = new ByteArrayOutputStream(totalToTransfer); long transferred = 0; byte[] bytes = new byte[4096]; @@ -147,8 +160,24 @@ public class GetRemoteUserAvatarOperation extends RemoteOperation { } finally { if (get != null) { - if (inputStream != null) { - client.exhaustResponse(inputStream); + try { + if (inputStream != null) { + client.exhaustResponse(inputStream); + if (bis != null) { + bis.close(); + } else { + inputStream.close(); + } + } + } catch (IOException i) { + Log_OC.e(TAG, "Unexpected exception closing input stream ", i); + } + try { + if (bos != null) { + bos.close(); + } + } catch (IOException o) { + Log_OC.e(TAG, "Unexpected exception closing output stream ", o); } get.releaseConnection(); } From 67d7217d13af47a6abb40e93a195f1ce1df005fd Mon Sep 17 00:00:00 2001 From: "David A. Velasco" Date: Tue, 22 Nov 2016 17:04:33 +0100 Subject: [PATCH 3/4] Revert "Allow conditional read of list of files in a folder based on ETag"; server does not work that way This reverts commit 5ff9062ea8fefb1bf2b3952b9d64daa4f9922a39. --- .../operations/RemoteOperationResult.java | 34 ++++++------- .../files/ReadRemoteFolderOperation.java | 50 +++++++------------ 2 files changed, 32 insertions(+), 52 deletions(-) diff --git a/src/com/owncloud/android/lib/common/operations/RemoteOperationResult.java b/src/com/owncloud/android/lib/common/operations/RemoteOperationResult.java index c0227b8f..cce359b6 100644 --- a/src/com/owncloud/android/lib/common/operations/RemoteOperationResult.java +++ b/src/com/owncloud/android/lib/common/operations/RemoteOperationResult.java @@ -61,8 +61,8 @@ import javax.net.ssl.SSLException; */ public class RemoteOperationResult implements Serializable { - /** Generated - should be refreshed every time the class changes!! */; - private static final long serialVersionUID = -1909603208238358633L; + /** Generated - should be refreshed every time the class changes!! */; + private static final long serialVersionUID = 1129130415603799707L; private static final String TAG = RemoteOperationResult.class.getSimpleName(); @@ -100,20 +100,19 @@ public class RemoteOperationResult implements Serializable { ACCOUNT_NOT_THE_SAME, INVALID_CHARACTER_IN_NAME, SHARE_NOT_FOUND, - LOCAL_STORAGE_NOT_REMOVED, - FORBIDDEN, - SHARE_FORBIDDEN, - OK_REDIRECT_TO_NON_SECURE_CONNECTION, - INVALID_MOVE_INTO_DESCENDANT, + LOCAL_STORAGE_NOT_REMOVED, + FORBIDDEN, + SHARE_FORBIDDEN, + OK_REDIRECT_TO_NON_SECURE_CONNECTION, + INVALID_MOVE_INTO_DESCENDANT, INVALID_COPY_INTO_DESCENDANT, - PARTIAL_MOVE_DONE, + PARTIAL_MOVE_DONE, PARTIAL_COPY_DONE, SHARE_WRONG_PARAMETER, WRONG_SERVER_RESPONSE, INVALID_CHARACTER_DETECT_IN_SERVER, DELAYED_FOR_WIFI, - LOCAL_FILE_NOT_FOUND, - NOT_MODIFIED + LOCAL_FILE_NOT_FOUND } private boolean mSuccess = false; @@ -128,7 +127,7 @@ public class RemoteOperationResult implements Serializable { public RemoteOperationResult(ResultCode code) { mCode = code; - mSuccess = (code == ResultCode.OK || code == ResultCode.OK_SSL || + mSuccess = (code == ResultCode.OK || code == ResultCode.OK_SSL || code == ResultCode.OK_NO_SSL || code == ResultCode.OK_REDIRECT_TO_NON_SECURE_CONNECTION); mData = null; @@ -158,11 +157,9 @@ public class RemoteOperationResult implements Serializable { case HttpStatus.SC_INSUFFICIENT_STORAGE: mCode = ResultCode.QUOTA_EXCEEDED; break; - case HttpStatus.SC_FORBIDDEN: - mCode = ResultCode.FORBIDDEN; + case HttpStatus.SC_FORBIDDEN: + mCode = ResultCode.FORBIDDEN; break; - case HttpStatus.SC_NOT_MODIFIED: - mCode = ResultCode.NOT_MODIFIED; default: mCode = ResultCode.UNHANDLED_HTTP_CODE; Log_OC.d(TAG, "RemoteOperationResult has processed UNHANDLED_HTTP_CODE: " + @@ -409,13 +406,10 @@ public class RemoteOperationResult implements Serializable { return "The file name contains an forbidden character"; } else if (mCode == ResultCode.FILE_NOT_FOUND) { - return "Local file does not exist"; + return "Local file does not exist"; - } else if (mCode == ResultCode.SYNC_CONFLICT) { + } else if (mCode == ResultCode.SYNC_CONFLICT) { return "Synchronization conflict"; - - } else if (mCode == ResultCode.NOT_MODIFIED) { - return "Resource in server was not modified"; } return "Operation finished with HTTP status code " + mHttpCode + " (" + diff --git a/src/com/owncloud/android/lib/resources/files/ReadRemoteFolderOperation.java b/src/com/owncloud/android/lib/resources/files/ReadRemoteFolderOperation.java index de4e15f1..ef2a06fa 100644 --- a/src/com/owncloud/android/lib/resources/files/ReadRemoteFolderOperation.java +++ b/src/com/owncloud/android/lib/resources/files/ReadRemoteFolderOperation.java @@ -47,42 +47,28 @@ import com.owncloud.android.lib.common.utils.Log_OC; public class ReadRemoteFolderOperation extends RemoteOperation { - private static final String TAG = ReadRemoteFolderOperation.class.getSimpleName(); + private static final String TAG = ReadRemoteFolderOperation.class.getSimpleName(); - private static final String IF_NONE_MATCH_HEADER = "If-None-Match"; - - private String mRemotePath; - private String mETagToNotMatch; - private ArrayList mFolderAndFiles; - - /** + private String mRemotePath; + private ArrayList mFolderAndFiles; + + /** * Constructor * * @param remotePath Remote path of the file. */ - public ReadRemoteFolderOperation(String remotePath) { - mRemotePath = remotePath; - mETagToNotMatch = ""; - } + public ReadRemoteFolderOperation(String remotePath) { + mRemotePath = remotePath; + } - /** - * Constructor - * - * @param remotePath Remote path of the file. - */ - public ReadRemoteFolderOperation(String remotePath, String eTagToNotMatch) { - mRemotePath = remotePath; - mETagToNotMatch = (eTagToNotMatch == null) ? "" : eTagToNotMatch; - } - - /** + /** * Performs the read operation. * * @param client Client object to communicate with the remote ownCloud server. */ - @Override - protected RemoteOperationResult run(OwnCloudClient client) { - RemoteOperationResult result = null; + @Override + protected RemoteOperationResult run(OwnCloudClient client) { + RemoteOperationResult result = null; PropFindMethod query = null; try { @@ -90,17 +76,13 @@ public class ReadRemoteFolderOperation extends RemoteOperation { query = new PropFindMethod(client.getWebdavUri() + WebdavUtils.encodePath(mRemotePath), WebdavUtils.getAllPropSet(), // PropFind Properties DavConstants.DEPTH_1); - if (mETagToNotMatch.length() > 0) { - query.addRequestHeader(IF_NONE_MATCH_HEADER, "\"" + mETagToNotMatch + "\""); - } - int status = client.executeMethod(query); // check and process response boolean isSuccess = ( status == HttpStatus.SC_MULTI_STATUS || status == HttpStatus.SC_OK - ); + ); if (isSuccess) { // get data from remote folder MultiStatus dataInServer = query.getResponseBodyAsMultiStatus(); @@ -113,7 +95,7 @@ public class ReadRemoteFolderOperation extends RemoteOperation { result.setData(mFolderAndFiles); } } else { - // synchronization failed, or no change in folder (mETagToNotMatch matched) + // synchronization failed client.exhaustResponse(query.getResponseBodyAsStream()); result = new RemoteOperationResult(false, status, query.getResponseHeaders()); } @@ -138,6 +120,10 @@ public class ReadRemoteFolderOperation extends RemoteOperation { } return result; + } + + public boolean isMultiStatus(int status) { + return (status == HttpStatus.SC_MULTI_STATUS); } /** From fdebb05a13124f4f89e142415a0fe7c91166d902 Mon Sep 17 00:00:00 2001 From: "David A. Velasco" Date: Fri, 9 Dec 2016 10:15:30 +0100 Subject: [PATCH 4/4] Update test for GetRemoteUserAvatarOperation after retrieval conditioned by ETag was disabled --- .../android/lib/test_project/test/GetUserAvatarTest.java | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/test_client/tests/src/com/owncloud/android/lib/test_project/test/GetUserAvatarTest.java b/test_client/tests/src/com/owncloud/android/lib/test_project/test/GetUserAvatarTest.java index e84ccd26..fc61ed31 100644 --- a/test_client/tests/src/com/owncloud/android/lib/test_project/test/GetUserAvatarTest.java +++ b/test_client/tests/src/com/owncloud/android/lib/test_project/test/GetUserAvatarTest.java @@ -68,7 +68,11 @@ public class GetUserAvatarTest extends RemoteTest { /** * Test get user avatar only if changed, but wasn't changed + * + * DISABLED: conditioned call has been disabled due to problems with the network stack; + * see comment in src/com/owncloud/android/lib/resources/users/GetRemoteUserAvatarOperation.java#87 */ + /* public void testGetUserAvatarOnlyIfChangedAfterUnchanged() { RemoteOperationResult result = mActivity.getUserAvatar(AVATAR_DIMENSION, null); ResultData userAvatar = (ResultData) result.getData().get(0); @@ -79,6 +83,7 @@ public class GetUserAvatarTest extends RemoteTest { assertFalse(result.isSuccess()); assertTrue(result.getHttpCode() == HttpStatus.SC_NOT_MODIFIED); } + */ /** * Test get user avatar only if changed, and was changed