Merge pull request #16 from homiedopie/hotfix/INV-1894

(INV-1984) - Circular reference issue with WSGIRequest JSON serialization

Author: homiedopie

requirements.txt

@@ -1,6 +1,6 @@
 flake8
 mock==2.0.0
-protobuf==3.9.1
+protobuf==3.15.0
 pytest==4.3.0
 pytest-cov==2.6.1
 requests==2.21.0

stackify/transport/default/log.py

@@ -2,6 +2,7 @@
 from stackify.transport.default.formats import JSONObject
 from stackify.transport.default.error import StackifyError
 from stackify.utils import data_to_json
+from stackify.utils import extract_request
 
 
 class LogMsg(JSONObject):
@@ -31,6 +32,8 @@ def from_record(self, record):
         data = {k: v for k, v in record.__dict__.items()
                 if k not in RECORD_VARS}
 
+        data = extract_request(data)
+
         if data:
             self.data = data_to_json(data)
 

stackify/utils.py

@@ -23,16 +23,41 @@ def arg_or_env(name, args, default=None, env_key=None):
 
 def data_to_json(data):
     try:
-        if object_is_iterable(data) and 'request' in data and hasattr(data['request'], '_messages'):
-            data['request'] = get_default_object(data['request'])
-
         return json.dumps(data, default=lambda x: get_default_object(x))
-    except ValueError as e:
-        internal_logger.exception('Failed to serialize object to json: {} - Exception: {}'.format(data.__str__(), str(e)))
-        return json.dumps(data.__str__())  # String representation of the object
+    except Exception as e:
+        internal_logger.exception('Failed to serialize object to json: {} - Exception: {}'.format(str(data), str(e)))
+        return str(data)  # String representation of the object
+
+
+def extract_request(data):
+    if 'request' in data and "WSGIRequest" in str(data['request']):
+        new_request = {}
+        obj = data['request']
+
+        if hasattr(obj, 'path'):
+            new_request['path'] = obj.path
+
+        if hasattr(obj, 'method'):
+            new_request['method'] = obj.method
+
+        if hasattr(obj, 'POST'):
+            new_request['form'] = obj.POST
+
+        if hasattr(obj, 'GET'):
+            new_request['query'] = obj.GET
+
+        if hasattr(obj, 'content_type'):
+            new_request['content_type'] = obj.content_type
+
+        data['request'] = new_request
+
+    return data
 
 
 def get_default_object(obj):
+    if object_is_iterable(obj):
+        return [item for item in obj]
+
     return hasattr(obj, '__dict__') and obj.__dict__ or obj.__str__()
 
 

test.sh

@@ -22,7 +22,14 @@ function runPyTest() {
     virtualenv -p python${python_version} ${test_venv}
 
     echo "Activating virtualenv ${test_venv}..."
-    source ${test_venv}/bin/activate
+
+    if [ -f ${test_venv}/bin/activate ]; then
+        source ${test_venv}/bin/activate
+    fi
+
+    if [ -f ${test_venv}/Scripts/activate ]; then
+        source ${test_venv}/Scripts/activate
+    fi
 
     echo 'Installing dependencies...'
     pip install -r requirements.txt

tests/test_utils.py

@@ -33,6 +33,11 @@ def tearDown(self):
         for logger in self.loggers:
             del global_loggers[logger.name]
 
+    def test_utils_data_to_json_string(self):
+        dummy = 'test'
+        result = stackify.utils.data_to_json(dummy)
+        self.assertEqual('"test"', result)
+
     def test_utils_data_to_json_unserializable(self):
         dummy = Dummy()
         result = stackify.utils.data_to_json(dummy)
@@ -60,9 +65,9 @@ def test_utils_data_to_json_tuple(self):
         self.assertEqual(expected, result)
 
     def test_utils_data_to_json_dummy_iterable(self):
-        dummy = DummyInterable()
+        dummy = DummyIterable()
         result = stackify.utils.data_to_json(dummy)
-        expected = '{"a": 21}'
+        expected = '[1, 2, 3, 4, 5, 6, 7, 8, 9, 10]'
         self.assertEqual(expected, result)
 
     def test_utils_data_to_json_dummy_object_with_property(self):
@@ -82,17 +87,38 @@ def test_utils_data_to_json_dummy_object_circular(self, func):
         data['payload']['dummy'] = data
 
         result = stackify.utils.data_to_json(data)
-        func.assert_called()
 
-        substring = "'dummy': <tests.test_utils.DummyProperty object at"
+        substring = "'payload': {'dummy': "
+        data_str = str(data)
+
+        assert func.called
+        assert "Failed to serialize object to json: {}".format(data_str) in func.call_args_list[0][0][0]
+
         self.assertTrue(substring in result)
+        self.assertEqual(result, data_str)
 
     def test_utils_data_to_json_dummy_request(self):
         dummy = DummyRequest()
         result = stackify.utils.data_to_json(dummy)
         substring = '{"_messages": "<tests.test_utils.Dummy object at'
         self.assertTrue(substring in result)
 
+    def test_utils_extract_request_dummy_wsgi_request(self):
+        dummy = WSGIRequestMock()
+        dummy.path = 'test'
+        dummy.POST = 'test_form'
+        dummy.not_exists = 'test'
+        data = {
+            'request': dummy
+        }
+
+        result = stackify.utils.extract_request(data)
+        request = result['request']
+
+        self.assertEqual(request['path'], 'test')
+        self.assertEqual(request['form'], 'test_form')
+        self.assertTrue('not_exists' not in request)
+
 
 class RegexValidatorTest(ClearEnvTest):
 
@@ -120,13 +146,13 @@ class Dummy(object):
     pass
 
 
-class DummyInterable:
+class DummyIterable:
     def __iter__(self):
         self.a = 1
         return self
 
     def __next__(self):
-        if self.a <= 20:
+        if self.a <= 10:
             x = self.a
             self.a += 1
             return x
@@ -147,5 +173,10 @@ def __init__(self):
         self._messages = Dummy()
 
 
+class WSGIRequestMock:
+    def __init__(self):
+        pass
+
+
 if __name__ == '__main__':
     unittest.main()

tests/transport/agent/test_agent_socket.py

@@ -2,6 +2,7 @@
 import retrying
 from unittest import TestCase
 from mock import patch
+import os
 
 import stackify
 from tests.bases import fake_retry_decorator
@@ -40,7 +41,10 @@ def test_send_should_use_defaut_socket(self, mock_post):
         self.agent_socket.send(url, message)
 
         assert mock_post.called
-        assert mock_post.call_args_list[0][0][0] == 'http+unix://%2Fusr%2Flocal%2Fstackify%2Fstackify.sock/test_url'
+        if os.name == 'nt':
+            assert mock_post.call_args_list[0][0][0] == 'http+unix://%2Fusr%2Flocal%2Fstackify%2Fstackify.sock\\test_url'
+        else:
+            assert mock_post.call_args_list[0][0][0] == 'http+unix://%2Fusr%2Flocal%2Fstackify%2Fstackify.sock/test_url'
 
     @patch('requests_unixsocket.Session.post')
     def test_send_should_include_headers(self, mock_post):