fix(auto-upload): handle existing sync conflicts

[alperozturk96]

• Tests written, or not not needed

Issue: #16126 (comment)

When a sync conflict occurs during auto-upload, the client repeatedly retries uploading the same file. This results in the user seeing the same conflict notification multiple times, even though the conflict already exists and requires manual resolution.

Changes

After the first auto-upload attempt that results in a sync conflict, the file is marked as handled by the auto-upload system. This prevents further automatic retries for the same file. The conflicted upload remains visible in the Uploads list, where the user can manually resolve the conflict or retry the upload if desired.

[alperozturk96]

/backport to stable-3.35

[ZetaTom]

If I understand correctly, this means that any files scheduled for Auto Upload which failed to upload due to a conflict, will not be rescheduled.

This improves usability, as users will not receive repeated notifications or prompts to manually resolve conflicts for Auto Upload.

However, it also means that no further attempts will be made once an upload conflict is detected. Conflicts may also be resolved on the server, e.g. by renaming or deleting the file in conflict.

We need to ask ourselves, if this is the right approach. Another option would be to silently retry the upload without telling the user in case it fails again. In either case, the original notification or entry in the failed uploads list would still remain.

If we decide that failed uploads should not be reattempted, this would be an acceptable solution. I've left a comment suggesting a minor improvement.

[ZetaTom]

Returning null in this case means that a prior upload failed due to a conflict. In my opinion we shouldn't introduce overall nullability to mean one specific thing. I would prefer if either the OCUpload becomes nullable, or if we return UploadEntity as is and check for uploadEntity?.lastResult in the calling function instead. I think that this would improve readability in the long term.

[alperozturk96]

I agree, and...

The createEntityAndUpload function should either create both the entity and the upload together or not create them at all. We should avoid partially successful executions; it should either succeed completely or fail entirely.

One approach would be to only pass items that are eligible for mapping, ensuring the function always returns valid values.

Alternatively, we could use a sealed class or a kotlin.Result wrapper so the function would never return null.

The current approach is okay, and this logic is only used inside the worker; it is not a general function, so it’s fine to keep it as is for now. In the future, we can improve this as needed, for example, if another condition prevents mapping and we have to handle it differently in the caller function.

[alperozturk96]

If I understand correctly, this means that any files scheduled for Auto Upload which failed to upload due to a conflict, will not be rescheduled.

This improves usability, as users will not receive repeated notifications or prompts to manually resolve conflicts for Auto Upload.

However, it also means that no further attempts will be made once an upload conflict is detected. Conflicts may also be resolved on the server, e.g. by renaming or deleting the file in conflict.

We need to ask ourselves, if this is the right approach. Another option would be to silently retry the upload without telling the user in case it fails again. In either case, the original notification or entry in the failed uploads list would still remain.

If we decide that failed uploads should not be reattempted, this would be an acceptable solution. I've left a comment suggesting a minor improvement.

“Conflicts may also be resolved on the server.” IMO, this scenario is not common. If a sync conflict is not resolved, retrying the upload will only cause unnecessary load. The app will silently hide it and keep retrying in the background. For each attempt on that specific folder, the app will try to upload a file that already has a conflict, which is very unlikely to be resolved automatically (due to changes on remote).

Since users can manage these from the Upload List screen, this PR's approach makes more sense and is the right one to take. @tobiasKaminsky what do you think?

[github-actions]

blue-Light-Screenshot test failed, but no output was generated. Maybe a preliminary stage failed.

[github-actions]

Codacy

SpotBugs

Category	Base	New
Bad practice	43	43
Correctness	74	74
Dodgy code	257	257
Experimental	1	1
Internationalization	7	7
Malicious code vulnerability	3	3
Multithreaded correctness	34	34
Performance	44	44
Security	18	18
Total	481	481

[github-actions]

APK file: https://www.kaminsky.me/nc-dev/android-artifacts/16273.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.

[tobiasKaminsky]

Yes, no re-schedule.
We wait for any user interaction.

(If people then complain that they forgot to resolve the conflict, we could add a recurring notification like "you have pending conflicts in your upload queue". But this is out of scope for that PR.

So fine from my side ✔