LTD-4652-send-email-task

conf/settings.py

@@ -283,3 +283,10 @@ def _build_redis_url(base_url, db_number, **query_args):
 CELERY_TASK_ALWAYS_EAGER = env.bool("CELERY_TASK_ALWAYS_EAGER", False)
 CELERY_TASK_STORE_EAGER_RESULT = env.bool("CELERY_TASK_STORE_EAGER_RESULT", False)
 CELERY_TASK_SEND_SENT_EVENT = env.bool("CELERY_TASK_SEND_SENT_EVENT", True)
+
+CACHES = {
+    "default": {
+        "BACKEND": "django.core.cache.backends.redis.RedisCache",
+        "LOCATION": REDIS_BASE_URL,
+    }
+}

mail/celery_tasks.py

@@ -1,6 +1,4 @@
 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
 
@@ -18,10 +16,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.
@@ -79,38 +77,6 @@ def _log_error(message, lite_usage_data_id):
 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)
-
-        smtp_send(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 = (

mail/libraries/builders.py

@@ -280,3 +280,14 @@ 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,6 +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
@@ -185,7 +186,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(message_to_send_dto)
+            send_email_task(mail_id=mail.id, message=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.celery_tasks import notify_users_of_rejected_licences
+        from mail.tasks import send_email_task
 
-        notify_users_of_rejected_licences.delay(str(id), response_subject)
+        send_email_task.delay(mail_id=id, mail_response_subject=response_subject)
 
 
 class LicenceData(models.Model):

mail/tasks.py

@@ -0,0 +1,55 @@
+import time
+
+from smtplib import SMTPException
+from contextlib import contextmanager
+from django.core.cache import cache
+from celery import shared_task
+from celery.utils.log import get_task_logger
+
+from mail.servers import smtp_send
+from mail.libraries.builders import build_email_rejected_licence_message, build_email_message
+
+logger = get_task_logger(__name__)
+
+MAX_ATTEMPTS = 3
+RETRY_BACKOFF = 180
+LOCK_EXPIRE = 60 * 10  # Lock expires in 10 minutes
+
+
+@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_email_task(self, **kwargs):
+    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:
+                if "message" in kwargs:
+                    multipart_msg = build_email_message(kwargs["message"])
+                elif "mail_response_subject" in kwargs and "mail_id" in kwargs:
+                    multipart_msg = build_email_rejected_licence_message(
+                        kwargs["mail_id"], kwargs["mail_response_subject"]
+                    )
+                else:
+                    raise ValueError("Insufficient parameters to build email.")
+                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_email_task is currently in progress, will retry...")
+
+            retry_delay = RETRY_BACKOFF * (2**self.request.retries)
+            raise self.retry(countdown=retry_delay)

mail/tests/test_celery_task.py

@@ -1,36 +1,9 @@
-import email.mime.multipart
 from unittest import mock
 
 import pytest
-from django.test import TestCase, override_settings
+from django.test import TestCase
 
-from mail.celery_tasks import manage_inbox, notify_users_of_rejected_licences
-
-
-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)
+from mail.celery_tasks import manage_inbox
 
 
 class ManageInboxTests(TestCase):

mail/tests/test_resend_email.py

@@ -14,14 +14,11 @@
 class LITEHMRCResendEmailTests(LiteHMRCTestClient):
     @mock.patch("mail.libraries.routing_controller.get_spire_to_dit_mailserver")
     @mock.patch("mail.libraries.routing_controller.get_hmrc_to_dit_mailserver")
-    @mock.patch("mail.libraries.routing_controller.send")
+    @mock.patch("mail.libraries.routing_controller.send_email_task")
     @mock.patch("mail.libraries.routing_controller.get_email_message_dtos")
+    @mock.patch("mail.libraries.routing_controller.send")
     def test_resend_licence_data_mail_to_hmrc(
-        self,
-        email_dtos,
-        send_mail,
-        mock_get_hmrc_to_dit_mailserver,
-        mock_get_spire_to_dit_mailserver,
+        self, email_dtos, send_mail, mock_get_hmrc_to_dit_mailserver, mock_get_spire_to_dit_mailserver, send
     ):
         """
         Tests resending of licence data mail to HMRC
@@ -51,7 +48,6 @@ def test_resend_licence_data_mail_to_hmrc(
 
         # assert that the pending mail is sent and status updated
         mail = Mail.objects.get(id=pending_mail.id)
-        send_mail.assert_called_once()
         self.assertEqual(mail.status, ReceptionStatusEnum.REPLY_PENDING)
 
         call_command("resend_email", "--hmrc_run_number", 49543)
@@ -63,10 +59,11 @@ def test_resend_licence_data_mail_to_hmrc(
         self.assertEqual(mail.status, ReceptionStatusEnum.REPLY_PENDING)
         self.assertEqual(mail.extract_type, ExtractTypeEnum.LICENCE_DATA)
         self.assertEqual(send_mail.call_count, 2)
+        self.assertEqual(send.call_count, 1)
 
     @mock.patch("mail.libraries.routing_controller.get_spire_to_dit_mailserver")
     @mock.patch("mail.libraries.routing_controller.get_hmrc_to_dit_mailserver")
-    @mock.patch("mail.libraries.routing_controller.send")
+    @mock.patch("mail.libraries.routing_controller.send_email_task")
     @mock.patch("mail.libraries.routing_controller.get_email_message_dtos")
     def test_resend_licence_reply_mail_to_spire(
         self,
@@ -133,7 +130,7 @@ def test_resend_licence_reply_mail_to_spire(
 
     @mock.patch("mail.libraries.routing_controller.get_spire_to_dit_mailserver")
     @mock.patch("mail.libraries.routing_controller.get_hmrc_to_dit_mailserver")
-    @mock.patch("mail.libraries.routing_controller.send")
+    @mock.patch("mail.libraries.routing_controller.send_email_task")
     @mock.patch("mail.libraries.routing_controller.get_email_message_dtos")
     def test_resend_usage_data_mail_to_spire(
         self,

mail/tests/test_select_email_for_sending.py

@@ -104,7 +104,7 @@ def test_email_selected_if_no_spire_data(self):
 
     @mock.patch("mail.libraries.routing_controller.get_spire_to_dit_mailserver")
     @mock.patch("mail.libraries.routing_controller.get_hmrc_to_dit_mailserver")
-    @mock.patch("mail.libraries.routing_controller.send")
+    @mock.patch("mail.libraries.routing_controller.send_email_task")
     @mock.patch("mail.libraries.routing_controller.get_email_message_dtos")
     def test_case1_sending_of_pending_licencedata_mails(
         self,
@@ -164,7 +164,7 @@ def test_case1_sending_of_pending_licencedata_mails(
 
     @mock.patch("mail.libraries.routing_controller.get_spire_to_dit_mailserver")
     @mock.patch("mail.libraries.routing_controller.get_hmrc_to_dit_mailserver")
-    @mock.patch("mail.libraries.routing_controller.send")
+    @mock.patch("mail.libraries.routing_controller.send_email_task")
     @mock.patch("mail.libraries.routing_controller.get_email_message_dtos")
     def test_case2_sending_of_pending_usagedata_mails(
         self,
@@ -229,7 +229,7 @@ def test_case2_sending_of_pending_usagedata_mails(
 
     @mock.patch("mail.libraries.routing_controller.get_spire_to_dit_mailserver")
     @mock.patch("mail.libraries.routing_controller.get_hmrc_to_dit_mailserver")
-    @mock.patch("mail.libraries.routing_controller.send")
+    @mock.patch("mail.libraries.routing_controller.send_email_task")
     @mock.patch("mail.libraries.routing_controller.get_email_message_dtos")
     def test_case3_sending_of_pending_licencedata_and_usagedata_mails_1(
         self,
@@ -319,7 +319,7 @@ def test_case3_sending_of_pending_licencedata_and_usagedata_mails_1(
 
     @mock.patch("mail.libraries.routing_controller.get_spire_to_dit_mailserver")
     @mock.patch("mail.libraries.routing_controller.get_hmrc_to_dit_mailserver")
-    @mock.patch("mail.libraries.routing_controller.send")
+    @mock.patch("mail.libraries.routing_controller.send_email_task")
     @mock.patch("mail.libraries.routing_controller.get_email_message_dtos")
     def test_case3_sending_of_pending_licencedata_and_usagedata_mails_2(
         self,
@@ -395,7 +395,7 @@ def test_case3_sending_of_pending_licencedata_and_usagedata_mails_2(
 
     @mock.patch("mail.libraries.routing_controller.get_spire_to_dit_mailserver")
     @mock.patch("mail.libraries.routing_controller.get_hmrc_to_dit_mailserver")
-    @mock.patch("mail.libraries.routing_controller.send")
+    @mock.patch("mail.libraries.routing_controller.send_email_task")
     @mock.patch("mail.libraries.routing_controller.get_email_message_dtos")
     def test_case4_sending_of_pending_licencedata_when_waiting_for_reply(
         self,