From 6d7c74d39c2a0704917875aa36289f3cecbd10d7 Mon Sep 17 00:00:00 2001 From: AlexandraBenson Date: Wed, 30 Oct 2024 17:05:01 +0000 Subject: [PATCH 01/22] Remove constants class --- recordforwarder/src/constants.py | 38 +++++++++---------- .../src/get_imms_id_and_version.py | 4 +- recordforwarder/src/send_request_to_lambda.py | 8 ++-- recordforwarder/src/update_ack_file.py | 4 +- .../src/utils_for_record_forwarder.py | 2 +- 5 files changed, 26 insertions(+), 30 deletions(-) diff --git a/recordforwarder/src/constants.py b/recordforwarder/src/constants.py index a8264bbf..852bed9b 100644 --- a/recordforwarder/src/constants.py +++ b/recordforwarder/src/constants.py @@ -1,24 +1,20 @@ """Constants for recordforwarder""" +ACK_HEADERS = [ + "MESSAGE_HEADER_ID", + "HEADER_RESPONSE_CODE", + "ISSUE_SEVERITY", + "ISSUE_CODE", + "ISSUE_DETAILS_CODE", + "RESPONSE_TYPE", + "RESPONSE_CODE", + "RESPONSE_DISPLAY", + "RECEIVED_TIME", + "MAILBOX_FROM", + "LOCAL_ID", + "IMMS_ID", + "OPERATION_OUTCOME", + "MESSAGE_DELIVERY", +] -class Constants: - """Constants for recordforwarder""" - - ack_headers = [ - "MESSAGE_HEADER_ID", - "HEADER_RESPONSE_CODE", - "ISSUE_SEVERITY", - "ISSUE_CODE", - "ISSUE_DETAILS_CODE", - "RESPONSE_TYPE", - "RESPONSE_CODE", - "RESPONSE_DISPLAY", - "RECEIVED_TIME", - "MAILBOX_FROM", - "LOCAL_ID", - "IMMS_ID", - "OPERATION_OUTCOME", - "MESSAGE_DELIVERY", - ] - - IMMS_BATCH_APP_NAME = "Imms-Batch-App" +IMMS_BATCH_APP_NAME = "Imms-Batch-App" diff --git a/recordforwarder/src/get_imms_id_and_version.py b/recordforwarder/src/get_imms_id_and_version.py index eeb921f0..d488ab8b 100644 --- a/recordforwarder/src/get_imms_id_and_version.py +++ b/recordforwarder/src/get_imms_id_and_version.py @@ -5,7 +5,7 @@ from errors import IdNotFoundError from clients import lambda_client from utils_for_record_forwarder import invoke_lambda -from constants import Constants +from constants import IMMS_BATCH_APP_NAME logger = logging.getLogger() @@ -13,7 +13,7 @@ def get_imms_id_and_version(fhir_json: dict) -> tuple[str, int]: """Send a GET request to Imms API requesting the id and version""" # Create payload - headers = {"SupplierSystem": Constants.IMMS_BATCH_APP_NAME} + headers = {"SupplierSystem": IMMS_BATCH_APP_NAME} identifier = fhir_json.get("identifier", [{}])[0] immunization_identifier = f"{identifier.get('system')}|{identifier.get('value')}" query_string_parameters = {"_element": "id,meta", "immunization.identifier": immunization_identifier} diff --git a/recordforwarder/src/send_request_to_lambda.py b/recordforwarder/src/send_request_to_lambda.py index a5f71be6..c9a73898 100644 --- a/recordforwarder/src/send_request_to_lambda.py +++ b/recordforwarder/src/send_request_to_lambda.py @@ -5,13 +5,13 @@ from get_imms_id_and_version import get_imms_id_and_version from clients import lambda_client from utils_for_record_forwarder import invoke_lambda -from constants import Constants +from constants import IMMS_BATCH_APP_NAME def send_create_request(fhir_json: dict, supplier: str) -> str: """Sends the create request and handles the response. Returns the imms_id.""" # Send create request - headers = {"SupplierSystem": Constants.IMMS_BATCH_APP_NAME, "BatchSupplierSystem": supplier} + headers = {"SupplierSystem": IMMS_BATCH_APP_NAME, "BatchSupplierSystem": supplier} payload = {"headers": headers, "body": fhir_json} status_code, body, headers = invoke_lambda(lambda_client, os.getenv("CREATE_LAMBDA_NAME"), payload) if status_code != 201: @@ -35,7 +35,7 @@ def send_update_request(fhir_json: dict, supplier: str) -> str: # Send update request fhir_json["id"] = imms_id - headers = {"SupplierSystem": Constants.IMMS_BATCH_APP_NAME, "BatchSupplierSystem": supplier, "E-Tag": version} + headers = {"SupplierSystem": IMMS_BATCH_APP_NAME, "BatchSupplierSystem": supplier, "E-Tag": version} payload = {"headers": headers, "body": fhir_json, "pathParameters": {"id": imms_id}} status_code, body, _ = invoke_lambda(lambda_client, os.getenv("UPDATE_LAMBDA_NAME"), payload) if status_code != 200: @@ -55,7 +55,7 @@ def send_delete_request(fhir_json: dict, supplier: str) -> str: raise MessageNotSuccessfulError("Unable to obtain Imms ID") # Send delete request - headers = {"SupplierSystem": Constants.IMMS_BATCH_APP_NAME, "BatchSupplierSystem": supplier} + headers = {"SupplierSystem": IMMS_BATCH_APP_NAME, "BatchSupplierSystem": supplier} payload = {"headers": headers, "body": fhir_json, "pathParameters": {"id": imms_id}} status_code, body, _ = invoke_lambda(lambda_client, os.getenv("DELETE_LAMBDA_NAME"), payload) if status_code != 204: diff --git a/recordforwarder/src/update_ack_file.py b/recordforwarder/src/update_ack_file.py index 0ed88f5c..4a5a7bff 100644 --- a/recordforwarder/src/update_ack_file.py +++ b/recordforwarder/src/update_ack_file.py @@ -6,7 +6,7 @@ from typing import Union from botocore.exceptions import ClientError from clients import s3_client -from constants import Constants +from constants import ACK_HEADERS from utils_for_record_forwarder import get_environment logger = logging.getLogger() @@ -58,7 +58,7 @@ def obtain_current_ack_content(ack_bucket_name: str, ack_file_key: str) -> Strin logger.error("error:%s", error) if error.response["Error"]["Code"] in ("404", "NoSuchKey"): # If ack file does not exist in S3 create a new file - accumulated_csv_content.write("|".join(Constants.ack_headers) + "\n") + accumulated_csv_content.write("|".join(ACK_HEADERS) + "\n") else: raise return accumulated_csv_content diff --git a/recordforwarder/src/utils_for_record_forwarder.py b/recordforwarder/src/utils_for_record_forwarder.py index 0007f5b6..97d1f579 100644 --- a/recordforwarder/src/utils_for_record_forwarder.py +++ b/recordforwarder/src/utils_for_record_forwarder.py @@ -20,6 +20,6 @@ def invoke_lambda(lambda_client, lambda_name: str, payload: dict) -> tuple[int, response = lambda_client.invoke( FunctionName=lambda_name, InvocationType="RequestResponse", Payload=json.dumps(payload) ) - response_payload = json.loads(response["Payload"].read()) + response_payload = json.load(response["Payload"]) body = json.loads(response_payload.get("body", "{}")) return response_payload.get("statusCode"), body, response_payload.get("headers") From fa5613eeef6a8aeee39da7841df535e3d3aa2d26 Mon Sep 17 00:00:00 2001 From: AlexandraBenson Date: Fri, 1 Nov 2024 16:59:49 +0000 Subject: [PATCH 02/22] Move lambda_client out of function args and update test mocking accordingly --- .../src/get_imms_id_and_version.py | 3 +- recordforwarder/src/send_request_to_lambda.py | 7 +- .../src/utils_for_record_forwarder.py | 4 +- .../tests/test_e2e_forwarding_lambda.py | 282 +++++++----------- .../tests/test_forwarding_lambda.py | 191 +++++++----- .../utils_for_recordforwarder_tests.py | 8 +- .../values_for_recordforwarder_tests.py | 10 - 7 files changed, 239 insertions(+), 266 deletions(-) diff --git a/recordforwarder/src/get_imms_id_and_version.py b/recordforwarder/src/get_imms_id_and_version.py index d488ab8b..f3abf2b9 100644 --- a/recordforwarder/src/get_imms_id_and_version.py +++ b/recordforwarder/src/get_imms_id_and_version.py @@ -3,7 +3,6 @@ import os import logging from errors import IdNotFoundError -from clients import lambda_client from utils_for_record_forwarder import invoke_lambda from constants import IMMS_BATCH_APP_NAME @@ -20,7 +19,7 @@ def get_imms_id_and_version(fhir_json: dict) -> tuple[str, int]: payload = {"headers": headers, "body": None, "queryStringParameters": query_string_parameters} # Invoke lambda - status_code, body, _ = invoke_lambda(lambda_client, os.getenv("SEARCH_LAMBDA_NAME"), payload) + status_code, body, _ = invoke_lambda(os.getenv("SEARCH_LAMBDA_NAME"), payload) # Handle non-200 or empty response if not (body.get("total") == 1 and status_code == 200): diff --git a/recordforwarder/src/send_request_to_lambda.py b/recordforwarder/src/send_request_to_lambda.py index c9a73898..6cdbd391 100644 --- a/recordforwarder/src/send_request_to_lambda.py +++ b/recordforwarder/src/send_request_to_lambda.py @@ -3,7 +3,6 @@ import os from errors import MessageNotSuccessfulError, IdNotFoundError from get_imms_id_and_version import get_imms_id_and_version -from clients import lambda_client from utils_for_record_forwarder import invoke_lambda from constants import IMMS_BATCH_APP_NAME @@ -13,7 +12,7 @@ def send_create_request(fhir_json: dict, supplier: str) -> str: # Send create request headers = {"SupplierSystem": IMMS_BATCH_APP_NAME, "BatchSupplierSystem": supplier} payload = {"headers": headers, "body": fhir_json} - status_code, body, headers = invoke_lambda(lambda_client, os.getenv("CREATE_LAMBDA_NAME"), payload) + status_code, body, headers = invoke_lambda(os.getenv("CREATE_LAMBDA_NAME"), payload) if status_code != 201: raise MessageNotSuccessfulError(get_operation_outcome_diagnostics(body)) @@ -37,7 +36,7 @@ def send_update_request(fhir_json: dict, supplier: str) -> str: fhir_json["id"] = imms_id headers = {"SupplierSystem": IMMS_BATCH_APP_NAME, "BatchSupplierSystem": supplier, "E-Tag": version} payload = {"headers": headers, "body": fhir_json, "pathParameters": {"id": imms_id}} - status_code, body, _ = invoke_lambda(lambda_client, os.getenv("UPDATE_LAMBDA_NAME"), payload) + status_code, body, _ = invoke_lambda(os.getenv("UPDATE_LAMBDA_NAME"), payload) if status_code != 200: raise MessageNotSuccessfulError(get_operation_outcome_diagnostics(body)) @@ -57,7 +56,7 @@ def send_delete_request(fhir_json: dict, supplier: str) -> str: # Send delete request headers = {"SupplierSystem": IMMS_BATCH_APP_NAME, "BatchSupplierSystem": supplier} payload = {"headers": headers, "body": fhir_json, "pathParameters": {"id": imms_id}} - status_code, body, _ = invoke_lambda(lambda_client, os.getenv("DELETE_LAMBDA_NAME"), payload) + status_code, body, _ = invoke_lambda(os.getenv("DELETE_LAMBDA_NAME"), payload) if status_code != 204: raise MessageNotSuccessfulError(get_operation_outcome_diagnostics(body)) diff --git a/recordforwarder/src/utils_for_record_forwarder.py b/recordforwarder/src/utils_for_record_forwarder.py index 97d1f579..a2033f31 100644 --- a/recordforwarder/src/utils_for_record_forwarder.py +++ b/recordforwarder/src/utils_for_record_forwarder.py @@ -3,6 +3,8 @@ import os import json +from clients import lambda_client + def get_environment() -> str: """Returns the current environment. Defaults to internal-dev for pr and user environments""" @@ -11,7 +13,7 @@ def get_environment() -> str: return _env if _env in ["internal-dev", "int", "ref", "sandbox", "prod"] else "internal-dev" -def invoke_lambda(lambda_client, lambda_name: str, payload: dict) -> tuple[int, dict, str]: +def invoke_lambda(lambda_name: str, payload: dict) -> tuple[int, dict, str]: """ Uses the lambda_client to invoke the specified lambda with the given payload. Returns the ressponse status code, body (loaded in as a dictionary) and headers. diff --git a/recordforwarder/tests/test_e2e_forwarding_lambda.py b/recordforwarder/tests/test_e2e_forwarding_lambda.py index 8de25d89..d4f3af71 100644 --- a/recordforwarder/tests/test_e2e_forwarding_lambda.py +++ b/recordforwarder/tests/test_e2e_forwarding_lambda.py @@ -1,8 +1,8 @@ import unittest from unittest.mock import patch from boto3 import client as boto3_client -from uuid import uuid4 import json +from io import StringIO from moto import mock_s3 from forwarding_lambda import forward_lambda_handler from tests.utils_for_recordfowarder_tests.values_for_recordforwarder_tests import ( @@ -15,14 +15,40 @@ TEST_SUPPLIER, TEST_ROW_ID, ) -from tests.utils_for_recordfowarder_tests.utils_for_recordforwarder_tests import create_mock_search_lambda_response +from tests.utils_for_recordfowarder_tests.utils_for_recordforwarder_tests import ( + response_body_id_and_version_found, + response_body_id_and_version_not_found, + create_mock_operation_outcome, +) import base64 + +def generate_payload(status_code: int, headers: dict = {}, body: dict = None): + return {"statusCode": status_code, **({"body": json.dumps(body)} if body is not None else {}), "headers": headers} + + +base_message_fields = {"row_id": TEST_ROW_ID, "file_key": TEST_FILE_KEY, "supplier": TEST_SUPPLIER} + s3_client = boto3_client("s3", region_name=AWS_REGION) kinesis_client = boto3_client("kinesis", region_name=AWS_REGION) +lambda_success_headers = {"Location": "https://example.com/immunization/test_id"} + +MOCK_ENVIRONMENT_DICT = { + "SOURCE_BUCKET_NAME": "immunisation-batch-internal-dev-data-sources", + "ACK_BUCKET_NAME": "immunisation-batch-internal-dev-data-destinations", + "ENVIRONMENT": "internal-dev", + "LOCAL_ACCOUNT_ID": "123456789012", + "SHORT_QUEUE_PREFIX": "imms-batch-internal-dev", + "CREATE_LAMBDA_NAME": "mock_create_lambda_name", + "UPDATE_LAMBDA_NAME": "mock_update_lambda_name", + "DELETE_LAMBDA_NAME": "mock_delete_lambda_name", + "SEARCH_LAMBDA_NAME": "mock_search_lambda_name", +} + @mock_s3 +@patch.dict("os.environ", MOCK_ENVIRONMENT_DICT) class TestForwardingLambdaE2E(unittest.TestCase): def setup_s3(self): @@ -44,216 +70,116 @@ def check_ack_file(self, s3_client, expected_content): ack_file_content = ack_file_obj["Body"].read().decode("utf-8") self.assertIn(expected_content, ack_file_content) - def execute_test( - self, - mock_api, - message, - response_code, - expected_content, - mock_diagnostics=None, - mock_get_imms_id_and_version=None, - id_and_version_found=True, - ): + def execute_test(self, message, expected_content, mock_lambda_payloads: dict): self.setup_s3() - mock_response = create_mock_search_lambda_response(response_code, mock_diagnostics, id_and_version_found) - mock_api.invoke.return_value = mock_response - kinesis_message = self.create_kinesis_message(message) - if mock_get_imms_id_and_version: - with patch("send_request_to_lambda.get_imms_id_and_version", return_value=mock_get_imms_id_and_version): - forward_lambda_handler(kinesis_message, None) - else: - forward_lambda_handler(kinesis_message, None) + # Mock the responses from the calls to the Imms FHIR API lambdas + # Note that a different response is mocked for each different lambda call + def lambda_invocation_side_effect(FunctionName, *_args, **_kwargs): # pylint: disable=invalid-name + response_payload = None + # Mock the response for the relevant lambda for the operation + operation = message["operation_requested"] + if FunctionName == f"mock_{operation.lower()}_lambda_name": + response_payload = mock_lambda_payloads[operation] + # Mock the search lambda (for the get_imms_id_and_version lambda call) + elif FunctionName == "mock_search_lambda_name": + response_payload = mock_lambda_payloads["SEARCH"] + return {"Payload": StringIO(json.dumps(response_payload))} + + with patch("utils_for_record_forwarder.lambda_client.invoke", side_effect=lambda_invocation_side_effect): + forward_lambda_handler(event=self.create_kinesis_message(message), _=None) self.check_ack_file(s3_client, expected_content) - @patch("get_imms_id_and_version.lambda_client") - def test_forward_lambda_e2e_update_failed_unable_to_get_id(self, mock_api): - # Set the mock response as the return value of invoke - message = { - "row_id": TEST_ROW_ID, - "fhir_json": test_fhir_json, - "operation_requested": "UPDATE", - "file_key": TEST_FILE_KEY, - "supplier": TEST_SUPPLIER, - } - self.execute_test(mock_api, message, 200, "Fatal", id_and_version_found=False) - - @patch("send_request_to_lambda.lambda_client") - def test_forward_lambda_e2e_create_success(self, mock_api): - # Set the mock response as the return value of invoke - message = { - "row_id": TEST_ROW_ID, - "fhir_json": test_fhir_json, - "operation_requested": "CREATE", - "file_key": TEST_FILE_KEY, - "supplier": TEST_SUPPLIER, - } - self.execute_test(mock_api, message, 201, "OK") - - @patch("send_request_to_lambda.lambda_client") - def test_forward_lambda_e2e_create_duplicate(self, mock_api): - message = { - "row_id": TEST_ROW_ID, - "fhir_json": test_fhir_json, - "operation_requested": "CREATE", - "file_key": TEST_FILE_KEY, - "supplier": TEST_SUPPLIER, - "imms_id": "test", - "version": 1, + def test_forward_lambda_e2e_update_failed_unable_to_get_id(self): + message = {**base_message_fields, "fhir_json": test_fhir_json, "operation_requested": "UPDATE"} + mock_lambda_payloads = { + "SEARCH": generate_payload(status_code=200, body=response_body_id_and_version_not_found), } + self.execute_test(message, "Fatal", mock_lambda_payloads) + + def test_forward_lambda_e2e_create_success(self): + message = {**base_message_fields, "fhir_json": test_fhir_json, "operation_requested": "CREATE"} + mock_lambda_payloads = {"CREATE": generate_payload(status_code=201, headers=lambda_success_headers)} + self.execute_test(message, "OK", mock_lambda_payloads=mock_lambda_payloads) + + def test_forward_lambda_e2e_create_duplicate(self): + message = {**base_message_fields, "fhir_json": test_fhir_json, "operation_requested": "CREATE"} mock_diagnostics = ( "The provided identifier: https://supplierABC/identifiers/vacc#test-identifier1 is duplicated" ) - self.execute_test(mock_api, message, 422, "Fatal Error", mock_diagnostics=mock_diagnostics) - - @patch("send_request_to_lambda.lambda_client") - def test_forward_lambda_e2e_create_failed(self, mock_api): - message = { - "row_id": TEST_ROW_ID, - "fhir_json": test_fhir_json, - "operation_requested": "CREATE", - "file_key": TEST_FILE_KEY, - "supplier": TEST_SUPPLIER, - "imms_id": "test", - "version": 1, - } + mock_body = create_mock_operation_outcome(diagnostics=mock_diagnostics, code="duplicate") + mock_lambda_payloads = {"CREATE": generate_payload(status_code=422, body=mock_body)} + self.execute_test(message, "Fatal Error", mock_lambda_payloads=mock_lambda_payloads) + + def test_forward_lambda_e2e_create_failed(self): + message = {**base_message_fields, "fhir_json": test_fhir_json, "operation_requested": "CREATE"} mock_diagnostics = "the provided event ID is either missing or not in the expected format." - self.execute_test(mock_api, message, 400, "Fatal Error", mock_diagnostics=mock_diagnostics) - - @patch("send_request_to_lambda.lambda_client") - def test_forward_lambda_e2e_create_multi_line_diagnostics(self, mock_api): - message = { - "row_id": TEST_ROW_ID, - "fhir_json": test_fhir_json, - "operation_requested": "CREATE", - "file_key": TEST_FILE_KEY, - "supplier": TEST_SUPPLIER, - "imms_id": "test", - "version": 1, - } + mock_body = create_mock_operation_outcome(diagnostics=mock_diagnostics, code="duplicate") + mock_lambda_payloads = {"CREATE": generate_payload(status_code=400, body=mock_body)} + self.execute_test(message, "Fatal Error", mock_lambda_payloads=mock_lambda_payloads) + + def test_forward_lambda_e2e_create_multi_line_diagnostics(self): + message = {**base_message_fields, "fhir_json": test_fhir_json, "operation_requested": "CREATE"} mock_diagnostics = """This a string of diagnostics which spans multiple lines and has some carriage returns\n\nand random space""" - + mock_body = create_mock_operation_outcome(diagnostics=mock_diagnostics) + mock_lambda_payloads = {"CREATE": generate_payload(status_code=404, body=mock_body)} expected_single_line_diagnostics = ( "This a string of diagnostics which spans multiple lines and has some carriage returns and random space" ) + self.execute_test(message, expected_single_line_diagnostics, mock_lambda_payloads) - self.setup_s3() - mock_response = create_mock_search_lambda_response(400, mock_diagnostics) - mock_api.invoke.return_value = mock_response - mock_api.create_immunization.return_value = mock_response - - kinesis_message = self.create_kinesis_message(message) - forward_lambda_handler(kinesis_message, None) - - ack_file_obj = s3_client.get_object(Bucket=DESTINATION_BUCKET_NAME, Key=TEST_ACK_FILE_KEY) - ack_file_content = ack_file_obj["Body"].read().decode("utf-8") - self.assertIn(expected_single_line_diagnostics, ack_file_content) - - @patch("send_request_to_lambda.lambda_client") + @patch("utils_for_record_forwarder.lambda_client.invoke") def test_forward_lambda_e2e_none_request(self, mock_api): self.setup_s3() + message = {**base_message_fields, "diagnostics": "Unsupported file type received as an attachment"} - message = { - "row_id": TEST_ROW_ID, - "file_key": TEST_FILE_KEY, - "supplier": TEST_SUPPLIER, - "diagnostics": "Unsupported file type received as an attachment", - } - - kinesis_message = self.create_kinesis_message(message) - forward_lambda_handler(kinesis_message, None) + forward_lambda_handler(self.create_kinesis_message(message), None) self.check_ack_file(s3_client, "Fatal Error") mock_api.create_immunization.assert_not_called() - @patch("send_request_to_lambda.lambda_client") - def test_forward_lambda_e2e_update_success(self, mock_api): - message = { - "row_id": TEST_ROW_ID, - "fhir_json": test_fhir_json, - "operation_requested": "UPDATE", - "file_key": TEST_FILE_KEY, - "supplier": TEST_SUPPLIER, + def test_forward_lambda_e2e_update_success(self): + message = {**base_message_fields, "fhir_json": test_fhir_json, "operation_requested": "UPDATE"} + mock_lambda_payloads = { + "UPDATE": generate_payload(200), + "SEARCH": generate_payload(200, body=response_body_id_and_version_found), } - self.execute_test(mock_api, message, 200, "OK", mock_get_imms_id_and_version=(str(uuid4()), 1)) - - @patch("send_request_to_lambda.lambda_client") - def test_forward_lambda_e2e_update_failed(self, mock_api): - message = { - "row_id": TEST_ROW_ID, - "fhir_json": test_fhir_json, - "operation_requested": "UPDATE", - "file_key": TEST_FILE_KEY, - "supplier": TEST_SUPPLIER, - } - mock_diagnstics = "the provided event ID is either missing or not in the expected format." - self.execute_test( - mock_api, - message, - 400, - "Fatal Error", - mock_diagnostics=mock_diagnstics, - mock_get_imms_id_and_version=("test", 1), - ) + self.execute_test(message, "OK", mock_lambda_payloads) - @patch("send_request_to_lambda.lambda_client") - def test_forward_lambda_e2e_delete_success(self, mock_api): - self.setup_s3() - mock_response = create_mock_search_lambda_response(204) - mock_api.invoke.return_value = mock_response - - message = { - "row_id": TEST_ROW_ID, - "fhir_json": test_fhir_json, - "operation_requested": "DELETE", - "file_key": TEST_FILE_KEY, - "supplier": TEST_SUPPLIER, - "imms_id": "test", - "version": 1, + def test_forward_lambda_e2e_update_failed(self): + message = {**base_message_fields, "fhir_json": test_fhir_json, "operation_requested": "UPDATE"} + mock_diagnstics = "the provided event ID is either missing or not in the expected format." + mock_lambda_payloads = { + "UPDATE": generate_payload(400, body=create_mock_operation_outcome(mock_diagnstics)), + "SEARCH": generate_payload(200, body=response_body_id_and_version_found), } + self.execute_test(message, "Fatal Error", mock_lambda_payloads) - kinesis_message = self.create_kinesis_message(message) - with patch("send_request_to_lambda.get_imms_id_and_version", return_value=("test", 1)): - forward_lambda_handler(kinesis_message, None) - - self.check_ack_file(s3_client, "OK") - - @patch("send_request_to_lambda.lambda_client") - def test_forward_lambda_e2e_delete_failed(self, mock_api): - self.setup_s3() - mock_response = create_mock_search_lambda_response(404, "not-found") - mock_api.invoke.return_value = mock_response - message = { - "row_id": TEST_ROW_ID, - "fhir_json": test_fhir_json, - "operation_requested": "DELETE", - "file_key": TEST_FILE_KEY, - "supplier": TEST_SUPPLIER, - "imms_id": "test", - "version": 1, + def test_forward_lambda_e2e_delete_success(self): + message = {**base_message_fields, "fhir_json": test_fhir_json, "operation_requested": "DELETE"} + mock_lambda_payloads = { + "DELETE": generate_payload(204), + "SEARCH": generate_payload(200, body=response_body_id_and_version_found), } + self.execute_test(message, "OK", mock_lambda_payloads) - kinesis_message = self.create_kinesis_message(message) - with patch("send_request_to_lambda.get_imms_id_and_version", return_value=("test", 1)): - forward_lambda_handler(kinesis_message, None) - - self.check_ack_file(s3_client, "Fatal Error") + def test_forward_lambda_e2e_delete_failed(self): + message = {**base_message_fields, "fhir_json": test_fhir_json, "operation_requested": "DELETE"} + mock_diagnstics = "the provided event ID is either missing or not in the expected format." + mock_lambda_payloads = { + "UPDATE": generate_payload(404, body=create_mock_operation_outcome(mock_diagnstics, code="not-found")), + "SEARCH": generate_payload(200, body=response_body_id_and_version_not_found), + } + self.execute_test(message, "Fatal Error", mock_lambda_payloads) def test_forward_lambda_e2e_no_permissions(self): self.setup_s3() + message = {**base_message_fields, "diagnostics": "No permissions for operation"} - message = { - "row_id": TEST_ROW_ID, - "file_key": TEST_FILE_KEY, - "supplier": TEST_SUPPLIER, - "diagnostics": "No permissions for operation", - } - - kinesis_message = self.create_kinesis_message(message) - forward_lambda_handler(kinesis_message, None) + forward_lambda_handler(self.create_kinesis_message(message), None) self.check_ack_file(s3_client, "Fatal Error") diff --git a/recordforwarder/tests/test_forwarding_lambda.py b/recordforwarder/tests/test_forwarding_lambda.py index 873f9961..0b9613cd 100644 --- a/recordforwarder/tests/test_forwarding_lambda.py +++ b/recordforwarder/tests/test_forwarding_lambda.py @@ -1,8 +1,9 @@ import unittest -from unittest.mock import patch, MagicMock +from unittest.mock import patch from moto import mock_s3 from boto3 import client as boto3_client import json +from io import StringIO from botocore.exceptions import ClientError from datetime import datetime import base64 @@ -10,13 +11,54 @@ from forwarding_lambda import forward_lambda_handler, forward_request_to_lambda from utils_for_record_forwarder import get_environment from update_ack_file import create_ack_data -from tests.utils_for_recordfowarder_tests.utils_for_recordforwarder_tests import create_mock_search_lambda_response +from tests.utils_for_recordfowarder_tests.utils_for_recordforwarder_tests import ( + create_mock_operation_outcome, + response_body_id_and_version_found, +) s3_client = boto3_client("s3", region_name=AWS_REGION) +def generate_lambda_invocation_side_effect(message, mock_lambda_payloads): + # Mock the responses from the calls to the Imms FHIR API lambdas + # Note that a different response is mocked for each different lambda call + def lambda_invocation_side_effect(FunctionName, *_args, **_kwargs): # pylint: disable=invalid-name + response_payload = None + # Mock the response for the relevant lambda for the operation + operation = message["operation_requested"] + if FunctionName == f"mock_{operation.lower()}_lambda_name": + response_payload = mock_lambda_payloads[operation] + # Mock the search lambda (for the get_imms_id_and_version lambda call) + elif FunctionName == "mock_search_lambda_name": + response_payload = mock_lambda_payloads["SEARCH"] + return {"Payload": StringIO(json.dumps(response_payload))} + + return lambda_invocation_side_effect + + +def generate_payload(status_code: int, headers: dict = {}, body: dict = None): + return {"statusCode": status_code, **({"body": json.dumps(body)} if body is not None else {}), "headers": headers} + + +lambda_success_headers = {"Location": "https://example.com/immunization/test_id"} + + +MOCK_ENVIRONMENT_DICT = { + "SOURCE_BUCKET_NAME": "immunisation-batch-internal-dev-data-sources", + "ACK_BUCKET_NAME": "immunisation-batch-internal-dev-data-destinations", + "ENVIRONMENT": "internal-dev", + "LOCAL_ACCOUNT_ID": "123456789012", + "SHORT_QUEUE_PREFIX": "imms-batch-internal-dev", + "CREATE_LAMBDA_NAME": "mock_create_lambda_name", + "UPDATE_LAMBDA_NAME": "mock_update_lambda_name", + "DELETE_LAMBDA_NAME": "mock_delete_lambda_name", + "SEARCH_LAMBDA_NAME": "mock_search_lambda_name", +} + + @mock_s3 +@patch.dict("os.environ", MOCK_ENVIRONMENT_DICT) class TestForwardingLambda(unittest.TestCase): @patch("utils_for_record_forwarder.os.getenv") @@ -85,80 +127,89 @@ def test_create_ack_data(self): expected_output, ) - @patch("send_request_to_lambda.lambda_client") @patch("update_ack_file.s3_client") - def test_forward_request_to_api_new_success(self, mock_s3_client, mock_lambda_client): + def test_forward_request_to_api_new_success(self, mock_s3_client): # Mock LastModified as a datetime object mock_s3_client.head_object.return_value = {"LastModified": datetime(2024, 8, 21, 10, 15, 30)} - mock_response = MagicMock() - mock_response["Payload"].read.return_value = json.dumps( - {"statusCode": 201, "headers": {"Location": "https://example.com/immunization/test_id"}} - ) - mock_lambda_client.invoke.return_value = mock_response # Simulate the case where the ack file does not exist mock_s3_client.get_object.side_effect = ClientError({"Error": {"Code": "404"}}, "HeadObject") - # Mock the create_ack_data method - with patch("update_ack_file.create_ack_data") as mock_create_ack_data: - # Prepare the message body for the forward_request_to_lambda function - message_body = { - "row_id": "test_1", - "file_key": "file.csv", - "supplier": "Test_supplier", - "operation_requested": "CREATE", - "fhir_json": {"Name": "test"}, - } - # Call the function you are testing - forward_request_to_lambda(message_body) - # Check that create_ack_data was called with the correct arguments - mock_create_ack_data.assert_called_with("20240821T10153000", "test_1", True, None, "test_id") + message = { + "row_id": "test_1", + "file_key": "file.csv", + "supplier": "Test_supplier", + "operation_requested": "CREATE", + "fhir_json": {"Name": "test"}, + } + + # Mock the create_ack_data method and lambda invocation repsonse payloads + mock_lambda_payloads = {"CREATE": generate_payload(status_code=201, headers=lambda_success_headers)} + with patch("update_ack_file.create_ack_data") as mock_create_ack_data, patch( + "utils_for_record_forwarder.lambda_client.invoke", + side_effect=generate_lambda_invocation_side_effect(message, mock_lambda_payloads), + ): + forward_request_to_lambda(message) + + mock_create_ack_data.assert_called_with("20240821T10153000", "test_1", True, None, "test_id") - @patch("send_request_to_lambda.lambda_client") @patch("update_ack_file.s3_client") - def test_forward_request_to_api_new_success_duplicate(self, mock_s3_client, mock_lambda_client): + def test_forward_request_to_api_new_duplicate(self, mock_s3_client): # Mock LastModified as a datetime object mock_s3_client.head_object.return_value = {"LastModified": datetime(2024, 8, 21, 10, 15, 30)} diagnostics = "The provided identifier: https://supplierABC/identifiers/vacc#test-identifier1 is duplicated" - mock_lambda_client.invoke.return_value = create_mock_search_lambda_response(422, diagnostics) # Simulate the case where the ack file does not exist mock_s3_client.get_object.side_effect = ClientError({"Error": {"Code": "404"}}, "HeadObject") - with patch("update_ack_file.create_ack_data") as mock_create_ack_data: - message_body = { - "row_id": "test_2", - "file_key": "file.csv", - "supplier": "Test_supplier", - "operation_requested": "CREATE", - "fhir_json": {"identifier": [{"system": "test_system", "value": "test_value"}]}, - } - forward_request_to_lambda(message_body) - # Check that the data_rows function was called with success status and formatted datetime - mock_create_ack_data.assert_called_with("20240821T10153000", "test_2", False, diagnostics, None) + message = { + "row_id": "test_2", + "file_key": "file.csv", + "supplier": "Test_supplier", + "operation_requested": "CREATE", + "fhir_json": {"identifier": [{"system": "test_system", "value": "test_value"}]}, + } + + mock_lambda_payloads = { + "CREATE": generate_payload(status_code=422, body=create_mock_operation_outcome(diagnostics)) + } + with patch("update_ack_file.create_ack_data") as mock_create_ack_data, patch( + "utils_for_record_forwarder.lambda_client.invoke", + side_effect=generate_lambda_invocation_side_effect(message, mock_lambda_payloads), + ): + forward_request_to_lambda(message) + + mock_create_ack_data.assert_called_with("20240821T10153000", "test_2", False, diagnostics, None) - @patch("send_request_to_lambda.lambda_client") @patch("update_ack_file.s3_client") - def test_forward_request_to_api_update_failure(self, mock_s3_client, mock_lambda_client): + def test_forward_request_to_api_update_failure(self, mock_s3_client): + # Mock LastModified as a datetime object mock_s3_client.head_object.return_value = {"LastModified": datetime(2024, 8, 21, 10, 15, 30)} diagnostics = ( "Validation errors: The provided immunization id:test_id doesn't match with the content of the request body" ) - mock_lambda_client.invoke.return_value = create_mock_search_lambda_response(422, diagnostics) + # Simulate the case where the ack file does not exist mock_s3_client.get_object.side_effect = ClientError({"Error": {"Code": "404"}}, "HeadObject") + message = { + "row_id": "test_3", + "file_key": "file.csv", + "supplier": "Test_supplier", + "operation_requested": "UPDATE", + "fhir_json": {"identifier": [{"system": "test_system", "value": "test_value"}]}, + } + + mock_lambda_payloads = { + "UPDATE": generate_payload(status_code=422, body=create_mock_operation_outcome(diagnostics)), + "SEARCH": generate_payload(status_code=200, body=response_body_id_and_version_found), + } with patch("update_ack_file.create_ack_data") as mock_create_ack_data, patch( - "send_request_to_lambda.get_imms_id_and_version", return_value=("an_imms_id", 1) + "utils_for_record_forwarder.lambda_client.invoke", + side_effect=generate_lambda_invocation_side_effect(message, mock_lambda_payloads), ): - message_body = { - "row_id": "test_3", - "file_key": "file.csv", - "supplier": "Test_supplier", - "operation_requested": "UPDATE", - "fhir_json": {"identifier": [{"system": "test_system", "value": "test_value"}]}, - } - forward_request_to_lambda(message_body) - mock_create_ack_data.assert_called_with("20240821T10153000", "test_3", False, diagnostics, None) + forward_request_to_lambda(message) + + mock_create_ack_data.assert_called_with("20240821T10153000", "test_3", False, diagnostics, None) - @patch("send_request_to_lambda.lambda_client") + @patch("utils_for_record_forwarder.lambda_client.invoke") @patch("update_ack_file.s3_client") def test_forward_request_to_api_update_failure_imms_id_none(self, mock_s3_client, mock_lambda_client): # Mock LastModified as a datetime object @@ -178,27 +229,33 @@ def test_forward_request_to_api_update_failure_imms_id_none(self, mock_s3_client ) mock_lambda_client.assert_not_called() - @patch("send_request_to_lambda.lambda_client") @patch("update_ack_file.s3_client") - def test_forward_request_to_api_delete_success(self, mock_s3_client, mock_lambda_client): + def test_forward_request_to_api_delete_success(self, mock_s3_client): + # Mock LastModified as a datetime object mock_s3_client.head_object.return_value = {"LastModified": datetime(2024, 8, 21, 10, 15, 30)} + # Simulate the case where the ack file does not exist mock_s3_client.get_object.side_effect = ClientError({"Error": {"Code": "404"}}, "HeadObject") - mock_response = MagicMock() - mock_response["Payload"].read.return_value = json.dumps( - {"statusCode": 204, "headers": {"Location": "https://example.com/immunization/test_id"}} - ) - mock_lambda_client.invoke.return_value = mock_response + + message = { + "row_id": "test_6", + "file_key": "file.csv", + "operation_requested": "DELETE", + "fhir_json": {"identifier": [{"system": "test_system", "value": "test_value"}]}, + } + + mock_lambda_payloads = { + "DELETE": generate_payload(status_code=204), + "SEARCH": generate_payload(status_code=200, body=response_body_id_and_version_found), + } with patch("update_ack_file.create_ack_data") as mock_create_ack_data, patch( - "send_request_to_lambda.get_imms_id_and_version", return_value=("an_imms_id", 1) + "utils_for_record_forwarder.lambda_client.invoke", + side_effect=generate_lambda_invocation_side_effect(message, mock_lambda_payloads), ): - message_body = { - "row_id": "test_6", - "file_key": "file.csv", - "operation_requested": "DELETE", - "fhir_json": {"identifier": [{"system": "test_system", "value": "test_value"}]}, - } - forward_request_to_lambda(message_body) - mock_create_ack_data.assert_called_with("20240821T10153000", "test_6", True, None, "an_imms_id") + forward_request_to_lambda(message) + + mock_create_ack_data.assert_called_with( + "20240821T10153000", "test_6", True, None, "277befd9-574e-47fe-a6ee-189858af3bb0" + ) @patch("forwarding_lambda.forward_request_to_lambda") @patch("utils_for_record_forwarder.get_environment") diff --git a/recordforwarder/tests/utils_for_recordfowarder_tests/utils_for_recordforwarder_tests.py b/recordforwarder/tests/utils_for_recordfowarder_tests/utils_for_recordforwarder_tests.py index 5c48fa90..42f9361f 100644 --- a/recordforwarder/tests/utils_for_recordfowarder_tests/utils_for_recordforwarder_tests.py +++ b/recordforwarder/tests/utils_for_recordfowarder_tests/utils_for_recordforwarder_tests.py @@ -3,17 +3,17 @@ import json -def create_mock_operation_outcome(diagnostics: str) -> dict: +def create_mock_operation_outcome(diagnostics: str, code: str = "duplicate") -> dict: return { "resourceType": "OperationOutcome", - "id": "45b552ca-755a-473f-84df-c7e7767bd2ac", + "id": "an_imms_id", "meta": {"profile": ["https://simplifier.net/guide/UKCoreDevelopment2/ProfileUKCore-OperationOutcome"]}, "issue": [ { "severity": "error", - "code": "duplicate", + "code": code, "details": { - "coding": [{"system": "https://fhir.nhs.uk/Codesystem/http-error-codes", "code": "DUPLICATE"}] + "coding": [{"system": "https://fhir.nhs.uk/Codesystem/http-error-codes", "code": code.upper()}] }, "diagnostics": diagnostics, } diff --git a/recordforwarder/tests/utils_for_recordfowarder_tests/values_for_recordforwarder_tests.py b/recordforwarder/tests/utils_for_recordfowarder_tests/values_for_recordforwarder_tests.py index ccdff6b5..f149063d 100644 --- a/recordforwarder/tests/utils_for_recordfowarder_tests/values_for_recordforwarder_tests.py +++ b/recordforwarder/tests/utils_for_recordfowarder_tests/values_for_recordforwarder_tests.py @@ -2,8 +2,6 @@ SOURCE_BUCKET_NAME = "immunisation-batch-internal-dev-data-sources" DESTINATION_BUCKET_NAME = "immunisation-batch-internal-dev-data-destinations" -CONFIG_BUCKET_NAME = "immunisation-batch-internal-dev-configs" -STREAM_NAME = "imms-batch-internal-dev-processingdata-stream" AWS_REGION = "eu-west-2" @@ -15,14 +13,6 @@ TEST_FILE_KEY = f"{TEST_VACCINE_TYPE}_Vaccinations_v5_{TEST_ODS_CODE}_20210730T12000000.csv" TEST_ACK_FILE_KEY = f"forwardedFile/{TEST_VACCINE_TYPE}_Vaccinations_v5_{TEST_ODS_CODE}_20210730T12000000_BusAck.csv" -MOCK_ENVIRONMENT_DICT = { - "ENVIRONMENT": "internal-dev", - "LOCAL_ACCOUNT_ID": "123456", - "ACK_BUCKET_NAME": DESTINATION_BUCKET_NAME, - "SHORT_QUEUE_PREFIX": "imms-batch-internal-dev", - "KINESIS_STREAM_ARN": f"arn:aws:kinesis:{AWS_REGION}:123456789012:stream/{STREAM_NAME}", -} - test_fhir_json = { "resourceType": "Immunization", "contained": [ From ec5787a2fe12145265774c84358ef61fd60a8947 Mon Sep 17 00:00:00 2001 From: AlexandraBenson Date: Mon, 4 Nov 2024 11:22:34 +0000 Subject: [PATCH 03/22] Re-arrange test values --- .../src/get_imms_id_and_version.py | 6 +- recordforwarder/src/send_request_to_lambda.py | 2 +- .../tests/test_e2e_forwarding_lambda.py | 38 +++-------- .../tests/test_forwarding_lambda.py | 25 ++------ .../tests/test_get_imms_id_and_version.py | 32 +++++++--- .../utils_for_recordforwarder_tests.py | 29 ++------- .../values_for_recordforwarder_tests.py | 64 +++++++------------ 7 files changed, 68 insertions(+), 128 deletions(-) diff --git a/recordforwarder/src/get_imms_id_and_version.py b/recordforwarder/src/get_imms_id_and_version.py index f3abf2b9..ec02bdbf 100644 --- a/recordforwarder/src/get_imms_id_and_version.py +++ b/recordforwarder/src/get_imms_id_and_version.py @@ -16,10 +16,10 @@ def get_imms_id_and_version(fhir_json: dict) -> tuple[str, int]: identifier = fhir_json.get("identifier", [{}])[0] immunization_identifier = f"{identifier.get('system')}|{identifier.get('value')}" query_string_parameters = {"_element": "id,meta", "immunization.identifier": immunization_identifier} - payload = {"headers": headers, "body": None, "queryStringParameters": query_string_parameters} + request_payload = {"headers": headers, "body": None, "queryStringParameters": query_string_parameters} # Invoke lambda - status_code, body, _ = invoke_lambda(os.getenv("SEARCH_LAMBDA_NAME"), payload) + status_code, body, _ = invoke_lambda(os.getenv("SEARCH_LAMBDA_NAME"), request_payload) # Handle non-200 or empty response if not (body.get("total") == 1 and status_code == 200): @@ -27,5 +27,5 @@ def get_imms_id_and_version(fhir_json: dict) -> tuple[str, int]: raise IdNotFoundError("Imms id not found") # Return imms_id and version - resource = body.get("entry", [])[0]["resource"] + resource = body.get("entry", [])[0].get("resource", {}) return resource.get("id"), resource.get("meta", {}).get("versionId") diff --git a/recordforwarder/src/send_request_to_lambda.py b/recordforwarder/src/send_request_to_lambda.py index 6cdbd391..28dccc74 100644 --- a/recordforwarder/src/send_request_to_lambda.py +++ b/recordforwarder/src/send_request_to_lambda.py @@ -17,7 +17,7 @@ def send_create_request(fhir_json: dict, supplier: str) -> str: raise MessageNotSuccessfulError(get_operation_outcome_diagnostics(body)) # Return imms id (default to None if unable to find the id) - return headers.get("Location").split("/")[-1] or None + return headers.get("Location", "").split("/")[-1] or None def send_update_request(fhir_json: dict, supplier: str) -> str: diff --git a/recordforwarder/tests/test_e2e_forwarding_lambda.py b/recordforwarder/tests/test_e2e_forwarding_lambda.py index d4f3af71..b28a1bce 100644 --- a/recordforwarder/tests/test_e2e_forwarding_lambda.py +++ b/recordforwarder/tests/test_e2e_forwarding_lambda.py @@ -1,8 +1,9 @@ import unittest from unittest.mock import patch -from boto3 import client as boto3_client +import base64 import json from io import StringIO +from boto3 import client as boto3_client from moto import mock_s3 from forwarding_lambda import forward_lambda_handler from tests.utils_for_recordfowarder_tests.values_for_recordforwarder_tests import ( @@ -12,40 +13,21 @@ DESTINATION_BUCKET_NAME, TEST_FILE_KEY, TEST_ACK_FILE_KEY, - TEST_SUPPLIER, - TEST_ROW_ID, + base_message_fields, + lambda_success_headers, + MOCK_ENVIRONMENT_DICT, ) from tests.utils_for_recordfowarder_tests.utils_for_recordforwarder_tests import ( response_body_id_and_version_found, response_body_id_and_version_not_found, create_mock_operation_outcome, + generate_payload, ) -import base64 - -def generate_payload(status_code: int, headers: dict = {}, body: dict = None): - return {"statusCode": status_code, **({"body": json.dumps(body)} if body is not None else {}), "headers": headers} - - -base_message_fields = {"row_id": TEST_ROW_ID, "file_key": TEST_FILE_KEY, "supplier": TEST_SUPPLIER} s3_client = boto3_client("s3", region_name=AWS_REGION) kinesis_client = boto3_client("kinesis", region_name=AWS_REGION) -lambda_success_headers = {"Location": "https://example.com/immunization/test_id"} - -MOCK_ENVIRONMENT_DICT = { - "SOURCE_BUCKET_NAME": "immunisation-batch-internal-dev-data-sources", - "ACK_BUCKET_NAME": "immunisation-batch-internal-dev-data-destinations", - "ENVIRONMENT": "internal-dev", - "LOCAL_ACCOUNT_ID": "123456789012", - "SHORT_QUEUE_PREFIX": "imms-batch-internal-dev", - "CREATE_LAMBDA_NAME": "mock_create_lambda_name", - "UPDATE_LAMBDA_NAME": "mock_update_lambda_name", - "DELETE_LAMBDA_NAME": "mock_delete_lambda_name", - "SEARCH_LAMBDA_NAME": "mock_search_lambda_name", -} - @mock_s3 @patch.dict("os.environ", MOCK_ENVIRONMENT_DICT) @@ -64,7 +46,7 @@ def create_kinesis_message(self, message): kinesis_encoded_data = base64.b64encode(json.dumps(message).encode("utf-8")).decode("utf-8") return {"Records": [{"kinesis": {"data": kinesis_encoded_data}}]} - def check_ack_file(self, s3_client, expected_content): + def check_ack_file(self, expected_content): """Helper to check the acknowledgment file content""" ack_file_obj = s3_client.get_object(Bucket=DESTINATION_BUCKET_NAME, Key=TEST_ACK_FILE_KEY) ack_file_content = ack_file_obj["Body"].read().decode("utf-8") @@ -89,7 +71,7 @@ def lambda_invocation_side_effect(FunctionName, *_args, **_kwargs): # pylint: d with patch("utils_for_record_forwarder.lambda_client.invoke", side_effect=lambda_invocation_side_effect): forward_lambda_handler(event=self.create_kinesis_message(message), _=None) - self.check_ack_file(s3_client, expected_content) + self.check_ack_file(expected_content) def test_forward_lambda_e2e_update_failed_unable_to_get_id(self): message = {**base_message_fields, "fhir_json": test_fhir_json, "operation_requested": "UPDATE"} @@ -138,7 +120,7 @@ def test_forward_lambda_e2e_none_request(self, mock_api): forward_lambda_handler(self.create_kinesis_message(message), None) - self.check_ack_file(s3_client, "Fatal Error") + self.check_ack_file("Fatal Error") mock_api.create_immunization.assert_not_called() def test_forward_lambda_e2e_update_success(self): @@ -181,7 +163,7 @@ def test_forward_lambda_e2e_no_permissions(self): forward_lambda_handler(self.create_kinesis_message(message), None) - self.check_ack_file(s3_client, "Fatal Error") + self.check_ack_file("Fatal Error") if __name__ == "__main__": diff --git a/recordforwarder/tests/test_forwarding_lambda.py b/recordforwarder/tests/test_forwarding_lambda.py index 0b9613cd..984c3c6c 100644 --- a/recordforwarder/tests/test_forwarding_lambda.py +++ b/recordforwarder/tests/test_forwarding_lambda.py @@ -14,6 +14,11 @@ from tests.utils_for_recordfowarder_tests.utils_for_recordforwarder_tests import ( create_mock_operation_outcome, response_body_id_and_version_found, + generate_payload, +) +from tests.utils_for_recordfowarder_tests.values_for_recordforwarder_tests import ( + lambda_success_headers, + MOCK_ENVIRONMENT_DICT, ) @@ -37,26 +42,6 @@ def lambda_invocation_side_effect(FunctionName, *_args, **_kwargs): # pylint: d return lambda_invocation_side_effect -def generate_payload(status_code: int, headers: dict = {}, body: dict = None): - return {"statusCode": status_code, **({"body": json.dumps(body)} if body is not None else {}), "headers": headers} - - -lambda_success_headers = {"Location": "https://example.com/immunization/test_id"} - - -MOCK_ENVIRONMENT_DICT = { - "SOURCE_BUCKET_NAME": "immunisation-batch-internal-dev-data-sources", - "ACK_BUCKET_NAME": "immunisation-batch-internal-dev-data-destinations", - "ENVIRONMENT": "internal-dev", - "LOCAL_ACCOUNT_ID": "123456789012", - "SHORT_QUEUE_PREFIX": "imms-batch-internal-dev", - "CREATE_LAMBDA_NAME": "mock_create_lambda_name", - "UPDATE_LAMBDA_NAME": "mock_update_lambda_name", - "DELETE_LAMBDA_NAME": "mock_delete_lambda_name", - "SEARCH_LAMBDA_NAME": "mock_search_lambda_name", -} - - @mock_s3 @patch.dict("os.environ", MOCK_ENVIRONMENT_DICT) class TestForwardingLambda(unittest.TestCase): diff --git a/recordforwarder/tests/test_get_imms_id_and_version.py b/recordforwarder/tests/test_get_imms_id_and_version.py index 11f471a3..cc9b4a3c 100644 --- a/recordforwarder/tests/test_get_imms_id_and_version.py +++ b/recordforwarder/tests/test_get_imms_id_and_version.py @@ -2,10 +2,18 @@ import unittest from unittest.mock import patch +import json +from io import StringIO from moto import mock_s3 from get_imms_id_and_version import get_imms_id_and_version from errors import IdNotFoundError -from tests.utils_for_recordfowarder_tests.utils_for_recordforwarder_tests import create_mock_search_lambda_response +from tests.utils_for_recordfowarder_tests.utils_for_recordforwarder_tests import ( + generate_payload, + response_body_id_and_version_found, + response_body_id_and_version_not_found, + create_mock_operation_outcome, +) + fhir_json_with_identifier_value_and_system = {"identifier": [{"value": "a_value", "system": "a_system"}]} @@ -19,7 +27,10 @@ class TestGetImmsIdAndVersion(unittest.TestCase): def test_success(self): """Test that imms_id and version are correctly identified from a successful search lambda response.""" - with patch("clients.lambda_client.invoke", return_value=create_mock_search_lambda_response(200)): + mock_lambda_response_payload = { + "Payload": StringIO(json.dumps(generate_payload(200, body=response_body_id_and_version_found))) + } + with patch("clients.lambda_client.invoke", return_value=mock_lambda_response_payload): imms_id, version = get_imms_id_and_version(fhir_json_with_identifier_value_and_system) self.assertEqual(imms_id, "277befd9-574e-47fe-a6ee-189858af3bb0") @@ -27,17 +38,20 @@ def test_success(self): def test_failure_due_to_empty_search_lambda_return(self): """Test that an IdNotFoundError is raised for a successful search lambda response which contains no entries.""" - with patch( - "clients.lambda_client.invoke", - return_value=create_mock_search_lambda_response(200, id_and_version_found=False), - ): + mock_lambda_response_payload = { + "Payload": StringIO(json.dumps(generate_payload(200, body=response_body_id_and_version_not_found))) + } + with patch("clients.lambda_client.invoke", return_value=mock_lambda_response_payload): with self.assertRaises(IdNotFoundError): get_imms_id_and_version(fhir_json_with_identifier_value_and_system) def test_failure_due_to_search_lambda_404(self): """Test that an IdNotFoundError is raised for an unsuccessful search lambda response.""" - with patch( - "clients.lambda_client.invoke", return_value=create_mock_search_lambda_response(404, "some diagnostics") - ): + mock_lambda_response_payload = { + "Payload": StringIO( + json.dumps(generate_payload(404, body=create_mock_operation_outcome("some_diagnostics"))) + ) + } + with patch("clients.lambda_client.invoke", return_value=mock_lambda_response_payload): with self.assertRaises(IdNotFoundError): get_imms_id_and_version(fhir_json_with_identifier_value_and_system) diff --git a/recordforwarder/tests/utils_for_recordfowarder_tests/utils_for_recordforwarder_tests.py b/recordforwarder/tests/utils_for_recordfowarder_tests/utils_for_recordforwarder_tests.py index 42f9361f..2154ff67 100644 --- a/recordforwarder/tests/utils_for_recordfowarder_tests/utils_for_recordforwarder_tests.py +++ b/recordforwarder/tests/utils_for_recordfowarder_tests/utils_for_recordforwarder_tests.py @@ -1,5 +1,3 @@ -from unittest.mock import MagicMock -import requests import json @@ -21,6 +19,10 @@ def create_mock_operation_outcome(diagnostics: str, code: str = "duplicate") -> } +def generate_payload(status_code: int, headers: dict = {}, body: dict = None): + return {"statusCode": status_code, **({"body": json.dumps(body)} if body is not None else {}), "headers": headers} + + response_body_id_and_version_not_found = { "resourceType": "Bundle", "type": "searchset", @@ -41,26 +43,3 @@ def create_mock_operation_outcome(diagnostics: str, code: str = "duplicate") -> "entry": [{"resource": {"id": "277befd9-574e-47fe-a6ee-189858af3bb0", "meta": {"versionId": 2}}}], "total": 1, } - - -def create_mock_search_lambda_response( - status_code: int, diagnostics: str = None, id_and_version_found: bool = True -) -> requests.Response: - """Creates a mock response for a request sent to the search lambda for imms_id and version.""" - - body = ( - create_mock_operation_outcome(diagnostics) - if diagnostics - else response_body_id_and_version_found if id_and_version_found else response_body_id_and_version_not_found - ) - - mock_response = MagicMock() - mock_response["Payload"].read.return_value = json.dumps( - { - "statusCode": status_code, - "headers": {"Location": "https://example.com/immunization/test_id"}, - **({"body": json.dumps(body)} if body is not None else {}), - } - ) - - return mock_response diff --git a/recordforwarder/tests/utils_for_recordfowarder_tests/values_for_recordforwarder_tests.py b/recordforwarder/tests/utils_for_recordfowarder_tests/values_for_recordforwarder_tests.py index f149063d..dabd53ed 100644 --- a/recordforwarder/tests/utils_for_recordfowarder_tests/values_for_recordforwarder_tests.py +++ b/recordforwarder/tests/utils_for_recordfowarder_tests/values_for_recordforwarder_tests.py @@ -13,23 +13,29 @@ TEST_FILE_KEY = f"{TEST_VACCINE_TYPE}_Vaccinations_v5_{TEST_ODS_CODE}_20210730T12000000.csv" TEST_ACK_FILE_KEY = f"forwardedFile/{TEST_VACCINE_TYPE}_Vaccinations_v5_{TEST_ODS_CODE}_20210730T12000000_BusAck.csv" +base_message_fields = {"row_id": TEST_ROW_ID, "file_key": TEST_FILE_KEY, "supplier": TEST_SUPPLIER} +lambda_success_headers = {"Location": "https://example.com/immunization/test_id"} + +MOCK_ENVIRONMENT_DICT = { + "SOURCE_BUCKET_NAME": "immunisation-batch-internal-dev-data-sources", + "ACK_BUCKET_NAME": "immunisation-batch-internal-dev-data-destinations", + "ENVIRONMENT": "internal-dev", + "LOCAL_ACCOUNT_ID": "123456789012", + "SHORT_QUEUE_PREFIX": "imms-batch-internal-dev", + "CREATE_LAMBDA_NAME": "mock_create_lambda_name", + "UPDATE_LAMBDA_NAME": "mock_update_lambda_name", + "DELETE_LAMBDA_NAME": "mock_delete_lambda_name", + "SEARCH_LAMBDA_NAME": "mock_search_lambda_name", +} + test_fhir_json = { "resourceType": "Immunization", "contained": [ - { - "resourceType": "Practitioner", - "id": "Pract1", - "name": [{"family": "Doe", "given": ["Jane"]}], - }, + {"resourceType": "Practitioner", "id": "Pract1", "name": [{"family": "Doe", "given": ["Jane"]}]}, { "resourceType": "Patient", "id": "Pat1", - "identifier": [ - { - "system": "https://fhir.nhs.uk/Id/nhs-number", - "value": "1234567890", - } - ], + "identifier": [{"system": "https://fhir.nhs.uk/Id/nhs-number", "value": "1234567890"}], "name": [{"family": "SMITH", "given": ["JOHN"]}], "gender": "male", "birthDate": "2000-01-01", @@ -50,12 +56,7 @@ }, } ], - "identifier": [ - { - "system": "https://www.ravs.england.nhs.uk/", - "value": "0001_TEST_v1_RUN_1_ABCD-123_Dose_seq_01", - } - ], + "identifier": [{"system": "https://www.ravs.england.nhs.uk/", "value": "0001_TEST_v1_RUN_1_ABCD-123_Dose_seq_01"}], "status": "completed", "vaccineCode": { "coding": [ @@ -71,13 +72,7 @@ "recorded": "2024-01-01T00:00:00+00:00", "primarySource": True, "manufacturer": {"display": "Dummy Pharma"}, - "location": { - "type": "Location", - "identifier": { - "value": "ABCDE", - "system": "https://fhir.nhs.uk/Id/ods-organization-code", - }, - }, + "location": {"identifier": {"value": "ABCDE", "system": "https://fhir.nhs.uk/Id/ods-organization-code"}}, "lotNumber": "DUMMYLOT", "expirationDate": "2024-12-31", "site": { @@ -91,11 +86,7 @@ }, "route": { "coding": [ - { - "system": "http://snomed.info/sct", - "code": "111111111", - "display": "Subcutaneous route (qualifier value)", - } + {"system": "http://snomed.info/sct", "code": "111111111", "display": "Subcutaneous route (qualifier value)"} ] }, "doseQuantity": { @@ -109,10 +100,7 @@ { "actor": { "type": "Organization", - "identifier": { - "system": "https://fhir.nhs.uk/Id/ods-organization-code", - "value": "DUMMYORG", - }, + "identifier": {"system": "https://fhir.nhs.uk/Id/ods-organization-code", "value": "DUMMYORG"}, } }, ], @@ -120,15 +108,7 @@ "protocolApplied": [ { "targetDisease": [ - { - "coding": [ - { - "system": "http://snomed.info/sct", - "code": "123456789", - "display": "Dummy disease caused by dummy virus", - } - ] - } + {"coding": [{"system": "http://snomed.info/sct", "code": "123456789", "display": "Dummy disease"}]} ], "doseNumberPositiveInt": 1, } From 97a66d614666296d3c1a922259ae46fbeac671c4 Mon Sep 17 00:00:00 2001 From: AlexandraBenson Date: Mon, 4 Nov 2024 11:34:19 +0000 Subject: [PATCH 04/22] Add tests for utils --- .../tests/test_utils_for_recordforwarder.py | 32 +++++++++++++++++++ 1 file changed, 32 insertions(+) create mode 100644 recordforwarder/tests/test_utils_for_recordforwarder.py diff --git a/recordforwarder/tests/test_utils_for_recordforwarder.py b/recordforwarder/tests/test_utils_for_recordforwarder.py new file mode 100644 index 00000000..6ad75c80 --- /dev/null +++ b/recordforwarder/tests/test_utils_for_recordforwarder.py @@ -0,0 +1,32 @@ +"""Tests for utils_for_recordforwarder functions""" + +from unittest import TestCase +from unittest.mock import patch +from utils_for_record_forwarder import get_environment +from constants import ACK_HEADERS +from update_ack_file import create_ack_data + + +class TestUtilsForRecordForwarder(TestCase): + """Tests for utils_for_recordforwarder functions""" + + def test_ack_headers_match_ack_data_keys(self): + """Ensures that the ACK_HEADERS found in constants, match the headers given as keys in create_ack_data""" + self.assertEqual(ACK_HEADERS, list(create_ack_data("TEST", "TEST", True).keys())) + + def test_get_environment(self): + "Tests that get_environment returns the correct environment" + # Each test case tuple has the structure (environment, expected_result) + test_cases = ( + ("internal-dev", "internal-dev"), + ("int", "int"), + ("ref", "ref"), + ("sandbox", "sandbox"), + ("prod", "prod"), + ("pr-22", "internal-dev"), + ) + + for environment, expected_result in test_cases: + with self.subTest(f"SubTest for environment: {environment}"): + with patch.dict("os.environ", {"ENVIRONMENT": environment}): + self.assertEqual(get_environment(), expected_result) From b6f06b004f301dfbc34c9abf6dfc8929e1c2b032 Mon Sep 17 00:00:00 2001 From: AlexandraBenson Date: Mon, 4 Nov 2024 11:48:48 +0000 Subject: [PATCH 05/22] Fix test issues --- recordforwarder/Makefile | 4 ++-- recordforwarder/tests/test_forwarding_lambda.py | 3 --- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/recordforwarder/Makefile b/recordforwarder/Makefile index 908b943c..12823b5d 100644 --- a/recordforwarder/Makefile +++ b/recordforwarder/Makefile @@ -8,12 +8,12 @@ test: python -m unittest coverage run: - coverage run -m unittest discover + coverage run -m unittest discover coverage report: coverage report -m coverage html: - coverage html + coverage html .PHONY: build package \ No newline at end of file diff --git a/recordforwarder/tests/test_forwarding_lambda.py b/recordforwarder/tests/test_forwarding_lambda.py index 4314f0b3..034d180f 100644 --- a/recordforwarder/tests/test_forwarding_lambda.py +++ b/recordforwarder/tests/test_forwarding_lambda.py @@ -33,9 +33,6 @@ from forwarding_lambda import forward_lambda_handler, forward_request_to_lambda # noqa: E402 from utils_for_record_forwarder import get_environment # noqa: E402 from update_ack_file import create_ack_data # noqa: E402 -from tests.utils_for_recordfowarder_tests.utils_for_recordforwarder_tests import ( # noqa: E402 - create_mock_search_lambda_response, -) s3_client = boto3_client("s3", region_name=AWS_REGION) From 53f9fc7110534921457e36d8a6cab82120cf0b53 Mon Sep 17 00:00:00 2001 From: AlexandraBenson Date: Mon, 4 Nov 2024 12:16:23 +0000 Subject: [PATCH 06/22] Fix imports --- .../tests/test_e2e_forwarding_lambda.py | 19 ++++---- .../tests/test_forwarding_lambda.py | 47 +++++++++---------- 2 files changed, 33 insertions(+), 33 deletions(-) diff --git a/recordforwarder/tests/test_e2e_forwarding_lambda.py b/recordforwarder/tests/test_e2e_forwarding_lambda.py index b6b8e203..9b989ef5 100644 --- a/recordforwarder/tests/test_e2e_forwarding_lambda.py +++ b/recordforwarder/tests/test_e2e_forwarding_lambda.py @@ -1,19 +1,22 @@ +"""E2e tests for forwarding lambda""" + import unittest from unittest.mock import patch import base64 import json from io import StringIO -from boto3 import client as boto3_client -from moto import mock_s3 import os import sys -import base64 +from boto3 import client as boto3_client +from moto import mock_s3 +# Import local modules after adjusting the path maindir = os.path.dirname(__file__) -srcdir = "../src" -sys.path.insert(0, os.path.abspath(os.path.join(maindir, srcdir))) -from forwarding_lambda import forward_lambda_handler # noqa: E402 -from tests.utils_for_recordfowarder_tests.values_for_recordforwarder_tests import ( # noqa: E402 +SRCDIR = "../src" +sys.path.insert(0, os.path.abspath(os.path.join(maindir, SRCDIR))) + +from forwarding_lambda import forward_lambda_handler # pylint:disable=wrong-import-position +from tests.utils_for_recordfowarder_tests.values_for_recordforwarder_tests import ( # pylint:disable=wrong-import-position test_fhir_json, AWS_REGION, SOURCE_BUCKET_NAME, @@ -24,7 +27,7 @@ lambda_success_headers, MOCK_ENVIRONMENT_DICT, ) -from tests.utils_for_recordfowarder_tests.utils_for_recordforwarder_tests import ( +from tests.utils_for_recordfowarder_tests.utils_for_recordforwarder_tests import ( # pylint:disable=wrong-import-position response_body_id_and_version_found, response_body_id_and_version_not_found, create_mock_operation_outcome, diff --git a/recordforwarder/tests/test_forwarding_lambda.py b/recordforwarder/tests/test_forwarding_lambda.py index 034d180f..2160bf6a 100644 --- a/recordforwarder/tests/test_forwarding_lambda.py +++ b/recordforwarder/tests/test_forwarding_lambda.py @@ -1,38 +1,35 @@ +"""Tests for forwarding lambda""" + import unittest from unittest.mock import patch -from moto import mock_s3 -from boto3 import client as boto3_client +import os +import sys import json from io import StringIO -from botocore.exceptions import ClientError from datetime import datetime import base64 -from tests.utils_for_recordfowarder_tests.values_for_recordforwarder_tests import AWS_REGION -from forwarding_lambda import forward_lambda_handler, forward_request_to_lambda -from utils_for_record_forwarder import get_environment -from update_ack_file import create_ack_data -from tests.utils_for_recordfowarder_tests.utils_for_recordforwarder_tests import ( +from moto import mock_s3 +from boto3 import client as boto3_client +from botocore.exceptions import ClientError + +# Import local modules after adjusting the path +maindir = os.path.dirname(__file__) +SRCDIR = "../src" +sys.path.insert(0, os.path.abspath(os.path.join(maindir, SRCDIR))) + +from tests.utils_for_recordfowarder_tests.values_for_recordforwarder_tests import ( # pylint: disable=wrong-import-position + lambda_success_headers, + MOCK_ENVIRONMENT_DICT, + AWS_REGION, +) +from tests.utils_for_recordfowarder_tests.utils_for_recordforwarder_tests import ( # pylint: disable=wrong-import-position create_mock_operation_outcome, response_body_id_and_version_found, generate_payload, ) -from tests.utils_for_recordfowarder_tests.values_for_recordforwarder_tests import ( - lambda_success_headers, - MOCK_ENVIRONMENT_DICT, -) -import os -import sys - -# Move the sys.path insertion to the top along with other imports -maindir = os.path.dirname(__file__) -srcdir = "../src" -sys.path.insert(0, os.path.abspath(os.path.join(maindir, srcdir))) - -# Import other modules after adjusting the path -from tests.utils_for_recordfowarder_tests.values_for_recordforwarder_tests import AWS_REGION # noqa: E402 -from forwarding_lambda import forward_lambda_handler, forward_request_to_lambda # noqa: E402 -from utils_for_record_forwarder import get_environment # noqa: E402 -from update_ack_file import create_ack_data # noqa: E402 +from forwarding_lambda import forward_lambda_handler, forward_request_to_lambda # pylint: disable=wrong-import-position +from utils_for_record_forwarder import get_environment # pylint: disable=wrong-import-position +from update_ack_file import create_ack_data # pylint: disable=wrong-import-position s3_client = boto3_client("s3", region_name=AWS_REGION) From fc1ca8bc936eef71cfcf326ca1fc87671c1d0204 Mon Sep 17 00:00:00 2001 From: AlexandraBenson Date: Mon, 4 Nov 2024 12:29:37 +0000 Subject: [PATCH 07/22] Fix linting issues --- recordforwarder/tests/test_e2e_forwarding_lambda.py | 7 ++++--- recordforwarder/tests/test_forwarding_lambda.py | 11 ++++++----- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/recordforwarder/tests/test_e2e_forwarding_lambda.py b/recordforwarder/tests/test_e2e_forwarding_lambda.py index 9b989ef5..31b37448 100644 --- a/recordforwarder/tests/test_e2e_forwarding_lambda.py +++ b/recordforwarder/tests/test_e2e_forwarding_lambda.py @@ -1,3 +1,4 @@ +# pylint: disable=wrong-import-position """E2e tests for forwarding lambda""" import unittest @@ -15,8 +16,8 @@ SRCDIR = "../src" sys.path.insert(0, os.path.abspath(os.path.join(maindir, SRCDIR))) -from forwarding_lambda import forward_lambda_handler # pylint:disable=wrong-import-position -from tests.utils_for_recordfowarder_tests.values_for_recordforwarder_tests import ( # pylint:disable=wrong-import-position +from forwarding_lambda import forward_lambda_handler # flake8: noqa: E402 +from tests.utils_for_recordfowarder_tests.values_for_recordforwarder_tests import ( # flake8: noqa: E402 test_fhir_json, AWS_REGION, SOURCE_BUCKET_NAME, @@ -27,7 +28,7 @@ lambda_success_headers, MOCK_ENVIRONMENT_DICT, ) -from tests.utils_for_recordfowarder_tests.utils_for_recordforwarder_tests import ( # pylint:disable=wrong-import-position +from tests.utils_for_recordfowarder_tests.utils_for_recordforwarder_tests import ( # flake8: noqa: E402 response_body_id_and_version_found, response_body_id_and_version_not_found, create_mock_operation_outcome, diff --git a/recordforwarder/tests/test_forwarding_lambda.py b/recordforwarder/tests/test_forwarding_lambda.py index 2160bf6a..28bc2fe8 100644 --- a/recordforwarder/tests/test_forwarding_lambda.py +++ b/recordforwarder/tests/test_forwarding_lambda.py @@ -1,3 +1,4 @@ +# pylint: disable=wrong-import-position """Tests for forwarding lambda""" import unittest @@ -17,19 +18,19 @@ SRCDIR = "../src" sys.path.insert(0, os.path.abspath(os.path.join(maindir, SRCDIR))) -from tests.utils_for_recordfowarder_tests.values_for_recordforwarder_tests import ( # pylint: disable=wrong-import-position +from tests.utils_for_recordfowarder_tests.values_for_recordforwarder_tests import ( # flake8: noqa: E402 lambda_success_headers, MOCK_ENVIRONMENT_DICT, AWS_REGION, ) -from tests.utils_for_recordfowarder_tests.utils_for_recordforwarder_tests import ( # pylint: disable=wrong-import-position +from tests.utils_for_recordfowarder_tests.utils_for_recordforwarder_tests import ( # flake8: noqa: E402 create_mock_operation_outcome, response_body_id_and_version_found, generate_payload, ) -from forwarding_lambda import forward_lambda_handler, forward_request_to_lambda # pylint: disable=wrong-import-position -from utils_for_record_forwarder import get_environment # pylint: disable=wrong-import-position -from update_ack_file import create_ack_data # pylint: disable=wrong-import-position +from forwarding_lambda import forward_lambda_handler, forward_request_to_lambda # flake8: noqa: E402 +from utils_for_record_forwarder import get_environment # flake8: noqa: E402 +from update_ack_file import create_ack_data # flake8: noqa: E402 s3_client = boto3_client("s3", region_name=AWS_REGION) From 1d573d033097eebc40c92a0aab2a284a53f1339a Mon Sep 17 00:00:00 2001 From: AlexandraBenson Date: Mon, 4 Nov 2024 12:41:11 +0000 Subject: [PATCH 08/22] Fix linting issues --- recordforwarder/tests/test_e2e_forwarding_lambda.py | 5 +++-- recordforwarder/tests/test_forwarding_lambda.py | 11 ++++++----- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/recordforwarder/tests/test_e2e_forwarding_lambda.py b/recordforwarder/tests/test_e2e_forwarding_lambda.py index 31b37448..b8f06479 100644 --- a/recordforwarder/tests/test_e2e_forwarding_lambda.py +++ b/recordforwarder/tests/test_e2e_forwarding_lambda.py @@ -1,4 +1,5 @@ # pylint: disable=wrong-import-position +# flake8: noqa: E402 """E2e tests for forwarding lambda""" import unittest @@ -16,8 +17,8 @@ SRCDIR = "../src" sys.path.insert(0, os.path.abspath(os.path.join(maindir, SRCDIR))) -from forwarding_lambda import forward_lambda_handler # flake8: noqa: E402 -from tests.utils_for_recordfowarder_tests.values_for_recordforwarder_tests import ( # flake8: noqa: E402 +from forwarding_lambda import forward_lambda_handler +from tests.utils_for_recordfowarder_tests.values_for_recordforwarder_tests import ( test_fhir_json, AWS_REGION, SOURCE_BUCKET_NAME, diff --git a/recordforwarder/tests/test_forwarding_lambda.py b/recordforwarder/tests/test_forwarding_lambda.py index 28bc2fe8..1cf62a69 100644 --- a/recordforwarder/tests/test_forwarding_lambda.py +++ b/recordforwarder/tests/test_forwarding_lambda.py @@ -1,4 +1,5 @@ # pylint: disable=wrong-import-position +# flake8: noqa: E402 """Tests for forwarding lambda""" import unittest @@ -18,19 +19,19 @@ SRCDIR = "../src" sys.path.insert(0, os.path.abspath(os.path.join(maindir, SRCDIR))) -from tests.utils_for_recordfowarder_tests.values_for_recordforwarder_tests import ( # flake8: noqa: E402 +from tests.utils_for_recordfowarder_tests.values_for_recordforwarder_tests import ( lambda_success_headers, MOCK_ENVIRONMENT_DICT, AWS_REGION, ) -from tests.utils_for_recordfowarder_tests.utils_for_recordforwarder_tests import ( # flake8: noqa: E402 +from tests.utils_for_recordfowarder_tests.utils_for_recordforwarder_tests import ( create_mock_operation_outcome, response_body_id_and_version_found, generate_payload, ) -from forwarding_lambda import forward_lambda_handler, forward_request_to_lambda # flake8: noqa: E402 -from utils_for_record_forwarder import get_environment # flake8: noqa: E402 -from update_ack_file import create_ack_data # flake8: noqa: E402 +from forwarding_lambda import forward_lambda_handler, forward_request_to_lambda +from utils_for_record_forwarder import get_environment +from update_ack_file import create_ack_data s3_client = boto3_client("s3", region_name=AWS_REGION) From 91690df0863f1167717c8232a34ac1f03aa526ff Mon Sep 17 00:00:00 2001 From: AlexandraBenson Date: Mon, 4 Nov 2024 18:44:25 +0000 Subject: [PATCH 09/22] Amend test utils and test_log_structure --- .../tests/test_e2e_forwarding_lambda.py | 46 +- .../tests/test_forwarding_lambda.py | 73 +--- .../tests/test_get_imms_id_and_version.py | 4 +- recordforwarder/tests/test_log_structure.py | 404 ++++++------------ .../utils_for_recordforwarder_tests.py | 36 +- .../values_for_recordforwarder_tests.py | 12 - 6 files changed, 213 insertions(+), 362 deletions(-) diff --git a/recordforwarder/tests/test_e2e_forwarding_lambda.py b/recordforwarder/tests/test_e2e_forwarding_lambda.py index b8f06479..bfc1f6a4 100644 --- a/recordforwarder/tests/test_e2e_forwarding_lambda.py +++ b/recordforwarder/tests/test_e2e_forwarding_lambda.py @@ -4,7 +4,6 @@ import unittest from unittest.mock import patch -import base64 import json from io import StringIO import os @@ -29,11 +28,13 @@ lambda_success_headers, MOCK_ENVIRONMENT_DICT, ) -from tests.utils_for_recordfowarder_tests.utils_for_recordforwarder_tests import ( # flake8: noqa: E402 +from tests.utils_for_recordfowarder_tests.utils_for_recordforwarder_tests import ( response_body_id_and_version_found, response_body_id_and_version_not_found, - create_mock_operation_outcome, + generate_mock_operation_outcome, generate_payload, + generate_kinesis_message, + generate_lambda_invocation_side_effect, ) s3_client = boto3_client("s3", region_name=AWS_REGION) @@ -52,11 +53,6 @@ def setup_s3(self): ) s3_client.put_object(Bucket=SOURCE_BUCKET_NAME, Key=TEST_FILE_KEY, Body="test_data") - def create_kinesis_message(self, message): - """Helper to create mock kinesis messages""" - kinesis_encoded_data = base64.b64encode(json.dumps(message).encode("utf-8")).decode("utf-8") - return {"Records": [{"kinesis": {"data": kinesis_encoded_data}}]} - def check_ack_file(self, expected_content): """Helper to check the acknowledgment file content""" ack_file_obj = s3_client.get_object(Bucket=DESTINATION_BUCKET_NAME, Key=TEST_ACK_FILE_KEY) @@ -66,21 +62,11 @@ def check_ack_file(self, expected_content): def execute_test(self, message, expected_content, mock_lambda_payloads: dict): self.setup_s3() - # Mock the responses from the calls to the Imms FHIR API lambdas - # Note that a different response is mocked for each different lambda call - def lambda_invocation_side_effect(FunctionName, *_args, **_kwargs): # pylint: disable=invalid-name - response_payload = None - # Mock the response for the relevant lambda for the operation - operation = message["operation_requested"] - if FunctionName == f"mock_{operation.lower()}_lambda_name": - response_payload = mock_lambda_payloads[operation] - # Mock the search lambda (for the get_imms_id_and_version lambda call) - elif FunctionName == "mock_search_lambda_name": - response_payload = mock_lambda_payloads["SEARCH"] - return {"Payload": StringIO(json.dumps(response_payload))} - - with patch("utils_for_record_forwarder.lambda_client.invoke", side_effect=lambda_invocation_side_effect): - forward_lambda_handler(event=self.create_kinesis_message(message), _=None) + with patch( + "utils_for_record_forwarder.lambda_client.invoke", + side_effect=generate_lambda_invocation_side_effect(mock_lambda_payloads), + ): + forward_lambda_handler(event=generate_kinesis_message(message), _=None) self.check_ack_file(expected_content) @@ -101,14 +87,14 @@ def test_forward_lambda_e2e_create_duplicate(self): mock_diagnostics = ( "The provided identifier: https://supplierABC/identifiers/vacc#test-identifier1 is duplicated" ) - mock_body = create_mock_operation_outcome(diagnostics=mock_diagnostics, code="duplicate") + mock_body = generate_mock_operation_outcome(diagnostics=mock_diagnostics, code="duplicate") mock_lambda_payloads = {"CREATE": generate_payload(status_code=422, body=mock_body)} self.execute_test(message, "Fatal Error", mock_lambda_payloads=mock_lambda_payloads) def test_forward_lambda_e2e_create_failed(self): message = {**base_message_fields, "fhir_json": test_fhir_json, "operation_requested": "CREATE"} mock_diagnostics = "the provided event ID is either missing or not in the expected format." - mock_body = create_mock_operation_outcome(diagnostics=mock_diagnostics, code="duplicate") + mock_body = generate_mock_operation_outcome(diagnostics=mock_diagnostics, code="duplicate") mock_lambda_payloads = {"CREATE": generate_payload(status_code=400, body=mock_body)} self.execute_test(message, "Fatal Error", mock_lambda_payloads=mock_lambda_payloads) @@ -117,7 +103,7 @@ def test_forward_lambda_e2e_create_multi_line_diagnostics(self): mock_diagnostics = """This a string of diagnostics which spans multiple lines and has some carriage returns\n\nand random space""" - mock_body = create_mock_operation_outcome(diagnostics=mock_diagnostics) + mock_body = generate_mock_operation_outcome(diagnostics=mock_diagnostics) mock_lambda_payloads = {"CREATE": generate_payload(status_code=404, body=mock_body)} expected_single_line_diagnostics = ( "This a string of diagnostics which spans multiple lines and has some carriage returns and random space" @@ -129,7 +115,7 @@ def test_forward_lambda_e2e_none_request(self, mock_api): self.setup_s3() message = {**base_message_fields, "diagnostics": "Unsupported file type received as an attachment"} - forward_lambda_handler(self.create_kinesis_message(message), None) + forward_lambda_handler(generate_kinesis_message(message), None) self.check_ack_file("Fatal Error") mock_api.create_immunization.assert_not_called() @@ -146,7 +132,7 @@ def test_forward_lambda_e2e_update_failed(self): message = {**base_message_fields, "fhir_json": test_fhir_json, "operation_requested": "UPDATE"} mock_diagnstics = "the provided event ID is either missing or not in the expected format." mock_lambda_payloads = { - "UPDATE": generate_payload(400, body=create_mock_operation_outcome(mock_diagnstics)), + "UPDATE": generate_payload(400, body=generate_mock_operation_outcome(mock_diagnstics)), "SEARCH": generate_payload(200, body=response_body_id_and_version_found), } self.execute_test(message, "Fatal Error", mock_lambda_payloads) @@ -163,7 +149,7 @@ def test_forward_lambda_e2e_delete_failed(self): message = {**base_message_fields, "fhir_json": test_fhir_json, "operation_requested": "DELETE"} mock_diagnstics = "the provided event ID is either missing or not in the expected format." mock_lambda_payloads = { - "UPDATE": generate_payload(404, body=create_mock_operation_outcome(mock_diagnstics, code="not-found")), + "UPDATE": generate_payload(404, body=generate_mock_operation_outcome(mock_diagnstics, code="not-found")), "SEARCH": generate_payload(200, body=response_body_id_and_version_not_found), } self.execute_test(message, "Fatal Error", mock_lambda_payloads) @@ -172,7 +158,7 @@ def test_forward_lambda_e2e_no_permissions(self): self.setup_s3() message = {**base_message_fields, "diagnostics": "No permissions for operation"} - forward_lambda_handler(self.create_kinesis_message(message), None) + forward_lambda_handler(generate_kinesis_message(message), None) self.check_ack_file("Fatal Error") diff --git a/recordforwarder/tests/test_forwarding_lambda.py b/recordforwarder/tests/test_forwarding_lambda.py index 1cf62a69..faa4e130 100644 --- a/recordforwarder/tests/test_forwarding_lambda.py +++ b/recordforwarder/tests/test_forwarding_lambda.py @@ -6,10 +6,7 @@ from unittest.mock import patch import os import sys -import json -from io import StringIO from datetime import datetime -import base64 from moto import mock_s3 from boto3 import client as boto3_client from botocore.exceptions import ClientError @@ -25,9 +22,11 @@ AWS_REGION, ) from tests.utils_for_recordfowarder_tests.utils_for_recordforwarder_tests import ( - create_mock_operation_outcome, + generate_mock_operation_outcome, response_body_id_and_version_found, generate_payload, + generate_kinesis_message, + generate_lambda_invocation_side_effect, ) from forwarding_lambda import forward_lambda_handler, forward_request_to_lambda from utils_for_record_forwarder import get_environment @@ -37,23 +36,6 @@ s3_client = boto3_client("s3", region_name=AWS_REGION) -def generate_lambda_invocation_side_effect(message, mock_lambda_payloads): - # Mock the responses from the calls to the Imms FHIR API lambdas - # Note that a different response is mocked for each different lambda call - def lambda_invocation_side_effect(FunctionName, *_args, **_kwargs): # pylint: disable=invalid-name - response_payload = None - # Mock the response for the relevant lambda for the operation - operation = message["operation_requested"] - if FunctionName == f"mock_{operation.lower()}_lambda_name": - response_payload = mock_lambda_payloads[operation] - # Mock the search lambda (for the get_imms_id_and_version lambda call) - elif FunctionName == "mock_search_lambda_name": - response_payload = mock_lambda_payloads["SEARCH"] - return {"Payload": StringIO(json.dumps(response_payload))} - - return lambda_invocation_side_effect - - @mock_s3 @patch.dict("os.environ", MOCK_ENVIRONMENT_DICT) class TestForwardingLambda(unittest.TestCase): @@ -143,7 +125,7 @@ def test_forward_request_to_api_new_success(self, mock_s3_client): mock_lambda_payloads = {"CREATE": generate_payload(status_code=201, headers=lambda_success_headers)} with patch("update_ack_file.create_ack_data") as mock_create_ack_data, patch( "utils_for_record_forwarder.lambda_client.invoke", - side_effect=generate_lambda_invocation_side_effect(message, mock_lambda_payloads), + side_effect=generate_lambda_invocation_side_effect(mock_lambda_payloads), ): forward_request_to_lambda(message) @@ -166,11 +148,11 @@ def test_forward_request_to_api_new_duplicate(self, mock_s3_client): } mock_lambda_payloads = { - "CREATE": generate_payload(status_code=422, body=create_mock_operation_outcome(diagnostics)) + "CREATE": generate_payload(status_code=422, body=generate_mock_operation_outcome(diagnostics)) } with patch("update_ack_file.create_ack_data") as mock_create_ack_data, patch( "utils_for_record_forwarder.lambda_client.invoke", - side_effect=generate_lambda_invocation_side_effect(message, mock_lambda_payloads), + side_effect=generate_lambda_invocation_side_effect(mock_lambda_payloads), ): forward_request_to_lambda(message) @@ -195,12 +177,12 @@ def test_forward_request_to_api_update_failure(self, mock_s3_client): } mock_lambda_payloads = { - "UPDATE": generate_payload(status_code=422, body=create_mock_operation_outcome(diagnostics)), + "UPDATE": generate_payload(status_code=422, body=generate_mock_operation_outcome(diagnostics)), "SEARCH": generate_payload(status_code=200, body=response_body_id_and_version_found), } with patch("update_ack_file.create_ack_data") as mock_create_ack_data, patch( "utils_for_record_forwarder.lambda_client.invoke", - side_effect=generate_lambda_invocation_side_effect(message, mock_lambda_payloads), + side_effect=generate_lambda_invocation_side_effect(mock_lambda_payloads), ): forward_request_to_lambda(message) @@ -246,7 +228,7 @@ def test_forward_request_to_api_delete_success(self, mock_s3_client): } with patch("update_ack_file.create_ack_data") as mock_create_ack_data, patch( "utils_for_record_forwarder.lambda_client.invoke", - side_effect=generate_lambda_invocation_side_effect(message, mock_lambda_payloads), + side_effect=generate_lambda_invocation_side_effect(mock_lambda_payloads), ): forward_request_to_lambda(message) @@ -255,52 +237,37 @@ def test_forward_request_to_api_delete_success(self, mock_s3_client): ) @patch("forwarding_lambda.forward_request_to_lambda") - @patch("utils_for_record_forwarder.get_environment") - def test_forward_lambda_handler(self, mock_get_environment, mock_forward_request_to_api): - # Mock the environment to return 'internal-dev' - mock_get_environment.return_value = "internal-dev" - - # Simulate the event data that Lambda would receive + def test_forward_lambda_handler(self, mock_forward_request_to_api): message_body = { "row_id": "test_7", "fhir_json": "{}", "operation_requested": "CREATE", "file_key": "test_file.csv", } - event = { - "Records": [ - {"kinesis": {"data": base64.b64encode(json.dumps(message_body).encode("utf-8")).decode("utf-8")}} - ] - } - forward_lambda_handler(event, None) + + forward_lambda_handler(generate_kinesis_message(message_body), None) mock_forward_request_to_api.assert_called_once_with(message_body) @patch("forwarding_lambda.forward_request_to_lambda") - @patch("utils_for_record_forwarder.get_environment") - def test_forward_lambda_handler_update(self, mock_get_environment, mock_forward_request_to_api): - mock_get_environment.return_value = "internal-dev" + def test_forward_lambda_handler_update(self, mock_forward_request_to_api): message_body = { "row_id": "test_8", "fhir_json": "{}", "operation_requested": "UPDATE", "file_key": "test_file.csv", } - event = { - "Records": [ - {"kinesis": {"data": base64.b64encode(json.dumps(message_body).encode("utf-8")).decode("utf-8")}} - ] - } - forward_lambda_handler(event, None) + forward_lambda_handler(generate_kinesis_message(message_body), None) mock_forward_request_to_api.assert_called_once_with(message_body) @patch("forwarding_lambda.logger") def test_forward_lambda_handler_with_exception(self, mock_logger): - event = { - "Records": [ - {"body": json.dumps({"fhir_json": "{}", "action_flag": "invalid_action", "file_key": "test_file.csv"})} - ] + message_body = { + "row_id": "test_9", + "fhir_json": "{}", + "operation_requested": "INVALID OPERATION", + "file_key": "test_file.csv", } - forward_lambda_handler(event, None) + forward_lambda_handler(generate_kinesis_message(message_body), None) mock_logger.error.assert_called() diff --git a/recordforwarder/tests/test_get_imms_id_and_version.py b/recordforwarder/tests/test_get_imms_id_and_version.py index cc9b4a3c..f9a50839 100644 --- a/recordforwarder/tests/test_get_imms_id_and_version.py +++ b/recordforwarder/tests/test_get_imms_id_and_version.py @@ -11,7 +11,7 @@ generate_payload, response_body_id_and_version_found, response_body_id_and_version_not_found, - create_mock_operation_outcome, + generate_mock_operation_outcome, ) @@ -49,7 +49,7 @@ def test_failure_due_to_search_lambda_404(self): """Test that an IdNotFoundError is raised for an unsuccessful search lambda response.""" mock_lambda_response_payload = { "Payload": StringIO( - json.dumps(generate_payload(404, body=create_mock_operation_outcome("some_diagnostics"))) + json.dumps(generate_payload(404, body=generate_mock_operation_outcome("some_diagnostics"))) ) } with patch("clients.lambda_client.invoke", return_value=mock_lambda_response_payload): diff --git a/recordforwarder/tests/test_log_structure.py b/recordforwarder/tests/test_log_structure.py index adf33379..bd94c130 100644 --- a/recordforwarder/tests/test_log_structure.py +++ b/recordforwarder/tests/test_log_structure.py @@ -1,287 +1,165 @@ +"""Tests for Splunk logging""" + import unittest from unittest.mock import patch import json +from copy import deepcopy from datetime import datetime from send_request_to_lambda import send_request_to_lambda -from tests.utils_for_recordfowarder_tests.values_for_recordforwarder_tests import ( - TEST_IMMS_ID, - test_fixed_time_taken, -) +from tests.utils_for_recordfowarder_tests.values_for_recordforwarder_tests import TEST_IMMS_ID from errors import MessageNotSuccessfulError -class Test_Splunk_logging(unittest.TestCase): - def setUp(self): - self.message_body_base = { - "row_id": "6543219", - "file_key": "flu_Vaccinations_v5_8HK48_20210730T12000000.csv", - "supplier": "EMIS", - "operation_requested": "operation_requested", - "fhir_json": {"resourceType": "Immunization"}, - } - - self.fixed_datetime = datetime(2024, 10, 29, 12, 0, 0) - - self.message_body_base_errors = { - "row_id": "6543219", - "file_key": "flu_Vaccinations_v5_8HK48_20210730T12000000.csv", - "supplier": "EMIS", - "operation_requested": "UPDATE", - "diagnostics": "Unable to obtain IMMS ID", - } - - self.expected_values = { - "function_name": "send_request_to_lambda", - "date_time": self.fixed_datetime.strftime("%Y-%m-%d %H:%M:%S"), - "status": "success", - "supplier": "EMIS", - "file_key": "flu_Vaccinations_v5_8HK48_20210730T12000000.csv", - "vaccine_type": "FLU", - "message_id": "6543219", - "action_flag": "action_flag", - "time_taken": 1.0, - } - - # Expected splunk log values when there is an error - self.expected_values_error = { - "event": { - "function_name": "send_request_to_lambda", - "date_time": self.fixed_datetime.strftime("%Y-%m-%d %H:%M:%S"), - "status": "Fail", - "supplier": "EMIS", - "file_key": "flu_Vaccinations_v5_8HK48_20210730T12000000.csv", - "vaccine_type": "FLU", - "message_id": "6543219", - "action_flag": "UPDATE", - "time_taken": "1.0s", - "status_code": 400, - "error": "Unable to obtain IMMS ID", - } - } - - def extract_log_json(self, log_entry): +class TestSplunkLogging(unittest.TestCase): + """Tests for Splunk logging""" + + test_fixed_time_taken = [ + 1000000.0, + 1000001.0, + 1000001.0, + 1000000.0, + 1000001.0, + 1000001.0, + 1000000.0, + 1000001.0, + 1000001.0, + ] + + fixed_datetime = datetime(2024, 10, 29, 12, 0, 0) + + example_diagnostics = "Unable to obtain IMMS ID" + + message_body_base = { + "row_id": "6543219", + "file_key": "flu_Vaccinations_v5_8HK48_20210730T12000000.csv", + "supplier": "EMIS", + "operation_requested": "UPDATE", + } + + log_json_base = { + "function_name": "send_request_to_lambda", + "date_time": fixed_datetime.strftime("%Y-%m-%d %H:%M:%S"), + "supplier": "EMIS", + "file_key": "flu_Vaccinations_v5_8HK48_20210730T12000000.csv", + "vaccine_type": "FLU", + "message_id": "6543219", + "action_flag": "action_flag", + } + + message_body_success = {**message_body_base, "fhir_json": {"resourceType": "Immunization"}} + + expected_log_json_success = {**log_json_base, "status": "success", "time_taken": 1.0} + + message_body_with_diagnostics = {**message_body_base, "diagnostics": example_diagnostics} + + expected_log_json_failure = { + **log_json_base, + "status": "Fail", + "time_taken": "1.0s", + "status_code": 400, + "error": example_diagnostics, + } + + def extract_log_json(self, log: str) -> dict: """Extracts JSON from log entry.""" + log_entry = log.output[0] json_start = log_entry.find("{") - json_str = log_entry[json_start:] + json_end = log_entry.find("}") + json_str = log_entry[json_start : json_end + 1] return json.loads(json_str) - @patch("send_request_to_lambda.send_create_request") - @patch("send_request_to_lambda.send_update_request") - @patch("send_request_to_lambda.send_delete_request") - @patch("log_structure.firehose_logger") - @patch("time.time") - @patch("log_structure.datetime") - def test_splunk_logging_successful_rows( - self, - mock_datetime, - mock_time, - mock_firehose_logger, - mock_send_delete_request, - mock_send_update_request, - mock_send_create_request, - ): + def test_splunk_logging_success(self): + """ + Test that for a successful row the log_json has all the expected keys and values, and the firehose logger + is called with the log_json. + """ + for operation in ["CREATE", "UPDATE", "DELETE"]: + with self.subTest(operation): + with ( + self.subTest(operation), + self.assertLogs(level="INFO") as log, + patch("log_structure.firehose_logger") as mock_firehose_logger, + patch("time.time", side_effect=self.test_fixed_time_taken), + patch("log_structure.datetime") as mock_datetime, + patch(f"send_request_to_lambda.send_{operation.lower()}_request", return_value=TEST_IMMS_ID), + ): + mock_datetime.now.return_value = self.fixed_datetime + + message_body = self.message_body_success.copy() + message_body["operation_requested"] = operation + result = send_request_to_lambda(message_body) + + self.assertEqual(result, TEST_IMMS_ID) - # mocking datetime and time_taken as fixed values - mock_datetime.now.return_value = self.fixed_datetime - mock_time.side_effect = test_fixed_time_taken - - # Mock return values for each operation - mock_send_create_request.return_value = TEST_IMMS_ID - mock_send_update_request.return_value = TEST_IMMS_ID - mock_send_delete_request.return_value = TEST_IMMS_ID - operations = [ - {"operation_requested": "CREATE"}, - {"operation_requested": "UPDATE"}, - {"operation_requested": "DELETE"}, - ] - - for op in operations: - with self.assertLogs(level="INFO") as log: - message_body = self.message_body_base.copy() - message_body["operation_requested"] = op["operation_requested"] - - result = send_request_to_lambda(message_body) - self.assertEqual(result, "imms_6543219") self.assertGreater(len(log.output), 0) - - log_json = self.extract_log_json(log.output[0]) - - expected_values = self.expected_values - expected_values["action_flag"] = op["operation_requested"] - - # Iterate over the expected values and assert each one - for key, expected in expected_values.items(): - self.assertEqual(log_json[key], expected) - + log_json = self.extract_log_json(log) + expected_log_json = deepcopy(self.expected_log_json_success) + expected_log_json["action_flag"] = operation + self.assertEqual(log_json, expected_log_json) self.assertIsInstance(log_json["time_taken"], float) - # Check firehose logging call mock_firehose_logger.forwarder_send_log.assert_called_once_with({"event": log_json}) mock_firehose_logger.forwarder_send_log.reset_mock() - @patch("log_structure.firehose_logger") - @patch("log_structure.logger") - @patch("time.time") - @patch("log_structure.datetime") - def test_splunk_logging_diagnostics_error(self, mock_datetime, mock_time, mock_logger, mock_firehose_logger): - # Message body with diagnostics to trigger an error, mocking datetime and time_taken as fixed values - mock_datetime.now.return_value = self.fixed_datetime - mock_time.side_effect = test_fixed_time_taken - message_body = self.message_body_base_errors - - # Exception raised in send_request_to_lambda - with self.assertRaises(MessageNotSuccessfulError) as context: - send_request_to_lambda(message_body) + def test_splunk_logging_failure_during_processing(self): + """ + Test that for a row which failed processing (and therefore has diagnostics in the message recevied from + kinesis), the log_json has all the expected keys and values, and the firehose logger is called with the + log_json. + """ + with ( + self.assertLogs(level="INFO") as log, + self.assertRaises(MessageNotSuccessfulError) as context, + patch("log_structure.firehose_logger") as mock_firehose_logger, + patch("time.time", side_effect=self.test_fixed_time_taken), + patch("log_structure.datetime") as mock_datetime, + ): + mock_datetime.now.return_value = self.fixed_datetime + send_request_to_lambda(self.message_body_with_diagnostics) - # Ensure the exception message is as expected self.assertEqual(str(context.exception), "Unable to obtain IMMS ID") - log_data = mock_logger.exception.call_args[0][0] - - self.assertIn("Unable to obtain IMMS ID", log_data) - - firehose_log_data = self.expected_values_error - mock_firehose_logger.forwarder_send_log.assert_called_once_with(firehose_log_data) - - @patch("send_request_to_lambda.send_create_request") - @patch("send_request_to_lambda.send_update_request") - @patch("send_request_to_lambda.send_delete_request") - @patch("send_request_to_lambda.forwarder_function_info") # Mock the decorator to simplify the test - @patch("log_structure.logger") # Patch the logger to verify error logs - def test_error_logging_in_send_request_to_lambda( - self, - mock_logger, - mock_forwarder_function_info, - mock_send_delete_request, - mock_send_update_request, - mock_send_create_request, - ): - - # Define message bodies for each operation to trigger errors - create_message_body = { - "operation_requested": "CREATE", - "file_key": "flu_Vaccinations_v5_8HK48_20210730T12000000.csv", - "supplier": "TestSupplier", - "fhir_json": {}, # Placeholder for any necessary data structure - "row_id": "12345", - } - - update_message_body = { - "operation_requested": "UPDATE", - "file_key": "flu_Vaccinations_v5_8HK48_20210730T12000000.csv", - "supplier": "TestSupplier", - "fhir_json": {}, # Placeholder for any necessary data structure - "row_id": "12345", - "imms_id": "67890", - "version": "1", - } - - delete_message_body = { - "operation_requested": "DELETE", - "file_key": "flu_Vaccinations_v5_8HK48_20210730T12000000.csv", - "supplier": "TestSupplier", - "fhir_json": {}, # Placeholder for any necessary data structure - "imms_id": "67890", - } - - # Set up each mock function to raise MessageNotSuccessfulError with custom error messages - mock_send_create_request.side_effect = MessageNotSuccessfulError("API Error: Unable to create resource") - mock_send_update_request.side_effect = MessageNotSuccessfulError("API Error: Unable to update resource") - mock_send_delete_request.side_effect = MessageNotSuccessfulError("API Error: Unable to delete resource") - - # Test the CREATE operation - with self.assertRaises(MessageNotSuccessfulError): - send_request_to_lambda(create_message_body) - - # Assert the logger recorded the error message for CREATE - mock_logger.exception.assert_called() # Check that the log was triggered - self.assertIn("API Error: Unable to create resource", str(mock_logger.exception.call_args)) # Verify message - - # Reset the mock logger for the next test case - mock_logger.exception.reset_mock() - - # Test the UPDATE operation - with self.assertRaises(MessageNotSuccessfulError): - send_request_to_lambda(update_message_body) - - # Assert the logger recorded the error message for UPDATE - mock_logger.exception.assert_called() - self.assertIn("API Error: Unable to update resource", str(mock_logger.exception.call_args)) - - # Reset the mock logger for the next test case - mock_logger.exception.reset_mock() - - # Test the DELETE operation - with self.assertRaises(MessageNotSuccessfulError): - send_request_to_lambda(delete_message_body) - - # Assert the logger recorded the error message for DELETE - mock_logger.exception.assert_called() - self.assertIn("API Error: Unable to delete resource", str(mock_logger.exception.call_args)) - - @patch("send_request_to_lambda.send_create_request") - @patch("send_request_to_lambda.send_update_request") - @patch("send_request_to_lambda.send_delete_request") - @patch("log_structure.logger") # Patch the logger to verify error logs - def test_error_logging_operation( - self, - mock_logger, - mock_send_delete_request, - mock_send_update_request, - mock_send_create_request, - ): - - create_message_body = { - "row_id": "555555", - "file_key": "flu_Vaccinations_v5_8HK48_20210730T12000000.csv", - "supplier": "EMIS", - "operation_requested": "CREATE", - "fhir_json": {"resourceType": "Immunization"}, - } - - update_message_body = { - "row_id": "7891011", - "file_key": "flu_Vaccinations_v5_8HK48_20210730T12000000.csv", - "supplier": "EMIS", - "operation_requested": "UPDATE", - "fhir_json": {"resourceType": "Immunization"}, - } - - delete_message_body = { - "row_id": "12131415", - "file_key": "flu_Vaccinations_v5_8HK48_20210730T12000000.csv", - "supplier": "EMIS", - "operation_requested": "DELETE", - "fhir_json": {"resourceType": "Immunization"}, - } - - # Set up each mock function to raise MessageNotSuccessfulError with custom error messages - mock_send_create_request.side_effect = MessageNotSuccessfulError("API Error: Unable to create resource") - mock_send_update_request.side_effect = MessageNotSuccessfulError("API Error: Unable to update resource") - mock_send_delete_request.side_effect = MessageNotSuccessfulError("API Error: Unable to delete resource") - - with self.assertRaises(MessageNotSuccessfulError): - send_request_to_lambda(create_message_body) - - mock_logger.exception.assert_called() - self.assertIn("API Error: Unable to create resource", str(mock_logger.exception.call_args)) - mock_logger.exception.reset_mock() - - # Test the UPDATE operation - with self.assertRaises(MessageNotSuccessfulError): - send_request_to_lambda(update_message_body) - - mock_logger.exception.assert_called() - self.assertIn("API Error: Unable to update resource", str(mock_logger.exception.call_args)) - mock_logger.exception.reset_mock() - - # Test the DELETE operation - with self.assertRaises(MessageNotSuccessfulError): - send_request_to_lambda(delete_message_body) - - mock_logger.exception.assert_called() - self.assertIn("API Error: Unable to delete resource", str(mock_logger.exception.call_args)) + self.assertGreater(len(log.output), 0) + log_json = self.extract_log_json(log) + expected_log_json = deepcopy(self.expected_log_json_failure) + expected_log_json["action_flag"] = "UPDATE" + self.assertEqual(log_json, expected_log_json) + + mock_firehose_logger.forwarder_send_log.assert_called_once_with({"event": log_json}) + mock_firehose_logger.forwarder_send_log.reset_mock() + + def test_splunk_logging_failure_during_forwarding(self): + """ + Test that for a row which failed processing (and therefore has diagnostics in the message recevied from + kinesis), the log_json has all the expected keys and values, and the firehose logger is called with the + log_json. + """ + for operation in ["create"]: + with self.subTest(operation): + with ( + self.assertLogs(level="INFO") as log, + self.assertRaises(MessageNotSuccessfulError) as context, + patch("log_structure.firehose_logger") as mock_firehose_logger, + patch("time.time", side_effect=self.test_fixed_time_taken), + patch("log_structure.datetime") as mock_datetime, + patch( + f"send_request_to_lambda.send_{operation}_request", + side_effect=MessageNotSuccessfulError(f"API Error: Unable to {operation} resource"), + ), + ): + mock_datetime.now.return_value = self.fixed_datetime + message_body = deepcopy(self.message_body_success) + message_body["operation_requested"] = operation.upper() + send_request_to_lambda(message_body) + + self.assertEqual(str(context.exception), f"API Error: Unable to {operation} resource") + self.assertGreater(len(log.output), 0) + log_json = self.extract_log_json(log) + expected_log_json = deepcopy(self.expected_log_json_failure) + expected_log_json["action_flag"] = operation.upper() + expected_log_json["error"] = f"API Error: Unable to {operation} resource" + self.assertEqual(log_json, expected_log_json) -if __name__ == "__main__": - unittest.main() + mock_firehose_logger.forwarder_send_log.assert_called_once_with({"event": log_json}) + mock_firehose_logger.forwarder_send_log.reset_mock() diff --git a/recordforwarder/tests/utils_for_recordfowarder_tests/utils_for_recordforwarder_tests.py b/recordforwarder/tests/utils_for_recordfowarder_tests/utils_for_recordforwarder_tests.py index 2154ff67..c759327e 100644 --- a/recordforwarder/tests/utils_for_recordfowarder_tests/utils_for_recordforwarder_tests.py +++ b/recordforwarder/tests/utils_for_recordfowarder_tests/utils_for_recordforwarder_tests.py @@ -1,7 +1,19 @@ +"""Utils for recordfowarder tests""" + import json +import base64 +from io import StringIO +from typing import Union + +def generate_kinesis_message(message: dict) -> str: + """Convert a dictionary to a kinesis message""" + kinesis_encoded_data = base64.b64encode(json.dumps(message).encode("utf-8")).decode("utf-8") + return {"Records": [{"kinesis": {"data": kinesis_encoded_data}}]} -def create_mock_operation_outcome(diagnostics: str, code: str = "duplicate") -> dict: + +def generate_mock_operation_outcome(diagnostics: str, code: str = "duplicate") -> dict: + """Generates an Operation Outcome, with the given diagnostics and code""" return { "resourceType": "OperationOutcome", "id": "an_imms_id", @@ -19,10 +31,30 @@ def create_mock_operation_outcome(diagnostics: str, code: str = "duplicate") -> } -def generate_payload(status_code: int, headers: dict = {}, body: dict = None): +def generate_payload(status_code: int, headers: Union[dict, None] = None, body: dict = None): + """ + Generates a payload with the given status code, headers and body + (body is converted to json string, and the key-value pair is omitted if there is no body) + """ return {"statusCode": status_code, **({"body": json.dumps(body)} if body is not None else {}), "headers": headers} +def generate_lambda_invocation_side_effect(mock_lambda_payloads): + """ + Takes a dictionary as input with key-value pairs in the format LAMBDA_TYPE: mock_response_payload, where + LAMBDA_TYPEs are CREATE, UPDATE, DELETE and SEARCH. + Returns a function which mocks the side effect of calling lambda_client.invoke on the relevant Imms FHIR API lambda. + """ + + def lambda_invocation_side_effect(FunctionName, *_args, **_kwargs): # pylint: disable=invalid-name + for key, value in mock_lambda_payloads.items(): + if FunctionName == f"mock_{key.lower()}_lambda_name": + response_payload = value + return {"Payload": StringIO(json.dumps(response_payload))} + + return lambda_invocation_side_effect + + response_body_id_and_version_not_found = { "resourceType": "Bundle", "type": "searchset", diff --git a/recordforwarder/tests/utils_for_recordfowarder_tests/values_for_recordforwarder_tests.py b/recordforwarder/tests/utils_for_recordfowarder_tests/values_for_recordforwarder_tests.py index c03670d0..238d1262 100644 --- a/recordforwarder/tests/utils_for_recordfowarder_tests/values_for_recordforwarder_tests.py +++ b/recordforwarder/tests/utils_for_recordfowarder_tests/values_for_recordforwarder_tests.py @@ -116,18 +116,6 @@ ], } -test_fixed_time_taken = [ - 1000000.0, - 1000001.0, - 1000001.0, - 1000000.0, - 1000001.0, - 1000001.0, - 1000000.0, - 1000001.0, - 1000001.0, -] - class Diagnostics: """Diagnostics messages""" From 3bf188e9b94f15cb557b9dba35baa89fc7b7376e Mon Sep 17 00:00:00 2001 From: AlexandraBenson Date: Mon, 4 Nov 2024 19:26:25 +0000 Subject: [PATCH 10/22] Tidy values for tests into classes --- .../tests/test_e2e_forwarding_lambda.py | 88 ++++++------- .../tests/test_forwarding_lambda.py | 6 +- .../tests/test_get_imms_id_and_version.py | 7 +- recordforwarder/tests/test_log_structure.py | 6 +- .../utils_for_recordforwarder_tests.py | 22 ---- .../values_for_recordforwarder_tests.py | 119 +++++++++++------- 6 files changed, 122 insertions(+), 126 deletions(-) diff --git a/recordforwarder/tests/test_e2e_forwarding_lambda.py b/recordforwarder/tests/test_e2e_forwarding_lambda.py index bfc1f6a4..7181573f 100644 --- a/recordforwarder/tests/test_e2e_forwarding_lambda.py +++ b/recordforwarder/tests/test_e2e_forwarding_lambda.py @@ -4,8 +4,6 @@ import unittest from unittest.mock import patch -import json -from io import StringIO import os import sys from boto3 import client as boto3_client @@ -18,19 +16,16 @@ from forwarding_lambda import forward_lambda_handler from tests.utils_for_recordfowarder_tests.values_for_recordforwarder_tests import ( - test_fhir_json, AWS_REGION, SOURCE_BUCKET_NAME, DESTINATION_BUCKET_NAME, - TEST_FILE_KEY, - TEST_ACK_FILE_KEY, - base_message_fields, lambda_success_headers, MOCK_ENVIRONMENT_DICT, + TestFile, + Message, + ResponseBody, ) from tests.utils_for_recordfowarder_tests.utils_for_recordforwarder_tests import ( - response_body_id_and_version_found, - response_body_id_and_version_not_found, generate_mock_operation_outcome, generate_payload, generate_kinesis_message, @@ -47,15 +42,13 @@ class TestForwardingLambdaE2E(unittest.TestCase): def setup_s3(self): """Helper to setup mock S3 buckets and upload test file""" - s3_client.create_bucket(Bucket=SOURCE_BUCKET_NAME, CreateBucketConfiguration={"LocationConstraint": AWS_REGION}) - s3_client.create_bucket( - Bucket=DESTINATION_BUCKET_NAME, CreateBucketConfiguration={"LocationConstraint": AWS_REGION} - ) - s3_client.put_object(Bucket=SOURCE_BUCKET_NAME, Key=TEST_FILE_KEY, Body="test_data") + for bucket_name in [SOURCE_BUCKET_NAME, DESTINATION_BUCKET_NAME]: + s3_client.create_bucket(Bucket=bucket_name, CreateBucketConfiguration={"LocationConstraint": AWS_REGION}) + s3_client.put_object(Bucket=SOURCE_BUCKET_NAME, Key=TestFile.FILE_KEY, Body="test_data") def check_ack_file(self, expected_content): """Helper to check the acknowledgment file content""" - ack_file_obj = s3_client.get_object(Bucket=DESTINATION_BUCKET_NAME, Key=TEST_ACK_FILE_KEY) + ack_file_obj = s3_client.get_object(Bucket=DESTINATION_BUCKET_NAME, Key=TestFile.ACK_FILE_KEY) ack_file_content = ack_file_obj["Body"].read().decode("utf-8") self.assertIn(expected_content, ack_file_content) @@ -70,36 +63,25 @@ def execute_test(self, message, expected_content, mock_lambda_payloads: dict): self.check_ack_file(expected_content) - def test_forward_lambda_e2e_update_failed_unable_to_get_id(self): - message = {**base_message_fields, "fhir_json": test_fhir_json, "operation_requested": "UPDATE"} - mock_lambda_payloads = { - "SEARCH": generate_payload(status_code=200, body=response_body_id_and_version_not_found), - } - self.execute_test(message, "Fatal", mock_lambda_payloads) - def test_forward_lambda_e2e_create_success(self): - message = {**base_message_fields, "fhir_json": test_fhir_json, "operation_requested": "CREATE"} mock_lambda_payloads = {"CREATE": generate_payload(status_code=201, headers=lambda_success_headers)} - self.execute_test(message, "OK", mock_lambda_payloads=mock_lambda_payloads) + self.execute_test(Message.create_message, "OK", mock_lambda_payloads=mock_lambda_payloads) def test_forward_lambda_e2e_create_duplicate(self): - message = {**base_message_fields, "fhir_json": test_fhir_json, "operation_requested": "CREATE"} mock_diagnostics = ( "The provided identifier: https://supplierABC/identifiers/vacc#test-identifier1 is duplicated" ) mock_body = generate_mock_operation_outcome(diagnostics=mock_diagnostics, code="duplicate") mock_lambda_payloads = {"CREATE": generate_payload(status_code=422, body=mock_body)} - self.execute_test(message, "Fatal Error", mock_lambda_payloads=mock_lambda_payloads) + self.execute_test(Message.create_message, "Fatal Error", mock_lambda_payloads=mock_lambda_payloads) def test_forward_lambda_e2e_create_failed(self): - message = {**base_message_fields, "fhir_json": test_fhir_json, "operation_requested": "CREATE"} mock_diagnostics = "the provided event ID is either missing or not in the expected format." mock_body = generate_mock_operation_outcome(diagnostics=mock_diagnostics, code="duplicate") mock_lambda_payloads = {"CREATE": generate_payload(status_code=400, body=mock_body)} - self.execute_test(message, "Fatal Error", mock_lambda_payloads=mock_lambda_payloads) + self.execute_test(Message.create_message, "Fatal Error", mock_lambda_payloads=mock_lambda_payloads) def test_forward_lambda_e2e_create_multi_line_diagnostics(self): - message = {**base_message_fields, "fhir_json": test_fhir_json, "operation_requested": "CREATE"} mock_diagnostics = """This a string of diagnostics which spans multiple lines and has some carriage returns\n\nand random space""" @@ -108,55 +90,57 @@ def test_forward_lambda_e2e_create_multi_line_diagnostics(self): expected_single_line_diagnostics = ( "This a string of diagnostics which spans multiple lines and has some carriage returns and random space" ) - self.execute_test(message, expected_single_line_diagnostics, mock_lambda_payloads) - - @patch("utils_for_record_forwarder.lambda_client.invoke") - def test_forward_lambda_e2e_none_request(self, mock_api): - self.setup_s3() - message = {**base_message_fields, "diagnostics": "Unsupported file type received as an attachment"} - - forward_lambda_handler(generate_kinesis_message(message), None) - - self.check_ack_file("Fatal Error") - mock_api.create_immunization.assert_not_called() + self.execute_test(Message.create_message, expected_single_line_diagnostics, mock_lambda_payloads) def test_forward_lambda_e2e_update_success(self): - message = {**base_message_fields, "fhir_json": test_fhir_json, "operation_requested": "UPDATE"} mock_lambda_payloads = { "UPDATE": generate_payload(200), - "SEARCH": generate_payload(200, body=response_body_id_and_version_found), + "SEARCH": generate_payload(200, body=ResponseBody.id_and_version_found), } - self.execute_test(message, "OK", mock_lambda_payloads) + self.execute_test(Message.update_message, "OK", mock_lambda_payloads) + + def test_forward_lambda_e2e_update_failed_unable_to_get_id(self): + mock_lambda_payloads = { + "SEARCH": generate_payload(status_code=200, body=ResponseBody.id_and_version_not_found), + } + self.execute_test(Message.update_message, "Fatal", mock_lambda_payloads) def test_forward_lambda_e2e_update_failed(self): - message = {**base_message_fields, "fhir_json": test_fhir_json, "operation_requested": "UPDATE"} mock_diagnstics = "the provided event ID is either missing or not in the expected format." mock_lambda_payloads = { "UPDATE": generate_payload(400, body=generate_mock_operation_outcome(mock_diagnstics)), - "SEARCH": generate_payload(200, body=response_body_id_and_version_found), + "SEARCH": generate_payload(200, body=ResponseBody.id_and_version_found), } - self.execute_test(message, "Fatal Error", mock_lambda_payloads) + self.execute_test(Message.update_message, "Fatal Error", mock_lambda_payloads) def test_forward_lambda_e2e_delete_success(self): - message = {**base_message_fields, "fhir_json": test_fhir_json, "operation_requested": "DELETE"} mock_lambda_payloads = { "DELETE": generate_payload(204), - "SEARCH": generate_payload(200, body=response_body_id_and_version_found), + "SEARCH": generate_payload(200, body=ResponseBody.id_and_version_found), } - self.execute_test(message, "OK", mock_lambda_payloads) + self.execute_test(Message.delete_message, "OK", mock_lambda_payloads) def test_forward_lambda_e2e_delete_failed(self): - message = {**base_message_fields, "fhir_json": test_fhir_json, "operation_requested": "DELETE"} mock_diagnstics = "the provided event ID is either missing or not in the expected format." mock_lambda_payloads = { "UPDATE": generate_payload(404, body=generate_mock_operation_outcome(mock_diagnstics, code="not-found")), - "SEARCH": generate_payload(200, body=response_body_id_and_version_not_found), + "SEARCH": generate_payload(200, body=ResponseBody.id_and_version_not_found), } - self.execute_test(message, "Fatal Error", mock_lambda_payloads) + self.execute_test(Message.delete_message, "Fatal Error", mock_lambda_payloads) + + @patch("utils_for_record_forwarder.lambda_client.invoke") + def test_forward_lambda_e2e_none_request(self, mock_api): + self.setup_s3() + message = {**Message.base_message_fields, "diagnostics": "Unsupported file type received as an attachment"} + + forward_lambda_handler(generate_kinesis_message(message), None) + + self.check_ack_file("Fatal Error") + mock_api.create_immunization.assert_not_called() def test_forward_lambda_e2e_no_permissions(self): self.setup_s3() - message = {**base_message_fields, "diagnostics": "No permissions for operation"} + message = {**Message.base_message_fields, "diagnostics": "No permissions for operation"} forward_lambda_handler(generate_kinesis_message(message), None) diff --git a/recordforwarder/tests/test_forwarding_lambda.py b/recordforwarder/tests/test_forwarding_lambda.py index faa4e130..0defb5b8 100644 --- a/recordforwarder/tests/test_forwarding_lambda.py +++ b/recordforwarder/tests/test_forwarding_lambda.py @@ -20,10 +20,10 @@ lambda_success_headers, MOCK_ENVIRONMENT_DICT, AWS_REGION, + ResponseBody, ) from tests.utils_for_recordfowarder_tests.utils_for_recordforwarder_tests import ( generate_mock_operation_outcome, - response_body_id_and_version_found, generate_payload, generate_kinesis_message, generate_lambda_invocation_side_effect, @@ -178,7 +178,7 @@ def test_forward_request_to_api_update_failure(self, mock_s3_client): mock_lambda_payloads = { "UPDATE": generate_payload(status_code=422, body=generate_mock_operation_outcome(diagnostics)), - "SEARCH": generate_payload(status_code=200, body=response_body_id_and_version_found), + "SEARCH": generate_payload(status_code=200, body=ResponseBody.id_and_version_found), } with patch("update_ack_file.create_ack_data") as mock_create_ack_data, patch( "utils_for_record_forwarder.lambda_client.invoke", @@ -224,7 +224,7 @@ def test_forward_request_to_api_delete_success(self, mock_s3_client): mock_lambda_payloads = { "DELETE": generate_payload(status_code=204), - "SEARCH": generate_payload(status_code=200, body=response_body_id_and_version_found), + "SEARCH": generate_payload(status_code=200, body=ResponseBody.id_and_version_found), } with patch("update_ack_file.create_ack_data") as mock_create_ack_data, patch( "utils_for_record_forwarder.lambda_client.invoke", diff --git a/recordforwarder/tests/test_get_imms_id_and_version.py b/recordforwarder/tests/test_get_imms_id_and_version.py index f9a50839..8e2155ad 100644 --- a/recordforwarder/tests/test_get_imms_id_and_version.py +++ b/recordforwarder/tests/test_get_imms_id_and_version.py @@ -9,10 +9,9 @@ from errors import IdNotFoundError from tests.utils_for_recordfowarder_tests.utils_for_recordforwarder_tests import ( generate_payload, - response_body_id_and_version_found, - response_body_id_and_version_not_found, generate_mock_operation_outcome, ) +from tests.utils_for_recordfowarder_tests.values_for_recordforwarder_tests import ResponseBody fhir_json_with_identifier_value_and_system = {"identifier": [{"value": "a_value", "system": "a_system"}]} @@ -28,7 +27,7 @@ class TestGetImmsIdAndVersion(unittest.TestCase): def test_success(self): """Test that imms_id and version are correctly identified from a successful search lambda response.""" mock_lambda_response_payload = { - "Payload": StringIO(json.dumps(generate_payload(200, body=response_body_id_and_version_found))) + "Payload": StringIO(json.dumps(generate_payload(200, body=ResponseBody.id_and_version_found))) } with patch("clients.lambda_client.invoke", return_value=mock_lambda_response_payload): imms_id, version = get_imms_id_and_version(fhir_json_with_identifier_value_and_system) @@ -39,7 +38,7 @@ def test_success(self): def test_failure_due_to_empty_search_lambda_return(self): """Test that an IdNotFoundError is raised for a successful search lambda response which contains no entries.""" mock_lambda_response_payload = { - "Payload": StringIO(json.dumps(generate_payload(200, body=response_body_id_and_version_not_found))) + "Payload": StringIO(json.dumps(generate_payload(200, body=ResponseBody.id_and_version_not_found))) } with patch("clients.lambda_client.invoke", return_value=mock_lambda_response_payload): with self.assertRaises(IdNotFoundError): diff --git a/recordforwarder/tests/test_log_structure.py b/recordforwarder/tests/test_log_structure.py index bd94c130..c1949b3d 100644 --- a/recordforwarder/tests/test_log_structure.py +++ b/recordforwarder/tests/test_log_structure.py @@ -6,7 +6,7 @@ from copy import deepcopy from datetime import datetime from send_request_to_lambda import send_request_to_lambda -from tests.utils_for_recordfowarder_tests.values_for_recordforwarder_tests import TEST_IMMS_ID +from tests.utils_for_recordfowarder_tests.values_for_recordforwarder_tests import Message from errors import MessageNotSuccessfulError @@ -81,7 +81,7 @@ def test_splunk_logging_success(self): patch("log_structure.firehose_logger") as mock_firehose_logger, patch("time.time", side_effect=self.test_fixed_time_taken), patch("log_structure.datetime") as mock_datetime, - patch(f"send_request_to_lambda.send_{operation.lower()}_request", return_value=TEST_IMMS_ID), + patch(f"send_request_to_lambda.send_{operation.lower()}_request", return_value=Message.IMMS_ID), ): mock_datetime.now.return_value = self.fixed_datetime @@ -89,7 +89,7 @@ def test_splunk_logging_success(self): message_body["operation_requested"] = operation result = send_request_to_lambda(message_body) - self.assertEqual(result, TEST_IMMS_ID) + self.assertEqual(result, Message.IMMS_ID) self.assertGreater(len(log.output), 0) log_json = self.extract_log_json(log) diff --git a/recordforwarder/tests/utils_for_recordfowarder_tests/utils_for_recordforwarder_tests.py b/recordforwarder/tests/utils_for_recordfowarder_tests/utils_for_recordforwarder_tests.py index c759327e..a809215a 100644 --- a/recordforwarder/tests/utils_for_recordfowarder_tests/utils_for_recordforwarder_tests.py +++ b/recordforwarder/tests/utils_for_recordfowarder_tests/utils_for_recordforwarder_tests.py @@ -53,25 +53,3 @@ def lambda_invocation_side_effect(FunctionName, *_args, **_kwargs): # pylint: d return {"Payload": StringIO(json.dumps(response_payload))} return lambda_invocation_side_effect - - -response_body_id_and_version_not_found = { - "resourceType": "Bundle", - "type": "searchset", - "link": [ - { - "relation": "self", - "url": "https://internal-dev.api.service.nhs.uk/immunisation-fhir-api/Immunization?" - + "immunization.identifier=None&_elements=None", - } - ], - "entry": [], - "total": 0, -} - -response_body_id_and_version_found = { - "resourceType": "Bundle", - "type": "searchset", - "entry": [{"resource": {"id": "277befd9-574e-47fe-a6ee-189858af3bb0", "meta": {"versionId": 2}}}], - "total": 1, -} diff --git a/recordforwarder/tests/utils_for_recordfowarder_tests/values_for_recordforwarder_tests.py b/recordforwarder/tests/utils_for_recordfowarder_tests/values_for_recordforwarder_tests.py index 238d1262..717b6a5e 100644 --- a/recordforwarder/tests/utils_for_recordfowarder_tests/values_for_recordforwarder_tests.py +++ b/recordforwarder/tests/utils_for_recordfowarder_tests/values_for_recordforwarder_tests.py @@ -1,22 +1,5 @@ """Values for use in recordforwarder tests""" -SOURCE_BUCKET_NAME = "immunisation-batch-internal-dev-data-sources" -DESTINATION_BUCKET_NAME = "immunisation-batch-internal-dev-data-destinations" - -AWS_REGION = "eu-west-2" - -TEST_VACCINE_TYPE = "flu" -TEST_SUPPLIER = "EMIS" -TEST_ODS_CODE = "8HK48" -TEST_ROW_ID = "123456" -TEST_IMMS_ID = "imms_6543219" - -TEST_FILE_KEY = f"{TEST_VACCINE_TYPE}_Vaccinations_v5_{TEST_ODS_CODE}_20210730T12000000.csv" -TEST_ACK_FILE_KEY = f"forwardedFile/{TEST_VACCINE_TYPE}_Vaccinations_v5_{TEST_ODS_CODE}_20210730T12000000_BusAck.csv" - -base_message_fields = {"row_id": TEST_ROW_ID, "file_key": TEST_FILE_KEY, "supplier": TEST_SUPPLIER} -lambda_success_headers = {"Location": "https://example.com/immunization/test_id"} - MOCK_ENVIRONMENT_DICT = { "SOURCE_BUCKET_NAME": "immunisation-batch-internal-dev-data-sources", "ACK_BUCKET_NAME": "immunisation-batch-internal-dev-data-destinations", @@ -29,6 +12,33 @@ "SEARCH_LAMBDA_NAME": "mock_search_lambda_name", } +SOURCE_BUCKET_NAME = "immunisation-batch-internal-dev-data-sources" +DESTINATION_BUCKET_NAME = "immunisation-batch-internal-dev-data-destinations" + +AWS_REGION = "eu-west-2" + + +class TestFile: + """Class containing a test file, it's constituent variables and its corresponding ack file""" + + VACCINE_TYPE = "flu" + SUPPLIER = "EMIS" + ODS_CODE = "8HK48" + + FILE_KEY = f"{VACCINE_TYPE}_Vaccinations_v5_{ODS_CODE}_20210730T12000000.csv" + ACK_FILE_KEY = f"forwardedFile/{FILE_KEY.split('.')[0]}_BusAck.csv" + + +class Urls: + """Urls for use within FHIR Immunization Resource json""" + + SNOMED = "http://snomed.info/sct" + NHS_NUMBER = "https://fhir.nhs.uk/Id/nhs-number" + VACCINATION_PROCEDURE = "https://fhir.hl7.org.uk/StructureDefinition/Extension-UKCore-VaccinationProcedure" + RAVS = "https://www.ravs.england.nhs.uk/" + ODS = "https://fhir.nhs.uk/Id/ods-organization-code" + + test_fhir_json = { "resourceType": "Immunization", "contained": [ @@ -36,7 +46,7 @@ { "resourceType": "Patient", "id": "Pat1", - "identifier": [{"system": "https://fhir.nhs.uk/Id/nhs-number", "value": "1234567890"}], + "identifier": [{"system": Urls.NHS_NUMBER, "value": "1234567890"}], "name": [{"family": "SMITH", "given": ["JOHN"]}], "gender": "male", "birthDate": "2000-01-01", @@ -45,11 +55,11 @@ ], "extension": [ { - "url": "https://fhir.hl7.org.uk/StructureDefinition/Extension-UKCore-VaccinationProcedure", + "url": Urls.VACCINATION_PROCEDURE, "valueCodeableConcept": { "coding": [ { - "system": "http://snomed.info/sct", + "system": Urls.SNOMED, "code": "123456789", "display": "Administration of vaccine product containing only Dummy antigen (procedure)", } @@ -57,12 +67,12 @@ }, } ], - "identifier": [{"system": "https://www.ravs.england.nhs.uk/", "value": "0001_TEST_v1_RUN_1_ABCD-123_Dose_seq_01"}], + "identifier": [{"system": Urls.RAVS, "value": "0001_TEST_v1_RUN_1_ABCD-123_Dose_seq_01"}], "status": "completed", "vaccineCode": { "coding": [ { - "system": "http://snomed.info/sct", + "system": Urls.SNOMED, "code": "987654321", "display": "Dummy vaccine powder and solvent for solution (product)", } @@ -73,50 +83,51 @@ "recorded": "2024-01-01T00:00:00+00:00", "primarySource": True, "manufacturer": {"display": "Dummy Pharma"}, - "location": {"identifier": {"value": "ABCDE", "system": "https://fhir.nhs.uk/Id/ods-organization-code"}}, + "location": {"identifier": {"value": "ABCDE", "system": Urls.ODS}}, "lotNumber": "DUMMYLOT", "expirationDate": "2024-12-31", "site": { "coding": [ - { - "system": "http://snomed.info/sct", - "code": "999999999", - "display": "Right upper arm structure (body structure)", - } + {"system": Urls.SNOMED, "code": "999999999", "display": "Right upper arm structure (body structure)"} ] }, "route": { - "coding": [ - {"system": "http://snomed.info/sct", "code": "111111111", "display": "Subcutaneous route (qualifier value)"} - ] - }, - "doseQuantity": { - "system": "http://snomed.info/sct", - "value": 0.5, - "unit": "Milliliter (qualifier value)", - "code": "123456789", + "coding": [{"system": Urls.SNOMED, "code": "111111111", "display": "Subcutaneous route (qualifier value)"}] }, + "doseQuantity": {"system": Urls.SNOMED, "value": 0.5, "unit": "Milliliter (qualifier value)", "code": "123456789"}, "performer": [ {"actor": {"reference": "#Pract1"}}, { "actor": { "type": "Organization", - "identifier": {"system": "https://fhir.nhs.uk/Id/ods-organization-code", "value": "DUMMYORG"}, + "identifier": {"system": Urls.ODS, "value": "DUMMYORG"}, } }, ], - "reasonCode": [{"coding": [{"system": "http://snomed.info/sct", "code": "dummy"}]}], + "reasonCode": [{"coding": [{"system": Urls.SNOMED, "code": "dummy"}]}], "protocolApplied": [ { - "targetDisease": [ - {"coding": [{"system": "http://snomed.info/sct", "code": "123456789", "display": "Dummy disease"}]} - ], + "targetDisease": [{"coding": [{"system": Urls.SNOMED, "code": "123456789", "display": "Dummy disease"}]}], "doseNumberPositiveInt": 1, } ], } +class Message: + """Class containing example kinesis messages""" + + ROW_ID = "123456" + IMMS_ID = "277befd9-574e-47fe-a6ee-189858af3bb0" + base_message_fields = {"row_id": ROW_ID, "file_key": TestFile.FILE_KEY, "supplier": TestFile.SUPPLIER} + create_message = {**base_message_fields, "fhir_json": test_fhir_json, "operation_requested": "CREATE"} + update_message = {**base_message_fields, "fhir_json": test_fhir_json, "operation_requested": "UPDATE"} + delete_message = {**base_message_fields, "fhir_json": test_fhir_json, "operation_requested": "DELETE"} + + +lambda_success_headers = {"Location": "https://example.com/immunization/test_id"} + + class Diagnostics: """Diagnostics messages""" @@ -126,3 +137,27 @@ class Diagnostics: UNABLE_TO_OBTAIN_IMMS_ID = "Unable to obtain imms event id" UNABLE_TO_OBTAIN_VERSION = "Unable to obtain current imms event version" INVALID_CONVERSION = "Unable to convert row to FHIR Immunization Resource JSON format" + + +class ResponseBody: + """Examples of response body for get_imms_id_and_version""" + id_and_version_not_found = { + "resourceType": "Bundle", + "type": "searchset", + "link": [ + { + "relation": "self", + "url": "https://internal-dev.api.service.nhs.uk/immunisation-fhir-api/Immunization?" + + "immunization.identifier=None&_elements=None", + } + ], + "entry": [], + "total": 0, + } + + id_and_version_found = { + "resourceType": "Bundle", + "type": "searchset", + "entry": [{"resource": {"id": Message.IMMS_ID, "meta": {"versionId": 2}}}], + "total": 1, + } From 06de3c3672a799fb17b15f73dd023390affbd3e1 Mon Sep 17 00:00:00 2001 From: AlexandraBenson Date: Tue, 5 Nov 2024 18:54:55 +0000 Subject: [PATCH 11/22] Fix incorrect log time format and improve logging tests --- recordforwarder/src/log_structure.py | 6 +- recordforwarder/tests/test_log_structure.py | 168 +++++++------------- 2 files changed, 64 insertions(+), 110 deletions(-) diff --git a/recordforwarder/src/log_structure.py b/recordforwarder/src/log_structure.py index 39e3e6a0..7c810dea 100644 --- a/recordforwarder/src/log_structure.py +++ b/recordforwarder/src/log_structure.py @@ -42,7 +42,7 @@ def wrapper(*args, **kwargs): try: result = func(*args, **kwargs) end_time = time.time() - log_data["time_taken"] = round(end_time - start_time, 5) + log_data["time_taken"] = f"{round(end_time - start_time, 5)}s" logger.info(json.dumps(log_data)) firehose_log["event"] = log_data firehose_logger.forwarder_send_log(firehose_log) @@ -53,8 +53,8 @@ def wrapper(*args, **kwargs): log_data["error"] = str(e) log_data["status"] = "Fail" log_data.pop("message", None) - end = time.time() - log_data["time_taken"] = f"{round(end - start_time, 5)}s" + end_time = time.time() + log_data["time_taken"] = f"{round(end_time - start_time, 5)}s" logger.exception(json.dumps(log_data)) firehose_log["event"] = log_data firehose_logger.forwarder_send_log(firehose_log) diff --git a/recordforwarder/tests/test_log_structure.py b/recordforwarder/tests/test_log_structure.py index c1949b3d..2aa344e9 100644 --- a/recordforwarder/tests/test_log_structure.py +++ b/recordforwarder/tests/test_log_structure.py @@ -5,60 +5,30 @@ import json from copy import deepcopy from datetime import datetime +from contextlib import contextmanager, ExitStack from send_request_to_lambda import send_request_to_lambda from tests.utils_for_recordfowarder_tests.values_for_recordforwarder_tests import Message from errors import MessageNotSuccessfulError +FIXED_DATETIME = datetime(2024, 10, 30, 12, 0, 0) + class TestSplunkLogging(unittest.TestCase): """Tests for Splunk logging""" - test_fixed_time_taken = [ - 1000000.0, - 1000001.0, - 1000001.0, - 1000000.0, - 1000001.0, - 1000001.0, - 1000000.0, - 1000001.0, - 1000001.0, - ] - - fixed_datetime = datetime(2024, 10, 29, 12, 0, 0) - - example_diagnostics = "Unable to obtain IMMS ID" - - message_body_base = { - "row_id": "6543219", - "file_key": "flu_Vaccinations_v5_8HK48_20210730T12000000.csv", - "supplier": "EMIS", - "operation_requested": "UPDATE", - } - log_json_base = { "function_name": "send_request_to_lambda", - "date_time": fixed_datetime.strftime("%Y-%m-%d %H:%M:%S"), + "date_time": FIXED_DATETIME.strftime("%Y-%m-%d %H:%M:%S"), "supplier": "EMIS", "file_key": "flu_Vaccinations_v5_8HK48_20210730T12000000.csv", "vaccine_type": "FLU", - "message_id": "6543219", - "action_flag": "action_flag", + "message_id": "123456", + "action_flag": "each test replaces this with the relevant action flag", } - message_body_success = {**message_body_base, "fhir_json": {"resourceType": "Immunization"}} - - expected_log_json_success = {**log_json_base, "status": "success", "time_taken": 1.0} + expected_log_json_success = {**log_json_base, "status": "success", "time_taken": "1.0s"} - message_body_with_diagnostics = {**message_body_base, "diagnostics": example_diagnostics} - - expected_log_json_failure = { - **log_json_base, - "status": "Fail", - "time_taken": "1.0s", - "status_code": 400, - "error": example_diagnostics, - } + expected_log_json_failure = {**log_json_base, "status": "Fail", "time_taken": "1.0s", "status_code": 400} def extract_log_json(self, log: str) -> dict: """Extracts JSON from log entry.""" @@ -68,98 +38,82 @@ def extract_log_json(self, log: str) -> dict: json_str = log_entry[json_start : json_end + 1] return json.loads(json_str) - def test_splunk_logging_success(self): + def make_log_assertions(self, log, mock_firehose_logger, operation: str, expected_error=None): + """Assert that the log_json is as expected, and that the firehose logger was called with the log_json""" + # Extract log_json + self.assertGreater(len(log.output), 0) + log_json = self.extract_log_json(log) + + # Prepare expected_log_json + expected_log_json = ( + deepcopy(self.expected_log_json_success) if not expected_error else deepcopy(self.expected_log_json_failure) + ) + expected_log_json["action_flag"] = operation.upper() + expected_log_json.update({"error": expected_error} if expected_error else {}) + + self.assertEqual(log_json, expected_log_json) + + mock_firehose_logger.forwarder_send_log.assert_called_once_with({"event": log_json}) + mock_firehose_logger.forwarder_send_log.reset_mock() + + @contextmanager + def common_contexts_for_splunk_logging_tests(self): """ - Test that for a successful row the log_json has all the expected keys and values, and the firehose logger - is called with the log_json. + A context manager which applies common patching for the tests in the TestSplunkLogging class. + Yields mock_firehose_logger and logs (where logs is a list of the captured log entries). """ + with ExitStack() as stack: + stack.enter_context(patch("time.time", side_effect=(1000000.0, 1000001.0, 1000003.0))) # (start, end, ???) + stack.enter_context(patch("log_structure.datetime")) + stack.enter_context(patch("log_structure.datetime.now", return_value=FIXED_DATETIME)) + mock_firehose_logger = stack.enter_context(patch("log_structure.firehose_logger")) + logs = stack.enter_context(self.assertLogs(level="INFO")) + yield mock_firehose_logger, logs + + def test_splunk_logging_success(self): + """Tests successful rows""" for operation in ["CREATE", "UPDATE", "DELETE"]: with self.subTest(operation): with ( - self.subTest(operation), - self.assertLogs(level="INFO") as log, - patch("log_structure.firehose_logger") as mock_firehose_logger, - patch("time.time", side_effect=self.test_fixed_time_taken), - patch("log_structure.datetime") as mock_datetime, + self.common_contexts_for_splunk_logging_tests() as (mock_firehose_logger, logs), patch(f"send_request_to_lambda.send_{operation.lower()}_request", return_value=Message.IMMS_ID), ): - mock_datetime.now.return_value = self.fixed_datetime - - message_body = self.message_body_success.copy() - message_body["operation_requested"] = operation + message_body = {**Message.base_message_fields, "operation_requested": operation, "fhir_json": {}} result = send_request_to_lambda(message_body) self.assertEqual(result, Message.IMMS_ID) - - self.assertGreater(len(log.output), 0) - log_json = self.extract_log_json(log) - expected_log_json = deepcopy(self.expected_log_json_success) - expected_log_json["action_flag"] = operation - self.assertEqual(log_json, expected_log_json) - self.assertIsInstance(log_json["time_taken"], float) - - mock_firehose_logger.forwarder_send_log.assert_called_once_with({"event": log_json}) - mock_firehose_logger.forwarder_send_log.reset_mock() + self.make_log_assertions(logs, mock_firehose_logger, operation) def test_splunk_logging_failure_during_processing(self): - """ - Test that for a row which failed processing (and therefore has diagnostics in the message recevied from - kinesis), the log_json has all the expected keys and values, and the firehose logger is called with the - log_json. - """ + """Tests a row which failed processing (and therefore has diagnostics in the message recevied from kinesis)""" + diagnostics = "Unable to obtain IMMS ID" + operation = "UPDATE" with ( - self.assertLogs(level="INFO") as log, + self.common_contexts_for_splunk_logging_tests() as (mock_firehose_logger, logs), self.assertRaises(MessageNotSuccessfulError) as context, - patch("log_structure.firehose_logger") as mock_firehose_logger, - patch("time.time", side_effect=self.test_fixed_time_taken), - patch("log_structure.datetime") as mock_datetime, ): - mock_datetime.now.return_value = self.fixed_datetime - send_request_to_lambda(self.message_body_with_diagnostics) - - self.assertEqual(str(context.exception), "Unable to obtain IMMS ID") - - self.assertGreater(len(log.output), 0) - log_json = self.extract_log_json(log) - expected_log_json = deepcopy(self.expected_log_json_failure) - expected_log_json["action_flag"] = "UPDATE" - self.assertEqual(log_json, expected_log_json) + message_body = {**Message.base_message_fields, "operation_requested": operation, "diagnostics": diagnostics} + send_request_to_lambda(message_body) - mock_firehose_logger.forwarder_send_log.assert_called_once_with({"event": log_json}) - mock_firehose_logger.forwarder_send_log.reset_mock() + self.assertEqual(str(context.exception), diagnostics) + self.make_log_assertions(logs, mock_firehose_logger, operation, expected_error=diagnostics) def test_splunk_logging_failure_during_forwarding(self): - """ - Test that for a row which failed processing (and therefore has diagnostics in the message recevied from - kinesis), the log_json has all the expected keys and values, and the firehose logger is called with the - log_json. - """ - for operation in ["create"]: + """Tests rows which fail during forwarding""" + + for operation in ["CREATE", "UPDATE", "DELETE"]: + error_message = f"API Error: Unable to {operation.lower()} resource" with self.subTest(operation): with ( - self.assertLogs(level="INFO") as log, + self.common_contexts_for_splunk_logging_tests() as (mock_firehose_logger, logs), self.assertRaises(MessageNotSuccessfulError) as context, - patch("log_structure.firehose_logger") as mock_firehose_logger, - patch("time.time", side_effect=self.test_fixed_time_taken), - patch("log_structure.datetime") as mock_datetime, patch( - f"send_request_to_lambda.send_{operation}_request", - side_effect=MessageNotSuccessfulError(f"API Error: Unable to {operation} resource"), + f"send_request_to_lambda.send_{operation.lower()}_request", + side_effect=MessageNotSuccessfulError(error_message), ), ): - mock_datetime.now.return_value = self.fixed_datetime - message_body = deepcopy(self.message_body_success) - message_body["operation_requested"] = operation.upper() + message_body = {**Message.base_message_fields, "operation_requested": operation, "fhir_json": {}} send_request_to_lambda(message_body) - self.assertEqual(str(context.exception), f"API Error: Unable to {operation} resource") - - self.assertGreater(len(log.output), 0) - log_json = self.extract_log_json(log) - expected_log_json = deepcopy(self.expected_log_json_failure) - expected_log_json["action_flag"] = operation.upper() - expected_log_json["error"] = f"API Error: Unable to {operation} resource" - self.assertEqual(log_json, expected_log_json) - - mock_firehose_logger.forwarder_send_log.assert_called_once_with({"event": log_json}) - mock_firehose_logger.forwarder_send_log.reset_mock() + self.assertEqual(str(context.exception), error_message) + self.make_log_assertions(logs, mock_firehose_logger, operation, expected_error=error_message) From 3000613b37e738d5706e1f142f043e381e7db2ca Mon Sep 17 00:00:00 2001 From: AlexandraBenson Date: Tue, 5 Nov 2024 18:55:41 +0000 Subject: [PATCH 12/22] Tidy forwarding lambda tests --- .../tests/test_e2e_forwarding_lambda.py | 29 ++++---- .../tests/test_forwarding_lambda.py | 66 +++++++++---------- .../utils_for_recordforwarder_tests.py | 6 +- 3 files changed, 47 insertions(+), 54 deletions(-) diff --git a/recordforwarder/tests/test_e2e_forwarding_lambda.py b/recordforwarder/tests/test_e2e_forwarding_lambda.py index 7181573f..b9b03f04 100644 --- a/recordforwarder/tests/test_e2e_forwarding_lambda.py +++ b/recordforwarder/tests/test_e2e_forwarding_lambda.py @@ -40,12 +40,19 @@ @patch.dict("os.environ", MOCK_ENVIRONMENT_DICT) class TestForwardingLambdaE2E(unittest.TestCase): - def setup_s3(self): - """Helper to setup mock S3 buckets and upload test file""" + def setUp(self) -> None: + """Set up the SOURCE and DESTINATION buckets, and upload the TestFile to the SOURCE bucket""" for bucket_name in [SOURCE_BUCKET_NAME, DESTINATION_BUCKET_NAME]: s3_client.create_bucket(Bucket=bucket_name, CreateBucketConfiguration={"LocationConstraint": AWS_REGION}) s3_client.put_object(Bucket=SOURCE_BUCKET_NAME, Key=TestFile.FILE_KEY, Body="test_data") + def tearDown(self) -> None: + """Deletes the buckets and their contents""" + for bucket_name in [SOURCE_BUCKET_NAME, DESTINATION_BUCKET_NAME]: + for obj in s3_client.list_objects_v2(Bucket=bucket_name).get("Contents", []): + s3_client.delete_object(Bucket=bucket_name, Key=obj["Key"]) + s3_client.delete_bucket(Bucket=bucket_name) + def check_ack_file(self, expected_content): """Helper to check the acknowledgment file content""" ack_file_obj = s3_client.get_object(Bucket=DESTINATION_BUCKET_NAME, Key=TestFile.ACK_FILE_KEY) @@ -53,13 +60,11 @@ def check_ack_file(self, expected_content): self.assertIn(expected_content, ack_file_content) def execute_test(self, message, expected_content, mock_lambda_payloads: dict): - self.setup_s3() - with patch( "utils_for_record_forwarder.lambda_client.invoke", side_effect=generate_lambda_invocation_side_effect(mock_lambda_payloads), ): - forward_lambda_handler(event=generate_kinesis_message(message), _=None) + forward_lambda_handler(generate_kinesis_message(message), None) self.check_ack_file(expected_content) @@ -130,21 +135,15 @@ def test_forward_lambda_e2e_delete_failed(self): @patch("utils_for_record_forwarder.lambda_client.invoke") def test_forward_lambda_e2e_none_request(self, mock_api): - self.setup_s3() message = {**Message.base_message_fields, "diagnostics": "Unsupported file type received as an attachment"} - - forward_lambda_handler(generate_kinesis_message(message), None) - - self.check_ack_file("Fatal Error") + mock_lambda_payloads = {} + self.execute_test(message, "Fatal Error", mock_lambda_payloads) mock_api.create_immunization.assert_not_called() def test_forward_lambda_e2e_no_permissions(self): - self.setup_s3() message = {**Message.base_message_fields, "diagnostics": "No permissions for operation"} - - forward_lambda_handler(generate_kinesis_message(message), None) - - self.check_ack_file("Fatal Error") + mock_lambda_payloads = {} + self.execute_test(message, "Fatal Error", mock_lambda_payloads) if __name__ == "__main__": diff --git a/recordforwarder/tests/test_forwarding_lambda.py b/recordforwarder/tests/test_forwarding_lambda.py index 0defb5b8..ff8c0e99 100644 --- a/recordforwarder/tests/test_forwarding_lambda.py +++ b/recordforwarder/tests/test_forwarding_lambda.py @@ -29,7 +29,6 @@ generate_lambda_invocation_side_effect, ) from forwarding_lambda import forward_lambda_handler, forward_request_to_lambda -from utils_for_record_forwarder import get_environment from update_ack_file import create_ack_data @@ -40,21 +39,6 @@ @patch.dict("os.environ", MOCK_ENVIRONMENT_DICT) class TestForwardingLambda(unittest.TestCase): - @patch("utils_for_record_forwarder.os.getenv") - def test_get_environment_internal_dev(self, mock_getenv): - mock_getenv.return_value = "internal-dev" - self.assertEqual(get_environment(), "internal-dev") - - @patch("utils_for_record_forwarder.os.getenv") - def test_get_environment_prod(self, mock_getenv): - mock_getenv.return_value = "prod" - self.assertEqual(get_environment(), "prod") - - @patch("utils_for_record_forwarder.os.getenv") - def test_get_environment_default(self, mock_getenv): - mock_getenv.return_value = None - self.assertEqual(get_environment(), "internal-dev") - def test_create_ack_data(self): created_at_formatted_string = "20241015T18504900" row_id = "test_file_id#1" @@ -93,7 +77,7 @@ def test_create_ack_data(self): "MESSAGE_DELIVERY": False, } - # Test cas tuples are structured as (test_name, successful_api_response, diagnostics, imms_id, expected output) + # Test case tuples are structured as (test_name, successful_api_response, diagnostics, imms_id, expected output) test_cases = [ ("ack data for success", True, None, "test_imms_id", success_ack_data), ("ack data for failure", False, "Some diagnostics", "", failure_ack_data), @@ -106,12 +90,7 @@ def test_create_ack_data(self): expected_output, ) - @patch("update_ack_file.s3_client") - def test_forward_request_to_api_new_success(self, mock_s3_client): - # Mock LastModified as a datetime object - mock_s3_client.head_object.return_value = {"LastModified": datetime(2024, 8, 21, 10, 15, 30)} - # Simulate the case where the ack file does not exist - mock_s3_client.get_object.side_effect = ClientError({"Error": {"Code": "404"}}, "HeadObject") + def test_forward_request_to_api_new_success(self): message = { "row_id": "test_1", @@ -123,10 +102,18 @@ def test_forward_request_to_api_new_success(self, mock_s3_client): # Mock the create_ack_data method and lambda invocation repsonse payloads mock_lambda_payloads = {"CREATE": generate_payload(status_code=201, headers=lambda_success_headers)} - with patch("update_ack_file.create_ack_data") as mock_create_ack_data, patch( - "utils_for_record_forwarder.lambda_client.invoke", - side_effect=generate_lambda_invocation_side_effect(mock_lambda_payloads), + with ( + patch("update_ack_file.create_ack_data") as mock_create_ack_data, + patch( + "utils_for_record_forwarder.lambda_client.invoke", + side_effect=generate_lambda_invocation_side_effect(mock_lambda_payloads), + ), + patch("update_ack_file.s3_client") as mock_s3_client, ): + mock_s3_client.head_object.return_value = {"LastModified": datetime(2024, 8, 21, 10, 15, 30)} + # Simulate the case where the ack file does not exist + mock_s3_client.get_object.side_effect = ClientError({"Error": {"Code": "404"}}, "HeadObject") + forward_request_to_lambda(message) mock_create_ack_data.assert_called_with("20240821T10153000", "test_1", True, None, "test_id") @@ -150,9 +137,12 @@ def test_forward_request_to_api_new_duplicate(self, mock_s3_client): mock_lambda_payloads = { "CREATE": generate_payload(status_code=422, body=generate_mock_operation_outcome(diagnostics)) } - with patch("update_ack_file.create_ack_data") as mock_create_ack_data, patch( - "utils_for_record_forwarder.lambda_client.invoke", - side_effect=generate_lambda_invocation_side_effect(mock_lambda_payloads), + with ( + patch("update_ack_file.create_ack_data") as mock_create_ack_data, + patch( + "utils_for_record_forwarder.lambda_client.invoke", + side_effect=generate_lambda_invocation_side_effect(mock_lambda_payloads), + ), ): forward_request_to_lambda(message) @@ -180,9 +170,12 @@ def test_forward_request_to_api_update_failure(self, mock_s3_client): "UPDATE": generate_payload(status_code=422, body=generate_mock_operation_outcome(diagnostics)), "SEARCH": generate_payload(status_code=200, body=ResponseBody.id_and_version_found), } - with patch("update_ack_file.create_ack_data") as mock_create_ack_data, patch( - "utils_for_record_forwarder.lambda_client.invoke", - side_effect=generate_lambda_invocation_side_effect(mock_lambda_payloads), + with ( + patch("update_ack_file.create_ack_data") as mock_create_ack_data, + patch( + "utils_for_record_forwarder.lambda_client.invoke", + side_effect=generate_lambda_invocation_side_effect(mock_lambda_payloads), + ), ): forward_request_to_lambda(message) @@ -226,9 +219,12 @@ def test_forward_request_to_api_delete_success(self, mock_s3_client): "DELETE": generate_payload(status_code=204), "SEARCH": generate_payload(status_code=200, body=ResponseBody.id_and_version_found), } - with patch("update_ack_file.create_ack_data") as mock_create_ack_data, patch( - "utils_for_record_forwarder.lambda_client.invoke", - side_effect=generate_lambda_invocation_side_effect(mock_lambda_payloads), + with ( + patch("update_ack_file.create_ack_data") as mock_create_ack_data, + patch( + "utils_for_record_forwarder.lambda_client.invoke", + side_effect=generate_lambda_invocation_side_effect(mock_lambda_payloads), + ), ): forward_request_to_lambda(message) diff --git a/recordforwarder/tests/utils_for_recordfowarder_tests/utils_for_recordforwarder_tests.py b/recordforwarder/tests/utils_for_recordfowarder_tests/utils_for_recordforwarder_tests.py index a809215a..71ec2fc9 100644 --- a/recordforwarder/tests/utils_for_recordfowarder_tests/utils_for_recordforwarder_tests.py +++ b/recordforwarder/tests/utils_for_recordfowarder_tests/utils_for_recordforwarder_tests.py @@ -47,9 +47,7 @@ def generate_lambda_invocation_side_effect(mock_lambda_payloads): """ def lambda_invocation_side_effect(FunctionName, *_args, **_kwargs): # pylint: disable=invalid-name - for key, value in mock_lambda_payloads.items(): - if FunctionName == f"mock_{key.lower()}_lambda_name": - response_payload = value - return {"Payload": StringIO(json.dumps(response_payload))} + lambda_type = FunctionName.split("_")[1] # Tests mock FunctionNames as mock_lambdatype_lambda_name + return {"Payload": StringIO(json.dumps(mock_lambda_payloads[lambda_type.upper()]))} return lambda_invocation_side_effect From f3702306359f7685b7648b4b13331599ba154cfd Mon Sep 17 00:00:00 2001 From: AlexandraBenson Date: Tue, 5 Nov 2024 19:09:11 +0000 Subject: [PATCH 13/22] Fix linting issue --- recordforwarder/tests/test_log_structure.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/recordforwarder/tests/test_log_structure.py b/recordforwarder/tests/test_log_structure.py index 2aa344e9..2e419498 100644 --- a/recordforwarder/tests/test_log_structure.py +++ b/recordforwarder/tests/test_log_structure.py @@ -75,7 +75,7 @@ def test_splunk_logging_success(self): for operation in ["CREATE", "UPDATE", "DELETE"]: with self.subTest(operation): with ( - self.common_contexts_for_splunk_logging_tests() as (mock_firehose_logger, logs), + self.common_contexts_for_splunk_logging_tests() as (mock_firehose_logger, logs), # noqa: E999 patch(f"send_request_to_lambda.send_{operation.lower()}_request", return_value=Message.IMMS_ID), ): message_body = {**Message.base_message_fields, "operation_requested": operation, "fhir_json": {}} @@ -89,7 +89,7 @@ def test_splunk_logging_failure_during_processing(self): diagnostics = "Unable to obtain IMMS ID" operation = "UPDATE" with ( - self.common_contexts_for_splunk_logging_tests() as (mock_firehose_logger, logs), + self.common_contexts_for_splunk_logging_tests() as (mock_firehose_logger, logs), # noqa: E999 self.assertRaises(MessageNotSuccessfulError) as context, ): message_body = {**Message.base_message_fields, "operation_requested": operation, "diagnostics": diagnostics} @@ -105,7 +105,7 @@ def test_splunk_logging_failure_during_forwarding(self): error_message = f"API Error: Unable to {operation.lower()} resource" with self.subTest(operation): with ( - self.common_contexts_for_splunk_logging_tests() as (mock_firehose_logger, logs), + self.common_contexts_for_splunk_logging_tests() as (mock_firehose_logger, logs), # noqa: E999 self.assertRaises(MessageNotSuccessfulError) as context, patch( f"send_request_to_lambda.send_{operation.lower()}_request", From e72fe1178ad8b512745a738df956ab12e7df2410 Mon Sep 17 00:00:00 2001 From: AlexandraBenson Date: Tue, 5 Nov 2024 19:11:58 +0000 Subject: [PATCH 14/22] Ignore linting issue that is inconsistent with some formatters --- recordforwarder/tests/test_log_structure.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/recordforwarder/tests/test_log_structure.py b/recordforwarder/tests/test_log_structure.py index 2e419498..6372e3a8 100644 --- a/recordforwarder/tests/test_log_structure.py +++ b/recordforwarder/tests/test_log_structure.py @@ -35,7 +35,7 @@ def extract_log_json(self, log: str) -> dict: log_entry = log.output[0] json_start = log_entry.find("{") json_end = log_entry.find("}") - json_str = log_entry[json_start : json_end + 1] + json_str = log_entry[json_start : json_end + 1] # noqa: E203 return json.loads(json_str) def make_log_assertions(self, log, mock_firehose_logger, operation: str, expected_error=None): From 3950770ae2caa3400b42a04ef503d02c489dc6cc Mon Sep 17 00:00:00 2001 From: AlexandraBenson Date: Wed, 6 Nov 2024 13:25:48 +0000 Subject: [PATCH 15/22] Refactor logging --- recordforwarder/src/clients.py | 1 + recordforwarder/src/forwarding_lambda.py | 2 +- recordforwarder/src/log_firehose.py | 25 ++++------- recordforwarder/src/log_structure.py | 42 +++++++------------ .../src/utils_for_record_forwarder.py | 12 ++---- 5 files changed, 29 insertions(+), 53 deletions(-) diff --git a/recordforwarder/src/clients.py b/recordforwarder/src/clients.py index 3f9b9b8f..d949c115 100644 --- a/recordforwarder/src/clients.py +++ b/recordforwarder/src/clients.py @@ -7,3 +7,4 @@ s3_client = boto3_client("s3", region_name=REGION_NAME) kinesis_client = boto3_client("kinesis", region_name=REGION_NAME) lambda_client = boto3_client("lambda", region_name=REGION_NAME) +firehose_client = boto3_client("firehose", region_name=REGION_NAME) diff --git a/recordforwarder/src/forwarding_lambda.py b/recordforwarder/src/forwarding_lambda.py index 594e189e..470ee380 100644 --- a/recordforwarder/src/forwarding_lambda.py +++ b/recordforwarder/src/forwarding_lambda.py @@ -15,7 +15,7 @@ def forward_request_to_lambda(message_body): """Forwards the request to the Imms API (where possible) and updates the ack file with the outcome""" file_key = message_body.get("file_key") row_id = message_body.get("row_id") - logger.info("BEGINNIING FORWARDING MESSAGE: ID %s", row_id) + logger.info("BEGINNING FORWARDING MESSAGE: ID %s", row_id) try: imms_id = send_request_to_lambda(message_body) update_ack_file(file_key, row_id, successful_api_response=True, diagnostics=None, imms_id=imms_id) diff --git a/recordforwarder/src/log_firehose.py b/recordforwarder/src/log_firehose.py index d275dc08..3d08e1cc 100644 --- a/recordforwarder/src/log_firehose.py +++ b/recordforwarder/src/log_firehose.py @@ -1,31 +1,22 @@ -import boto3 import logging import json import os -from botocore.config import Config +from clients import firehose_client logging.basicConfig() logger = logging.getLogger() logger.setLevel("INFO") +DELIVERY_STREAM_NAME = os.getenv("SPLUNK_FIREHOSE_NAME") + class Forwarder_FirehoseLogger: - def __init__( - self, - stream_name: str = os.getenv("SPLUNK_FIREHOSE_NAME"), - boto_client=boto3.client("firehose", config=Config(region_name="eu-west-2")), - ): - self.firehose_client = boto_client - self.delivery_stream_name = stream_name def forwarder_send_log(self, log_message): - log_to_splunk = log_message - encoded_log_data = json.dumps(log_to_splunk).encode("utf-8") try: - response = self.firehose_client.put_record( - DeliveryStreamName=self.delivery_stream_name, - Record={"Data": encoded_log_data}, + response = firehose_client.put_record( + DeliveryStreamName=DELIVERY_STREAM_NAME, Record={"Data": json.dumps(log_message).encode("utf-8")} ) - logger.info(f"Log sent to Firehose: {response}") - except Exception as e: - logger.exception(f"Error sending log to Firehose: {e}") + logger.info("Log sent to Firehose: %s", response) + except Exception as error: # pylint: disable=broad-exception-caught + logger.exception("Error sending log to Firehose: %s", error) diff --git a/recordforwarder/src/log_structure.py b/recordforwarder/src/log_structure.py index 7c810dea..ee9e82c8 100644 --- a/recordforwarder/src/log_structure.py +++ b/recordforwarder/src/log_structure.py @@ -4,7 +4,7 @@ from datetime import datetime from functools import wraps from log_firehose import Forwarder_FirehoseLogger -from utils_for_record_forwarder import extract_file_key_elements +from utils_for_record_forwarder import extract_vaccine_type_from_file_key logging.basicConfig() @@ -19,45 +19,35 @@ def forwarder_function_info(func): def wrapper(*args, **kwargs): event = args[0] if args else {} - supplier = event.get("supplier") - operation_requested = event.get("operation_requested") - message_id = event.get("row_id") - file_key = event.get("file_key") - vaccine_type = extract_file_key_elements(file_key).get("vaccine_type") log_data = { "function_name": func.__name__, "date_time": str(datetime.now()), - "status": "success", - "supplier": supplier, - "file_key": file_key, - "action_flag": operation_requested, - "vaccine_type": vaccine_type, - "message_id": message_id, - "time_taken": None, + "supplier": event.get("supplier"), + "file_key": event.get("file_key"), + "action_flag": event.get("operation_requested"), + "vaccine_type": extract_vaccine_type_from_file_key(event.get("file_key")), + "message_id": event.get("row_id"), } + def send_logs(start_time, log_data, is_success): + end_time = time.time() + log_data["time_taken"] = f"{round(end_time - start_time, 5)}s" + log_data["status"] = "success" if is_success else "Fail" + logging_function = logger.info if is_success else logger.exception + logging_function(json.dumps(log_data)) + firehose_logger.forwarder_send_log({"event": log_data}) + start_time = time.time() - firehose_log = dict() try: result = func(*args, **kwargs) - end_time = time.time() - log_data["time_taken"] = f"{round(end_time - start_time, 5)}s" - logger.info(json.dumps(log_data)) - firehose_log["event"] = log_data - firehose_logger.forwarder_send_log(firehose_log) + send_logs(start_time, log_data, is_success=True) return result except Exception as e: log_data["status_code"] = 400 log_data["error"] = str(e) - log_data["status"] = "Fail" - log_data.pop("message", None) - end_time = time.time() - log_data["time_taken"] = f"{round(end_time - start_time, 5)}s" - logger.exception(json.dumps(log_data)) - firehose_log["event"] = log_data - firehose_logger.forwarder_send_log(firehose_log) + send_logs(start_time, log_data, is_success=False) raise return wrapper diff --git a/recordforwarder/src/utils_for_record_forwarder.py b/recordforwarder/src/utils_for_record_forwarder.py index 337e69ad..22c14b4f 100644 --- a/recordforwarder/src/utils_for_record_forwarder.py +++ b/recordforwarder/src/utils_for_record_forwarder.py @@ -13,15 +13,9 @@ def get_environment() -> str: return _env if _env in ["internal-dev", "int", "ref", "sandbox", "prod"] else "internal-dev" -def extract_file_key_elements(file_key: str) -> dict: - """ - Returns a dictionary containing each of the elements which can be extracted from the file key. - All elements are converted to upper case.\n - """ - file_key = file_key.upper() - file_key_parts_without_extension = file_key.split(".")[0].split("_") - file_key_elements = {"vaccine_type": file_key_parts_without_extension[0]} - return file_key_elements +def extract_vaccine_type_from_file_key(file_key: str) -> dict: + """Returns the vaccine in upper case""" + return file_key.split("_")[0].upper() def invoke_lambda(lambda_name: str, payload: dict) -> tuple[int, dict, str]: From 5a77866e49152113d91adcf6f46850fb1a6f1c17 Mon Sep 17 00:00:00 2001 From: AlexandraBenson Date: Wed, 6 Nov 2024 17:46:03 +0000 Subject: [PATCH 16/22] Add common contexts function to test_forwarding_lambda --- recordforwarder/src/send_request_to_lambda.py | 47 ++-- .../tests/test_forwarding_lambda.py | 211 +++++++----------- .../values_for_recordforwarder_tests.py | 25 ++- 3 files changed, 120 insertions(+), 163 deletions(-) diff --git a/recordforwarder/src/send_request_to_lambda.py b/recordforwarder/src/send_request_to_lambda.py index 52c8db8d..d73b9bd2 100644 --- a/recordforwarder/src/send_request_to_lambda.py +++ b/recordforwarder/src/send_request_to_lambda.py @@ -8,17 +8,24 @@ from log_structure import forwarder_function_info +def generate_base_request_headers(supplier: str) -> dict: + """Generate request headers with SupplierSystem and BatchSupplierSystem populated.""" + return {"SupplierSystem": IMMS_BATCH_APP_NAME, "BatchSupplierSystem": supplier} + + def send_create_request(fhir_json: dict, supplier: str) -> str: """Sends the create request and handles the response. Returns the imms_id.""" # Send create request - headers = {"SupplierSystem": IMMS_BATCH_APP_NAME, "BatchSupplierSystem": supplier} - payload = {"headers": headers, "body": fhir_json} - status_code, body, headers = invoke_lambda(os.getenv("CREATE_LAMBDA_NAME"), payload) - if status_code != 201: - raise MessageNotSuccessfulError(get_operation_outcome_diagnostics(body)) + request_headers = generate_base_request_headers(supplier) + request_payload = {"headers": request_headers, "body": fhir_json} + response_status_code, response_body, response_headers = invoke_lambda( + os.getenv("CREATE_LAMBDA_NAME"), request_payload + ) + if response_status_code != 201: + raise MessageNotSuccessfulError(get_operation_outcome_diagnostics(response_body)) # Return imms id (default to None if unable to find the id) - return headers.get("Location", "").split("/")[-1] or None + return response_headers.get("Location", "").split("/")[-1] or None def send_update_request(fhir_json: dict, supplier: str) -> str: @@ -29,17 +36,17 @@ def send_update_request(fhir_json: dict, supplier: str) -> str: except IdNotFoundError as error: raise MessageNotSuccessfulError(error) from error if not imms_id: - raise MessageNotSuccessfulError("Unable to obtain Imms ID") + raise MessageNotSuccessfulError("Unable to obtain Imms id") if not version: raise MessageNotSuccessfulError("Unable to obtain Imms version") # Send update request fhir_json["id"] = imms_id - headers = {"SupplierSystem": IMMS_BATCH_APP_NAME, "BatchSupplierSystem": supplier, "E-Tag": version} - payload = {"headers": headers, "body": fhir_json, "pathParameters": {"id": imms_id}} - status_code, body, _ = invoke_lambda(os.getenv("UPDATE_LAMBDA_NAME"), payload) - if status_code != 200: - raise MessageNotSuccessfulError(get_operation_outcome_diagnostics(body)) + request_headers = {**generate_base_request_headers(supplier), "E-Tag": version} + request_payload = {"headers": request_headers, "body": fhir_json, "pathParameters": {"id": imms_id}} + response_status_code, response_body, _ = invoke_lambda(os.getenv("UPDATE_LAMBDA_NAME"), request_payload) + if response_status_code != 200: + raise MessageNotSuccessfulError(get_operation_outcome_diagnostics(response_body)) return imms_id @@ -55,22 +62,22 @@ def send_delete_request(fhir_json: dict, supplier: str) -> str: raise MessageNotSuccessfulError("Unable to obtain Imms ID") # Send delete request - headers = {"SupplierSystem": IMMS_BATCH_APP_NAME, "BatchSupplierSystem": supplier} - payload = {"headers": headers, "body": fhir_json, "pathParameters": {"id": imms_id}} - status_code, body, _ = invoke_lambda(os.getenv("DELETE_LAMBDA_NAME"), payload) - if status_code != 204: - raise MessageNotSuccessfulError(get_operation_outcome_diagnostics(body)) + request_headers = generate_base_request_headers(supplier) + request_payload = {"headers": request_headers, "body": fhir_json, "pathParameters": {"id": imms_id}} + response_status_code, response_body, _ = invoke_lambda(os.getenv("DELETE_LAMBDA_NAME"), request_payload) + if response_status_code != 204: + raise MessageNotSuccessfulError(get_operation_outcome_diagnostics(response_body)) return imms_id -def get_operation_outcome_diagnostics(body: dict) -> str: +def get_operation_outcome_diagnostics(response_body: dict) -> str: """ Returns the diagnostics from the API response. If the diagnostics can't be found in the API response, - returns a default diagnostics string + returns a default diagnostics string. """ try: - return body.get("issue")[0].get("diagnostics") + return response_body.get("issue")[0].get("diagnostics") except (AttributeError, IndexError): return "Unable to obtain diagnostics from API response" diff --git a/recordforwarder/tests/test_forwarding_lambda.py b/recordforwarder/tests/test_forwarding_lambda.py index ff8c0e99..fc5e88f2 100644 --- a/recordforwarder/tests/test_forwarding_lambda.py +++ b/recordforwarder/tests/test_forwarding_lambda.py @@ -3,10 +3,13 @@ """Tests for forwarding lambda""" import unittest -from unittest.mock import patch +from unittest.mock import patch, MagicMock import os import sys +from copy import deepcopy +from typing import Generator from datetime import datetime +from contextlib import contextmanager, ExitStack from moto import mock_s3 from boto3 import client as boto3_client from botocore.exceptions import ClientError @@ -21,6 +24,7 @@ MOCK_ENVIRONMENT_DICT, AWS_REGION, ResponseBody, + Message, ) from tests.utils_for_recordfowarder_tests.utils_for_recordforwarder_tests import ( generate_mock_operation_outcome, @@ -39,6 +43,45 @@ @patch.dict("os.environ", MOCK_ENVIRONMENT_DICT) class TestForwardingLambda(unittest.TestCase): + @contextmanager + def common_contexts_for_forwarding_lambda_tests( + self, mock_lambda_payloads=None + ) -> Generator[MagicMock, None, None]: + """ + A context manager which applies common patching for the tests in the TestForwardingLambda class. + Yields mock_firehose_logger and logs (where logs is a list of the captured log entries). + """ + with ExitStack() as stack: + stack.enter_context(patch("update_ack_file.s3_client")), # pylint: disable=expression-not-assigned + + stack.enter_context( + patch( + "update_ack_file.s3_client.head_object", + return_value={"LastModified": datetime(2024, 8, 21, 10, 15, 30)}, + ) + ) + + # Simulate the case where the ack file does not exist + stack.enter_context( + patch( + "update_ack_file.s3_client.get_object", + side_effect=ClientError({"Error": {"Code": "404"}}, "HeadObject"), + ), + ) + + if mock_lambda_payloads: + # Mock lambda.invoke with a different payload for each different lambda + stack.enter_context( + patch( + "utils_for_record_forwarder.lambda_client.invoke", + side_effect=generate_lambda_invocation_side_effect(mock_lambda_payloads), + ) + ) + + mock_create_ack_data = stack.enter_context(patch("update_ack_file.create_ack_data")) + + yield mock_create_ack_data + def test_create_ack_data(self): created_at_formatted_string = "20241015T18504900" row_id = "test_file_id#1" @@ -91,178 +134,82 @@ def test_create_ack_data(self): ) def test_forward_request_to_api_new_success(self): - - message = { - "row_id": "test_1", - "file_key": "file.csv", - "supplier": "Test_supplier", - "operation_requested": "CREATE", - "fhir_json": {"Name": "test"}, - } - - # Mock the create_ack_data method and lambda invocation repsonse payloads mock_lambda_payloads = {"CREATE": generate_payload(status_code=201, headers=lambda_success_headers)} - with ( - patch("update_ack_file.create_ack_data") as mock_create_ack_data, - patch( - "utils_for_record_forwarder.lambda_client.invoke", - side_effect=generate_lambda_invocation_side_effect(mock_lambda_payloads), - ), - patch("update_ack_file.s3_client") as mock_s3_client, - ): - mock_s3_client.head_object.return_value = {"LastModified": datetime(2024, 8, 21, 10, 15, 30)} - # Simulate the case where the ack file does not exist - mock_s3_client.get_object.side_effect = ClientError({"Error": {"Code": "404"}}, "HeadObject") - - forward_request_to_lambda(message) + with self.common_contexts_for_forwarding_lambda_tests(mock_lambda_payloads) as mock_create_ack_data: + forward_request_to_lambda(deepcopy(Message.create_message)) - mock_create_ack_data.assert_called_with("20240821T10153000", "test_1", True, None, "test_id") + # pylint: disable=no-member + mock_create_ack_data.assert_called_with("20240821T10153000", Message.ROW_ID, True, None, "test_id") - @patch("update_ack_file.s3_client") - def test_forward_request_to_api_new_duplicate(self, mock_s3_client): - # Mock LastModified as a datetime object - mock_s3_client.head_object.return_value = {"LastModified": datetime(2024, 8, 21, 10, 15, 30)} + def test_forward_request_to_api_new_duplicate(self): diagnostics = "The provided identifier: https://supplierABC/identifiers/vacc#test-identifier1 is duplicated" - # Simulate the case where the ack file does not exist - mock_s3_client.get_object.side_effect = ClientError({"Error": {"Code": "404"}}, "HeadObject") - - message = { - "row_id": "test_2", - "file_key": "file.csv", - "supplier": "Test_supplier", - "operation_requested": "CREATE", - "fhir_json": {"identifier": [{"system": "test_system", "value": "test_value"}]}, - } mock_lambda_payloads = { "CREATE": generate_payload(status_code=422, body=generate_mock_operation_outcome(diagnostics)) } - with ( - patch("update_ack_file.create_ack_data") as mock_create_ack_data, - patch( - "utils_for_record_forwarder.lambda_client.invoke", - side_effect=generate_lambda_invocation_side_effect(mock_lambda_payloads), - ), - ): - forward_request_to_lambda(message) + with self.common_contexts_for_forwarding_lambda_tests(mock_lambda_payloads) as mock_create_ack_data: + forward_request_to_lambda(deepcopy(Message.create_message)) - mock_create_ack_data.assert_called_with("20240821T10153000", "test_2", False, diagnostics, None) + # pylint: disable=no-member + mock_create_ack_data.assert_called_with("20240821T10153000", Message.ROW_ID, False, diagnostics, None) + + def test_forward_request_to_api_update_failure(self): - @patch("update_ack_file.s3_client") - def test_forward_request_to_api_update_failure(self, mock_s3_client): - # Mock LastModified as a datetime object - mock_s3_client.head_object.return_value = {"LastModified": datetime(2024, 8, 21, 10, 15, 30)} diagnostics = ( "Validation errors: The provided immunization id:test_id doesn't match with the content of the request body" ) - # Simulate the case where the ack file does not exist - mock_s3_client.get_object.side_effect = ClientError({"Error": {"Code": "404"}}, "HeadObject") - - message = { - "row_id": "test_3", - "file_key": "file.csv", - "supplier": "Test_supplier", - "operation_requested": "UPDATE", - "fhir_json": {"identifier": [{"system": "test_system", "value": "test_value"}]}, - } mock_lambda_payloads = { "UPDATE": generate_payload(status_code=422, body=generate_mock_operation_outcome(diagnostics)), "SEARCH": generate_payload(status_code=200, body=ResponseBody.id_and_version_found), } + with self.common_contexts_for_forwarding_lambda_tests(mock_lambda_payloads) as mock_create_ack_data: + forward_request_to_lambda(deepcopy(Message.update_message)) + + # pylint: disable=no-member + mock_create_ack_data.assert_called_with("20240821T10153000", Message.ROW_ID, False, diagnostics, None) + + def test_forward_request_to_api_update_failure_imms_id_none(self): with ( - patch("update_ack_file.create_ack_data") as mock_create_ack_data, - patch( - "utils_for_record_forwarder.lambda_client.invoke", - side_effect=generate_lambda_invocation_side_effect(mock_lambda_payloads), - ), + self.common_contexts_for_forwarding_lambda_tests() as mock_create_ack_data, + patch("utils_for_record_forwarder.lambda_client.invoke") as mock_lambda_client, ): - forward_request_to_lambda(message) - - mock_create_ack_data.assert_called_with("20240821T10153000", "test_3", False, diagnostics, None) - - @patch("utils_for_record_forwarder.lambda_client.invoke") - @patch("update_ack_file.s3_client") - def test_forward_request_to_api_update_failure_imms_id_none(self, mock_s3_client, mock_lambda_client): - # Mock LastModified as a datetime object - mock_s3_client.head_object.return_value = {"LastModified": datetime(2024, 8, 21, 10, 15, 30)} - mock_s3_client.get_object.side_effect = ClientError({"Error": {"Code": "404"}}, "HeadObject") - - with patch("update_ack_file.create_ack_data") as mock_create_ack_data: - message_body = { - "row_id": "test_4", - "file_key": "file.csv", - "supplier": "Test_supplier", - "diagnostics": "Unable to obtain imms_id", - } - forward_request_to_lambda(message_body) - mock_create_ack_data.assert_called_with( - "20240821T10153000", "test_4", False, "Unable to obtain imms_id", None - ) - mock_lambda_client.assert_not_called() - - @patch("update_ack_file.s3_client") - def test_forward_request_to_api_delete_success(self, mock_s3_client): - # Mock LastModified as a datetime object - mock_s3_client.head_object.return_value = {"LastModified": datetime(2024, 8, 21, 10, 15, 30)} - # Simulate the case where the ack file does not exist - mock_s3_client.get_object.side_effect = ClientError({"Error": {"Code": "404"}}, "HeadObject") - - message = { - "row_id": "test_6", - "file_key": "file.csv", - "operation_requested": "DELETE", - "fhir_json": {"identifier": [{"system": "test_system", "value": "test_value"}]}, - } + forward_request_to_lambda(Message.diagnostics_message) + + # pylint: disable=no-member + mock_create_ack_data.assert_called_with("20240821T10153000", Message.ROW_ID, False, Message.DIAGNOSTICS, None) + mock_lambda_client.assert_not_called() + + def test_forward_request_to_api_delete_success(self): mock_lambda_payloads = { "DELETE": generate_payload(status_code=204), "SEARCH": generate_payload(status_code=200, body=ResponseBody.id_and_version_found), } - with ( - patch("update_ack_file.create_ack_data") as mock_create_ack_data, - patch( - "utils_for_record_forwarder.lambda_client.invoke", - side_effect=generate_lambda_invocation_side_effect(mock_lambda_payloads), - ), - ): - forward_request_to_lambda(message) + with self.common_contexts_for_forwarding_lambda_tests(mock_lambda_payloads) as mock_create_ack_data: + forward_request_to_lambda(deepcopy(Message.delete_message)) + # pylint: disable=no-member mock_create_ack_data.assert_called_with( - "20240821T10153000", "test_6", True, None, "277befd9-574e-47fe-a6ee-189858af3bb0" + "20240821T10153000", Message.ROW_ID, True, None, "277befd9-574e-47fe-a6ee-189858af3bb0" ) @patch("forwarding_lambda.forward_request_to_lambda") def test_forward_lambda_handler(self, mock_forward_request_to_api): - message_body = { - "row_id": "test_7", - "fhir_json": "{}", - "operation_requested": "CREATE", - "file_key": "test_file.csv", - } - + message_body = deepcopy(Message.create_message) forward_lambda_handler(generate_kinesis_message(message_body), None) mock_forward_request_to_api.assert_called_once_with(message_body) @patch("forwarding_lambda.forward_request_to_lambda") def test_forward_lambda_handler_update(self, mock_forward_request_to_api): - message_body = { - "row_id": "test_8", - "fhir_json": "{}", - "operation_requested": "UPDATE", - "file_key": "test_file.csv", - } + message_body = deepcopy(Message.update_message) forward_lambda_handler(generate_kinesis_message(message_body), None) mock_forward_request_to_api.assert_called_once_with(message_body) @patch("forwarding_lambda.logger") def test_forward_lambda_handler_with_exception(self, mock_logger): - message_body = { - "row_id": "test_9", - "fhir_json": "{}", - "operation_requested": "INVALID OPERATION", - "file_key": "test_file.csv", - } + message_body = deepcopy(Message.create_message) + message_body["operation_requested"] = "INVALID_OPERATION" forward_lambda_handler(generate_kinesis_message(message_body), None) mock_logger.error.assert_called() diff --git a/recordforwarder/tests/utils_for_recordfowarder_tests/values_for_recordforwarder_tests.py b/recordforwarder/tests/utils_for_recordfowarder_tests/values_for_recordforwarder_tests.py index 717b6a5e..353aec80 100644 --- a/recordforwarder/tests/utils_for_recordfowarder_tests/values_for_recordforwarder_tests.py +++ b/recordforwarder/tests/utils_for_recordfowarder_tests/values_for_recordforwarder_tests.py @@ -114,33 +114,36 @@ class Urls: } +class Diagnostics: + """Diagnostics messages""" + + INVALID_ACTION_FLAG = "Invalid ACTION_FLAG - ACTION_FLAG must be 'NEW', 'UPDATE' or 'DELETE'" + NO_PERMISSIONS = "No permissions for requested operation" + MISSING_UNIQUE_ID = "UNIQUE_ID or UNIQUE_ID_URI is missing" + UNABLE_TO_OBTAIN_IMMS_ID = "Unable to obtain imms event id" + UNABLE_TO_OBTAIN_VERSION = "Unable to obtain current imms event version" + INVALID_CONVERSION = "Unable to convert row to FHIR Immunization Resource JSON format" + + class Message: """Class containing example kinesis messages""" ROW_ID = "123456" IMMS_ID = "277befd9-574e-47fe-a6ee-189858af3bb0" + DIAGNOSTICS = Diagnostics.MISSING_UNIQUE_ID base_message_fields = {"row_id": ROW_ID, "file_key": TestFile.FILE_KEY, "supplier": TestFile.SUPPLIER} create_message = {**base_message_fields, "fhir_json": test_fhir_json, "operation_requested": "CREATE"} update_message = {**base_message_fields, "fhir_json": test_fhir_json, "operation_requested": "UPDATE"} delete_message = {**base_message_fields, "fhir_json": test_fhir_json, "operation_requested": "DELETE"} + diagnostics_message = {**base_message_fields, "diagnostics": DIAGNOSTICS} lambda_success_headers = {"Location": "https://example.com/immunization/test_id"} -class Diagnostics: - """Diagnostics messages""" - - INVALID_ACTION_FLAG = "Invalid ACTION_FLAG - ACTION_FLAG must be 'NEW', 'UPDATE' or 'DELETE'" - NO_PERMISSIONS = "No permissions for requested operation" - MISSING_UNIQUE_ID = "UNIQUE_ID or UNIQUE_ID_URI is missing" - UNABLE_TO_OBTAIN_IMMS_ID = "Unable to obtain imms event id" - UNABLE_TO_OBTAIN_VERSION = "Unable to obtain current imms event version" - INVALID_CONVERSION = "Unable to convert row to FHIR Immunization Resource JSON format" - - class ResponseBody: """Examples of response body for get_imms_id_and_version""" + id_and_version_not_found = { "resourceType": "Bundle", "type": "searchset", From 0bf1ca4adc9c70cf97c4d627208daf86e4f729a7 Mon Sep 17 00:00:00 2001 From: AlexandraBenson Date: Wed, 6 Nov 2024 18:28:30 +0000 Subject: [PATCH 17/22] Simplify mocking for lambda invocation --- .../tests/test_e2e_forwarding_lambda.py | 36 +++++++++---------- .../tests/test_forwarding_lambda.py | 22 +++++------- .../tests/test_get_imms_id_and_version.py | 33 ++++++++--------- .../utils_for_recordforwarder_tests.py | 16 ++++++--- 4 files changed, 54 insertions(+), 53 deletions(-) diff --git a/recordforwarder/tests/test_e2e_forwarding_lambda.py b/recordforwarder/tests/test_e2e_forwarding_lambda.py index b9b03f04..2ee04a8e 100644 --- a/recordforwarder/tests/test_e2e_forwarding_lambda.py +++ b/recordforwarder/tests/test_e2e_forwarding_lambda.py @@ -26,8 +26,8 @@ ResponseBody, ) from tests.utils_for_recordfowarder_tests.utils_for_recordforwarder_tests import ( - generate_mock_operation_outcome, - generate_payload, + generate_operation_outcome, + generate_lambda_payload, generate_kinesis_message, generate_lambda_invocation_side_effect, ) @@ -69,29 +69,29 @@ def execute_test(self, message, expected_content, mock_lambda_payloads: dict): self.check_ack_file(expected_content) def test_forward_lambda_e2e_create_success(self): - mock_lambda_payloads = {"CREATE": generate_payload(status_code=201, headers=lambda_success_headers)} + mock_lambda_payloads = {"CREATE": generate_lambda_payload(status_code=201, headers=lambda_success_headers)} self.execute_test(Message.create_message, "OK", mock_lambda_payloads=mock_lambda_payloads) def test_forward_lambda_e2e_create_duplicate(self): mock_diagnostics = ( "The provided identifier: https://supplierABC/identifiers/vacc#test-identifier1 is duplicated" ) - mock_body = generate_mock_operation_outcome(diagnostics=mock_diagnostics, code="duplicate") - mock_lambda_payloads = {"CREATE": generate_payload(status_code=422, body=mock_body)} + mock_body = generate_operation_outcome(diagnostics=mock_diagnostics, code="duplicate") + mock_lambda_payloads = {"CREATE": generate_lambda_payload(status_code=422, body=mock_body)} self.execute_test(Message.create_message, "Fatal Error", mock_lambda_payloads=mock_lambda_payloads) def test_forward_lambda_e2e_create_failed(self): mock_diagnostics = "the provided event ID is either missing or not in the expected format." - mock_body = generate_mock_operation_outcome(diagnostics=mock_diagnostics, code="duplicate") - mock_lambda_payloads = {"CREATE": generate_payload(status_code=400, body=mock_body)} + mock_body = generate_operation_outcome(diagnostics=mock_diagnostics, code="duplicate") + mock_lambda_payloads = {"CREATE": generate_lambda_payload(status_code=400, body=mock_body)} self.execute_test(Message.create_message, "Fatal Error", mock_lambda_payloads=mock_lambda_payloads) def test_forward_lambda_e2e_create_multi_line_diagnostics(self): mock_diagnostics = """This a string of diagnostics which spans multiple lines and has some carriage returns\n\nand random space""" - mock_body = generate_mock_operation_outcome(diagnostics=mock_diagnostics) - mock_lambda_payloads = {"CREATE": generate_payload(status_code=404, body=mock_body)} + mock_body = generate_operation_outcome(diagnostics=mock_diagnostics) + mock_lambda_payloads = {"CREATE": generate_lambda_payload(status_code=404, body=mock_body)} expected_single_line_diagnostics = ( "This a string of diagnostics which spans multiple lines and has some carriage returns and random space" ) @@ -99,37 +99,37 @@ def test_forward_lambda_e2e_create_multi_line_diagnostics(self): def test_forward_lambda_e2e_update_success(self): mock_lambda_payloads = { - "UPDATE": generate_payload(200), - "SEARCH": generate_payload(200, body=ResponseBody.id_and_version_found), + "UPDATE": generate_lambda_payload(200), + "SEARCH": generate_lambda_payload(200, body=ResponseBody.id_and_version_found), } self.execute_test(Message.update_message, "OK", mock_lambda_payloads) def test_forward_lambda_e2e_update_failed_unable_to_get_id(self): mock_lambda_payloads = { - "SEARCH": generate_payload(status_code=200, body=ResponseBody.id_and_version_not_found), + "SEARCH": generate_lambda_payload(status_code=200, body=ResponseBody.id_and_version_not_found), } self.execute_test(Message.update_message, "Fatal", mock_lambda_payloads) def test_forward_lambda_e2e_update_failed(self): mock_diagnstics = "the provided event ID is either missing or not in the expected format." mock_lambda_payloads = { - "UPDATE": generate_payload(400, body=generate_mock_operation_outcome(mock_diagnstics)), - "SEARCH": generate_payload(200, body=ResponseBody.id_and_version_found), + "UPDATE": generate_lambda_payload(400, body=generate_operation_outcome(mock_diagnstics)), + "SEARCH": generate_lambda_payload(200, body=ResponseBody.id_and_version_found), } self.execute_test(Message.update_message, "Fatal Error", mock_lambda_payloads) def test_forward_lambda_e2e_delete_success(self): mock_lambda_payloads = { - "DELETE": generate_payload(204), - "SEARCH": generate_payload(200, body=ResponseBody.id_and_version_found), + "DELETE": generate_lambda_payload(204), + "SEARCH": generate_lambda_payload(200, body=ResponseBody.id_and_version_found), } self.execute_test(Message.delete_message, "OK", mock_lambda_payloads) def test_forward_lambda_e2e_delete_failed(self): mock_diagnstics = "the provided event ID is either missing or not in the expected format." mock_lambda_payloads = { - "UPDATE": generate_payload(404, body=generate_mock_operation_outcome(mock_diagnstics, code="not-found")), - "SEARCH": generate_payload(200, body=ResponseBody.id_and_version_not_found), + "UPDATE": generate_lambda_payload(404, body=generate_operation_outcome(mock_diagnstics, code="not-found")), + "SEARCH": generate_lambda_payload(200, body=ResponseBody.id_and_version_not_found), } self.execute_test(Message.delete_message, "Fatal Error", mock_lambda_payloads) diff --git a/recordforwarder/tests/test_forwarding_lambda.py b/recordforwarder/tests/test_forwarding_lambda.py index fc5e88f2..a06e3736 100644 --- a/recordforwarder/tests/test_forwarding_lambda.py +++ b/recordforwarder/tests/test_forwarding_lambda.py @@ -27,8 +27,8 @@ Message, ) from tests.utils_for_recordfowarder_tests.utils_for_recordforwarder_tests import ( - generate_mock_operation_outcome, - generate_payload, + generate_operation_outcome, + generate_lambda_payload, generate_kinesis_message, generate_lambda_invocation_side_effect, ) @@ -49,7 +49,7 @@ def common_contexts_for_forwarding_lambda_tests( ) -> Generator[MagicMock, None, None]: """ A context manager which applies common patching for the tests in the TestForwardingLambda class. - Yields mock_firehose_logger and logs (where logs is a list of the captured log entries). + Yields mock_create_ack_data. """ with ExitStack() as stack: stack.enter_context(patch("update_ack_file.s3_client")), # pylint: disable=expression-not-assigned @@ -134,7 +134,7 @@ def test_create_ack_data(self): ) def test_forward_request_to_api_new_success(self): - mock_lambda_payloads = {"CREATE": generate_payload(status_code=201, headers=lambda_success_headers)} + mock_lambda_payloads = {"CREATE": generate_lambda_payload(status_code=201, headers=lambda_success_headers)} with self.common_contexts_for_forwarding_lambda_tests(mock_lambda_payloads) as mock_create_ack_data: forward_request_to_lambda(deepcopy(Message.create_message)) @@ -143,9 +143,8 @@ def test_forward_request_to_api_new_success(self): def test_forward_request_to_api_new_duplicate(self): diagnostics = "The provided identifier: https://supplierABC/identifiers/vacc#test-identifier1 is duplicated" - mock_lambda_payloads = { - "CREATE": generate_payload(status_code=422, body=generate_mock_operation_outcome(diagnostics)) + "CREATE": generate_lambda_payload(status_code=422, body=generate_operation_outcome(diagnostics)) } with self.common_contexts_for_forwarding_lambda_tests(mock_lambda_payloads) as mock_create_ack_data: forward_request_to_lambda(deepcopy(Message.create_message)) @@ -154,14 +153,12 @@ def test_forward_request_to_api_new_duplicate(self): mock_create_ack_data.assert_called_with("20240821T10153000", Message.ROW_ID, False, diagnostics, None) def test_forward_request_to_api_update_failure(self): - diagnostics = ( "Validation errors: The provided immunization id:test_id doesn't match with the content of the request body" ) - mock_lambda_payloads = { - "UPDATE": generate_payload(status_code=422, body=generate_mock_operation_outcome(diagnostics)), - "SEARCH": generate_payload(status_code=200, body=ResponseBody.id_and_version_found), + "UPDATE": generate_lambda_payload(status_code=422, body=generate_operation_outcome(diagnostics)), + "SEARCH": generate_lambda_payload(status_code=200, body=ResponseBody.id_and_version_found), } with self.common_contexts_for_forwarding_lambda_tests(mock_lambda_payloads) as mock_create_ack_data: forward_request_to_lambda(deepcopy(Message.update_message)) @@ -181,10 +178,9 @@ def test_forward_request_to_api_update_failure_imms_id_none(self): mock_lambda_client.assert_not_called() def test_forward_request_to_api_delete_success(self): - mock_lambda_payloads = { - "DELETE": generate_payload(status_code=204), - "SEARCH": generate_payload(status_code=200, body=ResponseBody.id_and_version_found), + "DELETE": generate_lambda_payload(status_code=204), + "SEARCH": generate_lambda_payload(status_code=200, body=ResponseBody.id_and_version_found), } with self.common_contexts_for_forwarding_lambda_tests(mock_lambda_payloads) as mock_create_ack_data: forward_request_to_lambda(deepcopy(Message.delete_message)) diff --git a/recordforwarder/tests/test_get_imms_id_and_version.py b/recordforwarder/tests/test_get_imms_id_and_version.py index 8e2155ad..04cf6f55 100644 --- a/recordforwarder/tests/test_get_imms_id_and_version.py +++ b/recordforwarder/tests/test_get_imms_id_and_version.py @@ -2,14 +2,13 @@ import unittest from unittest.mock import patch -import json -from io import StringIO +from copy import deepcopy from moto import mock_s3 from get_imms_id_and_version import get_imms_id_and_version from errors import IdNotFoundError from tests.utils_for_recordfowarder_tests.utils_for_recordforwarder_tests import ( - generate_payload, - generate_mock_operation_outcome, + generate_operation_outcome, + generate_lambda_payload, ) from tests.utils_for_recordfowarder_tests.values_for_recordforwarder_tests import ResponseBody @@ -26,10 +25,10 @@ class TestGetImmsIdAndVersion(unittest.TestCase): def test_success(self): """Test that imms_id and version are correctly identified from a successful search lambda response.""" - mock_lambda_response_payload = { - "Payload": StringIO(json.dumps(generate_payload(200, body=ResponseBody.id_and_version_found))) - } - with patch("clients.lambda_client.invoke", return_value=mock_lambda_response_payload): + with patch( + "clients.lambda_client.invoke", + return_value=generate_lambda_payload(status_code=200, body=deepcopy(ResponseBody.id_and_version_found)), + ): imms_id, version = get_imms_id_and_version(fhir_json_with_identifier_value_and_system) self.assertEqual(imms_id, "277befd9-574e-47fe-a6ee-189858af3bb0") @@ -37,20 +36,18 @@ def test_success(self): def test_failure_due_to_empty_search_lambda_return(self): """Test that an IdNotFoundError is raised for a successful search lambda response which contains no entries.""" - mock_lambda_response_payload = { - "Payload": StringIO(json.dumps(generate_payload(200, body=ResponseBody.id_and_version_not_found))) - } - with patch("clients.lambda_client.invoke", return_value=mock_lambda_response_payload): + with patch( + "clients.lambda_client.invoke", + return_value=generate_lambda_payload(status_code=200, body=deepcopy(ResponseBody.id_and_version_not_found)), + ): with self.assertRaises(IdNotFoundError): get_imms_id_and_version(fhir_json_with_identifier_value_and_system) def test_failure_due_to_search_lambda_404(self): """Test that an IdNotFoundError is raised for an unsuccessful search lambda response.""" - mock_lambda_response_payload = { - "Payload": StringIO( - json.dumps(generate_payload(404, body=generate_mock_operation_outcome("some_diagnostics"))) - ) - } - with patch("clients.lambda_client.invoke", return_value=mock_lambda_response_payload): + with patch( + "clients.lambda_client.invoke", + return_value=generate_lambda_payload(status_code=404, body=generate_operation_outcome("some_diagnostics")), + ): with self.assertRaises(IdNotFoundError): get_imms_id_and_version(fhir_json_with_identifier_value_and_system) diff --git a/recordforwarder/tests/utils_for_recordfowarder_tests/utils_for_recordforwarder_tests.py b/recordforwarder/tests/utils_for_recordfowarder_tests/utils_for_recordforwarder_tests.py index 71ec2fc9..ca58bda6 100644 --- a/recordforwarder/tests/utils_for_recordfowarder_tests/utils_for_recordforwarder_tests.py +++ b/recordforwarder/tests/utils_for_recordfowarder_tests/utils_for_recordforwarder_tests.py @@ -12,7 +12,7 @@ def generate_kinesis_message(message: dict) -> str: return {"Records": [{"kinesis": {"data": kinesis_encoded_data}}]} -def generate_mock_operation_outcome(diagnostics: str, code: str = "duplicate") -> dict: +def generate_operation_outcome(diagnostics: str, code: str = "duplicate") -> dict: """Generates an Operation Outcome, with the given diagnostics and code""" return { "resourceType": "OperationOutcome", @@ -31,14 +31,22 @@ def generate_mock_operation_outcome(diagnostics: str, code: str = "duplicate") - } -def generate_payload(status_code: int, headers: Union[dict, None] = None, body: dict = None): +def generate_payload(status_code: int, headers: Union[dict, None] = None, body: dict = None) -> dict: """ Generates a payload with the given status code, headers and body - (body is converted to json string, and the key-value pair is omitted if there is no body) + (body is converted to json string, and the key-value pair is omitted if there is no body). """ return {"statusCode": status_code, **({"body": json.dumps(body)} if body is not None else {}), "headers": headers} +def generate_lambda_payload(status_code: int, headers: Union[dict, None] = None, body: dict = None) -> dict: + """ + Generates a mocked lambda return value, with the given status code, headers and body. + The body key-value pair is omitted if there is no body argument is given. + """ + return {"Payload": StringIO(json.dumps(generate_payload(status_code, headers, body)))} + + def generate_lambda_invocation_side_effect(mock_lambda_payloads): """ Takes a dictionary as input with key-value pairs in the format LAMBDA_TYPE: mock_response_payload, where @@ -48,6 +56,6 @@ def generate_lambda_invocation_side_effect(mock_lambda_payloads): def lambda_invocation_side_effect(FunctionName, *_args, **_kwargs): # pylint: disable=invalid-name lambda_type = FunctionName.split("_")[1] # Tests mock FunctionNames as mock_lambdatype_lambda_name - return {"Payload": StringIO(json.dumps(mock_lambda_payloads[lambda_type.upper()]))} + return mock_lambda_payloads[lambda_type.upper()] return lambda_invocation_side_effect From a38ee8e233b76e5c230e5a60d012f8b33bb97dee Mon Sep 17 00:00:00 2001 From: AlexandraBenson Date: Fri, 8 Nov 2024 12:40:49 +0000 Subject: [PATCH 18/22] Add multi-message e2e test and create class for operations --- recordforwarder/src/constants.py | 9 +++ recordforwarder/src/send_request_to_lambda.py | 8 +- .../tests/test_e2e_forwarding_lambda.py | 76 +++++++++++-------- .../tests/test_forwarding_lambda.py | 15 ++-- .../tests/test_get_imms_id_and_version.py | 28 +++---- recordforwarder/tests/test_log_structure.py | 11 +-- .../values_for_recordforwarder_tests.py | 27 +++++-- 7 files changed, 111 insertions(+), 63 deletions(-) diff --git a/recordforwarder/src/constants.py b/recordforwarder/src/constants.py index 852bed9b..9d2505ae 100644 --- a/recordforwarder/src/constants.py +++ b/recordforwarder/src/constants.py @@ -18,3 +18,12 @@ ] IMMS_BATCH_APP_NAME = "Imms-Batch-App" + + +class Operations: + """Class containing the CRUD operation lambdas which can be invoked by the batch process""" + + CREATE = "CREATE" + UPDATE = "UPDATE" + DELETE = "DELETE" + SEARCH = "SEARCH" diff --git a/recordforwarder/src/send_request_to_lambda.py b/recordforwarder/src/send_request_to_lambda.py index d73b9bd2..0cb465a9 100644 --- a/recordforwarder/src/send_request_to_lambda.py +++ b/recordforwarder/src/send_request_to_lambda.py @@ -4,7 +4,7 @@ from errors import MessageNotSuccessfulError, IdNotFoundError from get_imms_id_and_version import get_imms_id_and_version from utils_for_record_forwarder import invoke_lambda -from constants import IMMS_BATCH_APP_NAME +from constants import IMMS_BATCH_APP_NAME, Operations from log_structure import forwarder_function_info @@ -96,5 +96,9 @@ def send_request_to_lambda(message_body: dict) -> str: operation_requested = message_body.get("operation_requested") # Send request to Imms FHIR API and return the imms_id - function_map = {"CREATE": send_create_request, "UPDATE": send_update_request, "DELETE": send_delete_request} + function_map = { + Operations.CREATE: send_create_request, + Operations.UPDATE: send_update_request, + Operations.DELETE: send_delete_request, + } return function_map[operation_requested](fhir_json=fhir_json, supplier=supplier) diff --git a/recordforwarder/tests/test_e2e_forwarding_lambda.py b/recordforwarder/tests/test_e2e_forwarding_lambda.py index 2ee04a8e..de2fffc7 100644 --- a/recordforwarder/tests/test_e2e_forwarding_lambda.py +++ b/recordforwarder/tests/test_e2e_forwarding_lambda.py @@ -4,6 +4,7 @@ import unittest from unittest.mock import patch +from copy import deepcopy import os import sys from boto3 import client as boto3_client @@ -15,15 +16,16 @@ sys.path.insert(0, os.path.abspath(os.path.join(maindir, SRCDIR))) from forwarding_lambda import forward_lambda_handler +from constants import Operations from tests.utils_for_recordfowarder_tests.values_for_recordforwarder_tests import ( AWS_REGION, SOURCE_BUCKET_NAME, DESTINATION_BUCKET_NAME, - lambda_success_headers, MOCK_ENVIRONMENT_DICT, TestFile, Message, - ResponseBody, + SearchLambdaResponseBody, + LambdaPayloads, ) from tests.utils_for_recordfowarder_tests.utils_for_recordforwarder_tests import ( generate_operation_outcome, @@ -41,7 +43,7 @@ class TestForwardingLambdaE2E(unittest.TestCase): def setUp(self) -> None: - """Set up the SOURCE and DESTINATION buckets, and upload the TestFile to the SOURCE bucket""" + """Sets up the SOURCE and DESTINATION buckets, and upload the TestFile to the SOURCE bucket""" for bucket_name in [SOURCE_BUCKET_NAME, DESTINATION_BUCKET_NAME]: s3_client.create_bucket(Bucket=bucket_name, CreateBucketConfiguration={"LocationConstraint": AWS_REGION}) s3_client.put_object(Bucket=SOURCE_BUCKET_NAME, Key=TestFile.FILE_KEY, Body="test_data") @@ -60,30 +62,54 @@ def check_ack_file(self, expected_content): self.assertIn(expected_content, ack_file_content) def execute_test(self, message, expected_content, mock_lambda_payloads: dict): - with patch( - "utils_for_record_forwarder.lambda_client.invoke", - side_effect=generate_lambda_invocation_side_effect(mock_lambda_payloads), + with ( + patch( + "utils_for_record_forwarder.lambda_client.invoke", + side_effect=generate_lambda_invocation_side_effect(mock_lambda_payloads), + ), + patch("log_firehose.Forwarder_FirehoseLogger.forwarder_send_log"), ): forward_lambda_handler(generate_kinesis_message(message), None) self.check_ack_file(expected_content) - def test_forward_lambda_e2e_create_success(self): - mock_lambda_payloads = {"CREATE": generate_lambda_payload(status_code=201, headers=lambda_success_headers)} - self.execute_test(Message.create_message, "OK", mock_lambda_payloads=mock_lambda_payloads) + def test_forward_lambda_e2e_successes(self): + + messages = [ + {**deepcopy(Message.create_message), "row_id": "test#1"}, + {**deepcopy(Message.update_message), "row_id": "test#2"}, + {**deepcopy(Message.delete_message), "row_id": "test#3"}, + {**deepcopy(Message.create_message), "row_id": "test#4"}, + ] + # Mock the lambda invocation to return the correct response + with ( + patch("utils_for_record_forwarder.lambda_client.invoke") as mock_invoke, + patch("log_firehose.Forwarder_FirehoseLogger.forwarder_send_log"), + ): + + for message in messages: + mock_invoke.side_effect = generate_lambda_invocation_side_effect(deepcopy(LambdaPayloads.SUCCESS)) + forward_lambda_handler(generate_kinesis_message(message), None) + + ack_file_obj = s3_client.get_object(Bucket=DESTINATION_BUCKET_NAME, Key=TestFile.ACK_FILE_KEY) + ack_file_content = ack_file_obj["Body"].read().decode("utf-8") + self.assertIn("test#1|OK", ack_file_content) + self.assertIn("test#2|OK", ack_file_content) + self.assertIn("test#3|OK", ack_file_content) + self.assertIn("test#4|OK", ack_file_content) def test_forward_lambda_e2e_create_duplicate(self): mock_diagnostics = ( "The provided identifier: https://supplierABC/identifiers/vacc#test-identifier1 is duplicated" ) mock_body = generate_operation_outcome(diagnostics=mock_diagnostics, code="duplicate") - mock_lambda_payloads = {"CREATE": generate_lambda_payload(status_code=422, body=mock_body)} + mock_lambda_payloads = {Operations.CREATE: generate_lambda_payload(status_code=422, body=mock_body)} self.execute_test(Message.create_message, "Fatal Error", mock_lambda_payloads=mock_lambda_payloads) def test_forward_lambda_e2e_create_failed(self): mock_diagnostics = "the provided event ID is either missing or not in the expected format." mock_body = generate_operation_outcome(diagnostics=mock_diagnostics, code="duplicate") - mock_lambda_payloads = {"CREATE": generate_lambda_payload(status_code=400, body=mock_body)} + mock_lambda_payloads = {Operations.CREATE: generate_lambda_payload(status_code=400, body=mock_body)} self.execute_test(Message.create_message, "Fatal Error", mock_lambda_payloads=mock_lambda_payloads) def test_forward_lambda_e2e_create_multi_line_diagnostics(self): @@ -91,45 +117,33 @@ def test_forward_lambda_e2e_create_multi_line_diagnostics(self): of diagnostics which spans multiple lines and has some carriage returns\n\nand random space""" mock_body = generate_operation_outcome(diagnostics=mock_diagnostics) - mock_lambda_payloads = {"CREATE": generate_lambda_payload(status_code=404, body=mock_body)} + mock_lambda_payloads = {Operations.CREATE: generate_lambda_payload(status_code=404, body=mock_body)} expected_single_line_diagnostics = ( "This a string of diagnostics which spans multiple lines and has some carriage returns and random space" ) self.execute_test(Message.create_message, expected_single_line_diagnostics, mock_lambda_payloads) - def test_forward_lambda_e2e_update_success(self): - mock_lambda_payloads = { - "UPDATE": generate_lambda_payload(200), - "SEARCH": generate_lambda_payload(200, body=ResponseBody.id_and_version_found), - } - self.execute_test(Message.update_message, "OK", mock_lambda_payloads) - def test_forward_lambda_e2e_update_failed_unable_to_get_id(self): mock_lambda_payloads = { - "SEARCH": generate_lambda_payload(status_code=200, body=ResponseBody.id_and_version_not_found), + "SEARCH": generate_lambda_payload(status_code=200, body=SearchLambdaResponseBody.id_and_version_not_found), } self.execute_test(Message.update_message, "Fatal", mock_lambda_payloads) def test_forward_lambda_e2e_update_failed(self): mock_diagnstics = "the provided event ID is either missing or not in the expected format." mock_lambda_payloads = { - "UPDATE": generate_lambda_payload(400, body=generate_operation_outcome(mock_diagnstics)), - "SEARCH": generate_lambda_payload(200, body=ResponseBody.id_and_version_found), + Operations.UPDATE: generate_lambda_payload(400, body=generate_operation_outcome(mock_diagnstics)), + "SEARCH": generate_lambda_payload(200, body=SearchLambdaResponseBody.id_and_version_found), } self.execute_test(Message.update_message, "Fatal Error", mock_lambda_payloads) - def test_forward_lambda_e2e_delete_success(self): - mock_lambda_payloads = { - "DELETE": generate_lambda_payload(204), - "SEARCH": generate_lambda_payload(200, body=ResponseBody.id_and_version_found), - } - self.execute_test(Message.delete_message, "OK", mock_lambda_payloads) - def test_forward_lambda_e2e_delete_failed(self): mock_diagnstics = "the provided event ID is either missing or not in the expected format." mock_lambda_payloads = { - "UPDATE": generate_lambda_payload(404, body=generate_operation_outcome(mock_diagnstics, code="not-found")), - "SEARCH": generate_lambda_payload(200, body=ResponseBody.id_and_version_not_found), + Operations.UPDATE: generate_lambda_payload( + 404, body=generate_operation_outcome(mock_diagnstics, code="not-found") + ), + "SEARCH": generate_lambda_payload(200, body=SearchLambdaResponseBody.id_and_version_not_found), } self.execute_test(Message.delete_message, "Fatal Error", mock_lambda_payloads) diff --git a/recordforwarder/tests/test_forwarding_lambda.py b/recordforwarder/tests/test_forwarding_lambda.py index a06e3736..481c6db0 100644 --- a/recordforwarder/tests/test_forwarding_lambda.py +++ b/recordforwarder/tests/test_forwarding_lambda.py @@ -23,7 +23,7 @@ lambda_success_headers, MOCK_ENVIRONMENT_DICT, AWS_REGION, - ResponseBody, + SearchLambdaResponseBody, Message, ) from tests.utils_for_recordfowarder_tests.utils_for_recordforwarder_tests import ( @@ -33,6 +33,7 @@ generate_lambda_invocation_side_effect, ) from forwarding_lambda import forward_lambda_handler, forward_request_to_lambda +from constants import Operations from update_ack_file import create_ack_data @@ -134,7 +135,7 @@ def test_create_ack_data(self): ) def test_forward_request_to_api_new_success(self): - mock_lambda_payloads = {"CREATE": generate_lambda_payload(status_code=201, headers=lambda_success_headers)} + mock_lambda_payloads = {Operations.CREATE: generate_lambda_payload(status_code=201, headers=lambda_success_headers)} with self.common_contexts_for_forwarding_lambda_tests(mock_lambda_payloads) as mock_create_ack_data: forward_request_to_lambda(deepcopy(Message.create_message)) @@ -144,7 +145,7 @@ def test_forward_request_to_api_new_success(self): def test_forward_request_to_api_new_duplicate(self): diagnostics = "The provided identifier: https://supplierABC/identifiers/vacc#test-identifier1 is duplicated" mock_lambda_payloads = { - "CREATE": generate_lambda_payload(status_code=422, body=generate_operation_outcome(diagnostics)) + Operations.CREATE: generate_lambda_payload(status_code=422, body=generate_operation_outcome(diagnostics)) } with self.common_contexts_for_forwarding_lambda_tests(mock_lambda_payloads) as mock_create_ack_data: forward_request_to_lambda(deepcopy(Message.create_message)) @@ -157,8 +158,8 @@ def test_forward_request_to_api_update_failure(self): "Validation errors: The provided immunization id:test_id doesn't match with the content of the request body" ) mock_lambda_payloads = { - "UPDATE": generate_lambda_payload(status_code=422, body=generate_operation_outcome(diagnostics)), - "SEARCH": generate_lambda_payload(status_code=200, body=ResponseBody.id_and_version_found), + Operations.UPDATE: generate_lambda_payload(status_code=422, body=generate_operation_outcome(diagnostics)), + "SEARCH": generate_lambda_payload(status_code=200, body=SearchLambdaResponseBody.id_and_version_found), } with self.common_contexts_for_forwarding_lambda_tests(mock_lambda_payloads) as mock_create_ack_data: forward_request_to_lambda(deepcopy(Message.update_message)) @@ -179,8 +180,8 @@ def test_forward_request_to_api_update_failure_imms_id_none(self): def test_forward_request_to_api_delete_success(self): mock_lambda_payloads = { - "DELETE": generate_lambda_payload(status_code=204), - "SEARCH": generate_lambda_payload(status_code=200, body=ResponseBody.id_and_version_found), + Operations.DELETE: generate_lambda_payload(status_code=204), + "SEARCH": generate_lambda_payload(status_code=200, body=SearchLambdaResponseBody.id_and_version_found), } with self.common_contexts_for_forwarding_lambda_tests(mock_lambda_payloads) as mock_create_ack_data: forward_request_to_lambda(deepcopy(Message.delete_message)) diff --git a/recordforwarder/tests/test_get_imms_id_and_version.py b/recordforwarder/tests/test_get_imms_id_and_version.py index 04cf6f55..99a6089e 100644 --- a/recordforwarder/tests/test_get_imms_id_and_version.py +++ b/recordforwarder/tests/test_get_imms_id_and_version.py @@ -3,33 +3,33 @@ import unittest from unittest.mock import patch from copy import deepcopy -from moto import mock_s3 from get_imms_id_and_version import get_imms_id_and_version from errors import IdNotFoundError from tests.utils_for_recordfowarder_tests.utils_for_recordforwarder_tests import ( generate_operation_outcome, generate_lambda_payload, ) -from tests.utils_for_recordfowarder_tests.values_for_recordforwarder_tests import ResponseBody - - -fhir_json_with_identifier_value_and_system = {"identifier": [{"value": "a_value", "system": "a_system"}]} +from tests.utils_for_recordfowarder_tests.values_for_recordforwarder_tests import ( + SearchLambdaResponseBody, + test_imms_fhir_json, +) -@mock_s3 class TestGetImmsIdAndVersion(unittest.TestCase): """ - Tests for get_imms_id_and_version. Note that these test mock the lambda invocation, so do not test the - interaction with search lambda. + Tests for get_imms_id_and_version. + Note that these test mock the lambda invocation, therefore they do not test the interaction with search lambda. """ def test_success(self): """Test that imms_id and version are correctly identified from a successful search lambda response.""" with patch( "clients.lambda_client.invoke", - return_value=generate_lambda_payload(status_code=200, body=deepcopy(ResponseBody.id_and_version_found)), + return_value=generate_lambda_payload( + status_code=200, body=deepcopy(SearchLambdaResponseBody.id_and_version_found) + ), ): - imms_id, version = get_imms_id_and_version(fhir_json_with_identifier_value_and_system) + imms_id, version = get_imms_id_and_version(test_imms_fhir_json) self.assertEqual(imms_id, "277befd9-574e-47fe-a6ee-189858af3bb0") self.assertEqual(version, 2) @@ -38,10 +38,12 @@ def test_failure_due_to_empty_search_lambda_return(self): """Test that an IdNotFoundError is raised for a successful search lambda response which contains no entries.""" with patch( "clients.lambda_client.invoke", - return_value=generate_lambda_payload(status_code=200, body=deepcopy(ResponseBody.id_and_version_not_found)), + return_value=generate_lambda_payload( + status_code=200, body=deepcopy(SearchLambdaResponseBody.id_and_version_not_found) + ), ): with self.assertRaises(IdNotFoundError): - get_imms_id_and_version(fhir_json_with_identifier_value_and_system) + get_imms_id_and_version(test_imms_fhir_json) def test_failure_due_to_search_lambda_404(self): """Test that an IdNotFoundError is raised for an unsuccessful search lambda response.""" @@ -50,4 +52,4 @@ def test_failure_due_to_search_lambda_404(self): return_value=generate_lambda_payload(status_code=404, body=generate_operation_outcome("some_diagnostics")), ): with self.assertRaises(IdNotFoundError): - get_imms_id_and_version(fhir_json_with_identifier_value_and_system) + get_imms_id_and_version(test_imms_fhir_json) diff --git a/recordforwarder/tests/test_log_structure.py b/recordforwarder/tests/test_log_structure.py index 6372e3a8..1328a9e9 100644 --- a/recordforwarder/tests/test_log_structure.py +++ b/recordforwarder/tests/test_log_structure.py @@ -7,7 +7,8 @@ from datetime import datetime from contextlib import contextmanager, ExitStack from send_request_to_lambda import send_request_to_lambda -from tests.utils_for_recordfowarder_tests.values_for_recordforwarder_tests import Message +from constants import Operations +from tests.utils_for_recordfowarder_tests.values_for_recordforwarder_tests import Message, Diagnostics from errors import MessageNotSuccessfulError FIXED_DATETIME = datetime(2024, 10, 30, 12, 0, 0) @@ -72,7 +73,7 @@ def common_contexts_for_splunk_logging_tests(self): def test_splunk_logging_success(self): """Tests successful rows""" - for operation in ["CREATE", "UPDATE", "DELETE"]: + for operation in [Operations.CREATE, Operations.UPDATE, Operations.DELETE]: with self.subTest(operation): with ( self.common_contexts_for_splunk_logging_tests() as (mock_firehose_logger, logs), # noqa: E999 @@ -86,8 +87,8 @@ def test_splunk_logging_success(self): def test_splunk_logging_failure_during_processing(self): """Tests a row which failed processing (and therefore has diagnostics in the message recevied from kinesis)""" - diagnostics = "Unable to obtain IMMS ID" - operation = "UPDATE" + diagnostics = Diagnostics.INVALID_ACTION_FLAG + operation = Operations.UPDATE with ( self.common_contexts_for_splunk_logging_tests() as (mock_firehose_logger, logs), # noqa: E999 self.assertRaises(MessageNotSuccessfulError) as context, @@ -101,7 +102,7 @@ def test_splunk_logging_failure_during_processing(self): def test_splunk_logging_failure_during_forwarding(self): """Tests rows which fail during forwarding""" - for operation in ["CREATE", "UPDATE", "DELETE"]: + for operation in [Operations.CREATE, Operations.UPDATE, Operations.DELETE]: error_message = f"API Error: Unable to {operation.lower()} resource" with self.subTest(operation): with ( diff --git a/recordforwarder/tests/utils_for_recordfowarder_tests/values_for_recordforwarder_tests.py b/recordforwarder/tests/utils_for_recordfowarder_tests/values_for_recordforwarder_tests.py index 353aec80..8c568fb8 100644 --- a/recordforwarder/tests/utils_for_recordfowarder_tests/values_for_recordforwarder_tests.py +++ b/recordforwarder/tests/utils_for_recordfowarder_tests/values_for_recordforwarder_tests.py @@ -1,5 +1,8 @@ """Values for use in recordforwarder tests""" +from constants import Operations +from tests.utils_for_recordfowarder_tests.utils_for_recordforwarder_tests import generate_lambda_payload + MOCK_ENVIRONMENT_DICT = { "SOURCE_BUCKET_NAME": "immunisation-batch-internal-dev-data-sources", "ACK_BUCKET_NAME": "immunisation-batch-internal-dev-data-destinations", @@ -39,7 +42,7 @@ class Urls: ODS = "https://fhir.nhs.uk/Id/ods-organization-code" -test_fhir_json = { +test_imms_fhir_json = { "resourceType": "Immunization", "contained": [ {"resourceType": "Practitioner", "id": "Pract1", "name": [{"family": "Doe", "given": ["Jane"]}]}, @@ -132,16 +135,16 @@ class Message: IMMS_ID = "277befd9-574e-47fe-a6ee-189858af3bb0" DIAGNOSTICS = Diagnostics.MISSING_UNIQUE_ID base_message_fields = {"row_id": ROW_ID, "file_key": TestFile.FILE_KEY, "supplier": TestFile.SUPPLIER} - create_message = {**base_message_fields, "fhir_json": test_fhir_json, "operation_requested": "CREATE"} - update_message = {**base_message_fields, "fhir_json": test_fhir_json, "operation_requested": "UPDATE"} - delete_message = {**base_message_fields, "fhir_json": test_fhir_json, "operation_requested": "DELETE"} + create_message = {**base_message_fields, "fhir_json": test_imms_fhir_json, "operation_requested": Operations.CREATE} + update_message = {**base_message_fields, "fhir_json": test_imms_fhir_json, "operation_requested": Operations.UPDATE} + delete_message = {**base_message_fields, "fhir_json": test_imms_fhir_json, "operation_requested": Operations.DELETE} diagnostics_message = {**base_message_fields, "diagnostics": DIAGNOSTICS} lambda_success_headers = {"Location": "https://example.com/immunization/test_id"} -class ResponseBody: +class SearchLambdaResponseBody: """Examples of response body for get_imms_id_and_version""" id_and_version_not_found = { @@ -164,3 +167,17 @@ class ResponseBody: "entry": [{"resource": {"id": Message.IMMS_ID, "meta": {"versionId": 2}}}], "total": 1, } + + +class LambdaPayloads: + """ + Class containing dictionaries of mock lambda payloads, to be used as inputs for the + generate_lambda_invocation_side_effect fucntion + """ + + SUCCESS = { + Operations.CREATE: generate_lambda_payload(status_code=201, headers=lambda_success_headers), + Operations.UPDATE: generate_lambda_payload(status_code=200), + Operations.DELETE: generate_lambda_payload(status_code=204), + Operations.SEARCH: generate_lambda_payload(status_code=200, body=SearchLambdaResponseBody.id_and_version_found), + } From b67e8c8537c7055279319cf40045a5e71978ca60 Mon Sep 17 00:00:00 2001 From: AlexandraBenson Date: Fri, 8 Nov 2024 13:41:43 +0000 Subject: [PATCH 19/22] Create class for mock lambda payloads --- .../tests/test_e2e_forwarding_lambda.py | 56 ++++++-------- .../values_for_recordforwarder_tests.py | 77 +++++++++++++++++-- 2 files changed, 91 insertions(+), 42 deletions(-) diff --git a/recordforwarder/tests/test_e2e_forwarding_lambda.py b/recordforwarder/tests/test_e2e_forwarding_lambda.py index de2fffc7..b5793e9c 100644 --- a/recordforwarder/tests/test_e2e_forwarding_lambda.py +++ b/recordforwarder/tests/test_e2e_forwarding_lambda.py @@ -24,7 +24,6 @@ MOCK_ENVIRONMENT_DICT, TestFile, Message, - SearchLambdaResponseBody, LambdaPayloads, ) from tests.utils_for_recordfowarder_tests.utils_for_recordforwarder_tests import ( @@ -99,18 +98,9 @@ def test_forward_lambda_e2e_successes(self): self.assertIn("test#4|OK", ack_file_content) def test_forward_lambda_e2e_create_duplicate(self): - mock_diagnostics = ( - "The provided identifier: https://supplierABC/identifiers/vacc#test-identifier1 is duplicated" + self.execute_test( + Message.create_message, "Fatal Error", mock_lambda_payloads=deepcopy(LambdaPayloads.CREATE.DUPLICATE) ) - mock_body = generate_operation_outcome(diagnostics=mock_diagnostics, code="duplicate") - mock_lambda_payloads = {Operations.CREATE: generate_lambda_payload(status_code=422, body=mock_body)} - self.execute_test(Message.create_message, "Fatal Error", mock_lambda_payloads=mock_lambda_payloads) - - def test_forward_lambda_e2e_create_failed(self): - mock_diagnostics = "the provided event ID is either missing or not in the expected format." - mock_body = generate_operation_outcome(diagnostics=mock_diagnostics, code="duplicate") - mock_lambda_payloads = {Operations.CREATE: generate_lambda_payload(status_code=400, body=mock_body)} - self.execute_test(Message.create_message, "Fatal Error", mock_lambda_payloads=mock_lambda_payloads) def test_forward_lambda_e2e_create_multi_line_diagnostics(self): mock_diagnostics = """This a string @@ -124,40 +114,38 @@ def test_forward_lambda_e2e_create_multi_line_diagnostics(self): self.execute_test(Message.create_message, expected_single_line_diagnostics, mock_lambda_payloads) def test_forward_lambda_e2e_update_failed_unable_to_get_id(self): - mock_lambda_payloads = { - "SEARCH": generate_lambda_payload(status_code=200, body=SearchLambdaResponseBody.id_and_version_not_found), - } - self.execute_test(Message.update_message, "Fatal", mock_lambda_payloads) + self.execute_test( + Message.update_message, + "Fatal", + mock_lambda_payloads=deepcopy(LambdaPayloads.SEARCH.ID_AND_VERSION_NOT_FOUND), + ) def test_forward_lambda_e2e_update_failed(self): - mock_diagnstics = "the provided event ID is either missing or not in the expected format." - mock_lambda_payloads = { - Operations.UPDATE: generate_lambda_payload(400, body=generate_operation_outcome(mock_diagnstics)), - "SEARCH": generate_lambda_payload(200, body=SearchLambdaResponseBody.id_and_version_found), - } - self.execute_test(Message.update_message, "Fatal Error", mock_lambda_payloads) + self.execute_test( + Message.update_message, + "Fatal Error", + mock_lambda_payloads={ + **LambdaPayloads.UPDATE.MISSING_EVENT_ID, + **LambdaPayloads.SEARCH.ID_AND_VERSION_FOUND, + }, + ) def test_forward_lambda_e2e_delete_failed(self): - mock_diagnstics = "the provided event ID is either missing or not in the expected format." - mock_lambda_payloads = { - Operations.UPDATE: generate_lambda_payload( - 404, body=generate_operation_outcome(mock_diagnstics, code="not-found") - ), - "SEARCH": generate_lambda_payload(200, body=SearchLambdaResponseBody.id_and_version_not_found), - } - self.execute_test(Message.delete_message, "Fatal Error", mock_lambda_payloads) + self.execute_test( + Message.delete_message, + "Fatal Error", + mock_lambda_payloads=deepcopy(LambdaPayloads.SEARCH.ID_AND_VERSION_NOT_FOUND), + ) @patch("utils_for_record_forwarder.lambda_client.invoke") def test_forward_lambda_e2e_none_request(self, mock_api): message = {**Message.base_message_fields, "diagnostics": "Unsupported file type received as an attachment"} - mock_lambda_payloads = {} - self.execute_test(message, "Fatal Error", mock_lambda_payloads) + self.execute_test(message, "Fatal Error", mock_lambda_payloads={}) mock_api.create_immunization.assert_not_called() def test_forward_lambda_e2e_no_permissions(self): message = {**Message.base_message_fields, "diagnostics": "No permissions for operation"} - mock_lambda_payloads = {} - self.execute_test(message, "Fatal Error", mock_lambda_payloads) + self.execute_test(message, "Fatal Error", mock_lambda_payloads={}) if __name__ == "__main__": diff --git a/recordforwarder/tests/utils_for_recordfowarder_tests/values_for_recordforwarder_tests.py b/recordforwarder/tests/utils_for_recordfowarder_tests/values_for_recordforwarder_tests.py index 8c568fb8..009e7a83 100644 --- a/recordforwarder/tests/utils_for_recordfowarder_tests/values_for_recordforwarder_tests.py +++ b/recordforwarder/tests/utils_for_recordfowarder_tests/values_for_recordforwarder_tests.py @@ -1,7 +1,10 @@ """Values for use in recordforwarder tests""" from constants import Operations -from tests.utils_for_recordfowarder_tests.utils_for_recordforwarder_tests import generate_lambda_payload +from tests.utils_for_recordfowarder_tests.utils_for_recordforwarder_tests import ( + generate_lambda_payload, + generate_operation_outcome, +) MOCK_ENVIRONMENT_DICT = { "SOURCE_BUCKET_NAME": "immunisation-batch-internal-dev-data-sources", @@ -42,6 +45,10 @@ class Urls: ODS = "https://fhir.nhs.uk/Id/ods-organization-code" +MOCK_IDENTIFIER_SYSTEM = Urls.RAVS +MOCK_IDENTIFIER_VALUE = "Vacc1" + + test_imms_fhir_json = { "resourceType": "Immunization", "contained": [ @@ -70,7 +77,7 @@ class Urls: }, } ], - "identifier": [{"system": Urls.RAVS, "value": "0001_TEST_v1_RUN_1_ABCD-123_Dose_seq_01"}], + "identifier": [{"system": MOCK_IDENTIFIER_SYSTEM, "value": MOCK_IDENTIFIER_VALUE}], "status": "completed", "vaccineCode": { "coding": [ @@ -126,6 +133,11 @@ class Diagnostics: UNABLE_TO_OBTAIN_IMMS_ID = "Unable to obtain imms event id" UNABLE_TO_OBTAIN_VERSION = "Unable to obtain current imms event version" INVALID_CONVERSION = "Unable to convert row to FHIR Immunization Resource JSON format" + DUPLICATE = f"The provided identifier: {MOCK_IDENTIFIER_SYSTEM}#{MOCK_IDENTIFIER_VALUE} is duplicated" + MISSING_EVENT_ID = "the provided event ID is either missing or not in the expected format." + VALIDATION_ERROR = ( + "Validation errors: The provided immunization id:test_id doesn't match with the content of the request body" + ) class Message: @@ -175,9 +187,58 @@ class LambdaPayloads: generate_lambda_invocation_side_effect fucntion """ - SUCCESS = { - Operations.CREATE: generate_lambda_payload(status_code=201, headers=lambda_success_headers), - Operations.UPDATE: generate_lambda_payload(status_code=200), - Operations.DELETE: generate_lambda_payload(status_code=204), - Operations.SEARCH: generate_lambda_payload(status_code=200, body=SearchLambdaResponseBody.id_and_version_found), - } + class CREATE: + """LambdaPayloads for the CREATE lambda""" + + SUCCESS = {Operations.CREATE: generate_lambda_payload(status_code=201, headers=lambda_success_headers)} + + DUPLICATE = { + Operations.CREATE: generate_lambda_payload( + status_code=422, body=generate_operation_outcome(diagnostics=Diagnostics.DUPLICATE, code="duplicate") + ) + } + + class UPDATE: + """LambdaPayloads for the UPDATE lambda""" + + SUCCESS = {Operations.UPDATE: generate_lambda_payload(status_code=200)} + + MISSING_EVENT_ID = { + Operations.UPDATE: generate_lambda_payload( + 400, body=generate_operation_outcome(Diagnostics.MISSING_EVENT_ID) + ) + } + + VALIDATION_ERROR = { + Operations.UPDATE: generate_lambda_payload( + status_code=422, body=generate_operation_outcome(Diagnostics.VALIDATION_ERROR) + ) + } + + class DELETE: + """LambdaPayloads for the DELETE lambda""" + + SUCCESS = {Operations.DELETE: generate_lambda_payload(status_code=204)} + + class SEARCH: + """LambdaPayloads for the SEARCH lambda""" + + ID_AND_VERSION_FOUND = { + Operations.SEARCH: generate_lambda_payload( + status_code=200, body=SearchLambdaResponseBody.id_and_version_found + ) + } + + ID_AND_VERSION_NOT_FOUND = { + Operations.SEARCH: generate_lambda_payload( + status_code=200, body=SearchLambdaResponseBody.id_and_version_not_found + ) + } + + FAILURE = { + Operations.SEARCH: generate_lambda_payload( + status_code=404, body=generate_operation_outcome("some_diagnostics") + ) + } + + SUCCESS = {**CREATE.SUCCESS, **UPDATE.SUCCESS, **DELETE.SUCCESS, **SEARCH.ID_AND_VERSION_FOUND} From 7d78c2b4a6f1a95b39c6a15810bced0a2ad5ad14 Mon Sep 17 00:00:00 2001 From: AlexandraBenson Date: Fri, 8 Nov 2024 15:44:47 +0000 Subject: [PATCH 20/22] further standardisation of lambda mocking --- .../tests/test_e2e_forwarding_lambda.py | 14 ++-- .../tests/test_forwarding_lambda.py | 74 +++++++++---------- .../tests/test_get_imms_id_and_version.py | 17 ++--- .../utils_for_recordforwarder_tests.py | 4 +- 4 files changed, 51 insertions(+), 58 deletions(-) diff --git a/recordforwarder/tests/test_e2e_forwarding_lambda.py b/recordforwarder/tests/test_e2e_forwarding_lambda.py index b5793e9c..763f8d44 100644 --- a/recordforwarder/tests/test_e2e_forwarding_lambda.py +++ b/recordforwarder/tests/test_e2e_forwarding_lambda.py @@ -36,6 +36,8 @@ s3_client = boto3_client("s3", region_name=AWS_REGION) kinesis_client = boto3_client("kinesis", region_name=AWS_REGION) +LAMBDA_PAYLOADS = LambdaPayloads() + @mock_s3 @patch.dict("os.environ", MOCK_ENVIRONMENT_DICT) @@ -87,7 +89,7 @@ def test_forward_lambda_e2e_successes(self): ): for message in messages: - mock_invoke.side_effect = generate_lambda_invocation_side_effect(deepcopy(LambdaPayloads.SUCCESS)) + mock_invoke.side_effect = generate_lambda_invocation_side_effect(deepcopy(LAMBDA_PAYLOADS.SUCCESS)) forward_lambda_handler(generate_kinesis_message(message), None) ack_file_obj = s3_client.get_object(Bucket=DESTINATION_BUCKET_NAME, Key=TestFile.ACK_FILE_KEY) @@ -99,7 +101,7 @@ def test_forward_lambda_e2e_successes(self): def test_forward_lambda_e2e_create_duplicate(self): self.execute_test( - Message.create_message, "Fatal Error", mock_lambda_payloads=deepcopy(LambdaPayloads.CREATE.DUPLICATE) + Message.create_message, "Fatal Error", mock_lambda_payloads=deepcopy(LAMBDA_PAYLOADS.CREATE.DUPLICATE) ) def test_forward_lambda_e2e_create_multi_line_diagnostics(self): @@ -117,7 +119,7 @@ def test_forward_lambda_e2e_update_failed_unable_to_get_id(self): self.execute_test( Message.update_message, "Fatal", - mock_lambda_payloads=deepcopy(LambdaPayloads.SEARCH.ID_AND_VERSION_NOT_FOUND), + mock_lambda_payloads=deepcopy(LAMBDA_PAYLOADS.SEARCH.ID_AND_VERSION_NOT_FOUND), ) def test_forward_lambda_e2e_update_failed(self): @@ -125,8 +127,8 @@ def test_forward_lambda_e2e_update_failed(self): Message.update_message, "Fatal Error", mock_lambda_payloads={ - **LambdaPayloads.UPDATE.MISSING_EVENT_ID, - **LambdaPayloads.SEARCH.ID_AND_VERSION_FOUND, + **deepcopy(LAMBDA_PAYLOADS.UPDATE.MISSING_EVENT_ID), + **deepcopy(LAMBDA_PAYLOADS.SEARCH.ID_AND_VERSION_FOUND), }, ) @@ -134,7 +136,7 @@ def test_forward_lambda_e2e_delete_failed(self): self.execute_test( Message.delete_message, "Fatal Error", - mock_lambda_payloads=deepcopy(LambdaPayloads.SEARCH.ID_AND_VERSION_NOT_FOUND), + mock_lambda_payloads=deepcopy(LAMBDA_PAYLOADS.SEARCH.ID_AND_VERSION_NOT_FOUND), ) @patch("utils_for_record_forwarder.lambda_client.invoke") diff --git a/recordforwarder/tests/test_forwarding_lambda.py b/recordforwarder/tests/test_forwarding_lambda.py index 481c6db0..c2a79653 100644 --- a/recordforwarder/tests/test_forwarding_lambda.py +++ b/recordforwarder/tests/test_forwarding_lambda.py @@ -20,25 +20,24 @@ sys.path.insert(0, os.path.abspath(os.path.join(maindir, SRCDIR))) from tests.utils_for_recordfowarder_tests.values_for_recordforwarder_tests import ( - lambda_success_headers, MOCK_ENVIRONMENT_DICT, AWS_REGION, - SearchLambdaResponseBody, Message, + LambdaPayloads, + Diagnostics, ) from tests.utils_for_recordfowarder_tests.utils_for_recordforwarder_tests import ( - generate_operation_outcome, - generate_lambda_payload, generate_kinesis_message, generate_lambda_invocation_side_effect, ) from forwarding_lambda import forward_lambda_handler, forward_request_to_lambda -from constants import Operations from update_ack_file import create_ack_data s3_client = boto3_client("s3", region_name=AWS_REGION) +LAMBDA_PAYLOADS = LambdaPayloads() + @mock_s3 @patch.dict("os.environ", MOCK_ENVIRONMENT_DICT) @@ -135,37 +134,35 @@ def test_create_ack_data(self): ) def test_forward_request_to_api_new_success(self): - mock_lambda_payloads = {Operations.CREATE: generate_lambda_payload(status_code=201, headers=lambda_success_headers)} - with self.common_contexts_for_forwarding_lambda_tests(mock_lambda_payloads) as mock_create_ack_data: + with self.common_contexts_for_forwarding_lambda_tests( + deepcopy(LAMBDA_PAYLOADS.SUCCESS) + ) as mock_create_ack_data: forward_request_to_lambda(deepcopy(Message.create_message)) # pylint: disable=no-member mock_create_ack_data.assert_called_with("20240821T10153000", Message.ROW_ID, True, None, "test_id") def test_forward_request_to_api_new_duplicate(self): - diagnostics = "The provided identifier: https://supplierABC/identifiers/vacc#test-identifier1 is duplicated" - mock_lambda_payloads = { - Operations.CREATE: generate_lambda_payload(status_code=422, body=generate_operation_outcome(diagnostics)) - } - with self.common_contexts_for_forwarding_lambda_tests(mock_lambda_payloads) as mock_create_ack_data: + with self.common_contexts_for_forwarding_lambda_tests( + deepcopy(LAMBDA_PAYLOADS.CREATE.DUPLICATE) + ) as mock_create_ack_data: forward_request_to_lambda(deepcopy(Message.create_message)) # pylint: disable=no-member - mock_create_ack_data.assert_called_with("20240821T10153000", Message.ROW_ID, False, diagnostics, None) + mock_create_ack_data.assert_called_with("20240821T10153000", Message.ROW_ID, False, Diagnostics.DUPLICATE, None) def test_forward_request_to_api_update_failure(self): - diagnostics = ( - "Validation errors: The provided immunization id:test_id doesn't match with the content of the request body" - ) mock_lambda_payloads = { - Operations.UPDATE: generate_lambda_payload(status_code=422, body=generate_operation_outcome(diagnostics)), - "SEARCH": generate_lambda_payload(status_code=200, body=SearchLambdaResponseBody.id_and_version_found), + **deepcopy(LAMBDA_PAYLOADS.UPDATE.VALIDATION_ERROR), + **deepcopy(LAMBDA_PAYLOADS.SEARCH.ID_AND_VERSION_FOUND), } with self.common_contexts_for_forwarding_lambda_tests(mock_lambda_payloads) as mock_create_ack_data: forward_request_to_lambda(deepcopy(Message.update_message)) # pylint: disable=no-member - mock_create_ack_data.assert_called_with("20240821T10153000", Message.ROW_ID, False, diagnostics, None) + mock_create_ack_data.assert_called_with( + "20240821T10153000", Message.ROW_ID, False, Diagnostics.VALIDATION_ERROR, None + ) def test_forward_request_to_api_update_failure_imms_id_none(self): with ( @@ -179,11 +176,9 @@ def test_forward_request_to_api_update_failure_imms_id_none(self): mock_lambda_client.assert_not_called() def test_forward_request_to_api_delete_success(self): - mock_lambda_payloads = { - Operations.DELETE: generate_lambda_payload(status_code=204), - "SEARCH": generate_lambda_payload(status_code=200, body=SearchLambdaResponseBody.id_and_version_found), - } - with self.common_contexts_for_forwarding_lambda_tests(mock_lambda_payloads) as mock_create_ack_data: + with self.common_contexts_for_forwarding_lambda_tests( + deepcopy(LAMBDA_PAYLOADS.SUCCESS) + ) as mock_create_ack_data: forward_request_to_lambda(deepcopy(Message.delete_message)) # pylint: disable=no-member @@ -191,23 +186,20 @@ def test_forward_request_to_api_delete_success(self): "20240821T10153000", Message.ROW_ID, True, None, "277befd9-574e-47fe-a6ee-189858af3bb0" ) - @patch("forwarding_lambda.forward_request_to_lambda") - def test_forward_lambda_handler(self, mock_forward_request_to_api): - message_body = deepcopy(Message.create_message) - forward_lambda_handler(generate_kinesis_message(message_body), None) - mock_forward_request_to_api.assert_called_once_with(message_body) - - @patch("forwarding_lambda.forward_request_to_lambda") - def test_forward_lambda_handler_update(self, mock_forward_request_to_api): - message_body = deepcopy(Message.update_message) - forward_lambda_handler(generate_kinesis_message(message_body), None) - mock_forward_request_to_api.assert_called_once_with(message_body) - - @patch("forwarding_lambda.logger") - def test_forward_lambda_handler_with_exception(self, mock_logger): - message_body = deepcopy(Message.create_message) - message_body["operation_requested"] = "INVALID_OPERATION" - forward_lambda_handler(generate_kinesis_message(message_body), None) + def test_forward_lambda_handler(self): + for message in [ + deepcopy(Message.create_message), + deepcopy(Message.update_message), + deepcopy(Message.delete_message), + ]: + with patch("forwarding_lambda.forward_request_to_lambda") as mock_forward_request_to_api: + forward_lambda_handler(generate_kinesis_message(message), None) + mock_forward_request_to_api.assert_called_once_with(message) + + def test_forward_lambda_handler_with_exception(self): + message_body = {**deepcopy(Message.create_message), "operation_request": "INVALID_OPERATION"} + with patch("forwarding_lambda.logger") as mock_logger: + forward_lambda_handler(generate_kinesis_message(message_body), None) mock_logger.error.assert_called() diff --git a/recordforwarder/tests/test_get_imms_id_and_version.py b/recordforwarder/tests/test_get_imms_id_and_version.py index 99a6089e..e9b6b3cf 100644 --- a/recordforwarder/tests/test_get_imms_id_and_version.py +++ b/recordforwarder/tests/test_get_imms_id_and_version.py @@ -6,15 +6,16 @@ from get_imms_id_and_version import get_imms_id_and_version from errors import IdNotFoundError from tests.utils_for_recordfowarder_tests.utils_for_recordforwarder_tests import ( - generate_operation_outcome, - generate_lambda_payload, + generate_lambda_invocation_side_effect, ) from tests.utils_for_recordfowarder_tests.values_for_recordforwarder_tests import ( - SearchLambdaResponseBody, test_imms_fhir_json, + LambdaPayloads, + MOCK_ENVIRONMENT_DICT, ) +@patch.dict("os.environ", MOCK_ENVIRONMENT_DICT) class TestGetImmsIdAndVersion(unittest.TestCase): """ Tests for get_imms_id_and_version. @@ -25,9 +26,7 @@ def test_success(self): """Test that imms_id and version are correctly identified from a successful search lambda response.""" with patch( "clients.lambda_client.invoke", - return_value=generate_lambda_payload( - status_code=200, body=deepcopy(SearchLambdaResponseBody.id_and_version_found) - ), + side_effect=generate_lambda_invocation_side_effect(deepcopy(LambdaPayloads.SEARCH.ID_AND_VERSION_FOUND)), ): imms_id, version = get_imms_id_and_version(test_imms_fhir_json) @@ -38,8 +37,8 @@ def test_failure_due_to_empty_search_lambda_return(self): """Test that an IdNotFoundError is raised for a successful search lambda response which contains no entries.""" with patch( "clients.lambda_client.invoke", - return_value=generate_lambda_payload( - status_code=200, body=deepcopy(SearchLambdaResponseBody.id_and_version_not_found) + side_effect=generate_lambda_invocation_side_effect( + deepcopy(LambdaPayloads.SEARCH.ID_AND_VERSION_NOT_FOUND) ), ): with self.assertRaises(IdNotFoundError): @@ -49,7 +48,7 @@ def test_failure_due_to_search_lambda_404(self): """Test that an IdNotFoundError is raised for an unsuccessful search lambda response.""" with patch( "clients.lambda_client.invoke", - return_value=generate_lambda_payload(status_code=404, body=generate_operation_outcome("some_diagnostics")), + side_effect=generate_lambda_invocation_side_effect(deepcopy(LambdaPayloads.SEARCH.FAILURE)), ): with self.assertRaises(IdNotFoundError): get_imms_id_and_version(test_imms_fhir_json) diff --git a/recordforwarder/tests/utils_for_recordfowarder_tests/utils_for_recordforwarder_tests.py b/recordforwarder/tests/utils_for_recordfowarder_tests/utils_for_recordforwarder_tests.py index ca58bda6..f080d007 100644 --- a/recordforwarder/tests/utils_for_recordfowarder_tests/utils_for_recordforwarder_tests.py +++ b/recordforwarder/tests/utils_for_recordfowarder_tests/utils_for_recordforwarder_tests.py @@ -55,7 +55,7 @@ def generate_lambda_invocation_side_effect(mock_lambda_payloads): """ def lambda_invocation_side_effect(FunctionName, *_args, **_kwargs): # pylint: disable=invalid-name - lambda_type = FunctionName.split("_")[1] # Tests mock FunctionNames as mock_lambdatype_lambda_name - return mock_lambda_payloads[lambda_type.upper()] + lambda_type = FunctionName.split("_")[1].upper() # Tests mock FunctionNames as mock_lambdatype_lambda_name + return mock_lambda_payloads[lambda_type] return lambda_invocation_side_effect From 5bd79a5d8e263d5daffbe97c05fb326f81d082d0 Mon Sep 17 00:00:00 2001 From: AlexandraBenson Date: Wed, 13 Nov 2024 13:46:08 +0000 Subject: [PATCH 21/22] Fix liniting issues --- recordforwarder/tests/test_log_structure.py | 8 +++++--- recordforwarder/tests/test_utils_for_recordforwarder.py | 3 ++- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/recordforwarder/tests/test_log_structure.py b/recordforwarder/tests/test_log_structure.py index cb1c40cd..e678363e 100644 --- a/recordforwarder/tests/test_log_structure.py +++ b/recordforwarder/tests/test_log_structure.py @@ -47,7 +47,8 @@ # # Prepare expected_log_json # expected_log_json = ( -# deepcopy(self.expected_log_json_success) if not expected_error else deepcopy(self.expected_log_json_failure) +# deepcopy(self.expected_log_json_success) if not expected_error else +# deepcopy(self.expected_log_json_failure) # ) # expected_log_json["action_flag"] = operation.upper() # expected_log_json.update({"error": expected_error} if expected_error else {}) @@ -64,7 +65,7 @@ # Yields mock_firehose_logger and logs (where logs is a list of the captured log entries). # """ # with ExitStack() as stack: -# stack.enter_context(patch("time.time", side_effect=(1000000.0, 1000001.0, 1000003.0))) # (start, end, ???) +# stack.enter_context(patch("time.time", side_effect=(1000000.0, 1000001.0, 1000003.0)))# (start, end, ???) # stack.enter_context(patch("log_structure.datetime")) # stack.enter_context(patch("log_structure.datetime.now", return_value=FIXED_DATETIME)) # mock_firehose_logger = stack.enter_context(patch("log_structure.firehose_logger")) @@ -93,7 +94,8 @@ # self.common_contexts_for_splunk_logging_tests() as (mock_firehose_logger, logs), # noqa: E999 # self.assertRaises(MessageNotSuccessfulError) as context, # ): -# message_body = {**Message.base_message_fields, "operation_requested": operation, "diagnostics": diagnostics} +# message_body = {**Message.base_message_fields, "operation_requested": operation, +# "diagnostics": diagnostics} # send_request_to_lambda(message_body) # self.assertEqual(str(context.exception), diagnostics) diff --git a/recordforwarder/tests/test_utils_for_recordforwarder.py b/recordforwarder/tests/test_utils_for_recordforwarder.py index 3bceaeac..f63af20f 100644 --- a/recordforwarder/tests/test_utils_for_recordforwarder.py +++ b/recordforwarder/tests/test_utils_for_recordforwarder.py @@ -3,7 +3,8 @@ from unittest import TestCase from unittest.mock import patch from utils_for_record_forwarder import get_environment -from constants import ACK_HEADERS + +# from constants import ACK_HEADERS # from update_ack_file import create_ack_data From 8a7dc86cfe4a9c1849ed0a5ea6335d342177029d Mon Sep 17 00:00:00 2001 From: AlexandraBenson Date: Wed, 13 Nov 2024 16:31:53 +0000 Subject: [PATCH 22/22] Remove ack headers from constants --- recordforwarder/src/constants.py | 17 ----------------- .../src/utils_for_record_forwarder.py | 2 +- 2 files changed, 1 insertion(+), 18 deletions(-) diff --git a/recordforwarder/src/constants.py b/recordforwarder/src/constants.py index 9d2505ae..bf4fdf00 100644 --- a/recordforwarder/src/constants.py +++ b/recordforwarder/src/constants.py @@ -1,22 +1,5 @@ """Constants for recordforwarder""" -ACK_HEADERS = [ - "MESSAGE_HEADER_ID", - "HEADER_RESPONSE_CODE", - "ISSUE_SEVERITY", - "ISSUE_CODE", - "ISSUE_DETAILS_CODE", - "RESPONSE_TYPE", - "RESPONSE_CODE", - "RESPONSE_DISPLAY", - "RECEIVED_TIME", - "MAILBOX_FROM", - "LOCAL_ID", - "IMMS_ID", - "OPERATION_OUTCOME", - "MESSAGE_DELIVERY", -] - IMMS_BATCH_APP_NAME = "Imms-Batch-App" diff --git a/recordforwarder/src/utils_for_record_forwarder.py b/recordforwarder/src/utils_for_record_forwarder.py index 2878fd41..408efc68 100644 --- a/recordforwarder/src/utils_for_record_forwarder.py +++ b/recordforwarder/src/utils_for_record_forwarder.py @@ -37,7 +37,7 @@ def invoke_lambda(lambda_name: str, payload: dict) -> Union[tuple[int, dict, str Returns the ressponse status code, body (loaded in as a dictionary) and headers. """ # Change InvocationType to 'Event' for asynchronous invocation - if "search_imms" in lambda_name or "update" in lambda_name or "delete" in lambda_name: + if "search_imms" in lambda_name: response = lambda_client.invoke( FunctionName=lambda_name, InvocationType="RequestResponse", Payload=json.dumps(payload) )