Rework email send as a reference in context and provide its fake.

albu-diku · albu-diku · commit 1898891ff593 · 2026-04-08T15:41:12.000+02:00
Use the runtime configuration and its persistent context as the means
to expose the email sending logic to codepaths making use of it. This
allow centralised control over the active email sending implementation
without having to thread extra arguments through internal codepaths.

As a result, we are able to centrally intercept email sending. Do this,
and as a consequence sprinkle a bunch of additional assertions into calls
that happen to send email but this side-effect was completely uncovered.

Add a means for tests to check the the number of emails sent and whether
an email was sent to a particular recipient. For cases where the email
sent is not relevant to what is being tested, there is an escape hatch.
diff --git a/mig/server/grid_notify.py b/mig/server/grid_notify.py
@@ -130,12 +130,10 @@ def send_notifications(configuration):
         notify_message = "Found %s new events since: %s\n\n" \
             % (total_events, timestr) \
             + notify_message
-        status = send_email(
+        status = send_email(configuration,
             recipient,
             subject,
-            notify_message,
-            logger,
-            configuration)
+            notify_message)
         if status:
             logger.info("Send email with %s events to: %s"
                         % (total_events, recipient))
diff --git a/mig/shared/conf.py b/mig/shared/conf.py
@@ -54,6 +54,7 @@ class RuntimeConfiguration:
 
     def __init__(self, configuration):
         object.__setattr__(self, '_configuration', configuration)
+        object.__setattr__(self, '_context', {})
 
     def __delattr__(self, attr):
         return delattr(self._configuration, attr)
@@ -64,6 +65,17 @@ def __getattr__(self, attr):
     def __setattr__(self, attr, value):
         return setattr(self._configuration, attr, value)
 
+    def context_get(self, context_key):
+        return self._context.get(context_key, None)
+
+    def context_set(self, context_key, context_value):
+        self._context[context_key] = context_value
+
+    @classmethod
+    def is_runtime_configuration(cls, obj):
+        """Is the given object a runtime configuration."""
+        return isinstance(obj, cls)
+
 
 def get_configuration_object(config_file=None,
                              skip_log=False,
diff --git a/mig/shared/functionality/autocreate.py b/mig/shared/functionality/autocreate.py
@@ -740,8 +740,7 @@ def main(client_id, user_arguments_dict, environ=None):
         logger.info('Send email: to: %s, header: %s, smtp_server: %s'
                     % (email, email_header, smtp_server))
         logger.debug('email body:  %s' % email_msg)
-        if not send_email(email, email_header, email_msg, logger,
-                          configuration):
+        if not send_email(configuration, email, email_header, email_msg):
             output_objects.append({
                 'object_type': 'error_text', 'text': """An error occurred trying
 to send your account welcome email. Please contact support (%s) and include the
diff --git a/mig/shared/functionality/extcertaction.py b/mig/shared/functionality/extcertaction.py
@@ -316,8 +316,7 @@ def main(client_id, user_arguments_dict):
 
     logger.info('Sending email: to: %s, header: %s, msg: %s, smtp_server: %s'
                 % (admin_email, email_header, email_msg, smtp_server))
-    if not send_email(admin_email, email_header, email_msg, logger,
-                      configuration):
+    if not send_email(configuration, admin_email, email_header, email_msg):
         output_objects.append({'object_type': 'error_text', 'text':
                                """An error occurred trying to inform the site
 admins about your request for existing certificate sign up. Please contact %s site
diff --git a/mig/shared/functionality/extoidaction.py b/mig/shared/functionality/extoidaction.py
@@ -282,8 +282,7 @@ def main(client_id, user_arguments_dict):
 
     logger.info('Sending email: to: %s, header: %s, msg: %s, smtp_server: %s'
                 % (admin_email, email_header, email_msg, smtp_server))
-    if not send_email(admin_email, email_header, email_msg, logger,
-                      configuration):
+    if not send_email(configuration, admin_email, email_header, email_msg):
         output_objects.append({'object_type': 'error_text', 'text':
                                """An error occurred trying to inform the site
 admins about your request for OpenID account access. Please contact %s site
diff --git a/mig/shared/functionality/peersaction.py b/mig/shared/functionality/peersaction.py
@@ -309,8 +309,7 @@ def main(client_id, user_arguments_dict):
                     logger.info('Sending invitation: to: %s, header: %s, msg: %s, smtp_server: %s'
                                 % (peer_email, email_header, email_msg,
                                    smtp_server))
-                    if send_email(peer_email, email_header, email_msg, logger,
-                                  configuration):
+                    if send_email(configuration, peer_email, email_header, email_msg):
                         succeeded.append(peer_email)
                     else:
                         failed.append(peer_email)
@@ -369,8 +368,7 @@ def main(client_id, user_arguments_dict):
 
     logger.info('Sending email: to: %s, header: %s, msg: %s, smtp_server: %s'
                 % (admin_email, email_header, email_msg, smtp_server))
-    if not send_email(admin_email, email_header, email_msg, logger,
-                      configuration):
+    if not send_email(configuration, admin_email, email_header, email_msg):
         output_objects.append({'object_type': 'error_text', 'text': '''
 An error occurred trying to send the email about your %s peers to the site
 administrators. Please contact %s site support at %s or manually inform the
diff --git a/mig/shared/functionality/reqcertaction.py b/mig/shared/functionality/reqcertaction.py
@@ -346,8 +346,7 @@ def main(client_id, user_arguments_dict):
 
     logger.info('Sending email: to: %s, header: %s, msg: %s, smtp_server: %s'
                 % (admin_email, email_header, email_msg, smtp_server))
-    if not send_email(admin_email, email_header, email_msg, logger,
-                      configuration):
+    if not send_email(configuration, admin_email, email_header, email_msg):
         output_objects.append({'object_type': 'error_text', 'text':
                                """An error occurred trying to inform the site
 admins about your request for certificate sign up. Please contact %s site
diff --git a/mig/shared/functionality/reqoidaction.py b/mig/shared/functionality/reqoidaction.py
@@ -357,8 +357,7 @@ def main(client_id, user_arguments_dict):
 
     logger.info('Sending email: to: %s, header: %s, msg: %s, smtp_server: %s'
                 % (admin_email, email_header, email_msg, smtp_server))
-    if not send_email(admin_email, email_header, email_msg, logger,
-                      configuration):
+    if not send_email(configuration, admin_email, email_header, email_msg):
         output_objects.append({'object_type': 'error_text', 'text':
                                """An error occurred trying to inform the site
 admins about your request for OpenID account access. Please contact %s site
diff --git a/mig/shared/functionality/reqpwresetaction.py b/mig/shared/functionality/reqpwresetaction.py
@@ -271,8 +271,7 @@ def main(client_id, user_arguments_dict):
 
         logger.info('Sending email: to: %s, header: %s, msg: %s, smtp_server: %s'
                     % (email_to, email_header, email_msg, smtp_server))
-        if not send_email(email_to, email_header, email_msg, logger,
-                          configuration):
+        if not send_email(configuration, email_to, email_header, email_msg):
             output_objects.append({'object_type': 'error_text', 'text':
                                    """An error occurred trying to send the email
 for an account %s password reset request. Please contact %s site support at %s
diff --git a/mig/shared/gdp/base.py b/mig/shared/gdp/base.py
@@ -683,12 +683,10 @@ def __send_project_action_confirmation(configuration,
 
 Attached you'll find the details registered in relation to the operation.
         """ % mail_fill
-        status = send_email(
+        status = send_email(configuration,
             recipients,
             subject,
             message,
-            _logger,
-            configuration,
             files=[pdf_filepath],
         )
         if status:
diff --git a/mig/shared/notification.py b/mig/shared/notification.py
@@ -52,6 +52,7 @@
 
 from mig.shared.base import force_utf8, generate_https_urls, extract_field, \
     canonical_user_with_peers, cert_field_map, get_site_base_url
+from mig.shared.conf import RuntimeConfiguration
 from mig.shared.defaults import email_keyword_list, job_output_dir, \
     transfer_output_dir, keyword_auto, cert_auto_extend_days, \
     oid_auto_extend_days
@@ -561,11 +562,35 @@ def send_instant_message(
 
 
 def send_email(
+        configuration,
+        recipients,
+        subject,
+        message,
+        files=[],
+        custom_sender=None):
+    """Send email to recipients via the actively configured method."""
+
+    assert RuntimeConfiguration.is_runtime_configuration(configuration)
+
+    notifier = configuration.context_get('notifier')
+    if not notifier:
+        notifier = Notifier(configuration)
+        configuration.context_set('notifier', notifier)
+
+    return notifier.send_email(
+        recipients,
+        subject,
+        message,
+        files=files,
+        custom_sender=custom_sender,
+    )
+
+
+def direct_send_email(
+    configuration,
     recipients,
     subject,
     message,
-    logger,
-    configuration,
     files=[],
     custom_sender=None
 ):
@@ -589,13 +614,14 @@ def send_email(
     """
 
     _logger = configuration.logger
+
     gpg_sign = False
     if configuration.site_gpg_passphrase is not None:
         if gnupg is None:
-            logger.warning("the gnupg module is required for gpg signing")
+            _logger.warning("the gnupg module is required for gpg signing")
         else:
             gpg_sign = True
-            logger.debug("enabling automatic gpg signing of email")
+            _logger.debug("enabling automatic gpg signing of email")
 
     if recipients.find(', ') > -1:
         recipients_list = recipients.split(', ')
@@ -631,7 +657,7 @@ def send_email(
         basemsg = MIMEText(force_utf8(message), "plain", "utf8")
         mime_msg.attach(basemsg)
         if gpg_sign:
-            logger.info("signing message with gpg")
+            _logger.info("signing message with gpg")
             gpg = gnupg.GPG()
             basetext = basemsg.as_string().replace('\n', '\r\n')
             signature = gpg.sign(basetext, detach=True,
@@ -643,7 +669,7 @@ def send_email(
                 msg_sig.set_payload("%s" % signature)
                 mime_msg.attach(msg_sig)
             else:
-                logger.warning("failed to create gnupg signature")
+                _logger.warning("failed to create gnupg signature")
 
         for name in files:
             part = MIMEBase('application', "octet-stream")
@@ -652,7 +678,7 @@ def send_email(
             part.add_header('Content-Disposition',
                             'attachment', filename=os.path.basename(name))
             mime_msg.attach(part)
-        logger.debug('sending email from %s to %s:\n%s' %
+        _logger.debug('sending email from %s to %s:\n%s' %
                      (from_email, recipients, mime_msg.as_string()))
         server = smtplib.SMTP(configuration.smtp_server)
         server.set_debuglevel(0)
@@ -664,10 +690,10 @@ def send_email(
                             % errors)
             return False
         else:
-            logger.debug('Email was sent to %s' % recipients)
+            _logger.debug('Email was sent to %s' % recipients)
             return True
     except Exception as err:
-        logger.error('Sending email to %s through %s failed!: %s'
+        _logger.error('Sending email to %s through %s failed!: %s'
                      % (recipients, configuration.smtp_server, err))
         return False
 
@@ -819,8 +845,8 @@ def notify_user(
 
                     continue
 
-                if send_email(single_dest, header, message, logger,
-                              configuration, custom_sender=email_sender):
+                if send_email(configuration, single_dest, header, message,
+                              custom_sender=email_sender):
                     logger.info('email sent to %s telling that %s %s'
                                 % (single_dest, jobid, status))
                 else:
@@ -906,7 +932,7 @@ def send_resource_create_request_mail(
         os.path.basename(pending_file),
     )
 
-    status = send_email(recipients, subject, txt, logger, configuration)
+    status = send_email(configuration, recipients, subject, txt)
     if status:
         msg += '\nEmail was sent to admins'
     else:
@@ -961,3 +987,13 @@ def send_system_notification(user_id, category, message, configuration):
     return send_message_to_grid_notify(pickled_notification,
                                        configuration.logger,
                                        configuration)
+
+
+class Notifier:
+    def __init__(self, configuration):
+        self.configuration = configuration
+
+    def send_email(self, recipients, subject, message, files=[], custom_sender=None):
+        return direct_send_email(self.configuration, recipients, subject, message,
+                                                        files=[],
+                                                        custom_sender=None)
diff --git a/tests/fixture/mig_shared_configuration--new.json b/tests/fixture/mig_shared_configuration--new.json
@@ -29,7 +29,6 @@
   "ca_user": "mig-ca",
   "certs_path": "/some/place/certs",
   "cloud_services": [],
-  "config_file": null,
   "cputime_for_empty_jobs": 0,
   "default_page": [
     ""
@@ -104,14 +103,12 @@
   ],
   "log_dir": "",
   "logfile": "",
-  "logger": null,
   "logger_obj": null,
   "loglevel": "",
   "lrmstypes": [],
   "mig_code_base": "",
   "mig_path": "/some/place/mig",
   "mig_server_home": "",
-  "mig_server_id": null,
   "mig_system_files": "",
   "mig_system_run": "",
   "mig_system_storage": "",
diff --git a/tests/support/__init__.py b/tests/support/__init__.py
@@ -48,6 +48,7 @@
 from tests.support.suppconst import MIG_BASE, TEST_BASE, \
     TEST_DATA_DIR, TEST_OUTPUT_DIR, ENVHELP_OUTPUT_DIR
 from tests.support.usersupp import UserAssertMixin
+import tests.support.fakes as fakes
 
 from tests.support._env import MIG_ENV, PY2
 
@@ -252,6 +253,8 @@ def configuration(self):
 
         self._configuration = configuration_instance
 
+        fakes.instrument_test_case(self, self._configuration)
+
         return configuration_instance
 
     @property
diff --git a/tests/support/fakes.py b/tests/support/fakes.py
diff --git a/tests/test_mig_lib_janitor.py b/tests/test_mig_lib_janitor.py
diff --git a/tests/test_mig_shared_accountreq.py b/tests/test_mig_shared_accountreq.py
diff --git a/tests/test_mig_shared_configuration.py b/tests/test_mig_shared_configuration.py
