From 2e73689429f9e33a5476b2491fdc7bee1daf3d1a Mon Sep 17 00:00:00 2001
From: Danny Hermes <daniel.j.hermes@gmail.com>
Date: Fri, 10 Apr 2015 17:02:58 -0700
Subject: [PATCH 1/6] Adding _FutureDict type for storage batches.

---
 gcloud/storage/_helpers.py      |  2 ++
 gcloud/storage/batch.py         | 55 +++++++++++++++++++++++++++++++++
 gcloud/storage/test__helpers.py | 17 ++++++++++
 gcloud/storage/test_batch.py    | 32 +++++++++++++++++++
 4 files changed, 106 insertions(+)

diff --git a/gcloud/storage/_helpers.py b/gcloud/storage/_helpers.py
index e6b528b01a12..06b366c6145a 100644
--- a/gcloud/storage/_helpers.py
+++ b/gcloud/storage/_helpers.py
@@ -88,6 +88,8 @@ def _set_properties(self, value):
         :param value: The properties to be set.
         """
         self._properties = value
+        if hasattr(value, 'owner'):
+            value.owner = self
         # If the values are reset, the changes must as well.
         self._changes = set()
 
diff --git a/gcloud/storage/batch.py b/gcloud/storage/batch.py
index ad811d234184..b45eaef5a3b5 100644
--- a/gcloud/storage/batch.py
+++ b/gcloud/storage/batch.py
@@ -71,6 +71,61 @@ class NoContent(object):
     status = 204
 
 
+class _FutureDict(object):
+    """Class to hold a future value for a deferred request.
+
+    Used by for requests that get sent in a :class:`Batch`.
+
+    :type owner: object
+    :param owner: Optional. A blob or bucket (or other type) that depends on
+                  this future.
+    """
+
+    def __init__(self, owner=None):
+        self.owner = owner
+
+    @staticmethod
+    def get(key, default=None):
+        """Stand-in for dict.get.
+
+        :type key: object
+        :param key: Hashable dictionary key.
+
+        :type default: object
+        :param default: Fallback value to dict.get.
+
+        :raises: :class:`KeyError` always since the future is intended to fail
+                 as a dictionary.
+        """
+        raise KeyError('Cannot get(%r, default=%r) on a future' % (
+            key, default))
+
+    def __getitem__(self, key):
+        """Stand-in for dict[key].
+
+        :type key: object
+        :param key: Hashable dictionary key.
+
+        :raises: :class:`KeyError` always since the future is intended to fail
+                 as a dictionary.
+        """
+        raise KeyError('Cannot get item %r from a future' % (key,))
+
+    def __setitem__(self, key, value):
+        """Stand-in for dict[key] = value.
+
+        :type key: object
+        :param key: Hashable dictionary key.
+
+        :type value: object
+        :param value: Dictionary value.
+
+        :raises: :class:`KeyError` always since the future is intended to fail
+                 as a dictionary.
+        """
+        raise KeyError('Cannot set %r -> %r on a future' % (key, value))
+
+
 class Batch(Connection):
     """Proxy an underlying connection, batching up change operations.
 
diff --git a/gcloud/storage/test__helpers.py b/gcloud/storage/test__helpers.py
index fabf23d4718b..71ee5b4dd005 100644
--- a/gcloud/storage/test__helpers.py
+++ b/gcloud/storage/test__helpers.py
@@ -73,6 +73,23 @@ def test_reload_w_explicit_connection(self):
         # Make sure changes get reset by reload.
         self.assertEqual(derived._changes, set())
 
+    def test__set_properties_no_future(self):
+        mixin = self._makeOne()
+        self.assertEqual(mixin._properties, {})
+        VALUE = {'foo': 'bar'}
+        mixin._set_properties(VALUE)
+        self.assertEqual(mixin._properties, VALUE)
+        self.assertFalse(hasattr(VALUE, 'owner'))
+
+    def test__set_properties_future(self):
+        from gcloud.storage.batch import _FutureDict
+        mixin = self._makeOne()
+        future = _FutureDict()
+        self.assertEqual(future.owner, None)
+        mixin._set_properties(future)
+        self.assertEqual(mixin._properties, future)
+        self.assertEqual(future.owner, mixin)
+
     def test__patch_property(self):
         derived = self._derivedClass()()
         derived._patch_property('foo', 'Foo')
diff --git a/gcloud/storage/test_batch.py b/gcloud/storage/test_batch.py
index aaa2e94dc10e..d302b61e6fba 100644
--- a/gcloud/storage/test_batch.py
+++ b/gcloud/storage/test_batch.py
@@ -404,6 +404,38 @@ def test_unicode(self):
 """
 
 
+class Test__FutureDict(unittest2.TestCase):
+
+    def _makeOne(self, *args, **kw):
+        from gcloud.storage.batch import _FutureDict
+        return _FutureDict(*args, **kw)
+
+    def test_ctor_defaults(self):
+        future = self._makeOne()
+        self.assertEqual(future.owner, None)
+
+    def test_ctor_owner(self):
+        OWNER = object()
+        future = self._makeOne(owner=OWNER)
+        self.assertTrue(future.owner is OWNER)
+
+    def test_get(self):
+        future = self._makeOne()
+        self.assertRaises(KeyError, future.get, None)
+
+    def test___getitem__(self):
+        future = self._makeOne()
+        value = orig_value = object()
+        with self.assertRaises(KeyError):
+            value = future[None]
+        self.assertTrue(value is orig_value)
+
+    def test___setitem__(self):
+        future = self._makeOne()
+        with self.assertRaises(KeyError):
+            future[None] = None
+
+
 class _Connection(object):
 
     project = 'TESTING'

From 7821e1c2de36e6710562c95ab7b9a28210f63f91 Mon Sep 17 00:00:00 2001
From: Danny Hermes <daniel.j.hermes@gmail.com>
Date: Fri, 10 Apr 2015 17:47:55 -0700
Subject: [PATCH 2/6] Using futures in batched requests.

Also removing limitation on GETs in batch requests.
---
 gcloud/connection.py         |   7 +-
 gcloud/storage/_helpers.py   |   2 +-
 gcloud/storage/batch.py      | 106 +++++++++++---
 gcloud/storage/test_batch.py | 258 ++++++++++++++++++++++++++---------
 gcloud/test_connection.py    |   2 +-
 5 files changed, 283 insertions(+), 92 deletions(-)

diff --git a/gcloud/connection.py b/gcloud/connection.py
index f70b1e2720f8..cec179a81765 100644
--- a/gcloud/connection.py
+++ b/gcloud/connection.py
@@ -292,12 +292,13 @@ def api_request(self, method, path, query_params=None,
         if not 200 <= response.status < 300:
             raise make_exception(response, content)
 
-        if content and expect_json:
+        if isinstance(content, six.binary_type):
+            content = content.decode('utf-8')
+
+        if expect_json and content and isinstance(content, six.string_types):
             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
diff --git a/gcloud/storage/_helpers.py b/gcloud/storage/_helpers.py
index 06b366c6145a..0559311f39cf 100644
--- a/gcloud/storage/_helpers.py
+++ b/gcloud/storage/_helpers.py
@@ -84,7 +84,7 @@ def _patch_property(self, name, value):
     def _set_properties(self, value):
         """Set the properties for the current object.
 
-        :type value: dict
+        :type value: dict or :class:`gcloud.storage.batch._FutureDict`
         :param value: The properties to be set.
         """
         self._properties = value
diff --git a/gcloud/storage/batch.py b/gcloud/storage/batch.py
index b45eaef5a3b5..436b0f755218 100644
--- a/gcloud/storage/batch.py
+++ b/gcloud/storage/batch.py
@@ -20,12 +20,14 @@
 from email.mime.application import MIMEApplication
 from email.mime.multipart import MIMEMultipart
 from email.parser import Parser
+import httplib2
 import io
 import json
 
 import six
 
 from gcloud._helpers import _LocalStack
+from gcloud.exceptions import make_exception
 from gcloud.storage import _implicit_environ
 from gcloud.storage.connection import Connection
 
@@ -82,6 +84,7 @@ class _FutureDict(object):
     """
 
     def __init__(self, owner=None):
+        self._value = None
         self.owner = owner
 
     @staticmethod
@@ -125,6 +128,20 @@ def __setitem__(self, key, value):
         """
         raise KeyError('Cannot set %r -> %r on a future' % (key, value))
 
+    def set_future_value(self, value):
+        """Sets the value associated with the future.
+
+        :type value: object
+        :param value: The future value that was set.
+
+        :raises: :class:`ValueError` if the value is already set.
+        """
+        if self._value is not None:
+            raise ValueError('Future value is already set.')
+        self._value = value
+        if self.owner is not None:
+            self.owner._properties = value
+
 
 class Batch(Connection):
     """Proxy an underlying connection, batching up change operations.
@@ -141,7 +158,7 @@ def __init__(self, connection=None):
         super(Batch, self).__init__()
         self._connection = connection
         self._requests = []
-        self._responses = []
+        self._futures = []
 
     def _do_request(self, method, url, headers, data):
         """Override Connection:  defer actual HTTP request.
@@ -164,22 +181,20 @@ def _do_request(self, method, url, headers, data):
                 and ``content`` (a string).
         :returns: The HTTP response object and the content of the response.
         """
-        if method == 'GET':
-            _req = self._connection.http.request
-            return _req(method=method, uri=url, headers=headers, body=data)
-
         if len(self._requests) >= self._MAX_BATCH_SIZE:
             raise ValueError("Too many deferred requests (max %d)" %
                              self._MAX_BATCH_SIZE)
         self._requests.append((method, url, headers, data))
-        return NoContent(), ''
+        result = _FutureDict()
+        self._futures.append(result)
+        return NoContent(), result
 
-    def finish(self):
-        """Submit a single `multipart/mixed` request w/ deferred requests.
+    def _prepare_batch_request(self):
+        """Prepares headers and body for a batch request.
 
-        :rtype: list of tuples
-        :returns: one ``(status, reason, payload)`` tuple per deferred request.
-        :raises: ValueError if no requests have been deferred.
+        :rtype: tuple (dict, string)
+        :returns: The pair of headers and body of the batch request to be sent.
+        :raises: :class:`ValueError` if no requests have been deferred.
         """
         if len(self._requests) == 0:
             raise ValueError("No deferred requests")
@@ -201,14 +216,49 @@ def finish(self):
 
         # Strip off redundant header text
         _, body = payload.split('\n\n', 1)
-        headers = dict(multi._headers)
+        return dict(multi._headers), body
+
+    def _finish_futures(self, responses):
+        """Apply all the batch responses to the futures created.
+
+        :type responses: list of (headers, payload) tuples.
+        :param responses: List of headers and payloads from each response in
+                          the batch.
+
+        :raises: :class:`ValueError` if no requests have been deferred.
+        """
+        # If a bad status occurs, we track it, but don't raise an exception
+        # until all futures have been populated.
+        exception_args = None
+
+        if len(self._futures) != len(responses):
+            raise ValueError('Expected a response for every request.')
+
+        for future, sub_response in zip(self._futures, responses):
+            resp_headers, sub_payload = sub_response
+            if not 200 <= resp_headers.status < 300:
+                exception_args = exception_args or (resp_headers,
+                                                    sub_payload)
+            future.set_future_value(sub_payload)
+
+        if exception_args is not None:
+            raise make_exception(*exception_args)
+
+    def finish(self):
+        """Submit a single `multipart/mixed` request w/ deferred requests.
+
+        :rtype: list of tuples
+        :returns: one ``(headers, payload)`` tuple per deferred request.
+        """
+        headers, body = self._prepare_batch_request()
 
         url = '%s/batch' % self.API_BASE_URL
 
         _req = self._connection._make_request
         response, content = _req('POST', url, data=body, headers=headers)
-        self._responses = list(_unpack_batch_response(response, content))
-        return self._responses
+        responses = list(_unpack_batch_response(response, content))
+        self._finish_futures(responses)
+        return responses
 
     @staticmethod
     def current():
@@ -254,7 +304,20 @@ def _generate_faux_mime_message(parser, response, content):
 
 
 def _unpack_batch_response(response, content):
-    """Convert response, content -> [(status, reason, payload)]."""
+    """Convert response, content -> [(headers, payload)].
+
+    Creates a generator of tuples of emulating the responses to
+    :meth:`httplib2.Http.request` (a pair of headers and payload).
+
+    :type response: :class:`httplib2.Response`
+    :param response: HTTP response / headers from a request.
+
+    :type content: string
+    :param content: Response payload with a batch response.
+
+    :rtype: generator
+    :returns: A generator of header, payload pairs.
+    """
     parser = Parser()
     message = _generate_faux_mime_message(parser, response, content)
 
@@ -263,10 +326,13 @@ def _unpack_batch_response(response, content):
 
     for subrequest in message._payload:
         status_line, rest = subrequest._payload.split('\n', 1)
-        _, status, reason = status_line.split(' ', 2)
-        message = parser.parsestr(rest)
-        payload = message._payload
-        ctype = message['Content-Type']
+        _, status, _ = status_line.split(' ', 2)
+        sub_message = parser.parsestr(rest)
+        payload = sub_message._payload
+        ctype = sub_message['Content-Type']
+        msg_headers = dict(sub_message._headers)
+        msg_headers['status'] = status
+        headers = httplib2.Response(msg_headers)
         if ctype and ctype.startswith('application/json'):
             payload = json.loads(payload)
-        yield status, reason, payload
+        yield headers, payload
diff --git a/gcloud/storage/test_batch.py b/gcloud/storage/test_batch.py
index d302b61e6fba..9b8ec850cbbc 100644
--- a/gcloud/storage/test_batch.py
+++ b/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._responses), 0)
+        self.assertEqual(len(batch._futures), 0)
 
     def test_ctor_w_implicit_connection(self):
         from gcloud.storage._testing import _monkey_defaults
@@ -101,92 +101,100 @@ def test_ctor_w_implicit_connection(self):
 
         self.assertTrue(batch._connection is connection)
         self.assertEqual(len(batch._requests), 0)
-        self.assertEqual(len(batch._responses), 0)
+        self.assertEqual(len(batch._futures), 0)
 
-    def test__make_request_GET_forwarded_to_connection(self):
+    def test__make_request_GET_normal(self):
+        from gcloud.storage.batch import _FutureDict
         URL = 'http://example.com/api'
         expected = _Response()
         http = _HTTP((expected, ''))
         connection = _Connection(http=http)
         batch = self._makeOne(connection)
         response, content = batch._make_request('GET', URL)
-        self.assertTrue(response is expected)
-        self.assertEqual(content, '')
+        self.assertEqual(response.status, 204)
+        self.assertTrue(isinstance(content, _FutureDict))
+        self.assertEqual(content._value, None)
+        self.assertEqual(http._requests, [])
         EXPECTED_HEADERS = [
             ('Accept-Encoding', 'gzip'),
             ('Content-Length', 0),
         ]
-        self.assertEqual(len(http._requests), 1)
-        self.assertEqual(http._requests[0][0], 'GET')
-        self.assertEqual(http._requests[0][1], URL)
-        headers = http._requests[0][2]
+        solo_request, = batch._requests
+        self.assertEqual(solo_request[0], 'GET')
+        self.assertEqual(solo_request[1], URL)
+        headers = solo_request[2]
         for key, value in EXPECTED_HEADERS:
             self.assertEqual(headers[key], value)
-        self.assertEqual(http._requests[0][3], None)
-        self.assertEqual(batch._requests, [])
+        self.assertEqual(solo_request[3], None)
 
     def test__make_request_POST_normal(self):
+        from gcloud.storage.batch import _FutureDict
         URL = 'http://example.com/api'
         http = _HTTP()  # no requests expected
         connection = _Connection(http=http)
         batch = self._makeOne(connection)
         response, content = batch._make_request('POST', URL, data={'foo': 1})
         self.assertEqual(response.status, 204)
-        self.assertEqual(content, '')
+        self.assertTrue(isinstance(content, _FutureDict))
+        self.assertEqual(content._value, None)
         self.assertEqual(http._requests, [])
         EXPECTED_HEADERS = [
             ('Accept-Encoding', 'gzip'),
             ('Content-Length', 10),
         ]
-        self.assertEqual(len(batch._requests), 1)
-        self.assertEqual(batch._requests[0][0], 'POST')
-        self.assertEqual(batch._requests[0][1], URL)
-        headers = batch._requests[0][2]
+        solo_request, = batch._requests
+        self.assertEqual(solo_request[0], 'POST')
+        self.assertEqual(solo_request[1], URL)
+        headers = solo_request[2]
         for key, value in EXPECTED_HEADERS:
             self.assertEqual(headers[key], value)
-        self.assertEqual(batch._requests[0][3], {'foo': 1})
+        self.assertEqual(solo_request[3], {'foo': 1})
 
     def test__make_request_PATCH_normal(self):
+        from gcloud.storage.batch import _FutureDict
         URL = 'http://example.com/api'
         http = _HTTP()  # no requests expected
         connection = _Connection(http=http)
         batch = self._makeOne(connection)
         response, content = batch._make_request('PATCH', URL, data={'foo': 1})
         self.assertEqual(response.status, 204)
-        self.assertEqual(content, '')
+        self.assertTrue(isinstance(content, _FutureDict))
+        self.assertEqual(content._value, None)
         self.assertEqual(http._requests, [])
         EXPECTED_HEADERS = [
             ('Accept-Encoding', 'gzip'),
             ('Content-Length', 10),
         ]
-        self.assertEqual(len(batch._requests), 1)
-        self.assertEqual(batch._requests[0][0], 'PATCH')
-        self.assertEqual(batch._requests[0][1], URL)
-        headers = batch._requests[0][2]
+        solo_request, = batch._requests
+        self.assertEqual(solo_request[0], 'PATCH')
+        self.assertEqual(solo_request[1], URL)
+        headers = solo_request[2]
         for key, value in EXPECTED_HEADERS:
             self.assertEqual(headers[key], value)
-        self.assertEqual(batch._requests[0][3], {'foo': 1})
+        self.assertEqual(solo_request[3], {'foo': 1})
 
     def test__make_request_DELETE_normal(self):
+        from gcloud.storage.batch import _FutureDict
         URL = 'http://example.com/api'
         http = _HTTP()  # no requests expected
         connection = _Connection(http=http)
         batch = self._makeOne(connection)
         response, content = batch._make_request('DELETE', URL)
         self.assertEqual(response.status, 204)
-        self.assertEqual(content, '')
+        self.assertTrue(isinstance(content, _FutureDict))
+        self.assertEqual(content._value, None)
         self.assertEqual(http._requests, [])
         EXPECTED_HEADERS = [
             ('Accept-Encoding', 'gzip'),
             ('Content-Length', 0),
         ]
-        self.assertEqual(len(batch._requests), 1)
-        self.assertEqual(batch._requests[0][0], 'DELETE')
-        self.assertEqual(batch._requests[0][1], URL)
-        headers = batch._requests[0][2]
+        solo_request, = batch._requests
+        self.assertEqual(solo_request[0], 'DELETE')
+        self.assertEqual(solo_request[1], URL)
+        headers = solo_request[2]
         for key, value in EXPECTED_HEADERS:
             self.assertEqual(headers[key], value)
-        self.assertEqual(batch._requests[0][3], None)
+        self.assertEqual(solo_request[3], None)
 
     def test__make_request_POST_too_many_requests(self):
         URL = 'http://example.com/api'
@@ -223,18 +231,24 @@ def _check_subrequest_payload(self, chunk, method, url, payload):
         lines = chunk.splitlines()
         # blank + 2 headers + blank + request + 2 headers + blank + body
         payload_str = json.dumps(payload)
-        self.assertEqual(len(lines), 9)
         self.assertEqual(lines[0], '')
         self.assertEqual(lines[1], 'Content-Type: application/http')
         self.assertEqual(lines[2], 'MIME-Version: 1.0')
         self.assertEqual(lines[3], '')
         self.assertEqual(lines[4], '%s %s HTTP/1.1' % (method, url))
-        self.assertEqual(lines[5], 'Content-Length: %d' % len(payload_str))
-        self.assertEqual(lines[6], 'Content-Type: application/json')
-        self.assertEqual(lines[7], '')
-        self.assertEqual(json.loads(lines[8]), payload)
+        if method == 'GET':
+            self.assertEqual(len(lines), 7)
+            self.assertEqual(lines[5], '')
+            self.assertEqual(lines[6], '')
+        else:
+            self.assertEqual(len(lines), 9)
+            self.assertEqual(lines[5], 'Content-Length: %d' % len(payload_str))
+            self.assertEqual(lines[6], 'Content-Type: application/json')
+            self.assertEqual(lines[7], '')
+            self.assertEqual(json.loads(lines[8]), payload)
 
     def test_finish_nonempty(self):
+        import httplib2
         URL = 'http://api.example.com/other_api'
         expected = _Response()
         expected['content-type'] = 'multipart/mixed; boundary="DEADBEEF="'
@@ -242,20 +256,24 @@ def test_finish_nonempty(self):
         connection = _Connection(http=http)
         batch = self._makeOne(connection)
         batch.API_BASE_URL = 'http://api.example.com'
-        batch._requests.append(('POST', URL, {}, {'foo': 1, 'bar': 2}))
-        batch._requests.append(('PATCH', URL, {}, {'bar': 3}))
-        batch._requests.append(('DELETE', URL, {}, None))
+        batch._do_request('POST', URL, {}, {'foo': 1, 'bar': 2})
+        batch._do_request('PATCH', URL, {}, {'bar': 3})
+        batch._do_request('DELETE', URL, {}, None)
         result = batch.finish()
         self.assertEqual(len(result), len(batch._requests))
-        self.assertEqual(result[0][0], '200')
-        self.assertEqual(result[0][1], 'OK')
-        self.assertEqual(result[0][2], {'foo': 1, 'bar': 2})
-        self.assertEqual(result[1][0], '200')
-        self.assertEqual(result[1][1], 'OK')
-        self.assertEqual(result[1][2], {'foo': 1, 'bar': 3})
-        self.assertEqual(result[2][0], '204')
-        self.assertEqual(result[2][1], 'No Content')
-        self.assertEqual(result[2][2], '')
+        response0 = httplib2.Response({
+            'content-length': '20',
+            'content-type': 'application/json; charset=UTF-8',
+            'status': '200',
+        })
+        self.assertEqual(result[0], (response0, {'foo': 1, 'bar': 2}))
+        response1 = response0
+        self.assertEqual(result[1], (response1, {u'foo': 1, u'bar': 3}))
+        response2 = httplib2.Response({
+            'content-length': '0',
+            'status': '204',
+        })
+        self.assertEqual(result[2], (response2, ''))
         self.assertEqual(len(http._requests), 1)
         method, uri, headers, body = http._requests[0]
         self.assertEqual(method, 'POST')
@@ -279,6 +297,56 @@ def test_finish_nonempty(self):
 
         self._check_subrequest_no_payload(chunks[2], 'DELETE', URL)
 
+    def test_finish_responses_mismatch(self):
+        URL = 'http://api.example.com/other_api'
+        expected = _Response()
+        expected['content-type'] = 'multipart/mixed; boundary="DEADBEEF="'
+        http = _HTTP((expected, _TWO_PART_MIME_RESPONSE_WITH_FAIL))
+        connection = _Connection(http=http)
+        batch = self._makeOne(connection)
+        batch.API_BASE_URL = 'http://api.example.com'
+        batch._requests.append(('GET', URL, {}, None))
+        self.assertRaises(ValueError, batch.finish)
+
+    def test_finish_nonempty_with_status_failure(self):
+        from gcloud.exceptions import NotFound
+        URL = 'http://api.example.com/other_api'
+        expected = _Response()
+        expected['content-type'] = 'multipart/mixed; boundary="DEADBEEF="'
+        http = _HTTP((expected, _TWO_PART_MIME_RESPONSE_WITH_FAIL))
+        connection = _Connection(http=http)
+        batch = self._makeOne(connection)
+        batch.API_BASE_URL = 'http://api.example.com'
+        batch._do_request('GET', URL, {}, None)
+        batch._do_request('GET', URL, {}, None)
+        # Make sure futures are not populated.
+        self.assertEqual([future.owner for future in batch._futures],
+                         [None, None])
+        self.assertRaises(NotFound, batch.finish)
+        self.assertEqual(batch._futures[0]._value,
+                         {'foo': 1, 'bar': 2})
+        self.assertEqual(batch._futures[1]._value,
+                         {u'error': {u'message': u'Not Found'}})
+
+        self.assertEqual(len(http._requests), 1)
+        method, uri, headers, body = http._requests[0]
+        self.assertEqual(method, 'POST')
+        self.assertEqual(uri, 'http://api.example.com/batch')
+        self.assertEqual(len(headers), 2)
+        ctype, boundary = [x.strip()
+                           for x in headers['Content-Type'].split(';')]
+        self.assertEqual(ctype, 'multipart/mixed')
+        self.assertTrue(boundary.startswith('boundary="=='))
+        self.assertTrue(boundary.endswith('=="'))
+        self.assertEqual(headers['MIME-Version'], '1.0')
+
+        divider = '--' + boundary[len('boundary="'):-1]
+        chunks = body.split(divider)[1:-1]  # discard prolog / epilog
+        self.assertEqual(len(chunks), 2)
+
+        self._check_subrequest_payload(chunks[0], 'GET', URL, {})
+        self._check_subrequest_payload(chunks[1], 'GET', URL, {})
+
     def test_finish_nonempty_non_multipart_response(self):
         URL = 'http://api.example.com/other_api'
         expected = _Response()
@@ -312,16 +380,12 @@ 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(len(batch._responses), 3)
-        self.assertEqual(
-            batch._responses[0],
-            ('200', 'OK', {'foo': 1, 'bar': 2}))
-        self.assertEqual(
-            batch._responses[1],
-            ('200', 'OK', {'foo': 1, 'bar': 3}))
-        self.assertEqual(
-            batch._responses[2],
-            ('204', 'No Content', ''))
+        self.assertEqual(len(batch._futures), 3)
+        self.assertEqual(batch._futures[0]._value,
+                         {'foo': 1, 'bar': 2})
+        self.assertEqual(batch._futures[1]._value,
+                         {'foo': 1, 'bar': 3})
+        self.assertEqual(batch._futures[2]._value, '')
 
     def test_as_context_mgr_w_error(self):
         from gcloud.storage.batch import _BATCHES
@@ -344,7 +408,8 @@ 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(len(batch._responses), 0)
+        self.assertEqual([future._value for future in batch._futures],
+                         [None, None, None])
 
 
 class Test__unpack_batch_response(unittest2.TestCase):
@@ -353,24 +418,58 @@ def _callFUT(self, response, content):
         from gcloud.storage.batch import _unpack_batch_response
         return _unpack_batch_response(response, content)
 
+    def _unpack_helper(self, response, content):
+        import httplib2
+        result = list(self._callFUT(response, content))
+        self.assertEqual(len(result), 3)
+        response0 = httplib2.Response({
+            'content-length': '20',
+            'content-type': 'application/json; charset=UTF-8',
+            'status': '200',
+        })
+        self.assertEqual(result[0], (response0, {u'bar': 2, u'foo': 1}))
+        response1 = response0
+        self.assertEqual(result[1], (response1, {u'foo': 1, u'bar': 3}))
+        response2 = httplib2.Response({
+            'content-length': '0',
+            'status': '204',
+        })
+        self.assertEqual(result[2], (response2, ''))
+
     def test_bytes(self):
         RESPONSE = {'content-type': b'multipart/mixed; boundary="DEADBEEF="'}
         CONTENT = _THREE_PART_MIME_RESPONSE
-        result = list(self._callFUT(RESPONSE, CONTENT))
-        self.assertEqual(len(result), 3)
-        self.assertEqual(result[0], ('200', 'OK', {u'bar': 2, u'foo': 1}))
-        self.assertEqual(result[1], ('200', 'OK', {u'foo': 1, u'bar': 3}))
-        self.assertEqual(result[2], ('204', 'No Content', ''))
+        self._unpack_helper(RESPONSE, CONTENT)
 
     def test_unicode(self):
         RESPONSE = {'content-type': u'multipart/mixed; boundary="DEADBEEF="'}
         CONTENT = _THREE_PART_MIME_RESPONSE.decode('utf-8')
-        result = list(self._callFUT(RESPONSE, CONTENT))
-        self.assertEqual(len(result), 3)
-        self.assertEqual(result[0], ('200', 'OK', {u'bar': 2, u'foo': 1}))
-        self.assertEqual(result[1], ('200', 'OK', {u'foo': 1, u'bar': 3}))
-        self.assertEqual(result[2], ('204', 'No Content', ''))
+        self._unpack_helper(RESPONSE, CONTENT)
+
 
+_TWO_PART_MIME_RESPONSE_WITH_FAIL = b"""\
+--DEADBEEF=
+Content-Type: application/http
+Content-ID: <response-8a09ca85-8d1d-4f45-9eb0-da8e8b07ec83+1>
+
+HTTP/1.1 200 OK
+Content-Type: application/json; charset=UTF-8
+Content-Length: 20
+
+{"foo": 1, "bar": 2}
+
+--DEADBEEF=
+Content-Type: application/http
+Content-ID: <response-8a09ca85-8d1d-4f45-9eb0-da8e8b07ec83+2>
+
+HTTP/1.1 404 Not Found
+Content-Type: application/json; charset=UTF-8
+Content-Length: 35
+
+{"error": {"message": "Not Found"}}
+
+--DEADBEEF=--
+"""
 
 _THREE_PART_MIME_RESPONSE = b"""\
 --DEADBEEF=
@@ -435,6 +534,31 @@ def test___setitem__(self):
         with self.assertRaises(KeyError):
             future[None] = None
 
+    def test_set_future_value_no_owner(self):
+        future = self._makeOne()
+        self.assertEqual(future._value, None)
+        VALUE = object()
+        future.set_future_value(VALUE)
+        self.assertTrue(future._value is VALUE)
+
+    def test_set_future_value_with_owner(self):
+        from gcloud.storage._helpers import _PropertyMixin
+        future = self._makeOne()
+        future.owner = _PropertyMixin()
+        self.assertEqual(future._value, None)
+        self.assertEqual(future.owner._properties, {})
+        VALUE = object()
+        future.set_future_value(VALUE)
+        self.assertTrue(future._value is VALUE)
+        self.assertTrue(future.owner._properties is VALUE)
+
+    def test_set_future_value_already_set(self):
+        future = self._makeOne()
+        self.assertEqual(future._value, None)
+        VALUE = object()
+        future.set_future_value(VALUE)
+        self.assertRaises(ValueError, future.set_future_value, VALUE)
+
 
 class _Connection(object):
 
diff --git a/gcloud/test_connection.py b/gcloud/test_connection.py
index 06f70f3e8890..446ffbaffaa3 100644
--- a/gcloud/test_connection.py
+++ b/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),
-                         b'CONTENT')
+                         u'CONTENT')
 
     def test_api_request_w_query_params(self):
         from six.moves.urllib.parse import parse_qsl

From c261d0acd62b5142b225da40772167fdd6b3d26c Mon Sep 17 00:00:00 2001
From: Danny Hermes <daniel.j.hermes@gmail.com>
Date: Mon, 20 Apr 2015 23:10:47 -0700
Subject: [PATCH 3/6] Chaning JSONConnection methods to accept a target object.

This is to allow storage.Batch.finish() to update properties on
objects from requests.
---
 gcloud/connection.py            |  27 +++++--
 gcloud/storage/_helpers.py      |   2 -
 gcloud/storage/batch.py         |  33 ++-------
 gcloud/storage/test__helpers.py |  14 +---
 gcloud/storage/test_batch.py    | 121 +++++++++++++++-----------------
 5 files changed, 89 insertions(+), 108 deletions(-)

diff --git a/gcloud/connection.py b/gcloud/connection.py
index cec179a81765..2cea649cfe26 100644
--- a/gcloud/connection.py
+++ b/gcloud/connection.py
@@ -160,7 +160,7 @@ def build_api_url(cls, path, query_params=None,
         return url
 
     def _make_request(self, method, url, data=None, content_type=None,
-                      headers=None):
+                      headers=None, target_object=None):
         """A low level method to send a request to the API.
 
         Typically, you shouldn't need to use this method.
@@ -180,6 +180,12 @@ def _make_request(self, method, url, data=None, content_type=None,
         :type headers: dict
         :param headers: A dictionary of HTTP headers to send with the request.
 
+        :type target_object: object or :class:`NoneType`
+        :param target_object: Argument to be used by library callers.
+                              This can allow custom behavior, for example, to
+                              defer an HTTP request and complete initialization
+                              of the object at a later time.
+
         :rtype: tuple of ``response`` (a dictionary of sorts)
                 and ``content`` (a string).
         :returns: The HTTP response object and the content of the response,
@@ -200,9 +206,9 @@ def _make_request(self, method, url, data=None, content_type=None,
 
         headers['User-Agent'] = self.USER_AGENT
 
-        return self._do_request(method, url, headers, data)
+        return self._do_request(method, url, headers, data, target_object)
 
-    def _do_request(self, method, url, headers, data):
+    def _do_request(self, method, url, headers, data, dummy):
         """Low-level helper:  perform the actual API request over HTTP.
 
         Allows batch context managers to override and defer a request.
@@ -219,6 +225,10 @@ def _do_request(self, method, url, headers, data):
         :type data: string
         :param data: The data to send as the body of the request.
 
+        :type dummy: object or :class:`NoneType`
+        :param dummy: Unused ``target_object`` here but may be used
+                      by a superclass.
+
         :rtype: tuple of ``response`` (a dictionary of sorts)
                 and ``content`` (a string).
         :returns: The HTTP response object and the content of the response.
@@ -229,7 +239,7 @@ def _do_request(self, method, url, headers, data):
     def api_request(self, method, path, query_params=None,
                     data=None, content_type=None,
                     api_base_url=None, api_version=None,
-                    expect_json=True):
+                    expect_json=True, _target_object=None):
         """Make a request over the HTTP transport to the API.
 
         You shouldn't need to use this method, but if you plan to
@@ -274,6 +284,12 @@ def api_request(self, method, path, query_params=None,
                             response as JSON and raise an exception if
                             that cannot be done.  Default is True.
 
+        :type _target_object: object or :class:`NoneType`
+        :param _target_object: Protected argument to be used by library
+                               callers. This can allow custom behavior, for
+                               example, to defer an HTTP request and complete
+                               initialization of the object at a later time.
+
         :raises: Exception if the response code is not 200 OK.
         """
         url = self.build_api_url(path=path, query_params=query_params,
@@ -287,7 +303,8 @@ def api_request(self, method, path, query_params=None,
             content_type = 'application/json'
 
         response, content = self._make_request(
-            method=method, url=url, data=data, content_type=content_type)
+            method=method, url=url, data=data, content_type=content_type,
+            target_object=_target_object)
 
         if not 200 <= response.status < 300:
             raise make_exception(response, content)
diff --git a/gcloud/storage/_helpers.py b/gcloud/storage/_helpers.py
index 0559311f39cf..6c48e5aa19b5 100644
--- a/gcloud/storage/_helpers.py
+++ b/gcloud/storage/_helpers.py
@@ -88,8 +88,6 @@ def _set_properties(self, value):
         :param value: The properties to be set.
         """
         self._properties = value
-        if hasattr(value, 'owner'):
-            value.owner = self
         # If the values are reset, the changes must as well.
         self._changes = set()
 
diff --git a/gcloud/storage/batch.py b/gcloud/storage/batch.py
index 436b0f755218..a73d8abcd1db 100644
--- a/gcloud/storage/batch.py
+++ b/gcloud/storage/batch.py
@@ -77,16 +77,8 @@ class _FutureDict(object):
     """Class to hold a future value for a deferred request.
 
     Used by for requests that get sent in a :class:`Batch`.
-
-    :type owner: object
-    :param owner: Optional. A blob or bucket (or other type) that depends on
-                  this future.
     """
 
-    def __init__(self, owner=None):
-        self._value = None
-        self.owner = owner
-
     @staticmethod
     def get(key, default=None):
         """Stand-in for dict.get.
@@ -128,20 +120,6 @@ def __setitem__(self, key, value):
         """
         raise KeyError('Cannot set %r -> %r on a future' % (key, value))
 
-    def set_future_value(self, value):
-        """Sets the value associated with the future.
-
-        :type value: object
-        :param value: The future value that was set.
-
-        :raises: :class:`ValueError` if the value is already set.
-        """
-        if self._value is not None:
-            raise ValueError('Future value is already set.')
-        self._value = value
-        if self.owner is not None:
-            self.owner._properties = value
-
 
 class Batch(Connection):
     """Proxy an underlying connection, batching up change operations.
@@ -160,7 +138,7 @@ def __init__(self, connection=None):
         self._requests = []
         self._futures = []
 
-    def _do_request(self, method, url, headers, data):
+    def _do_request(self, method, url, headers, data, target_object):
         """Override Connection:  defer actual HTTP request.
 
         Only allow up to ``_MAX_BATCH_SIZE`` requests to be deferred.
@@ -186,7 +164,9 @@ def _do_request(self, method, url, headers, data):
                              self._MAX_BATCH_SIZE)
         self._requests.append((method, url, headers, data))
         result = _FutureDict()
-        self._futures.append(result)
+        self._futures.append(target_object)
+        if target_object is not None:
+            target_object._properties = result
         return NoContent(), result
 
     def _prepare_batch_request(self):
@@ -234,12 +214,13 @@ def _finish_futures(self, responses):
         if len(self._futures) != len(responses):
             raise ValueError('Expected a response for every request.')
 
-        for future, sub_response in zip(self._futures, responses):
+        for target_object, sub_response in zip(self._futures, responses):
             resp_headers, sub_payload = sub_response
             if not 200 <= resp_headers.status < 300:
                 exception_args = exception_args or (resp_headers,
                                                     sub_payload)
-            future.set_future_value(sub_payload)
+            if target_object is not None:
+                target_object._properties = sub_payload
 
         if exception_args is not None:
             raise make_exception(*exception_args)
diff --git a/gcloud/storage/test__helpers.py b/gcloud/storage/test__helpers.py
index 71ee5b4dd005..104ecb38bd05 100644
--- a/gcloud/storage/test__helpers.py
+++ b/gcloud/storage/test__helpers.py
@@ -73,22 +73,12 @@ def test_reload_w_explicit_connection(self):
         # Make sure changes get reset by reload.
         self.assertEqual(derived._changes, set())
 
-    def test__set_properties_no_future(self):
+    def test__set_properties(self):
         mixin = self._makeOne()
         self.assertEqual(mixin._properties, {})
-        VALUE = {'foo': 'bar'}
+        VALUE = object()
         mixin._set_properties(VALUE)
         self.assertEqual(mixin._properties, VALUE)
-        self.assertFalse(hasattr(VALUE, 'owner'))
-
-    def test__set_properties_future(self):
-        from gcloud.storage.batch import _FutureDict
-        mixin = self._makeOne()
-        future = _FutureDict()
-        self.assertEqual(future.owner, None)
-        mixin._set_properties(future)
-        self.assertEqual(mixin._properties, future)
-        self.assertEqual(future.owner, mixin)
 
     def test__patch_property(self):
         derived = self._derivedClass()()
diff --git a/gcloud/storage/test_batch.py b/gcloud/storage/test_batch.py
index 9b8ec850cbbc..1c51d2c75532 100644
--- a/gcloud/storage/test_batch.py
+++ b/gcloud/storage/test_batch.py
@@ -110,10 +110,12 @@ def test__make_request_GET_normal(self):
         http = _HTTP((expected, ''))
         connection = _Connection(http=http)
         batch = self._makeOne(connection)
-        response, content = batch._make_request('GET', URL)
+        target = _MockObject()
+        response, content = batch._make_request('GET', URL,
+                                                target_object=target)
         self.assertEqual(response.status, 204)
         self.assertTrue(isinstance(content, _FutureDict))
-        self.assertEqual(content._value, None)
+        self.assertEqual(target._properties, content)
         self.assertEqual(http._requests, [])
         EXPECTED_HEADERS = [
             ('Accept-Encoding', 'gzip'),
@@ -133,10 +135,12 @@ def test__make_request_POST_normal(self):
         http = _HTTP()  # no requests expected
         connection = _Connection(http=http)
         batch = self._makeOne(connection)
-        response, content = batch._make_request('POST', URL, data={'foo': 1})
+        target = _MockObject()
+        response, content = batch._make_request('POST', URL, data={'foo': 1},
+                                                target_object=target)
         self.assertEqual(response.status, 204)
         self.assertTrue(isinstance(content, _FutureDict))
-        self.assertEqual(content._value, None)
+        self.assertEqual(target._properties, content)
         self.assertEqual(http._requests, [])
         EXPECTED_HEADERS = [
             ('Accept-Encoding', 'gzip'),
@@ -156,10 +160,12 @@ def test__make_request_PATCH_normal(self):
         http = _HTTP()  # no requests expected
         connection = _Connection(http=http)
         batch = self._makeOne(connection)
-        response, content = batch._make_request('PATCH', URL, data={'foo': 1})
+        target = _MockObject()
+        response, content = batch._make_request('PATCH', URL, data={'foo': 1},
+                                                target_object=target)
         self.assertEqual(response.status, 204)
         self.assertTrue(isinstance(content, _FutureDict))
-        self.assertEqual(content._value, None)
+        self.assertEqual(target._properties, content)
         self.assertEqual(http._requests, [])
         EXPECTED_HEADERS = [
             ('Accept-Encoding', 'gzip'),
@@ -179,10 +185,12 @@ def test__make_request_DELETE_normal(self):
         http = _HTTP()  # no requests expected
         connection = _Connection(http=http)
         batch = self._makeOne(connection)
-        response, content = batch._make_request('DELETE', URL)
+        target = _MockObject()
+        response, content = batch._make_request('DELETE', URL,
+                                                target_object=target)
         self.assertEqual(response.status, 204)
         self.assertTrue(isinstance(content, _FutureDict))
-        self.assertEqual(content._value, None)
+        self.assertEqual(target._properties, content)
         self.assertEqual(http._requests, [])
         EXPECTED_HEADERS = [
             ('Accept-Encoding', 'gzip'),
@@ -256,9 +264,9 @@ def test_finish_nonempty(self):
         connection = _Connection(http=http)
         batch = self._makeOne(connection)
         batch.API_BASE_URL = 'http://api.example.com'
-        batch._do_request('POST', URL, {}, {'foo': 1, 'bar': 2})
-        batch._do_request('PATCH', URL, {}, {'bar': 3})
-        batch._do_request('DELETE', URL, {}, None)
+        batch._do_request('POST', URL, {}, {'foo': 1, 'bar': 2}, None)
+        batch._do_request('PATCH', URL, {}, {'bar': 3}, None)
+        batch._do_request('DELETE', URL, {}, None, None)
         result = batch.finish()
         self.assertEqual(len(result), len(batch._requests))
         response0 = httplib2.Response({
@@ -317,15 +325,17 @@ def test_finish_nonempty_with_status_failure(self):
         connection = _Connection(http=http)
         batch = self._makeOne(connection)
         batch.API_BASE_URL = 'http://api.example.com'
-        batch._do_request('GET', URL, {}, None)
-        batch._do_request('GET', URL, {}, None)
+        target1 = _MockObject()
+        target2 = _MockObject()
+        batch._do_request('GET', URL, {}, None, target1)
+        batch._do_request('GET', URL, {}, None, target2)
         # Make sure futures are not populated.
-        self.assertEqual([future.owner for future in batch._futures],
-                         [None, None])
+        self.assertEqual([future for future in batch._futures],
+                         [target1, target2])
         self.assertRaises(NotFound, batch.finish)
-        self.assertEqual(batch._futures[0]._value,
+        self.assertEqual(target1._properties,
                          {'foo': 1, 'bar': 2})
-        self.assertEqual(batch._futures[1]._value,
+        self.assertEqual(target2._properties,
                          {u'error': {u'message': u'Not Found'}})
 
         self.assertEqual(len(http._requests), 1)
@@ -369,25 +379,31 @@ def test_as_context_mgr_wo_error(self):
 
         self.assertEqual(list(_BATCHES), [])
 
+        target1 = _MockObject()
+        target2 = _MockObject()
+        target3 = _MockObject()
         with self._makeOne(connection) as batch:
             self.assertEqual(list(_BATCHES), [batch])
-            batch._make_request('POST', URL, {'foo': 1, 'bar': 2})
-            batch._make_request('PATCH', URL, {'bar': 3})
-            batch._make_request('DELETE', URL)
+            batch._make_request('POST', URL, {'foo': 1, 'bar': 2},
+                                target_object=target1)
+            batch._make_request('PATCH', URL, {'bar': 3},
+                                target_object=target2)
+            batch._make_request('DELETE', URL, target_object=target3)
 
         self.assertEqual(list(_BATCHES), [])
         self.assertEqual(len(batch._requests), 3)
         self.assertEqual(batch._requests[0][0], 'POST')
         self.assertEqual(batch._requests[1][0], 'PATCH')
         self.assertEqual(batch._requests[2][0], 'DELETE')
-        self.assertEqual(len(batch._futures), 3)
-        self.assertEqual(batch._futures[0]._value,
+        self.assertEqual(batch._futures, [target1, target2, target3])
+        self.assertEqual(target1._properties,
                          {'foo': 1, 'bar': 2})
-        self.assertEqual(batch._futures[1]._value,
+        self.assertEqual(target2._properties,
                          {'foo': 1, 'bar': 3})
-        self.assertEqual(batch._futures[2]._value, '')
+        self.assertEqual(target3._properties, '')
 
     def test_as_context_mgr_w_error(self):
+        from gcloud.storage.batch import _FutureDict
         from gcloud.storage.batch import _BATCHES
         URL = 'http://example.com/api'
         http = _HTTP()
@@ -395,12 +411,17 @@ def test_as_context_mgr_w_error(self):
 
         self.assertEqual(list(_BATCHES), [])
 
+        target1 = _MockObject()
+        target2 = _MockObject()
+        target3 = _MockObject()
         try:
             with self._makeOne(connection) as batch:
                 self.assertEqual(list(_BATCHES), [batch])
-                batch._make_request('POST', URL, {'foo': 1, 'bar': 2})
-                batch._make_request('PATCH', URL, {'bar': 3})
-                batch._make_request('DELETE', URL)
+                batch._make_request('POST', URL, {'foo': 1, 'bar': 2},
+                                    target_object=target1)
+                batch._make_request('PATCH', URL, {'bar': 3},
+                                    target_object=target2)
+                batch._make_request('DELETE', URL, target_object=target3)
                 raise ValueError()
         except ValueError:
             pass
@@ -408,8 +429,12 @@ 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([future._value for future in batch._futures],
-                         [None, None, None])
+        self.assertEqual(batch._futures, [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))
+        self.assertTrue(isinstance(target2._properties, _FutureDict))
+        self.assertTrue(isinstance(target3._properties, _FutureDict))
 
 
 class Test__unpack_batch_response(unittest2.TestCase):
@@ -509,15 +534,6 @@ def _makeOne(self, *args, **kw):
         from gcloud.storage.batch import _FutureDict
         return _FutureDict(*args, **kw)
 
-    def test_ctor_defaults(self):
-        future = self._makeOne()
-        self.assertEqual(future.owner, None)
-
-    def test_ctor_owner(self):
-        OWNER = object()
-        future = self._makeOne(owner=OWNER)
-        self.assertTrue(future.owner is OWNER)
-
     def test_get(self):
         future = self._makeOne()
         self.assertRaises(KeyError, future.get, None)
@@ -534,31 +550,6 @@ def test___setitem__(self):
         with self.assertRaises(KeyError):
             future[None] = None
 
-    def test_set_future_value_no_owner(self):
-        future = self._makeOne()
-        self.assertEqual(future._value, None)
-        VALUE = object()
-        future.set_future_value(VALUE)
-        self.assertTrue(future._value is VALUE)
-
-    def test_set_future_value_with_owner(self):
-        from gcloud.storage._helpers import _PropertyMixin
-        future = self._makeOne()
-        future.owner = _PropertyMixin()
-        self.assertEqual(future._value, None)
-        self.assertEqual(future.owner._properties, {})
-        VALUE = object()
-        future.set_future_value(VALUE)
-        self.assertTrue(future._value is VALUE)
-        self.assertTrue(future.owner._properties is VALUE)
-
-    def test_set_future_value_already_set(self):
-        future = self._makeOne()
-        self.assertEqual(future._value, None)
-        VALUE = object()
-        future.set_future_value(VALUE)
-        self.assertRaises(ValueError, future.set_future_value, VALUE)
-
 
 class _Connection(object):
 
@@ -601,3 +592,7 @@ def request(self, method, uri, headers, body):
         self._requests.append((method, uri, headers, body))
         response, self._responses = self._responses[0], self._responses[1:]
         return response
+
+
+class _MockObject(object):
+    pass

From eb8c984df5f9f5dcbb7442ebb97c12a139e4fade Mon Sep 17 00:00:00 2001
From: Danny Hermes <daniel.j.hermes@gmail.com>
Date: Mon, 20 Apr 2015 23:48:27 -0700
Subject: [PATCH 4/6] Sending along callers of JSONConnection.api_request as
 _target_object.

---
 gcloud/storage/_helpers.py    |  5 +++--
 gcloud/storage/batch.py       |  4 ++--
 gcloud/storage/blob.py        |  3 ++-
 gcloud/storage/bucket.py      | 35 ++++++++++++++++++++++++++---------
 gcloud/storage/test_bucket.py | 26 ++++++++++++++++++++++----
 5 files changed, 55 insertions(+), 18 deletions(-)

diff --git a/gcloud/storage/_helpers.py b/gcloud/storage/_helpers.py
index 6c48e5aa19b5..a5acb45c645e 100644
--- a/gcloud/storage/_helpers.py
+++ b/gcloud/storage/_helpers.py
@@ -60,7 +60,8 @@ def reload(self, connection=None):
         # are handled via custom endpoints.
         query_params = {'projection': 'noAcl'}
         api_response = connection.api_request(
-            method='GET', path=self.path, query_params=query_params)
+            method='GET', path=self.path, query_params=query_params,
+            _target_object=self)
         self._set_properties(api_response)
 
     def _patch_property(self, name, value):
@@ -108,7 +109,7 @@ def patch(self, connection=None):
                                  for key in self._changes)
         api_response = connection.api_request(
             method='PATCH', path=self.path, data=update_properties,
-            query_params={'projection': 'full'})
+            query_params={'projection': 'full'}, _target_object=self)
         self._set_properties(api_response)
 
 
diff --git a/gcloud/storage/batch.py b/gcloud/storage/batch.py
index a73d8abcd1db..37096551026a 100644
--- a/gcloud/storage/batch.py
+++ b/gcloud/storage/batch.py
@@ -235,8 +235,8 @@ def finish(self):
 
         url = '%s/batch' % self.API_BASE_URL
 
-        _req = self._connection._make_request
-        response, content = _req('POST', url, data=body, headers=headers)
+        response, content = self._connection._make_request(
+            'POST', url, data=body, headers=headers)
         responses = list(_unpack_batch_response(response, content))
         self._finish_futures(responses)
         return responses
diff --git a/gcloud/storage/blob.py b/gcloud/storage/blob.py
index ed6a00dda966..dd55525a6e25 100644
--- a/gcloud/storage/blob.py
+++ b/gcloud/storage/blob.py
@@ -227,7 +227,8 @@ def exists(self, connection=None):
             # minimize the returned payload.
             query_params = {'fields': 'name'}
             connection.api_request(method='GET', path=self.path,
-                                   query_params=query_params)
+                                   query_params=query_params,
+                                   _target_object=self)
             return True
         except NotFound:
             return False
diff --git a/gcloud/storage/bucket.py b/gcloud/storage/bucket.py
index efdb7b567c7f..6d1a502d5c95 100644
--- a/gcloud/storage/bucket.py
+++ b/gcloud/storage/bucket.py
@@ -131,8 +131,14 @@ def exists(self, connection=None):
             # We only need the status code (200 or not) so we seek to
             # minimize the returned payload.
             query_params = {'fields': 'name'}
+            # We intentionally pass `_target_object=None` since fields=name
+            # would limit the local properties.
             connection.api_request(method='GET', path=self.path,
-                                   query_params=query_params)
+                                   query_params=query_params,
+                                   _target_object=None)
+            # NOTE: This will not fail immediately in a batch. However, when
+            #       Batch.finish() is called, the resulting `NotFound` will be
+            #       raised.
             return True
         except NotFound:
             return False
@@ -169,7 +175,7 @@ def create(self, project=None, connection=None):
         query_params = {'project': project}
         api_response = connection.api_request(
             method='POST', path='/b', query_params=query_params,
-            data={'name': self.name})
+            data={'name': self.name}, _target_object=self)
         self._set_properties(api_response)
 
     @property
@@ -238,11 +244,13 @@ def get_blob(self, blob_name, connection=None):
         connection = _require_connection(connection)
         blob = Blob(bucket=self, name=blob_name)
         try:
-            response = connection.api_request(method='GET',
-                                              path=blob.path)
-            name = response.get('name')  # Expect this to be blob_name
-            blob = Blob(name, bucket=self)
+            response = connection.api_request(
+                method='GET', path=blob.path, _target_object=blob)
+            # NOTE: We assume response.get('name') matches `blob_name`.
             blob._set_properties(response)
+            # NOTE: This will not fail immediately in a batch. However, when
+            #       Batch.finish() is called, the resulting `NotFound` will be
+            #       raised.
             return blob
         except NotFound:
             return None
@@ -354,7 +362,11 @@ def delete(self, force=False, connection=None):
             self.delete_blobs(blobs, on_error=lambda blob: None,
                               connection=connection)
 
-        connection.api_request(method='DELETE', path=self.path)
+        # We intentionally pass `_target_object=None` since a DELETE
+        # request has no response value (whether in a standard request or
+        # in a batch request).
+        connection.api_request(method='DELETE', path=self.path,
+                               _target_object=None)
 
     def delete_blob(self, blob_name, connection=None):
         """Deletes a blob from the current bucket.
@@ -392,7 +404,11 @@ def delete_blob(self, blob_name, connection=None):
         """
         connection = _require_connection(connection)
         blob_path = Blob.path_helper(self.path, blob_name)
-        connection.api_request(method='DELETE', path=blob_path)
+        # We intentionally pass `_target_object=None` since a DELETE
+        # request has no response value (whether in a standard request or
+        # in a batch request).
+        connection.api_request(method='DELETE', path=blob_path,
+                               _target_object=None)
 
     def delete_blobs(self, blobs, on_error=None, connection=None):
         """Deletes a list of blobs from the current bucket.
@@ -456,7 +472,8 @@ def copy_blob(blob, destination_bucket, new_name=None,
             new_name = blob.name
         new_blob = Blob(bucket=destination_bucket, name=new_name)
         api_path = blob.path + '/copyTo' + new_blob.path
-        copy_result = connection.api_request(method='POST', path=api_path)
+        copy_result = connection.api_request(method='POST', path=api_path,
+                                             _target_object=new_blob)
         new_blob._set_properties(copy_result)
         return new_blob
 
diff --git a/gcloud/storage/test_bucket.py b/gcloud/storage/test_bucket.py
index 4f31859c7970..3cbfac8d836f 100644
--- a/gcloud/storage/test_bucket.py
+++ b/gcloud/storage/test_bucket.py
@@ -162,6 +162,7 @@ def api_request(cls, *args, **kwargs):
             'query_params': {
                 'fields': 'name',
             },
+            '_target_object': None,
         }
         expected_cw = [((), expected_called_kwargs)]
         self.assertEqual(_FakeConnection._called_with, expected_cw)
@@ -186,6 +187,7 @@ def api_request(cls, *args, **kwargs):
             'query_params': {
                 'fields': 'name',
             },
+            '_target_object': None,
         }
         expected_cw = [((), expected_called_kwargs)]
         self.assertEqual(_FakeConnection._called_with, expected_cw)
@@ -330,7 +332,11 @@ def test_delete_default_miss(self):
         connection = _Connection()
         bucket = self._makeOne(NAME)
         self.assertRaises(NotFound, bucket.delete, connection=connection)
-        expected_cw = [{'method': 'DELETE', 'path': bucket.path}]
+        expected_cw = [{
+            'method': 'DELETE',
+            'path': bucket.path,
+            '_target_object': None,
+        }]
         self.assertEqual(connection._deleted_buckets, expected_cw)
 
     def test_delete_explicit_hit(self):
@@ -341,7 +347,11 @@ def test_delete_explicit_hit(self):
         bucket = self._makeOne(NAME, connection)
         result = bucket.delete(force=True, connection=connection)
         self.assertTrue(result is None)
-        expected_cw = [{'method': 'DELETE', 'path': bucket.path}]
+        expected_cw = [{
+            'method': 'DELETE',
+            'path': bucket.path,
+            '_target_object': None,
+        }]
         self.assertEqual(connection._deleted_buckets, expected_cw)
 
     def test_delete_explicit_force_delete_blobs(self):
@@ -361,7 +371,11 @@ def test_delete_explicit_force_delete_blobs(self):
         bucket = self._makeOne(NAME, connection)
         result = bucket.delete(force=True, connection=connection)
         self.assertTrue(result is None)
-        expected_cw = [{'method': 'DELETE', 'path': bucket.path}]
+        expected_cw = [{
+            'method': 'DELETE',
+            'path': bucket.path,
+            '_target_object': None,
+        }]
         self.assertEqual(connection._deleted_buckets, expected_cw)
 
     def test_delete_explicit_force_miss_blobs(self):
@@ -374,7 +388,11 @@ def test_delete_explicit_force_miss_blobs(self):
         bucket = self._makeOne(NAME, connection)
         result = bucket.delete(force=True, connection=connection)
         self.assertTrue(result is None)
-        expected_cw = [{'method': 'DELETE', 'path': bucket.path}]
+        expected_cw = [{
+            'method': 'DELETE',
+            'path': bucket.path,
+            '_target_object': None,
+        }]
         self.assertEqual(connection._deleted_buckets, expected_cw)
 
     def test_delete_explicit_too_many(self):

From d89f68460576b7c75191e1209d30e34b1d903016 Mon Sep 17 00:00:00 2001
From: Danny Hermes <daniel.j.hermes@gmail.com>
Date: Thu, 23 Apr 2015 12:31:43 -0700
Subject: [PATCH 5/6] Addressing content-type and renaming Batch._futures.

NOTE: This commit made for review but will be folded into
      originals before merged.
---
 gcloud/connection.py         |  8 ++++----
 gcloud/storage/batch.py      | 11 ++++++-----
 gcloud/storage/test_batch.py | 22 +++++++++++-----------
 gcloud/test_connection.py    |  2 +-
 4 files changed, 22 insertions(+), 21 deletions(-)

diff --git a/gcloud/connection.py b/gcloud/connection.py
index 2cea649cfe26..eceee2ee8520 100644
--- a/gcloud/connection.py
+++ b/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
diff --git a/gcloud/storage/batch.py b/gcloud/storage/batch.py
index 37096551026a..151e99a18888 100644
--- a/gcloud/storage/batch.py
+++ b/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:
diff --git a/gcloud/storage/test_batch.py b/gcloud/storage/test_batch.py
index 1c51d2c75532..94695671090e 100644
--- a/gcloud/storage/test_batch.py
+++ b/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))
diff --git a/gcloud/test_connection.py b/gcloud/test_connection.py
index 446ffbaffaa3..06f70f3e8890 100644
--- a/gcloud/test_connection.py
+++ b/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

From 45649920a7afac4288d8d5610b2917573bd9670d Mon Sep 17 00:00:00 2001
From: Danny Hermes <daniel.j.hermes@gmail.com>
Date: Mon, 4 May 2015 11:33:18 -0700
Subject: [PATCH 6/6] Dropping `dummy` argument for target_object.

Had to trade-off here and use `# pylint: disable=unused-argument`.
---
 gcloud/connection.py | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/gcloud/connection.py b/gcloud/connection.py
index eceee2ee8520..d411b04e4b82 100644
--- a/gcloud/connection.py
+++ b/gcloud/connection.py
@@ -208,7 +208,8 @@ def _make_request(self, method, url, data=None, content_type=None,
 
         return self._do_request(method, url, headers, data, target_object)
 
-    def _do_request(self, method, url, headers, data, dummy):
+    def _do_request(self, method, url, headers, data,
+                    target_object):  # pylint: disable=unused-argument
         """Low-level helper:  perform the actual API request over HTTP.
 
         Allows batch context managers to override and defer a request.
@@ -225,9 +226,9 @@ def _do_request(self, method, url, headers, data, dummy):
         :type data: string
         :param data: The data to send as the body of the request.
 
-        :type dummy: object or :class:`NoneType`
-        :param dummy: Unused ``target_object`` here but may be used
-                      by a superclass.
+        :type target_object: object or :class:`NoneType`
+        :param target_object: Unused ``target_object`` here but may be used
+                              by a superclass.
 
         :rtype: tuple of ``response`` (a dictionary of sorts)
                 and ``content`` (a string).