From 976c5f49bbf926d49804e803d28ec55d5f459b93 Mon Sep 17 00:00:00 2001 From: Jon Wayne Parrott Date: Wed, 26 Jul 2017 16:58:14 -0700 Subject: [PATCH] Storage: replace httplib2 with Requests --- storage/google/cloud/storage/batch.py | 82 ++--- storage/google/cloud/storage/blob.py | 15 +- storage/tests/unit/test__http.py | 15 +- storage/tests/unit/test_batch.py | 447 +++++++++++++------------- storage/tests/unit/test_blob.py | 52 +-- storage/tests/unit/test_client.py | 221 +++++++------ 6 files changed, 432 insertions(+), 400 deletions(-) diff --git a/storage/google/cloud/storage/batch.py b/storage/google/cloud/storage/batch.py index 0ab95a98743c..c3f2cfb173f2 100644 --- a/storage/google/cloud/storage/batch.py +++ b/storage/google/cloud/storage/batch.py @@ -23,10 +23,10 @@ import io import json -import httplib2 +import requests import six -from google.cloud.exceptions import make_exception +from google.cloud import exceptions from google.cloud.storage._http import Connection @@ -70,11 +70,6 @@ def __init__(self, method, uri, headers, body): super_init(payload, 'http', encode_noop) -class NoContent(object): - """Emulate an HTTP '204 No Content' response.""" - status = 204 - - class _FutureDict(object): """Class to hold a future value for a deferred request. @@ -123,6 +118,19 @@ def __setitem__(self, key, value): raise KeyError('Cannot set %r -> %r on a future' % (key, value)) +class _FutureResponse(requests.Response): + def __init__(self, future_dict): + super(_FutureResponse, self).__init__() + self._future_dict = future_dict + self.status_code = 204 + + def json(self): + raise ValueError() + + def content(self): + return self._future_dict + + class Batch(Connection): """Proxy an underlying connection, batching up change operations. @@ -171,7 +179,7 @@ def _do_request(self, method, url, headers, data, target_object): self._target_objects.append(target_object) if target_object is not None: target_object._properties = result - return NoContent(), result + return _FutureResponse(result) def _prepare_batch_request(self): """Prepares headers and body for a batch request. @@ -218,17 +226,18 @@ def _finish_futures(self, responses): if len(self._target_objects) != len(responses): raise ValueError('Expected a response for every request.') - 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) + for target_object, subresponse in zip( + self._target_objects, responses): + if not 200 <= subresponse.status_code < 300: + exception_args = exception_args or subresponse elif target_object is not None: - target_object._properties = sub_payload + try: + target_object._properties = subresponse.json() + except ValueError: + target_object._properties = subresponse.content if exception_args is not None: - raise make_exception(*exception_args) + raise exceptions.from_http_response(exception_args) def finish(self): """Submit a single `multipart/mixed` request with deferred requests. @@ -243,9 +252,9 @@ def finish(self): # Use the private ``_base_connection`` rather than the property # ``_connection``, since the property may be this # current batch. - response, content = self._client._base_connection._make_request( + response = self._client._base_connection._make_request( 'POST', url, data=body, headers=headers) - responses = list(_unpack_batch_response(response, content)) + responses = list(_unpack_batch_response(response)) self._finish_futures(responses) return responses @@ -265,7 +274,7 @@ def __exit__(self, exc_type, exc_val, exc_tb): self._client._pop_batch() -def _generate_faux_mime_message(parser, response, content): +def _generate_faux_mime_message(parser, response): """Convert response, content -> (multipart) email.message. Helper for _unpack_batch_response. @@ -273,16 +282,15 @@ def _generate_faux_mime_message(parser, response, content): # We coerce to bytes to get consistent concat across # Py2 and Py3. Percent formatting is insufficient since # it includes the b in Py3. - if not isinstance(content, six.binary_type): - content = content.encode('utf-8') - content_type = response['content-type'] + content_type = response.headers.get('content-type', '') if not isinstance(content_type, six.binary_type): content_type = content_type.encode('utf-8') + faux_message = b''.join([ b'Content-Type: ', content_type, b'\nMIME-Version: 1.0\n\n', - content, + response.content, ]) if six.PY2: @@ -291,20 +299,17 @@ def _generate_faux_mime_message(parser, response, content): return parser.parsestr(faux_message.decode('utf-8')) -def _unpack_batch_response(response, content): - """Convert response, content -> [(headers, payload)]. +def _unpack_batch_response(response): + """Convert requests.Response -> [(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` + :type response: :class:`requests.Response` :param response: HTTP response / headers from a request. - - :type content: str - :param content: Response payload with a batch response. """ parser = Parser() - message = _generate_faux_mime_message(parser, response, content) + message = _generate_faux_mime_message(parser, response) if not isinstance(message._payload, list): raise ValueError('Bad response: not multi-part') @@ -314,10 +319,15 @@ def _unpack_batch_response(response, content): _, 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 headers, payload + content_id = msg_headers.get('Content-ID') + + subresponse = requests.Response() + subresponse.request = requests.Request( + method='BATCH', + url='contentid://{}'.format(content_id)).prepare() + subresponse.status_code = int(status) + subresponse.headers.update(msg_headers) + subresponse._content = payload.encode('utf-8') + + yield subresponse diff --git a/storage/google/cloud/storage/blob.py b/storage/google/cloud/storage/blob.py index dfefc3c1a4fa..b515d1e2c8c2 100644 --- a/storage/google/cloud/storage/blob.py +++ b/storage/google/cloud/storage/blob.py @@ -34,7 +34,6 @@ import time import warnings -import httplib2 from six.moves.urllib.parse import quote import google.auth.transport.requests @@ -44,11 +43,11 @@ from google.resumable_media.requests import MultipartUpload from google.resumable_media.requests import ResumableUpload +from google.cloud import exceptions from google.cloud._helpers import _rfc3339_to_datetime from google.cloud._helpers import _to_bytes from google.cloud._helpers import _bytes_to_unicode from google.cloud.exceptions import NotFound -from google.cloud.exceptions import make_exception from google.cloud.iam import Policy from google.cloud.storage._helpers import _PropertyMixin from google.cloud.storage._helpers import _scalar_property @@ -469,7 +468,7 @@ def download_to_file(self, file_obj, client=None): try: self._do_download(transport, file_obj, download_url, headers) except resumable_media.InvalidResponse as exc: - _raise_from_invalid_response(exc, download_url) + _raise_from_invalid_response(exc) def download_to_filename(self, filename, client=None): """Download the contents of this blob into a named file. @@ -1598,20 +1597,14 @@ def _maybe_rewind(stream, rewind=False): stream.seek(0, os.SEEK_SET) -def _raise_from_invalid_response(error, error_info=None): +def _raise_from_invalid_response(error): """Re-wrap and raise an ``InvalidResponse`` exception. :type error: :exc:`google.resumable_media.InvalidResponse` :param error: A caught exception from the ``google-resumable-media`` library. - :type error_info: str - :param error_info: (Optional) Extra information about the failed request. - :raises: :class:`~google.cloud.exceptions.GoogleCloudError` corresponding to the failed status code """ - response = error.response - faux_response = httplib2.Response({'status': response.status_code}) - raise make_exception(faux_response, response.content, - error_info=error_info, use_json=False) + raise exceptions.from_http_response(error.response) diff --git a/storage/tests/unit/test__http.py b/storage/tests/unit/test__http.py index cb9344a16389..5e03f94a6406 100644 --- a/storage/tests/unit/test__http.py +++ b/storage/tests/unit/test__http.py @@ -29,13 +29,17 @@ def _make_one(self, *args, **kw): return self._get_target_class()(*args, **kw) def test_extra_headers(self): + import requests + from google.cloud import _http as base_http from google.cloud.storage import _http as MUT - http = mock.Mock(spec=['request']) - response = mock.Mock(status=200, spec=['status']) + http = mock.create_autospec(requests.Session, instance=True) + response = requests.Response() + response.status_code = 200 data = b'brent-spiner' - http.request.return_value = response, data + response._content = data + http.request.return_value = response client = mock.Mock(_http=http, spec=['_http']) conn = self._make_one(client) @@ -45,17 +49,16 @@ def test_extra_headers(self): self.assertEqual(result, data) expected_headers = { - 'Content-Length': str(len(req_data)), 'Accept-Encoding': 'gzip', base_http.CLIENT_INFO_HEADER: MUT._CLIENT_INFO, 'User-Agent': conn.USER_AGENT, } expected_uri = conn.build_api_url('/rainbow') http.request.assert_called_once_with( - body=req_data, + data=req_data, headers=expected_headers, method='GET', - uri=expected_uri, + url=expected_uri, ) def test_build_api_url_no_extra_query_params(self): diff --git a/storage/tests/unit/test_batch.py b/storage/tests/unit/test_batch.py index 60157af8c06b..6e605e4e1e0e 100644 --- a/storage/tests/unit/test_batch.py +++ b/storage/tests/unit/test_batch.py @@ -15,6 +15,8 @@ import unittest import mock +import requests +from six.moves import http_client def _make_credentials(): @@ -23,6 +25,21 @@ def _make_credentials(): return mock.Mock(spec=google.auth.credentials.Credentials) +def _make_response(status=http_client.OK, content=b'', headers={}): + response = requests.Response() + response.status_code = status + response._content = content + response.headers = headers + response.request = requests.Request() + return response + + +def _make_requests_session(responses): + session = mock.create_autospec(requests.Session, instance=True) + session.request.side_effect = responses + return session + + class TestMIMEApplicationHTTP(unittest.TestCase): @staticmethod @@ -88,7 +105,7 @@ def _make_one(self, *args, **kw): return self._get_target_class()(*args, **kw) def test_ctor(self): - http = _HTTP() + http = _make_requests_session([]) connection = _Connection(http=http) client = _Client(connection) batch = self._make_one(client) @@ -115,125 +132,133 @@ def test_current(self): def test__make_request_GET_normal(self): from google.cloud.storage.batch import _FutureDict - URL = 'http://example.com/api' - expected = _Response() - http = _HTTP((expected, '')) + url = 'http://example.com/api' + http = _make_requests_session([]) connection = _Connection(http=http) batch = self._make_one(connection) target = _MockObject() - response, content = batch._make_request('GET', URL, - target_object=target) - self.assertEqual(response.status, 204) - self.assertIsInstance(content, _FutureDict) - self.assertIs(target._properties, content) - self.assertEqual(http._requests, []) - EXPECTED_HEADERS = [ - ('Accept-Encoding', 'gzip'), - ('Content-Length', '0'), - ] - 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.assertIsNone(solo_request[3]) + + response = batch._make_request('GET', url, target_object=target) + + # Check the respone + self.assertEqual(response.status_code, 204) + self.assertIsInstance(response.content(), _FutureDict) + self.assertIs(target._properties, response.content()) + + # The real http request should not have been called yet. + http.request.assert_not_called() + + # Check the queued request + self.assertEqual(len(batch._requests), 1) + request = batch._requests[0] + request_method, request_url, _, request_data = request + self.assertEqual(request_method, 'GET') + self.assertEqual(request_url, url) + self.assertIsNone(request_data) def test__make_request_POST_normal(self): from google.cloud.storage.batch import _FutureDict - URL = 'http://example.com/api' - http = _HTTP() # no requests expected + url = 'http://example.com/api' + http = _make_requests_session([]) connection = _Connection(http=http) batch = self._make_one(connection) + data = {'foo': 1} target = _MockObject() - response, content = batch._make_request('POST', URL, data={'foo': 1}, - target_object=target) - self.assertEqual(response.status, 204) - self.assertIsInstance(content, _FutureDict) - self.assertIs(target._properties, content) - self.assertEqual(http._requests, []) - EXPECTED_HEADERS = [ - ('Accept-Encoding', 'gzip'), - ('Content-Length', '10'), - ] - 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(solo_request[3], {'foo': 1}) + + response = batch._make_request( + 'POST', url, data={'foo': 1}, target_object=target) + + self.assertEqual(response.status_code, 204) + self.assertIsInstance(response.content(), _FutureDict) + self.assertIs(target._properties, response.content()) + + # The real http request should not have been called yet. + http.request.assert_not_called() + + request = batch._requests[0] + request_method, request_url, _, request_data = request + self.assertEqual(request_method, 'POST') + self.assertEqual(request_url, url) + self.assertEqual(request_data, data) def test__make_request_PATCH_normal(self): from google.cloud.storage.batch import _FutureDict - URL = 'http://example.com/api' - http = _HTTP() # no requests expected + url = 'http://example.com/api' + http = _make_requests_session([]) connection = _Connection(http=http) batch = self._make_one(connection) + data = {'foo': 1} target = _MockObject() - response, content = batch._make_request('PATCH', URL, data={'foo': 1}, - target_object=target) - self.assertEqual(response.status, 204) - self.assertIsInstance(content, _FutureDict) - self.assertIs(target._properties, content) - self.assertEqual(http._requests, []) - EXPECTED_HEADERS = [ - ('Accept-Encoding', 'gzip'), - ('Content-Length', '10'), - ] - 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(solo_request[3], {'foo': 1}) + + response = batch._make_request( + 'PATCH', url, data={'foo': 1}, target_object=target) + + self.assertEqual(response.status_code, 204) + self.assertIsInstance(response.content(), _FutureDict) + self.assertIs(target._properties, response.content()) + + # The real http request should not have been called yet. + http.request.assert_not_called() + + request = batch._requests[0] + request_method, request_url, _, request_data = request + self.assertEqual(request_method, 'PATCH') + self.assertEqual(request_url, url) + self.assertEqual(request_data, data) def test__make_request_DELETE_normal(self): from google.cloud.storage.batch import _FutureDict - URL = 'http://example.com/api' - http = _HTTP() # no requests expected + url = 'http://example.com/api' + http = _make_requests_session([]) connection = _Connection(http=http) batch = self._make_one(connection) target = _MockObject() - response, content = batch._make_request('DELETE', URL, - target_object=target) - self.assertEqual(response.status, 204) - self.assertIsInstance(content, _FutureDict) - self.assertIs(target._properties, content) - self.assertEqual(http._requests, []) - EXPECTED_HEADERS = [ - ('Accept-Encoding', 'gzip'), - ('Content-Length', '0'), - ] - 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.assertIsNone(solo_request[3]) + + response = batch._make_request('DELETE', url, target_object=target) + + # Check the respone + self.assertEqual(response.status_code, 204) + self.assertIsInstance(response.content(), _FutureDict) + self.assertIs(target._properties, response.content()) + + # The real http request should not have been called yet. + http.request.assert_not_called() + + # Check the queued request + self.assertEqual(len(batch._requests), 1) + request = batch._requests[0] + request_method, request_url, _, request_data = request + self.assertEqual(request_method, 'DELETE') + self.assertEqual(request_url, url) + self.assertIsNone(request_data) def test__make_request_POST_too_many_requests(self): - URL = 'http://example.com/api' - http = _HTTP() # no requests expected + url = 'http://example.com/api' + http = _make_requests_session([]) connection = _Connection(http=http) batch = self._make_one(connection) + batch._MAX_BATCH_SIZE = 1 - batch._requests.append(('POST', URL, {}, {'bar': 2})) - self.assertRaises(ValueError, - batch._make_request, 'POST', URL, data={'foo': 1}) - self.assertIs(connection.http, http) + batch._requests.append(('POST', url, {}, {'bar': 2})) + + with self.assertRaises(ValueError): + batch._make_request('POST', url, data={'foo': 1}) def test_finish_empty(self): - http = _HTTP() # no requests expected + http = _make_requests_session([]) connection = _Connection(http=http) batch = self._make_one(connection) - self.assertRaises(ValueError, batch.finish) - self.assertIs(connection.http, http) + + with self.assertRaises(ValueError): + batch.finish() + + def _get_payload_chunks(self, boundary, payload): + divider = '--' + boundary[len('boundary="'):-1] + chunks = payload.split(divider)[1:-1] # discard prolog / epilog + return chunks def _check_subrequest_no_payload(self, chunk, method, url): lines = chunk.splitlines() @@ -269,133 +294,144 @@ def _check_subrequest_payload(self, chunk, method, url, payload): self.assertEqual(lines[7], '') self.assertEqual(json.loads(lines[8]), payload) - def test_finish_nonempty(self): - import httplib2 + def _get_mutlipart_request(self, http): + request_call = http.request.mock_calls[0][2] + request_headers = request_call['headers'] + request_body = request_call['data'] + content_type, boundary = [ + value.strip() for value in + request_headers['Content-Type'].split(';')] + + return request_headers, request_body, content_type, boundary - URL = 'http://api.example.com/other_api' - expected = _Response() - expected['content-type'] = 'multipart/mixed; boundary="DEADBEEF="' - http = _HTTP((expected, _THREE_PART_MIME_RESPONSE)) + def test_finish_nonempty(self): + url = 'http://api.example.com/other_api' + expected_response = _make_response( + content=_THREE_PART_MIME_RESPONSE, + headers={'content-type': 'multipart/mixed; boundary="DEADBEEF="'}) + http = _make_requests_session([expected_response]) connection = _Connection(http=http) client = _Client(connection) batch = self._make_one(client) batch.API_BASE_URL = 'http://api.example.com' - 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) + + 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({ - 'content-length': '20', - 'content-type': 'application/json; charset=UTF-8', - 'status': '200', + + response1, response2, response3 = result + + self.assertEqual(response1.headers, { + 'Content-Length': '20', + 'Content-Type': 'application/json; charset=UTF-8', }) - 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(response1.json(), {'foo': 1, 'bar': 2}) + + self.assertEqual(response2.headers, { + 'Content-Length': '20', + 'Content-Type': 'application/json; charset=UTF-8', }) - self.assertEqual(result[2], (response2, '')) - 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') + self.assertEqual(response2.json(), {'foo': 1, 'bar': 3}) - divider = '--' + boundary[len('boundary="'):-1] - chunks = body.split(divider)[1:-1] # discard prolog / epilog - self.assertEqual(len(chunks), 3) + self.assertEqual(response3.headers, {'Content-Length': '0'}) + self.assertEqual(response3.status_code, http_client.NO_CONTENT) - self._check_subrequest_payload(chunks[0], 'POST', URL, - {'foo': 1, 'bar': 2}) + expected_url = '{}/batch'.format(batch.API_BASE_URL) + http.request.assert_called_once_with( + method='POST', url=expected_url, headers=mock.ANY, data=mock.ANY) - self._check_subrequest_payload(chunks[1], 'PATCH', URL, {'bar': 3}) + request_info = self._get_mutlipart_request(http) + request_headers, request_body, content_type, boundary = request_info - self._check_subrequest_no_payload(chunks[2], 'DELETE', URL) + self.assertEqual(content_type, 'multipart/mixed') + self.assertTrue(boundary.startswith('boundary="==')) + self.assertTrue(boundary.endswith('=="')) + self.assertEqual(request_headers['MIME-Version'], '1.0') + + chunks = self._get_payload_chunks(boundary, request_body) + self.assertEqual(len(chunks), 3) + self._check_subrequest_payload( + chunks[0], 'POST', url, {'foo': 1, 'bar': 2}) + self._check_subrequest_payload(chunks[1], 'PATCH', url, {'bar': 3}) + 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)) + url = 'http://api.example.com/other_api' + expected_response = _make_response( + content=_TWO_PART_MIME_RESPONSE_WITH_FAIL, + headers={'content-type': 'multipart/mixed; boundary="DEADBEEF="'}) + http = _make_requests_session([expected_response]) connection = _Connection(http=http) client = _Client(connection) batch = self._make_one(client) batch.API_BASE_URL = 'http://api.example.com' - batch._requests.append(('GET', URL, {}, None)) - self.assertRaises(ValueError, batch.finish) + + batch._requests.append(('GET', url, {}, None)) + with self.assertRaises(ValueError): + batch.finish() def test_finish_nonempty_with_status_failure(self): from google.cloud.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)) + url = 'http://api.example.com/other_api' + expected_response = _make_response( + content=_TWO_PART_MIME_RESPONSE_WITH_FAIL, + headers={'content-type': 'multipart/mixed; boundary="DEADBEEF="'}) + http = _make_requests_session([expected_response]) connection = _Connection(http=http) client = _Client(connection) batch = self._make_one(client) batch.API_BASE_URL = 'http://api.example.com' target1 = _MockObject() target2 = _MockObject() - batch._do_request('GET', URL, {}, None, target1) - batch._do_request('GET', URL, {}, None, target2) + + 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._target_objects], [target1, target2]) target2_future_before = target2._properties - self.assertRaises(NotFound, batch.finish) + + with self.assertRaises(NotFound): + batch.finish() + self.assertEqual(target1._properties, {'foo': 1, 'bar': 2}) self.assertIs(target2._properties, target2_future_before) - 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') + expected_url = '{}/batch'.format(batch.API_BASE_URL) + http.request.assert_called_once_with( + method='POST', url=expected_url, headers=mock.ANY, data=mock.ANY) - divider = '--' + boundary[len('boundary="'):-1] - chunks = body.split(divider)[1:-1] # discard prolog / epilog - self.assertEqual(len(chunks), 2) + _, request_body, _, boundary = self._get_mutlipart_request(http) - self._check_subrequest_payload(chunks[0], 'GET', URL, {}) - self._check_subrequest_payload(chunks[1], 'GET', URL, {}) + chunks = self._get_payload_chunks(boundary, request_body) + 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() - expected['content-type'] = 'text/plain' - http = _HTTP((expected, 'NOT A MIME_RESPONSE')) + url = 'http://api.example.com/other_api' + http = _make_requests_session([_make_response()]) connection = _Connection(http=http) client = _Client(connection) batch = self._make_one(client) - batch._requests.append(('POST', URL, {}, {'foo': 1, 'bar': 2})) - batch._requests.append(('PATCH', URL, {}, {'bar': 3})) - batch._requests.append(('DELETE', URL, {}, None)) - self.assertRaises(ValueError, batch.finish) + batch._requests.append(('POST', url, {}, {'foo': 1, 'bar': 2})) + + with self.assertRaises(ValueError): + batch.finish() def test_as_context_mgr_wo_error(self): from google.cloud.storage.client import Client - URL = 'http://example.com/api' - expected = _Response() - expected['content-type'] = 'multipart/mixed; boundary="DEADBEEF="' - http = _HTTP((expected, _THREE_PART_MIME_RESPONSE)) + url = 'http://example.com/api' + expected_response = _make_response( + content=_THREE_PART_MIME_RESPONSE, + headers={'content-type': 'multipart/mixed; boundary="DEADBEEF="'}) + http = _make_requests_session([expected_response]) project = 'PROJECT' credentials = _make_credentials() client = Client(project=project, credentials=credentials) @@ -406,13 +442,14 @@ def test_as_context_mgr_wo_error(self): target1 = _MockObject() target2 = _MockObject() target3 = _MockObject() + with self._make_one(client) as batch: self.assertEqual(list(client._batch_stack), [batch]) - batch._make_request('POST', URL, {'foo': 1, 'bar': 2}, + batch._make_request('POST', url, {'foo': 1, 'bar': 2}, target_object=target1) - batch._make_request('PATCH', URL, {'bar': 3}, + batch._make_request('PATCH', url, {'bar': 3}, target_object=target2) - batch._make_request('DELETE', URL, target_object=target3) + batch._make_request('DELETE', url, target_object=target3) self.assertEqual(list(client._batch_stack), []) self.assertEqual(len(batch._requests), 3) @@ -424,14 +461,14 @@ def test_as_context_mgr_wo_error(self): {'foo': 1, 'bar': 2}) self.assertEqual(target2._properties, {'foo': 1, 'bar': 3}) - self.assertEqual(target3._properties, '') + self.assertEqual(target3._properties, b'') def test_as_context_mgr_w_error(self): from google.cloud.storage.batch import _FutureDict from google.cloud.storage.client import Client URL = 'http://example.com/api' - http = _HTTP() + http = _make_requests_session([]) connection = _Connection(http=http) project = 'PROJECT' credentials = _make_credentials() @@ -455,8 +492,8 @@ def test_as_context_mgr_w_error(self): except ValueError: pass + http.request.assert_not_called() self.assertEqual(list(client._batch_stack), []) - self.assertEqual(len(http._requests), 0) self.assertEqual(len(batch._requests), 3) self.assertEqual(batch._target_objects, [target1, target2, target3]) # Since the context manager fails, finish will not get called and @@ -468,44 +505,37 @@ def test_as_context_mgr_w_error(self): class Test__unpack_batch_response(unittest.TestCase): - def _call_fut(self, response, content): + def _call_fut(self, headers, content): from google.cloud.storage.batch import _unpack_batch_response - return _unpack_batch_response(response, content) + response = _make_response(content=content, headers=headers) - def _unpack_helper(self, response, content): - import httplib2 + return _unpack_batch_response(response) + def _unpack_helper(self, response, content): result = list(self._call_fut(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): + self.assertEqual(result[0].status_code, http_client.OK) + self.assertEqual(result[0].json(), {u'bar': 2, u'foo': 1}) + self.assertEqual(result[1].status_code, http_client.OK) + self.assertEqual(result[1].json(), {u'foo': 1, u'bar': 3}) + self.assertEqual(result[2].status_code, http_client.NO_CONTENT) + + def test_bytes_headers(self): RESPONSE = {'content-type': b'multipart/mixed; boundary="DEADBEEF="'} CONTENT = _THREE_PART_MIME_RESPONSE self._unpack_helper(RESPONSE, CONTENT) - def test_unicode(self): + def test_unicode_headers(self): RESPONSE = {'content-type': u'multipart/mixed; boundary="DEADBEEF="'} - CONTENT = _THREE_PART_MIME_RESPONSE.decode('utf-8') + CONTENT = _THREE_PART_MIME_RESPONSE self._unpack_helper(RESPONSE, CONTENT) _TWO_PART_MIME_RESPONSE_WITH_FAIL = b"""\ --DEADBEEF= -Content-Type: application/http +Content-Type: application/json Content-ID: HTTP/1.1 200 OK @@ -515,7 +545,7 @@ def test_unicode(self): {"foo": 1, "bar": 2} --DEADBEEF= -Content-Type: application/http +Content-Type: application/json Content-ID: HTTP/1.1 404 Not Found @@ -529,7 +559,7 @@ def test_unicode(self): _THREE_PART_MIME_RESPONSE = b"""\ --DEADBEEF= -Content-Type: application/http +Content-Type: application/json Content-ID: HTTP/1.1 200 OK @@ -539,7 +569,7 @@ def test_unicode(self): {"foo": 1, "bar": 2} --DEADBEEF= -Content-Type: application/http +Content-Type: application/json Content-ID: HTTP/1.1 200 OK @@ -549,7 +579,7 @@ def test_unicode(self): {"foo": 1, "bar": 3} --DEADBEEF= -Content-Type: application/http +Content-Type: text/plain Content-ID: HTTP/1.1 204 No Content @@ -591,25 +621,8 @@ def __init__(self, **kw): self.__dict__.update(kw) def _make_request(self, method, url, data=None, headers=None): - return self.http.request(uri=url, method=method, - headers=headers, body=data) - - -class _Response(dict): - def __init__(self, status=200, **kw): - self.status = status - super(_Response, self).__init__(**kw) - - -class _HTTP(object): - def __init__(self, *responses): - self._requests = [] - self._responses = list(responses) - - def request(self, uri, method, headers, body): - self._requests.append((method, uri, headers, body)) - response, self._responses = self._responses[0], self._responses[1:] - return response + return self.http.request(url=url, method=method, + headers=headers, data=data) class _MockObject(object): diff --git a/storage/tests/unit/test_blob.py b/storage/tests/unit/test_blob.py index e2227adbd94a..7904ce86e89b 100644 --- a/storage/tests/unit/test_blob.py +++ b/storage/tests/unit/test_blob.py @@ -376,9 +376,15 @@ def test__get_download_url_on_the_fly_with_generation(self): @staticmethod def _mock_requests_response(status_code, headers, content=b''): - return mock.Mock( - content=content, headers=headers, status_code=status_code, - spec=['content', 'headers', 'status_code']) + import requests + + response = requests.Response() + response.status_code = status_code + response.headers.update(headers) + response._content = content + response.request = requests.Request( + 'POST', 'http://example.com').prepare() + return response def _mock_download_transport(self): fake_transport = mock.Mock(spec=['request']) @@ -1159,19 +1165,23 @@ def test_upload_from_file_with_rewind(self): assert stream.tell() == 0 def test_upload_from_file_failure(self): + import requests + from google.resumable_media import InvalidResponse from google.cloud import exceptions message = b'Someone is already in this spot.' - response = mock.Mock( - content=message, status_code=http_client.CONFLICT, - spec=[u'content', u'status_code']) + response = requests.Response() + response._content = message + response.status_code = http_client.CONFLICT + response.request = requests.Request( + 'POST', 'http://example.com').prepare() side_effect = InvalidResponse(response) with self.assertRaises(exceptions.Conflict) as exc_info: self._upload_from_file_helper(side_effect=side_effect) - self.assertEqual(exc_info.exception.message, message.decode('utf-8')) + self.assertIn(message.decode('utf-8'), exc_info.exception.message) self.assertEqual(exc_info.exception.errors, []) def _do_upload_mock_call_helper(self, blob, client, content_type, size): @@ -1309,16 +1319,16 @@ def test_create_resumable_upload_session_with_failure(self): from google.cloud import exceptions message = b'5-oh-3 woe is me.' - response = mock.Mock( + response = self._mock_requests_response( content=message, status_code=http_client.SERVICE_UNAVAILABLE, - spec=[u'content', u'status_code']) + headers={}) side_effect = InvalidResponse(response) with self.assertRaises(exceptions.ServiceUnavailable) as exc_info: self._create_resumable_upload_session_helper( side_effect=side_effect) - self.assertEqual(exc_info.exception.message, message.decode('utf-8')) + self.assertIn(message.decode('utf-8'), exc_info.exception.message) self.assertEqual(exc_info.exception.errors, []) def test_get_iam_policy(self): @@ -2225,12 +2235,16 @@ def _call_fut(*args, **kwargs): return _raise_from_invalid_response(*args, **kwargs) def _helper(self, message, **kwargs): + import requests + from google.resumable_media import InvalidResponse from google.cloud import exceptions - response = mock.Mock( - content=message, status_code=http_client.BAD_REQUEST, - spec=[u'content', u'status_code']) + response = requests.Response() + response.request = requests.Request( + 'GET', 'http://example.com').prepare() + response.status_code = http_client.BAD_REQUEST + response._content = message error = InvalidResponse(response) with self.assertRaises(exceptions.BadRequest) as exc_info: @@ -2241,17 +2255,9 @@ def _helper(self, message, **kwargs): def test_default(self): message = b'Failure' exc_info = self._helper(message) - self.assertEqual(exc_info.exception.message, message.decode('utf-8')) - self.assertEqual(exc_info.exception.errors, []) - - def test_with_error_info(self): - message = b'Eeek bad.' - error_info = 'http://test.invalid' - exc_info = self._helper(message, error_info=error_info) - message_str = message.decode('utf-8') - full_message = u'{} ({})'.format(message_str, error_info) - self.assertEqual(exc_info.exception.message, full_message) + expected = 'GET http://example.com/: {}'.format(message_str) + self.assertEqual(exc_info.exception.message, expected) self.assertEqual(exc_info.exception.errors, []) diff --git a/storage/tests/unit/test_client.py b/storage/tests/unit/test_client.py index 9696d4e5fa51..ab75d9be8fca 100644 --- a/storage/tests/unit/test_client.py +++ b/storage/tests/unit/test_client.py @@ -12,9 +12,12 @@ # See the License for the specific language governing permissions and # limitations under the License. +import json import unittest import mock +import requests +from six.moves import http_client def _make_credentials(): @@ -23,6 +26,30 @@ def _make_credentials(): return mock.Mock(spec=google.auth.credentials.Credentials) +def _make_response(status=http_client.OK, content=b'', headers={}): + response = requests.Response() + response.status_code = status + response._content = content + response.headers = headers + response.request = requests.Request() + return response + + +def _make_json_response(data, status=http_client.OK, headers=None): + headers = headers or {} + headers['Content-Type'] = 'application/json' + return _make_response( + status=status, + content=json.dumps(data).encode('utf-8'), + headers=headers) + + +def _make_requests_session(responses): + session = mock.create_autospec(requests.Session, instance=True) + session.request.side_effect = responses + return session + + class TestClient(unittest.TestCase): @staticmethod @@ -140,13 +167,15 @@ def test_get_bucket_miss(self): 'b', 'nonesuch?projection=noAcl', ]) - http = client._http_internal = _Http( - {'status': '404', 'content-type': 'application/json'}, - b'{}', - ) - self.assertRaises(NotFound, client.get_bucket, NONESUCH) - self.assertEqual(http._called_with['method'], 'GET') - self.assertEqual(http._called_with['uri'], URI) + http = _make_requests_session([ + _make_json_response({}, status=http_client.NOT_FOUND)]) + client._http_internal = http + + with self.assertRaises(NotFound): + client.get_bucket(NONESUCH) + + http.request.assert_called_once_with( + method='GET', url=URI, data=mock.ANY, headers=mock.ANY) def test_get_bucket_hit(self): from google.cloud.storage.bucket import Bucket @@ -163,16 +192,17 @@ def test_get_bucket_hit(self): 'b', '%s?projection=noAcl' % (BLOB_NAME,), ]) - http = client._http_internal = _Http( - {'status': '200', 'content-type': 'application/json'}, - '{{"name": "{0}"}}'.format(BLOB_NAME).encode('utf-8'), - ) + + data = {'name': BLOB_NAME} + http = _make_requests_session([_make_json_response(data)]) + client._http_internal = http bucket = client.get_bucket(BLOB_NAME) + self.assertIsInstance(bucket, Bucket) self.assertEqual(bucket.name, BLOB_NAME) - self.assertEqual(http._called_with['method'], 'GET') - self.assertEqual(http._called_with['uri'], URI) + http.request.assert_called_once_with( + method='GET', url=URI, data=mock.ANY, headers=mock.ANY) def test_lookup_bucket_miss(self): PROJECT = 'PROJECT' @@ -187,14 +217,15 @@ def test_lookup_bucket_miss(self): 'b', 'nonesuch?projection=noAcl', ]) - http = client._http_internal = _Http( - {'status': '404', 'content-type': 'application/json'}, - b'{}', - ) + http = _make_requests_session([ + _make_json_response({}, status=http_client.NOT_FOUND)]) + client._http_internal = http + bucket = client.lookup_bucket(NONESUCH) + self.assertIsNone(bucket) - self.assertEqual(http._called_with['method'], 'GET') - self.assertEqual(http._called_with['uri'], URI) + http.request.assert_called_once_with( + method='GET', url=URI, data=mock.ANY, headers=mock.ANY) def test_lookup_bucket_hit(self): from google.cloud.storage.bucket import Bucket @@ -211,16 +242,16 @@ def test_lookup_bucket_hit(self): 'b', '%s?projection=noAcl' % (BLOB_NAME,), ]) - http = client._http_internal = _Http( - {'status': '200', 'content-type': 'application/json'}, - '{{"name": "{0}"}}'.format(BLOB_NAME).encode('utf-8'), - ) + data = {'name': BLOB_NAME} + http = _make_requests_session([_make_json_response(data)]) + client._http_internal = http bucket = client.lookup_bucket(BLOB_NAME) + self.assertIsInstance(bucket, Bucket) self.assertEqual(bucket.name, BLOB_NAME) - self.assertEqual(http._called_with['method'], 'GET') - self.assertEqual(http._called_with['uri'], URI) + http.request.assert_called_once_with( + method='GET', url=URI, data=mock.ANY, headers=mock.ANY) def test_create_bucket_conflict(self): from google.cloud.exceptions import Conflict @@ -236,14 +267,14 @@ def test_create_bucket_conflict(self): client._connection.API_VERSION, 'b?project=%s' % (PROJECT,), ]) - http = client._http_internal = _Http( - {'status': '409', 'content-type': 'application/json'}, - '{"error": {"message": "Conflict"}}', - ) + data = {'error': {'message': 'Conflict'}} + http = _make_requests_session([ + _make_json_response(data, status=http_client.CONFLICT)]) + client._http_internal = http self.assertRaises(Conflict, client.create_bucket, BLOB_NAME) - self.assertEqual(http._called_with['method'], 'POST') - self.assertEqual(http._called_with['uri'], URI) + http.request.assert_called_once_with( + method='POST', url=URI, data=mock.ANY, headers=mock.ANY) def test_create_bucket_success(self): from google.cloud.storage.bucket import Bucket @@ -259,16 +290,16 @@ def test_create_bucket_success(self): client._connection.API_VERSION, 'b?project=%s' % (PROJECT,), ]) - http = client._http_internal = _Http( - {'status': '200', 'content-type': 'application/json'}, - '{{"name": "{0}"}}'.format(BLOB_NAME).encode('utf-8'), - ) + data = {'name': BLOB_NAME} + http = _make_requests_session([_make_json_response(data)]) + client._http_internal = http bucket = client.create_bucket(BLOB_NAME) + self.assertIsInstance(bucket, Bucket) self.assertEqual(bucket.name, BLOB_NAME) - self.assertEqual(http._called_with['method'], 'POST') - self.assertEqual(http._called_with['uri'], URI) + http.request.assert_called_once_with( + method='POST', url=URI, data=mock.ANY, headers=mock.ANY) def test_list_buckets_empty(self): from six.moves.urllib.parse import parse_qs @@ -278,59 +309,50 @@ def test_list_buckets_empty(self): CREDENTIALS = _make_credentials() client = self._make_one(project=PROJECT, credentials=CREDENTIALS) - EXPECTED_QUERY = { - 'project': [PROJECT], - 'projection': ['noAcl'], - } - http = client._http_internal = _Http( - {'status': '200', 'content-type': 'application/json'}, - b'{}', - ) + http = _make_requests_session([_make_json_response({})]) + client._http_internal = http + buckets = list(client.list_buckets()) + self.assertEqual(len(buckets), 0) - self.assertEqual(http._called_with['method'], 'GET') - self.assertIsNone(http._called_with['body']) - BASE_URI = '/'.join([ + http.request.assert_called_once_with( + method='GET', url=mock.ANY, data=mock.ANY, headers=mock.ANY) + + requested_url = http.request.mock_calls[0][2]['url'] + expected_base_url = '/'.join([ client._connection.API_BASE_URL, 'storage', client._connection.API_VERSION, 'b', ]) - URI = http._called_with['uri'] - self.assertTrue(URI.startswith(BASE_URI)) - uri_parts = urlparse(URI) - self.assertEqual(parse_qs(uri_parts.query), EXPECTED_QUERY) + self.assertTrue(requested_url.startswith(expected_base_url)) - def test_list_buckets_non_empty(self): - from six.moves.urllib.parse import parse_qs - from six.moves.urllib.parse import urlencode - from six.moves.urllib.parse import urlparse + expected_query = { + 'project': [PROJECT], + 'projection': ['noAcl'], + } + uri_parts = urlparse(requested_url) + self.assertEqual(parse_qs(uri_parts.query), expected_query) + def test_list_buckets_non_empty(self): PROJECT = 'PROJECT' CREDENTIALS = _make_credentials() client = self._make_one(project=PROJECT, credentials=CREDENTIALS) BUCKET_NAME = 'bucket-name' - query_params = urlencode({'project': PROJECT, 'projection': 'noAcl'}) - BASE_URI = '/'.join([ - client._connection.API_BASE_URL, - 'storage', - client._connection.API_VERSION, - ]) - URI = '/'.join([BASE_URI, 'b?%s' % (query_params,)]) - http = client._http_internal = _Http( - {'status': '200', 'content-type': 'application/json'}, - '{{"items": [{{"name": "{0}"}}]}}'.format(BUCKET_NAME) - .encode('utf-8'), - ) + + data = {'items': [{'name': BUCKET_NAME}]} + http = _make_requests_session([_make_json_response(data)]) + client._http_internal = http + buckets = list(client.list_buckets()) + self.assertEqual(len(buckets), 1) self.assertEqual(buckets[0].name, BUCKET_NAME) - self.assertEqual(http._called_with['method'], 'GET') - self.assertTrue(http._called_with['uri'].startswith(BASE_URI)) - self.assertEqual(parse_qs(urlparse(http._called_with['uri']).query), - parse_qs(urlparse(URI).query)) + + http.request.assert_called_once_with( + method='GET', url=mock.ANY, data=mock.ANY, headers=mock.ANY) def test_list_buckets_all_arguments(self): from six.moves.urllib.parse import parse_qs @@ -345,19 +367,10 @@ def test_list_buckets_all_arguments(self): PREFIX = 'subfolder' PROJECTION = 'full' FIELDS = 'items/id,nextPageToken' - EXPECTED_QUERY = { - 'project': [PROJECT], - 'maxResults': [str(MAX_RESULTS)], - 'pageToken': [PAGE_TOKEN], - 'prefix': [PREFIX], - 'projection': [PROJECTION], - 'fields': [FIELDS], - } - http = client._http_internal = _Http( - {'status': '200', 'content-type': 'application/json'}, - '{"items": []}', - ) + data = {'items': []} + http = _make_requests_session([_make_json_response(data)]) + client._http_internal = http iterator = client.list_buckets( max_results=MAX_RESULTS, page_token=PAGE_TOKEN, @@ -367,19 +380,28 @@ def test_list_buckets_all_arguments(self): ) buckets = list(iterator) self.assertEqual(buckets, []) - self.assertEqual(http._called_with['method'], 'GET') - self.assertIsNone(http._called_with['body']) + http.request.assert_called_once_with( + method='GET', url=mock.ANY, data=mock.ANY, headers=mock.ANY) - BASE_URI = '/'.join([ + requested_url = http.request.mock_calls[0][2]['url'] + expected_base_url = '/'.join([ client._connection.API_BASE_URL, 'storage', client._connection.API_VERSION, - 'b' + 'b', ]) - URI = http._called_with['uri'] - self.assertTrue(URI.startswith(BASE_URI)) - uri_parts = urlparse(URI) - self.assertEqual(parse_qs(uri_parts.query), EXPECTED_QUERY) + self.assertTrue(requested_url.startswith(expected_base_url)) + + expected_query = { + 'project': [PROJECT], + 'maxResults': [str(MAX_RESULTS)], + 'pageToken': [PAGE_TOKEN], + 'prefix': [PREFIX], + 'projection': [PROJECTION], + 'fields': [FIELDS], + } + uri_parts = urlparse(requested_url) + self.assertEqual(parse_qs(uri_parts.query), expected_query) def test_page_empty_response(self): from google.cloud.iterator import Page @@ -415,18 +437,3 @@ def dummy_response(): self.assertEqual(page.remaining, 0) self.assertIsInstance(bucket, Bucket) self.assertEqual(bucket.name, blob_name) - - -class _Http(object): - - _called_with = None - - def __init__(self, headers, content): - from httplib2 import Response - - self._response = Response(headers) - self._content = content - - def request(self, **kw): - self._called_with = kw - return self._response, self._content