Revert and update

mail/celery_tasks.py

@@ -1,6 +1,11 @@
 import urllib.parse
+from email.mime.multipart import MIMEMultipart
+from email.mime.text import MIMEText
 from smtplib import SMTPException
 from typing import List, MutableMapping, Tuple
+import time
+from contextlib import contextmanager
+from django.core.cache import cache
 
 from celery import Task, shared_task
 from celery.utils.log import get_task_logger
@@ -16,10 +21,10 @@
 from mail.libraries.routing_controller import check_and_route_emails, send, update_mail
 from mail.libraries.usage_data_decomposition import build_json_payload_from_data_blocks, split_edi_data_by_id
 from mail.models import LicenceIdMapping, LicencePayload, Mail, UsageData
+from mail.servers import smtp_send
 
 logger = get_task_logger(__name__)
 
-
 # Send Usage Figures to LITE API
 def get_lite_api_url():
     """The URL for the licence usage callback, from the LITE_API_URL setting.
@@ -73,10 +78,43 @@ def _log_error(message, lite_usage_data_id):
 
 MAX_ATTEMPTS = 3
 RETRY_BACKOFF = 180
+LOCK_EXPIRE = 60 * 10  # Lock expires in 10 minutes
 CELERY_SEND_LICENCE_UPDATES_TASK_NAME = "mail.celery_tasks.send_licence_details_to_hmrc"
 CELERY_MANAGE_INBOX_TASK_NAME = "mail.celery_tasks.manage_inbox"
 
 
+# Notify Users of Rejected Mail
+@shared_task(
+    autoretry_for=(SMTPException,),
+    max_retries=MAX_ATTEMPTS,
+    retry_backoff=RETRY_BACKOFF,
+)
+def notify_users_of_rejected_licences(mail_id, mail_response_subject):
+    """If a reply is received with rejected licences this task notifies users of the rejection"""
+
+    logger.info("Notifying users of rejected licences found in mail with subject %s", mail_response_subject)
+
+    try:
+        multipart_msg = MIMEMultipart()
+        multipart_msg["From"] = settings.EMAIL_USER
+        multipart_msg["To"] = ",".join(settings.NOTIFY_USERS)
+        multipart_msg["Subject"] = "Licence rejected by HMRC"
+        body = MIMEText(f"Mail (Id: {mail_id}) with subject {mail_response_subject} has rejected licences")
+        multipart_msg.attach(body)
+
+        send_smtp_task(multipart_msg)
+
+    except SMTPException:
+        logger.exception(
+            "An unexpected error occurred when notifying users of rejected licences, Mail Id: %s, subject: %s",
+            mail_id,
+            mail_response_subject,
+        )
+        raise
+
+    logger.info("Successfully notified users of rejected licences found in mail with subject %s", mail_response_subject)
+
+
 class SendUsageDataBaseTask(Task):
     def on_failure(self, exc, task_id, args, kwargs, einfo):
         message = (
@@ -239,3 +277,35 @@ def manage_inbox():
             exc_info=True,
         )
         raise exc
+
+
+
+@contextmanager
+def memcache_lock(lock_id, oid=None):
+    timeout_at = time.monotonic() + LOCK_EXPIRE - 3
+    status = cache.add(lock_id, "locked", LOCK_EXPIRE)
+    try:
+        yield status
+    finally:
+        if time.monotonic() < timeout_at and status:
+            cache.delete(lock_id)
+
+
+@shared_task(bind=True, autoretry_for=(SMTPException,), max_retries=MAX_ATTEMPTS, retry_backoff=RETRY_BACKOFF)
+def send_smtp_task(self, multipart_msg):
+    global_lock_id = "global_send_email_lock"
+
+    with memcache_lock(global_lock_id) as acquired:
+        if acquired:
+            logger.info("Global lock acquired, sending email")
+            try:
+                smtp_send(multipart_msg)
+                logger.info("Successfully sent email.")
+            except SMTPException as e:
+                logger.error(f"Failed to send email: {e}")
+                raise
+        else:
+            logger.info("Another send_smtp_task is currently in progress, will retry...")
+
+            retry_delay = RETRY_BACKOFF * (2**self.request.retries)
+            raise self.retry(countdown=retry_delay)

mail/libraries/builders.py

@@ -280,14 +280,3 @@ def _validate_dto(email_message_dto):
 
     if email_message_dto.attachment is None:
         raise TypeError("None file attachment received!")
-
-
-def build_email_rejected_licence_message(mail_id, mail_response_subject):
-    multipart_msg = MIMEMultipart()
-    multipart_msg["From"] = settings.EMAIL_USER
-    multipart_msg["To"] = ",".join(settings.NOTIFY_USERS)
-    multipart_msg["Subject"] = "Licence rejected by HMRC"
-    body = MIMEText(f"Mail (Id: {mail_id}) with subject {mail_response_subject} has rejected licences")
-    multipart_msg.attach(body)
-
-    return multipart_msg

mail/libraries/routing_controller.py

@@ -4,7 +4,7 @@
 
 from django.conf import settings
 from django.utils import timezone
-from mail.tasks import send_email_task
+
 from rest_framework.exceptions import ValidationError
 
 from mail.auth import BasicAuthentication, ModernAuthentication
@@ -170,9 +170,13 @@ def update_mail(mail: Mail, mail_dto: EmailMessageDto):
 
 
 def send(email_message_dto: EmailMessageDto):
+    # Needs to be imported here otherwise you will get a circular import errors
+    from mail.celery_tasks import send_smtp_task
+
     logger.info("Preparing to send email")
     message = build_email_message(email_message_dto)
-    smtp_send(message)
+
+    send_smtp_task.apply_async(args=[message])
 
 
 def _collect_and_send(mail: Mail):
@@ -186,7 +190,7 @@ def _collect_and_send(mail: Mail):
 
     if message_to_send_dto:
         if message_to_send_dto.receiver != SourceEnum.LITE and message_to_send_dto.subject:
-            send_email_task(mail_id=mail.id, message=message_to_send_dto)
+            send(message_to_send_dto)
             update_mail(mail, message_to_send_dto)
 
             logger.info(

mail/models.py

@@ -94,9 +94,9 @@ def set_response_date_time(self, offset: int = 0):
 
     @staticmethod
     def notify_users(id, response_subject):
-        from mail.tasks import send_email_task
+        from mail.celery_tasks import notify_users_of_rejected_licences
 
-        send_email_task.delay(mail_id=id, mail_response_subject=response_subject)
+        notify_users_of_rejected_licences.delay(str(id), response_subject)
 
 
 class LicenceData(models.Model):

mail/tests/test_celery_tasks.py

@@ -0,0 +1,153 @@
+import email.mime.multipart
+from unittest import mock
+
+import pytest
+from django.test import TestCase, override_settings
+from django.core.cache import cache
+from celery.exceptions import Retry
+
+from django.conf import settings
+from mail.celery_tasks import manage_inbox, notify_users_of_rejected_licences
+import email.mime.multipart
+from mail.libraries.email_message_dto import EmailMessageDto
+from mail.celery_tasks import send_smtp_task
+from mail.celery_tasks import get_lite_api_url
+
+
+class NotifyUsersOfRejectedMailTests(TestCase):
+    @override_settings(EMAIL_USER="[email protected]", NOTIFY_USERS=["[email protected]"])  # /PS-IGNORE
+    @mock.patch("mail.celery_tasks.send_smtp_task")
+    def test_send_success(self, mock_send):
+        notify_users_of_rejected_licences("123", "CHIEF_SPIRE_licenceReply_202401180900_42557")
+        mock_send.assert_called_once()
+
+        self.assertEqual(len(mock_send.call_args_list), 1)
+        message = mock_send.call_args[0][0]
+        self.assertIsInstance(message, email.mime.multipart.MIMEMultipart)
+
+        expected_headers = {
+            "Content-Type": "multipart/mixed",
+            "MIME-Version": "1.0",
+            "From": "[email protected]",  # /PS-IGNORE
+            "To": "[email protected]",  # /PS-IGNORE
+            "Subject": "Licence rejected by HMRC",
+        }
+        self.assertDictEqual(dict(message), expected_headers)
+
+        text_payload = message.get_payload(0)
+        expected_body = "Mail (Id: 123) with subject CHIEF_SPIRE_licenceReply_202401180900_42557 has rejected licences"
+        self.assertEqual(text_payload.get_payload(), expected_body)
+
+
+class ManageInboxTests(TestCase):
+    @mock.patch("mail.celery_tasks.check_and_route_emails")
+    def test_manage_inbox(self, mock_function):
+        manage_inbox()
+        mock_function.assert_called_once()
+
+    @mock.patch("mail.celery_tasks.check_and_route_emails")
+    def test_error_manage_inbox(self, mock_function):
+        mock_function.side_effect = Exception("Test Error")
+        with pytest.raises(Exception) as excinfo:
+            manage_inbox()
+        assert str(excinfo.value) == "Test Error"
+
+
+class GetLiteAPIUrlTests(TestCase):
+    def test_get_url_with_no_path(self):
+        with self.settings(LITE_API_URL="https://example.com"):
+            result = get_lite_api_url()
+
+        self.assertEqual(result, "https://example.com/licences/hmrc-integration/")
+
+    def test_get_url_with_root_path(self):
+        with self.settings(LITE_API_URL="https://example.com/"):
+            result = get_lite_api_url()
+
+        self.assertEqual(result, "https://example.com/licences/hmrc-integration/")
+
+    def test_get_url_with_path_from_setting(self):
+        with self.settings(LITE_API_URL="https://example.com/foo"):
+            result = get_lite_api_url()
+
+        self.assertEqual(result, "https://example.com/foo")
+
+
+class SendEmailTaskTests(TestCase):
+    def setUp(self):
+        attachment = "30 \U0001d5c4\U0001d5c6/\U0001d5c1 \u5317\u4EB0"
+        self.email_message_dto = EmailMessageDto(
+            run_number=1,
+            sender=settings.HMRC_ADDRESS,
+            receiver=settings.SPIRE_ADDRESS,
+            date="Mon, 17 May 2021 14:20:18 +0100",
+            body=None,
+            subject="Some subject",
+            attachment=["some filename", attachment],
+            raw_data="",
+        )
+
+    @mock.patch("mail.celery_tasks.smtp_send")
+    @mock.patch("mail.celery_tasks.cache")
+    def test_locking_prevents_multiple_executions(self, mock_cache, mock_smtp_send):
+        mock_cache.add.side_effect = [True, False]  # First call acquires the lock, second call finds it locked
+
+        # Simulate the lock being released after the first task finishes
+        mock_cache.delete.return_value = None
+
+        try:
+            send_smtp_task(self.email_message_dto)
+        except Retry:
+            self.fail("First task execution should not raise Retry.")
+
+        with self.assertRaises(Retry):
+            send_smtp_task(self.email_message_dto)
+
+        # Assert smtp_send was called once due to locking
+        mock_smtp_send.assert_called_once()
+        # After locked and being released
+        self.assertEqual(mock_cache.add.call_count, 2)
+        mock_cache.delete.assert_called_once_with("global_send_email_lock")
+
+    @mock.patch("mail.celery_tasks.send_smtp_task.retry", side_effect=Retry)
+    @mock.patch("mail.celery_tasks.smtp_send")
+    @mock.patch("mail.celery_tasks.cache")
+    def test_retry_on_lock_failure(self, mock_cache, mock_smtp_send, mock_retry):
+        mock_cache.add.return_value = False
+        mock_smtp_send.return_value = None
+
+        with self.assertRaises(Retry):
+            send_smtp_task(self.email_message_dto)
+
+        mock_retry.assert_called_once()
+
+        retry_call_args = mock_retry.call_args
+        self.assertIn("countdown", retry_call_args[1])
+        retry_delay = retry_call_args[1]["countdown"]
+        self.assertEqual(retry_delay, 180)
+
+
+class NotifyUsersOfRejectedMailTests(TestCase):
+    @override_settings(EMAIL_USER="[email protected]", NOTIFY_USERS=["[email protected]"])  # /PS-IGNORE
+    @mock.patch("mail.celery_tasks.smtp_send")
+    def test_send_success(self, mock_send):
+        notify_users_of_rejected_licences("123", "CHIEF_SPIRE_licenceReply_202401180900_42557")
+
+        mock_send.assert_called_once()
+
+        self.assertEqual(len(mock_send.call_args_list), 1)
+        message = mock_send.call_args[0][0]
+        self.assertIsInstance(message, email.mime.multipart.MIMEMultipart)
+
+        expected_headers = {
+            "Content-Type": "multipart/mixed",
+            "MIME-Version": "1.0",
+            "From": "[email protected]",  # /PS-IGNORE
+            "To": "[email protected]",  # /PS-IGNORE
+            "Subject": "Licence rejected by HMRC",
+        }
+        self.assertDictEqual(dict(message), expected_headers)
+
+        text_payload = message.get_payload(0)
+        expected_body = "Mail (Id: 123) with subject CHIEF_SPIRE_licenceReply_202401180900_42557 has rejected licences"
+        self.assertEqual(text_payload.get_payload(), expected_body)