Use the entry point name as the unique identifier for schema

When working on #88, I realized that using the class path as the unique
ID would be a bit odd when working from a different language. It seems
better to use the entry point name as the unique identifier.

Signed-off-by: Jeremy Cline <[email protected]>

docs/tutorial/schemas.rst

@@ -40,9 +40,9 @@ containing an important dictionary attribute: ``body_schema``. This is where
 the JSON schema lives.
 
 For clarity, edit the ``setup.py`` file and in the entry points list change the
-``mailman.schema`` name to something more relevant to your app, like
-``yourapp.schema``. The entry point name is currently unused (only the class
-path matters), but you should use something that identifies your app.
+``mailman.messageV1`` name to something more relevant to your app, like
+``yourapp.my_messageV1``. The entry point name needs to be unique to your
+application, so it's best to prefix it with your package or application name.
 
 Schema format
 ~~~~~~~~~~~~~

docs/wire-format.rst

@@ -34,9 +34,9 @@ debug-level information, 20 being informational, 30 being warning-level, and 40
 being critically important.
 
 The ``fedora_messaging_schema`` key should be set to a string that uniquely
-identifies the type of message. In the Python library, this is mapped to a
-class containing the schema and a Python API to interact with the message
-object.
+identifies the type of message. In the Python library this is the entry point
+name, which is mapped to a class containing the schema and a Python API to
+interact with the message object.
 
 The header's json-schema is::
 

fedora_messaging/message.py

@@ -1,24 +1,27 @@
 """
 This module defines the base class of message objects and keeps a registry of
 known message implementations. This registry is populated from Python entry
-points. The registry keys are Python paths and the values are sub-classes of
-:class:`Message`.
-
-When publishing, the API will add a header with the schema used so the consumer
-can locate the correct schema.
+points in the "fedora.messages" group.
 
 To implement your own message schema, simply create a class that inherits the
 :class:`Message` class, and add an entry point in your Python package under the
 "fedora.messages" group. For example, an entry point for the :class:`Message`
 schema would be::
 
     entry_points = {
-        'fedora.messages': ['base.message=fedora_messaging.message:Message']
+        'fedora.messages': [
+            'base.message=fedora_messaging.message:Message'
+        ]
     }
 
+The entry point name must be unique to your application and is used to map
+messages to your message class, so it's best to prefix it with your application
+name (e.g. ``bodhi.new_update_messageV1``). When publishing, the Fedora
+Messaging library will add a header with the entry point name of the class used
+so the consumer can locate the correct schema.
+
 Since every client needs to have the message schema installed, you should define
-this class in a small Python package of its own. Note that at this time, the
-entry point name is unused.
+this class in a small Python package of its own.
 """
 
 import datetime
@@ -58,8 +61,9 @@
 
 _log = logging.getLogger(__name__)
 
-# Maps regular expressions to message classes
-_class_registry = {}
+# Maps string names of message types to classes and back
+_schema_name_to_class = {}
+_class_to_schema_name = {}
 
 # Used to load the registry automatically on first use
 _registry_loaded = False
@@ -81,39 +85,59 @@ def get_class(schema_name):
     global _registry_loaded
     if not _registry_loaded:
         load_message_classes()
-        _registry_loaded = True
 
     try:
-        return _class_registry[schema_name]
+        return _schema_name_to_class[schema_name]
     except KeyError:
         _log.warning(
             'The schema "%s" is not in the schema registry! Either install '
             "the package with its schema definition or define a schema. "
             "Falling back to the default schema...",
             schema_name,
         )
-        return _class_registry[_schema_name(Message)]
+        return Message
 
 
-def load_message_classes():
-    """Load the 'fedora.messages' entry points and register the message classes."""
-    for message in pkg_resources.iter_entry_points("fedora.messages"):
-        cls = message.load()
-        _log.info("Registering the '%r' class as a Fedora Message", cls)
-        _class_registry[_schema_name(cls)] = cls
+def get_name(cls):
+    """
+    Retrieve the schema name associated with a message class.
 
+    Returns:
+        str: The schema name.
 
-def _schema_name(cls):
+    Raises:
+        TypeError: If the message class isn't registered. Check your entry point
+            for correctness.
     """
-    Get the Python path of a class, used to identify the message schema.
+    global _registry_loaded
+    if not _registry_loaded:
+        load_message_classes()
 
-    Args:
-        cls (object): The class (not the instance of the class).
+    try:
+        return _class_to_schema_name[cls]
+    except KeyError:
+        raise TypeError(
+            "The class {} is not in the message registry, which indicates it is"
+            ' not in the current list of entry points for "fedora_messaging".'
+            " Please check that the class has been added to your package's"
+            " entry points.".format(repr(cls))
+        )
 
-    Returns:
-        str: The path in the format "<module>:<class_name>".
-    """
-    return "{}:{}".format(cls.__module__, cls.__name__)
+
+def load_message_classes():
+    """Load the 'fedora.messages' entry points and register the message classes."""
+    for message in pkg_resources.iter_entry_points("fedora.messages"):
+        cls = message.load()
+        _log.info(
+            "Registering the '%s' key as the '%r' class in the Message "
+            "class registry",
+            message.name,
+            cls,
+        )
+        _schema_name_to_class[message.name] = cls
+        _class_to_schema_name[cls] = message.name
+    global _registry_loaded
+    _registry_loaded = True
 
 
 def get_message(routing_key, properties, body):
@@ -263,7 +287,7 @@ def __init__(
     def _build_properties(self, headers):
         # Consumers use this to determine what schema to use and if they're out
         # of date.
-        headers["fedora_messaging_schema"] = _schema_name(self.__class__)
+        headers["fedora_messaging_schema"] = get_name(self.__class__)
         now = datetime.datetime.utcnow().replace(microsecond=0, tzinfo=pytz.utc)
         headers["sent-at"] = now.isoformat()
         headers["fedora_messaging_severity"] = self.severity

fedora_messaging/tests/integration/test_publish_consume.py

@@ -104,7 +104,7 @@ def test_basic_pub_sub():
     )
     expected_headers = {
         u"fedora_messaging_severity": 20,
-        u"fedora_messaging_schema": u"fedora_messaging.message:Message",
+        u"fedora_messaging_schema": u"base.message",
         u"niceness": u"very",
     }
     messages_received = []

fedora_messaging/tests/unit/test_message.py

@@ -116,8 +116,7 @@ def test_properties_default(self):
         self.assertIn("sent-at", msg._properties.headers)
         self.assertIn("fedora_messaging_schema", msg._properties.headers)
         self.assertEqual(
-            msg._properties.headers["fedora_messaging_schema"],
-            "fedora_messaging.message:Message",
+            msg._properties.headers["fedora_messaging_schema"], "base.message"
         )
 
     def test_headers(self):
@@ -126,8 +125,7 @@ def test_headers(self):
         self.assertEqual(msg._properties.headers["foo"], "bar")
         # The fedora_messaging_schema key must also be added when headers are given.
         self.assertEqual(
-            msg._properties.headers["fedora_messaging_schema"],
-            "fedora_messaging.message:Message",
+            msg._properties.headers["fedora_messaging_schema"], "base.message"
         )
 
     def test_severity_default_header_set(self):
@@ -241,6 +239,7 @@ def flatpaks(self):
             return []
 
 
+@mock.patch.dict(message._class_to_schema_name, {CustomMessage: "custom_id"})
 class CustomMessageTests(unittest.TestCase):
     """Tests for a Message subclass that provides filter headers"""
 
@@ -288,35 +287,44 @@ def test_flatpaks(self):
 class ClassRegistryTests(unittest.TestCase):
     """Tests for the :func:`fedora_messaging.message.load_message_classes`."""
 
-    def test_load_message(self):
-        with mock.patch.dict(message._class_registry, {}, clear=True):
+    def test_load_message_name_to_class(self):
+        """Assert the entry point name maps to the class object."""
+        with mock.patch.dict(message._schema_name_to_class, {}, clear=True):
             message.load_message_classes()
-            self.assertIn("fedora_messaging.message:Message", message._class_registry)
+            self.assertIn("base.message", message._schema_name_to_class)
             self.assertTrue(
-                message._class_registry["fedora_messaging.message:Message"]
-                is message.Message
+                message._schema_name_to_class["base.message"] is message.Message
+            )
+
+    def test_load_message_class_to_name(self):
+        """Assert the entry point name maps to the class object."""
+        with mock.patch.dict(message._class_to_schema_name, {}, clear=True):
+            message.load_message_classes()
+            self.assertIn(message.Message, message._class_to_schema_name)
+            self.assertEqual(
+                "base.message", message._class_to_schema_name[message.Message]
             )
 
     @mock.patch("fedora_messaging.message._registry_loaded", False)
     def test_get_class_autoload(self):
         """Assert the registry is automatically loaded."""
-        with mock.patch.dict(message._class_registry, {}, clear=True):
-            self.assertEqual(
-                message.get_class("fedora_messaging.message:Message"), message.Message
-            )
+        with mock.patch.dict(message._schema_name_to_class, {}, clear=True):
+            self.assertEqual(message.get_class("base.message"), message.Message)
+
+    @mock.patch("fedora_messaging.message._registry_loaded", True)
+    def test_get_class_default(self):
+        """Assert the base class is returns if the class is unknown."""
+        with mock.patch.dict(message._schema_name_to_class, {}, clear=True):
+            self.assertEqual(message.get_class("no.such.message"), message.Message)
 
     @mock.patch("fedora_messaging.message._registry_loaded", False)
-    def test_get_class_autoload_first_call(self):
-        """Assert the registry loads classes on first call to get_class."""
-        with mock.patch.dict(message._class_registry, {}, clear=True):
-            self.assertEqual(
-                message.get_class("fedora_messaging.message:Message"), message.Message
-            )
+    def test_get_name_autoload(self):
+        """Assert the registry is automatically loaded."""
+        with mock.patch.dict(message._class_to_schema_name, {}, clear=True):
+            self.assertEqual(message.get_name(message.Message), "base.message")
 
     @mock.patch("fedora_messaging.message._registry_loaded", True)
-    def test_get_class_autoload_once(self):
+    def test_get_name_autoload_once(self):
         """Assert the registry doesn't repeatedly load."""
-        with mock.patch.dict(message._class_registry, {}, clear=True):
-            self.assertRaises(
-                KeyError, message.get_class, "fedora_messaging.message:Message"
-            )
+        with mock.patch.dict(message._class_to_schema_name, {}, clear=True):
+            self.assertRaises(TypeError, message.get_name, "this.is.not.an.entrypoint")

news/PR89.api

@@ -0,0 +1,2 @@
+The name of the entry point is now used to identify the message type. This is
+documented in detail in the wire format.