Skip to content

refactor(dav): make chunk target resolution more explicit#60941

Draft
joshtrichards wants to merge 7 commits into
masterfrom
jtr/refactor-ChunkingV2-upload-resolution
Draft

refactor(dav): make chunk target resolution more explicit#60941
joshtrichards wants to merge 7 commits into
masterfrom
jtr/refactor-ChunkingV2-upload-resolution

Conversation

@joshtrichards
Copy link
Copy Markdown
Member

@joshtrichards joshtrichards commented Jun 2, 2026

  • Resolves: #

Summary

This refactor makes chunk upload target resolution more explicit by replacing the old file-only path with a richer resolver that returns:

  • the target file
  • the storage to use
  • the internal storage path
  • whether the upload is direct-to-target or staged

It also improves comments/docblocks around the upload lifecycle.

It is a focused internal refactor rather than a behavior change.

Core Changes

The main improvement is the new resolveChunkWriteTarget() flow:

  • eliminates redundant calls (e.g. the former getUploadFile() followed immediately by getUploadStorage() which also then called getUploadFile()` again)
  • the returned isDirect flag is then used later to decide whether finalization needs copy/move
  • Direct writes are still allowed only when the destination file exists and is on the same storage as the upload folder (otherwise uploads go through the .target staging file), but the framework is there to easily expand the "direct-to-target" write scenarios supported.

The class’s actual unit of work is not just “file,” but “write target + storage + path + mode.”

Other Changes

  • Resolver decomposition to improve readability and make the staging/direct distinction explicit.
  • isDirect - from the centralized resolver - is used explicitly rather than finalization code inferring mode indirectly from path comparison.
  • Clearer/consistent variable naming - i.e. $uploadFile only for UploadFiles

TODO

  • Fix the spot in completeChunkedUpload() where I missed using the decompose
  • Try to eliminate FIXME regarding File/etc return typing before merger
  • Try to eliminate FIXME regarding public/private function declaration before merger

Checklist

AI (if applicable)

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

Signed-off-by: Josh <josh.t.richards@gmail.com>
Introduces a helper that replaces getUploadFile + getUploadStorage and resolves everything needed for chunk writing in one pass.

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>
…metadata

Signed-off-by: Josh <josh.t.richards@gmail.com>
@joshtrichards joshtrichards added 2. developing Work in progress feature: dav ♻️ refactor Refactor code (not a bug fix, not a feature just refactoring) labels Jun 2, 2026
@joshtrichards joshtrichards requested a review from juliusknorr June 2, 2026 17:35
@joshtrichards joshtrichards added this to the Nextcloud 35 milestone Jun 2, 2026
… actual UploadFile

Signed-off-by: Josh <josh.t.richards@gmail.com>
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