Fix conflicts for offline uploads when uploading same file again

app/src/androidTest/java/com/owncloud/android/datamodel/FileDataStorageManagerContentProviderClientIT.java

@@ -7,6 +7,9 @@
  */
 package com.owncloud.android.datamodel;
 
+import android.util.Pair;
+
+import com.nextcloud.utils.extensions.FileDataStorageManagerExtensionsKt;
 import com.owncloud.android.db.ProviderMeta;
 
 import org.junit.Test;
@@ -44,4 +47,31 @@ public void saveFile() {
 
         assertEquals(file.getRemotePath(), read.getRemotePath());
     }
+
+
+
+    @Test
+    public void testGenerateFileNameForConflictResolution() {
+        Pair<String, String>[] names = new Pair[]{
+            // Files
+            new Pair<>("hello", "hello (1)"),
+            new Pair<>("hello.txt", "hello (1).txt"),
+            new Pair<>("hello (1).txt", "hello (2).txt"),
+            new Pair<>("hello (18y5).txt", "hello (18y5) (1).txt"),
+            new Pair<>("hello (hey)", "hello (hey) (1)"),
+            new Pair<>("hello (hey).txt", "hello (hey) (1).txt"),
+            new Pair<>(".hello", ".hello (1)"),
+            // Folders
+            new Pair<>("hello/", "hello (1)/"),
+            new Pair<>("hello (1)/", "hello (2)/"),
+            new Pair<>("hello.hello/", "hello.hello (1)/"),
+            new Pair<>("hello.hello (y)/", "hello.hello (y) (1)/"),
+            new Pair<>(".hello/", ".hello (1)/"),
+        };
+
+        for (Pair<String, String> name : names) {
+            String gen = FileDataStorageManagerExtensionsKt.generateFileNameForConflictResolution(name.first);
+            assertEquals(gen, name.second);
+        }
+    }
 }

app/src/androidTest/java/com/owncloud/android/datamodel/FileDataStorageManagerIT.java

@@ -8,7 +8,9 @@
 package com.owncloud.android.datamodel;
 
 import android.content.ContentValues;
+import android.util.Pair;
 
+import com.nextcloud.utils.extensions.FileDataStorageManagerExtensionsKt;
 import com.owncloud.android.AbstractOnServerIT;
 import com.owncloud.android.db.ProviderMeta;
 import com.owncloud.android.lib.common.operations.RemoteOperationResult;
@@ -353,4 +355,27 @@ public void testOCCapability() {
         assertEquals(capability.getUserStatus(), newCapability.getUserStatus());
     }
 
+    @Test
+    public void testGenerateFileNameForConflictResolution() {
+        Pair<String, String>[] names = new Pair[]{
+            // Files
+            new Pair<String, String>("hello", "hello (1)"),
+            new Pair<String, String>("hello.txt", "hello (1).txt"),
+            new Pair<String, String>("hello (1).txt", "hello (2).txt"),
+            new Pair<String, String>("hello (18y5).txt", "hello (18y5) (1).txt"),
+            new Pair<String, String>("hello (hey)", "hello (hey) (1)"),
+            new Pair<String, String>("hello (hey).txt", "hello (hey) (1).txt"),
+            // Folders
+            new Pair<String, String>("hello/", "hello (1)/"),
+            new Pair<String, String>("hello (1)/", "hello (2)/"),
+            new Pair<String, String>("hello.hello/", "hello.hello (1)/"),
+            new Pair<String, String>("hello.hello (y)/", "hello.hello (y) (1)/"),
+        };
+
+        for (Pair<String, String> name : names) {
+            String gen = FileDataStorageManagerExtensionsKt.generateFileNameForConflictResolution(name.first);
+            assertEquals(gen, name.second);
+        }
+    }
+
 }

app/src/main/java/com/nextcloud/client/jobs/offlineOperations/repository/OfflineOperationsRepository.kt

@@ -73,34 +73,34 @@ class OfflineOperationsRepository(private val fileDataStorageManager: FileDataSt
                     fileDataStorageManager.getFileById(parentId)?.let { ocFile ->
                         ocFile.decryptedRemotePath?.let { updatedPath ->
                             val newPath = updatedPath + nextOperation.filename + pathSeparator
-
-                            if (newPath != nextOperation.path) {
-                                nextOperation.apply {
-                                    type = when (type) {
-                                        is OfflineOperationType.CreateFile ->
-                                            (type as OfflineOperationType.CreateFile).copy(
-                                                remotePath = newPath
-                                            )
-
-                                        is OfflineOperationType.CreateFolder ->
-                                            (type as OfflineOperationType.CreateFolder).copy(
-                                                path = newPath
-                                            )
-
-                                        else -> type
-                                    }
-                                    path = newPath
-                                }
-                            } else {
-                                null
-                            }
+                            updateOperationPath(newPath, nextOperation)
                         }
                     }
                 }
             }
             .forEach { dao.update(it) }
     }
 
+    private fun updateOperationPath(newPath: String, nextOperation: OfflineOperationEntity): OfflineOperationEntity? {
+        if (newPath == nextOperation.path) return null
+
+        val updatedType = when (val currentType = nextOperation.type) {
+            is OfflineOperationType.CreateFile -> currentType.copy(remotePath = newPath)
+            is OfflineOperationType.CreateFolder -> currentType.copy(path = newPath)
+            else -> currentType
+        }
+
+        return nextOperation.apply {
+            type = updatedType
+            path = newPath
+        }
+    }
+
+    override fun updateOperationForKeepBoth(operation: OfflineOperationEntity, newPath: String) {
+        updateOperationPath(newPath, operation)
+        dao.update(operation)
+    }
+
     override fun convertToOCFiles(fileId: Long): List<OCFile> =
         dao.getSubEntitiesByParentOCFileId(fileId).map { entity ->
             OCFile(entity.path).apply {

app/src/main/java/com/nextcloud/client/jobs/offlineOperations/repository/OfflineOperationsRepositoryType.kt

@@ -14,5 +14,6 @@ interface OfflineOperationsRepositoryType {
     fun getAllSubEntities(fileId: Long): List<OfflineOperationEntity>
     fun deleteOperation(file: OCFile)
     fun updateNextOperations(operation: OfflineOperationEntity)
+    fun updateOperationForKeepBoth(operation: OfflineOperationEntity, newPath: String)
     fun convertToOCFiles(fileId: Long): List<OCFile>
 }

app/src/main/java/com/nextcloud/utils/extensions/FileDataStorageManagerExtensions.kt

@@ -12,10 +12,13 @@ import com.nextcloud.client.database.entity.model.ShareeKey
 import com.nextcloud.client.database.entity.toOCCapability
 import com.owncloud.android.datamodel.FileDataStorageManager
 import com.owncloud.android.datamodel.OCFile
+import com.owncloud.android.lib.common.OwnCloudClient
 import com.owncloud.android.lib.common.utils.Log_OC
+import com.owncloud.android.lib.resources.files.ExistenceCheckRemoteOperation
 import com.owncloud.android.lib.resources.files.model.RemoteFile
 import com.owncloud.android.lib.resources.shares.OCShare
 import com.owncloud.android.lib.resources.status.OCCapability
+import com.owncloud.android.operations.upload.RemoteFileExistence
 import com.owncloud.android.utils.FileStorageUtils
 import com.owncloud.android.utils.MimeTypeUtil
 import kotlinx.coroutines.Dispatchers
@@ -185,6 +188,69 @@ fun FileDataStorageManager.moveFiles(ocFile: OCFile?, targetPath: String, target
     }
 }
 
+/**
+ * Finds a suitable file name to resolve a conflict.
+ * Tries to concatenate a number to the name until it finds a non-existent one.
+ * E.g. for "file.txt" it will propose "file (2).txt". If that exists, then "file (3).txt" and so on.
+ * E.g. for "folder" it will propose "folder (2)/". If that exists, then "folder (3)/" and so on.
+ *
+ * @return the new remote path, or null if the user is unauthorized in the provided path
+ */
+@Suppress("ReturnCount")
+fun getRemotePathForConflictResolution(client: OwnCloudClient, remotePath: String, fileName: String): String? {
+    val newName = generateFileNameForConflictResolution(fileName)
+    val newPath = "$remotePath$newName"
+
+    // Check if new name exists
+    val operation = ExistenceCheckRemoteOperation(newPath, false)
+    val existence = RemoteFileExistence.fromExistenceCheck(operation.execute(client))
+    if (existence == RemoteFileExistence.UNAUTHORIZED) {
+        return null
+    }
+    if (existence == RemoteFileExistence.DOES_NOT_EXIST) {
+        return newPath
+    }
+    return getRemotePathForConflictResolution(client, remotePath, newName)
+}
+
+fun generateFileNameForConflictResolution(fileName: String): String {
+    val isFolder = fileName.endsWith(OCFile.PATH_SEPARATOR)
+    val separator =
+        if (isFolder) {
+            OCFile.PATH_SEPARATOR
+        } else if (fileName.indexOf(".") > 0) {
+            "."
+        } else {
+            ""
+        }
+    var nameFirstPart =
+        if (separator.isNotEmpty()) {
+            fileName.substringBeforeLast(separator)
+        } else {
+            fileName
+        }
+    var nameLastPart =
+        if (separator.isNotEmpty()) {
+            fileName.substringAfterLast(separator, "")
+        } else {
+            "" // Extension or path separator
+        }
+    if (nameLastPart.isNotEmpty() || isFolder) nameLastPart = "$separator$nameLastPart"
+    val regex = Regex("""(.*)\((\d+)\)$""", RegexOption.MULTILINE)
+    nameFirstPart = if (regex.matches(nameFirstPart)) {
+        // Already a resolved conflict (i.e. "file (1).txt"). Update the number.
+        regex.replace(nameFirstPart, transform = { m ->
+            val baseName = m.groups[1]?.value
+            val number = m.groups[2]?.value?.toInt() ?: 0
+            "$baseName(${number + 1})"
+        })
+    } else {
+        // Add the number
+        "$nameFirstPart (1)"
+    }
+    return "$nameFirstPart$nameLastPart"
+}
+
 @Suppress("ReturnCount")
 private fun moveLocalFiles(accountName: String, ocFile: OCFile, defaultSavePath: String, targetPath: String): Boolean {
     val localFile = File(FileStorageUtils.getDefaultSavePathFor(accountName, ocFile))

app/src/main/java/com/nextcloud/utils/extensions/FileExtensions.kt

@@ -8,16 +8,22 @@
 package com.nextcloud.utils.extensions
 
 import android.graphics.Bitmap
+import android.util.Log
 import androidx.exifinterface.media.ExifInterface
+import com.owncloud.android.datamodel.FileDataStorageManager
 import com.owncloud.android.datamodel.OCFile
 import com.owncloud.android.datamodel.ThumbnailsCacheManager
 import com.owncloud.android.lib.common.utils.Log_OC
 import com.owncloud.android.lib.resources.files.model.ServerFileInterface
 import com.owncloud.android.utils.DisplayUtils
 import java.io.File
+import java.io.IOException
+import java.nio.file.Files
 import java.nio.file.Path
+import java.nio.file.attribute.BasicFileAttributes
 
 private const val TAG = "FileExtensions"
+private const val MS_IN_SECOND = 1000
 
 fun OCFile?.logFileSize(tag: String) {
     val size = DisplayUtils.bytesToHumanReadable(this?.fileLength ?: -1)
@@ -111,3 +117,25 @@ fun String.getBitmapSize(): Pair<Int, Int>? = try {
 } catch (_: Exception) {
     null
 }
+
+fun OCFile?.isTheSameAs(localFile: File?): Boolean = try {
+    this ?: return false
+    localFile ?: return false
+
+    val attr = Files.readAttributes(localFile.toPath(), BasicFileAttributes::class.java)
+    val localName = localFile.getName()
+    val remoteName = this.fileName
+    val localSize = localFile.length()
+    val remoteSize = this.fileLength
+    val localCreated = attr.creationTime().toMillis() / MS_IN_SECOND // Unix time in milliseconds
+    val localModified = attr.lastModifiedTime().toMillis() / MS_IN_SECOND // Unix time in milliseconds
+    val remoteCreated = this.creationTimestamp // Unix time in seconds!
+    val remoteModified = this.modificationTimestamp / MS_IN_SECOND // Unix time in milliseconds
+    remoteName == localName &&
+        remoteSize == localSize &&
+        remoteCreated == localCreated &&
+        remoteModified == localModified
+} catch (e: IOException) {
+    Log.e(FileDataStorageManager.TAG, "fileIsTheSame: unable to obtain local file attributes for comparing: $e")
+    false
+}

app/src/main/java/com/owncloud/android/datamodel/FileDataStorageManager.java

@@ -54,6 +54,7 @@
 import com.owncloud.android.MainApp;
 import com.owncloud.android.datamodel.e2e.v2.decrypted.DecryptedFolderMetadataFile;
 import com.owncloud.android.db.ProviderMeta.ProviderTableMeta;
+import com.owncloud.android.lib.common.OwnCloudClient;
 import com.owncloud.android.lib.common.network.WebdavEntry;
 import com.owncloud.android.lib.common.utils.Log_OC;
 import com.owncloud.android.lib.resources.files.ReadFileRemoteOperation;
@@ -70,6 +71,7 @@
 import com.owncloud.android.lib.resources.status.OCCapability;
 import com.owncloud.android.lib.resources.tags.Tag;
 import com.owncloud.android.operations.RemoteOperationFailedException;
+import com.owncloud.android.operations.UploadFileOperation;
 import com.owncloud.android.utils.FileStorageUtils;
 import com.owncloud.android.utils.MimeType;
 import com.owncloud.android.utils.MimeTypeUtil;
@@ -200,7 +202,7 @@ public void addCreateFileOfflineOperation(String[] localPaths, String[] remotePa
             }
 
             offlineOperationDao.insert(entity);
-            createPendingFile(remotePath, mimeType, createdAt, modificationTimestamp);
+            createPendingFile(remotePath, mimeType, createdAt, modificationTimestamp, localPath);
         }
     }
 
@@ -230,8 +232,22 @@ public OfflineOperationEntity addCreateFolderOfflineOperation(String path, Strin
         return entity;
     }
 
-    public void createPendingFile(String path, String mimeType, long createdAt, long modificationTimestamp) {
-        OCFile file = new OCFile(path);
+    public void createPendingFile(
+        String remotePath,
+        String mimeType,
+        long createdAt,
+        long modificationTimestamp,
+        String localPath
+     ) {
+        final OCFile existingFile = getFileByRemotePath(remotePath);
+        final File localFile = FileExtensionsKt.toFile(localPath);
+        if (FileExtensionsKt.isTheSameAs(existingFile, localFile)) {
+            Log_OC.i(TAG, "Creating pendingFile for an already uploaded file: " +
+                "keeping metadata to avoid triggering a conflict");
+            return;
+        }
+
+        OCFile file = new OCFile(remotePath);
         file.setMimeType(mimeType);
         file.setCreationTimestamp(createdAt);
         file.setModificationTimestamp(modificationTimestamp);
@@ -340,26 +356,22 @@ public void renameOfflineOperation(OCFile file, String newFolderName) {
         moveLocalFile(file, newPath, parentFolder.getDecryptedRemotePath());
     }
 
-    @SuppressLint("SimpleDateFormat")
-    public void keepOfflineOperationAndServerFile(OfflineOperationEntity entity, OCFile file) {
+    public void keepOfflineOperationAndServerFile(OfflineOperationEntity entity, OCFile file, OwnCloudClient client) {
         if (file == null) return;
 
         String oldFileName = entity.getFilename();
         if (oldFileName == null) return;
 
-        Long parentOCFileId = entity.getParentOCFileId();
-        if (parentOCFileId == null) return;
-
-        OCFile parentFolder = getFileById(parentOCFileId);
-        if (parentFolder == null) return;
-
-        DateFormatPattern formatPattern = DateFormatPattern.FullDateWithHours;
-        String currentDateTime = DateExtensionsKt.currentDateRepresentation(new Date(), formatPattern);
+        String parentRemotePath = file.getParentRemotePath();
+        if (parentRemotePath == null || parentRemotePath.isEmpty())
+            return;
 
-        String newFolderName = oldFileName + " - " + currentDateTime;
-        String newPath = parentFolder.getDecryptedRemotePath() + newFolderName + OCFile.PATH_SEPARATOR;
-        moveLocalFile(file, newPath, parentFolder.getDecryptedRemotePath());
-        offlineOperationsRepository.updateNextOperations(entity);
+        final String newPath = FileDataStorageManagerExtensionsKt.getRemotePathForConflictResolution(
+            client,
+            parentRemotePath,
+            oldFileName
+        );
+        offlineOperationsRepository.updateOperationForKeepBoth(entity, newPath);
     }
 
     @Nullable
@@ -541,7 +553,7 @@ public List<OCFile> getFolderImagesAndVideos(OCFile folder, boolean onlyOnDevice
     }
 
     public boolean saveFile(OCFile ocFile) {
-        Log_OC.d(TAG, "saving file: " + ocFile.getRemotePath());
+        Log_OC.d(TAG, "saving file " + ocFile.getFileName() + " into " + ocFile.getRemotePath());
 
         boolean overridden = false;
         final ContentValues cv = createContentValuesForFile(ocFile);

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

@@ -1797,6 +1797,7 @@ private void updateOCFile(OCFile file, RemoteFile remoteFile) {
         file.setModificationTimestamp(remoteFile.getModifiedTimestamp());
         file.setModificationTimestampAtLastSyncForData(remoteFile.getModifiedTimestamp());
         file.setEtag(remoteFile.getEtag());
+        file.setEtagOnServer(remoteFile.getEtag());
         file.setRemoteId(remoteFile.getRemoteId());
         file.setPermissions(remoteFile.getPermissions());
         file.setUploadTimestamp(remoteFile.getUploadTimestamp());