fix(auto-upload): files skipped after re-enabling a folder or saving its settings by Blackclaws · Pull Request #17906 · nextcloud/android

Blackclaws · 2026-10-29T08:16:04Z

Issue

Turning an auto upload folder off, or deleting it, removed its upload entries but kept its filesystem rows, which stay marked as sent for upload. After the folder was turned on again, FileSystemRepository found no upload entry for those rows and treated them as finished. Every file the folder had seen was then skipped forever, even with "Also upload existing files".
Saving a folder's settings, or toggling its visibility, reset enabledTimestampMs to the current time. With "Also upload existing files" off, files created before the save that hadn't been scanned yet were then skipped.

Fixes

FileSystemDao.deleteBySyncedFolderId and FileUploadHelper.clearAutoUploadScanCache. A folder's filesystem entities are cleared right before it is enabled again, so its scan starts from scratch.
AutoUploadClickHandler runs the disable cleanup and re-enabling on one sequential dispatcher, so switching a folder off and on quickly cannot scan before the cleanup finished. Enabling or disabling a folder in its settings dialog now does the same as the list toggle.
AutoUploadDisplayItemFactory uses the stored enabled timestamp for list items, and updateAutoUploadFolderItem only starts a new timestamp when the folder is switched on.

Behaviour change

After a folder is turned off and on again, files that were pending while it was off are no longer uploaded, unless "Also upload existing files" is set. This matches what "enabled since" already meant for files the scan hadn't found yet. With the option set, every file is scanned again, and the folder's name collision policy decides what happens to files already on the server. This is the same as deleting and recreating the folder, which is the workaround users are given today.

I didn't change the ?: return true in FileSystemRepository.wasUploadFinished. Upload entries are also removed when users clear the uploads list, so requeueing files without an entry would upload everything again.

🏁 Checklist

⛑️ Tests (unit and/or integration) are included or not needed: AutoUploadDisplayItemFactoryTest
🔙 Backport requests are created or not needed: /backport to stable-xx.x
📅 Milestone is set
🌸 PR title is meaningful (if it should be in the changelog: is it meaningful to users?)

🤖 AI (if applicable)

The content of this PR was partly or fully generated using AI (Claude Code). I haven't built it locally; the unit tests run in CI.

Opened by Claude Code on behalf of @Blackclaws.

🤖 Generated with Claude Code

Disabling or deleting an auto upload folder removed its upload entries but kept its filesystem entities, which stay marked as sent for upload. When the folder is enabled again, FileSystemRepository finds no upload entry for them and treats them as finished, so every file the folder had seen is skipped forever, even with "also upload existing files" set. Remove the folder's filesystem entities together with its upload entries, so enabling the folder again starts from a fresh scan. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Felix Winterhalter <felix@audiofair.de>

Saving a folder's settings, and toggling its visibility, stored the current time as its enabled timestamp, because list items were built with the current time and the settings dialog always reset it. Files created before that save which were not scanned yet then count as existing files and are skipped when "also upload existing files" is off. Keep the stored timestamp and only start a new one when the folder is switched on. Also read the column as Long in SyncedFolderEntity, a millisecond timestamp does not fit into an Int. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Felix Winterhalter <felix@audiofair.de>

alperozturk96

Hello,

Thank you for the PR.

alperozturk96 · 2026-10-29T16:50:45Z

    val enabled: Int?,
    @ColumnInfo(name = ProviderTableMeta.SYNCED_FOLDER_ENABLED_TIMESTAMP_MS)
-    val enabledTimestampMs: Int?,
+    val enabledTimestampMs: Long?,


If DB change is not necessary for the fix, please don't do it in the same PR.

Reverted in 6758178. It is not needed for the fix, the only reader of that field just checks for null.

Claude Code on behalf of @Blackclaws

alperozturk96 · 2026-10-29T16:54:33Z

     * on its next scan via FileSystemRepository.getFilePathsWithIds.
+     *
+     * The folder's filesystem entities are removed as well. They are marked as sent for upload, and once their
+     * upload entries are gone FileSystemRepository treats them as finished, so every file the folder had seen


In where exactly if upload entries (UploadEntity) are gone FileSystemRepository treats them as finished?

In FileSystemRepository.insertOrReplace, for a file whose entity is unchanged and marked as sent (lines 209-213), wasUploadFinished looks up the upload entity and returns true when there is none:

// FileSystemRepository.kt:253 val upload = getUpload(entity, syncedFolder, localPath, file) ?: return true

So the file is skipped as "hasn't changed since last scan". Switching a folder off calls removeEntityFromUploadEntities, which deletes exactly those upload entities but keeps the filesystem entities marked as sent. After the folder is switched on again, every file it had seen hits that return true and is never queued again, even with "also upload existing files" (that check in shouldSkipFile only matters for files without an entity), and even if the file is not on the server.

I left ?: return true as it is, because upload entities are also removed when users clear the uploads list, and requeueing then would upload everything again. Instead the folder's filesystem entities are now cleared right before it is enabled again (6758178).

Claude Code on behalf of @Blackclaws

Copilot

🟡 Changes recommended

Cleanup can race with re-enabling and delete newly processed upload records, risking duplicate uploads, while disabling through settings bypasses cleanup and leaves previously handled files incorrectly skipped.

2 open issues

What changed in this PR

This PR addresses #17905 by clearing stale auto-upload tracking records and preserving folder enable timestamps, preventing files from being skipped after re-enabling a folder or saving its settings.

Changes:

Delete a folder’s filesystem records alongside its upload entries during cleanup.
Preserve enabled timestamps across settings saves and store them as Long?.
Add regression tests for timestamp preservation and enable/disable transitions.

File	Description
`app/src/test/java/com/owncloud/android/ui/fragment/autoUpload/data/AutoUploadDisplayItemFactoryTest.kt`	Tests timestamp preservation and state transitions.
`app/src/main/java/com/owncloud/android/ui/fragment/autoUpload/data/AutoUploadDisplayItemFactory.kt`	Preserves stored timestamps unless enabled state changes.
`app/src/main/java/com/nextcloud/client/jobs/upload/FileUploadHelper.kt`	Clears filesystem records during folder cleanup.
`app/src/main/java/com/nextcloud/client/database/entity/SyncedFolderEntity.kt`	Uses `Long?` for millisecond timestamps.
`app/src/main/java/com/nextcloud/client/database/dao/FileSystemDao.kt`	Adds filesystem deletion scoped to a synced folder.

_{Review effort: Balanced}

Blackclaws · 2026-10-29T19:31:40Z

                    remotePath = it.remotePath!!
                )
            }
+        fileSystemDao.deleteBySyncedFolderId(id.toString())


Fixed in 6758178. The filesystem entities are no longer deleted by the disable cleanup. They are cleared right before the folder is enabled and its scan starts, in the same coroutine. Disable cleanup and re-enabling both run on one Dispatchers.IO.limitedParallelism(1) dispatcher in AutoUploadClickHandler, in the order they were requested, so a quick off/on cannot scan before the cleanup finished or delete rows of the new scan.

Claude Code on behalf of @Blackclaws

Blackclaws · 2026-10-29T19:31:42Z

+            if (syncedFolder.isEnabled != isEnabled) {
+                setEnabled(syncedFolder.isEnabled, clock.currentTime)
+            }


Fixed in 6758178. updateExistingFolder now does the same as the list toggle: switching a folder off in its settings runs the upload entity cleanup, and switching it on clears the scan cache before saving and starting the scan. I have not added a test for the handler path, it needs the fragment and adapter; the factory part is covered by AutoUploadDisplayItemFactoryTest.

Claude Code on behalf of @Blackclaws

Review feedback: - Clear the folder's filesystem entities right before it is enabled again and its scan starts, instead of at the end of the disable cleanup. Cleanup and re-enabling run on one sequential dispatcher, so switching a folder off and on quickly cannot scan before the cleanup finished or have the fresh scan removed by it. - Enabling and disabling a folder in its settings dialog now does the same as the toggle on the list. - Revert the SyncedFolderEntity type change, it is not needed for the fix. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Felix Winterhalter <felix@audiofair.de>

alperozturk96 · 2026-10-30T02:49:11Z

    }

+    /**
+     * Forgets which files of a synced folder were already seen, so its next scan starts from scratch.


Please remove comments. This is too much and unncessary.

Removed in e209985.

Claude Code on behalf of @Blackclaws

alperozturk96 · 2026-10-30T02:49:23Z

            uploadAction = syncedFolder.uploadAction
            setNameCollisionPolicy(syncedFolder.nameCollisionPolicy.serialize())
-            setEnabled(syncedFolder.isEnabled, clock.currentTime)
+            // only a folder being switched on starts a new "enabled since" period, files from before it count as


Removed in e209985.

Claude Code on behalf of @Blackclaws

alperozturk96 · 2026-10-30T02:50:37Z


+    // runs cleanup and re-enabling of folders in the order they were requested, so a folder switched off and on
+    // quickly cannot be scanned before its cleanup finished or have its fresh scan removed by that cleanup
+    private val folderStateDispatcher = Dispatchers.IO.limitedParallelism(1)


Why do we need to limit parallelism to 1? Also how come click handler can be triggered from multiple threads at the same time?

Additionally, please remove the comment. It makes hard to follow code.

Removed the comment in e209985.

The handler is only called on the main thread. The concern is the coroutines it launches: every click starts one on Dispatchers.IO, a thread pool, so the cleanup launched by "off" and the clear-and-start launched by a quick "on" right after can run at the same time or in reverse order. If "on" runs first, it clears the filesystem entities and starts the worker, and the "off" cleanup then finds no entities, so it leaves the upload entities behind. A leftover UPLOADED entity with the skip policy makes the worker treat the file as uploaded again, which brings back the bug this PR fixes. If the cleanup instead runs while the worker is uploading, it deletes the upload entities the worker just created.

limitedParallelism(1) runs these coroutines one at a time, in the order of the clicks. The window is small, since it needs off and on within the time the cleanup takes. If you consider that negligible, I can drop the dispatcher and use Dispatchers.IO as before.

Claude Code on behalf of @Blackclaws

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Felix Winterhalter <felix@audiofair.de>

Blackclaws and others added 2 commits October 29, 2026 09:15

alperozturk96 requested review from alperozturk96 and a balanced review from Copilot October 29, 2026 16:45

alperozturk96 requested changes Oct 29, 2026

View reviewed changes

Copilot started reviewing on behalf of alperozturk96 October 29, 2026 17:00

View session

Copilot AI reviewed Oct 29, 2026

View reviewed changes

alperozturk96 requested changes Oct 30, 2026

View reviewed changes

chore(auto-upload): remove comments

e209985

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Felix Winterhalter <felix@audiofair.de>

Conversation

Blackclaws commented Oct 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Issue

Fixes

Behaviour change

🏁 Checklist

🤖 AI (if applicable)

Uh oh!

alperozturk96 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Blackclaws commented Oct 29, 2026 •

edited

Loading