diff --git a/mail/libraries/routing_controller.py b/mail/libraries/routing_controller.py index a727b8da..2f1189ce 100644 --- a/mail/libraries/routing_controller.py +++ b/mail/libraries/routing_controller.py @@ -94,7 +94,7 @@ def get_spire_standin_mailserver() -> MailServer: def check_and_route_emails(): logger.info("Checking for emails") hmrc_to_dit_server = get_hmrc_to_dit_mailserver() - email_message_dtos = _get_email_message_dtos(hmrc_to_dit_server, number=None) + email_message_dtos = get_email_message_dtos(hmrc_to_dit_server, number=None) email_message_dtos = sort_dtos_by_date(email_message_dtos) logger.info("Incoming message dtos sorted by date: %s", email_message_dtos) @@ -103,7 +103,7 @@ def check_and_route_emails(): # if the config for the return path is different to outgoing mail path # then check the return path otherwise don't bother as it will contain the # same emails. - reply_message_dtos = _get_email_message_dtos(spire_to_dit_server) + reply_message_dtos = get_email_message_dtos(spire_to_dit_server) reply_message_dtos = sort_dtos_by_date(reply_message_dtos) logger.info("Reply message dtos sorted by date: %s", reply_message_dtos) @@ -121,8 +121,8 @@ def check_and_route_emails(): logger.info( "No new emails found from %s or %s", - hmrc_to_dit_server.auth.user, - spire_to_dit_server.auth.user, + hmrc_to_dit_server.user, + spire_to_dit_server.user, ) publish_queue_status() @@ -218,7 +218,7 @@ def _collect_and_send(mail: Mail): send_licence_data_to_hmrc(schedule=0) # noqa -def _get_email_message_dtos(server: MailServer, number: Optional[int] = 3) -> List[Tuple[EmailMessageDto, Callable]]: +def get_email_message_dtos(server: MailServer, number: Optional[int] = 3) -> List[Tuple[EmailMessageDto, Callable]]: pop3_connection = server.connect_to_pop3() emails_iter = get_message_iterator(pop3_connection, server.user) if number: diff --git a/mail/servers.py b/mail/servers.py index 7e9c9ccf..a87ab568 100644 --- a/mail/servers.py +++ b/mail/servers.py @@ -20,7 +20,6 @@ def __init__( self.pop3_connection = None def __eq__(self, other): - if not isinstance(other, MailServer): return False @@ -36,6 +35,10 @@ def connect_to_pop3(self) -> poplib.POP3_SSL: def quit_pop3_connection(self): self.pop3_connection.quit() + @property + def user(self): + return self.auth.user + def get_smtp_connection(): """Connect to an SMTP server, specified by environment variables.""" diff --git a/mail/tests/test_dtos.py b/mail/tests/test_dtos.py index 84110c43..460fd871 100644 --- a/mail/tests/test_dtos.py +++ b/mail/tests/test_dtos.py @@ -1,9 +1,13 @@ +from unittest.mock import Mock, patch + from dateutil.parser import parse from django.test import SimpleTestCase from parameterized import parameterized from mail.libraries.email_message_dto import * from mail.libraries.helpers import read_file, sort_dtos_by_date, to_mail_message_dto +from mail.libraries.routing_controller import get_email_message_dtos +from mail.servers import MailServer from mail.tests.libraries.client import LiteHMRCTestClient @@ -80,3 +84,21 @@ def test_multi_licence_message_from_spire(self): mail_message = to_mail_message_dto(self.batched_licence_data_email) self.assertEqual(mail_message.run_number, 96838) self.assertEqual(mail_message.receiver, "test-test-gateway@test.trade.gov.uk") + + +class TestGetEmailMessagesDTOs(SimpleTestCase): + @patch("mail.libraries.routing_controller.get_message_iterator") + def test_get_email_message_dtos(self, mock_get_message_iterator): + mock_mail_server = Mock(spec=MailServer) + mock_emails = [Mock(), Mock()] + mock_get_message_iterator.return_value = mock_emails + + emails = get_email_message_dtos(mock_mail_server) + + self.assertEqual(emails, mock_emails) + mock_mail_server.connect_to_pop3.assert_called_once_with() + mock_get_message_iterator.assert_called_once_with( + mock_mail_server.connect_to_pop3(), + mock_mail_server.user, + ) + mock_mail_server.quit_pop3_connection.assert_called_once_with() diff --git a/mail/tests/test_mail_service.py b/mail/tests/test_mail_service.py index ab871def..0cff558e 100644 --- a/mail/tests/test_mail_service.py +++ b/mail/tests/test_mail_service.py @@ -5,7 +5,8 @@ from django.test import SimpleTestCase from parameterized import parameterized -from mail.libraries.mailbox_service import get_message_iterator, read_last_message, read_last_three_emails +from mail.auth import Authenticator +from mail.libraries.mailbox_service import read_last_message, read_last_three_emails from mail.servers import MailServer from mail.tests.libraries.client import LiteHMRCTestClient @@ -126,7 +127,7 @@ def test_read_last_three_emails(self, email_list, retr_data): class MailServerTests(SimpleTestCase): def test_mail_server_equal(self): - auth = Mock() + auth = Mock(spec=Authenticator) m1 = MailServer(auth, hostname="host", pop3_port=1) # nosec m2 = MailServer(auth, hostname="host", pop3_port=1) # nosec @@ -134,7 +135,7 @@ def test_mail_server_equal(self): self.assertEqual(m1, m2) def test_mail_server_not_equal(self): - auth = Mock() + auth = Mock(spec=Authenticator) m1 = MailServer(auth, hostname="host", pop3_port=1) # nosec m2 = MailServer(auth, hostname="host", pop3_port=2) # nosec @@ -145,7 +146,7 @@ def test_mail_server_connect_to_pop3(self): hostname = "host" pop3_port = 1 - auth = Mock() + auth = Mock(spec=Authenticator) pop3conn = MagicMock(spec=POP3_SSL) with patch("mail.servers.poplib") as mock_poplib: @@ -166,3 +167,13 @@ def test_mail_server_connect_to_pop3(self): mock_connection = pop3conn() auth.authenticate.assert_called_with(mock_connection) + + def test_mail_server_user(self): + auth = Mock(spec=Authenticator) + auth.user = Mock() + mail_server = MailServer( + auth, + hostname="host", + pop3_port=1, + ) + self.assertEqual(mail_server.user, auth.user) diff --git a/mail/tests/test_resend_email.py b/mail/tests/test_resend_email.py index 16791301..dbb88b32 100644 --- a/mail/tests/test_resend_email.py +++ b/mail/tests/test_resend_email.py @@ -13,7 +13,7 @@ class LITEHMRCResendEmailTests(LiteHMRCTestClient): @mock.patch("mail.libraries.routing_controller.send") - @mock.patch("mail.libraries.routing_controller._get_email_message_dtos") + @mock.patch("mail.libraries.routing_controller.get_email_message_dtos") def test_resend_licence_data_mail_to_hmrc(self, email_dtos, send_mail): """ Tests resending of licence data mail to HMRC @@ -57,7 +57,7 @@ def test_resend_licence_data_mail_to_hmrc(self, email_dtos, send_mail): self.assertEqual(send_mail.call_count, 2) @mock.patch("mail.libraries.routing_controller.send") - @mock.patch("mail.libraries.routing_controller._get_email_message_dtos") + @mock.patch("mail.libraries.routing_controller.get_email_message_dtos") def test_resend_licence_reply_mail_to_spire(self, email_dtos, send_mail): source_run_number = 49530 hmrc_run_number = 49543 @@ -116,7 +116,7 @@ def test_resend_licence_reply_mail_to_spire(self, email_dtos, send_mail): send_mail.assert_called_once() @mock.patch("mail.libraries.routing_controller.send") - @mock.patch("mail.libraries.routing_controller._get_email_message_dtos") + @mock.patch("mail.libraries.routing_controller.get_email_message_dtos") def test_resend_usage_data_mail_to_spire(self, email_dtos, send_mail): source_run_number = 49530 hmrc_run_number = 49543 diff --git a/mail/tests/test_select_email_for_sending.py b/mail/tests/test_select_email_for_sending.py index 4eaeaad8..65cafd73 100644 --- a/mail/tests/test_select_email_for_sending.py +++ b/mail/tests/test_select_email_for_sending.py @@ -103,7 +103,7 @@ def test_email_selected_if_no_spire_data(self): self.assertEqual(mail, mail_1) @mock.patch("mail.libraries.routing_controller.send") - @mock.patch("mail.libraries.routing_controller._get_email_message_dtos") + @mock.patch("mail.libraries.routing_controller.get_email_message_dtos") def test_case1_sending_of_pending_licencedata_mails(self, email_dtos, send_mail): """ Ensure pending mails are sent and status updated as expected. @@ -155,7 +155,7 @@ def test_case1_sending_of_pending_licencedata_mails(self, email_dtos, send_mail) self.assertEqual(mail.status, ReceptionStatusEnum.REPLY_PENDING) @mock.patch("mail.libraries.routing_controller.send") - @mock.patch("mail.libraries.routing_controller._get_email_message_dtos") + @mock.patch("mail.libraries.routing_controller.get_email_message_dtos") def test_case2_sending_of_pending_usagedata_mails(self, email_dtos, send_mail): """ Case2: When only usageData mails are pending. Multiple mails are possible if none @@ -212,7 +212,7 @@ def test_case2_sending_of_pending_usagedata_mails(self, email_dtos, send_mail): self.assertEqual(mail.status, ReceptionStatusEnum.REPLY_SENT) @mock.patch("mail.libraries.routing_controller.send") - @mock.patch("mail.libraries.routing_controller._get_email_message_dtos") + @mock.patch("mail.libraries.routing_controller.get_email_message_dtos") def test_case3_sending_of_pending_licencedata_and_usagedata_mails_1(self, email_dtos, send_mail): """ Case3.1: When both licenceData and usageData mails are pending. This is possible if @@ -294,7 +294,7 @@ def test_case3_sending_of_pending_licencedata_and_usagedata_mails_1(self, email_ self.assertEqual(mail.status, ReceptionStatusEnum.REPLY_SENT) @mock.patch("mail.libraries.routing_controller.send") - @mock.patch("mail.libraries.routing_controller._get_email_message_dtos") + @mock.patch("mail.libraries.routing_controller.get_email_message_dtos") def test_case3_sending_of_pending_licencedata_and_usagedata_mails_2(self, email_dtos, send_mail): """ Another variation of case3 is, @@ -362,7 +362,7 @@ def test_case3_sending_of_pending_licencedata_and_usagedata_mails_2(self, email_ self.assertEqual(mail.status, ReceptionStatusEnum.REPLY_PENDING) @mock.patch("mail.libraries.routing_controller.send") - @mock.patch("mail.libraries.routing_controller._get_email_message_dtos") + @mock.patch("mail.libraries.routing_controller.get_email_message_dtos") def test_case4_sending_of_pending_licencedata_when_waiting_for_reply(self, email_dtos, send_mail): """ Another variation of case3 is,