Refactored out tag service and replaced mail normalization with built-in parseaddr

Also made some minor fixes based on pylint output

12 files changed, 23 insertions, 88 deletions

diff --git a/service/pixelated/adapter/model/mail.py b/service/pixelated/adapter/model/mail.py
index 20cb9a88..f912680f 100644
--- a/service/pixelated/adapter/model/mail.py
+++ b/service/pixelated/adapter/model/mail.py
@@ -273,7 +273,7 @@ class PixelatedMail(Mail):
             if not header_value:
                 continue
             _headers[header] = header_value if type(header_value) is list else header_value.split(',')
-            _headers[header] = map(lambda x: x.strip(), compact(_headers[header]))
+            _headers[header] = [head.strip() for head in compact(_headers[header])]
 
         for header in ['From', 'Subject']:
             _headers[header] = hdoc_headers.get(header)
diff --git a/service/pixelated/adapter/services/mail_sender.py b/service/pixelated/adapter/services/mail_sender.py
index 9f42fbbc..bbcc1721 100644
--- a/service/pixelated/adapter/services/mail_sender.py
+++ b/service/pixelated/adapter/services/mail_sender.py
@@ -14,7 +14,7 @@
 # You should have received a copy of the GNU Affero General Public License
 # along with Pixelated. If not, see <http://www.gnu.org/licenses/>.
 from StringIO import StringIO
-import re
+from email.utils import parseaddr
 
 from twisted.internet.defer import Deferred, fail
 from twisted.mail.smtp import SMTPSenderFactory
@@ -28,35 +28,22 @@ class SMTPDownException(Exception):
 
 
 class MailSender(object):
+
     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):
-        return set(mail_list)
-
-    def get_email_addresses(self, mail_list):
-        clean_mail_list = []
-        for mail_address in mail_list:
-            if "<" in mail_address:
-                match = re.search(r'<(.*)>', mail_address)
-                clean_mail_list.append(match.group(1))
-            else:
-                clean_mail_list.append(mail_address)
-        return self.recepients_normalizer(clean_mail_list)
-
     def sendmail(self, mail):
         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(
+            result_deferred = Deferred()
+            sender_factory = SMTPSenderFactory(
                 fromEmail=self.account_email_address,
-                toEmail=normalized_recipients,
+                toEmail=set([parseaddr(recipient)[1] for recipient in recipients]),
                 file=StringIO(mail.to_smtp_format()),
-                deferred=resultDeferred)
+                deferred=result_deferred)
 
-            reactor.connectTCP('localhost', 4650, senderFactory)
+            reactor.connectTCP('localhost', 4650, sender_factory)
 
-            return resultDeferred
+            return result_deferred
         return fail(SMTPDownException())
diff --git a/service/pixelated/adapter/services/mail_service.py b/service/pixelated/adapter/services/mail_service.py
index 5ef0a188..1bf29542 100644
--- a/service/pixelated/adapter/services/mail_service.py
+++ b/service/pixelated/adapter/services/mail_service.py
@@ -14,13 +14,13 @@
 # You should have received a copy of the GNU Affero General Public License
 # along with Pixelated. If not, see <http://www.gnu.org/licenses/>.
 from pixelated.adapter.model.mail import InputMail
+from pixelated.adapter.services.tag_service import extract_reserved_tags
 
 
 class MailService:
     __slots__ = ['leap_session', 'account', 'mailbox_name']
 
-    def __init__(self, mailboxes, mail_sender, tag_service, soledad_querier, search_engine):
-        self.tag_service = tag_service
+    def __init__(self, mailboxes, mail_sender, soledad_querier, search_engine):
         self.mailboxes = mailboxes
         self.querier = soledad_querier
         self.search_engine = search_engine
@@ -36,7 +36,7 @@ class MailService:
 
     def update_tags(self, mail_id, new_tags):
         new_tags = self._filter_white_space_tags(new_tags)
-        reserved_words = self.tag_service.extract_reserved(new_tags)
+        reserved_words = extract_reserved_tags(new_tags)
         if len(reserved_words):
             raise ValueError('None of the following words can be used as tags: ' + ' '.join(reserved_words))
         new_tags = self._favor_existing_tags_casing(new_tags)
diff --git a/service/pixelated/adapter/services/mailbox.py b/service/pixelated/adapter/services/mailbox.py
index f934abcc..e358abda 100644
--- a/service/pixelated/adapter/services/mailbox.py
+++ b/service/pixelated/adapter/services/mailbox.py
@@ -15,7 +15,7 @@
 # along with Pixelated. If not, see <http://www.gnu.org/licenses/>.
 
 
-class Mailbox:
+class Mailbox(object):
 
     def __init__(self, mailbox_name, querier, search_engine):
         self.mailbox_name = mailbox_name
diff --git a/service/pixelated/adapter/services/mailboxes.py b/service/pixelated/adapter/services/mailboxes.py
index c761255c..a7a3a591 100644
--- a/service/pixelated/adapter/services/mailboxes.py
+++ b/service/pixelated/adapter/services/mailboxes.py
@@ -17,7 +17,7 @@ from pixelated.adapter.services.mailbox import Mailbox
 from pixelated.adapter.listeners.mailbox_indexer_listener import MailboxIndexerListener
 
 
-class Mailboxes():
+class Mailboxes(object):
 
     def __init__(self, account, soledad_querier, search_engine):
         self.account = account
diff --git a/service/pixelated/adapter/services/tag_service.py b/service/pixelated/adapter/services/tag_service.py
index 601392bb..c51da625 100644
--- a/service/pixelated/adapter/services/tag_service.py
+++ b/service/pixelated/adapter/services/tag_service.py
@@ -15,13 +15,9 @@
 # along with Pixelated. If not, see <http://www.gnu.org/licenses/>.
 from pixelated.adapter.model.tag import Tag
 
+SPECIAL_TAGS = {Tag('inbox', True), Tag('sent', True), Tag('drafts', True), Tag('trash', True), Tag('ALL', True)}
 
-class TagService:
 
-    instance = None
-    SPECIAL_TAGS = {Tag('inbox', True), Tag('sent', True), Tag('drafts', True), Tag('trash', True), Tag('ALL', True)}
-
-    @classmethod
-    def extract_reserved(cls, tags):
-        tags = map(lambda tag: tag.lower(), tags)
-        return {tag.name for tag in cls.SPECIAL_TAGS if tag.name in tags}
+def extract_reserved_tags(tags):
+    tags = [tag.lower() for tag in tags]
+    return {tag.name for tag in SPECIAL_TAGS if tag.name in tags}
diff --git a/service/pixelated/config/app_factory.py b/service/pixelated/config/app_factory.py
index f63b49ed..f20b1229 100644
--- a/service/pixelated/config/app_factory.py
+++ b/service/pixelated/config/app_factory.py
@@ -33,7 +33,6 @@ from pixelated.adapter.listeners.mailbox_indexer_listener import MailboxIndexerL
 import pixelated.bitmask_libraries.session as LeapSession
 from pixelated.bitmask_libraries.leap_srp import LeapAuthException
 from requests.exceptions import ConnectionError
-from pixelated.adapter.services.tag_service import TagService
 from leap.common.events import (
     register,
     unregister,
@@ -100,14 +99,13 @@ def init_app(app, leap_home, leap_session):
 
     soledad_querier = SoledadQuerier(soledad=leap_session.account._soledad)
 
-    tag_service = TagService()
     search_engine = SearchEngine(soledad_querier, agent_home=leap_home)
     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)
-    mail_service = MailService(pixelated_mailboxes, pixelated_mail_sender, tag_service, soledad_querier, search_engine)
+    mail_service = MailService(pixelated_mailboxes, pixelated_mail_sender, soledad_querier, search_engine)
 
     MailboxIndexerListener.SEARCH_ENGINE = search_engine
     InputMail.FROM_EMAIL_ADDRESS = leap_session.account_email()
diff --git a/service/test/integration/test_tags.py b/service/test/integration/test_tags.py
index ad723067..2e178a4d 100644
--- a/service/test/integration/test_tags.py
+++ b/service/test/integration/test_tags.py
@@ -16,7 +16,7 @@
 import json
 
 from test.support.integration import *
-from pixelated.adapter.services.tag_service import TagService
+from pixelated.adapter.services.tag_service import SPECIAL_TAGS
 
 
 class TagsTest(SoledadTestBase):
@@ -82,7 +82,7 @@ class TagsTest(SoledadTestBase):
         mail = MailBuilder().with_subject('Mail with tags').build_input_mail()
         self.client.add_mail_to_inbox(mail)
 
-        for tag in TagService.SPECIAL_TAGS:
+        for tag in SPECIAL_TAGS:
             response = self.post_tags(mail.ident, self._tags_json([tag.name.upper()]))
             self.assertEquals("None of the following words can be used as tags: %s" % tag.name, response)
 
diff --git a/service/test/support/integration/app_test_client.py b/service/test/support/integration/app_test_client.py
index 474e5fd3..3ba6646e 100644
--- a/service/test/support/integration/app_test_client.py
+++ b/service/test/support/integration/app_test_client.py
@@ -35,7 +35,6 @@ from pixelated.adapter.services.draft_service import DraftService
 from pixelated.adapter.services.mail_service import MailService
 from pixelated.adapter.services.mailboxes import Mailboxes
 from pixelated.adapter.soledad.soledad_querier import SoledadQuerier
-from pixelated.adapter.services.tag_service import TagService
 from pixelated.config import App
 from pixelated.resources.root_resource import RootResource
 from test.support.integration.model import MailBuilder
@@ -149,8 +148,7 @@ class AppTestClient:
         return mail_sender
 
     def _create_mail_service(self, mailboxes, mail_sender, soledad_querier, search_engine):
-        tag_service = TagService()
-        mail_service = MailService(mailboxes, mail_sender, tag_service, soledad_querier, search_engine)
+        mail_service = MailService(mailboxes, mail_sender, soledad_querier, search_engine)
         return mail_service
 
     def _generate_soledad_test_folder_name(self, soledad_test_folder='/tmp/soledad-test/test'):
diff --git a/service/test/unit/adapter/test_email_recepient_normalizer.py b/service/test/unit/adapter/test_email_recepient_normalizer.py
deleted file mode 100644
index 79d50273..00000000
--- a/service/test/unit/adapter/test_email_recepient_normalizer.py
+++ /dev/null
@@ -1,42 +0,0 @@
-#
-# 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 <http://www.gnu.org/licenses/>.
-import unittest
-
-from pixelated.adapter.model.mail import PixelatedMail
-from pixelated.adapter.services.mailbox import Mailbox
-from pixelated.adapter.services.mail_sender import MailSender
-from mockito import *
-from test.support import test_helper
-
-
-class PixelatedDuplicateEmailTest(unittest.TestCase):
-    def setUp(self):
-        self.mail_sender = MailSender(self, "random@gmail.com")
-
-    def test_remove_duplicate_mail_recepients(self):
-        mail_list = ['simba@gmail.com', 'simba@gmail.com', 'fabio@gmail.com']
-        normalized_recepients = self.mail_sender.recepients_normalizer(mail_list)
-        self.assertEquals(normalized_recepients, set(['simba@gmail.com', 'fabio@gmail.com']))
-
-    def test_get_email_addresses(self):
-        mail_list = ['simbarashe<simba@gmail.com>', 'vic@gmail.com', 'Fabio<fabio@gmail.com>', 'slick@gmail.com']
-        selected_recepients = self.mail_sender.get_email_addresses(mail_list)
-        self.assertEquals(selected_recepients, set(['simba@gmail.com', 'vic@gmail.com', 'fabio@gmail.com', 'slick@gmail.com']))
-
-    def test_remove_duplicate_emails_with_routing_format(self):
-        mail_list = ['simbarashe<simba@gmail.com>', 'simba<simba@gmail.com>', 'Fabio<fabio@gmail.com>', 'Fabinho<fabio@gmail.com>']
-        selected_recepients = self.mail_sender.get_email_addresses(mail_list)
-        self.assertEquals(selected_recepients, set(['simba@gmail.com', 'fabio@gmail.com']))
diff --git a/service/test/unit/adapter/test_mail_service.py b/service/test/unit/adapter/test_mail_service.py
index 98ead126..f815268c 100644
--- a/service/test/unit/adapter/test_mail_service.py
+++ b/service/test/unit/adapter/test_mail_service.py
@@ -29,14 +29,13 @@ class TestMailService(unittest.TestCase):
         self.drafts = mock()
         self.querier = mock()
         self.mailboxes = mock()
-        self.tag_service = mock()
         self.mailboxes.drafts = lambda: self.drafts
         self.mailboxes.trash = lambda: mock()
         self.mailboxes.sent = lambda: mock()
 
         self.mail_sender = mock()
         self.search_engine = mock()
-        self.mail_service = MailService(self.mailboxes, self.mail_sender, self.tag_service, self.querier, self.search_engine)
+        self.mail_service = MailService(self.mailboxes, self.mail_sender, self.querier, self.search_engine)
 
     def tearDown(self):
         unstub()
diff --git a/service/test/unit/adapter/test_mailbox.py b/service/test/unit/adapter/test_mailbox.py
index b44f507b..5644969f 100644
--- a/service/test/unit/adapter/test_mailbox.py
+++ b/service/test/unit/adapter/test_mailbox.py
@@ -23,7 +23,6 @@ from test.support import test_helper
 
 class PixelatedMailboxTest(unittest.TestCase):
     def setUp(self):
-        self.tag_service = mock()
         self.querier = mock()
         self.search_engine = mock()
         self.mailbox = Mailbox('INBOX', self.querier, self.search_engine)