Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a severity to messages #48

Merged
merged 1 commit into from
Aug 28, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions docs/api.rst
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,18 @@ Message Schemas
.. automodule:: fedora_messaging.message
:members: Message, get_class

Message Severity
----------------

Each message can have a severity associated with it. The severity is used by
applications like the notification service to determine what messages to send
to users. The severity can be set at the class level, or on a message-by-message
basis. The following are valid severity levels:

.. autodata:: fedora_messaging.message.DEBUG
.. autodata:: fedora_messaging.message.INFO
.. autodata:: fedora_messaging.message.WARNING
.. autodata:: fedora_messaging.message.ERROR

.. _exceptions-api:

Expand Down
11 changes: 6 additions & 5 deletions docs/messages.rst
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@ Messages
========

Before you release your application, you should create a subclass of
:class:`fedora_messaging.message.Message`, define a schema, and implement
some methods.
:class:`fedora_messaging.message.Message`, define a schema, define a default
severity, and implement some methods.

Schema
======
Expand All @@ -33,9 +33,10 @@ Message schema are defined using `JSON Schema`_.
Header Schema
-------------

The default header schema simply declares that the header field must be a JSON
object. You can leave the schema as-is when you define your own message, or
refine it.
The default header schema declares that the header field must be a JSON object
with several expected keys. You can leave the schema as-is when you define your
own message, or you can refine it. The base schema will always be enforced in
addition to your custom schema.


.. _body-schema:
Expand Down
82 changes: 67 additions & 15 deletions fedora_messaging/message.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,25 @@
from .exceptions import ValidationError


#: Indicates the message is for debugging or is otherwise very low priority. Users
#: will not be notified unless they've explicitly requested DEBUG level messages.
DEBUG = 10

#: Indicates the message is informational. End users will not receive notifications
#: for these messages by default. For example, automated tests passed for their
#: package.
INFO = 20

#: Indicates a problem or an otherwise important problem. Users are notified of
#: these messages when they pertain to packages they are associated with by default.
#: For example, one or more automated tests failed against their package.
WARNING = 30

#: Indicates a critically important message that users should act upon as soon as
#: possible. For example, their package no longer builds.
ERROR = 40


_log = logging.getLogger(__name__)

# Maps regular expressions to message classes
Expand Down Expand Up @@ -122,6 +141,17 @@ def get_message(routing_key, properties, body):
)
MessageClass = Message

try:
severity = properties.headers["fedora_messaging_severity"]
except KeyError:
_log.error(
"Message (headers=%r, body=%r) arrived without a severity."
" A publisher is misbehaving! Defaulting to INFO.",
properties.headers,
body,
)
severity = INFO

if properties.content_encoding is None:
_log.error("Message arrived without a content encoding")
properties.content_encoding = "utf-8"
Expand All @@ -141,7 +171,9 @@ def get_message(routing_key, properties, body):
_log.error("Failed to load message body %r, %r", body, e)
raise ValidationError(e)

message = MessageClass(body=body, topic=routing_key, properties=properties)
message = MessageClass(
body=body, topic=routing_key, properties=properties, severity=severity
)
try:
message.validate()
_log.debug("Successfully validated message %r", message)
Expand Down Expand Up @@ -172,6 +204,9 @@ class Message(object):
:func:`jsonschema.validate` to validate the message headers.
body_schema (dict): A `JSON schema <http://json-schema.org/>`_ to be used with
:func:`jsonschema.validate` to validate the message headers.
severity (int): An integer that indicates the severity of the message. This is
used to determine what messages to notify end users about and should be
:data:`DEBUG`, :data:`INFO`, :data:`WARNING`, or :data:`ERROR`.

Args:
headers (dict): A set of message headers. Consult the headers schema for
Expand All @@ -184,23 +219,36 @@ class Message(object):
provided, they will be generated.
"""

severity = INFO
topic = ""
headers_schema = {
"$schema": "http://json-schema.org/draft-04/schema#",
"description": "Schema for message headers",
"type": "object",
"properties": {
"fedora_messaging_severity": {
"type": "number",
"enum": [DEBUG, INFO, WARNING, ERROR],
},
"fedora_messaging_schema": {"type": "string"},
"sent-at": {"type": "string"},
},
}
body_schema = {
"$schema": "http://json-schema.org/draft-04/schema#",
"description": "Schema for message body",
"type": "object",
}

def __init__(self, body=None, headers=None, topic=None, properties=None):
def __init__(
self, body=None, headers=None, topic=None, properties=None, severity=None
):
self._body = body or {}
if topic:
self.topic = topic
headers = headers or {}
if severity:
self.severity = severity
self._properties = properties or self._build_properties(headers)

def _build_properties(self, headers):
Expand All @@ -209,7 +257,7 @@ def _build_properties(self, headers):
headers["fedora_messaging_schema"] = _schema_name(self.__class__)
now = datetime.datetime.utcnow().replace(microsecond=0, tzinfo=pytz.utc)
headers["sent-at"] = now.isoformat()
# message_id = "{}.{}".format(now.year, uuid.uuid4())
headers["fedora_messaging_severity"] = self.severity
message_id = str(uuid.uuid4())
return pika.BasicProperties(
content_type="application/json",
Expand Down Expand Up @@ -280,6 +328,10 @@ def validate(self):
"""
Validate the headers and body with the message schema, if any.

In addition to the user-provided schema, all messages are checked against
the base schema which requires certain message headers and the that body
be a JSON object.

.. warning:: This method should not be overridden by sub-classes.

Raises:
Expand All @@ -288,18 +340,18 @@ def validate(self):
jsonschema.SchemaError: If either the message header schema or the message body
schema are invalid.
"""
_log.debug(
'Validating message headers "%r" with schema "%r"',
self._headers,
self.headers_schema,
)
jsonschema.validate(self._headers, self.headers_schema)
_log.debug(
'Validating message body "%r" with schema "%r"',
self._body,
self.body_schema,
)
jsonschema.validate(self._body, self.body_schema)
for schema in (self.headers_schema, Message.headers_schema):
_log.debug(
'Validating message headers "%r" with schema "%r"',
self._headers,
schema,
)
jsonschema.validate(self._headers, schema)
for schema in (self.body_schema, Message.body_schema):
_log.debug(
'Validating message body "%r" with schema "%r"', self._body, schema
)
jsonschema.validate(self._body, schema)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're doing this so people don't override the base schemas too much? Are you thinking of a specific use case?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was thinking that for the headers, it'd be good to validate we added the expected schema, timestamp, etc. keys and the easiest way would be to add it to the schema, but then I wanted people to have the ability to expand upon it. With the body I think it's reasonable to demand all messages be an object, so this makes it enforced.

I can't really think of a use-case for users to actually add headers at the moment so maybe that should all be an internal thing, though.


@property
def summary(self):
Expand Down
49 changes: 47 additions & 2 deletions fedora_messaging/tests/unit/test_message.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,44 @@
import jsonschema
import mock

from fedora_messaging import message
from fedora_messaging import message, exceptions


class GetMessageTests(unittest.TestCase):
"""Tests for the :func:`fedora_messaging.message.get_message` function."""

def test_missing_severity(self):
"""Assert the default severity is INFO if it's not in the headers."""
msg = message.Message(severity=message.ERROR)
del msg._headers["fedora_messaging_severity"]

recv_msg = message.get_message("", msg._properties, b"{}")
self.assertEqual(recv_msg.severity, message.INFO)

def test_invalid_severity(self):
"""Assert the invalid severity fails validation."""
msg = message.Message()
msg._headers["fedora_messaging_severity"] = 42

self.assertRaises(
exceptions.ValidationError, message.get_message, "", msg._properties, b"{}"
)

def test_missing_headers(self):
"""Assert missing headers results in a default message."""
msg = message.Message()
msg._headers = None
expected_message = message.Message()
expected_message.id = msg.id
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You don't seem to be using this variable.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, good eye


received_msg = message.get_message(
msg._encoded_routing_key, msg._properties, msg._encoded_body
)
self.assertIsInstance(received_msg, message.Message)


class MessageTests(unittest.TestCase):
"""Tests for the :mod:`fedora_messaging.message` module."""
"""Tests for the :class:`fedora_messaging.message.Message` class."""

def test_summary(self):
"""Assert message summaries default to the message topic."""
Expand Down Expand Up @@ -99,6 +132,18 @@ def test_headers(self):
"fedora_messaging.message:Message",
)

def test_severity_default_header_set(self):
"""Assert the default severity is placed in the header if unspecified."""
self.assertEqual(message.Message.severity, message.INFO)
msg = message.Message()
self.assertEqual(msg._headers["fedora_messaging_severity"], message.INFO)

def test_severity_custom_header_set(self):
"""Assert custom severity setting is placed in the header."""
self.assertEqual(message.Message.severity, message.INFO)
msg = message.Message(severity=message.ERROR)
self.assertEqual(msg._headers["fedora_messaging_severity"], message.ERROR)

def test_sent_at(self):
"""Assert a timestamp is inserted and contains explicit timezone information."""
mock_datetime = mock.Mock()
Expand Down