update code review

Author: HJLebbink

examples/bucket_versioning.rs

@@ -16,6 +16,7 @@
 mod common;
 
 use crate::common::{create_bucket_if_not_exists, create_client_on_play};
+use minio::s3::builders::VersioningStatus;
 use minio::s3::response::{GetBucketVersioningResponse, SetBucketVersioningResponse};
 use minio::s3::types::S3Api;
 use minio::s3::Client;
@@ -38,7 +39,7 @@ async fn main() -> Result<(), Box<dyn std::error::Error + Send + Sync>> {
 
     let _resp: SetBucketVersioningResponse = client
         .set_bucket_versioning(bucket_name)
-        .status(true)
+        .versioning_status(VersioningStatus::Enabled)
         .send()
         .await?;
 
@@ -53,7 +54,7 @@ async fn main() -> Result<(), Box<dyn std::error::Error + Send + Sync>> {
 
     let _resp: SetBucketVersioningResponse = client
         .set_bucket_versioning(bucket_name)
-        .status(false)
+        .versioning_status(VersioningStatus::Suspended)
         .send()
         .await?;
 

src/s3/builders/set_bucket_versioning.rs

@@ -21,6 +21,27 @@ use crate::s3::utils::{check_bucket_name, Multimap};
 use crate::s3::Client;
 use bytes::Bytes;
 use http::Method;
+use std::fmt;
+
+#[derive(Clone, Debug, Default, PartialEq)]
+pub enum VersioningStatus {
+    /// **Enable** object versioning in given bucket.
+    Enabled,
+    /// **Suspend** object versioning in given bucket.
+    Suspended,
+    #[default]
+    None,
+}
+
+impl fmt::Display for VersioningStatus {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        match self {
+            VersioningStatus::Enabled => write!(f, "Enabled"),
+            VersioningStatus::Suspended => write!(f, "Suspended"),
+            VersioningStatus::None => write!(f, "None"),
+        }
+    }
+}
 
 /// Argument builder for [set_bucket_encryption()](Client::set_bucket_encryption) API
 #[derive(Clone, Debug, Default)]
@@ -32,7 +53,7 @@ pub struct SetBucketVersioning {
     pub(crate) region: Option<String>,
     pub(crate) bucket: String,
 
-    pub(crate) status: bool,
+    pub(crate) status: VersioningStatus,
     pub(crate) mfa_delete: Option<bool>,
 }
 
@@ -63,7 +84,7 @@ impl SetBucketVersioning {
         self
     }
 
-    pub fn status(mut self, status: bool) -> Self {
+    pub fn versioning_status(mut self, status: VersioningStatus) -> Self {
         self.status = status;
         self
     }
@@ -97,21 +118,21 @@ impl ToS3Request for SetBucketVersioning {
 
         query_params.insert("versioning".into(), String::new());
 
-        let mfa_delete = self
-            .mfa_delete
-            .map(|v| {
-                format!(
-                    "<MFADelete>{}</MFADelete>",
-                    if v { "Enabled" } else { "Disabled" }
-                )
-            })
-            .unwrap_or_default();
+        let mut data = "<VersioningConfiguration>".to_string();
+
+        if let Some(v) = self.mfa_delete {
+            data.push_str("<MFADelete>");
+            data.push_str(if v { "Enabled" } else { "Disabled" });
+            data.push_str("</MFADelete>");
+        }
+
+        match self.status {
+            VersioningStatus::Enabled => data.push_str("<Status>Enabled</Status>"),
+            VersioningStatus::Suspended => data.push_str("<Status>Suspended</Status>"),
+            VersioningStatus::None => {}
+        }
 
-        let data: String = format!(
-            "<VersioningConfiguration><Status>{}</Status>{}</VersioningConfiguration>",
-            if self.status { "Enabled" } else { "Suspended" },
-            mfa_delete
-        );
+        data.push_str("</VersioningConfiguration>");
 
         let body: Option<SegmentedBytes> = Some(SegmentedBytes::from(Bytes::from(data)));
         let client: &Client = self.client.as_ref().ok_or(Error::NoClientProvided)?;

src/s3/response/get_bucket_versioning.rs

@@ -13,6 +13,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
+use crate::s3::builders::VersioningStatus;
 use crate::s3::error::Error;
 use crate::s3::types::{FromS3Response, S3Request};
 use crate::s3::utils::get_option_text;
@@ -29,7 +30,7 @@ pub struct GetBucketVersioningResponse {
     pub headers: HeaderMap,
     pub region: String,
     pub bucket: String,
-    pub status: Option<bool>,
+    pub status: Option<VersioningStatus>,
     pub mfa_delete: Option<bool>,
 }
 
@@ -47,7 +48,11 @@ impl FromS3Response for GetBucketVersioningResponse {
         let headers = resp.headers().clone();
         let body = resp.bytes().await?;
         let root = Element::parse(body.reader())?;
-        let status: Option<bool> = get_option_text(&root, "Status").map(|v| v == "Enabled");
+        let status: Option<VersioningStatus> =
+            get_option_text(&root, "Status").map(|v| match v.as_str() {
+                "Enabled" => VersioningStatus::Enabled,
+                _ => VersioningStatus::Suspended, // Default case
+            });
         let mfa_delete: Option<bool> = get_option_text(&root, "MFADelete").map(|v| v == "Enabled");
 
         Ok(GetBucketVersioningResponse {

tests/tests.rs

@@ -20,7 +20,7 @@ use futures_util::Stream;
 use http::header;
 use hyper::http::Method;
 
-use minio::s3::builders::{ObjectContent, ObjectToDelete};
+use minio::s3::builders::{ObjectContent, ObjectToDelete, VersioningStatus};
 use rand::{
     distributions::{Alphanumeric, DistString},
     rngs::SmallRng,
@@ -1259,7 +1259,7 @@ impl ClientTest {
 
         self.client
             .set_bucket_versioning(&bucket_name)
-            .status(true)
+            .versioning_status(VersioningStatus::Enabled)
             .send()
             .await
             .unwrap();
@@ -1270,11 +1270,11 @@ impl ClientTest {
             .send()
             .await
             .unwrap();
-        assert_eq!(resp.status, Some(true));
+        assert_eq!(resp.status, Some(VersioningStatus::Enabled));
 
         self.client
             .set_bucket_versioning(&bucket_name)
-            .status(false)
+            .versioning_status(VersioningStatus::Suspended)
             .send()
             .await
             .unwrap();
@@ -1285,7 +1285,7 @@ impl ClientTest {
             .send()
             .await
             .unwrap();
-        assert_eq!(resp.status, Some(false));
+        assert_eq!(resp.status, Some(VersioningStatus::Suspended));
 
         self.client
             .remove_bucket(&RemoveBucketArgs::new(&bucket_name).unwrap())