diff --git a/h/services/__init__.py b/h/services/__init__.py index 3ed27124858..a48d0f253a1 100644 --- a/h/services/__init__.py +++ b/h/services/__init__.py @@ -10,6 +10,7 @@ BulkGroupService, BulkLMSStatsService, ) +from h.services.email import EmailService from h.services.job_queue import JobQueueService from h.services.subscription import SubscriptionService @@ -161,3 +162,4 @@ def includeme(config): # pragma: no cover config.register_service_factory( "h.services.analytics.analytics_service_factory", name="analytics" ) + config.register_service_factory("h.services.email.factory", iface=EmailService) diff --git a/h/services/email.py b/h/services/email.py new file mode 100644 index 00000000000..34d7ee09f5f --- /dev/null +++ b/h/services/email.py @@ -0,0 +1,43 @@ +# noqa: A005 + +import smtplib + +import pyramid_mailer +import pyramid_mailer.message +from pyramid.request import Request +from pyramid_mailer import IMailer + +from h.tasks.celery import get_task_logger + +logger = get_task_logger(__name__) + + +class EmailService: + """A service for sending emails.""" + + def __init__(self, request: Request, mailer: IMailer) -> None: + self._request = request + self._mailer = mailer + + def send( + self, recipients: list[str], subject: str, body: str, html: str | None = None + ): + email = pyramid_mailer.message.Message( + subject=subject, recipients=recipients, body=body, html=html + ) + if self._request.debug: # pragma: no cover + logger.info("emailing in debug mode: check the `mail/` directory") + try: + self._mailer.send_immediately(email) + except smtplib.SMTPRecipientsRefused as exc: # pragma: no cover + logger.warning( + "Recipient was refused when trying to send an email. Does the user have an invalid email address?", + exc_info=exc, + ) + except smtplib.SMTPException: + raise + + +def factory(_context, request: Request) -> EmailService: + mailer = pyramid_mailer.get_mailer(request) + return EmailService(request, mailer) diff --git a/h/tasks/mailer.py b/h/tasks/mailer.py index 55ace8871f2..bf4ee51af2e 100644 --- a/h/tasks/mailer.py +++ b/h/tasks/mailer.py @@ -6,15 +6,11 @@ import smtplib -import pyramid_mailer -import pyramid_mailer.message - -from h.tasks.celery import celery, get_task_logger +from h.services.email import EmailService +from h.tasks.celery import celery __all__ = ("send",) -log = get_task_logger(__name__) - @celery.task(bind=True, max_retries=3, acks_late=True) def send(self, recipients, subject, body, html=None): @@ -30,19 +26,9 @@ def send(self, recipients, subject, body, html=None): :param body: the body of the email :type body: unicode """ - email = pyramid_mailer.message.Message( - subject=subject, recipients=recipients, body=body, html=html - ) - mailer = pyramid_mailer.get_mailer(celery.request) - if celery.request.debug: # pragma: no cover - log.info("emailing in debug mode: check the `mail/' directory") + service = celery.request.find_service(EmailService) try: - mailer.send_immediately(email) - except smtplib.SMTPRecipientsRefused as exc: # pragma: no cover - log.warning( - "Recipient was refused when trying to send an email. Does the user have an invalid email address?", - exc_info=exc, - ) + service.send(recipients=recipients, subject=subject, body=body, html=html) except smtplib.socket.error as exc: # Exponential backoff in case the SMTP service is having problems. countdown = self.default_retry_delay * 2**self.request.retries diff --git a/tests/common/fixtures/services.py b/tests/common/fixtures/services.py index 13646b591e6..2c447815faf 100644 --- a/tests/common/fixtures/services.py +++ b/tests/common/fixtures/services.py @@ -19,6 +19,7 @@ BulkLMSStatsService, ) from h.services.developer_token import DeveloperTokenService +from h.services.email import EmailService from h.services.flag import FlagService from h.services.group import GroupService from h.services.group_create import GroupCreateService @@ -58,6 +59,7 @@ "bulk_group_service", "bulk_stats_service", "developer_token_service", + "email_service", "flag_service", "group_create_service", "group_delete_service", @@ -306,3 +308,8 @@ def user_unique_service(mock_service): @pytest.fixture def user_update_service(mock_service): return mock_service(UserUpdateService, name="user_update") + + +@pytest.fixture +def email_service(mock_service): + return mock_service(EmailService) diff --git a/tests/unit/h/services/email_test.py b/tests/unit/h/services/email_test.py new file mode 100644 index 00000000000..83c2c1c08d1 --- /dev/null +++ b/tests/unit/h/services/email_test.py @@ -0,0 +1,94 @@ +import smtplib +from unittest.mock import sentinel + +import pytest + +from h.services.email import EmailService, factory + + +class TestEmailService: + def test_send_creates_email_message(self, email_service, pyramid_mailer): + email_service.send( + recipients=["foo@example.com"], + subject="My email subject", + body="Some text body", + ) + + pyramid_mailer.message.Message.assert_called_once_with( + recipients=["foo@example.com"], + subject="My email subject", + body="Some text body", + html=None, + ) + + def test_send_creates_email_message_with_html_body( + self, email_service, pyramid_mailer + ): + email_service.send( + recipients=["foo@example.com"], + subject="My email subject", + body="Some text body", + html="
An HTML body
", + ) + + pyramid_mailer.message.Message.assert_called_once_with( + recipients=["foo@example.com"], + subject="My email subject", + body="Some text body", + html="An HTML body
", + ) + + def test_send_dispatches_email_using_request_mailer( + self, email_service, pyramid_mailer + ): + request_mailer = pyramid_mailer.get_mailer.return_value + message = pyramid_mailer.message.Message.return_value + + email_service.send( + recipients=["foo@example.com"], + subject="My email subject", + body="Some text body", + ) + + request_mailer.send_immediately.assert_called_once_with(message) + + def test_raises_smtplib_exception(self, email_service, pyramid_mailer): + request_mailer = pyramid_mailer.get_mailer.return_value + request_mailer.send_immediately.side_effect = smtplib.SMTPException() + + with pytest.raises(smtplib.SMTPException): + email_service.send( + recipients=["foo@example.com"], + subject="My email subject", + body="Some text body", + ) + + @pytest.fixture + def pyramid_request(self, pyramid_request): + pyramid_request.debug = False + return pyramid_request + + @pytest.fixture + def email_service(self, pyramid_request, pyramid_mailer): + request_mailer = pyramid_mailer.get_mailer.return_value + return EmailService(pyramid_request, request_mailer) + + +class TestFactory: + def test_it(self, pyramid_request, pyramid_mailer, EmailService): + service = factory(sentinel.context, pyramid_request) + + EmailService.assert_called_once_with( + request=pyramid_request, mailer=pyramid_mailer.get_mailer.return_value + ) + + assert service == EmailService.return_value + + @pytest.fixture(autouse=True) + def EmailService(self, patch): + return patch("h.services.email.EmailService") + + +@pytest.fixture(autouse=True) +def pyramid_mailer(patch): + return patch("h.services.email.pyramid_mailer", autospec=True) diff --git a/tests/unit/h/tasks/mailer_test.py b/tests/unit/h/tasks/mailer_test.py index 2d4fdd0cefd..e3e8330796d 100644 --- a/tests/unit/h/tasks/mailer_test.py +++ b/tests/unit/h/tasks/mailer_test.py @@ -6,72 +6,8 @@ from h.tasks import mailer -@mock.patch("h.tasks.mailer.celery", autospec=True) -@mock.patch("h.tasks.mailer.pyramid_mailer", autospec=True) -def test_send_creates_email_message(pyramid_mailer, celery, pyramid_request): - celery.request = pyramid_request - - mailer.send( - recipients=["foo@example.com"], - subject="My email subject", - body="Some text body", - ) - - pyramid_mailer.message.Message.assert_called_once_with( - subject="My email subject", - recipients=["foo@example.com"], - body="Some text body", - html=None, - ) - - -@mock.patch("h.tasks.mailer.celery", autospec=True) -@mock.patch("h.tasks.mailer.pyramid_mailer", autospec=True) -def test_send_creates_email_message_with_html_body( - pyramid_mailer, celery, pyramid_request -): - celery.request = pyramid_request - - mailer.send( - recipients=["foo@example.com"], - subject="My email subject", - body="Some text body", - html="An HTML body
", - ) - - pyramid_mailer.message.Message.assert_called_once_with( - subject="My email subject", - recipients=["foo@example.com"], - body="Some text body", - html="An HTML body
", - ) - - -@mock.patch("h.tasks.mailer.celery", autospec=True) -@mock.patch("h.tasks.mailer.pyramid_mailer", autospec=True) -def test_send_dispatches_email_using_request_mailer( - pyramid_mailer, celery, pyramid_request -): - celery.request = pyramid_request - request_mailer = pyramid_mailer.get_mailer.return_value - message = pyramid_mailer.message.Message.return_value - - mailer.send( - recipients=["foo@example.com"], - subject="My email subject", - body="Some text body", - ) - - pyramid_mailer.get_mailer.assert_called_once_with(pyramid_request) - request_mailer.send_immediately.assert_called_once_with(message) - - -@mock.patch("h.tasks.mailer.celery", autospec=True) -@mock.patch("h.tasks.mailer.pyramid_mailer", autospec=True) -def test_send_retries_if_mailing_fails(pyramid_mailer, celery, pyramid_request): - celery.request = pyramid_request - request_mailer = pyramid_mailer.get_mailer.return_value - request_mailer.send_immediately.side_effect = SMTPServerDisconnected() +def test_send_retries_if_mailing_fails(email_service): + email_service.send.side_effect = SMTPServerDisconnected() mailer.send.retry = mock.Mock(spec_set=[]) mailer.send( @@ -87,3 +23,10 @@ def test_send_retries_if_mailing_fails(pyramid_mailer, celery, pyramid_request): def pyramid_request(pyramid_request): pyramid_request.debug = False return pyramid_request + + +@pytest.fixture(autouse=True) +def celery(patch, pyramid_request): + celery = patch("h.tasks.mailer.celery") + celery.request = pyramid_request + return celery