From dd63db87fdbb9667c1027edd700b4047b6983d1f Mon Sep 17 00:00:00 2001 From: Duda Dornelles Date: Wed, 18 Feb 2015 13:40:41 -0200 Subject: #294 - not failing service startup if smtp fails to start --- service/pixelated/adapter/services/mail_sender.py | 36 +++++++++++++--------- service/pixelated/bitmask_libraries/session.py | 9 ++++-- service/pixelated/bitmask_libraries/smtp.py | 13 ++++++++ service/pixelated/config/app_factory.py | 3 +- service/pixelated/resources/__init__.py | 5 ++- service/pixelated/resources/mails_resource.py | 6 +++- .../test/unit/adapter/services/test_mail_sender.py | 25 ++++++++++++--- .../test/unit/bitmask_libraries/test_session.py | 3 +- service/test/unit/bitmask_libraries/test_smtp.py | 4 +-- 9 files changed, 75 insertions(+), 29 deletions(-) (limited to 'service') diff --git a/service/pixelated/adapter/services/mail_sender.py b/service/pixelated/adapter/services/mail_sender.py index 24ae839d..9f42fbbc 100644 --- a/service/pixelated/adapter/services/mail_sender.py +++ b/service/pixelated/adapter/services/mail_sender.py @@ -16,14 +16,20 @@ from StringIO import StringIO import re -from twisted.internet.defer import Deferred +from twisted.internet.defer import Deferred, fail from twisted.mail.smtp import SMTPSenderFactory from twisted.internet import reactor from pixelated.support.functional import flatten +class SMTPDownException(Exception): + def __init__(self): + Exception.__init__(self, "Couldn't send mail now, try again later.") + + class MailSender(object): - def __init__(self, account_email_address, smtp_client=None): + def __init__(self, account_email_address, ensure_smtp_is_running_cb): + self.ensure_smtp_is_running_cb = ensure_smtp_is_running_cb self.account_email_address = account_email_address def recepients_normalizer(self, mail_list): @@ -40,15 +46,17 @@ class MailSender(object): return self.recepients_normalizer(clean_mail_list) def sendmail(self, mail): - recipients = flatten([mail.to, mail.cc, mail.bcc]) - normalized_recipients = self.get_email_addresses(recipients) - resultDeferred = Deferred() - senderFactory = SMTPSenderFactory( - fromEmail=self.account_email_address, - toEmail=normalized_recipients, - file=StringIO(mail.to_smtp_format()), - deferred=resultDeferred) - - reactor.connectTCP('localhost', 4650, senderFactory) - - return resultDeferred + if self.ensure_smtp_is_running_cb(): + recipients = flatten([mail.to, mail.cc, mail.bcc]) + normalized_recipients = self.get_email_addresses(recipients) + resultDeferred = Deferred() + senderFactory = SMTPSenderFactory( + fromEmail=self.account_email_address, + toEmail=normalized_recipients, + file=StringIO(mail.to_smtp_format()), + deferred=resultDeferred) + + reactor.connectTCP('localhost', 4650, senderFactory) + + return resultDeferred + return fail(SMTPDownException()) diff --git a/service/pixelated/bitmask_libraries/session.py b/service/pixelated/bitmask_libraries/session.py index 158d6605..9f21fbe6 100644 --- a/service/pixelated/bitmask_libraries/session.py +++ b/service/pixelated/bitmask_libraries/session.py @@ -14,6 +14,7 @@ # You should have received a copy of the GNU Affero General Public License # along with Pixelated. If not, see . import errno +import logging import traceback import sys @@ -65,7 +66,7 @@ class LeapSession(object): - ``incoming_mail_fetcher`` Background job for fetching incoming mails from LEAP server (LeapIncomingMail) """ - def __init__(self, provider, srp_session, soledad_session, nicknym, soledad_account, incoming_mail_fetcher): + def __init__(self, provider, srp_session, soledad_session, nicknym, soledad_account, incoming_mail_fetcher, smtp): """ Constructor. @@ -73,6 +74,7 @@ class LeapSession(object): :type leap_config: LeapConfig """ + self.smtp = smtp self.config = provider.config self.provider = provider self.srp_session = srp_session @@ -133,9 +135,10 @@ class LeapSessionFactory(object): account, auth) smtp = LeapSmtp(self._provider, nicknym.keymanager, auth) - smtp.start() - return LeapSession(self._provider, auth, soledad, nicknym, account, incoming_mail_fetcher) + smtp.ensure_running() + + return LeapSession(self._provider, auth, soledad, nicknym, account, incoming_mail_fetcher, smtp) def _lookup_session(self, key): global SESSIONS diff --git a/service/pixelated/bitmask_libraries/smtp.py b/service/pixelated/bitmask_libraries/smtp.py index 6bbe0051..d5236e8e 100644 --- a/service/pixelated/bitmask_libraries/smtp.py +++ b/service/pixelated/bitmask_libraries/smtp.py @@ -13,12 +13,16 @@ # # You should have received a copy of the GNU Affero General Public License # along with Pixelated. If not, see . +import logging import os import requests from .certs import which_bundle from leap.mail.smtp import setup_smtp_gateway +logger = logging.getLogger(__name__) + + class LeapSmtp(object): TWISTED_PORT = 4650 @@ -86,6 +90,15 @@ class LeapSmtp(object): encrypted_only=False ) + def ensure_running(self): + if not self._smtp_service: + try: + self.start() + except Exception as e: + logger.warning("Couldn't start the SMTP server now, will try again when the user tries to use it") + return False + return True + def stop(self): if self._smtp_service is not None: self._smtp_port.stopListening() diff --git a/service/pixelated/config/app_factory.py b/service/pixelated/config/app_factory.py index 8671eaa6..f63b49ed 100644 --- a/service/pixelated/config/app_factory.py +++ b/service/pixelated/config/app_factory.py @@ -102,7 +102,8 @@ def init_app(app, leap_home, leap_session): tag_service = TagService() search_engine = SearchEngine(soledad_querier, agent_home=leap_home) - pixelated_mail_sender = MailSender(leap_session.account_email()) + pixelated_mail_sender = MailSender(leap_session.account_email(), + lambda: leap_session.smtp.ensure_running()) pixelated_mailboxes = Mailboxes(leap_session.account, soledad_querier, search_engine) draft_service = DraftService(pixelated_mailboxes) diff --git a/service/pixelated/resources/__init__.py b/service/pixelated/resources/__init__.py index a2e4c9d4..b244900a 100644 --- a/service/pixelated/resources/__init__.py +++ b/service/pixelated/resources/__init__.py @@ -14,6 +14,8 @@ # You should have received a copy of the GNU Affero General Public License # along with Pixelated. If not, see . +import json + def respond_json(entity, request, status_code=200): json_response = json.dumps(entity) @@ -28,6 +30,3 @@ def respond_json_deferred(entity, request, status_code=200): request.code = status_code request.write(json_response) request.finish() - - -import json diff --git a/service/pixelated/resources/mails_resource.py b/service/pixelated/resources/mails_resource.py index f387076b..c057031a 100644 --- a/service/pixelated/resources/mails_resource.py +++ b/service/pixelated/resources/mails_resource.py @@ -1,4 +1,5 @@ import json +from pixelated.adapter.services.mail_sender import SMTPDownException from pixelated.adapter.model.mail import InputMail from pixelated.resources import respond_json, respond_json_deferred from twisted.web.resource import Resource @@ -95,7 +96,10 @@ class MailsResource(Resource): respond_json_deferred(data, request) def onError(error): - respond_json_deferred({'message': str(error)}, request, status_code=422) + if isinstance(error.value, SMTPDownException): + respond_json_deferred({'message': str(error.value)}, request, status_code=503) + else: + respond_json_deferred({'message': str(error)}, request, status_code=422) deferred.addCallback(onSuccess) deferred.addErrback(onError) diff --git a/service/test/unit/adapter/services/test_mail_sender.py b/service/test/unit/adapter/services/test_mail_sender.py index 8536e5e3..0e7ec015 100644 --- a/service/test/unit/adapter/services/test_mail_sender.py +++ b/service/test/unit/adapter/services/test_mail_sender.py @@ -16,7 +16,7 @@ from twisted.trial import unittest from mockito import mock, when, verify, any, unstub -from pixelated.adapter.services.mail_sender import MailSender +from pixelated.adapter.services.mail_sender import MailSender, SMTPDownException from pixelated.adapter.model.mail import PixelatedMail, InputMail from test.support.test_helper import mail_dict from twisted.internet import reactor @@ -24,10 +24,14 @@ from twisted.internet.defer import Deferred class MailSenderTest(unittest.TestCase): + def setUp(self): + self.ensure_smtp_is_running_cb = lambda: True + self.ensure_smtp_is_not_running_cb = lambda: False + 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') + mail_sender = MailSender('someone@somedomain.tld', self.ensure_smtp_is_running_cb) return self._succeed(mail_sender.sendmail(input_mail)) @@ -38,7 +42,8 @@ class MailSenderTest(unittest.TestCase): when(reactor).connectTCP('localhost', 4650, any()).thenReturn(None) input_mail = InputMail.from_dict(mail_dict()) - mail_sender = MailSender('someone@somedomain.tld') + + mail_sender = MailSender('someone@somedomain.tld', self.ensure_smtp_is_running_cb) sent_deferred = mail_sender.sendmail(input_mail) @@ -49,7 +54,7 @@ class MailSenderTest(unittest.TestCase): 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') + mail_sender = MailSender('someone@somedomain.tld', self.ensure_smtp_is_running_cb) deferred = mail_sender.sendmail(input_mail) @@ -58,6 +63,18 @@ class MailSenderTest(unittest.TestCase): return self._succeed(deferred) + def test_doesnt_send_mail_if_smtp_is_not_running(self): + mail_sender = MailSender('someone@somedomain.tld', self.ensure_smtp_is_not_running_cb) + + deferred = mail_sender.sendmail({}) + + def _assert(_): + self.assertTrue(isinstance(deferred.result.value, SMTPDownException)) + + deferred.addErrback(_assert) + + return deferred + def _succeed(self, deferred): deferred.callback(None) return deferred diff --git a/service/test/unit/bitmask_libraries/test_session.py b/service/test/unit/bitmask_libraries/test_session.py index 67722557..0bfd59f2 100644 --- a/service/test/unit/bitmask_libraries/test_session.py +++ b/service/test/unit/bitmask_libraries/test_session.py @@ -24,6 +24,7 @@ class SessionTest(AbstractLeapTest): def setUp(self): self.mail_fetcher_mock = MagicMock() + self.smtp_mock = MagicMock() def tearDown(self): self.mail_fetcher_mock = MagicMock() @@ -65,7 +66,7 @@ class SessionTest(AbstractLeapTest): def _create_session(self): return LeapSession(self.provider, self.srp_session, self.soledad_session, self.nicknym, self.soledad_account, - self.mail_fetcher_mock) + self.mail_fetcher_mock, self.smtp_mock) def _execute_func(func): diff --git a/service/test/unit/bitmask_libraries/test_smtp.py b/service/test/unit/bitmask_libraries/test_smtp.py index 4087cbf5..c0e30573 100644 --- a/service/test/unit/bitmask_libraries/test_smtp.py +++ b/service/test/unit/bitmask_libraries/test_smtp.py @@ -71,7 +71,7 @@ class LeapSmtpTest(AbstractLeapTest): smtp.TWISTED_PORT = port gateway_mock.return_value = (None, None) with HTTMock(ca_cert_mock, not_found_mock): - smtp.start() + smtp.ensure_running() cert_path = self._client_cert_path() gateway_mock.assert_called_with(keymanager=self.keymanager, smtp_cert=cert_path, smtp_key=cert_path, userid='test_user@some-server.test', smtp_port='1234', encrypted_only=False, smtp_host='smtp.some-sever.test', port=port) @@ -91,7 +91,7 @@ class LeapSmtpTest(AbstractLeapTest): gateway_mock.return_value = (smtp_service, smtp_port) with HTTMock(ca_cert_mock, not_found_mock): - smtp.start() + smtp.ensure_running() smtp.stop() smtp_port.stopListening.assert_called_with() -- cgit v1.2.3