Skip to content

feat(services/webdav): support rename with if_not_exists - #8089

Open
PDGGK wants to merge 1 commit into
apache:mainfrom
PDGGK:feat-webdav-rename-if-not-exists
Open

feat(services/webdav): support rename with if_not_exists#8089
PDGGK wants to merge 1 commit into
apache:mainfrom
PDGGK:feat-webdav-rename-if-not-exists

Conversation

@PDGGK

@PDGGK PDGGK commented Aug 15, 2026

Copy link
Copy Markdown
Contributor

Which issue does this PR close?

Part of #7828 — the webdav box of RFC-7818. Announced the intended shape there first; this is that.

Rationale for this change

The RFC calls webdav a "Strong candidate: RFC 4918 defines Overwrite: F and a failed precondition", and §10.6 is explicit — F means the server must not perform the MOVE if the destination maps to a resource.

webdav_move hardcoded the header, so OpRename was ignored entirely — the backend took it as _args:

req = req.header(HEADER_OVERWRITE, "T");

It now threads the flag through and sends F when if_not_exists is set.

Nothing was needed on the error side. RFC 4918 §9.9.4 has the server answer 412 when the precondition fails, and webdav already maps it:

// core.rs:1647
StatusCode::PRECONDITION_FAILED | StatusCode::NOT_MODIFIED => (ErrorKind::ConditionNotMatch, false)

which is exactly what test_rename_with_if_not_exists_returns_condition_not_match asserts.

On the capability, since #7828 asks

Plain rename_with_if_not_exists: true, no config flag. Overwrite is not an optional extension — RFC 4918 §10.6 makes it part of MOVE and a server that ignores it is non-conforming. That is unlike conditional read, where servers genuinely differ and enable_conditional_read earns its keep. Happy to move it behind a flag if you would rather have consistency with #7637.

On the mkcol

webdav_move creates the destination's parent before issuing the MOVE, and that stays: test_rename_with_if_not_exists_nested renames onto a three-level path that does not exist yet, so the parent creation is what makes it pass. The consequence is that a 412 can leave an empty parent behind. The destination itself is untouched, so it does not violate the RFC — flagging it rather than leaving you to find it.

Verification

Against the nginx fixture from .github/services/webdav, not by reading:

the three if_not_exists behaviour tests 3 passed
all rename behaviour tests 10 passed
the whole webdav behaviour suite 124 passed, 0 failed

test_rename_overwrite is in that set, so the default path still sends T.

And the control that makes those numbers mean something — hardcoding T back while keeping the capability bit:

test behavior::test_rename_with_if_not_exists_returns_condition_not_match ... FAILED
test result: FAILED. 9 passed; 1 failed

Exactly one test, the one asserting ConditionNotMatch. The header rather than the bit is doing the work.

cargo clippy -p opendal-service-webdav --all-targets (zero warnings) and cargo fmt --all -- --check are clean.

Scope

Only webdav. hdfs_native can follow — also docker-verifiable, no secrets.

I would leave azfile and azdls alone for now: their CI fixtures load credentials through 1Password/load-secrets-action, so a fork PR gets no behaviour coverage on them, and I would rather not advertise an atomicity-gated capability on a code reading alone. Happy to write them if someone who can run that CI wants to take the verification.

Are there any user-facing changes?

Yes — op.rename_with(from, to).if_not_exists(true) now works on webdav, returning ErrorKind::ConditionNotMatch when the destination exists. Ordinary rename is unchanged.

Implements the webdav box of apache#7828 (RFC-7818), which the RFC calls a
strong candidate: RFC 4918 defines Overwrite for MOVE, and section 10.6
makes "F" mean the server must not perform the move if the destination
maps to a resource.

webdav_move hardcoded Overwrite: T, so OpRename was ignored -- the
backend took it as _args. It now threads the flag through and sends F
when if_not_exists is set.

Nothing was needed on the error side. RFC 4918 section 9.9.4 has the
server answer 412 when the precondition fails, and webdav already maps
PRECONDITION_FAILED to ErrorKind::ConditionNotMatch, which is what the
behavior test expects.

The mkcol on the destination's parent stays. test_rename_with_if_not_
exists_nested renames onto a three-level path that does not exist yet,
so the parent creation is what makes it pass. The consequence is that a
412 can leave an empty parent behind; the destination itself is
untouched, so it does not violate the RFC.

Verified against the nginx fixture from .github/services/webdav rather
than by reading:

  all three if_not_exists behavior tests           pass
  all 10 rename behavior tests                     pass
  the whole webdav behavior suite, 124 tests       pass

test_rename_overwrite is in that set, so the default path still sends T.
Hardcoding T back while keeping the capability bit fails exactly one
test -- the one asserting ConditionNotMatch -- and leaves the other nine
green, which is what tells you the header rather than the bit is doing
the work.

Only webdav here. hdfs_native can follow, also docker-verifiable.
azfile and azdls need 1Password secrets for their CI, so a fork PR gets
no behavior coverage on them and I would rather not advertise an
atomicity-gated capability on a code reading alone.
@PDGGK
PDGGK requested a review from Xuanwo as a code owner August 15, 2026 08:55
@dosubot dosubot Bot added size:S This PR changes 10-29 lines, ignoring generated files. releases-note/feat The PR implements a new feature or has a title that begins with "feat" labels Aug 15, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

releases-note/feat The PR implements a new feature or has a title that begins with "feat" size:S This PR changes 10-29 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant