Deletion related APIs should be executed with "raw=True"

[xiaowl]

According to the documentation, deletion API will return a "empty 204 response", thus without raw=True, response.json() from client._request raises exception.

JSONDecodeError: No JSON object could be decoded: line 1 column 0 (char 0)

The delete_folder API does issue a raw=True request, but delete_file doesn't.

def delete_file(self, file_id, etag=None):
      """
      Discards a file to the trash.

      Args:
           - etag: (optional) If specified, the file will only be deleted if
                its etag matches the parameter
      """

      headers = {}
      if etag:
          headers['If-Match'] = etag

      self._request("delete", 'files/{}'.format(file_id), headers=headers)

I'd like to fix this, actually I've cloned this repo. But I want to do more. So I look into the test cases, found that mocked_response might "mocked" too much, I mean it just assume we always can get a valid response from the API server, when there is no content available, we assume None returned. But that's not the real case. Empty content is not accepted by both json or simplejson.

So I patched a little

 diff --git a/tests/__init__.py b/tests/__init__.py
 index eb87755..ab2f2ce 100644
 --- a/tests/__init__.py
 +++ b/tests/__init__.py
 @@ -1,5 +1,9 @@
  from StringIO import StringIO
  from datetime import tzinfo, timedelta, datetime
 +try:
 +    import simplejson as json
 +except:
 +    import json as json

  from flexmock import flexmock

 @@ -45,4 +49,9 @@ utc = UTC()

  def mocked_response(content=None, status_code=200, headers=None):
 -    return flexmock(ok=status_code < 400, status_code=status_code, json=lambda: content, raw=content, text=content, headers=headers)
 +    if content is None:
 +        content = ''
 +    if not isinstance(content, basestring):
 +        content = json.dumps(content)
 +
 +    return flexmock(ok=status_code < 400, status_code=status_code, json=lambda: json.loads(content), raw=content, text=content, headers=headers)

Of cause some cases are broken, and some of them should be. But some broken ones like

def test_get(self):
    client = self.make_client("get", "foo", params={'arg': 'value'}, crap=1)
    client._request('get', 'foo', {'arg': 'value'}, crap=1)

def test_post_dict(self):
    expected_data = {'arg': 'value'}
    client = self.make_client("post", "foo", data=expected_data, crap=1)
    actual_response = client._request('post', 'foo', data=expected_data, crap=1)
    self.assertEqual(None, actual_response)

def test_post_data(self):
    expected_data = "mooooo"
    client = self.make_client("post", "foo", data=expected_data, crap=1)
    actual_response = client._request('post', 'foo', data=expected_data, crap=1)
    self.assertEqual(None, actual_response)

def test_put_dict(self):
    expected_data = {'arg': 'value'}
    client = self.make_client("put", "foo", data=expected_data, crap=1)
    actual_response = client._request('put', 'foo', data=expected_data, crap=1)
    self.assertEqual(None, actual_response)

I just don't get why we need these cases, which issuing non-exist API path "foo". It won't be hard to just look through the source code and patch raw=True if needed, but I think we better make these test cases more robust.

Which way should we take to fix this problem? Simply add raw=True?

[tals]

You are totally right, and your patch looks good.

I think the test_post_xxx/test_get_xxx got degraded a bit in v1.2 (had some refactoring done). It needs fixing.

I think I should just kill this raw crap and instead always return the regular response. Caller will just have to do .json() when they want it.

[tals]

Would you like to submit a pull request? :)

[xiaowl]

Of cause, I'd like to. I will patch the mocked_response and fix the broken tests.