Merge pull request #15548 from nextcloud/backport/15449/stable-3.33

[stable-3.33] fix: correct query to fetch uploads by IDs and account name

Author: alperozturk96

app/src/main/java/com/nextcloud/client/database/dao/UploadDao.kt

@@ -9,6 +9,7 @@ package com.nextcloud.client.database.dao
 
 import androidx.room.Dao
 import androidx.room.Query
+import com.nextcloud.client.database.entity.UploadEntity
 import com.owncloud.android.db.ProviderMeta.ProviderTableMeta
 
 @Dao
@@ -19,4 +20,11 @@ interface UploadDao {
             ProviderTableMeta.UPLOADS_ACCOUNT_NAME + " = :accountName AND _id IS NOT NULL"
     )
     fun getAllIds(status: Int, accountName: String): List<Int>
+
+    @Query(
+        "SELECT * FROM " + ProviderTableMeta.UPLOADS_TABLE_NAME +
+            " WHERE " + ProviderTableMeta._ID + " IN (:ids) AND " +
+            ProviderTableMeta.UPLOADS_ACCOUNT_NAME + " = :accountName"
+    )
+    fun getUploadsByIds(ids: LongArray, accountName: String): List<UploadEntity>
 }

app/src/main/java/com/nextcloud/client/database/entity/UploadEntity.kt

@@ -10,7 +10,13 @@ package com.nextcloud.client.database.entity
 import androidx.room.ColumnInfo
 import androidx.room.Entity
 import androidx.room.PrimaryKey
+import com.nextcloud.utils.autoRename.AutoRename
+import com.owncloud.android.datamodel.UploadsStorageManager
+import com.owncloud.android.db.OCUpload
 import com.owncloud.android.db.ProviderMeta.ProviderTableMeta
+import com.owncloud.android.db.UploadResult
+import com.owncloud.android.files.services.NameCollisionPolicy
+import com.owncloud.android.lib.resources.status.OCCapability
 
 @Entity(tableName = ProviderTableMeta.UPLOADS_TABLE_NAME)
 data class UploadEntity(
@@ -48,3 +54,27 @@ data class UploadEntity(
     @ColumnInfo(name = ProviderTableMeta.UPLOADS_FOLDER_UNLOCK_TOKEN)
     val folderUnlockToken: String?
 )
+
+fun UploadEntity.toOCUpload(capability: OCCapability? = null): OCUpload {
+    val localPath = localPath
+    var remotePath = remotePath
+    if (capability != null && remotePath != null) {
+        remotePath = AutoRename.rename(remotePath, capability)
+    }
+    val upload = OCUpload(localPath, remotePath, accountName)
+
+    fileSize?.let { upload.fileSize = it }
+    id?.let { upload.uploadId = it.toLong() }
+    status?.let { upload.uploadStatus = UploadsStorageManager.UploadStatus.fromValue(it) }
+    localBehaviour?.let { upload.localAction = it }
+    nameCollisionPolicy?.let { upload.nameCollisionPolicy = NameCollisionPolicy.deserialize(it) }
+    isCreateRemoteFolder?.let { upload.isCreateRemoteFolder = it == 1 }
+    uploadEndTimestamp?.let { upload.uploadEndTimestamp = it.toLong() }
+    lastResult?.let { upload.lastResult = UploadResult.fromValue(it) }
+    createdBy?.let { upload.createdBy = it }
+    isWifiOnly?.let { upload.isUseWifiOnly = it == 1 }
+    isWhileChargingOnly?.let { upload.isWhileChargingOnly = it == 1 }
+    folderUnlockToken?.let { upload.folderUnlockToken = it }
+
+    return upload
+}

app/src/main/java/com/nextcloud/client/jobs/BackgroundJobManagerImpl.kt

@@ -610,13 +610,15 @@ internal class BackgroundJobManagerImpl(
                 .setRequiredNetworkType(NetworkType.CONNECTED)
                 .build()
 
+            val dataBuilder = Data.Builder()
+                .putBoolean(FileUploadWorker.IS_AUTO_UPLOAD, isAutoUpload)
+                .putString(FileUploadWorker.ACCOUNT, user.accountName)
+                .putInt(FileUploadWorker.TOTAL_UPLOAD_SIZE, uploadIds.size)
+
             val workRequests = batches.mapIndexed { index, batch ->
-                val dataBuilder = Data.Builder()
-                    .putBoolean(FileUploadWorker.IS_AUTO_UPLOAD, isAutoUpload)
-                    .putString(FileUploadWorker.ACCOUNT, user.accountName)
+                dataBuilder
                     .putLongArray(FileUploadWorker.UPLOAD_IDS, batch.toLongArray())
                     .putInt(FileUploadWorker.CURRENT_BATCH_INDEX, index)
-                    .putInt(FileUploadWorker.TOTAL_UPLOAD_SIZE, uploadIds.size)
 
                 oneTimeRequestBuilder(FileUploadWorker::class, JOB_FILES_UPLOAD, user)
                     .addTag(tag)

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

@@ -27,6 +27,7 @@ import com.owncloud.android.datamodel.ThumbnailsCacheManager
 import com.owncloud.android.datamodel.UploadsStorageManager
 import com.owncloud.android.db.OCUpload
 import com.owncloud.android.lib.common.OwnCloudAccount
+import com.owncloud.android.lib.common.OwnCloudClient
 import com.owncloud.android.lib.common.OwnCloudClientManagerFactory
 import com.owncloud.android.lib.common.network.OnDatatransferProgressListener
 import com.owncloud.android.lib.common.operations.RemoteOperationResult
@@ -157,9 +158,18 @@ class FileUploadWorker(
             return Result.failure()
         }
 
-        val previouslyUploadedFileSize = currentBatchIndex * FileUploadHelper.MAX_FILE_COUNT
+        // since worker's policy is append or replace and account name comes from there no need check in the loop
+        val optionalUser = userAccountManager.getUser(accountName)
+        if (!optionalUser.isPresent) {
+            Log_OC.e(TAG, "User not found for account: $accountName")
+            return Result.failure()
+        }
 
-        val uploads = uploadIds.map { id -> uploadsStorageManager.getUploadById(id) }.filterNotNull()
+        val user = optionalUser.get()
+        val previouslyUploadedFileSize = currentBatchIndex * FileUploadHelper.MAX_FILE_COUNT
+        val uploads = uploadsStorageManager.getUploadsByIds(uploadIds, accountName)
+        val ocAccount = OwnCloudAccount(user.toPlatformAccount(), context)
+        val client = OwnCloudClientManagerFactory.getDefaultSingleton().getClientFor(ocAccount, context)
 
         for ((index, upload) in uploads.withIndex()) {
             if (preferences.isGlobalUploadPaused) {
@@ -175,19 +185,12 @@ class FileUploadWorker(
                 return Result.failure()
             }
 
-            val user = userAccountManager.getUser(accountName)
-            if (!user.isPresent) {
-                uploadsStorageManager.removeUpload(upload.uploadId)
-                continue
-            }
-
             if (isStopped) {
                 continue
             }
 
-            setWorkerState(user.get())
-
-            val operation = createUploadFileOperation(upload, user.get())
+            setWorkerState(user)
+            val operation = createUploadFileOperation(upload, user)
             currentUploadFileOperation = operation
 
             val currentIndex = (index + 1)
@@ -200,7 +203,7 @@ class FileUploadWorker(
                 totalUploadSize = totalUploadSize
             )
 
-            val result = upload(operation, user.get())
+            val result = upload(operation, user, client)
             currentUploadFileOperation = null
             sendUploadFinishEvent(totalUploadSize, currentUploadIndex, operation, result)
         }
@@ -262,15 +265,16 @@ class FileUploadWorker(
     }
 
     @Suppress("TooGenericExceptionCaught", "DEPRECATION")
-    private fun upload(uploadFileOperation: UploadFileOperation, user: User): RemoteOperationResult<Any?> {
+    private fun upload(
+        uploadFileOperation: UploadFileOperation,
+        user: User,
+        client: OwnCloudClient
+    ): RemoteOperationResult<Any?> {
         lateinit var result: RemoteOperationResult<Any?>
 
         try {
             val storageManager = uploadFileOperation.storageManager
-            val ocAccount = OwnCloudAccount(user.toPlatformAccount(), context)
-            val uploadClient = OwnCloudClientManagerFactory.getDefaultSingleton().getClientFor(ocAccount, context)
-            result = uploadFileOperation.execute(uploadClient)
-
+            result = uploadFileOperation.execute(client)
             val task = ThumbnailsCacheManager.ThumbnailGenerationTask(storageManager, user)
             val file = File(uploadFileOperation.originalStoragePath)
             val remoteId: String? = uploadFileOperation.file.remoteId

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

@@ -27,6 +27,8 @@
 import com.nextcloud.client.account.User;
 import com.nextcloud.client.database.NextcloudDatabase;
 import com.nextcloud.client.database.dao.UploadDao;
+import com.nextcloud.client.database.entity.UploadEntity;
+import com.nextcloud.client.database.entity.UploadEntityKt;
 import com.nextcloud.client.jobs.upload.FileUploadHelper;
 import com.nextcloud.client.jobs.upload.FileUploadWorker;
 import com.nextcloud.utils.autoRename.AutoRename;
@@ -189,9 +191,22 @@ private ContentValues getContentValues(OCUpload ocUpload) {
      * @param ocUpload Upload object with state to update
      * @return num of updated uploads.
      */
-    public int updateUpload(OCUpload ocUpload) {
+    public synchronized int updateUpload(OCUpload ocUpload) {
         Log_OC.v(TAG, "Updating " + ocUpload.getLocalPath() + " with status=" + ocUpload.getUploadStatus());
 
+        OCUpload existingUpload = getUploadById(ocUpload.getUploadId());
+        if (existingUpload == null) {
+            Log_OC.e(TAG, "Upload not found for ID: " + ocUpload.getUploadId());
+            return 0;
+        }
+
+        if (!existingUpload.getAccountName().equals(ocUpload.getAccountName())) {
+            Log_OC.e(TAG, "Account mismatch for upload ID " + ocUpload.getUploadId() +
+                ": expected " + existingUpload.getAccountName() +
+                ", got " + ocUpload.getAccountName());
+            return 0;
+        }
+
         ContentValues cv = new ContentValues();
         cv.put(ProviderTableMeta.UPLOADS_LOCAL_PATH, ocUpload.getLocalPath());
         cv.put(ProviderTableMeta.UPLOADS_REMOTE_PATH, ocUpload.getRemotePath());
@@ -204,8 +219,8 @@ public int updateUpload(OCUpload ocUpload) {
 
         int result = getDB().update(ProviderTableMeta.CONTENT_URI_UPLOADS,
                                     cv,
-                                    ProviderTableMeta._ID + "=?",
-                                    new String[]{String.valueOf(ocUpload.getUploadId())}
+                                    ProviderTableMeta._ID + "=? AND " + ProviderTableMeta.UPLOADS_ACCOUNT_NAME + "=?",
+                                    new String[]{String.valueOf(ocUpload.getUploadId()), ocUpload.getAccountName()}
                                    );
 
         Log_OC.d(TAG, "updateUpload returns with: " + result + " for file: " + ocUpload.getLocalPath());
@@ -437,6 +452,20 @@ OCUpload getUploadById(long id) {
         return result;
     }
 
+    public List<OCUpload> getUploadsByIds(long[] uploadIds, String accountName) {
+        final List<OCUpload> result = new ArrayList<>();
+
+        final List<UploadEntity> entities = uploadDao.getUploadsByIds(uploadIds, accountName);
+        entities.forEach(uploadEntity -> {
+            OCUpload ocUpload = createOCUploadFromEntity(uploadEntity);
+            if (ocUpload != null) {
+                result.add(ocUpload);
+            }
+        });
+
+        return result;
+    }
+
     private OCUpload[] getUploads(@Nullable String selection, @Nullable String... selectionArgs) {
         final List<OCUpload> uploads = new ArrayList<>();
         long page = 0;
@@ -555,6 +584,15 @@ private List<OCUpload> getUploadPage(long limit, final long afterId, final boole
         return uploads;
     }
 
+    @Nullable
+    private OCUpload createOCUploadFromEntity(UploadEntity entity) {
+        if (entity == null) {
+            return null;
+        }
+        initOCCapability();
+        return UploadEntityKt.toOCUpload(entity, capability);
+    }
+
     private OCUpload createOCUploadFromCursor(Cursor c) {
         initOCCapability();
 
@@ -610,16 +648,6 @@ public long[] getCurrentUploadIds(final @NonNull String accountName) {
             .toArray();
     }
 
-    /**
-     * Gets a page of uploads after <code>afterId</code>, where uploads are sorted by ascending upload id.
-     * <p>
-     * If <code>afterId</code> is -1, returns the first page
-     */
-    public List<OCUpload> getCurrentAndPendingUploadsForAccountPageAscById(final long afterId, final @NonNull String accountName) {
-        final String selection = getInProgressAndDelayedUploadsSelection();
-        return getUploadPage(QUERY_PAGE_SIZE, afterId, false, selection, accountName);
-    }
-
     /**
      * Get all failed uploads.
      */
@@ -656,13 +684,6 @@ public OCUpload[] getCancelledUploadsForCurrentAccount() {
                               ProviderTableMeta.UPLOADS_ACCOUNT_NAME + IS_EQUAL, user.getAccountName());
     }
 
-    /**
-     * Get all uploads which where successfully completed.
-     */
-    public OCUpload[] getFinishedUploads() {
-        return getUploads(ProviderTableMeta.UPLOADS_STATUS + EQUAL + UploadStatus.UPLOAD_SUCCEEDED.value, (String[]) null);
-    }
-
     public OCUpload[] getFailedButNotDelayedUploadsForCurrentAccount() {
         User user = currentAccountProvider.getUser();
 
@@ -679,25 +700,6 @@ public OCUpload[] getFailedButNotDelayedUploadsForCurrentAccount() {
                           user.getAccountName());
     }
 
-    /**
-     * Get all failed uploads, except for those that were not performed due to lack of Wifi connection.
-     *
-     * @return Array of failed uploads, except for those that were not performed due to lack of Wifi connection.
-     */
-    public OCUpload[] getFailedButNotDelayedUploads() {
-
-        return getUploads(ProviderTableMeta.UPLOADS_STATUS + EQUAL + UploadStatus.UPLOAD_FAILED.value + AND +
-                              ProviderTableMeta.UPLOADS_LAST_RESULT + ANGLE_BRACKETS + UploadResult.LOCK_FAILED.getValue() +
-                              AND + ProviderTableMeta.UPLOADS_LAST_RESULT +
-                              ANGLE_BRACKETS + UploadResult.DELAYED_FOR_WIFI.getValue() +
-                              AND + ProviderTableMeta.UPLOADS_LAST_RESULT +
-                              ANGLE_BRACKETS + UploadResult.DELAYED_FOR_CHARGING.getValue() +
-                              AND + ProviderTableMeta.UPLOADS_LAST_RESULT +
-                              ANGLE_BRACKETS + UploadResult.DELAYED_IN_POWER_SAVE_MODE.getValue(),
-                          (String[]) null
-                         );
-    }
-
     private ContentResolver getDB() {
         return contentResolver;
     }
@@ -823,39 +825,6 @@ public void updateDatabaseUploadStart(UploadFileOperation upload) {
                           );
     }
 
-    /**
-     * Changes the status of any in progress upload from UploadStatus.UPLOAD_IN_PROGRESS to UploadStatus.UPLOAD_FAILED
-     *
-     * @return Number of uploads which status was changed.
-     */
-    public int failInProgressUploads(UploadResult fail) {
-        Log_OC.v(TAG, "Updating state of any killed upload");
-
-        ContentValues cv = new ContentValues();
-        cv.put(ProviderTableMeta.UPLOADS_STATUS, UploadStatus.UPLOAD_FAILED.getValue());
-        cv.put(
-            ProviderTableMeta.UPLOADS_LAST_RESULT,
-            fail != null ? fail.getValue() : UploadResult.UNKNOWN.getValue()
-              );
-        cv.put(ProviderTableMeta.UPLOADS_UPLOAD_END_TIMESTAMP, Calendar.getInstance().getTimeInMillis());
-
-        int result = getDB().update(
-            ProviderTableMeta.CONTENT_URI_UPLOADS,
-            cv,
-            ProviderTableMeta.UPLOADS_STATUS + "=?",
-            new String[]{String.valueOf(UploadStatus.UPLOAD_IN_PROGRESS.getValue())}
-                                   );
-
-        if (result == 0) {
-            Log_OC.v(TAG, "No upload was killed");
-        } else {
-            Log_OC.w(TAG, Integer.toString(result) + " uploads where abruptly interrupted");
-            notifyObserversNow();
-        }
-
-        return result;
-    }
-
     @VisibleForTesting
     public void removeAllUploads() {
         Log_OC.v(TAG, "Delete all uploads!");