CCM-12616: resolve comments

Author: lapenna-bjss

.coverage

@@ -42,9 +42,11 @@ def create_sqs_event(num_records=1, event_source='aws:sqs'):
 class TestHandler:
     """Test suite for Lambda handler"""
 
+    @patch('src.handler.EventPublisher')
+    @patch('src.handler.DocumentStore')
     @patch('src.handler.Config')
     @patch('src.handler.MeshDownloadProcessor')
-    def test_handler_success_single_message(self, mock_processor_class, mock_config_class):
+    def test_handler_success_single_message(self, mock_processor_class, mock_config_class, mock_doc_store_class, mock_event_pub_class):
         """Test successful handler execution with single SQS message"""
         from src.handler import handler
 
@@ -54,6 +56,11 @@ def test_handler_success_single_message(self, mock_processor_class, mock_config_
         mock_config_class.return_value.__exit__ = Mock(return_value=None)
         mock_processor_class.return_value = mock_processor
 
+        mock_doc_store = Mock()
+        mock_doc_store_class.return_value = mock_doc_store
+        mock_event_pub = Mock()
+        mock_event_pub_class.return_value = mock_event_pub
+
         event = create_sqs_event(num_records=1)
 
         result = handler(event, mock_context)
@@ -71,9 +78,11 @@ def test_handler_success_single_message(self, mock_processor_class, mock_config_
 
         assert result == {"batchItemFailures": []}
 
+    @patch('src.handler.EventPublisher')
+    @patch('src.handler.DocumentStore')
     @patch('src.handler.Config')
     @patch('src.handler.MeshDownloadProcessor')
-    def test_handler_success_multiple_messages(self, mock_processor_class, mock_config_class):
+    def test_handler_success_multiple_messages(self, mock_processor_class, mock_config_class, mock_doc_store_class, mock_event_pub_class):
         """Test successful handler execution with multiple SQS messages"""
         from src.handler import handler
 
@@ -83,6 +92,9 @@ def test_handler_success_multiple_messages(self, mock_processor_class, mock_conf
         mock_config_class.return_value.__exit__ = Mock(return_value=None)
         mock_processor_class.return_value = mock_processor
 
+        mock_doc_store_class.return_value = Mock()
+        mock_event_pub_class.return_value = Mock()
+
         event = create_sqs_event(num_records=3)
 
         result = handler(event, mock_context)
@@ -93,9 +105,11 @@ def test_handler_success_multiple_messages(self, mock_processor_class, mock_conf
         # Verify return value (no failures)
         assert result == {"batchItemFailures": []}
 
+    @patch('src.handler.EventPublisher')
+    @patch('src.handler.DocumentStore')
     @patch('src.handler.Config')
     @patch('src.handler.MeshDownloadProcessor')
-    def test_handler_config_cleanup_on_success(self, mock_processor_class, mock_config_class):
+    def test_handler_config_cleanup_on_success(self, mock_processor_class, mock_config_class, mock_doc_store_class, mock_event_pub_class):
         """Test that Config context manager cleanup is called on success"""
         from src.handler import handler
 
@@ -106,16 +120,21 @@ def test_handler_config_cleanup_on_success(self, mock_processor_class, mock_conf
         mock_config_class.return_value.__exit__ = mock_exit
         mock_processor_class.return_value = mock_processor
 
+        mock_doc_store_class.return_value = Mock()
+        mock_event_pub_class.return_value = Mock()
+
         event = create_sqs_event(num_records=1)
 
         handler(event, mock_context)
 
         mock_exit.assert_called_once()
         assert mock_exit.call_args[0] == (None, None, None)
 
+    @patch('src.handler.EventPublisher')
+    @patch('src.handler.DocumentStore')
     @patch('src.handler.Config')
     @patch('src.handler.MeshDownloadProcessor')
-    def test_handler_partial_batch_failure(self, mock_processor_class, mock_config_class):
+    def test_handler_partial_batch_failure(self, mock_processor_class, mock_config_class, mock_doc_store_class, mock_event_pub_class):
         """Test handler handles partial batch failures correctly"""
         from src.handler import handler
 
@@ -125,6 +144,9 @@ def test_handler_partial_batch_failure(self, mock_processor_class, mock_config_c
         mock_config_class.return_value.__exit__ = Mock(return_value=None)
         mock_processor_class.return_value = mock_processor
 
+        mock_doc_store_class.return_value = Mock()
+        mock_event_pub_class.return_value = Mock()
+
         # Make second message fail
         mock_processor.process_sqs_message.side_effect = [
             None,
@@ -140,9 +162,11 @@ def test_handler_partial_batch_failure(self, mock_processor_class, mock_config_c
         assert len(result["batchItemFailures"]) == 1
         assert result["batchItemFailures"][0]["itemIdentifier"] == "msg-1"
 
+    @patch('src.handler.EventPublisher')
+    @patch('src.handler.DocumentStore')
     @patch('src.handler.Config')
     @patch('src.handler.MeshDownloadProcessor')
-    def test_handler_skips_non_sqs_records(self, mock_processor_class, mock_config_class):
+    def test_handler_skips_non_sqs_records(self, mock_processor_class, mock_config_class, mock_doc_store_class, mock_event_pub_class):
         """Test handler skips records that are not from SQS"""
         from src.handler import handler
 
@@ -152,6 +176,9 @@ def test_handler_skips_non_sqs_records(self, mock_processor_class, mock_config_c
         mock_config_class.return_value.__exit__ = Mock(return_value=None)
         mock_processor_class.return_value = mock_processor
 
+        mock_doc_store_class.return_value = Mock()
+        mock_event_pub_class.return_value = Mock()
+
         event = create_sqs_event(num_records=1, event_source='aws:dynamodb')
 
         result = handler(event, mock_context)
@@ -160,29 +187,41 @@ def test_handler_skips_non_sqs_records(self, mock_processor_class, mock_config_c
 
         assert result == {"batchItemFailures": []}
 
+    @patch('src.handler.EventPublisher')
+    @patch('src.handler.DocumentStore')
     @patch('src.handler.Config')
     @patch('src.handler.MeshDownloadProcessor')
-    def test_handler_config_cleanup_on_exception(self, mock_processor_class, mock_config_class):
+    def test_handler_config_cleanup_on_exception(self, mock_processor_class, mock_config_class, mock_doc_store_class, mock_event_pub_class):
         """Test that Config context manager cleanup is called even on exception"""
         from src.handler import handler
 
         (mock_context, mock_config, mock_processor) = setup_mocks()
 
-        # Make config.__enter__ raise an exception
-        test_exception = RuntimeError("Config error")
-        mock_config_class.return_value.__enter__.side_effect = test_exception
+        mock_config_class.return_value.__enter__.return_value = mock_config
         mock_exit = Mock(return_value=None)
         mock_config_class.return_value.__exit__ = mock_exit
 
+        mock_doc_store_class.return_value = Mock()
+        mock_event_pub_class.return_value = Mock()
+
+        test_exception = RuntimeError("Processing error")
+        mock_processor.process_sqs_message.side_effect = test_exception
+        mock_processor_class.return_value = mock_processor
+
         event = create_sqs_event(num_records=1)
 
-        # Handler should raise the exception
-        with pytest.raises(RuntimeError, match="Config error"):
-            handler(event, mock_context)
+        result = handler(event, mock_context)
+
+        mock_exit.assert_called_once()
+
+        # Verify the failed message is in batch failures
+        assert len(result["batchItemFailures"]) == 1
 
+    @patch('src.handler.EventPublisher')
+    @patch('src.handler.DocumentStore')
     @patch('src.handler.Config')
     @patch('src.handler.MeshDownloadProcessor')
-    def test_handler_returns_empty_failures_on_empty_event(self, mock_processor_class, mock_config_class):
+    def test_handler_returns_empty_failures_on_empty_event(self, mock_processor_class, mock_config_class, mock_doc_store_class, mock_event_pub_class):
         """Test handler handles empty event gracefully"""
         from src.handler import handler
 
@@ -192,6 +231,9 @@ def test_handler_returns_empty_failures_on_empty_event(self, mock_processor_clas
         mock_config_class.return_value.__exit__ = Mock(return_value=None)
         mock_processor_class.return_value = mock_processor
 
+        mock_doc_store_class.return_value = Mock()
+        mock_event_pub_class.return_value = Mock()
+
         event = {'Records': []}
 
         result = handler(event, mock_context)
@@ -200,9 +242,11 @@ def test_handler_returns_empty_failures_on_empty_event(self, mock_processor_clas
 
         assert result == {"batchItemFailures": []}
 
+    @patch('src.handler.EventPublisher')
+    @patch('src.handler.DocumentStore')
     @patch('src.handler.Config')
     @patch('src.handler.MeshDownloadProcessor')
-    def test_handler_passes_correct_parameters_to_processor(self, mock_processor_class, mock_config_class):
+    def test_handler_passes_correct_parameters_to_processor(self, mock_processor_class, mock_config_class, mock_doc_store_class, mock_event_pub_class):
         """Test that handler passes all required parameters to MeshDownloadProcessor"""
         from src.handler import handler
 
@@ -217,13 +261,22 @@ def test_handler_passes_correct_parameters_to_processor(self, mock_processor_cla
         mock_config_class.return_value.__exit__ = Mock(return_value=None)
         mock_processor_class.return_value = mock_processor
 
+        mock_doc_store = Mock()
+        mock_doc_store_class.return_value = mock_doc_store
+        mock_event_pub = Mock()
+        mock_event_pub_class.return_value = mock_event_pub
+
         event = create_sqs_event(num_records=1)
 
         handler(event, mock_context)
 
         mock_processor_class.assert_called_once()
         call_kwargs = mock_processor_class.call_args[1]
 
-        # Handler now passes the entire config object
+        # Handler now passes the entire config object and dependencies
         assert call_kwargs['config'] == mock_config
+        assert call_kwargs['mesh_client'] == mock_mesh_client
+        assert call_kwargs['download_metric'] == mock_download_metric
+        assert call_kwargs['document_store'] == mock_doc_store
+        assert call_kwargs['event_publisher'] == mock_event_pub
         assert 'log' in call_kwargs

lambdas/mesh-download/src/__tests__/test_handler.py

@@ -90,15 +90,17 @@ def create_mesh_message(message_id='test_123', sender='SENDER_001', local_id='re
 class TestMeshDownloadProcessor:
     """Test suite for MeshDownloadProcessor"""
 
-    def test_processor_initialization_with_event_publisher(self):
-        """Processor initializes and handshakes mesh client when EventPublisher configured"""
+    def test_processor_initialization_calls_mesh_handshake(self):
+        """Processor initializes and handshakes mesh client"""
         from src.processor import MeshDownloadProcessor
 
         config, log, event_publisher, document_store = setup_mocks()
 
         processor = MeshDownloadProcessor(
             config=config,
             log=log,
+            mesh_client=config.mesh_client,
+            download_metric=config.download_metric,
             document_store=document_store,
             event_publisher=event_publisher
         )
@@ -122,6 +124,8 @@ def test_process_sqs_message_success(self, mock_datetime):
         processor = MeshDownloadProcessor(
             config=config,
             log=log,
+            mesh_client=config.mesh_client,
+            download_metric=config.download_metric,
             document_store=document_store,
             event_publisher=event_publisher
         )
@@ -149,6 +153,26 @@ def test_process_sqs_message_success(self, mock_datetime):
 
         event_publisher.send_events.assert_called_once()
 
+        # Verify the published event content
+        published_events = event_publisher.send_events.call_args[0][0]
+        assert len(published_events) == 1
+
+        published_event = published_events[0]
+
+        # Verify CloudEvent envelope fields
+        assert published_event['type'] == 'uk.nhs.notify.digital.letters.mesh.inbox.message.downloaded.v1'
+        assert published_event['source'] == '/nhs/england/notify/development/primary/data-plane/digitalletters/mesh'
+        assert published_event['subject'] == 'customer/00000000-0000-0000-0000-000000000000/recipient/00000000-0000-0000-0000-000000000000'
+        assert published_event['time'] == '2025-11-19T15:30:45+00:00'
+        assert 'id' in published_event
+
+        # Verify CloudEvent data payload
+        event_data = published_event['data']
+        assert event_data['senderId'] == 'TEST_SENDER'
+        assert event_data['messageReference'] == 'ref_001'
+        assert event_data['messageUri'] == 's3://test-pii-bucket/document-reference/SENDER_001_ref_001'
+        assert set(event_data.keys()) == {'senderId', 'messageReference', 'messageUri'}
+
     def test_process_sqs_message_validation_failure(self):
         """Malformed CloudEvents should be rejected by pydantic and not trigger downloads"""
         from src.processor import MeshDownloadProcessor
@@ -158,6 +182,8 @@ def test_process_sqs_message_validation_failure(self):
         processor = MeshDownloadProcessor(
             config=config,
             log=log,
+            mesh_client=config.mesh_client,
+            download_metric=config.download_metric,
             document_store=document_store,
             event_publisher=event_publisher
         )
@@ -180,6 +206,8 @@ def test_process_sqs_message_missing_mesh_message_id(self):
         processor = MeshDownloadProcessor(
             config=config,
             log=log,
+            mesh_client=config.mesh_client,
+            download_metric=config.download_metric,
             document_store=document_store,
             event_publisher=event_publisher
         )
@@ -203,6 +231,8 @@ def test_download_and_store_message_not_found(self):
         processor = MeshDownloadProcessor(
             config=config,
             log=log,
+            mesh_client=config.mesh_client,
+            download_metric=config.download_metric,
             document_store=document_store,
             event_publisher=event_publisher
         )
@@ -225,6 +255,8 @@ def test_document_store_failure_prevents_ack_and_raises(self):
         processor = MeshDownloadProcessor(
             config=config,
             log=log,
+            mesh_client=config.mesh_client,
+            download_metric=config.download_metric,
             document_store=document_store,
             event_publisher=event_publisher
         )
@@ -258,6 +290,8 @@ def test_bucket_selection_with_mesh_mock_enabled(self, mock_datetime):
         processor = MeshDownloadProcessor(
             config=config,
             log=log,
+            mesh_client=config.mesh_client,
+            download_metric=config.download_metric,
             document_store=document_store,
             event_publisher=event_publisher
         )
@@ -294,6 +328,8 @@ def test_bucket_selection_with_mesh_mock_disabled(self, mock_datetime):
         processor = MeshDownloadProcessor(
             config=config,
             log=log,
+            mesh_client=config.mesh_client,
+            download_metric=config.download_metric,
             document_store=document_store,
             event_publisher=event_publisher
         )
@@ -319,6 +355,8 @@ def test_message_reference_mismatch_raises_error(self):
         processor = MeshDownloadProcessor(
             config=config,
             log=log,
+            mesh_client=config.mesh_client,
+            download_metric=config.download_metric,
             document_store=document_store,
             event_publisher=event_publisher
         )

lambdas/mesh-download/src/__tests__/test_processor.py

@@ -5,6 +5,13 @@ class IntermediaryBodyStoreError(Exception):
     """Error to represent any failure to upload document to intermediate location"""
 
 
+class DocumentStoreConfig:
+    """Configuration holder for DocumentStore"""
+    def __init__(self, s3_client, transactional_data_bucket):
+        self.s3_client = s3_client
+        self.transactional_data_bucket = transactional_data_bucket
+
+
 class DocumentStore:  # pylint: disable=too-few-public-methods
     """Class for storing document references in S3"""
 

lambdas/mesh-download/src/document_store.py

@@ -1,8 +1,11 @@
 """lambda handler for mesh download"""
 
 import json
+from event_publisher import EventPublisher
+
 from .config import Config, log
 from .processor import MeshDownloadProcessor
+from .document_store import DocumentStore, DocumentStoreConfig
 
 
 def handler(event, context):
@@ -23,9 +26,25 @@ def handler(event, context):
 
     try:
         with Config() as config:
+            doc_store_config = DocumentStoreConfig(
+                s3_client=config.s3_client,
+                transactional_data_bucket=config.transactional_data_bucket
+            )
+            document_store = DocumentStore(doc_store_config)
+
+            event_publisher = EventPublisher(
+                event_bus_arn=config.event_publisher_event_bus_arn,
+                dlq_url=config.event_publisher_dlq_url,
+                logger=log
+            )
+
             processor = MeshDownloadProcessor(
                 config=config,
-                log=log
+                log=log,
+                mesh_client=config.mesh_client,
+                download_metric=config.download_metric,
+                document_store=document_store,
+                event_publisher=event_publisher
             )
 
             # Process each SQS record