webhooks: Allow event registration using webhook_view decorator.

In addition to event filtering, we add support for registering supported
events for a webhook integration using the webhook_view decorator.

The event types are stored in the view function directly as a function
attribute, and can be later accessed via the module path and the view
function name are given (which is already specified the integrations.py)

Note that the WebhookTestCase doesn't know the name of the view function
and the module of the webhook. WEBHOOK_DIR_NAME needs to be overridden
if we want exceptions to raised when one of our test functions triggered
a unspecified event, but this practice is not enforced.

all_event_type does not need to be given even if event filters are used
in the webhook. But if a list of event types is given, it will be possible
for us to include it in the documentation while ensuring that all the
tested events are included (but not vice versa at the current stage, as
we yet not required all the events included in the list to be tested)

This guarantees that we can always access the list of all the tested
events of a webhook. This feature will be later plumbed to marcos to
display all event types dynamically in doc.md.

zerver/decorator.py

@@ -4,7 +4,7 @@ import logging
 import urllib
 from functools import wraps
 from io import BytesIO
-from typing import Callable, Dict, Optional, Tuple, TypeVar, Union, cast
+from typing import Callable, Dict, Optional, Sequence, Tuple, TypeVar, Union, cast
 
 import django_otp
 from django.conf import settings
@@ -310,6 +310,7 @@ def full_webhook_client_name(raw_client_name: Optional[str] = None) -> Optional[
 def webhook_view(
     webhook_client_name: str,
     notify_bot_owner_on_invalid_json: bool = True,
+    all_event_types: Optional[Sequence[str]] = None,
 ) -> Callable[[Callable[..., HttpResponse]], Callable[..., HttpResponse]]:
     # Unfortunately, callback protocols are insufficient for this:
     # https://mypy.readthedocs.io/en/stable/protocols.html#callback-protocols
@@ -354,6 +355,7 @@ def webhook_view(
                     )
                 raise err
 
+        _wrapped_func_arguments._all_event_types = all_event_types
         return _wrapped_func_arguments
 
     return _wrapped_view_func

zerver/lib/test_classes.py

@@ -39,6 +39,7 @@ from django.test.client import BOUNDARY, MULTIPART_CONTENT, encode_multipart
 from django.test.testcases import SerializeMixin
 from django.urls import resolve
 from django.utils import translation
+from django.utils.module_loading import import_string
 from django.utils.timezone import now as timezone_now
 from fakeldap import MockLDAP
 from two_factor.models import PhoneDevice
@@ -72,7 +73,11 @@ from zerver.lib.test_console_output import (
 from zerver.lib.test_helpers import find_key_by_email, instrument_url
 from zerver.lib.users import get_api_key
 from zerver.lib.validator import check_string
-from zerver.lib.webhooks.common import get_fixture_http_headers, standardize_headers
+from zerver.lib.webhooks.common import (
+    check_send_webhook_message,
+    get_fixture_http_headers,
+    standardize_headers,
+)
 from zerver.models import (
     Client,
     Message,
@@ -1364,18 +1369,29 @@ Output:
 
 
 class WebhookTestCase(ZulipTestCase):
-    """
-    Common for all webhooks tests
+    """Shared test class for all incoming webhooks tests.
 
-    Override below class attributes and run send_and_test_message
-    If you create your URL in uncommon way you can override build_webhook_url method
-    In case that you need modify body or create it without using fixture you can also override get_body method
+    Used by configuring the below class attributes, and calling
+    send_and_test_message in individual tests.
+
+    * Tests can override build_webhook_url if the webhook requires a
+      different URL format.
+
+    * Tests can override get_body for cases where there is no
+      available fixture file.
+
+    * Tests should specify WEBHOOK_DIR_NAME to enforce that all event
+      types are declared in the @webhook_view decorator. This is
+      important for ensuring we document all fully supported event types.
     """
 
     STREAM_NAME: Optional[str] = None
     TEST_USER_EMAIL = "webhook-bot@zulip.com"
     URL_TEMPLATE: str
     WEBHOOK_DIR_NAME: Optional[str] = None
+    # This last parameter is a workaround to handle webhooks that do not
+    # name the main function api_{WEBHOOK_DIR_NAME}_webhook.
+    VIEW_FUNCTION_NAME: Optional[str] = None
 
     @property
     def test_user(self) -> UserProfile:
@@ -1385,6 +1401,52 @@ class WebhookTestCase(ZulipTestCase):
         super().setUp()
         self.url = self.build_webhook_url()
 
+        if self.WEBHOOK_DIR_NAME is not None and self.VIEW_FUNCTION_NAME is not None:
+            # If VIEW_FUNCTION_NAME is explicitly specified and
+            # WEBHOOK_DIR_NAME is not None, an exception will be
+            # raised when a test triggers events that are not
+            # explicitly specified via the event_types parameter to
+            # the @webhook_view decorator.
+            function = import_string(
+                f"zerver.webhooks.{self.WEBHOOK_DIR_NAME}.view.{self.VIEW_FUNCTION_NAME}"
+            )
+            all_event_types = None
+
+            if hasattr(function, "_all_event_types"):
+                all_event_types = function._all_event_types
+
+            if all_event_types is None:
+                return  # nocoverage
+
+            def side_effect(*args: Any, **kwargs: Any) -> None:
+                complete_event_type = (
+                    kwargs.get("complete_event_type")
+                    if len(args) < 5
+                    else args[4]  # complete_event_type is the argument at index 4
+                )
+                if (
+                    complete_event_type is not None
+                    and all_event_types is not None
+                    and complete_event_type not in all_event_types
+                ):
+                    raise Exception(
+                        f"""
+Error: This test triggered a message using the event "{complete_event_type}", which was not properly
+registered via the @webhook_view(..., event_types=[...]). These registrations are important for Zulip
+self-documenting the supported event types for this integration.
+
+You can fix this by adding "{complete_event_type}" to ALL_EVENT_TYPES for this webhook.
+""".strip()
+                    )
+                check_send_webhook_message(*args, **kwargs)
+
+            self.patch = mock.patch(
+                f"zerver.webhooks.{self.WEBHOOK_DIR_NAME}.view.check_send_webhook_message",
+                side_effect=side_effect,
+            )
+            self.patch.start()
+            self.addCleanup(self.patch.stop)
+
     def api_stream_message(self, user: UserProfile, *args: Any, **kwargs: Any) -> HttpResponse:
         kwargs["HTTP_AUTHORIZATION"] = self.encode_user(user)
         return self.check_webhook(*args, **kwargs)

zerver/webhooks/travis/tests.py

@@ -13,6 +13,7 @@ Author: josh_mandel
 Build status: Passed :thumbs_up:
 Details: [changes](https://github.com/hl7-fhir/fhir-svn/compare/6dccb98bcfd9...6c457d366a31), [build log](https://travis-ci.org/hl7-fhir/fhir-svn/builds/92495257)
 """.strip()
+    VIEW_FUNCTION_NAME = "api_travis_webhook"
 
     def test_travis_message(self) -> None:
         """
@@ -137,6 +138,25 @@ Details: [changes](https://github.com/hl7-fhir/fhir-svn/compare/6dccb98bcfd9...6
         msg = self.get_last_message()
         self.assertNotEqual(msg.topic_name(), self.TOPIC)
 
+    def test_travis_invalid_event(self) -> None:
+        payload = self.get_body("build")
+        payload = payload.replace("push", "invalid_event")
+        expected_error_messsage = """
+Error: This test triggered a message using the event "invalid_event", which was not properly
+registered via the @webhook_view(..., event_types=[...]). These registrations are important for Zulip
+self-documenting the supported event types for this integration.
+
+You can fix this by adding "invalid_event" to ALL_EVENT_TYPES for this webhook.
+""".strip()
+        with self.assertLogs("django.request"):
+            with self.assertLogs("zerver.middleware.json_error_handler", level="ERROR") as m:
+                self.client_post(
+                    self.url,
+                    payload,
+                    content_type="application/x-www-form-urlencoded",
+                )
+            self.assertIn(expected_error_messsage, m.output[0])
+
     def get_body(self, fixture_name: str) -> str:
         return urllib.parse.urlencode(
             {"payload": self.webhook_fixture_data("travis", fixture_name, file_type="json")}

zerver/webhooks/travis/view.py

@@ -13,6 +13,10 @@ from zerver.models import UserProfile
 GOOD_STATUSES = ["Passed", "Fixed"]
 BAD_STATUSES = ["Failed", "Broken", "Still Failing", "Errored", "Canceled"]
 PENDING_STATUSES = ["Pending"]
+ALL_EVENT_TYPES = [
+    "push",
+    "pull_request",
+]
 
 MESSAGE_TEMPLATE = """\
 Author: {}
@@ -20,7 +24,7 @@ Build status: {} {}
 Details: [changes]({}), [build log]({})"""
 
 
-@webhook_view("Travis")
+@webhook_view("Travis", all_event_types=ALL_EVENT_TYPES)
 @has_request_variables
 def api_travis_webhook(
     request: HttpRequest,