Skip to content

refactor(dav): improve readability of Sabre File::put upload flow#60974

Draft
joshtrichards wants to merge 6 commits into
masterfrom
jtr/refactor-Sabre-File-put
Draft

refactor(dav): improve readability of Sabre File::put upload flow#60974
joshtrichards wants to merge 6 commits into
masterfrom
jtr/refactor-Sabre-File-put

Conversation

@joshtrichards
Copy link
Copy Markdown
Member

@joshtrichards joshtrichards commented Jun 3, 2026

  • Resolves: #

Summary

This is a fairly mechanical refactor of OCA\DAV\Connector\Sabre\File::put() to improve readability and maintainability without intentionally changing behavior.

Why

put() is really long and handles several responsibilities at once:

  • direct vs part-file uploads
  • locking
  • stream writing
  • size validation
  • rename/finalization
  • metadata updates
  • hook emission
  • exception translation

This change keeps the existing flow but makes the method easier to follow by turning it into a higher-level orchestration method.

What changed

  • extracted repeated exclusive-lock acquisition into a helper
  • extracted request-body normalization into a helper
  • extracted requested hash wrapping into a helper
  • extracted content-length parsing and written-size validation into helpers
  • extracted upload target resolution into resolveUploadTarget()
  • extracted part-file finalization into a helper
  • extracted upload metadata / checksum handling into a helper
  • clarified local variable names in put()
  • cleaned up and added comments to better explain control flow and intent

Notes

  • no intentional behavior changes
  • checksum handling is preserved as-is
  • hook timing is preserved:
    • direct writes emit pre-hooks before writing the final target
    • part-file uploads emit pre-hooks before the final rename

Testing

  • no new tests added
  • intended as a readability/maintainability refactor only

Checklist

AI (if applicable)

  • The content of this PR was partly or fully generated using AI

Some low-risk quick wins:

- clearer local variable names
- section comments
- extracted repeated lock-upgrade logic
- extracted stream normalization
- extracted hash wrapping
- extracted expected-size parsing

Signed-off-by: Josh <josh.t.richards@gmail.com>
…ut()

- put() is shorter and more skimmable
- write mechanics are isolated in writePutDataToStorage()
- size validation is named explicitly
- metadata/header handling is isolated in applyUploadMetadata()

Signed-off-by: Josh <josh.t.richards@gmail.com>
… File::put()

Signed-off-by: Josh <josh.t.richards@gmail.com>
Signed-off-by: Josh <josh.t.richards@gmail.com>
Signed-off-by: Josh <josh.t.richards@gmail.com>
@joshtrichards joshtrichards added this to the Nextcloud 35 milestone Jun 3, 2026
@joshtrichards joshtrichards added 2. developing Work in progress feature: dav ♻️ refactor Refactor code (not a bug fix, not a feature just refactoring) labels Jun 3, 2026
Signed-off-by: Josh <josh.t.richards@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

2. developing Work in progress feature: dav ♻️ refactor Refactor code (not a bug fix, not a feature just refactoring)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant