feat(services/azblob): support if_match writes and honor preconditions on block commit - #7990
Open
shanielh wants to merge 1 commit into
Open
feat(services/azblob): support if_match writes and honor preconditions on block commit#7990shanielh wants to merge 1 commit into
shanielh wants to merge 1 commit into
Conversation
…s on block commit Azure Blob's Put Blob API supports If-Match with an arbitrary ETag, so wire it in alongside the existing If-None-Match support and declare write_with_if_match. Conditional args were only applied to the single-shot Put Blob request. Writing more than once before close() commits through Put Block + Put Block List instead, and that commit request carried no conditional headers, so if_match, if_none_match and if_not_exists were silently dropped and the write degraded to an unconditional overwrite. Put Block does not evaluate preconditions, so they are applied to Put Block List, which does. Add behavior tests that force the block path by writing twice before close(). They fail against Azurite on the previous code for if_none_match and if_not_exists, and pass with this change.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Which issue does this PR close?
Part of #7889.
This is the Azure Blob half of #7890, split out so it can land independently. The GCS half of #7890 is heading for an RFC discussion and is not included here; #7889 stays open for it.
Rationale for this change
Azure Blob's Put Blob API supports
If-Matchwith an arbitrary ETag, but OpenDAL only wired upIf-None-Match, sowrite_with_if_matchwas reported as unsupported even though the underlying API can do it. This blocks optimistic-concurrency writes ("only overwrite if the object hasn't changed since I read it") on azblob.While wiring that up, a related gap surfaced on the chunked write path. Conditional args were only applied to the single-shot Put Blob request. A caller that writes more than once before
close()— viawriter_with(..),chunk(..), or just twowrite()calls — commits through Put Block + Put Block List instead, andazblob_complete_put_block_list_requestcarried no conditional headers at all.if_match,if_none_matchandif_not_existswere silently dropped there, so a conditional write degraded to an unconditional overwrite with no error to signal that the precondition had been discarded.The
if_none_match/if_not_existspart of that is a pre-existing bug independent ofif_match: both capabilities are already declared for azblob and both work single-shot, soif_not_exists(true)through a chunked writer overwrote rather than failing. It is fixed here rather than separately because it is the same dropped-precondition bug in the same function, and fixing onlyif_matchwould leave the identical hole open two lines away. Happy to split it out if reviewers prefer.What changes are included in this PR?
if_matchon Put Blob: wire theIf-Matchheader intoazblob_put_blob_requestmirroring the existingif_none_matchhandling, and declarewrite_with_if_match.if_not_exists,if_none_matchandif_matchtoazblob_complete_put_block_list_request. Put Block does not evaluate preconditions, so they belong on Put Block List, which does. This matchesazblob_complete_copy_block_list_request, which already honorsif_not_exists.test_writer_write_with_if_not_exists,test_writer_write_with_if_none_match,test_writer_write_with_if_match) that force the multi-part commit path by writing twice beforeclose(). They are gated onwrite_can_multiplus the relevant conditional capability, so they exercise any service declaring those.parse_erroralready maps 412/409/304 toErrorKind::ConditionNotMatch.Capabilitydocs.Verified against Azurite:
test_writer_write_with_if_none_matchandtest_writer_write_with_if_not_existsfail (assertion failed: res.is_err()— the conditional write succeeded where it should have returnedConditionNotMatch).test_writer_write_with_if_matchearly-returns there since the capability isn't declared yet.test_write_with_if_match, which now runs on azblob for the first time.Also run:
cargo fmt --all -- --check,cargo clippy --workspace --all-targets --all-features -- -D warnings, andcargo test -p opendal-service-azblob.Are there any user-facing changes?
Yes, both additive:
if_matchtowrite_with/writer_with, matching what was already available forif_none_match.if_not_exists(true)orif_none_match(..)through a chunked writer and saw the write succeed will now correctly getErrorKind::ConditionNotMatchwhen the precondition does not hold. This is a bug fix, but it is a behavior change for anyone who was unknowingly relying on the precondition being dropped.No breaking changes to public APIs.
AI Usage Statement
This PR was developed with Claude Code (Anthropic), model Claude Opus 5: codebase investigation, implementation, and verification. Behavior tests were run against a local Azurite instance in Docker, both before and after the change; no live Azure account was used.