diff --git a/zerver/lib/outgoing_webhook.py b/zerver/lib/outgoing_webhook.py index 3a3922e5ff..ce073a1691 100644 --- a/zerver/lib/outgoing_webhook.py +++ b/zerver/lib/outgoing_webhook.py @@ -45,7 +45,7 @@ class OutgoingWebhookServiceInterface: # The main use case for this function is to massage data from # various APIs to have similar data structures. # It also allows bots to explictly set response_not_required. - def process_success(self, response: Response, + def process_success(self, response_json: Dict[str, Any], event: Dict[str, Any]) -> Optional[Dict[str, Any]]: raise NotImplementedError() @@ -63,10 +63,8 @@ class GenericOutgoingWebhookService(OutgoingWebhookServiceInterface): "trigger": event['trigger']} return rest_operation, json.dumps(request_data) - def process_success(self, response: Response, + def process_success(self, response_json: Dict[str, Any], event: Dict[str, Any]) -> Optional[Dict[str, Any]]: - response_json = json.loads(response.text) - if "response_not_required" in response_json and response_json['response_not_required']: return None @@ -111,10 +109,8 @@ class SlackOutgoingWebhookService(OutgoingWebhookServiceInterface): return rest_operation, request_data - def process_success(self, response: Response, + def process_success(self, response_json: Dict[str, Any], event: Dict[str, Any]) -> Optional[Dict[str, Any]]: - response_json = json.loads(response.text) - if "text" in response_json: content = response_json['text'] success_data = dict(content=content) @@ -258,7 +254,8 @@ def request_retry(event: Dict[str, Any], def process_success_response(event: Dict[str, Any], service_handler: Any, response: Response) -> None: - success_data = service_handler.process_success(response, event) + response_json = json.loads(response.text) + success_data = service_handler.process_success(response_json, event) if success_data is None: return diff --git a/zerver/tests/test_outgoing_webhook_interfaces.py b/zerver/tests/test_outgoing_webhook_interfaces.py index 46085dced2..2c2e5f1a71 100644 --- a/zerver/tests/test_outgoing_webhook_interfaces.py +++ b/zerver/tests/test_outgoing_webhook_interfaces.py @@ -1,10 +1,9 @@ # -*- coding: utf-8 -*- -from typing import Any +from typing import Any, Dict import mock import json -from requests.models import Response from zerver.lib.outgoing_webhook import GenericOutgoingWebhookService, \ SlackOutgoingWebhookService from zerver.lib.test_classes import ZulipTestCase @@ -36,16 +35,15 @@ class TestGenericOutgoingWebhookService(ZulipTestCase): self.assertEqual(request_data['message'], self.event['message']) def test_process_success(self) -> None: - response = mock.Mock(spec=Response) - response.text = json.dumps({"response_not_required": True}) + response = dict(response_not_required=True) # type: Dict[str, Any] success_response = self.handler.process_success(response, self.event) self.assertEqual(success_response, None) - response.text = json.dumps({"response_string": 'test_content'}) + response = dict(response_string='test_content') success_response = self.handler.process_success(response, self.event) self.assertEqual(success_response, dict(content='test_content')) - response.text = json.dumps({}) + response = dict() success_response = self.handler.process_success(response, self.event) self.assertEqual(success_response, None) @@ -123,11 +121,10 @@ class TestSlackOutgoingWebhookService(ZulipTestCase): self.assertTrue(mock_fail_with_message.called) def test_process_success(self) -> None: - response = mock.Mock(spec=Response) - response.text = json.dumps({"response_not_required": True}) + response = dict(response_not_required=True) # type: Dict[str, Any] success_response = self.handler.process_success(response, self.stream_message_event) self.assertEqual(success_response, None) - response.text = json.dumps({"text": 'test_content'}) + response = dict(text='test_content') success_response = self.handler.process_success(response, self.stream_message_event) self.assertEqual(success_response, dict(content='test_content')) diff --git a/zerver/tests/test_outgoing_webhook_system.py b/zerver/tests/test_outgoing_webhook_system.py index 419a39136a..0465cfb672 100644 --- a/zerver/tests/test_outgoing_webhook_system.py +++ b/zerver/tests/test_outgoing_webhook_system.py @@ -27,7 +27,7 @@ def timeout_error(http_method: Any, final_url: Any, data: Any, **request_kwargs: raise requests.exceptions.Timeout("Time is up!") class MockServiceHandler(OutgoingWebhookServiceInterface): - def process_success(self, response: Response, event: Dict[str, Any]) -> Optional[Dict[str, Any]]: + def process_success(self, response_json: Dict[str, Any], event: Dict[str, Any]) -> Optional[Dict[str, Any]]: # Our tests don't really look at the content yet. # They just ensure we use the "success" codepath. success_data = dict(