Merge pull request #17309 from nextcloud/fix/upload-marked-as-skip-instead-completed

fix(upload-list): distinguish between skip and complete

Author: tobiasKaminsky

app/src/main/java/com/nextcloud/client/jobs/autoUpload/AutoUploadWorker.kt

@@ -411,10 +411,9 @@ class AutoUploadWorker(
         }
 
         // only valid for skip collision policy other scenarios will be handled in UploadFileOperation.java
-        if (upload.lastResult == UploadResult.UPLOADED &&
-            syncedFolder.nameCollisionPolicy == NameCollisionPolicy.SKIP
-        ) {
-            Log_OC.d(TAG, "no need to create and process this entity file is already uploaded")
+        val alreadyHandled = upload.lastResult == UploadResult.UPLOADED || upload.lastResult == UploadResult.SKIPPED
+        if (alreadyHandled && syncedFolder.nameCollisionPolicy == NameCollisionPolicy.SKIP) {
+            Log_OC.d(TAG, "no need to create and process this entity file is already uploaded or skipped")
             return AutoUploadEntityResult.Uploaded
         }
 

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

@@ -8,17 +8,13 @@
 package com.nextcloud.utils.extensions
 
 import android.content.Context
-import com.nextcloud.client.preferences.AppPreferences
 import com.owncloud.android.R
 import com.owncloud.android.datamodel.UploadsStorageManager
 import com.owncloud.android.db.OCUpload
 import com.owncloud.android.db.UploadResult
-import com.owncloud.android.files.services.NameCollisionPolicy
 
 fun List<OCUpload>.getUploadIds(): LongArray = map { it.uploadId }.toLongArray()
 
-fun Array<OCUpload>.getUploadIds(): LongArray = map { it.uploadId }.toLongArray()
-
 fun List<OCUpload>.sortedByUploadOrder(): List<OCUpload> = sortedWith(
     compareBy<OCUpload> { it.fixedUploadStatus }
         .thenByDescending { it.isFixedUploadingNow }
@@ -41,14 +37,22 @@ fun OCUpload.getStatusText(activity: Context, isGlobalUploadPaused: Boolean, isU
         }
 
         UploadsStorageManager.UploadStatus.UPLOAD_SUCCEEDED -> {
-            status = if (lastResult == UploadResult.SAME_FILE_CONFLICT) {
-                res.getString(R.string.uploads_view_upload_status_succeeded_same_file)
-            } else if (lastResult == UploadResult.FILE_NOT_FOUND) {
-                lastResult.getFailedStatusText(activity)
-            } else if (nameCollisionPolicy == NameCollisionPolicy.SKIP) {
-                res.getString(R.string.uploads_view_upload_status_skip_reason)
-            } else {
-                res.getString(R.string.uploads_view_upload_status_succeeded)
+            status = when (lastResult) {
+                UploadResult.SAME_FILE_CONFLICT -> {
+                    res.getString(R.string.uploads_view_upload_status_succeeded_same_file)
+                }
+
+                UploadResult.FILE_NOT_FOUND -> {
+                    lastResult.getFailedStatusText(activity)
+                }
+
+                UploadResult.SKIPPED -> {
+                    res.getString(R.string.uploads_view_upload_status_skip_reason)
+                }
+
+                else -> {
+                    res.getString(R.string.uploads_view_upload_status_succeeded)
+                }
             }
         }
 

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

@@ -421,10 +421,10 @@ class UploadsStorageManager(
         val deleted = contentResolver.delete(
             ProviderTableMeta.CONTENT_URI_UPLOADS,
             ProviderTableMeta.UPLOADS_STATUS + EQUAL + UploadStatus.UPLOAD_FAILED.value +
-                AND + ProviderTableMeta.UPLOADS_LAST_RESULT + ANGLE_BRACKETS + UploadResult.LOCK_FAILED.value +
-                AND + ProviderTableMeta.UPLOADS_LAST_RESULT + ANGLE_BRACKETS + UploadResult.DELAYED_FOR_WIFI.value +
-                AND + ProviderTableMeta.UPLOADS_LAST_RESULT + ANGLE_BRACKETS + UploadResult.DELAYED_FOR_CHARGING.value +
-                AND + ProviderTableMeta.UPLOADS_LAST_RESULT + ANGLE_BRACKETS +
+                AND + ProviderTableMeta.UPLOADS_LAST_RESULT + NOT_EQUAL + UploadResult.LOCK_FAILED.value +
+                AND + ProviderTableMeta.UPLOADS_LAST_RESULT + NOT_EQUAL + UploadResult.DELAYED_FOR_WIFI.value +
+                AND + ProviderTableMeta.UPLOADS_LAST_RESULT + NOT_EQUAL + UploadResult.DELAYED_FOR_CHARGING.value +
+                AND + ProviderTableMeta.UPLOADS_LAST_RESULT + NOT_EQUAL +
                 UploadResult.DELAYED_IN_POWER_SAVE_MODE.value +
                 AND + ProviderTableMeta.UPLOADS_ACCOUNT_NAME + IS_EQUAL,
             arrayOf(user.accountName)
@@ -452,6 +452,7 @@ class UploadsStorageManager(
         val deleted = contentResolver.delete(
             ProviderTableMeta.CONTENT_URI_UPLOADS,
             ProviderTableMeta.UPLOADS_STATUS + EQUAL + UploadStatus.UPLOAD_SUCCEEDED.value +
+                AND + ProviderTableMeta.UPLOADS_LAST_RESULT + NOT_EQUAL + UploadResult.SKIPPED.value +
                 AND + ProviderTableMeta.UPLOADS_ACCOUNT_NAME + IS_EQUAL,
             arrayOf(user.accountName)
         )
@@ -464,7 +465,7 @@ class UploadsStorageManager(
         val deleted = contentResolver.delete(
             ProviderTableMeta.CONTENT_URI_UPLOADS,
             ProviderTableMeta.UPLOADS_STATUS + EQUAL + UploadStatus.UPLOAD_SUCCEEDED.value +
-                AND + ProviderTableMeta.UPLOADS_NAME_COLLISION_POLICY + EQUAL + NameCollisionPolicy.SKIP.serialize() +
+                AND + ProviderTableMeta.UPLOADS_LAST_RESULT + EQUAL + UploadResult.SKIPPED.value +
                 AND + ProviderTableMeta.UPLOADS_ACCOUNT_NAME + IS_EQUAL,
             arrayOf(user.accountName)
         )
@@ -493,7 +494,7 @@ class UploadsStorageManager(
 
         if (uploadResult.isSuccess) {
             status = UploadStatus.UPLOAD_SUCCEEDED
-            result = UploadResult.UPLOADED
+            result = if (upload.wasSkipped()) UploadResult.SKIPPED else UploadResult.UPLOADED
         } else if (code.isConflict()) {
             val isSame = FileUploadHelper().isSameFileOnRemote(
                 upload.user,
@@ -600,7 +601,7 @@ class UploadsStorageManager(
         private const val EQUAL = "=="
         private const val OR = " OR "
         private const val AND = " AND "
-        private const val ANGLE_BRACKETS = "<>"
+        private const val NOT_EQUAL = "<>"
         private const val SINGLE_RESULT = 1
 
         private const val QUERY_PAGE_SIZE: Long = 100

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

@@ -41,7 +41,8 @@ public enum UploadResult {
     CANNOT_CREATE_FILE(20),
     LOCAL_STORAGE_NOT_COPIED(21),
     QUOTA_EXCEEDED(22),
-    SAME_FILE_CONFLICT(23);
+    SAME_FILE_CONFLICT(23),
+    SKIPPED(24);
 
     private final int value;
 
@@ -81,7 +82,8 @@ public int getValue() {
         Map.entry(20, CANNOT_CREATE_FILE),
         Map.entry(21, LOCAL_STORAGE_NOT_COPIED),
         Map.entry(22, QUOTA_EXCEEDED),
-        Map.entry(23, SAME_FILE_CONFLICT)
+        Map.entry(23, SAME_FILE_CONFLICT),
+        Map.entry(24, SKIPPED)
                                                                             );
     public static UploadResult fromValue(int value) {
         return valueMap.getOrDefault(value, UNKNOWN);

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

@@ -135,16 +135,17 @@ public class UploadFileOperation extends SyncOperation {
     private OCFile mOldFile;
     private String mRemotePath;
     private String mFolderUnlockToken;
-    private boolean mRemoteFolderToBeCreated;
+    private volatile boolean mRemoteFolderToBeCreated;
     private NameCollisionPolicy mNameCollisionPolicy;
     private int mLocalBehaviour;
-    private int mCreatedBy;
+    private volatile int mCreatedBy;
     private boolean mOnWifiOnly;
     private boolean mWhileChargingOnly;
     private boolean mIgnoringPowerSaveMode;
     private final boolean mDisableRetries;
 
-    private boolean mWasRenamed;
+    private volatile boolean mWasRenamed;
+    private volatile boolean mWasSkipped;
     private long mOCUploadId;
     /**
      * Local path to file which is to be uploaded (before any possible renaming or moving).
@@ -168,7 +169,7 @@ public class UploadFileOperation extends SyncOperation {
     private final ConnectivityService connectivityService;
     private final PowerManagementService powerManagementService;
 
-    private boolean encryptedAncestor;
+    private volatile boolean encryptedAncestor;
     private OCFile duplicatedEncryptedFile;
     private AtomicBoolean missingPermissionThrown = new AtomicBoolean(false);
 
@@ -345,6 +346,10 @@ public boolean wasRenamed() {
         return mWasRenamed;
     }
 
+    public boolean wasSkipped() {
+        return mWasSkipped;
+    }
+
     public void setCreatedBy(int createdBy) {
         mCreatedBy = createdBy;
         if (createdBy < CREATED_BY_USER || CREATED_AS_INSTANT_VIDEO < createdBy) {
@@ -1268,6 +1273,7 @@ private RemoteOperationResult checkNameCollision(OCFile parentFile,
             switch (mNameCollisionPolicy) {
                 case SKIP:
                     Log_OC.d(TAG, "user choose to skip upload if same file exists");
+                    mWasSkipped = true;
                     return new RemoteOperationResult<>(ResultCode.OK);
                 case RENAME:
                     mRemotePath = getNewAvailableRemotePath(client, mRemotePath, fileNames, encrypted);

app/src/main/java/com/owncloud/android/ui/adapter/uploadList/UploadListAdapter.kt

@@ -635,13 +635,18 @@ class UploadListAdapter(
         val capabilities = optionalCapabilities.get()
 
         activity.lifecycleScope.launch(Dispatchers.IO) {
+            val succeededUploads = uploadHelper.getUploadsByStatus(
+                accountName,
+                UploadsStorageManager.UploadStatus.UPLOAD_SUCCEEDED,
+                capabilities
+            )
+
             val updatedSections = uploadListSections.map { sec ->
-                val uploads = uploadHelper.getUploadsByStatus(
-                    accountName,
-                    sec.status,
-                    capabilities,
-                    sec.collisionPolicy
-                )
+                val uploads = when (sec.type) {
+                    UploadListType.COMPLETED -> succeededUploads.filter { it.lastResult != UploadResult.SKIPPED }
+                    UploadListType.SKIPPED -> succeededUploads.filter { it.lastResult == UploadResult.SKIPPED }
+                    else -> uploadHelper.getUploadsByStatus(accountName, sec.status, capabilities)
+                }
                 uploads.forEach { it.setDataFixed(uploadHelper) }
                 sec.withItems(uploads.sortedByUploadOrder())
             }

app/src/main/java/com/owncloud/android/ui/adapter/uploadList/model/UploadListSection.kt

@@ -10,13 +10,11 @@ package com.owncloud.android.ui.adapter.uploadList.model
 import com.owncloud.android.R
 import com.owncloud.android.datamodel.UploadsStorageManager
 import com.owncloud.android.db.OCUpload
-import com.owncloud.android.files.services.NameCollisionPolicy
 
 data class UploadListSection(
     val type: UploadListType?,
     val titleRes: Int,
     val status: UploadsStorageManager.UploadStatus,
-    val collisionPolicy: NameCollisionPolicy?,
     val items: List<OCUpload>
 ) {
     fun withItems(newItems: List<OCUpload>) = copy(items = newItems)
@@ -27,35 +25,30 @@ data class UploadListSection(
                 UploadListType.CURRENT,
                 R.string.uploads_view_group_current_uploads,
                 UploadsStorageManager.UploadStatus.UPLOAD_IN_PROGRESS,
-                null,
                 listOf()
             ),
             UploadListSection(
                 UploadListType.FAILED,
                 R.string.uploads_view_group_failed_uploads,
                 UploadsStorageManager.UploadStatus.UPLOAD_FAILED,
-                null,
                 listOf()
             ),
             UploadListSection(
                 UploadListType.CANCELLED,
                 R.string.uploads_view_group_manually_cancelled_uploads,
                 UploadsStorageManager.UploadStatus.UPLOAD_CANCELLED,
-                null,
                 listOf()
             ),
             UploadListSection(
                 UploadListType.COMPLETED,
                 R.string.uploads_view_group_completed_uploads,
                 UploadsStorageManager.UploadStatus.UPLOAD_SUCCEEDED,
-                NameCollisionPolicy.ASK_USER,
                 listOf()
             ),
             UploadListSection(
                 UploadListType.SKIPPED,
                 R.string.uploads_view_upload_status_skip,
                 UploadsStorageManager.UploadStatus.UPLOAD_SUCCEEDED,
-                NameCollisionPolicy.SKIP,
                 listOf()
             )
         )

app/src/test/java/com/owncloud/android/db/UploadResultTest.kt

@@ -0,0 +1,26 @@
+/*
+ * Nextcloud - Android Client
+ *
+ * SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors
+ * SPDX-License-Identifier: AGPL-3.0-or-later
+ */
+package com.owncloud.android.db
+
+import org.junit.Assert.assertEquals
+import org.junit.Assert.assertNotEquals
+import org.junit.Test
+
+class UploadResultTest {
+
+    @Test
+    fun skippedRoundTripsThroughStoredValue() {
+        val stored = UploadResult.SKIPPED.value
+        assertEquals(UploadResult.SKIPPED, UploadResult.fromValue(stored))
+    }
+
+    @Test
+    fun skippedIsDistinctFromUploaded() {
+        assertNotEquals(UploadResult.SKIPPED.value, UploadResult.UPLOADED.value)
+        assertEquals(UploadResult.UPLOADED, UploadResult.fromValue(UploadResult.UPLOADED.value))
+    }
+}