From 8379aeee155573ebf1549d16dcf1ff2bb6ee975c Mon Sep 17 00:00:00 2001 From: Steve Howell Date: Thu, 11 Oct 2018 12:11:52 +0000 Subject: [PATCH] outgoing bots: Fix header for generic servers. For our bots that use GenericOutgoingWebhookService (which are basically Zulip style bots), we now include a "content-type" header of "application/json". We accomplish this by having the service classes implement their own custom method called `send_data_to_server`. For the Slack-related code, we just extracted code from `do_rest_call`, and then for the Zulip-related code, we added a `headers` parameter. --- zerver/lib/outgoing_webhook.py | 18 +++++++++++++++++- zerver/tests/test_outgoing_webhook_system.py | 13 ++++++++++++- 2 files changed, 29 insertions(+), 2 deletions(-) diff --git a/zerver/lib/outgoing_webhook.py b/zerver/lib/outgoing_webhook.py index 94c23b324e..8815638412 100644 --- a/zerver/lib/outgoing_webhook.py +++ b/zerver/lib/outgoing_webhook.py @@ -37,6 +37,13 @@ class GenericOutgoingWebhookService(OutgoingWebhookServiceInterface): "trigger": event['trigger']} return json.dumps(request_data) + def send_data_to_server(self, + base_url: str, + request_data: Any) -> Response: + headers = {'content-type': 'application/json'} + response = requests.request('POST', base_url, data=request_data, headers=headers) + return response + def process_success(self, response_json: Dict[str, Any], event: Dict[str, Any]) -> Optional[Dict[str, Any]]: if "response_not_required" in response_json and response_json['response_not_required']: @@ -78,6 +85,12 @@ class SlackOutgoingWebhookService(OutgoingWebhookServiceInterface): return request_data + def send_data_to_server(self, + base_url: str, + request_data: Any) -> Response: + response = requests.request('POST', base_url, data=request_data) + return response + def process_success(self, response_json: Dict[str, Any], event: Dict[str, Any]) -> Optional[Dict[str, Any]]: if "text" in response_json: @@ -250,7 +263,10 @@ def do_rest_call(base_url: str, event: Dict[str, Any], service_handler: Any) -> None: try: - response = requests.request('POST', base_url, data=request_data) + response = service_handler.send_data_to_server( + base_url=base_url, + request_data=request_data, + ) if str(response.status_code).startswith('2'): process_success_response(event, service_handler, response) else: diff --git a/zerver/tests/test_outgoing_webhook_system.py b/zerver/tests/test_outgoing_webhook_system.py index 96452da576..1068902af4 100644 --- a/zerver/tests/test_outgoing_webhook_system.py +++ b/zerver/tests/test_outgoing_webhook_system.py @@ -10,7 +10,12 @@ from django.test import override_settings from requests import Response from typing import Any, Dict, Tuple, Optional -from zerver.lib.outgoing_webhook import do_rest_call, GenericOutgoingWebhookService +from zerver.lib.outgoing_webhook import ( + do_rest_call, + GenericOutgoingWebhookService, + SlackOutgoingWebhookService, +) + from zerver.lib.test_classes import ZulipTestCase from zerver.models import get_realm, get_user, UserProfile, get_display_recipient @@ -59,6 +64,12 @@ class DoRestCallTests(ZulipTestCase): do_rest_call('', None, self.mock_event, service_handler) self.assertTrue(mock_send.called) + for service_class in [GenericOutgoingWebhookService, SlackOutgoingWebhookService]: + handler = service_class(None, None, None) + with mock.patch('requests.request', return_value=response): + do_rest_call('', None, self.mock_event, handler) + self.assertTrue(mock_send.called) + def test_retry_request(self: mock.Mock) -> None: response = ResponseMock(500)