paulmataruso/zulip
1
0
Fork 0
mirror of https://github.com/zulip/zulip.git synced 2025-11-04 05:53:43 +00:00
Code Issues Packages Projects Releases Wiki Activity

refactor: Simplify how we use base_url.

Browse Source 
Before this change, we instantiated base_url into a superclass
of subclasses that returned base_url into a dictionary that
gets returned to our caller.

Now we just pull base_url out of service when we need to make
the REST call.
This commit is contained in:
Steve Howell
2018-10-10 22:24:55 +00:00
committed by Tim Abbott
parent b89a94f730
commit 16eff75e49
4 changed files with 18 additions and 24 deletions
Download Patch File Download Diff File Expand all files Collapse all files

15
zerver/lib/outgoing_webhook.py
View File

@@ -22,8 +22,7 @@ from zerver.decorator import JsonableError
class OutgoingWebhookServiceInterface:
def __init__(self, base_url: str, token: str, user_profile: UserProfile, service_name: str) -> None:
self.base_url = base_url # type: str
def __init__(self, token: str, user_profile: UserProfile, service_name: str) -> None:
self.token = token # type: str
self.user_profile = user_profile # type: UserProfile
self.service_name = service_name # type: str
@@ -34,7 +33,6 @@ class OutgoingWebhookServiceInterface:
#
# The REST operation is a dictionary with the following keys:
# - method
# - base_url
# - relative_url_path
# - request_kwargs
def process_event(self, event: Dict[str, Any]) -> Tuple[Dict[str, Any], Any]:
@@ -54,7 +52,6 @@ class GenericOutgoingWebhookService(OutgoingWebhookServiceInterface):
def process_event(self, event: Dict[str, Any]) -> Tuple[Dict[str, Any], Any]:
rest_operation = {'method': 'POST',
'relative_url_path': '',
'base_url': self.base_url,
'request_kwargs': {}}
request_data = {"data": event['command'],
"message": event['message'],
@@ -86,7 +83,6 @@ class SlackOutgoingWebhookService(OutgoingWebhookServiceInterface):
def process_event(self, event: Dict[str, Any]) -> Tuple[Dict[str, Any], Any]:
rest_operation = {'method': 'POST',
'relative_url_path': '',
'base_url': self.base_url,
'request_kwargs': {}}
if event['message']['type'] == 'private':
@@ -132,8 +128,7 @@ def get_service_interface_class(interface: str) -> Any:
def get_outgoing_webhook_service_handler(service: Service) -> Any:
service_interface_class = get_service_interface_class(service.interface_name())
service_interface = service_interface_class(base_url=service.base_url,
token=service.token,
service_interface = service_interface_class(token=service.token,
user_profile=service.user_profile,
service_name=service.name)
return service_interface
@@ -277,7 +272,8 @@ def process_success_response(event: Dict[str, Any],
response_data = dict(content=content)
send_response_message(bot_id=bot_id, message_info=message_info, response_data=response_data)
def do_rest_call(rest_operation: Dict[str, Any],
def do_rest_call(base_url: str,
rest_operation: Dict[str, Any],
request_data: Optional[Dict[str, Any]],
event: Dict[str, Any],
service_handler: Any,
@@ -286,7 +282,6 @@ def do_rest_call(rest_operation: Dict[str, Any],
('method', check_string),
('relative_url_path', check_string),
('request_kwargs', check_dict([])),
('base_url', check_string),
])
error = rest_operation_validator('rest_operation', rest_operation)
@@ -294,7 +289,7 @@ def do_rest_call(rest_operation: Dict[str, Any],
raise JsonableError(error)
http_method = rest_operation['method']
final_url = urllib.parse.urljoin(rest_operation['base_url'], rest_operation['relative_url_path'])
final_url = urllib.parse.urljoin(base_url, rest_operation['relative_url_path'])
request_kwargs = rest_operation['request_kwargs']
request_kwargs['timeout'] = timeout

6
zerver/tests/test_outgoing_webhook_interfaces.py
View File

@@ -22,7 +22,6 @@ class TestGenericOutgoingWebhookService(ZulipTestCase):
}
self.bot_user = get_user("outgoing-webhook@zulip.com", get_realm("zulip"))
self.handler = GenericOutgoingWebhookService(service_name='test-service',
base_url='http://example.domain.com',
token='abcdef',
user_profile=self.bot_user)
@@ -65,7 +64,6 @@ class TestGenericOutgoingWebhookService(ZulipTestCase):
request_data = json.loads(request_data)
self.assertEqual(request_data['data'], "@**test**")
self.assertEqual(request_data['token'], "abcdef")
self.assertEqual(rest_operation['base_url'], "http://example.domain.com")
self.assertEqual(rest_operation['method'], "POST")
self.assertEqual(request_data['message'], self.event['message'])
@@ -123,15 +121,13 @@ class TestSlackOutgoingWebhookService(ZulipTestCase):
}
}
self.handler = SlackOutgoingWebhookService(base_url='http://example.domain.com',
token="abcdef",
self.handler = SlackOutgoingWebhookService(token="abcdef",
user_profile=None,
service_name='test-service')
def test_process_event_stream_message(self) -> None:
rest_operation, request_data = self.handler.process_event(self.stream_message_event)
self.assertEqual(rest_operation['base_url'], 'http://example.domain.com')
self.assertEqual(rest_operation['method'], 'POST')
self.assertEqual(request_data[0][1], "abcdef") # token
self.assertEqual(request_data[1][1], "zulip") # team_id

15
zerver/tests/test_outgoing_webhook_system.py
View File

@@ -38,7 +38,7 @@ class MockServiceHandler(OutgoingWebhookServiceInterface):
)
return success_data
service_handler = MockServiceHandler(None, None, None, None)
service_handler = MockServiceHandler(None, None, None)
class DoRestCallTests(ZulipTestCase):
def setUp(self) -> None:
@@ -60,8 +60,7 @@ class DoRestCallTests(ZulipTestCase):
self.rest_operation = {'method': "POST",
'relative_url_path': "",
'request_kwargs': {},
'base_url': ""}
'request_kwargs': {}}
self.bot_user = self.example_user('outgoing_webhook_bot')
logging.disable(logging.WARNING)
@@ -69,7 +68,7 @@ class DoRestCallTests(ZulipTestCase):
def test_successful_request(self, mock_send: mock.Mock) -> None:
response = ResponseMock(200)
with mock.patch('requests.request', return_value=response):
do_rest_call(self.rest_operation, None, self.mock_event, service_handler, None)
do_rest_call('', self.rest_operation, None, self.mock_event, service_handler, None)
self.assertTrue(mock_send.called)
def test_retry_request(self: mock.Mock) -> None:
@@ -77,7 +76,7 @@ class DoRestCallTests(ZulipTestCase):
self.mock_event['failed_tries'] = 3
with mock.patch('requests.request', return_value=response):
do_rest_call(self.rest_operation, None, self.mock_event, service_handler, None)
do_rest_call('', self.rest_operation, None, self.mock_event, service_handler, None)
bot_owner_notification = self.get_last_message()
self.assertEqual(bot_owner_notification.content,
'''[A message](http://zulip.testserver/#narrow/stream/999-Verona/subject/Foo/near/) triggered an outgoing webhook.
@@ -89,7 +88,7 @@ The webhook got a response with status code *500*.''')
def test_fail_request(self, mock_fail_with_message: mock.Mock) -> None:
response = ResponseMock(400)
with mock.patch('requests.request', return_value=response):
do_rest_call(self.rest_operation, None, self.mock_event, service_handler, None)
do_rest_call('', self.rest_operation, None, self.mock_event, service_handler, None)
bot_owner_notification = self.get_last_message()
self.assertTrue(mock_fail_with_message.called)
self.assertEqual(bot_owner_notification.content,
@@ -101,7 +100,7 @@ The webhook got a response with status code *400*.''')
def helper(side_effect: Any, error_text: str) -> None:
with mock.patch('logging.info'):
with mock.patch('requests.request', side_effect=side_effect):
do_rest_call(self.rest_operation, None, self.mock_event, service_handler, None)
do_rest_call('', self.rest_operation, None, self.mock_event, service_handler, None)
bot_owner_notification = self.get_last_message()
self.assertIn(error_text, bot_owner_notification.content)
self.assertIn('triggered', bot_owner_notification.content)
@@ -115,7 +114,7 @@ The webhook got a response with status code *400*.''')
@mock.patch('zerver.lib.outgoing_webhook.fail_with_message')
def test_request_exception(self, mock_fail_with_message: mock.Mock,
mock_requests_request: mock.Mock, mock_logger: mock.Mock) -> None:
do_rest_call(self.rest_operation, None, self.mock_event, service_handler, None)
do_rest_call('', self.rest_operation, None, self.mock_event, service_handler, None)
bot_owner_notification = self.get_last_message()
self.assertTrue(mock_fail_with_message.called)
self.assertEqual(bot_owner_notification.content,

6
zerver/worker/queue_processors.py
View File

@@ -483,7 +483,11 @@ class OutgoingWebhookWorker(QueueProcessingWorker):
service_handler = get_outgoing_webhook_service_handler(service)
rest_operation, request_data = service_handler.process_event(dup_event)
if rest_operation:
do_rest_call(rest_operation, request_data, dup_event, service_handler)
do_rest_call(service.base_url,
rest_operation,
request_data,
dup_event,
service_handler)
@assign_queue('embedded_bots')
class EmbeddedBotWorker(QueueProcessingWorker):