From 4674af08949dc53be513e1556882ea8083f254bc Mon Sep 17 00:00:00 2001 From: Tim Abbott Date: Mon, 25 Sep 2017 07:03:35 -0700 Subject: [PATCH] outgoing_webhook: Fix broken way of accessing realm.uri. Previously, this accessed realm.uri via trying to use zulip_default_context. That doesn't make any sense, because zulip_default_context expects an HttpRequest object, and those are nowhere in sight in the code path. We do, however, have the outgoing webhook bot user involved in the event, and that's the object to access realm.uri from here. --- zerver/lib/outgoing_webhook.py | 6 ++-- zerver/tests/test_outgoing_webhook_system.py | 38 +++++++++++--------- 2 files changed, 25 insertions(+), 19 deletions(-) diff --git a/zerver/lib/outgoing_webhook.py b/zerver/lib/outgoing_webhook.py index 35ec3637ca..a28e70c883 100644 --- a/zerver/lib/outgoing_webhook.py +++ b/zerver/lib/outgoing_webhook.py @@ -13,7 +13,6 @@ from requests import Response from django.utils.translation import ugettext as _ -from zerver.context_processors import zulip_default_context from zerver.models import Realm, UserProfile, get_user_profile_by_id, get_client, \ GENERIC_INTERFACE, Service, SLACK_INTERFACE, email_to_domain, get_service_profile from zerver.lib.actions import check_send_message @@ -186,6 +185,8 @@ def do_rest_call(rest_operation, request_data, event, service_handler, timeout=N if error: raise JsonableError(error) + bot_user = get_user_profile_by_id(event['user_profile_id']) + http_method = rest_operation['method'] final_url = urllib.parse.urljoin(rest_operation['base_url'], rest_operation['relative_url_path']) request_kwargs = rest_operation['request_kwargs'] @@ -198,9 +199,8 @@ def do_rest_call(rest_operation, request_data, event, service_handler, timeout=N if response_message is not None: succeed_with_message(event, response_message) else: - context = zulip_default_context(request_data) message_url = ("%(server)s/#narrow/stream/%(stream)s/subject/%(subject)s/near/%(id)s" - % {'server': context['realm_uri'], + % {'server': bot_user.realm.uri, 'stream': event['message']['display_recipient'], 'subject': event['message']['subject'], 'id': str(event['message']['id'])}) diff --git a/zerver/tests/test_outgoing_webhook_system.py b/zerver/tests/test_outgoing_webhook_system.py index cd88e682d4..2e74eaa8bd 100644 --- a/zerver/tests/test_outgoing_webhook_system.py +++ b/zerver/tests/test_outgoing_webhook_system.py @@ -9,19 +9,9 @@ from requests import Response from zerver.lib.outgoing_webhook import do_rest_call, OutgoingWebhookServiceInterface from zerver.lib.test_classes import ZulipTestCase +from zerver.models import get_realm, get_user from builtins import object -mock_event = {'message': {'display_recipient': '', - 'subject': '', - 'id': ''}, - 'command': '', - 'service_name': ''} - -rest_operation = {'method': "POST", - 'relative_url_path': "", - 'request_kwargs': {}, - 'base_url': ""} - class ResponseMock(object): def __init__(self, status_code, data, content): # type: (int, Any, str) -> None @@ -45,12 +35,28 @@ class MockServiceHandler(OutgoingWebhookServiceInterface): service_handler = MockServiceHandler(None, None, None, None) class DoRestCallTests(ZulipTestCase): + def setUp(self): + # type: () -> None + realm = get_realm("zulip") + user_profile = get_user("outgoing-webhook@zulip.com", realm) + self.mock_event = {'message': {'display_recipient': '', + 'subject': '', + 'id': ''}, + 'user_profile_id': user_profile.id, + 'command': '', + 'service_name': ''} + + self.rest_operation = {'method': "POST", + 'relative_url_path': "", + 'request_kwargs': {}, + 'base_url': ""} + @mock.patch('zerver.lib.outgoing_webhook.succeed_with_message') def test_successful_request(self, mock_succeed_with_message): # type: (mock.Mock) -> None response = ResponseMock(200, {"message": "testing"}, '') with mock.patch('requests.request', return_value=response): - do_rest_call(rest_operation, None, None, service_handler, None) + do_rest_call(self.rest_operation, None, self.mock_event, service_handler, None) self.assertTrue(mock_succeed_with_message.called) @mock.patch('zerver.lib.outgoing_webhook.request_retry') @@ -58,7 +64,7 @@ class DoRestCallTests(ZulipTestCase): # type: (mock.Mock) -> None response = ResponseMock(500, {"message": "testing"}, '') with mock.patch('requests.request', return_value=response): - do_rest_call(rest_operation, None, mock_event, service_handler, None) + do_rest_call(self.rest_operation, None, self.mock_event, service_handler, None) self.assertTrue(mock_request_retry.called) @mock.patch('zerver.lib.outgoing_webhook.fail_with_message') @@ -66,7 +72,7 @@ class DoRestCallTests(ZulipTestCase): # type: (mock.Mock) -> None response = ResponseMock(400, {"message": "testing"}, '') with mock.patch('requests.request', return_value=response): - do_rest_call(rest_operation, None, mock_event, service_handler, None) + do_rest_call(self.rest_operation, None, self.mock_event, service_handler, None) self.assertTrue(mock_fail_with_message.called) @mock.patch('logging.info') @@ -74,7 +80,7 @@ class DoRestCallTests(ZulipTestCase): @mock.patch('zerver.lib.outgoing_webhook.request_retry') def test_timeout_request(self, mock_request_retry, mock_requests_request, mock_logger): # type: (mock.Mock, mock.Mock, mock.Mock) -> None - do_rest_call(rest_operation, None, mock_event, service_handler, None) + do_rest_call(self.rest_operation, None, self.mock_event, service_handler, None) self.assertTrue(mock_request_retry.called) @mock.patch('logging.exception') @@ -82,5 +88,5 @@ class DoRestCallTests(ZulipTestCase): @mock.patch('zerver.lib.outgoing_webhook.fail_with_message') def test_request_exception(self, mock_fail_with_message, mock_requests_request, mock_logger): # type: (mock.Mock, mock.Mock, mock.Mock) -> None - do_rest_call(rest_operation, None, mock_event, service_handler, None) + do_rest_call(self.rest_operation, None, self.mock_event, service_handler, None) self.assertTrue(mock_fail_with_message.called)