Skip to content

Commit

Permalink
fix(filemanager): head object does not return storage class when it i…
Browse files Browse the repository at this point in the history
…s standard
  • Loading branch information
mmalenic committed Dec 19, 2023
1 parent 1f29f7e commit a4ff710
Show file tree
Hide file tree
Showing 5 changed files with 40 additions and 15 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,9 @@ create table object (
-- A unique identifier for the object, if it is present.
hash varchar(255) default null,
-- When this object was created.
created_date timestamptz not null,
created_date timestamptz not null default now(),
-- When this object was last modified.
last_modified_date timestamptz not null,
last_modified_date timestamptz not null default now(),
-- When this object was deleted, a null value means that the object has not yet been deleted.
deleted_date timestamptz default null,
-- The date of the object and its id combined.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 default null
storage_class storage_class not null default 'Standard'
);
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,7 @@ pub(crate) mod tests {
events.object_created.storage_classes[0] = Some(StorageClass::Standard);

events.object_removed.last_modified_dates[0] = Some(DateTime::default());
events.object_removed.storage_classes[0] = Some(StorageClass::Standard);
events.object_removed.storage_classes[0] = None;

events
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,14 +73,17 @@ impl Collecter {
) -> Result<FlatS3EventMessages> {
Ok(FlatS3EventMessages(
join_all(events.into_inner().into_iter().map(|mut event| async move {
// Only run this on created events.
// No need to run this unnecessarily on removed events.
match event.event_type {
EventType::Removed | EventType::Other => return Ok(event),
_ => {}
};

trace!(key = ?event.key, bucket = ?event.bucket, "updating event");

// Race condition: it's possible that an object gets deleted so quickly that it
// occurs before calling head. This means that there may be cases where the storage
// class and other fields are not known.
if let Some(head) = Self::head(client, &event.key, &event.bucket).await? {
trace!(head = ?head, "received head object output");

Expand Down Expand Up @@ -124,7 +127,7 @@ impl Collect for Collecter {
pub(crate) mod tests {
use crate::events::aws::collecter::Collecter;
use crate::events::aws::tests::expected_flat_events;
use crate::events::aws::StorageClass::Standard;
use crate::events::aws::StorageClass::IntelligentTiering;
use aws_sdk_s3::error::SdkError;
use aws_sdk_s3::primitives::{DateTimeFormat, SdkBody};
use aws_sdk_s3::types;
Expand Down Expand Up @@ -189,7 +192,7 @@ pub(crate) mod tests {
.into_iter();

let first = result.next().unwrap();
assert_eq!(first.storage_class, Some(Standard));
assert_eq!(first.storage_class, Some(IntelligentTiering));
assert_eq!(first.last_modified_date, Some(Default::default()));

let second = result.next().unwrap();
Expand All @@ -213,7 +216,10 @@ pub(crate) mod tests {

match result {
EventSourceType::S3(events) => {
assert_eq!(events.object_created.storage_classes[0], Some(Standard));
assert_eq!(
events.object_created.storage_classes[0],
Some(IntelligentTiering)
);
assert_eq!(
events.object_created.last_modified_dates[0],
Some(Default::default())
Expand Down Expand Up @@ -245,7 +251,7 @@ pub(crate) mod tests {
primitives::DateTime::from_str("1970-01-01T00:00:00Z", DateTimeFormat::DateTime)
.unwrap(),
)
.storage_class(types::StorageClass::Standard)
.storage_class(types::StorageClass::IntelligentTiering)
.build()
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -313,7 +313,7 @@ pub struct FlatS3EventMessage {
}

impl FlatS3EventMessage {
/// Update the storage class if not None.
/// Update the storage class if not None.`
pub fn update_storage_class(mut self, storage_class: Option<StorageClass>) -> Self {
storage_class
.into_iter()
Expand Down Expand Up @@ -422,6 +422,14 @@ 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,
Expand All @@ -432,8 +440,8 @@ impl TryFrom<S3EventMessage> for FlatS3EventMessages {
e_tag,
sequencer,
portal_run_id,
// Head field are optionally fetched later.
storage_class: None,
storage_class,
// Head field are fetched later.
last_modified_date: None,
event_type,
})
Expand All @@ -451,7 +459,9 @@ impl From<Vec<FlatS3EventMessages>> for FlatS3EventMessages {

#[cfg(test)]
pub(crate) mod tests {
use crate::events::aws::{Events, FlatS3EventMessage, FlatS3EventMessages, S3EventMessage};
use crate::events::aws::{
Events, FlatS3EventMessage, FlatS3EventMessages, S3EventMessage, StorageClass,
};
use chrono::{DateTime, Utc};
use serde_json::json;

Expand All @@ -470,6 +480,7 @@ pub(crate) mod tests {
"ObjectRemoved:Delete",
EXPECTED_SEQUENCER_DELETED,
None,
None,
);

let second = result.next().unwrap();
Expand All @@ -478,6 +489,7 @@ pub(crate) mod tests {
"ObjectCreated:Put",
EXPECTED_SEQUENCER_CREATED,
Some(0),
Some(StorageClass::Standard),
);

let third = result.next().unwrap();
Expand All @@ -486,6 +498,7 @@ pub(crate) mod tests {
"ObjectCreated:Put",
EXPECTED_SEQUENCER_CREATED,
Some(0),
Some(StorageClass::Standard),
);
}

Expand All @@ -500,6 +513,7 @@ pub(crate) mod tests {
"ObjectCreated:Put",
EXPECTED_SEQUENCER_CREATED,
Some(0),
Some(StorageClass::Standard),
);

let second = result.next().unwrap();
Expand All @@ -508,6 +522,7 @@ pub(crate) mod tests {
"ObjectRemoved:Delete",
EXPECTED_SEQUENCER_DELETED,
None,
None,
);
}

Expand All @@ -516,6 +531,7 @@ 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);
Expand All @@ -525,7 +541,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, None);
assert_eq!(event.storage_class, storage_class);
assert_eq!(event.last_modified_date, None);
}

Expand All @@ -550,7 +566,10 @@ 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], None);
assert_eq!(
result.object_created.storage_classes[0],
Some(StorageClass::Standard)
);
assert_eq!(result.object_created.last_modified_dates[0], None);

assert_eq!(
Expand Down

0 comments on commit a4ff710

Please sign in to comment.