From 1869cfeba7b588c259213456b8dbd1f57157dadd Mon Sep 17 00:00:00 2001 From: Henk-Jan Lebbink Date: Sat, 26 Apr 2025 20:55:48 +0200 Subject: [PATCH] minor issues (#149) --- src/s3/builders/copy_object.rs | 11 +++++---- src/s3/builders/put_object.rs | 9 ++++--- src/s3/builders/remove_objects.rs | 27 ++++++++++++-------- src/s3/client/append_object.rs | 12 ++++----- src/s3/client/copy_object.rs | 6 ++++- src/s3/client/put_object.rs | 6 ++--- src/s3/client/set_object_retention.rs | 6 ++--- src/s3/client/set_object_tags.rs | 6 ++++- src/s3/client/stat_object.rs | 6 ++++- src/s3/error.rs | 14 ----------- src/s3/multimap.rs | 2 +- src/s3/response/append_object.rs | 2 +- src/s3/response/get_bucket_versioning.rs | 3 ++- src/s3/response/get_object.rs | 2 +- src/s3/response/get_object_lock_config.rs | 2 +- src/s3/response/list_objects.rs | 2 +- src/s3/response/object_prompt.rs | 2 +- src/s3/response/put_object.rs | 12 +++------ src/s3/utils.rs | 2 +- tests/test_append_object.rs | 2 +- tests/test_object_put.rs | 2 +- tests/test_put_object.rs | 30 +++++++++++++++-------- 22 files changed, 89 insertions(+), 77 deletions(-) diff --git a/src/s3/builders/copy_object.rs b/src/s3/builders/copy_object.rs index 19212ad..a4d4fe5 100644 --- a/src/s3/builders/copy_object.rs +++ b/src/s3/builders/copy_object.rs @@ -14,7 +14,7 @@ // limitations under the License. use crate::s3::Client; -use crate::s3::client::MAX_PART_SIZE; +use crate::s3::client::{MAX_MULTIPART_COUNT, MAX_PART_SIZE}; use crate::s3::error::Error; use crate::s3::multimap::{Multimap, MultimapExt}; use crate::s3::response::{ @@ -96,10 +96,11 @@ impl ToS3Request for UploadPartCopy { if self.upload_id.is_empty() { return Err(Error::InvalidUploadId("upload ID cannot be empty".into())); } - if !(1..=10000).contains(&self.part_number) { - return Err(Error::InvalidPartNumber( - "part number must be between 1 and 10000".into(), - )); + if !(1..=MAX_MULTIPART_COUNT).contains(&self.part_number) { + return Err(Error::InvalidPartNumber(format!( + "part number must be between 1 and {}", + MAX_MULTIPART_COUNT + ))); } } diff --git a/src/s3/builders/put_object.rs b/src/s3/builders/put_object.rs index 9ee417f..7803cc3 100644 --- a/src/s3/builders/put_object.rs +++ b/src/s3/builders/put_object.rs @@ -415,10 +415,11 @@ impl ToS3Request for UploadPart { } } if let Some(part_number) = self.part_number { - if !(1..=10000).contains(&part_number) { - return Err(Error::InvalidPartNumber( - "part number must be between 1 and 10000".into(), - )); + if !(1..=MAX_MULTIPART_COUNT).contains(&part_number) { + return Err(Error::InvalidPartNumber(format!( + "part number must be between 1 and {}", + MAX_MULTIPART_COUNT + ))); } } } diff --git a/src/s3/builders/remove_objects.rs b/src/s3/builders/remove_objects.rs index d899044..72ed3d4 100644 --- a/src/s3/builders/remove_objects.rs +++ b/src/s3/builders/remove_objects.rs @@ -23,6 +23,7 @@ use std::pin::Pin; use tokio_stream::iter as stream_iter; +use crate::s3::client::MAX_MULTIPART_COUNT; use crate::s3::multimap::{Multimap, MultimapExt}; use crate::s3::response::DeleteError; use crate::s3::types::ListEntry; @@ -36,6 +37,12 @@ use crate::s3::{ }; // region: object-to-delete + +pub trait ValidKey: Into {} +impl ValidKey for String {} +impl ValidKey for &str {} +impl ValidKey for &String {} + /// Specify an object to be deleted. The object can be specified by key or by /// key and version_id via the From trait. #[derive(Debug, Clone, Default)] @@ -45,30 +52,30 @@ pub struct ObjectToDelete { } /// A key can be converted into a DeleteObject. The version_id is set to None. -impl From<&str> for ObjectToDelete { - fn from(key: &str) -> Self { +impl From for ObjectToDelete { + fn from(key: K) -> Self { Self { - key: key.to_owned(), + key: key.into(), version_id: None, } } } /// A tuple of key and version_id can be converted into a DeleteObject. -impl From<(&str, &str)> for ObjectToDelete { - fn from((key, version_id): (&str, &str)) -> Self { +impl From<(K, &str)> for ObjectToDelete { + fn from((key, version_id): (K, &str)) -> Self { Self { - key: key.to_string(), + key: key.into(), version_id: Some(version_id.to_string()), } } } /// A tuple of key and option version_id can be converted into a DeleteObject. -impl From<(&str, Option<&str>)> for ObjectToDelete { - fn from((key, version_id): (&str, Option<&str>)) -> Self { +impl From<(K, Option<&str>)> for ObjectToDelete { + fn from((key, version_id): (K, Option<&str>)) -> Self { Self { - key: key.to_string(), + key: key.into(), version_id: version_id.map(|v| v.to_string()), } } @@ -358,7 +365,7 @@ impl RemoveObjects { let mut objects = Vec::new(); while let Some(object) = self.objects.items.next().await { objects.push(object); - if objects.len() >= 1000 { + if objects.len() >= MAX_MULTIPART_COUNT as usize { break; } } diff --git a/src/s3/client/append_object.rs b/src/s3/client/append_object.rs index 51c12f7..1192e1d 100644 --- a/src/s3/client/append_object.rs +++ b/src/s3/client/append_object.rs @@ -25,10 +25,10 @@ impl Client { /// This is a lower-level API that performs a non-multipart object upload. /// /// 🛈 This operation is not supported for regular non-express buckets. - pub fn append_object>( + pub fn append_object, S2: Into>( &self, - bucket: S, - object: S, + bucket: S1, + object: S2, data: SegmentedBytes, offset_bytes: u64, ) -> AppendObject { @@ -44,10 +44,10 @@ impl Client { /// Creates an AppendObjectContent request builder to append data to the end of an existing /// object. The content is streamed and appended to MinIO/S3. This is a higher-level API that /// handles multipart appends transparently. - pub fn append_object_content, C: Into>( + pub fn append_object_content, S2: Into, C: Into>( &self, - bucket: S, - object: S, + bucket: S1, + object: S2, content: C, ) -> AppendObjectContent { AppendObjectContent::new(self.clone(), bucket.into(), object.into(), content) diff --git a/src/s3/client/copy_object.rs b/src/s3/client/copy_object.rs index 7aa5918..413a79a 100644 --- a/src/s3/client/copy_object.rs +++ b/src/s3/client/copy_object.rs @@ -44,7 +44,11 @@ impl Client { /// copy object is a high-order API that calls [`stat_object`] and based on the results calls /// either [`compose_object`] or [`copy_object_internal`] to copy the object. - pub fn copy_object>(&self, bucket: S, object: S) -> CopyObject { + pub fn copy_object, S2: Into>( + &self, + bucket: S1, + object: S2, + ) -> CopyObject { CopyObject::new(self.clone(), bucket.into(), object.into()) } diff --git a/src/s3/client/put_object.rs b/src/s3/client/put_object.rs index 79f6ae9..bb3a9f2 100644 --- a/src/s3/client/put_object.rs +++ b/src/s3/client/put_object.rs @@ -113,10 +113,10 @@ impl Client { /// Creates a PutObjectContent request builder to upload data to MinIO/S3. /// The content is streamed, and this higher-level API handles multipart uploads transparently. - pub fn put_object_content, C: Into>( + pub fn put_object_content, S2: Into, C: Into>( &self, - bucket: S, - object: S, + bucket: S1, + object: S2, content: C, ) -> PutObjectContent { PutObjectContent::new(self.clone(), bucket.into(), object.into(), content) diff --git a/src/s3/client/set_object_retention.rs b/src/s3/client/set_object_retention.rs index ea0d570..c67370c 100644 --- a/src/s3/client/set_object_retention.rs +++ b/src/s3/client/set_object_retention.rs @@ -48,10 +48,10 @@ impl Client { /// println!("set the object retention for object '{}'", resp.object); /// } /// ``` - pub fn set_object_retention>( + pub fn set_object_retention, S2: Into>( &self, - bucket: S, - object: S, + bucket: S1, + object: S2, ) -> SetObjectRetention { SetObjectRetention::new(self.clone(), bucket.into(), object.into()) } diff --git a/src/s3/client/set_object_tags.rs b/src/s3/client/set_object_tags.rs index faace38..fd1a736 100644 --- a/src/s3/client/set_object_tags.rs +++ b/src/s3/client/set_object_tags.rs @@ -23,7 +23,11 @@ impl Client { /// /// 🛈 This operation is not supported for express buckets. /// - pub fn set_object_tags>(&self, bucket: S, object: S) -> SetObjectTags { + pub fn set_object_tags, S2: Into>( + &self, + bucket: S1, + object: S2, + ) -> SetObjectTags { SetObjectTags::new(self.clone(), bucket.into(), object.into()) } } diff --git a/src/s3/client/stat_object.rs b/src/s3/client/stat_object.rs index 9a5c3a8..42d5386 100644 --- a/src/s3/client/stat_object.rs +++ b/src/s3/client/stat_object.rs @@ -41,7 +41,11 @@ impl Client { /// println!("stat of object '{}' are {:#?}", resp.object, resp); /// } /// ``` - pub fn stat_object>(&self, bucket: S, object: S) -> StatObject { + pub fn stat_object, S2: Into>( + &self, + bucket: S1, + object: S2, + ) -> StatObject { StatObject::new(self.clone(), bucket.into(), object.into()) } } diff --git a/src/s3/error.rs b/src/s3/error.rs index d7d387e..2865f35 100644 --- a/src/s3/error.rs +++ b/src/s3/error.rs @@ -178,13 +178,6 @@ pub enum Error { NoClientProvided, TagDecodingError(String, String), ContentLengthUnknown, - - //TODO are the following still needed? - NoSuchTagSet, - ReplicationConfigurationNotFoundError, - NoSuchObjectLockConfiguration, - NoSuchBucketPolicy, - NoSuchBucket, } impl std::error::Error for Error {} @@ -354,13 +347,6 @@ impl fmt::Display for Error { error_message, input ), Error::ContentLengthUnknown => write!(f, "content length is unknown"), - Error::NoSuchTagSet => write!(f, "no such tag set"), - Error::ReplicationConfigurationNotFoundError => { - write!(f, "Replication configuration not found") - } - Error::NoSuchObjectLockConfiguration => write!(f, "no such object lock"), - Error::NoSuchBucketPolicy => write!(f, "no such bucket policy"), - Error::NoSuchBucket => write!(f, "no such bucket"), } } } diff --git a/src/s3/multimap.rs b/src/s3/multimap.rs index 95fb0d7..3dd0e50 100644 --- a/src/s3/multimap.rs +++ b/src/s3/multimap.rs @@ -31,8 +31,8 @@ pub trait MultimapExt { fn add_multimap(&mut self, other: Multimap); fn add_version(&mut self, version: Option); - #[must_use] + #[must_use] fn take_version(self) -> Option; /// Converts multimap to HTTP query string diff --git a/src/s3/response/append_object.rs b/src/s3/response/append_object.rs index 474c708..d84709a 100644 --- a/src/s3/response/append_object.rs +++ b/src/s3/response/append_object.rs @@ -41,7 +41,7 @@ impl FromS3Response for AppendObjectResponse { resp: Result, ) -> Result { let mut resp = resp?; - let headers = mem::take(resp.headers_mut()); + let headers: HeaderMap = mem::take(resp.headers_mut()); let etag: String = match headers.get("etag") { Some(v) => v.to_str()?.to_string().trim_matches('"').to_string(), diff --git a/src/s3/response/get_bucket_versioning.rs b/src/s3/response/get_bucket_versioning.rs index 7d9611c..34ffd7b 100644 --- a/src/s3/response/get_bucket_versioning.rs +++ b/src/s3/response/get_bucket_versioning.rs @@ -53,7 +53,8 @@ impl FromS3Response for GetBucketVersioningResponse { "Enabled" => VersioningStatus::Enabled, _ => VersioningStatus::Suspended, // Default case }); - let mfa_delete: Option = get_option_text(&root, "MFADelete").map(|v| v == "Enabled"); + let mfa_delete: Option = + get_option_text(&root, "MFADelete").map(|v| v.eq_ignore_ascii_case("Enabled")); Ok(Self { headers, diff --git a/src/s3/response/get_object.rs b/src/s3/response/get_object.rs index 8e3bfc5..67d410e 100644 --- a/src/s3/response/get_object.rs +++ b/src/s3/response/get_object.rs @@ -44,7 +44,7 @@ impl FromS3Response for GetObjectResponse { ) -> Result { let mut resp = resp?; - let headers = mem::take(resp.headers_mut()); + let headers: HeaderMap = mem::take(resp.headers_mut()); let etag: Option = headers .get("etag") diff --git a/src/s3/response/get_object_lock_config.rs b/src/s3/response/get_object_lock_config.rs index 4781521..ec21242 100644 --- a/src/s3/response/get_object_lock_config.rs +++ b/src/s3/response/get_object_lock_config.rs @@ -42,7 +42,7 @@ impl FromS3Response for GetObjectLockConfigResponse { ) -> Result { let mut resp = resp?; - let headers = mem::take(resp.headers_mut()); + let headers: HeaderMap = mem::take(resp.headers_mut()); let body = resp.bytes().await?; let root = Element::parse(body.reader())?; diff --git a/src/s3/response/list_objects.rs b/src/s3/response/list_objects.rs index f95689a..4287e42 100644 --- a/src/s3/response/list_objects.rs +++ b/src/s3/response/list_objects.rs @@ -208,7 +208,7 @@ impl FromS3Response for ListObjectsV1Response { resp: Result, ) -> Result { let mut resp = resp?; - let headers = mem::take(resp.headers_mut()); + let headers: HeaderMap = mem::take(resp.headers_mut()); let body = resp.bytes().await?; let xmltree_root = xmltree::Element::parse(body.reader())?; let root = Element::from(&xmltree_root); diff --git a/src/s3/response/object_prompt.rs b/src/s3/response/object_prompt.rs index f66d863..b37dcb6 100644 --- a/src/s3/response/object_prompt.rs +++ b/src/s3/response/object_prompt.rs @@ -37,7 +37,7 @@ impl FromS3Response for ObjectPromptResponse { ) -> Result { let mut resp = resp?; - let headers = mem::take(resp.headers_mut()); + let headers: HeaderMap = mem::take(resp.headers_mut()); let body = resp.bytes().await?; let prompt_response: String = String::from_utf8(body.to_vec())?; diff --git a/src/s3/response/put_object.rs b/src/s3/response/put_object.rs index 2db302a..1e379cf 100644 --- a/src/s3/response/put_object.rs +++ b/src/s3/response/put_object.rs @@ -85,12 +85,6 @@ impl FromS3Response for CreateMultipartUploadResponse { req: S3Request, resp: Result, ) -> Result { - let bucket = req - .bucket - .ok_or_else(|| Error::InvalidBucketName("no bucket specified".into()))?; - let object = req - .object - .ok_or_else(|| Error::InvalidObjectName("no object specified".into()))?; let mut resp = resp?; let headers: HeaderMap = mem::take(resp.headers_mut()); @@ -99,11 +93,11 @@ impl FromS3Response for CreateMultipartUploadResponse { let upload_id: String = get_text(&root, "UploadId").map_err(|e| Error::InvalidUploadId(e.to_string()))?; - Ok(CreateMultipartUploadResponse { + Ok(Self { headers, region: req.inner_region, - bucket, - object, + bucket: take_bucket(req.bucket)?, + object: take_object(req.object)?, upload_id, }) } diff --git a/src/s3/utils.rs b/src/s3/utils.rs index 195fa89..0a80d26 100644 --- a/src/s3/utils.rs +++ b/src/s3/utils.rs @@ -493,7 +493,7 @@ pub mod xml { impl Element<'_> { pub fn name(&self) -> &str { - self.inner.name.as_str() + &self.inner.name } pub fn get_child_text(&self, tag: &str) -> Option { diff --git a/tests/test_append_object.rs b/tests/test_append_object.rs index 9a860fd..34de11f 100644 --- a/tests/test_append_object.rs +++ b/tests/test_append_object.rs @@ -351,7 +351,7 @@ async fn append_object_content_3() { assert_eq!(resp.size, sizes[idx] + initial_size); assert_eq!(resp.etag, etag); client - .remove_object(&test_bucket, object_name.as_str()) + .remove_object(&test_bucket, &object_name) .send() .await .unwrap(); diff --git a/tests/test_object_put.rs b/tests/test_object_put.rs index 36d4baa..c826d26 100644 --- a/tests/test_object_put.rs +++ b/tests/test_object_put.rs @@ -192,7 +192,7 @@ async fn put_object_content_2() { assert_eq!(resp.size, sizes[idx]); assert_eq!(resp.etag, etag); client - .remove_object(&test_bucket, object_name.as_str()) + .remove_object(&test_bucket, &object_name) .send() .await .unwrap(); diff --git a/tests/test_put_object.rs b/tests/test_put_object.rs index d5fb933..e7a2cd1 100644 --- a/tests/test_put_object.rs +++ b/tests/test_put_object.rs @@ -30,7 +30,8 @@ async fn put_object() { let object_name = rand_object_name(); let size = 16_u64; - ctx.client + let resp: PutObjectContentResponse = ctx + .client .put_object_content( &bucket_name, &object_name, @@ -39,7 +40,11 @@ async fn put_object() { .send() .await .unwrap(); - let resp = ctx + assert_eq!(resp.bucket, bucket_name); + assert_eq!(resp.object, object_name); + assert_eq!(resp.object_size, size); + + let resp: StatObjectResponse = ctx .client .stat_object(&bucket_name, &object_name) .send() @@ -47,18 +52,23 @@ async fn put_object() { .unwrap(); assert_eq!(resp.bucket, bucket_name); assert_eq!(resp.object, object_name); - assert_eq!(resp.size as u64, size); - ctx.client - .remove_object(&bucket_name, object_name.as_str()) + assert_eq!(resp.size, size); + + let resp: RemoveObjectResponse = ctx + .client + .remove_object(&bucket_name, &object_name) .send() .await .unwrap(); + assert!(!resp.version_id.is_some()); + // Validate delete succeeded. - let resp = ctx + let resp: Result = ctx .client .stat_object(&bucket_name, &object_name) .send() .await; + match resp.err().unwrap() { Error::S3Error(er) => { assert_eq!(er.code, ErrorCode::NoSuchKey) @@ -94,7 +104,7 @@ async fn put_object_multipart() { assert_eq!(resp.object, object_name); assert_eq!(resp.size as u64, size); ctx.client - .remove_object(&bucket_name, object_name.as_str()) + .remove_object(&bucket_name, &object_name) .send() .await .unwrap(); @@ -135,7 +145,7 @@ async fn put_object_content_1() { ); let resp: RemoveObjectResponse = ctx .client - .remove_object(&bucket_name, object_name.as_str()) + .remove_object(&bucket_name, &object_name) .send() .await .unwrap(); @@ -175,7 +185,7 @@ async fn put_object_content_2() { assert_eq!(resp.size, *size); assert_eq!(resp.etag, etag); ctx.client - .remove_object(&bucket_name, object_name.as_str()) + .remove_object(&bucket_name, &object_name) .send() .await .unwrap(); @@ -229,7 +239,7 @@ async fn put_object_content_3() { assert_eq!(resp.size, sizes[idx]); assert_eq!(resp.etag, etag); client - .remove_object(&test_bucket, object_name.as_str()) + .remove_object(&test_bucket, &object_name) .send() .await .unwrap();