chore: enable conditional put by default for S3 (#7181)

* fix cargo clippy

* add Disabled variant to S3ConditionalPut

* restore object_store.yml

Author: meteorgan

object_store/src/aws/builder.rs

@@ -160,7 +160,7 @@ pub struct AmazonS3Builder {
     /// Copy if not exists
     copy_if_not_exists: Option<ConfigValue<S3CopyIfNotExists>>,
     /// Put precondition
-    conditional_put: Option<ConfigValue<S3ConditionalPut>>,
+    conditional_put: ConfigValue<S3ConditionalPut>,
     /// Ignore tags
     disable_tagging: ConfigValue<bool>,
     /// Encryption (See [`S3EncryptionConfigKey`])
@@ -525,7 +525,7 @@ impl AmazonS3Builder {
                 self.copy_if_not_exists = Some(ConfigValue::Deferred(value.into()))
             }
             AmazonS3ConfigKey::ConditionalPut => {
-                self.conditional_put = Some(ConfigValue::Deferred(value.into()))
+                self.conditional_put = ConfigValue::Deferred(value.into())
             }
             AmazonS3ConfigKey::RequestPayer => {
                 self.request_payer = ConfigValue::Deferred(value.into())
@@ -583,9 +583,7 @@ impl AmazonS3Builder {
             AmazonS3ConfigKey::CopyIfNotExists => {
                 self.copy_if_not_exists.as_ref().map(ToString::to_string)
             }
-            AmazonS3ConfigKey::ConditionalPut => {
-                self.conditional_put.as_ref().map(ToString::to_string)
-            }
+            AmazonS3ConfigKey::ConditionalPut => Some(self.conditional_put.to_string()),
             AmazonS3ConfigKey::DisableTagging => Some(self.disable_tagging.to_string()),
             AmazonS3ConfigKey::RequestPayer => Some(self.request_payer.to_string()),
             AmazonS3ConfigKey::Encryption(key) => match key {
@@ -827,9 +825,10 @@ impl AmazonS3Builder {
         self
     }
 
-    /// Configure how to provide conditional put operations
+    /// Configure how to provide conditional put operations.
+    /// if not set, the default value will be `S3ConditionalPut::ETagMatch`
     pub fn with_conditional_put(mut self, config: S3ConditionalPut) -> Self {
-        self.conditional_put = Some(config.into());
+        self.conditional_put = config.into();
         self
     }
 
@@ -905,7 +904,6 @@ impl AmazonS3Builder {
         let region = self.region.unwrap_or_else(|| "us-east-1".to_string());
         let checksum = self.checksum_algorithm.map(|x| x.get()).transpose()?;
         let copy_if_not_exists = self.copy_if_not_exists.map(|x| x.get()).transpose()?;
-        let put_precondition = self.conditional_put.map(|x| x.get()).transpose()?;
 
         let credentials = if let Some(credentials) = self.credentials {
             credentials
@@ -1045,7 +1043,7 @@ impl AmazonS3Builder {
             disable_tagging: self.disable_tagging.get()?,
             checksum,
             copy_if_not_exists,
-            conditional_put: put_precondition,
+            conditional_put: self.conditional_put.get()?,
             encryption_headers,
             request_payer: self.request_payer.get()?,
         };

object_store/src/aws/client.rs

@@ -204,7 +204,7 @@ pub(crate) struct S3Config {
     pub disable_tagging: bool,
     pub checksum: Option<Checksum>,
     pub copy_if_not_exists: Option<S3CopyIfNotExists>,
-    pub conditional_put: Option<S3ConditionalPut>,
+    pub conditional_put: S3ConditionalPut,
     pub request_payer: bool,
     pub(super) encryption_headers: S3EncryptionHeaders,
 }

object_store/src/aws/mod.rs

@@ -169,8 +169,8 @@ impl ObjectStore for AmazonS3 {
 
         match (opts.mode, &self.client.config.conditional_put) {
             (PutMode::Overwrite, _) => request.idempotent(true).do_put().await,
-            (PutMode::Create | PutMode::Update(_), None) => Err(Error::NotImplemented),
-            (PutMode::Create, Some(S3ConditionalPut::ETagMatch)) => {
+            (PutMode::Create, S3ConditionalPut::Disabled) => Err(Error::NotImplemented),
+            (PutMode::Create, S3ConditionalPut::ETagMatch) => {
                 match request.header(&IF_NONE_MATCH, "*").do_put().await {
                     // Technically If-None-Match should return NotModified but some stores,
                     // such as R2, instead return PreconditionFailed
@@ -184,11 +184,11 @@ impl ObjectStore for AmazonS3 {
                     r => r,
                 }
             }
-            (PutMode::Create, Some(S3ConditionalPut::Dynamo(d))) => {
+            (PutMode::Create, S3ConditionalPut::Dynamo(d)) => {
                 d.conditional_op(&self.client, location, None, move || request.do_put())
                     .await
             }
-            (PutMode::Update(v), Some(put)) => {
+            (PutMode::Update(v), put) => {
                 let etag = v.e_tag.ok_or_else(|| Error::Generic {
                     store: STORE,
                     source: "ETag required for conditional put".to_string().into(),
@@ -221,6 +221,7 @@ impl ObjectStore for AmazonS3 {
                         })
                         .await
                     }
+                    S3ConditionalPut::Disabled => Err(Error::NotImplemented),
                 }
             }
         }
@@ -561,7 +562,7 @@ mod tests {
         let integration = config.build().unwrap();
         let config = &integration.client.config;
         let test_not_exists = config.copy_if_not_exists.is_some();
-        let test_conditional_put = config.conditional_put.is_some();
+        let test_conditional_put = config.conditional_put != S3ConditionalPut::Disabled;
 
         put_get_delete_list(&integration).await;
         get_opts(&integration).await;

object_store/src/aws/precondition.rs

@@ -126,7 +126,7 @@ impl Parse for S3CopyIfNotExists {
 /// Configure how to provide conditional put support for [`AmazonS3`].
 ///
 /// [`AmazonS3`]: super::AmazonS3
-#[derive(Debug, Clone, Eq, PartialEq)]
+#[derive(Debug, Clone, Eq, PartialEq, Default)]
 #[allow(missing_copy_implementations)]
 #[non_exhaustive]
 pub enum S3ConditionalPut {
@@ -136,6 +136,7 @@ pub enum S3ConditionalPut {
     /// Encoded as `etag` ignoring whitespace
     ///
     /// [HTTP precondition]: https://datatracker.ietf.org/doc/html/rfc9110#name-preconditions
+    #[default]
     ETagMatch,
 
     /// The name of a DynamoDB table to use for coordination
@@ -147,13 +148,17 @@ pub enum S3ConditionalPut {
     ///
     /// This will use the same region, credentials and endpoint as configured for S3
     Dynamo(DynamoCommit),
+
+    /// Disable `conditional put`
+    Disabled,
 }
 
 impl std::fmt::Display for S3ConditionalPut {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
         match self {
             Self::ETagMatch => write!(f, "etag"),
             Self::Dynamo(lock) => write!(f, "dynamo: {}", lock.table_name()),
+            Self::Disabled => write!(f, "disabled"),
         }
     }
 }
@@ -162,6 +167,7 @@ impl S3ConditionalPut {
     fn from_str(s: &str) -> Option<Self> {
         match s.trim() {
             "etag" => Some(Self::ETagMatch),
+            "disabled" => Some(Self::Disabled),
             trimmed => match trimmed.split_once(':')? {
                 ("dynamo", s) => Some(Self::Dynamo(DynamoCommit::from_str(s)?)),
                 _ => None,