fix wrong assumption how conflict dialog show work

updated tests

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

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

@@ -60,9 +60,10 @@ public class ConflictsResolveActivityIT extends AbstractIT {
 
     @Test
     public void screenshotTextFiles() throws InterruptedException {
-        OCUpload newUpload = new OCUpload(FileStorageUtils.getSavePath(account.name) + "/nonEmpty.txt",
-                                          "/newFile.txt",
-                                          account.name);
+        OCFile newFile = new OCFile("/newFile.txt");
+        newFile.setFileLength(56000);
+        newFile.setModificationTimestamp(1522019340);
+        newFile.setStoragePath(FileStorageUtils.getSavePath(account.name) + "/nonEmpty.txt");
 
         OCFile existingFile = new OCFile("/newFile.txt");
         existingFile.setFileLength(1024000);
@@ -72,13 +73,13 @@ public class ConflictsResolveActivityIT extends AbstractIT {
         storageManager.saveNewFile(existingFile);
 
         Intent intent = new Intent(targetContext, ConflictsResolveActivity.class);
-        intent.putExtra(ConflictsResolveActivity.EXTRA_FILE, existingFile);
-        intent.putExtra(ConflictsResolveActivity.EXTRA_CONFLICT_UPLOAD, newUpload);
+        intent.putExtra(ConflictsResolveActivity.EXTRA_FILE, newFile);
+        intent.putExtra(ConflictsResolveActivity.EXTRA_EXISTING_FILE, existingFile);
 
         ConflictsResolveActivity sut = activityRule.launchActivity(intent);
 
         ConflictsResolveDialog dialog = ConflictsResolveDialog.newInstance(existingFile,
-                                                                           newUpload,
+                                                                           newFile,
                                                                            UserAccountManagerImpl
                                                                                .fromContext(targetContext)
                                                                                .getUser()
@@ -97,8 +98,10 @@ public class ConflictsResolveActivityIT extends AbstractIT {
         FileDataStorageManager storageManager = new FileDataStorageManager(account,
                                                                            targetContext.getContentResolver());
 
-        OCUpload newUpload = new OCUpload(FileStorageUtils.getSavePath(account.name) + "/nonEmpty.txt",
-                                          "/newFile.txt", account.name);
+        OCFile newFile = new OCFile("/newFile.txt");
+        newFile.setFileLength(56000);
+        newFile.setModificationTimestamp(1522019340);
+        newFile.setStoragePath(FileStorageUtils.getSavePath(account.name) + "/nonEmpty.txt");
 
         File image = getFile("image.jpg");
 
@@ -119,8 +122,8 @@ public class ConflictsResolveActivityIT extends AbstractIT {
         OCFile existingFile = storageManager.getFileByPath("/image.jpg");
 
         Intent intent = new Intent(targetContext, ConflictsResolveActivity.class);
-        intent.putExtra(ConflictsResolveActivity.EXTRA_FILE, existingFile);
-        intent.putExtra(ConflictsResolveActivity.EXTRA_CONFLICT_UPLOAD, newUpload);
+        intent.putExtra(ConflictsResolveActivity.EXTRA_FILE, newFile);
+        intent.putExtra(ConflictsResolveActivity.EXTRA_EXISTING_FILE, existingFile);
 
         ConflictsResolveActivity sut = activityRule.launchActivity(intent);
 
@@ -129,7 +132,7 @@ public class ConflictsResolveActivityIT extends AbstractIT {
         };
 
         ConflictsResolveDialog dialog = ConflictsResolveDialog.newInstance(existingFile,
-                                                                           newUpload,
+                                                                           newFile,
                                                                            UserAccountManagerImpl
                                                                                .fromContext(targetContext)
                                                                                .getUser()
@@ -150,15 +153,22 @@ public class ConflictsResolveActivityIT extends AbstractIT {
         OCUpload newUpload = new OCUpload(FileStorageUtils.getSavePath(account.name) + "/nonEmpty.txt",
                                           "/newFile.txt",
                                           account.name);
+
         OCFile existingFile = new OCFile("/newFile.txt");
         existingFile.setFileLength(1024000);
         existingFile.setModificationTimestamp(1582019340);
 
+        OCFile newFile = new OCFile("/newFile.txt");
+        newFile.setFileLength(56000);
+        newFile.setModificationTimestamp(1522019340);
+        newFile.setStoragePath(FileStorageUtils.getSavePath(account.name) + "/nonEmpty.txt");
+
         FileDataStorageManager storageManager = new FileDataStorageManager(account, targetContext.getContentResolver());
         storageManager.saveNewFile(existingFile);
 
         Intent intent = new Intent(targetContext, ConflictsResolveActivity.class);
-        intent.putExtra(ConflictsResolveActivity.EXTRA_FILE, existingFile);
+        intent.putExtra(ConflictsResolveActivity.EXTRA_FILE, newFile);
+        intent.putExtra(ConflictsResolveActivity.EXTRA_EXISTING_FILE, existingFile);
         intent.putExtra(ConflictsResolveActivity.EXTRA_CONFLICT_UPLOAD, newUpload);
 
         ConflictsResolveActivity sut = activityRule.launchActivity(intent);
@@ -183,15 +193,22 @@ public class ConflictsResolveActivityIT extends AbstractIT {
         OCUpload newUpload = new OCUpload(FileStorageUtils.getSavePath(account.name) + "/nonEmpty.txt",
                                           "/newFile.txt",
                                           account.name);
+
         OCFile existingFile = new OCFile("/newFile.txt");
         existingFile.setFileLength(1024000);
         existingFile.setModificationTimestamp(1582019340);
 
+        OCFile newFile = new OCFile("/newFile.txt");
+        newFile.setFileLength(56000);
+        newFile.setModificationTimestamp(1522019340);
+        newFile.setStoragePath(FileStorageUtils.getSavePath(account.name) + "/nonEmpty.txt");
+
         FileDataStorageManager storageManager = new FileDataStorageManager(account, targetContext.getContentResolver());
         storageManager.saveNewFile(existingFile);
 
         Intent intent = new Intent(targetContext, ConflictsResolveActivity.class);
-        intent.putExtra(ConflictsResolveActivity.EXTRA_FILE, existingFile);
+        intent.putExtra(ConflictsResolveActivity.EXTRA_FILE, newFile);
+        intent.putExtra(ConflictsResolveActivity.EXTRA_EXISTING_FILE, existingFile);
         intent.putExtra(ConflictsResolveActivity.EXTRA_CONFLICT_UPLOAD, newUpload);
 
         ConflictsResolveActivity sut = activityRule.launchActivity(intent);
@@ -220,21 +237,28 @@ public class ConflictsResolveActivityIT extends AbstractIT {
         OCUpload newUpload = new OCUpload(FileStorageUtils.getSavePath(account.name) + "/nonEmpty.txt",
                                           "/newFile.txt",
                                           account.name);
+
         OCFile existingFile = new OCFile("/newFile.txt");
         existingFile.setFileLength(1024000);
         existingFile.setModificationTimestamp(1582019340);
 
+        OCFile newFile = new OCFile("/newFile.txt");
+        newFile.setFileLength(56000);
+        newFile.setModificationTimestamp(1522019340);
+        newFile.setStoragePath(FileStorageUtils.getSavePath(account.name) + "/nonEmpty.txt");
+
         FileDataStorageManager storageManager = new FileDataStorageManager(account, targetContext.getContentResolver());
         storageManager.saveNewFile(existingFile);
 
         Intent intent = new Intent(targetContext, ConflictsResolveActivity.class);
-        intent.putExtra(ConflictsResolveActivity.EXTRA_FILE, existingFile);
+        intent.putExtra(ConflictsResolveActivity.EXTRA_FILE, newFile);
+        intent.putExtra(ConflictsResolveActivity.EXTRA_EXISTING_FILE, existingFile);
         intent.putExtra(ConflictsResolveActivity.EXTRA_CONFLICT_UPLOAD, newUpload);
 
         ConflictsResolveActivity sut = activityRule.launchActivity(intent);
 
         sut.listener = decision -> {
-            assertEquals(decision, ConflictsResolveDialog.Decision.KEEP_SERVER);
+            assertEquals(decision, ConflictsResolveDialog.Decision.KEEP_LOCAL);
             returnCode = true;
         };
 
@@ -257,21 +281,28 @@ public class ConflictsResolveActivityIT extends AbstractIT {
         OCUpload newUpload = new OCUpload(FileStorageUtils.getSavePath(account.name) + "/nonEmpty.txt",
                                           "/newFile.txt",
                                           account.name);
+
         OCFile existingFile = new OCFile("/newFile.txt");
         existingFile.setFileLength(1024000);
         existingFile.setModificationTimestamp(1582019340);
 
+        OCFile newFile = new OCFile("/newFile.txt");
+        newFile.setFileLength(56000);
+        newFile.setModificationTimestamp(1522019340);
+        newFile.setStoragePath(FileStorageUtils.getSavePath(account.name) + "/nonEmpty.txt");
+
         FileDataStorageManager storageManager = new FileDataStorageManager(account, targetContext.getContentResolver());
         storageManager.saveNewFile(existingFile);
 
         Intent intent = new Intent(targetContext, ConflictsResolveActivity.class);
-        intent.putExtra(ConflictsResolveActivity.EXTRA_FILE, existingFile);
+        intent.putExtra(ConflictsResolveActivity.EXTRA_FILE, newFile);
+        intent.putExtra(ConflictsResolveActivity.EXTRA_EXISTING_FILE, existingFile);
         intent.putExtra(ConflictsResolveActivity.EXTRA_CONFLICT_UPLOAD, newUpload);
 
         ConflictsResolveActivity sut = activityRule.launchActivity(intent);
 
         sut.listener = decision -> {
-            assertEquals(decision, ConflictsResolveDialog.Decision.KEEP_SERVER);
+            assertEquals(decision, ConflictsResolveDialog.Decision.KEEP_BOTH);
             returnCode = true;
         };
 

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

@@ -296,7 +296,7 @@ public class SynchronizeFileOperation extends SyncOperation {
             mAccount,
             file,
             FileUploader.LOCAL_BEHAVIOUR_MOVE,
-            FileUploader.NameCollisionPolicy.ASK_USER
+            FileUploader.NameCollisionPolicy.OVERWRITE
         );
 
         mTransferWasRequested = true;

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

@@ -1,22 +1,18 @@
 /**
- *  ownCloud Android client application
+ * ownCloud Android client application
  *
- *  @author Bartek Przybylski
- *  @author David A. Velasco
- *  Copyright (C) 2012 Bartek Przybylski
- *  Copyright (C) 2016 ownCloud Inc.
- *
- *  This program is free software: you can redistribute it and/or modify
- *  it under the terms of the GNU General Public License version 2,
- *  as published by the Free Software Foundation.
- *
- *  This program is distributed in the hope that it will be useful,
- *  but WITHOUT ANY WARRANTY; without even the implied warranty of
- *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- *  GNU General Public License for more details.
- *  <p/>
- *  You should have received a copy of the GNU General Public License
- *  along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ * @author Bartek Przybylski
+ * @author David A. Velasco Copyright (C) 2012 Bartek Przybylski Copyright (C) 2016 ownCloud Inc.
+ * <p>
+ * This program is free software: you can redistribute it and/or modify it under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation.
+ * <p>
+ * This program is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY; without even the implied
+ * warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for more
+ * details.
+ * <p/>
+ * You should have received a copy of the GNU General Public License along with this program.  If not, see
+ * <http://www.gnu.org/licenses/>.
  */
 
 package com.owncloud.android.ui.activity;
@@ -27,15 +23,20 @@ import android.widget.Toast;
 
 import com.nextcloud.client.account.User;
 import com.nextcloud.java.util.Optional;
+import com.owncloud.android.R;
 import com.owncloud.android.datamodel.OCFile;
 import com.owncloud.android.datamodel.UploadsStorageManager;
 import com.owncloud.android.db.OCUpload;
 import com.owncloud.android.files.services.FileDownloader;
 import com.owncloud.android.files.services.FileUploader;
+import com.owncloud.android.lib.common.operations.RemoteOperationResult;
 import com.owncloud.android.lib.common.utils.Log_OC;
+import com.owncloud.android.lib.resources.files.ReadFileRemoteOperation;
+import com.owncloud.android.lib.resources.files.model.RemoteFile;
 import com.owncloud.android.ui.dialog.ConflictsResolveDialog;
 import com.owncloud.android.ui.dialog.ConflictsResolveDialog.Decision;
 import com.owncloud.android.ui.dialog.ConflictsResolveDialog.OnConflictDecisionMadeListener;
+import com.owncloud.android.utils.FileStorageUtils;
 
 import javax.inject.Inject;
 
@@ -57,12 +58,15 @@ public class ConflictsResolveActivity extends FileActivity implements OnConflict
      * Specify the upload local behaviour when there is no CONFLICT_UPLOAD.
      */
     public static final String EXTRA_LOCAL_BEHAVIOUR = "LOCAL_BEHAVIOUR";
+    public static final String EXTRA_EXISTING_FILE = "EXISTING_FILE";
 
     private static final String TAG = ConflictsResolveActivity.class.getSimpleName();
 
     @Inject UploadsStorageManager uploadsStorageManager;
 
     private OCUpload conflictUpload;
+    private OCFile existingFile;
+    private OCFile newFile;
     private int localBehaviour = FileUploader.LOCAL_BEHAVIOUR_FORGET;
     protected OnConflictDecisionMadeListener listener;
 
@@ -72,9 +76,11 @@ public class ConflictsResolveActivity extends FileActivity implements OnConflict
 
         if (savedInstanceState != null) {
             conflictUpload = savedInstanceState.getParcelable(EXTRA_CONFLICT_UPLOAD);
+            existingFile = savedInstanceState.getParcelable(EXTRA_EXISTING_FILE);
             localBehaviour = savedInstanceState.getInt(EXTRA_LOCAL_BEHAVIOUR);
         } else {
             conflictUpload = getIntent().getParcelableExtra(EXTRA_CONFLICT_UPLOAD);
+            existingFile = getIntent().getParcelableExtra(EXTRA_EXISTING_FILE);
             localBehaviour = getIntent().getIntExtra(EXTRA_LOCAL_BEHAVIOUR, localBehaviour);
         }
 
@@ -82,10 +88,14 @@ public class ConflictsResolveActivity extends FileActivity implements OnConflict
             localBehaviour = conflictUpload.getLocalAction();
         }
 
+        // new file was modified locally in file system
+        newFile = getFile();
+
         listener = new OnConflictDecisionMadeListener() {
             @Override
             public void conflictDecisionMade(Decision decision) {
-                OCFile file = getFile();
+                OCFile file = newFile; // local file got changed, so either upload it or replace it again by server
+                // version
 
                 switch (decision) {
                     case CANCEL:
@@ -141,6 +151,7 @@ public class ConflictsResolveActivity extends FileActivity implements OnConflict
         super.onSaveInstanceState(outState);
 
         outState.putParcelable(EXTRA_CONFLICT_UPLOAD, conflictUpload);
+        outState.putParcelable(EXTRA_EXISTING_FILE, existingFile);
         outState.putInt(EXTRA_LOCAL_BEHAVIOUR, localBehaviour);
     }
 
@@ -156,17 +167,43 @@ public class ConflictsResolveActivity extends FileActivity implements OnConflict
             finish();
         }
 
-        OCFile file = getFile();
-        if (getFile() == null) {
+        if (newFile == null) {
             Log_OC.e(TAG, "No file received");
             finish();
         }
 
+        if (existingFile == null) {
+            // fetch info of existing file from server
+            ReadFileRemoteOperation operation = new ReadFileRemoteOperation(newFile.getRemotePath());
+
+            new Thread(() -> {
+                try {
+                    RemoteOperationResult result = operation.execute(getAccount(), this);
+
+                    if (result.isSuccess()) {
+                        existingFile = FileStorageUtils.fillOCFile((RemoteFile) result.getData().get(0));
+                        existingFile.setLastSyncDateForProperties(System.currentTimeMillis());
+
+                        startDialog();
+                    } else {
+                        showErrorAndFinish();
+                    }
+                } catch (Exception e) {
+                    showErrorAndFinish();
+                }
+
+
+            }).start();
+        } else {
+            startDialog();
+        }
+    }
+
+    private void startDialog() {
         Optional<User> userOptional = getUser();
 
         if (!userOptional.isPresent()) {
-            Toast.makeText(this, "Error creating conflict dialog!", Toast.LENGTH_LONG).show();
-            finish();
+            showErrorAndFinish();
         }
 
         // Check whether the file is contained in the current Account
@@ -177,17 +214,22 @@ public class ConflictsResolveActivity extends FileActivity implements OnConflict
             fragmentTransaction.remove(prev);
         }
 
-        if (getStorageManager().fileExists(file.getRemotePath())) {
-            ConflictsResolveDialog dialog = ConflictsResolveDialog.newInstance(getFile(),
-                                                                               conflictUpload,
+        if (existingFile != null && getStorageManager().fileExists(newFile.getRemotePath())) {
+            ConflictsResolveDialog dialog = ConflictsResolveDialog.newInstance(existingFile,
+                                                                               newFile,
                                                                                userOptional.get());
             dialog.show(fragmentTransaction, "conflictDialog");
         } else {
             // Account was changed to a different one - just finish
-            finish();
+            showErrorAndFinish();
         }
     }
 
+    private void showErrorAndFinish() {
+        Toast.makeText(this, R.string.conflict_dialog_error, Toast.LENGTH_LONG).show();
+        finish();
+    }
+
     /**
      * @return whether the local version of the files is to be deleted.
      */

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

@@ -463,7 +463,7 @@ public abstract class FileActivity extends DrawerActivity
         if (!result.isSuccess()) {
             if (result.getCode() == ResultCode.SYNC_CONFLICT) {
                 Intent i = new Intent(this, ConflictsResolveActivity.class);
-                i.putExtra(ConflictsResolveActivity.EXTRA_FILE, syncedFile);
+                i.putExtra(ConflictsResolveActivity.EXTRA_FILE, syncedFile); // must be new file
                 i.putExtra(ConflictsResolveActivity.EXTRA_ACCOUNT, getAccount());
                 startActivity(i);
             }

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

@@ -37,7 +37,6 @@ import com.owncloud.android.R;
 import com.owncloud.android.datamodel.FileDataStorageManager;
 import com.owncloud.android.datamodel.OCFile;
 import com.owncloud.android.datamodel.ThumbnailsCacheManager;
-import com.owncloud.android.db.OCUpload;
 import com.owncloud.android.lib.common.utils.Log_OC;
 import com.owncloud.android.ui.adapter.LocalFileListAdapter;
 import com.owncloud.android.ui.adapter.OCFileListAdapter;
@@ -79,12 +78,12 @@ public class ConflictsResolveDialog extends DialogFragment {
         KEEP_SERVER,
     }
 
-    public static ConflictsResolveDialog newInstance(OCFile existingFile, OCUpload conflictUpload, User user) {
+    public static ConflictsResolveDialog newInstance(OCFile existingFile, OCFile newFile, User user) {
         ConflictsResolveDialog dialog = new ConflictsResolveDialog();
 
         Bundle args = new Bundle();
         args.putParcelable(KEY_EXISTING_FILE, existingFile);
-        args.putSerializable(KEY_NEW_FILE, new File(conflictUpload.getLocalPath()));
+        args.putSerializable(KEY_NEW_FILE, new File(newFile.getStoragePath()));
         args.putParcelable(KEY_USER, user);
         dialog.setArguments(args);
 
@@ -152,17 +151,17 @@ public class ConflictsResolveDialog extends DialogFragment {
         View view = inflater.inflate(R.layout.conflict_resolve_dialog, null);
         int accentColor = ThemeUtils.primaryAccentColor(getContext());
 
+        CheckBox newFileCheckbox = view.findViewById(R.id.new_checkbox);
+        CheckBox existingFileCheckbox = view.findViewById(R.id.existing_checkbox);
+
         // Build the dialog
         AlertDialog.Builder builder = new AlertDialog.Builder(getActivity());
         builder.setView(view)
             .setPositiveButton(R.string.common_ok, (dialog, which) -> {
                 if (listener != null) {
-                    CheckBox newFile = view.findViewById(R.id.new_checkbox);
-                    CheckBox existingFile = view.findViewById(R.id.existing_checkbox);
-
-                    if (newFile.isSelected() && existingFile.isSelected()) {
+                    if (newFileCheckbox.isChecked() && existingFileCheckbox.isChecked()) {
                         listener.conflictDecisionMade(Decision.KEEP_BOTH);
-                    } else if (newFile.isSelected()) {
+                    } else if (newFileCheckbox.isChecked()) {
                         listener.conflictDecisionMade(Decision.KEEP_LOCAL);
                     } else {
                         listener.conflictDecisionMade(Decision.KEEP_SERVER);
@@ -207,6 +206,12 @@ public class ConflictsResolveDialog extends DialogFragment {
                                        false,
                                        getContext());
 
+        view.findViewById(R.id.newFileContainer)
+            .setOnClickListener(v -> newFileCheckbox.setChecked(!newFileCheckbox.isChecked()));
+
+        view.findViewById(R.id.existingFileContainer)
+            .setOnClickListener(v -> existingFileCheckbox.setChecked(!existingFileCheckbox.isChecked()));
+
         return builder.create();
     }
 

src/main/res/values/strings.xml

@@ -938,4 +938,5 @@
     <string name="thumbnail_for_new_file_desc">Thumbnail for new file</string>
     <string name="thumbnail_for_existing_file_description">Thumbnail for existing file</string>
     <string name="invalid_url">Invalid URL</string>
+    <string name="conflict_dialog_error">Error creating conflict dialog!</string>
 </resources>