webhooks: Migrate to UnexpectedWebhookEventType.

This improves test coverage for a lot of our webhooks that relied
on ad-hoc methods to handle unexpected event types.

Note that I have deliberately skipped github_legacy, it isn't
advertised and is officially deprecated.

Also, I have refrained from making further changes to Trello, I
believe further improvements to test coverage should be covered
in separate per-webhook commits/PRs.
This commit is contained in:
Eeshan Garg
2018-05-22 12:16:45 -02:30
committed by Tim Abbott
parent 3ed20589f2
commit e0ef831993
15 changed files with 40 additions and 70 deletions

View File

@@ -97,30 +97,23 @@ not_yet_fully_covered = {
# Experimenatal
'zerver/lib/widget.py',
# Webhook integrations with incomplete coverage
'zerver/webhooks/basecamp/view.py',
'zerver/webhooks/beanstalk/view.py',
'zerver/webhooks/bitbucket2/view.py',
'zerver/webhooks/freshdesk/view.py',
'zerver/webhooks/github_legacy/view.py',
'zerver/webhooks/github/view.py',
'zerver/webhooks/github_legacy/view.py',
'zerver/webhooks/gitlab/view.py',
'zerver/webhooks/gogs/view.py',
'zerver/webhooks/greenhouse/view.py',
'zerver/webhooks/hellosign/view.py',
'zerver/webhooks/ifttt/view.py',
'zerver/webhooks/jira/view.py',
'zerver/webhooks/librato/view.py',
'zerver/webhooks/newrelic/view.py',
'zerver/webhooks/pingdom/view.py',
'zerver/webhooks/pivotal/view.py',
'zerver/webhooks/semaphore/view.py',
'zerver/webhooks/solano/view.py',
'zerver/webhooks/stripe/view.py',
'zerver/webhooks/taiga/view.py',
'zerver/webhooks/teamcity/view.py',
'zerver/webhooks/transifex/view.py',
'zerver/webhooks/travis/view.py',
'zerver/webhooks/updown/view.py',
'zerver/webhooks/zapier/view.py',
}

View File

@@ -7,7 +7,8 @@ from django.http import HttpRequest, HttpResponse
from zerver.decorator import api_key_only_webhook_view
from zerver.lib.request import REQ, has_request_variables
from zerver.lib.response import json_error, json_success
from zerver.lib.webhooks.common import check_send_webhook_message
from zerver.lib.webhooks.common import check_send_webhook_message, \
UnexpectedWebhookEventType
from zerver.models import UserProfile
from .support_event import SUPPORT_EVENTS
@@ -29,8 +30,7 @@ def api_basecamp_webhook(request: HttpRequest, user_profile: UserProfile,
event = get_event_type(payload)
if event not in SUPPORT_EVENTS:
logging.warning("Basecamp {} event is not supported".format(event))
return json_success()
raise UnexpectedWebhookEventType('Basecamp', event)
subject = get_project_name(payload)
if event.startswith('document_'):
@@ -48,8 +48,7 @@ def api_basecamp_webhook(request: HttpRequest, user_profile: UserProfile,
elif event.startswith('comment_'):
body = get_comment_body(event, payload)
else:
logging.warning("Basecamp handling of {} event is not implemented".format(event))
return json_success()
raise UnexpectedWebhookEventType('Basecamp', event)
check_send_webhook_message(request, user_profile, subject, body)
return json_success()

View File

@@ -10,7 +10,7 @@ from zerver.decorator import api_key_only_webhook_view
from zerver.lib.request import REQ, has_request_variables
from zerver.lib.response import json_error, json_success
from zerver.lib.webhooks.common import check_send_webhook_message, \
validate_extract_webhook_http_header
validate_extract_webhook_http_header, UnexpectedWebhookEventType
from zerver.lib.webhooks.git import SUBJECT_WITH_BRANCH_TEMPLATE, \
SUBJECT_WITH_PR_OR_ISSUE_INFO_TEMPLATE, \
get_commits_comment_action_message, get_force_push_commits_event_message, \
@@ -41,10 +41,6 @@ PULL_REQUEST_SUPPORTED_ACTIONS = [
'comment_deleted',
]
class UnknownTriggerType(Exception):
pass
@api_key_only_webhook_view('Bitbucket2')
@has_request_variables
def api_bitbucket2_webhook(request: HttpRequest, user_profile: UserProfile,
@@ -142,7 +138,7 @@ def get_type(request: HttpRequest, payload: Dict[str, Any]) -> str:
if event_key == 'repo:updated':
return event_key
raise UnknownTriggerType("We don't support {} event type".format(event_key))
raise UnexpectedWebhookEventType('BitBucket2', event_key)
def get_body_based_on_type(type: str) -> Callable[[Dict[str, Any]], str]:
fn = GET_SINGLE_MESSAGE_BODY_DEPENDING_ON_TYPE_MAPPER.get(type)

View File

@@ -9,7 +9,7 @@ from zerver.decorator import api_key_only_webhook_view
from zerver.lib.request import REQ, JsonableError, has_request_variables
from zerver.lib.response import json_success
from zerver.lib.webhooks.common import check_send_webhook_message, \
validate_extract_webhook_http_header
validate_extract_webhook_http_header, UnexpectedWebhookEventType
from zerver.lib.webhooks.git import CONTENT_MESSAGE_TEMPLATE, \
SUBJECT_WITH_BRANCH_TEMPLATE, SUBJECT_WITH_PR_OR_ISSUE_INFO_TEMPLATE, \
get_commits_comment_action_message, get_issue_event_message, \
@@ -402,8 +402,8 @@ def get_event(request: HttpRequest, payload: Dict[str, Any], branches: str) -> O
return "push_tags"
elif event in list(EVENT_FUNCTION_MAPPER.keys()) or event == 'ping':
return event
logging.warning(u'Event {} is unknown and cannot be handled'.format(event))
return None
raise UnexpectedWebhookEventType('GitHub', event)
def get_body_function_based_on_type(type: str) -> Any:
return EVENT_FUNCTION_MAPPER.get(type)

View File

@@ -8,7 +8,7 @@ from zerver.decorator import api_key_only_webhook_view
from zerver.lib.request import REQ, has_request_variables
from zerver.lib.response import json_success
from zerver.lib.webhooks.common import check_send_webhook_message, \
validate_extract_webhook_http_header
validate_extract_webhook_http_header, UnexpectedWebhookEventType
from zerver.lib.webhooks.git import EMPTY_SHA, \
SUBJECT_WITH_PR_OR_ISSUE_INFO_TEMPLATE, \
get_commits_comment_action_message, get_issue_event_message, \
@@ -16,9 +16,6 @@ from zerver.lib.webhooks.git import EMPTY_SHA, \
get_push_tag_event_message, get_remove_branch_event_message
from zerver.models import UserProfile
class UnknownEventType(Exception):
pass
def get_push_event_body(payload: Dict[str, Any]) -> str:
if payload.get('after') == EMPTY_SHA:
@@ -350,4 +347,5 @@ def get_event(request: HttpRequest, payload: Dict[str, Any], branches: Optional[
if event in list(EVENT_FUNCTION_MAPPER.keys()):
return event
raise UnknownEventType(u'Event {} is unknown and cannot be handled'.format(event))
raise UnexpectedWebhookEventType('GitLab', event)

View File

@@ -9,7 +9,7 @@ from zerver.decorator import api_key_only_webhook_view
from zerver.lib.request import REQ, has_request_variables
from zerver.lib.response import json_error, json_success
from zerver.lib.webhooks.common import check_send_webhook_message, \
validate_extract_webhook_http_header
validate_extract_webhook_http_header, UnexpectedWebhookEventType
from zerver.lib.webhooks.git import SUBJECT_WITH_BRANCH_TEMPLATE, \
SUBJECT_WITH_PR_OR_ISSUE_INFO_TEMPLATE, get_create_branch_event_message, \
get_pull_request_event_message, get_push_commits_event_message
@@ -92,7 +92,7 @@ def api_gogs_webhook(request: HttpRequest, user_profile: UserProfile,
title=payload['pull_request']['title']
)
else:
return json_error(_('Invalid event "{}" in request headers').format(event))
raise UnexpectedWebhookEventType('Gogs', event)
check_send_webhook_message(request, user_profile, topic, body)
return json_success()

View File

@@ -6,23 +6,6 @@ class JiraHookTests(WebhookTestCase):
STREAM_NAME = 'jira'
URL_TEMPLATE = u"/api/v1/external/jira?api_key={api_key}&stream={stream}"
def test_unknown(self) -> None:
url = self.build_webhook_url()
result = self.client_post(url,
self.get_body('unknown_v1'),
stream_name="jira",
content_type="application/json")
self.assert_json_success(result)
result = self.client_post(url,
self.get_body('unknown_v2'),
stream_name="jira",
content_type="application/json")
self.assert_json_success(result)
def test_custom_stream(self) -> None:
api_key = self.test_user.api_key
url = "/api/v1/external/jira?api_key=%s&stream=jira_custom" % (api_key,)

View File

@@ -12,7 +12,8 @@ from django.utils.translation import ugettext as _
from zerver.decorator import api_key_only_webhook_view
from zerver.lib.request import REQ, has_request_variables
from zerver.lib.response import json_error, json_success
from zerver.lib.webhooks.common import check_send_webhook_message
from zerver.lib.webhooks.common import check_send_webhook_message, \
UnexpectedWebhookEventType
from zerver.models import Realm, UserProfile, get_user
IGNORED_EVENTS = [
@@ -238,15 +239,7 @@ def api_jira_webhook(request: HttpRequest, user_profile: UserProfile,
elif event in IGNORED_EVENTS:
return json_success()
else:
if event is None:
if not settings.TEST_SUITE:
message = u"Got JIRA event with None event type: {}".format(payload)
logging.warning(message)
return json_error(_("Event is not given by JIRA"))
else:
if not settings.TEST_SUITE:
logging.warning("Got JIRA event type we don't support: {}".format(event))
return json_success()
raise UnexpectedWebhookEventType('Jira', event)
check_send_webhook_message(request, user_profile, subject, content)
return json_success()

View File

@@ -7,7 +7,8 @@ from django.utils.translation import ugettext as _
from zerver.decorator import api_key_only_webhook_view
from zerver.lib.request import REQ, has_request_variables
from zerver.lib.response import json_error, json_success
from zerver.lib.webhooks.common import check_send_webhook_message
from zerver.lib.webhooks.common import check_send_webhook_message, \
UnexpectedWebhookEventType
from zerver.lib.validator import check_dict
from zerver.models import Stream, UserProfile
@@ -30,7 +31,7 @@ def api_newrelic_webhook(request: HttpRequest, user_profile: UserProfile,
%(changelog)s""" % (deployment)
else:
return json_error(_("Unknown webhook request"))
raise UnexpectedWebhookEventType('New Relic', 'Unknown Event Type')
check_send_webhook_message(request, user_profile, subject, content)
return json_success()

View File

@@ -8,7 +8,8 @@ from django.utils.translation import ugettext as _
from zerver.decorator import api_key_only_webhook_view
from zerver.lib.request import REQ, has_request_variables
from zerver.lib.response import json_error, json_success
from zerver.lib.webhooks.common import check_send_webhook_message
from zerver.lib.webhooks.common import check_send_webhook_message, \
UnexpectedWebhookEventType
from zerver.models import UserProfile
PINGDOM_SUBJECT_TEMPLATE = '{name} status.'
@@ -41,7 +42,7 @@ def api_pingdom_webhook(request: HttpRequest, user_profile: UserProfile,
subject = get_subject_for_http_request(payload)
body = get_body_for_http_request(payload)
else:
return json_error(_('Unsupported check_type: {check_type}').format(check_type=check_type))
raise UnexpectedWebhookEventType('Pingdom', check_type)
check_send_webhook_message(request, user_profile, subject, body)
return json_success()

View File

@@ -12,7 +12,8 @@ from django.utils.translation import ugettext as _
from zerver.decorator import api_key_only_webhook_view
from zerver.lib.request import REQ, has_request_variables
from zerver.lib.response import json_error, json_success
from zerver.lib.webhooks.common import check_send_webhook_message
from zerver.lib.webhooks.common import check_send_webhook_message, \
UnexpectedWebhookEventType
from zerver.models import UserProfile
def api_pivotal_webhook_v3(request: HttpRequest, user_profile: UserProfile) -> Tuple[str, str]:
@@ -160,7 +161,7 @@ def api_pivotal_webhook_v5(request: HttpRequest, user_profile: UserProfile) -> T
# Known but unsupported Pivotal event types
pass
else:
logging.warning("Unknown Pivotal event type: %s" % (event_type,))
raise UnexpectedWebhookEventType('Pivotal Tracker', event_type)
return subject, content

View File

@@ -9,7 +9,8 @@ from django.utils.translation import ugettext as _
from zerver.decorator import api_key_only_webhook_view
from zerver.lib.request import REQ, has_request_variables
from zerver.lib.response import json_error, json_success
from zerver.lib.webhooks.common import check_send_webhook_message
from zerver.lib.webhooks.common import check_send_webhook_message, \
UnexpectedWebhookEventType
from zerver.models import UserProfile
@api_key_only_webhook_view('Stripe')
@@ -158,7 +159,7 @@ def api_stripe_webhook(request: HttpRequest, user_profile: UserProfile,
topic = "Transfer {}".format(object_id)
if body is None:
return json_error(_("We don't support {} event".format(event_type)))
raise UnexpectedWebhookEventType('Stripe', event_type)
check_send_webhook_message(request, user_profile, topic, body)

View File

@@ -7,7 +7,8 @@ from django.utils.translation import ugettext as _
from zerver.decorator import api_key_only_webhook_view
from zerver.lib.request import REQ, has_request_variables
from zerver.lib.response import json_error, json_success
from zerver.lib.webhooks.common import check_send_webhook_message
from zerver.lib.webhooks.common import check_send_webhook_message, \
UnexpectedWebhookEventType
from zerver.models import UserProfile
@api_key_only_webhook_view('Transifex')
@@ -22,6 +23,6 @@ def api_transifex_webhook(request: HttpRequest, user_profile: UserProfile,
elif reviewed:
body = "Resource {} fully reviewed.".format(resource)
else:
return json_error(_("Transifex wrong request"))
raise UnexpectedWebhookEventType('Transifex', 'Unknown Event Type')
check_send_webhook_message(request, user_profile, subject, body)
return json_success()

View File

@@ -6,7 +6,8 @@ from django.http import HttpRequest, HttpResponse
from zerver.decorator import api_key_only_webhook_view, return_success_on_head_request
from zerver.lib.response import json_success, json_error
from zerver.lib.request import REQ, has_request_variables
from zerver.lib.webhooks.common import check_send_webhook_message
from zerver.lib.webhooks.common import check_send_webhook_message, \
UnexpectedWebhookEventType
from zerver.models import UserProfile
from .card_actions import SUPPORTED_CARD_ACTIONS, process_card_action
@@ -28,7 +29,7 @@ def api_trello_webhook(request: HttpRequest,
else:
subject, body = message
except UnsupportedAction:
return json_error(_('Unsupported action_type: {action_type}'.format(action_type=action_type)))
raise UnexpectedWebhookEventType('Trello', action_type)
check_send_webhook_message(request, user_profile, subject, body)
return json_success()
@@ -38,4 +39,5 @@ def get_subject_and_body(payload: Mapping[str, Any], action_type: str) -> Option
return process_card_action(payload, action_type)
if action_type in SUPPORTED_BOARD_ACTIONS:
return process_board_action(payload, action_type)
raise UnsupportedAction('{} if not supported'.format(action_type))

View File

@@ -10,7 +10,8 @@ from zerver.decorator import api_key_only_webhook_view
from zerver.lib.exceptions import JsonableError
from zerver.lib.request import REQ, has_request_variables
from zerver.lib.response import json_error, json_success
from zerver.lib.webhooks.common import check_send_webhook_message
from zerver.lib.webhooks.common import check_send_webhook_message, \
UnexpectedWebhookEventType
from zerver.models import Client, UserProfile
SUBJECT_TEMPLATE = "{service_url}"
@@ -77,4 +78,4 @@ def get_event_type(event: Dict[str, Any]) -> str:
event_type = event_type_match.group(1)
if event_type in EVENT_TYPE_BODY_MAPPER:
return event_type
raise JsonableError(_('Unsupported Updown event type: %s') % (event['event'],))
raise UnexpectedWebhookEventType('Updown', event['event'])