Skip to content

fix(webdav): skip Nextcloud trashbin for backup cleanup - #1042

Open
arpitagarwal1301 wants to merge 2 commits into
seedvault-app:android17from
arpitagarwal1301:fix/1037-skip-nextcloud-trashbin
Open

fix(webdav): skip Nextcloud trashbin for backup cleanup#1042
arpitagarwal1301 wants to merge 2 commits into
seedvault-app:android17from
arpitagarwal1301:fix/1037-skip-nextcloud-trashbin

Conversation

@arpitagarwal1301

Copy link
Copy Markdown

Closes #1037

Summary

  • Send X-NC-Skip-Trashbin: true with WebDAV cleanup DELETE requests.
  • Cover both individual and whole-backup deletion paths with a focused unit test.

Validation

  • :core:testDebugUnitTest --tests '*WebDavBackendTest*'
  • :core:check :core:assemble :core:ktlintCheck

@grote grote left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the contribution. Did you actually test this against a supporting Nextcloud server?


val expected = mapOf("X-NC-Skip-Trashbin" to "true")
assertEquals(listOf(expected, expected), headers)
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems to mix the integration tests with a unit test. Best to make a new test class for it.

@arpitagarwal1301

Copy link
Copy Markdown
Author

I did not test this against a live Nextcloud instance because I do not currently have a supporting server available. I verified both cleanup paths pass X-NC-Skip-Trashbin: true to dav4jvm with the focused unit test, and I split that test into WebDavBackendUnitTest as requested. The existing credential-dependent WebDAV integration tests remain unchanged and skipped locally because the NEXTCLOUD_* variables are unavailable. If live-server verification is required, I will need a private test instance or a locally provisioned Nextcloud environment.

@tbrandirali

Copy link
Copy Markdown

@arpitagarwal1301 thank you so much for working on the feature I requested! I don't really have the time to build and test this branch myself right now, but I could probably spin up a test instance of Nextcloud for you to test it against. It might take me a few days to find the time, but if you don't have another option I'd be happy to help out.

@arpitagarwal1301

Copy link
Copy Markdown
Author

Thank you—that would be very helpful. Please don’t post credentials here. If you can prepare a temporary Nextcloud 31+ instance with a dedicated, empty test account, let me know and we can agree on a private handoff method. I’ll use it only to test this branch and confirm when the account can be deleted.

@grote

grote commented Jul 27, 2026

Copy link
Copy Markdown
Collaborator

@tbrandirali this would be very appreciated! I'd prefer merging this only after it has been actually verified in practice against a real server.

@tbrandirali

tbrandirali commented Jul 29, 2026

Copy link
Copy Markdown

After a bit more thought I think I'd rather run the test myself, spinning up a Nextcloud instance for someone else would be more work. @grote would running the integration tests against my live Nextcloud instance on a fresh account be sufficient for you, or would you require a full emulator backup test?

The integration tests do run DELETEs, so I can check that there are no leftovers in the trash bin after running them.

@grote

grote commented Jul 30, 2026

Copy link
Copy Markdown
Collaborator

@tbrandirali since seedvault needs to be a system app, getting a dev setup isn't trivial. I think creating a test account on your nextcloud instance should be way easier, but as you prefer. ideally, you run the integration tests against the server and also do a manual test.

@tbrandirali

Copy link
Copy Markdown

@arpitagarwal1301 I spun up a test instance for this and emailed you about exchanging credentials. @grote let me know if you would also want a test account.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Use 'X-NC-Skip-Trashbin' header to delete files without trashbin when cleaning up Nextcloud WebDAV backups

3 participants