diff --git a/infrastructure/instance/file_name_processor.tf b/infrastructure/instance/file_name_processor.tf index 64345fcfdd..d8733998a2 100644 --- a/infrastructure/instance/file_name_processor.tf +++ b/infrastructure/instance/file_name_processor.tf @@ -277,6 +277,7 @@ resource "aws_lambda_function" "file_processor_lambda" { environment { variables = { + ACCOUNT_ID = var.immunisation_account_id SOURCE_BUCKET_NAME = aws_s3_bucket.batch_data_source_bucket.bucket ACK_BUCKET_NAME = aws_s3_bucket.batch_data_destination_bucket.bucket QUEUE_URL = aws_sqs_queue.batch_file_created.url diff --git a/lambdas/filenameprocessor/src/constants.py b/lambdas/filenameprocessor/src/constants.py index 7388b0c3ef..da99debd28 100644 --- a/lambdas/filenameprocessor/src/constants.py +++ b/lambdas/filenameprocessor/src/constants.py @@ -11,12 +11,14 @@ ) SOURCE_BUCKET_NAME = os.getenv("SOURCE_BUCKET_NAME") +DPS_DESTINATION_BUCKET_NAME = os.getenv("ACK_BUCKET_NAME") AUDIT_TABLE_NAME = os.getenv("AUDIT_TABLE_NAME") AUDIT_TABLE_TTL_DAYS = os.getenv("AUDIT_TABLE_TTL_DAYS") VALID_VERSIONS = ["V5"] VACCINE_TYPE_TO_DISEASES_HASH_KEY = "vacc_to_diseases" ODS_CODE_TO_SUPPLIER_SYSTEM_HASH_KEY = "ods_code_to_supplier" +EXTENDED_ATTRIBUTES_PREFIXES = "Vaccination_Extended_Attributes" ERROR_TYPE_TO_STATUS_CODE_MAP = { VaccineTypePermissionsError: 403, diff --git a/lambdas/filenameprocessor/src/file_name_processor.py b/lambdas/filenameprocessor/src/file_name_processor.py index 5588f81eb0..a3ba91ce10 100644 --- a/lambdas/filenameprocessor/src/file_name_processor.py +++ b/lambdas/filenameprocessor/src/file_name_processor.py @@ -10,17 +10,19 @@ from uuid import uuid4 from audit_table import upsert_audit_table -from common.aws_s3_utils import move_file +from common.aws_s3_utils import move_file, move_file_outside_bucket from common.clients import STREAM_NAME, get_s3_client, logger from common.log_decorator import logging_decorator from common.models.errors import UnhandledAuditTableError from constants import ( + DPS_DESTINATION_BUCKET_NAME, ERROR_TYPE_TO_STATUS_CODE_MAP, + EXTENDED_ATTRIBUTES_PREFIXES, SOURCE_BUCKET_NAME, FileNotProcessedReason, FileStatus, ) -from file_validation import is_file_in_directory_root, validate_file_key +from file_validation import is_file_in_directory_root, validate_batch_file_key, validate_extended_attributes_file_key from make_and_upload_ack_file import make_and_upload_the_ack_file from models.errors import ( InvalidFileKeyError, @@ -77,38 +79,58 @@ def handle_record(record) -> dict: s3_response = get_s3_client().get_object(Bucket=bucket_name, Key=file_key) created_at_formatted_string, expiry_timestamp = get_creation_and_expiry_times(s3_response) - vaccine_type, supplier = validate_file_key(file_key) - permissions = validate_vaccine_type_permissions(vaccine_type=vaccine_type, supplier=supplier) - - queue_name = f"{supplier}_{vaccine_type}" - upsert_audit_table( - message_id, - file_key, - created_at_formatted_string, - expiry_timestamp, - queue_name, - FileStatus.QUEUED, - ) - make_and_send_sqs_message( - file_key, - message_id, - permissions, - vaccine_type, - supplier, - created_at_formatted_string, - ) - - logger.info("Lambda invocation successful for file '%s'", file_key) - - # Return details for logs - return { - "statusCode": 200, - "message": "Successfully sent to SQS for further processing", - "file_key": file_key, - "message_id": message_id, - "vaccine_type": vaccine_type, - "supplier": supplier, - } + if file_key.startswith(EXTENDED_ATTRIBUTES_PREFIXES): + extended_attribute_identifier = validate_extended_attributes_file_key(file_key) + move_file_outside_bucket(bucket_name, file_key, DPS_DESTINATION_BUCKET_NAME, f"archive/{file_key}") + queue_name = extended_attribute_identifier + + upsert_audit_table( + message_id, + file_key, + created_at_formatted_string, + expiry_timestamp, + queue_name, + FileStatus.PROCESSING, + ) + return { + "statusCode": 200, + "message": "Extended Attributes file successfully processed", + "file_key": file_key, + "message_id": message_id, + "queue_name": queue_name, + } + else: + vaccine_type, supplier = validate_batch_file_key(file_key) + permissions = validate_vaccine_type_permissions(vaccine_type=vaccine_type, supplier=supplier) + queue_name = f"{supplier}_{vaccine_type}" + upsert_audit_table( + message_id, + file_key, + created_at_formatted_string, + expiry_timestamp, + queue_name, + FileStatus.QUEUED, + ) + make_and_send_sqs_message( + file_key, + message_id, + permissions, + vaccine_type, + supplier, + created_at_formatted_string, + ) + + logger.info("Lambda invocation successful for file '%s'", file_key) + + # Return details for logs + return { + "statusCode": 200, + "message": "Successfully sent to SQS for further processing", + "file_key": file_key, + "message_id": message_id, + "vaccine_type": vaccine_type, + "supplier": supplier, + } except ( # pylint: disable=broad-exception-caught VaccineTypePermissionsError, @@ -140,6 +162,16 @@ def handle_record(record) -> dict: move_file(bucket_name, file_key, f"archive/{file_key}") # Return details for logs + if file_key.startswith(EXTENDED_ATTRIBUTES_PREFIXES): + extended_attribute_identifier = validate_extended_attributes_file_key(file_key) + return { + "statusCode": ERROR_TYPE_TO_STATUS_CODE_MAP.get(type(error), 500), + "message": "Infrastructure Level Response Value - Processing Error", + "file_key": file_key, + "message_id": message_id, + "vaccine_supplier_info": extended_attribute_identifier, + "error": str(error), + } return { "statusCode": ERROR_TYPE_TO_STATUS_CODE_MAP.get(type(error), 500), "message": "Infrastructure Level Response Value - Processing Error", @@ -163,21 +195,36 @@ def handle_unexpected_bucket_name(bucket_name: str, file_key: str) -> dict: """Handles scenario where Lambda was not invoked by the data-sources bucket. Should not occur due to terraform config and overarching design""" try: - vaccine_type, supplier = validate_file_key(file_key) - logger.error( - "Unable to process file %s due to unexpected bucket name %s", - file_key, - bucket_name, - ) - message = f"Failed to process file due to unexpected bucket name {bucket_name}" - - return { - "statusCode": 500, - "message": message, - "file_key": file_key, - "vaccine_type": vaccine_type, - "supplier": supplier, - } + if file_key.startswith(EXTENDED_ATTRIBUTES_PREFIXES): + extended_attribute_identifier = validate_extended_attributes_file_key(file_key) + logger.error( + "Unable to process file %s due to unexpected bucket name %s", + file_key, + bucket_name, + ) + message = f"Failed to process file due to unexpected bucket name {bucket_name}" + return { + "statusCode": 500, + "message": message, + "file_key": file_key, + "vaccine_supplier_info": extended_attribute_identifier, + } + else: + vaccine_type, supplier = validate_batch_file_key(file_key) + logger.error( + "Unable to process file %s due to unexpected bucket name %s", + file_key, + bucket_name, + ) + message = f"Failed to process file due to unexpected bucket name {bucket_name}" + + return { + "statusCode": 500, + "message": message, + "file_key": file_key, + "vaccine_type": vaccine_type, + "supplier": supplier, + } except Exception as error: logger.error( diff --git a/lambdas/filenameprocessor/src/file_validation.py b/lambdas/filenameprocessor/src/file_validation.py index b13a23c43b..19827a1cc8 100644 --- a/lambdas/filenameprocessor/src/file_validation.py +++ b/lambdas/filenameprocessor/src/file_validation.py @@ -37,7 +37,20 @@ def is_valid_datetime(timestamp: str) -> bool: return True -def validate_file_key(file_key: str) -> tuple[str, str]: +def validate_extended_attributes_file_key(file_key: str) -> str: + """ + Checks that all elements of the file key are valid, raises an exception otherwise. + Returns a string containing the organization code and COVID vaccine type needed in the audit table. + """ + if not match(r"^[^_.]*_[^_.]*_[^_.]*_[^_.]*_[^_.]*_[^_.]*_[^_.]*", file_key): + raise InvalidFileKeyError("Initial file validation failed: invalid extended attributes file key format") + + file_key_parts_without_extension, _ = split_file_key(file_key) + organization_code = file_key_parts_without_extension[5] + return f"{organization_code}_COVID" + + +def validate_batch_file_key(file_key: str) -> tuple[str, str]: """ Checks that all elements of the file key are valid, raises an exception otherwise. Returns a tuple containing the vaccine_type and supplier (both converted to upper case). @@ -46,20 +59,14 @@ def validate_file_key(file_key: str) -> tuple[str, str]: if not match(r"^[^_.]*_[^_.]*_[^_.]*_[^_.]*_[^_.]*", file_key): raise InvalidFileKeyError("Initial file validation failed: invalid file key format") - file_key = file_key.upper() - file_name_and_extension = file_key.rsplit(".", 1) - - if len(file_name_and_extension) != 2: - raise InvalidFileKeyError("Initial file validation failed: missing file extension") - - file_key_parts_without_extension = file_name_and_extension[0].split("_") + file_key_parts_without_extension, file_name_and_extension = split_file_key(file_key) vaccine_type = file_key_parts_without_extension[0] vaccination = file_key_parts_without_extension[1] version = file_key_parts_without_extension[2] ods_code = file_key_parts_without_extension[3] timestamp = file_key_parts_without_extension[4] - extension = file_name_and_extension[1] + extension = file_name_and_extension supplier = get_supplier_system_from_cache(ods_code) valid_vaccine_types = get_valid_vaccine_types_from_cache() @@ -76,3 +83,13 @@ def validate_file_key(file_key: str) -> tuple[str, str]: raise InvalidFileKeyError("Initial file validation failed: invalid file key") return vaccine_type, supplier + + +def split_file_key(file_key: str) -> tuple[list[str], str]: + file_key = file_key.upper() + file_name_and_extension = file_key.rsplit(".", 1) + + if len(file_name_and_extension) != 2: + raise InvalidFileKeyError("Initial file validation failed: missing file extension") + + return file_name_and_extension[0].split("_"), file_name_and_extension[1] diff --git a/lambdas/filenameprocessor/tests/test_file_key_validation.py b/lambdas/filenameprocessor/tests/test_file_key_validation.py index a5b2e7de97..4ba2649085 100644 --- a/lambdas/filenameprocessor/tests/test_file_key_validation.py +++ b/lambdas/filenameprocessor/tests/test_file_key_validation.py @@ -15,7 +15,9 @@ from file_validation import ( is_file_in_directory_root, is_valid_datetime, - validate_file_key, + split_file_key, + validate_batch_file_key, + validate_extended_attributes_file_key, ) from models.errors import InvalidFileKeyError @@ -63,7 +65,7 @@ def test_is_valid_datetime(self, _): with self.subTest(): self.assertEqual(is_valid_datetime(date_time_string), expected_result) - def test_validate_file_key(self, mock_get_redis_client): + def test_validate_batch_file_key(self, mock_get_redis_client): """Tests that file_key_validation returns True if all elements pass validation, and False otherwise""" # Test case tuples are structured as (file_key, expected_result) test_cases_for_success_scenarios = [ @@ -90,10 +92,50 @@ def test_validate_file_key(self, mock_get_redis_client): mock_redis.hkeys.return_value = ["FLU", "RSV"] mock_get_redis_client.return_value = mock_redis - self.assertEqual(validate_file_key(file_key), expected_result) + self.assertEqual(validate_batch_file_key(file_key), expected_result) mock_redis.hkeys.assert_called_with("vacc_to_diseases") mock_redis.hget.assert_called_with("ods_code_to_supplier", ods_code) + def test_split_file_key(self, _): + """Tests that split_file_key splits the file key into parts correctly""" + test_cases = [ + ( + "FLU_Vaccinations_V5_YGM41_20000101T00000001.csv", + (["FLU", "VACCINATIONS", "V5", "YGM41", "20000101T00000001"], "CSV"), + ), + ( + "Vaccination_Extended_Attributes_V1_5_X8E5B_20000101T00000001.csv", + (["VACCINATION", "EXTENDED", "ATTRIBUTES", "V1", "5", "X8E5B", "20000101T00000001"], "CSV"), + ), + ] + + for file_key, expected in test_cases: + with self.subTest(f"SubTest for file key: {file_key}"): + self.assertEqual(split_file_key(file_key), expected) + + def test_validate_extended_attributes_file_key(self, _): + """Tests that validate_extended_attributes_file_key returns organization code and COVID vaccine type if all + elements pass validation, and raises an exception otherwise""" + test_cases_for_success_scenarios = [ + # Valid extended attributes file key + ( + "Vaccination_Extended_Attributes_v1_5_X8E5B_20000101T00000001.csv", + "X8E5B_COVID", + ), + # Valid extended attributes file key with different organization code + ( + "Vaccination_Extended_Attributes_v1_5_YGM41_20221231T23595999.csv", + "YGM41_COVID", + ), + ] + + for file_key, expected_result in test_cases_for_success_scenarios: + with self.subTest(f"SubTest for file key: {file_key}"): + self.assertEqual( + validate_extended_attributes_file_key(file_key), + expected_result, + ) + def test_validate_file_key_false(self, mock_get_redis_client): """Tests that file_key_validation returns False if elements do not pass validation""" invalid_file_key_error_message = "Initial file validation failed: invalid file key" @@ -169,7 +211,7 @@ def test_validate_file_key_false(self, mock_get_redis_client): mock_get_redis_client.return_value = mock_redis with self.assertRaises(InvalidFileKeyError) as context: - validate_file_key(file_key) + validate_batch_file_key(file_key) self.assertEqual(str(context.exception), expected_result) mock_redis.hkeys.assert_called_with("vacc_to_diseases") @@ -207,6 +249,6 @@ def test_validate_file_key_invalid(self, mock_get_redis_client): mock_get_redis_client.return_value = mock_redis with self.assertRaises(InvalidFileKeyError) as context: - validate_file_key(file_key) + validate_batch_file_key(file_key) self.assertEqual(str(context.exception), expected_result) mock_redis.hkeys.assert_not_called() diff --git a/lambdas/filenameprocessor/tests/test_lambda_handler.py b/lambdas/filenameprocessor/tests/test_lambda_handler.py index 9c4539d73c..5a44b52634 100644 --- a/lambdas/filenameprocessor/tests/test_lambda_handler.py +++ b/lambdas/filenameprocessor/tests/test_lambda_handler.py @@ -28,6 +28,7 @@ MOCK_BATCH_FILE_CONTENT, MOCK_CREATED_AT_FORMATTED_STRING, MOCK_EXPIRES_AT, + MOCK_EXTENDED_ATTRIBUTES_FILE_CONTENT, MockFileDetails, ) @@ -239,6 +240,67 @@ def test_lambda_handler_non_root_file(self): self.assert_no_sqs_message() self.assert_no_ack_file(file_details) + def test_lambda_handler_extended_attributes_success(self): + """ + Tests that for an extended attributes file (prefix starts with 'Vaccination_Extended_Attributes'): + * The file is added to the audit table with a status of 'Processing' + * The queue_name stored is the extended attribute identifier + * The file is moved to the destination bucket under archive/ + * No SQS message is sent + * No ack file is created + """ + + # Build an extended attributes file. + # FileDetails supports this when vaccine_type starts with 'Vaccination_Extended_Attributes'. + test_cases = [MockFileDetails.extended_attributes_file] + + # Put file in source bucket + s3_client.put_object( + Bucket=BucketNames.SOURCE, + Key=test_cases[0].file_key, + Body=MOCK_EXTENDED_ATTRIBUTES_FILE_CONTENT, + ) + + # Patch uuid4 (message id), the identifier extraction, and prevent external copy issues by simulating move + with ( + patch("file_name_processor.uuid4", return_value=test_cases[0].message_id), + patch( + "file_name_processor.validate_extended_attributes_file_key", + return_value=test_cases[0].ods_code + "_COVID", + ), + patch( + "file_name_processor.move_file_outside_bucket", + side_effect=lambda src_bucket, key, dst_bucket, dst_key: ( + s3_client.put_object( + Bucket=BucketNames.DESTINATION, + Key=dst_key, + Body=s3_client.get_object(Bucket=src_bucket, Key=key)["Body"].read(), + ), + s3_client.delete_object(Bucket=src_bucket, Key=key), + ), + ), + ): + lambda_handler(self.make_event([self.make_record(test_cases[0].file_key)]), None) + + # Assert audit table entry captured with Processing and queue_name set to the identifier + table_items = self.get_audit_table_items() + self.assertEqual(len(table_items), 1) + item = table_items[0] + self.assertEqual(item[AuditTableKeys.MESSAGE_ID]["S"], test_cases[0].message_id) + self.assertEqual(item[AuditTableKeys.FILENAME]["S"], test_cases[0].file_key) + self.assertEqual(item[AuditTableKeys.QUEUE_NAME]["S"], test_cases[0].ods_code + "_COVID") + self.assertEqual(item[AuditTableKeys.TIMESTAMP]["S"], test_cases[0].created_at_formatted_string) + self.assertEqual(item[AuditTableKeys.EXPIRES_AT]["N"], str(test_cases[0].expires_at)) + # File should be moved to destination under archive/ + dest_key = f"archive/{test_cases[0].file_key}" + print(f" destination file is at {s3_client.list_objects(Bucket=BucketNames.DESTINATION)}") + retrieved = s3_client.get_object(Bucket=BucketNames.DESTINATION, Key=dest_key) + self.assertIsNotNone(retrieved) + + # No SQS and no ack file + self.assert_no_sqs_message() + self.assert_no_ack_file(test_cases[0]) + @patch("elasticache.get_redis_client") def test_lambda_invalid_file_key_no_other_files_in_queue(self, mock_get_redis_client): """ diff --git a/lambdas/filenameprocessor/tests/utils_for_tests/values_for_tests.py b/lambdas/filenameprocessor/tests/utils_for_tests/values_for_tests.py index 7020fa574c..c3a5325681 100644 --- a/lambdas/filenameprocessor/tests/utils_for_tests/values_for_tests.py +++ b/lambdas/filenameprocessor/tests/utils_for_tests/values_for_tests.py @@ -27,10 +27,17 @@ class FileDetails: vaccine type. """ - def __init__(self, supplier: str, vaccine_type: str, ods_code: str, file_number: int = 1): - self.vaccine_type = vaccine_type.upper() - self.ods_code = ods_code.upper() - self.supplier = supplier.upper() + def __init__( + self, + supplier: str = "RAVS", + vaccine_type: str = None, + ods_code: str = None, + file_number: int = 1, + organization_code: str = None, + ): + self.vaccine_type = vaccine_type.upper() if vaccine_type else None + self.ods_code = ods_code.upper() if ods_code else "X8E5B" + self.supplier = supplier.upper() if supplier else None self.queue_name = f"{self.supplier}_{self.vaccine_type}" self.created_at_formatted_string = f"200{file_number}0101T00000000" @@ -39,7 +46,14 @@ def __init__(self, supplier: str, vaccine_type: str, ods_code: str, file_number: self.name = f"{self.vaccine_type}/ {self.supplier} file" file_date_and_time_string = f"20000101T0000000{file_number}" - self.file_key = f"{vaccine_type}_Vaccinations_v5_{ods_code}_{file_date_and_time_string}.csv" + extended_attributes_prefix = "Vaccination_Extended_Attributes" + if vaccine_type.startswith(extended_attributes_prefix): + file_key = f"{extended_attributes_prefix}_v1_5_{organization_code}_{file_date_and_time_string}.csv" + else: + file_key = f"{vaccine_type}_Vaccinations_v5_{ods_code}_{file_date_and_time_string}.csv" + + self.file_key = file_key + self.ack_file_key = f"ack/{self.file_key[:-4]}_InfAck_{self.created_at_formatted_string}.csv" self.permissions_list = [f"{self.vaccine_type}_FULL"] @@ -78,6 +92,10 @@ class MockFileDetails: emis_flu = FileDetails("EMIS", "FLU", "YGM41") emis_rsv = FileDetails("EMIS", "RSV", "YGM41") ravs_flu = FileDetails("RAVS", "FLU", "X8E5B") + extended_attributes_file = FileDetails( + vaccine_type="Vaccination_Extended_Attributes", file_number=1, organization_code="RJ123" + ) + # extended_attributes_file = "Vaccination_Extended_Attributes_v1_5_RJ123_20000101T00000001.csv" MOCK_FILE_HEADERS = ( @@ -103,4 +121,9 @@ class MockFileDetails: '"J82067"|"https://fhir.nhs.uk/Id/ods-organization-code"' ) +MOCK_EXTENDED_ATTRIBUTES_FILE_DATA = '"VACCINATION_UNIQUE_ID"|"VACCINATION_UNIQUE_ID_URI"|"ACTION_FLAG"|"ATTRIBUTE_ID"|"ATTRIBUTE_DISPLAYED_TEXT"|"ATTRIBUTE_VALUE"|"RECORDED_DATE"' + +MOCK_EXTENDED_ATTRIBUTES_FILE_DATA = '"e045626e-4dc5-4df3-bc35-da25263f901e"|"https://supplierABC/identifiers/vacc"|"new"|"003"|"Are you a health care worker?"|"NOT_SPECIFIED"|"20210801"' + MOCK_BATCH_FILE_CONTENT = MOCK_FILE_HEADERS + MOCK_FILE_DATA +MOCK_EXTENDED_ATTRIBUTES_FILE_CONTENT = MOCK_FILE_HEADERS + MOCK_EXTENDED_ATTRIBUTES_FILE_DATA diff --git a/lambdas/shared/src/common/aws_s3_utils.py b/lambdas/shared/src/common/aws_s3_utils.py index 6231872958..fe9d039084 100644 --- a/lambdas/shared/src/common/aws_s3_utils.py +++ b/lambdas/shared/src/common/aws_s3_utils.py @@ -1,7 +1,12 @@ """Non-imms Utility Functions""" +import os + from common.clients import get_s3_client, logger +EXPECTED_BUCKET_OWNER_ACCOUNT = os.getenv("ACCOUNT_ID") +DSP_DESTINATION_BUCKET_NAME = os.getenv("ACK_BUCKET_NAME") + def move_file(bucket_name: str, source_file_key: str, destination_file_key: str) -> None: """Moves a file from one location to another within a single S3 bucket by copying and then deleting the file.""" @@ -13,3 +18,19 @@ def move_file(bucket_name: str, source_file_key: str, destination_file_key: str) ) s3_client.delete_object(Bucket=bucket_name, Key=source_file_key) logger.info("File moved from %s to %s", source_file_key, destination_file_key) + + +def move_file_outside_bucket(source_bucket: str, source_key: str, destination_bucket: str, destination_key: str) -> None: + s3_client = get_s3_client() + s3_client.copy_object( + CopySource={"Bucket": source_bucket, "Key": source_key}, + Bucket=destination_bucket, + Key=destination_key, + ExpectedBucketOwner=EXPECTED_BUCKET_OWNER_ACCOUNT, + ExpectedSourceBucketOwner=EXPECTED_BUCKET_OWNER_ACCOUNT, + ) + s3_client.delete_object( + Bucket=source_bucket, + Key=source_key, + ExpectedBucketOwner=EXPECTED_BUCKET_OWNER_ACCOUNT, + ) diff --git a/lambdas/shared/tests/test_common/test_s3_utils.py b/lambdas/shared/tests/test_common/test_s3_utils.py new file mode 100644 index 0000000000..c09d6a6be8 --- /dev/null +++ b/lambdas/shared/tests/test_common/test_s3_utils.py @@ -0,0 +1,105 @@ +"""Tests for common.aws_s3_utils S3 helper functions using shared resources""" + +import unittest +from unittest.mock import patch + +import boto3 +from moto import mock_aws + +from common import aws_s3_utils + + +@mock_aws +class TestS3UtilsShared(unittest.TestCase): + def setUp(self): + # Ensure environment variables used by aws_s3_utils are present + self.env_patch = patch.dict( + "os.environ", + { + "ACCOUNT_ID": "123456789012", + "DESTINATION_BUCKET_NAME": "immunisation-batch-internal-dev-data-destinations", + }, + ) + self.env_patch.start() + + # Region alignment with project defaults + self.s3 = boto3.client("s3", region_name="eu-west-2") + + # Buckets for single-bucket and cross-bucket operations + self.single_bucket = "move-bucket" + self.source_bucket = "immunisation-batch-internal-dev-data-sources" + self.destination_bucket = "immunisation-batch-internal-dev-data-destinations" + + for b in (self.single_bucket, self.source_bucket, self.destination_bucket): + self.s3.create_bucket( + Bucket=b, + CreateBucketConfiguration={"LocationConstraint": "eu-west-2"}, + ) + + # Ensure module-level variables are populated even if module was imported earlier + aws_s3_utils.EXPECTED_BUCKET_OWNER_ACCOUNT = "123456789012" + aws_s3_utils.DESTINATION_BUCKET_NAME = self.destination_bucket + + # Mock logger.info to verify log calls for move_file + self.logger_info_patcher = patch("logging.Logger.info") + self.mock_logger_info = self.logger_info_patcher.start() + + def tearDown(self): + # Clean up created objects and buckets + for bucket in (self.single_bucket, self.source_bucket, self.destination_bucket): + # Delete objects if any + resp = self.s3.list_objects_v2(Bucket=bucket) + for obj in resp.get("Contents", []): + self.s3.delete_object(Bucket=bucket, Key=obj["Key"]) + # Delete bucket + self.s3.delete_bucket(Bucket=bucket) + + self.logger_info_patcher.stop() + self.env_patch.stop() + + def test_move_file_within_bucket(self): + """move_file should copy and delete within the same bucket and log the move.""" + file_key = "src/move_file_test.csv" + dest_key = "dest/move_file_test.csv" + self.s3.put_object(Bucket=self.single_bucket, Key=file_key, Body=b"dummy content") + + aws_s3_utils.move_file(self.single_bucket, file_key, dest_key) + + # Destination has object + response = self.s3.get_object(Bucket=self.single_bucket, Key=dest_key) + self.assertEqual(response["Body"].read(), b"dummy content") + # Source deleted + with self.assertRaises(self.s3.exceptions.NoSuchKey): + self.s3.get_object(Bucket=self.single_bucket, Key=file_key) + # Logger called + self.mock_logger_info.assert_called_with("File moved from %s to %s", file_key, dest_key) + + def test_move_file_outside_bucket_copies_then_deletes(self): + """File should be copied to destination bucket under destination_key and removed from source bucket.""" + source_key = "RSV_Vaccinations_v5_X8E5B_20000101T00000001.csv" + destination_key = f"archive/{source_key}" + + # Put an object in the source bucket + body_content = b"dummy file content" + self.s3.put_object(Bucket=self.source_bucket, Key=source_key, Body=body_content) + + src_obj = self.s3.get_object(Bucket=self.source_bucket, Key=source_key) + self.assertEqual(src_obj["Body"].read(), body_content) + with self.assertRaises(self.s3.exceptions.NoSuchKey): + self.s3.get_object(Bucket=self.destination_bucket, Key=destination_key) + + # Execute move across buckets + aws_s3_utils.move_file_outside_bucket( + source_bucket=self.source_bucket, + source_key=source_key, + destination_bucket=self.destination_bucket, + destination_key=destination_key, + ) + + # Assert destination has the object + dest_obj = self.s3.get_object(Bucket=self.destination_bucket, Key=destination_key) + self.assertEqual(dest_obj["Body"].read(), body_content) + + # Assert source object was deleted + with self.assertRaises(self.s3.exceptions.NoSuchKey): + self.s3.get_object(Bucket=self.source_bucket, Key=source_key) diff --git a/lambdas/shared/tests/test_common/test_utils.py b/lambdas/shared/tests/test_common/test_utils.py deleted file mode 100644 index 6aa6af7780..0000000000 --- a/lambdas/shared/tests/test_common/test_utils.py +++ /dev/null @@ -1,46 +0,0 @@ -import unittest -from unittest.mock import patch - -import boto3 -from moto import mock_aws - -from common.aws_s3_utils import move_file - - -@mock_aws -class TestUtils(unittest.TestCase): - def setUp(self): - self.bucket_name = "move-bucket" - self.s3_client = boto3.client("s3", region_name="eu-west-2") - self.s3_client.create_bucket( - Bucket=self.bucket_name, - CreateBucketConfiguration={"LocationConstraint": "eu-west-2"}, - ) - - self.logger_info_patcher = patch("logging.Logger.info") - self.mock_logger_info = self.logger_info_patcher.start() - - def tearDown(self): - for obj in self.s3_client.list_objects_v2(Bucket=self.bucket_name).get("Contents", []): - self.s3_client.delete_object(Bucket=self.bucket_name, Key=obj["Key"]) - self.s3_client.delete_bucket(Bucket=self.bucket_name) - self.logger_info_patcher.stop() - - def test_move_file(self): - """VED-167 test that the file has been moved to the appropriate location""" - bucket_name = self.bucket_name - file_key = "src/move_file_test.csv" - dest_key = "dest/move_file_test.csv" - self.s3_client.put_object(Bucket=bucket_name, Key=file_key, Body="dummy content") - move_file(bucket_name, file_key, dest_key) - # Assert the destination object exists - response = self.s3_client.get_object(Bucket=bucket_name, Key=dest_key) - content = response["Body"].read().decode() - self.assertEqual(content, "dummy content") - - # Assert the source object no longer exists - with self.assertRaises(self.s3_client.exceptions.NoSuchKey): - self.s3_client.get_object(Bucket=bucket_name, Key=file_key) - - # Logger assertion (if logger is mocked) - self.mock_logger_info.assert_called_with("File moved from %s to %s", file_key, dest_key) diff --git a/tests/e2e_batch/scenarios.py b/tests/e2e_batch/scenarios.py index 0521464580..1fbaf82204 100644 --- a/tests/e2e_batch/scenarios.py +++ b/tests/e2e_batch/scenarios.py @@ -196,19 +196,6 @@ def generate_csv_files(test_cases: list[TestCase]) -> list[TestCase]: TestAction(ActionFlag.DELETE_LOGICAL), ], }, - { - "name": "Failed Update", - "description": "Failed Update - resource does not exist", - "ods_vax": TestPair.V0V8L_3IN1_CRUDS, - "actions": [ - TestAction( - ActionFlag.UPDATE, - expected_header_response_code=BusRowResult.FATAL_ERROR, - expected_operation_outcome=OperationOutcome.IMMS_NOT_FOUND, - ) - ], - "operation_outcome": ActionFlag.NONE, - }, { "name": "Failed Delete", "description": "Failed Delete - resource does not exist", diff --git a/tests/e2e_batch/test_e2e_batch.py b/tests/e2e_batch/test_e2e_batch.py index 63a45a077c..40b067fdeb 100644 --- a/tests/e2e_batch/test_e2e_batch.py +++ b/tests/e2e_batch/test_e2e_batch.py @@ -38,7 +38,6 @@ def setUp(self): "Successful Update", "Successful Delete", "Create with 1252 char", - "Failed Update", "Failed Delete", ], ) diff --git a/tests/e2e_batch/vax_suppliers.py b/tests/e2e_batch/vax_suppliers.py index 5e3df5741e..01a61bf904 100644 --- a/tests/e2e_batch/vax_suppliers.py +++ b/tests/e2e_batch/vax_suppliers.py @@ -84,7 +84,6 @@ class TestPair: "ods_vax": TestPair.E8HA94_COVID_CUD, "ods_vax": TestPair.DPSFULL_COVID_CRUDS, "ods_vax": TestPair.V0V8L_FLU_CRUDS, - "ods_vax": TestPair.V0V8L_3IN1_CRUDS, "ods_vax": TestPair.X26_MMR_CRUDS, "ods_vax": TestPair.YGA_MENACWY_CRUDS, """ @@ -119,7 +118,7 @@ class TestPair: # DPSREDUCED_MENACWY_CRUDS = OdsVax("DPSREDUCED", "MENACWY") # DPSREDUCED_MMR_CRUDS = OdsVax("DPSREDUCED", "MMR") # DPSREDUCED_RSV_CRUDS = OdsVax("DPSREDUCED", "RSV") - V0V8L_3IN1_CRUDS = OdsVax("V0V8L", "3IN1") + # V0V8L_3IN1_CRUDS = OdsVax("V0V8L", "3IN1") V0V8L_FLU_CRUDS = OdsVax("V0V8L", "FLU") # V0V8L_HPV_CRUDS = OdsVax("V0V8L", "HPV") # V0V8L_MENACWY_CRUDS = OdsVax("V0V8L", "MENACWY")