Don't dump the response body before dumping the whole response

sartography · Feb 20, 2024 · a0d332f · a0d332f
1 parent ac885dc
commit a0d332f
Show file tree

Hide file tree

Showing 7 changed files with 42 additions and 23 deletions.
diff --git a/src/connector_http/http_request_base.py b/src/connector_http/http_request_base.py
@@ -104,13 +104,14 @@ def log(msg: str) -> None:
 
         if http_response is not None:
             command_response = {"raw_response": http_response.text}
+            content_type = http_response.headers.get("Content-Type", "")
             # this string can include modifiers like UTF-8, which is why it's not using ==
-            if "application/json" in http_response.headers.get("Content-Type", ""):
+            if "application/json" in content_type:
                 try:
                     command_response = json.loads(http_response.text)
                 except Exception as e:
                     error = self._create_error_from_exception(exception=e, http_response=http_response)
-            elif "application/xml" in http_response.headers.get("Content-Type", ""):
+            elif "application/xml" in content_type or "text/xml" in content_type:
                 try:
                     command_response = xmltodict.parse(http_response.text)
                 except Exception as e:
@@ -121,7 +122,7 @@ def log(msg: str) -> None:
             error = self._create_error(error_code=f"HttpError{status}", http_response=http_response)
 
         return_response: CommandResponseDict = {
-            "body": json.dumps(command_response),
+            "body": command_response,
             "mimetype": mimetype,
             "http_status": status,
         }

diff --git a/tests/connector_http/unit/test_delete_request_v2.py b/tests/connector_http/unit/test_delete_request_v2.py
@@ -16,7 +16,7 @@ def test_delete_html_from_url(self) -> None:
             response = request.execute(None, {})
             assert mock_request.call_count == 1
 
-        assert response["command_response"]["body"] == json.dumps({"raw_response": return_html})
+        assert response["command_response"]["body"] == {"raw_response": return_html}
         assert response["command_response"]["http_status"] == 200
         assert response["command_response"]["mimetype"] == "application/json"
         assert response["error"] is None
@@ -35,7 +35,7 @@ def test_delete_json_from_url(self) -> None:
             assert mock_request.call_count == 1
 
         assert response is not None
-        assert response["command_response"]["body"] == json.dumps(return_json)
+        assert response["command_response"]["body"] == return_json
         assert response["command_response"]["http_status"] == 200
         assert response["command_response"]["mimetype"] == "application/json"
         assert response["error"] is None
@@ -53,7 +53,7 @@ def test_delete_can_handle_500(self, sleepless: Any) -> None:
             assert mock_request.call_count == 1
 
         assert response is not None
-        assert response["command_response"]["body"] == json.dumps(return_json)
+        assert response["command_response"]["body"] == return_json
         assert response["command_response"]["http_status"] == 500
         assert response["command_response"]["mimetype"] == "application/json"
         assert response["error"] is not None

diff --git a/tests/connector_http/unit/test_get_request_v2.py b/tests/connector_http/unit/test_get_request_v2.py
@@ -16,7 +16,7 @@ def test_get_html_from_url(self) -> None:
             response = request.execute(None, {})
 
         assert response is not None
-        assert response["command_response"]["body"] == json.dumps({"raw_response": return_html})
+        assert response["command_response"]["body"] == {"raw_response": return_html}
         assert response["command_response"]["http_status"] == 200
         assert response["command_response"]["mimetype"] == "application/json"
         assert response["error"] is None
@@ -34,14 +34,14 @@ def test_get_json_from_url(self) -> None:
             response = request.execute(None, {})
 
         assert response is not None
-        assert response["command_response"]["body"] == json.dumps(return_json)
+        assert response["command_response"]["body"] == return_json
         assert response["command_response"]["http_status"] == 200
         assert response["command_response"]["mimetype"] == "application/json"
         assert response["error"] is None
         assert response["spiff__logs"] is not None
         assert len(response["spiff__logs"]) > 0
 
-    def test_get_xml_from_url(self) -> None:
+    def test_get_application_xml_from_url(self) -> None:
         request = GetRequestV2(url="http://example.com")
         return_xml = "<hey>we_return</hey>"
         with patch("requests.get") as mock_request:
@@ -52,7 +52,25 @@ def test_get_xml_from_url(self) -> None:
             response = request.execute(None, {})
 
         assert response is not None
-        assert response["command_response"]["body"] == json.dumps({"hey": "we_return"})
+        assert response["command_response"]["body"] == {"hey": "we_return"}
+        assert response["command_response"]["http_status"] == 200
+        assert response["command_response"]["mimetype"] == "application/json"
+        assert response["error"] is None
+        assert response["spiff__logs"] is not None
+        assert len(response["spiff__logs"]) > 0
+
+    def test_get_text_xml_from_url(self) -> None:
+        request = GetRequestV2(url="http://example.com")
+        return_xml = "<hey>we_return</hey>"
+        with patch("requests.get") as mock_request:
+            mock_request.return_value.status_code = 200
+            mock_request.return_value.ok = True
+            mock_request.return_value.headers = {"Content-Type": "text/xml"}
+            mock_request.return_value.text = return_xml
+            response = request.execute(None, {})
+
+        assert response is not None
+        assert response["command_response"]["body"] == {"hey": "we_return"}
         assert response["command_response"]["http_status"] == 200
         assert response["command_response"]["mimetype"] == "application/json"
         assert response["error"] is None
@@ -70,7 +88,7 @@ def test_get_can_handle_500(self, sleepless: Any) -> None:
             assert mock_request.call_count == 3
 
         assert response is not None
-        assert response["command_response"]["body"] == json.dumps(return_json)
+        assert response["command_response"]["body"] == return_json
         assert response["command_response"]["http_status"] == 500
         assert response["command_response"]["mimetype"] == "application/json"
         assert response["error"] is not None

diff --git a/tests/connector_http/unit/test_head_request_v2.py b/tests/connector_http/unit/test_head_request_v2.py
@@ -16,7 +16,7 @@ def test_head_html_from_url(self) -> None:
             response = request.execute(None, {})
 
         assert response is not None
-        assert response["command_response"]["body"] == json.dumps({"raw_response": return_html})
+        assert response["command_response"]["body"] == {"raw_response": return_html}
         assert response["command_response"]["http_status"] == 200
         assert response["command_response"]["mimetype"] == "application/json"
         assert response["error"] is None
@@ -34,7 +34,7 @@ def test_head_json_from_url(self) -> None:
             response = request.execute(None, {})
 
         assert response is not None
-        assert response["command_response"]["body"] == json.dumps(return_json)
+        assert response["command_response"]["body"] == return_json
         assert response["command_response"]["http_status"] == 200
         assert response["command_response"]["mimetype"] == "application/json"
         assert response["error"] is None
@@ -52,7 +52,7 @@ def test_head_can_handle_500(self, sleepless: Any) -> None:
             assert mock_request.call_count == 3
 
         assert response is not None
-        assert response["command_response"]["body"] == json.dumps(return_json)
+        assert response["command_response"]["body"] == return_json
         assert response["command_response"]["http_status"] == 500
         assert response["command_response"]["mimetype"] == "application/json"
         assert response["error"] is not None

diff --git a/tests/connector_http/unit/test_patch_request_v2.py b/tests/connector_http/unit/test_patch_request_v2.py
@@ -16,7 +16,7 @@ def test_patch_html_from_url(self) -> None:
             response = request.execute(None, {})
             assert mock_request.call_count == 1
 
-        assert response["command_response"]["body"] == json.dumps({"raw_response": return_html})
+        assert response["command_response"]["body"] == {"raw_response": return_html}
         assert response["command_response"]["http_status"] == 200
         assert response["command_response"]["mimetype"] == "application/json"
         assert response["error"] is None
@@ -35,7 +35,7 @@ def test_patch_json_from_url(self) -> None:
             assert mock_request.call_count == 1
 
         assert response is not None
-        assert response["command_response"]["body"] == json.dumps(return_json)
+        assert response["command_response"]["body"] == return_json
         assert response["command_response"]["http_status"] == 200
         assert response["command_response"]["mimetype"] == "application/json"
         assert response["error"] is None
@@ -53,7 +53,7 @@ def test_patch_can_handle_500(self, sleepless: Any) -> None:
             assert mock_request.call_count == 1
 
         assert response is not None
-        assert response["command_response"]["body"] == json.dumps(return_json)
+        assert response["command_response"]["body"] == return_json
         assert response["command_response"]["http_status"] == 500
         assert response["command_response"]["mimetype"] == "application/json"
         assert response["error"] is not None

diff --git a/tests/connector_http/unit/test_post_request_v2.py b/tests/connector_http/unit/test_post_request_v2.py
@@ -16,7 +16,7 @@ def test_post_html_from_url(self) -> None:
             response = request.execute(None, {})
             assert mock_request.call_count == 1
 
-        assert response["command_response"]["body"] == json.dumps({"raw_response": return_html})
+        assert response["command_response"]["body"] == {"raw_response": return_html}
         assert response["command_response"]["http_status"] == 200
         assert response["command_response"]["mimetype"] == "application/json"
         assert response["error"] is None
@@ -35,7 +35,7 @@ def test_post_json_from_url(self) -> None:
             assert mock_request.call_count == 1
 
         assert response is not None
-        assert response["command_response"]["body"] == json.dumps(return_json)
+        assert response["command_response"]["body"] == return_json
         assert response["command_response"]["http_status"] == 200
         assert response["command_response"]["mimetype"] == "application/json"
         assert response["error"] is None
@@ -53,7 +53,7 @@ def test_post_can_handle_500(self, sleepless: Any) -> None:
             assert mock_request.call_count == 1
 
         assert response is not None
-        assert response["command_response"]["body"] == json.dumps(return_json)
+        assert response["command_response"]["body"] == return_json
         assert response["command_response"]["http_status"] == 500
         assert response["command_response"]["mimetype"] == "application/json"
         assert response["error"] is not None

diff --git a/tests/connector_http/unit/test_put_request_v2.py b/tests/connector_http/unit/test_put_request_v2.py
@@ -16,7 +16,7 @@ def test_put_html_from_url(self) -> None:
             response = request.execute(None, {})
             assert mock_request.call_count == 1
 
-        assert response["command_response"]["body"] == json.dumps({"raw_response": return_html})
+        assert response["command_response"]["body"] == {"raw_response": return_html}
         assert response["command_response"]["http_status"] == 200
         assert response["command_response"]["mimetype"] == "application/json"
         assert response["error"] is None
@@ -35,7 +35,7 @@ def test_put_json_from_url(self) -> None:
             assert mock_request.call_count == 1
 
         assert response is not None
-        assert response["command_response"]["body"] == json.dumps(return_json)
+        assert response["command_response"]["body"] == return_json
         assert response["command_response"]["http_status"] == 200
         assert response["command_response"]["mimetype"] == "application/json"
         assert response["error"] is None
@@ -53,7 +53,7 @@ def test_put_can_handle_500(self, sleepless: Any) -> None:
             assert mock_request.call_count == 1
 
         assert response is not None
-        assert response["command_response"]["body"] == json.dumps(return_json)
+        assert response["command_response"]["body"] == return_json
         assert response["command_response"]["http_status"] == 500
         assert response["command_response"]["mimetype"] == "application/json"
         assert response["error"] is not None