Addressing content-type and renaming Batch._futures.

NOTE: This commit made for review but will be folded into
      originals before merged.

Author: dhermes

gcloud/connection.py

@@ -309,13 +309,13 @@ def api_request(self, method, path, query_params=None,
         if not 200 <= response.status < 300:
             raise make_exception(response, content)
 
-        if isinstance(content, six.binary_type):
-            content = content.decode('utf-8')
-
-        if expect_json and content and isinstance(content, six.string_types):
+        string_or_bytes = (six.binary_type, six.text_type)
+        if content and expect_json and isinstance(content, string_or_bytes):
             content_type = response.get('content-type', '')
             if not content_type.startswith('application/json'):
                 raise TypeError('Expected JSON, got %s' % content_type)
+            if isinstance(content, six.binary_type):
+                content = content.decode('utf-8')
             return json.loads(content)
 
         return content

gcloud/storage/batch.py

@@ -136,7 +136,7 @@ def __init__(self, connection=None):
         super(Batch, self).__init__()
         self._connection = connection
         self._requests = []
-        self._futures = []
+        self._target_objects = []
 
     def _do_request(self, method, url, headers, data, target_object):
         """Override Connection:  defer actual HTTP request.
@@ -164,7 +164,7 @@ def _do_request(self, method, url, headers, data, target_object):
                              self._MAX_BATCH_SIZE)
         self._requests.append((method, url, headers, data))
         result = _FutureDict()
-        self._futures.append(target_object)
+        self._target_objects.append(target_object)
         if target_object is not None:
             target_object._properties = result
         return NoContent(), result
@@ -211,15 +211,16 @@ def _finish_futures(self, responses):
         # until all futures have been populated.
         exception_args = None
 
-        if len(self._futures) != len(responses):
+        if len(self._target_objects) != len(responses):
             raise ValueError('Expected a response for every request.')
 
-        for target_object, sub_response in zip(self._futures, responses):
+        for target_object, sub_response in zip(self._target_objects,
+                                               responses):
             resp_headers, sub_payload = sub_response
             if not 200 <= resp_headers.status < 300:
                 exception_args = exception_args or (resp_headers,
                                                     sub_payload)
-            if target_object is not None:
+            elif target_object is not None:
                 target_object._properties = sub_payload
 
         if exception_args is not None:

gcloud/storage/test_batch.py

@@ -89,7 +89,7 @@ def test_ctor_w_explicit_connection(self):
         batch = self._makeOne(connection)
         self.assertTrue(batch._connection is connection)
         self.assertEqual(len(batch._requests), 0)
-        self.assertEqual(len(batch._futures), 0)
+        self.assertEqual(len(batch._target_objects), 0)
 
     def test_ctor_w_implicit_connection(self):
         from gcloud.storage._testing import _monkey_defaults
@@ -101,7 +101,7 @@ def test_ctor_w_implicit_connection(self):
 
         self.assertTrue(batch._connection is connection)
         self.assertEqual(len(batch._requests), 0)
-        self.assertEqual(len(batch._futures), 0)
+        self.assertEqual(len(batch._target_objects), 0)
 
     def test__make_request_GET_normal(self):
         from gcloud.storage.batch import _FutureDict
@@ -115,7 +115,7 @@ def test__make_request_GET_normal(self):
                                                 target_object=target)
         self.assertEqual(response.status, 204)
         self.assertTrue(isinstance(content, _FutureDict))
-        self.assertEqual(target._properties, content)
+        self.assertTrue(target._properties is content)
         self.assertEqual(http._requests, [])
         EXPECTED_HEADERS = [
             ('Accept-Encoding', 'gzip'),
@@ -140,7 +140,7 @@ def test__make_request_POST_normal(self):
                                                 target_object=target)
         self.assertEqual(response.status, 204)
         self.assertTrue(isinstance(content, _FutureDict))
-        self.assertEqual(target._properties, content)
+        self.assertTrue(target._properties is content)
         self.assertEqual(http._requests, [])
         EXPECTED_HEADERS = [
             ('Accept-Encoding', 'gzip'),
@@ -165,7 +165,7 @@ def test__make_request_PATCH_normal(self):
                                                 target_object=target)
         self.assertEqual(response.status, 204)
         self.assertTrue(isinstance(content, _FutureDict))
-        self.assertEqual(target._properties, content)
+        self.assertTrue(target._properties is content)
         self.assertEqual(http._requests, [])
         EXPECTED_HEADERS = [
             ('Accept-Encoding', 'gzip'),
@@ -190,7 +190,7 @@ def test__make_request_DELETE_normal(self):
                                                 target_object=target)
         self.assertEqual(response.status, 204)
         self.assertTrue(isinstance(content, _FutureDict))
-        self.assertEqual(target._properties, content)
+        self.assertTrue(target._properties is content)
         self.assertEqual(http._requests, [])
         EXPECTED_HEADERS = [
             ('Accept-Encoding', 'gzip'),
@@ -330,13 +330,13 @@ def test_finish_nonempty_with_status_failure(self):
         batch._do_request('GET', URL, {}, None, target1)
         batch._do_request('GET', URL, {}, None, target2)
         # Make sure futures are not populated.
-        self.assertEqual([future for future in batch._futures],
+        self.assertEqual([future for future in batch._target_objects],
                          [target1, target2])
+        target2_future_before = target2._properties
         self.assertRaises(NotFound, batch.finish)
         self.assertEqual(target1._properties,
                          {'foo': 1, 'bar': 2})
-        self.assertEqual(target2._properties,
-                         {u'error': {u'message': u'Not Found'}})
+        self.assertTrue(target2._properties is target2_future_before)
 
         self.assertEqual(len(http._requests), 1)
         method, uri, headers, body = http._requests[0]
@@ -395,7 +395,7 @@ def test_as_context_mgr_wo_error(self):
         self.assertEqual(batch._requests[0][0], 'POST')
         self.assertEqual(batch._requests[1][0], 'PATCH')
         self.assertEqual(batch._requests[2][0], 'DELETE')
-        self.assertEqual(batch._futures, [target1, target2, target3])
+        self.assertEqual(batch._target_objects, [target1, target2, target3])
         self.assertEqual(target1._properties,
                          {'foo': 1, 'bar': 2})
         self.assertEqual(target2._properties,
@@ -429,7 +429,7 @@ def test_as_context_mgr_w_error(self):
         self.assertEqual(list(_BATCHES), [])
         self.assertEqual(len(http._requests), 0)
         self.assertEqual(len(batch._requests), 3)
-        self.assertEqual(batch._futures, [target1, target2, target3])
+        self.assertEqual(batch._target_objects, [target1, target2, target3])
         # Since the context manager fails, finish will not get called and
         # the _properties will still be futures.
         self.assertTrue(isinstance(target1._properties, _FutureDict))

gcloud/test_connection.py

@@ -255,7 +255,7 @@ def test_api_request_wo_json_expected(self):
             b'CONTENT',
         )
         self.assertEqual(conn.api_request('GET', '/', expect_json=False),
-                         u'CONTENT')
+                         b'CONTENT')
 
     def test_api_request_w_query_params(self):
         from six.moves.urllib.parse import parse_qsl