Some fixes for notifications (#11094)

This includes a few fixes and small restructuring to the notification/message classes.

- Centralize message lookup and format string population on the registry model
  instead of the Notification model. This change will make more sense with my next PR
- Use `copy()` when setting the format values, as to ensure thread safety,
  these shouldn't be set on the static instance class in the registry.
- Add a base class for notifications, for messages/notifications that fall
  outside doc building.

Author: agjohnson

readthedocs/doc_builder/exceptions.py

@@ -11,20 +11,18 @@
 Then the header/body texts should be defined in ``readthedocs/notifications/messages.py``.
 """
 
+from django.utils.translation import gettext_lazy as _
 
+from readthedocs.notifications.exceptions import NotificationBaseException
 
-class BuildBaseException(Exception):
 
-    default_message = "Build user exception"
+class BuildBaseException(NotificationBaseException):
 
-    def __init__(self, message_id, format_values=None, **kwargs):
-        self.message_id = message_id
-        self.format_values = format_values
-        super().__init__(self.default_message, **kwargs)
+    default_message = _("Build user exception")
 
 
 class BuildAppError(BuildBaseException):
-    default_message = "Build app exception"
+    default_message = _("Build application exception")
 
     GENERIC_WITH_BUILD_ID = "build:app:generic-with-build-id"
     BUILDS_DISABLED = "build:app:project-builds-disabled"

readthedocs/notifications/exceptions.py

@@ -0,0 +1,18 @@
+from django.utils.translation import gettext_lazy as _
+
+
+class NotificationBaseException(Exception):
+
+    """
+    The base exception class for notification and messages.
+
+    This provides the additional ``message_id`` and ``format_values`` attributes
+    that are used for message display and registry lookup.
+    """
+
+    default_message = _("Undefined error")
+
+    def __init__(self, message_id, format_values=None, **kwargs):
+        self.message_id = message_id
+        self.format_values = format_values
+        super().__init__(self.default_message, **kwargs)

readthedocs/notifications/messages.py

@@ -1,9 +1,11 @@
+import copy
 import textwrap
 
 import structlog
 from django.template import Context, Template
 from django.utils.translation import gettext_noop as _
 
+from readthedocs.core.context_processors import readthedocs_processor
 from readthedocs.doc_builder.exceptions import (
     BuildAppError,
     BuildCancelled,
@@ -514,8 +516,18 @@ def add(self, messages):
                 raise ValueError("A message with the same 'id' is already registered.")
             self.messages[message.id] = message
 
-    def get(self, message_id):
-        return self.messages.get(message_id)
+    def get(self, message_id, format_values=None):
+        # Copy to avoid setting format values on the static instance of the
+        # message inside the registry, set on a per-request instance instead.
+        message = copy.copy(self.messages.get(message_id))
+
+        if message is not None:
+            # Always include global variables, override with provided values
+            all_format_values = readthedocs_processor(None)
+            all_format_values.update(format_values or {})
+            message.set_format_values(all_format_values)
+
+        return message
 
 
 registry = MessagesRegistry()

readthedocs/notifications/models.py

@@ -7,7 +7,6 @@
 from django.utils.translation import gettext_noop as _
 from django_extensions.db.models import TimeStampedModel
 
-from readthedocs.core.context_processors import readthedocs_processor
 
 from .constants import CANCELLED, DISMISSED, READ, UNREAD, WARNING
 from .messages import Message, registry
@@ -67,7 +66,12 @@ def __str__(self):
         return self.message_id
 
     def get_message(self):
-        message = registry.get(self.message_id)
+        # Pass the instance attached to this notification
+        format_values = {
+            "instance": self.attached_to,
+        }
+
+        message = registry.get(self.message_id, format_values=format_values)
         if message is None:
             # Log the error and return an unknown error to avoid breaking the response.
             log.error(
@@ -88,15 +92,4 @@ def get_message(self):
                 type=WARNING,
             )
 
-        # Pass the instance attached to this notification
-        all_format_values = {
-            "instance": self.attached_to,
-        }
-
-        # Always include global variables
-        all_format_values.update(readthedocs_processor(None))
-
-        # Pass the values stored in the database
-        all_format_values.update(self.format_values or {})
-        message.set_format_values(all_format_values)
         return message