Move email handling into a service (pypi#3493)

Author: dstufft

tests/unit/email/__init__.py

@@ -0,0 +1,11 @@
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.

tests/unit/test_email.py renamed to tests/unit/email/test_init.py

@@ -14,57 +14,50 @@
 import pretend
 import pytest
 
-from pyramid_mailer.message import Message
-from pyramid_mailer.interfaces import IMailer
-
 from warehouse import email
 from warehouse.accounts.interfaces import ITokenService
+from warehouse.email.interfaces import IEmailSender
 
 
 class TestSendEmail:
 
     def test_send_email_success(self, monkeypatch):
-        message_obj = Message()
 
-        def mock_message(*args, **kwargs):
-            return message_obj
+        class FakeMailSender:
+
+            def __init__(self):
+                self.emails = []
 
-        monkeypatch.setattr(email, "Message", mock_message)
+            def send(self, subject, body, *, recipient):
+                self.emails.append(
+                    {"subject": subject, "body": body, "recipient": recipient},
+                )
 
+        sender = FakeMailSender()
         task = pretend.stub()
-        mailer = pretend.stub(
-            send_immediately=pretend.call_recorder(lambda i: None)
-        )
         request = pretend.stub(
-            registry=pretend.stub(
-                settings=pretend.stub(
-                    get=pretend.call_recorder(lambda k: 'SENDER'),
-                ),
-                getUtility=pretend.call_recorder(lambda mailr: mailer)
-            )
+            find_service=pretend.call_recorder(lambda *a, **kw: sender),
         )
 
         email.send_email(
             task,
             request,
-            "body",
             "subject",
-            recipients=["recipients"],
+            "body",
+            recipient="recipient",
         )
 
-        assert mailer.send_immediately.calls == [pretend.call(message_obj)]
-        assert request.registry.getUtility.calls == [pretend.call(IMailer)]
-        assert request.registry.settings.get.calls == [
-            pretend.call("mail.sender")]
+        assert request.find_service.calls == [pretend.call(IEmailSender)]
+        assert sender.emails == [
+            {"subject": "subject", "body": "body", "recipient": "recipient"},
+        ]
 
     def test_send_email_failure(self, monkeypatch):
         exc = Exception()
-        message_obj = Message()
 
-        class Mailer:
-            @staticmethod
-            @pretend.call_recorder
-            def send_immediately(message):
+        class FakeMailSender:
+
+            def send(self, subject, body, *, recipient):
                 raise exc
 
         class Task:
@@ -73,34 +66,18 @@ class Task:
             def retry(exc):
                 raise celery.exceptions.Retry
 
-        def mock_message(*args, **kwargs):
-            return message_obj
-
-        monkeypatch.setattr(email, "Message", mock_message)
-
-        mailer, task = Mailer(), Task()
-        request = pretend.stub(
-            registry=pretend.stub(
-                settings=pretend.stub(
-                    get=pretend.call_recorder(lambda k: 'SENDER'),
-                ),
-                getUtility=pretend.call_recorder(lambda mailr: mailer)
-            )
-        )
+        sender, task = FakeMailSender(), Task()
+        request = pretend.stub(find_service=lambda *a, **kw: sender)
 
         with pytest.raises(celery.exceptions.Retry):
             email.send_email(
                 task,
                 request,
-                "body",
                 "subject",
-                recipients=["recipients"],
+                "body",
+                recipient="recipient",
             )
 
-        assert mailer.send_immediately.calls == [pretend.call(message_obj)]
-        assert request.registry.getUtility.calls == [pretend.call(IMailer)]
-        assert request.registry.settings.get.calls == [
-            pretend.call("mail.sender")]
         assert task.retry.calls == [pretend.call(exc=exc)]
 
 
@@ -167,9 +144,9 @@ def test_send_password_reset_email(
         ]
         assert send_email.delay.calls == [
             pretend.call(
-                'Email Body',
                 'Email Subject',
-                recipients=[stub_user.email],
+                'Email Body',
+                recipient=stub_user.email,
             ),
         ]
 
@@ -232,9 +209,9 @@ def test_email_verification_email(
         ]
         assert send_email.delay.calls == [
             pretend.call(
-                'Email Body',
                 'Email Subject',
-                recipients=[stub_email.email],
+                'Email Body',
+                recipient=stub_email.email,
             ),
         ]
 
@@ -280,9 +257,9 @@ def test_password_change_email(
         ]
         assert send_email.delay.calls == [
             pretend.call(
-                'Email Body',
                 'Email Subject',
-                recipients=[stub_user.email],
+                'Email Body',
+                recipient=stub_user.email,
             ),
         ]
 
@@ -328,9 +305,9 @@ def test_account_deletion_email(
         ]
         assert send_email.delay.calls == [
             pretend.call(
-                'Email Body',
                 'Email Subject',
-                recipients=[stub_user.email],
+                'Email Body',
+                recipient=stub_user.email,
             ),
         ]
 
@@ -379,9 +356,9 @@ def test_primary_email_change_email(
         ]
         assert send_email.delay.calls == [
             pretend.call(
-                'Email Body',
                 'Email Subject',
-                recipients=['old_email'],
+                'Email Body',
+                recipient='old_email',
             ),
         ]
 
@@ -439,12 +416,18 @@ def test_collaborator_added_email(
 
         assert pyramid_request.task.calls == [
             pretend.call(send_email),
+            pretend.call(send_email),
         ]
         assert send_email.delay.calls == [
             pretend.call(
+                'Email Subject',
                 'Email Body',
+                recipient=stub_user.email,
+            ),
+            pretend.call(
                 'Email Subject',
-                bcc=[stub_user.email, stub_submitter_user.email],
+                'Email Body',
+                recipient=stub_submitter_user.email,
             ),
         ]
 
@@ -502,8 +485,8 @@ def test_added_as_collaborator_email(
         ]
         assert send_email.delay.calls == [
             pretend.call(
-                'Email Body',
                 'Email Subject',
-                recipients=[stub_user.email],
+                'Email Body',
+                recipient=stub_user.email,
             ),
         ]

tests/unit/email/test_services.py

@@ -0,0 +1,56 @@
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+import pretend
+
+from pyramid_mailer.mailer import DummyMailer
+from zope.interface.verify import verifyClass
+
+from warehouse.email.interfaces import IEmailSender
+from warehouse.email.services import SMTPEmailSender
+
+
+class TestSMTPEmailSender:
+
+    def test_verify_service(self):
+        assert verifyClass(IEmailSender, SMTPEmailSender)
+
+    def test_creates_service(self):
+        mailer = pretend.stub()
+        context = pretend.stub()
+        request = pretend.stub(
+            registry=pretend.stub(
+                settings=pretend.stub(get=lambda k: "SENDER"),
+                getUtility=lambda mailr: mailer,
+            )
+        )
+
+        service = SMTPEmailSender.create_service(context, request)
+
+        assert isinstance(service, SMTPEmailSender)
+        assert service.mailer is mailer
+        assert service.sender == "SENDER"
+
+    def test_send(self):
+        mailer = DummyMailer()
+        service = SMTPEmailSender(mailer, sender="[email protected]")
+
+        service.send("a subject", "a body", recipient="[email protected]")
+
+        assert len(mailer.outbox) == 1
+
+        msg = mailer.outbox[0]
+
+        assert msg.subject == "a subject"
+        assert msg.body == "a body"
+        assert msg.recipients == ["[email protected]"]
+        assert msg.sender == "[email protected]"

tests/unit/test_config.py

@@ -325,6 +325,7 @@ def __init__(self):
             pretend.call("pyramid_rpc.xmlrpc"),
             pretend.call(".legacy.action_routing"),
             pretend.call(".domain"),
+            pretend.call(".email"),
             pretend.call(".i18n"),
             pretend.call(".db"),
             pretend.call(".tasks"),

warehouse/config.py

@@ -344,6 +344,9 @@ def configure(settings=None):
     # Register support for template views.
     config.add_directive("add_template_view", template_view, action_wrap=False)
 
+    # Register support for sendnging emails
+    config.include(".email")
+
     # Register support for internationalization and localization
     config.include(".i18n")
 

warehouse/email.py renamed to warehouse/email/__init__.py

@@ -11,26 +11,19 @@
 # limitations under the License.
 
 from pyramid.renderers import render
-from pyramid_mailer import get_mailer
-from pyramid_mailer.message import Message
 
 from warehouse import tasks
 from warehouse.accounts.interfaces import ITokenService
+from warehouse.email.interfaces import IEmailSender
+from warehouse.email.services import SMTPEmailSender
 
 
 @tasks.task(bind=True, ignore_result=True, acks_late=True)
-def send_email(task, request, body, subject, *, recipients=None, bcc=None):
-
-    mailer = get_mailer(request)
-    message = Message(
-        body=body,
-        recipients=recipients,
-        bcc=bcc,
-        sender=request.registry.settings.get('mail.sender'),
-        subject=subject
-    )
+def send_email(task, request, subject, body, *, recipient=None):
+    sender = request.find_service(IEmailSender)
+
     try:
-        mailer.send_immediately(message)
+        sender.send(subject, body, recipient=recipient)
     except Exception as exc:
         task.retry(exc=exc)
 
@@ -58,7 +51,7 @@ def send_password_reset_email(request, user):
         'email/password-reset.body.txt', fields, request=request
     )
 
-    request.task(send_email).delay(body, subject, recipients=[user.email])
+    request.task(send_email).delay(subject, body, recipient=user.email)
 
     # Return the fields we used, in case we need to show any of them to the
     # user
@@ -87,7 +80,7 @@ def send_email_verification_email(request, email):
         'email/verify-email.body.txt', fields, request=request
     )
 
-    request.task(send_email).delay(body, subject, recipients=[email.email])
+    request.task(send_email).delay(subject, body, recipient=email.email)
 
     return fields
 
@@ -105,7 +98,7 @@ def send_password_change_email(request, user):
         'email/password-change.body.txt', fields, request=request
     )
 
-    request.task(send_email).delay(body, subject, recipients=[user.email])
+    request.task(send_email).delay(subject, body, recipient=user.email)
 
     return fields
 
@@ -123,7 +116,7 @@ def send_account_deletion_email(request, user):
         'email/account-deleted.body.txt', fields, request=request
     )
 
-    request.task(send_email).delay(body, subject, recipients=[user.email])
+    request.task(send_email).delay(subject, body, recipient=user.email)
 
     return fields
 
@@ -143,7 +136,7 @@ def send_primary_email_change_email(request, user, email):
         'email/primary-email-change.body.txt', fields, request=request
     )
 
-    request.task(send_email).delay(body, subject, recipients=[email])
+    request.task(send_email).delay(subject, body, recipient=email)
 
     return fields
 
@@ -165,7 +158,8 @@ def send_collaborator_added_email(request, user, submitter, project_name, role,
         'email/collaborator-added.body.txt', fields, request=request
     )
 
-    request.task(send_email).delay(body, subject, bcc=email_recipients)
+    for recipient in email_recipients:
+        request.task(send_email).delay(subject, body, recipient=recipient)
 
     return fields
 
@@ -186,6 +180,13 @@ def send_added_as_collaborator_email(request, submitter, project_name, role,
         'email/added-as-collaborator.body.txt', fields, request=request
     )
 
-    request.task(send_email).delay(body, subject, recipients=[user_email])
+    request.task(send_email).delay(subject, body, recipient=user_email)
 
     return fields
+
+
+def includeme(config):
+    config.register_service_factory(
+        SMTPEmailSender.create_service,
+        IEmailSender,
+    )