issue #685 remove duplicated email recipients

2 files changed, 58 insertions, 50 deletions

diff --git a/service/test/unit/adapter/services/test_mail_sender.py b/service/test/unit/adapter/services/test_mail_sender.py
index 77435b66..0f02f759 100644
--- a/service/test/unit/adapter/services/test_mail_sender.py
+++ b/service/test/unit/adapter/services/test_mail_sender.py
@@ -22,7 +22,7 @@ from pixelated.adapter.services.mail_sender import MailSender, MailSenderExcepti
 from pixelated.adapter.model.mail import InputMail
 from pixelated.bitmask_libraries.smtp import LeapSMTPConfig
 from pixelated.support.functional import flatten
-from test.support.test_helper import mail_dict, duplicates_in_fields_mail_dict
+from test.support.test_helper import mail_dict
 from twisted.internet import reactor, defer
 from twisted.internet.defer import Deferred
 from mockito.matchers import Matcher
@@ -100,41 +100,3 @@ class MailSenderTest(unittest.TestCase):
 
         for recipient in flatten([input_mail.to, input_mail.cc, input_mail.bcc]):
             verify(OutgoingMail).send_message(MailToSmtpFormatCapture(recipient, bccs), TwistedSmtpUserCapture(recipient))
-
-
-    @defer.inlineCallbacks
-    def test_if_recipent_doubled_in_fields_send_only_in_bcc(self):
-        input_mail = InputMail.from_dict(duplicates_in_fields_mail_dict(), from_address='pixelated@org')
-        when(OutgoingMail).send_message(any(), any()).thenReturn(defer.succeed(None))
-
-        yield self.sender.sendmail(input_mail)
-
-        self.assertIn('to@pixelated.org', input_mail.to)
-        self.assertNotIn('another@pixelated.org', input_mail.to)
-        #self.assertIn('another@pixelated.org', input_mail.bcc)
-
-    @defer.inlineCallbacks
-    def test_if_recipent_doubled_in_fields_send_only_in_to(self):
-        input_mail = InputMail.from_dict(duplicates_in_fields_mail_dict(), from_address='pixelated@org')
-        when(OutgoingMail).send_message(any(), any()).thenReturn(defer.succeed(None))
-
-        yield self.sender.sendmail(input_mail)
-
-        self.assertIn('third@pixelated.org', input_mail.to)
-        self.assertNotIn('third@pixelated.org', input_mail.cc)
-        self.assertIn('cc@pixelated.org', input_mail.cc)
-        self.assertNotIn(['third@pixelated.org', 'another@pixelated.org'], input_mail.cc)
-
-    def test_remove_canonical_recipient_when_it_is_not_canonical(self):
-        recipient = u'user@pixelated.org'
-
-        non_canonical = self.sender._remove_canonical_recipient(recipient)
-
-        self.assertEqual(recipient, non_canonical)
-
-    def test_remove_canonical_recipient_when_it_is_canonical(self):
-        recipient = u'User <user@pixelated.org>'
-
-        non_canonical = self.sender._remove_canonical_recipient(recipient)
-
-        self.assertEqual(u'user@pixelated.org', non_canonical)
diff --git a/service/test/unit/adapter/test_mail_service.py b/service/test/unit/adapter/test_mail_service.py
index a8b6d597..fc1da236 100644
--- a/service/test/unit/adapter/test_mail_service.py
+++ b/service/test/unit/adapter/test_mail_service.py
@@ -19,7 +19,7 @@ from pixelated.adapter.model.mail import InputMail
 from pixelated.adapter.model.status import Status
 
 from pixelated.adapter.services.mail_service import MailService
-from test.support.test_helper import mail_dict, leap_mail
+from test.support.test_helper import mail_dict, leap_mail, duplicates_in_fields_mail_dict
 from mockito import mock, unstub, when, verify, verifyNoMoreInteractions, any as ANY, never
 from twisted.internet import defer
 
@@ -39,18 +39,18 @@ class TestMailService(unittest.TestCase):
         self.mail_sender = mock()
         self.search_engine = mock()
         self.mail_service = MailService(self.mail_sender, self.mail_store, self.search_engine, 'acount@email', self.attachment_store)
+        self.mail = InputMail.from_dict(duplicates_in_fields_mail_dict(), from_address='pixelated@org')
 
     def tearDown(self):
         unstub()
 
     def test_send_mail(self):
-        input_mail = InputMail()
-        when(InputMail).from_dict(ANY(), ANY()).thenReturn(input_mail)
+        when(InputMail).from_dict(ANY(), ANY()).thenReturn(self.mail)
         when(self.mail_sender).sendmail(ANY()).thenReturn(defer.Deferred())
 
         sent_deferred = self.mail_service.send_mail(mail_dict())
 
-        verify(self.mail_sender).sendmail(input_mail)
+        verify(self.mail_sender).sendmail(self.mail)
 
         sent_deferred.callback('Assume sending mail succeeded')
 
@@ -60,6 +60,9 @@ class TestMailService(unittest.TestCase):
     def test_send_mail_removes_draft(self):
         mail = LeapMail('id', 'INBOX')
         when(mail).raw = 'raw mail'
+        mail._headers['To'] = []
+        mail._headers['Cc'] = []
+        mail._headers['Bcc'] = []
         when(InputMail).from_dict(ANY(), ANY()).thenReturn(mail)
         when(self.mail_store).delete_mail('12').thenReturn(defer.succeed(None))
         when(self.mail_store).add_mail('SENT', ANY()).thenReturn(mail)
@@ -75,11 +78,10 @@ class TestMailService(unittest.TestCase):
 
     @defer.inlineCallbacks
     def test_send_mail_marks_as_read(self):
-        mail = InputMail()
-        when(mail).raw = 'raw mail'
-        when(InputMail).from_dict(ANY(), ANY()).thenReturn(mail)
+        when(self.mail).raw = 'raw mail'
+        when(InputMail).from_dict(ANY(), ANY()).thenReturn(self.mail)
         when(self.mail_store).delete_mail('12').thenReturn(defer.succeed(None))
-        when(self.mail_sender).sendmail(mail).thenReturn(defer.succeed(None))
+        when(self.mail_sender).sendmail(self.mail).thenReturn(defer.succeed(None))
 
         sent_mail = LeapMail('id', 'INBOX')
         add_mail_deferral = defer.succeed(sent_mail)
@@ -92,8 +94,7 @@ class TestMailService(unittest.TestCase):
 
     @defer.inlineCallbacks
     def test_send_mail_does_not_delete_draft_on_error(self):
-        input_mail = InputMail()
-        when(InputMail).from_dict(ANY(), ANY()).thenReturn(input_mail)
+        when(InputMail).from_dict(ANY(), ANY()).thenReturn(self.mail)
 
         deferred_failure = defer.fail(Exception("Assume sending mail failed"))
         when(self.mail_sender).sendmail(ANY()).thenReturn(deferred_failure)
@@ -102,7 +103,7 @@ class TestMailService(unittest.TestCase):
             yield self.mail_service.send_mail({'ident': '12'})
             self.fail("send_mail is expected to raise if underlying call fails")
         except:
-            verify(self.mail_sender).sendmail(input_mail)
+            verify(self.mail_sender).sendmail(self.mail)
             verifyNoMoreInteractions(self.drafts)
 
     @defer.inlineCallbacks
@@ -175,3 +176,48 @@ class TestMailService(unittest.TestCase):
 
         verify(self.mail_store).update_mail(mail)
         self.assertEqual({'custom_1', 'custom_3'}, updated_mail.tags)
+
+    @defer.inlineCallbacks
+    def test_if_recipient_doubled_in_fields_send_only_in_bcc(self):
+        mail = InputMail.from_dict(duplicates_in_fields_mail_dict(), from_address='pixelated@org')
+
+        yield self.mail_service._deduplicate_recipients(mail)
+
+        self.assertIn('to@pixelated.org', mail.to)
+        self.assertNotIn('another@pixelated.org', mail.to)
+        self.assertIn('another@pixelated.org', mail.bcc)
+
+    @defer.inlineCallbacks
+    def test_if_recipient_doubled_in_fields_send_only_in_to(self):
+        mail = InputMail.from_dict(duplicates_in_fields_mail_dict(), from_address='pixelated@org')
+
+        yield self.mail_service._deduplicate_recipients(mail)
+
+        self.assertIn('third@pixelated.org', mail.to)
+        self.assertNotIn('third@pixelated.org', mail.cc)
+        self.assertIn('cc@pixelated.org', mail.cc)
+        self.assertNotIn('another@pixelated.org', mail.cc)
+
+    @defer.inlineCallbacks
+    def test_if_deduplicates_when_recipient_repeated_in_field(self):
+        mail = InputMail.from_dict(duplicates_in_fields_mail_dict(), from_address='pixelated@org')
+
+        yield self.mail_service._deduplicate_recipients(mail)
+
+        self.assertItemsEqual(['bcc@pixelated.org', 'another@pixelated.org'], mail.bcc)
+        self.assertItemsEqual(['third@pixelated.org', 'to@pixelated.org'], mail.to)
+        self.assertItemsEqual(['cc@pixelated.org'], mail.cc)
+
+    def test_remove_canonical_recipient_when_it_is_not_canonical(self):
+        recipient = u'user@pixelated.org'
+
+        non_canonical = self.mail_service._remove_canonical_recipient(recipient)
+
+        self.assertEqual(recipient, non_canonical)
+
+    def test_remove_canonical_recipient_when_it_is_canonical(self):
+        recipient = u'User <user@pixelated.org>'
+
+        non_canonical = self.mail_service._remove_canonical_recipient(recipient)
+
+        self.assertEqual(u'user@pixelated.org', non_canonical)