Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 33bfb0b0 authored by Mario Đanić's avatar Mario Đanić Committed by GitHub
Browse files

Merge pull request #30 from nextcloud/improveErrorFeedback

Improve error feedback
parents 37b86b2c be075de6
Loading
Loading
Loading
Loading
+148 −2
Original line number Diff line number Diff line
@@ -36,6 +36,7 @@ import com.owncloud.android.lib.resources.notifications.models.PushResponse;
import org.apache.commons.httpclient.ConnectTimeoutException;
import org.apache.commons.httpclient.Header;
import org.apache.commons.httpclient.HttpException;
import org.apache.commons.httpclient.HttpMethod;
import org.apache.commons.httpclient.HttpStatus;
import org.apache.jackrabbit.webdav.DavException;
import org.json.JSONException;
@@ -57,7 +58,7 @@ import javax.net.ssl.SSLException;

/**
 * The result of a remote operation required to an ownCloud server.
 * <p/>
 *
 * Provides a common classification of remote operation results for all the
 * application.
 *
@@ -124,6 +125,7 @@ public class RemoteOperationResult implements Serializable {

    private boolean mSuccess = false;
    private int mHttpCode = -1;
    private String mHttpPhrase = null;
    private Exception mException = null;
    private ResultCode mCode = ResultCode.UNKNOWN_ERROR;
    private String mRedirectedLocation;
@@ -134,6 +136,13 @@ public class RemoteOperationResult implements Serializable {
    private List<Notification> mNotificationData;
    private PushResponse mPushResponse;

    /**
     * Public constructor from result code.
     * <p>
     * To be used when the caller takes the responsibility of interpreting the result of a {@link RemoteOperation}
     *
     * @param code {@link ResultCode} decided by the caller.
     */
    public RemoteOperationResult(ResultCode code) {
        mCode = code;
		mSuccess = (code == ResultCode.OK || code == ResultCode.OK_SSL ||
@@ -228,9 +237,17 @@ public class RemoteOperationResult implements Serializable {
                            httpCode);
            }
        }

    }

    /**
     * Public constructor from exception.
     *
     * To be used when an exception prevented the end of the {@link RemoteOperation}.
     *
     * Determines a {@link ResultCode} depending on the type of the exception.
     *
     * @param e Exception that interrupted the {@link RemoteOperation}
     */
    public RemoteOperationResult(Exception e) {
        mException = e;

@@ -278,9 +295,134 @@ public class RemoteOperationResult implements Serializable {
        } else {
            mCode = ResultCode.UNKNOWN_ERROR;
        }
    }

    /**
     * Public constructor from separate elements of an HTTP or DAV response.
     *
     * To be used when the result needs to be interpreted from the response of an HTTP/DAV method.
     *
     * Determines a {@link ResultCode} from the already executed method received as a parameter. Generally,
     * will depend on the HTTP code and HTTP response headers received. In some cases will inspect also the
     * response body.
     *
     * @param success    The operation was considered successful or not.
     * @param httpMethod HTTP/DAV method already executed which response will be examined to interpret the
     *                   result.
     */
    public RemoteOperationResult(boolean success, HttpMethod httpMethod) throws IOException {
        this(
                success,
                httpMethod.getStatusCode(),
                httpMethod.getStatusText(),
                httpMethod.getResponseHeaders()
        );

        if (mHttpCode == HttpStatus.SC_BAD_REQUEST) {   // 400
            String bodyResponse = httpMethod.getResponseBodyAsString();
            // do not get for other HTTP codes!; could not be available

            if (bodyResponse != null && bodyResponse.length() > 0) {
                InputStream is = new ByteArrayInputStream(bodyResponse.getBytes());
                InvalidCharacterExceptionParser xmlParser = new InvalidCharacterExceptionParser();
                try {
                    if (xmlParser.parseXMLResponse(is)) {
                        mCode = ResultCode.INVALID_CHARACTER_DETECT_IN_SERVER;
                    }

                } catch (Exception e) {
                    Log_OC.w(TAG, "Error reading exception from server: " + e.getMessage());
                    // mCode stays as set in this(success, httpCode, headers)
                }
            }
        }
    }

    /**
     * Public constructor from separate elements of an HTTP or DAV response.
     *
     * To be used when the result needs to be interpreted from HTTP response elements that could come from
     * different requests (WARNING: black magic, try to avoid).
     *
     * If all the fields come from the same HTTP/DAV response, {@link #RemoteOperationResult(boolean, HttpMethod)}
     * should be used instead.
     *
     * Determines a {@link ResultCode} depending on the HTTP code and HTTP response headers received.
     *
     * @param success     The operation was considered successful or not.
     * @param httpCode    HTTP status code returned by an HTTP/DAV method.
     * @param httpPhrase  HTTP status line phrase returned by an HTTP/DAV method
     * @param httpHeaders HTTP response header returned by an HTTP/DAV method
     */
    public RemoteOperationResult(boolean success, int httpCode, String httpPhrase, Header[] httpHeaders) {
        this(success, httpCode, httpPhrase);
        if (httpHeaders != null) {
            Header current;
            for (Header httpHeader : httpHeaders) {
                current = httpHeader;
                if ("location".equals(current.getName().toLowerCase())) {
                    mRedirectedLocation = current.getValue();
                    continue;
                }
                if ("www-authenticate".equals(current.getName().toLowerCase())) {
                    mAuthenticate = current.getValue();
                    break;
                }
            }
        }
        if (isIdPRedirection()) {
            mCode = ResultCode.UNAUTHORIZED;    // overrides default ResultCode.UNKNOWN
        }
    }

    /**
     * Private constructor for results built interpreting a HTTP or DAV response.
     *
     * Determines a {@link ResultCode} depending of the type of the exception.
     *
     * @param success    Operation was successful or not.
     * @param httpCode   HTTP status code returned by the HTTP/DAV method.
     * @param httpPhrase HTTP status line phrase returned by the HTTP/DAV method
     */
    private RemoteOperationResult(boolean success, int httpCode, String httpPhrase) {
        mSuccess = success;
        mHttpCode = httpCode;
        mHttpPhrase = httpPhrase;

        if (success) {
            mCode = ResultCode.OK;

        } else if (httpCode > 0) {
            switch (httpCode) {
                case HttpStatus.SC_UNAUTHORIZED:                    // 401
                    mCode = ResultCode.UNAUTHORIZED;
                    break;
                case HttpStatus.SC_FORBIDDEN:                       // 403
                    mCode = ResultCode.FORBIDDEN;
                    break;
                case HttpStatus.SC_NOT_FOUND:                       // 404
                    mCode = ResultCode.FILE_NOT_FOUND;
                    break;
                case HttpStatus.SC_CONFLICT:                        // 409
                    mCode = ResultCode.CONFLICT;
                    break;
                case HttpStatus.SC_INTERNAL_SERVER_ERROR:           // 500
                    mCode = ResultCode.INSTANCE_NOT_CONFIGURED;     // assuming too much...
                    break;
                case HttpStatus.SC_SERVICE_UNAVAILABLE:             // 503
                    mCode = ResultCode.MAINTENANCE_MODE;
                    break;
                case HttpStatus.SC_INSUFFICIENT_STORAGE:            // 507
                    mCode = ResultCode.QUOTA_EXCEEDED;
                    break;
                default:
                    mCode = ResultCode.UNHANDLED_HTTP_CODE;         // UNKNOWN ERROR
                    Log_OC.d(TAG,
                            "RemoteOperationResult has processed UNHANDLED_HTTP_CODE: "
                                    + mHttpCode + " " + mHttpPhrase);
            }
        }
    }

    public void setData(ArrayList<Object> files) {
        mData = files;
@@ -318,6 +460,10 @@ public class RemoteOperationResult implements Serializable {
        return mHttpCode;
    }

    public String getHttpPhrase() {
        return mHttpPhrase;
    }

    public ResultCode getCode() {
        return mCode;
    }
+51 −47
Original line number Diff line number Diff line
@@ -31,15 +31,13 @@ import com.owncloud.android.lib.common.OwnCloudClient;
import com.owncloud.android.lib.common.network.ChunkFromFileChannelRequestEntity;
import com.owncloud.android.lib.common.network.ProgressiveDataTransferer;
import com.owncloud.android.lib.common.network.WebdavUtils;
import com.owncloud.android.lib.common.operations.InvalidCharacterExceptionParser;
import com.owncloud.android.lib.common.operations.RemoteOperationResult;
import com.owncloud.android.lib.common.utils.Log_OC;

import org.apache.commons.httpclient.methods.PutMethod;

import java.io.ByteArrayInputStream;
import java.io.File;
import java.io.IOException;
import java.io.InputStream;
import java.io.RandomAccessFile;
import java.nio.channels.FileChannel;
import java.util.Calendar;
@@ -70,8 +68,9 @@ public class ChunkedUploadRemoteFileOperation extends UploadRemoteFileOperation
    }
    
    @Override
    protected int uploadFile(OwnCloudClient client) throws IOException {
    protected RemoteOperationResult uploadFile(OwnCloudClient client) throws IOException {
        int status = -1;
        RemoteOperationResult result = null;

        FileChannel channel = null;
        RandomAccessFile raf = null;
@@ -111,44 +110,12 @@ public class ChunkedUploadRemoteFileOperation extends UploadRemoteFileOperation
                    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_CHUNK_SIZE_HEADER, chunkSizeStr);
                mPutMethod.addRequestHeader(OC_TOTAL_LENGTH_HEADER, totalLengthStr);
                mPutMethod.addRequestHeader(OC_CHUNK_X_OC_MTIME_HEADER, mFileLastModifTimestamp);

                ((ChunkFromFileChannelRequestEntity) mEntity).setOffset(offset);
                mPutMethod.setRequestEntity(mEntity);
                if (mCancellationRequested.get()) {
                    mPutMethod.abort();
                    // next method will throw an exception
                }

                if (chunkIndex == chunkCount - 1) {
                    // Added a high timeout to the last chunk due to when the last chunk
                    // arrives to the server with the last PUT, all chunks get assembled
                    // within that PHP request, so last one takes longer.
                    mPutMethod.getParams().setSoTimeout(LAST_CHUNK_TIMEOUT);
                }
                mPutMethod = createPutMethod(uriPrefix, chunkCount, chunkIndex, chunkSizeStr, totalLengthStr, offset);

                status = client.executeMethod(mPutMethod);

                if (status == 400) {
                    InvalidCharacterExceptionParser xmlParser =
                            new InvalidCharacterExceptionParser();
                    InputStream is = new ByteArrayInputStream(
                            mPutMethod.getResponseBodyAsString().getBytes());
                    try {
                        mForbiddenCharsInServer = xmlParser.parseXMLResponse(is);

                    } catch (Exception e) {
                        mForbiddenCharsInServer = false;
                        Log_OC.e(TAG, "Exception reading exception from server", e);
                    }
                }
                result = new RemoteOperationResult(isSuccess(status), mPutMethod);

                client.exhaustResponse(mPutMethod.getResponseBodyAsStream());
                Log_OC.d(TAG, "Upload of " + mLocalPath + " to " + mRemotePath +
@@ -156,11 +123,9 @@ public class ChunkedUploadRemoteFileOperation extends UploadRemoteFileOperation
                        ", HTTP result status " + status);

                if (isSuccess(status)) {

                    successfulChunks.add(String.valueOf(chunkIndex) + "_" + getDateAsString());
                } else {
                    SharedPreferences.Editor editor = sharedPref.edit();
                    editor.putStringSet(chunkId, successfulChunks).apply();
                    sharedPref.edit().putStringSet(chunkId, successfulChunks).apply();
                    break;
                }
            }
@@ -173,13 +138,52 @@ public class ChunkedUploadRemoteFileOperation extends UploadRemoteFileOperation
            }

            if (channel != null)
                try {
                    channel.close();
            if (raf != null)
                } catch (IOException e) {
                    Log_OC.e(TAG, "Error closing file channel!", e);
                }
            if (raf != null) {
                try {
                    raf.close();
                } catch (IOException e) {
                    Log_OC.e(TAG, "Error closing file access!", e);
                }
            }
            if (mPutMethod != null)
                mPutMethod.releaseConnection();    // let the connection available for other methods
        }
        return status;
        return result;
    }

    private PutMethod createPutMethod(String uriPrefix,
                                      long chunkCount,
                                      int chunkIndex,
                                      String chunkSizeStr,
                                      String totalLengthStr,
                                      long offset) {
        PutMethod putMethod = 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_CHUNK_SIZE_HEADER, chunkSizeStr);
        mPutMethod.addRequestHeader(OC_TOTAL_LENGTH_HEADER, totalLengthStr);
        ((ChunkFromFileChannelRequestEntity) mEntity).setOffset(offset);
        mPutMethod.setRequestEntity(mEntity);
        if (mCancellationRequested.get()) {
            mPutMethod.abort();
            // next method will throw an exception
        }

        if (chunkIndex == chunkCount - 1) {
            // Added a high timeout to the last chunk due to when the last chunk
            // arrives to the server with the last PUT, all chunks get assembled
            // within that PHP request, so last one takes longer.
            mPutMethod.getParams().setSoTimeout(LAST_CHUNK_TIMEOUT);
        }

        return putMethod;
    }

    private String getDateAsString() {
+10 −25
Original line number Diff line number Diff line
@@ -45,7 +45,7 @@ import java.io.IOException;
/**
 * Remote operation moving a remote file or folder in the ownCloud server to a different folder
 * in the same account.
 * <p/>
 *
 * Allows renaming the moving file/folder at the same time.
 *
 * @author David A. Velasco
@@ -65,7 +65,7 @@ public class CopyRemoteFileOperation extends RemoteOperation {

    /**
     * Constructor.
     * <p/>
     *
     * TODO Paths should finish in "/" in the case of folders. ?
     *
     * @param srcRemotePath    Remote path of the file/folder to move.
@@ -129,30 +129,22 @@ public class CopyRemoteFileOperation extends RemoteOperation {
                /// for other errors that could be explicitly handled, check first:
                /// http://www.webdav.org/specs/rfc4918.html#rfc.section.9.9.4

            } else if (status == 400) {
                result = new RemoteOperationResult(copyMethod.succeeded(),
                        copyMethod.getResponseBodyAsString(), status);
            } else {
                result = new RemoteOperationResult(
                        isSuccess(status),    // copy.succeeded()? trustful?
                        status,
                        copyMethod.getResponseHeaders()
                );
                result = new RemoteOperationResult(isSuccess(status), copyMethod);
                client.exhaustResponse(copyMethod.getResponseBodyAsStream());
            }

            Log.i(TAG, "Copy " + mSrcRemotePath + " to " + mTargetRemotePath + ": " +
                    result.getLogMessage());
            Log.i(TAG, "Copy " + mSrcRemotePath + " to " + mTargetRemotePath + ": " + result.getLogMessage());

        } catch (Exception e) {
            result = new RemoteOperationResult(e);
            Log.e(TAG, "Copy " + mSrcRemotePath + " to " + mTargetRemotePath + ": " +
                    result.getLogMessage(), e);
            Log.e(TAG, "Copy " + mSrcRemotePath + " to " + mTargetRemotePath + ": " + result.getLogMessage(), e);

        } finally {
            if (copyMethod != null)
            if (copyMethod != null) {
                copyMethod.releaseConnection();
            }
        }

        return result;
    }
@@ -160,10 +152,10 @@ public class CopyRemoteFileOperation extends RemoteOperation {

    /**
     * Analyzes a multistatus response from the OC server to generate an appropriate result.
     * <p/>
     *
     * In WebDAV, a COPY request on collections (folders) can be PARTIALLY successful: some
     * children are copied, some other aren't.
     * <p/>
     *
     * According to the WebDAV specification, a multistatus response SHOULD NOT include partial
     * successes (201, 204) nor for descendants of already failed children (424) in the response
     * entity. But SHOULD NOT != MUST NOT, so take carefully.
@@ -196,20 +188,13 @@ public class CopyRemoteFileOperation extends RemoteOperation {
        if (failFound) {
            result = new RemoteOperationResult(ResultCode.PARTIAL_COPY_DONE);
        } else {
            result = new RemoteOperationResult(
                    true,
                    HttpStatus.SC_MULTI_STATUS,
                    copyMethod.getResponseHeaders()
            );
            result = new RemoteOperationResult(true, copyMethod);
        }

        return result;

    }


    protected boolean isSuccess(int status) {
        return status == HttpStatus.SC_CREATED || status == HttpStatus.SC_NO_CONTENT;
    }

}
+41 −51
Original line number Diff line number Diff line
@@ -40,7 +40,6 @@ import com.owncloud.android.lib.resources.status.OwnCloudVersion;
 *
 * @author David A. Velasco
 * @author masensio
 *
 */
public class CreateRemoteFolderOperation extends RemoteOperation {

@@ -100,17 +99,9 @@ public class CreateRemoteFolderOperation extends RemoteOperation {
        MkColMethod mkcol = null;
        try {
            mkcol = new MkColMethod(client.getWebdavUri() + WebdavUtils.encodePath(mRemotePath));
    		int status =  client.executeMethod(mkcol, READ_TIMEOUT, CONNECTION_TIMEOUT);
            if ( status == 400 ) {
                result = new RemoteOperationResult(mkcol.succeeded(),
                        mkcol.getResponseBodyAsString(), status);
                Log_OC.d(TAG, mkcol.getResponseBodyAsString());

            } else {
                result = new RemoteOperationResult(mkcol.succeeded(), status,
                        mkcol.getResponseHeaders());
            client.executeMethod(mkcol, READ_TIMEOUT, CONNECTION_TIMEOUT);
            result = new RemoteOperationResult(mkcol.succeeded(), mkcol);
            Log_OC.d(TAG, "Create directory " + mRemotePath + ": " + result.getLogMessage());
            }
            client.exhaustResponse(mkcol.getResponseBodyAsStream());

        } catch (Exception e) {
@@ -131,5 +122,4 @@ public class CreateRemoteFolderOperation extends RemoteOperation {
    }



}
+41 −42
Original line number Diff line number Diff line
@@ -83,8 +83,7 @@ public class DownloadRemoteFileOperation extends RemoteOperation {
        try {
            tmpFile.getParentFile().mkdirs();
            int status = downloadFile(client, tmpFile);
        	result = new RemoteOperationResult(isSuccess(status), status,
                    (mGet != null ? mGet.getResponseHeaders() : null));
            result = new RemoteOperationResult(isSuccess(status), mGet);
            Log_OC.i(TAG, "Download of " + mRemotePath + " to " + getTmpPath() + ": " +
                result.getLogMessage());

Loading