mirror of https://github.com/zulip/zulip.git
Outgoing Webhook System: Add usage of Interfaces in DoRestCall.
This commit is contained in:
parent
a60552930e
commit
7f9b4d3a94
|
@ -100,8 +100,8 @@ def request_retry(event, failure_message):
|
|||
else:
|
||||
queue_json_publish("outgoing_webhooks", event, lambda x: None)
|
||||
|
||||
def do_rest_call(rest_operation, event, timeout=None):
|
||||
# type: (Dict[str, Any], Dict[str, Any], Any) -> None
|
||||
def do_rest_call(rest_operation, request_data, event, service_handler, timeout=None):
|
||||
# type: (Dict[str, Any], Dict[str, Any], Dict[str, Any], Any, Any) -> None
|
||||
rest_operation_validator = check_dict([
|
||||
('method', check_string),
|
||||
('relative_url_path', check_string),
|
||||
|
@ -119,16 +119,17 @@ def do_rest_call(rest_operation, event, timeout=None):
|
|||
request_kwargs['timeout'] = timeout
|
||||
|
||||
try:
|
||||
# TODO: Add comment describing structure of data being sent to third party URL.
|
||||
response = requests.request(http_method, final_url, data=json.dumps(event), **request_kwargs)
|
||||
response = requests.request(http_method, final_url, data=json.dumps(request_data), **request_kwargs)
|
||||
if str(response.status_code).startswith('2'):
|
||||
succeed_with_message(event, "received response: `" + str(response.content) + "`.")
|
||||
response_data = service_handler.process_success(response, event)
|
||||
succeed_with_message(event, response_data["response_message"])
|
||||
|
||||
# On 50x errors, try retry
|
||||
elif str(response.status_code).startswith('5'):
|
||||
request_retry(event, "unable to connect with the third party.")
|
||||
else:
|
||||
fail_with_message(event, "unable to communicate with the third party.")
|
||||
response_data = service_handler.process_failure(response, event)
|
||||
fail_with_message(event, response_data["response_message"])
|
||||
|
||||
except requests.exceptions.Timeout:
|
||||
logging.info("Trigger event %s on %s timed out. Retrying" % (event["command"], event['service_name']))
|
||||
|
|
|
@ -2,7 +2,7 @@ from __future__ import absolute_import
|
|||
from typing import Any, Dict, Text
|
||||
|
||||
from zerver.outgoing_webhooks.generic import GenericOutgoingWebhookService
|
||||
from zerver.models import GENERIC_INTERFACE
|
||||
from zerver.models import GENERIC_INTERFACE, Service
|
||||
|
||||
AVAILABLE_OUTGOING_WEBHOOK_INTERFACES = {
|
||||
GENERIC_INTERFACE: GenericOutgoingWebhookService
|
||||
|
@ -14,3 +14,13 @@ def get_service_interface_class(interface):
|
|||
return AVAILABLE_OUTGOING_WEBHOOK_INTERFACES[GENERIC_INTERFACE]
|
||||
else:
|
||||
return AVAILABLE_OUTGOING_WEBHOOK_INTERFACES[interface]
|
||||
|
||||
def get_outgoing_webhook_service_handler(service):
|
||||
# type: (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,
|
||||
user_profile=service.user_profile,
|
||||
service_name=service.name)
|
||||
return service_interface
|
||||
|
|
|
@ -4,10 +4,12 @@ from __future__ import print_function
|
|||
|
||||
import mock
|
||||
import requests
|
||||
from typing import Any
|
||||
from typing import Any, Dict, Tuple, Text
|
||||
from requests import Response
|
||||
|
||||
from zerver.lib.outgoing_webhook import do_rest_call
|
||||
from zerver.lib.outgoing_webhook import do_rest_call, OutgoingWebhookServiceInterface
|
||||
from zerver.lib.test_classes import ZulipTestCase
|
||||
from builtins import object
|
||||
|
||||
rest_operation = {'method': "POST",
|
||||
'relative_url_path': "",
|
||||
|
@ -29,13 +31,24 @@ def timeout_error(http_method, final_url, data, **request_kwargs):
|
|||
# type: (Any, Any, Any, Any) -> Any
|
||||
raise requests.exceptions.Timeout
|
||||
|
||||
class MockServiceHandler(OutgoingWebhookServiceInterface):
|
||||
def process_success(self, response, event):
|
||||
# type: (Response, Dict[Text, Any]) -> Dict[str, Any]
|
||||
return {"response_message": ""}
|
||||
|
||||
def process_failure(self, response, event):
|
||||
# type: (Response, Dict[Text, Any]) -> Dict[str, Any]
|
||||
return {"response_message": ""}
|
||||
|
||||
service_handler = MockServiceHandler(None, None, None, None)
|
||||
|
||||
class DoRestCallTests(ZulipTestCase):
|
||||
@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) # type: ignore
|
||||
do_rest_call(rest_operation, None, None, service_handler, None)
|
||||
self.assertTrue(mock_succeed_with_message.called)
|
||||
|
||||
@mock.patch('zerver.lib.outgoing_webhook.request_retry')
|
||||
|
@ -43,7 +56,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, None) # type: ignore
|
||||
do_rest_call(rest_operation, None, None, service_handler, None)
|
||||
self.assertTrue(mock_request_retry.called)
|
||||
|
||||
@mock.patch('zerver.lib.outgoing_webhook.fail_with_message')
|
||||
|
@ -51,7 +64,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, None) # type: ignore
|
||||
do_rest_call(rest_operation, None, None, service_handler, None)
|
||||
self.assertTrue(mock_fail_with_message.called)
|
||||
|
||||
@mock.patch('logging.info')
|
||||
|
@ -59,7 +72,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, {"command": "", "service_name": ""}, None)
|
||||
do_rest_call(rest_operation, None, {"command": "", "service_name": ""}, service_handler, None)
|
||||
self.assertTrue(mock_request_retry.called)
|
||||
|
||||
@mock.patch('logging.exception')
|
||||
|
@ -67,5 +80,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, {"command": ""}, None)
|
||||
do_rest_call(rest_operation, None, {"command": ""}, service_handler, None)
|
||||
self.assertTrue(mock_fail_with_message.called)
|
||||
|
|
|
@ -40,6 +40,7 @@ from zerver.lib.outgoing_webhook import do_rest_call
|
|||
from zerver.models import get_bot_services
|
||||
from zulip import Client
|
||||
from zerver.lib.bot_lib import EmbeddedBotHandler
|
||||
from zerver.outgoing_webhooks import get_outgoing_webhook_service_handler
|
||||
|
||||
import os
|
||||
import sys
|
||||
|
@ -438,19 +439,16 @@ class OutgoingWebhookWorker(QueueProcessingWorker):
|
|||
def consume(self, event):
|
||||
# type: (Mapping[str, Any]) -> None
|
||||
message = event['message']
|
||||
services = get_bot_services(event['user_profile_id'])
|
||||
rest_operation = {'method': 'POST',
|
||||
'relative_url_path': '',
|
||||
'request_kwargs': {},
|
||||
'base_url': ''}
|
||||
|
||||
dup_event = cast(Dict[str, Any], event)
|
||||
dup_event['command'] = message['content']
|
||||
|
||||
services = get_bot_services(event['user_profile_id'])
|
||||
for service in services:
|
||||
rest_operation['base_url'] = str(service.base_url)
|
||||
dup_event['service_name'] = str(service.name)
|
||||
do_rest_call(rest_operation, dup_event)
|
||||
dup_event['base_url'] = str(service.base_url)
|
||||
service_handler = get_outgoing_webhook_service_handler(service)
|
||||
rest_operation, request_data = service_handler.process_event(dup_event)
|
||||
do_rest_call(rest_operation, request_data, dup_event, service_handler)
|
||||
|
||||
@assign_queue('embedded_bots')
|
||||
class EmbeddedBotWorker(QueueProcessingWorker):
|
||||
|
|
Loading…
Reference in New Issue