add missing upload error messages

better handle temp file

Signed-off-by: tobiasKaminsky <tobias@kaminsky.me>

build.gradle

@@ -210,9 +210,9 @@ dependencies {
     // dependencies for app building
     implementation 'androidx.multidex:multidex:2.0.1'
 //    implementation project('nextcloud-android-library')
-    genericImplementation 'com.github.nextcloud:android-library:master-SNAPSHOT'
-    gplayImplementation 'com.github.nextcloud:android-library:master-SNAPSHOT'
-    versionDevImplementation 'com.github.nextcloud:android-library:master-SNAPSHOT'
+    genericImplementation 'com.github.nextcloud:android-library:newResultCode-SNAPSHOT'
+    gplayImplementation 'com.github.nextcloud:android-library:newResultCode-SNAPSHOT'
+    versionDevImplementation 'com.github.nextcloud:android-library:newResultCode-SNAPSHOT'
     implementation 'androidx.constraintlayout:constraintlayout:1.1.3'
     implementation 'androidx.legacy:legacy-support-v4:1.0.0'
     implementation 'com.google.android.material:material:1.0.0'

src/main/java/com/owncloud/android/db/UploadResult.java

@@ -1,4 +1,4 @@
-/**
+/*
  * ownCloud Android client application
  *
  * @author masensio
@@ -29,7 +29,7 @@ public enum UploadResult {
     FOLDER_ERROR(3),
     CONFLICT_ERROR(4),
     FILE_ERROR(5),
-    PRIVILEDGES_ERROR(6),
+    PRIVILEGES_ERROR(6),
     CANCELLED(7),
     FILE_NOT_FOUND(8),
     DELAYED_FOR_WIFI(9),
@@ -39,7 +39,12 @@ public enum UploadResult {
     LOCK_FAILED(13),
     DELAYED_IN_POWER_SAVE_MODE(14),
     SSL_RECOVERABLE_PEER_UNVERIFIED(15),
-    VIRUS_DETECTED(16);
+    VIRUS_DETECTED(16),
+    LOCAL_STORAGE_FULL(17),
+    OLD_ANDROID_API(18),
+    SYNC_CONFLICT(19),
+    CANNOT_CREATE_FILE(20),
+    LOCAL_STORAGE_NOT_COPIED(21);
 
     private final int value;
 
@@ -68,7 +73,7 @@ public enum UploadResult {
             case 5:
                 return FILE_ERROR;
             case 6:
-                return PRIVILEDGES_ERROR;
+                return PRIVILEGES_ERROR;
             case 7:
                 return CANCELLED;
             case 8:
@@ -89,8 +94,18 @@ public enum UploadResult {
                 return SSL_RECOVERABLE_PEER_UNVERIFIED;
             case 16:
                 return VIRUS_DETECTED;
+            case 17:
+                return LOCAL_STORAGE_FULL;
+            case 18:
+                return OLD_ANDROID_API;
+            case 19:
+                return SYNC_CONFLICT;
+            case 20:
+                return CANNOT_CREATE_FILE;
+            case 21:
+                return LOCAL_STORAGE_NOT_COPIED;
         }
-        return null;
+        return UNKNOWN;
     }
 
     public static UploadResult fromOperationResult(RemoteOperationResult result) {
@@ -115,9 +130,15 @@ public enum UploadResult {
             case CONFLICT:
                 return CONFLICT_ERROR;
             case LOCAL_STORAGE_NOT_COPIED:
-                return FILE_ERROR;
+                return LOCAL_STORAGE_NOT_COPIED;
+            case LOCAL_STORAGE_FULL:
+                return LOCAL_STORAGE_FULL;
+            case OLD_ANDROID_API:
+                return OLD_ANDROID_API;
+            case SYNC_CONFLICT:
+                return SYNC_CONFLICT;
             case FORBIDDEN:
-                return PRIVILEDGES_ERROR;
+                return PRIVILEGES_ERROR;
             case CANCELLED:
                 return CANCELLED;
             case DELAYED_FOR_WIFI:
@@ -139,6 +160,8 @@ public enum UploadResult {
                 return LOCK_FAILED;
             case VIRUS_DETECTED:
                 return VIRUS_DETECTED;
+            case CANNOT_CREATE_FILE:
+                return CANNOT_CREATE_FILE;
             default:
                 return UNKNOWN;
         }

src/main/java/com/owncloud/android/operations/UploadFileOperation.java

@@ -83,7 +83,6 @@ import java.nio.channels.FileLock;
 import java.nio.channels.OverlappingFileLockException;
 import java.util.HashMap;
 import java.util.HashSet;
-import java.util.Iterator;
 import java.util.Set;
 import java.util.UUID;
 import java.util.concurrent.atomic.AtomicBoolean;
@@ -502,7 +501,7 @@ public class UploadFileOperation extends SyncOperation {
             expectedFile = new File(expectedPath);
 
             result = copyFile(originalFile, expectedPath);
-            if (result != null) {
+            if (!result.isSuccess()) {
                 return result;
             }
 
@@ -551,7 +550,7 @@ public class UploadFileOperation extends SyncOperation {
                 Files.deleteIfExists(Paths.get(temporalPath));
                 result = copy(originalFile, temporalFile);
 
-                if (result == null) {
+                if (result.isSuccess()) {
                     if (temporalFile.length() == originalFile.length()) {
                         channel = new RandomAccessFile(temporalFile.getAbsolutePath(), "rw").getChannel();
                         fileLock = channel.tryLock();
@@ -586,9 +585,8 @@ public class UploadFileOperation extends SyncOperation {
                         mFile.getEtagInConflict(), timeStamp);
             }
 
-            Iterator<OnDatatransferProgressListener> listener = mDataTransferListeners.iterator();
-            while (listener.hasNext()) {
-                mUploadOperation.addDatatransferProgressListener(listener.next());
+            for (OnDatatransferProgressListener mDataTransferListener : mDataTransferListeners) {
+                mUploadOperation.addDatatransferProgressListener(mDataTransferListener);
             }
 
             if (mCancellationRequested.get()) {
@@ -755,7 +753,7 @@ public class UploadFileOperation extends SyncOperation {
         File originalFile = new File(mOriginalStoragePath);
         File expectedFile = null;
         FileLock fileLock = null;
-        long size = 0;
+        long size;
 
         try {
             // check conditions
@@ -772,7 +770,7 @@ public class UploadFileOperation extends SyncOperation {
             expectedFile = new File(expectedPath);
 
             result = copyFile(originalFile, expectedPath);
-            if (result != null) {
+            if (!result.isSuccess()) {
                 return result;
             }
 
@@ -795,7 +793,7 @@ public class UploadFileOperation extends SyncOperation {
                 Files.deleteIfExists(Paths.get(temporalPath));
                 result = copy(originalFile, temporalFile);
 
-                if (result == null) {
+                if (result.isSuccess()) {
                     if (temporalFile.length() == originalFile.length()) {
                         channel = new RandomAccessFile(temporalFile.getAbsolutePath(), "rw").getChannel();
                         fileLock = channel.tryLock();
@@ -807,7 +805,7 @@ public class UploadFileOperation extends SyncOperation {
 
             try {
                 size = channel.size();
-            } catch (IOException e1) {
+            } catch (Exception e1) {
                 size = new File(mFile.getStoragePath()).length();
             }
 
@@ -828,16 +826,15 @@ public class UploadFileOperation extends SyncOperation {
                         mFile.getRemotePath(), mFile.getMimeType(), mFile.getEtagInConflict(), timeStamp);
             }
 
-            Iterator<OnDatatransferProgressListener> listener = mDataTransferListeners.iterator();
-            while (listener.hasNext()) {
-                mUploadOperation.addDatatransferProgressListener(listener.next());
+            for (OnDatatransferProgressListener mDataTransferListener : mDataTransferListeners) {
+                mUploadOperation.addDatatransferProgressListener(mDataTransferListener);
             }
 
             if (mCancellationRequested.get()) {
                 throw new OperationCancelledException();
             }
 
-            if (result == null || result.isSuccess() && mUploadOperation != null) {
+            if (result.isSuccess() && mUploadOperation != null) {
                 result = mUploadOperation.execute(client, mFile.isEncrypted());
 
                 /// move local temporal file or original file to its corresponding
@@ -908,22 +905,20 @@ public class UploadFileOperation extends SyncOperation {
 
     private RemoteOperationResult copyFile(File originalFile, String expectedPath) throws OperationCancelledException,
             IOException {
-        RemoteOperationResult result = null;
-
         if (mLocalBehaviour == FileUploader.LOCAL_BEHAVIOUR_COPY && !mOriginalStoragePath.equals(expectedPath)) {
             String temporalPath = FileStorageUtils.getInternalTemporalPath(mAccount.name, mContext) +
                 mFile.getRemotePath();
             mFile.setStoragePath(temporalPath);
             File temporalFile = new File(temporalPath);
 
-            result = copy(originalFile, temporalFile);
+            return copy(originalFile, temporalFile);
         }
 
         if (mCancellationRequested.get()) {
             throw new OperationCancelledException();
         }
 
-        return result;
+        return new RemoteOperationResult(ResultCode.OK);
     }
 
     private void checkNameCollision(OwnCloudClient client, DecryptedFolderMetadata metadata, boolean encrypted)
@@ -1017,7 +1012,7 @@ public class UploadFileOperation extends SyncOperation {
             if (parentDir != null) {
                 result = new RemoteOperationResult(ResultCode.OK);
             } else {
-                result = new RemoteOperationResult(ResultCode.UNKNOWN_ERROR);
+                result = new RemoteOperationResult(ResultCode.CANNOT_CREATE_FILE);
             }
         }
         return result;
@@ -1168,25 +1163,19 @@ public class UploadFileOperation extends SyncOperation {
     private RemoteOperationResult copy(File sourceFile, File targetFile) throws IOException {
         Log_OC.d(TAG, "Copying local file");
 
-        RemoteOperationResult result = null;
-
         if (FileStorageUtils.getUsableSpace() < sourceFile.length()) {
-            result = new RemoteOperationResult(ResultCode.LOCAL_STORAGE_FULL);
-            return result;  // error condition when the file should be copied
-
+            return new RemoteOperationResult(ResultCode.LOCAL_STORAGE_FULL); // error when the file should be copied
         } else {
             Log_OC.d(TAG, "Creating temporal folder");
             File temporalParent = targetFile.getParentFile();
-            temporalParent.mkdirs();
-            if (!temporalParent.isDirectory()) {
-                throw new IOException(
-                        "Unexpected error: parent directory could not be created");
+
+            if (!temporalParent.mkdirs() && !temporalParent.isDirectory()) {
+                return new RemoteOperationResult(ResultCode.CANNOT_CREATE_FILE);
             }
+
             Log_OC.d(TAG, "Creating temporal file");
-            targetFile.createNewFile();
-            if (!targetFile.isFile()) {
-                throw new IOException(
-                        "Unexpected error: target file could not be created");
+            if (!targetFile.createNewFile() && !targetFile.isFile()) {
+                return new RemoteOperationResult(ResultCode.CANNOT_CREATE_FILE);
             }
 
             Log_OC.d(TAG, "Copying file contents");
@@ -1214,14 +1203,10 @@ public class UploadFileOperation extends SyncOperation {
                 } // else: weird but possible situation, nothing to copy
 
                 if (mCancellationRequested.get()) {
-                    result = new RemoteOperationResult(new OperationCancelledException());
-                    return result;
+                    return new RemoteOperationResult(new OperationCancelledException());
                 }
-
             } catch (Exception e) {
-                result = new RemoteOperationResult(ResultCode.LOCAL_STORAGE_NOT_COPIED);
-                return result;
-
+                return new RemoteOperationResult(ResultCode.LOCAL_STORAGE_NOT_COPIED);
             } finally {
                 try {
                     if (in != null) {
@@ -1241,19 +1226,17 @@ public class UploadFileOperation extends SyncOperation {
                 }
             }
         }
-        return result;
+        return new RemoteOperationResult(ResultCode.OK);
     }
 
 
     /**
      * TODO rewrite with homogeneous fail handling, remove dependency on {@link RemoteOperationResult},
      * TODO     use Exceptions instead
-     * <p>
      * TODO refactor both this and 'copy' in a single method
      *
      * @param sourceFile Source file to move.
      * @param targetFile Target location to move the file.
-     * @return {@link RemoteOperationResult} result from remote operation
      * @throws IOException exception if file cannot be read/wrote
      */
     private void move(File sourceFile, File targetFile) throws IOException {

src/main/java/com/owncloud/android/ui/adapter/UploadListAdapter.java

@@ -58,7 +58,6 @@ import java.util.Arrays;
 import java.util.Comparator;
 
 import androidx.annotation.NonNull;
-import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 
 /**
  * This Adapter populates a ListView with following types of uploads: pending,active, completed. Filtering possible.
@@ -436,7 +435,7 @@ public class UploadListAdapter extends SectionedRecyclerViewAdapter<SectionedVie
             case FILE_ERROR:
                 status = mParentActivity.getString(R.string.uploads_view_upload_status_failed_file_error);
                 break;
-            case PRIVILEDGES_ERROR:
+            case PRIVILEGES_ERROR:
                 status = mParentActivity.getString(R.string.uploads_view_upload_status_failed_permission_error);
                 break;
             case NETWORK_CONNECTION:
@@ -466,14 +465,16 @@ public class UploadListAdapter extends SectionedRecyclerViewAdapter<SectionedVie
                 status = mParentActivity.getString(R.string.maintenance_mode);
                 break;
             case SSL_RECOVERABLE_PEER_UNVERIFIED:
-                status =
-                        mParentActivity.getString(
+                status = mParentActivity.getString(
                                 R.string.uploads_view_upload_status_failed_ssl_certificate_not_trusted
                         );
                 break;
             case UNKNOWN:
                 status = mParentActivity.getString(R.string.uploads_view_upload_status_unknown_fail);
                 break;
+            case LOCK_FAILED:
+                status = mParentActivity.getString(R.string.upload_lock_failed);
+                break;
             case DELAYED_IN_POWER_SAVE_MODE:
                 status = mParentActivity.getString(
                         R.string.uploads_view_upload_status_waiting_exit_power_save_mode);
@@ -481,8 +482,23 @@ public class UploadListAdapter extends SectionedRecyclerViewAdapter<SectionedVie
             case VIRUS_DETECTED:
                 status = mParentActivity.getString(R.string.uploads_view_upload_status_virus_detected);
                 break;
+            case LOCAL_STORAGE_FULL:
+                status = mParentActivity.getString(R.string.upload_local_storage_full);
+                break;
+            case OLD_ANDROID_API:
+                status = mParentActivity.getString(R.string.upload_old_android);
+                break;
+            case SYNC_CONFLICT:
+                status = mParentActivity.getString(R.string.upload_sync_conflict);
+                break;
+            case CANNOT_CREATE_FILE:
+                status = mParentActivity.getString(R.string.upload_cannot_create_file);
+                break;
+            case LOCAL_STORAGE_NOT_COPIED:
+                status = mParentActivity.getString(R.string.upload_local_storage_not_copied);
+                break;
             default:
-                status = "New fail result but no description for the user";
+                status = mParentActivity.getString(R.string.upload_unknown_error);
                 break;
         }
 
@@ -656,14 +672,12 @@ public class UploadListAdapter extends SectionedRecyclerViewAdapter<SectionedVie
                 }
             }
 
-            @SuppressFBWarnings("Bx")
             private int compareUploadId(OCUpload upload1, OCUpload upload2) {
-                return Long.valueOf(upload1.getFixedUploadId()).compareTo(upload2.getFixedUploadId());
+                return Long.compare(upload1.getFixedUploadId(), upload2.getFixedUploadId());
             }
 
-            @SuppressFBWarnings("Bx")
             private int compareUpdateTime(OCUpload upload1, OCUpload upload2) {
-                return Long.valueOf(upload2.getFixedUploadEndTimeStamp()).compareTo(upload1.getFixedUploadEndTimeStamp());
+                return Long.compare(upload2.getFixedUploadEndTimeStamp(), upload1.getFixedUploadEndTimeStamp());
             }
         };
     }

src/main/res/values/strings.xml

@@ -850,4 +850,11 @@
     <string name="uploader_upload_files_behaviour_not_writable">source folder is read-only; file will only be uploaded</string>
     <string name="auto_upload_file_behaviour_kept_in_folder">kept in original folder, as it is readonly</string>
     <string name="scanQR_description">Login via QR code</string>
+    <string name="upload_unknown_error">Unknown error</string>
+    <string name="upload_lock_failed">Locking folder failed</string>
+    <string name="upload_local_storage_full">Local storage full</string>
+    <string name="upload_old_android"><![CDATA[Encryption is only possible with >= Android 5.0]]></string>
+    <string name="upload_sync_conflict">Sync conflict, please resolve manually</string>
+    <string name="upload_cannot_create_file">Cannot create local file</string>
+    <string name="upload_local_storage_not_copied">File could not be copied to local storage</string>
 </resources>