diff --git a/mountpoint-s3-client/CHANGELOG.md b/mountpoint-s3-client/CHANGELOG.md index f1e064f52..3ccd79365 100644 --- a/mountpoint-s3-client/CHANGELOG.md +++ b/mountpoint-s3-client/CHANGELOG.md @@ -26,6 +26,10 @@ * Both `ObjectInfo` and `ChecksumAlgorithm` structs are now marked `non_exhaustive`, to indicate that new fields may be added in the future. `ChecksumAlgorithm` no longer implements `Copy`. ([#1086](https://github.com/awslabs/mountpoint-s3/pull/1086)) +* `get_object` method now requires a `GetObjectParams` parameter. + Two of the existing parameters, `range` and `if_match` have been moved to `GetObjectParams`. + ([#1121](https://github.com/awslabs/mountpoint-s3/pull/1121)) + ## v0.11.0 (October 17, 2024) diff --git a/mountpoint-s3-client/examples/client_benchmark.rs b/mountpoint-s3-client/examples/client_benchmark.rs index aca5b1563..32915125c 100644 --- a/mountpoint-s3-client/examples/client_benchmark.rs +++ b/mountpoint-s3-client/examples/client_benchmark.rs @@ -9,7 +9,7 @@ use futures::StreamExt; use mountpoint_s3_client::config::{EndpointConfig, S3ClientConfig}; use mountpoint_s3_client::mock_client::throughput_client::ThroughputMockClient; use mountpoint_s3_client::mock_client::{MockClientConfig, MockObject}; -use mountpoint_s3_client::types::ETag; +use mountpoint_s3_client::types::{ETag, GetObjectParams}; use mountpoint_s3_client::{ObjectClient, S3CrtClient}; use mountpoint_s3_crt::common::rust_log_adapter::RustLogAdapter; use tracing_subscriber::fmt::Subscriber; @@ -46,7 +46,7 @@ fn run_benchmark( scope.spawn(|| { futures::executor::block_on(async move { let mut request = client - .get_object(bucket, key, None, None) + .get_object(bucket, key, &GetObjectParams::new()) .await .expect("couldn't create get request"); let mut request = pin!(request); diff --git a/mountpoint-s3-client/examples/download.rs b/mountpoint-s3-client/examples/download.rs index 7332fc36b..23b48bda1 100644 --- a/mountpoint-s3-client/examples/download.rs +++ b/mountpoint-s3-client/examples/download.rs @@ -4,6 +4,7 @@ use std::sync::{Arc, Mutex}; use clap::{Arg, Command}; use futures::StreamExt; use mountpoint_s3_client::config::{EndpointConfig, S3ClientConfig}; +use mountpoint_s3_client::types::GetObjectParams; use mountpoint_s3_client::{ObjectClient, S3CrtClient}; use mountpoint_s3_crt::common::rust_log_adapter::RustLogAdapter; use regex::Regex; @@ -58,7 +59,7 @@ fn main() { let last_offset_clone = Arc::clone(&last_offset); futures::executor::block_on(async move { let mut request = client - .get_object(bucket, key, range, None) + .get_object(bucket, key, &GetObjectParams::new().range(range)) .await .expect("couldn't create get request"); loop { diff --git a/mountpoint-s3-client/src/failure_client.rs b/mountpoint-s3-client/src/failure_client.rs index 37f26ca86..4db79dbde 100644 --- a/mountpoint-s3-client/src/failure_client.rs +++ b/mountpoint-s3-client/src/failure_client.rs @@ -4,7 +4,6 @@ use std::collections::HashMap; use std::fmt::Debug; -use std::ops::Range; use std::pin::Pin; use std::sync::Mutex; use std::task::{Context, Poll}; @@ -15,11 +14,11 @@ use mountpoint_s3_crt::s3::client::BufferPoolUsageStats; use pin_project::pin_project; use crate::object_client::{ - CopyObjectError, CopyObjectParams, CopyObjectResult, DeleteObjectError, DeleteObjectResult, ETag, GetBodyPart, - GetObjectAttributesError, GetObjectAttributesResult, GetObjectError, GetObjectRequest, HeadObjectError, - HeadObjectParams, HeadObjectResult, ListObjectsError, ListObjectsResult, ObjectAttribute, ObjectClient, - ObjectClientError, ObjectClientResult, ObjectMetadata, PutObjectError, PutObjectParams, PutObjectRequest, - PutObjectResult, PutObjectSingleParams, UploadReview, + CopyObjectError, CopyObjectParams, CopyObjectResult, DeleteObjectError, DeleteObjectResult, GetBodyPart, + GetObjectAttributesError, GetObjectAttributesResult, GetObjectError, GetObjectParams, GetObjectRequest, + HeadObjectError, HeadObjectParams, HeadObjectResult, ListObjectsError, ListObjectsResult, ObjectAttribute, + ObjectClient, ObjectClientError, ObjectClientResult, ObjectMetadata, PutObjectError, PutObjectParams, + PutObjectRequest, PutObjectResult, PutObjectSingleParams, UploadReview, }; // Wrapper for injecting failures into a get stream or a put request @@ -36,8 +35,7 @@ pub struct FailureClient { &mut State, &str, &str, - Option>, - Option, + &GetObjectParams, ) -> Result< FailureRequestWrapper, ObjectClientError, @@ -123,17 +121,10 @@ where &self, bucket: &str, key: &str, - range: Option>, - if_match: Option, + params: &GetObjectParams, ) -> ObjectClientResult { - let wrapper = (self.get_object_cb)( - &mut *self.state.lock().unwrap(), - bucket, - key, - range.clone(), - if_match.clone(), - )?; - let request = self.client.get_object(bucket, key, range, if_match).await?; + let wrapper = (self.get_object_cb)(&mut *self.state.lock().unwrap(), bucket, key, params)?; + let request = self.client.get_object(bucket, key, params).await?; Ok(FailureGetRequest { state: wrapper.state, result_fn: wrapper.result_fn, @@ -364,7 +355,7 @@ pub fn countdown_failure_client( FailureClient { client, state, - get_object_cb: |state, _bucket, _key, _range, _if_match| { + get_object_cb: |state, _bucket, _key, _get_object_params| { state.get_count += 1; let (fail_count, error) = if let Some(result) = state.get_failures.remove(&state.get_count) { let (fail_count, error) = result?; @@ -443,6 +434,7 @@ pub fn countdown_failure_client( mod tests { use super::*; use crate::mock_client::{MockClient, MockClientConfig, MockClientError, MockObject}; + use crate::types::ETag; use std::collections::HashSet; #[tokio::test] @@ -486,7 +478,7 @@ mod tests { let fail_set = HashSet::from([2, 4, 5]); for i in 1..=6 { - let r = fail_client.get_object(bucket, key, None, None).await; + let r = fail_client.get_object(bucket, key, &GetObjectParams::new()).await; if fail_set.contains(&i) { assert!(r.is_err()); } else { diff --git a/mountpoint-s3-client/src/lib.rs b/mountpoint-s3-client/src/lib.rs index 2aa3c318e..2790b46d6 100644 --- a/mountpoint-s3-client/src/lib.rs +++ b/mountpoint-s3-client/src/lib.rs @@ -21,7 +21,7 @@ //! //! let client = S3CrtClient::new(Default::default()).expect("client construction failed"); //! -//! let response = client.get_object("my-bucket", "my-key", None, None).await.expect("get_object failed"); +//! let response = client.get_object("my-bucket", "my-key", &GetObjectParams::new().await.expect("get_object failed")); //! let body = response.map_ok(|(offset, body)| body.to_vec()).try_concat().await.expect("body streaming failed"); //! # } //! ``` @@ -73,9 +73,9 @@ pub mod config { pub mod types { pub use super::object_client::{ Checksum, ChecksumAlgorithm, ChecksumMode, CopyObjectParams, CopyObjectResult, DeleteObjectResult, ETag, - GetBodyPart, GetObjectAttributesParts, GetObjectAttributesResult, GetObjectRequest, HeadObjectParams, - HeadObjectResult, ListObjectsResult, ObjectAttribute, ObjectClientResult, ObjectInfo, ObjectPart, - PutObjectParams, PutObjectResult, PutObjectSingleParams, PutObjectTrailingChecksums, RestoreStatus, + GetBodyPart, GetObjectAttributesParts, GetObjectAttributesResult, GetObjectParams, GetObjectRequest, + HeadObjectParams, HeadObjectResult, ListObjectsResult, ObjectAttribute, ObjectClientResult, ObjectInfo, + ObjectPart, PutObjectParams, PutObjectResult, PutObjectSingleParams, PutObjectTrailingChecksums, RestoreStatus, UploadChecksum, UploadReview, UploadReviewPart, }; } diff --git a/mountpoint-s3-client/src/mock_client.rs b/mountpoint-s3-client/src/mock_client.rs index faa8eebc4..7cc5b13cb 100644 --- a/mountpoint-s3-client/src/mock_client.rs +++ b/mountpoint-s3-client/src/mock_client.rs @@ -5,7 +5,6 @@ use std::borrow::Cow; use std::collections::{BTreeMap, BTreeSet, HashMap, HashSet}; use std::fmt::Write; -use std::ops::Range; use std::pin::Pin; use std::sync::{Arc, RwLock}; use std::task::{Context, Poll}; @@ -28,10 +27,11 @@ use crate::error_metadata::{ClientErrorMetadata, ProvideErrorMetadata}; use crate::object_client::{ Checksum, ChecksumAlgorithm, ChecksumMode, CopyObjectError, CopyObjectParams, CopyObjectResult, DeleteObjectError, DeleteObjectResult, ETag, GetBodyPart, GetObjectAttributesError, GetObjectAttributesParts, - GetObjectAttributesResult, GetObjectError, GetObjectRequest, HeadObjectError, HeadObjectParams, HeadObjectResult, - ListObjectsError, ListObjectsResult, ObjectAttribute, ObjectClient, ObjectClientError, ObjectClientResult, - ObjectInfo, ObjectMetadata, ObjectPart, PutObjectError, PutObjectParams, PutObjectRequest, PutObjectResult, - PutObjectSingleParams, PutObjectTrailingChecksums, RestoreStatus, UploadChecksum, UploadReview, UploadReviewPart, + GetObjectAttributesResult, GetObjectError, GetObjectParams, GetObjectRequest, HeadObjectError, HeadObjectParams, + HeadObjectResult, ListObjectsError, ListObjectsResult, ObjectAttribute, ObjectClient, ObjectClientError, + ObjectClientResult, ObjectInfo, ObjectMetadata, ObjectPart, PutObjectError, PutObjectParams, PutObjectRequest, + PutObjectResult, PutObjectSingleParams, PutObjectTrailingChecksums, RestoreStatus, UploadChecksum, UploadReview, + UploadReviewPart, }; mod leaky_bucket; @@ -660,10 +660,9 @@ impl ObjectClient for MockClient { &self, bucket: &str, key: &str, - range: Option>, - if_match: Option, + params: &GetObjectParams, ) -> ObjectClientResult { - trace!(bucket, key, ?range, ?if_match, "GetObject"); + trace!(bucket, key, ?params.range, ?params.if_match, "GetObject"); self.inc_op_count(Operation::GetObject); if bucket != self.config.bucket { @@ -673,13 +672,13 @@ impl ObjectClient for MockClient { let objects = self.objects.read().unwrap(); if let Some(object) = objects.get(key) { - if let Some(etag_match) = if_match { - if etag_match != object.etag { + if let Some(etag_match) = params.if_match.as_ref() { + if etag_match != &object.etag { return Err(ObjectClientError::ServiceError(GetObjectError::PreconditionFailed)); } } - let (next_offset, length) = if let Some(range) = range { + let (next_offset, length) = if let Some(range) = params.range.as_ref() { if range.start >= object.len() as u64 || range.end > object.len() as u64 { return mock_client_error(format!("invalid range, length={}", object.len())); } @@ -1050,6 +1049,7 @@ mod tests { use futures::{pin_mut, StreamExt}; use rand::{Rng, RngCore, SeedableRng}; use rand_chacha::ChaChaRng; + use std::ops::Range; use test_case::test_case; use super::*; @@ -1089,7 +1089,7 @@ mod tests { client.add_object(key, object); let mut get_request = client - .get_object("test_bucket", key, range.clone(), None) + .get_object("test_bucket", key, &GetObjectParams::new().range(range.clone())) .await .expect("should not fail"); @@ -1143,7 +1143,7 @@ mod tests { client.add_object(key, MockObject::from_bytes(&body, ETag::for_tests())); let get_request = client - .get_object("test_bucket", key, range.clone(), None) + .get_object("test_bucket", key, &GetObjectParams::new().range(range.clone())) .await .expect("should not fail"); pin_mut!(get_request); @@ -1191,33 +1191,45 @@ mod tests { client.add_object("key1", body[..].into()); assert!(matches!( - client.get_object("wrong_bucket", "key1", None, None).await, + client.get_object("wrong_bucket", "key1", &GetObjectParams::new()).await, Err(ObjectClientError::ServiceError(GetObjectError::NoSuchBucket)) )); assert!(matches!( - client.get_object("test_bucket", "wrong_key", None, None).await, + client + .get_object("test_bucket", "wrong_key", &GetObjectParams::new()) + .await, Err(ObjectClientError::ServiceError(GetObjectError::NoSuchKey)) )); assert_client_error!( - client.get_object("test_bucket", "key1", Some(0..2001), None).await, + client + .get_object("test_bucket", "key1", &GetObjectParams::new().range(Some(0..2001))) + .await, "invalid range, length=2000" ); assert_client_error!( - client.get_object("test_bucket", "key1", Some(2000..2000), None).await, + client + .get_object("test_bucket", "key1", &GetObjectParams::new().range(Some(2000..2000))) + .await, "invalid range, length=2000" ); assert_client_error!( - client.get_object("test_bucket", "key1", Some(500..2001), None).await, + client + .get_object("test_bucket", "key1", &GetObjectParams::new().range(Some(500..2001))) + .await, "invalid range, length=2000" ); assert_client_error!( - client.get_object("test_bucket", "key1", Some(5000..2001), None).await, + client + .get_object("test_bucket", "key1", &GetObjectParams::new().range(Some(5000..2001))) + .await, "invalid range, length=2000" ); assert_client_error!( - client.get_object("test_bucket", "key1", Some(5000..1), None).await, + client + .get_object("test_bucket", "key1", &GetObjectParams::new().range(Some(5000..1))) + .await, "invalid range, length=2000" ); } @@ -1245,7 +1257,7 @@ mod tests { client.add_object(key, MockObject::from_bytes(&expected_body, ETag::for_tests())); let mut get_request = client - .get_object("test_bucket", key, Some(range.clone()), None) + .get_object("test_bucket", key, &GetObjectParams::new().range(Some(range.clone()))) .await .expect("should not fail"); @@ -1282,7 +1294,7 @@ mod tests { .expect("Should not fail"); client - .get_object(bucket, dst_key, None, None) + .get_object(bucket, dst_key, &GetObjectParams::new()) .await .expect("get_object should succeed"); } @@ -1706,7 +1718,7 @@ mod tests { put_request.complete().await.expect("put_object failed"); let mut get_request = client - .get_object("test_bucket", "key1", None, None) + .get_object("test_bucket", "key1", &GetObjectParams::new()) .await .expect("get_object failed"); @@ -1739,7 +1751,7 @@ mod tests { .expect("put_object failed"); let get_request = client - .get_object("test_bucket", "key1", None, None) + .get_object("test_bucket", "key1", &GetObjectParams::new()) .await .expect("get_object failed"); diff --git a/mountpoint-s3-client/src/mock_client/throughput_client.rs b/mountpoint-s3-client/src/mock_client/throughput_client.rs index 5d6b3ba4d..dc4671503 100644 --- a/mountpoint-s3-client/src/mock_client/throughput_client.rs +++ b/mountpoint-s3-client/src/mock_client/throughput_client.rs @@ -1,4 +1,3 @@ -use std::ops::Range; use std::pin::Pin; use std::task::{Context, Poll}; use std::time::Duration; @@ -14,10 +13,11 @@ use crate::mock_client::{ MockClient, MockClientConfig, MockClientError, MockGetObjectRequest, MockObject, MockPutObjectRequest, }; use crate::object_client::{ - CopyObjectError, CopyObjectParams, CopyObjectResult, DeleteObjectError, DeleteObjectResult, ETag, GetBodyPart, - GetObjectAttributesError, GetObjectAttributesResult, GetObjectError, GetObjectRequest, HeadObjectError, - HeadObjectParams, HeadObjectResult, ListObjectsError, ListObjectsResult, ObjectAttribute, ObjectClient, - ObjectClientResult, ObjectMetadata, PutObjectError, PutObjectParams, PutObjectResult, PutObjectSingleParams, + CopyObjectError, CopyObjectParams, CopyObjectResult, DeleteObjectError, DeleteObjectResult, GetBodyPart, + GetObjectAttributesError, GetObjectAttributesResult, GetObjectError, GetObjectParams, GetObjectRequest, + HeadObjectError, HeadObjectParams, HeadObjectResult, ListObjectsError, ListObjectsResult, ObjectAttribute, + ObjectClient, ObjectClientResult, ObjectMetadata, PutObjectError, PutObjectParams, PutObjectResult, + PutObjectSingleParams, }; /// A [MockClient] that rate limits overall download throughput to simulate a target network @@ -148,10 +148,9 @@ impl ObjectClient for ThroughputMockClient { &self, bucket: &str, key: &str, - range: Option>, - if_match: Option, + params: &GetObjectParams, ) -> ObjectClientResult { - let request = self.inner.get_object(bucket, key, range, if_match).await?; + let request = self.inner.get_object(bucket, key, params).await?; let rate_limiter = self.rate_limiter.clone(); Ok(ThroughputGetObjectRequest { request, rate_limiter }) } @@ -219,6 +218,7 @@ mod tests { use futures::StreamExt; use crate::mock_client::MockObject; + use crate::types::ETag; use super::*; @@ -245,7 +245,10 @@ mod tests { let start = Instant::now(); let num_bytes = block_on(async move { let mut num_bytes = 0; - let mut get = client.get_object("test_bucket", "testfile", None, None).await.unwrap(); + let mut get = client + .get_object("test_bucket", "testfile", &GetObjectParams::new()) + .await + .unwrap(); while let Some(part) = get.next().await { let (_offset, part) = part.unwrap(); num_bytes += part.len(); diff --git a/mountpoint-s3-client/src/object_client.rs b/mountpoint-s3-client/src/object_client.rs index 69f8d3276..e64e3fd4c 100644 --- a/mountpoint-s3-client/src/object_client.rs +++ b/mountpoint-s3-client/src/object_client.rs @@ -78,8 +78,7 @@ pub trait ObjectClient { &self, bucket: &str, key: &str, - range: Option>, - if_match: Option, + params: &GetObjectParams, ) -> ObjectClientResult; /// List the objects in a bucket under a given prefix @@ -184,6 +183,33 @@ pub enum GetObjectError { PreconditionFailed, } +/// Parameters to a [`get_object`](ObjectClient::get_object) request +#[derive(Debug, Default, Clone)] +#[non_exhaustive] +pub struct GetObjectParams { + pub range: Option>, + pub if_match: Option, +} + +impl GetObjectParams { + /// Create a default [GetObjectParams]. + pub fn new() -> Self { + Self::default() + } + + /// Set the range retrieved by the GetObject request + pub fn range(mut self, value: Option>) -> Self { + self.range = value; + self + } + + /// Set the required etag on the object + pub fn if_match(mut self, value: Option) -> Self { + self.if_match = value; + self + } +} + /// Result of a [`list_objects`](ObjectClient::list_objects) request #[derive(Debug)] #[non_exhaustive] diff --git a/mountpoint-s3-client/src/s3_crt_client.rs b/mountpoint-s3-client/src/s3_crt_client.rs index fed1c3b7f..a9817f71e 100644 --- a/mountpoint-s3-client/src/s3_crt_client.rs +++ b/mountpoint-s3-client/src/s3_crt_client.rs @@ -1281,12 +1281,9 @@ impl ObjectClient for S3CrtClient { &self, bucket: &str, key: &str, - range: Option>, - if_match: Option, - // TODO: If more arguments are added to get object, make a request struct having those arguments - // along with bucket and key. + params: &GetObjectParams, ) -> ObjectClientResult { - self.get_object(bucket, key, range, if_match) + self.get_object(bucket, key, params) } async fn list_objects( diff --git a/mountpoint-s3-client/src/s3_crt_client/get_object.rs b/mountpoint-s3-client/src/s3_crt_client/get_object.rs index baf9566e7..710e01769 100644 --- a/mountpoint-s3-client/src/s3_crt_client/get_object.rs +++ b/mountpoint-s3-client/src/s3_crt_client/get_object.rs @@ -2,7 +2,6 @@ use async_cell::sync::AsyncCell; use async_trait::async_trait; use std::future::Future; use std::ops::Deref; -use std::ops::Range; use std::os::unix::prelude::OsStrExt; use std::pin::Pin; use std::sync::Arc; @@ -16,7 +15,9 @@ use mountpoint_s3_crt::s3::client::MetaRequestResult; use pin_project::pin_project; use thiserror::Error; -use crate::object_client::{ETag, GetBodyPart, GetObjectError, ObjectClientError, ObjectClientResult, ObjectMetadata}; +use crate::object_client::{ + GetBodyPart, GetObjectError, GetObjectParams, ObjectClientError, ObjectClientResult, ObjectMetadata, +}; use crate::s3_crt_client::{ GetObjectRequest, S3CrtClient, S3CrtClientInner, S3HttpRequest, S3Operation, S3RequestError, }; @@ -35,10 +36,9 @@ impl S3CrtClient { &self, bucket: &str, key: &str, - range: Option>, - if_match: Option, + params: &GetObjectParams, ) -> Result> { - let span = request_span!(self.inner, "get_object", bucket, key, ?range, ?if_match); + let span = request_span!(self.inner, "get_object", bucket, key, range=?params.range, if_match=?params.if_match); let mut message = self .inner @@ -50,14 +50,14 @@ impl S3CrtClient { .set_header(&Header::new("accept", "*/*")) .map_err(S3RequestError::construction_failure)?; - if let Some(etag) = if_match { + if let Some(etag) = params.if_match.as_ref() { // Return the object only if its entity tag (ETag) is matched message .set_header(&Header::new("If-Match", etag.as_str())) .map_err(S3RequestError::construction_failure)?; } - let next_offset = if let Some(range) = range { + let next_offset = if let Some(range) = params.range.as_ref() { // Range HTTP header is bounded below *inclusive* let range_value = format!("bytes={}-{}", range.start, range.end.saturating_sub(1)); message diff --git a/mountpoint-s3-client/tests/auth.rs b/mountpoint-s3-client/tests/auth.rs index 524632763..15755ef96 100644 --- a/mountpoint-s3-client/tests/auth.rs +++ b/mountpoint-s3-client/tests/auth.rs @@ -15,6 +15,7 @@ use common::*; use futures::StreamExt; use mountpoint_s3_client::config::{S3ClientAuthConfig, S3ClientConfig}; use mountpoint_s3_client::error::ObjectClientError; +use mountpoint_s3_client::types::GetObjectParams; #[cfg(not(feature = "s3express_tests"))] use mountpoint_s3_client::S3RequestError; use mountpoint_s3_client::{ObjectClient, S3CrtClient}; @@ -56,7 +57,7 @@ async fn test_static_provider() { let client = S3CrtClient::new(config).unwrap(); let result = client - .get_object(&bucket, &key, None, None) + .get_object(&bucket, &key, &GetObjectParams::new()) .await .expect("get_object should succeed"); check_get_result(result, None, &body[..]).await; @@ -76,7 +77,7 @@ async fn test_static_provider() { let client = S3CrtClient::new(config).unwrap(); let mut request = client - .get_object(&bucket, &key, None, None) + .get_object(&bucket, &key, &GetObjectParams::new()) .await .expect("get_object request should be sent"); @@ -138,7 +139,7 @@ async fn test_profile_provider_static_async() { let client = S3CrtClient::new(config).unwrap(); let result = client - .get_object(&bucket, &key, None, None) + .get_object(&bucket, &key, &GetObjectParams::new()) .await .expect("get_object should succeed"); check_get_result(result, None, &body[..]).await; @@ -154,7 +155,7 @@ async fn test_profile_provider_static_async() { let client = S3CrtClient::new(config).unwrap(); let mut request = client - .get_object(&bucket, &key, None, None) + .get_object(&bucket, &key, &GetObjectParams::new()) .await .expect("get_object should be sent"); @@ -219,7 +220,7 @@ async fn test_profile_provider_assume_role_async() { let client = S3CrtClient::new(config).unwrap(); let mut request = client - .get_object(&bucket, &key, None, None) + .get_object(&bucket, &key, &GetObjectParams::new()) .await .expect("get_object should be sent"); @@ -234,7 +235,7 @@ async fn test_profile_provider_assume_role_async() { let client = S3CrtClient::new(config).unwrap(); let result = client - .get_object(&bucket, &key, None, None) + .get_object(&bucket, &key, &GetObjectParams::new()) .await .expect("get_object should succeed"); check_get_result(result, None, &body[..]).await; @@ -430,7 +431,7 @@ async fn test_scoped_credentials() { // Inside the prefix, things should be fine let _result = client - .get_object(&bucket, &format!("{prefix}foo/foo.txt"), None, None) + .get_object(&bucket, &format!("{prefix}foo/foo.txt"), &GetObjectParams::new()) .await .expect("get_object should succeed"); let _result = client @@ -440,7 +441,7 @@ async fn test_scoped_credentials() { // Outside the prefix, requests should fail with permissions errors let mut request = client - .get_object(&bucket, &format!("{prefix}baz.txt"), None, None) + .get_object(&bucket, &format!("{prefix}baz.txt"), &GetObjectParams::new()) .await .expect("request should be sent"); let err = request diff --git a/mountpoint-s3-client/tests/endpoint_config.rs b/mountpoint-s3-client/tests/endpoint_config.rs index 59e0af7d9..4e1e30ee2 100644 --- a/mountpoint-s3-client/tests/endpoint_config.rs +++ b/mountpoint-s3-client/tests/endpoint_config.rs @@ -6,6 +6,7 @@ use aws_sdk_s3::primitives::ByteStream; use bytes::Bytes; use common::*; use mountpoint_s3_client::config::{AddressingStyle, EndpointConfig, S3ClientConfig}; +use mountpoint_s3_client::types::GetObjectParams; use mountpoint_s3_client::{ObjectClient, S3CrtClient}; use test_case::test_case; @@ -28,7 +29,7 @@ async fn run_test(endpoint_config: EndpointConfig, prefix: &str, bucket: String) let client = S3CrtClient::new(config).expect("could not create test client"); let result = client - .get_object(&bucket, &key, None, None) + .get_object(&bucket, &key, &GetObjectParams::new()) .await .expect("get_object should succeed"); check_get_result(result, None, &body[..]).await; diff --git a/mountpoint-s3-client/tests/get_object.rs b/mountpoint-s3-client/tests/get_object.rs index 2ec30007f..360fde1e2 100644 --- a/mountpoint-s3-client/tests/get_object.rs +++ b/mountpoint-s3-client/tests/get_object.rs @@ -13,7 +13,7 @@ use common::*; use futures::pin_mut; use futures::stream::StreamExt; use mountpoint_s3_client::error::{GetObjectError, ObjectClientError}; -use mountpoint_s3_client::types::{ETag, GetObjectRequest}; +use mountpoint_s3_client::types::{ETag, GetObjectParams, GetObjectRequest}; use mountpoint_s3_client::{ObjectClient, S3CrtClient, S3RequestError}; use test_case::test_case; @@ -44,7 +44,7 @@ async fn test_get_object(size: usize, range: Option>) { let client: S3CrtClient = get_test_client(); let result = client - .get_object(&bucket, &key, range.clone(), None) + .get_object(&bucket, &key, &GetObjectParams::new().range(range.clone())) .await .expect("get_object should succeed"); let expected = match range { @@ -81,7 +81,7 @@ async fn test_get_object_backpressure(size: usize, range: Option>) { let client: S3CrtClient = get_test_backpressure_client(initial_window_size, None); let request = client - .get_object(&bucket, &key, range.clone(), None) + .get_object(&bucket, &key, &GetObjectParams::new().range(range.clone())) .await .expect("get_object should succeed"); let expected = match range { @@ -115,7 +115,7 @@ async fn verify_backpressure_get_object() { .unwrap(); let mut get_request = client - .get_object(&bucket, &key, Some(range.clone()), None) + .get_object(&bucket, &key, &GetObjectParams::new().range(Some(range.clone()))) .await .expect("should not fail"); @@ -159,7 +159,7 @@ async fn test_mutated_during_get_object_backpressure() { .unwrap(); let mut get_request = client - .get_object(&bucket, &key, Some(range.clone()), None) + .get_object(&bucket, &key, &GetObjectParams::new().range(Some(range.clone()))) .await .expect("should not fail"); @@ -202,7 +202,7 @@ async fn test_get_object_404_key() { let client: S3CrtClient = get_test_client(); let mut result = client - .get_object(&bucket, &key, None, None) + .get_object(&bucket, &key, &GetObjectParams::new()) .await .expect("get_object should succeed"); let next = StreamExt::next(&mut result).await.expect("stream needs to return Err"); @@ -223,7 +223,7 @@ async fn test_get_object_404_bucket() { let client: S3CrtClient = get_test_client(); let mut result = client - .get_object("amzn-s3-demo-bucket", &key, None, None) + .get_object("amzn-s3-demo-bucket", &key, &GetObjectParams::new()) .await .expect("get_object failed"); let next = StreamExt::next(&mut result).await.expect("stream needs to return Err"); @@ -255,7 +255,7 @@ async fn test_get_object_success_if_match() { let etag = Some(ETag::from_str(response.e_tag().expect("E-Tag should be set")).unwrap()); let result = client - .get_object(&bucket, &key, None, etag) + .get_object(&bucket, &key, &GetObjectParams::new().if_match(etag)) .await .expect("get_object should succeed"); check_get_result(result, None, &body[..]).await; @@ -282,7 +282,7 @@ async fn test_get_object_412_if_match() { let etag = Some(ETag::from_str("incorrect_etag").unwrap()); let mut result = client - .get_object(&bucket, &key, None, etag) + .get_object(&bucket, &key, &GetObjectParams::new().if_match(etag)) .await .expect("get_object should succeed"); @@ -318,7 +318,7 @@ async fn test_get_object_cancel(read: bool) { let client: S3CrtClient = get_test_client(); let mut request = client - .get_object(&bucket, &key, None, None) + .get_object(&bucket, &key, &GetObjectParams::new()) .await .expect("get_object should succeed"); @@ -363,7 +363,7 @@ async fn test_get_object_user_metadata(size: usize, metadata: HashMap( bucket: &str, key: &str, ) -> ObjectClientResult<(), GetObjectError, Client::ClientError> { - let result = client.get_object(bucket, key, None, None).await?; + let result = client.get_object(bucket, key, &GetObjectParams::new()).await?; pin_mut!(result); result.next().await.unwrap()?; Ok(()) diff --git a/mountpoint-s3-client/tests/put_object_single.rs b/mountpoint-s3-client/tests/put_object_single.rs index 5674af11c..28d8dda69 100644 --- a/mountpoint-s3-client/tests/put_object_single.rs +++ b/mountpoint-s3-client/tests/put_object_single.rs @@ -7,7 +7,9 @@ use std::collections::HashMap; use common::*; use mountpoint_s3_client::checksums::{crc32c, crc32c_to_base64}; use mountpoint_s3_client::config::S3ClientConfig; -use mountpoint_s3_client::types::{ChecksumAlgorithm, PutObjectResult, PutObjectSingleParams, UploadChecksum}; +use mountpoint_s3_client::types::{ + ChecksumAlgorithm, GetObjectParams, PutObjectResult, PutObjectSingleParams, UploadChecksum, +}; use mountpoint_s3_client::{ObjectClient, S3CrtClient}; use rand::Rng; use test_case::test_case; @@ -31,7 +33,11 @@ async fn test_put_object_single( .expect("put_object should succeed"); let result = client - .get_object(bucket, key, None, Some(put_object_result.etag.clone())) + .get_object( + bucket, + key, + &GetObjectParams::new().if_match(Some(put_object_result.etag.clone())), + ) .await .expect("get_object should succeed"); check_get_result(result, None, &contents[..]).await; @@ -55,7 +61,11 @@ async fn test_put_object_single_empty( .expect("put_object should succeed"); let result = client - .get_object(bucket, key, None, Some(put_object_result.etag.clone())) + .get_object( + bucket, + key, + &GetObjectParams::new().if_match(Some(put_object_result.etag.clone())), + ) .await .expect("get_object should succeed"); check_get_result(result, None, &[]).await; diff --git a/mountpoint-s3/src/data_cache/express_data_cache.rs b/mountpoint-s3/src/data_cache/express_data_cache.rs index 99267b62a..296121f85 100644 --- a/mountpoint-s3/src/data_cache/express_data_cache.rs +++ b/mountpoint-s3/src/data_cache/express_data_cache.rs @@ -6,7 +6,7 @@ use async_trait::async_trait; use bytes::BytesMut; use futures::{pin_mut, StreamExt}; use mountpoint_s3_client::error::{GetObjectError, ObjectClientError}; -use mountpoint_s3_client::types::{GetObjectRequest, PutObjectParams}; +use mountpoint_s3_client::types::{GetObjectParams, GetObjectRequest, PutObjectParams}; use mountpoint_s3_client::{ObjectClient, PutObjectRequest}; use sha2::{Digest, Sha256}; use tracing::Instrument; @@ -71,7 +71,11 @@ where } let object_key = block_key(&self.prefix, cache_key, block_idx); - let result = match self.client.get_object(&self.bucket_name, &object_key, None, None).await { + let result = match self + .client + .get_object(&self.bucket_name, &object_key, &GetObjectParams::new()) + .await + { Ok(result) => result, Err(ObjectClientError::ServiceError(GetObjectError::NoSuchKey)) => return Ok(None), Err(e) => return Err(e.into()), diff --git a/mountpoint-s3/src/prefetch/part_stream.rs b/mountpoint-s3/src/prefetch/part_stream.rs index b5eb32e01..e4ab243a0 100644 --- a/mountpoint-s3/src/prefetch/part_stream.rs +++ b/mountpoint-s3/src/prefetch/part_stream.rs @@ -2,7 +2,7 @@ use async_stream::try_stream; use bytes::Bytes; use futures::task::{Spawn, SpawnExt}; use futures::{pin_mut, Stream, StreamExt}; -use mountpoint_s3_client::{types::GetObjectRequest, ObjectClient}; +use mountpoint_s3_client::{types::GetObjectParams, types::GetObjectRequest, ObjectClient}; use std::marker::{Send, Sync}; use std::sync::Arc; use std::{fmt::Debug, ops::Range}; @@ -360,7 +360,7 @@ fn read_from_request<'a, Client: ObjectClient + 'a>( ) -> impl Stream> + 'a { try_stream! { let request = client - .get_object(&bucket, id.key(), Some(request_range.clone()), Some(id.etag().clone())) + .get_object(&bucket, id.key(), &GetObjectParams::new().range(Some(request_range.clone())).if_match(Some(id.etag().clone()))) .await .inspect_err(|e| error!(key=id.key(), error=?e, "GetObject request failed")) .map_err(PrefetchReadError::GetRequestFailed)?; diff --git a/mountpoint-s3/tests/fs.rs b/mountpoint-s3/tests/fs.rs index 8f49eb81e..33e53a518 100644 --- a/mountpoint-s3/tests/fs.rs +++ b/mountpoint-s3/tests/fs.rs @@ -15,7 +15,7 @@ use mountpoint_s3_client::config::S3ClientConfig; use mountpoint_s3_client::error_metadata::ClientErrorMetadata; use mountpoint_s3_client::failure_client::{countdown_failure_client, CountdownFailureConfig}; use mountpoint_s3_client::mock_client::{MockClient, MockClientConfig, MockClientError, MockObject, Operation}; -use mountpoint_s3_client::types::{ETag, RestoreStatus}; +use mountpoint_s3_client::types::{ETag, GetObjectParams, RestoreStatus}; use mountpoint_s3_client::ObjectClient; #[cfg(all(feature = "s3_tests", not(feature = "s3express_tests")))] use mountpoint_s3_client::PutObjectRequest; @@ -600,7 +600,7 @@ async fn test_sequential_write(write_size: usize) { // Check that the object made it to S3 as we expected let get = client - .get_object(BUCKET_NAME, "dir1/file2.bin", None, None) + .get_object(BUCKET_NAME, "dir1/file2.bin", &GetObjectParams::new()) .await .unwrap(); let actual = get.collect().await.unwrap();