init

JamesW1-NHS · JamesW1-NHS · commit f60f7fa7df5b · 2025-10-28T15:39:33.000Z
diff --git a/lambdas/filenameprocessor/src/file_name_processor.py b/lambdas/filenameprocessor/src/file_name_processor.py
@@ -10,7 +10,7 @@
 from uuid import uuid4
 
 from audit_table import upsert_audit_table
-from common.clients import STREAM_NAME, logger, s3_client
+from common.clients import STREAM_NAME, logger, get_s3_client
 from common.log_decorator import logging_decorator
 from common.models.errors import (
     InvalidFileKeyError,
@@ -73,7 +73,7 @@ def handle_record(record) -> dict:
 
     try:
         message_id = str(uuid4())
-        s3_response = s3_client.get_object(Bucket=bucket_name, Key=file_key)
+        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)
diff --git a/lambdas/filenameprocessor/src/make_and_upload_ack_file.py b/lambdas/filenameprocessor/src/make_and_upload_ack_file.py
@@ -4,7 +4,7 @@
 from csv import writer
 from io import BytesIO, StringIO
 
-from common.clients import s3_client
+from common.clients import get_s3_client
 
 
 def make_the_ack_data(message_id: str, message_delivered: bool, created_at_formatted_string: str) -> dict:
@@ -43,7 +43,7 @@ def upload_ack_file(file_key: str, ack_data: dict, created_at_formatted_string:
     csv_buffer.seek(0)
     csv_bytes = BytesIO(csv_buffer.getvalue().encode("utf-8"))
     ack_bucket_name = os.getenv("ACK_BUCKET_NAME")
-    s3_client.upload_fileobj(csv_bytes, ack_bucket_name, ack_filename)
+    get_s3_client().upload_fileobj(csv_bytes, ack_bucket_name, ack_filename)
 
 
 def make_and_upload_the_ack_file(
diff --git a/lambdas/filenameprocessor/src/utils_for_filenameprocessor.py b/lambdas/filenameprocessor/src/utils_for_filenameprocessor.py
@@ -2,7 +2,7 @@
 
 from datetime import timedelta
 
-from common.clients import logger, s3_client
+from common.clients import logger, get_s3_client
 from constants import AUDIT_TABLE_TTL_DAYS
 
 
@@ -16,6 +16,7 @@ def get_creation_and_expiry_times(s3_response: dict) -> (str, int):
 
 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."""
+    s3_client = get_s3_client()
     s3_client.copy_object(
         Bucket=bucket_name,
         CopySource={"Bucket": bucket_name, "Key": source_file_key},
diff --git a/lambdas/recordprocessor/src/file_level_validation.py b/lambdas/recordprocessor/src/file_level_validation.py
@@ -6,7 +6,7 @@
 from csv import DictReader
 
 from audit_table import update_audit_table_status
-from common.clients import logger, s3_client
+from common.clients import logger, get_s3_client
 from constants import (
     ARCHIVE_DIR_NAME,
     EXPECTED_CSV_HEADERS,
@@ -63,12 +63,13 @@ def get_permitted_operations(supplier: str, vaccine_type: str, allowed_permissio
 
 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."""
-    s3_client.copy_object(
+    s3_client = get_s3_client()
+    get_s3_client().copy_object(
         Bucket=bucket_name,
         CopySource={"Bucket": bucket_name, "Key": source_file_key},
         Key=destination_file_key,
     )
-    s3_client.delete_object(Bucket=bucket_name, Key=source_file_key)
+    get_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)
 
 
diff --git a/lambdas/recordprocessor/src/make_and_upload_ack_file.py b/lambdas/recordprocessor/src/make_and_upload_ack_file.py
@@ -3,7 +3,7 @@
 from csv import writer
 from io import BytesIO, StringIO
 
-from common.clients import s3_client
+from common.clients import get_s3_client
 from constants import ACK_BUCKET_NAME
 
 
@@ -46,7 +46,7 @@ def upload_ack_file(file_key: str, ack_data: dict, created_at_formatted_string:
     # Upload the CSV file to S3
     csv_buffer.seek(0)
     csv_bytes = BytesIO(csv_buffer.getvalue().encode("utf-8"))
-    s3_client.upload_fileobj(csv_bytes, ACK_BUCKET_NAME, ack_filename)
+    get_s3_client().upload_fileobj(csv_bytes, ACK_BUCKET_NAME, ack_filename)
 
 
 def make_and_upload_ack_file(
diff --git a/lambdas/recordprocessor/src/utils_for_recordprocessor.py b/lambdas/recordprocessor/src/utils_for_recordprocessor.py
@@ -4,7 +4,7 @@
 from csv import DictReader
 from io import TextIOWrapper
 
-from common.clients import s3_client
+from common.clients import get_s3_client
 
 
 def get_environment() -> str:
@@ -16,7 +16,7 @@ def get_environment() -> str:
 
 def get_csv_content_dict_reader(file_key: str, encoder="utf-8") -> DictReader:
     """Returns the requested file contents from the source bucket in the form of a DictReader"""
-    response = s3_client.get_object(Bucket=os.getenv("SOURCE_BUCKET_NAME"), Key=file_key)
+    response = get_s3_client().get_object(Bucket=os.getenv("SOURCE_BUCKET_NAME"), Key=file_key)
     binary_io = response["Body"]
     text_io = TextIOWrapper(binary_io, encoding=encoder, newline="")
     return DictReader(text_io, delimiter="|")
diff --git a/lambdas/recordprocessor/tests/test_recordprocessor_edge_cases.py b/lambdas/recordprocessor/tests/test_recordprocessor_edge_cases.py
@@ -1,10 +1,11 @@
 import os
 import unittest
 from io import BytesIO
-from unittest.mock import call, patch
+from unittest.mock import Mock, call, patch
 
 from batch_processor import process_csv_to_fhir
 from utils_for_recordprocessor_tests.utils_for_recordprocessor_tests import (
+    BucketNames,
     create_patch,
 )
 
@@ -16,8 +17,7 @@ def setUp(self):
         self.mock_logger_error = create_patch("logging.Logger.error")
         self.mock_send_to_kinesis = create_patch("batch_processor.send_to_kinesis")
         self.mock_map_target_disease = create_patch("batch_processor.map_target_disease")
-        self.mock_s3_get_object = create_patch("utils_for_recordprocessor.s3_client.get_object")
-        self.mock_s3_put_object = create_patch("utils_for_recordprocessor.s3_client.put_object")
+        self.mock_get_s3_client = create_patch("utils_for_recordprocessor.get_s3_client")
         self.mock_make_and_move = create_patch("file_level_validation.make_and_upload_ack_file")
         self.mock_move_file = create_patch("file_level_validation.move_file")
         self.mock_get_permitted_operations = create_patch("file_level_validation.get_permitted_operations")
@@ -63,7 +63,9 @@ def test_process_large_file_cp1252(self):
         data = self.insert_cp1252_at_end(data, b"D\xe9cembre", 2)
         ret1 = {"Body": BytesIO(b"".join(data))}
         ret2 = {"Body": BytesIO(b"".join(data))}
-        self.mock_s3_get_object.side_effect = [ret1, ret2]
+        mock_s3 = Mock()
+        mock_s3.get_object.side_effect = [ret1, ret2]
+        self.mock_get_s3_client.return_value = mock_s3
         self.mock_map_target_disease.return_value = "some disease"
 
         message_body = {
@@ -80,10 +82,11 @@ def test_process_large_file_cp1252(self):
         self.mock_logger_warning.assert_called()
         warning_call_args = self.mock_logger_warning.call_args[0][0]
         self.assertTrue(warning_call_args.startswith("Encoding Error: 'utf-8' codec can't decode byte 0xe9"))
-        self.mock_s3_get_object.assert_has_calls(
+        # TODO: when running all tests this expects Bucket=None. not clear why.
+        mock_s3.get_object.assert_has_calls(
             [
-                call(Bucket=None, Key="test-filename"),
-                call(Bucket=None, Key="processing/test-filename"),
+                call(Bucket=BucketNames.SOURCE, Key="test-filename"),
+                call(Bucket=BucketNames.SOURCE, Key="processing/test-filename"),
             ]
         )
 
@@ -94,7 +97,9 @@ def test_process_large_file_utf8(self):
         data = self.expand_test_data(data, n_rows)
         ret1 = {"Body": BytesIO(b"".join(data))}
         ret2 = {"Body": BytesIO(b"".join(data))}
-        self.mock_s3_get_object.side_effect = [ret1, ret2]
+        mock_s3 = Mock()
+        mock_s3.get_object.side_effect = [ret1, ret2]
+        self.mock_get_s3_client.return_value = mock_s3
         self.mock_map_target_disease.return_value = "some disease"
 
         message_body = {
@@ -118,7 +123,9 @@ def test_process_small_file_cp1252(self):
 
         ret1 = {"Body": BytesIO(b"".join(data))}
         ret2 = {"Body": BytesIO(b"".join(data))}
-        self.mock_s3_get_object.side_effect = [ret1, ret2]
+        mock_s3 = Mock()
+        mock_s3.get_object.side_effect = [ret1, ret2]
+        self.mock_get_s3_client.return_value = mock_s3
         self.mock_map_target_disease.return_value = "some disease"
 
         message_body = {
@@ -143,7 +150,9 @@ def test_process_small_file_utf8(self):
 
         ret1 = {"Body": BytesIO(b"".join(data))}
         ret2 = {"Body": BytesIO(b"".join(data))}
-        self.mock_s3_get_object.side_effect = [ret1, ret2]
+        mock_s3 = Mock()
+        mock_s3.get_object.side_effect = [ret1, ret2]
+        self.mock_get_s3_client.return_value = mock_s3
         self.mock_map_target_disease.return_value = "some disease"
 
         message_body = {
diff --git a/lambdas/shared/src/common/clients.py b/lambdas/shared/src/common/clients.py
@@ -14,9 +14,6 @@
 
 REGION_NAME = os.getenv("AWS_REGION", "eu-west-2")
 
-s3_client = boto3_client("s3", region_name=REGION_NAME)
-
-# for lambdas which require a global s3_client
 global_s3_client = None
 
 
diff --git a/lambdas/shared/src/common/s3_reader.py b/lambdas/shared/src/common/s3_reader.py
@@ -1,4 +1,4 @@
-from common.clients import logger, s3_client
+from common.clients import logger, get_s3_client
 
 
 class S3Reader:
@@ -12,7 +12,7 @@ class S3Reader:
     @staticmethod
     def read(bucket_name, file_key):
         try:
-            s3_file = s3_client.get_object(Bucket=bucket_name, Key=file_key)
+            s3_file = get_s3_client().get_object(Bucket=bucket_name, Key=file_key)
             return s3_file["Body"].read().decode("utf-8")
 
         except Exception as error:  # pylint: disable=broad-except
diff --git a/lambdas/shared/tests/test_common/test_clients.py b/lambdas/shared/tests/test_common/test_clients.py
@@ -46,6 +46,8 @@ def test_env_variables_loaded(self):
 
     def test_boto3_client_created_for_s3(self):
         """Test that S3 boto3 client is created with correct region"""
+        importlib.reload(clients)
+        clients.get_s3_client()
         self.mock_boto3_client.assert_any_call("s3", region_name=self.AWS_REGION)
 
     def test_boto3_client_created_for_firehose(self):
diff --git a/lambdas/shared/tests/test_common/test_s3_reader.py b/lambdas/shared/tests/test_common/test_s3_reader.py
@@ -1,41 +1,41 @@
 import unittest
-from unittest.mock import MagicMock, patch
+from unittest.mock import Mock, MagicMock, patch
 
 from common.s3_reader import S3Reader
 
 
+@patch("common.s3_reader.get_s3_client")
 class TestS3Reader(unittest.TestCase):
     def setUp(self):
         self.s3_reader = S3Reader()
         self.bucket = "test-bucket"
         self.key = "test.json"
 
-        # Patch s3_client
-        self.s3_client_patcher = patch("common.s3_reader.s3_client")
-        self.mock_s3_client = self.s3_client_patcher.start()
-
         self.logger_info_patcher = patch("logging.Logger.info")
         self.mock_logger_info = self.logger_info_patcher.start()
         self.logger_exception_patcher = patch("logging.Logger.exception")
         self.mock_logger_exception = self.logger_exception_patcher.start()
 
     def tearDown(self):
-        self.s3_client_patcher.stop()
         self.logger_info_patcher.stop()
         self.logger_exception_patcher.stop()
 
-    def test_read_success(self):
+    def test_read_success(self, mock_get_s3_client):
+        mock_s3 = Mock()
         mock_body = MagicMock()
         mock_body.read.return_value = b'{"foo": "bar"}'
-        self.mock_s3_client.get_object.return_value = {"Body": mock_body}
+        mock_s3.get_object.return_value = {"Body": mock_body}
+        mock_get_s3_client.return_value = mock_s3
 
         result = self.s3_reader.read(self.bucket, self.key)
 
         self.assertEqual(result, '{"foo": "bar"}')
-        self.mock_s3_client.get_object.assert_called_once_with(Bucket=self.bucket, Key=self.key)
+        mock_s3.get_object.assert_called_once_with(Bucket=self.bucket, Key=self.key)
 
-    def test_read_raises_exception(self):
-        self.mock_s3_client.get_object.side_effect = Exception("S3 error")
+    def test_read_raises_exception(self, mock_get_s3_client):
+        mock_s3 = Mock()
+        mock_s3.get_object.side_effect = Exception("S3 error")
+        mock_get_s3_client.return_value = mock_s3
 
         with self.assertRaises(Exception) as context:
             self.s3_reader.read(self.bucket, self.key)
