Remove use of ObjectInfo in S3 client HeadObject response

Signed-off-by: Daniel Carl Jones <[email protected]>

mountpoint-s3-client/CHANGELOG.md

@@ -1,5 +1,16 @@
 ## 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`.
+  `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

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.as_str().to_string(),
+                storage_class: object.storage_class.clone(),
+                restore_status: object.restore_status,
             })
         } else {
             Err(ObjectClientError::ServiceError(HeadObjectError::NotFound))

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: String,
+
+    /// 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<String>,
 
-    /// 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<RestoreStatus>,
 }
 
 /// Errors returned by a [`head_object`](ObjectClient::head_object) request

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,23 +83,23 @@ impl HeadObjectResult {
         Ok(Some(RestoreStatus::Restored { expiry: expiry.into() }))
     }
 
-    fn parse_from_hdr(bucket: String, key: String, headers: &Headers) -> Result<Self, ParseError> {
+    /// Parse from HeadObject headers
+    fn parse_from_hdr(headers: &Headers) -> Result<Self, ParseError> {
         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")?)
             .map_err(|e| ParseError::Int(e, "ContentLength".into()))?;
         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,
         };
-        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| {

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")]
@@ -63,13 +65,14 @@ async fn test_head_object_storage_class(storage_class: &str) {
         .unwrap();
 
     let client: S3CrtClient = get_test_client();
-    let result = client.head_object(&bucket, &key).await.expect("head_object failed");
+    let result = client
+        .head_object(&bucket, &key, &HeadObjectParams::new())
+        .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 +143,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 +169,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");
 }

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), 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