diff --git a/mountpoint-s3-client/CHANGELOG.md b/mountpoint-s3-client/CHANGELOG.md index a74e47f1c..4bf56ff2a 100644 --- a/mountpoint-s3-client/CHANGELOG.md +++ b/mountpoint-s3-client/CHANGELOG.md @@ -1,5 +1,18 @@ ## Unreleased +### Other changes + +* No other changes. + +### Breaking changes + +* `HeadObjectResult` no longer contains an `ObjectInfo` struct. + Instead, it returns the object attributes as individual fields on the `HeadObjectResult`. + The entity tag field has also changed and is now of type `ETag` rather than `String`. + ([#1058](https://github.com/awslabs/mountpoint-s3/pull/1058)) +* `HeadObjectResult` no longer provides the bucket and key used in the original request. + ([#1058](https://github.com/awslabs/mountpoint-s3/pull/1058)) + ## v0.11.0 (October 17, 2024) ### Breaking changes diff --git a/mountpoint-s3-client/src/mock_client.rs b/mountpoint-s3-client/src/mock_client.rs index 9818237cd..d2bd442c3 100644 --- a/mountpoint-s3-client/src/mock_client.rs +++ b/mountpoint-s3-client/src/mock_client.rs @@ -687,15 +687,11 @@ impl ObjectClient for MockClient { let objects = self.objects.read().unwrap(); if let Some(object) = objects.get(key) { Ok(HeadObjectResult { - bucket: bucket.to_string(), - object: ObjectInfo { - key: key.to_string(), - size: object.size as u64, - last_modified: object.last_modified, - etag: object.etag.as_str().to_string(), - storage_class: object.storage_class.clone(), - restore_status: object.restore_status, - }, + size: object.size as u64, + last_modified: object.last_modified, + etag: object.etag.clone(), + storage_class: object.storage_class.clone(), + restore_status: object.restore_status, }) } else { Err(ObjectClientError::ServiceError(HeadObjectError::NotFound)) @@ -1681,7 +1677,7 @@ mod tests { // head_object returns storage class let head_result = client.head_object(bucket, key).await.unwrap(); - assert_eq!(head_result.object.storage_class.as_deref(), storage_class); + assert_eq!(head_result.storage_class.as_deref(), storage_class); // list_objects returns storage class let list_result = client.list_objects(bucket, None, "/", 1, "").await.unwrap(); diff --git a/mountpoint-s3-client/src/object_client.rs b/mountpoint-s3-client/src/object_client.rs index ad81bf5cc..b5db33387 100644 --- a/mountpoint-s3-client/src/object_client.rs +++ b/mountpoint-s3-client/src/object_client.rs @@ -210,11 +210,28 @@ pub enum ListObjectsError { #[derive(Debug)] #[non_exhaustive] pub struct HeadObjectResult { - /// The name of the bcuket - pub bucket: String, + /// Size of the object in bytes. + /// + /// Refers to the `Content-Length` HTTP header for HeadObject. + pub size: u64, + + /// The time this object was last modified. + pub last_modified: OffsetDateTime, + + /// Entity tag of this object. + pub etag: ETag, + + /// Storage class for this object. + /// + /// The value is optional because HeadObject does not return the storage class in its response + /// for objects in the S3 Standard storage class. + /// See examples in the + /// [Amazon S3 API Reference](https://docs.aws.amazon.com/AmazonS3/latest/API/API_HeadObject.html#API_HeadObject_Examples). + pub storage_class: Option, - /// Object metadata - pub object: ObjectInfo, + /// Objects in flexible retrieval storage classes (such as GLACIER and DEEP_ARCHIVE) are only + /// accessible after restoration + pub restore_status: Option, } /// Errors returned by a [`head_object`](ObjectClient::head_object) request diff --git a/mountpoint-s3-client/src/s3_crt_client/head_object.rs b/mountpoint-s3-client/src/s3_crt_client/head_object.rs index 9ffe9aad1..9beba4f54 100644 --- a/mountpoint-s3-client/src/s3_crt_client/head_object.rs +++ b/mountpoint-s3-client/src/s3_crt_client/head_object.rs @@ -11,9 +11,7 @@ use time::format_description::well_known::Rfc2822; use time::OffsetDateTime; use tracing::error; -use crate::object_client::{ - HeadObjectError, HeadObjectResult, ObjectClientError, ObjectClientResult, ObjectInfo, RestoreStatus, -}; +use crate::object_client::{HeadObjectError, HeadObjectResult, ObjectClientError, ObjectClientResult, RestoreStatus}; use crate::s3_crt_client::{S3CrtClient, S3Operation, S3RequestError}; #[derive(Error, Debug)] @@ -85,7 +83,8 @@ impl HeadObjectResult { Ok(Some(RestoreStatus::Restored { expiry: expiry.into() })) } - fn parse_from_hdr(bucket: String, key: String, headers: &Headers) -> Result { + /// Parse from HeadObject headers + fn parse_from_hdr(headers: &Headers) -> Result { let last_modified = OffsetDateTime::parse(&get_field(headers, "Last-Modified")?, &Rfc2822) .map_err(|e| ParseError::OffsetDateTime(e, "LastModified".into()))?; let size = u64::from_str(&get_field(headers, "Content-Length")?) @@ -93,15 +92,14 @@ impl HeadObjectResult { let etag = get_field(headers, "Etag")?; let storage_class = get_optional_field(headers, "x-amz-storage-class")?; let restore_status = Self::parse_restore_status(headers)?; - let object = ObjectInfo { - key, + let result = HeadObjectResult { size, last_modified, storage_class, restore_status, - etag, + etag: etag.into(), }; - Ok(HeadObjectResult { bucket, object }) + Ok(result) } } @@ -137,11 +135,7 @@ impl S3CrtClient { span, move |headers, _status| { let mut header = header1.lock().unwrap(); - *header = Some(HeadObjectResult::parse_from_hdr( - bucket.to_string(), - key.to_string(), - headers, - )); + *header = Some(HeadObjectResult::parse_from_hdr(headers)); }, |_, _| (), move |result| { diff --git a/mountpoint-s3-client/tests/head_object.rs b/mountpoint-s3-client/tests/head_object.rs index f5d8862af..7e54a59b6 100644 --- a/mountpoint-s3-client/tests/head_object.rs +++ b/mountpoint-s3-client/tests/head_object.rs @@ -22,7 +22,7 @@ async fn test_head_object() { let sdk_client = get_test_sdk_client().await; let (bucket, prefix) = get_test_bucket_and_prefix("test_head_object"); - let key = format!("{prefix}/hello"); + let key = format!("{prefix}hello"); let body = b"hello world!"; sdk_client .put_object() @@ -36,9 +36,11 @@ async fn test_head_object() { let client: S3CrtClient = get_test_client(); let result = client.head_object(&bucket, &key).await.expect("head_object failed"); - assert_eq!(result.bucket, bucket); - assert_eq!(result.object.key, key); - assert_eq!(result.object.size as usize, body.len()); + assert_eq!( + result.size as usize, + body.len(), + "HeadObject reported size should match uploaded body length", + ); } #[test_case("INTELLIGENT_TIERING")] @@ -65,11 +67,9 @@ async fn test_head_object_storage_class(storage_class: &str) { let client: S3CrtClient = get_test_client(); let result = client.head_object(&bucket, &key).await.expect("head_object failed"); - assert_eq!(result.bucket, bucket); - assert_eq!(result.object.key, key); - assert_eq!(result.object.size as usize, body.len()); - assert_eq!(result.object.storage_class.as_deref(), Some(storage_class)); - assert!(result.object.restore_status.is_none()); + assert_eq!(result.size as usize, body.len()); + assert_eq!(result.storage_class.as_deref(), Some(storage_class)); + assert!(result.restore_status.is_none()); } #[tokio::test] @@ -140,10 +140,8 @@ async fn test_head_object_restored() { let client: S3CrtClient = get_test_client(); let result = client.head_object(&bucket, &key).await.expect("head_object failed"); - assert_eq!(result.bucket, bucket); - assert_eq!(result.object.key, key); assert!( - result.object.restore_status.is_none(), + result.restore_status.is_none(), "object should become restored only after restoration" ); @@ -168,18 +166,14 @@ async fn test_head_object_restored() { let timeout = Duration::from_secs(300); let start = Instant::now(); - let mut timeouted = true; + let mut timeout_exceeded = true; while start.elapsed() < timeout { - let object = client - .head_object(&bucket, &key) - .await - .expect("head_object failed") - .object; - if let Some(RestoreStatus::Restored { expiry: _ }) = object.restore_status { - timeouted = false; + let response = client.head_object(&bucket, &key).await.expect("head_object failed"); + if let Some(RestoreStatus::Restored { expiry: _ }) = response.restore_status { + timeout_exceeded = false; break; } std::thread::sleep(Duration::from_secs(1)); } - assert!(!timeouted, "timeouted while waiting for object become restored"); + assert!(!timeout_exceeded, "timeouted while waiting for object become restored"); } diff --git a/mountpoint-s3/examples/prefetch_benchmark.rs b/mountpoint-s3/examples/prefetch_benchmark.rs index f9db56d36..3bdb76fe5 100644 --- a/mountpoint-s3/examples/prefetch_benchmark.rs +++ b/mountpoint-s3/examples/prefetch_benchmark.rs @@ -1,4 +1,3 @@ -use std::str::FromStr; use std::sync::atomic::{AtomicU64, Ordering}; use std::sync::Arc; use std::thread; @@ -10,7 +9,6 @@ use mountpoint_s3::mem_limiter::MemoryLimiter; use mountpoint_s3::object::ObjectId; use mountpoint_s3::prefetch::{default_prefetch, Prefetch, PrefetchResult}; use mountpoint_s3_client::config::{EndpointConfig, S3ClientConfig}; -use mountpoint_s3_client::types::ETag; use mountpoint_s3_client::{ObjectClient, S3CrtClient}; use mountpoint_s3_crt::common::rust_log_adapter::RustLogAdapter; use sysinfo::{RefreshKind, System}; @@ -113,8 +111,8 @@ fn main() { let mem_limiter = Arc::new(MemoryLimiter::new(client.clone(), max_memory_target)); let head_object_result = block_on(client.head_object(bucket, key)).expect("HeadObject failed"); - let size = head_object_result.object.size; - let etag = ETag::from_str(&head_object_result.object.etag).unwrap(); + let size = head_object_result.size; + let etag = head_object_result.etag; for i in 0..iterations.unwrap_or(1) { let runtime = client.event_loop_group(); diff --git a/mountpoint-s3/src/superblock.rs b/mountpoint-s3/src/superblock.rs index 285b61c08..5a2cba7c6 100644 --- a/mountpoint-s3/src/superblock.rs +++ b/mountpoint-s3/src/superblock.rs @@ -699,8 +699,8 @@ impl SuperblockInner { select_biased! { result = file_lookup => { match result { - Ok(HeadObjectResult { object, .. }) => { - let stat = InodeStat::for_file(object.size as usize, object.last_modified, Some(object.etag.clone()), object.storage_class, object.restore_status, self.config.cache_config.file_ttl); + Ok(HeadObjectResult { size, last_modified, restore_status ,etag, storage_class, .. }) => { + let stat = InodeStat::for_file(size as usize, last_modified, Some(etag.as_str().to_string()), storage_class, restore_status, self.config.cache_config.file_ttl); file_state = Some(stat); } // If the object is not found, might be a directory, so keep going @@ -1276,7 +1276,6 @@ mod tests { .head_object(bucket, file.inode.full_key()) .await .expect("object should exist") - .object .last_modified; assert_inode_stat!(file, InodeKind::File, modified_time, object_size); assert_eq!(