From b8f20993aa9763173992267ce6a6e9ebc3f16374 Mon Sep 17 00:00:00 2001 From: Alex Burke Date: Fri, 25 Jul 2025 15:13:48 +0200 Subject: [PATCH 1/6] Lay groundwork for splitting runtime and static configuration data. This change paves the way adding further runtime specific data to configuration objects. An instance of a configuration already contains such data, such as the mig_server_id property when in use by daemons and a reference to the logger. With needs arising for further runtime data, make a clear separation between runtime attributes and the static configuration data clear. Opt to do this by making an object which internally keeps a reference to an underlying configuration object, proxies attribute access to it and can then be used as a drop-in substitute in ay call stack of logic. Name this RuntimeConfiguration. Stop short of moving existing runtime properties as of this commit. --- mig/shared/conf.py | 32 ++++++++++++++++++++++++++++++-- tests/test_support.py | 5 ++--- 2 files changed, 32 insertions(+), 5 deletions(-) diff --git a/mig/shared/conf.py b/mig/shared/conf.py index 0de37b321..de731c697 100644 --- a/mig/shared/conf.py +++ b/mig/shared/conf.py @@ -32,17 +32,45 @@ import os import sys +from mig.shared.configuration import Configuration from mig.shared.defaults import MIG_ENV from mig.shared.fileio import unpickle +class RuntimeConfiguration: + """A proxy object to be passed in-place of a Configuration which can be + extended with information relevant only at runtime. + + Given Configuration objects are threaded into and through almost all + the necessary codepaths to make this information available, they are an + attractive place to put this - but a Configuration is currently loaded + from static per-site data. + + Resolve this dichotomy with this class - a Configuration instance will + continue to represent the static data while an object that proxies its + attributes and thus is entirely drop-in compatible with it can be handed + to callers without being mixed in with the statuc data. + """ + + def __init__(self, configuration): + object.__setattr__(self, '_configuration', configuration) + + def __delattr__(self, attr): + return setattr(self._configuration, attr) + + def __getattr__(self, attr): + return getattr(self._configuration, attr) + + def __setattr__(self, attr, value): + return setattr(self._configuration, attr, value) + + def get_configuration_object(config_file=None, skip_log=False, disable_auth_log=False): """Simple helper to call the general configuration init. Optional skip_log and disable_auth_log arguments are passed on to allow skipping the default log initialization and disabling auth log for unit tests. """ - from mig.shared.configuration import Configuration if config_file: _config_file = config_file elif os.environ.get('MIG_CONF', None): @@ -65,7 +93,7 @@ def get_configuration_object(config_file=None, skip_log=False, configuration = Configuration(_config_file, False, skip_log, disable_auth_log) - return configuration + return RuntimeConfiguration(configuration) def get_resource_configuration(resource_home, unique_resource_name, diff --git a/tests/test_support.py b/tests/test_support.py index 56b74f594..fe55395ac 100644 --- a/tests/test_support.py +++ b/tests/test_support.py @@ -35,8 +35,7 @@ from tests.support import MigTestCase, PY2, testmain, temppath, \ AssertOver, FakeConfiguration -from mig.shared.conf import get_configuration_object -from mig.shared.configuration import Configuration +from mig.shared.conf import get_configuration_object, RuntimeConfiguration class InstrumentedAssertOver(AssertOver): @@ -165,7 +164,7 @@ def test_provides_the_test_configuration(self): configuration = self.configuration # check we have a real config object - self.assertIsInstance(configuration, Configuration) + self.assertIsInstance(configuration, RuntimeConfiguration) # check for having loaded a config file from a test config dir config_file_path_parts = configuration.config_file.split(os.path.sep) config_file_path_parts.pop() # discard file part From 07f07957fab77c27426c4be2d176d67f8fd84fee Mon Sep 17 00:00:00 2001 From: Alex Burke Date: Wed, 18 Feb 2026 18:48:25 +0100 Subject: [PATCH 2/6] Expose email send as an assistant and provide its fake in the test suite. Use the runtime configuration concept to expose the sending of email to logic codepaths. This avoids adding an argument at multiple levels of the call stack of logic code purely to override email sending. 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. --- mig/server/grid_notify.py | 6 +- mig/shared/conf.py | 51 +++++++++++- mig/shared/configuration.py | 4 + mig/shared/functionality/autocreate.py | 3 +- mig/shared/functionality/extcertaction.py | 3 +- mig/shared/functionality/extoidaction.py | 3 +- mig/shared/functionality/peersaction.py | 6 +- mig/shared/functionality/reqcertaction.py | 3 +- mig/shared/functionality/reqoidaction.py | 3 +- mig/shared/functionality/reqpwresetaction.py | 3 +- mig/shared/gdp/base.py | 4 +- mig/shared/notification.py | 24 +++--- tests/support/__init__.py | 12 ++- tests/support/fakes.py | 86 ++++++++++++++++++++ tests/test_mig_lib_janitor.py | 8 ++ tests/test_mig_shared_accountreq.py | 3 + 16 files changed, 179 insertions(+), 43 deletions(-) create mode 100644 tests/support/fakes.py diff --git a/mig/server/grid_notify.py b/mig/server/grid_notify.py index b74c114ef..6cdd7f624 100755 --- 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 = configuration.send_email( 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 index de731c697..f2f579168 100644 --- a/mig/shared/conf.py +++ b/mig/shared/conf.py @@ -37,6 +37,20 @@ from mig.shared.fileio import unpickle +_RUNTIME_ASSISTANTS = { + 'send_email': None, +} + + +def default_runtime_assistants(): + runtime_services = dict(_RUNTIME_ASSISTANTS) + + from mig.shared.notification import send_email + runtime_services['send_email'] = send_email + + return runtime_services + + class RuntimeConfiguration: """A proxy object to be passed in-place of a Configuration which can be extended with information relevant only at runtime. @@ -52,8 +66,9 @@ class RuntimeConfiguration: to callers without being mixed in with the statuc data. """ - def __init__(self, configuration): + def __init__(self, configuration, assistants): object.__setattr__(self, '_configuration', configuration) + object.__setattr__(self, '_assistants', assistants) def __delattr__(self, attr): return setattr(self._configuration, attr) @@ -64,9 +79,32 @@ def __getattr__(self, attr): def __setattr__(self, attr, value): return setattr(self._configuration, attr, value) + def get_assistant(self, assistant_name): + return self._assistants[assistant_name] + + def send_email(self, *args, **kwargs): + send_email = self._assistants['send_email'] + return send_email(self, *args, **kwargs) -def get_configuration_object(config_file=None, skip_log=False, - disable_auth_log=False): + @classmethod + def create(cls, configuration, assistants): + supplied_assistants = set(assistants.keys()) + expected_assistants = set(_RUNTIME_ASSISTANTS.keys()) + + missing_assistants = expected_assistants - supplied_assistants + if missing_assistants: + raise RuntimeError('missing runtime assistants: %s' + % (missing_assistants,)) + + return cls(configuration, assistants) + + @staticmethod + def default_assistants(): + return default_runtime_assistants() + + +def get_configuration_object(config_file=None, + skip_log=False, disable_auth_log=False, raw=False): """Simple helper to call the general configuration init. Optional skip_log and disable_auth_log arguments are passed on to allow skipping the default log initialization and disabling auth log for unit tests. @@ -93,7 +131,12 @@ def get_configuration_object(config_file=None, skip_log=False, configuration = Configuration(_config_file, False, skip_log, disable_auth_log) - return RuntimeConfiguration(configuration) + + if raw: + return configuration + + assistants = RuntimeConfiguration.default_assistants() + return RuntimeConfiguration(configuration, assistants) def get_resource_configuration(resource_home, unique_resource_name, diff --git a/mig/shared/configuration.py b/mig/shared/configuration.py index 09a869905..27e02c854 100644 --- a/mig/shared/configuration.py +++ b/mig/shared/configuration.py @@ -2921,6 +2921,10 @@ def parse_peers(self, peerfile): peerfile) return peers_dict + @classmethod + def create(cls, *args, **kwargs): + return cls(*args, **kwargs) + @staticmethod def is_configuration_like(obj): """Does the given object quack like a MiG Configuration.""" diff --git a/mig/shared/functionality/autocreate.py b/mig/shared/functionality/autocreate.py index 9d704e81c..06af91a4e 100644 --- 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 configuration.send_email(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 index fb7a63010..78a22d07d 100644 --- 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 configuration.send_email(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 index cd73ba292..dd32c7028 100644 --- 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 configuration.send_email(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 index bf27c3870..c8906ed98 100644 --- 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 configuration.send_email(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 configuration.send_email(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 index 4c5c6fb4d..06c42b487 100644 --- 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 configuration.send_email(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 index bbc05dc64..4e47c4bf9 100644 --- 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 configuration.send_email(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 index b63027a6f..df4a1ef72 100644 --- 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 configuration.send_email(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 index ff5d68357..7d3de219d 100644 --- 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 = configuration.send_email( recipients, subject, message, - _logger, - configuration, files=[pdf_filepath], ) if status: diff --git a/mig/shared/notification.py b/mig/shared/notification.py index ca9af7912..a28576dfd 100644 --- a/mig/shared/notification.py +++ b/mig/shared/notification.py @@ -561,11 +561,10 @@ def send_instant_message( def send_email( + configuration, recipients, subject, message, - logger, - configuration, files=[], custom_sender=None ): @@ -589,13 +588,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 +631,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 +643,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 +652,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 +664,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 +819,8 @@ def notify_user( continue - if send_email(single_dest, header, message, logger, - configuration, custom_sender=email_sender): + if configuration.send_email(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 +906,7 @@ def send_resource_create_request_mail( os.path.basename(pending_file), ) - status = send_email(recipients, subject, txt, logger, configuration) + status = configuration.send_email(recipients, subject, txt) if status: msg += '\nEmail was sent to admins' else: diff --git a/tests/support/__init__.py b/tests/support/__init__.py index eb2c8019f..fbfa6aad7 100644 --- 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 +from tests.support.fakes import make_fake_assistants from tests.support._env import MIG_ENV, PY2 @@ -212,11 +213,14 @@ def _make_configuration_instance(testcase, configuration_to_make): if configuration_to_make == 'fakeconfig': return FakeConfiguration(logger=testcase.logger) elif configuration_to_make == 'testconfig': - from mig.shared.conf import get_configuration_object - configuration = get_configuration_object(skip_log=True, - disable_auth_log=True) + from mig.shared.conf import get_configuration_object, RuntimeConfiguration + configuration = get_configuration_object(skip_log=True, + disable_auth_log=True, + raw=True) configuration.logger = testcase.logger - return configuration + + assistants = make_fake_assistants() + return RuntimeConfiguration.create(configuration, assistants) else: raise AssertionError( "MigTestCase: unknown configuration %r" % (configuration_to_make,)) diff --git a/tests/support/fakes.py b/tests/support/fakes.py new file mode 100644 index 000000000..865be6437 --- /dev/null +++ b/tests/support/fakes.py @@ -0,0 +1,86 @@ +# -*- coding: utf-8 -*- +# +# --- BEGIN_HEADER --- +# +# __init__ - package marker and core package functions +# Copyright (C) 2003-2026 The MiG Project by the Science HPC Center at UCPH +# +# This file is part of MiG. +# +# MiG is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 2 of the License, or +# (at your option) any later version. +# +# MiG is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +# +# -- END_HEADER --- +# + +"""Fake implementations of various assistant functionality used by MiG logic.""" + +class FakeSendEmail: + def __init__(self): + self.calls = [] + self._checked = False + + def __call__(self, _, *args, **kwargs): + # Record any arguments upon a call to send email (discarding the + # leading configuration argument) + self.calls.append((args, kwargs)) + return True + + def _recipients(self): + recipients = set() + for args, kwargs in self.calls: + email_address = args[0] + recipients.add(email_address) + return recipients + + def check_empty_and_reset(self): + if self._checked: + # nothing to do + return + + has_calls = len(self.calls) > 0 + if has_calls: + suprise_recipients = [] + for args, kwargs in self.calls: + email_address = args[0] + suprise_recipients.append(email_address) + raise AssertionError('detected email sending without expectation: \n %s' + % ('\n '.join(suprise_recipients),)) + + def forgive_email(self): + self._checked = True + + @property + def called_once(self): + self._checked = True + return len(self.calls) == 1 + + def email_was_sent_to(self, email_address): + recipients = self._recipients() + return email_address in recipients + + +def make_fake_send_email(mig_test_case=None): + fake_send_email = FakeSendEmail() + + if mig_test_case: + mig_test_case._register_check(fake_send_email.check_empty_and_reset) + + return fake_send_email + + +def make_fake_assistants(mig_test_case=None): + return { + 'send_email': make_fake_send_email(mig_test_case=mig_test_case), + } diff --git a/tests/test_mig_lib_janitor.py b/tests/test_mig_lib_janitor.py index 59a522cde..916bc81ba 100644 --- a/tests/test_mig_lib_janitor.py +++ b/tests/test_mig_lib_janitor.py @@ -328,6 +328,9 @@ def test_expire_user_pending(self): handled = remind_and_expire_user_pending(self.configuration) # self.assertEqual(handled, 1) self.assertEqual(handled, 2) # counted stale and expired (see above) + fake_send_email = self.configuration.get_assistant('send_email') + self.assertTrue(fake_send_email.called_once) + self.assertTrue(fake_send_email.email_was_sent_to('test@example.com')) def test_handle_pending_requests(self): """Test combined request handling""" @@ -378,9 +381,13 @@ def test_handle_pending_requests(self): handled = handle_pending_requests(self.configuration) # self.assertEqual(handled, 2) # 1 manage + 1 expire self.assertEqual(handled, 3) # 1 manage + 1 expire + 1 stale + fake_send_email = self.configuration.get_assistant('send_email') + self.assertTrue(fake_send_email.called_once) + self.assertTrue(fake_send_email.email_was_sent_to('test@example.com')) def test_handle_janitor_tasks_full(self): """Test full janitor task scheduler""" + self.configuration.get_assistant('send_email').forgive_email() # Prepare environment with pending tasks of each kind mig_stamp = time.time() - EXPIRE_STATE_DAYS * SECS_PER_DAY - 1 mig_path = os.path.join(self.configuration.mig_system_files, @@ -849,6 +856,7 @@ def test_verify_reset_token_success(self): def test_remind_and_expire_edge_cases(self): """Test request expiration with exact boundary timestamps""" + self.configuration.get_assistant('send_email').forgive_email() now = time.time() test_cases = [ ('exact_remind', now - REMIND_REQ_DAYS * SECS_PER_DAY), diff --git a/tests/test_mig_shared_accountreq.py b/tests/test_mig_shared_accountreq.py index ecba42ff3..4c816dd4e 100644 --- a/tests/test_mig_shared_accountreq.py +++ b/tests/test_mig_shared_accountreq.py @@ -137,6 +137,7 @@ def test_listing_peers(self): self.assertEqual(peer_pickle['distinguished_name'], self.TEST_PEER_DN) def test_peer_acceptance(self): + fake_send_email = self.configuration.get_assistant('send_email') test_client_dir = self._provision_test_user(self, self.TEST_USER_DN) test_client_dir_name = os.path.basename(test_client_dir) self._record_peer_acceptance(test_client_dir_name, self.TEST_PEER_DN) @@ -155,6 +156,8 @@ def test_peer_acceptance(self): keyword_auto) self.assertTrue(success) + self.assertTrue(fake_send_email.called_once) + self.assertTrue(fake_send_email.email_was_sent_to('peer@example.com')) class MigSharedAccountreq__filters(MigTestCase, UserAssertMixin): From b88abd7b4000ea7e03ad97883a00f57245e95639 Mon Sep 17 00:00:00 2001 From: Alex Burke Date: Wed, 4 Mar 2026 12:33:17 +0100 Subject: [PATCH 3/6] Rework email sending as a reference placed in a context. --- mig/server/grid_notify.py | 2 +- mig/shared/conf.py | 46 ++-------- mig/shared/configuration.py | 1 + mig/shared/functionality/autocreate.py | 2 +- mig/shared/functionality/extcertaction.py | 2 +- mig/shared/functionality/extoidaction.py | 2 +- mig/shared/functionality/peersaction.py | 4 +- mig/shared/functionality/reqcertaction.py | 2 +- mig/shared/functionality/reqoidaction.py | 2 +- mig/shared/functionality/reqpwresetaction.py | 2 +- mig/shared/gdp/base.py | 2 +- mig/shared/notification.py | 34 ++++++- .../mig_shared_configuration--new.json | 3 - tests/support/__init__.py | 8 +- tests/support/configsupp.py | 7 ++ tests/support/fakes.py | 24 +++-- tests/test_mig_lib_janitor.py | 88 ++++++++++++------- tests/test_mig_shared_accountreq.py | 3 +- tests/test_mig_shared_configuration.py | 7 +- 19 files changed, 139 insertions(+), 102 deletions(-) diff --git a/mig/server/grid_notify.py b/mig/server/grid_notify.py index 6cdd7f624..75edc0e01 100755 --- a/mig/server/grid_notify.py +++ b/mig/server/grid_notify.py @@ -130,7 +130,7 @@ def send_notifications(configuration): notify_message = "Found %s new events since: %s\n\n" \ % (total_events, timestr) \ + notify_message - status = configuration.send_email( + status = send_email(configuration, recipient, subject, notify_message) diff --git a/mig/shared/conf.py b/mig/shared/conf.py index f2f579168..2da3dc517 100644 --- a/mig/shared/conf.py +++ b/mig/shared/conf.py @@ -37,20 +37,6 @@ from mig.shared.fileio import unpickle -_RUNTIME_ASSISTANTS = { - 'send_email': None, -} - - -def default_runtime_assistants(): - runtime_services = dict(_RUNTIME_ASSISTANTS) - - from mig.shared.notification import send_email - runtime_services['send_email'] = send_email - - return runtime_services - - class RuntimeConfiguration: """A proxy object to be passed in-place of a Configuration which can be extended with information relevant only at runtime. @@ -66,9 +52,9 @@ class RuntimeConfiguration: to callers without being mixed in with the statuc data. """ - def __init__(self, configuration, assistants): + def __init__(self, configuration): object.__setattr__(self, '_configuration', configuration) - object.__setattr__(self, '_assistants', assistants) + object.__setattr__(self, '_context', {}) def __delattr__(self, attr): return setattr(self._configuration, attr) @@ -79,28 +65,11 @@ def __getattr__(self, attr): def __setattr__(self, attr, value): return setattr(self._configuration, attr, value) - def get_assistant(self, assistant_name): - return self._assistants[assistant_name] - - def send_email(self, *args, **kwargs): - send_email = self._assistants['send_email'] - return send_email(self, *args, **kwargs) - - @classmethod - def create(cls, configuration, assistants): - supplied_assistants = set(assistants.keys()) - expected_assistants = set(_RUNTIME_ASSISTANTS.keys()) - - missing_assistants = expected_assistants - supplied_assistants - if missing_assistants: - raise RuntimeError('missing runtime assistants: %s' - % (missing_assistants,)) - - return cls(configuration, assistants) + def context_get(self, context_key): + return self._context.get(context_key, None) - @staticmethod - def default_assistants(): - return default_runtime_assistants() + def context_set(self, context_key, context_value): + self._context[context_key] = context_value def get_configuration_object(config_file=None, @@ -135,8 +104,7 @@ def get_configuration_object(config_file=None, if raw: return configuration - assistants = RuntimeConfiguration.default_assistants() - return RuntimeConfiguration(configuration, assistants) + return RuntimeConfiguration(configuration) def get_resource_configuration(resource_home, unique_resource_name, diff --git a/mig/shared/configuration.py b/mig/shared/configuration.py index 27e02c854..006812950 100644 --- a/mig/shared/configuration.py +++ b/mig/shared/configuration.py @@ -83,6 +83,7 @@ 'skip_log', 'verbose', 'logger', + '_context', ]) diff --git a/mig/shared/functionality/autocreate.py b/mig/shared/functionality/autocreate.py index 06af91a4e..5eccd55a2 100644 --- a/mig/shared/functionality/autocreate.py +++ b/mig/shared/functionality/autocreate.py @@ -740,7 +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 configuration.send_email(email, email_header, email_msg): + 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 index 78a22d07d..31f5588d9 100644 --- a/mig/shared/functionality/extcertaction.py +++ b/mig/shared/functionality/extcertaction.py @@ -316,7 +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 configuration.send_email(admin_email, email_header, email_msg): + 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 index dd32c7028..fba2ece57 100644 --- a/mig/shared/functionality/extoidaction.py +++ b/mig/shared/functionality/extoidaction.py @@ -282,7 +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 configuration.send_email(admin_email, email_header, email_msg): + 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 index c8906ed98..bb52599ca 100644 --- a/mig/shared/functionality/peersaction.py +++ b/mig/shared/functionality/peersaction.py @@ -309,7 +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 configuration.send_email(peer_email, email_header, email_msg): + if send_email(configuration, peer_email, email_header, email_msg): succeeded.append(peer_email) else: failed.append(peer_email) @@ -368,7 +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 configuration.send_email(admin_email, email_header, email_msg): + 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 index 06c42b487..b1d75a7b9 100644 --- a/mig/shared/functionality/reqcertaction.py +++ b/mig/shared/functionality/reqcertaction.py @@ -346,7 +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 configuration.send_email(admin_email, email_header, email_msg): + 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 index 4e47c4bf9..92fffa1ea 100644 --- a/mig/shared/functionality/reqoidaction.py +++ b/mig/shared/functionality/reqoidaction.py @@ -357,7 +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 configuration.send_email(admin_email, email_header, email_msg): + 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 index df4a1ef72..96a9f5bfa 100644 --- a/mig/shared/functionality/reqpwresetaction.py +++ b/mig/shared/functionality/reqpwresetaction.py @@ -271,7 +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 configuration.send_email(email_to, email_header, email_msg): + 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 index 7d3de219d..4c74ca3df 100644 --- a/mig/shared/gdp/base.py +++ b/mig/shared/gdp/base.py @@ -683,7 +683,7 @@ def __send_project_action_confirmation(configuration, Attached you'll find the details registered in relation to the operation. """ % mail_fill - status = configuration.send_email( + status = send_email(configuration, recipients, subject, message, diff --git a/mig/shared/notification.py b/mig/shared/notification.py index a28576dfd..3aad87eee 100644 --- a/mig/shared/notification.py +++ b/mig/shared/notification.py @@ -561,6 +561,26 @@ def send_instant_message( def send_email( + configuration, + recipients, + subject, + message, + files=[], + custom_sender=None): + 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, @@ -819,7 +839,7 @@ def notify_user( continue - if configuration.send_email(single_dest, header, message, + 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)) @@ -906,7 +926,7 @@ def send_resource_create_request_mail( os.path.basename(pending_file), ) - status = configuration.send_email(recipients, subject, txt) + status = send_email(configuration, recipients, subject, txt) if status: msg += '\nEmail was sent to admins' else: @@ -961,3 +981,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 index f98e271c3..5ab83c5c7 100644 --- 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 index fbfa6aad7..d5b5bc99b 100644 --- a/tests/support/__init__.py +++ b/tests/support/__init__.py @@ -48,7 +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 -from tests.support.fakes import make_fake_assistants +import tests.support.fakes as fakes from tests.support._env import MIG_ENV, PY2 @@ -218,9 +218,7 @@ def _make_configuration_instance(testcase, configuration_to_make): disable_auth_log=True, raw=True) configuration.logger = testcase.logger - - assistants = make_fake_assistants() - return RuntimeConfiguration.create(configuration, assistants) + return RuntimeConfiguration(configuration) else: raise AssertionError( "MigTestCase: unknown configuration %r" % (configuration_to_make,)) @@ -254,6 +252,8 @@ def configuration(self): self._configuration = configuration_instance + fakes.instrument_test_case(self, self._configuration) + return configuration_instance @property diff --git a/tests/support/configsupp.py b/tests/support/configsupp.py index 0846e465d..9a81e897f 100644 --- a/tests/support/configsupp.py +++ b/tests/support/configsupp.py @@ -50,6 +50,7 @@ def _generate_namespace_kwargs(): properties_and_defaults = dict(_CONFIGURATION_PROPERTIES) properties_and_defaults['logger'] = None + properties_and_defaults['_context'] = {} return properties_and_defaults @@ -83,6 +84,12 @@ def __init__(self, logger=None, **kwargs): for k, v in kwargs.items(): setattr(self, k, v) + 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 + def reload_config(self, *args, **kwargs): """Stub defined to quack like Configuration.""" diff --git a/tests/support/fakes.py b/tests/support/fakes.py index 865be6437..148810abc 100644 --- a/tests/support/fakes.py +++ b/tests/support/fakes.py @@ -24,6 +24,10 @@ # -- END_HEADER --- # +import inspect +from types import SimpleNamespace + + """Fake implementations of various assistant functionality used by MiG logic.""" class FakeSendEmail: @@ -31,7 +35,7 @@ def __init__(self): self.calls = [] self._checked = False - def __call__(self, _, *args, **kwargs): + def __call__(self, *args, **kwargs): # Record any arguments upon a call to send email (discarding the # leading configuration argument) self.calls.append((args, kwargs)) @@ -68,19 +72,27 @@ def called_once(self): def email_was_sent_to(self, email_address): recipients = self._recipients() + assert email_address in recipients, \ + f"no email was not set to recipient: {email_address}" + self._checked = True return email_address in recipients -def make_fake_send_email(mig_test_case=None): +def make_fake_notifier(mig_test_case=None): fake_send_email = FakeSendEmail() if mig_test_case: mig_test_case._register_check(fake_send_email.check_empty_and_reset) - return fake_send_email + return SimpleNamespace(send_email=fake_send_email) + +def instrument_test_case(mig_test_case=None, mig_configuration=None): + assert inspect.ismethod(getattr(mig_configuration, 'context_set', None)), \ + "supplied configuration must be usable at runtime" -def make_fake_assistants(mig_test_case=None): - return { - 'send_email': make_fake_send_email(mig_test_case=mig_test_case), + fakes_by_context_key = { + 'notifier': make_fake_notifier(mig_test_case=mig_test_case), } + for content_key, context_value in fakes_by_context_key.items(): + mig_configuration.context_set(content_key, context_value) diff --git a/tests/test_mig_lib_janitor.py b/tests/test_mig_lib_janitor.py index 916bc81ba..05ec319e4 100644 --- a/tests/test_mig_lib_janitor.py +++ b/tests/test_mig_lib_janitor.py @@ -328,7 +328,8 @@ def test_expire_user_pending(self): handled = remind_and_expire_user_pending(self.configuration) # self.assertEqual(handled, 1) self.assertEqual(handled, 2) # counted stale and expired (see above) - fake_send_email = self.configuration.get_assistant('send_email') + + fake_send_email = self.configuration.context_get('notifier').send_email self.assertTrue(fake_send_email.called_once) self.assertTrue(fake_send_email.email_was_sent_to('test@example.com')) @@ -381,13 +382,14 @@ def test_handle_pending_requests(self): handled = handle_pending_requests(self.configuration) # self.assertEqual(handled, 2) # 1 manage + 1 expire self.assertEqual(handled, 3) # 1 manage + 1 expire + 1 stale - fake_send_email = self.configuration.get_assistant('send_email') + + fake_send_email = self.configuration.context_get('notifier').send_email self.assertTrue(fake_send_email.called_once) self.assertTrue(fake_send_email.email_was_sent_to('test@example.com')) def test_handle_janitor_tasks_full(self): """Test full janitor task scheduler""" - self.configuration.get_assistant('send_email').forgive_email() + self.configuration.context_get('notifier').send_email.forgive_email() # Prepare environment with pending tasks of each kind mig_stamp = time.time() - EXPIRE_STATE_DAYS * SECS_PER_DAY - 1 mig_path = os.path.join(self.configuration.mig_system_files, @@ -504,9 +506,13 @@ def test_manage_single_req_invalid(self): self.assertTrue(any('invalid account request' in msg for msg in log_capture.output)) - # TODO: enable check for removed req once skip email allows it - # self.assertFalse(os.path.exists(req_path), - # "Failed to clean invalid req for %s" % req_path) + self.assertFalse(os.path.exists(req_path), + "Failed to clean invalid req for %s" % req_path) + + # FIXME: should this really be sending email? + fake_send_email = self.configuration.context_get('notifier').send_email + self.assertTrue(fake_send_email.called_once) + self.assertTrue(fake_send_email.email_was_sent_to('test@example.com')) def test_manage_single_req_expired_token(self): """Test request handling with expired reset token""" @@ -547,12 +553,15 @@ def test_manage_single_req_expired_token(self): self.assertTrue(any('reject expired reset token' in msg for msg in log_capture.output)) - # TODO: enable check for removed req once skip email allows it - # self.assertFalse(os.path.exists(req_path), - # "Failed to clean token req for %s" % req_path) + self.assertFalse(os.path.exists(req_path), + "Failed to clean token req for %s" % req_path) + + fake_send_email = self.configuration.context_get('notifier').send_email + self.assertTrue(fake_send_email.called_once) + self.assertTrue(fake_send_email.email_was_sent_to('test@example.com')) @unittest.skip("TODO: enable once fernet decrypt err handling is improved") - def test_manage_single_req_invalid_token(self): + def test_manage_single_req_invalid_token_decrypt(self): """Test request handling with invalid reset token""" req_dict = { 'client_id': TEST_USER_DN, @@ -586,6 +595,11 @@ def test_manage_single_req_invalid_token(self): self.assertFalse(os.path.exists(req_path), "Failed to clean token req for %s" % req_path) + # FIXME: should this really be sending an email? + fake_send_email = self.configuration.context_get('notifier').send_email + self.assertTrue(fake_send_email.called_once) + self.assertTrue(fake_send_email.email_was_sent_to('test@example.com')) + def test_manage_single_req_collision(self): """Test request handling with existing user collision""" # Create collision with the already provisioned user with TEST_USER_DN @@ -619,9 +633,14 @@ def test_manage_single_req_collision(self): ) self.assertTrue(any('ID collision' in msg for msg in log_capture.output)) - # TODO: enable check for removed req once skip email allows it - # self.assertFalse(os.path.exists(req_path), - # "Failed cleanup collision for %s" % req_path) + + self.assertFalse(os.path.exists(req_path), + "Failed cleanup collision for %s" % req_path) + + fake_send_email = self.configuration.context_get('notifier').send_email + self.assertTrue(fake_send_email.called_once) + self.assertTrue(fake_send_email.email_was_sent_to('test@example.com')) + def test_manage_single_req_auth_change(self): """Test request handling with auth password change""" @@ -643,10 +662,6 @@ def test_manage_single_req_auth_change(self): saved, req_path = save_account_request(self.configuration, req_dict) req_id = os.path.basename(req_path) - # NOTE: when using real user mail we currently hit send email errors. - # We forgive those errors here and only check any known warnings. - # TODO: integrate generic skip email support and adjust here to fit - self.logger.forgive_errors() with self.assertLogs(level='INFO') as log_capture: manage_single_req( self.configuration, @@ -661,6 +676,10 @@ def test_manage_single_req_auth_change(self): self.assertFalse(os.path.exists(req_path), "Failed to clean token req for %s" % req_path) + fake_send_email = self.configuration.context_get('notifier').send_email + self.assertTrue(fake_send_email.called_once) + self.assertTrue(fake_send_email.email_was_sent_to('test@example.com')) + def test_handle_cache_updates_stub(self): """Test handle_cache_updates placeholder returns zero""" handled = handle_cache_updates(self.configuration) @@ -808,9 +827,13 @@ def test_verify_reset_token_failure_logging(self): self.assertTrue(any('wrong hash' in msg.lower() for msg in log_capture.output)) - # TODO: enable check for removed req once skip email allows it - # self.assertFalse(os.path.exists(req_path), - # "Failed cleanup invalid token for %s" % req_path) + self.assertFalse(os.path.exists(req_path), + "Failed cleanup invalid token for %s" % req_path) + + # FIXME: should this really be sending email? + fake_send_email = self.configuration.context_get('notifier').send_email + self.assertTrue(fake_send_email.called_once) + self.assertTrue(fake_send_email.email_was_sent_to('test@example.com')) def test_verify_reset_token_success(self): """Test token verification success with valid token""" @@ -850,13 +873,16 @@ def test_verify_reset_token_success(self): self.assertTrue(any('accepted' in msg.lower() for msg in log_capture.output)) - # TODO: enable check for removed req once skip email allows it - # self.assertFalse(os.path.exists(req_path), - # "Failed cleanup invalid token for %s" % req_path) + self.assertFalse(os.path.exists(req_path), + "Failed cleanup invalid token for %s" % req_path) + + fake_send_email = self.configuration.context_get('notifier').send_email + self.assertTrue(fake_send_email.called_once) + self.assertTrue(fake_send_email.email_was_sent_to('test@example.com')) def test_remind_and_expire_edge_cases(self): """Test request expiration with exact boundary timestamps""" - self.configuration.get_assistant('send_email').forgive_email() + self.configuration.context_get('notifier').send_email.forgive_email() now = time.time() test_cases = [ ('exact_remind', now - REMIND_REQ_DAYS * SECS_PER_DAY), @@ -999,10 +1025,7 @@ def test_janitor_task_cleanup_after_reject(self): # Verify initial existence self.assertTrue(os.path.exists(req_path)) - # NOTE: when using real user mail we currently hit send email errors. - # We forgive those errors here and only check any known warnings. - # TODO: integrate generic skip email support and adjust here to fit - self.logger.forgive_errors() + manage_single_req( self.configuration, req_id, @@ -1011,9 +1034,12 @@ def test_janitor_task_cleanup_after_reject(self): time.time() ) - # TODO: enable check for removed req once skip email allows it - # self.assertFalse(os.path.exists(req_path), - # "Failed cleanup after reject for %s" % req_path) + self.assertFalse(os.path.exists(req_path), + "Failed cleanup after reject for %s" % req_path) + + fake_send_email = self.configuration.context_get('notifier').send_email + fake_send_email.email_was_sent_to('test@example.com') + def test_cleaner_with_multiple_patterns(self): """Test state cleaner with multiple filename patterns""" diff --git a/tests/test_mig_shared_accountreq.py b/tests/test_mig_shared_accountreq.py index 4c816dd4e..7eee8788c 100644 --- a/tests/test_mig_shared_accountreq.py +++ b/tests/test_mig_shared_accountreq.py @@ -137,7 +137,6 @@ def test_listing_peers(self): self.assertEqual(peer_pickle['distinguished_name'], self.TEST_PEER_DN) def test_peer_acceptance(self): - fake_send_email = self.configuration.get_assistant('send_email') test_client_dir = self._provision_test_user(self, self.TEST_USER_DN) test_client_dir_name = os.path.basename(test_client_dir) self._record_peer_acceptance(test_client_dir_name, self.TEST_PEER_DN) @@ -156,6 +155,8 @@ def test_peer_acceptance(self): keyword_auto) self.assertTrue(success) + + fake_send_email = self.configuration.context_get('notifier').send_email self.assertTrue(fake_send_email.called_once) self.assertTrue(fake_send_email.email_was_sent_to('peer@example.com')) diff --git a/tests/test_mig_shared_configuration.py b/tests/test_mig_shared_configuration.py index 1108ea3e0..56eb98b8f 100644 --- a/tests/test_mig_shared_configuration.py +++ b/tests/test_mig_shared_configuration.py @@ -38,11 +38,6 @@ _CONFIGURATION_ARGUMENTS, _CONFIGURATION_PROPERTIES -def _to_dict(obj): - return {k: v for k, v in inspect.getmembers(obj) - if not (k.startswith('__') or inspect.ismethod(v) or inspect.isfunction(v))} - - class MigSharedConfiguration__static_definitions(MigTestCase): """Coverage of the static definitions underlying Configuration objects.""" @@ -349,7 +344,7 @@ def test_default_object(self): configuration.state_path = '/some/place/state' configuration.mig_path = '/some/place/mig' - actual_values = _to_dict(configuration) + actual_values = Configuration.to_dict(configuration) prepared_fixture.assertAgainstFixture(actual_values) From 9dd028ed602c1c5b1988d863b9223b6a50094403 Mon Sep 17 00:00:00 2001 From: Alex Burke Date: Thu, 5 Mar 2026 15:02:37 +0100 Subject: [PATCH 4/6] correct attribute deletion --- mig/shared/conf.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mig/shared/conf.py b/mig/shared/conf.py index 2da3dc517..6cfc4ecf7 100644 --- a/mig/shared/conf.py +++ b/mig/shared/conf.py @@ -57,7 +57,7 @@ def __init__(self, configuration): object.__setattr__(self, '_context', {}) def __delattr__(self, attr): - return setattr(self._configuration, attr) + return delattr(self._configuration, attr) def __getattr__(self, attr): return getattr(self._configuration, attr) From ae30d7de90d9fe798bfd6a0154e7b1acb86f6074 Mon Sep 17 00:00:00 2001 From: Alex Burke Date: Thu, 5 Mar 2026 15:04:39 +0100 Subject: [PATCH 5/6] make it clear the send_email wrapper is expecting a runtime configuration --- mig/shared/conf.py | 5 +++++ mig/shared/notification.py | 6 ++++++ 2 files changed, 11 insertions(+) diff --git a/mig/shared/conf.py b/mig/shared/conf.py index 6cfc4ecf7..d835b516d 100644 --- a/mig/shared/conf.py +++ b/mig/shared/conf.py @@ -71,6 +71,11 @@ def context_get(self, context_key): 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, disable_auth_log=False, raw=False): diff --git a/mig/shared/notification.py b/mig/shared/notification.py index 3aad87eee..49b189041 100644 --- 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 @@ -567,10 +568,15 @@ def send_email( 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, From 58f2dd81cba5b1d53821f32a2bc2ad2421e08a07 Mon Sep 17 00:00:00 2001 From: Alex Burke Date: Wed, 11 Mar 2026 14:42:44 +0100 Subject: [PATCH 6/6] Add underscore to an argument not intended for common use. --- mig/shared/conf.py | 6 ++++-- tests/support/__init__.py | 2 +- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/mig/shared/conf.py b/mig/shared/conf.py index d835b516d..2e2354a1f 100644 --- a/mig/shared/conf.py +++ b/mig/shared/conf.py @@ -78,7 +78,9 @@ def is_runtime_configuration(cls, obj): def get_configuration_object(config_file=None, - skip_log=False, disable_auth_log=False, raw=False): + skip_log=False, + disable_auth_log=False, + _raw=False): """Simple helper to call the general configuration init. Optional skip_log and disable_auth_log arguments are passed on to allow skipping the default log initialization and disabling auth log for unit tests. @@ -106,7 +108,7 @@ def get_configuration_object(config_file=None, configuration = Configuration(_config_file, False, skip_log, disable_auth_log) - if raw: + if _raw: return configuration return RuntimeConfiguration(configuration) diff --git a/tests/support/__init__.py b/tests/support/__init__.py index d5b5bc99b..9dd002a76 100644 --- a/tests/support/__init__.py +++ b/tests/support/__init__.py @@ -216,7 +216,7 @@ def _make_configuration_instance(testcase, configuration_to_make): from mig.shared.conf import get_configuration_object, RuntimeConfiguration configuration = get_configuration_object(skip_log=True, disable_auth_log=True, - raw=True) + _raw=True) configuration.logger = testcase.logger return RuntimeConfiguration(configuration) else: