diff --git a/bigquery/google/cloud/bigquery/client.py b/bigquery/google/cloud/bigquery/client.py index 5f0101f35de5..f36d80978efd 100644 --- a/bigquery/google/cloud/bigquery/client.py +++ b/bigquery/google/cloud/bigquery/client.py @@ -64,10 +64,10 @@ class Client(ClientWithProject): passed), falls back to the default inferred from the environment. - :type _http: :class:`~httplib2.Http` + :type _http: :class:`~requests.Session` :param _http: (Optional) HTTP object to make requests. Can be any object that defines ``request()`` with the same interface as - :meth:`~httplib2.Http.request`. If not passed, an + :meth:`requests.Session.request`. If not passed, an ``_http`` object is created that is bound to the ``credentials`` for the current object. This parameter should be considered private, and could diff --git a/bigquery/google/cloud/bigquery/dataset.py b/bigquery/google/cloud/bigquery/dataset.py index bce74ca9f366..8fb986cb848d 100644 --- a/bigquery/google/cloud/bigquery/dataset.py +++ b/bigquery/google/cloud/bigquery/dataset.py @@ -364,7 +364,7 @@ def _parse_access_grants(access): def _set_properties(self, api_response): """Update properties from resource in body of ``api_response`` - :type api_response: httplib2.Response + :type api_response: dict :param api_response: response returned from an API call. """ self._properties.clear() diff --git a/bigquery/google/cloud/bigquery/job.py b/bigquery/google/cloud/bigquery/job.py index ef5353f9ff14..c2d1feee7120 100644 --- a/bigquery/google/cloud/bigquery/job.py +++ b/bigquery/google/cloud/bigquery/job.py @@ -14,7 +14,6 @@ """Define API Jobs.""" -import collections import threading import six @@ -58,8 +57,6 @@ 'tableUnavailable': http_client.BAD_REQUEST, } -_FakeResponse = collections.namedtuple('_FakeResponse', ['status']) - def _error_result_to_exception(error_result): """Maps BigQuery error reasons to an exception. @@ -79,13 +76,8 @@ def _error_result_to_exception(error_result): reason = error_result.get('reason') status_code = _ERROR_REASON_TO_EXCEPTION.get( reason, http_client.INTERNAL_SERVER_ERROR) - # make_exception expects an httplib2 response object. - fake_response = _FakeResponse(status=status_code) - return exceptions.make_exception( - fake_response, - error_result.get('message', ''), - error_info=error_result, - use_json=False) + return exceptions.from_http_status( + status_code, error_result.get('message', ''), errors=[error_result]) class Compression(_EnumProperty): @@ -307,7 +299,7 @@ def _scrub_local_properties(self, cleaned): def _set_properties(self, api_response): """Update properties from resource in body of ``api_response`` - :type api_response: httplib2.Response + :type api_response: dict :param api_response: response returned from an API call """ cleaned = api_response.copy() diff --git a/bigquery/google/cloud/bigquery/query.py b/bigquery/google/cloud/bigquery/query.py index d596deadfb40..502953b2c828 100644 --- a/bigquery/google/cloud/bigquery/query.py +++ b/bigquery/google/cloud/bigquery/query.py @@ -310,7 +310,7 @@ def schema(self): def _set_properties(self, api_response): """Update properties from resource in body of ``api_response`` - :type api_response: httplib2.Response + :type api_response: dict :param api_response: response returned from an API call """ self._properties.clear() diff --git a/bigquery/google/cloud/bigquery/table.py b/bigquery/google/cloud/bigquery/table.py index f7752bb8fc36..c32832a926ce 100644 --- a/bigquery/google/cloud/bigquery/table.py +++ b/bigquery/google/cloud/bigquery/table.py @@ -17,7 +17,6 @@ import datetime import os -import httplib2 import six import google.auth.transport.requests @@ -25,10 +24,9 @@ from google.resumable_media.requests import MultipartUpload from google.resumable_media.requests import ResumableUpload +from google.cloud import exceptions from google.cloud._helpers import _datetime_from_microseconds from google.cloud._helpers import _millis_from_datetime -from google.cloud.exceptions import NotFound -from google.cloud.exceptions import make_exception from google.cloud.iterator import HTTPIterator from google.cloud.bigquery.schema import SchemaField from google.cloud.bigquery._helpers import _item_to_row @@ -474,7 +472,7 @@ def _require_client(self, client): def _set_properties(self, api_response): """Update properties from resource in body of ``api_response`` - :type api_response: httplib2.Response + :type api_response: dict :param api_response: response returned from an API call """ self._properties.clear() @@ -563,7 +561,7 @@ def exists(self, client=None): try: client._connection.api_request(method='GET', path=self.path, query_params={'fields': 'id'}) - except NotFound: + except exceptions.NotFound: return False else: return True @@ -1113,7 +1111,7 @@ def upload_from_file(self, client, file_obj, metadata, size, num_retries) return client.job_from_resource(created_json) except resumable_media.InvalidResponse as exc: - _raise_from_invalid_response(exc) + raise exceptions.from_http_response(exc.response) # pylint: enable=too-many-arguments,too-many-locals @@ -1298,22 +1296,3 @@ def _get_upload_metadata(source_format, schema, dataset, name): 'load': load_config, }, } - - -def _raise_from_invalid_response(error, error_info=None): - """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) diff --git a/bigquery/tests/unit/test__http.py b/bigquery/tests/unit/test__http.py index 9972e9859313..b8af254d3614 100644 --- a/bigquery/tests/unit/test__http.py +++ b/bigquery/tests/unit/test__http.py @@ -15,6 +15,7 @@ import unittest import mock +import requests class TestConnection(unittest.TestCase): @@ -55,10 +56,12 @@ def test_extra_headers(self): from google.cloud import _http as base_http from google.cloud.bigquery 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) @@ -68,15 +71,14 @@ 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, ) diff --git a/bigquery/tests/unit/test_job.py b/bigquery/tests/unit/test_job.py index fcb518d9c502..d2ec7027d5e6 100644 --- a/bigquery/tests/unit/test_job.py +++ b/bigquery/tests/unit/test_job.py @@ -31,7 +31,7 @@ def test_simple(self): exception = self._call_fut(error_result) self.assertEqual(exception.code, http_client.BAD_REQUEST) self.assertTrue(exception.message.startswith('bad request')) - self.assertIn("'reason': 'invalid'", exception.message) + self.assertIn(error_result, exception.errors) def test_missing_reason(self): error_result = {} diff --git a/bigquery/tests/unit/test_table.py b/bigquery/tests/unit/test_table.py index 502c0495f9c9..eebb40a2e736 100644 --- a/bigquery/tests/unit/test_table.py +++ b/bigquery/tests/unit/test_table.py @@ -1744,9 +1744,11 @@ def _make_table(): def _make_response(status_code, content='', headers={}): """Make a mock HTTP response.""" import requests - response = mock.create_autospec(requests.Response, instance=True) - response.content = content.encode('utf-8') - response.headers = headers + response = requests.Response() + response.request = requests.Request( + 'POST', 'http://example.com').prepare() + response._content = content.encode('utf-8') + response.headers.update(headers) response.status_code = status_code return response @@ -1921,7 +1923,7 @@ def test_upload_from_file_failure(self): table.upload_from_file( file_obj, source_format='CSV', rewind=True) - assert exc_info.value.message == response.content.decode('utf-8') + assert response.text in exc_info.value.message assert exc_info.value.errors == [] def test_upload_from_file_bad_mode(self): diff --git a/core/google/cloud/_helpers.py b/core/google/cloud/_helpers.py index fdb22ecdf09c..83f6db3a20fc 100644 --- a/core/google/cloud/_helpers.py +++ b/core/google/cloud/_helpers.py @@ -25,12 +25,11 @@ import re from threading import local as Local -import google_auth_httplib2 -import httplib2 import six from six.moves import http_client import google.auth +import google.auth.transport.requests from google.protobuf import duration_pb2 from google.protobuf import timestamp_pb2 @@ -550,7 +549,7 @@ def make_secure_channel(credentials, user_agent, host, extra_options=()): :returns: gRPC secure channel with credentials attached. """ target = '%s:%d' % (host, http_client.HTTPS_PORT) - http_request = google_auth_httplib2.Request(http=httplib2.Http()) + http_request = google.auth.transport.requests.Request() user_agent_option = ('grpc.primary_user_agent', user_agent) options = (user_agent_option,) + extra_options diff --git a/core/google/cloud/_http.py b/core/google/cloud/_http.py index b7c17ca91d6d..2a0a24e38006 100644 --- a/core/google/cloud/_http.py +++ b/core/google/cloud/_http.py @@ -18,10 +18,9 @@ import platform from pkg_resources import get_distribution -import six from six.moves.urllib.parse import urlencode -from google.cloud.exceptions import make_exception +from google.cloud import exceptions API_BASE_URL = 'https://www.googleapis.com' @@ -67,8 +66,9 @@ def credentials(self): def http(self): """A getter for the HTTP transport used in talking to the API. - :rtype: :class:`httplib2.Http` - :returns: A Http object used to transport data. + Returns: + google.auth.transport.requests.AuthorizedSession: + A :class:`requests.Session` instance. """ return self._client._http @@ -168,23 +168,13 @@ def _make_request(self, method, url, data=None, content_type=None, 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, - returned by :meth:`_do_request`. + :rtype: :class:`requests.Response` + :returns: The HTTP response. """ headers = headers or {} headers.update(self._EXTRA_HEADERS) headers['Accept-Encoding'] = 'gzip' - if data: - content_length = len(str(data)) - else: - content_length = 0 - - # NOTE: str is intended, bytes are sufficient for headers. - headers['Content-Length'] = str(content_length) - if content_type: headers['Content-Type'] = content_type @@ -215,12 +205,11 @@ def _do_request(self, method, url, headers, data, (Optional) 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. + :rtype: :class:`requests.Response` + :returns: The HTTP response. """ - return self.http.request(uri=url, method=method, headers=headers, - body=data) + return self.http.request( + url=url, method=method, headers=headers, data=data) def api_request(self, method, path, query_params=None, data=None, content_type=None, headers=None, @@ -281,7 +270,7 @@ def api_request(self, method, path, query_params=None, :raises ~google.cloud.exceptions.GoogleCloudError: if the response code is not 200 OK. - :raises TypeError: if the response content type is not JSON. + :raises ValueError: if the response content type is not JSON. :rtype: dict or str :returns: The API response payload, either as a raw string or a dictionary if the response is valid JSON. @@ -296,21 +285,14 @@ def api_request(self, method, path, query_params=None, data = json.dumps(data) content_type = 'application/json' - response, content = self._make_request( + response = self._make_request( method=method, url=url, data=data, content_type=content_type, headers=headers, target_object=_target_object) - if not 200 <= response.status < 300: - raise make_exception(response, content, - error_info=method + ' ' + url) + if not 200 <= response.status_code < 300: + raise exceptions.from_http_response(response) - 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 + if expect_json and response.content: + return response.json() + else: + return response.content diff --git a/core/google/cloud/client.py b/core/google/cloud/client.py index 468cf9e40a52..7403be71f521 100644 --- a/core/google/cloud/client.py +++ b/core/google/cloud/client.py @@ -18,11 +18,11 @@ import json from pickle import PicklingError -import google_auth_httplib2 import six import google.auth import google.auth.credentials +import google.auth.transport.requests from google.cloud._helpers import _determine_default_project from google.oauth2 import service_account @@ -87,36 +87,23 @@ class Client(_ClientFactoryMixin): Stores ``credentials`` and an HTTP object so that subclasses can pass them along to a connection class. - If no value is passed in for ``_http``, a :class:`httplib2.Http` object + If no value is passed in for ``_http``, a :class:`requests.Session` object will be created and authorized with the ``credentials``. If not, the ``credentials`` and ``_http`` need not be related. Callers and subclasses may seek to use the private key from ``credentials`` to sign data. - A custom (non-``httplib2``) HTTP object must have a ``request`` method - which accepts the following arguments: - - * ``uri`` - * ``method`` - * ``body`` - * ``headers`` - - In addition, ``redirections`` and ``connection_type`` may be used. - - A custom ``_http`` object will also need to be able to add a bearer token - to API requests and handle token refresh on 401 errors. - :type credentials: :class:`~google.auth.credentials.Credentials` :param credentials: (Optional) The OAuth2 Credentials to use for this client. If not passed (and if no ``_http`` object is passed), falls back to the default inferred from the environment. - :type _http: :class:`~httplib2.Http` + :type _http: :class:`~requests.Session` :param _http: (Optional) HTTP object to make requests. Can be any object that defines ``request()`` with the same interface as - :meth:`~httplib2.Http.request`. If not passed, an + :meth:`requests.Session.request`. If not passed, an ``_http`` object is created that is bound to the ``credentials`` for the current object. This parameter should be considered private, and could @@ -151,12 +138,13 @@ def __getstate__(self): def _http(self): """Getter for object used for HTTP transport. - :rtype: :class:`~httplib2.Http` + :rtype: :class:`~requests.Session` :returns: An HTTP object. """ if self._http_internal is None: - self._http_internal = google_auth_httplib2.AuthorizedHttp( - self._credentials) + self._http_internal = ( + google.auth.transport.requests.AuthorizedSession( + self._credentials)) return self._http_internal @@ -204,10 +192,10 @@ class ClientWithProject(Client, _ClientProjectMixin): passed), falls back to the default inferred from the environment. - :type _http: :class:`~httplib2.Http` + :type _http: :class:`~requests.Session` :param _http: (Optional) HTTP object to make requests. Can be any object that defines ``request()`` with the same interface as - :meth:`~httplib2.Http.request`. If not passed, an + :meth:`~requests.Session.request`. If not passed, an ``_http`` object is created that is bound to the ``credentials`` for the current object. This parameter should be considered private, and could diff --git a/core/google/cloud/exceptions.py b/core/google/cloud/exceptions.py index e911980c6328..2e7eca3be98d 100644 --- a/core/google/cloud/exceptions.py +++ b/core/google/cloud/exceptions.py @@ -21,7 +21,6 @@ from __future__ import absolute_import import copy -import json import six @@ -186,56 +185,55 @@ class GatewayTimeout(ServerError): code = 504 -def make_exception(response, content, error_info=None, use_json=True): - """Factory: create exception based on HTTP response code. +def from_http_status(status_code, message, errors=()): + """Create a :class:`GoogleCloudError` from an HTTP status code. - :type response: :class:`httplib2.Response` or other HTTP response object - :param response: A response object that defines a status code as the - status attribute. + Args: + status_code (int): The HTTP status code. + message (str): The exception message. + errors (Sequence[Any]): A list of additional error information. + + Returns: + GoogleCloudError: An instance of the appropriate subclass of + :class:`GoogleCloudError`. + """ + error_class = _HTTP_CODE_TO_EXCEPTION.get(status_code, GoogleCloudError) + error = error_class(message, errors) + + if error.code is None: + error.code = status_code + + return error - :type content: str or dictionary - :param content: The body of the HTTP error response. - :type error_info: str - :param error_info: Optional string giving extra information about the - failed request. +def from_http_response(response): + """Create a :class:`GoogleCloudError` from a :class:`requests.Response`. - :type use_json: bool - :param use_json: Flag indicating if ``content`` is expected to be JSON. + Args: + response (requests.Response): The HTTP response. - :rtype: instance of :class:`GoogleCloudError`, or a concrete subclass. - :returns: Exception specific to the error response. + Returns: + GoogleCloudError: An instance of the appropriate subclass of + :class:`GoogleCloudError`, with the message and errors populated + from the response. """ - if isinstance(content, six.binary_type): - content = content.decode('utf-8') - - if isinstance(content, six.string_types): - payload = None - if use_json: - try: - payload = json.loads(content) - except ValueError: - # Expected JSON but received something else. - pass - if payload is None: - payload = {'error': {'message': content}} - else: - payload = content - - message = payload.get('error', {}).get('message', '') + try: + payload = response.json() + except ValueError: + payload = {'error': {'message': response.text or 'unknown error'}} + + error_message = payload.get('error', {}).get('message', 'unknown error') errors = payload.get('error', {}).get('errors', ()) - if error_info is not None: - message += ' (%s)' % (error_info,) + message = '{method} {url}: {error}'.format( + method=response.request.method, + url=response.request.url, + error=error_message) - try: - klass = _HTTP_CODE_TO_EXCEPTION[response.status] - except KeyError: - error = GoogleCloudError(message, errors) - error.code = response.status - else: - error = klass(message, errors) - return error + exception = from_http_status( + response.status_code, message, errors=errors) + exception.response = response + return exception def _walk_subclasses(klass): diff --git a/core/setup.py b/core/setup.py index ba84f2347d18..2a221ffe04b9 100644 --- a/core/setup.py +++ b/core/setup.py @@ -51,11 +51,10 @@ REQUIREMENTS = [ - 'httplib2 >= 0.9.1', 'googleapis-common-protos >= 1.3.4', 'protobuf >= 3.0.0', 'google-auth >= 0.4.0, < 2.0.0dev', - 'google-auth-httplib2', + 'requests >= 2.4.0, < 3.0.0dev', 'six', 'tenacity >= 4.0.0, <5.0.0dev' ] diff --git a/core/tests/unit/test__http.py b/core/tests/unit/test__http.py index 22df11566811..abf630b9a41f 100644 --- a/core/tests/unit/test__http.py +++ b/core/tests/unit/test__http.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 class TestConnection(unittest.TestCase): @@ -52,7 +55,24 @@ def test_user_agent_format(self): self.assertEqual(conn.USER_AGENT, expected_ua) +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 TestJSONConnection(unittest.TestCase): + JSON_HEADERS = {'content-type': 'application/json'} + EMPTY_JSON_RESPONSE = make_response(content=b'{}', headers=JSON_HEADERS) @staticmethod def _get_target_class(): @@ -119,129 +139,123 @@ def test_build_api_url_w_extra_query_params(self): self.assertEqual(parms['qux'], ['quux', 'corge']) def test__make_request_no_data_no_content_type_no_headers(self): - http = _Http( - {'status': '200', 'content-type': 'text/plain'}, - b'', - ) + http = make_requests_session([make_response()]) client = mock.Mock(_http=http, spec=['_http']) conn = self._make_one(client) - URI = 'http://example.com/test' - headers, content = conn._make_request('GET', URI) - self.assertEqual(headers['status'], '200') - self.assertEqual(headers['content-type'], 'text/plain') - self.assertEqual(content, b'') - self.assertEqual(http._called_with['method'], 'GET') - self.assertEqual(http._called_with['uri'], URI) - self.assertIsNone(http._called_with['body']) + url = 'http://example.com/test' + + response = conn._make_request('GET', url) + + self.assertEqual(response.status_code, http_client.OK) + self.assertEqual(response.content, b'') + expected_headers = { 'Accept-Encoding': 'gzip', - 'Content-Length': '0', 'User-Agent': conn.USER_AGENT, } - self.assertEqual(http._called_with['headers'], expected_headers) + http.request.assert_called_once_with( + method='GET', url=url, headers=expected_headers, data=None) def test__make_request_w_data_no_extra_headers(self): - http = _Http( - {'status': '200', 'content-type': 'text/plain'}, - b'', - ) + http = make_requests_session([make_response()]) client = mock.Mock(_http=http, spec=['_http']) conn = self._make_one(client) - URI = 'http://example.com/test' - conn._make_request('GET', URI, {}, 'application/json') - self.assertEqual(http._called_with['method'], 'GET') - self.assertEqual(http._called_with['uri'], URI) - self.assertEqual(http._called_with['body'], {}) + url = 'http://example.com/test' + data = b'data' + + conn._make_request('GET', url, data, 'application/json') + expected_headers = { 'Accept-Encoding': 'gzip', - 'Content-Length': '0', 'Content-Type': 'application/json', 'User-Agent': conn.USER_AGENT, } - self.assertEqual(http._called_with['headers'], expected_headers) + http.request.assert_called_once_with( + method='GET', url=url, headers=expected_headers, data=data) def test__make_request_w_extra_headers(self): - http = _Http( - {'status': '200', 'content-type': 'text/plain'}, - b'', - ) + http = make_requests_session([make_response()]) client = mock.Mock(_http=http, spec=['_http']) conn = self._make_one(client) - URI = 'http://example.com/test' - conn._make_request('GET', URI, headers={'X-Foo': 'foo'}) - self.assertEqual(http._called_with['method'], 'GET') - self.assertEqual(http._called_with['uri'], URI) - self.assertIsNone(http._called_with['body']) + + url = 'http://example.com/test' + conn._make_request('GET', url, headers={'X-Foo': 'foo'}) + expected_headers = { 'Accept-Encoding': 'gzip', - 'Content-Length': '0', 'X-Foo': 'foo', 'User-Agent': conn.USER_AGENT, } - self.assertEqual(http._called_with['headers'], expected_headers) + http.request.assert_called_once_with( + method='GET', url=url, headers=expected_headers, data=None) def test_api_request_defaults(self): - http = _Http( - {'status': '200', 'content-type': 'application/json'}, - b'{}', - ) + http = make_requests_session([ + make_response(content=b'{}', headers=self.JSON_HEADERS)]) client = mock.Mock(_http=http, spec=['_http']) conn = self._make_mock_one(client) - PATH = '/path/required' - # Intended to emulate self.mock_template - URI = '/'.join([ - conn.API_BASE_URL, - 'mock', - '%s%s' % (conn.API_VERSION, PATH), - ]) - self.assertEqual(conn.api_request('GET', PATH), {}) - self.assertEqual(http._called_with['method'], 'GET') - self.assertEqual(http._called_with['uri'], URI) - self.assertIsNone(http._called_with['body']) + path = '/path/required' + + self.assertEqual(conn.api_request('GET', path), {}) + expected_headers = { 'Accept-Encoding': 'gzip', - 'Content-Length': '0', 'User-Agent': conn.USER_AGENT, } - self.assertEqual(http._called_with['headers'], expected_headers) + expected_url = '{base}/mock/{version}{path}'.format( + base=conn.API_BASE_URL, + version=conn.API_VERSION, + path=path) + http.request.assert_called_once_with( + method='GET', + url=expected_url, + headers=expected_headers, + data=None) def test_api_request_w_non_json_response(self): - http = _Http( - {'status': '200', 'content-type': 'text/plain'}, - b'CONTENT', - ) + http = make_requests_session([ + make_response(content=b'content')]) client = mock.Mock(_http=http, spec=['_http']) conn = self._make_mock_one(client) - self.assertRaises(TypeError, conn.api_request, 'GET', '/') + with self.assertRaises(ValueError): + conn.api_request('GET', '/') def test_api_request_wo_json_expected(self): - http = _Http( - {'status': '200', 'content-type': 'text/plain'}, - b'CONTENT', - ) + http = make_requests_session([ + make_response(content=b'content')]) client = mock.Mock(_http=http, spec=['_http']) conn = self._make_mock_one(client) - self.assertEqual(conn.api_request('GET', '/', expect_json=False), - b'CONTENT') + + result = conn.api_request('GET', '/', expect_json=False) + + self.assertEqual(result, b'content') def test_api_request_w_query_params(self): from six.moves.urllib.parse import parse_qs from six.moves.urllib.parse import urlsplit - http = _Http( - {'status': '200', 'content-type': 'application/json'}, - b'{}', - ) + http = make_requests_session([self.EMPTY_JSON_RESPONSE]) client = mock.Mock(_http=http, spec=['_http']) conn = self._make_mock_one(client) - self.assertEqual(conn.api_request('GET', '/', { + + result = conn.api_request('GET', '/', { 'foo': 'bar', 'baz': ['qux', 'quux'] - }), {}) - self.assertEqual(http._called_with['method'], 'GET') - uri = http._called_with['uri'] - scheme, netloc, path, qs, _ = urlsplit(uri) + }) + + self.assertEqual(result, {}) + + expected_headers = { + 'Accept-Encoding': 'gzip', + 'User-Agent': conn.USER_AGENT, + } + http.request.assert_called_once_with( + method='GET', url=mock.ANY, headers=expected_headers, + data=None) + + url = http.request.call_args[1]['url'] + scheme, netloc, path, qs, _ = urlsplit(url) self.assertEqual('%s://%s' % (scheme, netloc), conn.API_BASE_URL) # Intended to emulate self.mock_template PATH = '/'.join([ @@ -254,175 +268,84 @@ def test_api_request_w_query_params(self): parms = dict(parse_qs(qs)) self.assertEqual(parms['foo'], ['bar']) self.assertEqual(parms['baz'], ['qux', 'quux']) - self.assertIsNone(http._called_with['body']) - expected_headers = { - 'Accept-Encoding': 'gzip', - 'Content-Length': '0', - 'User-Agent': conn.USER_AGENT, - } - self.assertEqual(http._called_with['headers'], expected_headers) def test_api_request_w_headers(self): - from six.moves.urllib.parse import urlsplit - - http = _Http( - {'status': '200', 'content-type': 'application/json'}, - b'{}', - ) + http = make_requests_session([self.EMPTY_JSON_RESPONSE]) client = mock.Mock(_http=http, spec=['_http']) conn = self._make_mock_one(client) - self.assertEqual( - conn.api_request('GET', '/', headers={'X-Foo': 'bar'}), {}) - self.assertEqual(http._called_with['method'], 'GET') - uri = http._called_with['uri'] - scheme, netloc, path, qs, _ = urlsplit(uri) - self.assertEqual('%s://%s' % (scheme, netloc), conn.API_BASE_URL) - # Intended to emulate self.mock_template - PATH = '/'.join([ - '', - 'mock', - conn.API_VERSION, - '', - ]) - self.assertEqual(path, PATH) - self.assertEqual(qs, '') - self.assertIsNone(http._called_with['body']) + + result = conn.api_request('GET', '/', headers={'X-Foo': 'bar'}) + self.assertEqual(result, {}) + expected_headers = { 'Accept-Encoding': 'gzip', - 'Content-Length': '0', 'User-Agent': conn.USER_AGENT, 'X-Foo': 'bar', } - self.assertEqual(http._called_with['headers'], expected_headers) + http.request.assert_called_once_with( + method='GET', url=mock.ANY, headers=expected_headers, + data=None) def test_api_request_w_extra_headers(self): - from six.moves.urllib.parse import urlsplit - - http = _Http( - {'status': '200', 'content-type': 'application/json'}, - b'{}', - ) + http = make_requests_session([self.EMPTY_JSON_RESPONSE]) client = mock.Mock(_http=http, spec=['_http']) conn = self._make_mock_one(client) conn._EXTRA_HEADERS = { 'X-Baz': 'dax-quux', 'X-Foo': 'not-bar', # Collision with ``headers``. } - self.assertEqual( - conn.api_request('GET', '/', headers={'X-Foo': 'bar'}), {}) - self.assertEqual(http._called_with['method'], 'GET') - uri = http._called_with['uri'] - scheme, netloc, path, qs, _ = urlsplit(uri) - self.assertEqual('%s://%s' % (scheme, netloc), conn.API_BASE_URL) - # Intended to emulate self.mock_template - PATH = '/'.join([ - '', - 'mock', - conn.API_VERSION, - '', - ]) - self.assertEqual(path, PATH) - self.assertEqual(qs, '') - self.assertIsNone(http._called_with['body']) + + result = conn.api_request('GET', '/', headers={'X-Foo': 'bar'}) + + self.assertEqual(result, {}) + expected_headers = { 'Accept-Encoding': 'gzip', - 'Content-Length': '0', 'User-Agent': conn.USER_AGENT, 'X-Foo': 'not-bar', # The one passed-in is overridden. 'X-Baz': 'dax-quux', } - self.assertEqual(http._called_with['headers'], expected_headers) + http.request.assert_called_once_with( + method='GET', url=mock.ANY, headers=expected_headers, + data=None) def test_api_request_w_data(self): - import json - - DATA = {'foo': 'bar'} - DATAJ = json.dumps(DATA) - http = _Http( - {'status': '200', 'content-type': 'application/json'}, - b'{}', - ) + http = make_requests_session([self.EMPTY_JSON_RESPONSE]) client = mock.Mock(_http=http, spec=['_http']) conn = self._make_mock_one(client) - # Intended to emulate self.mock_template - URI = '/'.join([ - conn.API_BASE_URL, - 'mock', - conn.API_VERSION, - '', - ]) - self.assertEqual(conn.api_request('POST', '/', data=DATA), {}) - self.assertEqual(http._called_with['method'], 'POST') - self.assertEqual(http._called_with['uri'], URI) - self.assertEqual(http._called_with['body'], DATAJ) + + data = {'foo': 'bar'} + self.assertEqual(conn.api_request('POST', '/', data=data), {}) + + expected_data = json.dumps(data) + expected_headers = { 'Accept-Encoding': 'gzip', - 'Content-Length': str(len(DATAJ)), 'Content-Type': 'application/json', 'User-Agent': conn.USER_AGENT, } - self.assertEqual(http._called_with['headers'], expected_headers) + + http.request.assert_called_once_with( + method='POST', url=mock.ANY, headers=expected_headers, + data=expected_data) def test_api_request_w_404(self): - from google.cloud.exceptions import NotFound + from google.cloud import exceptions - http = _Http( - {'status': '404', 'content-type': 'text/plain'}, - b'{}' - ) + http = make_requests_session([make_response(http_client.NOT_FOUND)]) client = mock.Mock(_http=http, spec=['_http']) conn = self._make_mock_one(client) - self.assertRaises(NotFound, conn.api_request, 'GET', '/') - def test_api_request_w_500(self): - from google.cloud.exceptions import InternalServerError + with self.assertRaises(exceptions.NotFound): + conn.api_request('GET', '/') - http = _Http( - {'status': '500', 'content-type': 'text/plain'}, - b'{}', - ) - client = mock.Mock(_http=http, spec=['_http']) - conn = self._make_mock_one(client) - self.assertRaises(InternalServerError, conn.api_request, 'GET', '/') + def test_api_request_w_500(self): + from google.cloud import exceptions - def test_api_request_non_binary_response(self): - http = _Http( - {'status': '200', 'content-type': 'application/json'}, - u'{}', - ) + http = make_requests_session([ + make_response(http_client.INTERNAL_SERVER_ERROR)]) client = mock.Mock(_http=http, spec=['_http']) conn = self._make_mock_one(client) - 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.assertIsNone(http._called_with['body']) - expected_headers = { - 'Accept-Encoding': 'gzip', - 'Content-Length': '0', - 'User-Agent': conn.USER_AGENT, - } - self.assertEqual(http._called_with['headers'], expected_headers) - - -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 + with self.assertRaises(exceptions.InternalServerError): + conn.api_request('GET', '/') diff --git a/core/tests/unit/test_client.py b/core/tests/unit/test_client.py index 25667712c69a..bed3ebe2c036 100644 --- a/core/tests/unit/test_client.py +++ b/core/tests/unit/test_client.py @@ -132,17 +132,17 @@ def test__http_property_new(self): client = self._make_one(credentials=credentials) self.assertIsNone(client._http_internal) - patch = mock.patch('google_auth_httplib2.AuthorizedHttp', - return_value=mock.sentinel.http) - with patch as mocked: + authorized_session_patch = mock.patch( + 'google.auth.transport.requests.AuthorizedSession', + return_value=mock.sentinel.http) + with authorized_session_patch as AuthorizedSession: self.assertIs(client._http, mock.sentinel.http) # Check the mock. - mocked.assert_called_once_with(credentials) - self.assertEqual(mocked.call_count, 1) + AuthorizedSession.assert_called_once_with(credentials) # Make sure the cached value is used on subsequent access. self.assertIs(client._http_internal, mock.sentinel.http) self.assertIs(client._http, mock.sentinel.http) - self.assertEqual(mocked.call_count, 1) + self.assertEqual(AuthorizedSession.call_count, 1) class TestClientWithProject(unittest.TestCase): diff --git a/core/tests/unit/test_exceptions.py b/core/tests/unit/test_exceptions.py index b3488296eff4..4be260831825 100644 --- a/core/tests/unit/test_exceptions.py +++ b/core/tests/unit/test_exceptions.py @@ -12,139 +12,116 @@ # See the License for the specific language governing permissions and # limitations under the License. -import unittest +import json +import requests +from six.moves import http_client -class Test_GoogleCloudError(unittest.TestCase): +from google.cloud import exceptions - @staticmethod - def _get_target_class(): - from google.cloud.exceptions import GoogleCloudError - return GoogleCloudError +def test_create_google_cloud_error(): + exception = exceptions.GoogleCloudError('Testing') + exception.code = 600 + assert str(exception) == '600 Testing' + assert exception.message == 'Testing' + assert exception.errors == [] - def _make_one(self, message, errors=()): - return self._get_target_class()(message, errors=errors) - def test_ctor_defaults(self): - e = self._make_one('Testing') - e.code = 600 - self.assertEqual(str(e), '600 Testing') - self.assertEqual(e.message, 'Testing') - self.assertEqual(list(e.errors), []) +def test_create_google_cloud_error_with_args(): + error = { + 'domain': 'global', + 'location': 'test', + 'locationType': 'testing', + 'message': 'Testing', + 'reason': 'test', + } + exception = exceptions.GoogleCloudError('Testing', [error]) + exception.code = 600 + assert str(exception) == '600 Testing' + assert exception.message == 'Testing' + assert exception.errors == [error] - def test_ctor_explicit(self): - ERROR = { - 'domain': 'global', - 'location': 'test', - 'locationType': 'testing', - 'message': 'Testing', - 'reason': 'test', - } - e = self._make_one('Testing', [ERROR]) - e.code = 600 - self.assertEqual(str(e), '600 Testing') - self.assertEqual(e.message, 'Testing') - self.assertEqual(list(e.errors), [ERROR]) - - -class Test_make_exception(unittest.TestCase): - - def _call_fut(self, response, content, error_info=None, use_json=True): - from google.cloud.exceptions import make_exception - - return make_exception(response, content, error_info=error_info, - use_json=use_json) - - def test_hit_w_content_as_str(self): - from google.cloud.exceptions import NotFound - - response = _Response(404) - content = b'{"error": {"message": "Not Found"}}' - exception = self._call_fut(response, content) - self.assertIsInstance(exception, NotFound) - self.assertEqual(exception.message, 'Not Found') - self.assertEqual(list(exception.errors), []) - - def test_hit_w_content_as_unicode(self): - import six - from google.cloud._helpers import _to_bytes - from google.cloud.exceptions import NotFound - - error_message = u'That\u2019s not found.' - expected = u'404 %s' % (error_message,) - - response = _Response(404) - content = u'{"error": {"message": "%s" }}' % (error_message,) - - exception = self._call_fut(response, content) - if six.PY2: - self.assertEqual(str(exception), - _to_bytes(expected, encoding='utf-8')) - else: # pragma: NO COVER - self.assertEqual(str(exception), expected) - - self.assertIsInstance(exception, NotFound) - self.assertEqual(exception.message, error_message) - self.assertEqual(list(exception.errors), []) - - def test_hit_w_content_as_unicode_as_py3(self): - import six - from google.cloud._testing import _Monkey - from google.cloud.exceptions import NotFound - - error_message = u'That is not found.' - expected = u'404 %s' % (error_message,) - - with _Monkey(six, PY2=False): - response = _Response(404) - content = u'{"error": {"message": "%s" }}' % (error_message,) - exception = self._call_fut(response, content) - - self.assertIsInstance(exception, NotFound) - self.assertEqual(exception.message, error_message) - self.assertEqual(list(exception.errors), []) - self.assertEqual(str(exception), expected) - - def test_miss_w_content_as_dict(self): - from google.cloud.exceptions import GoogleCloudError - - ERROR = { - 'domain': 'global', - 'location': 'test', - 'locationType': 'testing', - 'message': 'Testing', - 'reason': 'test', + +def test_from_http_status(): + message = 'message' + exception = exceptions.from_http_status(http_client.NOT_FOUND, message) + assert exception.code == http_client.NOT_FOUND + assert exception.message == message + assert exception.errors == [] + + +def test_from_http_status_with_errors(): + message = 'message' + errors = ['1', '2'] + exception = exceptions.from_http_status( + http_client.NOT_FOUND, message, errors=errors) + + assert isinstance(exception, exceptions.NotFound) + assert exception.code == http_client.NOT_FOUND + assert exception.message == message + assert exception.errors == errors + + +def test_from_http_status_unknown_code(): + message = 'message' + status_code = 156 + exception = exceptions.from_http_status(status_code, message) + assert exception.code == status_code + assert exception.message == message + + +def make_response(content): + response = requests.Response() + response._content = content + response.status_code = http_client.NOT_FOUND + response.request = requests.Request( + method='POST', url='https://example.com').prepare() + return response + + +def test_from_http_response_no_content(): + response = make_response(None) + + exception = exceptions.from_http_response(response) + + assert isinstance(exception, exceptions.NotFound) + assert exception.code == http_client.NOT_FOUND + assert exception.message == 'POST https://example.com/: unknown error' + assert exception.response == response + + +def test_from_http_response_text_content(): + response = make_response(b'message') + + exception = exceptions.from_http_response(response) + + assert isinstance(exception, exceptions.NotFound) + assert exception.code == http_client.NOT_FOUND + assert exception.message == 'POST https://example.com/: message' + + +def test_from_http_response_json_content(): + response = make_response(json.dumps({ + 'error': { + 'message': 'json message', + 'errors': ['1', '2'] } - response = _Response(600) - content = {"error": {"message": "Unknown Error", "errors": [ERROR]}} - exception = self._call_fut(response, content) - self.assertIsInstance(exception, GoogleCloudError) - self.assertEqual(exception.message, 'Unknown Error') - self.assertEqual(list(exception.errors), [ERROR]) - - def test_html_when_json_expected(self): - from google.cloud.exceptions import NotFound - - response = _Response(NotFound.code) - content = '404 Not Found' - exception = self._call_fut(response, content, use_json=True) - self.assertIsInstance(exception, NotFound) - self.assertEqual(exception.message, content) - self.assertEqual(list(exception.errors), []) - - def test_without_use_json(self): - from google.cloud.exceptions import TooManyRequests - - content = u'error-content' - response = _Response(TooManyRequests.code) - exception = self._call_fut(response, content, use_json=False) - - self.assertIsInstance(exception, TooManyRequests) - self.assertEqual(exception.message, content) - self.assertEqual(list(exception.errors), []) - - -class _Response(object): - def __init__(self, status): - self.status = status + }).encode('utf-8')) + + exception = exceptions.from_http_response(response) + + assert isinstance(exception, exceptions.NotFound) + assert exception.code == http_client.NOT_FOUND + assert exception.message == 'POST https://example.com/: json message' + assert exception.errors == ['1', '2'] + + +def test_from_http_response_bad_json_content(): + response = make_response(json.dumps({'meep': 'moop'}).encode('utf-8')) + + exception = exceptions.from_http_response(response) + + assert isinstance(exception, exceptions.NotFound) + assert exception.code == http_client.NOT_FOUND + assert exception.message == 'POST https://example.com/: unknown error' diff --git a/datastore/google/cloud/datastore/_http.py b/datastore/google/cloud/datastore/_http.py index 0723a97a0de4..de976f7e1bb3 100644 --- a/datastore/google/cloud/datastore/_http.py +++ b/datastore/google/cloud/datastore/_http.py @@ -39,7 +39,7 @@ def _request(http, project, method, data, base_url): """Make a request over the Http transport to the Cloud Datastore API. - :type http: :class:`~httplib2.Http` + :type http: :class:`requests.Session` :param http: HTTP object to make requests. :type project: str @@ -63,27 +63,26 @@ def _request(http, project, method, data, base_url): """ headers = { 'Content-Type': 'application/x-protobuf', - 'Content-Length': str(len(data)), 'User-Agent': connection_module.DEFAULT_USER_AGENT, connection_module.CLIENT_INFO_HEADER: _CLIENT_INFO, } api_url = build_api_url(project, method, base_url) - headers, content = http.request( - uri=api_url, method='POST', headers=headers, body=data) - status = headers['status'] - if status != '200': - error_status = status_pb2.Status.FromString(content) - raise exceptions.make_exception( - headers, error_status.message, use_json=False) + response = http.request( + url=api_url, method='POST', headers=headers, data=data) - return content + if response.status_code != 200: + error_status = status_pb2.Status.FromString(response.content) + raise exceptions.from_http_status( + response.status_code, error_status.message, errors=[error_status]) + + return response.content def _rpc(http, project, method, base_url, request_pb, response_pb_cls): """Make a protobuf RPC request. - :type http: :class:`~httplib2.Http` + :type http: :class:`requests.Session` :param http: HTTP object to make requests. :type project: str diff --git a/datastore/google/cloud/datastore/client.py b/datastore/google/cloud/datastore/client.py index af7d6d4f9113..0ccef9f5f8f0 100644 --- a/datastore/google/cloud/datastore/client.py +++ b/datastore/google/cloud/datastore/client.py @@ -177,10 +177,10 @@ class Client(ClientWithProject): passed), falls back to the default inferred from the environment. - :type _http: :class:`~httplib2.Http` + :type _http: :class:`~requests.Session` :param _http: (Optional) HTTP object to make requests. Can be any object that defines ``request()`` with the same interface as - :meth:`~httplib2.Http.request`. If not passed, an + :meth:`requests.Session.request`. If not passed, an ``_http`` object is created that is bound to the ``credentials`` for the current object. This parameter should be considered private, and could diff --git a/datastore/tests/system/test_system.py b/datastore/tests/system/test_system.py index 129018748e08..b33f7de21925 100644 --- a/datastore/tests/system/test_system.py +++ b/datastore/tests/system/test_system.py @@ -16,7 +16,7 @@ import os import unittest -import httplib2 +import requests import six from google.cloud._helpers import UTC @@ -57,7 +57,7 @@ def setUpModule(): Config.CLIENT = datastore.Client(namespace=test_namespace) else: credentials = EmulatorCreds() - http = httplib2.Http() # Un-authorized. + http = requests.Session() # Un-authorized. Config.CLIENT = datastore.Client(project=emulator_dataset, namespace=test_namespace, credentials=credentials, diff --git a/datastore/tests/unit/test__http.py b/datastore/tests/unit/test__http.py index db364ec4dd61..c416cd36671a 100644 --- a/datastore/tests/unit/test__http.py +++ b/datastore/tests/unit/test__http.py @@ -15,6 +15,9 @@ import unittest import mock +from six.moves import http_client + +import requests class Test__request(unittest.TestCase): @@ -32,29 +35,25 @@ def test_success(self): project = 'PROJECT' method = 'METHOD' data = b'DATA' - uri = 'http://api-url' - - # Make mock HTTP object with canned response. + base_url = 'http://api-url' response_data = 'CONTENT' - http = Http({'status': '200'}, response_data) + + http = _make_requests_session([_make_response(content=response_data)]) # Call actual function under test. - response = self._call_fut(http, project, method, data, uri) + response = self._call_fut(http, project, method, data, base_url) self.assertEqual(response, response_data) # Check that the mocks were called as expected. - called_with = http._called_with - self.assertEqual(len(called_with), 4) - self.assertTrue(called_with['uri'].startswith(uri)) - self.assertEqual(called_with['method'], 'POST') + expected_url = _build_expected_url(base_url, project, method) expected_headers = { 'Content-Type': 'application/x-protobuf', 'User-Agent': connection_module.DEFAULT_USER_AGENT, - 'Content-Length': '4', connection_module.CLIENT_INFO_HEADER: _CLIENT_INFO, } - self.assertEqual(called_with['headers'], expected_headers) - self.assertEqual(called_with['body'], data) + http.request.assert_called_once_with( + method='POST', url=expected_url, headers=expected_headers, + data=data) def test_failure(self): from google.cloud.exceptions import BadRequest @@ -66,17 +65,19 @@ def test_failure(self): data = 'DATA' uri = 'http://api-url' - # Make mock HTTP object with canned response. error = status_pb2.Status() error.message = 'Entity value is indexed.' error.code = code_pb2.FAILED_PRECONDITION - http = Http({'status': '400'}, error.SerializeToString()) - # Call actual function under test. + http = _make_requests_session([ + _make_response( + http_client.BAD_REQUEST, + content=error.SerializeToString()) + ]) + with self.assertRaises(BadRequest) as exc: self._call_fut(http, project, method, data, uri) - # Check that the mocks were called as expected. expected_message = '400 Entity value is indexed.' self.assertEqual(str(exc.exception), expected_message) @@ -147,7 +148,8 @@ def test_lookup_single_key_empty_response(self): read_options = datastore_pb2.ReadOptions() # Create mock HTTP and client with response. - http = Http({'status': '200'}, rsp_pb.SerializeToString()) + http = _make_requests_session( + [_make_response(content=rsp_pb.SerializeToString())]) client = mock.Mock( _http=http, _base_url='test.invalid', spec=['_http', '_base_url']) @@ -161,10 +163,9 @@ def test_lookup_single_key_empty_response(self): self.assertEqual(len(response.found), 0) self.assertEqual(len(response.missing), 0) self.assertEqual(len(response.deferred), 0) - cw = http._called_with - _verify_protobuf_call(self, cw, uri) - request = datastore_pb2.LookupRequest() - request.ParseFromString(cw['body']) + + request = _verify_protobuf_call( + http, uri, datastore_pb2.LookupRequest()) self.assertEqual(list(request.keys), [key_pb]) self.assertEqual(request.read_options, read_options) @@ -178,7 +179,8 @@ def test_lookup_single_key_empty_response_w_eventual(self): read_consistency=datastore_pb2.ReadOptions.EVENTUAL) # Create mock HTTP and client with response. - http = Http({'status': '200'}, rsp_pb.SerializeToString()) + http = _make_requests_session( + [_make_response(content=rsp_pb.SerializeToString())]) client = mock.Mock( _http=http, _base_url='test.invalid', spec=['_http', '_base_url']) @@ -192,10 +194,9 @@ def test_lookup_single_key_empty_response_w_eventual(self): self.assertEqual(len(response.found), 0) self.assertEqual(len(response.missing), 0) self.assertEqual(len(response.deferred), 0) - cw = http._called_with - _verify_protobuf_call(self, cw, uri) - request = datastore_pb2.LookupRequest() - request.ParseFromString(cw['body']) + + request = _verify_protobuf_call( + http, uri, datastore_pb2.LookupRequest()) self.assertEqual(list(request.keys), [key_pb]) self.assertEqual(request.read_options, read_options) @@ -209,7 +210,8 @@ def test_lookup_single_key_empty_response_w_transaction(self): read_options = datastore_pb2.ReadOptions(transaction=transaction) # Create mock HTTP and client with response. - http = Http({'status': '200'}, rsp_pb.SerializeToString()) + http = _make_requests_session( + [_make_response(content=rsp_pb.SerializeToString())]) client = mock.Mock( _http=http, _base_url='test.invalid', spec=['_http', '_base_url']) @@ -223,10 +225,9 @@ def test_lookup_single_key_empty_response_w_transaction(self): self.assertEqual(len(response.found), 0) self.assertEqual(len(response.missing), 0) self.assertEqual(len(response.deferred), 0) - cw = http._called_with - _verify_protobuf_call(self, cw, uri) - request = datastore_pb2.LookupRequest() - request.ParseFromString(cw['body']) + + request = _verify_protobuf_call( + http, uri, datastore_pb2.LookupRequest()) self.assertEqual(list(request.keys), [key_pb]) self.assertEqual(request.read_options, read_options) @@ -243,7 +244,8 @@ def test_lookup_single_key_nonempty_response(self): read_options = datastore_pb2.ReadOptions() # Create mock HTTP and client with response. - http = Http({'status': '200'}, rsp_pb.SerializeToString()) + http = _make_requests_session( + [_make_response(content=rsp_pb.SerializeToString())]) client = mock.Mock( _http=http, _base_url='test.invalid', spec=['_http', '_base_url']) @@ -260,10 +262,9 @@ def test_lookup_single_key_nonempty_response(self): found = response.found[0].entity self.assertEqual(found.key.path[0].kind, 'Kind') self.assertEqual(found.key.path[0].id, 1234) - cw = http._called_with - _verify_protobuf_call(self, cw, uri) - request = datastore_pb2.LookupRequest() - request.ParseFromString(cw['body']) + + request = _verify_protobuf_call( + http, uri, datastore_pb2.LookupRequest()) self.assertEqual(list(request.keys), [key_pb]) self.assertEqual(request.read_options, read_options) @@ -277,7 +278,8 @@ def test_lookup_multiple_keys_empty_response(self): read_options = datastore_pb2.ReadOptions() # Create mock HTTP and client with response. - http = Http({'status': '200'}, rsp_pb.SerializeToString()) + http = _make_requests_session( + [_make_response(content=rsp_pb.SerializeToString())]) client = mock.Mock( _http=http, _base_url='test.invalid', spec=['_http', '_base_url']) @@ -291,10 +293,9 @@ def test_lookup_multiple_keys_empty_response(self): self.assertEqual(len(response.found), 0) self.assertEqual(len(response.missing), 0) self.assertEqual(len(response.deferred), 0) - cw = http._called_with - _verify_protobuf_call(self, cw, uri) - request = datastore_pb2.LookupRequest() - request.ParseFromString(cw['body']) + + request = _verify_protobuf_call( + http, uri, datastore_pb2.LookupRequest()) self.assertEqual(list(request.keys), [key_pb1, key_pb2]) self.assertEqual(request.read_options, read_options) @@ -312,7 +313,8 @@ def test_lookup_multiple_keys_w_missing(self): read_options = datastore_pb2.ReadOptions() # Create mock HTTP and client with response. - http = Http({'status': '200'}, rsp_pb.SerializeToString()) + http = _make_requests_session( + [_make_response(content=rsp_pb.SerializeToString())]) client = mock.Mock( _http=http, _base_url='test.invalid', spec=['_http', '_base_url']) @@ -327,17 +329,14 @@ def test_lookup_multiple_keys_w_missing(self): self.assertEqual(len(response.deferred), 0) missing_keys = [result.entity.key for result in response.missing] self.assertEqual(missing_keys, [key_pb1, key_pb2]) - cw = http._called_with - _verify_protobuf_call(self, cw, uri) - request = datastore_pb2.LookupRequest() - request.ParseFromString(cw['body']) + + request = _verify_protobuf_call( + http, uri, datastore_pb2.LookupRequest()) self.assertEqual(list(request.keys), [key_pb1, key_pb2]) self.assertEqual(request.read_options, read_options) def test_lookup_multiple_keys_w_deferred(self): from google.cloud.proto.datastore.v1 import datastore_pb2 - from google.cloud import _http as connection_module - from google.cloud.datastore._http import _CLIENT_INFO project = 'PROJECT' key_pb1 = _make_key_pb(project) @@ -348,7 +347,8 @@ def test_lookup_multiple_keys_w_deferred(self): read_options = datastore_pb2.ReadOptions() # Create mock HTTP and client with response. - http = Http({'status': '200'}, rsp_pb.SerializeToString()) + http = _make_requests_session( + [_make_response(content=rsp_pb.SerializeToString())]) client = mock.Mock( _http=http, _base_url='test.invalid', spec=['_http', '_base_url']) @@ -362,19 +362,9 @@ def test_lookup_multiple_keys_w_deferred(self): self.assertEqual(len(response.found), 0) self.assertEqual(len(response.missing), 0) self.assertEqual(list(response.deferred), [key_pb1, key_pb2]) - cw = http._called_with - _verify_protobuf_call(self, cw, uri) - self.assertEqual(cw['uri'], uri) - self.assertEqual(cw['method'], 'POST') - expected_headers = { - 'Content-Type': 'application/x-protobuf', - 'User-Agent': connection_module.DEFAULT_USER_AGENT, - 'Content-Length': str(len(cw['body'])), - connection_module.CLIENT_INFO_HEADER: _CLIENT_INFO, - } - self.assertEqual(cw['headers'], expected_headers) - request = datastore_pb2.LookupRequest() - request.ParseFromString(cw['body']) + + request = _verify_protobuf_call( + http, uri, datastore_pb2.LookupRequest()) self.assertEqual(list(request.keys), [key_pb1, key_pb2]) self.assertEqual(request.read_options, read_options) @@ -399,7 +389,8 @@ def test_run_query_w_eventual_no_transaction(self): ) # Create mock HTTP and client with response. - http = Http({'status': '200'}, rsp_pb.SerializeToString()) + http = _make_requests_session( + [_make_response(content=rsp_pb.SerializeToString())]) client = mock.Mock( _http=http, _base_url='test.invalid', spec=['_http', '_base_url']) @@ -410,11 +401,10 @@ def test_run_query_w_eventual_no_transaction(self): # Check the result and verify the callers. self.assertEqual(response, rsp_pb) + uri = _build_expected_url(client._base_url, project, 'runQuery') - cw = http._called_with - _verify_protobuf_call(self, cw, uri) - request = datastore_pb2.RunQueryRequest() - request.ParseFromString(cw['body']) + request = _verify_protobuf_call( + http, uri, datastore_pb2.RunQueryRequest()) self.assertEqual(request.partition_id, partition_id) self.assertEqual(request.query, query_pb) self.assertEqual(request.read_options, read_options) @@ -440,7 +430,8 @@ def test_run_query_wo_eventual_w_transaction(self): ) # Create mock HTTP and client with response. - http = Http({'status': '200'}, rsp_pb.SerializeToString()) + http = _make_requests_session( + [_make_response(content=rsp_pb.SerializeToString())]) client = mock.Mock( _http=http, _base_url='test.invalid', spec=['_http', '_base_url']) @@ -451,11 +442,10 @@ def test_run_query_wo_eventual_w_transaction(self): # Check the result and verify the callers. self.assertEqual(response, rsp_pb) + uri = _build_expected_url(client._base_url, project, 'runQuery') - cw = http._called_with - _verify_protobuf_call(self, cw, uri) - request = datastore_pb2.RunQueryRequest() - request.ParseFromString(cw['body']) + request = _verify_protobuf_call( + http, uri, datastore_pb2.RunQueryRequest()) self.assertEqual(request.partition_id, partition_id) self.assertEqual(request.query, query_pb) self.assertEqual(request.read_options, read_options) @@ -480,7 +470,8 @@ def test_run_query_wo_namespace_empty_result(self): ) # Create mock HTTP and client with response. - http = Http({'status': '200'}, rsp_pb.SerializeToString()) + http = _make_requests_session( + [_make_response(content=rsp_pb.SerializeToString())]) client = mock.Mock( _http=http, _base_url='test.invalid', spec=['_http', '_base_url']) @@ -491,11 +482,10 @@ def test_run_query_wo_namespace_empty_result(self): # Check the result and verify the callers. self.assertEqual(response, rsp_pb) + uri = _build_expected_url(client._base_url, project, 'runQuery') - cw = http._called_with - _verify_protobuf_call(self, cw, uri) - request = datastore_pb2.RunQueryRequest() - request.ParseFromString(cw['body']) + request = _verify_protobuf_call( + http, uri, datastore_pb2.RunQueryRequest()) self.assertEqual(request.partition_id, partition_id) self.assertEqual(request.query, query_pb) self.assertEqual(request.read_options, read_options) @@ -523,7 +513,8 @@ def test_run_query_w_namespace_nonempty_result(self): ) # Create mock HTTP and client with response. - http = Http({'status': '200'}, rsp_pb.SerializeToString()) + http = _make_requests_session( + [_make_response(content=rsp_pb.SerializeToString())]) client = mock.Mock( _http=http, _base_url='test.invalid', spec=['_http', '_base_url']) @@ -534,11 +525,10 @@ def test_run_query_w_namespace_nonempty_result(self): # Check the result and verify the callers. self.assertEqual(response, rsp_pb) - cw = http._called_with + uri = _build_expected_url(client._base_url, project, 'runQuery') - _verify_protobuf_call(self, cw, uri) - request = datastore_pb2.RunQueryRequest() - request.ParseFromString(cw['body']) + request = _verify_protobuf_call( + http, uri, datastore_pb2.RunQueryRequest()) self.assertEqual(request.partition_id, partition_id) self.assertEqual(request.query, query_pb) @@ -551,7 +541,8 @@ def test_begin_transaction(self): rsp_pb.transaction = transaction # Create mock HTTP and client with response. - http = Http({'status': '200'}, rsp_pb.SerializeToString()) + http = _make_requests_session( + [_make_response(content=rsp_pb.SerializeToString())]) client = mock.Mock( _http=http, _base_url='test.invalid', spec=['_http', '_base_url']) @@ -561,12 +552,11 @@ def test_begin_transaction(self): # Check the result and verify the callers. self.assertEqual(response, rsp_pb) + uri = _build_expected_url( client._base_url, project, 'beginTransaction') - cw = http._called_with - _verify_protobuf_call(self, cw, uri) - request = datastore_pb2.BeginTransactionRequest() - request.ParseFromString(cw['body']) + request = _verify_protobuf_call( + http, uri, datastore_pb2.BeginTransactionRequest()) # The RPC-over-HTTP request does not set the project in the request. self.assertEqual(request.project_id, u'') @@ -585,7 +575,8 @@ def test_commit_wo_transaction(self): value_pb.string_value = u'Foo' # Create mock HTTP and client with response. - http = Http({'status': '200'}, rsp_pb.SerializeToString()) + http = _make_requests_session( + [_make_response(content=rsp_pb.SerializeToString())]) client = mock.Mock( _http=http, _base_url='test.invalid', spec=['_http', '_base_url']) @@ -597,11 +588,9 @@ def test_commit_wo_transaction(self): # Check the result and verify the callers. self.assertEqual(result, rsp_pb) + uri = _build_expected_url(client._base_url, project, 'commit') - cw = http._called_with - _verify_protobuf_call(self, cw, uri) - request = rq_class() - request.ParseFromString(cw['body']) + request = _verify_protobuf_call(http, uri, rq_class()) self.assertEqual(request.transaction, b'') self.assertEqual(list(request.mutations), [mutation]) self.assertEqual(request.mode, rq_class.NON_TRANSACTIONAL) @@ -621,7 +610,8 @@ def test_commit_w_transaction(self): value_pb.string_value = u'Foo' # Create mock HTTP and client with response. - http = Http({'status': '200'}, rsp_pb.SerializeToString()) + http = _make_requests_session( + [_make_response(content=rsp_pb.SerializeToString())]) client = mock.Mock( _http=http, _base_url='test.invalid', spec=['_http', '_base_url']) @@ -633,11 +623,9 @@ def test_commit_w_transaction(self): # Check the result and verify the callers. self.assertEqual(result, rsp_pb) + uri = _build_expected_url(client._base_url, project, 'commit') - cw = http._called_with - _verify_protobuf_call(self, cw, uri) - request = rq_class() - request.ParseFromString(cw['body']) + request = _verify_protobuf_call(http, uri, rq_class()) self.assertEqual(request.transaction, b'xact') self.assertEqual(list(request.mutations), [mutation]) self.assertEqual(request.mode, rq_class.TRANSACTIONAL) @@ -650,7 +638,8 @@ def test_rollback_ok(self): rsp_pb = datastore_pb2.RollbackResponse() # Create mock HTTP and client with response. - http = Http({'status': '200'}, rsp_pb.SerializeToString()) + http = _make_requests_session( + [_make_response(content=rsp_pb.SerializeToString())]) client = mock.Mock( _http=http, _base_url='test.invalid', spec=['_http', '_base_url']) @@ -660,11 +649,10 @@ def test_rollback_ok(self): # Check the result and verify the callers. self.assertEqual(response, rsp_pb) + uri = _build_expected_url(client._base_url, project, 'rollback') - cw = http._called_with - _verify_protobuf_call(self, cw, uri) - request = datastore_pb2.RollbackRequest() - request.ParseFromString(cw['body']) + request = _verify_protobuf_call( + http, uri, datastore_pb2.RollbackRequest()) self.assertEqual(request.transaction, transaction) def test_allocate_ids_empty(self): @@ -674,7 +662,8 @@ def test_allocate_ids_empty(self): rsp_pb = datastore_pb2.AllocateIdsResponse() # Create mock HTTP and client with response. - http = Http({'status': '200'}, rsp_pb.SerializeToString()) + http = _make_requests_session( + [_make_response(content=rsp_pb.SerializeToString())]) client = mock.Mock( _http=http, _base_url='test.invalid', spec=['_http', '_base_url']) @@ -685,11 +674,10 @@ def test_allocate_ids_empty(self): # Check the result and verify the callers. self.assertEqual(response, rsp_pb) self.assertEqual(list(response.keys), []) + uri = _build_expected_url(client._base_url, project, 'allocateIds') - cw = http._called_with - _verify_protobuf_call(self, cw, uri) - request = datastore_pb2.AllocateIdsRequest() - request.ParseFromString(cw['body']) + request = _verify_protobuf_call( + http, uri, datastore_pb2.AllocateIdsRequest()) self.assertEqual(list(request.keys), []) def test_allocate_ids_non_empty(self): @@ -709,7 +697,8 @@ def test_allocate_ids_non_empty(self): rsp_pb.keys.add().CopyFrom(after_key_pbs[1]) # Create mock HTTP and client with response. - http = Http({'status': '200'}, rsp_pb.SerializeToString()) + http = _make_requests_session( + [_make_response(content=rsp_pb.SerializeToString())]) client = mock.Mock( _http=http, _base_url='test.invalid', spec=['_http', '_base_url']) @@ -720,29 +709,28 @@ def test_allocate_ids_non_empty(self): # Check the result and verify the callers. self.assertEqual(list(response.keys), after_key_pbs) self.assertEqual(response, rsp_pb) + uri = _build_expected_url(client._base_url, project, 'allocateIds') - cw = http._called_with - _verify_protobuf_call(self, cw, uri) - request = datastore_pb2.AllocateIdsRequest() - request.ParseFromString(cw['body']) + request = _verify_protobuf_call( + http, uri, datastore_pb2.AllocateIdsRequest()) self.assertEqual(len(request.keys), len(before_key_pbs)) for key_before, key_after in zip(before_key_pbs, request.keys): self.assertEqual(key_before, key_after) -class Http(object): +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 - _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 +def _make_requests_session(responses): + session = mock.create_autospec(requests.Session, instance=True) + session.request.side_effect = responses + return session def _build_expected_url(api_base_url, project, method): @@ -765,16 +753,20 @@ def _make_key_pb(project, id_=1234): return Key(*path_args, project=project).to_protobuf() -def _verify_protobuf_call(testcase, called_with, uri): +def _verify_protobuf_call(http, expected_url, pb): from google.cloud import _http as connection_module from google.cloud.datastore._http import _CLIENT_INFO - testcase.assertEqual(called_with['uri'], uri) - testcase.assertEqual(called_with['method'], 'POST') expected_headers = { 'Content-Type': 'application/x-protobuf', 'User-Agent': connection_module.DEFAULT_USER_AGENT, - 'Content-Length': str(len(called_with['body'])), connection_module.CLIENT_INFO_HEADER: _CLIENT_INFO, } - testcase.assertEqual(called_with['headers'], expected_headers) + + http.request.assert_called_once_with( + method='POST', url=expected_url, headers=expected_headers, + data=mock.ANY) + + data = http.request.mock_calls[0][2]['data'] + pb.ParseFromString(data) + return pb diff --git a/dns/google/cloud/dns/client.py b/dns/google/cloud/dns/client.py index 1984c3d1a247..4025f7e9eb68 100644 --- a/dns/google/cloud/dns/client.py +++ b/dns/google/cloud/dns/client.py @@ -36,10 +36,10 @@ class Client(ClientWithProject): passed), falls back to the default inferred from the environment. - :type _http: :class:`~httplib2.Http` + :type _http: :class:`~requests.Session` :param _http: (Optional) HTTP object to make requests. Can be any object that defines ``request()`` with the same interface as - :meth:`~httplib2.Http.request`. If not passed, an + :meth:`requests.Session.request`. If not passed, an ``_http`` object is created that is bound to the ``credentials`` for the current object. This parameter should be considered private, and could diff --git a/dns/google/cloud/dns/zone.py b/dns/google/cloud/dns/zone.py index 3c589d493311..4c278e903862 100644 --- a/dns/google/cloud/dns/zone.py +++ b/dns/google/cloud/dns/zone.py @@ -219,7 +219,7 @@ def _require_client(self, client): def _set_properties(self, api_response): """Update properties from resource in body of ``api_response`` - :type api_response: httplib2.Response + :type api_response: dict :param api_response: response returned from an API call """ self._properties.clear() diff --git a/dns/tests/unit/test__http.py b/dns/tests/unit/test__http.py index 98264e2abe30..4a6c57415655 100644 --- a/dns/tests/unit/test__http.py +++ b/dns/tests/unit/test__http.py @@ -52,31 +52,34 @@ def test_build_api_url_w_extra_query_params(self): self.assertEqual(parms['bar'], 'baz') def test_extra_headers(self): + import requests + from google.cloud import _http as base_http from google.cloud.dns import _http as MUT - http = mock.Mock(spec=['request']) - response = mock.Mock(status=200, spec=['status']) - data = b'brent-spiner' - http.request.return_value = response, data + http = mock.create_autospec(requests.Session, instance=True) + response = requests.Response() + response.status_code = 200 + response_data = b'brent-spiner' + response._content = response_data + http.request.return_value = response client = mock.Mock(_http=http, spec=['_http']) conn = self._make_one(client) req_data = 'req-data-boring' result = conn.api_request( 'GET', '/rainbow', data=req_data, expect_json=False) - self.assertEqual(result, data) + self.assertEqual(result, response_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, ) diff --git a/docs/conf.py b/docs/conf.py index 8aa99a9753de..86ee7d427928 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -295,14 +295,11 @@ autoclass_content = 'both' # Configuration for intersphinx: -# Refer to the Python standard library and the oauth2client and -# httplib2 libraries. intersphinx_mapping = { 'google-auth': ('https://google-auth.readthedocs.io/en/stable', None), 'google-gax': ('https://gax-python.readthedocs.io/en/latest/', None), 'grpc': ('http://www.grpc.io/grpc/python/', None), - 'httplib2': ('http://httplib2.readthedocs.io/en/latest/', None), + 'requests': ('http://docs.python-requests.org/en/master/', None), 'pandas': ('http://pandas.pydata.org/pandas-docs/stable/', None), 'python': ('https://docs.python.org/3', None), - 'oauth2client': ('http://oauth2client.readthedocs.io/en/latest', None), } diff --git a/docs/core/auth.rst b/docs/core/auth.rst index ac3c4b29528a..3c2cc2b0c4ca 100644 --- a/docs/core/auth.rst +++ b/docs/core/auth.rst @@ -299,52 +299,3 @@ you add the correct scopes for the APIs you want to access: * ``https://www.googleapis.com/auth/devstorage.read_write`` .. _set up the GCE instance: https://cloud.google.com/compute/docs/authentication#using - -Advanced Customization -====================== - -.. warning:: - - The developers of this library want to improve our HTTP handling to - support more situations more easily, and use current tooling. - - In order to allow this, this particular mechanism may have to be altered - in a backwards-compatible way. Therefore, the following section should - be considered "private API" that is subject to change. - -The ``google-cloud-python`` library uses `google-auth`_ to sign -requests and ``httplib2`` for sending requests. - -.. _google-auth: http://google-auth.readthedocs.io/en/stable/ - -This is not a strict requirement: -The :class:`Client ` constructor accepts an -optional ``_http`` argument in place of a ``credentials`` object. -If passed, all HTTP requests made by the client will use your -custom HTTP object. - -In order for this to be possible, -the ``_http`` object must do two things: - -* Handle authentication on its own -* Define a method ``request()`` that can subsitute for - :meth:`httplib2.Http.request`. - -The entire signature from ``httplib2`` need not be implemented, -we only use it as - -.. code-block:: python - - http.request(uri, method=method_name, body=body, headers=headers) - -For an example of such an implementation, -a ``google-cloud-python`` user created a `custom HTTP class`_ -using the `requests`_ library. - -.. _custom HTTP class: https://github.com/GoogleCloudPlatform/google-cloud-python/issues/908#issuecomment-110811556 -.. _requests: http://www.python-requests.org/en/latest/ - -We hope to enable using `custom HTTP libraries`_ with this library at -some point. - -.. _custom HTTP libraries: https://github.com/google/oauth2client/issues/128 diff --git a/error_reporting/google/cloud/error_reporting/_logging.py b/error_reporting/google/cloud/error_reporting/_logging.py index d8bd7a12a477..5d7fd3ff3853 100644 --- a/error_reporting/google/cloud/error_reporting/_logging.py +++ b/error_reporting/google/cloud/error_reporting/_logging.py @@ -37,8 +37,10 @@ class _ErrorReportingLoggingAPI(object): ``_http`` object is passed), falls back to the default inferred from the environment. - :type _http: :class:`httplib2.Http` or class that defines ``request()``. - :param _http: An optional HTTP object to make requests. If not passed, an + :type _http: :class:`~requests.Session` + :param _http: (Optional) HTTP object to make requests. Can be any object + that defines ``request()`` with the same interface as + :meth:`requests.Session.request`. If not passed, an ``_http`` object is created that is bound to the ``credentials`` for the current object. This parameter should be considered private, and could diff --git a/error_reporting/google/cloud/error_reporting/client.py b/error_reporting/google/cloud/error_reporting/client.py index 77c2da631f20..bcd164c03eee 100644 --- a/error_reporting/google/cloud/error_reporting/client.py +++ b/error_reporting/google/cloud/error_reporting/client.py @@ -90,8 +90,10 @@ class Client(ClientWithProject): ``_http`` object is passed), falls back to the default inferred from the environment. - :type _http: :class:`httplib2.Http` or class that defines ``request()``. - :param _http: An optional HTTP object to make requests. If not passed, an + :type _http: :class:`~requests.Session` + :param _http: (Optional) HTTP object to make requests. Can be any object + that defines ``request()`` with the same interface as + :meth:`requests.Session.request`. If not passed, an ``_http`` object is created that is bound to the ``credentials`` for the current object. This parameter should be considered private, and could diff --git a/language/google/cloud/language/client.py b/language/google/cloud/language/client.py index da6ea90c156b..5a1afa6186aa 100644 --- a/language/google/cloud/language/client.py +++ b/language/google/cloud/language/client.py @@ -33,10 +33,10 @@ class Client(client_module.Client): passed), falls back to the default inferred from the environment. - :type _http: :class:`~httplib2.Http` + :type _http: :class:`~requests.Session` :param _http: (Optional) HTTP object to make requests. Can be any object that defines ``request()`` with the same interface as - :meth:`~httplib2.Http.request`. If not passed, an + :meth:`requests.Session.request`. If not passed, an ``_http`` object is created that is bound to the ``credentials`` for the current object. This parameter should be considered private, and could diff --git a/language/tests/unit/test__http.py b/language/tests/unit/test__http.py index 6071c697fd01..0b5ffd8ea9c6 100644 --- a/language/tests/unit/test__http.py +++ b/language/tests/unit/test__http.py @@ -40,13 +40,17 @@ def test_build_api_url(self): self.assertEqual(conn.build_api_url(method), uri) def test_extra_headers(self): + import requests + from google.cloud import _http as base_http from google.cloud.language 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) @@ -56,15 +60,14 @@ 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, ) diff --git a/logging/google/cloud/logging/client.py b/logging/google/cloud/logging/client.py index ca698dde99de..3ce67fba151c 100644 --- a/logging/google/cloud/logging/client.py +++ b/logging/google/cloud/logging/client.py @@ -73,10 +73,10 @@ class Client(ClientWithProject): passed), falls back to the default inferred from the environment. - :type _http: :class:`~httplib2.Http` + :type _http: :class:`~requests.Session` :param _http: (Optional) HTTP object to make requests. Can be any object that defines ``request()`` with the same interface as - :meth:`~httplib2.Http.request`. If not passed, an + :meth:`requests.Session.request`. If not passed, an ``_http`` object is created that is bound to the ``credentials`` for the current object. This parameter should be considered private, and could diff --git a/logging/tests/unit/test__http.py b/logging/tests/unit/test__http.py index 459c0cf304d7..d3e9970cb757 100644 --- a/logging/tests/unit/test__http.py +++ b/logging/tests/unit/test__http.py @@ -43,13 +43,17 @@ def test_default_url(self): self.assertIs(conn._client, client) def test_extra_headers(self): + import requests + from google.cloud import _http as base_http from google.cloud.logging 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) @@ -59,17 +63,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, ) diff --git a/logging/tests/unit/test_client.py b/logging/tests/unit/test_client.py index 1655dd7ad1c6..37bfc5c18214 100644 --- a/logging/tests/unit/test_client.py +++ b/logging/tests/unit/test_client.py @@ -560,13 +560,13 @@ def test_list_metrics_with_paging(self): }) def test_get_default_handler_app_engine(self): - import httplib2 + import requests import os from google.cloud._testing import _Monkey from google.cloud.logging.client import _APPENGINE_FLEXIBLE_ENV_VM from google.cloud.logging.handlers import AppEngineHandler - http_mock = mock.Mock(spec=httplib2.Http) + http_mock = mock.Mock(spec=requests.Session) credentials = _make_credentials() deepcopy = mock.Mock(return_value=http_mock) @@ -596,10 +596,10 @@ def test_get_default_handler_container_engine(self): self.assertIsInstance(handler, ContainerEngineHandler) def test_get_default_handler_general(self): - import httplib2 + import requests from google.cloud.logging.handlers import CloudLoggingHandler - http_mock = mock.Mock(spec=httplib2.Http) + http_mock = mock.Mock(spec=requests.Session) credentials = _make_credentials() deepcopy = mock.Mock(return_value=http_mock) @@ -613,9 +613,9 @@ def test_get_default_handler_general(self): self.assertIsInstance(handler, CloudLoggingHandler) def test_setup_logging(self): - import httplib2 + import requests - http_mock = mock.Mock(spec=httplib2.Http) + http_mock = mock.Mock(spec=requests.Session) deepcopy = mock.Mock(return_value=http_mock) setup_logging = mock.Mock(spec=[]) diff --git a/monitoring/google/cloud/monitoring/client.py b/monitoring/google/cloud/monitoring/client.py index 7712a072d793..5ce44d8c7e1f 100644 --- a/monitoring/google/cloud/monitoring/client.py +++ b/monitoring/google/cloud/monitoring/client.py @@ -62,10 +62,10 @@ class Client(ClientWithProject): passed), falls back to the default inferred from the environment. - :type _http: :class:`~httplib2.Http` + :type _http: :class:`~requests.Session` :param _http: (Optional) HTTP object to make requests. Can be any object that defines ``request()`` with the same interface as - :meth:`~httplib2.Http.request`. If not passed, an + :meth:`requests.Session.request`. If not passed, an ``_http`` object is created that is bound to the ``credentials`` for the current object. This parameter should be considered private, and could diff --git a/monitoring/tests/unit/test__http.py b/monitoring/tests/unit/test__http.py index d73bcc3498ff..47d1f81311f5 100644 --- a/monitoring/tests/unit/test__http.py +++ b/monitoring/tests/unit/test__http.py @@ -34,13 +34,17 @@ def test_constructor(self): self.assertIs(connection._client, client) def test_extra_headers(self): + import requests + from google.cloud import _http as base_http from google.cloud.monitoring 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) @@ -50,15 +54,14 @@ 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, ) diff --git a/pubsub/google/cloud/pubsub/client.py b/pubsub/google/cloud/pubsub/client.py index 6a7e60a1923d..ae808c038b7c 100644 --- a/pubsub/google/cloud/pubsub/client.py +++ b/pubsub/google/cloud/pubsub/client.py @@ -58,10 +58,10 @@ class Client(ClientWithProject): passed), falls back to the default inferred from the environment. - :type _http: :class:`~httplib2.Http` + :type _http: :class:`~requests.Session` :param _http: (Optional) HTTP object to make requests. Can be any object that defines ``request()`` with the same interface as - :meth:`~httplib2.Http.request`. If not passed, an + :meth:`requests.Session.request`. If not passed, an ``_http`` object is created that is bound to the ``credentials`` for the current object. This parameter should be considered private, and could diff --git a/pubsub/tests/system.py b/pubsub/tests/system.py index d55011a5254e..fd70f44165de 100644 --- a/pubsub/tests/system.py +++ b/pubsub/tests/system.py @@ -19,7 +19,7 @@ from google.gax.errors import GaxError from google.gax.grpc import exc_to_code from grpc import StatusCode -import httplib2 +import requests from google.cloud.environment_vars import PUBSUB_EMULATOR from google.cloud.exceptions import Conflict @@ -53,9 +53,9 @@ def setUpModule(): Config.IN_EMULATOR = os.getenv(PUBSUB_EMULATOR) is not None if Config.IN_EMULATOR: credentials = EmulatorCreds() - http = httplib2.Http() # Un-authorized. - Config.CLIENT = client.Client(credentials=credentials, - _http=http) + http = requests.Session() # Un-authorized. + Config.CLIENT = client.Client( + credentials=credentials, _http=http) else: Config.CLIENT = client.Client() diff --git a/pubsub/tests/unit/test__http.py b/pubsub/tests/unit/test__http.py index d4bbc29dd6dd..794fe093bbb3 100644 --- a/pubsub/tests/unit/test__http.py +++ b/pubsub/tests/unit/test__http.py @@ -102,13 +102,17 @@ def test_build_api_url_w_base_url_override(self): URI) def test_extra_headers(self): + import requests + from google.cloud import _http as base_http from google.cloud.pubsub 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) @@ -118,17 +122,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, ) diff --git a/resource_manager/google/cloud/resource_manager/client.py b/resource_manager/google/cloud/resource_manager/client.py index d2cea6cad2cd..90f187735974 100644 --- a/resource_manager/google/cloud/resource_manager/client.py +++ b/resource_manager/google/cloud/resource_manager/client.py @@ -40,10 +40,10 @@ class Client(BaseClient): passed), falls back to the default inferred from the environment. - :type _http: :class:`~httplib2.Http` + :type _http: :class:`~requests.Session` :param _http: (Optional) HTTP object to make requests. Can be any object that defines ``request()`` with the same interface as - :meth:`~httplib2.Http.request`. If not passed, an + :meth:`requests.Session.request`. If not passed, an ``_http`` object is created that is bound to the ``credentials`` for the current object. This parameter should be considered private, and could diff --git a/resource_manager/tests/unit/test__http.py b/resource_manager/tests/unit/test__http.py index a5e0e4a77666..250dcd6c64ba 100644 --- a/resource_manager/tests/unit/test__http.py +++ b/resource_manager/tests/unit/test__http.py @@ -51,13 +51,17 @@ def test_build_api_url_w_extra_query_params(self): self.assertEqual(parms['bar'], 'baz') def test_extra_headers(self): + import requests + from google.cloud import _http as base_http from google.cloud.resource_manager 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) @@ -67,15 +71,14 @@ 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, ) diff --git a/runtimeconfig/google/cloud/runtimeconfig/client.py b/runtimeconfig/google/cloud/runtimeconfig/client.py index 5921a5d1eb98..fc821345272f 100644 --- a/runtimeconfig/google/cloud/runtimeconfig/client.py +++ b/runtimeconfig/google/cloud/runtimeconfig/client.py @@ -35,10 +35,10 @@ class Client(ClientWithProject): passed), falls back to the default inferred from the environment. - :type _http: :class:`~httplib2.Http` + :type _http: :class:`~requests.Session` :param _http: (Optional) HTTP object to make requests. Can be any object that defines ``request()`` with the same interface as - :meth:`~httplib2.Http.request`. If not passed, an + :meth:`requests.Session.request`. If not passed, an ``_http`` object is created that is bound to the ``credentials`` for the current object. This parameter should be considered private, and could diff --git a/runtimeconfig/google/cloud/runtimeconfig/config.py b/runtimeconfig/google/cloud/runtimeconfig/config.py index 4b85ff5843bf..385b92a31c40 100644 --- a/runtimeconfig/google/cloud/runtimeconfig/config.py +++ b/runtimeconfig/google/cloud/runtimeconfig/config.py @@ -128,7 +128,7 @@ def _require_client(self, client): def _set_properties(self, api_response): """Update properties from resource in body of ``api_response`` - :type api_response: httplib2.Response + :type api_response: dict :param api_response: response returned from an API call """ self._properties.clear() diff --git a/runtimeconfig/tests/unit/test__http.py b/runtimeconfig/tests/unit/test__http.py index 324994bd4ef6..c4419e165f4e 100644 --- a/runtimeconfig/tests/unit/test__http.py +++ b/runtimeconfig/tests/unit/test__http.py @@ -34,13 +34,17 @@ def test_default_url(self): self.assertIs(conn._client, client) def test_extra_headers(self): + import requests + from google.cloud import _http as base_http from google.cloud.runtimeconfig 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) @@ -50,15 +54,14 @@ 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, ) diff --git a/speech/google/cloud/speech/client.py b/speech/google/cloud/speech/client.py index 7c066d48cb9d..df0bee95461b 100644 --- a/speech/google/cloud/speech/client.py +++ b/speech/google/cloud/speech/client.py @@ -39,10 +39,10 @@ class Client(BaseClient): passed), falls back to the default inferred from the environment. - :type _http: :class:`~httplib2.Http` + :type _http: :class:`~requests.Session` :param _http: (Optional) HTTP object to make requests. Can be any object that defines ``request()`` with the same interface as - :meth:`~httplib2.Http.request`. If not passed, an + :meth:`requests.Session.request`. If not passed, an ``_http`` object is created that is bound to the ``credentials`` for the current object. This parameter should be considered private, and could diff --git a/speech/tests/unit/test__http.py b/speech/tests/unit/test__http.py index fd39428ffdbb..4aa7613910e9 100644 --- a/speech/tests/unit/test__http.py +++ b/speech/tests/unit/test__http.py @@ -40,13 +40,17 @@ def test_build_api_url(self): self.assertEqual(conn.build_api_url(method), uri) def test_extra_headers(self): + import requests + from google.cloud import _http as base_http from google.cloud.speech 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) @@ -56,12 +60,14 @@ 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, headers=expected_headers, method='GET', - uri=expected_uri) + data=req_data, + headers=expected_headers, + method='GET', + url=expected_uri, + ) diff --git a/storage/google/cloud/storage/batch.py b/storage/google/cloud/storage/batch.py index 0ab95a98743c..30847ab12302 100644 --- a/storage/google/cloud/storage/batch.py +++ b/storage/google/cloud/storage/batch.py @@ -23,10 +23,11 @@ import io import json -import httplib2 +import requests import six -from google.cloud.exceptions import make_exception +from google.cloud import _helpers +from google.cloud import exceptions from google.cloud.storage._http import Connection @@ -70,11 +71,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 +119,21 @@ def __setitem__(self, key, value): raise KeyError('Cannot set %r -> %r on a future' % (key, value)) +class _FutureResponse(requests.Response): + """Reponse that returns a placeholder dictionary for a batched requests.""" + def __init__(self, future_dict): + super(_FutureResponse, self).__init__() + self._future_dict = future_dict + self.status_code = 204 + + def json(self): + return self._future_dict + + @property + def content(self): + return self._future_dict + + class Batch(Connection): """Proxy an underlying connection, batching up change operations. @@ -171,7 +182,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 +229,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 +255,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 +277,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 +285,14 @@ 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'] - if not isinstance(content_type, six.binary_type): - content_type = content_type.encode('utf-8') + content_type = _helpers._to_bytes( + response.headers.get('content-type', '')) + faux_message = b''.join([ b'Content-Type: ', content_type, b'\nMIME-Version: 1.0\n\n', - content, + response.content, ]) if six.PY2: @@ -291,20 +301,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). + :meth:`requests.Session.request`. - :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 +321,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/google/cloud/storage/client.py b/storage/google/cloud/storage/client.py index 93785e05269f..42b4bb7d9592 100644 --- a/storage/google/cloud/storage/client.py +++ b/storage/google/cloud/storage/client.py @@ -38,10 +38,10 @@ class Client(ClientWithProject): passed), falls back to the default inferred from the environment. - :type _http: :class:`~httplib2.Http` + :type _http: :class:`~requests.Session` :param _http: (Optional) HTTP object to make requests. Can be any object that defines ``request()`` with the same interface as - :meth:`~httplib2.Http.request`. If not passed, an + :meth:`requests.Session.request`. If not passed, an ``_http`` object is created that is bound to the ``credentials`` for the current object. This parameter should be considered private, and could diff --git a/storage/tests/system.py b/storage/tests/system.py index afab659882bf..a89c45edbf25 100644 --- a/storage/tests/system.py +++ b/storage/tests/system.py @@ -17,7 +17,7 @@ import time import unittest -import httplib2 +import requests import six from google.cloud import exceptions @@ -28,9 +28,6 @@ from test_utils.system import unique_resource_id -HTTP = httplib2.Http() - - def _bad_copy(bad_request): """Predicate: pass only exceptions for a failed copyTo.""" err_msg = bad_request.message @@ -426,9 +423,9 @@ def test_create_signed_read_url(self): signed_url = blob.generate_signed_url(expiration, method='GET', client=Config.CLIENT) - response, content = HTTP.request(signed_url, method='GET') - self.assertEqual(response.status, 200) - self.assertEqual(content, self.LOCAL_FILE) + response = requests.get(signed_url) + self.assertEqual(response.status_code, 200) + self.assertEqual(response.content, self.LOCAL_FILE) def test_create_signed_delete_url(self): blob = self.bucket.blob('LogoToSign.jpg') @@ -437,9 +434,9 @@ def test_create_signed_delete_url(self): method='DELETE', client=Config.CLIENT) - response, content = HTTP.request(signed_delete_url, method='DELETE') - self.assertEqual(response.status, 204) - self.assertEqual(content, b'') + response = requests.request('DELETE', signed_delete_url) + self.assertEqual(response.status_code, 204) + self.assertEqual(response.content, b'') # Check that the blob has actually been deleted. self.assertFalse(blob.exists()) 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..c40e4600097a 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,134 @@ 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.json(), _FutureDict) + 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 +295,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 +443,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 +462,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 +493,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 +506,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 +546,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 +560,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 +570,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 +580,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 +622,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 diff --git a/translate/google/cloud/translate_v2/client.py b/translate/google/cloud/translate_v2/client.py index d72993f0fffd..6bddfe3f2553 100644 --- a/translate/google/cloud/translate_v2/client.py +++ b/translate/google/cloud/translate_v2/client.py @@ -47,10 +47,10 @@ class Client(BaseClient): passed), falls back to the default inferred from the environment. - :type _http: :class:`~httplib2.Http` + :type _http: :class:`~requests.Session` :param _http: (Optional) HTTP object to make requests. Can be any object that defines ``request()`` with the same interface as - :meth:`~httplib2.Http.request`. If not passed, an + :meth:`requests.Session.request`. If not passed, an ``_http`` object is created that is bound to the ``credentials`` for the current object. This parameter should be considered private, and could diff --git a/translate/tests/unit/test__http.py b/translate/tests/unit/test__http.py index 2dc6b015d6de..edec628309b0 100644 --- a/translate/tests/unit/test__http.py +++ b/translate/tests/unit/test__http.py @@ -56,13 +56,17 @@ def test_build_api_url_w_extra_query_params(self): self.assertEqual(params, query_params) def test_extra_headers(self): + import requests + from google.cloud import _http as base_http from google.cloud.translate_v2 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) @@ -72,15 +76,14 @@ 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, ) diff --git a/vision/google/cloud/vision/client.py b/vision/google/cloud/vision/client.py index 8dc693006999..e0a0256e47c3 100644 --- a/vision/google/cloud/vision/client.py +++ b/vision/google/cloud/vision/client.py @@ -43,10 +43,10 @@ class Client(ClientWithProject): passed), falls back to the default inferred from the environment. - :type _http: :class:`~httplib2.Http` + :type _http: :class:`~requests.Session` :param _http: (Optional) HTTP object to make requests. Can be any object that defines ``request()`` with the same interface as - :meth:`~httplib2.Http.request`. If not passed, an + :meth:`requests.Session.request`. If not passed, an ``_http`` object is created that is bound to the ``credentials`` for the current object. This parameter should be considered private, and could diff --git a/vision/tests/unit/test__http.py b/vision/tests/unit/test__http.py index ee486e409b8a..ca5d470589bd 100644 --- a/vision/tests/unit/test__http.py +++ b/vision/tests/unit/test__http.py @@ -40,13 +40,17 @@ def test_default_url(self): self.assertEqual(conn._client, client) def test_extra_headers(self): + import requests + from google.cloud import _http as base_http from google.cloud.vision 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) @@ -56,17 +60,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, )