mirror of https://github.com/zulip/zulip.git
webhooks: Add a system for storing http headers for integrations.
Using this system, we can now associate any fixture of any integration with a particular set of HTTP headers. A helper method called determine_http_headers was introduced, and the test suite was upgraded to use determine_http_headers. Comments and documentation significantly edited by tabbott.
This commit is contained in:
parent
1817f657ee
commit
7c145c3f68
|
@ -43,6 +43,24 @@ the 3rd party service sends, your fixture may contain JSON, URL encoded text, or
|
|||
some other kind of data. See [Step 5: Create automated tests](#step-5-create-automated-tests) or
|
||||
[Testing](https://zulip.readthedocs.io/en/latest/testing/testing.html) for further details.
|
||||
|
||||
### HTTP Headers
|
||||
|
||||
Some third-party webhook APIs, such as GitHub's, don't encode all the
|
||||
information about an event in the JSON request body. Instead, they
|
||||
put key details like the event type in a separate HTTP header
|
||||
(generally this is clear in their API documentation). In order to
|
||||
test Zulip's handling of that integration, you will need to record
|
||||
which HTTP headers are used with each fixture you capture.
|
||||
|
||||
Since this integration-dependent, Zulip offers a simple API for doing
|
||||
this, which is probably best explained by looking at the example for
|
||||
GitHub: `zerver/webhooks/github/headers.py`; basically, as part of
|
||||
writing your integration, you'll write a special function at that path
|
||||
that maps the filename of the fixture to the set of HTTP headers to
|
||||
use. Most such functions will use the same strategy as the GitHub
|
||||
integration: encoding the third party variable header data (usually
|
||||
just an event type) in the fixture filename.
|
||||
|
||||
## Step 1: Initialize your webhook python package
|
||||
|
||||
In the `zerver/webhooks/` directory, create new subdirectory that will
|
||||
|
|
|
@ -23,6 +23,7 @@ from zerver.lib.initial_password import initial_password
|
|||
from zerver.lib.utils import is_remote_server
|
||||
from zerver.lib.users import get_api_key
|
||||
from zerver.lib.sessions import get_session_dict_user
|
||||
from zerver.lib.webhooks.common import get_fixture_http_headers, parse_headers_dict
|
||||
|
||||
from zerver.lib.actions import (
|
||||
check_send_message, create_stream_if_needed, bulk_add_subscriptions,
|
||||
|
@ -751,6 +752,9 @@ class WebhookTestCase(ZulipTestCase):
|
|||
payload = self.get_body(fixture_name)
|
||||
if content_type is not None:
|
||||
kwargs['content_type'] = content_type
|
||||
headers = get_fixture_http_headers(self.FIXTURE_DIR_NAME, fixture_name)
|
||||
headers = parse_headers_dict(headers)
|
||||
kwargs.update(headers)
|
||||
msg = self.send_json_payload(self.test_user, self.url, payload,
|
||||
self.STREAM_NAME, **kwargs)
|
||||
self.do_test_topic(msg, expected_topic)
|
||||
|
@ -764,7 +768,9 @@ class WebhookTestCase(ZulipTestCase):
|
|||
payload = self.get_body(fixture_name)
|
||||
if content_type is not None:
|
||||
kwargs['content_type'] = content_type
|
||||
|
||||
headers = get_fixture_http_headers(self.FIXTURE_DIR_NAME, fixture_name)
|
||||
headers = parse_headers_dict(headers)
|
||||
kwargs.update(headers)
|
||||
sender = kwargs.get('sender', self.test_user)
|
||||
msg = self.send_json_payload(sender, self.url, payload,
|
||||
stream_name=None, **kwargs)
|
||||
|
|
|
@ -1,8 +1,9 @@
|
|||
import importlib
|
||||
from urllib.parse import unquote
|
||||
|
||||
from django.http import HttpRequest
|
||||
from django.utils.translation import ugettext as _
|
||||
from typing import Optional
|
||||
from typing import Optional, Dict, Union, Any
|
||||
|
||||
from zerver.lib.actions import check_send_stream_message, \
|
||||
check_send_private_message, send_rate_limited_pm_notification_to_bot_owner
|
||||
|
@ -96,6 +97,22 @@ def check_send_webhook_message(
|
|||
# webhook-errors.log
|
||||
pass
|
||||
|
||||
def parse_headers_dict(custom_headers: Union[None, Dict[str, Any]]) -> Dict[str, str]:
|
||||
headers = {}
|
||||
if not custom_headers:
|
||||
return {}
|
||||
for header in custom_headers:
|
||||
# Ensure what we return is in the Django HTTP headers format,
|
||||
# with HTTP_ prefixes before most actual HTTP header names.
|
||||
if header.startswith("HTTP_") or header.startswith("http_"):
|
||||
headers[header.upper().replace("-", "_")] = str(custom_headers[header])
|
||||
else:
|
||||
new_header = header.upper().replace("-", "_")
|
||||
if new_header not in ["CONTENT_TYPE", "CONTENT_LENGTH"]:
|
||||
new_header = "HTTP_" + new_header
|
||||
headers[new_header] = str(custom_headers[header])
|
||||
return headers
|
||||
|
||||
def validate_extract_webhook_http_header(request: HttpRequest, header: str,
|
||||
integration_name: str,
|
||||
fatal: Optional[bool]=True) -> Optional[str]:
|
||||
|
@ -114,3 +131,33 @@ def validate_extract_webhook_http_header(request: HttpRequest, header: str,
|
|||
raise MissingHTTPEventHeader(header)
|
||||
|
||||
return extracted_header
|
||||
|
||||
|
||||
def get_fixture_http_headers(integration_name: str,
|
||||
fixture_name: str) -> Dict["str", "str"]:
|
||||
"""For integrations that require custom HTTP headers for some (or all)
|
||||
of their test fixtures, this method will call a specially named
|
||||
function from the target integration module to determine what set
|
||||
of HTTP headers goes with the given test fixture.
|
||||
"""
|
||||
|
||||
headers_module_name = "zerver.webhooks.{integration_name}.headers".format(
|
||||
integration_name=integration_name)
|
||||
try:
|
||||
# Try to import the headers.py file. If it does not exist,
|
||||
# then that means that the integration does not have any
|
||||
# special http headers registered.
|
||||
#
|
||||
# TODO: We may want to migrate to a more explicit registration
|
||||
# strategy for this behavior rather than a try/except import.
|
||||
headers_module = importlib.import_module(headers_module_name)
|
||||
except ImportError:
|
||||
return {}
|
||||
try:
|
||||
fixture_to_headers = headers_module.fixture_to_headers # type: ignore # we do extra exception handling in case it does not exist below.
|
||||
except AttributeError as e:
|
||||
msg = ". The {module_name} module must contain a fixture_to_headers method.".format(
|
||||
module_name=headers_module_name
|
||||
)
|
||||
raise AttributeError(str(e) + msg)
|
||||
return fixture_to_headers(fixture_name)
|
||||
|
|
|
@ -1,4 +1,8 @@
|
|||
# -*- coding: utf-8 -*-
|
||||
from types import SimpleNamespace
|
||||
from mock import MagicMock, patch
|
||||
from typing import Dict
|
||||
|
||||
from django.http import HttpRequest
|
||||
|
||||
from zerver.decorator import api_key_only_webhook_view
|
||||
|
@ -7,7 +11,7 @@ from zerver.lib.test_classes import ZulipTestCase, WebhookTestCase
|
|||
from zerver.lib.webhooks.common import \
|
||||
validate_extract_webhook_http_header, \
|
||||
MISSING_EVENT_HEADER_MESSAGE, MissingHTTPEventHeader, \
|
||||
INVALID_JSON_MESSAGE
|
||||
INVALID_JSON_MESSAGE, get_fixture_http_headers, parse_headers_dict
|
||||
from zerver.models import get_user, get_realm, UserProfile
|
||||
from zerver.lib.users import get_api_key
|
||||
from zerver.lib.send_email import FromAddress
|
||||
|
@ -85,6 +89,41 @@ class WebhooksCommonTestCase(ZulipTestCase):
|
|||
self.assertEqual(msg.sender.email, self.notification_bot().email)
|
||||
self.assertEqual(msg.content, expected_msg.strip())
|
||||
|
||||
@patch("zerver.lib.webhooks.common.importlib.import_module")
|
||||
def test_get_fixture_http_headers_for_success(self, import_module_mock: MagicMock) -> None:
|
||||
def fixture_to_headers(fixture_name: str) -> Dict[str, str]:
|
||||
# A sample function which would normally perform some
|
||||
# extra operations before returning a dictionary
|
||||
# corresponding to the fixture name passed. For this test,
|
||||
# we just return a fixed dictionary.
|
||||
return {"key": "value"}
|
||||
|
||||
fake_module = SimpleNamespace(fixture_to_headers=fixture_to_headers)
|
||||
import_module_mock.return_value = fake_module
|
||||
|
||||
headers = get_fixture_http_headers("some_integration", "complex_fixture")
|
||||
self.assertEqual(headers, {"key": "value"})
|
||||
|
||||
def test_get_fixture_http_headers_for_non_existant_integration(self) -> None:
|
||||
headers = get_fixture_http_headers("some_random_nonexistant_integration", "fixture_name")
|
||||
self.assertEqual(headers, {})
|
||||
|
||||
@patch("zerver.lib.webhooks.common.importlib.import_module")
|
||||
def test_get_fixture_http_headers_for_error_handling(self, import_module_mock: MagicMock) -> None:
|
||||
fake_module = SimpleNamespace()
|
||||
import_module_mock.return_value = fake_module
|
||||
|
||||
with self.assertRaises(AttributeError):
|
||||
get_fixture_http_headers("some_integration", "simple_fixture")
|
||||
|
||||
def test_parse_headers_dict(self) -> None:
|
||||
self.assertEqual(parse_headers_dict({}), {})
|
||||
|
||||
raw_headers = {"Content-Type": "text/plain", "X-Event-Type": "ping"}
|
||||
djangoified_headers = parse_headers_dict(raw_headers)
|
||||
expected_djangoified_headers = {"CONTENT_TYPE": "text/plain", "HTTP_X_EVENT_TYPE": "ping"}
|
||||
self.assertEqual(djangoified_headers, expected_djangoified_headers)
|
||||
|
||||
class MissingEventHeaderTestCase(WebhookTestCase):
|
||||
STREAM_NAME = 'groove'
|
||||
URL_TEMPLATE = '/api/v1/external/groove?stream={stream}&api_key={api_key}'
|
||||
|
|
Loading…
Reference in New Issue