Fixing sonar issues

Author: ajmu1

application/common/tests/test_secretsmanager.py

@@ -4,6 +4,8 @@
 import pytest
 from moto import mock_aws
 
+from application.common.errors import SecretsManagerError
+
 FILE_PATH = "application.common.secretsmanager"
 
 
@@ -26,5 +28,5 @@ def test_get_secret() -> None:
 def test_get_secret_resource_not_found() -> None:
     from application.common.secretsmanager import get_secret
 
-    with pytest.raises(Exception, match="Failed getting secret 'fake_secret_name' from secrets manager"):
+    with pytest.raises(SecretsManagerError, match="Failed getting secret 'fake_secret_name' from secrets manager"):
         get_secret("fake_secret_name")

application/event_replay/tests/test_event_replay.py

@@ -102,21 +102,20 @@ def test_build_correlation_id(mock_time_ns: MagicMock) -> None:
     assert response == f"{time}-local-replayed-event"
 
 
-@patch(f"{FILE_PATH}.client")
+@patch(f"{FILE_PATH}.dynamodb_client")
 def test_get_change_event(mock_client: MagicMock, change_event: dict[str, str], event: dict[str, str]) -> None:
     # Arrange
     table_name = "my-table"
     environ["CHANGE_EVENTS_TABLE_NAME"] = table_name
     environ["AWS_REGION"] = "eu-west-1"
     serializer = TypeSerializer()
 
-    mock_client.return_value.query.return_value = {"Items": [{"Event": serializer.serialize(change_event)}]}
+    mock_client.query.return_value = {"Items": [{"Event": serializer.serialize(change_event)}]}
     # Act
     response = get_change_event(event["odscode"], Decimal(event["sequence_number"]))
     # Assert
     assert response == change_event
-    mock_client.assert_called_with("dynamodb")
-    mock_client().query.assert_called_with(
+    mock_client.query.assert_called_with(
         TableName=table_name,
         IndexName="gsi_ods_sequence",
         ProjectionExpression="Event",
@@ -130,21 +129,20 @@ def test_get_change_event(mock_client: MagicMock, change_event: dict[str, str],
     del environ["AWS_REGION"]
 
 
-@patch(f"{FILE_PATH}.client")
+@patch(f"{FILE_PATH}.dynamodb_client")
 def test_get_change_event_no_change_event_in_dynamodb(
     mock_client: MagicMock, change_event: dict[str, str], event: dict[str, str]
 ) -> None:
     # Arrange
     table_name = "my-table"
     environ["CHANGE_EVENTS_TABLE_NAME"] = table_name
     environ["AWS_REGION"] = "eu-west-1"
-    mock_client.return_value.query.return_value = {"Items": []}
+    mock_client.query.return_value = {"Items": []}
     # Act
     with pytest.raises(ValueError, match="No change event found for ods code FXXX1 and sequence number 1"):
         get_change_event(event["odscode"], Decimal(event["sequence_number"]))
     # Assert
-    mock_client.assert_called_with("dynamodb")
-    mock_client().query.assert_called_with(
+    mock_client.query.assert_called_with(
         TableName=table_name,
         IndexName="gsi_ods_sequence",
         ProjectionExpression="Event",
@@ -158,16 +156,15 @@ def test_get_change_event_no_change_event_in_dynamodb(
     del environ["AWS_REGION"]
 
 
-@patch(f"{FILE_PATH}.client")
+@patch(f"{FILE_PATH}.sqs_client")
 def test_send_change_event(mock_client: MagicMock, change_event: dict[str, str], event: dict[str, str]) -> None:
     # Arrange
     correlation_id = "CORRELATION_ID"
     environ["CHANGE_EVENT_SQS_URL"] = queue_url = "https://sqs.eu-west-1.amazonaws.com/123456789/my-queue"
     # Act
     send_change_event(change_event, event["odscode"], int(event["sequence_number"]), correlation_id)
     # Assert
-    mock_client.assert_called_with("sqs")
-    mock_client().send_message.assert_called_with(
+    mock_client.send_message.assert_called_with(
         QueueUrl=queue_url,
         MessageBody=dumps(change_event),
         MessageGroupId=event["odscode"],

application/ingest_change_event/tests/test_ingest_change_event.py

@@ -12,7 +12,7 @@
 FILE_PATH = "application.ingest_change_event.ingest_change_event"
 
 
-@patch(f"{FILE_PATH}.sqs")
+@patch(f"{FILE_PATH}.sqs_client")
 @patch(f"{FILE_PATH}.HoldingQueueChangeEventItem")
 @patch(f"{FILE_PATH}.add_change_event_to_dynamodb")
 @patch(f"{FILE_PATH}.get_latest_sequence_id_for_a_given_odscode_from_dynamodb")
@@ -72,7 +72,7 @@ def test_lambda_handler(
     del environ["HOLDING_QUEUE_URL"]
 
 
-@patch(f"{FILE_PATH}.sqs")
+@patch(f"{FILE_PATH}.sqs_client")
 @patch(f"{FILE_PATH}.HoldingQueueChangeEventItem")
 @patch(f"{FILE_PATH}.add_change_event_to_dynamodb")
 @patch(f"{FILE_PATH}.get_latest_sequence_id_for_a_given_odscode_from_dynamodb")
@@ -130,7 +130,7 @@ def test_lambda_handler_with_sensitive_staff_key(
 
 
 @patch.object(Logger, "error")
-@patch(f"{FILE_PATH}.sqs")
+@patch(f"{FILE_PATH}.sqs_client")
 @patch(f"{FILE_PATH}.HoldingQueueChangeEventItem")
 @patch(f"{FILE_PATH}.add_change_event_to_dynamodb")
 @patch(f"{FILE_PATH}.get_latest_sequence_id_for_a_given_odscode_from_dynamodb")
@@ -183,7 +183,7 @@ def test_lambda_handler_no_sequence_number(
 
 
 @patch.object(Logger, "error")
-@patch(f"{FILE_PATH}.sqs")
+@patch(f"{FILE_PATH}.sqs_client")
 @patch(f"{FILE_PATH}.HoldingQueueChangeEventItem")
 @patch(f"{FILE_PATH}.add_change_event_to_dynamodb")
 @patch(f"{FILE_PATH}.get_latest_sequence_id_for_a_given_odscode_from_dynamodb")
@@ -239,7 +239,7 @@ def test_lambda_handler_less_than_latest_sequence_number(
     del environ["HOLDING_QUEUE_URL"]
 
 
-@patch(f"{FILE_PATH}.sqs")
+@patch(f"{FILE_PATH}.sqs_client")
 @patch(f"{FILE_PATH}.HoldingQueueChangeEventItem")
 @patch(f"{FILE_PATH}.add_change_event_to_dynamodb")
 @patch(f"{FILE_PATH}.get_latest_sequence_id_for_a_given_odscode_from_dynamodb")

application/service_matcher/service_matcher.py

@@ -120,7 +120,7 @@ def send_update_requests(
     for i, chunk in enumerate(chunks):
         # TODO: Handle errors?
         logger.debug(f"Sending off message chunk {i+1}/{len(chunks)}")
-        response = sqs.send_message_batch(QueueUrl=environ["UPDATE_REQUEST_QUEUE_URL"], Entries=chunk)
+        response = sqs_client.send_message_batch(QueueUrl=environ["UPDATE_REQUEST_QUEUE_URL"], Entries=chunk)
         logger.debug("Sent off message chunk", response=response)
         logger.warning(
             "Sent Off Update Request",

application/service_matcher/tests/test_service_matcher.py

@@ -119,7 +119,7 @@ def test_lambda_handler_should_throw_exception_if_event_records_len_not_eq_one(l
     del environ["ENV"]
 
 
-@patch(f"{FILE_PATH}.sqs")
+@patch(f"{FILE_PATH}.sqs_client")
 @patch.object(Logger, "get_correlation_id", return_value="1")
 @patch.object(Logger, "warning")
 def test_send_update_requests(

application/service_sync/reject_pending_changes/tests/test_s3.py

@@ -10,6 +10,7 @@
 def test_put_content_to_s3(mock_client: MagicMock) -> None:
     # Arrange
     environ["SEND_EMAIL_BUCKET_NAME"] = bucket_name = "bucket_name"
+    environ["AWS_ACCOUNT_ID"] = aws_account_id = "123456789012"
     s3_filename = "s3_filename"
     content = b"content"
     # Act
@@ -21,6 +22,8 @@ def test_put_content_to_s3(mock_client: MagicMock) -> None:
         Bucket=bucket_name,
         Key=s3_filename,
         ServerSideEncryption="AES256",
+        ExpectedBucketOwner=aws_account_id,
     )
     # Cleanup
     del environ["SEND_EMAIL_BUCKET_NAME"]
+    del environ["AWS_ACCOUNT_ID"]

application/service_sync/tests/test_service_sync.py

@@ -122,15 +122,14 @@ def test_lambda_handler_exception(
 
 
 @patch.object(Logger, "info")
-@patch(f"{FILE_PATH}.client")
+@patch(f"{FILE_PATH}.sqs_client")
 def test_remove_sqs_message_from_queue(mock_client: MagicMock, mock_logger_info: MagicMock) -> None:
     # Arrange
     environ["UPDATE_REQUEST_QUEUE_URL"] = update_request_queue_url = "update_request_queue_url"
     # Act
     remove_sqs_message_from_queue(receipt_handle=RECEIPT_HANDLE)
     # Assert
-    mock_client.assert_called_once_with("sqs")
-    mock_client.return_value.delete_message.assert_called_once_with(
+    mock_client.delete_message.assert_called_once_with(
         QueueUrl=update_request_queue_url,
         ReceiptHandle=RECEIPT_HANDLE,
     )