Skip to content

PR #85 extended / v.1.2.4#191

Open
guruz wants to merge 14 commits into
mainfrom
pr-185-extended
Open

PR #85 extended / v.1.2.4#191
guruz wants to merge 14 commits into
mainfrom
pr-185-extended

Conversation

@guruz

@guruz guruz commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

FYI @zerox80

@guruz

guruz commented Jun 10, 2026

Copy link
Copy Markdown
Contributor Author

(This is still WiP, will put something on top still)

@zerox80

zerox80 commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

has conflicts...

@guruz guruz changed the title PR #85 extended PR #85 extended / v.1.2.4 Jun 12, 2026
@zerox80

zerox80 commented Jun 13, 2026

Copy link
Copy Markdown
Contributor

I checked this against the OpenCloud backend implementation as well.

A couple of retry/resume edge cases look worth fixing before merge:

  1. UploadFileFromContentUriWorker.ensureOriginalTusChecksum() reopens the original contentUri when a valid cached file already exists. For old/pre-checksum retries, the original SAF URI may be gone or permission-revoked while cachePath is still valid. In that case we should hash the cached file instead of failing with LocalFileNotFoundException.

  2. In both upload workers, shouldTryTus is computed before clearing an old pending TUS state without SHA1. After clearTusState(), the stale shouldTryTus == true can still route the upload through a fresh TUS creation, even for small files or when capabilities no longer advertise TUS. Recomputing after clearing, or moving the clear before the decision, should avoid that.

Backend note: I initially wondered whether Upload-Checksum should be gated on tus_support.extension, but OpenCloud’s OCS capability currently advertises only creation,creation-with-upload while the actual TUS endpoint supports checksum. So sending the checksum as this PR does seems correct for OpenCloud.

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.

2 participants