VED-1072: Ack filenames (part 2) (#1234)

* VED-1072 ack file extension fixes

* os.path & update_json_ack_file

* pt.2 / file_utils

* updated  batch file scenario with dat extension

* ruff

* duplicate function name

* subTest

---------

Co-authored-by: FimranNHS <fatima.imran3@nhs.net>

Author: JamesW1-NHS

lambdas/ack_backend/src/update_ack_file.py

@@ -16,6 +16,7 @@
     update_audit_table_item,
 )
 from common.clients import get_s3_client, logger
+from common.file_utils import get_file_key_without_ext
 from common.log_decorator import generate_and_send_logs
 from common.models.batch_constants import (
     ACK_BUCKET_NAME,
@@ -142,7 +143,7 @@ def complete_batch_file_process(
     start_time = time.time()
 
     # finish CSV file
-    file_key_without_ext = os.path.splitext(file_key)[0]
+    file_key_without_ext = get_file_key_without_ext(file_key)
     ack_filename = f"{file_key_without_ext}_BusAck_{created_at_formatted_string}.csv"
 
     move_file(ACK_BUCKET_NAME, f"{TEMP_ACK_DIR}/{ack_filename}", f"{COMPLETED_ACK_DIR}/{ack_filename}")
@@ -247,7 +248,7 @@ def obtain_current_json_ack_content(message_id: str, supplier: str, file_key: st
             logger.info("No existing JSON ack file found in S3 - creating new file")
 
             ingestion_start_time = get_ingestion_start_time_by_message_id(message_id)
-            raw_ack_filename = os.path.splitext(file_key)[0]
+            raw_ack_filename = get_file_key_without_ext(file_key)
 
             # Generate the initial fields
             return _make_ack_data_dict_identifier_information(
@@ -269,7 +270,7 @@ def update_csv_ack_file(
     ack_data_rows: list,
 ) -> None:
     """Updates the ack file with the new data row based on the given arguments"""
-    file_key_without_ext = os.path.splitext(file_key)[0]
+    file_key_without_ext = get_file_key_without_ext(file_key)
     ack_filename = f"{file_key_without_ext}_BusAck_{created_at_formatted_string}.csv"
     temp_ack_file_key = f"{TEMP_ACK_DIR}/{ack_filename}"
     accumulated_csv_content = obtain_current_csv_ack_content(temp_ack_file_key)
@@ -293,7 +294,7 @@ def update_json_ack_file(
     ack_data_rows: list,
 ) -> None:
     """Updates the ack file with the new data row based on the given arguments"""
-    file_key_without_ext = os.path.splitext(file_key)[0]
+    file_key_without_ext = get_file_key_without_ext(file_key)
     ack_filename = f"{file_key_without_ext}_BusAck_{created_at_formatted_string}.json"
     temp_ack_file_key = f"{TEMP_ACK_DIR}/{ack_filename}"
     ack_data_dict = obtain_current_json_ack_content(message_id, supplier, file_key, temp_ack_file_key)

lambdas/batch_processor_filter/src/batch_file_repository.py

@@ -1,11 +1,11 @@
 """Module for the batch file repository"""
 
-import os
 from csv import writer
 from io import BytesIO, StringIO
 
 from batch_file_created_event import BatchFileCreatedEvent
 from common.clients import get_s3_client
+from common.file_utils import get_file_key_without_ext
 from common.models.batch_constants import ACK_BUCKET_NAME, SOURCE_BUCKET_NAME
 
 
@@ -49,7 +49,7 @@ def move_source_file_to_archive(self, file_key: str) -> None:
     def upload_failure_ack(self, batch_file_created_event: BatchFileCreatedEvent) -> None:
         ack_failure_data = self._create_ack_failure_data(batch_file_created_event)
 
-        filename_without_ext = os.path.splitext(batch_file_created_event["filename"])[0]
+        filename_without_ext = get_file_key_without_ext(batch_file_created_event["filename"])
         ack_filename = f"ack/{filename_without_ext}_InfAck_{batch_file_created_event['created_at_formatted_string']}.csv"
 
         # Create CSV file with | delimiter, filetype .csv

lambdas/shared/src/common/ack_file_utils.py

@@ -1,10 +1,10 @@
 """Create ack file and upload to S3 bucket"""
 
-import os
 from csv import writer
 from io import BytesIO, StringIO
 
 from common.clients import get_s3_client
+from common.file_utils import get_file_key_without_ext
 from common.models.batch_constants import ACK_BUCKET_NAME
 
 
@@ -36,7 +36,7 @@ def make_ack_data(
 
 def upload_ack_file(file_key: str, ack_data: dict, created_at_formatted_string: str) -> None:
     """Formats the ack data into a csv file and uploads it to the ack bucket"""
-    file_key_without_ext = os.path.splitext(file_key)[0]
+    file_key_without_ext = get_file_key_without_ext(file_key)
     ack_filename = f"ack/{file_key_without_ext}_InfAck_{created_at_formatted_string}.csv"
 
     # Create CSV file with | delimiter, filetype .csv

lambdas/shared/src/common/file_utils.py

@@ -0,0 +1,7 @@
+"""General file utils"""
+
+import os
+
+
+def get_file_key_without_ext(file_key: str) -> str:
+    return os.path.splitext(file_key)[0]

lambdas/shared/tests/test_common/test_file_utils.py

@@ -0,0 +1,19 @@
+import unittest
+
+import common.file_utils
+
+
+class TestFileUtils(unittest.TestCase):
+    def test_get_file_key_without_ext(self):
+        """Test get_file_key_without_ext returns the expected values"""
+
+        test_cases = [
+            ("file_key.csv", "file_key"),
+            ("file_key.dat", "file_key"),
+            ("file_key", "file_key"),
+            ("/mnt/c/dir.a/file_key.csv", "/mnt/c/dir.a/file_key"),
+        ]
+
+        for file_key, file_key_without_ext in test_cases:
+            with self.subTest():
+                self.assertEqual(common.file_utils.get_file_key_without_ext(file_key), file_key_without_ext)

tests/e2e_automation/features/batchTests/Steps/batch_common_steps.py

@@ -29,7 +29,11 @@
     validate_json_bus_ack_file_failure_records,
     validate_json_bus_ack_file_structure_and_metadata,
 )
-from utilities.batch_S3_buckets import upload_file_to_S3, wait_and_read_ack_file, wait_for_file_to_move_archive
+from utilities.batch_S3_buckets import (
+    upload_file_to_S3,
+    wait_and_read_ack_file,
+    wait_for_file_to_move_archive,
+)
 from utilities.enums import ActionFlag, ActionMap, Operation
 
 
@@ -86,6 +90,13 @@ def valid_batch_file_is_created_with_details(datatable, context):
     create_batch_file(context)
 
 
+@given("batch file is created for below data as full dataset with file extension dat")
+@ignore_if_local_run
+def valid_batch_file_is_created_with_details_and_dat_extension(datatable, context):
+    build_dataFrame_using_datatable(datatable, context)
+    create_batch_file(context, file_ext="dat")
+
+
 @when("same batch file is uploaded again in s3 bucket")
 @when("batch file is uploaded in s3 bucket")
 @ignore_local_run_set_test_data
@@ -140,7 +151,9 @@ def json_bus_ack_will_only_contain_file_metadata_and_no_record_entries(context):
 
 
 @then("Json bus ack will only contain file metadata and correct failure record entries")
-def json_bus_ack_will_only_contain_file_metadata_and_correct_failure_record_entries(context):
+def json_bus_ack_will_only_contain_file_metadata_and_correct_failure_record_entries(
+    context,
+):
     json_content = context.fileContentJson
     assert json_content is not None, "BUS Ack JSON content is None"
     validate_json_bus_ack_file_structure_and_metadata(context)
@@ -225,7 +238,11 @@ def validate_imms_event_table_for_all_records_in_batch_file(context, operation:
             ("Operation", Operation[operation].value, item.get("Operation")),
             ("SupplierSystem", context.supplier_name, item.get("SupplierSystem")),
             ("PatientPK", f"Patient#{nhs_number}", item.get("PatientPK")),
-            ("PatientSK", f"{context.vaccine_type.upper()}#{context.ImmsID}", item.get("PatientSK")),
+            (
+                "PatientSK",
+                f"{context.vaccine_type.upper()}#{context.ImmsID}",
+                item.get("PatientSK"),
+            ),
             ("Version", int(context.expected_version), int(item.get("Version"))),
         ]
 
@@ -269,7 +286,13 @@ def build_dataFrame_using_datatable(datatable, context):
     headers = datatable[0]
     rows = datatable[1:]
 
-    table_list = [(row[headers.index("patient_id")], f"{row[headers.index('unique_id')]}-{timestamp}") for row in rows]
+    table_list = [
+        (
+            row[headers.index("patient_id")],
+            f"{row[headers.index('unique_id')]}-{timestamp}",
+        )
+        for row in rows
+    ]
     records = []
     for patient_id, unique_id in table_list:
         context.patient_id = patient_id
@@ -315,7 +338,8 @@ def validate_imms_delta_table_for_newly_created_records_in_batch_file(context):
         create_items = [i for i in delta_items if i.get("Operation") == "CREATE"]
 
         check.is_true(
-            len(create_items) == 1, f"Expected exactly 1 CREATE record for IMMS_ID {clean_id}, found {len(create_items)}"
+            len(create_items) == 1,
+            f"Expected exactly 1 CREATE record for IMMS_ID {clean_id}, found {len(create_items)}",
         )
 
         create_item = create_items[0]
@@ -324,7 +348,11 @@ def validate_imms_delta_table_for_newly_created_records_in_batch_file(context):
             batch_record = {k: normalize(v) for k, v in row.to_dict().items()}
 
             validate_imms_delta_record_with_batch_record(
-                context, batch_record, create_item, Operation.created.value, ActionFlag.created.value
+                context,
+                batch_record,
+                create_item,
+                Operation.created.value,
+                ActionFlag.created.value,
             )
 
 
@@ -341,7 +369,11 @@ def validate_imms_delta_table_for_updated_records_in_batch_file(context):
             item = update_items.pop(updated_index)
 
             validate_imms_delta_record_with_batch_record(
-                context, batch_record, item, Operation.updated.value, ActionFlag.updated.value
+                context,
+                batch_record,
+                item,
+                Operation.updated.value,
+                ActionFlag.updated.value,
             )
 
 
@@ -365,5 +397,9 @@ def validate_imms_delta_table_for_deleted_records_in_batch_file(context):
         batch_record = {k: normalize(v) for k, v in row.to_dict().items()}
 
         validate_imms_delta_record_with_batch_record(
-            context, batch_record, delete_item, Operation.deleted.value, ActionFlag.deleted.value
+            context,
+            batch_record,
+            delete_item,
+            Operation.deleted.value,
+            ActionFlag.deleted.value,
         )