fix(uploads): converge staged cleanup and docstrings with #5756 (#5763)

This commit is contained in:
Shxiao 2026-09-25 17:33:41 +09:00 committed by GitHub
parent c814943215
commit 39351315bf
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 22 additions and 4 deletions

View File

@ -261,7 +261,8 @@ def _link_staged_no_overwrite(
place, for a caller that still holds a descriptor on the staged inode and
therefore must remove it itself. Windows refuses to remove a file that
has an open handle, so the removal cannot happen here in that case; the
caller retains ownership of the staged path on both success and failure.
caller owns the staged path from the moment this returns — including on
the failure arms below, which leave it for that caller's own cleanup.
"""
file_path = _pure_destination(uploads_dir, display_filename)
try:
@ -294,9 +295,10 @@ def _commit_upload_temp_no_overwrite(
Same no-overwrite contract as :func:`_link_staged_no_overwrite`:
:class:`FileExistsError` leaves the staged part in place for a
next-suffix retry (the handle's second ``close`` is idempotent); any
other failure removes it best-effort. With ``unlink_staged=False``, the
caller retains the staged name on success and failure until it releases
its descriptor on the inode.
other failure removes the staged name only when this call owns it.
``unlink_staged=False`` leaves the name for the caller on every exit,
including the failure arms: that caller still holds a descriptor on the
inode and removes it itself (``_abort_upload_temp`` on its error path).
"""
upload_temp.handle.close()
return _link_staged_no_overwrite(upload_temp.temp_path, uploads_dir, display_filename, unlink_staged=unlink_staged)

View File

@ -542,6 +542,22 @@ def test_link_staged_preserves_original_error_when_cleanup_fails(tmp_path):
assert not (tmp_path / "report.pdf").exists()
def test_link_staged_failure_leaves_staged_file_for_caller_when_not_owning(tmp_path):
staged_path = tmp_path / "upload.part"
staged_path.write_bytes(b"upload bytes")
original_error = OSError(errno.EIO, "original link failure")
with (
patch.object(os, "link", side_effect=original_error),
patch.object(uploads, "_remove_staged_file") as remove_mock,
pytest.raises(OSError) as exc_info,
):
uploads._link_staged_no_overwrite(staged_path, tmp_path, "report.pdf", unlink_staged=False)
assert exc_info.value is original_error
remove_mock.assert_not_called()
assert staged_path.read_bytes() == b"upload bytes"
def test_upload_files_does_not_adjust_permissions_for_local_sandbox(tmp_path):
thread_uploads_dir = tmp_path / "uploads"
thread_uploads_dir.mkdir(parents=True)