Upload are deleted after decision was made in conflict activity

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

src/androidTest/java/com/owncloud/android/datamodel/UploadStorageManagerTest.java

@@ -34,6 +34,8 @@ import androidx.test.filters.SmallTest;
 
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertTrue;
 
 /**
@@ -150,6 +152,25 @@ public class UploadStorageManagerTest extends AbstractIT {
         uploadsStorageManager.getAllStoredUploads();
     }
 
+    @Test
+    public void getById() {
+        OCUpload upload = createUpload(account);
+        long id = uploadsStorageManager.storeUpload(upload);
+
+        OCUpload newUpload = uploadsStorageManager.getUploadById(id);
+
+        assertNotNull(newUpload);
+        assertEquals(upload.getLocalAction(), newUpload.getLocalAction());
+        assertEquals(upload.getFolderUnlockToken(), newUpload.getFolderUnlockToken());
+    }
+
+    @Test
+    public void getByIdNull() {
+        OCUpload newUpload = uploadsStorageManager.getUploadById(-1);
+
+        assertNull(newUpload);
+    }
+
     private void insertUploads(Account account, int rowsToInsert) {
         for (int i = 0; i < rowsToInsert; i++) {
             uploadsStorageManager.storeUpload(createUpload(account));

src/androidTest/java/com/owncloud/android/ui/activity/ConflictsResolveActivityIT.java

@@ -169,7 +169,7 @@ public class ConflictsResolveActivityIT extends AbstractIT {
         Intent intent = new Intent(targetContext, ConflictsResolveActivity.class);
         intent.putExtra(ConflictsResolveActivity.EXTRA_FILE, newFile);
         intent.putExtra(ConflictsResolveActivity.EXTRA_EXISTING_FILE, existingFile);
-        intent.putExtra(ConflictsResolveActivity.EXTRA_CONFLICT_UPLOAD, newUpload);
+        intent.putExtra(ConflictsResolveActivity.EXTRA_CONFLICT_UPLOAD_ID, newUpload.getUploadId());
 
         ConflictsResolveActivity sut = activityRule.launchActivity(intent);
 
@@ -210,7 +210,7 @@ public class ConflictsResolveActivityIT extends AbstractIT {
         Intent intent = new Intent(targetContext, ConflictsResolveActivity.class);
         intent.putExtra(ConflictsResolveActivity.EXTRA_FILE, newFile);
         intent.putExtra(ConflictsResolveActivity.EXTRA_EXISTING_FILE, existingFile);
-        intent.putExtra(ConflictsResolveActivity.EXTRA_CONFLICT_UPLOAD, newUpload);
+        intent.putExtra(ConflictsResolveActivity.EXTRA_CONFLICT_UPLOAD_ID, newUpload.getUploadId());
 
         ConflictsResolveActivity sut = activityRule.launchActivity(intent);
 
@@ -256,7 +256,7 @@ public class ConflictsResolveActivityIT extends AbstractIT {
         Intent intent = new Intent(targetContext, ConflictsResolveActivity.class);
         intent.putExtra(ConflictsResolveActivity.EXTRA_FILE, newFile);
         intent.putExtra(ConflictsResolveActivity.EXTRA_EXISTING_FILE, existingFile);
-        intent.putExtra(ConflictsResolveActivity.EXTRA_CONFLICT_UPLOAD, newUpload);
+        intent.putExtra(ConflictsResolveActivity.EXTRA_CONFLICT_UPLOAD_ID, newUpload.getUploadId());
 
         ConflictsResolveActivity sut = activityRule.launchActivity(intent);
 
@@ -301,7 +301,7 @@ public class ConflictsResolveActivityIT extends AbstractIT {
         Intent intent = new Intent(targetContext, ConflictsResolveActivity.class);
         intent.putExtra(ConflictsResolveActivity.EXTRA_FILE, newFile);
         intent.putExtra(ConflictsResolveActivity.EXTRA_EXISTING_FILE, existingFile);
-        intent.putExtra(ConflictsResolveActivity.EXTRA_CONFLICT_UPLOAD, newUpload);
+        intent.putExtra(ConflictsResolveActivity.EXTRA_CONFLICT_UPLOAD_ID, newUpload.getUploadId());
 
         ConflictsResolveActivity sut = activityRule.launchActivity(intent);
 

src/main/java/com/owncloud/android/datamodel/UploadsStorageManager.java

@@ -231,13 +231,27 @@ public class UploadsStorageManager extends Observable {
      * @param upload Upload instance to remove from persisted storage.
      * @return true when the upload was stored and could be removed.
      */
-    public int removeUpload(OCUpload upload) {
+    public int removeUpload(@Nullable OCUpload upload) {
+        if (upload == null) {
+            return 0;
+        } else {
+            return removeUpload(upload.getUploadId());
+        }
+    }
+
+    /**
+     * Remove an upload from the uploads list, known its target account and remote path.
+     *
+     * @param id to remove from persisted storage.
+     * @return true when the upload was stored and could be removed.
+     */
+    public int removeUpload(long id) {
         int result = getDB().delete(
-                ProviderTableMeta.CONTENT_URI_UPLOADS,
-                ProviderTableMeta._ID + "=?",
-                new String[]{Long.toString(upload.getUploadId())}
-        );
-        Log_OC.d(TAG, "delete returns " + result + " for upload " + upload);
+            ProviderTableMeta.CONTENT_URI_UPLOADS,
+            ProviderTableMeta._ID + "=?",
+            new String[]{Long.toString(id)}
+                                   );
+        Log_OC.d(TAG, "delete returns " + result + " for upload with id " + id);
         if (result > 0) {
             notifyObserversNow();
         }
@@ -287,6 +301,25 @@ public class UploadsStorageManager extends Observable {
         return getUploads(null, (String[]) null);
     }
 
+    public @Nullable
+    OCUpload getUploadById(long id) {
+        OCUpload result = null;
+        Cursor cursor = getDB().query(
+            ProviderTableMeta.CONTENT_URI_UPLOADS,
+            null,
+            ProviderTableMeta._ID + "=?",
+            new String[]{Long.toString(id)},
+            "_id ASC");
+
+        if (cursor != null) {
+            if (cursor.moveToFirst()) {
+                result = createOCUploadFromCursor(cursor);
+            }
+        }
+        Log_OC.d(TAG, "Retrieve job " + result + " for id " + id);
+        return result;
+    }
+
     private OCUpload[] getUploads(@Nullable String selection, @Nullable String... selectionArgs) {
         ArrayList<OCUpload> uploads = new ArrayList<>();
         final long pageSize = 100;

src/main/java/com/owncloud/android/files/services/FileDownloader.java

@@ -46,7 +46,6 @@ import com.owncloud.android.authentication.AuthenticatorActivity;
 import com.owncloud.android.datamodel.FileDataStorageManager;
 import com.owncloud.android.datamodel.OCFile;
 import com.owncloud.android.datamodel.UploadsStorageManager;
-import com.owncloud.android.db.OCUpload;
 import com.owncloud.android.lib.common.OwnCloudAccount;
 import com.owncloud.android.lib.common.OwnCloudClient;
 import com.owncloud.android.lib.common.OwnCloudClientManagerFactory;
@@ -57,6 +56,7 @@ import com.owncloud.android.lib.common.utils.Log_OC;
 import com.owncloud.android.lib.resources.files.FileUtils;
 import com.owncloud.android.operations.DownloadFileOperation;
 import com.owncloud.android.providers.DocumentsStorageProvider;
+import com.owncloud.android.ui.activity.ConflictsResolveActivity;
 import com.owncloud.android.ui.activity.FileActivity;
 import com.owncloud.android.ui.activity.FileDisplayActivity;
 import com.owncloud.android.ui.dialog.SendShareDialog;
@@ -92,7 +92,6 @@ public class FileDownloader extends Service
     public static final String EXTRA_DOWNLOAD_RESULT = "RESULT";
     public static final String EXTRA_REMOTE_PATH = "REMOTE_PATH";
     public static final String EXTRA_LINKED_TO_PATH = "LINKED_TO";
-    public static final String EXTRA_CONFLICT_UPLOAD = "CONFLICT_UPLOAD";
     public static final String ACCOUNT_NAME = "ACCOUNT_NAME";
 
     private static final int FOREGROUND_SERVICE_ID = 412;
@@ -116,7 +115,7 @@ public class FileDownloader extends Service
 
     private Notification mNotification;
 
-    private OCUpload conflictUpload;
+    private long conflictUploadId;
 
     @Inject UserAccountManager accountManager;
     @Inject UploadsStorageManager uploadsStorageManager;
@@ -205,7 +204,7 @@ public class FileDownloader extends Service
             final String behaviour = intent.getStringExtra(OCFileListFragment.DOWNLOAD_BEHAVIOUR);
             String activityName = intent.getStringExtra(SendShareDialog.ACTIVITY_NAME);
             String packageName = intent.getStringExtra(SendShareDialog.PACKAGE_NAME);
-            this.conflictUpload = intent.getParcelableExtra(FileDownloader.EXTRA_CONFLICT_UPLOAD);
+            conflictUploadId = intent.getLongExtra(ConflictsResolveActivity.EXTRA_CONFLICT_UPLOAD_ID, -1);
             AbstractList<String> requestedDownloads = new Vector<String>();
             try {
                 DownloadFileOperation newDownload = new DownloadFileOperation(user.toPlatformAccount(),
@@ -644,13 +643,13 @@ public class FileDownloader extends Service
 
                 // Remove success notification
                 if (downloadResult.isSuccess()) {
-                    if (this.conflictUpload != null) {
-                        uploadsStorageManager.removeUpload(this.conflictUpload);
+                    if (conflictUploadId > 0) {
+                        uploadsStorageManager.removeUpload(conflictUploadId);
                     }
 
                     // Sleep 2 seconds, so show the notification before remove it
                     NotificationUtils.cancelWithDelay(mNotificationManager,
-                            R.string.downloader_download_succeeded_ticker, 2000);
+                                                      R.string.downloader_download_succeeded_ticker, 2000);
                 }
             }
         }

src/main/java/com/owncloud/android/files/services/FileUploader.java

@@ -808,6 +808,7 @@ public class FileUploader extends Service
                 if (uploadResult.getCode().equals(ResultCode.SYNC_CONFLICT)) {
                     intent = ConflictsResolveActivity.createIntent(upload.getFile(),
                                                                    upload.getAccount(),
+                                                                   upload.getOCUploadId(),
                                                                    Intent.FLAG_ACTIVITY_CLEAR_TOP,
                                                                    this);
                 } else {

src/main/java/com/owncloud/android/ui/activity/ConflictsResolveActivity.java

@@ -1,4 +1,4 @@
-/**
+/*
  * ownCloud Android client application
  *
  * @author Bartek Przybylski
@@ -54,7 +54,7 @@ public class ConflictsResolveActivity extends FileActivity implements OnConflict
     /**
      * A nullable upload entry that must be removed when and if the conflict is resolved.
      */
-    public static final String EXTRA_CONFLICT_UPLOAD = "CONFLICT_UPLOAD";
+    public static final String EXTRA_CONFLICT_UPLOAD_ID = "CONFLICT_UPLOAD_ID";
     /**
      * Specify the upload local behaviour when there is no CONFLICT_UPLOAD.
      */
@@ -65,19 +65,24 @@ public class ConflictsResolveActivity extends FileActivity implements OnConflict
 
     @Inject UploadsStorageManager uploadsStorageManager;
 
-    private OCUpload conflictUpload;
+    private long conflictUploadId;
     private OCFile existingFile;
     private OCFile newFile;
     private int localBehaviour = FileUploader.LOCAL_BEHAVIOUR_FORGET;
     protected OnConflictDecisionMadeListener listener;
 
-    public static Intent createIntent(OCFile file, Account account, Integer flag, Context context) {
+    public static Intent createIntent(OCFile file,
+                                      Account account,
+                                      long conflictUploadId,
+                                      Integer flag,
+                                      Context context) {
         Intent intent = new Intent(context, ConflictsResolveActivity.class);
         if (flag != null) {
             intent.setFlags(intent.getFlags() | flag);
         }
-        intent.putExtra(ConflictsResolveActivity.EXTRA_FILE, file);
-        intent.putExtra(ConflictsResolveActivity.EXTRA_ACCOUNT, account);
+        intent.putExtra(EXTRA_FILE, file);
+        intent.putExtra(EXTRA_ACCOUNT, account);
+        intent.putExtra(EXTRA_CONFLICT_UPLOAD_ID, conflictUploadId);
 
         return intent;
     }
@@ -87,74 +92,67 @@ public class ConflictsResolveActivity extends FileActivity implements OnConflict
         super.onCreate(savedInstanceState);
 
         if (savedInstanceState != null) {
-            conflictUpload = savedInstanceState.getParcelable(EXTRA_CONFLICT_UPLOAD);
+            conflictUploadId = savedInstanceState.getLong(EXTRA_CONFLICT_UPLOAD_ID);
             existingFile = savedInstanceState.getParcelable(EXTRA_EXISTING_FILE);
             localBehaviour = savedInstanceState.getInt(EXTRA_LOCAL_BEHAVIOUR);
         } else {
-            conflictUpload = getIntent().getParcelableExtra(EXTRA_CONFLICT_UPLOAD);
+            conflictUploadId = getIntent().getLongExtra(EXTRA_CONFLICT_UPLOAD_ID, -1);
             existingFile = getIntent().getParcelableExtra(EXTRA_EXISTING_FILE);
             localBehaviour = getIntent().getIntExtra(EXTRA_LOCAL_BEHAVIOUR, localBehaviour);
         }
 
-        if (conflictUpload != null) {
-            localBehaviour = conflictUpload.getLocalAction();
+        OCUpload upload = uploadsStorageManager.getUploadById(conflictUploadId);
+
+        if (upload != null) {
+            localBehaviour = upload.getLocalAction();
         }
 
         // new file was modified locally in file system
         newFile = getFile();
 
-        listener = new OnConflictDecisionMadeListener() {
-            @Override
-            public void conflictDecisionMade(Decision decision) {
-                OCFile file = newFile; // local file got changed, so either upload it or replace it again by server
-                // version
-
-                switch (decision) {
-                    case CANCEL:
-                        // nothing to do
-                        break;
-                    case KEEP_LOCAL: // Upload
-                        FileUploader.uploadUpdateFile(
-                            getBaseContext(),
-                            getAccount(),
-                            file,
-                            localBehaviour,
-                            FileUploader.NameCollisionPolicy.OVERWRITE
-                                                     );
-
-                        if (conflictUpload != null) {
-                            uploadsStorageManager.removeUpload(conflictUpload);
-                        }
-                        break;
-                    case KEEP_BOTH: // Upload
-                        FileUploader.uploadUpdateFile(
-                            getBaseContext(),
-                            getAccount(),
-                            file,
-                            localBehaviour,
-                            FileUploader.NameCollisionPolicy.RENAME
-                                                     );
-
-                        if (conflictUpload != null) {
-                            uploadsStorageManager.removeUpload(conflictUpload);
-                        }
-                        break;
-                    case KEEP_SERVER: // Download
-                        if (!shouldDeleteLocal()) {
-                            // Overwrite local file
-                            Intent intent = new Intent(getBaseContext(), FileDownloader.class);
-                            intent.putExtra(FileDownloader.EXTRA_USER, getUser().orElseThrow(RuntimeException::new));
-                            intent.putExtra(FileDownloader.EXTRA_FILE, file);
-                            if (conflictUpload != null) {
-                                intent.putExtra(FileDownloader.EXTRA_CONFLICT_UPLOAD, conflictUpload);
-                            }
-                            startService(intent);
-                        }
-                        break;
-                }
-
-                finish();
+        listener = decision -> {
+            OCFile file = newFile; // local file got changed, so either upload it or replace it again by server
+            // version
+
+            switch (decision) {
+                case CANCEL:
+                    // nothing to do
+                    break;
+                case KEEP_LOCAL: // Upload
+                    FileUploader.uploadUpdateFile(
+                        getBaseContext(),
+                        getAccount(),
+                        file,
+                        localBehaviour,
+                        FileUploader.NameCollisionPolicy.OVERWRITE
+                                                 );
+
+                    uploadsStorageManager.removeUpload(upload);
+                    break;
+                case KEEP_BOTH: // Upload
+                    FileUploader.uploadUpdateFile(
+                        getBaseContext(),
+                        getAccount(),
+                        file,
+                        localBehaviour,
+                        FileUploader.NameCollisionPolicy.RENAME
+                                                 );
+
+                    uploadsStorageManager.removeUpload(upload);
+                    break;
+                case KEEP_SERVER: // Download
+                    if (!shouldDeleteLocal()) {
+                        // Overwrite local file
+                        Intent intent = new Intent(getBaseContext(), FileDownloader.class);
+                        intent.putExtra(FileDownloader.EXTRA_USER, getUser().orElseThrow(RuntimeException::new));
+                        intent.putExtra(FileDownloader.EXTRA_FILE, file);
+                        intent.putExtra(EXTRA_CONFLICT_UPLOAD_ID, conflictUploadId);
+                        startService(intent);
+                    }
+                    break;
             }
+
+            finish();
         };
     }
 
@@ -162,7 +160,7 @@ public class ConflictsResolveActivity extends FileActivity implements OnConflict
     protected void onSaveInstanceState(@NonNull Bundle outState) {
         super.onSaveInstanceState(outState);
 
-        outState.putParcelable(EXTRA_CONFLICT_UPLOAD, conflictUpload);
+        outState.putLong(EXTRA_CONFLICT_UPLOAD_ID, conflictUploadId);
         outState.putParcelable(EXTRA_EXISTING_FILE, existingFile);
         outState.putInt(EXTRA_LOCAL_BEHAVIOUR, localBehaviour);
     }

src/main/java/com/owncloud/android/ui/activity/FileActivity.java

@@ -499,7 +499,7 @@ public abstract class FileActivity extends DrawerActivity
         OCFile syncedFile = operation.getLocalFile();
         if (!result.isSuccess()) {
             if (result.getCode() == ResultCode.SYNC_CONFLICT) {
-                Intent intent = ConflictsResolveActivity.createIntent(syncedFile, getAccount(), null, this);
+                Intent intent = ConflictsResolveActivity.createIntent(syncedFile, getAccount(), -1, null, this);
                 startActivity(intent);
             }
 

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

@@ -573,6 +573,7 @@ public class UploadListAdapter extends SectionedRecyclerViewAdapter<SectionedVie
         Context context = MainApp.getAppContext();
         Intent intent = ConflictsResolveActivity.createIntent(file,
                                                               upload.getAccount(accountManager),
+                                                              upload.getUploadId(),
                                                               Intent.FLAG_ACTIVITY_NEW_TASK,
                                                               context);
 

src/main/java/com/owncloud/android/ui/dialog/ConflictsResolveDialog.java

@@ -25,6 +25,8 @@ import android.app.Dialog;
 import android.content.Context;
 import android.content.DialogInterface;
 import android.os.Bundle;
+import android.view.View;
+import android.widget.Button;
 import android.widget.Toast;
 
 import com.nextcloud.client.account.User;
@@ -47,6 +49,7 @@ import androidx.annotation.NonNull;
 import androidx.annotation.Nullable;
 import androidx.appcompat.app.AlertDialog;
 import androidx.appcompat.app.AppCompatActivity;
+import androidx.core.content.res.ResourcesCompat;
 import androidx.fragment.app.DialogFragment;
 import androidx.fragment.app.Fragment;
 import androidx.fragment.app.FragmentTransaction;
@@ -64,6 +67,7 @@ public class ConflictsResolveDialog extends DialogFragment {
     public OnConflictDecisionMadeListener listener;
     private User user;
     private List<ThumbnailsCacheManager.ThumbnailGenerationTask> asyncTasks = new ArrayList<>();
+    private Button positiveButton;
 
     private static final String KEY_NEW_FILE = "file";
     private static final String KEY_EXISTING_FILE = "ocfile";
@@ -111,7 +115,9 @@ public class ConflictsResolveDialog extends DialogFragment {
         }
 
         int color = ThemeUtils.primaryAccentColor(getContext());
-        alertDialog.getButton(AlertDialog.BUTTON_POSITIVE).setTextColor(color);
+        positiveButton = alertDialog.getButton(AlertDialog.BUTTON_POSITIVE);
+        setPositiveButtonStatus(false);
+
         alertDialog.getButton(AlertDialog.BUTTON_NEGATIVE).setTextColor(color);
     }
 
@@ -159,9 +165,10 @@ public class ConflictsResolveDialog extends DialogFragment {
                         listener.conflictDecisionMade(Decision.KEEP_BOTH);
                     } else if (binding.newCheckbox.isChecked()) {
                         listener.conflictDecisionMade(Decision.KEEP_LOCAL);
-                    } else {
+                    } else if (binding.existingCheckbox.isChecked()) {
                         listener.conflictDecisionMade(Decision.KEEP_SERVER);
-                    }
+                    }  // else do nothing
+
                 }
             })
             .setNegativeButton(R.string.common_cancel, (dialog, which) -> {
@@ -192,12 +199,36 @@ public class ConflictsResolveDialog extends DialogFragment {
                                        false,
                                        getContext());
 
-        binding.newFileContainer.setOnClickListener(v -> binding.newCheckbox.setChecked(!binding.newCheckbox.isChecked()));
-        binding.existingFileContainer.setOnClickListener(v -> binding.existingCheckbox.setChecked(!binding.existingCheckbox.isChecked()));
+        View.OnClickListener checkBoxClickListener = v -> {
+            setPositiveButtonStatus(binding.newCheckbox.isChecked() || binding.existingCheckbox.isChecked());
+        };
+
+        binding.newCheckbox.setOnClickListener(checkBoxClickListener);
+        binding.existingCheckbox.setOnClickListener(checkBoxClickListener);
+
+        binding.newFileContainer.setOnClickListener(v -> {
+            binding.newCheckbox.setChecked(!binding.newCheckbox.isChecked());
+            setPositiveButtonStatus(binding.newCheckbox.isChecked() || binding.existingCheckbox.isChecked());
+        });
+        binding.existingFileContainer.setOnClickListener(v -> {
+            binding.existingCheckbox.setChecked(!binding.existingCheckbox.isChecked());
+            setPositiveButtonStatus(binding.newCheckbox.isChecked() || binding.existingCheckbox.isChecked());
+        });
 
         return builder.create();
     }
 
+    private void setPositiveButtonStatus(boolean enabled) {
+        if (enabled) {
+            positiveButton.setTextColor(ThemeUtils.primaryAccentColor(requireContext()));
+        } else {
+            positiveButton.setTextColor(ResourcesCompat.getColor(requireContext().getResources(),
+                                                                 R.color.grey_200,
+                                                                 null));
+        }
+        positiveButton.setEnabled(enabled);
+    }
+
     public void showDialog(AppCompatActivity activity) {
         Fragment prev = activity.getSupportFragmentManager().findFragmentByTag("dialog");
         FragmentTransaction ft = activity.getSupportFragmentManager().beginTransaction();

src/main/java/com/owncloud/android/ui/helpers/FileOperationsHelper.java

@@ -255,6 +255,7 @@ public class FileOperationsHelper {
             // this can be very intrusive; a notification should be preferred
             Intent intent = ConflictsResolveActivity.createIntent(file,
                                                                   user.toPlatformAccount(),
+                                                                  -1,
                                                                   Intent.FLAG_ACTIVITY_NEW_TASK,
                                                                   fileActivity);
 
@@ -324,6 +325,7 @@ public class FileOperationsHelper {
                         // this can be very intrusive; a notification should be preferred
                         Intent intent = ConflictsResolveActivity.createIntent(file,
                                                                               user.toPlatformAccount(),
+                                                                              -1,
                                                                               Intent.FLAG_ACTIVITY_NEW_TASK,
                                                                               fileActivity);
                         fileActivity.startActivity(intent);