From 9dea69fda2bb6168a1a7bd57b11ca4f28e8e6a73 Mon Sep 17 00:00:00 2001
Message-Id: <9dea69fda2bb6168a1a7bd57b11ca4f28e8e6a73.1515776904.git.jeremy@jcline.org>
From: Jeremy Cline <jeremy@jcline.org>
Date: Wed, 20 Dec 2017 14:24:59 -0500
Subject: [PATCH 1/3] Refactor the tasks formatting code
Signed-off-by: Jeremy Cline <jeremy@jcline.org>
---
fmn/tasks.py | 77 ++++++++++++++++++++++++++++++++-----------------
fmn/tests/test_tasks.py | 65 +++++++++++++++++++++++++++++++++++++++++
2 files changed, 115 insertions(+), 27 deletions(-)
diff --git a/fmn/tasks.py b/fmn/tasks.py
index 4cadb6e..034ac6c 100644
--- a/fmn/tasks.py
+++ b/fmn/tasks.py
@@ -207,22 +207,7 @@ class _FindRecipients(task.Task):
models.QueuedMessage.enqueue(session, user, context, message)
continue
- formatted_message = None
- if context == 'email':
- formatted_message = formatters.email(message['body'], recipient)
- elif context == 'irc':
- formatted_message = formatters.irc(message['body'], recipient)
- elif context == 'sse':
- try:
- formatted_message = formatters.sse(message['body'], recipient)
- except Exception:
- _log.exception('An exception occurred formatting the message '
- 'for delivery: falling back to sending the raw fedmsg')
- formatted_message = message
-
- if formatted_message is None:
- raise exceptions.FmnError(
- 'The message was not formatted in any way, aborting!')
+ formatted_message = _format(context, message, recipient)
_log.info('Queuing message for delivery to %s on the %s backend', user, context)
backend_message = {
@@ -237,6 +222,49 @@ class _FindRecipients(task.Task):
session.commit()
+def _format(context, message, recipient):
+ """
+ Format the message(s) using the context and recipient to determine settings.
+
+ Args:
+ context (str): The name of the context; this is used to determine what formatter
+ function to use.
+ message (dict or list): A fedmsg or list of fedmsgs to format.
+ recipient (dict): A recipient dictionary passed on to the formatter function.
+
+ Raises:
+ FmnError: If the message could not be formatted.
+ """
+ formatted_message = None
+
+ # If it's a dictionary, it's a single message that doesn't need batching
+ if isinstance(message, dict):
+ if context == 'email':
+ formatted_message = formatters.email(message['body'], recipient)
+ elif context == 'irc':
+ formatted_message = formatters.irc(message['body'], recipient)
+ elif context == 'sse':
+ try:
+ formatted_message = formatters.sse(message['body'], recipient)
+ except Exception:
+ _log.exception('An exception occurred formatting the message '
+ 'for delivery: falling back to sending the raw fedmsg')
+ formatted_message = message
+ elif isinstance(message, list):
+ if context == 'email':
+ formatted_message = formatters.email_batch(
+ [m['body'] for m in message], recipient)
+ elif context == 'irc':
+ formatted_message = formatters.irc_batch(
+ [m['body'] for m in message], recipient)
+
+ if formatted_message is None:
+ raise exceptions.FmnError(
+ 'The message was not formatted in any way, aborting!')
+
+ return formatted_message
+
+
@app.task(name='fmn.tasks.batch_messages', ignore_results=True)
def batch_messages():
"""
@@ -273,17 +301,12 @@ def batch_messages():
for value in pref.detail_values
]
for recipient in recipients:
- formatted_message = None
- if pref.context.name == 'email':
- formatted_message = formatters.email_batch(
- [m['body'] for m in messages], recipient)
- elif pref.context.name == 'irc':
- formatted_message = formatters.irc_batch(
- [m['body'] for m in messages], recipient)
- if formatted_message is None:
- _log.error('A batch message for %r was not formatted, skipping!',
- recipient)
- continue
+ try:
+ formatted_message = _format(pref.context.name, messages, recipient)
+ except exceptions.FmnError:
+ _log.error('A batch message for %r was not formatted, skipping!',
+ recipient)
+ continue
backend_message = {
"context": pref.context.name,
diff --git a/fmn/tests/test_tasks.py b/fmn/tests/test_tasks.py
index 0939a8e..80356b4 100644
--- a/fmn/tests/test_tasks.py
+++ b/fmn/tests/test_tasks.py
@@ -25,6 +25,7 @@ from kombu import Queue
import mock
from fmn import tasks, lib as fmn_lib, constants
+from fmn.exceptions import FmnError
from fmn.lib import models
from fmn.tests import Base
@@ -333,3 +334,67 @@ email notifications@fedoraproject.org if you have any concerns/issues/abuse."""
confirmation = models.Confirmation.query.all()
self.assertEqual(1, len(confirmation))
self.assertEqual('valid', confirmation[0].status)
+
+
+@mock.patch('fmn.tasks.formatters')
+class FormatTests(Base):
+
+ def setUp(self):
+ super(FormatTests, self).setUp()
+ self.message = {
+ 'body': {
+ "msg": {
+ "changed": "rules",
+ "context": "email",
+ "openid": "jcline.id.fedoraproject.org"
+ },
+ "msg_id": "2017-6aa71d5b-fbe4-49e7-afdd-afcf0d22802b",
+ "timestamp": 1507310730,
+ "topic": "org.fedoraproject.dev.fmn.filter.update",
+ "username": "vagrant",
+ }
+ }
+ self.recipient = {
+ "email address": "jeremy@jcline.org",
+ "filter_id": 11,
+ "filter_name": "test",
+ "filter_oneshot": False,
+ "markup_messages": False,
+ "shorten_links": False,
+ "triggered_by_links": False,
+ "user": "jcline.id.fedoraproject.org",
+ "verbose": True,
+ }
+ def test_failure(self, mock_formatters):
+ """Assert an exception is raised if there's no formatted message."""
+ mock_formatters.email.return_value = None
+ self.assertRaises(FmnError, tasks._format, 'email', self.message, self.recipient)
+
+ mock_formatters.email.assert_called_once_with(self.message['body'], self.recipient)
+
+
+ def test_single_email(self, mock_formatters):
+ """Assert single messages for email context use the correct formatter."""
+ tasks._format('email', self.message, self.recipient)
+
+ mock_formatters.email.assert_called_once_with(self.message['body'], self.recipient)
+
+ def test_multi_email(self, mock_formatters):
+ """Assert multiple messages for email context use the correct formatter."""
+ tasks._format('email', [self.message, self.message], self.recipient)
+
+ mock_formatters.email_batch.assert_called_once_with(
+ [self.message['body'], self.message['body']], self.recipient)
+
+ def test_single_irc(self, mock_formatters):
+ """Assert single messages for irc context use the correct formatter."""
+ tasks._format('irc', self.message, self.recipient)
+
+ mock_formatters.irc.assert_called_once_with(self.message['body'], self.recipient)
+
+ def test_multi_irc(self, mock_formatters):
+ """Assert multiple messages for irc context use the correct formatter."""
+ tasks._format('irc', [self.message, self.message], self.recipient)
+
+ mock_formatters.irc_batch.assert_called_once_with(
+ [self.message['body'], self.message['body']], self.recipient)
--
2.15.1