BugFix - Track File Upload Index

[alperozturk96]

• Tests written, or not not needed

Problem

Currently, the logic implemented in FileUploadWorker processes file uploads in discrete batches, which limits our ability to track the overall upload progress accurately and total upload index is wrong. We have outer while loop to update uploads and inner for loop for uploads.

For example, when a user attempts to upload 164 files:

1. The first batch is processed, triggering a progress notification for files 1–2.
2. The second batch then triggers a notification for files 23–2.
3. The third batch triggers a notification for files 53–2
4. ... and final batch 164-2.

How to reproduce?

1. Create a folder
2. Try to upload n times file
3. Observe file upload notification

Solution

• Pass totalUploadSize to the worker and utilize it internally.
• Update FileUploadWorker to support parallel job execution instead of queuing. This allows users to initiate multiple folder uploads independently, enabling better performance and more accurate tracking in notifications.

Demo

ddd.mp4

PR is updated

[alperozturk96]

During my recent testing, I found an issue with the use of getCurrentUploadsForAccountPageAscById(). This function returns uploads that are currently in progress or scheduled for execution. When a user triggers two separate FileUploadWorker instances consecutively it causes a problem.

Example scenario

Consider a user with a folder structure where folder A contains 101 files, including a subfolder named inner_folder which also contains 101 files. If the user selects all files in folder A except the inner_folder and initiates an upload, the first FileUploadWorker will be triggered and getCurrentUploadsForAccountPageAscById() will return 101 uploads. Before the first upload completes, if the user then selects inner_folder and triggers another upload, the second FileUploadWorker will call getCurrentUploadsForAccountPageAscById() again. This time, it will return more than 101 uploads because the previous job has not yet finished. As a result, the returned upload count will be inaccurate, causing notifications to be displayed incorrectly.

Using an external counter to track the number of uploads does not resolve this issue as well, since the counter increments based on the inaccurate item count retrieved from the same function.

Changes

The startFilesUploadJob() method accepts a list of specific upload IDs. This approach ensures the worker always has the correct number of items to process. For scenarios where the exact upload count cannot be determined upfront, such as in FilesSyncWork or cancelAndRestartUploadJob(). We can continue to rely on getCurrentUploadsForAccountPageAscById() or if it's possible to write query for these cases we can add new function to the UploadsStorageManager then we can use it.

The following tests need to be done

• Auto upload
• Uploading a folder
• Uploading multiple folders consecutively
• Uploading multiple files and folders consecutively
• Retrying failed uploads
• Retrying canceled uploads

Please compare these changes against the master branch to identify which aspects are within the current scope and which are not.

Demo:

test_fuw.mp4

[alperozturk96]

The OCUpload class is a wrapper around the UploadEntity data class and it is redundant. A similar redundancy exists between OCFile and FileEntity. These entity data classes can be used directly and can have helper methods.

The getCurrentUploadsForAccountPageAscById and getCurrentUploadsForAccount functions currently uses the ContentResolver to obtain a cursor, which is then mapped to OCUpload objects an unnecessary step. Instead, current uploads can be fetched directly from the Room database via DAO methods.

As a result, the getCurrentUploadsForAccountPageAscById and getCurrentUploadsForAccount functions become obsolete and should no longer be used.

Demo

faf9.mp4

[alperozturk96]

Unused function

[tobiasKaminsky]

Do not passs uploadIs, but instead access data inside worker

[alperozturk96]

In all cases, uploadIds are not directly accessible via the account name. For example:

• FileUploadHelper.retryUploads receives a failedUploads: Array<OCUpload> parameter, where the failed uploads originate from UploadsStorageManager.
• FileUploadHelper.uploadNewFiles maps the localPaths parameter passed from eight different classes to create OCUpload instances. Therefore, this process cannot be handled directly within the worker.
• FileUploadHelper.cancelFileUploads takes a uploads: List<OCUpload> argument, which is provided by various classes, making it unsuitable for direct handling inside the worker.
• FileUploadHelper.uploadUpdatedFile uses the existingFiles parameter to generate OCUpload objects through mapping, which similarly cannot be managed directly within the worker.

[tobiasKaminsky]

Inside loop check if upload still is pending in uploadsStorageManager

[alperozturk96]

UPLOAD_IN_PROGRESS (to see checkpublic enum UploadStatus description) represents both scheduled and currently uploading files, which makes it unsuitable for filtering within the loop without unintentionally skipping valid uploads. When I tried using the following SQL command, I wasn’t able to upload any files because it represents both scheduled and in-progress uploads.

@Query(
        "SELECT EXISTS(" +
            "SELECT 1 FROM " + ProviderTableMeta.UPLOADS_TABLE_NAME +
            " WHERE " + ProviderTableMeta._ID + " = :id AND " +
            ProviderTableMeta.UPLOADS_ACCOUNT_NAME + " = :accountName AND " +
            ProviderTableMeta.UPLOADS_STATUS + " = :pendingStatus" +
            ")"
    )
    fun isPendingUpload(
        id: Int,
        accountName: String,
        pendingStatus: Int = UploadStatus.UPLOAD_IN_PROGRESS.value
    ): Boolean

Solutions:

Introducing a new status (e.g., UPLOAD_PROCESSING_NOW) would require significant changes to the existing codebase, affecting multiple classes.

In-memory tracking (storing a set of upload IDs currently being processed to prevent re-processing them in the same run) is also unreliable because if the worker restarts unexpectedly (for example, due to system resource constraints or OS killing background tasks), this in-memory state is lost, leading to possible duplicate uploads.

Based on current testing, the worker executes uploads sequentially without any issues. If a user tries to upload the same files, no conflict resolve notification appears, just like in the master branch. If the file name is the same, but the content has changed, the conflict resolve notification appears, consistent with the master branch behavior.

I suggest addressing this in a separate PR.

Demo:

Screen.Recording.2025-06-27.at.11.50.48.mp4

[tobiasKaminsky]

see inline

[alperozturk96]

For me it does not trac any progress, but only stays at 0%.

What is the image size? If image size is too small, it goes 0 to %100 too fast and you won't notice it. Also, this PR doesn't touch the progress percent it fixes the track file index e.g.

1 / 10 file.jpg
2 / 10 file2.jpg
..
..

Please compare with master.

Master

master.mp4

[github-actions]

APK file: https://www.kaminsky.me/nc-dev/android-artifacts/14822.apk



To test this change/fix you can simply download above APK file and install and test it in parallel to your existing Nextcloud app.

[github-actions]

Codacy

Lint

Type	master	PR
Warnings	48	48
Errors	11	11

SpotBugs

Category	Base	New
Bad practice	65	63
Correctness	62	62
Dodgy code	299	299
Experimental	1	1
Internationalization	7	7
Malicious code vulnerability	2	2
Multithreaded correctness	35	35
Performance	48	48
Security	18	18
Total	537	535

[tobiasKaminsky]

Ah, I was under the impression that percentage is about how many files, not file transfer of one file.
But you are right 👍

[stefan2904]

is this mean to remove uploads that are not for the currently active user? (see #15320)

[alperozturk96]

Hey, thanks for the message.

Get the account that wants to upload the file. If that user is not available, remove the upload that belongs to the same user from the queue, just like before.

What do you think? How does this change affect the issue you linked to, it is same as before?

Maybe we can find the root cause of the issue please share your thoughts, thanks.