fix(filemanager): storage class can still be unknoen before a head ob…

…ject call
umccr · Dec 19, 2023 · ff9dbb0 · ff9dbb0
1 parent a4ff710
commit ff9dbb0
Show file tree

Hide file tree

Showing 3 changed files with 11 additions and 25 deletions.
diff --git a/lib/workload/stateful/filemanager/database/migrations/aws/0002_add_s3_object_table.sql b/lib/workload/stateful/filemanager/database/migrations/aws/0002_add_s3_object_table.sql
@@ -5,5 +5,5 @@ create table s3_object(
     -- The object id.
     object_id uuid references object (object_id) primary key,
     -- The S3 storage class of the object.
-    storage_class storage_class not null default 'Standard'
+    storage_class storage_class not null
 );
diff --git a/lib/workload/stateful/filemanager/filemanager/src/events/aws/collecter.rs b/lib/workload/stateful/filemanager/filemanager/src/events/aws/collecter.rs
@@ -4,6 +4,7 @@ use crate::error::Error::S3Error;
 use async_trait::async_trait;
 use aws_sdk_s3::operation::head_object::{HeadObjectError, HeadObjectOutput};
 use aws_sdk_s3::primitives;
+use aws_sdk_s3::types::StorageClass::Standard;
 use chrono::{DateTime, NaiveDateTime, Utc};
 use futures::future::join_all;
 use mockall_double::double;
@@ -95,8 +96,12 @@ impl Collecter {
                         ..
                     } = head;
 
+                    // S3 does not return a storage class for standard, which means this is the
+                    // default. See https://docs.aws.amazon.com/AmazonS3/latest/API/API_HeadObject.html#API_HeadObject_ResponseSyntax
                     event = event
-                        .update_storage_class(storage_class.and_then(StorageClass::from_aws))
+                        .update_storage_class(StorageClass::from_aws(
+                            storage_class.unwrap_or(Standard),
+                        ))
                         .update_last_modified_date(Self::convert_datetime(last_modified))
                         .update_size(content_length)
                         .update_e_tag(e_tag);

diff --git a/lib/workload/stateful/filemanager/filemanager/src/events/aws/mod.rs b/lib/workload/stateful/filemanager/filemanager/src/events/aws/mod.rs
@@ -422,14 +422,6 @@ impl TryFrom<S3EventMessage> for FlatS3EventMessages {
                         Other
                     };
 
-                    // S3 does not return a storage class for standard, so this is assumed to be
-                    // the default. See https://docs.aws.amazon.com/AmazonS3/latest/API/API_HeadObject.html#API_HeadObject_ResponseSyntax
-                    let storage_class = if let Created = event_type {
-                        Some(StorageClass::Standard)
-                    } else {
-                        None
-                    };
-
                     Ok(FlatS3EventMessage {
                         object_id,
                         event_time,
@@ -440,8 +432,8 @@ impl TryFrom<S3EventMessage> for FlatS3EventMessages {
                         e_tag,
                         sequencer,
                         portal_run_id,
-                        storage_class,
                         // Head field are fetched later.
+                        storage_class: None,
                         last_modified_date: None,
                         event_type,
                     })
@@ -459,9 +451,7 @@ impl From<Vec<FlatS3EventMessages>> for FlatS3EventMessages {
 
 #[cfg(test)]
 pub(crate) mod tests {
-    use crate::events::aws::{
-        Events, FlatS3EventMessage, FlatS3EventMessages, S3EventMessage, StorageClass,
-    };
+    use crate::events::aws::{Events, FlatS3EventMessage, FlatS3EventMessages, S3EventMessage};
     use chrono::{DateTime, Utc};
     use serde_json::json;
 
@@ -480,7 +470,6 @@ pub(crate) mod tests {
             "ObjectRemoved:Delete",
             EXPECTED_SEQUENCER_DELETED,
             None,
-            None,
         );
 
         let second = result.next().unwrap();
@@ -489,7 +478,6 @@ pub(crate) mod tests {
             "ObjectCreated:Put",
             EXPECTED_SEQUENCER_CREATED,
             Some(0),
-            Some(StorageClass::Standard),
         );
 
         let third = result.next().unwrap();
@@ -498,7 +486,6 @@ pub(crate) mod tests {
             "ObjectCreated:Put",
             EXPECTED_SEQUENCER_CREATED,
             Some(0),
-            Some(StorageClass::Standard),
         );
     }
 
@@ -513,7 +500,6 @@ pub(crate) mod tests {
             "ObjectCreated:Put",
             EXPECTED_SEQUENCER_CREATED,
             Some(0),
-            Some(StorageClass::Standard),
         );
 
         let second = result.next().unwrap();
@@ -522,7 +508,6 @@ pub(crate) mod tests {
             "ObjectRemoved:Delete",
             EXPECTED_SEQUENCER_DELETED,
             None,
-            None,
         );
     }
 
@@ -531,7 +516,6 @@ pub(crate) mod tests {
         event_name: &str,
         sequencer: &str,
         size: Option<i64>,
-        storage_class: Option<StorageClass>,
     ) {
         assert_eq!(event.event_time, DateTime::<Utc>::default());
         assert_eq!(event.event_name, event_name);
@@ -541,7 +525,7 @@ pub(crate) mod tests {
         assert_eq!(event.e_tag, Some(EXPECTED_E_TAG.to_string())); // pragma: allowlist secret
         assert_eq!(event.sequencer, Some(sequencer.to_string()));
         assert!(event.portal_run_id.starts_with("19700101"));
-        assert_eq!(event.storage_class, storage_class);
+        assert_eq!(event.storage_class, None);
         assert_eq!(event.last_modified_date, None);
     }
 
@@ -566,10 +550,7 @@ pub(crate) mod tests {
             Some(EXPECTED_SEQUENCER_CREATED.to_string())
         );
         assert!(result.object_created.portal_run_ids[0].starts_with("19700101"));
-        assert_eq!(
-            result.object_created.storage_classes[0],
-            Some(StorageClass::Standard)
-        );
+        assert_eq!(result.object_created.storage_classes[0], None);
         assert_eq!(result.object_created.last_modified_dates[0], None);
 
         assert_eq!(