chore: enable conditional put by default for S3

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`])
@@ -523,7 +523,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())
@@ -581,9 +581,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,7 +825,7 @@ impl AmazonS3Builder {
 
     /// Configure how to provide conditional put operations
     pub fn with_conditional_put(mut self, config: S3ConditionalPut) -> Self {
-        self.conditional_put = Some(config.into());
+        self.conditional_put = config.into();
         self
     }
 
@@ -893,7 +891,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
@@ -1034,7 +1031,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

@@ -201,7 +201,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,7 @@ 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::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 +183,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(),
@@ -561,7 +560,6 @@ 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();
 
         put_get_delete_list(&integration).await;
         get_opts(&integration).await;
@@ -594,9 +592,7 @@ mod tests {
         if test_not_exists {
             copy_if_not_exists(&integration).await;
         }
-        if test_conditional_put {
-            put_opts(&integration, true).await;
-        }
+        put_opts(&integration, true).await;
 
         // run integration test with unsigned payload enabled
         let builder = AmazonS3Builder::from_env().with_unsigned_payload(true);

object_store/src/aws/precondition.rs

@@ -18,6 +18,7 @@
 use crate::aws::dynamo::DynamoCommit;
 use crate::config::Parse;
 
+use crate::aws::S3ConditionalPut::ETagMatch;
 use itertools::Itertools;
 
 /// Configure how to provide [`ObjectStore::copy_if_not_exists`] for [`AmazonS3`].
@@ -149,6 +150,12 @@ pub enum S3ConditionalPut {
     Dynamo(DynamoCommit),
 }
 
+impl Default for S3ConditionalPut {
+    fn default() -> Self {
+        ETagMatch
+    }
+}
+
 impl std::fmt::Display for S3ConditionalPut {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
         match self {