From 1a568d35c6fe28d04372a42352d96608a65ef0fb Mon Sep 17 00:00:00 2001 From: Nikolay Mahotkin Date: Fri, 14 Nov 2014 13:08:39 +0300 Subject: [PATCH] Refactor std.email action Closes-bug: #1392652 Change-Id: I308b28aae758b2b99b5ae5f39f8f74743544f00a --- mistral/actions/std_actions.py | 16 ++-- .../unit/actions/test_std_email_action.py | 75 +++++++++++-------- .../unit/services/test_action_manager.py | 8 +- mistral/tests/unit/utils/test_inpect_utils.py | 4 +- 4 files changed, 60 insertions(+), 43 deletions(-) diff --git a/mistral/actions/std_actions.py b/mistral/actions/std_actions.py index 842c68a5..eda474ed 100644 --- a/mistral/actions/std_actions.py +++ b/mistral/actions/std_actions.py @@ -193,19 +193,19 @@ class MistralHTTPAction(HTTPAction): class SendEmailAction(base.Action): - def __init__(self, params, settings): + def __init__(self, from_addr, to_addrs, smtp_server, + smtp_password, subject=None, body=None): # TODO(dzimine): validate parameters # Task invocation parameters. - self.to = ', '.join(params['to']) - self.subject = params['subject'] - self.body = params['body'] + self.to = ', '.join(to_addrs) + self.subject = subject or "" + self.body = body # Action provider settings. - self.smtp_server = settings['smtp_server'] - self.sender = settings['from'] - self.password = settings['password'] \ - if 'password' in settings else None + self.smtp_server = smtp_server + self.sender = from_addr + self.password = smtp_password def run(self): LOG.info("Sending email message " diff --git a/mistral/tests/unit/actions/test_std_email_action.py b/mistral/tests/unit/actions/test_std_email_action.py index eb938cde..12478e42 100644 --- a/mistral/tests/unit/actions/test_std_email_action.py +++ b/mistral/tests/unit/actions/test_std_email_action.py @@ -49,79 +49,90 @@ class SendEmailActionTest(base.BaseTest): def setUp(self): super(SendEmailActionTest, self).setUp() - self.params = { - 'to': ["dz@example.com, deg@example.com", "xyz@example.com"], - 'subject': "Multi word subject с русскими буквами", - 'body': "short multiline\nbody\nc русскими буквами", - } - self.settings = { - 'smtp_server': 'mail.example.com:25', - 'from': "bot@example.com", - } - self.to_addrs = ', '.join(self.params['to']) + self.to_addrs = ["dz@example.com, deg@example.com", "xyz@example.com"] + self.subject = "Multi word subject с русскими буквами" + self.body = "short multiline\nbody\nc русскими буквами" + + self.smtp_server = 'mail.example.com:25' + self.from_addr = "bot@example.com" + + self.to_addrs_str = ", ".join(self.to_addrs) @testtools.skipIf(not LOCAL_SMTPD, "Setup local smtpd to run it") def test_send_email_real(self): - action = std.SendEmailAction(self.params, self.settings) + action = std.SendEmailAction( + self.from_addr, self.to_addrs, + self.smtp_server, None, self.subject, self.body + ) action.run() @testtools.skipIf(not REMOTE_SMTP, "Configure Remote SMTP to run it") def test_with_password_real(self): - self.params['to'] = ["dz@stackstorm.com"] - self.settings = { - 'smtp_server': 'smtp.gmail.com:587', - 'from': "username@gmail.com", - 'password': 'secret' - } + self.to_addrs = ["dz@stackstorm.com"] + self.smtp_server = 'mail.example.com:25' + self.from_addr = "bot@example.com" + self.smtp_password = 'secret' - action = std.SendEmailAction(self.params, self.settings) + action = std.SendEmailAction( + self.from_addr, self.to_addrs, + self.smtp_server, self.smtp_password, self.subject, self.body + ) action.run() @mock.patch('smtplib.SMTP') def test_send_email(self, smtp): - action = std.SendEmailAction(self.params, self.settings) + action = std.SendEmailAction( + self.from_addr, self.to_addrs, + self.smtp_server, None, self.subject, self.body + ) action.run() - smtp.assert_called_once_with(self.settings['smtp_server']) + smtp.assert_called_once_with(self.smtp_server) sendmail = smtp.return_value.sendmail self.assertTrue(sendmail.called, "should call sendmail") self.assertEqual( - sendmail.call_args[1]['from_addr'], self.settings['from']) + sendmail.call_args[1]['from_addr'], self.from_addr) self.assertEqual( - sendmail.call_args[1]['to_addrs'], self.to_addrs) + sendmail.call_args[1]['to_addrs'], self.to_addrs_str) message = parser.Parser().parsestr(sendmail.call_args[1]['msg']) - self.assertEqual(self.settings['from'], message['from']) - self.assertEqual(self.to_addrs, message['to']) - self.assertEqual(self.params['subject'], message['subject']) - self.assertEqual(self.params['body'], message.get_payload()) + self.assertEqual(self.from_addr, message['from']) + self.assertEqual(self.to_addrs_str, message['to']) + self.assertEqual(self.subject, message['subject']) + self.assertEqual(self.body, message.get_payload()) @mock.patch('smtplib.SMTP') def test_with_password(self, smtp): - self.settings['password'] = "secret" + self.smtp_password = "secret" - action = std.SendEmailAction(self.params, self.settings) + action = std.SendEmailAction( + self.from_addr, self.to_addrs, + self.smtp_server, self.smtp_password, self.subject, self.body + ) action.run() smtpmock = smtp.return_value calls = [mock.call.ehlo(), mock.call.starttls(), mock.call.ehlo(), - mock.call.login(self.settings['from'], - self.settings['password'])] + mock.call.login(self.from_addr, + self.smtp_password)] smtpmock.assert_has_calls(calls) self.assertTrue(smtpmock.sendmail.called, "should call sendmail") @mock.patch('mistral.actions.std_actions.LOG') def test_exception(self, log): - self.params['smtp_server'] = "wrong host" + self.smtp_server = "wrong host" - action = std.SendEmailAction(self.params, self.settings) + action = std.SendEmailAction( + self.from_addr, self.to_addrs, + self.smtp_server, None, self.subject, self.body + ) try: action.run() diff --git a/mistral/tests/unit/services/test_action_manager.py b/mistral/tests/unit/services/test_action_manager.py index 92d06e19..ccea8716 100644 --- a/mistral/tests/unit/services/test_action_manager.py +++ b/mistral/tests/unit/services/test_action_manager.py @@ -29,7 +29,13 @@ class ActionManagerTest(base.DbTestCase): ) self.assertEqual(http_action_input, std_http.input) - self.assertEqual("params, settings", std_email.input) + + std_email_input = ( + "from_addr, to_addrs, smtp_server, " + "smtp_password, subject=None, body=None" + ) + + self.assertEqual(std_email_input, std_email.input) def test_action_description(self): std_http = db_api.get_action("std.http") diff --git a/mistral/tests/unit/utils/test_inpect_utils.py b/mistral/tests/unit/utils/test_inpect_utils.py index ceb9afb7..586932b2 100644 --- a/mistral/tests/unit/utils/test_inpect_utils.py +++ b/mistral/tests/unit/utils/test_inpect_utils.py @@ -32,7 +32,7 @@ class InspectUtilsTest(base.BaseTest): self.assertEqual(http_action_params, parameters_str) def test_get_parameters_str_all_mandatory(self): - action_class = std_actions.SendEmailAction + action_class = std_actions.SSHAction parameters_str = i_u.get_arg_list_as_str(action_class.__init__) - self.assertEqual("params, settings", parameters_str) \ No newline at end of file + self.assertEqual("cmd, host, username, password", parameters_str) \ No newline at end of file