Tests cleanup; cargo clippy fixes, minor doc updates

.github/workflows/label-checker.yaml

@@ -1,19 +1,17 @@
-name: Label Checker
-on:
-  pull_request:
-    types:
-      - opened
-      - synchronize
-      - labeled
-      - unlabeled
-
-jobs:
-
-  check_labels:
-    name: Check for  labels
-    runs-on: ubuntu-latest
-    steps:
-      - uses: docker://agilepathway/pull-request-label-checker:latest
-        with:
-          one_of: highlight,breaking-change,security-fix,enhancement,bug
-          repo_token: ${{ secrets.GITHUB_TOKEN }}
+  name: Label Checker
+  on:
+    pull_request:
+      types:
+        - opened
+        - synchronize
+        - labeled
+        - unlabeled
+  jobs:
+    check_labels:
+      name: Check for labels
+      runs-on: ubuntu-latest
+      steps:
+        - uses: docker://agilepathway/pull-request-label-checker:latest
+          with:
+            any_of: highlight,breaking-change,security-fix,enhancement,bug,cleanup-rewrite,regression-fix,codex
+            repo_token: ${{ secrets.GITHUB_TOKEN }}

Cargo.toml

@@ -41,9 +41,10 @@ hmac = { version = "0.12.1", optional = true }
 hyper = { version = "1.6.0", features = ["full"] }
 lazy_static = "1.5.0"
 log = "0.4.27"
-md5 = "0.7.0"
+md5 = "0.8.0"
 multimap = "0.10.1"
 percent-encoding = "2.3.1"
+url = "2.5.4"
 rand = { version = "0.8.5", features = ["small_rng"] }
 regex = "1.11.1"
 ring = { version = "0.17.14", optional = true, default-features = false, features = ["alloc"] }

common/Cargo.toml

@@ -14,6 +14,7 @@ chrono = "0.4.41"
 reqwest = "0.12.20"
 http = "1.3.1"
 futures = "0.3.31"
+uuid = { version = "1.17.0", features = ["v4"] }
 
 [lib]
 name = "minio_common"

common/src/utils.rs

@@ -15,16 +15,24 @@
 
 use http::{Response as HttpResponse, StatusCode};
 use minio::s3::error::Error;
-use rand::distributions::{Alphanumeric, DistString};
+use rand::distributions::Standard;
+use rand::{Rng, thread_rng};
+use uuid::Uuid;
 
 pub fn rand_bucket_name() -> String {
-    Alphanumeric
-        .sample_string(&mut rand::thread_rng(), 8)
-        .to_lowercase()
+    format!("test-bucket-{}", Uuid::new_v4())
 }
 
 pub fn rand_object_name() -> String {
-    Alphanumeric.sample_string(&mut rand::thread_rng(), 20)
+    format!("test-object-{}", Uuid::new_v4())
+}
+
+pub fn rand_object_name_utf8(len: usize) -> String {
+    let rng = thread_rng();
+    rng.sample_iter::<char, _>(Standard)
+        .filter(|c| !c.is_control())
+        .take(len)
+        .collect()
 }
 
 pub async fn get_bytes_from_response(v: Result<reqwest::Response, Error>) -> bytes::Bytes {

src/s3/builders/delete_objects.rs

@@ -209,7 +209,7 @@ impl DeleteObjects {
     }
 
     /// Enable verbose mode (defaults to false). If enabled, the response will
-    /// include the keys of objects that were successfully deleted. Otherwise
+    /// include the keys of objects that were successfully deleted. Otherwise,
     /// only objects that encountered an error are returned.
     pub fn verbose_mode(mut self, verbose_mode: bool) -> Self {
         self.verbose_mode = verbose_mode;

src/s3/client/delete_bucket.rs

@@ -57,35 +57,33 @@ impl Client {
         bucket: S,
     ) -> Result<DeleteBucketResponse, Error> {
         let bucket: String = bucket.into();
-        if self.is_minio_express().await {
-            let mut stream = self.list_objects(&bucket).to_stream().await;
+        let is_express = self.is_minio_express().await;
 
+        let mut stream = self
+            .list_objects(&bucket)
+            .include_versions(!is_express)
+            .recursive(true)
+            .to_stream()
+            .await;
+
+        if is_express {
             while let Some(items) = stream.next().await {
+                let object_names = items?.contents.into_iter().map(ObjectToDelete::from);
                 let mut resp = self
-                    .delete_objects_streaming(
-                        &bucket,
-                        items?.contents.into_iter().map(ObjectToDelete::from),
-                    )
+                    .delete_objects_streaming(&bucket, object_names)
+                    .bypass_governance_mode(false) // Express does not support governance mode
                     .to_stream()
                     .await;
+
                 while let Some(item) = resp.next().await {
                     let _resp: DeleteObjectsResponse = item?;
                 }
             }
         } else {
-            let mut stream = self
-                .list_objects(&bucket)
-                .include_versions(true)
-                .recursive(true)
-                .to_stream()
-                .await;
-
             while let Some(items) = stream.next().await {
+                let object_names = items?.contents.into_iter().map(ObjectToDelete::from);
                 let mut resp = self
-                    .delete_objects_streaming(
-                        &bucket,
-                        items?.contents.into_iter().map(ObjectToDelete::from),
-                    )
+                    .delete_objects_streaming(&bucket, object_names)
                     .bypass_governance_mode(true)
                     .to_stream()
                     .await;
@@ -117,16 +115,41 @@ impl Client {
                 }
             }
         }
-        let request: DeleteBucket = self.delete_bucket(bucket);
+
+        let request: DeleteBucket = self.delete_bucket(&bucket);
         match request.send().await {
             Ok(resp) => Ok(resp),
-            Err(Error::S3Error(e)) => {
-                if e.code == ErrorCode::NoSuchBucket {
+            Err(Error::S3Error(mut e)) => {
+                if matches!(e.code, ErrorCode::NoSuchBucket) {
                     Ok(DeleteBucketResponse {
                         request: Default::default(), //TODO consider how to handle this
                         body: Bytes::new(),
                         headers: e.headers,
                     })
+                } else if let ErrorCode::BucketNotEmpty(reason) = &e.code {
+                    // for convenience, add the first 5 documents that were are still in the bucket
+                    // to the error message
+                    let mut stream = self
+                        .list_objects(&bucket)
+                        .include_versions(!is_express)
+                        .recursive(true)
+                        .to_stream()
+                        .await;
+
+                    let mut objs = Vec::new();
+                    while let Some(items_result) = stream.next().await {
+                        if let Ok(items) = items_result {
+                            objs.extend(items.contents);
+                            if objs.len() >= 5 {
+                                break;
+                            }
+                        }
+                        // else: silently ignore the error and keep looping
+                    }
+
+                    let new_reason = format!("{reason}: found content: {objs:?}");
+                    e.code = ErrorCode::BucketNotEmpty(new_reason);
+                    Err(Error::S3Error(e))
                 } else {
                     Err(Error::S3Error(e))
                 }

src/s3/client/delete_objects.rs

@@ -66,7 +66,7 @@ impl Client {
 
     /// Creates a [`DeleteObjectsStreaming`] request builder to delete a stream of objects from an S3 bucket.
     ///
-    /// to execute the request, call [`DeleteObjectsStreaming::to_stream()`](crate::s3::types::S3Api::send),
+    /// To execute the request, call [`DeleteObjectsStreaming::to_stream()`](crate::s3::types::S3Api::send),
     /// which returns a [`Result`] containing a [`DeleteObjectsResponse`](crate::s3::response::DeleteObjectsResponse).
     pub fn delete_objects_streaming<S: Into<String>, D: Into<ObjectsStream>>(
         &self,

src/s3/error.rs

@@ -44,7 +44,7 @@ pub enum ErrorCode {
     ResourceConflict,
     AccessDenied,
     NotSupported,
-    BucketNotEmpty,
+    BucketNotEmpty(String), // String contains optional reason msg
     BucketAlreadyOwnedByYou,
     InvalidWriteOffset,
 
@@ -75,7 +75,7 @@ impl ErrorCode {
             "resourceconflict" => ErrorCode::ResourceConflict,
             "accessdenied" => ErrorCode::AccessDenied,
             "notsupported" => ErrorCode::NotSupported,
-            "bucketnotempty" => ErrorCode::BucketNotEmpty,
+            "bucketnotempty" => ErrorCode::BucketNotEmpty("".to_string()),
             "bucketalreadyownedbyyou" => ErrorCode::BucketAlreadyOwnedByYou,
             "invalidwriteoffset" => ErrorCode::InvalidWriteOffset,
 

src/s3/lifecycle_config.rs

@@ -480,5 +480,5 @@ impl LifecycleRule {
 fn parse_iso8601(date_str: &str) -> Result<chrono::DateTime<chrono::Utc>, Error> {
     chrono::DateTime::parse_from_rfc3339(date_str)
         .map(|dt| dt.with_timezone(&chrono::Utc))
-        .map_err(|_| Error::XmlError(format!("Invalid date format: {}", date_str)))
+        .map_err(|_| Error::XmlError(format!("Invalid date format: {date_str}")))
 }

src/s3/response/bucket_exists.rs

@@ -50,7 +50,7 @@ impl FromS3Response for BucketExistsResponse {
                 body: resp.bytes().await?,
                 exists: true,
             }),
-            Err(Error::S3Error(e)) if e.code == ErrorCode::NoSuchBucket => Ok(Self {
+            Err(Error::S3Error(e)) if matches!(e.code, ErrorCode::NoSuchBucket) => Ok(Self {
                 request,
                 headers: e.headers,
                 body: Bytes::new(),

src/s3/response/delete_bucket_policy.rs

@@ -48,7 +48,7 @@ impl FromS3Response for DeleteBucketPolicyResponse {
                 headers: mem::take(resp.headers_mut()),
                 body: resp.bytes().await?,
             }),
-            Err(Error::S3Error(e)) if e.code == ErrorCode::NoSuchBucketPolicy => Ok(Self {
+            Err(Error::S3Error(e)) if matches!(e.code, ErrorCode::NoSuchBucketPolicy) => Ok(Self {
                 request,
                 headers: e.headers,
                 body: Bytes::new(),

src/s3/response/delete_bucket_replication.rs

@@ -49,7 +49,7 @@ impl FromS3Response for DeleteBucketReplicationResponse {
                 body: resp.bytes().await?,
             }),
             Err(Error::S3Error(e))
-                if e.code == ErrorCode::ReplicationConfigurationNotFoundError =>
+                if matches!(e.code, ErrorCode::ReplicationConfigurationNotFoundError) =>
             {
                 Ok(Self {
                     request,

src/s3/response/get_bucket_encryption.rs

@@ -85,7 +85,10 @@ impl FromS3Response for GetBucketEncryptionResponse {
                 body: resp.bytes().await?,
             }),
             Err(Error::S3Error(e))
-                if e.code == ErrorCode::ServerSideEncryptionConfigurationNotFoundError =>
+                if matches!(
+                    e.code,
+                    ErrorCode::ServerSideEncryptionConfigurationNotFoundError
+                ) =>
             {
                 Ok(Self {
                     request,

src/s3/response/get_bucket_policy.rs

@@ -48,7 +48,7 @@ impl GetBucketPolicyResponse {
     /// for accessing the bucket and its contents.
     pub fn config(&self) -> Result<&str, Error> {
         std::str::from_utf8(&self.body).map_err(|e| {
-            Error::Utf8Error(format!("Failed to parse bucket policy as UTF-8: {}", e).into())
+            Error::Utf8Error(format!("Failed to parse bucket policy as UTF-8: {e}").into())
         })
     }
 }
@@ -65,7 +65,7 @@ impl FromS3Response for GetBucketPolicyResponse {
                 headers: mem::take(resp.headers_mut()),
                 body: resp.bytes().await?,
             }),
-            Err(Error::S3Error(e)) if e.code == ErrorCode::NoSuchBucketPolicy => Ok(Self {
+            Err(Error::S3Error(e)) if matches!(e.code, ErrorCode::NoSuchBucketPolicy) => Ok(Self {
                 request,
                 headers: e.headers,
                 body: Bytes::from_static("{}".as_ref()),

src/s3/response/get_bucket_tagging.rs

@@ -54,7 +54,7 @@ impl FromS3Response for GetBucketTaggingResponse {
                 headers: mem::take(resp.headers_mut()),
                 body: resp.bytes().await?,
             }),
-            Err(Error::S3Error(e)) if e.code == ErrorCode::NoSuchTagSet => Ok(Self {
+            Err(Error::S3Error(e)) if matches!(e.code, ErrorCode::NoSuchTagSet) => Ok(Self {
                 request,
                 headers: e.headers,
                 body: Bytes::new(),

src/s3/response/get_object_prompt.rs

@@ -40,7 +40,7 @@ impl GetObjectPromptResponse {
     /// This method retrieves the content of the object as a UTF-8 encoded string.
     pub fn prompt_response(&self) -> Result<&str, Error> {
         std::str::from_utf8(&self.body).map_err(|e| {
-            Error::Utf8Error(format!("Failed to parse prompt_response as UTF-8: {}", e).into())
+            Error::Utf8Error(format!("Failed to parse prompt_response as UTF-8: {e}").into())
         })
     }
 }

src/s3/response/get_object_retention.rs

@@ -83,7 +83,9 @@ impl FromS3Response for GetObjectRetentionResponse {
                 headers: mem::take(resp.headers_mut()),
                 body: resp.bytes().await?,
             }),
-            Err(Error::S3Error(e)) if e.code == ErrorCode::NoSuchObjectLockConfiguration => {
+            Err(Error::S3Error(e))
+                if matches!(e.code, ErrorCode::NoSuchObjectLockConfiguration) =>
+            {
                 Ok(Self {
                     request,
                     headers: e.headers,

src/s3/response/list_objects.rs

@@ -10,18 +10,12 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-//! Response types for ListObjects APIs
-
 use crate::impl_has_s3fields;
+use crate::s3::error::Error;
 use crate::s3::response::a_response_traits::HasS3Fields;
-use crate::s3::{
-    error::Error,
-    types::{FromS3Response, ListEntry, S3Request},
-    utils::{
-        from_iso8601utc, parse_tags, urldecode,
-        xml::{Element, MergeXmlElements},
-    },
-};
+use crate::s3::types::{FromS3Response, ListEntry, S3Request};
+use crate::s3::utils::xml::{Element, MergeXmlElements};
+use crate::s3::utils::{from_iso8601utc, parse_tags, urldecode};
 use async_trait::async_trait;
 use bytes::{Buf, Bytes};
 use reqwest::header::HeaderMap;

src/s3/segmented_bytes.rs

@@ -80,7 +80,7 @@ impl SegmentedBytes {
 impl fmt::Display for SegmentedBytes {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         match std::str::from_utf8(self.to_bytes().as_ref()) {
-            Ok(s) => write!(f, "{}", s),
+            Ok(s) => write!(f, "{s}"),
             Err(_) => Ok(()), // or: write!(f, "<invalid utf8>")
         }
     }