From 8bf276377c219b490a52070035ea1ffeb03a77a7 Mon Sep 17 00:00:00 2001 From: "David A. Velasco" Date: Tue, 21 Jul 2015 16:50:48 +0200 Subject: [PATCH 1/5] Minor improvements for upload cancelation and conflicts in synchronization --- .../lib/common/operations/RemoteOperationResult.java | 9 +++++++-- src/com/owncloud/android/lib/common/utils/Log_OC.java | 3 +-- .../files/ChunkedUploadRemoteFileOperation.java | 2 +- .../lib/resources/files/DownloadRemoteFileOperation.java | 3 +++ .../lib/resources/files/UploadRemoteFileOperation.java | 6 ++---- 5 files changed, 14 insertions(+), 9 deletions(-) diff --git a/src/com/owncloud/android/lib/common/operations/RemoteOperationResult.java b/src/com/owncloud/android/lib/common/operations/RemoteOperationResult.java index f8eb8cc1..ceb3f25f 100644 --- a/src/com/owncloud/android/lib/common/operations/RemoteOperationResult.java +++ b/src/com/owncloud/android/lib/common/operations/RemoteOperationResult.java @@ -390,11 +390,16 @@ public class RemoteOperationResult implements Serializable { } else if (mCode == ResultCode.ACCOUNT_NOT_THE_SAME) { return "Authenticated with a different account than the one updating"; + } else if (mCode == ResultCode.INVALID_CHARACTER_IN_NAME) { 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) { + return "Synchronization conflict"; + } return "Operation finished with HTTP status code " + mHttpCode + " (" + (isSuccess() ? "success" : "fail") + ")"; diff --git a/src/com/owncloud/android/lib/common/utils/Log_OC.java b/src/com/owncloud/android/lib/common/utils/Log_OC.java index 8ed8c52c..4d4760e6 100644 --- a/src/com/owncloud/android/lib/common/utils/Log_OC.java +++ b/src/com/owncloud/android/lib/common/utils/Log_OC.java @@ -32,8 +32,7 @@ public class Log_OC { } public static void i(String TAG, String message){ - - // Write the log message to the file + Log.i(TAG, message); appendLog(TAG+" : "+ message); } diff --git a/src/com/owncloud/android/lib/resources/files/ChunkedUploadRemoteFileOperation.java b/src/com/owncloud/android/lib/resources/files/ChunkedUploadRemoteFileOperation.java index 1fa9d5fd..f8c19d3e 100644 --- a/src/com/owncloud/android/lib/resources/files/ChunkedUploadRemoteFileOperation.java +++ b/src/com/owncloud/android/lib/resources/files/ChunkedUploadRemoteFileOperation.java @@ -54,7 +54,7 @@ public class ChunkedUploadRemoteFileOperation extends UploadRemoteFileOperation } @Override - protected int uploadFile(OwnCloudClient client) throws HttpException, IOException { + protected int uploadFile(OwnCloudClient client) throws IOException { int status = -1; FileChannel channel = null; diff --git a/src/com/owncloud/android/lib/resources/files/DownloadRemoteFileOperation.java b/src/com/owncloud/android/lib/resources/files/DownloadRemoteFileOperation.java index bcb209db..95adc804 100644 --- a/src/com/owncloud/android/lib/resources/files/DownloadRemoteFileOperation.java +++ b/src/com/owncloud/android/lib/resources/files/DownloadRemoteFileOperation.java @@ -140,6 +140,9 @@ public class DownloadRemoteFileOperation extends RemoteOperation { if (transferred == totalToTransfer) { // Check if the file is completed savedFile = true; Header modificationTime = mGet.getResponseHeader("Last-Modified"); + if (modificationTime == null) { + modificationTime = mGet.getResponseHeader("last-modified"); + } if (modificationTime != null) { Date d = WebdavUtils.parseResponseDate((String) modificationTime.getValue()); mModificationTimestamp = (d != null) ? d.getTime() : 0; diff --git a/src/com/owncloud/android/lib/resources/files/UploadRemoteFileOperation.java b/src/com/owncloud/android/lib/resources/files/UploadRemoteFileOperation.java index 25dc87f5..d6e90922 100644 --- a/src/com/owncloud/android/lib/resources/files/UploadRemoteFileOperation.java +++ b/src/com/owncloud/android/lib/resources/files/UploadRemoteFileOperation.java @@ -102,8 +102,7 @@ public class UploadRemoteFileOperation extends RemoteOperation { (mPutMethod != null ? mPutMethod.getResponseHeaders() : null)); } } catch (Exception e) { - // TODO something cleaner with cancellations - if (mCancellationRequested.get()) { + if (mCancellationRequested.get() && !(e instanceof OperationCancelledException)) { result = new RemoteOperationResult(new OperationCancelledException()); } else { result = new RemoteOperationResult(e); @@ -117,8 +116,7 @@ public class UploadRemoteFileOperation extends RemoteOperation { status == HttpStatus.SC_NO_CONTENT)); } - protected int uploadFile(OwnCloudClient client) throws HttpException, IOException, - OperationCancelledException { + protected int uploadFile(OwnCloudClient client) throws IOException { int status = -1; try { File f = new File(mLocalPath); From 522ea30da06e2c1a6a7243847cd349a5a3a0149d Mon Sep 17 00:00:00 2001 From: "David A. Velasco" Date: Wed, 22 Jul 2015 10:40:50 +0200 Subject: [PATCH 2/5] Simplified flow of interruption of uploads in cancellations --- .../ChunkedUploadRemoteFileOperation.java | 5 ++- .../files/UploadRemoteFileOperation.java | 33 ++++++++++--------- 2 files changed, 21 insertions(+), 17 deletions(-) diff --git a/src/com/owncloud/android/lib/resources/files/ChunkedUploadRemoteFileOperation.java b/src/com/owncloud/android/lib/resources/files/ChunkedUploadRemoteFileOperation.java index f8c19d3e..281745ad 100644 --- a/src/com/owncloud/android/lib/resources/files/ChunkedUploadRemoteFileOperation.java +++ b/src/com/owncloud/android/lib/resources/files/ChunkedUploadRemoteFileOperation.java @@ -32,7 +32,6 @@ import java.io.RandomAccessFile; import java.nio.channels.FileChannel; import java.util.Random; -import org.apache.commons.httpclient.HttpException; import org.apache.commons.httpclient.methods.PutMethod; import com.owncloud.android.lib.common.OwnCloudClient; @@ -85,6 +84,10 @@ public class ChunkedUploadRemoteFileOperation extends UploadRemoteFileOperation mPutMethod.addRequestHeader(OC_TOTAL_LENGTH_HEADER, String.valueOf(file.length())); ((ChunkFromFileChannelRequestEntity) mEntity).setOffset(offset); mPutMethod.setRequestEntity(mEntity); + if (mCancellationRequested.get()) { + mPutMethod.abort(); + // next method will throw an exception + } status = client.executeMethod(mPutMethod); if (status == 400) { diff --git a/src/com/owncloud/android/lib/resources/files/UploadRemoteFileOperation.java b/src/com/owncloud/android/lib/resources/files/UploadRemoteFileOperation.java index d6e90922..7e4ad499 100644 --- a/src/com/owncloud/android/lib/resources/files/UploadRemoteFileOperation.java +++ b/src/com/owncloud/android/lib/resources/files/UploadRemoteFileOperation.java @@ -67,7 +67,7 @@ public class UploadRemoteFileOperation extends RemoteOperation { protected PutMethod mPutMethod = null; protected boolean mForbiddenCharsInServer = false; - private final AtomicBoolean mCancellationRequested = new AtomicBoolean(false); + protected final AtomicBoolean mCancellationRequested = new AtomicBoolean(false); protected Set mDataTransferListeners = new HashSet(); protected RequestEntity mEntity = null; @@ -83,27 +83,28 @@ public class UploadRemoteFileOperation extends RemoteOperation { RemoteOperationResult result = null; try { - // / perform the upload - synchronized (mCancellationRequested) { - if (mCancellationRequested.get()) { - throw new OperationCancelledException(); + mPutMethod = new PutMethod(client.getWebdavUri() + WebdavUtils.encodePath(mRemotePath)); + + if (mCancellationRequested.get()) { + // the operation was cancelled before getting it's turn to be executed in the queue of uploads + result = new RemoteOperationResult(new OperationCancelledException()); + + } else { + // perform the upload + int status = uploadFile(client); + if (mForbiddenCharsInServer){ + result = new RemoteOperationResult( + RemoteOperationResult.ResultCode.INVALID_CHARACTER_DETECT_IN_SERVER); } else { - mPutMethod = new PutMethod(client.getWebdavUri() + - WebdavUtils.encodePath(mRemotePath)); + result = new RemoteOperationResult(isSuccess(status), status, + (mPutMethod != null ? mPutMethod.getResponseHeaders() : null)); } } - int status = uploadFile(client); - if (mForbiddenCharsInServer){ - result = new RemoteOperationResult( - RemoteOperationResult.ResultCode.INVALID_CHARACTER_DETECT_IN_SERVER); - } else { - result = new RemoteOperationResult(isSuccess(status), status, - (mPutMethod != null ? mPutMethod.getResponseHeaders() : null)); - } } catch (Exception e) { - if (mCancellationRequested.get() && !(e instanceof OperationCancelledException)) { + if (mPutMethod != null && mPutMethod.isAborted()) { result = new RemoteOperationResult(new OperationCancelledException()); + } else { result = new RemoteOperationResult(e); } From 092c790030ef42b34352225e9c1d8480a0e38674 Mon Sep 17 00:00:00 2001 From: "David A. Velasco" Date: Tue, 28 Jul 2015 16:21:27 +0200 Subject: [PATCH 3/5] Add capability to reatin ETag value in download operations --- .../files/DownloadRemoteFileOperation.java | 22 ++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/src/com/owncloud/android/lib/resources/files/DownloadRemoteFileOperation.java b/src/com/owncloud/android/lib/resources/files/DownloadRemoteFileOperation.java index 95adc804..3873b5ec 100644 --- a/src/com/owncloud/android/lib/resources/files/DownloadRemoteFileOperation.java +++ b/src/com/owncloud/android/lib/resources/files/DownloadRemoteFileOperation.java @@ -61,6 +61,7 @@ public class DownloadRemoteFileOperation extends RemoteOperation { private Set mDataTransferListeners = new HashSet(); private final AtomicBoolean mCancellationRequested = new AtomicBoolean(false); private long mModificationTimestamp = 0; + private String mEtag = ""; private GetMethod mGet; private String mRemotePath; @@ -146,9 +147,25 @@ public class DownloadRemoteFileOperation extends RemoteOperation { if (modificationTime != null) { Date d = WebdavUtils.parseResponseDate((String) modificationTime.getValue()); mModificationTimestamp = (d != null) ? d.getTime() : 0; - } + } else { + Log_OC.e(TAG, "Could not read modification time from response downloading " + mRemotePath); + } + Header eTag = mGet.getResponseHeader("ETag"); + if (eTag == null) { + eTag = mGet.getResponseHeader("etag"); + } + if (eTag != null) { + mEtag = eTag.getValue(); + if (mEtag.charAt(0) == '"' && mEtag.charAt(mEtag.length() - 1) == '"') { + mEtag = mEtag.substring(1, mEtag.length() - 1); + } + } else { + Log_OC.e(TAG, "Could not read eTag from response downloading " + mRemotePath); + } + } else { client.exhaustResponse(mGet.getResponseBodyAsStream()); + // TODO some kind of error control! } } else { @@ -193,4 +210,7 @@ public class DownloadRemoteFileOperation extends RemoteOperation { return mModificationTimestamp; } + public String getEtag() { + return mEtag; + } } From f61b466124ad3a3ea2c905ebdb60ac3184d4dc7f Mon Sep 17 00:00:00 2001 From: "David A. Velasco" Date: Mon, 28 Sep 2015 10:49:45 +0200 Subject: [PATCH 4/5] Improved ETag parsing --- .../lib/common/network/WebdavEntry.java | 2 +- .../lib/common/network/WebdavUtils.java | 45 +++++++++++++++++++ .../files/DownloadRemoteFileOperation.java | 13 ++---- 3 files changed, 49 insertions(+), 11 deletions(-) diff --git a/src/com/owncloud/android/lib/common/network/WebdavEntry.java b/src/com/owncloud/android/lib/common/network/WebdavEntry.java index 1979cbc8..64d21e7e 100644 --- a/src/com/owncloud/android/lib/common/network/WebdavEntry.java +++ b/src/com/owncloud/android/lib/common/network/WebdavEntry.java @@ -129,7 +129,7 @@ public class WebdavEntry { prop = propSet.get(DavPropertyName.GETETAG); if (prop != null) { mEtag = (String) prop.getValue(); - mEtag = mEtag.substring(1, mEtag.length()-1); + mEtag = WebdavUtils.parseEtag(mEtag); } // {DAV:}quota-used-bytes diff --git a/src/com/owncloud/android/lib/common/network/WebdavUtils.java b/src/com/owncloud/android/lib/common/network/WebdavUtils.java index bf2e986d..c5d2e829 100644 --- a/src/com/owncloud/android/lib/common/network/WebdavUtils.java +++ b/src/com/owncloud/android/lib/common/network/WebdavUtils.java @@ -32,6 +32,8 @@ import java.util.Locale; import android.net.Uri; +import org.apache.commons.httpclient.Header; +import org.apache.commons.httpclient.HttpMethod; import org.apache.jackrabbit.webdav.property.DavPropertyName; import org.apache.jackrabbit.webdav.property.DavPropertyNameSet; import org.apache.jackrabbit.webdav.xml.Namespace; @@ -131,4 +133,47 @@ public class WebdavUtils { return propSet; } + + /** + * + * @param rawEtag + * @return + */ + public static String parseEtag(String rawEtag) { + if (rawEtag == null || rawEtag.length() == 0) { + return ""; + } + if (rawEtag.endsWith("-gzip")) { + rawEtag = rawEtag.substring(0, rawEtag.length() - 5); + } + if (rawEtag.length() >= 2 && rawEtag.startsWith("\"") && rawEtag.endsWith("\"")) { + rawEtag = rawEtag.substring(1, rawEtag.length() - 1); + } + return rawEtag; + } + + + /** + * + * @param method + * @return + */ + public static String getEtagFromResponse(HttpMethod method) { + Header eTag = method.getResponseHeader("OC-ETag"); + if (eTag == null) { + eTag = method.getResponseHeader("oc-etag"); + } + if (eTag == null) { + eTag = method.getResponseHeader("ETag"); + } + if (eTag == null) { + eTag = method.getResponseHeader("etag"); + } + String result = ""; + if (eTag != null) { + result = parseEtag(eTag.getValue()); + } + return result; + } + } diff --git a/src/com/owncloud/android/lib/resources/files/DownloadRemoteFileOperation.java b/src/com/owncloud/android/lib/resources/files/DownloadRemoteFileOperation.java index 3873b5ec..b9c404c3 100644 --- a/src/com/owncloud/android/lib/resources/files/DownloadRemoteFileOperation.java +++ b/src/com/owncloud/android/lib/resources/files/DownloadRemoteFileOperation.java @@ -150,16 +150,9 @@ public class DownloadRemoteFileOperation extends RemoteOperation { } else { Log_OC.e(TAG, "Could not read modification time from response downloading " + mRemotePath); } - Header eTag = mGet.getResponseHeader("ETag"); - if (eTag == null) { - eTag = mGet.getResponseHeader("etag"); - } - if (eTag != null) { - mEtag = eTag.getValue(); - if (mEtag.charAt(0) == '"' && mEtag.charAt(mEtag.length() - 1) == '"') { - mEtag = mEtag.substring(1, mEtag.length() - 1); - } - } else { + + mEtag = WebdavUtils.getEtagFromResponse(mGet); + if (mEtag.length() == 0) { Log_OC.e(TAG, "Could not read eTag from response downloading " + mRemotePath); } From eb4f8a723490a18c5a07fbdc3bf19928c49c4a10 Mon Sep 17 00:00:00 2001 From: "David A. Velasco" Date: Mon, 28 Sep 2015 11:13:44 +0200 Subject: [PATCH 5/5] Added constructors to grant that upload operations only succeed if the remote copy keeps a given ETag --- .../ChunkedUploadRemoteFileOperation.java | 25 +++++++++++++++---- .../files/UploadRemoteFileOperation.java | 13 ++++++++-- 2 files changed, 31 insertions(+), 7 deletions(-) diff --git a/src/com/owncloud/android/lib/resources/files/ChunkedUploadRemoteFileOperation.java b/src/com/owncloud/android/lib/resources/files/ChunkedUploadRemoteFileOperation.java index 281745ad..2bacc883 100644 --- a/src/com/owncloud/android/lib/resources/files/ChunkedUploadRemoteFileOperation.java +++ b/src/com/owncloud/android/lib/resources/files/ChunkedUploadRemoteFileOperation.java @@ -46,10 +46,17 @@ public class ChunkedUploadRemoteFileOperation extends UploadRemoteFileOperation public static final long CHUNK_SIZE = 1024000; private static final String OC_CHUNKED_HEADER = "OC-Chunked"; + private static final String OC_CHUNK_SIZE_HEADER = "OC-Chunk-Size"; private static final String TAG = ChunkedUploadRemoteFileOperation.class.getSimpleName(); public ChunkedUploadRemoteFileOperation(String storagePath, String remotePath, String mimeType){ - super(storagePath, remotePath, mimeType); + super(storagePath, remotePath, mimeType); + } + + public ChunkedUploadRemoteFileOperation( + String storagePath, String remotePath, String mimeType, String requiredEtag + ){ + super(storagePath, remotePath, mimeType, requiredEtag); } @Override @@ -63,8 +70,6 @@ public class ChunkedUploadRemoteFileOperation extends UploadRemoteFileOperation raf = new RandomAccessFile(file, "r"); channel = raf.getChannel(); mEntity = new ChunkFromFileChannelRequestEntity(channel, mMimeType, CHUNK_SIZE, file); - //((ProgressiveDataTransferer)mEntity). - // addDatatransferProgressListeners(getDataTransferListeners()); synchronized (mDataTransferListeners) { ((ProgressiveDataTransferer)mEntity) .addDatatransferProgressListeners(mDataTransferListeners); @@ -73,15 +78,25 @@ public class ChunkedUploadRemoteFileOperation extends UploadRemoteFileOperation long offset = 0; String uriPrefix = client.getWebdavUri() + WebdavUtils.encodePath(mRemotePath) + "-chunking-" + Math.abs((new Random()).nextInt(9000)+1000) + "-" ; - long chunkCount = (long) Math.ceil((double)file.length() / CHUNK_SIZE); + long totalLength = file.length(); + long chunkCount = (long) Math.ceil((double)totalLength / CHUNK_SIZE); + String chunkSizeStr = String.valueOf(CHUNK_SIZE); + String totalLengthStr = String.valueOf(file.length()); for (int chunkIndex = 0; chunkIndex < chunkCount ; chunkIndex++, offset += CHUNK_SIZE) { + if (chunkIndex == chunkCount - 1) { + chunkSizeStr = String.valueOf(CHUNK_SIZE * chunkCount - totalLength); + } if (mPutMethod != null) { mPutMethod.releaseConnection(); // let the connection available // for other methods } mPutMethod = new PutMethod(uriPrefix + chunkCount + "-" + chunkIndex); + if (mRequiredEtag != null && mRequiredEtag.length() > 0) { + mPutMethod.addRequestHeader(IF_MATCH_HEADER, "\"" + mRequiredEtag + "\""); + } mPutMethod.addRequestHeader(OC_CHUNKED_HEADER, OC_CHUNKED_HEADER); - mPutMethod.addRequestHeader(OC_TOTAL_LENGTH_HEADER, String.valueOf(file.length())); + mPutMethod.addRequestHeader(OC_CHUNK_SIZE_HEADER, chunkSizeStr); + mPutMethod.addRequestHeader(OC_TOTAL_LENGTH_HEADER, totalLengthStr); ((ChunkFromFileChannelRequestEntity) mEntity).setOffset(offset); mPutMethod.setRequestEntity(mEntity); if (mCancellationRequested.get()) { diff --git a/src/com/owncloud/android/lib/resources/files/UploadRemoteFileOperation.java b/src/com/owncloud/android/lib/resources/files/UploadRemoteFileOperation.java index 7e4ad499..422b17cf 100644 --- a/src/com/owncloud/android/lib/resources/files/UploadRemoteFileOperation.java +++ b/src/com/owncloud/android/lib/resources/files/UploadRemoteFileOperation.java @@ -32,7 +32,6 @@ import java.util.HashSet; import java.util.Set; import java.util.concurrent.atomic.AtomicBoolean; -import org.apache.commons.httpclient.HttpException; import org.apache.commons.httpclient.methods.PutMethod; import org.apache.commons.httpclient.methods.RequestEntity; import org.apache.http.HttpStatus; @@ -60,12 +59,14 @@ public class UploadRemoteFileOperation extends RemoteOperation { private static final String TAG = UploadRemoteFileOperation.class.getSimpleName(); protected static final String OC_TOTAL_LENGTH_HEADER = "OC-Total-Length"; + protected static final String IF_MATCH_HEADER = "If-Match"; protected String mLocalPath; protected String mRemotePath; protected String mMimeType; protected PutMethod mPutMethod = null; protected boolean mForbiddenCharsInServer = false; + protected String mRequiredEtag = null; protected final AtomicBoolean mCancellationRequested = new AtomicBoolean(false); protected Set mDataTransferListeners = new HashSet(); @@ -75,7 +76,12 @@ public class UploadRemoteFileOperation extends RemoteOperation { public UploadRemoteFileOperation(String localPath, String remotePath, String mimeType) { mLocalPath = localPath; mRemotePath = remotePath; - mMimeType = mimeType; + mMimeType = mimeType; + } + + public UploadRemoteFileOperation(String localPath, String remotePath, String mimeType, String requiredEtag) { + this(localPath, remotePath, mimeType); + mRequiredEtag = requiredEtag; } @Override @@ -126,6 +132,9 @@ public class UploadRemoteFileOperation extends RemoteOperation { ((ProgressiveDataTransferer)mEntity) .addDatatransferProgressListeners(mDataTransferListeners); } + if (mRequiredEtag != null && mRequiredEtag.length() > 0) { + mPutMethod.addRequestHeader(IF_MATCH_HEADER, "\"" + mRequiredEtag + "\""); + } mPutMethod.addRequestHeader(OC_TOTAL_LENGTH_HEADER, String.valueOf(f.length())); mPutMethod.setRequestEntity(mEntity); status = client.executeMethod(mPutMethod);