Merge pull request #335 from NHSDigital/AMB_2288-Upgrade-test-suite-filenameprocessor

Add unittests for cache and logging functions and move cache function…

Author: AlexandraBenson

.github/workflows/sonarcloud.yml

@@ -34,7 +34,7 @@ jobs:
         id: filenameprocessor
         continue-on-error: true
         run: |
-          pip install poetry==1.8.4 moto==4.2.11 coverage redis botocore==1.35.49 simplejson pandas freezegun responses structlog fhir.resources jsonpath_ng pydantic==1.10.13 requests aws-lambda-typing cffi pyjwt boto3-stubs-lite[dynamodb]~=1.26.90 python-stdnum==1.20
+          pip install poetry==1.8.4 moto==4.2.11 coverage redis botocore==1.35.49 simplejson pandas freezegun responses structlog fhir.resources jsonpath_ng pydantic==1.10.13 requests aws-lambda-typing cffi pyjwt boto3-stubs-lite[dynamodb]~=1.26.90 python-stdnum==1.20 fakeredis
           poetry run coverage run --source=filenameprocessor -m unittest discover -s filenameprocessor || echo "filenameprocessor tests failed" >> failed_tests.txt
           poetry run coverage xml -o sonarcloud-coverage-filenameprocessor-coverage.xml
 

filenameprocessor/poetry.lock

@@ -25,6 +25,7 @@ structlog = "^24.1.0"
 redis = "^5.1.1"
 coverage = "7.6.1"
 freezegun = "^1.5.1"
+fakeredis = "^2.0.0" 
 
 [build-system]
 requires = ["poetry-core >= 1.5.0"]

filenameprocessor/pyproject.toml

@@ -1,19 +1,21 @@
-"""Initialise clients, resources and logger. Note that all clients, resources and logger for the filenameprocessor
-lambda should be initialised ONCE ONLY (in this file) and then imported into the files where they are needed."""
+"""
+Initialise clients, resources and logger. Note that all clients, resources and logger for the filenameprocessor
+lambda should be initialised ONCE ONLY (in this file) and then imported into the files where they are needed.
+"""
 
 import os
 import logging
 import redis
 from boto3 import client as boto3_client, resource as boto3_resource
 
-# AWS Clients and Resources
+# AWS Clients and Resources.
 REGION_NAME = "eu-west-2"
 
 s3_client = boto3_client("s3", region_name=REGION_NAME)
 sqs_client = boto3_client("sqs", region_name=REGION_NAME)
 dynamodb_client = boto3_client("dynamodb", region_name=REGION_NAME)
 firehose_client = boto3_client("firehose", region_name=REGION_NAME)
-lambda_client = boto3_client('lambda', region_name=REGION_NAME)
+lambda_client = boto3_client("lambda", region_name=REGION_NAME)
 
 dynamodb_resource = boto3_resource("dynamodb", region_name=REGION_NAME)
 

filenameprocessor/src/clients.py

@@ -16,6 +16,8 @@
 AUDIT_TABLE_QUEUE_NAME_GSI = "queue_name_index"
 AUDIT_TABLE_FILENAME_GSI = "filename_index"
 
+PERMISSIONS_CONFIG_FILE_KEY = "permissions_config.json"
+
 ERROR_TYPE_TO_STATUS_CODE_MAP = {
     VaccineTypePermissionsError: 403,
     InvalidFileKeyError: 400,  # Includes invalid ODS code, therefore unable to identify supplier

filenameprocessor/src/constants.py

@@ -1,5 +1,8 @@
 "Upload the content from a config file in S3 to ElastiCache (Redis)"
+
+import json
 from clients import s3_client, redis_client
+from constants import PERMISSIONS_CONFIG_FILE_KEY
 
 
 def upload_to_elasticache(file_key: str, bucket_name: str) -> None:
@@ -8,3 +11,8 @@ def upload_to_elasticache(file_key: str, bucket_name: str) -> None:
     config_file_content = config_file["Body"].read().decode("utf-8")
     # Use the file_key as the Redis key and file content as the value
     redis_client.set(file_key, config_file_content)
+
+
+def get_permissions_config_json_from_cache() -> dict:
+    """Gets and returns the permissions config file content from ElastiCache (Redis)."""
+    return json.loads(redis_client.get(PERMISSIONS_CONFIG_FILE_KEY))

filenameprocessor/src/elasticcache.py filenameprocessor/src/elasticache.pyfilenameprocessor/src/elasticcache.py renamed to filenameprocessor/src/elasticache.py

@@ -13,7 +13,7 @@
 from make_and_upload_ack_file import make_and_upload_the_ack_file
 from audit_table import upsert_audit_table, get_next_queued_file_details, ensure_file_is_not_a_duplicate
 from clients import logger
-from elasticcache import upload_to_elasticache
+from elasticache import upload_to_elasticache
 from logging_decorator import logging_decorator
 from supplier_permissions import validate_vaccine_type_permissions
 from errors import (

filenameprocessor/src/file_name_processor.py

@@ -1,15 +1,8 @@
 """Functions for fetching supplier permissions"""
 
-import json
-from clients import redis_client, logger
+from clients import logger
 from errors import VaccineTypePermissionsError
-
-PERMISSIONS_CONFIG_FILE_KEY = "permissions_config.json"
-
-
-def get_permissions_config_json_from_cache() -> dict:
-    """Gets and returns the permissions config file content from ElastiCache."""
-    return json.loads(redis_client.get(PERMISSIONS_CONFIG_FILE_KEY))
+from elasticache import get_permissions_config_json_from_cache
 
 
 def get_supplier_permissions(supplier: str) -> list:

filenameprocessor/src/supplier_permissions.py

@@ -1,8 +1,6 @@
 """Utils for filenameprocessor lambda"""
 
 import json
-from csv import DictReader
-from io import StringIO
 from constants import Constants, SOURCE_BUCKET_NAME, FILE_NAME_PROC_LAMBDA_NAME
 from clients import s3_client, logger, lambda_client
 
@@ -13,13 +11,6 @@ def get_created_at_formatted_string(bucket_name: str, file_key: str) -> str:
     return response["LastModified"].strftime("%Y%m%dT%H%M%S00")
 
 
-def get_csv_content_dict_reader(bucket_name: str, file_key: str) -> DictReader:
-    """Downloads the csv data and returns a csv_reader with the content of the csv"""
-    csv_obj = s3_client.get_object(Bucket=bucket_name, Key=file_key)
-    csv_content_string = csv_obj["Body"].read().decode("utf-8")
-    return DictReader(StringIO(csv_content_string), delimiter="|")
-
-
 def identify_supplier(ods_code: str) -> str:
     """
     Identifies the supplier from the ods code using the mapping.

filenameprocessor/src/utils_for_filenameprocessor.py

@@ -2,17 +2,20 @@
 
 from unittest import TestCase
 from unittest.mock import patch
-from boto3 import resource as boto3_resource
+from boto3 import client as boto3_client
 from moto import mock_dynamodb
-from errors import UnhandledAuditTableError
-from clients import REGION_NAME
-from tests.utils_for_tests.values_for_tests import MOCK_ENVIRONMENT_DICT
 
-# Some environment variables are evaluated when  constants and upsert_audit_table are imported,
-# so environment dictionary must be mocked first
+from tests.utils_for_tests.mock_environment_variables import MOCK_ENVIRONMENT_DICT
+from tests.utils_for_tests.generic_setup_and_teardown import GenericSetUp, GenericTearDown
+
+# Ensure environment variables are mocked before importing from src files
 with patch.dict("os.environ", MOCK_ENVIRONMENT_DICT):
-    from constants import AuditTableKeys, AUDIT_TABLE_NAME, AUDIT_TABLE_QUEUE_NAME_GSI, AUDIT_TABLE_FILENAME_GSI
+    from constants import AUDIT_TABLE_NAME
     from audit_table import upsert_audit_table
+    from errors import UnhandledAuditTableError
+    from clients import REGION_NAME
+
+dynamodb_client = boto3_client("dynamodb", region_name=REGION_NAME)
 
 
 @mock_dynamodb
@@ -22,43 +25,16 @@ class TestAuditTable(TestCase):
 
     def setUp(self):
         """Set up test values to be used for the tests"""
-        self.dynamodb_resource = boto3_resource("dynamodb", region_name=REGION_NAME)
-        self.table = self.dynamodb_resource.create_table(
-            TableName=AUDIT_TABLE_NAME,
-            KeySchema=[
-                {"AttributeName": AuditTableKeys.MESSAGE_ID, "KeyType": "HASH"},
-                # {"AttributeName": "placeholder", "KeyType": "RANGE"},
-            ],
-            AttributeDefinitions=[
-                {"AttributeName": AuditTableKeys.MESSAGE_ID, "AttributeType": "S"},
-                {"AttributeName": AuditTableKeys.FILENAME, "AttributeType": "S"},
-                {"AttributeName": AuditTableKeys.QUEUE_NAME, "AttributeType": "S"},
-                {"AttributeName": AuditTableKeys.STATUS, "AttributeType": "S"},
-            ],
-            ProvisionedThroughput={"ReadCapacityUnits": 5, "WriteCapacityUnits": 5},
-            GlobalSecondaryIndexes=[
-                {
-                    "IndexName": AUDIT_TABLE_FILENAME_GSI,
-                    "KeySchema": [{"AttributeName": AuditTableKeys.FILENAME, "KeyType": "HASH"}],
-                    "Projection": {"ProjectionType": "KEYS_ONLY"},
-                    "ProvisionedThroughput": {"ReadCapacityUnits": 5, "WriteCapacityUnits": 5},
-                },
-                {
-                    "IndexName": AUDIT_TABLE_QUEUE_NAME_GSI,
-                    "KeySchema": [
-                        {"AttributeName": AuditTableKeys.QUEUE_NAME, "KeyType": "HASH"},
-                        {"AttributeName": AuditTableKeys.STATUS, "KeyType": "RANGE"},
-                    ],
-                    "Projection": {"ProjectionType": "ALL"},
-                    "ProvisionedThroughput": {"ReadCapacityUnits": 5, "WriteCapacityUnits": 5},
-                },
-            ],
-        )
+        GenericSetUp(dynamodb_client=dynamodb_client)
+
+    def tearDown(self):
+        """Tear down the test values"""
+        GenericTearDown(dynamodb_client=dynamodb_client)
 
-        assert self.table.table_status == "ACTIVE"
-        assert len(self.table.global_secondary_indexes) == 2
-        assert self.table.global_secondary_indexes[0]["IndexName"] == AUDIT_TABLE_FILENAME_GSI
-        assert self.table.global_secondary_indexes[1]["IndexName"] == AUDIT_TABLE_QUEUE_NAME_GSI
+    @staticmethod
+    def get_table_items():
+        """Return all items in the audit table"""
+        return dynamodb_client.scan(TableName=AUDIT_TABLE_NAME).get("Items", [])
 
     def test_upsert_audit_table(self):
         """Test that the upsert_audit_table function works as expected for the following:
@@ -79,42 +55,42 @@ def test_upsert_audit_table(self):
 
         # Test case 1: file_key_1 should be added to the audit table with status set to "Processing"
         expected_table_item_1 = {
-            "message_id": message_id_1,
-            "filename": file_key_1,
-            "queue_name": "test_queue",
-            "status": "Processing",
-            "timestamp": created_at_formatted_string_1,
+            "message_id": {"S": message_id_1},
+            "filename": {"S": file_key_1},
+            "queue_name": {"S": "test_queue"},
+            "status": {"S": "Processing"},
+            "timestamp": {"S": created_at_formatted_string_1},
         }
 
         # Test case 2: file_key_2 should be added to the audit table with status set to "Queued"
         expected_table_item_2 = {
-            "message_id": message_id_2,
-            "filename": file_key_2,
-            "queue_name": "test_queue",
-            "status": "Queued",
-            "timestamp": created_at_formatted_string_2,
+            "message_id": {"S": message_id_2},
+            "filename": {"S": file_key_2},
+            "queue_name": {"S": "test_queue"},
+            "status": {"S": "Queued"},
+            "timestamp": {"S": created_at_formatted_string_2},
         }
 
         # Test case 3: file_key_1 should be added to the audit table again,
         # with status set to "Not processed - duplicate"
         expected_table_item_3 = {
-            "message_id": message_id_3,
-            "filename": file_key_1,
-            "queue_name": "test_queue",
-            "status": "Not processed - duplicate",
-            "timestamp": created_at_formatted_string_3,
+            "message_id": {"S": message_id_3},
+            "filename": {"S": file_key_1},
+            "queue_name": {"S": "test_queue"},
+            "status": {"S": "Not processed - duplicate"},
+            "timestamp": {"S": created_at_formatted_string_3},
         }
 
         # Test case 4: file_key_4 should not be added to the audit table because the message_id is a duplicate.
         # Note that this scenario should never occur as a new unique message_id is generated for each file upload,
         # even if the file name is a duplicate, but this test is included for safety because default behaviour of
         # dynamodb is to overwrite the existing item if the message_id is the same.
-        expected_table_item_4 = {  # This item should not be added to the table
-            "message_id": message_id_3,
-            "filename": file_key_4,
-            "queue_name": "test_queue",
-            "status": "Processed",
-            "timestamp": created_at_formatted_string_4,
+        expected_table_item_4 = {
+            "message_id": {"S": message_id_3},
+            "filename": {"S": file_key_4},
+            "queue_name": {"S": "test_queue"},
+            "status": {"S": "Processed"},
+            "timestamp": {"S": created_at_formatted_string_4},
         }
 
         # Add a file to the audit table
@@ -128,7 +104,7 @@ def test_upsert_audit_table(self):
                 is_existing_file=False,
             )
         )
-        table_items = self.table.scan()["Items"]
+        table_items = self.get_table_items()
         assert len(table_items) == 1
         assert expected_table_item_1 in table_items
 
@@ -143,7 +119,7 @@ def test_upsert_audit_table(self):
                 is_existing_file=False,
             )
         )
-        table_items = self.table.scan()["Items"]
+        table_items = self.get_table_items()
         assert len(table_items) == 2
         assert expected_table_item_1 in table_items
         assert expected_table_item_2 in table_items
@@ -162,7 +138,7 @@ def test_upsert_audit_table(self):
 
         # Check that the file has been added to the audit table again,
         # with the different message_id and created_at_formatted_string
-        table_items = self.table.scan()["Items"]
+        table_items = self.get_table_items()
         assert len(table_items) == 3
         assert expected_table_item_1 in table_items
         assert expected_table_item_2 in table_items
@@ -180,7 +156,7 @@ def test_upsert_audit_table(self):
             )
 
         # Check that the file has not been added to the audit table
-        table_items = self.table.scan()["Items"]
+        table_items = self.get_table_items()
         assert len(table_items) == 3
         assert expected_table_item_1 in table_items
         assert expected_table_item_2 in table_items