Skip to content

Commit d8ac6b8

Browse files
committed
minor issues
1 parent 58d9203 commit d8ac6b8

22 files changed

+89
-77
lines changed

src/s3/builders/copy_object.rs

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414
// limitations under the License.
1515

1616
use crate::s3::Client;
17-
use crate::s3::client::MAX_PART_SIZE;
17+
use crate::s3::client::{MAX_MULTIPART_COUNT, MAX_PART_SIZE};
1818
use crate::s3::error::Error;
1919
use crate::s3::multimap::{Multimap, MultimapExt};
2020
use crate::s3::response::{
@@ -96,10 +96,11 @@ impl ToS3Request for UploadPartCopy {
9696
if self.upload_id.is_empty() {
9797
return Err(Error::InvalidUploadId("upload ID cannot be empty".into()));
9898
}
99-
if !(1..=10000).contains(&self.part_number) {
100-
return Err(Error::InvalidPartNumber(
101-
"part number must be between 1 and 10000".into(),
102-
));
99+
if !(1..=MAX_MULTIPART_COUNT).contains(&self.part_number) {
100+
return Err(Error::InvalidPartNumber(format!(
101+
"part number must be between 1 and {}",
102+
MAX_MULTIPART_COUNT
103+
)));
103104
}
104105
}
105106

src/s3/builders/put_object.rs

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -415,10 +415,11 @@ impl ToS3Request for UploadPart {
415415
}
416416
}
417417
if let Some(part_number) = self.part_number {
418-
if !(1..=10000).contains(&part_number) {
419-
return Err(Error::InvalidPartNumber(
420-
"part number must be between 1 and 10000".into(),
421-
));
418+
if !(1..=MAX_MULTIPART_COUNT).contains(&part_number) {
419+
return Err(Error::InvalidPartNumber(format!(
420+
"part number must be between 1 and {}",
421+
MAX_MULTIPART_COUNT
422+
)));
422423
}
423424
}
424425
}

src/s3/builders/remove_objects.rs

Lines changed: 17 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ use std::pin::Pin;
2323

2424
use tokio_stream::iter as stream_iter;
2525

26+
use crate::s3::client::MAX_MULTIPART_COUNT;
2627
use crate::s3::multimap::{Multimap, MultimapExt};
2728
use crate::s3::response::DeleteError;
2829
use crate::s3::types::ListEntry;
@@ -36,6 +37,12 @@ use crate::s3::{
3637
};
3738

3839
// region: object-to-delete
40+
41+
pub trait ValidKey: Into<String> {}
42+
impl ValidKey for String {}
43+
impl ValidKey for &str {}
44+
impl ValidKey for &String {}
45+
3946
/// Specify an object to be deleted. The object can be specified by key or by
4047
/// key and version_id via the From trait.
4148
#[derive(Debug, Clone, Default)]
@@ -45,30 +52,30 @@ pub struct ObjectToDelete {
4552
}
4653

4754
/// A key can be converted into a DeleteObject. The version_id is set to None.
48-
impl From<&str> for ObjectToDelete {
49-
fn from(key: &str) -> Self {
55+
impl<K: ValidKey> From<K> for ObjectToDelete {
56+
fn from(key: K) -> Self {
5057
Self {
51-
key: key.to_owned(),
58+
key: key.into(),
5259
version_id: None,
5360
}
5461
}
5562
}
5663

5764
/// A tuple of key and version_id can be converted into a DeleteObject.
58-
impl From<(&str, &str)> for ObjectToDelete {
59-
fn from((key, version_id): (&str, &str)) -> Self {
65+
impl<K: ValidKey> From<(K, &str)> for ObjectToDelete {
66+
fn from((key, version_id): (K, &str)) -> Self {
6067
Self {
61-
key: key.to_string(),
68+
key: key.into(),
6269
version_id: Some(version_id.to_string()),
6370
}
6471
}
6572
}
6673

6774
/// A tuple of key and option version_id can be converted into a DeleteObject.
68-
impl From<(&str, Option<&str>)> for ObjectToDelete {
69-
fn from((key, version_id): (&str, Option<&str>)) -> Self {
75+
impl<K: ValidKey> From<(K, Option<&str>)> for ObjectToDelete {
76+
fn from((key, version_id): (K, Option<&str>)) -> Self {
7077
Self {
71-
key: key.to_string(),
78+
key: key.into(),
7279
version_id: version_id.map(|v| v.to_string()),
7380
}
7481
}
@@ -358,7 +365,7 @@ impl RemoveObjects {
358365
let mut objects = Vec::new();
359366
while let Some(object) = self.objects.items.next().await {
360367
objects.push(object);
361-
if objects.len() >= 1000 {
368+
if objects.len() >= MAX_MULTIPART_COUNT as usize {
362369
break;
363370
}
364371
}

src/s3/client/append_object.rs

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -25,10 +25,10 @@ impl Client {
2525
/// This is a lower-level API that performs a non-multipart object upload.
2626
///
2727
/// 🛈 This operation is not supported for regular non-express buckets.
28-
pub fn append_object<S: Into<String>>(
28+
pub fn append_object<S1: Into<String>, S2: Into<String>>(
2929
&self,
30-
bucket: S,
31-
object: S,
30+
bucket: S1,
31+
object: S2,
3232
data: SegmentedBytes,
3333
offset_bytes: u64,
3434
) -> AppendObject {
@@ -44,10 +44,10 @@ impl Client {
4444
/// Creates an AppendObjectContent request builder to append data to the end of an existing
4545
/// object. The content is streamed and appended to MinIO/S3. This is a higher-level API that
4646
/// handles multipart appends transparently.
47-
pub fn append_object_content<S: Into<String>, C: Into<ObjectContent>>(
47+
pub fn append_object_content<S1: Into<String>, S2: Into<String>, C: Into<ObjectContent>>(
4848
&self,
49-
bucket: S,
50-
object: S,
49+
bucket: S1,
50+
object: S2,
5151
content: C,
5252
) -> AppendObjectContent {
5353
AppendObjectContent::new(self.clone(), bucket.into(), object.into(), content)

src/s3/client/copy_object.rs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,11 @@ impl Client {
4444

4545
/// copy object is a high-order API that calls [`stat_object`] and based on the results calls
4646
/// either [`compose_object`] or [`copy_object_internal`] to copy the object.
47-
pub fn copy_object<S: Into<String>>(&self, bucket: S, object: S) -> CopyObject {
47+
pub fn copy_object<S1: Into<String>, S2: Into<String>>(
48+
&self,
49+
bucket: S1,
50+
object: S2,
51+
) -> CopyObject {
4852
CopyObject::new(self.clone(), bucket.into(), object.into())
4953
}
5054

src/s3/client/put_object.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -113,10 +113,10 @@ impl Client {
113113

114114
/// Creates a PutObjectContent request builder to upload data to MinIO/S3.
115115
/// The content is streamed, and this higher-level API handles multipart uploads transparently.
116-
pub fn put_object_content<S: Into<String>, C: Into<ObjectContent>>(
116+
pub fn put_object_content<S1: Into<String>, S2: Into<String>, C: Into<ObjectContent>>(
117117
&self,
118-
bucket: S,
119-
object: S,
118+
bucket: S1,
119+
object: S2,
120120
content: C,
121121
) -> PutObjectContent {
122122
PutObjectContent::new(self.clone(), bucket.into(), object.into(), content)

src/s3/client/set_object_retention.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -48,10 +48,10 @@ impl Client {
4848
/// println!("set the object retention for object '{}'", resp.object);
4949
/// }
5050
/// ```
51-
pub fn set_object_retention<S: Into<String>>(
51+
pub fn set_object_retention<S1: Into<String>, S2: Into<String>>(
5252
&self,
53-
bucket: S,
54-
object: S,
53+
bucket: S1,
54+
object: S2,
5555
) -> SetObjectRetention {
5656
SetObjectRetention::new(self.clone(), bucket.into(), object.into())
5757
}

src/s3/client/set_object_tags.rs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,11 @@ impl Client {
2323
///
2424
/// 🛈 This operation is not supported for express buckets.
2525
///
26-
pub fn set_object_tags<S: Into<String>>(&self, bucket: S, object: S) -> SetObjectTags {
26+
pub fn set_object_tags<S1: Into<String>, S2: Into<String>>(
27+
&self,
28+
bucket: S1,
29+
object: S2,
30+
) -> SetObjectTags {
2731
SetObjectTags::new(self.clone(), bucket.into(), object.into())
2832
}
2933
}

src/s3/client/stat_object.rs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,11 @@ impl Client {
4141
/// println!("stat of object '{}' are {:#?}", resp.object, resp);
4242
/// }
4343
/// ```
44-
pub fn stat_object<S: Into<String>>(&self, bucket: S, object: S) -> StatObject {
44+
pub fn stat_object<S1: Into<String>, S2: Into<String>>(
45+
&self,
46+
bucket: S1,
47+
object: S2,
48+
) -> StatObject {
4549
StatObject::new(self.clone(), bucket.into(), object.into())
4650
}
4751
}

src/s3/error.rs

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -178,13 +178,6 @@ pub enum Error {
178178
NoClientProvided,
179179
TagDecodingError(String, String),
180180
ContentLengthUnknown,
181-
182-
//TODO are the following still needed?
183-
NoSuchTagSet,
184-
ReplicationConfigurationNotFoundError,
185-
NoSuchObjectLockConfiguration,
186-
NoSuchBucketPolicy,
187-
NoSuchBucket,
188181
}
189182

190183
impl std::error::Error for Error {}
@@ -354,13 +347,6 @@ impl fmt::Display for Error {
354347
error_message, input
355348
),
356349
Error::ContentLengthUnknown => write!(f, "content length is unknown"),
357-
Error::NoSuchTagSet => write!(f, "no such tag set"),
358-
Error::ReplicationConfigurationNotFoundError => {
359-
write!(f, "Replication configuration not found")
360-
}
361-
Error::NoSuchObjectLockConfiguration => write!(f, "no such object lock"),
362-
Error::NoSuchBucketPolicy => write!(f, "no such bucket policy"),
363-
Error::NoSuchBucket => write!(f, "no such bucket"),
364350
}
365351
}
366352
}

src/s3/multimap.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,8 +31,8 @@ pub trait MultimapExt {
3131
fn add_multimap(&mut self, other: Multimap);
3232

3333
fn add_version(&mut self, version: Option<String>);
34-
#[must_use]
3534

35+
#[must_use]
3636
fn take_version(self) -> Option<String>;
3737

3838
/// Converts multimap to HTTP query string

src/s3/response/append_object.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ impl FromS3Response for AppendObjectResponse {
4141
resp: Result<reqwest::Response, Error>,
4242
) -> Result<Self, Error> {
4343
let mut resp = resp?;
44-
let headers = mem::take(resp.headers_mut());
44+
let headers: HeaderMap = mem::take(resp.headers_mut());
4545

4646
let etag: String = match headers.get("etag") {
4747
Some(v) => v.to_str()?.to_string().trim_matches('"').to_string(),

src/s3/response/get_bucket_versioning.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,8 @@ impl FromS3Response for GetBucketVersioningResponse {
5353
"Enabled" => VersioningStatus::Enabled,
5454
_ => VersioningStatus::Suspended, // Default case
5555
});
56-
let mfa_delete: Option<bool> = get_option_text(&root, "MFADelete").map(|v| v == "Enabled");
56+
let mfa_delete: Option<bool> =
57+
get_option_text(&root, "MFADelete").map(|v| v.eq_ignore_ascii_case("Enabled"));
5758

5859
Ok(Self {
5960
headers,

src/s3/response/get_object.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ impl FromS3Response for GetObjectResponse {
4444
) -> Result<Self, Error> {
4545
let mut resp = resp?;
4646

47-
let headers = mem::take(resp.headers_mut());
47+
let headers: HeaderMap = mem::take(resp.headers_mut());
4848

4949
let etag: Option<String> = headers
5050
.get("etag")

src/s3/response/get_object_lock_config.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ impl FromS3Response for GetObjectLockConfigResponse {
4242
) -> Result<Self, Error> {
4343
let mut resp = resp?;
4444

45-
let headers = mem::take(resp.headers_mut());
45+
let headers: HeaderMap = mem::take(resp.headers_mut());
4646
let body = resp.bytes().await?;
4747
let root = Element::parse(body.reader())?;
4848

src/s3/response/list_objects.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -208,7 +208,7 @@ impl FromS3Response for ListObjectsV1Response {
208208
resp: Result<reqwest::Response, Error>,
209209
) -> Result<Self, Error> {
210210
let mut resp = resp?;
211-
let headers = mem::take(resp.headers_mut());
211+
let headers: HeaderMap = mem::take(resp.headers_mut());
212212
let body = resp.bytes().await?;
213213
let xmltree_root = xmltree::Element::parse(body.reader())?;
214214
let root = Element::from(&xmltree_root);

src/s3/response/object_prompt.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ impl FromS3Response for ObjectPromptResponse {
3737
) -> Result<Self, Error> {
3838
let mut resp = resp?;
3939

40-
let headers = mem::take(resp.headers_mut());
40+
let headers: HeaderMap = mem::take(resp.headers_mut());
4141
let body = resp.bytes().await?;
4242
let prompt_response: String = String::from_utf8(body.to_vec())?;
4343

src/s3/response/put_object.rs

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -85,12 +85,6 @@ impl FromS3Response for CreateMultipartUploadResponse {
8585
req: S3Request,
8686
resp: Result<reqwest::Response, Error>,
8787
) -> Result<Self, Error> {
88-
let bucket = req
89-
.bucket
90-
.ok_or_else(|| Error::InvalidBucketName("no bucket specified".into()))?;
91-
let object = req
92-
.object
93-
.ok_or_else(|| Error::InvalidObjectName("no object specified".into()))?;
9488
let mut resp = resp?;
9589

9690
let headers: HeaderMap = mem::take(resp.headers_mut());
@@ -99,11 +93,11 @@ impl FromS3Response for CreateMultipartUploadResponse {
9993
let upload_id: String =
10094
get_text(&root, "UploadId").map_err(|e| Error::InvalidUploadId(e.to_string()))?;
10195

102-
Ok(CreateMultipartUploadResponse {
96+
Ok(Self {
10397
headers,
10498
region: req.inner_region,
105-
bucket,
106-
object,
99+
bucket: take_bucket(req.bucket)?,
100+
object: take_object(req.object)?,
107101
upload_id,
108102
})
109103
}

src/s3/utils.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -493,7 +493,7 @@ pub mod xml {
493493

494494
impl Element<'_> {
495495
pub fn name(&self) -> &str {
496-
self.inner.name.as_str()
496+
&self.inner.name
497497
}
498498

499499
pub fn get_child_text(&self, tag: &str) -> Option<String> {

tests/test_append_object.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -351,7 +351,7 @@ async fn append_object_content_3() {
351351
assert_eq!(resp.size, sizes[idx] + initial_size);
352352
assert_eq!(resp.etag, etag);
353353
client
354-
.remove_object(&test_bucket, object_name.as_str())
354+
.remove_object(&test_bucket, &object_name)
355355
.send()
356356
.await
357357
.unwrap();

tests/test_object_put.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -192,7 +192,7 @@ async fn put_object_content_2() {
192192
assert_eq!(resp.size, sizes[idx]);
193193
assert_eq!(resp.etag, etag);
194194
client
195-
.remove_object(&test_bucket, object_name.as_str())
195+
.remove_object(&test_bucket, &object_name)
196196
.send()
197197
.await
198198
.unwrap();

0 commit comments

Comments
 (0)