mirror of https://github.com/zulip/zulip.git
webhooks: Eliminate the usage of a headers.py file.
For storing HTTP headers as a function of fixture name, previously we required that the fixture_to_headers method should reside in a separate module called headers.py. However, as in many cases, this method will only take a few lines, we decided to move this function into the view.py file of the integration instead of requiring a whole new file called headers.py This commit introduces the small change in the system architecture, migrates the GitHub integration, and updates the docs accordingly.
This commit is contained in:
parent
4691028097
commit
ef52aa0fc1
|
@ -52,16 +52,18 @@ put key details like the event type in a separate HTTP header
|
|||
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, in such a case, you
|
||||
won't need to explicitly write such a special function and can instead
|
||||
just use the same helper method that the GitHub integration uses.
|
||||
Since this is 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/view.py`; basically, as part of
|
||||
writing your integration, you'll write a special function in your
|
||||
view.py file that maps the filename of the fixture to the set of HTTP
|
||||
headers to use. This function must be named "fixture_to_headers". Most
|
||||
integrations 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, in such a case, you won't need to
|
||||
explicitly write the logic for such a special function again,
|
||||
instead you can just use the same helper method that the GitHub
|
||||
integration uses.
|
||||
|
||||
## Step 1: Initialize your webhook python package
|
||||
|
||||
|
|
|
@ -146,26 +146,16 @@ def get_fixture_http_headers(integration_name: str,
|
|||
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)
|
||||
view_module_name = "zerver.webhooks.{integration_name}.view".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:
|
||||
view_module = importlib.import_module(view_module_name)
|
||||
fixture_to_headers = view_module.fixture_to_headers # type: ignore # we do extra exception handling in case it does not exist below.
|
||||
except (ImportError, AttributeError):
|
||||
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)
|
||||
|
||||
|
||||
|
|
|
@ -109,12 +109,18 @@ class WebhooksCommonTestCase(ZulipTestCase):
|
|||
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:
|
||||
def test_get_fixture_http_headers_with_no_fixtures_to_headers_function(
|
||||
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")
|
||||
self.assertEqual(
|
||||
get_fixture_http_headers("some_integration", "simple_fixture"),
|
||||
{}
|
||||
)
|
||||
|
||||
def test_standardize_headers(self) -> None:
|
||||
self.assertEqual(standardize_headers({}), {})
|
||||
|
|
|
@ -1,4 +0,0 @@
|
|||
from zerver.lib.webhooks.common import get_http_headers_from_filename
|
||||
|
||||
key = "HTTP_X_GITHUB_EVENT"
|
||||
fixture_to_headers = get_http_headers_from_filename(key)
|
|
@ -16,6 +16,10 @@ from zerver.lib.webhooks.git import CONTENT_MESSAGE_TEMPLATE, \
|
|||
get_pull_request_event_message, get_push_commits_event_message, \
|
||||
get_push_tag_event_message, get_setup_webhook_message
|
||||
from zerver.models import UserProfile
|
||||
from zerver.lib.webhooks.common import \
|
||||
get_http_headers_from_filename
|
||||
|
||||
fixture_to_headers = get_http_headers_from_filename("HTTP_X_GITHUB_EVENT")
|
||||
|
||||
class UnknownEventType(Exception):
|
||||
pass
|
||||
|
|
Loading…
Reference in New Issue