From 65c2b6d4bcb48517ad7a2e55adf1b94bcc1129fa Mon Sep 17 00:00:00 2001 From: mfrankie Date: Thu, 19 May 2016 13:29:49 +0200 Subject: issue #685 remove duplicated email recipients --- service/pixelated/adapter/services/mail_sender.py | 20 ------- service/pixelated/adapter/services/mail_service.py | 22 +++++++ service/test/support/test_helper.py | 6 +- .../test/unit/adapter/services/test_mail_sender.py | 40 +------------ service/test/unit/adapter/test_mail_service.py | 68 ++++++++++++++++++---- 5 files changed, 83 insertions(+), 73 deletions(-) (limited to 'service') diff --git a/service/pixelated/adapter/services/mail_sender.py b/service/pixelated/adapter/services/mail_sender.py index 08e81872..5ce3f7f1 100644 --- a/service/pixelated/adapter/services/mail_sender.py +++ b/service/pixelated/adapter/services/mail_sender.py @@ -63,9 +63,6 @@ class MailSender(object): bccs = mail.bcc deferreds = [] - recipients = self._remove_canonical(mail, recipients) - self._remove_duplicates_form_cc_and_to(mail) - for recipient in recipients: self._define_bcc_field(mail, recipient, bccs) smtp_recipient = self._create_twisted_smtp_recipient(recipient) @@ -73,20 +70,6 @@ class MailSender(object): return defer.DeferredList(deferreds, fireOnOneErrback=False, consumeErrors=True) - def _remove_duplicates_form_cc_and_to(self, mail): - mail.headers['To'] = list(set(self._remove_duplicates(mail.to)).difference(set(mail.bcc))) - mail.headers['Cc'] = list((set(self._remove_duplicates(mail.cc)).difference(set(mail.bcc)).difference(set(mail.to)))) - - def _remove_canonical(self, mail, recipients): - mail.headers['To'] = self._remove_duplicates(map(self._remove_canonical_recipient, mail.to)) - mail.headers['Cc'] = self._remove_duplicates(map(self._remove_canonical_recipient, mail.cc)) - mail.headers['Bcc'] = self._remove_duplicates(map(self._remove_canonical_recipient, mail.bcc)) - - return self._remove_duplicates(map(self._remove_canonical_recipient, recipients)) - - def _remove_duplicates(self, recipient): - return list(set(recipient)) - def _define_bcc_field(self, mail, recipient, bccs): if recipient in bccs: mail.headers['Bcc'] = [recipient] @@ -110,6 +93,3 @@ class MailSender(object): def _create_twisted_smtp_recipient(self, recipient): # TODO: Better is fix Twisted instead return User(str(recipient), NOT_NEEDED, NOT_NEEDED, NOT_NEEDED) - - def _remove_canonical_recipient(self, recipient): - return recipient.split('<')[1][0:-1] if '<' in recipient else recipient diff --git a/service/pixelated/adapter/services/mail_service.py b/service/pixelated/adapter/services/mail_service.py index a5ed0897..1dce51fe 100644 --- a/service/pixelated/adapter/services/mail_service.py +++ b/service/pixelated/adapter/services/mail_service.py @@ -98,11 +98,33 @@ class MailService(object): def send_mail(self, content_dict): mail = InputMail.from_dict(content_dict, self.account_email) draft_id = content_dict.get('ident') + self._deduplicate_recipients(mail) yield self.mail_sender.sendmail(mail) sent_mail = yield self.move_to_sent(draft_id, mail) defer.returnValue(sent_mail) + def _deduplicate_recipients(self, mail): + self._remove_canonical(mail) + self._remove_duplicates_form_cc_and_to(mail) + + def _remove_canonical(self, mail): + mail.headers['To'] = map(self._remove_canonical_recipient, mail.to) + mail.headers['Cc'] = map(self._remove_canonical_recipient, mail.cc) + mail.headers['Bcc'] = map(self._remove_canonical_recipient, mail.bcc) + + def _remove_duplicates_form_cc_and_to(self, mail): + mail.headers['To'] = list(set(self._remove_duplicates(mail.to)).difference(set(mail.bcc))) + mail.headers['Cc'] = list((set(self._remove_duplicates(mail.cc)).difference(set(mail.bcc)).difference(set(mail.to)))) + mail.headers['Bcc'] = self._remove_duplicates(mail.bcc) + + def _remove_duplicates(self, recipient): + return list(set(recipient)) + + # TODO removing canocical should, be added back later + def _remove_canonical_recipient(self, recipient): + return recipient.split('<')[1][0:-1] if '<' in recipient else recipient + @defer.inlineCallbacks def move_to_sent(self, last_draft_ident, mail): if last_draft_ident: diff --git a/service/test/support/test_helper.py b/service/test/support/test_helper.py index 57f41a6e..b78da4cd 100644 --- a/service/test/support/test_helper.py +++ b/service/test/support/test_helper.py @@ -48,9 +48,9 @@ def mail_dict(): def duplicates_in_fields_mail_dict(): return { 'header': { - 'to': ['to@pixelated.org', 'another@pixelated.org', 'third@pixelated.org'], - 'cc': ['cc@pixelated.org', 'another@pixelated.org', 'third@pixelated.org'], - 'bcc': ['bcc@pixelated.org', 'another@pixelated.org'], + 'to': ['to@pixelated.org', 'another@pixelated.org', 'third@pixelated.org', 'third@pixelated.org'], + 'cc': ['cc@pixelated.org', 'another@pixelated.org', 'third@pixelated.org', 'cc@pixelated.org'], + 'bcc': ['bcc@pixelated.org', 'another@pixelated.org', 'bcc@pixelated.org'], 'subject': 'Subject' }, 'body': 'Body', 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 ' - - 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 ' + + non_canonical = self.mail_service._remove_canonical_recipient(recipient) + + self.assertEqual(u'user@pixelated.org', non_canonical) -- cgit v1.2.3