fix: choose correct upload ids for retry

Signed-off-by: alperozturk <alper_ozturk@proton.me>

Author: alperozturk96

app/src/main/java/com/nextcloud/client/jobs/upload/FileUploadHelper.kt

@@ -88,32 +88,39 @@ class FileUploadHelper {
         fun buildRemoteName(accountName: String, remotePath: String): String = accountName + remotePath
     }
 
+    fun getUploadsByStatus(accountName: String, status: UploadStatus, onCompleted: (Array<OCUpload>) -> Unit) {
+        ioScope.launch {
+            val result = uploadsStorageManager.uploadDao.getUploadsByStatus(
+                accountName,
+                status.value
+            ).map { it.toOCUpload(null) }.toTypedArray()
+            onCompleted(result)
+        }
+    }
+
     fun retryFailedUploads(
         uploadsStorageManager: UploadsStorageManager,
         connectivityService: ConnectivityService,
         accountManager: UserAccountManager,
         powerManagementService: PowerManagementService
     ) {
-        if (retryFailedUploadsSemaphore.tryAcquire()) {
-            try {
-                val failedUploads = uploadsStorageManager.failedUploads
-                if (failedUploads == null || failedUploads.isEmpty()) {
-                    Log_OC.d(TAG, "Failed uploads are empty or null")
-                    return
-                }
+        if (!retryFailedUploadsSemaphore.tryAcquire()) {
+            Log_OC.d(TAG, "skipping retryFailedUploads, already running")
+            return
+        }
 
+        try {
+            getUploadsByStatus(accountManager.user.accountName, UploadStatus.UPLOAD_FAILED) {
                 retryUploads(
                     uploadsStorageManager,
                     connectivityService,
                     accountManager,
                     powerManagementService,
-                    failedUploads
+                    uploads = it
                 )
-            } finally {
-                retryFailedUploadsSemaphore.release()
             }
-        } else {
-            Log_OC.d(TAG, "Skip retryFailedUploads since it is already running")
+        } finally {
+            retryFailedUploadsSemaphore.release()
         }
     }
 
@@ -123,18 +130,18 @@ class FileUploadHelper {
         accountManager: UserAccountManager,
         powerManagementService: PowerManagementService
     ): Boolean {
-        val cancelledUploads = uploadsStorageManager.cancelledUploadsForCurrentAccount
-        if (cancelledUploads == null || cancelledUploads.isEmpty()) {
-            return false
+        var result = false
+        getUploadsByStatus(accountManager.user.accountName, UploadStatus.UPLOAD_CANCELLED) {
+            result = retryUploads(
+                uploadsStorageManager,
+                connectivityService,
+                accountManager,
+                powerManagementService,
+                it
+            )
         }
 
-        return retryUploads(
-            uploadsStorageManager,
-            connectivityService,
-            accountManager,
-            powerManagementService,
-            cancelledUploads
-        )
+        return result
     }
 
     @Suppress("ComplexCondition")
@@ -143,51 +150,51 @@ class FileUploadHelper {
         connectivityService: ConnectivityService,
         accountManager: UserAccountManager,
         powerManagementService: PowerManagementService,
-        failedUploads: Array<OCUpload>
+        uploads: Array<OCUpload>
     ): Boolean {
         var showNotExistMessage = false
         val isOnline = checkConnectivity(connectivityService)
         val connectivity = connectivityService.connectivity
         val batteryStatus = powerManagementService.battery
-        val accountNames = accountManager.accounts.filter { account ->
-            accountManager.getUser(account.name).isPresent
-        }.map { account ->
-            account.name
-        }.toHashSet()
-
-        for (failedUpload in failedUploads) {
-            if (!accountNames.contains(failedUpload.accountName)) {
-                uploadsStorageManager.removeUpload(failedUpload)
-                continue
-            }
 
-            val uploadResult =
-                checkUploadConditions(failedUpload, connectivity, batteryStatus, powerManagementService, isOnline)
+        val uploadsToRetry = mutableListOf<Long>()
+
+        for (upload in uploads) {
+            val uploadResult = checkUploadConditions(
+                upload,
+                connectivity,
+                batteryStatus,
+                powerManagementService,
+                isOnline
+            )
 
             if (uploadResult != UploadResult.UPLOADED) {
-                if (failedUpload.lastResult != uploadResult) {
+                if (upload.lastResult != uploadResult) {
                     // Setting Upload status else cancelled uploads will behave wrong, when retrying
                     // Needs to happen first since lastResult wil be overwritten by setter
-                    failedUpload.uploadStatus = UploadStatus.UPLOAD_FAILED
+                    upload.uploadStatus = UploadStatus.UPLOAD_FAILED
 
-                    failedUpload.lastResult = uploadResult
-                    uploadsStorageManager.updateUpload(failedUpload)
+                    upload.lastResult = uploadResult
+                    uploadsStorageManager.updateUpload(upload)
                 }
                 if (uploadResult == UploadResult.FILE_NOT_FOUND) {
                     showNotExistMessage = true
                 }
                 continue
             }
 
-            failedUpload.uploadStatus = UploadStatus.UPLOAD_IN_PROGRESS
-            uploadsStorageManager.updateUpload(failedUpload)
+            // Only uploads that passed checks get marked in progress and are collected for scheduling
+            upload.uploadStatus = UploadStatus.UPLOAD_IN_PROGRESS
+            uploadsStorageManager.updateUpload(upload)
+            uploadsToRetry.add(upload.uploadId)
         }
 
-        accountNames.forEach { accountName ->
-            val user = accountManager.getUser(accountName)
-            if (user.isPresent) {
-                backgroundJobManager.startFilesUploadJob(user.get(), failedUploads.getUploadIds(), false)
-            }
+        if (uploadsToRetry.isNotEmpty()) {
+            backgroundJobManager.startFilesUploadJob(
+                accountManager.user,
+                uploadsToRetry.toLongArray(),
+                false
+            )
         }
 
         return showNotExistMessage
@@ -235,15 +242,6 @@ class FileUploadHelper {
         }
     }
 
-    fun getUploadsByStatus(accountName: String, status: UploadStatus, onCompleted: (Array<OCUpload>) -> Unit) {
-        ioScope.launch {
-            val result =
-                uploadsStorageManager.uploadDao.getUploadsByStatus(accountName, status.value)
-                    .map { it.toOCUpload(null) }.toTypedArray()
-            onCompleted(result)
-        }
-    }
-
     fun cancelAndRestartUploadJob(user: User, uploadIds: LongArray) {
         backgroundJobManager.run {
             cancelFilesUploadJob(user)

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

@@ -483,35 +483,10 @@ public long[] getCurrentUploadIds(final @NonNull String accountName) {
             .toArray();
     }
 
-    /**
-     * Get all failed uploads.
-     */
-    public OCUpload[] getFailedUploads() {
-        return getUploads("(" + ProviderTableMeta.UPLOADS_STATUS + IS_EQUAL +
-                              OR + ProviderTableMeta.UPLOADS_LAST_RESULT +
-                              EQUAL + UploadResult.DELAYED_FOR_WIFI.getValue() +
-                              OR + ProviderTableMeta.UPLOADS_LAST_RESULT +
-                              EQUAL + UploadResult.LOCK_FAILED.getValue() +
-                              OR + ProviderTableMeta.UPLOADS_LAST_RESULT +
-                              EQUAL + UploadResult.DELAYED_FOR_CHARGING.getValue() +
-                              OR + ProviderTableMeta.UPLOADS_LAST_RESULT +
-                              EQUAL + UploadResult.DELAYED_IN_POWER_SAVE_MODE.getValue() +
-                              " ) AND " + ProviderTableMeta.UPLOADS_LAST_RESULT +
-                              "!= " + UploadResult.VIRUS_DETECTED.getValue()
-            , String.valueOf(UploadStatus.UPLOAD_FAILED.value));
-    }
-
     public OCUpload[] getUploadsForAccount(final @NonNull String accountName) {
         return getUploads(ProviderTableMeta.UPLOADS_ACCOUNT_NAME + IS_EQUAL, accountName);
     }
 
-    public OCUpload[] getCancelledUploadsForCurrentAccount() {
-        User user = currentAccountProvider.getUser();
-
-        return getUploads(ProviderTableMeta.UPLOADS_STATUS + EQUAL + UploadStatus.UPLOAD_CANCELLED.value + AND +
-                              ProviderTableMeta.UPLOADS_ACCOUNT_NAME + IS_EQUAL, user.getAccountName());
-    }
-
     private ContentResolver getDB() {
         return contentResolver;
     }

app/src/main/java/com/owncloud/android/ui/activity/UploadListActivity.java

@@ -35,6 +35,7 @@
 import com.owncloud.android.datamodel.OCFile;
 import com.owncloud.android.datamodel.SyncedFolderProvider;
 import com.owncloud.android.datamodel.UploadsStorageManager;
+import com.owncloud.android.db.OCUpload;
 import com.owncloud.android.lib.common.operations.RemoteOperation;
 import com.owncloud.android.lib.common.operations.RemoteOperationResult;
 import com.owncloud.android.lib.common.utils.Log_OC;
@@ -50,6 +51,8 @@
 import androidx.recyclerview.widget.GridLayoutManager;
 import androidx.swiperefreshlayout.widget.SwipeRefreshLayout;
 import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
+import kotlin.Unit;
+import kotlin.jvm.functions.Function1;
 
 /**
  * Activity listing pending, active, and completed uploads. User can delete completed uploads from view. Content of this
@@ -186,18 +189,21 @@ private void refresh() {
                                                     backgroundJobManager,
                                                     true);
 
-        if (uploadsStorageManager.getFailedUploads().length > 0) {
-            new Thread(() -> {
-                FileUploadHelper.Companion.instance().retryFailedUploads(
-                    uploadsStorageManager,
-                    connectivityService,
-                    accountManager,
-                    powerManagementService);
-                uploadListAdapter.loadUploadItemsFromDb();
-            }).start();
-            DisplayUtils.showSnackMessage(this, R.string.uploader_local_files_uploaded);
-        }
-
+        FileUploadHelper.Companion.instance().getUploadsByStatus(accountManager.getUser().getAccountName(), UploadsStorageManager.UploadStatus.UPLOAD_FAILED, new Function1<>() {
+            @Override
+            public Unit invoke(OCUpload[] ocUploads) {
+                new Thread(() -> {
+                    FileUploadHelper.Companion.instance().retryFailedUploads(
+                        uploadsStorageManager,
+                        connectivityService,
+                        accountManager,
+                        powerManagementService);
+                    uploadListAdapter.loadUploadItemsFromDb();
+                }).start();
+                DisplayUtils.showSnackMessage(UploadListActivity.this, R.string.uploader_local_files_uploaded);
+                return Unit.INSTANCE;
+            }
+        });
 
         // update UI
         uploadListAdapter.loadUploadItemsFromDb(() -> swipeListRefreshLayout.setRefreshing(false));