Skip to content

Conversation

zzhpro
Copy link
Contributor

@zzhpro zzhpro commented Aug 14, 2025

Type of Change

  • New Feature
  • Bug Fix
  • Documentation
  • Performance Improvement
  • Test/CI
  • Refactor
  • Other:

Related Issues

#406

Summary of Changes

When executing PutObject and CompleteMultipartUpload, if the If-None-Match or If-Match header is set, check the etag of the object before proceeding. The implementation strategy comes from MinIO.

I've done some tests with the following settings:

  • Single-node and multi-node setup
  • PutObject and CompleteMultipartUpload
  • Various combination of If-None-Match and If-Match values

Looks good for now.

Any further tests and comments are welcome!

Checklist

  • I have read and followed the CONTRIBUTING.md guidelines
  • Passed make pre-commit
  • Added/updated necessary tests
  • Documentation updated (if needed)
  • CI/CD passed (if applicable)

Impact

  • Breaking change (compatibility)
  • Requires doc/config/deployment update
  • Other impact:

Additional Notes


Thank you for your contribution! Please ensure your PR follows the community standards (CODE_OF_CONDUCT.md) and sign the CLA if this is your first contribution.

@zzhpro zzhpro marked this pull request as draft August 15, 2025 00:23
@zzhpro zzhpro marked this pull request as ready for review August 15, 2025 01:05
@guojidan guojidan self-requested a review August 15, 2025 03:10
@guojidan
Copy link
Contributor

I will review this PR in a few days.

@loverustfs
Copy link
Contributor

@guojidan If the code review is complete and all aspects of the functionality are ready, you can merge the feature.
Close the issue and PR.

@@ -3218,6 +3219,42 @@ impl SetDisks {
obj?;
Ok(())
}

async fn check_write_precondition(&self, bucket: &str, object: &str, mut opts: ObjectOptions) -> Option<StorageError> {
let http_preconditions = opts.http_preconditions.unwrap();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do not use unwarp()

@@ -3218,6 +3219,42 @@ impl SetDisks {
obj?;
Ok(())
}

async fn check_write_precondition(&self, bucket: &str, object: &str, mut opts: ObjectOptions) -> Option<StorageError> {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should use opts: &ObjectOptions, and later use let opts = opts.clone(), because we do not intend to modify the incoming opts itself.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your advice! I've fixed them

@zzhpro zzhpro requested a review from guojidan August 19, 2025 03:21
@guojidan
Copy link
Contributor

LGTM,

  1. you need to add test cases in the e2e crate to verify the integrity of this function.
  2. rebase onto the main branch

@zzhpro
Copy link
Contributor Author

zzhpro commented Aug 19, 2025

LGTM,

  1. you need to add test cases in the e2e crate to verify the integrity of this function.
  2. rebase onto the main branch

I'll add the tests sometime this week, kind of busy today~

@zzhpro
Copy link
Contributor Author

zzhpro commented Aug 22, 2025

I've added some e2e tests~

Copy link
Contributor

@guojidan guojidan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@guojidan guojidan merged commit c2d782b into rustfs:main Aug 26, 2025
12 checks passed
houseme added a commit that referenced this pull request Aug 26, 2025
* 'main' of github.com:rustfs/rustfs:
  feat: support conditional writes (#409)
houseme added a commit that referenced this pull request Aug 27, 2025
…audit

* 'main' of github.com:rustfs/rustfs:
  feat: support conditional writes (#409)
  Fix/addtier (#454)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants