From 9893a5409560e1cc7123ec42d12b49e6edd6283c Mon Sep 17 00:00:00 2001 From: Folker Bernitt Date: Thu, 5 Feb 2015 11:21:48 +0100 Subject: (Re-)added error handling for twisted smtp sender. - Issue #249 - Fixed all tests with that rely on sendmail deferred. --- service/pixelated/adapter/services/mail_sender.py | 6 +-- service/pixelated/adapter/services/mail_service.py | 8 +-- service/pixelated/resources/mails_resource.py | 18 +++++-- service/test/integration/test_drafts.py | 57 +++++++++++++++----- .../test/support/integration/soledad_test_base.py | 4 -- service/test/unit/adapter/services/__init__.py | 0 .../test/unit/adapter/services/test_mail_sender.py | 63 ++++++++++++++++++++++ service/test/unit/adapter/test_mail_service.py | 55 +++++++++++++++++-- 8 files changed, 181 insertions(+), 30 deletions(-) create mode 100644 service/test/unit/adapter/services/__init__.py create mode 100644 service/test/unit/adapter/services/test_mail_sender.py (limited to 'service') diff --git a/service/pixelated/adapter/services/mail_sender.py b/service/pixelated/adapter/services/mail_sender.py index d29b7d49..24ae839d 100644 --- a/service/pixelated/adapter/services/mail_sender.py +++ b/service/pixelated/adapter/services/mail_sender.py @@ -22,7 +22,7 @@ from twisted.internet import reactor from pixelated.support.functional import flatten -class MailSender(): +class MailSender(object): def __init__(self, account_email_address, smtp_client=None): self.account_email_address = account_email_address @@ -41,11 +41,11 @@ class MailSender(): def sendmail(self, mail): recipients = flatten([mail.to, mail.cc, mail.bcc]) - normalized_recepients = self.get_email_addresses(recipients) + normalized_recipients = self.get_email_addresses(recipients) resultDeferred = Deferred() senderFactory = SMTPSenderFactory( fromEmail=self.account_email_address, - toEmail=normalized_recepients, + toEmail=normalized_recipients, file=StringIO(mail.to_smtp_format()), deferred=resultDeferred) diff --git a/service/pixelated/adapter/services/mail_service.py b/service/pixelated/adapter/services/mail_service.py index 6e309ee0..1e0a0414 100644 --- a/service/pixelated/adapter/services/mail_service.py +++ b/service/pixelated/adapter/services/mail_service.py @@ -55,10 +55,12 @@ class MailService: mail = InputMail.from_dict(content_dict) draft_id = content_dict.get('ident') - self.mail_sender.sendmail(mail) - sent_mail = self.move_to_sent(draft_id, mail) + def move_to_sent(_): + return self.move_to_sent(draft_id, mail) - return sent_mail + deferred = self.mail_sender.sendmail(mail) + deferred.addCallback(move_to_sent) + return deferred def move_to_sent(self, last_draft_ident, mail): if last_draft_ident: diff --git a/service/pixelated/resources/mails_resource.py b/service/pixelated/resources/mails_resource.py index 77a47cda..f387076b 100644 --- a/service/pixelated/resources/mails_resource.py +++ b/service/pixelated/resources/mails_resource.py @@ -1,7 +1,8 @@ import json from pixelated.adapter.model.mail import InputMail -from pixelated.resources import respond_json +from pixelated.resources import respond_json, respond_json_deferred from twisted.web.resource import Resource +from twisted.web import server from leap.common.events import ( register, events_pb2 as proto @@ -86,9 +87,20 @@ class MailsResource(Resource): def render_POST(self, request): content_dict = json.loads(request.content.read()) - sent_mail = self._mail_service.send_mail(content_dict) - return respond_json(sent_mail.as_dict(), request) + deferred = self._mail_service.send_mail(content_dict) + + def onSuccess(sent_mail): + data = sent_mail.as_dict() + respond_json_deferred(data, request) + + def onError(error): + respond_json_deferred({'message': str(error)}, request, status_code=422) + + deferred.addCallback(onSuccess) + deferred.addErrback(onError) + + return server.NOT_DONE_YET def render_PUT(self, request): content_dict = json.loads(request.content.read()) diff --git a/service/test/integration/test_drafts.py b/service/test/integration/test_drafts.py index c555cb89..a5901b67 100644 --- a/service/test/integration/test_drafts.py +++ b/service/test/integration/test_drafts.py @@ -15,39 +15,68 @@ # along with Pixelated. If not, see . from test.support.integration import * +from mockito import * +from twisted.internet.defer import Deferred class DraftsTest(SoledadTestBase): + def tearDown(self): + unstub() + def test_post_sends_mail_and_deletes_previous_draft_if_it_exists(self): + # act is if sending the mail by SMTP succeeded + sendmail_deferred = Deferred() + when(self.client.mail_sender).sendmail(any()).thenReturn(sendmail_deferred) + # creates one draft first_draft = MailBuilder().with_subject('First draft').build_json() first_draft_ident = self.put_mail(first_draft)[0]['ident'] # sends an updated version of the draft second_draft = MailBuilder().with_subject('Second draft').with_ident(first_draft_ident).build_json() - self.post_mail(second_draft) + deferred_res = self.post_mail(second_draft) - sent_mails = self.get_mails_by_tag('sent') - drafts = self.get_mails_by_tag('drafts') + sendmail_deferred.callback(None) # SMTP succeeded + + def onSuccess(mail): + sent_mails = self.get_mails_by_tag('sent') + drafts = self.get_mails_by_tag('drafts') + + # make sure there is one email in the sent mailbox and it is the second draft + self.assertEquals(1, len(sent_mails)) + self.assertEquals('Second draft', sent_mails[0].subject) - # make sure there is one email in the sent mailbox and it is the second draft - self.assertEquals(1, len(sent_mails)) - self.assertEquals('Second draft', sent_mails[0].subject) + # make sure that there are no drafts in the draft mailbox + self.assertEquals(0, len(drafts)) - # make sure that there are no drafts in the draft mailbox - self.assertEquals(0, len(drafts)) + deferred_res.addCallback(onSuccess) + return deferred_res def test_post_sends_mail_even_when_draft_does_not_exist(self): + # act is if sending the mail by SMTP succeeded + sendmail_deferred = Deferred() + when(self.client.mail_sender).sendmail(any()).thenReturn(sendmail_deferred) + first_draft = MailBuilder().with_subject('First draft').build_json() - self.post_mail(first_draft) + deferred_res = self.post_mail(first_draft) + sendmail_deferred.callback(True) - sent_mails = self.get_mails_by_tag('sent') - drafts = self.get_mails_by_tag('drafts') + def onSuccess(result): + sent_mails = self.get_mails_by_tag('sent') + drafts = self.get_mails_by_tag('drafts') + + self.assertEquals(1, len(sent_mails)) + self.assertEquals('First draft', sent_mails[0].subject) + self.assertEquals(0, len(drafts)) + + deferred_res.addCallback(onSuccess) + return deferred_res - self.assertEquals(1, len(sent_mails)) - self.assertEquals('First draft', sent_mails[0].subject) - self.assertEquals(0, len(drafts)) + def post_mail(self, data): + deferred_res, req = self.client.post('/mails', data) + deferred_res.callback(None) + return deferred_res def test_put_creates_a_draft_if_it_does_not_exist(self): mail = MailBuilder().with_subject('A new draft').build_json() diff --git a/service/test/support/integration/soledad_test_base.py b/service/test/support/integration/soledad_test_base.py index 60b88768..2c8bb023 100644 --- a/service/test/support/integration/soledad_test_base.py +++ b/service/test/support/integration/soledad_test_base.py @@ -42,10 +42,6 @@ class SoledadTestBase(unittest.TestCase): }) return [ResponseMail(m) for m in res['mails']] - def post_mail(self, data): - res, req = self.client.post('/mails', data) - return ResponseMail(res) - def get_attachment(self, ident, encoding): res, req = self.client.get("/attachment/%s" % ident, {'encoding': [encoding]}, as_json=False) return res diff --git a/service/test/unit/adapter/services/__init__.py b/service/test/unit/adapter/services/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/service/test/unit/adapter/services/test_mail_sender.py b/service/test/unit/adapter/services/test_mail_sender.py new file mode 100644 index 00000000..8536e5e3 --- /dev/null +++ b/service/test/unit/adapter/services/test_mail_sender.py @@ -0,0 +1,63 @@ +# +# Copyright (c) 2014 ThoughtWorks, Inc. +# +# Pixelated is free software: you can redistribute it and/or modify +# it under the terms of the GNU Affero General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# Pixelated is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU Affero General Public License for more details. +# +# You should have received a copy of the GNU Affero General Public License +# along with Pixelated. If not, see . +from twisted.trial import unittest + +from mockito import mock, when, verify, any, unstub +from pixelated.adapter.services.mail_sender import MailSender +from pixelated.adapter.model.mail import PixelatedMail, InputMail +from test.support.test_helper import mail_dict +from twisted.internet import reactor +from twisted.internet.defer import Deferred + + +class MailSenderTest(unittest.TestCase): + def test_sendmail(self): + when(reactor).connectTCP('localhost', 4650, any()).thenReturn(None) + input_mail = InputMail.from_dict(mail_dict()) + mail_sender = MailSender('someone@somedomain.tld') + + return self._succeed(mail_sender.sendmail(input_mail)) + + def tearDown(self): + unstub() + + def test_sendmail_uses_twisted(self): + when(reactor).connectTCP('localhost', 4650, any()).thenReturn(None) + + input_mail = InputMail.from_dict(mail_dict()) + mail_sender = MailSender('someone@somedomain.tld') + + sent_deferred = mail_sender.sendmail(input_mail) + + verify(reactor).connectTCP('localhost', 4650, any()) + + return self._succeed(sent_deferred) + + def test_senmail_returns_deffered(self): + when(reactor).connectTCP('localhost', 4650, any()).thenReturn(None) + input_mail = InputMail.from_dict(mail_dict()) + mail_sender = MailSender('someone@somedomain.tld') + + deferred = mail_sender.sendmail(input_mail) + + self.assertIsNotNone(deferred) + self.assertTrue(isinstance(deferred, Deferred)) + + return self._succeed(deferred) + + def _succeed(self, deferred): + deferred.callback(None) + return deferred diff --git a/service/test/unit/adapter/test_mail_service.py b/service/test/unit/adapter/test_mail_service.py index 137c17ee..98ead126 100644 --- a/service/test/unit/adapter/test_mail_service.py +++ b/service/test/unit/adapter/test_mail_service.py @@ -13,20 +13,24 @@ # # You should have received a copy of the GNU Affero General Public License # along with Pixelated. If not, see . -import unittest +from twisted.trial import unittest from pixelated.adapter.model.mail import InputMail, PixelatedMail from pixelated.adapter.services.mail_service import MailService from test.support.test_helper import mail_dict, leap_mail from mockito import * +from twisted.internet.defer import Deferred + +from twisted.internet import defer class TestMailService(unittest.TestCase): def setUp(self): + self.drafts = mock() self.querier = mock() self.mailboxes = mock() self.tag_service = mock() - self.mailboxes.drafts = lambda: mock() + self.mailboxes.drafts = lambda: self.drafts self.mailboxes.trash = lambda: mock() self.mailboxes.sent = lambda: mock() @@ -34,13 +38,58 @@ class TestMailService(unittest.TestCase): self.search_engine = mock() self.mail_service = MailService(self.mailboxes, self.mail_sender, self.tag_service, self.querier, self.search_engine) + def tearDown(self): + unstub() + def test_send_mail(self): when(InputMail).from_dict(any()).thenReturn('inputmail') + when(self.mail_sender).sendmail(any()).thenReturn(Deferred()) - self.mail_service.send_mail(mail_dict()) + sent_deferred = self.mail_service.send_mail(mail_dict()) verify(self.mail_sender).sendmail("inputmail") + sent_deferred.callback('Assume sending mail succeeded') + + return sent_deferred + + def test_send_mail_removes_draft(self): + mail_ident = 'Some ident' + mail = mail_dict() + mail['ident'] = mail_ident + when(InputMail).from_dict(any()).thenReturn('inputmail') + deferred = Deferred() + when(self.mail_sender).sendmail(any()).thenReturn(deferred) + + sent_deferred = self.mail_service.send_mail(mail) + + verify(self.mail_sender).sendmail("inputmail") + + def assert_removed_from_drafts(_): + verify(self.drafts).remove(any()) + + sent_deferred.addCallback(assert_removed_from_drafts) + sent_deferred.callback('Assume sending mail succeeded') + + return sent_deferred + + def test_send_mail_does_not_delete_draft_on_error(self): + when(InputMail).from_dict(any()).thenReturn('inputmail') + when(self.mail_sender).sendmail(any()).thenReturn(Deferred()) + + send_deferred = self.mail_service.send_mail(mail_dict()) + + verify(self.mail_sender).sendmail("inputmail") + + def assert_not_removed_from_drafts(_): + verifyNoMoreInteractions(self.drafts) + + send_deferred.addErrback(assert_not_removed_from_drafts) + + send_deferred.errback(Exception('Assume sending mail failed')) + + return send_deferred + def test_mark_as_read(self): mail = mock() when(self.mail_service).mail(any()).thenReturn(mail) -- cgit v1.2.3