VED-1072: Ack file extension fixes (#1231)

* VED-1072 ack file extension fixes

* os.path & update_json_ack_file

Author: JamesW1-NHS

lambdas/ack_backend/src/update_ack_file.py

@@ -142,7 +142,8 @@ def complete_batch_file_process(
     start_time = time.time()
 
     # finish CSV file
-    ack_filename = f"{file_key.replace('.csv', f'_BusAck_{created_at_formatted_string}.csv')}"
+    file_key_without_ext = os.path.splitext(file_key)[0]
+    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}")
     move_file(SOURCE_BUCKET_NAME, f"{BATCH_FILE_PROCESSING_DIR}/{file_key}", f"{BATCH_FILE_ARCHIVE_DIR}/{file_key}")
@@ -164,7 +165,7 @@ def complete_batch_file_process(
     )
 
     # finish JSON file
-    json_ack_filename = f"{file_key.replace('.csv', f'_BusAck_{created_at_formatted_string}.json')}"
+    json_ack_filename = f"{file_key_without_ext}_BusAck_{created_at_formatted_string}.json"
     temp_ack_file_key = f"{TEMP_ACK_DIR}/{json_ack_filename}"
     ack_data_dict = obtain_current_json_ack_content(message_id, supplier, file_key, temp_ack_file_key)
 
@@ -246,7 +247,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 = file_key.split(".")[0]
+            raw_ack_filename = os.path.splitext(file_key)[0]
 
             # Generate the initial fields
             return _make_ack_data_dict_identifier_information(
@@ -268,7 +269,8 @@ def update_csv_ack_file(
     ack_data_rows: list,
 ) -> None:
     """Updates the ack file with the new data row based on the given arguments"""
-    ack_filename = f"{file_key.replace('.csv', f'_BusAck_{created_at_formatted_string}.csv')}"
+    file_key_without_ext = os.path.splitext(file_key)[0]
+    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)
 
@@ -291,7 +293,8 @@ def update_json_ack_file(
     ack_data_rows: list,
 ) -> None:
     """Updates the ack file with the new data row based on the given arguments"""
-    ack_filename = f"{file_key.replace('.csv', f'_BusAck_{created_at_formatted_string}.json')}"
+    file_key_without_ext = os.path.splitext(file_key)[0]
+    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/ack_backend/tests/test_splunk_logging.py

@@ -167,13 +167,12 @@ def test_splunk_logging_successful_rows(self):
 
     def test_splunk_logging_missing_data(self):
         """Tests missing key values in the body of the event"""
-
         with (  # noqa: E999
             patch("common.log_decorator.send_log_to_firehose") as mock_send_log_to_firehose,  # noqa: E999
             patch("common.log_decorator.logger") as mock_logger,  # noqa: E999
             patch("ack_processor.increment_records_failed_count"),  # noqa: E999
         ):  # noqa: E999
-            with self.assertRaises(AttributeError):
+            with self.assertRaises(TypeError):
                 lambda_handler(event={"Records": [{"body": json.dumps([{"": "456", "row_id": "test^1"}])}]}, context={})
 
             expected_first_logger_info_data = {**InvalidValues.logging_with_no_values}
@@ -182,7 +181,7 @@ def test_splunk_logging_missing_data(self):
                 success=False,
                 number_of_rows=1,
                 ingestion_complete=False,
-                diagnostics="'NoneType' object has no attribute 'replace'",
+                diagnostics="expected str, bytes or os.PathLike object, not NoneType",
             )
 
             first_logger_info_call_args = json.loads(self.extract_all_call_args_for_logger_info(mock_logger)[0])

lambdas/ack_backend/tests/test_update_ack_file.py

@@ -433,6 +433,63 @@ def test_complete_batch_file_process_json_ack_file(self):
         )
         self.assertEqual(result, ValidValues.json_ack_complete_content)
 
+    def test_complete_batch_file_process_with_dat_extension(self):
+        """Test that complete_batch_file_process correctly handles files with .dat extension."""
+        file_key_with_dat = "flu_Vaccinations_v5_YGA_20210730T12000000.dat"
+        created_at_formatted_string = "20211120T12000000"
+        expected_csv_ack_file_key = (
+            f"forwardedFile/flu_Vaccinations_v5_YGA_20210730T12000000_BusAck_{created_at_formatted_string}.csv"
+        )
+        expected_json_ack_file_key = (
+            f"forwardedFile/flu_Vaccinations_v5_YGA_20210730T12000000_BusAck_{created_at_formatted_string}.json"
+        )
+
+        generate_sample_existing_json_ack_content()
+        self.s3_client.put_object(
+            Bucket=BucketNames.SOURCE,
+            Key=f"processing/{file_key_with_dat}",
+            Body="dummy content",
+        )
+
+        update_csv_ack_file(
+            file_key=file_key_with_dat,
+            created_at_formatted_string=created_at_formatted_string,
+            ack_data_rows=[ValidValues.ack_data_failure_dict],
+        )
+        update_json_ack_file(
+            message_id=MOCK_MESSAGE_DETAILS.message_id,
+            supplier=MOCK_MESSAGE_DETAILS.supplier,
+            file_key=file_key_with_dat,
+            created_at_formatted_string=created_at_formatted_string,
+            ack_data_rows=[ValidValues.ack_data_failure_dict],
+        )
+
+        self.mock_get_record_and_failure_count.return_value = 10, 1
+
+        complete_batch_file_process(
+            message_id=MOCK_MESSAGE_DETAILS.message_id,
+            supplier=MOCK_MESSAGE_DETAILS.supplier,
+            vaccine_type=MOCK_MESSAGE_DETAILS.vaccine_type,
+            created_at_formatted_string=created_at_formatted_string,
+            file_key=file_key_with_dat,
+        )
+
+        # Verify CSV ACK file was created with .csv extension
+        try:
+            self.s3_client.head_object(Bucket=BucketNames.DESTINATION, Key=expected_csv_ack_file_key)
+            csv_ack_exists = True
+        except Exception:
+            csv_ack_exists = False
+        self.assertTrue(csv_ack_exists, f"Expected CSV ACK file at {expected_csv_ack_file_key}")
+
+        # Verify JSON ACK file was created with .json extension (not .dat.json)
+        try:
+            self.s3_client.head_object(Bucket=BucketNames.DESTINATION, Key=expected_json_ack_file_key)
+            json_ack_exists = True
+        except Exception:
+            json_ack_exists = False
+        self.assertTrue(json_ack_exists, f"Expected JSON ACK file at {expected_json_ack_file_key}")
+
     def test_update_json_ack_file_with_empty_ack_data_rows(self):
         """Test that update_json_ack_file correctly updates the ack file when given an empty list"""
         # Mock existing content in the ack file

lambdas/batch_processor_filter/src/batch_file_repository.py

@@ -1,5 +1,6 @@
 """Module for the batch file repository"""
 
+import os
 from csv import writer
 from io import BytesIO, StringIO
 
@@ -48,10 +49,8 @@ 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)
 
-        ack_filename = "ack/" + batch_file_created_event["filename"].replace(
-            ".csv",
-            f"_InfAck_{batch_file_created_event['created_at_formatted_string']}.csv",
-        )
+        filename_without_ext = os.path.splitext(batch_file_created_event["filename"])[0]
+        ack_filename = f"ack/{filename_without_ext}_InfAck_{batch_file_created_event['created_at_formatted_string']}.csv"
 
         # Create CSV file with | delimiter, filetype .csv
         csv_buffer = StringIO()

lambdas/filenameprocessor/tests/test_lambda_handler.py

@@ -1,6 +1,7 @@
 """Tests for lambda_handler"""
 
 import json
+import os
 from contextlib import ExitStack
 from copy import deepcopy
 from json import loads as json_loads
@@ -120,7 +121,8 @@ def get_ack_file_key(
         created_at_formatted_string: str = MOCK_CREATED_AT_FORMATTED_STRING,
     ) -> str:
         """Returns the ack file key for the given file key"""
-        return f"ack/{file_key.replace('.csv', '_InfAck_' + created_at_formatted_string + '.csv')}"
+        file_key_without_ext = os.path.splitext(file_key)[0]
+        return f"ack/{file_key_without_ext}_InfAck_{created_at_formatted_string}.csv"
 
     @staticmethod
     def generate_expected_failure_inf_ack_content(message_id: str, created_at_formatted_string: str) -> str:

lambdas/filenameprocessor/tests/utils_for_tests/values_for_tests.py

@@ -1,5 +1,6 @@
 """File of values which can be used for testing"""
 
+import os
 from datetime import datetime
 from unittest.mock import patch
 
@@ -54,7 +55,8 @@ def __init__(
 
         self.file_key = file_key
 
-        self.ack_file_key = f"ack/{self.file_key[:-4]}_InfAck_{self.created_at_formatted_string}.csv"
+        file_key_without_ext = os.path.splitext(self.file_key)[0]
+        self.ack_file_key = f"ack/{file_key_without_ext}_InfAck_{self.created_at_formatted_string}.csv"
 
         self.permissions_list = [f"{self.vaccine_type}_FULL"]
         self.permissions_config = {self.supplier: self.permissions_list}

lambdas/shared/src/common/ack_file_utils.py

@@ -1,5 +1,6 @@
 """Create ack file and upload to S3 bucket"""
 
+import os
 from csv import writer
 from io import BytesIO, StringIO
 
@@ -35,7 +36,8 @@ 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"""
-    ack_filename = "ack/" + file_key.replace(".csv", f"_InfAck_{created_at_formatted_string}.csv")
+    file_key_without_ext = os.path.splitext(file_key)[0]
+    ack_filename = f"ack/{file_key_without_ext}_InfAck_{created_at_formatted_string}.csv"
 
     # Create CSV file with | delimiter, filetype .csv
     csv_buffer = StringIO()

lambdas/shared/tests/test_common/test_ack_file_utils.py

@@ -145,6 +145,24 @@ def test_upload_ack_file_failure(self):
         )
         self.assertEqual(list(csv_dict_reader), expected_result)
 
+    def test_upload_ack_file_with_dat_extension(self):
+        """Test that upload_ack_file generates correct filename when source file has .dat extension"""
+
+        # Create a file_key with .dat extension
+        file_key_with_dat = "flu_Vaccinations_v5_YGA_20210730T12000000.dat"
+        expected_ack_file_key = "ack/flu_Vaccinations_v5_YGA_20210730T12000000_InfAck_20211120T12000000.csv"
+        upload_ack_file(
+            file_key=file_key_with_dat,
+            ack_data=deepcopy(self.ack_data_validation_passed_and_message_delivered),
+            created_at_formatted_string=self.created_at_formatted_string,
+        )
+
+        expected_result = [deepcopy(self.ack_data_validation_passed_and_message_delivered)]
+        # Note that the data downloaded from the CSV will contain the bool as a string
+        expected_result[0]["MESSAGE_DELIVERY"] = "True"
+        csv_dict_reader = get_csv_file_dict_reader(self.s3_client, BucketNames.DESTINATION, expected_ack_file_key)
+        self.assertEqual(list(csv_dict_reader), expected_result)
+
     def test_make_and_upload_ack_file_success(self):
         """Test that make_and_upload_ack_file uploads an ack file containing the correct values"""
         make_and_upload_ack_file(