Skip to content

fix(dav): finalize upload bookkeeping before downgrading lock#60453

Open
joshtrichards wants to merge 1 commit into
masterfrom
jtr/fix-dav-put-bookkeeping-consistency
Open

fix(dav): finalize upload bookkeeping before downgrading lock#60453
joshtrichards wants to merge 1 commit into
masterfrom
jtr/fix-dav-put-bookkeeping-consistency

Conversation

@joshtrichards
Copy link
Copy Markdown
Member

@joshtrichards joshtrichards commented May 15, 2026

  • Resolves: #

Summary

Refactor the final upload/publish phase in apps/dav/lib/Connector/Sabre/File.php into a dedicated finalizeUpload() helper and keep the exclusive lock until final bookkeeping is complete.

What changed

  • extract the post-move finalization logic into finalizeUpload()
  • keep the exclusive lock until after:
    • updater/cache reconciliation
    • mtime / creation time / upload time persistence
    • checksum persistence
    • info refresh
    • post-write hook emission
  • move checksum persistence before post hooks
  • reduce the number of intermediate observable states during final upload publication

Why

This upload path intentionally bypasses the normal view-layer publish path and then reconciles cache/bookkeeping state manually.

Previously, the code downgraded the lock to shared immediately after getUpdater()->update($internalPath), and only then applied additional metadata and checksum updates. That created a window where the file could be visible at the final path while parts of the visibility/bookkeeping state were still catching up.

I believe this is the probable culprit of transient failures, which show up in our CI runs in the FilesDrop integration tests like this:

{"reqId":"ExN63msWomhYw6LqRBee","level":3,"time":"2026-05-15T00:12:28+00:00","remoteAddr":"::1","user":"--","app":"no app in context","method":"PUT","url":"/public.php/dav/files/jHybAbajiZcXgfy/folder","scriptName":"/public.php","message":"Cannot set extra headers for non-existing file 'files/jHybAbajiZcXgfy/Alice/folder (2)'","userAgent":"GuzzleHttp/7","version":"34.0.0.6","data":[]}
{"reqId":"2pwd9QrZNEDnza5kbPnv","level":3,"time":"2026-05-15T00:12:28+00:00","remoteAddr":"::1","user":"user0","app":"webdav","method":"GET","url":"/remote.php/webdav/drop/Alice/folder/a.txt","scriptName":"/remote.php","message":"Could not open file: drop/Alice/folder/a.txt (214), file does seem to exist","userAgent":"GuzzleHttp/7","version":"34.0.0.6",[...]

Since it doesn't always fail (and re-running eventually leads to a it passing), it's not a path handling issue per se, but more a timing matter.

This refactor keeps the existing storage-level behavior but makes the finalization boundary more explicit and narrows that inconsistent window.

Cc: @icewind1991 because you last adjusted the logging scenarios like this in #56166 so may be related to some other work you were doing / problem you encountered.

Scope / non-goals

This change does not alter:

  • the part-file upload strategy
  • cross-storage final move behavior
  • the stream write path
  • pre-hook behavior

It only tightens ordering during the final publish/finalization phase.

Testing / review focus

Please review especially for:

  • lock ordering during the final publish step
  • checksum persistence ordering relative to post hooks
  • any assumptions around batching final metadata updates through putFileInfo()

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>
@joshtrichards
Copy link
Copy Markdown
Member Author

/backport to stable33

@joshtrichards joshtrichards added this to the Nextcloud 34 milestone May 15, 2026
@joshtrichards joshtrichards marked this pull request as ready for review May 15, 2026 14:13
@joshtrichards joshtrichards requested a review from a team as a code owner May 15, 2026 14:13
@joshtrichards joshtrichards requested review from ArtificialOwl, CarlSchwan, icewind1991 and leftybournes and removed request for a team May 15, 2026 14:13
@joshtrichards joshtrichards added 3. to review Waiting for reviews feature: filesystem and removed 2. developing Work in progress labels May 15, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant