From f42b3eea4030487d9dd93185386a6ee2ed9de914 Mon Sep 17 00:00:00 2001
From: Marko Malenic <mmalenic1@gmail.com>
Date: Fri, 16 Aug 2024 14:43:51 +1000
Subject: [PATCH 1/3] feat(filemanager): patch should be append only and allow
 omitting outer attributes.

---
 .../stacks/filemanager/docs/API_GUIDE.md      |   4 +-
 .../filemanager/src/queries/update.rs         | 128 +++++++++--------
 .../filemanager/src/routes/update.rs          | 129 +++++++++++++-----
 3 files changed, 170 insertions(+), 91 deletions(-)

diff --git a/lib/workload/stateless/stacks/filemanager/docs/API_GUIDE.md b/lib/workload/stateless/stacks/filemanager/docs/API_GUIDE.md
index 6372fdcf1..a9a223bf0 100644
--- a/lib/workload/stateless/stacks/filemanager/docs/API_GUIDE.md
+++ b/lib/workload/stateless/stacks/filemanager/docs/API_GUIDE.md
@@ -136,7 +136,7 @@ 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
 ```
 
@@ -144,7 +144,7 @@ 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
 ```
 
diff --git a/lib/workload/stateless/stacks/filemanager/filemanager/src/queries/update.rs b/lib/workload/stateless/stacks/filemanager/filemanager/src/queries/update.rs
index ea41d4e2a..32c1a53e3 100644
--- a/lib/workload/stateless/stacks/filemanager/filemanager/src/queries/update.rs
+++ b/lib/workload/stateless/stacks/filemanager/filemanager/src/queries/update.rs
@@ -1,7 +1,7 @@
 //! Query builder to handle updating record columns.
 //!
 
-use json_patch::patch;
+use json_patch::{patch, PatchOperation};
 use sea_orm::prelude::{Expr, Json};
 use sea_orm::sea_query::{
     Alias, Asterisk, CommonTableExpression, Query, SelectStatement, SimpleExpr, WithClause,
@@ -135,6 +135,31 @@ where
         Ok(self.all().await?.into_iter().nth(0))
     }
 
+    /// Verifies that the JSON patch operation is supported.
+    fn verify_patch(
+        patch: Vec<PatchOperation>,
+        current_attributes: &serde_json::Value,
+    ) -> Result<Vec<PatchOperation>> {
+        let check_exists = |path: String, patch: PatchOperation| {
+            let exists = current_attributes.pointer(&path);
+            if exists.is_some() {
+                Err(InvalidQuery("path already exists".to_string()))
+            } else {
+                Ok(patch)
+            }
+        };
+
+        patch
+            .into_iter()
+            .map(|patch| match patch {
+                PatchOperation::Test(_) => Ok(patch),
+                PatchOperation::Add(ref op) => check_exists(op.path.to_string(), patch),
+                PatchOperation::Copy(ref op) => check_exists(op.path.to_string(), patch),
+                _ => Err(InvalidQuery("unsupported JSON patch operation".to_string())),
+            })
+            .collect::<Result<Vec<_>>>()
+    }
+
     /// Update the attributes on an object using the attribute patch. This first queries the
     /// required records to update using a previously specified select query in functions like
     /// `Self::for_id` and `Self::filter_all`. It then applies a JSON patch to the attributes of
@@ -185,8 +210,11 @@ where
                     return Err(QueryError("expected JSON attribute column".to_string()));
                 };
 
+                // Only append-style patching is supported.
+                let operations = Self::verify_patch(patch_body.clone().into_inner().0, &current)?;
+
                 // Patch it based on JSON patch.
-                patch(&mut current, &patch_body.get_ref().0).map_err(|err| {
+                patch(&mut current, operations.as_slice()).map_err(|err| {
                     InvalidQuery(format!(
                         "JSON patch {} operation for {} path failed: {}",
                         err.operation, err.path, err.kind
@@ -276,7 +304,7 @@ pub(crate) mod tests {
     use crate::routes::filter::wildcard::WildcardEither;
 
     #[sqlx::test(migrator = "MIGRATOR")]
-    async fn update_attributes_replace(pool: PgPool) {
+    async fn update_attributes_unsupported(pool: PgPool) {
         let client = Client::from_pool(pool);
         let mut entries = EntriesBuilder::default().build(&client).await;
 
@@ -288,16 +316,48 @@ pub(crate) mod tests {
         )
         .await;
 
+        let patch = json!([
+            { "op": "remove", "path": "/attributeId" },
+        ]);
+        let results = test_s3_builder_result(
+            &client,
+            patch,
+            Some(json!({
+                "attributeId": "1"
+            })),
+        )
+        .await;
+        assert!(matches!(results, Err(InvalidQuery(_))));
+
         let patch = json!([
             { "op": "test", "path": "/attributeId", "value": "1" },
             { "op": "replace", "path": "/attributeId", "value": "attributeId" },
         ]);
+        let results = test_s3_builder_result(
+            &client,
+            patch,
+            Some(json!({
+                "attributeId": "1"
+            })),
+        )
+        .await;
+        assert!(matches!(results, Err(InvalidQuery(_))));
 
-        let results = test_update_with_attribute_id(&client, patch).await;
-
-        entries_many(&mut entries, &[0, 1], json!({"attributeId": "attributeId"}));
+        let patch = json!([
+            { "op": "test", "path": "/attributeId", "value": "1" },
+            { "op": "add", "path": "/attributeId", "value": "attributeId" },
+        ]);
+        let results = test_s3_builder_result(
+            &client,
+            patch,
+            Some(json!({
+                "attributeId": "1"
+            })),
+        )
+        .await;
+        assert!(matches!(results, Err(InvalidQuery(_))));
 
-        assert_contains(&results, &entries, 0..2);
+        entries_many(&mut entries, &[0, 1], json!({"attributeId": "1"}));
         assert_correct_records(&client, entries).await;
     }
 
@@ -523,31 +583,6 @@ pub(crate) mod tests {
         assert_correct_records(&client, entries).await;
     }
 
-    #[sqlx::test(migrator = "MIGRATOR")]
-    async fn update_attributes_remove(pool: PgPool) {
-        let client = Client::from_pool(pool);
-        let mut entries = EntriesBuilder::default().build(&client).await;
-
-        change_many(
-            &client,
-            &entries,
-            &[0, 1],
-            Some(json!({"attributeId": "1"})),
-        )
-        .await;
-
-        let patch = json!([
-            { "op": "remove", "path": "/attributeId" },
-        ]);
-
-        let results = test_update_with_attribute_id(&client, patch).await;
-
-        entries_many(&mut entries, &[0, 1], json!({}));
-
-        assert_contains(&results, &entries, 0..2);
-        assert_correct_records(&client, entries).await;
-    }
-
     #[sqlx::test(migrator = "MIGRATOR")]
     async fn update_attributes_no_op(pool: PgPool) {
         let client = Client::from_pool(pool);
@@ -562,7 +597,7 @@ pub(crate) mod tests {
         .await;
 
         let patch = json!([
-            { "op": "remove", "path": "/attributeId" },
+            { "op": "add", "path": "/anotherAttribute", "value": "3" },
         ]);
 
         let results = test_update_with_attribute_id(&client, patch).await;
@@ -587,7 +622,7 @@ pub(crate) mod tests {
         .await;
 
         let patch = json!([
-            { "op": "replace", "path": "/attributeId", "value": "attributeId" },
+            { "op": "add", "path": "/anotherAttribute", "value": "anotherAttribute" },
             { "op": "test", "path": "/attributeId", "value": "2" },
         ]);
 
@@ -607,27 +642,6 @@ pub(crate) mod tests {
         assert_correct_records(&client, entries).await;
     }
 
-    #[sqlx::test(migrator = "MIGRATOR")]
-    async fn update_attributes_for_id(pool: PgPool) {
-        let client = Client::from_pool(pool);
-        let mut entries = EntriesBuilder::default().build(&client).await;
-
-        change_attributes(&client, &entries, 0, Some(json!({"attributeId": "1"}))).await;
-
-        let patch = json!([
-            { "op": "test", "path": "/attributeId", "value": "1" },
-            { "op": "replace", "path": "/attributeId", "value": "attributeId" },
-        ]);
-
-        let result =
-            test_update_attributes_for_id(&client, patch, entries.s3_objects[0].s3_object_id).await;
-
-        change_attribute_entries(&mut entries, 0, json!({"attributeId": "attributeId"}));
-
-        assert_contains(&result, &entries, 0..1);
-        assert_correct_records(&client, entries).await;
-    }
-
     #[sqlx::test(migrator = "MIGRATOR")]
     async fn update_attributes_replace_different_attribute_ids(pool: PgPool) {
         let client = Client::from_pool(pool);
@@ -638,7 +652,7 @@ pub(crate) mod tests {
         change_attributes(&client, &entries, 2, Some(json!({"attributeId": "2"}))).await;
 
         let patch = json!([
-            { "op": "replace", "path": "/attributeId", "value": "attributeId" },
+            { "op": "add", "path": "/anotherAttribute", "value": "anotherAttribute" },
         ]);
 
         let results_s3_objects = test_update_attributes(
@@ -653,7 +667,7 @@ pub(crate) mod tests {
         entries_many(
             &mut entries,
             &[0, 1, 2],
-            json!({"attributeId": "attributeId"}),
+            json!({"attributeId": "2", "anotherAttribute": "anotherAttribute"}),
         );
 
         assert_model_contains(&results_s3_objects, &entries.s3_objects, 0..3);
diff --git a/lib/workload/stateless/stacks/filemanager/filemanager/src/routes/update.rs b/lib/workload/stateless/stacks/filemanager/filemanager/src/routes/update.rs
index f6c6876d2..0d0910c9b 100644
--- a/lib/workload/stateless/stacks/filemanager/filemanager/src/routes/update.rs
+++ b/lib/workload/stateless/stacks/filemanager/filemanager/src/routes/update.rs
@@ -20,22 +20,23 @@ use crate::routes::AppState;
 /// The attributes to update for the request. This updates attributes according to JSON patch.
 /// See [JSON patch](https://jsonpatch.com/) and [RFC6902](https://datatracker.ietf.org/doc/html/rfc6902/).
 ///
-/// In order to apply the patch, the outer type of the JSON input must have one key called "attributes".
-/// Then any JSON patch operation can be used to update the attributes, e.g. "add" or "replace". The
-/// "test" operation can be used to confirm whether a key is a specific value before updating. If the
-/// check fails,  a `BAD_REQUEST` is returned and no records are updated.
-#[derive(Debug, Deserialize, Default, Clone, ToSchema)]
+/// In order to apply the patch, JSON body must contain an array with patch operations. The patch operations
+/// are append-only, which means that only "add" and "test" is supported. If a "test" check fails,
+/// a patch operations that isn't "add" or "test" is used, or if a key already exists, a `BAD_REQUEST`
+/// is returned and no records are updated.
+#[derive(Debug, Deserialize, Clone, ToSchema)]
+#[serde(untagged)]
 #[schema(
-    example = json!({
-        "attributes": [
-            { "op": "test", "path": "/attributeId", "value": "1" },
-            { "op": "replace", "path": "/attributeId", "value": "attributeId" }
-        ]
-    })
+    example = json!([
+        { "op": "add", "path": "/attributeId", "value": "attributeId" }
+    ])
 )]
-pub struct PatchBody {
-    /// The JSON patch for a record's attributes.
-    attributes: Patch,
+pub enum PatchBody {
+    Nested {
+        /// The JSON patch for a record's attributes.
+        attributes: Patch,
+    },
+    Unnested(Patch),
 }
 
 /// The JSON patch for attributes.
@@ -47,17 +48,23 @@ pub struct Patch(json_patch::Patch);
 impl PatchBody {
     /// Create a new attribute body.
     pub fn new(attributes: Patch) -> Self {
-        Self { attributes }
+        Self::Unnested(attributes)
     }
 
     /// Get the inner map.
     pub fn into_inner(self) -> json_patch::Patch {
-        self.attributes.0
+        match self {
+            PatchBody::Nested { attributes } => attributes.0,
+            PatchBody::Unnested(attributes) => attributes.0,
+        }
     }
 
     /// Get the inner map as a reference
     pub fn get_ref(&self) -> &json_patch::Patch {
-        &self.attributes.0
+        match self {
+            PatchBody::Nested { attributes } => &attributes.0,
+            PatchBody::Unnested(attributes) => &attributes.0,
+        }
     }
 }
 
@@ -166,7 +173,7 @@ mod tests {
     use serde_json::Value;
 
     #[sqlx::test(migrator = "MIGRATOR")]
-    async fn update_attribute_api_replace(pool: PgPool) {
+    async fn update_attribute_api_unsupported(pool: PgPool) {
         let state = AppState::from_pool(pool).await;
         let mut entries = EntriesBuilder::default()
             .build(state.database_client())
@@ -180,22 +187,21 @@ mod tests {
         )
         .await;
 
-        let patch = json!({"attributes": [
+        let patch = json!([
             { "op": "test", "path": "/attributeId", "value": "1" },
             { "op": "replace", "path": "/attributeId", "value": "attributeId" },
-        ]});
+        ]);
 
-        let (_, s3_object) = response_from::<S3>(
+        let (status, _) = response_from::<Value>(
             state.clone(),
             &format!("/s3/{}", entries.s3_objects[0].s3_object_id),
             Method::PATCH,
             Body::new(patch.to_string()),
         )
         .await;
+        assert_eq!(status, StatusCode::BAD_REQUEST);
 
-        change_attribute_entries(&mut entries, 0, json!({"attributeId": "attributeId"}));
-
-        assert_model_contains(&[s3_object], &entries.s3_objects, 0..1);
+        change_attribute_entries(&mut entries, 0, json!({"attributeId": "1"}));
         assert_correct_records(state.database_client(), entries).await;
     }
 
@@ -216,7 +222,7 @@ mod tests {
 
         let patch = json!({"attributes": [
             { "op": "test", "path": "/attributeId", "value": "1" },
-            { "op": "replace", "path": "/attributeId", "value": "attributeId" },
+            { "op": "add", "path": "/anotherAttribute", "value": "anotherAttribute" },
         ]});
 
         let (s3_object_status_code, _) = response_from::<Value>(
@@ -234,7 +240,7 @@ mod tests {
     }
 
     #[sqlx::test(migrator = "MIGRATOR")]
-    async fn update_collection_attributes_api_replace(pool: PgPool) {
+    async fn update_collection_attributes_api_add_nested(pool: PgPool) {
         let state = AppState::from_pool(pool).await;
         let mut entries = EntriesBuilder::default()
             .build(state.database_client())
@@ -257,7 +263,7 @@ mod tests {
 
         let patch = json!({"attributes": [
             { "op": "test", "path": "/attributeId", "value": "1" },
-            { "op": "replace", "path": "/attributeId", "value": "attributeId" },
+            { "op": "add", "path": "/anotherAttribute", "value": "anotherAttribute" },
         ]});
 
         let (_, s3_objects) = response_from::<Vec<S3>>(
@@ -268,8 +274,66 @@ mod tests {
         )
         .await;
 
-        change_attribute_entries(&mut entries, 0, json!({"attributeId": "attributeId"}));
-        change_attribute_entries(&mut entries, 1, json!({"attributeId": "attributeId"}));
+        change_attribute_entries(
+            &mut entries,
+            0,
+            json!({"attributeId": "1", "anotherAttribute": "anotherAttribute"}),
+        );
+        change_attribute_entries(
+            &mut entries,
+            1,
+            json!({"attributeId": "1", "anotherAttribute": "anotherAttribute"}),
+        );
+
+        assert_model_contains(&s3_objects, &entries.s3_objects, 0..2);
+        assert_correct_records(state.database_client(), entries).await;
+    }
+
+    #[sqlx::test(migrator = "MIGRATOR")]
+    async fn update_collection_attributes_api(pool: PgPool) {
+        let state = AppState::from_pool(pool).await;
+        let mut entries = EntriesBuilder::default()
+            .build(state.database_client())
+            .await;
+
+        change_attributes(
+            state.database_client(),
+            &entries,
+            0,
+            Some(json!({"attributeId": "1"})),
+        )
+        .await;
+        change_attributes(
+            state.database_client(),
+            &entries,
+            1,
+            Some(json!({"attributeId": "1"})),
+        )
+        .await;
+
+        let patch = json!([
+            { "op": "test", "path": "/attributeId", "value": "1" },
+            { "op": "add", "path": "/anotherAttribute", "value": "anotherAttribute" },
+        ]);
+
+        let (_, s3_objects) = response_from::<Vec<S3>>(
+            state.clone(),
+            "/s3?attributes[attributeId]=1",
+            Method::PATCH,
+            Body::new(patch.to_string()),
+        )
+        .await;
+
+        change_attribute_entries(
+            &mut entries,
+            0,
+            json!({"attributeId": "1", "anotherAttribute": "anotherAttribute"}),
+        );
+        change_attribute_entries(
+            &mut entries,
+            1,
+            json!({"attributeId": "1", "anotherAttribute": "anotherAttribute"}),
+        );
 
         assert_model_contains(&s3_objects, &entries.s3_objects, 0..2);
         assert_correct_records(state.database_client(), entries).await;
@@ -299,7 +363,7 @@ mod tests {
 
         let patch = json!({"attributes": [
             { "op": "test", "path": "/attributeId", "value": "1" },
-            { "op": "replace", "path": "/attributeId", "value": "attributeId" },
+            { "op": "add", "path": "/anotherAttribute", "value": "1" },
         ]});
 
         let (_, s3_objects) = response_from::<Vec<S3>>(
@@ -311,7 +375,8 @@ mod tests {
         .await;
 
         // Only the created event should be updated.
-        entries.s3_objects[0].attributes = Some(json!({"attributeId": "attributeId"}));
+        entries.s3_objects[0].attributes =
+            Some(json!({"attributeId": "1", "anotherAttribute": "1"}));
         entries.s3_objects[1].attributes = Some(json!({"attributeId": "1"}));
 
         assert_model_contains(&s3_objects, &entries.s3_objects, 0..1);
@@ -341,7 +406,7 @@ mod tests {
         .await;
 
         let patch = json!({"attributes": [
-            { "op": "remove", "path": "/attributeId" },
+            { "op": "add", "path": "/anotherAttribute", "value": "1" },
         ]});
 
         let (_, s3_objects) = response_from::<Vec<S3>>(

From f8b3cd7136c1d5c94ae12667faf2db58ace8344f Mon Sep 17 00:00:00 2001
From: Marko Malenic <mmalenic1@gmail.com>
Date: Fri, 16 Aug 2024 15:12:07 +1000
Subject: [PATCH 2/3] feat(filemanager): remove wildcards from enums as they
 are not necessary

---
 .../filemanager/src/queries/list.rs           | 57 +++++++------------
 .../filemanager/src/queries/update.rs         | 42 ++------------
 .../filemanager/src/routes/filter/mod.rs      | 10 ++--
 3 files changed, 29 insertions(+), 80 deletions(-)

diff --git a/lib/workload/stateless/stacks/filemanager/filemanager/src/queries/list.rs b/lib/workload/stateless/stacks/filemanager/filemanager/src/queries/list.rs
index a09d41470..b75ab0785 100644
--- a/lib/workload/stateless/stacks/filemanager/filemanager/src/queries/list.rs
+++ b/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),
@@ -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(),
+                date: Some(WildcardEither::Wildcard(Wildcard::new(
+                    "1970-01-0%".to_string(),
                 ))),
                 ..Default::default()
             },
@@ -799,23 +794,11 @@ 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.date.unwrap().to_string().starts_with("1970-01-0"))
+                .collect::<Vec<_>>()
         );
 
         let result = filter_all_s3_from(
diff --git a/lib/workload/stateless/stacks/filemanager/filemanager/src/queries/update.rs b/lib/workload/stateless/stacks/filemanager/filemanager/src/queries/update.rs
index 32c1a53e3..11d3fe36e 100644
--- a/lib/workload/stateless/stacks/filemanager/filemanager/src/queries/update.rs
+++ b/lib/workload/stateless/stacks/filemanager/filemanager/src/queries/update.rs
@@ -491,7 +491,9 @@ pub(crate) mod tests {
             .current_state()
             .filter_all(
                 S3ObjectsFilter {
-                    event_type: Some(WildcardEither::Wildcard(Wildcard::new("C%".to_string()))),
+                    date: Some(WildcardEither::Wildcard(Wildcard::new(
+                        "1970-01-0%".to_string(),
+                    ))),
                     ..Default::default()
                 },
                 true,
@@ -507,43 +509,6 @@ pub(crate) mod tests {
         assert_correct_records(&client, entries).await;
     }
 
-    #[sqlx::test(migrator = "MIGRATOR")]
-    async fn update_attributes_wildcard_ilike(pool: PgPool) {
-        let client = Client::from_pool(pool);
-        let mut entries = EntriesBuilder::default().build(&client).await;
-
-        change_many(
-            &client,
-            &entries,
-            &[0, 2, 4, 6, 8],
-            Some(json!({"attributeId": "1"})),
-        )
-        .await;
-
-        let patch = json!([
-            { "op": "add", "path": "/anotherAttribute", "value": "1" },
-        ]);
-
-        let results = UpdateQueryBuilder::<_, s3_object::Entity>::new(client.connection_ref())
-            .current_state()
-            .filter_all(
-                S3ObjectsFilter {
-                    event_type: Some(WildcardEither::Wildcard(Wildcard::new("c%".to_string()))),
-                    ..Default::default()
-                },
-                false,
-            )
-            .update_s3_attributes(PatchBody::new(from_value(patch).unwrap()))
-            .await
-            .unwrap()
-            .all()
-            .await
-            .unwrap();
-
-        assert_wildcard_update(&mut entries, &results);
-        assert_correct_records(&client, entries).await;
-    }
-
     #[sqlx::test(migrator = "MIGRATOR")]
     async fn update_attributes_add_from_null_json(pool: PgPool) {
         let client = Client::from_pool(pool);
@@ -735,6 +700,7 @@ pub(crate) mod tests {
             // Only the created event should be updated.
             entries.s3_objects[i].attributes =
                 Some(json!({"attributeId": "1", "anotherAttribute": "1"}));
+
             assert_model_contains(&[results[i / 2].clone()], &entries.s3_objects, i..i + 1);
         }
     }
diff --git a/lib/workload/stateless/stacks/filemanager/filemanager/src/routes/filter/mod.rs b/lib/workload/stateless/stacks/filemanager/filemanager/src/routes/filter/mod.rs
index 6ce645ac5..329353086 100644
--- a/lib/workload/stateless/stacks/filemanager/filemanager/src/routes/filter/mod.rs
+++ b/lib/workload/stateless/stacks/filemanager/filemanager/src/routes/filter/mod.rs
@@ -16,9 +16,9 @@ use utoipa::IntoParams;
 #[serde(default, rename_all = "camelCase")]
 #[into_params(parameter_in = Query)]
 pub struct S3ObjectsFilter {
-    #[param(required = false, value_type = Wildcard)]
-    /// Query by event type. Supports wildcards.
-    pub(crate) event_type: Option<WildcardEither<EventType>>,
+    #[param(required = false)]
+    /// Query by event type.
+    pub(crate) event_type: Option<EventType>,
     #[param(required = false)]
     /// Query by bucket. Supports wildcards.
     pub(crate) bucket: Option<Wildcard>,
@@ -43,9 +43,9 @@ pub struct S3ObjectsFilter {
     #[param(required = false)]
     /// Query by the e_tag.
     pub(crate) e_tag: Option<String>,
-    #[param(required = false, value_type = Wildcard)]
+    #[param(required = false)]
     /// Query by the storage class. Supports wildcards.
-    pub(crate) storage_class: Option<WildcardEither<StorageClass>>,
+    pub(crate) storage_class: Option<StorageClass>,
     #[param(required = false)]
     /// Query by the object delete marker.
     pub(crate) is_delete_marker: Option<bool>,

From 726fd287759ab1c08990cf8490ff0c9e0c2249d3 Mon Sep 17 00:00:00 2001
From: Marko Malenic <mmalenic1@gmail.com>
Date: Fri, 16 Aug 2024 15:34:30 +1000
Subject: [PATCH 3/3] refactor(filemanager): rename date to event_time for
 added clarity

---
 .../database/migrations/0005_rename_date.sql  |  2 ++
 .../api/select_existing_by_bucket_key.sql     |  2 +-
 .../queries/api/select_object_ids.sql         |  3 ---
 .../aws/insert_s3_created_objects.sql         |  2 +-
 .../ingester/aws/insert_s3_objects.sql        |  2 +-
 .../aws/update_reordered_for_created.sql      |  4 ++--
 .../filemanager/src/database/aws/ingester.rs  | 10 ++++-----
 .../src/database/aws/ingester_paired.rs       |  2 +-
 .../filemanager/src/database/aws/query.rs     | 21 +------------------
 .../filemanager/src/database/mod.rs           |  8 +++----
 .../filemanager/src/queries/list.rs           | 12 +++++++----
 .../filemanager/src/queries/mod.rs            |  2 +-
 .../filemanager/src/queries/update.rs         |  2 +-
 .../filemanager/src/routes/filter/mod.rs      |  4 ++--
 .../filemanager/src/routes/update.rs          |  4 ++--
 15 files changed, 32 insertions(+), 48 deletions(-)
 create mode 100644 lib/workload/stateless/stacks/filemanager/database/migrations/0005_rename_date.sql
 delete mode 100644 lib/workload/stateless/stacks/filemanager/database/queries/api/select_object_ids.sql

diff --git a/lib/workload/stateless/stacks/filemanager/database/migrations/0005_rename_date.sql b/lib/workload/stateless/stacks/filemanager/database/migrations/0005_rename_date.sql
new file mode 100644
index 000000000..967c5bafb
--- /dev/null
+++ b/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;
diff --git a/lib/workload/stateless/stacks/filemanager/database/queries/api/select_existing_by_bucket_key.sql b/lib/workload/stateless/stacks/filemanager/database/queries/api/select_existing_by_bucket_key.sql
index 1911a75b2..182a7d086 100644
--- a/lib/workload/stateless/stacks/filemanager/database/queries/api/select_existing_by_bucket_key.sql
+++ b/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,
diff --git a/lib/workload/stateless/stacks/filemanager/database/queries/api/select_object_ids.sql b/lib/workload/stateless/stacks/filemanager/database/queries/api/select_object_ids.sql
deleted file mode 100644
index 819bb0c0b..000000000
--- a/lib/workload/stateless/stacks/filemanager/database/queries/api/select_object_ids.sql
+++ /dev/null
@@ -1,3 +0,0 @@
--- Select all objects that meet regexp criteria
--- FIXME: Should not trust user input, should be a bit more robust than like/similar to
-select from s3_object where key like $1;
\ No newline at end of file
diff --git a/lib/workload/stateless/stacks/filemanager/database/queries/ingester/aws/insert_s3_created_objects.sql b/lib/workload/stateless/stacks/filemanager/database/queries/ingester/aws/insert_s3_created_objects.sql
index c91dab319..a182e1bef 100644
--- a/lib/workload/stateless/stacks/filemanager/database/queries/ingester/aws/insert_s3_created_objects.sql
+++ b/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,
diff --git a/lib/workload/stateless/stacks/filemanager/database/queries/ingester/aws/insert_s3_objects.sql b/lib/workload/stateless/stacks/filemanager/database/queries/ingester/aws/insert_s3_objects.sql
index c91dab319..a182e1bef 100644
--- a/lib/workload/stateless/stacks/filemanager/database/queries/ingester/aws/insert_s3_objects.sql
+++ b/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,
diff --git a/lib/workload/stateless/stacks/filemanager/database/queries/ingester/aws/update_reordered_for_created.sql b/lib/workload/stateless/stacks/filemanager/database/queries/ingester/aws/update_reordered_for_created.sql
index 2211b160d..ba2112daf 100644
--- a/lib/workload/stateless/stacks/filemanager/database/queries/ingester/aws/update_reordered_for_created.sql
+++ b/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,
diff --git a/lib/workload/stateless/stacks/filemanager/filemanager/src/database/aws/ingester.rs b/lib/workload/stateless/stacks/filemanager/filemanager/src/database/aws/ingester.rs
index 1eff0acf2..3d44ba3c9 100644
--- a/lib/workload/stateless/stacks/filemanager/filemanager/src/database/aws/ingester.rs
+++ b/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 {
diff --git a/lib/workload/stateless/stacks/filemanager/filemanager/src/database/aws/ingester_paired.rs b/lib/workload/stateless/stacks/filemanager/filemanager/src/database/aws/ingester_paired.rs
index 5f1e4d4b8..6cdf309fb 100644
--- a/lib/workload/stateless/stacks/filemanager/filemanager/src/database/aws/ingester_paired.rs
+++ b/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,
diff --git a/lib/workload/stateless/stacks/filemanager/filemanager/src/database/aws/query.rs b/lib/workload/stateless/stacks/filemanager/filemanager/src/database/aws/query.rs
index 568f89908..3fd449549 100644
--- a/lib/workload/stateless/stacks/filemanager/filemanager/src/database/aws/query.rs
+++ b/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(
diff --git a/lib/workload/stateless/stacks/filemanager/filemanager/src/database/mod.rs b/lib/workload/stateless/stacks/filemanager/filemanager/src/database/mod.rs
index 9a120474f..e74d2cf6b 100644
--- a/lib/workload/stateless/stacks/filemanager/filemanager/src/database/mod.rs
+++ b/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()));
     }
 }
diff --git a/lib/workload/stateless/stacks/filemanager/filemanager/src/queries/list.rs b/lib/workload/stateless/stacks/filemanager/filemanager/src/queries/list.rs
index b75ab0785..d58e4b1cc 100644
--- a/lib/workload/stateless/stacks/filemanager/filemanager/src/queries/list.rs
+++ b/lib/workload/stateless/stacks/filemanager/filemanager/src/queries/list.rs
@@ -98,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)))
@@ -784,7 +784,7 @@ pub(crate) mod tests {
         let result = filter_all_s3_from(
             &client,
             S3ObjectsFilter {
-                date: Some(WildcardEither::Wildcard(Wildcard::new(
+                event_time: Some(WildcardEither::Wildcard(Wildcard::new(
                     "1970-01-0%".to_string(),
                 ))),
                 ..Default::default()
@@ -797,7 +797,11 @@ pub(crate) mod tests {
             s3_entries
                 .clone()
                 .into_iter()
-                .filter(|entry| entry.date.unwrap().to_string().starts_with("1970-01-0"))
+                .filter(|entry| entry
+                    .event_time
+                    .unwrap()
+                    .to_string()
+                    .starts_with("1970-01-0"))
                 .collect::<Vec<_>>()
         );
 
diff --git a/lib/workload/stateless/stacks/filemanager/filemanager/src/queries/mod.rs b/lib/workload/stateless/stacks/filemanager/filemanager/src/queries/mod.rs
index 084392567..d0834633c 100644
--- a/lib/workload/stateless/stacks/filemanager/filemanager/src/queries/mod.rs
+++ b/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(),
diff --git a/lib/workload/stateless/stacks/filemanager/filemanager/src/queries/update.rs b/lib/workload/stateless/stacks/filemanager/filemanager/src/queries/update.rs
index 11d3fe36e..a324761ce 100644
--- a/lib/workload/stateless/stacks/filemanager/filemanager/src/queries/update.rs
+++ b/lib/workload/stateless/stacks/filemanager/filemanager/src/queries/update.rs
@@ -491,7 +491,7 @@ pub(crate) mod tests {
             .current_state()
             .filter_all(
                 S3ObjectsFilter {
-                    date: Some(WildcardEither::Wildcard(Wildcard::new(
+                    event_time: Some(WildcardEither::Wildcard(Wildcard::new(
                         "1970-01-0%".to_string(),
                     ))),
                     ..Default::default()
diff --git a/lib/workload/stateless/stacks/filemanager/filemanager/src/routes/filter/mod.rs b/lib/workload/stateless/stacks/filemanager/filemanager/src/routes/filter/mod.rs
index 329353086..8bc57cd8f 100644
--- a/lib/workload/stateless/stacks/filemanager/filemanager/src/routes/filter/mod.rs
+++ b/lib/workload/stateless/stacks/filemanager/filemanager/src/routes/filter/mod.rs
@@ -29,8 +29,8 @@ pub struct S3ObjectsFilter {
     /// Query by version_id. Supports wildcards.
     pub(crate) version_id: Option<Wildcard>,
     #[param(required = false, value_type = Wildcard)]
-    /// Query by date. Supports wildcards.
-    pub(crate) date: Option<WildcardEither<DateTimeWithTimeZone>>,
+    /// Query by event_time. Supports wildcards.
+    pub(crate) event_time: Option<WildcardEither<DateTimeWithTimeZone>>,
     #[param(required = false)]
     /// Query by size.
     pub(crate) size: Option<i64>,
diff --git a/lib/workload/stateless/stacks/filemanager/filemanager/src/routes/update.rs b/lib/workload/stateless/stacks/filemanager/filemanager/src/routes/update.rs
index 0d0910c9b..f07ea7752 100644
--- a/lib/workload/stateless/stacks/filemanager/filemanager/src/routes/update.rs
+++ b/lib/workload/stateless/stacks/filemanager/filemanager/src/routes/update.rs
@@ -527,7 +527,7 @@ mod tests {
 
         let (_, s3_objects) = response_from::<Vec<S3>>(
             state.clone(),
-            "/s3?eventType=C__%",
+            "/s3?currentState=true&eventTime=1970-01-0%",
             Method::PATCH,
             Body::new(patch.to_string()),
         )
@@ -559,7 +559,7 @@ mod tests {
         let (_, s3_objects) = response_from::<Vec<S3>>(
             state.clone(),
             // Percent-encoding should work too.
-            "/s3?caseSensitive=false&eventType=c%25_%25d",
+            "/s3?caseSensitive=false&currentState=true&eventTime=1970-01-_%25",
             Method::PATCH,
             Body::new(patch.to_string()),
         )