From 35c78ddc2dd101df7d36166219ba7a00a60f4ceb Mon Sep 17 00:00:00 2001 From: Craig Loftus Date: Wed, 11 Mar 2015 09:43:30 +0000 Subject: [PATCH 1/3] Ensuring content returned by httplib2 in gcloud.storage.connection.Connection.api_request is decoded to a string before being passed to json.loads --- gcloud/storage/connection.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gcloud/storage/connection.py b/gcloud/storage/connection.py index d00b37b8d24e..466f12794e38 100644 --- a/gcloud/storage/connection.py +++ b/gcloud/storage/connection.py @@ -264,7 +264,7 @@ 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) - return json.loads(content) + return json.loads(content.decode('utf-8')) return content From 4376eaaa2307dd9c8c664052cccd3104c1e0f178 Mon Sep 17 00:00:00 2001 From: Craig Loftus Date: Wed, 11 Mar 2015 14:12:48 +0000 Subject: [PATCH 2/3] Changing responses from Http mock in gcloud.storage.test_connection to use six.binary_type --- gcloud/storage/test_connection.py | 48 ++++++++++++++++++++----------- 1 file changed, 32 insertions(+), 16 deletions(-) diff --git a/gcloud/storage/test_connection.py b/gcloud/storage/test_connection.py index 1073f0969d18..b97fb49c2387 100644 --- a/gcloud/storage/test_connection.py +++ b/gcloud/storage/test_connection.py @@ -101,12 +101,13 @@ def test_build_api_url_w_upload(self): self.assertEqual(conn.build_api_url('/foo', upload=True), URI) def test__make_request_no_data_no_content_type_no_headers(self): + import six PROJECT = 'project' conn = self._makeOne(PROJECT) URI = 'http://example.com/test' http = conn._http = Http( {'status': '200', 'content-type': 'text/plain'}, - '', + six.binary_type(''), ) headers, content = conn._make_request('GET', URI) self.assertEqual(headers['status'], '200') @@ -123,12 +124,13 @@ def test__make_request_no_data_no_content_type_no_headers(self): self.assertEqual(http._called_with['headers'], expected_headers) def test__make_request_w_data_no_extra_headers(self): + import six PROJECT = 'project' conn = self._makeOne(PROJECT) URI = 'http://example.com/test' http = conn._http = Http( {'status': '200', 'content-type': 'text/plain'}, - '', + six.binary_type(''), ) conn._make_request('GET', URI, {}, 'application/json') self.assertEqual(http._called_with['method'], 'GET') @@ -143,12 +145,13 @@ def test__make_request_w_data_no_extra_headers(self): self.assertEqual(http._called_with['headers'], expected_headers) def test__make_request_w_extra_headers(self): + import six PROJECT = 'project' conn = self._makeOne(PROJECT) URI = 'http://example.com/test' http = conn._http = Http( {'status': '200', 'content-type': 'text/plain'}, - '', + six.binary_type(''), ) conn._make_request('GET', URI, headers={'X-Foo': 'foo'}) self.assertEqual(http._called_with['method'], 'GET') @@ -163,6 +166,7 @@ def test__make_request_w_extra_headers(self): self.assertEqual(http._called_with['headers'], expected_headers) def test_api_request_defaults(self): + import six PROJECT = 'project' PATH = '/path/required' conn = self._makeOne(PROJECT) @@ -173,7 +177,7 @@ def test_api_request_defaults(self): ]) http = conn._http = Http( {'status': '200', 'content-type': 'application/json'}, - '{}', + six.binary_type('{}'), ) self.assertEqual(conn.api_request('GET', PATH), {}) self.assertEqual(http._called_with['method'], 'GET') @@ -187,21 +191,23 @@ def test_api_request_defaults(self): self.assertEqual(http._called_with['headers'], expected_headers) def test_api_request_w_non_json_response(self): + import six PROJECT = 'project' conn = self._makeOne(PROJECT) conn._http = Http( {'status': '200', 'content-type': 'text/plain'}, - 'CONTENT', + six.binary_type('CONTENT'), ) self.assertRaises(TypeError, conn.api_request, 'GET', '/') def test_api_request_wo_json_expected(self): + import six PROJECT = 'project' conn = self._makeOne(PROJECT) conn._http = Http( {'status': '200', 'content-type': 'text/plain'}, - 'CONTENT', + six.binary_type('CONTENT'), ) self.assertEqual(conn.api_request('GET', '/', expect_json=False), 'CONTENT') @@ -209,11 +215,12 @@ def test_api_request_wo_json_expected(self): def test_api_request_w_query_params(self): from six.moves.urllib.parse import parse_qsl from six.moves.urllib.parse import urlsplit + import six PROJECT = 'project' conn = self._makeOne(PROJECT) http = conn._http = Http( {'status': '200', 'content-type': 'application/json'}, - '{}', + six.binary_type('{}'), ) self.assertEqual(conn.api_request('GET', '/', {'foo': 'bar'}), {}) self.assertEqual(http._called_with['method'], 'GET') @@ -235,6 +242,7 @@ def test_api_request_w_query_params(self): def test_api_request_w_data(self): import json + import six PROJECT = 'project' DATA = {'foo': 'bar'} DATAJ = json.dumps(DATA) @@ -247,7 +255,7 @@ def test_api_request_w_data(self): ]) http = conn._http = Http( {'status': '200', 'content-type': 'application/json'}, - '{}', + six.binary_type('{}'), ) self.assertEqual(conn.api_request('POST', '/', data=DATA), {}) self.assertEqual(http._called_with['method'], 'POST') @@ -263,25 +271,28 @@ def test_api_request_w_data(self): def test_api_request_w_404(self): from gcloud.exceptions import NotFound + import six PROJECT = 'project' conn = self._makeOne(PROJECT) conn._http = Http( {'status': '404', 'content-type': 'text/plain'}, - '{}' + six.binary_type('{}') ) self.assertRaises(NotFound, conn.api_request, 'GET', '/') def test_api_request_w_500(self): from gcloud.exceptions import InternalServerError + import six PROJECT = 'project' conn = self._makeOne(PROJECT) conn._http = Http( {'status': '500', 'content-type': 'text/plain'}, - '{}', + six.binary_type('{}'), ) self.assertRaises(InternalServerError, conn.api_request, 'GET', '/') def test_get_all_buckets_empty(self): + import six PROJECT = 'project' conn = self._makeOne(PROJECT) URI = '/'.join([ @@ -292,7 +303,7 @@ def test_get_all_buckets_empty(self): ]) http = conn._http = Http( {'status': '200', 'content-type': 'application/json'}, - '{}', + six.binary_type('{}'), ) buckets = list(conn.get_all_buckets()) self.assertEqual(len(buckets), 0) @@ -300,6 +311,7 @@ def test_get_all_buckets_empty(self): self.assertEqual(http._called_with['uri'], URI) def test_get_all_buckets_non_empty(self): + import six PROJECT = 'project' BUCKET_NAME = 'bucket-name' conn = self._makeOne(PROJECT) @@ -311,7 +323,7 @@ def test_get_all_buckets_non_empty(self): ]) http = conn._http = Http( {'status': '200', 'content-type': 'application/json'}, - '{"items": [{"name": "%s"}]}' % BUCKET_NAME, + six.binary_type('{"items": [{"name": "%s"}]}' % BUCKET_NAME), ) buckets = list(conn.get_all_buckets()) self.assertEqual(len(buckets), 1) @@ -321,6 +333,7 @@ def test_get_all_buckets_non_empty(self): def test_get_bucket_miss(self): from gcloud.exceptions import NotFound + import six PROJECT = 'project' NONESUCH = 'nonesuch' conn = self._makeOne(PROJECT) @@ -333,7 +346,7 @@ def test_get_bucket_miss(self): ]) http = conn._http = Http( {'status': '404', 'content-type': 'application/json'}, - '{}', + six.binary_type('{}'), ) self.assertRaises(NotFound, conn.get_bucket, NONESUCH) self.assertEqual(http._called_with['method'], 'GET') @@ -341,6 +354,7 @@ def test_get_bucket_miss(self): def test_get_bucket_hit(self): from gcloud.storage.bucket import Bucket + import six PROJECT = 'project' BLOB_NAME = 'blob-name' conn = self._makeOne(PROJECT) @@ -353,7 +367,7 @@ def test_get_bucket_hit(self): ]) http = conn._http = Http( {'status': '200', 'content-type': 'application/json'}, - '{"name": "%s"}' % BLOB_NAME, + six.binary_type('{"name": "%s"}' % BLOB_NAME), ) bucket = conn.get_bucket(BLOB_NAME) self.assertTrue(isinstance(bucket, Bucket)) @@ -364,6 +378,7 @@ def test_get_bucket_hit(self): def test_create_bucket_ok(self): from gcloud.storage.bucket import Bucket + import six PROJECT = 'project' BLOB_NAME = 'blob-name' conn = self._makeOne(PROJECT) @@ -375,7 +390,7 @@ def test_create_bucket_ok(self): ]) http = conn._http = Http( {'status': '200', 'content-type': 'application/json'}, - '{"name": "%s"}' % BLOB_NAME, + six.binary_type('{"name": "%s"}' % BLOB_NAME), ) bucket = conn.create_bucket(BLOB_NAME) self.assertTrue(isinstance(bucket, Bucket)) @@ -385,6 +400,7 @@ def test_create_bucket_ok(self): self.assertEqual(http._called_with['uri'], URI) def test_delete_bucket_defaults_miss(self): + import six _deleted_blobs = [] PROJECT = 'project' @@ -399,7 +415,7 @@ def test_delete_bucket_defaults_miss(self): ]) http = conn._http = Http( {'status': '200', 'content-type': 'application/json'}, - '{}', + six.binary_type('{}'), ) self.assertEqual(conn.delete_bucket(BLOB_NAME), None) From 3bb23988caf099d11abc727b90fe4dda97f886f6 Mon Sep 17 00:00:00 2001 From: Craig Loftus Date: Wed, 11 Mar 2015 14:28:03 +0000 Subject: [PATCH 3/3] Apparently six.binary_type was the wrong approach --- gcloud/storage/test_connection.py | 48 +++++++++++-------------------- 1 file changed, 16 insertions(+), 32 deletions(-) diff --git a/gcloud/storage/test_connection.py b/gcloud/storage/test_connection.py index b97fb49c2387..5ce189dca5c3 100644 --- a/gcloud/storage/test_connection.py +++ b/gcloud/storage/test_connection.py @@ -101,13 +101,12 @@ def test_build_api_url_w_upload(self): self.assertEqual(conn.build_api_url('/foo', upload=True), URI) def test__make_request_no_data_no_content_type_no_headers(self): - import six PROJECT = 'project' conn = self._makeOne(PROJECT) URI = 'http://example.com/test' http = conn._http = Http( {'status': '200', 'content-type': 'text/plain'}, - six.binary_type(''), + b'', ) headers, content = conn._make_request('GET', URI) self.assertEqual(headers['status'], '200') @@ -124,13 +123,12 @@ def test__make_request_no_data_no_content_type_no_headers(self): self.assertEqual(http._called_with['headers'], expected_headers) def test__make_request_w_data_no_extra_headers(self): - import six PROJECT = 'project' conn = self._makeOne(PROJECT) URI = 'http://example.com/test' http = conn._http = Http( {'status': '200', 'content-type': 'text/plain'}, - six.binary_type(''), + b'', ) conn._make_request('GET', URI, {}, 'application/json') self.assertEqual(http._called_with['method'], 'GET') @@ -145,13 +143,12 @@ def test__make_request_w_data_no_extra_headers(self): self.assertEqual(http._called_with['headers'], expected_headers) def test__make_request_w_extra_headers(self): - import six PROJECT = 'project' conn = self._makeOne(PROJECT) URI = 'http://example.com/test' http = conn._http = Http( {'status': '200', 'content-type': 'text/plain'}, - six.binary_type(''), + b'', ) conn._make_request('GET', URI, headers={'X-Foo': 'foo'}) self.assertEqual(http._called_with['method'], 'GET') @@ -166,7 +163,6 @@ def test__make_request_w_extra_headers(self): self.assertEqual(http._called_with['headers'], expected_headers) def test_api_request_defaults(self): - import six PROJECT = 'project' PATH = '/path/required' conn = self._makeOne(PROJECT) @@ -177,7 +173,7 @@ def test_api_request_defaults(self): ]) http = conn._http = Http( {'status': '200', 'content-type': 'application/json'}, - six.binary_type('{}'), + b'{}', ) self.assertEqual(conn.api_request('GET', PATH), {}) self.assertEqual(http._called_with['method'], 'GET') @@ -191,23 +187,21 @@ def test_api_request_defaults(self): self.assertEqual(http._called_with['headers'], expected_headers) def test_api_request_w_non_json_response(self): - import six PROJECT = 'project' conn = self._makeOne(PROJECT) conn._http = Http( {'status': '200', 'content-type': 'text/plain'}, - six.binary_type('CONTENT'), + b'CONTENT', ) self.assertRaises(TypeError, conn.api_request, 'GET', '/') def test_api_request_wo_json_expected(self): - import six PROJECT = 'project' conn = self._makeOne(PROJECT) conn._http = Http( {'status': '200', 'content-type': 'text/plain'}, - six.binary_type('CONTENT'), + b'CONTENT', ) self.assertEqual(conn.api_request('GET', '/', expect_json=False), 'CONTENT') @@ -215,12 +209,11 @@ def test_api_request_wo_json_expected(self): def test_api_request_w_query_params(self): from six.moves.urllib.parse import parse_qsl from six.moves.urllib.parse import urlsplit - import six PROJECT = 'project' conn = self._makeOne(PROJECT) http = conn._http = Http( {'status': '200', 'content-type': 'application/json'}, - six.binary_type('{}'), + b'{}', ) self.assertEqual(conn.api_request('GET', '/', {'foo': 'bar'}), {}) self.assertEqual(http._called_with['method'], 'GET') @@ -242,7 +235,6 @@ def test_api_request_w_query_params(self): def test_api_request_w_data(self): import json - import six PROJECT = 'project' DATA = {'foo': 'bar'} DATAJ = json.dumps(DATA) @@ -255,7 +247,7 @@ def test_api_request_w_data(self): ]) http = conn._http = Http( {'status': '200', 'content-type': 'application/json'}, - six.binary_type('{}'), + b'{}', ) self.assertEqual(conn.api_request('POST', '/', data=DATA), {}) self.assertEqual(http._called_with['method'], 'POST') @@ -271,28 +263,25 @@ def test_api_request_w_data(self): def test_api_request_w_404(self): from gcloud.exceptions import NotFound - import six PROJECT = 'project' conn = self._makeOne(PROJECT) conn._http = Http( {'status': '404', 'content-type': 'text/plain'}, - six.binary_type('{}') + b'{}', ) self.assertRaises(NotFound, conn.api_request, 'GET', '/') def test_api_request_w_500(self): from gcloud.exceptions import InternalServerError - import six PROJECT = 'project' conn = self._makeOne(PROJECT) conn._http = Http( {'status': '500', 'content-type': 'text/plain'}, - six.binary_type('{}'), + b'{}', ) self.assertRaises(InternalServerError, conn.api_request, 'GET', '/') def test_get_all_buckets_empty(self): - import six PROJECT = 'project' conn = self._makeOne(PROJECT) URI = '/'.join([ @@ -303,7 +292,7 @@ def test_get_all_buckets_empty(self): ]) http = conn._http = Http( {'status': '200', 'content-type': 'application/json'}, - six.binary_type('{}'), + b'{}', ) buckets = list(conn.get_all_buckets()) self.assertEqual(len(buckets), 0) @@ -311,7 +300,6 @@ def test_get_all_buckets_empty(self): self.assertEqual(http._called_with['uri'], URI) def test_get_all_buckets_non_empty(self): - import six PROJECT = 'project' BUCKET_NAME = 'bucket-name' conn = self._makeOne(PROJECT) @@ -323,7 +311,7 @@ def test_get_all_buckets_non_empty(self): ]) http = conn._http = Http( {'status': '200', 'content-type': 'application/json'}, - six.binary_type('{"items": [{"name": "%s"}]}' % BUCKET_NAME), + b'{"items": [{"name": "%s"}]}' % BUCKET_NAME, ) buckets = list(conn.get_all_buckets()) self.assertEqual(len(buckets), 1) @@ -333,7 +321,6 @@ def test_get_all_buckets_non_empty(self): def test_get_bucket_miss(self): from gcloud.exceptions import NotFound - import six PROJECT = 'project' NONESUCH = 'nonesuch' conn = self._makeOne(PROJECT) @@ -346,7 +333,7 @@ def test_get_bucket_miss(self): ]) http = conn._http = Http( {'status': '404', 'content-type': 'application/json'}, - six.binary_type('{}'), + b'{}', ) self.assertRaises(NotFound, conn.get_bucket, NONESUCH) self.assertEqual(http._called_with['method'], 'GET') @@ -354,7 +341,6 @@ def test_get_bucket_miss(self): def test_get_bucket_hit(self): from gcloud.storage.bucket import Bucket - import six PROJECT = 'project' BLOB_NAME = 'blob-name' conn = self._makeOne(PROJECT) @@ -367,7 +353,7 @@ def test_get_bucket_hit(self): ]) http = conn._http = Http( {'status': '200', 'content-type': 'application/json'}, - six.binary_type('{"name": "%s"}' % BLOB_NAME), + b'{"name": "%s"}' % BLOB_NAME, ) bucket = conn.get_bucket(BLOB_NAME) self.assertTrue(isinstance(bucket, Bucket)) @@ -378,7 +364,6 @@ def test_get_bucket_hit(self): def test_create_bucket_ok(self): from gcloud.storage.bucket import Bucket - import six PROJECT = 'project' BLOB_NAME = 'blob-name' conn = self._makeOne(PROJECT) @@ -390,7 +375,7 @@ def test_create_bucket_ok(self): ]) http = conn._http = Http( {'status': '200', 'content-type': 'application/json'}, - six.binary_type('{"name": "%s"}' % BLOB_NAME), + b'{"name": "%s"}' % BLOB_NAME, ) bucket = conn.create_bucket(BLOB_NAME) self.assertTrue(isinstance(bucket, Bucket)) @@ -400,7 +385,6 @@ def test_create_bucket_ok(self): self.assertEqual(http._called_with['uri'], URI) def test_delete_bucket_defaults_miss(self): - import six _deleted_blobs = [] PROJECT = 'project' @@ -415,7 +399,7 @@ def test_delete_bucket_defaults_miss(self): ]) http = conn._http = Http( {'status': '200', 'content-type': 'application/json'}, - six.binary_type('{}'), + b'{}', ) self.assertEqual(conn.delete_bucket(BLOB_NAME), None)