diff --git a/queue_services/strr-email/pyproject.toml b/queue_services/strr-email/pyproject.toml index b2d99b50a..aa5e32c11 100644 --- a/queue_services/strr-email/pyproject.toml +++ b/queue_services/strr-email/pyproject.toml @@ -1,6 +1,6 @@ [tool.poetry] name = "strr-email" -version = "1.2.0" +version = "1.2.1" description = "BC Registries - strr-email" authors = [] license = "BSD-3-Clause" @@ -113,4 +113,4 @@ omit = [ ] [tool.coverage.report] -fail_under = 80 \ No newline at end of file +fail_under = 80 diff --git a/queue_services/strr-email/src/strr_email/resources/email_listener.py b/queue_services/strr-email/src/strr_email/resources/email_listener.py index a245c5d89..6d802ddbb 100644 --- a/queue_services/strr-email/src/strr_email/resources/email_listener.py +++ b/queue_services/strr-email/src/strr_email/resources/email_listener.py @@ -87,6 +87,17 @@ } +def _is_invalid_email_error(error_body: object) -> bool: + """Return True when the upstream error indicates a malformed email address.""" + error_text = str(error_body).lower() + return bool( + re.search( + r"(invalid|malformed|bad|not valid).{0,25}email|email.{0,25}(invalid|malformed|not valid)", + error_text, + ) + ) + + @bp.route("/", methods=("POST",)) def worker(): """Process the incoming file uploaded event.""" @@ -194,21 +205,66 @@ def worker(): return jsonify({"message": "Error posting email to notify-api."}), 400 else: - token = AuthService.get_service_client_token() - resp = requests.post( - current_app.config["NOTIFY_SVC_URL"], - json=email, - headers={ - "Content-Type": "application/json", - "Authorization": f"Bearer {token}", - }, - timeout=current_app.config["NOTIFY_API_TIMEOUT"], - ) + # Send a separate notify-api request per recipient so that a single + # malformed email address does not block delivery to the other recipients. + recipients_list = [ + r.strip() for r in (email.get("recipients") or "").split(",") if r.strip() + ] + if not recipients_list: + logger.error(f"No recipients to email for ce: {str(ce)}") + return jsonify({"message": "No recipients for email."}), HTTPStatus.BAD_REQUEST - if resp.status_code not in [HTTPStatus.OK, HTTPStatus.ACCEPTED, HTTPStatus.CREATED]: - logger.info(f"Error {resp.status_code} - {str(resp.json())}") - logger.error(f"Error posting email to notify-api for: {str(ce)}") - return jsonify({"message": "Error posting email to notify-api."}), resp.status_code + token = AuthService.get_service_client_token() + headers = { + "Content-Type": "application/json", + "Authorization": f"Bearer {token}", + } + success_count = 0 + has_non_format_failure = False + for recipient in recipients_list: + single_email = {**email, "recipients": recipient} + try: + resp = requests.post( + current_app.config["NOTIFY_SVC_URL"], + json=single_email, + headers=headers, + timeout=current_app.config["NOTIFY_API_TIMEOUT"], + ) + except Exception as err: # noqa: BLE001 + logger.info(f"Error sending recipient {recipient}: {str(err)}") + logger.error( + f"Error posting email to notify-api for recipient {recipient}: {str(ce)}" + ) + has_non_format_failure = True + continue + if resp.status_code in [HTTPStatus.OK, HTTPStatus.ACCEPTED, HTTPStatus.CREATED]: + success_count += 1 + else: + try: + err_body = resp.json() + except Exception: # noqa: BLE001 + err_body = resp.text + logger.info(f"Error {resp.status_code} - {str(err_body)}") + logger.error( + f"Error posting email to notify-api for recipient {recipient}: {str(ce)}" + ) + if ( + HTTPStatus.BAD_REQUEST <= resp.status_code < HTTPStatus.INTERNAL_SERVER_ERROR + and _is_invalid_email_error(err_body) + ): + # Invalid recipient format is treated as a user-level 4xx. + pass + else: + has_non_format_failure = True + + if success_count == 0: + failure_status = ( + HTTPStatus.BAD_GATEWAY if has_non_format_failure else HTTPStatus.BAD_REQUEST + ) + return ( + jsonify({"message": "Error posting email to notify-api."}), + failure_status, + ) logger.info(f"completed ce: {str(ce)}") return {}, HTTPStatus.OK diff --git a/queue_services/strr-email/tests/unit/test_email_listener_worker.py b/queue_services/strr-email/tests/unit/test_email_listener_worker.py index 048909d42..dfb0a7b20 100644 --- a/queue_services/strr-email/tests/unit/test_email_listener_worker.py +++ b/queue_services/strr-email/tests/unit/test_email_listener_worker.py @@ -1,5 +1,7 @@ from http import HTTPStatus +import json from unittest.mock import MagicMock +from unittest.mock import patch import pytest import responses @@ -325,7 +327,7 @@ def _patch_legacy_application_flow(mocker, app_obj, ce): (HTTPStatus.OK, HTTPStatus.OK), (HTTPStatus.ACCEPTED, HTTPStatus.OK), (HTTPStatus.CREATED, HTTPStatus.OK), - (502, 502), + (502, HTTPStatus.BAD_GATEWAY), ], ) @responses.activate @@ -343,3 +345,108 @@ def test_worker_legacy_notify_status(app, mocker, ce_factory, notify_status, exp ) with app.test_request_context("/", method="POST", data=b"{}"): assert worker()[1] == expected_status + + +def _multi_recipient_app_dict(): + return { + "header": {"registrationNumber": "RN1", "registrationEndDate": "2026-01-01T00:00:00+00:00"}, + "registration": { + "registrationType": Registration.RegistrationType.HOST.value, + "unitAddress": {"streetNumber": "1", "city": "Vic", "postalCode": "V8V1A1"}, + "primaryContact": {"emailAddress": "host@example.com"}, + "propertyManager": {"contact": {"emailAddress": "bas@ba$.com"}}, + }, + } + + +@responses.activate +def test_worker_legacy_sends_one_request_per_recipient(app, mocker, ce_factory): + """Each recipient gets its own notify-api request so a bad email does not block the others.""" + ce = ce_factory(applicationNumber="APP-1", emailType="HOST_DECLINED") + app_obj = MagicMock( + application_number="APP-1", + registration_type=Registration.RegistrationType.HOST, + noc=None, + ) + _patch_legacy_application_flow(mocker, app_obj, ce) + mocker.patch( + "strr_email.resources.email_listener.ApplicationSerializer.to_dict", + return_value=_multi_recipient_app_dict(), + ) + + responses.add(responses.POST, app.config["NOTIFY_SVC_URL"], json={"id": 1}, status=200) + responses.add( + responses.POST, + app.config["NOTIFY_SVC_URL"], + json={"message": "bad email"}, + status=400, + ) + + with app.test_request_context("/", method="POST", data=b"{}"): + status = worker()[1] + + assert status == HTTPStatus.OK # at least one recipient succeeded + assert len(responses.calls) == 3 + sent_recipients = [json.loads(call.request.body)["recipients"] for call in responses.calls] + assert sent_recipients == [ + app.config["EMAIL_HOUSING_RECIPIENT_EMAIL"], + "host@example.com", + "bas@ba$.com", + ] + + +@responses.activate +def test_worker_legacy_all_recipients_fail(app, mocker, ce_factory): + """If every recipient send fails, the worker returns the failing notify-api status.""" + ce = ce_factory(applicationNumber="APP-1", emailType="HOST_DECLINED") + app_obj = MagicMock( + application_number="APP-1", + registration_type=Registration.RegistrationType.HOST, + noc=None, + ) + _patch_legacy_application_flow(mocker, app_obj, ce) + mocker.patch( + "strr_email.resources.email_listener.ApplicationSerializer.to_dict", + return_value=_multi_recipient_app_dict(), + ) + + responses.add( + responses.POST, app.config["NOTIFY_SVC_URL"], json={"message": "bad email"}, status=400 + ) + responses.add( + responses.POST, app.config["NOTIFY_SVC_URL"], json={"message": "bad email"}, status=400 + ) + responses.add( + responses.POST, app.config["NOTIFY_SVC_URL"], json={"message": "bad email"}, status=400 + ) + + with app.test_request_context("/", method="POST", data=b"{}"): + status = worker()[1] + + assert status == HTTPStatus.BAD_REQUEST + assert len(responses.calls) == 3 + + +def test_worker_legacy_request_exception_returns_502(app, mocker, ce_factory): + """A request exception should be treated as system failure and return 502 if all fail.""" + ce = ce_factory(applicationNumber="APP-1", emailType="HOST_DECLINED") + app_obj = MagicMock( + application_number="APP-1", + registration_type=Registration.RegistrationType.HOST, + noc=None, + ) + _patch_legacy_application_flow(mocker, app_obj, ce) + mocker.patch( + "strr_email.resources.email_listener.ApplicationSerializer.to_dict", + return_value=_multi_recipient_app_dict(), + ) + + with patch( + "strr_email.resources.email_listener.requests.post", + side_effect=RuntimeError("network timeout"), + ) as mock_post: + with app.test_request_context("/", method="POST", data=b"{}"): + status = worker()[1] + + assert status == HTTPStatus.BAD_GATEWAY + assert mock_post.call_count == 3 diff --git a/strr-api/src/strr_api/services/interaction.py b/strr-api/src/strr_api/services/interaction.py index 90aa044a6..bce4761fe 100644 --- a/strr-api/src/strr_api/services/interaction.py +++ b/strr-api/src/strr_api/services/interaction.py @@ -94,7 +94,6 @@ def dispatch( customer_id: int | None = None, ): """Dispatch interaction.""" - the_type = type(payload) match channel_type: case ChannelType.EMAIL: if not isinstance(payload, EmailInfo): @@ -108,7 +107,7 @@ def dispatch( not notify_json or not isinstance(notify_json, dict) or not (notify_id := notify_json.get("id")) - or (notify_id < 1) + or (isinstance(notify_id, (int, float)) and notify_id < 1) ): raise ExternalServiceException(error="Email not sent", status_code=HTTPStatus.BAD_REQUEST) @@ -120,7 +119,13 @@ def dispatch( interaction.registration_id = registration_id interaction.customer_id = customer_id interaction.user_id = user_id - interaction.notify_reference = notify_id + # Keep notify_reference backward compatible as a single notify ID + interaction.notify_reference = str(notify_id) + if notify_ids := notify_json.get("ids"): + interaction.meta_data = { + **(interaction.meta_data or {}), + "notify_references": notify_ids, + } interaction.idempotency_key = idempotency_key interaction.save() @@ -169,11 +174,43 @@ def queued( @staticmethod def _send_email_to_notify_service(email_info): + """Send an email via the notify-api. + + When the email has multiple recipients, dispatch a separate notify request + per recipient so a single malformed address cannot block delivery to the + rest of the recipients. + """ + email = email_info.email if isinstance(email_info.email, dict) else None + raw_recipients = (email or {}).get("recipients") if email else None + recipients = [r.strip() for r in (raw_recipients or "").split(",") if r.strip()] + + if len(recipients) <= 1: + return InteractionService._post_email_to_notify(email_info.email) + + success_ids: list = [] + last_result: dict = {"id": -1} + for recipient in recipients: + single_email = {**email, "recipients": recipient} + result = InteractionService._post_email_to_notify(single_email) + notify_id = result.get("id") if isinstance(result, dict) else None + if notify_id and not (isinstance(notify_id, (int, float)) and notify_id < 1): + success_ids.append(notify_id) + last_result = result + + if not success_ids: + return {"id": -1} + + combined_ids = ",".join(str(i) for i in success_ids)[:100] + return {**last_result, "id": success_ids[0], "ids": combined_ids} + + @staticmethod + def _post_email_to_notify(email_payload): + """Post a single email payload to the notify-api.""" token = AuthService.get_service_client_token() try: resp = requests.post( current_app.config["NOTIFY_SVC_URL"], - json=email_info.email, + json=email_payload, headers={ "Content-Type": "application/json", "Authorization": f"Bearer {token}", diff --git a/strr-api/tests/unit/services/test_interaction_service.py b/strr-api/tests/unit/services/test_interaction_service.py index 0ad2214ef..4106f4d6c 100644 --- a/strr-api/tests/unit/services/test_interaction_service.py +++ b/strr-api/tests/unit/services/test_interaction_service.py @@ -16,7 +16,7 @@ Test suite to ensure that the Interaction service routines are working as expected. """ from http import HTTPStatus -from unittest.mock import patch +from unittest.mock import MagicMock, patch import pytest @@ -206,3 +206,97 @@ def test_dispatch_email_interaction_failure( mock_requests_post.assert_called_once() assert excinfo.value.status_code == HTTPStatus.BAD_REQUEST assert excinfo.value.error == "'Email not sent', 400" + + +@pytest.mark.conf(NOTIFY_SVC_URL="dummy", NOTIFY_API_TIMEOUT=30) +@patch("strr_api.services.auth_service.AuthService.get_service_client_token", return_value="dummy_token") +@patch("strr_api.services.interaction.requests.post") +def test_dispatch_email_splits_recipients(mock_requests_post, mock_get_token, session, setup_parents, inject_config): + """Assert that each recipient is sent to notify-api as a separate request.""" + mock_responses = [ + MagicMock(status_code=HTTPStatus.OK, **{"json.return_value": {"id": 111}}), + MagicMock(status_code=HTTPStatus.OK, **{"json.return_value": {"id": 222}}), + MagicMock(status_code=HTTPStatus.OK, **{"json.return_value": {"id": 333}}), + ] + mock_requests_post.side_effect = mock_responses + + email_payload = { + "recipients": "foo@foo.com,bar@bar.com,baz@baz.com", + "requestBy": "STRR", + "content": {"subject": "s", "body": "b"}, + } + email_info = EmailInfo(application_number="123", email_type="HOST_RENEWAL_REMINDER", email=email_payload) + + interaction = InteractionService.dispatch( + channel_type=ChannelType.EMAIL, + payload=email_info, + application_id=setup_parents["application_id"], + ) + + assert mock_requests_post.call_count == 3 + posted_recipients = [call.kwargs["json"]["recipients"] for call in mock_requests_post.call_args_list] + assert posted_recipients == ["foo@foo.com", "bar@bar.com", "baz@baz.com"] + assert interaction.notify_reference == "111" + assert interaction.meta_data == {"notify_references": "111,222,333"} + + +@pytest.mark.conf(NOTIFY_SVC_URL="dummy", NOTIFY_API_TIMEOUT=30) +@patch("strr_api.services.auth_service.AuthService.get_service_client_token", return_value="dummy_token") +@patch("strr_api.services.interaction.requests.post") +def test_dispatch_email_partial_recipient_failure( + mock_requests_post, mock_get_token, session, setup_parents, inject_config +): + """Assert that a malformed recipient does not block delivery to valid recipients.""" + mock_responses = [ + MagicMock(status_code=HTTPStatus.OK, **{"json.return_value": {"id": 111}}), + MagicMock(status_code=HTTPStatus.BAD_REQUEST, **{"json.return_value": {"message": "bad email"}}), + MagicMock(status_code=HTTPStatus.OK, **{"json.return_value": {"id": 333}}), + ] + mock_requests_post.side_effect = mock_responses + + email_payload = { + "recipients": "foo@foo.com,bas@ba$.com,baz@baz.com", + "requestBy": "STRR", + "content": {"subject": "s", "body": "b"}, + } + email_info = EmailInfo(application_number="123", email_type="HOST_RENEWAL_REMINDER", email=email_payload) + + interaction = InteractionService.dispatch( + channel_type=ChannelType.EMAIL, + payload=email_info, + application_id=setup_parents["application_id"], + ) + + assert mock_requests_post.call_count == 3 + assert interaction.notify_reference == "111" + assert interaction.meta_data == {"notify_references": "111,333"} + + +@pytest.mark.conf(NOTIFY_SVC_URL="dummy", NOTIFY_API_TIMEOUT=30) +@patch("strr_api.services.auth_service.AuthService.get_service_client_token", return_value="dummy_token") +@patch("strr_api.services.interaction.requests.post") +def test_dispatch_email_all_recipients_fail(mock_requests_post, mock_get_token, session, setup_parents, inject_config): + """Assert that if all recipient sends fail the dispatch raises an exception.""" + mock_responses = [ + MagicMock(status_code=HTTPStatus.BAD_REQUEST, **{"json.return_value": {"message": "bad email"}}), + MagicMock(status_code=HTTPStatus.BAD_REQUEST, **{"json.return_value": {"message": "bad email"}}), + ] + mock_requests_post.side_effect = mock_responses + + email_payload = { + "recipients": "bas@ba$.com,also$bad.com", + "requestBy": "STRR", + "content": {"subject": "s", "body": "b"}, + } + email_info = EmailInfo(application_number="123", email_type="HOST_RENEWAL_REMINDER", email=email_payload) + + with pytest.raises(ExternalServiceException) as excinfo: + InteractionService.dispatch( + channel_type=ChannelType.EMAIL, + payload=email_info, + application_id=setup_parents["application_id"], + ) + + assert mock_requests_post.call_count == 2 + assert excinfo.value.status_code == HTTPStatus.BAD_REQUEST + assert excinfo.value.error == "'Email not sent', 400"