From ef98211f68f60944bdf785c0c9b01a8a38c817e9 Mon Sep 17 00:00:00 2001 From: "Hemanth V. Alluri" Date: Thu, 16 May 2019 23:59:18 +0530 Subject: [PATCH] devtools: Add support for non-json fixtures for the integrations tool. Note: If you're going to send fixtures which are not JSON or of the text/plain content type, make sure you set the correct content type in the custom headers. E.g. For the wordpress fixtures the "Content-Type" should be set to "application/x-www-form-urlencoded". --- static/js/integrations_dev_panel.js | 33 +++-- .../integrations/development/dev_panel.html | 2 +- zerver/tests/test_integrations_dev_panel.py | 114 +++++++++++++++--- zerver/views/development/integrations.py | 35 +++--- 4 files changed, 135 insertions(+), 49 deletions(-) diff --git a/static/js/integrations_dev_panel.js b/static/js/integrations_dev_panel.js index 5656e08aea..f96388a08d 100644 --- a/static/js/integrations_dev_panel.js +++ b/static/js/integrations_dev_panel.js @@ -62,6 +62,11 @@ function get_selected_integration_name() { return $("#integration_name").children("option:selected").val(); } +function get_fixture_format(fixture_name) { + var pieces = fixture_name.split("."); + return pieces[pieces.length - 1]; +} + function get_custom_http_headers() { var custom_headers = $("#custom_http_headers").val(); if (custom_headers !== "") { @@ -80,13 +85,14 @@ function get_custom_http_headers() { function load_fixture_body(fixture_name) { /* Given a fixture name, use the loaded_fixtures dictionary to set the fixture body field. */ var integration_name = get_selected_integration_name(); - var element = loaded_fixtures[integration_name][fixture_name]; - var fixture_body = JSON.stringify(element, null, 4); // 4 is for the pretty print indent factor. - + var fixture_body = loaded_fixtures[integration_name][fixture_name]; if (fixture_body === undefined) { set_message("Fixture does not have a body.", "warning"); return; } + if (get_fixture_format(fixture_name) === "json") { + fixture_body = JSON.stringify(fixture_body, null, 4);// 4 is for pretty print . + } $("#fixture_body")[0].value = fixture_body; return; @@ -166,7 +172,7 @@ function get_fixtures(integration_name) { return; } - // We don't have the fixutures for this integration; fetch them using Zulip's channel library. + // We don't have the fixtures for this integration; fetch them using Zulip's channel library. // Relative url pattern: /devtools/integrations/(?P.+)/fixtures channel.get({ url: "/devtools/integrations/" + integration_name + "/fixtures", @@ -198,19 +204,24 @@ function send_webhook_fixture_message() { } var body = $("#fixture_body").val(); - try { - // Let JavaScript validate the JSON for us. - body = JSON.stringify(JSON.parse(body)); - } catch (err) { - set_message("Invalid JSON in fixture body.", "warning"); - return; + var fixture_name = $("#fixture_name").val(); + var is_json = false; + if (fixture_name && get_fixture_format(fixture_name) === "json") { + try { + // Let JavaScript validate the JSON for us. + body = JSON.stringify(JSON.parse(body)); + is_json = true; + } catch (err) { + set_message("Invalid JSON in fixture body.", "warning"); + return; + } } var custom_headers = get_custom_http_headers(); channel.post({ url: "/devtools/integrations/check_send_webhook_fixture_message", - data: {url: url, body: body, custom_headers: custom_headers}, + data: {url: url, body: body, custom_headers: custom_headers, is_json: is_json}, beforeSend: function (xhr) {xhr.setRequestHeader('X-CSRFToken', csrftoken);}, success: function () { // If the previous fixture body was sent successfully, then we should change the success diff --git a/templates/zerver/integrations/development/dev_panel.html b/templates/zerver/integrations/development/dev_panel.html index 743011d5fe..56d0963b63 100644 --- a/templates/zerver/integrations/development/dev_panel.html +++ b/templates/zerver/integrations/development/dev_panel.html @@ -75,7 +75,7 @@ - JSON Body + Fixture Body diff --git a/zerver/tests/test_integrations_dev_panel.py b/zerver/tests/test_integrations_dev_panel.py index 2a17abf574..a85d0979cb 100644 --- a/zerver/tests/test_integrations_dev_panel.py +++ b/zerver/tests/test_integrations_dev_panel.py @@ -17,6 +17,7 @@ class TestIntegrationsDevPanel(ZulipTestCase): "url": url, "body": body, "custom_headers": "", + "is_json": True } response = self.client_post(target_url, data) @@ -36,6 +37,7 @@ class TestIntegrationsDevPanel(ZulipTestCase): "url": url, "body": body, "custom_headers": "", + "is_json": True } response = self.client_post(target_url, data) @@ -58,6 +60,7 @@ class TestIntegrationsDevPanel(ZulipTestCase): "url": url, "body": body, "custom_headers": ujson.dumps({"X_GITHUB_EVENT": "ping"}), + "is_json": True } response = self.client_post(target_url, data) @@ -69,6 +72,29 @@ class TestIntegrationsDevPanel(ZulipTestCase): self.assertEqual(Stream.objects.get(id=latest_msg.recipient.type_id).name, "Denmark") self.assertEqual(latest_msg.topic_name(), "GitHub Notifications") + def test_check_send_webhook_fixture_message_for_success_with_headers_and_non_json_fixtures(self) -> None: + bot = get_user('webhook-bot@zulip.com', self.zulip_realm) + url = "/api/v1/external/wordpress?api_key={key}&stream=Denmark&topic=Wordpress Notifications".format(key=bot.api_key) + target_url = "/devtools/integrations/check_send_webhook_fixture_message" + with open("zerver/webhooks/wordpress/fixtures/publish_post_no_data_provided.txt", "r") as f: + body = f.read() + + data = { + "url": url, + "body": body, + "custom_headers": ujson.dumps({"Content-Type": "application/x-www-form-urlencoded"}), + "is_json": False, + } + + response = self.client_post(target_url, data) + self.assertEqual(response.status_code, 200) + + latest_msg = Message.objects.latest('id') + expected_message = "New post published:\n* [New WordPress Post](WordPress Post URL)" + self.assertEqual(latest_msg.content, expected_message) + self.assertEqual(Stream.objects.get(id=latest_msg.recipient.type_id).name, "Denmark") + self.assertEqual(latest_msg.topic_name(), "Wordpress Notifications") + def test_get_fixtures_for_nonexistant_integration(self) -> None: target_url = "/devtools/integrations/somerandomnonexistantintegration/fixtures" response = self.client_get(target_url) @@ -85,13 +111,6 @@ class TestIntegrationsDevPanel(ZulipTestCase): self.assertEqual(response.status_code, 404) self.assertEqual(ujson.loads(response.content), expected_response) - def test_get_fixtures_for_integration_without_json_fixtures(self) -> None: - target_url = "/devtools/integrations/deskdotcom/fixtures" - response = self.client_get(target_url) - expected_response = {'msg': 'The integration "deskdotcom" has non-JSON fixtures.', 'result': 'error'} - self.assertEqual(response.status_code, 400) - self.assertEqual(ujson.loads(response.content), expected_response) - def test_get_fixtures_for_success(self) -> None: target_url = "/devtools/integrations/airbrake/fixtures" response = self.client_get(target_url) @@ -148,6 +167,74 @@ class TestIntegrationsDevPanel(ZulipTestCase): self.assertEqual(Stream.objects.get(id=msg.recipient.type_id).name, "Denmark") self.assertEqual(msg.topic_name(), "Appfollow Bulk Notifications") + def test_send_all_webhook_fixture_messages_for_success_with_non_json_fixtures(self) -> None: + bot = get_user('webhook-bot@zulip.com', self.zulip_realm) + url = "/api/v1/external/wordpress?api_key={key}&stream=Denmark&topic=Wordpress Bulk Notifications".format(key=bot.api_key) + target_url = "/devtools/integrations/send_all_webhook_fixture_messages" + + data = { + "url": url, + "custom_headers": "", + "integration_name": "wordpress", + } + + response = self.client_post(target_url, data) + expected_responses = [ + { + "message": {'msg': 'Unknown WordPress webhook action: WordPress Action', 'result': 'error'}, + "fixture_name": "user_register.txt", + "status_code": 400 + }, + { + "message": {'msg': 'Unknown WordPress webhook action: WordPress Action', 'result': 'error'}, + "fixture_name": "publish_post_no_data_provided.txt", + "status_code": 400 + }, + { + "message": {'msg': 'Unknown WordPress webhook action: WordPress Action', 'result': 'error'}, + "fixture_name": "unknown_action_no_data.txt", + "status_code": 400 + }, + { + "message": {'msg': 'Unknown WordPress webhook action: WordPress Action', 'result': 'error'}, + "fixture_name": "publish_page.txt", + "status_code": 400 + }, + { + "message": {'msg': 'Unknown WordPress webhook action: WordPress Action', 'result': 'error'}, + "fixture_name": "unknown_action_no_hook_provided.txt", + "status_code": 400 + }, + { + "message": {'msg': 'Unknown WordPress webhook action: WordPress Action', 'result': 'error'}, + "fixture_name": "publish_post_type_not_provided.txt", + "status_code": 400 + }, + { + "message": {'msg': 'Unknown WordPress webhook action: WordPress Action', 'result': 'error'}, + "fixture_name": "wp_login.txt", + "status_code": 400 + }, + { + "message": {'msg': 'Unknown WordPress webhook action: WordPress Action', 'result': 'error'}, + "fixture_name": "publish_post.txt", + "status_code": 400 + } + ] + responses = ujson.loads(response.content)["responses"] + for r in responses: + r["message"] = ujson.loads(r["message"]) + self.assertEqual(response.status_code, 200) + for r in responses: + # We have to use this roundabout manner since the order may vary each time. This is not + # an issue. Basically, we're trying to compare 2 lists and since we're not resorting to + # using sets or a sorted order, we're sticking with O(n*m) time complexity for this + # comparision (where n and m are the lengths of the two lists respectively). But since + # this is just a unit test and more importantly n = m = some-low-number we don't really + # care about the time complexity being what it is. + self.assertTrue(r in expected_responses) + expected_responses.remove(r) + @patch("zerver.views.development.integrations.os.path.exists") def test_send_all_webhook_fixture_messages_for_missing_fixtures(self, os_path_exists_mock: MagicMock) -> None: os_path_exists_mock.return_value = False @@ -162,16 +249,3 @@ class TestIntegrationsDevPanel(ZulipTestCase): expected_response = {'msg': 'The integration "appfollow" does not have fixtures.', 'result': 'error'} self.assertEqual(response.status_code, 404) self.assertEqual(ujson.loads(response.content), expected_response) - - def test_send_all_webhook_fixture_messages_for_non_json_fixtures(self) -> None: - bot = get_user('webhook-bot@zulip.com', self.zulip_realm) - url = "/api/v1/external/appfollow?api_key={key}&stream=Denmark&topic=Desktdotcom Bulk Notifications".format(key=bot.api_key) - data = { - "url": url, - "custom_headers": "", - "integration_name": "deskdotcom", - } - response = self.client_post("/devtools/integrations/send_all_webhook_fixture_messages", data) - expected_response = {'msg': 'The integration "deskdotcom" has non-JSON fixtures.', 'result': 'error'} - self.assertEqual(response.status_code, 400) - self.assertEqual(ujson.loads(response.content), expected_response) diff --git a/zerver/views/development/integrations.py b/zerver/views/development/integrations.py index 87ea0b238e..e7fe96ed75 100644 --- a/zerver/views/development/integrations.py +++ b/zerver/views/development/integrations.py @@ -26,9 +26,9 @@ def dev_panel(request: HttpRequest) -> HttpResponse: context = {"integrations": integrations, "bots": bots} return render(request, "zerver/integrations/development/dev_panel.html", context) - def send_webhook_fixture_message(url: str=REQ(), body: str=REQ(), + is_json: bool=REQ(), custom_headers: str=REQ()) -> HttpResponse: client = Client() realm = get_realm("zulip") @@ -39,10 +39,12 @@ def send_webhook_fixture_message(url: str=REQ(), if not headers: headers = {} http_host = headers.pop("HTTP_HOST", realm.host) - content_type = headers.pop("HTTP_CONTENT_TYPE", "application/json") + if is_json: + content_type = headers.pop("HTTP_CONTENT_TYPE", "application/json") + else: + content_type = headers.pop("HTTP_CONTENT_TYPE", "text/plain") return client.post(url, body, content_type=content_type, HTTP_HOST=http_host, **headers) - @has_request_variables def get_fixtures(request: HttpResponse, integration_name: str=REQ()) -> HttpResponse: @@ -61,13 +63,12 @@ def get_fixtures(request: HttpResponse, for fixture in os.listdir(fixtures_dir): fixture_path = os.path.join(fixtures_dir, fixture) + content = open(fixture_path).read() try: - json_data = ujson.loads(open(fixture_path).read()) + content = ujson.loads(content) except ValueError: - msg = ("The integration \"{integration_name}\" has non-JSON fixtures.").format( - integration_name=integration_name) - return json_error(msg) - fixtures[fixture] = json_data + pass # The file extension will be used to determine the type. + fixtures[fixture] = content return json_success({"fixtures": fixtures}) @@ -76,8 +77,9 @@ def get_fixtures(request: HttpResponse, def check_send_webhook_fixture_message(request: HttpRequest, url: str=REQ(), body: str=REQ(), + is_json: bool=REQ(), custom_headers: str=REQ()) -> HttpResponse: - response = send_webhook_fixture_message(url, body, custom_headers) + response = send_webhook_fixture_message(url, body, is_json, custom_headers) if response.status_code == 200: return json_success() else: @@ -99,14 +101,13 @@ def send_all_webhook_fixture_messages(request: HttpRequest, responses = [] for fixture in os.listdir(fixtures_dir): fixture_path = os.path.join(fixtures_dir, fixture) - try: - # Just a quick check to make sure that the fixtures are of a valid JSON format. - json_data = ujson.dumps(ujson.loads(open(fixture_path).read())) - except ValueError: - msg = ("The integration \"{integration_name}\" has non-JSON fixtures.").format( - integration_name=integration_name) - return json_error(msg) - response = send_webhook_fixture_message(url, json_data, custom_headers) + content = open(fixture_path).read() + fixture_format = fixture.split(".")[-1] + if fixture_format == "json": + is_json = True + else: + is_json = False + response = send_webhook_fixture_message(url, content, is_json, custom_headers) responses.append({"status_code": response.status_code, "fixture_name": fixture, "message": response.content})