VED-881: refactor constants for batch lambdas (#1096)

* Consolidate filenameprocessor consts

* Consolidate batch_processor_filer consts

* Consolidate recordprocessor consts

* Consolidate ack_backend consts

* Consolidate batch_processor_filter env var consts

* Consolidate filenameprocessor env var consts

* Consolidate recordprocessor env var consts

* Consolidate ack_backend env var consts

* Self review

* Address review comments

* Self review round 2

* Add sonar coverage exclusion for batch_constants

---------

Co-authored-by: Akinola Olutola <akinola.olutola1@nhs.net>

Author: edhall-nhs

lambdas/ack_backend/src/audit_table.py

@@ -1,8 +1,8 @@
 """Add the filename to the audit table and check for duplicates."""
 
 from common.clients import dynamodb_client, logger
+from common.models.batch_constants import AUDIT_TABLE_NAME, AuditTableKeys, FileStatus
 from common.models.errors import UnhandledAuditTableError
-from constants import AUDIT_TABLE_NAME, AuditTableKeys, FileStatus
 
 CONDITION_EXPRESSION = "attribute_exists(message_id)"
 

lambdas/ack_backend/src/constants.py

@@ -1,48 +1,11 @@
 """Constants for ack lambda"""
 
-import os
-
-AUDIT_TABLE_NAME = os.getenv("AUDIT_TABLE_NAME")
-
 COMPLETED_ACK_DIR = "forwardedFile"
 TEMP_ACK_DIR = "TempAck"
 BATCH_FILE_PROCESSING_DIR = "processing"
 BATCH_FILE_ARCHIVE_DIR = "archive"
 
 
-def get_source_bucket_name() -> str:
-    """Get the SOURCE_BUCKET_NAME environment from environment variables."""
-    return os.getenv("SOURCE_BUCKET_NAME")
-
-
-def get_ack_bucket_name() -> str:
-    """Get the ACK_BUCKET_NAME environment from environment variables."""
-    return os.getenv("ACK_BUCKET_NAME")
-
-
-class FileStatus:
-    """File status constants"""
-
-    QUEUED = "Queued"
-    PROCESSING = "Processing"
-    PROCESSED = "Processed"
-    DUPLICATE = "Duplicate"
-
-
-class AuditTableKeys:
-    """Audit table keys"""
-
-    FILENAME = "filename"
-    MESSAGE_ID = "message_id"
-    QUEUE_NAME = "queue_name"
-    RECORD_COUNT = "record_count"
-    STATUS = "status"
-    TIMESTAMP = "timestamp"
-    INGESTION_END_TIME = "ingestion_end_time"
-    RECORDS_SUCCEEDED = "records_succeeded"
-    RECORDS_FAILED = "records_failed"
-
-
 ACK_HEADERS = [
     "MESSAGE_HEADER_ID",
     "HEADER_RESPONSE_CODE",

lambdas/ack_backend/src/update_ack_file.py

@@ -12,14 +12,13 @@
 )
 from common.aws_s3_utils import move_file
 from common.clients import get_s3_client, logger
+from common.models.batch_constants import ACK_BUCKET_NAME, SOURCE_BUCKET_NAME
 from constants import (
     ACK_HEADERS,
     BATCH_FILE_ARCHIVE_DIR,
     BATCH_FILE_PROCESSING_DIR,
     COMPLETED_ACK_DIR,
     TEMP_ACK_DIR,
-    get_ack_bucket_name,
-    get_source_bucket_name,
 )
 from logging_decorators import complete_batch_file_process_logging_decorator
 
@@ -71,10 +70,8 @@ def complete_batch_file_process(
     the audit table status"""
     ack_filename = f"{file_key.replace('.csv', f'_BusAck_{created_at_formatted_string}.csv')}"
 
-    move_file(get_ack_bucket_name(), f"{TEMP_ACK_DIR}/{ack_filename}", f"{COMPLETED_ACK_DIR}/{ack_filename}")
-    move_file(
-        get_source_bucket_name(), f"{BATCH_FILE_PROCESSING_DIR}/{file_key}", f"{BATCH_FILE_ARCHIVE_DIR}/{file_key}"
-    )
+    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}")
 
     total_ack_rows_processed, total_failures = get_record_count_and_failures_by_message_id(message_id)
     change_audit_table_status_to_processed(file_key, message_id)
@@ -99,7 +96,7 @@ def obtain_current_ack_content(temp_ack_file_key: str) -> StringIO:
     """Returns the current ack file content if the file exists, or else initialises the content with the ack headers."""
     try:
         # If ack file exists in S3 download the contents
-        existing_ack_file = get_s3_client().get_object(Bucket=get_ack_bucket_name(), Key=temp_ack_file_key)
+        existing_ack_file = get_s3_client().get_object(Bucket=ACK_BUCKET_NAME, Key=temp_ack_file_key)
         existing_content = existing_ack_file["Body"].read().decode("utf-8")
     except ClientError as error:
         # If ack file does not exist in S3 create a new file containing the headers only
@@ -132,7 +129,6 @@ def update_ack_file(
         accumulated_csv_content.write(cleaned_row + "\n")
 
     csv_file_like_object = BytesIO(accumulated_csv_content.getvalue().encode("utf-8"))
-    ack_bucket_name = get_ack_bucket_name()
 
-    get_s3_client().upload_fileobj(csv_file_like_object, ack_bucket_name, temp_ack_file_key)
-    logger.info("Ack file updated to %s: %s", ack_bucket_name, completed_ack_file_key)
+    get_s3_client().upload_fileobj(csv_file_like_object, ACK_BUCKET_NAME, temp_ack_file_key)
+    logger.info("Ack file updated to %s: %s", ACK_BUCKET_NAME, completed_ack_file_key)

lambdas/ack_backend/tests/test_ack_processor.py

@@ -63,6 +63,12 @@ def setUp(self) -> None:
         self.logger_info_patcher = patch("common.log_decorator.logger.info")
         self.mock_logger_info = self.logger_info_patcher.start()
 
+        self.ack_bucket_patcher = patch("update_ack_file.ACK_BUCKET_NAME", BucketNames.DESTINATION)
+        self.ack_bucket_patcher.start()
+
+        self.source_bucket_patcher = patch("update_ack_file.SOURCE_BUCKET_NAME", BucketNames.SOURCE)
+        self.source_bucket_patcher.start()
+
     def tearDown(self) -> None:
         GenericTearDown(self.s3_client, self.firehose_client, self.dynamodb_client)
         self.mock_logger_info.stop()

lambdas/ack_backend/tests/test_audit_table.py

@@ -2,8 +2,8 @@
 from unittest.mock import call, patch
 
 import audit_table
+from common.models.batch_constants import AUDIT_TABLE_NAME, AuditTableKeys, FileStatus
 from common.models.errors import UnhandledAuditTableError
-from constants import AUDIT_TABLE_NAME, AuditTableKeys, FileStatus
 
 
 class TestAuditTable(unittest.TestCase):

lambdas/ack_backend/tests/test_logging_decorators.py

@@ -2,6 +2,7 @@
 from unittest.mock import patch
 
 import logging_decorators
+from utils.mock_environment_variables import BucketNames
 
 
 class TestLoggingDecorators(unittest.TestCase):
@@ -12,6 +13,9 @@ def setUp(self):
         self.firehose_patcher = patch("common.log_firehose.firehose_client")
         self.mock_firehose = self.firehose_patcher.start()
 
+        self.source_bucket_patcher = patch("update_ack_file.SOURCE_BUCKET_NAME", BucketNames.SOURCE)
+        self.source_bucket_patcher.start()
+
     def tearDown(self):
         self.logger_patcher.stop()
         self.firehose_patcher.stop()

lambdas/ack_backend/tests/test_splunk_logging.py

@@ -45,6 +45,9 @@ def setUp(self):
             Body=mock_source_file_with_100_rows.getvalue(),
         )
 
+        self.ack_bucket_patcher = patch("update_ack_file.ACK_BUCKET_NAME", BucketNames.DESTINATION)
+        self.ack_bucket_patcher.start()
+
     def tearDown(self):
         GenericTearDown(self.s3_client)
 

lambdas/ack_backend/tests/test_update_ack_file.py

@@ -57,6 +57,9 @@ def setUp(self) -> None:
         self.logger_patcher = patch("update_ack_file.logger")
         self.mock_logger = self.logger_patcher.start()
 
+        self.ack_bucket_patcher = patch("update_ack_file.ACK_BUCKET_NAME", BucketNames.DESTINATION)
+        self.ack_bucket_patcher.start()
+
     def tearDown(self) -> None:
         GenericTearDown(self.s3_client)
 

lambdas/ack_backend/tests/test_update_ack_file_flow.py

@@ -5,23 +5,21 @@
 from moto import mock_aws
 
 import update_ack_file
+from utils.mock_environment_variables import BucketNames
 
 
 @mock_aws
 class TestUpdateAckFileFlow(unittest.TestCase):
     def setUp(self):
         self.s3_client = boto3.client("s3", region_name="eu-west-2")
 
-        self.ack_bucket_name = "my-ack-bucket"
-        self.source_bucket_name = "my-source-bucket"
-        self.ack_bucket_patcher = patch("update_ack_file.get_ack_bucket_name", return_value=self.ack_bucket_name)
-        self.mock_get_ack_bucket_name = self.ack_bucket_patcher.start()
+        self.ack_bucket_name = BucketNames.DESTINATION
+        self.source_bucket_name = BucketNames.SOURCE
+        self.ack_bucket_patcher = patch("update_ack_file.ACK_BUCKET_NAME", self.ack_bucket_name)
+        self.ack_bucket_patcher.start()
 
-        self.source_bucket_patcher = patch(
-            "update_ack_file.get_source_bucket_name",
-            return_value=self.source_bucket_name,
-        )
-        self.mock_get_source_bucket_name = self.source_bucket_patcher.start()
+        self.source_bucket_patcher = patch("update_ack_file.SOURCE_BUCKET_NAME", self.source_bucket_name)
+        self.source_bucket_patcher.start()
 
         self.s3_client.create_bucket(
             Bucket=self.ack_bucket_name,

lambdas/ack_backend/tests/utils/generic_setup_and_teardown_for_ack_backend.py

@@ -1,6 +1,6 @@
 """Generic setup and teardown for ACK backend tests"""
 
-from constants import AuditTableKeys
+from common.models.batch_constants import AuditTableKeys
 from tests.utils.mock_environment_variables import AUDIT_TABLE_NAME, REGION_NAME, BucketNames, Firehose