Picture Upload: Fix concurrency issues

For #78

Author: guruz

opencloudApp/src/main/java/eu/opencloud/android/workers/AutomaticUploadsWorker.kt

@@ -90,7 +90,6 @@ class AutomaticUploadsWorker(
                     } catch (illegalArgumentException: IllegalArgumentException) {
                         Timber.e(illegalArgumentException, "Source path for picture uploads is not valid")
                         showNotificationToUpdateUri(SyncType.PICTURE_UPLOADS)
-                        return Result.failure()
                     }
                 }
                 cameraUploadsConfiguration.videoUploadsConfiguration?.let { videoUploadsConfiguration ->
@@ -100,7 +99,6 @@ class AutomaticUploadsWorker(
                     } catch (illegalArgumentException: IllegalArgumentException) {
                         Timber.e(illegalArgumentException, "Source path for video uploads is not valid")
                         showNotificationToUpdateUri(SyncType.VIDEO_UPLOADS)
-                        return Result.failure()
                     }
                 }
             }
@@ -144,6 +142,16 @@ class AutomaticUploadsWorker(
         showNotification(syncType, localPicturesDocumentFiles.size)
 
         for (documentFile in localPicturesDocumentFiles) {
+            // Dedup: if this content URI already has a queued, in-progress, or succeeded transfer,
+            // skip it. Without this, a worker killed mid-loop (before updateTimestamp) or a
+            // file whose lastModified changed (e.g. media scanner) would be re-discovered and
+            // enqueued with a new upload ID — leading to duplicate uploads or 0-byte files
+            // when two workers race on the same cache path.
+            val contentUri = documentFile.uri.toString()
+            if (transferRepository.existsNonFailedTransferForUri(contentUri)) {
+                Timber.d("Skipping already-tracked file: %s", documentFile.name)
+                continue
+            }
             val uploadId = storeInUploadsDatabase(
                 documentFile = documentFile,
                 uploadPath = folderBackUpConfiguration.uploadPath.plus(File.separator).plus(documentFile.name),
@@ -305,6 +313,7 @@ class AutomaticUploadsWorker(
             forceOverwrite = false,
             createdBy = createdByWorker,
             spaceId = spaceId,
+            sourcePath = documentFile.uri.toString(),
         )
 
         return transferRepository.saveTransfer(ocTransfer)

opencloudApp/src/main/java/eu/opencloud/android/workers/UploadFileFromContentUriWorker.kt

@@ -204,13 +204,26 @@ class UploadFileFromContentUriWorker(
         }
         cacheFile.createNewFile()
 
+        // openInputStream can return null if the content provider is unavailable or permissions were revoked.
+        // Failing here avoids silently uploading a 0-byte file.
         val inputStream = appContext.contentResolver.openInputStream(contentUri)
+        if (inputStream == null) {
+            Timber.e("Failed to open input stream for %s — content provider unavailable or permissions revoked", contentUri)
+            throw LocalFileNotFoundException()
+        }
         val outputStream = FileOutputStream(cachePath)
-        outputStream.use { fileOut ->
-            inputStream?.copyTo(fileOut)
+        inputStream.use { input ->
+            outputStream.use { output ->
+                input.copyTo(output)
+            }
+        }
+
+        // Guard against a truncated or empty copy (e.g. file deleted mid-read).
+        if (cacheFile.length() == 0L) {
+            Timber.e("Cache file is 0 bytes after copy from %s — source may have been deleted mid-read", contentUri)
+            cacheFile.delete()
+            throw LocalFileNotFoundException()
         }
-        inputStream?.close()
-        outputStream.close()
 
         transferRepository.updateTransferSourcePath(uploadIdInStorageManager, contentUri.toString())
         transferRepository.updateTransferLocalPath(uploadIdInStorageManager, cachePath)

opencloudData/src/main/java/eu/opencloud/android/data/transfers/datasources/LocalTransferDataSource.kt

@@ -57,6 +57,7 @@ interface LocalTransferDataSource {
     fun getFinishedTransfers(): List<OCTransfer>
     fun clearFailedTransfers()
     fun clearSuccessfulTransfers()
+    fun existsNonFailedTransferForUri(uri: String): Boolean
 
     // TUS state management
     fun updateTusState(

opencloudData/src/main/java/eu/opencloud/android/data/transfers/datasources/implementation/OCLocalTransferDataSource.kt

@@ -141,6 +141,9 @@ class OCLocalTransferDataSource(
         transferDao.deleteTransfersWithStatus(TransferStatus.TRANSFER_SUCCEEDED.value)
     }
 
+    override fun existsNonFailedTransferForUri(uri: String): Boolean =
+        transferDao.existsNonFailedTransferForUri(uri)
+
     // TUS state management
     override fun updateTusState(
         id: Long,

opencloudData/src/main/java/eu/opencloud/android/data/transfers/db/TransferDao.kt

@@ -88,6 +88,9 @@ interface TransferDao {
     @Query(DELETE_TRANSFERS_WITH_STATUS)
     fun deleteTransfersWithStatus(status: Int)
 
+    @Query(EXISTS_NON_FAILED_TRANSFER_FOR_URI)
+    fun existsNonFailedTransferForUri(uri: String): Boolean
+
     companion object {
         private const val SELECT_TRANSFER_WITH_ID = """
             SELECT *
@@ -177,5 +180,15 @@ interface TransferDao {
             FROM $TRANSFERS_TABLE_NAME
             WHERE status = :status
         """
+
+        /** status 2 = TRANSFER_FAILED, see [TransferStatus] */
+        private const val EXISTS_NON_FAILED_TRANSFER_FOR_URI = """
+            SELECT EXISTS(
+                SELECT 1
+                FROM $TRANSFERS_TABLE_NAME
+                WHERE sourcePath = :uri
+                  AND status != 2
+            )
+        """
     }
 }

opencloudData/src/main/java/eu/opencloud/android/data/transfers/repository/OCTransferRepository.kt

@@ -112,6 +112,9 @@ class OCTransferRepository(
     override fun clearSuccessfulTransfers() =
         localTransferDataSource.clearSuccessfulTransfers()
 
+    override fun existsNonFailedTransferForUri(uri: String): Boolean =
+        localTransferDataSource.existsNonFailedTransferForUri(uri)
+
     // TUS state management
     override fun updateTusState(
         id: Long,

opencloudDomain/src/main/java/eu/opencloud/android/domain/transfers/TransferRepository.kt

@@ -57,6 +57,7 @@ interface TransferRepository {
     fun getFinishedTransfers(): List<OCTransfer>
     fun clearFailedTransfers()
     fun clearSuccessfulTransfers()
+    fun existsNonFailedTransferForUri(uri: String): Boolean
 
     // TUS state management
     fun updateTusState(