[FIX] Handle null response body from delete methods (#37)

* [FIX] Handle null response body from delete methods
* Set response_json to empty dict if no response body
* Return None from RequestPaginator._call if no response body
* Return None instead of API object if no response body

* [ADD] Add test

Author: hughesbm

helpscout/base_api.py

@@ -71,10 +71,10 @@ def __new__(cls, endpoint, data=None,
         )
         if singleton:
             results = paginator.call(paginator.data)
-            result_length = len(results)
-            if result_length == 0:
+            try:
+                return out_type.from_api(**results[0])
+            except (IndexError, TypeError):
                 return None
-            return out_type.from_api(**results[0])
         obj = super(BaseApi, cls).__new__(cls)
         obj.paginator = paginator
         return obj

helpscout/request_paginator/__init__.py

@@ -143,7 +143,7 @@ def _call(self, method, *args, **kwargs):
             kwargs['verify'] = self.SSL_VERIFY
 
         response = method(*args, **kwargs)
-        response_json = response.json()
+        response_json = response.text and response.json() or {}
 
         if response.status_code < 200 or response.status_code >= 300:
             message = response_json.get('error', response_json.get('message'))
@@ -162,4 +162,4 @@ def _call(self, method, *args, **kwargs):
         except KeyError:
             pass
 
-        return True
+        return None

helpscout/tests/test_base_api.py

@@ -61,12 +61,19 @@ def test_new_paginator_singleton(self, paginator):
         self.assertEqual(res.id, 9876)
 
     @mock.patch(PAGINATOR)
-    def test_new_paginator_singleton_none(self, paginator):
+    def test_new_paginator_singleton_not_found(self, paginator):
         """It should return None if singleton and not found."""
         paginator().call.return_value = []
         res = self.new_api(singleton=True)
         self.assertIs(res, None)
 
+    @mock.patch(PAGINATOR)
+    def test_new_paginator_singleton_none(self, paginator):
+        """It should return None if singleton and return value is None."""
+        paginator().call.return_value = None
+        res = self.new_api(singleton=True)
+        self.assertIs(res, None)
+
     @mock.patch(PAGINATOR)
     def test_base_api_iterates_paginator(self, paginator):
         """It should pass iteration to the paginator."""

helpscout/tests/test_request_paginator.py

@@ -49,8 +49,6 @@ def setUp(self):
     @contextmanager
     def mock_session(self, response_code=200, responses=None,
                      mock_attrs=None):
-        if responses is None:
-            responses = self.test_responses
         if mock_attrs is None:
             mock_attrs = ['get', 'delete', 'post', 'put']
         elif isinstance(mock_attrs, str):
@@ -59,14 +57,16 @@ def mock_session(self, response_code=200, responses=None,
             response = mock.MagicMock()
             response.status_code = response_code
             response.json.side_effect = responses
+            response.json.return_value = responses
             for attr in mock_attrs:
                 method = getattr(session, attr)
                 method.return_value = response
             yield session
 
-    def do_call(self, request_type='get'):
+    def do_call(self, request_type='get', responses=None):
         self.params = {'param_test': 23234}
-        with self.mock_session(mock_attrs=request_type) as session:
+        with self.mock_session(mock_attrs=request_type,
+                               responses=responses) as session:
             return session, getattr(self.paginator, request_type)(self.params)
 
     def _test_result(self, res):
@@ -96,7 +96,7 @@ def test_init_session(self, requests):
 
     def test_get_gets(self):
         """ It should call the session with proper args. """
-        session, _ = self.do_call()
+        session, _ = self.do_call(responses=self.test_responses)
         session.get.assert_called_once_with(
             url=self.vals['endpoint'],
             params=self.params,
@@ -106,14 +106,19 @@ def test_get_gets(self):
     def test_returns(self):
         """The returns should all return properly."""
         for request_type in self.REQUEST_TYPES:
-            _, res = self.do_call(request_type)
+            _, res = self.do_call(request_type, self.test_responses)
             self._test_result(res)
 
+    def test_delete_return_null_response_body(self):
+        """The delete return should be None if response body is null."""
+        _, res = self.do_call('delete', None)
+        self.assertEqual(res, None)
+
     def test_session_calls(self):
         """The calls should all be handled properly (tests all but GET)."""
         self.REQUEST_TYPES.remove('get')
         for request_type in self.REQUEST_TYPES:
-            session, _ = self.do_call(request_type)
+            session, _ = self.do_call(request_type, self.test_responses)
             self._test_session_call_json(session, request_type)
 
     def test_call_get(self):