[stable-35.0.x] Fix conflicts for offline uploads when uploading same file again

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

@@ -7,10 +7,15 @@
  */
 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;
 
+import java.util.Arrays;
+
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotNull;
 
@@ -44,4 +49,30 @@ public void saveFile() {
 
         assertEquals(file.getRemotePath(), read.getRemotePath());
     }
+
+    @Test
+    public void testGenerateFileNameForConflictResolution() {
+        final var names = Arrays.asList(
+            // 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 (final var name: names) {
+            String gen = FileDataStorageManagerExtensionsKt.generateFileNameForConflictResolution(name.first);
+            assertEquals(gen, name.second);
+        }
+    }
 }

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

@@ -50,10 +50,11 @@ public void before() {
         // make sure everything is removed
         sut.deleteAllFiles();
         sut.deleteVirtuals(VirtualFolderType.GALLERY);
-
         assertEquals(0, sut.getAllFiles().size());
-
-        capability = new GetCapabilitiesRemoteOperation(null).execute(client).getResultData();
+        final var result = new GetCapabilitiesRemoteOperation(null)
+            .execute(client);
+        assertTrue(result.isSuccess());
+        capability = result.getResultData();
     }
 
     @After
@@ -352,5 +353,4 @@ public void testOCCapability() {
 
         assertEquals(capability.getUserStatus(), newCapability.getUserStatus());
     }
-
 }

app/src/androidTest/java/com/owncloud/android/files/services/FileUploaderIT.kt

@@ -10,9 +10,12 @@
 package com.owncloud.android.files.services
 
 import com.nextcloud.client.account.UserAccountManagerImpl
-import com.nextcloud.utils.PowerManagementFactory
 import com.nextcloud.client.jobs.upload.FileUploadWorker
+import com.nextcloud.utils.PowerManagementFactory
+import com.nextcloud.utils.extensions.getRemotePathForConflictResolution
+import com.nextcloud.utils.extensions.isTheSameAs
 import com.owncloud.android.AbstractOnServerIT
+import com.owncloud.android.datamodel.OCFile
 import com.owncloud.android.datamodel.UploadsStorageManager
 import com.owncloud.android.db.OCUpload
 import com.owncloud.android.lib.resources.files.ReadFileRemoteOperation
@@ -106,19 +109,71 @@ class FileUploaderIT : AbstractOnServerIT() {
         assertRemoteFileLength(REMOTE_PATH, originalFile.length())
     }
 
+    @Test
+    fun uploadAndCheckFileIsTheSame() {
+        val filename = NON_EMPTY_FILE
+        val remotePath = OCFile.PATH_SEPARATOR + filename
+
+        // Upload file
+        val originalFile = getDummyFile(filename)
+        val operation = uploadOperation(
+            originalFile,
+            NameCollisionPolicy.DEFAULT,
+            remotePath
+        ).setRemoteFolderToBeCreated()
+        assertTrue(operation.execute(client).isSuccess)
+
+        // Check file metadata against remote one
+        val ocFile = storageManager.getFileByRemotePath(remotePath)
+        assertTrue(ocFile.isTheSameAs(originalFile))
+    }
+
+    @Test
+    fun uploadAndCheckFileIsNotTheSame() {
+        val filename = NON_EMPTY_FILE
+
+        // Upload file
+        val originalFile = getDummyFile(filename)
+        val operation = uploadOperation(
+            originalFile,
+            NameCollisionPolicy.DEFAULT
+        ).setRemoteFolderToBeCreated()
+        assertTrue(operation.execute(client).isSuccess)
+
+        // Check file metadata against remote one
+        val ocFile = storageManager.getFileByRemotePath(REMOTE_PATH)
+        assertFalse(ocFile.isTheSameAs(originalFile))
+    }
+
+    @Test
+    fun uploadAndCheckConflictingName() {
+        val originalFile = getDummyFile(EMPTY_FILE)
+        uploadOriginalFile(originalFile)
+        val newPath = getRemotePathForConflictResolution(
+            client,
+            OCFile.PATH_SEPARATOR,
+            REMOTE_FILE
+        )
+        assertEquals(newPath, "${OCFile.PATH_SEPARATOR}$REMOTE_FILE_NAME (1).$REMOTE_FILE_EXTENSION")
+    }
+
     private fun uploadOriginalFile(originalFile: File) {
         val operation = uploadOperation(originalFile, NameCollisionPolicy.DEFAULT).setRemoteFolderToBeCreated()
         assertTrue(operation.execute(client).isSuccess)
         assertRemoteFileLength(REMOTE_PATH, originalFile.length())
     }
 
-    private fun uploadOperation(localFile: File, nameCollisionPolicy: NameCollisionPolicy) = UploadFileOperation(
+    private fun uploadOperation(
+        localFile: File,
+        nameCollisionPolicy: NameCollisionPolicy,
+        remotePath: String = REMOTE_PATH
+    ) = UploadFileOperation(
         uploadsStorageManager,
         connectivityServiceMock,
         powerManagementServiceMock,
         user,
         null,
-        OCUpload(localFile.absolutePath, REMOTE_PATH, account.name),
+        OCUpload(localFile.absolutePath, remotePath, account.name),
         nameCollisionPolicy,
         FileUploadWorker.LOCAL_BEHAVIOUR_COPY,
         targetContext,
@@ -134,7 +189,10 @@ class FileUploaderIT : AbstractOnServerIT() {
     }
 
     companion object {
-        private const val REMOTE_PATH = "/testFile.txt"
+        private const val REMOTE_FILE_NAME = "testFile"
+        private const val REMOTE_FILE_EXTENSION = "txt"
+        private const val REMOTE_FILE = "$REMOTE_FILE_NAME.$REMOTE_FILE_EXTENSION"
+        private const val REMOTE_PATH = "${OCFile.PATH_SEPARATOR}$REMOTE_FILE"
         private const val REMOTE_PATH_RENAMED = "/testFile (2).txt"
         private const val CHUNKED_FILE = "chunkedFile.txt"
         private const val NON_EMPTY_FILE = "nonEmpty.txt"

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

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

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

@@ -10,19 +10,24 @@ package com.nextcloud.utils.extensions
 import com.nextcloud.client.database.dao.FileDao
 import com.nextcloud.client.database.entity.model.ShareeKey
 import com.nextcloud.client.database.entity.toOCCapability
+import com.nextcloud.model.HTTPStatusCodes
 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
 import kotlinx.coroutines.withContext
 import java.io.File
 
 private const val SHARE_PATH_QUERY_CHUNK_SIZE = 400
+private val conflictNameCounterSelectionRegex = Regex("""(.*)\((\d+)\)$""", RegexOption.MULTILINE)
 
 /**
  * Detects sharee additions/removals (by userId + shareType) for [remoteFiles], compared to what is stored
@@ -185,6 +190,73 @@ 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
+ */
+private const val MAX_CONFLICT_RESOLUTION_ATTEMPTS = 10
+
+@Suppress("ReturnCount")
+fun getRemotePathForConflictResolution(client: OwnCloudClient, remotePath: String, fileName: String): String? {
+    var name = fileName
+    repeat(MAX_CONFLICT_RESOLUTION_ATTEMPTS) {
+        name = generateFileNameForConflictResolution(name)
+        val newPath = "$remotePath$name"
+
+        val result = ExistenceCheckRemoteOperation(newPath, false).execute(client)
+        // 404 (File not found) case is a valid one in this case
+        if (!result.isSuccess && result.httpCode != HTTPStatusCodes.NOT_FOUND.code) return null
+
+        when (RemoteFileExistence.fromExistenceCheck(result)) {
+            RemoteFileExistence.DOES_NOT_EXIST -> return newPath
+            RemoteFileExistence.UNAUTHORIZED -> return null
+            else -> Unit // exists, try next name
+        }
+    }
+    return null
+}
+
+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"
+    nameFirstPart = if (conflictNameCounterSelectionRegex.matches(nameFirstPart)) {
+        // Already a resolved conflict (i.e. "file (1).txt"). Update the number.
+        conflictNameCounterSelectionRegex.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

@@ -5,19 +5,26 @@
  * SPDX-License-Identifier: AGPL-3.0-or-later
  */
 
+@file:Suppress("TooManyFunctions")
+
 package com.nextcloud.utils.extensions
 
 import android.graphics.Bitmap
+import android.util.Log
 import androidx.exifinterface.media.ExifInterface
 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 +118,22 @@ 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 localModified = attr.lastModifiedTime().toMillis() / MS_IN_SECOND // Unix time in milliseconds
+    val remoteModified = this.modificationTimestamp / MS_IN_SECOND // Unix time in milliseconds
+    remoteName == localName &&
+        remoteSize == localSize &&
+        remoteModified == localModified
+} catch (e: IOException) {
+    Log.e(TAG, "fileIsTheSame: unable to obtain local file attributes for comparing: $e")
+    false
+}