Making regression3 pass by using bytes / unicode correctly.

- Incorporates changes from googleapis#724.
- Also requires httplib2 from HEAD since the bytes/unicode
  header issues have not been released on PyPI yet.

gcloud/connection.py

@@ -16,6 +16,7 @@
 
 import json
 from pkg_resources import get_distribution
+import six
 from six.moves.urllib.parse import urlencode  # pylint: disable=F0401
 
 import httplib2
@@ -295,6 +296,8 @@ def api_request(self, method, path, query_params=None,
             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/credentials.py

@@ -181,6 +181,8 @@ def _get_signed_query_params(credentials, expiration, signature_string):
     pem_key = _get_pem_key(credentials)
     # Sign the string with the RSA key.
     signer = PKCS1_v1_5.new(pem_key)
+    if not isinstance(signature_string, six.binary_type):
+        signature_string = signature_string.encode('utf-8')
     signature_hash = SHA256.new(signature_string)
     signature_bytes = signer.sign(signature_hash)
     signature = base64.b64encode(signature_bytes)

gcloud/datastore/test_connection.py

@@ -152,7 +152,7 @@ def test__request_not_200(self):
         METHOD = 'METHOD'
         DATA = 'DATA'
         conn = self._makeOne()
-        conn._http = Http({'status': '400'}, 'Entity value is indexed.')
+        conn._http = Http({'status': '400'}, b'Entity value is indexed.')
         with self.assertRaises(BadRequest) as e:
             conn._request(DATASET_ID, METHOD, DATA)
         expected_message = '400 Entity value is indexed.'

gcloud/exceptions.py

@@ -18,6 +18,7 @@
 """
 
 import json
+import six
 
 _HTTP_CODE_TO_EXCEPTION = {}  # populated at end of module
 
@@ -171,18 +172,18 @@ def make_exception(response, content, use_json=True):
     :rtype: instance of :class:`GCloudError`, or a concrete subclass.
     :returns: Exception specific to the error response.
     """
-    message = content
-    errors = ()
+    if isinstance(content, six.binary_type):
+        content = content.decode('utf-8')
 
-    if isinstance(content, str):
+    if isinstance(content, six.string_types):
         if use_json:
             payload = json.loads(content)
         else:
-            payload = {}
+            payload = {'message': content}
     else:
         payload = content
 
-    message = payload.get('message', message)
+    message = payload.get('message', '')
     errors = payload.get('error', {}).get('errors', ())
 
     try:

gcloud/storage/batch.py

@@ -170,8 +170,14 @@ def __exit__(self, exc_type, exc_val, exc_tb):
 def _unpack_batch_response(response, content):
     """Convert response, content -> [(status, reason, payload)]."""
     parser = Parser()
-    faux_message = ('Content-Type: %s\nMIME-Version: 1.0\n\n%s' %
-                    (response['content-type'], content))
+    if isinstance(content, six.binary_type):
+        content = content.decode('utf-8')
+    faux_message = ''.join([
+        'Content-Type: ',
+        response['content-type'],
+        '\nMIME-Version: 1.0\n\n',
+        content,
+    ])
 
     message = parser.parsestr(faux_message)
 

gcloud/storage/test_api.py

@@ -34,7 +34,7 @@ def test_miss(self):
         ])
         http = conn._http = Http(
             {'status': '404', 'content-type': 'application/json'},
-            '{}',
+            b'{}',
         )
         bucket = self._callFUT(NONESUCH, connection=conn)
         self.assertEqual(bucket, None)
@@ -56,7 +56,7 @@ def _lookup_bucket_hit_helper(self, use_default=False):
         ])
         http = conn._http = Http(
             {'status': '200', 'content-type': 'application/json'},
-            '{"name": "%s"}' % BLOB_NAME,
+            '{{"name": "{0}"}}'.format(BLOB_NAME).encode('utf-8'),
         )
 
         if use_default:
@@ -96,7 +96,7 @@ def test_empty(self):
         ])
         http = conn._http = Http(
             {'status': '200', 'content-type': 'application/json'},
-            '{}',
+            b'{}',
         )
         buckets = list(self._callFUT(PROJECT, conn))
         self.assertEqual(len(buckets), 0)
@@ -117,7 +117,8 @@ def _get_all_buckets_non_empty_helper(self, project, use_default=False):
         ])
         http = conn._http = Http(
             {'status': '200', 'content-type': 'application/json'},
-            '{"items": [{"name": "%s"}]}' % BUCKET_NAME,
+            '{{"items": [{{"name": "{0}"}}]}}'.format(BUCKET_NAME)
+            .encode('utf-8'),
         )
 
         if use_default:
@@ -159,7 +160,7 @@ def test_miss(self):
         ])
         http = conn._http = Http(
             {'status': '404', 'content-type': 'application/json'},
-            '{}',
+            b'{}',
         )
         self.assertRaises(NotFound, self._callFUT, NONESUCH, connection=conn)
         self.assertEqual(http._called_with['method'], 'GET')
@@ -180,7 +181,7 @@ def _get_bucket_hit_helper(self, use_default=False):
         ])
         http = conn._http = Http(
             {'status': '200', 'content-type': 'application/json'},
-            '{"name": "%s"}' % BLOB_NAME,
+            '{{"name": "{0}"}}'.format(BLOB_NAME).encode('utf-8'),
         )
 
         if use_default:
@@ -224,7 +225,7 @@ def _create_bucket_success_helper(self, project, use_default=False):
             ])
         http = conn._http = Http(
             {'status': '200', 'content-type': 'application/json'},
-            '{"name": "%s"}' % BLOB_NAME,
+            '{{"name": "{0}"}}'.format(BLOB_NAME).encode('utf-8'),
         )
 
         if use_default:

gcloud/test_connection.py

@@ -161,12 +161,12 @@ def test__make_request_no_data_no_content_type_no_headers(self):
         URI = 'http://example.com/test'
         http = conn._http = _Http(
             {'status': '200', 'content-type': 'text/plain'},
-            '',
+            b'',
         )
         headers, content = conn._make_request('GET', URI)
         self.assertEqual(headers['status'], '200')
         self.assertEqual(headers['content-type'], 'text/plain')
-        self.assertEqual(content, '')
+        self.assertEqual(content, b'')
         self.assertEqual(http._called_with['method'], 'GET')
         self.assertEqual(http._called_with['uri'], URI)
         self.assertEqual(http._called_with['body'], None)
@@ -182,7 +182,7 @@ def test__make_request_w_data_no_extra_headers(self):
         URI = 'http://example.com/test'
         http = conn._http = _Http(
             {'status': '200', 'content-type': 'text/plain'},
-            '',
+            b'',
         )
         conn._make_request('GET', URI, {}, 'application/json')
         self.assertEqual(http._called_with['method'], 'GET')
@@ -201,7 +201,7 @@ def test__make_request_w_extra_headers(self):
         URI = 'http://example.com/test'
         http = conn._http = _Http(
             {'status': '200', 'content-type': 'text/plain'},
-            '',
+            b'',
         )
         conn._make_request('GET', URI, headers={'X-Foo': 'foo'})
         self.assertEqual(http._called_with['method'], 'GET')
@@ -226,7 +226,7 @@ def test_api_request_defaults(self):
         ])
         http = conn._http = _Http(
             {'status': '200', 'content-type': 'application/json'},
-            '{}',
+            b'{}',
         )
         self.assertEqual(conn.api_request('GET', PATH), {})
         self.assertEqual(http._called_with['method'], 'GET')
@@ -243,7 +243,7 @@ def test_api_request_w_non_json_response(self):
         conn = self._makeMockOne()
         conn._http = _Http(
             {'status': '200', 'content-type': 'text/plain'},
-            'CONTENT',
+            b'CONTENT',
         )
 
         self.assertRaises(TypeError, conn.api_request, 'GET', '/')
@@ -252,18 +252,18 @@ def test_api_request_wo_json_expected(self):
         conn = self._makeMockOne()
         conn._http = _Http(
             {'status': '200', 'content-type': 'text/plain'},
-            'CONTENT',
+            b'CONTENT',
         )
         self.assertEqual(conn.api_request('GET', '/', expect_json=False),
-                         'CONTENT')
+                         b'CONTENT')
 
     def test_api_request_w_query_params(self):
         from six.moves.urllib.parse import parse_qsl
         from six.moves.urllib.parse import urlsplit
         conn = self._makeMockOne()
         http = conn._http = _Http(
             {'status': '200', 'content-type': 'application/json'},
-            '{}',
+            b'{}',
         )
         self.assertEqual(conn.api_request('GET', '/', {'foo': 'bar'}), {})
         self.assertEqual(http._called_with['method'], 'GET')
@@ -302,7 +302,7 @@ def test_api_request_w_data(self):
         ])
         http = conn._http = _Http(
             {'status': '200', 'content-type': 'application/json'},
-            '{}',
+            b'{}',
         )
         self.assertEqual(conn.api_request('POST', '/', data=DATA), {})
         self.assertEqual(http._called_with['method'], 'POST')
@@ -321,7 +321,7 @@ def test_api_request_w_404(self):
         conn = self._makeMockOne()
         conn._http = _Http(
             {'status': '404', 'content-type': 'text/plain'},
-            '{}'
+            b'{}'
         )
         self.assertRaises(NotFound, conn.api_request, 'GET', '/')
 
@@ -330,10 +330,35 @@ def test_api_request_w_500(self):
         conn = self._makeMockOne()
         conn._http = _Http(
             {'status': '500', 'content-type': 'text/plain'},
-            '{}',
+            b'{}',
         )
         self.assertRaises(InternalServerError, conn.api_request, 'GET', '/')
 
+    def test_api_request_non_binary_response(self):
+        conn = self._makeMockOne()
+        http = conn._http = _Http(
+            {'status': '200', 'content-type': 'application/json'},
+            u'{}',
+        )
+        result = conn.api_request('GET', '/')
+        # Intended to emulate self.mock_template
+        URI = '/'.join([
+            conn.API_BASE_URL,
+            'mock',
+            conn.API_VERSION,
+            '',
+        ])
+        self.assertEqual(result, {})
+        self.assertEqual(http._called_with['method'], 'GET')
+        self.assertEqual(http._called_with['uri'], URI)
+        self.assertEqual(http._called_with['body'], None)
+        expected_headers = {
+            'Accept-Encoding': 'gzip',
+            'Content-Length': 0,
+            'User-Agent': conn.USER_AGENT,
+        }
+        self.assertEqual(http._called_with['headers'], expected_headers)
+
 
 class _Http(object):
 

gcloud/test_exceptions.py

@@ -55,7 +55,7 @@ def _callFUT(self, response, content):
     def test_hit_w_content_as_str(self):
         from gcloud.exceptions import NotFound
         response = _Response(404)
-        content = '{"message": "Not Found"}'
+        content = b'{"message": "Not Found"}'
         exception = self._callFUT(response, content)
         self.assertTrue(isinstance(exception, NotFound))
         self.assertEqual(exception.message, 'Not Found')

regression/storage.py

@@ -13,6 +13,7 @@
 # limitations under the License.
 
 import httplib2
+import six
 import tempfile
 import time
 import unittest2
@@ -122,7 +123,10 @@ def test_large_file_write_from_stream(self):
             blob.upload_from_file(file_obj)
             self.case_blobs_to_delete.append(blob)
 
-        self.assertEqual(blob.md5_hash, file_data['hash'])
+        md5_hash = blob.md5_hash
+        if not isinstance(md5_hash, six.binary_type):
+            md5_hash = md5_hash.encode('utf-8')
+        self.assertEqual(md5_hash, file_data['hash'])
 
     def test_small_file_write_from_filename(self):
         blob = storage.Blob(bucket=self.bucket, name='SmallFile')
@@ -132,7 +136,10 @@ def test_small_file_write_from_filename(self):
         blob.upload_from_filename(file_data['path'])
         self.case_blobs_to_delete.append(blob)
 
-        self.assertEqual(blob.md5_hash, file_data['hash'])
+        md5_hash = blob.md5_hash
+        if not isinstance(md5_hash, six.binary_type):
+            md5_hash = md5_hash.encode('utf-8')
+        self.assertEqual(md5_hash, file_data['hash'])
 
     def test_write_metadata(self):
         blob = self.bucket.upload_file(self.FILES['logo']['path'])
@@ -145,14 +152,14 @@ def test_write_metadata(self):
 
     def test_direct_write_and_read_into_file(self):
         blob = storage.Blob(bucket=self.bucket, name='MyBuffer')
-        file_contents = 'Hello World'
+        file_contents = b'Hello World'
         blob.upload_from_string(file_contents)
         self.case_blobs_to_delete.append(blob)
 
         same_blob = storage.Blob(bucket=self.bucket, name='MyBuffer')
         same_blob._reload_properties()  # Initialize properties.
         temp_filename = tempfile.mktemp()
-        with open(temp_filename, 'w') as file_obj:
+        with open(temp_filename, 'wb') as file_obj:
             same_blob.download_to_file(file_obj)
 
         with open(temp_filename, 'rb') as file_obj:
@@ -299,7 +306,7 @@ def setUp(self):
         super(TestStorageSignURLs, self).setUp()
 
         logo_path = self.FILES['logo']['path']
-        with open(logo_path, 'r') as file_obj:
+        with open(logo_path, 'rb') as file_obj:
             self.LOCAL_FILE = file_obj.read()
 
         blob = storage.Blob(bucket=self.bucket, name='LogoToSign.jpg')
@@ -328,7 +335,7 @@ def test_create_signed_delete_url(self):
 
         response, content = HTTP.request(signed_delete_url, method='DELETE')
         self.assertEqual(response.status, 204)
-        self.assertEqual(content, '')
+        self.assertEqual(content, b'')
 
         # Check that the blob has actually been deleted.
         self.assertFalse(blob.name in self.bucket)

tox.ini

@@ -74,4 +74,8 @@ commands =
     {toxinidir}/scripts/run_regression.sh
 deps =
     unittest2
+    # Use a development checkout of httplib2 until a release is made
+    # incorporating https://github.com/jcgregorio/httplib2/pull/291
+    # and https://github.com/jcgregorio/httplib2/pull/296
+    -egit+https://github.com/jcgregorio/httplib2.git#egg=httplib2
     protobuf>=3.0.0-alpha-1