Merge pull request #156 from uktrade/LTD-2533-oauth

Reverts back to previous API for user on MailServer

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:

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."""

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, "[email protected]")
+
+
+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()

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,15 +127,15 @@ 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
 
         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)

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

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,