From 637716fa2f1e3f39ca4642a0688a98f7e465378a Mon Sep 17 00:00:00 2001 From: alperozturk96 Date: Tue, 14 Jul 2026 15:30:26 +0200 Subject: [PATCH 1/3] fix(auto-upload): uploading logic Signed-off-by: alperozturk96 --- .../jobs/autoUpload/AutoUploadWorker.kt | 23 +++-- .../jobs/autoUpload/FileSystemRepository.kt | 88 +++++++++++-------- .../extensions/UploadResultExtensions.kt | 4 - .../owncloud/android/utils/PermissionUtil.kt | 25 +++++- 4 files changed, 92 insertions(+), 48 deletions(-) diff --git a/app/src/main/java/com/nextcloud/client/jobs/autoUpload/AutoUploadWorker.kt b/app/src/main/java/com/nextcloud/client/jobs/autoUpload/AutoUploadWorker.kt index 777d03235757..efbd8e0084e5 100644 --- a/app/src/main/java/com/nextcloud/client/jobs/autoUpload/AutoUploadWorker.kt +++ b/app/src/main/java/com/nextcloud/client/jobs/autoUpload/AutoUploadWorker.kt @@ -44,6 +44,7 @@ import com.owncloud.android.lib.common.utils.Log_OC import com.owncloud.android.lib.resources.status.OCCapability import com.owncloud.android.operations.UploadFileOperation import com.owncloud.android.ui.activity.SettingsActivity +import com.owncloud.android.utils.PermissionUtil import com.owncloud.android.utils.theme.CapabilityUtils import com.owncloud.android.utils.theme.ViewThemeUtils import kotlinx.coroutines.Dispatchers @@ -53,6 +54,7 @@ import kotlinx.coroutines.isActive import kotlinx.coroutines.withContext import java.io.File import kotlin.coroutines.cancellation.CancellationException +import kotlin.time.Duration.Companion.milliseconds @Suppress("LongParameterList", "TooManyFunctions", "TooGenericExceptionCaught") class AutoUploadWorker( @@ -94,6 +96,13 @@ class AutoUploadWorker( Log_OC.d(TAG, syncedFolder.getLog()) + if (!PermissionUtil.checkStoragePermission(context)) { + Log_OC.w( + TAG, + "⚠️ storage/media permission missing or partial — MediaStore scan may return no files" + ) + } + if (canExitEarly(syncFolderId)) { return Result.success() } @@ -274,7 +283,7 @@ class AutoUploadWorker( filePathsWithIds.forEachIndexed { batchIndex, (path, id) -> ensureActive() - delay(retryPolicy.getDelay()) + delay(retryPolicy.getDelay().milliseconds) val file = File(path) val localPath = file.absolutePath @@ -308,8 +317,8 @@ class AutoUploadWorker( val operation = createUploadFileOperation(upload, user) Log_OC.d(TAG, "🕒 uploading: $localPath, id: $generatedId") - val result = operation.execute(client) fileUploadEventBroadcaster.sendUploadStarted(operation, context) + val result = operation.execute(client) UploadErrorNotificationManager.handleResult( context, @@ -330,10 +339,14 @@ class AutoUploadWorker( "❌ upload failed $localPath (${upload.accountName}): ${result.logMessage}" ) - // Mark CONFLICT files as handled to prevent retries - if (result.code.isConflict()) { + // Only SKIP-policy collisions are terminal. For other policies the recorded + // conflict result and its resolve notification must keep the file resolvable + // instead of silently marking it handled. + if (result.code.isConflict() && + syncedFolder.nameCollisionPolicy == NameCollisionPolicy.SKIP + ) { repository.markFileAsHandled(localPath, syncedFolder) - Log_OC.w(TAG, "Marked CONFLICT file as handled: $localPath") + Log_OC.w(TAG, "Marked CONFLICT file as handled (SKIP policy): $localPath") } } diff --git a/app/src/main/java/com/nextcloud/client/jobs/autoUpload/FileSystemRepository.kt b/app/src/main/java/com/nextcloud/client/jobs/autoUpload/FileSystemRepository.kt index 6ca9e37629fc..4aeca879b75b 100644 --- a/app/src/main/java/com/nextcloud/client/jobs/autoUpload/FileSystemRepository.kt +++ b/app/src/main/java/com/nextcloud/client/jobs/autoUpload/FileSystemRepository.kt @@ -8,6 +8,7 @@ package com.nextcloud.client.jobs.autoUpload import android.content.Context +import android.database.Cursor import android.net.Uri import android.os.Build import android.provider.MediaStore @@ -59,22 +60,39 @@ class FileSystemRepository( val syncedFolderId = syncedFolder.id.toString() Log_OC.d(TAG, "Fetching candidate files for syncedFolderId = $syncedFolderId") - val entities = dao.getAutoUploadFilesEntities(syncedFolderId, BATCH_SIZE, lastId) val filtered = mutableListOf>() - - for (entity in entities) { - val file = SyncedFolderUtils.validateForAutoUpload(entity.localPath) - if (file == null) { - deleteAutoUploadAndUploadEntity(syncedFolder, entity.localPath ?: "", entity) - continue + var currentLastId = lastId + + // Keep scanning past batches whose rows are all invalid so valid files with higher ids are not + // skipped when an early batch consists entirely of stale/unqualified entries. + while (filtered.size < BATCH_SIZE) { + val entities = dao.getAutoUploadFilesEntities(syncedFolderId, BATCH_SIZE, currentLastId) + if (entities.isEmpty()) { + break } - Log_OC.d(TAG, "Adding path to upload: ${entity.localPath}") + val batchStartId = currentLastId + + for (entity in entities) { + entity.id?.let { currentLastId = maxOf(currentLastId, it) } + + val file = SyncedFolderUtils.validateForAutoUpload(entity.localPath) + if (file == null) { + deleteAutoUploadAndUploadEntity(syncedFolder, entity.localPath ?: "", entity) + continue + } + + Log_OC.d(TAG, "Adding path to upload: ${entity.localPath}") - if (entity.id != null) { - filtered.add(entity.localPath!! to entity.id) - } else { - Log_OC.w(TAG, "cant adding path to upload, id is null") + if (entity.id != null) { + filtered.add(entity.localPath!! to entity.id) + } else { + Log_OC.w(TAG, "cant adding path to upload, id is null") + } + } + + if (currentLastId == batchStartId) { + break } } @@ -142,38 +160,32 @@ class FileSystemRepository( null ) - cursor?.use { - val idxData = cursor.getColumnIndex(MediaStore.MediaColumns.DATA) - val idxModified = cursor.getColumnIndex(MediaStore.MediaColumns.DATE_MODIFIED) - val idxAdded = cursor.getColumnIndex(MediaStore.MediaColumns.DATE_ADDED) + if (cursor == null) { + Log_OC.w(TAG, "MediaStore query returned null (missing permission or unavailable provider): $uri") + return + } - if (idxData == -1) { - Log_OC.e(TAG, "MediaStore column DATA missing — cannot process URI: $uri") - return - } + cursor.use { insertFromCursor(it, uri, syncedFolder, checkFileType) } + } - while (cursor.moveToNext()) { - val filePath = cursor.getString(idxData) + private fun insertFromCursor(cursor: Cursor, uri: Uri, syncedFolder: SyncedFolder, checkFileType: Boolean) { + val idxData = cursor.getColumnIndex(MediaStore.MediaColumns.DATA) + val idxModified = cursor.getColumnIndex(MediaStore.MediaColumns.DATE_MODIFIED) + val idxAdded = cursor.getColumnIndex(MediaStore.MediaColumns.DATE_ADDED) - val lastModifiedMs = if (idxModified != -1) { - cursor.getLong(idxModified) * 1000 - } else { - null - } + if (idxData == -1) { + Log_OC.e(TAG, "MediaStore column DATA missing — cannot process URI: $uri") + return + } - val creationTimeMs = if (idxAdded != -1) { - cursor.getLong(idxAdded) * 1000 - } else { - null - } + while (cursor.moveToNext()) { + val filePath = cursor.getString(idxData) + val lastModifiedMs = if (idxModified != -1) cursor.getLong(idxModified) * 1000 else null + val creationTimeMs = if (idxAdded != -1) cursor.getLong(idxAdded) * 1000 else null - Log_OC.d( - TAG, - "Found file: $filePath (created=$creationTimeMs, modified=$lastModifiedMs)" - ) + Log_OC.d(TAG, "Found file: $filePath (created=$creationTimeMs, modified=$lastModifiedMs)") - insertOrReplace(filePath, lastModifiedMs, creationTimeMs, syncedFolder, checkFileType) - } + insertOrReplace(filePath, lastModifiedMs, creationTimeMs, syncedFolder, checkFileType) } } diff --git a/app/src/main/java/com/nextcloud/utils/extensions/UploadResultExtensions.kt b/app/src/main/java/com/nextcloud/utils/extensions/UploadResultExtensions.kt index 4ca297751522..96fc79da7023 100644 --- a/app/src/main/java/com/nextcloud/utils/extensions/UploadResultExtensions.kt +++ b/app/src/main/java/com/nextcloud/utils/extensions/UploadResultExtensions.kt @@ -25,10 +25,6 @@ fun UploadResult.isNonRetryable(): Boolean = when (this) { UploadResult.PRIVILEGES_ERROR, UploadResult.CREDENTIAL_ERROR, - // most cases covered and mapped from RemoteOperationResult. Most likely UploadResult.UNKNOWN this error will - // occur again - UploadResult.UNKNOWN, - // user's choice UploadResult.CANCELLED -> true diff --git a/app/src/main/java/com/owncloud/android/utils/PermissionUtil.kt b/app/src/main/java/com/owncloud/android/utils/PermissionUtil.kt index 7234995b2ab3..35cf717e87c6 100644 --- a/app/src/main/java/com/owncloud/android/utils/PermissionUtil.kt +++ b/app/src/main/java/com/owncloud/android/utils/PermissionUtil.kt @@ -190,7 +190,30 @@ object PermissionUtil { fun checkAllFilesAccess(): Boolean = Build.VERSION.SDK_INT >= Build.VERSION_CODES.R && Environment.isExternalStorageManager() - fun checkMediaAccess(context: Context): Boolean = checkPermissions(context, getRequiredStoragePermissions()) + fun checkMediaAccess(context: Context): Boolean = when { + Build.VERSION.SDK_INT >= Build.VERSION_CODES.TIRAMISU -> hasVisualMediaAccess(context) + + Build.VERSION.SDK_INT >= Build.VERSION_CODES.Q -> + checkSelfPermission(context, Manifest.permission.READ_EXTERNAL_STORAGE) + + else -> checkPermissions(context, getLegacyStoragePermissions()) + } + + /** + * ACCESS_MEDIA_LOCATION only exposes EXIF GPS metadata and must never gate media access, and image-only + * access is enough to read images. Requiring every permission together previously disabled media access + * (and auto-upload) whenever the user declined location-in-media or granted only one media type. + * + * Partial access (READ_MEDIA_VISUAL_USER_SELECTED) is intentionally excluded: MediaStore then only exposes + * the user-selected subset, which cannot back reliable browsing or auto-upload of newly created files. + */ + @RequiresApi(Build.VERSION_CODES.TIRAMISU) + private fun hasVisualMediaAccess(context: Context): Boolean { + val hasImages = checkSelfPermission(context, Manifest.permission.READ_MEDIA_IMAGES) + val hasVideo = checkSelfPermission(context, Manifest.permission.READ_MEDIA_VIDEO) + + return hasImages || hasVideo + } private fun getRequiredStoragePermissions() = when { Build.VERSION.SDK_INT >= Build.VERSION_CODES.UPSIDE_DOWN_CAKE -> getApiLevel34StoragePermissions() From 766c36a801a0a7f2f288db0ab8d3eb79905e9141 Mon Sep 17 00:00:00 2001 From: alperozturk96 Date: Tue, 14 Jul 2026 15:31:55 +0200 Subject: [PATCH 2/3] wip Signed-off-by: alperozturk96 --- .../nextcloud/client/jobs/autoUpload/AutoUploadWorker.kt | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/app/src/main/java/com/nextcloud/client/jobs/autoUpload/AutoUploadWorker.kt b/app/src/main/java/com/nextcloud/client/jobs/autoUpload/AutoUploadWorker.kt index efbd8e0084e5..a7e1bda594d0 100644 --- a/app/src/main/java/com/nextcloud/client/jobs/autoUpload/AutoUploadWorker.kt +++ b/app/src/main/java/com/nextcloud/client/jobs/autoUpload/AutoUploadWorker.kt @@ -339,12 +339,7 @@ class AutoUploadWorker( "❌ upload failed $localPath (${upload.accountName}): ${result.logMessage}" ) - // Only SKIP-policy collisions are terminal. For other policies the recorded - // conflict result and its resolve notification must keep the file resolvable - // instead of silently marking it handled. - if (result.code.isConflict() && - syncedFolder.nameCollisionPolicy == NameCollisionPolicy.SKIP - ) { + if (result.code.isConflict()) { repository.markFileAsHandled(localPath, syncedFolder) Log_OC.w(TAG, "Marked CONFLICT file as handled (SKIP policy): $localPath") } From 217035e05160e0a04baa3b7b48a02ceeaaff33f6 Mon Sep 17 00:00:00 2001 From: alperozturk96 Date: Tue, 14 Jul 2026 15:45:45 +0200 Subject: [PATCH 3/3] wip Signed-off-by: alperozturk96 --- .../jobs/autoUpload/AutoUploadWorker.kt | 2 +- .../jobs/autoUpload/FileSystemRepository.kt | 41 ++++++------------- .../owncloud/android/utils/PermissionUtil.kt | 25 +---------- 3 files changed, 14 insertions(+), 54 deletions(-) diff --git a/app/src/main/java/com/nextcloud/client/jobs/autoUpload/AutoUploadWorker.kt b/app/src/main/java/com/nextcloud/client/jobs/autoUpload/AutoUploadWorker.kt index a7e1bda594d0..1c7e20031e2f 100644 --- a/app/src/main/java/com/nextcloud/client/jobs/autoUpload/AutoUploadWorker.kt +++ b/app/src/main/java/com/nextcloud/client/jobs/autoUpload/AutoUploadWorker.kt @@ -341,7 +341,7 @@ class AutoUploadWorker( if (result.code.isConflict()) { repository.markFileAsHandled(localPath, syncedFolder) - Log_OC.w(TAG, "Marked CONFLICT file as handled (SKIP policy): $localPath") + Log_OC.w(TAG, "Marked CONFLICT file as handled: $localPath") } } diff --git a/app/src/main/java/com/nextcloud/client/jobs/autoUpload/FileSystemRepository.kt b/app/src/main/java/com/nextcloud/client/jobs/autoUpload/FileSystemRepository.kt index 4aeca879b75b..0746c4e66d99 100644 --- a/app/src/main/java/com/nextcloud/client/jobs/autoUpload/FileSystemRepository.kt +++ b/app/src/main/java/com/nextcloud/client/jobs/autoUpload/FileSystemRepository.kt @@ -60,39 +60,22 @@ class FileSystemRepository( val syncedFolderId = syncedFolder.id.toString() Log_OC.d(TAG, "Fetching candidate files for syncedFolderId = $syncedFolderId") + val entities = dao.getAutoUploadFilesEntities(syncedFolderId, BATCH_SIZE, lastId) val filtered = mutableListOf>() - var currentLastId = lastId - - // Keep scanning past batches whose rows are all invalid so valid files with higher ids are not - // skipped when an early batch consists entirely of stale/unqualified entries. - while (filtered.size < BATCH_SIZE) { - val entities = dao.getAutoUploadFilesEntities(syncedFolderId, BATCH_SIZE, currentLastId) - if (entities.isEmpty()) { - break - } - - val batchStartId = currentLastId - - for (entity in entities) { - entity.id?.let { currentLastId = maxOf(currentLastId, it) } - - val file = SyncedFolderUtils.validateForAutoUpload(entity.localPath) - if (file == null) { - deleteAutoUploadAndUploadEntity(syncedFolder, entity.localPath ?: "", entity) - continue - } - Log_OC.d(TAG, "Adding path to upload: ${entity.localPath}") - - if (entity.id != null) { - filtered.add(entity.localPath!! to entity.id) - } else { - Log_OC.w(TAG, "cant adding path to upload, id is null") - } + for (entity in entities) { + val file = SyncedFolderUtils.validateForAutoUpload(entity.localPath) + if (file == null) { + deleteAutoUploadAndUploadEntity(syncedFolder, entity.localPath ?: "", entity) + continue } - if (currentLastId == batchStartId) { - break + Log_OC.d(TAG, "Adding path to upload: ${entity.localPath}") + + if (entity.id != null) { + filtered.add(entity.localPath!! to entity.id) + } else { + Log_OC.w(TAG, "cant adding path to upload, id is null") } } diff --git a/app/src/main/java/com/owncloud/android/utils/PermissionUtil.kt b/app/src/main/java/com/owncloud/android/utils/PermissionUtil.kt index 35cf717e87c6..7234995b2ab3 100644 --- a/app/src/main/java/com/owncloud/android/utils/PermissionUtil.kt +++ b/app/src/main/java/com/owncloud/android/utils/PermissionUtil.kt @@ -190,30 +190,7 @@ object PermissionUtil { fun checkAllFilesAccess(): Boolean = Build.VERSION.SDK_INT >= Build.VERSION_CODES.R && Environment.isExternalStorageManager() - fun checkMediaAccess(context: Context): Boolean = when { - Build.VERSION.SDK_INT >= Build.VERSION_CODES.TIRAMISU -> hasVisualMediaAccess(context) - - Build.VERSION.SDK_INT >= Build.VERSION_CODES.Q -> - checkSelfPermission(context, Manifest.permission.READ_EXTERNAL_STORAGE) - - else -> checkPermissions(context, getLegacyStoragePermissions()) - } - - /** - * ACCESS_MEDIA_LOCATION only exposes EXIF GPS metadata and must never gate media access, and image-only - * access is enough to read images. Requiring every permission together previously disabled media access - * (and auto-upload) whenever the user declined location-in-media or granted only one media type. - * - * Partial access (READ_MEDIA_VISUAL_USER_SELECTED) is intentionally excluded: MediaStore then only exposes - * the user-selected subset, which cannot back reliable browsing or auto-upload of newly created files. - */ - @RequiresApi(Build.VERSION_CODES.TIRAMISU) - private fun hasVisualMediaAccess(context: Context): Boolean { - val hasImages = checkSelfPermission(context, Manifest.permission.READ_MEDIA_IMAGES) - val hasVideo = checkSelfPermission(context, Manifest.permission.READ_MEDIA_VIDEO) - - return hasImages || hasVideo - } + fun checkMediaAccess(context: Context): Boolean = checkPermissions(context, getRequiredStoragePermissions()) private fun getRequiredStoragePermissions() = when { Build.VERSION.SDK_INT >= Build.VERSION_CODES.UPSIDE_DOWN_CAKE -> getApiLevel34StoragePermissions()