Merge pull request #492 from umccr/refactor/simpler-attributes

refactor(filemanager): misc simpler and clearer API

lib/workload/stateless/stacks/filemanager/database/migrations/0005_rename_date.sql

@@ -0,0 +1,2 @@
+-- Rename `date` to `event_time` as it is clearer what the meaning is compared to `last_modified_date`.
+alter table s3_object rename column date to event_time;

lib/workload/stateless/stacks/filemanager/database/queries/api/select_existing_by_bucket_key.sql

@@ -23,7 +23,7 @@ select
     s3_object_id,
     s3_object.bucket,
     s3_object.key,
-    date as event_time,
+    event_time,
     last_modified_date,
     e_tag,
     sha256,

lib/workload/stateless/stacks/filemanager/database/queries/ingester/aws/insert_s3_created_objects.sql

@@ -3,7 +3,7 @@ insert into s3_object (
     s3_object_id,
     bucket,
     key,
-    date,
+    event_time,
     size,
     sha256,
     last_modified_date,

lib/workload/stateless/stacks/filemanager/database/queries/ingester/aws/insert_s3_objects.sql

@@ -3,7 +3,7 @@ insert into s3_object (
     s3_object_id,
     bucket,
     key,
-    date,
+    event_time,
     size,
     sha256,
     last_modified_date,

lib/workload/stateless/stacks/filemanager/database/queries/ingester/aws/update_reordered_for_created.sql

@@ -92,7 +92,7 @@ objects_to_update as (
 update as (
     update s3_object
     set sequencer = objects_to_update.input_created_sequencer,
-        date = objects_to_update.input_created_date,
+        event_time = objects_to_update.input_created_date,
         size = objects_to_update.input_size,
         sha256 = objects_to_update.input_sha256,
         last_modified_date = objects_to_update.input_last_modified_date,
@@ -116,7 +116,7 @@ select
     input_id as "s3_object_id!",
     bucket,
     key,
-    date as event_time,
+    event_time,
     last_modified_date,
     e_tag,
     sha256,

lib/workload/stateless/stacks/filemanager/docs/API_GUIDE.md

@@ -137,15 +137,15 @@ For example, update attributes on a single record:
 
 ```sh
 curl -X PATCH -H "Authorization: Bearer $TOKEN" -H "Content-Type: application/json" \
---data '{ "attributes": [ { "op": "add", "path": "/portalRunId", "value": "portalRunIdValue" } ] }' \
+--data '[ { "op": "add", "path": "/portalRunId", "value": "portalRunIdValue" } ]' \
 "https://file.dev.umccr.org/api/v1/s3/0190465f-68fa-76e4-9c36-12bdf1a1571d" | jq
 ```
 
 Or, update attributes for multiple records with the same key prefix:
 
 ```sh
 curl -X PATCH -H "Authorization: Bearer $TOKEN" -H "Content-Type: application/json" \
---data '{ "attributes": [ { "op": "add", "path": "/portalRunId", "value": "portalRunIdValue" } ] }' \
+--data '[ { "op": "add", "path": "/portalRunId", "value": "portalRunIdValue" } ]' \
 "https://file.dev.umccr.org/api/v1/s3?key=%25202405212aecb782%25" | jq
 ```
 

lib/workload/stateless/stacks/filemanager/filemanager/src/database/aws/ingester.rs

@@ -1017,7 +1017,7 @@ pub(crate) mod tests {
         s3_object_results: &PgRow,
         message: FlatS3EventMessage,
         sequencer: Option<String>,
-        date: Option<DateTime<Utc>>,
+        event_time: Option<DateTime<Utc>>,
     ) {
         assert_eq!(message.bucket, s3_object_results.get::<String, _>("bucket"));
         assert_eq!(message.key, s3_object_results.get::<String, _>("key"));
@@ -1046,8 +1046,8 @@ pub(crate) mod tests {
             s3_object_results.get::<Option<DateTime<Utc>>, _>("last_modified_date")
         );
         assert_eq!(
-            date,
-            s3_object_results.get::<Option<DateTime<Utc>>, _>("date")
+            event_time,
+            s3_object_results.get::<Option<DateTime<Utc>>, _>("event_time")
         );
         assert_eq!(
             message.is_delete_marker,
@@ -1082,12 +1082,12 @@ pub(crate) mod tests {
         size: Option<i64>,
         sequencer: Option<String>,
         version_id: String,
-        date: Option<DateTime<Utc>>,
+        event_time: Option<DateTime<Utc>>,
         event_type: EventType,
     ) {
         let message = expected_message(size, version_id, false, event_type);
 
-        assert_row(s3_object_results, message, sequencer, date);
+        assert_row(s3_object_results, message, sequencer, event_time);
     }
 
     fn update_test_events(mut events: TransposedS3EventMessages) -> TransposedS3EventMessages {

lib/workload/stateless/stacks/filemanager/filemanager/src/database/aws/ingester_paired.rs

@@ -1582,7 +1582,7 @@ pub(crate) mod tests {
         );
         assert_eq!(
             created_date,
-            s3_object_results.get::<Option<DateTime<Utc>>, _>("date")
+            s3_object_results.get::<Option<DateTime<Utc>>, _>("event_time")
         );
         assert_eq!(
             deleted_date,

lib/workload/stateless/stacks/filemanager/filemanager/src/database/aws/query.rs

@@ -1,4 +1,4 @@
-use sqlx::{query_file, query_file_as, Acquire, Postgres, Row, Transaction};
+use sqlx::{query_file_as, Acquire, Postgres, Transaction};
 
 use crate::database::Client;
 use crate::error::Result;
@@ -21,25 +21,6 @@ impl Query {
         Self { client }
     }
 
-    /// Creates a new filemanager query client with default connection settings.
-    /// -- FIXME: Should not trust user input, should be a bit more robust than like/similar to
-    pub async fn query_objects(&self, query: String) -> Result<QueryResults> {
-        let mut tx = self.client.pool().begin().await?;
-
-        let query_results: Vec<String> =
-            query_file!("../database/queries/api/select_object_ids.sql", &query)
-                .fetch_all(&mut *tx)
-                .await?
-                .into_iter()
-                .map(|row| row.get(0))
-                .collect();
-
-        tx.commit().await?;
-
-        let query_results = QueryResults::new(query_results); // Convert PgQueryResult to QueryResults
-        Ok(query_results)
-    }
-
     /// Selects existing objects by the bucket and key for update. This does not start a transaction.
     /// TODO, ideally this should use some better types. Potentially use sea-orm codegen to simplify queries.
     pub async fn select_existing_by_bucket_key(

lib/workload/stateless/stacks/filemanager/filemanager/src/database/mod.rs

@@ -193,7 +193,7 @@ pub(crate) mod tests {
             "select s3_object_id as \"s3_object_id!\",
                 bucket,
                 key,
-                date,
+                event_time,
                 last_modified_date,
                 e_tag,
                 storage_class as \"storage_class: StorageClass\",
@@ -210,7 +210,7 @@ pub(crate) mod tests {
             inserted[0].sequencer,
             Some(EXPECTED_SEQUENCER_CREATED_ONE.to_string())
         );
-        assert_eq!(inserted[0].date, Some(DateTime::default()));
+        assert_eq!(inserted[0].event_time, Some(DateTime::default()));
     }
 
     #[sqlx::test(migrator = "MIGRATOR")]
@@ -243,7 +243,7 @@ pub(crate) mod tests {
             "select s3_object_id as \"s3_object_id!\",
                 bucket,
                 key,
-                date,
+                event_time,
                 last_modified_date,
                 e_tag,
                 storage_class as \"storage_class: StorageClass\",
@@ -260,6 +260,6 @@ pub(crate) mod tests {
             inserted[0].sequencer,
             Some(EXPECTED_SEQUENCER_CREATED_ONE.to_string())
         );
-        assert_eq!(inserted[0].date, Some(DateTime::default()));
+        assert_eq!(inserted[0].event_time, Some(DateTime::default()));
     }
 }

lib/workload/stateless/stacks/filemanager/filemanager/src/queries/list.rs

@@ -16,9 +16,8 @@ use sea_orm::sea_query::{
 };
 use sea_orm::Order::{Asc, Desc};
 use sea_orm::{
-    ActiveEnum, ColumnTrait, Condition, ConnectionTrait, EntityTrait, FromQueryResult,
-    IntoSimpleExpr, JsonValue, PaginatorTrait, QueryFilter, QueryOrder, QuerySelect, QueryTrait,
-    Select, Value,
+    ColumnTrait, Condition, ConnectionTrait, EntityTrait, FromQueryResult, IntoSimpleExpr,
+    JsonValue, PaginatorTrait, QueryFilter, QueryOrder, QuerySelect, QueryTrait, Select, Value,
 };
 use tracing::trace;
 use url::Url;
@@ -73,13 +72,11 @@ where
     /// Create a condition to filter a query.
     pub fn filter_condition(filter: S3ObjectsFilter, case_sensitive: bool) -> Condition {
         let mut condition = Condition::all()
-            .add_option(filter.event_type.map(|v| {
-                Self::filter_operation(
-                    Expr::col(s3_object::Column::EventType),
-                    v.map(|or| or.as_enum()),
-                    case_sensitive,
-                )
-            }))
+            .add_option(
+                filter
+                    .event_type
+                    .map(|v| s3_object::Column::EventType.eq(v)),
+            )
             .add_option(filter.bucket.map(|v| {
                 Self::filter_operation(
                     Expr::col(s3_object::Column::Bucket),
@@ -101,8 +98,8 @@ where
                     case_sensitive,
                 )
             }))
-            .add_option(filter.date.map(|v| {
-                Self::filter_operation(Expr::col(s3_object::Column::Date), v, case_sensitive)
+            .add_option(filter.event_time.map(|v| {
+                Self::filter_operation(Expr::col(s3_object::Column::EventTime), v, case_sensitive)
             }))
             .add_option(filter.size.map(|v| s3_object::Column::Size.eq(v)))
             .add_option(filter.sha256.map(|v| s3_object::Column::Sha256.eq(v)))
@@ -114,13 +111,11 @@ where
                 )
             }))
             .add_option(filter.e_tag.map(|v| s3_object::Column::ETag.eq(v)))
-            .add_option(filter.storage_class.map(|v| {
-                Self::filter_operation(
-                    Expr::col(s3_object::Column::StorageClass),
-                    v.map(|or| or.as_enum()),
-                    case_sensitive,
-                )
-            }))
+            .add_option(
+                filter
+                    .storage_class
+                    .map(|v| s3_object::Column::StorageClass.eq(v)),
+            )
             .add_option(
                 filter
                     .is_delete_marker
@@ -572,7 +567,7 @@ pub(crate) mod tests {
         let result = filter_all_s3_from(
             &client,
             S3ObjectsFilter {
-                event_type: Some(WildcardEither::Or(EventType::Created)),
+                event_type: Some(EventType::Created),
                 ..Default::default()
             },
             true,
@@ -789,8 +784,8 @@ pub(crate) mod tests {
         let result = filter_all_s3_from(
             &client,
             S3ObjectsFilter {
-                event_type: Some(WildcardEither::Wildcard(Wildcard::new(
-                    "Cr___ed".to_string(),
+                event_time: Some(WildcardEither::Wildcard(Wildcard::new(
+                    "1970-01-0%".to_string(),
                 ))),
                 ..Default::default()
             },
@@ -799,23 +794,15 @@ pub(crate) mod tests {
         .await;
         assert_eq!(
             result,
-            filter_event_type(s3_entries.clone(), EventType::Created)
-        );
-
-        let result = filter_all_s3_from(
-            &client,
-            S3ObjectsFilter {
-                event_type: Some(WildcardEither::Wildcard(Wildcard::new(
-                    "cr___ed".to_string(),
-                ))),
-                ..Default::default()
-            },
-            false,
-        )
-        .await;
-        assert_eq!(
-            result,
-            filter_event_type(s3_entries.clone(), EventType::Created)
+            s3_entries
+                .clone()
+                .into_iter()
+                .filter(|entry| entry
+                    .event_time
+                    .unwrap()
+                    .to_string()
+                    .starts_with("1970-01-0"))
+                .collect::<Vec<_>>()
         );
 
         let result = filter_all_s3_from(

lib/workload/stateless/stacks/filemanager/filemanager/src/queries/mod.rs

@@ -86,7 +86,7 @@ impl Entries {
             bucket: Set((index / bucket_divisor).to_string()),
             key: Set((index / key_divisor).to_string()),
             version_id: Set((index / key_divisor).to_string()),
-            date: date(),
+            event_time: date(),
             size: Set(Some(index as i64)),
             sha256: Set(Some(index.to_string())),
             last_modified_date: date(),