Correctly handle upload responses (#219)

Upload responses look like {"entries": [{"type": "file", ...}]},
not {"type": "file", ...}. However, the code for uploading a new
file version was incorrectly assuming the latter, causing it to
return File({"entries": [{"type": "file", ...}]}) instead of
File({"type": "file", ...}).

Now we will expand the "entries", and grab only the file object.

We'll also be future-proof, in case a future version of the API
starts returning {"type": "file", ...}.

Fixes #150.

Author: jmoldow

HISTORY.rst

@@ -31,6 +31,12 @@ Release History
   automatically backwards-compatibile (as long as there aren't any changes to
   change/remove any of the keywords).
 
+- ``File.update_contents()`` and ``File.update_contents_with_stream()`` now
+  correctly return a ``File`` object with the correct internal JSON structure.
+  Previously it would return a ``File`` object where the file JSON is hidden
+  inside ``file['entries'][0]``. This is a bugfix, but will be a breaking
+  change for any clients that have already written code to handle the bug.
+
 **Features**
 
 - Added more flexibility to the object translation system:

boxsdk/object/file.py

@@ -130,10 +130,13 @@ def update_contents_with_stream(
 
         files = {'file': ('unused', file_stream)}
         headers = {'If-Match': etag} if etag is not None else None
+        file_response = self._session.post(url, expect_json_response=False, files=files, headers=headers).json()
+        if 'entries' in file_response:
+            file_response = file_response['entries'][0]
         return self.__class__(
             session=self._session,
             object_id=self._object_id,
-            response_object=self._session.post(url, expect_json_response=False, files=files, headers=headers).json(),
+            response_object=file_response,
         )
 
     @api_call

boxsdk/object/folder.py

@@ -279,8 +279,9 @@ def upload_stream(
         files = {
             'file': ('unused', file_stream),
         }
-        box_response = self._session.post(url, data=data, files=files, expect_json_response=False)
-        file_response = box_response.json()['entries'][0]
+        file_response = self._session.post(url, data=data, files=files, expect_json_response=False).json()
+        if 'entries' in file_response:
+            file_response = file_response['entries'][0]
         file_id = file_response['id']
         return self.translator.translate(file_response['type'])(
             session=self._session,

test/functional/mock_box/behavior/file_behavior.py

@@ -37,7 +37,7 @@ def update_file(self, file_id):
         file_object.size = len(content)
         self._db_session.add(EventModel(event_type='ITEM_UPLOAD', source_id=file_object.file_id, source_type='file'))
         self._db_session.commit()
-        return json.dumps(file_object)
+        return json.dumps({'entries': [file_object]})
 
     def upload_file(self):
         """

test/unit/object/conftest.py

@@ -40,11 +40,22 @@ def mock_content_response(make_mock_box_request):
     return mock_box_response
 
 
+@pytest.fixture(scope='function', params=[False, True])
+def mock_upload_response_contains_entries(request):
+    """Is the upload response formatted as {"type": "file", "id": "123", ...}, or as {"entries": [{...}]}.
+
+    The v2.0 API does the latter, but future versions might do the former. So
+    we'll test both.
+    """
+    return request.param
+
+
 @pytest.fixture(scope='function')
-def mock_upload_response(mock_object_id, make_mock_box_request):
-    mock_box_response, _ = make_mock_box_request(
-        response={'entries': [{'type': 'file', 'id': mock_object_id}]},
-    )
+def mock_upload_response(mock_object_id, make_mock_box_request, mock_upload_response_contains_entries):
+    response = {'type': 'file', 'id': mock_object_id}
+    if mock_upload_response_contains_entries:
+        response = {'entries': [response]}
+    mock_box_response, _ = make_mock_box_request(response=response)
     return mock_box_response
 
 

test/unit/object/test_file.py

@@ -108,7 +108,11 @@ def test_update_contents(
         headers=if_match_header,
     )
     assert isinstance(new_file, File)
-    assert new_file.object_id == mock_upload_response.json()['entries'][0]['id']
+    assert new_file.object_id == test_file.object_id
+    assert 'id' in new_file
+    assert new_file['id'] == test_file.object_id
+    assert not hasattr(new_file, 'entries')
+    assert 'entries' not in new_file
 
 
 def test_update_contents_with_stream_does_preflight_check_if_specified(

test/unit/object/test_folder.py

@@ -195,7 +195,11 @@ def test_upload(
     data = {'attributes': json.dumps({'name': basename(mock_file_path), 'parent': {'id': mock_object_id}})}
     mock_box_session.post.assert_called_once_with(expected_url, expect_json_response=False, files=mock_files, data=data)
     assert isinstance(new_file, File)
-    assert new_file.object_id == mock_upload_response.json()['entries'][0]['id']
+    assert new_file.object_id == mock_object_id
+    assert 'id' in new_file
+    assert new_file['id'] == mock_object_id
+    assert not hasattr(new_file, 'entries')
+    assert 'entries' not in new_file
 
 
 def test_upload_stream_does_preflight_check_if_specified(